chore: introduce Sarif to UFM

[PeterSchafer]

Description

This PR introduces a new transformation from Sarif to UFM to eventually replace Sarif to Local Findings.

Checklist

• Tests added and all succeed (make test)
• Regenerated mocks, etc. (make generate)
• Linted (make lint)
• Test your changes work for the CLI
  1. Clone / pull the latest CLI main.
  2. Run go get github.com/snyk/go-application-framework@YOUR_LATEST_GAF_COMMIT in the cliv2 directory.
     • Tip: for local testing, you can uncomment the line near the bottom of the CLI's go.mod to point to your local GAF code.
  3. Run go mod tidy in the cliv2 directory.
  4. Run the CLI tests and do any required manual testing.
  5. Open a PR in the CLI repo now with the go.mod and go.sum changes.
  • Once this PR is merged, repeat these steps, but pointing to the latest GAF commit on main and update your CLI PR.

---

Note

Medium Risk
Large change to the code findings presentation and transformation path with parity tests against Local Findings; incorrect mapping could change SARIF output, severities, or ignores for CLI consumers.

Overview
Adds a SARIF → UFM path intended to replace the existing SARIF → Local Findings pipeline for code results, plus presenter and workflow wiring so UFM can round-trip SAST SARIF with richer fields.

Transform layer: TransformToUFMFromSarif maps SARIF results into testapi.TestResult (SAST findings, suppressions, code flows, policy mods, priority score, fingerprints in finding-extras metadata). NewSarifTestResult implements the in-memory result type; TransformSarifToUFM exposes the same transform from local workflows. Scan metadata (report URL, project/snapshot IDs) is applied via TranslateMetadataToTestResult.

SARIF output: ufm.sarif.tmpl now renders Snyk Code rule metadata, execution codeFlows, finding extras (messages, rule index, policy, priority score), coverage and upload metadata from test result metadata, and uses effective severity plus expanded suppression details when extras exist.

Presenters: New template helpers read execution flows, policy mods, finding extras, coverage, and snyk_code_rule problems from issues (with local structs to avoid an import cycle with ufm). toInt accepts uint16/uint32 for priority scores.

Tests: End-to-end cases assert normalized SARIF from SARIF → UFM → SARIF matches SARIF → LF → SARIF, plus JSON round-trip rendering and human/HTML output from SARIF fixtures.

Other: issues builder handles snyk_code_rule problems; FilterSeverityASC is exported for shared severity filtering.

^{Reviewed by Cursor Bugbot for commit 9481aff. Bugbot is set up for automated code reviews on this repo. Configure here.}

[snyk-io]

✅ 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
✅	Secrets	0	0	0	0	0 issues

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

[snyk-io]

✅ 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.

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 9293f7d. Configure here.}

[cursor]

Stale comment

Automated verification

Ran a 4-pass review (semantic analysis, adversarial review, security scan, code review) against this diff (base 0044a65...head 4850156).

Not auto-approved. Per this automation's policy, only PRs that are exclusively a dependency bump (e.g. go.mod/go.sum only) are auto-approved. This PR adds ~2,400 lines of new/changed logic across 10 files (new SARIF→UFM transform, presenter template/helpers, testapi issue builder changes) with no go.mod/go.sum changes, so it does not qualify — a human reviewer should approve this one.

Oracle: local build/lint/test could not be run in this sandbox (repo requires a Go 1.26 toolchain that could not be downloaded here). CI's Unit tests and security-scans checks are green for this exact head commit, which covers that gap.

Security scan: manual review only — Snyk CLI/MCP auth was unavailable in this sandbox (no SNYK_TOKEN). No exploitable issues found manually (bounds/nil checks on SARIF-derived data are consistently present, no injection/secrets surfaces introduced, go.mod/go.sum untouched so no new dependency surface).

Should Fix (4)

1. pkg/utils/ufm/transform_sarif.go:103 — severity filter silently drops SARIF results with an "unmapped" level even when no severityThreshold is passed (see inline comment).
2. pkg/utils/ufm/transform_sarif.go:378 — PolicyModification.Prior is derived from OriginalLevel instead of OriginalSeverity, so it can never report "critical" (see inline comment).
3. internal/presenters/templates/ufm.sarif.tmpl:437 — the run-level "properties" block is entirely omitted when there's no coverage/report-url, unlike the parallel Local-Findings template which always renders it (see inline comment).
4. internal/presenters/presenter_ufm_test.go:206 — the regression test strips priorityScore from both sides of the comparison instead of asserting the new value, so the toInt behavior change it introduces ships without golden-fixture verification (see inline comment).

Suggestions (3)

