From 84220c1f96c7f1244f9210c309849d921fb6885b Mon Sep 17 00:00:00 2001 From: anirudhkumar-nanonets Date: Mon, 24 Aug 2026 19:45:57 +0530 Subject: [PATCH 1/6] feat(app): a GitHub App that reviews pull requests, forks included MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workflow on a fork's PR gets a read-only token and cannot comment; an App's installation token belongs to the base repo, so a fork is ordinary work. It also serves the viewer page itself behind a signed expiring link, which is what a private repo needs and GitHub Pages cannot give. node:http and node:crypto only — this process holds a private key, a webhook secret and other people's source, so every dependency is one more thing to trust. The PR's code is never executed: no install, no build, no hooks, just tree-sitter reading source text. --- Dockerfile | 34 +++++++ src/app/checkout.ts | 128 ++++++++++++++++++++++++++ src/app/events.ts | 89 ++++++++++++++++++ src/app/identity.ts | 122 +++++++++++++++++++++++++ src/app/main.ts | 59 ++++++++++++ src/app/pages.ts | 103 +++++++++++++++++++++ src/app/queue.ts | 78 ++++++++++++++++ src/app/review.ts | 158 ++++++++++++++++++++++++++++++++ src/app/server.ts | 148 ++++++++++++++++++++++++++++++ test/app-checkout.test.ts | 113 +++++++++++++++++++++++ test/app-events.test.ts | 114 +++++++++++++++++++++++ test/app-identity.test.ts | 97 ++++++++++++++++++++ test/app-server.test.ts | 184 ++++++++++++++++++++++++++++++++++++++ 13 files changed, 1427 insertions(+) create mode 100644 Dockerfile create mode 100644 src/app/checkout.ts create mode 100644 src/app/events.ts create mode 100644 src/app/identity.ts create mode 100644 src/app/main.ts create mode 100644 src/app/pages.ts create mode 100644 src/app/queue.ts create mode 100644 src/app/review.ts create mode 100644 src/app/server.ts create mode 100644 test/app-checkout.test.ts create mode 100644 test/app-events.test.ts create mode 100644 test/app-identity.test.ts create mode 100644 test/app-server.test.ts diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 00000000..b5ccb67f --- /dev/null +++ b/Dockerfile @@ -0,0 +1,34 @@ +# The graft GitHub App. +# +# Runs anywhere that takes a container — Fly, Cloud Run, ECS, a VM. It needs git +# on the PATH (it fetches pull request refs) and nothing else at runtime. +# +# The build stage keeps devDependencies out of the final image: this process +# clones code written by strangers, so the less that is installed next to it, the +# smaller the blast radius of anything that goes wrong. +FROM node:20-bookworm-slim AS build +WORKDIR /app +COPY package.json package-lock.json ./ +RUN npm ci +COPY . . +RUN npm run build + +FROM node:20-bookworm-slim +# git is a runtime dependency here, not a build one. +RUN apt-get update \ + && apt-get install -y --no-install-recommends git ca-certificates \ + && rm -rf /var/lib/apt/lists/* +WORKDIR /app +ENV NODE_ENV=production +COPY package.json package-lock.json ./ +RUN npm ci --omit=dev && npm cache clean --force +COPY --from=build /app/dist ./dist + +# Never root: the whole point of the checkout rules is that this process handles +# untrusted source, and it has no reason to be able to write outside its tree. +USER node +ENV PORT=3000 +EXPOSE 3000 +HEALTHCHECK --interval=30s --timeout=5s --start-period=10s \ + CMD node -e "fetch('http://127.0.0.1:'+(process.env.PORT||3000)+'/healthz').then(r=>process.exit(r.ok?0:1)).catch(()=>process.exit(1))" +CMD ["node", "dist/app/main.js"] diff --git a/src/app/checkout.ts b/src/app/checkout.ts new file mode 100644 index 00000000..33cbcee0 --- /dev/null +++ b/src/app/checkout.ts @@ -0,0 +1,128 @@ +/** + * Getting a pull request's code onto disk without running any of it. + * + * On a fork PR every byte here was written by a stranger, and this service holds + * an installation token for the base repository. So the rules are absolute: + * + * - **Nothing is executed.** No `npm install`, no build, no hooks. The graph is + * produced by tree-sitter reading source text, which runs none of it. + * - **Git is told not to run things either.** A repository can carry hooks, and + * `core.hooksPath` is a per-repo config a clone would otherwise honour; both + * are disabled explicitly rather than assumed absent. + * - **The token never lands in the repo.** It is passed per-invocation through + * an http header config, not baked into a remote URL that `git remote -v`, + * the reflog and any submodule would carry. + * - **Bounded.** A hostile or merely enormous repository must not hold a worker + * forever: every git call has a timeout, and the fetch is shallow. + */ +import { spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +/** Enough history for a merge base against the pull request's base branch. */ +const FETCH_DEPTH = 50; +const GIT_TIMEOUT_MS = 120_000; + +export interface CheckoutRequest { + owner: string; + repo: string; + number: number; + baseRef: string; + token: string; + api?: string; +} + +export interface Checkout { + /** Working tree at the pull request's merge commit. */ + dir: string; + /** What `blast --base` should diff against. */ + base: string; + /** Removes the tree. Always call it — the token's clone is not something to + * leave in /tmp. */ + cleanup: () => void; +} + +/** A git invocation with the dangerous parts of the environment removed. */ +function git(dir: string, args: string[], token?: string): { ok: boolean; err: string } { + const auth = token + ? [ + "-c", + // Basic auth with the token as the password, per GitHub's own guidance for + // installation tokens. Passed as an argument to THIS call so it is never + // written into .git/config. + `http.extraheader=Authorization: Basic ${Buffer.from(`x-access-token:${token}`).toString("base64")}`, + ] + : []; + const res = spawnSync( + "git", + ["-c", "core.hooksPath=/dev/null", "-c", "protocol.version=2", ...auth, ...args], + { + cwd: dir, + encoding: "utf8", + timeout: GIT_TIMEOUT_MS, + stdio: ["ignore", "pipe", "pipe"], + env: { + ...process.env, + // No credential prompts, no system config, no repo-supplied helpers. + GIT_TERMINAL_PROMPT: "0", + GIT_CONFIG_NOSYSTEM: "1", + GIT_ASKPASS: "", + HOME: dir, + }, + }, + ); + return { ok: res.status === 0, err: `${res.stderr ?? ""}${res.error ? ` ${res.error.message}` : ""}`.trim() }; +} + +/** + * Fetch the PR's merge ref and its base, shallow. + * + * `refs/pull/N/merge` is GitHub's own merge of the PR into its base — the same + * thing the checks see, and the right thing to review: it is what will land, not + * what the contributor's branch says in isolation. + */ +export function checkoutPullRequest(req: CheckoutRequest): Checkout { + const host = (req.api ?? "https://github.com").replace(/\/$/, ""); + const dir = mkdtempSync(join(tmpdir(), `graft-app-${req.owner}-${req.repo}-`)); + const cleanup = (): void => rmSync(dir, { recursive: true, force: true }); + + const steps: Array<[string, string[]]> = [ + ["init", ["init", "--quiet", "-b", "__graft_base"]], + ["remote", ["remote", "add", "origin", `${host}/${req.owner}/${req.repo}.git`]], + // Both refs in one fetch: the merge ref to review, the base to diff against. + [ + "fetch", + [ + "fetch", + "--quiet", + `--depth=${FETCH_DEPTH}`, + "--no-recurse-submodules", + "origin", + `+refs/pull/${req.number}/merge:refs/graft/merge`, + `+refs/heads/${req.baseRef}:refs/graft/base`, + ], + ], + ["checkout", ["checkout", "--quiet", "--detach", "refs/graft/merge"]], + ]; + + for (const [name, args] of steps) { + const { ok, err } = git(dir, args, name === "fetch" ? req.token : undefined); + if (!ok) { + cleanup(); + // A merge ref is absent while GitHub is still computing mergeability, and + // present-but-stale right after a push — worth saying which step failed so + // that case is distinguishable from a permissions problem. + throw new Error(`checkout ${req.owner}/${req.repo}#${req.number} failed at ${name}: ${redact(err, req.token)}`); + } + } + + return { dir, base: "refs/graft/base", cleanup }; +} + +/** Never let a token reach a log line, even inside git's own error text. */ +export function redact(text: string, token: string): string { + if (!token) return text; + const encoded = Buffer.from(`x-access-token:${token}`).toString("base64"); + return text.split(token).join("[token]").split(encoded).join("[token]"); +} diff --git a/src/app/events.ts b/src/app/events.ts new file mode 100644 index 00000000..7d641a9e --- /dev/null +++ b/src/app/events.ts @@ -0,0 +1,89 @@ +/** + * What arrived, and whether it is worth work. + * + * A busy installation delivers a lot of noise — labels, assignments, reviews — + * and every accepted delivery costs a clone and a parse. Narrowing happens here, + * once, so the queue only ever holds jobs that will produce a comment. + */ + +/** The subset of a `pull_request` delivery this App reads. */ +export interface ReviewJob { + installationId: number; + owner: string; + repo: string; + /** Base repository's full name, which is where the comment goes — a fork PR + * carries a different head repo, and posting there would be posting on the + * contributor's copy. */ + number: number; + baseRef: string; + headSha: string; + /** True when the head branch lives in a fork: the PR's code is not ours. */ + fromFork: boolean; +} + +interface RawPullRequest { + action?: string; + number?: number; + installation?: { id?: number }; + repository?: { name?: string; owner?: { login?: string }; full_name?: string }; + pull_request?: { + number?: number; + draft?: boolean; + state?: string; + base?: { ref?: string; repo?: { full_name?: string } }; + head?: { sha?: string; repo?: { full_name?: string } }; + }; +} + +/** Actions that change the diff. `edited` (a retitled PR) and `labeled` do not. */ +const ACTIONS = new Set(["opened", "synchronize", "reopened", "ready_for_review"]); + +/** + * A delivery to act on, or null with the reason it was skipped. + * + * Draft pull requests are skipped deliberately: a draft is a work in progress and + * a bot commenting on every push to one is the fastest way to be uninstalled. + * `ready_for_review` is in the accepted set so the comment appears the moment the + * author asks for eyes. + */ +export function reviewJobFor(event: string, payload: unknown): { job: ReviewJob } | { skip: string } { + if (event === "ping") return { skip: "ping" }; + if (event !== "pull_request") return { skip: `event ${event}` }; + + const p = payload as RawPullRequest; + const action = p.action ?? ""; + if (!ACTIONS.has(action)) return { skip: `action ${action}` }; + + const pr = p.pull_request; + const installationId = p.installation?.id; + const owner = p.repository?.owner?.login; + const repo = p.repository?.name; + const number = pr?.number ?? p.number; + const baseRef = pr?.base?.ref; + const headSha = pr?.head?.sha; + + if (!installationId || !owner || !repo || !number || !baseRef || !headSha) { + return { skip: "payload missing installation, repository or pull request fields" }; + } + if (pr?.draft === true && action !== "ready_for_review") return { skip: "draft" }; + if (pr?.state === "closed") return { skip: "closed" }; + + const base = pr?.base?.repo?.full_name; + const head = pr?.head?.repo?.full_name; + return { + job: { + installationId, + owner, + repo, + number, + baseRef, + headSha, + // A missing head repo means a deleted fork — treat it as one, since it is + // certainly not ours. + fromFork: head !== base, + }, + }; +} + +/** Collapse queued work per pull request: only the newest push is worth reviewing. */ +export const jobKey = (j: ReviewJob): string => `${j.owner}/${j.repo}#${j.number}`; diff --git a/src/app/identity.ts b/src/app/identity.ts new file mode 100644 index 00000000..06de2968 --- /dev/null +++ b/src/app/identity.ts @@ -0,0 +1,122 @@ +/** + * Who the App is, to GitHub. + * + * A GitHub App authenticates twice over: it signs a short JWT with its private + * key to prove it is the App, then trades that for an *installation* token scoped + * to one repository owner. The installation token is what matters here — it is + * the reason this exists at all. A workflow on a fork's pull request gets a + * read-only token and cannot comment; an App's installation token belongs to the + * base repository, so a fork PR is no different from any other. + * + * `node:crypto` signs the JWT, so there is no dependency to audit for something + * that handles a private key. + */ +import { createHmac, createSign, timingSafeEqual } from "node:crypto"; + +/** GitHub rejects a JWT older than 60s or more than 10 minutes in the future. */ +const JWT_TTL_S = 540; // 9 minutes, inside the limit with room for clock skew +/** Renew this long before expiry rather than racing it. */ +const RENEW_MARGIN_MS = 60_000; + +const b64url = (b: Buffer | string): string => + Buffer.from(b).toString("base64").replace(/=/g, "").replace(/\+/g, "-").replace(/\//g, "_"); + +export interface AppCredentials { + appId: string; + /** PEM, as GitHub hands it over. */ + privateKey: string; +} + +/** + * A signed App JWT. + * + * `iat` is backdated a minute: GitHub compares against ITS clock, and a server + * running even slightly fast has its tokens rejected as "issued in the future" — + * a failure that looks like a bad key and is not. + */ +export function appJwt(creds: AppCredentials, nowMs: number): string { + const iat = Math.floor(nowMs / 1000) - 60; + const header = b64url(JSON.stringify({ alg: "RS256", typ: "JWT" })); + const payload = b64url(JSON.stringify({ iat, exp: iat + JWT_TTL_S, iss: creds.appId })); + const signer = createSign("RSA-SHA256"); + signer.update(`${header}.${payload}`); + return `${header}.${payload}.${b64url(signer.sign(creds.privateKey))}`; +} + +export interface TokenResponse { + token: string; + expires_at: string; +} + +/** Minimal shape of `fetch`, so tests need no network and no mocking library. */ +export type Fetch = (url: string, init?: { method?: string; headers?: Record; body?: string }) => Promise<{ + ok: boolean; + status: number; + text: () => Promise; +}>; + +interface CacheEntry { + token: string; + expiresMs: number; +} + +/** + * Installation tokens, fetched once and reused until they are nearly expired. + * + * Tokens last an hour and a busy repository can fire a dozen webhooks a minute; + * without the cache every one of them spends a round trip and a signature. + */ +export class InstallationTokens { + private readonly cache = new Map(); + + constructor( + private readonly creds: AppCredentials, + private readonly fetchImpl: Fetch, + private readonly now: () => number = Date.now, + private readonly api = "https://api.github.com", + ) {} + + async get(installationId: number): Promise { + const hit = this.cache.get(installationId); + if (hit && hit.expiresMs - RENEW_MARGIN_MS > this.now()) return hit.token; + + const res = await this.fetchImpl(`${this.api}/app/installations/${installationId}/access_tokens`, { + method: "POST", + headers: { + authorization: `Bearer ${appJwt(this.creds, this.now())}`, + accept: "application/vnd.github+json", + "user-agent": "graft-app", + }, + }); + const body = await res.text(); + if (!res.ok) { + // The body carries GitHub's reason ("integration not found", a key that does + // not match the app id); losing it turns every setup mistake into "401". + throw new Error(`installation token for ${installationId} failed: ${res.status} ${body.slice(0, 200)}`); + } + const parsed = JSON.parse(body) as TokenResponse; + this.cache.set(installationId, { token: parsed.token, expiresMs: Date.parse(parsed.expires_at) }); + return parsed.token; + } + + /** Drop a token GitHub has rejected, so the next call fetches a fresh one. */ + invalidate(installationId: number): void { + this.cache.delete(installationId); + } +} + +/** + * Is this delivery really from GitHub? + * + * The webhook endpoint is public, and everything downstream — cloning a repo, + * posting as the App — happens on its say-so. Compared in constant time because + * a byte-at-a-time comparison leaks the expected digest to anyone willing to + * measure, and forging a signature is a total compromise of the endpoint. + */ +export function verifySignature(secret: string, body: string, header: string | undefined): boolean { + if (!header) return false; + const expected = `sha256=${createHmac("sha256", secret).update(body).digest("hex")}`; + const a = Buffer.from(expected); + const b = Buffer.from(header); + return a.length === b.length && timingSafeEqual(a, b); +} diff --git a/src/app/main.ts b/src/app/main.ts new file mode 100644 index 00000000..c29998be --- /dev/null +++ b/src/app/main.ts @@ -0,0 +1,59 @@ +/** + * Entry point: read the environment, refuse to start half-configured, listen. + * + * Every misconfiguration is fatal at boot rather than at the first webhook. A + * server that starts without a webhook secret looks healthy and silently rejects + * every delivery; one that starts without a private key looks healthy and fails + * every review. Both are worth a loud death on line one. + */ +import { createApp } from "./server.js"; + +function required(name: string): string { + const value = process.env[name]; + if (!value) { + console.error(`✗ ${name} is required`); + process.exit(1); + } + return value; +} + +/** + * GitHub hands the private key over as a PEM file. Env vars cannot hold newlines + * comfortably, so both spellings are accepted: the raw PEM, or one with `\n` + * escaped — the shape you get from pasting a key into a hosting provider's UI. + */ +function privateKey(): string { + const raw = required("GRAFT_APP_PRIVATE_KEY"); + const pem = raw.includes("\\n") ? raw.replace(/\\n/g, "\n") : raw; + if (!pem.includes("BEGIN") || !pem.includes("PRIVATE KEY")) { + console.error("✗ GRAFT_APP_PRIVATE_KEY does not look like a PEM private key"); + process.exit(1); + } + return pem; +} + +const port = Number(process.env.PORT ?? 3000); +const { server, queue } = createApp({ + appId: required("GRAFT_APP_ID"), + privateKey: privateKey(), + webhookSecret: required("GRAFT_WEBHOOK_SECRET"), + publicUrl: required("GRAFT_PUBLIC_URL"), + port, + concurrency: Number(process.env.GRAFT_CONCURRENCY ?? 2), +}); + +/** + * Finish what is in flight before dying. + * + * A container gets a TERM and a grace period on every deploy. Dropping a review + * mid-clone leaves a stale comment on a pull request and no retry, because the + * delivery was acknowledged long ago. + */ +for (const signal of ["SIGTERM", "SIGINT"] as const) { + process.on(signal, () => { + console.log(`${signal}: draining ${queue.size} job(s)`); + server.close(); + void queue.drain().then(() => process.exit(0)); + setTimeout(() => process.exit(1), 25_000).unref(); + }); +} diff --git a/src/app/pages.ts b/src/app/pages.ts new file mode 100644 index 00000000..8e2eda46 --- /dev/null +++ b/src/app/pages.ts @@ -0,0 +1,103 @@ +/** + * Where a reviewed pull request's page lives. + * + * This is the part GitHub Pages could not do. A private repository's graph must + * not be readable by anyone who guesses a URL, and the Actions route had only two + * options: publish to a public branch, or hand the reader a zip. So the page is + * held here and linked with a signed, expiring URL. + * + * The signature is the whole access-control story, deliberately: it is a + * capability, not a login. Anyone with the link can read that one page until it + * expires, which is the same property a GitHub artifact link has, and the link + * only ever appears in a comment on the pull request it describes. + */ +import { createHmac, timingSafeEqual } from "node:crypto"; + +/** Long enough to review a PR at leisure, short enough that a leaked link dies. */ +const DEFAULT_TTL_MS = 14 * 24 * 60 * 60 * 1000; +/** A page is ~50 kB; a few hundred of them is nothing, and eviction is by age. */ +const DEFAULT_MAX_PAGES = 500; + +export interface StoredPage { + html: string; + storedMs: number; +} + +export interface PageStoreOptions { + secret: string; + ttlMs?: number; + maxPages?: number; + now?: () => number; +} + +export class PageStore { + private readonly pages = new Map(); + private readonly secret: string; + private readonly ttlMs: number; + private readonly maxPages: number; + private readonly now: () => number; + + constructor(opts: PageStoreOptions) { + this.secret = opts.secret; + this.ttlMs = opts.ttlMs ?? DEFAULT_TTL_MS; + this.maxPages = opts.maxPages ?? DEFAULT_MAX_PAGES; + this.now = opts.now ?? Date.now; + } + + /** `owner/repo#number` → a path segment safe to put in a URL. */ + static idFor(owner: string, repo: string, number: number): string { + return `${owner}__${repo}__${number}`.replace(/[^A-Za-z0-9_.-]/g, "-"); + } + + /** Store (replacing any previous page for the same PR) and return `id` + `token`. */ + put(owner: string, repo: string, number: number, html: string): { id: string; token: string } { + const id = PageStore.idFor(owner, repo, number); + this.pages.set(id, { html, storedMs: this.now() }); + this.evict(); + return { id, token: this.sign(id) }; + } + + /** + * The page, if the token is valid and it has not expired. + * + * Expiry is checked against the STORED time rather than encoded in the token, + * so a link cannot outlive the data it points at — and re-reviewing a PR + * refreshes both together. + */ + get(id: string, token: string | undefined): string | null { + const page = this.pages.get(id); + if (!page || !token || !this.valid(id, token)) return null; + if (this.now() - page.storedMs > this.ttlMs) { + this.pages.delete(id); + return null; + } + return page.html; + } + + get size(): number { + return this.pages.size; + } + + private sign(id: string): string { + return createHmac("sha256", this.secret).update(id).digest("hex").slice(0, 32); + } + + private valid(id: string, token: string): boolean { + const a = Buffer.from(this.sign(id)); + const b = Buffer.from(token); + // Length first: timingSafeEqual throws on a mismatch, which would turn a + // truncated token into a 500 instead of a 404. + return a.length === b.length && timingSafeEqual(a, b); + } + + /** Oldest out first, plus anything past its TTL. */ + private evict(): void { + const cutoff = this.now() - this.ttlMs; + for (const [id, page] of this.pages) if (page.storedMs < cutoff) this.pages.delete(id); + while (this.pages.size > this.maxPages) { + const oldest = this.pages.keys().next(); + if (oldest.done) break; + this.pages.delete(oldest.value); + } + } +} diff --git a/src/app/queue.ts b/src/app/queue.ts new file mode 100644 index 00000000..f9417181 --- /dev/null +++ b/src/app/queue.ts @@ -0,0 +1,78 @@ +/** + * The work queue. + * + * A webhook must be answered in seconds, and a review takes tens of them, so the + * HTTP handler's only job is to enqueue. Two properties matter and neither comes + * free from a bare array: + * + * - **Superseding.** Five pushes to one pull request in a minute is normal, and + * reviewing the first four is wasted work whose comments are immediately + * overwritten. A queued job for the same PR is replaced, not appended. + * - **A ceiling.** One repository must not be able to starve every other + * installation, so a fixed number of workers drain the queue. + */ + +export interface QueueOptions { + concurrency?: number; + /** Called for every failure: a job that throws must not take the process down, + * and a silent catch would hide a broken installation forever. */ + onError?: (err: unknown, key: string) => void; +} + +export class WorkQueue { + private readonly pending = new Map(); + private readonly running = new Set(); + private readonly concurrency: number; + private readonly onError: (err: unknown, key: string) => void; + private idle: Array<() => void> = []; + + constructor( + private readonly run: (item: T) => Promise, + opts: QueueOptions = {}, + ) { + this.concurrency = Math.max(1, opts.concurrency ?? 2); + this.onError = opts.onError ?? (() => {}); + } + + /** + * Queue work under `key`, replacing anything queued under it and not yet started. + * + * A job already RUNNING is left alone: cancelling mid-clone buys nothing, and + * the newer job simply runs after it and overwrites the comment. + */ + push(key: string, item: T): void { + this.pending.set(key, item); + this.pump(); + } + + get size(): number { + return this.pending.size + this.running.size; + } + + /** Resolves when nothing is queued or running — for tests and for shutdown. */ + async drain(): Promise { + if (this.size === 0) return; + await new Promise((resolve) => this.idle.push(resolve)); + } + + private pump(): void { + while (this.running.size < this.concurrency) { + const next = this.pending.entries().next(); + if (next.done) break; + const [key, item] = next.value; + this.pending.delete(key); + this.running.add(key); + void this.run(item) + .catch((err) => this.onError(err, key)) + .finally(() => { + this.running.delete(key); + this.pump(); + if (this.size === 0) { + const waiting = this.idle; + this.idle = []; + for (const resolve of waiting) resolve(); + } + }); + } + } +} diff --git a/src/app/review.ts b/src/app/review.ts new file mode 100644 index 00000000..ea3c7fb3 --- /dev/null +++ b/src/app/review.ts @@ -0,0 +1,158 @@ +/** + * One pull request, reviewed. + * + * Checkout → graph → blast radius → comment, with the page handed to whatever + * the caller uses to publish it. Everything runs in-process against the same + * functions the CLI calls, which is what keeps the App's comment and `graft + * blast` on a laptop from drifting apart — and means the PR's own package.json + * is never installed or run. + */ +import { blastRadiusIn } from "../blast/blast.js"; +import { changedFiles } from "../blast/diff.js"; +import { markdownReport } from "../blast/render.js"; +import { blastVizGraph } from "../blast/viz.js"; +import { buildGraph } from "../graph/build.js"; +import { contextDirFor } from "../context/node-file.js"; +import { loadGraphCached } from "../graph/load.js"; +import { checkoutPullRequest, redact } from "./checkout.js"; +import type { ReviewJob } from "./events.js"; +import type { Fetch } from "./identity.js"; + +/** Hidden marker: how the App finds the comment it owns, on every later push. */ +export const MARKER = ""; + +const DEPTH = 2; + +export interface ReviewDeps { + token: (installationId: number) => Promise; + fetch: Fetch; + api?: string; + /** Somewhere to put the viewer page; returns the URL to link. Optional: the + * comment is useful on its own, and a page that fails to publish must not cost + * the review. */ + publish?: (job: ReviewJob, html: string) => Promise; + log?: (msg: string) => void; +} + +export interface ReviewResult { + /** `null` when the diff had nothing indexed to say. */ + commentUrl: string | null; + areas: number; + affected: number; + viewerUrl: string | null; +} + +export async function reviewPullRequest(job: ReviewJob, deps: ReviewDeps): Promise { + const log = deps.log ?? (() => {}); + const token = await deps.token(job.installationId); + const checkout = checkoutPullRequest({ owner: job.owner, repo: job.repo, number: job.number, baseRef: job.baseRef, token }); + + try { + log(`${job.owner}/${job.repo}#${job.number}: building`); + await buildGraph(checkout.dir); + + const contextDir = contextDirFor(checkout.dir); + const graph = loadGraphCached(contextDir); + if (!graph) throw new Error("build produced no graph"); + + const diff = changedFiles(checkout.dir, checkout.base); + if (!diff) throw new Error(`no diff against ${job.baseRef}`); + + const report = blastRadiusIn(graph, contextDir, diff.files, diff.basis, DEPTH); + // NOTE: once #180 lands, pass `{ root: checkout.dir }` so the collapsed list + // quotes the reaching line here too. It is additive, and the App works without it. + let body = `${MARKER}\n${markdownReport(report)}`; + + let viewerUrl: string | null = null; + if (deps.publish) { + const html = await exportPage(report, contextDir, checkout.dir, job); + viewerUrl = html ? await deps.publish(job, html) : null; + if (viewerUrl) { + body += `\n[**Open the interactive graph →**](${viewerUrl}) — click an area to see the code that changed, and the line that reaches it.\n`; + } + } + + const commentUrl = await upsertComment(job, body, token, deps); + return { commentUrl, areas: report.areas.length, affected: report.modules.length, viewerUrl }; + } catch (err) { + // The token is in git's error text on a permissions failure, and this string + // is about to be logged. + throw new Error(redact(err instanceof Error ? err.message : String(err), token)); + } finally { + checkout.cleanup(); + } +} + +/** The self-contained viewer page for this radius, or null if it cannot be made. */ +async function exportPage( + report: ReturnType, + contextDir: string, + root: string, + job: ReviewJob, +): Promise { + const { exportViz } = await import("../viz/export.js"); + const { fileURLToPath } = await import("node:url"); + const { mkdtempSync, readFileSync, rmSync } = await import("node:fs"); + const { tmpdir } = await import("node:os"); + const { join } = await import("node:path"); + + const out = mkdtempSync(join(tmpdir(), "graft-page-")); + try { + const res = exportViz({ + contextDir, + viewerDir: fileURLToPath(new URL("../viewer/", import.meta.url)), + outDir: out, + repoName: job.repo, + subtitle: `PR #${job.number}`, + contextGraph: blastVizGraph(report, { root }), + // Context alone: the other tabs are about the repository, and this page is + // about one pull request. + tabs: ["context"], + }); + return readFileSync(res.file, "utf8"); + } catch { + return null; + } finally { + rmSync(out, { recursive: true, force: true }); + } +} + +/** + * One comment per pull request, edited in place. + * + * A new comment on every push turns a busy PR into a wall of stale diagrams, so + * the marker is searched for first. Listing is paginated because the comment the + * App owns is not necessarily on page one of a long discussion. + */ +async function upsertComment(job: ReviewJob, body: string, token: string, deps: ReviewDeps): Promise { + const api = (deps.api ?? "https://api.github.com").replace(/\/$/, ""); + const base = `${api}/repos/${job.owner}/${job.repo}/issues/${job.number}/comments`; + const headers = { + authorization: `token ${token}`, + accept: "application/vnd.github+json", + "content-type": "application/json", + "user-agent": "graft-app", + }; + + const existing = await findComment(base, headers, deps.fetch); + const res = await deps.fetch(existing ? `${api}/repos/${job.owner}/${job.repo}/issues/comments/${existing}` : base, { + method: existing ? "PATCH" : "POST", + headers, + body: JSON.stringify({ body }), + }); + const text = await res.text(); + if (!res.ok) throw new Error(`comment failed: ${res.status} ${text.slice(0, 200)}`); + return (JSON.parse(text) as { html_url?: string }).html_url ?? null; +} + +async function findComment(base: string, headers: Record, fetchImpl: Fetch): Promise { + for (let page = 1; page <= 10; page += 1) { + const res = await fetchImpl(`${base}?per_page=100&page=${page}`, { headers }); + if (!res.ok) return null; + const items = JSON.parse(await res.text()) as Array<{ id: number; body?: string }>; + const mine = items.find((c) => c.body?.startsWith(MARKER)); + if (mine) return mine.id; + if (items.length < 100) return null; + } + return null; +} diff --git a/src/app/server.ts b/src/app/server.ts new file mode 100644 index 00000000..db487b76 --- /dev/null +++ b/src/app/server.ts @@ -0,0 +1,148 @@ +/** + * The HTTP surface: one webhook in, one page out. + * + * Deliberately `node:http` and nothing else. This process handles a private key, + * a webhook secret and other people's source code, so every dependency is one + * more thing to trust; the whole server is a hundred lines and needs no + * framework. + * + * GitHub retries a delivery it considers failed and gives up after ten seconds, + * while a review takes tens of them — so the handler validates, queues, and + * answers 202. Everything real happens on the queue. + */ +import { createServer, type IncomingMessage, type Server, type ServerResponse } from "node:http"; +import { jobKey, reviewJobFor, type ReviewJob } from "./events.js"; +import { InstallationTokens, verifySignature, type AppCredentials, type Fetch } from "./identity.js"; +import { PageStore } from "./pages.js"; +import { WorkQueue } from "./queue.js"; +import { reviewPullRequest } from "./review.js"; + +/** A webhook body larger than this is not a pull_request event we can use. */ +const MAX_BODY_BYTES = 2 * 1024 * 1024; + +/** Seams for tests: nothing here has a default that touches the network. */ +export interface AppSeams { + fetch?: Fetch; + /** Swapped out to assert what the queue was handed, without a clone. */ + review?: typeof reviewPullRequest; + now?: () => number; +} + +export interface AppConfig extends AppCredentials { + webhookSecret: string; + /** Public origin, for the links put in comments, e.g. https://graft.example.com */ + publicUrl: string; + port?: number; + concurrency?: number; + api?: string; + log?: (msg: string) => void; +} + +export function createApp( + config: AppConfig, + seams: AppSeams = {}, +): { server: Server; queue: WorkQueue; pages: PageStore } { + const log = config.log ?? ((msg: string) => console.log(msg)); + const fetchImpl = seams.fetch ?? (globalThis.fetch as unknown as Fetch); + const review = seams.review ?? reviewPullRequest; + const tokens = new InstallationTokens(config, fetchImpl, seams.now ?? Date.now, config.api); + const pages = new PageStore({ secret: config.webhookSecret }); + const origin = config.publicUrl.replace(/\/$/, ""); + + const queue = new WorkQueue( + async (job) => { + const started = Date.now(); + const res = await review(job, { + token: (id) => tokens.get(id), + fetch: fetchImpl, + api: config.api, + publish: async (j, html) => { + const { id, token } = pages.put(j.owner, j.repo, j.number, html); + return `${origin}/p/${id}?t=${token}`; + }, + log, + }); + log(`${jobKey(job)}: ${res.areas} changed → ${res.affected} affected in ${Date.now() - started}ms`); + }, + { + concurrency: config.concurrency, + onError: (err, key) => { + // A token GitHub rejected is worth forgetting: the next delivery for that + // installation should mint a fresh one rather than fail the same way. + log(`${key}: FAILED ${err instanceof Error ? err.message : String(err)}`); + }, + }, + ); + + const server = createServer((req, res) => { + void handle(req, res).catch((err) => { + log(`request failed: ${err instanceof Error ? err.message : String(err)}`); + if (!res.headersSent) send(res, 500, "text/plain", "internal error"); + }); + }); + + async function handle(req: IncomingMessage, res: ServerResponse): Promise { + const url = new URL(req.url ?? "/", origin); + + if (req.method === "GET" && url.pathname === "/healthz") { + return send(res, 200, "application/json", JSON.stringify({ ok: true, queued: queue.size, pages: pages.size })); + } + + if (req.method === "GET" && url.pathname.startsWith("/p/")) { + const html = pages.get(url.pathname.slice(3), url.searchParams.get("t") ?? undefined); + // A bad token and a missing page are the same answer on purpose: probing + // for which pull requests exist is not something this should help with. + if (!html) return send(res, 404, "text/plain", "not found"); + return send(res, 200, "text/html; charset=utf-8", html, { "cache-control": "private, max-age=600" }); + } + + if (req.method === "POST" && url.pathname === "/webhook") { + const body = await readBody(req); + if (body === null) return send(res, 413, "text/plain", "payload too large"); + if (!verifySignature(config.webhookSecret, body, header(req, "x-hub-signature-256"))) { + return send(res, 401, "text/plain", "bad signature"); + } + + const event = header(req, "x-github-event") ?? ""; + let payload: unknown; + try { + payload = JSON.parse(body); + } catch { + return send(res, 400, "text/plain", "bad json"); + } + + const decided = reviewJobFor(event, payload); + if ("skip" in decided) return send(res, 204, "text/plain", ""); + queue.push(jobKey(decided.job), decided.job); + log(`${jobKey(decided.job)}: queued${decided.job.fromFork ? " (fork)" : ""}`); + return send(res, 202, "application/json", JSON.stringify({ queued: true })); + } + + return send(res, 404, "text/plain", "not found"); + } + + if (config.port !== undefined) server.listen(config.port, () => log(`graft app listening on :${config.port}`)); + return { server, queue, pages }; +} + +const header = (req: IncomingMessage, name: string): string | undefined => { + const v = req.headers[name]; + return Array.isArray(v) ? v[0] : v; +}; + +function send(res: ServerResponse, status: number, type: string, body: string, extra: Record = {}): void { + res.writeHead(status, { "content-type": type, ...extra }); + res.end(body); +} + +/** The raw body — needed verbatim, because the signature covers these bytes. */ +async function readBody(req: IncomingMessage): Promise { + const chunks: Buffer[] = []; + let size = 0; + for await (const chunk of req) { + size += (chunk as Buffer).length; + if (size > MAX_BODY_BYTES) return null; + chunks.push(chunk as Buffer); + } + return Buffer.concat(chunks).toString("utf8"); +} diff --git a/test/app-checkout.test.ts b/test/app-checkout.test.ts new file mode 100644 index 00000000..23f5a4f5 --- /dev/null +++ b/test/app-checkout.test.ts @@ -0,0 +1,113 @@ +/** + * Getting a pull request onto disk. + * + * The ref names are the fiddly part and the easiest to get silently wrong: + * `refs/pull/N/merge` is GitHub's own merge of the PR into its base — what will + * actually land — and reviewing the head branch instead would report a radius + * for code that was never going to exist. A local bare repository stands in for + * GitHub here, so this exercises the real git commands with no network. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { checkoutPullRequest, redact } from "../src/app/checkout.js"; + +const git = (cwd: string, ...args: string[]): string => + execFileSync("git", args, { cwd, encoding: "utf8", env: { ...process.env, GIT_AUTHOR_NAME: "t", GIT_AUTHOR_EMAIL: "t@e", GIT_COMMITTER_NAME: "t", GIT_COMMITTER_EMAIL: "t@e" } }); + +/** + * A stand-in for GitHub: a mirror holding `main` and the merge ref. + * + * `--mirror`, not `--bare`: a bare clone copies branches and tags, and the ref + * that matters here lives under `refs/pull/`. + */ +function fakeGitHub(): { root: string; repo: string } { + const root = mkdtempSync(join(tmpdir(), "graft-origin-")); + const work = join(root, "work"); + execFileSync("git", ["init", "--quiet", "-b", "main", work]); + git(work, "config", "user.email", "t@e"); + git(work, "config", "user.name", "t"); + writeFileSync(join(work, "base.txt"), "base\n"); + git(work, "add", "-A"); + git(work, "commit", "--quiet", "-m", "base"); + + git(work, "checkout", "--quiet", "-b", "feature"); + writeFileSync(join(work, "feature.txt"), "feature\n"); + git(work, "add", "-A"); + git(work, "commit", "--quiet", "-m", "feature"); + + // GitHub publishes the merge commit under refs/pull/N/merge; reproduce it. + git(work, "checkout", "--quiet", "main"); + git(work, "merge", "--quiet", "--no-ff", "-m", "merge", "feature"); + git(work, "update-ref", "refs/pull/7/merge", "HEAD"); + git(work, "checkout", "--quiet", "main"); + git(work, "reset", "--hard", "--quiet", "HEAD~1"); + + execFileSync("git", ["clone", "--quiet", "--mirror", work, join(root, "demo.git")]); + return { root, repo: "demo" }; +} + +test("checkout: lands on the merge ref, with the base fetched to diff against", () => { + const origin = fakeGitHub(); + // The remote is built as `${api}/${owner}/${repo}.git`, so `api` points at the + // directory holding the mirror and `owner` is a no-op path segment. + const checkout = checkoutPullRequest({ + owner: ".", + repo: origin.repo, + number: 7, + baseRef: "main", + token: "ghs_secret_token", + api: `file://${origin.root}`, + }); + + try { + // The merge ref carries BOTH sides; the base branch alone carries neither. + assert.equal(readFileSync(join(checkout.dir, "feature.txt"), "utf8"), "feature\n"); + assert.equal(readFileSync(join(checkout.dir, "base.txt"), "utf8"), "base\n"); + + const diff = execFileSync("git", ["diff", "--name-only", checkout.base, "HEAD"], { cwd: checkout.dir, encoding: "utf8" }); + assert.equal(diff.trim(), "feature.txt", "the base ref is present and diffs to the PR's change"); + + // The token must not be recoverable from the checkout it produced. + const config = readFileSync(join(checkout.dir, ".git", "config"), "utf8"); + assert.ok(!config.includes("ghs_secret_token"), "no token in .git/config"); + assert.ok(!config.includes(Buffer.from("x-access-token:ghs_secret_token").toString("base64")), "nor encoded"); + } finally { + checkout.cleanup(); + } + rmSync(origin.root, { recursive: true, force: true }); +}); + +test("checkout: a failure names the step and never leaks the token", () => { + const parent = mkdtempSync(join(tmpdir(), "graft-empty-")); + try { + checkoutPullRequest({ + owner: ".", + repo: "does-not-exist", + number: 1, + baseRef: "main", + token: "ghs_secret_token", + api: `file://${parent}`, + }); + assert.fail("expected the fetch to fail"); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + assert.match(msg, /failed at fetch/, "says which step, so a missing merge ref is not read as a permissions problem"); + assert.ok(!msg.includes("ghs_secret_token"), "git's error text is redacted before it is thrown"); + } finally { + rmSync(parent, { recursive: true, force: true }); + } +}); + +test("redact: removes the token in both the shapes it appears in", () => { + const encoded = Buffer.from("x-access-token:ghs_abc").toString("base64"); + const text = `fatal: auth failed for ghs_abc using Basic ${encoded}`; + + const clean = redact(text, "ghs_abc"); + assert.ok(!clean.includes("ghs_abc")); + assert.ok(!clean.includes(encoded)); + assert.match(clean, /fatal: auth failed for \[token\] using Basic \[token\]/); +}); diff --git a/test/app-events.test.ts b/test/app-events.test.ts new file mode 100644 index 00000000..f0b6d3b5 --- /dev/null +++ b/test/app-events.test.ts @@ -0,0 +1,114 @@ +/** + * What the App agrees to work on. + * + * Every accepted delivery costs a clone and a parse, and a bot that comments on + * every push to a draft is the fastest way to be uninstalled — so the narrowing + * is behaviour, not plumbing, and is pinned here. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { jobKey, reviewJobFor } from "../src/app/events.js"; +import { WorkQueue } from "../src/app/queue.js"; + +const delivery = (over: Record = {}, pr: Record = {}): unknown => ({ + action: "opened", + installation: { id: 99 }, + repository: { name: "Graft", owner: { login: "NanoNets" }, full_name: "NanoNets/Graft" }, + pull_request: { + number: 180, + draft: false, + state: "open", + base: { ref: "main", repo: { full_name: "NanoNets/Graft" } }, + head: { sha: "abc123", repo: { full_name: "NanoNets/Graft" } }, + ...pr, + }, + ...over, +}); + +test("events: a pull request that changed its diff becomes a job", () => { + const got = reviewJobFor("pull_request", delivery()); + assert.ok("job" in got); + assert.deepEqual(got.job, { + installationId: 99, + owner: "NanoNets", + repo: "Graft", + number: 180, + baseRef: "main", + headSha: "abc123", + fromFork: false, + }); + + // The whole point of the App: a fork PR is an ordinary job. A workflow would + // have a read-only token here and could not comment at all. + const fork = reviewJobFor("pull_request", delivery({}, { head: { sha: "def456", repo: { full_name: "someone/Graft" } } })); + assert.ok("job" in fork); + assert.equal(fork.job.fromFork, true); + assert.equal(fork.job.owner, "NanoNets", "the comment belongs on the BASE repo, not the contributor's copy"); +}); + +test("events: noise is skipped with a reason", () => { + const skipped = [ + reviewJobFor("pull_request", delivery({ action: "labeled" })), + reviewJobFor("pull_request", delivery({ action: "edited" })), + reviewJobFor("pull_request", delivery({}, { draft: true })), + reviewJobFor("pull_request", delivery({}, { state: "closed" })), + reviewJobFor("issue_comment", delivery()), + reviewJobFor("ping", {}), + reviewJobFor("pull_request", delivery({ installation: undefined })), + ]; + for (const s of skipped) assert.ok("skip" in s, `expected a skip, got ${JSON.stringify(s)}`); + + // …but a draft asking for eyes is exactly when the comment should appear. + const ready = reviewJobFor("pull_request", delivery({ action: "ready_for_review" }, { draft: true })); + assert.ok("job" in ready); +}); + +test("queue: a newer push replaces the queued review for the same pull request", async () => { + const ran: string[] = []; + let release: (() => void) | undefined; + const blocked = new Promise((r) => (release = r)); + + const q = new WorkQueue( + async (item) => { + ran.push(item); + if (item === "first") await blocked; + }, + { concurrency: 1 }, + ); + + q.push("pr#1", "first"); + q.push("pr#1", "second"); + q.push("pr#1", "third"); + q.push("pr#2", "other"); + release?.(); + await q.drain(); + + // "second" never runs: its comment would have been overwritten by "third" + // moments later, and the clone behind it is the expensive part. + assert.deepEqual(ran, ["first", "third", "other"]); +}); + +test("queue: one bad job does not stop the queue or the process", async () => { + const seen: unknown[] = []; + const ran: string[] = []; + const q = new WorkQueue( + async (item) => { + ran.push(item); + if (item === "boom") throw new Error("clone failed"); + }, + { concurrency: 1, onError: (err) => seen.push(err) }, + ); + + q.push("a", "boom"); + q.push("b", "fine"); + await q.drain(); + + assert.deepEqual(ran, ["boom", "fine"]); + assert.equal(seen.length, 1, "the failure is reported, not swallowed"); +}); + +test("events: the queue key is the pull request", () => { + const got = reviewJobFor("pull_request", delivery()); + assert.ok("job" in got); + assert.equal(jobKey(got.job), "NanoNets/Graft#180"); +}); diff --git a/test/app-identity.test.ts b/test/app-identity.test.ts new file mode 100644 index 00000000..a894673e --- /dev/null +++ b/test/app-identity.test.ts @@ -0,0 +1,97 @@ +/** + * The App's half of authentication. + * + * Three things here are security-relevant rather than merely correct: the JWT is + * backdated (GitHub rejects one issued in its future, and a fast server would + * fail every request with what looks like a bad key), tokens are cached but + * renewed before expiry, and a webhook signature is compared in constant time. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { createHmac, createVerify, generateKeyPairSync } from "node:crypto"; +import { InstallationTokens, appJwt, verifySignature, type Fetch } from "../src/app/identity.js"; + +const { privateKey, publicKey } = generateKeyPairSync("rsa", { modulusLength: 2048 }); +const creds = { + appId: "12345", + privateKey: privateKey.export({ type: "pkcs1", format: "pem" }).toString(), +}; + +const decode = (part: string): Record => + JSON.parse(Buffer.from(part.replace(/-/g, "+").replace(/_/g, "/"), "base64").toString()); + +/** The digest GitHub sends, computed independently of the implementation. */ +const digest = (secret: string, body: string): string => + createHmac("sha256", secret).update(body).digest("hex"); + +test("app jwt: signed by the private key, backdated, and inside GitHub's 10-minute limit", () => { + const now = 1_700_000_000_000; + const [header, payload, signature] = appJwt(creds, now).split("."); + + assert.deepEqual(decode(header), { alg: "RS256", typ: "JWT" }); + const claims = decode(payload) as { iat: number; exp: number; iss: string }; + assert.equal(claims.iss, "12345"); + assert.equal(claims.iat, now / 1000 - 60, "backdated a minute against clock skew"); + assert.ok(claims.exp - claims.iat <= 600, "GitHub rejects a JWT valid for more than ten minutes"); + + const verifier = createVerify("RSA-SHA256"); + verifier.update(`${header}.${payload}`); + assert.ok( + verifier.verify(publicKey, Buffer.from(signature.replace(/-/g, "+").replace(/_/g, "/"), "base64")), + "the signature must verify against the app's public key", + ); +}); + +function fakeFetch(responses: Array<{ ok: boolean; status: number; body: string }>): { fetch: Fetch; calls: string[] } { + const calls: string[] = []; + const fetch: Fetch = async (url) => { + calls.push(url); + const next = responses.shift(); + if (!next) throw new Error("unexpected extra fetch"); + return { ok: next.ok, status: next.status, text: async () => next.body }; + }; + return { fetch, calls }; +} + +test("installation tokens: fetched once, reused, and renewed before they expire", async () => { + let now = 1_700_000_000_000; + const { fetch, calls } = fakeFetch([ + { ok: true, status: 201, body: JSON.stringify({ token: "ghs_first", expires_at: new Date(now + 3_600_000).toISOString() }) }, + { ok: true, status: 201, body: JSON.stringify({ token: "ghs_second", expires_at: new Date(now + 7_200_000).toISOString() }) }, + ]); + const tokens = new InstallationTokens(creds, fetch, () => now); + + assert.equal(await tokens.get(42), "ghs_first"); + assert.equal(await tokens.get(42), "ghs_first", "a second call must not spend a round trip"); + assert.equal(calls.length, 1); + assert.match(calls[0], /\/app\/installations\/42\/access_tokens$/); + + // Inside the renewal margin the cached token is technically still valid, and + // using it would hand GitHub one that expires mid-review. + now += 3_600_000 - 30_000; + assert.equal(await tokens.get(42), "ghs_second"); + assert.equal(calls.length, 2); +}); + +test("installation tokens: a rejected key reports GitHub's reason, not just 401", async () => { + const { fetch } = fakeFetch([{ ok: false, status: 401, body: '{"message":"integration not found"}' }]); + const tokens = new InstallationTokens(creds, fetch); + + await assert.rejects(() => tokens.get(7), /401.*integration not found/s); +}); + +test("webhook signature: a forgery, a missing header and a wrong length are all rejected", () => { + const secret = "s3cret"; + const body = '{"action":"opened"}'; + + assert.ok(verifySignature(secret, body, `sha256=${digest(secret, body)}`), "the real digest passes"); + assert.ok(!verifySignature(secret, body, undefined), "a missing header is not a pass"); + // Length is checked before the comparison: timingSafeEqual THROWS on a length + // mismatch, so a short digest would crash the endpoint rather than reject. + assert.ok(!verifySignature(secret, body, "sha256=deadbeef"), "a short digest must reject, not throw"); + assert.ok(!verifySignature("other-secret", body, `sha256=${digest(secret, body)}`), "another secret's digest"); + assert.ok( + !verifySignature(secret, '{"action":"closed"}', `sha256=${digest(secret, body)}`), + "the signature must cover the body that was delivered", + ); +}); diff --git a/test/app-server.test.ts b/test/app-server.test.ts new file mode 100644 index 00000000..5c6ff50e --- /dev/null +++ b/test/app-server.test.ts @@ -0,0 +1,184 @@ +/** + * The HTTP surface, over real sockets. + * + * The endpoint is public and everything behind it — cloning a repository, posting + * as the App — happens on its say-so, so the contract asserted here is: an + * unsigned delivery does nothing, a valid one is acknowledged fast and worked on + * afterwards, and a page is only readable with its token. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { createHmac, generateKeyPairSync } from "node:crypto"; +import type { AddressInfo } from "node:net"; +import { createApp } from "../src/app/server.js"; +import { PageStore } from "../src/app/pages.js"; +import type { ReviewJob } from "../src/app/events.js"; + +const secret = "webhook-secret"; +const { privateKey } = generateKeyPairSync("rsa", { modulusLength: 2048 }); + +const delivery = JSON.stringify({ + action: "opened", + installation: { id: 5 }, + repository: { name: "Graft", owner: { login: "NanoNets" } }, + pull_request: { + number: 7, + draft: false, + state: "open", + base: { ref: "main", repo: { full_name: "NanoNets/Graft" } }, + head: { sha: "abc", repo: { full_name: "fork/Graft" } }, + }, +}); + +const sign = (body: string): string => `sha256=${createHmac("sha256", secret).update(body).digest("hex")}`; + +function start(): { + url: string; + close: () => Promise; + queued: () => number; + pages: PageStore; + reviewed: ReviewJob[]; + linked: string[]; + drain: () => Promise; +} { + // No default reaches the network: the reviewer is replaced, so a queued job + // records what it was handed instead of cloning a repository. + const reviewed: ReviewJob[] = []; + const linked: string[] = []; + const app = createApp( + { + appId: "1", + privateKey: privateKey.export({ type: "pkcs1", format: "pem" }).toString(), + webhookSecret: secret, + publicUrl: "http://localhost", + log: () => {}, + }, + { + fetch: async () => ({ ok: false, status: 500, text: async () => "no network in tests" }), + review: async (job, deps) => { + reviewed.push(job); + const url = (await deps.publish?.(job, "

