DONT MERGE: feat: add kosli attest sarif command#838
DONT MERGE: feat: add kosli attest sarif command#838AlexKantor87 wants to merge 3 commits intomainfrom
kosli attest sarif command#838Conversation
The package parses generic SARIF v2.1.0 (used by Snyk, Checkov, Trivy, Semgrep, etc.), not Snyk-specifically. Rename to reflect the actual scope ahead of adding a first-class `kosli attest sarif` command that will share this parser. No behaviour change: the snyk attestation wire format keeps the `snyk_results` JSON key, and the snyk command continues to invoke the shared parser via its new name (`sarif.ProcessSarifResultFile`). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
A first-class SARIF attestation type, parallel to `attest snyk`, that:
- Accepts SARIF v2.1.0 from any compatible scanner (Checkov, Trivy,
Semgrep, Snyk, CodeQL, etc.) and reports the parsed findings to Kosli.
- Surfaces the tool name and version dynamically from the SARIF report's
`runs[0].tool.driver` fields rather than hardcoding a label.
- Adds a `-C, --compliant` flag (default true, mirroring `attest generic`)
so the caller controls whether the attestation is compliant. The CLI
does not derive compliance from the SARIF findings — that decision
is the caller's, expressed via the flag (and typically driven by their
own policy or rego rules).
POSTs to a new server endpoint `api/v2/attestations/{org}/{flow}/trail/{trail}/sarif`
which is added in a follow-up commit on the server side.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @AlexKantor87's task in 2m 33s —— View job PR Review:
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if !strings.HasPrefix(report.Schema, "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/") && !strings.HasPrefix(report.Schema, "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-") && !strings.HasPrefix(report.Schema, "https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/schemas/sarif-schema-2.1.0.json") { |
There was a problem hiding this comment.
This very long line is hard to read. Consider breaking it into a slice of valid prefixes and iterating, e.g.:
validPrefixes := []string{
"https://raw.githubusercontent.com/oasis-tcs/sarif-spec/main/",
"https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-",
"https://docs.oasis-open.org/sarif/sarif/v2.1.0/errata01/os/schemas/sarif-schema-2.1.0.json",
}Also worth noting: not all SARIF v2.1.0 producers set the $schema field (e.g., some Checkov / Trivy versions emit a valid SARIF file without it, or use a different URL). Since this command is explicitly marketed as "any compatible SARIF scanner", this validation may reject valid SARIF from those tools. Worth testing with real Checkov/Trivy output to confirm.
(This is pre-existing logic from the snyk package, so not a blocker for this PR — but flagging it as something to revisit.)
| payload SarifAttestationPayload | ||
| } | ||
|
|
||
| const attestSarifShortDesc = `Report a SARIF attestation to an artifact or a trail in a Kosli flow. ` |
There was a problem hiding this comment.
Nit: Trailing whitespace in the short description string (two spaces before the closing backtick). This is consistent with the existing attestSnykShortDesc, but if you're cleaning things up it's worth removing.
| const attestSarifShortDesc = `Report a SARIF attestation to an artifact or a trail in a Kosli flow. ` | |
| const attestSarifShortDesc = `Report a SARIF attestation to an artifact or a trail in a Kosli flow.` |
Emit a debug line when SARIF parsing starts (with file path) and another once parsing succeeds with the tool name, severity counts, and the caller-supplied --compliant flag value. Useful when troubleshooting CI pipelines where the upload silently looks empty. The error path is unchanged — parse failures still surface via the wrapped error, consistent with `attest snyk`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
| payload SarifAttestationPayload | ||
| } | ||
|
|
||
| const attestSarifShortDesc = `Report a SARIF attestation to an artifact or a trail in a Kosli flow. ` |
There was a problem hiding this comment.
Nit: trailing whitespace (two spaces before the closing backtick). Already flagged in the prior review — just confirming it's still present.
| const attestSarifShortDesc = `Report a SARIF attestation to an artifact or a trail in a Kosli flow. ` | |
| const attestSarifShortDesc = `Report a SARIF attestation to an artifact or a trail in a Kosli flow.` |
| func (suite *AttestSarifCommandTestSuite) TestAttestSarifCmd() { | ||
| tests := []cmdTestCase{ |
There was a problem hiding this comment.
The test suite only exercises Snyk SARIF fixtures (testdata/snyk_sarif.json). Since the PR markets this command as accepting "any compatible SARIF scanner", consider adding at least one test case using a non-Snyk fixture (Checkov or Trivy output) to prove the parser handles different tool.driver.name values and different level formats end-to-end. The existing internal/sarif/sarif_test.go covers the parser, but a command-level integration test would close the loop.
| if len(o.payload.SarifResults.Results) > 0 { | ||
| r := o.payload.SarifResults.Results[0] | ||
| logger.Debug("SARIF parsed: tool=%s findings=%d high, %d medium, %d low (compliant=%t)", | ||
| o.payload.SarifResults.Tool.Name, r.HighCount, r.MediumCount, r.LowCount, o.payload.Compliant) | ||
| } |
There was a problem hiding this comment.
Minor: the len(o.payload.SarifResults.Results) > 0 guard protects the debug log, but if a valid SARIF file has zero runs (e.g. an empty scan), this silently skips without any debug output. Consider adding an else branch:
} else {
logger.Debug("SARIF parsed: tool=%s, no results found", o.payload.SarifResults.Tool.Name)
}Nice addition of the debug logging overall — it's a good practice for production troubleshooting.
1 participant
Summary
Adds a first-class
kosli attest sarifcommand, parallel tokosli attest snyk, that:runs[0].tool.driverfields, so the Kosli UI tile reads "Checkov 3.2.1" / "Trivy 0.52" / etc. depending on what was run.-C, --compliantflag (default true, mirroringattest generic) so the caller controls whether the attestation is compliant. The CLI does not derive compliance from the SARIF findings — that decision is the caller's, expressed via the flag (typically driven by their own policy or rego rules).Why
Customers currently map non-Snyk SARIF scans (notably Checkov) to the
snykattestation type because no native SARIF type exists. This works for parsing/display but bakes Snyk-specific compliance logic ("any high/medium/low → fail") into a generic SARIF flow, hardcodes "Snyk" as the tool label, and gives customers no way to express their own threshold. The newsariftype fixes all three.Changes
Slice 1 — refactor:
internal/snyk→internal/sarifThe existing snyk parser is genuinely a generic SARIF v2.1.0 parser — it just lives in a
snyk-named package. Renamed to reflect actual scope; theattest snykcommand keeps working unchanged via the renamed package. Wire format preserved (still postssnyk_resultsJSON key for backwards compatibility).Slice 2 — new
attest sarifcommandcmd/kosli/attestSarif.gowith--scan-results,--upload-results, and--compliantflagsapi/v2/attestations/{org}/{flow}/trail/{trail}/sarif(added in server#5538)is_compliant: booltaken from the flag verbatim--compliant=falseand explicit--compliant=truecasesLocal verification
End-to-end smoke-tested against a local demo running the server branch:
kosli attest sarif --scan-results sarif-iac.json→ 201, attestation stored with dynamic tool name "Snyk IaC" 1.1177.0 andis_compliant: truekosli attest sarif --scan-results sarif-helm.json --compliant=false→ 201,is_compliant: falsestored verbatim despite zero high-severity findingsgo test ./internal/sarif/...— passesgo test ./...) — would only pass against a server with the/sarifroute, hence the merge blockTest plan
--compliant=trueand--compliant=false, confirm both tiles render correctly🤖 Generated with Claude Code