Make the V5 diagnostic harness executable and fail closed - #3603
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## Ghh32fbkyuqkrndyfjzwwarkb4dmfosug #3603 +/- ##
==================================================================
Coverage 91.85% 91.85%
==================================================================
Files 20 20
Lines 6093 6093
==================================================================
Hits 5597 5597
Misses 496 496 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b1b6f491c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if consistency_inputs[0] != consistency_inputs[1]: | ||
| raise ProtocolError(f"consistency reviewers did not receive byte-equal input: {mode}") |
There was a problem hiding this comment.
When c1 and c2 are leased with JSON packets that differ only in whitespace or key order, load_bound_input_packet parses both and this object comparison succeeds, even though each reviewer saw different bytes. build_expected_evaluator_launch binds each launch to its supplied raw packet independently, so the aggregate reconstruction does not reject this elsewhere. This violates the byte-equal reviewer-input invariant and permits reviewer-specific signaling; compare load_bound_input_packet_bytes(...) for the two attempts and require the deterministic canonical encoding.
Useful? React with 👍 / 👎.
| packets = [load_bound_input_packet(item["lease"]) for item in reviewer_attempts] | ||
| if packets[0] != packets[1]: | ||
| raise ProtocolError("materiality reviewers did not receive byte-equal input") |
There was a problem hiding this comment.
When m1 and m2 receive semantically equal JSON with different serialization, this parsed-object comparison accepts it despite build_materiality_review_packet explicitly defining a byte-identical shared packet. The launch verifier authenticates each supplied byte string separately, and the later comparison to expected_packet is also object-level, so reviewer-specific formatting survives aggregate verification and can compromise the claimed independent review. Compare both raw packet byte strings and require the canonical deterministic packet bytes.
Useful? React with 👍 / 👎.
| raise IntegrationError("unknown bundle kind") | ||
| snapshot = snapshot.resolve() | ||
| receipts = receipts.resolve() | ||
| output = output.resolve() |
There was a problem hiding this comment.
When finalize --output is passed a dangling symlink, calling resolve() first discards the symlink entry, after which output.exists() checks the absent target and finalization publishes the bundle there. This bypasses the advertised no-replace semantics for the caller-supplied pathname and can redirect the production bundle outside its intended custody directory; unlike _prepare_snapshot, this path never checks is_symlink() before resolution. Validate the lexical output entry with lstat/is_symlink before resolving it.
Useful? React with 👍 / 👎.
8b1b6f4 to
245af42
Compare
245af42 to
2ddc311
Compare
2ddc311 to
98c9f96
Compare
fa2f962 to
9197f43
Compare
2c9a61a to
8d546d7
Compare
9197f43 to
013d38a
Compare
8d546d7 to
ee97491
Compare
6163891 to
886ad3a
Compare
91c37c6 to
82f1cbd
Compare
886ad3a to
271bd9a
Compare
82f1cbd to
879e1c9
Compare
271bd9a to
7d4ace9
Compare
879e1c9 to
df52065
Compare
7d4ace9 to
5ba5deb
Compare
df52065 to
c941428
Compare
5ba5deb to
0393e94
Compare
The first V5 draft described the intended diagnostic study but could not safely execute it. Review found that READY promotion was impossible, several CLI routes had stale arities, padded report IDs disagreed with their validators, host-specific paths destroyed prompt equality, evaluator packets named only digests rather than readable evidence, materiality had no runnable lifecycle, and DRAFT/READY schemas and runtime-state rules contradicted one another. Replace that draft boundary with an authenticated prepare-snapshot, private-review, and finalize lifecycle. The production lock now binds the trusted source declaration, both skill packages, every target, the harness programs, the staged word counter, 120 report prompts/plans/launches, 43 evaluator assignments, hook-specific review contracts and receipts, empty pre-lock runtime state, and a separately custodied external commitment. Synthetic paths carry an authenticated test-only kind and cannot mint production artifacts. Adversarial review exposed further trust failures: executing an unverified candidate verifier, source-copy TOCTOU, ambiguous line-oriented manifests, arbitrary review claims and evaluator launches, stale receipts, unreviewed runtime state, irrelevant schema leakage, optional or crash-unsafe external commitments, and a coherent attack that rebound an F report to target E and a V5 condition to the V4 package. Use injective framed commitments, trusted in-process regeneration, exact artifact/check/evidence inventories, private review copies, atomic no-replace publication, explicit custody-bound recovery, and exact target/condition/package joins to close those failures. Preserve each attack as a negative self-test. Complete the execution protocol around those locked inputs: readable content-addressed packets, two independent consistency reviews, conditional adjudication, materiality review and ledger reconstruction, exact projection and control joins, deterministic aggregate rebuilding, production state authentication, canonical path checks, leases and seals with crash recovery, and fail-closed bound gate evaluation. Unbound caller data cannot make D-STATIC pass. Validation covers prepare, integration, protocol, and draft-verification self-tests; hostile temporary paths; all CLI help surfaces; all JSON parsing and schemas; adversarial provenance, packet, lease, gate, commitment, and assignment mutations; whitespace; and cache hygiene. This remains diagnostic infrastructure: G-ISOLATION and G-OUTPUT-FINALIZATION deliberately remain FAIL, so the commit cannot support a release or terminal-VN claim. gherrit-pr-id: Ghchu3g3fkri2ofhgm5addjto4rfkyvyi Agent-Authored-By: AI agent acting on Josh Liebow-Feeser's behalf
c941428 to
afc8a38
Compare
0393e94 to
cbcb1ae
Compare
3 participants
The first V5 draft described the intended diagnostic study but could not safely execute it. Review found that READY promotion was impossible, several CLI routes had stale arities, padded report IDs disagreed with their validators, host-specific paths destroyed prompt equality, evaluator packets named only digests rather than readable evidence, materiality had no runnable lifecycle, and DRAFT/READY schemas and runtime-state rules contradicted one another.
Replace that draft boundary with an authenticated prepare-snapshot, private-review, and finalize lifecycle. The production lock now binds the trusted source declaration, both skill packages, every target, the harness programs, the staged word counter, 120 report prompts/plans/launches, 43 evaluator assignments, hook-specific review contracts and receipts, empty pre-lock runtime state, and a separately custodied external commitment. Synthetic paths carry an authenticated test-only kind and cannot mint production artifacts.
Adversarial review exposed further trust failures: executing an unverified candidate verifier, source-copy TOCTOU, ambiguous line-oriented manifests, arbitrary review claims and evaluator launches, stale receipts, unreviewed runtime state, irrelevant schema leakage, optional or crash-unsafe external commitments, and a coherent attack that rebound an F report to target E and a V5 condition to the V4 package. Use injective framed commitments, trusted in-process regeneration, exact artifact/check/evidence inventories, private review copies, atomic no-replace publication, explicit custody-bound recovery, and exact target/condition/package joins to close those failures. Preserve each attack as a negative self-test.
Complete the execution protocol around those locked inputs: readable content-addressed packets, two independent consistency reviews, conditional adjudication, materiality review and ledger reconstruction, exact projection and control joins, deterministic aggregate rebuilding, production state authentication, canonical path checks, leases and seals with crash recovery, and fail-closed bound gate evaluation. Unbound caller data cannot make D-STATIC pass.
Validation covers prepare, integration, protocol, and draft-verification self-tests; hostile temporary paths; all CLI help surfaces; all JSON parsing and schemas; adversarial provenance, packet, lease, gate, commitment, and assignment mutations; whitespace; and cache hygiene. This remains diagnostic infrastructure: G-ISOLATION and G-OUTPUT-FINALIZATION deliberately remain FAIL, so the commit cannot support a release or terminal-VN claim.
Agent-Authored-By: AI agent acting on Josh Liebow-Feeser's behalf
This PR is on branch codex/unsafe-rust-stack.
Latest Update: v15 — Compare vs v14
📚 Full Patch History
Links show the diff between the row version and the column version.
⬇️ Download this PR
Branch
git fetch origin refs/heads/Ghchu3g3fkri2ofhgm5addjto4rfkyvyi && git checkout -b pr-Ghchu3g3fkri2ofhgm5addjto4rfkyvyi FETCH_HEADCheckout
git fetch origin refs/heads/Ghchu3g3fkri2ofhgm5addjto4rfkyvyi && git checkout FETCH_HEADCherry Pick
git fetch origin refs/heads/Ghchu3g3fkri2ofhgm5addjto4rfkyvyi && git cherry-pick FETCH_HEADPull
Stacked PRs enabled by GHerrit.