Draft: [PoC] Enforce approval policies on analyzer-reported scan failures via scan.status

Alan (Maciej) Paruszewskirequested to merge
604648-poc-scan-status-fail-closed into master

What does this MR do and why?

This is a proof of concept, not a merge-ready change. It demonstrates a topology-independent way to make approval policies react when a security analyzer reports a failed or incomplete scan, closing a bypass discovered while reviewing !243540 (merged).

Problem

Security report schemas carry a top-level scan.status ("success" / "failure") that an analyzer sets independently of its CI job exit status. Today this field is parsed into report.scan.status but only used for an SLI metric — nothing acts on it. A report declaring scan.status: "failure" is still stored as a succeeded Security::Scan as long as the job exited 0 and the schema validated.

This lets approval policies be bypassed with Dependency Scanning v2:

  • The dependency-scanning analyzer job declares the dependency_scanning report and succeeds, uploading a degraded report.
  • A resolution helper job (for example dependency-scanning:maven-resolution, default allow_failure: true) fails and declares no report.
  • enforce_scan_completion! sees a succeeded scan, and detection based on the report-declaring CI job (the approach in !243540 (merged)) sees a success — so neither blocks.

DS v1 is a single job that both resolves and declares the report, so a failure there is caught. Hence "works in v1, not v2".

Approach

Treat scan.status as the signal, since it comes from the component that actually knows the scan failed, and is topology-independent:

  • New Security::Scan status scan_failed (analyzer self-reported failure; distinct from job_failed, since the job did succeed).
  • StoreScanService#initial_scan_status maps a report whose scan.status is present and != "success" to scan_failed, and short-circuits finding ingestion (otherwise store_findings would flip the scan back to succeeded).
  • SCAN_TO_ANALYZER_STATUS maps scan_failed → :failed.

No new enforcement code is needed: enforce_scan_completion! already blocks any required scan type that is present but not succeeded, emitting the existing SCAN_NOT_SUCCEEDED violation. This covers both scanner versions and both user-authored include: template and policy-injected pipelines.

Template side (illustrative only)

The change to Dependency-Scanning.v2.gitlab-ci.yml (resolution job records its own failure; analyzer after_script flips scan.status to "failure") is a PoC of the analyzer contract. In production the analyzer binary owns scan.status — see the open question below. It is included here only to demonstrate the end-to-end flow.

Open decisions before this could ship

  1. Feature-flag gating. enforce_scan_completion! is not behind approval_policies_block_on_failed_scan_job, so the new blocking would be live regardless of the flag. Decide whether to gate the scan.status mapping or treat it as a general ingestion-correctness fix.
  2. Analyzer contract. Does Dependency Scanning v2 already set scan.status: "failure" when a resolution job fails? If yes, only the ingestion change is needed and the template PoC is unnecessary; if no, that is the ask for the Dependency Scanning analyzer team. This is why the template change stays illustrative.

How to set up and validate locally

Validated two ways.

Automated specs

  • ee/spec/services/security/store_scan_service_spec.rb — a report with scan.status: "failure" produces a scan_failed scan and skips finding ingestion; "success" still yields succeeded.
  • ee/spec/services/security/scan_result_policies/update_approvals_service_spec.rb — a scan_failed scan blocks a fail-closed rule with a SCAN_NOT_SUCCEEDED violation.

End-to-end in a local GDK

A runner-executed pipeline published a dependency_scanning report with scan.status: "failure". The real ingestion path produced:

scan_type=dependency_scanning status=scan_failed latest=true errors=[]

confirming the mapping works in a running instance, not just in specs.

References

Merge request reports