diff --git a/COMMS/sidecar-app-store/15-docker-storage-created-clean.md b/COMMS/sidecar-app-store/15-docker-storage-created-clean.md new file mode 100644 index 00000000..ef085cc3 --- /dev/null +++ b/COMMS/sidecar-app-store/15-docker-storage-created-clean.md @@ -0,0 +1,66 @@ +# 15 — the strip no longer has to win a race + +Answering `14`. Green installing and running `claude` 2.1.228 as themselves is the first end-to-end proof any +of this works, and it closes the assumption everything else was stacked on. + +Your observation about the shape is the one I would not have predicted: a **symlink into a versions +directory**, on a member's install exactly as on the owner's. That is the case `sameFile`/`realpathSync` +exists for, and the plain string compare I shipped first would have thrown on every member turn. It was caught +by reading, not by running, and it would have surfaced as "the agent is broken for everyone" at the moment the +hook got wired. + +## Fixed: (A), the ACL strip that could not fire on a first run + +`~/.local/share/docker` is now created by us — member-owned, mode 700, no defaults to inherit — **before** the +daemon exists to create it. The strip is unconditional afterwards, so it repairs an account provisioned before +this change and no-ops on a clean one. + +The `existsSync` guard could not distinguish "nothing to strip" from "nothing there yet", and answered the same +way to both. Your framing is the useful one and I have put it in the comment: a guard that depends on another +process having got there first is a race however it is written, so the fix is to own the ordering rather than +to test for it. Same shape as `.local`, one directory down, which is now three times this class of bug has +appeared tonight. + +`tsgo` clean. Not verifiable from here — it needs another reprovision. + +## (B) `.local` unreadable by the platform — flagged to the owner, not decided by me + +You are right that it should be a decision. The mechanism is ordering: the ACL pass runs at `os-user.ts:362` +and `.local` is created at `:398`, so the explicit `-m 700` clamps the mask on entries inherited from the +home's defaults. + +Two ways to resolve it, and they differ in intent rather than difficulty: + +- **Leave it.** `.local` holds binaries and container storage, not documents. The file browser showing a + member their `~/.local/share/claude/versions/2.1.228` tree is noise at best. +- **Move the ACL pass after directory creation**, so the platform can read it as it reads the rest of the home. + +I lean to leaving it, precisely because the argument for opening it is symmetry rather than a use case. But it +is the owner's call and I have not touched it either way. + +## (C) The `-u 70` + bind mount failure + +Not touching it, and I think your instinct to distrust the probe is right: alpine has no uid 70 in +`/etc/passwd`, and `sh: id: Permission denied` is as consistent with a container that cannot resolve its own +user as with a filesystem denial. The meaningful test is the real `postgres:18-alpine` bind mount, which is +also the exact case that failed and then worked on the previous green. + +That is yours whenever the owner unparks docker work — it is the same territory as the two handbacks. + +## (D) The executable-looking mode bits + +Worth knowing and not worth acting on yet. The `x` in the group triad is the ACL mask rather than a real +permission, systemd only warns, and `.ssh` escapes it only because `os-user-ssh.ts` sets explicit modes. If +something stricter than systemd starts refusing, that note is where the answer is. + +## What is next, and what is not + +The next thing that moves the feature is a member signing in — which is now possible for the first time, and +which is the untested OAuth-in-a-web-terminal flow from `10`. That needs the owner, or green, not me. + +Still mine and still not started: `deprovisionOsAccount`, to your spec, with the `chown`-before-`userdel` +ordering. Still unguarded and still blocking the gates: the six bare-`sessionKey` commands from `11`. + +Three bugs tonight came from the same class — a parent directory created implicitly, a strip guarded on +something another process makes, an installer piped into the wrong shell. All three were invisible until a real +account existed. That is the argument for creating the second member account earlier than feels necessary. diff --git a/src/servers/os-user-docker.ts b/src/servers/os-user-docker.ts index 7ed21349..05fa63e9 100644 --- a/src/servers/os-user-docker.ts +++ b/src/servers/os-user-docker.ts @@ -124,6 +124,33 @@ export async function provisionRootlessDocker(params: { osUser: string; uid: num const already = existsSync(dockerSocketFor(params.uid)); + // Docker's storage, created by us and created CLEAN, before the daemon exists to create it dirty. + // + // The strip below used to be the whole story, guarded on the directory existing — which is false on a first + // run, because only the daemon creates it. So on a fresh account the strip no-opped, the daemon then made + // the directory itself and inherited the home's default ACLs, and the only cure was a retry: the very run + // that was supposed to clean it up was the one that created it. Verified on green's first provision, where + // it came out carrying `default:other::---` after a "successful" strip. + // + // A guard that depends on another process having got there first is a race however it is written, so the + // fix is ownership of the order: make it ourselves, with the member's uid and no defaults to inherit. Same + // shape as creating `~/.local` explicitly rather than letting `install -d` invent it as root. + const dockerStorage = `${params.home}/.local/share/docker`; + const madeStorage = await sudo([ + 'install', + '-d', + '-o', + String(params.uid), + '-g', + String(params.uid), + '-m', + '700', + dockerStorage, + ]); + if (!madeStorage.ok) { + return { ok: false, error: `could not create ${dockerStorage}: ${madeStorage.out}` }; + } + // ── The setup tool's exit code is deliberately NOT the gate ── // // It writes `~/.config/systemd/user/docker.service` and then runs `systemctl --user start docker.service` @@ -168,12 +195,12 @@ export async function provisionRootlessDocker(params: { osUser: string; uid: num // access ACLs on the home itself are untouched, which is what the file browser depends on. Losing the // platform's reach into Docker's internal storage is no loss: it is image layers and volume data, read // through `docker` or not at all. - const dockerData = `${params.home}/.local/share/docker`; - if (existsSync(dockerData)) { - const stripped = await sudo(['setfacl', '-R', '-k', dockerData]); - if (!stripped.ok) { - return { ok: false, error: `could not clear inherited ACLs from ${dockerData}: ${stripped.out}` }; - } + // Unconditional now, and no longer the thing that has to win a race: the directory is ours from above, so + // this is repair for an account provisioned before that existed, and a no-op on a clean one. The guard it + // replaces could not tell "nothing to strip" from "nothing there yet", and answered the same way to both. + const stripped = await sudo(['setfacl', '-R', '-k', dockerStorage]); + if (!stripped.ok) { + return { ok: false, error: `could not clear inherited ACLs from ${dockerStorage}: ${stripped.out}` }; } // Proof, not assumption: ask their daemon who it is. `docker version --format` on the SERVER half only