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
179 changes: 179 additions & 0 deletions review-enrichment/src/analyzers/codeowners.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
// CODEOWNERS + blast-radius analyzer (#1515). Fetches .github/CODEOWNERS (with fallbacks to CODEOWNERS and
// docs/CODEOWNERS), matches each changed file against the glob rules using last-match-wins semantics (per GitHub),
// and reports files where the PR author is absent from the owner list — plus the blast radius derived at render
// time from the unique set of ownership domains (users/teams) crossed by the PR.
// Glob-to-regex conversion uses only atomic `[^/]*`, `.*`, and literal escapes — no catastrophic backtracking.
// Fail-safe: returns [] on any network error, non-ok response, or missing/unreadable CODEOWNERS file.
import type { EnrichRequest, CodeownersFinding } from "../types.js";

const SLUG_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/; // rejects `..` and other path-traversal segments
const CODEOWNERS_PATHS = [
".github/CODEOWNERS",
"CODEOWNERS",
"docs/CODEOWNERS",
] as const;
const MAX_FILES_REPORTED = 20;

interface ParsedRule {
regex: RegExp;
owners: string[];
}

// ── Glob matching ─────────────────────────────────────────────────────────────

/** Convert a CODEOWNERS glob pattern to a RegExp that matches repo-root-relative file paths.
* `*` matches any non-`/` characters; `**` matches across separators; `?` matches one non-`/` char.
* A leading `/` or interior `/` anchors the pattern to the repo root; a leading glob does not. */
export function patternToRegex(pattern: string): RegExp {
let p = pattern;

const leadingSlash = p.startsWith("/");
if (leadingSlash) p = p.slice(1);

// Trailing `/` means "all files under this directory" — expand to `<dir>/**`.
if (p.endsWith("/")) p += "**";

// Anchored when explicitly rooted, or when a path separator appears outside a leading `**/`.
const anchored = leadingSlash || (p.includes("/") && !p.startsWith("**/"));

let re = "";
let i = 0;
while (i < p.length) {
const c = p[i]!;
if (c === "*" && i + 1 < p.length && p[i + 1] === "*") {
i += 2;
if (p[i] === "/") i++; // consume the `/` that follows `**`
re += ".*";
} else if (c === "*") {
re += "[^/]*";
i++;
} else if (c === "?") {
re += "[^/]";
i++;
} else {
re += c.replace(/[.+^()|\{\}\[\]\\$]/g, "\\$&");
i++;
}
}

return new RegExp(anchored ? `^${re}$` : `(^|/)${re}$`);
}

// ── CODEOWNERS parser ─────────────────────────────────────────────────────────

/** Parse CODEOWNERS text into ordered rules. Lines are returned in source order; last match wins at query time. */
export function parseCodeowners(content: string): ParsedRule[] {
const rules: ParsedRule[] = [];
for (const rawLine of content.split("\n")) {
const line = rawLine.trim();
if (!line || line.startsWith("#")) continue;
const parts = line.split(/\s+/);
const pattern = parts[0];
if (!pattern) continue;
// Accept @handle and @org/team; also plain email (contains `@` but no leading `@`).
const owners = parts
.slice(1)
.filter((o) => o.startsWith("@") || (o.includes("@") && !o.startsWith("#")));
if (owners.length === 0) continue; // no owners → unowned pattern, skip
try {
rules.push({ regex: patternToRegex(pattern), owners });
} catch {
// malformed pattern — skip
}
}
return rules;
}

/** Find the owners for a repo-root-relative file path. Last matching rule wins (CODEOWNERS semantics). */
export function findOwners(rules: ParsedRule[], filePath: string): string[] {
let owners: string[] = [];
for (const rule of rules) {
if (rule.regex.test(filePath)) owners = rule.owners;
}
return owners;
}

/** True when the PR author (GitHub login) appears in the CODEOWNERS owner list, normalising the leading `@`. */
export function authorMatchesOwner(author: string, owners: string[]): boolean {
const norm = author.startsWith("@")
? author.toLowerCase()
: `@${author.toLowerCase()}`;
return owners.some((o) => o.toLowerCase() === norm);
}

// ── Network ───────────────────────────────────────────────────────────────────

