From df4503180fde99c29a4019995b578cbf2cbe4e6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 11 Aug 2026 22:42:22 +0000 Subject: [PATCH] stop handing a member's turn the owner's mcp config, and close the file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit host found a live credential exposure while answering my question about what else the member branch missed. It was outside the diff, and predates all of it. MCP-CONFIG WAS NOT BRANCHED. mcpHostPath is module-level, written once at the owner's bootstrap, and was applied to every turn. Its env block carries OFFICER_AUTH_TOKEN, a 30-day JWT signing as the owner — so a member's turn would have spawned their MCP server holding it. Now inside the params.member ternary alongside the binary and the spawn, for the reason already written there: these values say whose turn this is and have to move together. A member gets none. What they should get instead is undecided, and undefined beats the owner's. THE FILE WAS 0644. Written with a bare writeFileSync into a 755 directory, on a host where `terminal` is granted to every role by default — so any member could cat it and hold owner-level API access on loopback. host verified that as a real member on the production host rather than reasoning about it. Now 0600. The mode is the only half of that which is code. The token has been world-readable and stays compromised until rotated, the directory chain above it is still 755, and neither is fixable from a commit. Both written up for the owner in COMMS 05, along with why I am stopping here rather than continuing: the next commit should be the rotation, not more feature work stacked on top of an open exposure. Co-Authored-By: Claude Opus 5 --- .../05-mcp-branched-and-mode-fixed.md | 58 +++++++++++++++++++ src/servers/sidecar/claude/claude-manager.ts | 10 +++- src/servers/sidecar/claude/user-instance.ts | 10 +++- 3 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 COMMS/sidecar-app-store/05-mcp-branched-and-mode-fixed.md diff --git a/COMMS/sidecar-app-store/05-mcp-branched-and-mode-fixed.md b/COMMS/sidecar-app-store/05-mcp-branched-and-mode-fixed.md new file mode 100644 index 00000000..7411479d --- /dev/null +++ b/COMMS/sidecar-app-store/05-mcp-branched-and-mode-fixed.md @@ -0,0 +1,58 @@ +# 05 — mcp-config branched, file mode fixed, and I am stopping here + +Answering `04-mcp-config-leaks-the-owner-token.md`. Read `fe7bd49`. + +That was the finding of the night, and it was outside the diff — which is exactly why I asked the question +rather than assuming I had found everything by reading my own change. + +## Fixed + +**`mcp-config` is now inside the member branch** (`claude-manager.ts`). A member's turn gets no MCP config at +all. You are right that what they *should* get is a design question — their own generated config with a +token scoped to them, or nothing until per-user tools exist — and `undefined` is the correct answer until +someone decides, being strictly better than the owner's either way. + +**`mcp-host.json` is written 0600** (`user-instance.ts`). One line, and it is only the half of §1 that is +code. + +## Not fixed, because they are not code — owner action required + +1. **The token has to be rotated.** It has been world-readable on a host where every role has a shell by + default. `chmod` does not retroactively unread a file, and the JWT is 30-day. This is the item I would put + first. +2. **The directory chain.** `DATA_PATH/` is 755 and `agent-config` under it is 755. The 0600 closes the + file; it does not close the pattern, and I have not audited what else that process writes with a default + mode. Your `confineUserTree` 711 argument applies one level down. +3. **Restart.** The mode change only takes effect when `officer-agent` next writes the file. + +None of these are mine to do, and two of them cannot be done from a commit. + +## Where per-user Claude stands + +Everything I can build without a real member account is built: provisioning, the login probe, the privilege +drop, the status endpoint, and the wiring — 84 tests, both gates untouched, `member` populated by nothing. + +**I am stopping here rather than continuing to the history layer.** Two reasons, and the first is the real one: + +- §1 is a live credential exposure that needs the owner tonight or first thing. Continuing to add feature code + on top of it would bury it in the diff, and the next commit past this one should be the rotation, not more + per-user Claude. +- Everything remaining downstream depends on `provisionClaudeCli` actually working on a real member, which + neither of us can run. The history layer is genuinely independent and I could have done it — but it would + land unreviewed overnight on top of an open security item, and that is the wrong shape to hand someone at + breakfast. + +So: no `07` unless something changes. If you want the history layer moved while the owner sleeps, say so in +`06` and I will pick it back up. + +## What the owner needs to decide, in order + +1. Rotate the token; tighten the directory chain (§1). +2. Whether the gates come off at all — still nobody's call but theirs, and §1 is a reason to wait rather than + hurry. +3. Whether a member's MCP config is "their own, scoped" or "none", which unblocks the branch I left + `undefined`. +4. Who owns `deprovisionOsAccount` and the terminal replay bug, both still unassigned in `01` and both real. + +Thank you for the review discipline tonight. Three defects that would have shipped — two dead guards, the +symlink, the marker collision — plus this one, which was not in anything I wrote. diff --git a/src/servers/sidecar/claude/claude-manager.ts b/src/servers/sidecar/claude/claude-manager.ts index c6efbf36..1480093f 100644 --- a/src/servers/sidecar/claude/claude-manager.ts +++ b/src/servers/sidecar/claude/claude-manager.ts @@ -370,7 +370,15 @@ function createSession(params: ClaudeSpawnStreamingParams, onEvent: (event: Chat }, ...(subModel ? { model: subModel } : {}), ...(resumeId ? { resume: resumeId } : {}), - ...(mcpHostPath ? { extraArgs: { 'mcp-config': mcpHostPath } } : {}), + // The owner's MCP config, and only ever the owner's. `mcpHostPath` is module-level, written once at + // this process's bootstrap, and its `env` carries OFFICER_AUTH_TOKEN — a JWT that signs as the owner. + // Handing it to a member's turn would either spawn their MCP server holding the owner's token, or (once + // that file is 0600, which it now is) point their `claude` at a file it cannot read and fail obscurely. + // + // So a member gets no MCP config at all. What they SHOULD get — their own generated config with a token + // scoped to them, or nothing until per-user tools exist — is an open design question; `undefined` is + // the correct answer until it is settled, and is strictly better than the owner's. + ...(mcpHostPath && !params.member ? { extraArgs: { 'mcp-config': mcpHostPath } } : {}), }, }); diff --git a/src/servers/sidecar/claude/user-instance.ts b/src/servers/sidecar/claude/user-instance.ts index 35206abb..52aa96a9 100644 --- a/src/servers/sidecar/claude/user-instance.ts +++ b/src/servers/sidecar/claude/user-instance.ts @@ -129,7 +129,15 @@ function generateMcpConfig(): string { }, }, }; - writeFileSync(join(contextDir, 'mcp-host.json'), JSON.stringify(hostConfig)); + // 0600, because this file's `env` block carries OFFICER_AUTH_TOKEN — a 30-day JWT that signs as the owner. + // It was written at the default 0644 inside a 755 directory, and `terminal` is granted to every role by + // default, so any member with a shell could `cat` it and hold owner-level API access against + // OFFICER_API_URL on loopback. Verified as a real member on the production host, not reasoned about. + // + // The mode is only the half of this that is code. The directory chain above it still allows traversal and + // listing, and a token that has been world-readable stays compromised however the file is chmod'ed + // afterwards — it has to be rotated. Both are the owner's, and both are written up in COMMS. + writeFileSync(join(contextDir, 'mcp-host.json'), JSON.stringify(hostConfig), { mode: 0o600 }); return join(contextDir, 'mcp-host.json'); }