Disable dynamic DS scan on SBOM parsing and ingestion when a DS security report is present

Olivier Gonzalezrequested to merge
remove_ds_on_sbom_ingestion into master

What does this MR do and why?

Current problematic workflow: During default branch ingestion, DS findings generated by the SBOM scanner are explicitly excluded, then a separate DS scan runs after SBOM ingestion via Sbom::ProcessVulnerabilitiesWorker. This creates duplicate vulnerability creation and potential ingestion mismatches. Also, now that we have brought back DS report generation in the new DS analyzer we no longer need the extra DS scan on SBOM parsing and ingestion.

The Problem

  1. On pipeline completion (or when all security jobs finish) we store Security::Scan objects in DB and parse the content of the security reports to create Security::Finding records. At this moment we also parse the cycloneDX SBOM reports and do the DS scan "on the fly", generating an in-memory DS report and the corresponding findings, making it look as if it was coming from a regular DS security report.
  2. When running on the default branch, the ingestion services are then executed. When we ingest a report we explicitly exclude findings generated by our SBOM scanner: https://gitlab.com/gitlab-org/gitlab/-/blob/a42369d9bc9ce9fa809ddb0129668e9b0043c7ba/ee/app/services/security/ingestion/finding_map_collection.rb#L44
  3. When vulnerability ingestion process completes, the SBOM ingestion process starts
  4. When SBOM ingestion completes, we fire a Sbom::SbomIngestedEvent event and the Sbom::ProcessVulnerabilitiesWorker worker will trigger another DS scan and create the corresponding vulnerabilities in DB.

This implementation certainly comes from the fact that we introduced DS scans of SBOM reports in the default branch first, and then later in the feature branches. Ultimately we did Remove cyclonedx related findings from (!180470 - merged) to avoid ingesting findings generated at the time of report parsing and instead rely on the scan we do later, after SBOM ingestion.

However, now the new DS analyzer emits a DS security report but still uses the GitLab SBOM Vulnerability Scanner (id: gitlab-sbom-vulnerability-scanner). This means the findings coming from the DS report generated by the DS analyzer in a CI job are simply skipped during the default branch ingestion process.

This problem doesn't exist with the old Gemnasium analyzer since it uses a different scanner id in the generated DS report.

The Solution

Always use the generic vulnerability ingestion for all DS findings generated with the GitLab SBOM Vulnerability Scanner, whether they come from a DS security report artifact, or from a dynamically generated DS report when parsing SBOM report artifacts.

  1. Prevent duplicate SBOM artifact processing in StoreScansService:
    • When a job has both a DS report artifact and an SBOM report artifact, skip processing the SBOM one
    • When a job only has an SBOM report artifact, process it
    • Gated behind FF disable_ds_on_sbom_report for gradual rollout
  2. Fix and enabling ingestion of SBOM findings during generic ingestion process in FindingMapCollection:
    • Fix the logic to fetch report_findings on a Security::Scan of type dependency_scanning but actually coming from SBOM reports.
    • Include all findings generated by the GitLab SBOM Vulnerability Scanner in the generic ingestion process
    • The fix is permanent but the inclusion of the SBOM scanner findings is gated behind FF disable_ds_on_sbom_report for gradual rollout
  3. Avoid duplicate vulnerability scan in CreateVulnerabilitiesService:
    • When the SBOM source type is dependency_scanning, skip this additional security scan after SBOM ingestion (keeping it for Container Scanning experimental, for now).
    • Gated behind FF disable_ds_on_sbom_report for gradual rollout
  4. Ensure MarkAsResolvedService is scoped by the scan type (report_type) during generic ingestion process:
    • The report_type argument allows to supoprt security scanners that produce different types of report (e.g. the GitLab SBOM Vulnerability Scanner can generate both DS and CS report types)
    • Keep the logic to skip findings generated with the SBOM scanner, but gate it behind this new FF disable_ds_on_sbom_report instead.

