From f74a0c6e7c031ebab0674f708b1231ea1f3cb7d5 Mon Sep 17 00:00:00 2001 From: Cole Tebou Date: Wed, 10 Jun 2026 16:42:36 +0900 Subject: [PATCH] feat(review): verify bounded npm publication claims --- CHANGELOG.md | 1 + docs/code-review.md | 40 ++ docs/configuration.md | 10 + docs/spec.md | 3 + src/app.test.ts | 12 +- src/app.ts | 44 ++- src/cli.ts | 9 + src/config.ts | 3 + src/provider.ts | 14 +- src/registry-verifier.test.ts | 725 ++++++++++++++++++++++++++++++++++ src/registry-verifier.ts | 446 +++++++++++++++++++++ src/review-validation.ts | 64 ++- src/types.ts | 13 + 13 files changed, 1373 insertions(+), 11 deletions(-) create mode 100644 src/registry-verifier.test.ts create mode 100644 src/registry-verifier.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index f89f2e2a..d55583b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## 0.5.1 - Unreleased +- Added opt-in npm registry verification that drops only matching single-package, whole-title-and-reasoning public-npm publication claims when the exact version is confirmed published, thanks @coletebou. - Fixed revalidation to include linked patch attempts, validation results, feature context, and current relevant files so repaired findings can move out of `uncertain`. - Added `clawpatch review --feature-list ` for reviewing an explicit ordered, de-duplicated set of feature IDs, thanks @camwest. diff --git a/docs/code-review.md b/docs/code-review.md index 9a5ebb57..138c655a 100644 --- a/docs/code-review.md +++ b/docs/code-review.md @@ -145,3 +145,43 @@ CUDA-specific category. Deslopify mode is unaffected. Review does not edit files. Use `clawpatch fix --finding ` for the explicit patch loop. + +## Registry verifier + +After per-finding evidence validation, review can run an opt-in npm-registry +verifier. Findings whose entire title and reasoning both state the same bounded +`pkg@semver` publication claim such as `mongodb@7.0.0 is unpublished on npm` +get resolved against +`https://registry.npmjs.org/{name}/{version}`. When the registry confirms +the version is published, the finding is partitioned into +`droppedFindings` with `layer: "registry-verifier"` instead of being +surfaced as a real finding. + +This addresses a recurring failure mode where providers backed by an LLM +with a fixed knowledge cutoff confidently flag post-cutoff package +versions as nonexistent. (See _We Have a Package for You!_ — Spracklen et +al., USENIX Security 2025, [arXiv:2406.10279][slop-paper] — for measured +hallucination rates of the symmetric failure: invented package names. +The registry-grounded mitigation is the same.) + +The verifier is intentionally biased toward keeping findings: + +| Registry response | Verdict | Action | +| -------------------------------------- | -------------------- | ------------ | +| 200 with matching `name` AND `version` | `verified-published` | drop finding | +| 404 | `verified-missing` | keep finding | +| 5xx, transport error, timeout | `unknown` | keep finding | +| 200 with non-JSON content-type | `unknown` | keep finding | +| 200 with mismatched body name/version | `unknown` | keep finding | +| 200 with body > 1 MiB | `unknown` | keep finding | +| Any redirect (`redirect: "error"`) | `unknown` | keep finding | + +Failure of the verifier never creates a false negative — only refutable +single-package claims drop. Compound, multi-package, or context-disagreeing findings are always kept. The verifier is +disabled by default because it sends package +coordinates to the public npm registry. Enable it explicitly with +`registryVerifier.enabled = true` in `.clawpatch/config.json`; the +`--no-registry-verify` flag can still disable it for a single run. Within a single review run it +deduplicates registry calls per `(name, version)`. + +[slop-paper]: https://arxiv.org/abs/2406.10279 diff --git a/docs/configuration.md b/docs/configuration.md index d41dca63..5857ac3c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -51,10 +51,20 @@ Default shape: "requireCleanWorktreeForFix": true, "commit": false, "openPr": false + }, + "registryVerifier": { + "enabled": false } } ``` +`registryVerifier.enabled` controls the npm-registry post-validator that +drops direct `pkg@semver` public-npm publication claims refuted by +the public npm registry. It is disabled by default because lookups disclose +package coordinates; set it to `true` only when that network access is acceptable. See +[Code review > Registry verifier](code-review.md#registry-verifier) for +the full verdict matrix. + Environment overrides: - `CLAWPATCH_STATE_DIR` diff --git a/docs/spec.md b/docs/spec.md index e2bd83e5..057bc360 100644 --- a/docs/spec.md +++ b/docs/spec.md @@ -440,6 +440,9 @@ Initial config: "requireCleanWorktreeForFix": true, "commit": false, "openPr": false + }, + "registryVerifier": { + "enabled": false } } ``` diff --git a/src/app.test.ts b/src/app.test.ts index e57e44ba..5035730e 100644 --- a/src/app.test.ts +++ b/src/app.test.ts @@ -1,10 +1,12 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { __testing as appTesting, AppContext } from "./app.js"; +import { defaultConfig } from "./config.js"; import { ClawpatchError } from "./errors.js"; import type { ReviewOutput } from "./types.js"; // eslint-disable-next-line no-underscore-dangle -const { isRetryableReviewError, reviewRetries, runProviderReviewWithRetry } = appTesting; +const { isRetryableReviewError, reviewFlagSubset, reviewRetries, runProviderReviewWithRetry } = + appTesting; const QUIET_CONTEXT: AppContext = { root: "/tmp/test-root", @@ -24,6 +26,14 @@ function emptyReview(): ReviewOutput { return { findings: [], inspected: { files: [], symbols: [], notes: ["ok"] } }; } +it("forwards the registry-verifier opt-out into CI review flags", () => { + expect(reviewFlagSubset({ noRegistryVerify: true })).toEqual({ noRegistryVerify: true }); +}); + +it("keeps public registry verification opt-in", () => { + expect(defaultConfig().registryVerifier.enabled).toBe(false); +}); + function withEnv(name: string, value: string | undefined, fn: () => void): void { const previous = process.env[name]; if (value === undefined) { diff --git a/src/app.ts b/src/app.ts index 839e1a6f..eaa0bdf4 100644 --- a/src/app.ts +++ b/src/app.ts @@ -32,7 +32,12 @@ import { renderFindingDetail, renderReport, } from "./reporting.js"; -import { validateReviewOutputPartitioned } from "./review-validation.js"; +import { + buildRegistryVerifierValidator, + validateReviewOutputPartitioned, + type FindingPostValidator, + type ValidatePartitionedOptions, +} from "./review-validation.js"; import { filterFeaturesByChangedFiles, filterFeaturesByProject, @@ -320,6 +325,9 @@ export async function reviewCommand( const limiter = createRpmLimiter( rpmFromFlag(stringFlag(flags, "rateLimitPerMinute"), process.env["CLAWPATCH_RPM"]), ); + const registryPostValidator = config.registryVerifier.enabled + ? buildRegistryVerifierValidator() + : undefined; let cursor = 0; emitProgress(context, "review", "start", { run: currentRunId, @@ -348,13 +356,19 @@ export async function reviewCommand( mode, customPrompt, limiter, + registryPostValidator, allowNonPendingFeatureReview: stringFlag(flags, "feature") !== undefined || stringFlag(flags, "featureList") !== undefined, }); findingIds.push(...reviewed.findingIds); for (const dropped of reviewed.droppedFindings) { - const code = dropped.layer === "validation" ? "validation-drop" : "schema-drop"; + const code = + dropped.layer === "validation" + ? "validation-drop" + : dropped.layer === "registry-verifier" + ? "registry-verifier-drop" + : "schema-drop"; errors.push({ message: `dropped 1 finding from feature ${feature.featureId} ` + @@ -374,7 +388,10 @@ export async function reviewCommand( }), ); const fatalErrors = errors.filter( - (entry) => entry.code !== "schema-drop" && entry.code !== "validation-drop", + (entry) => + entry.code !== "schema-drop" && + entry.code !== "validation-drop" && + entry.code !== "registry-verifier-drop", ); if (fatalErrors.length > 0) { await writeRun(loaded.paths, { @@ -660,6 +677,7 @@ type ReviewFeatureOptions = { mode: ReviewMode; customPrompt: string | null; limiter: RpmLimiter; + registryPostValidator: FindingPostValidator | undefined; allowNonPendingFeatureReview: boolean; }; @@ -678,6 +696,7 @@ async function reviewFeature( mode, customPrompt, limiter, + registryPostValidator, allowNonPendingFeatureReview, } = options; const started = Date.now(); @@ -729,12 +748,19 @@ async function reviewFeature( // Layer 2 drops: per-finding evidence validation (line ranges, quotes, // included files). Partition so a single bad finding doesn't lose the // whole feature. + // Layer 3 drops (optional): registry verifier rejects findings whose + // "package X@Y is unpublished" claim is refuted by the npm registry. + const validatePartitionedOptions: ValidatePartitionedOptions = {}; + if (registryPostValidator !== undefined) { + validatePartitionedOptions.postValidator = registryPostValidator; + } const validated = await validateReviewOutputPartitioned( loaded.root, lockedFeature, config, reviewPrompt.manifest, reviewOutput, + validatePartitionedOptions, ); droppedFindings.push(...validated.droppedFindings); const records = validated.findings.map((finding) => @@ -1410,6 +1436,14 @@ function applyProviderFlags( reasoningEffort: reasoningEffort ?? config.provider.reasoningEffort, skipGitRepoCheck: flags["skipGitRepoCheck"] === true, }, + registryVerifier: { + ...config.registryVerifier, + // CLI flag is one-way: --no-registry-verify forces off, but absence + // of the flag preserves whatever config.json says. This matches the + // negative-flag convention (`--no-color`, `--no-input`) used + // elsewhere in the CLI surface. + enabled: flags["noRegistryVerify"] === true ? false : config.registryVerifier.enabled, + }, }; } @@ -1442,6 +1476,9 @@ function reviewFlagSubset( if (flags["includeDirty"] === true) { subset["includeDirty"] = true; } + if (flags["noRegistryVerify"] === true) { + subset["noRegistryVerify"] = true; + } return subset; } @@ -2229,6 +2266,7 @@ function stringFlag(flags: Record, name: string): stri // eslint-disable-next-line no-underscore-dangle export const __testing = { isRetryableReviewError, + reviewFlagSubset, reviewRetries, runProviderReviewWithRetry, }; diff --git a/src/cli.ts b/src/cli.ts index 635d2912..d613f914 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -171,6 +171,7 @@ const commandFlags = { "promptFile", "exportTribunalLedger", "includeDirty", + "noRegistryVerify", ]), ci: new Set([ "limit", @@ -183,6 +184,7 @@ const commandFlags = { "skipGitRepoCheck", "output", "includeDirty", + "noRegistryVerify", ]), report: new Set(["status", "severity", "feature", "project", "category", "triage", "output"]), show: new Set(["finding"]), @@ -262,6 +264,7 @@ const booleanFlagNames = new Set([ "all", "draft", "include-dirty", + "no-registry-verify", ]); const shortFlagNames = new Set(["-h", "-q", "-v", "-o"]); @@ -454,6 +457,11 @@ Flags: JSONL file with one line per finding shaped for downstream Tribunal-style signed-ledger ingest. Opt-in; no effect when omitted. + --no-registry-verify disable a configured npm-registry post-validator that + drops findings whose "package X@Y is + unpublished" claim is refuted by the registry. + Set registryVerifier.enabled=true in config.json + to opt in; this flag disables it for one run. --json -q, --quiet `); @@ -494,6 +502,7 @@ Flags: --reasoning-effort --skip-git-repo-check --output + --no-registry-verify see clawpatch review --help for details --json `); return; diff --git a/src/config.ts b/src/config.ts index c6723f29..28037277 100644 --- a/src/config.ts +++ b/src/config.ts @@ -60,6 +60,9 @@ export function defaultConfig(): ClawpatchConfig { commit: false, openPr: false, }, + registryVerifier: { + enabled: false, + }, }; } diff --git a/src/provider.ts b/src/provider.ts index 2dc60b5a..e163a06f 100644 --- a/src/provider.ts +++ b/src/provider.ts @@ -157,16 +157,20 @@ export type ProviderOptions = { * One review finding rejected by per-finding validation. `layer` records * which gate dropped it: `schema` is the per-finding `reviewFindingSchema` * Zod parse, `validation` is the evidence/quote/line-range check in - * `validateReviewOutputPartitioned`. Operators can use `layer` to tell - * "the model emitted nonsense for this finding" (schema) apart from - * "the model cited a real-looking finding but pointed at the wrong file - * or quoted text that isn't there" (validation). + * `validateReviewOutputPartitioned`, and `registry-verifier` is the + * post-validation pass that drops findings whose central claim about a + * package version's nonexistence is refuted by the npm registry. + * Operators can use `layer` to tell these apart: "the model emitted + * nonsense" (schema), "the model cited a real-looking finding but quoted + * text that isn't there" (validation), or "the model asserted a package + * version is unpublished but the registry says otherwise" + * (registry-verifier). */ export type DroppedFinding = { path: (string | number)[]; message: string; sample: string; - layer?: "schema" | "validation"; + layer?: "schema" | "validation" | "registry-verifier"; }; export type PartitionedReviewOutput = { diff --git a/src/registry-verifier.test.ts b/src/registry-verifier.test.ts new file mode 100644 index 00000000..7264b7d2 --- /dev/null +++ b/src/registry-verifier.test.ts @@ -0,0 +1,725 @@ +import { describe, expect, it } from "vitest"; +import { + evaluateFindingForDrop, + extractPackageSpecs, + findingClaimsNonexistence, + verifyPackageSpec, + type PackageSpec, + type RegistryVerdict, +} from "./registry-verifier.js"; + +describe("findingClaimsNonexistence", () => { + it("matches the canonical nonexistence phrasings", () => { + expect(findingClaimsNonexistence("vitest@4.0.16 does not exist on npm")).toBe(true); + expect(findingClaimsNonexistence("react@19.2.4 is not published on npm")).toBe(true); + expect(findingClaimsNonexistence("@aws-sdk/client-s3@3.1000.0 is unpublished on npm")).toBe( + true, + ); + expect(findingClaimsNonexistence("mongodb@7.0.0 has ETARGET on public npm")).toBe(true); + expect(findingClaimsNonexistence("Package mongodb@7.0.0 does not exist on npm")).toBe(true); + expect(findingClaimsNonexistence("Pinned mongodb@7.0.0 is unpublished on npm")).toBe(true); + }); + + it("rejects findings that merely mention the words in unrelated contexts", () => { + expect(findingClaimsNonexistence("Race condition allows duplicate message processing")).toBe( + false, + ); + expect(findingClaimsNonexistence("Authorization check missing on /internal route")).toBe(false); + expect( + findingClaimsNonexistence("Counter starts at zero and never increments existing rows"), + ).toBe(false); + expect(findingClaimsNonexistence("Vitest 4.0.16 is unreleased")).toBe(false); + expect(findingClaimsNonexistence("invalid version pinned 11.1.3")).toBe(false); + expect(findingClaimsNonexistence("react@19.2.4 doesn't exist in package-lock.json")).toBe( + false, + ); + expect( + findingClaimsNonexistence("react@19.2.4 does not exist in the configured private registry"), + ).toBe(false); + expect( + findingClaimsNonexistence( + "@acme/widget@1.2.3 is unpublished from the configured GitHub Packages registry", + ), + ).toBe(false); + expect(findingClaimsNonexistence("README says foo@1.2.3 is unpublished on npm")).toBe(false); + expect( + findingClaimsNonexistence( + "foo@1.2.3 does not exist in package-lock.json but is available on npm", + ), + ).toBe(false); + expect(findingClaimsNonexistence("foo@1.2.3 does not exist in npm cache")).toBe(false); + expect( + findingClaimsNonexistence( + "foo@1.2.3 does not exist in npm and the lockfile integrity is corrupted", + ), + ).toBe(false); + expect( + findingClaimsNonexistence( + "foo@1.2.3 is unpublished on npm. Its install script also leaks credentials", + ), + ).toBe(false); + expect( + findingClaimsNonexistence( + "foo@1.2.3 runs a credential-stealing install script and is unpublished on npm", + ), + ).toBe(false); + }); + + it("rejects 'reads currentUserId from a non-existent field' (real bug, not a version claim)", () => { + expect( + findingClaimsNonexistence( + "WatchedActivities reads currentUserId from a non-existent field on AppUser", + ), + ).toBe(false); + }); +}); + +describe("extractPackageSpecs", () => { + it("extracts bare-name specs", () => { + expect(extractPackageSpecs("mongodb@7.0.0 is not on npm")).toEqual([ + { name: "mongodb", version: "7.0.0" }, + ]); + }); + + it("extracts scoped-name specs", () => { + expect(extractPackageSpecs("uses @types/node@24.10.4 which doesn't exist")).toEqual([ + { name: "@types/node", version: "24.10.4" }, + ]); + }); + + it("extracts deeply-scoped AWS SDK packages", () => { + expect(extractPackageSpecs("pinned @aws-sdk/client-kms@3.1000.0")).toEqual([ + { name: "@aws-sdk/client-kms", version: "3.1000.0" }, + ]); + }); + + it("extracts prerelease versions", () => { + expect(extractPackageSpecs("react@19.0.0-rc.1 is unreleased")).toEqual([ + { name: "react", version: "19.0.0-rc.1" }, + ]); + }); + + it("extracts versions with build metadata", () => { + expect(extractPackageSpecs("pkg@1.2.3+sha.abcd0123 is invalid")).toEqual([ + { name: "pkg", version: "1.2.3+sha.abcd0123" }, + ]); + }); + + it("dedupes repeated specs in document order", () => { + expect( + extractPackageSpecs("mongodb@7.0.0 is invalid. Also mongodb@7.0.0. And vitest@4.0.16."), + ).toEqual([ + { name: "mongodb", version: "7.0.0" }, + { name: "vitest", version: "4.0.16" }, + ]); + }); + + it("ignores trailing punctuation", () => { + expect(extractPackageSpecs("mongodb@7.0.0, vitest@4.0.16.")).toEqual([ + { name: "mongodb", version: "7.0.0" }, + { name: "vitest", version: "4.0.16" }, + ]); + }); + + it("ignores partial / loose phrasings (mongodb 7.0)", () => { + expect(extractPackageSpecs("mongodb 7.0 might not exist")).toEqual([]); + expect(extractPackageSpecs("uses mongodb at version 7")).toEqual([]); + }); + + it("ignores email-shaped tokens", () => { + expect(extractPackageSpecs("contact ops@example.com if pinned wrong")).toEqual([]); + }); + + it("returns no specs when text contains none", () => { + expect(extractPackageSpecs("some unrelated finding text")).toEqual([]); + expect(extractPackageSpecs("")).toEqual([]); + }); +}); + +type FetchInput = Parameters[0]; + +describe("verifyPackageSpec", () => { + function makeFetch(implementations: Array<(input: FetchInput) => Promise>) { + let call = 0; + return ((input: FetchInput) => { + const handler = implementations[call]; + call += 1; + if (!handler) { + throw new Error(`unexpected fetch call #${call}`); + } + return handler(input); + }) as typeof fetch; + } + + it("returns verified-published when registry returns 200 with matching name+version", async () => { + const fetchImpl = makeFetch([ + async () => + new Response(JSON.stringify({ version: "7.0.0", name: "mongodb" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ]); + const verdict = await verifyPackageSpec({ name: "mongodb", version: "7.0.0" }, { fetchImpl }); + expect(verdict).toEqual({ + kind: "verified-published", + name: "mongodb", + version: "7.0.0", + }); + }); + + it("returns unknown when 200 body's name does not match the requested spec (mirror trickery defense)", async () => { + const fetchImpl = makeFetch([ + async () => + new Response(JSON.stringify({ name: "different-pkg", version: "7.0.0" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ]); + const verdict = await verifyPackageSpec({ name: "mongodb", version: "7.0.0" }, { fetchImpl }); + expect(verdict.kind).toBe("unknown"); + }); + + it("returns unknown when 200 response has non-JSON content-type (HTML proxy login etc.)", async () => { + const fetchImpl = makeFetch([ + async () => + new Response("SSO sign-in", { + status: 200, + headers: { "Content-Type": "text/html; charset=utf-8" }, + }), + ]); + const verdict = await verifyPackageSpec({ name: "mongodb", version: "7.0.0" }, { fetchImpl }); + expect(verdict.kind).toBe("unknown"); + }); + + it("returns unknown when content-length advertises a body larger than the cap", async () => { + const fetchImpl = makeFetch([ + async () => + new Response("{}", { + status: 200, + headers: { + "Content-Type": "application/json", + "Content-Length": String(10 * 1024 * 1024), // 10 MiB advertised + }, + }), + ]); + const verdict = await verifyPackageSpec({ name: "mongodb", version: "7.0.0" }, { fetchImpl }); + expect(verdict.kind).toBe("unknown"); + }); + + it("sends a User-Agent that identifies clawpatch", async () => { + let seenUa: string | null = null; + const fetchImpl = ((input: FetchInput, init?: RequestInit) => { + const headers = new Headers(init?.headers); + seenUa = headers.get("User-Agent"); + void input; + return Promise.resolve( + new Response(JSON.stringify({ name: "mongodb", version: "7.0.0" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + }) as typeof fetch; + await verifyPackageSpec({ name: "mongodb", version: "7.0.0" }, { fetchImpl }); + expect(seenUa).toMatch(/clawpatch/iu); + }); + + it("does not follow redirects (treats 302 as unknown)", async () => { + const fetchImpl = ((_input: FetchInput, init?: RequestInit) => { + // Node's global fetch with redirect:"error" rejects synchronously + // when it sees a redirect; simulate that path explicitly. + void init; + return Promise.reject(new TypeError("unexpected redirect")); + }) as typeof fetch; + const verdict = await verifyPackageSpec({ name: "mongodb", version: "7.0.0" }, { fetchImpl }); + expect(verdict.kind).toBe("unknown"); + }); + + it("returns unknown when 200 body has version but no name (defensive against partial mirror responses)", async () => { + const fetchImpl = makeFetch([ + async () => + new Response(JSON.stringify({ version: "7.0.0" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ]); + const verdict = await verifyPackageSpec({ name: "mongodb", version: "7.0.0" }, { fetchImpl }); + expect(verdict.kind).toBe("unknown"); + }); + + it("propagates a caller-supplied abort signal (Ctrl-C / parent cancellation)", async () => { + const fetchImpl = ((_input: FetchInput, init?: RequestInit) => { + return new Promise((_, reject) => { + init?.signal?.addEventListener("abort", () => { + reject(new DOMException("aborted", "AbortError")); + }); + }); + }) as typeof fetch; + const callerController = new AbortController(); + const verdictPromise = verifyPackageSpec( + { name: "mongodb", version: "7.0.0" }, + { fetchImpl, signal: callerController.signal }, + ); + callerController.abort(); + const verdict = await verdictPromise; + expect(verdict.kind).toBe("unknown"); + if (verdict.kind === "unknown") { + expect(verdict.reason).toMatch(/caller signal/iu); + } + }); + + it("annotates timeout with explicit timeoutMs in the unknown reason", async () => { + const fetchImpl = ((_input: FetchInput, init?: RequestInit) => { + return new Promise((_, reject) => { + init?.signal?.addEventListener("abort", () => { + reject(new DOMException("aborted", "AbortError")); + }); + }); + }) as typeof fetch; + const verdict = await verifyPackageSpec( + { name: "mongodb", version: "7.0.0" }, + { fetchImpl, timeoutMs: 5 }, + ); + expect(verdict.kind).toBe("unknown"); + if (verdict.kind === "unknown") { + expect(verdict.reason).toMatch(/timed out after 5ms/iu); + } + }); + + it("returns a typed verdict (not a rejected promise) when fetchImpl throws a non-Error value", async () => { + let calls = 0; + const fetchImpl = (() => { + calls += 1; + if (calls === 1) { + // Symbol/null/undefined are valid throw targets but bypass + // `instanceof Error` checks. The verifier's inner try/catch + // converts them to a typed `unknown` verdict; the outer + // `.catch()` wrapper at verifyPackageSpec is the belt-and- + // suspenders backstop if a future refactor lets one through. + throw null as unknown as Error; + } + return Promise.resolve( + new Response(JSON.stringify({ name: "mongodb", version: "7.0.0" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + }) as typeof fetch; + const cache = new Map>(); + const first = await verifyPackageSpec( + { name: "mongodb", version: "7.0.0" }, + { fetchImpl, cache }, + ); + expect(first.kind).toBe("unknown"); + // Subsequent callers receive the cached typed verdict, not a + // rejected promise — confirms the catch-wrapper invariant holds + // even if the fetcher's exit shape is degenerate. + const second = await verifyPackageSpec( + { name: "mongodb", version: "7.0.0" }, + { fetchImpl, cache }, + ); + expect(second.kind).toBe("unknown"); + }); + + it("returns verified-missing on 404", async () => { + const fetchImpl = makeFetch([async () => new Response("Not Found", { status: 404 })]); + const verdict = await verifyPackageSpec( + { name: "doesnt-exist", version: "1.0.0" }, + { fetchImpl }, + ); + expect(verdict.kind).toBe("verified-missing"); + }); + + it("returns unknown on transport failure (offline / DNS)", async () => { + const fetchImpl = (() => { + throw new Error("getaddrinfo ENOTFOUND registry.npmjs.org"); + }) as typeof fetch; + const verdict = await verifyPackageSpec({ name: "mongodb", version: "7.0.0" }, { fetchImpl }); + expect(verdict.kind).toBe("unknown"); + if (verdict.kind === "unknown") { + expect(verdict.reason).toContain("ENOTFOUND"); + } + }); + + it("returns unknown on 5xx responses", async () => { + const fetchImpl = makeFetch([async () => new Response("Bad Gateway", { status: 502 })]); + const verdict = await verifyPackageSpec({ name: "mongodb", version: "7.0.0" }, { fetchImpl }); + expect(verdict.kind).toBe("unknown"); + if (verdict.kind === "unknown") { + expect(verdict.reason).toContain("502"); + } + }); + + it("returns unknown when 200 body is malformed (registry contract drift)", async () => { + const fetchImpl = makeFetch([ + async () => + new Response(JSON.stringify({ unrelated: "shape" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ]); + const verdict = await verifyPackageSpec({ name: "mongodb", version: "7.0.0" }, { fetchImpl }); + expect(verdict.kind).toBe("unknown"); + }); + + it("returns unknown when 200 body's version differs from the requested one (defensive)", async () => { + const fetchImpl = makeFetch([ + async () => + new Response(JSON.stringify({ version: "7.0.1" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ]); + const verdict = await verifyPackageSpec({ name: "mongodb", version: "7.0.0" }, { fetchImpl }); + expect(verdict.kind).toBe("unknown"); + }); + + it("URL-encodes scoped package names correctly", async () => { + const seen: string[] = []; + const fetchImpl = makeFetch([ + async (input) => { + seen.push(String(input)); + return new Response(JSON.stringify({ name: "@types/node", version: "24.10.4" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }, + ]); + await verifyPackageSpec({ name: "@types/node", version: "24.10.4" }, { fetchImpl }); + expect(seen[0]).toBe("https://registry.npmjs.org/%40types%2Fnode/24.10.4"); + }); + + it("uses cache to avoid duplicate registry calls", async () => { + let calls = 0; + const fetchImpl = (async () => { + calls += 1; + return new Response(JSON.stringify({ name: "mongodb", version: "7.0.0" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }) as typeof fetch; + const cache = new Map>(); + const spec: PackageSpec = { name: "mongodb", version: "7.0.0" }; + await verifyPackageSpec(spec, { fetchImpl, cache }); + await verifyPackageSpec(spec, { fetchImpl, cache }); + await verifyPackageSpec(spec, { fetchImpl, cache }); + expect(calls).toBe(1); + }); + + it("dedupes concurrent in-flight requests for the same spec (no thundering herd)", async () => { + let calls = 0; + const resolvers: Array<(response: Response) => void> = []; + const fetchImpl = (() => { + calls += 1; + return new Promise((resolve) => { + resolvers.push(resolve); + }); + }) as typeof fetch; + const cache = new Map>(); + const spec: PackageSpec = { name: "mongodb", version: "7.0.0" }; + const promises = [ + verifyPackageSpec(spec, { fetchImpl, cache }), + verifyPackageSpec(spec, { fetchImpl, cache }), + verifyPackageSpec(spec, { fetchImpl, cache }), + ]; + // Yield to the microtask queue so any synchronous-in-our-impl reads + // of the cache settle before we assert on the call count. + await Promise.resolve(); + // Resolve the single in-flight request; all three callers receive + // the same verdict. + resolvers[0]?.( + new Response(JSON.stringify({ name: "mongodb", version: "7.0.0" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }), + ); + const verdicts = await Promise.all(promises); + // The dedup invariant: by the time everything settles, only one + // network call was issued regardless of microtask interleaving. + expect(calls).toBe(1); + expect(verdicts.every((verdict) => verdict.kind === "verified-published")).toBe(true); + }); + + it("respects custom registry base URL", async () => { + const seen: string[] = []; + const fetchImpl = makeFetch([ + async (input) => { + seen.push(String(input)); + return new Response(JSON.stringify({ name: "mongodb", version: "1.0.0" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }, + ]); + await verifyPackageSpec( + { name: "mongodb", version: "1.0.0" }, + { fetchImpl, registryBase: "https://corp-registry.example/api" }, + ); + expect(seen[0]).toBe("https://corp-registry.example/api/mongodb/1.0.0"); + }); + + it("aborts on timeout", async () => { + const fetchImpl = ((_url: FetchInput, init?: RequestInit) => { + return new Promise((_, reject) => { + init?.signal?.addEventListener("abort", () => { + reject(new DOMException("aborted", "AbortError")); + }); + }); + }) as typeof fetch; + const verdict = await verifyPackageSpec( + { name: "mongodb", version: "7.0.0" }, + { fetchImpl, timeoutMs: 5 }, + ); + expect(verdict.kind).toBe("unknown"); + }); +}); + +function publishedFetchTracking(seen: string[]): typeof fetch { + return (async (input: FetchInput) => { + const url = String(input); + seen.push(url); + const segments = url.split("/"); + const version = segments.pop() ?? ""; + const name = decodeURIComponent(segments.pop() ?? ""); + return new Response(JSON.stringify({ name, version }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }) as typeof fetch; +} + +function publishedFetch(): typeof fetch { + return (async (input: FetchInput) => { + const url = String(input); + // The URL is `${base}/${encodedName}/${version}` — pop version, then + // pop and decode the (possibly scoped) name segment. + const segments = url.split("/"); + const version = segments.pop() ?? ""; + const namePart = segments.pop() ?? ""; + // Scoped names are `%40scope/pkg` so the previous segment may be a + // bare scope; restore it. + let name = decodeURIComponent(namePart); + if (name.startsWith("@")) { + // No scoped names hit this helper currently, but be defensive. + name = `${name}/${decodeURIComponent(segments.pop() ?? "")}`; + } + return new Response(JSON.stringify({ name, version }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }) as typeof fetch; +} + +describe("evaluateFindingForDrop", () => { + it("drops the finding when an extracted spec is verified-published", async () => { + const result = await evaluateFindingForDrop( + { + title: "mongodb@7.0.0 does not exist on npm", + reasoning: "mongodb@7.0.0 does not exist on npm.", + recommendation: "Pin mongodb@6.21.0 instead.", + }, + { fetchImpl: publishedFetch() }, + ); + expect(result).not.toBeNull(); + expect(result?.spec).toEqual({ name: "mongodb", version: "7.0.0" }); + expect(result?.dropReason).toContain("mongodb@7.0.0"); + }); + + it("keeps the finding when its title doesn't make a nonexistence claim", async () => { + const result = await evaluateFindingForDrop( + { + title: "Race condition in checkForNextMessage allows duplicate processing", + reasoning: "May affect mongodb@7.0.0 connection pool, see provider.ts:117.", + recommendation: "Add a mutex around the read-modify-write.", + }, + { fetchImpl: publishedFetch() }, + ); + expect(result).toBeNull(); + }); + + it("does not drop a compound finding based on a reasoning-only registry claim", async () => { + const result = await evaluateFindingForDrop( + { + title: "Dependency installation is broken", + reasoning: + "mongodb@7.0.0 does not exist on npm. The lockfile also contains an invalid integrity hash.", + recommendation: "Fix both defects.", + }, + { fetchImpl: publishedFetch() }, + ); + expect(result).toBeNull(); + }); + + it("keeps a public-npm title when reasoning identifies a configured private registry", async () => { + const result = await evaluateFindingForDrop( + { + title: "@acme/widget@1.2.3 does not exist on npm", + reasoning: "@acme/widget@1.2.3 does not exist in the configured GitHub Packages registry.", + recommendation: "Publish it to the configured registry.", + }, + { fetchImpl: publishedFetch() }, + ); + expect(result).toBeNull(); + }); + + it("keeps the finding when no extractable spec is present in title or body", async () => { + const result = await evaluateFindingForDrop( + { + title: "Test script runs vitest with no test files present", + reasoning: "package.json declares a test script but no tests/ directory exists.", + recommendation: "Add tests or remove the script.", + }, + { fetchImpl: publishedFetch() }, + ); + expect(result).toBeNull(); + }); + + it("keeps the finding when registry returns 404 (claim stands)", async () => { + const fetchImpl = (async () => new Response("Not Found", { status: 404 })) as typeof fetch; + const result = await evaluateFindingForDrop( + { + title: "fictional-pkg@99.99.99 is unpublished on npm", + reasoning: "fictional-pkg@99.99.99 was never published on npm.", + recommendation: "Remove the pin.", + }, + { fetchImpl }, + ); + expect(result).toBeNull(); + }); + + it("keeps the finding when registry call fails (offline / network error)", async () => { + const fetchImpl = (() => { + throw new Error("ECONNRESET"); + }) as typeof fetch; + const result = await evaluateFindingForDrop( + { + title: "mongodb@7.0.0 is unpublished on npm", + reasoning: "mongodb@7.0.0 does not exist on npm.", + recommendation: "Pin mongodb@6.21.0 instead.", + }, + { fetchImpl }, + ); + expect(result).toBeNull(); + }); + + it("keeps multi-version publication claims", async () => { + const seen: string[] = []; + const fetchImpl = publishedFetchTracking(seen); + const result = await evaluateFindingForDrop( + { + title: "mongodb@7.0.0 and vitest@4.0.16 are unpublished on npm", + reasoning: "Both packages are pinned to versions that don't exist on npm.", + recommendation: "Use mongodb@6.21.0 and vitest@3.2.4.", + }, + { fetchImpl }, + ); + expect(result).toBeNull(); + expect(seen).toHaveLength(0); + }); + + it("keeps a multi-version finding when any claimed version is missing", async () => { + const fetchImpl = (async (input: FetchInput) => { + const url = String(input); + if (url.includes("/fictional-pkg/")) { + return new Response("Not Found", { status: 404 }); + } + const segments = url.split("/"); + const version = segments.pop() ?? ""; + const name = decodeURIComponent(segments.pop() ?? ""); + return new Response(JSON.stringify({ name, version }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }) as typeof fetch; + const result = await evaluateFindingForDrop( + { + title: "fictional-pkg@1.0.0 and mongodb@7.0.0 are unpublished on npm", + reasoning: "Both do not exist on npm.", + recommendation: "Fix.", + }, + { fetchImpl }, + ); + expect(result).toBeNull(); + }); + + it("does not treat a published recommendation as refuting a missing version", async () => { + const fetchImpl = (async (input: FetchInput) => { + if (String(input).includes("/99.99.99")) { + return new Response("Not Found", { status: 404 }); + } + return new Response(JSON.stringify({ name: "fictional-pkg", version: "1.0.0" }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }) as typeof fetch; + const result = await evaluateFindingForDrop( + { + title: "fictional-pkg@99.99.99 is unpublished on npm", + reasoning: "fictional-pkg@99.99.99 does not exist on npm.", + recommendation: "Use fictional-pkg@1.0.0 instead.", + }, + { fetchImpl }, + ); + expect(result).toBeNull(); + }); + + it("keeps a finding when reasoning adds another missing claimed version", async () => { + const fetchImpl = (async (input: FetchInput) => { + const url = String(input); + if (url.includes("/bar/9.0.0")) { + return new Response("Not Found", { status: 404 }); + } + const segments = url.split("/"); + const version = segments.pop() ?? ""; + const name = decodeURIComponent(segments.pop() ?? ""); + return new Response(JSON.stringify({ name, version }), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + }) as typeof fetch; + const result = await evaluateFindingForDrop( + { + title: "foo@2.0.0 and bar@9.0.0 are unpublished on npm", + reasoning: "foo@2.0.0 and bar@9.0.0 are unpublished on npm.", + recommendation: "Fix the pins.", + }, + { fetchImpl }, + ); + expect(result).toBeNull(); + }); + + it("does not treat generic invalid-version compatibility findings as nonexistence claims", async () => { + const result = await evaluateFindingForDrop( + { + title: "Invalid version pinned: react@19.0.0 conflicts with the supported peer range", + reasoning: "The version is published but incompatible with the declared peer dependency.", + recommendation: "Use a compatible published version.", + }, + { fetchImpl: publishedFetch() }, + ); + expect(result).toBeNull(); + }); + + it("does not combine a private-registry claim with unrelated public npm context", async () => { + const result = await evaluateFindingForDrop( + { + title: "foo@1.2.3 is unpublished from the configured GitHub Packages registry", + reasoning: "Unlike public npm, this project installs from GitHub Packages.", + recommendation: "Publish the package to the configured registry.", + }, + { fetchImpl: publishedFetch() }, + ); + expect(result).toBeNull(); + }); + + it("does not drop documentation findings that quote an unpublished-package claim", async () => { + const result = await evaluateFindingForDrop( + { + title: "README incorrectly says foo@1.2.3 is unpublished on npm", + reasoning: "The package exists and the documentation is stale.", + recommendation: "Correct the README.", + }, + { fetchImpl: publishedFetch() }, + ); + expect(result).toBeNull(); + }); +}); diff --git a/src/registry-verifier.ts b/src/registry-verifier.ts new file mode 100644 index 00000000..20050f2d --- /dev/null +++ b/src/registry-verifier.ts @@ -0,0 +1,446 @@ +/** + * Registry verifier — drops review findings that claim a package version is + * unpublished when the npm registry says otherwise. + * + * Background: providers backed by an LLM with a fixed knowledge cutoff + * routinely surface "package X@Y does not exist on npm" findings for + * versions released after the cutoff. The model has no way to know about + * post-cutoff releases without a tool, so the assertion is a hallucination + * dressed as ground truth. (See "We Have a Package for You!" Spracklen et + * al., USENIX Security 2025, arXiv:2406.10279 — measured 5-22% rates of + * the symmetric failure: hallucinating *nonexistent* packages.) + * + * The mitigation is the obvious one already practised in dependency tools + * (Renovate's `lib/modules/datasource/npm/get.ts`, Dependabot, Socket): ask + * the registry. This module is the partition-layer addition that lets + * `validateReviewOutputPartitioned` reject findings whose central claim is + * trivially refuted by a registry GET. + * + * Failure modes are biased toward "keep the finding": + * - registry says version is published → drop finding (positive signal) + * - registry returns 404 → keep finding (claim stands) + * - registry returns any non-200 → keep finding (no false drop) + * - request errors / offline → keep finding (no false drop) + * - finding text has no extractable spec → keep finding (out of scope) + * + * Only a `verified-published` outcome causes a drop. Errors are + * non-fatal and surfaced via `notes` for operators. + */ + +const NPM_REGISTRY_BASE = "https://registry.npmjs.org"; +const REQUEST_TIMEOUT_MS = 5_000; +const RESPONSE_BODY_BYTE_CAP = 1_048_576; // 1 MiB; per-version manifests are ~1-3 KB +const USER_AGENT = "clawpatch (+https://github.com/openclaw/clawpatch)"; + +/** + * Match an npm `pkg@version` spec inside arbitrary prose. Accepts: + * - bare names: `mongodb@7.0.0`, `tsx@4.21.0` + * - scoped names: `@types/node@24.10.4`, `@aws-sdk/client-s3@3.1000.0` + * - prereleases: `vitest@4.0.16-beta.1`, `react@19.0.0-rc.1` + * - build metadata: `pkg@1.2.3+sha.abcd` + * + * The version segment matches semver-shaped strings; non-semver tags + * ("latest", "next") are intentionally excluded — those aren't subject to + * the hallucination failure mode this verifier addresses. + * + * The trailing `.` (sentence period) is never consumed: the + * `[0-9A-Za-z-]+` core forbids `.` and is followed by an optional + * `(?:\.[0-9A-Za-z-]+)*` for dotted parts, so the final character of the + * match is always a digit/letter/dash, never a `.`. + * + * The `u` flag (no `i`) keeps package names lowercase per npm's naming + * rule (RFC 7468 / npm-package-json). Uppercase tokens like + * `WatchedActivities@1.0.0` (a class name in prose) won't extract. + */ +const SPEC_PATTERN = + /(?>; + /** Override `globalThis.fetch` (testing, custom transport). */ + fetchImpl?: typeof fetch; + /** Wall-clock timeout per request. Default 5s. */ + timeoutMs?: number; + /** Override registry base URL. Default `https://registry.npmjs.org`. */ + registryBase?: string; + /** + * Optional caller-supplied AbortSignal (e.g. wired to user Ctrl-C). + * Combined with the per-request timeout signal so either source can + * cancel. Aborted requests resolve to `kind: "unknown"`. + */ + signal?: AbortSignal; +}; + +/** + * Returns true only for a narrow direct package-publication claim. The + * verifier intentionally prefers false negatives over dropping a finding + * whose wider title may describe documentation, lockfile, cache, or private + * registry behavior. + */ +export function findingClaimsNonexistence(title: string): boolean { + const specs = extractPackageSpecs(title); + if (specs.length !== 1) { + return false; + } + const firstSpec = specs[0]!; + const specText = `${firstSpec.name}@${firstSpec.version}`; + const trimmed = title.trimStart(); + const specIndex = trimmed.indexOf(specText); + const prefix = specIndex < 0 ? "" : trimmed.slice(0, specIndex).trim(); + if (specIndex < 0 || !/^(?:package|pinned|dependency|version)?$/iu.test(prefix)) { + return false; + } + const claim = trimmed.slice(specIndex + specText.length); + return ( + /^\s+(?:is\s+)?(?:unpublished|not published|not a published version|non[- ]?existent|does ?n['']?t exist|does not exist)\s+(?:on|in)\s+(?:public\s+)?npm\s*[.!?]?$/iu.test( + claim, + ) || + /^\s+(?:is\s+)?(?:unpublished|not published|non[- ]?existent|does ?n['']?t exist|does not exist)\s+(?:at|on)\s+(?:https?:\/\/)?registry\.npmjs\.org\s*[.!?]?$/iu.test( + claim, + ) || + /^\s+(?:has\s+)?(?:ETARGET|no matching version)(?:\s+(?:error|response))?\s+(?:on|from)\s+(?:public\s+)?npm\s*[.!?]?$/iu.test( + claim, + ) + ); +} + +/** + * Extract candidate `pkg@version` specs from the union of a finding's + * narrative fields. Returns deduplicated specs in document order. + * + * The extractor is conservative: it only matches tokens that look like + * complete semver-shaped specs. Loose phrasing like "mongodb 7" or + * "version 7.0" is not extracted — verification of an under-specified + * claim could not be reliable. + */ +export function extractPackageSpecs(text: string): PackageSpec[] { + const seen = new Set(); + const specs: PackageSpec[] = []; + for (const match of text.matchAll(SPEC_PATTERN)) { + const name = match[1]!; + const version = match[2]!; + const key = `${name}@${version}`; + if (seen.has(key)) { + continue; + } + seen.add(key); + specs.push({ name, version }); + } + return specs; +} + +/** + * Resolve a single `(name, version)` spec against the npm registry. + * Uses the per-version document endpoint (`/{name}/{version}`) to avoid + * downloading the full packument when only an existence check is needed. + * + * The endpoint contract: + * 200 with JSON body whose `name` AND `version` match → published + * 404 → not published + * anything else / transport failure / redirect / mismatched body → unknown + * + * Defensive against intercepting proxies and misconfigured mirrors: + * - `redirect: "error"` so a 302 to an SSO gate becomes "unknown", + * not a successful drop; + * - response Content-Type must contain `application/json`; + * - response body capped at 1 MiB to prevent hostile mirrors from + * exhausting memory inside the timeout window; + * - both `name` and `version` of the response body must match the + * requested spec (a mirror that returns `{version:"X"}` for any + * path is otherwise indistinguishable from a real publish). + */ +export async function verifyPackageSpec( + spec: PackageSpec, + options: RegistryVerifierOptions = {}, +): Promise { + const cache = options.cache; + const cacheKey = `${spec.name}@${spec.version}`; + if (cache) { + const inflight = cache.get(cacheKey); + if (inflight) { + return inflight; + } + } + // Wrap rejections defensively: every code path in `resolveVerdict` + // already returns a verdict, but a future refactor or a rogue + // `fetchImpl` that throws a non-Error (e.g. a Symbol or null) would + // otherwise poison the cache with a permanently-rejected promise. + const verdictPromise = resolveVerdict(spec, options).catch( + (error: unknown): RegistryVerdict => + unknownVerdict( + spec, + error instanceof Error ? `${error.name}: ${error.message}` : String(error), + ), + ); + if (cache) { + cache.set(cacheKey, verdictPromise); + } + return verdictPromise; +} + +async function resolveVerdict( + spec: PackageSpec, + options: RegistryVerifierOptions, +): Promise { + const fetchImpl = options.fetchImpl ?? globalThis.fetch; + const base = options.registryBase ?? NPM_REGISTRY_BASE; + const url = `${base}/${encodeRegistryName(spec.name)}/${encodeURIComponent(spec.version)}`; + const timeoutController = new AbortController(); + const timeoutMs = options.timeoutMs ?? REQUEST_TIMEOUT_MS; + const timer = setTimeout( + () => timeoutController.abort(new DOMException(`timeout ${timeoutMs}ms`, "TimeoutError")), + timeoutMs, + ); + // Don't let the timeout pin the event loop on caller paths that exit + // before the timer fires. + if (typeof timer === "object" && "unref" in timer && typeof timer.unref === "function") { + timer.unref(); + } + const signal = options.signal + ? anyAbortSignal([timeoutController.signal, options.signal]) + : timeoutController.signal; + try { + const response = await fetchImpl(url, { + method: "GET", + headers: { Accept: "application/json", "User-Agent": USER_AGENT }, + signal, + redirect: "error", + }); + if (response.status === 404) { + return { kind: "verified-missing", name: spec.name, version: spec.version }; + } + if (response.status !== 200) { + return unknownVerdict(spec, `registry returned status ${response.status}`); + } + const contentType = response.headers.get("content-type") ?? ""; + if (!contentType.toLowerCase().includes("application/json")) { + return unknownVerdict( + spec, + `registry returned 200 with non-JSON content-type: ${contentType || ""}`, + ); + } + const text = await readCappedText(response, RESPONSE_BODY_BYTE_CAP); + if (text === null) { + return unknownVerdict( + spec, + `registry response body exceeded ${RESPONSE_BODY_BYTE_CAP} bytes`, + ); + } + const body = safeJsonParse(text); + if ( + body && + typeof body["version"] === "string" && + body["version"] === spec.version && + typeof body["name"] === "string" && + body["name"] === spec.name + ) { + return { kind: "verified-published", name: spec.name, version: spec.version }; + } + return unknownVerdict( + spec, + "registry returned 200 but body did not match expected name/version", + ); + } catch (error: unknown) { + if (error instanceof Error && error.name === "AbortError") { + const reason = timeoutController.signal.aborted + ? `request timed out after ${timeoutMs}ms` + : "request aborted by caller signal"; + return unknownVerdict(spec, reason); + } + const reason = error instanceof Error ? `${error.name}: ${error.message}` : String(error); + return unknownVerdict(spec, reason); + } finally { + clearTimeout(timer); + } +} + +function unknownVerdict(spec: PackageSpec, reason: string): RegistryVerdict { + return { kind: "unknown", name: spec.name, version: spec.version, reason }; +} + +function safeJsonParse(text: string): Record | null { + try { + const parsed = JSON.parse(text); + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { + return parsed as Record; + } + return null; + } catch { + return null; + } +} + +/** + * Read the response body up to `cap` bytes; return null if the limit is + * exceeded. Uses the underlying ReadableStream so a hostile slow-trickle + * body trips the cap before it trips the wall-clock timeout, and the + * stream is cancelled (closing the underlying connection) instead of + * being released-and-drained. + * + * When the response has no readable body (some 204/304 paths, or + * runtime quirks) we cannot enforce the cap by streaming. If the + * `Content-Length` header is missing in that case, we conservatively + * report cap-exceeded rather than calling `response.text()` (which has + * no upper bound). + */ +async function readCappedText(response: Response, cap: number): Promise { + const headerLength = response.headers.get("content-length"); + const contentLength = headerLength === null ? null : Number(headerLength); + if (contentLength !== null && Number.isFinite(contentLength) && contentLength > cap) { + return null; + } + if (!response.body) { + if (contentLength === null) { + return null; + } + return response.text(); + } + const reader = response.body.getReader(); + const chunks: Uint8Array[] = []; + let total = 0; + let exceeded = false; + try { + for (;;) { + const { value, done } = await reader.read(); + if (done) { + break; + } + total += value.byteLength; + if (total > cap) { + exceeded = true; + break; + } + chunks.push(value); + } + } finally { + if (exceeded) { + // Cancel propagates upstream and closes the underlying connection + // so a slow-trickle body doesn't keep transferring after we bail. + // releaseLock on its own only detaches the reader. + await reader.cancel("body cap exceeded").catch(() => undefined); + } else { + reader.releaseLock(); + } + } + if (exceeded) { + return null; + } + const merged = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + merged.set(chunk, offset); + offset += chunk.byteLength; + } + return new TextDecoder("utf-8").decode(merged); +} + +/** + * Combine multiple AbortSignals so the resulting signal aborts when any + * input does. Prefers `AbortSignal.any` (Node 22+, Bun ≥1.1, modern + * browsers) with a manual fallback for older runtimes. + * + * The fallback is careful to remove sibling listeners as soon as one + * input fires, so a long-lived caller signal (e.g. a process-wide + * Ctrl-C controller passed to many verifications) doesn't accrete dead + * listeners. On runtimes with native `AbortSignal.any` this concern is + * handled internally by the platform. + */ +function anyAbortSignal(signals: readonly AbortSignal[]): AbortSignal { + const ctor = AbortSignal as unknown as { any?: (s: readonly AbortSignal[]) => AbortSignal }; + if (typeof ctor.any === "function") { + return ctor.any(signals); + } + const controller = new AbortController(); + for (const signal of signals) { + if (signal.aborted) { + controller.abort(signal.reason); + return controller.signal; + } + } + const handlers: Array<{ signal: AbortSignal; handler: () => void }> = []; + const cleanup = () => { + for (const { signal, handler } of handlers) { + signal.removeEventListener("abort", handler); + } + }; + for (const signal of signals) { + const handler = () => { + controller.abort(signal.reason); + cleanup(); + }; + signal.addEventListener("abort", handler, { once: true }); + handlers.push({ signal, handler }); + } + return controller.signal; +} + +/** + * Encode a package name for the registry URL. Scoped names like + * `@types/node` must encode the `/` so the URL parser keeps the scope as + * a single path segment; bare names pass through unchanged. + */ +function encodeRegistryName(name: string): string { + return encodeURIComponent(name); +} + +/** + * Decide whether a finding should be dropped as a registry-verified + * false positive. Returns the spec that was verified-published, or null + * if the finding should be kept. A finding is dropped only when every + * claimed spec resolves to `verified-published` — every other outcome + * (missing, unknown, no specs) leaves the finding in place. + * + * Returns the first published spec encountered so callers can include it + * in the drop's `message` for operator clarity. + */ +export async function evaluateFindingForDrop( + finding: { title: string; reasoning: string; recommendation: string }, + options: RegistryVerifierOptions = {}, +): Promise<{ dropReason: string; spec: PackageSpec } | null> { + if (!findingClaimsNonexistence(finding.title) || !findingClaimsNonexistence(finding.reasoning)) { + return null; + } + const claimedSpecs = extractPackageSpecs(finding.title); + const reasoningSpec = extractPackageSpecs(finding.reasoning)[0]; + if ( + reasoningSpec === undefined || + reasoningSpec.name !== claimedSpecs[0]?.name || + reasoningSpec.version !== claimedSpecs[0]?.version + ) { + return null; + } + const specs = Array.from( + new Map(claimedSpecs.map((spec) => [`${spec.name}@${spec.version}`, spec])).values(), + ); + if (specs.length === 0) { + return null; + } + let firstPublished: PackageSpec | null = null; + for (const spec of specs) { + const verdict = await verifyPackageSpec(spec, options); + if (verdict.kind !== "verified-published") { + return null; + } + firstPublished ??= spec; + } + return firstPublished === null + ? null + : { + spec: firstPublished, + dropReason: `npm registry confirms every claimed package version is published (${specs.map((spec) => `${spec.name}@${spec.version}`).join(", ")}); finding's nonexistence claim is refuted by ground truth`, + }; +} diff --git a/src/review-validation.ts b/src/review-validation.ts index 446acab1..63bee25a 100644 --- a/src/review-validation.ts +++ b/src/review-validation.ts @@ -3,6 +3,11 @@ import { isAbsolute, relative, resolve } from "node:path"; import { ClawpatchError } from "./errors.js"; import { REVIEW_PROMPT_FILE_CHAR_LIMIT, type ReviewPromptManifest } from "./prompt.js"; import type { DroppedFinding } from "./provider.js"; +import { + evaluateFindingForDrop, + type RegistryVerdict, + type RegistryVerifierOptions, +} from "./registry-verifier.js"; import { ClawpatchConfig, FeatureRecord, ReviewOutput } from "./types.js"; export async function validateReviewOutput( @@ -26,6 +31,30 @@ export async function validateReviewOutput( return { ...output, findings }; } +/** + * Optional post-validation hook: receives a finding that has already + * passed schema and evidence checks, returns a drop reason if the + * finding's central claim is independently refuted, or null to keep it. + * + * The default implementation, when enabled by config, runs the npm + * registry verifier — see `src/registry-verifier.ts`. Callers may inject + * a stub for testing or provide a different verifier (e.g. PyPI) without + * touching the partitioning logic here. + */ +export type FindingPostValidator = ( + finding: ReviewOutput["findings"][number], +) => Promise<{ dropReason: string } | null>; + +export type ValidatePartitionedOptions = { + /** + * Optional post-validation hook applied to findings that have already + * passed schema + evidence validation. Findings the hook returns a + * drop reason for are partitioned into `droppedFindings` with + * `layer: "registry-verifier"` instead of being kept. + */ + postValidator?: FindingPostValidator; +}; + /** * Same evidence validation as {@link validateReviewOutput}, but runs * per-finding and partitions failures instead of throwing on the first @@ -40,6 +69,7 @@ export async function validateReviewOutputPartitioned( config: ClawpatchConfig, manifest: ReviewPromptManifest, output: ReviewOutput, + options: ValidatePartitionedOptions = {}, ): Promise<{ findings: ReviewOutput["findings"]; droppedFindings: DroppedFinding[] }> { void feature; void config; @@ -50,12 +80,10 @@ export async function validateReviewOutputPartitioned( const cache = new Map>(); const validFindings: ReviewOutput["findings"] = []; const droppedFindings: DroppedFinding[] = []; - output.findings.forEach(() => undefined); for (let idx = 0; idx < output.findings.length; idx += 1) { const finding = output.findings[idx]!; try { await validateFinding(root, finding, included, promptFiles, cache); - validFindings.push(finding); } catch (error: unknown) { if (error instanceof ClawpatchError && error.code === "malformed-output") { droppedFindings.push({ @@ -68,10 +96,42 @@ export async function validateReviewOutputPartitioned( } throw error; } + if (options.postValidator) { + const verdict = await options.postValidator(finding); + if (verdict) { + droppedFindings.push({ + path: ["findings", idx], + message: verdict.dropReason, + sample: truncateValidationSample(finding), + layer: "registry-verifier", + }); + continue; + } + } + validFindings.push(finding); } return { findings: validFindings, droppedFindings }; } +/** + * Build a `FindingPostValidator` that runs the npm registry verifier + * with a shared per-call cache. Returned validator returns a drop + * decision when the finding's "X@Y is unpublished" claim is contradicted + * by the registry, or null to keep the finding. + * + * `verifierOptions` lets callers inject a stub fetch (testing) or + * override the registry base URL (corporate mirror). The cache is + * managed internally so concurrent finding validations within one call + * deduplicate identical registry lookups. + */ +export function buildRegistryVerifierValidator( + verifierOptions: RegistryVerifierOptions = {}, +): FindingPostValidator { + const cache = verifierOptions.cache ?? new Map>(); + const merged: RegistryVerifierOptions = { ...verifierOptions, cache }; + return async (finding) => evaluateFindingForDrop(finding, merged); +} + async function validateFinding( root: string, finding: ReviewOutput["findings"][number], diff --git a/src/types.ts b/src/types.ts index d21df35e..94cefc1f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -141,6 +141,19 @@ export const configSchema = z.object({ commit: z.boolean(), openPr: z.boolean(), }), + /** + * Post-validation registry verifier — drops review findings that claim + * a package version is unpublished when the npm registry says + * otherwise. Explicit opt-in because lookups send package coordinates + * to the public npm registry. Only `verified-published` + * outcomes drop a finding; 404, network errors, and unknown responses + * keep it). See `src/registry-verifier.ts` for the verdict matrix. + */ + registryVerifier: z + .object({ + enabled: z.boolean().default(false), + }) + .default({ enabled: false }), }); export type ClawpatchConfig = z.infer;