The plumbing is right where it matters: userId comes from ws.data, the authenticated socket, never the client message. Gates up, tests pass, path inert. The resolution is not. Three inputs collapse to undefined, and undefined means "run as the server owner": the caller IS the owner (correct), resolveHomeDir FAILED, and the account has no os_user. The last two are "I could not determine whose this is", and they are answered with the owner's binary, the owner's ~/.claude credential, the owner's HOME, and — since mcp-config branches on the same field — the owner's MCP config carrying OFFICER_AUTH_TOKEN. 23's own text says the caller must not fall back to running as the owner, and names a wrong answer here as the one thing that must not happen by accident. The code does exactly that. The no-os_user case is not hypothetical. provisionOsAccount is non-fatal at every stage and provision-os.ts records the account either way; provisioning failed three separate ways on a real member tonight while the row continued to exist. Such a member, once the gates lift, does not get an error — they get the owner's agent. Suggested a discriminated result — owner | member | refuse — so that the owner's identity can only be reached by positively establishing it, never by failing to establish anything else. resolveHomeDir already returns isOwner as a positive fact; only the funnel through undefined throws it away. Of everything tonight this is the one I would least want to discover after the gates moved, and I would fix it before the history layer: that one is a correctness bug when it lands wrong, this is a credential boundary that fails silently and looks like success. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4.1 KiB
24 — resolveMemberRun fails open to the owner, and your own comment says it must not
Commit read: 311b2ea5 (b646140d..311b2ea5).
The plumbing is right where it matters most: userId comes from const { email, username, userId } = ws.data — the authenticated socket, never the client message. Same rule as the pty sidecar. Gates up, 25
registry tests pass, and the path is genuinely inert today.
But the resolution itself fails in the dangerous direction, and this is the one place where that matters more than anywhere else in the feature.
Three cases collapse into one, and two of them mean "I don't know"
async function resolveMemberRun(userId: number) {
const resolved = await resolveHomeDir(userId);
if (!resolved.ok || resolved.isOwner) return undefined; // ← two very different things
const row = await getUserById(userId);
if (!row?.osUser) return undefined; // ← a third
return { osUser: row.osUser, home: resolved.home };
}
undefined means "run this turn as the server owner". Three inputs produce it:
- the caller is the owner — correct
resolveHomeDirfailed — a database blip, a row that could not be read: unknown- the account has no
osUser— a member whose Linux provisioning did not complete: unknown
Cases 2 and 3 are not "this is the owner". They are "I could not determine whose this is", and the code
answers them with the owner's identity — the owner's binary, the owner's ~/.claude credential, the owner's
HOME, and (because mcp-config branches on the same params.member) the owner's MCP config carrying
OFFICER_AUTH_TOKEN.
Your 23 names this exactly:
"the caller must not fall back to running it as the owner — undefined means 'the owner', so the failure mode of a wrong answer here is the one thing that must not happen by accident."
The sentence is right and the code does the opposite. You identified the hazard and then implemented it.
Case 3 is not hypothetical
provisionOsAccount is deliberately non-fatal at every stage, and the account row is recorded either way —
provision-os.ts says so: "The Linux account is recorded either way." Tonight alone, provisioning failed
three separate ways on a real member while the account continued to exist. A member sitting in that state, once
the gates lift, does not get an error. They get the owner's agent.
The attack version is worse but less likely than the accident version, and the accident version is a Tuesday.
Fix: make "unknown" impossible to confuse with "owner"
A discriminated result rather than a nullable one:
type TurnIdentity =
| { kind: 'owner' }
| { kind: 'member'; run: MemberRun }
| { kind: 'refuse'; reason: string };
isOwner→owner- resolved member with
osUser→member - everything else →
refuse, and the caller ends the turn with the reason rather than spawning anything
The property to hold onto: the owner's identity should only ever be reached by positively establishing that
the caller is the owner, never by failing to establish anything else. resolveHomeDir already returns
isOwner as a positive fact, so the information is there — it is only the funnel through undefined that
throws it away.
Worth a test in the shape of the spawn-as-member ones: a userId whose resolveHomeDir fails, and one whose
row has a null osUser, both asserting refuse rather than owner.
Why I would fix this before the history layer
It is inert only because both gates refuse non-owners, and the entire purpose of 311b2ea5 is to be ready for
the moment they do not. The history layer is a correctness bug when it lands wrong — a member sees the owner's
transcript list. This is a credential boundary, and it fails silently and looks like success.
Also worth stating plainly since we are close to the end: of everything tonight, this is the one I would least want to discover after the gates moved.
Otherwise
tsgo clean, gates unchanged, member still unreachable in production. Nothing else in the diff — the
send-claude-code.ts change is two lines threading the same field, and it inherits whatever resolveMemberRun
decides.