5. pkg/apiclients/testapi/issues.go:777 — processSecretsRuleProblem/processSnykCodeRuleProblem silently flip problemID assignment from last-wins to first-wins, inconsistent with the untouched vuln/license processors (see inline comment).
6. pkg/utils/ufm/transform_sarif.go:19 — new public TransformOption/WithSeverityThreshold on pkg/utils/ufm (load-bearing API) has no caller in this PR (see inline comment).
7. internal/presenters/funcs.go:574 — getExecutionFlowsFromIssue is also registered in getCliTemplateFuncMap, where nothing calls it (see inline comment).

Good coverage overall: the new SARIF→UFM mapping has thorough, edge-case-focused unit tests (severity thresholds, suppression round-tripping, code flows, policy modifications, ID-collision handling), and finding-ID generation correctly falls back to a deterministic UUID with a dedicated collision test.

  

_{Sent by Cursor Automation: Automatic PR verification}

[cursor]

Should Fix: This {{- if or $coverage $reportURL }} guard omits the entire run-level "properties" key whenever there's no coverage data and no report URL. local_finding.sarif.tmpl's equivalent block has no such guard — it unconditionally renders "properties": {"coverage": [...]} (empty array when there's no coverage). Since Test_UfmPresenter_SarifFromSarifInput exists specifically to validate UFM/LocalFindings SARIF-output parity, any input with zero coverage entries and no report metadata will produce structurally different SARIF between the two pipelines.

Suggest: drop this guard and instead render "coverage": [] unconditionally when $coverage is empty, keeping uploadResult conditional only on $reportURL — mirroring the LF template.

[cursor]

Suggestion: This changes processSecretsRuleProblem's problemID assignment from last-wins (id != "") to first-wins (id != "" && b.problemID == ""), and the new processSnykCodeRuleProblem below uses the same first-wins guard — but the untouched processSnykVulnProblem/processSnykLicenseProblem still overwrite unconditionally (last-wins) for findings with multiple matching problems. This inconsistency isn't explained by the PR description and doesn't look required for SAST/code-rule support.

Suggest: either revert this guard to match the vuln/license last-wins convention, or add a comment explaining why secrets/code-rule problems intentionally differ.

[cursor]

Suggestion: TransformOption/WithSeverityThreshold add a public option to TransformToUFMFromSarif, but the only caller in this PR (TransformSarifToUFM in pkg/local_workflows/data_transformation_workflow.go) never passes it — no code in this diff calls WithSeverityThreshold. Per this repo's API stability rules, new pkg/ surface is load-bearing once shipped. The equivalent LocalFindings path has no such option; severity filtering there is a separate downstream workflow step.

Suggest: drop this option until a concrete caller needs it (YAGNI), or wire it through TransformSarifToUFM now if one is already planned.

[cursor]

Suggestion: getExecutionFlowsFromIssue is registered here in getCliTemplateFuncMap (used by CLI/human-readable templates), but it's only ever called from ufm.sarif.tmpl:140, which uses getSarifTemplateFuncMap (where it's already registered at line 542). No template using getCliTemplateFuncMap calls it. Looks like a copy-paste leftover.

Suggest: remove this registration.

[snyk-pr-review-bot]

