WT-4-codex-only: swap Claude→Codex baseline + ship 5 per-workload wrappers#84
Merged
Conversation
Consolidates original 8-worktree p4 plan into one branch:
- New single_machine_codex/ baseline binary (parallels single_machine/)
- Enum swap in tools/eval/fulltable/{matrix,schema,plan.py,tests,snapshot}
- New tools/eval/experiments/ with 5 per-workload wrapper scripts
- run.sh --workload / --results-root extensions
- LOOM_FULLTABLE_WRAPPER_SHIM test seam (dual-guarded)
Security highlights closed under codex review (session
019f3d15-c371-7b23-b053-e34c3bd8e676):
- Pinned codex exec argv (workspace-write sandbox, --ephemeral,
--skip-git-repo-check, --json, -C ws.Root, -- prompt separator)
- AST-only anti-drift (codexPrompts vs claudePrompts) — no go:embed
- Route (b) [env_key] DETECTED but DECLARED UNSUPPORTED; route-b-only
configs exit 2. Reason: existing LOOM_ passthrough forwards to
both agent and oracle, violates T1 no-implicit-forwarding
- Codex stderr scrub test uses nonzero exit path (leak path)
- --workload / --filter-workload rejected in wrapper AND run.sh
duplicate detection (both belts and suspenders)
- Sample cap enforced at run.sh (not planner) — --workload cannot bypass
- Snapshot no-secrets test (sk-, ghp_, Bearer, path patterns)
- results-root allowlist (/root/.codex, /, /tmp, /root, /root rejected)
Deferred to follow-up worktrees (documented in handoff):
- Route-b agent-only forwarding path
- run.sh --reps / --config-subset flags
- paper_writing 24 md replacements (separate small PR)
Phase A (1-5): single_machine_codex/ baseline binary + AST anti-drift + LOCKED codex exec argv + stderr scrub via secretscrub.Sanitize + run.sh. Phase B (6-9): matrix.yaml/schema/plan.py enum swap + snapshot regen + snapshot-no-secrets + snapshot-baseline-path-exact + harness row_test + matrix_test + README. Phase C (10-11): plan.py filter_workload / WORKLOADS / UnknownWorkloadError / --filter-workload / --list-workloads / --results-root with allowlist + run.sh --workload (at-most-once) / --results-root / LOOM_FULLTABLE_WRAPPER_SHIM dual-guard + sample-cap-with-workload + duplicate-workload rejection tests. Phase D (12-19): _common.sh (codex_bin_present + codex_config_readable + codex_config_has_route_a/b [python3 tomllib, never logs values] + require_writable_tmp/fixture/windows_host + die/warn_and_exit_zero) + 10-fixture credential-bound preflight (route-b unsupported) + 5 wrappers (cross_device_code_mod, missing_parser_converter, remote_data_processing, windows_only_artifact with --skip-if-not-windows, credential_bound_model with route-a required + route-b detected-but-rejected) + wrapper-forwards tests with --workload / --filter-workload rejection + Windows-guard test table (MSYS/CYGWIN/MINGW/*NT accept, Linux/Darwin/missing reject). Phase E (20-21): experiments/README + repo-wide no-Claude-baseline grep guard + handoff.md (route-b agent-only forwarding follow-up + --reps / --config-subset deferral + paper_writing 24-md swap PR + record of consciously-untouched files + P2/P3 disposition). TDD throughout; every task RED → GREEN → commit. Every Global Constraint maps to at least one test.
Fixes applied over 9 rounds of adversarial codex review: R1 (P0/P1×7): - P0 --results-root not scoped through run.sh → threaded via override_out_root through enumerator + shell mkdir/rm/write sites - P0 filter-first/sample-second wrong → moved filter INTO enumerate_matrix_argvs before sample_n truncation - P1 TDD claim false → softened for Task 1 scaffold + Task 5 mirror + Task 20 README - P1 pinned argv shape-only → reflect.DeepEqual against full slice - P1 --workload acceptance under-tested → non-shim 12-row test for prefix AND non-prefix workloads - P1 credential parse errors ignored → check rc==2 before route logic, die with parse-error - P1 results-root validation incomplete → git-top + symlink + non-empty-unless-resume - P1 env allowlist unchanged has no guard → Task 19.5 diff test against baseline - P1 success-path scrub can pass vacuously → require [REDACTED] sentinel R2 (P0/P1×3): - P0 --results-root not through planner → override_out_root also bypasses _smoke_out_root - P1 pinned argv self-referential → prompt compared against codexPrompts constant, not argv - P1 resume/E4 skip missing tests → real dispatch-shim tests - P1 scoping test not wired → added to param list + git-add R3 (P0/P1×3): - P0 /root/.codex/subdirs allowed → reject codex_dir in p.parents; shell case adds /root/.codex/* - P1 credential wrapper set -e capture broken → if/then/else form - P1 dispatch-shim tests didn't reach path → remove --dry-run, add WORKTREE_ROOT clean-repo - P1 run.sh own writes not tested → test_results_root_scoping_dispatch.sh R4 (P0×1): - P0 wrappers eager mkdir dirties tree → wrappers COMPUTE path only, don't mkdir; Task 13.5 .gitignore experiments/ R5-R6 (P0×1): - P0 symlink prefix + missing child escapes → realpath -m OR python3 Path.resolve(strict=False); refuse if both fail R7 (P0×2): - P0 python fallback argv bug (-- put path at wrong index) → no -- separator - P0 tests mutate real ~/.codex → fake HOME under mktemp - P2 /tmp files in shell test → all under tmpdir R8 (P1×1): - P1 vacuous symlink tests (base under /tmp rejected by raw guard) → allowlisted base under _symlink_test/ + SETUP FAIL guards R9: plan is ready; no P0/P1 remaining. Final structure: 22 tasks (1-13, 13.5 gitignore, 14-18 wrappers, 19 windows guard, 19.5 env allowlist guard, 19.6 P2 fixes, 20 README+repo grep, 21 handoff). 6 shell tests, 20+ pytest cases, byte-exact argv, filter-before-sample enumerator, results-root allowlist with symlink resolution, route-b declared unsupported (leaks-to-oracle problem), credential-parse-error handling, gitignored experiments dir, wrapper preflight non-dirtying. Codex session for full history: 019f3d15-c371-7b23-b053-e34c3bd8e676
… + forward-flag test
…quire tools + drop dead import
…dex in matrix + schema
README keeps single_machine_claude_code as a REFERENCE row (single_machine/ dir untouched); active baseline is single_machine_codex.
…s-root with allowlist + override_out_root through enumerator
…PPER_SHIM + 6 shell tests (SHIM + DISPATCH_SHIM + resume/E4 + symlink escape + duplicate + sample cap)
…eject empty --workload + resume assert-11 + drop dup WORKLOADS + mktemp for case3 out
…g parsers never log values)
…Task 18 flips to green)
…eal-preflight tests
… guard + --skip-if-not-windows
…ute-b unsupported)
…OSSIBLE scan + equals-form injection tests + safe uname in windows wrapper
…ing PR, P2/P3 disposition, 4-phase code-review outcomes
…agent/ + README security wording precision
…lish Fresh adversarial review of PR #84 by an isolated Claude Code subagent surfaced 1 P0 + 3 P1 + 4 P2 + 4 P3. ## P0 — env allowlist anti-drift test was silently vacuous test_env_allowlists_unchanged in tools/eval/fulltable/tests/test_env_allowlist_unchanged.py had a regex that only matched map[K]V{...} literals; the two slice literals alwaysAllowedEnvKeys and alwaysAllowedIfSetEnvKeys silently extracted to <not-found>, so adding OPENAI_API_KEY to the always-forwarded list would still make current == base == <not-found> and PASS. This defeated the entire Global Constraint that env.go is frozen in this PR for the two most dangerous lists. Fix: replace the regex with a bracket-matching extractor that handles BOTH slice literals AND map literals, PLUS added: - test_extractor_finds_all_three_lists_non_empty (guard against future extractor regressions returning <not-found>) - test_extractor_catches_tampered_fixture (empirically proves the extractor detects an added key in a synthetic tampered fixture, so a broken extractor can't silently pass the main comparison) Verified: tampering the real env.go with a spurious OPENAI_API_KEY now correctly fails the main test. ## P1 — run.sh git-top guard skipped for non-existent --results-root fails when path doesn't yet exist (common for fresh runs), so git_top stays empty. Mirrored plan.py: walk up parents until an existing dir is found, then git rev-parse from there. ## P1 — --results-root '' silently accepted Empty --results-root now rejected via a results_root_count guard, symmetric to the --workload '' rejection. ## P1 — Bearer regex only matched \s+ separator Broadened bearerRE to '(?i)Bearer[\s:=]+...' to cover Bearer=<tok> and Bearer:<tok> forms (curl-style dumps + HTTP header dumps). Added test cases for all three separator styles. ## P2 — $HOME unset under set -u aborts before security refusal Bind home="${HOME:-/nonexistent-home}" once in run.sh and _common.sh, use $home in case patterns. ## P2 — pinned-argv test guards -C position before dereferencing wsRoot ## P2 — snapshot gh-token regex '[opsruA-Z]' → '[opsruc]' (actual token grammar) ## P3 — 4 wrapper header comments corrected + stale WORKLOADS drop-comment removed Full sweep: 5 Go packages green + 161 pytest (was 159 + 2 new self-tests for the env allowlist extractor).
…ents (comment-} bypass) + shell coverage for --results-root ''
Round-2 fresh review found a residual P1 on the round-1 P0 fix:
_find_var_body walked braces on RAW source, so a tampered Go file
like
var alwaysAllowedEnvKeys = []string{
"PATH",
// }
"EVIL_KEY",
}
compiles fine (the } is inside a // comment) but the walker treated
the comment } as the real close, silently missing EVIL_KEY. Same
bypass with /* } */.
Fix: added _strip_go_comments() pre-pass that blanks out //…\n
and /*…*/ spans (preserving newlines to keep offsets stable) while
correctly honoring "…" and `…` string literals. _find_var_body
now walks the scrubbed source.
Added 3 regression tests:
- test_extractor_catches_tampered_with_brace_in_line_comment
- test_extractor_catches_tampered_with_brace_in_block_comment
- test_extractor_honors_brace_inside_string_literal (belt for the
string-literal case that was already correct)
Verified end-to-end: tampering real env.go with the // } +
EVIL_KEY shape now correctly FAILS the main comparison test.
Also closed the P2 gap on --results-root '' shell coverage: added
two cases (space form + equals form) to test_results_root_scoping.sh
asserting exit 2 with the correct rejection message.
Sweep: 164 pytest + 5 Go packages green (was 161 + 3 new extractor
tests).
…deleted The r2 commit (7864b51) unintentionally deleted 6 Phase-3 smoke fixture files under tests/eval/results/smoke/runs/ because the shell tests touched-and-cleaned them during the local test run and the sweep of 'git add -A' picked up the deletion. Restore them from e246b26 (r1) so the smoke-scaffold evidence stays committed as intended by Phase 3 (spec §7 (j) — the smoke README references these as the fixture evidence). No test or code change — just a fixture restore.
…aw strings
Round-3 fresh review found a residual P1 exploitable bypass in the
r2 fix:
Go raw strings `…` can contain `}`, e.g.
var alwaysAllowedEnvKeys = []string{
"PATH",
`raw with } brace`,
"EVIL_KEY",
}
compiles cleanly (verified: 'go run' outputs [PATH raw with } brace
EVIL_KEY]), so the runtime allowlist gains EVIL_KEY. The r2 brace
walker only tracked '"…"' state, so the `` `}` `` inside the
raw string fake-closed the walker — attacker who also removes an
existing key can hold extraction length constant and slip a real
allowlist change past the comparison.
Two fixes in _find_var_body + _extract_string_literals:
1. Added in_raw state to _find_var_body's brace walker: when a
`````` is seen outside a "…" string, treat everything until
the next `````` as raw-string content — brace tracking suspended.
Raw strings have NO escape sequences; only the terminating
backtick ends them.
2. Extended _extract_string_literals to enumerate raw strings too,
so a tamper using ``EVIL_KEY`` as an allowlist entry is caught.
Added regression test:
- test_extractor_catches_tampered_with_brace_in_raw_string
(asserts EVIL_KEY IS detected + the raw-string entry itself
is extracted alongside "…" entries)
End-to-end verified: tampering real env.go with the raw-string-}
+ EVIL_KEY shape now correctly FAILS the main comparison test.
Sweep: 165 pytest + 5 Go packages green (was 164 + 1 new).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
WT-4-codex-only — swap Claude Code baseline for OpenAI Codex CLI in Phase 3 harness + ship 5 per-workload wrappers
Consolidates the original Phase 4 8-worktree plan (
docs/final/prompts/p4-experiment-plan.md) into ONE branch → ONE PR after the user chose 2-worktree consolidation on 2026-07-07. Companion PR inpaper-npc: paper/v4/codex-only.What this PR does
tests/eval/baselines/single_machine_codex/— mirrorssingle_machine/(Claude) with the OpenAI Codex CLI as the coding-agent shell-out. Pinned argv (LOCKED, byte-exact test):tools/eval/fulltable/{matrix.yaml, matrix.schema.json, lib/plan.py, tests/*.py, dry_run_snapshot.txt}+tests/eval/baselines/harness/row_test.go+tests/eval/baselines/matrix_test.go+tests/eval/baselines/README.mdrun.shextensions:--workload <id>(at-most-once, exact-string allowlist derived fromplan.py --list-workloads),--results-root <abs-dir>(allowlisted + canonicalized viarealpath -m/ python3 fallback, rejects/tmp,$HOME,$HOME/.codex/**, git-top, symlink-prefix escapes),LOOM_FULLTABLE_WRAPPER_SHIM=1dual-guard test seamplan.pyextensions:filter_workload()+WORKLOADSconst +UnknownWorkloadError; CLI--filter-workload/--list-workloads/--results-root;override_out_rootparameter throughenumerate_matrix_argvs/enumerate_e4_argvsbypasses_smoke_out_rootguardtools/eval/experiments/:cross_device_code_mod.sh— codex bin + config preflightmissing_parser_converter.sh— sameremote_data_processing.sh— + writable-tmp + fixture checkwindows_only_artifact.sh— +require_windows_hostguard with--skip-if-not-windowsopt-outcredential_bound_model.sh— + route (a) required, route (b) unsupported here_common.sh— helpers (codex_bin_present,codex_config_readable,codex_config_has_route_a/b,require_writable_tmp,require_fixture,require_windows_host,die,warn_and_exit_zero) that NEVER invokecodexand NEVER print config values / env-var names.gitignoreentry fortests/eval/results/experiments/Security highlights (called out because of long adversarial review)
codex execargv — byte-exactreflect.DeepEqualtest against pinned form; prompt sourced fromcodexPromptsconstant, not self-referentialsecretscrub.Sanitize+ localbearerRE(Bearer pattern is not in shared secretscrub) on BOTH nonzero-exit AND success-but-missing-output leak paths; test with fake codex exiting 42 and emittingsk-*+ghp_*+Bearer *tokensTestCodexPromptsMatchClaudePromptsparses../single_machine/workloads.goat test time to keepcodexPromptsin lockstep withclaudePrompts(nogo:embedgolden that could go stale)codex— onlycommand -v codex. Test verifies via a fakecodexthat printsIMPOSSIBLE: preflight must NEVER exec it+ exit 99, plus an "IMPOSSIBLE sentinel scan" that fails if any wrapper's stderr contains itpresent/absent/errorfrompython3 tomllib; never token values, never env-var names. 10-fixture matrix (including malformed TOML, duplicate tables, commented tokens, wrong-table tokens) verifies zero-leak--workloadallowlist — exact-string[[ "$workload" == "$allowed" ]]iteration (NOTgrep -qxregex — that let--workload '.*'through in an earlier round); empty--workload ''explicitly rejected--workloadat most once — duplicate rejected BEFORE preflight, BEFORE any dispatch--workload/--filter-workloadin both space AND equals formsrealpath -m(or python3Path.resolve(strict=False)) to resolve symlink-prefixes with missing children; refuses if BOTH canonicalizers fail (safer than raw-path fallthrough)test_env_allowlist_unchanged.pygit-diffsharness/env.goallowlists againstorigin/paper/v3-integration; a change fails the testdry_run_snapshot.txtscanned forsk-/ghp_/Bearer/refresh_token//root//$USER//home/*/patternstest_no_claude_baseline_repo.pysearches WHOLEmulti-agent/tree (not just tests/tools), whitelists onlysingle_machine/dir + baselines/README + test's own sourceTest counts
cloud_sandbox,harness,manual_ssh,single_machine(untouched),single_machine_codex(new; 9 tests including LOCKED argv + Bearer scrub on both leak paths + AST anti-drift + build belt + forward-flag)_common.shhelper cases, 10 credential-fixture cases, 6run.shshell-integration tests, 3 allowlist tests, symlink-escape tests, windows-guard test table, repo-wide grep, env-allowlist-unchanged, snapshot-no-secrets, snapshot-baseline-path-exactDeferred (documented in
docs/specs/wt4-codex-only.handoff.md)env_key] agent-only credential forwarding — needs newalwaysAllowedIfSetEnvKeysAgentOnlymap inharness/env.goor--forward-modelserver-env-keyflag. This PR detects route-b for operator visibility but rejects route-b-only configs (using LOOM_-prefix would leak the secret to the oracle via existing LOOM_ passthrough)run.sh --reps/--config-subset— needs converging with existing--sample/--resume; deferred to real-run planning worktreeReview process
019f3d15-c371-7b23-b053-e34c3bd8e676(all reviews shared context)What is NOT touched (recorded in handoff)
internal/driver/slave_tools.gotool names (get/update_slave_claude_permissions— stable API surface, backend-agnostic)compose-test/entrypoint-driver.shdefaultclaude(dev-only)mock-model/mock-claude-opus-4-8/(referential label)tests/eval/baselines/single_machine/(kept as reference; removed from fulltable matrix only)Cross-repo pin
paper-npc PR:
paper/v4/codex-only— swaps 10 occurrences ofsingle_machine_claude_codeinpaper_outputs/{motivation,evaluation}_v3.md. That PR should reference this PR's merge sha.🤖 Generated with Claude Code