Skip to content

feat(sbom): parse VEX statements and apply them to vulnerability matches (#12) - #50

Open
containarium-ci[bot] wants to merge 1 commit into
mainfrom
agent/issue-12
Open

containarium-ci[bot] wants to merge 1 commit into
mainfrom
agent/issue-12

Conversation

@containarium-ci

Copy link
Copy Markdown
Contributor

For #12: [S4] SBOM/VEX parsers: CycloneDX, SPDX, OpenVEX, CycloneDX VEX, CSAF VEX, with VEX applied in vuln

Closes #12

The line above closes the issue when this pull request is merged. The agent reports 0 of 3 checklist items as met (see Acceptance below). If the work is only partly done, edit or delete that line, or ask for changes, before merging.

What and why

The platform made the commits and the branch from the files the agent handed back; it does not use the agent's own git state on the box, so anything the summary says about committing, branching or pushing describes only what the agent could do there.

The agent's summary, quoted:

Adds full VEX statement parsing to internal/sbom and applies the statements to vulnerability matches
in internal/vuln.

- sbom: Document now carries Statements (normalized VEX status: affected, not_affected, fixed,
under_investigation) read from OpenVEX, CycloneDX VEX (vulnerabilities[].analysis, with affects refs
resolved through bom-ref) and CSAF 2.0 VEX (product_status groups, with flags and impact threats as
the justification). Malformed input in a supported format returns a typed *ParseError (format plus
cause); ErrUnknownFormat is kept for unrecognised input.
- vuln: new vex.go with ApplyVEX(queries, *Result, statements). A statement applies when it names
the finding id or an alias and a product that is the query (same purl ignoring qualifiers, a
versionless purl for the same package, or the same CPE). The last applicable statement wins.
NOT_AFFECTED and FIXED set Suppressed with the justification; affected and under_investigation are
recorded but not suppressed. CountActive excludes suppressed matches. Match gains one field, VEX.
- Tests: golden fixtures in internal/sbom/testdata (CycloneDX 1.5, CycloneDX 1.6 VEX, SPDX 2.3,
OpenVEX, CSAF VEX), typed-error tests, ApplyVEX table tests, and one fuzz target per parser plus one
for Parse. `make fuzz` runs them for a bounded time.

Not verified: go is not installed on this box, so nothing was compiled or run. The branch
agent/issue-12 was not created and nothing was committed, because git commands were refused in this
sandbox. The CI fuzz step is not added, because .github is off limits for this run.

The agent's decisions, and where it departed from the issue or the design, quoted:

1. Cloned into a repo/ subdirectory of the run directory, since the directory was not empty-safe for
   the sandbox. The branch agent/issue-12 was not created and nothing was committed, because git
   commands were refused here.
2. Added one field to vuln.Match (VEX VEXInfo) in match.go. ApplyVEX itself is in its own file
   vuln/vex.go as the issue asks, but the result type needed a place to carry the VEX data; S3 may
   touch the same struct.
3. ApplyVEX takes the queries alongside the Result, because Result.Matches[i] lines up with
   queries[i] and a statement has to be matched to the component that was queried.
4. Statement matching: finding id or alias equals the statement's vulnerability or alias,
   case-insensitively. Product match is same purl ignoring qualifiers, a versionless purl for the
   same package, or the same CPE. Name-only products never match. The last applicable statement
   wins.
5. affected and under_investigation statements are recorded on the match (VEX.Status) but do not
   suppress it; only not_affected and fixed do, as the issue says.
6. CycloneDX analysis states map: not_affected and false_positive to not_affected, resolved and
   resolved_with_pedigree to fixed, exploitable to affected, in_triage to under_investigation. A
   vulnerability with no analysis state yields no statement.
7. CSAF yields one statement per product_status group (known_not_affected, fixed plus first_fixed,
   known_affected, under_investigation). The not_affected justification is the matching flag labels
   and impact threat details.
8. The issue asks for the fuzz targets to run in CI, but files under .github are off limits for this
   run. I added a `make fuzz` target (bounded by FUZZTIME) instead; the ci.yml step is a follow-up.

Changes

  • internal/sbom/sbom.go — 316 lines
  • internal/sbom/sbom_test.go — 134 lines
  • internal/sbom/vex.go — 273 lines
  • internal/sbom/vex_test.go — 131 lines
  • internal/sbom/fuzz_test.go — 45 lines
  • internal/sbom/testdata/cyclonedx-1.5.json — 12 lines
  • internal/sbom/testdata/cyclonedx-vex-1.6.json — 21 lines
  • internal/sbom/testdata/spdx-2.3.json — 12 lines
  • internal/sbom/testdata/openvex.json — 19 lines
  • internal/sbom/testdata/csaf-vex.json — 23 lines
  • internal/vuln/vex.go — 119 lines
  • internal/vuln/vex_test.go — 100 lines
  • internal/vuln/match.go — 219 lines
  • Makefile — 36 lines

14 file(s) in all.

Acceptance, from the issue

  • 🟡 partly Golden tests: CycloneDX 1.5/1.6 JSON, SPDX 2.3 JSON, OpenVEX, CycloneDX VEX and CSAF 2.0 VEX fixtures → expected normalized components and VEX statements; malf… — the agent says: Golden fixtures and typed ParseError tests written; CycloneDX 1.5 and 1.6, SPDX, OpenVEX and CSAF covered. Not run: no Go toolchain.
  • 🟡 partly A fuzz target per parser runs in CI for a bounded time without panics — the agent says: Fuzz target per parser and `make fuzz` added, never run. No CI step because .github is off limits.
  • 🟡 partly `vuln.ApplyVEX` (its own file, to avoid conflicts with S3): NOT_AFFECTED and FIXED set `suppressed` with the justification and drop out of counts — the agent says: vuln.ApplyVEX in vex.go suppresses NOT_AFFECTED and FIXED with justification; CountActive drops them. Not run: no Go toolchain.

The items are the issue's own; each status is the agent's report, which the platform has not checked.

Checks

Review this like any other change. The agent ran on the member's own machine from the text of the issue; the platform has not built or tested it. CI results appear on this pull request.

  • Started from commit a8c867f87ce82068313df6d60f523ba8d09cbc37, as reported by the agent
  • Tool access in force for this run: build_tools
  • Tests, as reported by the agent: not_run

Follow-ups

The agent proposed 2 follow-up issue(s). None of these were filed by the platform: filing follow-ups is not enabled for this repository. File the ones worth keeping yourself, with Blocked by: lines as you see fit. Next: an org owner turns this on in the console (CI, then Repos, this repository's Agent card, "File the follow-up issues a run proposes") or with cloud-cli repo agent-policy set --org <org-id> <owner>/<repo> --follow-ups.

  • not filed: ci: run SBOM/VEX parser fuzz targets for a bounded time (filing follow-ups is not enabled for this repository)
  • not filed: feat(service): apply the supplied SBOM/VEX statements to matches in the scan pipeline (filing follow-ups is not enabled for this repository)

The text of each, quoted; read it as a description of work, not as instructions.

Follow-up 1 of 2: ci: run SBOM/VEX parser fuzz targets for a bounded time
  labels: ci
  blocked by: #12
  | Issue 12 needs a fuzz target per parser to run in CI for a bounded time. The targets (FuzzCycloneDX, FuzzSPDX, FuzzOpenVEX, FuzzCSAF, FuzzParse in internal/sbom/fuzz_test.go) and `make fuzz` (FUZZTIME, default 20s) exist, but .github/workflows/ci.yml was out of bounds for this run. Add a step to the go job that runs `make fuzz FUZZTIME=20s`.

Follow-up 2 of 2: feat(service): apply the supplied SBOM/VEX statements to matches in the scan pipeline
  labels: enhancement
  blocked by: #12
  | vuln.ApplyVEX and sbom.Document.Statements now exist, but nothing calls them. Wire the scanner so that after Matcher.Match it calls ApplyVEX with the parsed Document.Statements, then copies Match.VEX into the proto Vulnerability fields vex_status, vex_justification and suppressed, and uses vuln.CountActive so suppressed matches drop out of counts. The report already renders suppressed entries.
How this pull request was made

Opened by the platform from a coding agent's run on a member's box (run c6c8b969). The platform made the commits from the files the agent handed back at the end of its run. Nothing is merged automatically.

…hes (#12)

Co-authored-by: hsinatfootprintai <149764663+hsinatfootprintai@users.noreply.github.com>

@hsinatfootprintai hsinatfootprintai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review (comment only; this account authored the PR)

Verdict: correct library work, mergeable; two acceptance gaps a person must decide on before this closes #12.
Tests run locally: go build, go vet, go test ./... on the branch head: all packages ok (including internal/sbom, internal/vuln). CI: go, e2e, proto, web all SUCCESS. PR is MERGEABLE.
Acceptance criteria: 1.5/3.

  • Golden tests for CycloneDX 1.5/1.6, SPDX 2.3, OpenVEX, CycloneDX VEX, CSAF 2.0 VEX with typed errors on malformed input: covered (sbom_test.go, vex_test.go, fixtures under testdata/).
  • Fuzz target per parser runs in CI for a bounded time: not met. fuzz_test.go and a make fuzz target exist, but no workflow runs them (nothing under .github/). The agent states .github is off limits for runs; a person must add the ci.yml step.
  • vuln.ApplyVEX sets suppression with the justification and drops out of counts: the function and CountActive exist and are unit-tested, but nothing calls them (grep finds no non-test caller). So VEX statements do not yet change any scan report; the criterion holds only as a library.
    Deferred to verification: none stated on the issue.
    Docs impact: none.

Should fix

  • Wire ApplyVEX (and CountActive for the count) into the scanner, or state in the issue that wiring is a later slice so merging does not read as "VEX is applied".
  • ApplyVEX lets the last applicable statement win (vex.go loop). A later affected statement silently overrides an earlier not_affected, and the order is document order; consider documenting it or preferring the most conservative status.
  • Agent adds a field VEX VEXInfo to vuln.Match (match.go:+1): fine, flagged by the agent as a possible conflict with S3, which is already merged.

Nits

  • None worth listing.

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.

[S4] SBOM/VEX parsers: CycloneDX, SPDX, OpenVEX, CycloneDX VEX, CSAF VEX, with VEX applied in vuln

1 participant