fix(security): fail closed on unavailable dependency review - #897
fix(security): fail closed on unavailable dependency review#897seonghobae wants to merge 25 commits into
Conversation
|
Warning Review limit reached
Next review available in: 55 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughdependency-review 게이트가 exact base/head 비교의 HTTP 200 증거만 허용하도록 변경되었습니다. 오류, 빈 응답, 부분 전송, 비정상 상태 코드는 실패 처리합니다. PR head checkout, 회귀 계약 테스트, 운영 문서를 갱신했습니다. Changes공급망 검증 제어
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The dependency-review gate is changed to fail closed when comparison evidence is unavailable, and no actionable merge-blocking risk remains in the supplied evidence beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant security_scan
participant GitHubCompareAPI
participant dependency_review
PullRequest->>security_scan: PR head repository와 head SHA 전달
security_scan->>GitHubCompareAPI: 정확한 base...head 비교 요청
GitHubCompareAPI-->>security_scan: HTTP 상태와 전송 결과 반환
security_scan->>dependency_review: 전송 코드 0 및 HTTP 200일 때 실행
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
/oc Refetch and abort unless exact head is still Fix test-first on this existing branch only. Add a permanent regression that models a fake/controlled curl which writes HTTP Strengthen |
|
Exact-head RCA and proof for
Hosted exact-head security and supply-chain results remain authoritative. |
|
@opencode-agent review Evaluate exact current head |
|
Follow-up at current head |
|
@opencode-agent review Review exact current head |
|
@coderabbitai review Review exact current head |
|
I will verify the dependency-review fail-closed boundary and the listed workflow invariants. I will keep exact-head check evidence separate from review judgment, approval, and merge authority. I will not modify code, merge, or change protections. I used the repository guidance that only successful checks for the exact current head are valid merge evidence. 🧠 Learnings used
|
|
Exact-current-head review request for Review the bounded six-file supply-chain evidence repair: dependency-review support accepts only HTTP 200; non-200, timeout, or unavailable comparison fails closed with exact identity and operator guidance; response bodies are discarded; checkout is bound to the explicit PR head repository/SHA; and read-only permissions plus immutable action pins remain. Strix Changed Path Quality CI, CodeQL PR, Python Security, Security Scan, SAST Semgrep, Secret Scan, OSV-Scanner PR, Scorecard PR, and SBOM Generation are terminal-success on this exact head. No review thread exists. Keep the exact head unchanged. Do not update the branch, synthesize author approval, substitute another scanner for dependency-review evidence, add credentials, merge, release, or bypass protection. @coderabbitai review |
Rate Limit Exceeded
|
|
@opencode-agent review Re-evaluate exact current head |
|
Please perform a fresh exact-head semantic review of |
|
@cwl-noema-review Please provide an independent exact-head review of |
|
@coderabbitai review Please review exact head |
Rate Limit Exceeded
|
Pull request was converted to draft
|
Returned to Draft because this branch overlaps the authoritative prerequisite #821 and no longer matches its own four-file bounded-scope claim. #821 already owns the fail-closed dependency-review boundary and exact-head focused quality gate. This branch contains a useful incremental refinement—capturing the After #821 reaches protected |
|
@coderabbitai review Please review exact head |
Rate Limit Exceeded
|
|
Please perform a fresh exact-head semantic review of |
|
@cwl-noema-review After a qualifying exact-head OpenCode verdict, independently review |
|
@opencode-agent review exact head |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head38bf4b268fa6bd9bfd5f8155d273c6b2b66e49bd. -
Head SHA:
38bf4b268fa6bd9bfd5f8155d273c6b2b66e49bd -
Workflow run: 32822053544
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: security-scan.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: security-scan.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file: CHANGELOG.md"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file: CHANGELOG.md"]
R2 --> V2["required checks"]
Evidence --> S3["Docs: dependency-review-fail-closed.md"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs: dependency-review-fail-closed.md"]
R3 --> V3["docs review"]
Evidence --> S4["Test: test_required_workflow_queue_contract.py"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test: test_required_workflow_queue_contract.py"]
R4 --> V4["targeted test run"]
|
Fresh downstream-owner handoff from Inkspan after protected central main materially advanced; no foreign source/ref mutation is being performed here. Current owner identities:
The new protected-main commit is material to the previously observed #897 Strix blocker: #1320 includes fallback-model contract alignment, NVIDIA-overload fallback, direct-OpenAI fallback endpoint cleanup, repository-visibility handling, provider-leak regression coverage, and scanner-notice classification fixes. Therefore another unchanged #897 rerun on the pre-#1320 branch is not a materially distinct remedy, and predecessor review/check evidence must not be promoted. First causal boundary remains this existing central Security owner branch plus then-current central workflow source. Smallest safe owner action is to non-destructively reconcile #897 with protected Inkspan revalidation after protected integration remains unchanged: rerun the then-current unchanged/successor Inkspan consumer head and require head-oriented Security jobs to attest |
|
Current owner-path convergence for #1222: exact head Hosted run 32843524109 / job 97788034968 checked out and attested the exact head, completed |
|
@opencode-agent review Please publish an independent substantive formal Reviews API verdict for exact current head Review the current Security Scan exact-head repository/SHA checkout and attestation, live-base versus event-base identity separation, fail-closed dependency evidence, and preserved scanner severity/SARIF semantics. Bind APPROVED or CHANGES_REQUESTED only to this SHA through the formal review surface. Review-only: do not mutate, merge, or close #1222; downstream operational acceptance remains separately required. |
|
Additional downstream RED evidence from Inkspan confirms this owner path is still release-relevant. Affected downstream source: First causal boundary / falsifiable RCA: the currently protected central workflow still treats unavailable comparison support as a successful capability probe and converts the required dependency-review evidence path into a skipped action. If that proposition is wrong, a protected-main consumer run should show a non-success gate whenever the exact comparison cannot be established; this Inkspan run instead shows success + skipped required evidence. Acceptance for #897 remains fail-closed: comparison transport must complete successfully, exact base/head comparison must return HTTP 200, the immutable dependency-review action must actually run, and only then may the gate pass. A 403/404/transport failure/empty or partial response must be terminal non-passing evidence. The smallest remedy remains this existing #897 central-writer change; no Inkspan-local workflow workaround is correct because the organization ruleset owns the required workflow. Post-merge GREEN proof required before closing the owning defect: rerun an Inkspan public-repository PR on an exact current head and verify (1) the central Security Scan checks out/identifies the intended exact source generation, (2) the comparison reaches HTTP 200 with successful transport, (3) the Dependency Review action is not skipped, and (4) the job is terminal success only after that action. Inkspan will continue treating run |
|
Fresh Inkspan consumer canary for the current exact owner head, captured after protected-main synchronization:
This gives a current RED consumer / candidate-GREEN pair at the actual owning boundary. Keep the remedy fail-closed: do not convert unavailable comparison evidence into skip/success. Before normal integration, independently revalidate #897's unchanged exact head/live base, current required jobs and checkout identities, current formal review state, and protection; do not transfer stale reviews or bypass the two-approval policy. After protected integration, re-run an Inkspan public-repository PR canary and require the Dependency Review action itself to execute terminally on the exact submitted head before treating Security Scan as release evidence. |
|
Exact-current-head convergence and verification (2026-08-26)
The newly dispatched security, CodeQL, SBOM, provenance, and quality runs are queued and remain non-passing until terminal. #1222 still requires a downstream ScopeWeave exact-head SAST/Trivy canary after both owner changes integrate; this source convergence is not operational acceptance by itself. |
|
Inkspan consumer revalidation for the current fail-closed owner path (no Inkspan-local workaround): |
|
Additional live public-consumer RED evidence for the existing fail-closed owner path; no Inkspan-local workflow workaround is correct. Affected consumer:
Under Inkspan's release/governance contract this is a false-green supply-chain classification: skipped required evidence is non-passing, even when the job/run aggregate is green. First causal boundary remains central required No duplicate owner/source branch requested; this comment only binds an additional exact affected SHA/run/job to the existing #897/#810 repair. |
|
Fresh owner-path reconciliation found the same revision-identity failure mode on this Security Scan repair. GitHub currently reports protected |
|
Fresh downstream and owner-state revalidation after protected-main movement:
Current owner PR #897 is still exact head Protected Smallest safe owner next action remains: reconcile/validate this existing head against the new protected base without force-push/destructive rebase, regenerate any evidence invalidated by base movement, obtain the required qualifying non-author formal approval, and integrate normally only if live governance permits. Then rerun the public Inkspan #389 canary and require the dependency-review action itself—not only the aggregate job—to execute on the exact release head/base comparison before #810 or Inkspan #118 can treat this boundary as GREEN. |
|
Fresh downstream canary review found one still-live exact-evidence defect on current #897 head Please keep this in the existing owner lane (no new writer): add a realistic workflow-contract RED that fails while |
There was a problem hiding this comment.
Pull request overview
OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
9c565db161800bdf54d2a7c552b3d35b03d0e5d7. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- Strix Security Scan/strix: FAILURE (https://github.com/ContextualWisdomLab/.github/actions/runs/33006256415/job/98300661537)
- Strix Security Scan/strix: failure (https://github.com/ContextualWisdomLab/.github/actions/runs/33006256415/job/98300661537)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: security-scan.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: security-scan.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file: CHANGELOG.md"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file: CHANGELOG.md"]
R2 --> V2["required checks"]
Evidence --> S3["Docs: dependency-review-fail-closed.md"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs: dependency-review-fail-closed.md"]
R3 --> V3["docs review"]
Evidence --> S4["Test: test_required_workflow_queue_contract.py"]
S4 --> I4["regression suite"]
I4 --> R4["Review risk: Test: test_required_workflow_queue_contract.py"]
R4 --> V4["targeted test run"]
|
Fresh owner-path evidence on exact #897 head Owner-side next action should be evidence repair, not #897 source churn or an unchanged rerun while the provider state is unchanged: restore one authorized working Strix provider/lease on the existing central control path, then rerun the unchanged exact head and require a terminal scan result whose findings (or clean result) are attributable to |
1 participant
Purpose
Replace the stale, conflicted #799 integration path with a bounded current-main repair for #810. Preserve #799 as historical evidence only; do not import its divergent removal of later Strix work.
Root cause
The central required
Security Scancould conclude success when the exact dependency-review comparison returned HTTP 403 or 404. The support probe emittedsupported=false, exited zero, and skipped the pinned Dependency Review action. A hard supply-chain evidence boundary had therefore been modeled as an optional capability optimization.Remedy and TDD identity
6eb06cdd08c79a06f7b390069d4ffa49e2eb7dba;b06bb25c1af99b784730209dceb299cd493734f7reverses the stale fail-open queue contract;71947d24828cfb42806f5ef0a8abf83f9279d0afaccepts only HTTP 200, preserves curl transport status separately, bounds connect/total time, discards the response body, checks out the explicit PR-head repository/SHA, and emits exact-identity failure evidence;b527192d7b758fc2f8a95545f25545cec7747c99proves a partial transfer that prints200but exits non-zero cannot pass;59d702f30fa419a016e24c18e87e05c76fbee4a3;Scope correction
Unrelated scheduler-secret and Python-lock-materializer source/test changes that had accumulated on this branch were removed. Flattening an include-bearing requirements lock under a generated filename does not preserve relative include targets and requires a separately designed, test-first path-preservation contract.
The final scope changes only the Dependency Review control plane and its durable architecture, operator, release-note, and workflow-contract evidence. The existing executable mode of
scripts/ci/materialize_base_python_requirements.pyis preserved; the final mode-only correction introduces no source-content change.Acceptance contract
Merge only after:
After merge, require a protected-main public-repository consumer run proving a non-200 or failed-transfer comparison cannot produce a green Dependency Review gate before closing #810.
Summary by CodeRabbit
보안 강화
문서
테스트