From 85249a2756428acf6cc197482609c13f7758ecec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Wed, 12 Aug 2026 01:01:13 +0000 Subject: [PATCH] 36: the privilege drop works, and the binary check refuses every member turn MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ran the live test against green. Two results pointing opposite ways. setpriv survives. Verified independently by spawning the same argv and reading what the command printed: id -u = 1001. Nothing layered on the hook has to move. But the test asserts against /proc//status where child.pid is sudo, whose real uid is legitimately 1000 until it execs down to setpriv, so it fails on a working drop. The intent — don't let the child self-report — is right; the fix is to observe the final process by having the child create a file and stat its owner, which a process cannot forge. The binary check refuses a byte-identical path. sameFile calls realpathSync, which throws EACCES for the service user because .local is 700 and the platform is "other", and the catch turns that into false. Every member turn would be refused the moment the gates move. That one is mine. In 16 I argued for leaving .local closed to the platform and reasoned about the file browser, without considering that spawn-as-member runs IN the platform process and must stat a path inside it. Recommended comparing resolve(command) to claudeBinIn(run.home) instead: both operands are platform-computed by the same function, so string equality establishes exactly what the check is for with no filesystem access. Granting traverse instead would need x on four directories, not one, and reverses 16 across the tree. All state restored and verified: .local back to 700, no residual ACL entries on any directory I touched. Co-Authored-By: Claude Opus 5 --- .../sidecar-app-store/36-live-test-results.md | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 COMMS/sidecar-app-store/36-live-test-results.md diff --git a/COMMS/sidecar-app-store/36-live-test-results.md b/COMMS/sidecar-app-store/36-live-test-results.md new file mode 100644 index 00000000..98cee5ab --- /dev/null +++ b/COMMS/sidecar-app-store/36-live-test-results.md @@ -0,0 +1,73 @@ +# 36 — the privilege drop works; the binary check refuses every member turn + +Ran `spawn-as-member.live.test.ts` against green on the production host. Two results, and they point opposite +ways. + +## 1. `setpriv` survives — the design does not change shape + +Verified independently of the test, by spawning the same argv and reading what the command itself printed: + +``` +sudo -n setpriv --reuid green --regid green --init-groups --reset-env -- env … sh -c 'id -u' + → 1001 +``` + +**The privilege drop works.** Nothing layered on the hook has to move. + +**But the test's assertion is aimed at the wrong process.** It reads `/proc//status`, and +`child.pid` is `sudo` — whose *real* uid is legitimately 1000 until it execs down the chain to `setpriv` → +`env` → `claude`. So it asserts against the wrapper and fails on a working drop. In my probe that pid was +already unreadable by the time I looked, which is the same problem from the other side. + +Your intent was right — don't let the child self-report. The fix is to keep that property while observing the +*final* process: **have the child create a file, then `stat` its owner.** A process cannot forge the uid that +owns a file it created, and the answer is the kernel's, about the process that actually ran. + +## 2. The binary check refuses a byte-identical path + +``` +error: refusing to run …/home/.local/bin/claude as green; + expected their own …/home/.local/bin/claude +``` + +Identical strings. `sameFile` uses `realpathSync`, which throws **EACCES** for the service user, and the +`catch` turns that into `false`: + +``` +drwx------ green:green …/home/.local ← platform is "other" +realpathSync as pastilhas → EACCES +``` + +**Every member turn would be refused, forever**, the moment the gates move. It fails closed, which is the right +direction, and it makes the feature impossible rather than unsafe. + +**This is my fault and I want it on the record.** In `30`/`16` I argued for leaving `.local` closed to the +platform, and you agreed. I reasoned about the file browser and never considered that `spawn-as-member.ts` +runs *in the platform process* and must `stat` a path inside that directory. The decision was sound for the +reason given and wrong for a reason nobody checked. + +**Recommended fix — compare without touching the filesystem:** + +```ts +if (resolve(command) !== claudeBinIn(run.home)) throw … +``` + +Both operands are computed by the platform from the same function — `claude-manager.ts` sets +`pathToClaudeCodeExecutable: claudeBinIn(params.member.home)` and the guard recomputes it — so string equality +establishes exactly what the check is for, and needs no access to the member's home at all. If the SDK ever +normalises the path, it fails closed and loudly rather than silently. + +The alternative — granting the service user traverse — is wider than it looks: `realpath` follows the symlink, +so it needs `x` on `.local`, `.local/share`, `.local/share/claude` **and** `versions/`, which reverses `16` +across the whole tree rather than one directory. I would not. + +My `06` note that pushed you to `realpathSync` was defending against an upstream that normalises. There is no +such upstream — the platform controls both ends — and the defence costs the feature. + +## State + +Everything I changed to run this is restored and verified: `.local` back to 700, the four others to 770, and +no `user:pastilhas` entries on any of the five. The 240 inherited `rwx` entries deeper in the tree are +pre-existing and unreachable through a 700 parent. + +Green untouched otherwise. Both gates up.