Automatically resolve vulnerabilities when a SAST rule is disabled or removed
- Summary
- Goals
- Challenges
-
Opportunity
- Framework to define and enforce limits
- Usage guidelines for flags vs parsing behaviors
- Example 1: Post-analyzer processing identifies a false-positive
- Example 2: Post-analyzer processing eliminates a false-positive
- Example 3A: Analyzer drops noisy rule (prone to FPs)
- Example 3B: Using two analyzers, one drops noisy rule (prone to FPs)
- Principles
- Background
- Status
Summary
Sec section analyzers should include metadata on the outcomes of scans in order to inform report ingestion.
This data can be used to direct the processing of reports for more efficient vulnerability management.
Goals
Implement a framework where analyzer reports can influence their parsing.
Challenges
- Clear delineation between analyzer responsibilities, vulnerability management behavior, automation, and user expectations
- Analyzers should remain simple and stateless without understanding the full scope of past and future behavior (which rules were previously enabled)
- Increased artifact size due to added metadata
Opportunity
We want to continue to iterate on our analyzer prepacked rulesets by adding, removing, and modifying rules without causing significant data issues within user projects.
At present when a finding is no longer detected, we lack granularity in clearly differentiating between the potential outcomes and communicating whether the resolution is due to rule refinement, analyzer behavior, or external factors (such as the relevant code being addressed within the repository).
To better distinguish between these cases we must:
- Build a framework to more clearly inform the relationship between analyzers and their parsers
- Build usage guidelines around how findings should be processed in relation to their rule states
The scenarios to be handled include:
- Post-analyzer identifies a false-positive
- Analyzer drops noisy rule (i.e. one prone to FPs)
- SAST Custom Rulesets disables prepackaged rule
The actions taken by these scenarios cover:
- Identifying false-positives
- Resolving false-positives
- Identifying high-noise rules
- Resolving high-noise rules
Framework to define and enforce limits
Our analyzer reports are currently stateless with no knowledge of past executions.
For comparisons between different repository states (such as source and target branches), the current ingestion flow must calculate set differences between added
, removed
, and unchanged
vulnerabilities.
By introducing two levels of report enrichment we can include metadata around both vulnerabilities and scan executions to better inform our parsing logic. At the finding level, this is already supported via vulnerability.flags\[\]
. At the scan execution level, there is no existing mechanism.
The existing vulnerability.flags\[\]
framework is flexible in providing identifier data on top of existing findings to inform processing. This is currently limited to attaching alerts to identify findings as false-positives, but could be expanded to take actions, as well.
To cover usecases in which analyzers require a more holistic view of rule coverage, a new scan.primary_identifiers\[\]
field must be introduced to allow analyzers to include the full list of scanned rules.
Relationship to Scan Result Policies
The existing Scan Result Policies feature is a similar automation framework, however the focus is currently around user-driven policy actions. The goal of the analyzer automation flow is to provide an analyzer-driven approach in which outcomes are driven by result refinements rather than user business needs.
vulnerability.flags\[\]
Limitations of There are performance implications in relying solely on flags
. Since flags must attach to specific findings we must include the full result-set regardless of whether we wish to discard a portion. This bloats reports and has performance implications by relying on post-filtering of results.
In the event of a noisy rule that produces many insignificant findings, it is inefficient to include those findings in the report simply to be excluded later. Further, in some cases the scanner may fail or experience slowdown by including such rules, such as this issue.
scan.primary_identifiers
Inclusion of exhaustive rule identifiers in By including a comprehensive list of scanned rules in scan.primary_identifiers
we can provide the parser with a means of identifying past and present rulesets generating vulnerability results. This would allow the parser to identify existing findings associated with rules that are no longer being included in analyzer runs, such as the removal of a rule category.
By segmenting which findings are no longer applicable instead of no longer detected we can more accurately communicate which findings can be auto-resolved and which should require manual review.
flags
vs parsing behaviors
Usage guidelines for TODO
Example 1: Post-analyzer processing identifies a false-positive
This work has already been implemented in %14.2 as part of VET FP elimination.
-
brakeman-sast
runs and returns a false-positive finding in vulnerability report -
vet
post-analyzer runs in same pipeline and attaches flag to report identifying specific finding as false-positive - Vulnerability page displays alert marking finding as false-positive
graph LR
repository --> brakeman_sast
subgraph post-analyzer pipeline
brakeman_sast --> report1("gl-sast-report.json")
report1 --> vet::verify
vet::verify --> reportout1("gl-sast-report.json")
end
subgraph ingestion
reportout1 --> ingestsvc["Security::Ingestion::IngestReportsService"]
ingestsvc-- persist flags --> flagtask["Security::Ingestion::Tasks::IngestVulnerabilityFlags"]
end
Example 2: Post-analyzer processing eliminates a false-positive
This scenario is closely aligned with the existing false-positive identification flow, but involves an automated action
-
brakeman-sast
runs and returns a false-positive finding in vulnerability report -
vet
post-analyzer runs in same pipeline and attaches flag to report identifying specific finding as false-positive - Vulnerability page displays alert marking finding as false-positive
- Vulnerability page changes vulnerability status to "resolved" with note that an automated action was performed
graph LR
repository --> brakeman_sast
subgraph post-analyzer pipeline
brakeman_sast --> report1("gl-sast-report.json")
report1 --> vet::verify
vet::verify --> reportout1("gl-sast-report.json")
end
subgraph ingestion
reportout1 --> ingestsvc["Security::Ingestion::IngestReportsService"]
ingestsvc --> flagtask1["Tasks::IngestVulnerabilityFlags"] --> flagtask2["Security::Ingestion::Tasks::UpdateVulnerabilityStatus"]
end
Example 3A: Analyzer drops noisy rule (prone to FPs)
-
semgrep-sast
is updated to remove rule that has been identified as overly noisy and prone to producing high false-positive results - Report includes list of rules executed during scan
- Report parser identifies all existing findings that are not included in list of rules and marks as auto-resolved due to rule no longer being included
graph LR
repository --> brakeman_sast
subgraph analyzer pipeline
brakeman_sast --> report1a["noisy-rule-123 dropped"]
report1a --> report1b["scan.primary_identifiers populated"]
report1b --> reportout1("gl-sast-report.json")
end
subgraph ingestion
reportout1 --> ingestsvc["IngestReportService"]
ingestsvc--> flagtask1["Tasks::MarkDroppedAsResolved"]
end
Alternative approach using flags
graph LR
repository --> brakeman_sast
subgraph analyzer pipeline
brakeman_sast --> report1a["noisy-rule-123 dropped"]
report1a --> report1b["scan.primary_identifiers populated"]
report1b --> reportout1("gl-sast-report.json")
end
subgraph ingestion
reportout1 --> ingestsvc["IngestReportsService"]
ingestsvc-- generate flags based on identifiers diff --> flagtask1
flagtask1["Tasks::IngestVulnerabilityFlags"] --> flagtask2
flagtask2["Tasks::UpdateVulnerabilityStatus"]
end
Example 3B: Using two analyzers, one drops noisy rule (prone to FPs)
-
spotbugs-sast
is used returning kotlin results (kotlin-rule-1
) -
semgrep-sast
is updated to removesemgrep-rule-1
rule that has been identified as overly noisy and prone to producing high false-positive results - Report includes a partial list of rules executed during scan (those belonging to semgrep)
- Report parser merges result-sets, identifies all existing semgrep findings that are not included in list of rules and marks as auto-resolved due to rule no longer being included
-
spotbugs
results are unaffected
graph LR
repository --> semgrep_sast
subgraph analyzer pipeline
semgrep_sast --- report1a1["DROP semgrep-rule-1"]
semgrep_sast --- report1a2["SCAN semgrep-rule-2"]
report1a2 -- "primary_identifiers: [semgrep-rule-2]" --> reportout1("gl-sast-report.json")
spotbugs_sast --- report2a["SCAN kotlin-rule-1"]
report2a -- "primary_identifiers []" --> reportout2("gl-sast-report.json")
end
subgraph ingestion
reportout1 --> MergeReports["Merged identifiers: [semgrep-rule-2]"]
reportout2 --> MergeReports
MergeReports --> ingestsvc["..."]
ingestsvc--> MarkDroppedAsResolved
MarkDroppedAsResolved -- "resolves only vulns where ident match semgrep" --> MarkDroppedAsResolved
end
Principles
- Avoid duplicating behavior in Scan Result policies
- Avoid duplicating behavior in auto-resolution mechanisms within vulnerability management
- Avoid duplicating behavior in user-driven vulnerability management
- Build framework in a way that defines actions analyzers can take to handle existing data
Background
As proposed by @twoodham this pattern could be very useful for handling rule changes going forward.
Excerpts from Slack thread set to expire 2022-10-08
@twoodham Wild idea: think about including identifiers of rules used by the analyzer as part of its operation. That could then inform report handling - and maybe auto-resolve of vulnerabilities with identifiers which aren't part of the reported set.
that's a great way of handle future pain. I swear we had that in a collective issue from before but I could only find the "enriching" one above. It came up during FP-elimination work. We have the scaffolding already with vulnerability_flags https://gitlab.com/gitlab-org/security-products/security-report-schemas/-/blob/8caade7ba7575579b99d9c4be3401ad14757a2e6/src/security-report-format.json#L506-512, so it should be a fairly small lift to implement a "Do X based on Flag Y" parsing step
I'm thinking it could play into report parsing. Existing findings, if no longer detected, could be auto-resolved if their identifier wasn't part of the new report’s metadata.
Status
- 2022-07-27: Started working on the design document (current issue) following Architecture Evolution Workflow.
- 2022-08-15: Opened PoC demonstrating capability !95422 (merged)
- 2022-08-17: Added feature flag gating capability
- 2022-08-25: Opened MR to include
security-report-schema
changes supporting capability - 2022-09-09:
security-report-schema
MR merged: gitlab-org/security-products/security-report-schemas!126 (merged) - 2022-09-22: Feature Flag issue opened #375128 (closed)
- 2022-10-20: Opened MR to include comment in state transition !101704 (merged)
- 2022-11-09: comment in state transition MR merged
- 2022-12-09: MR to tie high-FP rule disablement to auto-resolve tied to %15.7 server version gitlab-org/security-products/analyzers/semgrep!187 (merged)
- 2022-12-12: feature flag enabled on
#production
#375128 (comment 1206297741) - 2022-12-12: feature flag disabled on
#production
after statement timeouts surfaced. Currently investigating #375128 (comment 1206357602) - 2022-12-27: typebug MR to limit auto-resolve behavior to scanners which provide
primary_identifiers
!107763 (merged) - 2023-01-09: feature flag split between scheduler and background worker to allow a "dry-run" rollout of to assess identifier auto-resolution prior to general action !108486 (merged)
- 2023-01-17: feature flag for scheduler enabled globally
- 2023-01-19: Scheduler FF working as expected with no added overhead to
StoreSecurityReportsWorker
. - 2023-01-19: Identifier analysis ongoing to measure auto-resolution blast radius.
- 2023-02-22: Rollout of auto-resolution to internal projects, targeting Gitlab.com customers in following week
- 2023-03-09: Rollout of auto-resolution to all of GitLab.com
- 2023-03-16: feature flag enabled by default and merged to ship with %15.10