Close API holes the docs writers found (#25)
* Close API holes the docs writers found - Anonymous callers can no longer create issues, comments or reactions on public repositories; anonymous git fetches still work. - Token scopes are enforced: a read-scoped token only reads, account-level writes need write scope, and orgs, invitations, agents and tokens need admin scope. Scopes are validated on the server. - Agents can't add, change or delete secrets or variables at any level. - A merge commit is refused on branches that require linear history, and admins bypass unmet rules only with an explicit, audited bypass_rules. - Drafts can be marked ready for review (PATCH draft, web button, CLI `pr ready`). - Failed webhook deliveries are retried with backoff; workflow_dispatch and edited events are emitted. - The exact branch protection rule wins over patterns, then the oldest. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AizKXSsengXZRegJxRATkL * Account-level changes refuse tokens limited to repositories Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AizKXSsengXZRegJxRATkL * JSON 404s for unknown API paths; self-hosted domains get their own route Unknown /api paths returned the web UI's HTML 404; they now answer like GitHub, in JSON. render-config only changed PUBLIC_URL for another domain, leaving the route and email sender on diffinery.com; they now follow DIFFINERY_DOMAIN (with DIFFINERY_ZONE and MAIL_DOMAIN to override). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AizKXSsengXZRegJxRATkL --------- Co-authored-by: Claude <noreply@anthropic.com>
Canberk Demirci authored · GitHub committed signed1 parent 75ee9accommit 1d950ba8a159e2937fecaf8294522afc0eacce1bBrowse files
23 files changed+480−81
Files (23)
- README.md+26−1
- apps/cli/src/commands/pr.ts+21−2
- apps/cli/test/pr.test.ts+1−1
- apps/diffinery/src/app.ts+18−30
- apps/diffinery/src/auth/principal.ts+29−0
- apps/diffinery/src/http/api.ts+33−13
- apps/diffinery/src/http/common.ts+24−4
- apps/diffinery/src/http/deployments.ts+4−0
- apps/diffinery/src/http/git.ts+2−2
- apps/diffinery/src/http/misc.ts+5−2
- apps/diffinery/src/index.ts+4−1
- apps/diffinery/src/lib/webhooks.ts+58−0
- apps/diffinery/src/mail/api.ts+4−1
- apps/diffinery/src/mcp/server.ts+3−1
- apps/diffinery/src/repo/RepoDO.ts+8−5
- apps/diffinery/src/repo/actions.ts+2−0
- apps/diffinery/src/repo/collab.ts+38−4
- apps/diffinery/src/web/collab.tsx+17−1
- apps/diffinery/test/e2e/security.test.mjs+100−6
- apps/diffinery/test/unit/webhooks.test.ts+62−0
- packages/shared/src/events.ts+1−1
- scripts/bootstrap.sh+6−4
- scripts/render-config.mjs+14−2
modifiedREADME.md+26−1View file
| @@ -74,6 +74,13 @@ | ||
| 74 | 74 | If the token can't read your Zero Trust organization, set a repository |
| 75 | 75 | **variable** `ACCESS_TEAM_DOMAIN` (for example `codician.cloudflareaccess.com`). |
| 76 | 76 | |
| 77 | +To run your own Diffinery on another domain, set `DIFFINERY_DOMAIN` (for | |
| 78 | +example `git.example.com`) when running `scripts/bootstrap.sh`. The Worker's | |
| 79 | +route, `PUBLIC_URL` and the email sender follow it. Set `DIFFINERY_ZONE` if | |
| 80 | +the Cloudflare zone isn't the domain's last two labels (for example | |
| 81 | +`example.co.uk`), and `MAIL_DOMAIN` to send email from another domain. The | |
| 82 | +token then needs Workers Routes Edit on that zone instead of `diffinery.com`. | |
| 83 | + | |
| 77 | 84 | Email recovery uses Access's email one-time PIN, which works without any setup. |
| 78 | 85 | You can also add GitHub as a login method under Zero Trust → Settings → Authentication. |
| 79 | 86 | |
| @@ -312,6 +319,20 @@ | ||
| 312 | 319 | conversations refuse comments, review comments and reactions from people |
| 313 | 320 | without triage access. Dismissing a review takes maintain access, and the |
| 314 | 321 | pull request's author can't do it. |
| 322 | +- **Drafts and merging:** `PATCH /pulls/{n}` with `"draft": false` marks a | |
| 323 | + draft ready for review (`"draft": true` turns it back; GitHub only offers | |
| 324 | + this through GraphQL). The web page has a "Ready for review" button and the | |
| 325 | + CLI has `diffinery pr ready`. A merge commit is refused on a branch that | |
| 326 | + requires linear history. Admins can merge past unmet branch rules only by | |
| 327 | + sending `"bypass_rules": true` (the CLI's `pr merge --admin`); the bypass is | |
| 328 | + written to the audit log. When several rules match a branch, the rule that | |
| 329 | + names it exactly wins, then the oldest pattern. | |
| 330 | +- **Who can write:** anonymous callers can read public repositories but can't | |
| 331 | + create or change anything. A `read`-scoped token only reads; creating | |
| 332 | + organizations, invitations, agents and tokens takes an `admin`-scoped token | |
| 333 | + or a browser session. Token scopes must be `read`, `write` or `admin`. | |
| 334 | + Agents can't add, change or delete secrets or variables, whatever their | |
| 335 | + role. | |
| 315 | 336 | - **Archived repositories** are read-only through the API, for everyone. |
| 316 | 337 | - **Tree entry names:** every write refuses `.`, `..`, `.git` (in any case, |
| 317 | 338 | and spellings like `.git.` or `git~1`), and names with `\` or NUL. |
| @@ -336,7 +357,11 @@ | ||
| 336 | 357 | |
| 337 | 358 | Workflows can react to these too. A published release triggers `on: release`, |
| 338 | 359 | a `POST /dispatches` triggers `on: repository_dispatch`, and webhooks get |
| 339 | -`release`, `status`, `deployment` and `deployment_status` events. A job with | |
| 360 | +`release`, `status`, `deployment`, `deployment_status` and | |
| 361 | +`workflow_dispatch` events, and `edited`, `ready_for_review` and | |
| 362 | +`converted_to_draft` actions on issues and pull requests. A delivery that | |
| 363 | +fails with a network error, 408, 429 or 5xx is retried up to five times over | |
| 364 | +about five hours. A job with | |
| 340 | 365 | `environment:` creates a deployment, gets that environment's secrets and |
| 341 | 366 | variables, and is refused if the environment's branch policy doesn't allow |
| 342 | 367 | the ref. As on GitHub, events caused by `GITHUB_TOKEN` don't start new |
modifiedapps/cli/src/commands/pr.ts+21−2View file
| @@ -269,7 +269,7 @@ | ||
| 269 | 269 | subject: { type: "string", short: "t", desc: "Commit title", arg: "TEXT" }, |
| 270 | 270 | ...bodyOpts, |
| 271 | 271 | "match-head-commit": { type: "string", desc: "Only merge if the head is still at this SHA", arg: "SHA" }, |
| 272 | - admin: { type: "boolean", desc: "Merge directly even if the base branch uses a merge queue (repository admins)" }, | |
| 272 | + admin: { type: "boolean", desc: "Merge now, bypassing unmet branch rules and any merge queue (repository admins; audited)" }, | |
| 273 | 273 | "disable-auto": { type: "boolean", desc: "Remove the pull request from the merge queue" }, |
| 274 | 274 | }, |
| 275 | 275 | repo: true, |
| @@ -294,6 +294,7 @@ | ||
| 294 | 294 | commit_message: await bodyFrom(ctx, o), |
| 295 | 295 | sha: str(o, "match-head-commit"), |
| 296 | 296 | bypass_merge_queue: o.admin === true ? true : undefined, |
| 297 | + bypass_rules: o.admin === true ? true : undefined, | |
| 297 | 298 | }; |
| 298 | 299 | let r: { merged: boolean; sha: string; method: string }; |
| 299 | 300 | try { |
| @@ -335,4 +336,22 @@ | ||
| 335 | 336 | }, |
| 336 | 337 | }; |
| 337 | 338 | |
| 338 | -export const prGroup: Group = { name: "pr", summary: "Work with pull requests", commands: [create, listCmd, view, diff, checks, review, merge, comment] }; | |
| 339 | +const ready: Command = { | |
| 340 | + name: "pr ready", | |
| 341 | + summary: "Mark a draft pull request ready for review", | |
| 342 | + args: "<number>", | |
| 343 | + options: { undo: { type: "boolean", desc: "Convert the pull request back to a draft" } }, | |
| 344 | + repo: true, | |
| 345 | + json: true, | |
| 346 | + minArgs: 1, | |
| 347 | + maxArgs: 1, | |
| 348 | + async run(ctx, o, [num]) { | |
| 349 | + const n = positiveInt(num, "pull request number"); | |
| 350 | + const repo = await ctx.repo(); | |
| 351 | + const pr = await (await ctx.authedApi()).patch<IssueJson>(`${repoPath(repo)}/pulls/${n}`, { draft: o.undo === true }); | |
| 352 | + if (ctx.json) return json(ctx.io, pr); | |
| 353 | + ctx.out(`${ctx.c.green("✓")} ${o.undo === true ? `Converted #${n} to a draft` : `Marked #${n} ready for review`}`); | |
| 354 | + }, | |
| 355 | +}; | |
| 356 | + | |
| 357 | +export const prGroup: Group = { name: "pr", summary: "Work with pull requests", commands: [create, listCmd, view, diff, checks, review, merge, ready, comment] }; |
modifiedapps/cli/test/pr.test.ts+1−1View file
| @@ -89,7 +89,7 @@ | ||
| 89 | 89 | expect((await run(["pr", "merge", "6", "-R", "o/r", "--disable-auto"], { env })).stdout).toContain("Removed #6 from the merge queue"); |
| 90 | 90 | const admin = await run(["pr", "merge", "6", "-R", "o/r", "--admin"], { env }); |
| 91 | 91 | expect(admin.code).toBe(1); |
| 92 | - expect(srv.find("PUT", "/api/v3/repos/o/r/pulls/6/merge").at(-1)!.body).toEqual({ merge_method: "merge", bypass_merge_queue: true }); | |
| 92 | + expect(srv.find("PUT", "/api/v3/repos/o/r/pulls/6/merge").at(-1)!.body).toEqual({ merge_method: "merge", bypass_merge_queue: true, bypass_rules: true }); | |
| 93 | 93 | }); |
| 94 | 94 | |
| 95 | 95 | it("surfaces the server's { message } on errors", async () => { |
modifiedapps/diffinery/src/app.ts+18−30View file
| @@ -7,8 +7,9 @@ | ||
| 7 | 7 | import { Directory } from "./lib/directory.js"; |
| 8 | 8 | import { BOOKMARK_COOKIE, BOOKMARK_HEADER, d1Session, sessionConstraint } from "./lib/d1.js"; |
| 9 | 9 | import { SESSION_COOKIE } from "./auth/sessions.js"; |
| 10 | -import { hmacSha256Hex, newId } from "./lib/util.js"; | |
| 11 | -import { errorResponse, loadRepo, repoStub } from "./http/common.js"; | |
| 10 | +import { newId } from "./lib/util.js"; | |
| 11 | +import { deliverWebhook, type WebhookDeps, type WebhookRetry } from "./lib/webhooks.js"; | |
| 12 | +import { apiNotFound, errorResponse, loadRepo, repoStub } from "./http/common.js"; | |
| 12 | 13 | import { git } from "./http/git.js"; |
| 13 | 14 | import { runner } from "./http/runner.js"; |
| 14 | 15 | import { api, wellKnown } from "./http/api.js"; |
| @@ -27,6 +28,7 @@ | ||
| 27 | 28 | export const app = new Hono<{ Bindings: Env; Variables: AppVars }>(); |
| 28 | 29 | |
| 29 | 30 | app.onError((e) => errorResponse(e)); |
| 31 | +app.notFound((c) => (c.req.path.startsWith("/api/") ? apiNotFound() : c.text("404 Not Found", 404))); | |
| 30 | 32 | |
| 31 | 33 | // Security headers on every response, then identity: tokens, Access JWT, or anonymous. |
| 32 | 34 | app.use("*", async (c, next) => { |
| @@ -143,34 +145,17 @@ | ||
| 143 | 145 | const hooks = (await stub.webhooks()).filter((h) => h.active && (h.events.includes("*") || h.events.includes(GITHUB_EVENT[ev.type]))); |
| 144 | 146 | if (!hooks.length) return; |
| 145 | 147 | const body = JSON.stringify(webhookPayload(env, ev)); |
| 146 | - await Promise.all( | |
| 147 | - hooks.map(async (h) => { | |
| 148 | - const delivery = newId(); | |
| 149 | - let status = 0; | |
| 150 | - try { | |
| 151 | - const res = await fetch(h.url, { | |
| 152 | - method: "POST", | |
| 153 | - headers: { | |
| 154 | - "content-type": "application/json", | |
| 155 | - "user-agent": "Diffinery-Hookshot/1", | |
| 156 | - "x-github-event": GITHUB_EVENT[ev.type], | |
| 157 | - "x-github-delivery": delivery, | |
| 158 | - "x-diffinery-event": ev.type, | |
| 159 | - "x-hub-signature-256": `sha256=${await hmacSha256Hex(h.secret, body)}`, | |
| 160 | - }, | |
| 161 | - body, | |
| 162 | - redirect: "manual", | |
| 163 | - signal: AbortSignal.timeout(10_000), | |
| 164 | - }); | |
| 165 | - status = res.status; | |
| 166 | - } catch { | |
| 167 | - status = 0; | |
| 168 | - } | |
| 169 | - await stub.recordDelivery(h.id, status); | |
| 170 | - }), | |
| 171 | - ); | |
| 148 | + await Promise.all(hooks.map((h) => deliverWebhook(webhookDeps(env), { type: "webhook_retry", repoId: ev.repoId, hookId: h.id, event: GITHUB_EVENT[ev.type], diffineryEvent: ev.type, body, delivery: newId(), attempt: 0 }, h))); | |
| 172 | 149 | } |
| 173 | 150 | |
| 151 | +const webhookDeps = (env: Env): WebhookDeps => ({ | |
| 152 | + hook: async (repoId, hookId) => (await repoStub(env, repoId).webhooks()).find((h) => h.id === hookId && h.active) ?? null, | |
| 153 | + record: (repoId, hookId, status) => repoStub(env, repoId).recordDelivery(hookId, status), | |
| 154 | + requeue: async (r, delaySeconds) => { | |
| 155 | + await env.EVENTS.send(r as unknown as DiffineryEvent, { delaySeconds }); | |
| 156 | + }, | |
| 157 | +}); | |
| 158 | + | |
| 174 | 159 | async function handleEvent(env: Env, dir: Directory, ev: DiffineryEvent): Promise<void> { |
| 175 | 160 | if (ev.type === "audit") { |
| 176 | 161 | await dir.audit({ actorId: ev.actor.id, actorLogin: ev.actor.login, onBehalfOf: ev.actor.onBehalfOf, action: ev.action, target: ev.target, detail: ev.detail, at: ev.at }); |
| @@ -195,10 +180,13 @@ | ||
| 195 | 180 | const dir = new Directory(env); |
| 196 | 181 | // Keep an append-only copy of every event in R2 for long-term audit retention. |
| 197 | 182 | const day = new Date().toISOString().slice(0, 10).replaceAll("-", "/"); |
| 198 | - await env.BUCKET.put(`events/${day}/${Date.now()}-${newId()}.jsonl`, batch.messages.map((m) => JSON.stringify(m.body)).join("\n")).catch((e) => console.error("event archive failed", e)); | |
| 183 | + const fresh = batch.messages.filter((m) => (m.body as { type: string }).type !== "webhook_retry"); | |
| 184 | + if (fresh.length) await env.BUCKET.put(`events/${day}/${Date.now()}-${newId()}.jsonl`, fresh.map((m) => JSON.stringify(m.body)).join("\n")).catch((e) => console.error("event archive failed", e)); | |
| 199 | 185 | for (const msg of batch.messages) { |
| 200 | 186 | try { |
| 201 | - await handleEvent(env, dir, msg.body); | |
| 187 | + const body = msg.body as DiffineryEvent | WebhookRetry; | |
| 188 | + if (body.type === "webhook_retry") await deliverWebhook(webhookDeps(env), body); | |
| 189 | + else await handleEvent(env, dir, body); | |
| 202 | 190 | msg.ack(); |
| 203 | 191 | } catch (e) { |
| 204 | 192 | console.error("event handling failed", msg.body.type, e); |
modifiedapps/diffinery/src/auth/principal.ts+29−0View file
| @@ -14,6 +14,8 @@ | ||
| 14 | 14 | export interface AppVars { |
| 15 | 15 | principal: Principal; |
| 16 | 16 | dir: Directory; |
| 17 | + /** Set for MCP tools that only read, so POST-carried reads count as reads. */ | |
| 18 | + readOnly?: boolean; | |
| 17 | 19 | } |
| 18 | 20 | |
| 19 | 21 | export type AppContext = Context<{ Bindings: Env; Variables: AppVars }>; |
| @@ -80,6 +82,33 @@ | ||
| 80 | 82 | return p.account; |
| 81 | 83 | } |
| 82 | 84 | |
| 85 | +export const TOKEN_SCOPES: readonly TokenScope[] = ["read", "write", "admin"]; | |
| 86 | + | |
| 87 | +/** | |
| 88 | + * Personal access tokens carry a scope; browser sessions act with full scope. | |
| 89 | + * Throws 403 when the request's token lacks `need` (read < write < admin). | |
| 90 | + */ | |
| 91 | +export function requireScope(c: AppContext, need: TokenScope): void { | |
| 92 | + const p = c.get("principal"); | |
| 93 | + if (p.type !== "account" || p.via !== "token") return; | |
| 94 | + const have = Math.max(...p.scopes.map((s) => TOKEN_SCOPES.indexOf(s))); | |
| 95 | + if (have < TOKEN_SCOPES.indexOf(need)) throw new HttpError(403, `this token needs the ${need} scope`); | |
| 96 | +} | |
| 97 | + | |
| 98 | +/** Account-level changes (orgs, invitations, agents, tokens, settings) also refuse tokens limited to repositories. */ | |
| 99 | +export function requireAccountScope(c: AppContext, need: TokenScope): void { | |
| 100 | + requireScope(c, need); | |
| 101 | + const p = c.get("principal"); | |
| 102 | + if (p.type === "account" && p.via === "token" && p.repoIds) throw new HttpError(403, "this token is limited to repositories and can't change account settings"); | |
| 103 | +} | |
| 104 | + | |
| 105 | +/** Validates scopes sent by a client: known names only, default write. */ | |
| 106 | +export function parseScopes(input: unknown): TokenScope[] { | |
| 107 | + if (input === undefined || input === null) return ["write"]; | |
| 108 | + if (!Array.isArray(input) || !input.length || input.some((s) => !TOKEN_SCOPES.includes(s as TokenScope))) throw new HttpError(422, `scopes must be a non-empty list of: ${TOKEN_SCOPES.join(", ")}`); | |
| 109 | + return [...new Set(input as TokenScope[])]; | |
| 110 | +} | |
| 111 | + | |
| 83 | 112 | const SCOPE_CAP: Record<TokenScope, Role> = { read: "read", write: "maintain", admin: "admin" }; |
| 84 | 113 | |
| 85 | 114 | /** |
modifiedapps/diffinery/src/http/api.ts+33−13View file
| @@ -1,7 +1,7 @@ | ||
| 1 | 1 | import { Hono } from "hono"; |
| 2 | 2 | import { fromUtf8 } from "@diffinery/git-core"; |
| 3 | 3 | import type { Env } from "../env.js"; |
| 4 | -import { requireAccount, type AppContext, type AppVars } from "../auth/principal.js"; | |
| 4 | +import { parseScopes, requireAccount, requireAccountScope, type AppContext, type AppVars } from "../auth/principal.js"; | |
| 5 | 5 | import type { Account, Repo, TokenScope } from "../lib/directory.js"; |
| 6 | 6 | import { encryptSecret } from "../lib/crypto.js"; |
| 7 | 7 | import { publicJwk } from "../lib/jwt.js"; |
| @@ -10,7 +10,7 @@ | ||
| 10 | 10 | import type { JobInfo, RunInfo } from "../repo/actions.js"; |
| 11 | 11 | import type { CommitInfo } from "../repo/RepoDO.js"; |
| 12 | 12 | import type { QueueEntry } from "../repo/queue.js"; |
| 13 | -import { loadRepo, repoStub } from "./common.js"; | |
| 13 | +import { loadRepo, refuseAgent, repoStub } from "./common.js"; | |
| 14 | 14 | import { repoCreated } from "../mail/notify.js"; |
| 15 | 15 | import { compat } from "./compat.js"; |
| 16 | 16 | import { baseOf, encPath, ghCheckRun, ghComment, ghCommit, ghGitCommit, ghJob, ghRef, ghRun, ghFile, ghLabel, ghReview, ghReviewComment, ghUser, isoSeconds, nodeId, pageOf, paginate, repoApi, repoHtml, setLinks } from "./gh.js"; |
| @@ -66,6 +66,7 @@ | ||
| 66 | 66 | }); |
| 67 | 67 | api.get("/user/orgs", async (c) => c.json((await c.get("dir").orgsFor(requireAccount(c).id)).map((o) => ({ ...userJson(c, o), role: o.role })))); |
| 68 | 68 | api.post("/user/orgs", async (c) => { |
| 69 | + requireAccountScope(c, "admin"); | |
| 69 | 70 | const a = requireAccount(c); |
| 70 | 71 | const b = await body<{ login: string; name?: string }>(c); |
| 71 | 72 | return c.json(userJson(c, await c.get("dir").createOrg(b.login, b.name ?? b.login, a)), 201); |
| @@ -79,6 +80,7 @@ | ||
| 79 | 80 | return c.json((await dir.orgMembers(org.id)).map((m) => ({ ...userJson(c, m), role: m.role }))); |
| 80 | 81 | }); |
| 81 | 82 | api.post("/orgs/:org/invitations", async (c) => { |
| 83 | + requireAccountScope(c, "admin"); | |
| 82 | 84 | const a = requireAccount(c); |
| 83 | 85 | const dir = c.get("dir"); |
| 84 | 86 | const org = await dir.accountByLogin(c.req.param("org")); |
| @@ -94,6 +96,7 @@ | ||
| 94 | 96 | |
| 95 | 97 | // Site-wide invitation (no organization): site admins only. |
| 96 | 98 | api.post("/invitations", async (c) => { |
| 99 | + requireAccountScope(c, "admin"); | |
| 97 | 100 | const a = requireAccount(c); |
| 98 | 101 | if (!a.site_admin) throw new HttpError(403, "only site admins can invite people without an organization"); |
| 99 | 102 | const b = await body<{ email: string }>(c); |
| @@ -108,6 +111,7 @@ | ||
| 108 | 111 | |
| 109 | 112 | api.get("/user/repos", async (c) => c.json(paginate(c, (await c.get("dir").reposVisibleTo(requireAccount(c))).map((r) => repoJson(c, r))))); |
| 110 | 113 | api.post("/user/repos", async (c) => { |
| 114 | + requireAccountScope(c, "write"); | |
| 111 | 115 | const a = requireAccount(c); |
| 112 | 116 | const b = await body<{ name: string; description?: string; private?: boolean; visibility?: string; default_branch?: string }>(c); |
| 113 | 117 | const owner = a.kind === "agent" ? (await c.get("dir").account(a.owner_id!))! : a; |
| @@ -128,6 +132,7 @@ | ||
| 128 | 132 | return c.json(paginate(c, (await c.get("dir").reposVisibleTo(a, org.id)).map((r) => repoJson(c, r)))); |
| 129 | 133 | }); |
| 130 | 134 | api.post("/orgs/:org/repos", async (c) => { |
| 135 | + requireAccountScope(c, "write"); | |
| 131 | 136 | const a = requireAccount(c); |
| 132 | 137 | const org = await c.get("dir").accountByLogin(c.req.param("org")); |
| 133 | 138 | if (!org || org.kind !== "org") throw new HttpError(404, "organization not found"); |
| @@ -480,8 +485,10 @@ | ||
| 480 | 485 | }); |
| 481 | 486 | api.patch("/repos/:owner/:repo/pulls/:number", async (c) => { |
| 482 | 487 | const { stub, caller, repo } = await loadRepo(c, P(c).owner, P(c).repo); |
| 483 | - const b = await body<{ title?: string; body?: string | null; state?: "open" | "closed" }>(c); | |
| 484 | - const issue = await stub.updateIssue(num(c), { title: b.title, body: b.body ?? undefined, state: b.state }, caller); | |
| 488 | + const b = await body<{ title?: string; body?: string | null; state?: "open" | "closed"; draft?: boolean }>(c); | |
| 489 | + // `draft` is a Diffinery extension (GitHub only offers it through GraphQL). | |
| 490 | + if (typeof b.draft === "boolean") await stub.setPullDraft(num(c), b.draft, caller); | |
| 491 | + const issue = b.title !== undefined || b.body !== undefined || b.state !== undefined ? await stub.updateIssue(num(c), { title: b.title, body: b.body ?? undefined, state: b.state }, caller) : (await stub.issue(num(c))).issue; | |
| 485 | 492 | if (issue.kind !== "pull") throw new HttpError(404, "not a pull request"); |
| 486 | 493 | return c.json((await pullsOut(c, repo, stub, [issue]))[0]); |
| 487 | 494 | }); |
| @@ -508,8 +515,8 @@ | ||
| 508 | 515 | }); |
| 509 | 516 | api.put("/repos/:owner/:repo/pulls/:number/merge", async (c) => { |
| 510 | 517 | const { stub, caller } = await loadRepo(c, P(c).owner, P(c).repo, "write"); |
| 511 | - const b = await body<{ merge_method?: "merge" | "squash" | "rebase"; commit_title?: string; commit_message?: string; sha?: string; delete_branch?: boolean; bypass_merge_queue?: boolean }>(c).catch(() => ({}) as Record<string, undefined>); | |
| 512 | - const r = await stub.mergePull(num(c), { method: b.merge_method, title: b.commit_title, message: b.commit_message, expectedHeadSha: b.sha, deleteBranch: b.delete_branch, bypassMergeQueue: b.bypass_merge_queue }, caller); | |
| 518 | + const b = await body<{ merge_method?: "merge" | "squash" | "rebase"; commit_title?: string; commit_message?: string; sha?: string; delete_branch?: boolean; bypass_merge_queue?: boolean; bypass_rules?: boolean }>(c).catch(() => ({}) as Record<string, undefined>); | |
| 519 | + const r = await stub.mergePull(num(c), { method: b.merge_method, title: b.commit_title, message: b.commit_message, expectedHeadSha: b.sha, deleteBranch: b.delete_branch, bypassMergeQueue: b.bypass_merge_queue, bypassRules: b.bypass_rules }, caller); | |
| 513 | 520 | return c.json({ ...r, message: "Pull Request successfully merged" }); |
| 514 | 521 | }); |
| 515 | 522 | |
| @@ -702,7 +709,8 @@ | ||
| 702 | 709 | return c.body(null, 204); |
| 703 | 710 | }); |
| 704 | 711 | api.delete("/repos/:owner/:repo/actions/secrets/:name", async (c) => { |
| 705 | - const { stub } = await loadRepo(c, P(c).owner, P(c).repo, "admin"); | |
| 712 | + const { stub, caller } = await loadRepo(c, P(c).owner, P(c).repo, "admin"); | |
| 713 | + refuseAgent(caller, "secrets"); | |
| 706 | 714 | await stub.deleteSecret(c.req.param("name")); |
| 707 | 715 | return c.body(null, 204); |
| 708 | 716 | }); |
| @@ -712,16 +720,19 @@ | ||
| 712 | 720 | return c.json({ total_count: all.length, variables: paginate(c, all) }); |
| 713 | 721 | }); |
| 714 | 722 | api.delete("/repos/:owner/:repo/actions/variables/:name", async (c) => { |
| 715 | - const { stub } = await loadRepo(c, P(c).owner, P(c).repo, "admin"); | |
| 723 | + const { stub, caller } = await loadRepo(c, P(c).owner, P(c).repo, "admin"); | |
| 724 | + refuseAgent(caller, "variables"); | |
| 716 | 725 | await stub.setVariable(c.req.param("name"), null); |
| 717 | 726 | return c.body(null, 204); |
| 718 | 727 | }); |
| 719 | 728 | api.put("/repos/:owner/:repo/actions/variables/:name", async (c) => { |
| 720 | - const { stub } = await loadRepo(c, P(c).owner, P(c).repo, "admin"); | |
| 729 | + const { stub, caller } = await loadRepo(c, P(c).owner, P(c).repo, "admin"); | |
| 730 | + refuseAgent(caller, "variables"); | |
| 721 | 731 | await stub.setVariable(c.req.param("name"), (await body<{ value: string }>(c)).value); |
| 722 | 732 | return c.body(null, 204); |
| 723 | 733 | }); |
| 724 | 734 | async function orgAdmin(c: AppContext) { |
| 735 | + requireAccountScope(c, "admin"); | |
| 725 | 736 | const a = requireAccount(c); |
| 726 | 737 | if (a.kind === "agent") throw new HttpError(403, "agents cannot manage secrets"); |
| 727 | 738 | const dir = c.get("dir"); |
| @@ -779,6 +790,7 @@ | ||
| 779 | 790 | const p = c.get("principal"); |
| 780 | 791 | if (p.type === "account" && p.via === "token" && !p.scopes.includes("admin")) throw new HttpError(403, "creating tokens requires an admin-scoped token or a browser session"); |
| 781 | 792 | const b = await body<{ name: string; scopes?: TokenScope[]; repositories?: string[]; expires_in_days?: number }>(c); |
| 793 | + const scopes = parseScopes(b.scopes); | |
| 782 | 794 | let repoIds: string[] | null = null; |
| 783 | 795 | if (b.repositories?.length) { |
| 784 | 796 | repoIds = []; |
| @@ -789,11 +801,12 @@ | ||
| 789 | 801 | repoIds.push(r.id); |
| 790 | 802 | } |
| 791 | 803 | } |
| 792 | - const t = await c.get("dir").createToken(a, { name: b.name, scopes: b.scopes, repoIds, expiresInDays: b.expires_in_days }); | |
| 793 | - c.executionCtx.waitUntil(c.get("dir").audit({ actorId: a.id, actorLogin: a.login, action: "token.create", detail: { name: b.name, scopes: b.scopes } })); | |
| 804 | + const t = await c.get("dir").createToken(a, { name: b.name, scopes, repoIds, expiresInDays: b.expires_in_days }); | |
| 805 | + c.executionCtx.waitUntil(c.get("dir").audit({ actorId: a.id, actorLogin: a.login, action: "token.create", detail: { name: b.name, scopes } })); | |
| 794 | 806 | return c.json(t, 201); |
| 795 | 807 | }); |
| 796 | 808 | api.delete("/diffinery/tokens/:id", async (c) => { |
| 809 | + requireAccountScope(c, "admin"); | |
| 797 | 810 | await c.get("dir").revokeToken(requireAccount(c).id, c.req.param("id")); |
| 798 | 811 | return c.body(null, 204); |
| 799 | 812 | }); |
| @@ -803,17 +816,19 @@ | ||
| 803 | 816 | return c.json((await c.get("dir").agentsOwnedBy([a.id, ...orgs])).map((x) => ({ ...userJson(c, x), owner_id: x.owner_id, policy: x.agent_policy ? JSON.parse(x.agent_policy) : {} }))); |
| 804 | 817 | }); |
| 805 | 818 | api.post("/diffinery/agents", async (c) => { |
| 819 | + requireAccountScope(c, "admin"); | |
| 806 | 820 | const a = requireAccount(c); |
| 807 | 821 | if (a.kind !== "user") throw new HttpError(403, "only people can create agents"); |
| 808 | 822 | const b = await body<{ login: string; name?: string; org?: string; policy?: Record<string, unknown>; token?: { scopes?: TokenScope[]; expires_in_days?: number } }>(c); |
| 809 | 823 | const dir = c.get("dir"); |
| 810 | 824 | const org = b.org ? await dir.accountByLogin(b.org) : null; |
| 811 | 825 | const agent = await dir.createAgent(a, { login: b.login, name: b.name, orgId: org?.id, policy: b.policy }); |
| 812 | - const token = await dir.createToken(agent, { name: "default", scopes: b.token?.scopes ?? ["write"], expiresInDays: b.token?.expires_in_days ?? 90 }); | |
| 826 | + const token = await dir.createToken(agent, { name: "default", scopes: parseScopes(b.token?.scopes), expiresInDays: b.token?.expires_in_days ?? 90 }); | |
| 813 | 827 | c.executionCtx.waitUntil(dir.audit({ actorId: a.id, actorLogin: a.login, action: "agent.create", target: agent.login })); |
| 814 | 828 | return c.json({ agent: userJson(c, agent), token }, 201); |
| 815 | 829 | }); |
| 816 | 830 | api.post("/diffinery/agents/:login/tokens", async (c) => { |
| 831 | + requireAccountScope(c, "admin"); | |
| 817 | 832 | const a = requireAccount(c); |
| 818 | 833 | const dir = c.get("dir"); |
| 819 | 834 | const agent = await dir.accountByLogin(c.req.param("login")); |
| @@ -821,9 +836,13 @@ | ||
| 821 | 836 | const orgRole = agent.owner_id && agent.owner_id !== a.id ? await dir.orgRole(agent.owner_id, a.id) : null; |
| 822 | 837 | if (agent.owner_id !== a.id && orgRole !== "owner" && orgRole !== "admin") throw new HttpError(403, "not your agent"); |
| 823 | 838 | const b = await body<{ name?: string; scopes?: TokenScope[]; expires_in_days?: number }>(c).catch(() => ({}) as Record<string, undefined>); |
| 824 | - return c.json(await dir.createToken(agent, { name: b.name ?? "token", scopes: b.scopes, expiresInDays: b.expires_in_days }), 201); | |
| 839 | + const scopes = parseScopes(b.scopes); | |
| 840 | + const t = await dir.createToken(agent, { name: b.name ?? "token", scopes, expiresInDays: b.expires_in_days }); | |
| 841 | + c.executionCtx.waitUntil(dir.audit({ actorId: a.id, actorLogin: a.login, action: "agent.token.create", target: agent.login, detail: { scopes } })); | |
| 842 | + return c.json(t, 201); | |
| 825 | 843 | }); |
| 826 | 844 | api.delete("/diffinery/agents/:login", async (c) => { |
| 845 | + requireAccountScope(c, "admin"); | |
| 827 | 846 | const a = requireAccount(c); |
| 828 | 847 | const dir = c.get("dir"); |
| 829 | 848 | const agent = await dir.accountByLogin(c.req.param("login")); |
| @@ -857,6 +876,7 @@ | ||
| 857 | 876 | |
| 858 | 877 | api.get("/notifications", async (c) => c.json(await c.get("dir").notifications(requireAccount(c).id, c.req.query("all") !== "true"))); |
| 859 | 878 | api.put("/notifications", async (c) => { |
| 879 | + requireAccountScope(c, "write"); | |
| 860 | 880 | await c.get("dir").markRead(requireAccount(c).id); |
| 861 | 881 | return c.body(null, 205); |
| 862 | 882 | }); |
modifiedapps/diffinery/src/http/common.ts+24−4View file
| @@ -2,18 +2,28 @@ | ||
| 2 | 2 | import type { Repo } from "../lib/directory.js"; |
| 3 | 3 | import { HttpError } from "../lib/util.js"; |
| 4 | 4 | import { GITHUB_DOCS } from "./gh.js"; |
| 5 | -import { callerFor, type AppContext } from "../auth/principal.js"; | |
| 5 | +import { callerFor, requireScope, type AppContext } from "../auth/principal.js"; | |
| 6 | 6 | import { atLeast, type Caller, type Role } from "../repo/types.js"; |
| 7 | 7 | |
| 8 | 8 | export const repoStub = (env: Env, repoId: string) => env.REPO.get(env.REPO.idFromName(repoId)); |
| 9 | 9 | |
| 10 | -/** Load a repository and the caller's role, enforcing a minimum role. 404 hides private repos. */ | |
| 11 | -export async function loadRepo(c: AppContext, owner: string, name: string, min: Role = "read"): Promise<{ repo: Repo; caller: Caller; stub: ReturnType<typeof repoStub> }> { | |
| 10 | +/** | |
| 11 | + * Load a repository and the caller's role, enforcing a minimum role (404 hides | |
| 12 | + * private repos). Anonymous callers get read access to public repositories but | |
| 13 | + * may only read: any other method needs a signed-in caller, as on GitHub, | |
| 14 | + * unless the route is a read that happens to use POST (git fetch, LFS | |
| 15 | + * download) and says so. Writes also need a token with the write scope. | |
| 16 | + */ | |
| 17 | +export async function loadRepo(c: AppContext, owner: string, name: string, min: Role = "read", opts: { readOnlyPost?: boolean } = {}): Promise<{ repo: Repo; caller: Caller; stub: ReturnType<typeof repoStub> }> { | |
| 12 | 18 | const repo = await c.get("dir").repo(owner, name.replace(/\.git$/, "")); |
| 13 | 19 | if (!repo) throw new HttpError(404, "repository not found"); |
| 20 | + const anonymous = c.get("principal").type === "anonymous"; | |
| 21 | + const reading = c.req.method === "GET" || c.req.method === "HEAD" || (opts.readOnlyPost && c.req.method === "POST") || c.get("readOnly") === true; | |
| 22 | + if (anonymous && !reading) throw new HttpError(401, "authentication required"); | |
| 23 | + if (!reading) requireScope(c, "write"); | |
| 14 | 24 | const caller = await callerFor(c, repo); |
| 15 | 25 | if (!caller) { |
| 16 | - if (c.get("principal").type === "anonymous") throw new HttpError(401, "authentication required"); | |
| 26 | + if (anonymous) throw new HttpError(401, "authentication required"); | |
| 17 | 27 | throw new HttpError(404, "repository not found"); |
| 18 | 28 | } |
| 19 | 29 | if (!atLeast(caller.role, min)) throw new HttpError(403, `${min} access required`); |
| @@ -21,6 +31,16 @@ | ||
| 21 | 31 | return { repo, caller, stub: repoStub(c.env, repo.id) }; |
| 22 | 32 | } |
| 23 | 33 | |
| 34 | +/** Secrets and Actions variables are managed by people only. */ | |
| 35 | +export function refuseAgent(caller: Caller, what = "secrets and variables"): void { | |
| 36 | + if (caller.kind === "agent") throw new HttpError(403, `agents cannot manage ${what}`); | |
| 37 | +} | |
| 38 | + | |
| 39 | +/** API clients get GitHub's JSON 404 for unknown paths, never the HTML page. */ | |
| 40 | +export function apiNotFound(): Response { | |
| 41 | + return new Response(JSON.stringify({ message: "Not Found", documentation_url: GITHUB_DOCS }), { status: 404, headers: { "content-type": "application/json" } }); | |
| 42 | +} | |
| 43 | + | |
| 24 | 44 | export function errorResponse(e: unknown): Response { |
| 25 | 45 | if (e instanceof HttpError) { |
| 26 | 46 | const headers: Record<string, string> = { "content-type": "application/json" }; |
modifiedapps/diffinery/src/http/deployments.ts+4−0View file
| @@ -6,6 +6,7 @@ | ||
| 6 | 6 | import type { Caller } from "../repo/types.js"; |
| 7 | 7 | import { baseOf, ghUser, isoSeconds, nodeId, pageOf, paginate, repoApi, repoHtml, setLinks } from "./gh.js"; |
| 8 | 8 | import { intParam, jsonBody, loadTarget, repoParams } from "./ghctx.js"; |
| 9 | +import { refuseAgent } from "./common.js"; | |
| 9 | 10 | |
| 10 | 11 | /** Deployments, deployment statuses and environments, with environment secrets and variables (GitHub REST). */ |
| 11 | 12 | export const deployments = new Hono<{ Bindings: Env; Variables: AppVars }>(); |
| @@ -215,6 +216,7 @@ | ||
| 215 | 216 | }); |
| 216 | 217 | deployments.post(`${R}/environments/:env/variables`, async (c) => { |
| 217 | 218 | const { stub, caller } = await load(c, "admin"); |
| 219 | + refuseAgent(caller, "variables"); | |
| 218 | 220 | const b = await jsonBody<{ name?: string; value?: string }>(c); |
| 219 | 221 | if (typeof b.name !== "string" || typeof b.value !== "string") throw new HttpError(422, "name and value are required"); |
| 220 | 222 | if ((await stub.environmentVariables(envName(c))).some((x) => x.name === b.name!.toUpperCase())) throw new HttpError(409, "Already exists - Variable already exists"); |
| @@ -223,6 +225,7 @@ | ||
| 223 | 225 | }); |
| 224 | 226 | deployments.patch(`${R}/environments/:env/variables/:name`, async (c) => { |
| 225 | 227 | const { stub, caller } = await load(c, "admin"); |
| 228 | + refuseAgent(caller, "variables"); | |
| 226 | 229 | const b = await jsonBody<{ name?: string; value?: string }>(c); |
| 227 | 230 | const name = c.req.param("name"); |
| 228 | 231 | const current = (await stub.environmentVariables(envName(c))).find((x) => x.name === name.toUpperCase()); |
| @@ -234,6 +237,7 @@ | ||
| 234 | 237 | }); |
| 235 | 238 | deployments.delete(`${R}/environments/:env/variables/:name`, async (c) => { |
| 236 | 239 | const { stub, caller } = await load(c, "admin"); |
| 240 | + refuseAgent(caller, "variables"); | |
| 237 | 241 | await stub.setEnvironmentVariable(envName(c), c.req.param("name"), null, caller); |
| 238 | 242 | return c.body(null, 204); |
| 239 | 243 | }); |
modifiedapps/diffinery/src/http/git.ts+2−2View file
| @@ -24,7 +24,7 @@ | ||
| 24 | 24 | }); |
| 25 | 25 | |
| 26 | 26 | git.post("/:owner/:repo/git-upload-pack", async (c) => { |
| 27 | - const { stub } = await loadRepo(c, c.req.param("owner"), c.req.param("repo"), "read"); | |
| 27 | + const { stub } = await loadRepo(c, c.req.param("owner"), c.req.param("repo"), "read", { readOnlyPost: true }); | |
| 28 | 28 | const body = await readBody(c.req.raw); |
| 29 | 29 | const out = await stub.gitUploadPack(body); |
| 30 | 30 | return new Response(out, { headers: { "content-type": "application/x-git-upload-pack-result", ...noCache } }); |
| @@ -52,7 +52,7 @@ | ||
| 52 | 52 | |
| 53 | 53 | git.post("/:owner/:repo/info/lfs/objects/batch", async (c) => { |
| 54 | 54 | const req = (await c.req.json()) as { operation: "upload" | "download"; objects: Array<{ oid: string; size: number }> }; |
| 55 | - const { repo } = await loadRepo(c, c.req.param("owner"), c.req.param("repo"), req.operation === "upload" ? "write" : "read"); | |
| 55 | + const { repo } = await loadRepo(c, c.req.param("owner"), c.req.param("repo"), req.operation === "upload" ? "write" : "read", { readOnlyPost: req.operation !== "upload" }); | |
| 56 | 56 | const base = `${c.env.PUBLIC_URL}/${c.req.param("owner")}/${c.req.param("repo")}/info/lfs/objects`; |
| 57 | 57 | const auth = c.req.header("authorization"); |
| 58 | 58 | const objects = []; |
modifiedapps/diffinery/src/http/misc.ts+5−2View file
| @@ -4,6 +4,7 @@ | ||
| 4 | 4 | import { HttpError } from "../lib/util.js"; |
| 5 | 5 | import { baseOf, ghRun, isoSeconds, nodeId, repoApi, repoHtml } from "./gh.js"; |
| 6 | 6 | import { intParam, jsonBody, loadTarget, repoParams } from "./ghctx.js"; |
| 7 | +import { refuseAgent } from "./common.js"; | |
| 7 | 8 | |
| 8 | 9 | /** |
| 9 | 10 | * Smaller GitHub endpoints scripts and actions call: rate limits, topics, |
| @@ -125,7 +126,8 @@ | ||
| 125 | 126 | return c.json({ name, value: vars[name], created_at: null, updated_at: null }); |
| 126 | 127 | }); |
| 127 | 128 | misc.post(`${R}/actions/variables`, async (c) => { |
| 128 | - const { stub } = await load(c, "admin"); | |
| 129 | + const { stub, caller } = await load(c, "admin"); | |
| 130 | + refuseAgent(caller, "variables"); | |
| 129 | 131 | const b = await jsonBody<{ name?: string; value?: string }>(c); |
| 130 | 132 | if (typeof b.name !== "string" || typeof b.value !== "string") throw new HttpError(422, "name and value are required", { errors: [{ resource: "ActionsVariable", code: "missing_field", field: typeof b.name !== "string" ? "name" : "value" }] }); |
| 131 | 133 | if (b.name.toUpperCase() in (await stub.variables())) throw new HttpError(409, "Already exists - Variable already exists"); |
| @@ -133,7 +135,8 @@ | ||
| 133 | 135 | return c.json({}, 201); |
| 134 | 136 | }); |
| 135 | 137 | misc.patch(`${R}/actions/variables/:name`, async (c) => { |
| 136 | - const { stub } = await load(c, "admin"); | |
| 138 | + const { stub, caller } = await load(c, "admin"); | |
| 139 | + refuseAgent(caller, "variables"); | |
| 137 | 140 | const name = c.req.param("name"); |
| 138 | 141 | const vars = await stub.variables(); |
| 139 | 142 | if (!(name.toUpperCase() in vars)) throw new HttpError(404, "Not Found"); |
modifiedapps/diffinery/src/index.ts+4−1View file
| @@ -2,13 +2,16 @@ | ||
| 2 | 2 | import type { Env } from "./env.js"; |
| 3 | 3 | import type { MailJob } from "./mail/types.js"; |
| 4 | 4 | import { app, queue, scheduled } from "./app.js"; |
| 5 | +import { apiNotFound } from "./http/common.js"; | |
| 5 | 6 | import { web } from "./web/routes.js"; |
| 6 | 7 | |
| 7 | 8 | export { RepoDO } from "./repo/RepoDO.js"; |
| 8 | 9 | export { RunnerContainer, SandboxContainer } from "./runner/containers.js"; |
| 9 | 10 | export { RunWorkflow } from "./runner/workflow.js"; |
| 10 | 11 | |
| 11 | -// The web UI is mounted last so API, git and runner routes take precedence. | |
| 12 | +// The web UI is mounted last so API, git and runner routes take precedence; | |
| 13 | +// unknown API paths answer in JSON rather than falling through to its HTML 404. | |
| 14 | +app.all("/api/*", () => apiNotFound()); | |
| 12 | 15 | app.route("/", web); |
| 13 | 16 | |
| 14 | 17 | export default { |
addedapps/diffinery/src/lib/webhooks.ts+58−0View file
| @@ -1,0 +1,58 @@ | ||
| 1 | +import { hmacSha256Hex } from "./util.js"; | |
| 2 | + | |
| 3 | +/** One webhook delivery; failed ones go back on the events queue with a delay. */ | |
| 4 | +export interface WebhookRetry { | |
| 5 | + type: "webhook_retry"; | |
| 6 | + repoId: string; | |
| 7 | + hookId: string; | |
| 8 | + event: string; | |
| 9 | + diffineryEvent: string; | |
| 10 | + body: string; | |
| 11 | + delivery: string; | |
| 12 | + /** Attempts made so far. */ | |
| 13 | + attempt: number; | |
| 14 | +} | |
| 15 | + | |
| 16 | +export interface WebhookDeps { | |
| 17 | + /** The hook as it is now (it may have been deleted, disabled or re-keyed), or null. */ | |
| 18 | + hook(repoId: string, hookId: string): Promise<{ id: string; url: string; secret: string } | null>; | |
| 19 | + record(repoId: string, hookId: string, status: number): Promise<void>; | |
| 20 | + requeue(r: WebhookRetry, delaySeconds: number): Promise<void>; | |
| 21 | + fetch?: typeof fetch; | |
| 22 | +} | |
| 23 | + | |
| 24 | +// Delays after the first, second, ... failed attempt: about five hours in all. | |
| 25 | +export const WEBHOOK_RETRY_DELAYS = [60, 300, 1800, 3600, 3 * 3600]; | |
| 26 | + | |
| 27 | +/** Network errors, timeouts, rate limits and server errors are worth another try. */ | |
| 28 | +export const retryable = (status: number) => status === 0 || status === 408 || status === 429 || status >= 500; | |
| 29 | + | |
| 30 | +/** Send one delivery (to `known` when the caller already has the hook) and schedule a retry if it fails. */ | |
| 31 | +export async function deliverWebhook(deps: WebhookDeps, r: WebhookRetry, known?: { id: string; url: string; secret: string }): Promise<number | null> { | |
| 32 | + const h = known ?? (await deps.hook(r.repoId, r.hookId)); | |
| 33 | + if (!h) return null; | |
| 34 | + let status = 0; | |
| 35 | + try { | |
| 36 | + const res = await (deps.fetch ?? fetch)(h.url, { | |
| 37 | + method: "POST", | |
| 38 | + headers: { | |
| 39 | + "content-type": "application/json", | |
| 40 | + "user-agent": "Diffinery-Hookshot/1", | |
| 41 | + "x-github-event": r.event, | |
| 42 | + "x-github-delivery": r.delivery, | |
| 43 | + "x-diffinery-event": r.diffineryEvent, | |
| 44 | + "x-hub-signature-256": `sha256=${await hmacSha256Hex(h.secret, r.body)}`, | |
| 45 | + }, | |
| 46 | + body: r.body, | |
| 47 | + redirect: "manual", | |
| 48 | + signal: AbortSignal.timeout(10_000), | |
| 49 | + }); | |
| 50 | + status = res.status; | |
| 51 | + } catch { | |
| 52 | + status = 0; | |
| 53 | + } | |
| 54 | + await deps.record(r.repoId, h.id, status); | |
| 55 | + const delay = WEBHOOK_RETRY_DELAYS[r.attempt]; | |
| 56 | + if (retryable(status) && delay !== undefined) await deps.requeue({ ...r, attempt: r.attempt + 1 }, delay); | |
| 57 | + return status; | |
| 58 | +} |
modifiedapps/diffinery/src/mail/api.ts+4−1View file
| @@ -1,6 +1,6 @@ | ||
| 1 | 1 | import { Hono } from "hono"; |
| 2 | 2 | import type { Env } from "../env.js"; |
| 3 | -import { requireAccount, type AppContext, type AppVars } from "../auth/principal.js"; | |
| 3 | +import { requireAccount, requireAccountScope, type AppContext, type AppVars } from "../auth/principal.js"; | |
| 4 | 4 | import type { Account } from "../lib/directory.js"; |
| 5 | 5 | import { loadRepo } from "../http/common.js"; |
| 6 | 6 | import { HttpError, now } from "../lib/util.js"; |
| @@ -84,6 +84,7 @@ | ||
| 84 | 84 | |
| 85 | 85 | /** Partial update: send only the groups to change. */ |
| 86 | 86 | mailApi.put("/diffinery/notification-settings", async (c) => { |
| 87 | + requireAccountScope(c, "write"); | |
| 87 | 88 | const me = writer(c); |
| 88 | 89 | const store = new MailStore(c.env.DB); |
| 89 | 90 | const cur = await store.prefs(me.id); |
| @@ -116,6 +117,7 @@ | ||
| 116 | 117 | |
| 117 | 118 | /** Send yourself a test email right away; returns what Cloudflare said. */ |
| 118 | 119 | mailApi.post("/diffinery/emails/test", async (c) => { |
| 120 | + requireAccountScope(c, "write"); | |
| 119 | 121 | const me = writer(c); |
| 120 | 122 | return c.json(await deliver(c.env, { kind: "test", data: { by: me.login }, to: { accountId: me.id }, reason: "account", at: now() })); |
| 121 | 123 | }); |
| @@ -132,6 +134,7 @@ | ||
| 132 | 134 | * notification settings, and return each result with Cloudflare's message id. |
| 133 | 135 | */ |
| 134 | 136 | mailApi.post("/diffinery/admin/emails/samples", async (c) => { |
| 137 | + requireAccountScope(c, "admin"); | |
| 135 | 138 | const me = siteAdmin(c, "admin"); |
| 136 | 139 | const b = await json<{ only?: string[] }>(c).catch(() => ({}) as { only?: string[] }); |
| 137 | 140 | const samples = sampleJobs(c.env.PUBLIC_URL, { accountId: me.id }, me.login).filter((s) => !b.only?.length || b.only.includes(s.id)); |
modifiedapps/diffinery/src/mcp/server.ts+3−1View file
| @@ -1,6 +1,6 @@ | ||
| 1 | 1 | import { Hono } from "hono"; |
| 2 | 2 | import type { Env } from "../env.js"; |
| 3 | -import { requireAccount, type AppContext, type AppVars } from "../auth/principal.js"; | |
| 3 | +import { requireAccount, requireScope, type AppContext, type AppVars } from "../auth/principal.js"; | |
| 4 | 4 | import { HttpError, b64, fromB64 } from "../lib/util.js"; |
| 5 | 5 | import type { Role } from "../repo/types.js"; |
| 6 | 6 | import { loadRepo } from "../http/common.js"; |
| @@ -437,6 +437,8 @@ | ||
| 437 | 437 | const tool = TOOLS[name]; |
| 438 | 438 | if (!tool) return rpcError(req.id, -32602, `unknown tool: ${name}`); |
| 439 | 439 | try { |
| 440 | + c.set("readOnly", !!tool.readOnly); | |
| 441 | + if (!tool.readOnly) requireScope(c, "write"); | |
| 440 | 442 | const out = await tool.run(c, (req.params?.arguments as Args) ?? {}); |
| 441 | 443 | return { jsonrpc: "2.0", id: req.id, result: { content: [{ type: "text", text: JSON.stringify(out, null, 2) }], structuredContent: out && typeof out === "object" && !Array.isArray(out) ? out : { result: out }, isError: false } }; |
| 442 | 444 | } catch (e) { |
modifiedapps/diffinery/src/repo/RepoDO.ts+8−5View file
| @@ -305,11 +305,11 @@ | ||
| 305 | 305 | return reportStatus(req, null, results, messages); |
| 306 | 306 | } |
| 307 | 307 | |
| 308 | + /** As on GitHub: a rule naming the branch exactly wins; otherwise the oldest matching pattern. */ | |
| 308 | 309 | protectionFor(branch: string): ProtectionRules | null { |
| 309 | - for (const row of this.sql.exec<{ pattern: string; rules: string }>("SELECT pattern, rules FROM protections").toArray()) { | |
| 310 | - if (globToRegExp(row.pattern).test(branch)) return JSON.parse(row.rules) as ProtectionRules; | |
| 311 | - } | |
| 312 | - return null; | |
| 310 | + const rows = this.sql.exec<{ pattern: string; rules: string }>("SELECT pattern, rules FROM protections ORDER BY rowid").toArray(); | |
| 311 | + const row = rows.find((r) => r.pattern === branch) ?? rows.find((r) => globToRegExp(r.pattern).test(branch)); | |
| 312 | + return row ? (JSON.parse(row.rules) as ProtectionRules) : null; | |
| 313 | 313 | } |
| 314 | 314 | |
| 315 | 315 | /** Server-side rules for a ref update: permissions, protection, fast-forward, agent guardrails. */ |
| @@ -556,7 +556,7 @@ | ||
| 556 | 556 | setProtection(pattern: string, rules: ProtectionRules | null): void { |
| 557 | 557 | if (rules?.mergeQueue) rules = { ...rules, mergeQueue: normalizeMergeQueue(rules.mergeQueue) }; |
| 558 | 558 | if (rules?.requiredCheckSources) rules = { ...rules, requiredCheckSources: normalizeCheckSources(rules.requiredCheckSources) }; |
| 559 | - if (rules) this.sql.exec("INSERT OR REPLACE INTO protections (pattern, rules) VALUES (?, ?)", pattern, JSON.stringify(rules)); | |
| 559 | + if (rules) this.sql.exec("INSERT INTO protections (pattern, rules) VALUES (?, ?) ON CONFLICT(pattern) DO UPDATE SET rules = excluded.rules", pattern, JSON.stringify(rules)); | |
| 560 | 560 | else this.sql.exec("DELETE FROM protections WHERE pattern = ?", pattern); |
| 561 | 561 | // Turning a queue off (or changing its checks) affects what is queued. |
| 562 | 562 | this.queue.schedule(); |
| @@ -577,6 +577,9 @@ | ||
| 577 | 577 | updateIssue(...a: Parameters<Collab["update"]>) { |
| 578 | 578 | return this.collab.update(...a); |
| 579 | 579 | } |
| 580 | + setPullDraft(...a: Parameters<Collab["setDraft"]>) { | |
| 581 | + return this.collab.setDraft(...a); | |
| 582 | + } | |
| 580 | 583 | addComment(...a: Parameters<Collab["addComment"]>) { |
| 581 | 584 | return this.collab.addComment(...a); |
| 582 | 585 | } |
modifiedapps/diffinery/src/repo/actions.ts+2−0View file
| @@ -344,6 +344,8 @@ | ||
| 344 | 344 | } |
| 345 | 345 | const fullRef = ref.startsWith("refs/") ? ref : this.repo.getRef(`refs/heads/${ref}`) ? `refs/heads/${ref}` : `refs/tags/${ref}`; |
| 346 | 346 | const payload = { inputs: resolved, ref: fullRef, repository: this.repositoryPayload(), sender: { login: caller.login }, workflow: w.path }; |
| 347 | + const m = this.repo.meta; | |
| 348 | + this.repo.emit({ type: "workflow_dispatch", repoId: m.repoId, owner: m.owner, repo: m.name, ref: fullRef, workflow: w.path, inputs: resolved as Record<string, Json>, actor: { id: caller.id, login: caller.login, kind: caller.kind, onBehalfOf: caller.onBehalfOf }, at: now() }); | |
| 347 | 349 | return (await this.createRun(w.workflow, { event: "workflow_dispatch", ref: fullRef, sha, payload, caller, inputs: resolved, title: w.workflow.name ?? w.path }))!; |
| 348 | 350 | } |
| 349 | 351 |
modifiedapps/diffinery/src/repo/collab.ts+38−4View file
| @@ -360,6 +360,13 @@ | ||
| 360 | 360 | ); |
| 361 | 361 | this.repo.social.recordUpdate(r, patch, caller); |
| 362 | 362 | const m = this.repo.meta; |
| 363 | + if ((patch.title !== undefined && patch.title !== r.title) || (patch.body !== undefined && patch.body !== r.body)) { | |
| 364 | + if (r.kind === "pull") { | |
| 365 | + const p = this.pullRow(number); | |
| 366 | + this.repo.emit({ type: "pull_request", action: "edited", repoId: m.repoId, owner: m.owner, repo: m.name, number, headRef: p.head_ref, headSha: p.head_sha, baseRef: p.base_ref, actor: actorOf(caller), at: t }); | |
| 367 | + this.repo.background(this.repo.actions.onPullRequest("edited", number, caller).catch((e) => console.error("pr workflow trigger", e))); | |
| 368 | + } else this.repo.emit({ type: "issues", action: "edited", repoId: m.repoId, owner: m.owner, repo: m.name, number, actor: actorOf(caller), at: t }); | |
| 369 | + } | |
| 363 | 370 | if (patch.state && patch.state !== r.state) { |
| 364 | 371 | const action = patch.state === "closed" ? "closed" : "reopened"; |
| 365 | 372 | if (r.kind === "pull") { |
| @@ -381,6 +388,26 @@ | ||
| 381 | 388 | return this.toIssue(this.row(number)); |
| 382 | 389 | } |
| 383 | 390 | |
| 391 | + /** Mark a pull request ready for review, or convert it back to a draft. */ | |
| 392 | + setDraft(number: number, draft: boolean, caller: Caller): Issue { | |
| 393 | + const r = this.row(number); | |
| 394 | + if (r.kind !== "pull") throw new HttpError(404, "not a pull request"); | |
| 395 | + if (r.author_id !== caller.id && !atLeast(caller.role, "write")) throw new HttpError(403, "only the author or someone with write access can change draft status"); | |
| 396 | + if (r.state !== "open") throw new HttpError(422, "the pull request is not open"); | |
| 397 | + const p = this.pullRow(number); | |
| 398 | + if (!!p.draft === draft) return this.toIssue(r); | |
| 399 | + const t = now(); | |
| 400 | + this.sql.exec("UPDATE pulls SET draft = ? WHERE number = ?", draft ? 1 : 0, number); | |
| 401 | + this.sql.exec("UPDATE issues SET updated_at = ? WHERE number = ?", t, number); | |
| 402 | + const action = draft ? "converted_to_draft" : "ready_for_review"; | |
| 403 | + this.repo.social.record(number, action, caller, {}); | |
| 404 | + const m = this.repo.meta; | |
| 405 | + this.repo.emit({ type: "pull_request", action, repoId: m.repoId, owner: m.owner, repo: m.name, number, headRef: p.head_ref, headSha: p.head_sha, baseRef: p.base_ref, actor: actorOf(caller), at: t }); | |
| 406 | + this.repo.background(this.repo.actions.onPullRequest(action, number, caller).catch((e) => console.error("pr workflow trigger", e))); | |
| 407 | + this.repo.broadcast(`pr:${number}`, { action: "updated" }); | |
| 408 | + return this.toIssue(this.row(number)); | |
| 409 | + } | |
| 410 | + | |
| 384 | 411 | addComment(number: number, body: string, caller: Caller): Comment { |
| 385 | 412 | const r = this.row(number); |
| 386 | 413 | if (r.locked && !atLeast(caller.role, "triage")) throw new HttpError(403, "conversation is locked"); |
| @@ -709,21 +736,28 @@ | ||
| 709 | 736 | return result; |
| 710 | 737 | } |
| 711 | 738 | |
| 712 | - async merge(number: number, input: { method?: "merge" | "squash" | "rebase"; title?: string; message?: string; expectedHeadSha?: string; deleteBranch?: boolean; bypassMergeQueue?: boolean }, caller: Caller) { | |
| 739 | + async merge(number: number, input: { method?: "merge" | "squash" | "rebase"; title?: string; message?: string; expectedHeadSha?: string; deleteBranch?: boolean; bypassMergeQueue?: boolean; bypassRules?: boolean }, caller: Caller) { | |
| 713 | 740 | const p = this.pullRow(number); |
| 714 | 741 | const r = this.row(number); |
| 715 | 742 | if (input.expectedHeadSha && input.expectedHeadSha !== p.head_sha) throw new HttpError(409, "head branch was modified; review and try again"); |
| 716 | 743 | const m = await this.mergeability(number, caller); |
| 717 | 744 | const rules = this.repo.protectionFor(p.base_ref) ?? {}; |
| 718 | - const adminBypass = caller.role === "admin" && !rules.enforceAdmins && caller.kind === "user" && m.conflicts.length === 0 && !p.draft; | |
| 745 | + // Admins (people, not agents) may bypass unmet rules, but only when they ask to. | |
| 746 | + const mayBypass = caller.role === "admin" && !rules.enforceAdmins && caller.kind === "user" && m.conflicts.length === 0 && !p.draft; | |
| 747 | + const adminBypass = mayBypass && !!(input.bypassRules || input.bypassMergeQueue); | |
| 719 | 748 | if (rules.mergeQueue && !(adminBypass && input.bypassMergeQueue)) { |
| 720 | - throw new HttpError(405, `${p.base_ref} requires a merge queue: add the pull request to the queue${adminBypass ? " (or, as an admin, bypass the queue with bypass_merge_queue)" : ""}`, { blockers: [`${p.base_ref} merges through a merge queue`] }); | |
| 749 | + throw new HttpError(405, `${p.base_ref} requires a merge queue: add the pull request to the queue${mayBypass ? " (or, as an admin, bypass the queue with bypass_merge_queue)" : ""}`, { blockers: [`${p.base_ref} merges through a merge queue`] }); | |
| 721 | 750 | } |
| 722 | - if (!m.mergeable && !adminBypass) throw new HttpError(405, `pull request is not mergeable: ${m.blockers.join("; ")}`, { blockers: m.blockers }); | |
| 751 | + if (!m.mergeable && !adminBypass) throw new HttpError(405, `pull request is not mergeable: ${m.blockers.join("; ")}${mayBypass ? " (as an admin you can bypass with bypass_rules)" : ""}`, { blockers: m.blockers }); | |
| 723 | 752 | const baseRef = `refs/heads/${p.base_ref}`; |
| 724 | 753 | const baseSha = this.repo.getRef(baseRef)!; |
| 725 | 754 | const store = this.repo.store; |
| 726 | 755 | const method = input.method ?? (rules.requireLinearHistory ? "squash" : "merge"); |
| 756 | + if (method === "merge" && rules.requireLinearHistory && !adminBypass) throw new HttpError(405, `${p.base_ref} requires linear history; use squash or rebase`); | |
| 757 | + if (adminBypass && !m.mergeable) { | |
| 758 | + const meta = this.repo.meta; | |
| 759 | + this.repo.emit({ type: "audit", action: "pull_request.merge_bypassed", actor: caller, target: `${meta.owner}/${meta.name}#${number}`, detail: { blockers: m.blockers, base: p.base_ref }, at: now() }); | |
| 760 | + } | |
| 727 | 761 | let newSha: string; |
| 728 | 762 | const ts = Math.floor(Date.now() / 1000); |
| 729 | 763 | const sig = { name: caller.login, email: `${caller.login}@users.noreply.diffinery.com`, timestamp: ts, timezone: "+0000" }; |
modifiedapps/diffinery/src/web/collab.tsx+17−1View file
| @@ -681,8 +681,15 @@ | ||
| 681 | 681 | const showChecks = checks.length ? checks : m.checks.map((ch) => ({ name: ch.name, status: ch.state === "queued" || ch.state === "in_progress" ? ch.state : "completed", conclusion: ch.state, runId: null, jobId: null, output: null }) as unknown as Check); |
| 682 | 682 | const failing = showChecks.filter((ch) => ch.status === "completed" && ch.conclusion !== "success" && ch.conclusion !== "skipped" && ch.conclusion !== "neutral").length; |
| 683 | 683 | const pending = showChecks.filter((ch) => ch.status !== "completed").length; |
| 684 | + const canReady = p.draft && isAccount && (caller.id === i.author.id || atLeast(caller.role, "write")) && !x.l.repo.archived; | |
| 684 | 685 | return ( |
| 685 | 686 | <div class={`mergebox ${m.state}`} data-fragment="merge"> |
| 687 | + {canReady ? ( | |
| 688 | + <Form action={`${base}/pull/${i.number}/ready`} t={csrfToken(x.c)} class="row" style="margin-bottom:8px"> | |
| 689 | + <span class="grow">This pull request is still a draft and can't be merged yet.</span> | |
| 690 | + <button class="btn">Ready for review</button> | |
| 691 | + </Form> | |
| 692 | + ) : null} | |
| 686 | 693 | {showChecks.length ? ( |
| 687 | 694 | <div> |
| 688 | 695 | <div class="row mb" style="margin-bottom:6px"> |
| @@ -742,6 +749,7 @@ | ||
| 742 | 749 | <div> |
| 743 | 750 | <Form action={`${base}/pull/${i.number}/merge`} t={csrfToken(x.c)} class="row" confirm={bypass ? "Merge without meeting the branch requirements?" : undefined}> |
| 744 | 751 | <input type="hidden" name="expected_head" value={p.headSha} /> |
| 752 | + {bypass ? <input type="hidden" name="bypass_rules" value="1" /> : null} | |
| 745 | 753 | <KSelect |
| 746 | 754 | name="method" |
| 747 | 755 | label="Merge method" |
| @@ -1260,6 +1268,14 @@ | ||
| 1260 | 1268 | }); |
| 1261 | 1269 | }); |
| 1262 | 1270 | |
| 1271 | + post(web, "/:owner/:repo/pull/:n/ready", async (c) => { | |
| 1272 | + const l = await forAction(c, "read"); | |
| 1273 | + const n = issueNumber(c); | |
| 1274 | + return act(c, `${repoUrl(l.repo)}/pull/${n}`, async () => { | |
| 1275 | + await l.stub.setPullDraft(n, false, l.caller); | |
| 1276 | + }, "Pull request marked ready for review."); | |
| 1277 | + }); | |
| 1278 | + | |
| 1263 | 1279 | post(web, "/:owner/:repo/pull/:n/merge", async (c, f) => { |
| 1264 | 1280 | const l = await forAction(c, "write"); |
| 1265 | 1281 | const n = issueNumber(c); |
| @@ -1268,7 +1284,7 @@ | ||
| 1268 | 1284 | c, |
| 1269 | 1285 | `${repoUrl(l.repo)}/pull/${n}`, |
| 1270 | 1286 | async () => { |
| 1271 | - await l.stub.mergePull(n, { method, deleteBranch: f.b("delete_branch"), expectedHeadSha: f.s("expected_head") || undefined, bypassMergeQueue: f.b("bypass_queue") }, l.caller); | |
| 1287 | + await l.stub.mergePull(n, { method, deleteBranch: f.b("delete_branch"), expectedHeadSha: f.s("expected_head") || undefined, bypassMergeQueue: f.b("bypass_queue"), bypassRules: f.b("bypass_rules") }, l.caller); | |
| 1272 | 1288 | c.executionCtx.waitUntil(c.get("dir").touchPushed(l.repo.id)); |
| 1273 | 1289 | }, |
| 1274 | 1290 | "Pull request merged.", |
modifiedapps/diffinery/test/e2e/security.test.mjs+100−6View file
| @@ -9,7 +9,10 @@ | ||
| 9 | 9 | // environments or dismiss reviews; secrets default environments to protected |
| 10 | 10 | // branches; locked conversations and archived repositories refuse changes; |
| 11 | 11 | // draft releases stay off readers' live sockets; unsafe tree names, tree bombs, |
| 12 | -// long search queries and percent signs in label names. | |
| 12 | +// long search queries and percent signs in label names. Anonymous callers and | |
| 13 | +// read-scoped tokens can't write; agents can't change secrets or variables; | |
| 14 | +// merges honour linear history, need an explicit admin bypass, and drafts can | |
| 15 | +// be marked ready; the exact branch protection rule wins over a pattern. | |
| 13 | 16 | import { spawn, execFileSync } from "node:child_process"; |
| 14 | 17 | import { createHash, randomBytes } from "node:crypto"; |
| 15 | 18 | import { mkdtempSync, rmSync } from "node:fs"; |
| @@ -28,7 +31,7 @@ | ||
| 28 | 31 | |
| 29 | 32 | const hash = (s) => createHash("sha256").update(s).digest("hex"); |
| 30 | 33 | const tok = (name) => `dfy_${name}_${randomBytes(12).toString("hex")}`; |
| 31 | -const T = { olive: tok("olive"), bot: tok("bot"), rita: tok("rita"), dora: tok("dora"), orphan: tok("orphan") }; | |
| 34 | +const T = { olive: tok("olive"), bot: tok("bot"), rita: tok("rita"), dora: tok("dora"), orphan: tok("orphan"), reader: tok("reader"), ritaAdmin: tok("ritaAdmin") }; | |
| 32 | 35 | const S = { penny: `penny-session-${randomBytes(12).toString("hex")}`, dora: `dora-session-${randomBytes(12).toString("hex")}` }; |
| 33 | 36 | const OWNER = "olive"; |
| 34 | 37 | |
| @@ -107,6 +110,8 @@ | ||
| 107 | 110 | token("t5", "a-orphan", T.orphan, ["write"]), |
| 108 | 111 | account("o-guild", "guild", "org", "active"), |
| 109 | 112 | `INSERT INTO org_members (org_id, user_id, role, created_at) VALUES ('o-guild', 'u-olive', 'owner', '${at}'), ('o-guild', 'u-rita', 'admin', '${at}')`, |
| 113 | + token("t6", "u-olive", T.reader, ["read"]), | |
| 114 | + token("t7", "u-rita", T.ritaAdmin, ["admin"]), | |
| 110 | 115 | session("s1", "u-penny", S.penny), |
| 111 | 116 | session("s2", "u-dora", S.dora), |
| 112 | 117 | ].join("; "), |
| @@ -303,9 +308,98 @@ | ||
| 303 | 308 | }); |
| 304 | 309 | |
| 305 | 310 | test("security: organization admins can't hand out ownership", async () => { |
| 306 | - // rita is an admin of guild, olive its owner. | |
| 307 | - await status(403, "POST", "/orgs/guild/invitations", { email: "crown@example.com", role: "owner" }, T.rita); | |
| 308 | - await status(422, "POST", "/orgs/guild/invitations", { email: "crown@example.com", role: "superuser" }, T.rita); | |
| 309 | - await status(201, "POST", "/orgs/guild/invitations", { email: "helper@example.com", role: "admin" }, T.rita); | |
| 311 | + // rita is an admin of guild, olive its owner. Invitations need an admin-scoped token. | |
| 312 | + await status(403, "POST", "/orgs/guild/invitations", { email: "helper@example.com", role: "member" }, T.rita); | |
| 313 | + await status(403, "POST", "/orgs/guild/invitations", { email: "crown@example.com", role: "owner" }, T.ritaAdmin); | |
| 314 | + await status(422, "POST", "/orgs/guild/invitations", { email: "crown@example.com", role: "superuser" }, T.ritaAdmin); | |
| 315 | + await status(201, "POST", "/orgs/guild/invitations", { email: "helper@example.com", role: "admin" }, T.ritaAdmin); | |
| 310 | 316 | await status(201, "POST", "/orgs/guild/invitations", { email: "crown@example.com", role: "owner" }, T.olive); |
| 311 | 317 | }); |
| 318 | + | |
| 319 | +/** Open a pull request from a new branch with one changed file. */ | |
| 320 | +async function pullFrom(R, base, branch, opts = {}) { | |
| 321 | + await ok("POST", `${R}/git/refs`, { ref: `refs/heads/${branch}`, sha: base }); | |
| 322 | + await ok("PUT", `${R}/contents/${branch}.txt`, { message: branch, content: b64(`${branch}\n`), branch }); | |
| 323 | + return ok("POST", `${R}/pulls`, { title: branch, head: branch, base: "main", ...opts }); | |
| 324 | +} | |
| 325 | + | |
| 326 | +test("security: anonymous writes, token scopes, agent secrets, merge rules and drafts", async () => { | |
| 327 | + const R = `/repos/${OWNER}/commons`; | |
| 328 | + const mainSha = await repoWithReadme("commons", { visibility: "public" }); | |
| 329 | + const issue = await ok("POST", `${R}/issues`, { title: "public issue" }); | |
| 330 | + | |
| 331 | + // ------------------------------------------------------------ unknown API paths answer like GitHub, in JSON | |
| 332 | + const missing = await status(404, "GET", "/no/such/endpoint"); | |
| 333 | + assert.equal(missing.message, "Not Found"); | |
| 334 | + | |
| 335 | + // ------------------------------------------------------------ anonymous callers read public repos but never write | |
| 336 | + await status(200, "GET", `${R}/issues/${issue.number}`, undefined, null); | |
| 337 | + await status(401, "POST", `${R}/issues`, { title: "spam" }, null); | |
| 338 | + await status(401, "POST", `${R}/issues/${issue.number}/comments`, { body: "spam" }, null); | |
| 339 | + await status(401, "POST", `${R}/issues/${issue.number}/reactions`, { content: "+1" }, null); | |
| 340 | + await status(401, "PUT", `${R}/subscription`, { subscribed: true }, null); | |
| 341 | + // ...while anonymous git fetches (a POST) keep working. | |
| 342 | + const clone = join(work, "commons-anon"); | |
| 343 | + execFileSync("git", ["-c", "protocol.version=2", "clone", "-q", `${base}/${OWNER}/commons.git`, clone], { stdio: "pipe" }); | |
| 344 | + | |
| 345 | + // ------------------------------------------------------------ read-scoped tokens only read | |
| 346 | + await status(200, "GET", R, undefined, T.reader); | |
| 347 | + await status(403, "POST", `${R}/issues`, { title: "nope" }, T.reader); | |
| 348 | + await status(403, "POST", `${R}/issues/${issue.number}/comments`, { body: "nope" }, T.reader); | |
| 349 | + await status(403, "POST", "/user/orgs", { login: "reader-org" }, T.reader); | |
| 350 | + await status(403, "POST", "/user/repos", { name: "reader-repo" }, T.reader); | |
| 351 | + await status(403, "POST", "/diffinery/agents", { login: "reader-bot" }, T.reader); | |
| 352 | + await status(403, "POST", "/diffinery/tokens", { name: "escalate", scopes: ["admin"] }, T.reader); | |
| 353 | + await status(403, "PUT", "/notifications", {}, T.reader); | |
| 354 | + // Token scopes are checked on the server. | |
| 355 | + await status(422, "POST", "/diffinery/tokens", { name: "bad", scopes: ["root"] }); | |
| 356 | + await status(422, "POST", "/diffinery/tokens", { name: "bad", scopes: [] }); | |
| 357 | + await status(422, "POST", "/diffinery/agents/olive-bot/tokens", { scopes: ["superuser"] }); | |
| 358 | + const minted = await ok("POST", "/diffinery/tokens", { name: "fine", scopes: ["read", "read"] }); | |
| 359 | + assert.ok(minted.token); | |
| 360 | + assert.deepEqual((await ok("GET", "/diffinery/tokens")).find((t) => t.id === minted.id).scopes, ["read"]); | |
| 361 | + | |
| 362 | + // ------------------------------------------------------------ admin-role agents can't change secrets or variables | |
| 363 | + await status(204, "PUT", `${R}/collaborators/olive-bot`, { permission: "admin" }); | |
| 364 | + await status(204, "PUT", `${R}/actions/secrets/TOKEN`, { value: "s3cret" }); | |
| 365 | + await status(204, "PUT", `${R}/actions/variables/REGION`, { value: "eu" }); | |
| 366 | + await status(403, "DELETE", `${R}/actions/secrets/TOKEN`, undefined, T.bot); | |
| 367 | + await status(403, "PUT", `${R}/actions/variables/REGION`, { value: "attacker" }, T.bot); | |
| 368 | + await status(403, "DELETE", `${R}/actions/variables/REGION`, undefined, T.bot); | |
| 369 | + await status(403, "POST", `${R}/actions/variables`, { name: "NEW", value: "x" }, T.bot); | |
| 370 | + await status(403, "PATCH", `${R}/actions/variables/REGION`, { value: "attacker" }, T.bot); | |
| 371 | + assert.equal((await ok("GET", `${R}/actions/variables/REGION`)).value, "eu"); | |
| 372 | + | |
| 373 | + // ------------------------------------------------------------ the exact rule wins over a wildcard, whatever their order | |
| 374 | + await ok("PUT", `${R}/branches/${encodeURIComponent("*")}/protection`, { requireLinearHistory: true, requiredApprovals: 2 }); | |
| 375 | + await ok("PUT", `${R}/branches/main/protection`, { requireLinearHistory: true }); | |
| 376 | + const linear = await pullFrom(R, mainSha, "linear"); | |
| 377 | + // A merge commit breaks linear history, through the API as through the queue. | |
| 378 | + const refused = await status(405, "PUT", `${R}/pulls/${linear.number}/merge`, { merge_method: "merge" }); | |
| 379 | + assert.match(refused.message, /linear history/); | |
| 380 | + const squashed = await ok("PUT", `${R}/pulls/${linear.number}/merge`, { merge_method: "squash" }); | |
| 381 | + assert.equal(squashed.merged, true); | |
| 382 | + | |
| 383 | + // ------------------------------------------------------------ admins bypass unmet rules only when they ask to | |
| 384 | + await ok("PUT", `${R}/branches/main/protection`, { requiredApprovals: 1 }); | |
| 385 | + const head = (await ok("GET", `${R}/branches/main`)).commit.sha; | |
| 386 | + const unreviewed = await pullFrom(R, head, "unreviewed"); | |
| 387 | + const blocked = await status(405, "PUT", `${R}/pulls/${unreviewed.number}/merge`, { merge_method: "squash" }); | |
| 388 | + assert.match(blocked.message, /approving review/); | |
| 389 | + assert.match(blocked.message, /bypass_rules/); | |
| 390 | + assert.equal((await ok("PUT", `${R}/pulls/${unreviewed.number}/merge`, { merge_method: "squash", bypass_rules: true })).merged, true); | |
| 391 | + | |
| 392 | + // ------------------------------------------------------------ drafts can be marked ready for review | |
| 393 | + await status(204, "DELETE", `${R}/branches/main/protection`); | |
| 394 | + await status(204, "DELETE", `${R}/branches/${encodeURIComponent("*")}/protection`); | |
| 395 | + const head2 = (await ok("GET", `${R}/branches/main`)).commit.sha; | |
| 396 | + const draft = await pullFrom(R, head2, "wip", { draft: true }); | |
| 397 | + assert.equal(draft.draft, true); | |
| 398 | + await status(405, "PUT", `${R}/pulls/${draft.number}/merge`, { merge_method: "squash" }); | |
| 399 | + await status(403, "PATCH", `${R}/pulls/${draft.number}`, { draft: false }, T.rita); | |
| 400 | + const ready = await ok("PATCH", `${R}/pulls/${draft.number}`, { draft: false }); | |
| 401 | + assert.equal(ready.draft, false); | |
| 402 | + const events = await ok("GET", `${R}/issues/${draft.number}/events`); | |
| 403 | + assert.ok(events.some((e) => e.event === "ready_for_review"), JSON.stringify(events)); | |
| 404 | + assert.equal((await ok("PUT", `${R}/pulls/${draft.number}/merge`, { merge_method: "squash" })).merged, true); | |
| 405 | +}); |
addedapps/diffinery/test/unit/webhooks.test.ts+62−0View file
| @@ -1,0 +1,62 @@ | ||
| 1 | +import { describe, expect, it } from "vitest"; | |
| 2 | +import { deliverWebhook, WEBHOOK_RETRY_DELAYS, type WebhookDeps, type WebhookRetry } from "../../src/lib/webhooks.js"; | |
| 3 | + | |
| 4 | +const hook = { id: "h1", url: "https://hooks.example/x", secret: "s3cret" }; | |
| 5 | +const msg: WebhookRetry = { type: "webhook_retry", repoId: "r1", hookId: "h1", event: "push", diffineryEvent: "push", body: '{"ok":true}', delivery: "d1", attempt: 0 }; | |
| 6 | + | |
| 7 | +function harness(statuses: Array<number | Error>, current: typeof hook | null = hook) { | |
| 8 | + const recorded: number[] = []; | |
| 9 | + const requeued: Array<{ r: WebhookRetry; delay: number }> = []; | |
| 10 | + const sent: Request[] = []; | |
| 11 | + const deps: WebhookDeps = { | |
| 12 | + hook: async () => current, | |
| 13 | + record: async (_repo, _hook, status) => void recorded.push(status), | |
| 14 | + requeue: async (r, delay) => void requeued.push({ r, delay }), | |
| 15 | + fetch: (async (url: string, init: RequestInit) => { | |
| 16 | + sent.push(new Request(url, init)); | |
| 17 | + const next = statuses.shift()!; | |
| 18 | + if (next instanceof Error) throw next; | |
| 19 | + return new Response(null, { status: next }); | |
| 20 | + }) as typeof fetch, | |
| 21 | + }; | |
| 22 | + return { deps, recorded, requeued, sent }; | |
| 23 | +} | |
| 24 | + | |
| 25 | +describe("webhook delivery", () => { | |
| 26 | + it("signs the body and does not retry a success", async () => { | |
| 27 | + const h = harness([200]); | |
| 28 | + expect(await deliverWebhook(h.deps, msg)).toBe(200); | |
| 29 | + expect(h.recorded).toEqual([200]); | |
| 30 | + expect(h.requeued).toEqual([]); | |
| 31 | + expect(h.sent[0]!.headers.get("x-github-event")).toBe("push"); | |
| 32 | + expect(h.sent[0]!.headers.get("x-hub-signature-256")).toMatch(/^sha256=[0-9a-f]{64}$/); | |
| 33 | + }); | |
| 34 | + | |
| 35 | + it("retries network errors, rate limits and server errors with growing delays", async () => { | |
| 36 | + for (const failure of [new Error("connection reset"), 429, 503]) { | |
| 37 | + const h = harness([failure]); | |
| 38 | + await deliverWebhook(h.deps, msg); | |
| 39 | + expect(h.requeued).toEqual([{ r: { ...msg, attempt: 1 }, delay: WEBHOOK_RETRY_DELAYS[0] }]); | |
| 40 | + } | |
| 41 | + const h = harness([500]); | |
| 42 | + await deliverWebhook(h.deps, { ...msg, attempt: 2 }); | |
| 43 | + expect(h.requeued[0]!.delay).toBe(WEBHOOK_RETRY_DELAYS[2]); | |
| 44 | + expect(h.requeued[0]!.r.delivery).toBe("d1"); | |
| 45 | + }); | |
| 46 | + | |
| 47 | + it("does not retry client errors, and gives up after the last delay", async () => { | |
| 48 | + const rejected = harness([404]); | |
| 49 | + await deliverWebhook(rejected.deps, msg); | |
| 50 | + expect(rejected.requeued).toEqual([]); | |
| 51 | + const last = harness([500]); | |
| 52 | + await deliverWebhook(last.deps, { ...msg, attempt: WEBHOOK_RETRY_DELAYS.length }); | |
| 53 | + expect(last.recorded).toEqual([500]); | |
| 54 | + expect(last.requeued).toEqual([]); | |
| 55 | + }); | |
| 56 | + | |
| 57 | + it("drops a retry when the hook was deleted or disabled", async () => { | |
| 58 | + const h = harness([200], null); | |
| 59 | + expect(await deliverWebhook(h.deps, { ...msg, attempt: 1 })).toBeNull(); | |
| 60 | + expect(h.sent).toEqual([]); | |
| 61 | + }); | |
| 62 | +}); |
modifiedscripts/bootstrap.sh+6−4View file
| @@ -9,8 +9,10 @@ | ||
| 9 | 9 | # Then renders apps/diffinery/wrangler.deploy.jsonc and applies D1 migrations. |
| 10 | 10 | # |
| 11 | 11 | # Requires: CLOUDFLARE_API_TOKEN, CLOUDFLARE_ACCOUNT_ID, node, jq, curl. |
| 12 | -# Optional: DIFFINERY_DOMAIN (default diffinery.com), ACCESS_TEAM_DOMAIN | |
| 13 | -# (e.g. codician.cloudflareaccess.com) when the token cannot read it. | |
| 12 | +# Optional: DIFFINERY_DOMAIN (default diffinery.com; the Worker's route, PUBLIC_URL | |
| 13 | +# and sender follow it), DIFFINERY_ZONE (the Cloudflare zone holding that host, | |
| 14 | +# default its last two labels), MAIL_DOMAIN (default DIFFINERY_DOMAIN), | |
| 15 | +# ACCESS_TEAM_DOMAIN (e.g. codician.cloudflareaccess.com) when the token cannot read it. | |
| 14 | 16 | set -euo pipefail |
| 15 | 17 | |
| 16 | 18 | : "${CLOUDFLARE_API_TOKEN:?set CLOUDFLARE_API_TOKEN}" |
| @@ -75,7 +77,7 @@ | ||
| 75 | 77 | endlog |
| 76 | 78 | |
| 77 | 79 | # ------------------------------------------------------------------ Email Sending |
| 78 | -# notifications@diffinery.com; warns (and email is skipped) until the domain is onboarded. | |
| 80 | +# notifications@<MAIL_DOMAIN or DIFFINERY_DOMAIN>; warns (and email is skipped) until the domain is onboarded. | |
| 79 | 81 | DIFFINERY_DOMAIN="$DOMAIN" "$ROOT/scripts/email-sending.sh" || warn "Email Sending check failed; email may not be delivered." |
| 80 | 82 | |
| 81 | 83 | # ------------------------------------------------------------------ Access |
| @@ -123,7 +125,7 @@ | ||
| 123 | 125 | |
| 124 | 126 | # ------------------------------------------------------------------ config + migrations |
| 125 | 127 | log "Render config and migrate D1" |
| 126 | -node "$ROOT/scripts/render-config.mjs" --d1 "$D1_ID" ${TEAM:+--team "$TEAM"} ${AUD:+--aud "$AUD"} --public-url "https://$DOMAIN" | |
| 128 | +node "$ROOT/scripts/render-config.mjs" --d1 "$D1_ID" ${TEAM:+--team "$TEAM"} ${AUD:+--aud "$AUD"} --public-url "https://$DOMAIN" ${DIFFINERY_ZONE:+--zone "$DIFFINERY_ZONE"} --mail-from "notifications@${MAIL_DOMAIN:-$DOMAIN}" | |
| 127 | 129 | "${WRANGLER[@]}" d1 migrations apply diffinery --remote -c wrangler.deploy.jsonc |
| 128 | 130 | endlog |
| 129 | 131 |
modifiedscripts/render-config.mjs+14−2View file
| @@ -3,12 +3,16 @@ | ||
| 3 | 3 | // discovered by scripts/bootstrap.sh (real D1 id, Access team domain and AUD). |
| 4 | 4 | // |
| 5 | 5 | // node scripts/render-config.mjs --d1 <uuid> [--team <x.cloudflareaccess.com>] [--aud <tag>] |
| 6 | +// [--public-url https://git.example.com] [--zone example.com] [--mail-from notifications@example.com] | |
| 7 | +// | |
| 8 | +// --public-url also points the Worker's route at that host (in --zone, by default | |
| 9 | +// the host's last two labels) and sends email from notifications@<host>. | |
| 6 | 10 | import { readFileSync, writeFileSync } from "node:fs"; |
| 7 | 11 | import { dirname, join } from "node:path"; |
| 8 | 12 | import { fileURLToPath } from "node:url"; |
| 9 | 13 | import { parseArgs } from "node:util"; |
| 10 | 14 | |
| 11 | -const { values } = parseArgs({ options: { d1: { type: "string" }, team: { type: "string" }, aud: { type: "string" }, "public-url": { type: "string" } } }); | |
| 15 | +const { values } = parseArgs({ options: { d1: { type: "string" }, team: { type: "string" }, aud: { type: "string" }, "public-url": { type: "string" }, zone: { type: "string" }, "mail-from": { type: "string" } } }); | |
| 12 | 16 | if (!values.d1) throw new Error("--d1 <database id> is required"); |
| 13 | 17 | |
| 14 | 18 | /** Strip // and /* *\/ comments and trailing commas from JSONC, leaving strings intact. */ |
| @@ -38,6 +42,14 @@ | ||
| 38 | 42 | cfg.d1_databases[0].database_id = values.d1; |
| 39 | 43 | if (values.team) cfg.vars.ACCESS_TEAM_DOMAIN = values.team; |
| 40 | 44 | if (values.aud) cfg.vars.ACCESS_AUD = values.aud; |
| 41 | -if (values["public-url"]) cfg.vars.PUBLIC_URL = values["public-url"]; | |
| 45 | +if (values["public-url"]) { | |
| 46 | + const url = new URL(values["public-url"]); | |
| 47 | + if (url.protocol !== "https:" || url.pathname !== "/") throw new Error("--public-url must be an https origin, like https://git.example.com"); | |
| 48 | + cfg.vars.PUBLIC_URL = url.origin; | |
| 49 | + const zone = values.zone ?? url.hostname.split(".").slice(-2).join("."); | |
| 50 | + cfg.routes = [{ pattern: `${url.hostname}/*`, zone_name: zone }]; | |
| 51 | + cfg.vars.MAIL_FROM = `notifications@${url.hostname}`; | |
| 52 | +} | |
| 53 | +if (values["mail-from"]) cfg.vars.MAIL_FROM = values["mail-from"]; | |
| 42 | 54 | writeFileSync(join(appDir, "wrangler.deploy.jsonc"), `// Generated by scripts/render-config.mjs; do not edit.\n${JSON.stringify(cfg, null, 2)}\n`); |
| 43 | 55 | console.log(`wrote ${join(appDir, "wrangler.deploy.jsonc")}`); |