Skip to content

Commit c46fb91

Browse files
authored
fix: wrong coordinates, npm v7+ lockfile & audit compatibility, SPDX evaluation & license additions (AGPL-3.0-only, Python-2.0, Ruby, Sleepycat, UPL-1.0, Vim, W3C, X11, Zlib) (#253)
* Fix: wrong package coordinates sent to NCM API # Fix: wrong package coordinates sent to NCM API ## Summary | # | Report | Status | Where | |---|---|---|---| | 1 | "sending odd packages versions to the API like this one jwt version 0.0.0" | Fixed | `lib/ncm-analyze-tree.js`, `commands/report.js` | | 2 | `NCM_TOKEN=… NCM_API=… ncm` "with no success" | Not a bug; documented below | — | | 3 | "still a diff on npm audit and ncm report" | Fixed (root cause shared with #1) | `lib/ncm-analyze-tree.js` | | 4 | "we would need to investigate if our vulnDB has all the vulns" | Now meaningfully testable | see below | Tests: **24 / 24** pass (`npm run test-only`). --- ## Root cause (issues 1 & 3) `lib/ncm-analyze-tree.js` had been rewritten to use [`dependency-tree`](https://www.npmjs.com/package/dependency-tree) — a **source-code import analyzer** — instead of [`universal-module-tree`](https://www.npmjs.com/package/universal-module-tree), which walks the real npm dependency graph (`package-lock.json` → `yarn.lock` → `node_modules`). Three concrete defects in that code produced the payloads the user saw: 1. **Filename-as-package-name with placeholder version.** `dependency-tree` returns resolved file paths. The replacement code turned each path into a "package": ```js const name = path.basename(dep, path.extname(dep)); const childNode = { data: { name, version: '0.0.0' }, // hard-coded children: [] }; ``` So `jsonwebtoken.js` → `jwt @ 0.0.0`, `index.js` → `index @ 0.0.0`, and similar. That is the exact `jwt version 0.0.0` entry the user saw going out. 2. **Range-string fallback also produced `0.0.0`.** The auxiliary `getNpmDependencies` helper only read direct deps from `package.json` and did: ```js const cleanVersion = version.replace(/[^\d.]/g, '') || version; ``` For `"latest"`, `"file:…"`, `"git+https://…"`, `"workspace:*"`, etc. the strip leaves `''` and the `||` fallback is still non-semver — so those deps were emitted as `name @ 0.0.0` too. 3. **Missing transitive deps.** `dependency-tree` only surfaces files the entry point actually `require()`s, and the `package.json` fallback only listed direct dependencies. The full transitive graph was never walked. This is the direct mechanical cause of the diff vs `npm audit`. 4. **`commands/report.js` had been patched to hide the fallout.** Any `null` / `undefined` `version` returned by the NCM service was coerced to `'0.0.0'` and still included in the rendered report, so bogus rows showed up in the UI too. ## Fix ### `lib/ncm-analyze-tree.js` — rewritten (550 → 131 lines) - Reverted to `universal-module-tree` (already in `dependencies`, and the same library `commands/details.js` uses). - Removed, in full: - the `dependency-tree` invocation and entry-point discovery logic (guessing `index.js`, `app.js`, `bin/*.js`, …), - filename-as-package-name logic, - `getNpmDependencies` and the duplicate `readPackagesFromPackageJson` that stripped `^` / `~` and fell back to `'0.0.0'`, - dead counters. The new file gets a tree from `universalModuleTree(dir)`, walks it keeping dedup + `paths` info, and POSTs `{ name, version }` pairs to the GraphQL endpoint — same shape that already worked in `commands/details.js`. ### `commands/report.js` - Removed the `effectiveVersion = '0.0.0'` coercion. Packages the NCM service returns with `version === null` (unpublished / unknown) are now **skipped** instead of rendered as `@ 0.0.0`: ```js if (version === null || version === undefined) continue; ``` - Removed unused `includedCount` / `skippedCount` counters and the dead `getLicenseScore` helper. ### `package.json` / `package-lock.json` - Removed the unused `dependency-tree` dependency (`npm uninstall dependency-tree`). `universal-module-tree` was already listed and is now the single source of truth for the dependency graph. --- ## Verification ### Empirical: correct coordinates go out ``` $ node -e "const umt=require('universal-module-tree'); (async()=>{ const t=await umt('./test/fixtures/mock-project'); const f=umt.flatten(t); console.log('total:', f.length); console.log('zero-versions:', f.filter(p=>p.version==='0.0.0')); console.log('filename-like:', f.filter(p=>/\\.|\\//.test(p.name))); })()" total: 37 zero-versions: [] filename-like: [] ``` No `@ 0.0.0`, no `jwt`-style filename names — just real installed packages from `package-lock.json`. ### Tests: `npm run test-only` → 24 / 24 pass Notable guards: - `report › report output matches snapshot` — asserts **"36 packages checked"**, which only holds when the transitive graph is walked correctly. - `report › report with poisoned project` — asserts a mock package returned with `version: null` is **skipped**, not rendered as `@ 0.0.0`. - All `--filter=*`, `--compliance`, `--security`, `--long` snapshot tests pass, plus `details`, `whitelist`, `install`, `github-actions`, and `proxied` suites. --- ## Issue 2: bare `ncm` "with no success" `ncm` with no subcommand shows help and exits 0 — same as `git` or `npm` without args. `bin/ncm-cli.js`: ```js let [command = 'help', ...subargs] = argv._ if (!Object.keys(commands).includes(command)) command = 'help' ``` No API request is ever made, which is why the debug session looked like "nothing happening". Verified post-fix: `node bin/ncm-cli.js` prints help and exits 0. To actually exercise the analyzer against the local API: ```sh NCM_TOKEN=6c044113-c485-40cb-915b-cbc9d3a26730 \ NCM_API=http://localhost:3000 \ ncm report --dir=/path/to/some/project ``` --- ## Issue 4: vulnDB coverage investigation Previously impossible to do meaningfully — the payload was garbage (issue 1) and incomplete (issue 3). Now that real coordinates for the full transitive tree are sent, a direct diff is valid: ```sh cd /path/to/project npm install # ensure lockfile + node_modules npm audit --json > /tmp/npm-audit.json # npm's view NCM_TOKEN=… NCM_API=http://localhost:3000 \ ncm report --long --json > /tmp/ncm-report.json # ncm's view ``` Compare the `{name, version}` sets: - advisories in `npm-audit` but not `ncm-report` → real gaps in the vulnDB, feed back to the data team; - advisories in `ncm-report` but not `npm-audit` → extra NCM coverage (or a false positive to review). --- ## Files changed - `lib/ncm-analyze-tree.js` — rewritten (550 → 131 lines). - `commands/report.js` — `0.0.0` coercion and dead code removed. - `package.json`, `package-lock.json` — `dependency-tree` removed. * Fix: wrong package coordinates sent to NCM API # Fix: wrong package coordinates sent to NCM API ## Summary | # | Report | Status | Where | |---|---|---|---| | 1 | "sending odd packages versions to the API like this one jwt version 0.0.0" | Fixed | `lib/ncm-analyze-tree.js`, `commands/report.js` | | 2 | `NCM_TOKEN=… NCM_API=… ncm` "with no success" | Not a bug; documented below | — | | 3 | "still a diff on npm audit and ncm report" | Fixed (root cause shared with #1) | `lib/ncm-analyze-tree.js` | | 4 | "we would need to investigate if our vulnDB has all the vulns" | Now meaningfully testable | see below | Tests: **24 / 24** pass (`npm run test-only`). --- ## Root cause (issues 1 & 3) `lib/ncm-analyze-tree.js` had been rewritten to use [`dependency-tree`](https://www.npmjs.com/package/dependency-tree) — a **source-code import analyzer** — instead of [`universal-module-tree`](https://www.npmjs.com/package/universal-module-tree), which walks the real npm dependency graph (`package-lock.json` → `yarn.lock` → `node_modules`). Three concrete defects in that code produced the payloads the user saw: 1. **Filename-as-package-name with placeholder version.** `dependency-tree` returns resolved file paths. The replacement code turned each path into a "package": ```js const name = path.basename(dep, path.extname(dep)); const childNode = { data: { name, version: '0.0.0' }, // hard-coded children: [] }; ``` So `jsonwebtoken.js` → `jwt @ 0.0.0`, `index.js` → `index @ 0.0.0`, and similar. That is the exact `jwt version 0.0.0` entry the user saw going out. 2. **Range-string fallback also produced `0.0.0`.** The auxiliary `getNpmDependencies` helper only read direct deps from `package.json` and did: ```js const cleanVersion = version.replace(/[^\d.]/g, '') || version; ``` For `"latest"`, `"file:…"`, `"git+https://…"`, `"workspace:*"`, etc. the strip leaves `''` and the `||` fallback is still non-semver — so those deps were emitted as `name @ 0.0.0` too. 3. **Missing transitive deps.** `dependency-tree` only surfaces files the entry point actually `require()`s, and the `package.json` fallback only listed direct dependencies. The full transitive graph was never walked. This is the direct mechanical cause of the diff vs `npm audit`. 4. **`commands/report.js` had been patched to hide the fallout.** Any `null` / `undefined` `version` returned by the NCM service was coerced to `'0.0.0'` and still included in the rendered report, so bogus rows showed up in the UI too. ## Fix ### `lib/ncm-analyze-tree.js` — rewritten (550 → 131 lines) - Reverted to `universal-module-tree` (already in `dependencies`, and the same library `commands/details.js` uses). - Removed, in full: - the `dependency-tree` invocation and entry-point discovery logic (guessing `index.js`, `app.js`, `bin/*.js`, …), - filename-as-package-name logic, - `getNpmDependencies` and the duplicate `readPackagesFromPackageJson` that stripped `^` / `~` and fell back to `'0.0.0'`, - dead counters. The new file gets a tree from `universalModuleTree(dir)`, walks it keeping dedup + `paths` info, and POSTs `{ name, version }` pairs to the GraphQL endpoint — same shape that already worked in `commands/details.js`. ### `commands/report.js` - Removed the `effectiveVersion = '0.0.0'` coercion. Packages the NCM service returns with `version === null` (unpublished / unknown) are now **skipped** instead of rendered as `@ 0.0.0`: ```js if (version === null || version === undefined) continue; ``` - Removed unused `includedCount` / `skippedCount` counters and the dead `getLicenseScore` helper. ### `package.json` / `package-lock.json` - Removed the unused `dependency-tree` dependency (`npm uninstall dependency-tree`). `universal-module-tree` was already listed and is now the single source of truth for the dependency graph. --- ## Verification ### Empirical: correct coordinates go out ``` $ node -e "const umt=require('universal-module-tree'); (async()=>{ const t=await umt('./test/fixtures/mock-project'); const f=umt.flatten(t); console.log('total:', f.length); console.log('zero-versions:', f.filter(p=>p.version==='0.0.0')); console.log('filename-like:', f.filter(p=>/\\.|\\//.test(p.name))); })()" total: 37 zero-versions: [] filename-like: [] ``` No `@ 0.0.0`, no `jwt`-style filename names — just real installed packages from `package-lock.json`. ### Tests: `npm run test-only` → 24 / 24 pass Notable guards: - `report › report output matches snapshot` — asserts **"36 packages checked"**, which only holds when the transitive graph is walked correctly. - `report › report with poisoned project` — asserts a mock package returned with `version: null` is **skipped**, not rendered as `@ 0.0.0`. - All `--filter=*`, `--compliance`, `--security`, `--long` snapshot tests pass, plus `details`, `whitelist`, `install`, `github-actions`, and `proxied` suites. --- ## Issue 2: bare `ncm` "with no success" `ncm` with no subcommand shows help and exits 0 — same as `git` or `npm` without args. `bin/ncm-cli.js`: ```js let [command = 'help', ...subargs] = argv._ if (!Object.keys(commands).includes(command)) command = 'help' ``` No API request is ever made, which is why the debug session looked like "nothing happening". Verified post-fix: `node bin/ncm-cli.js` prints help and exits 0. To actually exercise the analyzer against the local API: ```sh NCM_TOKEN=6c044113-c485-40cb-915b-cbc9d3a26730 \ NCM_API=http://localhost:3000 \ ncm report --dir=/path/to/some/project ``` --- ## Issue 4: vulnDB coverage investigation Previously impossible to do meaningfully — the payload was garbage (issue 1) and incomplete (issue 3). Now that real coordinates for the full transitive tree are sent, a direct diff is valid: ```sh cd /path/to/project npm install # ensure lockfile + node_modules npm audit --json > /tmp/npm-audit.json # npm's view NCM_TOKEN=… NCM_API=http://localhost:3000 \ ncm report --long --json > /tmp/ncm-report.json # ncm's view ``` Compare the `{name, version}` sets: - advisories in `npm-audit` but not `ncm-report` → real gaps in the vulnDB, feed back to the data team; - advisories in `ncm-report` but not `npm-audit` → extra NCM coverage (or a false positive to review). --- ## Files changed - `lib/ncm-analyze-tree.js` — rewritten (550 → 131 lines). - `commands/report.js` — `0.0.0` coercion and dead code removed. - `package.json`, `package-lock.json` — `dependency-tree` removed. * Fix: wrong package coordinates sent to NCM API # Fix: wrong package coordinates sent to NCM API ## Summary | # | Report | Status | Where | |---|---|---|---| | 1 | "sending odd packages versions to the API like this one jwt version 0.0.0" | Fixed | `lib/ncm-analyze-tree.js`, `commands/report.js` | | 2 | `NCM_TOKEN=… NCM_API=… ncm` "with no success" | Not a bug; documented below | — | | 3 | "still a diff on npm audit and ncm report" | Fixed (root cause shared with #1) | `lib/ncm-analyze-tree.js` | | 4 | "we would need to investigate if our vulnDB has all the vulns" | Now meaningfully testable | see below | Tests: **24 / 24** pass (`npm run test-only`). --- ## Root cause (issues 1 & 3) `lib/ncm-analyze-tree.js` had been rewritten to use [`dependency-tree`](https://www.npmjs.com/package/dependency-tree) — a **source-code import analyzer** — instead of [`universal-module-tree`](https://www.npmjs.com/package/universal-module-tree), which walks the real npm dependency graph (`package-lock.json` → `yarn.lock` → `node_modules`). Three concrete defects in that code produced the payloads the user saw: 1. **Filename-as-package-name with placeholder version.** `dependency-tree` returns resolved file paths. The replacement code turned each path into a "package": ```js const name = path.basename(dep, path.extname(dep)); const childNode = { data: { name, version: '0.0.0' }, // hard-coded children: [] }; ``` So `jsonwebtoken.js` → `jwt @ 0.0.0`, `index.js` → `index @ 0.0.0`, and similar. That is the exact `jwt version 0.0.0` entry the user saw going out. 2. **Range-string fallback also produced `0.0.0`.** The auxiliary `getNpmDependencies` helper only read direct deps from `package.json` and did: ```js const cleanVersion = version.replace(/[^\d.]/g, '') || version; ``` For `"latest"`, `"file:…"`, `"git+https://…"`, `"workspace:*"`, etc. the strip leaves `''` and the `||` fallback is still non-semver — so those deps were emitted as `name @ 0.0.0` too. 3. **Missing transitive deps.** `dependency-tree` only surfaces files the entry point actually `require()`s, and the `package.json` fallback only listed direct dependencies. The full transitive graph was never walked. This is the direct mechanical cause of the diff vs `npm audit`. 4. **`commands/report.js` had been patched to hide the fallout.** Any `null` / `undefined` `version` returned by the NCM service was coerced to `'0.0.0'` and still included in the rendered report, so bogus rows showed up in the UI too. ## Fix ### `lib/ncm-analyze-tree.js` — rewritten (550 → 131 lines) - Reverted to `universal-module-tree` (already in `dependencies`, and the same library `commands/details.js` uses). - Removed, in full: - the `dependency-tree` invocation and entry-point discovery logic (guessing `index.js`, `app.js`, `bin/*.js`, …), - filename-as-package-name logic, - `getNpmDependencies` and the duplicate `readPackagesFromPackageJson` that stripped `^` / `~` and fell back to `'0.0.0'`, - dead counters. The new file gets a tree from `universalModuleTree(dir)`, walks it keeping dedup + `paths` info, and POSTs `{ name, version }` pairs to the GraphQL endpoint — same shape that already worked in `commands/details.js`. ### `commands/report.js` - Removed the `effectiveVersion = '0.0.0'` coercion. Packages the NCM service returns with `version === null` (unpublished / unknown) are now **skipped** instead of rendered as `@ 0.0.0`: ```js if (version === null || version === undefined) continue; ``` - Removed unused `includedCount` / `skippedCount` counters and the dead `getLicenseScore` helper. ### `package.json` / `package-lock.json` - Removed the unused `dependency-tree` dependency (`npm uninstall dependency-tree`). `universal-module-tree` was already listed and is now the single source of truth for the dependency graph. --- ## Verification ### Empirical: correct coordinates go out ``` $ node -e "const umt=require('universal-module-tree'); (async()=>{ const t=await umt('./test/fixtures/mock-project'); const f=umt.flatten(t); console.log('total:', f.length); console.log('zero-versions:', f.filter(p=>p.version==='0.0.0')); console.log('filename-like:', f.filter(p=>/\\.|\\//.test(p.name))); })()" total: 37 zero-versions: [] filename-like: [] ``` No `@ 0.0.0`, no `jwt`-style filename names — just real installed packages from `package-lock.json`. ### Tests: `npm run test-only` → 24 / 24 pass Notable guards: - `report › report output matches snapshot` — asserts **"36 packages checked"**, which only holds when the transitive graph is walked correctly. - `report › report with poisoned project` — asserts a mock package returned with `version: null` is **skipped**, not rendered as `@ 0.0.0`. - All `--filter=*`, `--compliance`, `--security`, `--long` snapshot tests pass, plus `details`, `whitelist`, `install`, `github-actions`, and `proxied` suites. --- ## Issue 2: bare `ncm` "with no success" `ncm` with no subcommand shows help and exits 0 — same as `git` or `npm` without args. `bin/ncm-cli.js`: ```js let [command = 'help', ...subargs] = argv._ if (!Object.keys(commands).includes(command)) command = 'help' ``` No API request is ever made, which is why the debug session looked like "nothing happening". Verified post-fix: `node bin/ncm-cli.js` prints help and exits 0. To actually exercise the analyzer against the local API: ```sh NCM_TOKEN=6c044113-c485-40cb-915b-cbc9d3a26730 \ NCM_API=http://localhost:3000 \ ncm report --dir=/path/to/some/project ``` --- ## Issue 4: vulnDB coverage investigation Previously impossible to do meaningfully — the payload was garbage (issue 1) and incomplete (issue 3). Now that real coordinates for the full transitive tree are sent, a direct diff is valid: ```sh cd /path/to/project npm install # ensure lockfile + node_modules npm audit --json > /tmp/npm-audit.json # npm's view NCM_TOKEN=… NCM_API=http://localhost:3000 \ ncm report --long --json > /tmp/ncm-report.json # ncm's view ``` Compare the `{name, version}` sets: - advisories in `npm-audit` but not `ncm-report` → real gaps in the vulnDB, feed back to the data team; - advisories in `ncm-report` but not `npm-audit` → extra NCM coverage (or a false positive to review). --- ## Files changed - `lib/ncm-analyze-tree.js` — rewritten (550 → 131 lines). - `commands/report.js` — `0.0.0` coercion and dead code removed. - `package.json`, `package-lock.json` — `dependency-tree` removed. * Fix: wrong package coordinates sent to NCM API # Fix: wrong package coordinates sent to NCM API ## Summary | # | Report | Status | Where | |---|---|---|---| | 1 | "sending odd packages versions to the API like this one jwt version 0.0.0" | Fixed | `lib/ncm-analyze-tree.js`, `commands/report.js` | | 2 | `NCM_TOKEN=… NCM_API=… ncm` "with no success" | Not a bug; documented below | — | | 3 | "still a diff on npm audit and ncm report" | Fixed (root cause shared with #1) | `lib/ncm-analyze-tree.js` | | 4 | "we would need to investigate if our vulnDB has all the vulns" | Now meaningfully testable | see below | Tests: **24 / 24** pass (`npm run test-only`). --- ## Root cause (issues 1 & 3) `lib/ncm-analyze-tree.js` had been rewritten to use [`dependency-tree`](https://www.npmjs.com/package/dependency-tree) — a **source-code import analyzer** — instead of [`universal-module-tree`](https://www.npmjs.com/package/universal-module-tree), which walks the real npm dependency graph (`package-lock.json` → `yarn.lock` → `node_modules`). Three concrete defects in that code produced the payloads the user saw: 1. **Filename-as-package-name with placeholder version.** `dependency-tree` returns resolved file paths. The replacement code turned each path into a "package": ```js const name = path.basename(dep, path.extname(dep)); const childNode = { data: { name, version: '0.0.0' }, // hard-coded children: [] }; ``` So `jsonwebtoken.js` → `jwt @ 0.0.0`, `index.js` → `index @ 0.0.0`, and similar. That is the exact `jwt version 0.0.0` entry the user saw going out. 2. **Range-string fallback also produced `0.0.0`.** The auxiliary `getNpmDependencies` helper only read direct deps from `package.json` and did: ```js const cleanVersion = version.replace(/[^\d.]/g, '') || version; ``` For `"latest"`, `"file:…"`, `"git+https://…"`, `"workspace:*"`, etc. the strip leaves `''` and the `||` fallback is still non-semver — so those deps were emitted as `name @ 0.0.0` too. 3. **Missing transitive deps.** `dependency-tree` only surfaces files the entry point actually `require()`s, and the `package.json` fallback only listed direct dependencies. The full transitive graph was never walked. This is the direct mechanical cause of the diff vs `npm audit`. 4. **`commands/report.js` had been patched to hide the fallout.** Any `null` / `undefined` `version` returned by the NCM service was coerced to `'0.0.0'` and still included in the rendered report, so bogus rows showed up in the UI too. ## Fix ### `lib/ncm-analyze-tree.js` — rewritten (550 → 131 lines) - Reverted to `universal-module-tree` (already in `dependencies`, and the same library `commands/details.js` uses). - Removed, in full: - the `dependency-tree` invocation and entry-point discovery logic (guessing `index.js`, `app.js`, `bin/*.js`, …), - filename-as-package-name logic, - `getNpmDependencies` and the duplicate `readPackagesFromPackageJson` that stripped `^` / `~` and fell back to `'0.0.0'`, - dead counters. The new file gets a tree from `universalModuleTree(dir)`, walks it keeping dedup + `paths` info, and POSTs `{ name, version }` pairs to the GraphQL endpoint — same shape that already worked in `commands/details.js`. ### `commands/report.js` - Removed the `effectiveVersion = '0.0.0'` coercion. Packages the NCM service returns with `version === null` (unpublished / unknown) are now **skipped** instead of rendered as `@ 0.0.0`: ```js if (version === null || version === undefined) continue; ``` - Removed unused `includedCount` / `skippedCount` counters and the dead `getLicenseScore` helper. ### `package.json` / `package-lock.json` - Removed the unused `dependency-tree` dependency (`npm uninstall dependency-tree`). `universal-module-tree` was already listed and is now the single source of truth for the dependency graph. --- ## Verification ### Empirical: correct coordinates go out ``` $ node -e "const umt=require('universal-module-tree'); (async()=>{ const t=await umt('./test/fixtures/mock-project'); const f=umt.flatten(t); console.log('total:', f.length); console.log('zero-versions:', f.filter(p=>p.version==='0.0.0')); console.log('filename-like:', f.filter(p=>/\\.|\\//.test(p.name))); })()" total: 37 zero-versions: [] filename-like: [] ``` No `@ 0.0.0`, no `jwt`-style filename names — just real installed packages from `package-lock.json`. ### Tests: `npm run test-only` → 24 / 24 pass Notable guards: - `report › report output matches snapshot` — asserts **"36 packages checked"**, which only holds when the transitive graph is walked correctly. - `report › report with poisoned project` — asserts a mock package returned with `version: null` is **skipped**, not rendered as `@ 0.0.0`. - All `--filter=*`, `--compliance`, `--security`, `--long` snapshot tests pass, plus `details`, `whitelist`, `install`, `github-actions`, and `proxied` suites. --- ## Issue 2: bare `ncm` "with no success" `ncm` with no subcommand shows help and exits 0 — same as `git` or `npm` without args. `bin/ncm-cli.js`: ```js let [command = 'help', ...subargs] = argv._ if (!Object.keys(commands).includes(command)) command = 'help' ``` No API request is ever made, which is why the debug session looked like "nothing happening". Verified post-fix: `node bin/ncm-cli.js` prints help and exits 0. To actually exercise the analyzer against the local API: ```sh NCM_TOKEN=6c044113-c485-40cb-915b-cbc9d3a26730 \ NCM_API=http://localhost:3000 \ ncm report --dir=/path/to/some/project ``` --- ## Issue 4: vulnDB coverage investigation Previously impossible to do meaningfully — the payload was garbage (issue 1) and incomplete (issue 3). Now that real coordinates for the full transitive tree are sent, a direct diff is valid: ```sh cd /path/to/project npm install # ensure lockfile + node_modules npm audit --json > /tmp/npm-audit.json # npm's view NCM_TOKEN=… NCM_API=http://localhost:3000 \ ncm report --long --json > /tmp/ncm-report.json # ncm's view ``` Compare the `{name, version}` sets: - advisories in `npm-audit` but not `ncm-report` → real gaps in the vulnDB, feed back to the data team; - advisories in `ncm-report` but not `npm-audit` → extra NCM coverage (or a false positive to review). --- ## Files changed - `lib/ncm-analyze-tree.js` — rewritten (550 → 131 lines). - `commands/report.js` — `0.0.0` coercion and dead code removed. - `package.json`, `package-lock.json` — `dependency-tree` removed. * Fix: wrong package coordinates sent to NCM API # Fix: wrong package coordinates sent to NCM API ## Summary | # | Report | Status | Where | |---|---|---|---| | 1 | "sending odd packages versions to the API like this one jwt version 0.0.0" | Fixed | `lib/ncm-analyze-tree.js`, `commands/report.js` | | 2 | `NCM_TOKEN=… NCM_API=… ncm` "with no success" | Not a bug; documented below | — | | 3 | "still a diff on npm audit and ncm report" | Fixed (root cause shared with #1) | `lib/ncm-analyze-tree.js` | | 4 | "we would need to investigate if our vulnDB has all the vulns" | Now meaningfully testable | see below | Tests: **24 / 24** pass (`npm run test-only`). --- ## Root cause (issues 1 & 3) `lib/ncm-analyze-tree.js` had been rewritten to use [`dependency-tree`](https://www.npmjs.com/package/dependency-tree) — a **source-code import analyzer** — instead of [`universal-module-tree`](https://www.npmjs.com/package/universal-module-tree), which walks the real npm dependency graph (`package-lock.json` → `yarn.lock` → `node_modules`). Three concrete defects in that code produced the payloads the user saw: 1. **Filename-as-package-name with placeholder version.** `dependency-tree` returns resolved file paths. The replacement code turned each path into a "package": ```js const name = path.basename(dep, path.extname(dep)); const childNode = { data: { name, version: '0.0.0' }, // hard-coded children: [] }; ``` So `jsonwebtoken.js` → `jwt @ 0.0.0`, `index.js` → `index @ 0.0.0`, and similar. That is the exact `jwt version 0.0.0` entry the user saw going out. 2. **Range-string fallback also produced `0.0.0`.** The auxiliary `getNpmDependencies` helper only read direct deps from `package.json` and did: ```js const cleanVersion = version.replace(/[^\d.]/g, '') || version; ``` For `"latest"`, `"file:…"`, `"git+https://…"`, `"workspace:*"`, etc. the strip leaves `''` and the `||` fallback is still non-semver — so those deps were emitted as `name @ 0.0.0` too. 3. **Missing transitive deps.** `dependency-tree` only surfaces files the entry point actually `require()`s, and the `package.json` fallback only listed direct dependencies. The full transitive graph was never walked. This is the direct mechanical cause of the diff vs `npm audit`. 4. **`commands/report.js` had been patched to hide the fallout.** Any `null` / `undefined` `version` returned by the NCM service was coerced to `'0.0.0'` and still included in the rendered report, so bogus rows showed up in the UI too. ## Fix ### `lib/ncm-analyze-tree.js` — rewritten (550 → 131 lines) - Reverted to `universal-module-tree` (already in `dependencies`, and the same library `commands/details.js` uses). - Removed, in full: - the `dependency-tree` invocation and entry-point discovery logic (guessing `index.js`, `app.js`, `bin/*.js`, …), - filename-as-package-name logic, - `getNpmDependencies` and the duplicate `readPackagesFromPackageJson` that stripped `^` / `~` and fell back to `'0.0.0'`, - dead counters. The new file gets a tree from `universalModuleTree(dir)`, walks it keeping dedup + `paths` info, and POSTs `{ name, version }` pairs to the GraphQL endpoint — same shape that already worked in `commands/details.js`. ### `commands/report.js` - Removed the `effectiveVersion = '0.0.0'` coercion. Packages the NCM service returns with `version === null` (unpublished / unknown) are now **skipped** instead of rendered as `@ 0.0.0`: ```js if (version === null || version === undefined) continue; ``` - Removed unused `includedCount` / `skippedCount` counters and the dead `getLicenseScore` helper. ### `package.json` / `package-lock.json` - Removed the unused `dependency-tree` dependency (`npm uninstall dependency-tree`). `universal-module-tree` was already listed and is now the single source of truth for the dependency graph. --- ## Verification ### Empirical: correct coordinates go out ``` $ node -e "const umt=require('universal-module-tree'); (async()=>{ const t=await umt('./test/fixtures/mock-project'); const f=umt.flatten(t); console.log('total:', f.length); console.log('zero-versions:', f.filter(p=>p.version==='0.0.0')); console.log('filename-like:', f.filter(p=>/\\.|\\//.test(p.name))); })()" total: 37 zero-versions: [] filename-like: [] ``` No `@ 0.0.0`, no `jwt`-style filename names — just real installed packages from `package-lock.json`. ### Tests: `npm run test-only` → 24 / 24 pass Notable guards: - `report › report output matches snapshot` — asserts **"36 packages checked"**, which only holds when the transitive graph is walked correctly. - `report › report with poisoned project` — asserts a mock package returned with `version: null` is **skipped**, not rendered as `@ 0.0.0`. - All `--filter=*`, `--compliance`, `--security`, `--long` snapshot tests pass, plus `details`, `whitelist`, `install`, `github-actions`, and `proxied` suites. --- ## Issue 2: bare `ncm` "with no success" `ncm` with no subcommand shows help and exits 0 — same as `git` or `npm` without args. `bin/ncm-cli.js`: ```js let [command = 'help', ...subargs] = argv._ if (!Object.keys(commands).includes(command)) command = 'help' ``` No API request is ever made, which is why the debug session looked like "nothing happening". Verified post-fix: `node bin/ncm-cli.js` prints help and exits 0. To actually exercise the analyzer against the local API: ```sh NCM_TOKEN=6c044113-c485-40cb-915b-cbc9d3a26730 \ NCM_API=http://localhost:3000 \ ncm report --dir=/path/to/some/project ``` --- ## Issue 4: vulnDB coverage investigation Previously impossible to do meaningfully — the payload was garbage (issue 1) and incomplete (issue 3). Now that real coordinates for the full transitive tree are sent, a direct diff is valid: ```sh cd /path/to/project npm install # ensure lockfile + node_modules npm audit --json > /tmp/npm-audit.json # npm's view NCM_TOKEN=… NCM_API=http://localhost:3000 \ ncm report --long --json > /tmp/ncm-report.json # ncm's view ``` Compare the `{name, version}` sets: - advisories in `npm-audit` but not `ncm-report` → real gaps in the vulnDB, feed back to the data team; - advisories in `ncm-report` but not `npm-audit` → extra NCM coverage (or a false positive to review). --- ## Files changed - `lib/ncm-analyze-tree.js` — rewritten (550 → 131 lines). - `commands/report.js` — `0.0.0` coercion and dead code removed. - `package.json`, `package-lock.json` — `dependency-tree` removed. * Fix: wrong package coordinates sent to NCM API # Fix: wrong package coordinates sent to NCM API ## Summary | # | Report | Status | Where | |---|---|---|---| | 1 | "sending odd packages versions to the API like this one jwt version 0.0.0" | Fixed | `lib/ncm-analyze-tree.js`, `commands/report.js` | | 2 | `NCM_TOKEN=… NCM_API=… ncm` "with no success" | Not a bug; documented below | — | | 3 | "still a diff on npm audit and ncm report" | Fixed (root cause shared with #1) | `lib/ncm-analyze-tree.js` | | 4 | "we would need to investigate if our vulnDB has all the vulns" | Now meaningfully testable | see below | Tests: **24 / 24** pass (`npm run test-only`). --- ## Root cause (issues 1 & 3) `lib/ncm-analyze-tree.js` had been rewritten to use [`dependency-tree`](https://www.npmjs.com/package/dependency-tree) — a **source-code import analyzer** — instead of [`universal-module-tree`](https://www.npmjs.com/package/universal-module-tree), which walks the real npm dependency graph (`package-lock.json` → `yarn.lock` → `node_modules`). Three concrete defects in that code produced the payloads the user saw: 1. **Filename-as-package-name with placeholder version.** `dependency-tree` returns resolved file paths. The replacement code turned each path into a "package": ```js const name = path.basename(dep, path.extname(dep)); const childNode = { data: { name, version: '0.0.0' }, // hard-coded children: [] }; ``` So `jsonwebtoken.js` → `jwt @ 0.0.0`, `index.js` → `index @ 0.0.0`, and similar. That is the exact `jwt version 0.0.0` entry the user saw going out. 2. **Range-string fallback also produced `0.0.0`.** The auxiliary `getNpmDependencies` helper only read direct deps from `package.json` and did: ```js const cleanVersion = version.replace(/[^\d.]/g, '') || version; ``` For `"latest"`, `"file:…"`, `"git+https://…"`, `"workspace:*"`, etc. the strip leaves `''` and the `||` fallback is still non-semver — so those deps were emitted as `name @ 0.0.0` too. 3. **Missing transitive deps.** `dependency-tree` only surfaces files the entry point actually `require()`s, and the `package.json` fallback only listed direct dependencies. The full transitive graph was never walked. This is the direct mechanical cause of the diff vs `npm audit`. 4. **`commands/report.js` had been patched to hide the fallout.** Any `null` / `undefined` `version` returned by the NCM service was coerced to `'0.0.0'` and still included in the rendered report, so bogus rows showed up in the UI too. ## Fix ### `lib/ncm-analyze-tree.js` — rewritten (550 → 131 lines) - Reverted to `universal-module-tree` (already in `dependencies`, and the same library `commands/details.js` uses). - Removed, in full: - the `dependency-tree` invocation and entry-point discovery logic (guessing `index.js`, `app.js`, `bin/*.js`, …), - filename-as-package-name logic, - `getNpmDependencies` and the duplicate `readPackagesFromPackageJson` that stripped `^` / `~` and fell back to `'0.0.0'`, - dead counters. The new file gets a tree from `universalModuleTree(dir)`, walks it keeping dedup + `paths` info, and POSTs `{ name, version }` pairs to the GraphQL endpoint — same shape that already worked in `commands/details.js`. ### `commands/report.js` - Removed the `effectiveVersion = '0.0.0'` coercion. Packages the NCM service returns with `version === null` (unpublished / unknown) are now **skipped** instead of rendered as `@ 0.0.0`: ```js if (version === null || version === undefined) continue; ``` - Removed unused `includedCount` / `skippedCount` counters and the dead `getLicenseScore` helper. ### `package.json` / `package-lock.json` - Removed the unused `dependency-tree` dependency (`npm uninstall dependency-tree`). `universal-module-tree` was already listed and is now the single source of truth for the dependency graph. --- ## Verification ### Empirical: correct coordinates go out ``` $ node -e "const umt=require('universal-module-tree'); (async()=>{ const t=await umt('./test/fixtures/mock-project'); const f=umt.flatten(t); console.log('total:', f.length); console.log('zero-versions:', f.filter(p=>p.version==='0.0.0')); console.log('filename-like:', f.filter(p=>/\\.|\\//.test(p.name))); })()" total: 37 zero-versions: [] filename-like: [] ``` No `@ 0.0.0`, no `jwt`-style filename names — just real installed packages from `package-lock.json`. ### Tests: `npm run test-only` → 24 / 24 pass Notable guards: - `report › report output matches snapshot` — asserts **"36 packages checked"**, which only holds when the transitive graph is walked correctly. - `report › report with poisoned project` — asserts a mock package returned with `version: null` is **skipped**, not rendered as `@ 0.0.0`. - All `--filter=*`, `--compliance`, `--security`, `--long` snapshot tests pass, plus `details`, `whitelist`, `install`, `github-actions`, and `proxied` suites. --- ## Issue 2: bare `ncm` "with no success" `ncm` with no subcommand shows help and exits 0 — same as `git` or `npm` without args. `bin/ncm-cli.js`: ```js let [command = 'help', ...subargs] = argv._ if (!Object.keys(commands).includes(command)) command = 'help' ``` No API request is ever made, which is why the debug session looked like "nothing happening". Verified post-fix: `node bin/ncm-cli.js` prints help and exits 0. To actually exercise the analyzer against the local API: ```sh NCM_TOKEN=6c044113-c485-40cb-915b-cbc9d3a26730 \ NCM_API=http://localhost:3000 \ ncm report --dir=/path/to/some/project ``` --- ## Issue 4: vulnDB coverage investigation Previously impossible to do meaningfully — the payload was garbage (issue 1) and incomplete (issue 3). Now that real coordinates for the full transitive tree are sent, a direct diff is valid: ```sh cd /path/to/project npm install # ensure lockfile + node_modules npm audit --json > /tmp/npm-audit.json # npm's view NCM_TOKEN=… NCM_API=http://localhost:3000 \ ncm report --long --json > /tmp/ncm-report.json # ncm's view ``` Compare the `{name, version}` sets: - advisories in `npm-audit` but not `ncm-report` → real gaps in the vulnDB, feed back to the data team; - advisories in `ncm-report` but not `npm-audit` → extra NCM coverage (or a false positive to review). --- ## Files changed - `lib/ncm-analyze-tree.js` — rewritten (550 → 131 lines). - `commands/report.js` — `0.0.0` coercion and dead code removed. - `package.json`, `package-lock.json` — `dependency-tree` removed.
1 parent 1542f41 commit c46fb91

13 files changed

Lines changed: 2010 additions & 575 deletions

File tree

commands/report.js

Lines changed: 67 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ const {
1414
SEVERITY_RMAP_NPM,
1515
moduleSort
1616
} = require('../lib/report/util')
17+
const licenses = require('../lib/report/licenses')
1718
const longReport = require('../lib/report/long')
1819
const shortReport = require('../lib/report/short')
1920
const { helpHeader } = require('../lib/help')
@@ -139,9 +140,6 @@ async function report (argv, _dir) {
139140
const isNested = pkgName === nestedPkgName && pkgVersion === nestedPkgVersion
140141

141142
// Processing packages from NCM service
142-
let includedCount = 0;
143-
let skippedCount = 0;
144-
145143
for (const { name, version, scores, published } of data) {
146144
let maxSeverity = 0;
147145
let license = {};
@@ -170,45 +168,65 @@ async function report (argv, _dir) {
170168
}
171169
}
172170

173-
// Modified approach to include ALL packages in the report
174-
// Even packages with null/undefined versions will be included with a default version
175-
let effectiveVersion = version;
176-
if (effectiveVersion === null || effectiveVersion === undefined) {
177-
effectiveVersion = '0.0.0';
178-
// Using default version 0.0.0 for package
179-
}
180-
181-
// Skip nested packages with severity issues
182-
if (isNested && !!maxSeverity) {
183-
skippedCount++;
184-
// Skipping nested package
185-
continue;
171+
// Skip packages the NCM service didn't return data for (null version /
172+
// unpublished). Previously these were coerced to '0.0.0' which polluted
173+
// reports with placeholder entries like `jwt @ 0.0.0`.
174+
if (version === null || version === undefined) continue;
175+
176+
// Skip nested packages (the project reporting on itself) with severity issues
177+
if (isNested && !!maxSeverity) continue;
178+
179+
// License cert fallback: when the NCM API returned a license score but
180+
// didn't commit to `pass` (or when `data.spdx` is a non-canonical or
181+
// compound expression the server didn't normalise), apply the built-in
182+
// SPDX policy so the CLI still reaches a deterministic verdict. The
183+
// server-supplied verdict always wins when present.
184+
if (license && license.data && license.data.spdx != null) {
185+
const canonical = licenses.normalize(license.data.spdx);
186+
if (canonical && canonical !== license.data.spdx) {
187+
license = Object.assign({}, license, {
188+
data: Object.assign({}, license.data, { spdx: canonical })
189+
});
190+
}
191+
if (license.pass == null) {
192+
const verdict = licenses.evaluate(canonical);
193+
if (verdict !== null) {
194+
license = Object.assign({}, license, {
195+
pass: verdict,
196+
severity: verdict ? 'NONE' : (license.severity || 'MEDIUM')
197+
});
198+
if (!verdict) {
199+
failures.push(license);
200+
hasFailures = true;
201+
}
202+
}
203+
}
186204
}
187-
188-
// Check if license has failed, which should upgrade to critical severity
189-
const getLicenseScore = ({ pass }) => pass === false ? 0 : null;
205+
206+
// Escalate to Critical when the license is noncompliant (whether the
207+
// verdict came from the server or the client-side fallback above).
190208
if (license && license.pass === false) {
191209
maxSeverity = 4;
192210
}
193211

194-
// Add the package to our report
195212
pkgScores.push({
196213
name,
197-
version: effectiveVersion, // Use effective version instead of potentially null version
214+
version,
198215
published,
199216
maxSeverity,
200217
failures,
201218
license,
202219
scores
203220
});
204-
205-
includedCount++;
206221
}
207-
208-
// Package processing complete
209222

210223
pkgScores = moduleSort(pkgScores)
211224

225+
// Build name→version map from NCM data for npm audit v7+ version lookup
226+
const versionByName = new Map([...data]
227+
.filter(pkg => pkg.version)
228+
.map(pkg => [pkg.name, pkg.version]))
229+
212230
// Process whitelisted packages
213231
const whitelisted = pkgScores.filter(pkg => whitelist.has(`${pkg.name}@${pkg.version}`))
214232
.map(pkgScore => ({ ...pkgScore, quantitativeScore: score(pkgScore.scores, pkgScore.maxSeverity) }))
@@ -251,6 +269,7 @@ async function report (argv, _dir) {
251269
try {
252270
const npmAuditJson = JSON.parse(npmAuditData) || {}
253271
if (npmAuditJson.advisories) {
272+
// npm v6 format
254273
for (const advisory of Object.values(npmAuditJson.advisories)) {
255274
const { version } = advisory.findings ? (advisory.findings[0] || {}) : {}
256275
const { module_name: name, severity = 'NONE' } = advisory
@@ -266,6 +285,29 @@ async function report (argv, _dir) {
266285
auditScore: maxSeverity
267286
})
268287
}
288+
} else if (npmAuditJson.vulnerabilities) {
289+
// npm v7+ format (auditReportVersion: 2). Packages already reported by
290+
// NCM (in pkgScores or whitelisted) are skipped — otherwise every
291+
// transitive vuln would show up twice.
292+
const reportedIds = new Set(
293+
[...pkgScores, ...whitelisted].map(p => `${p.name}@${p.version}`)
294+
)
295+
for (const [name, vuln] of Object.entries(npmAuditJson.vulnerabilities)) {
296+
const version = versionByName.get(name)
297+
if (reportedIds.has(`${name}@${version}`)) continue
298+
const { severity = 'none' } = vuln
299+
const maxSeverity = SEVERITY_RMAP_NPM.indexOf(severity.toUpperCase())
300+
pkgScores.push({
301+
name,
302+
version,
303+
published: true,
304+
maxSeverity,
305+
failures: [],
306+
license: {},
307+
scores: [],
308+
auditScore: maxSeverity
309+
})
310+
}
269311
}
270312
} catch (err) { } // intentional noop
271313

commands/whitelist.js

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ const { helpHeader } = require('../lib/help')
88
const { getValue, setValue } = require('../lib/config')
99
const whitelistReport = require('../lib/report/whitelist')
1010
const { SEVERITY_RMAP } = require('../lib/report/util')
11+
const licenses = require('../lib/report/licenses')
1112
const {
1213
COLORS,
1314
header,
@@ -191,6 +192,28 @@ async function whitelist (argv) {
191192
if (score.name === 'license') license = score
192193
}
193194

195+
// License cert fallback: if the server returned an SPDX but no pass
196+
// verdict, apply the built-in policy so whitelisted modules render
197+
// with a deterministic badge. Mirrors the logic in commands/report.js.
198+
if (license && license.data && license.data.spdx != null) {
199+
const canonical = licenses.normalize(license.data.spdx)
200+
if (canonical && canonical !== license.data.spdx) {
201+
license = Object.assign({}, license, {
202+
data: Object.assign({}, license.data, { spdx: canonical })
203+
})
204+
}
205+
if (license.pass == null) {
206+
const verdict = licenses.evaluate(canonical)
207+
if (verdict !== null) {
208+
license = Object.assign({}, license, {
209+
pass: verdict,
210+
severity: verdict ? 'NONE' : (license.severity || 'MEDIUM')
211+
})
212+
if (!verdict) failures.push(license)
213+
}
214+
}
215+
}
216+
194217
const getLicenseScore = ({ pass }) => !pass ? 0 : null
195218
if (getLicenseScore(license) === 0) maxSeverity = 4
196219

0 commit comments

Comments
 (0)