diff --git a/README.md b/README.md index 7abacb83..292786ce 100644 --- a/README.md +++ b/README.md @@ -790,7 +790,12 @@ token; a new plan reads your idea and pasted context there. kix publishes as its pauses for you to pick another account; the runner never switches on its own. On SIGINT or SIGTERM, running work is published and its jobs move to another runner. Runners take only trusted revisions until they can build untrusted -heads' devShells in a VM, and a job resumes with another maintainer's answer +heads' devShells in a VM. A pull request's head is trusted only when every +commit beyond the default branch has a valid signature from a current +maintainer, so sign your commits (for example with +[SSH signing](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification#ssh-commit-signature-verification)) +for its iterate and merge jobs to run; a head kix has pushed to stays +untrusted. A job resumes with another maintainer's answer only once you approve it in the inbox. opencode runs as `opencode serve` on localhost behind a password made for each turn. diff --git a/cloudflare/README.md b/cloudflare/README.md index 4435d749..4e93a936 100644 --- a/cloudflare/README.md +++ b/cloudflare/README.md @@ -90,8 +90,12 @@ target; a retry keeps both. A completed job's result may list suggested idea and takes the follow-ups and summary as context from the job's own result, never from the request, and offers them once. A job on an issue or pull request rereads it from GitHub, refusing one that closed or whose head moved since -observations read it. `iterate` and `merge` jobs are untrusted and route -only to Linux runners that declare VM builds. A `merge` job runs the merge +observations read it. `iterate` and `merge` jobs trust a head only when +every commit beyond the repository's default branch, independently of the +PR's chosen base, has a valid signature from a current maintain or admin +collaborator. The check runs at creation and again at assignment. Other +heads, including unsigned ones and those kix pushed to, route only to Linux +runners that declare VM builds. A `merge` job runs the merge skill's loop on an open, non-draft pull request and lands it; it holds the PR with the `quest:merge` label that local `quest merge` sessions share (`src/jobs/landing.ts`), as described in [GITHUB.md](GITHUB.md#merge-jobs). A five-minute scheduled sweep retries ended jobs’ hold releases even when a lifecycle callback or GitHub read was missed; a live job, a replacement hold, or an active landing keeps its label. Without a `flake.nix`, the route diff --git a/cloudflare/dev/github.ts b/cloudflare/dev/github.ts index 0487b64e..3d568a3f 100644 --- a/cloudflare/dev/github.ts +++ b/cloudflare/dev/github.ts @@ -18,6 +18,8 @@ export type Issue = { }; export type Thread = { path: string; line: number | null; resolved?: boolean; outdated?: boolean; comments: Comment[] }; export type Pull = Issue & { + /** The PR base; retargeting changes its commit list, never the trusted default branch. */ + base?: string; /** Head branch; `quest/` associates the pull request with that quest. */ branch: string; checks: "SUCCESS" | "FAILURE" | "PENDING" | null; @@ -51,7 +53,13 @@ export type Pull = Issue & { /** A fork's head branch accepts pushes from the upstream's maintainers. */ maintainerCanModify?: boolean; }; -export type Commit = { parents: string[]; tree: string; message: string }; +export type Commit = { + parents: string[]; + tree: string; + message: string; + /** GitHub's authenticated signature result, independent of author identity. */ + signature?: { isValid: boolean; signer: { databaseId: number; login: string } | null } | null; +}; /** A tree written through the API: its base tree and the entries changed on it. */ export type Tree = { base: string; entries: unknown[] }; export type Repo = { @@ -60,6 +68,8 @@ export type Repo = { name: string; /** Logins with the maintain role; everyone else can only read. */ maintainers: string[]; + /** Defaults to main. */ + defaultBranch?: string; /** The default branch's files. */ files: { path: string; content: string }[]; issues: Issue[]; @@ -327,6 +337,61 @@ function graphql(repo: Repo, query: string, variables: Record, ); return { data: { repository: objects } }; } + if (query.includes("signature {")) { + const pull = repo.pulls.find((p) => p.number === variables.number); + if (!pull) return { data: { repository: { pullRequest: null } } }; + const branch = repo.defaultBranch ?? "main"; + const anchor = repo.branches[branch] ?? commitOf(repo); + const comparison = query.includes("compare(headRef:"); + const base = comparison ? anchor : (repo.branches[pull.base ?? branch] ?? anchor); + const head = comparison ? String(variables.revision) : headOf(repo, pull); + const ancestors = (tip: string, seen = new Set()): Set => { + if (seen.has(tip)) return seen; + seen.add(tip); + for (const parent of repo.commits[tip]?.parents ?? []) ancestors(parent, seen); + return seen; + }; + const inherited = ancestors(base); + const commits = [...ancestors(head)].filter((oid) => !inherited.has(oid)); + const start = variables.after ? Number(variables.after) : 0; + const end = Math.min(start + 100, commits.length); + const nodes = commits + .slice(start, end) + .map((oid) => ({ oid, signature: repo.commits[oid]?.signature ?? null })); + const connection = { + totalCount: commits.length, + nodes, + pageInfo: { hasNextPage: end < commits.length, endCursor: String(end) }, + }; + const live = { state: pull.closed ? "CLOSED" : "OPEN", headRefOid: headOf(repo, pull) }; + return { + data: { + repository: comparison + ? { + pullRequest: live, + defaultBranchRef: { + name: branch, + target: { oid: anchor }, + compare: repo.commits[head] + ? { + baseTarget: { oid: base }, + headTarget: { oid: head }, + commits: connection, + } + : null, + }, + } + : { + pullRequest: { + ...live, + baseRefOid: base, + commits: { ...connection, nodes: nodes.map((commit) => ({ commit })) }, + }, + }, + }, + }; + } + if (query.includes("timelineItems(")) { const pull = repo.pulls.find((p) => p.number === variables.number); return { data: { repository: { pullRequest: pull ? holdsTarget(repo, pull) : null } } }; @@ -692,7 +757,7 @@ export function fakeGitHub(world: World) { owner: { login: repo.owner }, private: false, visibility: "public", - default_branch: "main", + default_branch: repo.defaultBranch ?? "main", archived: false, disabled: false, license: { spdx_id: "MIT" }, diff --git a/cloudflare/src/github/service.ts b/cloudflare/src/github/service.ts index 2fc53a8f..52ca2760 100644 --- a/cloudflare/src/github/service.ts +++ b/cloudflare/src/github/service.ts @@ -812,7 +812,7 @@ export class GitHubService { * open at the observed head, and whether its revision has a flake for the * runner's devShell. */ - jobSource( + jobSource( repo: GitHubRepository, userId: string, resolve: (state: ProjectState) => T, @@ -839,6 +839,7 @@ export class GitHubService { if (live.state !== "open" || (kind === "pulls" && live.head?.sha !== job.revision)) throw new GitHubError("This changed on GitHub since it was read; refresh and review it again"); } + if (kind === "pulls") job.trusted = await this.#pullTrust(token, repo, Number(target), job.revision); let flake = true; try { await this.#api(token, `${repositoryPath(repo)}/contents/flake.nix?ref=${sha(job.revision)}`); @@ -850,6 +851,109 @@ export class GitHubService { }); } + /** Recheck a queued PR head against current signatures and repository roles. */ + jobTrust(repo: GitHubRepository, target: string, revision: string) { + return this.#withRepository(repo, { contents: "read", pull_requests: "read" }, async (token, current) => { + this.#public(current); + return this.#pullTrust(token, repo, pullNumber(target), revision); + }); + } + + async #pullTrust(token: string, repo: GitHubRepository, pull: number, revision: string) { + type Signer = { databaseId: number; login: string }; + type Commit = { oid: string; signature: { isValid: boolean; signer: Signer | null } | null }; + type Comparison = { + baseTarget: { oid: string }; + headTarget: { oid: string }; + commits: { + totalCount: number; + nodes: Commit[]; + pageInfo: { hasNextPage: boolean; endCursor: string | null }; + }; + }; + type Page = { + pullRequest: { state: string; headRefOid: string } | null; + defaultBranchRef: { name: string; target: { oid: string } | null; compare: Comparison | null } | null; + }; + // A PR's base is contributor-controlled. Compare the exact head against the + // repository's trusted default branch so retargeting cannot hide ancestors. + const query = `query($owner:String!,$name:String!,$number:Int!,$revision:String!,$after:String) { + repository(owner:$owner,name:$name) { + pullRequest(number:$number) { state headRefOid } + defaultBranchRef { name target { oid } compare(headRef:$revision) { + baseTarget { oid } headTarget { oid } + commits(first:100,after:$after) { + totalCount nodes { oid signature { isValid signer { databaseId login } } } + pageInfo { hasNextPage endCursor } + } } } } }`; + let after: string | null = null; + let base: string | undefined; + let branch: string | undefined; + let count: number | undefined; + const cursors = new Set(); + const commits = new Set(); + const maintainers = new Map(); + do { + const response: { data?: { repository?: Page }; errors?: unknown[] } = await this.#api( + token, + "/graphql", + "POST", + { + query, + variables: { owner: repo.owner, name: repo.name, number: pull, revision: sha(revision), after }, + }, + ); + if (response.errors?.length) throw new GitHubError("Could not verify pull request commits"); + const repository: Page | undefined = response.data?.repository; + const live = repository?.pullRequest; + const anchor = repository?.defaultBranchRef; + const comparison = anchor?.compare; + if (!live || live.state !== "OPEN" || live.headRefOid !== revision || !anchor?.target || !comparison) + return false; + base ??= sha(anchor.target.oid); + branch ??= anchor.name; + const page = comparison.commits; + count ??= page.totalCount; + if ( + base !== anchor.target.oid || + branch !== anchor.name || + count !== page.totalCount || + comparison.baseTarget.oid !== base || + comparison.headTarget.oid !== revision + ) + return false; + for (const commit of page.nodes) { + if (commits.has(commit.oid)) throw new GitHubError("Repeated pull request commit"); + commits.add(sha(commit.oid)); + // Only a valid signature counts, never author email. kix's own commits + // are signed as the App, which is no maintainer, so they stay untrusted. + const signer = commit.signature?.isValid && commit.signature.signer; + if (!signer || !positiveId(signer.databaseId) || !name(signer.login)) return false; + if (!maintainers.has(signer.databaseId)) { + const permission = await this.#api<{ user: { id: number }; permission: string; role_name: string }>( + token, + `${repositoryPath(repo)}/collaborators/${signer.login}/permission`, + ).catch((error: unknown) => { + if (error instanceof GitHubError && error.status === 404) return null; + throw error; + }); + maintainers.set( + signer.databaseId, + !!permission && + permission.user.id === signer.databaseId && + (permission.permission === "admin" || permission.role_name === "maintain"), + ); + } + if (!maintainers.get(signer.databaseId)) return false; + } + if (!page.pageInfo.hasNextPage) break; + after = page.pageInfo.endCursor; + if (!after || cursors.has(after)) throw new GitHubError("Missing or repeated pull request commit cursor"); + cursors.add(after); + } while (true); + return commits.size === count && (count === 0 || commits.has(revision)); + } + previewProject(repo: GitHubRepository, userId: string) { return this.#authorized(repo, userId, { contents: "read" }, (token, current, installation) => this.#preview(token, repo, current, installation), diff --git a/cloudflare/src/jobs/hub.ts b/cloudflare/src/jobs/hub.ts index 071c641b..c8d07f5b 100644 --- a/cloudflare/src/jobs/hub.ts +++ b/cloudflare/src/jobs/hub.ts @@ -4,6 +4,7 @@ import * as landing from "./landing"; import * as output from "./output"; import { MessageError, type RunnerMessage, parse } from "./protocol"; import * as store from "./store"; +import * as trust from "./trust"; import { JobError } from "./store"; import { instructions } from "./trail"; @@ -183,7 +184,13 @@ export class RunnerHub extends DurableObject { const reported = (ws.deserializeAttachment() as RunnerAttachment).slots; if (reported !== null) slots.set(runner, reported); } - const { assign, cancel, ended, wake } = await store.dispatch(this.env.DB, owner, slots, Date.now()); + const { assign, cancel, ended, wake } = await store.dispatch( + this.env.DB, + owner, + slots, + Date.now(), + trust.verify(this.env), + ); for (const { runner, job } of assign) { const message = { type: "assign", job, instructions: instructions(job.workflow) }; connections.get(runner)?.send(JSON.stringify(message)); diff --git a/cloudflare/src/jobs/routes.ts b/cloudflare/src/jobs/routes.ts index b18df13e..7a3bfb51 100644 --- a/cloudflare/src/jobs/routes.ts +++ b/cloudflare/src/jobs/routes.ts @@ -313,8 +313,7 @@ function resolve( const data = pull.data as Pull; if (workflow === "merge" && data.isDraft) throw new JobError("Mark the draft ready for review before merging it"); - // Until the sandbox classifies commit authors, every PR head is untrusted - // and routes only to Linux runners that build its devShell in a VM. + // jobSource verifies every commit's provenance before creating this job. return { workflow, target: `pulls/${pull.number}`, diff --git a/cloudflare/src/jobs/store.ts b/cloudflare/src/jobs/store.ts index 90a4529f..b545c8d9 100644 --- a/cloudflare/src/jobs/store.ts +++ b/cloudflare/src/jobs/store.ts @@ -298,6 +298,9 @@ export function route(job: Job, offers: Offer[]): Offer | null { return eligible.find((offer) => offer.free && !offer.limited) ?? null; } +/** Whether a queued PR job's current head is still trusted. */ +export type Verify = (job: Pick) => Promise; + export type Claimed = NonNullable>> & { owner: string; name: string }; /** @@ -310,7 +313,13 @@ export type Claimed = NonNullable>> & { owner * claim commits. Until acknowledged, an assignment holds one of the runner's * reported free slots, which it could not count yet. */ -export async function dispatch(db: D1Database, owner: string, slots: Map, now: number) { +export async function dispatch( + db: D1Database, + owner: string, + slots: Map, + now: number, + verify: Verify = async () => false, +) { const connected = JSON.stringify([...slots.keys()]); const [, , cancelled, , candidates, offers, cancels, pending] = await db.batch([ db @@ -364,9 +373,20 @@ export async function dispatch(db: D1Database, owner: string, slots: Map & { limited: number }; const accounts = offers!.results as Row[]; + // The attempt and head each claim verified. A publication can still move the + // head after the claim, so the read below checks any other head again. + const verified = new Map(); + const revoked: (Attempt & { runner: string })[] = []; // Every queued job is considered, so unroutable older ones cannot starve newer ones. for (const job of candidates!.results as Job[]) { if (![...free.values()].some((n) => n > 0)) break; + if (onPull(job.workflow)) { + job.trusted = (await verify(job)) ? 1 : 0; + await db + .prepare("UPDATE jobs SET trusted=? WHERE id=? AND state='queued' AND attempt=? AND head IS ?") + .bind(job.trusted, job.id, job.attempt, job.head) + .run(); + } const chosen = route( job, accounts.map((o) => ({ ...o, free: (free.get(o.runner) ?? 0) > 0, limited: !!o.limited })), @@ -376,9 +396,19 @@ export async function dispatch(db: D1Database, owner: string, slots: Map offer.runner === runner); + const eligible = trusted || (machine?.platform === "linux" && machine.vm); + const update = await db + .prepare( + "UPDATE jobs SET trusted=?,state=? WHERE id=? AND attempt=? AND head IS ? AND cancel=0 AND state='running' AND delivered=0", + ) + .bind(trusted, eligible ? "running" : "queued", job.id, job.attempt, job.head) + .run(); + if (update.meta.changes !== 1) continue; + if (!eligible) { + revoked.push({ runner, job: job.id, attempt: job.attempt }); + continue; + } + job.trusted = trusted; + } + assign.push({ runner, job: { ...job, ...project } }); } const wake = await db .prepare("SELECT MIN(lease) AS at FROM jobs WHERE actor=? AND state='running' AND lease IS NOT NULL") @@ -408,9 +458,9 @@ export async function dispatch(db: D1Database, owner: string, slots: Map(); return { assign, - cancel: cancels!.results as (Attempt & { runner: string })[], + cancel: [...(cancels!.results as (Attempt & { runner: string })[]), ...revoked], ended: ended(cancelled!), - wake: wake?.at ?? null, + wake: revoked.length ? now : (wake?.at ?? null), }; } diff --git a/cloudflare/src/jobs/trust.ts b/cloudflare/src/jobs/trust.ts new file mode 100644 index 00000000..b04ef1db --- /dev/null +++ b/cloudflare/src/jobs/trust.ts @@ -0,0 +1,18 @@ +import { GitHubService } from "../github/service"; +import type { Verify } from "./store"; + +/** Recheck a queued PR job's head against current maintainer signatures. */ +export const verify = + (env: Env): Verify => + async (job) => { + const repo = await env.DB.prepare("SELECT id,owner,name FROM github_projects WHERE id=?") + .bind(job.repository) + .first<{ id: number; owner: string; name: string }>(); + if (!repo) return false; + try { + return await new GitHubService(env).jobTrust(repo, job.target, job.head ?? job.revision); + } catch { + // A failed read never grants trust. + return false; + } + }; diff --git a/cloudflare/test/inbox.test.ts b/cloudflare/test/inbox.test.ts index 5abcaf85..e696a846 100644 --- a/cloudflare/test/inbox.test.ts +++ b/cloudflare/test/inbox.test.ts @@ -131,7 +131,7 @@ async function running(workflow: store.Workflow = "start", target = "quest/first { repository: 10, actor: user, workflow, target, revision: commit, branch, trusted: true }, 1, ); - await store.dispatch(db, user, new Map([[runner.id, 1]]), 2); + await store.dispatch(db, user, new Map([[runner.id, 1]]), 2, async (job) => !!job.trusted); return { id, runner }; } diff --git a/cloudflare/test/jobs.test.ts b/cloudflare/test/jobs.test.ts index f7bd98cd..3d1634ec 100644 --- a/cloudflare/test/jobs.test.ts +++ b/cloudflare/test/jobs.test.ts @@ -56,7 +56,7 @@ async function state(id: string) { } /** One dispatch pass over connected runners and their free slots, whose runners acknowledge what they receive. */ async function run(slots: Record, now: number, owner = "owner", deliver = true) { - const pass = await store.dispatch(db, owner, new Map(Object.entries(slots)), now); + const pass = await store.dispatch(db, owner, new Map(Object.entries(slots)), now, async (job) => !!job.trusted); if (deliver) for (const { job } of pass.assign) await db.prepare("UPDATE jobs SET delivered=1 WHERE id=?").bind(job.id).run(); @@ -991,6 +991,27 @@ describe("job routes", () => { ); }); + it("classifies a verified maintainer PR at creation and rechecks before hub assignment", async () => { + const identity = await db + .prepare("SELECT accountId FROM account WHERE userId=? AND providerId='github'") + .bind(maintainer) + .first<{ accountId: string }>(); + const databaseId = Number(identity!.accountId); + github.signature = { isValid: true, signer: { databaseId, login: `person${databaseId}` } }; + const response = await post("", { workflow: "iterate", target: "pulls/2" }); + expect(response.status).toBe(201); + const { id } = (await response.json()) as { id: string }; + expect(await state(id)).toMatchObject({ trusted: 1, state: "queued" }); + // A role revoked after creation must not execute on an ordinary runner. + github.role = "read"; + const machine = await runner(maintainer, "machine"); + const live = await connect(machine.token); + live.send(status(1, "main")); + expect(await live.next()).toMatchObject({ type: "reply", id: 1 }); + await env.RUNNERS.get(env.RUNNERS.idFromName(maintainer)).dispatch(maintainer); + expect(await state(id)).toMatchObject({ trusted: 0, state: "queued" }); + }); + it("starts a new plan from the maintainer's idea and keeps it across a retry", async () => { const refuse = async (body: Record, error: string) => { const response = await post("", body); @@ -1524,7 +1545,14 @@ describe("job routes", () => { describe("merge wait webhook wakeups", () => { it("wakes only the current merge attempt for the enrolled repository and matching head or PR", async () => { - const machine = await runner("owner", "webhooks"); + // GitHub cannot verify the head here, so the merge job routes as untrusted to a VM runner. + const real = globalThis.fetch; + vi.spyOn(globalThis, "fetch").mockImplementation(async (input, init) => + new URL(input instanceof Request ? input.url : input).hostname === "api.github.com" + ? new Response(null, { status: 503 }) + : real(input, init), + ); + const machine = await runner("owner", "webhooks", "linux", true); await connected(machine, [account("main")]); const id = await job(10, "owner", { workflow: "merge", target: "pulls/2", branch: "feature", revision: head }); await store.held(db, id, "merge-token"); diff --git a/cloudflare/test/project-fixtures.ts b/cloudflare/test/project-fixtures.ts index 9d858448..10481f71 100644 --- a/cloudflare/test/project-fixtures.ts +++ b/cloudflare/test/project-fixtures.ts @@ -70,6 +70,7 @@ export async function maintainerOf() { const github = { flake: true, pullFlake: true, + signature: null as { isValid: boolean; signer: { databaseId: number; login: string } | null } | null, role: "maintain" as string | null, issue: "open", pull: "open", @@ -117,6 +118,28 @@ export async function maintainerOf() { suspended_at: null, permissions: { metadata: "read", contents: "write", issues: "write", pull_requests: "write" }, }); + if (path === "/graphql" && String(init?.body).includes("signature {")) + return Response.json({ + data: { + repository: { + pullRequest: { state: github.pull.toUpperCase(), headRefOid: github.head }, + defaultBranchRef: { + name: "trunk", + target: { oid: commit }, + compare: { + baseTarget: { oid: commit }, + headTarget: { oid: github.head }, + commits: { + totalCount: 1, + nodes: [{ oid: github.head, signature: github.signature }], + pageInfo: { hasNextPage: false, endCursor: null }, + }, + }, + }, + }, + }, + }); + if (path === "/graphql" && String(init?.body).includes("timelineItems")) { return Response.json({ data: { diff --git a/cloudflare/test/publish.test.ts b/cloudflare/test/publish.test.ts index c1a1af79..d6512af1 100644 --- a/cloudflare/test/publish.test.ts +++ b/cloudflare/test/publish.test.ts @@ -115,7 +115,7 @@ async function running(extra: Partial = {}, now = 0) { }, now, ); - await store.dispatch(db, user, new Map([[machine.id, 1]]), now); + await store.dispatch(db, user, new Map([[machine.id, 1]]), now, async (job) => !!job.trusted); return id; } /** Fail a job's current attempt, as its runner would. */ diff --git a/cloudflare/test/trust.test.ts b/cloudflare/test/trust.test.ts new file mode 100644 index 00000000..c2fa0e07 --- /dev/null +++ b/cloudflare/test/trust.test.ts @@ -0,0 +1,297 @@ +import { env } from "cloudflare:workers"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { type Commit, type Repo, type World, fakeGitHub, intercept } from "../dev/github"; +import { GitHubService } from "../src/github/service"; +import * as store from "../src/jobs/store"; +import { verify } from "../src/jobs/trust"; +import { privateKey } from "./github-key"; + +const db = env.DB; +const base = "a".repeat(40); +const head = "b".repeat(40); +const tree = "c".repeat(40); +const repository = { id: 10, owner: "upstream", name: "project" }; +const signature = (login = "maintainer", databaseId = 7) => ({ isValid: true, signer: { login, databaseId } }); +const commit = (parents = [base]): Commit => ({ parents, tree, message: "Change", signature: signature() }); +let repo: Repo; +let github: typeof fetch; +const testEnv = { ...env, GITHUB_APP_PRIVATE_KEY: privateKey }; +const service = () => new GitHubService(testEnv); +const trusted = (revision = head) => service().jobTrust(repository, "pulls/2", revision); + +beforeEach(async () => { + repo = { + ...repository, + maintainers: ["maintainer"], + files: [], + issues: [], + pulls: [{ number: 2, title: "Fix", author: "maintainer", branch: "fix", checks: null, review: null }], + branches: { main: base, fix: head }, + fork: {}, + trees: {}, + commits: { [base]: { parents: [], tree, message: "Base" }, [head]: commit() }, + }; + const world: World = { + app: { id: Number(env.GITHUB_APP_ID), slug: "quest" }, + installation: 5, + forkInstallation: 6, + forks: env.GITHUB_FORK_OWNER, + users: [ + { id: 7, login: "maintainer", name: "Maintainer", email: "maintainer@example.test" }, + { id: 8, login: "contributor", name: "Contributor", email: "contributor@example.test" }, + ], + repos: [repo], + }; + github = fakeGitHub(world); + vi.spyOn(globalThis, "fetch").mockImplementation(github); + await db.batch([ + db.prepare( + "INSERT INTO user(id,name,email,emailVerified,createdAt,updatedAt) VALUES('owner','Owner','owner@example.test',1,0,0)", + ), + db.prepare( + "INSERT INTO account(id,accountId,providerId,userId,createdAt,updatedAt) VALUES('owner-github','7','github','owner',0,0)", + ), + db.prepare( + "INSERT INTO github_projects(id,owner,name,installation,state,enrolledBy,updatedAt) VALUES(10,'upstream','project',5,'enabled','owner',0)", + ), + ]); +}); +afterEach(() => vi.restoreAllMocks()); + +async function queued() { + return ( + await store.createJob( + db, + { + repository: repo.id, + actor: "owner", + workflow: "iterate", + target: "pulls/2", + revision: head, + branch: "fix", + trusted: await trusted(), + }, + 0, + ) + ).id; +} + +async function runner(name: string, platform: store.Platform, vm = false) { + const { id } = await store.createRunner(db, "owner", name, platform, vm, 0); + const machine = { id, owner: "owner", name, platform, vm: vm ? 1 : 0 }; + await store.report( + db, + machine, + [{ id: "account", provider: "anthropic", kind: "subscription", label: name, limited: false }], + [], + 0, + ); + return machine; +} + +const dispatch = (machines: store.Runner[]) => + store.dispatch(db, "owner", new Map(machines.map((r) => [r.id, 1])), 1, verify(testEnv)); + +describe("authenticated PR provenance", () => { + it("routes a maintainer-signed head to an ordinary macOS runner", async () => { + const id = await queued(); + const mac = await runner("mac", "darwin"); + expect((await dispatch([mac])).assign).toMatchObject([{ runner: mac.id, job: { id, trusted: 1 } }]); + }); + + it("requires every commit, including the parents of a signed merge, to be authenticated", async () => { + const other = "d".repeat(40); + repo.commits[other] = { ...commit(), signature: signature("contributor", 8) }; + repo.commits[head]!.parents = [base, other]; + expect(await trusted()).toBe(false); + }); + + it("does not let a retargeted PR hide an unsigned ancestor under its signed head", async () => { + const hidden = "d".repeat(40); + repo.commits[hidden] = { ...commit(), signature: null }; + repo.branches.feature = hidden; + repo.pulls[0]!.base = "feature"; + repo.commits[head]!.parents = [hidden]; + expect(await trusted()).toBe(false); + // A non-default PR base itself does not make an authenticated ancestry untrusted. + repo.commits[hidden]!.signature = signature(); + expect(await trusted()).toBe(true); + }); + + it("trusts revisions already inherited by the repository default branch", async () => { + repo.branches.main = head; + repo.commits[head]!.signature = null; + expect(await trusted()).toBe(true); + }); + + it("refuses unsigned commits and a contributor forging a maintainer's author email", async () => { + // The PR author is a maintainer; only the signature signer proves who committed. + repo.commits[head]!.signature = null; + expect(await trusted()).toBe(false); + repo.commits[head]!.signature = signature("contributor", 8); + expect(await trusted()).toBe(false); + repo.commits[head]!.signature = { ...signature(), isValid: false }; + expect(await trusted()).toBe(false); + }); + + it("reclassifies a queued head after a contributor pushes and routes it only to a Linux VM runner", async () => { + const id = await queued(); + const next = "d".repeat(40); + repo.commits[next] = { ...commit([head]), signature: signature("contributor", 8) }; + repo.branches.fix = next; + const mac = await runner("mac", "darwin", true); + const linux = await runner("linux", "linux"); + const vm = await runner("vm", "linux", true); + expect((await dispatch([mac, linux, vm])).assign).toMatchObject([{ runner: vm.id, job: { id, trusted: 0 } }]); + }); + + it("rechecks current maintainer roles and leaves a revoked signer's head queued without a VM", async () => { + const id = await queued(); + repo.maintainers = []; + const mac = await runner("mac", "darwin"); + expect((await dispatch([mac])).assign).toEqual([]); + expect(await store.readJob(db, id)).toMatchObject({ trusted: 0, state: "queued" }); + }); + + it("pages all commits and checks a contributor beyond the first page", async () => { + let parent = base; + for (let i = 1; i <= 102; i++) { + const oid = i.toString(16).padStart(40, "0"); + repo.commits[oid] = commit([parent]); + parent = oid; + } + repo.branches.fix = parent; + expect(await trusted(parent)).toBe(true); + repo.commits["1".padStart(40, "0")]!.signature = signature("contributor", 8); + expect(await trusted(parent)).toBe(false); + }); + + it.each(["default tip", "default name", "pull head", "comparison head", "comparison base", "commit count"])( + "fails closed when %s changes during comparison paging", + async (change) => { + let parent = base; + for (let i = 1; i <= 102; i++) { + const oid = i.toString(16).padStart(40, "0"); + repo.commits[oid] = commit([parent]); + parent = oid; + } + repo.branches.fix = parent; + let pages = 0; + vi.mocked(globalThis.fetch).mockImplementation( + intercept( + github, + (url) => url.pathname === "/graphql" && ++pages === 2, + async (next) => { + const response = (await (await next()).json()) as any; + const repository = response.data.repository; + const branch = repository.defaultBranchRef; + if (change === "default tip") branch.target.oid = head; + if (change === "default name") branch.name = "release"; + if (change === "pull head") repository.pullRequest.headRefOid = head; + if (change === "comparison head") branch.compare.headTarget.oid = head; + if (change === "comparison base") branch.compare.baseTarget.oid = head; + if (change === "commit count") branch.compare.commits.totalCount++; + return Response.json(response); + }, + ), + ); + expect(await trusted(parent)).toBe(false); + }, + ); + + it("fails closed when GitHub cannot finish paging", async () => { + vi.mocked(globalThis.fetch).mockImplementation( + intercept( + github, + (url) => url.pathname === "/graphql", + async (next) => { + const response = (await (await next()).json()) as any; + response.data.repository.defaultBranchRef.compare.commits.pageInfo = { + hasNextPage: true, + endCursor: null, + }; + return Response.json(response); + }, + ), + ); + await expect(trusted()).rejects.toThrow("cursor"); + }); + + it("rechecks an undelivered assignment and cancels a revoked trusted attempt", async () => { + const id = await queued(); + const mac = await runner("mac", "darwin"); + expect((await dispatch([mac])).assign).toMatchObject([{ job: { id, trusted: 1 } }]); + repo.maintainers = []; + const pass = await dispatch([mac]); + expect(pass.assign).toEqual([]); + expect(pass.cancel).toEqual([{ runner: mac.id, job: id, attempt: 1 }]); + expect(await store.readJob(db, id)).toMatchObject({ trusted: 0, state: "queued" }); + }); + + it("rechecks a head a lost publication settles between the claim and the assignment", async () => { + const id = await queued(); + const first = await runner("first", "darwin"); + await dispatch([first]); + await store.report( + db, + first, + [{ id: "account", provider: "anthropic", kind: "subscription", label: "First", limited: false }], + [{ job: id, attempt: 1 }], + 1, + ); + const published = "d".repeat(40); + repo.commits[published] = { ...commit([head]), signature: { isValid: true, signer: null } }; + await store.claim(db, first, id, 1, head, published, 2); + await store.dispatch(db, "owner", new Map(), 3, verify(testEnv)); + // The lost push lands and settles only after the next pass claims the signed head. + const racing = new Proxy(db, { + get(target, key) { + if (key !== "prepare") return Reflect.get(target, key).bind(target); + return (sql: string) => { + const statement = target.prepare(sql); + if (!sql.startsWith("SELECT j.id,j.runner")) return statement; + return { + bind: (...values: unknown[]) => ({ + all: async () => { + repo.branches.fix = published; + await store.settle(db, id, published, true, 5); + return statement.bind(...values).all(); + }, + }), + }; + }; + }, + }); + const mac = await runner("mac", "darwin"); + const pass = await store.dispatch(racing, "owner", new Map([[mac.id, 1]]), store.GRACE + 4, verify(testEnv)); + expect(pass.assign).toEqual([]); + expect(pass.cancel).toEqual([{ runner: mac.id, job: id, attempt: 2 }]); + expect(await store.readJob(db, id)).toMatchObject({ head: published, trusted: 0, state: "queued" }); + }); + + it("leaves a head kix published to VM-only, even after a lost response", async () => { + const id = await queued(); + const first = await runner("first", "darwin"); + await dispatch([first]); + await store.report( + db, + first, + [{ id: "account", provider: "anthropic", kind: "subscription", label: "First", limited: false }], + [{ job: id, attempt: 1 }], + 1, + ); + // kix's commits are signed for the App, never a maintainer. + const published = "d".repeat(40); + repo.commits[published] = { ...commit([head]), signature: { isValid: true, signer: null } }; + await store.claim(db, first, id, 1, head, published, 2); + repo.branches.fix = published; + expect(await trusted(published)).toBe(false); + // The push lands but its response is lost, and the runner's lease expires. + await store.dispatch(db, "owner", new Map(), 3, verify(testEnv)); + const mac = await runner("mac", "darwin"); + const vm = await runner("vm", "linux", true); + const slots = new Map([mac, vm].map((r) => [r.id, 1])); + const pass = await store.dispatch(db, "owner", slots, store.GRACE + 4, verify(testEnv)); + expect(pass.assign).toMatchObject([{ runner: vm.id, job: { id, trusted: 0, attempt: 2 } }]); + }); +}); diff --git a/quest/m0/kix/sandbox/README.md b/quest/m0/kix/sandbox/README.md index 034c5b32..e90c6d78 100644 --- a/quest/m0/kix/sandbox/README.md +++ b/quest/m0/kix/sandbox/README.md @@ -6,7 +6,7 @@ Jobs on PR heads containing non-maintainer commits route to Linux runners, which build the PR's devShell in a per-job VM before its commands run in the OS sandbox every Claude Code job already uses. opencode jobs' commands run in that sandbox too. kix classifies PR heads whose commits all -come from maintainers or kix jobs as trusted, so those run on any runner. The +carry a current maintainer's valid signature as trusted, so those run on any runner. The sandbox is verified on macOS, and the owner's approval of another maintainer's answer is lifted once it is. @@ -73,7 +73,6 @@ Shipped in [kixelated/quest#212](https://github.com/kixelated/quest/pull/212) ## Required - [Claude Code pin](/quest/m0/kix/sandbox/claude-pin.md) - the runner's pinned Claude Code moves to the version the sandbox docs describe -- [Trusted PR heads](/quest/m0/kix/sandbox/trust.md) - PR heads whose commits all come from maintainers or kix jobs run on any runner - [Head approval](/quest/m0/kix/sandbox/head-approval.md) - a maintainer approves an exact head, such as one kix pushed, for host runners - [opencode sandboxing](/quest/m0/kix/sandbox/opencode.md) - opencode jobs' commands run in the same sandbox as Claude Code's - [macOS sandbox](/quest/m0/kix/sandbox/macos.md) - the boundary verified under Seatbelt, then the answer-approval gate removed diff --git a/quest/m0/kix/sandbox/head-approval.md b/quest/m0/kix/sandbox/head-approval.md index e37edbf8..8a348ec1 100644 --- a/quest/m0/kix/sandbox/head-approval.md +++ b/quest/m0/kix/sandbox/head-approval.md @@ -16,7 +16,7 @@ Decided 2026-10-28 (user, /quest-plan trust audit, from Codex Security's P1 on - **Publication is not trust.** kix publishes whatever the runner sends. A prompt-injected agent or a compromised runner could therefore publish a `flake.nix` that host Nix evaluates outside the sandbox. So "kix pushed it" - never grants host trust. [Trusted PR heads](/quest/m0/kix/sandbox/trust.md) + never grants host trust. [kixelated/quest#243](https://github.com/kixelated/quest/pull/243) trusts only commits signed by current maintainers and drops the publication ledger. - This quest adds the explicit path. Show the approval as a decision item @@ -28,7 +28,3 @@ Decided 2026-10-28 (user, /quest-plan trust audit, from Codex Security's P1 on - Test approval of a kix-pushed head, a head that moves after approval, an approver who loses the role, and an approval from someone who isn't a maintainer. - -## Required - -- [Trusted PR heads](/quest/m0/kix/sandbox/trust.md) - signature-only host trust, which this extends diff --git a/quest/m0/kix/sandbox/trust.md b/quest/m0/kix/sandbox/trust.md deleted file mode 100644 index 6cb880a3..00000000 --- a/quest/m0/kix/sandbox/trust.md +++ /dev/null @@ -1,33 +0,0 @@ -# [M] Run maintainers' PR heads on any runner - -## Goal - -kix classifies a PR head whose commits all come from the project's -maintainers or kix jobs as trusted, so its iterate and merge jobs run on any -runner, in the same sandbox as other trusted revisions. Any other PR head is -untrusted and routes only to Linux runners that register with `vm: true`. - -## Plan - -- Decided 2026-10-15 (from Codex's review of kixelated/quest#121): job - coordination marks every `iterate` job untrusted and routes it only to - Linux runners with `vm: true`, unless every commit on the head comes from a - maintainer or a kix job. -- Classify by authenticated provenance, never by commit authorship: GitHub - links a commit to an account by its author email, which a contributor can - forge (from Codex's review of - [kixelated/quest#220](https://github.com/kixelated/quest/pull/220)). A - commit comes from a maintainer only when GitHub reports its signature - verified for that maintainer's account, and from a kix job only when kix - recorded that job pushing it. Any other commit, including a maintainer's - unsigned one, makes the head untrusted. -- Check against the project's current maintainers when the job is created - and again when it is assigned, so a head that gains a contributor's commit - is reclassified. -- The runner keeps refusing untrusted jobs until - [untrusted devShells](/quest/m0/kix/sandbox/vm.md) ship, so until then an - untrusted head's job waits with "needs a Linux runner". -- Test a maintainer-only head, a head with one contributor commit, a head - that gains one after the job is queued, a revoked maintainer's commit, an - unsigned maintainer commit, and a contributor commit forging a - maintainer's author email. diff --git a/quest/m0/kix/sandbox/vm.md b/quest/m0/kix/sandbox/vm.md index 83dafdff..e49e6927 100644 --- a/quest/m0/kix/sandbox/vm.md +++ b/quest/m0/kix/sandbox/vm.md @@ -15,6 +15,6 @@ sandbox with no network. arbitrary builders, and never publish them to the [shared cache](/quest/m1/automation/cache.md). - The runner refuses untrusted jobs until this ships; - [trusted PR heads](/quest/m0/kix/sandbox/trust.md) route them here. + the trusted-head classifier routes untrusted heads here. - Test a PR whose flake or shellHook tries to run on the host, and one whose build tries to reach the network.