Skip to content

Commit ac576c0

Browse files
authored
fix: validate hex colours in hexColor() to prevent XML injection (#115)
* fix: validate hex colours in hexColor() to prevent XML injection 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 <a:srgbClr val='<P:BG>...'/> → 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 <simongdavies@users.noreply.github.com> * fix: address PR #115 review feedback on hexColor validation - 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 <simongdavies@users.noreply.github.com> --------- Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>
1 parent 7ef5d60 commit ac576c0

4 files changed

Lines changed: 77 additions & 12 deletions

File tree

builtin-modules/doc-core.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
"description": "Format-agnostic document infrastructure — themes, colour validation, contrast utilities, input guards. Used by ha:pptx, ha:pdf, and other format modules.",
44
"author": "system",
55
"mutable": false,
6-
"sourceHash": "sha256:b9119a600839812d",
7-
"dtsHash": "sha256:1f311b99f56fdcbb",
6+
"sourceHash": "sha256:dffdb06f7812466e",
7+
"dtsHash": "sha256:b39c60e35b4359bd",
88
"importStyle": "named",
99
"hints": {
1010
"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.",

builtin-modules/src/doc-core.ts

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,35 @@
1212

1313
// ── Colour Utilities ─────────────────────────────────────────────────
1414

15+
/** Regex for a valid 6-character hex colour (with optional #). */
16+
const HEX_RE = /^#?[0-9A-Fa-f]{6}$/;
17+
1518
/**
16-
* Convert a hex colour string to normalised format (strip leading #, uppercase).
17-
* This is the **lenient** version — it does NOT throw on bad input.
18-
* Prefer `requireHex()` at public API boundaries; this is kept for
19-
* internal paths where the value has already been validated.
19+
* Normalise and validate a hex colour string.
20+
*
21+
* Throws on invalid input (non-hex strings, XML fragments, named
22+
* colours, rgb() notation, etc.) to prevent malformed OOXML output.
23+
* Prefer `requireHex()` at public API boundaries for more descriptive
24+
* error messages with parameter names.
2025
*
2126
* @param hex - Colour like "#2196F3" or "2196F3"
2227
* @returns Normalised colour like "2196F3"
28+
* @throws {Error} If hex is not a valid 6-character hex colour
2329
*/
2430
export function hexColor(hex: string): string {
31+
if (typeof hex !== "string" || !HEX_RE.test(hex)) {
32+
// Safely render non-string values without risking Symbol/object toString
33+
const display =
34+
typeof hex === "string"
35+
? hex.length > 30
36+
? hex.slice(0, 30) + "..."
37+
: hex
38+
: `[${typeof hex}]`;
39+
throw new Error(
40+
`Invalid hex colour: "${display}". ` +
41+
`Expected a 6-character hex string like "2196F3" or "#FF9800".`,
42+
);
43+
}
2544
return hex.replace(/^#/, "").toUpperCase();
2645
}
2746

@@ -314,8 +333,7 @@ export function isDark(hex: string): boolean {
314333
// three layers deep. Every error message is LLM-actionable: it tells
315334
// the caller WHAT is wrong, WHY, and HOW to fix it.
316335

317-
/** Regex for a valid 6-character hex colour (with optional #). */
318-
const HEX_RE = /^#?[0-9A-Fa-f]{6}$/;
336+
// HEX_RE is defined at the top of the file alongside hexColor().
319337

320338
/**
321339
* Validate and normalise a hex colour string.

builtin-modules/src/types/ha-modules.d.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,16 @@ declare module "ha:crc32" {
4343

4444
declare module "ha:doc-core" {
4545
/**
46-
* Convert a hex colour string to normalised format (strip leading #, uppercase).
47-
* This is the **lenient** version — it does NOT throw on bad input.
48-
* Prefer `requireHex()` at public API boundaries; this is kept for
49-
* internal paths where the value has already been validated.
46+
* Normalise and validate a hex colour string.
47+
*
48+
* Throws on invalid input (non-hex strings, XML fragments, named
49+
* colours, rgb() notation, etc.) to prevent malformed OOXML output.
50+
* Prefer `requireHex()` at public API boundaries for more descriptive
51+
* error messages with parameter names.
5052
*
5153
* @param hex - Colour like "#2196F3" or "2196F3"
5254
* @returns Normalised colour like "2196F3"
55+
* @throws {Error} If hex is not a valid 6-character hex colour
5356
*/
5457
export declare function hexColor(hex: string): string;
5558
/**

tests/docgen-modules.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,50 @@ describe("ooxml-core", () => {
5151
it("should handle already-clean input", () => {
5252
expect(core.hexColor("ABCDEF")).toBe("ABCDEF");
5353
});
54+
55+
it("should throw on XML fragment input", () => {
56+
expect(() =>
57+
core.hexColor(
58+
'<p:bg><p:bgPr><a:solidFill><a:srgbClr val="FF0000"/></a:solidFill></p:bgPr></p:bg>',
59+
),
60+
).toThrow("Invalid hex colour");
61+
});
62+
63+
it("should throw on named colour", () => {
64+
expect(() => core.hexColor("red")).toThrow("Invalid hex colour");
65+
});
66+
67+
it("should throw on 3-char shorthand", () => {
68+
expect(() => core.hexColor("FFF")).toThrow("Invalid hex colour");
69+
});
70+
71+
it("should throw on rgb() notation", () => {
72+
expect(() => core.hexColor("rgb(255,0,0)")).toThrow("Invalid hex colour");
73+
});
74+
75+
it("should throw on empty string", () => {
76+
expect(() => core.hexColor("")).toThrow("Invalid hex colour");
77+
});
78+
79+
it("should truncate long strings in error message", () => {
80+
const longXml = "<a:gradFill>" + "x".repeat(100);
81+
try {
82+
core.hexColor(longXml);
83+
} catch (e: unknown) {
84+
const msg = (e as Error).message;
85+
expect(msg).toContain("...");
86+
expect(msg.length).toBeLessThan(200);
87+
}
88+
});
89+
90+
it("should safely handle non-string input", () => {
91+
expect(() => core.hexColor(42 as unknown as string)).toThrow(
92+
"Invalid hex colour",
93+
);
94+
expect(() => core.hexColor(null as unknown as string)).toThrow(
95+
"Invalid hex colour",
96+
);
97+
});
5498
});
5599

56100
describe("themes", () => {

0 commit comments

Comments
 (0)