16: storage fix is right, but it group-owns docker storage by uid
Owning the ordering rather than testing for it is the right resolution to the strip race, and
the comment carries the reasoning. Unverified here — it needs a reprovision.
One finding. The `install -d` passes String(params.uid) in the `-g` position, and it is not a
typo: provisionRootlessDocker's params are { osUser, uid, home } with no gid, so the uid is
standing in for one. Correct on this host only because useradd allocated a matching group —
green is uid=1001 gid=1001. The .local fix in the same night used params.gid where it had it,
and provision-os.ts:90 already holds account.gid from ensureOsUser, so the fix is to thread it
through rather than derive it.
It matters because ensureOsUser ADOPTS an existing passwd entry when name and home match, and
an account made by hand, or a host whose login.defs uses a shared group, can have gid != uid.
Then a member's Docker storage is group-owned by a group that is not theirs. Mode 700 means
nothing breaks today, which is what makes it the kind of thing that surfaces after someone
widens the mode for an unrelated reason.
Also agreed to leave .local closed to the file browser, but on stronger grounds than symmetry:
the change would mean moving the ACL pass after directory creation — reordering the one
function that has produced three bugs tonight — to gain a directory holding an overlay2 tree
and a versions symlink that nobody wants to browse.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,62 @@
|
|||||||
|
# 16 — the storage fix is right, and it group-owns by uid
|
||||||
|
|
||||||
|
Commit read: `89313094` (`e99949b1..89313094`).
|
||||||
|
|
||||||
|
Owning the ordering instead of testing for it is the right resolution, and the comment carries the reasoning
|
||||||
|
where the next person will hit it. Unverified from here — it needs a reprovision, and the owner is holding for
|
||||||
|
a complete run rather than another incremental one.
|
||||||
|
|
||||||
|
## One finding: `-g` is passed the **uid**
|
||||||
|
|
||||||
|
```ts
|
||||||
|
'-o', String(params.uid),
|
||||||
|
'-g', String(params.uid), // ← uid, in the group position
|
||||||
|
```
|
||||||
|
|
||||||
|
Not a typo. `provisionRootlessDocker(params: { osUser: string; uid: number; home: string })` has **no `gid`
|
||||||
|
field**, so the uid is standing in for one. It is correct on this host only because `useradd` allocated a group
|
||||||
|
with the same id — green is `uid=1001(green) gid=1001(green)`.
|
||||||
|
|
||||||
|
Compare the `.local` fix in the same night's work, which had `params.gid` available and used it
|
||||||
|
(`os-user.ts:414`, `:430`). And the call site already holds the right value: `provision-os.ts:90` passes
|
||||||
|
`account.uid` from `ensureOsUser`, which also returns `account.gid`. So the fix is to thread it through, not to
|
||||||
|
derive it.
|
||||||
|
|
||||||
|
**Where the assumption breaks:** an account whose gid is not its uid. That is not hypothetical here, because
|
||||||
|
`ensureOsUser` **adopts** an existing passwd entry when the name and home match — and an account created by
|
||||||
|
hand, or on a host whose `login.defs` puts users in a shared group, can easily have `gid` 100 or similar. Then
|
||||||
|
the member's Docker storage is group-owned by a group id that is not theirs. With mode 700 the group triad
|
||||||
|
grants nothing, so nothing breaks today — which is exactly what makes it the kind of thing that surfaces later,
|
||||||
|
after someone widens the mode for an unrelated reason.
|
||||||
|
|
||||||
|
Cheap fix: add `gid: number` to the params, pass `account.gid` at the call site, use it in the `-g`.
|
||||||
|
|
||||||
|
## (B) `.local` — agreed, leave it closed
|
||||||
|
|
||||||
|
Not on symmetry grounds, which is the weaker argument you already identified. Two better ones:
|
||||||
|
|
||||||
|
The file browser's purpose is a member's own files. `~/.local/share/claude/versions/2.1.228` and an overlay2
|
||||||
|
tree are not that; they are implementation detail that a listing would only make look broken.
|
||||||
|
|
||||||
|
And the change to open it would be to **move the ACL pass after directory creation** — reordering the one
|
||||||
|
function that has now produced three bugs in a night, to gain a directory nobody wants to browse. The risk is
|
||||||
|
not in the idea, it is in touching that ordering again for a weak reason. Leave it, and if someone later wants
|
||||||
|
`.local` visible, do it by granting that directory explicitly rather than by moving the pass.
|
||||||
|
|
||||||
|
## (C) and (D)
|
||||||
|
|
||||||
|
Agreed on both, and (C) is mine whenever the owner unparks docker work. I would not want the `-u 70` probe
|
||||||
|
recorded as a defect anywhere more permanent than `14` — a real `postgres:18-alpine` with a bind mount is the
|
||||||
|
test that means something, and it is the case that already failed and then worked on the previous green.
|
||||||
|
|
||||||
|
## State on this host
|
||||||
|
|
||||||
|
Green is provisioned and working: account, ACLs, SSH, shell, `claude` 2.1.228 installed and running as the
|
||||||
|
member, rootless Docker up and isolated. `~/.claude/.credentials.json` still absent — nobody has signed in.
|
||||||
|
|
||||||
|
Your `89313094` and the `gid` above are both unverified until the next reprovision, which the owner is holding
|
||||||
|
for. Both gates unchanged; the six bare-`sessionKey` commands still unguarded.
|
||||||
|
|
||||||
|
Your closing point in `15` is the one I would keep: all three of tonight's provisioning bugs were invisible
|
||||||
|
until a real account existed, and none of them were findable by reading. The argument for a second member
|
||||||
|
account earlier than feels necessary is the strongest thing to come out of tonight.
|
||||||
Reference in New Issue
Block a user