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.