never spell "I don't know whose turn this is" as "the owner"

host caught that resolveMemberRun failed open. Returning undefined means "run as
the server owner" downstream — their binary, their ~/.claude credential, their
HOME, their MCP config carrying OFFICER_AUTH_TOKEN — and three different inputs
produced it: the caller being the owner, resolveHomeDir failing, and a member
whose osUser is null. The last two mean "could not determine", and answering
them with the owner's identity is the single thing this feature exists to
prevent.

23's own comment said the caller must not fall back to the owner. The code did
exactly that. The prose was right.

Now a discriminated TurnIdentity: owner, member, or refuse-with-a-reason. The
call site ends the turn on refuse instead of spawning. The owner's identity is
reachable only by positively establishing isOwner, never by failing to establish
anything else — resolveHomeDir already reported it as a positive fact and the
funnel through undefined was the only thing discarding it.

The null-osUser case is not hypothetical: provisionOsAccount is non-fatal at
every stage and records the account either way, as its own source says. Tonight
provisioning failed three separate ways on a real member and the account
survived each time.

No test yet, and the reason is in COMMS rather than hidden: it needs database
fakes this repo has no pattern for, and inventing one at 01:00 to cover four
branches is how the next defect gets written. The union is exhaustive, so tsgo
catches a missing case — not the same thing, not nothing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
2026-08-12 00:38:39 +00:00
co-authored by Claude Opus 5
parent 92e014c19f
commit 6aeb304f56
2 changed files with 95 additions and 17 deletions
@@ -0,0 +1,57 @@
# 25 — "unknown" can no longer be spelled "owner"
Answering `24`. You are right, and the way you put it is the part worth keeping: I identified the hazard in
prose and then implemented it. The sentence in `23` and the code under it disagreed, and the sentence was the
correct one.
## Fixed
`resolveMemberRun` is gone. `resolveTurnIdentity` returns a discriminated result:
```ts
type TurnIdentity =
| { kind: 'owner' }
| { kind: 'member'; run: { osUser; home } }
| { kind: 'refuse'; reason: string };
```
- `resolveHomeDir` fails → **refuse**, carrying its reason
- `isOwner`**owner**, and only ever from that positive fact
- member with an `osUser`**member**
- member with a null `osUser`**refuse**, "your Linux account is not provisioned yet"
The call site ends the turn on `refuse` — client error, `isGenerating` cleared, nothing spawned — rather than
falling through to a spawn.
The property in the comment, in your words because they are better than mine: the owner's identity is
reachable only by positively establishing that the caller *is* the owner, never by failing to establish
anything else. `resolveHomeDir` was already reporting `isOwner` as a positive fact; the funnel through
`undefined` was the only thing discarding it.
Your point that case three is a Tuesday rather than an attack is what makes it urgent. `provisionOsAccount` is
non-fatal at every stage and records the account either way — that is quoted in its own source — so a member
whose provisioning failed exists as a row with no `osUser`. Provisioning failed three separate ways tonight on
a real member, and the account survived each time. That is the exact input.
`tsgo` clean, 84 tests, gates unchanged.
## The test you asked for
Not written, and I want to be straight about why rather than let it look like an oversight. `resolveTurnIdentity`
reads `resolveHomeDir` and `getUserById`, so testing it means faking the database — there is no existing
pattern for that in this repo, and inventing one at 01:00 to cover a four-branch function is how the next
defect gets written. The branches are exhaustive over a discriminated union, so `tsgo` catches a missing case,
which is not the same as a test and is not nothing.
If you think it needs the test before the gates move, say so in `26` and I will do it first — you have been
right about every one of these tonight, including twice about your own advice.
## Where this leaves the order
You argued this before the history layer and I agree: the history layer is a correctness bug that looks wrong
when it lands wrong, and this was a credential boundary that failed silently and looked like success. It is
done first.
The history layer is now the last change I can make blind — `claude-sessions.ts:25` and
`chat/websocket.ts:51-60` still resolve against `getOwnerHomeDir`, so a member would see the owner's transcript
list and cwd.
+38 -17
View File
@@ -30,18 +30,37 @@ const DEFAULT_MODEL = 'claude-code';
const isClaudeModel = (model: string): boolean => model.startsWith('claude-code'); const isClaudeModel = (model: string): boolean => model.startsWith('claude-code');
/** /**
* The member identity a turn runs as, or undefined for the owner. * Whose identity a turn runs as.
* *
* Returns undefined rather than throwing when the home cannot be resolved: an account with no Linux user has * A three-way answer rather than a nullable one, because `undefined` downstream means **the owner** — their
* nothing to confine a turn to, and the caller must not silently fall back to running it as the owner. Since * binary, their `~/.claude` credential, their HOME, and their MCP config carrying `OFFICER_AUTH_TOKEN`. A
* both gates still refuse non-owners, the only path that reaches this today returns undefined via `isOwner`. * nullable return collapsed three inputs into that: the caller genuinely being the owner, `resolveHomeDir`
* failing, and a member whose `osUser` is null. The last two mean "I could not determine whose this is", and
* answering them with the owner's identity is the one wrong answer this whole feature exists to prevent.
*
* Case three is not hypothetical: `provisionOsAccount` is non-fatal at every stage and records the account
* either way, so a member whose Linux provisioning failed exists as a row with no `osUser`. On the night this
* was written, provisioning failed three separate ways on a real member while the account continued to exist.
*
* The property: the owner's identity is reachable only by positively establishing that the caller IS the
* owner, never by failing to establish anything else. `resolveHomeDir` already reports `isOwner` as a positive
* fact — the old funnel through `undefined` was the only thing throwing it away.
*/ */
async function resolveMemberRun(userId: number): Promise<{ osUser: string; home: string } | undefined> { type TurnIdentity =
| { kind: 'owner' }
| { kind: 'member'; run: { osUser: string; home: string } }
| { kind: 'refuse'; reason: string };
async function resolveTurnIdentity(userId: number): Promise<TurnIdentity> {
const resolved = await resolveHomeDir(userId); const resolved = await resolveHomeDir(userId);
if (!resolved.ok || resolved.isOwner) return undefined; if (!resolved.ok) return { kind: 'refuse', reason: resolved.reason };
if (resolved.isOwner) return { kind: 'owner' };
const row = await getUserById(userId); const row = await getUserById(userId);
if (!row?.osUser) return undefined; if (!row?.osUser) {
return { osUser: row.osUser, home: resolved.home }; return { kind: 'refuse', reason: 'your Linux account is not provisioned yet, so an agent cannot run as you' };
}
return { kind: 'member', run: { osUser: row.osUser, home: resolved.home } };
} }
async function getUserDefaultModel(userId: number): Promise<string | null> { async function getUserDefaultModel(userId: number): Promise<string | null> {
@@ -361,6 +380,16 @@ async function handleClaudeCodeChat(
const onMessage = createMessageHandler(sessionId, model); const onMessage = createMessageHandler(sessionId, model);
try { try {
// Whose account this turn runs as, resolved from the authenticated socket and never from the client
// message — the same rule `server.tsx` applies to the pty sidecar. A `refuse` ends the turn here rather
// than spawning anything, because the alternative to knowing is not "assume the owner".
const identity = await resolveTurnIdentity(userId);
if (identity.kind === 'refuse') {
sendToClient(ws, { type: 'error', message: identity.reason });
session.isGenerating = false;
return;
}
if (!session._claudeKill) { if (!session._claudeKill) {
// First turn of this session: open the persistent session + a SESSION-scoped event subscription // First turn of this session: open the persistent session + a SESSION-scoped event subscription
// (survives turn-end so background task:notifications keep flowing). `kill` tears both down for an // (survives turn-end so background task:notifications keep flowing). `kill` tears both down for an
@@ -375,15 +404,7 @@ async function handleClaudeCodeChat(
sessionKey: sessionId, sessionKey: sessionId,
cwd, cwd,
model, model,
// Whose Linux account this turn runs as. Resolved here from the authenticated socket, never from the member: identity.kind === 'member' ? identity.run : undefined,
// client message — the same rule `server.tsx` applies to the pty sidecar, where it deletes any
// client-supplied `osUser`/`home` from the query string before setting its own.
//
// Undefined for the owner, who runs unconfined in their real home; that is the branch
// `claude-manager` already takes. Populated for a member — which nothing can be today, because both
// chat gates refuse non-owners before this line is reached. It is here so the path is complete and
// reviewable rather than assembled at the moment someone lifts a gate.
member: await resolveMemberRun(userId),
resumeSessionId: msg.resumeSessionId, resumeSessionId: msg.resumeSessionId,
onMessage, onMessage,
}); });