Skip to content

fix(sbom): make sc sbom generate fail loudly on syft errors#292

Merged
Cre-eD merged 4 commits into
mainfrom
fix/sbom-cli-required
May 26, 2026
Merged

fix(sbom): make sc sbom generate fail loudly on syft errors#292
Cre-eD merged 4 commits into
mainfrom
fix/sbom-cli-required

Conversation

@Cre-eD

@Cre-eD Cre-eD commented May 25, 2026

Copy link
Copy Markdown
Contributor

Summary

sc sbom generate built a SecurityConfig with SBOM.Required=false, which tells the executor to fail-open: on syft errors it prints Warning: SBOM generation failed, continuing and returns (nil, nil). The CLI wrapper then converted that nil,nil into Error: no SBOM generated and exited 1 — fail-open semantics with a fail-closed exit code, and the underlying syft error (e.g. signal: killed for OOM) never reached the user.

This PR sets Required: true for the standalone CLI so the executor returns the real error directly, which the wrapper already maps to failed to generate SBOM: <reason>. The redundant if generatedSBOM == nil check after a non-nil-err path is also removed.

The Pulumi deploy path (pkg/clouds/pulumi/docker/build_and_push.go) is unchanged — it still honors whatever the user set in their stack's sbom.required.

Background

Surfaced by a PAY-SPACE pay_space_wallet production deploy where syft was OOM-killed on a large image. Pulumi saw the SBOM step exit 1 with the misleading no SBOM generated message and aborted the stack update. With this change, the user will see failed to generate SBOM: syft command failed: signal: killed instead and can act on it (bump runner memory, opt into fail-open in their stack config, etc).

Test plan

  • go build ./pkg/cmd/cmd_sbom/... ./pkg/security/... clean
  • CI: full test suite
  • Manual: invoke sc sbom generate against a tiny image — still succeeds
  • Manual: invoke against a deliberately broken image ref — now surfaces the real error instead of no SBOM generated

Scope

Touches only the standalone sc sbom generate CLI. No doc updates needed: docs/docs/reference/security-cli-commands.md only lists flags; fail-open mentions in docs/product-manager/container-security/acceptance-criteria.md (and api-contracts.md) are about the executor's deploy-path default, which is unchanged.

The standalone CLI built a SecurityConfig with SBOM.Required=false, which
told the executor to fail-open: on syft errors it printed
"Warning: SBOM generation failed, continuing" and returned (nil, nil).
The CLI wrapper then turned that nil-nil into "Error: no SBOM generated"
and exited 1 — fail-open semantics with a fail-closed exit code.

For a user-invoked `sc sbom generate`, the right behavior is fail-loud:
surface the real syft error (signal: killed for OOM, registry timeouts,
etc.) so the caller knows why. Setting Required=true makes the executor
return the underlying error directly, which the wrapper already maps to
"failed to generate SBOM: <reason>".

Surfaced by a PAY-SPACE pay_space_wallet production deploy where syft
was OOM-killed on a large image; Pulumi saw "no SBOM generated" exit 1
and aborted the stack update.

Signed-off-by: Dmitrii Creed <creeed22@gmail.com>
@Cre-eD
Cre-eD requested a review from smecsia as a code owner May 25, 2026 12:02
@github-actions

github-actions Bot commented May 25, 2026

Copy link
Copy Markdown

Semgrep Scan Results

Repository: api | Commit: 3eb8dca

Check Status Details
✅ Semgrep Pass 0 total findings (no error/warning)

Scanned at 2026-05-26 17:15 UTC

@github-actions

github-actions Bot commented May 25, 2026

Copy link
Copy Markdown

Security Scan Results

Repository: api | Commit: 3eb8dca

Check Status Details
✅ Secret Scan Pass No secrets detected
✅ Dependencies (Trivy) Pass 0 total (no critical/high)
✅ Dependencies (Grype) Pass 0 total (no critical/high)
📦 SBOM Generated 527 components (CycloneDX)

Scanned at 2026-05-26 17:15 UTC

@Cre-eD
Cre-eD merged commit 437b57e into main May 26, 2026
6 of 8 checks passed
@Cre-eD
Cre-eD deleted the fix/sbom-cli-required branch May 26, 2026 17:13
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