fix(security): prototype pollution, HTML text and ReDoS backlog (CodeQL)

.changeset/codeql-backlog-security.md

@@ -0,0 +1,14 @@
+---
+"@cloudhal/core-ts": patch
+"@cloudhal/astro-renderer": patch
+"@cloudhal/api": patch
+"@cloudhal/mcp-server": patch
+"@cloudhal/cloudflare": patch
+"@cloudhal/cli": patch
+---
+
+Security fixes from the CodeQL backlog on `main`.
+
+- **Prototype pollution.** A2UI `updateDataModel` paths (`@cloudhal/core-ts/interop`) that name `__proto__`, `constructor` or `prototype` at any depth are now refused with an `InteropError` instead of being skipped, step into arrays by index only, and write own properties only; an input whose component id is one of those keys gets the data key `<id>-value`. Surface pointers the page writes through (an input's `value`/`checked`, a Scrolly's `step`, a Button action's `path`) may not name those keys (a `checkSurface` issue); read bindings still may. The Build eval's sample inputs never write through them, and `set_chrome` and `update_entry`'s `fields` merge patch refuse them as keys. `RESERVED_KEYS` and `isReservedKey` are new in `@cloudhal/core-ts/names`.
+- **Plain text from HTML.** `htmlText` (meta and feed descriptions, JSON-LD, table-of-contents labels) is plain text: `&lt;script&gt;` in markdown is the text `<script>`, as before, and every place it goes escapes it (now tested sink by sink). Tags are dropped with a linear scan, and entities are decoded once (`&amp;#60;` is the text `&#60;`, no longer `<`).
+- **Linear scans (AGENTS.md rule 13).** The svg sniff of uploads (exponential on `<!--` followed by many `--><!--`), the code-fence opener of surface bodies, the rich-text-to-text and text-to-rich-text field conversions, hostname normalization, the leading-H1 check of articles, the table of contents, the `::embed` check, the sender-address check (`isMailAddress`, new in `@cloudhal/api`; `MAIL_ADDRESS` in `@cloudhal/cloudflare/mail` is deprecated) and Studio's origin trim no longer backtrack. Each gives the old regex's answers; one intended difference: an svg comment ends at its first `-->`.

packages/api/src/build-eval.ts

@@ -409,9 +409,12 @@ export function withSampleInputs(page: Page): { page: Page; inputs: number } {
       if (c.component !== "NumberField" && c.component !== "RadioGroup") continue;
       inputs++;
       const keys = bound.path.split("/").slice(1);
+      // The store refuses these in pointers; checked again here because this writes through them.
+      if (keys.some((k) => k === "__proto__" || k === "constructor" || k === "prototype")) continue;
       let at: Record<string, unknown> = data;
       for (const k of keys.slice(0, -1)) {
-        if (typeof at[k] !== "object" || at[k] === null) at[k] = {};
+        const next = Object.hasOwn(at, k) ? at[k] : undefined;
+        if (typeof next !== "object" || next === null) at[k] = {};
         at = at[k] as Record<string, unknown>;
       }
       const last = keys[keys.length - 1] as string;

packages/api/src/email.ts

@@ -19,3 +19,17 @@ export function isEmailAddress(value: string): boolean {
   // `[^\s@]+\.[^\s@]+`: a dot that is neither the domain's first nor its last character.
   return value.slice(at + 2, -1).includes(".");
 }
+
+/**
+ * A sender address (`mail.from` in cloudhal.json, `CLOUDHAL_MAIL_FROM`): a plain `local@domain`
+ * with no display name, no whitespace or line breaks and no `<`, `>` or `"`. It accepts exactly
+ * what `/^[^\s@<>"]+@[^\s@<>"]+\.[^\s@<>"]+$/` accepted, linearly: that regex backtracked
+ * quadratically on `a@` followed by many `.x` (CodeQL js/polynomial-redos). No length cap here;
+ * callers that need one check `EMAIL_MAX_LENGTH`.
+ */
+export function isMailAddress(value: string): boolean {
+  if (/[\s<>"]/.test(value)) return false;
+  const at = value.indexOf("@");
+  if (at < 1 || value.indexOf("@", at + 1) !== -1) return false;
+  return value.slice(at + 2, -1).includes(".");
+}

packages/api/src/index.ts

@@ -40,6 +40,7 @@ export {
   TOOL_EVAL_BRIEFS,
 } from "./build-eval.ts";
 export type { ActorResolver, ApiDeps } from "./context.ts";
+export { EMAIL_MAX_LENGTH, isEmailAddress, isMailAddress } from "./email.ts";
 export type { ApiErrorBody } from "./errors.ts";
 export { MailError, type MailMessage, type MailSender } from "./mail.ts";
 export { oauthClientsPruneJob } from "./oauth-prune-job.ts";

packages/api/src/tokens.ts

@@ -4,6 +4,7 @@
  * returned once, travels in a URL fragment, and never appears in audit rows or logs.
  */
 import { sha256Hex } from "@cloudhal/core-ts";
+import { trimEndChar } from "@cloudhal/core-ts/names";
 import type { Context } from "hono";
 import type { Env } from "./context.ts";
 import { apiError } from "./errors.ts";
@@ -135,7 +136,8 @@ export function studioOrigin(
   configured: string | undefined,
   trusted: ReadonlySet<string>,
 ): string {
-  if (configured) return configured.replace(/\/+$/, "");
+  // `trimEndChar`, not `/\/+$/`, which backtracks on a long run of "/" (js/polynomial-redos).
+  if (configured) return trimEndChar(configured, "/");
   const own = new URL(c.req.url).origin;
   const origin = c.req.header("origin");
   return origin && (origin === own || trusted.has(origin)) ? origin : own;

packages/api/test/design-pass.test.ts

@@ -435,5 +435,27 @@ describe("tool briefs (RFC-0022 22e)", () => {
       ],
     } as never);
     expect((kept.page.sections[0] as { data: unknown }).data).toEqual({ kg: 70 });
+    // a pointer through a reserved key is skipped: nothing is written to Object.prototype
+    for (const path of ["/__proto__/kg", "/constructor/prototype/kg"]) {
+      const skipped = withSampleInputs({
+        sections: [
+          {
+            id: "sec_x0000001",
+            kind: "surface",
+            catalog: "blog@1",
+            data: {},
+            components: [
+              {
+                id: "root",
+                component: "NumberField",
+                props: { label: "W", min: 1, value: { path } },
+              },
+            ],
+          },
+        ],
+      } as never);
+      expect((skipped.page.sections[0] as { data: unknown }).data).toEqual({});
+      expect(({} as Record<string, unknown>).kg).toBeUndefined();
+    }
   });
 });