PR Reviewer Guide 🔍

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Fragile Type Assertions 🟠 [major] The function findingExtraFromMap performs hardcoded type assertions on map values, such as m["ruleIndex"].(float64) and m["isAutofixable"].(bool). If the metadata map is populated via a mechanism other than the default encoding/json decoder (which converts all numbers to float64), or if the underlying data structure changes, these assertions will cause a runtime panic. A safer approach would be to use a decoder that handles type conversion or to use the comma-ok idiom for every field. if v, ok := m["ruleIndex"].(float64); ok { fe.RuleIndex = int(v) } Risk of ID Collisions 🟡 [minor] In generateFindingID, if Fingerprints.Identity, Num0, and Num1 are all empty, the fallback mechanism uses only the URI and the StartLine. In large projects with multiple findings on the same line (e.g., multiple secrets or distinct vulnerabilities in a single complex statement), this will result in identical UUIDs for distinct findings, leading to deduplication errors in downstream consumers like the CLI or Web UI. seed := res.RuleID + res.Fingerprints.Num0 + res.Fingerprints.Num1 if res.Fingerprints.Num0 == "" && res.Fingerprints.Num1 == "" && len(res.Locations) > 0 { loc := res.Locations[0].PhysicalLocation seed += loc.ArtifactLocation.URI + fmt.Sprintf(":%d", loc.Region.StartLine) } Order-Dependent Issue IDs 🟡 [minor] The logic in processSecretsRuleProblem and processSnykCodeRuleProblem now uses a 'first-wins' strategy for setting b.problemID (if ... && b.problemID == ""). If a finding contains multiple problems (e.g., a Snyk Code rule and a secret), the resulting Issue ID will depend entirely on the order of problems in the SARIF Problems slice, which is not guaranteed to be stable across different scan engines. if id := problem.GetID(); id != "" && b.problemID == "" { b.problemID = id

📚 Repository Context Analyzed This review considered 51 relevant code sections from 14 files (average relevance: 0.98) pkg/utils/sarif/sarif.go (lines 1-248) pkg/local_workflows/local_models/transform.go (lines 1-250) pkg/utils/ufm/ufm_workflow_helper.go (lines 1-74) CONTRIBUTING.md (lines 170-300) internal/presenters/presenter_ufm.go (lines 1-234) pkg/utils/ufm/json_test_result.go (2 segments, lines 1-207) pkg/local_workflows/local_models/types_rules_builder.go (lines 1-229) ... and 7 more file(s)

🤖 Repository instructions applied (from AGENTS.md)

[cursor]

Automated verification

Ran a 4-pass review (semantic analysis, adversarial review, security scan, code review) against the diff base 0044a65...head 9481aff.

Not auto-approved. Per this automation's policy, only PRs that are exclusively a dependency bump (e.g. go.mod/go.sum only) are auto-approved. This PR touches 10 non-dependency files (~2,400 lines) implementing a new SARIF→UFM transform, presenter/template changes, and testapi issue-builder changes — it does not qualify, and this run also found data-correctness issues (below), so it is not approved.

Progress since the last run (head 4850156): the two prior Should-Fix findings on the severity filter and policy-modification severity mapping are fixed. The priorityScore golden-fixture gap is now mitigated by a dedicated TestTransformToUFMFromSarif_RiskScore unit test (partially addressed). The remaining prior findings (run-level properties omission, secrets problemID first-wins inconsistency, unused WithSeverityThreshold option, stray getExecutionFlowsFromIssue registration) are still open — see the existing review threads.

Oracle: local build/lint/test could not be run in this sandbox (this repo requires a Go 1.26 toolchain unavailable here; unrelated to this diff). CI's test/lint checks for this head commit should be treated as authoritative.

Security scan: manual review only — Snyk CLI/MCP auth was unavailable (no SNYK_TOKEN secret configured for this repo). No exploitable issues found manually: all new string interpolation into the SARIF/JSON template is escaped via strconv.Quote, SARIF-derived RuleIndex values are bounds-checked before use, and new deserialization uses typed encoding/json, not dynamic mechanisms.

Critical (2)

1. pkg/utils/ufm/transform_sarif.go:120 — the input SARIF's RuleIndex is copied verbatim into the finding, then rendered as the output document's result.ruleIndex (ufm.sarif.tmpl:144) against a deduplicated/reordered rules array — producing an invalid SARIF document once the driver's rule catalog contains rules unrelated to the emitted subset (see inline comment).
2. pkg/utils/ufm/transform_sarif.go:439 — suppression expiration is silently dropped (no error, no log) whenever it isn't an RFC3339 timestamp; Snyk's own "15 days"-style relative expirations (present in this PR's own with-ignores.json fixture) never survive into FindingData.Attributes.Suppression.ExpiresAt for consumers reading the structured API directly (see inline comment).

Should Fix (2)

3. pkg/utils/ufm/transform_sarif.go:168 — findings that share a rule ID but lack fingerprints collapse into a single Issue (grouped by Attributes.Key, which falls back to RuleID alone), silently dropping the second finding's distinct message/arguments/fingerprints from the rendered SARIF (see inline comment).
4. internal/presenters/templates/ufm.sarif.tmpl:189 — the Region JSON's comma placement is gated on the next field's presence rather than what was actually emitted; a common SARIF shape (startColumn set, endLine absent) renders "startLine": 5 "startColumn": 3 with no comma — invalid JSON (see inline comment).

Suggestions (5)

5. internal/presenters/funcs.go:283 — findingExtraLocal/suppressionExtraLocal/priorityScoreFactorLocal duplicate pkg/utils/ufm's exported types, justified by an import-cycle claim that doesn't hold when tracing pkg/utils/ufm's actual (non-test) imports — worth confirming before merge since it's a hand-synced duplication risk (see inline comment).
6. internal/presenters/templates/ufm.sarif.tmpl:422 — when $findingExtra is absent but multiple $policyMods are present, each is rendered as its own unkeyed "reason" entry with no separator, producing invalid/duplicate-key JSON (see inline comment).
7. internal/utils/findings/findings.go:12 — newly exported FilterSeverityASC duplicates the pre-existing unexported filterSeverityASC in internal/presenters/components.go; the two must now be kept in sync by hand (see inline comment).
8. internal/presenters/templates/ufm.sarif.tmpl:146 — switching to GetEffectiveSeverity has no observable effect today, since extractEffectiveSeverity always sets it equal to severity — worth a test asserting the two can actually diverge, or reverting the line.
9. pkg/apiclients/testapi/issues.go:786 — ruleShortDescription remains last-wins while problemID (line 777, from the still-open prior thread) is now first-wins for the same secrets problem loop — the two fields describing "the rule" for one issue can now disagree.

Good test coverage overall on the new transform (severity thresholds, suppression round-tripping, code flows, policy modifications, ID-collision handling via deterministic UUID fallback).

  

_{Sent by Cursor Automation: Automatic PR verification}

[cursor]

Critical: RuleIndex: res.RuleIndex copies the index from the input SARIF's full tool.driver.rules array. ufm.sarif.tmpl:144 renders this value verbatim as the output document's result.ruleIndex, but the output's rules array is deduplicateIssues $issues "problemID" — only the rules that actually produced findings, in a different order/size than the input driver's full rule catalog. Once the input driver lists rules beyond the emitted subset (the normal case for real Snyk Code scans), the original index no longer points at the correct entry in the output rules array — an invalid SARIF document per spec, breaking any consumer that resolves rule metadata via ruleIndex instead of ruleId.

Fix: build a problemID → outputIndex map while iterating the deduplicated rules (in the template or in Go) and use that mapped index instead of the original SARIF's index.

[cursor]

Critical: This only sets ExpiresAt if err == nil from time.Parse(time.RFC3339, ...). Snyk's own relative-duration expirations (e.g. "15 days", present in this PR's own internal/presenters/testdata/with-ignores.json:678 fixture) fail this parse and are silently dropped — no error, no log, no fallback. The SARIF-render path avoids this because the template reads the raw string from FindingExtra.Suppression.Expiration instead, so the structured API (TestResult.Findings()) and the rendered SARIF diverge on identical input. The sibling Local Findings transform keeps Expiration as a plain string with no parsing/loss.

Fix: at minimum, don't silently discard unparseable values — log/surface the parse failure. Ideally interpret Snyk's relative-duration format relative to IgnoredOn to compute a real ExpiresAt.

[cursor]

Should Fix: key falls back to res.RuleID alone when there's no fingerprint. selectGrouper in issues.go always uses keyBasedIssueGrouper for SAST findings (this transform always sets FindingType: FindingTypeSast), which groups strictly by Attributes.Key. Two distinct SARIF results for the same rule without fingerprints (common for tools that don't emit stable fingerprints) get the same key and merge into one Issue; getFindingExtraFromIssue then only returns the first finding's message/arguments/fingerprints, silently dropping the second's data from the rendered output.

