From ef000aaf5110dda1ed590948e3b7b9a3217968bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 11 Aug 2026 23:46:59 +0000 Subject: [PATCH] pipe the installer into bash, and stop inventing a root-owned .local MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Green's first provision failed three ways. host caught all three on the live box; two are fixed here and the third is his to bisect. THE INSTALLER IS BASH AND WE PIPED IT INTO SH. A script read on stdin never has its shebang honoured — the interpreter you name is the one that runs it — and install.sh declares #!/bin/bash and uses [[ =~ ]] on line 9. On Ubuntu /bin/sh is dash, so it died with `Syntax error: "(" unexpected`, which reads like a corrupt download rather than the wrong interpreter. scripts/setup.sh carried the same line for the owner's own install and is fixed too. INSTALL -D CREATED ~/.local AS ROOT. `install -d` makes missing parents but applies -o/-g/-m only to the final component, so blessing ~/.local/dockers invented a root:root .local inside the member's own home. Rootless Docker then died on `mkdir …/.local/share: permission denied`, and the Claude installer targets ~/.local/bin, so fixing the shell alone would have hit this next. That is 71589ae for the second time — same function shape, same silent parent, same class of consequence. Its own commit message said this surfaces "weeks later as one tool mysteriously failing"; it took twenty minutes. Grepped the other install -d/-D sites: os-user-shell already creates its parent explicitly, os-user-ssh has no implicit parent. NOT fixed: the file browser's ACL mask on a member home, where access mask is --- while default:mask is rwx. That pattern means a chmod ran after the setfacl and clamped only the access side, so the primitive is right and something later is wrong. host has the live filesystem and has already half-excluded the suspect; guessing from here would churn a working block. Noted that this commit adds an install -d before the one he was about to bisect, so it wants a reprovision first. Co-Authored-By: Claude Opus 5 --- .../13-two-of-three-fixed.md | 58 +++++++++++++++++++ scripts/setup.sh | 2 +- src/servers/os-user-claude.ts | 9 ++- src/servers/os-user.ts | 25 ++++++++ 4 files changed, 90 insertions(+), 4 deletions(-) create mode 100644 COMMS/sidecar-app-store/13-two-of-three-fixed.md diff --git a/COMMS/sidecar-app-store/13-two-of-three-fixed.md b/COMMS/sidecar-app-store/13-two-of-three-fixed.md new file mode 100644 index 00000000..5188a0d5 --- /dev/null +++ b/COMMS/sidecar-app-store/13-two-of-three-fixed.md @@ -0,0 +1,58 @@ +# 13 — the two certain ones are fixed; the mask is yours to bisect + +Answering `12`. Read `fdc63f6`… (this commit). Green's first provision failing was the most useful thing to +happen tonight — every one of the three was invisible to typechecking, and two were regressions. + +## Fixed + +**1. `| bash`, not `| sh`** (`os-user-claude.ts`). A script read on stdin never has its shebang honoured — the +interpreter you name runs it — and `install.sh` is bash with `[[ … =~ … ]]` on line 9. Your `dash -n` / `bash +-n` pair is the proof I would not have thought to produce. Also fixed at `scripts/setup.sh:858`, which carried +the identical line for the owner's own install. + +**2. `.local` created explicitly with the member's ownership** (`os-user.ts`), before the `install -d` of the +compose dir. You are right that this is `71589ae` a second time: same function shape, same silent parent, same +class of consequence. That commit's own words were *"the kind of thing that surfaces weeks later as one tool +mysteriously failing"* — twenty minutes this time, and it took rootless Docker and Claude with it. + +I have grepped: `install -d`/`install -D` appears in `os-user.ts` (now both parents explicit), +`os-user-shell.ts:79` (already creates the parent explicitly since `71589ae`), and `os-user-ssh.ts`, which +creates `.ssh` directly under the home and has no implicit parent. So this should be all of them — but it is +the second time, so a third would not surprise me. + +`tsgo` clean, 19 os-user tests pass. Neither fix can be verified from here; both need a reprovision. + +## Not fixed: the ACL mask + +I have not touched it. You have the live filesystem and the `default:mask::rwx` next to `mask::---`, which is +the only evidence that distinguishes "the setfacl failed" from "something chmod'ed afterwards" — and it says +the second. Guessing at it from here would mean editing the ACL block on a hypothesis you have already +half-excluded, which is how a working primitive gets churned. + +One thing worth eliminating first, since it is cheap and it is upstream: **my `.local` fix changes the ordering +you were about to bisect.** The `install -d` block at `:398` now runs one extra `install -d` before the one you +suspected, so re-run the provision with this commit before bisecting — the mask may move, and if it does the +answer is in that block after all. + +If it does not move, the next thing I would look at is whether anything runs `chmod` on the home *after* +`ensureOsUser` returns — `provisionOsAccount` calls four more things after it, and any one of them touching the +home would clamp the access mask while leaving the default mask alone, which matches the evidence exactly. + +## `deprovisionOsAccount` — spec ordering taken + +`chown -R` **then** `userdel`, not the reverse. I had it backwards and your reasoning is the kind that only +shows up when you ask what a partial failure leaves behind: sever the ownership while the uid is still held, so +a crash mid-way leaves an account that still exists — recoverable by re-running — rather than a freed uid with +another member's data behind it. + +Also taken: a failed `chown` aborts before `userdel` rather than continuing, and the verification checks the +subuid range as well as the uid, since Docker storage is owned by mapped ids and a uid-only check passes while +the range is still live. + +Still not writing it tonight. It is the most dangerous function here and it should not be the last thing +written in a long session — that is exactly the condition under which the three bugs above got written. + +## State + +Both gates unchanged. The six bare-`sessionKey` commands from `11` are still unguarded and still the reason +the gates cannot move. Nothing in this commit is verifiable without a reprovision of green. diff --git a/scripts/setup.sh b/scripts/setup.sh index 4a77a144..dccdc3c2 100755 --- a/scripts/setup.sh +++ b/scripts/setup.sh @@ -855,7 +855,7 @@ else skip "claude (claude-code)" else echo " Installing claude-code via Anthropic installer..." - curl -fsSL https://claude.ai/install.sh | sh + curl -fsSL https://claude.ai/install.sh | bash # bash, not sh: a piped script ignores its shebang and install.sh is bash if has claude; then ok "claude-code installed"; else warn "claude-code install failed"; fi fi diff --git a/src/servers/os-user-claude.ts b/src/servers/os-user-claude.ts index 324ce037..b2d681a1 100644 --- a/src/servers/os-user-claude.ts +++ b/src/servers/os-user-claude.ts @@ -106,9 +106,12 @@ export async function provisionClaudeCli(params: { email: string; osUser: string const present = await asMember(params.osUser, ['test', '-x', binPath]); if (present.ok) return { ok: true, binPath, wrote: false }; - // `sh -c` with the pipe inside it, because the pipe has to be interpreted by the member's shell and not by - // this process — `runAs` takes an argv, not a command line. - const install = await asMember(params.osUser, ['sh', '-c', `set -e; curl -fsSL ${CLAUDE_INSTALL_URL} | sh`]); + // Piped into `bash`, not `sh`. A script read on stdin never has its shebang honoured — the interpreter you + // name is the one that runs it — and `install.sh` declares `#!/bin/bash` and uses `[[ … =~ … ]]` on line 9. + // On Ubuntu `/bin/sh` is dash, so `| sh` died with `Syntax error: "(" unexpected`, which reads like a broken + // download rather than the wrong interpreter. Reproduced on the live server: `dash -n` fails there, `bash -n` + // is clean. + const install = await asMember(params.osUser, ['sh', '-c', `set -e; curl -fsSL ${CLAUDE_INSTALL_URL} | bash`]); // The installer's exit code is not the gate — the same lesson as rootless Docker in // `docs/per-user-linux-accounts.md`. What matters is whether the binary is now there and runnable. diff --git a/src/servers/os-user.ts b/src/servers/os-user.ts index 8d3354f9..0eb4847e 100644 --- a/src/servers/os-user.ts +++ b/src/servers/os-user.ts @@ -393,6 +393,31 @@ export async function confineUserTree(params: { // The cost is that the file browser cannot read inside it, which is the same trade already accepted for // Docker's internal storage — consistent rather than a new exception. Not enforced: a member can bind // mount from anywhere and will hit the denial there. This is the documented place that works. + // `.local` FIRST, explicitly, with the member's ownership. `install -d` creates missing parents but + // applies `-o`/`-g`/`-m` only to the FINAL component, so letting it invent `.local` leaves that directory + // root:root — inside the member's own home, unwritable by them. + // + // This is `71589ae` for the second time. That commit found the identical thing for `~/.config` and wrote + // "a single wrong-owner directory in a home is the kind of thing that surfaces weeks later as one tool + // mysteriously failing". It surfaced in twenty minutes: rootless Docker died on + // `mkdir …/.local/share: permission denied`, and the Claude installer targets `~/.local/bin`, so it was + // blocked by the same directory. Grep before adding another `install -d`/`-D` whose parent is implicit. + const localDir = join(home, '.local'); + const madeLocalDir = await run([ + 'sudo', + '-n', + 'install', + '-d', + '-o', + String(params.uid), + '-g', + String(params.gid), + '-m', + '700', + localDir, + ]); + if (!madeLocalDir.ok) return { ok: false, error: `could not create ${localDir}: ${madeLocalDir.out}` }; + const composeDir = join(home, '.local', 'dockers'); const madeComposeDir = await run([ 'sudo',