P3.35: correct OS-process isolation claims to module/data-level independence - #74
Merged
Merged
Conversation
…e shipped runner does not implement redteam/campaign.py::run_campaign calls Orchestrator/RedTeam/Judge/Documentation as in-process Python objects inside one for loop -- no multiprocessing, subprocess, Popen, or fork anywhere in redteam/ or tools/. docs/ARCHITECTURE.md and docs/ATO_EVIDENCE_PACKET.md nonetheless assert each role "runs as its own OS process with its own local model instance," and use that property to differentiate the platform from Garak. This guard fails on current main (quoted in issue #73 / PR) and will pass once the docs are corrected to claim module/data-level independence instead.
… independence (issue #73) The shipped runner never implemented the OS-process isolation docs/ARCHITECTURE.md and docs/ATO_EVIDENCE_PACKET.md claimed: redteam/campaign.py::run_campaign calls Orchestrator/RedTeam/Judge/ Documentation as in-process Python objects in one for loop, and the Garak comparison (ARCHITECTURE.md:229) faulted Garak for exactly what this platform's own runner does. Orchestrator ruling (option 2): correct the docs, do not implement process isolation as part of this finish-out task. - ARCHITECTURE.md: prose summary, interaction diagram labels, the six required components, the fully-local model strategy section, the Garak/framework-choice comparisons, and the P3.4-prototype relationship section all now describe module- and data-level independence (separate modules, no shared imports, typed-only inputs) as the *implemented* property, and OS-process isolation with its own model instance per role as a stated design goal not yet implemented. - ATO_EVIDENCE_PACKET.md: data-flow diagram labels, the trust-zone boundary paragraph, the runtime-inventory table, and the network- egress prose corrected the same way, plus: only the Red Team Agent is model-backed (Judge/Orchestrator/Documentation are deterministic by default, no model call) -- corrected everywhere "four AI roles run locally" implied all four call a model. - DEMO_SCRIPT.md: fixed a second, related stale claim caught in review -- Beat 2 described the Judge's independence guard as scanning only `redteam.agents`/`redteam.harness`, and as a process boundary. - tests/redteam/test_judge_agent.py: widened the AST import guard's forbidden-prefix set to include `redteam.observability` (its action log carries full Red Team attempt payloads per campaign.py; judge.py does not import anything under it today, confirmed before widening) and rewrote the test's docstring to state precisely what the AST scan does NOT catch (alias imports, importlib, dynamic strings, transitive imports via a module judge.py itself imports). - redteam/agents/judge.py: module docstring corrected to match -- "separate module/context by construction", not "separate process". - tests/test_doc_consistency.py: new test_no_committed_doc_claims_os_process_isolation_is_implemented, scoped to docs/ (planning/'s verbatim historical kickoff prompt is a requirement, not a claim about shipped behaviour, and is excluded on that basis). - Updated the two "N passed[, M skipped]" test-count claims in ATO_EVIDENCE_PACKET.md and DEMO_SCRIPT.md for the one new test (tests/test_doc_test_counts.py enforces these). Leaves open for the owner: whether to actually implement OS-process isolation, or formally adopt module-level independence as the intended architecture. This PR only makes the docs match shipped behaviour.
… packet; widen test-count guard FIX 1 (blocks merge): docs/ATO_EVIDENCE_PACKET.md:398,406,617 still said 339/232 while :408 already said 340 passed -- re-measured live (pytest tests/ -q with the sibling checkout: 340 passed; with the sibling .git temporarily moved aside to reproduce the CI shape: 234 passed, 106 skipped) and corrected every stale number to match. tests/test_doc_test_counts.py's existing regexes require the literal token "passed" adjacent to the number, so "339-test suite", "339 tests", and "move it to 339 with" all evaded it. Added three narrowly-scoped patterns tied to the exact current phrasings (section heading, "move it to N with the sibling checkout present", "N tests with the sibling checkout") rather than any bare "N-test"/"N tests" shape -- a broad version would also flag the legitimate historical mention "the 171-test count that PR reported" (PR #35's own count at the time, not a current-suite claim); verified the new patterns match the three drifted strings and do not match the historical one. Refs #73
…RCHITECTURE.md Sec6 Sec6's drift-correction sentence said correction means to "re-anchor the Judge's prompt/criteria against the gold set, or roll back the Judge model version" -- contradicting Sec4 (the Judge is deterministic rule-based by default, no model call, no prompt). Reworded to match judge.py's reanchor() docstring wording: re-anchor the Judge's detect/scoring criteria, or roll back the Judge/scorer version. Re-read Sec6 in full; no other model-driven-Judge implication found (the gold-label clarification already correctly frames verdicts as detect()'s deterministic output, not a model judgement). Refs #73
…s; scope prose; distinguish env failure
FIX 3: _PROCESS_ISOLATION_CLAIM_RE missed the exact strings this PR
deleted ("isolated process/context", "separate isolated processes/
contexts", "process/context isolation", "own isolated process and
context") and plausible future regressions ("its own process", "never
share a process", "own separate model instance"). Added alternatives for
isolated process(es), process\s*/\s*context, own process, separate model
instance(s), and never share a process. Verified all seven strings from
the cold review now match.
FIX 4: _GOAL_QUALIFIER_RE missed "design target", which PR #72's release
notes correctly use ("sets a further goal, per-role OS-process isolation,
as the design target; as shipped, run_campaign wires all six components
... into one Python process"). Added design target/objective, stated
goal, not implemented, as shipped. Verified by running this guard's exact
matching logic against origin/docs/p3-29-release-notes:docs/
RELEASE_NOTES_v3.0.0.md: 2 process-isolation-claim matches, 0 violations.
ALSO: scoped the "no process, thread, or subprocess boundary" prose to
redteam/ and tools/ (evals/runner.py legitimately uses subprocess.run for
docker exec against the target -- unrelated to inter-agent isolation).
Split the git-grep-failure assertion so a checkout without .git fails
with an "environment cannot be checked" message instead of a message that
blames the docs for something it never claimed.
Refs #73
…scan; drop false alias-import understatement FIX 5: the AST import scan in tests/redteam/test_judge_agent.py (test_independence_module_imports_no_red_team_or_sibling_agent_internals) silently passed the most idiomatic bypass for a module inside redteam/agents/ -- relative imports: `from .red_team import RedTeamAgent`, `from . import red_team` (node.module is None), and `from ..observability.action_log import ActionLog`. judge.py currently has no relative imports (checked before adding this), so resolving them is safe. Added `_resolve_relative`, which maps any ast.ImportFrom with level > 0 to the fully-qualified module it refers to (given judge.py's own package, redteam.agents) before the forbidden-prefix check -- verified all three bypass strings are now caught, alongside the two alias-import forms that were already caught before this fix. judge.py's own docstring (:22) claimed alias imports were an undocumented gap of the scan -- they were not: `import redteam.agents.red_team as rt` and `from redteam.agents import red_team as r2` both trip it, since the scan checks the imported dotted path, not the local `as` binding. Removed that false understatement and documented the relative-import handling instead. Refs #73
…irection independence test; strengthen context-window claim FIX 6: docs/ARCHITECTURE.md:47 cited judge.py:96-100,343 (the evals.* import block and def score(). This PR's own docstring edit to judge.py (architectural-independence section) shifted both -- verified against the branch: the evals.* import block is now 101-105 and def score( is now 348 (a +5 shift, not the +4 estimated in the cold review). Corrected. Checked for other judge.py line citations in docs/ and redteam/agents/ -- none found. ALSO: docs/ARCHITECTURE.md:53(ish) cited only test_judge_agent.py, but tests/redteam/test_red_team_agent.py::test_independence_module_imports_no_judge_internals enforces the reverse direction (Red Team must not import Judge internals) -- a real shipped mechanism that was going undersold. Cited both. docs/ARCHITECTURE.md:141 said the boundary is "not (yet) an OS-process or context-window boundary", which could be read as implying a context window exists and is merely kept separate. Reworded: the Judge's default path has no model call and no context window at all (per Sec4), so there is nothing for Red Team reasoning to leak through even in principle -- stronger than a correctly-separated context window, not merely equivalent to one. Refs #73
franciszver
added a commit
that referenced
this pull request
Jul 26, 2026
…, upstream filings, and kickoff-constraint gap (issue #59) Extends tests/test_release_notes.py red-first with the corrected facts, then rewrites docs/RELEASE_NOTES_v3.0.0.md to match: - #63 and #68 are CLOSED (P3.31/P3.34) -- rewrites the limitations section as gaps found and closed, not open, and adds the three new limitations P3.31 introduced (pending_human_triage_count is per-run not directory-wide, --approve's --db-path opt-out, --reports-dir requiring --db-path). - All four findings are now filed upstream (#167-#170); corrects the false "no upstream issue for VULN-0001/2/3" paragraph and notes #169/#170 as evidence against upstream #130's closure premise, without demanding a reopen. - Owns the kickoff-brief hard-constraint gap explicitly: intent (no shared context) is met; the parenthesised OS-process mechanism is not, per ARCHITECTURE.md and the ATO packet, both already public. - Corrects the stale "redteam.observability not in the forbidden import set" claim -- it now is (test_judge_agent.py), and adds the symmetric Red-Team-side AST scan. - Re-derives test counts post-rebase (405 with sibling / 299 in CI, 106 skipped) across RELEASE_NOTES/ATO_EVIDENCE_PACKET/DEMO_SCRIPT. Rebase note: docs/ATO_EVIDENCE_PACKET.md and docs/DEMO_SCRIPT.md conflicts were resolved by taking main's post-#74/#75/#76 versions wholesale (verified main's wording already supersedes this branch's pre-rebase edits to the same passages), then bumping only the test counts to the new post-rebase live total. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTi1zKDS1ajefLSmM9oL7E
franciszver
added a commit
that referenced
this pull request
Jul 26, 2026
… by the rebase judge.py's (case, response, attempt_id) triple was cited at judge.py:96-100,343 -- score()'s actual signature moved to 348-357 as other main PRs (#74/#75/#76) touched the file. campaign.py's try:-guarded component calls were cited at ...,436 for the documentation.file_report try: block -- it moved to 446. Both verified against the current file on disk before correcting; no other citation in the doc drifted (spot-checked judge.py:44-47, chat.py:570-594, the recording path, and the JSON-report field claims against the live tree -- all still accurate). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTi1zKDS1ajefLSmM9oL7E
franciszver
added a commit
that referenced
this pull request
Jul 26, 2026
…, upstream filings, and kickoff-constraint gap (issue #59) Extends tests/test_release_notes.py red-first with the corrected facts, then rewrites docs/RELEASE_NOTES_v3.0.0.md to match: - #63 and #68 are CLOSED (P3.31/P3.34) -- rewrites the limitations section as gaps found and closed, not open, and adds the three new limitations P3.31 introduced (pending_human_triage_count is per-run not directory-wide, --approve's --db-path opt-out, --reports-dir requiring --db-path). - All four findings are now filed upstream (#167-#170); corrects the false "no upstream issue for VULN-0001/2/3" paragraph and notes #169/#170 as evidence against upstream #130's closure premise, without demanding a reopen. - Owns the kickoff-brief hard-constraint gap explicitly: intent (no shared context) is met; the parenthesised OS-process mechanism is not, per ARCHITECTURE.md and the ATO packet, both already public. - Corrects the stale "redteam.observability not in the forbidden import set" claim -- it now is (test_judge_agent.py), and adds the symmetric Red-Team-side AST scan. - Re-derives test counts post-rebase (405 with sibling / 299 in CI, 106 skipped) across RELEASE_NOTES/ATO_EVIDENCE_PACKET/DEMO_SCRIPT. Rebase note: docs/ATO_EVIDENCE_PACKET.md and docs/DEMO_SCRIPT.md conflicts were resolved by taking main's post-#74/#75/#76 versions wholesale (verified main's wording already supersedes this branch's pre-rebase edits to the same passages), then bumping only the test counts to the new post-rebase live total. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTi1zKDS1ajefLSmM9oL7E
franciszver
added a commit
that referenced
this pull request
Jul 26, 2026
… by the rebase judge.py's (case, response, attempt_id) triple was cited at judge.py:96-100,343 -- score()'s actual signature moved to 348-357 as other main PRs (#74/#75/#76) touched the file. campaign.py's try:-guarded component calls were cited at ...,436 for the documentation.file_report try: block -- it moved to 446. Both verified against the current file on disk before correcting; no other citation in the doc drifted (spot-checked judge.py:44-47, chat.py:570-594, the recording path, and the JSON-report field claims against the live tree -- all still accurate). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TTi1zKDS1ajefLSmM9oL7E
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
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.
Summary
The project's headline differentiator was stated as OS-process isolation between the Zone-A (Red Team) and Zone-B (Judge/Orchestrator/Documentation) roles. The shipped code never implemented it.
redteam/campaign.py::run_campaigncalls all four components as in-process Python objects inside oneforloop;grep -rn "multiprocessing|subprocess|Popen|fork(" redteam/ tools/returns nothing.docs/ARCHITECTURE.mdanddocs/ATO_EVIDENCE_PACKET.mdnonetheless asserted "each Zone-A and Zone-B role runs as its own OS process with its own local model instance," and used that property to differentiate this platform from Garak by name — while the shipped runner does exactly what that comparison criticized Garak for.Orchestrator ruling: option 2 (correct the docs), not option 1 (implement isolation). This PR does not implement process isolation — that is a design decision for the owner, not a finish-out task, and is called out explicitly below as still open.
What changed
docs/ARCHITECTURE.md— prose summary (§1), interaction diagram labels (§2), the six required components (§3), the fully-local model strategy (§4), the Garak/framework-choice comparisons (§5), and the P3.4-prototype relationship (§7) all now describe module- and data-level independence (separate modules, no shared imports, typed-only inputs — e.g. the Judge scores from(case, response, attempt_id)alone,judge.py:96-100,:343) as the implemented property, with OS-process isolation stated explicitly as a design goal, not yet implemented.redteam/campaign.py::run_campaigncurrently calls all four components as in-process Python objects inside oneforloop..."docs/ATO_EVIDENCE_PACKET.md— data-flow diagram labels, the trust-zone-boundary paragraph, the runtime-inventory table (line 268), and the network-egress prose corrected the same way. Also corrected: only the Red Team Agent is model-backed — Judge, Orchestrator, and Documentation are deterministic by default (rule-baseddetect(), rule/threshold logic, fixed template respectively; confirmed no model-client code inorchestrator.py/documentation.py) — every "four AI roles run locally" phrasing that implied all four call a model is corrected.docs/DEMO_SCRIPT.md— a second, related stale claim found during the deep-review gate: Beat 2 described the Judge's independence guard as scanning onlyredteam.agents/redteam.harnessand as a process boundary. Fixed to match the widened guard and the module/data framing.tests/redteam/test_judge_agent.py— widened the AST import guard's forbidden-prefix set from("redteam.agents", "redteam.harness")to add"redteam.observability"(its action log carries full Red Team attempt payloads percampaign.py:311-317; confirmedjudge.pyimports nothing underredteam.observabilitytoday, so this doesn't break the Judge). Rewrote the test's docstring to state precisely what the AST scan does not catch:import redteam+ attribute access,from redteam import agents,importlib.import_module(...),__import__, dynamic/runtime-built import strings, and transitive imports via a modulejudge.pyitself imports.redteam/agents/judge.py— module docstring corrected: "separate module/context by construction," not "separate process."tests/test_doc_consistency.py— new red-first guard,test_no_committed_doc_claims_os_process_isolation_is_implemented. Scoped todocs/;planning/KICKOFF_PROMPT.md's verbatim historical kickoff prompt ("Build four agents with architectural (separate process/context) independence") is excluded — it's the original requirement as asked, written before any code existed, not a claim about shipped behaviour, and rewriting it would falsify the historical record.ATO_EVIDENCE_PACKET.mdandDEMO_SCRIPT.mdfor the one new test (tests/test_doc_test_counts.pyenforces these; 233->234 passed CI-shape, 339->340 with-sibling).Left open for the owner (not decided here)
Correcting the docs does not settle whether the platform should implement OS-process isolation. That decision — implement it, or formally adopt module-level independence as the intended architecture — remains open.
AST guard: widened, not just documented
Chose to widen (
redteam.observabilityadded to the forbidden-prefix set) rather than leave the gap only documented, because the actual attack it misses (a Judge quietly reading the action log's full Red-Team attempt payloads) is cheap to close and the Judge doesn't need anything underredteam.observabilitytoday — verified viagrep -n "^import\|^from" redteam/agents/judge.pybefore widening (no such import exists). The guard's remaining, undocumented-until-now limits (alias imports,importlib, dynamic strings, transitive imports) are now stated precisely in the test's own docstring rather than implied to be covered.Red-first proof
The new guard genuinely fails on unmodified
main(verified by stashing the doc fixes and running the test alone):The red-first test was committed first (
90cba1c), confirmed failing, then the doc fix landed in a second commit (later amended locally to drop an accidental "Closes #73" from the commit body, before push).Gates
subprocess.run(["git", "grep", ...])call with static list-form args (noshell=True, no untrusted input). No findings.DEMO_SCRIPT.md's Beat 2 described the pre-widening two-prefix guard as a process boundary) before landing.Verification
sha256sum docs/vuln_reports/*.jsonunchanged before/after (never touched):VULN-00016269b60e...,VULN-00022afe57f1...,VULN-0003bb49602d...,VULN-0004836cd053....pytest tests/ -q-> 340 passed (sibling Phase 2 checkout present, local).pytest tests/test_doc_consistency.py tests/test_doc_test_counts.py -q-> 2 passed / 1 passed respectively, confirming the new guard and the doc-count self-check both hold post-fix.docs/RELEASE_NOTES_v3.0.0.mdor anydocs/vuln_reports/*.json.Closes #73