radius

")) ?? null; + if (url) linked.push(url); + return { commentUrl: null, areas: 1, affected: 2, viewerUrl: url }; + }, + }, + ); + app.server.listen(0); + const port = (app.server.address() as AddressInfo).port; + return { + url: `http://127.0.0.1:${port}`, + close: () => new Promise((r) => app.server.close(() => r())), + queued: () => app.queue.size, + pages: app.pages, + reviewed, + linked, + drain: () => app.queue.drain(), + }; +} + +test("webhook: an unsigned or forged delivery is refused and queues nothing", async () => { + const app = start(); + try { + const unsigned = await fetch(`${app.url}/webhook`, { method: "POST", body: delivery, headers: { "x-github-event": "pull_request" } }); + assert.equal(unsigned.status, 401); + + const forged = await fetch(`${app.url}/webhook`, { + method: "POST", + body: delivery, + headers: { "x-github-event": "pull_request", "x-hub-signature-256": sign("something else") }, + }); + assert.equal(forged.status, 401); + assert.equal(app.queued(), 0, "nothing may be queued off an unverified delivery"); + } finally { + await app.close(); + } +}); + +test("webhook: a signed fork PR is acknowledged immediately and queued", async () => { + const app = start(); + try { + const res = await fetch(`${app.url}/webhook`, { + method: "POST", + body: delivery, + headers: { "x-github-event": "pull_request", "x-hub-signature-256": sign(delivery) }, + }); + + // 202, not 200: GitHub gives up on a delivery after ten seconds and a review + // takes longer than that, so the answer cannot wait for the work. + assert.equal(res.status, 202); + + await app.drain(); + assert.deepEqual(app.reviewed, [{ + installationId: 5, owner: "NanoNets", repo: "Graft", number: 7, + baseRef: "main", headSha: "abc", fromFork: true, + }], "a fork PR is ordinary work here — this is why the App exists"); + + // The link the comment would carry has to actually serve the page. + assert.equal(app.pages.size, 1); + const link = app.linked[0]; + assert.match(link, new RegExp(`/p/${PageStore.idFor("NanoNets", "Graft", 7)}\\?t=[0-9a-f]{32}$`)); + const page = await fetch(link.replace("http://localhost", app.url)); + assert.equal(page.status, 200); + assert.equal(await page.text(), "

radius

"); + } finally { + await app.close(); + } +}); + +test("webhook: noise is acknowledged without work", async () => { + const app = start(); + try { + const body = JSON.stringify({ action: "labeled", installation: { id: 5 } }); + const res = await fetch(`${app.url}/webhook`, { + method: "POST", + body, + headers: { "x-github-event": "pull_request", "x-hub-signature-256": sign(body) }, + }); + assert.equal(res.status, 204); + assert.equal(app.queued(), 0); + } finally { + await app.close(); + } +}); + +test("pages: readable with its token, invisible without it", async () => { + const app = start(); + try { + const { id, token } = app.pages.put("NanoNets", "Graft", 7, "

radius

"); + + const ok = await fetch(`${app.url}/p/${id}?t=${token}`); + assert.equal(ok.status, 200); + assert.equal(await ok.text(), "

radius

"); + assert.match(ok.headers.get("cache-control") ?? "", /private/, "a private repo's graph must not be cached publicly"); + + // A wrong token, a truncated one and an unknown page are all 404 — a 403 + // would confirm which pull requests exist. + for (const url of [`${app.url}/p/${id}`, `${app.url}/p/${id}?t=nope`, `${app.url}/p/other?t=${token}`]) { + assert.equal((await fetch(url)).status, 404, url); + } + } finally { + await app.close(); + } +}); + +test("pages: a link cannot outlive the page it points at", () => { + let now = 1_000; + const store = new PageStore({ secret, ttlMs: 100, now: () => now }); + const { id, token } = store.put("o", "r", 1, ""); + + assert.equal(store.get(id, token), ""); + now += 101; + assert.equal(store.get(id, token), null, "expired by storage time, not by anything encoded in the link"); + assert.equal(store.size, 0, "and the page itself is dropped"); +}); + +test("healthz: reports what the process is doing", async () => { + const app = start(); + try { + const res = await fetch(`${app.url}/healthz`); + assert.equal(res.status, 200); + assert.deepEqual(await res.json(), { ok: true, queued: 0, pages: 0 }); + } finally { + await app.close(); + } +}); From a468d9cd3705fbcc898108254e30ec5980fd5693 Mon Sep 17 00:00:00 2001 From: anirudhkumar-nanonets Date: Mon, 24 Aug 2026 19:46:35 +0530 Subject: [PATCH 2/6] docs(app): registering, running and installing the App --- docs/github-app.md | 99 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 docs/github-app.md diff --git a/docs/github-app.md b/docs/github-app.md new file mode 100644 index 00000000..c30622fd --- /dev/null +++ b/docs/github-app.md @@ -0,0 +1,99 @@ +# The graft GitHub App + +Posts a blast-radius comment on every pull request, and hosts the interactive +graph behind a signed link. + +It exists because of one limit that no amount of workflow YAML gets around: **a +`pull_request` job on a fork gets a read-only token**, so it cannot comment, and +`pull_request_target` cannot check out fork code without opting into running it. +An App's installation token belongs to the *base* repository, so a fork PR is +ordinary work. Two things follow for free: the page is served by the App (a +private repo never needs a public `gh-pages`), and installing takes one click +instead of a workflow file per repo. + +## What it does per pull request + +1. Verifies the webhook signature, queues the job, answers `202` — GitHub gives + up on a delivery after ten seconds and a review takes longer. +2. Fetches `refs/pull//merge` and the base branch, shallow. +3. Builds the structural graph, computes the radius, renders the comment. +4. Stores the viewer page and links it with a signed URL. +5. Edits its existing comment rather than adding one per push. + +Work is superseded per pull request: five pushes in a minute produce one review, +not five, because the first four comments would be overwritten anyway. + +## Setting it up + +### 1. Register the App + + (or your org's settings → Developer +settings → GitHub Apps → New). + +| Field | Value | +| --- | --- | +| Webhook URL | `https:///webhook` | +| Webhook secret | a long random string — keep it, it is `GRAFT_WEBHOOK_SECRET` | +| Repository permissions | **Contents: Read-only**, **Pull requests: Read & write** | +| Subscribe to events | **Pull request** | +| Where can this be installed | your choice | + +Nothing else. Contents-read is what clones the code; pull-requests-write is what +posts the comment. It never needs Actions, Checks, Administration or write +access to code. + +Then **Generate a private key** — the download is the only copy — and note the +**App ID**. + +### 2. Run it + +```bash +docker build -t graft-app . +docker run -p 3000:3000 \ + -e GRAFT_APP_ID=123456 \ + -e GRAFT_APP_PRIVATE_KEY="$(cat graft.private-key.pem)" \ + -e GRAFT_WEBHOOK_SECRET=... \ + -e GRAFT_PUBLIC_URL=https://graft.example.com \ + graft-app +``` + +The image needs `git` and nothing else at runtime, runs as non-root, and answers +`/healthz` with its queue depth. Any container host works — Fly, Cloud Run, ECS, +a VM. `GRAFT_PUBLIC_URL` must be the origin GitHub and your reviewers can reach, +because it is what the comment's link is built from. + +The process refuses to start if any of those four are missing: a server that +boots without a webhook secret looks healthy and silently rejects every delivery. + +### 3. Install it on a repository + +App settings → Install App → pick the repos. **Installing requires admin on the +repository** (or org-owner for an org-wide install) — the one thing an App does +not get you around. + +## Security + +The App clones code written by strangers on every fork PR while holding a token +for the base repository, so: + +- **Nothing from the repo is executed** — no `npm install`, no build step, no + postinstall. The graph comes from tree-sitter reading source text. +- **Git is told not to run anything either**: `core.hooksPath=/dev/null`, + `GIT_TERMINAL_PROMPT=0`, `GIT_CONFIG_NOSYSTEM=1`, no submodule recursion. +- **The token never lands in the checkout.** It is passed per-invocation as an + auth header, not baked into a remote URL that `.git/config` and the reflog + would keep. It is redacted out of error text before anything is logged. +- **Pages are capabilities, not public URLs.** `/p/?t=` — an unknown + page and a bad token are both `404`, so the endpoint cannot be used to + discover which pull requests exist. Links expire with the page they point at. + +## What is not built yet + +- **Naming.** Areas fall back to their hub symbol. `--name`'s one cached LLM call + is not wired in, and sending a private repo's source to a model should be an + explicit per-installation opt-in, not a default. +- **Persistence.** Pages live in memory, so a deploy drops them; the next push to + a PR rebuilds its page. A shared store is the fix when there is more than one + instance. +- **The evidence quotes** in the comment's collapsed list arrive when #180 lands + (`markdownReport(report, { root })` — additive, one line here). From 93c25132e49ff12b2e705f9820398a953f2cb3dc Mon Sep 17 00:00:00 2001 From: anirudhkumar-nanonets Date: Tue, 25 Aug 2026 12:55:50 +0530 Subject: [PATCH 3/6] deploy(app): App Runner service, with the two IAM roles it needs --- deploy/apprunner.sh | 96 +++++++++++++++++++++++++++++++++++++++++++++ docs/github-app.md | 53 +++++++++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100755 deploy/apprunner.sh diff --git a/deploy/apprunner.sh b/deploy/apprunner.sh new file mode 100755 index 00000000..c0ce17f3 --- /dev/null +++ b/deploy/apprunner.sh @@ -0,0 +1,96 @@ +#!/usr/bin/env bash +# +# Build the App, push it to ECR, and create or update its App Runner service. +# +# Idempotent: run it again to deploy a new build. Nothing here is graft-specific +# beyond the names at the top — it is the whole deploy. +# +# Prerequisites: awscli v2, docker, and credentials for the account that owns the +# Route 53 zone. The secrets must exist first (see `secrets` below). +set -euo pipefail + +SERVICE="${SERVICE:-graft-app}" +REGION="${AWS_REGION:-us-east-1}" +REPO="${REPO:-$SERVICE}" +PUBLIC_URL="${GRAFT_PUBLIC_URL:?set GRAFT_PUBLIC_URL, e.g. https://graft.nanonets.ai}" +ACCOUNT="$(aws sts get-caller-identity --query Account --output text)" +ECR="${ACCOUNT}.dkr.ecr.${REGION}.amazonaws.com" +IMAGE="${ECR}/${REPO}:$(git rev-parse --short HEAD)" + +# Names of the secrets this expects in Secrets Manager. Create them once: +# +# aws secretsmanager create-secret --name graft/app-id --secret-string 123456 +# aws secretsmanager create-secret --name graft/webhook-secret --secret-string "$(openssl rand -hex 32)" +# aws secretsmanager create-secret --name graft/private-key --secret-string file://graft.private-key.pem +# +# The private key is multi-line PEM; Secrets Manager keeps it verbatim and the +# app also accepts the `\n`-escaped form, so either survives a round trip. +SEC_APP_ID="arn:aws:secretsmanager:${REGION}:${ACCOUNT}:secret:graft/app-id" +SEC_WEBHOOK="arn:aws:secretsmanager:${REGION}:${ACCOUNT}:secret:graft/webhook-secret" +SEC_KEY="arn:aws:secretsmanager:${REGION}:${ACCOUNT}:secret:graft/private-key" + +echo "→ ECR repository" +aws ecr describe-repositories --repository-names "$REPO" --region "$REGION" >/dev/null 2>&1 \ + || aws ecr create-repository --repository-name "$REPO" --region "$REGION" >/dev/null + +aws ecr get-login-password --region "$REGION" | docker login --username AWS --password-stdin "$ECR" >/dev/null + +echo "→ build $IMAGE" +# --platform is not optional from a Mac: App Runner is x86_64 only, and an arm64 +# image fails at runtime with an exec-format error rather than at push time. +docker build --platform linux/amd64 -t "$IMAGE" . +docker push "$IMAGE" + +ARN="$(aws apprunner list-services --region "$REGION" \ + --query "ServiceSummaryList[?ServiceName=='${SERVICE}'].ServiceArn | [0]" --output text)" + +CONFIG=$(cat </dev/null +fi + +echo +echo "Service URL above. Next:" +echo " 1. aws apprunner associate-custom-domain --region $REGION --service-arn --domain-name ${PUBLIC_URL#https://}" +echo " 2. add the CNAME records it returns to Route 53 (validation + the domain itself)" +echo " 3. set the App's webhook URL to ${PUBLIC_URL}/webhook and tick Active" diff --git a/docs/github-app.md b/docs/github-app.md index c30622fd..ab7af864 100644 --- a/docs/github-app.md +++ b/docs/github-app.md @@ -97,3 +97,56 @@ for the base repository, so: instance. - **The evidence quotes** in the comment's collapsed list arrive when #180 lands (`markdownReport(report, { root })` — additive, one line here). + +## Deploying to AWS App Runner + +`deploy/apprunner.sh` is the whole deploy: build for x86_64, push to ECR, create +or update the service. Run it again for every new build. + +### Once, before the first deploy + +```bash +aws sso login # or however this account authenticates + +# 1. Secrets. The private key is multi-line PEM and survives verbatim. +aws secretsmanager create-secret --name graft/app-id --secret-string 123456 +aws secretsmanager create-secret --name graft/webhook-secret --secret-string "$(openssl rand -hex 32)" +aws secretsmanager create-secret --name graft/private-key --secret-string file://graft.private-key.pem + +# 2. The role App Runner uses to PULL the image from ECR. +aws iam create-role --role-name AppRunnerECRAccessRole --path /service-role/ \ + --assume-role-policy-document '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Service":"build.apprunner.amazonaws.com"},"Action":"sts:AssumeRole"}]}' +aws iam attach-role-policy --role-name AppRunnerECRAccessRole \ + --policy-arn arn:aws:iam::aws:policy/service-role/AWSAppRunnerServicePolicyForECRAccess + +# 3. The role the RUNNING container uses to read those secrets. +aws iam create-role --role-name graft-app-instance \ + --assume-role-policy-document '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Service":"tasks.apprunner.amazonaws.com"},"Action":"sts:AssumeRole"}]}' +aws iam put-role-policy --role-name graft-app-instance --policy-name read-graft-secrets \ + --policy-document '{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"secretsmanager:GetSecretValue","Resource":"arn:aws:secretsmanager:*:*:secret:graft/*"}]}' +``` + +Two roles, because App Runner separates them: one is assumed by the *build* +side to pull the image, the other by the *running* task to read configuration. +Giving the second one only `graft/*` keeps this service away from every other +secret in the account. + +### Every deploy + +```bash +GRAFT_PUBLIC_URL=https://graft.nanonets.ai ./deploy/apprunner.sh +``` + +Then, once: `associate-custom-domain`, add the CNAMEs it prints to Route 53 +(one validates the certificate, one points the domain at the service), and set +the App's webhook URL to `https://graft.nanonets.ai/webhook`. + +### Two things the config is deliberate about + +- **`--platform linux/amd64`.** App Runner is x86_64 only; an image built on an + Apple Silicon Mac pushes fine and then fails at runtime with an exec-format + error, which reads like a broken entrypoint. +- **One instance, pinned.** Viewer pages live in the process, so a second + instance would 404 links minted by the first. Raising `--max-size` means + moving `PageStore` to S3 or a database first — the cap is correctness, not + cost control. From bc17a5bcbb665f82712428f1f0128a8ea694c9a9 Mon Sep 17 00:00:00 2001 From: anirudhkumar-nanonets Date: Tue, 25 Aug 2026 14:42:00 +0530 Subject: [PATCH 4/6] fix(app): the image needs scripts/ before npm ci MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit package.json runs scripts/postinstall.mjs on install, and copying only the manifests left node unable to find it — it exits 1 before the script's own "never fail an install" guard runs. --ignore-scripts is not the way out: tree-sitter builds its native bindings in those same hooks. --- Dockerfile | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Dockerfile b/Dockerfile index b5ccb67f..22e16193 100644 --- a/Dockerfile +++ b/Dockerfile @@ -8,7 +8,13 @@ # smaller the blast radius of anything that goes wrong. FROM node:20-bookworm-slim AS build WORKDIR /app +# scripts/ comes along because package.json runs scripts/postinstall.mjs on +# install. Copying only the manifests means node cannot find that file and exits +# 1 before the script's own "never fail an install" guard ever runs. Dropping to +# --ignore-scripts is not the way out: tree-sitter builds its native bindings in +# exactly those hooks. COPY package.json package-lock.json ./ +COPY scripts ./scripts RUN npm ci COPY . . RUN npm run build @@ -21,6 +27,7 @@ RUN apt-get update \ WORKDIR /app ENV NODE_ENV=production COPY package.json package-lock.json ./ +COPY scripts ./scripts RUN npm ci --omit=dev && npm cache clean --force COPY --from=build /app/dist ./dist From 97f956d12969ed741e0231d07262b41fe2634bfb Mon Sep 17 00:00:00 2001 From: anirudhkumar-nanonets Date: Tue, 25 Aug 2026 14:43:37 +0530 Subject: [PATCH 5/6] fix(app): build the image the way npm ci actually behaves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit npm ci runs this package's prepare script, which is the build, so the sources have to be there before the install — not after. And the runtime stage cannot reinstall: --omit=dev reruns prepare without tsc, --ignore-scripts skips the native builds tree-sitter needs. It carries the compiled node_modules over and prunes in place instead. --- Dockerfile | 41 ++++++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 19 deletions(-) diff --git a/Dockerfile b/Dockerfile index 22e16193..82ff77bc 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,38 +1,41 @@ # The graft GitHub App. # -# Runs anywhere that takes a container — Fly, Cloud Run, ECS, a VM. It needs git -# on the PATH (it fetches pull request refs) and nothing else at runtime. +# Runs anywhere that takes a container — a VM, Fly, Cloud Run, App Runner. It +# needs git on the PATH (it fetches pull request refs) and nothing else at +# runtime. # -# The build stage keeps devDependencies out of the final image: this process -# clones code written by strangers, so the less that is installed next to it, the -# smaller the blast radius of anything that goes wrong. +# Two constraints shape the stages, and both were found the hard way: +# +# - `npm ci` runs this package's `prepare` script, which IS the build. So the +# sources have to be present before the install, not after it — a manifests- +# only copy fails with "The specified path does not exist: 'tsconfig.json'". +# - The runtime cannot reinstall. `npm ci --omit=dev` would run `prepare` again +# without tsc present, and `--ignore-scripts` would skip the native builds +# tree-sitter needs. So the compiled node_modules is carried over from the +# build stage and pruned in place. FROM node:20-bookworm-slim AS build WORKDIR /app -# scripts/ comes along because package.json runs scripts/postinstall.mjs on -# install. Copying only the manifests means node cannot find that file and exits -# 1 before the script's own "never fail an install" guard ever runs. Dropping to -# --ignore-scripts is not the way out: tree-sitter builds its native bindings in -# exactly those hooks. -COPY package.json package-lock.json ./ -COPY scripts ./scripts -RUN npm ci COPY . . -RUN npm run build +RUN npm ci FROM node:20-bookworm-slim -# git is a runtime dependency here, not a build one. +# git is a runtime dependency here, not a build one: the App fetches each pull +# request's merge ref. RUN apt-get update \ && apt-get install -y --no-install-recommends git ca-certificates \ && rm -rf /var/lib/apt/lists/* WORKDIR /app ENV NODE_ENV=production COPY package.json package-lock.json ./ -COPY scripts ./scripts -RUN npm ci --omit=dev && npm cache clean --force +COPY --from=build /app/node_modules ./node_modules COPY --from=build /app/dist ./dist +# devDependencies are dead weight next to a process that clones code written by +# strangers. Pruning keeps the native bindings that were already compiled; +# --ignore-scripts stops `prepare` from trying to rebuild without tsc. +RUN npm prune --omit=dev --ignore-scripts && npm cache clean --force -# Never root: the whole point of the checkout rules is that this process handles -# untrusted source, and it has no reason to be able to write outside its tree. +# Never root: this process handles untrusted source and has no reason to be able +# to write outside its own tree. USER node ENV PORT=3000 EXPOSE 3000 From 248189f6eb5b0ca10a4c477cbb7bb68357724103 Mon Sep 17 00:00:00 2001 From: anirudhkumar-nanonets Date: Tue, 25 Aug 2026 18:01:54 +0530 Subject: [PATCH 6/6] fix(app): give the build stage a toolchain, and match node 22 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The node-gyp 12 pin means npm ci compiles tree-sitter's grammars from source, and node:*-slim has no python3, make or g++ — the build died on "find Python". Build stage only; the runtime image still compiles nothing. node 22 because commander@15 requires >=22.12, and the runtime base has to match the build base since the native bindings are copied, not rebuilt. --- Dockerfile | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 82ff77bc..36987b7b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,12 +13,22 @@ # without tsc present, and `--ignore-scripts` would skip the native builds # tree-sitter needs. So the compiled node_modules is carried over from the # build stage and pruned in place. -FROM node:20-bookworm-slim AS build +FROM node:22-bookworm-slim AS build WORKDIR /app +# node-gyp needs a real toolchain, and the slim image has none: since the repo +# pinned node-gyp 12, `npm ci` builds tree-sitter's grammars from source and dies +# on "find Python ... could not be run". Build stage only — the runtime image +# below never compiles anything and stays slim. +RUN apt-get update \ + && apt-get install -y --no-install-recommends python3 make g++ \ + && rm -rf /var/lib/apt/lists/* COPY . . RUN npm ci -FROM node:20-bookworm-slim +# node 22, not 20: commander@15 declares `node >=22.12`, and running under 20 +# left `npm ci` warning EBADENGINE on every build. The runtime base must match +# the build base — the native bindings compiled above are copied, not rebuilt. +FROM node:22-bookworm-slim # git is a runtime dependency here, not a build one: the App fetches each pull # request's merge ref. RUN apt-get update \