Diffinery

codician/diffinerypublic

Open-source, agent-first GitHub and GitHub Actions alternative that runs on your own Cloudflare account: git hosting, pull requests, code review and drop-in GitHub Actions YAML on Workers, Durable Objects, R2 and Containers.

Security audit, part 3: agent origin per commit (ACT-3), protected runs can't be cancelled from other branches (ACT-7) (#201)

* Security audit batch 3: per-commit agent origin (ACT-3), and protected runs can't be cancelled from other refs (ACT-7)
* E2E: approve the agent's pull request before the person merges it
test-bot authored 1 parent 8994af4commit 26c09b152037265bc1cbd6dfcb1905b028365c4dBrowse files
5 files changed+182−12
Files (5)
modifiedapps/diffinery/src/repo/RepoDO.ts+1−0View file
@@ -796,6 +796,7 @@
796796 };
797797 this.emit(ev);
798798 this.policy.recordPush(ref, newOid, caller);
799+ await this.policy.trackAgentCommits(ref, oldOid, newOid, caller).catch((e) => console.error("could not record an agent's commits", ref, e));
799800 this.deploys.noteRefUpdate(ref, newOid, caller);
800801 if (ref.startsWith("refs/heads/") && newOid !== ZERO_OID) {
801802 const branch = ref.slice(11);
modifiedapps/diffinery/src/repo/actions.ts+26−7View file
@@ -700,12 +700,13 @@
700700 concurrency_group: null,
701701 display_title: o.title,
702702 head_sha: o.headSha && o.headSha !== o.sha ? o.headSha : null,
703- agent_origin: this.repo.policy.agentOrigin({
703+ agent_origin: (await this.repo.policy.agentOrigin({
704704 caller: o.caller,
705705 ref: o.ref,
706+ sha: o.sha,
706707 prNumbers: [...(o.prNumber !== undefined ? [o.prNumber] : []), ...(o.prNumbers ?? [])],
707708 inherited: o.agentOrigin,
708- })
709+ }))
709710 ? 1
710711 : 0,
711712 prior_actors: o.rerun ? JSON.stringify(o.rerun.priorActors) : null,
@@ -744,8 +745,9 @@
744745 );
745746 if (run.concurrency_group && wf.concurrency) {
746747 const cancelInProgress = evaluateValue(wf.concurrency.cancelInProgress, ctx);
747- const others = this.sql.exec<{ id: string }>("SELECT id FROM runs WHERE concurrency_group = ? AND status != 'completed' AND id != ?", run.concurrency_group, id).toArray();
748- for (const other of others) if (cancelInProgress === true || cancelInProgress === "true") await this.cancelInternal(other.id, "cancelled by a newer run in the same concurrency group");
748+ const others = this.sql.exec<{ id: string; ref: string }>("SELECT id, ref FROM runs WHERE concurrency_group = ? AND status != 'completed' AND id != ?", run.concurrency_group, id).toArray();
749+ // A run the newcomer may not cancel stays, and the newcomer waits for it like any run in the group.
750+ for (const other of others) if ((cancelInProgress === true || cancelInProgress === "true") && this.mayCancel(run, other)) await this.cancelInternal(other.id, "cancelled by a newer run in the same concurrency group");
749751 }
750752 await this.env.RUN_WORKFLOW.create({ id, params: { repoId: this.repo.meta.repoId, runId: id } });
751753 const m = this.repo.meta;
@@ -1544,11 +1546,17 @@
15441546 */
15451547 private async concurrencySlot(job: JobRow): Promise<boolean> {
15461548 if (!job.concurrency_group) return true;
1547- const others = this.sql.exec<JobRow>("SELECT * FROM jobs WHERE concurrency_group = ? AND id != ? AND status IN ('pending', 'waiting', 'in_progress') ORDER BY rowid", job.concurrency_group, job.id).toArray();
1548- const pending = others.filter((o) => o.status === "pending");
1549+ type Other = JobRow & { run_ref: string };
1550+ const others = this.sql
1551+ .exec<Other>("SELECT j.*, r.ref AS run_ref FROM jobs j JOIN runs r ON r.id = j.run_id WHERE j.concurrency_group = ? AND j.id != ? AND j.status IN ('pending', 'waiting', 'in_progress') ORDER BY j.rowid", job.concurrency_group, job.id)
1552+ .toArray();
1553+ const mine = this.sql.exec<{ ref: string }>("SELECT ref FROM runs WHERE id = ?", job.run_id).toArray()[0];
1554+ // Jobs this one may replace or cancel (ACT-7); the rest it waits behind.
1555+ const cancellable = (o: Other) => !mine || this.mayCancel(mine, { ref: o.run_ref });
1556+ const pending = others.filter((o) => o.status === "pending" && cancellable(o));
15491557 const running = others.filter((o) => o.status !== "pending");
15501558 const failure = { message: `Canceling since a higher priority waiting request for '${job.concurrency_group}' exists` };
1551- if (running.length && !job.concurrency_cancel) {
1559+ if (running.length && (!job.concurrency_cancel || !running.every(cancellable))) {
15521560 this.sql.exec("UPDATE jobs SET status = 'pending' WHERE id = ?", job.id);
15531561 this.repo.broadcast(`run:${job.run_id}`, { action: "job", jobId: job.id });
15541562 for (const o of pending) await this.completeJob(o.id, { conclusion: "cancelled", outputs: {}, failure });
@@ -1563,6 +1571,17 @@
15631571 return true;
15641572 }
15651573
1574+ /**
1575+ * Whether a run of `by` may cancel or replace `other` in a shared concurrency group. Groups are
1576+ * repository-wide, as on GitHub, but a workflow pushed to some branch could otherwise name a
1577+ * deploy's group and cancel it halfway: a run on a protected branch is cancelled only by runs on
1578+ * that same branch (ACT-7).
1579+ */
1580+ private mayCancel(by: { ref: string }, other: { ref: string }): boolean {
1581+ if (by.ref === other.ref) return true;
1582+ return !(other.ref.startsWith("refs/heads/") && this.repo.protectionFor(other.ref.slice(11)));
1583+ }
1584+
15661585 /** Whether jobs wait on the account's hosted-job quota (RepoDO retries them on its alarm). */
15671586 hasAccountHeldJobs(): boolean {
15681587 return this.sql.exec<{ n: number }>("SELECT count(*) AS n FROM jobs WHERE fleet_held = 2 AND status = 'queued'").one().n > 0;
modifiedapps/diffinery/src/repo/schema.ts+3−0View file
@@ -145,6 +145,9 @@
145145 -- Who last moved each ref: tags made by agents never match environment tag patterns
146146 -- (./deployments.ts), and runs of code an agent pushed act as the agent (./token-policy.ts).
147147 CREATE TABLE IF NOT EXISTS ref_actors (ref TEXT PRIMARY KEY, actor_id TEXT NOT NULL, actor_kind TEXT NOT NULL, updated_at TEXT NOT NULL);
148+-- Commits an agent pushed that aren't on the default branch yet: a run of code that includes one acts
149+-- as an agent, whoever pushed last or made the tag (./token-policy.ts). Dropped once they land.
150+CREATE TABLE IF NOT EXISTS agent_commits (oid TEXT PRIMARY KEY, pushed_at TEXT NOT NULL);
148151 -- GitHub imports and milestones (see ./imports.ts).
149152 CREATE TABLE IF NOT EXISTS milestones (
150153 number INTEGER PRIMARY KEY, title TEXT NOT NULL, description TEXT NOT NULL DEFAULT '', state TEXT NOT NULL DEFAULT 'open',
modifiedapps/diffinery/src/repo/token-policy.ts+83−5View file
@@ -3,6 +3,9 @@
33 import type { RepoDO } from "./RepoDO.js";
44 import type { Caller } from "./types.js";
55
6+/** How far back from a ref's tip to look for (or record) agents' commits. */
7+const AGENT_WALK = 5000;
8+
69 /**
710 * What a workflow's GITHUB_TOKEN is trusted with, beyond its `permissions:`
811 * (lib/job-permissions.ts):
@@ -79,24 +82,99 @@
7982 else this.sql.exec("INSERT OR REPLACE INTO ref_actors (ref, actor_id, actor_kind, updated_at) VALUES (?, ?, ?, ?)", ref, caller.id, caller.kind, now());
8083 }
8184
85+ /**
86+ * Keeps track of commits agents pushed that aren't on the default branch yet. An agent's push
87+ * records the commits it brought (up to the default branch or the ref's old tip); the default
88+ * branch moving forward drops the ones that landed, since its runs go by who moved it.
89+ */
90+ async trackAgentCommits(ref: string, oldOid: string, newOid: string, caller: Caller): Promise<void> {
91+ if (newOid === ZERO_OID) return;
92+ const isDefault = ref === this.repo.headRef;
93+ if (!isDefault && caller.kind !== "agent") return;
94+ if (isDefault && !this.sql.exec("SELECT 1 FROM agent_commits LIMIT 1").toArray().length) return;
95+ const tip = await peelToCommit(this.repo.store, newOid).catch(() => null);
96+ if (!tip) return;
97+ const stop = [oldOid, isDefault ? null : await this.forkPoint(tip)].filter((o): o is string => !!o && o !== ZERO_OID);
98+ const fresh = await this.commitsSince(tip, stop, AGENT_WALK);
99+ const at = now();
100+ for (const oid of fresh) {
101+ if (isDefault) this.sql.exec("DELETE FROM agent_commits WHERE oid = ?", oid);
102+ else this.sql.exec("INSERT OR IGNORE INTO agent_commits (oid, pushed_at) VALUES (?, ?)", oid, at);
103+ }
104+ this.agentCodeSeen.clear();
105+ }
106+
107+ /** Where `tip` meets the default branch: the commits past it are the ref's own. */
108+ private async forkPoint(tip: string): Promise<string | null> {
109+ const head = this.repo.getRef(this.repo.headRef);
110+ return head ? await mergeBase(this.repo.store, head, tip).catch(() => null) : null;
111+ }
112+
113+ /** Commits reachable from `tip`, nearest first (by parents, not dates, which a pusher sets), short of `stop`. */
114+ private async commitsSince(tip: string, stop: string[], limit: number): Promise<string[]> {
115+ const seen = new Set(stop);
116+ const out: string[] = [];
117+ let level = [tip];
118+ while (level.length && out.length < limit) {
119+ const next: string[] = [];
120+ for (const oid of level) {
121+ if (seen.has(oid) || out.length >= limit) continue;
122+ seen.add(oid);
123+ const c = await readCommit(this.repo.store, oid).catch(() => null);
124+ if (!c) continue; // shallow or missing parents
125+ out.push(oid);
126+ next.push(...c.parents);
127+ }
128+ level = next;
129+ }
130+ return out;
131+ }
132+
133+ /** Whether the code at `sha` includes a commit an agent pushed that hasn't landed on the default branch. */
134+ private async includesAgentCommit(sha: string): Promise<boolean> {
135+ if (!this.sql.exec("SELECT 1 FROM agent_commits LIMIT 1").toArray().length) return false;
136+ const cached = this.agentCodeSeen.get(sha);
137+ if (cached !== undefined) return cached;
138+ const tip = await peelToCommit(this.repo.store, sha).catch(() => null);
139+ let found = false;
140+ if (tip) {
141+ const base = await this.forkPoint(tip);
142+ for (const oid of await this.commitsSince(tip, base ? [base] : [], AGENT_WALK)) {
143+ if (this.sql.exec("SELECT 1 FROM agent_commits WHERE oid = ?", oid).toArray().length) {
144+ found = true;
145+ break;
146+ }
147+ }
148+ }
149+ if (this.agentCodeSeen.size > 200) this.agentCodeSeen.clear();
150+ this.agentCodeSeen.set(sha, found);
151+ return found;
152+ }
153+
154+ /** includesAgentCommit's answers, per commit, until the next agent push or default branch move. */
155+ private agentCodeSeen = new Map<string, boolean>();
156+
82157 lastPusherKind(ref: string): string | null {
83158 return this.sql.exec<{ actor_kind: string }>("SELECT actor_kind FROM ref_actors WHERE ref = ?", ref).toArray()[0]?.actor_kind ?? null;
84159 }
85160
86161 /**
87162 * Whether a new run's code came from an agent: the trigger was an agent, the run
88- * re-runs one that did (`inherited`), the branch it runs was last pushed by an
89- * agent, or, for pull request events, an agent opened the pull request or last
90- * pushed its head branch.
163+ * re-runs one that did (`inherited`), the branch or tag it runs was last moved by an
164+ * agent, for pull request events an agent opened the pull request or last pushed its
165+ * head branch, or, off the default branch, its code includes a commit an agent pushed
166+ * (so a person's push, merge, release tag or dispatch on top doesn't make it theirs).
91167 */
92- agentOrigin(o: { caller: { kind: string }; ref: string; prNumbers?: number[]; inherited?: boolean }): boolean {
168+ async agentOrigin(o: { caller: { kind: string }; ref: string; sha?: string; prNumbers?: number[]; inherited?: boolean }): Promise<boolean> {
93169 if (o.inherited || o.caller.kind === "agent") return true;
94- if (o.ref.startsWith("refs/heads/") && this.lastPusherKind(o.ref) === "agent") return true;
170+ if ((o.ref.startsWith("refs/heads/") || o.ref.startsWith("refs/tags/")) && this.lastPusherKind(o.ref) === "agent") return true;
95171 for (const n of o.prNumbers ?? []) {
96172 const pr = this.sql.exec<{ author_kind: string; head_ref: string }>("SELECT i.author_kind, p.head_ref FROM issues i JOIN pulls p ON p.number = i.number WHERE i.number = ?", n).toArray()[0];
97173 if (!pr) continue;
98174 if (pr.author_kind === "agent" || this.lastPusherKind(`refs/heads/${pr.head_ref}`) === "agent") return true;
99175 }
176+ // The default branch's own runs go by who moved it: what a person merged there is theirs.
177+ if (o.sha && o.ref !== this.repo.headRef && (await this.includesAgentCommit(o.sha))) return true;
100178 return false;
101179 }
102180
modifiedapps/diffinery/test/e2e/security.test.mjs+69−0View file
@@ -1809,6 +1809,75 @@
18091809 assert.deepEqual(await secretsOfNextJob(), ["DEPLOY", "DIFFINERY_TOKEN", "GITHUB_TOKEN"]);
18101810 });
18111811
1812+test("security: an agent's commits stay agent code when a person pushes on top, until a person merges them", async () => {
1813+ const R = `/repos/${OWNER}/agent-laundry`;
1814+ const mainSha = await repoWithReadme("agent-laundry");
1815+ await status(204, "PUT", `${R}/collaborators/olive-bot`, { permission: "write" });
1816+ await status(204, "PUT", `${R}/actions/secrets/DEPLOY`, { value: "deploy-value" });
1817+ const wf = "name: ci\non: push\njobs:\n build:\n runs-on: [self-hosted, linux]\n steps:\n - run: echo hi\n env:\n T: ${{ secrets.DEPLOY }}\n";
1818+ const secretsOfNextJob = async () => {
1819+ let names;
1820+ await until("the next job", async () => {
1821+ const r = await runnerClaim(R, ["linux"]);
1822+ if (r.status !== 200) return false;
1823+ const spec = await (await fetch(`${base}/runner/jobs/${r.json.job_id}`, { headers: { authorization: `Bearer ${r.json.token}` } })).json();
1824+ names = Object.keys(spec.secrets).sort();
1825+ await fetch(`${base}/runner/jobs/${r.json.job_id}/complete`, { method: "POST", headers: { authorization: `Bearer ${r.json.token}`, "content-type": "application/json" }, body: JSON.stringify({ conclusion: "success", outputs: {} }) });
1826+ return true;
1827+ });
1828+ return names;
1829+ };
1830+ // The agent's commit asks for the secret: its own push runs without it.
1831+ await ok("POST", `${R}/git/refs`, { ref: "refs/heads/agent-work", sha: mainSha }, T.bot);
1832+ await ok("PUT", `${R}/contents/.github/workflows/ci.yml`, { message: "ci", content: b64(wf), branch: "agent-work" }, T.bot);
1833+ assert.deepEqual(await secretsOfNextJob(), ["DIFFINERY_TOKEN", "GITHUB_TOKEN"]);
1834+ // A person pushing on top is the branch's last pusher, but the code still includes the agent's commit (ACT-3).
1835+ await ok("PUT", `${R}/contents/notes.txt`, { message: "notes", content: b64("notes\n"), branch: "agent-work" });
1836+ assert.deepEqual(await secretsOfNextJob(), ["DIFFINERY_TOKEN", "GITHUB_TOKEN"]);
1837+ // A person merging it makes it theirs: the default branch's runs go by who moved it.
1838+ const pr = await ok("POST", `${R}/pulls`, { title: "Agent work", head: "agent-work", base: "main" }, T.bot);
1839+ // An agent's pull request needs a person's approval before a person merges it.
1840+ await ok("POST", `${R}/pulls/${pr.number}/reviews`, { event: "APPROVE" });
1841+ await ok("PUT", `${R}/pulls/${pr.number}/merge`, { merge_method: "merge" });
1842+ assert.deepEqual(await secretsOfNextJob(), ["DEPLOY", "DIFFINERY_TOKEN", "GITHUB_TOKEN"]);
1843+ // A person's new branch from there has no agent commits of its own.
1844+ const tip = (await ok("GET", `${R}/branches/main`)).commit.sha;
1845+ await ok("POST", `${R}/git/refs`, { ref: "refs/heads/person-work", sha: tip });
1846+ await ok("PUT", `${R}/contents/more.txt`, { message: "more", content: b64("more\n"), branch: "person-work" });
1847+ assert.deepEqual(await secretsOfNextJob(), ["DEPLOY", "DIFFINERY_TOKEN", "GITHUB_TOKEN"]);
1848+});
1849+
1850+test("security: a workflow on another branch can't cancel a protected branch's run by naming its concurrency group", async () => {
1851+ const R = `/repos/${OWNER}/deploy-group`;
1852+ await repoWithReadme("deploy-group");
1853+ await ok("PUT", `${R}/branches/main/protection`, { requireLinearHistory: true });
1854+ const deploy = "name: deploy\non:\n push:\n branches: [main]\nconcurrency:\n group: production\n cancel-in-progress: false\njobs:\n deploy:\n runs-on: [self-hosted, linux]\n steps:\n - run: echo deploying\n";
1855+ await ok("PUT", `${R}/contents/.github/workflows/deploy.yml`, { message: "deploy", content: b64(deploy) });
1856+ let claim;
1857+ await until("the deploy job", async () => {
1858+ const r = await runnerClaim(R, ["linux"]);
1859+ if (r.status === 200) claim = r.json;
1860+ return !!claim;
1861+ });
1862+ const runOf = async (path) => (await ok("GET", `${R}/actions/runs`)).workflow_runs.find((w) => w.path === path);
1863+ const deployRun = await runOf(".github/workflows/deploy.yml");
1864+ // A branch's workflow names the same group with cancel-in-progress (ACT-7).
1865+ const rogue = "name: rogue\non: push\nconcurrency:\n group: production\n cancel-in-progress: true\njobs:\n noop:\n runs-on: [self-hosted, other]\n steps:\n - run: echo hi\n";
1866+ await ok("POST", `${R}/git/refs`, { ref: "refs/heads/rogue", sha: (await ok("GET", `${R}/branches/main`)).commit.sha });
1867+ await ok("PUT", `${R}/contents/.github/workflows/rogue.yml`, { message: "rogue", content: b64(rogue), branch: "rogue" });
1868+ await until("the rogue run", async () => !!(await runOf(".github/workflows/rogue.yml")));
1869+ await new Promise((r) => setTimeout(r, 2000));
1870+ const still = await ok("GET", `${R}/actions/runs/${deployRun.id}`);
1871+ assert.notEqual(still.conclusion, "cancelled", "the deploy run was cancelled from another branch");
1872+ assert.equal(still.status, "in_progress");
1873+ await fetch(`${base}/runner/jobs/${claim.job_id}/complete`, { method: "POST", headers: { authorization: `Bearer ${claim.token}`, "content-type": "application/json" }, body: JSON.stringify({ conclusion: "success", outputs: {} }) });
1874+ const done = await until("the deploy run to finish", async () => {
1875+ const r = await ok("GET", `${R}/actions/runs/${deployRun.id}`);
1876+ return r.status === "completed" ? r : null;
1877+ });
1878+ assert.equal(done.conclusion, "success");
1879+});
1880+
18121881 test("security: adding someone by email always invites them, so the address doesn't reveal their username and they agree first", async () => {
18131882 await ok("POST", "/user/orgs", { login: "by-mail" });
18141883 const members = async () => (await ok("GET", "/orgs/by-mail/members")).map((u) => u.login);