diff --git a/.codex/config.toml b/.codex/config.toml index 9b1f51190b..1c16b5cb04 100644 --- a/.codex/config.toml +++ b/.codex/config.toml @@ -3,8 +3,8 @@ # # checked-in policy: # - enabled = false so ordinary/offline Codex sessions do not initialize providers -# - default_tools_approval_mode = "auto" so once a connected Cloud/host layer enables -# a server for a task, agents use it without per-tool permission prompts +# - read-only Supabase tools use "auto"; write-capable Figma/Railway/Sentry tools use +# "writes" so reads stay frictionless while writes still require explicit approval # - Paid API canaries (eval:rag, eval:retrieval:quality, eval:quality, verify:release, # test:live, check:supabase-project) still need explicit confirmation per AGENTS.md # Runtime Cloud MCP remains `.mcp.json` (Railway + read-only Supabase only). @@ -12,7 +12,7 @@ [mcp_servers.figma_cloud] url = "https://mcp.figma.com/mcp" enabled = false -default_tools_approval_mode = "auto" +default_tools_approval_mode = "writes" [mcp_servers.supabase_cloud] # Production project, read_only=true, approved feature groups only. @@ -24,9 +24,9 @@ default_tools_approval_mode = "auto" [mcp_servers.railway_cloud] url = "https://mcp.railway.com" enabled = false -default_tools_approval_mode = "auto" +default_tools_approval_mode = "writes" [mcp_servers.sentry_cloud] url = "https://mcp.sentry.dev/mcp" enabled = false -default_tools_approval_mode = "auto" +default_tools_approval_mode = "writes" diff --git a/AGENTS.md b/AGENTS.md index 0dd8abbbb5..e092df524e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -870,8 +870,9 @@ Use `docs/codex-cloud.md` as the environment contract: - Configure maintenance as `bash scripts/maintain-codex-cloud.sh && bash scripts/install-codex-cloud-command-shims.sh`. - Default to `CODEX_CLOUD_ACCESS_PROFILE=offline` for ordinary and protected RAG work. Use `connected` only when the user explicitly authorizes the required provider access. -- When MCP tools are already available in a Cloud session and the task needs them, use them - without asking for per-tool permission. Paid API canaries (`eval:rag`, +- When MCP tools are already available in a Cloud session and the task needs them, read-only tools + can run without per-tool approval. Write-capable Figma, Railway, and Sentry tools still require + explicit confirmation. Paid API canaries (`eval:rag`, `eval:retrieval:quality`, `eval:quality`, `verify:release`, `test:live`, `check:supabase-project`) still need explicit confirmation. Project `.codex/config.toml` keeps MCP entries `enabled = false` so ordinary/offline Codex hosts do not initialize them; diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 71222d951d..c7cdd329f5 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -593,5 +593,9 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-08-02 | claude/ds-v2-architecture | 93bb4b4756a7fad22f93008325f2c0f72471b0db | PR-Arch Wave4 motion/z/overlays/print | local gates green; frontend-ui-reviewer API-limited — glance required before auto-merge | unit 4972p; e2e:critical 15p; verify:ui 347p; verify:pr-local 0; eval:rag:offline pass | | 2026-08-02 | claude/ds-v2-architecture | 649389ba7223f67281c8f3836dd542997c5cbd83 | PR #1583 review-and-fix | fixed Devin --ease-out Tailwind collision as --ease-out-keyword; synced main; Codex ledger-squash note outdated vs tip | vitest overlay+ckb-v2 34p; npm run test 4973p; merge-tree clean | | 2026-08-02 | claude/ds-v2-architecture | 84147ee123bde50fceefad627d8c89791b27a713 | PR #1583 review-and-fix | fixed Devin --ease-out Tailwind collision as --ease-out-keyword; synced main; Codex ledger-squash note outdated vs tip | vitest overlay+ckb-v2 34p; npm run test 4973p; merge-tree clean | +| 2026-08-02 | codex/mcp-config-hardening-merge | 020cbfaab3257c8bb873b31d1617f9f17a9361b3 | MCP Cloud config hardening | No findings; write approvals and fail-closed validation verified | check:codex-cloud; focused MCP config test | +| 2026-08-02 | codex/mcp-config-hardening-merge | a4a6a584f2ced6ffcd3a0fe0cb2471bbe71db7be | MCP Cloud config hardening | Supersedes prior review; no findings after Windows shell-test guard | check:codex-cloud; Cloud tests 16 passed, 2 Windows-skipped | +| 2026-08-02 | codex/mcp-config-hardening-merge | 4b0d7b712b789e771643f2b7dce85673778a5a7d | MCP Cloud config hardening | Supersedes prior review; parser bypasses fixed with separate metadata | check:codex-cloud; Cloud config tests | | 2026-08-02 | codex/cloud-python-self-diagnosis | 2de7b7abbdea9a1286b198e14eecb021fff32693 | Cloud Python lock and self-diagnostics | PASS: no actionable findings | verify:pr-local PASS; focused Vitest 21/21; Python 3.11 and 3.12 lock reproduction PASS | | 2026-08-02 | codex/cloud-python-self-diagnosis | 32cb80020bf1a63628dbf805f54f393aee5af528 | Cloud Python lock and self-diagnostics | PASS: review findings fixed; fail return claim refuted by Bash execution proof | verify:pr-local PASS; focused Vitest 22/22; Bash ERR trap proof PASS | +| 2026-08-02 | codex/mcp-config-hardening-merge | 30aac79e655225f41006d5739fc4e91ef7791514 | MCP Cloud config hardening | Supersedes prior review; synced main and retained parser hardening with Windows Cloud test coverage | check:codex-cloud; Cloud/Python tests 22 passed | diff --git a/docs/codebase-index.md b/docs/codebase-index.md index 58720e91f9..adc6096dd8 100644 --- a/docs/codebase-index.md +++ b/docs/codebase-index.md @@ -42,7 +42,7 @@ Smaller top-level directories that are easy to miss: | `plugins/` | `plugins/clinical-kb/` Codex plugin manifest and workflow skill | | `.agents/` | Single-word skill catalogue (`npm run skills`, validated by `npm run check:skills`) | | `.claude/` | Claude Code agents, skills, hooks, settings — plus the `.claude/worktrees/` working copies | -| `.codex/` | Project-scoped Codex host config; tracked `config.toml` holds secret-free MCP registrations (Figma, read-only Supabase, Railway, Sentry) with `enabled = false` and `default_tools_approval_mode = "auto"`. Runtime Cloud MCP stays `.mcp.json`. Other `.codex/*` stays gitignored; OAuth stays in the host credential store | +| `.codex/` | Project-scoped Codex host config; tracked `config.toml` has disabled, secret-free Figma, Supabase, Railway, and Sentry MCP registrations. Figma/Railway/Sentry use `"writes"`; read-only Supabase uses `"auto"`. Runtime Cloud MCP stays `.mcp.json`; all `.codex/*` stay ignored; OAuth stays in the host credential store. | | `.cursor/` | Cursor project rules and local-agent configuration | | `.design-sync/` | Generated design-system package metadata, validation notes, and project-sync artifacts | | `.githooks/` | Installed by `npm install`; `pre-push` runs `scripts/guard-push.mjs` (format, auto-merge race, drift staleness) | diff --git a/docs/codex-cloud.md b/docs/codex-cloud.md index 9b9503df92..369e30adfd 100644 --- a/docs/codex-cloud.md +++ b/docs/codex-cloud.md @@ -283,14 +283,13 @@ hosts load in addition to `$CODEX_HOME/config.toml` (where `setup-codex-cloud.sh shell-environment policy). It is not inert documentation: Codex applies project-local `.codex/config.toml` when the project is trusted. The tracked template lists Figma (`https://mcp.figma.com/mcp`), Railway, read-only Supabase, and Sentry -(`https://mcp.sentry.dev/mcp`) as URL-only registrations with `enabled = false` and -`default_tools_approval_mode = "auto"`. Ordinary/offline sessions therefore do not initialize -those providers. When a connected Cloud or host layer enables a server for a task, tools are -auto-approved so agents avoid per-tool permission prompts; paid API canaries, Railway -mutations/deploys, and other cost-or-write provider actions still require explicit confirmation -per AGENTS.md. Figma and Sentry OAuth credentials stay in the host credential store — never in -the tracked file. Runtime Cloud MCP allowlist remains `.mcp.json` (Railway + read-only Supabase -only). `npm run check:codex-cloud` validates both files. +(`https://mcp.sentry.dev/mcp`) as URL-only registrations with `enabled = false`. Ordinary/offline +sessions therefore do not initialize those providers. Production read-only Supabase uses +`default_tools_approval_mode = "auto"`; write-capable Figma, Railway, and Sentry use `"writes"` +so reads avoid per-tool prompts while writes still require explicit confirmation per AGENTS.md. +Paid API canaries also require explicit confirmation. Figma and Sentry OAuth credentials stay in +the host credential store — never in the tracked file. Runtime Cloud MCP allowlist remains +`.mcp.json` (Railway + read-only Supabase only). `npm run check:codex-cloud` validates both files. In a fresh connected Cloud session, run `npm run check:codex-cloud -- --environment` before any provider call. The sanitized report must show `CODEX_CLOUD_ACCESS_PROFILE=connected`, every diff --git a/scripts/check-codex-cloud-setup.mjs b/scripts/check-codex-cloud-setup.mjs index 36fbe26de4..feb0c93a41 100644 --- a/scripts/check-codex-cloud-setup.mjs +++ b/scripts/check-codex-cloud-setup.mjs @@ -29,15 +29,18 @@ export const expectedMcpConfiguration = Object.freeze({ /** Project `.codex/config.toml` registrations — disabled by default, secret-free URLs only. */ export const expectedCodexProjectMcpServers = Object.freeze({ - figma_cloud: Object.freeze({ url: "https://mcp.figma.com/mcp" }), - railway_cloud: Object.freeze({ url: expectedMcpConfiguration.railwayUrl }), - sentry_cloud: Object.freeze({ url: "https://mcp.sentry.dev/mcp" }), + figma_cloud: Object.freeze({ url: "https://mcp.figma.com/mcp", approvalMode: "writes" }), + railway_cloud: Object.freeze({ url: expectedMcpConfiguration.railwayUrl, approvalMode: "writes" }), + sentry_cloud: Object.freeze({ url: "https://mcp.sentry.dev/mcp", approvalMode: "writes" }), supabase_cloud: Object.freeze({ // URL validated with the same project/read-only/feature rules as `.mcp.json`. kind: "supabase", + approvalMode: "auto", }), }); +const allowedCodexProjectMcpKeys = Object.freeze(["default_tools_approval_mode", "enabled", "url"]); + const forbiddenCodexProjectMcpKeys = Object.freeze([ "bearer_token_env_var", "command", @@ -62,11 +65,13 @@ function parseTomlScalar(value) { * Parse `[mcp_servers.name]` tables from a Codex project config.toml. * Supports only the scalar keys this gate governs. * @param {string} text - * @returns {Record>} + * @returns {{ servers: Record>, nestedServers: Set, unparsedServers: Set }} */ export function parseCodexProjectMcpServers(text) { /** @type {Record>} */ const servers = {}; + const nestedServers = new Set(); + const unparsedServers = new Set(); let current = null; for (const rawLine of text.split(/\r?\n/)) { const line = rawLine.trim(); @@ -76,7 +81,7 @@ export function parseCodexProjectMcpServers(text) { if (nested) { current = nested[1]; servers[current] ??= {}; - servers[current].__hasNestedTables = true; + nestedServers.add(current); continue; } @@ -91,13 +96,23 @@ export function parseCodexProjectMcpServers(text) { current = null; continue; } - if (!current) continue; + if (!current) { + const dotted = line.match(/^mcp_servers\.([A-Za-z0-9_-]+)\.([A-Za-z0-9_.-]+)\s*=\s*(.+)$/); + if (dotted) { + servers[dotted[1]] ??= {}; + servers[dotted[1]][dotted[2]] = parseTomlScalar(dotted[3]); + } + continue; + } - const kv = line.match(/^([A-Za-z0-9_]+)\s*=\s*(.+)$/); - if (!kv) continue; + const kv = line.match(/^([A-Za-z0-9_.-]+)\s*=\s*(.+)$/); + if (!kv) { + unparsedServers.add(current); + continue; + } servers[current][kv[1]] = parseTomlScalar(kv[2]); } - return servers; + return { servers, nestedServers, unparsedServers }; } function validateSupabaseMcpUrl(urlString, label, errors) { @@ -133,7 +148,7 @@ function validateSupabaseMcpUrl(urlString, label, errors) { */ export function validateCodexProjectMcpConfiguration(text) { const errors = []; - const servers = parseCodexProjectMcpServers(text); + const { servers, nestedServers, unparsedServers } = parseCodexProjectMcpServers(text); const expectedNames = Object.keys(expectedCodexProjectMcpServers).sort(); const actualNames = Object.keys(servers).sort(); if (JSON.stringify(actualNames) !== JSON.stringify(expectedNames)) { @@ -144,29 +159,37 @@ export function validateCodexProjectMcpConfiguration(text) { const server = servers[name]; if (!server) continue; const label = `.codex/config.toml ${name}`; + const expected = expectedCodexProjectMcpServers[name]; if (server.enabled !== false) { errors.push(`${label} must set enabled = false (host/connected layers opt in).`); } - if (server.default_tools_approval_mode !== "auto") { - errors.push( - `${label} must set default_tools_approval_mode = "auto" so connected Cloud tasks avoid per-tool prompts.`, - ); + if (server.default_tools_approval_mode !== expected.approvalMode) { + const reason = + expected.approvalMode === "writes" + ? "write-capable tools require explicit approval" + : "the production server is constrained read-only"; + errors.push(`${label} must set default_tools_approval_mode = "${expected.approvalMode}" because ${reason}.`); } - for (const key of forbiddenCodexProjectMcpKeys) { - if (server[key] !== undefined) { - errors.push(`${label} must not embed ${key}; keep OAuth credentials in the host store.`); + for (const key of Object.keys(server)) { + const rootKey = key.split(".")[0]; + if (forbiddenCodexProjectMcpKeys.includes(rootKey)) { + errors.push(`${label} must not embed ${rootKey}; keep OAuth credentials in the host store.`); + } else if (!allowedCodexProjectMcpKeys.includes(key)) { + errors.push(`${label} must be URL-only; unsupported key ${key}.`); } } - if (server.__hasNestedTables) { + if (nestedServers.has(name)) { errors.push(`${label} must not declare nested tool override tables in the shared project config.`); } + if (unparsedServers.has(name)) { + errors.push(`${label} contains unsupported or unparsed entries.`); + } if (typeof server.url !== "string" || !server.url) { errors.push(`${label} must declare a secret-free url.`); continue; } - const expected = expectedCodexProjectMcpServers[name]; if (expected.kind === "supabase") { validateSupabaseMcpUrl(server.url, label, errors); } else if (server.url !== expected.url) { diff --git a/tests/codex-cloud-setup.test.ts b/tests/codex-cloud-setup.test.ts index cab39a6e9f..e0369cf212 100644 --- a/tests/codex-cloud-setup.test.ts +++ b/tests/codex-cloud-setup.test.ts @@ -317,10 +317,17 @@ describe("Codex Cloud environment contract", () => { ); expect( validateCodexProjectMcpConfiguration( - tracked.replaceAll('default_tools_approval_mode = "auto"', 'default_tools_approval_mode = "prompt"'), + tracked.replace('default_tools_approval_mode = "writes"', 'default_tools_approval_mode = "auto"'), ), ).toContain( - `.codex/config.toml figma_cloud must set default_tools_approval_mode = "auto" so connected Cloud tasks avoid per-tool prompts.`, + `.codex/config.toml figma_cloud must set default_tools_approval_mode = "writes" because write-capable tools require explicit approval.`, + ); + expect( + validateCodexProjectMcpConfiguration( + tracked.replace('default_tools_approval_mode = "auto"', 'default_tools_approval_mode = "writes"'), + ), + ).toContain( + `.codex/config.toml supabase_cloud must set default_tools_approval_mode = "auto" because the production server is constrained read-only.`, ); expect( validateCodexProjectMcpConfiguration( @@ -332,6 +339,52 @@ describe("Codex Cloud environment contract", () => { ).toContain( `.codex/config.toml figma_cloud must not embed bearer_token_env_var; keep OAuth credentials in the host store.`, ); + expect( + validateCodexProjectMcpConfiguration( + tracked.replace( + 'url = "https://mcp.figma.com/mcp"', + 'url = "https://mcp.figma.com/mcp"\nhttp_headers.Authorization = "Bearer redacted-test"', + ), + ), + ).toContain( + `.codex/config.toml figma_cloud must not embed http_headers; keep OAuth credentials in the host store.`, + ); + expect( + validateCodexProjectMcpConfiguration( + tracked.replace( + 'url = "https://mcp.figma.com/mcp"', + 'url = "https://mcp.figma.com/mcp"\nscopes = ["files:write"]', + ), + ), + ).toContain(`.codex/config.toml figma_cloud must be URL-only; unsupported key scopes.`); + expect( + validateCodexProjectMcpConfiguration( + tracked.replace( + 'url = "https://mcp.figma.com/mcp"', + 'url = "https://mcp.figma.com/mcp"\n__anything = "secret"', + ), + ), + ).toContain(`.codex/config.toml figma_cloud must be URL-only; unsupported key __anything.`); + expect( + validateCodexProjectMcpConfiguration( + tracked.replace( + "[mcp_servers.supabase_cloud]", + "[mcp_servers.figma_cloud.tools]\n__hasNestedTables = false\n\n[mcp_servers.supabase_cloud]", + ), + ), + ).toContain( + `.codex/config.toml figma_cloud must not declare nested tool override tables in the shared project config.`, + ); + expect( + validateCodexProjectMcpConfiguration( + tracked.replace( + "[mcp_servers.figma_cloud]", + 'mcp_servers.figma_cloud.http_headers.Authorization = "Bearer redacted-test"\n\n[mcp_servers.figma_cloud]', + ), + ), + ).toContain( + `.codex/config.toml figma_cloud must not embed http_headers; keep OAuth credentials in the host store.`, + ); expect( validateCodexProjectMcpConfiguration( tracked.replace(