fix(security): fail closed on ambiguous dependency-review HTTP responses

.github/workflows/dependency-review.yml

@@ -9,26 +9,33 @@
 # 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 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):
 #
 #   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@main
+#       uses: ContextualWisdomLab/.github/.github/workflows/dependency-review.yml@<protected-main-commit-sha>
 #       with:
 #         fail_on_severity: high
 #         allow_ghsas: "GHSA-69w3-r845-3855"
@@ -113,14 +120,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
@@ -131,9 +134,3 @@ jobs:
           fail-on-severity: ${{ inputs.fail_on_severity }}
           allow-ghsas: ${{ inputs.allow_ghsas }}
           comment-summary-in-pr: on-failure
-
-      - 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."

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 `<protected-main-commit-sha>` 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

docs/doctoring/dependency-review-fail-closed-permission-envelope.md

@@ -0,0 +1,74 @@
+# Dependency Review fail-closed authority and caller permission envelope
+
+## Incident boundary
+
+Protected `ContextualWisdomLab/.github` main introduced the central reusable Dependency Review workflow through #1724. Two migration defects were then observed independently and are repaired together in #1725 because both belong to the canonical reusable-workflow contract.
+
+### 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 `<protected-main-commit-sha>`.
+
+## Security invariants
+
+1. Pull-request Dependency Review executes only after an exact base/head compare returns HTTP 200.
+2. No HTTP 403/404 or other non-200 response is translated into a successful "unavailable" state.
+3. OSV-Scanner, Scorecard, and the separate Security Scan path remain independent controls; they do not satisfy a failed Dependency Review gate.
+4. The called workflow and each caller use only `contents: read` and `pull-requests: read` for this path. No write permission is introduced.
+5. Product callers pin the central workflow to an immutable protected-main commit after merge. `@main`, PR heads, and branch URLs are not production authority.
+6. 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 contract is:
+
+```bash
+PYTHONPATH=. pytest -q tests/test_dependency_review_reusable_workflow_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

tests/test_dependency_review_reusable_workflow_contract.py

@@ -36,28 +36,28 @@ def test_declares_workflow_call_with_three_inputs_and_recorded_defaults() -> Non
     assert "default: false" in workflow
 
 
-def test_step_order_is_checkout_then_preflight_then_gated_steps() -> None:
-    """checkout -> dependency-graph preflight -> conditional gate/note, in that order."""
+def test_step_order_is_checkout_then_preflight_then_dependency_review() -> None:
+    """Checkout, capability preflight, then the gated action must remain ordered."""
     workflow = _workflow_text()
     order = [
         "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:
@@ -83,6 +83,18 @@ def test_uniform_fields_are_hardcoded_not_parameterized() -> None:
     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_forces_node24_runtime_for_js_actions() -> None:
     """newsdom-api's Node24 opt-in applies uniformly, not only to that one caller."""
     workflow = _workflow_text()
@@ -96,19 +108,23 @@ def test_availability_check_uses_the_dependency_graph_compare_api() -> None:
     assert "github.event.repository.private" not 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.
+
+    GitHub may use 403 or 404 for permission/policy failures as well as feature
+    availability boundaries. A reusable security gate therefore cannot turn
+    either response into success or delegate coverage to another scanner.
+    """
     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