diff --git a/review-enrichment/src/analyzers/native-build.ts b/review-enrichment/src/analyzers/native-build.ts new file mode 100644 index 0000000000..697c2cf828 --- /dev/null +++ b/review-enrichment/src/analyzers/native-build.ts @@ -0,0 +1,139 @@ +// Native-build / install-cost analyzer (#1512). For each dependency a PR newly adds or upgrades, flags the ones +// whose install does real work the manifest diff never shows: an npm package that compiles a native addon +// (node-gyp / gypfile) on install, or a PyPI release that ships no prebuilt wheel (sdist-only) so pip compiles from +// source. Both are a hidden CI cold-start cost and a frequent cross-platform breakage source. Factual signals from +// the registry metadata only — the no-checkout reviewer can fetch neither. Reports package@version + the property. +import type { EnrichRequest, NativeBuildFinding } from "../types.js"; +import { extractDependencyChanges } from "./dependency-scan.js"; + +const MAX_QUERIES = 25; +const INSTALL_HOOKS = ["preinstall", "install", "postinstall"]; +// Tokens in an install-lifecycle script that indicate a native toolchain runs on install. +const NATIVE_TOOL_RE = /\b(node-gyp|node-pre-gyp|prebuild|prebuild-install|cmake-js|node-addon-api|nan)\b/; +// Tokens that mean prebuilt binaries are DOWNLOADED (compile only as a fallback for an unmatched platform/ABI). +const PREBUILT_TOOL_RE = /\b(node-pre-gyp|prebuild-install)\b/; + +const NPM_PACKAGE_RE = /^(?:@[a-z0-9][a-z0-9._-]*\/[a-z0-9][a-z0-9._-]*|[a-z0-9][a-z0-9._-]*)$/; +const PYPI_PACKAGE_RE = /^[A-Za-z0-9](?:[A-Za-z0-9._-]*[A-Za-z0-9])?$/; +const SEMVER_RE = /^(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)\.(?:0|[1-9]\d*)(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$/; +// PyPI versions are PEP 440, not semver: `1.0`, `24.1`, `1.0rc1`, `1.0.post1`, `1!2.0`. Validate only that the +// string is non-empty and URL-path-safe (it goes into the version JSON URL) rather than imposing semver. +const PYPI_VERSION_RE = /^[A-Za-z0-9][A-Za-z0-9._+!-]{0,63}$/; + +/** Is this dependency change one we can query a registry for (supported ecosystem + URL-safe name/version)? */ +function isQueryable(change: { ecosystem: string; package: string; to: string }): boolean { + if (change.ecosystem === "npm") return NPM_PACKAGE_RE.test(change.package) && SEMVER_RE.test(change.to); + if (change.ecosystem === "PyPI") return PYPI_PACKAGE_RE.test(change.package) && PYPI_VERSION_RE.test(change.to); + return false; +} + +interface ScanLimits { + maxQueries?: number; +} + +interface ScanOptions { + signal?: AbortSignal; + limits?: ScanLimits; +} + +/** npm packument version metadata, the subset that signals a native build. */ +export interface NpmVersionMeta { + gypfile?: boolean; + binary?: unknown; + scripts?: Record; +} + +/** Pure: does this npm version compile a native addon on install? Returns a reason (+ whether a prebuilt fallback + * exists), or null. Signals: `gypfile: true`, or an install/preinstall/postinstall script that runs a native tool. */ +export function npmNativeBuild(meta: NpmVersionMeta): { reason: string; prebuiltFallback: boolean } | null { + const installScript = INSTALL_HOOKS.map((hook) => meta.scripts?.[hook] ?? "").join(" "); + const isNative = meta.gypfile === true || NATIVE_TOOL_RE.test(installScript); + if (!isNative) return null; + const prebuiltFallback = Boolean(meta.binary) || PREBUILT_TOOL_RE.test(installScript); + const reason = prebuiltFallback + ? "ships a native addon with prebuilt binaries — compiles from source only when no prebuilt matches the platform/Node ABI" + : "compiles a native addon (node-gyp) on install — cold-CI build cost and a cross-platform breakage source"; + return { reason, prebuiltFallback }; +} + +/** A PyPI release file entry (from the `urls` array of the version JSON). */ +export interface PypiUrl { + packagetype?: string; +} + +/** Pure: is this PyPI version sdist-only (a source dist is published but no prebuilt wheel)? Requires an actual + * `sdist` so an empty or wheel-less-but-also-sdist-less file set is not mistaken for "compiles from source". */ +export function pypiSdistOnly(urls: PypiUrl[]): boolean { + const hasSdist = urls.some((url) => url.packagetype === "sdist"); + const hasWheel = urls.some((url) => url.packagetype === "bdist_wheel"); + return hasSdist && !hasWheel; +} + +async function fetchJson( + fetchImpl: typeof fetch, + url: string, + signal?: AbortSignal, +): Promise { + if (signal?.aborted) return null; + try { + const response = await fetchImpl(url, { signal }); + if (!response.ok) return null; + return await response.json(); + } catch { + return null; + } +} + +/** Analyzer entrypoint: added/changed deps → registry metadata → only the versions with a native-build install cost. */ +export async function scanNativeBuild( + req: EnrichRequest, + fetchImpl: typeof fetch = fetch, + options: ScanOptions = {}, +): Promise { + // Filter to queryable (supported, URL-safe) changes BEFORE applying the cap, so unsupported/invalid entries can't + // consume the budget and starve a later native dependency. + const changes = extractDependencyChanges(req.files ?? []) + .filter(isQueryable) + .slice(0, options.limits?.maxQueries ?? MAX_QUERIES); + const findings: NativeBuildFinding[] = []; + for (const change of changes) { + if (options.signal?.aborted) break; + + if (change.ecosystem === "npm") { + const data = (await fetchJson( + fetchImpl, + `https://registry.npmjs.org/${encodeURIComponent(change.package)}`, + options.signal, + )) as { versions?: Record } | null; + const meta = data?.versions?.[change.to]; + const native = meta && npmNativeBuild(meta); + if (native) { + findings.push({ + ecosystem: change.ecosystem, + package: change.package, + version: change.to, + kind: "native-addon", + prebuiltFallback: native.prebuiltFallback, + reason: native.reason, + }); + } + } else { + // PyPI — the only other ecosystem isQueryable admits. + const data = (await fetchJson( + fetchImpl, + `https://pypi.org/pypi/${encodeURIComponent(change.package)}/${encodeURIComponent(change.to)}/json`, + options.signal, + )) as { urls?: PypiUrl[] } | null; + if (data && pypiSdistOnly(data.urls ?? [])) { + findings.push({ + ecosystem: change.ecosystem, + package: change.package, + version: change.to, + kind: "sdist-only", + reason: "no prebuilt wheel for this version — pip compiles from source (sdist) on install", + }); + } + } + } + return findings; +} diff --git a/review-enrichment/src/brief.ts b/review-enrichment/src/brief.ts index 7550f7db89..7f762894cb 100644 --- a/review-enrichment/src/brief.ts +++ b/review-enrichment/src/brief.ts @@ -20,6 +20,7 @@ import { scanCodeowners } from "./analyzers/codeowners.js"; import { scanSecretLog } from "./analyzers/secret-log.js"; import { scanAssetWeight } from "./analyzers/asset-weight.js"; import { scanTyposquat } from "./analyzers/typosquat.js"; +import { scanNativeBuild } from "./analyzers/native-build.js"; import { renderBrief } from "./render.js"; import { captureAnalyzerDegradation } from "./sentry.js"; @@ -41,6 +42,7 @@ const ANALYZERS: Record = { secretLog: (req, signal) => scanSecretLog(req, signal), assetWeight: (req, signal) => scanAssetWeight(req, fetch, { signal }), typosquat: (req, signal) => scanTyposquat(req, fetch, { signal }), + nativeBuild: (req, signal) => scanNativeBuild(req, fetch, { signal }), }; function runWithTimeout( diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index e9fc157841..f11af38cb7 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -254,6 +254,18 @@ export function renderBrief( } } + const nativeBuilds = findings.nativeBuild ?? []; + if (nativeBuilds.length) { + lines.push( + "### Native-build / install-cost dependencies (CI cold-start + cross-platform build cost)", + ); + for (const item of nativeBuilds) { + lines.push( + `- ${safeCodeSpan(`${item.package}@${item.version}`)} (${item.ecosystem}): ${item.reason}`, + ); + } + } + if (!lines.length) return { promptSection: "", systemSuffix: "" }; const header = diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index 70dcf66619..c89954abfb 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -162,6 +162,21 @@ export interface TyposquatFinding { reason: string; } +/** A newly-added dependency whose install compiles native code (npm node-gyp addon) or has no prebuilt wheel + * (PyPI sdist-only) — a hidden CI cold-start/install cost and a frequent cross-platform breakage source. Reports + * package@version + the factual build property only. (#1512) */ +export interface NativeBuildFinding { + ecosystem: string; + package: string; + version: string; + kind: "native-addon" | "sdist-only"; + /** npm only: a prebuilt-binary path exists (node-pre-gyp/prebuild or a `binary` field), so a compile is the + * fallback when no prebuilt matches the platform/ABI rather than guaranteed. */ + prebuiltFallback?: boolean; + /** Short, public-safe explanation of the build cost. */ + reason: string; +} + /** Structured analyzer output. Each analyzer fills its own key; more land as analyzers ship (#1477/#1478). */ export interface BriefFindings { dependency?: DependencyFinding[]; @@ -177,6 +192,7 @@ export interface BriefFindings { secretLog?: SecretLogFinding[]; assetWeight?: AssetWeightFinding[]; typosquat?: TyposquatFinding[]; + nativeBuild?: NativeBuildFinding[]; } export type AnalyzerStatus = "ok" | "degraded" | "skipped"; diff --git a/review-enrichment/test/native-build.test.ts b/review-enrichment/test/native-build.test.ts new file mode 100644 index 0000000000..f3314424fe --- /dev/null +++ b/review-enrichment/test/native-build.test.ts @@ -0,0 +1,150 @@ +// Units for the native-build / install-cost analyzer (#1512). Own file (not enrichment.test.ts) so concurrent +// analyzer PRs don't collide. Runs against the compiled dist/. +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + npmNativeBuild, + pypiSdistOnly, + scanNativeBuild, +} from "../dist/analyzers/native-build.js"; +import { renderBrief } from "../dist/render.js"; + +const npmAdd = (name, version = "1.0.0") => ({ + repoFullName: "o/r", + prNumber: 1, + files: [{ path: "package.json", patch: `@@ -1,0 +1,1 @@\n+ "${name}": "^${version}"` }], +}); +const pypiAdd = (name, version = "1.0.0") => ({ + repoFullName: "o/r", + prNumber: 1, + files: [{ path: "requirements.txt", patch: `@@ -1,0 +1,1 @@\n+${name}==${version}` }], +}); +const npmFetch = (meta) => async () => ({ ok: true, json: async () => ({ versions: { "1.0.0": meta } }) }); +const pypiFetch = (urls) => async () => ({ ok: true, json: async () => ({ urls }) }); +const status = (code) => async () => ({ ok: code >= 200 && code < 300, status: code, json: async () => ({}) }); +const throwingFetch = async () => { + throw new Error("network down"); +}; + +test("npmNativeBuild: gypfile compiles without a prebuilt fallback", () => { + const hit = npmNativeBuild({ gypfile: true }); + assert.equal(hit?.prebuiltFallback, false); + assert.match(hit.reason, /compiles a native addon/); +}); + +test("npmNativeBuild: install script running node-gyp flags a compile", () => { + assert.ok(npmNativeBuild({ scripts: { install: "node-gyp rebuild" } })); + assert.ok(npmNativeBuild({ scripts: { postinstall: "cmake-js compile" } })); +}); + +test("npmNativeBuild: a prebuilt-binary path is reported as a fallback compile", () => { + const viaBinary = npmNativeBuild({ gypfile: true, binary: { module_name: "x" } }); + assert.equal(viaBinary?.prebuiltFallback, true); + assert.match(viaBinary.reason, /prebuilt/); + const viaScript = npmNativeBuild({ scripts: { install: "node-pre-gyp install --fallback-to-build" } }); + assert.equal(viaScript?.prebuiltFallback, true); +}); + +test("npmNativeBuild: a pure-JS package is not flagged", () => { + assert.equal(npmNativeBuild({ scripts: { build: "tsc", postinstall: "echo hi" } }), null); + assert.equal(npmNativeBuild({}), null); +}); + +test("pypiSdistOnly: true only when an sdist exists and no wheel does", () => { + assert.equal(pypiSdistOnly([{ packagetype: "sdist" }]), true); + assert.equal(pypiSdistOnly([{ packagetype: "sdist" }, { packagetype: "bdist_wheel" }]), false); + assert.equal(pypiSdistOnly([{ packagetype: "bdist_wheel" }]), false); // wheel present + assert.equal(pypiSdistOnly([{ packagetype: "bdist_egg" }]), false); // no sdist → not "sdist-only" + assert.equal(pypiSdistOnly([]), false); // undeterminable → no finding +}); + +test("scanNativeBuild: npm gypfile dependency is flagged native-addon", async () => { + const findings = await scanNativeBuild(npmAdd("bcrypt"), npmFetch({ gypfile: true })); + assert.equal(findings.length, 1); + assert.equal(findings[0].kind, "native-addon"); + assert.equal(findings[0].package, "bcrypt"); + assert.equal(findings[0].prebuiltFallback, false); +}); + +test("scanNativeBuild: a pure-JS npm dependency is not flagged", async () => { + assert.deepEqual(await scanNativeBuild(npmAdd("lodash"), npmFetch({ scripts: { build: "tsc" } })), []); +}); + +test("scanNativeBuild: PyPI sdist-only release is flagged", async () => { + const findings = await scanNativeBuild(pypiAdd("ujson"), pypiFetch([{ packagetype: "sdist" }])); + assert.equal(findings.length, 1); + assert.equal(findings[0].kind, "sdist-only"); + assert.match(findings[0].reason, /compiles from source/); +}); + +test("scanNativeBuild: a PyPI release with a wheel is not flagged", async () => { + assert.deepEqual( + await scanNativeBuild(pypiAdd("requests"), pypiFetch([{ packagetype: "bdist_wheel" }])), + [], + ); +}); + +test("scanNativeBuild: unsupported ecosystems and invalid names/versions are never queried", async () => { + const req = { + repoFullName: "o/r", + prNumber: 1, + files: [ + { path: "go.mod", patch: `@@ -1,0 +1,1 @@\n+require example.com/x v1.0.0` }, // Go — unsupported + { path: "package.json", patch: `@@ -1,0 +1,1 @@\n+ "BadCaps": "^1.0.0"` }, // invalid npm name + ], + }; + let called = false; + const out = await scanNativeBuild(req, async () => { + called = true; + return status(200)(); + }); + assert.deepEqual(out, []); + assert.equal(called, false); // nothing queryable → no registry call +}); + +test("scanNativeBuild: the query cap counts only queryable changes (skips don't starve a later native dep)", async () => { + // 25 unsupported Go changes precede one native npm dep; with filter-before-cap the npm dep is still queried. + const goLines = Array.from({ length: 25 }, (_, i) => `+require example.com/m${i} v1.0.0`).join("\n"); + const req = { + repoFullName: "o/r", + prNumber: 1, + files: [ + { path: "go.mod", patch: `@@ -1,0 +1,25 @@\n${goLines}` }, + { path: "package.json", patch: `@@ -1,0 +1,1 @@\n+ "bcrypt": "^1.0.0"` }, + ], + }; + const findings = await scanNativeBuild(req, npmFetch({ gypfile: true }), { limits: { maxQueries: 25 } }); + assert.equal(findings.length, 1); + assert.equal(findings[0].package, "bcrypt"); +}); + +test("scanNativeBuild: a PyPI PEP 440 (non-semver) sdist-only version is flagged", async () => { + const findings = await scanNativeBuild(pypiAdd("ujson", "24.1"), pypiFetch([{ packagetype: "sdist" }])); + assert.equal(findings.length, 1); + assert.equal(findings[0].kind, "sdist-only"); + assert.equal(findings[0].version, "24.1"); +}); + +test("scanNativeBuild fails safe on a non-ok or throwing fetch", async () => { + assert.deepEqual(await scanNativeBuild(npmAdd("bcrypt"), status(404)), []); + assert.deepEqual(await scanNativeBuild(npmAdd("bcrypt"), throwingFetch), []); +}); + +test("scanNativeBuild stops on an already-aborted signal", async () => { + const findings = await scanNativeBuild(npmAdd("bcrypt"), npmFetch({ gypfile: true }), { + signal: AbortSignal.abort(), + }); + assert.deepEqual(findings, []); +}); + +test("renderBrief emits a public-safe native-build block", () => { + const { promptSection } = renderBrief({ + nativeBuild: [ + { ecosystem: "npm", package: "bcrypt", version: "5.1.0", kind: "native-addon", prebuiltFallback: false, reason: "compiles a native addon (node-gyp) on install — cold-CI build cost and a cross-platform breakage source" }, + { ecosystem: "PyPI", package: "ujson", version: "5.0.0", kind: "sdist-only", reason: "no prebuilt wheel for this version — pip compiles from source (sdist) on install" }, + ], + }); + assert.match(promptSection, /Native-build \/ install-cost dependencies/); + assert.match(promptSection, /bcrypt@5\.1\.0/); + assert.match(promptSection, /ujson@5\.0\.0/); +});