From afc68a204d910337a9515d175b8653ffba3100f3 Mon Sep 17 00:00:00 2001 From: Juan Cruz Fortunatti Date: Sun, 13 Sep 2026 15:36:31 +0200 Subject: [PATCH] fix: isolate Codex tool discovery from ambient apps --- docs/engines.md | 13 +++++ src/pi/cliEngineSpawn.test.ts | 6 +- src/pi/cliEngineSpawn.ts | 6 +- src/pi/cliSessionToolIsolation.test.ts | 78 ++++++++++++++++++++++++++ 4 files changed, 99 insertions(+), 4 deletions(-) create mode 100644 src/pi/cliSessionToolIsolation.test.ts diff --git a/docs/engines.md b/docs/engines.md index 7c1cdb9..7512b3a 100644 --- a/docs/engines.md +++ b/docs/engines.md @@ -62,6 +62,19 @@ standalone and strict sandbox launches. If the server cannot initialize, Codex fails the wake before model work instead of continuing with only built-in tools. The startup error follows the existing bounded, redacted engine failure path. +Daimon also disables Codex subscription apps and installed plugins on every +wake with `features.apps=false` and `features.plugins=false`. This keeps ambient +account tools out of discovery while preserving the declared per-wake MCP +tools and built-in coding tools. These settings are invocation arguments; +changing an engine home's config does not affect a strict launch that ignores +user configuration. Caller `--enable` arguments are rejected because Codex +applies them after ordinary config overrides. + +Codex may still defer declared tools behind `tool_search`. Server startup and +tool discovery are separate checks; `list_mcp_resources` does not list tools. +Codex 0.142.3 accepts the two isolation settings above, but its former +`tool_search` feature toggle is a removed no-op and cannot disable discovery. + The production Grok path uses an external Daimon engine broker with one durable subscription credential authority. Agent workers receive scoped capabilities; the broker owns refresh and stale-credential recovery. The runtime checks broker diff --git a/src/pi/cliEngineSpawn.test.ts b/src/pi/cliEngineSpawn.test.ts index b60f768..ed89655 100644 --- a/src/pi/cliEngineSpawn.test.ts +++ b/src/pi/cliEngineSpawn.test.ts @@ -57,7 +57,7 @@ for (const mode of ["standalone", "strict"] as const) { test("codex argv preserves model defaults while requiring the per-wake MCP server", () => { const args = renderCodexArgs({ commandArgs: [] }, "/workspace", "http://127.0.0.1:1/mcp"); assert.deepEqual(args, ["exec", "--sandbox", "danger-full-access", "--skip-git-repo-check", "--color", "never", "--json", "-C", "/workspace", - "-c", "mcp_servers.daimon.url=http://127.0.0.1:1/mcp", "-c", "mcp_servers.daimon.enabled=true", "-c", "mcp_servers.daimon.required=true", "-"]); + "-c", "mcp_servers.daimon.url=http://127.0.0.1:1/mcp", "-c", "features.apps=false", "-c", "features.plugins=false", "-c", "mcp_servers.daimon.enabled=true", "-c", "mcp_servers.daimon.required=true", "-"]); }); test("codex strict policy is rendered as per-turn Daimon-owned CLI config", () => { @@ -133,13 +133,13 @@ test("Codex permission profile lets protected denies override readable paths", ( test("codex argv renders -m for a pinned model and leaves everything else untouched", () => { const args = renderCodexArgs({ commandArgs: [], model: "gpt-5-codex" }, "/workspace", "http://127.0.0.1:1/mcp"); assert.deepEqual(args, ["exec", "--sandbox", "danger-full-access", "--skip-git-repo-check", "--model=gpt-5-codex", "--color", "never", "--json", "-C", "/workspace", - "-c", "mcp_servers.daimon.url=http://127.0.0.1:1/mcp", "-c", "mcp_servers.daimon.enabled=true", "-c", "mcp_servers.daimon.required=true", "-"]); + "-c", "mcp_servers.daimon.url=http://127.0.0.1:1/mcp", "-c", "features.apps=false", "-c", "features.plugins=false", "-c", "mcp_servers.daimon.enabled=true", "-c", "mcp_servers.daimon.required=true", "-"]); }); test("codex argv renders both model and reasoningEffort together in stable order", () => { const args = renderCodexArgs({ commandArgs: [], model: "gpt-5-codex", reasoningEffort: "xhigh" }, "/workspace", "http://127.0.0.1:1/mcp"); assert.deepEqual(args, ["exec", "--sandbox", "danger-full-access", "--skip-git-repo-check", "--model=gpt-5-codex", "-c", "model_reasoning_effort=xhigh", "--color", "never", "--json", "-C", "/workspace", - "-c", "mcp_servers.daimon.url=http://127.0.0.1:1/mcp", "-c", "mcp_servers.daimon.enabled=true", "-c", "mcp_servers.daimon.required=true", "-"]); + "-c", "mcp_servers.daimon.url=http://127.0.0.1:1/mcp", "-c", "features.apps=false", "-c", "features.plugins=false", "-c", "mcp_servers.daimon.enabled=true", "-c", "mcp_servers.daimon.required=true", "-"]); }); test("codex argv renders reasoningEffort alone without a model flag", () => { diff --git a/src/pi/cliEngineSpawn.ts b/src/pi/cliEngineSpawn.ts index eedae24..861cc58 100644 --- a/src/pi/cliEngineSpawn.ts +++ b/src/pi/cliEngineSpawn.ts @@ -27,6 +27,9 @@ export const renderGrokSandboxArgs = ( * * The per-wake MCP server is always enabled and required. Otherwise Codex can * continue a normal turn with only built-in tools after MCP startup fails. + * Subscription apps and installed plugins are not caller-declared tools. + * Disable their discovery per invocation, including with strict config: the + * CLI's defaults can otherwise add them even when user config is ignored. */ export const renderCodexArgs = ( options: Pick, @@ -55,6 +58,7 @@ export const renderCodexArgs = ( ...(options.reasoningEffort === undefined ? [] : ["-c", `model_reasoning_effort=${options.reasoningEffort}`]), "--color", "never", "--json", "-C", cwd, "-c", `mcp_servers.daimon.url=${endpoint}`, + "-c", "features.apps=false", "-c", "features.plugins=false", "-c", "mcp_servers.daimon.enabled=true", "-c", "mcp_servers.daimon.required=true", "-"]; }; @@ -165,7 +169,7 @@ const assertSafeAgyCommandArgs = (args: readonly string[] | undefined): readonly /** Caller arguments cannot reopen Codex's sandbox, output, cwd, or config boundary. */ const assertSafeCodexCommandArgs = (args: readonly string[] | undefined, strictPolicy = false): readonly string[] => { const values = args ?? []; - const pattern = /^(?:--json|--sandbox|--dangerously-bypass-approvals-and-sandbox|--output-last-message|--config|--ignore-user-config|--ignore-rules|--skip-git-repo-check|--color|--cd|-c|-C)(?:=|$)/u; + const pattern = /^(?:--json|--sandbox|--dangerously-bypass-approvals-and-sandbox|--output-last-message|--config|--ignore-user-config|--ignore-rules|--skip-git-repo-check|--color|--cd|--enable|-c|-C)(?:=|$)/u; const strictPattern = /^(?:--strict-config|--profile|-p|-P|--permissions-profile|--enable|--disable|--add-dir|--search)(?:=|$)|^(?:default_permissions|permissions)(?:=|\.)/u; if (values.some((value) => pattern.test(value) || (strictPolicy && strictPattern.test(value)))) { throw new Error("Codex security-boundary arguments are Daimon-owned"); diff --git a/src/pi/cliSessionToolIsolation.test.ts b/src/pi/cliSessionToolIsolation.test.ts new file mode 100644 index 0000000..fa29daf --- /dev/null +++ b/src/pi/cliSessionToolIsolation.test.ts @@ -0,0 +1,78 @@ +import assert from "node:assert/strict"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { createRequire } from "node:module"; +import os from "node:os"; +import path from "node:path"; +import { pathToFileURL } from "node:url"; +import test from "node:test"; + +import { createCliSessionFactory } from "./cliSession.js"; +import { renderCodexArgs } from "./cliEngineSpawn.js"; + +const require = createRequire(import.meta.url); +const clientEntry = pathToFileURL(require.resolve("@modelcontextprotocol/sdk/client/index.js")).href; +const transportEntry = pathToFileURL(require.resolve("@modelcontextprotocol/sdk/client/streamableHttp.js")).href; + +for (const mode of ["standalone", "strict"] as const) { + test(`Codex ${mode} isolates ambient discovery while the declared MCP tool remains callable`, async () => { + const root = await mkdtemp(path.join(os.tmpdir(), "daimon-tool-isolation-")); + const engine = path.join(root, "engine.mjs"); + await writeFile(engine, [ + `import { Client } from ${JSON.stringify(clientEntry)};`, + `import { StreamableHTTPClientTransport } from ${JSON.stringify(transportEntry)};`, + "for await (const chunk of process.stdin) {}", + "const config = new Map(process.argv.flatMap((arg, index, args) => arg === '-c' ? [args[index + 1].split(/=(.*)/s).slice(0, 2)] : []));", + // Codex 0.142.3 enables these stable features by default, even with + // --ignore-user-config. The stub models that default; it does not supply + // an opt-out to the adapter under test. + "const ambient = ['apps', 'plugins'].filter(name => config.get('features.' + name) !== 'false');", + "const client = new Client({ name: 'isolation-test', version: '1' });", + "await client.connect(new StreamableHTTPClientTransport(new URL(config.get('mcp_servers.daimon.url'))));", + "const listed = await client.listTools();", + "const result = await client.callTool({ name: 'declared_lookup', arguments: {} });", + "await client.close();", + "const text = JSON.stringify({ ambient, listed: listed.tools.map(tool => tool.name), result });", + "process.stdout.write(JSON.stringify({ type: 'item.completed', item: { type: 'agent_message', text } }) + '\\n');", + "process.stdout.write(JSON.stringify({ type: 'turn.completed' }) + '\\n');" + ].join("\n")); + let invoked = false; + let output = ""; + const { session } = await createCliSessionFactory({ + command: process.execPath, commandArgs: [engine], engine: "codex", timeoutMs: 10_000, + ...(mode === "strict" ? { codexSandbox: { mode: "workspace-write", networkAccess: false, webSearch: "disabled" } as const } : {}) + })({ cwd: root, customTools: [{ + name: "declared_lookup", label: "Declared lookup", description: "Return the declared result.", + parameters: { type: "object", additionalProperties: false, properties: {} }, + async execute() { + invoked = true; + return { content: [{ type: "text", text: "verified" }] }; + } + } as never] }); + session.subscribe((event) => { + if (event.type === "turn_end" && "content" in event.message) { + const content = event.message.content; + output = typeof content === "string" ? content : content.filter(item => item.type === "text").map(item => item.text).join(""); + } + }); + try { + await session.prompt("Use the declared lookup."); + const report = JSON.parse(output) as { ambient: string[]; listed: string[]; result: { content: unknown[] } }; + assert.deepEqual(report.ambient, []); + assert.deepEqual(report.listed, ["declared_lookup"]); + assert.equal(invoked, true); + assert.deepEqual(report.result.content, [{ type: "text", text: "verified" }]); + } finally { + await session.disposeAsync?.(); + await rm(root, { recursive: true, force: true }); + } + }); + + test(`Codex ${mode} caller cannot re-enable ambient discovery`, () => { + for (const commandArgs of [["--enable", "apps"], ["--enable=plugins"], ["-c", "features.apps=true"]]) { + assert.throws(() => renderCodexArgs({ + commandArgs, + ...(mode === "strict" ? { codexSandbox: { mode: "workspace-write", networkAccess: false, webSearch: "disabled" } as const } : {}) + }, "/workspace", "http://127.0.0.1:1/mcp"), /Daimon-owned/u); + } + }); +}