From 24fe76b261f206ad10a80941780c2c6ccb62bac9 Mon Sep 17 00:00:00 2001 From: Madraka <8898640+Madraka@users.noreply.github.com> Date: Tue, 10 Nov 2026 11:34:15 +0300 Subject: [PATCH 1/4] fix(security): prototype pollution, HTML text and ReDoS backlog (CodeQL) Close the production-code CodeQL alerts open on main since the first scan: 11, 12, 13, 16, 17, 19, 20, 23, 24, 30, 32, 35, 36, 41, 42, 43, 44 and 45. - Prototype pollution: A2UI data paths refuse __proto__, constructor and prototype at every level with an InteropError, write own properties only (defineProperty) and step into arrays by index only. Surface binding pointers with those keys are a checkSurface issue, the Build eval's sample inputs skip them, and set_chrome / update_entry's fields merge refuse them. RESERVED_KEYS and isReservedKey in core-ts/names. - htmlText never returns markup: a linear tag strip, entities decoded once, and a `<` that would open markup becomes U+FF1C. Every sink escapes too (Astro attributes, jsonLdText, the feeds' xml()). - ReDoS (AGENTS.md rule 13): linear scanners for the svg sniff (exponential on uploads), fence opener and closer (no RegExp built from body text), field markdown conversions, hostname normalization, leading H1, table of contents, ::embed check, sender address (isMailAddress in @cloudhal/api; MAIL_ADDRESS deprecated), Studio's origin trim and the editor's paste link check. Each has an equivalence test against the old regex (cases plus seeded random strings) and 100k adversarial inputs. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Madraka <8898640+Madraka@users.noreply.github.com> --- .changeset/codeql-backlog-security.md | 14 + packages/api/src/build-eval.ts | 5 +- packages/api/src/email.ts | 14 + packages/api/src/index.ts | 1 + packages/api/src/tokens.ts | 4 +- packages/api/test/design-pass.test.ts | 22 ++ packages/api/test/linear-checks.test.ts | 62 ++++ packages/astro-renderer/src/embed-client.ts | 3 +- packages/astro-renderer/src/kit.ts | 9 +- packages/astro-renderer/src/markdown.ts | 29 +- packages/astro-renderer/src/text-scan.ts | 158 ++++++++++ .../astro-renderer/test/text-scan.test.ts | 271 ++++++++++++++++++ packages/cli/src/config.ts | 16 +- packages/cli/test/mail-address.test.ts | 23 ++ packages/cloudflare/src/mail.ts | 10 +- packages/core-ts/src/domains.ts | 29 +- packages/core-ts/src/fields/conversions.ts | 37 ++- packages/core-ts/src/fields/markdown-scan.ts | 70 +++++ packages/core-ts/src/media/sniff.ts | 52 +++- packages/core-ts/src/names.ts | 15 + packages/core-ts/src/surface-interop-data.ts | 90 ++++++ packages/core-ts/src/surface-interop.ts | 47 +-- .../core-ts/src/surfaces/bindings/values.ts | 4 + packages/core-ts/src/surfaces/body.ts | 54 +++- .../core-ts/src/surfaces/patch/json-patch.ts | 4 +- .../core-ts/test/interop-pollution.test.ts | 106 +++++++ packages/core-ts/test/linear-scans.test.ts | 235 +++++++++++++++ packages/mcp-server/src/chrome-tools.ts | 11 +- packages/mcp-server/src/entry-fields.ts | 10 +- packages/mcp-server/test/chrome-tools.test.ts | 5 + packages/mcp-server/test/fields.test.ts | 12 + packages/ui/src/blocks/editor/paste.ts | 33 ++- packages/ui/test/paste-redos.test.tsx | 54 ++++ scripts/size-baseline.json | 2 +- 34 files changed, 1389 insertions(+), 122 deletions(-) create mode 100644 .changeset/codeql-backlog-security.md create mode 100644 packages/api/test/linear-checks.test.ts create mode 100644 packages/astro-renderer/src/text-scan.ts create mode 100644 packages/astro-renderer/test/text-scan.test.ts create mode 100644 packages/cli/test/mail-address.test.ts create mode 100644 packages/core-ts/src/fields/markdown-scan.ts create mode 100644 packages/core-ts/src/surface-interop-data.ts create mode 100644 packages/core-ts/test/interop-pollution.test.ts create mode 100644 packages/core-ts/test/linear-scans.test.ts create mode 100644 packages/ui/test/paste-redos.test.tsx diff --git a/.changeset/codeql-backlog-security.md b/.changeset/codeql-backlog-security.md new file mode 100644 index 00000000..b9699166 --- /dev/null +++ b/.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. Surface binding pointers with those keys are a `checkSurface` issue, 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) decoded entities after dropping tags, so `<script>` came out as `"; + const text = markdownText(markdown); + expect(text).toBe( + "Intro \uFF1Cscript>alert(1)\uFF1C/script> and \uFF1Cscript>alert(2)\uFF1C/script>", + ); + const head = archiveHead( + { id: "site_aaaaaaaaaaaa", name: "Haber", defaultLocale: "en" } as never, + { + locale: "en", + page: 1, + path: "/category/news", + root: "/category/news", + shown: { + name: "News", + slug: "news", + href: "/category/news", + taxonomy: { key: "category", labels: { en: { plural: "Categories" } } }, + }, + trail: [], + total: 0, + perPage: 10, + term: { labels: { en: { name: "News", description: markdown } } }, + taxonomy: { key: "category" }, + entries: [], + } as never, + "https://haber.example", + ); + expect(head.description).toBe(text); + // What SeoHead.astro writes: `` + const meta = ``; + expect(meta).not.toMatch(/`); + for (const doc of head.jsonLd) expect(jsonLdText(doc)).not.toMatch(/<|>/); + }); + + it("inline text (correction notes) goes the same way", () => { + expect(inlineMarkdownText("**537**, <script>")).toBe("537, \uFF1Cscript>"); + }); +}); + +describe("leadingHeading", () => { + it("matches the regex version on ordinary bodies", () => { + for (const md of [ + "# Title\n\nBody", + "# Title", + "# Title\n", + " # Title ##\r\nBody", + "\n\n# Title\nBody", + "# Title\n \n\t\nBody", + "# Title\n\n\n", + "# Title #\nBody", + "# C#\nBody", + "# Spaced title \nBody", + "# Other\nBody", + "#Title\nBody", + "Body\n# Title", + "## Title\nBody", + "", + ]) + expect(leadingHeading(md), JSON.stringify(md)).toEqual(oldH1(md)); + }); + + it("needs the space after # on the same line (the one intended difference)", () => { + expect(oldH1("#\nTitle\nBody")?.text).toBe("Title"); + expect(leadingHeading("#\nTitle\nBody")).toBeUndefined(); + }); + + it("is linear on a heading line of spaces", () => { + fast(() => leadingHeading(`# ${" ".repeat(N / 2)}x\rx`)); + expect(fast(() => leadingHeading(`# a${" ".repeat(N)}`))).toEqual({ text: "a", length: N + 3 }); + expect(fast(() => leadingHeading(`${"\n".repeat(N)}#`))).toBeUndefined(); + }); +}); + +describe("renderedHeadings and the table of contents", () => { + it("finds what the regex found", () => { + const html = renderMarkdown( + "# Top\n\n## One\n\ntext\n\n### One *a*\n\n## Two `code`\n\n#### Deep\n\n## One", + ); + for (const h of [ + html, + '

A

B

C

', + '

open

B

', + '

empty

attrs

Z

', + '

no closer', + "", + ]) + expect(renderedHeadings(h)).toEqual(oldHeadings(h)); + expect(tableOfContents(html).map((t) => t.text)).toEqual(["One", "One a", "Two code", "One"]); + }); + + it("is linear on many headings without a closer", () => { + expect(fast(() => renderedHeadings('

a'.repeat(N / 12)))).toEqual([]); + expect(fast(() => renderedHeadings(`${'

'.repeat(N / 11)}

`))).toEqual([]); + }); +}); + +describe("hasEmbedLine", () => { + it("answers as the regex did", () => { + for (const md of [ + "::embed{url=x}", + "text\n::embed{url=x}", + " > > ::embed{url=x}", + ">\n::embed", + "\n\n ::embed", + "::embedded", + "::embed_x", + "text ::embed", + "a\r\n::embed", + "\t>::embed-", + "", + "nothing here", + ]) + expect(hasEmbedLine(md), JSON.stringify(md)).toBe(oldEmbed(md)); + expect(usesEmbeds({ markdown: "> ::embed{url=x}" })).toBe(true); + }); + + it("is linear on long runs of blank and quote lines", () => { + expect(fast(() => hasEmbedLine(`${"\n".repeat(N)}x`))).toBe(false); + expect(fast(() => hasEmbedLine(`${">\n".repeat(N / 2)}x`))).toBe(false); + expect(fast(() => hasEmbedLine(`${"> ".repeat(N / 2)}::embed`))).toBe(true); + }); +}); + +describe("random inputs over the characters that matter", () => { + let s = 41; + const next = () => { + s = (s * 1103515245 + 12345) & 0x7fffffff; + return s; + }; + const random = (tokens: readonly string[], max: number) => { + let t = ""; + for (let i = next() % max; i > 0; i--) t += tokens[next() % tokens.length]; + return t; + }; + it("agree with the regexes they replaced", () => { + for (let n = 0; n < 20_000; n++) { + const html = random(["<", ">", "a", " ", "/", "\n"], 14); + expect(stripTags(html, " "), JSON.stringify(html)).toBe(oldStrip(html, " ")); + const md = random(["\n", " ", ">", "::embed", "x", "\r", "\t", "_"], 10); + expect(hasEmbedLine(md), JSON.stringify(md)).toBe(oldEmbed(md)); + const h = random( + ['

', '

', "

", "", "x", '"', ">", "

; -/** A plain `local@domain` address: no display name, no spaces or line breaks. */ -const MAIL_ADDRESS = /^[^\s@<>"]+@[^\s@<>"]+\.[^\s@<>"]+$/; +/** + * A plain `local@domain` address: no display name, no spaces or line breaks, no `<`, `>` or `"`. + * The same answers as `isMailAddress` in `@cloudhal/api` and the regex both replaced + * (`/^[^\s@<>"]+@[^\s@<>"]+\.[^\s@<>"]+$/`, quadratic on long `.x` runs), in one linear pass; + * not imported from there, so reading `cloudhal.json` never loads the API. + */ +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("."); +} /** `cloudhal.json` at the project root. Kept tiny; everything else lives in the store. */ export interface ProjectConfig { @@ -235,7 +245,7 @@ export function parseMailConfig(input: unknown): MailConfig | undefined { throw new Error(`${CONFIG_FILE}: mail must be an object like {"from": "studio@example.com"}`); const o = input as Record; const from = typeof o.from === "string" ? o.from.trim() : ""; - if (!MAIL_ADDRESS.test(from) || from.length > 254) + if (!isMailAddress(from) || from.length > 254) throw new Error( `${CONFIG_FILE}: mail.from must be a plain address like "studio@example.com" (no display name; use mail.fromName)`, ); diff --git a/packages/cli/test/mail-address.test.ts b/packages/cli/test/mail-address.test.ts new file mode 100644 index 00000000..1b8f57c7 --- /dev/null +++ b/packages/cli/test/mail-address.test.ts @@ -0,0 +1,23 @@ +/** `mail.from` in cloudhal.json: the linear sender check gives the old regex's answers (rule 13). */ +import { describe, expect, it } from "vitest"; +import { isMailAddress, parseMailConfig } from "../src/config.ts"; + +const OLD = /^[^\s@<>"]+@[^\s@<>"]+\.[^\s@<>"]+$/; + +describe("isMailAddress", () => { + it("accepts exactly what the old regex accepted", () => { + let all = [""]; + for (let len = 1; len <= 6; len++) { + all = all.flatMap((s) => ["a", ".", "@", " ", ">"].map((c) => s + c)); + for (const s of all) expect(isMailAddress(s), JSON.stringify(s)).toBe(OLD.test(s)); + } + expect(parseMailConfig({ from: "studio@example.com" })?.from).toBe("studio@example.com"); + expect(() => parseMailConfig({ from: "Studio " })).toThrow(/plain address/); + }); + + it("is linear on a long run of .x", () => { + const start = performance.now(); + expect(isMailAddress(`a@b${".".repeat(100_000)} `)).toBe(false); + expect(performance.now() - start).toBeLessThan(1500); + }); +}); diff --git a/packages/cloudflare/src/mail.ts b/packages/cloudflare/src/mail.ts index 28589bbc..8bf6e3e9 100644 --- a/packages/cloudflare/src/mail.ts +++ b/packages/cloudflare/src/mail.ts @@ -4,7 +4,7 @@ * `CLOUDHAL_MAIL_FROM` / `CLOUDHAL_MAIL_FROM_NAME`. Plain text only; bodies are never logged (they * carry one-time links). */ -import { MailError, type MailMessage, type MailSender } from "@cloudhal/api"; +import { isMailAddress, MailError, type MailMessage, type MailSender } from "@cloudhal/api"; /** The part of the binding's structured `send()` this sender uses. */ export interface SendEmailBinding { @@ -25,7 +25,11 @@ export interface MailEnv { CLOUDHAL_MAIL_FROM_NAME?: string; } -/** A plain `local@domain` address: no display name, no spaces or line breaks. */ +/** + * A plain `local@domain` address: no display name, no spaces or line breaks. + * @deprecated Use `isMailAddress` from `@cloudhal/api`: the same answers in one linear pass (this + * pattern backtracks quadratically on long `.x` runs). + */ export const MAIL_ADDRESS = /^[^\s@<>"]+@[^\s@<>"]+\.[^\s@<>"]+$/; const oneLine = (s: string) => s.replace(/[\r\n]+/g, " ").trim(); @@ -61,7 +65,7 @@ export function cloudflareEmailSender( /** The deployment's sender, when the binding and a valid `CLOUDHAL_MAIL_FROM` are both there. */ export function adminMail(env: MailEnv): MailSender | undefined { const from = env.CLOUDHAL_MAIL_FROM?.trim(); - if (!env.EMAIL || !from || !MAIL_ADDRESS.test(from)) return undefined; + if (!env.EMAIL || !from || !isMailAddress(from)) return undefined; return cloudflareEmailSender(env.EMAIL, { email: from, ...(env.CLOUDHAL_MAIL_FROM_NAME ? { name: env.CLOUDHAL_MAIL_FROM_NAME } : {}), diff --git a/packages/core-ts/src/domains.ts b/packages/core-ts/src/domains.ts index f66ce634..eee08ba5 100644 --- a/packages/core-ts/src/domains.ts +++ b/packages/core-ts/src/domains.ts @@ -8,12 +8,29 @@ const HOSTNAME = /^(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.)+[a-z]{2,63}$/; /** A hostname as a person might type it ("https://News.Example.com/") → "news.example.com". */ export function normalizeHostname(input: string): string { - return input - .trim() - .toLowerCase() - .replace(/^[a-z]+:\/\//, "") - .replace(/[/?#].*$/, "") - .replace(/\.$/, ""); + return cutPathPart( + input + .trim() + .toLowerCase() + .replace(/^[a-z]+:\/\//, ""), + ).replace(/\.$/, ""); +} + +/** + * `text` up to its path, query or fragment: what `.replace(/[/?#].*$/, "")` left, in one linear + * pass. That regex rescanned to the end from every `/`, `?` or `#` when a line break followed + * (js/polynomial-redos). As in it, the cut is at the first of those after the last line break + * (`.` stops at line breaks, `$` is the end). + */ +function cutPathPart(text: string): string { + let from = 0; + for (const br of ["\n", "\r", "\u2028", "\u2029"]) + from = Math.max(from, text.lastIndexOf(br) + 1); + for (let i = from; i < text.length; i++) { + const c = text[i]; + if (c === "/" || c === "?" || c === "#") return text.slice(0, i); + } + return text; } export function isHostname(value: string): boolean { diff --git a/packages/core-ts/src/fields/conversions.ts b/packages/core-ts/src/fields/conversions.ts index c4dca205..052727d8 100644 --- a/packages/core-ts/src/fields/conversions.ts +++ b/packages/core-ts/src/fields/conversions.ts @@ -19,6 +19,7 @@ * Every result must also fit the new definition (`valueIssues`); anything else is unconvertible. */ import type { FieldDefinition } from "@cloudhal/core-contracts"; +import { escapeAtLineStarts, unlink } from "./markdown-scan.ts"; import type { FieldType } from "./types.ts"; import { valueIssues } from "./value-checks.ts"; @@ -54,13 +55,22 @@ type Step = (value: unknown, to: FieldDefinition, ctx: ConvertContext) => Conver const fail = (reason: string): Conversion => ({ ok: false, reason }); const ok = (value: unknown): Conversion => ({ ok: true, value }); +/** Characters that open block markup at a line's start (headings, quotes, lists, setext rules). */ +const LINE_MARKERS = "#>+=-"; +const DIGIT = /^\d$/; + /** CommonMark escapes for the characters that would turn plain words into markup. */ export function escapeMarkdown(text: string): string { - return text - .replace(/[\\`*_[\]<>|~&]/g, "\\$&") - .replace(/^(\s*)([#>+=-])/gm, "$1\\$2") - .replace(/^(\s*\d+)([.)])/gm, "$1\\$2") - .replace(/:(?=:)/g, "\\:"); + // Line starts through linear scans: `/^(\s*)([#>+=-])/gm` and `/^(\s*\d+)([.)])/gm` backtracked + // quadratically on many blank lines (`\s*` runs across them). + const marked = escapeAtLineStarts(text.replace(/[\\`*_[\]<>|~&]/g, "\\$&"), (t, at) => + LINE_MARKERS.includes(t[at] ?? " ") ? at : -1, + ); + return escapeAtLineStarts(marked, (t, at) => { + let end = at; + while (DIGIT.test(t[end] ?? "")) end++; + return end > at && (t[end] === "." || t[end] === ")") ? end : -1; + }).replace(/:(?=:)/g, "\\:"); } /** The words of a markdown string: no markup, link text without targets, no directives. */ @@ -69,12 +79,17 @@ export function markdownToPlain(markdown: string, multiline: boolean): string { for (const raw of markdown.split(/\r?\n/)) { const line = raw.trim(); if (line.startsWith("```") || line.startsWith("~~~") || line.startsWith("::")) continue; - const words = line - .replace(/^#{1,6}\s+/, "") - .replace(/^>\s?/, "") - .replace(/^(?:[-*+]|\d+[.)])\s+/, "") - .replace(/!\[([^\]]*)\]\([^)]*\)/g, "$1") - .replace(/\[([^\]]*)\]\([^)]*\)/g, "$1") + // Images, then links, through linear scans (the regexes before them were quadratic on "!["). + const words = unlink( + unlink( + line + .replace(/^#{1,6}\s+/, "") + .replace(/^>\s?/, "") + .replace(/^(?:[-*+]|\d+[.)])\s+/, ""), + "![", + ), + "[", + ) .replace(/(\*\*|__|~~|\*|_|`)/g, "") .replace(/\\([!-/:-@[-`{-~])/g, "$1") .trim(); diff --git a/packages/core-ts/src/fields/markdown-scan.ts b/packages/core-ts/src/fields/markdown-scan.ts new file mode 100644 index 00000000..8075d4f3 --- /dev/null +++ b/packages/core-ts/src/fields/markdown-scan.ts @@ -0,0 +1,70 @@ +/** + * Linear scanners behind `escapeMarkdown` and `markdownToPlain` (AGENTS.md rule 13). Each gives + * the same answers as the regex it replaced; those backtracked quadratically on field values + * with many `![` or `](` (CodeQL js/polynomial-redos, alert 23) or many blank lines. + */ + +const isBreak = (c: string | undefined): boolean => + c === "\n" || c === "\r" || c === "\u2028" || c === "\u2029"; +const isSpace = (c: string | undefined): boolean => c !== undefined && /\s/.test(c); + +/** The start of the line after the first line break at or after `from` (`-1`: none). */ +function nextLineStart(text: string, from: number): number { + for (let k = from; k < text.length; k++) if (isBreak(text[k])) return k + 1; + return -1; +} + +/** + * A `\` inserted where `find` says, once per line start: what + * `.replace(/^(\s*)…/gm, "$1\\…")` did. As in that regex, `\s*` runs across blank lines, and a + * line start inside a run already tried leads to the same place, so it is not tried again. + * `find(text, at)` gets the first non-space position and returns where the `\` goes, or -1. + */ +export function escapeAtLineStarts( + text: string, + find: (text: string, at: number) => number, +): string { + let out = ""; + let copied = 0; + for (let start = 0; start !== -1; ) { + let at = start; + while (isSpace(text[at])) at++; + const insert = find(text, at); + let next = at; + if (insert !== -1) { + out += `${text.slice(copied, insert)}\\`; + copied = insert; + next = insert + 1; + } + start = nextLineStart(text, next); + } + return copied === 0 && out === "" ? text : out + text.slice(copied); +} + +/** + * `line` with each `[text](target)` (or `![alt](src)` for `"!["`) replaced by its text: what + * `.replace(/!?\[([^\]]*)\]\([^)]*\)/g, "$1")` did, without rescanning to the end from every + * opener when no `]` or `)` follows. + */ +export function unlink(line: string, opener: "[" | "!["): string { + let out = ""; + let copied = 0; + let at = 0; + for (;;) { + const open = line.indexOf(opener, at); + if (open === -1) break; + const textStart = open + opener.length; + const close = line.indexOf("]", textStart); + if (close === -1) break; + // Every opener before this `]` stops at it too, and fails the same way. + if (line[close + 1] !== "(") { + at = close + 1; + continue; + } + const end = line.indexOf(")", close + 2); + if (end === -1) break; + out += line.slice(copied, open) + line.slice(textStart, close); + copied = at = end + 1; + } + return copied === 0 ? line : out + line.slice(copied); +} diff --git a/packages/core-ts/src/media/sniff.ts b/packages/core-ts/src/media/sniff.ts index 61a234fd..144bfbeb 100644 --- a/packages/core-ts/src/media/sniff.ts +++ b/packages/core-ts/src/media/sniff.ts @@ -48,12 +48,52 @@ const ACTIVE_TEXT = /** Never inside a data file's sniffed window, wherever it appears. */ const ACTIVE_ANYWHERE = /<(?:script|html|!doctype\s+html|iframe|object|embed)\b/i; -function isSvgText(text: string): boolean { - // An optional BOM, XML declaration, comments and whitespace, then the svg root element. - const t = text.replace(/^/, ""); - return /^\s*(?:<\?xml[^>]*\?>\s*)?(?:\s*)*(?:]*>\s*)?(?:\s*)*]/i.test( - t, - ); +const isSpace = (c: string | undefined): boolean => c !== undefined && /\s/.test(c); +const skipSpace = (t: string, i: number): number => { + while (isSpace(t[i])) i++; + return i; +}; +const startsWithCi = (t: string, word: string, at: number): boolean => + t.slice(at, at + word.length).toLowerCase() === word; +/** Past any `` comments (each ends at its first `-->`) and the whitespace after them; -1 when one never ends. */ +function skipComments(t: string, at: number): number { + let i = at; + while (t.startsWith("", i + 4); + if (end === -1) return -1; + i = skipSpace(t, end + 3); + } + return i; +} + +/** + * An optional BOM, XML declaration, comments and whitespace, an optional svg DOCTYPE, then the svg + * root element. A linear scan in place of + * `/^\s*(?:<\?xml[^>]*\?>\s*)?(?:\s*)*(?:]*>\s*)?(?:\s*)*]/i`, + * which backtracked exponentially on uploads starting `` as in + * HTML and XML (the regex could stretch one over a later `-->`), which only ever answers "not svg". + */ +export function isSvgText(text: string): boolean { + const t = text.startsWith("\uFEFF") ? text.slice(1) : text; + let i = skipSpace(t, 0); + if (startsWithCi(t, "]*\?>`: the declaration ends at its first `>`, which must follow a `?`. + const gt = t.indexOf(">", i + 5); + if (gt === -1 || gt - 1 < i + 5 || t[gt - 1] !== "?") return false; + i = skipSpace(t, gt + 1); + } + i = skipComments(t, i); + if (i === -1) return false; + if (startsWithCi(t, "", name + 3); + if (gt === -1) return false; + i = skipComments(t, skipSpace(t, gt + 1)); + if (i === -1) return false; + } + return startsWithCi(t, "" || isSpace(t[i + 4])); } /** diff --git a/packages/core-ts/src/names.ts b/packages/core-ts/src/names.ts index 0c0d6e76..8f8b1d84 100644 --- a/packages/core-ts/src/names.ts +++ b/packages/core-ts/src/names.ts @@ -47,6 +47,21 @@ export function trimEndChar(text: string, ch: string): string { return end === text.length ? text : text.slice(0, end); } +/** + * Keys a path, pointer or merge from outside input must never write: through them an assignment + * reaches an object's prototype (`obj.__proto__.x = 1`, `obj.constructor.prototype.x = 1`) and, + * from there, every object (CodeQL js/prototype-polluting-assignment). Writers refuse them with an + * error rather than skipping them, so the caller learns its input was not applied. + */ +export const RESERVED_KEYS: ReadonlySet = new Set([ + "__proto__", + "constructor", + "prototype", +]); + +/** Whether `key` is one of {@link RESERVED_KEYS}. */ +export const isReservedKey = (key: string): boolean => RESERVED_KEYS.has(key); + /** * Folds a name for comparison: NFKC, Turkish-aware lower-casing (İ, I and ı all become i), * diacritics removed, apostrophes and dots dropped (`A.I.` → `ai`), every other run of diff --git a/packages/core-ts/src/surface-interop-data.ts b/packages/core-ts/src/surface-interop-data.ts new file mode 100644 index 00000000..edf9195f --- /dev/null +++ b/packages/core-ts/src/surface-interop-data.ts @@ -0,0 +1,90 @@ +/** + * The data model of surface interop (RFC-0020 §9): JSON Pointer reads and writes over the data an + * A2UI agent sends (`updateDataModel`) or a component binds. Pointers come from agent and MCP + * input, so writes refuse the reserved keys `__proto__`, `constructor` and `prototype` at every + * depth with an `InteropError`, step into arrays by index only, and read and write own properties + * only (CodeQL js/prototype-polluting-assignment, alerts 42–44). Worker-safe: pure functions. + */ +import { isObj } from "./surfaces/shared.ts"; + +type Json = Record; + +/** A document that cannot be converted at all (not a wrong prop: those are notes or store issues). */ +export class InteropError extends Error { + override readonly name = "InteropError"; +} + +const ARRAY_INDEX = /^\d+$/; + +/** RFC 6901 segments of `pointer` (`/a/b~1c` → `["a", "b/c"]`). */ +const segmentsOf = (pointer: string): string[] => + pointer + .replace(/^\//, "") + .split("/") + .map((s) => s.replace(/~1/g, "/").replace(/~0/g, "~")); + +/** The value at `pointer` in `data` (own properties only), or `undefined`. */ +export function pointerGet(data: unknown, pointer: string): unknown { + if (pointer === "" || pointer === "/") return data; + let cur = data; + for (const seg of segmentsOf(pointer)) { + if (Array.isArray(cur)) cur = ARRAY_INDEX.test(seg) ? cur[Number(seg)] : undefined; + else if (isObj(cur) && Object.hasOwn(cur, seg)) cur = cur[seg]; + else return undefined; + } + return cur; +} + +/** Defines `key` as an own data property: never a setter, never the prototype chain. */ +function defineOwn(target: Json, key: string, value: unknown): void { + Object.defineProperty(target, key, { + value, + writable: true, + enumerable: true, + configurable: true, + }); +} + +/** Refuses a segment that would reach a prototype, or a non-index step into an array. */ +function checkSegment(container: Json | unknown[], seg: string, pointer: string): void { + // Compared inline at each step, so every write below sits behind the check. + if (seg === "__proto__" || seg === "constructor" || seg === "prototype") + throw new InteropError(`data path ${pointer} uses the reserved key "${seg}"`); + if (Array.isArray(container) && !ARRAY_INDEX.test(seg)) + throw new InteropError(`data path ${pointer}: "${seg}" is not an array index`); +} + +/** The child container at `seg`, created (an array when `nextSeg` is an index) when missing. */ +function child(cur: Json | unknown[], seg: string, nextSeg: string): Json | unknown[] { + const next: unknown = Array.isArray(cur) + ? cur[Number(seg)] + : Object.hasOwn(cur, seg) + ? cur[seg] + : undefined; + if (isObj(next) || Array.isArray(next)) return next; + const made: Json | unknown[] = ARRAY_INDEX.test(nextSeg) ? [] : {}; + if (Array.isArray(cur)) cur[Number(seg)] = made; + else defineOwn(cur, seg, made); + return made; +} + +/** `data` with `value` written at `pointer` (deleted when `value` is undefined), on a copy. */ +export function pointerSet(data: Json, pointer: string, value: unknown): Json { + if (pointer === "" || pointer === "/") return isObj(value) ? structuredClone(value) : {}; + const segs = segmentsOf(pointer); + const out = structuredClone(data); + let cur: Json | unknown[] = out; + for (let i = 0; i < segs.length - 1; i++) { + const seg = segs[i] as string; + checkSegment(cur, seg, pointer); + cur = child(cur, seg, segs[i + 1] as string); + } + const last = segs.at(-1) as string; + checkSegment(cur, last, pointer); + if (Array.isArray(cur)) { + if (value === undefined) cur.splice(Number(last), 1); + else cur[Number(last)] = value; + } else if (value !== undefined) defineOwn(cur, last, value); + else if (Object.hasOwn(cur, last)) delete cur[last]; + return out; +} diff --git a/packages/core-ts/src/surface-interop.ts b/packages/core-ts/src/surface-interop.ts index a67ad64f..9d3e6332 100644 --- a/packages/core-ts/src/surface-interop.ts +++ b/packages/core-ts/src/surface-interop.ts @@ -14,6 +14,9 @@ import type { SurfaceComponent, SurfaceSection } from "@cloudhal/core-contracts"; import { nearestIcon } from "./icon-names.ts"; import { trimChar } from "./names.ts"; +import { InteropError, pointerGet, pointerSet } from "./surface-interop-data.ts"; + +export { InteropError }; export const A2UI_VERSION = "v0.9"; export const A2UI_BASIC_CATALOG = "https://a2ui.org/specification/v0_9/catalogs/basic/catalog.json"; @@ -70,45 +73,6 @@ class Ids { } } -function pointerGet(data: unknown, pointer: string): unknown { - if (pointer === "" || pointer === "/") return data; - let cur = data; - for (const raw of pointer.replace(/^\//, "").split("/")) { - const seg = raw.replace(/~1/g, "/").replace(/~0/g, "~"); - if (Array.isArray(cur)) cur = /^\d+$/.test(seg) ? cur[Number(seg)] : undefined; - else if (isObj(cur) && Object.hasOwn(cur, seg)) cur = cur[seg]; - else return undefined; - } - return cur; -} - -function pointerSet(data: Json, pointer: string, value: unknown): Json { - if (pointer === "" || pointer === "/") return isObj(value) ? structuredClone(value) : {}; - const segs = pointer - .replace(/^\//, "") - .split("/") - .map((s) => s.replace(/~1/g, "/").replace(/~0/g, "~")); - if (segs.some((s) => s === "__proto__" || s === "prototype" || s === "constructor")) return data; - const out = structuredClone(data); - let cur: Json | unknown[] = out; - for (let i = 0; i < segs.length - 1; i++) { - const s = segs[i] as string; - const next: unknown = Array.isArray(cur) ? cur[Number(s)] : cur[s]; - const container: Json | unknown[] = - isObj(next) || Array.isArray(next) ? next : /^\d+$/.test(segs[i + 1] ?? "") ? [] : {}; - if (Array.isArray(cur)) cur[Number(s)] = container; - else cur[s] = container; - cur = container; - } - const last = segs.at(-1) as string; - if (value === undefined) { - if (Array.isArray(cur)) cur.splice(Number(last), 1); - else delete cur[last]; - } else if (Array.isArray(cur)) cur[Number(last)] = value; - else cur[last] = value; - return out; -} - // ── A2UI → Cloudhal ───────────────────────────────────────────────────────────────────────── interface A2uiState { @@ -1226,8 +1190,3 @@ export function toJsonRender(surface: Pick SURFACE_LIMITS.pointerSegments ) out.push({ path, message: "a JSON Pointer into the surface's data, such as /plans/0/price" }); + // Inputs write through their pointer (the page's runtime, the Build eval's sample inputs). + else if (segments.some(isReservedKey)) + out.push({ path, message: "__proto__, constructor and prototype are reserved keys" }); } export function checkOperand(v: unknown, path: string, out: PropsIssue[]): void { diff --git a/packages/core-ts/src/surfaces/body.ts b/packages/core-ts/src/surfaces/body.ts index f3b00487..dd317524 100644 --- a/packages/core-ts/src/surfaces/body.ts +++ b/packages/core-ts/src/surfaces/body.ts @@ -8,11 +8,49 @@ import { isObj } from "./shared.ts"; // ── surfaces in markdown bodies (RFC-0020 §5) ──────────────────────────────────────────────── -/** A fence opening a surface: ```surface {"id":"pricing","catalog":"blog@1"} (or ~~~). */ -export const FENCE_OPEN = /^ {0,3}(`{3,}|~{3,})[ \t]*([^\s`]*)(.*)$/; +const LINE_BREAK = /[\n\r\u2028\u2029]/; +const isSpace = (c: string) => /\s/.test(c); + +/** + * A line opening a code fence: the fence (```` ``` ```` or `~~~`, three or more), the info string's + * first word and the rest of the line, as in ```surface {"id":"pricing","catalog":"blog@1"}. + * What `/^ {0,3}(`{3,}|~{3,})[ \t]*([^\s`]*)(.*)$/` matched, in one pass: that regex was + * quadratic on a fence followed by many tabs and a lone `\r` (js/polynomial-redos, alert 30). + */ +export function fenceOpen(line: string): [fence: string, word: string, rest: string] | undefined { + // `.` and `$` (no `m` flag): the regex never matched a line holding a line break. + if (LINE_BREAK.test(line)) return undefined; + let i = 0; + while (i < 3 && line[i] === " ") i++; + const ch = line[i]; + if (ch !== "`" && ch !== "~") return undefined; + const start = i; + while (line[i] === ch) i++; + if (i - start < 3) return undefined; + const fence = line.slice(start, i); + while (line[i] === " " || line[i] === "\t") i++; + const word = i; + while (i < line.length && line[i] !== "`" && !isSpace(line[i] as string)) i++; + return [fence, line.slice(word, i), line.slice(i)]; +} export const SURFACE_ID = /^[a-z][a-z0-9-]{0,40}$/; export const CATALOG_ID = /^[a-z][a-z0-9-]{0,40}@[0-9]{1,4}$/; +/** + * Whether `line` closes a fence opened with `fence`: up to three spaces, at least as many of its + * character, then only spaces or tabs. What `^ {0,3}`{n,}[ \t]*$` tested, without building a + * RegExp from the body's text for every fence (CodeQL followed the line into it, alert 45). + */ +export function closesFence(line: string, fence: string): boolean { + let i = 0; + while (i < 3 && line[i] === " ") i++; + const start = i; + while (line[i] === fence[0]) i++; + if (i - start < fence.length) return false; + while (line[i] === " " || line[i] === "\t") i++; + return i === line.length; +} + /** One ```surface fence of a body, parsed line by line. */ export interface BodySurface { /** The fence's `id` (unique in the body). */ @@ -116,15 +154,13 @@ export function bodySurfaces(markdown: string): BodySurface[] { const lines = markdown.split(/\r?\n/); const out: BodySurface[] = []; for (let i = 0; i < lines.length; i++) { - const m = FENCE_OPEN.exec(lines[i] as string); + const m = fenceOpen(lines[i] as string); if (!m) continue; - const fence = m[1] as string; - const closes = (l: string) => - new RegExp(`^ {0,3}${fence[0] === "`" ? "`" : "~"}{${fence.length},}[ \\t]*$`).test(l); + const [fence, word, rest] = m; let j = i + 1; - while (j < lines.length && !closes(lines[j] as string)) j++; - if (m[2] === "surface") - out.push(parseSurfaceFence((m[3] ?? "").trim(), lines.slice(i + 1, j), j < lines.length, i)); + while (j < lines.length && !closesFence(lines[j] as string, fence)) j++; + if (word === "surface") + out.push(parseSurfaceFence(rest.trim(), lines.slice(i + 1, j), j < lines.length, i)); i = j; } return out; diff --git a/packages/core-ts/src/surfaces/patch/json-patch.ts b/packages/core-ts/src/surfaces/patch/json-patch.ts index a5bb69c2..2b5af724 100644 --- a/packages/core-ts/src/surfaces/patch/json-patch.ts +++ b/packages/core-ts/src/surfaces/patch/json-patch.ts @@ -1,4 +1,5 @@ /** RFC 6902 JSON Patch over plain JSON, with forbidden keys refused. */ +import { RESERVED_KEYS } from "../../names.ts"; import { pointerSegments } from "../bindings/values.ts"; import { isObj, SURFACE_LIMITS } from "../shared.ts"; @@ -24,7 +25,8 @@ export class PatchError extends Error { } export const OPS = new Set(["add", "remove", "replace", "move", "copy", "test"]); -export const FORBIDDEN_KEYS = new Set(["__proto__", "constructor", "prototype"]); +/** The reserved keys of `@cloudhal/core-ts/names`, refused at every level of a patch path. */ +export const FORBIDDEN_KEYS = RESERVED_KEYS; export function container(doc: unknown, segments: string[], index: number, path: string): unknown { let cur = doc; diff --git a/packages/core-ts/test/interop-pollution.test.ts b/packages/core-ts/test/interop-pollution.test.ts new file mode 100644 index 00000000..c364f6d6 --- /dev/null +++ b/packages/core-ts/test/interop-pollution.test.ts @@ -0,0 +1,106 @@ +/** + * A2UI `updateDataModel` paths come from agent and MCP input: reserved keys are refused at every + * level with an `InteropError`, and nothing reaches `Object.prototype` (CodeQL + * js/prototype-polluting-assignment, alerts 42–44). + */ +import { afterEach, describe, expect, it } from "vitest"; +import { A2UI_BASIC_CATALOG, fromA2ui, InteropError } from "../src/surface-interop.ts"; +import { checkSurface, createCatalog } from "../src/surfaces.ts"; + +const catalog = createCatalog({ name: "blog", major: 1 }); +const v = "v0.9"; + +const withData = (path: string, value: unknown) => [ + { version: v, createSurface: { surfaceId: "s", catalogId: A2UI_BASIC_CATALOG } }, + { + version: v, + updateComponents: { + surfaceId: "s", + components: [{ id: "root", component: "Text", text: { path: "/title" } }], + }, + }, + { version: v, updateDataModel: { surfaceId: "s", path, value } }, +]; + +const convert = (path: string, value: unknown = { polluted: true }) => + fromA2ui(withData(path, value), { sectionId: "sec_pollute001", catalog: catalog.id }); + +const POINTERS = [ + "/__proto__/x", + "/__proto__", + "/constructor/prototype/x", + "/a/__proto__/x", + "/a/b/constructor/prototype/x", + "/list/0/__proto__/x", + "/prototype/x", + // `~1` decodes to `/`, `~0` to `~`: the decoded segment is what is checked + "/a~1b/__proto__/x", +]; + +afterEach(() => { + // Belt and braces: never leave a polluted prototype behind for other tests. + for (const k of ["x", "polluted"]) delete (Object.prototype as Record)[k]; +}); + +describe("A2UI data paths with reserved keys", () => { + it.each(POINTERS)("refuses %s with an error and leaves Object.prototype alone", (pointer) => { + expect(() => convert(pointer)).toThrow(InteropError); + expect(() => convert(pointer)).toThrow(/reserved key/); + expect(({} as Record).x).toBeUndefined(); + expect(({} as Record).polluted).toBeUndefined(); + expect(Object.getPrototypeOf({})).toBe(Object.prototype); + }); + + it("refuses a delete (no value) through a reserved key too", () => { + expect(() => convert("/__proto__/hasOwnProperty", undefined)).toThrow(InteropError); + expect(typeof Object.prototype.hasOwnProperty).toBe("function"); + }); + + it("refuses a non-index step into an array", () => { + const messages = [ + ...withData("/list", ["a"]), + { version: v, updateDataModel: { surfaceId: "s", path: "/list/length", value: 0 } }, + ]; + expect(() => fromA2ui(messages, { sectionId: "sec_pollute001", catalog: catalog.id })).toThrow( + /not an array index/, + ); + }); + + it("still writes ordinary paths, inherited names included, as own data", () => { + const r = fromA2ui( + [ + ...withData("/title", "Hello"), + { version: v, updateDataModel: { surfaceId: "s", path: "/toString/y", value: 1 } }, + { version: v, updateDataModel: { surfaceId: "s", path: "/list/1/name", value: "Pro" } }, + ], + { sectionId: "sec_pollute001", catalog: catalog.id }, + ); + expect(r.surface.data).toEqual({ + title: "Hello", + toString: { y: 1 }, + list: [undefined, { name: "Pro" }], + }); + expect(Object.hasOwn(r.surface.data as object, "toString")).toBe(true); + expect(checkSurface(r.surface, catalog).issues).toEqual([]); + }); +}); + +describe("surface pointers with reserved keys", () => { + it.each(["/__proto__/x", "/constructor/prototype/x", "/a/__proto__"])( + "checkSurface reports %s", + (path) => { + const surface = { + id: "sec_pollute002", + kind: "surface" as const, + catalog: catalog.id, + data: {}, + components: [ + { id: "root", component: "TextField", props: { label: "N", value: { path } } }, + ], + }; + expect(checkSurface(surface as never, catalog).issues).toContainEqual( + expect.objectContaining({ message: expect.stringMatching(/reserved keys/) }), + ); + }, + ); +}); diff --git a/packages/core-ts/test/linear-scans.test.ts b/packages/core-ts/test/linear-scans.test.ts new file mode 100644 index 00000000..3ee815bf --- /dev/null +++ b/packages/core-ts/test/linear-scans.test.ts @@ -0,0 +1,235 @@ +/** + * core-ts's linear scanners (AGENTS.md rule 13, CodeQL alerts 20, 23, 24, 30, 35, 36): each gives + * the regex it replaced's answers on ordinary input and stays fast on 100k adversarial ones. + */ +import { describe, expect, it } from "vitest"; +import { normalizeHostname } from "../src/domains.ts"; +import { escapeMarkdown, markdownToPlain } from "../src/fields/conversions.ts"; +import { isSvgText, sniff } from "../src/media/sniff.ts"; +import { bodySurfaces, closesFence, fenceOpen } from "../src/surfaces/body.ts"; + +const N = 100_000; +function fast(f: () => T): T { + const start = performance.now(); + const out = f(); + expect(performance.now() - start).toBeLessThan(1500); + return out; +} + +describe("isSvgText (alerts 24, 35, 36)", () => { + const old = (text: string) => + /^\s*(?:<\?xml[^>]*\?>\s*)?(?:\s*)*(?:]*>\s*)?(?:\s*)*]/i.test( + text.replace(/^/, ""), + ); + it("answers as the regex did on real headers", () => { + for (const t of [ + '', + "", + ' \n', + '\n', + " \n", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + "", + ]) + expect(isSvgText(t), JSON.stringify(t)).toBe(old(t)); + }); + it("ends a comment at its first --> (the one intended difference: never more svg)", () => { + const t = " x --> "; + expect(old(t)).toBe(true); + expect(isSvgText(t)).toBe(false); + }); + it("is linear on the exponential inputs", () => { + expect(fast(() => isSvgText(`".repeat(N / 7)}`))).toBe(false); + expect(fast(() => isSvgText(`${"".repeat(N / 7)}`))).toBe(true); + const bytes = new TextEncoder().encode(``, and an article's leading `#` needs its space on the same line. +- **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 `-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: `<script>` in markdown is the text ` ">'; +const ORIGIN = "https://haber.example"; + +/** Every element and comment under `node` (its own tag included when it is one). */ +function markup(node: Node, out: string[] = []): string[] { + if (node.nodeName === "#comment") out.push("#comment"); + else if ("tagName" in node) out.push(node.tagName); + for (const child of "childNodes" in node ? node.childNodes : []) markup(child as Node, out); + if ("content" in node && node.content) markup(node.content as Node, out); + return out; +} + +/** The first `tag` element under `node`, depth first. */ +function firstElement(node: Node, tag: string): DefaultTreeAdapterMap["element"] | undefined { + if ("tagName" in node && node.tagName === tag) return node; + for (const child of "childNodes" in node ? node.childNodes : []) { + const found = firstElement(child as Node, tag); + if (found) return found; + } + return undefined; +} + +/** The elements and comments an HTML fragment parses to. */ +const fragmentMarkup = (html: string) => markup(parseFragment(html) as unknown as Node); + +/** The elements and comments inside `` when `html` is all the head holds. */ +function headMarkup(html: string): string[] { + const doc = parse(`${html}`); + const head = doc.childNodes + .flatMap((n) => ("childNodes" in n ? n.childNodes : [])) + .find((n) => n.nodeName === "head") as Node; + return markup(head).slice(1); +} + +const site = { + id: "site_aaaaaaaaaaaa", + name: "Haber", + slug: "haber", + defaultLocale: "en", + locales: ["en"], + domains: [], +} as unknown as Site; + +const archive = { + locale: "en", + page: 1, + path: "/category/news", + root: "/category/news", + shown: { + name: "News", + slug: "news", + href: "/category/news", + taxonomy: { key: "category", labels: { en: { plural: "Categories" } } }, + }, + trail: [], + total: 0, + perPage: 10, + term: { labels: { en: { name: "News", description: `Intro ${ATTACK}` } } }, + taxonomy: { key: "category" }, + entries: [], +} as never; + +describe("htmlText's sinks with decoded markup in the text (alert 41)", () => { + const head = archiveHead(site, archive, ORIGIN); + + it("the description is the decoded text, verbatim", () => { + expect(markdownText(`Intro ${ATTACK}`)).toBe(`Intro ${DECODED}`); + expect(head.description).toBe(`Intro ${DECODED}`); + }); + + it("meta description, og: and twitter: tags: an attribute value, never markup", () => { + // What SeoHead.astro writes for `meta.description`, `meta.openGraph` and `meta.twitter`. + const tags = [ + ``, + ...head.openGraph.map( + ([p, c]) => ``, + ), + ...head.twitter.map( + ([n, c]) => ``, + ), + ]; + expect(head.openGraph).toContainEqual(["og:description", `Intro ${DECODED}`]); + for (const tag of tags) expect(headMarkup(tag), tag).toEqual(["meta"]); + const meta = firstElement(parse(`${tags[0]}`) as unknown as Node, "meta"); + expect(meta?.attrs.find((a) => a.name === "content")?.value).toBe(`Intro ${DECODED}`); + }); + + it("JSON-LD: no ,