diff --git a/.env.example b/.env.example index 843edbcd52..a2cc364cb7 100644 --- a/.env.example +++ b/.env.example @@ -54,15 +54,29 @@ GITTENSORY_REVIEW_ENRICHMENT=false # REES_URL=https://enrichment.example.internal # REES_SHARED_SECRET= # bearer secret configured on the REES service # REES_TIMEOUT_MS=8000 # optional; minimum 1000, default 8000 +# REES_PROFILE=balanced # optional; fast | balanced | deep. Unset uses balanced. # REES_FORWARD_GITHUB_TOKEN=false # optional; default false. Set true only when REES_URL is inside # # your trust boundary and token-aware analyzers need a GitHub # # token for CODEOWNERS/blob-size reads (installation token when # # available; otherwise GITHUB_PUBLIC_TOKEN). -# REES_ANALYZERS=all # all | comma-list of exact names: -# # dependency,lockfileDrift,secret,license,installScript, -# # actionPin,eol,redos,provenance,codeowners,secretLog, -# # assetWeight,typosquat +# REES_ANALYZERS=all # all | comma-list of exact names. # # Unknown names warn and are ignored; a typo-only list runs no analyzers. +# BEGIN GENERATED REES ANALYZERS +# Current analyzer names: +# dependency,lockfileDrift,secret,license,installScript,heavyDependency,actionPin,eol,redos +# provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature,iacMisconfig,nativeBuild +# history,docCommentDrift +# +# Profile defaults: +# fast: dependency,lockfileDrift,secret,license,installScript,heavyDependency,actionPin,eol +# redos,provenance,secretLog,typosquat,iacMisconfig,nativeBuild +# balanced (default): dependency,lockfileDrift,secret,license,installScript,heavyDependency +# actionPin,eol,redos,provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature +# iacMisconfig,nativeBuild,history,docCommentDrift +# deep: dependency,lockfileDrift,secret,license,installScript,heavyDependency,actionPin,eol +# redos,provenance,codeowners,secretLog,assetWeight,typosquat,commitSignature,iacMisconfig +# nativeBuild,history,docCommentDrift +# END GENERATED REES ANALYZERS # Submitter-reputation spend control (internal-only): downgrades new/burst/low-rep # submitters to a deterministic-only review. Never surfaced publicly. diff --git a/apps/gittensory-ui/src/lib/rees-analyzers.ts b/apps/gittensory-ui/src/lib/rees-analyzers.ts index aaf4fd4e9e..a73ff5eec9 100644 --- a/apps/gittensory-ui/src/lib/rees-analyzers.ts +++ b/apps/gittensory-ui/src/lib/rees-analyzers.ts @@ -1,153 +1,510 @@ +// Generated by review-enrichment/scripts/generate-analyzer-metadata.mjs. +// Do not edit by hand; update review-enrichment analyzer descriptors instead. + +export type ReesProfileName = "fast" | "balanced" | "deep"; + export type ReesAnalyzerDoc = { name: string; title: string; - summary: string; - looksAt: string; - reports: string; - network: string; - notes: string; + category: string; + cost: string; + defaultEnabled: boolean; + profiles: readonly ReesProfileName[]; + requires: readonly string[]; + limits: Readonly>; + docs: { + summary: string; + looksAt: string; + reports: string; + network: string; + notes: string; + }; +}; + +export type ReesProfileDoc = { + name: ReesProfileName; + default: boolean; + costClasses: readonly string[]; + concurrency: Readonly>; + timeoutMs: Readonly>; + responseReserveMs: number; }; -export const REES_ANALYZERS: ReesAnalyzerDoc[] = [ +export const REES_DEFAULT_PROFILE = "balanced" as const; + +export const REES_PROFILES = [ + { + name: "fast", + default: false, + costClasses: ["local", "registry"], + concurrency: { + local: 8, + registry: 2, + "github-light": 0, + "github-heavy": 0, + tooling: 0, + }, + timeoutMs: { + local: 400, + registry: 800, + "github-light": 0, + "github-heavy": 0, + tooling: 0, + }, + responseReserveMs: 500, + }, + { + name: "balanced", + default: true, + costClasses: ["local", "registry", "github-light", "github-heavy", "tooling"], + concurrency: { + local: 8, + registry: 3, + "github-light": 2, + "github-heavy": 1, + tooling: 1, + }, + timeoutMs: { + local: 750, + registry: 1400, + "github-light": 1400, + "github-heavy": 2200, + tooling: 1400, + }, + responseReserveMs: 750, + }, + { + name: "deep", + default: false, + costClasses: ["local", "registry", "github-light", "github-heavy", "tooling"], + concurrency: { + local: 8, + registry: 4, + "github-light": 2, + "github-heavy": 1, + tooling: 1, + }, + timeoutMs: { + local: 1000, + registry: 2500, + "github-light": 2500, + "github-heavy": 4000, + tooling: 2500, + }, + responseReserveMs: 1000, + }, +] as const satisfies readonly ReesProfileDoc[]; + +export const REES_ANALYZERS = [ { name: "dependency", title: "Dependency vulnerabilities", - 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.", + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + 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.", + }, }, { name: "lockfileDrift", title: "Lockfile drift", - 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.", + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + 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.", + }, }, { name: "secret", title: "Hardcoded secrets", - 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.", + category: "security", + cost: "local", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + requires: ["files"], + limits: {}, + 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.", + }, }, { name: "license", title: "Dependency licenses", - 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.", + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + 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.", + }, }, { name: "installScript", title: "npm install scripts", - 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.", + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + requires: ["files", "public-network"], + limits: {}, + 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.", + }, + }, + { + name: "heavyDependency", + title: "Heavy dependencies used trivially", + category: "performance", + cost: "registry", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + 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.", + }, }, { name: "actionPin", title: "Unpinned GitHub Actions", - 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.", + category: "supply-chain", + cost: "local", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + requires: ["files"], + limits: {}, + 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.", + }, }, { name: "eol", title: "End-of-life runtimes", - 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.", + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + 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.", + }, }, { name: "redos", title: "ReDoS-prone regex", - 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+)+.", + category: "security", + cost: "local", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + 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+)+.", + }, }, { name: "provenance", title: "Provenance and committed artifacts", - 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.", + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + 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.", + }, }, { name: "codeowners", title: "CODEOWNERS coverage", - 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.", + category: "ownership", + cost: "github-light", + defaultEnabled: true, + profiles: ["balanced", "deep"], + requires: ["files", "author", "github-token"], + limits: { + maxFilesReported: 20, + maxCodeownersBytes: 65536, + 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.", + }, }, { name: "secretLog", title: "Secrets or PII in logs", - 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.", + category: "security", + cost: "local", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + 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.", + }, }, { name: "assetWeight", title: "Heavy binary assets", - 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.", + category: "performance", + cost: "github-heavy", + defaultEnabled: true, + profiles: ["balanced", "deep"], + 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.", + }, }, { name: "typosquat", title: "Typosquat and dependency-confusion risk", - 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.", - }, -]; + category: "supply-chain", + cost: "registry", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + 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.", + }, + }, + { + name: "commitSignature", + title: "Head commit signature", + category: "supply-chain", + cost: "github-light", + defaultEnabled: true, + profiles: ["balanced", "deep"], + requires: ["github-token", "head-sha"], + limits: {}, + 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.", + }, + }, + { + name: "iacMisconfig", + title: "IaC / config misconfiguration", + category: "config", + cost: "local", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + 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.", + }, + }, + { + name: "nativeBuild", + title: "Native-build dependencies", + category: "performance", + cost: "registry", + defaultEnabled: true, + profiles: ["fast", "balanced", "deep"], + requires: ["files", "public-network"], + limits: { + maxQueries: 25, + maxRegistryJsonBytes: 2097152, + }, + 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.", + }, + }, + { + name: "history", + title: "Author and change-area history", + category: "history", + cost: "github-heavy", + defaultEnabled: true, + profiles: ["balanced", "deep"], + 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.", + }, + }, + { + name: "docCommentDrift", + title: "Doc-comment drift", + category: "quality", + cost: "github-light", + defaultEnabled: true, + profiles: ["balanced", "deep"], + requires: ["files", "github-token", "head-sha"], + limits: { + maxFiles: 20, + maxFindings: 50, + }, + docs: { + summary: + "Flags a JSDoc/TSDoc @param that names a parameter the PR removed or renamed but left documented.", + looksAt: + "Changed TS/JS source files at headSha, comparing each named function's old vs new parameter list.", + reports: "File, line, function, and the stale parameter name(s).", + network: + "Calls the GitHub API for changed file contents. Requires headSha and token forwarding for private repos.", + notes: + "Conservative: only named function declarations with confidently-enumerable params; non-parameter signature edits are not reported.", + }, + }, +] as const satisfies readonly ReesAnalyzerDoc[]; export const REES_ANALYZER_NAMES = REES_ANALYZERS.map((analyzer) => analyzer.name); diff --git a/apps/gittensory-ui/src/routes/docs.self-hosting-rees-analyzers.tsx b/apps/gittensory-ui/src/routes/docs.self-hosting-rees-analyzers.tsx index d5c384a03b..f25def2ea9 100644 --- a/apps/gittensory-ui/src/routes/docs.self-hosting-rees-analyzers.tsx +++ b/apps/gittensory-ui/src/routes/docs.self-hosting-rees-analyzers.tsx @@ -2,7 +2,7 @@ import { createFileRoute, Link } from "@tanstack/react-router"; import { DocsPage } from "@/components/site/docs-page"; import { Callout, CodeBlock, FeatureRow } from "@/components/site/primitives"; -import { REES_ANALYZERS, REES_ANALYZER_NAMES } from "@/lib/rees-analyzers"; +import { REES_ANALYZERS, REES_ANALYZER_NAMES, REES_PROFILES } from "@/lib/rees-analyzers"; export const Route = createFileRoute("/docs/self-hosting-rees-analyzers")({ head: () => ({ @@ -37,7 +37,8 @@ function SelfHostingReesAnalyzers() { REES runs analyzers independently. A failed analyzer is marked degraded, completed analyzers still return findings, and an empty result produces no user-facing brief. Use exact analyzer names in REES_ANALYZERS. A typo-only analyzer list fails closed with no - analyzers selected. + analyzers selected. Leave REES_PROFILE unset for the balanced profile, or set + fast during incidents to favor local and low-cost registry checks.

