Required checks gate direct pushes, linear-time Markdown, safe redirects, text limits, R2 purge on delete, and fsck on push (#74)
* Required checks gate direct pushes to a protected branch, as on GitHub Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yhgAu6oKfqes812Yd4qjo * DEV_AUTH only works on the local machine Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yhgAu6oKfqes812Yd4qjo * Markdown block patterns run in linear time, and sign-in redirects can't be bent off-site Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yhgAu6oKfqes812Yd4qjo * Issue, pull request and comment text has GitHub's length limits Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yhgAu6oKfqes812Yd4qjo * Markdown hard breaks and long fence runs render in linear time Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yhgAu6oKfqes812Yd4qjo * Deleting a repository deletes its git objects, LFS files and release assets from R2 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yhgAu6oKfqes812Yd4qjo * Pushes are checked like git's fsck: no '..' or '.git' tree entries, duplicates or unsafe .gitmodules Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016yhgAu6oKfqes812Yd4qjo --------- Co-authored-by: Claude <noreply@anthropic.com>
Canberk Demirci authored · GitHub committed signed1 parent 83c1b97commit d305aa276ae3cc33030a55bf2569e891aa46c474Browse files
27 files changed+563−44
Files (27)
- apps/diffinery/docs/reference/limits.md+1−0
- apps/diffinery/docs/repositories/branch-protection.md+2−0
- apps/diffinery/docs/repositories/overview.md+1−1
- apps/diffinery/docs/security/overview.md+3−1
- apps/diffinery/docs/self-hosting/architecture.md+1−1
- apps/diffinery/docs/self-hosting/configuration.md+3−3
- apps/diffinery/src/auth/dev.ts+21−0
- apps/diffinery/src/auth/invite-proof.ts+2−1
- apps/diffinery/src/auth/principal.ts+3−2
- apps/diffinery/src/git/fsck.ts+88−0
- apps/diffinery/src/imports/service.ts+2−1
- apps/diffinery/src/lib/directory.ts+1−1
- apps/diffinery/src/lib/storage-keys.ts+5−2
- apps/diffinery/src/lib/util.ts+12−0
- apps/diffinery/src/mail/api.ts+2−1
- apps/diffinery/src/mail/send.ts+2−1
- apps/diffinery/src/mail/unsubscribe.ts+2−1
- apps/diffinery/src/repo/RepoDO.ts+40−5
- apps/diffinery/src/repo/collab.ts+8−1
- apps/diffinery/src/repo/social.ts+2−1
- apps/diffinery/src/web/account.tsx+2−3
- apps/diffinery/src/web/auth.tsx+10−1
- apps/diffinery/src/web/markdown.ts+74−17
- apps/diffinery/test/e2e/security.test.mjs+99−0
- apps/diffinery/test/unit/dev-auth.test.ts+26−0
- apps/diffinery/test/unit/fsck.test.ts+59−0
- apps/diffinery/test/unit/markdown-safety.test.ts+92−0
modifiedapps/diffinery/docs/reference/limits.md+1−0View file
| @@ -60,6 +60,7 @@ | ||
| 60 | 60 | | Topics | 20 per repository, 50 characters each | `PUT .../topics`. Lowercase letters, digits and hyphens, starting with a letter or digit. | |
| 61 | 61 | | Labels | Names up to 100 characters; 100 labels per issue | The REST API. The web UI's label form allows names up to 50 characters. | |
| 62 | 62 | | Assignees | 10 per issue or pull request | `an issue can have at most 10 assignees`. | |
| 63 | +| Issue and pull request text | Titles up to 256 characters; bodies, comments and review text up to 65,536 characters | As on GitHub, everywhere they're written (web, REST, MCP). Longer text fails with `422 body is too long (maximum is 65536 characters)`. | | |
| 63 | 64 | | `repository_dispatch` | `event_type` up to 100 characters; `client_payload` up to 10 top-level properties and 64 KiB | `POST .../dispatches`. | |
| 64 | 65 | | Check run name | 255 characters | `POST .../check-runs`. | |
| 65 | 66 | | Deployment `required_contexts` | 100 contexts | `POST .../deployments`. | |
modifiedapps/diffinery/docs/repositories/branch-protection.md+2−0View file
| @@ -91,6 +91,8 @@ | ||
| 91 | 91 | |
| 92 | 92 | A required check with no counted report blocks the merge with `required check '<name>' has not run`. That includes a check that only an agent reported. Make sure the workflow or CI system runs for every pull request into the protected branch. |
| 93 | 93 | |
| 94 | +Required checks also gate direct pushes, as on GitHub. A push, an API commit or a ref update that moves the protected branch is accepted only when its new commit has already passed every required check, counted the same way. Otherwise it is refused with `protected branch: required check '<name>' has not run on <sha>; push to another branch and open a pull request` (or `has not finished`, or `ended with <conclusion>`). Repository administrators are exempt unless `enforceAdmins` is set. | |
| 95 | + | |
| 94 | 96 | > **Note:** GitHub names an Actions check run after the job only. In Diffinery the check name includes the workflow name, so required check names copied from GitHub usually need the `<workflow> / ` prefix added. |
| 95 | 97 | |
| 96 | 98 | ### Binding a check to a reporter |
modifiedapps/diffinery/docs/repositories/overview.md+1−1View file
| @@ -384,7 +384,7 @@ | ||
| 384 | 384 | |
| 385 | 385 | ## Delete a repository |
| 386 | 386 | |
| 387 | -Deleting permanently removes the repository with its history, issues, pull requests, releases, runs, secrets, environments and webhooks, and its Actions caches, artifacts, job logs and cached source archives. It can't be undone. An archived repository can be deleted without unarchiving it first. | |
| 387 | +Deleting permanently removes the repository with its history, issues, pull requests, releases, runs, secrets, environments and webhooks, its LFS files and release assets, and its Actions caches, artifacts, job logs and cached source archives. It can't be undone. An archived repository can be deleted without unarchiving it first. | |
| 388 | 388 | |
| 389 | 389 | - **Web UI**: **Settings → General → Danger zone → Delete this repository**. Type the full name (for example `acme/website`) and click **Delete this repository**. |
| 390 | 390 | - **API**: `DELETE /api/v3/repos/acme/website` returns `204 No Content`. It needs an admin role. Agents can't delete repositories (`agents cannot delete repositories`, HTTP 403). |
modifiedapps/diffinery/docs/security/overview.md+3−1View file
| @@ -190,8 +190,10 @@ | ||
| 190 | 190 | - contains `\` or a NUL character, |
| 191 | 191 | - is longer than 255 characters, |
| 192 | 192 | |
| 193 | -or when the whole path is longer than 4,096 characters. The commit APIs answer `422` with `invalid path "<path>": <reason>` (for example `invalid path ".git/config": '.git' is reserved`), and the git data API with `invalid tree path ...`. Git's own `fsck` rejects the same names. Downloaded `zipball` and `tarball` archives leave such entries, and everything under them, out. Pushes made with git are not checked for these names. | |
| 193 | +or when the whole path is longer than 4,096 characters. The commit APIs answer `422` with `invalid path "<path>": <reason>` (for example `invalid path ".git/config": '.git' is reserved`), and the git data API with `invalid tree path ...`. Git's own `fsck` rejects the same names. Downloaded `zipball` and `tarball` archives leave such entries, and everything under them, out. | |
| 194 | 194 | |
| 195 | +Pushes made with git are checked the way GitHub checks them with git's `fsck`. A push is refused as a whole, and nothing from it is kept, when a tree in it has an entry that is empty, `.`, `..` or a spelling of `.git`, contains `/`, or appears twice; when `.gitmodules` (in any spelling) is a symbolic link; or when a `.gitmodules` file names a submodule with a `..` part, or gives a `url` or `path` that starts with `-`, which older git clients run as an option. Git prints `remote: Push refused: <reason>`. Names with `\` are allowed in pushes, as in git. | |
| 196 | + | |
| 195 | 197 | ## Secrets encryption |
| 196 | 198 | |
| 197 | 199 | Actions secrets (repository, environment and organization) are encrypted at rest with envelope encryption: |
modifiedapps/diffinery/docs/self-hosting/architecture.md+1−1View file
| @@ -92,7 +92,7 @@ | ||
| 92 | 92 | - **Merge queue.** One queue per protected branch. The object's single alarm wakes it when a merge group's check timeout runs out. |
| 93 | 93 | - **Live updates.** WebSocket connections, see [Live updates](#live-updates). |
| 94 | 94 | |
| 95 | -Deleting a repository wipes the object's storage and deletes the repository's caches, artifacts, archived job logs, unfinished uploads and cached source archives from R2. | |
| 95 | +Deleting a repository wipes the object's storage and deletes all of the repository's R2 objects: large git objects, LFS files, release assets, caches, artifacts, archived job logs, unfinished uploads and cached source archives. A repository with more objects than one call can delete is finished by the object's alarm. | |
| 96 | 96 | |
| 97 | 97 | ## D1 and read replication |
| 98 | 98 |
modifiedapps/diffinery/docs/self-hosting/configuration.md+3−3View file
| @@ -57,9 +57,9 @@ | ||
| 57 | 57 | |
| 58 | 58 | ### DEV_AUTH |
| 59 | 59 | |
| 60 | -When `DEV_AUTH` is set and `ACCESS_AUD` is empty, every request without credentials (no token, no session) acts as the account with the email in `DEV_AUTH`, with full access. That account is created on first use and is a site administrator if the email is also in `BOOTSTRAP_ADMINS`. While `DEV_AUTH` is set, the email log also keeps each whole message, and `GET /api/v3/diffinery/emails` returns it, so tests can read emails without delivering them. | |
| 60 | +When `DEV_AUTH` is set, `ACCESS_AUD` is empty and both `PUBLIC_URL` and the request's own address are on the local machine (`localhost`, `127.x.x.x` or `[::1]`), every request without credentials (no token, no session) acts as the account with the email in `DEV_AUTH`, with full access. That account is created on first use and is a site administrator if the email is also in `BOOTSTRAP_ADMINS`. While `DEV_AUTH` is set, the email log also keeps each whole message, and `GET /api/v3/diffinery/emails` returns it, so tests can read emails without delivering them. | |
| 61 | 61 | |
| 62 | -> **Important:** Never set `DEV_AUTH` on a deployed instance. Anyone on the internet would act as that account. | |
| 62 | +> **Important:** Never set `DEV_AUTH` on a deployed instance. As a safeguard it does nothing there: a request that reaches the Worker under any other host name, such as `workers.dev` or your domain, or a `PUBLIC_URL` that isn't on the local machine, turns it off, and so does Cloudflare Access. | |
| 63 | 63 | |
| 64 | 64 | ## Worker secrets |
| 65 | 65 | |
| @@ -145,7 +145,7 @@ | ||
| 145 | 145 | DIFFINERY_WORK_ROOT=/tmp/diffinery-import node containers/runner/dist/agent.mjs |
| 146 | 146 | ``` |
| 147 | 147 | |
| 148 | -Only the account that started the import can claim it. In local development (`DEV_AUTH` set and `ACCESS_AUD` empty), imports also accept plain `http://` and loopback GitHub API URLs, for test doubles. | |
| 148 | +Only the account that started the import can claim it. In local development (`DEV_AUTH` in effect, as described in [DEV_AUTH](#dev_auth)), imports also accept plain `http://` and loopback GitHub API URLs, for test doubles. | |
| 149 | 149 | |
| 150 | 150 | `MASTER_KEY` is not set in `dev`. Add it to `apps/diffinery/.dev.vars` (`MASTER_KEY=<base64 of 32 random bytes>`) or pass `--var MASTER_KEY:<value>` to `wrangler dev` to store Actions secrets locally. The end-to-end tests pass a random one with `--var`, together with `PUBLIC_URL`. |
| 151 | 151 |
addedapps/diffinery/src/auth/dev.ts+21−0View file
| @@ -1,0 +1,21 @@ | ||
| 1 | +import type { Env } from "../env.js"; | |
| 2 | + | |
| 3 | +const LOOPBACK = /^(localhost|127(\.\d{1,3}){3}|\[::1\])$/; | |
| 4 | +const loopback = (url: string | undefined): boolean => { | |
| 5 | + try { | |
| 6 | + return !!url && LOOPBACK.test(new URL(url).hostname); | |
| 7 | + } catch { | |
| 8 | + return false; | |
| 9 | + } | |
| 10 | +}; | |
| 11 | + | |
| 12 | +/** | |
| 13 | + * Local development's fixed sign-in (DEV_AUTH) makes every visitor that person, so it works only | |
| 14 | + * where nobody else can reach: PUBLIC_URL and the request itself must both be on this machine | |
| 15 | + * (localhost, 127.x or [::1]), and Cloudflare Access must be off. A DEV_AUTH left in a deployed | |
| 16 | + * configuration does nothing. Without a request, only PUBLIC_URL is checked. | |
| 17 | + */ | |
| 18 | +export function devAuth(env: Pick<Env, "DEV_AUTH" | "ACCESS_AUD" | "PUBLIC_URL">, requestUrl?: string): boolean { | |
| 19 | + if (!env.DEV_AUTH || env.ACCESS_AUD || !loopback(env.PUBLIC_URL)) return false; | |
| 20 | + return requestUrl === undefined || loopback(requestUrl); | |
| 21 | +} |
modifiedapps/diffinery/src/auth/invite-proof.ts+2−1View file
| @@ -1,3 +1,4 @@ | ||
| 1 | +import { devAuth } from "./dev.js"; | |
| 1 | 2 | import type { Env } from "../env.js"; |
| 2 | 3 | import { b64url, safeEqual } from "../lib/util.js"; |
| 3 | 4 | |
| @@ -10,7 +11,7 @@ | ||
| 10 | 11 | */ |
| 11 | 12 | async function key(env: Env): Promise<CryptoKey> { |
| 12 | 13 | // Local development has no MASTER_KEY unless one is passed; production always does. |
| 13 | - const secret = env.MASTER_KEY ?? (env.DEV_AUTH ? "diffinery-dev-invite-proof" : null); | |
| 14 | + const secret = env.MASTER_KEY ?? (devAuth(env) ? "diffinery-dev-invite-proof" : null); | |
| 14 | 15 | if (!secret) throw new Error("MASTER_KEY is not set, so invitation emails can't be signed"); |
| 15 | 16 | return crypto.subtle.importKey("raw", new TextEncoder().encode(`diffinery-invite-proof-v1:${secret}`), { name: "HMAC", hash: "SHA-256" }, false, ["sign"]); |
| 16 | 17 | } |
modifiedapps/diffinery/src/auth/principal.ts+3−2View file
| @@ -1,5 +1,6 @@ | ||
| 1 | 1 | import type { Context } from "hono"; |
| 2 | 2 | import type { Env } from "../env.js"; |
| 3 | +import { devAuth } from "./dev.js"; | |
| 3 | 4 | import { Directory, type Account, type Repo, type TokenScope } from "../lib/directory.js"; |
| 4 | 5 | import { Auth, SESSION_COOKIE, cookieFrom } from "./sessions.js"; |
| 5 | 6 | import { HttpError, fromB64 } from "../lib/util.js"; |
| @@ -86,8 +87,8 @@ | ||
| 86 | 87 | const s = await new Auth(env).session(session); |
| 87 | 88 | if (s) return { type: "account", account: s.account, via: "session", scopes: ["admin"], repoIds: null, sessionId: s.session.id }; |
| 88 | 89 | } |
| 89 | - if (env.DEV_AUTH && !env.ACCESS_AUD) { | |
| 90 | - const account = await dir.userForEmail(env.DEV_AUTH); | |
| 90 | + if (devAuth(env, req.url)) { | |
| 91 | + const account = await dir.userForEmail(env.DEV_AUTH!); | |
| 91 | 92 | return { type: "account", account, via: "dev", scopes: ["admin"], repoIds: null }; |
| 92 | 93 | } |
| 93 | 94 | return { type: "anonymous" }; |
addedapps/diffinery/src/git/fsck.ts+88−0View file
| @@ -1,0 +1,88 @@ | ||
| 1 | +import { fromUtf8, parseTree } from "@diffinery/git-core"; | |
| 2 | +import { isDotGitName } from "../lib/tree-names.js"; | |
| 3 | + | |
| 4 | +/** | |
| 5 | + * Git's fsck for pushed objects, as GitHub runs it (`receive.fsckObjects`): trees a git | |
| 6 | + * client would refuse to check out, or that would write outside the work tree, and | |
| 7 | + * `.gitmodules` files that make a client run an option as a command (CVE-2018-17456) | |
| 8 | + * or clone a submodule outside `.git/modules` (CVE-2018-11235). | |
| 9 | + */ | |
| 10 | + | |
| 11 | +export interface PushedObject { | |
| 12 | + oid: string; | |
| 13 | + type: string; | |
| 14 | + content: Uint8Array; | |
| 15 | +} | |
| 16 | + | |
| 17 | +// Characters HFS+ ignores when comparing names, as in tree-names.ts. | |
| 18 | +const HFS_IGNORABLE = /[---]/g; | |
| 19 | + | |
| 20 | +/** `.gitmodules` as Windows (8.3 short name, trailing dots and spaces) and macOS read it. */ | |
| 21 | +export function isDotGitmodulesName(name: string): boolean { | |
| 22 | + const n = name.replace(HFS_IGNORABLE, "").toLowerCase().replace(/[. ]+$/, ""); | |
| 23 | + return n === ".gitmodules" || n === "gitmod~1"; | |
| 24 | +} | |
| 25 | + | |
| 26 | +/** Why a submodule's name, path or URL in `.gitmodules` is unsafe, or null. */ | |
| 27 | +export function unsafeGitmodules(text: string): string | null { | |
| 28 | + let section: string | null = null; | |
| 29 | + for (const raw of text.split("\n")) { | |
| 30 | + const line = raw.trim(); | |
| 31 | + if (!line || line.startsWith("#") || line.startsWith(";")) continue; | |
| 32 | + const head = /^\[\s*submodule\s+"((?:[^"\\]|\\.)*)"\s*\]/i.exec(line); | |
| 33 | + if (head) { | |
| 34 | + section = head[1]!.replace(/\\(.)/g, "$1"); | |
| 35 | + if (section.split(/[/\\]/).includes("..")) return `submodule name '${section}' can't contain '..'`; | |
| 36 | + continue; | |
| 37 | + } | |
| 38 | + if (line.startsWith("[")) { | |
| 39 | + section = null; | |
| 40 | + continue; | |
| 41 | + } | |
| 42 | + if (section === null) continue; | |
| 43 | + const kv = /^([A-Za-z][A-Za-z0-9-]*)\s*=\s*(.*)$/.exec(line); | |
| 44 | + if (!kv) continue; | |
| 45 | + const key = kv[1]!.toLowerCase(); | |
| 46 | + const value = kv[2]!.replace(/^"(.*)"$/, "$1").trim(); | |
| 47 | + if ((key === "url" || key === "path") && value.startsWith("-")) return `submodule '${section}' has a ${key} that starts with '-'`; | |
| 48 | + } | |
| 49 | + return null; | |
| 50 | +} | |
| 51 | + | |
| 52 | +/** Why the pushed objects can't be accepted, or null when they pass. */ | |
| 53 | +export function fsckObjects(objects: PushedObject[]): string | null { | |
| 54 | + const gitmodules = new Set<string>(); | |
| 55 | + for (const o of objects) { | |
| 56 | + if (o.type !== "tree") continue; | |
| 57 | + let entries; | |
| 58 | + try { | |
| 59 | + entries = parseTree(o.content); | |
| 60 | + } catch { | |
| 61 | + return `tree ${o.oid}: malformed tree`; | |
| 62 | + } | |
| 63 | + const seen = new Set<string>(); | |
| 64 | + for (const e of entries) { | |
| 65 | + const why = | |
| 66 | + !e.name ? "empty name" | |
| 67 | + : e.name === "." || e.name === ".." ? `'${e.name}' is not a valid name` | |
| 68 | + : e.name.includes("/") ? `'${e.name}' contains '/'` | |
| 69 | + : isDotGitName(e.name) ? `'${e.name}' is reserved ('.git')` | |
| 70 | + : seen.has(e.name) ? `duplicate entry '${e.name}'` | |
| 71 | + : null; | |
| 72 | + if (why) return `tree ${o.oid}: ${why}`; | |
| 73 | + seen.add(e.name); | |
| 74 | + if (isDotGitmodulesName(e.name)) { | |
| 75 | + if (e.mode === "120000") return `tree ${o.oid}: '.gitmodules' is a symbolic link`; | |
| 76 | + gitmodules.add(e.oid); | |
| 77 | + } | |
| 78 | + } | |
| 79 | + } | |
| 80 | + if (gitmodules.size) { | |
| 81 | + for (const o of objects) { | |
| 82 | + if (o.type !== "blob" || !gitmodules.has(o.oid)) continue; | |
| 83 | + const why = unsafeGitmodules(fromUtf8(o.content)); | |
| 84 | + if (why) return `.gitmodules ${o.oid}: ${why}`; | |
| 85 | + } | |
| 86 | + } | |
| 87 | + return null; | |
| 88 | +} |
modifiedapps/diffinery/src/imports/service.ts+2−1View file
| @@ -1,3 +1,4 @@ | ||
| 1 | +import { devAuth } from "../auth/dev.js"; | |
| 1 | 2 | import { NonRetryableError } from "cloudflare:workflows"; |
| 2 | 3 | import type { ImportGitResult, ImportProgress, ImportSecretFinding, ImportSpec } from "@diffinery/shared"; |
| 3 | 4 | import type { Env } from "../env.js"; |
| @@ -305,7 +306,7 @@ | ||
| 305 | 306 | |
| 306 | 307 | /** Local development: plain HTTP and loopback GitHub doubles are allowed. */ |
| 307 | 308 | get dev(): boolean { |
| 308 | - return !!this.env.DEV_AUTH && !this.env.ACCESS_AUD; | |
| 309 | + return devAuth(this.env); | |
| 309 | 310 | } |
| 310 | 311 | |
| 311 | 312 | private get db() { |
modifiedapps/diffinery/src/lib/directory.ts+1−1View file
| @@ -622,7 +622,7 @@ | ||
| 622 | 622 | this.db.prepare("DELETE FROM repo_redirects WHERE repo_id = ?").bind(repo.id), |
| 623 | 623 | this.db.prepare("DELETE FROM repos WHERE id = ?").bind(repo.id), |
| 624 | 624 | ]); |
| 625 | - // R2 objects under repos/<id>/ are removed by the cleanup job; the DO's storage is wiped here. | |
| 625 | + // Wipes the Durable Object's storage, then the repository's R2 objects. | |
| 626 | 626 | await this.env.REPO.get(this.env.REPO.idFromName(repo.id)).destroy(); |
| 627 | 627 | } |
| 628 | 628 |
modifiedapps/diffinery/src/lib/storage-keys.ts+5−2View file
| @@ -20,8 +20,8 @@ | ||
| 20 | 20 | * age rule on `repos/` would have deleted git objects with them); reads fall back |
| 21 | 21 | * to those keys, listed after the current one. |
| 22 | 22 | * |
| 23 | - * Deleting a repository removes its objects under the transient prefixes | |
| 24 | - * ({@link transientPrefixes}); nothing else would remove its caches. | |
| 23 | + * Deleting a repository removes all of its objects ({@link repoPrefixes}), git | |
| 24 | + * objects, LFS and release assets included. | |
| 25 | 25 | */ |
| 26 | 26 | |
| 27 | 27 | export const STAGING_ROOT = "actions-staging/"; |
| @@ -44,6 +44,9 @@ | ||
| 44 | 44 | return [`actions-cache/${repo}/`, `artifacts/${repo}/`, `logs/${repo}/`, `${STAGING_ROOT}${repo}/`, archivePrefix(repo)]; |
| 45 | 45 | } |
| 46 | 46 | |
| 47 | +/** Every prefix a repository's objects live under, for deleting the repository. */ | |
| 48 | +export const repoPrefixes = (repo: string): string[] => [`repos/${repo}/`, ...transientPrefixes(repo)]; | |
| 49 | + | |
| 47 | 50 | type ListDelete = { |
| 48 | 51 | list(o: { prefix: string; cursor?: string; limit?: number }): Promise<{ objects: Array<{ key: string }>; truncated: boolean; cursor?: string }>; |
| 49 | 52 | delete(keys: string | string[]): Promise<unknown>; |
modifiedapps/diffinery/src/lib/util.ts+12−0View file
| @@ -56,6 +56,18 @@ | ||
| 56 | 56 | return r === 0; |
| 57 | 57 | } |
| 58 | 58 | |
| 59 | +/** GitHub's limits on issue, pull request and comment text. */ | |
| 60 | +export const MAX_TITLE = 256; | |
| 61 | +export const MAX_BODY = 65_536; | |
| 62 | + | |
| 63 | +/** Refuses a title or body over GitHub's limits, with GitHub's error. */ | |
| 64 | +export function checkText(resource: string, text: { title?: unknown; body?: unknown }): void { | |
| 65 | + for (const [field, max] of [["title", MAX_TITLE], ["body", MAX_BODY]] as const) { | |
| 66 | + const v = text[field]; | |
| 67 | + if (typeof v === "string" && v.length > max) throw new HttpError(422, `${field} is too long (maximum is ${max} characters)`, { errors: [{ resource, code: "custom", field }] }); | |
| 68 | + } | |
| 69 | +} | |
| 70 | + | |
| 59 | 71 | export class HttpError extends Error { |
| 60 | 72 | /** The message without the status prefix. */ |
| 61 | 73 | readonly detail: string; |
modifiedapps/diffinery/src/mail/api.ts+2−1View file
| @@ -1,3 +1,4 @@ | ||
| 1 | +import { devAuth } from "../auth/dev.js"; | |
| 1 | 2 | import { Hono } from "hono"; |
| 2 | 3 | import type { Env } from "../env.js"; |
| 3 | 4 | import { requireAccount, requireAccountScope, type AppContext, type AppVars } from "../auth/principal.js"; |
| @@ -112,7 +113,7 @@ | ||
| 112 | 113 | limit: Number(c.req.query("per_page") ?? 50) || 50, |
| 113 | 114 | }); |
| 114 | 115 | // The full message is kept only in local development, for tests. |
| 115 | - return c.json(rows.map((r) => ({ ...r, dev_copy: c.env.DEV_AUTH && r.dev_copy ? JSON.parse(r.dev_copy) : undefined }))); | |
| 116 | + return c.json(rows.map((r) => ({ ...r, dev_copy: devAuth(c.env, c.req.url) && r.dev_copy ? JSON.parse(r.dev_copy) : undefined }))); | |
| 116 | 117 | }); |
| 117 | 118 | |
| 118 | 119 | /** Send yourself a test email right away; returns what Cloudflare said. */ |
modifiedapps/diffinery/src/mail/send.ts+2−1View file
| @@ -1,3 +1,4 @@ | ||
| 1 | +import { devAuth } from "../auth/dev.js"; | |
| 1 | 2 | import type { Env } from "../env.js"; |
| 2 | 3 | import { Directory } from "../lib/directory.js"; |
| 3 | 4 | import { MailStore, wantsEmail } from "./prefs.js"; |
| @@ -119,7 +120,7 @@ | ||
| 119 | 120 | } |
| 120 | 121 | |
| 121 | 122 | const msg = await buildMessage(env, job, to); |
| 122 | - const devCopy = env.DEV_AUTH ? JSON.stringify(msg) : null; | |
| 123 | + const devCopy = devAuth(env) ? JSON.stringify(msg) : null; | |
| 123 | 124 | const base = { account_id: to.accountId ?? null, to_addr: to.email, kind: job.kind, subject: msg.subject, dev_copy: devCopy }; |
| 124 | 125 | if (!env.EMAIL) { |
| 125 | 126 | await store.log({ ...base, status: "skipped", detail: "email sending is not configured (no EMAIL binding)", message_id: null }); |
modifiedapps/diffinery/src/mail/unsubscribe.ts+2−1View file
| @@ -1,3 +1,4 @@ | ||
| 1 | +import { devAuth } from "../auth/dev.js"; | |
| 1 | 2 | import type { Env } from "../env.js"; |
| 2 | 3 | import { b64url, fromB64url, safeEqual } from "../lib/util.js"; |
| 3 | 4 | import type { Category } from "./types.js"; |
| @@ -22,7 +23,7 @@ | ||
| 22 | 23 | |
| 23 | 24 | async function key(env: Env): Promise<CryptoKey> { |
| 24 | 25 | // Local development has no MASTER_KEY unless one is passed; production always does. |
| 25 | - const secret = env.MASTER_KEY ?? (env.DEV_AUTH ? "diffinery-dev-unsubscribe" : null); | |
| 26 | + const secret = env.MASTER_KEY ?? (devAuth(env) ? "diffinery-dev-unsubscribe" : null); | |
| 26 | 27 | if (!secret) throw new Error("MASTER_KEY is not set, so unsubscribe links can't be signed"); |
| 27 | 28 | return crypto.subtle.importKey("raw", new TextEncoder().encode(`diffinery-unsubscribe-v1:${secret}`), { name: "HMAC", hash: "SHA-256" }, false, ["sign"]); |
| 28 | 29 | } |
modifiedapps/diffinery/src/repo/RepoDO.ts+40−5View file
| @@ -1,5 +1,5 @@ | ||
| 1 | 1 | import { DurableObject } from "cloudflare:workers"; |
| 2 | -import { normalizeCheckSources } from "./required.js"; | |
| 2 | +import { normalizeCheckSources, requiredCheck } from "./required.js"; | |
| 3 | 3 | import { REVIEW_APP, REVIEW_CHECK } from "../agentic/review.js"; |
| 4 | 4 | import { unsafeTreePath } from "../lib/tree-names.js"; |
| 5 | 5 | import { |
| @@ -40,9 +40,10 @@ | ||
| 40 | 40 | type RefEntry, |
| 41 | 41 | type RefResult, |
| 42 | 42 | } from "../git/protocol.js"; |
| 43 | +import { fsckObjects } from "../git/fsck.js"; | |
| 43 | 44 | import { scanForSecrets, secretBypassReason, type SecretHit } from "../git/secret-scan.js"; |
| 44 | 45 | import { HttpError, isValidRefName, now } from "../lib/util.js"; |
| 45 | -import { deletePrefixes, transientPrefixes } from "../lib/storage-keys.js"; | |
| 46 | +import { deletePrefixes, repoPrefixes } from "../lib/storage-keys.js"; | |
| 46 | 47 | import { Actions } from "./actions.js"; |
| 47 | 48 | import { AutoMerge, isPerson } from "./auto-merge.js"; |
| 48 | 49 | import { Collab } from "./collab.js"; |
| @@ -63,6 +64,8 @@ | ||
| 63 | 64 | import { atLeast, globToRegExp, type Caller, type ProtectionRules, type RepoMeta } from "./types.js"; |
| 64 | 65 | |
| 65 | 66 | const MAX_PUSH_BYTES = 100 * 1024 * 1024; |
| 67 | +/** Storage key that marks a deleted repository whose R2 objects are still being deleted. */ | |
| 68 | +const PURGE_KEY = "purge"; | |
| 66 | 69 | |
| 67 | 70 | export type TreeItem = { |
| 68 | 71 | name: string; |
| @@ -170,15 +173,31 @@ | ||
| 170 | 173 | await this.queue.repositoryArchived(); |
| 171 | 174 | } |
| 172 | 175 | |
| 173 | - /** Wipe all storage (repository deletion). */ | |
| 176 | + /** | |
| 177 | + * Wipe all storage (repository deletion), then its R2 objects: git objects, LFS, release | |
| 178 | + * assets, caches, artifacts, logs and archives. One call can't delete more than about | |
| 179 | + * 450,000 objects, so the alarm keeps going until they are all gone. | |
| 180 | + */ | |
| 174 | 181 | async destroy(): Promise<void> { |
| 175 | - // Caches, artifacts, logs and archives live outside repos/<id>/; with the rows gone, nothing would remove them. | |
| 176 | 182 | const repoId = this.readMeta()?.repoId; |
| 177 | - if (repoId) await deletePrefixes(this.env.BUCKET, transientPrefixes(repoId)).catch((e) => console.error("could not delete the repository's Actions objects", e)); | |
| 183 | + await this.ctx.storage.deleteAlarm(); | |
| 178 | 184 | await this.ctx.storage.deleteAll(); |
| 179 | 185 | this.metaCache = null; |
| 186 | + if (!repoId) return; | |
| 187 | + await this.ctx.storage.put(PURGE_KEY, repoId); | |
| 188 | + await this.purge(repoId); | |
| 180 | 189 | } |
| 181 | 190 | |
| 191 | + /** Delete a deleted repository's R2 objects; returns false while some remain (the alarm retries). */ | |
| 192 | + private async purge(repoId: string): Promise<void> { | |
| 193 | + const done = await deletePrefixes(this.env.BUCKET, repoPrefixes(repoId)).catch((e) => { | |
| 194 | + console.error("could not delete the repository's objects", repoId, e); | |
| 195 | + return false; | |
| 196 | + }); | |
| 197 | + if (done) await this.ctx.storage.deleteAll(); | |
| 198 | + else await this.ctx.storage.setAlarm(Date.now() + 5_000); | |
| 199 | + } | |
| 200 | + | |
| 182 | 201 | async stats(): Promise<{ objects: number; bytes: number; refs: number; openIssues: number; openPulls: number }> { |
| 183 | 202 | const q = (s: string) => this.sql.exec<{ n: number }>(s).one().n; |
| 184 | 203 | return { |
| @@ -201,6 +220,8 @@ | ||
| 201 | 220 | } |
| 202 | 221 | |
| 203 | 222 | override async alarm(): Promise<void> { |
| 223 | + const purging = await this.ctx.storage.get<string>(PURGE_KEY); | |
| 224 | + if (purging) return this.purge(purging); | |
| 204 | 225 | await this.queue.process(); |
| 205 | 226 | } |
| 206 | 227 | |
| @@ -325,6 +346,12 @@ | ||
| 325 | 346 | for (const c of req.commands) results.push({ ref: c.ref, ok: false, reason: "unpacker error" }); |
| 326 | 347 | return reportStatus(req, unpackError, results); |
| 327 | 348 | } |
| 349 | + // Like GitHub's receive.fsckObjects: no tree a git client would refuse, or that writes outside the work tree. | |
| 350 | + const bad = fsckObjects(objects); | |
| 351 | + if (bad) { | |
| 352 | + for (const c of req.commands) results.push({ ref: c.ref, ok: false, reason: "fsck failed" }); | |
| 353 | + return reportStatus(req, `fsck error: ${bad}`, results, [`Push refused: ${bad}`]); | |
| 354 | + } | |
| 328 | 355 | |
| 329 | 356 | const marked: SecretHit[] = []; |
| 330 | 357 | const leaks = scanForSecrets(newBlobs, marked); |
| @@ -437,6 +464,14 @@ | ||
| 437 | 464 | if (bypass) return { ref, ok: true }; |
| 438 | 465 | if (deleting) return rules.allowDeletion ? { ref, ok: true } : { ref, ok: false, reason: "protected branch: deletion not allowed" }; |
| 439 | 466 | if (rules.requirePullRequest) return { ref, ok: false, reason: "protected branch: changes must be made through a pull request" }; |
| 467 | + // As on GitHub, required checks gate direct pushes too: the commit must already have passed them. | |
| 468 | + for (const name of rules.requiredChecks ?? []) { | |
| 469 | + const c = requiredCheck(name, [...this.collab.checks(newOid), ...this.statuses.asChecks(newOid)], rules.requiredCheckSources?.[name]); | |
| 470 | + if (c.state !== "success") { | |
| 471 | + const why = c.state === "missing" ? "has not run" : c.state === "pending" ? "has not finished" : `ended with ${c.conclusion}`; | |
| 472 | + return { ref, ok: false, reason: `protected branch: required check '${name}' ${why} on ${newOid.slice(0, 7)}; push to another branch and open a pull request` }; | |
| 473 | + } | |
| 474 | + } | |
| 440 | 475 | if (oldOid !== ZERO_OID && !rules.allowForcePush && !(await isAncestor(this.store, oldOid, newOid))) { |
| 441 | 476 | return { ref, ok: false, reason: "protected branch: force push not allowed" }; |
| 442 | 477 | } |
modifiedapps/diffinery/src/repo/collab.ts+8−1View file
| @@ -1,6 +1,6 @@ | ||
| 1 | 1 | import { ZERO_OID, isAncestor, mergeBase, mergeTrees, readCommit, serializeCommit, walkCommits } from "@diffinery/git-core"; |
| 2 | 2 | import type { QueueEntry } from "./queue.js"; |
| 3 | -import { HttpError, newId, now, randomToken } from "../lib/util.js"; | |
| 3 | +import { HttpError, checkText, newId, now, randomToken } from "../lib/util.js"; | |
| 4 | 4 | import type { RepoDO } from "./RepoDO.js"; |
| 5 | 5 | import { toInfo } from "./RepoDO.js"; |
| 6 | 6 | import { atLeast, type Caller } from "./types.js"; |
| @@ -354,6 +354,7 @@ | ||
| 354 | 354 | |
| 355 | 355 | createIssue(input: { title: string; body?: string; labels?: string[]; assignees?: string[] }, caller: Caller): Issue { |
| 356 | 356 | if (!input.title?.trim()) throw new HttpError(422, "title is required"); |
| 357 | + checkText("Issue", input); | |
| 357 | 358 | // As on GitHub, labels and assignees from someone without triage access are silently dropped. |
| 358 | 359 | if (!atLeast(caller.role, "triage")) input = { ...input, labels: [], assignees: [] }; |
| 359 | 360 | const n = this.nextNumber(); |
| @@ -379,6 +380,7 @@ | ||
| 379 | 380 | |
| 380 | 381 | update(number: number, patch: { title?: string; body?: string; state?: "open" | "closed"; stateReason?: string; labels?: string[]; assignees?: string[] }, caller: Caller): Issue { |
| 381 | 382 | const r = this.row(number); |
| 383 | + checkText(r.kind === "pull" ? "PullRequest" : "Issue", patch); | |
| 382 | 384 | const isAuthor = r.author_id === caller.id; |
| 383 | 385 | if (!isAuthor && !atLeast(caller.role, "triage")) throw new HttpError(403, "permission denied"); |
| 384 | 386 | // As when creating: authors without triage access can't set labels or assignees, which are silently dropped. |
| @@ -457,6 +459,7 @@ | ||
| 457 | 459 | const r = this.row(number); |
| 458 | 460 | if (r.locked && !atLeast(caller.role, "triage")) throw new HttpError(403, "conversation is locked"); |
| 459 | 461 | if (!body?.trim()) throw new HttpError(422, "body is required"); |
| 462 | + checkText("IssueComment", { body }); | |
| 460 | 463 | const id = newId(); |
| 461 | 464 | const t = now(); |
| 462 | 465 | this.sql.exec("INSERT INTO comments (id, number, author_id, author_login, author_kind, body, created_at, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?)", id, number, caller.id, caller.login, caller.kind, body, t, t); |
| @@ -468,6 +471,7 @@ | ||
| 468 | 471 | } |
| 469 | 472 | |
| 470 | 473 | editComment(id: string, body: string, caller: Caller): void { |
| 474 | + checkText("IssueComment", { body }); | |
| 471 | 475 | id = this.repo.social.commentKey(id); |
| 472 | 476 | const c = this.sql.exec<{ author_id: string }>("SELECT author_id FROM comments WHERE id = ?", id).toArray()[0]; |
| 473 | 477 | if (!c) throw new HttpError(404, "comment not found"); |
| @@ -488,6 +492,7 @@ | ||
| 488 | 492 | |
| 489 | 493 | async createPull(input: { title: string; body?: string; head: string; base?: string; draft?: boolean; intent?: string; reviewers?: string[] }, caller: Caller): Promise<Issue> { |
| 490 | 494 | this.repo.policy.assertMayCreateOrApprove(caller, "create"); |
| 495 | + checkText("PullRequest", input); | |
| 491 | 496 | const base = input.base ?? this.repo.meta.defaultBranch; |
| 492 | 497 | if (input.head === base) throw new HttpError(422, "head and base must be different branches"); |
| 493 | 498 | const headSha = this.repo.getRef(`refs/heads/${input.head}`); |
| @@ -624,6 +629,8 @@ | ||
| 624 | 629 | input: { event: "APPROVE" | "REQUEST_CHANGES" | "COMMENT"; body?: string; comments?: Array<{ path: string; line: number; side?: "LEFT" | "RIGHT"; body: string; inReplyTo?: string }> }, |
| 625 | 630 | caller: Caller, |
| 626 | 631 | ): Review { |
| 632 | + checkText("PullRequestReview", input); | |
| 633 | + for (const c of input.comments ?? []) checkText("PullRequestReviewComment", c); | |
| 627 | 634 | const r = this.row(number); |
| 628 | 635 | const p = this.pullRow(number); |
| 629 | 636 | if (r.state !== "open") throw new HttpError(422, "pull request is closed"); |
modifiedapps/diffinery/src/repo/social.ts+2−1View file
| @@ -1,5 +1,5 @@ | ||
| 1 | 1 | import { isAncestor, mergeBase, mergeTrees, readCommit, serializeCommit } from "@diffinery/git-core"; |
| 2 | -import { HttpError, now } from "../lib/util.js"; | |
| 2 | +import { HttpError, checkText, now } from "../lib/util.js"; | |
| 3 | 3 | import type { Comment, IssueRow, Review } from "./collab.js"; |
| 4 | 4 | import type { RepoDO } from "./RepoDO.js"; |
| 5 | 5 | import { atLeast, type Caller } from "./types.js"; |
| @@ -317,6 +317,7 @@ | ||
| 317 | 317 | /** A single review comment, or a reply to one (`in_reply_to`), as a one-comment COMMENTED review. */ |
| 318 | 318 | createReviewComment(number: number, input: { body?: string; commit_id?: string; path?: string; line?: number; position?: number; side?: "LEFT" | "RIGHT"; in_reply_to?: string }, caller: Caller): Comment { |
| 319 | 319 | if (!input.body?.trim()) throw new HttpError(422, "body is required", { errors: [{ resource: "PullRequestReviewComment", code: "missing_field", field: "body" }] }); |
| 320 | + checkText("PullRequestReviewComment", input); | |
| 320 | 321 | let path = input.path; |
| 321 | 322 | let line = input.line ?? input.position; |
| 322 | 323 | let side = input.side; |
modifiedapps/diffinery/src/web/account.tsx+2−3View file
| @@ -12,7 +12,7 @@ | ||
| 12 | 12 | import { csrfToken } from "./csrf.js"; |
| 13 | 13 | import { Auth } from "../auth/sessions.js"; |
| 14 | 14 | import { repoCreated } from "../mail/notify.js"; |
| 15 | -import { browserAccount, deviceLabel, invitePasskeyPage } from "./auth.js"; | |
| 15 | +import { browserAccount, deviceLabel, invitePasskeyPage, localPath } from "./auth.js"; | |
| 16 | 16 | import { deleteCookie, setCookie } from "hono/cookie"; |
| 17 | 17 | import { APPEARANCE_COOKIE, Actor, Avatar, CopyButton, Form, ROLES, Time, cap, LOGIN_PATTERN, PendingInvites, act, ago, flash, post, publicUrl, render, repoUrl, type Stub, type WebApp, prm } from "./ui.js"; |
| 18 | 18 | import type { FormData } from "./csrf.js"; |
| @@ -500,8 +500,7 @@ | ||
| 500 | 500 | const base = publicUrl(c); |
| 501 | 501 | let to = n.url; |
| 502 | 502 | if (to.startsWith(base)) to = to.slice(base.length) || "/"; |
| 503 | - if (!to.startsWith("/") || to.startsWith("//")) to = "/notifications"; | |
| 504 | - return c.redirect(to, 302); | |
| 503 | + return c.redirect(localPath(to) ?? "/notifications", 302); | |
| 505 | 504 | }); |
| 506 | 505 | |
| 507 | 506 | post(web, "/notifications/read", async (c, f) => { |
modifiedapps/diffinery/src/web/auth.tsx+10−1View file
| @@ -31,9 +31,18 @@ | ||
| 31 | 31 | |
| 32 | 32 | const CEREMONY_TIMEOUT = 120_000; |
| 33 | 33 | |
| 34 | +/** | |
| 35 | + * `path` when it is a path on this site, otherwise null. Browsers drop tabs and newlines | |
| 36 | + * from URLs and read a backslash as `/`, so `/<tab>/evil.com` and `/\evil.com` lead off-site. | |
| 37 | + */ | |
| 38 | +export function localPath(path: unknown): string | null { | |
| 39 | + if (typeof path !== "string" || /[\x00-\x1f\x7f]/.test(path)) return null; | |
| 40 | + return /^\/(?![/\\])/.test(path) ? path : null; | |
| 41 | +} | |
| 42 | + | |
| 34 | 43 | /** Only same-site relative paths are allowed as post-sign-in destinations. */ |
| 35 | 44 | export function safeNext(next: unknown): string { |
| 36 | - return typeof next === "string" && next.startsWith("/") && !next.startsWith("//") && !next.startsWith("/\\") ? next : "/"; | |
| 45 | + return localPath(next) ?? "/"; | |
| 37 | 46 | } |
| 38 | 47 | |
| 39 | 48 | const str = (v: unknown): string => (typeof v === "string" ? v : ""); |
modifiedapps/diffinery/src/web/markdown.ts+74−17View file
| @@ -30,6 +30,16 @@ | ||
| 30 | 30 | const MAX_DEPTH = 12; |
| 31 | 31 | const MAX_INLINE = 60_000; |
| 32 | 32 | const MAX_SCAN = 2000; |
| 33 | +/** | |
| 34 | + * Characters that link, code and emphasis lookahead may scan in one render: 4 per source | |
| 35 | + * character, where real documents scan well under 1. Each unmatched `[`, `*` or `_` scans up | |
| 36 | + * to MAX_SCAN ahead, so a crafted paragraph would cost MAX_SCAN times its length; past the | |
| 37 | + * budget the rest of the inline text is shown escaped, without formatting. | |
| 38 | + */ | |
| 39 | +const scanBudget = (sourceLength: number): number => 250_000 + 4 * sourceLength; | |
| 40 | +// Rendering is synchronous, so one counter per isolate is safe. | |
| 41 | +let scanned = 0; | |
| 42 | +let budget = 0; | |
| 33 | 43 | |
| 34 | 44 | const ESC: Record<string, string> = { "&": "&", "<": "<", ">": ">", '"': """, "'": "'" }; |
| 35 | 45 | export const escapeHtml = (s: string): string => s.replace(/[&<>"']/g, (c) => ESC[c]!); |
| @@ -39,7 +49,7 @@ | ||
| 39 | 49 | export function slugify(text: string): string { |
| 40 | 50 | return text |
| 41 | 51 | .toLowerCase() |
| 42 | - .replace(/<[^>]*>/g, "") | |
| 52 | + .replace(/<[^<>]*>/g, "") | |
| 43 | 53 | .replace(/[^\p{L}\p{N}\s_-]/gu, "") |
| 44 | 54 | .trim() |
| 45 | 55 | .replace(/\s+/g, "-") |
| @@ -69,7 +79,7 @@ | ||
| 69 | 79 | // Browsers ignore control characters and whitespace when parsing schemes. |
| 70 | 80 | const probe = u.replace(/[\x00-\x20\x7f]/g, ""); |
| 71 | 81 | if (/^https?:\/\//i.test(probe)) return u; |
| 72 | - if (/^[a-z][a-z0-9+.-]*:/i.test(probe) || probe.startsWith("//") || probe.startsWith("\\\\") || probe.startsWith("/\\")) return null; | |
| 82 | + if (/^[a-z][a-z0-9+.-]*:/i.test(probe) || /^[/\\]{2}/.test(probe)) return null; | |
| 73 | 83 | if (u.startsWith("#")) return kind === "link" ? `#${opts.idPrefix ?? "h-"}${slugify(decodeSafe(u.slice(1)))}` : null; |
| 74 | 84 | const base = opts.base; |
| 75 | 85 | if (!base) return u.startsWith("/") ? u : null; |
| @@ -98,9 +108,11 @@ | ||
| 98 | 108 | function findClosing(src: string, delim: string, from: number, wordy: boolean): number { |
| 99 | 109 | let at = from; |
| 100 | 110 | const limit = Math.min(src.length, from + MAX_SCAN * 2); |
| 111 | + const window = src.slice(0, limit); | |
| 101 | 112 | for (;;) { |
| 102 | - const idx = src.indexOf(delim, at); | |
| 103 | - if (idx < 0 || idx >= limit) return -1; | |
| 113 | + const idx = window.indexOf(delim, at); | |
| 114 | + scanned += (idx < 0 ? limit : idx) - at + 1; | |
| 115 | + if (idx < 0) return -1; | |
| 104 | 116 | const before = src[idx - 1]; |
| 105 | 117 | const after = src[idx + delim.length]; |
| 106 | 118 | if (idx > from && before !== undefined && !/\s/.test(before) && (!wordy || after === undefined || !/[\p{L}\p{N}]/u.test(after))) { |
| @@ -144,6 +156,7 @@ | ||
| 144 | 156 | if (depth === 0) break; |
| 145 | 157 | } |
| 146 | 158 | } |
| 159 | + scanned += j - i; | |
| 147 | 160 | if (depth !== 0 || src[j] !== "]" || src[j + 1] !== "(") return null; |
| 148 | 161 | const text = src.slice(i + 1, j); |
| 149 | 162 | let k = j + 2; |
| @@ -151,6 +164,7 @@ | ||
| 151 | 164 | let url = ""; |
| 152 | 165 | if (src[k] === "<") { |
| 153 | 166 | const close = src.indexOf(">", k); |
| 167 | + scanned += (close < 0 ? src.length : close) - k; | |
| 154 | 168 | if (close < 0 || close - k > MAX_SCAN) return null; |
| 155 | 169 | url = src.slice(k + 1, close); |
| 156 | 170 | k = close + 1; |
| @@ -170,6 +184,7 @@ | ||
| 170 | 184 | parens--; |
| 171 | 185 | } |
| 172 | 186 | } |
| 187 | + scanned += k - start; | |
| 173 | 188 | url = src.slice(start, k).replace(/\\([()])/g, "$1"); |
| 174 | 189 | } |
| 175 | 190 | while (src[k] === " ") k++; |
| @@ -178,6 +193,7 @@ | ||
| 178 | 193 | if (q === '"' || q === "'" || q === "(") { |
| 179 | 194 | const closeCh = q === "(" ? ")" : q; |
| 180 | 195 | const close = src.indexOf(closeCh, k + 1); |
| 196 | + scanned += (close < 0 ? src.length : close) - k; | |
| 181 | 197 | if (close < 0) return null; |
| 182 | 198 | title = src.slice(k + 1, close); |
| 183 | 199 | k = close + 1; |
| @@ -195,6 +211,7 @@ | ||
| 195 | 211 | let i = 0; |
| 196 | 212 | const n = src.length; |
| 197 | 213 | while (i < n) { |
| 214 | + if (scanned > budget) return out + escapeHtml(src.slice(i)); | |
| 198 | 215 | const ch = src[i]!; |
| 199 | 216 | // Backslash escapes and hard line breaks. |
| 200 | 217 | if (ch === "\\") { |
| @@ -210,10 +227,18 @@ | ||
| 210 | 227 | continue; |
| 211 | 228 | } |
| 212 | 229 | } |
| 213 | - if (ch === "\n") { | |
| 214 | - if (out.endsWith(" ")) out = `${out.trimEnd()}<br>`; | |
| 215 | - out += "\n"; | |
| 216 | - i++; | |
| 230 | + // Two or more spaces before a newline are a hard line break. Decided from the source: | |
| 231 | + // looking at the end of `out` would flatten the whole string on every line. | |
| 232 | + if (ch === " ") { | |
| 233 | + let run = 1; | |
| 234 | + while (src[i + run] === " ") run++; | |
| 235 | + if (run >= 2 && src[i + run] === "\n") { | |
| 236 | + out += "<br>\n"; | |
| 237 | + i += run + 1; | |
| 238 | + continue; | |
| 239 | + } | |
| 240 | + out += " ".repeat(run); | |
| 241 | + i += run; | |
| 217 | 242 | continue; |
| 218 | 243 | } |
| 219 | 244 | // Code spans. |
| @@ -227,6 +252,7 @@ | ||
| 227 | 252 | while (src[skip] === "`") skip++; |
| 228 | 253 | close = src.indexOf(fence, skip); |
| 229 | 254 | } |
| 255 | + scanned += (close < 0 ? n : close) - i; | |
| 230 | 256 | if (close >= 0) { |
| 231 | 257 | let code = src.slice(i + run, close).replace(/\n/g, " "); |
| 232 | 258 | if (code.length > 2 && code.startsWith(" ") && code.endsWith(" ") && code.trim()) code = code.slice(1, -1); |
| @@ -330,13 +356,42 @@ | ||
| 330 | 356 | |
| 331 | 357 | // ---------------------------------------------------------------- blocks |
| 332 | 358 | |
| 333 | -const FENCE = /^ {0,3}(`{3,}|~{3,})\s*([^`\s]*)[^`]*$/; | |
| 334 | -const HEADING = /^ {0,3}(#{1,6})(?:[ \t]+(.*?))?(?:[ \t]+#+)?[ \t]*$/; | |
| 359 | +// Block patterns run on every line, so none may backtrack: no two adjacent quantifiers that | |
| 360 | +// can match the same characters, and `[^]` rather than `.`, which stops at U+2028 and fails | |
| 361 | +// the `$` after it. Trailing spaces and closing `#`s are trimmed by hand (see headingText). | |
| 362 | +const FENCE = /^ {0,3}(`{3,}(?!`)|~{3,}(?!~))(?![^`]*`)\s*([^`\s]*)/; | |
| 363 | +const HEADING = /^ {0,3}(#{1,6})(?:[ \t]([^]*))?$/; | |
| 335 | 364 | const HR = /^ {0,3}([-*_])(?:[ \t]*\1){2,}[ \t]*$/; |
| 336 | -const QUOTE = /^ {0,3}> ?(.*)$/; | |
| 337 | -const LIST_ITEM = /^( {0,12})([*+-]|\d{1,9}[.)])([ \t]+(.*))?$/; | |
| 338 | -const TABLE_SEP = /^ {0,3}\|?[ \t]*:?-+:?[ \t]*(\|[ \t]*:?-+:?[ \t]*)*\|?[ \t]*$/; | |
| 365 | +const QUOTE = /^ {0,3}> ?([^]*)$/; | |
| 366 | +const LIST_ITEM = /^( {0,12})([*+-]|\d{1,9}[.)])([ \t]+([^]*))?$/; | |
| 367 | +const SEP_CELL = /^:?-+:?$/; | |
| 339 | 368 | |
| 369 | +function trimBlank(s: string): string { | |
| 370 | + let a = 0; | |
| 371 | + let b = s.length; | |
| 372 | + while (a < b && (s[a] === " " || s[a] === "\t")) a++; | |
| 373 | + while (b > a && (s[b - 1] === " " || s[b - 1] === "\t")) b--; | |
| 374 | + return s.slice(a, b); | |
| 375 | +} | |
| 376 | + | |
| 377 | +/** A heading's text without its surrounding spaces and optional closing `#`s. */ | |
| 378 | +function headingText(raw: string): string { | |
| 379 | + const t = trimBlank(raw); | |
| 380 | + let j = t.length; | |
| 381 | + while (j > 0 && t[j - 1] === "#") j--; | |
| 382 | + if (j === t.length) return t; | |
| 383 | + if (j === 0) return ""; | |
| 384 | + return t[j - 1] === " " || t[j - 1] === "\t" ? trimBlank(t.slice(0, j)) : t; | |
| 385 | +} | |
| 386 | + | |
| 387 | +/** A table's delimiter row, like `| :-- | --: |`, checked cell by cell. */ | |
| 388 | +function tableSep(line: string): boolean { | |
| 389 | + let s = trimBlank(line); | |
| 390 | + if (s.startsWith("|")) s = s.slice(1); | |
| 391 | + if (s.endsWith("|")) s = s.slice(0, -1); | |
| 392 | + return s.split("|").every((cell) => SEP_CELL.test(trimBlank(cell))); | |
| 393 | +} | |
| 394 | + | |
| 340 | 395 | function splitRow(line: string): string[] { |
| 341 | 396 | let s = line.trim(); |
| 342 | 397 | if (s.startsWith("|")) s = s.slice(1); |
| @@ -370,7 +425,7 @@ | ||
| 370 | 425 | HR.test(line) || |
| 371 | 426 | QUOTE.test(line) || |
| 372 | 427 | /^ {0,3}([*+-]|1[.)])[ \t]+\S/.test(line) || |
| 373 | - (line.includes("|") && next !== undefined && TABLE_SEP.test(next) && next.includes("-")) | |
| 428 | + (line.includes("|") && next !== undefined && tableSep(next)) | |
| 374 | 429 | ); |
| 375 | 430 | } |
| 376 | 431 | |
| @@ -417,7 +472,7 @@ | ||
| 417 | 472 | const h = HEADING.exec(line); |
| 418 | 473 | if (h) { |
| 419 | 474 | const level = h[1]!.length; |
| 420 | - const text = h[2] ?? ""; | |
| 475 | + const text = headingText(h[2] ?? ""); | |
| 421 | 476 | const html = inline(text, opts, depth); |
| 422 | 477 | const id = escapeHtml(`${opts.idPrefix ?? "h-"}${slugify(text)}`); |
| 423 | 478 | parts.push(`<h${level} id="${id}"><a class="anchor" href="#${id}" aria-hidden="true">#</a>${html}</h${level}>`); |
| @@ -442,7 +497,7 @@ | ||
| 442 | 497 | continue; |
| 443 | 498 | } |
| 444 | 499 | // Tables. |
| 445 | - if (line.includes("|") && i + 1 < lines.length && TABLE_SEP.test(lines[i + 1]!) && lines[i + 1]!.includes("-")) { | |
| 500 | + if (line.includes("|") && i + 1 < lines.length && tableSep(lines[i + 1]!)) { | |
| 446 | 501 | const head = splitRow(line); |
| 447 | 502 | const aligns = splitRow(lines[i + 1]!).map((c) => (c.startsWith(":") && c.endsWith(":") ? "center" : c.endsWith(":") ? "right" : c.startsWith(":") ? "left" : "")); |
| 448 | 503 | i += 2; |
| @@ -535,7 +590,7 @@ | ||
| 535 | 590 | i++; |
| 536 | 591 | } |
| 537 | 592 | if (para.length) { |
| 538 | - const html = inline(para.join("\n").replace(/[ \t]+$/, ""), opts, depth); | |
| 593 | + const html = inline(trimBlank(para.join("\n")), opts, depth); | |
| 539 | 594 | parts.push(tight ? html : `<p>${html}</p>`); |
| 540 | 595 | } |
| 541 | 596 | } |
| @@ -557,5 +612,7 @@ | ||
| 557 | 612 | export function renderMarkdown(source: string, opts: MarkdownOptions = {}): string { |
| 558 | 613 | if (source.length > MAX_SOURCE) return `<pre>${escapeHtml(source)}</pre>`; |
| 559 | 614 | const lines = source.replace(/\r\n?/g, "\n").split("\n"); |
| 615 | + scanned = 0; | |
| 616 | + budget = scanBudget(source.length); | |
| 560 | 617 | return blocks(lines, opts, 0); |
| 561 | 618 | } |
modifiedapps/diffinery/test/e2e/security.test.mjs+99−0View file
| @@ -1551,3 +1551,102 @@ | ||
| 1551 | 1551 | const entry = await until("the opt-out in the audit log", async () => auditLog("api.secret_marked").find((e) => e.target === `${OWNER}/messages`)); |
| 1552 | 1552 | assert.deepEqual(JSON.parse(entry.detail).paths, ["fixture.env"]); |
| 1553 | 1553 | }); |
| 1554 | + | |
| 1555 | +test("security: required checks gate direct pushes to a protected branch, as on GitHub", async () => { | |
| 1556 | + const R = `/repos/${OWNER}/gated`; | |
| 1557 | + const mainSha = await repoWithReadme("gated"); | |
| 1558 | + await status(204, "PUT", `${R}/collaborators/rita`, { permission: "write" }); | |
| 1559 | + await ok("PUT", `${R}/branches/main/protection`, { requiredChecks: ["ci"] }); | |
| 1560 | + // A writer's commit straight to main hasn't been checked: refused, whichever way it comes. | |
| 1561 | + const refused = await status(403, "PUT", `${R}/contents/direct.txt`, { message: "direct", content: b64("x\n"), branch: "main" }, T.rita); | |
| 1562 | + assert.match(refused.message, /required check 'ci' has not run/); | |
| 1563 | + // The same commit, tested on a branch first, can move main. | |
| 1564 | + await ok("POST", `${R}/git/refs`, { ref: "refs/heads/tested", sha: mainSha }, T.rita); | |
| 1565 | + const tested = (await ok("PUT", `${R}/contents/direct.txt`, { message: "tested", content: b64("x\n"), branch: "tested" }, T.rita)).commit.sha; | |
| 1566 | + assert.match((await status(403, "PATCH", `${R}/git/refs/heads/main`, { sha: tested }, T.rita)).message, /has not run/); | |
| 1567 | + await ok("POST", `${R}/check-runs`, { name: "ci", head_sha: tested, status: "in_progress" }); | |
| 1568 | + assert.match((await status(403, "PATCH", `${R}/git/refs/heads/main`, { sha: tested }, T.rita)).message, /has not finished/); | |
| 1569 | + await ok("POST", `${R}/check-runs`, { name: "ci", head_sha: tested, status: "completed", conclusion: "success" }); | |
| 1570 | + // (Two runs named ci: the in-progress one still counts until it completes.) | |
| 1571 | + const runs = (await ok("GET", `${R}/commits/${tested}/check-runs`)).check_runs.filter((r) => r.status !== "completed"); | |
| 1572 | + for (const r of runs) await ok("PATCH", `${R}/check-runs/${r.id}`, { status: "completed", conclusion: "success" }); | |
| 1573 | + await ok("PATCH", `${R}/git/refs/heads/main`, { sha: tested }, T.rita); | |
| 1574 | + assert.equal((await ok("GET", `${R}/branches/main`)).commit.sha, tested); | |
| 1575 | + // A repository admin isn't held to it unless the rule says so. | |
| 1576 | + await ok("PUT", `${R}/contents/admin.txt`, { message: "admin", content: b64("y\n"), branch: "main" }); | |
| 1577 | +}); | |
| 1578 | + | |
| 1579 | +test("security: issue, pull request and comment text has GitHub's length limits", async () => { | |
| 1580 | + await ok("POST", "/user/repos", { name: "long-text" }); | |
| 1581 | + const R = `/repos/${OWNER}/long-text`; | |
| 1582 | + const long = "x".repeat(65_537); | |
| 1583 | + const tooLong = (r) => assert.match(r.message, /too long \(maximum is (65536|256) characters\)/); | |
| 1584 | + tooLong(await status(422, "POST", `${R}/issues`, { title: "Big", body: long })); | |
| 1585 | + tooLong(await status(422, "POST", `${R}/issues`, { title: "t".repeat(257) })); | |
| 1586 | + const issue = await status(201, "POST", `${R}/issues`, { title: "Small", body: "x".repeat(65_536) }); | |
| 1587 | + tooLong(await status(422, "PATCH", `${R}/issues/${issue.number}`, { body: long })); | |
| 1588 | + tooLong(await status(422, "POST", `${R}/issues/${issue.number}/comments`, { body: long })); | |
| 1589 | + const comment = await status(201, "POST", `${R}/issues/${issue.number}/comments`, { body: "fine" }); | |
| 1590 | + tooLong(await status(422, "PATCH", `${R}/issues/comments/${comment.id}`, { body: long })); | |
| 1591 | + assert.equal((await ok("GET", `${R}/issues/${issue.number}`)).body.length, 65_536); | |
| 1592 | +}); | |
| 1593 | + | |
| 1594 | +test("security: deleting a repository deletes its stored objects too", async () => { | |
| 1595 | + await repoWithReadme("doomed"); | |
| 1596 | + // Objects over 512 KiB compressed live in R2, under repos/<id>/. | |
| 1597 | + const big = randomBytes(700 * 1024); | |
| 1598 | + await ok("PUT", `/repos/${OWNER}/doomed/contents/big.bin`, { message: "big", content: big.toString("base64") }); | |
| 1599 | + const oid = createHash("sha1").update(Buffer.concat([Buffer.from(`blob ${big.length}\0`), big])).digest("hex"); | |
| 1600 | + const persist = join(work, "state"); | |
| 1601 | + const run = (...args) => execFileSync("pnpm", ["exec", "wrangler", ...args], { cwd: APP, stdio: "pipe", env: { ...process.env, CI: "1" } }).toString(); | |
| 1602 | + const rows = JSON.parse(run("d1", "execute", "diffinery-dev", "--local", "-e", "dev", "--persist-to", persist, "--json", "--command", "SELECT id FROM repos WHERE name = 'doomed'")); | |
| 1603 | + const repoId = rows[0].results[0].id; | |
| 1604 | + const key = `diffinery-data-dev/repos/${repoId}/objects/${oid.slice(0, 2)}/${oid.slice(2)}`; | |
| 1605 | + const stored = () => { | |
| 1606 | + try { | |
| 1607 | + run("r2", "object", "get", key, "--local", "-e", "dev", "--persist-to", persist, "--pipe"); | |
| 1608 | + return true; | |
| 1609 | + } catch { | |
| 1610 | + return false; | |
| 1611 | + } | |
| 1612 | + }; | |
| 1613 | + assert.ok(stored(), "the large blob is in R2 before the repository is deleted"); | |
| 1614 | + await status(204, "DELETE", `/repos/${OWNER}/doomed`); | |
| 1615 | + await until("the repository's objects are deleted", () => !stored(), 30_000); | |
| 1616 | +}); | |
| 1617 | + | |
| 1618 | +test("security: pushes are checked like git's fsck: no '..' or '.git' entries, no unsafe .gitmodules", async () => { | |
| 1619 | + const R = `/repos/${OWNER}/fsck`; | |
| 1620 | + const mainSha = await repoWithReadme("fsck"); | |
| 1621 | + const clone = join(work, "fsck"); | |
| 1622 | + const authed = `${base.replace("://", `://x:${T.olive}@`)}/${OWNER}/fsck.git`; | |
| 1623 | + execFileSync("git", ["clone", "-q", authed, clone], { stdio: "pipe" }); | |
| 1624 | + const g = (args, input) => execFileSync("git", ["-c", "user.name=Olive", "-c", "user.email=olive@example.com", ...args], { cwd: clone, stdio: "pipe", input }).toString().trim(); | |
| 1625 | + const push = (sha) => spawnSync("git", ["push", "origin", `${sha}:refs/heads/main`], { cwd: clone, encoding: "utf8" }); | |
| 1626 | + const blob = g(["hash-object", "-w", "--stdin"], "payload\n"); | |
| 1627 | + // A tree git itself would never write: git's own tools refuse these names, so build it by hand. | |
| 1628 | + const crafted = (name) => { | |
| 1629 | + const bytes = Buffer.concat([Buffer.from(`100644 ${name}\0`), Buffer.from(blob, "hex")]); | |
| 1630 | + const tree = g(["hash-object", "-t", "tree", "-w", "--literally", "--stdin"], bytes); | |
| 1631 | + return g(["commit-tree", tree, "-p", mainSha, "-m", `entry ${name}`]); | |
| 1632 | + }; | |
| 1633 | + for (const [name, why] of [["..", /not a valid name/], [".git", /reserved/], [".GIT", /reserved/], ["git~1", /reserved/]]) { | |
| 1634 | + const r = push(crafted(name)); | |
| 1635 | + assert.notEqual(r.status, 0, `a tree entry named ${name} was accepted`); | |
| 1636 | + assert.match(r.stderr, why); | |
| 1637 | + } | |
| 1638 | + // .gitmodules that would make git run an option (CVE-2018-17456). | |
| 1639 | + writeFileSync(join(clone, ".gitmodules"), '[submodule "x"]\n\tpath = x\n\turl = --upload-pack=touch /tmp/pwned\n'); | |
| 1640 | + g(["add", ".gitmodules"]); | |
| 1641 | + g(["commit", "-q", "-m", "submodule"]); | |
| 1642 | + const r = push("HEAD"); | |
| 1643 | + assert.notEqual(r.status, 0); | |
| 1644 | + assert.match(r.stderr, /url that starts with '-'/); | |
| 1645 | + assert.equal((await ok("GET", `${R}/branches/main`)).commit.sha, mainSha); | |
| 1646 | + // An ordinary .gitmodules goes through. | |
| 1647 | + g(["reset", "-q", "--hard", mainSha]); | |
| 1648 | + writeFileSync(join(clone, ".gitmodules"), '[submodule "x"]\n\tpath = x\n\turl = https://example.com/x.git\n'); | |
| 1649 | + g(["add", ".gitmodules"]); | |
| 1650 | + g(["commit", "-q", "-m", "submodule"]); | |
| 1651 | + assert.equal(push("HEAD").status, 0); | |
| 1652 | +}); |
addedapps/diffinery/test/unit/dev-auth.test.ts+26−0View file
| @@ -1,0 +1,26 @@ | ||
| 1 | +import { describe, expect, it } from "vitest"; | |
| 2 | +import { devAuth } from "../../src/auth/dev.js"; | |
| 3 | + | |
| 4 | +const dev = { DEV_AUTH: "dev@diffinery.local", ACCESS_AUD: "", PUBLIC_URL: "http://localhost:8787" }; | |
| 5 | + | |
| 6 | +describe("the development sign-in", () => { | |
| 7 | + it("works only on this machine", () => { | |
| 8 | + expect(devAuth(dev, "http://localhost:8787/api/v3/user")).toBe(true); | |
| 9 | + expect(devAuth(dev, "http://127.0.0.1:41234/")).toBe(true); | |
| 10 | + expect(devAuth(dev, "http://[::1]:8787/")).toBe(true); | |
| 11 | + expect(devAuth(dev)).toBe(true); | |
| 12 | + }); | |
| 13 | + | |
| 14 | + it("does nothing when a deployed configuration keeps DEV_AUTH", () => { | |
| 15 | + // Reached under a public name, whatever PUBLIC_URL says. | |
| 16 | + expect(devAuth(dev, "https://diffinery-dev.example.workers.dev/")).toBe(false); | |
| 17 | + expect(devAuth(dev, "https://localhost.evil.example/")).toBe(false); | |
| 18 | + // A public PUBLIC_URL turns it off everywhere, request or not. | |
| 19 | + const deployed = { ...dev, PUBLIC_URL: "https://diffinery.com" }; | |
| 20 | + expect(devAuth(deployed, "http://localhost:8787/")).toBe(false); | |
| 21 | + expect(devAuth(deployed)).toBe(false); | |
| 22 | + // So does Cloudflare Access, or no DEV_AUTH at all. | |
| 23 | + expect(devAuth({ ...dev, ACCESS_AUD: "aud" }, "http://localhost:8787/")).toBe(false); | |
| 24 | + expect(devAuth({ ...dev, DEV_AUTH: undefined }, "http://localhost:8787/")).toBe(false); | |
| 25 | + }); | |
| 26 | +}); |
addedapps/diffinery/test/unit/fsck.test.ts+59−0View file
| @@ -1,0 +1,59 @@ | ||
| 1 | +import { describe, expect, it } from "vitest"; | |
| 2 | +import { fsckObjects, isDotGitmodulesName, unsafeGitmodules } from "../../src/git/fsck.js"; | |
| 3 | + | |
| 4 | +const enc = new TextEncoder(); | |
| 5 | +const OID = "a".repeat(40); | |
| 6 | + | |
| 7 | +function tree(...entries: Array<[mode: string, name: string, oid?: string]>): { oid: string; type: string; content: Uint8Array } { | |
| 8 | + const parts: number[] = []; | |
| 9 | + for (const [mode, name, oid = OID] of entries) { | |
| 10 | + parts.push(...enc.encode(`${mode} ${name}\0`)); | |
| 11 | + for (let i = 0; i < 40; i += 2) parts.push(parseInt(oid.slice(i, i + 2), 16)); | |
| 12 | + } | |
| 13 | + return { oid: "t".repeat(40), type: "tree", content: new Uint8Array(parts) }; | |
| 14 | +} | |
| 15 | +const blob = (oid: string, text: string) => ({ oid, type: "blob", content: enc.encode(text) }); | |
| 16 | + | |
| 17 | +describe("fsck of pushed objects", () => { | |
| 18 | + it("accepts ordinary trees", () => { | |
| 19 | + expect(fsckObjects([tree(["100644", "README.md"], ["40000", "src"], ["120000", "link"], ["160000", "vendor"])])).toBeNull(); | |
| 20 | + expect(fsckObjects([tree(["100644", ".gitignore"], ["100644", "..."], ["100644", ".github"])])).toBeNull(); | |
| 21 | + }); | |
| 22 | + | |
| 23 | + it.each([ | |
| 24 | + ["..", /not a valid name/], | |
| 25 | + [".", /not a valid name/], | |
| 26 | + ["", /empty name/], | |
| 27 | + ["a/b", /contains '\/'/], | |
| 28 | + [".git", /reserved/], | |
| 29 | + [".GIT", /reserved/], | |
| 30 | + [".git.", /reserved/], | |
| 31 | + ["git~1", /reserved/], | |
| 32 | + [".git", /reserved/], | |
| 33 | + ])("refuses a tree entry named %j", (name, why) => { | |
| 34 | + expect(fsckObjects([tree(["40000", name])])).toMatch(why); | |
| 35 | + }); | |
| 36 | + | |
| 37 | + it("refuses duplicate entries", () => { | |
| 38 | + expect(fsckObjects([tree(["100644", "a"], ["40000", "a"])])).toMatch(/duplicate entry 'a'/); | |
| 39 | + }); | |
| 40 | + | |
| 41 | + it("refuses a symlinked .gitmodules, in any spelling", () => { | |
| 42 | + expect(fsckObjects([tree(["120000", ".gitmodules"])])).toMatch(/symbolic link/); | |
| 43 | + expect(fsckObjects([tree(["120000", "GITMOD~1"])])).toMatch(/symbolic link/); | |
| 44 | + expect(isDotGitmodulesName(".GitModules. ")).toBe(true); | |
| 45 | + }); | |
| 46 | + | |
| 47 | + it("refuses .gitmodules that escape .git/modules or pass options to git", () => { | |
| 48 | + const bad = "b".repeat(40); | |
| 49 | + expect(fsckObjects([tree(["100644", ".gitmodules", bad]), blob(bad, '[submodule "../../hooks"]\n\tpath = x\n\turl = https://example.com/x.git\n')])).toMatch(/can't contain '\.\.'/); | |
| 50 | + expect(unsafeGitmodules('[submodule "x"]\n\turl = --upload-pack=touch /tmp/pwned\n')).toMatch(/url that starts with '-'/); | |
| 51 | + expect(unsafeGitmodules('[submodule "x"]\n\tpath = "-x"\n')).toMatch(/path that starts with '-'/); | |
| 52 | + expect(unsafeGitmodules('[submodule "a\\\\..\\\\b"]\n')).toMatch(/can't contain/); | |
| 53 | + expect(unsafeGitmodules('[submodule "lib/x"]\n\tpath = lib/x\n\turl = ../x.git\n[core]\n\turl = -fine-outside-submodules\n')).toBeNull(); | |
| 54 | + }); | |
| 55 | + | |
| 56 | + it("refuses malformed trees", () => { | |
| 57 | + expect(fsckObjects([{ oid: "t".repeat(40), type: "tree", content: enc.encode("100644 broken") }])).toMatch(/malformed tree/); | |
| 58 | + }); | |
| 59 | +}); |
addedapps/diffinery/test/unit/markdown-safety.test.ts+92−0View file
| @@ -1,0 +1,92 @@ | ||
| 1 | +import { describe, expect, it } from "vitest"; | |
| 2 | +import { renderMarkdown, safeUrl } from "../../src/web/markdown.js"; | |
| 3 | +import { localPath, safeNext } from "../../src/web/auth.js"; | |
| 4 | + | |
| 5 | +const spaces = " ".repeat(50_000); | |
| 6 | + | |
| 7 | +function timed(source: string): { html: string; ms: number } { | |
| 8 | + const t = performance.now(); | |
| 9 | + const html = renderMarkdown(source); | |
| 10 | + return { html, ms: performance.now() - t }; | |
| 11 | +} | |
| 12 | + | |
| 13 | +describe("markdown block patterns", () => { | |
| 14 | + // Each of these took seconds with a single backtracking regex per line. | |
| 15 | + it.each([ | |
| 16 | + ["a fence followed by spaces and a backtick", "```" + spaces + "`"], | |
| 17 | + ["a heading with trailing spaces before a line separator", "# a" + spaces + " "], | |
| 18 | + ["a heading with spaces before text", "# a" + spaces + "b"], | |
| 19 | + ["a list item with spaces before a line separator", "-" + spaces + " "], | |
| 20 | + ["a table delimiter row with spaces before text", "a|b\n|-" + spaces + "x"], | |
| 21 | + ["a dash run with spaces before text", "a|b\n---" + spaces + "x"], | |
| 22 | + ])("renders %s in linear time", (_name, source) => { | |
| 23 | + expect(timed(source).ms).toBeLessThan(500); | |
| 24 | + }); | |
| 25 | + | |
| 26 | + // Unmatched link and emphasis openers each scan ahead; a budget per render bounds the total. | |
| 27 | + it.each([ | |
| 28 | + ["unclosed links", "[a](".repeat(14_000)], | |
| 29 | + ["unclosed images", "], | |
| 30 | + ["emphasis openers that never close", "_1.".repeat(19_000)], | |
| 31 | + ["list items of emphasis openers", "- " + "*> ".repeat(19_000)], | |
| 32 | + ["many such paragraphs", ("_1.".repeat(19_000) + "\n\n").repeat(8)], | |
| 33 | + ])("bounds the lookahead for %s", (_name, source) => { | |
| 34 | + expect(timed(source).ms).toBeLessThan(500); | |
| 35 | + }); | |
| 36 | + | |
| 37 | + it("formats ordinary text in full", () => { | |
| 38 | + const text = Array.from({ length: 2000 }, (_, i) => `Item ${i} has *emphasis*, \`code\` and a [link](/p/${i}).`).join("\n\n"); | |
| 39 | + const html = renderMarkdown(text); | |
| 40 | + expect(html.match(/<em>/g)?.length).toBe(2000); | |
| 41 | + expect(html.match(/<a href=/g)?.length).toBe(2000); | |
| 42 | + }); | |
| 43 | + | |
| 44 | + it("still parses headings, fences, lists and tables", () => { | |
| 45 | + expect(renderMarkdown("## Title ##")).toContain(">Title</h2>"); | |
| 46 | + expect(renderMarkdown("# foo#")).toContain(">foo#</h1>"); | |
| 47 | + expect(renderMarkdown("#\tTabbed ")).toContain(">Tabbed</h1>"); | |
| 48 | + expect(renderMarkdown("# ###")).toMatch(/<h1 [^>]*><a [^>]*>#<\/a><\/h1>/); | |
| 49 | + expect(renderMarkdown("#nope")).toContain("<p>#nope</p>"); | |
| 50 | + expect(renderMarkdown("```js extra\nx\n```")).toContain('class="language-js'); | |
| 51 | + expect(renderMarkdown("``` a`b\nx")).not.toContain("<pre>"); | |
| 52 | + expect(renderMarkdown("~~~ a`b\nx")).not.toContain("<pre>"); | |
| 53 | + expect(renderMarkdown("- one\n- two")).toContain("<li>one</li>"); | |
| 54 | + const table = renderMarkdown("| a | b |\n| :-- | --: |\n| 1 | 2 |"); | |
| 55 | + expect(table).toContain('<th style="text-align:left">a</th>'); | |
| 56 | + expect(table).toContain('<td style="text-align:right">2</td>'); | |
| 57 | + expect(renderMarkdown("a | b\n-|-")).toContain("<table>"); | |
| 58 | + expect(renderMarkdown("a | b\n-||-")).not.toContain("<table>"); | |
| 59 | + expect(renderMarkdown("a | b\n|")).not.toContain("<table>"); | |
| 60 | + }); | |
| 61 | +}); | |
| 62 | + | |
| 63 | +describe("same-site paths", () => { | |
| 64 | + it("refuses paths that browsers resolve to another site", () => { | |
| 65 | + for (const p of ["//evil.com", "/\\evil.com", "/\t/evil.com", "/\n/evil.com", "/\r\n/evil.com", "https://evil.com", "evil.com", "", 7, null]) { | |
| 66 | + expect(localPath(p)).toBeNull(); | |
| 67 | + expect(safeNext(p)).toBe("/"); | |
| 68 | + } | |
| 69 | + expect(safeNext("/owner/repo/pulls?q=is:open")).toBe("/owner/repo/pulls?q=is:open"); | |
| 70 | + expect(safeNext("/a b")).toBe("/a b"); | |
| 71 | + }); | |
| 72 | + | |
| 73 | + it("refuses Markdown links that start with a slash and a backslash in either order", () => { | |
| 74 | + for (const u of ["\\/evil.com", "/\\evil.com", "\\\\evil.com", "//evil.com", "/\t/evil.com"]) expect(safeUrl(u, "link", {})).toBeNull(); | |
| 75 | + expect(safeUrl("/docs", "link", {})).toBe("/docs"); | |
| 76 | + }); | |
| 77 | +}); | |
| 78 | + | |
| 79 | +describe("markdown over many lines", () => { | |
| 80 | + it.each([ | |
| 81 | + ["short lines", "&\n".repeat(29_000)], | |
| 82 | + ["hard line breaks", "a \n".repeat(14_000)], | |
| 83 | + ["a long tilde run", "~".repeat(59_000) + "`"], | |
| 84 | + ["a long backtick run", "`".repeat(59_000) + "x`"], | |
| 85 | + ])("renders %s in linear time", (_name, source) => { | |
| 86 | + expect(timed(source).ms).toBeLessThan(500); | |
| 87 | + }); | |
| 88 | + | |
| 89 | + it("keeps hard line breaks", () => { | |
| 90 | + expect(renderMarkdown("one \ntwo\nthree \nfour")).toBe("<p>one<br>\ntwo\nthree<br>\nfour</p>"); | |
| 91 | + }); | |
| 92 | +}); |