From 640b37d811a39587d49ef76eb2e1635fb7658f13 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 17 Aug 2026 00:19:01 +0900 Subject: [PATCH 01/13] 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 c3b8fa5dbb..1511c4e066 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 fd1aebf43f..2ea797f4fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,8 @@ Semantic Versioning where the repository publishes a release. ### Security - Keep the Quarantine Sandbox Runtime caller read-only and model-secret-free, grant only job-scoped OIDC to the reusable scheduler, and preserve the product boundary in which the sandbox returns artifact-analysis evidence while hosts retain WAF/IDS, admission, final verdict, incident, and retention authority. +- 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 0000000000..81681d3f0c --- /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 535fd513a3..70fd270e52 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -839,16 +839,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 e4f1a2de880d5de53b2281e1e64b2bd9a625e148 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 15:35:33 +0000 Subject: [PATCH 02/13] 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 1511c4e066..5199bd52f8 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 bd6a96a11f..6aab567ee7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,3 +7,4 @@ Materialize accepts only exact SHA-256 pins or a bounded relative `-r` include ( Conflict-scope roots fail closed when the immediate parent directory is a symbolic link. OriginWeave hourly NVIDIA NIM repair is a thin caller at minute 10. See [`docs/doctoring/originweave-hourly-review-caller.md`](docs/doctoring/originweave-hourly-review-caller.md). nonnest2 hourly NVIDIA NIM repair is a thin caller at minute 16. See [`docs/doctoring/nonnest2-hourly-review-caller.md`](docs/doctoring/nonnest2-hourly-review-caller.md). +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 3e2e70b58e..eb9954f846 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -104,6 +104,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 2ea797f4fe..f5b0944ad8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,7 +52,7 @@ Semantic Versioning where the repository publishes a release. ### Security - Keep the Quarantine Sandbox Runtime caller read-only and model-secret-free, grant only job-scoped OIDC to the reusable scheduler, and preserve the product boundary in which the sandbox returns artifact-analysis evidence while hosts retain WAF/IDS, admission, final verdict, incident, and retention authority. -- 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 81681d3f0c..dc3a4c9e1e 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 70fd270e52..5d48e532ab 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") @@ -842,11 +885,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 @@ -858,8 +911,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@" ) @@ -868,42 +921,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 a70e5dc4a7323cbd09d73e30392a4cd5124afc21 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 15:54:55 +0000 Subject: [PATCH 03/13] 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 dc3a4c9e1e..521ab01c8d 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 5d48e532ab..1ce445c9f6 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") @@ -911,9 +936,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( @@ -926,16 +985,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"), @@ -946,17 +1022,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: @@ -967,10 +1045,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( @@ -984,9 +1064,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 fa63c43b914d6c20befcdbaa715e02bf87713f10 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 15:56:21 +0000 Subject: [PATCH 04/13] 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 1ce445c9f6..6d75dbdeaa 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 b8276c6548cbdf085accc1f01aa1618a1bdc74bd Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:00:51 +0000 Subject: [PATCH 05/13] fix(security): treat curl 000 as unavailable dependency-review evidence Classify curl's no-status 000 write-out as unavailable instead of a three-digit HTTP code, and lock private/internal visibility plus raw visibility non-leakage with executable regressions. Co-authored-by: Seongho Bae --- .github/workflows/security-scan.yml | 2 +- ARCHITECTURE.md | 6 +- CHANGELOG.md | 2 +- .../dependency-review-fail-closed.md | 12 ++-- .../test_required_workflow_queue_contract.py | 65 +++++++++++++++++++ 5 files changed, 77 insertions(+), 10 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 5199bd52f8..76dd06ae93 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -294,8 +294,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/ARCHITECTURE.md b/ARCHITECTURE.md index eb9954f846..1345f71921 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -109,9 +109,9 @@ sequenceDiagram 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. +`403`, `404`, timeout, truncated transfer, curl `000` sentinel, 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 diff --git a/CHANGELOG.md b/CHANGELOG.md index f5b0944ad8..a0543074a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,7 +52,7 @@ Semantic Versioning where the repository publishes a release. ### Security - Keep the Quarantine Sandbox Runtime caller read-only and model-secret-free, grant only job-scoped OIDC to the reusable scheduler, and preserve the product boundary in which the sandbox returns artifact-analysis evidence while hosts retain WAF/IDS, admission, final verdict, incident, and retention authority. -- 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. +- 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 or raw visibility strings. - 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 521ab01c8d..778641f534 100644 --- a/docs/doctoring/dependency-review-fail-closed.md +++ b/docs/doctoring/dependency-review-fail-closed.md @@ -4,11 +4,11 @@ Status: `active_pr` until the matching workflow and regression contract are pres ## 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. +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, curl `000` sentinel, 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. -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). curl's `%{http_code}` write-out is the numeric status from the last retrieved transfer; when no HTTP status was received it emits `000` (Stenberg, n.d.). That sentinel is unavailable evidence, not an HTTP status. 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. GitHub documents `403` as the private-repository response when GitHub Advanced Security is not enabled, or when the comparison targets a fork (GitHub, n.d.). Record the allowlisted visibility and exact revisions, then verify dependency-graph or Advanced Security configuration. Do not infer `not-applicable` from `403`. ## Identity and authority @@ -19,9 +19,9 @@ 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, 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`. +- 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. curl `000` is recorded as `unavailable`. 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. +- HTTP `403` on a private or internal repository: verify whether GitHub Advanced Security / dependency review is entitled for that repository. Keep the job failed until a separately reviewed organization exception with compensating controls exists. 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. @@ -42,12 +42,14 @@ Fielding, R., Nottingham, M., & Reschke, J. (Eds.). (2022). *HTTP semantics* 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.). *REST API endpoints for dependency review*. GitHub Docs. Retrieved August 16, 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 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 +Stenberg, D. (n.d.). *curl -- write out variables*. curl. Retrieved August 16, 2026, from https://curl.se/docs/manpage.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 6d75dbdeaa..d37ab6787d 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -927,6 +927,7 @@ 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 '000|"") http_status="unavailable"' 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 @@ -1075,6 +1076,70 @@ def test_dependency_review_records_unknown_visibility_when_unset( ) +@pytest.mark.parametrize("visibility", ["private", "internal"]) +def test_dependency_review_records_allowlisted_visibility( + tmp_path: Path, + visibility: str, +) -> None: + """Private and internal 403s must keep the allowlisted visibility label.""" + + 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, + ) + + +def test_dependency_review_does_not_echo_raw_visibility( + tmp_path: Path, +) -> None: + """Untrusted visibility strings must not appear in probe diagnostics.""" + + raw_visibility = "Public; curl https://evil.example" + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '403'\nexit 0\n", + repository_visibility=raw_visibility, + ) + + assert_dependency_review_probe_failed( + result, + tmp_path, + expected_http="403", + expected_curl_exit="0", + expected_visibility="unknown", + ) + combined = f"{result.stdout}{result.stderr}" + assert raw_visibility not in combined + assert "evil.example" not in combined + + +def test_dependency_review_curl_000_status_is_unavailable( + tmp_path: Path, +) -> None: + """curl's 000 write-out sentinel is not a completed HTTP exchange.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '000'\nexit 6\n", + ) + + assert_dependency_review_probe_failed( + result, + tmp_path, + expected_http="unavailable", + expected_curl_exit="6", + ) + + def test_security_scan_allows_repositories_without_supported_lockfiles() -> None: workflow = workflow_text("security-scan.yml") From d4ab623d1d456472b1b7e54954b8888d22bb1b6d Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:07:18 +0000 Subject: [PATCH 06/13] fix(security): reject named refs before dependency-review compare GitHub's compare API resolves named revisions to moving HEADs. Require exact 40- or 64-character hexadecimal object IDs and an owner/name repository identity before any compare request. Co-authored-by: Seongho Bae --- .github/workflows/security-scan.yml | 10 ++ AGENTS.md | 2 +- ARCHITECTURE.md | 10 +- CHANGELOG.md | 1 + .../dependency-review-fail-closed.md | 24 ++- .../test_required_workflow_queue_contract.py | 137 +++++++++++++++++- 6 files changed, 171 insertions(+), 13 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 76dd06ae93..44b2d3dd14 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -280,6 +280,16 @@ jobs: public|private|internal) visibility="${REPOSITORY_VISIBILITY}" ;; *) visibility="unknown" ;; esac + git_object_id='^[0-9a-f]{40}([0-9a-f]{24})?$' + repository_identity='^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$' + if ! [[ "${BASE_SHA}" =~ $git_object_id ]] || ! [[ "${HEAD_SHA}" =~ $git_object_id ]]; then + echo "::error::Dependency review evidence unavailable for the allowlisted repository (visibility ${visibility}): exact 40- or 64-character hexadecimal base and head revisions are required before any compare request. Named refs are not evidence. Verify the pull-request event SHAs, then rerun. Failing closed." + exit 1 + fi + if ! [[ "${REPOSITORY}" =~ $repository_identity ]]; then + echo "::error::Dependency review evidence unavailable (visibility ${visibility}): owner/name repository identity is required before any compare request. Verify the pull-request repository, then rerun. Failing closed." + exit 1 + fi set +e status="$( curl -sS --connect-timeout 10 --max-time 30 \ diff --git a/AGENTS.md b/AGENTS.md index 6aab567ee7..3b08e0daa8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,4 +7,4 @@ Materialize accepts only exact SHA-256 pins or a bounded relative `-r` include ( Conflict-scope roots fail closed when the immediate parent directory is a symbolic link. OriginWeave hourly NVIDIA NIM repair is a thin caller at minute 10. See [`docs/doctoring/originweave-hourly-review-caller.md`](docs/doctoring/originweave-hourly-review-caller.md). nonnest2 hourly NVIDIA NIM repair is a thin caller at minute 16. See [`docs/doctoring/nonnest2-hourly-review-caller.md`](docs/doctoring/nonnest2-hourly-review-caller.md). -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 non-`owner/name` repository values are rejected before the compare request. See [`docs/doctoring/dependency-review-fail-closed.md`](docs/doctoring/dependency-review-fail-closed.md). diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 1345f71921..f64e5ef828 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -107,19 +107,25 @@ 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 +comparison as a hard supply-chain evidence boundary. The probe rejects named +refs and non-`owner/name` repository values before it calls the compare API, +because GitHub would otherwise resolve `main` to a moving HEAD. 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 exact revisions. Other scanners are complementary; they are not substitutes. ```mermaid flowchart TD + Identity{"owner/name and 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"] + Identity -->|"yes"| Probe + Identity -->|"no"| Fail Probe --> Transport Transport -->|"yes"| Action Transport -->|"no"| Fail diff --git a/CHANGELOG.md b/CHANGELOG.md index a0543074a8..6a459d747e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ Semantic Versioning where the repository publishes a release. - Keep the Quarantine Sandbox Runtime caller read-only and model-secret-free, grant only job-scoped OIDC to the reusable scheduler, and preserve the product boundary in which the sandbox returns artifact-analysis evidence while hosts retain WAF/IDS, admission, final verdict, incident, and retention authority. - 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 or raw visibility strings. +- Reject named Git revisions and non-`owner/name` repository values before the dependency-review compare request so GitHub cannot resolve `main` to a moving HEAD and so path injection cannot reach the compare URL. - 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 778641f534..1cfc169828 100644 --- a/docs/doctoring/dependency-review-fail-closed.md +++ b/docs/doctoring/dependency-review-fail-closed.md @@ -4,20 +4,23 @@ Status: `active_pr` until the matching workflow and regression contract are pres ## 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, curl `000` sentinel, timeout, transport failure, truncated exchange, or other unexpected outcome is unavailable evidence and fails closed. +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`, `400`, `500`, `503`, empty or malformed status, curl `000` sentinel, 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 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, raw untrusted visibility strings, named refs, and raw invalid repository paths are never diagnostic output. After a successful probe the pinned action is not independently skippable. + +Before any compare request, the probe requires an `owner/name` repository identity and exact Git object IDs: 40 hexadecimal characters for SHA-1 or 64 hexadecimal characters for SHA-256 (Chacon & Straub, 2014; National Institute of Standards and Technology, 2015). GitHub's compare API resolves named revisions such as `main` to the current HEAD of that name (GitHub, n.d.). That moving target is not the pull-request head and is unavailable evidence. Rejecting it before interpolation also prevents path injection into `/repos/{owner}/{repo}/dependency-graph/compare/{basehead}` (MITRE, 2026). 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). curl's `%{http_code}` write-out is the numeric status from the last retrieved transfer; when no HTTP status was received it emits `000` (Stenberg, n.d.). That sentinel is unavailable evidence, not an HTTP status. 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. GitHub documents `403` as the private-repository response when GitHub Advanced Security is not enabled, or when the comparison targets a fork (GitHub, n.d.). Record the allowlisted visibility and exact revisions, then verify dependency-graph or Advanced Security configuration. Do not infer `not-applicable` from `403`. ## 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. +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 Git object IDs after those values pass the hexadecimal length check. 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 +- Identity rejected (named ref, empty or non-hex revision, or non-`owner/name` repository): fail the job before curl. Use the pull-request event's exact hexadecimal SHAs and `owner/name`, then rerun. - Transport exit `0` plus HTTP `200`: proceed to the pinned dependency-review action. - 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. curl `000` is recorded as `unavailable`. 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. @@ -31,12 +34,15 @@ 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 named refs and non-`owner/name` repository values before any compare request, 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. +Rollback requires an independently reviewed revert and fresh exact-head evidence. A rollback must not restore the `403`/`404` success path, accept named revisions as compare evidence, or print an API response body. ## References +Chacon, S., & Straub, B. (2014). *Pro Git* (2nd ed.). Apress. +https://git-scm.com/book/en/v2/Git-Internals-Git-Objects + Fielding, R., Nottingham, M., & Reschke, J. (Eds.). (2022). *HTTP semantics* (RFC 9110). Internet Engineering Task Force. https://doi.org/10.17487/RFC9110 @@ -48,7 +54,11 @@ 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 -Stenberg, D. (n.d.). *curl -- write out variables*. curl. Retrieved August 16, 2026, from https://curl.se/docs/manpage.html +MITRE. (2026). *CWE-20: Improper input validation*. +https://cwe.mitre.org/data/definitions/20.html + +National Institute of Standards and Technology. (2015). *Secure hash +standard (SHS)* (FIPS 180-4). https://doi.org/10.6028/NIST.FIPS.180-4 National Institute of Standards and Technology. (2020). *Security and privacy controls for information systems and organizations* (NIST SP @@ -56,3 +66,5 @@ privacy controls for information systems and organizations* (NIST SP SLSA. (2023). *SLSA v1.0: Supply-chain Levels for Software Artifacts*. Open Source Security Foundation. https://slsa.dev/spec/v1.0/ + +Stenberg, D. (n.d.). *curl -- write out variables*. curl. Retrieved August 16, 2026, from https://curl.se/docs/manpage.html diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index d37ab6787d..4f927854a7 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -37,12 +37,18 @@ def run_dependency_review_support_probe( *, curl_script: str, repository_visibility: str = "public", + base_sha: str = PROBE_BASE_SHA, + head_sha: str = PROBE_HEAD_SHA, + repository: str = "ContextualWisdomLab/.github", ) -> 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. + synthetic environment variables the support step reads. Optional SHA + and repository overrides let identity-validation regressions prove + that a named ref or path-injection value cannot reach the compare + request. """ fake_bin = tmp_path / "bin" @@ -66,9 +72,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, @@ -928,6 +934,21 @@ 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 '000|"") http_status="unavailable"' in support_probe + assert '^[0-9a-f]{40}([0-9a-f]{24})?$' in support_probe + assert '^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$' in support_probe + assert support_probe.index("^[0-9a-f]{40}") < support_probe.index("curl -sS") + assert support_probe.index("^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$") < support_probe.index( + "curl -sS" + ) + assert "continue-on-error:" not in support_probe + action_lines = [] + for line in workflow.split(" - name: Dependency review\n", 1)[1].splitlines(): + if line and not line.startswith(" "): + break + action_lines.append(line) + action_step = "\n".join(action_lines) + assert "if:" not in action_step + assert "continue-on-error:" not in action_step assert 'if [ "$curl_status" -ne 0 ] || [ "$http_status" != "200" ]; then' in workflow assert "--connect-timeout 10" in workflow assert "--max-time 30" in workflow @@ -1140,6 +1161,114 @@ def test_dependency_review_curl_000_status_is_unavailable( ) +def test_dependency_review_curl_000_with_exit_0_is_unavailable( + tmp_path: Path, +) -> None: + """A proxy that prints 000 and exits 0 still has no HTTP status.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script="#!/usr/bin/env bash\nprintf '000'\nexit 0\n", + ) + + assert_dependency_review_probe_failed( + result, + tmp_path, + expected_http="unavailable", + expected_curl_exit="0", + ) + + +def _successful_probe_curl_script(tmp_path: Path) -> str: + """Return a fake curl that would make the probe go green if reached.""" + + marker = tmp_path / "curl-invoked" + return ( + "#!/usr/bin/env bash\n" + f"printf 'invoked' >{shlex.quote(str(marker))}\n" + "printf '200'\n" + "exit 0\n" + ) + + +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=_successful_probe_curl_script(tmp_path), + head_sha="main", + ) + + combined = f"{result.stdout}{result.stderr}" + assert result.returncode == 1 + assert not (tmp_path / "curl-invoked").exists() + assert not (tmp_path / "github-output").exists() + assert "exact 40- or 64-character hexadecimal" in result.stdout + assert "main" not in combined + assert PROBE_TOKEN not in combined + + +def test_dependency_review_empty_base_sha_cannot_go_green( + tmp_path: Path, +) -> None: + """An empty base revision must not become a compare request.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script=_successful_probe_curl_script(tmp_path), + base_sha="", + ) + + assert result.returncode == 1 + assert not (tmp_path / "curl-invoked").exists() + assert not (tmp_path / "github-output").exists() + assert "exact 40- or 64-character hexadecimal" in result.stdout + + +def test_dependency_review_sha256_object_id_may_reach_compare( + tmp_path: Path, +) -> None: + """A 64-character Git object ID is exact evidence and may be compared.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script=_successful_probe_curl_script(tmp_path), + base_sha="c" * 64, + head_sha="d" * 64, + ) + + assert result.returncode == 0 + assert (tmp_path / "curl-invoked").exists() + assert (tmp_path / "github-output").read_text(encoding="utf-8") == ( + "supported=true\n" + ) + + +def test_dependency_review_rejects_repository_path_injection( + tmp_path: Path, +) -> None: + """owner/name is required before the compare path is interpolated.""" + + raw_repository = "ContextualWisdomLab/../evil" + result = run_dependency_review_support_probe( + tmp_path, + curl_script=_successful_probe_curl_script(tmp_path), + repository=raw_repository, + ) + + combined = f"{result.stdout}{result.stderr}" + assert result.returncode == 1 + assert not (tmp_path / "curl-invoked").exists() + assert not (tmp_path / "github-output").exists() + assert "owner/name repository identity" in result.stdout + assert raw_repository not in combined + assert "../evil" not in combined + assert PROBE_TOKEN not in combined + + def test_security_scan_allows_repositories_without_supported_lockfiles() -> None: workflow = workflow_text("security-scan.yml") From 948de32e869e1656e7ae1ba770b16c0b652f4c29 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 16 Aug 2026 16:12:51 +0000 Subject: [PATCH 07/13] fix(security): reject dot path components before dependency-review compare A single-slash owner/name whose owner or name is . or .. still matched the previous identity regex, so ../.github and ContextualWisdomLab/.. could reach curl. RFC 3986 remove-dot-segments would collapse those into a different compare URL. The organization .github repository remains a legal name. Related to #810. Prefer this branch over #1042 and #1048 for integration. Do not close #810 until an EgressWeave canary proves a non-200 cannot green the job. Co-authored-by: Seongho Bae --- .github/workflows/security-scan.yml | 6 ++ AGENTS.md | 2 +- ARCHITECTURE.md | 11 ++-- CHANGELOG.md | 2 +- .../dependency-review-fail-closed.md | 16 +++-- .../test_required_workflow_queue_contract.py | 61 +++++++++++++++++++ 6 files changed, 87 insertions(+), 11 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 44b2d3dd14..ac7eddb2ec 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -290,6 +290,12 @@ jobs: echo "::error::Dependency review evidence unavailable (visibility ${visibility}): owner/name repository identity is required before any compare request. Verify the pull-request repository, then rerun. Failing closed." exit 1 fi + repository_owner="${REPOSITORY%%/*}" + repository_name="${REPOSITORY#*/}" + if [ "${repository_owner}" = "." ] || [ "${repository_owner}" = ".." ] || [ "${repository_name}" = "." ] || [ "${repository_name}" = ".." ]; then + echo "::error::Dependency review evidence unavailable (visibility ${visibility}): owner/name repository identity is required before any compare request. Dot or parent-directory path components are not evidence. Verify the pull-request repository, then rerun. Failing closed." + exit 1 + fi set +e status="$( curl -sS --connect-timeout 10 --max-time 30 \ diff --git a/AGENTS.md b/AGENTS.md index 3b08e0daa8..30ebc46207 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -7,4 +7,4 @@ Materialize accepts only exact SHA-256 pins or a bounded relative `-r` include ( Conflict-scope roots fail closed when the immediate parent directory is a symbolic link. OriginWeave hourly NVIDIA NIM repair is a thin caller at minute 10. See [`docs/doctoring/originweave-hourly-review-caller.md`](docs/doctoring/originweave-hourly-review-caller.md). nonnest2 hourly NVIDIA NIM repair is a thin caller at minute 16. See [`docs/doctoring/nonnest2-hourly-review-caller.md`](docs/doctoring/nonnest2-hourly-review-caller.md). -Dependency-review unavailability fails closed; diagnostics include allowlisted repository visibility. Named refs and non-`owner/name` repository values are rejected before the compare request. 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, non-`owner/name` repository values, and `.`/`..` path components are rejected before the compare request. See [`docs/doctoring/dependency-review-fail-closed.md`](docs/doctoring/dependency-review-fail-closed.md). diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index f64e5ef828..7adfeeaeca 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -108,17 +108,18 @@ sequenceDiagram The central `Security Scan` job treats GitHub's exact `BASE_SHA...HEAD_SHA` comparison as a hard supply-chain evidence boundary. The probe rejects named -refs and non-`owner/name` repository values before it calls the compare API, -because GitHub would otherwise resolve `main` to a moving HEAD. 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 +refs, non-`owner/name` repository values, and `.`/`..` path components before +it calls the compare API, because GitHub would otherwise resolve `main` to a +moving HEAD and RFC 3986 would collapse `../.github` out of `/repos`. 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 exact revisions. Other scanners are complementary; they are not substitutes. ```mermaid flowchart TD - Identity{"owner/name and 40- or 64-hex SHAs?"} + Identity{"owner/name, no . or .., and 40- or 64-hex SHAs?"} Probe["Exact base/head compare probe"] Transport{"curl exit 0 and HTTP 200?"} Action["Pinned dependency-review action"] diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a459d747e..d8e6cc5f79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,7 +53,7 @@ Semantic Versioning where the repository publishes a release. - Keep the Quarantine Sandbox Runtime caller read-only and model-secret-free, grant only job-scoped OIDC to the reusable scheduler, and preserve the product boundary in which the sandbox returns artifact-analysis evidence while hosts retain WAF/IDS, admission, final verdict, incident, and retention authority. - 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 or raw visibility strings. -- Reject named Git revisions and non-`owner/name` repository values before the dependency-review compare request so GitHub cannot resolve `main` to a moving HEAD and so path injection cannot reach the compare URL. +- Reject named Git revisions, non-`owner/name` repository values, and `.`/`..` path components before the dependency-review compare request so GitHub cannot resolve `main` to a moving HEAD and so RFC 3986 remove-dot-segments cannot turn `../.github` into a compare URL. The organization `.github` special repository remains a legal name. - 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 1cfc169828..6d29a4e161 100644 --- a/docs/doctoring/dependency-review-fail-closed.md +++ b/docs/doctoring/dependency-review-fail-closed.md @@ -8,7 +8,7 @@ Dependency review is a hard supply-chain gate. The central workflow accepts only 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, raw untrusted visibility strings, named refs, and raw invalid repository paths are never diagnostic output. After a successful probe the pinned action is not independently skippable. -Before any compare request, the probe requires an `owner/name` repository identity and exact Git object IDs: 40 hexadecimal characters for SHA-1 or 64 hexadecimal characters for SHA-256 (Chacon & Straub, 2014; National Institute of Standards and Technology, 2015). GitHub's compare API resolves named revisions such as `main` to the current HEAD of that name (GitHub, n.d.). That moving target is not the pull-request head and is unavailable evidence. Rejecting it before interpolation also prevents path injection into `/repos/{owner}/{repo}/dependency-graph/compare/{basehead}` (MITRE, 2026). +Before any compare request, the probe requires an `owner/name` repository identity and exact Git object IDs: 40 hexadecimal characters for SHA-1 or 64 hexadecimal characters for SHA-256 (Chacon & Straub, 2014; National Institute of Standards and Technology, 2015). GitHub's compare API resolves named revisions such as `main` to the current HEAD of that name (GitHub, n.d.). That moving target is not the pull-request head and is unavailable evidence. A single-slash `owner/name` whose owner or name is `.` or `..` is still refused: RFC 3986 remove-dot-segments would turn `/repos/../.github/...` into `/.github/...` (Berners-Lee et al., 2005; MITRE, 2026a, 2026b). The organization `.github` special repository remains a legal name; only the path-segment sentinels are rejected. Rejecting these values before interpolation prevents path injection into `/repos/{owner}/{repo}/dependency-graph/compare/{basehead}`. 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). curl's `%{http_code}` write-out is the numeric status from the last retrieved transfer; when no HTTP status was received it emits `000` (Stenberg, n.d.). That sentinel is unavailable evidence, not an HTTP status. 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. GitHub documents `403` as the private-repository response when GitHub Advanced Security is not enabled, or when the comparison targets a fork (GitHub, n.d.). Record the allowlisted visibility and exact revisions, then verify dependency-graph or Advanced Security configuration. Do not infer `not-applicable` from `403`. @@ -20,7 +20,7 @@ Checks, status contexts, review submissions, and merge authorization remain sepa ## Failure classification and remediation -- Identity rejected (named ref, empty or non-hex revision, or non-`owner/name` repository): fail the job before curl. Use the pull-request event's exact hexadecimal SHAs and `owner/name`, then rerun. +- Identity rejected (named ref, empty or non-hex revision, non-`owner/name` repository, or a `.`/`..` path component): fail the job before curl. Use the pull-request event's exact hexadecimal SHAs and `owner/name`, then rerun. Do not retry a named ref or a dotted path component. - Transport exit `0` plus HTTP `200`: proceed to the pinned dependency-review action. - 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. curl `000` is recorded as `unavailable`. 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. @@ -34,12 +34,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, reject named refs and non-`owner/name` repository values before any compare request, 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, non-`owner/name` repository values, and `.`/`..` path components before any compare request, 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, accept named revisions as compare evidence, 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 + Chacon, S., & Straub, B. (2014). *Pro Git* (2nd ed.). Apress. https://git-scm.com/book/en/v2/Git-Internals-Git-Objects @@ -54,9 +58,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. (2026). *CWE-20: Improper input validation*. +MITRE. (2026a). *CWE-20: Improper input validation*. https://cwe.mitre.org/data/definitions/20.html +MITRE. (2026b). *CWE-22: Improper limitation of a pathname to a restricted +directory ('Path Traversal')*. +https://cwe.mitre.org/data/definitions/22.html + National Institute of Standards and Technology. (2015). *Secure hash standard (SHS)* (FIPS 180-4). https://doi.org/10.6028/NIST.FIPS.180-4 diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 4f927854a7..2e6f2be196 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -940,6 +940,13 @@ def test_security_scan_fails_closed_when_dependency_review_is_unavailable() -> N assert support_probe.index("^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$") < support_probe.index( "curl -sS" ) + assert 'repository_owner="${REPOSITORY%%/*}"' in support_probe + assert 'repository_name="${REPOSITORY#*/}"' in support_probe + assert '[ "${repository_owner}" = "." ]' in support_probe + assert '[ "${repository_name}" = ".." ]' in support_probe + assert support_probe.index('repository_owner="${REPOSITORY%%/*}"') < ( + support_probe.index("curl -sS") + ) assert "continue-on-error:" not in support_probe action_lines = [] for line in workflow.split(" - name: Dependency review\n", 1)[1].splitlines(): @@ -1269,6 +1276,60 @@ def test_dependency_review_rejects_repository_path_injection( assert PROBE_TOKEN not in combined +@pytest.mark.parametrize( + "raw_repository", + [ + "../.github", + "ContextualWisdomLab/..", + "ContextualWisdomLab/.", + "./.github", + ], +) +def test_dependency_review_rejects_dot_path_components( + tmp_path: Path, + raw_repository: str, +) -> None: + """A single-slash owner/name whose owner or name is ``.`` or ``..`` is still injection. + + RFC 3986 remove-dot-segments would turn ``/repos/../.github/...`` into + ``/.github/...`` (Berners-Lee et al., 2005). The ``.github`` product + repository must remain legal; only the path-segment sentinels are refused. + """ + + result = run_dependency_review_support_probe( + tmp_path, + curl_script=_successful_probe_curl_script(tmp_path), + repository=raw_repository, + ) + + combined = f"{result.stdout}{result.stderr}" + assert result.returncode == 1 + assert not (tmp_path / "curl-invoked").exists() + assert not (tmp_path / "github-output").exists() + assert "owner/name repository identity" in result.stdout + assert "Dot or parent-directory path components" in result.stdout + assert raw_repository not in combined + assert PROBE_TOKEN not in combined + + +def test_dependency_review_allows_dot_github_repository_name( + tmp_path: Path, +) -> None: + """The organization ``.github`` special repository remains a legal compare target.""" + + result = run_dependency_review_support_probe( + tmp_path, + curl_script=_successful_probe_curl_script(tmp_path), + repository="ContextualWisdomLab/.github", + ) + + assert result.returncode == 0 + assert (tmp_path / "curl-invoked").exists() + assert (tmp_path / "github-output").read_text(encoding="utf-8") == ( + "supported=true\n" + ) + + def test_security_scan_allows_repositories_without_supported_lockfiles() -> None: workflow = workflow_text("security-scan.yml") From ee5c15711f0b0a346bb19a634288a49fcd981fab Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 22 Aug 2026 04:25:21 +0900 Subject: [PATCH 08/13] ci: refresh pip audit runtime --- requirements-pip-audit-ci-hashes.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements-pip-audit-ci-hashes.txt b/requirements-pip-audit-ci-hashes.txt index ade197a49a..0ae099d8fe 100644 --- a/requirements-pip-audit-ci-hashes.txt +++ b/requirements-pip-audit-ci-hashes.txt @@ -213,9 +213,9 @@ packaging==26.2 \ # via # pip-audit # pip-requirements-parser -pip==26.1.2 \ - --hash=sha256:382ff9f685ee3bc25864f820aa50505825f10f5458ffff07e30a6d96e5715cab \ - --hash=sha256:f49cd134c61cf2fd75e0ce2676db03e4054504a5a4986d00f8299ae632dc4605 +pip==26.2.1 \ + --hash=sha256:71138adf1f4ca900cdb7d289c21b7494329f2332b6d85f0e1c42108c0384ed3e \ + --hash=sha256:f6ad667e89a1fe78046c8f13232b247200f5258d7828f3f7883d660878e0813f # via pip-api pip-api==0.0.34 \ --hash=sha256:8b2d7d7c37f2447373aa2cf8b1f60a2f2b27a84e1e9e0294a3f6ef10eb3ba6bb \ From 2a02647800ef4267bd97584f4bcc09f9a1086e11 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 04:53:38 +0900 Subject: [PATCH 09/13] test(security): reject dependency-review dot path identities --- ...y_review_repository_identity_regression.py | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 tests/test_dependency_review_repository_identity_regression.py diff --git a/tests/test_dependency_review_repository_identity_regression.py b/tests/test_dependency_review_repository_identity_regression.py new file mode 100644 index 0000000000..17dd316ab1 --- /dev/null +++ b/tests/test_dependency_review_repository_identity_regression.py @@ -0,0 +1,86 @@ +"""Regressions for dependency-review repository identity validation.""" + +from __future__ import annotations + +import os +import subprocess +import textwrap +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[1] + + +def _support_probe_script() -> str: + """Return the executable shell body of the dependency-review support probe.""" + workflow = (REPO_ROOT / ".github" / "workflows" / "security-scan.yml").read_text( + encoding="utf-8" + ) + step = " - name: Check dependency review support\n" + start = workflow.index(step) + end = workflow.index("\n - name:", start + len(step)) + block = workflow[start:end] + run_marker = " run: |\n" + run_start = block.index(run_marker) + len(run_marker) + return textwrap.dedent(block[run_start:]) + + +def _run_probe(tmp_path: Path, repository: str) -> tuple[subprocess.CompletedProcess[str], Path, Path]: + """Execute the probe with a fake curl and return process plus evidence paths.""" + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + curl_marker = tmp_path / "curl-called" + fake_curl = fake_bin / "curl" + fake_curl.write_text( + "#!/usr/bin/env bash\n" + "set -euo pipefail\n" + "printf 'called\\n' >\"${CURL_MARKER}\"\n" + "printf '200'\n", + encoding="utf-8", + ) + fake_curl.chmod(0o755) + output = tmp_path / "github-output" + env = os.environ.copy() + env.update( + { + "PATH": f"{fake_bin}:{env.get('PATH', '')}", + "GH_TOKEN": "test-token", + "BASE_SHA": "a" * 40, + "HEAD_SHA": "b" * 40, + "REPOSITORY": repository, + "REPOSITORY_VISIBILITY": "public", + "GITHUB_API_URL": "https://api.github.invalid", + "GITHUB_OUTPUT": str(output), + "CURL_MARKER": str(curl_marker), + } + ) + result = subprocess.run( + ["bash", "-c", _support_probe_script()], + cwd=REPO_ROOT, + env=env, + text=True, + capture_output=True, + check=False, + ) + return result, curl_marker, output + + +def test_dependency_review_rejects_dot_path_components_before_curl(tmp_path: Path) -> None: + """Reject dot-segment repository identities before any authenticated request.""" + for index, repository in enumerate( + ("../.github", "ContextualWisdomLab/..", "ContextualWisdomLab/.", "./.github") + ): + case_dir = tmp_path / str(index) + case_dir.mkdir() + result, curl_marker, _output = _run_probe(case_dir, repository) + assert result.returncode != 0, repository + assert not curl_marker.exists(), repository + assert "repository identity" in result.stdout.lower(), repository + + +def test_dependency_review_allows_dotgithub_product_repository(tmp_path: Path) -> None: + """Keep the organization .github product name valid while rejecting sentinels.""" + result, curl_marker, output = _run_probe(tmp_path, "ContextualWisdomLab/.github") + assert result.returncode == 0, result.stdout + result.stderr + assert curl_marker.exists() + assert output.read_text(encoding="utf-8") == "supported=true\n" From 6e72fa6407c6d1515191a771542389cfabd08728 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 04:55:26 +0900 Subject: [PATCH 10/13] test(security): cover dependency-review immutable identities --- ...y_review_repository_identity_regression.py | 46 +++++++++++++++++-- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/tests/test_dependency_review_repository_identity_regression.py b/tests/test_dependency_review_repository_identity_regression.py index 17dd316ab1..0fb54c182f 100644 --- a/tests/test_dependency_review_repository_identity_regression.py +++ b/tests/test_dependency_review_repository_identity_regression.py @@ -1,4 +1,4 @@ -"""Regressions for dependency-review repository identity validation.""" +"""Regressions for dependency-review immutable identity validation.""" from __future__ import annotations @@ -25,7 +25,13 @@ def _support_probe_script() -> str: return textwrap.dedent(block[run_start:]) -def _run_probe(tmp_path: Path, repository: str) -> tuple[subprocess.CompletedProcess[str], Path, Path]: +def _run_probe( + tmp_path: Path, + repository: str, + *, + base_sha: str = "a" * 40, + head_sha: str = "b" * 40, +) -> tuple[subprocess.CompletedProcess[str], Path, Path]: """Execute the probe with a fake curl and return process plus evidence paths.""" fake_bin = tmp_path / "bin" fake_bin.mkdir() @@ -45,8 +51,8 @@ def _run_probe(tmp_path: Path, repository: str) -> tuple[subprocess.CompletedPro { "PATH": f"{fake_bin}:{env.get('PATH', '')}", "GH_TOKEN": "test-token", - "BASE_SHA": "a" * 40, - "HEAD_SHA": "b" * 40, + "BASE_SHA": base_sha, + "HEAD_SHA": head_sha, "REPOSITORY": repository, "REPOSITORY_VISIBILITY": "public", "GITHUB_API_URL": "https://api.github.invalid", @@ -70,7 +76,7 @@ def test_dependency_review_rejects_dot_path_components_before_curl(tmp_path: Pat for index, repository in enumerate( ("../.github", "ContextualWisdomLab/..", "ContextualWisdomLab/.", "./.github") ): - case_dir = tmp_path / str(index) + case_dir = tmp_path / f"dot-{index}" case_dir.mkdir() result, curl_marker, _output = _run_probe(case_dir, repository) assert result.returncode != 0, repository @@ -78,6 +84,36 @@ def test_dependency_review_rejects_dot_path_components_before_curl(tmp_path: Pat assert "repository identity" in result.stdout.lower(), repository +def test_dependency_review_rejects_non_owner_name_identity_before_curl(tmp_path: Path) -> None: + """Reject repository values that are not exactly one owner/name pair.""" + for index, repository in enumerate( + ("ContextualWisdomLab", "ContextualWisdomLab/Orgmetra/extra", "/Orgmetra") + ): + case_dir = tmp_path / f"shape-{index}" + case_dir.mkdir() + result, curl_marker, _output = _run_probe(case_dir, repository) + assert result.returncode != 0, repository + assert not curl_marker.exists(), repository + assert "repository identity" in result.stdout.lower(), repository + + +def test_dependency_review_rejects_named_revisions_before_curl(tmp_path: Path) -> None: + """Require immutable 40- or 64-hex Git object ids before comparison.""" + cases = (("main", "b" * 40), ("a" * 40, "develop"), ("a" * 39, "b" * 40)) + for index, (base_sha, head_sha) in enumerate(cases): + case_dir = tmp_path / f"revision-{index}" + case_dir.mkdir() + result, curl_marker, _output = _run_probe( + case_dir, + "ContextualWisdomLab/Orgmetra", + base_sha=base_sha, + head_sha=head_sha, + ) + assert result.returncode != 0, (base_sha, head_sha) + assert not curl_marker.exists(), (base_sha, head_sha) + assert "exact 40- or 64-character hexadecimal" in result.stdout.lower() + + def test_dependency_review_allows_dotgithub_product_repository(tmp_path: Path) -> None: """Keep the organization .github product name valid while rejecting sentinels.""" result, curl_marker, output = _run_probe(tmp_path, "ContextualWisdomLab/.github") From a7aadb9fdfdd9a6d49f2a2d4b750fec6426e884a Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 04:58:06 +0900 Subject: [PATCH 11/13] fix(security): validate immutable dependency-review identity --- .github/workflows/security-scan.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 940b688183..c944391b43 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -313,6 +313,23 @@ jobs: set -euo pipefail api_url="${GITHUB_API_URL:-https://api.github.com}" + git_object_id='^[0-9a-f]{40}([0-9a-f]{24})?$' + repository_identity='^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$' + if ! [[ "${BASE_SHA}" =~ $git_object_id ]] || ! [[ "${HEAD_SHA}" =~ $git_object_id ]]; then + echo "::error::Dependency review evidence unavailable: exact 40- or 64-character hexadecimal base and head revisions are required before any compare request. Named refs are not evidence. Verify the pull-request event SHAs, then rerun. Failing closed." + exit 1 + fi + if ! [[ "${REPOSITORY}" =~ $repository_identity ]]; then + echo "::error::Dependency review evidence unavailable: owner/name repository identity is required before any compare request. Verify the pull-request repository, then rerun. Failing closed." + exit 1 + fi + repository_owner="${REPOSITORY%%/*}" + repository_name="${REPOSITORY#*/}" + if [ "${repository_owner}" = "." ] || [ "${repository_owner}" = ".." ] || [ "${repository_name}" = "." ] || [ "${repository_name}" = ".." ]; then + echo "::error::Dependency review evidence unavailable: owner/name repository identity is required before any compare request. Dot or parent-directory path components are not evidence. Verify the pull-request repository, then rerun. Failing closed." + exit 1 + fi + set +e status="$( curl -sS --connect-timeout 10 --max-time 30 \ From c26c2f24172260b77428421efd1b838e303a97d3 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 04:58:59 +0900 Subject: [PATCH 12/13] docs(security): trace dependency-review identity boundary --- .../dependency-review-fail-closed.md | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/docs/doctoring/dependency-review-fail-closed.md b/docs/doctoring/dependency-review-fail-closed.md index 81681d3f0c..8c984183d1 100644 --- a/docs/doctoring/dependency-review-fail-closed.md +++ b/docs/doctoring/dependency-review-fail-closed.md @@ -6,20 +6,23 @@ 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. +Before any compare request, the support probe also validates the evidence identity. Base and head must be immutable 40- or 64-character hexadecimal Git object IDs, and the repository must be exactly one `owner/name` pair. The owner and name path components may not be the RFC 3986 dot-segment sentinels `.` or `..`; `ContextualWisdomLab/.github` remains valid because `.github` is an ordinary repository name, not a dot segment. This prevents a named ref or path-normalized repository value from changing what object the comparison actually addresses. + 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. +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 §5.2.4 defines dot-segment removal, so accepting `.` or `..` as a repository path component would make URL interpolation ambiguous even when a superficial single-slash shape check passes (Berners-Lee et al., 2005). 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. +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. Before URL construction, the workflow rejects named revisions, malformed object IDs, repository strings that are not exactly `owner/name`, and `.`/`..` path components. These checks are executable regressions: invalid identity must fail before the fake HTTP client is reached, while the `ContextualWisdomLab/.github` product repository must still reach an otherwise successful compare. -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. +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 +- Invalid evidence identity (named/non-hex revision, non-`owner/name` repository value, or a `.`/`..` component): fail before curl. Correct the event identity; do not retry a moving or path-normalized target. - 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 transport/status 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. @@ -27,24 +30,22 @@ Retries are operator-initiated only after the capability or service condition ch ## 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 named revisions and malformed repository identities before transport, and prove that only transport success plus HTTP `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. +Rollback requires an independently reviewed revert and fresh exact-head evidence. A rollback must not restore the `403`/`404` success path, accept moving/nonnormalized comparison identities, 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 +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 -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.). *Dependency review*. GitHub Docs. Retrieved September 2, 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.). *REST API endpoints for dependency review*. GitHub Docs. Retrieved September 2, 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 +GitHub. (n.d.). *Dependency graph*. GitHub Docs. Retrieved September 2, 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 +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/ +SLSA. (2023). *SLSA v1.0: Supply-chain Levels for Software Artifacts*. Open Source Security Foundation. https://slsa.dev/spec/v1.0/ From 09908aaf56e568420105b81434c6cdd147856657 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Wed, 2 Sep 2026 06:24:22 +0900 Subject: [PATCH 13/13] fix(security): preserve current-main tree during identity reconciliation --- .../workflows/pr-review-merge-scheduler.yml | 82 ++-- .../repository-metadata-reconcile.yml | 8 +- config/repository-label-taxonomy.json | 100 +++++ config/repository-metadata.json | 84 ++++ .../0019-cloudflare-pingora-edge-standard.md | 6 + ...epository-public-surface-reconciliation.md | 5 +- .../nvidia-nim-opencode-hotfix-retirement.md | 21 + .../pingora-documentation-image-evidence.md | 17 + docs/doctoring/queue-hygiene-live-ref-race.md | 25 ++ ...epository-public-surface-reconciliation.md | 14 +- docs/nvidia-nim-opencode-hotfix.md | 53 --- docs/policies/PINGORA_EDGE_POLICY.md | 11 +- docs/product-technical-gap-baseline.md | 1 + scripts/ci/pingora_edge_policy.py | 197 ++++++++-- scripts/ci/revalidate_queue_cancellation.sh | 178 +++++++++ tests/test_pingora_edge_policy.py | 188 ++++++++- ...queue_cancellation_open_pr_revalidation.py | 129 +++++++ tests/test_queue_cancellation_revalidation.py | 364 ++++++++++++++++++ ...t_queue_cancellation_scheduler_contract.py | 52 +++ tests/test_repository_label_taxonomy.py | 52 +++ ...test_repository_metadata_reconciliation.py | 16 +- tests/test_repository_metadata_workflow.py | 18 + 22 files changed, 1500 insertions(+), 121 deletions(-) create mode 100644 docs/doctoring/nvidia-nim-opencode-hotfix-retirement.md create mode 100644 docs/doctoring/pingora-documentation-image-evidence.md create mode 100644 docs/doctoring/queue-hygiene-live-ref-race.md delete mode 100644 docs/nvidia-nim-opencode-hotfix.md create mode 100755 scripts/ci/revalidate_queue_cancellation.sh create mode 100644 tests/test_queue_cancellation_open_pr_revalidation.py create mode 100644 tests/test_queue_cancellation_revalidation.py create mode 100644 tests/test_queue_cancellation_scheduler_contract.py create mode 100644 tests/test_repository_metadata_workflow.py diff --git a/.github/workflows/pr-review-merge-scheduler.yml b/.github/workflows/pr-review-merge-scheduler.yml index a15cdf36e1..fe5cf4206f 100644 --- a/.github/workflows/pr-review-merge-scheduler.yml +++ b/.github/workflows/pr-review-merge-scheduler.yml @@ -1087,37 +1087,40 @@ jobs: fi fi - # Queue hygiene, part 1: cancel every queued/in-progress PR run whose - # head SHA no longer matches its open PR's Current HEAD, plus default- - # branch push/schedule runs superseded by a newer default HEAD. PR - # concurrency normally does this on synchronize/close events, but it - # cannot repair runs left behind by an outage or a manual dispatch. - # Compare live refs on every sweep instead of waiting for an age - # threshold: previous-head checks are never useful merge evidence. + # Queue hygiene, part 1: classify queued/in-progress runs against a + # bounded PR/default-branch snapshot. The snapshot is intentionally + # cheap and may race with a subsequent head move; every destructive + # cancellation is therefore revalidated against live run/PR/ref state + # immediately before the mutation by the production helper below. queue_hygiene_ready=true - if ! open_pr_heads_json="$( + open_pr_heads_json="{}" + if open_pr_payload_json="$( gh api \ -H "Accept: application/vnd.github+json" \ "/repos/${repo_full_name}/pulls?state=open&per_page=100" \ --paginate \ - | jq -sc ' - add - | map( - select( - .head.repo.full_name != null and - .head.ref != null and - .head.sha != null - ) - | { - key: "\(.head.repo.full_name):\(.head.ref)", - value: .head.sha - } - ) - | from_entries - ' + | jq -sc '[.[] | .[]]' )"; then + if ! jq -e ' + all(.[]; + (.head.repo.full_name | type) == "string" and (.head.repo.full_name | length) > 0 and + (.head.ref | type) == "string" and (.head.ref | length) > 0 and + (.head.sha | type) == "string" and (.head.sha | test("^[0-9a-fA-F]{40}$")) + ) + ' <<<"$open_pr_payload_json" >/dev/null; then + echo "::warning::Current-HEAD cancellation skipped for ${repo_full_name}: an open PR has malformed head repository/ref/SHA metadata. No run will be cancelled from incomplete evidence." + queue_hygiene_ready=false + else + open_pr_heads_json="$( + jq -c ' + reduce .[] as $pr ({}; + . + {(($pr.head.repo.full_name + ":" + $pr.head.ref)): $pr.head.sha} + ) + ' <<<"$open_pr_payload_json" + )" + fi + else echo "::warning::Current-HEAD cancellation skipped for ${repo_full_name}: open PR head refs could not be read safely. No run will be cancelled from incomplete evidence." - open_pr_heads_json="{}" queue_hygiene_ready=false fi if ! current_default_sha="$( @@ -1129,6 +1132,10 @@ jobs: echo "::warning::Current-HEAD cancellation skipped for ${repo_full_name}: default-branch HEAD could not be read safely. No run will be cancelled from incomplete evidence." current_default_sha="" queue_hygiene_ready=false + elif ! [[ "$current_default_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + echo "::warning::Current-HEAD cancellation skipped for ${repo_full_name}: default-branch HEAD is malformed. No run will be cancelled from incomplete evidence." + current_default_sha="" + queue_hygiene_ready=false fi if ! active_runs_json="$( for active_status in queued in_progress; do @@ -1187,13 +1194,17 @@ jobs: fi superseded_count="$(jq 'length' <<<"$superseded_runs_json")" if [ "$superseded_count" -gt 0 ]; then - echo "Cancelling ${superseded_count} queued/in-progress run(s) that do not match an open PR or default-branch Current HEAD:" - jq -r '.[] | " run \(.id) [\(.name)] status=\(.status) event=\(.event) branch=\(.head_branch) run_head=\(.run_head) current_head=\(.current_head // "closed-or-no-open-pr")"' <<<"$superseded_runs_json" + echo "Revalidating ${superseded_count} queued/in-progress run(s) classified as not matching an open PR or default-branch Current HEAD:" + jq -r '.[] | " run \(.id) [\(.name)] status=\(.status) event=\(.event) branch=\(.head_branch) run_head=\(.run_head) classified_head=\(.current_head // "closed-or-no-open-pr")"' <<<"$superseded_runs_json" if [ "$DRY_RUN" != "true" ]; then while IFS= read -r run_id; do - if ! gh api -X POST "/repos/${repo_full_name}/actions/runs/${run_id}/cancel" >/dev/null; then - echo "Could not cancel superseded run ${run_id} in ${repo_full_name}; it may have finished already." - fi + scripts/ci/revalidate_queue_cancellation.sh \ + "$repo_full_name" \ + "$run_id" \ + "$default_branch" \ + "$current_default_sha" \ + "$open_pr_heads_json" \ + "superseded" done < <(jq -r '.[].id' <<<"$superseded_runs_json") fi fi @@ -1202,6 +1213,7 @@ jobs: # runs that are not tied to a currently open PR head. This catches # orphaned manual/workflow-chain runs without cancelling a valid # current-head PR check merely because runner capacity was scarce. + # The helper re-checks late PR association/live refs before mutation. stale_runs_json="[]" if [ "$queue_hygiene_ready" = "true" ]; then stale_cutoff="$(date -u -d "${ORG_SWEEP_STALE_QUEUE_HOURS} hours ago" +%Y-%m-%dT%H:%M:%SZ)" @@ -1224,13 +1236,17 @@ jobs: fi stale_count="$(jq 'length' <<<"$stale_runs_json")" if [ "$stale_count" -gt 0 ]; then - echo "Cancelling ${stale_count} queued run(s) older than ${ORG_SWEEP_STALE_QUEUE_HOURS}h:" + echo "Revalidating ${stale_count} queued run(s) older than ${ORG_SWEEP_STALE_QUEUE_HOURS}h:" jq -r '.[] | " run \(.id) [\(.name)] on \(.head_branch) queued since \(.created_at)"' <<<"$stale_runs_json" if [ "$DRY_RUN" != "true" ]; then while IFS= read -r run_id; do - if ! gh api -X POST "/repos/${repo_full_name}/actions/runs/${run_id}/cancel" >/dev/null; then - echo "Could not cancel run ${run_id} in ${repo_full_name}; it may have started or finished already." - fi + scripts/ci/revalidate_queue_cancellation.sh \ + "$repo_full_name" \ + "$run_id" \ + "$default_branch" \ + "$current_default_sha" \ + "$open_pr_heads_json" \ + "aged-orphan" done < <(jq -r '.[].id' <<<"$stale_runs_json") fi fi diff --git a/.github/workflows/repository-metadata-reconcile.yml b/.github/workflows/repository-metadata-reconcile.yml index 3bb9b6944d..e05a8b155a 100644 --- a/.github/workflows/repository-metadata-reconcile.yml +++ b/.github/workflows/repository-metadata-reconcile.yml @@ -11,6 +11,7 @@ on: - "tests/test_repository_metadata_convergence.py" - "tests/test_repository_metadata_identity.py" - "tests/test_repository_metadata_live_verification.py" + - "tests/test_repository_metadata_workflow.py" - "tests/test_repository_metadata_workflow_pages.py" - "tests/test_repository_label_taxonomy.py" - "tests/test_repository_label_reconciliation.py" @@ -122,9 +123,14 @@ jobs: uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.12" + - name: Require dedicated repository settings credential + env: + GH_TOKEN: ${{ secrets.CWL_REPOSITORY_METADATA_TOKEN }} + shell: bash + run: test -n "${GH_TOKEN}" - name: Reconcile and verify repository public surfaces env: - GH_TOKEN: ${{ secrets.PR_REVIEW_MERGE_TOKEN }} + GH_TOKEN: ${{ secrets.CWL_REPOSITORY_METADATA_TOKEN }} run: | set +e python scripts/ci/reconcile_repository_metadata.py \ diff --git a/config/repository-label-taxonomy.json b/config/repository-label-taxonomy.json index a1831221ed..0dd7ac6ee4 100644 --- a/config/repository-label-taxonomy.json +++ b/config/repository-label-taxonomy.json @@ -11,6 +11,21 @@ "issue": 1582, "type": "feature" }, + { + "repository": ".github", + "issue": 1622, + "type": "feature" + }, + { + "repository": ".github", + "issue": 1625, + "type": "bug" + }, + { + "repository": ".github", + "issue": 1634, + "type": "documentation" + }, { "repository": "CalendarWeave", "issue": 1, @@ -100,6 +115,91 @@ "repository": "noema", "issue": 530, "type": "feature" + }, + { + "repository": "bandscope", + "issue": 1125, + "type": "documentation" + }, + { + "repository": "saju-caldav", + "issue": 44, + "type": "documentation" + }, + { + "repository": "OriginWeave", + "issue": 274, + "type": "documentation" + }, + { + "repository": "semantic-data-portal", + "issue": 90, + "type": "documentation" + }, + { + "repository": "accounting-information-platform", + "issue": 45, + "type": "documentation" + }, + { + "repository": "clearfolio", + "issue": 538, + "type": "documentation" + }, + { + "repository": "pg-erd-cloud", + "issue": 1046, + "type": "documentation" + }, + { + "repository": "DiagramWeave", + "issue": 34, + "type": "documentation" + }, + { + "repository": "keyverse", + "issue": 127, + "type": "documentation" + }, + { + "repository": "mhtml-etl-gateway", + "issue": 56, + "type": "documentation" + }, + { + "repository": "j-planner", + "issue": 2, + "type": "documentation" + }, + { + "repository": "learning-record-store", + "issue": 1, + "type": "documentation" + }, + { + "repository": "learning-content-studio", + "issue": 1, + "type": "documentation" + }, + { + "repository": "learning-management-platform", + "issue": 1, + "type": "documentation" + }, + { + "repository": "metering-billing-platform", + "issue": 157, + "type": "documentation" + }, + { + "repository": "PolicyWeave", + "issue": 1, + "type": "feature" + }, + { + "repository": "supply-chain-control-plane", + "issue": 1, + "type": "feature" } ] } diff --git a/config/repository-metadata.json b/config/repository-metadata.json index fcf8471236..bb95527ee7 100644 --- a/config/repository-metadata.json +++ b/config/repository-metadata.json @@ -49,6 +49,90 @@ "topics": ["psychometrics", "assessment", "measurement", "longitudinal", "research", "privacy", "rust", "contextualwisdomlab"], "deepwiki": true, "pages": true + }, + "keyverse": { + "description": "Keyverse — passwordless identity, federation, provisioning, account unification, and authorization services for ContextualWisdomLab.", + "topics": ["identity", "openid-connect", "oauth2", "scim", "keycloak", "python", "contextualwisdomlab"], + "deepwiki": true, + "pages": true + }, + "OriginWeave": { + "description": "Let agents use the web without losing control. OriginWeave gives AI agents a Chromium-compatible web runtime with isolated sessions, typed actions, resource governance, and verifiable evidence.", + "topics": ["browser-automation", "ai-agents", "chromium", "security", "rust", "web", "contextualwisdomlab"], + "deepwiki": true, + "pages": true + }, + "accounting-information-platform": { + "description": "Accounting Information Platform — statutory accounting, journal posting, period control, reconciliation, and financial reporting authority for ContextualWisdomLab.", + "topics": ["accounting", "ledger", "journal", "reconciliation", "financial-reporting", "postgresql", "python", "contextualwisdomlab"], + "deepwiki": true, + "pages": true + }, + "pg-erd-cloud": { + "description": "PostgreSQL 스키마를 리버스 엔지니어링하고 ERD·DDL 공유 흐름으로 관리하는 클라우드 서비스.", + "topics": ["cloud", "database-schema", "ddl", "erd", "postgresql", "reverse-engineering", "saas", "python", "javascript", "contextualwisdomlab"], + "deepwiki": true, + "pages": true + }, + "clearfolio": { + "description": "Clearfolio — secure document conversion, tenant-scoped viewing, and controlled artifact delivery.", + "topics": ["document-viewer", "document-conversion", "file-preview", "pdf", "java", "spring-boot", "javascript", "web-app", "contextualwisdomlab"], + "deepwiki": true, + "pages": true + }, + "DiagramWeave": { + "description": "DiagramWeave — a source-first, AI-assisted editor and tooling platform for PlantUML diagrams.", + "topics": ["diagram-editor", "plantuml", "developer-tools", "language-server", "javascript", "ai-assisted", "contextualwisdomlab"], + "deepwiki": true, + "pages": true + }, + "semantic-data-portal": { + "description": "Semantic Data Portal — governed discovery, graph traversal, and semantic search for enterprise data catalogs.", + "topics": ["data-catalog", "knowledge-graph", "ontology", "semantic-web", "semantic-search", "data-governance", "postgresql", "python", "contextualwisdomlab"], + "deepwiki": true, + "pages": true + }, + "contextual-orchestrator": { + "description": "Contextual Orchestrator — an OpenAI-compatible control plane for model routing, delegation, verification, and multi-agent orchestration.", + "topics": ["enterprise-admin", "llm-orchestration", "model-orchestration", "model-routing", "ai-agents", "openai-compatible", "research", "python", "contextualwisdomlab"], + "deepwiki": true, + "pages": true + }, + "mhtml-etl-gateway": { + "description": "Enterprise MHTML ingestion gateway that converts browser, SAP ALV, and Excel Web Archive exports into governed PostgreSQL data assets.", + "topics": ["mhtml", "etl", "data-ingestion", "sap", "postgresql", "data-governance", "python", "contextualwisdomlab"], + "deepwiki": true, + "pages": true + }, + "PolicyWeave": { + "description": "PolicyWeave — local-first privacy-policy fact authoring, completeness review, and deterministic draft generation for web and app operators.", + "topics": ["privacy", "privacy-policy", "privacy-engineering", "policy-authoring", "local-first", "react", "typescript", "vite", "contextualwisdomlab"], + "deepwiki": true, + "pages": true + }, + "supply-chain-control-plane": { + "description": "Supply Chain Control Plane — evidence-backed supply-network dependency modeling and deterministic downstream disruption-impact analysis.", + "topics": ["supply-chain", "disruption-management", "dependency-graph", "provenance", "risk-analysis", "rust", "contextualwisdomlab"], + "deepwiki": true, + "pages": true + }, + "learning-management-platform": { + "description": "Learning Management Platform — enrollment, learning-journey, completion, and credential orchestration for employee and external learners.", + "topics": ["learning-management-system", "learning-platform", "enrollment", "completion", "credentialing", "rust", "postgresql", "contextualwisdomlab"], + "deepwiki": true, + "pages": true + }, + "learning-content-studio": { + "description": "Learning Content Studio — evidence-bound LCMS for authoring, approving, releasing, and deterministically publishing reusable learning content.", + "topics": ["lcms", "learning-content", "content-authoring", "content-management", "accessibility", "scorm", "cmi5", "rust", "contextualwisdomlab"], + "deepwiki": true, + "pages": true + }, + "learning-record-store": { + "description": "Authoritative xAPI learning-record persistence for the CWL Learning Platform.", + "topics": ["learning-record-store", "xapi", "cmi5", "learning-technology", "interoperability", "contextualwisdomlab"], + "deepwiki": true, + "pages": true } } } diff --git a/docs/adr/0019-cloudflare-pingora-edge-standard.md b/docs/adr/0019-cloudflare-pingora-edge-standard.md index 805e538b86..9f92f0f046 100644 --- a/docs/adr/0019-cloudflare-pingora-edge-standard.md +++ b/docs/adr/0019-cloudflare-pingora-edge-standard.md @@ -33,6 +33,12 @@ so a governed shared implementation is required. 6. Initial migration does not use Pingora's experimental cache integration. 7. PHP workloads move to an HTTP application server or reviewed FastCGI adapter behind Pingora before the public listener changes. +8. Documentation PNG screenshots and PDF papers without a text diff are verified + from bounded format evidence (a complete CRC-valid PNG chunk stream with + conforming chunk names, palette bounds, and palette indices whose bounded null- or + Adam7-interlaced decompressed scanlines match IHDR, or a PDF signature) and excluded + from runtime-content scanning; + runtime paths and malformed or unsupported binary evidence still fail closed. ## Consequences diff --git a/docs/adr/0020-repository-public-surface-reconciliation.md b/docs/adr/0020-repository-public-surface-reconciliation.md index c0c8dc650f..000bd05a19 100644 --- a/docs/adr/0020-repository-public-surface-reconciliation.md +++ b/docs/adr/0020-repository-public-surface-reconciliation.md @@ -19,7 +19,7 @@ The organization therefore needs one auditable owner for the desired state and o 5. DeepWiki README content is not mutated centrally. `deepwiki: true` requires the exact linked badge on the default branch before metadata writes; `deepwiki: false` fails closed while that exact badge is still present so desired state cannot silently contradict the public README. 6. Pages has two explicit ownership modes. Legacy mode requires the repository default branch to contain the regular file `docs/index.md`; absent legacy sites may be created at `/docs`, drifted legacy sites may be updated, and converged sites receive no write. Workflow mode requires the regular file `.github/workflows/pages.yml` on the protected default branch **and** an already-existing live Pages configuration with `build_type: workflow`. The central reconciler never creates or converts a workflow-backed site. Those workflow-mode source and live-configuration preconditions are validated before description, topic, or Pages mutation so an invalid workflow declaration cannot leave a partially applied metadata record. 7. Contents API source probes are type-aware. A successful response satisfies a required-source precondition only when the response is a single object with `type: file`; a directory object or directory listing is not accepted as reviewed file evidence. -8. Pull-request execution is read-only validation. Privileged reconciliation runs only from trusted `.github/main`, uses the existing maintainer credential, does not widen pull-request tokens, and does not bypass repository rulesets or reviews. +8. Pull-request execution is read-only validation. Privileged reconciliation runs only from trusted `.github/main` and obtains write authority only from the protected `repository-metadata-maintenance` environment's dedicated `CWL_REPOSITORY_METADATA_TOKEN`. The apply job fails before either mutation lane starts when that credential is absent. It must not fall back to `PR_REVIEW_MERGE_TOKEN`, reviewer/model/provider credentials, or a widened pull-request `GITHUB_TOKEN`, and it does not bypass repository rulesets or reviews. 9. Reconciliation runs from the trusted hourly schedule and exposes no branch-selectable `workflow_dispatch` entrypoint. Pull-request validation keeps a PR-stable concurrency lineage and cancels superseded validation runs; trusted scheduled protected-main apply remains non-cancellable so a replacement heartbeat cannot abandon a partially updated fleet. 10. Metadata and label lanes retain independent exit statuses during apply: label reconciliation still runs after an aggregated metadata failure, and the job fails afterward if either lane failed. 11. Repository-wide tests, focused 100% statement/branch coverage for both reconciliation scripts, docstring gates, manifest/taxonomy validation, and `git diff --check` are required before apply can run. @@ -32,11 +32,12 @@ The organization therefore needs one auditable owner for the desired state and o - Actions-backed Pages can be enrolled without silently rewriting a repository's reviewed deployment architecture to legacy `/docs`. - Workflow-mode failure is fail-before-write for the repository record: missing workflow source, missing Pages, or a non-workflow live build type prevents description/topic mutation as well as Pages mutation. - Explicit label assignments intentionally favor evidence over broad title heuristics. Expanding classification coverage requires a reviewed assignment or a separately justified deterministic classifier. -- The privileged token must retain only the repository-administration/Pages/issue permissions required by the declared fleet. Credential values never enter the manifest or logs. +- `CWL_REPOSITORY_METADATA_TOKEN` is a distinct least-privilege settings identity. It must retain only the repository-administration/Pages/issue permissions required by the declared fleet, remain unavailable to pull-request code and model processes, and never enter the manifest, logs, or artifacts. Removing it makes protected-main apply fail closed while read-only PR validation remains usable. ## Rejected alternatives - **Report missing connector mutations without repair.** Rejected because the organization owns a GitHub Actions/API control plane that can safely provide the capability. +- **Reuse `PR_REVIEW_MERGE_TOKEN` for repository settings writes.** Rejected because merge/review authority and organization-wide repository-settings authority are separate security capabilities; coupling them unnecessarily broadens blast radius and makes least-privilege revocation impossible. - **Mutate README badges from the central control plane.** Rejected because that would bypass the active product writer and make customer-facing content independent of product review. - **Convert workflow-backed Pages to legacy `/docs` for uniformity.** Rejected because deployment ownership is a reviewed product boundary; reconciliation must preserve an explicitly declared Actions-backed deployment rather than rewrite it. - **Treat any successful Contents API response as file evidence.** Rejected because a directory can exist at the same path and must not satisfy a regular-file precondition. diff --git a/docs/doctoring/nvidia-nim-opencode-hotfix-retirement.md b/docs/doctoring/nvidia-nim-opencode-hotfix-retirement.md new file mode 100644 index 0000000000..0036bf10c2 --- /dev/null +++ b/docs/doctoring/nvidia-nim-opencode-hotfix-retirement.md @@ -0,0 +1,21 @@ +# NVIDIA NIM OpenCode hotfix retirement + +## Decision + +The legacy direct-provider OpenCode hotfix is retired. Protected `main` now enables only the `contextual-orchestrator` provider in `opencode.jsonc`, with both normal and small-model review requests routed through `contextual-orchestrator/orchestrator/free`. Direct NVIDIA NIM provider selection is therefore not part of the OpenCode review contract. + +The removed `docs/nvidia-nim-opencode-hotfix.md` described a superseded architecture: direct `nvidia-nim` provider configuration, `NVIDIA_API_KEY` binding, and an administrator-bypass hotfix window. Keeping that document beside the current gateway-only configuration created an operational contradiction and could mislead a maintainer into restoring a retired direct-provider path. + +## Current authority boundary + +- `ContextualWisdomLab/.github` owns the review workflows and gateway integration. +- `opencode.jsonc` enables only `contextual-orchestrator` and denies direct-provider fallback. +- NVIDIA NIM credentials may be registered into contextual-orchestrator's provider-discovery boundary; they are not an OpenCode provider credential or a direct workflow model binding. +- The write-capable scheduled autofix path follows the same gateway-only boundary documented in `docs/doctoring/hourly-nvidia-nim-autofix.md` and ADR-0003. +- Queue-saturation administrator bypass, when separately proven under the current control-plane contract, is an admission-recovery mechanism and must not be documented as a provider-specific hotfix permission. + +## Verification + +This record was created from protected `main@81b6f20d7f701bd2e50642ab107ab0f187ae6dc9`. At that revision, `opencode.jsonc` declares `enabled_providers: ["contextual-orchestrator"]`, uses `contextual-orchestrator/orchestrator/free`, and contains no live `nvidia-nim` provider block. The existing `docs/doctoring/hourly-nvidia-nim-autofix.md` already records the corrected gateway-only provider contract. + +No runtime source, credential, model-selection rule, security threshold, branch-protection rule, or review authority is changed by this documentation cleanup. \ No newline at end of file diff --git a/docs/doctoring/pingora-documentation-image-evidence.md b/docs/doctoring/pingora-documentation-image-evidence.md new file mode 100644 index 0000000000..af10942cd8 --- /dev/null +++ b/docs/doctoring/pingora-documentation-image-evidence.md @@ -0,0 +1,17 @@ +# Pingora documentation image evidence + +The required Pingora gate previously sent a changed PNG screenshot through its +UTF-8 runtime-content decoder because GitHub omits text patches for binary files. +That rejected UI evidence before the policy could determine whether it described +an active edge runtime. + +ADR-0019 now admits documentation PNG screenshots only when the bounded final +file is a complete CRC-valid PNG chunk stream ending at IEND with no trailing +payload, conforming chunk names, palette bounds and indices, and bounded null- or +Adam7-interlaced decompressed scanlines that match IHDR. A signature or +CRC-valid arbitrary IDAT is insufficient. Files in a runtime path, malformed signatures, +unsupported binary formats, and unavailable evidence continue to fail closed. +The gate establishes bounded binary evidence rather than general image-rendering +fidelity; optional ancillary-chunk semantics are outside this policy boundary. +`tests/test_pingora_edge_policy.py` covers the accepted PNG and the existing fake +PDF/runtime cases; targeted branch coverage remains 100%. diff --git a/docs/doctoring/queue-hygiene-live-ref-race.md b/docs/doctoring/queue-hygiene-live-ref-race.md new file mode 100644 index 0000000000..029cdf31f3 --- /dev/null +++ b/docs/doctoring/queue-hygiene-live-ref-race.md @@ -0,0 +1,25 @@ +# Queue-hygiene live-ref race doctoring + +## Incident + +The organization queue sweep classified queued/in-progress Actions runs against a pull-request list snapshot and later cancelled the selected run IDs. A PR head can advance after that snapshot but before the destructive cancellation. GitHub's run and PR payloads may also lag the branch ref. Trusting either predecessor snapshot as final authority can therefore cancel the sole current-head review/check evidence and amplify Actions-capacity saturation. + +## Owner and boundary + +`ContextualWisdomLab/.github` owns this defect because the destructive organization queue hygiene and required review/merge scheduler are central control-plane behavior. Leaf repositories must not duplicate cancellation policy. The scheduler may use cheap PR payloads to classify candidates, but every destructive cancellation must revalidate the live run and its authoritative current ref immediately before the mutation. + +## Contract + +The repaired scheduler keeps a bounded initial snapshot and delegates every selected cancellation to `scripts/ci/revalidate_queue_cancellation.sh`. The helper fails closed when run/PR/ref evidence cannot be read or is malformed. For an attached PR it re-fetches the PR and resolves the head branch through the Git ref endpoint. For an Actions PR run whose `pull_requests` association is still empty, it re-fetches open PRs only to discover a matching head repository/ref and then resolves that branch ref; the payload SHA is explicitly non-authoritative. If the live ref equals the run head, the run is preserved. Default-branch push/schedule candidates are similarly revalidated against the live protected-branch head. + +The final design intentionally removes the earlier serial live-ref lookup for every open PR and its repository-wide lookup ceiling. Live-ref traffic is proportional to destructive candidates, so a large open-PR queue cannot disable all cleanup merely by exceeding a fanout cap. + +## Reconciliation and one-shot retirement + +PR #1348 diverged while protected `main` advanced. The reconciliation tree is based on the live protected-main tree and preserves the later scheduler fixes: hourly organization sweep cadence, explicit Ubuntu 24.04 queue-draining runners, and review-event dispatch after thread updates. The obsolete `_temp_pr1348_final_revalidation_repair.yml` source-fix workflow is not carried forward. The production helper is executable in the Git tree and is covered by focused executable regressions, including the stale-PR-payload/live-ref race. + +## Evidence + +`tests/test_queue_cancellation_revalidation.py` covers post-classification head movement, current-head preservation, fail-closed API/ref failures, predecessor cancellation, and aged-orphan behavior. `tests/test_queue_cancellation_open_pr_revalidation.py` specifically proves that a stale open-PR payload SHA cannot authorize cancellation when the authoritative live branch ref still points at the queued run. `tests/test_queue_cancellation_scheduler_contract.py` proves the scheduler routes both cancellation modes through the helper, removes serial upfront ref fanout and the lookup ceiling, preserves current-main scheduler fixes, keeps the helper executable, and retires the temporary writer workflow. + +Hosted exact-head CI, security, coverage and review evidence remain authoritative before merge; this doctoring note does not substitute for those gates. diff --git a/docs/doctoring/repository-public-surface-reconciliation.md b/docs/doctoring/repository-public-surface-reconciliation.md index 6fa36c5ddc..f62086dfbf 100644 --- a/docs/doctoring/repository-public-surface-reconciliation.md +++ b/docs/doctoring/repository-public-surface-reconciliation.md @@ -42,7 +42,7 @@ The fleet loop is deliberately non-blocking. Every repository or label assignmen - Pull-request validation has `contents: read` only. It cannot mutate repository settings or labels. - Apply runs only when the scheduled workflow is executing from trusted `refs/heads/main` after validation. -- The apply step uses the established maintainer credential rather than widening the ordinary workflow token. +- Apply obtains repository-settings write authority only from the protected `repository-metadata-maintenance` environment's dedicated `CWL_REPOSITORY_METADATA_TOKEN`. The job fails before either mutation lane starts when that credential is absent and never falls back to `PR_REVIEW_MERGE_TOKEN`, reviewer/model/provider credentials, or a widened pull-request `GITHUB_TOKEN`. External provisioning remains owned by issue #1579; source integration alone does not prove the secret exists. - Repository README changes remain leaf-owned. The central reconciler verifies exact DeepWiki linkage but never fabricates or silently edits customer-facing README copy. - Pages has two reviewed deployment modes. Legacy mode requires a regular `docs/index.md` file on the live default branch. Explicit `pages_mode: workflow` requires a regular `.github/workflows/pages.yml` file **and** an already-configured live Pages site whose `build_type` is `workflow`. - Workflow mode is preserve-only: the reconciler does not create or convert the Pages configuration. Missing Pages, a legacy live configuration, a directory at the required workflow path, or a missing workflow file fails before description/topic/Page writes for that repository. @@ -54,11 +54,15 @@ The fleet loop is deliberately non-blocking. Every repository or label assignmen ## Desired-state fleet in this increment -The repository metadata manifest currently covers eight repositories selected because their public-surface work already has a concrete leaf source or active writer: `CalendarWeave`, `ConceptWeave`, `context-graph-contracts`, `ThreadWeave`, `RankWeave`, `fast-mlsirm`, `EgressWeave`, and `psychometrics-commons`. EgressWeave and Psychometrics Commons joined the fleet after their exact-cased DeepWiki badges and bounded `docs/index.md` Pages sources reached their protected default branches. +The repository metadata manifest covers 22 reviewed repositories whose public-surface work has a concrete leaf source or active writer: `CalendarWeave`, `ConceptWeave`, `context-graph-contracts`, `ThreadWeave`, `RankWeave`, `fast-mlsirm`, `EgressWeave`, `psychometrics-commons`, `keyverse`, `OriginWeave`, `accounting-information-platform`, `pg-erd-cloud`, `clearfolio`, `DiagramWeave`, `semantic-data-portal`, `contextual-orchestrator`, `mhtml-etl-gateway`, `PolicyWeave`, `supply-chain-control-plane`, `learning-management-platform`, `learning-content-studio`, and `learning-record-store`. + +EgressWeave and Psychometrics Commons joined the original fleet after their exact-cased DeepWiki badges and bounded `docs/index.md` Pages sources reached their protected default branches. Later entries are deliberately declared before live convergence only when an owned leaf lane exists for the required badge and Pages source. Until those prerequisites reach each protected default branch, that repository fails closed while sibling repositories remain independently actionable. The `semantic-data-portal` desired description also removes the internal `(PRD/TRD draft implementation)` qualifier rather than propagating it to the customer-facing repository surface. + +The newest cohort has explicit source ownership: `ContextualWisdomLab/PolicyWeave#1` carries its exact-cased badge and `docs/index.md`; `ContextualWisdomLab/supply-chain-control-plane#1` carries its exact badge and bounded Pages landing source on the active product writer; `ContextualWisdomLab/learning-management-platform#1` owns the product-first README badge and `docs/index.md`; `ContextualWisdomLab/learning-content-studio#1` now owns its product-first README, exact badge, Apache-2.0 grant, and the `docs/index.md` content folded from closed child #8; and `ContextualWisdomLab/learning-record-store#1` now owns its product-first README, exact badge, Apache-2.0 grant, and the bounded `docs/index.md` content folded from closed child #7. The closed child PRs retain discussion history but no longer own unique public-surface source. Their live repositories still report Pages disabled until protected integration and trusted reconciliation complete. An Actions-backed repository is not enrolled merely because `pages_mode: workflow` is supported. Enrollment requires an explicit reviewed manifest change after the repository's standard Pages workflow and live `build_type: workflow` configuration both exist. This preserves the deployment architecture of repositories such as ScopeWeave instead of silently rewriting them to legacy `/docs`. -The explicit label assignments now cover 19 evidence-backed targets: `.github#1582`, `CalendarWeave#1`, `ConceptWeave#1`, `context-graph-contracts#20`, `RankWeave#40`, `fast-mlsirm#1717`, `EgressWeave#231`, `psychometrics-commons#442`, `contextual-orchestrator#994`, `contextual-orchestrator#1003`, `appguardrail#1077`, `naruon#1513`, `LineageWeave#908`, `ContextualWisdomLab.github.io#203`, `TEPP#435`, `semantic-data-portal#72`, `Orgmetra#160`, `learning-interoperability-contracts#1`, and `noema#530`. The assignment reconciler preserves richer repository-local labels such as priority, status, and `type: maintenance` when those labels are outside the managed semantic set. +The explicit label assignments cover 39 active evidence-backed targets: `ContextualWisdomLab/.github#1582`, `ContextualWisdomLab/.github#1622`, `ContextualWisdomLab/.github#1625`, `ContextualWisdomLab/.github#1634`, `ContextualWisdomLab/CalendarWeave#1`, `ContextualWisdomLab/ConceptWeave#1`, `ContextualWisdomLab/context-graph-contracts#20`, `ContextualWisdomLab/RankWeave#40`, `ContextualWisdomLab/fast-mlsirm#1717`, `ContextualWisdomLab/EgressWeave#231`, `ContextualWisdomLab/psychometrics-commons#442`, `ContextualWisdomLab/contextual-orchestrator#994`, `ContextualWisdomLab/contextual-orchestrator#1003`, `ContextualWisdomLab/appguardrail#1077`, `ContextualWisdomLab/naruon#1513`, `ContextualWisdomLab/LineageWeave#908`, `ContextualWisdomLab/ContextualWisdomLab.github.io#203`, `ContextualWisdomLab/TEPP#435`, `ContextualWisdomLab/semantic-data-portal#72`, `ContextualWisdomLab/Orgmetra#160`, `ContextualWisdomLab/learning-interoperability-contracts#1`, `ContextualWisdomLab/noema#530`, `ContextualWisdomLab/bandscope#1125`, `ContextualWisdomLab/saju-caldav#44`, `ContextualWisdomLab/OriginWeave#274`, `ContextualWisdomLab/semantic-data-portal#90`, `ContextualWisdomLab/accounting-information-platform#45`, `ContextualWisdomLab/clearfolio#538`, `ContextualWisdomLab/pg-erd-cloud#1046`, `ContextualWisdomLab/DiagramWeave#34`, `ContextualWisdomLab/keyverse#127`, `ContextualWisdomLab/mhtml-etl-gateway#56`, `ContextualWisdomLab/j-planner#2`, `ContextualWisdomLab/learning-record-store#1`, `ContextualWisdomLab/learning-content-studio#1`, `ContextualWisdomLab/learning-management-platform#1`, `ContextualWisdomLab/metering-billing-platform#157`, `ContextualWisdomLab/PolicyWeave#1`, and `ContextualWisdomLab/supply-chain-control-plane#1`. Closed superseded child PRs `learning-record-store#7`, `learning-content-studio#8`, and `metering-billing-platform#175` are deliberately absent from the active reconciliation target list because their unique documentation deltas were folded into their authoritative parent writers. Historical labels on those closed PRs are not erased by this desired-state change. The assignment reconciler preserves richer repository-local labels such as priority, status, and `type: maintenance` when those labels are outside the managed semantic set. ## Verification contract @@ -72,7 +76,7 @@ A central source commit is not completion. After protected integration and apply 6. the Pages status is `built`, its URL remains under `https://contextualwisdomlab.github.io`, and the published endpoint returns non-empty content before publication is claimed; 7. reviewed issue/PR targets carry the desired managed label while unrelated labels remain intact. -GitHub's current REST Pages contract supports `build_type` values `legacy` and `workflow`, and branch sources with `/` or `/docs`. Legacy desired-state records continue to use `/docs`. The explicit workflow mode exists to preserve a repository whose deployment is already owned by a reviewed GitHub Actions workflow; it is not a central creation/conversion mechanism. +GitHub's current REST Pages contract supports `build_type` values `legacy` and `workflow`, and branch sources with `/` or `/docs`. Current fleet entries use the legacy `/docs` contract unless an entry explicitly declares `pages_mode: workflow`. The workflow mode exists to preserve a repository whose deployment is already owned by a reviewed GitHub Actions workflow; it is not a central creation/conversion mechanism. ## Workflow-mode operating procedure @@ -86,4 +90,4 @@ GitHub's current REST Pages contract supports `build_type` values `legacy` and ` ## Known integration boundary -Until the central PR is merged through normal governance or a verified queue-saturation chicken-and-egg exception, the workflow-mode preservation contract cannot run from trusted `.github/main`; leaf PRs whose badge or Pages source is still branch-only also remain repository-local precondition blockers. These are integration states, not reasons to stop independent repository work. The same run should continue classifying labels, preparing other leaf public surfaces, and re-checking earlier lanes when exact-head evidence becomes available. +The 22-repository desired state and 39-target label taxonomy are protected on `.github/main@ad65125acfe901bf4c4958b6c705ffce17714358`. This lane changes only the credential boundary and its durable contracts. After source integration, issue #1579 remains open until the dedicated GitHub App/token is actually provisioned in the protected environment, a trusted-main reconciliation run obtains it without disclosure, and a live canary re-read proves the intended repository settings. Source integration is therefore necessary but not sufficient evidence of live convergence. diff --git a/docs/nvidia-nim-opencode-hotfix.md b/docs/nvidia-nim-opencode-hotfix.md deleted file mode 100644 index df8c193b28..0000000000 --- a/docs/nvidia-nim-opencode-hotfix.md +++ /dev/null @@ -1,53 +0,0 @@ -# NVIDIA NIM OpenCode model priority (hotfix) - -## Why - -OpenCode Agent failed to produce a usable review on the PR thread starting at -ContextualWisdomLab/fast-mlsirm#290 (`opencode-review` check **skipped**, no -`opencode-agent[bot]` review comment). Central review therefore prioritizes -**NVIDIA NIM** models as additional catalog candidates so the model pool can -still emit APPROVE / REQUEST_CHANGES when GitHub Models / free tiers stall. - -## Changes - -1. `opencode.jsonc` - - `enabled_providers`: `nvidia-nim` first, then `github-models` - - default `model` / `small_model` prefer NIM Nemotron / Llama 3.3 - - new OpenAI-compatible provider `nvidia-nim` → `https://integrate.api.nvidia.com/v1` - with `apiKey: {env:NVIDIA_API_KEY}` -2. `.github/workflows/opencode-review-dispatch.yml` - - `OPENCODE_MODEL_CANDIDATES` prefixes six NIM models before existing pool - - binds `NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }}` -3. `scripts/ci/run_opencode_review_model_pool.sh` - - skips `nvidia-nim/*` when `NVIDIA_API_KEY` is unset (same pattern as OpenRouter) - -## Temporary permission bypass (hotfix only) - -For this merge-aid hotfix only: - -- Branch-protection / ruleset admin override may be used to land the central - `.github` change if required checks conflict during the hotfix window. -- **Do not** permanently weaken Security Scan, trivy-fs, osv-scan, or - CodeQL gates. -- **Do not** flip OpenCode agent `permission.edit` / `bash` from `deny` to - `allow` permanently; review agents remain read-only. -- Org secret `NVIDIA_API_KEY` must be set on ContextualWisdomLab for NIM pool - entries to execute; without it the pool falls through to prior candidates. - -## Rollback - -Remove the `nvidia-nim/*` prefixes from `OPENCODE_MODEL_CANDIDATES`, drop the -`nvidia-nim` provider block, and delete this note once GitHub Models / OpenCode -catalog reliability is restored. - -## Secret name - -Org secret is **`NVIDIA_NIM_API_KEY`**. Workflows bind it to process env `NVIDIA_API_KEY` -(fallback: `secrets.NVIDIA_API_KEY` if present) so `opencode.jsonc` `{env:NVIDIA_API_KEY}` resolves. - -## Large-repo OpenCode timeouts (~1 hour) - -Primary/default run timeouts and the dynamic queue timeout cap default to -**3600s** (hour-class) so large repositories are not cut off by the old 600s -default when env is unset. Free-tier failover remains capped at 600s. -Workflow-provided values (e.g. 5400s) still win over defaults. diff --git a/docs/policies/PINGORA_EDGE_POLICY.md b/docs/policies/PINGORA_EDGE_POLICY.md index 4d4c0752e1..619374a13d 100644 --- a/docs/policies/PINGORA_EDGE_POLICY.md +++ b/docs/policies/PINGORA_EDGE_POLICY.md @@ -53,8 +53,15 @@ The organization-required `required-workflow-bootstrap` job runs trusted base-branch scanner code at the immutable required-workflow SHA. It reads bounded changed-file metadata and final UTF-8 content through GitHub's REST API. It does not check out or execute pull-request content and receives only read permissions. -Malformed, truncated, binary, symlink, oversized, or unavailable evidence fails -closed. +Malformed, truncated, symlinked, oversized, or unavailable runtime evidence fails +closed. Documentation PNG screenshots and PDF papers without a text diff are +excluded only after bounded format verification; PNG evidence must be a complete +CRC-valid chunk stream ending at IEND with conforming chunk names, palette +bounds, and palette indices whose bounded null- or Adam7-interlaced decompressed +scanlines match IHDR. +This is a bounded binary-evidence classifier, not a general image renderer; +visual fidelity and optional ancillary-chunk semantics are outside this gate. +Other binary files remain unavailable evidence and fail closed. ## Exception process diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 41d95b6f57..2a8f4c7b54 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -93,6 +93,7 @@ flowchart LR | G-13 | hourly scheduler는 존재하지만 no-op/credential unavailable/queued Checks의 customer next action을 모든 caller가 동일한 receipt로 내는지 미확인이다 | 자동화가 실패해도 운영자가 무엇을 고쳐야 하는지 알 수 없다 | `skipped_credential_unavailable` receipt와 다음 행동 문구를 exact-head Checks로 검증하고, bounded receipt schema, retry floor, single-flight, no secret fallback을 모든 caller contract test로 고정한다 | | G-14 | release/changelog/version 증거가 각 PR에 분산되고 현재 central repo 보호 main의 release candidate가 명확하지 않다 | 운영자는 어떤 기능이 supportable release인지 확인할 수 없다 | merge 후 release readiness ledger, CHANGELOG, semantic version/tag, rollback/operability evidence를 함께 갱신한다 | | G-15 | 첨부파일 처리 경계가 제품별로 다르고, 1MB 상한은 업무 데이터와 맞지 않으며 미지원 MIME/컨테이너가 parser registry에서 명시적으로 pending/quarantine 되는지 확인되지 않았다. 현재 20MB 초과 파일 가능성과 PDF/HWP/HWPX·이미지·압축파일의 parse/sidecar 흐름을 하나의 exact contract로 묶지 못했다 | 큰 업무 첨부를 거부하거나 파싱 실패를 조용히 잃으면 고객의 메일·문서 업무가 중단된다 | naruon/newsdom-api 소유 PR에서 streaming upload, configurable bounded limit above 20MB, MIME sniffing, parser capability registry, quarantine/retry, source-position provenance, and ADR를 추가하고 size/unsupported-type/zip-bomb tests를 required evidence로 만든다 | +| G-16 | Required Pingora policy treated a changed documentation PNG screenshot as UTF-8 runtime evidence | Valid UI evidence blocked otherwise valid product PRs before policy evaluation | This branch verifies bounded PNG magic before exemption while runtime paths and malformed assets continue to fail closed; protected-main delivery remains the release gate | ## 4. 열린 PR live inventory diff --git a/scripts/ci/pingora_edge_policy.py b/scripts/ci/pingora_edge_policy.py index 823e17fbe5..33e58ed876 100644 --- a/scripts/ci/pingora_edge_policy.py +++ b/scripts/ci/pingora_edge_policy.py @@ -15,6 +15,7 @@ import os import re import sys +import zlib from dataclasses import dataclass from pathlib import PurePosixPath from typing import Callable, Mapping, Sequence @@ -38,7 +39,11 @@ # 1 MiB base64 ceiling -- rejecting a legitimate research-paper citation # (this org's own "attach the relevant paper PDF" convention) for a reason # that has nothing to do with the Nginx runtime policy this module enforces. -BINARY_DOCUMENT_SUFFIXES = frozenset({".pdf"}) +BINARY_DOCUMENT_MAGIC = { + ".pdf": (b"%PDF-",), + ".png": (b"\x89PNG\r\n\x1a\n",), +} +PNG_SIGNATURE = BINARY_DOCUMENT_MAGIC[".png"][0] SOURCE_TEST_SUFFIXES = frozenset({".py", ".pyi", ".js", ".mjs", ".cjs", ".ts", ".tsx", ".rs"}) LICENSE_NAMES = frozenset({"license", "license.md", "copying", "copyrights", "notice"}) DOCUMENTATION_DIRECTORIES = frozenset({"doc", "docs", "documentation"}) @@ -171,7 +176,7 @@ def _is_documentation_or_source_fixture(path: str) -> bool: """Return whether *path* is prose, license text, or scanner source fixture. Textual suffixes only: a ``.pdf`` is handled separately by - ``_is_binary_documentation_pdf`` and gated on GitHub reporting no diff + ``_is_binary_documentation_asset`` and gated on GitHub reporting no diff ``patch`` for it, so a textual file merely named with a ``.pdf`` suffix (one GitHub *can* diff, meaning it could carry inspectable content) is never exempted here. @@ -206,15 +211,15 @@ def _is_documentation_or_source_fixture(path: str) -> bool: return False -def _is_binary_documentation_pdf(changed: ChangedFile) -> bool: - """Return whether *changed* is a plausibly binary documentation PDF. +def _is_binary_documentation_asset(changed: ChangedFile) -> bool: + """Return whether *changed* is a plausibly binary documentation asset. This is only the cheap, patch-presence pre-filter: GitHub's changed-files API never returns a diff ``patch`` for a true binary file, so a missing ``patch`` is *necessary* but not *sufficient* evidence -- GitHub also omits one for a textual diff that merely exceeds its own rendering limit. A caller with network access (``evaluate_pull_request``) must - still confirm this with ``_pdf_evidence_confirms_binary`` before + still confirm this with ``_binary_documentation_evidence_confirms`` before trusting it; a caller without one (this module's own unit tests calling this function directly) is only checking the necessary condition. """ @@ -223,8 +228,9 @@ def _is_binary_documentation_pdf(changed: ChangedFile) -> bool: return False pure = PurePosixPath(changed.path) return ( - pure.suffix.lower() in BINARY_DOCUMENT_SUFFIXES + pure.suffix.lower() in BINARY_DOCUMENT_MAGIC and _is_known_documentation_path(pure) + and _runtime_path_rule(changed.path) is None ) @@ -414,10 +420,7 @@ def _load_file_content(api_url: str, repository: str, path: str, head_sha: str, raise PolicyError(f"Runtime policy candidate {path} is not valid UTF-8") from exc -_PDF_MAGIC_PREFIX = b"%PDF-" - - -def _pdf_evidence_confirms_binary( +def _binary_documentation_evidence_confirms( changed: ChangedFile, *, api_url: str, @@ -426,17 +429,18 @@ def _pdf_evidence_confirms_binary( token: str, opener: OpenJson, ) -> bool: - """Return whether a claimed binary documentation PDF is genuinely binary. + """Return whether a claimed binary documentation asset is genuine. A missing diff ``patch`` alone is not proof of binary content: GitHub also omits a patch for a textual diff that exceeds its own rendering limit, well under this module's ``MAX_FILE_BYTES`` content-fetch ceiling. Whenever the file's raw bytes can be fetched at all, this - verifies the real ``%PDF-`` magic prefix instead of trusting + verifies the declared format's magic prefix instead of trusting patch-presence alone. Only a file whose content evidently exceeds the - Contents API's size ceiling -- the exact case ``_is_binary_documentation_pdf`` + Contents API's size ceiling -- the exact case ``_is_binary_documentation_asset`` exists for, a cited, large research paper -- falls back to trusting the - path+suffix convention; every other content-evidence failure (a + path+suffix convention for oversized PDFs only; every other + content-evidence failure (a malformed API response, corrupt base64, a declared size that does not match the decoded bytes) propagates and fails the whole check closed, same as for any other file that needs scanning. @@ -445,22 +449,169 @@ def _pdf_evidence_confirms_binary( try: raw = _load_raw_file_bytes(api_url, repository, changed.path, head_sha, token, opener) except ContentSizeExceededError: - return True - return raw.startswith(_PDF_MAGIC_PREFIX) + return PurePosixPath(changed.path).suffix.lower() == ".pdf" + suffix = PurePosixPath(changed.path).suffix.lower() + if suffix == ".png": + return _is_complete_png(raw) + return raw.startswith(BINARY_DOCUMENT_MAGIC[suffix]) + + +def _png_unfilter_row(filtered: bytes, previous: bytes, filter_type: int, bytes_per_pixel: int) -> bytes: + """Reconstruct one PNG scanline for bounded indexed-pixel validation.""" + + reconstructed = bytearray(len(filtered)) + for index, value in enumerate(filtered): + left = reconstructed[index - bytes_per_pixel] if index >= bytes_per_pixel else 0 + above = previous[index] if previous else 0 + upper_left = previous[index - bytes_per_pixel] if previous and index >= bytes_per_pixel else 0 + if filter_type == 0: + predictor = 0 + elif filter_type == 1: + predictor = left + elif filter_type == 2: + predictor = above + elif filter_type == 3: + predictor = (left + above) // 2 + else: + estimate = left + above - upper_left + distances = (abs(estimate - left), abs(estimate - above), abs(estimate - upper_left)) + predictor = (left, above, upper_left)[distances.index(min(distances))] + reconstructed[index] = (value + predictor) & 0xFF + return bytes(reconstructed) + + +def _is_complete_png(raw: bytes) -> bool: + """Validate one bounded PNG including its null- or Adam7-interlaced stream.""" + + if not raw.startswith(PNG_SIGNATURE): + return False + offset = len(PNG_SIGNATURE) + header: tuple[int, int, int, int, int] | None = None + palette_entries = 0 + image_data: list[bytes] = [] + image_data_closed = False + while offset + 12 <= len(raw): + length = int.from_bytes(raw[offset : offset + 4], "big") + chunk_end = offset + 12 + length + if chunk_end > len(raw): + return False + chunk_type = raw[offset + 4 : offset + 8] + chunk_data = raw[offset + 8 : offset + 8 + length] + expected_crc = int.from_bytes(raw[offset + 8 + length : chunk_end], "big") + if ( + any(not (65 <= byte <= 90 or 97 <= byte <= 122) for byte in chunk_type) + or chunk_type[2] & 0x20 + or zlib.crc32(chunk_type + chunk_data) != expected_crc + ): + return False + if header is None: + if chunk_type != b"IHDR" or length != 13 or offset != len(PNG_SIGNATURE): + return False + width = int.from_bytes(chunk_data[0:4], "big") + height = int.from_bytes(chunk_data[4:8], "big") + bit_depth, color_type, compression, filtering, interlace = chunk_data[8:13] + allowed_depths = { + 0: {1, 2, 4, 8, 16}, 2: {8, 16}, 3: {1, 2, 4, 8}, + 4: {8, 16}, 6: {8, 16}, + } + if ( + width == 0 or height == 0 + or bit_depth not in allowed_depths.get(color_type, set()) + or compression != 0 or filtering != 0 or interlace not in {0, 1} + ): + return False + header = (width, height, bit_depth, color_type, interlace) + elif chunk_type == b"IHDR": + return False + elif chunk_type == b"PLTE": + if palette_entries or image_data or length == 0 or length > 768 or length % 3: + return False + _width, _height, bit_depth, color_type, _interlace = header + if color_type == 3 and length // 3 > 1 << bit_depth: + return False + palette_entries = length // 3 + elif chunk_type == b"IDAT": + if image_data_closed: + return False + image_data.append(chunk_data) + elif chunk_type == b"IEND": + if length != 0 or not image_data or chunk_end != len(raw): + return False + width, height, bit_depth, color_type, interlace = header + if (color_type == 3 and not palette_entries) or ( + color_type in {0, 4} and palette_entries + ): + return False + channels = {0: 1, 2: 3, 3: 1, 4: 2, 6: 4}[color_type] + passes = ( + ((0, 0, 8, 8), (4, 0, 8, 8), (0, 4, 4, 8), (2, 0, 4, 4), + (0, 2, 2, 4), (1, 0, 2, 2), (0, 1, 1, 2)) + if interlace else ((0, 0, 1, 1),) + ) + scanlines: list[tuple[int, int, int]] = [] + expected_size = 0 + for x_start, y_start, x_step, y_step in passes: + if width <= x_start or height <= y_start: + continue + pass_width = (width - x_start + x_step - 1) // x_step + pass_height = (height - y_start + y_step - 1) // y_step + row_bytes = (pass_width * channels * bit_depth + 7) // 8 + expected_size += pass_height * (row_bytes + 1) + if expected_size > MAX_RESPONSE_BYTES: + return False + scanlines.append((pass_height, row_bytes, pass_width)) + decoder = zlib.decompressobj() + try: + decoded = decoder.decompress(b"".join(image_data), expected_size + 1) + except zlib.error: + return False + if ( + len(decoded) != expected_size or not decoder.eof + or decoder.unused_data or decoder.unconsumed_tail + ): + return False + decoded_offset = 0 + for row_count, row_bytes, pass_width in scanlines: + previous = b"" + for _ in range(row_count): + filter_type = decoded[decoded_offset] + if filter_type > 4: + return False + filtered = decoded[decoded_offset + 1 : decoded_offset + row_bytes + 1] + if color_type == 3: + reconstructed = _png_unfilter_row(filtered, previous, filter_type, 1) + mask = (1 << bit_depth) - 1 + for pixel in range(pass_width): + bit_offset = pixel * bit_depth + palette_index = ( + reconstructed[bit_offset // 8] + >> (8 - bit_depth - bit_offset % 8) + ) & mask + if palette_index >= palette_entries: + return False + previous = reconstructed + decoded_offset += row_bytes + 1 + return decoded_offset == len(decoded) + elif chunk_type[0] & 0x20 == 0: + return False + elif image_data: + image_data_closed = True + offset = chunk_end + return False def _needs_content_scan(changed: ChangedFile) -> bool: """Return whether a changed final file can carry an active edge runtime. - A claimed binary documentation PDF (``_is_binary_documentation_pdf``) + A claimed binary documentation asset (``_is_binary_documentation_asset``) exempts here on the cheap, offline pre-filter alone; ``evaluate_pull_request`` - never actually relies on that -- it runs ``_pdf_evidence_confirms_binary`` + never actually relies on that -- it runs ``_binary_documentation_evidence_confirms`` for that case before this function is even consulted. """ if changed.status == "removed" or _is_documentation_or_source_fixture(changed.path): return False - if _is_binary_documentation_pdf(changed): + if _is_binary_documentation_asset(changed): return False if not changed.patch_available: return True @@ -499,17 +650,17 @@ def evaluate_pull_request( changed_files = _load_changed_files(api_url.rstrip("/"), repository, pull_request, token, opener) violations: list[Violation] = [] for changed in changed_files: - # A claimed binary documentation PDF gets its own network-verified + # A claimed binary documentation asset gets its own network-verified # check ahead of _needs_content_scan's patch-presence-only signal: # a missing patch does not by itself prove binary content (GitHub # also omits one for an oversized textual diff), so this confirms - # the real %PDF- magic prefix whenever the bytes can be fetched at + # the format's magic prefix whenever the bytes can be fetched at # all, falling back to the path+suffix convention only when the # content genuinely exceeds the Contents API's size ceiling. A # removed file has no head content to fetch at all -- _needs_content_scan # already special-cases this the same way for every other file. - if changed.status != "removed" and _is_binary_documentation_pdf(changed): - if _pdf_evidence_confirms_binary( + if changed.status != "removed" and _is_binary_documentation_asset(changed): + if _binary_documentation_evidence_confirms( changed, api_url=api_url.rstrip("/"), repository=repository, diff --git a/scripts/ci/revalidate_queue_cancellation.sh b/scripts/ci/revalidate_queue_cancellation.sh new file mode 100755 index 0000000000..14bf5d2eca --- /dev/null +++ b/scripts/ci/revalidate_queue_cancellation.sh @@ -0,0 +1,178 @@ +#!/usr/bin/env bash +set -euo pipefail + +if [ "$#" -ne 6 ]; then + echo "usage: $0 " >&2 + exit 2 +fi + +repo_full_name="$1" +run_id="$2" +default_branch="$3" +classified_default_sha="$4" +classified_open_pr_heads_json="$5" +cancellation_mode="$6" + +case "$cancellation_mode" in + superseded|aged-orphan) ;; + *) + echo "invalid cancellation mode: ${cancellation_mode}" >&2 + exit 2 + ;; +esac + +warn_preserve() { + echo "::warning::Preserving run ${run_id} in ${repo_full_name}: $1" + exit 0 +} + +encode_ref_path() { + jq -rn --arg value "$1" '$value | split("/") | map(@uri) | join("/")' +} + +if ! run_json="$(gh api -H "Accept: application/vnd.github+json" "/repos/${repo_full_name}/actions/runs/${run_id}")"; then + warn_preserve "live run metadata could not be re-fetched before cancellation." +fi + +event="$(jq -r '.event // empty' <<<"$run_json")" +status="$(jq -r '.status // empty' <<<"$run_json")" +run_head="$(jq -r '.head_sha // empty' <<<"$run_json")" +run_branch="$(jq -r '.head_branch // empty' <<<"$run_json")" +run_head_repo="$(jq -r '.head_repository.full_name // empty' <<<"$run_json")" +if ! [[ "$run_head" =~ ^[0-9a-fA-F]{40}$ ]]; then + warn_preserve "live run head is malformed." +fi + +if [ "$cancellation_mode" = "aged-orphan" ]; then + if [ "$status" != "queued" ]; then + warn_preserve "aged-orphan candidate is no longer queued (status=${status:-})." + fi +elif [ "$status" != "queued" ] && [ "$status" != "in_progress" ]; then + warn_preserve "superseded candidate is no longer queued or in progress (status=${status:-})." +fi + +case "$event" in + pull_request|pull_request_target) + pr_number="$(jq -r '.pull_requests[0].number // empty' <<<"$run_json")" + if ! [[ "$pr_number" =~ ^[1-9][0-9]*$ ]]; then + if [ "$cancellation_mode" = "aged-orphan" ]; then + # Association metadata on an Actions run can lag the PR itself. Re-read + # open PRs immediately before destructive cancellation, but use that + # payload only to discover the authoritative head repository/ref. The + # payload SHA itself can be stale, so resolve a matching branch through + # the Git reference endpoint before deciding whether the run is current. + if [ -z "$run_head_repo" ] || [ -z "$run_branch" ]; then + warn_preserve "unassociated PR run has no authoritative head repository/ref." + fi + if ! fresh_open_pr_refs_json="$( + gh api \ + -H "Accept: application/vnd.github+json" \ + "/repos/${repo_full_name}/pulls?state=open&per_page=100" \ + --paginate \ + | jq -sc '[.[] | .[] | { + repo: (.head.repo.full_name // null), + ref: (.head.ref // null) + }]' + )"; then + warn_preserve "open PR heads could not be re-fetched for an unassociated PR run." + fi + if ! jq -e ' + all(.[]; + (.repo | type) == "string" and (.repo | length) > 0 and + (.ref | type) == "string" and (.ref | length) > 0 + ) + ' <<<"$fresh_open_pr_refs_json" >/dev/null; then + warn_preserve "fresh open PR head evidence is malformed." + fi + if jq -e \ + --arg repo "$run_head_repo" \ + --arg ref "$run_branch" \ + 'any(.[]; .repo == $repo and .ref == $ref)' \ + <<<"$fresh_open_pr_refs_json" >/dev/null; then + encoded_run_ref="$(encode_ref_path "$run_branch")" + if ! final_ref_sha="$( + gh api \ + -H "Accept: application/vnd.github+json" \ + "/repos/${run_head_repo}/git/ref/heads/${encoded_run_ref}" \ + --jq '.object.sha // empty' + )"; then + warn_preserve "live ref for newly associated PR head could not be re-fetched before cancellation." + fi + if ! [[ "$final_ref_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + warn_preserve "live ref for newly associated PR head is malformed." + fi + if [ "$run_head" = "$final_ref_sha" ]; then + warn_preserve "run became associated with an open PR at its authoritative current head after queue classification." + fi + fi + else + warn_preserve "no authoritative PR identity is attached to the live run." + fi + else + if ! pr_json="$(gh api -H "Accept: application/vnd.github+json" "/repos/${repo_full_name}/pulls/${pr_number}")"; then + warn_preserve "live PR ${pr_number} could not be re-fetched before cancellation." + fi + live_state="$(jq -r '.state // empty' <<<"$pr_json")" + if [ "$live_state" = "open" ]; then + live_head_repo="$(jq -r '.head.repo.full_name // empty' <<<"$pr_json")" + live_head_ref="$(jq -r '.head.ref // empty' <<<"$pr_json")" + live_head_sha="$(jq -r '.head.sha // empty' <<<"$pr_json")" + if [ -z "$live_head_repo" ] || [ -z "$live_head_ref" ] || ! [[ "$live_head_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + warn_preserve "live PR ${pr_number} head metadata is malformed." + fi + encoded_head_ref="$(encode_ref_path "$live_head_ref")" + if ! final_ref_sha="$(gh api -H "Accept: application/vnd.github+json" "/repos/${live_head_repo}/git/ref/heads/${encoded_head_ref}" --jq '.object.sha // empty')"; then + warn_preserve "live ref for PR ${pr_number} could not be re-fetched before cancellation." + fi + if ! [[ "$final_ref_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + warn_preserve "live ref for PR ${pr_number} is malformed." + fi + classified_sha="$(jq -r --arg key "${live_head_repo}:${live_head_ref}" '.[$key] // empty' <<<"$classified_open_pr_heads_json")" + if ! [[ "$classified_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + warn_preserve "the classification snapshot has no valid head for PR ${pr_number}." + fi + if [ "$live_head_sha" != "$classified_sha" ] || [ "$final_ref_sha" != "$classified_sha" ]; then + warn_preserve "PR ${pr_number} moved after queue classification." + fi + if [ "$run_head" = "$final_ref_sha" ]; then + echo "Preserving run ${run_id} in ${repo_full_name}: authoritative current-head evidence for PR ${pr_number}." + exit 0 + fi + elif [ "$live_state" != "closed" ]; then + warn_preserve "live PR ${pr_number} state is malformed." + fi + # A closed PR cannot supply current merge evidence. If the run is still + # active and was selected from the trusted snapshot, closure remains an + # authoritative reason to retire it. + fi + ;; + push|schedule) + if [ "$run_branch" = "$default_branch" ] || [ "$cancellation_mode" = "superseded" ]; then + if ! live_default_sha="$(gh api -H "Accept: application/vnd.github+json" "/repos/${repo_full_name}/commits/${default_branch}" --jq '.sha // empty')"; then + warn_preserve "live default-branch HEAD could not be re-fetched before cancellation." + fi + if ! [[ "$live_default_sha" =~ ^[0-9a-fA-F]{40}$ ]]; then + warn_preserve "live default-branch HEAD is malformed." + fi + if [ "$live_default_sha" != "$classified_default_sha" ]; then + warn_preserve "default branch moved after queue classification." + fi + if [ "$run_head" = "$live_default_sha" ]; then + echo "Preserving run ${run_id} in ${repo_full_name}: authoritative current default-branch evidence." + exit 0 + fi + fi + ;; + *) + if [ "$cancellation_mode" = "superseded" ]; then + warn_preserve "event ${event:-} is outside the authoritative superseded-run contract." + fi + # Aged-orphan mode intentionally retains the legacy cleanup contract for + # workflow_dispatch, workflow_run, repository_dispatch, and other queued + # events that the trusted initial snapshot proved were not current PR heads. + ;; +esac + +if ! gh api -X POST "/repos/${repo_full_name}/actions/runs/${run_id}/cancel" >/dev/null; then + echo "Could not cancel ${cancellation_mode} run ${run_id} in ${repo_full_name}; it may have started or finished already." +fi diff --git a/tests/test_pingora_edge_policy.py b/tests/test_pingora_edge_policy.py index 96692b4430..c5d4e9d7a3 100644 --- a/tests/test_pingora_edge_policy.py +++ b/tests/test_pingora_edge_policy.py @@ -7,6 +7,7 @@ import inspect import re import sys +import zlib from io import BytesIO from pathlib import Path from urllib.error import HTTPError, URLError @@ -383,10 +384,195 @@ def opener(url: str, _token: str) -> object: assert result == () +def test_evaluate_pull_request_exempts_a_real_documentation_png() -> None: + """A screenshot is verified by PNG magic instead of decoded as UTF-8.""" + + def opener(url: str, _token: str) -> object: + if "/pulls/15/files" in url: + return [{"filename": "docs/screenshots/dashboard.png", "status": "added"}] + assert "/contents/docs/screenshots/dashboard.png" in url + raw = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=" + ) + return { + "type": "file", + "encoding": "base64", + "size": len(raw), + "content": base64.b64encode(raw).decode("ascii"), + } + + assert policy.evaluate_pull_request( + api_url="https://api.github.test", + repository="ContextualWisdomLab/example", + pull_request=15, + head_sha="a" * 40, + event_action="opened", + token="token", + opener=opener, + ) == () + + +def test_evaluate_pull_request_rejects_a_fake_documentation_png() -> None: + """A PNG suffix without PNG magic remains runtime-content evidence.""" + + def opener(url: str, _token: str) -> object: + if "/pulls/16/files" in url: + return [{"filename": "docs/screenshots/fake.png", "status": "added"}] + return encoded_file("cat /etc/nginx/nginx.conf\n") + + result = policy.evaluate_pull_request( + api_url="https://api.github.test", + repository="ContextualWisdomLab/example", + pull_request=16, + head_sha="b" * 40, + event_action="opened", + token="token", + opener=opener, + ) + assert [item.rule for item in result] == ["nginx_runtime_path"] + + +def test_evaluate_pull_request_rejects_png_with_appended_runtime_text() -> None: + """A valid image prefix cannot hide bytes appended after the IEND chunk.""" + + image = base64.b64decode( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNk+A8AAQUBAScY42YAAAAASUVORK5CYII=" + ) + + def opener(url: str, _token: str) -> object: + if "/pulls/17/files" in url: + return [{"filename": "docs/screenshots/forged.png", "status": "added"}] + raw = image + b"\ncat /etc/nginx/nginx.conf\n" + return { + "type": "file", "encoding": "base64", "size": len(raw), + "content": base64.b64encode(raw).decode("ascii"), + } + + with pytest.raises(policy.PolicyError, match="not valid UTF-8"): + policy.evaluate_pull_request( + api_url="https://api.github.test", + repository="ContextualWisdomLab/example", + pull_request=17, + head_sha="c" * 40, + event_action="opened", + token="token", + opener=opener, + ) + + +def test_png_structure_validation_fails_closed_on_malformed_chunks() -> None: + """Every malformed PNG boundary returns false without parsing past bounds.""" + + def chunk(kind: bytes, data: bytes) -> bytes: + payload = kind + data + return len(data).to_bytes(4, "big") + payload + zlib.crc32(payload).to_bytes(4, "big") + + signature = policy.PNG_SIGNATURE + header = chunk(b"IHDR", b"\0" * 13) + assert not policy._is_complete_png(b"not-png") + assert not policy._is_complete_png(signature) + assert not policy._is_complete_png( + signature + (99).to_bytes(4, "big") + b"IHDR" + b"\0" * 4 + ) + assert not policy._is_complete_png(signature + header[:-1] + b"\0") + assert not policy._is_complete_png(signature + chunk(b"TEXT", b"")) + assert not policy._is_complete_png(signature + header + chunk(b"IEND", b"")) + assert not policy._is_complete_png(signature + header + chunk(b"TEXT", b"")) + + +def test_png_semantic_validation_fails_closed() -> None: + """CRC-valid chunks still need a valid bounded PNG image stream.""" + + def chunk(kind: bytes, data: bytes) -> bytes: + payload = kind + data + return len(data).to_bytes(4, "big") + payload + zlib.crc32(payload).to_bytes(4, "big") + + def png(header: bytes, *chunks: bytes) -> bytes: + return policy.PNG_SIGNATURE + chunk(b"IHDR", header) + b"".join(chunks) + + def indexed_png( + width: int, + height: int, + bit_depth: int, + palette_entries: int, + decoded: bytes, + *, + interlace: int = 0, + ) -> bytes: + header = width.to_bytes(4, "big") + height.to_bytes(4, "big") + bytes((bit_depth, 3, 0, 0, interlace)) + return png( + header, + chunk(b"PLTE", b"\0\0\0" * palette_entries), + chunk(b"IDAT", zlib.compress(decoded)), + chunk(b"IEND", b""), + ) + + rgba = (1).to_bytes(4, "big") * 2 + bytes((8, 6, 0, 0, 0)) + indexed = (1).to_bytes(4, "big") * 2 + bytes((8, 3, 0, 0, 0)) + gray = (1).to_bytes(4, "big") * 2 + bytes((8, 0, 0, 0, 0)) + image = chunk(b"IDAT", zlib.compress(b"\0\0\0\0\0")) + end = chunk(b"IEND", b"") + + invalid_headers = ( + b"\0" * 13, + (1).to_bytes(4, "big") * 2 + bytes((4, 2, 0, 0, 0)), + (1).to_bytes(4, "big") * 2 + bytes((8, 6, 1, 0, 0)), + (1).to_bytes(4, "big") * 2 + bytes((8, 6, 0, 1, 0)), + (1).to_bytes(4, "big") * 2 + bytes((8, 6, 0, 0, 2)), + ) + assert all(not policy._is_complete_png(png(header, image, end)) for header in invalid_headers) + assert not policy._is_complete_png(png(rgba, chunk(b"IHDR", rgba), image, end)) + assert not policy._is_complete_png(png(rgba, chunk(b"PLTE", b""), image, end)) + assert not policy._is_complete_png(png(rgba, chunk(b"PLTE", b"x" * 769), image, end)) + assert not policy._is_complete_png(png(rgba, chunk(b"PLTE", b"x"), image, end)) + assert not policy._is_complete_png(png(rgba, chunk(b"1EXt", b""), image, end)) + assert not policy._is_complete_png(png(rgba, chunk(b"tExt", b""), image, end)) + assert not policy._is_complete_png(png(rgba, chunk(b"ABCD", b""), image, end)) + assert policy._is_complete_png(png(rgba, chunk(b"tEXt", b"x"), image, end)) + assert not policy._is_complete_png(png(rgba, image, chunk(b"tEXt", b"x"), image, end)) + assert not policy._is_complete_png(png(indexed, image, end)) + indexed_one_bit = (1).to_bytes(4, "big") * 2 + bytes((1, 3, 0, 0, 0)) + assert not policy._is_complete_png( + png(indexed_one_bit, chunk(b"PLTE", b"\0" * 9), chunk(b"IDAT", zlib.compress(b"\0\0")), end) + ) + for filter_type in range(5): + second_row = b"\1\0" if filter_type == 0 else b"\1\xff" + assert policy._is_complete_png( + indexed_png(2, 2, 8, 2, bytes((filter_type, 0, 1, filter_type)) + second_row) + ) + assert not policy._is_complete_png(indexed_png(2, 1, 8, 1, b"\0\0\1")) + assert not policy._is_complete_png(indexed_png(2, 2, 8, 2, b"\0\0\1\4\2\xfe")) + assert policy._is_complete_png(indexed_png(2, 1, 1, 2, b"\0\x40")) + assert not policy._is_complete_png(indexed_png(2, 1, 1, 1, b"\0\x40")) + assert policy._is_complete_png(indexed_png(1, 1, 8, 1, b"\0\0", interlace=1)) + assert not policy._is_complete_png(indexed_png(1, 1, 8, 1, b"\0\1", interlace=1)) + assert not policy._is_complete_png(png(gray, chunk(b"PLTE", b"\0\0\0"), chunk(b"IDAT", zlib.compress(b"\0\0")), end)) + assert not policy._is_complete_png(png(rgba, chunk(b"IDAT", b"not-zlib"), end)) + assert not policy._is_complete_png(png(rgba, chunk(b"IDAT", zlib.compress(b"\0")), end)) + assert not policy._is_complete_png(png(rgba, chunk(b"IDAT", zlib.compress(b"\0\0\0\0\0") + b"x"), end)) + assert not policy._is_complete_png(png(rgba, chunk(b"IDAT", zlib.compress(b"\5\0\0\0\0")), end)) + huge = (policy.MAX_RESPONSE_BYTES).to_bytes(4, "big") + (1).to_bytes(4, "big") + bytes((8, 6, 0, 0, 0)) + assert not policy._is_complete_png(png(huge, image, end)) + + adam7 = (8).to_bytes(4, "big") * 2 + bytes((8, 6, 0, 0, 1)) + adam7_scanlines = b"".join( + b"\0" + b"\0" * (pass_width * 4) + for pass_width, pass_height in ((1, 1), (1, 1), (2, 1), (2, 2), (4, 2), (4, 4), (8, 4)) + for _ in range(pass_height) + ) + assert policy._is_complete_png( + png(adam7, chunk(b"IDAT", zlib.compress(adam7_scanlines)), end) + ) + adam7_one_pixel = (1).to_bytes(4, "big") * 2 + bytes((8, 6, 0, 0, 1)) + assert policy._is_complete_png( + png(adam7_one_pixel, chunk(b"IDAT", zlib.compress(b"\0\0\0\0\0")), end) + ) + + def test_evaluate_pull_request_does_not_fetch_a_removed_binary_pdf() -> None: """A removed documentation PDF has no head content to fetch at all. - Regression coverage for Devin Review's finding: _is_binary_documentation_pdf + Regression coverage for Devin Review's finding: _is_binary_documentation_asset does not itself check status, so without an explicit removed-status guard in evaluate_pull_request's own loop, a deleted PDF would try to fetch its (nonexistent) head content and fail evidence collection for every such diff --git a/tests/test_queue_cancellation_open_pr_revalidation.py b/tests/test_queue_cancellation_open_pr_revalidation.py new file mode 100644 index 0000000000..72ecc9b9e9 --- /dev/null +++ b/tests/test_queue_cancellation_open_pr_revalidation.py @@ -0,0 +1,129 @@ +"""Regressions for aged PR-run cancellation after late PR association.""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + + +REPO_ROOT = Path(__file__).resolve().parents[1] +SCRIPT = REPO_ROOT / "scripts" / "ci" / "revalidate_queue_cancellation.sh" + + +def _run_late_association_case( + tmp_path: Path, *, payload_sha: str, live_ref_sha: str, fail_ref: bool = False +) -> tuple[subprocess.CompletedProcess[str], bool]: + if shutil.which("jq") is None: + pytest.skip("jq is required for the queue-cancellation regression") + + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + cancelled = tmp_path / "cancelled" + current = "b" * 40 + run_payload = json.dumps( + { + "event": "pull_request", + "status": "queued", + "head_sha": current, + "head_branch": "feature/late-pr", + "head_repository": {"full_name": "ContextualWisdomLab/example"}, + "pull_requests": [], + }, + separators=(",", ":"), + ) + # Deliberately include a payload SHA that may lag the authoritative branch + # ref. The helper must use this response only to discover repo/ref identity. + open_prs = json.dumps( + [ + { + "state": "open", + "head": { + "repo": {"full_name": "ContextualWisdomLab/example"}, + "ref": "feature/late-pr", + "sha": payload_sha, + }, + } + ], + separators=(",", ":"), + ) + fake_gh = bin_dir / "gh" + fake_gh.write_text( + f"""#!/usr/bin/env bash +set -euo pipefail +args="$*" +if [[ "$args" == *"/actions/runs/77/cancel"* ]]; then + : > {cancelled!s} + exit 0 +fi +if [[ "$args" == *"/actions/runs/77"* ]]; then + printf '%s\\n' '{run_payload}' + exit 0 +fi +if [[ "$args" == *"/pulls?state=open&per_page=100"* ]]; then + printf '%s\\n' '{open_prs}' + exit 0 +fi +if [[ "$args" == *"/git/ref/heads/feature/late-pr"* ]]; then + {'exit 74' if fail_ref else f"printf '%s\\n' '{live_ref_sha}'"} + exit 0 +fi +exit 79 +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + env = os.environ.copy() + env["PATH"] = f"{bin_dir}{os.pathsep}{env['PATH']}" + result = subprocess.run( + [ + "bash", + str(SCRIPT), + "ContextualWisdomLab/example", + "77", + "main", + "d" * 40, + "{}", + "aged-orphan", + ], + capture_output=True, + text=True, + env=env, + check=False, + ) + return result, cancelled.exists() + + +def test_aged_unassociated_pr_run_resolves_authoritative_live_ref(tmp_path: Path) -> None: + """A stale PR payload cannot authorize cancellation of the live current head.""" + current = "b" * 40 + stale_payload = "a" * 40 + result, cancelled = _run_late_association_case( + tmp_path, + payload_sha=stale_payload, + live_ref_sha=current, + ) + + assert result.returncode == 0, result.stderr + assert "authoritative current head" in result.stdout + assert not cancelled + + +def test_aged_unassociated_pr_run_fails_closed_when_live_ref_is_unreadable( + tmp_path: Path, +) -> None: + """A matching late PR with unreadable ref must preserve the queued run.""" + result, cancelled = _run_late_association_case( + tmp_path, + payload_sha="a" * 40, + live_ref_sha="b" * 40, + fail_ref=True, + ) + + assert result.returncode == 0, result.stderr + assert "could not be re-fetched" in result.stdout + assert not cancelled diff --git a/tests/test_queue_cancellation_revalidation.py b/tests/test_queue_cancellation_revalidation.py new file mode 100644 index 0000000000..23ac824d59 --- /dev/null +++ b/tests/test_queue_cancellation_revalidation.py @@ -0,0 +1,364 @@ +"""Executable regressions for destructive queue-cancellation revalidation.""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + + +REPO_ROOT = Path(__file__).resolve().parents[1] +SCRIPT = REPO_ROOT / "scripts" / "ci" / "revalidate_queue_cancellation.sh" + + +def _run_case( + tmp_path: Path, + *, + snapshot_sha: str, + pr_sha: str, + ref_sha: str, + run_sha: str, + fail_lookup: str | None = None, +) -> tuple[subprocess.CompletedProcess[str], bool]: + """Run the production shell helper against a deterministic fake GitHub CLI.""" + if shutil.which("jq") is None: + pytest.skip("jq is required for the queue-cancellation regression") + + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + cancelled = tmp_path / "cancelled" + pr_payload = json.dumps( + { + "state": "open", + "head": { + "repo": {"full_name": "ContextualWisdomLab/example"}, + "ref": "feature/race", + "sha": pr_sha, + }, + }, + separators=(",", ":"), + ) + run_payload = json.dumps( + { + "event": "pull_request", + "status": "queued", + "head_sha": run_sha, + "pull_requests": [{"number": 12}], + }, + separators=(",", ":"), + ) + fake_gh = bin_dir / "gh" + fake_gh.write_text( + f"""#!/usr/bin/env bash +set -euo pipefail +args="$*" +if [[ "$args" == *"/actions/runs/77/cancel"* ]]; then + : > {cancelled!s} + exit 0 +fi +if [[ "$args" == *"/actions/runs/77"* ]]; then + printf '%s\\n' '{run_payload}' + exit 0 +fi +if [[ "$args" == *"/pulls/12"* ]]; then + {'exit 73' if fail_lookup == 'pr' else f"printf '%s\\n' '{pr_payload}'"} + exit 0 +fi +if [[ "$args" == *"/git/ref/heads/feature/race"* ]]; then + {'exit 74' if fail_lookup == 'ref' else f"printf '%s\\n' '{ref_sha}'"} + exit 0 +fi +exit 79 +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + env = os.environ.copy() + env["PATH"] = f"{bin_dir}{os.pathsep}{env['PATH']}" + snapshot = json.dumps( + {"ContextualWisdomLab/example:feature/race": snapshot_sha}, + separators=(",", ":"), + ) + result = subprocess.run( + [ + "bash", + str(SCRIPT), + "ContextualWisdomLab/example", + "77", + "main", + "d" * 40, + snapshot, + "superseded", + ], + capture_output=True, + text=True, + env=env, + check=False, + ) + return result, cancelled.exists() + + +def _run_aged_orphan_case( + tmp_path: Path, *, event: str, status: str = "queued" +) -> tuple[subprocess.CompletedProcess[str], bool]: + """Run an aged orphan candidate that has no current PR/default-branch authority.""" + if shutil.which("jq") is None: + pytest.skip("jq is required for the queue-cancellation regression") + + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + cancelled = tmp_path / "cancelled" + run_payload = json.dumps( + { + "event": event, + "status": status, + "head_sha": "a" * 40, + "pull_requests": [], + }, + separators=(",", ":"), + ) + fake_gh = bin_dir / "gh" + fake_gh.write_text( + f"""#!/usr/bin/env bash +set -euo pipefail +args="$*" +if [[ "$args" == *"/actions/runs/77/cancel"* ]]; then + : > {cancelled!s} + exit 0 +fi +if [[ "$args" == *"/actions/runs/77"* ]]; then + printf '%s\\n' '{run_payload}' + exit 0 +fi +exit 79 +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + env = os.environ.copy() + env["PATH"] = f"{bin_dir}{os.pathsep}{env['PATH']}" + result = subprocess.run( + [ + "bash", + str(SCRIPT), + "ContextualWisdomLab/example", + "77", + "main", + "d" * 40, + "{}", + "aged-orphan", + ], + capture_output=True, + text=True, + env=env, + check=False, + ) + return result, cancelled.exists() + + +def _run_unassociated_pr_aged_orphan_case( + tmp_path: Path, + *, + listed_sha: str, + ref_sha: str, + run_sha: str, + fail_ref_lookup: bool = False, +) -> tuple[subprocess.CompletedProcess[str], bool]: + """Run an unassociated aged PR run against stale listing and live-ref evidence.""" + if shutil.which("jq") is None: + pytest.skip("jq is required for the queue-cancellation regression") + + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + cancelled = tmp_path / "cancelled" + run_payload = json.dumps( + { + "event": "pull_request", + "status": "queued", + "head_sha": run_sha, + "head_branch": "feature/race", + "head_repository": {"full_name": "ContextualWisdomLab/example"}, + "pull_requests": [], + }, + separators=(",", ":"), + ) + open_pr_payload = json.dumps( + [ + { + "head": { + "repo": {"full_name": "ContextualWisdomLab/example"}, + "ref": "feature/race", + "sha": listed_sha, + } + } + ], + separators=(",", ":"), + ) + fake_gh = bin_dir / "gh" + fake_gh.write_text( + f"""#!/usr/bin/env bash +set -euo pipefail +args="$*" +if [[ "$args" == *"/actions/runs/77/cancel"* ]]; then + : > {cancelled!s} + exit 0 +fi +if [[ "$args" == *"/actions/runs/77"* ]]; then + printf '%s\\n' '{run_payload}' + exit 0 +fi +if [[ "$args" == *"/pulls?state=open&per_page=100"* ]]; then + printf '%s\\n' '{open_pr_payload}' + exit 0 +fi +if [[ "$args" == *"/git/ref/heads/feature/race"* ]]; then + {'exit 74' if fail_ref_lookup else f"printf '%s\\n' '{ref_sha}'"} + exit 0 +fi +exit 79 +""", + encoding="utf-8", + ) + fake_gh.chmod(0o755) + env = os.environ.copy() + env["PATH"] = f"{bin_dir}{os.pathsep}{env['PATH']}" + result = subprocess.run( + [ + "bash", + str(SCRIPT), + "ContextualWisdomLab/example", + "77", + "main", + "d" * 40, + "{}", + "aged-orphan", + ], + capture_output=True, + text=True, + env=env, + check=False, + ) + return result, cancelled.exists() + + +def test_post_classification_head_movement_fails_closed(tmp_path: Path) -> None: + """A new exact head arriving after classification must never be cancelled.""" + old = "a" * 40 + new = "b" * 40 + result, cancelled = _run_case( + tmp_path, + snapshot_sha=old, + pr_sha=new, + ref_sha=new, + run_sha=new, + ) + assert result.returncode == 0, result.stderr + assert "moved after queue classification" in result.stdout + assert not cancelled + + +@pytest.mark.parametrize("failed_lookup", ["pr", "ref"]) +def test_final_lookup_failure_fails_closed( + tmp_path: Path, failed_lookup: str +) -> None: + """Unavailable final authoritative PR/ref state must preserve the candidate.""" + current = "b" * 40 + result, cancelled = _run_case( + tmp_path, + snapshot_sha=current, + pr_sha=current, + ref_sha=current, + run_sha="a" * 40, + fail_lookup=failed_lookup, + ) + assert result.returncode == 0, result.stderr + assert "could not be re-fetched" in result.stdout + assert not cancelled + + +def test_current_head_is_preserved(tmp_path: Path) -> None: + """Final live-ref validation must preserve sole current-head evidence.""" + current = "b" * 40 + result, cancelled = _run_case( + tmp_path, + snapshot_sha=current, + pr_sha=current, + ref_sha=current, + run_sha=current, + ) + assert result.returncode == 0, result.stderr + assert "authoritative current-head evidence" in result.stdout + assert not cancelled + + +def test_proven_predecessor_is_cancelled(tmp_path: Path) -> None: + """An unchanged final live ref may cancel a proven predecessor run.""" + current = "b" * 40 + result, cancelled = _run_case( + tmp_path, + snapshot_sha=current, + pr_sha=current, + ref_sha=current, + run_sha="a" * 40, + ) + assert result.returncode == 0, result.stderr + assert cancelled + + +def test_unassociated_aged_pr_uses_live_ref_not_stale_listing_sha( + tmp_path: Path, +) -> None: + """A stale PR payload cannot authorize cancelling the live branch head.""" + listed = "a" * 40 + current = "b" * 40 + result, cancelled = _run_unassociated_pr_aged_orphan_case( + tmp_path, + listed_sha=listed, + ref_sha=current, + run_sha=current, + ) + assert result.returncode == 0, result.stderr + assert "authoritative current-head evidence" in result.stdout + assert not cancelled + + +def test_unassociated_aged_pr_live_ref_lookup_failure_fails_closed( + tmp_path: Path, +) -> None: + """Missing final ref evidence must preserve an unassociated PR candidate.""" + result, cancelled = _run_unassociated_pr_aged_orphan_case( + tmp_path, + listed_sha="a" * 40, + ref_sha="b" * 40, + run_sha="b" * 40, + fail_ref_lookup=True, + ) + assert result.returncode == 0, result.stderr + assert "live ref" in result.stdout + assert "could not be re-fetched" in result.stdout + assert not cancelled + + +@pytest.mark.parametrize( + "event", + ["workflow_dispatch", "workflow_run", "repository_dispatch", "issues"], +) +def test_aged_orphan_events_remain_cancellable(tmp_path: Path, event: str) -> None: + """Final revalidation must not disable legacy aged-orphan queue cleanup.""" + result, cancelled = _run_aged_orphan_case(tmp_path, event=event) + assert result.returncode == 0, result.stderr + assert cancelled + + +def test_aged_orphan_that_started_running_is_preserved(tmp_path: Path) -> None: + """Aged-orphan mode applies only while the candidate is still queued.""" + result, cancelled = _run_aged_orphan_case( + tmp_path, event="workflow_dispatch", status="in_progress" + ) + assert result.returncode == 0, result.stderr + assert "no longer queued" in result.stdout + assert not cancelled diff --git a/tests/test_queue_cancellation_scheduler_contract.py b/tests/test_queue_cancellation_scheduler_contract.py new file mode 100644 index 0000000000..16c291a6f1 --- /dev/null +++ b/tests/test_queue_cancellation_scheduler_contract.py @@ -0,0 +1,52 @@ +"""Structural contracts for final-state queue cancellation revalidation.""" + +from __future__ import annotations + +import os +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +WORKFLOW = ROOT / ".github" / "workflows" / "pr-review-merge-scheduler.yml" +HELPER = ROOT / "scripts" / "ci" / "revalidate_queue_cancellation.sh" +TEMP_WRITER = ROOT / ".github" / "workflows" / "_temp_pr1348_final_revalidation_repair.yml" + + +def test_scheduler_revalidates_each_destructive_candidate() -> None: + workflow = WORKFLOW.read_text(encoding="utf-8") + + assert workflow.count("scripts/ci/revalidate_queue_cancellation.sh") == 2 + assert '"superseded"' in workflow + assert '"aged-orphan"' in workflow + assert 'gh api -X POST "/repos/${repo_full_name}/actions/runs/${run_id}/cancel"' not in workflow + + +def test_initial_snapshot_is_bounded_without_serial_live_ref_fanout() -> None: + workflow = WORKFLOW.read_text(encoding="utf-8") + queue_block = workflow.split("# Queue hygiene, part 1:", 1)[1].split( + "# Queue hygiene, part 2:", 1 + )[0] + + assert "/pulls?state=open&per_page=100" in queue_block + assert "all(.[];" in queue_block + assert 'test("^[0-9a-fA-F]{40}$")' in queue_block + assert "/git/ref/heads/" not in queue_block + assert "ORG_QUEUE_HYGIENE_MAX_REF_LOOKUPS" not in workflow + + +def test_revalidation_helper_is_executable_and_temp_writer_is_retired() -> None: + assert HELPER.is_file() + assert os.access(HELPER, os.X_OK) + assert not TEMP_WRITER.exists() + + +def test_reconciled_scheduler_preserves_current_main_control_plane_fixes() -> None: + workflow = WORKFLOW.read_text(encoding="utf-8") + + assert '- cron: "0 * * * *"' in workflow + assert '*/15 * * * *' not in workflow + assert workflow.count("runs-on: ubuntu-24.04") >= 3 + scan_job = workflow.split(" scan-pr-queue:", 1)[1].split(" org-queue-sweep:", 1)[0] + assert "github.event_name == 'pull_request_review'" in scan_job.split( + "TRIGGER_REVIEWS:", 1 + )[1].splitlines()[0] diff --git a/tests/test_repository_label_taxonomy.py b/tests/test_repository_label_taxonomy.py index 0a9161c803..67762347ef 100644 --- a/tests/test_repository_label_taxonomy.py +++ b/tests/test_repository_label_taxonomy.py @@ -24,6 +24,9 @@ def test_repository_label_taxonomy_maps_evidence_backed_types() -> None: # Keep assignments exact so reviewed target drift cannot silently escape CI. assert payload["assignments"] == [ {"repository": ".github", "issue": 1582, "type": "feature"}, + {"repository": ".github", "issue": 1622, "type": "feature"}, + {"repository": ".github", "issue": 1625, "type": "bug"}, + {"repository": ".github", "issue": 1634, "type": "documentation"}, {"repository": "CalendarWeave", "issue": 1, "type": "documentation"}, {"repository": "ConceptWeave", "issue": 1, "type": "feature"}, { @@ -70,5 +73,54 @@ def test_repository_label_taxonomy_maps_evidence_backed_types() -> None: "type": "feature", }, {"repository": "noema", "issue": 530, "type": "feature"}, + {"repository": "bandscope", "issue": 1125, "type": "documentation"}, + {"repository": "saju-caldav", "issue": 44, "type": "documentation"}, + {"repository": "OriginWeave", "issue": 274, "type": "documentation"}, + { + "repository": "semantic-data-portal", + "issue": 90, + "type": "documentation", + }, + { + "repository": "accounting-information-platform", + "issue": 45, + "type": "documentation", + }, + {"repository": "clearfolio", "issue": 538, "type": "documentation"}, + {"repository": "pg-erd-cloud", "issue": 1046, "type": "documentation"}, + {"repository": "DiagramWeave", "issue": 34, "type": "documentation"}, + {"repository": "keyverse", "issue": 127, "type": "documentation"}, + { + "repository": "mhtml-etl-gateway", + "issue": 56, + "type": "documentation", + }, + {"repository": "j-planner", "issue": 2, "type": "documentation"}, + { + "repository": "learning-record-store", + "issue": 1, + "type": "documentation", + }, + { + "repository": "learning-content-studio", + "issue": 1, + "type": "documentation", + }, + { + "repository": "learning-management-platform", + "issue": 1, + "type": "documentation", + }, + { + "repository": "metering-billing-platform", + "issue": 157, + "type": "documentation", + }, + {"repository": "PolicyWeave", "issue": 1, "type": "feature"}, + { + "repository": "supply-chain-control-plane", + "issue": 1, + "type": "feature", + }, ] assert len(set(payload["type"].values())) == len(payload["type"]) diff --git a/tests/test_repository_metadata_reconciliation.py b/tests/test_repository_metadata_reconciliation.py index 2122c5d070..2bfc9d1386 100644 --- a/tests/test_repository_metadata_reconciliation.py +++ b/tests/test_repository_metadata_reconciliation.py @@ -71,6 +71,20 @@ def test_metadata_manifest_declares_exact_casing_and_public_surfaces() -> None: "fast-mlsirm": ("psychometrics", "rust"), "EgressWeave": ("ssrf", "python"), "psychometrics-commons": ("psychometrics", "rust"), + "keyverse": ("identity", "openid-connect"), + "OriginWeave": ("browser-automation", "ai-agents"), + "accounting-information-platform": ("accounting", "ledger"), + "pg-erd-cloud": ("erd", "postgresql"), + "clearfolio": ("document-viewer", "document-conversion"), + "DiagramWeave": ("diagram-editor", "plantuml"), + "semantic-data-portal": ("data-catalog", "semantic-search"), + "contextual-orchestrator": ("llm-orchestration", "model-routing"), + "mhtml-etl-gateway": ("mhtml", "etl"), + "PolicyWeave": ("privacy-policy", "typescript"), + "supply-chain-control-plane": ("supply-chain", "rust"), + "learning-management-platform": ("learning-management-system", "rust"), + "learning-content-studio": ("lcms", "content-authoring"), + "learning-record-store": ("learning-record-store", "xapi"), } assert set(repositories) == set(expected) for repository, required_topics in expected.items(): @@ -227,7 +241,7 @@ def test_pages_and_docs_probes(monkeypatch) -> None: responses = iter( [ - completed(out='{"type": "file"}'), + completed(out='{"type":"file"}'), completed(code=1, out="Not Found"), completed(code=1, err="boom"), ] diff --git a/tests/test_repository_metadata_workflow.py b/tests/test_repository_metadata_workflow.py new file mode 100644 index 0000000000..7b41a667d9 --- /dev/null +++ b/tests/test_repository_metadata_workflow.py @@ -0,0 +1,18 @@ +"""Static contracts for the privileged repository metadata workflow.""" + +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +WORKFLOW = ROOT / ".github" / "workflows" / "repository-metadata-reconcile.yml" + + +def test_metadata_apply_uses_dedicated_least_privilege_credential() -> None: + """Repository settings writes must not reuse the review/merge credential.""" + source = WORKFLOW.read_text(encoding="utf-8") + + assert "secrets.CWL_REPOSITORY_METADATA_TOKEN" in source + apply_source = source.split(" apply:", 1)[1] + assert "secrets.PR_REVIEW_MERGE_TOKEN" not in apply_source + assert "Require dedicated repository settings credential" in apply_source + assert 'test -n "${GH_TOKEN}"' in apply_source