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
170 changes: 170 additions & 0 deletions review-enrichment/src/analyzers/commit-signature.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
// Commit-signature / verified-author provenance analyzer (#1517). Inspects the PR head commit's signature
// verification verdict, its author/committer identity, and — when the head is from an author with no prior
// verified history in a repo that otherwise carries verified commits — flags a never-before-seen committer.
// These are supply-chain / impersonation signals the no-checkout `claude --print` reviewer cannot derive
// (no GitHub commit-verification API access, no repo history). Surfaces ONLY GitHub's public verification
// verdict (`verified` + `reason`) and boolean provenance flags — never tokens, emails, or private identities.
import type { EnrichRequest, CommitSignatureFinding } from "../types.js";

const GITHUB_API = "https://api.github.com";
// Pull a bounded slice of recent commits — enough to decide "has any verified history" without paging the whole
// repo. The history check runs at most two such queries (author-filtered + repo-wide), matching how the other
// analyzers cap their network round-trips.
const HISTORY_PER_PAGE = 30;
// Only repository slugs that look like real `owner/repo` segments are ever interpolated into a request URL.
const SLUG_RE = /^[A-Za-z0-9._-]+$/;

interface ScanOptions {
signal?: AbortSignal;
}

// The slice of the GitHub commit payload this analyzer reads. Everything else on the response is ignored.
interface CommitResponse {
commit?: {
verification?: { verified?: boolean; reason?: string };
author?: { name?: string };
committer?: { name?: string };
};
author?: { login?: string } | null;
committer?: { login?: string } | null;
}

interface HistoryCommit {
commit?: { verification?: { verified?: boolean } };
}

function githubHeaders(token: string): Record<string, string> {
return {
Authorization: `Bearer ${token}`,
Accept: "application/vnd.github+json",
"X-GitHub-Api-Version": "2022-11-28",
};
}

/** Fetch the head commit's verification + identity payload. Returns null on any error / non-200 (fail-safe). */
export async function fetchHeadCommit(
owner: string,
repo: string,
headSha: string,
headers: Record<string, string>,
fetchFn: typeof fetch,
signal?: AbortSignal,
): Promise<CommitResponse | null> {
if (signal?.aborted) return null;
try {
const resp = await fetchFn(
`${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/commits/${encodeURIComponent(headSha)}`,
{ headers, signal },
);
if (!resp.ok) return null;
return (await resp.json()) as CommitResponse;
} catch {
return null;
}
}

/** Fetch one bounded page of the repo's recent commits, optionally filtered to a single author, and report
* whether ANY of them carry a verified signature. Returns true/false on a definitive answer, or null when
* undeterminable (network error / non-200 / unexpected shape) — callers fail safe on null. */
export async function hasVerifiedHistory(
owner: string,
repo: string,
headers: Record<string, string>,
fetchFn: typeof fetch,
author?: string,
signal?: AbortSignal,
): Promise<boolean | null> {
if (signal?.aborted) return null;
const authorQuery = author ? `author=${encodeURIComponent(author)}&` : "";
try {
const resp = await fetchFn(
`${GITHUB_API}/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/commits?${authorQuery}per_page=${HISTORY_PER_PAGE}`,
{ headers, signal },
);
if (!resp.ok) return null;
const commits = (await resp.json()) as HistoryCommit[];
if (!Array.isArray(commits)) return null;
return commits.some((c) => c.commit?.verification?.verified === true);
} catch {
return null;
}
}

