Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions review-enrichment/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
218 changes: 218 additions & 0 deletions review-enrichment/src/analyzers/iac-misconfig.ts
Original file line number Diff line number Diff line change
@@ -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<string> {
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<string>,
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<string>();
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<IacMisconfigFinding[]> {
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;
}
2 changes: 2 additions & 0 deletions review-enrichment/src/brief.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -45,6 +46,7 @@ const ANALYZERS: Record<keyof BriefFindings, AnalyzerFn> = {
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 }),
};

Expand Down
31 changes: 31 additions & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
15 changes: 15 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) */
Expand Down Expand Up @@ -209,6 +223,7 @@ export interface BriefFindings {
secretLog?: SecretLogFinding[];
assetWeight?: AssetWeightFinding[];
typosquat?: TyposquatFinding[];
iacMisconfig?: IacMisconfigFinding[];
nativeBuild?: NativeBuildFinding[];
}

Expand Down
Loading