/** Try each CODEOWNERS location in priority order; return raw content of the first found, or null. */
async function fetchCodeowners(
owner: string,
repo: string,
headers: Record<string, string>,
fetchFn: typeof fetch,
signal?: AbortSignal,
): Promise<string | null> {
for (const path of CODEOWNERS_PATHS) {
try {
const resp = await fetchFn(
`https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/contents/${path}`,
{ headers, signal },
);
if (!resp.ok) continue;
return await resp.text();
} catch {
// network error or already-aborted signal → try next location
}
}
return null;
}

// ── Analyzer entrypoint ───────────────────────────────────────────────────────

/** Report changed files whose CODEOWNERS rule does not include the PR author, and surface blast-radius context. */
export async function scanCodeowners(
req: EnrichRequest,
fetchFn: typeof fetch,
opts?: { signal?: AbortSignal },
): Promise<CodeownersFinding[]> {
const { repoFullName, githubToken, author, files = [] } = req;
if (!githubToken || !author) return [];

const parts = repoFullName.split("/");
const repoOwner = parts[0];
const repoName = parts[1];
if (
!repoOwner ||
!repoName ||
!SLUG_RE.test(repoOwner) ||
!SLUG_RE.test(repoName)
)
return [];

const headers: Record<string, string> = {
Authorization: `Bearer ${githubToken}`,
Accept: "application/vnd.github.raw",
"X-GitHub-Api-Version": "2022-11-28",
};

const content = await fetchCodeowners(
repoOwner,
repoName,
headers,
fetchFn,
opts?.signal,
);
if (!content) return [];

const rules = parseCodeowners(content);
if (rules.length === 0) return [];

const findings: CodeownersFinding[] = [];
for (const file of files) {
if (findings.length >= MAX_FILES_REPORTED) break;
const owners = findOwners(rules, file.path);
if (owners.length === 0) continue; // unowned file — not a violation
if (authorMatchesOwner(author, owners)) continue; // author is listed — no violation
findings.push({ file: file.path, owners });
}

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 @@ -14,6 +14,7 @@ import { scanInstallScripts } from "./analyzers/install-scripts.js";
import { scanActionPins } from "./analyzers/actions-pin.js";
import { scanEol } from "./analyzers/eol-check.js";
import { scanRedos } from "./analyzers/redos.js";
import { scanCodeowners } from "./analyzers/codeowners.js";
import { renderBrief } from "./render.js";

type AnalyzerFn = (req: EnrichRequest, signal: AbortSignal) => Promise<unknown>;
Expand All @@ -27,6 +28,7 @@ const ANALYZERS: Record<keyof BriefFindings, AnalyzerFn> = {
actionPin: (req) => scanActionPins(req),
eol: (req) => scanEol(req),
redos: (req) => scanRedos(req),
codeowners: (req, signal) => scanCodeowners(req, fetch, { signal }),
};

function runWithTimeout<T>(
Expand Down
13 changes: 13 additions & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,19 @@ export function renderBrief(
}
}

const codeownersViolations = findings.codeowners ?? [];
if (codeownersViolations.length) {
const allOwners = new Set(codeownersViolations.flatMap((f) => f.owners));
const blastRadius = allOwners.size;
lines.push(
`### CODEOWNERS violations — ${blastRadius} ownership domain${blastRadius === 1 ? "" : "s"} affected`,
);
for (const item of codeownersViolations) {
const ownerList = item.owners.map((o) => safeCodeSpan(o)).join(", ");
lines.push(`- ${safeCodeSpan(item.file)} — owned by ${ownerList}`);
}
}

if (!lines.length) return { promptSection: "", systemSuffix: "" };

const header =
Expand Down
8 changes: 8 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ export interface RedosFinding {
pattern: string;
}

/** A changed file governed by a CODEOWNERS rule where the PR author is not listed as an owner (#1515).
* The blast radius (distinct ownership domains crossed) is derived at render time from the full findings set. */
export interface CodeownersFinding {
file: string;
owners: string[]; // sorted owners from the last-matching CODEOWNERS rule; always non-empty
}

/** Structured analyzer output. Each analyzer fills its own key; more land as analyzers ship (#1477/#1478). */
export interface BriefFindings {
dependency?: DependencyFinding[];
Expand All @@ -102,6 +109,7 @@ export interface BriefFindings {
installScript?: InstallScriptFinding[];
eol?: EolFinding[];
redos?: RedosFinding[];
codeowners?: CodeownersFinding[];
}

export type AnalyzerStatus = "ok" | "degraded" | "skipped";
Expand Down