fix: support npm new bulk audit#8671
Conversation
There was a problem hiding this comment.
The legacy endpoint is gone. Support/code should be removed, not leaving it around for the project to maintain, and certainly not deriving URLs. Makes no reference to how it was tested, or any semantic differences to the API - leaving all work entirely to the reviewer to figure out. No understanding of the existing project is demonstrated here.
This PR is likely entirely AI generated, with some absolutely terrible logic embedded to hack versions - and seemingly done without even demonstrating basic respect for the human concerns in the linked issue (created by me).
-1 from me.
As I said on the linked issue, I do not support this approach (without at least some kind of actual justification/research), and definitely don't support drive-by PRs which increase, rather than decrease, maintenance burden.
|
Yes, I was just trying to see how the pipeline works here, I am frontend dev did not wanted to run all the test suite on my side. I generated this with AI of course and asked explicitely to fallback to the old registry and not try to fix Yarn1 just solve NPM issue the bulk endpoint. I can remove any fallback just wanted to make it work on fork to see if my npm projects works. Is anyone else with more experience on this Project trying to do such a fix? Can you point me to any PR where this is being discussed or done? Anyway I just wanted to help here without having not that much context here on this repo at all. Anyway, do you foresees 1 MR to fix NPM dropping the custom HTTP integration and using the npm CLI (like PnpmAuditAnalyzer / Yarn Berry) ? is there any work around it if the support to custom http integration is going to not be there. Thank you |
|
You can create a PR to master on your own fork to run the tests/build, just like for every project on GitHub - don't open experimental PRs on open source projects for this. When you spam the upstream someone has to review your code to see if it's safe to run the tests. There is some discussion on the very issue your PR links. The problem is that I am not a maintainer, just another contributor, and there is no input from maintainers yet (but nor have I, or anyone, summarised a proposal cleanly). It's really not difficult to use an LLM to create a naive PR from a couple sentences of text - that would take little time. It's the "other stuff" in engineering that actually takes time, not the typing - validating the approach, weighing trade-offs, deciding if it's worth any complexity it introduces, figuring out what to do with other anaiyzers which are using the same logic so we can delete it, determining if the test coverage is good enough to validate, deciding how to approach testing to ensure things don't break over time etc. |
Description of Change
The npm registry retired the legacy Node Audit API (
/-/npm/v1/security/audits), which now returns HTTP 410 Gone with a message to use the bulk advisory endpoint instead. This caused the Node Audit Analyzer to fail and be disabled during scans, leading to false negatives for npm/package-lock projects.This change updates the Node Audit Analyzer to use the npm bulk advisory API (
/-/npm/v1/security/advisories/bulk) as the primary lookup path, with automatic fallback to the legacy endpoint when the bulk API is unavailable or cannot be derived from the configured URL.Key changes:
analyzer.node.audit.urlsetting when it ends with/-/npm/v1/security/auditssubmitPackageWithBulkFallback()to try the bulk endpoint first, then fall back to the legacy audit APINpmPayloadBuilder.buildBulk(), normalizing exact installed versions and excluding ranges, aliases, and local/workspace referencesNpmAuditParser.parseBulk()Related issues
Have test cases been added to cover the new functionality?
yes