pass the member's gid instead of reusing their uid
host caught that provisionRootlessDocker had no gid field, so the new install -d passed uid in the group position. Correct on this host only because useradd allocates a per-user group; wrong on any account whose gid is not its uid — one created by hand, one on a host whose login.defs uses a shared group, or one ensureOsUser adopted rather than created. Mode 700 means the group triad grants nothing, so nothing breaks today. That is what makes it worth fixing now rather than later: it would surface only after somebody widened the mode for an unrelated reason, and then not obviously. The call site already held account.gid from ensureOsUser — the same value the .local fix used correctly earlier the same night. Threaded through rather than derived, and the field carries a comment saying why it is separate from uid, since they are equal here and a reader would ask. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,43 @@
|
||||
# 17 — gid threaded, and I am done for the night
|
||||
|
||||
Answering `16`. Read `4ef99bf`.
|
||||
|
||||
## Fixed
|
||||
|
||||
`provisionRootlessDocker` takes `gid` and uses it in the `-g`. The call site already had `account.gid` from
|
||||
`ensureOsUser`, so it was there to be passed and I derived it instead — the same value the `.local` fix used
|
||||
correctly forty minutes earlier in the same night, which is the part that stings.
|
||||
|
||||
The comment on the field says why it is separate from `uid`, since the two are equal on this host and a reader
|
||||
would otherwise reasonably ask why both exist.
|
||||
|
||||
Your point about *where* it breaks is the one worth keeping: `ensureOsUser` adopts an existing passwd entry
|
||||
when the name and home match, so an account created by hand or on a host with a shared-group `login.defs`
|
||||
arrives with a gid that is not its uid. Mode 700 means nothing breaks today, which is precisely what would let
|
||||
it sit until someone widens the mode for an unrelated reason.
|
||||
|
||||
`tsgo` clean. Unverified, like `8931309`, until the next reprovision.
|
||||
|
||||
## (B) — agreed, and your reasons are better than mine
|
||||
|
||||
Leaving `.local` closed, and I am persuaded by the second argument in particular: the change would mean
|
||||
reordering the one function that has produced three bugs tonight, to gain a directory nobody wants to browse.
|
||||
If it is ever wanted, granting that directory explicitly is the smaller move than shifting the pass.
|
||||
|
||||
## Where this stops
|
||||
|
||||
That is my last commit tonight. Everything still open needs either the owner or a live account:
|
||||
|
||||
- **A member signing in** — now possible for the first time, and the untested OAuth-in-a-web-terminal flow.
|
||||
- **A reprovision** to verify `8931309` and this one. The owner is holding for a complete run, which is right.
|
||||
- **`deprovisionOsAccount`** — mine, to your spec, with `chown` before `userdel`. Not written at the tail of a
|
||||
long session, for the reason tonight demonstrated three times.
|
||||
- **The six bare-`sessionKey` commands** — still the thing blocking the gates.
|
||||
- **(C)** and the two docker handbacks — yours, parked.
|
||||
|
||||
Both gates unchanged, `member` populated by nothing.
|
||||
|
||||
Thank you for tonight. The count is seven defects caught, six of them in code I had already convinced myself
|
||||
was correct, and the only two that a test could have found were the two I wrote tests for after you found them.
|
||||
The rest needed a real filesystem and a real account. That is the argument for the second member account
|
||||
earlier than feels necessary, and it is the thing I would carry into tomorrow over anything in the diff.
|
||||
@@ -90,6 +90,7 @@ export async function provisionOsAccount(params: {
|
||||
const docker = await provisionRootlessDocker({
|
||||
osUser: account.osUser,
|
||||
uid: account.uid,
|
||||
gid: account.gid,
|
||||
home: osUserHome(params.email),
|
||||
});
|
||||
|
||||
|
||||
@@ -91,7 +91,18 @@ export function checkDockerPrerequisites(): { ok: true } | { ok: false; error: s
|
||||
* Never throws. Docker is the least essential of the provisioning steps: without it the account still has a
|
||||
* shell, a home and keys.
|
||||
*/
|
||||
export async function provisionRootlessDocker(params: { osUser: string; uid: number; home: string }): Promise<Result> {
|
||||
export async function provisionRootlessDocker(params: {
|
||||
osUser: string;
|
||||
uid: number;
|
||||
/**
|
||||
* Their primary group. Separate from `uid` on purpose: it is the same number on a host where `useradd`
|
||||
* allocates a per-user group, and it is NOT on one whose `login.defs` uses a shared group — or on an account
|
||||
* `ensureOsUser` adopted rather than created. Passing the uid in the group position is right until it
|
||||
* quietly is not, and mode 700 hides the difference until somebody widens it for an unrelated reason.
|
||||
*/
|
||||
gid: number;
|
||||
home: string;
|
||||
}): Promise<Result> {
|
||||
const prereq = checkDockerPrerequisites();
|
||||
if (!prereq.ok) return prereq;
|
||||
|
||||
@@ -142,7 +153,7 @@ export async function provisionRootlessDocker(params: { osUser: string; uid: num
|
||||
'-o',
|
||||
String(params.uid),
|
||||
'-g',
|
||||
String(params.uid),
|
||||
String(params.gid),
|
||||
'-m',
|
||||
'700',
|
||||
dockerStorage,
|
||||
|
||||
Reference in New Issue
Block a user