docs(security): document the bundled Node.js runtime the SBOM misses - #584
Conversation
SECURITY.md already scoped the SBOM to the dependency closure and named the runtime gap, but never said which Node.js is inside the artefacts or where it comes from. A reader could learn the gap exists and still not answer the question the gap raises. Adds a hand-maintained component entry under the SBOM section: version 24.18.0 (Krypton LTS), the nodejs.org dist directory it is fetched from, the three exact artefacts (linux-x64, linux-arm64, win-x64), the two scripts that fetch them, sha256 pinned in-repo, the licence, and which packaged artefacts ship it and which do not. No script changes. It also records why the digests are pinned in-repo rather than read from the SHASUMS256.txt served alongside the download, since that is the reason the pin is duplicated in two places and looks redundant. tests/unit/bundled-node-runtime-docs.test.ts keeps the entry from going stale: it reads NODE_VERSION out of both scripts, requires them to agree, and requires the document to name that version, its upstream dist directory, all three artefact filenames and both scripts. Two more assertions guard the surrounding claims: the SBOM-does-not-cover-it sentence must survive, so the document cannot end up with two contradictory statements, and the pin must stay at or above package.json's engines.node floor, which the scripts ask for and nothing checked. Verified the guard bites: bumping NODE_VERSION in one script without touching SECURITY.md fails four of the seven tests. Closes libredb#545
|
Thanks, this is a good first PR. Two small things in what you already wrote:
And one that is yours to take or leave. Your entry meets the done-when on
The narrower thing that stays open, making the SBOM itself carry the component, I will file as its own entry. That one is my claim to write, not yours. Happy to do the C7 part myself if you would rather stop at the two fixes above. |
…#603) #584 closes C7 by giving SECURITY.md a hand-maintained "Bundled Node.js runtime" table, which is exactly what C7's Done-when accepted: a sibling document, or a hand-maintained component entry. The residual is the half a scanner reads. The CycloneDX document the release job generates still has no component for the runtime, and its own verify step names the three ecosystems it expects, none of which is a shell script that curls a tarball. Filed as its own entry rather than folded back into C7, because narrowing a Done-when after the work that satisfies it has landed moves a pre-committed acceptance. C7 is the contributor's to delete on #584; this is the claim that outlives it. The index line moves with it, C3-C10 7 becomes C3-C11 8, and both halves are measured against tests/unit/backlog-structure.test.ts: leaving the count at 7 fails "counts its entries", leaving the extreme at C10 fails "names the real extremes".
|
Heads up before you push the review fixes: Your checks are green, but they started at 13:05 +03, an hour before that tag, so they were measured against a main that no longer exists. Refreshing makes the green mean something again. Please take current main before the next round. From a fork: Why now rather than at merge time: the chart version sync guard is the first step of |
Three things from the review. The "SECURITY.md names the pinned version" test could not fail on a missing heading: indexOf returns -1, slice(-1) is a one-character string, and `length > 0` holds. Assert on the index, then read the section from it. SECURITY.md quotes the engines.node floor as >=24.0.0 in prose, and nothing guarded that literal. The floor test already reads engines.node out of package.json, so one more assertion keeps the prose from going stale the day the floor moves. This entry meets the done-when on BACKLOG C7 (a hand-maintained component entry), and that file retires an entry by deleting it rather than marking it DONE. So C7 goes, the section index drops to C3-C10 · 6 with C3 and C10 still the extremes, and SECURITY.md no longer points at a backlog entry that is not there. The narrower gap -- making the SBOM itself carry the component -- stays open as the maintainer's own entry to write.
…time # Conflicts: # docs/BACKLOG.md
|
Both fixes are in, plus the C7 retirement, and the branch now carries current 1. The heading check that could not fail. You were right that const heading = SECURITY.indexOf("#### Bundled Node.js runtime");
expect(heading, "SECURITY.md has no bundled-runtime section").not.toBe(-1);
expect(SECURITY.slice(heading)).toContain(version);With the heading renamed that fails on the check that means what it says. 2. The unguarded expect(SECURITY, `SECURITY.md does not quote the engines.node floor ${engines}`).toContain(engines);3. C7. Taken — thanks for pointing at the file's rule. The block is gone from One thing changed since you wrote that: The narrower gap, making the SBOM itself carry the component, is untouched and yours to file. 4. Refreshed against main. Merged Local run of the two guard suites: 100 pass, 0 fail. (First run, one test timed out at the 5s default while it walked the repo cold; it passes well inside the limit once the cache is warm.) |
cevheri
left a comment
There was a problem hiding this comment.
Both fixes are in, C7 is retired, and I ran each one against a mutation rather than reading the diff. Baseline on your head: 7 pass in the new guard, 93 in backlog-structure, the 100 you reported.
| Mutation | Result |
|---|---|
rename the #### Bundled Node.js runtime heading |
1 fail, SECURITY.md has no bundled-runtime section |
raise engines.node to >=26.0.0 |
1 fail, toBeGreaterThanOrEqual |
lower engines.node to >=22.0.0 |
1 fail, SECURITY.md does not quote the engines.node floor >=22.0.0 |
bump NODE_VERSION in the linux script alone |
4 fail |
| soften the SBOM-gap sentence | 1 fail |
The first row is the fix I asked for, and it now fails on the check that means what it says. The third row is the one worth keeping in mind. Raising the floor is the obvious probe, but it dies at the major-version comparison two lines earlier, so it never reaches your new assertion — it would pass identically if that line were not there. Lowering the floor keeps the comparison satisfied and leaves only the prose to fail. Two assertions in one test can shadow each other; a probe has to be aimed at the one you are trying to prove.
C7: block gone, index line correct, and the pointer dropped from the paragraph. You were right that main had moved it to C3–C11 · 7 — reverting the count to 6 fails "Security Phase 2 deferrals counts its entries", so it is guarded either way. C11 is the narrower entry, filed in #603.
I pushed one commit to your branch, fa419b7f. The Check formatting (Biome) step wanted your floor assertion on a single line: joined it is 118 characters, and this repo's lineWidth is 120, so Biome will not accept the wrapped form. Nothing about the code, purely the formatter's opinion. Worth knowing because that step is the ninth of seventeen in Lint, Typecheck and Build and the job stops there — lint, typecheck, knip, build, build:lib, attw and the Go launcher tests all came back skipped, so a single wrapped line reads as eight gates unmeasured. bun run format:fix before you push is the whole fix; CLAUDE.md lists it first in the pre-commit set for that reason. I ran the other seven guards on your head locally and they all pass.
Good work on this one. You took a documentation issue and left behind a guard that fails for the right reason, which is the harder half.
Closes #545. Docs plus one test file; no script changes, as the issue specifies.
SECURITY.mdalready scoped the SBOM to the dependency closure and named the runtime gap honestly. What it did not do is answer the question that gap raises: which Node.js is inside, and where did the bytes come from. A reader could learn a gap exists and still be unable to act on it.The new Bundled Node.js runtime entry sits directly under that paragraph and records: version 24.18.0 (Krypton LTS), the
nodejs.org/dist/v24.18.0/directory it is fetched from, the three exact artefacts (linux-x64.tar.xz,linux-arm64.tar.xz,win-x64.zip), both fetch scripts, sha256 pinned in-repo and verified after download, the licence, and which packaged artefacts ship it — including that the npm package does not, because it uses the host's Node.I also recorded why the digests are pinned in-repo rather than read from the
SHASUMS256.txtserved next to the download. That is the reason the pin is duplicated across two scripts and looks redundant at a glance, and it is exactly the kind of thing a security reader wants stated rather than inferred.The entry is hand-maintained, so it needs a guard.
tests/unit/bundled-node-runtime-docs.test.tsreadsNODE_VERSIONout of both scripts, requires them to agree, and requires the document to name that version, its upstream dist directory, all three artefact filenames, and both scripts by path.Two further assertions guard the surrounding claims rather than the table:
package.json'sengines.nodefloor. Both scripts already instruct this in comments and nothing checked it.Verified the guard bites: bumping
NODE_VERSIONto25.1.0inpackaging/linux/fetch-node.shalone fails 4 of the 7 tests — the two-scripts-agree check plus the version, dist-URL and artefact-name checks.7 pass, 0 fail on the real tree.