NB: while it is common that a CI job generate multiple SBOM documents, when they are uploaded as CI job report artifact, they actually all end up in a single DB record. Indeed the Ci::JobArtifact model with type cyclonedx actually store these files as an archive. When we process it, these files are decompressed and processed one by one with the each_blob enumerator.

See previous solution (which doesn't work with some post processing tasks like MarkAsResolvedService)
  1. Preventing duplicate SBOM artifact processing in StoreScansService:
    • When a job has both a DS report artifact and SBOM artifacts, skip processing the SBOM artifact
    • Gated behind FF disable_ds_on_sbom_report for gradual rollout
  2. Enabling SBOM findings ingestion in FindingMapCollection:
    • When FF is enabled AND a DS report artifact exists for the job, include SBOM findings in ingestion
    • This allows findings from the DS report generated during SBOM parsing to be properly ingested
  3. Preventing duplicate vulnerability creation in CreateVulnerabilitiesService:
    • When FF is enabled AND a DS report artifact exists for the job, skip the post-ingestion DS scan
    • This eliminates the duplicate and useless scan and vulnerability upsert attempt that was happening after SBOM ingestion
    • Gated behind the same FF for consistent behavior across the pipeline
  4. Tracking job context in SBOM reports:
    • Added job_id parameter to Gitlab::Ci::Reports::Sbom::Report to track which job each SBOM report came from
    • This enables excluding sboms for jobs that already have a DS report

Rollout Strategy

The change is gated behind the feature flag disable_ds_on_sbom_report and support all use cases:

  • single or multiple jobs in whole pipeline hierarchy with DS report only
  • single or multiple jobs in whole pipeline hierarchy with SBOM report(s) only
  • single or multiple jobs in whole pipeline hierarchy with both DS and SBOM report(s)
  • a mix of some jobs with DS job only and some jobs with SBOM report only and/or both report types.

This approach provides:

  • Gradual rollout: Control when the new behavior is enabled per project/group on gitlab.com for validation
  • Backward compatibility: Projects using the new DS analyzer without a successful DS report generation will fallback to the Beta behavior using the dynamic DS scan on SBOM.

Migration Path

  1. Phase 1 (Current): FF-gated behavior. This allows for a gradual rollout on gitlab.com
  2. Phase 2 (Future): Remove FF, keep only the DS artifact check to avoid duplicate processing and permanently avoid the DS scan after SBOM ingestion on default branch.

This change is part of the work to deliver SBOM based Dependency Scanning feature as Generally Available, and thus replacing the Beta behavior. This solution also contributes to solving adjacent bugs like Dependency List missing vulnerabilities that ar... (#571526)

Workflow overview

These diagrams show the 4 major steps of the workflow and how their internal logic is modified by this MR.

Before 


%%{init: {'theme': 'base', 'themeVariables': { 'primaryColor':'#ffffff', 'primaryTextColor':'#000000', 'primaryBorderColor':'#333333', 'lineColor':'#333333', 'secondBkgColor':'#f0f0f0', 'tertiaryColor':'#ffffff', 'tertiaryTextColor':'#000000', 'tertiaryBorderColor':'#333333'}}}%%

graph TD
    subgraph ReportParsing["1️⃣ Report Parsing"]
        direction LR
        A["Fetch all DS and SBOM report artifacts in the pipeline hierarchy"]
        B{CycloneDX ?}
        D["parsed findings from raw DS report"]
        E["store findings in a  Security::Scan (one per CI job)"]
        C1["parse SBOM report"]
        C2["DS scan on SBOM components"]
        A --> B
        B --> |No|D
        B --> |Yes|C1
        C1 --> C2
        C2 --> E
        D --> E
    end

    subgraph VulnIngestion["2️⃣ Vulnerability Ingestion"]
        direction LR
        F["fetch findings from Security::Scan"]
        G{"scanner == GitLab SBOM<br/>Vulnerability Scanner?"}
        H["❌ Skip ingestion"]
        I["create Vulnerabilities<br/>on default branch"]
        F --> G
        G -->|Yes| H
        G -->|No| I
    end

    subgraph SbomIngestion["3️⃣ SBOM Ingestion"]
        direction LR
        J["Parse and store<br/>SBOM components"]
        K["Emit SbomIngestedEvent"]
        J --> K
    end

    subgraph DsScanAfter["4️⃣ SBOM Scan After Ingestion"]
        direction LR
        L["parse SBOM report"]
        M["DS scan on SBOM components"]
        N["create Vulnerabilities<br/>on default branch"]
        L --> M --> N
    end

    ReportParsing --> VulnIngestion
    VulnIngestion --> SbomIngestion
    SbomIngestion --> DsScanAfter
    
    style ReportParsing fill:#e7f5ff,stroke:#333333,color:#000000
    style VulnIngestion fill:#e7f5ff,stroke:#333333,color:#000000
    style SbomIngestion fill:#e7f5ff,stroke:#333333,color:#000000
    style DsScanAfter fill:#e7f5ff,stroke:#333333,color:#000000

After (With FF for rollout) 


%%{init: {'theme': 'base', 'themeVariables': { 'primaryColor':'#ffffff', 'primaryTextColor':'#000000', 'primaryBorderColor':'#333333', 'lineColor':'#333333', 'secondBkgColor':'#f0f0f0', 'tertiaryColor':'#ffffff', 'tertiaryTextColor':'#000000', 'tertiaryBorderColor':'#333333'}}}%%

graph TD
    subgraph ReportParsing["1️⃣ Report Parsing"]
        direction LR
        A["Fetch all DS and SBOM report artifacts in the pipeline hierarchy"]
        B{CycloneDX ?}
        D["parsed findings from raw DS report"]
        E["store findings in a  Security::Scan (one per CI job)"]
        C1["parse SBOM report(s)"]
        C2["DS scan on SBOM components"]
        FF1{FF enabled?<br/>AND DS report<br/>exists for thic CI job?}
        SKIP1["❌ Skip SBOM report(s)"]
        A --> B
        B --> |No|D
        B --> |Yes|FF1
        FF1 --> |Yes|SKIP1
        FF1 --> |No|C1
        C1 --> C2
        C2 --> E
        D --> E
    end

    subgraph VulnIngestion["2️⃣ Vulnerability Ingestion"]
        direction LR
        F["fetch findings from Security::Scan"]
        G{"scanner == GitLab SBOM<br/>Vulnerability Scanner?"}
        FF2{FF enabled?}
        H["❌ Skip ingestion"]
        I["create Vulnerabilities<br/>on default branch"]
        F --> G
        G -->|Yes| FF2
        FF2 --> |Yes|I
        FF2 --> |No|H
        G -->|No| I
    end

    subgraph SbomIngestion["3️⃣ SBOM Ingestion"]
        direction LR
        J["Parse and store<br/>SBOM components"]
        K["Emit SbomIngestedEvent"]
        J --> K
    end

    subgraph DsScanAfter["4️⃣ SBOM Scan After Ingestion"]
        direction LR
        L["parse SBOM report"]
        M["DS scan on SBOM components"]
        N["create Vulnerabilities<br/>on default branch"]
        FF3{FF enabled?</BR> AND SBOM type == dependency_sanning}
        SKIP2["❌ Skip SBOM scan"]
        L --> FF3
        FF3 --> |Yes|SKIP2
        FF3 --> |No|M
        M --> N
    end

    ReportParsing --> VulnIngestion
    VulnIngestion --> SbomIngestion
    SbomIngestion --> DsScanAfter
    
    style SKIP1 fill:#ffd43b,stroke:#333333,color:#000000
    style SKIP2 fill:#ffd43b,stroke:#333333,color:#000000
    style H fill:#ffd43b,stroke:#333333,color:#000000
    style I fill:#51cf66,stroke:#333333,color:#000000
    style N fill:#51cf66,stroke:#333333,color:#000000
    style ReportParsing fill:#e7f5ff,stroke:#333333,color:#000000
    style VulnIngestion fill:#e7f5ff,stroke:#333333,color:#000000
    style SbomIngestion fill:#f0f9ff,stroke:#333333,color:#000000
    style DsScanAfter fill:#f0f9ff,stroke:#333333,color:#000000

Final (when FF is removed) 


%%{init: {'theme': 'base', 'themeVariables': { 'primaryColor':'#ffffff', 'primaryTextColor':'#000000', 'primaryBorderColor':'#333333', 'lineColor':'#333333', 'secondBkgColor':'#f0f0f0', 'tertiaryColor':'#ffffff', 'tertiaryTextColor':'#000000', 'tertiaryBorderColor':'#333333'}}}%%

graph TD
    subgraph ReportParsing["1️⃣ Report Parsing"]
        direction LR
        A["Fetch all DS and SBOM report artifacts in the pipeline hierarchy"]
        B{CycloneDX ?}
        D["parsed findings from raw DS report"]
        E["store findings in a  Security::Scan (one per CI job)"]
        C1["parse SBOM report(s)"]
        C2["DS scan on SBOM components"]
        FF1{DS report<br/>exists for thic CI job?}
        SKIP1["❌ Skip SBOM report(s)"]
        A --> B
        B --> |No|D
        B --> |Yes|FF1
        FF1 --> |Yes|SKIP1
        FF1 --> |No|C1
        C1 --> C2
        C2 --> E
        D --> E
    end

    subgraph VulnIngestion["2️⃣ Vulnerability Ingestion"]
        direction LR
        F["fetch findings from Security::Scan"]
        G["create Vulnerabilities<br/>on default branch (including all DS findings)"]
        F --> G
    end

    subgraph SbomIngestion["3️⃣ SBOM Ingestion"]
        direction LR
        J["Parse and store<br/>SBOM components"]
        K["Emit SbomIngestedEvent"]
        J --> K
    end

    subgraph DsScanAfter["4️⃣ SBOM Scan After Ingestion"]
        direction LR
        L["parse SBOM report"]
        M["DS scan on SBOM components"]
        N["create Vulnerabilities<br/>on default branch"]
        FF3{SBOM type == dependency_sanning}
        SKIP2["❌ Skip SBOM scan"]
        L --> FF3
        FF3 --> |Yes|SKIP2
        FF3 --> |No|M
        M --> N
    end

    ReportParsing --> VulnIngestion
    VulnIngestion --> SbomIngestion
    SbomIngestion --> DsScanAfter
    
    style SKIP1 fill:#ffd43b,stroke:#333333,color:#000000
    style SKIP2 fill:#ffd43b,stroke:#333333,color:#000000
    style G fill:#51cf66,stroke:#333333,color:#000000
    style N fill:#51cf66,stroke:#333333,color:#000000
    style ReportParsing fill:#e7f5ff,stroke:#333333,color:#000000
    style VulnIngestion fill:#e7f5ff,stroke:#333333,color:#000000
    style SbomIngestion fill:#f0f9ff,stroke:#333333,color:#000000
    style DsScanAfter fill:#f0f9ff,stroke:#333333,color:#000000

References

Screenshots or screen recordings

How to set up and validate locally

  1. Configure Dependency Scanning feature for a compatible project (example: https://gitlab.com/gitlab-org/secure/tests/olivier/monorepo-multi-language
  2. Run a pipeline. Alternatively, to speed up and skip setting up the real feature, you can simply run a custom job that exposes the SBOM and Dependency Scanning report artifacts by putting them directly in the repository directly. artifacts_example.zip)
  3. Verify the ingestion results on the vulnerablity report and the dependency list
  4. enable the FF disable_ds_on_sbom_report with Feature.enable :disable_ds_on_sbom_report in the rails console
  5. Create a new project with same configuration and run a pipeline
  6. Verify the ingestion results on the vulnerablity report and the dependency list

When the FF is enabled, the dependency list should have a fully functional sort by vulnerabilities severity, whereas with the FF enabled it presents the problems from #571526.

Otherwise, the difference is mostly internal.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Olivier Gonzalez

Merge request reports