diff --git a/.gitignore b/.gitignore index febbc85c48..0f8edd3d37 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,5 @@ __pycache__/ .pytest_cache/ .codegraph/ strix_runs/ +.venv/ +.venv*/ diff --git a/CHANGELOG.md b/CHANGELOG.md index bf192f6a9e..95f0ae1d3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,19 @@ - `opencode-review-dispatch.yml`'s `emit_strix_provider_failure_finding` rendered one fixed finding for every `STRIX_PROVIDER_UNAVAILABLE` line, whose Root cause read "The contextual-orchestrator gateway or its discovered provider pool was unavailable for this run". `#1953` had just given the Strix sandbox bootstrap failure its own second verdict token (`STRIX_SANDBOX_UNAVAILABLE`) precisely because that attribution is wrong for it -- the sandbox container never reaches its Caido proxy, so the run dies before the gateway serves anything -- and this consumer re-applied the wrong attribution one step downstream, into the review findings and the failure census. The emitter now branches on the second token: a sandbox verdict gets a finding that names Strix's sandbox, says the verdict does not name the gateway, and tells the reader not to change gateway or provider configuration on its strength. A `STRIX_PROVIDER_UNAVAILABLE` line without the token keeps its existing text verbatim, so the gateway class has no regression surface. No test covered this finding text at all before (`gateway or its discovered provider pool` matched nothing under `tests/`); `tests/test_opencode_dispatch_strix_sandbox_finding.py` now runs the production emitter from the published run block and pins both directions plus the no-signal case. Refs #1953, #1935. -### Strix gate keeps a recovered transient model error from failing a completed scan +### Strix gate proves a recovered transient model error with current-attempt receipts -- `scripts/ci/strix_quick_gate.sh` `sanitize_known_strix_report_warnings` now also strips strix-agent's `strix.core.execution: transient model/provider error for ; replaying turn (attempt n/m, backoff Ns): …` WARNING lines before the report failure-signal scan. strix-agent 1.5.3 (`strix/core/execution.py:763`) emits that line only inside its bounded transient-retry branch, immediately before the replay runs; an exhausted retry logs `agent run failed for …; marking failed` at ERROR with a traceback and exits non-zero, and both of those still fail the gate. Observed on `.github#1689` run `34013778497`: a completed 63-minute scan (`run.json` `completed`, SARIF 0 results, attempt exit 0) was failed closed as `STRIX_PROVIDER_UNAVAILABLE … exhausted` on three such warnings, and the scheduler then dispatched another same-head scan. The pattern is anchored before the exception repr so the same class keeps matching after a gateway pin advance changes the exception type; re-verify the message format on every strix-agent bump. One documented side effect: when a provider's 503 body appears only inside a retry line's exception repr, removing that line also removes the only text `has_strix_report_provider_failure_signal` would have matched in the report log, which can make `is_model_retryable_error`'s report-only branch read a genuine outage as non-retryable. The direction is fail-closed (an exhausted retry still exits non-zero with its ERROR and traceback retained), and with a contextual-orchestrator primary the verdict branch answers before that classifier is consulted, so no path today changes its outcome; if fallback-model classification is ever wanted for a non-gateway primary, read the pre-sanitize attempt copy that `preserve_attempt_log` already keeps. Tests: `tests/test_strix_recovered_transient_sanitizer.py`. +- The gate retains strix-agent's raw `strix.core.execution: transient + model/provider error ... replaying turn (attempt n/m, backoff Ns)` warning + and classifies it as recovered only when that same attempt supplies a new + completed/successful `run.json` and valid SARIF. The initial sanitizer-only + implementation deleted the warning before checking either receipt; that + made malformed terminal evidence and exhausted `attempt n/n` warnings pass. + Unknown warnings, exhausted retries, GitHub `::error::` commands, malformed + or stale receipts, and the terminal ERROR/traceback remain fail-closed. + Observed on `.github#1689` run `34013778497`; regression coverage lives in + `tests/test_strix_recovered_transient_sanitizer.py` and + `tests/test_strix_attempt_evidence_provenance.py`. ### Review sidecar preflight postpones a rate-limited account's candidates instead of banning them @@ -68,6 +78,27 @@ - Raised `hourly-review-repair.yml`'s discovery ceiling from 50 to 200 while rotating deterministic 50-PR deep-inspection windows by hourly run number. The scheduler hydrates only the selected window and stops immediately after its single dispatch, preserving access to newer PRs without quadrupling expensive review/check/comment work. See `docs/doctoring/hourly-review-repair-single-file-consolidation.md`'s 2026-09-03 follow-up. ## [Unreleased] +- Classify a Strix in-process provider retry warning as recovered only when + the same current attempt produces a new structured `run.json` with + `status=completed`, `scan_results.scan_completed=true`, and + `scan_results.success=true`, plus well-formed SARIF 2.1.0 results. Raw logs + remain published. Exhausted retries, malformed or stale terminal evidence, + unknown warnings, GitHub `::error::` commands, fatal/denied/timeout signals, + and source findings remain fail-closed. This fixes the Inkspan #402 false + infrastructure verdict from run `33927906573` without treating its 20-file + PR snapshot as a full-repository security approval. +- Stop treating ordinary security prose copied into Strix's captured console + as an infrastructure receipt. OriginWeave #166 completed its current attempt + with `scan_completed=true`, `success=true`, process exit 0, and empty SARIF, + but phrases such as “hard-denied first” and “mutations are denied outright” + matched the former word-anywhere `denied` console grep. Ambiguous console + `denied` now requires a `Denied:` control record; warning/fatal text, report + logs, typed provider/timeout detectors, incomplete or stale receipts, + exhausted retries, malformed evidence, and blocking findings remain + fail-closed. +- Bind attempt-scoped `vulnerabilities/*.md` evidence to content digests as + well as paths. A current attempt that rewrites Strix's reused report path is + now fresh evidence, while an unchanged predecessor report remains stale. - Include merge-scheduler entrypoint, core, and regression-test changes in the existing runtime-quality workflow's trigger and suite selector. Scheduler workflow edits retain queue checks and also select the full review-repair @@ -307,6 +338,95 @@ Semantic Versioning where the repository publishes a release. docstrings; no production behavior change except the two dead-code removals (both provably unreachable, so behavior-neutral). - **Pin the three central required review workflows (Strix, OpenCode Review, Noema Review) off the observed starved floating `ubuntu-latest` runner image.** Following the same repair already rolled out to security gates (`#1618`) and the merge scheduler (`#1609`), `strix.yml`, `opencode-review.yml`, and `noema-review.yml` now request the explicit `ubuntu-24.04` image on every job. These three workflows are the org's own required-workflow gate for every sibling repository, so a starved floating image here directly contributes to organization-wide required-check queuing. New `tests/test_required_review_runner_image_contract.py` asserts no job in any of the three files still requests the floating image. Also fixed 4 pre-existing, unrelated test failures on `main` left by `#1630`'s organization-sweep rotation cadence change (every 15 minutes to hourly, to reduce control-plane pressure under the same Actions saturation): `tests/test_required_workflow_queue_contract.py`'s rotation-index tests still asserted the old `/ 900` (15-minute) divisor against the new `/ 3600` (hourly) production value. +- Fix `strix_quick_gate.sh` failing to fail closed when Strix exits `0` with + zero `vulnerabilities/*.md` report artifacts (log-only "success" is not + evidence of a clean scan). Devin review on the successor PR then caught a + gap in that fix: the artifact-presence check was scoped to the whole gate + run's accumulated reports, so a genuinely hollow attempt (its own Strix + invocation exited `0` and wrote nothing) could still pass by riding on an + *earlier*, already-superseded attempt's leftover report (same-model retry + or a different fallback model tried first). The check is now attempt-scoped + -- each `run_strix_once()` invocation snapshots which report artifacts + already existed immediately before it launches Strix, and only accepts one + that is new since that snapshot -- while severity scanning for blocking + (HIGH/CRITICAL) findings stays cumulative across every attempt, so a real + finding from an earlier attempt is never silently dropped. A second, deeper + Devin Review finding on the same PR then showed the artifact-presence + contract itself was wrong even before attempt-scoping: the pinned + `strix-agent==1.5.3` only writes `vulnerabilities/*.md` when a scan has + findings, so a genuinely clean (zero-finding) scan never produces one and + would fail closed every time, since `#1495`. The success-evidence contract + now checks Strix's own always-written `run.json` (`"status": "completed"`) + instead, still attempt-scoped the same way; blocking-finding severity + scanning over `vulnerabilities/*.md` remains cumulative and unchanged. + A third round then found the run.json switch had been applied to the wrong + call site too: `has_only_below_threshold_vulnerabilities()`'s presence + guard needs proof of genuine severity evidence from an attempt, even one + whose process later exited non-zero (e.g. a real below-threshold finding + written just before a mid-scan connection error), not proof the attempt + reached full completion -- restored the `vulnerabilities/*.md`-based + attempt-scoped check there, keeping run.json-based completion only for + `run_strix_once()`'s own success acceptance. `has_new_completed_strix_run()` + itself was also hardened: structural JSON parsing (via `python3`) instead + of a raw-text regex match, so completion text nested under an unrelated + field or a malformed record can no longer be mistaken for a genuine + top-level `"status": "completed"`, and attempt identity now compares + SHA-256 content digests instead of paths alone, so a run.json rewritten in + place with new results counts as new evidence while an unchanged + predecessor record does not. The test harness's implicit `trap ... EXIT` + backstop mechanism (which silently manufactured default evidence for any + untested success scenario) was replaced with an explicit helper each + scenario that wants that evidence calls deliberately, removing the + opt-out list this pattern previously needed. + A fourth round then found a related gap Devin flagged after round 3 shipped: + `has_only_below_threshold_vulnerabilities()`'s presence guard correctly uses + the attempt-scoped `vulnerabilities/*.md` check (not run.json completion) so + it can still accept genuine partial evidence from an attempt whose process + later crashed non-zero, but that same guard could also rescue an *rc=0* + attempt `run_strix_once()` had already determined was hollow (no completed + run record), as long as that same hollow attempt happened to also write a + below-threshold report before failing to record completion. Added a sticky + `STRIX_HOLLOW_SUCCESS_DETECTED` flag (set by `run_strix_once()`'s own + hollow-success branch, reset once per `run_current_target_scan()` call + alongside the existing `INFRA_ERROR_DETECTED`/`ZERO_FINDINGS_REPORTED` + flags) that `has_only_below_threshold_vulnerabilities()` now checks and + fails closed on, mirroring its existing `INFRA_ERROR_DETECTED` guard. + New regression: `hollow-success-with-below-threshold-report-fails-closed`. + A fifth round then found the fourth round's flag only guarded one of two + alternate success paths: `evaluate_pull_request_findings()` can + independently set `PR_FINDINGS_DECISION=allow_baseline` (an at-or-above- + threshold finding confined to unchanged PR files) and let the caller + return success, with no visibility into completion evidence at all -- + reachable at both the primary and fallback-model call sites once + `has_only_below_threshold_vulnerabilities()` had already failed. Gated + both call sites' success branch on `STRIX_HOLLOW_SUCCESS_DETECTED` too + (the function itself is still always called, so `PR_FINDINGS_DECISION` + stays freshly computed for downstream logic), with an explicit fail-closed + return immediately after. This also surfaced that the flag needed + rescoping: it was reset once per `run_current_target_scan()` call + (matching the deliberately cumulative `INFRA_ERROR_DETECTED`), but a + hollow *primary* attempt must not taint a genuinely completed *fallback* + attempt's own evaluation -- moved the reset to the top of every + `run_strix_once()` attempt instead, so it reflects only the + most-recently-concluded attempt. New regression: + `hollow-success-with-baseline-unchanged-report-fails-closed`. + A sixth round then found the fifth round's explicit fail-closed `return 1` + (added right after gating the `evaluate_pull_request_findings()` success + branch) was itself too broad: it also blocked the unrelated, legitimate + fallback-to-a-distinct-model path whenever a hollow primary attempt's + failure looked retryable, even though the flag is attempt-scoped so a + genuinely completed fallback attempt cannot be tainted by an earlier + hollow one. Removed that blanket return at both call sites (primary and + fallback), keeping only the two success-path gates already added -- a + hollow attempt not rescued by either alternate success path now falls + through to the same `case`/`fail_unmapped_threshold_report()`/ + `is_model_retryable_error()`/fallback-model logic every other failed + attempt already goes through, unchanged. New regression: + `hollow-primary-recovers-via-completed-fallback` (a hollow primary whose + log carries a retryable `strix.ModelBehaviorError` -- deliberately not a + rate-limit/timeout marker, since those are infrastructure-error signals + `run_strix_once()` itself already fails closed on earlier -- reaches and + succeeds via a distinct, genuinely completed fallback model). - **Refresh Noema reviewer App authority after long model work (`#1616`).** A real `naruon#1497` review outlived its repository-scoped GitHub App installation token and failed the next exact-head GitHub operation with HTTP 401. The trusted workflow now prepares the validated verdict into a private runner-local envelope, remints the same least-privilege repository-scoped App authority after model work, independently re-fetches exact live head/reviewer identity, and only then publishes. Skipped preparation creates no envelope, predecessor App tokens cannot authorize publication, PAT/OIDC remain explicit fail-closed sources, malformed handoffs are cleaned up, and executable plus step-scoped regressions cover stale-head, identity, alias, workflow wiring, and migration of legacy broader-suite contracts away from the retired single-process reviewer path. - Fix `existing_noema_review()` treating a "legacy" Noema review (one posted before `NOEMA_REVIEW_FOOTER_MARKER` existed) as proof the current head was already reviewed. @@ -1326,6 +1446,19 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Removed `scripts/ci/source_fix_pr1714_no_model_job_timeout.py` and + `scripts/ci/source_fix_pr1715_no_model_job_timeout.py` plus their paired + one-shot workflows. Verified both were fully orphaned debris before + deleting: their target files (`pr-review-autofix.yml`, `noema-review.yml`, + and the two associated test files) had already been hand-repaired with + differently-worded fixes and new test names + (`test_autofix_job_has_no_job_level_timeout`, + `test_noema_review_job_has_no_job_level_timeout` — see + `docs/doctoring/autofix-and-noema-review-model-job-timeout-removal.md`), + so neither script's exact literal-text preconditions matched current + content any longer; running either would only raise `SystemExit`. Their + presence with 0% test coverage was failing this repo's 100% coverage gate + after merging `main` into PR #1563. - Prefer the job-scoped `github.token` when the central OpenCode dispatch publishes a commit status back to the same `.github` repository. The job's declared `statuses: write` permission now reaches the endpoint instead of an diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 1cc9e20313..5e3eba3900 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -211,6 +211,40 @@ flowchart LR | #790 | fix(coverage): retry transient trusted uv downloads | `463ddbad84ee40f56f2196af2aa41f1dd4100907` | `main` | DIRTY | CHANGES_REQUESTED | ready | | #789 | feat(coverage): add bounded PyO3 peer-evidence gate | `3ffde3c5d3c98f0c840abcba151af08cf0255b46` | `main` | DIRTY | CHANGES_REQUESTED | ready +## 2026-09-05 recovered Strix retry evidence boundary + +- Inkspan PR #402 at `637b910d25dabb363e40d535c6d89f4a5beb8c6d` + supplied a current-attempt counterexample in Actions run `33927906573`, job + `101234352982`, artifact `9967936086`. Strix recorded one bounded + `strix.core.execution` HTTP 500 replay (`attempt 1/5`) and subsequently + wrote `run.json` with completed/successful scan state, SARIF 2.1.0 with an + empty results array, and process exit 0. The central gate nevertheless + classified the retained warning as terminal provider unavailability. +- The owner contract now distinguishes a recovered in-process retry from a + terminal provider failure only through new, current-attempt structured + evidence: exact completed/success booleans plus structurally valid SARIF. + Raw warnings remain in the artifact. Unknown warnings, exhausted retries, + GitHub `::error::` commands, fatal/denied/timeout signals, stale or malformed + records, and all blocking source findings continue to fail closed. The + companion partial-finding predicate snapshots SHA-256 digests, so a reused + `vulnerabilities/*.md` path counts only when the current attempt changed its + contents; unchanged predecessor bytes remain stale. +- This is evidence for the 20-file changed-source snapshot declared by the + report. It is not full-repository security approval and cannot be reused for + another head or scope. +- OriginWeave PR #166 at `e84a1a2cc82b1c666218efd441da97849f47b8c2` + exposed the adjacent console/report provenance bug in run `33929688857`, job + `101237371800`, artifact `9968177796`. Its final current attempt exited 0 and + produced a completed/successful `run.json` plus empty SARIF, while the broad + console predicate matched legitimate report prose: “forbidden R5 class is + hard-denied first” and “cross-origin mutations are denied outright”. The + ambiguous console `denied` token now requires a `Denied:` control record; + warning/fatal text and raw report logs retain their broad fail-closed scan, + and typed provider, timeout, exhausted-retry, incomplete/stale/malformed- + receipt and source-finding controls are unchanged. Earlier attempts remain + audit evidence but cannot override the authoritative terminal receipt for + the current attempt. + ## 2026-08-25 central Strix fallback contract recheck - `main` at `a724582a0768129d481385070bf8f05b2620dd2c` changed the direct-OpenAI @@ -2416,6 +2450,278 @@ contract assertion, and `docs/adr/0003-contextual-orchestrator-vendored-free-zdr "today" reference. Landed in the same PR (`#1463`) as the streaming revert, not split out, since the revert is unsafe without it. +## 2026-09-01 strix_quick_gate.sh: pipeline-scoped artifact presence let a hollow rc=0 attempt ride on an earlier attempt's evidence + +**Context**: `#1495` fixed `strix_quick_gate.sh`'s "hollow path" bug -- Strix exiting `0` (success) with +zero `vulnerabilities/*.md` report artifacts anywhere was previously treated as a clean scan. The fix, +`has_any_strix_vulnerability_report_artifact()`, required at least one artifact to exist anywhere +under `ACTIVE_REPORTS_DIR` (excluding only directories that predated the whole gate run), used by both +`run_strix_once()`'s own `rc==0` acceptance and `has_only_below_threshold_vulnerabilities()`'s guard. + +**Devin Review caught a real gap in that fix, on `#1495`'s successor `#1563`**: the guard was +*pipeline*-scoped, not *attempt*-scoped. `ACTIVE_REPORTS_DIR` intentionally accumulates report +directories across same-model transient retries and cross-model fallback attempts (for audit and +vulnerability-blocking). So a genuinely hollow attempt -- one whose own Strix invocation exited `0` +and wrote *nothing* -- could still pass, because an *earlier*, already-superseded attempt (same model, +retried after a transient error, or a different model tried before it) had left a report artifact +sitting in that same accumulated directory. A "successful" attempt validated entirely by a stale +predecessor's leftover evidence is exactly as hollow as the original zero-artifact bug: it proves +nothing about what *this* attempt's own scan actually did. + +**Fix**: added `capture_attempt_start_vulnerability_files()`, called at the top of every +`run_strix_once()` invocation (immediately before it launches Strix), snapshotting every +`vulnerabilities/*.md` path already present at that moment -- including artifacts an earlier attempt +within the same gate run already wrote. `has_new_strix_vulnerability_report_artifact()` replaces the +old pipeline-wide check for both call sites: it only accepts an artifact that is *not* in that +snapshot, i.e. one this specific attempt itself contributed. The old +`has_any_strix_vulnerability_report_artifact()` is deleted (no longer needed by either caller). +Deliberately **not** attempt-scoped: `has_only_below_threshold_vulnerabilities()`'s severity-scanning +loop, which still walks every non-preexisting report cumulatively -- a blocking (HIGH/CRITICAL) +finding from an earlier attempt must never be silently dropped just because a later attempt didn't +reproduce it. Provider-failure fail-closed behavior and finding thresholds are unchanged. + +**Regression**: `retry-hollow-second-attempt-fails-closed` in `test_strix_quick_gate.sh` -- attempt one +(same model) writes a genuine below-threshold report then fails with a transient rate-limit error +(retried); attempt two exits `0` with no new artifact. Before the fix this passed (validated by attempt +one's leftover report); after the fix it fails closed with the same "produced no report artifacts" +message. Exercising this also surfaced a second, harness-only bug: the shared fake-`strix` stub's +"backstop" `EXIT` trap (which writes a default report on any untested `rc==0` success path) reused the +*same file path* when reusing an existing run directory (deliberate, to avoid shadowing +`latest_strix_report_dir()`'s mtime selection with a brand-new "latest" directory) -- overwriting that +path is invisible to path-keyed attempt-scoped tracking. Four pre-existing GitHub-Models-fallback +scenarios (`github-models-fallback-provider-signal-tries-next` and its siblings) broke under the new +production behavior until the trap was updated to pick an unused path (`vuln-0002.md`, etc.) within the +reused directory when its default `vuln-0001.md` is already attempt-preexisting. Full suite: pytest +2246 passed / 1 skipped / 21 subtests (repository-wide 99% coverage shortfall is the pre-existing, +unrelated gap independently owned by `#1567`); `test_strix_quick_gate.sh` full harness: PASS. + +**Round 2 -- Devin Review caught a deeper, pre-existing gap in the same fix, still on `#1563`**: the +attempt-scoped `has_new_strix_vulnerability_report_artifact()` above still required *some* +`vulnerabilities/*.md` file to exist for an attempt to count as successful -- but that requirement was +never actually satisfiable by a genuinely clean scan. Verified empirically against the pinned +`strix-agent==1.5.3` package source (`report/writer.py`, `report/state.py`, `core/paths.py`): +`write_vulnerabilities()` writes one Markdown file per entry in `ReportState.vulnerability_reports` and +is only called when that list is non-empty; a scan that finds *zero* vulnerabilities never calls it and +so never writes a `vulnerabilities/` directory at all. What `ReportState._save_artifacts()` *always* +writes on completion, finding count aside, is `findings.sarif` (explicitly "even empty, so a clean run +overwrites a prior findings.sarif") and `run.json` via `write_run_record()`, with +`run_record["status"]` set to `"completed"` by `save_run_data(mark_complete=True)`. This means both the +original `#1495` fix and the round-1 attempt-scoping refinement above would fail-closed on *every* +clean, zero-finding scan -- the exact false-positive failure mode "hollow success" detection exists to +avoid, just triggered by a passing scan instead of a hollow one. This flaw predates round 1; it shipped +with `#1495` and was only surfaced now. + +**Fix**: replaced the artifact-presence contract outright. `capture_attempt_start_vulnerability_files()` +/ `is_attempt_start_vulnerability_file()` / `has_new_strix_vulnerability_report_artifact()` are removed. +`capture_attempt_start_run_records()` (called at the same point in `run_strix_once()`, immediately +before `set -o pipefail`) snapshots every `run_dir/run.json` path under `$STRIX_REPORTS_DIR` present at +attempt start. `has_new_completed_strix_run()` replaces the old check at both call sites (the +`run_strix_once()` `rc==0` acceptance and `has_only_below_threshold_vulnerabilities()`'s presence +guard): it walks non-preexisting report directories for a `run.json` that is *not* in the attempt-start +snapshot and whose contents match `"status"\s*:\s*"completed"` (plain `grep`, no new `jq` dependency -- +none was otherwise used in this script). Attempt-scoping from round 1 is preserved exactly, just +re-keyed to the artifact that is actually always written. `has_only_below_threshold_vulnerabilities()`'s +severity-scanning loop is unchanged and stays cumulative/pipeline-wide over `vulnerabilities/*.md`: a +real HIGH/CRITICAL finding from an earlier attempt still blocks regardless of whether a later attempt's +own scan reproduced it. Finding thresholds and provider-failure fail-closed behavior are unchanged. + +**Regression**: new scenario `success-clean-scan-zero-findings` in `test_strix_quick_gate.sh` models a +clean scan directly -- a `run.json` with `"status": "completed"` and no `vulnerabilities/` directory at +all -- and asserts the gate accepts it (`exit=0`), proving a genuinely clean scan no longer fails +closed. `retry-hollow-second-attempt-fails-closed` was re-modeled to match the new contract: attempt one +now writes both a below-threshold `vulnerabilities/*.md` report and its own completed `run.json` before +failing with a transient rate-limit error; attempt two exits `0` with no new `run.json` of its own and +still fails closed with the same "no report artifact" message, proving attempt-scoping survived the +contract switch. The shared fake-`strix` stub's backstop `EXIT` trap (both the simple per-scenario +copies and the shared signal-aware trap) now independently tracks and writes both evidence kinds +(`wrote_vuln` / `wrote_run_record` flags), with a preexisting-run-record snapshot mirroring the +production `ATTEMPT_START_RUN_RECORDS` scoping; a reused run directory whose `run.json` is already +attempt-preexisting routes the backstop's own run-record write to a fresh fallback directory rather than +overwriting it (no `vuln-NNNN.md`-style incrementing filename convention applies to `run.json`). Full +suite: pytest 2246 passed / 1 skipped / 21 subtests, repository-wide coverage 99% (same pre-existing gap +owned by `#1567`, unaffected by this change); `test_strix_quick_gate.sh` full harness: PASS. + +**Round 3 -- the repo owner directly confirmed two round-2 review findings as valid and blocking on +`#1563`**, requiring both a production fix and a full test-harness redesign before merge-readiness. + +**Finding 1 (production regression, self-discovered via CI red after round 2 shipped)**: +`has_only_below_threshold_vulnerabilities()`'s presence guard had been pointed at the new run.json-based +`has_new_completed_strix_run()` alongside `run_strix_once()`'s own rc=0 check, but that guard answers a +narrower question than rc=0 acceptance does -- "is there genuine severity evidence to trust from the +attempt that just concluded, even if that attempt's own process later exited non-zero" (e.g. a real +below-threshold INFO finding written just before a mid-scan `ConnectionError`). A real Strix invocation +that crashes after writing partial findings but before its final `_save_artifacts()` completion pass may +never record `status: "completed"` at all, so requiring it broke every such partial-crash-with-real- +findings scenario (`below-threshold-with-connection-error-no-provider` and three siblings failed on +`#1563`'s own required check). Restored `has_new_strix_vulnerability_report_artifact()` (round 1's +`vulnerabilities/*.md`-based, attempt-scoped check) for this call site specifically; `run_strix_once()`'s +own rc=0 acceptance keeps using run.json-based completion, since that is the one path that actually needs +proof of a genuinely completed (possibly zero-finding) scan. + +**Finding 2 (owner-confirmed, from Devin's informational round-2 findings)**: the owner directed that two +previously-informational (🔍) Devin findings be treated as blocking: (a) `has_new_completed_strix_run()` +matched `"completed"` via a plain regex over the raw run.json bytes and tracked attempt-start state by +path only; (b) the test harness's implicit `trap ... EXIT` backstop mechanism manufactured evidence for +unrelated success scenarios, hiding which branches had real vs. manufactured evidence. + +**Fix (a)**: rewrote `has_new_completed_strix_run()` to shell out to `python3` for structural JSON +parsing -- rejects non-JSON, non-object JSON, symlinks, and completion text that only appears nested in +some other field rather than the top-level `"status"` key -- and switched attempt identity from path-only +membership to SHA-256 content-digest comparison (`ATTEMPT_START_RUN_RECORD_DIGESTS`, still keyed by path +but compared by content), so a run directory reused in place with genuinely new results counts as new +evidence while an unchanged predecessor record does not. + +**Fix (b)**: replaced every `trap strix_fake_backstop_vuln_report_on_success EXIT` registration (the one +shared signal-aware copy plus 11 duplicated ~50-line per-heredoc copies) with an explicit, deliberately- +called helper (`strix_fake_emit_default_success_evidence()` in the shared case-statement; a small local +helper or an inline write in each of the 11 standalone fake-`strix` scripts) invoked immediately before +`exit 0` by every scenario that wants generic default evidence for an unremarkable successful scan. 76 +call sites needed the explicit call added across the ~170-scenario shared case-statement; scenarios that +want no evidence or genuinely custom evidence (`success-zero-report-artifacts`, +`retry-hollow-second-attempt-fails-closed`, `success-clean-scan-zero-findings`) simply do not call it, +which is now the unremarkable case rather than a tracked exception (no opt-out list needed). This also +removed the need to track real signal delivery for the `sleep`-based timeout scenarios: a plain sequential +call made only on the path that actually reaches `exit 0` cannot run if the process is killed by SIGTERM +first, unlike a trap that fires unconditionally on any process exit. + +**New regressions for fix (a)**: `run-record-in-place-rewrite-counts-as-new-evidence` (an attempt reuses +the same run.json path with genuinely different content after a prior attempt's transient failure -- gate +accepts it); `unchanged-run-record-rewrite-fails-closed` (the exact positive scenario's mirror -- an +attempt rewrites the same path with byte-identical content -- gate still fails closed, proving digest +equality, not mere path reuse, governs acceptance); `forged-nested-completed-status-fails-closed` (a +run.json whose top-level `status` is not `"completed"` but which contains that literal substring nested +under an unrelated field -- gate fails closed, proving structural parsing beats substring matching); +`malformed-run-record-fails-closed` (a run.json that is not valid JSON at all -- gate fails closed +gracefully end-to-end, not just in the isolated python snippet). + +**Validation**: independently re-run (not just the implementing agent's own report) -- full +`test_strix_quick_gate.sh` harness: PASS; full pytest suite and coverage confirmed clean with the same +pre-existing 99% repository-wide gap owned by `#1567`, unaffected by this change. + +**Round 4 -- Devin Review found a real gap in round 3's own fix, still on `#1563`**: round 3 restored +`has_new_strix_vulnerability_report_artifact()` (attempt-scoped `vulnerabilities/*.md` presence) as +`has_only_below_threshold_vulnerabilities()`'s guard specifically so a nonzero-exit crash's genuine +partial findings are not lost. Devin correctly pointed out that guard is *too* permissive in one +narrower case it was never meant to cover: an **rc=0** attempt that `run_strix_once()` itself already +determined was hollow (no completed run record) can still be rescued by this same guard if that hollow +attempt happened to also write a below-threshold report before failing to record completion. That is +exactly the class of false-green this gate exists to prevent -- a "successful" scan accepted on +incomplete evidence -- just reached through the below-threshold path instead of the direct rc=0 +acceptance path in `run_strix_once()`. + +**Fix**: added a sticky `STRIX_HOLLOW_SUCCESS_DETECTED` flag, set inside `run_strix_once()`'s existing +rc=0-but-not-completed branch, reset once per `run_current_target_scan()` invocation alongside the +existing `INFRA_ERROR_DETECTED`/`ZERO_FINDINGS_REPORTED` sticky flags (same scope: it must survive +across the primary attempt and every fallback-model attempt within one target's scan, since +`has_only_below_threshold_vulnerabilities()`'s severity scan is itself cumulative across all of them). +`has_only_below_threshold_vulnerabilities()` now checks this flag immediately after its existing +artifact-presence check and fails closed with a dedicated message, mirroring the existing +`INFRA_ERROR_DETECTED` guard directly below it in the same function. + +**Regression**: new scenario `hollow-success-with-below-threshold-report-fails-closed` in +`test_strix_quick_gate.sh` -- a fake Strix invocation exits `0`, writes a genuine below-threshold +(INFO) `vulnerabilities/*.md` report, and deliberately never writes a `run.json`. Before the fix this +passed the gate (`exit 0`) via the below-threshold bypass; after the fix it fails closed with the new +`STRIX_HOLLOW_SUCCESS_DETECTED` message, distinct from `has_new_strix_vulnerability_report_artifact()`'s +own "no report artifact" message (this scenario deliberately has one). + +**Validation**: `STRIX_TEST_CASE_FILTER=hollow-success-with-below-threshold-report-fails-closed bash +scripts/ci/test_strix_quick_gate.sh` -- PASS (exit 0); full `test_strix_quick_gate.sh` harness -- PASS; +full pytest suite -- 2268 passed, 1 skipped, 21 subtests; `coverage run -m pytest tests && coverage +report` -- 100% on `scripts/ci`; `interrogate` -- 100% docstrings. + +**Round 5 -- Devin Review found round 4's fix guarded only one of two alternate success paths, still +on `#1563`**: `has_only_below_threshold_vulnerabilities()` and `run_strix_once()`'s own rc=0 acceptance +were both correctly gated on `STRIX_HOLLOW_SUCCESS_DETECTED`, but `run_current_target_scan()` has a +*third* path to success -- `evaluate_pull_request_findings()`, called at both the primary and +fallback-model call sites once the below-threshold guard has already failed. That function can +independently set `PR_FINDINGS_DECISION=allow_baseline` (an at-or-above-threshold finding confined to +files this PR does not change) and let the caller return success; it has no visibility into completion +evidence at all, since it answers a different question ("is this finding in scope for this PR") +entirely orthogonal to "did this attempt genuinely complete." A hollow rc=0 attempt whose report happens +to contain such a finding could therefore still be rescued via this second, unguarded path. + +**Fix**: gated the `return 0` branch immediately following each `evaluate_pull_request_findings()` call +(primary and fallback) on `STRIX_HOLLOW_SUCCESS_DETECTED` too, with an explicit fail-closed `return 1` +right after (rather than letting a hollow, baseline-allowed attempt fall through into the unrelated +`case "$PR_FINDINGS_DECISION"` / `fail_unmapped_threshold_report()` / fallback-model logic below and +hoping it happens to fail there too). `evaluate_pull_request_findings()` itself is still always called +unconditionally at both sites -- skipping it when hollow would leave `PR_FINDINGS_DECISION` stale from +whatever last set it, which that downstream logic depends on being freshly computed for the current +attempt. + +Implementing this exposed a second, deeper bug in round 4's own scoping: `STRIX_HOLLOW_SUCCESS_DETECTED` +was reset once per `run_current_target_scan()` call, matching the *cumulative* `INFRA_ERROR_DETECTED`/ +`ZERO_FINDINGS_REPORTED` flags (correct for them, since `has_only_below_threshold_vulnerabilities()`'s +own severity scan is itself cumulative across every attempt). But hollow-success is not a cumulative +property of the whole scan -- it is a property of one specific attempt. With the once-per-scan reset, a +hollow *primary* attempt would leave the flag set to `1` for the rest of the scan, wrongly blocking a +*fallback* model's own genuinely completed attempt from ever succeeding via either alternate path, even +though that fallback attempt itself did nothing wrong. Moved the reset to the top of every +`run_strix_once()` invocation (alongside `capture_attempt_start_vulnerability_files()`/ +`capture_attempt_start_run_records()`), so by the time `run_current_target_scan()` reads it after +`run_strix_with_transient_retry()` returns, it reflects only the most-recently-concluded individual +attempt -- consistent with how the existing attempt-scoped evidence snapshots already behave (each +`run_strix_once()` call only recognizes evidence written since its own start, not an earlier retry's). + +**Regression**: new scenario `hollow-success-with-baseline-unchanged-report-fails-closed` -- a fake Strix +invocation exits `0`, writes a CRITICAL-severity finding whose location is a file this PR does not +change (the sibling `pr-baseline-critical-unchanged` scenario models the legitimate nonzero-exit-crash +version of the identical report), and never writes `run.json`. Before the fix this passed the gate via +`evaluate_pull_request_findings()`'s baseline-allow path; after the fix it fails closed with the new +message. + +**Validation**: `STRIX_TEST_CASE_FILTER=hollow-success-with-baseline-unchanged-report-fails-closed bash +scripts/ci/test_strix_quick_gate.sh` -- PASS (exit 0); full `test_strix_quick_gate.sh` harness -- PASS +(also re-confirms round 4's `hollow-success-with-below-threshold-report-fails-closed` and the unrelated +`pr-baseline-critical-unchanged`/`retry-hollow-second-attempt-fails-closed`/`success-zero-report-artifacts` +scenarios still pass under the rescoped per-attempt flag); full pytest suite -- 2301 passed, 1 skipped, +21 subtests; `coverage run -m pytest tests && coverage report` -- 100% on `scripts/ci`; `interrogate` -- +100% docstrings. + +**Round 6 -- Devin Review found round 5's own fail-closed return was itself over-broad, still on +`#1563`**: the explicit `if [ "$STRIX_HOLLOW_SUCCESS_DETECTED" -eq 1 ]; then ... return 1; fi` added +immediately after gating the `evaluate_pull_request_findings()` success branch (both the primary and +fallback-model call sites) correctly stopped a hollow attempt from being rescued by either alternate +success path -- but it ALSO unconditionally short-circuited execution before it could ever reach the +existing, unrelated `case "$PR_FINDINGS_DECISION"` / `fail_unmapped_threshold_report()` / +`is_model_retryable_error()` / fallback-model-loop logic further down in `run_current_target_scan()`. +That logic is what decides whether a FAILED attempt (of any kind, hollow or otherwise) is eligible to +retry with a distinct fallback model. Since `STRIX_HOLLOW_SUCCESS_DETECTED` had just been correctly +rescoped to be attempt-scoped (round 5), a genuinely completed fallback attempt cannot be tainted by an +earlier hollow primary's flag value -- so blocking the fallback path entirely for any hollow primary was +unnecessary and regressive: a healthy, distinct fallback model could no longer recover the required +security check for a hollow-but-otherwise-retryable primary failure. + +**Fix**: removed the blanket `return 1` at both call sites, keeping only the two success-path gates +already added in round 5 (`... && [ "$STRIX_HOLLOW_SUCCESS_DETECTED" -ne 1 ]` on each +`evaluate_pull_request_findings()` branch). A hollow attempt that is not rescued by either alternate +success path now falls through to exactly the same downstream logic every other failed attempt already +goes through, unchanged -- including `is_model_retryable_error()`'s own gate on whether a fallback model +is even attempted, and the fallback loop itself, whose own `has_only_below_threshold_vulnerabilities()`/ +`evaluate_pull_request_findings()` calls are independently guarded by the SAME (attempt-scoped) flag, so +a hollow fallback attempt cannot rescue itself either -- only a genuinely non-hollow one can. + +**Regression**: new scenario `hollow-primary-recovers-via-completed-fallback` -- the primary model's fake +Strix invocation exits `0`, writes no `run.json` (hollow), and its log carries a `strix.ModelBehaviorError` +line (retryable per `is_model_retryable_error()`, and deliberately NOT a rate-limit/timeout marker, since +those are infrastructure-error signals `run_strix_once()` itself already fails closed on earlier, before +ever reaching the hollow-run.json check -- a different, already-covered code path); a configured distinct +fallback model then exits `0` with a genuinely completed `run.json` and must succeed. Also updated the +existing `hollow-success-with-baseline-unchanged-report-fails-closed` scenario's expected message: with +the blanket return removed, that scenario (no fallback model configured) now falls through to +`is_model_retryable_error()`'s own "non-recoverable error" message instead of the round-5-specific one, +which no longer exists as a distinct code path -- the scenario's exit code and fail-closed outcome are +unchanged, only which existing message reports it. + +**Validation**: `STRIX_TEST_CASE_FILTER=hollow-primary-recovers-via-completed-fallback bash +scripts/ci/test_strix_quick_gate.sh` -- PASS (exit 0); re-ran +`hollow-success-with-baseline-unchanged-report-fails-closed`, +`hollow-success-with-below-threshold-report-fails-closed`, +`retry-hollow-second-attempt-fails-closed`, and `success-zero-report-artifacts` individually -- all PASS; +full `test_strix_quick_gate.sh` harness -- PASS; full pytest suite -- 2268 passed, 1 skipped, 21 +subtests; `coverage run -m pytest tests && coverage report` -- 100% on `scripts/ci`; `interrogate` -- +100% docstrings. ## 2026-09-01 post-#1546 `scripts/ci` coverage regression on protected main: root-caused and closed **Context**: `#1546` (merged, exact head `5686de41660d51a7a7f22b8840dfa6ccfe5ff3f1`) reconciled diff --git a/scripts/ci/strix_quick_gate.sh b/scripts/ci/strix_quick_gate.sh old mode 100755 new mode 100644 index c08f2fa36c..a6d009a485 --- a/scripts/ci/strix_quick_gate.sh +++ b/scripts/ci/strix_quick_gate.sh @@ -64,6 +64,9 @@ RUN_START_EPOCH=0 TOTAL_TIMEOUT_EXCEEDED=0 ATTEMPT_LOG_SEQUENCE=0 PREEXISTING_REPORT_DIRS=() +ATTEMPT_START_VULNERABILITY_FILES=() +declare -gA ATTEMPT_START_VULNERABILITY_DIGESTS=() +declare -gA ATTEMPT_START_RUN_RECORD_DIGESTS=() REPO_NAME="${REPO_ROOT##*/}" # shellcheck source=scripts/ci/strix_model_utils.sh # shellcheck disable=SC1091 # source path is repo-local; local lint may omit -x @@ -72,8 +75,29 @@ REPO_NAME="${REPO_ROOT##*/}" # LLM connection failure, mid-stream fallback, etc.), this flag stays 1 for # the rest of the run. It prevents the "all findings below threshold" bypass # from masking scan incompleteness — a successful strix run (exit 0) ignores -# this flag because the scan itself produced a complete result set. +# this flag because the scan itself produced a complete result set, *unless* +# STRIX_HOLLOW_SUCCESS_DETECTED below says otherwise. INFRA_ERROR_DETECTED=0 +# Per-attempt flag (reset at the top of every run_strix_once() call, unlike +# the cumulative INFRA_ERROR_DETECTED/ZERO_FINDINGS_REPORTED flags above and +# below): set when that specific attempt's own Strix invocation exited 0 +# (claimed success) but has_new_completed_strix_run() found no genuinely new +# completed run.json for it (run_strix_once()'s own hollow-success +# fail-closed branch). A rc=0 attempt can still leave behind a genuine +# below-threshold vulnerabilities/*.md report or an at-threshold finding +# confined to unchanged PR files -- has_new_strix_vulnerability_report_artifact() +# and evaluate_pull_request_findings() are deliberately not completion-scoped, +# since they also have to accept a nonzero-exit crash's partial-but-real +# findings (see has_new_strix_vulnerability_report_artifact()'s own +# docstring). Without this flag, has_only_below_threshold_vulnerabilities() +# and evaluate_pull_request_findings() cannot tell those cases apart from an +# rc=0 attempt Strix itself already declared incomplete, and would let their +# alternate-success paths rescue exactly the hollow-success case +# run_strix_once() just failed closed on (Devin review on `#1563`, rounds 4 +# and 5). Reset per attempt, not per run_current_target_scan() call: a +# fallback model's own genuinely completed attempt must not be judged hollow +# just because an earlier attempt in the same scan was. +STRIX_HOLLOW_SUCCESS_DETECTED=0 ZERO_FINDINGS_REPORTED=0 PR_FINDINGS_DECISION="not_applicable" CHANGED_FILES=() @@ -185,16 +209,6 @@ known_internal_warning = re.compile( r"produced non-lifecycle final output in non-interactive mode" r"|ended a turn without a lifecycle tool call \(interactive=False\)" r"); forcing tool continuation \(\d+/\d+\): " - # strix-agent 1.5.3 strix/core/execution.py:763 logs this only inside its - # bounded transient-retry branch, immediately before the replay runs, so - # the line means "a retry is happening now", not "the scan failed". An - # exhausted retry logs `agent run failed for …; marking failed` at ERROR - # with a traceback and exits non-zero; neither of those is touched here. - # Anchored before the exception repr on purpose: the same class appears - # as InternalServerError today and as a different type after a gateway - # pin advance. Re-verify the message format on every strix-agent bump. - r"|transient model/provider error for [0-9a-f]+; replaying turn " - r"\(attempt \d+/\d+, backoff [0-9.]+s\): " r")" ) known_scanner_warning = re.compile( @@ -238,6 +252,121 @@ PY done } +strix_report_has_authoritative_recovered_transient_completion() { + local report_root="$1" + local extra_log="${2:-}" + local run_record="$report_root/run.json" + local digest + + if [ ! -d "$report_root" ] || [ -L "$report_root" ]; then + return 1 + fi + + # A recovered in-process provider turn is not terminal infrastructure + # failure when (and only when) the same current attempt produced Strix's + # complete structured terminal evidence. Keep the raw warning in the + # published artifact; this validator changes classification, not evidence. + digest="$(python3 - "$report_root" "$extra_log" <<'PY' +from pathlib import Path +import hashlib +import json +import os +import re +import sys + +root = Path(sys.argv[1]) +extra_log = Path(sys.argv[2]) if len(sys.argv) > 2 and sys.argv[2] else None +run_record = root / "run.json" +sarif_record = root / "findings.sarif" + + +def read_regular(path: Path) -> bytes: + if path.is_symlink() or not path.is_file(): + raise SystemExit(1) + try: + return path.read_bytes() + except OSError: + raise SystemExit(1) + + +try: + run_bytes = read_regular(run_record) + run_data = json.loads(run_bytes) + sarif_data = json.loads(read_regular(sarif_record)) +except (ValueError, UnicodeDecodeError): + raise SystemExit(1) + +scan_results = run_data.get("scan_results") if isinstance(run_data, dict) else None +if ( + not isinstance(run_data, dict) + or run_data.get("status") != "completed" + or not isinstance(scan_results, dict) + or scan_results.get("scan_completed") is not True + or scan_results.get("success") is not True +): + raise SystemExit(1) + +if not isinstance(sarif_data, dict) or sarif_data.get("version") != "2.1.0": + raise SystemExit(1) +sarif_runs = sarif_data.get("runs") +if not isinstance(sarif_runs, list) or not sarif_runs: + raise SystemExit(1) +for sarif_run in sarif_runs: + if not isinstance(sarif_run, dict) or not isinstance(sarif_run.get("results"), list): + raise SystemExit(1) + +signal = re.compile( + r"(?i)(?= int(match.group(2)): + raise SystemExit(1) + saw_recovered = True + +if not saw_recovered: + raise SystemExit(1) +print(hashlib.sha256(run_bytes).hexdigest()) +PY +)" || return 1 + + if [ -z "$digest" ] || [ "${ATTEMPT_START_RUN_RECORD_DIGESTS[$run_record]:-}" = "$digest" ]; then + return 1 + fi + return 0 +} + has_strix_report_failure_signal() { local report_root local report_log @@ -257,6 +386,9 @@ has_strix_report_failure_signal() { fi report_root="$newest_report_root" fi + if strix_report_has_authoritative_recovered_transient_completion "$report_root" "$STRIX_LOG"; then + continue + fi while IFS= read -r -d '' report_log; do if grep -Eiq '(^|[^[:alpha:]])(Fatal|Denied|Warn|Warning|WARNING|Timeout)([^[:alpha:]]|$)' "$report_log"; then return 0 @@ -2697,6 +2829,15 @@ run_strix_once() { child_llm_api_key="$STRIX_OPENROUTER_FALLBACK_KEY" fi fi + capture_attempt_start_vulnerability_files + capture_attempt_start_run_records + # Reset per attempt, not per run_current_target_scan() call (unlike the + # cumulative INFRA_ERROR_DETECTED/ZERO_FINDINGS_REPORTED flags): a run + # reused for a fallback model, or a later transient retry of the same + # model, must not still be judged hollow because an *earlier* attempt in + # this scan was, once this specific attempt goes on to write its own + # genuine completed run record. + STRIX_HOLLOW_SUCCESS_DETECTED=0 set -o pipefail set +e STRIX_CHILD_MODEL="$child_model" \ @@ -2955,6 +3096,11 @@ PY fi if [ "$rc" -eq 0 ]; then + if ! has_new_completed_strix_run; then + STRIX_HOLLOW_SUCCESS_DETECTED=1 + echo "Strix exited successfully but produced no report artifacts; log-only success is incomplete evidence, so the scan is failing closed." >&2 + return 1 + fi if has_blocking_vulnerability_reports; then if ! evaluate_pull_request_findings || [ "$PR_FINDINGS_DECISION" != "allow_baseline" ]; then echo "Strix exited successfully but emitted a vulnerability at or above '$STRIX_FAIL_ON_MIN_SEVERITY'; failing closed." >&2 @@ -3404,8 +3550,37 @@ is_llm_token_limit_error() { # errors (timeout, rate-limit, transport failures) that indicate the scan # was interrupted or incomplete. Used as a guard to prevent the # below-threshold override from silently passing an aborted scan. +has_strix_console_failure_signal() { + # STRIX_LOG is the captured process console, not a typed scanner receipt. + # It can contain the rendered report itself, where words such as "denied" + # describe application policy rather than provider state. Warning and fatal + # text remain conservatively broad; only the ambiguous denied token requires + # a control-record shape. Report *.log files remain subject to the broader + # fail-closed classifier in has_strix_report_failure_signal(). + if grep -Eiq '(^|[^[:alpha:]])(Fatal|Warn|Warning)([^[:alpha:]]|$)' "$STRIX_LOG"; then + return 0 + fi + + if grep -Eiq '^[[:space:]]*Denied:([[:space:]]|$)' "$STRIX_LOG"; then + return 0 + fi + + if grep -Eiq '(^|[[:space:]])::error::' "$STRIX_LOG"; then + return 0 + fi + + return 1 +} + has_detected_infrastructure_error() { - if grep -Eiq '(^|[^[:alpha:]])(Fatal|Denied|Warn|Warning)([^[:alpha:]]|$)' "$STRIX_LOG"; then + local newest_report_root="" + newest_report_root="$(latest_strix_report_dir 2>/dev/null || true)" + if [ -n "$newest_report_root" ] && + strix_report_has_authoritative_recovered_transient_completion "$newest_report_root" "$STRIX_LOG"; then + return 1 + fi + + if has_strix_console_failure_signal; then return 0 fi @@ -3484,11 +3659,280 @@ latest_strix_report_dir() { echo "$latest" } +# Compute a stable content digest for a vulnerability report candidate, or +# print nothing if it is not a readable regular, non-symlink file. The path +# alone is not attempt provenance: Strix can reuse its latest run directory +# and rewrite the same vulnerabilities/*.md file in place. +strix_vulnerability_report_digest() { + python3 - "$1" <<'PY' +import hashlib +import os +import sys + +path = sys.argv[1] +if os.path.islink(path) or not os.path.isfile(path): + raise SystemExit(0) +try: + with open(path, "rb") as handle: + data = handle.read() +except OSError: + raise SystemExit(0) +print(hashlib.sha256(data).hexdigest()) +PY +} + +# Snapshot every vulnerabilities/*.md path and content digest already present +# under STRIX_REPORTS_DIR, regardless of preexisting-directory status. Called at +# the top of run_strix_once() before each individual attempt launches +# Strix, so the snapshot always reflects exactly what existed before *this* +# attempt -- including artifacts an earlier attempt within the same gate run +# already wrote. ACTIVE_REPORTS_DIR deliberately accumulates those artifacts +# across retries and fallback models for audit and vulnerability-blocking +# purposes (see has_only_below_threshold_vulnerabilities, which intentionally +# still considers that cumulative evidence). +capture_attempt_start_vulnerability_files() { + ATTEMPT_START_VULNERABILITY_FILES=() + ATTEMPT_START_VULNERABILITY_DIGESTS=() + local run_dir vulnerabilities_dir vuln_file digest + for run_dir in "$STRIX_REPORTS_DIR"/*; do + if [ ! -d "$run_dir" ] || [ -L "$run_dir" ]; then + continue + fi + + vulnerabilities_dir="$run_dir/vulnerabilities" + if [ ! -d "$vulnerabilities_dir" ] || [ -L "$vulnerabilities_dir" ]; then + continue + fi + + for vuln_file in "$vulnerabilities_dir"/*.md; do + if [ ! -f "$vuln_file" ] || [ -L "$vuln_file" ]; then + continue + fi + digest="$(strix_vulnerability_report_digest "$vuln_file")" + if [ -n "$digest" ]; then + ATTEMPT_START_VULNERABILITY_FILES+=("$vuln_file") + ATTEMPT_START_VULNERABILITY_DIGESTS["$vuln_file"]="$digest" + fi + done + done +} + +is_attempt_start_vulnerability_file() { + local candidate="$1" + local existing + + for existing in "${ATTEMPT_START_VULNERABILITY_FILES[@]}"; do + if [ "$candidate" = "$existing" ]; then + return 0 + fi + done + + return 1 +} + +# Return success (0) only when the most recent Strix invocation produced at +# least one vulnerabilities/*.md artifact whose path is new or whose content +# digest differs from the pre-attempt snapshot (per +# capture_attempt_start_vulnerability_files(), called at the top of every +# run_strix_once() attempt). A "successful" rc=0 Strix invocation that wrote +# nothing new must not be validated by a leftover report an earlier, +# already-superseded attempt or model left behind, while an in-place rewrite +# by the current attempt remains genuine evidence. Used only +# by has_only_below_threshold_vulnerabilities()'s presence guard, which asks +# a narrower question than run_strix_once()'s own rc=0 acceptance: "is there +# genuine severity evidence to trust from the attempt that just concluded, +# even if that attempt's own process exited non-zero" (e.g. a below-threshold +# INFO finding written before a mid-scan ConnectionError). run.json +# completion status is the wrong evidence contract here -- a real Strix +# invocation that crashes after writing partial findings but before its +# final _save_artifacts() completion pass may never record status +# "completed" at all, yet the findings it did write are still genuine +# evidence, not hollow (round 3 of the same review restored this after round +# 2 mistakenly pointed this call site at has_new_completed_strix_run() too, +# which made every such partial-crash-with-real-findings scenario fail +# closed alongside the genuinely hollow ones it was meant to catch). +has_new_strix_vulnerability_report_artifact() { + local run_dir vulnerabilities_dir vuln_file digest + for run_dir in "$STRIX_REPORTS_DIR"/*; do + if [ ! -d "$run_dir" ] || [ -L "$run_dir" ]; then + continue + fi + + if is_preexisting_report_dir "$run_dir"; then + continue + fi + + vulnerabilities_dir="$run_dir/vulnerabilities" + if [ ! -d "$vulnerabilities_dir" ] || [ -L "$vulnerabilities_dir" ]; then + continue + fi + + for vuln_file in "$vulnerabilities_dir"/*.md; do + if [ ! -f "$vuln_file" ] || [ -L "$vuln_file" ]; then + continue + fi + if is_attempt_start_vulnerability_file "$vuln_file"; then + digest="$(strix_vulnerability_report_digest "$vuln_file")" + if [ -z "$digest" ] || [ "${ATTEMPT_START_VULNERABILITY_DIGESTS[$vuln_file]:-}" = "$digest" ]; then + continue + fi + fi + return 0 + done + done + + return 1 +} + +# Compute a stable content digest for a run.json candidate path, or print +# nothing if it is not a readable regular, non-symlink file. Shared by +# capture_attempt_start_run_records() (pre-attempt snapshot) and +# has_new_completed_strix_run() (post-attempt comparison) so both sides +# agree on exactly what "unchanged" means. +strix_run_record_digest() { + python3 - "$1" <<'PY' +import hashlib +import os +import sys + +path = sys.argv[1] +if os.path.islink(path) or not os.path.isfile(path): + raise SystemExit(0) +try: + with open(path, "rb") as handle: + data = handle.read() +except OSError: + raise SystemExit(0) +print(hashlib.sha256(data).hexdigest()) +PY +} + +# Snapshot a content digest for every run.json already present under +# STRIX_REPORTS_DIR, regardless of preexisting-directory status, keyed by +# path. Called at the top of run_strix_once() before each individual +# attempt launches Strix, so ATTEMPT_START_RUN_RECORD_DIGESTS always +# reflects exactly what existed before *this* attempt -- including run +# records an earlier attempt within the same gate run already wrote, which +# ACTIVE_REPORTS_DIR deliberately accumulates across retries and fallback +# models for audit and vulnerability-blocking purposes (see +# has_only_below_threshold_vulnerabilities, whose severity scan +# intentionally still considers that cumulative evidence). Digests, not +# just paths, are captured so an in-place rewrite of the same run.json path +# (a fresh attempt reusing an existing "latest" run directory, mirroring +# production's own latest_strix_report_dir() mtime selection) still counts +# as new evidence: an unchanged predecessor record must not, but a +# genuinely rewritten one must (Devin review on `#1495`'s successor +# `#1563`, round 3). +capture_attempt_start_run_records() { + ATTEMPT_START_RUN_RECORD_DIGESTS=() + local run_dir run_record digest + for run_dir in "$STRIX_REPORTS_DIR"/*; do + if [ ! -d "$run_dir" ] || [ -L "$run_dir" ]; then + continue + fi + + run_record="$run_dir/run.json" + digest="$(strix_run_record_digest "$run_record")" + if [ -n "$digest" ]; then + ATTEMPT_START_RUN_RECORD_DIGESTS["$run_record"]="$digest" + fi + done +} + +# Structurally validate one run.json candidate as an authoritative, +# genuinely completed Strix run record: a regular, non-symlink file whose +# content parses as a JSON object with a top-level "status" key equal to +# the exact string "completed" (rejecting malformed/non-object JSON and +# completion text that only appears nested in some other field, or as +# text elsewhere in the file, rather than as that top-level key -- a naive +# substring/regex match over the raw file content cannot tell those apart +# from a genuinely forged or unrelated occurrence of the same text). Prints +# the run record's own content digest on stdout when it validates, so the +# caller can compare it against the pre-attempt snapshot without a second +# read of the file (Devin review on `#1495`'s successor `#1563`, round 3). +strix_run_record_is_completed() { + python3 - "$1" <<'PY' +import hashlib +import json +import os +import sys + +path = sys.argv[1] +if os.path.islink(path) or not os.path.isfile(path): + raise SystemExit(1) +try: + with open(path, "rb") as handle: + data = handle.read() +except OSError: + raise SystemExit(1) +try: + parsed = json.loads(data) +except (ValueError, UnicodeDecodeError): + raise SystemExit(1) +if not isinstance(parsed, dict): + raise SystemExit(1) +if parsed.get("status") != "completed": + raise SystemExit(1) +print(hashlib.sha256(data).hexdigest()) +PY +} + +# Return success (0) only when the most recent Strix invocation produced a +# structurally valid run.json (Strix's own always-written run record, +# regardless of finding count -- strix-agent's ReportState._save_artifacts +# calls write_run_record and write_sarif() unconditionally on every save, +# while write_vulnerabilities() runs only "if self.vulnerability_reports") +# whose top-level status is exactly "completed", and whose content digest +# differs from (or whose path did not exist in) the pre-attempt snapshot +# from capture_attempt_start_run_records(). vulnerabilities/*.md is the +# wrong evidence contract for "this attempt genuinely completed": a real, +# clean scan with zero findings never writes one at all, so requiring it +# made every clean scan fail exactly like the hollow-success bug it was +# meant to catch (Devin review on `#1495`'s successor `#1563`, round 2). A +# "successful" rc=0 Strix invocation that produced no new completed run +# record of its own must not be validated by a leftover, unchanged one an +# earlier, already-superseded attempt or model left behind (round 1 of the +# same review) -- that is exactly as hollow as producing no evidence at +# all, and content-digest comparison (round 3) closes the narrower gap +# where a same-path reused run directory is genuinely rewritten with new +# results: path identity alone cannot tell that apart from an untouched +# predecessor record. Used only for run_strix_once()'s own rc=0 acceptance; +# has_only_below_threshold_vulnerabilities()'s presence guard uses +# has_new_strix_vulnerability_report_artifact() instead (see that +# function's own docstring for why). Severity scanning below that guard +# deliberately stays cumulative across every accumulated, non-preexisting +# vulnerabilities/*.md report: a blocking finding from an earlier attempt +# must never be silently missed just because a later attempt did not +# reproduce it. +has_new_completed_strix_run() { + local run_dir run_record digest + for run_dir in "$STRIX_REPORTS_DIR"/*; do + if [ ! -d "$run_dir" ] || [ -L "$run_dir" ]; then + continue + fi + + if is_preexisting_report_dir "$run_dir"; then + continue + fi + + run_record="$run_dir/run.json" + digest="$(strix_run_record_is_completed "$run_record")" + if [ -z "$digest" ]; then + continue + fi + if [ "${ATTEMPT_START_RUN_RECORD_DIGESTS[$run_record]:-}" = "$digest" ]; then + continue + fi + return 0 + done + + return 1 +} + has_only_below_threshold_vulnerabilities() { local threshold_rank threshold_rank="$(severity_rank "$STRIX_FAIL_ON_MIN_SEVERITY")" - local found_any_vuln_file=0 local global_max_rank=-1 STRIX_MAX_SEVERITY_RANK=-1 local saw_any_severity=0 @@ -3518,6 +3962,42 @@ has_only_below_threshold_vulnerabilities() { done < <(grep -Ei 'severity[[:space:]]*:' "$source_path" || true) } + # Presence is attempt-scoped (did the just-concluded, terminal attempt for + # this model contribute genuine new evidence -- not a leftover report an + # earlier, already-superseded attempt left behind, per Devin review on + # `#1495`'s successor `#1563`), but severity scanning below stays + # cumulative across every accumulated, non-preexisting report: a + # blocking finding from an earlier attempt must never be silently missed + # just because a later attempt did not reproduce it. This guard checks + # for a genuine vulnerabilities/*.md artifact, not run.json completion + # status (has_new_completed_strix_run(), used only by run_strix_once()'s + # own rc=0 acceptance): an attempt whose process later crashed non-zero + # (e.g. a mid-scan ConnectionError) may have written real below-threshold + # findings before crashing without ever reaching a "completed" run + # record, and that partial evidence is still genuine, not hollow (round + # 3 of the same review restored this after round 2 briefly pointed this + # guard at completion status too, which incorrectly failed closed on + # every such partial-crash-with-real-findings case). + if ! has_new_strix_vulnerability_report_artifact; then + echo "No Strix vulnerability report artifact was produced; log-only severity markers are incomplete evidence, so the scan is failing closed." >&2 + return 1 + fi + + # Guard against an rc=0 attempt Strix's own run_strix_once() already + # determined was hollow (exited 0 but never wrote a genuinely new + # completed run.json). That attempt can still leave behind a real + # below-threshold vulnerabilities/*.md report (the presence check just + # above intentionally accepts that, since it also has to accept a + # nonzero-exit crash's partial-but-real findings) -- but a "successful" + # exit with no completion evidence is exactly the hollow-success bug + # class this gate exists to fail closed on, and must not be rescued by + # the below-threshold bypass just because it happened to write a + # low-severity report before failing to record completion. + if [ "$STRIX_HOLLOW_SUCCESS_DETECTED" -eq 1 ]; then + echo "Below-threshold findings detected, but an rc=0 attempt produced no completed run record; refusing bypass due to incomplete success evidence." >&2 + return 1 + fi + local run_dir for run_dir in "$STRIX_REPORTS_DIR"/*; do if [ ! -d "$run_dir" ] || [ -L "$run_dir" ]; then @@ -3540,16 +4020,10 @@ has_only_below_threshold_vulnerabilities() { continue fi - found_any_vuln_file=1 update_max_severity_from_stream "$vuln_file" done done - if [ "$found_any_vuln_file" -eq 0 ]; then - echo "No Strix vulnerability report artifact was produced; log-only severity markers are incomplete evidence, so the scan is failing closed." >&2 - return 1 - fi - if [ "$saw_any_severity" -eq 0 ]; then return 1 fi @@ -4361,6 +4835,7 @@ is_model_retryable_error() { run_current_target_scan() { INFRA_ERROR_DETECTED=0 + STRIX_HOLLOW_SUCCESS_DETECTED=0 ZERO_FINDINGS_REPORTED=0 local primary_scan_rc=0 @@ -4407,11 +4882,34 @@ run_current_target_scan() { return 0 fi + # evaluate_pull_request_findings() is always called (not short-circuited + # on STRIX_HOLLOW_SUCCESS_DETECTED) so it still freshly computes + # PR_FINDINGS_DECISION for this attempt -- the case statement and + # fail_unmapped_threshold_report() below depend on that, and skipping + # the call would leave a stale decision from whatever last set it. + # STRIX_HOLLOW_SUCCESS_DETECTED must gate every alternate success path + # below has_only_below_threshold_vulnerabilities(), not just that one: + # evaluate_pull_request_findings() can independently set + # PR_FINDINGS_DECISION=allow_baseline (an at-or-above-threshold finding + # confined to unchanged PR files) and let the caller return success, and + # it has no visibility into the completion-evidence question at all + # (Devin Review on `#1563`). if evaluate_pull_request_findings; then - if [ "$strict_primary_provider_fallback" -eq 0 ]; then + if [ "$strict_primary_provider_fallback" -eq 0 ] && [ "$STRIX_HOLLOW_SUCCESS_DETECTED" -ne 1 ]; then return 0 fi fi + # Deliberately no unconditional "return 1" here for + # STRIX_HOLLOW_SUCCESS_DETECTED: that would also block the unrelated, + # legitimate fallback-to-a-distinct-model path below when this + # attempt's own failure looks retryable, even though the flag is now + # attempt-scoped so a genuinely completed fallback attempt cannot be + # tainted by this hollow one (Devin Review on `#1563`, round 6). The + # flag only needs to gate the two alternate SUCCESS paths above and at + # the fallback call site below; a hollow attempt that isn't rescued by + # either one already falls through to the same + # case/fail_unmapped_threshold_report/retryability logic every other + # failed attempt does. case "$PR_FINDINGS_DECISION" in block_changed | block_unmapped | block_manifest_unverified) @@ -4489,10 +4987,14 @@ run_current_target_scan() { fi if evaluate_pull_request_findings; then - if [ "$strict_fallback_provider_signal" -eq 0 ]; then + if [ "$strict_fallback_provider_signal" -eq 0 ] && [ "$STRIX_HOLLOW_SUCCESS_DETECTED" -ne 1 ]; then return 0 fi fi + # See the matching comment at the primary call site above: no + # unconditional "return 1" here either, so a hollow fallback + # attempt can itself still fall through to a further distinct + # fallback model when retryable. case "$PR_FINDINGS_DECISION" in block_changed | block_unmapped | block_manifest_unverified) diff --git a/scripts/ci/test_strix_quick_gate.sh b/scripts/ci/test_strix_quick_gate.sh old mode 100755 new mode 100644 index b9b1c43de3..5e58bed648 --- a/scripts/ci/test_strix_quick_gate.sh +++ b/scripts/ci/test_strix_quick_gate.sh @@ -356,6 +356,7 @@ assert_strix_workflow_pr_trigger_hardened() { assert_file_contains "$GATE_SCRIPT" "os.walk(root, topdown=True, followlinks=False)" "strix gate does not recurse into symlinked report directories" assert_file_not_contains "$GATE_SCRIPT" 'root.rglob("*.log")' "strix gate avoids recursive pathlib glob traversal for report logs" assert_file_contains "$GATE_SCRIPT" "has_strix_report_failure_signal" "strix gate fails closed on warning-class Strix report artifacts" + assert_file_contains "$GATE_SCRIPT" "has_strix_console_failure_signal" "strix gate distinguishes control-shaped console failures from rendered report prose" assert_file_not_contains "$workflow_file" "ignore::UserWarning" "strix workflow must not blanket-suppress all UserWarning output" assert_file_contains "$GATE_SCRIPT" "vulnerability_file_reports_generic_github_actions_workflow_insecurity" "strix gate fact-checks generic GitHub Actions workflow security reports before accepting whole-file claims" assert_file_not_contains "$workflow_file" "vertex_ai/* | vertex_ai_beta/*" "strix workflow must not accept arbitrary Vertex models" @@ -3352,9 +3353,420 @@ printf '%s\n' "$target_path" >> "${FAKE_STRIX_TARGET_LOG:?}" STRIX_REPORTS_DIR="${STRIX_REPORTS_DIR:-strix_runs}" +# This stub has dozens of independent "scan succeeded" exit points scattered +# across the case branches below. Earlier revisions hand-waved default +# evidence for all of them via a single blanket `trap ... EXIT` handler that +# fired no matter which branch (or bare fallthrough) produced the zero exit +# status, and had to carry its own opt-out list for the handful of scenarios +# that deliberately want no (or different) evidence. That made it hard to +# tell, for any given branch, whether its evidence was real or manufactured +# by an implicit handler running behind its back (Devin review on `#1495`'s +# successor `#1563`, round 4). +# +# Replaced with an explicit, deliberately-called helper, +# strix_fake_emit_default_success_evidence() below: every branch that wants +# generic default evidence for an unremarkable successful scan calls it +# itself, immediately before its own `exit 0`. Nothing is automatic anymore, +# so nothing needs an opt-out list -- a branch that wants no evidence (e.g. +# success-zero-report-artifacts), only partial evidence, or genuinely custom +# evidence (e.g. success-clean-scan-zero-findings, retry-hollow-second-attempt- +# fails-closed) simply does not call it, which is now the natural, +# unremarkable case rather than a special exception. This also removes the +# need to track real signal delivery for the `sleep`-based timeout scenarios +# above (they are killed with SIGTERM before their own trailing `exit 0` is +# ever meant to run): a plain sequential call made only on the path that +# actually reaches `exit 0` cannot run if the process is killed first, unlike +# a trap that fires unconditionally on any process exit. +# +# Production's own success-evidence guard is attempt-scoped (a "success" +# rc=0 Strix invocation must not be validated by a leftover run record an +# earlier, already-superseded attempt or model left behind -- Devin review +# on `#1495`'s successor `#1563`, round 1) and keys off run.json's +# "completed" status (Strix's own always-written run record) rather than +# vulnerabilities/*.md (only ever written when there are findings -- a real +# clean scan makes it hollow-fail-closed too, round 2 of the same review). +# The helper below must match: it snapshots which vulnerabilities/*.md and +# run.json paths already existed before this specific invocation started (a +# fresh process per attempt, so a plain array survives for its whole +# lifetime) and only treats each as already covered when a path *not* in +# that snapshot exists -- i.e. this attempt (or an earlier one reused via the +# same latest-directory selection just below) itself contributed genuine +# evidence, not merely inherited it. +strix_fake_preexisting_vuln_files=() +for strix_fake_preexisting_run_dir in "$STRIX_REPORTS_DIR"/*/vulnerabilities; do + if [ ! -d "$strix_fake_preexisting_run_dir" ]; then + continue + fi + for strix_fake_preexisting_vuln_file in "$strix_fake_preexisting_run_dir"/*.md; do + if [ -f "$strix_fake_preexisting_vuln_file" ]; then + strix_fake_preexisting_vuln_files+=("$strix_fake_preexisting_vuln_file") + fi + done +done +strix_fake_is_preexisting_vuln_file() { + local candidate="$1" + local existing + for existing in "${strix_fake_preexisting_vuln_files[@]}"; do + if [ "$candidate" = "$existing" ]; then + return 0 + fi + done + return 1 +} +strix_fake_preexisting_run_records=() +for strix_fake_preexisting_run_dir in "$STRIX_REPORTS_DIR"/*; do + if [ -f "$strix_fake_preexisting_run_dir/run.json" ]; then + strix_fake_preexisting_run_records+=("$strix_fake_preexisting_run_dir/run.json") + fi +done +strix_fake_is_preexisting_run_record() { + local candidate="$1" + local existing + for existing in "${strix_fake_preexisting_run_records[@]}"; do + if [ "$candidate" = "$existing" ]; then + return 0 + fi + done + return 1 +} +# Explicit, deliberately-invoked helper: emits generic INFO-severity +# vulnerability-report and/or "completed" run.json evidence for a +# fake-Strix scenario that models an unremarkable successful scan, filling +# in only whichever piece (if either) the calling branch has not already +# written for itself -- idempotent and safe to call unconditionally from a +# success branch, since a branch that already wrote valid new evidence of +# one or both kinds leaves this a no-op for that kind. Call it explicitly, +# immediately before `exit 0`, from any case branch below that wants this +# default evidence; a branch that wants no evidence or genuinely custom +# evidence simply does not call it (Devin review on `#1495`'s successor +# `#1563`, round 4). +strix_fake_emit_default_success_evidence() { + local run_dir vuln_file wrote_vuln=0 wrote_run_record=0 + for run_dir in "$STRIX_REPORTS_DIR"/*/vulnerabilities; do + if [ ! -d "$run_dir" ]; then + continue + fi + for vuln_file in "$run_dir"/*.md; do + if [ -f "$vuln_file" ] && ! strix_fake_is_preexisting_vuln_file "$vuln_file"; then + wrote_vuln=1 + fi + done + done + for run_dir in "$STRIX_REPORTS_DIR"/*; do + if [ -f "$run_dir/run.json" ] && ! strix_fake_is_preexisting_run_record "$run_dir/run.json"; then + wrote_run_record=1 + fi + done + if [ "$wrote_vuln" -eq 1 ] && [ "$wrote_run_record" -eq 1 ]; then + return + fi + # Reuse the existing *latest* run directory (e.g. one holding only a + # strix.log), mirroring production's own latest_strix_report_dir() + # mtime selection, instead of creating a brand-new sibling directory -- + # a new directory would itself become "latest" and shadow whichever run + # directory other detection logic (e.g. has_strix_report_failure_signal) + # actually depends on inspecting. + local target_run_dir="" + for run_dir in "$STRIX_REPORTS_DIR"/*; do + if [ -d "$run_dir" ] && [ ! -L "$run_dir" ]; then + if [ -z "$target_run_dir" ] || [ "$run_dir" -nt "$target_run_dir" ]; then + target_run_dir="$run_dir" + fi + fi + done + if [ -z "$target_run_dir" ]; then + target_run_dir="$STRIX_REPORTS_DIR/fake-success-backstop" + fi + if [ "$wrote_vuln" -eq 0 ]; then + mkdir -p "$target_run_dir/vulnerabilities" + # A reused directory (the common case -- see above) can already hold + # a vuln-0001.md from an earlier attempt; overwriting that same path + # would not register as new evidence under production's + # attempt-scoped tracking (keyed on path, not content or mtime), so + # pick a path that is not already in this attempt's preexisting + # snapshot. + local backstop_index=1 + local backstop_file="$target_run_dir/vulnerabilities/vuln-0001.md" + while strix_fake_is_preexisting_vuln_file "$backstop_file"; do + backstop_index=$((backstop_index + 1)) + backstop_file="$target_run_dir/vulnerabilities/vuln-$(printf '%04d' "$backstop_index").md" + done + cat >"$backstop_file" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT + fi + if [ "$wrote_run_record" -eq 0 ]; then + mkdir -p "$target_run_dir" + # run.json has no severity-ordered filename convention to bump like + # vuln-NNNN.md -- a reused directory's run.json is always the same + # single path, so an already-preexisting one can only be superseded + # by overwriting it in place. Attempt-scoped tracking is keyed on + # path, not content, so overwriting a preexisting path here would be + # invisible to production the same way a reused vuln-0001.md was; + # route to a fresh directory instead whenever the reused one's + # run.json is already attempt-preexisting. + if [ -f "$target_run_dir/run.json" ] && strix_fake_is_preexisting_run_record "$target_run_dir/run.json"; then + target_run_dir="$STRIX_REPORTS_DIR/fake-success-backstop-run-record" + mkdir -p "$target_run_dir" + fi + cat >"$target_run_dir/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD + fi +} + case "${FAKE_STRIX_SCENARIO:?}" in success|runtime-env-forwarding|custom-openai-compatible-preserves-effort|vertex-primary-success-timing-message|direct-openai-gpt-does-not-require-github-models-api-base|pr-executable-integrity-mismatch|pr-executable-group-writable) + mkdir -p "$STRIX_REPORTS_DIR/fake-success/vulnerabilities" + cat >"$STRIX_REPORTS_DIR/fake-success/vulnerabilities/vuln-0001.md" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT echo "scan ok" + strix_fake_emit_default_success_evidence + exit 0 + ;; + success-zero-report-artifacts) + # Deliberately mirrors the historical "hollow path" bug: Strix exits + # 0 (a clean process exit) but writes no run.json run record + # anywhere under STRIX_REPORTS_DIR. This is the regression case for + # has_new_completed_strix_run()'s fail-closed guard in + # run_strix_once(); deliberately never calls + # strix_fake_emit_default_success_evidence. + echo "scan ok with zero report artifacts" + exit 0 + ;; + hollow-success-with-below-threshold-report-fails-closed) + # Devin review on #1563: an rc=0 attempt can write a genuine + # below-threshold (INFO) vulnerabilities/*.md report and STILL never + # write a completed run.json (e.g. a bug between the two writes, or + # a wrapper that reports success despite an incomplete + # _save_artifacts() pass). Before the STRIX_HOLLOW_SUCCESS_DETECTED + # guard, has_only_below_threshold_vulnerabilities() could not tell + # this apart from a genuine nonzero-exit crash's partial-but-real + # findings and would rescue it -- exactly the hollow-success bug + # class this gate exists to fail closed on. Deliberately never + # calls strix_fake_emit_default_success_evidence (no run.json). + mkdir -p "$STRIX_REPORTS_DIR/fake-hollow-below-threshold/vulnerabilities" + cat >"$STRIX_REPORTS_DIR/fake-hollow-below-threshold/vulnerabilities/vuln-0001.md" <<'EOS' +Severity: INFO +EOS + echo "scan ok with a below-threshold report but no completed run record" + exit 0 + ;; + success-clean-scan-zero-findings) + # Regression for Devin's review on `#1495`'s successor `#1563`, + # round 2: the pinned strix-agent only writes vulnerabilities/*.md + # when ReportState.vulnerability_reports is non-empty -- a genuinely + # clean scan with zero findings never writes one at all, only its + # always-written run.json (status "completed") and findings.sarif. + # Before this fix, requiring a vulnerabilities/*.md artifact made + # every clean scan fail exactly like the hollow-success bug it was + # meant to catch. This stub models that real shape directly (no + # vulnerabilities/ directory at all) rather than calling + # strix_fake_emit_default_success_evidence, so it fails loudly if a + # future change reintroduces a vulnerabilities/*.md requirement. + mkdir -p "$STRIX_REPORTS_DIR/fake-clean-scan" + cat >"$STRIX_REPORTS_DIR/fake-clean-scan/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD + echo "scan ok with zero findings" + exit 0 + ;; + recovered-transient-warning-completed-clean-scan) + # Inkspan #402 / run 33927906573 / job 101234352982: Strix + # retried a transient HTTP 500 inside the current scan, then emitted an + # authoritative completed run record and empty SARIF. The raw warning is + # retained for audit, but must not be confused with an exhausted provider + # failure when the same attempt's structured terminal evidence is clean. + mkdir -p "$STRIX_REPORTS_DIR/fake-recovered-transient" + cat >"$STRIX_REPORTS_DIR/fake-recovered-transient/strix.log" <<'EOS' +2026-09-05 10:03:16.171 WARNING strix-pr-scope-hctgst_8456 - strix.core.execution: transient model/provider error for c0ffee12; replaying turn (attempt 1/5, backoff 2.0s): InternalServerError(Error code: 500 - temporary gateway failure) +EOS + cat >"$STRIX_REPORTS_DIR/fake-recovered-transient/run.json" <<'RUNRECORD' +{"status":"completed","scan_results":{"scan_completed":true,"success":true}} +RUNRECORD + cat >"$STRIX_REPORTS_DIR/fake-recovered-transient/findings.sarif" <<'SARIF' +{"$schema":"https://json.schemastore.org/sarif-2.1.0.json","version":"2.1.0","runs":[{"tool":{"driver":{"name":"Strix"}},"results":[]}]} +SARIF + echo "scan recovered from a transient provider turn and completed with zero findings" + exit 0 + ;; + completed-clean-scan-with-denied-report-prose) + # OriginWeave #166 / run 33929688857 / job 101237371800: + # the final current attempt completed successfully with empty SARIF, but + # scanner-rendered report prose in the captured console contained ordinary + # security-language uses of "denied". Those sentences are not provider or + # infrastructure receipts and must not override the same attempt's clean + # structured terminal evidence. + mkdir -p "$STRIX_REPORTS_DIR/fake-denied-report-prose" + cat >"$STRIX_REPORTS_DIR/fake-denied-report-prose/strix.log" <<'EOS' +2026-09-05 10:53:29.000 INFO strix-pr-scope-originweave - strix.scan: completed scan with 0 vulnerability report(s) +EOS + cat >"$STRIX_REPORTS_DIR/fake-denied-report-prose/run.json" <<'RUNRECORD' +{"status":"completed","scan_results":{"scan_completed":true,"success":true}} +RUNRECORD + cat >"$STRIX_REPORTS_DIR/fake-denied-report-prose/findings.sarif" <<'SARIF' +{"$schema":"https://json.schemastore.org/sarif-2.1.0.json","version":"2.1.0","runs":[{"tool":{"driver":{"name":"Strix"}},"results":[]}]} +SARIF + echo "The forbidden R5 class is hard-denied first." + echo "Cross-origin mutations are denied outright." + exit 0 + ;; + recovered-transient-warning-exhausted-fails-closed) + mkdir -p "$STRIX_REPORTS_DIR/fake-recovered-transient-exhausted" + cat >"$STRIX_REPORTS_DIR/fake-recovered-transient-exhausted/strix.log" <<'EOS' +2026-09-05 10:03:16.171 WARNING strix-pr-scope-hctgst_8456 - strix.core.execution: transient model/provider error for c0ffee12; replaying turn (attempt 5/5, backoff 2.0s): InternalServerError(Error code: 500 - exhausted gateway retries) +EOS + cat >"$STRIX_REPORTS_DIR/fake-recovered-transient-exhausted/run.json" <<'RUNRECORD' +{"status":"completed","scan_results":{"scan_completed":true,"success":true}} +RUNRECORD + cat >"$STRIX_REPORTS_DIR/fake-recovered-transient-exhausted/findings.sarif" <<'SARIF' +{"version":"2.1.0","runs":[{"tool":{"driver":{"name":"Strix"}},"results":[]}]} +SARIF + exit 0 + ;; + recovered-transient-warning-malformed-terminal-evidence-fails-closed) + mkdir -p "$STRIX_REPORTS_DIR/fake-recovered-transient-malformed" + cat >"$STRIX_REPORTS_DIR/fake-recovered-transient-malformed/strix.log" <<'EOS' +2026-09-05 10:03:16.171 WARNING strix-pr-scope-hctgst_8456 - strix.core.execution: transient model/provider error for c0ffee12; replaying turn (attempt 1/5, backoff 2.0s): InternalServerError(Error code: 500 - temporary gateway failure) +EOS + cat >"$STRIX_REPORTS_DIR/fake-recovered-transient-malformed/run.json" <<'RUNRECORD' +{"status":"completed","scan_results":{"scan_completed":true}} +RUNRECORD + cat >"$STRIX_REPORTS_DIR/fake-recovered-transient-malformed/findings.sarif" <<'SARIF' +{"version":"2.1.0","runs":[{"tool":{"driver":{"name":"Strix"}},"results":[]}]} +SARIF + exit 0 + ;; + run-record-in-place-rewrite-counts-as-new-evidence) + # Regression for Devin's review on `#1495`'s successor `#1563`, + # round 3: has_new_completed_strix_run() compares run.json CONTENT + # digests, not just path identity, when deciding whether an + # attempt produced new evidence. Attempt one writes a completed + # run.json to a fixed path and then the wrapping process still + # exits non-zero (a transient rate-limit signal after real work + # was already done, so run_strix_with_transient_retry retries the + # same model); attempt two rewrites the SAME path with genuinely + # different content (a distinguishable second completion) and + # exits 0. The gate must accept it -- an in-place rewrite of an + # already-existing run.json path is still new evidence when its + # content actually changed, mirroring production's own + # latest_strix_report_dir() mtime-based directory reuse (a fresh + # attempt reusing an existing "latest" run directory). This is the + # positive mirror of unchanged-run-record-rewrite-fails-closed + # below. + case "${STRIX_LLM:-}" in + vertex_ai/rewrite-retry-primary) + attempt="0" + if [ -f "${FAKE_STRIX_STATE_FILE:?}" ]; then + attempt="$(cat "${FAKE_STRIX_STATE_FILE:?}")" + fi + attempt="$((attempt + 1))" + echo "$attempt" > "${FAKE_STRIX_STATE_FILE:?}" + mkdir -p "$STRIX_REPORTS_DIR/rewrite-retry" + if [ "$attempt" -eq 1 ]; then + cat >"$STRIX_REPORTS_DIR/rewrite-retry/run.json" <<'RUNRECORD' +{"status": "completed", "attempt": "first"} +RUNRECORD + echo "Penetration test failed: LLM request failed: RateLimitError" + exit 1 + fi + cat >"$STRIX_REPORTS_DIR/rewrite-retry/run.json" <<'RUNRECORD' +{"status": "completed", "attempt": "second"} +RUNRECORD + echo "scan ok after in-place run record rewrite" + exit 0 + ;; + vertex_ai/fallback-one) + echo "Error: fallback should not be needed for in-place run record rewrite scenario" >&2 + exit 31 + ;; + *) + echo "Error: in-place run record rewrite path unexpected (${STRIX_LLM:-})" >&2 + exit 31 + ;; + esac + ;; + unchanged-run-record-rewrite-fails-closed) + # Regression for Devin's review on `#1495`'s successor `#1563`, + # round 3: mirrors retry-hollow-second-attempt-fails-closed's + # general shape but specifically proves content-identical reuse + # does not count as new evidence -- not merely "attempt two + # touched nothing" (which retry-hollow-second-attempt-fails-closed + # already covers) but "attempt two actively rewrote the exact same + # path with byte-identical content" (e.g. because it re-selected + # the same latest run directory and reasserted the same + # completion, mirroring the in-place-rewrite scenario above except + # the rewritten bytes are unchanged). has_new_completed_strix_run()'s + # digest comparison must still reject it: the gate fails closed + # overall, proving digest equality -- not whether the path was + # merely written to again -- is what governs acceptance. + case "${STRIX_LLM:-}" in + vertex_ai/unchanged-rewrite-primary) + attempt="0" + if [ -f "${FAKE_STRIX_STATE_FILE:?}" ]; then + attempt="$(cat "${FAKE_STRIX_STATE_FILE:?}")" + fi + attempt="$((attempt + 1))" + echo "$attempt" > "${FAKE_STRIX_STATE_FILE:?}" + mkdir -p "$STRIX_REPORTS_DIR/unchanged-rewrite" + cat >"$STRIX_REPORTS_DIR/unchanged-rewrite/run.json" <<'RUNRECORD' +{"status": "completed", "attempt": "identical"} +RUNRECORD + if [ "$attempt" -eq 1 ]; then + echo "Penetration test failed: LLM request failed: RateLimitError" + exit 1 + fi + echo "scan ok with zero new report artifacts on identical rewrite" + exit 0 + ;; + vertex_ai/fallback-one) + echo "Error: fallback should not be needed for unchanged run record rewrite scenario" >&2 + exit 31 + ;; + *) + echo "Error: unchanged run record rewrite path unexpected (${STRIX_LLM:-})" >&2 + exit 31 + ;; + esac + ;; + forged-nested-completed-status-fails-closed) + # Regression for Devin's review on `#1495`'s successor `#1563`, + # round 3: proves strix_run_record_is_completed() parses run.json + # structurally rather than matching the raw text -- a run.json + # whose top-level "status" key is NOT "completed", but which + # happens to contain the literal substring `"status": "completed"` + # nested under some other field (a forged or unrelated occurrence + # of the same text), must still fail closed exactly like a + # genuinely absent or incomplete run record. A naive + # substring/regex match over the raw file content cannot tell this + # apart from a genuine top-level completion. + mkdir -p "$STRIX_REPORTS_DIR/forged-nested-status" + cat >"$STRIX_REPORTS_DIR/forged-nested-status/run.json" <<'RUNRECORD' +{"status": "running", "child_process": {"status": "completed"}} +RUNRECORD + echo "scan ok but run record status is forged" + exit 0 + ;; + malformed-run-record-fails-closed) + # Regression for Devin's review on `#1495`'s successor `#1563`, + # round 3: proves strix_run_record_is_completed() and + # has_new_completed_strix_run() reject a run.json that is not + # valid JSON at all -- gracefully, via json.JSONDecodeError, not by + # crashing the gate script -- exactly like a genuinely absent + # completion record. Proves the *gate script* handles this + # end-to-end, not just the python snippet in isolation. + mkdir -p "$STRIX_REPORTS_DIR/malformed-run-record" + cat >"$STRIX_REPORTS_DIR/malformed-run-record/run.json" <<'RUNRECORD' +{"status": "completed", this is not valid json +RUNRECORD + echo "scan ok but run record is malformed" exit 0 ;; contextual-orchestrator-gateway-model-qualification) @@ -3367,6 +3779,7 @@ success|runtime-env-forwarding|custom-openai-compatible-preserves-effort|vertex- exit 11 fi echo "scan ok through contextual-orchestrator gateway" + strix_fake_emit_default_success_evidence exit 0 ;; scan-working-directory-isolated) @@ -3379,6 +3792,7 @@ success|runtime-env-forwarding|custom-openai-compatible-preserves-effort|vertex- exit 82 fi echo "scan ok with isolated Strix working directory" + strix_fake_emit_default_success_evidence exit 0 ;; success-with-critical-report) @@ -3390,15 +3804,18 @@ success|runtime-env-forwarding|custom-openai-compatible-preserves-effort|vertex- - Title: Successful process still emitted a blocking vulnerability REPORT echo "Vulnerabilities 1" + strix_fake_emit_default_success_evidence exit 0 ;; slow-timeout) sleep "${FAKE_STRIX_TIMEOUT_SLEEP_SECONDS:?}" + strix_fake_emit_default_success_evidence exit 0 ;; timeout-disabled-success) sleep 1 echo "scan ok with timeout disabled" + strix_fake_emit_default_success_evidence exit 0 ;; vertex-primary-notfound-fallback-success|github-models-fallback-success|github-models-fallback-success-deepseek-v3|github-models-token-limit-fallback-success|github-models-fallback-requires-api-base|github-models-model-prefix-with-api-base-succeeds|github-models-meta-prefix-with-api-base-succeeds|github-models-mistral-prefix-with-api-base-succeeds) @@ -3410,6 +3827,7 @@ REPORT ;; vertex_ai/fallback-one) echo "scan ok with fallback" + strix_fake_emit_default_success_evidence exit 0 ;; openai/gpt-5|openai/openai/gpt-5.4|openai/meta/test-github-model|openai/mistral-ai/test-github-model) @@ -3418,6 +3836,7 @@ REPORT exit 1 fi echo "scan ok with GitHub Models fallback" + strix_fake_emit_default_success_evidence exit 0 ;; openai/deepseek/deepseek-r1-0528) @@ -3428,10 +3847,12 @@ REPORT exit 1 fi echo "scan ok with GitHub Models fallback" + strix_fake_emit_default_success_evidence exit 0 ;; openai/deepseek/deepseek-v3-0324) echo "scan ok with GitHub Models fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3461,6 +3882,7 @@ REPORT exit 27 fi echo "scan ok after direct-OpenAI fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3486,6 +3908,7 @@ REPORT exit 16 fi echo "scan ok with GitHub Models fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3506,6 +3929,7 @@ REPORT provider-prefix-required) if [ "${STRIX_LLM:-}" = "vertex_ai/gemini-2.5-pro" ]; then echo "scan ok with normalized provider" + strix_fake_emit_default_success_evidence exit 0 fi echo "Error: provider prefix not normalized (${STRIX_LLM:-})" >&2 @@ -3520,6 +3944,7 @@ REPORT ;; vertex_ai/fallback-one) echo "scan ok after fallback normalization" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3531,6 +3956,7 @@ REPORT provider-prefix-required-resource-path-primary-implicit-default-provider | provider-prefix-required-resource-path-primary-explicit-empty-default-provider) if [ "${STRIX_LLM:-}" = "vertex_ai/gemini-2.5-pro" ]; then echo "scan ok with resource-path normalization" + strix_fake_emit_default_success_evidence exit 0 fi echo "Error: resource-path model not normalized (${STRIX_LLM:-})" >&2 @@ -3545,6 +3971,7 @@ REPORT ;; vertex_ai/fallback-one) echo "scan ok after resource-path fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3557,6 +3984,7 @@ REPORT # projects/

