fix: include shipped dependencies in JavaScript SBOMs#17
Conversation
cdxgen was invoked with --required-only. For bun lockfiles cdxgen derives component scope from source-usage evidence rather than the manifest: a package is marked `required` only when it has a production source import, and everything else (shipped transitive dependencies, and production dependencies imported only from type-declaration or test files) is marked `optional`. --required-only then drops that whole set, so the generated SBOM and third-party notices silently under-reported shipped dependencies. Stop passing --required-only so the SBOM reflects the full installed closure. Development dependencies are now excluded by installing production dependencies only before generation (documented in the README Boundaries section) instead of relying on cdxgen's unreliable bun scope. Add a real-cdxgen integration test (a workspace with a source-imported dependency plus a production dependency imported only from a .d.ts and a test file, installed production-only) asserting the shipped dependency and its transitive runtime dependency appear while a devDependency does not. The CI integration-real job gains a bun toolchain to run it.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the --required-only flag from the cdxgen invocation to prevent shipped transitive JavaScript dependencies from being dropped when using bun lockfiles. It also updates the README and adds integration tests to verify this behavior. The reviewer pointed out that omitting --required-only globally causes a regression for non-JavaScript projects (such as Rust), which would now include development dependencies in their SBOMs. The reviewer suggested conditionally applying the --required-only flag only when the JavaScript ecosystem is not present.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // Deliberately omit cdxgen's --required-only. For bun lockfiles cdxgen | ||
| // derives component scope from source-usage evidence rather than the | ||
| // manifest, so it marks shipped transitive dependencies (and production | ||
| // dependencies imported only from type-declaration or test files) as | ||
| // optional and drops them, producing an incomplete SBOM. Completeness of | ||
| // the shipped closure is gated on the install instead: callers must | ||
| // install production dependencies only (see README "Boundaries"). |
There was a problem hiding this comment.
Omitting --required-only globally will cause non-JavaScript projects (such as pure Rust projects) to include development dependencies in their generated SBOMs and third-party notices. This is a regression for those ecosystems.\n\nWe should conditionally omit --required-only only when the project contains the Javascript ecosystem, preserving the correct behavior for Rust-only projects.
| // Deliberately omit cdxgen's --required-only. For bun lockfiles cdxgen | |
| // derives component scope from source-usage evidence rather than the | |
| // manifest, so it marks shipped transitive dependencies (and production | |
| // dependencies imported only from type-declaration or test files) as | |
| // optional and drops them, producing an incomplete SBOM. Completeness of | |
| // the shipped closure is gated on the install instead: callers must | |
| // install production dependencies only (see README "Boundaries"). | |
| // Deliberately omit cdxgen's --required-only for JavaScript projects. For bun lockfiles cdxgen\n // derives component scope from source-usage evidence rather than the\n // manifest, so it marks shipped transitive dependencies (and production\n // dependencies imported only from type-declaration or test files) as\n // optional and drops them, producing an incomplete SBOM. Completeness of\n // the shipped closure is gated on the install instead: callers must\n // install production dependencies only (see README "Boundaries").\n // For other ecosystems (like Rust), we still want to enforce --required-only.\n .args(if !project.ecosystems.contains(&Ecosystem::Javascript) {\n Some(\"--required-only\")\n } else {\n None\n }) |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5988c4a294
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .arg("--exclude-regex") | ||
| .arg(build_exclude_regex(sbom_config)) | ||
| .arg("--required-only") | ||
| // Deliberately omit cdxgen's --required-only. For bun lockfiles cdxgen |
There was a problem hiding this comment.
This command is shared by Rust-only and mixed projects, but the new install-time workaround only constrains JavaScript node_modules; there is no equivalent production install step for Cargo. I checked cdxgen's CLI docs, and --required-only is the flag that limits output to components with required scope, so dropping it globally means a Rust project with test-only/dev packages in Cargo.lock can now leak those components into the generated SBOM and notices. Please keep the omission scoped to the JavaScript/bun case, or otherwise preserve required-scope filtering for Rust scans.
Useful? React with 👍 / 👎.
Address review feedback: only omit cdxgen's --required-only for projects that include the JavaScript ecosystem, where the flag drops shipped bun dependencies. Non-JavaScript projects (e.g. Rust crates) keep the flag so their behavior is unchanged. (Verified that under --no-install-deps cdxgen assigns no scope to Rust components, so --required-only is currently a no-op there; keeping it scoped avoids changing the Rust invocation and is future-proof if cdxgen starts scoping cargo dev-dependencies.)
1 participant
Problem
The JavaScript SBOM and third-party notices silently omit shipped dependencies. Concretely, a production dependency can be present in
package.json, resolved in the lockfile, installed, and shipped — yet absent fromprovenance/.../sbom.cdx.jsonandTHIRD-PARTY-NOTICES.txt.Root cause is the
cdxgen --required-onlyflag (src/sbom.rs). For bun lockfiles cdxgen does not resolve the dependency graph (it emits an emptydependenciesarray) and instead derives component scope from source-usage evidence:scope: requiredscope: optionalThat "everything else" includes genuinely shipped code:
.d.tsor test files (e.g. a package used solely for adeclare moduletype augmentation).--required-onlythen drops the entireoptionalset. cdxgen cannot distinguish these from devDependencies — they are byte-for-byte identical in scope/evidence.Reproduction (real cdxgen 12.1.5, bun, hoisted)
Workspace with a source-imported dep (
picocolors) + a production dep imported only via.d.ts/test (debug→ transitivems) + a devDep (left-pad):--required-only(current)picocolors—debug/msdroppedleft-pad(over-reports devDep)--required-onlypicocolors— still dropped (prod install alone is not enough)picocolors,debug,ms;left-padexcluded ✅Only the last combination yields the correct shipped closure, so both changes are required.
Fix
--required-onlyso the SBOM reflects the full installed closure.install-commandmust add--production(paired PRs follow).Tests
Adds a
#[ignore]real-cdxgen integration test mirroring the reproduction (workspace, production install) that assertsdebug+ transitivemsare present and theleft-paddevDependency is absent. It fails if--required-onlyis reintroduced. CI'sintegration-realjob gains a bun toolchain to run it.Rollout
This changes generated artifacts: the next provenance release must be cut, and consumer repos must bump
provenance-versionto it and switch their install to--production(paired PRs). Merging a consumer bump before the release is cut would break its nightly download.