fix: adapt audit client to npmjs /advisories/bulk endpoint#11268
fix: adapt audit client to npmjs /advisories/bulk endpoint#11268
Conversation
| const quickAuditUrl = `${registry}-/npm/v1/security/audits/quick` | ||
| const auditUrl = `${registry}-/npm/v1/security/advisories/bulk` | ||
| const authHeaderValue = getAuthHeader(registry) | ||
| const requestBody = JSON.stringify(auditTree) |
There was a problem hiding this comment.
The changes are less trivial than just changing the endpoint. The expected body of the POST request is completely different to what is generated by lockfileToAuditTree and sent as body.
Expected:
{
"@npmcli/arborist": [
"1.0.0",
"2.0.1"
],
"@npmcli/config": [
"10.4.2"
]
}The type of auditTree is:
export interface AuditNode {
version?: string
integrity?: string
requires?: Record<string, string>
dependencies?: { [name: string]: AuditNode }
dev: boolean
}There was a problem hiding this comment.
More complex you mean?
yikes 😬 I meant less trivial!
There was a problem hiding this comment.
Before putting together this PR I hacked into my local pnpm script files, to change the URL, and it seemed to work. But if formats can differ, we should cope...
There was a problem hiding this comment.
Retracing my steps; it seems you're right: even the message body does not conform to the one required by the new endpoint:
ERR_PNPM_AUDIT_BAD_RESPONSE The audit endpoint (at https://registry.npmjs.org/-/npm/v1/security/advisories/bulk) responded with 400: {"statusCode":400,"error":"Bad Request","message":"Invalid request payload input"}
|
What about the people using private registries? |
Good point; we could re-instate the fallback mechanism, with that endpoint fallback. Any opinions? |
The new endpoint expects a flat {name: [versions]} request body and
returns advisories keyed by package name, so simply renaming the URL is
not enough. Flatten the audit tree for the request and map the bulk
response into the existing AuditReport shape, recomputing findings
paths from the lockfile and severity counts locally, since the new
endpoint no longer returns them.
There was a problem hiding this comment.
Pull request overview
Updates pnpm’s audit implementation to use npmjs.org’s replacement /-/npm/v1/security/advisories/bulk endpoint now that the legacy /-/npm/v1/security/audits{,/quick} endpoints are retired, while keeping downstream consumers working by mapping the new response back into AuditReport.
Changes:
- Switch audit client networking from
/audits{,/quick}to/advisories/bulk, including new request/response adaptation logic. - Update audit-related tests/mocks and regenerate
audit --jsonsnapshot for the new report shape (actions: [], locally computed counts/paths). - Fix
AuditAdvisory.findingstyping from a tuple to an array and addsemverto support local vulnerability matching.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
deps/compliance/audit/src/index.ts |
Implements /advisories/bulk request/response mapping and local findings/count computation. |
deps/compliance/audit/src/types.ts |
Fixes findings typing by introducing AuditFinding and using AuditFinding[]. |
deps/compliance/audit/test/index.ts |
Updates mock endpoint paths/responses for the new bulk endpoint behavior. |
deps/compliance/audit/package.json |
Adds semver (+ types) required for local version range matching. |
deps/compliance/commands/test/audit/utils/responses/index.ts |
Converts legacy captured fixtures into bulk response shape at load time. |
deps/compliance/commands/test/audit/index.ts |
Updates audit command integration tests to intercept the new bulk endpoint and adjusts expectations. |
deps/compliance/commands/test/audit/fix.ts |
Updates audit fix tests to intercept the new bulk endpoint. |
deps/compliance/commands/test/audit/ignore.ts |
Updates ignore tests to intercept the new bulk endpoint. |
deps/compliance/commands/test/audit/fixWithUpdate.ts |
Converts legacy on-disk fixture responses to bulk shape before replying. |
deps/compliance/commands/test/audit/preserveReferenceOverrides.ts |
Updates endpoint interception to bulk. |
deps/compliance/commands/test/audit/__snapshots__/index.ts.snap |
Regenerates snapshot for the new JSON output shape (no actions, new counts). |
pnpm-lock.yaml |
Records semver and @types/semver additions. |
.changeset/fix-npmjs-audit-endpoint.md |
Declares release impacts (major/minor/patch) for the endpoint migration. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ures The /advisories/bulk endpoint does not return CVE identifiers or most of the metadata the old /audits endpoint provided. Rename auditConfig.ignoreCves to auditConfig.ignoreGhsas, make --ignore and --ignore-unfixable operate on GitHub advisory IDs, and derive the GHSA from each advisory URL. patched_versions is inferred from vulnerable_versions for simple < and <= ranges so audit --fix still produces useful overrides. Regenerate the top-level fixtures with real bulk responses captured from registry.npmjs.org and convert the synthetic update-* fixtures to the bulk shape. Drop the runtime fixture-shape conversion from the tests.
- Skip importer wrapper nodes in buildBulkRequestBody so only real package names appear in the /advisories/bulk payload. - Compute metadata.dependencies as the non-dev portion so the counts in AuditMetadata stay internally consistent. - Explicitly populate every required AuditAdvisory field in normalizeAdvisory instead of relying on a force cast, so downstream code never sees undefined in required fields. - Add a unit test that drives the locally-computed findings path with a bare bulk response matching what registry.npmjs.org returns today.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 32 changed files in this pull request and generated 2 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- buildFindings no longer emits a placeholder finding when no installed version satisfies the vulnerable range; bulkResponseToAuditReport now skips advisories that end up with zero findings so the CLI does not report false positives for packages the lockfile doesn't use. - Rename advisoryWthNoResolutions to advisoryWithNoResolutions. - Update info-vulnerability fixture to key on a package the lockfile actually uses (axios) so the new skip-empty-findings logic exercises the info-severity path. - Backfill the preserve-reference-overrides lockfile with a snapshots block so lockfileToAuditTree walks its dependencies; without it, the fixture had only importer-level deps and no advisories could match.
The nested AuditTree shape existed to match the legacy /audits request
body. The new /advisories/bulk endpoint needs only a flat
{name: [versions]} map, and finding paths are computed locally from the
lockfile. Walking the lockfile into a deep tree and then traversing it
twice more to flatten it and index paths was wasted work.
Replace lockfileToAuditTree with lockfileToAuditIndex, which walks the
lockfile once and returns both the POST body and a name→version→paths
index. Drop AuditTree/AuditNode, buildBulkRequestBody, buildPathIndex,
and the manifest-read/ramda/workspace.project-manifest-reader deps they
needed.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 42 changed files in this pull request and generated 1 comment.
Files not reviewed (2)
- deps/compliance/commands/test/audit/fixtures/preserve-reference-overrides/pnpm-lock.yaml: Language not supported
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 42 changed files in this pull request and generated 3 comments.
Files not reviewed (2)
- deps/compliance/commands/test/audit/fixtures/preserve-reference-overrides/pnpm-lock.yaml: Language not supported
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 42 changed files in this pull request and generated no new comments.
Files not reviewed (2)
- deps/compliance/commands/test/audit/fixtures/preserve-reference-overrides/pnpm-lock.yaml: Language not supported
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… returns
The PR targets npm's /advisories/bulk endpoint specifically. Carrying
declarations for fields npm doesn't return (cves, patched_versions,
github_advisory_id, module_name, created, updated, deleted, access,
overview, recommendation, references, found_by, reported_by, metadata,
npm_advisory_id, findings) suggested a wider compatibility we don't
actually have.
BulkAdvisory now only declares id, url, title, severity,
vulnerable_versions, and cwe — the shape npm actually returns.
normalizeAdvisory stops reading the dropped fields and fills them with
static defaults for downstream consumers. buildFindings loses the
"if adv.findings is present" branch; findings are always computed from
the lockfile + vulnerable_versions.
The "<0.0.0" sentinel handling in fix.ts and ignore.ts is dead with
this shape (patched_versions is only ever the inferred range or
undefined), so simplify both: getFixableAdvisories keeps advisories
with an inferrable range, filterAdvisoriesWithNoResolutions returns
those without one. Update UNFIXABLE_RESPONSE in ignore.ts test to use
an uninferrable vulnerable_versions (">=0.0.0" / "*") instead of the
now-unused patched_versions: "<0.0.0".
…turn The PR targets npm's /advisories/bulk endpoint and nothing else, so carrying AuditAdvisory fields npm never populates (cves, created, updated, deleted, access, overview, recommendation, references, found_by, reported_by, metadata) and AuditReport.actions / .muted was misleading — they were always empty strings, empty arrays, or empty objects after normalization. AuditReport now has only advisories and metadata. AuditAdvisory now has findings, id, title, module_name, vulnerable_versions, patched_versions, severity, cwe, github_advisory_id, and url. AuditAction/AuditResolution/ AuditActionRecommendation are dropped. normalizeAdvisory and bulkResponseToAuditReport return the trimmed shapes; the --json snapshot was regenerated. Changeset updated to document the shape change.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 42 changed files in this pull request and generated 2 comments.
Files not reviewed (2)
- deps/compliance/commands/test/audit/fixtures/preserve-reference-overrides/pnpm-lock.yaml: Language not supported
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Two failure modes in recordPath: - The same joined trail can be generated more than once when a package appears in both dependencies and optionalDependencies of the same parent, or via equivalent peer-suffix variants. An includes() check before push drops those duplicates. - For dep graphs with heavy sharing, the number of distinct root-to-dependency paths can grow very large. The CLI only displays a handful before suggesting `pnpm why`, so cap the recorded array at MAX_PATHS_PER_FINDING (100) — more than enough to inform users, bounded enough to keep the worst case in check.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 42 changed files in this pull request and generated 2 comments.
Files not reviewed (2)
- deps/compliance/commands/test/audit/fixtures/preserve-reference-overrides/pnpm-lock.yaml: Language not supported
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
deps/compliance/commands/src/audit/audit.ts:313
ignoreGhsasmatching is currently case-sensitive (Set.has ongithub_advisory_id). If a user providesghsa-...(different casing) or a registry returns a differently-cased GHSA in the URL, the advisory won’t be filtered/ignored. Consider normalizing both configured IDs and derivedgithub_advisory_idto a canonical form (e.g. uppercaseGHSA-prefix + lowercase remainder) before comparison.
const ignoreGhsas = opts.auditConfig?.ignoreGhsas
if (ignoreGhsas?.length) {
const ignoreSet = new Set(ignoreGhsas)
auditReport.advisories = pickBy(({ github_advisory_id: githubAdvisoryId, severity }) => {
if (!ignoreSet.has(githubAdvisoryId)) {
return true
}
ignoredVulnerabilities[severity as AuditLevelString] += 1
return false
}, auditReport.advisories)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
GHSA identifiers are canonically uppercase but users can type them any way, and registry urls sometimes use mixed case. Without normalization, a config entry of \"ghsa-abcd-...\" would not match the uppercase id derived from the advisory url, silently leaving that advisory unignored. - deriveGithubAdvisoryId returns the matched id in uppercase. - ignore writes canonicalized ids (upper + trim) to auditConfig.ignoreGhsas and normalizes what it reads from there. - audit's ignoreGhsas filter and fix.getFixableAdvisories compare both sides uppercased, so pre-existing lower/mixed-case config entries still match. - Snapshots and ignore.ts tests updated to the canonical upper form.
GHSA identifiers on github.com are written with an uppercase prefix and a lowercase hexadecimal suffix (e.g. GHSA-cph5-m8f7-6c5x). Previously we forced the whole id to uppercase, which didn't match the external convention and looked off in pnpm-workspace.yaml. Introduce normalizeGhsaId in @pnpm/deps.compliance.audit that uppercases the \`GHSA-\` prefix and lowercases everything after. Use it in deriveGithubAdvisoryId, in ignore.ts for both reads and writes, and in audit.ts + fix.ts for the Set comparisons. Fixture tests adjusted to the canonical form and snapshot regenerated.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 42 changed files in this pull request and generated 1 comment.
Files not reviewed (2)
- deps/compliance/commands/test/audit/fixtures/preserve-reference-overrides/pnpm-lock.yaml: Language not supported
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
collectOptionalOnlyDepPaths always treated importer.devDependencies as non-optional roots. When pnpm audit --prod excludes dev deps from the walker, a package reachable only via a devDependency subgraph and via an optionalDependency chain would still be classified as "reachable without optional" (because of the dev chain) and therefore NOT marked as optional-only. Under --prod that's wrong — the dev chain isn't in the audited graph. Thread `include` into collectOptionalOnlyDepPaths and build the root sets from the same flags the walker uses. Callers that don't pass include keep the previous behaviour (include everything). Test added in deps/compliance/audit/test/index.ts exercises both the with-dev and --prod cases against a lockfile where the same package is reachable via a devDependency and an optionalDependency root; the optional flag flips correctly depending on include.devDependencies.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 42 changed files in this pull request and generated 1 comment.
Files not reviewed (2)
- deps/compliance/commands/test/audit/fixtures/preserve-reference-overrides/pnpm-lock.yaml: Language not supported
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The test was skipped because DEV_VULN_ONLY_RESP contains high and
critical advisories, so --audit-level=high would always match something
and never exercise the "exit 0" path. The assertion also had outdated
grammar and an expected summary that didn't match reportSummary's
current output shape.
Replace the fixture with an inline one-advisory moderate response so
--audit-level=high filters to zero, drop the .skip, rename the test,
and assert the real summary output ("1 vulnerabilities found\nSeverity:
1 moderate"). All 34 audit tests now pass with none skipped.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 38 out of 42 changed files in this pull request and generated no new comments.
Files not reviewed (2)
- deps/compliance/commands/test/audit/fixtures/preserve-reference-overrides/pnpm-lock.yaml: Language not supported
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
core/types/src/misc.ts:105
- Adding
'info'toVulnerabilitySeveritychanges the union type used outside the audit command (e.g.installing/commands/src/installDeps.tsmaps severities to numeric weights and currently has no'info'branch). This will treatinfoas an unknown/low severity in those flows, which can skew comparisons and version selection. Please update the severity-to-number/penalty mappings to handle'info'explicitly (as lower thanlow) anywhereVulnerabilitySeverityis switched over.
export type VulnerabilitySeverity =
| 'info'
| 'low'
| 'moderate'
| 'high'
| 'critical'
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Congrats on merging your first pull request! 🎉🎉🎉 |
|
This really needs to be backported into v10.33 ASAP, because the registry error is breaking both CI and local environments for a number of our teams right now, and there needs to be a better way of fixing it than upgrading to an RC or using dlx. |
The legacy
/-/npm/v1/security/audits{,/quick}endpoints have been retired by npmjs.org. This PR rewires the audit client to the replacement/-/npm/v1/security/advisories/bulkendpoint.The new endpoint is not a drop-in rename — the request and response contracts are both different:
{ pkgName: [versions] }map.lockfileToAuditRequestwalks the lockfile once and builds the POST body directly; there is no more nestedAuditTree.id,url,title,severity,vulnerable_versions, andcweper advisory. Everything else the old endpoint returned is computed locally:findings[].pathsare walked from the lockfile (skipped entirely when the response is empty; the second walk intentionally avoids@pnpm/lockfile.walker's global dedup so alternate install chains to the same shared dep aren't dropped).metadata.vulnerabilitiescounts advisories per severity.metadata.dependencies/devDependencies/optionalDependencies/totalDependenciescome from a classified lockfile walk; the classifier respects--prod/--devinclude flags when deciding whether a subgraph is reachable non-optionally.patched_versionsis inferred from the vulnerable range for common<X.Y.Z/<=X.Y.Zshapes soaudit --fixcan still produce usable overrides; leftundefinedwhen inference fails.github_advisory_idis parsed from the advisory URL and canonicalized to the github.com form (uppercaseGHSA-prefix, lowercase suffix).infoseverity is now supported end-to-end (severity type,--audit-level, filters, colors).Breaking changes (v11)
/advisories/bulknow fail withAuditEndpointNotExistsError.auditConfig.ignoreCves→auditConfig.ignoreGhsas(the old key is no longer recognized).pnpm audit --ignore <id>and--ignore-unfixablenow read and write GHSAs.CVE-YYYY-NNNNNinauditConfig.ignoreCveswith the matchingGHSA-xxxx-xxxx-xxxx(visible in theMore infocolumn ofpnpm auditoutput) underauditConfig.ignoreGhsas.--ignore-unfixablenow only targets advisories whose patched range couldn't be inferred — the only "no fix available" signal the bulk endpoint provides.AuditReportandAuditAdvisoryare trimmed to just the fields the audit client actually populates:AuditReport:advisories+metadataonly (actionsandmutedremoved).AuditAdvisory:findings,id,title,module_name,vulnerable_versions,patched_versions?,severity,cwe,github_advisory_id,url. Dropped:cves,created,updated,deleted,access,overview,recommendation,references,found_by,reported_by,metadata.AuditAction,AuditResolution,AuditActionRecommendationremoved (no consumers).Hardening
ERR_PNPM_AUDIT_BAD_RESPONSEwith a body excerpt. Advisoryidmust be a finite number andseveritymust be a known value before being indexed.Object.create(null)so a hostile/unusual package name can't trigger prototype pollution.findings[].pathsare deduped and capped per (name, version) to keep pathologically shared graphs from blowing up memory.Internals
AuditTree/AuditNode/lockfileToAuditTreeremoved.lockfileToAuditIndex.tsexportslockfileToAuditRequest(flat POST body + counts) andbuildAuditPathIndex(only invoked when the response has advisories).AuditAdvisory.findingsis nowAuditFinding[](was an unintended 1-tuple).registry.npmjs.orgresponses; syntheticupdate-*fixtures converted in place to bulk shape.