From 8d2b4099e227369568c98ddac9233601a4337dfd Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 2 Jul 2026 21:07:34 -0700 Subject: [PATCH] feat(gate): add a lockfile-tamper-risk check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contributor supply-chain attacks can hand-edit a package-lock.json entry's resolved/integrity value (or point it at a non-registry host) without touching the corresponding package.json version, which the existing dependency-diff/OSV.dev analyzer never sees since it only looks for KNOWN-CVE versions, not resolved-URL/integrity tampering. Adds a deterministic lockfile_tamper_risk finding (src/review/lockfile-tamper.ts) wired through the same config-as-code chain as sizeGateMode/premergeContentRecheck (gate.lockfileIntegrity: off|advisory|block, off by default, config-as-code only — no DB column or dashboard toggle, matching sizeGateMode's footprint) so a repo that has not opted in sees zero behavior change. Closes #2563 --- .gittensory.yml.example | 9 + apps/gittensory-ui/public/openapi.json | 8 + src/openapi/schemas.ts | 1 + src/queue/processors.ts | 53 +++++ src/review/lockfile-tamper.ts | 182 +++++++++++++++++ src/rules/advisory.ts | 9 + src/signals/focus-manifest.ts | 10 + src/types.ts | 7 + test/unit/focus-manifest.test.ts | 32 ++- test/unit/gate-check-policy.test.ts | 27 +++ test/unit/lockfile-tamper-wiring.test.ts | 139 +++++++++++++ test/unit/lockfile-tamper.test.ts | 241 +++++++++++++++++++++++ 12 files changed, 716 insertions(+), 2 deletions(-) create mode 100644 src/review/lockfile-tamper.ts create mode 100644 test/unit/lockfile-tamper-wiring.test.ts create mode 100644 test/unit/lockfile-tamper.test.ts diff --git a/.gittensory.yml.example b/.gittensory.yml.example index 1f222656aa..eba7d13840 100644 --- a/.gittensory.yml.example +++ b/.gittensory.yml.example @@ -143,6 +143,15 @@ gate: size: mode: off + # Lockfile-tamper-risk gate. Scans a changed package-lock.json diff for a + # resolved/integrity value that changed WITHOUT the same package's version + # changing in a changed package.json, or a resolved URL outside + # registry.npmjs.org — the classic supply-chain hand-edit tell. Distinct + # from the OSV.dev known-CVE dependency scan (a different threat model). + # off | advisory | block. Default: off. Config-as-code only — no DB column + # or dashboard toggle; this can only be set here. + lockfileIntegrity: off + # Composite merge-readiness gate (no min score). # off | advisory | block. Default: off. mergeReadiness: off diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index 81b276a664..18c2ec3a3a 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -8720,6 +8720,14 @@ "type": "integer", "minimum": 0, "exclusiveMinimum": true + }, + "lockfileIntegrityGateMode": { + "type": "string", + "enum": [ + "off", + "advisory", + "block" + ] } }, "required": [ diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index 5c8d068ea8..77a367bd36 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -597,6 +597,7 @@ export const RepositorySettingsSchema = z qualityGateMinScore: z.number().nullable().optional(), slopGateMode: z.enum(["off", "advisory", "block"]), sizeGateMode: z.enum(["off", "advisory", "block"]).optional(), + lockfileIntegrityGateMode: z.enum(["off", "advisory", "block"]).optional(), gateDryRun: z.boolean().optional(), premergeContentRecheck: z.boolean().optional(), requireFreshRebaseWindowMinutes: z.number().int().positive().nullable().optional(), diff --git a/src/queue/processors.ts b/src/queue/processors.ts index 95d3ac7a16..3a85d53c8f 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -352,6 +352,7 @@ import { } from "../review/inline-comments"; import { evaluatePreMergeChecks } from "../review/pre-merge-checks"; import { secretLeakFinding } from "../review/safety"; +import { lockfileTamperRiskFinding } from "../review/lockfile-tamper"; import { buildIssuePlanComment, classifyPlanCommandRequest, @@ -430,6 +431,7 @@ import type { ContributorEvidenceRecord, ContributorRepoStatRecord, DetectedNotificationEvent, + GateRuleMode, GitHubWebhookPayload, IssueRecord, JobMessage, @@ -4895,6 +4897,7 @@ export function gateCheckPolicy( // thresholds default to 10 files / 1000 lines (advisory.ts constants); the live counts + guardrail-hit come from // the per-PR sizeContext threaded by the caller. sizeGateMode: settings.sizeGateMode, + lockfileIntegrityGateMode: settings.lockfileIntegrityGateMode, changedFileCount: sizeContext?.changedFileCount ?? null, changedLineCount: sizeContext?.changedLineCount ?? null, guardrailHit: sizeContext?.guardrailHit ?? false, @@ -5587,6 +5590,46 @@ export async function maybeAddSecretLeakFinding( } } +/** + * Lockfile-tamper-risk scan (#2563, opt-in via `lockfileIntegrityGateMode`). Scans a changed + * `package-lock.json`'s diff for a `resolved`/`integrity` value that changed without the corresponding + * `package.json` dependency version changing, or a `resolved` URL outside `registry.npmjs.org`, and on a hit + * appends ONE warning-severity `lockfile_tamper_risk` finding to the advisory BEFORE evaluateGateCheck runs — + * the gate treats that code as a blocker only when the repo has set `lockfileIntegrityGateMode: block` + * (rules/advisory.ts). Mode `off` (the default) skips the scan entirely so the advisory/gate stays + * byte-identical to today. Fail-safe: a file-load error is swallowed so it can never destabilize the gate. + */ +export async function maybeAddLockfileTamperFinding( + env: Env, + args: { + advisory: Awaited>; + repoFullName: string; + pullNumber: number; + lockfileIntegrityGateMode: GateRuleMode | undefined; + files: Awaited> | null; + }, +): Promise { + if (!args.lockfileIntegrityGateMode || args.lockfileIntegrityGateMode === "off") return; + try { + const files = + args.files ?? + (await listPullRequestFiles(env, args.repoFullName, args.pullNumber)); + const finding = lockfileTamperRiskFinding(files); + if (finding) args.advisory.findings.push(finding); + } catch (error) { + /* v8 ignore next -- fail-safe: a file-load error never destabilizes the gate. */ + console.error( + JSON.stringify({ + level: "error", + event: "lockfile_tamper_scan_failed", + repository: args.repoFullName, + pullNumber: args.pullNumber, + error: errorMessage(error), + }), + ); + } +} + /** * AI-assisted slop advisory (opt-in `slopAiAdvisory`). Appends at most one ADVISORY-only `ai_slop_advisory` * finding to the advisory; NEVER touches slopRisk or the gate (only the deterministic core can block). The @@ -6677,6 +6720,16 @@ async function maybePublishPrPublicSurface( files: await getReviewFiles(), }); + // Lockfile-tamper-risk scan (#2563): opt-in via `lockfileIntegrityGateMode` (default off — the scan is + // skipped entirely). getReviewFiles() is memoized, so this reuses the already-loaded diff when present. + await maybeAddLockfileTamperFinding(env, { + advisory, + repoFullName, + pullNumber: pr.number, + lockfileIntegrityGateMode: settings.lockfileIntegrityGateMode, + files: await getReviewFiles(), + }); + // Unresolved GitHub review threads (for example external security scanner inline findings) are blocking // review facts. Fetch them before gate evaluation so the normal blocker path drives the check-run, comment, // and disposition consistently. Fail-open on GitHub/GraphQL errors: a transient thread-read failure should not diff --git a/src/review/lockfile-tamper.ts b/src/review/lockfile-tamper.ts new file mode 100644 index 0000000000..a049cffbbb --- /dev/null +++ b/src/review/lockfile-tamper.ts @@ -0,0 +1,182 @@ +// Lockfile-tamper-risk gate check (#2563). Deterministic scan of a changed `package-lock.json` (or another +// `*.lock` file) diff for the classic supply-chain tell: a `resolved`/`integrity` value changed WITHOUT the +// corresponding `package.json` dependency version changing, or a `resolved` URL that points outside the public +// npm registry. Distinct from the OSV.dev CVE analyzer (review-enrichment/src/analyzers/lockfile-drift.ts) — +// that flags KNOWN-CVE versions; this flags tamper/integrity-substitution regardless of whether the substituted +// version has a published CVE. Config-driven, off by default (see rules/advisory.ts isConfiguredGateBlocker + +// signals/focus-manifest.ts gate.lockfileIntegrity) — this module only PRODUCES the finding; it never decides +// whether the finding blocks. + +import type { AdvisoryFinding, PullRequestFileRecord } from "../types"; + +const NPM_REGISTRY_HOST_RE = /^https:\/\/registry\.npmjs\.org\//i; + +// Package-lock "packages" entries are keyed either `"node_modules/"` (lockfileVersion 2/3) or a bare +// `""` (lockfileVersion 1 "dependencies" tree, and yarn/pnpm equivalents keep a similar bare-name header). +// Root ("": {...}) and pure container headers ("packages": {...}, "dependencies": {...}) are never package +// entries themselves. +const CONTAINER_KEYS = new Set(["", "packages", "dependencies", "devDependencies", "optionalDependencies"]); + +function npmPackageFromNodeModulesPath(path: string): string | null { + const marker = "node_modules/"; + const i = path.lastIndexOf(marker); + if (i < 0) return null; + const rest = path.slice(i + marker.length); + if (rest.startsWith("@")) { + const parts = rest.split("/"); + return parts.length >= 2 ? `${parts[0]}/${parts[1]}` : null; + } + return rest.split("/")[0] || null; +} + +/** True when `path`'s basename is `package-lock.json` — the only lockfile format this check parses today + * (npm/lockfileVersion 2-3 JSON shape). Matches ANY directory depth (root, `review-enrichment/`, + * `apps/gittensory-ui/`, or a future workspace) rather than a hardcoded path list, so a new workspace package + * is covered without a code change. */ +export function isNpmLockfilePath(path: string): boolean { + const normalized = path.replace(/\\/g, "/").toLowerCase(); + const slash = normalized.lastIndexOf("/"); + const basename = slash >= 0 ? normalized.slice(slash + 1) : normalized; + return basename === "package-lock.json"; +} + +type PatchLine = { sign: "+" | "-" | " "; content: string }; + +function* patchLines(patch: string): Generator { + for (const raw of patch.split("\n")) { + if (raw.startsWith("+++ ") || raw.startsWith("--- ") || raw.startsWith("@@")) continue; + const first = raw[0]; + if (first === "+") yield { sign: "+", content: raw.slice(1) }; + else if (first === "-") yield { sign: "-", content: raw.slice(1) }; + else yield { sign: " ", content: raw.slice(1) }; + } +} + +type LockfileTamperCandidate = { + file: string; + package: string; + /** True when a `resolved`/`integrity` value changed for this package block in the diff. */ + resolvedOrIntegrityChanged: boolean; + /** A `+resolved` URL seen for this package block that does not point at registry.npmjs.org, or null. */ + offRegistryResolvedUrl: string | null; +}; + +/** Parse one `package-lock.json` unified-diff patch for per-package resolved/integrity changes. Heuristic + * line-based scan (mirrors review-enrichment's lockfile-drift parser), not a full JSON parse — good enough to + * flag suspicious hunks without needing the complete (potentially huge) lockfile tree in memory. */ +function scanPackageLockPatch(path: string, patch: string): LockfileTamperCandidate[] { + const byPackage = new Map(); + let currentPackage: string | null = null; + let sawPackagesEntry = false; + for (const line of patchLines(patch)) { + const body = line.content.trim(); + const objectHeader = /^"([^"]+)"\s*:\s*\{/.exec(body); + if (objectHeader) { + const key = objectHeader[1]!; + const nodeModulesPackage = npmPackageFromNodeModulesPath(key); + if (nodeModulesPackage) { + currentPackage = nodeModulesPackage; + sawPackagesEntry = true; + } else if (!sawPackagesEntry && !CONTAINER_KEYS.has(key)) { + currentPackage = key; + } else { + currentPackage = null; + } + continue; + } + if (body === "}" || body.startsWith("},")) currentPackage = null; + if (!currentPackage || line.sign === " ") continue; + + const resolvedMatch = /^"resolved"\s*:\s*"([^"]*)"/.exec(body); + const integrityMatch = /^"integrity"\s*:\s*"([^"]*)"/.exec(body); + if (!resolvedMatch && !integrityMatch) continue; + + const entry = + byPackage.get(currentPackage) ?? + ({ file: path, package: currentPackage, resolvedOrIntegrityChanged: false, offRegistryResolvedUrl: null } satisfies LockfileTamperCandidate); + entry.resolvedOrIntegrityChanged = true; + if (resolvedMatch && line.sign === "+" && resolvedMatch[1] && !NPM_REGISTRY_HOST_RE.test(resolvedMatch[1])) { + entry.offRegistryResolvedUrl = resolvedMatch[1]; + } + byPackage.set(currentPackage, entry); + } + return [...byPackage.values()]; +} + +// `"": ""` inside a package.json dependency block, e.g. `"lodash": "^4.17.21",`. Line-based, not a +// full JSON parse — the same heuristic review-enrichment's dependency-scan.ts uses for the same shape. +const PACKAGE_JSON_DEP_RE = /^"([^"]+)"\s*:\s*"([^"]+)"/; + +/** Package names whose declared `package.json` version range CHANGED somewhere in this PR's diff (across every + * changed `package.json`, any dependency block) — a `+`/`-` pair with different range strings for the same key + * counts as changed; a line present on only one side (add/remove of the dependency entirely) also counts. */ +function packagesWithManifestVersionChange(files: PullRequestFileRecord[]): Set { + const changed = new Set(); + for (const file of files) { + if (file.path.replace(/\\/g, "/").toLowerCase().split("/").pop() !== "package.json") continue; + const patch = typeof file.payload?.patch === "string" ? file.payload.patch : ""; + if (!patch) continue; + const removedVersions = new Map(); + const addedVersions = new Map(); + for (const line of patchLines(patch)) { + if (line.sign === " ") continue; + const match = PACKAGE_JSON_DEP_RE.exec(line.content.trim()); + if (!match) continue; + const [, name, range] = match as unknown as [string, string, string]; + (line.sign === "+" ? addedVersions : removedVersions).set(name, range); + } + for (const [name, addedRange] of addedVersions) { + const removedRange = removedVersions.get(name); + if (removedRange === undefined || removedRange !== addedRange) changed.add(name); + } + for (const name of removedVersions.keys()) { + if (!addedVersions.has(name)) changed.add(name); + } + } + return changed; +} + +const MAX_FLAGGED_PACKAGES_IN_TITLE = 3; + +/** + * Scan every changed `package-lock.json` in the PR for a tamper-risk hunk: a `resolved`/`integrity` value + * changed WITHOUT the same package's version changing in a changed `package.json`, or a `resolved` URL outside + * `registry.npmjs.org`. Returns ONE `lockfile_tamper_risk` advisory finding on any hit, else null. Callers gate + * this on the repo's `lockfileIntegrityGateMode` (default `off` — see rules/advisory.ts) before invoking it. + */ +export function lockfileTamperRiskFinding(files: PullRequestFileRecord[]): AdvisoryFinding | null { + const lockfiles = files.filter((file) => isNpmLockfilePath(file.path)); + if (lockfiles.length === 0) return null; + const bumpedPackages = packagesWithManifestVersionChange(files); + + const flagged: { file: string; package: string; reason: "off_registry" | "unbumped_resolved" }[] = []; + for (const file of lockfiles) { + const patch = typeof file.payload?.patch === "string" ? file.payload.patch : ""; + if (!patch) continue; + for (const candidate of scanPackageLockPatch(file.path, patch)) { + if (candidate.offRegistryResolvedUrl) { + flagged.push({ file: candidate.file, package: candidate.package, reason: "off_registry" }); + } else if (candidate.resolvedOrIntegrityChanged && !bumpedPackages.has(candidate.package)) { + flagged.push({ file: candidate.file, package: candidate.package, reason: "unbumped_resolved" }); + } + } + } + if (flagged.length === 0) return null; + + const names = [...new Set(flagged.map((f) => f.package))]; + const shownNames = names.slice(0, MAX_FLAGGED_PACKAGES_IN_TITLE).join(", "); + const moreSuffix = names.length > MAX_FLAGGED_PACKAGES_IN_TITLE ? ` +${names.length - MAX_FLAGGED_PACKAGES_IN_TITLE} more` : ""; + const hasOffRegistry = flagged.some((f) => f.reason === "off_registry"); + const hasUnbumped = flagged.some((f) => f.reason === "unbumped_resolved"); + const detailParts: string[] = []; + if (hasOffRegistry) detailParts.push("a resolved URL points outside registry.npmjs.org"); + if (hasUnbumped) detailParts.push("a resolved/integrity value changed without a matching package.json version bump"); + + return { + code: "lockfile_tamper_risk", + severity: "warning", + title: `Possible lockfile tamper risk (${shownNames}${moreSuffix})`, + detail: `The lockfile diff for ${[...new Set(flagged.map((f) => f.file))].join(", ")} is suspicious: ${detailParts.join("; ")}. Affected package(s): ${names.join(", ")}.`, + action: "Re-run the package manager's install/lock command to regenerate the lockfile from package.json rather than hand-editing resolved/integrity entries, and confirm every resolved URL is on the public npm registry.", + }; +} diff --git a/src/rules/advisory.ts b/src/rules/advisory.ts index 4ab817adfc..5e758bd9f2 100644 --- a/src/rules/advisory.ts +++ b/src/rules/advisory.ts @@ -72,6 +72,11 @@ export type GateCheckPolicy = { * neutral gate → "manual" verdict, never auto-merged and never a hard failure. Defaults off; thresholds default * to 10 files / 1000 lines. This is a HOLD (advisory dry-run friendly), not a close. */ sizeGateMode?: GateRuleMode | undefined; + /** Lockfile-tamper-risk gate (#2563). When `block`, a `lockfile_tamper_risk` finding (produced by + * review/lockfile-tamper.ts when a changed package-lock.json's resolved/integrity value changed without a + * matching package.json version bump, or points off the npm registry) becomes a hard blocker. Defaults to + * `off` — the finding is never produced when off, and never blocks under `advisory`. */ + lockfileIntegrityGateMode?: GateRuleMode | undefined; /** Aggregate change size, threaded from the resolved file list (changedLineCount = additions + deletions). */ changedFileCount?: number | null | undefined; changedLineCount?: number | null | undefined; @@ -874,6 +879,10 @@ function isConfiguredGateBlocker(finding: AdvisoryFinding, policy: GateCheckPoli // Self-authored linked-issue gate: blocks only when the maintainer opts in with `block`. Defaults to // advisory — the finding surfaces in the panel without ever closing the PR unless explicitly configured. if (code === "self_authored_linked_issue") return gateMode(policy.selfAuthoredLinkedIssueGateMode ?? "advisory") === "block"; + // Lockfile-tamper-risk gate (#2563): blocks only when the maintainer opts in with `block`. Defaults to `off` + // (the finding is never even produced — see maybeAddLockfileTamperFinding's mode gate in queue/processors.ts), + // so this branch only matters once a repo has explicitly turned the scan on. + if (code === "lockfile_tamper_risk") return gateMode(policy.lockfileIntegrityGateMode ?? "off") === "block"; return false; } diff --git a/src/signals/focus-manifest.ts b/src/signals/focus-manifest.ts index 549d533093..8c8847e9b9 100644 --- a/src/signals/focus-manifest.ts +++ b/src/signals/focus-manifest.ts @@ -32,6 +32,11 @@ export type FocusManifestGateConfig = { slopMinScore: number | null; slopAiAdvisory: boolean | null; sizeMode: GateRuleMode | null; + /** `gate.lockfileIntegrity` (#2563): off|advisory|block, off by default. When not off, a changed + * `package-lock.json` diff is scanned for a `resolved`/`integrity` change unaccompanied by a matching + * `package.json` version bump, or a `resolved` URL outside `registry.npmjs.org` — a `lockfile_tamper_risk` + * finding (`block` additionally hard-blocks). Config-as-code only — no DB column or dashboard toggle. */ + lockfileIntegrityMode: GateRuleMode | null; aiReviewMode: GateRuleMode | null; aiReviewByok: boolean | null; aiReviewProvider: "anthropic" | "openai" | null; @@ -303,6 +308,7 @@ const EMPTY_GATE_CONFIG: FocusManifestGateConfig = { slopMinScore: null, slopAiAdvisory: null, sizeMode: null, + lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, @@ -526,6 +532,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu slopMinScore: normalizeOptionalScore(slopRecord?.minScore, "gate.slop.minScore", warnings), slopAiAdvisory: normalizeOptionalBoolean(slopRecord?.aiAdvisory, "gate.slop.aiAdvisory", warnings), sizeMode: normalizeOptionalGateMode(sizeRecord?.mode, "gate.size.mode", warnings), + lockfileIntegrityMode: normalizeOptionalGateMode(record.lockfileIntegrity, "gate.lockfileIntegrity", warnings), aiReviewMode: normalizeOptionalGateMode(aiReviewRecord?.mode, "gate.aiReview.mode", warnings), aiReviewByok: normalizeOptionalBoolean(aiReviewRecord?.byok, "gate.aiReview.byok", warnings), aiReviewProvider: normalizeOptionalEnum(aiReviewRecord?.provider, "gate.aiReview.provider", ["anthropic", "openai"] as const, warnings), @@ -558,6 +565,7 @@ function parseGateConfig(value: JsonValue | undefined, warnings: string[]): Focu gate.slopMinScore !== null || gate.slopAiAdvisory !== null || gate.sizeMode !== null || + gate.lockfileIntegrityMode !== null || gate.aiReviewMode !== null || gate.aiReviewByok !== null || gate.aiReviewProvider !== null || @@ -592,6 +600,7 @@ export function gateConfigToJson(gate: FocusManifestGateConfig): JsonValue { out.readiness = readiness; } if (gate.sizeMode !== null) out.size = { mode: gate.sizeMode }; + if (gate.lockfileIntegrityMode !== null) out.lockfileIntegrity = gate.lockfileIntegrityMode; if (gate.slopMode !== null || gate.slopMinScore !== null || gate.slopAiAdvisory !== null) { const slop: Record = {}; if (gate.slopMode !== null) slop.mode = gate.slopMode; @@ -1242,6 +1251,7 @@ export function resolveEffectiveSettings( if (gate.readinessMode !== null) effective.qualityGateMode = gate.readinessMode; if (gate.readinessMinScore !== null) effective.qualityGateMinScore = gate.readinessMinScore; if (gate.sizeMode !== null) effective.sizeGateMode = gate.sizeMode; + if (gate.lockfileIntegrityMode !== null) effective.lockfileIntegrityGateMode = gate.lockfileIntegrityMode; if (gate.slopMode !== null) effective.slopGateMode = gate.slopMode; if (gate.slopMinScore !== null) effective.slopGateMinScore = gate.slopMinScore; if (gate.slopAiAdvisory !== null) effective.slopAiAdvisory = gate.slopAiAdvisory; diff --git a/src/types.ts b/src/types.ts index a5d6fe11d4..e9cc94b61f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -537,6 +537,13 @@ export type RepositorySettings = { * >= 10 changed files OR >= 1000 changed (added+deleted) lines that would otherwise pass is HELD for manual review * (neutral gate → "manual" verdict), never auto-merged and never a hard failure. Opt-in via `gate.size.mode`. */ sizeGateMode?: GateRuleMode | undefined; + /** Lockfile-tamper-risk gate (#2563). `off` (default/absent) = no scan; `advisory`/`block` = a changed + * `package-lock.json` whose diff changes a `resolved`/`integrity` value WITHOUT the same package's version + * changing in a changed `package.json`, or whose `resolved` URL points outside `registry.npmjs.org`, produces + * a `lockfile_tamper_risk` finding (`block` additionally hard-blocks). Distinct from the OSV.dev CVE analyzer + * in review-enrichment — this is a tamper/integrity-substitution check, not a known-CVE check. Config-as-code + * only — no DB column or dashboard toggle; set via `.gittensory.yml gate.lockfileIntegrity`. */ + lockfileIntegrityGateMode?: GateRuleMode | undefined; /** Dry-run disposition (#gate-dryrun). When true, the gate renders the would-be merge/close/manual verdict (every * advisory sub-gate promoted to block) WITHOUT enforcing — the posted check stays non-blocking. Lets advisory mode * preview exactly what it would do before the maintainer flips to real enforcement. Default off. */ diff --git a/test/unit/focus-manifest.test.ts b/test/unit/focus-manifest.test.ts index 3d1c568472..622c54c857 100644 --- a/test/unit/focus-manifest.test.ts +++ b/test/unit/focus-manifest.test.ts @@ -182,6 +182,8 @@ describe("parseFocusManifestContent", () => { expect(manifest.gate.aiReviewCloseConfidence).toBeNull(); // #2552: requireFreshRebaseWindow also round-trips through the real parser. expect(manifest.gate.requireFreshRebaseWindowMinutes).toBe(10); + // #2563: gate.lockfileIntegrity also round-trips through the real parser. + expect(manifest.gate.lockfileIntegrityMode).toBe("off"); }); }); @@ -494,7 +496,7 @@ describe("compileFocusManifestPolicy", () => { issueDiscoveryPolicy: "neutral", maintainerNotes: [], publicNotes: ["Keep PRs focused.", "Maximize your reward payout"], - gate: { present: false, enabled: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null }, + gate: { present: false, enabled: null, pack: null, linkedIssue: null, duplicates: null, readinessMode: null, readinessMinScore: null, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null }, settings: {}, review: { present: false, footerText: null, note: null, fields: {}, profile: null, inlineComments: null, pathInstructions: [], instructions: null, excludePaths: [], preMergeChecks: [] }, features: { present: false, rag: null, reputation: null, unifiedComment: null, safety: null }, @@ -802,7 +804,7 @@ describe("parseFocusManifest gate config", () => { // the block→advisory deprecation-downgrade behavior itself is covered separately below. const m = parseFocusManifest({ gate: { linkedIssue: "block", duplicates: "advisory", readiness: { mode: "advisory", minScore: 70 } } }); expect(m.present).toBe(true); - expect(m.gate).toEqual({ present: true, enabled: null, pack: null, linkedIssue: "block", duplicates: "advisory", readinessMode: "advisory", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null }); + expect(m.gate).toEqual({ present: true, enabled: null, pack: null, linkedIssue: "block", duplicates: "advisory", readinessMode: "advisory", readinessMinScore: 70, slopMode: null, slopMinScore: null, slopAiAdvisory: null, sizeMode: null, lockfileIntegrityMode: null, aiReviewMode: null, aiReviewByok: null, aiReviewProvider: null, aiReviewModel: null, aiReviewAllAuthors: null, aiReviewCloseConfidence: null, mergeReadiness: null, selfAuthoredLinkedIssue: null, manifestPolicy: null, dryRun: null, firstTimeContributorGrace: null, premergeContentRecheck: null, requireFreshRebaseWindowMinutes: null }); }); it("parses gate.mergeReadiness + gate.firstTimeContributorGrace, round-trips them, and warns on bad values (#822)", () => { @@ -1861,6 +1863,32 @@ describe("gate.size manual-review hold config (#gate-size)", () => { }); }); +describe("gate.lockfileIntegrity lockfile-tamper-risk gate config (#2563)", () => { + it("parses gate.lockfileIntegrity, sets present, round-trips via gateConfigToJson, and resolves into effective settings", () => { + const m = parseFocusManifest({ gate: { lockfileIntegrity: "block" } }); + expect(m.gate.lockfileIntegrityMode).toBe("block"); + expect(m.gate.present).toBe(true); + expect(gateConfigToJson(m.gate)).toMatchObject({ lockfileIntegrity: "block" }); + const round = parseFocusManifest({ gate: gateConfigToJson(m.gate) }); + expect(round.gate.lockfileIntegrityMode).toBe("block"); + const eff = resolveEffectiveSettings({} as unknown as RepositorySettings, m); + expect(eff.lockfileIntegrityGateMode).toBe("block"); + }); + + it("defaults to unset/null when omitted — byte-identical to today (off)", () => { + const m = parseFocusManifest({}); + expect(m.gate.lockfileIntegrityMode).toBeNull(); + const eff = resolveEffectiveSettings({} as unknown as RepositorySettings, m); + expect(eff.lockfileIntegrityGateMode).toBeUndefined(); + }); + + it("warns and drops an invalid mode value rather than silently coercing it", () => { + const m = parseFocusManifest({ gate: { lockfileIntegrity: "sometimes" as never } }); + expect(m.gate.lockfileIntegrityMode).toBeNull(); + expect(m.warnings.some((w) => /gate\.lockfileIntegrity/i.test(w))).toBe(true); + }); +}); + describe("gate.dryRun dry-run disposition config (#gate-dryrun)", () => { it("parses gate.dryRun, sets present, and round-trips via gateConfigToJson", () => { const m = parseFocusManifest({ gate: { dryRun: true } }); diff --git a/test/unit/gate-check-policy.test.ts b/test/unit/gate-check-policy.test.ts index 436ca62379..3a989b2e2d 100644 --- a/test/unit/gate-check-policy.test.ts +++ b/test/unit/gate-check-policy.test.ts @@ -764,6 +764,33 @@ describe("size + guardrail manual-review HOLD (#gate-size / #gate-guardrail)", ( }); }); +describe("lockfile-tamper-risk gate blocker (#2563)", () => { + const lockfileAdvisory = (): Advisory => ({ + ...missingIssueAdvisory(), + findings: [{ code: "lockfile_tamper_risk", title: "Possible lockfile tamper risk (lodash)", severity: "warning", detail: "resolved/integrity changed without a version bump.", action: "Regenerate the lockfile." }], + }); + + it("blocks (failure) under lockfileIntegrityGateMode: block, confirmed contributor", () => { + const result = evaluateGateCheck(lockfileAdvisory(), { lockfileIntegrityGateMode: "block", confirmedContributor: true }); + expect(result.conclusion).toBe("failure"); + expect(result.blockers.map((b) => b.code)).toContain("lockfile_tamper_risk"); + }); + + it("stays advisory (never blocks) under off (default/unset) or advisory mode", () => { + expect(evaluateGateCheck(lockfileAdvisory(), {}).conclusion).toBe("success"); // unset ⇒ off + expect(evaluateGateCheck(lockfileAdvisory(), { lockfileIntegrityGateMode: "off" }).conclusion).toBe("success"); + const advisoryResult = evaluateGateCheck(lockfileAdvisory(), { lockfileIntegrityGateMode: "advisory" }); + expect(advisoryResult.conclusion).toBe("success"); + expect(advisoryResult.warnings.map((w) => w.code)).toContain("lockfile_tamper_risk"); + }); + + it("resolveEffectiveSettings maps gate.lockfileIntegrity → lockfileIntegrityGateMode, and gateCheckPolicy threads it", () => { + const eff = resolveEffectiveSettings(settings({}), parseFocusManifest({ gate: { lockfileIntegrity: "block" } })); + expect(eff.lockfileIntegrityGateMode).toBe("block"); + expect(gateCheckPolicy(settings({ lockfileIntegrityGateMode: "block" }), null, true).lockfileIntegrityGateMode).toBe("block"); + }); +}); + describe("dry-run disposition (#gate-dryrun): would-be verdict without enforcing", () => { // #disposition-redesign: the dry-run shadow promotes ONLY the AI sub-gate. CLOSE is driven by AI confidence; the // advisory signals (linked issue, readiness/quality, slop, duplicates) can NEVER drive a would-be close. diff --git a/test/unit/lockfile-tamper-wiring.test.ts b/test/unit/lockfile-tamper-wiring.test.ts new file mode 100644 index 0000000000..49a77f2aa3 --- /dev/null +++ b/test/unit/lockfile-tamper-wiring.test.ts @@ -0,0 +1,139 @@ +import { describe, expect, it } from "vitest"; +import { maybeAddLockfileTamperFinding } from "../../src/queue/processors"; +import type { Advisory, PullRequestFileRecord } from "../../src/types"; +import { createTestEnv } from "../helpers/d1"; + +function advisory(): Advisory { + return { + id: "adv-1", + targetType: "pull_request", + targetKey: "acme/widgets#7", + repoFullName: "acme/widgets", + pullNumber: 7, + headSha: "sha7", + conclusion: "neutral", + severity: "info", + title: "Gittensory advisory available", + summary: "ok", + findings: [], + generatedAt: "2026-07-02T00:00:00.000Z", + }; +} + +const TAMPERED_LOCKFILE_PATCH = [ + '@@ -1,4 +1,4 @@', + ' "node_modules/lodash": {', + '- "integrity": "sha512-oldoldold=="', + '+ "integrity": "sha512-tamperedtampered=="', + ' },', +].join("\n"); + +function tamperedFiles(): PullRequestFileRecord[] { + return [ + { + repoFullName: "acme/widgets", + pullNumber: 7, + path: "package-lock.json", + status: "modified", + additions: 1, + deletions: 1, + changes: 2, + payload: { patch: TAMPERED_LOCKFILE_PATCH }, + }, + ]; +} + +describe("maybeAddLockfileTamperFinding (#2563 wiring)", () => { + it("mode OFF (default): does not scan, no finding appended", async () => { + const env = createTestEnv(); + const adv = advisory(); + await maybeAddLockfileTamperFinding(env, { + advisory: adv, + repoFullName: "acme/widgets", + pullNumber: 7, + lockfileIntegrityGateMode: "off", + files: tamperedFiles(), + }); + expect(adv.findings).toEqual([]); + }); + + it("mode UNDEFINED (unset ⇒ treated as off): does not scan, no finding appended", async () => { + const env = createTestEnv(); + const adv = advisory(); + await maybeAddLockfileTamperFinding(env, { + advisory: adv, + repoFullName: "acme/widgets", + pullNumber: 7, + lockfileIntegrityGateMode: undefined, + files: tamperedFiles(), + }); + expect(adv.findings).toEqual([]); + }); + + it("mode ADVISORY: a tampered lockfile appends a warning-severity lockfile_tamper_risk finding", async () => { + const env = createTestEnv(); + const adv = advisory(); + await maybeAddLockfileTamperFinding(env, { + advisory: adv, + repoFullName: "acme/widgets", + pullNumber: 7, + lockfileIntegrityGateMode: "advisory", + files: tamperedFiles(), + }); + const finding = adv.findings.find((f) => f.code === "lockfile_tamper_risk"); + expect(finding).toBeDefined(); + expect(finding?.severity).toBe("warning"); + }); + + it("mode BLOCK: a clean (non-tampered) lockfile change appends no finding", async () => { + const env = createTestEnv(); + const adv = advisory(); + const cleanPatch = ['@@ -1,6 +1,6 @@', ' "node_modules/lodash": {', '- "version": "4.17.20",', '+ "version": "4.17.21",', ' },'].join("\n"); + await maybeAddLockfileTamperFinding(env, { + advisory: adv, + repoFullName: "acme/widgets", + pullNumber: 7, + lockfileIntegrityGateMode: "block", + files: [{ repoFullName: "acme/widgets", pullNumber: 7, path: "package-lock.json", status: "modified", additions: 1, deletions: 1, changes: 2, payload: { patch: cleanPatch } }], + }); + expect(adv.findings).toEqual([]); + }); + + it("reuses the passed files (no DB fetch) when files is non-null, and lazy-loads when null", async () => { + const env = createTestEnv(); + const adv = advisory(); + // files: null with no matching DB rows ⇒ listPullRequestFiles returns [] ⇒ no finding, no throw. + await maybeAddLockfileTamperFinding(env, { + advisory: adv, + repoFullName: "acme/widgets", + pullNumber: 7, + lockfileIntegrityGateMode: "advisory", + files: null, + }); + expect(adv.findings).toEqual([]); + }); + + it("fail-safe: a thrown error while loading files never propagates and appends no finding", async () => { + const env = createTestEnv(); + const adv = advisory(); + const throwingEnv = { + ...env, + DB: { + ...env.DB, + prepare: () => { + throw new Error("boom"); + }, + }, + } as unknown as typeof env; + await expect( + maybeAddLockfileTamperFinding(throwingEnv, { + advisory: adv, + repoFullName: "acme/widgets", + pullNumber: 7, + lockfileIntegrityGateMode: "advisory", + files: null, + }), + ).resolves.toBeUndefined(); + expect(adv.findings).toEqual([]); + }); +}); diff --git a/test/unit/lockfile-tamper.test.ts b/test/unit/lockfile-tamper.test.ts new file mode 100644 index 0000000000..b5b9a49892 --- /dev/null +++ b/test/unit/lockfile-tamper.test.ts @@ -0,0 +1,241 @@ +import { describe, expect, it } from "vitest"; +import { isNpmLockfilePath, lockfileTamperRiskFinding } from "../../src/review/lockfile-tamper"; +import type { PullRequestFileRecord } from "../../src/types"; + +function fileRecord(over: Partial & { path: string }): PullRequestFileRecord { + return { repoFullName: "acme/widgets", pullNumber: 3, status: "modified", additions: 1, deletions: 0, changes: 1, payload: {}, ...over }; +} + +function lockfilePatch(body: string): PullRequestFileRecord { + return fileRecord({ path: "package-lock.json", payload: { patch: body } }); +} + +function manifestPatch(body: string): PullRequestFileRecord { + return fileRecord({ path: "package.json", payload: { patch: body } }); +} + +describe("isNpmLockfilePath", () => { + it("matches package-lock.json at any depth", () => { + expect(isNpmLockfilePath("package-lock.json")).toBe(true); + expect(isNpmLockfilePath("review-enrichment/package-lock.json")).toBe(true); + expect(isNpmLockfilePath("apps/gittensory-ui/package-lock.json")).toBe(true); + expect(isNpmLockfilePath("PACKAGE-LOCK.JSON")).toBe(true); // case-insensitive + }); + + it("does not match other lockfiles or unrelated files", () => { + expect(isNpmLockfilePath("yarn.lock")).toBe(false); + expect(isNpmLockfilePath("pnpm-lock.yaml")).toBe(false); + expect(isNpmLockfilePath("src/package-lock.json.ts")).toBe(false); + expect(isNpmLockfilePath("package.json")).toBe(false); + }); +}); + +describe("lockfileTamperRiskFinding", () => { + it("returns null when no lockfile changed", () => { + expect(lockfileTamperRiskFinding([fileRecord({ path: "src/index.ts", payload: { patch: "@@\n+const x = 1;" } })])).toBeNull(); + }); + + it("returns null for a lockfile change with no patch", () => { + expect(lockfileTamperRiskFinding([fileRecord({ path: "package-lock.json", payload: {} })])).toBeNull(); + }); + + it("does NOT trigger a legitimate dependency bump (version + resolved + integrity all change together)", () => { + const lockPatch = [ + '@@ -100,8 +100,8 @@', + ' "node_modules/lodash": {', + '- "version": "4.17.20",', + '- "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.20.tgz",', + '- "integrity": "sha512-oldoldold=="', + '+ "version": "4.17.21",', + '+ "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz",', + '+ "integrity": "sha512-newnewnew=="', + ' },', + ].join("\n"); + const manifestDiff = ['@@ -10,7 +10,7 @@', ' "dependencies": {', '- "lodash": "^4.17.20",', '+ "lodash": "^4.17.21",'].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch), manifestPatch(manifestDiff)]); + expect(finding).toBeNull(); + }); + + it("triggers on a hand-edited resolved/integrity with NO corresponding package.json version bump", () => { + const lockPatch = [ + '@@ -100,8 +100,8 @@', + ' "node_modules/lodash": {', + '- "version": "4.17.20",', + '- "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.20.tgz",', + '- "integrity": "sha512-oldoldold=="', + '+ "version": "4.17.20",', + '+ "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.20.tgz",', + '+ "integrity": "sha512-tamperedtampered=="', + ' },', + ].join("\n"); + // No package.json change at all — the resolved tree was hand-edited without any manifest bump. + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch)]); + expect(finding).not.toBeNull(); + expect(finding?.code).toBe("lockfile_tamper_risk"); + expect(finding?.severity).toBe("warning"); + expect(finding?.title).toContain("lodash"); + }); + + it("triggers when package.json changed but NOT the flagged package's version", () => { + const lockPatch = [ + '@@ -100,8 +100,8 @@', + ' "node_modules/lodash": {', + '- "version": "4.17.20",', + '- "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.20.tgz",', + '- "integrity": "sha512-oldoldold=="', + '+ "version": "4.17.20",', + '+ "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.20.tgz",', + '+ "integrity": "sha512-tamperedtampered=="', + ' },', + ].join("\n"); + // package.json changed, but for a DIFFERENT package (express), so lodash's unbumped resolved is still suspicious. + const manifestDiff = ['@@ -10,7 +10,7 @@', ' "dependencies": {', '- "express": "^4.18.0",', '+ "express": "^4.19.0",'].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch), manifestPatch(manifestDiff)]); + expect(finding).not.toBeNull(); + expect(finding?.detail).toContain("lodash"); + }); + + it("triggers on a resolved URL outside the npm registry, even with a version bump", () => { + const lockPatch = [ + '@@ -100,8 +100,8 @@', + ' "node_modules/lodash": {', + '- "version": "4.17.20",', + '- "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.20.tgz",', + '- "integrity": "sha512-oldoldold=="', + '+ "version": "4.17.21",', + '+ "resolved": "https://evil.example.com/lodash/-/lodash-4.17.21.tgz",', + '+ "integrity": "sha512-newnewnew=="', + ' },', + ].join("\n"); + const manifestDiff = ['@@ -10,7 +10,7 @@', ' "dependencies": {', '- "lodash": "^4.17.20",', '+ "lodash": "^4.17.21",'].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch), manifestPatch(manifestDiff)]); + expect(finding).not.toBeNull(); + expect(finding?.detail).toContain("outside registry.npmjs.org"); + }); + + it("scans review-enrichment/package-lock.json and apps/gittensory-ui/package-lock.json the same way", () => { + const lockPatch = [ + '@@ -1,4 +1,4 @@', + ' "node_modules/left-pad": {', + '- "integrity": "sha512-oldoldold=="', + '+ "integrity": "sha512-tamperedtampered=="', + ' },', + ].join("\n"); + const finding = lockfileTamperRiskFinding([fileRecord({ path: "review-enrichment/package-lock.json", payload: { patch: lockPatch } })]); + expect(finding).not.toBeNull(); + expect(finding?.detail).toContain("review-enrichment/package-lock.json"); + + const findingUi = lockfileTamperRiskFinding([fileRecord({ path: "apps/gittensory-ui/package-lock.json", payload: { patch: lockPatch } })]); + expect(findingUi).not.toBeNull(); + expect(findingUi?.detail).toContain("apps/gittensory-ui/package-lock.json"); + }); + + it("bare (non-node_modules) top-level package key is tracked as a package name (lockfileVersion 1 shape)", () => { + const lockPatch = ['@@ -1,4 +1,4 @@', ' "dependencies": {', ' "left-pad": {', '- "integrity": "sha512-oldoldold=="', '+ "integrity": "sha512-tamperedtampered=="', ' }'].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch)]); + expect(finding).not.toBeNull(); + expect(finding?.detail).toContain("left-pad"); + }); + + it("ignores unrelated context lines and only reacts to resolved/integrity keys", () => { + const lockPatch = ['@@ -1,4 +1,4 @@', ' "node_modules/lodash": {', ' "version": "4.17.21",', '- "dev": true', '+ "dev": false', ' },'].join("\n"); + expect(lockfileTamperRiskFinding([lockfilePatch(lockPatch)])).toBeNull(); + }); + + it("resolves a scoped package name from a node_modules/@scope/name path", () => { + const lockPatch = ['@@ -1,4 +1,4 @@', ' "node_modules/@babel/core": {', '- "integrity": "sha512-oldoldold=="', '+ "integrity": "sha512-tamperedtampered=="', ' },'].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch)]); + expect(finding).not.toBeNull(); + expect(finding?.detail).toContain("@babel/core"); + }); + + it("treats a node_modules/ key with nothing after the marker as not a package entry", () => { + // rest.split("/")[0] is "" (falsy) for a key that is exactly "node_modules/" — npmPackageFromNodeModulesPath + // returns null, and since "node_modules/" is not in CONTAINER_KEYS but sawPackagesEntry may already be true + // from a prior real entry, it is skipped rather than mis-tracked as a package named "node_modules/". + const lockPatch = [ + '@@ -1,8 +1,8 @@', + ' "node_modules/lodash": {', + '- "version": "4.17.20",', + '+ "version": "4.17.21",', + ' },', + ' "node_modules/": {', + '- "integrity": "sha512-oldoldold=="', + '+ "integrity": "sha512-tamperedtampered=="', + ' },', + ].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch)]); + expect(finding).toBeNull(); + }); + + it("falls back to the literal key for a malformed node_modules/@scope path (no package segment)", () => { + // npmPackageFromNodeModulesPath returns null for a bare "@scope" segment (no "/name" after it); the parser + // then falls through to treating the full key as a literal (non-container) package name — still flagged, + // just under the raw key rather than a resolved "@scope/name". + const lockPatch = ['@@ -1,4 +1,4 @@', ' "node_modules/@babel": {', '- "integrity": "sha512-oldoldold=="', '+ "integrity": "sha512-tamperedtampered=="', ' },'].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch)]); + expect(finding).not.toBeNull(); + expect(finding?.detail).toContain("node_modules/@babel"); + }); + + it("ignores a package.json file with no patch at all", () => { + const lockPatch = ['@@ -1,4 +1,4 @@', ' "node_modules/lodash": {', '- "integrity": "sha512-oldoldold=="', '+ "integrity": "sha512-tamperedtampered=="', ' },'].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch), fileRecord({ path: "package.json", payload: {} })]); + expect(finding).not.toBeNull(); + }); + + it("ignores a package.json patch line that is not a string-valued key (not a dependency assignment)", () => { + const lockPatch = ['@@ -1,4 +1,4 @@', ' "node_modules/lodash": {', '- "integrity": "sha512-oldoldold=="', '+ "integrity": "sha512-tamperedtampered=="', ' },'].join("\n"); + const manifestDiff = ['@@ -1,3 +1,3 @@', ' "dependencies": {', '- "private": true,', '+ "private": false,'].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch), manifestPatch(manifestDiff)]); + expect(finding).not.toBeNull(); + }); + + it("does not flag a package whose manifest range is REMOVED and RE-ADDED with the identical range (no real bump)", () => { + const lockPatch = [ + '@@ -100,8 +100,8 @@', + ' "node_modules/lodash": {', + '- "version": "4.17.20",', + '- "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.20.tgz",', + '- "integrity": "sha512-oldoldold=="', + '+ "version": "4.17.20",', + '+ "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.20.tgz",', + '+ "integrity": "sha512-tamperedtampered=="', + ' },', + ].join("\n"); + // Manifest re-orders (removes + re-adds) lodash at the SAME range, and genuinely bumps express — proves the + // "identical range" case does not spuriously mark lodash as bumped while a real bump still registers. + const manifestDiff = [ + '@@ -10,8 +10,8 @@', + ' "dependencies": {', + '- "express": "^4.18.0",', + '- "lodash": "^4.17.20",', + '+ "express": "^4.19.0",', + '+ "lodash": "^4.17.20",', + ].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch), manifestPatch(manifestDiff)]); + expect(finding).not.toBeNull(); + expect(finding?.detail).toContain("lodash"); + }); + + it("counts a fully removed manifest dependency as a version change (no matching add)", () => { + const lockPatch = ['@@ -1,4 +1,4 @@', ' "node_modules/lodash": {', '- "integrity": "sha512-oldoldold=="', '+ "integrity": "sha512-tamperedtampered=="', ' },'].join("\n"); + // lodash is removed from package.json entirely (no corresponding "+" line) — still counts as a version + // change for tamper-risk purposes (the dependency's presence itself changed), so lodash is NOT flagged. + const manifestDiff = ['@@ -10,4 +10,3 @@', ' "dependencies": {', '- "lodash": "^4.17.20",', ' "express": "^4.18.0"'].join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch), manifestPatch(manifestDiff)]); + expect(finding).toBeNull(); + }); + + it("collapses multiple flagged packages into one finding, capping the title list and reporting the overflow count", () => { + const packages = ["alpha", "bravo", "charlie", "delta", "echo"]; + const lockPatch = packages + .map((name) => [` "node_modules/${name}": {`, '- "integrity": "sha512-oldoldold=="', '+ "integrity": "sha512-tamperedtampered=="', " },"].join("\n")) + .join("\n"); + const finding = lockfileTamperRiskFinding([lockfilePatch(lockPatch)]); + expect(finding).not.toBeNull(); + expect(finding?.title).toContain("+2 more"); + expect(finding?.detail).toContain("alpha"); + expect(finding?.detail).toContain("echo"); + }); +});