/** Analyzer entrypoint: inspect the head commit's signature + author provenance. Fail-safe — returns no finding
* on a missing token / head SHA, an unresolvable repo slug, or any fetch error, and never throws. */
export async function scanCommitSignature(
req: EnrichRequest,
fetchFn: typeof fetch = fetch,
options: ScanOptions = {},
): Promise<CommitSignatureFinding[]> {
const { repoFullName, githubToken, headSha } = req;
if (!githubToken || !headSha) return [];

// Require EXACTLY `owner/repo`. A 3+ segment value like `o/r/extra` would otherwise keep parts[0]/parts[1]
// and silently query the wrong repository (`o/r`) instead of failing safe, so reject anything that is not a
// clean two-segment slug before building any GitHub URL.
const parts = repoFullName.split("/");
const owner = parts[0];
const repo = parts[1];
if (parts.length !== 2 || !owner || !repo || !SLUG_RE.test(owner) || !SLUG_RE.test(repo)) return [];

const headers = githubHeaders(githubToken);
const head = await fetchHeadCommit(
owner,
repo,
headSha,
headers,
fetchFn,
options.signal,
);
if (!head?.commit) return [];

const verified = head.commit.verification?.verified === true;
const reason = head.commit.verification?.reason ?? "unknown";
const authorLogin = head.author?.login;
const committerLogin = head.committer?.login;
// An author/committer login mismatch can indicate a rewritten/impersonated authorship; only compare when both
// logins are resolved (GitHub leaves them null for unmatched email identities, which is not itself a mismatch).
const authorMismatch =
Boolean(authorLogin) &&
Boolean(committerLogin) &&
authorLogin !== committerLogin;

// A never-before-seen committer is only a signal when the repo otherwise HAS verified history but THIS author
// has none — a repo with no verified commits at all is simply unsigned, not impersonated. Two bounded history
// queries (author-filtered + repo-wide); either being undeterminable (null) fails safe to no flag.
let newCommitter = false;
if (authorLogin && !options.signal?.aborted) {
const authorVerified = await hasVerifiedHistory(
owner,
repo,
headers,
fetchFn,
authorLogin,
options.signal,
);
if (authorVerified === false && !options.signal?.aborted) {
const repoVerified = await hasVerifiedHistory(
owner,
repo,
headers,
fetchFn,
undefined,
options.signal,
);
newCommitter = repoVerified === true;
}
}

// Nothing noteworthy: a verified head with a matching author and no new-committer signal needs no finding.
if (verified && !authorMismatch && !newCommitter) return [];

const finding: CommitSignatureFinding = {
verified,
reason,
authorMismatch,
newCommitter,
...(authorLogin ? { authorLogin } : {}),
};
return [finding];
}
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 { scanCommitSignature } from "./analyzers/commit-signature.js";
import { scanIacMisconfig } from "./analyzers/iac-misconfig.js";
import { scanNativeBuild } from "./analyzers/native-build.js";
import { renderBrief } from "./render.js";
Expand All @@ -46,6 +47,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 }),
commitSignature: (req, signal) => scanCommitSignature(req, fetch, { signal }),
iacMisconfig: (req, signal) => scanIacMisconfig(req, signal),
nativeBuild: (req, signal) => scanNativeBuild(req, fetch, { signal }),
};
Expand Down
26 changes: 26 additions & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,32 @@ export function renderBrief(
}
}

const commitSignatures = findings.commitSignature ?? [];
if (commitSignatures.length) {
lines.push(
"### Head-commit signature / author provenance (verify before merging)",
);
for (const item of commitSignatures) {
const status = item.verified
? "signature **verified**"
: "signature **unverified**";
const flags: string[] = [];
if (item.authorMismatch)
flags.push("commit author and committer logins differ");
if (item.newCommitter)
flags.push(
"author has no verified history in a repo that otherwise carries verified commits",
);
const who = item.authorLogin
? ` by ${safeCodeSpan(item.authorLogin)}`
: "";
const detail = flags.length ? ` — ${flags.join("; ")}` : "";
lines.push(
`- head commit${who}: ${status} (${safeCodeSpan(item.reason)})${detail}`,
);
}
}

const iacMisconfigs = findings.iacMisconfig ?? [];
if (iacMisconfigs.length) {
const explain = (
Expand Down
19 changes: 19 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,24 @@ export interface TyposquatFinding {
reason: string;
}

/** A head commit whose signature/author provenance warrants scrutiny: an unsigned/unverified-signature head, an
* author/committer login mismatch, or a never-before-seen committer in a repo that otherwise has verified history
* — supply-chain/impersonation signals the no-checkout reviewer cannot derive. Surfaces ONLY the public GitHub
* verification verdict (`verified` + `reason`) and boolean provenance flags — never tokens, emails, or identities
* beyond the public commit author login GitHub already exposes. (#1517) */
export interface CommitSignatureFinding {
/** GitHub's signature verification verdict for the head commit. */
verified: boolean;
/** GitHub's machine-readable verification reason (e.g. `unsigned`, `valid`, `unknown_key`). Public-safe string. */
reason: string;
/** The head commit author's GitHub login, when GitHub resolves one — public, already shown on the PR. */
authorLogin?: string;
/** True when the commit author login differs from the committer login (a potential authorship mismatch). */
authorMismatch: boolean;
/** True when the author login has no prior verified commit in a repo that otherwise carries verified history. */
newCommitter: boolean;
}

/** A static IaC / config misconfiguration introduced by the PR. Reports the location + rule only. */
export interface IacMisconfigFinding {
file: string;
Expand Down Expand Up @@ -223,6 +241,7 @@ export interface BriefFindings {
secretLog?: SecretLogFinding[];
assetWeight?: AssetWeightFinding[];
typosquat?: TyposquatFinding[];
commitSignature?: CommitSignatureFinding[];
iacMisconfig?: IacMisconfigFinding[];
nativeBuild?: NativeBuildFinding[];
}
Expand Down
Loading