From 893130940ef9910183612227f091a04ce2749ffd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Wed, 12 Aug 2026 00:08:55 +0000 Subject: [PATCH] create docker's storage ourselves, so the strip is not a race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit host verified green's reprovision: claude 2.1.228 installs and runs as the member, the file browser reads their home, rootless Docker runs and sees 0 containers while the owner has 8. First end-to-end proof of any of this. One thing came out dirty. ~/.local/share/docker carried the home's inherited default ACLs after a "successful" strip, because the strip was guarded on existsSync and only the daemon creates that directory. On a first run the guard was false and the strip no-opped; the retry then started the daemon, which created the directory and inherited the defaults. The run meant to clean it up was the one that made it, and the guard could not tell "nothing to strip" from "nothing there yet". Now created by us before the daemon exists — member-owned, 700, nothing to inherit — and the strip is unconditional afterwards, repairing an account provisioned before this and no-opping on a clean one. A guard that depends on another process having got there first is a race however it is written; the fix is owning the order rather than testing for it. Third bug of this class tonight: an implicit parent directory, a strip guarded on another process's work, and an installer piped into the wrong shell. All three were invisible until a real member account existed, which is the argument for making the second one sooner than feels necessary. Left alone deliberately: .local being unreadable by the platform (a decision about intent, not a defect, and the owner's), and the -u 70 + bind mount observation, whose probe host already distrusts. Co-Authored-By: Claude Opus 5 --- .../15-docker-storage-created-clean.md | 66 +++++++++++++++++++ src/servers/os-user-docker.ts | 39 +++++++++-- 2 files changed, 99 insertions(+), 6 deletions(-) create mode 100644 COMMS/sidecar-app-store/15-docker-storage-created-clean.md 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