From d43a7893a9deebd8e0a2ac10fef7d09589430a1f Mon Sep 17 00:00:00 2001 From: Peter Szabo Date: Mon, 13 Jul 2026 10:43:54 +0200 Subject: [PATCH 1/2] feat: fail closed on unverified head snapshot in dependency-vuln-check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The base side of the dependency diff is resolved to a verified snapshotted ancestor and fails closed when it cannot be established. The head side had no equivalent guard: check.py trusted compare(effective_base...head_sha) and only failed closed if the API call itself errored — never verifying the head SBOM was actually submitted and indexed. If the head snapshot submission failed, or had not finished indexing when the compare ran, the head side of the diff read empty and a genuinely new vulnerable dependency passed silently (false negative). Harmless while the consuming gate was observe-only; under blocking it is a false green. Two guards close it: - Submission failure — new optional `head-snapshot-succeeded` input. The consumer passes the result of its head-SBOM submission job; when 'false' the gate fails closed (honors the override label, like the base-side paths). Unset preserves current behavior for existing consumers. - Indexing lag — when the head side of the diff looks empty (0 added, deps removed) the compare is re-fetched a few times with backoff to let the dependency graph settle. A removal-only PR has the same shape and simply re-confirms; the gate never blocks on this signal, it only waits out lag, so it cannot misfire into a false positive. 68/68 pytest pass (8 new: helper, gap-1 fail-closed + override bypass, gap-2 retry-then-block and persistent-empty-never-blocks). README + action inputs documented. --- dependency-vuln-check/README.md | 20 ++++- dependency-vuln-check/action.yml | 11 +++ dependency-vuln-check/check.py | 76 +++++++++++++++++-- dependency-vuln-check/test_check.py | 114 ++++++++++++++++++++++++++++ 4 files changed, 212 insertions(+), 9 deletions(-) diff --git a/dependency-vuln-check/README.md b/dependency-vuln-check/README.md index 627024f6..aa3fd811 100644 --- a/dependency-vuln-check/README.md +++ b/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 (`'true'`/`'false'`). Pass e.g. `needs.pr-maven-snapshot.result == 'success'`. When `'false'` the gate **fails closed** (an un-submitted head SBOM leaves the head side of the diff empty → a real new vuln would silently pass). 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 diff --git a/dependency-vuln-check/action.yml b/dependency-vuln-check/action.yml index a609a258..21d3bbb5 100644 --- a/dependency-vuln-check/action.yml +++ b/dependency-vuln-check/action.yml @@ -27,6 +27,16 @@ 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 ('true'/'false'). Pass the result of that job (e.g. + needs.pr-maven-snapshot.result == 'success'). When 'false' the gate fails + 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 +93,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 }} diff --git a/dependency-vuln-check/check.py b/dependency-vuln-check/check.py index fc791066..9f457848 100644 --- a/dependency-vuln-check/check.py +++ b/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,10 @@ 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 ('true'/'false'), when + # reported. Empty/unset = a consumer that does not wire it → skip the check + # (backward compatible with existing pins). + 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 +800,49 @@ 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. 'true'/unset proceeds; only an explicit 'false' blocks. + if head_snapshot_ok == "false": fail_closed( repository, pr_number, token, override_label, - f"dependency review API failed ({e.reason})", summary_path, + "head dependency snapshot submission did not succeed — 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 diff --git a/dependency-vuln-check/test_check.py b/dependency-vuln-check/test_check.py index 96b019af..2551a976 100644 --- a/dependency-vuln-check/test_check.py +++ b/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,119 @@ 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 + + +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) From 3ad68b8e48fd6fbb24f9e080277549c30ce4c044 Mon Sep 17 00:00:00 2001 From: Peter Szabo Date: Mon, 13 Jul 2026 11:53:41 +0200 Subject: [PATCH 2/2] fix: fail closed on any non-success head-snapshot value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Copilot review on #749: the guard blocked only on the literal 'false', so a consumer wiring `needs..result` directly (yielding 'failure' / 'cancelled' / 'skipped') or a typo would silently be treated as "proceed" — turning the guard into a no-op and reintroducing the false negative. Now only '', 'true', 'success' proceed; every other value fails closed (the submission result is named in the reason). A wiring mistake fails safe (blocks, visible, override-able) instead of silently disabling the guard. README + action input doc updated; 76/76 pytest (added unexpected-value + recognized-ok parametrized tests). --- dependency-vuln-check/README.md | 2 +- dependency-vuln-check/action.yml | 11 ++++++----- dependency-vuln-check/check.py | 17 +++++++++++------ dependency-vuln-check/test_check.py | 27 +++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 12 deletions(-) diff --git a/dependency-vuln-check/README.md b/dependency-vuln-check/README.md index aa3fd811..d25e9248 100644 --- a/dependency-vuln-check/README.md +++ b/dependency-vuln-check/README.md @@ -55,7 +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 (`'true'`/`'false'`). Pass e.g. `needs.pr-maven-snapshot.result == 'success'`. When `'false'` the gate **fails closed** (an un-submitted head SBOM leaves the head side of the diff empty → a real new vuln would silently pass). Unset skips the check (backward compatible) | +| `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` | diff --git a/dependency-vuln-check/action.yml b/dependency-vuln-check/action.yml index 21d3bbb5..b23394be 100644 --- a/dependency-vuln-check/action.yml +++ b/dependency-vuln-check/action.yml @@ -30,11 +30,12 @@ inputs: head-snapshot-succeeded: description: > Whether the job that submits the PR head's Maven dependency snapshot - succeeded ('true'/'false'). Pass the result of that job (e.g. - needs.pr-maven-snapshot.result == 'success'). When 'false' the gate fails - 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). + 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: diff --git a/dependency-vuln-check/check.py b/dependency-vuln-check/check.py index 9f457848..74f4e0da 100644 --- a/dependency-vuln-check/check.py +++ b/dependency-vuln-check/check.py @@ -720,9 +720,10 @@ 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 ('true'/'false'), when - # reported. Empty/unset = a consumer that does not wire it → skip the check - # (backward compatible with existing pins). + # 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")) @@ -804,11 +805,15 @@ def main() -> None: # 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. 'true'/unset proceeds; only an explicit 'false' blocks. - if head_snapshot_ok == "false": + # 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, - "head dependency snapshot submission did not succeed — head dependencies cannot be verified", + f"head dependency snapshot did not succeed (submission result: '{head_snapshot_ok}') " + "— head dependencies cannot be verified", summary_path, ) return diff --git a/dependency-vuln-check/test_check.py b/dependency-vuln-check/test_check.py index 2551a976..bc7495f4 100644 --- a/dependency-vuln-check/test_check.py +++ b/dependency-vuln-check/test_check.py @@ -441,6 +441,33 @@ def test_main_head_snapshot_failed_bypassed_by_override(monkeypatch): 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)