From 2490547fc6a2c9f41d64e24fe1e86a623561d36c Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 18 Sep 2026 05:04:10 -0700 Subject: [PATCH 1/4] fix(cli): a daemon-provisioned seat is never born unconfined (TASK-052) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The self-serve install ships no environment, the daemon projected that verbatim, and the seat spawned with no sandbox — `sandbox.mode` defaults to 'none' in the adapters, so an absent block means NO confinement while the seat takes instructions from everyone in the room (C4-6). The c4 smoke seat had to be hand-confined through a row PATCH. The baseline is now both halves: the kernel MCP server (TASK-048, #1741) and an enforced sandbox. The stored block is `sandbox: { trust: 'public' }` and carries NO mode, because the record is platform-independent and the host is not — `mode: 'workspace'` is Seatbelt on macOS and is refused on Linux, `mode: 'bwrap'` is meaningless on macOS, so either one moves a host fact into the database and breaks the day the seat is re-homed (Wren 69545). The mode is resolved at spawn instead, in cli/src/lib/sandbox/mode.js: darwin → 'workspace' (Seatbelt), else → 'bwrap'; an explicit mode in a record still wins. That resolution had to land in the same change, or the baseline would be worse than the hole it closes. Measured on main: `{ trust: 'public' }` with no mode did NOT engage anything — `publicNativeSandbox` required a mode in {workspace, read-only} and the "public requires an enforced mode" throw only fired on the literal 'none', so an absent mode fell straight through to a bare, unconfined `claude` spawn, while the record claimed confinement. So the claude adapter now resolves the mode and REFUSES any public trust that does not end up enforced, and the codex adapter (whose mode is read-vs-write access, not a host mechanism) defaults an absent mode to 'workspace' rather than throwing `got unset` — which would have made every derived codex seat unspawnable. Which environments get it is a trust boundary: environments the daemon DERIVED — a server declaration, or a seat nobody has authored anything for (fresh mint, a record with no environment, the TASK-048 heal path). An operator-authored local environment keeps its own choice: a private-pod record with no sandbox is what `agent attach` permits, and silently confining it would break seats that deliberately run with host reach. `withDefaultSandbox` replaces an explicitly disengaged `mode: 'none'` rather than honouring it, because on this path 'none' and absent are the same thing to the spawn. `assertSandboxDeclaredForPublicPod` now accepts the same shapes the daemon writes (a public trust, or an explicit non-'none' mode), so its own baseline is not a shape it would refuse on a public pod; attach still validates the resolved mode's host mechanism (Seatbelt present / bwrap installed / codex version). Tests: 6 new in default-environment.test.mjs (including "stores NO mode"), 5 in daemon-supervisor.test.mjs, 3 in adapters.claude.environment.test.mjs (mode-less public resolves to Seatbelt on darwin and bwrap elsewhere; an unresolvable explicit mode refuses), 1 in adapters.codex.test.mjs, 2 in public-pod-sandbox-gate.test.mjs. Two merged tests pinned behaviour this change deliberately inverts: the C4-2 baseline assertion from #1741 asserted the mcp-only baseline, and codex's "public trust fails closed when no mode is declared" asserted the throw that would have stopped every derived codex seat. Cli suite 39 suites / 550 passed / 10 skipped; lint:cli clean. Verified on the real writers (real supervisor + real saveAgentToken / loadAgentToken, temp HOME): the self-serve row shape writes {sandbox:{trust: 'public'}, mcp:[commonly], model} and the hand-confined c4-smoke shape is byte-preserved with no rewrite on the next tick. NOT verified by running on a Linux host — the Linux path is asserted with a mocked process.platform plus the adapter's own bwrap branch. Confinement holds for claude and codex. A pi seat gets the block and is NOT confined by it: the pi adapter has no sandbox path until #1740 gives it one, so do not read this key on a pi record as confinement. Cli version 0.1.48 → 0.1.49 (main took 0.1.48 with #1726; Wave's #1744 slots this in the cli chain). Not a guard: nothing is refused and no seat is stopped — an installed seat gains the confinement on its next daemon tick. Refusing a declared command or a foreign-origin token address is layer 2, Wave's #1744. --- .../adapters.claude.environment.test.mjs | 73 ++++++++++ cli/__tests__/adapters.codex.test.mjs | 38 +++++- cli/__tests__/daemon-supervisor.test.mjs | 127 ++++++++++++++++++ cli/__tests__/default-environment.test.mjs | 94 +++++++++++++ .../public-pod-sandbox-gate.test.mjs | 21 +++ cli/package.json | 2 +- cli/src/commands/agent.js | 27 ++-- cli/src/lib/adapters/claude.js | 18 ++- cli/src/lib/adapters/codex.js | 8 +- cli/src/lib/daemon-supervisor.js | 27 +++- cli/src/lib/default-environment.js | 67 +++++++++ cli/src/lib/sandbox/mode.js | 36 +++++ 12 files changed, 515 insertions(+), 23 deletions(-) create mode 100644 cli/src/lib/sandbox/mode.js diff --git a/cli/__tests__/adapters.claude.environment.test.mjs b/cli/__tests__/adapters.claude.environment.test.mjs index 8b8a65d3b..4c5b88902 100644 --- a/cli/__tests__/adapters.claude.environment.test.mjs +++ b/cli/__tests__/adapters.claude.environment.test.mjs @@ -269,6 +269,79 @@ describe('claude adapter — ctx.environment', () => { expect(calls).toHaveLength(0); }); + // Vera 69548: a mode-less public block used to fall straight through to the + // bare `claude` spawn — a record claiming confinement it never got. The + // derived record stores trust only (the block is portable, the host is not), + // so the adapter has to resolve the mode here. + test('public trust with NO mode resolves to Seatbelt workspace on darwin', async () => { + const originalPlatform = process.platform; + const publicState = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-claude-public-state-')); + const { impl, calls } = makeSpawnImpl(); + try { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + spawnSync.mockImplementation((cmd) => ( + cmd === 'which' + ? { status: 0, stdout: '/usr/bin/true\n' } + : { status: 0, stdout: '' } + )); + + await claude.spawn('hi', { + agentName: 'derived-sandbox', + sessionId: null, + cwd, + env: { PATH: process.env.PATH }, + environment: { sandbox: { trust: 'public' } }, + _publicClaudeState: publicState, + _spawnImpl: impl, + }); + + expect(calls).toHaveLength(1); + expect(calls[0].cmd).toBe('/usr/bin/sandbox-exec'); + expect(calls[0].args[1]).toContain('(deny default)'); + expect(calls[0].args).toContain('--setting-sources'); + expect(calls[0].args).toContain('--permission-mode'); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform }); + spawnSync.mockReset(); + fs.rmSync(publicState, { recursive: true, force: true }); + } + }); + + test('public trust with NO mode resolves to bwrap off darwin', async () => { + const originalPlatform = process.platform; + const { impl, calls } = makeSpawnImpl(); + try { + Object.defineProperty(process, 'platform', { value: 'linux' }); + await claude.spawn('hi', { + sessionId: null, + cwd, + env: { PATH: process.env.PATH }, + environment: { sandbox: { trust: 'public' } }, + _spawnImpl: impl, + }); + expect(calls).toHaveLength(1); + expect(calls[0].cmd).toBe('bwrap'); + expect(calls[0].args).toContain('--unshare-all'); + expect(calls[0].args).toContain('--die-with-parent'); + // The inner claude argv rides after `--`, inside the namespace. + const inner = calls[0].args.slice(calls[0].args.indexOf('--') + 1); + expect(inner[0]).toMatch(/claude$/); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform }); + } + }); + + test('public trust with an unresolvable explicit mode refuses rather than falls through', async () => { + const { impl, calls } = makeSpawnImpl(); + await expect(claude.spawn('hi', { + sessionId: null, + cwd, + environment: { sandbox: { mode: 'unconfined', trust: 'public' } }, + _spawnImpl: impl, + })).rejects.toThrow(/require an enforced sandbox mode/); + expect(calls).toHaveLength(0); + }); + // ── ${COMMONLY_*} native MCP environment expansion ──────────────────────── // Values exist only in Claude's per-spawn environment. The JSON retains // placeholders so a cm_agent_* bearer token never exists on disk. diff --git a/cli/__tests__/adapters.codex.test.mjs b/cli/__tests__/adapters.codex.test.mjs index e955c361f..9918c716f 100644 --- a/cli/__tests__/adapters.codex.test.mjs +++ b/cli/__tests__/adapters.codex.test.mjs @@ -312,14 +312,48 @@ describe('codex adapter — spawn()', () => { expect(args).not.toContain('--dangerously-bypass-approvals-and-sandbox'); }); - test('public trust fails closed when no enforced public sandbox mode is declared', async () => { + // INVERTED deliberately (TASK-052, Wren 69545): this test used to require a + // throw for a mode-less public record. The derived record stores trust only — + // the block is portable, the host is not — so a mode-less public record must + // now spawn under the public profile with the write-capable workspace default. + // Leaving the throw in place would make every derived codex seat unspawnable, + // which is the same breakage Vera measured on Linux for claude (69542). + test('public trust with no mode defaults to the workspace permission profile', async () => { + const operatorHome = await mkdtemp(join(tmpdir(), 'commonly-codex-operator-home-')); + const publicHome = await mkdtemp(join(tmpdir(), 'commonly-codex-public-home-')); + await writeFile(join(operatorHome, 'auth.json'), '{"test":true}', 'utf8'); + const { impl, calls } = makeSpawnImpl({ + stdoutChunks: ['{"type":"thread.started","thread_id":"sid-derived"}\n'], + outputContents: 'ok', + }); + + await codex.spawn('work safely', { + sessionId: null, + cwd: '/tmp/public-agent-workspace', + environment: { sandbox: { trust: 'public' } }, + env: { ...process.env, CODEX_HOME: operatorHome }, + agentName: 'derived-sandbox-agent', + _publicCodexHome: publicHome, + _spawnImpl: impl, + }); + + const args = calls[0].args; + const cFlags = args.map((a, i) => (a === '-c' ? args[i + 1] : null)).filter(Boolean); + expect(cFlags).toContain('default_permissions="commonly_public"'); + expect(cFlags.find((flag) => flag.startsWith( + 'permissions.commonly_public.filesystem=', + ))).toContain('"."="write"'); + expect(args).not.toContain('--dangerously-bypass-approvals-and-sandbox'); + }); + + test('public trust with an unreadable explicit mode still fails closed', async () => { const { impl } = makeSpawnImpl({ stdoutChunks: ['{"type":"turn.completed"}\n'], outputContents: 'should not run', }); await expect(codex.spawn('x', { - environment: { sandbox: { trust: 'public' } }, + environment: { sandbox: { mode: 'unconfined', trust: 'public' } }, _spawnImpl: impl, })).rejects.toThrow(/require sandbox.mode=workspace or read-only/); }); diff --git a/cli/__tests__/daemon-supervisor.test.mjs b/cli/__tests__/daemon-supervisor.test.mjs index 54ec4e1ca..b1114f55c 100644 --- a/cli/__tests__/daemon-supervisor.test.mjs +++ b/cli/__tests__/daemon-supervisor.test.mjs @@ -152,10 +152,15 @@ describe('tick', () => { await supervisor.tick(); const record = saveToken.mock.calls[0][1]; expect(record.adapter).toBe('pi'); + // The baseline is both halves: the kernel MCP server (TASK-048) and an + // enforced sandbox (TASK-052). This assertion pinned mcp-only until the + // sandbox half landed, which is the gap the C4-6 row is about. The sandbox + // carries NO mode — the adapters resolve it per platform at spawn. expect(record.environment).toEqual({ model: 'deepseek-v4-flash', effort: 'high', mcp: [expect.objectContaining({ name: 'commonly', command: ['npx', '-y', '@commonlyai/mcp@latest'] })], + sandbox: { trust: 'public' }, }); }); @@ -403,6 +408,128 @@ describe('tick', () => { expect(saveToken).not.toHaveBeenCalled(); }); +// TASK-052 / C4-6: an undeclared sandbox is an unconfined seat. The +// self-serve install ships no environment at all, the daemon projected it +// verbatim, and the seat spawned with no OS-level confinement — the socket +// `sandbox.mode` defaults to is 'none'. The default here is the same one the +// c4 smoke room was hand-confined with. +describe('the baseline a seat nobody authored gets', () => { + const DEFAULT_SANDBOX = { trust: 'public' }; + const shippedCommonly = { + name: 'commonly', + transport: 'stdio', + command: ['npx', '-y', '@commonlyai/mcp@latest'], + }; + + test('a fresh self-serve seat is minted confined, not only tooled', async () => { + const { supervisor, saveToken, children } = makeHarness({ rows: () => [boundRow()] }); + await supervisor.tick(); + const { environment } = saveToken.mock.calls[0][1]; + expect(environment.sandbox).toEqual(DEFAULT_SANDBOX); + expect(environment.mcp).toEqual([expect.objectContaining({ name: 'commonly' })]); + expect(children).toHaveLength(1); + }); + + test('a declared environment that names no sandbox gains the default', async () => { + const tokens = { + 'wren-test': { + agentName: 'wren-test', + runtimeToken: 'cm_agent_old', + adapter: 'claude', + environment: { model: 'opus', mcp: [shippedCommonly] }, + }, + }; + const { supervisor, saveToken } = makeHarness({ + rows: () => [boundRow({ + runtime: { runtimeType: 'wrapper', model: 'opus' }, + environment: { version: 1, mcp: [shippedCommonly] }, + })], + tokens, + }); + await supervisor.tick(); + expect(saveToken).toHaveBeenCalledTimes(1); + expect(saveToken.mock.calls[0][1].environment).toEqual({ + version: 1, + mcp: [shippedCommonly], + model: 'opus', + sandbox: DEFAULT_SANDBOX, + }); + }); + + test('a declared environment that already confines the seat is left alone', async () => { + // The live c4-smoke shape: hand-confined, both MCP servers present. + const declared = { + version: 1, + sandbox: { trust: 'public' }, + mcp: [ + { ...shippedCommonly, env: { COMMONLY_AGENT_TOKEN: '${COMMONLY_AGENT_TOKEN}' } }, + { + name: 'commonly-grant-broker', + transport: 'http', + url: '${COMMONLY_API_URL}/api/mcp/grants/grant_4df79b67', + headers: { Authorization: 'Bearer ${COMMONLY_AGENT_TOKEN}' }, + }, + ], + }; + const { supervisor, saveToken } = makeHarness({ + rows: () => [boundRow({ + runtime: { runtimeType: 'wrapper', model: 'claude-opus-5' }, + environment: declared, + })], + }); + await supervisor.tick(); + expect(saveToken).toHaveBeenCalledTimes(1); + expect(saveToken.mock.calls[0][1].environment) + .toEqual({ ...declared, model: 'claude-opus-5' }); + // Idempotent: the second tick rewrites nothing, so the seat is not + // restarted on the tick after it was provisioned. + await supervisor.tick(); + expect(saveToken).toHaveBeenCalledTimes(1); + }); + + test('a locally authored environment keeps its own sandbox choice', async () => { + // The operator's record is theirs: a private-pod seat with no sandbox is + // allowed (that is what `agent attach` permits), so the daemon must not + // silently confine it. Nothing here is authored by the server. + const tokens = { + 'wren-test': { + agentName: 'wren-test', + runtimeToken: 'cm_agent_old', + adapter: 'claude', + environment: { model: 'opus', mcp: [shippedCommonly] }, + }, + }; + const { supervisor, saveToken, children } = makeHarness({ + rows: () => [boundRow({ runtime: { runtimeType: 'wrapper' } })], + tokens, + }); + await supervisor.tick(); + expect(saveToken).not.toHaveBeenCalled(); + expect(children).toHaveLength(1); + expect(tokens['wren-test'].environment.sandbox).toBeUndefined(); + }); + + test('a local record with no environment at all is confined too', async () => { + // A record past the mcp heal still carries no environment: nobody has + // authored anything, so it is in the same position as a fresh mint. + const tokens = { + 'wren-test': { + agentName: 'wren-test', runtimeToken: 'cm_agent_old', adapter: 'claude', + }, + }; + const { supervisor, saveToken } = makeHarness({ + rows: () => [boundRow({ runtime: { runtimeType: 'wrapper' } })], + tokens, + }); + await supervisor.tick(); + expect(saveToken).toHaveBeenCalledTimes(1); + expect(saveToken.mock.calls[0][1].environment).toEqual({ + mcp: [expect.objectContaining({ name: 'commonly' })], + sandbox: DEFAULT_SANDBOX, + }); + }); +}); + test('an existing token file skips the mint entirely', async () => { const { supervisor, client, children } = makeHarness({ rows: () => [boundRow()], diff --git a/cli/__tests__/default-environment.test.mjs b/cli/__tests__/default-environment.test.mjs index 833e2b2f3..ca9c07976 100644 --- a/cli/__tests__/default-environment.test.mjs +++ b/cli/__tests__/default-environment.test.mjs @@ -2,10 +2,14 @@ // declares no commonly server (TASK-048). import { ADAPTERS_WITH_DEFAULT_MCP, + COMMONLY_DEFAULT_SANDBOX, COMMONLY_MCP_SERVER_NAME, commonlyMcpServer, defaultMcpServers, + defaultSeatSandbox, + seatBaseline, withDefaultMcpServer, + withDefaultSandbox, } from '../src/lib/default-environment.js'; describe('defaultMcpServers', () => { @@ -82,3 +86,93 @@ describe('withDefaultMcpServer', () => { expect(withDefaultMcpServer(once, 'codex')).toBe(once); }); }); + +// TASK-052 / C4-6: an undeclared sandbox is an unconfined seat. `sandbox.mode` +// defaults to 'none' in the adapters, so "no sandbox block" and "mode: none" +// are the same thing to the spawn. +describe('withDefaultSandbox', () => { + test('declares the default sandbox when there is none', () => { + expect(withDefaultSandbox(null)).toEqual({ sandbox: defaultSeatSandbox() }); + expect(withDefaultSandbox({ model: 'opus' })).toEqual({ + model: 'opus', + sandbox: { trust: 'public' }, + }); + }); + + test('stores NO mode: the record is portable, the host is not', () => { + // `mode: 'workspace'` in a row breaks every Linux daemon host (it maps to + // Seatbelt on macOS and is refused elsewhere) and `mode: 'bwrap'` is + // meaningless on macOS. The adapters resolve the mode at spawn instead, so + // re-homing a seat to the other platform cannot make it unspawnable. + expect(COMMONLY_DEFAULT_SANDBOX).toEqual({ trust: 'public' }); + expect(Object.prototype.hasOwnProperty.call(COMMONLY_DEFAULT_SANDBOX, 'mode')).toBe(false); + expect(withDefaultSandbox(null).sandbox.mode).toBeUndefined(); + }); + + test('replaces an explicitly disengaged sandbox on this path', () => { + // Only ever called for daemon-derived environments; a server that says + // `mode: 'none'` is saying "unconfined", which is what the default is for. + expect(withDefaultSandbox({ sandbox: { mode: 'none' } }).sandbox) + .toEqual({ trust: 'public' }); + expect(withDefaultSandbox({ sandbox: { mode: 'none', trust: 'public' } }).sandbox) + .toEqual({ trust: 'public' }); + expect(withDefaultSandbox({ sandbox: {} }).sandbox) + .toEqual({ trust: 'public' }); + expect(withDefaultSandbox({ sandbox: 'none' }).sandbox) + .toEqual({ trust: 'public' }); + }); + + test('never overrides an enforced sandbox, including read-only', () => { + const declared = { sandbox: { mode: 'read-only', trust: 'public' } }; + expect(withDefaultSandbox(declared)).toBe(declared); + const workspace = { sandbox: { mode: 'workspace', trust: 'internal' } }; + expect(withDefaultSandbox(workspace)).toBe(workspace); + const bwrap = { sandbox: { mode: 'bwrap' } }; + expect(withDefaultSandbox(bwrap)).toBe(bwrap); + // The derived shape itself is enforced — without this, every tick would + // rewrite the record and restart the seat forever. + const derived = { sandbox: { trust: 'public' } }; + expect(withDefaultSandbox(derived)).toBe(derived); + }); + + test('leaves a malformed environment alone, and is idempotent', () => { + expect(withDefaultSandbox('nonsense')).toBe('nonsense'); + expect(withDefaultSandbox([1])).toEqual([1]); + const once = withDefaultSandbox({ model: 'x' }); + expect(withDefaultSandbox(once)).toBe(once); + }); + + test('the constant is frozen: the default cannot be edited in place', () => { + expect(Object.isFrozen(COMMONLY_DEFAULT_SANDBOX)).toBe(true); + }); +}); + +describe('seatBaseline', () => { + test('adds the sandbox only when asked, and only for a consuming adapter', () => { + const bare = seatBaseline(null, 'claude'); + expect(bare.sandbox).toBeUndefined(); + expect(bare.mcp).toEqual([commonlyMcpServer()]); + + const confined = seatBaseline(null, 'claude', { sandbox: true }); + expect(confined).toEqual({ + sandbox: { trust: 'public' }, + mcp: [commonlyMcpServer()], + }); + + // No consumption path: nothing is invented, sandbox included. + expect(seatBaseline(null, 'stub', { sandbox: true })).toBeNull(); + }); + + test('keeps an enforced sandbox while adding the mcp default', () => { + const declared = { sandbox: { mode: 'read-only', trust: 'public' } }; + expect(seatBaseline(declared, 'codex', { sandbox: true })).toEqual({ + sandbox: { mode: 'read-only', trust: 'public' }, + mcp: [commonlyMcpServer()], + }); + }); + + test('is idempotent on both halves', () => { + const once = seatBaseline(null, 'pi', { sandbox: true }); + expect(seatBaseline(once, 'pi', { sandbox: true })).toBe(once); + }); +}); diff --git a/cli/__tests__/public-pod-sandbox-gate.test.mjs b/cli/__tests__/public-pod-sandbox-gate.test.mjs index 65864ea0a..1e740172c 100644 --- a/cli/__tests__/public-pod-sandbox-gate.test.mjs +++ b/cli/__tests__/public-pod-sandbox-gate.test.mjs @@ -66,6 +66,27 @@ describe('public-pod sandbox gate', () => { })).resolves.toBeUndefined(); }); + test('allows the derived mode-less public declaration — the daemon\'s own shape', async () => { + // lib/default-environment.js writes `sandbox: { trust: 'public' }` with no + // mode, because the mode is a host fact the adapters resolve at spawn. The + // gate refusing that shape would make the daemon's own baseline + // unattachable to a public pod. + const client = clientFor(PUBLIC_POD); + await expect(assertSandboxDeclaredForPublicPod({ + client, podId: 'p1', environment: { sandbox: { trust: 'public' } }, + })).resolves.toBeUndefined(); + expect(client.get).not.toHaveBeenCalled(); + }); + + test('allows bwrap without a trust field — that IS confinement', async () => { + // The old predicate required trust AND mode, so a genuinely confined + // `mode: 'bwrap'` seat was refused on a public pod. bwrap wraps the spawn + // whatever the trust says, and attach separately checks it is installed. + await expect(assertSandboxDeclaredForPublicPod({ + client: clientFor(PUBLIC_POD), podId: 'p1', environment: { sandbox: { mode: 'bwrap' } }, + })).resolves.toBeUndefined(); + }); + test('leaves private pods alone — this gate is only about public exposure', async () => { await expect(assertSandboxDeclaredForPublicPod({ client: clientFor(PRIVATE_POD), podId: 'p2', environment: null, diff --git a/cli/package.json b/cli/package.json index 01a79cb55..cce89ebf0 100644 --- a/cli/package.json +++ b/cli/package.json @@ -1,6 +1,6 @@ { "name": "@commonlyai/cli", - "version": "0.1.50", + "version": "0.1.51", "license": "Apache-2.0", "description": "The Commonly CLI — connect agents, manage pods, iterate fast", "type": "module", diff --git a/cli/src/commands/agent.js b/cli/src/commands/agent.js index f3f4031f5..c03aa65c2 100644 --- a/cli/src/commands/agent.js +++ b/cli/src/commands/agent.js @@ -40,6 +40,7 @@ import { readPodFocus, } from '../lib/pod-focus.js'; import { detectBwrap } from '../lib/sandbox/bwrap.js'; +import { resolvePublicSandboxMode } from '../lib/sandbox/mode.js'; import { detectSeatbelt } from '../lib/sandbox/seatbelt.js'; import { DEFAULT_HOOK_TIMEOUT_MS, @@ -302,8 +303,9 @@ export const buildDefaultEnvironment = (adapterName) => { * enforced sandbox. * * The public-agent sandbox is real and attack-tested, but it only engages once - * `sandbox.trust` and `sandbox.mode` are declared: `sandbox.mode` defaults to - * `'none'`, and nothing previously connected "this pod is public" to "this + * an enforced sandbox is declared: `sandbox.trust: 'public'` (the adapters then + * resolve the mode for the host at spawn) or an explicit non-'none' + * `sandbox.mode`. Nothing previously connected "this pod is public" to "this * agent must be confined". An agent attached with no sandbox block simply ran * unconfined, silently, with the operator none the wiser. * @@ -327,7 +329,12 @@ export const assertSandboxDeclaredForPublicPod = async ({ const mode = environment?.sandbox?.mode; const trust = environment?.sandbox?.trust; - const declared = Boolean(trust) && Boolean(mode) && mode !== 'none'; + // An ENFORCED declaration is one the adapters act on: a public trust (whose + // mode they resolve per host at spawn) or an explicit non-'none' mode. This + // mirrors the daemon's predicate in lib/default-environment.js, so the shape + // the daemon writes for an unconfigured seat is one attach also accepts. + const declared = mode !== 'none' + && (trust === 'public' || typeof mode === 'string'); if (declared) return; let pod = null; @@ -350,10 +357,11 @@ export const assertSandboxDeclaredForPublicPod = async ({ + 'people you do not control — but its environment declares no sandbox, and ' + 'an undeclared sandbox means NO sandbox.\n\n' + 'Add a sandbox block to the environment file and retry:\n\n' - + ' "sandbox": { "trust": "public", "mode": "read-only" }\n\n' - + 'Modes for a public agent: "read-only" or "workspace" (macOS Seatbelt / ' - + 'Linux bwrap). To attach an agent to a private pod instead, pass that ' - + 'pod id.', + + ' "sandbox": { "trust": "public" }\n\n' + + 'That is the whole declaration: the adapters pick the enforced mode for ' + + 'the host (Seatbelt on macOS, bwrap on Linux). Pin one explicitly with ' + + '"mode": "read-only" or "workspace" if you want a specific access level. ' + + 'To attach an agent to a private pod instead, pass that pod id.', ); }; @@ -501,7 +509,10 @@ export const performAttach = async ({ workspace = await resolveWorkspace(environment, agentName, dirname(envPath)); log(`workspace: ${workspace.path}${workspace.created ? ' (created)' : ''}`); - const sandboxMode = environment.sandbox?.mode || 'none'; + const sandboxMode = environment.sandbox?.mode + || (environment.sandbox?.trust === 'public' + ? resolvePublicSandboxMode(environment.sandbox) + : 'none'); const sandboxTrust = environment.sandbox?.trust; if (sandboxTrust === 'public' && sandboxMode === 'none') { throw new Error( diff --git a/cli/src/lib/adapters/claude.js b/cli/src/lib/adapters/claude.js index 92f6cc43b..bcbeee451 100644 --- a/cli/src/lib/adapters/claude.js +++ b/cli/src/lib/adapters/claude.js @@ -63,6 +63,7 @@ import { delimiter, isAbsolute, join } from 'path'; import { mountSkills } from '../environment.js'; import { wrapArgvWithBwrap } from '../sandbox/bwrap.js'; +import { PUBLIC_SANDBOX_MODES, resolvePublicSandboxMode } from '../sandbox/mode.js'; import { publicClaudeStateRoot, wrapArgvWithSeatbelt, @@ -82,7 +83,6 @@ const DEFAULT_TIMEOUT_MS = (() => { const buildPrompt = buildMemoryPreamble; -const PUBLIC_SANDBOX_MODES = new Set(['workspace', 'read-only']); const PUBLIC_DENIED_TOOLS = [ 'WebSearch', 'WebFetch', @@ -410,12 +410,20 @@ const prepareArgv = async (innerArgv, ctx) => { .map((name) => `mcp__${name}__*`); } - const sandboxMode = env.sandbox?.mode; const sandboxTrust = env.sandbox?.trust; + const sandboxMode = resolvePublicSandboxMode(env.sandbox); const publicNativeSandbox = sandboxTrust === 'public' && PUBLIC_SANDBOX_MODES.has(sandboxMode); - if (sandboxTrust === 'public' && sandboxMode === 'none') { - throw new Error('public Claude agents require an enforced sandbox mode'); + // A public trust MUST resolve to an enforced mode. Absent mode is resolved + // above; anything else that lands here (the literal 'none', a typo, a + // non-string) used to fall through to the bare `claude` spawn below — an + // unconfined seat with a public record, which is how a record can claim + // confinement it never applies (Vera 69548). + if (sandboxTrust === 'public' && !publicNativeSandbox && sandboxMode !== 'bwrap') { + throw new Error( + 'public Claude agents require an enforced sandbox mode ' + + `(workspace or read-only on macOS, bwrap elsewhere), got ${JSON.stringify(sandboxMode)}`, + ); } if (publicNativeSandbox) { if (process.platform !== 'darwin') { @@ -546,7 +554,7 @@ export default { let publicClaudeState = null; try { const publicNativeSandbox = ctx.environment?.sandbox?.trust === 'public' - && PUBLIC_SANDBOX_MODES.has(ctx.environment?.sandbox?.mode); + && PUBLIC_SANDBOX_MODES.has(resolvePublicSandboxMode(ctx.environment?.sandbox)); if (publicNativeSandbox) { publicClaudeState = await preparePublicClaudeState(ctx); } diff --git a/cli/src/lib/adapters/codex.js b/cli/src/lib/adapters/codex.js index f330439a9..6ed4f83ab 100644 --- a/cli/src/lib/adapters/codex.js +++ b/cli/src/lib/adapters/codex.js @@ -439,8 +439,14 @@ export default { runtimeToken: ctx.runtimeToken, instanceUrl: ctx.instanceUrl, }); + // A derived record stores `trust: 'public'` and no mode (the block is + // platform-independent; see cli/src/lib/default-environment.js). Codex's + // mode only selects read vs write access — its permission profiles run on + // both macOS and Linux — so the derived default is `workspace`, and an + // explicit mode in the record still wins. Before this, a mode-less public + // record threw `got unset` and no codex seat spawned at all. const publicSandboxMode = ctx.environment?.sandbox?.trust === 'public' - ? ctx.environment?.sandbox?.mode || 'unset' + ? ctx.environment?.sandbox?.mode ?? 'workspace' : null; const args = buildArgs({ sessionId: ctx.sessionId || null, diff --git a/cli/src/lib/daemon-supervisor.js b/cli/src/lib/daemon-supervisor.js index 554cfbe09..6a5727dc5 100644 --- a/cli/src/lib/daemon-supervisor.js +++ b/cli/src/lib/daemon-supervisor.js @@ -2,7 +2,7 @@ import { isDeepStrictEqual } from 'node:util'; import { homedir } from 'node:os'; import { isAbsolute, resolve as pathResolve } from 'node:path'; -import { withDefaultMcpServer } from './default-environment.js'; +import { seatBaseline } from './default-environment.js'; /** * ADR-026 Phase 2, slice 2: the resident supervision loop behind @@ -190,7 +190,15 @@ export const createDaemonSupervisor = ({ // declaration carries no mcp[] would come back tool-less. Re-apply the // shipped default here and below, or the seat silently loses every // commonly_* tool the next time the UI edits its model (TASK-048). - const nextEnvironment = withDefaultMcpServer(merged, nextAdapter); + // + // A DECLARED environment also gets the sandbox default when it names + // none: the declaration is not the operator's, and `sandbox.mode` + // defaults to 'none' in the adapters, so an omitted block is an + // unconfined seat (TASK-052). A local record with no environment at + // all is in the same position — nobody has authored anything. + const nextEnvironment = seatBaseline(merged, nextAdapter, { + sandbox: wanted.declared || !existing.environment, + }); const workspacePath = workspacePathFor(nextEnvironment); const nextRecord = { ...existing, @@ -209,7 +217,9 @@ export const createDaemonSupervisor = ({ if (adapterChanged) { // An adapter that consumes mcp[] must not be started on a record that // declares none, even when only the adapter itself changed. - const nextEnvironment = withDefaultMcpServer(existing.environment, nextAdapter); + const nextEnvironment = seatBaseline(existing.environment, nextAdapter, { + sandbox: !existing.environment, + }); saveToken(row.agentName, { ...existing, adapter: nextAdapter, @@ -226,7 +236,9 @@ export const createDaemonSupervisor = ({ // stays tool-less for as long as it runs. Heal it here, and only when // the environment actually changed — otherwise every tick rewrites the // file and restarts the seat forever. - const nextEnvironment = withDefaultMcpServer(existing.environment, existing.adapter); + const nextEnvironment = seatBaseline(existing.environment, existing.adapter, { + sandbox: !existing.environment, + }); if (!isDeepStrictEqual(existing.environment || null, nextEnvironment || null)) { saveToken(row.agentName, { ...existing, environment: nextEnvironment }); log('record predates the commonly MCP baseline — restarting the seat to load it'); @@ -277,10 +289,13 @@ export const createDaemonSupervisor = ({ const environment = environmentFor(row); // A seat installed server-side (no local `agent attach`) arrives with no // mcp[] at all: without the default it spawns a CLI that has no commonly_* - // tools and cannot post. See lib/default-environment.js. - const recordEnvironment = withDefaultMcpServer( + // tools and cannot post. See lib/default-environment.js. Nothing here is + // operator-authored, so this seat also gets the sandbox default — the + // self-serve install's seat used to be born unconfined (TASK-052). + const recordEnvironment = seatBaseline( environment ? environment.value : null, adapter, + { sandbox: true }, ); saveToken(row.agentName, { agentName: row.agentName, diff --git a/cli/src/lib/default-environment.js b/cli/src/lib/default-environment.js index 8dec10374..a79946491 100644 --- a/cli/src/lib/default-environment.js +++ b/cli/src/lib/default-environment.js @@ -24,6 +24,35 @@ export const ADAPTERS_WITH_DEFAULT_MCP = new Set(['claude', 'codex', 'pi']); +/** + * The sandbox an unconfigured seat gets. + * + * `sandbox.mode` defaults to `'none'` in the adapters, so an ABSENT sandbox + * block means NO sandbox — the seat runs unconfined on the operator's machine + * while taking instructions from whoever is in the room. That is what a + * self-serve install shipped (C4-6 / TASK-052): the install declares no + * environment, the daemon projected nothing, and the socket was born with no + * confinement and no way to notice. + * + * `trust: 'public'` is the conservative declaration — "this seat takes + * instructions from people I do not control" — and it is the one that engages + * the real sandbox in the adapters. + * + * NO MODE is stored, deliberately. The record is platform-independent and the + * host is not: the adapters read a public trust with no mode as Seatbelt + * workspace on macOS and bwrap elsewhere. `mode: 'workspace'` in the row is + * refused on Linux, and `mode: 'bwrap'` is meaningless on macOS, so either one + * moves a host fact into the database and breaks the day the seat is re-homed. + * An explicit mode in a record still wins over the derived one. + * + * Confinement holds for claude and codex. A pi seat gets the block and is NOT + * confined by it — the pi adapter has no sandbox path until #1740's transport + * work gives it one, so do not read this key on a pi record as confinement. + */ +export const COMMONLY_DEFAULT_SANDBOX = Object.freeze({ trust: 'public' }); + +export const defaultSeatSandbox = () => ({ ...COMMONLY_DEFAULT_SANDBOX }); + export const COMMONLY_MCP_SERVER_NAME = 'commonly'; export const commonlyMcpServer = () => ({ @@ -68,3 +97,41 @@ export const withDefaultMcpServer = (environment, adapterName) => { } return { ...(environment || {}), mcp: [...(declared || []), commonlyMcpServer()] }; }; + +/** + * Ensure the environment declares an ENFORCED sandbox, touching nothing that + * is already enforced. Returns the SAME reference when nothing needs adding. + * + * Only for environments this daemon derived from the server's declaration or + * from nothing at all — never for a local record the operator authored. A + * declared `mode: 'none'` is replaced rather than honoured: absence and 'none' + * are the same thing to the adapter, and this is the path that decides what an + * undeclared sandbox means. + */ +export const withDefaultSandbox = (environment) => { + if (environment !== null && environment !== undefined + && (typeof environment !== 'object' || Array.isArray(environment))) { + return environment; + } + const sandbox = environment?.sandbox; + // An ENFORCED declaration is one the adapters act on: a public trust (whose + // mode they resolve, and refuse to spawn without) or an explicit non-'none' + // mode. `mode: 'none'`, an empty block and a missing one are all the same + // thing to a spawn — no confinement — so all three are replaced here. + const enforced = sandbox !== null && typeof sandbox === 'object' + && sandbox.mode !== 'none' + && (sandbox.trust === 'public' || typeof sandbox.mode === 'string'); + if (enforced) return environment; + return { ...(environment || {}), sandbox: defaultSeatSandbox() }; +}; + +/** + * The full baseline a seat gets from the daemon: the kernel MCP server, plus an + * enforced sandbox when the environment is one the daemon derived rather than + * one the operator wrote (`sandbox: true` at the call sites). + */ +export const seatBaseline = (environment, adapterName, { sandbox = false } = {}) => { + const withMcp = withDefaultMcpServer(environment, adapterName); + if (!sandbox || !ADAPTERS_WITH_DEFAULT_MCP.has(adapterName)) return withMcp; + return withDefaultSandbox(withMcp); +}; diff --git a/cli/src/lib/sandbox/mode.js b/cli/src/lib/sandbox/mode.js new file mode 100644 index 000000000..6d7eea706 --- /dev/null +++ b/cli/src/lib/sandbox/mode.js @@ -0,0 +1,36 @@ +/** + * Which host mechanism enforces a public seat. + * + * A stored environment is platform-independent and the host is not. The daemon + * derives `sandbox: { trust: 'public' }` with NO mode (lib/default-environment.js) + * because `mode: 'workspace'` is Seatbelt on macOS and is refused on Linux, + * while `mode: 'bwrap'` is meaningless on macOS — writing either into the row + * moves a host fact into the database and breaks the day the seat is re-homed + * (Wren 69545). + * + * So the mode is resolved here, at the point that knows the host: + * darwin → 'workspace' (the claude adapter maps it to Seatbelt) + * else → 'bwrap' (bubblewrap; attach checks it is installed) + * + * Resolution is PUBLIC-only on purpose. A non-public declaration keeps whatever + * mode it wrote, and an adapter that cannot enforce it is expected to refuse + * rather than quietly spawn unconfined. An explicit mode on a public + * declaration still wins — including a wrong one, which the caller then + * refuses (Vera 69548: an unresolvable public mode must not fall through to a + * bare, unconfined spawn). + * + * NOT used by the codex adapter. Codex's `mode` is read-vs-write access, not a + * host mechanism — its permission profiles run on both platforms — so its + * derived default is 'workspace' everywhere. + */ + +export const PUBLIC_SANDBOX_MODES = new Set(['workspace', 'read-only']); + +export const resolvePublicSandboxMode = (sandbox, platform = process.platform) => { + const declared = sandbox?.mode; + if (sandbox?.trust !== 'public') return declared; + if (declared !== undefined && declared !== null) return declared; + return platform === 'darwin' ? 'workspace' : 'bwrap'; +}; + +export default { PUBLIC_SANDBOX_MODES, resolvePublicSandboxMode }; From 64063a7571a3f10f32a0c351134d2f8e90ace394 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 18 Sep 2026 05:44:22 -0700 Subject: [PATCH 2/4] fix(cli): the Linux public seat gets the same tool floor as the macOS one (TASK-052) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wren 69586 + Vera 69578, folded in before the PR opens. "public" was one contract implemented twice, and the Linux half was half of it. `buildPublicClaudePolicyArgs` — settings off, permission mode dontAsk, the denied reads and tools, strict MCP config — was only reachable inside the Seatbelt branch, so a derived Linux seat ran inside bwrap with Bash, Write and WebFetch intact and the operator's own Claude settings loaded. The jail bounds the filesystem and the policy bounds the tools; a jail alone is half a floor. Now a public trust that resolves to bwrap gets the same policy args inside the namespace, and a bwrap seat with no public trust still gets only the jail — the policy is the public contract, not a property of wrapping. Asserted both ways. The mechanism is also verified where the mode is chosen: a public seat on a host without bwrap now fails its derivation with an actionable message (`bwrap not found on PATH. Install bubblewrap: ...`) instead of every spawn failing with a bare ENOENT. On macOS it reports the macOS-only message, the same one the wrapper would have thrown later. `ctx._detectBwrap` is a test seam, alongside the existing `_spawnImpl`. Tests: the derived-Linux test now asserts the policy args are inside the namespace, and two cases cover a host with no bwrap (refused, and nothing spawned) and a trust-less bwrap seat (no policy args). cli 39 suites / 553 passed / 10 skipped, lint clean. --- .../adapters.claude.environment.test.mjs | 55 ++++++++++++++++++- cli/src/lib/adapters/claude.js | 23 +++++++- 2 files changed, 74 insertions(+), 4 deletions(-) diff --git a/cli/__tests__/adapters.claude.environment.test.mjs b/cli/__tests__/adapters.claude.environment.test.mjs index 4c5b88902..6776b097b 100644 --- a/cli/__tests__/adapters.claude.environment.test.mjs +++ b/cli/__tests__/adapters.claude.environment.test.mjs @@ -307,7 +307,7 @@ describe('claude adapter — ctx.environment', () => { } }); - test('public trust with NO mode resolves to bwrap off darwin', async () => { + test('public trust with NO mode resolves to bwrap off darwin, with the same tool policy macOS gets', async () => { const originalPlatform = process.platform; const { impl, calls } = makeSpawnImpl(); try { @@ -318,6 +318,7 @@ describe('claude adapter — ctx.environment', () => { env: { PATH: process.env.PATH }, environment: { sandbox: { trust: 'public' } }, _spawnImpl: impl, + _detectBwrap: () => ({ available: true, path: 'bwrap' }), }); expect(calls).toHaveLength(1); expect(calls[0].cmd).toBe('bwrap'); @@ -326,6 +327,58 @@ describe('claude adapter — ctx.environment', () => { // The inner claude argv rides after `--`, inside the namespace. const inner = calls[0].args.slice(calls[0].args.indexOf('--') + 1); expect(inner[0]).toMatch(/claude$/); + // The jail bounds the filesystem, the policy bounds the tools — the + // Linux seat gets the same floor as the macOS one, not the half of it + // that happened to be on the other side of a branch (Vera 69578). + const settingSources = inner.indexOf('--setting-sources'); + expect(settingSources).toBeGreaterThan(-1); + expect(inner[settingSources + 1]).toBe(''); + expect(inner).toContain('--strict-mcp-config'); + expect(inner).toContain('--no-chrome'); + expect(inner).toEqual(expect.arrayContaining(['--permission-mode', 'dontAsk'])); + expect(inner).toContain('--disallowedTools'); + expect(inner.join(' ')).toContain('Read(./.env)'); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform }); + } + }); + + test('a public Linux seat whose host has no bwrap refuses to derive, rather than failing per spawn', async () => { + const originalPlatform = process.platform; + const { impl, calls } = makeSpawnImpl(); + try { + Object.defineProperty(process, 'platform', { value: 'linux' }); + await expect(claude.spawn('hi', { + sessionId: null, + cwd, + env: { PATH: process.env.PATH }, + environment: { sandbox: { trust: 'public' } }, + _spawnImpl: impl, + _detectBwrap: () => ({ available: false, error: 'bwrap not found on PATH.' }), + })).rejects.toThrow(/require bwrap: bwrap not found/); + expect(calls).toHaveLength(0); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform }); + } + }); + + test('a bwrap seat with no public trust gets the jail without the public tool policy', async () => { + const originalPlatform = process.platform; + const { impl, calls } = makeSpawnImpl(); + try { + Object.defineProperty(process, 'platform', { value: 'linux' }); + await claude.spawn('hi', { + sessionId: null, + cwd, + env: { PATH: process.env.PATH }, + environment: { sandbox: { mode: 'bwrap' } }, + _spawnImpl: impl, + _detectBwrap: () => ({ available: true, path: 'bwrap' }), + }); + expect(calls).toHaveLength(1); + const inner = calls[0].args.slice(calls[0].args.indexOf('--') + 1); + expect(inner).not.toContain('--setting-sources'); + expect(inner).not.toContain('--permission-mode'); } finally { Object.defineProperty(process, 'platform', { value: originalPlatform }); } diff --git a/cli/src/lib/adapters/claude.js b/cli/src/lib/adapters/claude.js index bcbeee451..283eda484 100644 --- a/cli/src/lib/adapters/claude.js +++ b/cli/src/lib/adapters/claude.js @@ -62,7 +62,7 @@ import { homedir, tmpdir } from 'os'; import { delimiter, isAbsolute, join } from 'path'; import { mountSkills } from '../environment.js'; -import { wrapArgvWithBwrap } from '../sandbox/bwrap.js'; +import { detectBwrap, wrapArgvWithBwrap } from '../sandbox/bwrap.js'; import { PUBLIC_SANDBOX_MODES, resolvePublicSandboxMode } from '../sandbox/mode.js'; import { publicClaudeStateRoot, @@ -414,6 +414,11 @@ const prepareArgv = async (innerArgv, ctx) => { const sandboxMode = resolvePublicSandboxMode(env.sandbox); const publicNativeSandbox = sandboxTrust === 'public' && PUBLIC_SANDBOX_MODES.has(sandboxMode); + // A public trust is ONE contract on both platforms: the jail bounds the + // filesystem and the policy bounds the tools. The bwrap branch below used to + // get the jail only — a derived Linux seat kept Bash/Write/WebFetch and ran on + // the operator's own Claude settings inside the namespace (Vera 69578). + const publicBwrapSandbox = sandboxTrust === 'public' && sandboxMode === 'bwrap'; // A public trust MUST resolve to an enforced mode. Absent mode is resolved // above; anything else that lands here (the literal 'none', a typo, a // non-string) used to fall through to the bare `claude` spawn below — an @@ -458,7 +463,20 @@ const prepareArgv = async (innerArgv, ctx) => { }; } - if (allowedPatterns.length > 0) { + if (publicBwrapSandbox) { + // The mode is chosen here, so the mechanism is verified here: a missing + // bwrap must fail the seat's derivation with an actionable message, not + // every one of its spawns with a bare ENOENT (Wren 69586). On a non-Linux + // host this reports the macOS-only message, same as the wrapper would. + const bwrap = (ctx._detectBwrap || detectBwrap)(); + if (!bwrap.available) { + throw new Error(`public Claude agents require bwrap: ${bwrap.error}`); + } + innerArgv = [ + ...innerArgv, + ...buildPublicClaudePolicyArgs(allowedPatterns), + ]; + } else if (allowedPatterns.length > 0) { innerArgv = [...innerArgv, '--allowedTools', ...allowedPatterns]; } if (sandboxMode === 'bwrap') { @@ -469,7 +487,6 @@ const prepareArgv = async (innerArgv, ctx) => { }); return { cmd: wrapped[0], args: wrapped.slice(1), env: claudeEnv }; } - return { cmd: 'claude', args: innerArgv, env: claudeEnv }; }; From 6ba9b63330ec3eed6d2624461bbdb26985424b31 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 18 Sep 2026 05:48:07 -0700 Subject: [PATCH 3/4] fix(cli): a trust value no adapter reads is refused, and a stored one resolves toward confinement (TASK-059) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wren 69585, on Vera's 69592 premise correction, folded into this PR because the runtime half of it IS this PR's resolver: `internal` resolves as `public`, and a public declaration is then confined or refuses to derive. Landing the enum change alone would have made a legacy record *look* public while still falling through to the bare spawn. Two halves: 1. Validation refuses `trust: 'internal'` for new declarations, and the message names what does something — `public`, or omitting the field for your own seat. The value was in ALLOWED_SANDBOX_TRUST and read by no adapter: on the attach path it could only name a mode the platform cannot resolve, and on the daemon path it was silently inert — claude fell through to an unconfined spawn and codex took `--dangerously-bypass-approvals-and-sandbox`. A declaration that reads as "confine me, not as a public agent" meant the opposite of what it said. 2. A record that already carries it is resolved TOWARD confinement (`normalizeSandboxTrust`/`isLegacySandboxTrust` in environment.js), with one line in the spawn log saying what was read and why. On macOS a legacy internal+workspace seat now gets Seatbelt; on Linux that same record refuses to derive rather than running bare, and codex gets the public permission profile instead of the bypass flag. Nothing resolves toward the bare spawn. The claude adapter resolves it ONCE at the top of spawn and threads it to both consumers, because my first cut normalized inside the argv builder only, and the seat then died with "public Claude state was not prepared before argv construction" — the state preparation and the argv builder had read the same declaration independently. That is the bug this whole change is about, in miniature. Tests: validation rejects `internal` and accepts `public` or an omitted field; the legacy resolution is unit-tested (and leaves every other declaration, and an absent sandbox block, untouched); claude confines a legacy internal+workspace seat under Seatbelt; codex gives it the public permission profile and never the bypass flag. cli 39 suites / 559 passed / 10 skipped, lint clean. --- .../adapters.claude.environment.test.mjs | 35 ++++++++++++++++++ cli/__tests__/adapters.codex.test.mjs | 35 ++++++++++++++++++ cli/__tests__/environment.test.mjs | 36 +++++++++++++++++++ cli/src/lib/adapters/claude.js | 32 +++++++++++++++-- cli/src/lib/adapters/codex.js | 14 ++++++-- cli/src/lib/environment.js | 29 +++++++++++++-- 6 files changed, 175 insertions(+), 6 deletions(-) diff --git a/cli/__tests__/adapters.claude.environment.test.mjs b/cli/__tests__/adapters.claude.environment.test.mjs index 6776b097b..37433d384 100644 --- a/cli/__tests__/adapters.claude.environment.test.mjs +++ b/cli/__tests__/adapters.claude.environment.test.mjs @@ -384,6 +384,41 @@ describe('claude adapter — ctx.environment', () => { } }); + test('a legacy trust=internal record is resolved as public and confined, never run unconfined', async () => { + const originalPlatform = process.platform; + const publicState = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-claude-internal-state-')); + const { impl, calls } = makeSpawnImpl(); + try { + Object.defineProperty(process, 'platform', { value: 'darwin' }); + spawnSync.mockImplementation((cmd) => ( + cmd === 'which' + ? { status: 0, stdout: '/usr/bin/true\n' } + : { status: 0, stdout: '' } + )); + + await claude.spawn('hi', { + agentName: 'legacy-internal', + sessionId: null, + cwd, + env: { PATH: process.env.PATH }, + environment: { sandbox: { mode: 'workspace', trust: 'internal' } }, + _publicClaudeState: publicState, + _spawnImpl: impl, + }); + + // It reads as "confine me" and used to mean the opposite: before this the + // adapter skipped Seatbelt entirely and spawned a bare, unconfined claude + // (Vera 69592). + expect(calls).toHaveLength(1); + expect(calls[0].cmd).toBe('/usr/bin/sandbox-exec'); + expect(calls[0].args).toContain('--setting-sources'); + } finally { + Object.defineProperty(process, 'platform', { value: originalPlatform }); + spawnSync.mockReset(); + fs.rmSync(publicState, { recursive: true, force: true }); + } + }); + test('public trust with an unresolvable explicit mode refuses rather than falls through', async () => { const { impl, calls } = makeSpawnImpl(); await expect(claude.spawn('hi', { diff --git a/cli/__tests__/adapters.codex.test.mjs b/cli/__tests__/adapters.codex.test.mjs index 9918c716f..3b0caa021 100644 --- a/cli/__tests__/adapters.codex.test.mjs +++ b/cli/__tests__/adapters.codex.test.mjs @@ -218,6 +218,41 @@ describe('codex adapter — spawn()', () => { .toContain('=== Current turn ===\nhi\n=== Before this session ends ==='); }); + test('a legacy trust=internal record gets the public profile, never the bypass flag', async () => { + const operatorHome = await mkdtemp(join(tmpdir(), 'commonly-codex-operator-home-')); + const publicHome = await mkdtemp(join(tmpdir(), 'commonly-codex-public-home-')); + const operatorAuth = join(operatorHome, 'auth.json'); + await writeFile(operatorAuth, '{"test":true}', 'utf8'); + const { impl, calls } = makeSpawnImpl({ + stdoutChunks: ['{"type":"thread.started","thread_id":"sid-internal"}\n'], + outputContents: 'ok', + }); + + await codex.spawn('work safely', { + sessionId: null, + cwd: '/tmp/legacy-internal-workspace', + environment: { + sandbox: { mode: 'workspace', trust: 'internal' }, + }, + env: { ...process.env, CODEX_HOME: operatorHome }, + agentName: 'legacy-internal-agent', + _publicCodexHome: publicHome, + _spawnImpl: impl, + }); + + // Before this, `internal` read by no adapter meant the operator got the + // bypass flag — the exact opposite of the confinement they declared + // (Vera 69592). It is now read as public and confined. + const args = calls[0].args; + expect(args).not.toContain('--dangerously-bypass-approvals-and-sandbox'); + expect(args).not.toContain('--sandbox'); + const cFlags = args + .map((a, i) => (a === '-c' ? args[i + 1] : null)) + .filter(Boolean); + expect(cFlags).toContain('default_permissions="commonly_public"'); + expect(calls[0].opts.env.CODEX_HOME).toBe(publicHome); + }); + test('public workspace mode uses a deny-by-default permission profile and never the legacy sandbox or bypass', async () => { const operatorHome = await mkdtemp(join(tmpdir(), 'commonly-codex-operator-home-')); const publicHome = await mkdtemp(join(tmpdir(), 'commonly-codex-public-home-')); diff --git a/cli/__tests__/environment.test.mjs b/cli/__tests__/environment.test.mjs index 67e939a8a..550c12c8f 100644 --- a/cli/__tests__/environment.test.mjs +++ b/cli/__tests__/environment.test.mjs @@ -24,6 +24,8 @@ await jest.unstable_mockModule('os', () => { }); const { + isLegacySandboxTrust, + normalizeSandboxTrust, parseEnvironmentFile, validateEnvironmentSpec, resolveWorkspace, @@ -140,6 +142,23 @@ describe('validateEnvironmentSpec', () => { })).toEqual({ ok: true, errors: [] }); }); + test('rejects sandbox.trust=internal — a value no adapter reads', () => { + const res = validateEnvironmentSpec({ + sandbox: { mode: 'workspace', trust: 'internal' }, + }); + expect(res.ok).toBe(false); + // The message has to name what DOES something, or the next author just + // guesses again: `public`, or no trust field at all. + expect(res.errors.join(' ')).toMatch(/sandbox\.trust must be 'public'/); + expect(res.errors.join(' ')).toMatch(/omit it for your own seat/); + expect(res.errors.join(' ')).toMatch(/got "internal"/); + }); + + test('accepts the two declarations that do something: public trust, or none', () => { + expect(validateEnvironmentSpec({ sandbox: { trust: 'public' } }).ok).toBe(true); + expect(validateEnvironmentSpec({ sandbox: { mode: 'bwrap' } }).ok).toBe(true); + }); + test('rejects bad sandbox.trust', () => { const res = validateEnvironmentSpec({ sandbox: { mode: 'workspace', trust: 'sometimes' }, @@ -177,6 +196,23 @@ describe('validateEnvironmentSpec', () => { }); }); +describe('legacy sandbox trust', () => { + test('reads a stored internal as public, so the seat cannot stay silently unconfined', () => { + const legacy = { mode: 'workspace', trust: 'internal' }; + expect(isLegacySandboxTrust(legacy)).toBe(true); + expect(normalizeSandboxTrust(legacy)).toEqual({ mode: 'workspace', trust: 'public' }); + }); + + test('leaves every other declaration alone, including an absent sandbox block', () => { + const publicTrust = { trust: 'public' }; + expect(normalizeSandboxTrust(publicTrust)).toBe(publicTrust); + expect(isLegacySandboxTrust(undefined)).toBe(false); + expect(isLegacySandboxTrust({})).toBe(false); + expect(isLegacySandboxTrust({ trust: 'workspace' })).toBe(false); + expect(normalizeSandboxTrust(undefined)).toBeUndefined(); + }); +}); + describe('resolveWorkspace', () => { test('expands ~ and creates the workspace dir; reports created=true', async () => { const spec = { workspace: { path: '~/projects/sandbox-research' } }; diff --git a/cli/src/lib/adapters/claude.js b/cli/src/lib/adapters/claude.js index 283eda484..5ed78dadb 100644 --- a/cli/src/lib/adapters/claude.js +++ b/cli/src/lib/adapters/claude.js @@ -61,7 +61,11 @@ import { import { homedir, tmpdir } from 'os'; import { delimiter, isAbsolute, join } from 'path'; -import { mountSkills } from '../environment.js'; +import { + isLegacySandboxTrust, + mountSkills, + normalizeSandboxTrust, +} from '../environment.js'; import { detectBwrap, wrapArgvWithBwrap } from '../sandbox/bwrap.js'; import { PUBLIC_SANDBOX_MODES, resolvePublicSandboxMode } from '../sandbox/mode.js'; import { @@ -524,7 +528,31 @@ export default { } }, - async spawn(prompt, ctx = {}) { + async spawn(prompt, rawCtx = {}) { + // `sandbox.trust: 'internal'` is refused for new declarations and read as + // `public` for a record that already carries it; such a seat is confined + // where it can be and refuses to derive where it cannot (Wren 69585). + // Resolved ONCE here and threaded to BOTH consumers — the public-state + // preparation below and the argv builder — because two independent reads of + // one declaration is exactly how this seat crashed with "public Claude + // state was not prepared" instead of spawning confined. + const ctx = rawCtx.environment + ? { + ...rawCtx, + environment: { + ...rawCtx.environment, + sandbox: normalizeSandboxTrust(rawCtx.environment.sandbox), + }, + } + : rawCtx; + if (isLegacySandboxTrust(rawCtx.environment?.sandbox)) { + // eslint-disable-next-line no-console + console.warn( + '[claude] sandbox.trust=internal is no longer accepted: it reads as ' + + 'confinement and engaged none. Resolving this seat as trust=public, so ' + + 'it is confined or it refuses to derive — never unconfined (Wren 69585).', + ); + } const isResume = !!ctx.sessionId; const sessionId = ctx.sessionId || randomUUID(); // Passed through UNCOALESCED. `ctx.memoryLongTerm || ''` was here, and diff --git a/cli/src/lib/adapters/codex.js b/cli/src/lib/adapters/codex.js index 6ed4f83ab..41979ba73 100644 --- a/cli/src/lib/adapters/codex.js +++ b/cli/src/lib/adapters/codex.js @@ -57,6 +57,7 @@ import { resolve as pathResolve, } from 'path'; import { buildMemoryPreamble } from '../memory-bridge.js'; +import { isLegacySandboxTrust, normalizeSandboxTrust } from '../environment.js'; // Default timeout for a single codex spawn (exec mode). // @@ -445,8 +446,17 @@ export default { // both macOS and Linux — so the derived default is `workspace`, and an // explicit mode in the record still wins. Before this, a mode-less public // record threw `got unset` and no codex seat spawned at all. - const publicSandboxMode = ctx.environment?.sandbox?.trust === 'public' - ? ctx.environment?.sandbox?.mode ?? 'workspace' + if (isLegacySandboxTrust(ctx.environment?.sandbox)) { + // eslint-disable-next-line no-console + console.warn( + '[codex] sandbox.trust=internal is no longer accepted: it reads as ' + + 'confinement and engaged none. Resolving this seat as trust=public ' + + '(Wren 69585).', + ); + } + const sandbox = normalizeSandboxTrust(ctx.environment?.sandbox); + const publicSandboxMode = sandbox?.trust === 'public' + ? sandbox?.mode ?? 'workspace' : null; const args = buildArgs({ sessionId: ctx.sessionId || null, diff --git a/cli/src/lib/environment.js b/cli/src/lib/environment.js index da740ab05..9219caccf 100644 --- a/cli/src/lib/environment.js +++ b/cli/src/lib/environment.js @@ -46,9 +46,30 @@ const ALLOWED_TOP_KEYS = new Set([ const ALLOWED_SANDBOX_MODES = new Set([ 'none', 'workspace', 'read-only', 'bwrap', 'firejail', 'container', 'managed', ]); -const ALLOWED_SANDBOX_TRUST = new Set(['public', 'internal']); +const ALLOWED_SANDBOX_TRUST = new Set(['public']); const ALLOWED_NETWORK_POLICIES = new Set(['unrestricted', 'restricted']); +// `trust: 'internal'` was accepted by this schema and read by NO adapter. On the +// attach path it could only name a mode the platform cannot resolve; on the +// daemon path it was silently inert — claude fell through to a bare unconfined +// spawn and codex took `--dangerously-bypass-approvals-and-sandbox` — so a +// declaration that reads as "confine me, not as a public agent" meant the +// opposite of what it said (Vera 69592). It is refused for new declarations +// above, and a record that already carries it is resolved TOWARD confinement, +// never toward the bare spawn (Wren 69585): `internal` is read as `public`, so +// such a seat is confined where it can be and refuses to derive where it +// cannot. A real middle trust arrives as its own adapter that reads it. +export const LEGACY_SANDBOX_TRUST = Object.freeze({ internal: 'public' }); +export const isLegacySandboxTrust = (sandbox) => ( + typeof sandbox?.trust === 'string' + && Object.prototype.hasOwnProperty.call(LEGACY_SANDBOX_TRUST, sandbox.trust) +); +export const normalizeSandboxTrust = (sandbox) => ( + isLegacySandboxTrust(sandbox) + ? { ...sandbox, trust: LEGACY_SANDBOX_TRUST[sandbox.trust] } + : sandbox +); + const expandHome = (p) => { if (!p || typeof p !== 'string') return p; if (p === '~') return homedir(); @@ -172,7 +193,11 @@ export const validateEnvironmentSpec = (spec) => { errors.push(`sandbox.mode must be one of: ${[...ALLOWED_SANDBOX_MODES].join(', ')}`); } if (trust !== undefined && !ALLOWED_SANDBOX_TRUST.has(trust)) { - errors.push(`sandbox.trust must be one of: ${[...ALLOWED_SANDBOX_TRUST].join(', ')}`); + errors.push( + "sandbox.trust must be 'public' — it marks a seat anyone in the pod can " + + 'talk to; omit it for your own seat ' + + `(got ${JSON.stringify(trust)})`, + ); } if (network !== undefined) { if (typeof network !== 'object' || network === null) { From 70ded0441238490b2ff590556644d65f95b6ee63 Mon Sep 17 00:00:00 2001 From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com> Date: Fri, 18 Sep 2026 07:06:40 -0700 Subject: [PATCH 4/4] fix(cli): a pi seat is not handed the sandbox its own adapter refuses (TASK-052) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sandbox half of the daemon baseline was written for every adapter that consumes mcp[] — claude, codex AND pi — because it piggybacked on ADAPTERS_WITH_DEFAULT_MCP. pi cannot enforce a sandbox, and since #1727 `sandbox.trust: 'public'` makes it fail closed BEFORE pi starts (`assertNoSandboxDeclared`, adapters/pi.js). So the default did not merely fail to confine a pi seat: it made every daemon-derived pi seat unspawnable, with `pi adapter: public-trust seats are not supported`. The seat would have been born dead rather than born unconfined, which is the failure that hides best — the daemon writes the record, the spawn throws, and the record looks correct. The predicate was the bug: "consumes mcp[]" is not "can enforce a sandbox". `ADAPTERS_WITH_DEFAULT_SANDBOX` (claude, codex) is now the set the sandbox half uses, so a pi seat gets the mcp half only, exactly as it did before this branch. The stale claim in the doc comment ("a pi seat gets the block and is not confined by it") was the opposite of the truth and is replaced with the mechanism. The C4-2 fixture in daemon-supervisor.test.mjs is a pi row and pinned the buggy both-halves shape; it now asserts the mcp half and runs the derived value through pi's own guard, because the invariant is not "no sandbox key" but "the spec this adapter receives is one this adapter accepts". Mutation-proven: reverting the guard to ADAPTERS_WITH_DEFAULT_MCP reds exactly two tests, the new seatBaseline case and that fixture. cli 39 suites / 567 passed / 10 skipped, lint:cli clean. Residual, stated rather than implied: a derived pi seat still runs unconfined, because refusing it is a policy choice (fail closed, as #1727 does when a sandbox IS declared) and not something this default gets to decide by writing a declaration pi rejects. Raised with the pod rather than settled here. --- cli/__tests__/daemon-supervisor.test.mjs | 15 +++++++----- cli/__tests__/default-environment.test.mjs | 28 ++++++++++++++++++++-- cli/src/lib/default-environment.js | 24 +++++++++++++++---- 3 files changed, 55 insertions(+), 12 deletions(-) diff --git a/cli/__tests__/daemon-supervisor.test.mjs b/cli/__tests__/daemon-supervisor.test.mjs index b1114f55c..4cdfbf5d9 100644 --- a/cli/__tests__/daemon-supervisor.test.mjs +++ b/cli/__tests__/daemon-supervisor.test.mjs @@ -9,6 +9,7 @@ import { BACKOFF_MAX_MS, createDaemonSupervisor, } from '../src/lib/daemon-supervisor.js'; +import { assertNoSandboxDeclared } from '../src/lib/adapters/pi.js'; const record = { machineDbId: '507f1f77bcf86cd799439011', @@ -140,7 +141,7 @@ describe('tick', () => { // runtime.adapter/model/effort and no environment at all, so the daemon mints // the token. Before this fix that record had no mcp[], and c4-smoke spawned // with no commonly_* tools until the operator hand-added the entry. - test('the C4-2 row shape (wrapper + pi + model/effort, no environment) mints the baseline', async () => { + test('the C4-2 row shape (wrapper + pi + model/effort, no environment) mints the mcp half only', async () => { const { supervisor, saveToken } = makeHarness({ rows: () => [boundRow({ runtime: { @@ -152,16 +153,18 @@ describe('tick', () => { await supervisor.tick(); const record = saveToken.mock.calls[0][1]; expect(record.adapter).toBe('pi'); - // The baseline is both halves: the kernel MCP server (TASK-048) and an - // enforced sandbox (TASK-052). This assertion pinned mcp-only until the - // sandbox half landed, which is the gap the C4-6 row is about. The sandbox - // carries NO mode — the adapters resolve it per platform at spawn. + // The kernel MCP server (TASK-048), and NO sandbox: pi fails closed on a + // declared sandbox (#1727), so the both-halves assertion that used to sit + // here pinned a record whose own adapter refuses to start — the seat would + // have died on every spawn with 'public-trust seats are not supported'. + // The sandbox half is asserted for claude in 'the baseline a seat nobody + // authored gets' below, which is where the enforcing adapters are covered. expect(record.environment).toEqual({ model: 'deepseek-v4-flash', effort: 'high', mcp: [expect.objectContaining({ name: 'commonly', command: ['npx', '-y', '@commonlyai/mcp@latest'] })], - sandbox: { trust: 'public' }, }); + expect(() => assertNoSandboxDeclared(record.environment)).not.toThrow(); }); test('an adapter with no mcp consumption path still invents no environment', async () => { diff --git a/cli/__tests__/default-environment.test.mjs b/cli/__tests__/default-environment.test.mjs index ca9c07976..8123706e5 100644 --- a/cli/__tests__/default-environment.test.mjs +++ b/cli/__tests__/default-environment.test.mjs @@ -2,6 +2,7 @@ // declares no commonly server (TASK-048). import { ADAPTERS_WITH_DEFAULT_MCP, + ADAPTERS_WITH_DEFAULT_SANDBOX, COMMONLY_DEFAULT_SANDBOX, COMMONLY_MCP_SERVER_NAME, commonlyMcpServer, @@ -11,6 +12,7 @@ import { withDefaultMcpServer, withDefaultSandbox, } from '../src/lib/default-environment.js'; +import { assertNoSandboxDeclared } from '../src/lib/adapters/pi.js'; describe('defaultMcpServers', () => { test.each(['claude', 'codex', 'pi'])('hands %s the commonly server', (adapterName) => { @@ -33,6 +35,11 @@ describe('defaultMcpServers', () => { test('ADAPTERS_WITH_DEFAULT_MCP matches the adapters this test names', () => { expect([...ADAPTERS_WITH_DEFAULT_MCP].sort()).toEqual(['claude', 'codex', 'pi']); }); + + test('ADAPTERS_WITH_DEFAULT_SANDBOX is the enforcing subset, pi excluded', () => { + expect([...ADAPTERS_WITH_DEFAULT_SANDBOX].sort()).toEqual(['claude', 'codex']); + expect(ADAPTERS_WITH_DEFAULT_SANDBOX.has('pi')).toBe(false); + }); }); describe('withDefaultMcpServer', () => { @@ -172,7 +179,24 @@ describe('seatBaseline', () => { }); test('is idempotent on both halves', () => { - const once = seatBaseline(null, 'pi', { sandbox: true }); - expect(seatBaseline(once, 'pi', { sandbox: true })).toBe(once); + const once = seatBaseline(null, 'codex', { sandbox: true }); + expect(seatBaseline(once, 'codex', { sandbox: true })).toBe(once); + }); + + // The sandbox half is written for the adapters that can enforce it, and NOT + // for every adapter that consumes mcp[]. pi fails closed on a declared + // sandbox (#1727), so a baseline carrying one is a seat that cannot start — + // the invariant is not "no sandbox key" but "the spec this adapter receives + // is one this adapter accepts", which is what is asserted here by running the + // derived value through pi's own guard rather than by inspecting a key. + test('gives pi the mcp half only, and a spec its own guard accepts', () => { + const derived = seatBaseline(null, 'pi', { sandbox: true }); + expect(derived.sandbox).toBeUndefined(); + expect(derived).toEqual({ mcp: [commonlyMcpServer()] }); + expect(() => assertNoSandboxDeclared(derived)).not.toThrow(); + // The control: the same baseline handed to an enforcing adapter does carry + // it, so the absence above is the adapter set and not a missing default. + expect(seatBaseline(null, 'claude', { sandbox: true }).sandbox) + .toEqual(defaultSeatSandbox()); }); }); diff --git a/cli/src/lib/default-environment.js b/cli/src/lib/default-environment.js index a79946491..7597b4af8 100644 --- a/cli/src/lib/default-environment.js +++ b/cli/src/lib/default-environment.js @@ -24,6 +24,22 @@ export const ADAPTERS_WITH_DEFAULT_MCP = new Set(['claude', 'codex', 'pi']); +/** + * The adapters that can ENFORCE the default sandbox — a strict subset of the + * ones that consume `mcp[]`. + * + * `pi` is absent deliberately. It has no sandbox path until #1740's transport + * work gives it one, and since #1727 it REFUSES TO START on a spec that + * declares one: `assertNoSandboxDeclared` in adapters/pi.js throws on + * `trust: 'public'` and on any `mode` other than 'none'. So handing pi this + * block is not a harmless no-op — it is an unspawnable seat. A pi seat gets the + * `mcp[]` half only, and the residual is that such a seat runs unconfined: that + * belongs to the row that owns pi confinement, not to a declaration written + * here and hoped for. A derived pi seat used to be exactly this shape and would + * have failed every spawn with `public-trust seats are not supported`. + */ +export const ADAPTERS_WITH_DEFAULT_SANDBOX = new Set(['claude', 'codex']); + /** * The sandbox an unconfigured seat gets. * @@ -45,9 +61,9 @@ export const ADAPTERS_WITH_DEFAULT_MCP = new Set(['claude', 'codex', 'pi']); * moves a host fact into the database and breaks the day the seat is re-homed. * An explicit mode in a record still wins over the derived one. * - * Confinement holds for claude and codex. A pi seat gets the block and is NOT - * confined by it — the pi adapter has no sandbox path until #1740's transport - * work gives it one, so do not read this key on a pi record as confinement. + * Confinement holds for claude and codex, and only they are ever handed this + * block (ADAPTERS_WITH_DEFAULT_SANDBOX). A pi seat must never be: it fails + * closed on a declared sandbox rather than run under a spec it cannot honour. */ export const COMMONLY_DEFAULT_SANDBOX = Object.freeze({ trust: 'public' }); @@ -132,6 +148,6 @@ export const withDefaultSandbox = (environment) => { */ export const seatBaseline = (environment, adapterName, { sandbox = false } = {}) => { const withMcp = withDefaultMcpServer(environment, adapterName); - if (!sandbox || !ADAPTERS_WITH_DEFAULT_MCP.has(adapterName)) return withMcp; + if (!sandbox || !ADAPTERS_WITH_DEFAULT_SANDBOX.has(adapterName)) return withMcp; return withDefaultSandbox(withMcp); };