Fix: derive the fallback key from rule ID plus location (mirroring generateFindingID a few lines below), not RuleID alone.

[cursor]

Should Fix: The comma after startLine is gated on $physicalLoc.Region.EndLine being present, and after endLine on StartColumn being present — but if EndLine is absent while StartColumn is set (a common single-line-span SARIF shape), you get "startLine": 5 "startColumn": 3 with no separating comma — invalid JSON. This is a variant of a pre-existing pattern (gate on the next field's presence, not on what was actually emitted), now reachable under a different, more common trigger since the field emission order changed in this PR.

Fix: track whether a field was actually emitted and gate each comma on that, rather than looking ahead to the next field.

[cursor]

Suggestion: This comment claims importing pkg/utils/ufm directly would create an import cycle, but tracing pkg/utils/ufm's non-test imports (internal/utils/findings, pkg/apiclients/testapi, pkg/configuration, pkg/local_workflows/json_schemas, pkg/utils/sarif, pkg/local_workflows/content_type, pkg/workflow) finds no path back to internal/presenters — only a _test.go file in ufm imports presenters, which doesn't create a build cycle. If that holds, findingExtraLocal/suppressionExtraLocal/priorityScoreFactorLocal are an unnecessary hand-synced duplicate of ufm.FindingExtra/SuppressionExtra/PriorityScoreFactor that will silently drift if a field is added to one side and not the other.

Suggest confirming whether the cycle is real; if not, import the types directly and drop the duplicates. If a cycle is found, please document the exact offending import path in the comment.

[cursor]

Suggestion: This branch (reached when $findingExtra is absent but $policyMods is non-empty — a real, reachable combination for pre-existing Local-Findings-sourced UFM data) ranges over $policyMods emitting one unkeyed "reason" entry per element with no comma/separator. More than one policy modification produces {"reason": "a" "reason": "b"} — invalid JSON with duplicate keys.

Suggest using an index-based range with a comma condition, and confirming the intended SARIF shape when more than one policy modification exists on an issue.

[cursor]

Suggestion: This newly-exported FilterSeverityASC is a near-duplicate of the pre-existing unexported filterSeverityASC in internal/presenters/components.go, kept separate presumably to let pkg/utils/ufm reuse the logic without an import cycle. The two implementations now need to be kept in sync by hand.

Suggest having internal/presenters/components.go's filterSeverityASC delegate to this exported version instead of maintaining a parallel copy, if no cycle prevents it.