diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index c3b8fa5db..1511c4e06 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -16,10 +16,10 @@ # pull_request workflows upload to refs/pull/N/merge, so no single ref ever holds # all tools. Bundling at the workflow/check level is ref-independent. # -# NOTE on dependency-review: dependency graph can be unavailable on some repos. -# Treat that as "not enforceable here" instead of making the required workflow -# unsatisfiable; keep medium-or-higher dependency findings hard-failing where the -# API is supported. +# NOTE on dependency-review: unavailable evidence is not a clean result. Only +# an exact base/head comparison returning HTTP 200 may reach the pinned hard +# gate. Every other probe outcome fails closed without printing the response +# body. See docs/doctoring/dependency-review-fail-closed.md. # # NOTE on trivy-fs: it scans the whole repo, so a pre-existing FIXABLE # MEDIUM/HIGH/CRITICAL finding blocks every PR in that repo until it is fixed. @@ -257,9 +257,11 @@ jobs: contents: read pull-requests: read steps: - - name: Checkout + - name: Checkout exact head uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: + repository: ${{ github.event.pull_request.head.repo.full_name }} + ref: ${{ github.event.pull_request.head.sha }} persist-credentials: false - name: Check dependency review support id: dependency_review_support @@ -272,30 +274,31 @@ jobs: set -euo pipefail api_url="${GITHUB_API_URL:-https://api.github.com}" - response_file="$(mktemp)" + set +e status="$( - curl -fsS -o "$response_file" -w '%{http_code}' \ + curl -sS --connect-timeout 10 --max-time 30 \ + -o /dev/null \ + -w '%{http_code}' \ -H "Accept: application/vnd.github+json" \ -H "Authorization: Bearer ${GH_TOKEN}" \ -H "X-GitHub-Api-Version: 2022-11-28" \ - "${api_url}/repos/${REPOSITORY}/dependency-graph/compare/${BASE_SHA}...${HEAD_SHA}" \ - || true + "${api_url}/repos/${REPOSITORY}/dependency-graph/compare/${BASE_SHA}...${HEAD_SHA}" )" + curl_status=$? + set -e - if [ "$status" = "200" ]; then - echo "supported=true" >>"$GITHUB_OUTPUT" - exit 0 - fi + case "$status" in + [0-9][0-9][0-9]) http_status="$status" ;; + "") http_status="unavailable" ;; + *) http_status="malformed" ;; + esac - if [ "$status" = "403" ] || [ "$status" = "404" ]; then - echo "::warning::Dependency review is unavailable for ${REPOSITORY}; skipping dependency-review hard gate." - echo "supported=false" >>"$GITHUB_OUTPUT" - exit 0 + if [ "$curl_status" -ne 0 ] || [ "$http_status" != "200" ]; then + echo "::error::Dependency review evidence unavailable for ${REPOSITORY} at exact base ${BASE_SHA} and head ${HEAD_SHA}: HTTP ${http_status}; curl exit ${curl_status}. Verify dependency-graph/security configuration and GitHub service behavior, then rerun. Failing closed." + exit 1 fi - echo "::error::Dependency review support check failed with HTTP ${status}." - cat "$response_file" - exit 1 + echo "supported=true" >>"$GITHUB_OUTPUT" - name: Dependency review if: steps.dependency_review_support.outputs.supported == 'true' uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 diff --git a/CHANGELOG.md b/CHANGELOG.md index 1de9130a5..ab435ebd1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,8 @@ Semantic Versioning where the repository publishes a release. ### Security +- Fail closed when GitHub dependency-review evidence is unavailable (non-200, transport failure, or truncated compare) instead of treating HTTP 403/404 as a clean skip; the probe checks out the exact head SHA and never prints the API body. + - Reject `.github/` and `scripts/ci/` from review-thread-derived autofix path authority so an untrusted inline reviewer cannot authorize the write-capable repair agent to modify workflows, CODEOWNERS, actions, scheduler code, or CI helpers that govern its own control plane. - Require the model-write snapshot and exact-path allowlist to remain outside the pull-request worktree, checking both absolute and resolved locations so repository-local controls and outside-looking symlinks resolving into the repository fail closed before they can authorize or verify model changes. - Snapshot the complete pre-model worktree for ordinary and conflict repair and reject every model-caused created, deleted, modified, mode-changed, retargeted, ignored, dangling, directory-backed, external-link, metadata-race, or out-of-scope path before staging or push. diff --git a/docs/doctoring/dependency-review-fail-closed.md b/docs/doctoring/dependency-review-fail-closed.md new file mode 100644 index 000000000..81681d3f0 --- /dev/null +++ b/docs/doctoring/dependency-review-fail-closed.md @@ -0,0 +1,50 @@ +# Dependency review fail-closed operations + +Status: `active_pr` until the matching workflow and regression contract are present on protected `main`; thereafter `implemented_on_protected_main`. + +## Decision + +Dependency review is a hard supply-chain gate. The central workflow accepts only HTTP `200` from GitHub's exact `BASE_SHA...HEAD_SHA` comparison before invoking the immutably pinned dependency-review action. A `403`, `404`, empty or malformed status, timeout, transport failure, truncated exchange, or other unexpected outcome is unavailable evidence and fails closed. + +The support probe has a 10-second connection limit and 30-second total limit. It preserves curl's transport exit code separately from the bounded HTTP status and requires transport exit `0` plus exact HTTP `200`. It discards the response body and logs only repository identity, exact base/head revisions, the normalized HTTP status, and the numeric transport exit. Credentials and response bodies are never diagnostic output. + +RFC 9110 ยง15.3.1 defines `200` as a completed successful representation, not as a status that can be inferred after a truncated transfer (Fielding et al., 2022). NIST SP 800-53 Rev. 5 RA-5 and SA-12 require that vulnerability and supply-chain evidence be obtained, not assumed absent (National Institute of Standards and Technology, 2020). SLSA v1.0 likewise treats missing provenance as unverified rather than passing (SLSA, 2023). An HTTP `403` or `404` is therefore unavailable evidence, not a clean skip. + +## Identity and authority + +The dependency-review job checks out the pull request's explicit head repository and immutable head SHA with persisted credentials disabled. The API comparison independently binds the event's exact base and head revisions. The job retains `contents: read` and `pull-requests: read`; it receives no write, OIDC, model, release, package, or deployment authority. + +Checks, status contexts, review submissions, and merge authorization remain separate evidence classes. OSV, Trivy, CodeQL, Semgrep, Secret Scan, Scorecard, and Dependabot are complementary controls and are not semantic substitutes for dependency review. + +## Failure classification and remediation + +- Transport exit `0` plus HTTP `200`: proceed to the pinned dependency-review action. +- Any other result: fail the job and retain exact repository/base/head/status and transport-exit evidence. An HTTP `200` emitted by a failed or partial transfer is unavailable evidence. +- Public repository failure: verify dependency graph and security configuration, organization policy, token read access, and GitHub service health. +- Private or internal exception: require a separately reviewed organization policy with explicit entitlement evidence and compensating controls. Never infer `not-applicable` from an unavailable response. + +Retries are operator-initiated only after the capability or service condition changes. Do not rerun unchanged evidence repeatedly and do not convert an unavailable endpoint into a green skip. + +## Acceptance and rollback + +Acceptance requires the permanent queue contract to reject the former `supported=false` path, require bounded probing and discarded bodies, require exact-head checkout, and prove that only `200` reaches the action. Exact-head CI/security evidence, current review, protected integration, and a real protected-main consumer run remain required. + +Rollback requires an independently reviewed revert and fresh exact-head evidence. A rollback must not restore the `403`/`404` success path or print an API response body. + +## References + +Fielding, R., Nottingham, M., & Reschke, J. (Eds.). (2022). *HTTP semantics* +(RFC 9110). Internet Engineering Task Force. https://doi.org/10.17487/RFC9110 + +GitHub. (n.d.). *Dependency review*. GitHub Docs. Retrieved August 9, 2026, from https://docs.github.com/en/code-security/concepts/supply-chain-security/dependency-review + +GitHub. (n.d.). *REST API endpoints for dependency review*. GitHub Docs. Retrieved August 9, 2026, from https://docs.github.com/en/rest/dependency-graph/dependency-review + +GitHub. (n.d.). *Dependency graph*. GitHub Docs. Retrieved August 9, 2026, from https://docs.github.com/en/code-security/concepts/supply-chain-security/dependency-graph + +National Institute of Standards and Technology. (2020). *Security and +privacy controls for information systems and organizations* (NIST SP +800-53 Rev. 5). https://doi.org/10.6028/NIST.SP.800-53r5 + +SLSA. (2023). *SLSA v1.0: Supply-chain Levels for Software Artifacts*. +Open Source Security Foundation. https://slsa.dev/spec/v1.0/ diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 233c08584..eea1bf1f4 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -826,16 +826,71 @@ def test_fix_scheduler_cancels_superseded_cron_runs() -> None: assert "cancel-in-progress: true" in workflow -def test_security_scan_skips_dependency_review_when_dependency_graph_is_unavailable() -> ( - None -): +def test_security_scan_fails_closed_when_dependency_review_is_unavailable() -> None: workflow = workflow_text("security-scan.yml") + support_probe = workflow_step(workflow, "Check dependency review support") assert "id: dependency_review_support" in workflow assert "/dependency-graph/compare/${BASE_SHA}...${HEAD_SHA}" in workflow - assert '"$status" = "403"' in workflow - assert '"$status" = "404"' in workflow - assert "steps.dependency_review_support.outputs.supported == 'true'" in workflow + assert "repository: ${{ github.event.pull_request.head.repo.full_name }}" in workflow + assert "ref: ${{ github.event.pull_request.head.sha }}" in workflow + assert 'if [ "$curl_status" -ne 0 ] || [ "$http_status" != "200" ]; then' in workflow + assert "--connect-timeout 10" in workflow + assert "--max-time 30" in workflow + assert "-o /dev/null" in workflow + assert "curl_status=$?" in support_probe + assert "set +e" in support_probe + assert "set -e" in support_probe + assert "|| true" not in support_probe + assert "HTTP ${http_status}; curl exit ${curl_status}" in workflow + assert "supported=false" not in workflow + assert "skipping dependency-review hard gate" not in workflow + assert ( + "steps.dependency_review_support.outputs.supported == 'true'" in workflow + ) + + +def test_dependency_review_transport_failure_cannot_hide_behind_http_200( + tmp_path: Path, +) -> None: + """A failed curl transport must not make HTTP 200 acceptable evidence.""" + + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + fake_curl = fake_bin / "curl" + fake_curl.write_text( + "#!/usr/bin/env bash\nprintf '200'\nexit 18\n", + encoding="utf-8", + ) + fake_curl.chmod(0o755) + github_output = tmp_path / "github-output" + script = textwrap.dedent( + workflow_step( + workflow_text("security-scan.yml"), + "Check dependency review support", + ).split(" run: |\n", 1)[1] + ) + + result = subprocess.run( + ["bash", "-c", script], + env={ + **os.environ, + "PATH": f"{fake_bin}{os.pathsep}{os.environ['PATH']}", + "GITHUB_API_URL": "https://api.example.invalid", + "GITHUB_OUTPUT": str(github_output), + "GH_TOKEN": "synthetic-read-token", + "BASE_SHA": "a" * 40, + "HEAD_SHA": "b" * 40, + "REPOSITORY": "ContextualWisdomLab/.github", + }, + capture_output=True, + text=True, + check=False, + ) + + assert result.returncode == 1 + assert "HTTP 200; curl exit 18" in result.stdout + assert not github_output.exists() def test_security_scan_allows_repositories_without_supported_lockfiles() -> None: