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 1: forged job tokens, retired names, tokens on web pages, review and jail hardening (#187)

* WIP: retire logins whose old URLs still redirect
* Security audit batch 1: forged job tokens, retired names, tokens on web pages, agent environment secrets, jail /run, review config, provider tools
* Web pages: only limited tokens are treated as signed out outside repository pages
test-bot authored 1 parent effb7cacommit 1e9b201621af5e94bc0650cdc3703c4e533ac19dBrowse files
21 files changed+170−33
Files (21)
modifiedapps/diffinery/src/agentic/ProjectDO.ts+1−1View file
@@ -1362,7 +1362,7 @@
13621362 const token = `dmt_${t.repo_id}_${randomToken(32)}`;
13631363 this.sql.exec("UPDATE runs SET token_hash = ?, checked_at = ? WHERE task = ? AND seq = ?", await sha256(token), now(), r.task, r.seq);
13641364 const { env, secret } = agentEnv(JAIL_MODEL_URL, providers, JAIL_KEY, { runtime: t.runtime, route: { via: t.via, model: t.model } });
1365- const cmd = t.runtime === "custom" ? 'bash -c "$DIFFINERY_COMMAND"' : RUNTIMES[t.runtime].command(undefined, t.via ?? undefined, t.kind !== "task");
1365+ const cmd = t.runtime === "custom" ? 'bash -c "$DIFFINERY_COMMAND"' : RUNTIMES[t.runtime].command(undefined, t.via ?? undefined, t.kind !== "task", t.kind === "review");
13661366 return this.startProc(
13671367 r,
13681368 "agent",
modifiedapps/diffinery/src/agentic/api.ts+2−0View file
@@ -54,6 +54,8 @@
5454 const me = requirePersonSession(c);
5555 const provider = c.req.param("provider");
5656 if (!isProvider(provider)) throw new HttpError(404, "unknown provider: use anthropic, openai or openrouter");
57+ // Swapping in another key sends the person's later prompts and code to that key's account: a sign-in from the last 2 hours, as for organizations' keys.
58+ requireRecentSignIn(c, "/settings/model-access");
5759 const b = await jsonBody<{ key?: unknown; accept_terms?: unknown }>(c);
5860 if (typeof b.key !== "string") throw new HttpError(422, "`key` is required");
5961 const k = await new ModelKeys(c.env).set(me, provider, b.key, b.accept_terms === true);
modifiedapps/diffinery/src/agentic/proxy.ts+1−0View file
@@ -120,6 +120,7 @@
120120 // monthly allowance depends on them) send the capped, tool-stripped body their booking assumes.
121121 const limited = run.bounded || run.orgKey;
122122 if (limited && prepared.reviewBody) body = prepared.reviewBody;
123+ else if (prepared.jailBody) body = prepared.jailBody;
123124 // A booked request that never reaches the provider, or that it answers with an error, costs nothing.
124125 // `spend` is the booking against the person's monthly allowance on an organization's key, once made.
125126 let spend: { org: string; tokens: number; month: string } | null = null;
modifiedapps/diffinery/src/agentic/runtimes.ts+7−4View file
@@ -29,7 +29,7 @@
2929 * The run itself; the prompt is in $DIFFINERY_TASK_PROMPT and an OpenRouter
3030 * model in $DIFFINERY_MODEL, never on the command line we build.
3131 */
32- command: (custom?: string, via?: Provider, review?: boolean) => string;
32+ command: (custom?: string, via?: Provider, review?: boolean, untrustedCheckout?: boolean) => string;
3333 }
3434
3535 export const RUNTIMES: Record<RuntimeId, Runtime> = {
@@ -43,7 +43,9 @@
4343 install: `npm install --global --no-fund --no-audit --loglevel=error @anthropic-ai/claude-code@${RUNTIME_VERSIONS["claude-code"]} && claude --version`,
4444 // The sandbox runs as root in its own container; IS_SANDBOX lets Claude Code skip permission prompts there.
4545 // A review has no web tools: they run on the provider and cost extra (the model proxy strips them too).
46- command: (_custom, _via, review) => `claude -p "$DIFFINERY_TASK_PROMPT" --output-format stream-json --verbose --permission-mode bypassPermissions${review ? ' --disallowedTools "WebSearch,WebFetch"' : ""}`,
46+ // A review runs in the pull request's own checkout, which may add CLAUDE.md, .claude/settings.json, hooks or
47+ // MCP servers telling it to find nothing: --safe-mode loads none of the repository's customizations.
48+ command: (_custom, _via, review, untrusted) => `claude -p "$DIFFINERY_TASK_PROMPT" --output-format stream-json --verbose --permission-mode bypassPermissions${review ? ' --disallowedTools "WebSearch,WebFetch"' : ""}${untrusted ? " --safe-mode" : ""}`,
4749 },
4850 codex: {
4951 id: "codex",
@@ -53,8 +55,9 @@
5355 alternatives: ["openrouter"],
5456 install: `npm install --global --no-fund --no-audit --loglevel=error @openai/codex@${RUNTIME_VERSIONS.codex} && codex --version`,
5557 // Through OpenRouter: a model provider for its OpenAI-compatible Responses API, defined on the command line.
56- command: (_custom, via, review) =>
57- `codex exec --json --dangerously-bypass-approvals-and-sandbox --skip-git-repo-check${review ? " -c web_search=disabled" : ""}${
58+ // In a pull request's own checkout, its AGENTS.md isn't read (project_doc_max_bytes=0): see claude-code above.
59+ command: (_custom, via, review, untrusted) =>
60+ `codex exec --json --dangerously-bypass-approvals-and-sandbox --skip-git-repo-check${review ? " -c web_search=disabled" : ""}${untrusted ? " -c project_doc_max_bytes=0" : ""}${
5861 via === "openrouter"
5962 ? ' -c model_provider=openrouter -c model_providers.openrouter.name=OpenRouter -c "model_providers.openrouter.base_url=$OPENROUTER_BASE_URL" -c model_providers.openrouter.env_key=OPENROUTER_API_KEY -c model_providers.openrouter.wire_api=responses -m "$DIFFINERY_MODEL"'
6063 : ""
modifiedapps/diffinery/src/agentic/tasks-web.tsx+3−1View file
@@ -1,6 +1,6 @@
11 /** @jsxImportSource hono/jsx */
22 import type { FC } from "hono/jsx";
3-import { requireAccount, type AppContext } from "../auth/principal.js";
3+import { requireAccount, type AppContext, requireRecentSignIn } from "../auth/principal.js";
44 import { loadRepo } from "../http/common.js";
55 import { HttpError } from "../lib/util.js";
66 import { SettingsNav } from "../web/account.js";
@@ -668,6 +668,8 @@
668668 const me = requireAccount(c);
669669 const provider = prm(c, "provider");
670670 if (!isProvider(provider)) throw new HttpError(404, "not found");
671+ // Another key would send this person's later prompts and code to its account: a recent sign-in, as for organizations' keys.
672+ requireRecentSignIn(c, "/settings/model-access");
671673 return act(
672674 c,
673675 "/settings/model-access",
modifiedapps/diffinery/src/agentic/usage.ts+6−0View file
@@ -242,6 +242,11 @@
242242 reviewRefuse?: string;
243243 /** The body a run held to its bookings sends instead: every output cap set to the booked one and no paid provider-side tools, so the estimate holds. */
244244 reviewBody?: string;
245+ /**
246+ * The body any other run sends when the request asks for provider-run tools (MCP servers, web fetch or search):
247+ * the provider would reach hosts on the run's behalf, past the jail's allowlist, so they are dropped.
248+ */
249+ jailBody?: string;
245250 /** The model the request asks for (null when it names none), for runs pinned to one. */
246251 model?: string | null;
247252 /** A routing field the request sets, which a pinned run may not. */
@@ -414,6 +419,7 @@
414419 const set = caps.filter((k) => x[k] !== undefined);
415420 const recap = caps[0] && (!set.length || set.some((k) => x[k] !== estimate.output));
416421 const unpaid = withoutPaidTools(fields);
422+ if (unpaid !== fields) out.jailBody = JSON.stringify(unpaid);
417423 const capTo = (f: Record<string, unknown>) => ({ ...f, ...Object.fromEntries((set.length ? set : [caps[0]!]).map((k) => [k, estimate.output])) });
418424 // Always written out again, so a key the request repeats reaches the provider once, as read here.
419425 out.reviewBody = JSON.stringify(recap ? capTo(unpaid) : unpaid);
modifiedapps/diffinery/src/auth/principal.ts+9−2View file
@@ -201,8 +201,15 @@
201201 // job, sandbox and runner tokens stop working (lib/moderation.ts).
202202 if (repo.owner_suspended_at && !(p.type === "account" && p.account.site_admin)) return null;
203203 if (p.type === "repo-token") {
204- // Like GITHUB_TOKEN, a job or sandbox token reads other repositories only when they are public.
205- if (p.repoId !== repo.id) return repo.visibility === "public" ? { id: "anonymous", login: "anonymous", kind: "user", role: "read" } : null;
204+ // Like GITHUB_TOKEN, a job or sandbox token reads other repositories only when they are public,
205+ // and only once its own repository has vouched for it: a made-up token must not pass as a
206+ // credential (it would skip the anonymous rate limit). Reads only: loadRepo refuses writes.
207+ if (p.repoId !== repo.id) {
208+ if (repo.visibility !== "public") return null;
209+ const own = await dir.repoById(p.repoId);
210+ if (!own || !(await c.env.REPO.get(c.env.REPO.idFromName(own.id)).runnerAuth(p.token))) throw new HttpError(401, "invalid, expired or revoked token");
211+ return { id: "anonymous", login: "anonymous", kind: "user", role: "read" };
212+ }
206213 const stub = c.env.REPO.get(c.env.REPO.idFromName(repo.id));
207214 const auth = await stub.runnerAuth(p.token);
208215 if (!auth) return null;
modifiedapps/diffinery/src/http/api.ts+4−1View file
@@ -237,6 +237,8 @@
237237 api.post("/user/orgs", async (c) => {
238238 requireAccountScope(c, "admin");
239239 const a = requireAccount(c);
240+ // An organization is a person's: an agent that owned one could send its invitations, past its owner's say.
241+ if (a.kind !== "user") throw new HttpError(403, "only people can create organizations");
240242 const b = await body<{ login: string; name?: unknown }>(c);
241243 if (typeof b.login !== "string") throw new HttpError(422, "login is required", { errors: [{ resource: "Organization", code: "missing_field", field: "login" }] });
242244 if (b.name !== undefined && b.name !== null && typeof b.name !== "string") throw new HttpError(422, "name must be a string", { errors: [{ resource: "Organization", code: "invalid", field: "name" }] });
@@ -452,7 +454,8 @@
452454 return c.json(repoOut(c, moved, (await dir.roleFor(p.account, moved)) ?? "read"), 202);
453455 });
454456 api.get("/repos/:owner/:repo/collaborators", async (c) => {
455- const { repo } = await loadRepo(c, P(c).owner, P(c).repo);
457+ // Who has which role is for people with push access, as on GitHub, not for every reader of a public repository.
458+ const { repo } = await loadRepo(c, P(c).owner, P(c).repo, "write");
456459 return c.json(paginate(c, (await c.get("dir").collaborators(repo.id)).map((x) => ({ ...userJson(c, x.account), permissions: permissionsOf(x.role), role_name: x.role }))));
457460 });
458461 api.put("/repos/:owner/:repo/collaborators/:login", async (c) => {
modifiedapps/diffinery/src/http/common.ts+2−0View file
@@ -67,6 +67,8 @@
6767 if (!repo) throw hidden();
6868 const caller = await callerFor(c, repo);
6969 if (!caller) throw hidden();
70+ // A job or sandbox token reading another public repository acts as nobody in particular: it may read, never write.
71+ if (caller.id === "anonymous" && !reading) throw new HttpError(403, "this token can only write to its own repository");
7072 if (moved && !opts.follow) {
7173 const url = new URL(c.req.url);
7274 const path = movedPath(url.pathname, { owner, name }, { owner: repo.owner_login, name: repo.name });
modifiedapps/diffinery/src/lib/directory.ts+30−7View file
@@ -131,15 +131,35 @@
131131 return r?.available_at ?? null;
132132 }
133133
134- /** Whether an account has this login, or a deleted account had it too recently for anyone to take it. */
134+ /**
135+ * Whether old URLs at this login still lead somewhere else: a renamed account's profile, or
136+ * repositories and images that moved away with a transfer, a rename or before their organization
137+ * was deleted. Whoever took the login would take over those redirects and could serve their own
138+ * code at the old clone URLs, so it stays retired. `forAccountId` may take back its own old login.
139+ */
140+ async loginRetired(login: string, forAccountId = ""): Promise<boolean> {
141+ const r = await this.db
142+ .prepare(
143+ `SELECT 1 FROM login_redirects WHERE login = ?1 AND account_id != ?2
144+ UNION ALL SELECT 1 FROM repo_redirects r JOIN repos p ON p.id = r.repo_id WHERE r.owner_login = ?1 AND p.owner_id != ?2
145+ UNION ALL SELECT 1 FROM package_redirects r JOIN packages p ON p.id = r.package_id WHERE r.owner_login = ?1 AND p.owner_id != ?2
146+ LIMIT 1`,
147+ )
148+ .bind(login, forAccountId)
149+ .first();
150+ return !!r;
151+ }
152+
153+ /** Whether an account has this login, a deleted account had it too recently for anyone to take it, or old URLs at it lead elsewhere. */
135154 async loginTaken(login: string): Promise<boolean> {
136- return !!(await this.accountByLogin(login)) || !!(await this.loginCooldown(login));
155+ return !!(await this.accountByLogin(login)) || !!(await this.loginCooldown(login)) || (await this.loginRetired(login));
137156 }
138157
139- /** Refuse a login that a deleted account gave up less than 90 days ago. */
140- private async checkCooldown(login: string, what = "This username"): Promise<void> {
158+ /** Refuse a login that a deleted account gave up less than 90 days ago, or whose old URLs lead to another account's work. */
159+ private async checkCooldown(login: string, what = "This username", forAccountId = ""): Promise<void> {
141160 const until = await this.loginCooldown(login);
142161 if (until) throw new HttpError(422, `${what} belonged to an account that was deleted recently; it can be used again from ${until.slice(0, 10)}`);
162+ if (await this.loginRetired(login, forAccountId)) throw new HttpError(422, `${what} is retired: links to it still lead to repositories or a profile that moved elsewhere`);
143163 }
144164
145165 /**
@@ -176,7 +196,7 @@
176196 if (patch.login) {
177197 const clash = await this.accountByLogin(patch.login);
178198 if (clash && clash.id !== id) throw new HttpError(422, "login already taken");
179- await this.checkCooldown(patch.login, "This name");
199+ await this.checkCooldown(patch.login, "This name", id);
180200 }
181201 // A blank display name clears it, so pages fall back to the login.
182202 const name = patch.name === undefined ? undefined : (patch.name as string | null)?.trim() || null;
@@ -476,8 +496,11 @@
476496 if (!row || row.revoked_at || row.expires_at! < now()) return null;
477497 const account = await this.account(row.account_id!);
478498 if (!account || account.status !== "active") return null;
479- // An agent's tokens stop working while its owner is suspended (lib/moderation.ts).
480- if (account.kind === "agent" && account.owner_id && (await this.account(account.owner_id))?.suspended_at) return null;
499+ // An agent's tokens stop working while its owner is suspended (lib/moderation.ts), disabled or otherwise not active.
500+ if (account.kind === "agent" && account.owner_id) {
501+ const owner = await this.account(account.owner_id);
502+ if (!owner || owner.suspended_at || owner.status !== "active") return null;
503+ }
481504 if (!row.last_used_at || Date.now() - Date.parse(row.last_used_at) > 3600_000) {
482505 await this.db.prepare("UPDATE tokens SET last_used_at = ? WHERE id = ?").bind(now(), row.id!).run();
483506 }
modifiedapps/diffinery/src/mcp/server.ts+5−1View file
@@ -620,7 +620,11 @@
620620 const content = [{ type: "text", text: JSON.stringify(out, null, 2) }, ...(UNTRUSTED_TOOLS.has(name) ? [{ type: "text", text: UNTRUSTED_NOTE }] : [])];
621621 return { jsonrpc: "2.0", id: req.id, result: { content, structuredContent: out && typeof out === "object" && !Array.isArray(out) ? out : { result: out }, isError: false } };
622622 } catch (e) {
623- const msg = e instanceof HttpError ? e.detail : String((e as Error)?.message ?? e).replace(/^\d{3}: /, "");
623+ // HttpErrors (including those a Durable Object threw, which arrive as "NNN: detail") are meant for the caller;
624+ // anything else (D1, storage, a failed fetch) is logged and answered generically, as the REST API does.
625+ const raw = String((e as Error)?.message ?? e);
626+ const msg = e instanceof HttpError ? e.detail : /^\d{3}: /.test(raw) ? raw.slice(5) : "internal error";
627+ if (!(e instanceof HttpError) && !/^\d{3}: /.test(raw)) console.error("mcp tool failed", name, e);
624628 return { jsonrpc: "2.0", id: req.id, result: { content: [{ type: "text", text: msg }], isError: true } };
625629 }
626630 }
modifiedapps/diffinery/src/registry/api.ts+12−2View file
@@ -98,6 +98,14 @@
9898 ["/orgs/:owner/packages", (c) => ownerAccount(c, c.req.param("owner")!, "org")],
9999 ];
100100
101+/**
102+ * A workflow's or sandbox's token pushes to its repository's packages and deletes their versions, as on GitHub,
103+ * but never makes one public or deletes it outright: that takes a person (or agent) with admin access.
104+ */
105+function workflowTokensMayNot(c: AppContext, what: string): void {
106+ if (c.get("principal").type === "repo-token") throw new HttpError(403, `a workflow's token can't ${what}; someone with admin access to the package can`);
107+}
108+
101109 for (const [prefix, ownerOf] of owners) {
102110 packagesApi.get(prefix, async (c) => c.json(await listFor(c, await ownerOf(c))));
103111
@@ -111,7 +119,8 @@
111119 // Diffinery: change visibility or the linked repository ({"repository": "name"} or null).
112120 packagesApi.patch(`${prefix}/:type/:name`, async (c) => {
113121 const owner = await ownerOf(c);
114- const { store, pkg } = await loadPackage(c, owner, "admin");
122+ workflowTokensMayNot(c, "change a package's visibility or repository");
123+ const { store, pkg, access } = await loadPackage(c, owner, "admin");
115124 const b = (await c.req.json().catch(() => ({}))) as { visibility?: string; repository?: string | null };
116125 const patch: Parameters<RegistryStore["updatePackage"]>[1] = {};
117126 if (b.visibility !== undefined) {
@@ -127,12 +136,13 @@
127136 }
128137 }
129138 await store.updatePackage(pkg.id, patch);
130- await c.get("dir").audit({ actorId: requireAccount(c).id, actorLogin: requireAccount(c).login, action: "package.update", target: `${owner.login}/${pkg.name}`, detail: b });
139+ await c.get("dir").audit({ actorId: access.actor?.id, actorLogin: access.actor?.login, action: "package.update", target: `${owner.login}/${pkg.name}`, detail: b });
131140 return c.json(packageJson(c, (await store.packageById(pkg.id))!));
132141 });
133142
134143 packagesApi.delete(`${prefix}/:type/:name`, async (c) => {
135144 const owner = await ownerOf(c);
145+ workflowTokensMayNot(c, "delete a whole package");
136146 const { store, pkg, access } = await loadPackage(c, owner, "admin");
137147 await store.deletePackage(pkg.id);
138148 await c.get("dir").audit({ actorId: access.actor?.id, actorLogin: access.actor?.login, action: "package.delete", target: `${owner.login}/${pkg.name}` });
modifiedapps/diffinery/src/repo/deployments.ts+6−3View file
@@ -84,9 +84,11 @@
8484 * pull request says. Runs started by agents only do when the environment is
8585 * limited to protected branches, where changes need review.
8686 */
87-export function secretsAllowedFor(policy: BranchPolicy | null, ref: string, actorKind: string): boolean {
87+export function secretsAllowedFor(policy: BranchPolicy | null, ref: string, actorKind: string, rules: { agentsMayPush?: boolean } | null): boolean {
8888 if (ref.startsWith("refs/pull/")) return false;
89- if (actorKind === "agent") return !!policy?.protected_branches;
89+ // "Protected" has to mean agents can't push there themselves: a rule that lets them push
90+ // straight to the branch would hand the environment's secrets to any agent run.
91+ if (actorKind === "agent") return !!policy?.protected_branches && !!rules && !rules.agentsMayPush;
9092 return true;
9193 }
9294
@@ -485,7 +487,8 @@
485487 /** Whether a job may receive the environment's secrets (it already passed refAllowed); see secretsAllowedFor. */
486488 secretsAllowed(name: string, ref: string, actorKind: string): boolean {
487489 const r = this.envRow(name);
488- return secretsAllowedFor(r?.branch_policy ? (JSON.parse(r.branch_policy) as BranchPolicy) : null, ref, actorKind);
490+ const rules = ref.startsWith("refs/heads/") ? this.repo.protectionFor(ref.slice(11)) : null;
491+ return secretsAllowedFor(r?.branch_policy ? (JSON.parse(r.branch_policy) as BranchPolicy) : null, ref, actorKind, rules);
489492 }
490493
491494 // ---------------------------------------------------------------- environment secrets and variables
modifiedapps/diffinery/src/web/routes.tsx+20−0View file
@@ -22,6 +22,7 @@
2222 import { registerImports } from "./imports.js";
2323 import { registerSeo } from "./seo.js";
2424 import { errorPage } from "./ui.js";
25+import { RESERVED_LOGINS } from "../lib/util.js";
2526 import { registerKumoAssets } from "./kumo.js";
2627 import { registerPackages } from "./packages.js";
2728 import { registerDocs } from "../docs/web.js";
@@ -39,6 +40,25 @@
3940
4041 web.onError((err, c) => errorPage(c, err));
4142
43+/**
44+ * A limited API token (read or write scope, or limited to some repositories) reaches the web only
45+ * for repository pages (/:owner/:repo/...), where loadRepo applies its scope and repository limits as
46+ * the API does. Everywhere else (the dashboard, the command palette, notifications, profiles,
47+ * settings, site administration) it would see what the API refuses it, so those pages treat it as
48+ * signed out. An unlimited admin token can already read all of that through the API.
49+ */
50+export function tokenMayUseWebPage(path: string): boolean {
51+ const segs = path.split("/").filter(Boolean);
52+ return segs.length >= 2 && !RESERVED_LOGINS.has(segs[0]!.toLowerCase()) && segs[1] !== "-";
53+}
54+
55+web.use("*", async (c, next) => {
56+ const p = c.get("principal");
57+ const limited = p.type === "account" && p.via === "token" && (!!p.repoIds || !p.scopes.includes("admin"));
58+ if (limited && !tokenMayUseWebPage(c.req.path)) c.set("principal", { type: "anonymous" });
59+ await next();
60+});
61+
4262 // Browsers and crawlers ask for these on every origin; keep them off the /:owner route.
4363 registerSeo(web);
4464 // Public docs (/docs, /llms.txt, ...): before /:owner/:repo, which would otherwise match /docs/<page>.
modifiedapps/diffinery/test/e2e/registry.test.mjs+7−0View file
@@ -491,6 +491,9 @@
491491 steps:
492492 - uses: actions/checkout@v4
493493 - run: bash push.sh ci
494+ - run: |
495+ curl -sS -o /dev/null -w "public=%{http_code}\\n" -X PATCH "$GITHUB_SERVER_URL/api/v3/users/$GITHUB_REPOSITORY_OWNER/packages/container/shipyard" -H "authorization: Bearer $GITHUB_TOKEN" -H "content-type: application/json" -d '{"visibility":"public"}'
496+ curl -sS -o /dev/null -w "gone=%{http_code}\\n" -X DELETE "$GITHUB_SERVER_URL/api/v3/users/$GITHUB_REPOSITORY_OWNER/packages/container/shipyard" -H "authorization: Bearer $GITHUB_TOKEN"
494497 reader:
495498 needs: publish
496499 runs-on: [self-hosted, linux]
@@ -526,6 +529,10 @@
526529 const pkg = await ok("GET", "/user/packages/container/shipyard");
527530 assert.equal(pkg.repository.full_name, `${owner}/shipyard`);
528531 assert.equal((await ok("GET", "/user/packages/container/shipyard/versions"))[0].pushed_by, "diffinery-actions[bot]");
532+ // It can't make the package public or delete it: that takes someone with admin access.
533+ assert.match(publish.log, /public=403/, publish.log);
534+ assert.match(publish.log, /gone=403/, publish.log);
535+ assert.equal((await ok("GET", "/user/packages/container/shipyard")).visibility, "private");
529536
530537 const reader = await runOneJob("shipyard");
531538 assert.equal(reader.name, "reader");
modifiedapps/diffinery/test/e2e/security.test.mjs+28−2View file
@@ -234,7 +234,6 @@
234234 for (const [who, as] of [
235235 ["pending account (session)", { cookie: S.penny }],
236236 ["disabled account (session)", { cookie: S.dora }],
237- ["agent of a disabled owner", T.orphan],
238237 ["anonymous", null],
239238 ]) {
240239 const r = await api("GET", "/search/issues?q=needle", undefined, as);
@@ -244,7 +243,9 @@
244243 assert.equal(repos.total_count, 0, `${who} must not find private repos`);
245244 }
246245 await status(401, "GET", "/search/issues?q=needle", undefined, T.dora);
247- await status(404, "GET", R, undefined, T.orphan);
246+ // An agent's tokens stop working while its owner is disabled, as they do while it's suspended.
247+ await status(401, "GET", "/search/issues?q=needle", undefined, T.orphan);
248+ await status(401, "GET", R, undefined, T.orphan);
248249 // Search input is bounded.
249250 await status(422, "GET", `/search/issues?q=${"x".repeat(300)}`);
250251 await status(422, "GET", "/search/issues");
@@ -443,6 +444,12 @@
443444 await status(401, "POST", `${R}/issues`, { title: "spam" }, null);
444445 await status(401, "POST", `${R}/issues/${issue.number}/comments`, { body: "spam" }, null);
445446 await status(401, "POST", `${R}/issues/${issue.number}/reactions`, { content: "+1" }, null);
447+ // A made-up job, sandbox or runner token is no credential: it can't write as "anonymous" or skip the anonymous rate limit.
448+ for (const forged of ["djt_x_x", `djt_${randomBytes(4).toString("hex")}_${randomBytes(16).toString("hex")}`, "dst_nope_nope", "drn_a_b"]) {
449+ await status(401, "POST", `${R}/issues`, { title: "spam" }, forged);
450+ await status(401, "POST", `${R}/issues/${issue.number}/comments`, { body: "spam" }, forged);
451+ await status(401, "GET", `${R}/issues/${issue.number}`, undefined, forged);
452+ }
446453 await status(401, "PUT", `${R}/subscription`, { subscribed: true }, null);
447454 // ...while anonymous git fetches (a POST) keep working.
448455 const clone = join(work, "commons-anon");
@@ -453,6 +460,10 @@
453460 await status(403, "POST", `${R}/issues`, { title: "nope" }, T.reader);
454461 await status(403, "POST", `${R}/issues/${issue.number}/comments`, { body: "nope" }, T.reader);
455462 await status(403, "POST", "/user/orgs", { login: "reader-org" }, T.reader);
463+ assert.match((await status(403, "POST", "/user/orgs", { login: "bot-org" }, T.bot)).message, /only people/);
464+ // Who has which role is for people with push access, as on GitHub.
465+ await status(403, "GET", `${R}/collaborators`, undefined, T.reader);
466+ await status(403, "GET", `${R}/collaborators`, undefined, null);
456467 await status(403, "POST", "/user/repos", { name: "reader-repo" }, T.reader);
457468 await status(403, "POST", "/diffinery/agents", { login: "reader-bot" }, T.reader);
458469 await status(403, "POST", "/diffinery/tokens", { name: "escalate", scopes: ["admin"] }, T.reader);
@@ -590,6 +601,16 @@
590601 const listed = await mcpTool(limited, "list_repos", {});
591602 assert.deepEqual(JSON.parse(listed.text).map((r) => r.full_name), [`${OWNER}/limits`]);
592603 assert.match((await status(403, "GET", "/notifications", undefined, limited)).message, /limited to repositories/);
604+ // The web's account pages treat a token as signed out, so it can't read there what the API refuses it.
605+ const asToken = (path) => fetch(`${base}${path}`, { headers: { authorization: `Bearer ${limited}` }, redirect: "manual" });
606+ const palette = await asToken("/-/palette.json");
607+ assert.ok(palette.status >= 300, `palette.json answered ${palette.status}`);
608+ assert.doesNotMatch(await palette.text(), /not-limits/);
609+ for (const path of ["/notifications", "/settings/tokens", "/settings/admin"]) assert.equal((await asToken(path)).status, 302, path);
610+ assert.doesNotMatch(await (await asToken("/")).text(), /not-limits/);
611+ // Repository pages still take it, within its limits.
612+ assert.equal((await asToken(`/${OWNER}/limits`)).status, 200);
613+ assert.notEqual((await asToken(`/${OWNER}/not-limits`)).status, 200);
593614 // MCP results that carry other people's text (files, issues, logs) say it's data, not instructions.
594615 const call = async (method, params) => (await (await fetch(`${base}/mcp`, { method: "POST", headers: { authorization: `Bearer ${T.olive}`, "content-type": "application/json", accept: "application/json, text/event-stream" }, body: JSON.stringify({ jsonrpc: "2.0", id: 1, method, params }) })).json()).result;
595616 const file = await call("tools/call", { name: "get_file", arguments: { owner: OWNER, repo: "limits", path: "README.md" } });
@@ -937,6 +958,11 @@
937958 assert.equal((await manual("GET", `/api/v3${N}`)).location, `${base}/api/v3${M}`);
938959 assert.equal((await manual("GET", "/team-old/moving", { as: { cookie: true } })).location, `${base}/team-new/moved`);
939960 assert.equal((await pullManifest("team-old/moving", "v1", T.olive)).status, 200);
961+ // Nobody can take the old name while links to it lead elsewhere: a new team-old could create
962+ // team-old/moving and serve its own code at the old clone URL.
963+ assert.match((await status(422, "POST", "/user/orgs", { login: "team-old" }, T.sam)).message, /retired/);
964+ assert.match((await status(422, "POST", "/user/orgs", { login: "Team-Old" })).message, /retired/);
965+ assert.equal((await manual("GET", `/api/v3${N}`)).location, `${base}/api/v3${M}`);
940966
941967 // ------------------------------------------------------------ renames leave redirects too, and refuse a taken name
942968 await ok("POST", "/orgs/team-new/repos", { name: "taken", private: true });
modifiedapps/diffinery/test/unit/environment-secrets.test.ts+13−6View file
@@ -2,23 +2,30 @@
22 import { secretsAllowedFor, type BranchPolicy } from "../../src/repo/deployments.js";
33
44 const protectedOnly: BranchPolicy = { protected_branches: true, custom_branch_policies: false, patterns: [] };
5+const main = { requirePullRequest: true };
56 const custom: BranchPolicy = { protected_branches: false, custom_branch_policies: true, patterns: [{ id: 1, name: "release/*", type: "branch" }] };
67
78 describe("environment secrets", () => {
89 it("never go to pull request refs", () => {
910 for (const p of [null, protectedOnly, custom]) {
10- for (const kind of ["user", "agent", "system"]) expect(secretsAllowedFor(p, "refs/pull/7/merge", kind)).toBe(false);
11+ for (const kind of ["user", "agent", "system"]) expect(secretsAllowedFor(p, "refs/pull/7/merge", kind, main)).toBe(false);
1112 }
1213 });
1314
1415 it("go to runs by people on allowed refs", () => {
15- expect(secretsAllowedFor(null, "refs/heads/main", "user")).toBe(true);
16- expect(secretsAllowedFor(custom, "refs/heads/release/1", "user")).toBe(true);
16+ expect(secretsAllowedFor(null, "refs/heads/main", "user", main)).toBe(true);
17+ expect(secretsAllowedFor(custom, "refs/heads/release/1", "user", null)).toBe(true);
1718 });
1819
1920 it("go to runs by agents only when the environment is limited to protected branches", () => {
20- expect(secretsAllowedFor(null, "refs/heads/main", "agent")).toBe(false);
21- expect(secretsAllowedFor(custom, "refs/heads/release/1", "agent")).toBe(false);
22- expect(secretsAllowedFor(protectedOnly, "refs/heads/main", "agent")).toBe(true);
21+ expect(secretsAllowedFor(null, "refs/heads/main", "agent", main)).toBe(false);
22+ expect(secretsAllowedFor(custom, "refs/heads/release/1", "agent", null)).toBe(false);
23+ expect(secretsAllowedFor(protectedOnly, "refs/heads/main", "agent", main)).toBe(true);
2324 });
25+
26+ it("don't go to runs by agents on a protected branch agents may push to themselves", () => {
27+ expect(secretsAllowedFor(protectedOnly, "refs/heads/agents/x", "agent", { agentsMayPush: true })).toBe(false);
28+ expect(secretsAllowedFor(protectedOnly, "refs/heads/agents/x", "user", { agentsMayPush: true })).toBe(true);
29+ expect(secretsAllowedFor(protectedOnly, "refs/heads/other", "agent", null)).toBe(false);
30+ });
2431 });
modifiedapps/diffinery/test/unit/tasks.test.ts+4−0View file
@@ -82,6 +82,10 @@
8282 expect(RUNTIMES["claude-code"].command(undefined, undefined, true)).toContain('--disallowedTools "WebSearch,WebFetch"');
8383 expect(RUNTIMES["claude-code"].command()).not.toContain("disallowedTools");
8484 expect(RUNTIMES.codex.command(undefined, undefined, true)).toContain("-c web_search=disabled");
85+ // A review's checkout is the pull request's: none of its agent configuration is loaded.
86+ expect(RUNTIMES["claude-code"].command(undefined, undefined, true, true)).toContain("--safe-mode");
87+ expect(RUNTIMES["claude-code"].command(undefined, undefined, true)).not.toContain("--safe-mode");
88+ expect(RUNTIMES.codex.command(undefined, undefined, true, true)).toContain("-c project_doc_max_bytes=0");
8589 expect(RUNTIMES.codex.command()).not.toContain("web_search");
8690 });
8791 });
modifiedapps/diffinery/test/unit/usage.test.ts+3−0View file
@@ -125,6 +125,9 @@
125125 const fn = { name: "Read", input_schema: {} };
126126 const a = prep("POST", "/v1/messages", JSON.stringify({ max_tokens: 9, tools: [fn, { type: "web_search_20250305", name: "web_search" }, { type: "web_fetch_20250910", name: "web_fetch" }, { type: "code_execution_20250825", name: "code_execution" }, { type: "mcp_toolset", mcp_server_name: "x" }], mcp_servers: [{ url: "https://x" }], tool_choice: { type: "auto" } }));
127127 expect(JSON.parse(a.reviewBody!)).toEqual({ max_tokens: 9, tools: [fn], tool_choice: { type: "auto" } });
128+ // Every other run drops them too (the provider would reach hosts past the jail's allowlist), caps untouched.
129+ expect(JSON.parse(a.jailBody!)).toEqual({ max_tokens: 9, tools: [fn], tool_choice: { type: "auto" } });
130+ expect(prep("POST", "/v1/messages", JSON.stringify({ max_tokens: 9, tools: [fn] })).jailBody).toBeUndefined();
128131 const local = { type: "local_shell" };
129132 const o = prep("POST", "/v1/responses", JSON.stringify({ max_output_tokens: 9, tools: [local, { type: "web_search" }, { type: "web_search_preview_2025_03_11" }, { type: "file_search" }, { type: "code_interpreter" }, { type: "image_generation" }, { type: "mcp", server_url: "https://x" }], tool_choice: { type: "web_search" } }));
130133 expect(JSON.parse(o.reviewBody!)).toEqual({ max_output_tokens: 9, tools: [local] });
modifiedcontainers/runner/src/jail.ts+5−2View file
@@ -248,8 +248,11 @@
248248 /** The bubblewrap command line for a jail (exported for tests). */
249249 export function bwrapArgs(a: JailArgs, o: { home: string; tools: string; env: NodeJS.ProcessEnv; cliPath?: string | null; nodePath: string }): string[] {
250250 // The home directory (shell history, other sign-ins, other checkouts), the work root (the person's checkout and
251- // the runner's state) and /tmp are empty in the jail; /root too, where it isn't the home directory.
252- const hidden = [...new Set(["/tmp", o.home, a.root, ...(existsSync("/root") ? ["/root"] : [])])].sort((x, y) => x.length - y.length);
251+ // the runner's state) and /tmp are empty in the jail; /root too, where it isn't the home directory. So is /run
252+ // (and /var/run where it isn't a link to it): it holds the Docker daemon's and BuildKit's sockets, and a socket
253+ // file can be reached from the jail's separate network namespace, through a read-only bind, as root.
254+ const varRun = existsSync("/var/run") && realpathSync("/var/run") !== "/run" ? ["/var/run"] : [];
255+ const hidden = [...new Set(["/tmp", "/run", ...varRun, o.home, a.root, ...(existsSync("/root") ? ["/root"] : [])])].sort((x, y) => x.length - y.length);
253256 const args = ["--unshare-all", "--die-with-parent", "--new-session", "--cap-drop", "ALL", "--ro-bind", "/", "/", "--dev", "/dev", "--proc", "/proc"];
254257 for (const h of hidden) args.push("--tmpfs", h);
255258 const isHidden = (p: string) => hidden.some((h) => under(p, h));
modifiedcontainers/runner/test/jail.test.ts+2−1View file
@@ -43,7 +43,8 @@
4343 for (const f of ["--unshare-all", "--die-with-parent", "--new-session", "--clearenv"]) expect(args).toContain(f);
4444 expect(s).toContain("--cap-drop ALL");
4545 expect(args).not.toContain("--share-net");
46- for (const h of ["/tmp", "/home/p", "/w"]) expect(s).toContain(`--tmpfs ${h}`);
46+ // /run too: the Docker daemon's and BuildKit's sockets live there, and a socket file reaches across network namespaces.
47+ for (const h of ["/tmp", "/run", "/home/p", "/w"]) expect(s).toContain(`--tmpfs ${h}`);
4748 expect(s).toContain("--bind /w/t /w/t");
4849 expect(s).toContain("--bind /w/t.out /w/t.out");
4950 expect(s).toContain("--ro-bind /w/repo/.git/objects /w/repo/.git/objects");