fix(security): second ReDoS sweep (#174)

.changeset/redos-sweep-174.md

@@ -0,0 +1,15 @@
+---
+"@cloudhal/api": patch
+"@cloudhal/core-ts": patch
+"@cloudhal/schedules": patch
+"@cloudhal/agent-runtime": patch
+"@cloudhal/cli": patch
+---
+
+Second ReDoS sweep (#174). Input that made a regex backtrack is now handled in linear time; results are unchanged.
+
+- **Unauthenticated password-reset request.** `POST /api/password-reset/request` (and invite creation) checks the address length before anything else, and the address check no longer backtracks: `a@` followed by many dots used to cost seconds of CPU per request. Public token endpoints (`/api/password-reset/*`, invite acceptance) refuse a JSON body over 64 KiB with `413 payload_too_large`, whether or not it declares a `content-length`.
+- **core-ts.** Redirect path normalising and matching (on every site request): `normalisePath`, `joinTarget`, and `ancestorPaths`, which no longer lists ancestors that no redirect can match: those longer than a `from` may be (1024 code points) or holding `//`. Directive lines in search text keep their braces on one line, so a directive whose `{…}` spans a lone `\r` is no longer read as one. Search text built at publish time: fenced code, links, images and tags. SVG upload checks: CSS comments, `url(…)` references (about cubic on `url(` followed by spaces) and width/height lengths (new `css-scan.ts`).
+- **schedules.** The feed check's CDATA, element and `<enclosure>`/`<media:content>` url scans, and `urlPath`.
+- **agent-runtime.** SKILL.md frontmatter lines (a line of spaces was quadratic).
+- **Studio** (in `@cloudhal/cli`): the markdown body editor's trailing-newline trims.

AGENTS.md

@@ -56,6 +56,7 @@ pnpm i18n:export / pnpm i18n:import [--locale xx]  # translations as nested ICU
 10. **UI strings** go through `useT()` with keys in `en.ts` (only English is edited by hand; every other `<locale>.ts` is generated by `pnpm i18n:import`, ADR-0034) and every language's `<locale>.ts` (or every file of a lazy namespace, read with `useT("<ns>")`; languages are registered in `apps/studio/src/i18n/locales.ts`); no hard-coded words in `apps/studio/src` or `packages/ui/src` — visible text, `aria-label`/`title`/`placeholder`/`alt`, toasts, banners, label tables and block defaults (blocks take every word as a prop, no English fallback; API errors through `apiErrorText`). Enforced by the `i18n`, `i18n-plural`, `i18n-concat` and `i18n-format` rules (and `rtl-logical` for physical CSS utilities, ADR-0033) of `pnpm design:lint` (`apps/studio/scripts/i18n-lint.ts`, run in `pnpm check` by `test/design-rules.test.ts` and on every agent edit); legitimate literals go in its `ALLOWED` list or carry an `i18n-ignore: <reason>` comment.
 11. Conventional commits, `git commit -s` (DCO). Changeset for published-package changes.
 12. **Size ratchet** ([docs/engineering/code-structure.md](docs/engineering/code-structure.md)): files ≤ 800 lines (soft 400; `.astro` 500, tests 1500; i18n dictionaries are data and exempt — the Studio bundle budget guards them), functions ≤ 150 lines, depth ≤ 5, ≤ 4 positional params (use an options object beyond 3); one responsibility per module, dispatch tables instead of giant switches. `pnpm size:check` (in `pnpm check`) fails on a new violation or when a file in `scripts/size-baseline.json` grows — put new code in a new module next to it; after shrinking one, run `node scripts/size-check.ts --update`.
+13. **Regexes over external input** (request paths and bodies, agent or editor markdown, uploads, fetched feeds) must be linear (#168, #174): no unanchored or end-anchored quantified runs (`/-+$/`, `/\s+$/`, `/\/+$/`), no lazy scans that rescan to the end when the closer is missing (`/<x>[\s\S]*?<\/x>/`, `/<[^>]*>/`), no adjacent quantifiers over overlapping classes (`\s+[^:]+`, `\s*(.*)`). Use `trimChar`/`trimEndChar` (`@cloudhal/core-ts/names`), `trimEnd()`, `indexOf`/`lastIndexOf` scanners, and check lengths before patterns. A rewrite gets an equivalence test against the old regex and a 100k-character adversarial case.
 
 ## Versions (pinned via pnpm catalog / Cargo workspace)
 Node 24 · pnpm 12 · TypeScript 7.0 · Biome 2.6 · Vitest 5 · Turborepo 2.11 · Rust 2024 edition · json-schema-to-typescript 16.0.0 · cargo-typify 0.8.0 · ajv 8.20 · jsonschema 0.60 · better-sqlite3 13 · miniflare 4.20260730 (last stable 4.x; its sharp overridden to 0.35.5; see ADR-0012) · wrangler 4.158 · @astrojs/cloudflare 14.4 · Astro 7.4 · Tailwind 4.3 · marked 18 · Hono 4.13 · Better Auth 1.7.12 · Kumo 2.14 · React 19.3 · TanStack Router 1.170 / Query 5.104 · Vite 8.3.

CONTRIBUTING.md

@@ -40,6 +40,7 @@ Commit the regenerated files; CI fails on drift. Add a valid and an invalid fixt
 - Add a changeset (`pnpm changeset`) when a published package changes behaviour.
 - A PR that changes what the renderer gives themes freezes the last released starter in the same PR: copy it under `packages/cli/test/fixtures/theme-v<last release>/` and add that version to `FROZEN` in `packages/cli/test/old-theme-compat.test.ts` (see `theme-v0.5.0/README.md` there), so the release that ships the change is already tested against the theme existing sites have.
 - Keep modules small and single-purpose: [`docs/engineering/code-structure.md`](docs/engineering/code-structure.md) sets file/function limits that `pnpm size:check` enforces with a shrink-only baseline.
+- Regexes over input from outside (requests, agent or editor content, uploads, feeds) must not backtrack: prefer `indexOf` scanners and the linear trims in `@cloudhal/core-ts/names`, check lengths first, and test a rewrite against the old regex plus a 100k-character adversarial input (AGENTS.md rule 13).
 - CI must be green: lint, size check, typecheck, tests (TS + Rust), contract drift.
 
 ## Releasing

apps/docs/src/worker/paths.ts

@@ -38,5 +38,9 @@ export function pageId(input: string, selfHost?: string): string | undefined {
 
 export const markdownPath = (id: string) => (id === "index" ? "/index.md" : `/${id}.md`);
 
-/** A path that may be a page: no file extension in its last segment. */
-export const isPagePath = (pathname: string) => !/\.[^/]*$/.test(pathname);
+/**
+ * A path that may be a page: no file extension in its last segment, that is no "." after the last
+ * "/". Two `lastIndexOf`s, not `/\.[^/]*$/`, which backtracks on a long run of dots (#174).
+ */
+export const isPagePath = (pathname: string) =>
+  pathname.lastIndexOf(".") <= pathname.lastIndexOf("/");

apps/docs/test/paths.test.ts

@@ -0,0 +1,28 @@
+/** #174 item 3: `isPagePath` answers as `/\.[^/]*$/` did, in linear time. */
+import { describe, expect, it } from "vitest";
+import { isPagePath } from "../src/worker/paths.ts";
+
+describe("isPagePath (#174)", () => {
+  it("answers as the regex did", () => {
+    const old = (p: string) => !/\.[^/]*$/.test(p);
+    let s = 174;
+    const next = () => {
+      s = (s * 1103515245 + 12345) & 0x7fffffff;
+      return s;
+    };
+    for (let n = 0; n < 5000; n++) {
+      let p = "";
+      for (let i = next() % 10; i > 0; i--) p += ["/", ".", "a", "md"][next() % 4];
+      expect(isPagePath(p), p).toBe(old(p));
+    }
+    expect(isPagePath("/guides/search")).toBe(true);
+    expect(isPagePath("/llms.txt")).toBe(false);
+    expect(isPagePath("/v0.6/guide")).toBe(true);
+  });
+
+  it("stays linear on a long run of dots", () => {
+    const start = performance.now();
+    expect(isPagePath(`/${".".repeat(200_000)}/`)).toBe(true);
+    expect(performance.now() - start).toBeLessThan(1500);
+  });
+});

packages/agent-runtime/src/frontmatter-lines.ts

@@ -0,0 +1,42 @@
+/**
+ * The two line shapes of SKILL.md frontmatter, matched in one linear pass each (#174). They give
+ * the groups `/^\s+([^:]+):\s*(.*)$/` and `/^([A-Za-z][\w-]*):\s*(.*)$/` gave, including where
+ * those regexes backtracked: the first one's `\s+` and `[^:]+` both take whitespace, which made a
+ * line of spaces quadratic, and each `\s*` overlapped `(.*)` before a lone `\r`.
+ */
+
+const isSpace = (ch: string | undefined) => ch !== undefined && /\s/.test(ch);
+/** What `.` does not match without the `s` flag. */
+const LINE_END = /[\n\r\u2028\u2029]/;
+
+/** After the `:` at `colon`: whitespace skipped; undefined when the rest holds a line end. */
+function valueAfter(raw: string, colon: number): string | undefined {
+  let i = colon + 1;
+  while (isSpace(raw[i])) i++;
+  const rest = raw.slice(i);
+  return LINE_END.test(rest) ? undefined : rest;
+}
+
+/** An indented `key: value` line (a nested map entry): `[key, value]`, or undefined. */
+export function nestedLine(raw: string): [string, string] | undefined {
+  let lead = 0;
+  while (isSpace(raw[lead])) lead++;
+  if (lead === 0) return undefined;
+  const colon = raw.indexOf(":");
+  if (colon < 0) return undefined;
+  // The key needs a character: when the indent runs up to the `:`, its last space is the key.
+  const start = lead < colon ? lead : lead >= 2 ? lead - 1 : -1;
+  if (start < 0) return undefined;
+  const value = valueAfter(raw, colon);
+  return value === undefined ? undefined : [raw.slice(start, colon), value];
+}
+
+/** A top-level `key: value` line, the key a letter then letters, digits, `_` or `-`. */
+export function topLine(raw: string): [string, string] | undefined {
+  if (!/^[A-Za-z]/.test(raw)) return undefined;
+  let j = 1;
+  while (j < raw.length && /[\w-]/.test(raw[j] as string)) j++;
+  if (raw[j] !== ":") return undefined;
+  const value = valueAfter(raw, j);
+  return value === undefined ? undefined : [raw.slice(0, j), value];
+}

packages/agent-runtime/src/skills.ts

@@ -1,6 +1,7 @@
 import { readdir, readFile, stat } from "node:fs/promises";
 import path from "node:path";
 import { assertValid, type SkillManifest } from "@cloudhal/core-contracts";
+import { nestedLine, topLine } from "./frontmatter-lines.ts";
 
 export interface LoadedSkill {
   manifest: SkillManifest;
@@ -36,14 +37,14 @@ export function parseFrontmatter(markdown: string): {
   let currentMap: Record<string, string> | undefined;
   for (const raw of (m[1] as string).split(/\r?\n/)) {
     if (!raw.trim() || raw.trim().startsWith("#")) continue;
-    const nested = raw.match(/^\s+([^:]+):\s*(.*)$/);
+    const nested = nestedLine(raw);
     if (nested && currentMap) {
-      currentMap[nested[1] as string] = unquote((nested[2] as string).trim());
+      currentMap[nested[0]] = unquote(nested[1].trim());
       continue;
     }
-    const top = raw.match(/^([A-Za-z][\w-]*):\s*(.*)$/);
+    const top = topLine(raw);
     if (!top) throw new SkillError(`invalid frontmatter line: ${raw}`);
-    const [, key, value] = top as unknown as [string, string, string];
+    const [key, value] = top;
     if (value.trim() === "") {
       currentMap = {};
       fm[key] = currentMap;

packages/agent-runtime/test/frontmatter-lines.test.ts

@@ -0,0 +1,49 @@
+/**
+ * #174 item 5: the frontmatter line parsers give the regexes' groups (seeded random lines over the
+ * characters that matter) and stay linear on 100k-character lines that made them backtrack.
+ */
+import { describe, expect, it } from "vitest";
+import { nestedLine, topLine } from "../src/frontmatter-lines.ts";
+import { parseFrontmatter } from "../src/skills.ts";
+
+function strings(alphabet: readonly string[], count: number, maxLength: number, seed = 174) {
+  let s = seed;
+  const next = () => {
+    s = (s * 1103515245 + 12345) & 0x7fffffff;
+    return s;
+  };
+  const out: string[] = [];
+  for (let n = 0; n < count; n++) {
+    let v = "";
+    const len = next() % (maxLength + 1);
+    for (let i = 0; i < len; i++) v += alphabet[next() % alphabet.length];
+    out.push(v);
+  }
+  return out;
+}
+
+const groups = (m: RegExpMatchArray | null) => (m ? [m[1], m[2]] : undefined);
+
+describe("frontmatter lines (#174 item 5)", () => {
+  it("match exactly what the regexes matched", () => {
+    const parts = [" ", " ", "\t", ":", "a", "B", "-", "_", "1", "\r", "\u2028", "'"];
+    for (const raw of strings(parts, 10_000, 9)) {
+      expect(nestedLine(raw), JSON.stringify(raw)).toEqual(
+        groups(raw.match(/^\s+([^:]+):\s*(.*)$/)),
+      );
+      expect(topLine(raw), JSON.stringify(raw)).toEqual(
+        groups(raw.match(/^([A-Za-z][\w-]*):\s*(.*)$/)),
+      );
+    }
+  });
+
+  it("stays linear on lines that made the regexes backtrack", () => {
+    const start = performance.now();
+    expect(nestedLine(" ".repeat(200_000))).toBeUndefined();
+    expect(nestedLine(`  key:${" ".repeat(200_000)}x\r`)).toBeUndefined();
+    expect(topLine(`name:${" ".repeat(200_000)}x\r`)).toBeUndefined();
+    const fm = `---\nmetadata:\n${" ".repeat(200_000)}\n---\nbody`;
+    expect(() => parseFrontmatter(fm)).not.toThrow();
+    expect(performance.now() - start).toBeLessThan(1500);
+  });
+});

packages/api/src/email.ts

@@ -0,0 +1,21 @@
+/**
+ * The address check of the invite and password-reset routes (#174). It accepts exactly what
+ * `/^[^\s@]+@[^\s@]+\.[^\s@]+$/` accepted, in one linear pass, and checks the length first. That
+ * regex backtracked quadratically on `a@` followed by many dots (the domain part is ambiguous on
+ * "."), and it ran before the length cap on the unauthenticated `POST /api/password-reset/request`.
+ */
+
+/** RFC 5321's limit on a forward path: longer is never an address. */
+export const EMAIL_MAX_LENGTH = 254;
+
+/**
+ * One `@` with something before it; after it, no `@` and a `.` with something on both sides; no
+ * whitespace anywhere; at most `EMAIL_MAX_LENGTH` characters.
+ */
+export function isEmailAddress(value: string): boolean {
+  if (value.length > EMAIL_MAX_LENGTH || /\s/.test(value)) return false;
+  const at = value.indexOf("@");
+  if (at < 1 || value.indexOf("@", at + 1) !== -1) return false;
+  // `[^\s@]+\.[^\s@]+`: a dot that is neither the domain's first nor its last character.
+  return value.slice(at + 2, -1).includes(".");
+}

packages/api/src/routes/invites.ts

@@ -29,6 +29,7 @@ import type { Actor } from "@cloudhal/core-contracts";
 import { type Context, Hono } from "hono";
 import { actorIdForUser, type StaffService } from "../auth.ts";
 import type { Env } from "../context.ts";
+import { isEmailAddress } from "../email.ts";
 import { apiError, badRequest, notFound } from "../errors.ts";
 import { MailError } from "../mail.ts";
 import { inviteMail } from "../mail-messages.ts";
@@ -82,8 +83,6 @@ export function inviteView(i: InviteRow): InviteView {
   };
 }
 
-export const EMAIL = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
-
 function gone() {
   return apiError(
     410,
@@ -210,7 +209,7 @@ export function inviteRoutes(options: {
       const { person, actor, staff } = me(c);
       const body = ((await c.req.json().catch(() => ({}))) ?? {}) as Record<string, unknown>;
       const email = typeof body.email === "string" ? body.email.trim().toLowerCase() : "";
-      if (!EMAIL.test(email) || email.length > 254) throw badRequest("a valid email is required");
+      if (!isEmailAddress(email)) throw badRequest("a valid email is required");
       const { role, sites } = await inviteTarget(c, person, body);
       const hours =
         body.expiresInHours === undefined ? inviteDefaultHours(role) : body.expiresInHours;

packages/api/src/routes/password-reset.ts

@@ -18,6 +18,7 @@ import type { Actor } from "@cloudhal/core-contracts";
 import { type Context, Hono } from "hono";
 import { actorIdForUser, type StaffService } from "../auth.ts";
 import type { Env } from "../context.ts";
+import { isEmailAddress } from "../email.ts";
 import { apiError, badRequest, notFound } from "../errors.ts";
 import { MailError, type MailSender } from "../mail.ts";
 import { passwordChangedMail, resetMail } from "../mail-messages.ts";
@@ -30,7 +31,6 @@ import {
   RESET_TOKEN_PREFIX,
   studioOrigin,
 } from "../tokens.ts";
-import { EMAIL } from "./invites.ts";
 import { auditEvent, RESET_LINK_MINUTES } from "./members.ts";
 
 /** RFC-0011: reset mails one address can receive per hour, whoever asks. */
@@ -120,7 +120,7 @@ export function passwordResetRoutes(options: {
       await limitPublic(c, "reset-request");
       const raw = (await jsonBody(c)).email;
       const email = typeof raw === "string" ? raw.trim().toLowerCase() : "";
-      if (!EMAIL.test(email) || email.length > 254) throw badRequest("a valid email is required");
+      if (!isEmailAddress(email)) throw badRequest("a valid email is required");
       c.get("defer")(
         mailResetLink({
           staff,

packages/api/src/tokens.ts

@@ -37,15 +37,64 @@ export function hashToken(token: string): Promise<string> {
   return sha256Hex(token);
 }
 
-/** Public token endpoints take JSON bodies only (a cross-site form post needs a preflight). */
+/**
+ * The largest body a public token endpoint reads (#174): an email, a token, a password and a name
+ * fit many times over. Anything larger is refused before it is parsed or checked.
+ */
+export const JSON_BODY_MAX_BYTES = 64 * 1024;
+
+function tooLarge() {
+  return apiError(
+    413,
+    "payload_too_large",
+    `the body must be at most ${JSON_BODY_MAX_BYTES} bytes`,
+  );
+}
+
+/** The body as text, or a 413 once more than `max` bytes arrive (a chunked body has no length). */
+async function cappedText(request: Request, max: number): Promise<string> {
+  if (Number(request.headers.get("content-length") ?? 0) > max) throw tooLarge();
+  const reader = request.body?.getReader();
+  if (!reader) return "";
+  const chunks: Uint8Array[] = [];
+  let size = 0;
+  for (;;) {
+    const { done, value } = await reader.read();
+    if (done) break;
+    size += value.byteLength;
+    if (size > max) {
+      await reader.cancel().catch(() => undefined);
+      throw tooLarge();
+    }
+    chunks.push(value);
+  }
+  const all = new Uint8Array(size);
+  let at = 0;
+  for (const chunk of chunks) {
+    all.set(chunk, at);
+    at += chunk.byteLength;
+  }
+  return new TextDecoder().decode(all);
+}
+
+/**
+ * Public token endpoints take JSON bodies only (a cross-site form post needs a preflight), of at
+ * most `JSON_BODY_MAX_BYTES`.
+ */
 export async function jsonBody(c: Context<Env>): Promise<Record<string, unknown>> {
   if (!(c.req.header("content-type") ?? "").toLowerCase().startsWith("application/json"))
     throw apiError(
       415,
       "unsupported_media_type",
       "send a JSON body (content-type: application/json)",
     );
-  const body = (await c.req.json().catch(() => undefined)) as unknown;
+  const text = await cappedText(c.req.raw, JSON_BODY_MAX_BYTES);
+  let body: unknown;
+  try {
+    body = JSON.parse(text);
+  } catch {
+    body = undefined;
+  }
   if (!body || typeof body !== "object" || Array.isArray(body))
     throw apiError(400, "bad_request", "the body must be a JSON object");
   return body as Record<string, unknown>;