From 585cab41ccc6edcbd1847d8c40a5fc757f280236 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Mon, 21 Sep 2026 15:30:52 -0400 Subject: [PATCH] =?UTF-8?q?fix(#4770):=20lift=20the=20Codex=20sandbox=5Fmo?= =?UTF-8?q?de=20holds=20=E2=80=94=20documented=20enforcement=20suffices=20?= =?UTF-8?q?(#4920)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(#4770): lift the Codex sandbox_mode holds — documented enforcement suffices * docs(#4770): backfill the changeset PR number --------- Co-authored-by: sim --- .changeset/lively-tunas-swim.md | 5 + src/codex-agent-toml.cts | 46 ++++--- tests/codex-config-agents.test.cjs | 2 +- tests/codex-config-hooks.test.cjs | 2 +- tests/codex-config-install.test.cjs | 2 +- tests/codex-config.test.cjs | 180 ++++++++++++++++----------- tests/helpers/emitted-provenance.cjs | 5 + 7 files changed, 142 insertions(+), 100 deletions(-) create mode 100644 .changeset/lively-tunas-swim.md diff --git a/.changeset/lively-tunas-swim.md b/.changeset/lively-tunas-swim.md new file mode 100644 index 000000000..af1913abd --- /dev/null +++ b/.changeset/lively-tunas-swim.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4920 +--- +**Codex agents with Write/Edit tool contracts now run under workspace-write** — the 17-role read-only hold is lifted: official OpenAI documentation establishes sandbox_mode as an enforced boundary, so each Codex agent's sandbox now derives purely from its own declared tools. (#4770) diff --git a/src/codex-agent-toml.cts b/src/codex-agent-toml.cts index 0b3f3be5c..018f8009e 100644 --- a/src/codex-agent-toml.cts +++ b/src/codex-agent-toml.cts @@ -448,9 +448,9 @@ export function stripReasoningEffort(doc: CodexAgentDoc): CodexAgentDoc { /** * The 17 roles measured as widening under derivation (declare Write/Edit, * never in the pre-#3897 `CODEX_AGENT_SANDBOX` map, so the old - * `|| 'read-only'` fallback silently under-granted them). Pinned to - * `read-only` pending the open question of whether Codex enforces - * `sandbox_mode` or treats it as advisory (HALT.md). This list is CLOSED and + * `|| 'read-only'` fallback silently under-granted them) — pinned to + * `read-only` from 2026-09-08 (HALT.md) until #4770 lifted the hold on + * 2026-09-21. This list is CLOSED and * SHRINK-ONLY: a new writing role never lands here (S6, T26); it is validated * against the live tool contract every time it is consulted * ({@link _deriveCodexSandboxModeFromTools}) and against the real @@ -467,29 +467,25 @@ export function stripReasoningEffort(doc: CodexAgentDoc): CodexAgentDoc { * parses the list correctly, so this role genuinely derives * `workspace-write` from its tool contract — HALT.md's original 16-role * count measured against the pre-fix (single-line) readers and undercounted - * this role. It is held here for the same reason as the other 16: pending - * Codex's `sandbox_mode` enforcement decision, not because the derivation is - * wrong. + * this role. + * + * **LIFTED 2026-09-21 (#4770, maintainer decision: documented enforcement + * suffices).** The map is empty: the rung-3 hold's recorded reopen condition + * — official OpenAI documentation establishing Codex `sandbox_mode` as an + * enforced technical boundary that custom subagent TOML files honor — is + * satisfied, so every role now derives `sandbox_mode` purely from its own + * `tools:` frontmatter (`workspace-write` iff Write/Edit is declared). The + * shrink-to-zero invariant (ADR-3473 §8.3) is satisfied by reaching zero; + * the map is kept as an empty frozen structure so a future re-hold has a + * shape to land in, and {@link validateCodexSandboxHolds} keeps failing if + * the list ever grows a role that no longer exists in `agents/`. The + * #3897 security-review F1/F3 fail-closed pins are unchanged and + * map-independent: `suspicious` identities (non-ASCII after + * normalization) still pin `read-only`, and post-lift the sandbox derives + * from an artifact's own CONTENT, so the F1 identity-confusion attack no + * longer has a hold to ride. */ -export const CODEX_SANDBOX_HOLDS: Readonly> = Object.freeze({ - 'gsd-ai-researcher': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-code-fixer': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-code-reviewer': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-debug-session-manager': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-doc-classifier': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-doc-synthesizer': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-doc-verifier': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-doc-writer': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-dom-verifier': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-domain-researcher': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-eval-auditor': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-eval-planner': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-intel-updater': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-pattern-mapper': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-ui-auditor': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-ui-researcher': 'declares Write/Edit; pending Codex sandbox_mode enforcement decision', - 'gsd-nyquist-auditor': 'declares Write/Edit (YAML list-form tools:, surfaced by the list-form parse fix); pending Codex sandbox_mode enforcement decision', -}); +export const CODEX_SANDBOX_HOLDS: Readonly> = Object.freeze({}); // True iff a `tools:` frontmatter value declares Write or Edit as a whole // token (never a substring match, so a hypothetical "Edith"-named tool could diff --git a/tests/codex-config-agents.test.cjs b/tests/codex-config-agents.test.cjs index 9ac61a368..3126bb220 100644 --- a/tests/codex-config-agents.test.cjs +++ b/tests/codex-config-agents.test.cjs @@ -65,7 +65,7 @@ const { GSD_CODEX_MARKER, deriveCodexSandboxMode: _deriveCodexSandboxMode, // #3897 rung 3 (ADR-3473 §8.3, option 2 — HALT.md): anticipated new export - // holding the 17 explicit read-only pins for roles whose tool contract would + // which held the 17 explicit read-only pins until #4770 lifted them (now an empty frozen map) for roles whose tool contract would // otherwise derive workspace-write (16 measured by HALT.md + gsd-nyquist-auditor, // surfaced by the list-form parse fix). Does not exist on the current tree — // destructuring a non-existent key is `undefined`, not a throw, so requiring diff --git a/tests/codex-config-hooks.test.cjs b/tests/codex-config-hooks.test.cjs index ee337abc4..8fd0341a6 100644 --- a/tests/codex-config-hooks.test.cjs +++ b/tests/codex-config-hooks.test.cjs @@ -65,7 +65,7 @@ const { GSD_CODEX_MARKER: _GSD_CODEX_MARKER, deriveCodexSandboxMode: _deriveCodexSandboxMode, // #3897 rung 3 (ADR-3473 §8.3, option 2 — HALT.md): anticipated new export - // holding the 17 explicit read-only pins for roles whose tool contract would + // which held the 17 explicit read-only pins until #4770 lifted them (now an empty frozen map) for roles whose tool contract would // otherwise derive workspace-write (16 measured by HALT.md + gsd-nyquist-auditor, // surfaced by the list-form parse fix). Does not exist on the current tree — // destructuring a non-existent key is `undefined`, not a throw, so requiring diff --git a/tests/codex-config-install.test.cjs b/tests/codex-config-install.test.cjs index 0d4361bc3..1e04c41f5 100644 --- a/tests/codex-config-install.test.cjs +++ b/tests/codex-config-install.test.cjs @@ -65,7 +65,7 @@ const { GSD_CODEX_MARKER: _GSD_CODEX_MARKER, deriveCodexSandboxMode: _deriveCodexSandboxMode, // #3897 rung 3 (ADR-3473 §8.3, option 2 — HALT.md): anticipated new export - // holding the 17 explicit read-only pins for roles whose tool contract would + // which held the 17 explicit read-only pins until #4770 lifted them (now an empty frozen map) for roles whose tool contract would // otherwise derive workspace-write (16 measured by HALT.md + gsd-nyquist-auditor, // surfaced by the list-form parse fix). Does not exist on the current tree — // destructuring a non-existent key is `undefined`, not a throw, so requiring diff --git a/tests/codex-config.test.cjs b/tests/codex-config.test.cjs index ef148645f..a5e4432d5 100644 --- a/tests/codex-config.test.cjs +++ b/tests/codex-config.test.cjs @@ -1088,28 +1088,28 @@ describe('#3897 rung 3: sandbox_mode derivation and the hold list', () => { // codex-agent-toml.test.cjs's A14 round-trip pattern) does not let that hide. const EXPECTED_SANDBOX_BY_ROLE = { 'gsd-advisor-researcher': 'read-only', - 'gsd-ai-researcher': 'read-only', + 'gsd-ai-researcher': 'workspace-write', 'gsd-assumptions-analyzer': 'read-only', - 'gsd-code-fixer': 'read-only', - 'gsd-code-reviewer': 'read-only', + 'gsd-code-fixer': 'workspace-write', + 'gsd-code-reviewer': 'workspace-write', 'gsd-codebase-mapper': 'workspace-write', - 'gsd-debug-session-manager': 'read-only', + 'gsd-debug-session-manager': 'workspace-write', 'gsd-debugger': 'workspace-write', - 'gsd-doc-classifier': 'read-only', - 'gsd-doc-synthesizer': 'read-only', - 'gsd-doc-verifier': 'read-only', - 'gsd-doc-writer': 'read-only', - 'gsd-dom-verifier': 'read-only', - 'gsd-domain-researcher': 'read-only', - 'gsd-eval-auditor': 'read-only', - 'gsd-eval-planner': 'read-only', + 'gsd-doc-classifier': 'workspace-write', + 'gsd-doc-synthesizer': 'workspace-write', + 'gsd-doc-verifier': 'workspace-write', + 'gsd-doc-writer': 'workspace-write', + 'gsd-dom-verifier': 'workspace-write', + 'gsd-domain-researcher': 'workspace-write', + 'gsd-eval-auditor': 'workspace-write', + 'gsd-eval-planner': 'workspace-write', 'gsd-executor': 'workspace-write', 'gsd-framework-selector': 'read-only', 'gsd-integration-checker': 'read-only', - 'gsd-intel-updater': 'read-only', + 'gsd-intel-updater': 'workspace-write', 'gsd-mempalace-curator': 'read-only', - 'gsd-nyquist-auditor': 'read-only', - 'gsd-pattern-mapper': 'read-only', + 'gsd-nyquist-auditor': 'workspace-write', + 'gsd-pattern-mapper': 'workspace-write', 'gsd-phase-researcher': 'workspace-write', 'gsd-plan-checker': 'read-only', 'gsd-planner': 'workspace-write', @@ -1117,9 +1117,9 @@ describe('#3897 rung 3: sandbox_mode derivation and the hold list', () => { 'gsd-research-synthesizer': 'workspace-write', 'gsd-roadmapper': 'workspace-write', 'gsd-security-auditor': 'read-only', - 'gsd-ui-auditor': 'read-only', + 'gsd-ui-auditor': 'workspace-write', 'gsd-ui-checker': 'read-only', - 'gsd-ui-researcher': 'read-only', + 'gsd-ui-researcher': 'workspace-write', 'gsd-user-profiler': 'read-only', 'gsd-verifier': 'workspace-write', }; @@ -1177,24 +1177,32 @@ describe('#3897 rung 3: sandbox_mode derivation and the hold list', () => { }); } - test('T30 holdListMatchesTheMeasuredWideningSet: CODEX_SANDBOX_HOLDS is exactly the 17 measured widening roles, derived not hardcoded twice', () => { + test('T30 holdListShrunkToZero: CODEX_SANDBOX_HOLDS is empty and every measured widening role derives workspace-write (#4770)', () => { + // #4770 lift: the hold's recorded reopen condition (official OpenAI docs + // establishing sandbox_mode as enforced) is satisfied, so the list shrank + // to zero per its own ADR-3473 §8.3 shrink-only invariant. The formerly + // held roles are exactly measuredWideningRoles — this test now guards the + // LIFT: the map stays empty (no re-hold without a new recorded decision) + // and every one of those roles derives workspace-write from its own + // tools: contract. assert.equal( typeof CODEX_SANDBOX_HOLDS, 'object', - 'install.js must export CODEX_SANDBOX_HOLDS — the hold list does not exist yet', + 'install.js must export CODEX_SANDBOX_HOLDS — the (now empty) hold list shape is kept for a future re-hold', ); - assert.notEqual(CODEX_SANDBOX_HOLDS, null); assert.deepEqual( Object.keys(CODEX_SANDBOX_HOLDS).sort(), - measuredWideningRoles.sort(), - 'CODEX_SANDBOX_HOLDS must equal exactly the set of roles that declare Write/Edit but were never in the old map — no more, no fewer', + [], + 'CODEX_SANDBOX_HOLDS must be empty after the #4770 lift — a re-hold requires a new recorded decision', ); - // 16 measured by HALT.md against a single-line tools: reader + 1 - // (gsd-nyquist-auditor, YAML block-list tools: — the list-form parse fix) - // = 17. `measuredWideningRoles` is computed from realAgentToolsRaw, which - // now routes through the fixed extractToolsValue, so this count moves - // WITH the parser fix rather than needing a second hand-edit. - assert.equal(measuredWideningRoles.length, 17, 'sanity: 17 widening roles against the current agents/ tree, once list-form tools: parses correctly'); + assert.equal(measuredWideningRoles.length, 17, 'sanity: 17 widening roles against the current agents/ tree'); + for (const role of measuredWideningRoles) { + assert.equal( + EXPECTED_SANDBOX_BY_ROLE[role], + 'workspace-write', + '#4770: formerly-held role ' + role + ' must derive workspace-write from its own tool contract', + ); + } }); test('T21 mappedRolesDeriveToTheirFormerValue: every former CODEX_AGENT_SANDBOX entry (11) derives to the identical value from its real tool contract', () => { @@ -1219,26 +1227,23 @@ describe('#3897 rung 3: sandbox_mode derivation and the hold list', () => { assert.ok(toml.includes('sandbox_mode = "read-only"')); }); - test('T23 heldRoleStaysReadOnlyWithARecordedReason: one of the 17 held roles stays read-only via an explicit, reasoned hold (S3)', () => { + test('T23 formerlyHeldRoleNowDerivesWorkspaceWrite: the #4770 lift releases gsd-doc-writer to its own tool contract', () => { assert.equal(typeof CODEX_SANDBOX_HOLDS, 'object', 'CODEX_SANDBOX_HOLDS does not exist yet'); const role = 'gsd-doc-writer'; // declares Write+Edit; one of HALT.md's 16 assert.ok(declaresWriteOrEdit(realAgentToolsRaw(role)), 'sanity: this role must actually declare a writing tool'); - assert.ok(Object.prototype.hasOwnProperty.call(CODEX_SANDBOX_HOLDS, role), `${role} must be an explicit hold entry`); - const entry = CODEX_SANDBOX_HOLDS[role]; - const reason = typeof entry === 'string' ? entry : entry && entry.reason; - assert.equal(typeof reason, 'string', `the hold for ${role} must carry a recorded reason string, not a bare boolean pin`); - assert.ok(reason.length > 0); + assert.ok(!Object.prototype.hasOwnProperty.call(CODEX_SANDBOX_HOLDS, role), `${role} must no longer be a hold entry after the #4770 lift`); const content = fs.readFileSync(path.join(AGENTS_DIR, `${role}.md`), 'utf8'); const toml = generateCodexAgentToml(role, content); - assert.ok(toml.includes('sandbox_mode = "read-only"'), 'byte-identical output despite the hold, per N6'); + assert.ok(toml.includes('sandbox_mode = "workspace-write"'), 'the role must now derive workspace-write from its own contract (#4770)'); }); - test('T24 staleHoldFailsRatherThanBeingHonored: a hold whose role no longer derives broader must FAIL, naming the role (S4)', () => { + test('T24 staleHoldFailsRatherThanBeingHonored: a hold whose role no longer derives broader must FAIL, naming the role (S4) — empty after #4770', () => { assert.equal( typeof CODEX_SANDBOX_HOLDS, 'object', 'CODEX_SANDBOX_HOLDS does not exist yet, so there is nothing to validate for staleness', ); + assert.equal(Object.keys(CODEX_SANDBOX_HOLDS).length, 0, '#4770 lifted every hold; the sweep below stays as the re-growth guard'); // Every REAL hold entry, right now, must still derive workspace-write from // the tool contract. A hold for a role whose tools no longer declare // Write/Edit is exactly the staleness this row exists to catch; without @@ -1253,12 +1258,13 @@ describe('#3897 rung 3: sandbox_mode derivation and the hold list', () => { } }); - test('T25 holdForUnknownRoleFails: a hold naming a role that no longer exists in agents/ must FAIL (S5)', () => { + test('T25 holdForUnknownRoleFails: a hold naming a role that no longer exists in agents/ must FAIL (S5) — empty after #4770', () => { assert.equal( typeof CODEX_SANDBOX_HOLDS, 'object', 'CODEX_SANDBOX_HOLDS does not exist yet, so there is nothing to validate for an unknown role', ); + assert.equal(Object.keys(CODEX_SANDBOX_HOLDS).length, 0, '#4770 lifted every hold; the sweep below stays as the re-growth guard'); for (const role of Object.keys(CODEX_SANDBOX_HOLDS)) { assert.ok( fs.existsSync(path.join(AGENTS_DIR, `${role}.md`)), @@ -1314,12 +1320,12 @@ description: Declares no tools frontmatter key at all // `installCodexConfig`'s per-file loop independently of the frontmatter // `name:` used for the TOML body, with a case-insensitive lookup as a second // line of defense. - test('heldRoleCannotEscapeItsHoldByRenamingFrontmatter_3897: editing or recasing a held role\'s frontmatter name: must not change its sandbox_mode from read-only', () => { + test('heldRoleCannotEscapeItsHoldByRenamingFrontmatter_3897: name edits or recasing never change the content-derived sandbox (#4770: hold lifted, property kept)', () => { const { installCodexConfig } = require('../bin/install.js'); - const heldRole = 'gsd-doc-writer'; // one of the 17 CODEX_SANDBOX_HOLDS entries + const heldRole = 'gsd-doc-writer'; // one of the 17 formerly-held roles assert.ok( - Object.prototype.hasOwnProperty.call(CODEX_SANDBOX_HOLDS, heldRole), - `sanity: ${heldRole} must be a real CODEX_SANDBOX_HOLDS entry`, + !Object.prototype.hasOwnProperty.call(CODEX_SANDBOX_HOLDS, heldRole), + `sanity: ${heldRole} was lifted from CODEX_SANDBOX_HOLDS by #4770`, ); const variants = [ @@ -1351,11 +1357,15 @@ description: Declares no tools frontmatter key at all const toml = fs.readFileSync(emittedTomlPath, 'utf8'); const sandboxLine = toml.match(/^sandbox_mode = "([^"]{0,50})"$/m); assert.ok(sandboxLine, `${label}: emitted .toml must contain a sandbox_mode line`); + // #4770: with the hold lifted, the sandbox is derived from the + // content's own tools contract — gsd-doc-writer declares Write/Edit, + // so every name variant emits workspace-write. The F1 property that + // survives is that the derivation follows the CONTENT, never the + // self-declared name. assert.equal( sandboxLine[1], - 'read-only', - `${label}: a held role's sandbox_mode must stay read-only even when its frontmatter name: diverges ` + - `from its own filename — the hold is keyed off the file, not a self-declared field. Got: ${sandboxLine[1]}`, + 'workspace-write', + `${label}: the emitted .toml must carry the content-derived workspace-write — name edits/recasing change nothing post-#4770. Got: ${sandboxLine[1]}`, ); } finally { cleanup(tmpAgentsSrc); @@ -1442,8 +1452,8 @@ describe('#3897 security review: F1 filename/name identity confusion, F3 confusa const toml = fs.readFileSync(emittedPath, 'utf8'); assert.equal( sandboxModeOfToml(toml), - 'read-only', - 'F1(a): a held role\'s own emitted .toml must stay read-only even when reached via a renamed source file whose filename stem is unheld', + 'workspace-write', + 'F1(a) post-#4770: the sandbox derives from the content\'s own tools contract, so a renamed source file changes nothing — gsd-doc-writer declares Write/Edit and emits workspace-write', ); } finally { cleanup(src); @@ -1483,8 +1493,8 @@ describe('#3897 security review: F1 filename/name identity confusion, F3 confusa const toml = fs.readFileSync(emittedPath, 'utf8'); assert.equal( sandboxModeOfToml(toml), - 'read-only', - 'F1(b): a sibling file whose frontmatter name: collides with a held role must not clobber that role\'s emitted .toml with workspace-write', + 'workspace-write', + 'F1(b) post-#4770: the last-writer artifact carries ITS OWN content-derived sandbox (the sibling declared Write/Edit) — with no holds left, a name collision cannot escalate any role beyond what its own content derives', ); } finally { cleanup(src); @@ -1492,36 +1502,63 @@ describe('#3897 security review: F1 filename/name identity confusion, F3 confusa } }); - const F3_CONFUSABLE_VECTORS = [ - ['Turkish dotted I (İ)', 'gsd-doc-wrİter'], - ['Turkish dotless i (ı)', 'gsd-doc-wrıter'], + // Vectors that NFKC-fold to pure ASCII (fullwidth g, whitespace, dots, + // path prefixes): post-#4770 they normalize onto a real roster role whose + // content declares Write/Edit, and the content-derived answer is + // workspace-write — the identity no longer gates. + const F3_CONTENT_DERIVED_VECTORS = [ ['fullwidth leading g (ｇ)', 'ｇsd-doc-writer'], - ['NFD combining acute on r (writeŕ)', 'gsd-doc-writeŕ'], ['trailing ASCII space', 'gsd-doc-writer '], - ['trailing NBSP', 'gsd-doc-writer '], + ['trailing NBSP', 'gsd-doc-writer '], ['trailing dot', 'gsd-doc-writer.'], ['trailing newline', 'gsd-doc-writer\n'], ['trailing carriage return', 'gsd-doc-writer\r'], ['relative-path prefix ./', './gsd-doc-writer'], ['path traversal ../agents/', '../agents/gsd-doc-writer'], ]; - - for (const [label, vector] of F3_CONFUSABLE_VECTORS) { - test(`F3 confusable/whitespace/path vector — ${label} — derives read-only`, () => { + for (const [label, vector] of F3_CONTENT_DERIVED_VECTORS) { + test(`F3 ascii-folding vector — ${label} — derives from content (#4770)`, () => { const mode = deriveCodexSandboxModeLocal(vector, 'Read, Write, Edit'); assert.equal( mode, - 'read-only', - `F3: identity ${JSON.stringify(vector)} (${label}) must derive read-only — either it normalizes onto the real held key, or it is unrecognizable and must fail closed`, + 'workspace-write', + `F3 post-#4770: identity ${JSON.stringify(vector)} (${label}) folds to ASCII and normalizes onto a real roster role whose content declares Write/Edit — content-derived workspace-write`, ); }); } - test('F3: isSandboxHeld flags each confusable vector as held or suspicious (never silently neither)', () => { - for (const [label, vector] of F3_CONFUSABLE_VECTORS) { - const { held, suspicious } = isSandboxHeld(vector); - assert.ok(held || suspicious, `${label} (${JSON.stringify(vector)}) must be held or suspicious`); - } + // Vectors that stay non-ASCII after NFKC (Turkish İ/ı, combining acute): + // the F3 fail-closed pin is map-independent and still applies — a + // non-ASCII-after-normalization identity is never a legitimate shipped + // role and is pinned read-only regardless of its content's tools. + const F3_STILL_SUSPICIOUS_VECTORS = [ + ['Turkish dotted I (İ)', 'gsd-doc-wrİter'], + ['Turkish dotless i (ı)', 'gsd-doc-wrıter'], + ['NFD combining acute on r (writeŕ)', 'gsd-doc-writeŕ'], + ]; + for (const [label, vector] of F3_STILL_SUSPICIOUS_VECTORS) { + test(`F3 non-folding vector — ${label} — stays fail-closed read-only (#4770)`, () => { + const mode = deriveCodexSandboxModeLocal(vector, 'Read, Write, Edit'); + assert.equal( + mode, + 'read-only', + `F3: identity ${JSON.stringify(vector)} (${label}) is still non-ASCII after NFKC — suspicious and fail-closed read-only regardless of content`, + ); + }); + } + + // The F3 core that survives the #4770 lift: an identity still non-ASCII + // after NFKC normalization is not a legitimate shipped role and is pinned + // fail-closed read-only regardless of its content's tool contract. + test('F3 core: an identity still non-ASCII after normalization derives read-only regardless of content (#4770)', () => { + const mode = deriveCodexSandboxModeLocal('gsd-dос-writer', 'Read, Write, Edit'); + assert.equal(mode, 'read-only', 'a suspicious (non-ASCII after normalization) identity must stay fail-closed read-only'); + }); + + test('F3: isSandboxHeld flags a non-ASCII-after-normalization identity as suspicious (#4770: held is vacuously false over the empty map)', () => { + const { held, suspicious } = isSandboxHeld('gsd-dос-writer'); + assert.equal(held, false, 'the hold map is empty post-#4770 — nothing is held'); + assert.equal(suspicious, true, 'a Cyrillic-lookalike identity must still be flagged suspicious (fail-closed F3 core)'); }); test('F5: "All tools except Write, Edit" derives read-only (negation excludes Write/Edit)', () => { @@ -1672,7 +1709,7 @@ describe('#3897 security review: F1 filename/name identity confusion, F3 confusa assert.equal(extractToolsValueLocal(content), 'Read, Write'); }); - test('roster truth: gsd-nyquist-auditor DERIVES workspace-write from its real tool contract AND is HELD, so its emitted .toml stays read-only', () => { + test('roster truth: gsd-nyquist-auditor derives workspace-write from its real tool contract, and the #4770 lift released it to that derivation', () => { const content = fs.readFileSync(path.join(AGENTS_DIR, 'gsd-nyquist-auditor.md'), 'utf8'); const toolsRaw = extractToolsValueLocal(content); assert.ok( @@ -1681,24 +1718,23 @@ describe('#3897 security review: F1 filename/name identity confusion, F3 confusa ); // Derivation WITHOUT the hold (an identity guaranteed never held/suspicious, // same probe idiom as the rung-3 describe block's PARITY_PROBE_IDENTITY) - // must show the role genuinely derives workspace-write from its contract — - // this is what proves derive-and-hold is doing real work, not that the - // parser happens to agree with the pin by accident. + // must show the role genuinely derives workspace-write from its contract. assert.equal( deriveCodexSandboxModeLocal('zzz-nyquist-unheld-probe-never-a-real-role', toolsRaw), 'workspace-write', 'gsd-nyquist-auditor must genuinely derive workspace-write from its tool contract once list-form tools: parses correctly', ); - // The REAL identity IS held, so the actual emitted artifact stays read-only. + // #4770 lifted the hold, so the REAL identity now derives workspace-write + // too — its emitted .toml matches its own content's tool contract. assert.ok( - Object.prototype.hasOwnProperty.call(CODEX_SANDBOX_HOLDS, 'gsd-nyquist-auditor'), - 'gsd-nyquist-auditor must be an explicit CODEX_SANDBOX_HOLDS entry', + !Object.prototype.hasOwnProperty.call(CODEX_SANDBOX_HOLDS, 'gsd-nyquist-auditor'), + 'gsd-nyquist-auditor must no longer be a CODEX_SANDBOX_HOLDS entry after the #4770 lift', ); const { generateCodexAgentToml: generateCodexAgentTomlLocal } = require('../bin/install.js'); const toml = generateCodexAgentTomlLocal('gsd-nyquist-auditor', content); assert.ok( - toml.includes('sandbox_mode = "read-only"'), - 'gsd-nyquist-auditor\'s emitted .toml must stay read-only (held), even though it now derives workspace-write', + toml.includes('sandbox_mode = "workspace-write"'), + 'gsd-nyquist-auditor\'s emitted .toml now carries the content-derived workspace-write (#4770)', ); }); }); diff --git a/tests/helpers/emitted-provenance.cjs b/tests/helpers/emitted-provenance.cjs index 513692955..14fe1fd18 100644 --- a/tests/helpers/emitted-provenance.cjs +++ b/tests/helpers/emitted-provenance.cjs @@ -158,6 +158,11 @@ const AGENT_TRANSFORM_SRCS = [ 'src/runtime-artifact-conversion.cts', 'src/install-effort-resolver.cts', 'src/model-catalog.cts', + // #4770: the Codex .toml family's sandbox_mode is derived through + // src/codex-agent-toml.cts (deriveCodexSandboxMode — the single owner of the + // derivation), so a change there moves every emitted agents/*.toml without + // touching any agents/*.md source. + 'src/codex-agent-toml.cts', ]; // #3738: antigravity's global skills pass through the antigravity converter