Skip to content

chore: add fingerprints to UFM SARIF output#581

Open
danskmt wants to merge 1 commit intomainfrom
chore/cli-1328_fingerprintUFMSarif-v2
Open

chore: add fingerprints to UFM SARIF output#581
danskmt wants to merge 1 commit intomainfrom
chore/cli-1328_fingerprintUFMSarif-v2

Conversation

@danskmt
Copy link
Copy Markdown

@danskmt danskmt commented Apr 7, 2026

Pull Request Submission Checklist

  • Follows CONTRIBUTING guidelines
  • Commit messages are release-note ready, emphasizing what was changed, not how.
  • Includes detailed description of changes
  • Contains risk assessment (Low | Medium | High)
  • Highlights breaking API changes (if applicable)
  • Links to automated tests covering new functionality
  • Includes manual testing instructions (if necessary)
  • Updates relevant GitBook documentation (PR link: ___)
  • Includes product update to be announced in the next stable release notes

What does this PR do?

Adds fingerprints to the UFM SARIF output, per SARIF v2.1.0 section 3.27.16, to provide stable identifiers for results. Each result now includes two fingerprint entries: identity and snyk/asset/finding/v1, both using the issue ID.

Fingerprints are conditionally excluded for OSS issues (sca and license finding types) because their IDs are vulnerability/license identifiers rather than stable code-centric fingerprints suitable for result tracking.

Where should the reviewer start?

  • internal/presenters/templates/ufm.sarif.tmpl – conditional fingerprint inclusion logic
  • internal/presenters/testdata/ufm/secrets.sarif.json – updated expected SARIF with fingerprints for non-OSS results

How should this be manually tested?

  1. Run go test ./internal/presenters/ -run Test_UfmPresenter_Sarif -v
  2. All 6 sub-tests should pass
  3. Verify that SCA test cases (cli, webgoat, multiproject) produce SARIF without fingerprints
  4. Verify that secrets test case produces SARIF with fingerprints

What's the product update that needs to be communicated to CLI users?

None. This is an internal SARIF output enhancement for result management systems.

Risk assessment (Low | Medium | High)?

Low

What are the relevant tickets?

CLI-1328

@danskmt danskmt requested review from a team as code owners April 7, 2026 13:28
@snyk-io
Copy link
Copy Markdown

snyk-io bot commented Apr 7, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@snyk-io
Copy link
Copy Markdown

snyk-io bot commented Apr 7, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@snyk-pr-review-bot

This comment has been minimized.

@danskmt danskmt force-pushed the chore/cli-1328_fingerprintUFMSarif-v2 branch from 97a6eee to f1804b3 Compare April 7, 2026 13:50
@snyk-pr-review-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Brittle Filtering Logic 🟡 [minor]

The conditional logic to exclude fingerprints for SCA and License findings relies on a hardcoded string comparison: ne (printf "%s" $issue.GetFindingType) "sca". However, pkg/utils/sarif/sarif.go (in ConvertTypeToDriverName) uses identifiers like 'sast', 'iac', and 'container', but does not reference 'sca', defaulting to 'Snyk Open Source' instead. If the actual string identifier for SCA findings is 'oss' or 'open-source', this filter will fail to exclude them, resulting in fingerprints being added to findings where they were intended to be omitted.

{{- if and (ne (printf "%s" $issue.GetFindingType) "sca") (ne (printf "%s" $issue.GetFindingType) "license") }},
📚 Repository Context Analyzed

This review considered 4 relevant code sections from 3 files (average relevance: 0.74)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants