Use rules_nodejs toolchain for hermetic cdxgen execution by olivembo · Pull Request #4 · eclipse-score/sbom-tool

olivembo · 2026-04-07T08:33:05Z

Summary

This PR makes SBOM generation more reproducible and closer to hermetic builds.

What changed

Switched cdxgen execution to Bazel-managed Node toolchain in rules.bzl.
Added cdxgen_version parameter through macro/rule in defs.bzl and rules.bzl.
Replaced shell-based crates cache generation with a Bazel py_binary action path in BUILD.bazel and rules.bzl.
Updated module/tooling setup and reproducibility settings in MODULE.bazel, MODULE.bazel.lock, and .bazelrc.
Updated docs/tests and removed legacy npm wrapper in README.md, test_generate_crates_metadata_cache.py, BUILD.bazel, and npm_wrapper.sh.

Notes

Includes both branch commits (23d543e, d56768c).
auto_cdxgen still requires network and no-sandbox behavior as configured.

Copilot

Pull request overview

This PR aims to make SBOM generation more reproducible by moving cdxgen execution onto a Bazel-managed Node/npm toolchain and by refining how Rust crate metadata is generated (crates.io-first, with optional dash-license-scan).

Changes:

Switched auto-cdxgen to run via rules_nodejs toolchain and added a pinned cdxgen_version parameter.
Updated Rust crates metadata cache generation to use crates.io metadata by default, with optional dash-license-scan via CLI flag.
Removed the legacy host npm_wrapper and tightened Bazel reproducibility with lockfile enforcement.

Reviewed changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`internal/rules.bzl`	Uses Bazel-managed Node toolchain for cdxgen; refactors crates cache generation to a Bazel `py_binary`; adds `cdxgen_version`; adds Node toolchain requirement.
`defs.bzl`	Threads `cdxgen_version` through the public `sbom()` macro.
`scripts/generate_crates_metadata_cache.py`	Adds crates.io license fallback and optional dash-license-scan execution via `--use-dash-license-scan`.
`scripts/BUILD.bazel`	Adds `py_binary` target for `generate_crates_metadata_cache` to support Bazel execution.
`tests/test_generate_crates_metadata_cache.py`	Adds unit tests for crates.io license normalization and precedence over dash-license-scan.
`README.md`	Updates documentation to reflect Bazel-managed Node/npm usage, crates.io-first Rust metadata, and new `cdxgen_version`.
`MODULE.bazel`	Adds `rules_nodejs` and configures Node toolchain; configures `uv` extension as a dev dependency.
`MODULE.bazel.lock`	Adds the module lockfile to the repo to support `--lockfile_mode=error` reproducibility.
`BUILD.bazel`	Removes the legacy `npm_wrapper` `sh_binary`.
`npm_wrapper.sh`	Deletes the host npm/nvm wrapper script.
`.bazelrc`	Enforces lockfile reproducibility with `--lockfile_mode=error`.
`.gitignore`	Stops ignoring `MODULE.bazel.lock` so it can be checked in.

Comments suppressed due to low confidence (1)

internal/rules.bzl:146

The cdxgen action passes node_info.npm.path as a script argument, but node_info.npm itself is not explicitly declared as an input/tool. If npm_sources doesn’t include the entrypoint file, the action will fail with a missing file at execution time.

        ctx.actions.run(
            outputs = [cdxgen_sbom],
            tools = [node_info.node],
            inputs = node_info.npm_sources,
            executable = node_info.node,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

olivembo · 2026-08-21T07:10:55Z

+        ctx.actions.run(
            inputs = cache_inputs,
            outputs = [crates_cache],
-            command = cache_cmd,
+            executable = ctx.executable._crates_cache_script,
+            arguments = [cache_args],


Ran this end-to-end with auto_crates_cache = True in examples/dummy_project: the action executed fine and produced a correct _crates_metadata.json with valid SHA-256 checksums. Bazel wires up the py_binary's runfiles automatically when it's passed via ctx.executable.<attr> as executable=, so no separate tools= entry is needed here. Not an issue.

olivembo · 2026-08-21T07:10:57Z

            cfg = "exec",
        ),
    },
+    toolchains = ["@rules_nodejs//nodejs:toolchain_type"],


Fair point. Declaring toolchains = ["@rules_nodejs//nodejs:toolchain_type"] at the rule level does force Node toolchain resolution for every sbom_rule instantiation regardless of auto_cdxgen, unlike the old npm_wrapper.sh which only needed npm on the host if actually invoked. It's a deliberate trade-off for hermetic auto_cdxgen support, not a bug, but worth calling out for Rust-only/air-gapped consumers who never set auto_cdxgen = True.

masc2023 · 2026-08-20T16:26:52Z

@olivembo , is this PR still valid? Then please update and resolve conflicts, otherwise please close it

olivembo · 2026-08-21T06:29:20Z

Still valid. Rebased onto current main and resolved conflicts. Kept the crates.io-first (with optional dash-license-scan) approach from this branch alongside the new Python/Java collector and CI additions from main, regenerated MODULE.bazel.lock (root + examples/dummy_project), and dropped the now-unused npm_wrapper.sh/`//:npm_wrapper" references in favor of the Bazel-managed Node toolchain. All unit tests, the dummy_project integration test, and the format/copyright checks pass locally.

node_info.npm is a self-contained shell wrapper (it locates its own node + npm-cli.js internally), not a JS entry point despite the NodeInfo docstring, so passing its path as an argument to node_info.node crashed with a JS syntax error. Run it directly as the executable instead. Also needs use_default_shell_env, without which npm's own "exec -- cdxgen" invocation could not find the cdxgen binary it had just resolved. Verified end-to-end by temporarily flipping auto_cdxgen/auto_crates_cache to True in examples/dummy_project and running a real build.

Copilot

Pull request overview

Copilot reviewed 11 out of 13 changed files in this pull request and generated 3 comments.

Suppressed comments (4)

README.md:143

This blanket prerequisite is inaccurate for consumers that provide python_lockfiles: auto_python_cache remains enabled by default and generate_python_metadata_cache.py still invokes host uvx to run dash-license-scan, whose Java-based scanner requires a host Java runtime. Qualify this statement or update the Python cache path before claiming that no host Java installation is required.

No host-level installation of `nvm`, `npm`, `node`, or `openjdk` is required when using this SBOM rule as intended.

README.md:146

This new prerequisite statement conflicts with the downstream smoke-test documentation: examples/dummy_project/BUILD.bazel:29-34 still says auto_cdxgen requires host npm and a globally installed cdxgen. Update that example comment (and any similar usage notes) so consumers are not told to install tooling that this change removed.

- `auto_cdxgen = True`: cdxgen is executed via Bazel-managed tooling.
- `auto_crates_cache = True`: crate metadata cache generation is executed via Bazel-managed tooling.

internal/rules.bzl:164

The new action is not exercised by the repository's integration test: examples/dummy_project explicitly sets auto_cdxgen = False, so CI does not validate the Node toolchain provider, npm exec arguments, or the cdxgen output path introduced here. Add an opt-in integration test with auto_cdxgen enabled (or a hermetic toolchain stub) that verifies this execution path.

        ctx.actions.run(
            outputs = [cdxgen_sbom],
            inputs = node_info.npm_sources,
            executable = node_info.npm,
            arguments = [

internal/rules.bzl:149

This replacement drops the old action's use_default_shell_env = True even though the script still calls urllib.request for crates.io. Bazel actions do not inherit HTTP_PROXY/HTTPS_PROXY/NO_PROXY by default, so builds behind a required proxy can fail despite requires-network. Preserve the needed proxy environment explicitly (or set use_default_shell_env = True).

            execution_requirements = {"requires-network": ""},
        )

olivembo · 2026-08-21T07:44:36Z

        dep_module_files = None,
        cdxgen_sbom = None,
        auto_cdxgen = False,
+        cdxgen_version = "12.1.4",


Good catch. Fixed in d732a15 by moving cdxgen_version after exclude_patterns, at the end of the optional parameter list, so pre-existing positional call sites keep binding correctly.

olivembo · 2026-08-21T07:45:02Z

        ctx.actions.run(
            outputs = [cdxgen_sbom],
-            executable = ctx.executable._npm,
+            inputs = node_info.npm_sources,


Confirmed this is real and pre-existing (not introduced by this diff — the prior implementation had no inputs at all for this action, so it had the same cache-invalidation gap). Documented the caveat inline as a known limitation rather than attempting a full fix here, since correctly keying this action would need collecting the transitive source-file list for the scanned target(s) — a bigger change than this PR's scope. Left as a follow-up.

olivembo · 2026-08-21T07:45:04Z

+            license_expr = _normalize_license_expression(
+                (crate.get("license") or "").strip()
+            )


Actually worse than described. I checked the live crates.io API and the crate-level endpoint (/api/v1/crates/{name}) has no "license" field at all, so this fallback was returning "" for every crate, not just the wrong version's license. Fixed in d732a15 by fetching the version-scoped endpoint (/api/v1/crates/{name}/{version}) pinned to the locked version, plus a unit test asserting the locked version's license wins over a differently-licensed later release.

- generate_crates_metadata_cache.py: crates.io's crate-level endpoint has no license field at all (always ""), so the "crates.io license fallback" was a no-op. Fetch license from the version-scoped endpoint pinned to the locked version instead, so a crate that relicensed between the locked and latest version reports the correct one. - internal/rules.bzl: restore use_default_shell_env on the crates cache action, dropped when it was converted from run_shell to actions.run; without it, proxy env vars aren't inherited and requires-network builds can fail behind a required proxy. - defs.bzl: move the new cdxgen_version parameter to the end of sbom()'s optional parameter list so callers using the old positional argument order still bind correctly. - README.md / examples/dummy_project/BUILD.bazel: fix now-inaccurate prerequisite claims (python_lockfiles still needs a host JRE via dash-license-scan; the dummy_project comment still described the removed npm_wrapper.sh flow). - examples/dummy_project: add an opt-in, network-tagged smoke test (cdxgen_smoke_test) exercising auto_cdxgen=True end-to-end, since the default hermetic integration test always sets it False and so never caught the Node toolchain bugs fixed in the previous commit.

AlexanderLanin

sbom-tool is currently not truly maintained; no obvious problems in PR; merging PR as-is.

olivembo requested a review from evinoth1206 April 7, 2026 08:43

masc2023 requested a review from Copilot July 29, 2026 09:40

Copilot started reviewing on behalf of masc2023 July 29, 2026 09:41 View session

Copilot AI reviewed Jul 29, 2026

View reviewed changes

olivembo added 2 commits August 21, 2026 08:04

Try to get closer to a hermetic bujild

1df1389

Use Node toolchain for hermetic cdxgen execution

38b4864

olivembo force-pushed the hermetic branch from d56768c to 38b4864 Compare August 21, 2026 06:28

olivembo requested review from AlexanderLanin and masc2023 as code owners August 21, 2026 06:28

masc2023 requested a lite review from Copilot August 21, 2026 07:16

Copilot started reviewing on behalf of masc2023 August 21, 2026 07:16 View session

Copilot AI reviewed Aug 21, 2026

View reviewed changes

AlexanderLanin approved these changes Aug 21, 2026

View reviewed changes

AlexanderLanin merged commit d152da7 into eclipse-score:main Aug 21, 2026
7 checks passed

Navigation Menu

Use rules_nodejs toolchain for hermetic cdxgen execution - #4

Use rules_nodejs toolchain for hermetic cdxgen execution#4
AlexanderLanin merged 4 commits into
eclipse-score:mainfrom
etas-contrib:hermetic

olivembo commented Apr 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

olivembo Aug 21, 2026

Uh oh!

olivembo Aug 21, 2026 •

edited

Loading

Uh oh!

masc2023 commented Aug 20, 2026

Uh oh!

olivembo commented Aug 21, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

olivembo Aug 21, 2026 •

edited

Loading

Uh oh!

olivembo Aug 21, 2026

Uh oh!

olivembo Aug 21, 2026 •

edited

Loading

Uh oh!

AlexanderLanin left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

olivembo commented Apr 7, 2026

Summary

What changed

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

olivembo Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

olivembo Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

masc2023 commented Aug 20, 2026

Uh oh!

olivembo commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

olivembo Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

olivembo Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

olivembo Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexanderLanin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

olivembo Aug 21, 2026 •

edited

Loading

olivembo commented Aug 21, 2026 •

edited

Loading

olivembo Aug 21, 2026 •

edited

Loading

olivembo Aug 21, 2026 •

edited

Loading