+

Profiles

+
+ {REES_PROFILES.map((profile) => ( +
+
+

{profile.name}

+ {profile.default ? ( + + default + + ) : null} +
+
+
+
Cost classes
+
{profile.costClasses.join(", ")}
+
+
+
Concurrency caps
+
+ {Object.entries(profile.concurrency) + .filter(([, value]) => value > 0) + .map(([key, value]) => `${key}:${value}`) + .join(", ")} +
+
+
+
Response reserve
+
{profile.responseReserveMs} ms
+
+
+
+ ))} +
+

All analyzer names

@@ -61,17 +97,17 @@ REES_ANALYZERS=unknownName`} { title: "Pure analyzers", description: - "secret, actionPin, redos, and secretLog work only from the diff/files sent to REES.", + "secret, actionPin, redos, secretLog, and iacMisconfig work only from the diff/files sent to REES.", }, { title: "Public registry analyzers", description: - "dependency, lockfileDrift, license, installScript, eol, provenance, and typosquat call public package or lifecycle APIs.", + "dependency, lockfileDrift, license, installScript, heavyDependency, eol, provenance, typosquat, and nativeBuild call public package or lifecycle APIs.", }, { title: "GitHub API analyzers", description: - "codeowners and assetWeight need author/head metadata and GitHub token forwarding when the repo is private.", + "codeowners, assetWeight, commitSignature, and history need author/head metadata and GitHub token forwarding when the repo is private.", }, ]} /> @@ -89,29 +125,42 @@ REES_ANALYZERS=unknownName`}

{analyzer.title}

- {analyzer.summary} + {analyzer.docs.summary}

- - {analyzer.name} - +
+ + {analyzer.name} + + + {analyzer.cost} + +
Looks at
-
{analyzer.looksAt}
+
{analyzer.docs.looksAt}
Reports
-
{analyzer.reports}
+
{analyzer.docs.reports}
Network
-
{analyzer.network}
+
{analyzer.docs.network}
Operational note
-
{analyzer.notes}
+
{analyzer.docs.notes}
+
+
+
Profiles
+
{analyzer.profiles.join(", ")}
+
+
+
Requirements
+
{analyzer.requires.join(", ")}
diff --git a/apps/gittensory-ui/src/routes/docs.self-hosting-rees.tsx b/apps/gittensory-ui/src/routes/docs.self-hosting-rees.tsx index b160cd29a6..b75ec86e38 100644 --- a/apps/gittensory-ui/src/routes/docs.self-hosting-rees.tsx +++ b/apps/gittensory-ui/src/routes/docs.self-hosting-rees.tsx @@ -79,6 +79,7 @@ GITTENSORY_REVIEW_ENRICHMENT=true REES_URL=https://enrichment.example.internal REES_SHARED_SECRET= REES_TIMEOUT_MS=8000 +REES_PROFILE=balanced REES_FORWARD_GITHUB_TOKEN=false REES_ANALYZERS=all`} /> @@ -101,6 +102,11 @@ REES_ANALYZERS=all`} title: "REES_TIMEOUT_MS", description: "Request timeout. Defaults to 8000 ms and is clamped to at least 1000 ms.", }, + { + title: "REES_PROFILE", + description: + "Optional analyzer profile. balanced is the default; fast favors local/registry checks during incidents; deep allows larger per-class budgets.", + }, { title: "REES_FORWARD_GITHUB_TOKEN", description: @@ -127,13 +133,18 @@ REES_FORWARD_GITHUB_TOKEN=true`}

Analyzer selection

- Leave REES_ANALYZERS unset, all, or * to run the full - REES registry. To run a subset, use exact comma-separated analyzer names. Unknown names are - ignored with a rees_analyzer_config_invalid warning and the remaining valid - analyzers still run. If every configured name is invalid, the engine sends an empty analyzer - list so the typo fails closed instead of running the full registry. + Leave REES_ANALYZERS unset, all, or * to use the + selected REES_PROFILE defaults. To run a subset, use exact comma-separated + analyzer names. Unknown names are ignored with a rees_analyzer_config_invalid{" "} + warning and the remaining valid analyzers still run. If every configured name is invalid, + the engine sends an empty analyzer list so the typo fails closed instead of running the full + registry.

- +

