diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index 1bb83c2baf..9062f15b32 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -1,6 +1,6 @@ # Reusable Dependency Review (workflow_call), consolidating the near- -# identical dependency-review.yml files argos, mightyETL, newsdom-api, -# scopeweave, and naruon each carried independently. See +# identical dependency-review.yml files argos, mightyETL, naruon, newsdom-api, +# and scopeweave each carried independently. See # docs/adr/0024-dependency-review-reusable-workflow-consolidation.md and # docs/doctoring/dependency-review-reusable-workflow-consolidation.md for the # per-repo field audit behind these inputs. @@ -9,34 +9,41 @@ # calling repo's own thin workflow file -- a workflow_call target cannot also # be the thing GitHub triggers directly on pull_request. # -# Dependency Review requires GitHub Dependency Graph (and, on private repos -# without GitHub Advanced Security, it is unavailable regardless of a repo's -# own settings). scopeweave's original workflow already detected this -# dynamically via the dependency-graph compare API instead of assuming from -# public/private repository status (mightyETL's original approach, which is -# wrong for a private repo that does have GHAS). This reusable workflow -# adopts the dynamic detection as the common, more-correct behavior for -# every caller, so no per-repo public/private input is needed. +# Dependency Review requires a successful Dependency Graph comparison for the +# exact pull-request base/head. Repository visibility is not a sufficient +# capability signal, and HTTP 403/404 are not safe availability signals: +# GitHub can use those statuses for authorization/policy denials as well as +# unavailable resources. For a pull_request, only exact immutable base/head +# object IDs plus a valid owner/name repository identity may reach transport, +# and only HTTP 200 authorizes running the pinned action. Every non-200 +# comparison fails closed. Non-pull_request triggers may skip because they do +# not carry the exact PR base/head identity. +# +# Reusable-workflow permissions can only be maintained or reduced through the +# call chain. Every thin caller therefore must declare at least `contents: +# read` and `pull-requests: read`; this workflow cannot elevate a caller token +# that omitted those scopes. # # Example caller (.github/workflows/dependency-review.yml in a product repo). -# Pin `uses:` to this file's exact commit SHA, not @main: an unpinned mutable -# ref would run an unreviewed central change against every PR check in the -# calling repo (Devin flagged this on the first four callers; fixed in all of -# them). If the calling repo's branch protection requires a status check -# literally named after the old standalone job, converting to `uses:` here -# will rename the published check to " / dependency-review" and -# silently break that required check -- update the branch protection's -# required-check name to match before or immediately after merging a caller. +# Pin `uses:` to the exact protected-main commit SHA carrying this workflow, +# never `@main`: a mutable central ref could run an unreviewed workflow change +# against the caller's PR checks. If branch protection requires the old +# standalone job name, note that reusable workflow adoption publishes the +# combined check name ` / dependency-review`; update the required +# check name to the exact published context without weakening the gate. # # name: Dependency Review # on: # pull_request: +# permissions: +# contents: read +# pull-requests: read # concurrency: # group: dependency-review-${{ github.event.pull_request.number || github.ref }} # cancel-in-progress: true # jobs: # dependency-review: -# uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@ +# uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@ # with: # fail_on_severity: high # allow_ghsas: "GHSA-69w3-r845-3855" @@ -121,6 +128,23 @@ jobs: exit 0 fi + 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 + api_url="${GITHUB_API_URL:-https://api.github.com}" response_file="$(mktemp)" status="$( @@ -137,14 +161,10 @@ jobs: exit 0 fi - if [ "$status" = "403" ] || [ "$status" = "404" ]; then - echo "::warning::Dependency graph compare returned HTTP ${status} for ${REPOSITORY}; skipping the dependency-review hard gate (GitHub Dependency Graph, or GitHub Advanced Security on a private repository, is unavailable)." - echo "available=false" >>"$GITHUB_OUTPUT" - exit 0 + echo "::error::Dependency graph comparison failed with HTTP ${status}. For a pull_request, non-200 responses are ambiguous between feature availability and authorization/policy/transport failure, so Dependency Review fails closed instead of being skipped." + if [ -s "$response_file" ]; then + cat "$response_file" fi - - echo "::error::Dependency graph availability check failed with HTTP ${status}. This is not a 'graph unavailable' response (403/404) -- treating it as a genuine failure instead of silently skipping the security gate." - cat "$response_file" exit 1 - name: Dependency review @@ -155,9 +175,3 @@ jobs: fail-on-severity: ${{ inputs.fail_on_severity }} allow-ghsas: ${{ inputs.allow_ghsas }} comment-summary-in-pr: ${{ inputs.comment_summary_in_pr }} - - - name: Dependency graph unavailable note - if: steps.dependency_graph.outputs.available != 'true' && github.event_name == 'pull_request' - run: | - echo "Dependency Review requires GitHub Dependency Graph to be enabled for this repository (and, on private repositories, GitHub Advanced Security)." - echo "Other required dependency-vulnerability gates (OSV-Scanner, Scorecard) remain the blocking coverage until Dependency Graph is available here." diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 500e22b4ab..3c41dbf819 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -383,6 +383,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 \ diff --git a/docs/adr/0025-dependency-review-fail-closed-permission-envelope.md b/docs/adr/0025-dependency-review-fail-closed-permission-envelope.md new file mode 100644 index 0000000000..e7eb7528b5 --- /dev/null +++ b/docs/adr/0025-dependency-review-fail-closed-permission-envelope.md @@ -0,0 +1,72 @@ +# ADR-0025: Fail closed on ambiguous Dependency Review authority and preserve caller permissions + +- **Status:** Proposed +- **Date:** 2026-09-02 +- **Scope:** `.github/workflows/dependency-review.yml`, its thin product callers, and the reusable-workflow security contract +- **Supersedes:** ADR-0024 only where ADR-0024 treated HTTP 403/404 as confirmed Dependency Graph unavailability or showed callers without an explicit permission envelope + +## Problem + +The protected-main consolidation from #1724 exposed two independent security-contract defects. + +First, the reusable workflow classified Dependency Graph compare HTTP 403/404 as `available=false` and therefore skipped the blocking Dependency Review action. Those responses are not authoritative proof that the feature is unavailable: they can also be authorization or policy failures. Turning an ambiguous authorization-shaped response into success silently removes a security gate. + +Second, the migration examples and thin callers omitted the permission envelope that the original repository-local workflows carried. GitHub reusable workflows cannot elevate `GITHUB_TOKEN` permissions through the call chain. A called workflow may maintain or reduce permissions granted by the caller, but it cannot manufacture `pull-requests: read` when the caller did not grant it. The result is a workflow-level `startup_failure` before any job is created. + +## Constraints + +1. Dependency Review remains a distinct hard gate; OSV-Scanner, Scorecard, or another scanner cannot substitute for an ambiguous Dependency Review failure. +2. The called workflow needs only `contents: read` and `pull-requests: read`; no write permission is introduced. +3. Product callers remain thin and repository-owned. They keep repository-specific trigger, severity, allowlist, and `continue_on_error` policy. +4. Consumers pin the reusable workflow to an immutable protected-main commit after this proposal is merged. `@main`, branch URLs, and unmerged PR heads are not production authority. +5. Non-`pull_request` invocations may skip because they lack an exact PR base/head pair; pull requests fail closed unless the comparison endpoint returns HTTP 200. + +## Considered alternatives + +### Treat 403/404 as feature unavailable + +Rejected. The status code alone cannot distinguish a genuinely unavailable Dependency Graph from denied authorization/policy. A false negative here converts a required security control into a warning. + +### Infer support from repository visibility or GHAS assumptions + +Rejected. Visibility is not a capability proof and was already the weaker design ADR-0024 replaced. + +### Rely on the called workflow's `permissions:` block + +Rejected as insufficient. GitHub does not let a reusable workflow elevate permissions beyond the caller's grant. The called workflow still declares its least-privilege ceiling, but each thin caller must explicitly grant the same read scopes. + +### Grant broader token permissions globally + +Rejected. It increases blast radius and hides a caller-contract defect instead of repairing it. + +## Decision + +1. For `pull_request`, the Dependency Graph compare preflight sets `available=true` only on HTTP 200. Every other HTTP status is emitted with an error and terminates the job nonzero. +2. Remove the pull-request "Dependency graph unavailable" success path. No alternate scanner is described as replacement authority. +3. Keep the called workflow at `contents: read` + `pull-requests: read` and require every thin caller to declare at least those same scopes explicitly. +4. Make the executable central contract fail when the canonical caller example omits either required scope. +5. Replace the mutable `@main` caller example with an immutable `` placeholder. After merge, consumers pin the resulting protected-main SHA. + +## Exact evidence + +- #1725 first RED: `cb07b8bb28ef9d3a147cc966a0c70654d132da1d`; first production repair: `31f60e532e135008cabd09fcddd46a53062b0ea0`. +- Permission-envelope RED: `ee0f1ce544965772775b590050e40476df4ea8f6`; it changes only the contract and requires the missing caller permission example. +- Permission-envelope production repair: `ca3bdbd210de988ccd31f7fb96d3a97adfdb9bff`. +- `newsdom-api#784@1623977e6c37c78cb1a94a7a48c48f6d02cac86c`: run `33622976911`, `startup_failure`, zero jobs, reusable workflow immutably resolved to `.github@0bcd22d8bb07650aafb0a8f116e4c2bbb8744f03`. +- `mightyETL#330@65efdf7b4064df5b9811c0403defb707e6efbc02`: run `33623035969`, `startup_failure`, zero jobs. +- Consumer permission repairs then produced materialized current-head runs: newsdom-api `9a798d5ac7b9b295a1accb2327fc76611352290f` run `33623818000`; mightyETL `4576f863ede9fca0673d6cce5ae8a4093246f5ab` run `33623854807`; scopeweave `db8b8ed6d36a6dc6cc1d07255a7a9a86bc88bf4f` run `33623761776`; Argos #557 `ee4c5dd326977407435b0f2425fdecebc34a810f` run `33623867278`. + +Hosted exact-current-head Checks and independent review remain required before this ADR may become Accepted. + +## Consequences and follow-up + +- A missing or denied Dependency Graph comparison is visible as a blocking failure instead of silent coverage loss. +- Caller permission omissions become an executable contract defect rather than an undocumented deployment prerequisite. +- The central workflow still cannot repair a consumer's omitted permissions by itself; each consumer must carry the explicit read-only envelope and later bump its immutable reusable-workflow pin to the protected-main SHA that contains this decision. +- #1643 remains a separate diagnostic lane for the required Security Scan path and is not evidence transfer for this reusable Dependency Review gate. + +## References + +GitHub. (n.d.). *Reusing workflow configurations*. GitHub Docs. https://docs.github.com/actions/using-workflows/reusing-workflows + +GitHub. (n.d.). *Use GITHUB_TOKEN for authentication in workflows*. GitHub Docs. https://docs.github.com/actions/security-guides/automatic-token-authentication diff --git a/docs/doctoring/dependency-review-fail-closed-permission-envelope.md b/docs/doctoring/dependency-review-fail-closed-permission-envelope.md new file mode 100644 index 0000000000..ef010dc741 --- /dev/null +++ b/docs/doctoring/dependency-review-fail-closed-permission-envelope.md @@ -0,0 +1,99 @@ +# Dependency Review fail-closed authority and caller permission envelope + +## Incident boundary + +Protected `ContextualWisdomLab/.github` main introduced the central reusable Dependency Review workflow through #1724. Subsequent exact-head evidence exposed three owner-contract defects that are repaired together in #1725 because all belong to the canonical dependency-review admission boundary. + +### Defect A — ambiguous HTTP status normalized to success + +The initial reusable preflight treated Dependency Graph compare HTTP 403/404 as proof that the feature was unavailable, wrote `available=false`, and let the pull-request gate finish successfully. That inference is unsafe: an authorization/policy denial can present the same status shape. The repair keeps the exact base/head compare request but authorizes the action only on HTTP 200; every other pull-request response is blocking and reports its status. + +Test-first evidence: + +- RED `cb07b8bb28ef9d3a147cc966a0c70654d132da1d` makes 403/404-as-unavailable, the fallback note, and any non-explicit fail-closed response illegal. +- Production `31f60e532e135008cabd09fcddd46a53062b0ea0` removes the 403/404 success branch and fallback note, preserving the pinned action, inputs, and non-`pull_request` skip boundary. + +### Defect B — thin callers lost required `GITHUB_TOKEN` permissions + +The original repository-local workflows carried read permission envelopes, but the migration examples/thin callers did not preserve them uniformly. GitHub reusable workflows cannot elevate the caller token. Consequently the called workflow's own `permissions: contents: read, pull-requests: read` declaration is only a ceiling; it cannot grant `pull-requests: read` when the caller omitted it. + +Live immutable-pin evidence isolates this from mutable-ref resolution: + +| Consumer | Exact head | Run | Result before caller repair | +| --- | --- | --- | --- | +| `ContextualWisdomLab/newsdom-api#784` | `1623977e6c37c78cb1a94a7a48c48f6d02cac86c` | `33622976911` | `startup_failure`, zero jobs; referenced workflow resolved to `.github@0bcd22d8bb07650aafb0a8f116e4c2bbb8744f03` | +| `ContextualWisdomLab/mightyETL#330` | `65efdf7b4064df5b9811c0403defb707e6efbc02` | `33623035969` | `startup_failure`, zero jobs | + +The consumer-side repair explicitly restores: + +```yaml +permissions: + contents: read + pull-requests: read +``` + +Fresh heads then materialized Dependency Review runs instead of failing before job creation: + +- newsdom-api `9a798d5ac7b9b295a1accb2327fc76611352290f`, run `33623818000`; +- mightyETL `4576f863ede9fca0673d6cce5ae8a4093246f5ab`, run `33623854807`; +- scopeweave `db8b8ed6d36a6dc6cc1d07255a7a9a86bc88bf4f`, run `33623761776`; +- Argos #557 `ee4c5dd326977407435b0f2425fdecebc34a810f`, run `33623867278`. + +Central test-first repair for this second defect: + +- RED `ee0f1ce544965772775b590050e40476df4ea8f6` adds `test_example_caller_preserves_required_permission_envelope()` without changing production/example workflow text. Against its parent it fails because the canonical caller example has no `permissions:` block. +- GREEN production `ca3bdbd210de988ccd31f7fb96d3a97adfdb9bff` adds the least-privilege caller envelope, explains the reusable-workflow permission ceiling, and replaces the mutable `@main` example with ``. + +### Defect C — compare identity was trusted before transport + +The earlier bundled Security Scan accepted the event-provided repository/base/head strings without first proving they were exact immutable Git object identities and one legal `owner/name` repository identity. That left the preflight contract weaker than the evidence it claimed to authorize. + +The predecessor owner lane #1643 added fail-before-transport validation and a temporary A/B canary. Its exact-head canary run `33589436750`, job `100120235906`, executed on `2026-09-02` and produced decisive evidence for the same immutable pair: + +- repository: `ContextualWisdomLab/.github`; +- base: `bb14b014eee31e6abdb5d2fffbb805aa29420eac`; +- head: `a6a2759640e6aa1d1e1219e1cd7aacdeffef32c0`; +- anonymous compare: HTTP `404`, curl exit `0`; +- job-token compare with `contents: read` and `pull-requests: read`: HTTP `200`, curl exit `0`. + +This proves that anonymous status is not an availability authority and that the least-privilege job token can establish the exact comparison. The temporary canary itself is diagnostic-only and is not part of the publishable successor contract. + +#1725 carries the durable part of that predecessor delta into the canonical writer: + +- both the reusable workflow and bundled Security Scan reject non-exact base/head revisions before curl; +- repository identity must be exactly one non-dot `owner/name` pair; `ContextualWisdomLab/.github` remains legal; +- the compare request uses the job token and only HTTP 200 authorizes the pinned Dependency Review action; +- named refs, malformed identities, 403/404 and all other non-200 outcomes fail closed. + +## Security invariants + +1. Pull-request Dependency Review executes only after an exact base/head compare returns HTTP 200. +2. Base/head revisions must be exact 40- or 64-character lowercase hexadecimal Git object IDs before transport. +3. Repository identity must be exactly one non-dot `owner/name` pair before transport. +4. No anonymous response, HTTP 403/404, or other non-200 response is translated into a successful "unavailable" state. +5. OSV-Scanner, Scorecard, and the separate Security Scan path remain independent controls; they do not satisfy a failed Dependency Review gate. +6. The called workflow and each caller use only `contents: read` and `pull-requests: read` for this path. No write permission is introduced. +7. Product callers pin the central workflow to an immutable protected-main commit after merge. `@main`, PR heads, and branch URLs are not production authority. +8. A non-`pull_request` event may skip because it lacks the PR base/head identity required for the comparison. + +## Verification and merge boundary + +The focused owner contracts are: + +```bash +PYTHONPATH=. pytest -q \ + tests/test_dependency_review_reusable_workflow_contract.py \ + tests/test_dependency_review_bundled_scan_identity_contract.py +``` + +The repository's normal exact-current-head required Checks, full coverage evidence, security scans, and independent reviews remain authoritative. #1725 stays Proposed/Draft while those gates are non-terminal or any substantive finding is unresolved. Queue saturation does not authorize bypass of a startup, permission, provenance, review, or security defect. + +After #1725 reaches protected main through ordinary protection, each consumer must bump its immutable reusable-workflow pin to that protected-main SHA and regenerate exact-head Dependency Review evidence. No consumer should return to `@main`. + +## References + +GitHub. (n.d.-a). *Reusing workflow configurations*. GitHub Docs. https://docs.github.com/actions/using-workflows/reusing-workflows + +GitHub. (n.d.-b). *Use GITHUB_TOKEN for authentication in workflows*. GitHub Docs. https://docs.github.com/actions/security-guides/automatic-token-authentication + +GitHub. (n.d.-c). *REST API endpoints for the dependency graph*. GitHub Docs. https://docs.github.com/rest/dependency-graph diff --git a/tests/test_dependency_review_bundled_scan_identity_contract.py b/tests/test_dependency_review_bundled_scan_identity_contract.py new file mode 100644 index 0000000000..cee8d44374 --- /dev/null +++ b/tests/test_dependency_review_bundled_scan_identity_contract.py @@ -0,0 +1,46 @@ +"""Regression contract for the bundled Security Scan Dependency Review preflight.""" + +from __future__ import annotations + +from pathlib import Path + + +_WORKFLOW = Path(".github/workflows/security-scan.yml") + + +def _workflow_text() -> str: + """Return the bundled Security Scan workflow as UTF-8 text.""" + return _WORKFLOW.read_text(encoding="utf-8") + + +def test_bundled_scan_requires_exact_git_object_ids_before_transport() -> None: + """Named or malformed base/head revisions must fail before compare transport.""" + workflow = _workflow_text() + assert "git_object_id='^[0-9a-f]{40}([0-9a-f]{24})?$'" in workflow + assert 'if ! [[ "${BASE_SHA}" =~ $git_object_id ]]' in workflow + assert '! [[ "${HEAD_SHA}" =~ $git_object_id ]]' in workflow + assert "exact 40- or 64-character hexadecimal base and head revisions" in workflow + assert "Named refs are not evidence" in workflow + + +def test_bundled_scan_requires_one_non_dot_owner_name_identity() -> None: + """Repository identity validation keeps .github legal but rejects path sentinels.""" + workflow = _workflow_text() + assert "repository_identity='^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$'" in workflow + assert 'if ! [[ "${REPOSITORY}" =~ $repository_identity ]]' in workflow + assert 'repository_owner="${REPOSITORY%%/*}"' in workflow + assert 'repository_name="${REPOSITORY#*/}"' in workflow + assert '[ "${repository_owner}" = "." ]' in workflow + assert '[ "${repository_owner}" = ".." ]' in workflow + assert '[ "${repository_name}" = "." ]' in workflow + assert '[ "${repository_name}" = ".." ]' in workflow + + +def test_bundled_scan_uses_job_token_and_fails_closed_on_non_200() -> None: + """Only an authenticated successful exact comparison may admit Dependency Review.""" + workflow = _workflow_text() + assert "GH_TOKEN: ${{ github.token }}" in workflow + assert '-H "Authorization: Bearer ${GH_TOKEN}"' in workflow + assert 'if [ "$curl_status" -ne 0 ] || [ "$http_status" != "200" ]; then' in workflow + assert 'echo "supported=true" >>"$GITHUB_OUTPUT"' in workflow + assert "actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294" in workflow diff --git a/tests/test_dependency_review_reusable_workflow_contract.py b/tests/test_dependency_review_reusable_workflow_contract.py index cadefcb8ac..ea55f91cfd 100644 --- a/tests/test_dependency_review_reusable_workflow_contract.py +++ b/tests/test_dependency_review_reusable_workflow_contract.py @@ -1,6 +1,6 @@ """Contract for the reusable Dependency Review workflow. -Replaces argos's, mightyETL's, newsdom-api's, and scopeweave's +Replaces Argos's, mightyETL's, naruon's, newsdom-api's, and scopeweave's independently hand-written ``dependency-review.yml`` files with one reusable ``workflow_call`` workflow, ``.github/workflows/dependency-review.yml``, plus a thin caller left in each product repository. See @@ -11,6 +11,9 @@ from __future__ import annotations +import os +import subprocess +import textwrap from pathlib import Path _WORKFLOW = Path(".github/workflows/dependency-review.yml") @@ -24,8 +27,72 @@ def _workflow_text() -> str: return _WORKFLOW.read_text(encoding="utf-8") +def _availability_probe_script() -> str: + """Extract the dependency-graph preflight shell body for executable tests.""" + workflow = _workflow_text() + step = " - name: Check dependency graph availability\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) + script = textwrap.dedent(block[run_start:]) + return script.replace('${{ github.event_name }}', "pull_request") + + +def _run_availability_probe( + tmp_path: Path, + repository: str, + *, + base_sha: str = "a" * 40, + head_sha: str = "b" * 40, + http_status: str = "200", +) -> tuple[subprocess.CompletedProcess[str], Path, Path]: + """Execute the real preflight shell against a marker-only fake curl.""" + 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" + "authorized=false\n" + "for arg in \"$@\"; do\n" + " if [[ \"$arg\" == Authorization:* ]]; then authorized=true; fi\n" + "done\n" + "printf '%s\\n' \"$authorized\" >>\"${CURL_MARKER}\"\n" + "printf '%s' \"${HTTP_STATUS:-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": base_sha, + "HEAD_SHA": head_sha, + "REPOSITORY": repository, + "GITHUB_API_URL": "https://api.github.invalid", + "GITHUB_OUTPUT": str(output), + "CURL_MARKER": str(curl_marker), + "HTTP_STATUS": http_status, + } + ) + result = subprocess.run( + ["bash", "-c", _availability_probe_script()], + cwd=Path.cwd(), + env=env, + text=True, + capture_output=True, + check=False, + ) + return result, curl_marker, output + + def test_declares_workflow_call_with_four_inputs_and_recorded_defaults() -> None: - """Every genuinely-varying field found while auditing the five originals is an input.""" + """Every genuinely varying field found while auditing the five originals is an input.""" workflow = _workflow_text() assert "on:\n workflow_call:\n inputs:" in workflow for name in ( @@ -42,33 +109,33 @@ def test_declares_workflow_call_with_four_inputs_and_recorded_defaults() -> None assert 'default: "on-failure"' in workflow -def test_step_order_is_harden_then_checkout_then_preflight_then_gated_steps() -> None: - """harden-runner -> checkout -> dependency-graph preflight -> conditional gate/note.""" +def test_step_order_is_harden_then_checkout_then_preflight_then_dependency_review() -> None: + """Runner hardening, checkout, capability proof, then the gated action stay ordered.""" workflow = _workflow_text() order = [ "Harden the runner", "actions/checkout@", "Check dependency graph availability", "Dependency review", - "Dependency graph unavailable note", ] positions = [workflow.index(marker) for marker in order] assert positions == sorted(positions), "steps are out of order" -def test_dependency_review_and_note_steps_are_mutually_exclusive_on_availability() -> None: - """The gate and the fallback note must never both run.""" +def test_dependency_review_runs_only_after_a_confirmed_successful_comparison() -> None: + """The action must execute only after the compare endpoint returned HTTP 200.""" workflow = _workflow_text() assert ( "if: steps.dependency_graph.outputs.available == 'true'\n" " continue-on-error: ${{ inputs.continue_on_error }}" in workflow ) - assert "if: steps.dependency_graph.outputs.available != 'true'" in workflow + assert 'if [ "$status" = "200" ]; then' in workflow + assert 'echo "available=true" >>"$GITHUB_OUTPUT"' in workflow def test_inputs_are_forwarded_to_the_dependency_review_action() -> None: - """fail_on_severity, allow_ghsas, and comment_summary_in_pr must reach the action untouched.""" + """Every caller-varying action input must reach the pinned action untouched.""" workflow = _workflow_text() assert "fail-on-severity: ${{ inputs.fail_on_severity }}" in workflow assert "allow-ghsas: ${{ inputs.allow_ghsas }}" in workflow @@ -76,67 +143,126 @@ def test_inputs_are_forwarded_to_the_dependency_review_action() -> None: def test_harden_runner_audits_egress() -> None: - """naruon's harden-runner step applies uniformly, not only to that one caller.""" + """naruon's harden-runner control applies uniformly in the reusable owner.""" workflow = _workflow_text() assert "step-security/harden-runner@" in workflow assert "egress-policy: audit" in workflow def test_action_pins_are_current_and_uniform() -> None: - """checkout and dependency-review-action share one current pin, not per-caller drift.""" + """Checkout and Dependency Review use one immutable current pin.""" workflow = _workflow_text() assert f"actions/checkout@{_CHECKOUT_PIN}" in workflow - assert ( - f"actions/dependency-review-action@{_DEPENDENCY_REVIEW_PIN}" in workflow - ) + assert f"actions/dependency-review-action@{_DEPENDENCY_REVIEW_PIN}" in workflow def test_uniform_fields_are_hardcoded_not_parameterized() -> None: - """Fields byte-identical across all four originals stay static, not inputs.""" + """Uniform least-privilege and checkout controls stay static.""" workflow = _workflow_text() assert "permissions:\n contents: read\n pull-requests: read" in workflow assert "persist-credentials: false" in workflow +def test_example_caller_preserves_required_permission_envelope() -> None: + """Thin callers must explicitly pass the reusable job's read permission ceiling.""" + workflow = _workflow_text() + assert ( + "# permissions:\n" + "# contents: read\n" + "# pull-requests: read\n" + "# concurrency:" + in workflow + ) + + +def test_example_caller_requires_immutable_protected_main_pin() -> None: + """The canonical example must never teach consumers to execute a mutable owner ref.""" + workflow = _workflow_text() + assert "@" in workflow + assert "uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@main" not in workflow + + def test_forces_node24_runtime_for_js_actions() -> None: - """newsdom-api's Node24 opt-in applies uniformly, not only to that one caller.""" + """newsdom-api's Node24 opt-in applies uniformly, not only to one caller.""" workflow = _workflow_text() assert "FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true" in workflow def test_availability_check_uses_the_dependency_graph_compare_api() -> None: - """The preflight must query the real capability, not infer from repository visibility.""" + """The preflight must query the real capability, not infer from visibility.""" workflow = _workflow_text() assert "dependency-graph/compare" in workflow assert "github.event.repository.private" not in workflow + assert '-H "Authorization: Bearer ${GH_TOKEN}"' in workflow -def test_availability_check_distinguishes_unavailable_from_genuine_failure() -> None: - """403/404 means 'unavailable, skip gracefully'; any other status must hard-fail - the job instead of silently treating a real error the same as unavailability.""" +def test_pull_request_http_403_and_404_are_not_normalized_to_unavailable() -> None: + """Authorization-shaped HTTP responses are ambiguous and must remain blocking.""" workflow = _workflow_text() - assert 'if [ "$status" = "403" ] || [ "$status" = "404" ]' in workflow - assert "available=false" in workflow - assert "::error::Dependency graph availability check failed with HTTP" in workflow + assert 'if [ "$status" = "403" ] || [ "$status" = "404" ]' not in workflow + assert "skipping the dependency-review hard gate" not in workflow + assert "Dependency graph unavailable note" not in workflow + assert "::error::Dependency graph comparison failed with HTTP" in workflow assert "exit 1" in workflow def test_availability_check_only_runs_the_gate_for_pull_request_events() -> None: - """A non-pull_request trigger (e.g. workflow_dispatch) must skip the gate, not error, - since base/head SHAs only exist on a pull_request event.""" + """A non-pull_request trigger may skip because it has no PR base/head identity.""" workflow = _workflow_text() assert '"${{ github.event_name }}" != "pull_request"' in workflow +def test_preflight_rejects_named_revisions_before_transport(tmp_path: Path) -> None: + """Named or malformed revisions never reach the dependency-graph endpoint.""" + for index, (base_sha, head_sha) in enumerate( + (("main", "b" * 40), ("a" * 40, "develop"), ("a" * 39, "b" * 40)) + ): + case_dir = tmp_path / f"revision-{index}" + case_dir.mkdir() + result, curl_marker, _output = _run_availability_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_preflight_rejects_malformed_repository_before_transport(tmp_path: Path) -> None: + """Only one non-dot owner/name repository identity may reach transport.""" + repositories = ( + "ContextualWisdomLab", + "ContextualWisdomLab/Orgmetra/extra", + "/Orgmetra", + "../.github", + "ContextualWisdomLab/..", + "ContextualWisdomLab/.", + "./.github", + ) + for index, repository in enumerate(repositories): + case_dir = tmp_path / f"repository-{index}" + case_dir.mkdir() + result, curl_marker, _output = _run_availability_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_preflight_accepts_dotgithub_and_uses_job_token(tmp_path: Path) -> None: + """The legitimate .github repository reaches exactly one authenticated compare.""" + result, curl_marker, output = _run_availability_probe( + tmp_path, "ContextualWisdomLab/.github" + ) + assert result.returncode == 0, result.stdout + result.stderr + assert curl_marker.read_text(encoding="utf-8") == "true\n" + assert output.read_text(encoding="utf-8") == "available=true\n" + + def test_dependency_review_comment_summary_defaults_to_on_failure() -> None: - """scopeweave's PR-comment-on-failure UX applies uniformly by default, overridable per caller. - - naruon explicitly overrides it to "never" -- see - test_declares_workflow_call_with_four_inputs_and_recorded_defaults for - the default assertion and test_inputs_are_forwarded_to_the_dependency_review_action - for the forwarding assertion; this test just pins the specific default - value chosen (scopeweave's original, not naruon's or some other value). - """ + """The shared UX defaults to on-failure while remaining caller-overridable.""" workflow = _workflow_text() - assert 'comment_summary_in_pr:\n' in workflow + assert "comment_summary_in_pr:" in workflow assert 'default: "on-failure"' in workflow + assert "comment-summary-in-pr: ${{ inputs.comment_summary_in_pr }}" in workflow