From 07efb4a1978c10c8243becf2627ad72df982e229 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 17 Aug 2026 00:19:01 +0900 Subject: [PATCH 1/6] fix(security): fail closed on unavailable dependency review Replay unique #897 source onto current origin/main. Skip shared ARCHITECTURE/CLAUDE trees. Treat non-200 or failed transport as unavailable evidence rather than a clean skip. --- .github/workflows/security-scan.yml | 43 ++++++------ CHANGELOG.md | 2 + .../dependency-review-fail-closed.md | 50 ++++++++++++++ .../test_required_workflow_queue_contract.py | 67 +++++++++++++++++-- 4 files changed, 136 insertions(+), 26 deletions(-) create mode 100644 docs/doctoring/dependency-review-fail-closed.md 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: From 4ca7d9e968f2833d3e6724e223e4e0149d393fc6 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 15:35:33 +0000 Subject: [PATCH 2/6] fix(security): record visibility on dependency-review fail-closed Close the remaining ContextualWisdomLab/.github#810 diagnostic gap: allowlist public/private/internal/unknown visibility in probe diagnostics, execute 403/404/empty/malformed regressions, and stop making the pinned action independently skippable after a successful probe. Co-authored-by: Seongho Bae --- .github/workflows/security-scan.yml | 11 +- AGENTS.md | 1 + ARCHITECTURE.md | 23 +++ CHANGELOG.md | 2 +- .../dependency-review-fail-closed.md | 6 +- .../test_required_workflow_queue_contract.py | 147 ++++++++++++++---- 6 files changed, 152 insertions(+), 38 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 1511c4e06..5199bd52f 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -19,7 +19,8 @@ # 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. +# body. Diagnostics include allowlisted repository visibility. 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. @@ -270,10 +271,15 @@ jobs: BASE_SHA: ${{ github.event.pull_request.base.sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} REPOSITORY: ${{ github.repository }} + REPOSITORY_VISIBILITY: ${{ github.event.repository.visibility }} run: | set -euo pipefail api_url="${GITHUB_API_URL:-https://api.github.com}" + case "${REPOSITORY_VISIBILITY}" in + public|private|internal) visibility="${REPOSITORY_VISIBILITY}" ;; + *) visibility="unknown" ;; + esac set +e status="$( curl -sS --connect-timeout 10 --max-time 30 \ @@ -294,13 +300,12 @@ jobs: esac 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." + echo "::error::Dependency review evidence unavailable for ${REPOSITORY} (visibility ${visibility}) 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 "supported=true" >>"$GITHUB_OUTPUT" - name: Dependency review - if: steps.dependency_review_support.outputs.supported == 'true' uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 with: fail-on-severity: moderate diff --git a/AGENTS.md b/AGENTS.md index 16f0981c0..ff733cc13 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,3 +5,4 @@ Materialize accepts only exact SHA-256 pins or a bounded relative `-r` include (no `.`/`..`); a lone `--require-hashes` directive is not trust evidence. See [`docs/doctoring/hourly-nvidia-nim-autofix.md`](docs/doctoring/hourly-nvidia-nim-autofix.md). Conflict-scope roots fail closed when the immediate parent directory is a symbolic link. +Dependency-review unavailability fails closed; diagnostics include allowlisted repository visibility. See [`docs/doctoring/dependency-review-fail-closed.md`](docs/doctoring/dependency-review-fail-closed.md). diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 6fe6621b6..14f0bd3b3 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -90,6 +90,29 @@ sequenceDiagram - Rust remains the psychometric arithmetic owner. Repair never substitutes Python for scoring math. +## Dependency-review evidence + +The central `Security Scan` job treats GitHub's exact `BASE_SHA...HEAD_SHA` +comparison as a hard supply-chain evidence boundary. Only transport exit `0` +plus HTTP `200` may reach the immutably pinned dependency-review action. A +`403`, `404`, timeout, truncated transfer, or malformed status fails closed +and records allowlisted repository visibility with the exact revisions. Other +scanners are complementary; they are not substitutes. + +```mermaid +flowchart TD + Probe["Exact base/head compare probe"] + Transport{"curl exit 0 and HTTP 200?"} + Action["Pinned dependency-review action"] + Fail["Fail closed with repo, visibility, SHAs, status"] + + Probe --> Transport + Transport -->|"yes"| Action + Transport -->|"no"| Fail +``` + +See [`docs/doctoring/dependency-review-fail-closed.md`](docs/doctoring/dependency-review-fail-closed.md). + ## Quality gates `scripts/ci/` ships with 100% statement/branch coverage and 100% docstrings. diff --git a/CHANGELOG.md b/CHANGELOG.md index ab435ebd1..53c8ccc91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,7 +39,7 @@ 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. +- 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 records allowlisted repository visibility with 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. diff --git a/docs/doctoring/dependency-review-fail-closed.md b/docs/doctoring/dependency-review-fail-closed.md index 81681d3f0..dc3a4c9e1 100644 --- a/docs/doctoring/dependency-review-fail-closed.md +++ b/docs/doctoring/dependency-review-fail-closed.md @@ -6,7 +6,7 @@ Status: `active_pr` until the matching workflow and regression contract are pres 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. +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, allowlisted visibility (`public`, `private`, `internal`, or `unknown`), exact base/head revisions, the normalized HTTP status, and the numeric transport exit. Credentials, response bodies, and raw untrusted visibility strings are never diagnostic output. After a successful probe the pinned action is not independently skippable. 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. @@ -19,7 +19,7 @@ Checks, status contexts, review submissions, and merge authorization remain sepa ## 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. +- Any other result: fail the job and retain exact repository, allowlisted visibility, base/head, status, and transport-exit evidence. An HTTP `200` emitted by a failed or partial transfer is unavailable evidence. Do not infer a root cause from HTTP `403` or `404`. - 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. @@ -42,6 +42,8 @@ GitHub. (n.d.). *REST API endpoints for dependency review*. GitHub Docs. Retriev 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 +GitHub. (n.d.). *Webhook events and payloads*. GitHub Docs. Retrieved August 16, 2026, from https://docs.github.com/en/webhooks/webhook-events-and-payloads#repository + 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 diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index eea1bf1f4..cce55d4b6 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -27,6 +27,49 @@ def workflow_step(workflow: str, name: str) -> str: return workflow[start:end] +def run_dependency_review_support_probe( + tmp_path: Path, + *, + curl_script: str, + repository_visibility: str = "public", +) -> subprocess.CompletedProcess: + """Run the workflow support probe against a controlled curl binary. + + The helper places a fake ``curl`` first on ``PATH`` so the extracted + workflow script cannot call the real binary, then supplies only the + synthetic environment variables the support step reads. + """ + + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + fake_curl = fake_bin / "curl" + fake_curl.write_text(curl_script, encoding="utf-8") + fake_curl.chmod(0o755) + script = textwrap.dedent( + workflow_step( + workflow_text("security-scan.yml"), + "Check dependency review support", + ).split(" run: |\n", 1)[1] + ) + return 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(tmp_path / "github-output"), + "GH_TOKEN": "synthetic-read-token", + "BASE_SHA": "a" * 40, + "HEAD_SHA": "b" * 40, + "REPOSITORY": "ContextualWisdomLab/.github", + "REPOSITORY_VISIBILITY": repository_visibility, + }, + capture_output=True, + text=True, + check=False, + ) + + def test_merge_scheduler_dispatches_one_review_by_default() -> None: workflow = workflow_text("pr-review-merge-scheduler.yml") @@ -829,11 +872,21 @@ def test_fix_scheduler_cancels_superseded_cron_runs() -> 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") + action_first_line = workflow.split( + " - name: Dependency review\n", + 1, + )[1].splitlines()[0] assert "id: dependency_review_support" in workflow assert "/dependency-graph/compare/${BASE_SHA}...${HEAD_SHA}" 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 ( + "REPOSITORY_VISIBILITY: ${{ github.event.repository.visibility }}" + in support_probe + ) + assert "public|private|internal)" in support_probe + assert "visibility ${visibility}" in support_probe assert 'if [ "$curl_status" -ne 0 ] || [ "$http_status" != "200" ]; then' in workflow assert "--connect-timeout 10" in workflow assert "--max-time 30" in workflow @@ -845,8 +898,8 @@ def test_security_scan_fails_closed_when_dependency_review_is_unavailable() -> N 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 + assert action_first_line.startswith( + " uses: actions/dependency-review-action@" ) @@ -855,42 +908,72 @@ def test_dependency_review_transport_failure_cannot_hide_behind_http_200( ) -> 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", + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '200'\nexit 18\n", ) - 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] + + assert result.returncode == 1 + assert "HTTP 200; curl exit 18" in result.stdout + assert "visibility public" in result.stdout + assert not (tmp_path / "github-output").exists() + + +@pytest.mark.parametrize( + ("curl_script", "expected_http"), + [ + ("#!/usr/bin/env bash\nprintf '403'\nexit 0\n", "403"), + ("#!/usr/bin/env bash\nprintf '404'\nexit 0\n", "404"), + ("#!/usr/bin/env bash\nprintf ''\nexit 0\n", "unavailable"), + ("#!/usr/bin/env bash\nprintf 'OK'\nexit 0\n", "malformed"), + ], +) +def test_dependency_review_non_200_status_fails_closed( + tmp_path: Path, + curl_script: str, + expected_http: str, +) -> None: + """HTTP 403/404, empty, and malformed statuses must fail closed.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script=curl_script, ) - 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 f"HTTP {expected_http}; curl exit 0" in result.stdout + assert "visibility public" in result.stdout + assert not (tmp_path / "github-output").exists() + + +def test_dependency_review_success_writes_supported_true(tmp_path: Path) -> None: + """Only a complete HTTP 200 with transport exit 0 may emit supported=true.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '200'\nexit 0\n", + ) + + assert result.returncode == 0 + assert (tmp_path / "github-output").read_text(encoding="utf-8") == ( + "supported=true\n" + ) + + +def test_dependency_review_records_unknown_visibility_when_unset( + tmp_path: Path, +) -> None: + """Missing or unrecognized visibility must be recorded as unknown.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '403'\nexit 0\n", + repository_visibility="", ) assert result.returncode == 1 - assert "HTTP 200; curl exit 18" in result.stdout - assert not github_output.exists() + assert "visibility unknown" in result.stdout + assert not (tmp_path / "github-output").exists() def test_security_scan_allows_repositories_without_supported_lockfiles() -> None: From 9650fd87121985edd4a211482e740baef57648eb Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 15:54:55 +0000 Subject: [PATCH 3/6] test(security): prove 403 skip and transport failure cannot go green Add executable regressions for the EgressWeave #66 canary (HTTP 403 skip-was-success), a bare transport failure, and curl exit 18 with a printed 200. Record exact SHAs and allowlisted visibility without leaking the probe token. Co-authored-by: Seongho Bae --- .../dependency-review-fail-closed.md | 4 + .../test_required_workflow_queue_contract.py | 122 +++++++++++++++--- 2 files changed, 107 insertions(+), 19 deletions(-) diff --git a/docs/doctoring/dependency-review-fail-closed.md b/docs/doctoring/dependency-review-fail-closed.md index dc3a4c9e1..521ab01c8 100644 --- a/docs/doctoring/dependency-review-fail-closed.md +++ b/docs/doctoring/dependency-review-fail-closed.md @@ -25,6 +25,10 @@ Checks, status contexts, review submissions, and merge authorization remain sepa 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. +## Known canary + +ContextualWisdomLab/EgressWeave#66, Security Scan run `31108241013`, job `92638903658`, compared `10d0c51daf2ad278d66f43be479df8cf6b08ba6d...c038a9509d1a8eae8561cc9081e67e12bd373d42` and received HTTP `403`. The required workflow printed the skip warning, omitted `actions/dependency-review-action`, and still concluded success. Downstream tracking: ContextualWisdomLab/EgressWeave#76. Keep ContextualWisdomLab/.github#810 open until a protected-main public consumer run proves a non-200 or failed-transfer comparison cannot green this job. + ## 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. diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index cce55d4b6..4cfe67d2e 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -27,6 +27,11 @@ def workflow_step(workflow: str, name: str) -> str: return workflow[start:end] +PROBE_TOKEN = "synthetic-read-token" +PROBE_BASE_SHA = "a" * 40 +PROBE_HEAD_SHA = "b" * 40 + + def run_dependency_review_support_probe( tmp_path: Path, *, @@ -35,7 +40,7 @@ def run_dependency_review_support_probe( ) -> subprocess.CompletedProcess: """Run the workflow support probe against a controlled curl binary. - The helper places a fake ``curl`` first on ``PATH`` so the extracted + The helper places a fake ``curl`` alone on ``PATH`` so the extracted workflow script cannot call the real binary, then supplies only the synthetic environment variables the support step reads. """ @@ -54,13 +59,13 @@ def run_dependency_review_support_probe( return subprocess.run( ["bash", "-c", script], env={ - **os.environ, - "PATH": f"{fake_bin}{os.pathsep}{os.environ['PATH']}", + "PATH": str(fake_bin), + "HOME": str(tmp_path), "GITHUB_API_URL": "https://api.example.invalid", "GITHUB_OUTPUT": str(tmp_path / "github-output"), - "GH_TOKEN": "synthetic-read-token", - "BASE_SHA": "a" * 40, - "HEAD_SHA": "b" * 40, + "GH_TOKEN": PROBE_TOKEN, + "BASE_SHA": PROBE_BASE_SHA, + "HEAD_SHA": PROBE_HEAD_SHA, "REPOSITORY": "ContextualWisdomLab/.github", "REPOSITORY_VISIBILITY": repository_visibility, }, @@ -70,6 +75,26 @@ def run_dependency_review_support_probe( ) +def assert_dependency_review_probe_failed( + result: subprocess.CompletedProcess, + tmp_path: Path, + *, + expected_http: str, + expected_curl_exit: str, + expected_visibility: str = "public", +) -> None: + """Require a failed probe that records identity without leaking secrets.""" + + combined = f"{result.stdout}{result.stderr}" + assert result.returncode == 1 + assert f"HTTP {expected_http}; curl exit {expected_curl_exit}" in result.stdout + assert f"visibility {expected_visibility}" in result.stdout + assert f"exact base {PROBE_BASE_SHA} and head {PROBE_HEAD_SHA}" in result.stdout + assert PROBE_TOKEN not in combined + assert "Authorization:" not in combined + assert not (tmp_path / "github-output").exists() + + def test_merge_scheduler_dispatches_one_review_by_default() -> None: workflow = workflow_text("pr-review-merge-scheduler.yml") @@ -898,9 +923,43 @@ def test_security_scan_fails_closed_when_dependency_review_is_unavailable() -> N 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 'cat "$response_file"' not in support_probe + assert "fail-on-severity: moderate" in workflow assert action_first_line.startswith( " uses: actions/dependency-review-action@" ) + osv_job_header = workflow.split(" osv-scan:\n", 1)[1].split(" steps:", 1)[0] + trivy_job_header = workflow.split(" trivy-fs:\n", 1)[1].split(" steps:", 1)[0] + dependency_job_header = workflow.split(" dependency-review:\n", 1)[1].split( + " steps:", + 1, + )[0] + assert "continue-on-error: true" not in osv_job_header + assert "continue-on-error: true" not in trivy_job_header + assert "continue-on-error: true" not in dependency_job_header + + +def test_dependency_review_http_403_skip_cannot_go_green(tmp_path: Path) -> None: + """The EgressWeave #66 canary must not be a green skip. + + ContextualWisdomLab/EgressWeave#66 Security Scan run ``31108241013``, + job ``92638903658``, compared ``10d0c51d…c038a950`` and received HTTP + 403. Current ``main`` printed the skip warning, omitted the pinned + action, and still exited 0. That path is forbidden. + """ + + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '403'\nexit 0\n", + ) + + assert_dependency_review_probe_failed( + result, + tmp_path, + expected_http="403", + expected_curl_exit="0", + ) + assert "skipping dependency-review hard gate" not in result.stdout def test_dependency_review_transport_failure_cannot_hide_behind_http_200( @@ -913,16 +972,33 @@ def test_dependency_review_transport_failure_cannot_hide_behind_http_200( curl_script="#!/usr/bin/env bash\nprintf '200'\nexit 18\n", ) - assert result.returncode == 1 - assert "HTTP 200; curl exit 18" in result.stdout - assert "visibility public" in result.stdout - assert not (tmp_path / "github-output").exists() + assert_dependency_review_probe_failed( + result, + tmp_path, + expected_http="200", + expected_curl_exit="18", + ) + + +def test_dependency_review_transport_failure_fails_closed(tmp_path: Path) -> None: + """A transport failure with no HTTP status must fail the hard gate.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf ''\nexit 28\n", + ) + + assert_dependency_review_probe_failed( + result, + tmp_path, + expected_http="unavailable", + expected_curl_exit="28", + ) @pytest.mark.parametrize( ("curl_script", "expected_http"), [ - ("#!/usr/bin/env bash\nprintf '403'\nexit 0\n", "403"), ("#!/usr/bin/env bash\nprintf '404'\nexit 0\n", "404"), ("#!/usr/bin/env bash\nprintf ''\nexit 0\n", "unavailable"), ("#!/usr/bin/env bash\nprintf 'OK'\nexit 0\n", "malformed"), @@ -933,17 +1009,19 @@ def test_dependency_review_non_200_status_fails_closed( curl_script: str, expected_http: str, ) -> None: - """HTTP 403/404, empty, and malformed statuses must fail closed.""" + """HTTP 404, empty, and malformed statuses must fail closed.""" result = run_dependency_review_support_probe( tmp_path, curl_script=curl_script, ) - assert result.returncode == 1 - assert f"HTTP {expected_http}; curl exit 0" in result.stdout - assert "visibility public" in result.stdout - assert not (tmp_path / "github-output").exists() + assert_dependency_review_probe_failed( + result, + tmp_path, + expected_http=expected_http, + expected_curl_exit="0", + ) def test_dependency_review_success_writes_supported_true(tmp_path: Path) -> None: @@ -954,10 +1032,12 @@ def test_dependency_review_success_writes_supported_true(tmp_path: Path) -> None curl_script="#!/usr/bin/env bash\nprintf '200'\nexit 0\n", ) + combined = f"{result.stdout}{result.stderr}" assert result.returncode == 0 assert (tmp_path / "github-output").read_text(encoding="utf-8") == ( "supported=true\n" ) + assert PROBE_TOKEN not in combined def test_dependency_review_records_unknown_visibility_when_unset( @@ -971,9 +1051,13 @@ def test_dependency_review_records_unknown_visibility_when_unset( repository_visibility="", ) - assert result.returncode == 1 - assert "visibility unknown" in result.stdout - assert not (tmp_path / "github-output").exists() + assert_dependency_review_probe_failed( + result, + tmp_path, + expected_http="403", + expected_curl_exit="0", + expected_visibility="unknown", + ) def test_security_scan_allows_repositories_without_supported_lockfiles() -> None: From fbb8432345ac6565724668afdbf9e3259475b94b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 15:56:21 +0000 Subject: [PATCH 4/6] test(security): keep bash resolvable in the dependency-review probe harness Invoke the extracted support probe with an absolute bash path and keep the fake curl first on PATH so isolated executable regressions can run without calling the real binary. Co-authored-by: Seongho Bae --- tests/test_required_workflow_queue_contract.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 4cfe67d2e..3d1407b60 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -40,7 +40,7 @@ def run_dependency_review_support_probe( ) -> subprocess.CompletedProcess: """Run the workflow support probe against a controlled curl binary. - The helper places a fake ``curl`` alone on ``PATH`` so the extracted + The helper places a fake ``curl`` first on ``PATH`` so the extracted workflow script cannot call the real binary, then supplies only the synthetic environment variables the support step reads. """ @@ -56,10 +56,12 @@ def run_dependency_review_support_probe( "Check dependency review support", ).split(" run: |\n", 1)[1] ) + bash = shutil.which("bash") + assert bash is not None return subprocess.run( - ["bash", "-c", script], + [bash, "-c", script], env={ - "PATH": str(fake_bin), + "PATH": f"{fake_bin}{os.pathsep}{os.environ.get('PATH', '/usr/bin')}", "HOME": str(tmp_path), "GITHUB_API_URL": "https://api.example.invalid", "GITHUB_OUTPUT": str(tmp_path / "github-output"), From 6a7103a48a814ab4892cf9a4aabc9c07177c8147 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:05:12 +0000 Subject: [PATCH 5/6] fix(security): reject malformed dependency-review identity before compare Empty or non-hex base/head revisions and non-canonical repository names no longer reach curl. A forged HTTP 200 therefore cannot write supported=true. Executable regressions now record the exact compare argv and keep the pinned action ungated after a successful probe. Co-authored-by: Seongho Bae --- .github/workflows/security-scan.yml | 15 +- ARCHITECTURE.md | 12 +- CHANGELOG.md | 1 + .../dependency-review-fail-closed.md | 14 +- .../test_required_workflow_queue_contract.py | 169 +++++++++++++++++- 5 files changed, 194 insertions(+), 17 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 5199bd52f..a23b04caf 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -18,8 +18,9 @@ # # 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. Diagnostics include allowlisted repository visibility. See +# gate. Malformed revisions or repository names fail closed before the +# network call. Every other probe outcome fails closed without printing the +# response body. Diagnostics include allowlisted repository visibility. See # docs/doctoring/dependency-review-fail-closed.md. # # NOTE on trivy-fs: it scans the whole repo, so a pre-existing FIXABLE @@ -280,6 +281,16 @@ jobs: public|private|internal) visibility="${REPOSITORY_VISIBILITY}" ;; *) visibility="unknown" ;; esac + revision_pattern='^[0-9a-fA-F]{40}$|^[0-9a-fA-F]{64}$' + repository_pattern='^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$' + if ! [[ "${BASE_SHA}" =~ $revision_pattern ]] || ! [[ "${HEAD_SHA}" =~ $revision_pattern ]]; then + echo "::error::Dependency review evidence unavailable for ${REPOSITORY} (visibility ${visibility}) at exact base ${BASE_SHA} and head ${HEAD_SHA}: HTTP unavailable; curl exit uncalled. Malformed revision. Supply the pull request's exact 40- or 64-character hex base and head SHAs, then rerun. Failing closed." + exit 1 + fi + if ! [[ "${REPOSITORY}" =~ $repository_pattern ]]; then + echo "::error::Dependency review evidence unavailable for ${REPOSITORY} (visibility ${visibility}) at exact base ${BASE_SHA} and head ${HEAD_SHA}: HTTP unavailable; curl exit uncalled. Malformed repository. Use the canonical owner/name and rerun. Failing closed." + exit 1 + fi set +e status="$( curl -sS --connect-timeout 10 --max-time 30 \ diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 14f0bd3b3..daadda4a2 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -93,11 +93,13 @@ sequenceDiagram ## Dependency-review evidence The central `Security Scan` job treats GitHub's exact `BASE_SHA...HEAD_SHA` -comparison as a hard supply-chain evidence boundary. Only transport exit `0` -plus HTTP `200` may reach the immutably pinned dependency-review action. A -`403`, `404`, timeout, truncated transfer, or malformed status fails closed -and records allowlisted repository visibility with the exact revisions. Other -scanners are complementary; they are not substitutes. +comparison as a hard supply-chain evidence boundary. Revisions must be 40- or +64-character hex and the repository must be canonical `owner/name` before the +probe opens a socket. Only transport exit `0` plus HTTP `200` may reach the +immutably pinned dependency-review action. A `403`, `404`, timeout, truncated +transfer, malformed status, or malformed identity fails closed and records +allowlisted repository visibility with the exact revisions. Other scanners +are complementary; they are not substitutes. ```mermaid flowchart TD diff --git a/CHANGELOG.md b/CHANGELOG.md index 53c8ccc91..efe60c4f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,7 @@ 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 records allowlisted repository visibility with the exact head SHA and never prints the API body. +- Reject malformed dependency-review revisions and repository names before the compare request so an empty SHA or `../` repository cannot mint a green `supported=true` from a forged HTTP 200. - 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. diff --git a/docs/doctoring/dependency-review-fail-closed.md b/docs/doctoring/dependency-review-fail-closed.md index 521ab01c8..c8941dc40 100644 --- a/docs/doctoring/dependency-review-fail-closed.md +++ b/docs/doctoring/dependency-review-fail-closed.md @@ -6,9 +6,9 @@ Status: `active_pr` until the matching workflow and regression contract are pres 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, allowlisted visibility (`public`, `private`, `internal`, or `unknown`), exact base/head revisions, the normalized HTTP status, and the numeric transport exit. Credentials, response bodies, and raw untrusted visibility strings are never diagnostic output. After a successful probe the pinned action is not independently skippable. +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 rejects a base or head revision that is not exactly 40 or 64 hexadecimal characters, and a repository name that is not `owner/name`, before any network call (curl exit `uncalled`). It discards the response body and logs only repository identity, allowlisted visibility (`public`, `private`, `internal`, or `unknown`), exact base/head revisions, the normalized HTTP status, and the numeric transport exit. Credentials, response bodies, and raw untrusted visibility strings are never diagnostic output. After a successful probe the pinned action is not independently skippable. Executable regressions record the exact compare argv so a hardcoded `supported=true` path cannot satisfy the success contract. -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. +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). RFC 3986 forbids unvalidated path segments in a request-target (Berners-Lee et al., 2005). CWE-20 requires rejecting malformed identity before it is interpolated into that target (MITRE, n.d.). 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 @@ -19,6 +19,7 @@ Checks, status contexts, review submissions, and merge authorization remain sepa ## Failure classification and remediation - Transport exit `0` plus HTTP `200`: proceed to the pinned dependency-review action. +- Malformed revision or repository: fail closed with HTTP `unavailable` and curl exit `uncalled` before opening a socket. Re-supply the pull request's exact hex SHAs and canonical `owner/name`, then rerun. - Any other result: fail the job and retain exact repository, allowlisted visibility, base/head, status, and transport-exit evidence. An HTTP `200` emitted by a failed or partial transfer is unavailable evidence. Do not infer a root cause from HTTP `403` or `404`. - 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. @@ -31,12 +32,16 @@ ContextualWisdomLab/EgressWeave#66, Security Scan run `31108241013`, job `926389 ## 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. +Acceptance requires the permanent queue contract to reject the former `supported=false` path, require bounded probing and discarded bodies, require exact-head checkout, reject malformed revisions and repository names before the network call, prove the success path invoked the exact compare URL, 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 +Berners-Lee, T., Fielding, R., & Masinter, L. (2005). *Uniform Resource +Identifier (URI): Generic syntax* (RFC 3986). Internet Engineering Task +Force. https://doi.org/10.17487/RFC3986 + Fielding, R., Nottingham, M., & Reschke, J. (Eds.). (2022). *HTTP semantics* (RFC 9110). Internet Engineering Task Force. https://doi.org/10.17487/RFC9110 @@ -48,6 +53,9 @@ GitHub. (n.d.). *Dependency graph*. GitHub Docs. Retrieved August 9, 2026, from GitHub. (n.d.). *Webhook events and payloads*. GitHub Docs. Retrieved August 16, 2026, from https://docs.github.com/en/webhooks/webhook-events-and-payloads#repository +MITRE. (n.d.). *CWE-20: Improper input validation*. Retrieved August 16, +2026, from https://cwe.mitre.org/data/definitions/20.html + 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 diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 3d1407b60..bd533f4de 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -37,18 +37,30 @@ def run_dependency_review_support_probe( *, curl_script: str, repository_visibility: str = "public", + repository: str = "ContextualWisdomLab/.github", + base_sha: str = PROBE_BASE_SHA, + head_sha: str = PROBE_HEAD_SHA, ) -> subprocess.CompletedProcess: """Run the workflow support probe against a controlled curl binary. The helper places a fake ``curl`` first on ``PATH`` so the extracted - workflow script cannot call the real binary, then supplies only the - synthetic environment variables the support step reads. + workflow script cannot call the real binary, records the exact argv + the probe used, then supplies only the synthetic environment + variables the support step reads. """ fake_bin = tmp_path / "bin" fake_bin.mkdir() fake_curl = fake_bin / "curl" - fake_curl.write_text(curl_script, encoding="utf-8") + argv_path = tmp_path / "curl-argv" + shebang, separator, rest = curl_script.partition("\n") + if not shebang.startswith("#!"): + shebang = "#!/usr/bin/env bash" + rest = curl_script + elif not separator: + rest = "" + recorder = f"printf '%s\\0' \"$0\" \"$@\" > {shlex.quote(str(argv_path))}\n" + fake_curl.write_text(f"{shebang}\n{recorder}{rest}\n", encoding="utf-8") fake_curl.chmod(0o755) script = textwrap.dedent( workflow_step( @@ -66,9 +78,9 @@ def run_dependency_review_support_probe( "GITHUB_API_URL": "https://api.example.invalid", "GITHUB_OUTPUT": str(tmp_path / "github-output"), "GH_TOKEN": PROBE_TOKEN, - "BASE_SHA": PROBE_BASE_SHA, - "HEAD_SHA": PROBE_HEAD_SHA, - "REPOSITORY": "ContextualWisdomLab/.github", + "BASE_SHA": base_sha, + "HEAD_SHA": head_sha, + "REPOSITORY": repository, "REPOSITORY_VISIBILITY": repository_visibility, }, capture_output=True, @@ -77,6 +89,29 @@ def run_dependency_review_support_probe( ) +def assert_dependency_review_compare_invoked( + tmp_path: Path, + *, + repository: str = "ContextualWisdomLab/.github", + base_sha: str = PROBE_BASE_SHA, + head_sha: str = PROBE_HEAD_SHA, +) -> None: + """Require the probe to request the exact base/head compare URL.""" + + argv = [ + part.decode("utf-8") + for part in (tmp_path / "curl-argv").read_bytes().split(b"\0") + if part + ] + expected = ( + f"https://api.example.invalid/repos/{repository}/" + f"dependency-graph/compare/{base_sha}...{head_sha}" + ) + assert expected in argv + assert "-o" in argv + assert "/dev/null" in argv + + def assert_dependency_review_probe_failed( result: subprocess.CompletedProcess, tmp_path: Path, @@ -84,6 +119,8 @@ def assert_dependency_review_probe_failed( expected_http: str, expected_curl_exit: str, expected_visibility: str = "public", + expected_base_sha: str = PROBE_BASE_SHA, + expected_head_sha: str = PROBE_HEAD_SHA, ) -> None: """Require a failed probe that records identity without leaking secrets.""" @@ -91,7 +128,9 @@ def assert_dependency_review_probe_failed( assert result.returncode == 1 assert f"HTTP {expected_http}; curl exit {expected_curl_exit}" in result.stdout assert f"visibility {expected_visibility}" in result.stdout - assert f"exact base {PROBE_BASE_SHA} and head {PROBE_HEAD_SHA}" in result.stdout + assert ( + f"exact base {expected_base_sha} and head {expected_head_sha}" in result.stdout + ) assert PROBE_TOKEN not in combined assert "Authorization:" not in combined assert not (tmp_path / "github-output").exists() @@ -914,6 +953,10 @@ def test_security_scan_fails_closed_when_dependency_review_is_unavailable() -> N ) assert "public|private|internal)" in support_probe assert "visibility ${visibility}" in support_probe + assert 'revision_pattern=' in support_probe + assert 'repository_pattern=' in support_probe + assert "Malformed revision" in support_probe + assert "Malformed repository" in support_probe assert 'if [ "$curl_status" -ne 0 ] || [ "$http_status" != "200" ]; then' in workflow assert "--connect-timeout 10" in workflow assert "--max-time 30" in workflow @@ -930,6 +973,11 @@ def test_security_scan_fails_closed_when_dependency_review_is_unavailable() -> N assert action_first_line.startswith( " uses: actions/dependency-review-action@" ) + action_step = workflow.split(" - name: Dependency review\n", 1)[1].split( + "\n trivy-fs:", + 1, + )[0] + assert "if:" not in action_step osv_job_header = workflow.split(" osv-scan:\n", 1)[1].split(" steps:", 1)[0] trivy_job_header = workflow.split(" trivy-fs:\n", 1)[1].split(" steps:", 1)[0] dependency_job_header = workflow.split(" dependency-review:\n", 1)[1].split( @@ -962,6 +1010,7 @@ def test_dependency_review_http_403_skip_cannot_go_green(tmp_path: Path) -> None expected_curl_exit="0", ) assert "skipping dependency-review hard gate" not in result.stdout + assert_dependency_review_compare_invoked(tmp_path) def test_dependency_review_transport_failure_cannot_hide_behind_http_200( @@ -1040,6 +1089,30 @@ def test_dependency_review_success_writes_supported_true(tmp_path: Path) -> None "supported=true\n" ) assert PROBE_TOKEN not in combined + assert_dependency_review_compare_invoked(tmp_path) + + +def test_dependency_review_accepts_sha256_object_ids(tmp_path: Path) -> None: + """A 64-character hex object id is a legal Git revision and may be compared.""" + + base_sha = "c" * 64 + head_sha = "d" * 64 + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '200'\nexit 0\n", + base_sha=base_sha, + head_sha=head_sha, + ) + + assert result.returncode == 0 + assert (tmp_path / "github-output").read_text(encoding="utf-8") == ( + "supported=true\n" + ) + assert_dependency_review_compare_invoked( + tmp_path, + base_sha=base_sha, + head_sha=head_sha, + ) def test_dependency_review_records_unknown_visibility_when_unset( @@ -1060,6 +1133,88 @@ def test_dependency_review_records_unknown_visibility_when_unset( expected_curl_exit="0", expected_visibility="unknown", ) + assert_dependency_review_compare_invoked(tmp_path) + + +@pytest.mark.parametrize("visibility", ["public", "private", "internal"]) +def test_dependency_review_records_allowlisted_visibility( + tmp_path: Path, + visibility: str, +) -> None: + """Allowlisted visibility values must appear in fail-closed diagnostics.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '403'\nexit 0\n", + repository_visibility=visibility, + ) + + assert_dependency_review_probe_failed( + result, + tmp_path, + expected_http="403", + expected_curl_exit="0", + expected_visibility=visibility, + ) + assert_dependency_review_compare_invoked(tmp_path) + + +@pytest.mark.parametrize( + ("base_sha", "head_sha"), + [ + ("", PROBE_HEAD_SHA), + (PROBE_BASE_SHA, ""), + ("not-a-revision", PROBE_HEAD_SHA), + (PROBE_BASE_SHA, "../" + ("c" * 37)), + ("d" * 39, PROBE_HEAD_SHA), + ], +) +def test_dependency_review_rejects_malformed_revision_before_network( + tmp_path: Path, + base_sha: str, + head_sha: str, +) -> None: + """Empty or non-hex revisions must fail closed without calling curl.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '200'\nexit 0\n", + base_sha=base_sha, + head_sha=head_sha, + ) + + assert_dependency_review_probe_failed( + result, + tmp_path, + expected_http="unavailable", + expected_curl_exit="uncalled", + expected_base_sha=base_sha, + expected_head_sha=head_sha, + ) + assert "Malformed revision" in result.stdout + assert not (tmp_path / "curl-argv").exists() + + +def test_dependency_review_rejects_malformed_repository_before_network( + tmp_path: Path, +) -> None: + """A repository name that cannot form a compare URL must not reach curl.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '200'\nexit 0\n", + repository="../evil/repo", + ) + + assert_dependency_review_probe_failed( + result, + tmp_path, + expected_http="unavailable", + expected_curl_exit="uncalled", + expected_visibility="public", + ) + assert "Malformed repository" in result.stdout + assert not (tmp_path / "curl-argv").exists() def test_security_scan_allows_repositories_without_supported_lockfiles() -> None: From e4a7dffa4c4ee8cceb423504d0f01d8a803f7ddb Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:11:31 +0000 Subject: [PATCH 6/6] fix(security): reject dot segments and hide raw dependency-review identity Close the remaining CWE-22 hole where owner/.. still matched owner/name, stop echoing untrusted revisions on identity rejection, and classify curl 000 as unavailable evidence before the pinned action. Co-authored-by: Seongho Bae --- .github/workflows/security-scan.yml | 19 +-- AGENTS.md | 2 +- ARCHITECTURE.md | 21 +-- CHANGELOG.md | 4 +- .../dependency-review-fail-closed.md | 21 ++- .../test_required_workflow_queue_contract.py | 127 ++++++++++++++++-- 6 files changed, 158 insertions(+), 36 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index a23b04caf..065ae5c30 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -18,10 +18,11 @@ # # 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. Malformed revisions or repository names fail closed before the -# network call. Every other probe outcome fails closed without printing the -# response body. Diagnostics include allowlisted repository visibility. See -# docs/doctoring/dependency-review-fail-closed.md. +# gate. Named refs, `.`/`..` repository segments, and other malformed +# identity fail closed before the network call and are not echoed. Curl's +# `000` sentinel is unavailable evidence. Every other probe outcome fails +# closed without printing the response body. Diagnostics include allowlisted +# repository visibility. 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. @@ -283,12 +284,14 @@ jobs: esac revision_pattern='^[0-9a-fA-F]{40}$|^[0-9a-fA-F]{64}$' repository_pattern='^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$' + repository_owner="${REPOSITORY%%/*}" + repository_name="${REPOSITORY#*/}" if ! [[ "${BASE_SHA}" =~ $revision_pattern ]] || ! [[ "${HEAD_SHA}" =~ $revision_pattern ]]; then - echo "::error::Dependency review evidence unavailable for ${REPOSITORY} (visibility ${visibility}) at exact base ${BASE_SHA} and head ${HEAD_SHA}: HTTP unavailable; curl exit uncalled. Malformed revision. Supply the pull request's exact 40- or 64-character hex base and head SHAs, then rerun. Failing closed." + echo "::error::Dependency review evidence unavailable for the allowlisted repository (visibility ${visibility}): HTTP unavailable; curl exit uncalled. Malformed revision. Supply the pull request's exact 40- or 64-character hex base and head SHAs, then rerun. Named refs are not evidence. Failing closed." exit 1 fi - if ! [[ "${REPOSITORY}" =~ $repository_pattern ]]; then - echo "::error::Dependency review evidence unavailable for ${REPOSITORY} (visibility ${visibility}) at exact base ${BASE_SHA} and head ${HEAD_SHA}: HTTP unavailable; curl exit uncalled. Malformed repository. Use the canonical owner/name and rerun. Failing closed." + if ! [[ "${REPOSITORY}" =~ $repository_pattern ]] || [ "${repository_owner}" = "." ] || [ "${repository_owner}" = ".." ] || [ "${repository_name}" = "." ] || [ "${repository_name}" = ".." ]; then + echo "::error::Dependency review evidence unavailable (visibility ${visibility}): HTTP unavailable; curl exit uncalled. Malformed repository. Use the canonical owner/name without . or .. path segments, then rerun. Failing closed." exit 1 fi set +e @@ -305,8 +308,8 @@ jobs: set -e case "$status" in + 000|"") http_status="unavailable" ;; [0-9][0-9][0-9]) http_status="$status" ;; - "") http_status="unavailable" ;; *) http_status="malformed" ;; esac diff --git a/AGENTS.md b/AGENTS.md index ff733cc13..c9fbbd027 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -5,4 +5,4 @@ Materialize accepts only exact SHA-256 pins or a bounded relative `-r` include (no `.`/`..`); a lone `--require-hashes` directive is not trust evidence. See [`docs/doctoring/hourly-nvidia-nim-autofix.md`](docs/doctoring/hourly-nvidia-nim-autofix.md). Conflict-scope roots fail closed when the immediate parent directory is a symbolic link. -Dependency-review unavailability fails closed; diagnostics include allowlisted repository visibility. See [`docs/doctoring/dependency-review-fail-closed.md`](docs/doctoring/dependency-review-fail-closed.md). +Dependency-review unavailability fails closed; diagnostics include allowlisted repository visibility. Named refs and `.`/`..` repository segments are rejected before the compare request and are not echoed. See [`docs/doctoring/dependency-review-fail-closed.md`](docs/doctoring/dependency-review-fail-closed.md). diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index daadda4a2..c933551a4 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -93,21 +93,26 @@ sequenceDiagram ## Dependency-review evidence The central `Security Scan` job treats GitHub's exact `BASE_SHA...HEAD_SHA` -comparison as a hard supply-chain evidence boundary. Revisions must be 40- or -64-character hex and the repository must be canonical `owner/name` before the -probe opens a socket. Only transport exit `0` plus HTTP `200` may reach the -immutably pinned dependency-review action. A `403`, `404`, timeout, truncated -transfer, malformed status, or malformed identity fails closed and records -allowlisted repository visibility with the exact revisions. Other scanners -are complementary; they are not substitutes. +comparison as a hard supply-chain evidence boundary. The probe rejects named +refs, `.`/`..` repository segments, and other non-`owner/name` values before +it opens a socket, and it does not echo those raw invalid values. Only an +exact 40- or 64-character hexadecimal object ID pair plus transport exit `0` +plus HTTP `200` may reach the immutably pinned dependency-review action. A +`403`, `404`, timeout, truncated transfer, curl `000` sentinel, or malformed +status fails closed and records allowlisted repository visibility with the +now-validated revisions. Other scanners are complementary; they are not +substitutes. ```mermaid flowchart TD + Identity{"owner/name, no dot segments, 40- or 64-hex SHAs?"} Probe["Exact base/head compare probe"] Transport{"curl exit 0 and HTTP 200?"} Action["Pinned dependency-review action"] - Fail["Fail closed with repo, visibility, SHAs, status"] + Fail["Fail closed; do not echo raw invalid identity"] + Identity -->|"yes"| Probe + Identity -->|"no"| Fail Probe --> Transport Transport -->|"yes"| Action Transport -->|"no"| Fail diff --git a/CHANGELOG.md b/CHANGELOG.md index efe60c4f1..acb85f407 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,8 +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 records allowlisted repository visibility with the exact head SHA and never prints the API body. -- Reject malformed dependency-review revisions and repository names before the compare request so an empty SHA or `../` repository cannot mint a green `supported=true` from a forged HTTP 200. +- Fail closed when GitHub dependency-review evidence is unavailable (non-200, transport failure, truncated compare, or curl's `000` no-status sentinel) instead of treating HTTP 403/404 as a clean skip; the probe records allowlisted repository visibility with the exact head SHA and never prints the API body. +- Reject named Git revisions, `.`/`..` repository segments, and other non-`owner/name` values before the dependency-review compare request so GitHub cannot resolve `main` to a moving HEAD and so path traversal cannot leave `/repos/{owner}/{repo}`. Identity-rejection diagnostics tell the operator to resupply exact event SHAs and do not echo the raw invalid values. - 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. diff --git a/docs/doctoring/dependency-review-fail-closed.md b/docs/doctoring/dependency-review-fail-closed.md index c8941dc40..aad5e16c2 100644 --- a/docs/doctoring/dependency-review-fail-closed.md +++ b/docs/doctoring/dependency-review-fail-closed.md @@ -6,9 +6,9 @@ Status: `active_pr` until the matching workflow and regression contract are pres 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 rejects a base or head revision that is not exactly 40 or 64 hexadecimal characters, and a repository name that is not `owner/name`, before any network call (curl exit `uncalled`). It discards the response body and logs only repository identity, allowlisted visibility (`public`, `private`, `internal`, or `unknown`), exact base/head revisions, the normalized HTTP status, and the numeric transport exit. Credentials, response bodies, and raw untrusted visibility strings are never diagnostic output. After a successful probe the pinned action is not independently skippable. Executable regressions record the exact compare argv so a hardcoded `supported=true` path cannot satisfy the success contract. +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 rejects a base or head revision that is not exactly 40 or 64 hexadecimal characters, a repository name that is not `owner/name`, and any owner or name segment that is `.` or `..`, before any network call (curl exit `uncalled`). Named refs such as `main` are not evidence: GitHub's compare API would resolve them to a moving HEAD. Identity-rejection diagnostics name the failure class and the operator next step; they do not echo the raw invalid revision or repository. After a validated identity, the probe discards the response body and logs only the now-validated repository identity, allowlisted visibility (`public`, `private`, `internal`, or `unknown`), exact base/head revisions, the normalized HTTP status, and the numeric transport exit. Curl's `000` no-status sentinel is unavailable evidence even when transport exit is `0`. Credentials, response bodies, and raw untrusted visibility strings are never diagnostic output. After a successful probe the pinned action is not independently skippable. Executable regressions record the exact compare argv so a hardcoded `supported=true` path cannot satisfy the success contract. The special `.github` repository name remains legal owner/name. -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). RFC 3986 forbids unvalidated path segments in a request-target (Berners-Lee et al., 2005). CWE-20 requires rejecting malformed identity before it is interpolated into that target (MITRE, n.d.). 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. +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). RFC 3986 forbids unvalidated path segments, including `.` and `..`, in a request-target (Berners-Lee et al., 2005). CWE-20 requires rejecting malformed identity before it is interpolated into that target (MITRE, n.d.-a). CWE-22 requires rejecting `.` and `..` segments that would traverse out of `/repos/{owner}/{repo}` (MITRE, n.d.-b). Git object IDs are exact hexadecimal names of immutable objects, not branch or tag refs (Chacon & Straub, 2014; Dang, 2015). 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 @@ -19,7 +19,8 @@ Checks, status contexts, review submissions, and merge authorization remain sepa ## Failure classification and remediation - Transport exit `0` plus HTTP `200`: proceed to the pinned dependency-review action. -- Malformed revision or repository: fail closed with HTTP `unavailable` and curl exit `uncalled` before opening a socket. Re-supply the pull request's exact hex SHAs and canonical `owner/name`, then rerun. +- Malformed revision, named ref, or repository (including `.` / `..` segments): fail closed with HTTP `unavailable` and curl exit `uncalled` before opening a socket. Re-supply the pull request's exact hex SHAs and canonical `owner/name`, then rerun. Do not retry a branch name. +- Curl status `000` or empty with any transport exit: treat as unavailable evidence and fail closed. Check runner egress and GitHub service health, then rerun. - Any other result: fail the job and retain exact repository, allowlisted visibility, base/head, status, and transport-exit evidence. An HTTP `200` emitted by a failed or partial transfer is unavailable evidence. Do not infer a root cause from HTTP `403` or `404`. - 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. @@ -32,7 +33,7 @@ ContextualWisdomLab/EgressWeave#66, Security Scan run `31108241013`, job `926389 ## 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, reject malformed revisions and repository names before the network call, prove the success path invoked the exact compare URL, 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. +Acceptance requires the permanent queue contract to reject the former `supported=false` path, require bounded probing and discarded bodies, require exact-head checkout, reject named refs, `.`/`..` repository segments, and other malformed identity before the network call without echoing those raw values, classify curl `000` as unavailable, prove the success path invoked the exact compare URL, 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. @@ -42,6 +43,12 @@ Berners-Lee, T., Fielding, R., & Masinter, L. (2005). *Uniform Resource Identifier (URI): Generic syntax* (RFC 3986). Internet Engineering Task Force. https://doi.org/10.17487/RFC3986 +Chacon, S., & Straub, B. (2014). *Pro Git* (2nd ed.). Apress. +https://git-scm.com/book/en/v2/Git-Internals-Git-Objects + +Dang, Q. H. (2015). *Secure Hash Standard (SHS)* (FIPS PUB 180-4). National +Institute of Standards and Technology. https://doi.org/10.6028/NIST.FIPS.180-4 + Fielding, R., Nottingham, M., & Reschke, J. (Eds.). (2022). *HTTP semantics* (RFC 9110). Internet Engineering Task Force. https://doi.org/10.17487/RFC9110 @@ -53,9 +60,13 @@ GitHub. (n.d.). *Dependency graph*. GitHub Docs. Retrieved August 9, 2026, from GitHub. (n.d.). *Webhook events and payloads*. GitHub Docs. Retrieved August 16, 2026, from https://docs.github.com/en/webhooks/webhook-events-and-payloads#repository -MITRE. (n.d.). *CWE-20: Improper input validation*. Retrieved August 16, +MITRE. (n.d.-a). *CWE-20: Improper input validation*. Retrieved August 16, 2026, from https://cwe.mitre.org/data/definitions/20.html +MITRE. (n.d.-b). *CWE-22: Improper limitation of a pathname to a restricted +directory ('Path Traversal')*. Retrieved August 16, 2026, from +https://cwe.mitre.org/data/definitions/22.html + 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 diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index bd533f4de..3dd1d570a 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -136,6 +136,28 @@ def assert_dependency_review_probe_failed( assert not (tmp_path / "github-output").exists() +def assert_dependency_review_identity_rejected( + result: subprocess.CompletedProcess, + tmp_path: Path, + *, + expected_visibility: str = "public", + forbidden_substrings: tuple[str, ...] = (), +) -> None: + """Require identity rejection before curl, without echoing raw values.""" + + combined = f"{result.stdout}{result.stderr}" + assert result.returncode == 1 + assert "HTTP unavailable; curl exit uncalled" in result.stdout + assert f"visibility {expected_visibility}" in result.stdout + assert PROBE_TOKEN not in combined + assert "Authorization:" not in combined + assert not (tmp_path / "github-output").exists() + assert not (tmp_path / "curl-argv").exists() + for needle in forbidden_substrings: + assert needle + assert needle not in combined + + def test_merge_scheduler_dispatches_one_review_by_default() -> None: workflow = workflow_text("pr-review-merge-scheduler.yml") @@ -955,8 +977,22 @@ def test_security_scan_fails_closed_when_dependency_review_is_unavailable() -> N assert "visibility ${visibility}" in support_probe assert 'revision_pattern=' in support_probe assert 'repository_pattern=' in support_probe + assert "repository_owner=" in support_probe + assert "repository_name=" in support_probe + assert '[ "${repository_owner}" = ".." ]' in support_probe + assert '[ "${repository_name}" = ".." ]' in support_probe assert "Malformed revision" in support_probe assert "Malformed repository" in support_probe + assert "Named refs are not evidence" in support_probe + assert '000|"") http_status="unavailable"' in support_probe + assert ( + "Dependency review evidence unavailable for the allowlisted repository" + in support_probe + ) + assert "at exact base ${BASE_SHA} and head ${HEAD_SHA}" not in support_probe.split( + "set +e", + 1, + )[0] assert 'if [ "$curl_status" -ne 0 ] || [ "$http_status" != "200" ]; then' in workflow assert "--connect-timeout 10" in workflow assert "--max-time 30" in workflow @@ -1052,6 +1088,7 @@ def test_dependency_review_transport_failure_fails_closed(tmp_path: Path) -> Non [ ("#!/usr/bin/env bash\nprintf '404'\nexit 0\n", "404"), ("#!/usr/bin/env bash\nprintf ''\nexit 0\n", "unavailable"), + ("#!/usr/bin/env bash\nprintf '000'\nexit 0\n", "unavailable"), ("#!/usr/bin/env bash\nprintf 'OK'\nexit 0\n", "malformed"), ], ) @@ -1183,16 +1220,37 @@ def test_dependency_review_rejects_malformed_revision_before_network( head_sha=head_sha, ) - assert_dependency_review_probe_failed( + forbidden = tuple( + value + for value in (base_sha, head_sha) + if value and value not in {PROBE_BASE_SHA, PROBE_HEAD_SHA} + ) + assert_dependency_review_identity_rejected( result, tmp_path, - expected_http="unavailable", - expected_curl_exit="uncalled", - expected_base_sha=base_sha, - expected_head_sha=head_sha, + forbidden_substrings=forbidden, + ) + assert "Malformed revision" in result.stdout + assert "Named refs are not evidence" in result.stdout + + +def test_dependency_review_named_head_revision_cannot_go_green( + tmp_path: Path, +) -> None: + """GitHub resolves named revisions to moving HEADs; that is not evidence.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '200'\nexit 0\n", + head_sha="main", + ) + + assert_dependency_review_identity_rejected( + result, + tmp_path, + forbidden_substrings=("main",), ) assert "Malformed revision" in result.stdout - assert not (tmp_path / "curl-argv").exists() def test_dependency_review_rejects_malformed_repository_before_network( @@ -1200,21 +1258,66 @@ def test_dependency_review_rejects_malformed_repository_before_network( ) -> None: """A repository name that cannot form a compare URL must not reach curl.""" + raw_repository = "../evil/repo" result = run_dependency_review_support_probe( tmp_path, curl_script="#!/usr/bin/env bash\nprintf '200'\nexit 0\n", - repository="../evil/repo", + repository=raw_repository, ) - assert_dependency_review_probe_failed( + assert_dependency_review_identity_rejected( result, tmp_path, - expected_http="unavailable", - expected_curl_exit="uncalled", - expected_visibility="public", + forbidden_substrings=(raw_repository, "../evil"), ) assert "Malformed repository" in result.stdout - assert not (tmp_path / "curl-argv").exists() + + +@pytest.mark.parametrize( + "repository", + [ + "ContextualWisdomLab/..", + "../.github", + "ContextualWisdomLab/.", + "./.github", + ], +) +def test_dependency_review_rejects_dot_repository_segments( + tmp_path: Path, + repository: str, +) -> None: + """`.` and `..` path segments must not reach the compare URL.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '200'\nexit 0\n", + repository=repository, + ) + + assert_dependency_review_identity_rejected( + result, + tmp_path, + forbidden_substrings=(repository,), + ) + assert "Malformed repository" in result.stdout + + +def test_dependency_review_accepts_dot_github_repository_name( + tmp_path: Path, +) -> None: + """The special `.github` repository name is canonical owner/name, not `..`.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '200'\nexit 0\n", + repository="ContextualWisdomLab/.github", + ) + + assert result.returncode == 0 + assert (tmp_path / "github-output").read_text(encoding="utf-8") == ( + "supported=true\n" + ) + assert_dependency_review_compare_invoked(tmp_path) def test_security_scan_allows_repositories_without_supported_lockfiles() -> None: