Skip to content
108 changes: 108 additions & 0 deletions review-enrichment/src/analyzers/commit-signature.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Commit-signature / verified-author provenance analyzer (#1517). Detects two supply-chain signals that the
// no-checkout reviewer cannot assess on their own:
// "unsigned" — the PR head commit is not signed/verified by GitHub
// (commit.verification.verified = false; reason exposed as context).
// "new-committer" — the committing author has no prior commits in this repo, yet the repo's recent history
// is ≥80% verified-commit signed — a potential impersonation/injection vector.
// Network: two or three GitHub REST calls (head commit, recent repo commits, author history) under the shared
// AbortSignal timeout. Fail-safe: returns [] on any error or missing prerequisite.
import type { EnrichRequest, CommitSignatureFinding } from "../types.js";

const MAX_HISTORY_COMMITS = 20;
const MIN_HISTORY_FOR_PATTERN = 3; // need ≥ this many non-head commits to infer the repo's signing pattern
const VERIFIED_RATIO_THRESHOLD = 0.8; // only flag new-committer when ≥80% of recent commits are verified

// Allowlists to prevent path traversal when these values are interpolated into API URL paths.
const SHA_RE = /^[a-f0-9]{7,40}$/i;
const SLUG_RE = /^[a-zA-Z0-9][a-zA-Z0-9._-]*$/; // must start with alphanumeric — rejects ".." and other dot-only traversal segments

interface GitHubCommit {
sha: string;
commit: {
verification: {
verified: boolean;
reason: string;
};
};
author: { login: string } | null;
}

/** Fetch head-commit verification status, then optionally check for never-before-seen committers. */
export async function scanCommitSignature(
req: EnrichRequest,
fetchFn: typeof fetch,
opts?: { signal?: AbortSignal },
): Promise<CommitSignatureFinding[]> {
const { repoFullName, headSha, githubToken } = req;
if (!githubToken || !headSha) return [];

if (!SHA_RE.test(headSha)) return [];

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

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

// Phase 1: fetch the head commit and check its verification status.
let headCommit: GitHubCommit;
try {
const resp = await fetchFn(
`https://api.github.com/repos/${encodeURIComponent(owner)}/${encodeURIComponent(repo)}/commits/${encodeURIComponent(headSha)}`,
{ headers, signal: opts?.signal },
);
if (!resp.ok) return [];
headCommit = (await resp.json()) as GitHubCommit;
} catch {
return [];
}

const authorLogin = headCommit.author?.login ?? null;
const verification = headCommit.commit.verification;

if (!verification.verified) {
return [{ headSha, authorLogin, kind: "unsigned", reason: verification.reason }];
}

// Phase 2: check whether this is a new committer in a repo with a verified-commit pattern.
// Skip the check when the author identity is unknown (no GitHub user linked to the commit email).
if (!authorLogin) return [];

try {
const recentResp = await fetchFn(
`https://api.github.com/repos/${owner}/${repo}/commits?per_page=${MAX_HISTORY_COMMITS}`,
{ headers, signal: opts?.signal },
);
if (!recentResp.ok) return [];
const recentCommits = (await recentResp.json()) as GitHubCommit[];

// Exclude the current head from history so the ratio reflects the pre-existing signing pattern.
const others = recentCommits.filter((c) => c.sha !== headSha);
if (others.length < MIN_HISTORY_FOR_PATTERN) return [];

const verifiedCount = others.filter((c) => c.commit.verification.verified).length;
if (verifiedCount / others.length < VERIFIED_RATIO_THRESHOLD) return [];

// Repo uses verified commits — does this author have any prior commits here?
const authorHistoryResp = await fetchFn(
`https://api.github.com/repos/${owner}/${repo}/commits?author=${encodeURIComponent(authorLogin)}&per_page=3`,
{ headers, signal: opts?.signal },
);
if (!authorHistoryResp.ok) return [];
const authorHistory = (await authorHistoryResp.json()) as { sha: string }[];

const priorCommits = authorHistory.filter((c) => c.sha !== headSha);
if (priorCommits.length === 0) {
return [{ headSha, authorLogin, kind: "new-committer", reason: null }];
}
} catch {
return [];
}

return [];
}
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 { scanCommitSignature } from "./analyzers/commit-signature.js";
import { scanCodeowners } from "./analyzers/codeowners.js";
import { scanSecretLog } from "./analyzers/secret-log.js";
import { renderBrief } from "./render.js";
Expand All @@ -29,6 +30,7 @@ const ANALYZERS: Record<keyof BriefFindings, AnalyzerFn> = {
actionPin: (req) => scanActionPins(req),
eol: (req) => scanEol(req),
redos: (req) => scanRedos(req),
commitSignature: (req, signal) => scanCommitSignature(req, fetch, { signal }),
codeowners: (req, signal) => scanCodeowners(req, fetch, { signal }),
secretLog: (req, signal) => scanSecretLog(req, signal),
};
Expand Down
21 changes: 21 additions & 0 deletions review-enrichment/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,27 @@ export function renderBrief(
}
}

const commitSigs = findings.commitSignature ?? [];
if (commitSigs.length) {
lines.push("### Commit-signature / verified-author provenance");
for (const item of commitSigs) {
const sha = safeCodeSpan(item.headSha.slice(0, 8));
if (item.kind === "unsigned") {
const reason = safeCodeSpan(item.reason ?? "unknown");
lines.push(
`- ${sha} — commit is not signed or verified (reason: ${reason}); sign commits via gpg or ssh`,
);
} else {
const who = item.authorLogin
? safeCodeSpan(item.authorLogin)
: "the commit author";
lines.push(
`- ${sha} — ${who} is a first-time committer in this repository, which otherwise uses verified commits (supply-chain risk: potential impersonation)`,
);
}
}
}

const codeownersViolations = findings.codeowners ?? [];
if (codeownersViolations.length) {
const allOwners = new Set(codeownersViolations.flatMap((f) => f.owners));
Expand Down
12 changes: 12 additions & 0 deletions review-enrichment/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,17 @@ export interface RedosFinding {
pattern: string;
}

/** A commit-signature or verified-author provenance signal (#1517).
* "unsigned": head commit is not signed/verified by GitHub.
* "new-committer": the author has no prior commits in a repo that otherwise uses verified commits
* (supply-chain / impersonation risk). */
export interface CommitSignatureFinding {
headSha: string;
authorLogin: string | null;
kind: "unsigned" | "new-committer";
reason: string | null;
}

/** 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 {
Expand All @@ -118,6 +129,7 @@ export interface BriefFindings {
installScript?: InstallScriptFinding[];
eol?: EolFinding[];
redos?: RedosFinding[];
commitSignature?: CommitSignatureFinding[];
codeowners?: CodeownersFinding[];
secretLog?: SecretLogFinding[];
}
Expand Down
Loading