See the REES analyzer reference for each diff --git a/package.json b/package.json index c4166829c3..dc4f8a0401 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,8 @@ "test:mcp-pack": "node scripts/check-mcp-package.mjs", "rees:install": "npm ci --prefix review-enrichment --prefer-offline --no-audit --no-fund", "rees:test": "npm run rees:install && npm --prefix review-enrichment test", + "rees:metadata": "npm --prefix review-enrichment run metadata", + "rees:metadata:check": "npm --prefix review-enrichment run metadata:check", "rees:validate-sourcemaps": "npm --prefix review-enrichment run validate:sourcemaps", "db:migrations:check": "node scripts/check-migrations.mjs", "actionlint": "node scripts/actionlint.mjs", diff --git a/review-enrichment/analyzer-metadata.json b/review-enrichment/analyzer-metadata.json new file mode 100644 index 0000000000..9ad7e58ad8 --- /dev/null +++ b/review-enrichment/analyzer-metadata.json @@ -0,0 +1,584 @@ +{ + "schemaVersion": 1, + "generatedFrom": "review-enrichment/src/analyzers/registry.ts", + "defaultProfile": "balanced", + "profiles": [ + { + "name": "fast", + "default": false, + "costClasses": [ + "local", + "registry" + ], + "concurrency": { + "local": 8, + "registry": 2, + "github-light": 0, + "github-heavy": 0, + "tooling": 0 + }, + "timeoutMs": { + "local": 400, + "registry": 800, + "github-light": 0, + "github-heavy": 0, + "tooling": 0 + }, + "responseReserveMs": 500 + }, + { + "name": "balanced", + "default": true, + "costClasses": [ + "local", + "registry", + "github-light", + "github-heavy", + "tooling" + ], + "concurrency": { + "local": 8, + "registry": 3, + "github-light": 2, + "github-heavy": 1, + "tooling": 1 + }, + "timeoutMs": { + "local": 750, + "registry": 1400, + "github-light": 1400, + "github-heavy": 2200, + "tooling": 1400 + }, + "responseReserveMs": 750 + }, + { + "name": "deep", + "default": false, + "costClasses": [ + "local", + "registry", + "github-light", + "github-heavy", + "tooling" + ], + "concurrency": { + "local": 8, + "registry": 4, + "github-light": 2, + "github-heavy": 1, + "tooling": 1 + }, + "timeoutMs": { + "local": 1000, + "registry": 2500, + "github-light": 2500, + "github-heavy": 4000, + "tooling": 2500 + }, + "responseReserveMs": 1000 + } + ], + "analyzers": [ + { + "name": "dependency", + "title": "Dependency vulnerabilities", + "category": "supply-chain", + "cost": "registry", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "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." + } + }, + { + "name": "lockfileDrift", + "title": "Lockfile drift", + "category": "supply-chain", + "cost": "registry", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "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." + } + }, + { + "name": "secret", + "title": "Hardcoded secrets", + "category": "security", + "cost": "local", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "requires": [ + "files" + ], + "limits": {}, + "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." + } + }, + { + "name": "license", + "title": "Dependency licenses", + "category": "supply-chain", + "cost": "registry", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "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." + } + }, + { + "name": "installScript", + "title": "npm install scripts", + "category": "supply-chain", + "cost": "registry", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "requires": [ + "files", + "public-network" + ], + "limits": {}, + "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." + } + }, + { + "name": "heavyDependency", + "title": "Heavy dependencies used trivially", + "category": "performance", + "cost": "registry", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "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." + } + }, + { + "name": "actionPin", + "title": "Unpinned GitHub Actions", + "category": "supply-chain", + "cost": "local", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "requires": [ + "files" + ], + "limits": {}, + "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." + } + }, + { + "name": "eol", + "title": "End-of-life runtimes", + "category": "supply-chain", + "cost": "registry", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "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." + } + }, + { + "name": "redos", + "title": "ReDoS-prone regex", + "category": "security", + "cost": "local", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "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+)+." + } + }, + { + "name": "provenance", + "title": "Provenance and committed artifacts", + "category": "supply-chain", + "cost": "registry", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "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." + } + }, + { + "name": "codeowners", + "title": "CODEOWNERS coverage", + "category": "ownership", + "cost": "github-light", + "defaultEnabled": true, + "profiles": [ + "balanced", + "deep" + ], + "requires": [ + "files", + "author", + "github-token" + ], + "limits": { + "maxFilesReported": 20, + "maxCodeownersBytes": 65536, + "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." + } + }, + { + "name": "secretLog", + "title": "Secrets or PII in logs", + "category": "security", + "cost": "local", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "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." + } + }, + { + "name": "assetWeight", + "title": "Heavy binary assets", + "category": "performance", + "cost": "github-heavy", + "defaultEnabled": true, + "profiles": [ + "balanced", + "deep" + ], + "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." + } + }, + { + "name": "typosquat", + "title": "Typosquat and dependency-confusion risk", + "category": "supply-chain", + "cost": "registry", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "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." + } + }, + { + "name": "commitSignature", + "title": "Head commit signature", + "category": "supply-chain", + "cost": "github-light", + "defaultEnabled": true, + "profiles": [ + "balanced", + "deep" + ], + "requires": [ + "github-token", + "head-sha" + ], + "limits": {}, + "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." + } + }, + { + "name": "iacMisconfig", + "title": "IaC / config misconfiguration", + "category": "config", + "cost": "local", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "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." + } + }, + { + "name": "nativeBuild", + "title": "Native-build dependencies", + "category": "performance", + "cost": "registry", + "defaultEnabled": true, + "profiles": [ + "fast", + "balanced", + "deep" + ], + "requires": [ + "files", + "public-network" + ], + "limits": { + "maxQueries": 25, + "maxRegistryJsonBytes": 2097152 + }, + "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." + } + }, + { + "name": "history", + "title": "Author and change-area history", + "category": "history", + "cost": "github-heavy", + "defaultEnabled": true, + "profiles": [ + "balanced", + "deep" + ], + "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." + } + }, + { + "name": "docCommentDrift", + "title": "Doc-comment drift", + "category": "quality", + "cost": "github-light", + "defaultEnabled": true, + "profiles": [ + "balanced", + "deep" + ], + "requires": [ + "files", + "github-token", + "head-sha" + ], + "limits": { + "maxFiles": 20, + "maxFindings": 50 + }, + "docs": { + "summary": "Flags a JSDoc/TSDoc @param that names a parameter the PR removed or renamed but left documented.", + "looksAt": "Changed TS/JS source files at headSha, comparing each named function's old vs new parameter list.", + "reports": "File, line, function, and the stale parameter name(s).", + "network": "Calls the GitHub API for changed file contents. Requires headSha and token forwarding for private repos.", + "notes": "Conservative: only named function declarations with confidently-enumerable params; non-parameter signature edits are not reported." + } + } + ] +} diff --git a/review-enrichment/package-lock.json b/review-enrichment/package-lock.json index 5d4b500539..11a02820da 100644 --- a/review-enrichment/package-lock.json +++ b/review-enrichment/package-lock.json @@ -15,6 +15,7 @@ }, "devDependencies": { "@types/node": "^22.10.2", + "prettier": "3.8.4", "typescript": "^5.7.2" }, "engines": { @@ -609,6 +610,22 @@ "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==", "license": "MIT" }, + "node_modules/prettier": { + "version": "3.8.4", + "resolved": "https://registry.npmjs.org/prettier/-/prettier-3.8.4.tgz", + "integrity": "sha512-N2MylSdi48+5N/6S5j+maeHbUSIzzZ5uOcX5Hm4QpV8Dkb1HFjfAKTKX6yNPJQD9AhcT3ifHNB66tWTTJDi11Q==", + "dev": true, + "license": "MIT", + "bin": { + "prettier": "bin/prettier.cjs" + }, + "engines": { + "node": ">=14" + }, + "funding": { + "url": "https://github.com/prettier/prettier?sponsor=1" + } + }, "node_modules/progress": { "version": "2.0.3", "resolved": "https://registry.npmjs.org/progress/-/progress-2.0.3.tgz", diff --git a/review-enrichment/package.json b/review-enrichment/package.json index 0b919e0d61..404ebd3477 100644 --- a/review-enrichment/package.json +++ b/review-enrichment/package.json @@ -9,10 +9,12 @@ }, "scripts": { "build": "tsc -p tsconfig.json", + "metadata": "npm run build && node scripts/generate-analyzer-metadata.mjs", + "metadata:check": "npm run build && node scripts/generate-analyzer-metadata.mjs --check", "validate:sourcemaps": "node scripts/validate-sourcemaps.mjs", "start": "node dist/server.js", "dev": "node --experimental-strip-types --watch src/server.ts", - "test": "npm run build && npm run validate:sourcemaps && node --test --experimental-strip-types \"test/**/*.test.ts\"" + "test": "npm run build && npm run validate:sourcemaps && node scripts/generate-analyzer-metadata.mjs --check && node --test --experimental-strip-types \"test/**/*.test.ts\"" }, "dependencies": { "@hono/node-server": "^1.13.7", @@ -22,6 +24,7 @@ }, "devDependencies": { "@types/node": "^22.10.2", + "prettier": "3.8.4", "typescript": "^5.7.2" } } diff --git a/review-enrichment/scripts/generate-analyzer-metadata.mjs b/review-enrichment/scripts/generate-analyzer-metadata.mjs new file mode 100644 index 0000000000..6bc05c3715 --- /dev/null +++ b/review-enrichment/scripts/generate-analyzer-metadata.mjs @@ -0,0 +1,157 @@ +import { readFile, writeFile } from "node:fs/promises"; +import { dirname, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +import { format, resolveConfig } from "prettier"; + +import { ANALYZER_DESCRIPTORS } from "../dist/analyzers/registry.js"; +import { reesProfileMetadata } from "../dist/scheduler.js"; + +const CHECK = process.argv.includes("--check"); +const scriptDir = dirname(fileURLToPath(import.meta.url)); +const reviewRoot = resolve(scriptDir, ".."); +const repoRoot = resolve(reviewRoot, ".."); +const jsonPath = resolve(reviewRoot, "analyzer-metadata.json"); +const uiPath = resolve(repoRoot, "apps/gittensory-ui/src/lib/rees-analyzers.ts"); +const envPath = resolve(repoRoot, ".env.example"); +const envStart = "# BEGIN GENERATED REES ANALYZERS"; +const envEnd = "# END GENERATED REES ANALYZERS"; + +const profiles = reesProfileMetadata(); +const defaultProfile = profiles.find((profile) => profile.default)?.name ?? "balanced"; + +const analyzers = ANALYZER_DESCRIPTORS.map((descriptor) => ({ + name: descriptor.name, + title: descriptor.title, + category: descriptor.category, + cost: descriptor.cost, + defaultEnabled: descriptor.defaultEnabled, + profiles: profiles + .filter( + (profile) => + descriptor.defaultEnabled && + profile.costClasses.includes(descriptor.cost), + ) + .map((profile) => profile.name), + requires: [...descriptor.requires], + limits: descriptor.limits ? { ...descriptor.limits } : {}, + docs: { ...descriptor.docs }, +})); + +const metadata = { + schemaVersion: 1, + generatedFrom: "review-enrichment/src/analyzers/registry.ts", + defaultProfile, + profiles, + analyzers, +}; + +const generatedJson = `${JSON.stringify(metadata, null, 2)}\n`; +const rawGeneratedUi = `// Generated by review-enrichment/scripts/generate-analyzer-metadata.mjs. +// Do not edit by hand; update review-enrichment analyzer descriptors instead. + +export type ReesProfileName = "fast" | "balanced" | "deep"; + +export type ReesAnalyzerDoc = { + name: string; + title: string; + category: string; + cost: string; + defaultEnabled: boolean; + profiles: readonly ReesProfileName[]; + requires: readonly string[]; + limits: Readonly>; + docs: { + summary: string; + looksAt: string; + reports: string; + network: string; + notes: string; + }; +}; + +export type ReesProfileDoc = { + name: ReesProfileName; + default: boolean; + costClasses: readonly string[]; + concurrency: Readonly>; + timeoutMs: Readonly>; + responseReserveMs: number; +}; + +export const REES_DEFAULT_PROFILE = ${JSON.stringify(defaultProfile)} as const; + +export const REES_PROFILES = ${JSON.stringify(profiles, null, 2)} as const satisfies readonly ReesProfileDoc[]; + +export const REES_ANALYZERS = ${JSON.stringify(analyzers, null, 2)} as const satisfies readonly ReesAnalyzerDoc[]; + +export const REES_ANALYZER_NAMES = REES_ANALYZERS.map((analyzer) => analyzer.name); +`; +const uiPrettierOptions = (await resolveConfig(uiPath)) ?? {}; +const generatedUi = await format(rawGeneratedUi, { + ...uiPrettierOptions, + filepath: uiPath, + parser: "typescript", +}); + +const envBlock = [ + envStart, + "# Current analyzer names:", + ...wrapComment(analyzers.map((analyzer) => analyzer.name).join(",")), + "#", + "# Profile defaults:", + ...profiles.flatMap((profile) => + wrapComment( + `${profile.name}${profile.default ? " (default)" : ""}: ${analyzers + .filter((analyzer) => analyzer.profiles.includes(profile.name)) + .map((analyzer) => analyzer.name) + .join(",")}`, + ), + ), + envEnd, +].join("\n"); + +const currentEnv = await readFile(envPath, "utf8"); +const generatedEnv = replaceGeneratedEnvBlock(currentEnv, envBlock); + +await writeOrCheck(jsonPath, generatedJson); +await writeOrCheck(uiPath, generatedUi); +await writeOrCheck(envPath, generatedEnv); + +function wrapComment(value, max = 96) { + const words = value.split(","); + const lines = []; + let current = "# "; + for (const word of words) { + const next = current === "# " ? `${current}${word}` : `${current},${word}`; + if (next.length > max && current !== "# ") { + lines.push(current); + current = `# ${word}`; + } else { + current = next; + } + } + if (current !== "# ") lines.push(current); + return lines; +} + +function replaceGeneratedEnvBlock(content, block) { + const start = content.indexOf(envStart); + const end = content.indexOf(envEnd); + if (start === -1 || end === -1 || end < start) { + throw new Error(".env.example is missing generated REES analyzer markers"); + } + const afterEnd = end + envEnd.length; + return `${content.slice(0, start)}${block}${content.slice(afterEnd)}`; +} + +async function writeOrCheck(path, expected) { + if (!CHECK) { + await writeFile(path, expected); + return; + } + const actual = await readFile(path, "utf8").catch(() => ""); + if (actual !== expected) { + throw new Error(`${path.replace(`${repoRoot}/`, "")} is stale; run npm --prefix review-enrichment run metadata`); + } +} diff --git a/review-enrichment/src/brief.ts b/review-enrichment/src/brief.ts index fd936fbf8d..0982d4f52d 100644 --- a/review-enrichment/src/brief.ts +++ b/review-enrichment/src/brief.ts @@ -7,6 +7,7 @@ import type { BriefFindings, AnalyzerStatus, AnalyzerDiagnostics, + AnalyzerTelemetry, } from "./types.js"; import type { AnalyzerRegistry, @@ -31,6 +32,7 @@ import { captureAnalyzerDegradation } from "./sentry.js"; const DEFAULT_ANALYZER_TIMEOUT_MS = 8000; const MIN_ANALYZER_TIMEOUT_MS = 1; +const PUBLIC_PARTIAL_REASON_RE = /^[A-Za-z0-9_.:-]{1,120}$/; interface BuildBriefOptions { requestId?: string; @@ -135,6 +137,11 @@ function timeoutStatus(error: unknown, diagnostics: AnalyzerDiagnostics): Analyz return statusFromDiagnostics(diagnostics, "degraded"); } +function publicPartialReason(value: string | undefined, fallback: string): string { + if (value && PUBLIC_PARTIAL_REASON_RE.test(value)) return value; + return fallback; +} + function captureDegradation( error: unknown, input: { @@ -144,6 +151,9 @@ function captureDegradation( timeoutMs: number; elapsedMs: number; analyzerStatus: AnalyzerStatus; + profile: string; + costClass?: string; + responseReserveMs?: number; diagnostics: AnalyzerDiagnostics; options: BuildBriefOptions; }, @@ -157,6 +167,9 @@ function captureDegradation( timeoutMs: input.timeoutMs, elapsedMs: input.elapsedMs, analyzerStatus: input.analyzerStatus, + profile: input.profile, + costClass: input.costClass, + responseReserveMs: input.responseReserveMs, partialStatus: input.diagnostics.partialStatus, partialReason: input.diagnostics.partialReason, phase: input.diagnostics.phase, @@ -213,9 +226,18 @@ export async function buildBrief( const findings: BriefFindings = {}; const analyzerStatus: Record = {}; + const analyzerTelemetry: Record = {}; let partial = false; - for (const item of plan.skipped) analyzerStatus[item.name] = "skipped"; + for (const item of plan.skipped) { + analyzerStatus[item.name] = "skipped"; + analyzerTelemetry[item.name] = { + status: "skipped", + elapsedMs: 0, + costClass: item.descriptor.cost, + skipReason: item.skipReason, + }; + } async function runAnalyzer(item: AnalyzerPlanItem): Promise { const name = item.name; @@ -226,6 +248,14 @@ export async function buildBrief( const remainingMs = plan.executionDeadlineMs - Date.now(); if (!shouldStartAnalyzer(plan.profile, remainingMs)) { analyzerStatus[name] = "capped"; + analyzerTelemetry[name] = { + status: "capped", + elapsedMs: Date.now() - analyzerStartedAt, + costClass: item.descriptor.cost, + partialStatus: "partial", + partialReason: "analyzer_budget_exhausted", + capped: true, + }; partial = true; analysis.metrics.recordCappedWork("analyzer_budget", 1); return; @@ -238,6 +268,15 @@ export async function buildBrief( ); if (timeoutMs <= 0) { analyzerStatus[name] = "capped"; + analyzerTelemetry[name] = { + status: "capped", + elapsedMs: Date.now() - analyzerStartedAt, + timeoutMs, + costClass: item.descriptor.cost, + partialStatus: "partial", + partialReason: "analyzer_budget_exhausted", + capped: true, + }; partial = true; analysis.metrics.recordCappedWork(`analyzer_${item.descriptor.cost}`, 1); return; @@ -259,10 +298,23 @@ export async function buildBrief( findings[name] = result as never; if (resultIsPartial(result) || diagnostics.partialStatus === "partial") { const status = statusFromDiagnostics(diagnostics, "degraded"); + const partialReason = publicPartialReason( + diagnostics.partialReason, + status === "capped" ? "analyzer_capped" : "analyzer_partial", + ); analyzerStatus[name] = status; + analyzerTelemetry[name] = { + status, + elapsedMs: Date.now() - analyzerStartedAt, + timeoutMs, + costClass: item.descriptor.cost, + partialStatus: "partial", + partialReason, + capped: status === "capped" || diagnostics.capped, + }; partial = true; diagnostics.partialStatus = "partial"; - diagnostics.partialReason ??= status === "capped" ? "analyzer_capped" : "analyzer_partial"; + diagnostics.partialReason = partialReason; if (diagnostics.captureDegradation) { attachAnalysisMetrics(diagnostics, analysis); captureDegradation(new Error(diagnostics.partialReason), { @@ -272,27 +324,50 @@ export async function buildBrief( timeoutMs, elapsedMs: Date.now() - analyzerStartedAt, analyzerStatus: status, + profile: plan.profile, + costClass: item.descriptor.cost, + responseReserveMs: plan.responseReserveMs, diagnostics, options, }); } } else { analyzerStatus[name] = "ok"; + analyzerTelemetry[name] = { + status: "ok", + elapsedMs: Date.now() - analyzerStartedAt, + timeoutMs, + costClass: item.descriptor.cost, + partialStatus: diagnostics.partialStatus, + }; } } catch (error) { const status = timeoutStatus(error, diagnostics); + const partialReason = publicPartialReason(diagnostics.partialReason, "analyzer_error"); analyzerStatus[name] = status; + analyzerTelemetry[name] = { + status, + elapsedMs: Date.now() - analyzerStartedAt, + timeoutMs, + costClass: item.descriptor.cost, + partialStatus: "partial", + partialReason, + capped: status === "capped" || diagnostics.capped, + }; partial = true; diagnostics.partialStatus = "partial"; - diagnostics.partialReason ??= error instanceof Error ? error.message : "analyzer_error"; + diagnostics.partialReason = partialReason; attachAnalysisMetrics(diagnostics, analysis); - captureDegradation(error, { + captureDegradation(new Error(partialReason), { analyzer: name, requested: plan.requested, req, timeoutMs, elapsedMs: Date.now() - analyzerStartedAt, analyzerStatus: status, + profile: plan.profile, + costClass: item.descriptor.cost, + responseReserveMs: plan.responseReserveMs, diagnostics, options, }); @@ -310,21 +385,49 @@ export async function buildBrief( } for (const name of all) - if (!plan.requested.includes(name)) analyzerStatus[name] = "skipped"; + if (!plan.requested.includes(name)) { + analyzerStatus[name] = "skipped"; + analyzerTelemetry[name] ??= { + status: "skipped", + elapsedMs: 0, + skipReason: "not_requested", + }; + } const { promptSection, systemSuffix } = renderBrief( findings, req.budget?.maxBriefChars ?? 6000, ); + const elapsedMs = Date.now() - start; + const metrics = analysis.snapshotMetrics(); + const cacheTotal = metrics.cacheHits + metrics.cacheMisses; return { schemaVersion: 1, repoFullName: req.repoFullName, prNumber: req.prNumber, headSha: req.headSha ?? null, generatedAtIso: new Date().toISOString(), - elapsedMs: Date.now() - start, + elapsedMs, partial, analyzerStatus, + telemetry: { + profile: plan.profile, + responseReserveMs: plan.responseReserveMs, + requestedAnalyzers: plan.requested, + analyzerCount: { + requested: plan.requested.length, + runnable: plan.runnable.length, + skipped: plan.skipped.length, + }, + analyzers: analyzerTelemetry, + cacheHits: metrics.cacheHits, + cacheMisses: metrics.cacheMisses, + cacheHitRate: cacheTotal > 0 ? metrics.cacheHits / cacheTotal : 0, + externalCallsByCategory: metrics.externalCallsByCategory, + skippedWorkByCategory: metrics.skippedWorkByCategory, + cappedWorkByCategory: metrics.cappedWorkByCategory, + elapsedMs, + }, findings, promptSection, systemSuffix, diff --git a/review-enrichment/src/request-guardrails.ts b/review-enrichment/src/request-guardrails.ts new file mode 100644 index 0000000000..5cbf2ade61 --- /dev/null +++ b/review-enrichment/src/request-guardrails.ts @@ -0,0 +1,152 @@ +import type { EnrichRequest } from "./types.js"; + +export const MAX_BODY_BYTES = 2 * 1024 * 1024; +const MAX_FILES = 300; +const MAX_DIFF_BYTES = 1_000_000; +const MAX_TOTAL_PATCH_BYTES = 1_500_000; +const MAX_PATH_CHARS = 1000; +const MAX_ANALYZERS = 100; + +export type EnrichRequestParseResult = + | { ok: true; payload: EnrichRequest; bodyBytes: number } + | { ok: false; status: 400 | 413; error: string; bodyBytes: number }; + +export type EnrichRequestBodyReadResult = + | { ok: true; raw: string; bodyBytes: number } + | { ok: false; status: 413; error: "request_too_large"; bodyBytes: number }; + +export async function readEnrichRequestText(request: Request): Promise { + const contentLength = request.headers.get("content-length"); + if (contentLength) { + const parsedLength = Number.parseInt(contentLength, 10); + if (Number.isFinite(parsedLength) && parsedLength > MAX_BODY_BYTES) { + return { + ok: false, + status: 413, + error: "request_too_large", + bodyBytes: parsedLength, + }; + } + } + + const reader = request.body?.getReader(); + if (!reader) return { ok: true, raw: "", bodyBytes: 0 }; + + const chunks: Uint8Array[] = []; + let bodyBytes = 0; + try { + while (true) { + const { done, value } = await reader.read(); + if (done) break; + bodyBytes += value.byteLength; + if (bodyBytes > MAX_BODY_BYTES) { + await reader.cancel(); + return { + ok: false, + status: 413, + error: "request_too_large", + bodyBytes, + }; + } + chunks.push(value); + } + } finally { + reader.releaseLock(); + } + + return { ok: true, raw: decodeChunks(chunks, bodyBytes), bodyBytes }; +} + +export function parseEnrichRequestBody(raw: string): EnrichRequestParseResult { + const bodyBytes = byteLength(raw); + if (bodyBytes > MAX_BODY_BYTES) { + return { ok: false, status: 413, error: "request_too_large", bodyBytes }; + } + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return { ok: false, status: 400, error: "bad_json", bodyBytes }; + } + + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { + return { ok: false, status: 400, error: "bad_request", bodyBytes }; + } + + const payload = parsed as EnrichRequest; + if (!validRepo(payload.repoFullName) || !validPullNumber(payload.prNumber)) { + return { ok: false, status: 400, error: "bad_request", bodyBytes }; + } + if (payload.files !== undefined && !Array.isArray(payload.files)) { + return { ok: false, status: 400, error: "bad_files", bodyBytes }; + } + if ((payload.files?.length ?? 0) > MAX_FILES) { + return { ok: false, status: 413, error: "too_many_files", bodyBytes }; + } + if (typeof payload.diff === "string" && byteLength(payload.diff) > MAX_DIFF_BYTES) { + return { ok: false, status: 413, error: "diff_too_large", bodyBytes }; + } + if (!validAnalyzers(payload.analyzers)) { + return { ok: false, status: 400, error: "bad_analyzers", bodyBytes }; + } + if (!validFiles(payload.files)) { + return { ok: false, status: 400, error: "bad_files", bodyBytes }; + } + if (totalPatchBytes(payload.files) > MAX_TOTAL_PATCH_BYTES) { + return { ok: false, status: 413, error: "patches_too_large", bodyBytes }; + } + + return { ok: true, payload, bodyBytes }; +} + +function validRepo(value: unknown): value is string { + return ( + typeof value === "string" && + /^[A-Za-z0-9_.-]+\/[A-Za-z0-9_.-]+$/.test(value) && + value.length <= 200 + ); +} + +function validPullNumber(value: unknown): value is number { + return typeof value === "number" && Number.isInteger(value) && value > 0; +} + +function validAnalyzers(value: unknown): boolean { + if (value === undefined) return true; + if (!Array.isArray(value) || value.length > MAX_ANALYZERS) return false; + return value.every((entry) => typeof entry === "string" && entry.length <= 80); +} + +function validFiles(files: EnrichRequest["files"]): boolean { + if (!files) return true; + return files.every((file) => { + if (!file || typeof file !== "object") return false; + if (typeof file.path !== "string" || !file.path || file.path.length > MAX_PATH_CHARS) return false; + if (file.patch !== undefined && typeof file.patch !== "string") return false; + if (file.status !== undefined && typeof file.status !== "string") return false; + if (file.previousPath !== undefined && typeof file.previousPath !== "string") return false; + return true; + }); +} + +function totalPatchBytes(files: EnrichRequest["files"]): number { + return (files ?? []).reduce( + (total, file) => total + (typeof file.patch === "string" ? byteLength(file.patch) : 0), + 0, + ); +} + +function decodeChunks(chunks: readonly Uint8Array[], bodyBytes: number): string { + const buffer = new Uint8Array(bodyBytes); + let offset = 0; + for (const chunk of chunks) { + buffer.set(chunk, offset); + offset += chunk.byteLength; + } + return new TextDecoder().decode(buffer); +} + +function byteLength(value: string): number { + return new TextEncoder().encode(value).byteLength; +} diff --git a/review-enrichment/src/scheduler.ts b/review-enrichment/src/scheduler.ts index 3748d4dc63..51fdb523bc 100644 --- a/review-enrichment/src/scheduler.ts +++ b/review-enrichment/src/scheduler.ts @@ -114,6 +114,15 @@ export interface AnalyzerPlan { executionDeadlineMs: number; } +export interface ReesProfileMetadata { + name: ReesProfileName; + default: boolean; + costClasses: AnalyzerCostClass[]; + concurrency: Record; + timeoutMs: Record; + responseReserveMs: number; +} + export function resolveReesProfile(value: unknown): ReesProfileName { if (typeof value !== "string") return DEFAULT_REES_PROFILE; const normalized = value.trim().toLowerCase(); @@ -124,6 +133,20 @@ export function isReesProfileName(value: string): value is ReesProfileName { return (REES_PROFILES as readonly string[]).includes(value); } +export function reesProfileMetadata(): ReesProfileMetadata[] { + return REES_PROFILES.map((name) => { + const config = PROFILE_CONFIG[name]; + return { + name, + default: name === DEFAULT_REES_PROFILE, + costClasses: COST_ORDER.filter((cost) => config.costs.has(cost)), + concurrency: { ...config.concurrency }, + timeoutMs: { ...config.timeoutMs }, + responseReserveMs: config.responseReserveMs, + }; + }); +} + export function responseReserveMs(profile: ReesProfileName, budgetMs: number): number { const configured = PROFILE_CONFIG[profile].responseReserveMs; const proportional = Math.floor(Math.max(0, budgetMs) * 0.2); diff --git a/review-enrichment/src/sentry.ts b/review-enrichment/src/sentry.ts index 0722d0cec1..c178dbbf69 100644 --- a/review-enrichment/src/sentry.ts +++ b/review-enrichment/src/sentry.ts @@ -111,6 +111,9 @@ export interface AnalyzerDegradationContext { timeoutMs?: number; elapsedMs?: number; analyzerStatus?: string; + profile?: string; + costClass?: string; + responseReserveMs?: number; partialStatus?: string; partialReason?: string; phase?: string; @@ -147,6 +150,9 @@ export function captureAnalyzerDegradation(error: unknown, context: AnalyzerDegr timeoutMs: context.timeoutMs, elapsedMs: context.elapsedMs, analyzerStatus: context.analyzerStatus, + profile: context.profile, + costClass: context.costClass, + responseReserveMs: context.responseReserveMs, partialStatus: context.partialStatus, partialReason: context.partialReason, phase: context.phase, @@ -187,6 +193,9 @@ export function captureAnalyzerDegradation(error: unknown, context: AnalyzerDegr if (timeoutTag) scope.setTag("timeoutMs", timeoutTag); if (releaseTag) scope.setTag("release", releaseTag); const analyzerStatusTag = sentryTagValue(context.analyzerStatus); + const profileTag = sentryTagValue(context.profile); + const costClassTag = sentryTagValue(context.costClass); + const responseReserveTag = sentryTagValue(context.responseReserveMs); const partialStatusTag = sentryTagValue(context.partialStatus); const phaseTag = sentryTagValue(context.phase); const endpointCategoryTag = sentryTagValue(context.endpointCategory); @@ -197,6 +206,9 @@ export function captureAnalyzerDegradation(error: unknown, context: AnalyzerDegr const cacheHitsTag = sentryTagValue(context.cacheHits); const cacheMissesTag = sentryTagValue(context.cacheMisses); if (analyzerStatusTag) scope.setTag("analyzerStatus", analyzerStatusTag); + if (profileTag) scope.setTag("profile", profileTag); + if (costClassTag) scope.setTag("costClass", costClassTag); + if (responseReserveTag) scope.setTag("responseReserveMs", responseReserveTag); if (partialStatusTag) scope.setTag("partialStatus", partialStatusTag); if (phaseTag) scope.setTag("phase", phaseTag); if (endpointCategoryTag) scope.setTag("endpointCategory", endpointCategoryTag); diff --git a/review-enrichment/src/server.ts b/review-enrichment/src/server.ts index 248ac2f8cc..c981b80186 100644 --- a/review-enrichment/src/server.ts +++ b/review-enrichment/src/server.ts @@ -10,8 +10,11 @@ import { serve } from "@hono/node-server"; import { Hono } from "hono"; import { normalizeSharedSecret, verifyBearer } from "./auth.js"; -import type { EnrichRequest } from "./types.js"; import { buildBrief } from "./brief.js"; +import { + parseEnrichRequestBody, + readEnrichRequestText, +} from "./request-guardrails.js"; import { captureError, flushSentry, @@ -54,18 +57,13 @@ app.post("/v1/enrich", async (c) => { if (!verifyBearer(c.req.header("authorization"), secret)) return c.json({ error: "unauthorized" }, 401); - const payload = (await c.req - .json() - .catch(() => null)) as EnrichRequest | null; - if ( - !payload || - typeof payload.repoFullName !== "string" || - typeof payload.prNumber !== "number" - ) { - return c.json({ error: "bad_request" }, 400); - } + const body = await readEnrichRequestText(c.req.raw); + if (!body.ok) return c.json({ error: body.error }, body.status); + + const parsed = parseEnrichRequestBody(body.raw); + if (!parsed.ok) return c.json({ error: parsed.error }, parsed.status); - const brief = await buildBrief(payload, undefined, { + const brief = await buildBrief(parsed.payload, undefined, { requestId: c.req.header("x-gittensory-request-id") ?? c.req.header("x-request-id"), traceId: traceIdFromTraceparent(c.req.header("traceparent")), }); diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index 671882d042..d3e4bd61e7 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -350,7 +350,38 @@ export interface ReviewBrief { elapsedMs: number; partial: boolean; analyzerStatus: Record; + telemetry: ReviewBriefTelemetry; findings: BriefFindings; promptSection: string; systemSuffix: string; } + +export interface ReviewBriefTelemetry { + profile: ReesProfileName; + responseReserveMs: number; + requestedAnalyzers: string[]; + analyzerCount: { + requested: number; + runnable: number; + skipped: number; + }; + analyzers: Record; + cacheHits: number; + cacheMisses: number; + cacheHitRate: number; + externalCallsByCategory: Record; + skippedWorkByCategory: Record; + cappedWorkByCategory: Record; + elapsedMs: number; +} + +export interface AnalyzerTelemetry { + status: AnalyzerStatus; + elapsedMs: number; + timeoutMs?: number; + costClass?: string; + partialStatus?: "complete" | "partial"; + partialReason?: string; + skipReason?: string; + capped?: boolean; +} diff --git a/review-enrichment/test/analyzer-metadata.test.ts b/review-enrichment/test/analyzer-metadata.test.ts new file mode 100644 index 0000000000..e7e2c80486 --- /dev/null +++ b/review-enrichment/test/analyzer-metadata.test.ts @@ -0,0 +1,49 @@ +import { readFileSync } from "node:fs"; +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { ANALYZER_DESCRIPTORS, ANALYZER_NAMES } from "../dist/analyzers/registry.js"; +import { reesProfileMetadata } from "../dist/scheduler.js"; + +test("generated analyzer metadata matches the runtime registry and profiles", () => { + const metadata = JSON.parse(readFileSync("analyzer-metadata.json", "utf8")) as { + schemaVersion: number; + defaultProfile: string; + profiles: Array<{ name: string; default: boolean; costClasses: string[] }>; + analyzers: Array<{ + name: string; + title: string; + category: string; + cost: string; + defaultEnabled: boolean; + profiles: string[]; + requires: string[]; + limits: Record; + docs: Record; + }>; + }; + + assert.equal(metadata.schemaVersion, 1); + assert.equal(metadata.defaultProfile, "balanced"); + assert.deepEqual( + metadata.profiles.map((profile) => profile.name), + reesProfileMetadata().map((profile) => profile.name), + ); + assert.deepEqual( + metadata.analyzers.map((analyzer) => analyzer.name), + ANALYZER_NAMES, + ); + + for (const descriptor of ANALYZER_DESCRIPTORS) { + const generated = metadata.analyzers.find((analyzer) => analyzer.name === descriptor.name); + assert.ok(generated, `missing generated metadata for ${descriptor.name}`); + assert.equal(generated.title, descriptor.title); + assert.equal(generated.category, descriptor.category); + assert.equal(generated.cost, descriptor.cost); + assert.equal(generated.defaultEnabled, descriptor.defaultEnabled); + assert.deepEqual(generated.requires, descriptor.requires); + assert.deepEqual(generated.limits, descriptor.limits ?? {}); + assert.equal(generated.docs.summary, descriptor.docs.summary); + assert.ok(generated.profiles.includes("balanced")); + } +}); diff --git a/review-enrichment/test/request-guardrails.test.ts b/review-enrichment/test/request-guardrails.test.ts new file mode 100644 index 0000000000..ae7b2fc953 --- /dev/null +++ b/review-enrichment/test/request-guardrails.test.ts @@ -0,0 +1,153 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; + +import { + MAX_BODY_BYTES, + parseEnrichRequestBody, + readEnrichRequestText, +} from "../dist/request-guardrails.js"; + +test("parseEnrichRequestBody accepts a minimal valid enrichment request", () => { + const result = parseEnrichRequestBody( + JSON.stringify({ + repoFullName: "JSONbored/gittensory", + prNumber: 1814, + files: [{ path: "src/a.ts", patch: "@@ -1,0 +1,1 @@\n+export const a = 1;" }], + }), + ); + + assert.equal(result.ok, true); + if (result.ok) { + assert.equal(result.payload.repoFullName, "JSONbored/gittensory"); + assert.equal(result.payload.prNumber, 1814); + assert.ok(result.bodyBytes > 0); + } +}); + +test("parseEnrichRequestBody rejects malformed JSON and invalid shallow schema", () => { + const malformed = parseEnrichRequestBody("{not json"); + assert.deepEqual(malformed, { + ok: false, + status: 400, + error: "bad_json", + bodyBytes: 9, + }); + + const badSchema = parseEnrichRequestBody(JSON.stringify({ repoFullName: "bad", prNumber: 0 })); + assert.equal(badSchema.ok, false); + if (!badSchema.ok) { + assert.equal(badSchema.status, 400); + assert.equal(badSchema.error, "bad_request"); + } +}); + +test("parseEnrichRequestBody rejects oversized body, file list, diff, and patch payloads", () => { + const hugeBody = parseEnrichRequestBody("x".repeat(2 * 1024 * 1024 + 1)); + assert.equal(hugeBody.ok, false); + if (!hugeBody.ok) { + assert.equal(hugeBody.status, 413); + assert.equal(hugeBody.error, "request_too_large"); + } + + const tooManyFiles = parseEnrichRequestBody( + JSON.stringify({ + repoFullName: "JSONbored/gittensory", + prNumber: 1814, + files: Array.from({ length: 301 }, (_, index) => ({ path: `src/${index}.ts` })), + }), + ); + assert.equal(tooManyFiles.ok, false); + if (!tooManyFiles.ok) assert.equal(tooManyFiles.error, "too_many_files"); + + const hugeDiff = parseEnrichRequestBody( + JSON.stringify({ + repoFullName: "JSONbored/gittensory", + prNumber: 1814, + diff: "x".repeat(1_000_001), + }), + ); + assert.equal(hugeDiff.ok, false); + if (!hugeDiff.ok) assert.equal(hugeDiff.error, "diff_too_large"); + + const hugePatch = parseEnrichRequestBody( + JSON.stringify({ + repoFullName: "JSONbored/gittensory", + prNumber: 1814, + files: [{ path: "src/a.ts", patch: "x".repeat(1_500_001) }], + }), + ); + assert.equal(hugePatch.ok, false); + if (!hugePatch.ok) assert.equal(hugePatch.error, "patches_too_large"); +}); + +test("readEnrichRequestText rejects an oversized Content-Length without reading the body", async () => { + const body = new ReadableStream({ + pull(controller) { + controller.enqueue(new Uint8Array([123])); + controller.close(); + }, + }); + const request = new Request("https://rees.example/v1/enrich", { + method: "POST", + headers: { "content-length": String(MAX_BODY_BYTES + 1) }, + body, + duplex: "half", + } as RequestInit); + + const result = await readEnrichRequestText(request); + + assert.equal(result.ok, false); + if (!result.ok) { + assert.equal(result.status, 413); + assert.equal(result.error, "request_too_large"); + assert.equal(result.bodyBytes, MAX_BODY_BYTES + 1); + } + assert.equal(request.bodyUsed, false); +}); + +test("readEnrichRequestText stops streaming once the request body exceeds the cap", async () => { + let pulls = 0; + let canceled = false; + const body = new ReadableStream({ + pull(controller) { + pulls += 1; + controller.enqueue(new Uint8Array(1024 * 1024)); + if (pulls > 5) controller.close(); + }, + cancel() { + canceled = true; + }, + }); + const request = new Request("https://rees.example/v1/enrich", { + method: "POST", + body, + duplex: "half", + } as RequestInit); + + const result = await readEnrichRequestText(request); + + assert.equal(result.ok, false); + if (!result.ok) { + assert.equal(result.status, 413); + assert.equal(result.error, "request_too_large"); + assert.ok(result.bodyBytes > MAX_BODY_BYTES); + } + assert.equal(canceled, true); + assert.ok(pulls < 6); +}); + +test("readEnrichRequestText returns a small request body", async () => { + const raw = JSON.stringify({ repoFullName: "JSONbored/gittensory", prNumber: 1836 }); + const request = new Request("https://rees.example/v1/enrich", { + method: "POST", + body: raw, + }); + + const result = await readEnrichRequestText(request); + + assert.equal(result.ok, true); + if (result.ok) { + assert.equal(result.raw, raw); + assert.equal(result.bodyBytes, new TextEncoder().encode(raw).byteLength); + } +}); diff --git a/review-enrichment/test/scheduler.test.ts b/review-enrichment/test/scheduler.test.ts index 071244e237..c562479651 100644 --- a/review-enrichment/test/scheduler.test.ts +++ b/review-enrichment/test/scheduler.test.ts @@ -81,6 +81,12 @@ test("slow analyzers time out inside the reserved response budget", async () => assert.equal(brief.partial, true); assert.equal(brief.analyzerStatus.history, "timeout"); + assert.equal(brief.telemetry.profile, "balanced"); + assert.equal(brief.telemetry.requestedAnalyzers[0], "history"); + assert.equal(brief.telemetry.analyzers.history.status, "timeout"); + assert.equal(brief.telemetry.analyzers.history.partialReason, "analyzer_timeout"); + assert.ok((brief.telemetry.analyzers.history.timeoutMs ?? 0) < 300); + assert.ok(brief.telemetry.responseReserveMs > 0); assert.ok(Date.now() - started < 1000); assert.ok(brief.elapsedMs < 1000); }); @@ -168,4 +174,7 @@ test("registry analyzers skip when their relevant inputs are absent", async () = assert.equal(dependencyRan, false); assert.equal(brief.analyzerStatus.dependency, "skipped"); assert.equal(brief.analyzerStatus.secret, "ok"); + assert.equal(brief.telemetry.analyzers.dependency.skipReason, "no_dependency_manifest"); + assert.equal(brief.telemetry.analyzers.secret.status, "ok"); + assert.ok(brief.telemetry.skippedWorkByCategory.analyzer_no_dependency_manifest >= 1); }); diff --git a/review-enrichment/test/sentry-degradation.test.ts b/review-enrichment/test/sentry-degradation.test.ts index 8c07cd9326..75195b4b58 100644 --- a/review-enrichment/test/sentry-degradation.test.ts +++ b/review-enrichment/test/sentry-degradation.test.ts @@ -130,6 +130,9 @@ test("captureAnalyzerDegradation attaches safe attribution context for history f timeoutMs: 7000, elapsedMs: 6812, analyzerStatus: "degraded", + profile: "balanced", + costClass: "github-heavy", + responseReserveMs: 750, partialStatus: "partial", partialReason: "history_budget_exhausted", phase: "similar_past_prs", @@ -161,6 +164,9 @@ test("captureAnalyzerDegradation attaches safe attribution context for history f assert.equal(sentry.tags.headShaPrefix, "abcdef123456"); assert.equal(sentry.tags.timeoutMs, "7000"); assert.equal(sentry.tags.analyzerStatus, "degraded"); + assert.equal(sentry.tags.profile, "balanced"); + assert.equal(sentry.tags.costClass, "github-heavy"); + assert.equal(sentry.tags.responseReserveMs, "750"); assert.equal(sentry.tags.partialStatus, "partial"); assert.equal(sentry.tags.phase, "similar_past_prs"); assert.equal(sentry.tags.endpointCategory, "github-commit-pulls"); @@ -180,6 +186,9 @@ test("captureAnalyzerDegradation attaches safe attribution context for history f timeoutMs: 7000, elapsedMs: 6812, analyzerStatus: "degraded", + profile: "balanced", + costClass: "github-heavy", + responseReserveMs: 750, partialStatus: "partial", partialReason: "history_budget_exhausted", phase: "similar_past_prs", @@ -212,6 +221,7 @@ test("captureAnalyzerDegradation attaches safe attribution context for history f test("buildBrief stays fail-open and captures a degraded analyzer", async () => { const sentry = sentryHarness(); + const fakeToken = ["ghp", "abcdefghijklmnopqrstuvwxyz1234567890"].join("_"); const brief = await buildBrief( { @@ -224,7 +234,7 @@ test("buildBrief stays fail-open and captures a degraded analyzer", async () => }, { dependency: async () => { - throw new Error("osv unavailable"); + throw new Error(`osv unavailable for ${fakeToken}`); }, }, ); @@ -234,8 +244,11 @@ test("buildBrief stays fail-open and captures a degraded analyzer", async () => assert.deepEqual(brief.findings, {}); assert.equal(brief.repoFullName, "JSONbored/gittensory"); assert.equal(brief.prNumber, 42); + assert.equal(brief.telemetry.analyzers.dependency.partialReason, "analyzer_error"); + assert.equal(JSON.stringify(brief.telemetry).includes(fakeToken), false); + assert.equal(JSON.stringify(brief.telemetry).includes("osv unavailable"), false); assert.equal(sentry.captured.length, 1); - assert.equal(sentry.captured[0].message, "osv unavailable"); + assert.equal(sentry.captured[0].message, "analyzer_error"); assert.equal(sentry.tags.analyzer, "dependency"); assert.equal(sentry.tags.repo, "JSONbored/gittensory"); assert.equal(sentry.tags.pullNumber, "42"); @@ -245,6 +258,32 @@ test("buildBrief stays fail-open and captures a degraded analyzer", async () => assert.ok(capturedTimeoutMs <= 200); }); +test("buildBrief normalizes unsafe analyzer partial reasons before response telemetry", async () => { + const fakeToken = ["ghp", "abcdefghijklmnopqrstuvwxyz1234567890"].join("_"); + + const brief = await buildBrief( + { + repoFullName: "JSONbored/gittensory", + prNumber: 42, + analyzers: ["history"], + linkedIssue: { number: 9, title: "add history context" }, + diff: `+${fakeToken}`, + budget: { timeoutMs: 200 }, + }, + { + history: async (_req, context) => { + context.diagnostics.partialReason = `unsafe ${fakeToken}`; + return [{ author: null, similarPastPrs: [], linkedIssueAlignment: null, partial: true }]; + }, + }, + ); + + assert.equal(brief.partial, true); + assert.equal(brief.analyzerStatus.history, "degraded"); + assert.equal(brief.telemetry.analyzers.history.partialReason, "analyzer_partial"); + assert.equal(JSON.stringify(brief.telemetry).includes(fakeToken), false); +}); + test("buildBrief returns a timed-out partial response before the caller timeout budget is spent", async () => { const started = Date.now(); const brief = await buildBrief(