diff --git a/review-enrichment/README.md b/review-enrichment/README.md index 8f9d7acac2..acd1e8519c 100644 --- a/review-enrichment/README.md +++ b/review-enrichment/README.md @@ -38,6 +38,7 @@ read CODEOWNERS and blob sizes. The engine prefers a short-lived installation to | `secretLog` | Secrets, PII, or request/session objects written to logs/stdout. | Pure local. | | `assetWeight` | Heavy binary assets added or grown. | Calls GitHub API; needs headSha, baseSha for growth, and token for private repos. | | `typosquat` | New dependency names that look squatted or publicly claimable. | Uses bundled popular-package lists plus npm/PyPI lookups. | +| `iacMisconfig` | Risky IaC/config changes like public buckets, open ingress, or insecure CORS. | Pure local. | The engine can send `analyzers: ["secret", "actionPin"]` to run a subset. If the field is omitted, REES runs the full registry. An explicit empty array runs no analyzers; the engine uses that fail-closed shape when an diff --git a/review-enrichment/src/analyzers/iac-misconfig.ts b/review-enrichment/src/analyzers/iac-misconfig.ts new file mode 100644 index 0000000000..2a81c7d82f --- /dev/null +++ b/review-enrichment/src/analyzers/iac-misconfig.ts @@ -0,0 +1,218 @@ +import type { EnrichRequest, IacMisconfigFinding } from "../types.js"; + +const MAX_FINDINGS = 25; +const MAX_LINE_CHARS = 2000; + +const CONFIG_PATH_RE = + /(?:^|\/)(?:docker-compose[^/]*\.ya?ml|compose[^/]*\.ya?ml|values(?:\.[^/]+)?\.ya?ml|.*\.(?:tf|ya?ml|json|toml|ini|conf|env)|Dockerfile(?:\.[^/]+)?|nginx[^/]*\.conf)$/i; + +const CORS_ORIGIN_RE = + /\b(?:access-control-allow-origin|allow_origin|cors_origin|origin)\b[\s"'=:,\[\]-]*\*/i; +const CORS_CREDENTIALS_RE = + /\b(?:access-control-allow-credentials|allow_credentials|credentials)\b[\s"'=:,-]*(?:true|yes|on)\b/i; +const OPEN_INGRESS_RE = + /\b(?:cidr_blocks|source_ranges|ipv4_cidr_blocks|cidr|ip_range|value)\b[^\n#]*0\.0\.0\.0\/0\b|\b0\.0\.0\.0\/0\b/i; +const PUBLIC_BUCKET_RE = + /(?:(?:["'])?(?:bucket_)?acl(?:["'])?\s*[=:]\s*["']public-(?:read|read-write)["']|(?:["'])?public_access(?:["'])?\s*[=:]\s*true\b|(?:["'])?public(?:["'])?\s*[=:]\s*true\b|(?:["'])?block_public_(?:acls|policy)(?:["'])?\s*[=:]\s*false\b)/i; +const SAME_SITE_NONE_RE = /\bsameSite\b[\s"'=:,-]*["']?none["']?\b/i; +const SECURE_FALSE_RE = /\bsecure\b[\s"'=:,-]*false\b/i; +const TLS_DISABLED_RE = + /\brejectUnauthorized\b[\s"'=:,-]*false\b|\bverify\s*=\s*False\b|\bssl_verify\b[\s"'=:,-]*false\b/i; +const PROD_RE = + /\b(?:NODE_ENV|ENVIRONMENT|APP_ENV)\b[\s"'=:,-]*production\b|\bproduction\s*:/i; +const DEBUG_TRUE_RE = /\bdebug\b[\s"'=:,-]*true\b|\bDEBUG\b[\s"'=:,-]*true\b/i; +const HARDCODED_URL_RE = + /\b(?:[A-Z][A-Z0-9_]*(?:URL|URI|ENDPOINT)|(?:api|base|service|backend|frontend|server|webhook)[_-]?(?:url|uri|endpoint)|baseUrl)\b[\s"'=:,-]*https?:\/\/[^\s"',#}]+/i; + +function* patchLines(patch: string): Generator { + let start = 0; + for (let i = 0; i <= patch.length; i++) { + if (i === patch.length || patch[i] === "\n") { + yield patch.slice(start, i); + start = i + 1; + } + } +} + +type ScanLimits = { + maxFindings?: number; + signal?: AbortSignal; +}; + +export function isRelevantConfigPath(path: string): boolean { + return CONFIG_PATH_RE.test(path); +} + +function pushFinding( + findings: IacMisconfigFinding[], + seen: Set, + file: string, + line: number, + kind: IacMisconfigFinding["kind"], + maxFindings: number, +): boolean { + const key = `${kind}:${line}`; + if (seen.has(key)) return false; + seen.add(key); + findings.push({ file, line, kind }); + return findings.length >= maxFindings; +} + +export function scanPatchForIacMisconfig( + path: string, + patch: string, + limits: ScanLimits = {}, +): IacMisconfigFinding[] { + const maxFindings = limits.maxFindings ?? MAX_FINDINGS; + if (maxFindings <= 0) return []; + + const findings: IacMisconfigFinding[] = []; + const seen = new Set(); + let newLine = 0; + let corsOriginLine = 0; + let corsCredentialsLine = 0; + let sameSiteLine = 0; + let secureFalseLine = 0; + let prodLine = 0; + let debugLine = 0; + + for (const line of patchLines(patch)) { + if (limits.signal?.aborted) throw new Error("analyzer_aborted"); + if (line.startsWith("+++") || line.startsWith("---")) continue; + const hunk = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line); + if (hunk) { + newLine = Number(hunk[1]); + corsOriginLine = 0; + corsCredentialsLine = 0; + sameSiteLine = 0; + secureFalseLine = 0; + prodLine = 0; + debugLine = 0; + continue; + } + if (!line.startsWith("+")) { + if (!line.startsWith("-")) newLine++; + continue; + } + + const body = line.slice(1); + if (body.length > MAX_LINE_CHARS) { + newLine++; + continue; + } + + if (CORS_ORIGIN_RE.test(body)) corsOriginLine = newLine; + if (CORS_CREDENTIALS_RE.test(body)) corsCredentialsLine = newLine; + if (SAME_SITE_NONE_RE.test(body)) sameSiteLine = newLine; + if (SECURE_FALSE_RE.test(body)) secureFalseLine = newLine; + if (PROD_RE.test(body)) prodLine = newLine; + if (DEBUG_TRUE_RE.test(body)) debugLine = newLine; + + if ( + corsOriginLine && + corsCredentialsLine && + pushFinding( + findings, + seen, + path, + Math.max(corsOriginLine, corsCredentialsLine), + "wildcard-cors-credentials", + maxFindings, + ) + ) { + return findings; + } + + if ( + sameSiteLine && + secureFalseLine && + pushFinding( + findings, + seen, + path, + Math.max(sameSiteLine, secureFalseLine), + "insecure-cookie", + maxFindings, + ) + ) { + return findings; + } + + if ( + prodLine && + debugLine && + pushFinding( + findings, + seen, + path, + Math.max(prodLine, debugLine), + "prod-debug", + maxFindings, + ) + ) { + return findings; + } + + if ( + OPEN_INGRESS_RE.test(body) && + pushFinding(findings, seen, path, newLine, "open-ingress", maxFindings) + ) { + return findings; + } + if ( + PUBLIC_BUCKET_RE.test(body) && + pushFinding(findings, seen, path, newLine, "public-bucket", maxFindings) + ) { + return findings; + } + if ( + TLS_DISABLED_RE.test(body) && + pushFinding( + findings, + seen, + path, + newLine, + "tls-verification-disabled", + maxFindings, + ) + ) { + return findings; + } + if ( + HARDCODED_URL_RE.test(body) && + pushFinding( + findings, + seen, + path, + newLine, + "hardcoded-service-url", + maxFindings, + ) + ) { + return findings; + } + + newLine++; + } + + return findings; +} + +export async function scanIacMisconfig( + req: EnrichRequest, + signal?: AbortSignal, +): Promise { + const findings: IacMisconfigFinding[] = []; + for (const file of req.files ?? []) { + if (signal?.aborted) throw new Error("analyzer_aborted"); + if (!file.patch || !isRelevantConfigPath(file.path)) continue; + for (const finding of scanPatchForIacMisconfig(file.path, file.patch, { + maxFindings: MAX_FINDINGS - findings.length, + signal, + })) { + findings.push(finding); + if (findings.length >= MAX_FINDINGS) return findings; + } + } + return findings; +} diff --git a/review-enrichment/src/brief.ts b/review-enrichment/src/brief.ts index ed19645260..69d2742df6 100644 --- a/review-enrichment/src/brief.ts +++ b/review-enrichment/src/brief.ts @@ -21,6 +21,7 @@ import { scanCodeowners } from "./analyzers/codeowners.js"; import { scanSecretLog } from "./analyzers/secret-log.js"; import { scanAssetWeight } from "./analyzers/asset-weight.js"; import { scanTyposquat } from "./analyzers/typosquat.js"; +import { scanIacMisconfig } from "./analyzers/iac-misconfig.js"; import { scanNativeBuild } from "./analyzers/native-build.js"; import { renderBrief } from "./render.js"; import { captureAnalyzerDegradation } from "./sentry.js"; @@ -45,6 +46,7 @@ const ANALYZERS: Record = { secretLog: (req, signal) => scanSecretLog(req, signal), assetWeight: (req, signal) => scanAssetWeight(req, fetch, { signal }), typosquat: (req, signal) => scanTyposquat(req, fetch, { signal }), + iacMisconfig: (req, signal) => scanIacMisconfig(req, signal), nativeBuild: (req, signal) => scanNativeBuild(req, fetch, { signal }), }; diff --git a/review-enrichment/src/render.ts b/review-enrichment/src/render.ts index 7edcebbdbf..780694562e 100644 --- a/review-enrichment/src/render.ts +++ b/review-enrichment/src/render.ts @@ -281,6 +281,37 @@ export function renderBrief( } } + const iacMisconfigs = findings.iacMisconfig ?? []; + if (iacMisconfigs.length) { + const explain = ( + kind: (typeof iacMisconfigs)[number]["kind"], + ): string => { + switch (kind) { + case "wildcard-cors-credentials": + return "allows wildcard CORS together with credentials; browsers can send authenticated cross-origin requests"; + case "open-ingress": + return "opens ingress to `0.0.0.0/0`; verify the service is not world-accessible"; + case "public-bucket": + return "makes object storage public; verify this bucket is intended for anonymous access"; + case "insecure-cookie": + return "sets `SameSite=None` without `Secure=true`; browsers can send the cookie cross-site over insecure transport"; + case "tls-verification-disabled": + return "disables TLS certificate verification; this permits man-in-the-middle interception"; + case "prod-debug": + return "enables debug mode in production configuration; this can expose internals or sensitive data"; + case "hardcoded-service-url": + return "hardcodes a service URL in config; prefer environment-specific injection or secrets-managed config"; + } + }; + + lines.push("### IaC / config misconfigurations (review before merging)"); + for (const item of iacMisconfigs) { + lines.push( + `- ${safeCodeSpan(`${item.file}:${item.line}`)} — ${explain(item.kind)}`, + ); + } + } + const nativeBuilds = findings.nativeBuild ?? []; if (nativeBuilds.length) { lines.push( diff --git a/review-enrichment/src/types.ts b/review-enrichment/src/types.ts index 639b7d1970..1ff483ff8a 100644 --- a/review-enrichment/src/types.ts +++ b/review-enrichment/src/types.ts @@ -178,6 +178,20 @@ export interface TyposquatFinding { reason: string; } +/** A static IaC / config misconfiguration introduced by the PR. Reports the location + rule only. */ +export interface IacMisconfigFinding { + file: string; + line: number; + kind: + | "wildcard-cors-credentials" + | "open-ingress" + | "public-bucket" + | "insecure-cookie" + | "tls-verification-disabled" + | "prod-debug" + | "hardcoded-service-url"; +} + /** A newly-added dependency whose install compiles native code (npm node-gyp addon) or has no prebuilt wheel * (PyPI sdist-only) — a hidden CI cold-start/install cost and a frequent cross-platform breakage source. Reports * package@version + the factual build property only. (#1512) */ @@ -209,6 +223,7 @@ export interface BriefFindings { secretLog?: SecretLogFinding[]; assetWeight?: AssetWeightFinding[]; typosquat?: TyposquatFinding[]; + iacMisconfig?: IacMisconfigFinding[]; nativeBuild?: NativeBuildFinding[]; } diff --git a/review-enrichment/test/enrichment.test.ts b/review-enrichment/test/enrichment.test.ts index c5481d618b..fbeb062211 100644 --- a/review-enrichment/test/enrichment.test.ts +++ b/review-enrichment/test/enrichment.test.ts @@ -53,6 +53,11 @@ import { scanPatchForSecretLog, scanSecretLog, } from "../dist/analyzers/secret-log.js"; +import { + isRelevantConfigPath, + scanPatchForIacMisconfig, + scanIacMisconfig, +} from "../dist/analyzers/iac-misconfig.js"; const NOW = new Date("2026-06-26").getTime(); const eolFetch = @@ -1282,6 +1287,179 @@ test("buildBrief: action-pin analyzer runs (pure, no network)", async () => { } }); +test("isRelevantConfigPath: matches infra/config targets and skips code files", () => { + assert.equal(isRelevantConfigPath("infra/main.tf"), true); + assert.equal(isRelevantConfigPath("deploy/values.prod.yaml"), true); + assert.equal(isRelevantConfigPath("Dockerfile"), true); + assert.equal(isRelevantConfigPath("src/server.ts"), false); +}); + +test("scanPatchForIacMisconfig: flags paired and direct config risks with line citations", () => { + const patch = [ + "@@ -1,0 +1,13 @@", + "+cors:", + "+ origin: '*'", + "+ credentials: true", + "+security_group_rules:", + '+ cidr_blocks = ["0.0.0.0/0"]', + '+bucket_acl = "public-read"', + "+cookie:", + "+ sameSite: none", + "+ secure: false", + "+production:", + "+ debug: true", + '+ API_URL: "https://internal.example.com"', + ].join("\n"); + + assert.deepEqual( + scanPatchForIacMisconfig("infra/stack.yaml", patch).map( + ({ line, kind }) => ({ line, kind }), + ), + [ + { line: 3, kind: "wildcard-cors-credentials" }, + { line: 5, kind: "open-ingress" }, + { line: 6, kind: "public-bucket" }, + { line: 9, kind: "insecure-cookie" }, + { line: 11, kind: "prod-debug" }, + { line: 12, kind: "hardcoded-service-url" }, + ], + ); +}); + +test("scanPatchForIacMisconfig: flags TLS verification disabled and handles debug before prod", () => { + const patch = [ + "@@ -1,0 +1,4 @@", + "+DEBUG=true", + "+rejectUnauthorized: false", + "+verify=False", + "+NODE_ENV=production", + ].join("\n"); + + assert.deepEqual( + scanPatchForIacMisconfig("Dockerfile", patch).map(({ line, kind }) => ({ + line, + kind, + })), + [ + { line: 2, kind: "tls-verification-disabled" }, + { line: 3, kind: "tls-verification-disabled" }, + { line: 4, kind: "prod-debug" }, + ], + ); +}); + +test("scanPatchForIacMisconfig: flags public bucket settings with quoted JSON keys", () => { + const patch = [ + "@@ -1,0 +1,4 @@", + '+ "public_access": true,', + '+ "public": true,', + '+ "block_public_acls": false,', + '+ "bucket_acl": "public-read"', + ].join("\n"); + + assert.deepEqual( + scanPatchForIacMisconfig("infra/bucket.json", patch).map( + ({ line, kind }) => ({ line, kind }), + ), + [ + { line: 1, kind: "public-bucket" }, + { line: 2, kind: "public-bucket" }, + { line: 3, kind: "public-bucket" }, + { line: 4, kind: "public-bucket" }, + ], + ); +}); + +test("scanPatchForIacMisconfig: respects the finding budget", () => { + const findings = scanPatchForIacMisconfig( + "infra/main.tf", + [ + "@@ -1,0 +1,3 @@", + '+cidr_blocks = ["0.0.0.0/0"]', + "+rejectUnauthorized: false", + '+BASE_URL = "https://svc.example.com"', + ].join("\n"), + { maxFindings: 2 }, + ); + + assert.deepEqual( + findings.map(({ line, kind }) => ({ line, kind })), + [ + { line: 1, kind: "open-ingress" }, + { line: 2, kind: "tls-verification-disabled" }, + ], + ); +}); + +test("scanIacMisconfig: scans only matching config files and forwards abort", async () => { + const findings = await scanIacMisconfig({ + repoFullName: "o/r", + prNumber: 1, + files: [ + { + path: "infra/main.tf", + patch: '@@ -1,0 +1,1 @@\n+cidr_blocks = ["0.0.0.0/0"]', + }, + { + path: "src/index.ts", + patch: '@@ -1,0 +1,1 @@\n+const baseUrl = "https://svc.example.com";', + }, + ], + }); + assert.equal(findings.length, 1); + assert.equal(findings[0].kind, "open-ingress"); + + const controller = new AbortController(); + controller.abort(); + await assert.rejects( + () => + scanIacMisconfig( + { + repoFullName: "o/r", + prNumber: 1, + files: [ + { + path: "infra/main.tf", + patch: '@@ -1,0 +1,1 @@\n+cidr_blocks = ["0.0.0.0/0"]', + }, + ], + }, + controller.signal, + ), + /analyzer_aborted/, + ); +}); + +test("renderBrief: renders the IaC misconfig block", () => { + const r = renderBrief({ + iacMisconfig: [ + { file: "infra/main.tf", line: 14, kind: "open-ingress" }, + { file: "deploy/values.yaml", line: 9, kind: "insecure-cookie" }, + ], + }); + assert.match(r.promptSection, /IaC \/ config misconfigurations/); + assert.match(r.promptSection, /`infra\/main\.tf:14`/); + assert.match(r.promptSection, /world-accessible/); + assert.match(r.promptSection, /SameSite=None/); +}); + +test("buildBrief: iac-misconfig analyzer runs and renders findings", async () => { + const brief = await buildBrief({ + repoFullName: "o/r", + prNumber: 1, + analyzers: ["iacMisconfig"], + files: [ + { + path: "infra/main.tf", + patch: '@@ -1,0 +1,1 @@\n+cidr_blocks = ["0.0.0.0/0"]', + }, + ], + }); + assert.equal(brief.analyzerStatus.iacMisconfig, "ok"); + assert.equal(brief.findings.iacMisconfig.length, 1); + assert.match(brief.promptSection, /IaC \/ config misconfigurations/); +}); + test("hasCatastrophicBacktracking: flags nested unbounded quantifiers, not linear/bounded shapes", () => { for (const vuln of [ "(a+)+",