fix(cli): stop socket cdxgen from silently shipping empty-components SBOMs#1266
Open
John-David Dalton (jdalton) wants to merge 2 commits intomainfrom
Open
fix(cli): stop socket cdxgen from silently shipping empty-components SBOMs#1266John-David Dalton (jdalton) wants to merge 2 commits intomainfrom
John-David Dalton (jdalton) wants to merge 2 commits intomainfrom
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Async finally callback races with process.exit, warning never displays
- Reassigned the
.finally()chain back tocdxgenResult.spawnPromiseso the caller awaits the full chain including the asyncwarnIfEmptyComponentscall, preventingprocess.exit()from terminating before the warning is displayed.
- Reassigned the
Or push these changes by commenting:
@cursor push 61777b7392
Preview (61777b7392)
diff --git a/packages/cli/src/commands/manifest/run-cdxgen.mts b/packages/cli/src/commands/manifest/run-cdxgen.mts
--- a/packages/cli/src/commands/manifest/run-cdxgen.mts
+++ b/packages/cli/src/commands/manifest/run-cdxgen.mts
@@ -172,7 +172,8 @@
})
// Use finally handler for cleanup instead of process.on('exit').
- cdxgenResult.spawnPromise.finally(async () => {
+ // Reassign so the caller awaits the full chain, including the async warning.
+ cdxgenResult.spawnPromise = cdxgenResult.spawnPromise.finally(async () => {
if (cleanupPackageLock) {
try {
// This removes the temporary package-lock.json we created for cdxgen.
@@ -203,7 +204,7 @@
await warnIfEmptyComponents(fullOutputPath, argvMutable)
}
}
- })
+ }) as typeof cdxgenResult.spawnPromise
return cdxgenResult
}You can send follow-ups to the cloud agent here.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit fb64b27. Configure here.
…SBOMs When `socket cdxgen` runs with its safe defaults (`--lifecycle pre-build` + `--no-install-deps`) against a Node.js project that has neither a lockfile nor an installed `node_modules/`, cdxgen produces a structurally-valid CycloneDX document with `"components": []`. The Socket dashboard ingests the SBOM cleanly but has no dependency data to render, so the Alerts tab shows nothing — indistinguishable from a genuinely clean repo. Add two gates to the command: * Hard gate in cmd-manifest-cdxgen.mts: when the default lifecycle path kicks in for a Node.js type and neither a lockfile nor node_modules/ is findable upward from cwd, refuse with exit code 2 and an actionable message (install first or pass --lifecycle build). Skips the gate when the user passes --lifecycle, --filter, --only, or a non-Node --type. * Soft gate in run-cdxgen.mts: after cdxgen writes its output, parse it and warn loudly when `components` is empty. Covers configurations that slip past the hard gate (overly narrow --filter/--only, ecosystem mismatch, etc.) so an empty SBOM cannot ship silently. The detection helpers (`detectNodejsCdxgenSources`, `isNodejsCdxgenType`) are exported for unit testing and accept a `cwd` override so the probe can be exercised without relying on process.chdir (not supported in vitest workers).
fb64b27 to
493701a
Compare
The soft gate attached an `async .finally()` handler to `spawnPromise` but `runCdxgen` returned the original `cdxgenResult` — not the chained promise. The caller in `cmd-manifest-cdxgen.mts` awaited the original `spawnPromise` and then called `process.exit(result.code)`, so when the async finally yielded at `await warnIfEmptyComponents(...)` the caller's continuation fired first and killed the process before the warning printed. Rebind `spawnPromise` on the returned result to the chained promise so `await spawnPromise` in the caller naturally awaits the cleanup + warning. .finally() preserves the original value and rejection, so semantics for existing callers are unchanged. TypeScript note: .finally() returns plain Promise<T> which strips the `process` / `stdin` extras that SpawnResult carries. Callers only use the promise shape, so cast back to SpawnResult.
This comment was marked as resolved.
This comment was marked as resolved.
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
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.


