diff --git a/.env.example b/.env.example index 460a4492c7..f31e1c8a03 100644 --- a/.env.example +++ b/.env.example @@ -63,27 +63,28 @@ GITTENSORY_REVIEW_ENRICHMENT=false # # Unknown names warn and are ignored; a typo-only list runs no analyzers. # BEGIN GENERATED REES ANALYZERS # Current analyzer names: -# dependency,lockfileDrift,secret,license,installScript,heavyDependency,hardcodedUrl,actionPin -# eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature,iacMisconfig -# nativeBuild,history,docCommentDrift,duplication,churnHotspot,blameLink,approvalIntegrity -# ciCheckSignals,undocumentedExport,staleBranch,commitHygiene,pendingReviewRequests,testRatio -# migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker,debugLeftover -# commitLint -# -# Profile defaults: -# fast: dependency,lockfileDrift,secret,license,installScript,heavyDependency,hardcodedUrl -# actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild,testRatio -# migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker,debugLeftover -# balanced (default): dependency,lockfileDrift,secret,license,installScript,heavyDependency +# dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency # hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat # commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot # blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene # pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber # conflictMarker,debugLeftover,commitLint -# deep: dependency,lockfileDrift,secret,license,installScript,heavyDependency,hardcodedUrl -# actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature -# iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot,blameLink -# approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene +# +# Profile defaults: +# fast: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency +# hardcodedUrl,actionPin,eol,redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild +# testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber,conflictMarker +# debugLeftover +# balanced (default): dependency,dependencyDiff,lockfileDrift,secret,license,installScript +# heavyDependency,hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight +# typosquat,commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication +# churnHotspot,blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch +# commitHygiene,pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology +# todoMarker,magicNumber,conflictMarker,debugLeftover,commitLint +# deep: dependency,dependencyDiff,lockfileDrift,secret,license,installScript,heavyDependency +# hardcodedUrl,actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat +# commitSignature,iacMisconfig,nativeBuild,history,docCommentDrift,duplication,churnHotspot +# blameLink,approvalIntegrity,ciCheckSignals,undocumentedExport,staleBranch,commitHygiene # pendingReviewRequests,testRatio,migrationSafety,looseRange,terminology,todoMarker,magicNumber # conflictMarker,debugLeftover,commitLint # END GENERATED REES ANALYZERS diff --git a/apps/gittensory-ui/src/lib/rees-analyzers.ts b/apps/gittensory-ui/src/lib/rees-analyzers.ts index 30e72ae3f9..f7e6f4c13f 100644 --- a/apps/gittensory-ui/src/lib/rees-analyzers.ts +++ b/apps/gittensory-ui/src/lib/rees-analyzers.ts @@ -119,6 +119,29 @@ export const REES_ANALYZERS = [ notes: "Manifest-only by design; use lockfileDrift for transitive lockfile changes.", }, }, + { + name: "dependencyDiff", + title: "Dependency inventory changes", + category: "supply-chain", + cost: "local", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + requires: ["files"], + limits: { + maxFindings: 25, + maxManifestFiles: 20, + maxPatchLinesPerFile: 500, + }, + docs: { + summary: + "Summarizes direct dependency add/remove/version-change deltas in changed manifest patches — informational, not a CVE scan.", + looksAt: "Added/removed lines in package.json, requirements.txt, and go.mod diffs.", + reports: "Ecosystem, package name, direction (add/remove/change), and from/to versions.", + network: "Pure local analyzer. No external network call.", + notes: + "Distinct from the dependency CVE analyzer: this is the neutral inventory delta only. Reuses the same manifest line parsers.", + }, + }, { name: "lockfileDrift", title: "Lockfile drift", diff --git a/review-enrichment/analyzer-metadata.json b/review-enrichment/analyzer-metadata.json index 5bd5db75a5..a07d7ddeda 100644 --- a/review-enrichment/analyzer-metadata.json +++ b/review-enrichment/analyzer-metadata.json @@ -108,6 +108,33 @@ "notes": "Manifest-only by design; use lockfileDrift for transitive lockfile changes." } }, + { + "name": "dependencyDiff", + "title": "Dependency inventory changes", + "category": "supply-chain", + "cost": "local", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "requires": [ + "files" + ], + "limits": { + "maxFindings": 25, + "maxManifestFiles": 20, + "maxPatchLinesPerFile": 500 + }, + "docs": { + "summary": "Summarizes direct dependency add/remove/version-change deltas in changed manifest patches — informational, not a CVE scan.", + "looksAt": "Added/removed lines in package.json, requirements.txt, and go.mod diffs.", + "reports": "Ecosystem, package name, direction (add/remove/change), and from/to versions.", + "network": "Pure local analyzer. No external network call.", + "notes": "Distinct from the dependency CVE analyzer: this is the neutral inventory delta only. Reuses the same manifest line parsers." + } + }, { "name": "lockfileDrift", "title": "Lockfile drift", diff --git a/review-enrichment/src/analyzers/dependency-diff.ts b/review-enrichment/src/analyzers/dependency-diff.ts new file mode 100644 index 0000000000..32edb7023a --- /dev/null +++ b/review-enrichment/src/analyzers/dependency-diff.ts @@ -0,0 +1,28 @@ +// Dependency-diff inventory analyzer (#2020). Emits a neutral summary of direct dependency manifest +// changes — added, removed, or version-changed packages across package.json, requirements.txt, and go.mod. +// Distinct from the CVE-scanning dependency analyzer: no registry calls, pure compute over manifest patches. +import type { DependencyDiffFinding, EnrichRequest } from "../types.js"; +import { + extractDependencyInventoryChanges, + type ScanLimits, +} from "./dependency-scan.js"; + +const MAX_FINDINGS = 25; +const LIMITS: ScanLimits = { + maxManifestFiles: 20, + maxPatchLinesPerFile: 500, +}; + +/** Scan changed manifest patches for direct dependency inventory deltas. Pure. */ +export function scanDependencyDiff(req: EnrichRequest): DependencyDiffFinding[] { + return extractDependencyInventoryChanges(req.files ?? [], LIMITS, MAX_FINDINGS); +} + +/** Analyzer entrypoint: summarize direct dependency add/remove/change deltas from manifest patches. */ +export async function scanDependencyDiffInventory( + req: EnrichRequest, + signal?: AbortSignal, +): Promise { + if (signal?.aborted) throw new Error("analyzer_aborted"); + return scanDependencyDiff(req); +} diff --git a/review-enrichment/src/analyzers/dependency-scan.ts b/review-enrichment/src/analyzers/dependency-scan.ts index 12f383c73f..0d43eee901 100644 --- a/review-enrichment/src/analyzers/dependency-scan.ts +++ b/review-enrichment/src/analyzers/dependency-scan.ts @@ -87,11 +87,12 @@ const ECOSYSTEM: Record = { "go.mod": "Go", }; -/** Extract added/changed (not removed) dependency versions from the changed manifests in the diff. Pure. */ -export function extractDependencyChanges( +/** Collect direct-dependency add/remove versions from changed manifest patches. Pure. */ +function collectManifestDependencyEntries( files: NonNullable, limits: ScanLimits = {}, -): DepChange[] { + keyByFile = false, +): Map { const byKey = new Map< string, { ecosystem: string; package: string; added?: string; removed?: string } @@ -112,15 +113,25 @@ export function extractDependencyChanges( continue; const parsed = parseLine(manifest, line.slice(1).trim()); if (!parsed) continue; - const key = ecosystem + "::" + parsed.name; + const key = keyByFile + ? `${file.path}::${ecosystem}::${parsed.name}` + : `${ecosystem}::${parsed.name}`; const entry = byKey.get(key) ?? { ecosystem, package: parsed.name }; if (sign === "+") entry.added = parsed.version; else entry.removed = parsed.version; byKey.set(key, entry); } } + return byKey; +} + +/** Extract added/changed (not removed) dependency versions from the changed manifests in the diff. Pure. */ +export function extractDependencyChanges( + files: NonNullable, + limits: ScanLimits = {}, +): DepChange[] { const changes: DepChange[] = []; - for (const entry of byKey.values()) { + for (const entry of collectManifestDependencyEntries(files, limits).values()) { // Only scan a version that's present after the change, and only when it actually changed. if (!entry.added || entry.added === entry.removed) continue; changes.push({ @@ -133,6 +144,54 @@ export function extractDependencyChanges( return changes; } +export type DependencyInventoryDirection = "add" | "remove" | "change"; + +export interface DependencyInventoryChange { + ecosystem: string; + package: string; + from: string | null; + to: string | null; + direction: DependencyInventoryDirection; +} + +/** Neutral direct-dependency inventory delta from manifest patches — add, remove, and version change. Pure. */ +export function extractDependencyInventoryChanges( + files: NonNullable, + limits: ScanLimits = {}, + maxFindings = 25, +): DependencyInventoryChange[] { + const findings: DependencyInventoryChange[] = []; + for (const entry of collectManifestDependencyEntries(files, limits, true).values()) { + if (findings.length >= maxFindings) break; + if (entry.added && entry.removed && entry.added !== entry.removed) { + findings.push({ + ecosystem: entry.ecosystem, + package: entry.package, + from: entry.removed, + to: entry.added, + direction: "change", + }); + } else if (entry.added && !entry.removed) { + findings.push({ + ecosystem: entry.ecosystem, + package: entry.package, + from: null, + to: entry.added, + direction: "add", + }); + } else if (entry.removed && !entry.added) { + findings.push({ + ecosystem: entry.ecosystem, + package: entry.package, + from: entry.removed, + to: null, + direction: "remove", + }); + } + } + return findings; +} + interface OsvVuln { id: string; summary?: string; diff --git a/review-enrichment/src/analyzers/registry.ts b/review-enrichment/src/analyzers/registry.ts index f5fcc1ea39..f3896e17b9 100644 --- a/review-enrichment/src/analyzers/registry.ts +++ b/review-enrichment/src/analyzers/registry.ts @@ -8,6 +8,7 @@ import { scanCodeowners } from "./codeowners.js"; import { scanCommitHygiene } from "./commit-hygiene.js"; import { scanCommitSignature } from "./commit-signature.js"; import { dependencyAnalyzer } from "./dependency/descriptor.js"; +import { scanDependencyDiffInventory } from "./dependency-diff.js"; import { scanDocCommentDrift } from "./doc-comment-drift.js"; import { scanDuplication } from "./duplication-scan.js"; import { scanEol } from "./eol-check.js"; @@ -52,6 +53,41 @@ function descriptor( export const ANALYZER_DESCRIPTORS = [ dependencyAnalyzer, + descriptor({ + name: "dependencyDiff", + title: "Dependency inventory changes", + category: "supply-chain", + cost: "local", + defaultEnabled: true, + requires: ["files"], + limits: { maxFindings: 25, maxManifestFiles: 20, maxPatchLinesPerFile: 500 }, + docs: { + summary: + "Summarizes direct dependency add/remove/version-change deltas in changed manifest patches — informational, not a CVE scan.", + looksAt: "Added/removed lines in package.json, requirements.txt, and go.mod diffs.", + reports: "Ecosystem, package name, direction (add/remove/change), and from/to versions.", + network: "Pure local analyzer. No external network call.", + notes: + "Distinct from the dependency CVE analyzer: this is the neutral inventory delta only. Reuses the same manifest line parsers.", + }, + render: (findings, helpers) => { + if (!findings.length) return []; + const lines = ["### Dependency inventory changes (direct manifest delta)"]; + for (const item of findings) { + const detail = + item.direction === "add" + ? `added @ ${helpers.safeCodeSpan(item.to ?? "?")}` + : item.direction === "remove" + ? `removed (was ${helpers.safeCodeSpan(item.from ?? "?")})` + : `${helpers.safeCodeSpan(item.from ?? "?")} → ${helpers.safeCodeSpan(item.to ?? "?")}`; + lines.push( + `- ${helpers.safeCodeSpan(item.package)} (${helpers.safeCodeSpan(item.ecosystem)}) — ${helpers.safeCodeSpan(item.direction)}: ${detail}`, + ); + } + return lines; + }, + run: (req, { signal }) => scanDependencyDiffInventory(req, signal), + }), descriptor({ name: "lockfileDrift", title: "Lockfile drift", diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index b865adefd9..b2a830feaf 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -28,6 +28,7 @@ export function renderBrief( const lines: string[] = []; lines.push(...renderDescriptorSection("dependency", findings.dependency)); + lines.push(...renderDescriptorSection("dependencyDiff", findings.dependencyDiff)); const lockfileDrift = findings.lockfileDrift ?? []; if (lockfileDrift.length) { diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index 3080f0a32f..bccb796a75 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -57,6 +57,16 @@ export interface DependencyFinding { cves: Cve[]; } +/** Neutral direct-dependency inventory delta from manifest patches (#2020, part of #1499). + * Reports add/remove/version-change only — no CVE or registry data. */ +export interface DependencyDiffFinding { + ecosystem: string; + package: string; + from: string | null; + to: string | null; + direction: "add" | "remove" | "change"; +} + /** A vulnerable lockfile-only dependency resolution. The package was not changed in a top-level manifest diff, * so it is treated as transitive lockfile drift and reported with the lockfile location that introduced it. */ export interface LockfileDriftFinding { @@ -493,6 +503,7 @@ export interface CommitLintFinding { /** Structured analyzer output. Each analyzer fills its own key; more land as analyzers ship (#1477/#1478). */ export interface BriefFindings { dependency?: DependencyFinding[]; + dependencyDiff?: DependencyDiffFinding[]; lockfileDrift?: LockfileDriftFinding[]; secret?: SecretFinding[]; license?: LicenseFinding[]; diff --git a/review-enrichment/test/analyzer-registry.test.ts b/review-enrichment/test/analyzer-registry.test.ts index 509f2c8758..c3d68528b7 100644 --- a/review-enrichment/test/analyzer-registry.test.ts +++ b/review-enrichment/test/analyzer-registry.test.ts @@ -11,6 +11,7 @@ import { renderBrief } from "../dist/render.js"; const EXPECTED_ANALYZERS = [ "dependency", + "dependencyDiff", "lockfileDrift", "secret", "license", diff --git a/review-enrichment/test/dependency-diff.test.ts b/review-enrichment/test/dependency-diff.test.ts new file mode 100644 index 0000000000..393b630a0f --- /dev/null +++ b/review-enrichment/test/dependency-diff.test.ts @@ -0,0 +1,104 @@ +// Units for the dependency-diff analyzer (#2020). Own file so concurrent analyzer PRs don't collide. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + extractDependencyInventoryChanges, + extractDependencyChanges, +} from "../dist/analyzers/dependency-scan.js"; +import { + scanDependencyDiff, + scanDependencyDiffInventory, +} from "../dist/analyzers/dependency-diff.js"; +import { renderBrief } from "../dist/render.js"; + +const npmPatch = (lines: string[]) => + [`@@ -1,3 +1,3 @@`, ...lines.map((l) => (l.startsWith("+") || l.startsWith("-") ? l : ` ${l}`))].join( + "\n", + ); + +test("extractDependencyInventoryChanges: reports npm add, remove, and change", () => { + const files = [ + { + path: "package.json", + patch: npmPatch(['-"lodash": "4.17.20",', '+"lodash": "4.17.21",', '+"axios": "1.6.0",', '-"left-pad": "1.0.0",']), + }, + ]; + assert.deepEqual(extractDependencyInventoryChanges(files), [ + { + ecosystem: "npm", + package: "lodash", + from: "4.17.20", + to: "4.17.21", + direction: "change", + }, + { ecosystem: "npm", package: "axios", from: null, to: "1.6.0", direction: "add" }, + { ecosystem: "npm", package: "left-pad", from: "1.0.0", to: null, direction: "remove" }, + ]); + assert.deepEqual(extractDependencyChanges(files), [ + { + ecosystem: "npm", + package: "lodash", + from: "4.17.20", + to: "4.17.21", + }, + { ecosystem: "npm", package: "axios", from: null, to: "1.6.0" }, + ]); +}); + +test("extractDependencyInventoryChanges: reports PyPI requirements.txt changes", () => { + const files = [ + { + path: "requirements.txt", + patch: ["@@ -1,2 +1,2 @@", "-requests==2.31.0", "+requests==2.32.0"].join("\n"), + }, + ]; + assert.deepEqual(extractDependencyInventoryChanges(files), [ + { + ecosystem: "PyPI", + package: "requests", + from: "2.31.0", + to: "2.32.0", + direction: "change", + }, + ]); +}); + +test("extractDependencyInventoryChanges: respects the findings cap", () => { + const lines = Array.from({ length: 30 }, (_, i) => [`+"pkg${i}": "1.0.0",`]).flat(); + const files = [{ path: "package.json", patch: npmPatch(lines) }]; + assert.equal(extractDependencyInventoryChanges(files, {}, 3).length, 3); +}); + +test("extractDependencyInventoryChanges: keeps add/remove separate across manifest files", () => { + const files = [ + { + path: "apps/a/package.json", + patch: npmPatch(['-"axios": "1.6.0",']), + }, + { + path: "apps/b/package.json", + patch: npmPatch(['+"axios": "1.6.0",']), + }, + ]; + assert.deepEqual(extractDependencyInventoryChanges(files), [ + { ecosystem: "npm", package: "axios", from: "1.6.0", to: null, direction: "remove" }, + { ecosystem: "npm", package: "axios", from: null, to: "1.6.0", direction: "add" }, + ]); +}); + +test("scanDependencyDiffInventory: renders a public-safe brief", async () => { + const findings = await scanDependencyDiffInventory({ + files: [ + { + path: "package.json", + patch: npmPatch(['+"axios": "1.6.0",']), + }, + ], + }); + assert.deepEqual(findings, [ + { ecosystem: "npm", package: "axios", from: null, to: "1.6.0", direction: "add" }, + ]); + const { promptSection } = renderBrief({ dependencyDiff: findings }); + assert.match(promptSection, /Dependency inventory changes/); + assert.match(promptSection, /axios/); +}); diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index f3400057e2..1df8b14105 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -671,6 +671,7 @@ test("buildBrief: analyzer throw → degraded + partial, still returns a brief", const brief = await buildBrief({ repoFullName: "o/r", prNumber: 8, + analyzers: ["dependency"], files: [{ path: "package.json", patch: '+ "lodash": "4.17.20",' }], }); assert.equal(brief.partial, true); diff --git a/src/review/enrichment-analyzer-names.ts b/src/review/enrichment-analyzer-names.ts index f1f7d44de3..021296f400 100644 --- a/src/review/enrichment-analyzer-names.ts +++ b/src/review/enrichment-analyzer-names.ts @@ -5,11 +5,13 @@ export const REES_ANALYZER_NAMES = [ "dependency", + "dependencyDiff", "lockfileDrift", "secret", "license", "installScript", "heavyDependency", + "hardcodedUrl", "actionPin", "eol", "redos", @@ -39,6 +41,7 @@ export const REES_ANALYZER_NAMES = [ "todoMarker", "magicNumber", "conflictMarker", + "debugLeftover", "commitLint", ] as const;