From b2339b235e58103a6673063fd6accf0713383678 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 16:22:53 +0000 Subject: [PATCH 1/2] fix(security): refuse raw HTML and unsafe URL schemes, symlinked entries and off-site redirects A security review of the scripts and workflows found four weak spots; this closes all of them. Raw HTML and script-scheme URLs shipped in the signed database. The build rendered description/details/specs with marked and no sanitizer, so ")).toBe( + "<script>alert(2)</script>" + ); + expect(markdownToHtml("[click](javascript:alert(3))")).toBe("

click

"); + expect(markdownToHtml("![alt](javascript:alert(4))")).toBe("

alt

"); + expect(markdownToHtml("[site](https://x.example/) and [mail](mailto:a@x.example)")).toBe( + '

site and mail

' + ); + }); + it("normalizeCategory maps an alias to its canonical value and passes the rest through", () => { expect(normalizeCategory("eq")).toBe("equalizer"); expect(normalizeCategory("equalizer")).toBe("equalizer"); diff --git a/scripts/__tests__/utils.test.ts b/scripts/__tests__/utils.test.ts index 3a883247f..48a664ae2 100644 --- a/scripts/__tests__/utils.test.ts +++ b/scripts/__tests__/utils.test.ts @@ -1,5 +1,52 @@ -import { describe, expect, it } from "vitest"; -import { escapeSQL, findClosestMatch, formatValidOptions, parseErrorPath } from "../lib/utils.js"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { + escapeSQL, + findClosestMatch, + formatValidOptions, + loadYamlFile, + parseErrorPath, + readRegularFile, +} from "../lib/utils.js"; + +describe("readRegularFile", () => { + const dirs: string[] = []; + afterEach(() => { + for (const dir of dirs.splice(0)) fs.rmSync(dir, { recursive: true, force: true }); + }); + const tmp = (): string => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "catalog-utils-")); + dirs.push(dir); + return dir; + }; + + it("reads a regular file", () => { + const dir = tmp(); + const file = path.join(dir, "a.yaml"); + fs.writeFileSync(file, "name: A\n"); + expect(readRegularFile(file)).toBe("name: A\n"); + expect(loadYamlFile<{ name: string }>(file)).toEqual({ name: "A" }); + }); + + it("refuses a symlink rather than reading through it", () => { + // A committed `data/software/x.yaml -> /dev/zero` was an unbounded + // read in validate and build; the link is refused as a link. + const dir = tmp(); + const target = path.join(dir, "real.yaml"); + fs.writeFileSync(target, "name: A\n"); + const link = path.join(dir, "link.yaml"); + fs.symlinkSync(target, link); + expect(() => readRegularFile(link)).toThrow(/symbolic link/); + expect(() => loadYamlFile(link)).toThrow(/symbolic link/); + }); + + it("refuses something that is not a regular file", () => { + const dir = tmp(); + expect(() => readRegularFile(dir)).toThrow(/not a regular file/); + }); +}); describe("findClosestMatch", () => { it("returns exact match when found", () => { diff --git a/scripts/__tests__/validate.test.ts b/scripts/__tests__/validate.test.ts index 7ce97870a..fa284406e 100644 --- a/scripts/__tests__/validate.test.ts +++ b/scripts/__tests__/validate.test.ts @@ -70,6 +70,34 @@ describe("getErrorCodeFromZodIssue", () => { ).toBe(ValidationErrorCode.E127_INVALID_PRICE_AMOUNT); }); + it("classifies an unsafe scheme as E128 before the generic url branch", () => { + expect( + getErrorCodeFromZodIssue( + issue({ message: "Unsafe URL scheme 'javascript:': only http and https are allowed." }) + ) + ).toBe(ValidationErrorCode.E128_UNSAFE_URL_SCHEME); + expect( + getErrorCodeFromZodIssue( + issue({ + message: + "Unsafe URL scheme in prose link 'data:x': only http, https and mailto links render.", + path: ["details"], + }) + ) + ).toBe(ValidationErrorCode.E128_UNSAFE_URL_SCHEME); + }); + + it("classifies raw html as E305 before the markdown branch", () => { + expect( + getErrorCodeFromZodIssue( + issue({ + message: + "Raw HTML in prose: '\n` + ); + expect(validateFile(block, COLLECTION_SCHEMAS.hardware, MAKERS)?.details?.[0]).toMatchObject({ + code: ValidationErrorCode.E305_RAW_HTML_IN_PROSE, + path: "details", + }); + + const link = writeEntry("hardware", `${HARDWARE_OK}specs: "- [Click](javascript:alert(3))"\n`); + expect(validateFile(link, COLLECTION_SCHEMAS.hardware, MAKERS)?.details?.[0]).toMatchObject({ + code: ValidationErrorCode.E128_UNSAFE_URL_SCHEME, + path: "specs", + }); + }); + + it("reads angle brackets the lexer takes as text as text, not html", () => { + // In the corpus: a tracklist entry written as . + const file = writeEntry( + "hardware", + `${HARDWARE_OK}details: |-\n Tracks include and a [page](https://x.example).\n` + ); + expect(validateFile(file, COLLECTION_SCHEMAS.hardware, MAKERS)).toBeNull(); + }); + + it("refuses to read an entry through a symlink", () => { + const real = writeEntry("hardware", HARDWARE_OK); + const link = path.join(path.dirname(real), "linked.yaml"); + fs.symlinkSync(real, link); + const result = validateFile(link, COLLECTION_SCHEMAS.hardware, MAKERS); + expect(result?.errors[0]).toMatch(/symbolic link/); + }); + it("keeps a wrong-typed price amount on E101, not E127", () => { // The error map fires only when the input really was a number, so a // string keeps Zod's own wrong-type message and its existing code. diff --git a/scripts/build-sqlite.ts b/scripts/build-sqlite.ts index a841cbf4a..acb571788 100644 --- a/scripts/build-sqlite.ts +++ b/scripts/build-sqlite.ts @@ -10,8 +10,8 @@ import fs from "node:fs"; import path from "node:path"; import { pathToFileURL } from "node:url"; import Database from "better-sqlite3"; -import { marked } from "marked"; - +import { Marked, type Tokens } from "marked"; +import { escapeHtml, isSafeHref } from "./lib/prose-markup.js"; import type { Accessory, CategoryAliasesSchema, @@ -31,10 +31,38 @@ import { slugify, } from "./lib/utils.js"; -// Configure marked for safe HTML output -marked.setOptions({ +/** + * The renderer behind every `*_html` column. Its own `Marked` instance, + * so the overrides never reach the global `marked` that `validate.ts` + * lexes with. + * + * marked passes raw HTML through by design, and Studio shows these + * columns as HTML, so until this renderer existed a `