feat: fail closed on unverified head snapshot in dependency-vuln-check

dependency-vuln-check/README.md

@@ -55,6 +55,7 @@ version shows up as `added` in the diff and is evaluated normally.
 | `base-ref` | yes | — | Base **branch** of the PR (e.g. `main`, `stable/8.8`). Used to find the nearest snapshotted ancestor on the correct branch |
 | `fallback-base-ref` | no | `main` | Branch to fall back to when `base-ref` has no dependency snapshots (e.g. stacked PRs targeting a feature branch). The gate searches this branch for the nearest snapshotted ancestor of `base-sha` instead of failing closed, and posts a notice to the PR comment |
 | `snapshot-workflow` | yes | — | Filename of the workflow that submits the base snapshot (e.g. `maven-dependency-snapshot.yml`). Its successful push-event runs are scanned to resolve the effective base |
+| `head-snapshot-succeeded` | no | `""` | Whether the job that submits the PR **head** snapshot succeeded. Pass `'true'`/`'success'` (e.g. `needs.pr-maven-snapshot.result == 'success'`). **Any other value** (`'false'`, or a raw result like `'failure'`/`'cancelled'`/`'skipped'`) **fails closed** — an un-submitted head SBOM leaves the head side empty → a real new vuln would silently pass, so an unverified head must block, not no-op. Unset skips the check (backward compatible) |
 | `max-snapshot-lookback` | no | `30` | How many recent successful snapshot runs to scan when resolving the effective base |
 | `override-label` | no | `ci:vuln-gate-override` | PR label that bypasses the gate **only** when it cannot verify the PR (outage / no-ancestor). Never bypasses a real finding |
 | `config-file` | no | `.github/dependency-review-config.json` | Path to the JSON config holding `allow-ghsas` |
@@ -101,12 +102,27 @@ The gate **fails closed** when it cannot verify a PR:
 | API error survives retries | **fail closed** (block), reason named in the log |
 | No snapshotted ancestor on `base-ref`; `base-ref` differs from `fallback-base-ref` | retry on `fallback-base-ref`; notice posted to PR comment |
 | No snapshotted ancestor on either branch within `max-snapshot-lookback` | **fail closed** (block) |
+| `head-snapshot-succeeded` reported as `'false'` (head SBOM submission failed) | **fail closed** (block) |
+| Head side of the diff looks empty (0 added, deps removed) — head SBOM still indexing | re-fetch (3 attempts, backoff), then trust the settled diff |
 | API works, real vulnerable dependency added | block (normal finding) |
 | API works, no vulnerable dependency added | pass |
 
 Every run writes a summary trail (resolved base, runs scanned, verdict). Failure reasons are
 named explicitly in the log (rate-limit vs 5xx vs timeout vs permissions vs not-found).
 
+### Head-side verification
+
+The base side is resolved to a verified snapshotted ancestor, but the head side of the diff is
+only as good as the head SBOM. Two guards keep an unverified head from silently passing a real
+new vulnerability (a false **negative**):
+
+- **Submission failure** — the consumer passes `head-snapshot-succeeded`. If the head-SBOM job
+  did not succeed the head tree is stale/absent, so the gate fails closed rather than trust it.
+- **Indexing lag** — a fresh head SBOM can take a few seconds to index. When the head side of
+  the diff reads as empty (0 added, deps removed) the gate re-fetches the compare a few times to
+  let the graph settle. A legitimate removal-only PR has the same shape and simply re-confirms —
+  the gate never *blocks* on this signal, it only waits out indexing lag, so it cannot misfire.
+
 ### Ignoring dependencies the base branch itself added
 
 **What this prevents:** a PR being blocked for a vulnerable dependency it never touched,
@@ -242,7 +258,9 @@ your-repo/
 - For transitive / BOM-pinned dependencies to appear in the diff, both base and head
   commits need a submitted dependency snapshot (GitHub Dependency Submission API). The
   base side is resolved automatically to the nearest snapshotted ancestor; the head side