What this fixes
socket cdxgencan silently produce a CycloneDX SBOM with zero components — a structurally valid document that the Socket dashboard ingests cleanly but has no dependency data to render. The Alerts tab then shows "no alerts," which is indistinguishable from a genuinely clean repo.Here's how it happens:
When you run
socket cdxgenwithout passing--lifecycle, we default it to--lifecycle pre-buildand--no-install-deps. That's intentional — it's a safety setting so that generating an SBOM doesn't run arbitrarypostinstallscripts from the user's dependencies.But for a Node.js project, that mode needs one of two things to find any dependencies:
pnpm-lock.yaml,package-lock.json, oryarn.lock), ORnode_modules/folder.If the user has neither, cdxgen happily writes out a CycloneDX file with zero components. No error. Our CLI then prints
socket-cdx.json created!and exits cleanly. The user uploads it, and the dashboard has nothing to show. Nothing in the CLI output tells them anything is wrong.What I changed
Two new checks in
socket cdxgen:1. Hard gate (stops the run before it generates a bad SBOM)
In
cmd-manifest-cdxgen.mts: right where we apply the default lifecycle, I added a check. If ALL of these are true:--lifecycle(so we're using the default),--filteror--only(those can intentionally produce empty SBOMs),node_modules/anywhere from cwd upward,…then we exit with code 2 and a clear error message telling the user exactly how to fix it:
We walk up from cwd (not just check cwd) because a lot of users run this in
packages/fooinside a monorepo, where the lockfile is at the repo root.2. Soft gate (catches anything the hard gate missed)
In
run-cdxgen.mts: after cdxgen finishes and writes its output file, we read the JSON back, check ifcomponentsis empty, and if so print a warning. This catches edge cases the hard gate doesn't cover:--lifecycle buildbut something went wrong and it still produced zero components.--filteror--onlytoo aggressively and every component got filtered out.The warning tells them what's wrong and suggests the same fix. Better than them finding out from a blank dashboard.
How I made the code testable
run-cdxgen.mtsalready had a function that ran cdxgen as a subprocess. I pulled out two small helper functions:detectNodejsCdxgenSources(cwd)— returns{ hasLockfile, hasNodeModules }by walking up from cwdisNodejsCdxgenType(argvType)— returns true if the--typeflag is a Node.js platformBoth are exported so the test file can import them.
detectNodejsCdxgenSourcestakes an optionalcwdargument so tests can point it at a specific directory without callingprocess.chdir(), which vitest's worker threads don't allow.Tests
Two test files in
packages/cli/test/unit/commands/manifest/:cmd-manifest-cdxgen.test.mts(existing file, added a new describe block):node_modules/node_modules/is present--lifecycleexplicitlyI also had to update the existing
beforeEachand some existing tests to mock the new helpers so they don't actually hit the filesystem — the mock returns "sources present" by default so the pre-existing tests behave the same as before.run-cdxgen.test.mts(new file):isNodejsCdxgenTypeacross the Node.js types we care about (js, javascript, typescript, nodejs, npm, pnpm, ts)isNodejsCdxgenTyperejects non-Node types (python, java, go, rust)isNodejsCdxgenTypeon arrays (if any entry is Node, it matches)detectNodejsCdxgenSourceswith various combinations of lockfiles/node_modules present/absent (via mockedfindUp)Test plan
pnpm --filter @socketsecurity/cli run type— cleanpnpm --filter @socketsecurity/cli run lint— cleanpnpm --filter @socketsecurity/cli run test:unit test/unit/commands/manifest/cmd-manifest-cdxgen.test.mts— 38/38 pass (5 new tests for the hard gate)pnpm --filter @socketsecurity/cli run test:unit test/unit/commands/manifest/run-cdxgen.test.mts— 18/18 pass (new test file)pnpm --filter @socketsecurity/cli run test:unit test/unit/commands/manifest/— 211/211 pass across the whole manifest folder, no regressionssocket cdxgenexits 2 with the expected message; with a lockfile, runs normally; with--lifecycle build, bypasses the gate.What I added to CHANGELOG.md
One entry under
[Unreleased] > Fixed, explaining the behavior change in user-facing terms.