feat(PPSC-1136): unify scan sbom — one command for CPE and purl SBOMs - #276
Merged
Conversation
Adds `armis-cli scan sbom-cpe <path>` — uploads a CycloneDX asset SBOM (single file, directory, or pre-built tar.gz) and drives it through the backend's new artifact_type=sbom-cpe scan flow. Findings appear in the standard tenant findings view; the raw per-CPE JSON (with low_confidence markers for synthesised CPEs) is downloaded to disk alongside for triage. - internal/scan/sbomcpe/sbomcpe.go: scan driver mirroring the image scanner. Packs the input into a tar.gz (skipping symlinks), enforces a 100MB cap, uploads via the existing StartIngest/WaitForIngest pipeline with ArtifactType="sbom-cpe", polls until completion, retrieves normalized findings, and downloads the raw JSON from sbom_cpe_results S3 key. - internal/cmd/scan_sbom_cpe.go: cobra command registered under `scan`. Explicitly rejects --sbom and --vex (the input already is an SBOM; SBOM→VEX generation belongs on `scan sbom`). Exposes --sbom-cpe-output with a default under .armis/. - Result plumbing: buildScanResult / convertNormalizedFindings / isEmptyFinding / cleanDescription duplicated from internal/scan/image verbatim, matching the pattern the repo scanner already follows. Kept local for minimal blast radius; a future ticket can lift the trio into internal/scan. - Purl-only SBOMs (e.g. npm/NuGet application manifests) surface as a clear error rather than silently returning zero findings — the backend emits a SBOM_PURL_ONLY_NOT_SUPPORTED sentinel finding pointing at the regular repo scanner. - No-findings UX intact: --fail-on / summary / findings-table degrade gracefully via the standard formatter pipeline. - 26 unit tests: extension helpers, tar packing (single file, directory with mixed extensions and nested subdirs, symlink skipping, empty-dir error), retry classifier, constructor + options, RunE flag rejection branches. ResultKeySBOMCPE constant is asserted against the backend contract string so a rename on either side fails loudly. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Exercises the full sbom-cpe scan flow against a bespoke httptest.Server that impersonates every endpoint the CLI hits, in the exact sequence: 1. POST /api/v1/ingest/presigned-url (artifact_type must be "sbom-cpe") 2. POST /_s3/upload (multipart-body S3 receiver) 3. POST /api/v1/ingest/scan 4. GET /api/v1/ingest/status/... 5. GET /api/v1/ingest/normalized-results 6. GET /api/v1/ingest/results (returns sbom_cpe_results URL) 7. GET /_s3/download (raw JSON dump) Test cases: - EndToEnd: full happy path. Asserts artifact_type contract, per-endpoint hit counts, findings pass through to the returned ScanResult, and the raw JSON dump lands on disk with the expected CVE. - TarballInput: verifies a pre-built tar.gz is forwarded verbatim (no re-packing). - RejectsMissingSbom: fail-fast on missing input path. - RejectsWrongExtension: single-file input with a non-.json/.xml extension. - EmptyDirRejected: directory containing no SBOM files errors before uploading an empty tarball. Also exports a PackForTest shim so the integration test can build a real tar.gz payload using the driver's own packing logic without duplicating tar/gzip code in the test file. Verified locally by building the binary and driving it against a Python mock server; the full round-trip completes with the expected finding in the human-readable output and the raw dump written to --sbom-cpe-output. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- packDir: prune common VCS/build/dependency directories (.git, node_modules, vendor, __pycache__, dist, build, target, .venv, .idea, .vscode, etc.) when walking a directory input, so an accidental repo-root scan does not pull thousands of package.json manifests into the tarball or blow the 100MB cap. - isRetryableError: match on structured *api.APIError (retry on 5xx) and errors.Is(context.DeadlineExceeded) / os.IsTimeout, instead of substring matching on err.Error() which missed APIError values whose stringified form does not contain the hardcoded markers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Drop redundant "var server; server = ..." split (staticcheck S1021). - Silence errcheck on defer gr.Close() in the tar-reading helper. - Silence gosec G304 on os.ReadFile/os.Create in test-only paths under t.TempDir(); those paths are constructed inside the test and cannot escape the sandbox. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
filepath.Rel returns "sub\c.json" on Windows, which tar treats as a literal filename rather than a directory separator. The backend extractor (Python tarfile on Linux) would produce a single file named "sub\c.json" instead of the "sub/c.json" the scanner expects, and the Windows-only Test & Coverage job caught it via TestPackDir_PicksUpJsonAndXml. Wrap the relative path in filepath.ToSlash so tar entries always use forward slashes, matching the tar spec and the Unix output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…PSC-1136-scan-sbom-cpe
The CLI now has a single ` + "`" + `armis-cli scan sbom <path>` + "`" + ` command. Users no longer choose between ` + "`" + `scan sbom` + "`" + ` (VEX-only) and ` + "`" + `scan sbom-cpe` + "`" + ` (findings-only) — the backend picks the right scanner based on the SBOM's contents and the CLI shows findings by default: * CPE-shaped SBOMs (asset inventories with explicit CPEs) → CpeSbomScanner → NVD findings. * Purl-shaped SBOMs (npm / NuGet / PyPI application manifests) → TrivySbomScanner → deps.dev findings. * Both paths produce a normalised findings table in the terminal; both gate on --fail-on; both write a raw JSON dump to .armis/<artifact>-sbom.json (override with --sbom-output). --vex-output opts into VEX generation. VEX comes from the sibling of whichever scanner ran (CpeVexGenerator or GrypeVexGenerator). Without --vex-output the CLI skips the VEX slot entirely. Deleted: - internal/cmd/scan_sbom_cpe.go + test - internal/scan/sbomcpe/ (whole package: driver, unit tests, integration tests, ~750 lines) Added: - internal/scan/normalized_findings.go — BuildScanResult / ConvertNormalizedFindings / IsEmptyFinding / CleanDescription lifted from the sbomcpe package so scan/sbom (and future scanners) can share them without duplicating ~150 lines. Modified: - internal/scan/sbom/sbom.go — driver now fetches normalized findings on both paths, downloads the raw JSON dump from whichever results_refs key is set (sbom_cpe_results or sbom_results), and only downloads VEX when --vex-output was requested. Adds ResultKeySBOMCPE constant with a contract test. - internal/cmd/scan_sbom.go — updated NewScanner call with page limit + includeNonExploitable; --vex-output opts into VEX; --sbom-output is now the raw-findings path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… on scan sbom scanCmd.PersistentPreRunE warns when --sbom-output is set without --sbom (and similarly for --vex-output without --vex). PPSC-1136 repurposes both flags for the unified `scan sbom` subcommand: --sbom-output → raw-findings JSON dump path --vex-output → opts into VEX generation Without this guard, every legitimate `scan sbom --sbom-output …` invocation prints a false "flag ignored" warning before it even starts. Skip the warnings only when the subcommand is `sbom`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds six httptest-based integration tests that exercise the unified
Scanner.Scan() end-to-end without touching the real backend:
1. CPE path, no VEX — backend advertises sbom_cpe_results; driver
downloads raw dump; no VEX file written.
2. Purl path, no VEX — backend advertises sbom_results; driver falls
back to that key; no VEX file written.
3. CPE path + --vex-output — findings + raw dump + VEX doc all written.
4. Purl path + --vex-output — same, on the purl branch.
5. --vex-output requested but backend omits vex_results — scan still
succeeds, warns, no VEX written.
6. Empty results_refs (no raw JSON advertised) — scan still succeeds,
warns, findings table still populated.
Each test wraps the shared MockScanServer (testutil.NewMockScanServer)
with a mux that:
- records the artifact_type sent to /presigned-url (must be "sbom" on
both paths — no more artifact_type=sbom-cpe)
- overrides /ingest/results to advertise the right results_refs keys
for the scenario under test
- serves /_download/raw and /_download/vex with canned bodies
Replaces the coverage lost when internal/scan/sbomcpe/ was deleted in the
unification commit (53841be). Same pattern the old sbomcpe integration
test used — just now against the single scan sbom driver.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
| Severity | Count |
|---|---|
| 🟠 HIGH | 1 |
Total: 1
View all 1 findings
🟠 HIGH (1)
CWE-22 - Broken Access Control (CWE-22
Location: internal/scan/sbom/sbom.go:285
Improper Limitation of a Pathname to a Restricted Directory (Path Traversal)): The code saves the downloaded raw findings to a file whose location comes from a command‑line option. That file path is taken from the user (the --sbom-output flag or the default) and is only passed through a function named SanitizePath. Because we don't see how SanitizePath works, we cannot be sure it blocks characters like ".." that could move the path outside the intended directory. If an attacker supplies a path such as ../../evil.txt, the program could write the data to an unexpected location, potentially overwriting important files. This means the vulnerable code can be reached directly from user input, and the risk is present whenever the CLI tool is used. No additional checks are performed after the sanitization step, so the unsafe write operation remains exposed.
// armis:ignore cwe:918 reason:ValidatePresignedURL enforces HTTPS + allowlisted S3 hosts
if err := s.client.ValidatePresignedURL(rawURL); err != nil {
return fmt.Errorf("invalid raw-results URL: %w", err)
}
// armis:ignore cwe:770 reason:DownloadFromPresignedURL enforces 100MB limit
data, err := s.client.DownloadFromPresignedURL(ctx, rawURL)
if err != nil {
return fmt.Errorf("failed to download raw findings (%s): %w", source, err)
}
if err := os.WriteFile(outputPath, data, 0600); err != nil {
return fmt.Errorf("failed to write raw findings to %s: %w", outputPath, err)
}
styles := output.GetStyles()
fmt.Fprintf(os.Stderr, "%s %s\n",
styles.SuccessText.Render(fmt.Sprintf("Raw findings (%s) saved to:", source)),
styles.Bold.Render(outputPath))
return nil
}
CWEs: CWE-22: Improper Limitation of a Pathname to a Restricted Directory (Path Traversal)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Test Coverage Reporttotal: (statements) 72.5% Coverage by function |
…aware retry Follow-up improvements to the scan sbom unification landed earlier in this PR: * Move the "--sbom-output ignored without --sbom" warning out of scanCmd.PersistentPreRunE (which had to test `cmd.Name() != "sbom"` — parents shouldn't know about children) into a warnOnUnusedSBOMVEXFlags() helper. scan_repo.go and scan_image.go call it explicitly; scan_sbom.go doesn't, so its repurposed flags don't trip a false warning. * Findings-retry loop in scan/sbom now respects ctx cancellation via a select on ctx.Done() and time.After — the previous time.Sleep held the goroutine hostage for the full backoff even after SIGINT. * Deleted the unused EnableVEX() method — WithVEXOutput already sets generateVEX=true, so there was no caller that needed the empty-path variant. * pickRawFindingsURL: rename the map-check local `ok` so it no longer shadows the named return of the same name. Same behaviour, cleaner reads. * TestIntegration_CPE_NoVEX: anchor cwd to tmpDir via t.Chdir so the "no VEX written" assertion checks the same path the driver would actually write to (the driver's default is .armis/<artifact>-vex.json relative to cwd). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
yiftach-armis
approved these changes
Jul 14, 2026
| if err != nil { | ||
| return fmt.Errorf("failed to download raw findings (%s): %w", source, err) | ||
| } | ||
| if err := os.WriteFile(outputPath, data, 0600); err != nil { |
10 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
Type of Change
Problem
Users had to know CycloneDX internals to pick between two commands:
scan sbom→ uploaded an SBOM, downloaded a VEX doc (findings were hidden).scan sbom-cpe→ uploaded an SBOM, printed CVE findings.Nobody knew which one they wanted. "scan sbom" also silently threw away Trivy findings that the backend produced, so users doing a normal-looking SBOM scan saw "VEX generated → path (N statements)" and nothing else.
Solution
One command, one code path, findings-first. The backend classifies the SBOM and picks the scanner (see companion Moose PR); the CLI is oblivious to the routing and treats
artifact_type=sbomuniformly:/ingest/normalized-results).scan repo/scan image).results_refskey the backend advertised (sbom_cpe_resultson the CPE path,sbom_resultson the purl path). Bifurcation is client-side by key presence.--vex-outputwas explicitly requested.CLI surface
Removed:
scan sbom-cpecommand + its test fileinternal/scan/sbomcpe/package (driver + unit tests + integration tests — ~1600 lines)--sbom-cpe-outputflagRepurposed:
--sbom-outputonscan sbom→ raw-findings JSON dump path (default.armis/<artifact>-sbom.json). Onscan repo/scan imageit still means "where to write the generated SBOM" — the parent PersistentPreRunE's flag-mismatch warning was scoped to skip thesbomsubcommand.--vex-outputonscan sbom→ opts into VEX generation; empty ⇒ VEX not requested.Added:
internal/scan/normalized_findings.go—BuildScanResult/ConvertNormalizedFindings/IsEmptyFinding/CleanDescriptionlifted out of the deleted sbomcpe package so the newscan sbomdriver (and any future scanners) can share them without ~150 lines of duplication.Backward compat
scan sbom-cpeinvocation → cobra printsscan --helpand exits 0 (unknown subcommand). Since PPSC-1136's CLI half never shipped, no existing user script breaks.scan sbombehaviour change: findings are now printed (previously hidden), and VEX is opt-in via--vex-output(previously always downloaded). This is a small breaking change for the week PPSC-971 has been merged; acceptable since it fixes the "findings hidden" UX issue.Testing
Automated Tests
Added
internal/scan/sbom/sbom_integration_test.go— six httptest-based integration tests that exercise the full driver against a mock backend without hitting the network:TestIntegration_CPE_NoVEXsbom_cpe_resultsTestIntegration_Purl_NoVEXsbom_resultsTestIntegration_CPE_WithVEXsbom_cpe_results+vex_resultsTestIntegration_Purl_WithVEXsbom_results+vex_resultsTestIntegration_VEXRequestedButBackendMissingItsbom_resultsonly,--vex-outputsetTestIntegration_NoResultsRefsresultsdictPlus the pre-existing
sbom_test.go(path validation, artifact-name derivation, VEX statement counting, contract test lockingResultKeySBOMCPE = \"sbom_cpe_results\"to backend).Manual Testing
Ran the CLI binary against the local
make devbackend end-to-end for both fixture types (CPE Torizon SBOM + synthetic purl SBOM). Full matrix (routing decisions, results_refs bifurcation, VEX download) covered by the integration tests since local dev's OAuth device flow makes CLI-binary auth painful; the driver code exercised by the tests is the same code the binary runs.go test ./...— 22/22 packages green (78s scan pkg, sub-second everything else).golangci-lint run ./...— 0 issues.Reviewer Notes
--sbom-outputsemantics diverge across subcommands. Onscan repo/scan imageit's still "where to write the generated SBOM". Onscan sbomit's "where to write the raw findings JSON". The parent PersistentPreRunE only fires the flag-mismatch warning when subcommand ≠sbom. This is intentional (unified UX) but does mean the--helptext is command-dependent.scan sbomusers on PPSC-971 (~one week). Their scripts that relied on.armis/<artifact>-vex.jsonappearing implicitly need to pass--vex-outputnow.internal/scan/sbomcpe/deleted wholesale. No external callers; all coverage moved tointernal/scan/sbom/sbom_integration_test.go.CpeVexGenerator) is a new backend scanner; see companion PR. Its output flows through the samevex_resultsresults_refs slot as Grype/Trivy VEX, so the CLI doesn't have to distinguish them.Checklist
go vet+golangci-lint runclean)scan sbom)