From 8adb60f7f74ff77a459ff46ea439b9110b4f3319 Mon Sep 17 00:00:00 2001 From: Simon Davies Date: Thu, 7 May 2026 19:47:14 +0100 Subject: [PATCH 1/2] fix: validate hex colours in hexColor() to prevent XML injection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hexColor() previously did no validation — just stripped '#' and uppercased. If a non-hex string (like gradient XML) was passed, it was silently embedded as an srgbClr val attribute, producing corrupt OOXML that PowerPoint would repair by stripping entire slides. Bug reproduction: LLM passes gradient XML or background XML as a 'color' parameter → hexColor uppercases it → solidFill embeds it as → PowerPoint repair strips the slide. Fix: hexColor() now validates input against /^#?[0-9A-Fa-f]{6}$/ and throws a descriptive error on invalid input, matching the same regex used by requireHex(). Error message truncates long strings to avoid dumping XML fragments into the console. Signed-off-by: Simon Davies --- builtin-modules/doc-core.json | 4 ++-- builtin-modules/src/doc-core.ts | 15 +++++++++++++-- builtin-modules/src/types/ha-modules.d.ts | 9 +++++++-- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/builtin-modules/doc-core.json b/builtin-modules/doc-core.json index 1e78973..cc974ef 100644 --- a/builtin-modules/doc-core.json +++ b/builtin-modules/doc-core.json @@ -3,8 +3,8 @@ "description": "Format-agnostic document infrastructure — themes, colour validation, contrast utilities, input guards. Used by ha:pptx, ha:pdf, and other format modules.", "author": "system", "mutable": false, - "sourceHash": "sha256:b9119a600839812d", - "dtsHash": "sha256:1f311b99f56fdcbb", + "sourceHash": "sha256:f17eeec053c65a77", + "dtsHash": "sha256:4bbfced3ab780ce8", "importStyle": "named", "hints": { "overview": "Shared utilities for all document formats. Provides themes, colour validation (WCAG AA contrast), and input guards. You rarely import this directly — ha:pptx and ha:pdf re-use it internally.", diff --git a/builtin-modules/src/doc-core.ts b/builtin-modules/src/doc-core.ts index 0998f5e..db542aa 100644 --- a/builtin-modules/src/doc-core.ts +++ b/builtin-modules/src/doc-core.ts @@ -15,13 +15,24 @@ /** * Convert a hex colour string to normalised format (strip leading #, uppercase). * This is the **lenient** version — it does NOT throw on bad input. - * Prefer `requireHex()` at public API boundaries; this is kept for - * internal paths where the value has already been validated. + * Normalise and validate a hex colour string. + * + * Throws on invalid input (non-hex strings, XML fragments, named + * colours, rgb() notation, etc.) to prevent malformed OOXML output. + * Prefer `requireHex()` at public API boundaries for more descriptive + * error messages with parameter names. * * @param hex - Colour like "#2196F3" or "2196F3" * @returns Normalised colour like "2196F3" + * @throws {Error} If hex is not a valid 6-character hex colour */ export function hexColor(hex: string): string { + if (typeof hex !== "string" || !/^#?[0-9A-Fa-f]{6}$/.test(hex)) { + throw new Error( + `Invalid hex colour: "${typeof hex === "string" && hex.length > 30 ? hex.slice(0, 30) + "..." : hex}". ` + + `Expected a 6-character hex string like "2196F3" or "#FF9800".`, + ); + } return hex.replace(/^#/, "").toUpperCase(); } diff --git a/builtin-modules/src/types/ha-modules.d.ts b/builtin-modules/src/types/ha-modules.d.ts index fe9538e..cbec17c 100644 --- a/builtin-modules/src/types/ha-modules.d.ts +++ b/builtin-modules/src/types/ha-modules.d.ts @@ -45,11 +45,16 @@ declare module "ha:doc-core" { /** * Convert a hex colour string to normalised format (strip leading #, uppercase). * This is the **lenient** version — it does NOT throw on bad input. - * Prefer `requireHex()` at public API boundaries; this is kept for - * internal paths where the value has already been validated. + * Normalise and validate a hex colour string. + * + * Throws on invalid input (non-hex strings, XML fragments, named + * colours, rgb() notation, etc.) to prevent malformed OOXML output. + * Prefer `requireHex()` at public API boundaries for more descriptive + * error messages with parameter names. * * @param hex - Colour like "#2196F3" or "2196F3" * @returns Normalised colour like "2196F3" + * @throws {Error} If hex is not a valid 6-character hex colour */ export declare function hexColor(hex: string): string; /** From 52c8649d68bc15d415c9f884137a80fbab602c0f Mon Sep 17 00:00:00 2001 From: Simon Davies Date: Thu, 7 May 2026 20:21:03 +0100 Subject: [PATCH 2/2] fix: address PR #115 review feedback on hexColor validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove stale JSDoc ('lenient', 'does NOT throw') — now accurate - Safely render non-string values in error message using typeof instead of interpolating directly (prevents Symbol/object issues) - Reuse shared HEX_RE constant (moved above hexColor, removed dupe) - Add 8 unit tests: XML fragments, named colours, 3-char shorthand, rgb() notation, empty string, long string truncation, non-string input Signed-off-by: Simon Davies --- builtin-modules/doc-core.json | 4 +-- builtin-modules/src/doc-core.ts | 19 ++++++---- builtin-modules/src/types/ha-modules.d.ts | 2 -- tests/docgen-modules.test.ts | 44 +++++++++++++++++++++++ 4 files changed, 59 insertions(+), 10 deletions(-) diff --git a/builtin-modules/doc-core.json b/builtin-modules/doc-core.json index cc974ef..630c0c6 100644 --- a/builtin-modules/doc-core.json +++ b/builtin-modules/doc-core.json @@ -3,8 +3,8 @@ "description": "Format-agnostic document infrastructure — themes, colour validation, contrast utilities, input guards. Used by ha:pptx, ha:pdf, and other format modules.", "author": "system", "mutable": false, - "sourceHash": "sha256:f17eeec053c65a77", - "dtsHash": "sha256:4bbfced3ab780ce8", + "sourceHash": "sha256:dffdb06f7812466e", + "dtsHash": "sha256:b39c60e35b4359bd", "importStyle": "named", "hints": { "overview": "Shared utilities for all document formats. Provides themes, colour validation (WCAG AA contrast), and input guards. You rarely import this directly — ha:pptx and ha:pdf re-use it internally.", diff --git a/builtin-modules/src/doc-core.ts b/builtin-modules/src/doc-core.ts index db542aa..c791371 100644 --- a/builtin-modules/src/doc-core.ts +++ b/builtin-modules/src/doc-core.ts @@ -12,9 +12,10 @@ // ── Colour Utilities ───────────────────────────────────────────────── +/** Regex for a valid 6-character hex colour (with optional #). */ +const HEX_RE = /^#?[0-9A-Fa-f]{6}$/; + /** - * Convert a hex colour string to normalised format (strip leading #, uppercase). - * This is the **lenient** version — it does NOT throw on bad input. * Normalise and validate a hex colour string. * * Throws on invalid input (non-hex strings, XML fragments, named @@ -27,9 +28,16 @@ * @throws {Error} If hex is not a valid 6-character hex colour */ export function hexColor(hex: string): string { - if (typeof hex !== "string" || !/^#?[0-9A-Fa-f]{6}$/.test(hex)) { + if (typeof hex !== "string" || !HEX_RE.test(hex)) { + // Safely render non-string values without risking Symbol/object toString + const display = + typeof hex === "string" + ? hex.length > 30 + ? hex.slice(0, 30) + "..." + : hex + : `[${typeof hex}]`; throw new Error( - `Invalid hex colour: "${typeof hex === "string" && hex.length > 30 ? hex.slice(0, 30) + "..." : hex}". ` + + `Invalid hex colour: "${display}". ` + `Expected a 6-character hex string like "2196F3" or "#FF9800".`, ); } @@ -325,8 +333,7 @@ export function isDark(hex: string): boolean { // three layers deep. Every error message is LLM-actionable: it tells // the caller WHAT is wrong, WHY, and HOW to fix it. -/** Regex for a valid 6-character hex colour (with optional #). */ -const HEX_RE = /^#?[0-9A-Fa-f]{6}$/; +// HEX_RE is defined at the top of the file alongside hexColor(). /** * Validate and normalise a hex colour string. diff --git a/builtin-modules/src/types/ha-modules.d.ts b/builtin-modules/src/types/ha-modules.d.ts index cbec17c..0d88510 100644 --- a/builtin-modules/src/types/ha-modules.d.ts +++ b/builtin-modules/src/types/ha-modules.d.ts @@ -43,8 +43,6 @@ declare module "ha:crc32" { declare module "ha:doc-core" { /** - * Convert a hex colour string to normalised format (strip leading #, uppercase). - * This is the **lenient** version — it does NOT throw on bad input. * Normalise and validate a hex colour string. * * Throws on invalid input (non-hex strings, XML fragments, named diff --git a/tests/docgen-modules.test.ts b/tests/docgen-modules.test.ts index 41617f8..6585030 100644 --- a/tests/docgen-modules.test.ts +++ b/tests/docgen-modules.test.ts @@ -51,6 +51,50 @@ describe("ooxml-core", () => { it("should handle already-clean input", () => { expect(core.hexColor("ABCDEF")).toBe("ABCDEF"); }); + + it("should throw on XML fragment input", () => { + expect(() => + core.hexColor( + '', + ), + ).toThrow("Invalid hex colour"); + }); + + it("should throw on named colour", () => { + expect(() => core.hexColor("red")).toThrow("Invalid hex colour"); + }); + + it("should throw on 3-char shorthand", () => { + expect(() => core.hexColor("FFF")).toThrow("Invalid hex colour"); + }); + + it("should throw on rgb() notation", () => { + expect(() => core.hexColor("rgb(255,0,0)")).toThrow("Invalid hex colour"); + }); + + it("should throw on empty string", () => { + expect(() => core.hexColor("")).toThrow("Invalid hex colour"); + }); + + it("should truncate long strings in error message", () => { + const longXml = "" + "x".repeat(100); + try { + core.hexColor(longXml); + } catch (e: unknown) { + const msg = (e as Error).message; + expect(msg).toContain("..."); + expect(msg.length).toBeLessThan(200); + } + }); + + it("should safely handle non-string input", () => { + expect(() => core.hexColor(42 as unknown as string)).toThrow( + "Invalid hex colour", + ); + expect(() => core.hexColor(null as unknown as string)).toThrow( + "Invalid hex colour", + ); + }); }); describe("themes", () => {