From e3174e2e70fe4e5d3e0785460ed5ad4733ec1c4b Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Mon, 29 Jun 2026 22:29:00 -0700 Subject: [PATCH] refactor(rees): add modular analyzer manifests --- review-enrichment/README.md | 22 + .../src/analyzers/dependency/descriptor.ts | 49 +++ review-enrichment/src/analyzers/registry.ts | 380 ++++++++++++++++++ .../src/analyzers/secret/descriptor.ts | 34 ++ review-enrichment/src/analyzers/types.ts | 83 ++++ review-enrichment/src/brief.ts | 63 +-- review-enrichment/src/render-helpers.ts | 61 +++ review-enrichment/src/render.ts | 91 +---- .../test/analyzer-registry.test.ts | 106 +++++ 9 files changed, 758 insertions(+), 131 deletions(-) create mode 100644 review-enrichment/src/analyzers/dependency/descriptor.ts create mode 100644 review-enrichment/src/analyzers/registry.ts create mode 100644 review-enrichment/src/analyzers/secret/descriptor.ts create mode 100644 review-enrichment/src/analyzers/types.ts create mode 100644 review-enrichment/src/render-helpers.ts create mode 100644 review-enrichment/test/analyzer-registry.test.ts diff --git a/review-enrichment/README.md b/review-enrichment/README.md index dfaca40a74..4d1b417045 100644 --- a/review-enrichment/README.md +++ b/review-enrichment/README.md @@ -48,6 +48,28 @@ The engine can send `analyzers: ["secret", "actionPin"]` to run a subset. If the full registry. An explicit empty array runs no analyzers; the engine uses that fail-closed shape when an operator-configured analyzer list contains no valid names. +## Analyzer manifests + +Analyzer runtime metadata lives in `src/analyzers/registry.ts` as `AnalyzerDescriptor` entries. New analyzer work +should prefer the modular shape introduced for `dependency` and `secret`: + +| File | Purpose | +| ----------------------------------------- | -------------------------------------------------------------- | +| `src/analyzers//descriptor.ts` | Analyzer name, title, category, cost, requirements, docs, run function, and optional renderer. | +| `src/analyzers/.ts` | Pure scanner helpers and the analyzer implementation. | +| `test/.test.ts` | Focused tests for scanner behavior, rendering, and degradation. | + +Descriptors are the extension point future REES runtime work will use for profiles, docs generation, scheduler cost +classes, per-analyzer limits, and self-host configuration. When adding or migrating an analyzer: + +- Keep the public analyzer name stable; it is what `REES_ANALYZERS` and the engine request body use. +- Put operator-facing metadata in the descriptor: `category`, `cost`, `defaultEnabled`, `requires`, `limits`, and + `docs`. +- Keep renderer output public-safe. Never include tokens, request bodies, diffs, raw prompts, comments, or private + config values. +- Make external-call analyzers fail open and respect the orchestrator abort signal when the scanner supports it. +- Prefer a focused analyzer test file instead of expanding the shared `enrichment.test.ts` mega-test. + The engine also sends `budget.timeoutMs` with one second of headroom below `REES_TIMEOUT_MS`, so REES can return a partial/degraded brief before the caller aborts the HTTP request. If Railway is still running an older REES build, temporarily raise the engine-side `REES_TIMEOUT_MS` above the REES analyzer budget, or set `REES_ANALYZERS` to a diff --git a/review-enrichment/src/analyzers/dependency/descriptor.ts b/review-enrichment/src/analyzers/dependency/descriptor.ts new file mode 100644 index 0000000000..b42b7b526f --- /dev/null +++ b/review-enrichment/src/analyzers/dependency/descriptor.ts @@ -0,0 +1,49 @@ +import type { AnalyzerDescriptor } from "../types.js"; +import { scanDependencies } from "../dependency-scan.js"; +import { SEVERITY_RANK } from "../../render-helpers.js"; + +export const dependencyAnalyzer: AnalyzerDescriptor<"dependency"> = { + name: "dependency", + title: "Dependency vulnerabilities", + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + requires: ["files", "public-network"], + limits: { + maxManifestFiles: 20, + maxPatchLinesPerFile: 500, + maxDependencyQueries: 25, + }, + docs: { + summary: "Checks changed direct dependency versions against OSV.dev.", + looksAt: + "Added or upgraded dependencies in package.json, requirements.txt, and go.mod diffs.", + reports: + "Known CVEs with severity, advisory id, summary, and fixed version when OSV publishes one.", + network: "Calls OSV.dev. No GitHub token required.", + notes: + "Manifest-only by design; use lockfileDrift for transitive lockfile changes.", + }, + run: (req, { signal }) => scanDependencies(req, fetch, { signal }), + render: (deps, { safeCodeSpan, promptText }) => { + const lines: string[] = []; + if (!deps.length) return lines; + lines.push("### Dependency vulnerabilities (OSV.dev)"); + const flat = deps + .flatMap((dep) => dep.cves.map((cve) => ({ dep, cve }))) + .sort( + (a, b) => + (SEVERITY_RANK[a.cve.severity] ?? 4) - + (SEVERITY_RANK[b.cve.severity] ?? 4), + ); + for (const { dep, cve } of flat) { + const fix = cve.fixedIn + ? ` — fixed in ${safeCodeSpan(cve.fixedIn)}` + : ""; + lines.push( + `- ${safeCodeSpan(`${dep.package}@${dep.to}`)} (${dep.ecosystem}): **${cve.severity}** ${safeCodeSpan(cve.id)} — ${promptText(cve.summary)}${fix}`, + ); + } + return lines; + }, +}; diff --git a/review-enrichment/src/analyzers/registry.ts b/review-enrichment/src/analyzers/registry.ts new file mode 100644 index 0000000000..89369c84ec --- /dev/null +++ b/review-enrichment/src/analyzers/registry.ts @@ -0,0 +1,380 @@ +import { scanActionPins } from "./actions-pin.js"; +import { scanAssetWeight } from "./asset-weight.js"; +import { scanCodeowners } from "./codeowners.js"; +import { scanCommitSignature } from "./commit-signature.js"; +import { dependencyAnalyzer } from "./dependency/descriptor.js"; +import { scanEol } from "./eol-check.js"; +import { scanHeavyDependencies } from "./heavy-dependency.js"; +import { scanHistory } from "./history.js"; +import { scanIacMisconfig } from "./iac-misconfig.js"; +import { scanInstallScripts } from "./install-scripts.js"; +import { scanLicenses } from "./license-check.js"; +import { scanLockfileDrift } from "./lockfile-drift.js"; +import { scanNativeBuild } from "./native-build.js"; +import { scanProvenance } from "./provenance.js"; +import { scanRedos } from "./redos.js"; +import { secretAnalyzer } from "./secret/descriptor.js"; +import { scanSecretLog } from "./secret-log.js"; +import { scanTyposquat } from "./typosquat.js"; +import type { + AnalyzerDescriptor, + AnalyzerFn, + AnalyzerName, + AnalyzerRegistry, + AnyAnalyzerDescriptor, +} from "./types.js"; + +function descriptor( + definition: AnalyzerDescriptor, +): AnalyzerDescriptor { + return definition; +} + +export const ANALYZER_DESCRIPTORS = [ + dependencyAnalyzer, + descriptor({ + name: "lockfileDrift", + title: "Lockfile drift", + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + requires: ["files", "public-network"], + limits: { + maxLockfileFiles: 12, + maxPatchLinesPerFile: 1200, + maxOsvQueries: 40, + }, + docs: { + summary: + "Finds vulnerable transitive dependency versions introduced only through lockfile changes.", + looksAt: + "package-lock.json, yarn.lock, and poetry.lock patches, excluding packages already named in a changed manifest.", + reports: + "Lockfile line, package/version, ecosystem, direction, and OSV vulnerability details.", + network: "Calls OSV.dev querybatch. No GitHub token required.", + notes: + "Useful when a PR does not touch a top-level manifest but changes resolved dependency pins.", + }, + run: (req, { signal }) => scanLockfileDrift(req, fetch, { signal }), + }), + secretAnalyzer, + descriptor({ + name: "license", + title: "Dependency licenses", + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + requires: ["files", "public-network"], + limits: { maxLicenseLookups: 25 }, + docs: { + summary: "Checks licenses for newly added or upgraded dependencies.", + looksAt: "The same direct dependency changes used by the dependency analyzer.", + reports: + "Copyleft or unknown license classifications that need maintainer compatibility review.", + network: "Calls deps.dev. No GitHub token required.", + notes: "Permissive and otherwise-known licenses are intentionally silent.", + }, + run: (req) => scanLicenses(req), + }), + descriptor({ + name: "installScript", + title: "npm install scripts", + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + requires: ["files", "public-network"], + docs: { + summary: "Flags npm packages that run lifecycle hooks during install.", + looksAt: "New or upgraded npm dependencies.", + reports: "Package, version, hook names, and publish date when available.", + network: "Calls the npm registry. No GitHub token required.", + notes: + "The script body is not returned, which keeps the brief compact and non-executable.", + }, + run: (req) => scanInstallScripts(req), + }), + descriptor({ + name: "heavyDependency", + title: "Heavy dependencies used trivially", + category: "performance", + cost: "registry", + defaultEnabled: true, + requires: ["files", "public-network"], + limits: { maxWeightLookups: 20, maxFindings: 15 }, + docs: { + summary: + "Flags materially heavy npm dependencies used only a few times in changed lines.", + looksAt: "New or upgraded npm dependencies plus direct uses in added lines.", + reports: + "Package size, dependency count, usage count, and line-cited usage locations.", + network: "Calls Bundlephobia. No GitHub token required.", + notes: + "Only reports packages with trivial direct usage so the finding stays actionable.", + }, + run: (req, { signal }) => scanHeavyDependencies(req, fetch, { signal }), + }), + descriptor({ + name: "actionPin", + title: "Unpinned GitHub Actions", + category: "supply-chain", + cost: "local", + defaultEnabled: true, + requires: ["files"], + docs: { + summary: "Detects third-party workflow actions pinned to mutable tags or branches.", + looksAt: "Added uses: lines in .github/workflows YAML patches.", + reports: "Workflow file, line, action, and mutable ref.", + network: "Pure local analyzer. No external network call.", + notes: "Official actions/* and github/* actions are excluded to keep the signal focused.", + }, + run: (req) => scanActionPins(req), + }), + descriptor({ + name: "eol", + title: "End-of-life runtimes", + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + requires: ["files", "public-network"], + limits: { maxFiles: 40, maxPatchLines: 1000, maxPins: 80 }, + docs: { + summary: "Checks changed runtime and base-image pins against EOL calendars.", + looksAt: "Dockerfile FROM lines, .nvmrc, and go.mod runtime pins.", + reports: + "File, product, version, EOL date, and whether the release is already EOL or close to EOL.", + network: "Calls endoflife.date. No GitHub token required.", + notes: "Only changed pins are checked; existing old runtimes outside the PR are not reported.", + }, + run: (req) => scanEol(req), + }), + descriptor({ + name: "redos", + title: "ReDoS-prone regex", + category: "security", + cost: "local", + defaultEnabled: true, + requires: ["files"], + limits: { maxFindings: 25, maxPatternChars: 1000, maxLineChars: 2000 }, + docs: { + summary: "Finds newly introduced regex shapes that can catastrophically backtrack.", + looksAt: "Regex literals and RegExp constructor string arguments in added lines.", + reports: "File, line, and a truncated vulnerable pattern.", + network: "Pure local analyzer. No external network call.", + notes: + "Structural and precision-first; it flags nested unbounded quantifier shapes such as (a+)+.", + }, + run: (req) => scanRedos(req), + }), + descriptor({ + name: "provenance", + title: "Provenance and committed artifacts", + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + requires: ["files", "public-network"], + limits: { maxAttestationChecks: 20, maxFindings: 30 }, + docs: { + summary: "Checks package attestations and reviewability of newly added artifacts.", + looksAt: "New npm/PyPI dependency versions plus added binary, vendored, and minified files.", + reports: + "Missing attestations, binary files without reviewable source, and vendored or minified code.", + network: + "Calls npm and PyPI attestation/provenance endpoints for package checks. Path checks are local.", + notes: "Network failures fail safe; it flags only confident no-attestation responses.", + }, + run: (req, { signal }) => scanProvenance(req, fetch, { signal }), + }), + descriptor({ + name: "codeowners", + title: "CODEOWNERS coverage", + category: "ownership", + cost: "github-light", + defaultEnabled: true, + requires: ["files", "author", "github-token"], + limits: { + maxFilesReported: 20, + maxCodeownersBytes: 64 * 1024, + maxCodeownersRules: 1000, + }, + docs: { + summary: "Checks whether changed files cross ownership domains not owned by the PR author.", + looksAt: ".github/CODEOWNERS, CODEOWNERS, or docs/CODEOWNERS plus the changed file list.", + reports: + "Owned files where the PR author is not listed, plus ownership blast-radius context in the rendered brief.", + network: + "Calls the GitHub API. Requires author plus GitHub token forwarding for private repos.", + notes: + "Leave REES_FORWARD_GITHUB_TOKEN unset/false to disable token forwarding; this analyzer will then skip when it cannot read CODEOWNERS.", + }, + run: (req, { signal }) => scanCodeowners(req, fetch, { signal }), + }), + descriptor({ + name: "secretLog", + title: "Secrets or PII in logs", + category: "security", + cost: "local", + defaultEnabled: true, + requires: ["files"], + limits: { maxFindings: 25, maxLineChars: 2000 }, + docs: { + summary: "Flags added code that writes sensitive values to logs or stdout.", + looksAt: "Added lines that call console, logger, process.stdout, or process.stderr sinks.", + reports: "File, line, sink, and category: secret, pii, or request-object.", + network: "Pure local analyzer. No external network call.", + notes: + "String log messages are stripped before matching, so ordinary prose like password reset is not enough to trigger.", + }, + run: (req, { signal }) => scanSecretLog(req, signal), + }), + descriptor({ + name: "assetWeight", + title: "Heavy binary assets", + category: "performance", + cost: "github-heavy", + defaultEnabled: true, + requires: ["files", "github-token", "head-sha"], + limits: { maxFindings: 50 }, + docs: { + summary: + "Finds large binary assets added to a PR, and growth deltas when base size is available.", + looksAt: + "Changed binary assets such as images, fonts, archives, PDFs, videos, and compiled binaries.", + reports: "Path, size, delta, and whether the asset was added or grown.", + network: + "Calls the GitHub API. Requires headSha and GitHub token forwarding for private repos.", + notes: + "Added asset detection works from headSha. Growth comparison needs baseSha in the enrichment request.", + }, + run: (req, { signal }) => scanAssetWeight(req, fetch, { signal }), + }), + descriptor({ + name: "typosquat", + title: "Typosquat and dependency-confusion risk", + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + requires: ["files", "public-network"], + limits: { maxDeps: 50, maxConfusionQueries: 15 }, + docs: { + summary: + "Checks newly added dependency names for near-miss and publicly claimable package names.", + looksAt: "Newly added npm and PyPI dependency names.", + reports: + "Typosquat matches against popular packages, or unscoped names missing from the public registry.", + network: + "Uses bundled popular-package lists plus npm/PyPI registry lookups for dependency-confusion checks.", + notes: + "Scoped npm packages are treated as namespace-protected and are not flagged as typosquats.", + }, + run: (req, { signal }) => scanTyposquat(req, fetch, { signal }), + }), + descriptor({ + name: "commitSignature", + title: "Head commit signature", + category: "supply-chain", + cost: "github-light", + defaultEnabled: true, + requires: ["github-token", "head-sha"], + docs: { + summary: "Checks head commit signature and public author provenance.", + looksAt: "The head commit plus a bounded slice of recent repository commit history.", + reports: + "GitHub signature verification reason and public boolean provenance flags.", + network: + "Calls the GitHub API. Requires headSha and GitHub token forwarding for private repos.", + notes: + "Does not expose emails or private identity data; only public GitHub commit facts are surfaced.", + }, + run: (req, { signal }) => scanCommitSignature(req, fetch, { signal }), + }), + descriptor({ + name: "iacMisconfig", + title: "IaC / config misconfiguration", + category: "config", + cost: "local", + defaultEnabled: true, + requires: ["files"], + limits: { maxFindings: 25, maxLineChars: 2000 }, + docs: { + summary: "Flags risky IaC/config changes such as public buckets or insecure CORS.", + looksAt: "Added lines in Docker, Terraform, YAML, JSON, and similar config files.", + reports: "File, line, and public-safe rule kind.", + network: "Pure local analyzer. No external network call.", + notes: "Reports configuration shapes only; it does not inspect private runtime config.", + }, + run: (req, { signal }) => scanIacMisconfig(req, signal), + }), + descriptor({ + name: "nativeBuild", + title: "Native-build dependencies", + category: "performance", + cost: "registry", + defaultEnabled: true, + requires: ["files", "public-network"], + limits: { maxQueries: 25, maxRegistryJsonBytes: 2 * 1024 * 1024 }, + docs: { + summary: + "Flags newly-added dependencies that compile native code or ship sdist-only builds.", + looksAt: "New npm/PyPI dependency versions.", + reports: "Package, version, ecosystem, native-build kind, and public-safe reason.", + network: "Calls npm and PyPI registries. No GitHub token required.", + notes: + "Registry JSON is capped so large package metadata cannot monopolize REES memory.", + }, + run: (req, { signal }) => scanNativeBuild(req, fetch, { signal }), + }), + descriptor({ + name: "history", + title: "Author and change-area history", + category: "history", + cost: "github-heavy", + defaultEnabled: true, + requires: ["files", "github-token", "author"], + limits: { + maxFilesProbed: 5, + commitsPerFile: 10, + maxPrLookups: 12, + maxSimilarPrs: 8, + }, + docs: { + summary: "Shows public author track record, same-file PR history, and linked-issue alignment.", + looksAt: + "The PR author, changed file paths, linked issue text, added diff lines, and bounded GitHub history lookups.", + reports: + "Prior PR counts, similar past PRs, linked issue coverage, and partial/degraded status.", + network: + "Calls GitHub API with bounded fanout. Requires author plus GitHub token forwarding for private repos.", + notes: + "Returns partial findings when GitHub lookups are skipped, capped, or budget-exhausted.", + }, + run: (req, context) => + scanHistory(req, fetch, { + signal: context.signal, + deadlineMs: context.deadlineMs, + timeoutMs: context.timeoutMs, + diagnostics: context.diagnostics, + }), + }), +] as const satisfies readonly AnyAnalyzerDescriptor[]; + +export const ANALYZER_NAMES = ANALYZER_DESCRIPTORS.map( + (analyzer) => analyzer.name, +) as AnalyzerName[]; + +export const ANALYZERS = Object.fromEntries( + ANALYZER_DESCRIPTORS.map((analyzer) => [analyzer.name, analyzer.run]), +) as Record; + +export const ANALYZER_REGISTRY: AnalyzerRegistry = ANALYZERS; + +export const ANALYZER_DESCRIPTORS_BY_NAME = Object.fromEntries( + ANALYZER_DESCRIPTORS.map((analyzer) => [analyzer.name, analyzer]), +) as Partial>; + +export function getAnalyzerDescriptor( + name: Name, +): AnalyzerDescriptor | undefined { + return ANALYZER_DESCRIPTORS_BY_NAME[name] as + | AnalyzerDescriptor + | undefined; +} diff --git a/review-enrichment/src/analyzers/secret/descriptor.ts b/review-enrichment/src/analyzers/secret/descriptor.ts new file mode 100644 index 0000000000..0820becbf6 --- /dev/null +++ b/review-enrichment/src/analyzers/secret/descriptor.ts @@ -0,0 +1,34 @@ +import type { AnalyzerDescriptor } from "../types.js"; +import { scanSecrets } from "../secret-scan.js"; + +export const secretAnalyzer: AnalyzerDescriptor<"secret"> = { + name: "secret", + title: "Hardcoded secrets", + category: "security", + cost: "local", + defaultEnabled: true, + requires: ["files"], + docs: { + summary: "Scans added diff lines for credential-shaped values.", + looksAt: "Added lines in every changed file patch.", + reports: + "File, line, secret kind, and confidence. The matched value is never returned.", + network: "Pure local analyzer. No external network call.", + notes: + "High-confidence patterns are treated as rotate-and-remove candidates; generic assignments stay verify-first.", + }, + run: (req) => scanSecrets(req), + render: (secrets, { safeCodeSpan }) => { + const lines: string[] = []; + if (!secrets.length) return lines; + lines.push( + "### Potential leaked secrets (value-redacted — verify + rotate)", + ); + for (const secret of secrets) { + lines.push( + `- ${safeCodeSpan(`${secret.file}:${secret.line}`)} — ${secret.kind} (${secret.confidence} confidence)`, + ); + } + return lines; + }, +}; diff --git a/review-enrichment/src/analyzers/types.ts b/review-enrichment/src/analyzers/types.ts new file mode 100644 index 0000000000..0b7eda6855 --- /dev/null +++ b/review-enrichment/src/analyzers/types.ts @@ -0,0 +1,83 @@ +import type { + AnalyzerDiagnostics, + BriefFindings, + EnrichRequest, +} from "../types.js"; +import type { AnalyzerRenderHelpers } from "../render-helpers.js"; + +export type AnalyzerName = keyof BriefFindings; + +export type AnalyzerCategory = + | "security" + | "supply-chain" + | "ownership" + | "history" + | "quality" + | "performance" + | "config"; + +export type AnalyzerCostClass = + | "local" + | "registry" + | "github-light" + | "github-heavy" + | "tooling"; + +export type AnalyzerRequirement = + | "diff" + | "files" + | "public-network" + | "github-token" + | "head-sha" + | "base-sha" + | "author" + | "linked-issue"; + +export interface AnalyzerRunContext { + signal: AbortSignal; + timeoutMs: number; + startedAtMs: number; + deadlineMs: number; + diagnostics: AnalyzerDiagnostics; +} + +export type AnalyzerResult = + NonNullable; + +export type AnalyzerFn = ( + req: EnrichRequest, + context: AnalyzerRunContext, +) => Promise; + +export type AnalyzerRegistry = Partial>; + +export interface AnalyzerDocs { + summary: string; + looksAt: string; + reports: string; + network: string; + notes: string; +} + +export interface AnalyzerDescriptor { + name: Name; + title: string; + category: AnalyzerCategory; + cost: AnalyzerCostClass; + defaultEnabled: boolean; + requires: AnalyzerRequirement[]; + limits?: Record; + docs: AnalyzerDocs; + run: ( + req: EnrichRequest, + context: AnalyzerRunContext, + ) => Promise>; + render?: ( + result: AnalyzerResult, + helpers: AnalyzerRenderHelpers, + ) => string[]; +} + +export type AnyAnalyzerDescriptor = { + [Name in AnalyzerName]: AnalyzerDescriptor; +}[AnalyzerName]; diff --git a/review-enrichment/src/brief.ts b/review-enrichment/src/brief.ts index 5adb7fb1ea..5fff68c040 100644 --- a/review-enrichment/src/brief.ts +++ b/review-enrichment/src/brief.ts @@ -8,75 +8,22 @@ import type { AnalyzerStatus, AnalyzerDiagnostics, } from "./types.js"; -import { scanDependencies } from "./analyzers/dependency-scan.js"; -import { scanLockfileDrift } from "./analyzers/lockfile-drift.js"; -import { scanSecrets } from "./analyzers/secret-scan.js"; -import { scanLicenses } from "./analyzers/license-check.js"; -import { scanInstallScripts } from "./analyzers/install-scripts.js"; -import { scanHeavyDependencies } from "./analyzers/heavy-dependency.js"; -import { scanActionPins } from "./analyzers/actions-pin.js"; -import { scanEol } from "./analyzers/eol-check.js"; -import { scanRedos } from "./analyzers/redos.js"; -import { scanProvenance } from "./analyzers/provenance.js"; -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 { scanCommitSignature } from "./analyzers/commit-signature.js"; -import { scanIacMisconfig } from "./analyzers/iac-misconfig.js"; -import { scanNativeBuild } from "./analyzers/native-build.js"; -import { scanHistory } from "./analyzers/history.js"; +import type { + AnalyzerRegistry, + AnalyzerRunContext, +} from "./analyzers/types.js"; +import { ANALYZERS } from "./analyzers/registry.js"; import { renderBrief } from "./render.js"; import { captureAnalyzerDegradation } from "./sentry.js"; const DEFAULT_ANALYZER_TIMEOUT_MS = 8000; const MIN_ANALYZER_TIMEOUT_MS = 1; -interface AnalyzerRunContext { - signal: AbortSignal; - timeoutMs: number; - startedAtMs: number; - deadlineMs: number; - diagnostics: AnalyzerDiagnostics; -} - interface BuildBriefOptions { requestId?: string; traceId?: string; } -type AnalyzerFn = (req: EnrichRequest, context: AnalyzerRunContext) => Promise; -type AnalyzerRegistry = Partial>; - -// The analyzer registry. Each key is the exact name accepted by the engine's REES_ANALYZERS setting. -const ANALYZERS: Record = { - dependency: (req, { signal }) => scanDependencies(req, fetch, { signal }), - lockfileDrift: (req, { signal }) => scanLockfileDrift(req, fetch, { signal }), - secret: (req) => scanSecrets(req), - license: (req) => scanLicenses(req), - installScript: (req) => scanInstallScripts(req), - heavyDependency: (req, { signal }) => - scanHeavyDependencies(req, fetch, { signal }), - actionPin: (req) => scanActionPins(req), - eol: (req) => scanEol(req), - redos: (req) => scanRedos(req), - provenance: (req, { signal }) => scanProvenance(req, fetch, { signal }), - codeowners: (req, { signal }) => scanCodeowners(req, fetch, { signal }), - secretLog: (req, { signal }) => scanSecretLog(req, signal), - assetWeight: (req, { signal }) => scanAssetWeight(req, fetch, { signal }), - typosquat: (req, { signal }) => scanTyposquat(req, fetch, { signal }), - commitSignature: (req, { signal }) => scanCommitSignature(req, fetch, { signal }), - iacMisconfig: (req, { signal }) => scanIacMisconfig(req, signal), - nativeBuild: (req, { signal }) => scanNativeBuild(req, fetch, { signal }), - history: (req, context) => - scanHistory(req, fetch, { - signal: context.signal, - deadlineMs: context.deadlineMs, - timeoutMs: context.timeoutMs, - diagnostics: context.diagnostics, - }), -}; - function resolveAnalyzerTimeoutMs(value: number | undefined): number { const parsed = Number(value ?? DEFAULT_ANALYZER_TIMEOUT_MS); if (!Number.isFinite(parsed)) return DEFAULT_ANALYZER_TIMEOUT_MS; diff --git a/review-enrichment/src/render-helpers.ts b/review-enrichment/src/render-helpers.ts new file mode 100644 index 0000000000..e4b2f31ec9 --- /dev/null +++ b/review-enrichment/src/render-helpers.ts @@ -0,0 +1,61 @@ +// Shared helpers for analyzer-owned renderers. These keep prompt output public-safe and deterministic while allowing +// analyzer modules to own their own brief sections. + +const CODE_SPAN_UNSAFE = /[`\u0000-\u001f\u007f]/g; + +const CODE_SPAN_REPLACEMENTS: Record = { + "`": "\u02cb", + "\n": "\u2424", + "\r": "\u240d", + "\t": "\u2409", +}; + +export const SEVERITY_RANK: Record = { + critical: 0, + high: 1, + medium: 2, + low: 3, + unknown: 4, +}; + +export interface AnalyzerRenderHelpers { + safeCodeSpan(value: string): string; + promptText(value: string): string; + formatBytes(value: number): string; + bytesLabel(value: number | null): string; +} + +export function safeCodeSpan(value: string): string { + return `\`${value.replace( + CODE_SPAN_UNSAFE, + (char) => CODE_SPAN_REPLACEMENTS[char] ?? "\ufffd", + )}\``; +} + +export function promptText(value: string): string { + return value + .replace(/[\u0000-\u001f\u007f]/g, " ") + .replace(/\\/g, "\\\\") + .replace(/`/g, "\\`") + .replace(/([*_{}[\]()#+.!|-])/g, "\\$1"); +} + +export function formatBytes(n: number): string { + if (n >= 1048576) return `${(n / 1048576).toFixed(1)} MiB`; + if (n >= 1024) return `${(n / 1024).toFixed(0)} KiB`; + return `${n} B`; +} + +export function bytesLabel(value: number | null): string { + if (value === null) return "unknown"; + if (value >= 1_000_000) return `${(value / 1_000_000).toFixed(1)} MB`; + if (value >= 1_000) return `${Math.round(value / 1_000)} KB`; + return `${value} B`; +} + +export const RENDER_HELPERS: AnalyzerRenderHelpers = { + safeCodeSpan, + promptText, + formatBytes, + bytesLabel, +}; diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 88c4cbdf18..4fcbad3524 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -1,50 +1,23 @@ // Render structured findings into the public-safe prompt block the engine splices into the review. Kept separate // so each analyzer's rendering is one function and the brief stays deterministic + cap-bounded. import type { BriefFindings } from "./types.js"; +import { getAnalyzerDescriptor } from "./analyzers/registry.js"; +import type { AnalyzerName } from "./analyzers/types.js"; +import { + bytesLabel, + formatBytes, + promptText, + RENDER_HELPERS, + safeCodeSpan, + SEVERITY_RANK, +} from "./render-helpers.js"; -const CODE_SPAN_UNSAFE = /[`\u0000-\u001f\u007f]/g; - -const CODE_SPAN_REPLACEMENTS: Record = { - "`": "\u02cb", - "\n": "\u2424", - "\r": "\u240d", - "\t": "\u2409", -}; - -function safeCodeSpan(value: string): string { - return `\`${value.replace( - CODE_SPAN_UNSAFE, - (char) => CODE_SPAN_REPLACEMENTS[char] ?? "\ufffd", - )}\``; -} - -const SEVERITY_RANK: Record = { - critical: 0, - high: 1, - medium: 2, - low: 3, - unknown: 4, -}; - -function promptText(value: string): string { - return value - .replace(/[\u0000-\u001f\u007f]/g, " ") - .replace(/\\/g, "\\\\") - .replace(/`/g, "\\`") - .replace(/([*_{}[\]()#+.!|-])/g, "\\$1"); -} - -function formatBytes(n: number): string { - if (n >= 1048576) return `${(n / 1048576).toFixed(1)} MiB`; - if (n >= 1024) return `${(n / 1024).toFixed(0)} KiB`; - return `${n} B`; -} - -function bytesLabel(value: number | null): string { - if (value === null) return "unknown"; - if (value >= 1_000_000) return `${(value / 1_000_000).toFixed(1)} MB`; - if (value >= 1_000) return `${Math.round(value / 1_000)} KB`; - return `${value} B`; +function renderDescriptorSection(name: AnalyzerName, result: unknown): string[] { + if (!result) return []; + const renderer = getAnalyzerDescriptor(name)?.render as + | ((value: never, helpers: typeof RENDER_HELPERS) => string[]) + | undefined; + return renderer ? renderer(result as never, RENDER_HELPERS) : []; } /** Build the `promptSection` (verbatim splice) + a one-line `systemSuffix` from the findings. Empty when nothing found. */ @@ -54,25 +27,7 @@ export function renderBrief( ): { promptSection: string; systemSuffix: string } { const lines: string[] = []; - const deps = findings.dependency ?? []; - if (deps.length) { - lines.push("### Dependency vulnerabilities (OSV.dev)"); - const flat = deps - .flatMap((dep) => dep.cves.map((cve) => ({ dep, cve }))) - .sort( - (a, b) => - (SEVERITY_RANK[a.cve.severity] ?? 4) - - (SEVERITY_RANK[b.cve.severity] ?? 4), - ); - for (const { dep, cve } of flat) { - const fix = cve.fixedIn - ? ` — fixed in ${safeCodeSpan(cve.fixedIn)}` - : ""; - lines.push( - `- ${safeCodeSpan(`${dep.package}@${dep.to}`)} (${dep.ecosystem}): **${cve.severity}** ${safeCodeSpan(cve.id)} — ${promptText(cve.summary)}${fix}`, - ); - } - } + lines.push(...renderDescriptorSection("dependency", findings.dependency)); const lockfileDrift = findings.lockfileDrift ?? []; if (lockfileDrift.length) { @@ -95,17 +50,7 @@ export function renderBrief( } } - const secrets = findings.secret ?? []; - if (secrets.length) { - lines.push( - "### Potential leaked secrets (value-redacted — verify + rotate)", - ); - for (const secret of secrets) { - lines.push( - `- ${safeCodeSpan(`${secret.file}:${secret.line}`)} — ${secret.kind} (${secret.confidence} confidence)`, - ); - } - } + lines.push(...renderDescriptorSection("secret", findings.secret)); const licenses = findings.license ?? []; if (licenses.length) { diff --git a/review-enrichment/test/analyzer-registry.test.ts b/review-enrichment/test/analyzer-registry.test.ts new file mode 100644 index 0000000000..9041ad97db --- /dev/null +++ b/review-enrichment/test/analyzer-registry.test.ts @@ -0,0 +1,106 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { + ANALYZER_DESCRIPTORS, + ANALYZER_NAMES, + ANALYZERS, + getAnalyzerDescriptor, +} from "../dist/analyzers/registry.js"; +import { buildBrief } from "../dist/brief.js"; +import { renderBrief } from "../dist/render.js"; + +const EXPECTED_ANALYZERS = [ + "dependency", + "lockfileDrift", + "secret", + "license", + "installScript", + "heavyDependency", + "actionPin", + "eol", + "redos", + "provenance", + "codeowners", + "secretLog", + "assetWeight", + "typosquat", + "commitSignature", + "iacMisconfig", + "nativeBuild", + "history", +]; + +test("analyzer descriptors cover the runtime registry in stable order", () => { + assert.deepEqual(ANALYZER_NAMES, EXPECTED_ANALYZERS); + assert.equal(new Set(ANALYZER_NAMES).size, ANALYZER_NAMES.length); + + for (const descriptor of ANALYZER_DESCRIPTORS) { + assert.equal(getAnalyzerDescriptor(descriptor.name), descriptor); + assert.equal(typeof ANALYZERS[descriptor.name], "function"); + assert.equal(descriptor.defaultEnabled, true); + assert.ok(descriptor.title.length > 3); + assert.ok(descriptor.docs.summary.length > 10); + assert.ok(descriptor.docs.looksAt.length > 10); + assert.ok(descriptor.docs.reports.length > 10); + assert.ok(descriptor.docs.network.length > 10); + } +}); + +test("buildBrief uses the descriptor-derived default registry", async () => { + const syntheticGithubToken = ["ghp", "abcdefghijklmnopqrstuvwxyz1234567890"].join("_"); + const brief = await buildBrief({ + repoFullName: "JSONbored/gittensory", + prNumber: 1809, + analyzers: ["secret"], + files: [ + { + path: "src/config.ts", + patch: `@@ -1,0 +1,1 @@\n+const token = "${syntheticGithubToken}";`, + }, + ], + }); + + assert.equal(brief.partial, false); + assert.equal(brief.analyzerStatus.secret, "ok"); + assert.equal(brief.findings.secret?.[0]?.kind, "github_token"); + assert.match(brief.promptSection, /Potential leaked secrets/); + assert.equal(brief.analyzerStatus.dependency, "skipped"); +}); + +test("migrated analyzers own their prompt rendering through descriptors", () => { + assert.equal(typeof getAnalyzerDescriptor("dependency")?.render, "function"); + assert.equal(typeof getAnalyzerDescriptor("secret")?.render, "function"); + + const { promptSection } = renderBrief({ + dependency: [ + { + ecosystem: "npm", + package: "lodash", + from: "4.17.20", + to: "4.17.21", + direction: "change", + cves: [ + { + id: "GHSA-test", + severity: "high", + summary: "Prototype pollution in dependency", + fixedIn: "4.17.22", + }, + ], + }, + ], + secret: [ + { + file: "src/config.ts", + line: 7, + kind: "github_token", + confidence: "high", + }, + ], + }); + + assert.match(promptSection, /Dependency vulnerabilities/); + assert.match(promptSection, /Potential leaked secrets/); + assert.match(promptSection, /GHSA-test/); + assert.match(promptSection, /src\/config\.ts:7/); +});