From d5b943976c6c4fe2824593a9aac31aeb19f99ee5 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 27 Oct 2026 21:23:54 -0700 Subject: [PATCH 1/7] chore(quest): claim quest/m0/kix/sandbox/trust.md Quest-Claim: 82c845638ccb6d74 From 2c2c0f71197e9e4f44b5198f1dd904960f62eb27 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 27 Oct 2026 21:36:27 -0700 Subject: [PATCH 2/7] feat: trust authenticated maintainer and kix PR commits --- quest/m0/kix/sandbox/README.md | 1 - quest/m0/kix/sandbox/trust.md | 33 --------------------------------- 2 files changed, 34 deletions(-) delete mode 100644 quest/m0/kix/sandbox/trust.md diff --git a/quest/m0/kix/sandbox/README.md b/quest/m0/kix/sandbox/README.md index 8d5176e5..a149ed17 100644 --- a/quest/m0/kix/sandbox/README.md +++ b/quest/m0/kix/sandbox/README.md @@ -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 - [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 - [Ubuntu AppArmor](/quest/m0/kix/sandbox/apparmor.md) - the sandbox starts on stock Ubuntu, or the runner refuses at startup 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. From 59b86a2bcf27f3be2779dfbd4dfebc3b6c2792f6 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 27 Oct 2026 21:36:39 -0700 Subject: [PATCH 3/7] feat: verify PR provenance when creating and assigning jobs --- cloudflare/README.md | 4 +- cloudflare/dev/github.ts | 41 ++++- cloudflare/migrations/0020_job_commits.sql | 7 + cloudflare/src/github/service.ts | 89 ++++++++- cloudflare/src/jobs/hub.ts | 21 ++- cloudflare/src/jobs/routes.ts | 3 +- cloudflare/src/jobs/store.ts | 102 ++++++++--- cloudflare/test/inbox.test.ts | 2 +- cloudflare/test/jobs.test.ts | 23 ++- cloudflare/test/project-fixtures.ts | 18 ++ cloudflare/test/publish.test.ts | 2 +- cloudflare/test/trust.test.ts | 200 +++++++++++++++++++++ quest/m0/kix/sandbox/vm.md | 2 +- 13 files changed, 484 insertions(+), 30 deletions(-) create mode 100644 cloudflare/migrations/0020_job_commits.sql create mode 100644 cloudflare/test/trust.test.ts diff --git a/cloudflare/README.md b/cloudflare/README.md index dad318e5..f3586941 100644 --- a/cloudflare/README.md +++ b/cloudflare/README.md @@ -90,7 +90,9 @@ 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 +observations read it. `iterate` and `merge` jobs verify every PR commit: only a verified signature +from a current maintainer, or a confirmed publication recorded by kix, is +trusted. The check runs at creation and again at assignment. Other heads 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 diff --git a/cloudflare/dev/github.ts b/cloudflare/dev/github.ts index b28a4f7e..36ccfad0 100644 --- a/cloudflare/dev/github.ts +++ b/cloudflare/dev/github.ts @@ -51,7 +51,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 = { @@ -320,6 +326,39 @@ 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 base = repo.branches.main ?? commitOf(repo); + 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(headOf(repo, pull))].filter((oid) => !inherited.has(oid)); + const start = variables.after ? Number(variables.after) : 0; + const end = Math.min(start + 100, commits.length); + return { + data: { + repository: { + pullRequest: { + state: pull.closed ? "CLOSED" : "OPEN", + headRefOid: headOf(repo, pull), + baseRefOid: base, + commits: { + totalCount: commits.length, + nodes: commits + .slice(start, end) + .map((oid) => ({ commit: { oid, signature: repo.commits[oid]?.signature ?? null } })), + pageInfo: { hasNextPage: end < commits.length, endCursor: String(end) }, + }, + }, + }, + }, + }; + } if (query.includes("timelineItems(")) { const pull = repo.pulls.find((p) => p.number === variables.number); return { data: { repository: { pullRequest: pull ? holdsTarget(repo, pull) : null } } }; diff --git a/cloudflare/migrations/0020_job_commits.sql b/cloudflare/migrations/0020_job_commits.sql new file mode 100644 index 00000000..a2c2432d --- /dev/null +++ b/cloudflare/migrations/0020_job_commits.sql @@ -0,0 +1,7 @@ +-- Authenticated publication provenance, recorded only after a branch update succeeds. +CREATE TABLE job_commits ( + repository INTEGER NOT NULL REFERENCES github_projects(id) ON DELETE CASCADE, + sha TEXT NOT NULL, + job TEXT NOT NULL REFERENCES jobs(id) ON DELETE CASCADE, + PRIMARY KEY(repository,sha) +); diff --git a/cloudflare/src/github/service.ts b/cloudflare/src/github/service.ts index 94fbb16c..f50595b7 100644 --- a/cloudflare/src/github/service.ts +++ b/cloudflare/src/github/service.ts @@ -716,7 +716,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, @@ -743,6 +743,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)}`); @@ -754,6 +755,92 @@ 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 Page = { + state: string; + headRefOid: string; + baseRefOid: string; + commits: { + totalCount: number; + nodes: { commit: Commit }[]; + pageInfo: { hasNextPage: boolean; endCursor: string | null }; + }; + }; + const query = `query($owner:String!,$name:String!,$number:Int!,$after:String) { + repository(owner:$owner,name:$name) { pullRequest(number:$number) { + state headRefOid baseRefOid commits(first:100,after:$after) { + totalCount nodes { commit { oid signature { isValid signer { databaseId login } } } } + pageInfo { hasNextPage endCursor } + } } } }`; + let after: string | null = null; + let base: string | undefined; + let count: number | undefined; + const cursors = new Set(); + const commits = new Set(); + const maintainers = new Map(); + do { + const response: { data?: { repository?: { pullRequest: Page | null } }; errors?: unknown[] } = + await this.#api(token, "/graphql", "POST", { + query, + variables: { owner: repo.owner, name: repo.name, number: pull, after }, + }); + if (response.errors?.length) throw new GitHubError("Could not verify pull request commits"); + const page: Page | null | undefined = response.data?.repository?.pullRequest; + if (!page || page.state !== "OPEN" || page.headRefOid !== sha(revision)) return false; + base ??= page.baseRefOid; + count ??= page.commits.totalCount; + if (base !== page.baseRefOid || count !== page.commits.totalCount) return false; + const published = new Set( + ( + await this.#env.DB.prepare( + "SELECT sha FROM job_commits WHERE repository=? AND sha IN (SELECT value FROM json_each(?))", + ) + .bind(repo.id, JSON.stringify(page.commits.nodes.map(({ commit }) => sha(commit.oid)))) + .all<{ sha: string }>() + ).results.map(({ sha }) => sha), + ); + for (const { commit } of page.commits.nodes) { + if (commits.has(commit.oid)) throw new GitHubError("Repeated pull request commit"); + commits.add(sha(commit.oid)); + // Only confirmed publications recorded by kix count, never agent claims or author email. + if (published.has(commit.oid)) continue; + 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.commits.pageInfo.hasNextPage) break; + after = page.commits.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 > 0 && commits.size === count && 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 d30e9dc1..00471736 100644 --- a/cloudflare/src/jobs/hub.ts +++ b/cloudflare/src/jobs/hub.ts @@ -1,4 +1,5 @@ import { DurableObject } from "cloudflare:workers"; +import { GitHubService } from "../github/service"; import * as push from "../push/notify"; import * as landing from "./landing"; import * as output from "./output"; @@ -121,7 +122,25 @@ export class RunnerHub extends DurableObject { const reported = (ws.deserializeAttachment() as Attachment).slots; if (reported !== null) slots.set(runner, reported); } - const { assign, cancel, ended, wake } = await store.dispatch(this.env.DB, owner, slots, Date.now()); + const github = new GitHubService(this.env); + const { assign, cancel, ended, wake } = await store.dispatch( + this.env.DB, + owner, + slots, + Date.now(), + async (job) => { + const repo = await this.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 github.jobTrust(repo, job.target, job.head ?? job.revision); + } catch { + // No current proof means no trusted execution, including during a GitHub outage. + return false; + } + }, + ); 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 b8bd7892..d0bf3b33 100644 --- a/cloudflare/src/jobs/routes.ts +++ b/cloudflare/src/jobs/routes.ts @@ -312,8 +312,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 57291a43..21ecc5bf 100644 --- a/cloudflare/src/jobs/store.ts +++ b/cloudflare/src/jobs/store.ts @@ -310,7 +310,15 @@ 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: ( + job: Pick, + ) => Promise = async () => false, +) { const connected = JSON.stringify([...slots.keys()]); const [, , cancelled, , candidates, offers, cancels, pending] = await db.batch([ db @@ -364,9 +372,19 @@ export async function dispatch(db: D1Database, owner: string, slots: Map & { limited: number }; const accounts = offers!.results as Row[]; + const verified = new Set(); + 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; + verified.add(job.id); + 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 +394,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 +454,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), }; } @@ -578,12 +624,18 @@ export async function publishing(db: D1Database, runner: Runner, job: string, at /** Record a pending commit that the branch shows landed after all. */ export async function adopt(db: D1Database, job: string, head: string | null, pending: string, now: number) { - await db - .prepare( - "UPDATE jobs SET head=pending,pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND head IS ? AND pending=?", - ) - .bind(now, job, head, pending) - .run(); + await db.batch([ + db + .prepare( + "INSERT OR IGNORE INTO job_commits(repository,sha,job) SELECT repository,pending,id FROM jobs WHERE id=? AND head IS ? AND pending=?", + ) + .bind(job, head, pending), + db + .prepare( + "UPDATE jobs SET head=pending,pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND head IS ? AND pending=?", + ) + .bind(now, job, head, pending), + ]); } /** @@ -612,14 +664,24 @@ export async function claim( /** Record whether the reserved branch update landed. The branch's head follows it either way. */ export async function settle(db: D1Database, job: string, commit: string, landed: boolean, now: number) { - await db - .prepare( - landed - ? "UPDATE jobs SET head=pending,pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND pending=?" - : "UPDATE jobs SET pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND pending=?", - ) - .bind(now, job, commit) - .run(); + await db.batch([ + ...(landed + ? [ + db + .prepare( + "INSERT OR IGNORE INTO job_commits(repository,sha,job) SELECT repository,pending,id FROM jobs WHERE id=? AND pending=?", + ) + .bind(job, commit), + ] + : []), + db + .prepare( + landed + ? "UPDATE jobs SET head=pending,pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND pending=?" + : "UPDATE jobs SET pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND pending=?", + ) + .bind(now, job, commit), + ]); } /** diff --git a/cloudflare/test/inbox.test.ts b/cloudflare/test/inbox.test.ts index 5e2db90a..b73ab172 100644 --- a/cloudflare/test/inbox.test.ts +++ b/cloudflare/test/inbox.test.ts @@ -97,7 +97,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 8bd507b5..7c54e0fa 100644 --- a/cloudflare/test/jobs.test.ts +++ b/cloudflare/test/jobs.test.ts @@ -55,7 +55,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(); @@ -990,6 +990,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); diff --git a/cloudflare/test/project-fixtures.ts b/cloudflare/test/project-fixtures.ts index 9d858448..861ffe94 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,23 @@ 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, + baseRefOid: commit, + commits: { + totalCount: 1, + nodes: [{ commit: { 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 2ef282a4..63f9b671 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..9caf503d --- /dev/null +++ b/cloudflare/test/trust.test.ts @@ -0,0 +1,200 @@ +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 { 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 service = () => new GitHubService({ ...env, GITHUB_APP_PRIVATE_KEY: privateKey }); +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 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, (job) => + service().jobTrust(repository, job.target, job.head ?? job.revision), + ); + +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("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("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.pullRequest.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("trusts confirmed kix publications, retaining earlier commits but refusing unconfirmed and failed pushes", async () => { + const id = await queued(); + const machine = await runner("linux", "linux"); + await dispatch([machine]); + const published = "d".repeat(40); + const next = "e".repeat(40); + for (const oid of [published, next]) repo.commits[oid] = { ...commit([base]), signature: null }; + repo.branches.fix = published; + expect(await trusted(published)).toBe(false); + await store.claim(db, machine, id, 1, head, published, 2); + await store.settle(db, id, published, false, 3); + expect(await trusted(published)).toBe(false); + await store.claim(db, machine, id, 1, head, published, 4); + await store.settle(db, id, published, true, 5); + expect(await trusted(published)).toBe(true); + repo.commits[next]!.parents = [published]; + repo.branches.fix = next; + await store.claim(db, machine, id, 1, published, next, 6); + // A confirmed update recovered after a crash records the same provenance. + await store.adopt(db, id, published, next, 7); + expect(await trusted(next)).toBe(true); + expect((await db.prepare("SELECT sha FROM job_commits ORDER BY sha").all()).results).toEqual([ + { sha: published }, + { sha: next }, + ]); + }); +}); 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. From 4a61f5ae3451842093fac2dd4ff4a6fa432cdc90 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 27 Oct 2026 21:50:11 -0700 Subject: [PATCH 4/7] fix: anchor PR trust to the repository default branch --- cloudflare/README.md | 3 +- cloudflare/dev/github.ts | 58 +++++++++++++++------- cloudflare/src/github/service.ts | 74 +++++++++++++++++++---------- cloudflare/test/project-fixtures.ts | 21 ++++---- cloudflare/test/trust.test.ts | 56 +++++++++++++++++++++- 5 files changed, 162 insertions(+), 50 deletions(-) diff --git a/cloudflare/README.md b/cloudflare/README.md index f3586941..00ac6fe5 100644 --- a/cloudflare/README.md +++ b/cloudflare/README.md @@ -90,7 +90,8 @@ 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 verify every PR commit: only a verified signature +observations read it. `iterate` and `merge` jobs verify every commit beyond the repository's default branch, independently +of the PR's chosen base: only a verified signature from a current maintainer, or a confirmed publication recorded by kix, is trusted. The check runs at creation and again at assignment. Other heads route only to Linux runners that declare VM builds. A `merge` job runs the merge diff --git a/cloudflare/dev/github.ts b/cloudflare/dev/github.ts index 36ccfad0..0753574f 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; @@ -66,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[]; @@ -329,7 +333,11 @@ function graphql(repo: Repo, query: string, variables: Record, if (query.includes("signature {")) { const pull = repo.pulls.find((p) => p.number === variables.number); if (!pull) return { data: { repository: { pullRequest: null } } }; - const base = repo.branches.main ?? commitOf(repo); + 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); @@ -337,28 +345,46 @@ function graphql(repo: Repo, query: string, variables: Record, return seen; }; const inherited = ancestors(base); - const commits = [...ancestors(headOf(repo, pull))].filter((oid) => !inherited.has(oid)); + 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: { - pullRequest: { - state: pull.closed ? "CLOSED" : "OPEN", - headRefOid: headOf(repo, pull), - baseRefOid: base, - commits: { - totalCount: commits.length, - nodes: commits - .slice(start, end) - .map((oid) => ({ commit: { oid, signature: repo.commits[oid]?.signature ?? null } })), - pageInfo: { hasNextPage: end < commits.length, endCursor: String(end) }, + 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 } } }; @@ -710,7 +736,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 f50595b7..ea8f6d13 100644 --- a/cloudflare/src/github/service.ts +++ b/cloudflare/src/github/service.ts @@ -766,50 +766,76 @@ export class GitHubService { 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 Page = { - state: string; - headRefOid: string; - baseRefOid: string; + type Comparison = { + baseTarget: { oid: string }; + headTarget: { oid: string }; commits: { totalCount: number; - nodes: { commit: Commit }[]; + nodes: Commit[]; pageInfo: { hasNextPage: boolean; endCursor: string | null }; }; }; - const query = `query($owner:String!,$name:String!,$number:Int!,$after:String) { - repository(owner:$owner,name:$name) { pullRequest(number:$number) { - state headRefOid baseRefOid commits(first:100,after:$after) { - totalCount nodes { commit { oid signature { isValid signer { databaseId login } } } } + 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?: { pullRequest: Page | null } }; errors?: unknown[] } = - await this.#api(token, "/graphql", "POST", { + const response: { data?: { repository?: Page }; errors?: unknown[] } = await this.#api( + token, + "/graphql", + "POST", + { query, - variables: { owner: repo.owner, name: repo.name, number: pull, after }, - }); + 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 page: Page | null | undefined = response.data?.repository?.pullRequest; - if (!page || page.state !== "OPEN" || page.headRefOid !== sha(revision)) return false; - base ??= page.baseRefOid; - count ??= page.commits.totalCount; - if (base !== page.baseRefOid || count !== page.commits.totalCount) return false; + 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; const published = new Set( ( await this.#env.DB.prepare( "SELECT sha FROM job_commits WHERE repository=? AND sha IN (SELECT value FROM json_each(?))", ) - .bind(repo.id, JSON.stringify(page.commits.nodes.map(({ commit }) => sha(commit.oid)))) + .bind(repo.id, JSON.stringify(page.nodes.map((commit) => sha(commit.oid)))) .all<{ sha: string }>() ).results.map(({ sha }) => sha), ); - for (const { commit } of page.commits.nodes) { + for (const commit of page.nodes) { if (commits.has(commit.oid)) throw new GitHubError("Repeated pull request commit"); commits.add(sha(commit.oid)); // Only confirmed publications recorded by kix count, never agent claims or author email. @@ -833,12 +859,12 @@ export class GitHubService { } if (!maintainers.get(signer.databaseId)) return false; } - if (!page.commits.pageInfo.hasNextPage) break; - after = page.commits.pageInfo.endCursor; + 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 > 0 && commits.size === count && commits.has(revision); + return commits.size === count && (count === 0 || commits.has(revision)); } previewProject(repo: GitHubRepository, userId: string) { diff --git a/cloudflare/test/project-fixtures.ts b/cloudflare/test/project-fixtures.ts index 861ffe94..10481f71 100644 --- a/cloudflare/test/project-fixtures.ts +++ b/cloudflare/test/project-fixtures.ts @@ -122,19 +122,24 @@ export async function maintainerOf() { return Response.json({ data: { repository: { - pullRequest: { - state: github.pull.toUpperCase(), - headRefOid: github.head, - baseRefOid: commit, - commits: { - totalCount: 1, - nodes: [{ commit: { oid: github.head, signature: github.signature } }], - pageInfo: { hasNextPage: false, endCursor: null }, + 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/trust.test.ts b/cloudflare/test/trust.test.ts index 9caf503d..e27aa843 100644 --- a/cloudflare/test/trust.test.ts +++ b/cloudflare/test/trust.test.ts @@ -103,6 +103,24 @@ describe("authenticated PR provenance", () => { 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; @@ -145,6 +163,39 @@ describe("authenticated PR provenance", () => { 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( @@ -152,7 +203,10 @@ describe("authenticated PR provenance", () => { (url) => url.pathname === "/graphql", async (next) => { const response = (await (await next()).json()) as any; - response.data.repository.pullRequest.commits.pageInfo = { hasNextPage: true, endCursor: null }; + response.data.repository.defaultBranchRef.compare.commits.pageInfo = { + hasNextPage: true, + endCursor: null, + }; return Response.json(response); }, ), From 495c782b691f8ef804e707fa7618adcacae81ba7 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Tue, 27 Oct 2026 22:02:39 -0700 Subject: [PATCH 5/7] fix: recover pending publications before routing PR jobs --- cloudflare/src/jobs/hub.ts | 22 ++------------ cloudflare/src/jobs/publish.ts | 10 +++++-- cloudflare/src/jobs/store.ts | 33 ++++++++++++++++---- cloudflare/src/jobs/trust.ts | 22 ++++++++++++++ cloudflare/test/inbox.test.ts | 5 +++- cloudflare/test/jobs.test.ts | 5 +++- cloudflare/test/publish.test.ts | 5 +++- cloudflare/test/trust.test.ts | 53 ++++++++++++++++++++++++++++++++- 8 files changed, 124 insertions(+), 31 deletions(-) create mode 100644 cloudflare/src/jobs/trust.ts diff --git a/cloudflare/src/jobs/hub.ts b/cloudflare/src/jobs/hub.ts index 00471736..8c7302e4 100644 --- a/cloudflare/src/jobs/hub.ts +++ b/cloudflare/src/jobs/hub.ts @@ -1,10 +1,10 @@ import { DurableObject } from "cloudflare:workers"; -import { GitHubService } from "../github/service"; import * as push from "../push/notify"; 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"; @@ -122,24 +122,8 @@ export class RunnerHub extends DurableObject { const reported = (ws.deserializeAttachment() as Attachment).slots; if (reported !== null) slots.set(runner, reported); } - const github = new GitHubService(this.env); - const { assign, cancel, ended, wake } = await store.dispatch( - this.env.DB, - owner, - slots, - Date.now(), - async (job) => { - const repo = await this.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 github.jobTrust(repo, job.target, job.head ?? job.revision); - } catch { - // No current proof means no trusted execution, including during a GitHub outage. - return false; - } - }, + const { assign, cancel, ended, wake } = await store.dispatch(this.env.DB, owner, slots, Date.now(), (job) => + trust.verify(this.env, job), ); for (const { runner, job } of assign) { const message = { type: "assign", job, instructions: instructions(job.workflow) }; diff --git a/cloudflare/src/jobs/publish.ts b/cloudflare/src/jobs/publish.ts index d97ede45..6f0ab307 100644 --- a/cloudflare/src/jobs/publish.ts +++ b/cloudflare/src/jobs/publish.ts @@ -206,7 +206,11 @@ async function decide(db: D1Database, runner: Runner, input: Publication, prompt * commit whose outcome went unrecorded is adopted first, so an interrupted * publication is never mistaken for someone else's write. */ -async function verify(db: D1Database, job: store.Publishing, input: Publication, branch: Branch) { +export async function publicationHead( + db: D1Database, + job: Pick, + branch: Branch, +) { const current = await branch.head(); let head = job.head; if (current !== head && job.pending && current === job.pending) { @@ -219,7 +223,6 @@ async function verify(db: D1Database, job: store.Publishing, input: Publication, ? `${branch.repository.owner}/${branch.repository.name} already has the branch ${job.branch} from earlier work.` : `The branch ${job.branch} changed outside this job.`, ); - if (input.head !== head) throw new HeadMoved(head); return head; } @@ -269,7 +272,8 @@ export async function publish(env: Env, runner: Runner, input: Publication): Pro try { // A pull request opens from the branch, so a delivery alone still checks where it is. head = await github.contribute(upstream, job.actor, job, async (branch) => { - const head = await verify(db, job, input, branch); + const head = await publicationHead(db, job, branch); + if (input.head !== head) throw new HeadMoved(head); return input.changes.length || input.merge ? commit(db, runner, job, input, branch, head) : head; }); } catch (error) { diff --git a/cloudflare/src/jobs/store.ts b/cloudflare/src/jobs/store.ts index 21ecc5bf..cd940c3c 100644 --- a/cloudflare/src/jobs/store.ts +++ b/cloudflare/src/jobs/store.ts @@ -298,6 +298,23 @@ export function route(job: Job, offers: Offer[]): Offer | null { return eligible.find((offer) => offer.free && !offer.limited) ?? null; } +export type AssignmentSource = Pick< + Job, + | "id" + | "repository" + | "workflow" + | "target" + | "revision" + | "branch" + | "actor" + | "trusted" + | "head" + | "pending" + | "pull" + | "createdAt" +>; +export type VerifiedSource = { head: string | null; trusted: boolean }; + export type Claimed = NonNullable>> & { owner: string; name: string }; /** @@ -315,9 +332,7 @@ export async function dispatch( owner: string, slots: Map, now: number, - verify: ( - job: Pick, - ) => Promise = async () => false, + verify: (job: AssignmentSource) => Promise = async (job) => ({ head: job.head, trusted: false }), ) { const connected = JSON.stringify([...slots.keys()]); const [, , cancelled, , candidates, offers, cancels, pending] = await db.batch([ @@ -378,7 +393,9 @@ export async function dispatch( 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; + const source = await verify(job); + job.head = source.head; + job.trusted = source.trusted ? 1 : 0; verified.add(job.id); await db .prepare("UPDATE jobs SET trusted=? WHERE id=? AND state='queued' AND attempt=? AND head IS ?") @@ -430,7 +447,13 @@ export async function dispatch( const job = await readJob(db, id); if (!job || job.state !== "running" || job.cancel) continue; if (onPull(job.workflow) && !verified.has(job.id)) { - const trusted = (await verify(job)) ? 1 : 0; + const source = await verify(job); + job.head = source.head; + if (job.pending === source.head) { + job.pending = null; + job.pendingAt = null; + } + const trusted = source.trusted ? 1 : 0; const machine = accounts.find((offer) => offer.runner === runner); const eligible = trusted || (machine?.platform === "linux" && machine.vm); const update = await db diff --git a/cloudflare/src/jobs/trust.ts b/cloudflare/src/jobs/trust.ts new file mode 100644 index 00000000..90614744 --- /dev/null +++ b/cloudflare/src/jobs/trust.ts @@ -0,0 +1,22 @@ +import { GitHubService } from "../github/service"; +import { publicationHead } from "./publish"; +import type { AssignmentSource, VerifiedSource } from "./store"; + +/** Recover only a confirmed, fenced publication before classifying a queued PR revision. */ +export async function verify(env: Env, job: AssignmentSource): Promise { + const untrusted = { head: job.head, trusted: false }; + 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 untrusted; + const github = new GitHubService(env); + try { + const head = job.pending + ? await github.contribute(repo, job.actor, job, (branch) => publicationHead(env.DB, job, branch)) + : job.head; + return { head, trusted: await github.jobTrust(repo, job.target, head ?? job.revision) }; + } catch { + // A failed read, changed branch, or unconfirmed publication never grants trust. + return untrusted; + } +} diff --git a/cloudflare/test/inbox.test.ts b/cloudflare/test/inbox.test.ts index b73ab172..27195c15 100644 --- a/cloudflare/test/inbox.test.ts +++ b/cloudflare/test/inbox.test.ts @@ -97,7 +97,10 @@ 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, async (job) => !!job.trusted); + await store.dispatch(db, user, new Map([[runner.id, 1]]), 2, async (job) => ({ + head: job.head, + trusted: !!job.trusted, + })); return { id, runner }; } diff --git a/cloudflare/test/jobs.test.ts b/cloudflare/test/jobs.test.ts index 7c54e0fa..644ddd7b 100644 --- a/cloudflare/test/jobs.test.ts +++ b/cloudflare/test/jobs.test.ts @@ -55,7 +55,10 @@ 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, async (job) => !!job.trusted); + const pass = await store.dispatch(db, owner, new Map(Object.entries(slots)), now, async (job) => ({ + head: job.head, + trusted: !!job.trusted, + })); if (deliver) for (const { job } of pass.assign) await db.prepare("UPDATE jobs SET delivered=1 WHERE id=?").bind(job.id).run(); diff --git a/cloudflare/test/publish.test.ts b/cloudflare/test/publish.test.ts index 63f9b671..af819605 100644 --- a/cloudflare/test/publish.test.ts +++ b/cloudflare/test/publish.test.ts @@ -115,7 +115,10 @@ async function running(extra: Partial = {}, now = 0) { }, now, ); - await store.dispatch(db, user, new Map([[machine.id, 1]]), now, async (job) => !!job.trusted); + await store.dispatch(db, user, new Map([[machine.id, 1]]), now, async (job) => ({ + head: job.head, + trusted: !!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 index e27aa843..4e73d715 100644 --- a/cloudflare/test/trust.test.ts +++ b/cloudflare/test/trust.test.ts @@ -3,6 +3,7 @@ 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 as verifyAssignment } from "../src/jobs/trust"; import { privateKey } from "./github-key"; const db = env.DB; @@ -46,6 +47,9 @@ beforeEach(async () => { 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)", ), @@ -86,7 +90,7 @@ async function runner(name: string, platform: store.Platform, vm = false) { const dispatch = (machines: store.Runner[]) => store.dispatch(db, "owner", new Map(machines.map((r) => [r.id, 1])), 1, (job) => - service().jobTrust(repository, job.target, job.head ?? job.revision), + verifyAssignment({ ...env, GITHUB_APP_PRIVATE_KEY: privateKey }, job), ); describe("authenticated PR provenance", () => { @@ -225,6 +229,53 @@ describe("authenticated PR provenance", () => { expect(await store.readJob(db, id)).toMatchObject({ trusted: 0, state: "queued" }); }); + it.each(["landed", "unconfirmed", "changed"])( + "reassigns safely after a %s pending publication loses its response", + async (outcome) => { + const id = await queued(); + const first = await runner("first", "linux"); + await dispatch([first]); + await store.report( + db, + first, + [{ id: "account", provider: "anthropic", kind: "subscription", label: "First", limited: false }], + [{ job: id, attempt: 1 }], + 1, + ); + const pending = "d".repeat(40); + repo.commits[pending] = { ...commit([head]), signature: null }; + await store.claim(db, first, id, 1, head, pending, 2); + if (outcome === "landed") repo.branches.fix = pending; + if (outcome === "changed") { + const other = "e".repeat(40); + repo.commits[other] = { ...commit([head]), signature: null }; + repo.branches.fix = other; + } + // Its disconnected runner's lease expires. Recovery must precede trust and routing. + const verify = (job: store.AssignmentSource) => + verifyAssignment({ ...env, GITHUB_APP_PRIVATE_KEY: privateKey }, job); + await store.dispatch(db, "owner", new Map(), 3, verify); + const replacement = await runner("replacement", "darwin"); + const pass = await store.dispatch(db, "owner", new Map([[replacement.id, 1]]), store.GRACE + 4, verify); + if (outcome === "changed") { + expect(pass.assign).toEqual([]); + expect(await store.readJob(db, id)).toMatchObject({ head, pending, trusted: 0, state: "queued" }); + } else { + const expected = outcome === "landed" ? pending : head; + expect(pass.assign).toMatchObject([ + { runner: replacement.id, job: { id, head: expected, trusted: 1, attempt: 2 } }, + ]); + expect(await store.readJob(db, id)).toMatchObject({ + head: expected, + pending: outcome === "landed" ? null : pending, + }); + } + expect((await db.prepare("SELECT sha FROM job_commits").all()).results).toEqual( + outcome === "landed" ? [{ sha: pending }] : [], + ); + }, + ); + it("trusts confirmed kix publications, retaining earlier commits but refusing unconfirmed and failed pushes", async () => { const id = await queued(); const machine = await runner("linux", "linux"); From 8a3bce2b25516b27fd15785898eadb0db0f76ec5 Mon Sep 17 00:00:00 2001 From: Luke Curley <432854+kixelated@users.noreply.github.com> Date: Wed, 28 Oct 2026 10:13:37 -0700 Subject: [PATCH 6/7] fix: trust PR heads by maintainer signatures only Drop the kix publication ledger (job_commits, migration 0020, its writes on settle/adopt, and pending-publication recovery before assignment). A compromised runner can publish any flake.nix, so a confirmed publication is no execution trust. A head is trusted only when every commit beyond the default branch has a valid signature from a current maintain/admin collaborator, rechecked at creation and assignment; heads kix pushed to stay VM-only. Co-Authored-By: Claude Opus 5.5 --- README.md | 7 +- cloudflare/README.md | 11 +-- cloudflare/migrations/0020_job_commits.sql | 7 -- cloudflare/src/github/service.ts | 13 +-- cloudflare/src/jobs/hub.ts | 8 +- cloudflare/src/jobs/publish.ts | 10 +-- cloudflare/src/jobs/store.ts | 76 +++++------------ cloudflare/src/jobs/trust.ts | 36 ++++---- cloudflare/test/inbox.test.ts | 5 +- cloudflare/test/jobs.test.ts | 14 ++-- cloudflare/test/publish.test.ts | 5 +- cloudflare/test/trust.test.ts | 97 ++++++---------------- quest/m0/kix/sandbox/README.md | 2 +- 13 files changed, 94 insertions(+), 197 deletions(-) delete mode 100644 cloudflare/migrations/0020_job_commits.sql diff --git a/README.md b/README.md index 324a0e68..bcb4e5dd 100644 --- a/README.md +++ b/README.md @@ -788,7 +788,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 3c4a5da0..bd8ac95b 100644 --- a/cloudflare/README.md +++ b/cloudflare/README.md @@ -90,11 +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 verify every commit beyond the repository's default branch, independently -of the PR's chosen base: only a verified signature -from a current maintainer, or a confirmed publication recorded by kix, is -trusted. The check runs at creation and again at assignment. Other heads 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/migrations/0020_job_commits.sql b/cloudflare/migrations/0020_job_commits.sql deleted file mode 100644 index a2c2432d..00000000 --- a/cloudflare/migrations/0020_job_commits.sql +++ /dev/null @@ -1,7 +0,0 @@ --- Authenticated publication provenance, recorded only after a branch update succeeds. -CREATE TABLE job_commits ( - repository INTEGER NOT NULL REFERENCES github_projects(id) ON DELETE CASCADE, - sha TEXT NOT NULL, - job TEXT NOT NULL REFERENCES jobs(id) ON DELETE CASCADE, - PRIMARY KEY(repository,sha) -); diff --git a/cloudflare/src/github/service.ts b/cloudflare/src/github/service.ts index 5fced929..155b33ef 100644 --- a/cloudflare/src/github/service.ts +++ b/cloudflare/src/github/service.ts @@ -843,20 +843,11 @@ export class GitHubService { comparison.headTarget.oid !== revision ) return false; - const published = new Set( - ( - await this.#env.DB.prepare( - "SELECT sha FROM job_commits WHERE repository=? AND sha IN (SELECT value FROM json_each(?))", - ) - .bind(repo.id, JSON.stringify(page.nodes.map((commit) => sha(commit.oid)))) - .all<{ sha: string }>() - ).results.map(({ sha }) => sha), - ); for (const commit of page.nodes) { if (commits.has(commit.oid)) throw new GitHubError("Repeated pull request commit"); commits.add(sha(commit.oid)); - // Only confirmed publications recorded by kix count, never agent claims or author email. - if (published.has(commit.oid)) continue; + // 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)) { diff --git a/cloudflare/src/jobs/hub.ts b/cloudflare/src/jobs/hub.ts index 8a5269cc..abd82314 100644 --- a/cloudflare/src/jobs/hub.ts +++ b/cloudflare/src/jobs/hub.ts @@ -129,8 +129,12 @@ export class RunnerHub extends DurableObject { const reported = (ws.deserializeAttachment() as Attachment).slots; if (reported !== null) slots.set(runner, reported); } - const { assign, cancel, ended, wake } = await store.dispatch(this.env.DB, owner, slots, Date.now(), (job) => - trust.verify(this.env, job), + 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) }; diff --git a/cloudflare/src/jobs/publish.ts b/cloudflare/src/jobs/publish.ts index 7bb7f377..2c81ca95 100644 --- a/cloudflare/src/jobs/publish.ts +++ b/cloudflare/src/jobs/publish.ts @@ -212,11 +212,7 @@ async function decide(db: D1Database, runner: Runner, input: Publication, prompt * commit whose outcome went unrecorded is adopted first, so an interrupted * publication is never mistaken for someone else's write. */ -export async function publicationHead( - db: D1Database, - job: Pick, - branch: Branch, -) { +async function verify(db: D1Database, job: store.Publishing, input: Publication, branch: Branch) { const current = await branch.head(); let head = job.head; if (current !== head && job.pending && current === job.pending) { @@ -229,6 +225,7 @@ export async function publicationHead( ? `${branch.repository.owner}/${branch.repository.name} already has the branch ${job.branch} from earlier work.` : `The branch ${job.branch} changed outside this job.`, ); + if (input.head !== head) throw new HeadMoved(head); return head; } @@ -277,8 +274,7 @@ export async function publish(env: Env, runner: Runner, input: Publication): Pro try { // A pull request opens from the branch, so a delivery alone still checks where it is. head = await github.contribute(upstream, job.actor, job, async (branch) => { - const head = await publicationHead(db, job, branch); - if (input.head !== head) throw new HeadMoved(head); + const head = await verify(db, job, input, branch); return input.changes.length || input.merge ? commit(db, runner, job, input, branch, head) : head; }); } catch (error) { diff --git a/cloudflare/src/jobs/store.ts b/cloudflare/src/jobs/store.ts index dbee968f..c513c33d 100644 --- a/cloudflare/src/jobs/store.ts +++ b/cloudflare/src/jobs/store.ts @@ -298,22 +298,8 @@ export function route(job: Job, offers: Offer[]): Offer | null { return eligible.find((offer) => offer.free && !offer.limited) ?? null; } -export type AssignmentSource = Pick< - Job, - | "id" - | "repository" - | "workflow" - | "target" - | "revision" - | "branch" - | "actor" - | "trusted" - | "head" - | "pending" - | "pull" - | "createdAt" ->; -export type VerifiedSource = { head: string | null; trusted: boolean }; +/** 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 }; @@ -332,7 +318,7 @@ export async function dispatch( owner: string, slots: Map, now: number, - verify: (job: AssignmentSource) => Promise = async (job) => ({ head: job.head, trusted: false }), + verify: Verify = async () => false, ) { const connected = JSON.stringify([...slots.keys()]); const [, , cancelled, , candidates, offers, cancels, pending] = await db.batch([ @@ -393,9 +379,7 @@ export async function dispatch( for (const job of candidates!.results as Job[]) { if (![...free.values()].some((n) => n > 0)) break; if (onPull(job.workflow)) { - const source = await verify(job); - job.head = source.head; - job.trusted = source.trusted ? 1 : 0; + job.trusted = (await verify(job)) ? 1 : 0; verified.add(job.id); await db .prepare("UPDATE jobs SET trusted=? WHERE id=? AND state='queued' AND attempt=? AND head IS ?") @@ -447,13 +431,7 @@ export async function dispatch( const job = await readJob(db, id); if (!job || job.state !== "running" || job.cancel) continue; if (onPull(job.workflow) && !verified.has(job.id)) { - const source = await verify(job); - job.head = source.head; - if (job.pending === source.head) { - job.pending = null; - job.pendingAt = null; - } - const trusted = source.trusted ? 1 : 0; + const trusted = (await verify(job)) ? 1 : 0; const machine = accounts.find((offer) => offer.runner === runner); const eligible = trusted || (machine?.platform === "linux" && machine.vm); const update = await db @@ -647,18 +625,12 @@ export async function publishing(db: D1Database, runner: Runner, job: string, at /** Record a pending commit that the branch shows landed after all. */ export async function adopt(db: D1Database, job: string, head: string | null, pending: string, now: number) { - await db.batch([ - db - .prepare( - "INSERT OR IGNORE INTO job_commits(repository,sha,job) SELECT repository,pending,id FROM jobs WHERE id=? AND head IS ? AND pending=?", - ) - .bind(job, head, pending), - db - .prepare( - "UPDATE jobs SET head=pending,pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND head IS ? AND pending=?", - ) - .bind(now, job, head, pending), - ]); + await db + .prepare( + "UPDATE jobs SET head=pending,pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND head IS ? AND pending=?", + ) + .bind(now, job, head, pending) + .run(); } /** @@ -687,24 +659,14 @@ export async function claim( /** Record whether the reserved branch update landed. The branch's head follows it either way. */ export async function settle(db: D1Database, job: string, commit: string, landed: boolean, now: number) { - await db.batch([ - ...(landed - ? [ - db - .prepare( - "INSERT OR IGNORE INTO job_commits(repository,sha,job) SELECT repository,pending,id FROM jobs WHERE id=? AND pending=?", - ) - .bind(job, commit), - ] - : []), - db - .prepare( - landed - ? "UPDATE jobs SET head=pending,pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND pending=?" - : "UPDATE jobs SET pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND pending=?", - ) - .bind(now, job, commit), - ]); + await db + .prepare( + landed + ? "UPDATE jobs SET head=pending,pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND pending=?" + : "UPDATE jobs SET pending=NULL,pendingAt=NULL,updatedAt=? WHERE id=? AND pending=?", + ) + .bind(now, job, commit) + .run(); } /** diff --git a/cloudflare/src/jobs/trust.ts b/cloudflare/src/jobs/trust.ts index 90614744..b04ef1db 100644 --- a/cloudflare/src/jobs/trust.ts +++ b/cloudflare/src/jobs/trust.ts @@ -1,22 +1,18 @@ import { GitHubService } from "../github/service"; -import { publicationHead } from "./publish"; -import type { AssignmentSource, VerifiedSource } from "./store"; +import type { Verify } from "./store"; -/** Recover only a confirmed, fenced publication before classifying a queued PR revision. */ -export async function verify(env: Env, job: AssignmentSource): Promise { - const untrusted = { head: job.head, trusted: false }; - 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 untrusted; - const github = new GitHubService(env); - try { - const head = job.pending - ? await github.contribute(repo, job.actor, job, (branch) => publicationHead(env.DB, job, branch)) - : job.head; - return { head, trusted: await github.jobTrust(repo, job.target, head ?? job.revision) }; - } catch { - // A failed read, changed branch, or unconfirmed publication never grants trust. - return untrusted; - } -} +/** 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 27195c15..b73ab172 100644 --- a/cloudflare/test/inbox.test.ts +++ b/cloudflare/test/inbox.test.ts @@ -97,10 +97,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, async (job) => ({ - head: job.head, - trusted: !!job.trusted, - })); + 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 14a499da..0c6c76ca 100644 --- a/cloudflare/test/jobs.test.ts +++ b/cloudflare/test/jobs.test.ts @@ -56,10 +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, async (job) => ({ - head: job.head, - trusted: !!job.trusted, - })); + 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(); @@ -1521,7 +1518,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/publish.test.ts b/cloudflare/test/publish.test.ts index a2ebc5fd..d6512af1 100644 --- a/cloudflare/test/publish.test.ts +++ b/cloudflare/test/publish.test.ts @@ -115,10 +115,7 @@ async function running(extra: Partial = {}, now = 0) { }, now, ); - await store.dispatch(db, user, new Map([[machine.id, 1]]), now, async (job) => ({ - head: job.head, - trusted: !!job.trusted, - })); + 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 index 4e73d715..4f6b9e7b 100644 --- a/cloudflare/test/trust.test.ts +++ b/cloudflare/test/trust.test.ts @@ -3,7 +3,7 @@ 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 as verifyAssignment } from "../src/jobs/trust"; +import { verify } from "../src/jobs/trust"; import { privateKey } from "./github-key"; const db = env.DB; @@ -15,7 +15,8 @@ const signature = (login = "maintainer", databaseId = 7) => ({ isValid: true, si const commit = (parents = [base]): Commit => ({ parents, tree, message: "Change", signature: signature() }); let repo: Repo; let github: typeof fetch; -const service = () => new GitHubService({ ...env, GITHUB_APP_PRIVATE_KEY: privateKey }); +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 () => { @@ -89,9 +90,7 @@ async function runner(name: string, platform: store.Platform, vm = false) { } const dispatch = (machines: store.Runner[]) => - store.dispatch(db, "owner", new Map(machines.map((r) => [r.id, 1])), 1, (job) => - verifyAssignment({ ...env, GITHUB_APP_PRIVATE_KEY: privateKey }, job), - ); + 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 () => { @@ -229,77 +228,29 @@ describe("authenticated PR provenance", () => { expect(await store.readJob(db, id)).toMatchObject({ trusted: 0, state: "queued" }); }); - it.each(["landed", "unconfirmed", "changed"])( - "reassigns safely after a %s pending publication loses its response", - async (outcome) => { - const id = await queued(); - const first = await runner("first", "linux"); - await dispatch([first]); - await store.report( - db, - first, - [{ id: "account", provider: "anthropic", kind: "subscription", label: "First", limited: false }], - [{ job: id, attempt: 1 }], - 1, - ); - const pending = "d".repeat(40); - repo.commits[pending] = { ...commit([head]), signature: null }; - await store.claim(db, first, id, 1, head, pending, 2); - if (outcome === "landed") repo.branches.fix = pending; - if (outcome === "changed") { - const other = "e".repeat(40); - repo.commits[other] = { ...commit([head]), signature: null }; - repo.branches.fix = other; - } - // Its disconnected runner's lease expires. Recovery must precede trust and routing. - const verify = (job: store.AssignmentSource) => - verifyAssignment({ ...env, GITHUB_APP_PRIVATE_KEY: privateKey }, job); - await store.dispatch(db, "owner", new Map(), 3, verify); - const replacement = await runner("replacement", "darwin"); - const pass = await store.dispatch(db, "owner", new Map([[replacement.id, 1]]), store.GRACE + 4, verify); - if (outcome === "changed") { - expect(pass.assign).toEqual([]); - expect(await store.readJob(db, id)).toMatchObject({ head, pending, trusted: 0, state: "queued" }); - } else { - const expected = outcome === "landed" ? pending : head; - expect(pass.assign).toMatchObject([ - { runner: replacement.id, job: { id, head: expected, trusted: 1, attempt: 2 } }, - ]); - expect(await store.readJob(db, id)).toMatchObject({ - head: expected, - pending: outcome === "landed" ? null : pending, - }); - } - expect((await db.prepare("SELECT sha FROM job_commits").all()).results).toEqual( - outcome === "landed" ? [{ sha: pending }] : [], - ); - }, - ); - - it("trusts confirmed kix publications, retaining earlier commits but refusing unconfirmed and failed pushes", async () => { + it("leaves a head kix published to VM-only, even after a lost response", async () => { const id = await queued(); - const machine = await runner("linux", "linux"); - await dispatch([machine]); + 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); - const next = "e".repeat(40); - for (const oid of [published, next]) repo.commits[oid] = { ...commit([base]), signature: null }; + 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); - await store.claim(db, machine, id, 1, head, published, 2); - await store.settle(db, id, published, false, 3); - expect(await trusted(published)).toBe(false); - await store.claim(db, machine, id, 1, head, published, 4); - await store.settle(db, id, published, true, 5); - expect(await trusted(published)).toBe(true); - repo.commits[next]!.parents = [published]; - repo.branches.fix = next; - await store.claim(db, machine, id, 1, published, next, 6); - // A confirmed update recovered after a crash records the same provenance. - await store.adopt(db, id, published, next, 7); - expect(await trusted(next)).toBe(true); - expect((await db.prepare("SELECT sha FROM job_commits ORDER BY sha").all()).results).toEqual([ - { sha: published }, - { sha: next }, - ]); + // 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 a149ed17..101b288d 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. From 4db733e28606b162a5c01f7b458cc8d5aa679dd8 Mon Sep 17 00:00:00 2001 From: Luke Curley <432854+kixelated@users.noreply.github.com> Date: Wed, 28 Oct 2026 11:19:56 -0700 Subject: [PATCH 7/7] fix(dispatch): recheck trust when the claimed head moves before delivery A lost publication can settle between a claim and the assignment read, moving the head with no attempt fence. Key the claim's verification to its attempt and head, and verify any other head again. Co-Authored-By: Claude Opus 5.5 --- cloudflare/src/jobs/store.ts | 9 +++++--- cloudflare/test/trust.test.ts | 41 +++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/cloudflare/src/jobs/store.ts b/cloudflare/src/jobs/store.ts index 6e1a6b6a..b545c8d9 100644 --- a/cloudflare/src/jobs/store.ts +++ b/cloudflare/src/jobs/store.ts @@ -373,14 +373,15 @@ export async function dispatch( if (free.has(runner)) free.set(runner, free.get(runner)! - n); type Row = Omit & { limited: number }; const accounts = offers!.results as Row[]; - const verified = new Set(); + // 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; - verified.add(job.id); 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) @@ -417,6 +418,7 @@ export async function dispatch( ]); // The job or its project changed underneath this pass; the next one sees it. if (update!.meta.changes !== 1) continue; + if (onPull(job.workflow)) verified.set(job.id, { attempt: job.attempt + 1, head: job.head }); free.set(chosen.runner, free.get(chosen.runner)! - 1); } const undelivered = await db @@ -430,7 +432,8 @@ export async function dispatch( for (const { id, runner, ...project } of undelivered.results) { const job = await readJob(db, id); if (!job || job.state !== "running" || job.cancel) continue; - if (onPull(job.workflow) && !verified.has(job.id)) { + const checked = verified.get(job.id); + if (onPull(job.workflow) && (checked?.attempt !== job.attempt || checked.head !== job.head)) { const trusted = (await verify(job)) ? 1 : 0; const machine = accounts.find((offer) => offer.runner === runner); const eligible = trusted || (machine?.platform === "linux" && machine.vm); diff --git a/cloudflare/test/trust.test.ts b/cloudflare/test/trust.test.ts index 4f6b9e7b..c2fa0e07 100644 --- a/cloudflare/test/trust.test.ts +++ b/cloudflare/test/trust.test.ts @@ -228,6 +228,47 @@ describe("authenticated PR provenance", () => { 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");