-
Notifications
You must be signed in to change notification settings - Fork 0
release: fail the SBOM job unless every declared runtime dependency is enumerated #112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -194,7 +194,8 @@ sudo install -m 0755 "$WORKDIR/syft" /usr/local/bin/syft | |
| syft version | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # 4. Generate SPDX JSON for the extracted (published) tarball contents. | ||
| # 4. Generate SPDX JSON for the INSTALLED tree: the extracted (published) | ||
| # tarball contents plus the node_modules that `npm ci` resolved in step 2. | ||
| # --------------------------------------------------------------------------- | ||
| SPDX_RAW="$WORKDIR/raw.spdx.json" | ||
| syft scan "dir:${PKG_DIR}" --source-name "$PKG_NAME" --source-version "$PKG_VERSION" -o "spdx-json=${SPDX_RAW}" | ||
|
|
@@ -206,6 +207,25 @@ if [[ "$PKG_COUNT" -lt 1 ]]; then | |
| fi | ||
| echo "SPDX packages[] count: $PKG_COUNT" | ||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # 4b. Fail closed unless the SBOM actually enumerates what ships. The count | ||
| # check above is necessary but not sufficient: an SBOM generated from a | ||
| # bare, never-installed tarball still has ONE entry (the package's own | ||
| # package.json), so "packages[] >= 1" would wave a dependency-less | ||
| # document through. validate-sbom.mjs (a sibling of this script, so it is | ||
| # always present on the workflow's own ref even when backfilling a tag | ||
| # that predates it) requires a versioned entry for $PKG_NAME@$PKG_VERSION | ||
| # AND for every key of the PUBLISHED package.json's `dependencies`, and | ||
| # lists the missing names on failure. The floor is derived from that | ||
| # manifest, never hardcoded, so it tracks dependency changes on its own. | ||
| # --------------------------------------------------------------------------- | ||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| node "$SCRIPT_DIR/validate-sbom.mjs" \ | ||
| --sbom "$SPDX_RAW" \ | ||
| --manifest "$PKG_DIR/package.json" \ | ||
| --name "$PKG_NAME" \ | ||
| --version "$PKG_VERSION" | ||
|
Comment on lines
+223
to
+227
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion: A valid dependency-free package always fails this workflow because the caller never passes Assessment: π Prompt for AI Agent π€This is a comment left during a code review.
**Path:** scripts/sbom/generate-sbom.sh
**Line:** 223:227
**Comment:**
*Api Mismatch: A valid dependency-free package always fails this workflow because the caller never passes `--allow-no-dependencies`, making backfills impossible for such releases.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix |
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # 5. Stage the tarball + SBOM into OUT_DIR for the caller to upload as a | ||
| # workflow artifact (the `release` job attaches both to the GitHub | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,146 @@ | ||||||||||||||||||||||||||
| #!/usr/bin/env node | ||||||||||||||||||||||||||
| // validate-sbom.mjs -- fail loud unless an SPDX SBOM enumerates this package AND | ||||||||||||||||||||||||||
| // every runtime dependency the published manifest declares. | ||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||
| // Why this exists: the release SBOM is generated from the INSTALLED dependency | ||||||||||||||||||||||||||
| // tree (npm ci --omit=dev against the tag's lockfile) so it describes what a | ||||||||||||||||||||||||||
| // consumer's `npm install` actually pulls in. The check this replaces only | ||||||||||||||||||||||||||
| // asserted `packages[]` was non-empty -- which an SBOM produced from a bare, | ||||||||||||||||||||||||||
| // never-installed tarball satisfies with a single self-describing entry and | ||||||||||||||||||||||||||
| // zero dependencies. That is the defect: an SBOM that predates install is a | ||||||||||||||||||||||||||
| // claim, not an inventory. This script derives its floor from the published | ||||||||||||||||||||||||||
| // package.json itself (the `dependencies` keys), so it tracks the manifest as | ||||||||||||||||||||||||||
| // dependencies are added or removed and never needs a hardcoded number. | ||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||
| // Plain Node stdlib only (no install, no network) so it can be run locally | ||||||||||||||||||||||||||
| // against any SPDX document + package.json pair without pushing a tag: | ||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||
| // node scripts/sbom/validate-sbom.mjs --sbom out.spdx.json \ | ||||||||||||||||||||||||||
| // --manifest package/package.json --name @scope/pkg --version 1.2.3 | ||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||
| // Exit 0 on pass, 1 with a `::error::` line on failure, 2 on unusable inputs. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| import { readFileSync } from 'node:fs'; | ||||||||||||||||||||||||||
| import { resolve } from 'node:path'; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const NO_VERSION = new Set(['', 'NOASSERTION']); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export function parseArgs(argv) { | ||||||||||||||||||||||||||
| const out = { allowNoDependencies: false }; | ||||||||||||||||||||||||||
| for (let i = 0; i < argv.length; i += 1) { | ||||||||||||||||||||||||||
| const arg = argv[i]; | ||||||||||||||||||||||||||
| if (arg === '--allow-no-dependencies') { out.allowNoDependencies = true; continue; } | ||||||||||||||||||||||||||
| const key = { '--sbom': 'sbom', '--manifest': 'manifest', '--name': 'name', '--version': 'version' }[arg]; | ||||||||||||||||||||||||||
| if (!key) throw new UsageError(`unknown argument: ${arg}`); | ||||||||||||||||||||||||||
| const value = argv[i + 1]; | ||||||||||||||||||||||||||
| if (value === undefined || value.startsWith('--')) throw new UsageError(`${arg} needs a value`); | ||||||||||||||||||||||||||
| out[key] = value; | ||||||||||||||||||||||||||
| i += 1; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| for (const required of ['sbom', 'manifest', 'name', 'version']) { | ||||||||||||||||||||||||||
| if (!out[required]) throw new UsageError(`--${required} is required`); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+28
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. π‘ Quality: New validator ships with no unit tests despite being designed for them
Fix: Was this helpful? React with π / π |
||||||||||||||||||||||||||
| return out; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export class UsageError extends Error {} | ||||||||||||||||||||||||||
| export class ValidationError extends Error {} | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** Names of the runtime dependencies the published manifest declares. */ | ||||||||||||||||||||||||||
| export function declaredDependencies(manifest) { | ||||||||||||||||||||||||||
| const deps = manifest && typeof manifest.dependencies === 'object' && manifest.dependencies !== null | ||||||||||||||||||||||||||
| ? manifest.dependencies | ||||||||||||||||||||||||||
| : {}; | ||||||||||||||||||||||||||
| return Object.keys(deps).sort(); | ||||||||||||||||||||||||||
|
Comment on lines
+51
to
+54
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: When Prompt for AI agents
Suggested change
|
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** name -> Set(versions) for every SPDX packages[] entry that carries a real version. */ | ||||||||||||||||||||||||||
| export function versionedPackages(spdx) { | ||||||||||||||||||||||||||
| const found = new Map(); | ||||||||||||||||||||||||||
| for (const pkg of Array.isArray(spdx?.packages) ? spdx.packages : []) { | ||||||||||||||||||||||||||
| const name = typeof pkg?.name === 'string' ? pkg.name : ''; | ||||||||||||||||||||||||||
| const version = typeof pkg?.versionInfo === 'string' ? pkg.versionInfo : ''; | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: A whitespace-only Prompt for AI agents
Suggested change
|
||||||||||||||||||||||||||
| if (!name || NO_VERSION.has(version)) continue; | ||||||||||||||||||||||||||
| if (!found.has(name)) found.set(name, new Set()); | ||||||||||||||||||||||||||
| found.get(name).add(version); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| return found; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||
| * Validate that `spdx` enumerates `name@version` and every dependency `manifest` declares. | ||||||||||||||||||||||||||
| * Returns a one-line summary on success; throws ValidationError otherwise. | ||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||
| export function validate({ spdx, manifest, name, version, allowNoDependencies = false }) { | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Add Prompt for AI agents |
||||||||||||||||||||||||||
| const packages = Array.isArray(spdx?.packages) ? spdx.packages : []; | ||||||||||||||||||||||||||
| if (packages.length === 0) throw new ValidationError('SPDX document has an empty packages[] array'); | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const declared = declaredDependencies(manifest); | ||||||||||||||||||||||||||
| if (declared.length === 0 && !allowNoDependencies) { | ||||||||||||||||||||||||||
| throw new ValidationError( | ||||||||||||||||||||||||||
| `${name} declares no runtime dependencies in its published package.json -- refusing to validate ` + | ||||||||||||||||||||||||||
| 'against a floor of 0 (pass --allow-no-dependencies if that is genuinely intended)', | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const versioned = versionedPackages(spdx); | ||||||||||||||||||||||||||
| const rootVersions = versioned.get(name); | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Bind the expected root identity to Prompt for AI agents |
||||||||||||||||||||||||||
| if (!rootVersions || !rootVersions.has(version)) { | ||||||||||||||||||||||||||
| const seen = rootVersions ? [...rootVersions].join(', ') : 'none'; | ||||||||||||||||||||||||||
| throw new ValidationError(`no versioned entry for the package itself, ${name}@${version} (versions seen: ${seen})`); | ||||||||||||||||||||||||||
|
Comment on lines
+87
to
+90
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ποΈ Data Integrity & Integration | π Major | β‘ Quick win Bind root-package validation to the published manifest.
π Affects 2 files
π€ Prompt for AI Agents |
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const missing = declared.filter((dep) => !versioned.has(dep)); | ||||||||||||||||||||||||||
| const floor = 1 + declared.length; | ||||||||||||||||||||||||||
| const summary = | ||||||||||||||||||||||||||
| `${packages.length} packages[], ${versioned.size} distinct with a real versionInfo; ` + | ||||||||||||||||||||||||||
| `floor ${floor} (${name} + ${declared.length} declared runtime deps${declared.length ? `: ${declared.join(', ')}` : ''})`; | ||||||||||||||||||||||||||
| if (missing.length > 0) { | ||||||||||||||||||||||||||
| throw new ValidationError( | ||||||||||||||||||||||||||
| `${summary}\n::error::sbom: no versioned entry for declared runtime dependency(ies): ${missing.join(', ')} ` + | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: Prompt for AI agents |
||||||||||||||||||||||||||
| '-- syft likely scanned an uninstalled tree', | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if (versioned.size < floor) { | ||||||||||||||||||||||||||
| throw new ValidationError(`${summary}\n::error::sbom: only ${versioned.size} versioned package(s), below the floor of ${floor}`); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| return summary; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function readJson(file) { | ||||||||||||||||||||||||||
| const path = resolve(file); | ||||||||||||||||||||||||||
| let text; | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| text = readFileSync(path, 'utf8'); | ||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||
| throw new UsageError(`cannot read ${path}: ${err.message}`); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| if (text.length === 0) throw new UsageError(`${path} is empty`); | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| return JSON.parse(text); | ||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||
| throw new UsageError(`${path} is not valid JSON: ${err.message}`); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function main(argv) { | ||||||||||||||||||||||||||
| const args = parseArgs(argv); | ||||||||||||||||||||||||||
| const summary = validate({ | ||||||||||||||||||||||||||
| spdx: readJson(args.sbom), | ||||||||||||||||||||||||||
| manifest: readJson(args.manifest), | ||||||||||||||||||||||||||
| name: args.name, | ||||||||||||||||||||||||||
| version: args.version, | ||||||||||||||||||||||||||
| allowNoDependencies: args.allowNoDependencies, | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| process.stdout.write(`${summary}\nSBOM OK: ${args.name}@${args.version} and every declared runtime dependency are enumerated\n`); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) { | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ποΈ Data Integrity & Integration | π Major | β‘ Quick win π Supported by static analysisπ Script executed: #!/usr/bin/env bash
set -euo pipefail
tmp="$(mktemp -d)"
mkdir -p "$tmp/path with space"
cat > "$tmp/path with space/guard.mjs" <<'EOF'
if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) {
process.stdout.write('main ran\n')
}
EOF
output="$(node "$tmp/path with space/guard.mjs")"
test "$output" != "main ran"
echo "Current guard skips execution for an encoded path."Repository: wave-av/adk Length of output: 672 π Script executed: #!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- scripts/sbom/validate-sbom.mjs ---'
nl -ba scripts/sbom/validate-sbom.mjs | sed -n '1,180p'
printf '%s\n' '--- scripts/sbom/generate-sbom.sh ---'
nl -ba scripts/sbom/generate-sbom.sh | sed -n '1,220p'
printf '%s\n' '--- related invocations ---'
rg -n -C 3 'validate-sbom|generate-sbom' scripts package.json .github 2>/dev/null || trueRepository: wave-av/adk Length of output: 327 π Script executed: node - <<'JS'
const { pathToFileURL } = require('node:url');
const { resolve } = require('node:path');
for (const p of [
'/tmp/path with space/guard.mjs',
'/tmp/path#fragment/guard.mjs',
'/tmp/path%value/guard.mjs',
]) {
console.log(JSON.stringify({
path: p,
raw: `file://${p}`,
normalized: pathToFileURL(resolve(p)).href,
}));
}
JSRepository: wave-av/adk Length of output: 327 Normalize the entrypoint path before comparing it to If the script path contains a space, Proposed fix import { resolve } from 'node:path';
+import { pathToFileURL } from 'node:url';
-if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) {
+if (process.argv[1] && import.meta.url === pathToFileURL(resolve(process.argv[1])).href) {π Committable suggestion
Suggested change
π§° Toolsπͺ ESLint[error] 138-138: 'process' is not defined. (no-undef) [error] 138-138: 'process' is not defined. (no-undef) π€ Prompt for AI AgentsThere was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Normalize the entrypoint path before comparing it with Prompt for AI agents |
||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| main(process.argv.slice(2)); | ||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||
| const code = err instanceof UsageError ? 2 : 1; | ||||||||||||||||||||||||||
| process.stderr.write(`::error::sbom: ${err.message}\n`); | ||||||||||||||||||||||||||
| process.exit(code); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: When the published package has zero runtime dependencies, this release entrypoint always exits 1 before producing an SBOM because it cannot pass
--allow-no-dependencies. Expose an explicit generator option and append that validator flag only when enabled, so valid zero-dependency releases can opt in without weakening the default fail-closed behavior.Prompt for AI agents