-  must be submitted by the consuming workflow before this action runs.
+  must be submitted by the consuming workflow before this action runs. Pass
+  `head-snapshot-succeeded` so the gate fails closed if that submission failed, and it
+  re-fetches to absorb indexing lag (see [Head-side verification](#head-side-verification)).
 
 ## Tests
 

dependency-vuln-check/action.yml

@@ -27,6 +27,17 @@ inputs:
       (e.g. maven-dependency-snapshot.yml). Its successful push-event runs are
       scanned to resolve the effective base.
     required: true
+  head-snapshot-succeeded:
+    description: >
+      Whether the job that submits the PR head's Maven dependency snapshot
+      succeeded. Pass 'true'/'success' when it did (e.g.
+      needs.pr-maven-snapshot.result == 'success'). Any OTHER value — 'false' or a
+      raw job result like 'failure'/'cancelled'/'skipped' — fails the gate closed,
+      because an un-submitted head SBOM leaves the head side of the diff empty and
+      a real new vulnerability would silently pass. Leave unset to skip the check
+      (backward compatible).
+    required: false
+    default: ""
   fallback-base-ref:
     description: >
       Branch to fall back to when base-ref has no dependency snapshots (e.g.
@@ -83,6 +94,7 @@ runs:
         HEAD_SHA: ${{ inputs.head-sha }}
         BASE_REF: ${{ inputs.base-ref }}
         SNAPSHOT_WORKFLOW: ${{ inputs.snapshot-workflow }}
+        HEAD_SNAPSHOT_SUCCEEDED: ${{ inputs.head-snapshot-succeeded }}
         MAX_SNAPSHOT_LOOKBACK: ${{ inputs.max-snapshot-lookback }}
         OVERRIDE_LABEL: ${{ inputs.override-label }}
         CONFIG_FILE: ${{ inputs.config-file }}

dependency-vuln-check/check.py

@@ -53,6 +53,12 @@
 _MAX_ATTEMPTS = 3
 _BACKOFF_BASE_SECONDS = 2
 
+# Head-snapshot indexing can lag a successful submission by a few seconds. When
+# the head side of the compare looks empty we re-fetch a few times to let the
+# dependency graph settle before trusting the result (see _looks_like_missing_head).
+_HEAD_INDEX_MAX_ATTEMPTS = 3
+_HEAD_INDEX_BACKOFF_SECONDS = 5
+
 # Severity ranking used by both threshold gates. These are the only values the
 # Dependency Review API emits for `severity`.
 SEVERITY_ORDER = {"low": 0, "moderate": 1, "high": 2, "critical": 3}
@@ -260,6 +266,26 @@ def _dep_triple(dep: dict) -> tuple:
     return (dep.get("name", ""), dep.get("version", ""), dep.get("manifest", ""))
 
 
+def _looks_like_missing_head(diff: list) -> bool:
+    """True when the diff has removed deps but zero added deps.
+
+    After a fresh head-snapshot submission, an all-removed / nothing-added diff
+    usually means the head SBOM has not finished indexing yet: the head tree
+    reads as empty against the base, so every base dep shows as ``removed`` and
+    nothing shows as ``added``. A legitimate removal-only PR has the same shape —
+    so this signal is used ONLY to wait out indexing lag (re-fetch), never to
+    block, which keeps it from misfiring on a real removal.
+    """
+    added = removed = 0
+    for dep in diff:
+        change = dep.get("change_type")
+        if change == "added":
+            added += 1
+        elif change == "removed":
+            removed += 1
+    return added == 0 and removed > 0
+
+
 def _base_branch_pre_existing(
     repository: str, effective_base: str, snapshot_tip: str | None,
     base_tip: str | None, token: str,
@@ -694,6 +720,11 @@ def main() -> None:
     base_sha = os.environ["BASE_SHA"]
     head_sha = os.environ["HEAD_SHA"]
     base_ref = os.environ["BASE_REF"]
+    # Result of the consumer's head-SBOM submission job, when reported. Empty/unset
+    # = a consumer that does not wire it → skip the check (backward compatible with
+    # existing pins). 'true'/'success' = verified. Any OTHER value fails closed
+    # (see the guard below) so a wiring mistake can't silently disable it.
+    head_snapshot_ok = os.environ.get("HEAD_SNAPSHOT_SUCCEEDED", "").strip().lower()
     snapshot_workflow = os.environ["SNAPSHOT_WORKFLOW"]
     lookback = int(os.environ.get("MAX_SNAPSHOT_LOOKBACK", "30"))
     override_label = os.environ.get("OVERRIDE_LABEL", "ci:vuln-gate-override")
@@ -770,19 +801,53 @@ def main() -> None:
             f"(scanned {scanned} run(s); snapshot run {run_id})"
         )
 
-    # ── Dependency diff against the resolved base ──
-    try:
-        diff = _api_get(
-            f"{_GITHUB_API}/repos/{repository}/dependency-graph/compare/{effective_base}...{head_sha}",
-            token,
-        )
-    except ApiError as e:
+    # ── Head snapshot must have been submitted successfully ──
+    # The consumer reports the result of its head-SBOM submission job. If it did
+    # not succeed, the head side of the diff is stale/absent and a genuinely new
+    # vulnerable dependency would silently pass — so fail closed rather than trust
+    # an unverifiable head. Only 'true'/'success' (or unset, for consumers that do
+    # not wire it) proceed; ANY other value — 'false', a raw job result like
+    # 'failure'/'cancelled'/'skipped', or a typo — fails closed, so a wiring
+    # mistake cannot silently turn this guard into a no-op.
+    if head_snapshot_ok not in ("", "true", "success"):
         fail_closed(
             repository, pr_number, token, override_label,
-            f"dependency review API failed ({e.reason})", summary_path,
+            f"head dependency snapshot did not succeed (submission result: '{head_snapshot_ok}') "
+            "— head dependencies cannot be verified",
+            summary_path,
         )
         return
 
+    # ── Dependency diff against the resolved base ──
+    # The head SBOM can still be indexing for a few seconds after a successful
+    # submission. If the head side looks empty (0 added, deps removed) we re-fetch
+    # a few times to let the graph settle before trusting it. A removal-only PR
+    # shows the same shape and simply re-confirms — we never block on this signal,
+    # only wait out indexing lag, so it cannot misfire on a legitimate removal.
+    diff = None
+    for attempt in range(1, _HEAD_INDEX_MAX_ATTEMPTS + 1):
+        try:
+            diff = _api_get(
+                f"{_GITHUB_API}/repos/{repository}/dependency-graph/compare/{effective_base}...{head_sha}",
+                token,
+            )
+        except ApiError as e:
+            fail_closed(
+                repository, pr_number, token, override_label,
+                f"dependency review API failed ({e.reason})", summary_path,
+            )
+            return
+        if attempt == _HEAD_INDEX_MAX_ATTEMPTS or not _looks_like_missing_head(diff):
+            break
+        delay = _HEAD_INDEX_BACKOFF_SECONDS * attempt
+        print(
+            f"::notice::Head side of the dependency diff looks empty "
+            f"(0 added, {sum(1 for d in diff if d.get('change_type') == 'removed')} removed) — "
+            f"the head snapshot may still be indexing; re-fetching in {delay}s "
+            f"(attempt {attempt}/{_HEAD_INDEX_MAX_ATTEMPTS})"
+        )
+        time.sleep(delay)
+
     # ── Pre-existing dep filter: suppress base-branch-added deps (Workstream F) ──
     # Find deps the base branch itself added after our snapshot — those are
     # pre-existing on the branch, not PR-introduced. We diff effective_base against

dependency-vuln-check/test_check.py

@@ -367,6 +367,7 @@ def _set_main_env(monkeypatch):
     monkeypatch.delenv("GITHUB_STEP_SUMMARY", raising=False)
     monkeypatch.delenv("GITHUB_EVENT_PATH", raising=False)
     monkeypatch.delenv("CONFIG_FILE", raising=False)
+    monkeypatch.delenv("HEAD_SNAPSHOT_SUCCEEDED", raising=False)
 
 
 def test_main_real_vuln_blocks_even_with_override_label(monkeypatch):
@@ -391,6 +392,146 @@ def test_main_fail_closed_when_no_ancestor(monkeypatch):
     assert ei.value.code == 1
 
 
+# ── Head-snapshot guard: _looks_like_missing_head helper ──
+
+
+def test_looks_like_missing_head_all_removed():
+    # 0 added, some removed → head reads as empty (indexing-lag signature).
+    assert check._looks_like_missing_head([_dep(change_type="removed")]) is True
+
+
+def test_looks_like_missing_head_has_added():
+    # Any added dep means the head is populated — not the missing-head shape.
+    assert check._looks_like_missing_head(
+        [_dep(change_type="removed"), _dep(change_type="added")]
+    ) is False
+
+
+def test_looks_like_missing_head_empty_diff():
+    # A no-op PR (nothing added or removed) is not the missing-head shape.
+    assert check._looks_like_missing_head([]) is False
+
+
+# ── Head-snapshot guard: gap 1 (submission-failed → fail closed) ──
+
+
+def test_main_fail_closed_when_head_snapshot_failed(monkeypatch):
+    # Consumer reports the head-SBOM submission failed → the head side is
+    # unverifiable → fail closed (exit 1), regardless of the (empty) diff.
+    _set_main_env(monkeypatch)
+    monkeypatch.setenv("HEAD_SNAPSHOT_SUCCEEDED", "false")
+    monkeypatch.setattr(check, "latest_snapshotted_ancestor", lambda *a, **k: ("eff", 1, 1, "latest"))
+    monkeypatch.setattr(check, "has_override_label", lambda *a, **k: False)
+    monkeypatch.setattr(check, "_pr_number", lambda: None)
+    with pytest.raises(SystemExit) as ei:
+        check.main()
+    assert ei.value.code == 1
+
+
+def test_main_head_snapshot_failed_bypassed_by_override(monkeypatch):
+    # Same as above but the override label is present → the unverifiable PR is
+    # allowed through (exit 0), consistent with the other fail-closed paths.
+    _set_main_env(monkeypatch)
+    monkeypatch.setenv("HEAD_SNAPSHOT_SUCCEEDED", "false")
+    monkeypatch.setattr(check, "latest_snapshotted_ancestor", lambda *a, **k: ("eff", 1, 1, "latest"))
+    monkeypatch.setattr(check, "has_override_label", lambda *a, **k: True)
+    monkeypatch.setattr(check, "_pr_number", lambda: None)
+    with pytest.raises(SystemExit) as ei:
+        check.main()
+    assert ei.value.code == 0
+
+
+@pytest.mark.parametrize("value", ["failure", "cancelled", "skipped", "False", "oops"])
+def test_main_fail_closed_on_unexpected_head_snapshot_value(monkeypatch, value):
+    # A wiring mistake (raw job result or typo) must fail closed, not silently
+    # no-op the guard. Only 'true'/'success'/unset proceed.
+    _set_main_env(monkeypatch)
+    monkeypatch.setenv("HEAD_SNAPSHOT_SUCCEEDED", value)
+    monkeypatch.setattr(check, "latest_snapshotted_ancestor", lambda *a, **k: ("eff", 1, 1, "latest"))
+    monkeypatch.setattr(check, "has_override_label", lambda *a, **k: False)
+    monkeypatch.setattr(check, "_pr_number", lambda: None)
+    with pytest.raises(SystemExit) as ei:
+        check.main()
+    assert ei.value.code == 1
+
+
+@pytest.mark.parametrize("value", ["success", "SUCCESS", "true"])
+def test_main_head_snapshot_recognized_ok_values_proceed(monkeypatch, value):
+    # Both the documented boolean ('true') and a raw successful result ('success')
+    # proceed; matching is case-insensitive.
+    _set_main_env(monkeypatch)
+    monkeypatch.setenv("HEAD_SNAPSHOT_SUCCEEDED", value)
+    monkeypatch.setattr(check, "latest_snapshotted_ancestor", lambda *a, **k: ("eff", 1, 1, "latest"))
+    monkeypatch.setattr(check, "_base_branch_pre_existing", lambda *a, **k: set())
+    monkeypatch.setattr(check, "_api_get", lambda url, tok: [_dep(change_type="added", severity="low", fix=None)])
+    monkeypatch.setattr(check, "_pr_number", lambda: None)
+    check.main()  # clean/low → no SystemExit
+
+
+def test_main_head_snapshot_true_proceeds(monkeypatch):
+    # Submission succeeded and the diff is clean → no block (main returns).
+    _set_main_env(monkeypatch)
+    monkeypatch.setenv("HEAD_SNAPSHOT_SUCCEEDED", "true")
+    monkeypatch.setattr(check, "latest_snapshotted_ancestor", lambda *a, **k: ("eff", 1, 1, "latest"))
+    monkeypatch.setattr(check, "_base_branch_pre_existing", lambda *a, **k: set())
+    monkeypatch.setattr(check, "_api_get", lambda url, tok: [_dep(change_type="added", severity="low", fix=None)])
+    monkeypatch.setattr(check, "_pr_number", lambda: None)
+    check.main()  # low/no-fix does not block → no SystemExit
+
+
+# ── Head-snapshot guard: gap 2 (indexing-lag retry, never misfires) ──
+
+
+def test_main_retries_when_head_appears_unindexed(monkeypatch):
+    # First fetch: head looks empty (all removed) → indexing still settling.
+    # Second fetch: settled, a real fixable vuln is added → blocks on the final
+    # result. Confirms we re-fetch and trust the settled diff.
+    _set_main_env(monkeypatch)
+    monkeypatch.setenv("HEAD_SNAPSHOT_SUCCEEDED", "true")
+    monkeypatch.setattr(check, "latest_snapshotted_ancestor", lambda *a, **k: ("eff", 1, 1, "latest"))
+    monkeypatch.setattr(check, "_base_branch_pre_existing", lambda *a, **k: set())
+    monkeypatch.setattr(check, "has_override_label", lambda *a, **k: False)
+    monkeypatch.setattr(check, "_pr_number", lambda: None)
+    sleeps = []
+    monkeypatch.setattr(check.time, "sleep", lambda s: sleeps.append(s))
+    responses = [[_dep(change_type="removed")], [_dep(severity="high", fix="2.0.0")]]
+    calls = {"n": 0}
+
+    def fake_get(url, tok):
+        i = calls["n"]
+        calls["n"] += 1
+        return responses[min(i, len(responses) - 1)]
+
+    monkeypatch.setattr(check, "_api_get", fake_get)
+    with pytest.raises(SystemExit) as ei:
+        check.main()
+    assert ei.value.code == 1     # settled diff has the real vuln
+    assert calls["n"] == 2        # re-fetched once
+    assert sleeps                 # waited for indexing
+
+
+def test_main_persistent_empty_head_never_blocks(monkeypatch):
+    # Head stays all-removed across every retry. A legitimate removal-only PR
+    # looks identical, so the gate must NOT block on this signal — it exhausts
+    # the retries and proceeds (0 blocking). Guards against a misfire FP.
+    _set_main_env(monkeypatch)
+    monkeypatch.setenv("HEAD_SNAPSHOT_SUCCEEDED", "true")
+    monkeypatch.setattr(check, "latest_snapshotted_ancestor", lambda *a, **k: ("eff", 1, 1, "latest"))
+    monkeypatch.setattr(check, "_base_branch_pre_existing", lambda *a, **k: set())
+    monkeypatch.setattr(check, "has_override_label", lambda *a, **k: False)
+    monkeypatch.setattr(check, "_pr_number", lambda: None)
+    monkeypatch.setattr(check.time, "sleep", lambda s: None)
+    calls = {"n": 0}
+
+    def fake_get(url, tok):
+        calls["n"] += 1
+        return [_dep(change_type="removed")]
+
+    monkeypatch.setattr(check, "_api_get", fake_get)
+    check.main()  # must NOT raise SystemExit(1)
+    assert calls["n"] == check._HEAD_INDEX_MAX_ATTEMPTS  # exhausted retries, then proceeded
+
+
 def test_stacked_pr_falls_back_to_default_branch(monkeypatch):
     # base_ref targets a feature branch (no snapshots); fallback_ref (main) resolves.
     _set_main_env(monkeypatch)