/locations//models/ (no publishers/ segment) if [ "${STRIX_LLM:-}" = "vertex_ai/my-custom-model-123" ]; then echo "scan ok with custom model resource-path normalization" + strix_fake_emit_default_success_evidence exit 0 fi echo "Error: custom model resource-path not normalized (${STRIX_LLM:-})" >&2 @@ -3570,6 +3998,7 @@ REPORT ;; vertex_ai/fallback-one) echo "scan ok after status-less not found fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3587,6 +4016,7 @@ REPORT ;; vertex_ai/fallback-one) echo "scan ok after compact-status not found fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3598,6 +4028,7 @@ REPORT nonvertex-slash-model-passthrough) if [ "${STRIX_LLM:-}" = "foo/bar" ]; then echo "scan ok with non-vertex slash model passthrough" + strix_fake_emit_default_success_evidence exit 0 fi echo "Error: non-vertex slash model was rewritten (${STRIX_LLM:-})" >&2 @@ -3612,6 +4043,7 @@ REPORT ;; vertex_ai/fallback-one) echo "scan ok after duplicate-primary skip" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3634,6 +4066,7 @@ REPORT ;; vertex_ai/fallback-two) echo "scan ok after multiline fallback parsing" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3650,6 +4083,7 @@ REPORT ;; vertex_ai/fallback-one) echo "scan ok after rate-limit fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3666,6 +4100,7 @@ REPORT ;; vertex_ai/fallback-one) echo "scan ok after resource exhausted fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3682,6 +4117,7 @@ REPORT ;; openai/fallback-one) echo "scan ok after quota fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3698,6 +4134,7 @@ REPORT ;; vertex_ai/fallback-one) echo "scan ok after 429 fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3714,6 +4151,7 @@ REPORT ;; vertex_ai/fallback-one) echo "scan ok after midstream fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3736,6 +4174,7 @@ REPORT exit 1 fi echo "scan ok after same-model retry" + strix_fake_emit_default_success_evidence exit 0 ;; vertex_ai/fallback-one) @@ -3762,6 +4201,7 @@ REPORT exit 1 fi echo "scan ok after same-model rate-limit retry" + strix_fake_emit_default_success_evidence exit 0 ;; vertex_ai/fallback-one) @@ -3774,6 +4214,53 @@ REPORT ;; esac ;; + retry-hollow-second-attempt-fails-closed) + # Regression for Devin's review on `#1495`'s successor `#1563`: + # has_new_completed_strix_run() must not validate a later hollow + # rc=0 attempt using an earlier, already-superseded attempt's + # leftover run.json. Attempt one genuinely completes (writes both a + # below-threshold vulnerability report and a completed run.json) + # and then the wrapping process itself still exits non-zero (a + # transient rate-limit signal after real work was already done, so + # run_strix_with_transient_retry retries the same model); attempt + # two exits 0 with no new run.json of its own anywhere. The overall + # gate must fail closed, not silently accept attempt one's stale + # completion evidence. + case "${STRIX_LLM:-}" in + vertex_ai/retry-hollow-primary) + attempt="0" + if [ -f "${FAKE_STRIX_STATE_FILE:?}" ]; then + attempt="$(cat "${FAKE_STRIX_STATE_FILE:?}")" + fi + attempt="$((attempt + 1))" + echo "$attempt" > "${FAKE_STRIX_STATE_FILE:?}" + if [ "$attempt" -eq 1 ]; then + mkdir -p "$STRIX_REPORTS_DIR/attempt-one/vulnerabilities" + cat >"$STRIX_REPORTS_DIR/attempt-one/vulnerabilities/vuln-0001.md" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT + cat >"$STRIX_REPORTS_DIR/attempt-one/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD + echo "Penetration test failed: LLM request failed: RateLimitError" + exit 1 + fi + echo "scan ok with zero new report artifacts on retry" + exit 0 + ;; + vertex_ai/fallback-one) + echo "Error: fallback should not be needed for retry-hollow-second-attempt-fails-closed scenario" >&2 + exit 31 + ;; + *) + echo "Error: retry-hollow fallback path unexpected (${STRIX_LLM:-})" >&2 + exit 31 + ;; + esac + ;; vertex-primary-api-connection-retry-same-model-success|github-models-internal-server-connection-retry-same-model-success|internal-server-error-unrelated-output-nonretryable|internal-server-error-many-blocks-retry-same-model-success) case "${STRIX_LLM:-}" in gemini/retry-api-connection-primary|vertex_ai/retry-api-connection-primary|openai/openai/retry-api-connection-primary) @@ -3821,6 +4308,7 @@ REPORT exit 1 fi echo "scan ok after same-model api connection retry" + strix_fake_emit_default_success_evidence exit 0 ;; vertex_ai/fallback-one) @@ -3855,6 +4343,7 @@ REPORT exit 1 fi echo "scan ok after OpenRouter 502 same-model retry" + strix_fake_emit_default_success_evidence exit 0 ;; vertex_ai/fallback-two) @@ -3882,6 +4371,7 @@ REPORT ;; vertex_ai/fallback-two) echo "scan ok after distant target output" + strix_fake_emit_default_success_evidence exit 0 ;; esac @@ -3900,6 +4390,7 @@ REPORT ;; openai/deepseek/deepseek-r1-0528) echo "scan ok after GitHub Models unavailable fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3938,6 +4429,7 @@ REPORT ;; openai/deepseek/deepseek-r1-0528) echo "scan ok after authenticated GitHub Models HTTP 410 retirement" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -3956,6 +4448,7 @@ REPORT ;; openai/deepseek/deepseek-r1-0528) echo "scan ok after GitHub Models rate-limit fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4010,6 +4503,7 @@ EOS exit 1 fi echo "scan ok after second GitHub Models fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4033,6 +4527,7 @@ EOS exit 1 fi echo "scan ok after same-model high-demand retry" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4051,6 +4546,7 @@ EOS ;; nvidia_nim/nvidia/fallback-one) echo "scan ok after NVIDIA overload fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4068,6 +4564,7 @@ EOS ;; gemini/fallback-one) echo "scan ok after timeout fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4085,6 +4582,7 @@ EOS ;; gemini/fallback-one) echo "scan ok after gemini fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4149,6 +4647,7 @@ EOS ;; vertex_ai/fallback-one) echo "scan ok after hallucinated-endpoint fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4175,6 +4674,7 @@ EOS ;; vertex_ai/fallback-one) echo "scan ok after documented OpenCode env apiKey false positive" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4225,6 +4725,7 @@ EOS ;; vertex_ai/fallback-one) echo "scan ok after generic GitHub Actions workflow false positive" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4269,6 +4770,7 @@ EOS ;; vertex_ai/fallback-one) echo "scan ok after stale-source fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4318,6 +4820,7 @@ EOS ;; vertex_ai/fallback-one) echo "scan ok after stale snapshot snippet fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4418,6 +4921,7 @@ EOS ;; vertex_ai/fallback-one) echo "scan ok after excluded-dir hallucination fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4513,6 +5017,7 @@ EOS nonvertex-slash-model-not-rewritten) if [ "${STRIX_LLM:-}" = "deepseek/models/deepseek-r1" ]; then echo "scan ok with deepseek model passthrough" + strix_fake_emit_default_success_evidence exit 0 fi echo "Error: deepseek model was rewritten (${STRIX_LLM:-})" >&2 @@ -4521,6 +5026,7 @@ EOS preserve-existing-api-base) if [ "${LLM_API_BASE:-}" = "https://preexisting.invalid" ]; then echo "scan ok with preserved api base" + strix_fake_emit_default_success_evidence exit 0 fi echo "Error: existing LLM_API_BASE was not preserved (${LLM_API_BASE:-})" >&2 @@ -4535,6 +5041,7 @@ EOS ;; vertex_ai/gemini-2.5-pro) echo "scan ok with default fast fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4551,6 +5058,7 @@ EOS ;; vertex_ai/fallback-one) echo "scan ok after timeout fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4575,6 +5083,7 @@ EOS ;; vertex_ai/fallback-one) echo "scan ok after timeout-exhausted fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4591,6 +5100,7 @@ EOS echo "│ Vulnerabilities 0 │" echo "╰──────────────────────────────────────────────────────────────────────────────╯" sleep "${FAKE_STRIX_TIMEOUT_SLEEP_SECONDS:?}" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4607,10 +5117,12 @@ EOS echo "│ Vulnerabilities 0 │" echo "╰──────────────────────────────────────────────────────────────────────────────╯" sleep "${FAKE_STRIX_TIMEOUT_SLEEP_SECONDS:?}" + strix_fake_emit_default_success_evidence exit 0 ;; vertex_ai/fallback-one) sleep "${FAKE_STRIX_TIMEOUT_SLEEP_SECONDS:?}" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4631,10 +5143,12 @@ EOS echo "│ Vulnerabilities 0 │" echo "╰──────────────────────────────────────────────────────────────────────────────╯" sleep "${FAKE_STRIX_TIMEOUT_SLEEP_SECONDS:?}" + strix_fake_emit_default_success_evidence exit 0 ;; vertex_ai/fallback-one) sleep "${FAKE_STRIX_TIMEOUT_SLEEP_SECONDS:?}" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4645,14 +5159,17 @@ EOS ;; provider-fatal-success-signal) echo "Fatal: provider stream aborted" + strix_fake_emit_default_success_evidence exit 0 ;; provider-warning-success-signal) echo "Warning: provider response included incomplete scan state" + strix_fake_emit_default_success_evidence exit 0 ;; provider-denied-success-signal) echo "Denied: provider credentials were rejected" + strix_fake_emit_default_success_evidence exit 0 ;; provider-report-rate-limit-fallback-success) @@ -4668,6 +5185,7 @@ EOS vertex_ai/fallback-one) mkdir -p "$STRIX_REPORTS_DIR/fake-report-rate-limit-fallback" echo "scan ok after report-only provider fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4696,6 +5214,7 @@ EOS EOS ln -s "$outside_report_dir" "$STRIX_REPORTS_DIR/fake-known-internal-warning/linked-outside" echo "scan ok with sanitized internal Strix report notice" + strix_fake_emit_default_success_evidence exit 0 ;; report-known-internal-warning-variant-sanitized) @@ -4705,6 +5224,7 @@ EOS 2026-06-18 13:10:44.089 INFO strix-pr-scope-example - strix.tools.finish.tool: finish_scan: completed scan with 0 vulnerability report(s) EOS echo "scan ok with sanitized internal Strix report notice variant" + strix_fake_emit_default_success_evidence exit 0 ;; report-unknown-warning-fails) @@ -4713,6 +5233,7 @@ EOS 2026-06-18 13:08:05.986 WARNING strix-pr-scope-example - strix.provider: provider returned incomplete scan state EOS echo "scan ok but unknown report warning remains" + strix_fake_emit_default_success_evidence exit 0 ;; bare-timeout-with-provider-marker) @@ -4730,6 +5251,7 @@ EOS ;; vertex_ai/fallback-one) echo "scan ok after bare-timeout fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4840,6 +5362,7 @@ EOS ;; vertex_ai/fallback-one) echo "scan ok after bare-timeout-exhaust fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4859,6 +5382,7 @@ EOS ;; vertex_ai/fallback-one) echo "scan ok after httpx-timeout fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4885,6 +5409,7 @@ EOS ;; vertex_ai/fallback-one) echo "scan ok after httpcore-timeout fallback" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -4938,6 +5463,55 @@ EOS echo "Penetration test failed: baseline critical finding" exit 1 ;; + hollow-success-with-baseline-unchanged-report-fails-closed) + # Devin review round 5 on #1563: an rc=0 attempt with no completed + # run record can still leave behind an at-or-above-threshold finding + # confined to an unchanged PR file (the sibling + # pr-baseline-critical-unchanged scenario above models the + # legitimate nonzero-exit-crash version of this same report). + # evaluate_pull_request_findings() would classify that as + # PR_FINDINGS_DECISION=allow_baseline and let the caller return + # success -- has_only_below_threshold_vulnerabilities() alone cannot + # catch this, since the finding is at/above threshold, not below it. + # Deliberately never calls strix_fake_emit_default_success_evidence + # (no run.json). + mkdir -p "$STRIX_REPORTS_DIR/fake-hollow-baseline/vulnerabilities" + cat >"$STRIX_REPORTS_DIR/fake-hollow-baseline/vulnerabilities/vuln-0001.md" <<'EOS' +Severity: CRITICAL +Location 1: +sync-module-system/smart-crawling-biz/src/main/java/org/empasy/sync/modules/system/service/impl/SysUserServiceImpl.java:5 +EOS + echo "scan ok with a baseline-unchanged-file report but no completed run record" + exit 0 + ;; + hollow-primary-recovers-via-completed-fallback) + # Devin review round 6 on #1563: STRIX_HOLLOW_SUCCESS_DETECTED must + # not become an unconditional "return 1" after the primary attempt + # -- that would also block the unrelated, legitimate + # fallback-to-a-distinct-model path when the hollow attempt's own + # failure looks retryable (is_model_retryable_error), even though + # the flag is attempt-scoped so a genuinely completed fallback + # cannot be tainted by an earlier hollow primary. Uses + # strix.ModelBehaviorError (retryable per is_model_retryable_error) + # rather than a rate-limit/timeout marker, since those are also + # infrastructure-error signals that run_strix_once() itself already + # fails closed on before ever reaching the hollow-run.json check. + case "${STRIX_LLM:-}" in + vertex_ai/hollow-retryable-primary) + echo "strix.ModelBehaviorError: unexpected tool call shape" + echo "scan ok despite no completed run record" + exit 0 + ;; + vertex_ai/completed-fallback) + mkdir -p "$STRIX_REPORTS_DIR/fake-completed-fallback" + cat >"$STRIX_REPORTS_DIR/fake-completed-fallback/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD + echo "scan ok via completed fallback" + exit 0 + ;; + esac + ;; pr-critical-changed) mkdir -p "$STRIX_REPORTS_DIR/fake-pr-changed/vulnerabilities" cat >"$STRIX_REPORTS_DIR/fake-pr-changed/vulnerabilities/vuln-0001.md" <<'EOS' @@ -5288,6 +5862,7 @@ EOS exit 43 fi echo "scan ok with bounded changed-file scope" + strix_fake_emit_default_success_evidence exit 0 ;; pr-python-scope-context) @@ -5320,6 +5895,7 @@ EOS exit 61 fi echo "scan ok with python dependency scope" + strix_fake_emit_default_success_evidence exit 0 ;; pr-changed-scope-full) @@ -5343,6 +5919,7 @@ EOS exit 46 fi echo "scan ok with full changed-file scope" + strix_fake_emit_default_success_evidence exit 0 fi echo "Error: unexpected full-scope scan attempt $attempt" >&2 @@ -5361,6 +5938,7 @@ EOS [ -f "$target_path/sync-module-system/smart-crawling-biz/src/main/java/org/empasy/sync/modules/system/service/impl/SysUserServiceImpl.java" ] && \ [ -f "$target_path/sync-module-system/smart-crawling-common/src/main/java/org/empasy/sync/common/system/util/JwtUtil.java" ]; then echo "scan ok with full configured PR scope" + strix_fake_emit_default_success_evidence exit 0 fi echo "Error: PR changed-file scope did not include the complete changed-file set on one scan attempt $attempt ($target_path)" >&2 @@ -5368,11 +5946,13 @@ EOS ;; pr-large-scope-full-set) echo "scan ok with large full PR scope" + strix_fake_emit_default_success_evidence exit 0 ;; pr-changed-scope-includes-ci-dependency) if [ -f "$target_path/scripts/ci/strix_quick_gate.sh" ] && [ -f "$target_path/scripts/ci/strix_model_utils.sh" ]; then echo "scan ok with CI support dependency" + strix_fake_emit_default_success_evidence exit 0 fi echo "Error: PR changed-file scope missing CI support dependency ($target_path)" >&2 @@ -5381,6 +5961,7 @@ EOS pr-changed-scope-includes-opencode-normalizer) if [ -f "$target_path/fuzz/fuzz_opencode_review_normalize_output.py" ] && [ -f "$target_path/scripts/ci/opencode_review_normalize_output.py" ]; then echo "scan ok with opencode normalizer support dependency" + strix_fake_emit_default_success_evidence exit 0 fi echo "Error: PR changed-file scope missing opencode normalizer support dependency ($target_path)" >&2 @@ -5408,6 +5989,7 @@ EOS exit 59 fi echo "scan ok with deployment entrypoint context" + strix_fake_emit_default_success_evidence exit 0 ;; pr-rust-workspace-context) @@ -5422,6 +6004,7 @@ EOS exit 62 fi echo "scan ok with Rust workspace context" + strix_fake_emit_default_success_evidence exit 0 ;; *) @@ -6154,6 +6737,160 @@ run_filtered_gate_case_if_requested() { "vertex_ai/ready-primary" \ "" ;; + success-zero-report-artifacts) + run_gate_case "success-zero-report-artifacts" \ + "vertex_ai/ready-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "1" \ + "Strix exited successfully but produced no report artifacts; log-only success is incomplete evidence, so the scan is failing closed." \ + "1" \ + "vertex_ai/ready-primary" \ + "" + ;; + hollow-success-with-below-threshold-report-fails-closed) + run_gate_case "hollow-success-with-below-threshold-report-fails-closed" \ + "vertex_ai/hollow-below-threshold-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "1" \ + "an rc=0 attempt produced no completed run record; refusing bypass due to incomplete success evidence" \ + "1" \ + "vertex_ai/hollow-below-threshold-primary" \ + "" + ;; + hollow-success-with-baseline-unchanged-report-fails-closed) + run_gate_case "hollow-success-with-baseline-unchanged-report-fails-closed" \ + "openai/gpt-4o-mini" \ + "" \ + "1" \ + "Strix quick scan failed with a non-recoverable error." \ + "1" \ + "openai/gpt-4o-mini" \ + "https://example.invalid" \ + "vertex_ai" \ + "__DEFAULT__" \ + "" \ + "0" \ + "CRITICAL" \ + "0" \ + "" \ + "" \ + "1200" \ + "0" \ + "pull_request" \ + "sync-module-system/smart-crawling-biz/src/main/java/org/empasy/sync/modules/system/controller/SysPositionController.java" + ;; + hollow-primary-recovers-via-completed-fallback) + run_gate_case "hollow-primary-recovers-via-completed-fallback" \ + "vertex_ai/hollow-retryable-primary" \ + "vertex_ai/completed-fallback" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'vertex_ai/completed-fallback' in [0-9]+s\\." \ + "2" \ + "vertex_ai/hollow-retryable-primary|vertex_ai/completed-fallback" \ + "|" + ;; + retry-hollow-second-attempt-fails-closed) + run_gate_case_allow_provider_signal "retry-hollow-second-attempt-fails-closed" \ + "vertex_ai/retry-hollow-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "1" \ + "Strix exited successfully but produced no report artifacts; log-only success is incomplete evidence, so the scan is failing closed." \ + "2" \ + "vertex_ai/retry-hollow-primary|vertex_ai/retry-hollow-primary" \ + "|" \ + "vertex_ai" \ + "__DEFAULT__" \ + "" \ + "1" + ;; + success-clean-scan-zero-findings) + run_gate_case "success-clean-scan-zero-findings" \ + "vertex_ai/ready-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "0" \ + "scan ok with zero findings" \ + "1" \ + "vertex_ai/ready-primary" \ + "" + ;; + recovered-transient-warning-completed-clean-scan) + run_gate_case "recovered-transient-warning-completed-clean-scan" \ + "vertex_ai/ready-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "0" \ + "scan recovered from a transient provider turn and completed with zero findings" \ + "1" \ + "vertex_ai/ready-primary" \ + "" + ;; + completed-clean-scan-with-denied-report-prose) + run_gate_case "completed-clean-scan-with-denied-report-prose" \ + "vertex_ai/ready-primary" \ + "" \ + "0" \ + "Strix run succeeded for model 'vertex_ai/ready-primary'" \ + "1" \ + "vertex_ai/ready-primary" \ + "" + ;; + recovered-transient-warning-exhausted-fails-closed | recovered-transient-warning-malformed-terminal-evidence-fails-closed) + run_gate_case "$STRIX_TEST_CASE_FILTER" \ + "vertex_ai/ready-primary" \ + "" \ + "1" \ + "Strix run emitted provider infrastructure or failure-signal output; failing closed." \ + "1" \ + "vertex_ai/ready-primary" \ + "" + ;; + run-record-in-place-rewrite-counts-as-new-evidence) + run_gate_case_allow_provider_signal "run-record-in-place-rewrite-counts-as-new-evidence" \ + "vertex_ai/rewrite-retry-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "0" \ + "scan ok after in-place run record rewrite" \ + "2" \ + "vertex_ai/rewrite-retry-primary|vertex_ai/rewrite-retry-primary" \ + "|" \ + "vertex_ai" \ + "__DEFAULT__" \ + "" \ + "1" + ;; + unchanged-run-record-rewrite-fails-closed) + run_gate_case_allow_provider_signal "unchanged-run-record-rewrite-fails-closed" \ + "vertex_ai/unchanged-rewrite-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "1" \ + "Strix exited successfully but produced no report artifacts; log-only success is incomplete evidence, so the scan is failing closed." \ + "2" \ + "vertex_ai/unchanged-rewrite-primary|vertex_ai/unchanged-rewrite-primary" \ + "|" \ + "vertex_ai" \ + "__DEFAULT__" \ + "" \ + "1" + ;; + forged-nested-completed-status-fails-closed) + run_gate_case "forged-nested-completed-status-fails-closed" \ + "vertex_ai/ready-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "1" \ + "Strix exited successfully but produced no report artifacts; log-only success is incomplete evidence, so the scan is failing closed." \ + "1" \ + "vertex_ai/ready-primary" \ + "" + ;; + malformed-run-record-fails-closed) + run_gate_case "malformed-run-record-fails-closed" \ + "vertex_ai/ready-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "1" \ + "Strix exited successfully but produced no report artifacts; log-only success is incomplete evidence, so the scan is failing closed." \ + "1" \ + "vertex_ai/ready-primary" \ + "" + ;; contextual-orchestrator-missing-api-base-fails-closed) run_gate_case "contextual-orchestrator-missing-api-base-fails-closed" \ "orchestrator/free" \ @@ -6716,7 +7453,7 @@ run_filtered_gate_case_if_requested() { "vertex_ai/report-known-internal-warning-sanitized" \ "" ;; - provider-fatal-success-signal | provider-warning-success-signal) + provider-fatal-success-signal | provider-warning-success-signal | provider-denied-success-signal) run_gate_case "$STRIX_TEST_CASE_FILTER" \ "vertex_ai/$STRIX_TEST_CASE_FILTER" \ "" \ @@ -7075,6 +7812,19 @@ else fi fi echo "scan ok with PR head content" +# Explicit success evidence -- see this same function's fake-strix script +# above for why (Devin review on `#1495`'s successor `#1563`, round 4). +reports_dir="${STRIX_REPORTS_DIR:-strix_runs}" +mkdir -p "$reports_dir/fake-success/vulnerabilities" +cat >"$reports_dir/fake-success/vulnerabilities/vuln-0001.md" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT +cat >"$reports_dir/fake-success/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD EOF chmod +x "$fake_strix" printf '%s' 'gemini/test-model' >"$strix_llm_file" @@ -7319,6 +8069,20 @@ if [ -e "$context_file" ]; then exit 66 fi echo "scan ok with bounded PR head backend context" +# Explicit success evidence -- see run_pull_request_target_head_scope_case's +# fake-strix script for why (Devin review on `#1495`'s successor `#1563`, +# round 4). +reports_dir="${STRIX_REPORTS_DIR:-strix_runs}" +mkdir -p "$reports_dir/fake-success/vulnerabilities" +cat >"$reports_dir/fake-success/vulnerabilities/vuln-0001.md" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT +cat >"$reports_dir/fake-success/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD EOF chmod +x "$fake_strix" printf '%s' 'gemini/test-model' >"$strix_llm_file" @@ -7452,6 +8216,20 @@ if [ "$attempt" -eq 1 ]; then exit 70 fi echo "scan ok with changed PR head backend context" + # Explicit success evidence -- see run_pull_request_target_head_scope_case's + # fake-strix script for why (Devin review on `#1495`'s successor + # `#1563`, round 4). + reports_dir="${STRIX_REPORTS_DIR:-strix_runs}" + mkdir -p "$reports_dir/fake-success/vulnerabilities" + cat >"$reports_dir/fake-success/vulnerabilities/vuln-0001.md" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT + cat >"$reports_dir/fake-success/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD exit 0 fi @@ -7580,6 +8358,24 @@ run_pull_request_target_changed_backend_context_scope_case() { #!/usr/bin/env bash set -euo pipefail +# Explicit success evidence -- see run_pull_request_target_head_scope_case's +# fake-strix script for why (Devin review on `#1495`'s successor `#1563`, +# round 4). This script has two success exit points below, so the write is +# factored into a small local helper instead of being duplicated. +emit_default_success_evidence() { + local reports_dir="${STRIX_REPORTS_DIR:-strix_runs}" + mkdir -p "$reports_dir/fake-success/vulnerabilities" + cat >"$reports_dir/fake-success/vulnerabilities/vuln-0001.md" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT + cat >"$reports_dir/fake-success/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD +} + printf 'called\n' >> "${FAKE_STRIX_CALL_LOG:?}" target_path="" @@ -7695,10 +8491,12 @@ if [ -f "$target_path/contextual_orchestrator/__main__.py" ]; then fi if [ "$matched_backend_context" -eq 1 ]; then + emit_default_success_evidence exit 0 fi echo "scan ok with non-email backend scope" +emit_default_success_evidence EOF chmod +x "$fake_strix" printf '%s' 'gemini/test-model' >"$strix_llm_file" @@ -7940,6 +8738,20 @@ if grep -Fq -- 'HEAD_THREADING_SERVICE_SHOULD_NOT_BE_SCANNED' "$target_path/back fi echo "scan ok with frontend email trusted backend authorization context" +# Explicit success evidence -- see run_pull_request_target_head_scope_case's +# fake-strix script for why (Devin review on `#1495`'s successor `#1563`, +# round 4). +reports_dir="${STRIX_REPORTS_DIR:-strix_runs}" +mkdir -p "$reports_dir/fake-success/vulnerabilities" +cat >"$reports_dir/fake-success/vulnerabilities/vuln-0001.md" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT +cat >"$reports_dir/fake-success/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD EOF chmod +x "$fake_strix" printf '%s' 'gemini/test-model' >"$strix_llm_file" @@ -8027,7 +8839,22 @@ run_pull_request_target_shallow_head_merge_base_fallback_case() { cat >"$fake_strix" <<'EOF' #!/usr/bin/env bash set -euo pipefail + echo "scan ok" +# Explicit success evidence -- see run_pull_request_target_head_scope_case's +# fake-strix script for why (Devin review on `#1495`'s successor `#1563`, +# round 4). +reports_dir="${STRIX_REPORTS_DIR:-strix_runs}" +mkdir -p "$reports_dir/fake-success/vulnerabilities" +cat >"$reports_dir/fake-success/vulnerabilities/vuln-0001.md" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT +cat >"$reports_dir/fake-success/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD exit 0 EOF chmod +x "$fake_strix" @@ -8525,6 +9352,7 @@ run_full_head_scope_skips_gitlink_case() { cat >"$fake_strix" <<'EOF' #!/usr/bin/env bash set -euo pipefail + target_path="" while [ "$#" -gt 0 ]; do if [ "$1" = "-t" ] && [ "$#" -ge 2 ]; then @@ -8548,6 +9376,23 @@ if [ -e "$target_path/vendor/newsdom-api" ]; then exit 69 fi echo "scan ok with PR head content" +# Explicit success evidence (production's has_new_completed_strix_run() +# requires a run.json with top-level "status": "completed"; a real Strix +# always writes one on completion regardless of finding count). This +# scenario has exactly one success path, so no shared backstop trap is +# needed -- write it directly (Devin review on `#1495`'s successor +# `#1563`, round 4). +reports_dir="${STRIX_REPORTS_DIR:-strix_runs}" +mkdir -p "$reports_dir/fake-success/vulnerabilities" +cat >"$reports_dir/fake-success/vulnerabilities/vuln-0001.md" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT +cat >"$reports_dir/fake-success/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD EOF chmod +x "$fake_strix" printf '%s' 'gemini/test-model' >"$strix_llm_file" @@ -8807,12 +9652,27 @@ run_vertex_model_ignores_untrusted_llm_api_base_file_case() { cat >"$fake_strix" <<'EOF' #!/usr/bin/env bash set -euo pipefail + if [ "${LLM_API_BASE+x}" = "x" ]; then echo "Error: Vertex scan should not receive LLM_API_BASE" >&2 exit 64 fi printf 'called\n' >"${FAKE_STRIX_CALL_LOG:?}" echo "vertex scan ok without external LLM_API_BASE" +# Explicit success evidence -- see run_pull_request_target_head_scope_case's +# fake-strix script for why (Devin review on `#1495`'s successor `#1563`, +# round 4). +reports_dir="${STRIX_REPORTS_DIR:-strix_runs}" +mkdir -p "$reports_dir/fake-success/vulnerabilities" +cat >"$reports_dir/fake-success/vulnerabilities/vuln-0001.md" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT +cat >"$reports_dir/fake-success/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD exit 0 EOF chmod +x "$fake_strix" @@ -9032,6 +9892,7 @@ run_vertex_without_llm_api_key_case() { cat >"$fake_strix" <<'EOF' #!/usr/bin/env bash set -euo pipefail + echo "1" >> "${FAKE_STRIX_CALL_COUNT_FILE:?}" if [ "${LLM_API_KEY+x}" = "x" ]; then echo "unexpected LLM_API_KEY for Vertex" >&2 @@ -9041,6 +9902,20 @@ if [ "${LLM_API_KEY_FILE+x}" = "x" ]; then echo "unexpected LLM_API_KEY_FILE for Vertex" >&2 exit 1 fi +# Explicit success evidence -- see run_pull_request_target_head_scope_case's +# fake-strix script for why (Devin review on `#1495`'s successor `#1563`, +# round 4). +reports_dir="${STRIX_REPORTS_DIR:-strix_runs}" +mkdir -p "$reports_dir/fake-success/vulnerabilities" +cat >"$reports_dir/fake-success/vulnerabilities/vuln-0001.md" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT +cat >"$reports_dir/fake-success/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD exit 0 EOF chmod +x "$fake_strix" @@ -9082,6 +9957,7 @@ run_vertex_with_llm_api_key_file_does_not_forward_case() { cat >"$fake_strix" <<'EOF' #!/usr/bin/env bash set -euo pipefail + echo "1" >> "${FAKE_STRIX_CALL_COUNT_FILE:?}" if [ "${LLM_API_KEY+x}" = "x" ]; then echo "unexpected LLM_API_KEY for Vertex" >&2 @@ -9091,6 +9967,20 @@ if [ "${LLM_API_KEY_FILE+x}" = "x" ]; then echo "unexpected LLM_API_KEY_FILE for Vertex" >&2 exit 1 fi +# Explicit success evidence -- see run_pull_request_target_head_scope_case's +# fake-strix script for why (Devin review on `#1495`'s successor `#1563`, +# round 4). +reports_dir="${STRIX_REPORTS_DIR:-strix_runs}" +mkdir -p "$reports_dir/fake-success/vulnerabilities" +cat >"$reports_dir/fake-success/vulnerabilities/vuln-0001.md" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT +cat >"$reports_dir/fake-success/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD exit 0 EOF chmod +x "$fake_strix" @@ -9372,7 +10262,22 @@ run_input_file_root_override_takes_precedence_over_runner_temp_case() { cat >"$fake_strix" <<'EOF' #!/usr/bin/env bash set -euo pipefail + printf 'called\n' >"${FAKE_STRIX_CALL_LOG:?}" +# Explicit success evidence -- see run_pull_request_target_head_scope_case's +# fake-strix script for why (Devin review on `#1495`'s successor `#1563`, +# round 4). +reports_dir="${STRIX_REPORTS_DIR:-strix_runs}" +mkdir -p "$reports_dir/fake-success/vulnerabilities" +cat >"$reports_dir/fake-success/vulnerabilities/vuln-0001.md" <<'REPORT' +# Vulnerability Report + +- Severity: INFO +- Title: Completed scan produced no findings at or above the fail threshold +REPORT +cat >"$reports_dir/fake-success/run.json" <<'RUNRECORD' +{"status": "completed"} +RUNRECORD exit 0 EOF chmod +x "$fake_strix" @@ -9899,6 +10804,34 @@ run_gate_case "success" \ "vertex_ai/ready-primary" \ "" +# Regression for the zero-evidence "hollow path" bug: Strix exits 0 but +# writes no run.json run record anywhere. Before the fix in run_strix_once() +# (has_new_completed_strix_run()) this was indistinguishable from a +# genuinely clean scan and the gate passed; it must now fail closed with the +# dedicated log-only-success message. +run_gate_case "success-zero-report-artifacts" \ + "vertex_ai/ready-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "1" \ + "Strix exited successfully but produced no report artifacts; log-only success is incomplete evidence, so the scan is failing closed." \ + "1" \ + "vertex_ai/ready-primary" \ + "" + +# Devin review on #1563: an rc=0 attempt that never wrote a completed +# run.json must not be rescued by the below-threshold bypass just because it +# also left behind a genuine below-threshold (INFO) vulnerabilities/*.md +# report. STRIX_HOLLOW_SUCCESS_DETECTED must fail this closed even though +# has_new_strix_vulnerability_report_artifact() finds real evidence. +run_gate_case "hollow-success-with-below-threshold-report-fails-closed" \ + "vertex_ai/hollow-below-threshold-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "1" \ + "an rc=0 attempt produced no completed run record; refusing bypass due to incomplete success evidence" \ + "1" \ + "vertex_ai/hollow-below-threshold-primary" \ + "" + run_gate_case "contextual-orchestrator-missing-api-base-fails-closed" \ "orchestrator/free" \ "" \ @@ -10171,6 +11104,127 @@ run_gate_case_allow_provider_signal "vertex-primary-ratelimit-retry-same-model-s "" \ "1" +# Regression for Devin's review on `#1495`'s successor `#1563`: attempt one +# genuinely completes (writes both a below-threshold report and a completed +# run.json) then the wrapping process still fails transiently (retried); +# attempt two exits 0 with no new run.json of its own. The gate must fail +# closed overall -- has_new_completed_strix_run() must not let attempt two's +# hollow success ride on attempt one's leftover completion evidence, and +# has_only_below_threshold_vulnerabilities()'s presence guard (reached after +# the retry sequence exhausts) must not accept that same stale evidence +# either. +run_gate_case_allow_provider_signal "retry-hollow-second-attempt-fails-closed" \ + "vertex_ai/retry-hollow-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "1" \ + "Strix exited successfully but produced no report artifacts; log-only success is incomplete evidence, so the scan is failing closed." \ + "2" \ + "vertex_ai/retry-hollow-primary|vertex_ai/retry-hollow-primary" \ + "|" \ + "vertex_ai" \ + "__DEFAULT__" \ + "" \ + "1" + +# Regression for Devin's review on `#1495`'s successor `#1563`, round 2: the +# pinned strix-agent only writes vulnerabilities/*.md when there are +# findings, so a genuinely clean scan (zero findings) never writes one -- +# only its always-written run.json and findings.sarif. Requiring a +# vulnerabilities/*.md artifact for the success gate made every clean scan +# fail exactly like the hollow-success bug it was meant to catch; this must +# now pass on run.json's completed status alone. +run_gate_case "success-clean-scan-zero-findings" \ + "vertex_ai/ready-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "0" \ + "scan ok with zero findings" \ + "1" \ + "vertex_ai/ready-primary" \ + "" + +# Regression for Devin's review on `#1495`'s successor `#1563`, round 3: +# has_new_completed_strix_run() compares run.json CONTENT digests, not just +# path identity, when deciding whether an attempt produced new evidence. +# Attempt one writes a completed run.json to a fixed path and then the +# wrapping process still exits non-zero (a transient rate-limit signal after +# real work was already done, so the same model is retried); attempt two +# rewrites the SAME path with genuinely different content (a distinguishable +# second completion) and exits 0. The gate must accept it -- an in-place +# rewrite of an already-existing run.json path is still new evidence when its +# content actually changed, mirroring production's own +# latest_strix_report_dir() mtime-based directory reuse. This is the positive +# mirror of unchanged-run-record-rewrite-fails-closed below. +run_gate_case_allow_provider_signal "run-record-in-place-rewrite-counts-as-new-evidence" \ + "vertex_ai/rewrite-retry-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "0" \ + "scan ok after in-place run record rewrite" \ + "2" \ + "vertex_ai/rewrite-retry-primary|vertex_ai/rewrite-retry-primary" \ + "|" \ + "vertex_ai" \ + "__DEFAULT__" \ + "" \ + "1" + +# Regression for Devin's review on `#1495`'s successor `#1563`, round 3: +# mirrors retry-hollow-second-attempt-fails-closed's general shape but +# specifically proves content-identical reuse does not count as new evidence +# -- not merely "attempt two touched nothing" (which +# retry-hollow-second-attempt-fails-closed already covers) but "attempt two +# actively rewrote the exact same path with byte-identical content" (e.g. +# because it re-selected the same latest run directory and reasserted the +# same completion, mirroring the in-place-rewrite scenario above except the +# rewritten bytes are unchanged). has_new_completed_strix_run()'s digest +# comparison must still reject it: the gate fails closed overall, proving +# digest equality -- not whether the path was merely written to again -- is +# what governs acceptance. +run_gate_case_allow_provider_signal "unchanged-run-record-rewrite-fails-closed" \ + "vertex_ai/unchanged-rewrite-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "1" \ + "Strix exited successfully but produced no report artifacts; log-only success is incomplete evidence, so the scan is failing closed." \ + "2" \ + "vertex_ai/unchanged-rewrite-primary|vertex_ai/unchanged-rewrite-primary" \ + "|" \ + "vertex_ai" \ + "__DEFAULT__" \ + "" \ + "1" + +# Regression for Devin's review on `#1495`'s successor `#1563`, round 3: +# proves strix_run_record_is_completed() parses run.json structurally rather +# than matching the raw text -- a run.json whose top-level "status" key is +# NOT "completed", but which happens to contain the literal substring +# `"status": "completed"` nested under some other field (a forged or +# unrelated occurrence of the same text), must still fail closed exactly like +# a genuinely absent or incomplete run record. A naive substring/regex match +# over the raw file content cannot tell this apart from a genuine top-level +# completion. +run_gate_case "forged-nested-completed-status-fails-closed" \ + "vertex_ai/ready-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "1" \ + "Strix exited successfully but produced no report artifacts; log-only success is incomplete evidence, so the scan is failing closed." \ + "1" \ + "vertex_ai/ready-primary" \ + "" + +# Regression for Devin's review on `#1495`'s successor `#1563`, round 3: +# proves strix_run_record_is_completed() and has_new_completed_strix_run() +# reject a run.json that is not valid JSON at all -- gracefully, via +# json.JSONDecodeError, not by crashing the gate script -- exactly like a +# genuinely absent completion record. Proves the *gate script* handles this +# end-to-end, not just the python snippet in isolation. +run_gate_case "malformed-run-record-fails-closed" \ + "vertex_ai/ready-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "1" \ + "Strix exited successfully but produced no report artifacts; log-only success is incomplete evidence, so the scan is failing closed." \ + "1" \ + "vertex_ai/ready-primary" \ + "" + run_gate_case_allow_provider_signal "vertex-primary-api-connection-retry-same-model-success" \ "gemini/retry-api-connection-primary" \ "vertex_ai/fallback-one vertex_ai/fallback-two" \ @@ -10909,6 +11963,42 @@ run_gate_case "provider-warning-success-signal" \ "" \ "1" +run_gate_case "recovered-transient-warning-completed-clean-scan" \ + "vertex_ai/ready-primary" \ + "vertex_ai/fallback-one vertex_ai/fallback-two" \ + "0" \ + "scan recovered from a transient provider turn and completed with zero findings" \ + "1" \ + "vertex_ai/ready-primary" \ + "" + +run_gate_case "completed-clean-scan-with-denied-report-prose" \ + "vertex_ai/ready-primary" \ + "" \ + "0" \ + "Strix run succeeded for model 'vertex_ai/ready-primary'" \ + "1" \ + "vertex_ai/ready-primary" \ + "" + +run_gate_case "recovered-transient-warning-exhausted-fails-closed" \ + "vertex_ai/ready-primary" \ + "" \ + "1" \ + "Strix run emitted provider infrastructure or failure-signal output; failing closed." \ + "1" \ + "vertex_ai/ready-primary" \ + "" + +run_gate_case "recovered-transient-warning-malformed-terminal-evidence-fails-closed" \ + "vertex_ai/ready-primary" \ + "" \ + "1" \ + "Strix run emitted provider infrastructure or failure-signal output; failing closed." \ + "1" \ + "vertex_ai/ready-primary" \ + "" + run_gate_case "provider-report-rate-limit-fallback-success" \ "vertex_ai/report-rate-limit-primary" \ "vertex_ai/fallback-one vertex_ai/fallback-two" \ @@ -11964,6 +13054,45 @@ run_gate_case "pr-baseline-critical-unchanged" \ "pull_request" \ "sync-module-system/smart-crawling-biz/src/main/java/org/empasy/sync/modules/system/controller/SysPositionController.java" +# Devin review round 5 on #1563: an rc=0 attempt that never wrote a +# completed run.json must not be rescued by evaluate_pull_request_findings()'s +# baseline-allow path (an at-or-above-threshold finding confined to an +# unchanged PR file) any more than by has_only_below_threshold_vulnerabilities(). +run_gate_case "hollow-success-with-baseline-unchanged-report-fails-closed" \ + "openai/gpt-4o-mini" \ + "" \ + "1" \ + "Strix quick scan failed with a non-recoverable error." \ + "1" \ + "openai/gpt-4o-mini" \ + "https://example.invalid" \ + "vertex_ai" \ + "__DEFAULT__" \ + "" \ + "0" \ + "CRITICAL" \ + "0" \ + "" \ + "" \ + "1200" \ + "0" \ + "pull_request" \ + "sync-module-system/smart-crawling-biz/src/main/java/org/empasy/sync/modules/system/controller/SysPositionController.java" + +# Devin review round 6 on #1563: a hollow rc=0 primary whose failure looks +# retryable must still be able to reach a distinct, genuinely completed +# fallback model -- STRIX_HOLLOW_SUCCESS_DETECTED only guards the two +# alternate-success paths immediately after the primary attempt, not the +# unrelated retryability/fallback-model logic further down. +run_gate_case "hollow-primary-recovers-via-completed-fallback" \ + "vertex_ai/hollow-retryable-primary" \ + "vertex_ai/completed-fallback" \ + "0" \ + "REGEX:Strix quick scan succeeded with fallback model 'vertex_ai/completed-fallback' in [0-9]+s\\." \ + "2" \ + "vertex_ai/hollow-retryable-primary|vertex_ai/completed-fallback" \ + "|" + run_gate_case "pr-baseline-critical-absolute-target" \ "openai/gpt-4o-mini" \ "" \ diff --git a/tests/test_strix_attempt_evidence_provenance.py b/tests/test_strix_attempt_evidence_provenance.py new file mode 100644 index 0000000000..ebcd2ead6c --- /dev/null +++ b/tests/test_strix_attempt_evidence_provenance.py @@ -0,0 +1,174 @@ +"""Attempt-scoped Strix evidence must be classified by provenance and content.""" + +from __future__ import annotations + +import re +import subprocess +import tempfile +import unittest +from pathlib import Path + + +REPOSITORY_ROOT = Path(__file__).resolve().parents[1] +STRIX_GATE = REPOSITORY_ROOT / "scripts" / "ci" / "strix_quick_gate.sh" + + +def _function_block(source: str, function_name: str) -> str: + """Return one top-level Bash function, including its closing brace.""" + + match = re.search( + rf"(?ms)^{re.escape(function_name)}\(\) \{{\n.*?^\}}\n", + source, + ) + if match is None: + raise AssertionError(f"missing Bash function: {function_name}") + return match.group(0) + + +def _optional_function_block(source: str, function_name: str) -> str: + """Return a function when present, allowing the pre-fix RED to execute.""" + + try: + return _function_block(source, function_name) + except AssertionError: + return "" + + +def _vulnerability_artifact_is_new(*, rewrite: bytes | None) -> bool: + """Replay the production snapshot/predicate around an in-place report write.""" + + source = STRIX_GATE.read_text(encoding="utf-8") + blocks = [ + _optional_function_block(source, "strix_vulnerability_report_digest"), + _function_block(source, "capture_attempt_start_vulnerability_files"), + _function_block(source, "is_attempt_start_vulnerability_file"), + _function_block(source, "is_preexisting_report_dir"), + _function_block(source, "has_new_strix_vulnerability_report_artifact"), + ] + with tempfile.TemporaryDirectory(prefix="strix-vulnerability-provenance-") as temp: + reports = Path(temp) / "reports" + vulnerability = reports / "current-run" / "vulnerabilities" / "vuln-0001.md" + vulnerability.parent.mkdir(parents=True) + original = b"# Finding\n\nSeverity: INFO\nAttempt: first\n" + vulnerability.write_bytes(original) + script = "\n".join( + ( + "set -euo pipefail", + 'STRIX_REPORTS_DIR="$1"', + 'VULNERABILITY_PATH="$2"', + 'REWRITE_PATH="$3"', + 'PREEXISTING_REPORT_DIRS=("/nonexistent/preexisting")', + "ATTEMPT_START_VULNERABILITY_FILES=()", + "declare -A ATTEMPT_START_VULNERABILITY_DIGESTS=()", + *blocks, + "capture_attempt_start_vulnerability_files", + 'if [ -n "$REWRITE_PATH" ]; then cp "$REWRITE_PATH" "$VULNERABILITY_PATH"; fi', + "has_new_strix_vulnerability_report_artifact", + ) + ) + rewrite_path = Path(temp) / "rewrite.md" + if rewrite is not None: + rewrite_path.write_bytes(rewrite) + rewrite_argument = str(rewrite_path) + else: + rewrite_argument = "" + completed = subprocess.run( + [ + "bash", + "-c", + script, + "strix-vulnerability-provenance", + str(reports), + str(vulnerability), + rewrite_argument, + ], + check=False, + capture_output=True, + text=True, + ) + if completed.returncode not in {0, 1}: + raise AssertionError(completed.stderr) + return completed.returncode == 0 + + +def _accepts_recovered_warning(extra_console: str) -> bool: + """Run the current-attempt structured completion classifier verbatim.""" + + source = STRIX_GATE.read_text(encoding="utf-8") + function = _function_block( + source, + "strix_report_has_authoritative_recovered_transient_completion", + ) + with tempfile.TemporaryDirectory(prefix="strix-current-attempt-") as temp: + report = Path(temp) / "report" + report.mkdir() + (report / "run.json").write_text( + '{"status":"completed","scan_results":' + '{"scan_completed":true,"success":true}}\n', + encoding="utf-8", + ) + (report / "findings.sarif").write_text( + '{"version":"2.1.0","runs":[{"results":[]}]}\n', + encoding="utf-8", + ) + (report / "strix.log").write_text( + "2026-09-05 10:03:16.171 WARNING run - " + "strix.core.execution: transient model/provider error for c0ffee12; " + "replaying turn (attempt 1/5, backoff 2.0s): temporary HTTP 500\n", + encoding="utf-8", + ) + console = Path(temp) / "console.log" + console.write_text(extra_console, encoding="utf-8") + script = "\n".join( + ( + "set -euo pipefail", + "declare -A ATTEMPT_START_RUN_RECORD_DIGESTS=()", + function, + 'strix_report_has_authoritative_recovered_transient_completion "$1" "$2"', + ) + ) + completed = subprocess.run( + ["bash", "-c", script, "strix-current-attempt", str(report), str(console)], + check=False, + capture_output=True, + text=True, + ) + if completed.returncode not in {0, 1}: + raise AssertionError(completed.stderr) + return completed.returncode == 0 + + +class StrixAttemptEvidenceProvenanceTests(unittest.TestCase): + """Protect fresh content evidence and fail-closed terminal signals.""" + + def test_in_place_vulnerability_report_rewrite_is_new_evidence(self) -> None: + """A current attempt may reuse a report path but replace its contents.""" + + self.assertTrue( + _vulnerability_artifact_is_new( + rewrite=b"# Finding\n\nSeverity: INFO\nAttempt: second\n" + ) + ) + + def test_unchanged_vulnerability_report_is_not_new_evidence(self) -> None: + """An untouched predecessor report must not validate a later attempt.""" + + self.assertFalse(_vulnerability_artifact_is_new(rewrite=None)) + + def test_github_error_command_overrides_recovered_warning_completion(self) -> None: + """A typed console error remains terminal even beside clean receipts.""" + + self.assertFalse( + _accepts_recovered_warning( + "scan completed after replay\n::error::artifact publication failed\n" + ) + ) + + def test_recovered_warning_with_clean_console_is_accepted(self) -> None: + """The narrow Inkspan recovered-transient exception remains supported.""" + + self.assertTrue(_accepts_recovered_warning("scan completed after replay\n")) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_strix_recovered_transient_sanitizer.py b/tests/test_strix_recovered_transient_sanitizer.py index 28fda47205..30e161e6c6 100644 --- a/tests/test_strix_recovered_transient_sanitizer.py +++ b/tests/test_strix_recovered_transient_sanitizer.py @@ -12,22 +12,10 @@ module logs ``agent run failed for ; marking failed`` at ERROR with a traceback and the process exits non-zero. -Observed on ContextualWisdomLab/.github#1689 run ``34013778497``: a completed -63-minute scan (``run.json`` status ``completed``, SARIF 0 results, attempt exit -code 0) was failed closed as ``STRIX_PROVIDER_UNAVAILABLE … exhausted`` because -three such WARNING lines survived ``sanitize_known_strix_report_warnings`` and -tripped ``has_strix_report_failure_signal``'s ``WARNING`` scan. - -Negative control, as measured by running this file against ``main``'s gate before -this change: **3 failed, 4 passed.** The three that fail are -``test_recovered_transient_replay_warnings_are_sanitized`` (the lines remain and -the failure signal fires), ``test_production_argument_shape_sanitizes_the_scanned_directory`` -(the same, through the narrowing branch), and -``test_unrecovered_transient_keeps_the_error_and_traceback`` on its first assertion -only, since ``assertNotIn("replaying turn", ...)`` also needs the new alternative -while its ERROR-and-traceback retention assertions hold on both gates. The four -that pass on both gates are the guards: the two unknown-warning cases, the -foreign-module case, and the pre-existing forced-continuation case. +The raw retry warning is retained. A separate current-attempt classifier may +accept it only beside a new completed/successful run receipt and valid SARIF. +This keeps malformed receipts and exhausted ``attempt n/n`` warnings fail-closed +instead of deleting the evidence before classification. """ from __future__ import annotations @@ -141,6 +129,7 @@ def _sanitize_then_signal(log_text: str) -> tuple[str, bool]: _function_block(gate_source, name) for name in ( "sanitize_known_strix_report_warnings", + "strix_report_has_authoritative_recovered_transient_completion", "has_strix_report_failure_signal", ) ] @@ -153,6 +142,8 @@ def _sanitize_then_signal(log_text: str) -> tuple[str, bool]: ( "set -uo pipefail", 'STRIX_REPORTS_DIR="/nonexistent/strix-reports"', + 'STRIX_LOG="$1/strix.log"', + "declare -A ATTEMPT_START_RUN_RECORD_DIGESTS=()", *blocks, 'sanitize_known_strix_report_warnings "$1"', 'if has_strix_report_failure_signal "$1"; then echo signal=1; else echo signal=0; fi', @@ -186,6 +177,7 @@ def _sanitize_then_signal_production_shape(log_text: str) -> tuple[str, bool]: _function_block(gate_source, name) for name in ( "sanitize_known_strix_report_warnings", + "strix_report_has_authoritative_recovered_transient_completion", "has_strix_report_failure_signal", "latest_strix_report_dir", "is_preexisting_report_dir", @@ -201,6 +193,8 @@ def _sanitize_then_signal_production_shape(log_text: str) -> tuple[str, bool]: ( "set -uo pipefail", f'STRIX_REPORTS_DIR="{reports_root}"', + f'STRIX_LOG="{log_path}"', + "declare -A ATTEMPT_START_RUN_RECORD_DIGESTS=()", # Non-empty so "${PREEXISTING_REPORT_DIRS[@]}" is safe under set -u. 'PREEXISTING_REPORT_DIRS=("/nonexistent/preexisting")', *blocks, @@ -221,26 +215,27 @@ def _sanitize_then_signal_production_shape(log_text: str) -> tuple[str, bool]: class StrixRecoveredTransientSanitizerTests(unittest.TestCase): - """Keep a recovered transient model error from failing a completed scan.""" + """Retain transient evidence until structured classification.""" - def test_recovered_transient_replay_warnings_are_sanitized(self) -> None: - """The three observed lines are removed and the WARNING scan stays quiet.""" + def test_recovered_transient_replay_warnings_remain_without_receipts(self) -> None: + """A warning alone is not enough to prove the retry recovered.""" remaining, signal = _sanitize_then_signal(RECOVERED_LOG) - self.assertNotIn("replaying turn", remaining) - self.assertNotIn("InternalServerError", remaining) + self.assertIn("replaying turn", remaining) + self.assertIn("InternalServerError", remaining) self.assertIn("strix.core.runner: Strix scan strix-pr-scope-qd1fsv_9ee6 done", remaining) self.assertIn("strix.llm.context_budget", remaining) - self.assertFalse(signal) + self.assertTrue(signal) def test_unrecovered_transient_keeps_the_error_and_traceback(self) -> None: - """Only the retry line goes; the ERROR record and its traceback stay for the rc!=0 path.""" + """An exhausted retry keeps its warning, error, and traceback.""" - remaining, _signal = _sanitize_then_signal(UNRECOVERED_LOG) - self.assertNotIn("replaying turn", remaining) + remaining, signal = _sanitize_then_signal(UNRECOVERED_LOG) + self.assertIn("replaying turn", remaining) self.assertIn("agent run failed for 6c480eb0; marking failed", remaining) self.assertIn("Traceback (most recent call last):", remaining) self.assertIn("openai.InternalServerError: Error code: 500", remaining) + self.assertTrue(signal) def test_unknown_execution_warning_still_fails_closed(self) -> None: """A WARNING from the same logger with a different message is not sanitized.""" @@ -256,13 +251,13 @@ def test_same_words_from_another_module_still_fail_closed(self) -> None: self.assertEqual(remaining, FOREIGN_MODULE_LOG) self.assertTrue(signal) - def test_production_argument_shape_sanitizes_the_scanned_directory(self) -> None: - """With the reports root passed as production passes it, the narrowed scan is quiet.""" + def test_production_shape_keeps_warning_without_structured_receipts(self) -> None: + """The production narrowing branch retains unproven retry evidence.""" remaining, signal = _sanitize_then_signal_production_shape(RECOVERED_LOG) - self.assertNotIn("replaying turn", remaining) + self.assertIn("replaying turn", remaining) self.assertIn("strix.core.runner: Strix scan strix-pr-scope-qd1fsv_9ee6 done", remaining) - self.assertFalse(signal) + self.assertTrue(signal) def test_production_argument_shape_still_fails_closed_on_an_unknown_warning(self) -> None: """The narrowing branch does not swallow a warning the sanitizer does not know."""