packages/api/test/linear-checks.test.ts

@@ -0,0 +1,62 @@
+/** CodeQL alerts 11 and 19: the sender-address check and Studio's origin trim, linear (rule 13). */
+import { describe, expect, it } from "vitest";
+import { isMailAddress } from "../src/email.ts";
+import { studioOrigin } from "../src/tokens.ts";
+
+const OLD_MAIL_ADDRESS = /^[^\s@<>"]+@[^\s@<>"]+\.[^\s@<>"]+$/;
+
+/** Every string of up to `max` characters over `alphabet`. */
+function* strings(alphabet: readonly string[], max: number): Generator<string> {
+  let all = [""];
+  yield "";
+  for (let len = 1; len <= max; len++) {
+    all = all.flatMap((s) => alphabet.map((c) => s + c));
+    yield* all;
+  }
+}
+
+const fast = (f: () => void) => {
+  const start = performance.now();
+  f();
+  expect(performance.now() - start).toBeLessThan(1500);
+};
+
+describe("isMailAddress (alert 19)", () => {
+  it("accepts exactly what the old regex accepted", () => {
+    for (const s of [
+      "[email protected]",
+      "Studio <[email protected]>",
+      '"q"@example.com',
+      "[email protected]",
+      "[email protected]\n",
+      " [email protected]",
+      `${"a".repeat(300)}@b.c`,
+    ])
+      expect(isMailAddress(s), JSON.stringify(s)).toBe(OLD_MAIL_ADDRESS.test(s));
+    for (const s of strings(["a", ".", "@", " ", "<", '"'], 6))
+      expect(isMailAddress(s), JSON.stringify(s)).toBe(OLD_MAIL_ADDRESS.test(s));
+  });
+
+  it("is linear on a long run of .x after the @", () => {
+    fast(() => expect(isMailAddress(`!@!.${"!.".repeat(100_000)} `)).toBe(false));
+    fast(() => expect(isMailAddress(`a@b${".c".repeat(100_000)}`)).toBe(true));
+  });
+});
+
+describe("studioOrigin (alert 11)", () => {
+  const origin = (configured: string) => studioOrigin({} as never, configured, new Set());
+  it("trims trailing slashes as /\\/+$/ did", () => {
+    for (const s of [
+      "https://studio.example",
+      "https://studio.example/",
+      "https://s.example///",
+      "/",
+    ])
+      expect(origin(s)).toBe(s.replace(/\/+$/, ""));
+    // An empty result falls through to the request's own origin, as before: not tested here.
+  });
+  it("is linear on a long run of / with something after it", () => {
+    const inner = `https://a${"/".repeat(100_000)}b`;
+    fast(() => expect(origin(`${inner}/`)).toBe(inner));
+  });
+});

packages/astro-renderer/src/embed-client.ts

@@ -10,6 +10,7 @@
  * version (`/_cloudhal/*` is cached as immutable): bump it with any change to the code; a test
  * pins the code's hash to the version.
  */
+import { hasEmbedLine } from "./text-scan.ts";
 
 /** The only hosts an `embed` player may load from (also the CSP `frame-src`). */
 export const EMBED_FRAME_ORIGINS = [
@@ -88,7 +89,7 @@ export function usesEmbeds(content: {
   markdown?: string | undefined;
   sections?: readonly { kind: string; type?: string; format?: string; body?: string }[] | undefined;
 }): boolean {
-  const md = (s: string | undefined) => !!s && /(^|\n)\s*(?:>\s*)*::embed\b/.test(s);
+  const md = (s: string | undefined) => !!s && hasEmbedLine(s);
   if (md(content.markdown)) return true;
   return (content.sections ?? []).some(
     (s) =>

packages/astro-renderer/src/kit.ts

@@ -5,6 +5,7 @@
  */
 import type { ContentEntry, Correction } from "@cloudhal/core-contracts";
 import { htmlText, renderBody, renderInlineMarkdown } from "./markdown.ts";
+import { renderedHeadings } from "./text-scan.ts";
 
 /** One heading of an article's table of contents; `id` is the anchor the rendered body carries. */
 export interface TocItem {
@@ -22,18 +23,12 @@ export interface ArticleOutline {
 /** Words per minute when a theme sets none (RFC-0018 §1 `readingSpeed`). */
 export const READING_SPEED = 230;
 
-const HEADING = /<h([23]) id="([^"]+)">([\s\S]*?)<\/h\1>/g;
-
 /**
  * Headings of HTML rendered by `renderMarkdown`/`renderBody` (their ids come from `headingSlug`).
  * HTML bodies keep no ids through the sanitizer, so they have no table of contents.
  */
 export function tableOfContents(html: string): TocItem[] {
-  return [...html.matchAll(HEADING)].map((m) => ({
-    id: m[2] as string,
-    text: htmlText(m[3] as string),
-    depth: Number(m[1]) as 2 | 3,
-  }));
+  return renderedHeadings(html).map((h) => ({ id: h.id, text: htmlText(h.inner), depth: h.depth }));
 }
 
 /**

packages/astro-renderer/src/markdown.ts

@@ -19,6 +19,7 @@ import {
 import { safeHref, safeSrc } from "./safe-urls.ts";
 import { sanitizeHtml } from "./sanitize.ts";
 import { renderSurface, surfaceProblems } from "./surfaces.ts";
+import { htmlText, leadingHeading, stripTags } from "./text-scan.ts";
 
 export type { MediaContext } from "./media.ts";
 
@@ -357,35 +358,17 @@ export function renderInlineMarkdown(source: string): string {
   }
 }
 
-const ENTITIES: Record<string, string> = {
-  "&amp;": "&",
-  "&lt;": "<",
-  "&gt;": ">",
-  "&quot;": '"',
-  "&#39;": "'",
-};
-
 /** Markdown as plain text (meta descriptions): rendered safely, then tags dropped and entities decoded. */
 export function markdownText(source: string): string {
   return htmlText(renderMarkdown(source));
 }
 
 /** Inline markdown as plain text: inline tags go without leaving gaps ("**537**," → "537,"). */
 export function inlineMarkdownText(source: string): string {
-  return htmlText(renderInlineMarkdown(source).replace(/<[^>]*>/g, ""));
+  return htmlText(stripTags(renderInlineMarkdown(source), ""));
 }
 
-/** Text of HTML this module produced: tags dropped, entities decoded, whitespace collapsed. */
-export function htmlText(html: string): string {
-  return html
-    .replace(/<[^>]*>/g, " ")
-    .replace(/&(?:amp|lt|gt|quot|#39);/g, (m) => ENTITIES[m] ?? m)
-    .replace(/&#(\d{1,7});/g, (m, n: string) =>
-      Number(n) <= 0x10ffff ? String.fromCodePoint(Number(n)) : m,
-    )
-    .replace(/\s+/g, " ")
-    .trim();
-}
+export { htmlText };
 
 /**
  * `markdown` bodies are rendered as data; `html` bodies go through the allowlist sanitizer
@@ -492,9 +475,9 @@ export function entryArticleParts(
 ): { body: ContentEntry["body"]; header: string; cover: string; media: MediaContext } {
   let body = entry.body;
   if (body.format === "markdown") {
-    const m = body.value.match(/^\s*#\s+(.+?)\s*#*\s*(?:\r?\n|$)/);
-    if (m && m[1]?.trim().toLowerCase() === entry.title.trim().toLowerCase())
-      body = { ...body, value: body.value.slice(m[0].length) };
+    const h1 = leadingHeading(body.value);
+    if (h1 && h1.text.trim().toLowerCase() === entry.title.trim().toLowerCase())
+      body = { ...body, value: body.value.slice(h1.length) };
   }
   const when = entry.publishedAt ?? undefined;
   const date = when