Repository navigation
fix(overrides): make OA010 reachable from the scan path and disclose rules that could not run - #1262
Merged
Merged
Conversation
audit() populated skippedDetectors in five places, had ten detectors honouring it, and then never returned it, so a rule that could not run was indistinguishable from a rule that ran and found nothing. The recording was also only in catch blocks, so the common case, the network opt-in simply being absent, recorded nothing at all. That is now recorded, with a separate reason when the opt-in is present but no advisory source was wired, since that one is ours to fix rather than the user's. Nothing is recorded when the manifest declares no overrides, matching the guard the lockfile and node_modules skips already use: there is no audit to be incomplete. The batch registry fetchers now report which lookups failed instead of dropping the names, so a total failure records a skip rather than leaving OA007 and OA010 reporting nothing. A 404 is classified as the package being absent rather than a failed lookup, because this client only talks to registry.npmjs.org and every private scoped package in an overrides block would otherwise read as a network problem. skippedDetectors.ruleId is now the OverrideRuleId union rather than a bare string: detectors self-suppress by matching their own id against the list, so a typo would compile and then silently never suppress or render anything.
The flag only existed on the overrides subcommand, so the documented scan command rejected it as an unknown option and OA007 and OA010 could never run from the path we document everywhere. Two guards come with it. Without --check-overrides the flag does nothing, so it is refused rather than silently ignored. And both offline flags are refused alongside it, because the network-backed rules query the npm registry for dist-tags and published version lists, which no local advisory database can supply; honouring it only for the advisory source would print "no external advisory calls will be made" and then call the registry anyway. requiresMessage joins conflictMessage so the two shapes of flag error stay consistent, and the existing --allow-private-osv-url message now uses it. --check-network is added to the scan flag list in --help, where it was missing, and the subcommand help no longer documents a conflict that cannot occur there.
…l three callers The scan path threaded the network opt-in through but never built an advisory source, so OA010 still could not run even once the flag was accepted. The single folder scan, the multi-folder scan and the overrides subcommand each wired the audit up by hand, which is how that drift happened. runOverrideAudit is now the one place it is wired. It builds the context, builds the advisory source when the opt-in is present, closes it in a finally, and forwards the cert-aware fetch into the registry calls so a CA certificate set through `config set ca-cert` applies to OA007 and OA010; NODE_EXTRA_CA_CERTS is only read at process start, which is why it has to be threaded explicitly. Multi-folder was holding a fetch implementation and not passing it, and the overrides subcommand was getting neither that nor a debug logger, on the one surface that accepts --debug and the first place a TLS problem gets investigated. Both now do.
…rface A clean bill of health from an audit that skipped a rule is worse than no audit, so the skips now reach the terminal, --json, both HTML reports and SARIF, and are scoped per folder in multi-folder mode. Grouped by reason rather than keyed on rule ids, so it covers the lockfile and node_modules skips as well as the network-backed ones. The headline sentence and the grouping live in one module because they had already drifted to two different pluralisations when they lived in two. The all-clear is qualified when rules were skipped, since an unqualified tick is the line a skimming reader takes away. In --json the field is present whenever override checking ran and absent when it did not, so an empty array means every rule ran rather than being indistinguishable from never having audited. CI and the GitHub App read that file. SARIF puts them in invocations[].toolExecutionNotifications, with a descriptor declared on the driver for every id referenced, since a notification's descriptor resolves against driver.notifications and not driver.rules. Emitted only when there is something to report, so a plain scan's SARIF is unchanged.
…needs it The finding printed `cve-lite <path> overrides --fix --rule OA010`, and running that reported no issues followed by a notice that OA010 had not run. Now that OA010 is reachable from the main scan the contradiction is visible rather than theoretical.
Each of these kills a mutation that previously left the whole suite green. The advisory source could stop being built, which is the original bug, and nothing failed. The notice could be deleted from the terminal, from the JSON writer, from the HTML report data, from the multi-folder JSON, from the overrides subcommand or from either HTML reporter, and nothing failed. The cert-aware fetch could stop being forwarded, the custom OSV url could stop being passed, and the stylesheet rule for the HTML notice could be removed, and nothing failed. tests/override-audit.test.ts exercises the module the fix lives in, which nothing imported before: the two suites that named it mocked it away. Three existing suites were retargeted because the shared helper moved the seam they mock at, and two tests that were quietly reaching the real npm registry now use an injected fetch.
The CLI reference described it as OA007 only, omitted OA010, and showed it only under the overrides subcommand. It now names both rules, says it is accepted on a plain scan alongside --check-overrides, and says both rules are reported as skipped without it. The Action table previously implied the flag works on the inline check-overrides path. It does not: action.yml only reads it in the dedicated audit step, so it needs overrides: "true". Wiring it into the scan step is a separate change.
…uilder #1176 moved the --json and report.json payloads into src/output/scan-json.ts after this branch was written, so the field had to move with them. The single-folder report.json call is gated on --check-overrides while the overrideFindings line beside it is not. That asymmetry is deliberate: an absent skippedOverrideRules means override checking never ran and an empty array means it ran with every rule running, and passing it unconditionally collapses those two. Verified on a real report.json, present with the flag and absent without it. overrideSkipped is optional on MultiFolderScanJsonFolder because that type structurally duplicates MultiFolderScanResult rather than projecting it, so a required member would not be enforced at the call sites and would only break fixtures written before the field existed.
sonukapoor
force-pushed
the
bugfix/issue-1222-check-network-on-scan-path
branch
from
October 2, 2026 12:07
c7b6b3a to
be9dd2c
Compare
Contributor
|
Heads-up: I'm picking up #1244 (replace the SARIF entry points' eight positional Once this lands I'll branch from updated |
This was referenced Oct 3, 2026
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OA010 could only be reached through
cve-lite overrides <path> --check-network. On the documented scan path the flag was rejected as an unknown option, so OA007 and OA010 never ran andcve-lite . --check-overridesreported a clean bill of health without them. Skipped rules were never surfaced anywhere either.Measured on CopilotKit, a real pnpm monorepo with 69 overrides: on
mainthe scan path rejects the flag outright and mentions OA010 zero times. With this change it returns the same 109 findings as the subcommand, including 20 OA010 findings, compared finding by finding with zero differences in either direction. Those 20 are genuinely stale floors onminimatch,vite,esbuild,basic-ftp,brace-expansionand others.Three things changed, in that order of importance.
The flag is accepted on the scan path, and the scan path now builds the advisory source OA010 needs. It threaded the opt-in through before but never built one, so the rule still could not run. All three entry points, single folder, multi-folder and the subcommand, now go through one
runOverrideAudit, which is also what gets the cert-aware fetch to the registry calls;NODE_EXTRA_CA_CERTSis only read at process start.A rule that cannot run says so, across the terminal,
--json, both HTML reports and SARIF, scoped per folder in multi-folder mode. Grouped by reason rather than rule id, so it covers thenode_modulesand lockfile skips too. Across our own examples that surfaces 100 previously silentnode_modules missingskips on 25 projects.And a total registry failure is no longer silent. The batch fetchers report which lookups failed, so behind a TLS proxy OA007 and OA010 say they saw nothing instead of reporting a clean result. A 404 is classified as the package being absent rather than a failed lookup, since this client only talks to registry.npmjs.org and every private scoped package would otherwise read as a network problem.
Two deliberate calls worth flagging. Both offline flags are refused alongside
--check-network, because the rules query the registry and no local database can answer that; honouring it halfway printed "no external advisory calls will be made" and then called npm. And only a total registry failure records a skip, because a skip entry makes the detector suppress itself, so recording a partial failure would discard real findings. The partial case stays silent and wants its own channel.Closes #1222