Security audit, part 2: issue permissions, pinned called workflows, OIDC agent claim, registry upload cap, importer token handling, agent role caps (#191)
* Security audit batch 2 (part 1): issue permissions for blocked and read-only callers, importer token handling, agent role caps * Security audit batch 2 (part 2): admin email throttle, audited bootstrap admins, own-key role checks, suspended authors' issue events, review re-queues, model proxy checks tokens first, called workflows pinned and kept in workflow directories, OIDC environment claim follows secrets, per-run job cap, registry upload cap * Security audit batch 2: review fixes (remote called workflows in workflow directories, pins kept when expansion fails, over-quota uploads discarded)
- CI / Typecheck and bundlesuccess
- CI / Unit tests (1/2)success
- CI / Unit tests (2/2)success
- Deploy / Deploy to Cloudflaresuccess
- End-to-end / End-to-end (agent reviews, tasks, Today)success
- End-to-end / End-to-end (git, PRs, Actions)success
- End-to-end / End-to-end (large pushes)success
- End-to-end / End-to-end (passkeys, registry, email)Failed
- End-to-end / End-to-end (real histories)success
- End-to-end / End-to-end (security)success
- Live test / Containers, registry, sandboxes, webhooks, schedules, LFS and pull requestssuccess
23 files changed+215−38
Files (23)
- apps/diffinery/docs/actions/compatibility.md+2−1
- apps/diffinery/docs/actions/oidc.md+4−3
- apps/diffinery/docs/actions/reusable-workflows.md+3−2
- apps/diffinery/src/agentic/ProjectDO.ts+10−2
- apps/diffinery/src/agentic/proxy.ts+9−1
- apps/diffinery/src/http/api.ts+2−2
- apps/diffinery/src/http/issues.ts+9−3
- apps/diffinery/src/http/runner.ts+2−0
- apps/diffinery/src/imports/options.ts+8−5
- apps/diffinery/src/imports/service.ts+9−1
- apps/diffinery/src/lib/directory.ts+11−2
- apps/diffinery/src/lib/quotas.ts+7−0
- apps/diffinery/src/registry/http.ts+26−4
- apps/diffinery/src/registry/upload.ts+9−1
- apps/diffinery/src/repo/actions.ts+26−5
- apps/diffinery/src/repo/collab.ts+19−3
- apps/diffinery/src/repo/reusable.ts+11−2
- apps/diffinery/src/repo/schema.ts+2−0
- apps/diffinery/src/web/imports.tsx+1−1
- apps/diffinery/test/e2e/moderation.test.mjs+7−0
- apps/diffinery/test/e2e/security.test.mjs+27−0
- apps/diffinery/test/unit/imports.test.ts+4−0
- apps/diffinery/test/unit/reusable.test.ts+7−0
modifiedapps/diffinery/docs/actions/compatibility.md+2−1View file
| @@ -68,7 +68,7 @@ | ||
| 68 | 68 | | `jobs.<id>.environment` | Supported: the branch policy is enforced, the environment's secrets and variables are added and a deployment is recorded. Required reviewers and wait timers hold the job before it starts, and `url` can't use step outputs. See [environment](/docs/actions/workflow-syntax#environment). | |
| 69 | 69 | | `jobs.<id>.snapshot` | Accepted and ignored. | |
| 70 | 70 | | `jobs.<id>.timeout-minutes` | Default and maximum 360. Larger values are capped at 360. | |
| 71 | -| `jobs.<id>.strategy.matrix` | At most 256 jobs per matrix. | | |
| 71 | +| `jobs.<id>.strategy.matrix` | At most 256 jobs per matrix, and at most 1,000 jobs in a run, matrix legs and the jobs of called workflows included. | | |
| 72 | 72 | | `jobs.<id>.uses` with `strategy.matrix` | Not supported yet. The run fails with `jobs.<id>: a matrix on a job that calls a reusable workflow is not supported yet`. | |
| 73 | 73 | | `concurrency` (workflow level) | Several runs can wait in a group; GitHub keeps only the newest pending one. Job-level `concurrency` keeps only the newest pending job, as on GitHub. See [Concurrency](/docs/actions/overview#concurrency). | |
| 74 | 74 | | `defaults.run.shell`, `jobs.<id>.steps[*].shell` | `cmd` isn't supported. `pwsh` and `powershell` need PowerShell, which hosted runners don't have. | |
| @@ -412,6 +412,7 @@ | ||
| 412 | 412 | | `No go-version specified and Go is not installed on this runner.` | Set `go-version` or `go-version-file`. | |
| 413 | 413 | | `jobs.<id>: a matrix on a job that calls a reusable workflow is not supported yet` | Move the matrix into the called workflow, or write one calling job per value. | |
| 414 | 414 | | `A matrix may produce at most 256 jobs.` | Split the matrix. | |
| 415 | +| `A run may have at most 1000 jobs, ...` | Split the workflow, or trim its matrices. | | |
| 415 | 416 | | `Composite action nesting is too deep (max 10).` | Flatten nested composite actions. | |
| 416 | 417 | | A built-in action behaves differently from the real one | Run the real action: set `DIFFINERY_BUILTINS: false` in the step's `env:`, or pin the action to a commit SHA. | |
| 417 | 418 | | A job is skipped although its `if:` looks true | A job further up its `needs` chain failed, was cancelled or was skipped, and the `if:` doesn't call a status function. Use `if: always() && ...` or `if: ${{ !cancelled() && !failure() }}`. | |
modifiedapps/diffinery/docs/actions/oidc.md+4−3View file
| @@ -55,7 +55,7 @@ | ||
| 55 | 55 | "response_types_supported": ["id_token"], |
| 56 | 56 | "subject_types_supported": ["public"], |
| 57 | 57 | "id_token_signing_alg_values_supported": ["ES256"], |
| 58 | - "claims_supported": ["sub", "aud", "iss", "repository", "repository_id", "repository_owner", "repository_owner_id", "environment", "ref", "ref_type", "sha", "workflow", "event_name", "actor", "actor_id", "run_id"] | |
| 58 | + "claims_supported": ["sub", "aud", "iss", "repository", "repository_id", "repository_owner", "repository_owner_id", "environment", "agent_origin", "ref", "ref_type", "sha", "workflow", "event_name", "actor", "actor_id", "run_id"] | |
| 59 | 59 | } |
| 60 | 60 | ``` |
| 61 | 61 | |
| @@ -107,7 +107,8 @@ | ||
| 107 | 107 | | `repository_id` | `2918374650` | The repository's numeric id. It stays the same when the repository is renamed or transferred, and a new repository with an old name gets a new one. | |
| 108 | 108 | | `repository_owner` | `acme` | Its owner (user or organization). | |
| 109 | 109 | | `repository_owner_id` | `3482910375` | The owner's numeric id, which a new account with a reused name doesn't get. | |
| 110 | -| `environment` | `production` | The environment the job deployed to. Only on jobs with `environment:`, once its protection rules passed. | | |
| 110 | +| `environment` | `production` | The environment the job deployed to. Only on jobs with `environment:`, once its protection rules passed, and only when the job may also have the environment's secrets. | | |
| 111 | +| `agent_origin` | `false` | Diffinery's own claim: `true` when the code the run executes came from an agent (an agent pushed it, or merged it). | | |
| 111 | 112 | | `ref_type` | `branch` | `branch` or `tag`. | |
| 112 | 113 | | `head_ref`, `base_ref` | `feature`, `main` | The pull request's branches, on pull request runs only. | |
| 113 | 114 | | `run_number`, `run_attempt` | `128`, `1` | The run's number and attempt. | |
| @@ -138,7 +139,7 @@ | ||
| 138 | 139 | |
| 139 | 140 | ## Jobs that use an environment |
| 140 | 141 | |
| 141 | -A job with `environment:` gets GitHub's environment form of `sub`, `repo:<owner>/<repo>:environment:<name>`, and an `environment` claim. The token is only issued after the environment's deployment branch policy has passed, so a trust policy on `repo:acme/website:environment:production` only trusts jobs that were allowed to deploy to `production` (see [Deployments and environments](/docs/actions/environments)). | |
| 142 | +A job with `environment:` gets GitHub's environment form of `sub`, `repo:<owner>/<repo>:environment:<name>`, and an `environment` claim. The token is only issued after the environment's deployment branch policy has passed, so a trust policy on `repo:acme/website:environment:production` only trusts jobs that were allowed to deploy to `production` (see [Deployments and environments](/docs/actions/environments)). A job that the environment lets deploy but keeps its secrets from, such as agent code on a branch the policy doesn't trust agents with, gets the `ref:` or `pull_request` subject instead. To keep agent code out of a role altogether, also require `agent_origin` to be `false`. | |
| 142 | 143 | |
| 143 | 144 | To keep trusting a reused name out, also check `repository_id` and `repository_owner_id` where your provider lets you: a repository deleted and recreated, or an owner name taken by a new account, gets new ids. |
| 144 | 145 |
modifiedapps/diffinery/docs/actions/reusable-workflows.md+3−2View file
| @@ -94,7 +94,7 @@ | ||
| 94 | 94 | uses: ./.github/workflows/build.yml |
| 95 | 95 | ``` |
| 96 | 96 | |
| 97 | -The path starts with `./`, is relative to the repository root and ends in `.yml` or `.yaml`. The file is read from the **same commit** as the calling workflow, so a branch or pull request can change both together. Workflows under `.diffinery/workflows/` can be called the same way (`./.diffinery/workflows/build.yml`). | |
| 97 | +The path starts with `./`, is relative to the repository root, names a file in `.github/workflows/` or `.diffinery/workflows/` and ends in `.yml` or `.yaml`. The file is read from the **same commit** as the calling workflow, so a branch or pull request can change both together. Workflows under `.diffinery/workflows/` can be called the same way (`./.diffinery/workflows/build.yml`). | |
| 98 | 98 | |
| 99 | 99 | ### Workflows in other repositories |
| 100 | 100 | |
| @@ -115,7 +115,7 @@ | ||
| 115 | 115 | |
| 116 | 116 | Inside a workflow called from another repository, a local `./` reference resolves in that other repository, at the commit the called workflow came from. |
| 117 | 117 | |
| 118 | -> **Important:** A branch `<ref>` is resolved each time Diffinery plans or starts a job of the run, so if the branch moves during a run, later jobs can get a newer version of the called workflow. Pin cross-repository calls to a tag or a commit SHA when that matters. | |
| 118 | +A branch or tag `<ref>` is resolved once, when the run is planned: every job of the run, including one that waits for an environment's approval, uses the called workflow from that commit, even if the branch moves during the run. | |
| 119 | 119 | |
| 120 | 120 | ## Inputs |
| 121 | 121 | |
| @@ -394,6 +394,7 @@ | ||
| 394 | 394 | | Message | Fix | |
| 395 | 395 | |---|---| |
| 396 | 396 | | `reusable workflow reference '...' must be './path/to/workflow.yml' or 'owner/repo/path/to/workflow.yml@ref'` | Fix the `uses` value. Cross-repository references need `@<ref>`; local ones need `./`. | |
| 397 | +| `'./...' must be a file in ./.github/workflows or ./.diffinery/workflows` | Move the called workflow into a workflow directory. Files there count as workflow changes, which agents can't merge on their own and job tokens can't push. | | |
| 397 | 398 | | `a job that calls a reusable workflow ('uses') cannot have 'steps'` (or `'runs-on'`) | Move the steps into the called workflow. | |
| 398 | 399 | | `input '<name>' must declare a 'type'` | Add `type: string`, `boolean` or `number` to the input under `on.workflow_call.inputs`. | |
| 399 | 400 | | `input '<name>': invalid type '<t>' (expected one of string, boolean, number)` | `choice` and `environment` exist only for `workflow_dispatch`. | |
modifiedapps/diffinery/src/agentic/ProjectDO.ts+10−2View file
| @@ -1030,6 +1030,12 @@ | ||
| 1030 | 1030 | * cost) is booked here, before it is sent, so requests in flight at once can't together overrun a review's |
| 1031 | 1031 | * budget; chargeUsage settles it to what the answer reports. |
| 1032 | 1032 | */ |
| 1033 | + /** Whether a model token belongs to a run that is talking to its model now; books nothing. */ | |
| 1034 | + async modelTokenLive(token: string): Promise<boolean> { | |
| 1035 | + const hash = await sha256(token); | |
| 1036 | + return this.sql.exec("SELECT 1 FROM runs WHERE token_hash = ? AND step = 'agent'", hash).toArray().length > 0; | |
| 1037 | + } | |
| 1038 | + | |
| 1033 | 1039 | async modelAuth( |
| 1034 | 1040 | token: string, |
| 1035 | 1041 | req: { provider: Provider; estimate: Usage | null; reviewRefuse?: string; model?: string | null; routing?: string }, |
| @@ -1717,8 +1723,10 @@ | ||
| 1717 | 1723 | if (issue.pull.headSha !== head) { |
| 1718 | 1724 | // Its lines may no longer match: review the new head instead. |
| 1719 | 1725 | await this.reviewCheck(t, head, "completed", "cancelled", "Superseded by a newer push", ""); |
| 1720 | - this.note(t.number, r.seq, `#${pr} changed while it was being reviewed; reviewing the new head next.`); | |
| 1721 | - if (!this.task(t.number)!.queued) this.sql.exec("UPDATE tasks SET queued = 'automatic' WHERE number = ?", t.number); | |
| 1726 | + // A review someone asked for carries on as their request. An automatic one needs nothing here: the push | |
| 1727 | + // queued the next review already if the repository's review settings (trigger, drafts, authors) allow one. | |
| 1728 | + if (!this.task(t.number)!.queued && r.message) this.sql.exec("UPDATE tasks SET queued = ? WHERE number = ?", r.message, t.number); | |
| 1729 | + this.note(t.number, r.seq, this.task(t.number)!.queued ? `#${pr} changed while it was being reviewed; reviewing the new head next.` : `#${pr} changed while it was being reviewed.`); | |
| 1722 | 1730 | return this.endRun(r, "done", null); |
| 1723 | 1731 | } |
| 1724 | 1732 | const lines = this.sql.exec<TranscriptLine>("SELECT seq, run, at, stream, text FROM lines WHERE task = ? AND run = ? AND stream = 'out' ORDER BY seq", t.number, r.seq).toArray(); |
modifiedapps/diffinery/src/agentic/proxy.ts+9−1View file
| @@ -4,6 +4,7 @@ | ||
| 4 | 4 | import { isProvider, ModelKeys, PROVIDERS, type Provider } from "./keys.js"; |
| 5 | 5 | import { bookSpend, proxyPayer, settleSpend, spendMonth } from "./org-policy.js"; |
| 6 | 6 | import { freeSlot, MAX_IN_FLIGHT, takeSlot } from "./inflight.js"; |
| 7 | +import { overLimit } from "../lib/rate-limit.js"; | |
| 7 | 8 | import { meterUsage, NO_USAGE, prepareRequest, readBody, unreadCharge, weighted } from "./usage.js"; |
| 8 | 9 | |
| 9 | 10 | /** The largest model request the proxy forwards (it reads each one to charge it). */ |
| @@ -86,7 +87,14 @@ | ||
| 86 | 87 | const repoId = token.split("_")[1]; |
| 87 | 88 | if (!repoId || !/^[A-Za-z0-9-]{8,64}$/.test(repoId)) return err(401, "malformed model token"); |
| 88 | 89 | const env = c.env; |
| 89 | - const project = env.PROJECT.get(env.PROJECT.idFromName(repoId)); | |
| 90 | + // Check the token before reading a body of up to 32 MB, and only wake a project that exists. Wrong | |
| 91 | + // tokens count against the sign-in limit per address. | |
| 92 | + const known = await env.DB.prepare("SELECT 1 FROM repos WHERE id = ?").bind(repoId).first(); | |
| 93 | + const project = known ? env.PROJECT.get(env.PROJECT.idFromName(repoId)) : null; | |
| 94 | + if (!project || !(await project.modelTokenLive(token))) { | |
| 95 | + if (await overLimit(env, [["RL_AUTH", `ip:${c.req.header("cf-connecting-ip") ?? "unknown"}`]])) return err(429, "too many requests with invalid model tokens"); | |
| 96 | + return err(401, "this model token isn't valid: its task run has ended"); | |
| 97 | + } | |
| 90 | 98 | // Read and prepare the request first: its estimate is booked against the run before it goes upstream. |
| 91 | 99 | let body: string | undefined; |
| 92 | 100 | let prepared: ReturnType<typeof prepareRequest> = { body: "", estimate: null }; |
modifiedapps/diffinery/src/http/api.ts+2−2View file
| @@ -466,7 +466,7 @@ | ||
| 466 | 466 | const b = await body<{ permission?: Role | "pull" | "push" }>(c).catch(() => ({ permission: "write" as Role })); |
| 467 | 467 | const map: Record<string, Role> = { pull: "read", push: "write" }; |
| 468 | 468 | const role = map[b.permission ?? ""] ?? (b.permission as Role) ?? "write"; |
| 469 | - if (!(role in ROLE_LEVEL)) throw new HttpError(422, "permission must be pull, triage, push, maintain or admin"); | |
| 469 | + if (!Object.hasOwn(ROLE_LEVEL, role)) throw new HttpError(422, "permission must be pull, triage, push, maintain or admin"); | |
| 470 | 470 | // As on GitHub: someone new gets an invitation (201) and has access once they accept it; an |
| 471 | 471 | // existing collaborator's role changes at once (204). |
| 472 | 472 | const r = await c.get("dir").grantRepoAccess(repo, requireAccount(c), who, role); |
| @@ -1418,7 +1418,7 @@ | ||
| 1418 | 1418 | // OIDC discovery for workflow id tokens. |
| 1419 | 1419 | export const wellKnown = new Hono<{ Bindings: Env; Variables: AppVars }>(); |
| 1420 | 1420 | wellKnown.get("/.well-known/openid-configuration", (c) => |
| 1421 | - c.json({ issuer: c.env.PUBLIC_URL, jwks_uri: `${c.env.PUBLIC_URL}/.well-known/jwks`, response_types_supported: ["id_token"], subject_types_supported: ["public"], id_token_signing_alg_values_supported: ["ES256"], claims_supported: ["sub", "aud", "iss", "repository", "repository_id", "repository_owner", "repository_owner_id", "environment", "ref", "ref_type", "sha", "workflow", "event_name", "actor", "actor_id", "run_id"] }), | |
| 1421 | + c.json({ issuer: c.env.PUBLIC_URL, jwks_uri: `${c.env.PUBLIC_URL}/.well-known/jwks`, response_types_supported: ["id_token"], subject_types_supported: ["public"], id_token_signing_alg_values_supported: ["ES256"], claims_supported: ["sub", "aud", "iss", "repository", "repository_id", "repository_owner", "repository_owner_id", "environment", "agent_origin", "ref", "ref_type", "sha", "workflow", "event_name", "actor", "actor_id", "run_id"] }), | |
| 1422 | 1422 | ); |
| 1423 | 1423 | wellKnown.get("/.well-known/jwks", (c) => c.json({ keys: c.env.OIDC_PRIVATE_KEY ? [publicJwk(JSON.parse(c.env.OIDC_PRIVATE_KEY))] : [] })); |
| 1424 | 1424 |
modifiedapps/diffinery/src/http/issues.ts+9−3View file
| @@ -296,21 +296,27 @@ | ||
| 296 | 296 | const res = await stub.repoIssueEvents(p.perPage, p.offset); |
| 297 | 297 | setLinks(c, p, res.total); |
| 298 | 298 | const numbers = [...new Set(res.items.map((e) => e.number))]; |
| 299 | + const hide = await hiddenAuthors(c); | |
| 299 | 300 | const list = []; |
| 300 | - for (const n of numbers) list.push((await stub.issue(n)).issue); | |
| 301 | + // A suspended author's issues stay hidden here too: their events are left out. | |
| 302 | + for (const n of numbers) { | |
| 303 | + const found = await stub.issue(n, hide).catch((e) => (e instanceof Error && /not found/.test(e.message) ? null : Promise.reject(e))); | |
| 304 | + if (found) list.push(found.issue); | |
| 305 | + } | |
| 301 | 306 | const x = await issueCtx(c, repo, stub, list); |
| 302 | 307 | const byNumber = new Map(list.map((i) => [i.number, ghIssue(x, i)])); |
| 303 | - return c.json(res.items.map((e) => eventJson(x, e, byNumber.get(e.number)))); | |
| 308 | + return c.json(res.items.filter((e) => byNumber.has(e.number)).map((e) => eventJson(x, e, byNumber.get(e.number)))); | |
| 304 | 309 | }); |
| 305 | 310 | issues.get(`${R}/issues/events/:id`, async (c) => { |
| 306 | 311 | const { stub, repo } = await load(c); |
| 307 | 312 | const e = await stub.issueEvent(intParam(c, "id")); |
| 308 | - const { issue } = await stub.issue(e.number); | |
| 313 | + const { issue } = await stub.issue(e.number, await hiddenAuthors(c)); | |
| 309 | 314 | const x = await issueCtx(c, repo, stub, [issue]); |
| 310 | 315 | return c.json(eventJson(x, e, ghIssue(x, issue))); |
| 311 | 316 | }); |
| 312 | 317 | issues.get(`${R}/issues/:number/events`, async (c) => { |
| 313 | 318 | const { stub, repo } = await load(c); |
| 319 | + await stub.issue(intParam(c, "number"), await hiddenAuthors(c)); | |
| 314 | 320 | const x = await issueCtx(c, repo, stub, []); |
| 315 | 321 | return c.json(paginate(c, (await stub.issueEvents(intParam(c, "number"))).map((e) => eventJson(x, e)))); |
| 316 | 322 | }); |
modifiedapps/diffinery/src/http/runner.ts+2−0View file
| @@ -153,6 +153,8 @@ | ||
| 153 | 153 | repository_owner_id: g.repository_owner_id, |
| 154 | 154 | actor_id: g.actor_id, |
| 155 | 155 | environment: g.environment, |
| 156 | + // Diffinery's own claim: whether the code that runs came from an agent. | |
| 157 | + agent_origin: g.agent_origin === "true", | |
| 156 | 158 | ref_type: g.ref_type, |
| 157 | 159 | head_ref: g.head_ref || undefined, |
| 158 | 160 | base_ref: g.base_ref || undefined, |
modifiedapps/diffinery/src/imports/options.ts+8−5View file
| @@ -113,10 +113,13 @@ | ||
| 113 | 113 | } |
| 114 | 114 | |
| 115 | 115 | /** The REST API base for a GitHub host: github.com → api.github.com, Enterprise → https://host/api/v3. */ |
| 116 | -export function normalizeApiUrl(raw: string | undefined | null, dev: boolean): string { | |
| 116 | +export function normalizeApiUrl(raw: string | undefined | null, dev: boolean, ownHost?: string): string { | |
| 117 | 117 | const s = (raw ?? "").trim(); |
| 118 | 118 | if (!s) return "https://api.github.com"; |
| 119 | - const u = checkPublicUrl(s, dev, "github_api_url"); | |
| 119 | + const u = checkPublicUrl(s, dev, "github_api_url", ownHost); | |
| 120 | + // The import container clones from this host on its own network, so only the standard HTTPS port: other | |
| 121 | + // ports would let an import probe services next to whatever address the name resolves to. | |
| 122 | + if (!dev && u.port && u.port !== "443") bad("github_api_url must use the standard HTTPS port (443)"); | |
| 120 | 123 | const host = u.host.toLowerCase(); |
| 121 | 124 | if (host === "github.com" || host === "www.github.com" || host === "api.github.com") return "https://api.github.com"; |
| 122 | 125 | let p = u.pathname.replace(/\/+$/, ""); |
| @@ -184,7 +187,7 @@ | ||
| 184 | 187 | * one repository: { source: "octo/app", owner, name?, visibility?, options?, github_token?, github_api_url? } |
| 185 | 188 | * many: { source_owner: "octo", repositories?: [...], exclude_forks?, exclude_archived?, concurrency?, owner, … } |
| 186 | 189 | */ |
| 187 | -export function parseImportRequest(body: unknown, o: { dev: boolean }): ImportRequest { | |
| 190 | +export function parseImportRequest(body: unknown, o: { dev: boolean; ownHost?: string }): ImportRequest { | |
| 188 | 191 | if (!body || typeof body !== "object" || Array.isArray(body)) return bad("request body must be a JSON object"); |
| 189 | 192 | const b = body as Record<string, unknown>; |
| 190 | 193 | const str = (k: string): string | null => { |
| @@ -216,7 +219,7 @@ | ||
| 216 | 219 | if (parsed.host) { |
| 217 | 220 | if (token) { |
| 218 | 221 | if (!apiUrl) bad("github_api_url is required when a github_token is given and the source names a GitHub Enterprise host"); |
| 219 | - if (new URL(normalizeApiUrl(apiUrl, o.dev)).host.toLowerCase() !== parsed.host.toLowerCase()) bad("github_api_url must be on the same host as the source repository"); | |
| 222 | + if (new URL(normalizeApiUrl(apiUrl, o.dev, o.ownHost)).host.toLowerCase() !== parsed.host.toLowerCase()) bad("github_api_url must be on the same host as the source repository"); | |
| 220 | 223 | } else if (!apiUrl) { |
| 221 | 224 | apiUrl = `https://${parsed.host}/api/v3`; |
| 222 | 225 | } |
| @@ -248,7 +251,7 @@ | ||
| 248 | 251 | name, |
| 249 | 252 | visibility, |
| 250 | 253 | options: parseOptions(b.options), |
| 251 | - apiUrl: normalizeApiUrl(apiUrl, o.dev), | |
| 254 | + apiUrl: normalizeApiUrl(apiUrl, o.dev, o.ownHost), | |
| 252 | 255 | token, |
| 253 | 256 | concurrency, |
| 254 | 257 | }; |
modifiedapps/diffinery/src/imports/service.ts+9−1View file
| @@ -394,7 +394,7 @@ | ||
| 394 | 394 | async create(actor: Account, scopes: TokenScope[], body: unknown, o: { draft?: boolean } = {}): Promise<ImportRow> { |
| 395 | 395 | if (actor.kind === "org") throw new HttpError(403, "organizations cannot start imports"); |
| 396 | 396 | if (!scopes.includes("write") && !scopes.includes("admin")) throw new HttpError(403, "starting an import needs a token with write or admin scope"); |
| 397 | - const req = parseImportRequest(body, { dev: this.dev }); | |
| 397 | + const req = parseImportRequest(body, { dev: this.dev, ownHost: this.env.PUBLIC_URL ? new URL(this.env.PUBLIC_URL).hostname : undefined }); | |
| 398 | 398 | const owner = await this.dir.accountByLogin(req.owner); |
| 399 | 399 | if (!owner || owner.kind === "agent") throw new HttpError(422, `no Diffinery user or organization named ${req.owner}`); |
| 400 | 400 | if (!(await this.canImportInto(actor, owner))) { |
| @@ -671,6 +671,14 @@ | ||
| 671 | 671 | .prepare("UPDATE imports SET status = ?, error = ?, token_ciphertext = NULL, finished_at = ?, updated_at = ? WHERE id = ? AND status IN ('draft', 'queued', 'running')") |
| 672 | 672 | .bind(status, error ? error.slice(0, 2000) : null, t, t, row.id) |
| 673 | 673 | .run(); |
| 674 | + // An organization import that ended leaves no repository waiting with a copy of the GitHub token: one still | |
| 675 | + // queued would never start, and would keep the token (and block deleting the account or organization) for good. | |
| 676 | + if (row.kind === "bulk" && status !== "succeeded") { | |
| 677 | + await this.db | |
| 678 | + .prepare("UPDATE imports SET status = 'cancelled', error = ?, token_ciphertext = NULL, finished_at = ?, updated_at = ? WHERE parent_id = ? AND status IN ('draft', 'queued')") | |
| 679 | + .bind(`the organization import ${status === "failed" ? "failed" : "was cancelled"} before this repository started`, t, t, row.id) | |
| 680 | + .run(); | |
| 681 | + } | |
| 674 | 682 | } |
| 675 | 683 | |
| 676 | 684 | // ---------------------------------------------------------------- workflow: shared |
modifiedapps/diffinery/src/lib/directory.ts+11−2View file
| @@ -172,6 +172,7 @@ | ||
| 172 | 172 | if (existing) { |
| 173 | 173 | if (existing.status === "pending" && this.isBootstrapAdmin(email)) { |
| 174 | 174 | await this.db.prepare("UPDATE accounts SET status = 'active', site_admin = 1 WHERE id = ?").bind(existing.id).run(); |
| 175 | + await this.audit({ actorId: existing.id, actorLogin: existing.login, action: "site_admin.grant", target: existing.login, detail: { via: "bootstrap" } }); | |
| 175 | 176 | return { ...existing, status: "active", site_admin: 1 }; |
| 176 | 177 | } |
| 177 | 178 | return existing; |
| @@ -184,6 +185,7 @@ | ||
| 184 | 185 | .prepare("INSERT INTO accounts (id, login, kind, name, email, status, site_admin, created_at) VALUES (?, ?, 'user', ?, ?, ?, ?, ?)") |
| 185 | 186 | .bind(id, login, hints.name ?? null, email, admin || hasInvite || hints.active ? "active" : "pending", admin ? 1 : 0, now()) |
| 186 | 187 | .run(); |
| 188 | + if (admin) await this.audit({ actorId: id, actorLogin: login, action: "site_admin.grant", target: login, detail: { via: "bootstrap" } }); | |
| 187 | 189 | if (hasInvite) await this.acceptInvitesForEmail(id, email); |
| 188 | 190 | return (await this.account(id))!; |
| 189 | 191 | } |
| @@ -525,7 +527,7 @@ | ||
| 525 | 527 | if (input.role !== "member" && input.role !== "admin" && input.role !== "owner") throw new HttpError(422, "role must be member, admin or owner"); |
| 526 | 528 | if (input.role === "owner" && !inviter.site_admin && (await this.orgRole(input.orgId, inviter.id)) !== "owner") throw new HttpError(403, "only owners can invite owners"); |
| 527 | 529 | } |
| 528 | - if (input.repoId && !(input.role in ROLE_LEVEL)) throw new HttpError(422, "permission must be read, triage, write, maintain or admin"); | |
| 530 | + if (input.repoId && !Object.hasOwn(ROLE_LEVEL, input.role)) throw new HttpError(422, "permission must be read, triage, write, maintain or admin"); | |
| 529 | 531 | // Invitations are email to any address: a daily limit per inviter keeps an account from being used to spam. |
| 530 | 532 | if (isNew && !inviter.site_admin) { |
| 531 | 533 | const sent = (await this.db.prepare("SELECT count(*) AS n FROM invites WHERE invited_by = ? AND created_at > ?").bind(inviter.id, new Date(Date.now() - 86400_000).toISOString()).first<{ n: number }>())?.n ?? 0; |
| @@ -687,8 +689,11 @@ | ||
| 687 | 689 | } |
| 688 | 690 | } |
| 689 | 691 | |
| 692 | + /** Record a pending account's request. Administrators hear about it once a day at most, however often it is sent. */ | |
| 690 | 693 | async requestAccess(accountId: string, note: string): Promise<void> { |
| 694 | + const before = await this.db.prepare("SELECT created_at FROM access_requests WHERE account_id = ?").bind(accountId).first<{ created_at: string }>(); | |
| 691 | 695 | await this.db.prepare("INSERT OR REPLACE INTO access_requests (account_id, note, created_at) VALUES (?, ?, ?)").bind(accountId, note.slice(0, 500), now()).run(); |
| 696 | + if (before && Date.parse(before.created_at) > Date.now() - 86_400_000) return; | |
| 692 | 697 | const who = await this.account(accountId); |
| 693 | 698 | const admins = await this.db.prepare("SELECT id FROM accounts WHERE kind = 'user' AND site_admin = 1 AND status = 'active' LIMIT 20").all<{ id: string }>(); |
| 694 | 699 | if (who) await enqueueMail(this.env, admins.results.map((a) => ({ kind: "access_request" as const, data: { email: who.email ?? "", login: who.login, note: note.slice(0, 500) }, to: { accountId: a.id }, reason: "admin" as const, at: now() }))); |
| @@ -1110,7 +1115,11 @@ | ||
| 1110 | 1115 | if (!ownerRole) return null; |
| 1111 | 1116 | const explicit = await this.db.prepare("SELECT role FROM repo_access WHERE repo_id = ? AND account_id = ?").bind(repo.id, account.id).first<{ role: Role }>(); |
| 1112 | 1117 | const policy = account.agent_policy ? (JSON.parse(account.agent_policy) as { maxRole?: Role }) : {}; |
| 1113 | - const cap: Role = explicit?.role ?? policy.maxRole ?? "write"; | |
| 1118 | + // A grant on the repository can lower an agent's role, never lift it past the maximum its owner set: | |
| 1119 | + // any of the repository's admins can grant, and only the owner decides how far the agent goes. | |
| 1120 | + const max = policy.maxRole && Object.hasOwn(ROLE_LEVEL, policy.maxRole) ? policy.maxRole : null; | |
| 1121 | + const granted = explicit?.role ?? max ?? "write"; | |
| 1122 | + const cap: Role = max && ROLE_LEVEL[granted] > ROLE_LEVEL[max] ? max : granted; | |
| 1114 | 1123 | return ROLE_LEVEL[ownerRole] < ROLE_LEVEL[cap] ? ownerRole : cap; |
| 1115 | 1124 | } |
| 1116 | 1125 | if (account.site_admin) return "admin"; |
modifiedapps/diffinery/src/lib/quotas.ts+7−0View file
| @@ -141,6 +141,13 @@ | ||
| 141 | 141 | return `${login} has reached its storage limit of ${q.storage_bytes} bytes (git objects, Git LFS objects, release assets and packages); delete some or ask a site administrator to raise it`; |
| 142 | 142 | } |
| 143 | 143 | |
| 144 | +/** Bytes the account may still store: Infinity when it has no limit. */ | |
| 145 | +export async function storageHeadroom(env: Env, accountId: string): Promise<number> { | |
| 146 | + const q = await quotasFor(env, accountId); | |
| 147 | + if (q.exempt || q.storage_bytes < 0) return Infinity; | |
| 148 | + return Math.max(0, q.storage_bytes - (await storedBytes(env, accountId))); | |
| 149 | +} | |
| 150 | + | |
| 144 | 151 | /** Refuse a write past the account's storage quota (see storageRefusal). */ |
| 145 | 152 | export async function checkStorageQuota(env: Env, accountId: string, login: string, adding = 0): Promise<void> { |
| 146 | 153 | const refused = await storageRefusal(env, accountId, login, adding); |
modifiedapps/diffinery/src/registry/http.ts+26−4View file
| @@ -3,7 +3,7 @@ | ||
| 3 | 3 | import type { AppContext, AppVars } from "../auth/principal.js"; |
| 4 | 4 | import type { Account } from "../lib/directory.js"; |
| 5 | 5 | import { HttpError } from "../lib/util.js"; |
| 6 | -import { checkStorageQuota } from "../lib/quotas.js"; | |
| 6 | +import { checkStorageQuota, storageHeadroom } from "../lib/quotas.js"; | |
| 7 | 7 | import { allows, packageAccess, type PackageAccess, type PackageRole } from "./access.js"; |
| 8 | 8 | import { DIGEST_RE, NAME_RE, RegistryStore, TAG_RE, manifestKey, type ManifestRow, type PackageRow, type UploadRow } from "./store.js"; |
| 9 | 9 | import { UploadError, digestOf, discard, ingest } from "./upload.js"; |
| @@ -231,6 +231,28 @@ | ||
| 231 | 231 | return checkStorageQuota(c.env, t.owner.id, t.owner.login, sofar + (Number(c.req.header("content-length")) || 0)); |
| 232 | 232 | } |
| 233 | 233 | |
| 234 | +/** The largest blob Diffinery stores (GitHub's container registry takes layers up to 10 GB). */ | |
| 235 | +export const MAX_BLOB_BYTES = 10 * 1024 ** 3; | |
| 236 | + | |
| 237 | +/** How big this upload may grow: the blob cap, and what is left of the owner's storage quota. */ | |
| 238 | +async function uploadLimit(c: AppContext, t: Target): Promise<{ bytes: number; error: () => Error }> { | |
| 239 | + const headroom = await storageHeadroom(c.env, t.owner.id); | |
| 240 | + if (headroom < MAX_BLOB_BYTES) return { bytes: headroom, error: () => new HttpError(403, `${t.owner.login} has reached its storage limit (git objects, Git LFS objects, release assets and packages); delete some or ask a site administrator to raise it`) }; | |
| 241 | + return { bytes: MAX_BLOB_BYTES, error: () => new UploadError(`a blob may be at most ${MAX_BLOB_BYTES / 1024 ** 3} GiB`) }; | |
| 242 | +} | |
| 243 | + | |
| 244 | +/** Append this request's body within uploadLimit. Past it, ingest has thrown the bytes away and the upload is gone too. */ | |
| 245 | +async function ingestCapped(c: AppContext, t: Target, u: UploadRow, final: boolean): Promise<UploadRow> { | |
| 246 | + const limit = await uploadLimit(c, t); | |
| 247 | + let over = false; | |
| 248 | + try { | |
| 249 | + return await ingest(c.env, u, c.req.raw.body, final, { bytes: limit.bytes, error: () => { over = true; return limit.error(); } }); | |
| 250 | + } catch (e) { | |
| 251 | + if (over) await t.store.deleteUpload(u.id); | |
| 252 | + throw e; | |
| 253 | + } | |
| 254 | +} | |
| 255 | + | |
| 234 | 256 | async function startUpload(c: AppContext, full: string): Promise<Response> { |
| 235 | 257 | const t = await target(c, full, "push"); |
| 236 | 258 | await checkStorage(c, t); |
| @@ -257,7 +279,7 @@ | ||
| 257 | 279 | if (digest) { |
| 258 | 280 | // Monolithic upload: the whole blob in the POST body. |
| 259 | 281 | if (!DIGEST_RE.test(digest)) throw new RegistryError(400, "DIGEST_INVALID", "only sha256 digests are supported"); |
| 260 | - return finishUpload(c, t, pkg, await ingest(c.env, u, c.req.raw.body, true), digest); | |
| 282 | + return finishUpload(c, t, pkg, await ingestCapped(c, t, u, true), digest); | |
| 261 | 283 | } |
| 262 | 284 | return new Response(null, { status: 202, headers: uploadHeaders(t, u) }); |
| 263 | 285 | } |
| @@ -286,7 +308,7 @@ | ||
| 286 | 308 | const { t, u } = await loadUpload(c, full, id); |
| 287 | 309 | checkContentRange(c, t, u); |
| 288 | 310 | await checkStorage(c, t, u.size); |
| 289 | - const next = await ingest(c.env, u, c.req.raw.body, false); | |
| 311 | + const next = await ingestCapped(c, t, u, false); | |
| 290 | 312 | await t.store.saveUpload(next); |
| 291 | 313 | return new Response(null, { status: 202, headers: uploadHeaders(t, next) }); |
| 292 | 314 | } |
| @@ -297,7 +319,7 @@ | ||
| 297 | 319 | if (!digest || !DIGEST_RE.test(digest)) throw new RegistryError(400, "DIGEST_INVALID", "the digest query parameter must be a sha256 digest"); |
| 298 | 320 | if (c.req.raw.body) checkContentRange(c, t, u); |
| 299 | 321 | await checkStorage(c, t, u.size); |
| 300 | - return finishUpload(c, t, pkg, await ingest(c.env, u, c.req.raw.body, true), digest); | |
| 322 | + return finishUpload(c, t, pkg, await ingestCapped(c, t, u, true), digest); | |
| 301 | 323 | } |
| 302 | 324 | |
| 303 | 325 | async function cancelUpload(c: AppContext, full: string, id: string): Promise<Response> { |
modifiedapps/diffinery/src/registry/upload.ts+9−1View file
| @@ -21,7 +21,7 @@ | ||
| 21 | 21 | * back to hash it (for a big layer that took longer than clients wait for a response). |
| 22 | 22 | * Uploads started before the state was kept are hashed when they finish. |
| 23 | 23 | */ |
| 24 | -export async function ingest(env: Env, u: UploadRow, body: ReadableStream<Uint8Array> | null, final: boolean): Promise<UploadRow> { | |
| 24 | +export async function ingest(env: Env, u: UploadRow, body: ReadableStream<Uint8Array> | null, final: boolean, limit?: { bytes: number; error: () => Error }): Promise<UploadRow> { | |
| 25 | 25 | const bucket = env.BUCKET; |
| 26 | 26 | const parts = JSON.parse(u.parts) as R2UploadedPart[]; |
| 27 | 27 | let mp = u.multipart_id ? bucket.resumeMultipartUpload(u.r2_key, u.multipart_id) : null; |
| @@ -54,6 +54,14 @@ | ||
| 54 | 54 | if (done) break; |
| 55 | 55 | if (!value?.length) continue; |
| 56 | 56 | added += value.length; |
| 57 | + // Counted as the bytes arrive: a chunked body declares no length to check up front. | |
| 58 | + if (limit && u.size + added > limit.bytes) { | |
| 59 | + // The upload can't finish, so none of it is kept: earlier chunks, parts this request | |
| 60 | + // already sent and the multipart upload it may have started (the caller drops the row). | |
| 61 | + await reader.cancel().catch(() => {}); | |
| 62 | + await discard(env, { ...u, multipart_id: mp?.uploadId ?? u.multipart_id }); | |
| 63 | + throw limit.error(); | |
| 64 | + } | |
| 57 | 65 | hasher?.update(value); |
| 58 | 66 | for (let off = 0; off < value.length; ) { |
| 59 | 67 | const n = Math.min(PART_SIZE - fill, value.length - off); |
modifiedapps/diffinery/src/repo/actions.ts+26−5View file
| @@ -48,6 +48,8 @@ | ||
| 48 | 48 | export const MAX_JOB_LOG = 16 * 1048576; |
| 49 | 49 | /** Every push reads and parses a commit's workflow files: at most this many, each at most this large. */ |
| 50 | 50 | export const MAX_WORKFLOW_FILES = 256; |
| 51 | +/** Jobs in one run, counting each matrix leg and each job of a called workflow: keeps a run's rows and its RunWorkflow steps bounded. */ | |
| 52 | +export const MAX_RUN_JOBS = 1000; | |
| 51 | 53 | export const MAX_WORKFLOW_BYTES = 512 * 1024; |
| 52 | 54 | /** Hosted jobs one repository runs at once (GitHub's free plan allows 20 per account); more wait their turn. */ |
| 53 | 55 | export const MAX_HOSTED_JOBS = 20; |
| @@ -133,6 +135,8 @@ | ||
| 133 | 135 | prior_actors?: string | null; |
| 134 | 136 | /** What a partial re-run keeps from the attempt it re-runs (JSON RerunCarry); null for every other run. */ |
| 135 | 137 | rerun_carry?: string | null; |
| 138 | + /** Commits that the run's `owner/repo/path@ref` calls resolved to when it was planned (JSON, "repoId@ref" → sha). */ | |
| 139 | + call_refs?: string | null; | |
| 136 | 140 | }; |
| 137 | 141 | |
| 138 | 142 | /** |
| @@ -833,6 +837,7 @@ | ||
| 833 | 837 | } |
| 834 | 838 | const { order, errors } = jobOrder(exp.workflow); |
| 835 | 839 | if (errors.length) return { jobs: [], error: errors.map((e) => e.message).join("; "), concurrencyBlocked: false }; |
| 840 | + if (order.flat().length > MAX_RUN_JOBS) return { jobs: [], error: `A run may have at most ${MAX_RUN_JOBS} jobs, those of called workflows included; this one has ${order.flat().length}.`, concurrencyBlocked: false }; | |
| 836 | 841 | const blocked = |
| 837 | 842 | !!run.concurrency_group && |
| 838 | 843 | this.sql.exec<{ n: number }>("SELECT count(*) AS n FROM runs WHERE concurrency_group = ? AND status = 'in_progress' AND id != ?", run.concurrency_group, runId).one().n > 0; |
| @@ -844,11 +849,21 @@ | ||
| 844 | 849 | private async expanded(run: RunRow): Promise<Expanded> { |
| 845 | 850 | const wf = (await this.loadWorkflows(run.sha)).find((w) => w.path === run.workflow_path); |
| 846 | 851 | if (!wf?.workflow) throw new ReusableError(`workflow file not found or invalid: ${wf?.errors.join("; ") || run.workflow_path}`); |
| 847 | - return expandReusable(wf.workflow, { repoId: null, sha: run.sha }, (uses, from) => this.loadCalled(uses, from)); | |
| 852 | + // A called workflow named by branch or tag runs the commit it pointed at when the run was planned, so a job | |
| 853 | + // that waits for approval runs the steps that were approved, however the branch moves meanwhile. | |
| 854 | + const saved = this.sql.exec<{ call_refs: string | null }>("SELECT call_refs FROM runs WHERE id = ?", run.id).toArray()[0]?.call_refs; | |
| 855 | + const pinned: Record<string, string> = saved ? JSON.parse(saved) : {}; | |
| 856 | + const before = Object.keys(pinned).length; | |
| 857 | + try { | |
| 858 | + return await expandReusable(wf.workflow, { repoId: null, sha: run.sha }, (uses, from) => this.loadCalled(uses, from, pinned)); | |
| 859 | + } finally { | |
| 860 | + // Kept even when a later call fails, so the next expansion (a re-plan while the run waits) sees the same commits. | |
| 861 | + if (Object.keys(pinned).length !== before) this.sql.exec("UPDATE runs SET call_refs = ? WHERE id = ?", JSON.stringify(pinned), run.id); | |
| 862 | + } | |
| 848 | 863 | } |
| 849 | 864 | |
| 850 | 865 | /** Load a called workflow: `./path` from the same commit, or `owner/repo/path@ref` from a repository on this instance. */ |
| 851 | - private async loadCalled(uses: string, from: Source): Promise<{ workflow: Workflow; source: Source }> { | |
| 866 | + private async loadCalled(uses: string, from: Source, pinned: Record<string, string> = {}): Promise<{ workflow: Workflow; source: Source }> { | |
| 852 | 867 | const ref = parseCallRef(uses); |
| 853 | 868 | const m = this.repo.meta; |
| 854 | 869 | let source: Source; |
| @@ -870,14 +885,14 @@ | ||
| 870 | 885 | repoId = target.id; |
| 871 | 886 | } |
| 872 | 887 | if (repoId === m.repoId) { |
| 873 | - const sha = ref.kind === "local" ? from.sha : await this.repo.resolve(ref.ref); | |
| 888 | + const sha = ref.kind === "local" ? from.sha : (pinned[`${repoId}@${ref.ref}`] ??= (await this.repo.resolve(ref.ref)) ?? ""); | |
| 874 | 889 | if (!sha) throw new Error(`unknown ref ${ref.kind === "remote" ? ref.ref : ""}`); |
| 875 | 890 | source = { repoId: null, sha }; |
| 876 | 891 | const b = await this.repo.blob(sha, ref.path); |
| 877 | 892 | text = b && !b.binary ? fromUtf8(b.content) : null; |
| 878 | 893 | } else { |
| 879 | 894 | const stub = this.env.REPO.get(this.env.REPO.idFromName(repoId)); |
| 880 | - const sha = ref.kind === "local" ? from.sha : await stub.resolve(ref.ref); | |
| 895 | + const sha = ref.kind === "local" ? from.sha : (pinned[`${repoId}@${ref.ref}`] ??= (await stub.resolve(ref.ref)) ?? ""); | |
| 881 | 896 | if (!sha) throw new Error(`unknown ref ${ref.kind === "remote" ? ref.ref : ""} in ${ref.kind === "remote" ? `${ref.owner}/${ref.repo}` : "the called repository"}`); |
| 882 | 897 | source = { repoId, sha }; |
| 883 | 898 | const b = await stub.blob(sha, ref.path); |
| @@ -1026,6 +1041,8 @@ | ||
| 1026 | 1041 | return fail(label(job.name ?? short), `Error expanding matrix: ${(e as Error).message}`); |
| 1027 | 1042 | } |
| 1028 | 1043 | if (combos.length > 256) return fail(label(job.name ?? short), "A matrix may produce at most 256 jobs."); |
| 1044 | + const started = this.sql.exec<{ n: number }>("SELECT count(*) AS n FROM jobs WHERE run_id = ?", run.id).one().n; | |
| 1045 | + if (started + combos.length > MAX_RUN_JOBS) return fail(label(job.name ?? short), `A run may have at most ${MAX_RUN_JOBS} jobs, matrix legs included.`); | |
| 1029 | 1046 | const failFast = evaluateValue(job.strategy?.failFast ?? true, baseCtx) !== false; |
| 1030 | 1047 | const maxParallel = Number(evaluateValue(job.strategy?.maxParallel ?? combos.length, baseCtx)) || combos.length; |
| 1031 | 1048 | const ids: string[] = []; |
| @@ -1877,7 +1894,11 @@ | ||
| 1877 | 1894 | const actor = JSON.parse(run.actor) as Actor; |
| 1878 | 1895 | if (actor.id) github.actor_id = String(numericId(actor.id)); |
| 1879 | 1896 | const env = this.sql.exec<{ environment: string }>("SELECT environment FROM deployments WHERE job_id = ?", jobId).toArray()[0]; |
| 1880 | - if (env) github.environment = env.environment; | |
| 1897 | + // The environment claim vouches for the job like the environment's secrets do: a job refused those | |
| 1898 | + // (a pull request ref, or agent code the policy keeps them from) gets the ref or pull request subject. | |
| 1899 | + if (env && this.repo.deploys.secretsAllowed(env.environment, run.ref, run.agent_origin ? "agent" : actor.kind)) github.environment = env.environment; | |
| 1900 | + // Lets a cloud trust policy refuse code an agent wrote, whatever ref or environment it ran on. | |
| 1901 | + github.agent_origin = run.agent_origin ? "true" : "false"; | |
| 1881 | 1902 | return { allowed, github }; |
| 1882 | 1903 | } |
| 1883 | 1904 |
modifiedapps/diffinery/src/repo/collab.ts+19−3View file
| @@ -400,8 +400,18 @@ | ||
| 400 | 400 | checkText(r.kind === "pull" ? "PullRequest" : "Issue", patch); |
| 401 | 401 | const isAuthor = r.author_id === caller.id; |
| 402 | 402 | if (!isAuthor && !atLeast(caller.role, "triage")) throw new HttpError(403, "permission denied"); |
| 403 | - // As when creating: authors without triage access can't set labels or assignees, which are silently dropped. | |
| 404 | - if (!atLeast(caller.role, "triage")) patch = { ...patch, labels: undefined, assignees: undefined }; | |
| 403 | + if (!atLeast(caller.role, "triage")) { | |
| 404 | + // As on GitHub, an author without triage access takes no part once they're blocked or the conversation is locked, | |
| 405 | + // and can't reopen what someone else closed. | |
| 406 | + refuseBlocked(caller); | |
| 407 | + if (r.locked) throw new HttpError(403, "conversation is locked"); | |
| 408 | + if (patch.state === "open" && r.state === "closed") { | |
| 409 | + const closer = this.sql.exec<{ actor_id: string }>("SELECT actor_id FROM issue_events WHERE number = ? AND event = 'closed' ORDER BY rowid DESC LIMIT 1", number).toArray()[0]; | |
| 410 | + if (closer && closer.actor_id !== caller.id) throw new HttpError(403, "only someone with triage access can reopen an issue someone else closed"); | |
| 411 | + } | |
| 412 | + // As when creating: labels and assignees are silently dropped. | |
| 413 | + patch = { ...patch, labels: undefined, assignees: undefined }; | |
| 414 | + } | |
| 405 | 415 | if (r.kind === "pull" && patch.state === "open" && this.pullRow(number).merged) throw new HttpError(422, "a merged pull request cannot be reopened"); |
| 406 | 416 | const t = now(); |
| 407 | 417 | const state = patch.state ?? r.state; |
| @@ -453,6 +463,7 @@ | ||
| 453 | 463 | const r = this.row(number); |
| 454 | 464 | if (r.kind !== "pull") throw new HttpError(404, "not a pull request"); |
| 455 | 465 | 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"); |
| 466 | + if (!atLeast(caller.role, "write")) refuseBlocked(caller); | |
| 456 | 467 | if (r.state !== "open") throw new HttpError(422, "the pull request is not open"); |
| 457 | 468 | const p = this.pullRow(number); |
| 458 | 469 | if (!!p.draft === draft) return this.toIssue(r); |
| @@ -507,9 +518,14 @@ | ||
| 507 | 518 | editComment(id: string, body: string, caller: Caller): void { |
| 508 | 519 | checkText("IssueComment", { body }); |
| 509 | 520 | id = this.repo.social.commentKey(id); |
| 510 | - const c = this.sql.exec<{ author_id: string }>("SELECT author_id FROM comments WHERE id = ?", id).toArray()[0]; | |
| 521 | + const c = this.sql.exec<{ author_id: string; number: number | null }>("SELECT author_id, number FROM comments WHERE id = ?", id).toArray()[0]; | |
| 511 | 522 | if (!c) throw new HttpError(404, "comment not found"); |
| 512 | 523 | if (c.author_id !== caller.id && !atLeast(caller.role, "maintain")) throw new HttpError(403, "permission denied"); |
| 524 | + if (!atLeast(caller.role, "maintain")) { | |
| 525 | + // Its author edits it only while they may still comment: not blocked, and the conversation not locked. | |
| 526 | + refuseBlocked(caller); | |
| 527 | + if (c.number != null && this.row(c.number).locked && !atLeast(caller.role, "triage")) throw new HttpError(403, "conversation is locked"); | |
| 528 | + } | |
| 513 | 529 | this.sql.exec("UPDATE comments SET body = ?, updated_at = ? WHERE id = ?", body, now(), id); |
| 514 | 530 | } |
| 515 | 531 |
modifiedapps/diffinery/src/repo/reusable.ts+11−2View file
| @@ -97,9 +97,18 @@ | ||
| 97 | 97 | |
| 98 | 98 | /** Parse a `uses:` reference to a reusable workflow. */ |
| 99 | 99 | export function parseCallRef(uses: string): { kind: "local"; path: string } | { kind: "remote"; owner: string; repo: string; path: string; ref: string } { |
| 100 | + // Only from the workflow directories, which agents' auto-merge and job tokens treat as workflow changes, | |
| 101 | + // here or in the called repository (as on GitHub). | |
| 102 | + const inWorkflowDir = (path: string) => /^\.(?:github|diffinery)\/workflows\//.test(path) && !path.split("/").some((s) => s === ".." || s === "." || s === ""); | |
| 100 | 103 | const local = /^\.\/(.+\.ya?ml)$/.exec(uses); |
| 101 | - if (local) return { kind: "local", path: local[1]! }; | |
| 104 | + if (local) { | |
| 105 | + if (!inWorkflowDir(local[1]!)) throw new Error(`'${uses}' must be a file in ./.github/workflows or ./.diffinery/workflows`); | |
| 106 | + return { kind: "local", path: local[1]! }; | |
| 107 | + } | |
| 102 | 108 | const remote = /^([A-Za-z0-9][\w.-]*)\/([\w.-]+)\/(.+\.ya?ml)@(.+)$/.exec(uses); |
| 103 | - if (remote) return { kind: "remote", owner: remote[1]!, repo: remote[2]!, path: remote[3]!, ref: remote[4]! }; | |
| 109 | + if (remote) { | |
| 110 | + if (!inWorkflowDir(remote[3]!)) throw new Error(`'${uses}' must name a file in .github/workflows or .diffinery/workflows of ${remote[1]}/${remote[2]}`); | |
| 111 | + return { kind: "remote", owner: remote[1]!, repo: remote[2]!, path: remote[3]!, ref: remote[4]! }; | |
| 112 | + } | |
| 104 | 113 | throw new Error(`'${uses}' must be ./.github/workflows/<file>.yml or <owner>/<repo>/.github/workflows/<file>.yml@<ref>`); |
| 105 | 114 | } |
modifiedapps/diffinery/src/repo/schema.ts+2−0View file
| @@ -231,6 +231,8 @@ | ||
| 231 | 231 | // What a partial re-run (failed jobs, or one job) keeps from the attempt it re-runs (JSON RerunCarry in |
| 232 | 232 | // repo/actions.ts): per job key, the earlier attempt's job rows copied into this one instead of running. |
| 233 | 233 | ["runs", "rerun_carry", "TEXT"], |
| 234 | + // Commits a run's remote reusable workflow calls resolved to at plan time (JSON, "repoId@ref" → sha). | |
| 235 | + ["runs", "call_refs", "TEXT"], | |
| 234 | 236 | // A job copied unchanged from an earlier attempt by a partial re-run: the job that actually ran, whose log it shows. |
| 235 | 237 | ["jobs", "carried_from", "TEXT"], |
| 236 | 238 | // Bytes an object takes as stored (compressed, in SQLite or R2); null in objects stored before it was kept. |
modifiedapps/diffinery/src/web/imports.tsx+1−1View file
| @@ -85,7 +85,7 @@ | ||
| 85 | 85 | <label for="github_token"> |
| 86 | 86 | GitHub token <span class="faint">(required for private repositories, LFS, variables and protection)</span> |
| 87 | 87 | </label> |
| 88 | - <input id="github_token" name="github_token" type="password" autocomplete="off" placeholder="ghp_… or a fine-grained token" value={values.github_token} /> | |
| 88 | + <input id="github_token" name="github_token" type="password" autocomplete="off" placeholder="ghp_… or a fine-grained token" /> | |
| 89 | 89 | <p class="muted small m0">A read token (classic `repo`, or a fine-grained token with Contents, Issues, Pull requests, Metadata). Used only for this import, encrypted at rest, and deleted when the import finishes.</p> |
| 90 | 90 | </div> |
| 91 | 91 | <details> |
modifiedapps/diffinery/test/e2e/moderation.test.mjs+7−0View file
| @@ -233,6 +233,9 @@ | ||
| 233 | 233 | const { town, park, pub, welcome } = await setup(); |
| 234 | 234 | const sidIssue = await status(201, "POST", `${town}/issues`, { title: "sid was here" }, T.sid); |
| 235 | 235 | const sidComment = await status(201, "POST", `${town}/issues/${welcome.number}/comments`, { body: "sid's comment" }, T.sid); |
| 236 | + await ok("PATCH", `${town}/issues/${sidIssue.number}`, { state: "closed" }); | |
| 237 | + const closedSid = (await ok("GET", `${town}/issues/events`)).find((e) => e.issue?.number === sidIssue.number); | |
| 238 | + assert.ok(closedSid, "closing sid's issue is an event"); | |
| 236 | 239 | await ok("GET", "/user", undefined, T.sid); |
| 237 | 240 | await ok("GET", pub, undefined, T.olive); |
| 238 | 241 | assert.equal((await page("/sid", S.olive)).status, 200); |
| @@ -251,6 +254,10 @@ | ||
| 251 | 254 | await status(404, "GET", "/users/sid", undefined, T.olive); |
| 252 | 255 | await status(404, "GET", `${town}/issues/${sidIssue.number}`, undefined, T.olive); |
| 253 | 256 | assert.ok(!(await ok("GET", `${town}/issues?state=all`)).some((i) => i.number === sidIssue.number)); |
| 257 | + // The events endpoints embed the issue, so its events are hidden too. | |
| 258 | + assert.ok(!(await ok("GET", `${town}/issues/events`)).some((e) => e.issue?.number === sidIssue.number)); | |
| 259 | + await status(404, "GET", `${town}/issues/events/${closedSid.id}`); | |
| 260 | + await status(404, "GET", `${town}/issues/${sidIssue.number}/events`); | |
| 254 | 261 | assert.ok(!(await ok("GET", `${town}/issues/${welcome.number}/comments`)).some((c) => c.id === sidComment.id)); |
| 255 | 262 | assert.doesNotMatch((await page(`/olive/town/issues/${welcome.number}`, S.olive)).html, /sid's comment|sid's comment/); |
| 256 | 263 | // ...but not for site administrators, who see why. |
modifiedapps/diffinery/test/e2e/security.test.mjs+27−0View file
| @@ -643,6 +643,17 @@ | ||
| 643 | 643 | const edited = await ok("PATCH", `${R}/issues/${fromReader.number}`, { title: "edited by its author", labels: ["bug"], assignees: [OWNER], milestone: milestone.number }, T.rita); |
| 644 | 644 | assert.equal(edited.title, "edited by its author"); |
| 645 | 645 | assert.deepEqual([edited.labels.length, edited.assignees.length, edited.milestone], [0, 0, null]); |
| 646 | + // They can close and reopen their own issue, but not reopen one a maintainer closed, nor edit it once it's locked. | |
| 647 | + assert.equal((await ok("PATCH", `${R}/issues/${fromReader.number}`, { state: "closed" }, T.rita)).state, "closed"); | |
| 648 | + assert.equal((await ok("PATCH", `${R}/issues/${fromReader.number}`, { state: "open" }, T.rita)).state, "open"); | |
| 649 | + await ok("PATCH", `${R}/issues/${fromReader.number}`, { state: "closed" }); | |
| 650 | + assert.match((await status(403, "PATCH", `${R}/issues/${fromReader.number}`, { state: "open" }, T.rita)).message, /reopen/); | |
| 651 | + await ok("PATCH", `${R}/issues/${fromReader.number}`, { state: "open" }); | |
| 652 | + const said = await status(201, "POST", `${R}/issues/${fromReader.number}/comments`, { body: "mine" }, T.rita); | |
| 653 | + await status(204, "PUT", `${R}/issues/${fromReader.number}/lock`, {}); | |
| 654 | + assert.match((await status(403, "PATCH", `${R}/issues/${fromReader.number}`, { body: "after the lock" }, T.rita)).message, /locked/); | |
| 655 | + assert.match((await status(403, "PATCH", `${R}/issues/comments/${said.id}`, { body: "after the lock" }, T.rita)).message, /locked/); | |
| 656 | + await status(204, "DELETE", `${R}/issues/${fromReader.number}/lock`); | |
| 646 | 657 | |
| 647 | 658 | // ------------------------------------------------------------ settings changed through the API are on the record |
| 648 | 659 | await collaborator(R, "rita", "push", T.rita); |
| @@ -1922,6 +1933,7 @@ | ||
| 1922 | 1933 | assert.equal(tokens.deploy.sub, `repo:${OWNER}/oidc-claims:environment:production`); |
| 1923 | 1934 | assert.equal(tokens.deploy.environment, "production"); |
| 1924 | 1935 | assert.equal(tokens.plain.environment, undefined); |
| 1936 | + assert.equal(tokens.plain.agent_origin, false); | |
| 1925 | 1937 | assert.equal(tokens.plain.aud, "sts.example.com"); |
| 1926 | 1938 | assert.equal(tokens.plain.repository_id, String(repo.id)); |
| 1927 | 1939 | assert.equal(tokens.plain.repository_owner_id, String(repo.owner.id)); |
| @@ -2496,6 +2508,21 @@ | ||
| 2496 | 2508 | assert.match((await late.json()).message, full); |
| 2497 | 2509 | assert.match((await status(403, "PUT", `${R}/contents/c.txt`, { message: "c", content: b64("more") }, T.milo)).message, full); |
| 2498 | 2510 | assert.match((await status(403, "POST", `${R}/git/blobs`, { content: "more" }, T.milo)).message, full); |
| 2511 | + // A chunked registry upload declares no length, so its bytes are counted as they arrive. | |
| 2512 | + const reg = { authorization: `Basic ${Buffer.from(`milo:${T.milo}`).toString("base64")}` }; | |
| 2513 | + const started = await fetch(`${base}/v2/milo/hoard/blobs/uploads/`, { method: "POST", headers: reg }); | |
| 2514 | + assert.equal(started.status, 202, await started.clone().text()); | |
| 2515 | + const chunked = new ReadableStream({ | |
| 2516 | + start(ctl) { | |
| 2517 | + ctl.enqueue(randomBytes(1000)); | |
| 2518 | + ctl.close(); | |
| 2519 | + }, | |
| 2520 | + }); | |
| 2521 | + const sneak = await fetch(`${base}${started.headers.get("location")}`, { method: "PATCH", headers: { ...reg, "content-type": "application/octet-stream" }, body: chunked, duplex: "half" }); | |
| 2522 | + assert.equal(sneak.status, 403); | |
| 2523 | + assert.match(await sneak.text(), /milo has reached its storage limit/); | |
| 2524 | + // An upload refused at the quota can't finish, so it is gone with its bytes: a retry has to start again. | |
| 2525 | + assert.equal((await fetch(`${base}${started.headers.get("location")}`, { headers: reg })).status, 404); | |
| 2499 | 2526 | |
| 2500 | 2527 | // ------------------------------------------------------------ reads, ref deletions and ref moves still work |
| 2501 | 2528 | assert.equal((await ok("GET", `${R}/contents/a.txt`, undefined, T.milo)).size, 4000); |
modifiedapps/diffinery/test/unit/imports.test.ts+4−0View file
| @@ -88,6 +88,10 @@ | ||
| 88 | 88 | expect(normalizeApiUrl("", false)).toBe("https://api.github.com"); |
| 89 | 89 | expect(normalizeApiUrl("https://github.com", false)).toBe("https://api.github.com"); |
| 90 | 90 | expect(normalizeApiUrl("https://ghe.example.com", false)).toBe("https://ghe.example.com/api/v3"); |
| 91 | + // The import container clones from this host itself: no other ports, and never Diffinery. | |
| 92 | + expect(normalizeApiUrl("https://ghe.example.com:443", false)).toBe("https://ghe.example.com/api/v3"); | |
| 93 | + expect(() => normalizeApiUrl("https://ghe.example.com:8443", false)).toThrow(/port/); | |
| 94 | + expect(() => normalizeApiUrl("https://diffinery.com/api/v3", false, "diffinery.com")).toThrow(/Diffinery itself/); | |
| 91 | 95 | }); |
| 92 | 96 | }); |
| 93 | 97 |
modifiedapps/diffinery/test/unit/reusable.test.ts+7−0View file
| @@ -97,5 +97,12 @@ | ||
| 97 | 97 | expect(parseCallRef("./.github/workflows/a.yml")).toEqual({ kind: "local", path: ".github/workflows/a.yml" }); |
| 98 | 98 | expect(parseCallRef("octo/tools/.github/workflows/b.yaml@v1")).toEqual({ kind: "remote", owner: "octo", repo: "tools", path: ".github/workflows/b.yaml", ref: "v1" }); |
| 99 | 99 | expect(() => parseCallRef("octo/tools/.github/workflows/b.yaml")).toThrow(); |
| 100 | + expect(parseCallRef("./.diffinery/workflows/c.yml")).toEqual({ kind: "local", path: ".diffinery/workflows/c.yml" }); | |
| 101 | + // A called workflow outside the workflow directories would dodge the guards on workflow changes. | |
| 102 | + expect(() => parseCallRef("./ci/build.yml")).toThrow(/workflows/); | |
| 103 | + expect(() => parseCallRef("./.github/workflows/../../ci/build.yml")).toThrow(/workflows/); | |
| 104 | + expect(() => parseCallRef("octo/tools/ci/build.yml@main")).toThrow(/workflows/); | |
| 105 | + expect(() => parseCallRef("octo/tools/.github/workflows/../ci/build.yml@main")).toThrow(/workflows/); | |
| 106 | + expect(parseCallRef("octo/tools/.diffinery/workflows/d.yml@main")).toEqual({ kind: "remote", owner: "octo", repo: "tools", path: ".diffinery/workflows/d.yml", ref: "main" }); | |
| 100 | 107 | }); |
| 101 | 108 | }); |