fix(review): stop rejecting the approved autoreview fixture - #1309
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs maintainer review before merge. Reviewed August 30, 2026, 1:47 PM ET / 17:47 UTC. ClawSweeper reviewWhat this changesThis PR expands the host-side reviewed-fixture registry and makes secret-scan classification require every Git endpoint associated with a scanned blob to match that fixture’s exact digest, approved path, and regular-file mode. Merge readinessKeep this PR open for normal landing review. The endpoint-association fix resolves the earlier P1 mode-inheritance flaw while preserving fail-closed scanning for every unapproved digest, path, or mode. Priority: P1 Review scores
Verification
How this fits togetherClawSweeper stages review inputs and Git diff endpoints for TruffleHog before any model review begins. This change affects the narrow post-scan classifier that may recognize explicitly approved synthetic test fixtures; all other findings still stop the review. flowchart TD
A[Review inputs and Git endpoints] --> B[Stage source blobs]
B --> C[Native TruffleHog scan]
C --> D{Exact fixture and detector match?}
D -->|No| E[Refuse review]
D -->|Yes| F{Every path and mode approved?}
F -->|No| E
F -->|Yes| G[Emit value-free audit]
G --> H[Continue to review model]
Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land only the narrowly documented exact-digest/path/mode exception while retaining the existing fail-closed behavior for every other scanner finding. Do we have a high-confidence way to reproduce the issue? Yes. The PR supplies current-head native TruffleHog traces for the original false rejection and the previously accepted mode/alias boundary cases, and the focused source coverage matches those scenarios. Is this the best way to solve the issue? Yes. Retaining every endpoint association and requiring all of them to satisfy the same exact fixture policy is the narrowest way to admit the approved synthetic URI without introducing a broad scanner exemption. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c179a466606f. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
|
Additional reproduction for the same source-association repair requested in the current review: the fence must preserve path/digest associations as well as endpoint modes. This is verified against published head I reproduced two cases with real TruffleHog 3.97.1 and the immutable production scanner owner. Both associated source files remain mode
Current main The reason is visible across the two owners: the producer retains only qualifying source associations, while the classifier accepts a nonempty intersection instead of requiring every association to match that digest's policy. Blob deduplication therefore lets the approved occurrence lend eligibility to a different source. Please cover these cases in the same endpoint-provenance repair: retain all scanned logical source/mode associations and reject classification if any association is ineligible for the exact digest, including associations supplied by the index/snapshot path. Preserve positive sharing of the actual autoreview digest at its two approved paths, with the existing per-source audit notices. The existing The existing owner retains branch, push, and merge control. This note adds concrete pre-merge evidence; it requests no policy broadening, fixture rewrite, historical-comment edit, or scanner bypass. |
What Problem This Solves
Resolves a problem where exact reviews stop before model execution when an unrelated edit touches the autoreview hardening test file containing an unchanged, reviewed synthetic credential-rejection fixture.
The confirmed example is agent-skills PR205, with exact-review job 99170200224 failing with
AgentInputScanError: findingsat host source8d11c21558ba90ac6812b80b95efd0dd9b0aa936. Complete before/after blob coverage includes that unchanged fixture even outside patch context; the clean independent exact-diff scan was not contradictory.Why This Change Was Made
Register the maintainer-approved full-URI digest
662a886a0fd7447dad0acda3aeccc9eb539fc90438b453de7e2f523ca7ee6c83only for mode100644Git blobs at:skills/autoreview/tests/test_autoreview_hardening.py.agents/skills/autoreview/tests/test_autoreview_hardening.pyThe existing action-ledger entry remains supported. Each staged OID retains every scanned logical endpoint's path and mode, including ineligible references. Every reference must satisfy the same digest/path/mode policy before classification; a valid base cannot authorize an executable head/index/worktree or an unapproved alias. Raw bytes remain staged once per OID. Unique audit sources are derived only after this validation.
This is not a domain, credential-word, test-name or unchanged-line exemption; no neighboring URI is implicitly approved. Full before/after/deleted/index/raw-worktree and prompt/schema/diff/additional coverage, scanner flags, budgets, strict output/version/verification/literal-line checks, source fences and cleanup remain intact. One unmatched finding refuses the whole scan, with no success notice. No original fixture literal was added to repository test sources, rewritten upstream, or obfuscated.
Review Finding Disposition
Accepted and fixed the P1 endpoint-mode finding. Published head
e8dfeaf52a8fd6c29d5ced427c80a0ce1207df8binherited approval across a same-OID100644→100755transition because it retained only eligible references. Real TruffleHog reproduced that admission at both approved paths. The legacy classifier at production8d11c21558had the same inheritance pattern, so the repair consistently enforces the fence for both entries rather than adding a legacy exception.All three rank-up requests were applied: retain complete endpoint eligibility context; add mode/alias/snapshot regressions; publish a redacted after-fix rejection trace. New hermetic regressions produced 75 passed / 7 failed against the prior code, then 82 passed / 0 failed after the repair. They cover both mode-transition directions, executable head/index/worktree snapshots, ineligible shared aliases, and valid repeated regular snapshot OIDs.
User Impact
Reviews containing this exact approved fixture can proceed, while executable or otherwise ineligible source references remain blocking. TruffleHog findings are not hidden: accepted records emit digest/source/blob/line/decoder/count notices. Shared-blob counts are per-source references and must not be summed as unique findings.
OpenClaw Bay Impact
Unaffected: no lifecycle, queue, workflow, publication, observer API or dashboard data-contract change. The raw-Git provenance repair and historical review comments are untouched.
Documentation Impact
Updated the active README Safety Model and existing unreleased changelog entry. Documentation now states that every scanned endpoint association must qualify and distinguishes classification from detector coverage. No new documentation page or lifecycle category.
Evidence
c8f03ea935473ee2702435f0bb6ca1f36e825918.scoped-clean, no P0/P1 findings. The separate committed-base Codex review of the complete PR at this head is alsoscoped-cleanwith no findings.pnpm run checkpassed: 4,090 passed, zero failed, nine skipped, plus all 13 changed-coverage tests; focused scanner/raw-Git suite 82 passed. All coverage gates ran unchanged.Real Behavior Proof
Surface: the real
scanAgentInputadmission API and native TruffleHog. A counter immediately after admission is an inert downstream canary; no review model or target helper is invoked.Environment: macOS, Node 24.20.0, external TruffleHog 3.97.1, binary SHA-256
f6899e5521ff060634a9599e02215195d29d330e9a902feb3f5322eedcb7ba21. The original baseline scanner was extracted from production8d11c21558ba90ac6812b80b95efd0dd9b0aa936. A separate frozen copy of published heade8dfeaf52a8fd6c29d5ced427c80a0ce1207df8bestablishes the endpoint bug before this repair.Every run keeps the production flags:
Original false-positive regression
The pinned target range is
ae75f60e8d454f1cf44ec4613e10ec9ea7f2ade7..a8466c1d860588a083610fe41fd277c1d88b14e0: five changed paths, ten complete before/after blobs, plus raw patch/metadata. The fixture and named test AST are unchanged; neither literal occurs in patch context.findings, canary 0findings, canary 0findings, canary 0findings, canary 0findings, canary 0, no success noticeThe negatives cover wrong path, executable Git mode, altered password/host, mixed findings, prompt, schema, additional input, raw patch context, deleted bytes, the wrong digest at an otherwise approved path, and real Base64-decoded URI findings. All sixteen original scenarios were requalified; the production-before/current-after comparison preserves 32 matching admission assertions.
Primary scans still report the same two URI findings and 112 chunks / 1,388,714 processed bytes. The vendored source/path replay reports 53 chunks / 654,618 bytes. Both findings are URI detector 17, filesystem source 15, PLAIN, unknown verification state, equal Raw/RawV2 matching the approved digest, null extras, and complete error-free scanner logs. Their value-free identities are:
[ {"blob":"3430fd935c089a5f4c5c1cc4b5422d1d43773778","line":3424,"occurrences":1}, {"blob":"58dc0fbb4309a9b02a31be69bbcc65f7bddd244f","line":3563,"occurrences":1} ]Endpoint-boundary regression
The canonical/vendored mode fixtures reuse exact candidate blob
58dc0fbb4309a9b02a31be69bbcc65f7bddd244f. The legacy case reuses its existing approved literal in a small fixture at its approved path. Each mode-only commit preserves its blob OID, and raw patches contain no URI literal, so rejection cannot be attributed to a patch finding.100644→100755, same OIDfindings, canary 0, no success notice100644→100755, same OIDfindings, canary 0, no success notice100755→100644, same OIDfindings, canary 0, no success notice100644→100755, same OIDfindings, canary 0, no success noticefindings, canary 0, no success noticeEach mode-transition case has one actual native URI finding. The normalized after-fix record for the canonical case is:
{ "source":"skills/autoreview/tests/test_autoreview_hardening.py", "fixtureSha256":"662a886a0fd7447dad0acda3aeccc9eb539fc90438b453de7e2f523ca7ee6c83", "endpointModes":["100644","100755"], "endpointBlobOids":["58dc0fbb4309a9b02a31be69bbcc65f7bddd244f","58dc0fbb4309a9b02a31be69bbcc65f7bddd244f"], "rawPatchContainsFixture":false, "nativeUriFindings":1, "outcome":"findings", "downstreamCanaryStarts":0, "successAuditNotices":0, "modelInvoked":false }Current-head binding: Re-executed against committed head
c8f03ea935473ee2702435f0bb6ca1f36e825918: thirteen original admission cases, seven endpoint cases, the remaining three original cases, and the exact minimal replay below all met their stated outcomes. The numeric-HTML control remains separately recorded as unchanged nondetection, not refusal proof. Mode-transition findings reject with canary 0 and no success notice; ordinary canonical/vendored cases admit with the same native findings and audits. All five committed source fingerprints match the executed code. Compiled scanner dependency-closure SHA-256:7957d23b1c0fb097dacc1db6db9264959ddf6b2894d042090393ce8a30c28e42. No review model ran.Minimal replay: after building either host scanner version, set
SCAN_MODULEto its absolute compiledagent-input-scan.jspath andSCAN_TARGET_DIRto the clean pinned agent-skills head with its base available. This command was executed for the candidate:Limits: original production prompt/schema bytes were unavailable; replay uses harmless text with real source bytes. Vendored and mode-transition cases are controlled source/path replays, not a claimed full historical OpenClaw range. The local scanner binary is macOS, not the original Linux binary. A fully numeric-HTML-encoded Python-literal probe yields zero findings on both old and fixed code: that unsuccessful detection assumption is retained and not counted as refusal proof. Reported encoded findings still reject; real BASE64 URI findings matching the approved digest were observed and refused. No detector option, coverage check, or threshold was weakened. Raw URI values and verification diagnostics remain undisclosed; these normalized receipts are not agent transcripts.