feat(pptx): add ShapeFragment safety system and validation engine

- Introduce ShapeFragment branded type to prevent raw XML injection
- addBody() now validates ShapeFragment, rejects raw strings
- Hide _createShapeFragment from LLM discovery (underscore prefix)
- Add validation engine: relationship caps, NaN/Infinity detection,
  cross-slide duplicate shape ID checks, notes sanitization
- Chart complexity caps: 50 charts/deck, 24 series, 100 categories
- Deduplicate MAX constants (single source in pptx-charts.ts)
- Update hints/prompts and SKILL.md with ShapeFragment API docs
- Add 59 safety tests covering all shape builders and edge cases

Signed-off-by: Simon Davies <simongdavies@users.noreply.github.com>

Author: simongdavies

builtin-modules/ooxml-core.json

@@ -3,8 +3,8 @@
   "description": "Shared OOXML infrastructure - units, colors, themes, Content_Types, relationships",
   "author": "system",
   "mutable": false,
-  "sourceHash": "sha256:24c8441a3504052f",
-  "dtsHash": "sha256:9f88e7c59a56854c",
+  "sourceHash": "sha256:00cfaa1e652856b2",
+  "dtsHash": "sha256:6aac85502082bf89",
   "importStyle": "named",
   "hints": {
     "overview": "Low-level OOXML infrastructure. Most users should use ha:pptx instead.",

builtin-modules/pptx-charts.json

@@ -3,8 +3,8 @@
   "description": "OOXML DrawingML chart generation - bar, pie, line charts for PPTX presentations",
   "author": "system",
   "mutable": false,
-  "sourceHash": "sha256:029765ed53b96536",
-  "dtsHash": "sha256:5f653830226c3554",
+  "sourceHash": "sha256:27d40e5c5095ec38",
+  "dtsHash": "sha256:4353b8263dc99405",
   "importStyle": "named",
   "hints": {
     "overview": "Chart generation for PPTX. Always used with ha:pptx.",
@@ -16,7 +16,10 @@
       "Use chartSlide(pres, {title, chart}) for simple full-slide charts",
       "Chart values must be finite numbers — not null, undefined, or strings",
       "pieChart: labels[] and values[] must have the same length",
-      "barChart/lineChart: each series.values[] length must equal categories[] length"
+      "barChart/lineChart: each series.values[] length must equal categories[] length",
+      "Max 50 charts per deck, 24 series per chart, 100 categories per chart",
+      "embedChart returns {shape, ...} — pass result.shape to customSlide, NOT result directly",
+      "Do NOT call .toString() on chart results — it throws. Use .shape property."
     ],
     "antiPatterns": [
       "Don't import chart functions from ha:pptx — they're in this module"

builtin-modules/pptx-tables.json

@@ -3,14 +3,17 @@
   "description": "Styled tables for PPTX presentations - headers, borders, alternating rows",
   "author": "system",
   "mutable": false,
-  "sourceHash": "sha256:399b5349b1c8c187",
-  "dtsHash": "sha256:82d903ffbf4dfb1e",
+  "sourceHash": "sha256:5940fb396a67f801",
+  "dtsHash": "sha256:3ba75bbc44353467",
   "importStyle": "named",
   "hints": {
     "overview": "Table generation for PPTX. Always used with ha:pptx.",
-    "relatedModules": ["ha:pptx"],
+    "relatedModules": [
+      "ha:pptx"
+    ],
     "criticalRules": [
-      "comparisonTable: options array must not be empty, each option needs {name, values}"
+      "comparisonTable: options array must not be empty, each option needs {name, values}",
+      "All table functions return ShapeFragment — pass directly to customSlide shapes array"
     ],
     "commonPatterns": [
       "table({x, y, w, headers, rows, style}) for data tables",

builtin-modules/pptx.json

@@ -3,8 +3,8 @@
   "description": "PowerPoint PPTX presentation builder - slides, text, shapes, themes, layouts",
   "author": "system",
   "mutable": false,
-  "sourceHash": "sha256:a13871a41506a523",
-  "dtsHash": "sha256:2107e369816b4bd5",
+  "sourceHash": "sha256:895d7188d5ba8b46",
+  "dtsHash": "sha256:27520514e4401465",
   "importStyle": "named",
   "hints": {
     "overview": "Core PPTX slide building. Charts in ha:pptx-charts, tables in ha:pptx-tables.",
@@ -27,13 +27,18 @@
       "ALL slide functions need pres as FIRST parameter: titleSlide(pres, opts)",
       "Charts are NOT in this module — import from ha:pptx-charts",
       "Tables are NOT in this module — import from ha:pptx-tables",
+      "Shape builders return ShapeFragment objects — NEVER construct raw XML strings",
+      "customSlide shapes must be ShapeFragment or ShapeFragment[] — raw strings are rejected",
       "Use getThemeNames() to see valid themes",
       "DARK THEMES auto-handle contrast — don't use forceColor",
-      "Don't specify text color — theme auto-selects readable colours"
+      "Don't specify text color — theme auto-selects readable colours",
+      "Speaker notes are plain text only — max 12,000 chars, auto-sanitized"
     ],
     "antiPatterns": [
       "Don't store pres object in shared-state — use pres.serialize()",
-      "Don't write raw OOXML XML — use module functions",
+      "Don't write raw OOXML XML — use module shape builder functions",
+      "Don't concatenate ShapeFragment objects with + — pass as arrays",
+      "Don't call .toString() on chart results — use .shape property",
       "Don't guess function names — call module_info first",
       "series.name is REQUIRED for all chart data series"
     ],

builtin-modules/src/ooxml-core.ts

@@ -652,6 +652,93 @@ export function isDark(hex: string): boolean {
   return luminance(hex) < 0.5;
 }
 
+// ── ShapeFragment (Opaque Branded Type) ──────────────────────────────
+// All shape builders (textBox, rect, table, etc.) return ShapeFragment.
+// Only code that holds the private SHAPE_BRAND symbol can forge one.
+// This prevents LLMs from injecting arbitrary XML strings into slides.
+//
+// SECURITY MODEL:
+// The sandbox architecture shares all ha:* module exports at runtime.
+// We cannot make _createShapeFragment truly unexportable for cross-module
+// use (pptx.ts, pptx-charts.ts, pptx-tables.ts all need it).
+// Defence layers:
+//   1. Underscore prefix → excluded from module_info / hints by convention
+//   2. Filtered from ha-modules.d.ts → invisible to LLM type discovery
+//      (generate-ha-modules-dts.ts skips _-prefixed exports)
+//   3. SKILL.md documents only builder functions, not the factory
+//   4. Code-validator + sandbox provide the hard security boundary
+// The threat model is LLM hallucinations, not adversarial humans.
+
+/** Private brand symbol — never exported by module boundary. */
+const SHAPE_BRAND: unique symbol = Symbol("ShapeFragment");
+
+/**
+ * Opaque shape fragment produced by official shape builders.
+ * Cannot be constructed from raw strings by LLM code.
+ *
+ * Internal code can read `._xml`; external (LLM) code treats this as opaque.
+ */
+export interface ShapeFragment {
+  /** @internal Raw OOXML XML for this shape element. */
+  readonly _xml: string;
+  /** Returns the internal XML (for string concatenation in internal code). */
+  toString(): string;
+}
+
+/**
+ * Create a branded ShapeFragment wrapping validated XML.
+ * Called internally by shape builder functions (textBox, rect, table, etc.).
+ * Underscore-prefixed to signal internal-only — LLMs should use builder
+ * functions (textBox, rect, etc.) not this directly.
+ * @internal
+ */
+export function _createShapeFragment(xml: string): ShapeFragment {
+  const obj = {
+    _xml: xml,
+    toString(): string {
+      return xml;
+    },
+  } as ShapeFragment;
+  // Brand the object with the private symbol (runtime check)
+  (obj as unknown as Record<symbol, boolean>)[SHAPE_BRAND] = true;
+  return Object.freeze(obj);
+}
+
+/**
+ * Check whether a value is a genuine ShapeFragment from a builder function.
+ * Uses the private symbol brand — cannot be forged by LLM code.
+ */
+export function isShapeFragment(x: unknown): x is ShapeFragment {
+  return (
+    x != null &&
+    typeof x === "object" &&
+    (x as Record<symbol, unknown>)[SHAPE_BRAND] === true
+  );
+}
+
+/**
+ * Convert an array of ShapeFragments to a single XML string.
+ * Validates that every element is a genuine branded ShapeFragment.
+ * @throws If any element is not a ShapeFragment
+ */
+export function fragmentsToXml(
+  fragments: ShapeFragment | ShapeFragment[],
+): string {
+  const arr = Array.isArray(fragments) ? fragments : [fragments];
+  const parts: string[] = [];
+  for (let i = 0; i < arr.length; i++) {
+    const f = arr[i];
+    if (!isShapeFragment(f)) {
+      throw new Error(
+        `shapes[${i}]: expected a ShapeFragment from textBox/rect/table/bulletList/etc, ` +
+          `but got ${typeof f}. Do NOT pass raw XML strings — use the shape builder functions.`,
+      );
+    }
+    parts.push(f._xml);
+  }
+  return parts.join("");
+}
+
 // ── Shape ID Counter ─────────────────────────────────────────────────
 // OOXML requires each shape in a presentation to have a unique positive integer ID.
 // PowerPoint will show a "found a problem with content" error if multiple

builtin-modules/src/pptx-charts.ts

@@ -16,9 +16,23 @@ import {
   requireArray,
   requireNumber,
   nextShapeId,
+  _createShapeFragment,
+  type ShapeFragment,
 } from "ha:ooxml-core";
 import { escapeXml } from "ha:xml-escape";
 
+// ── Chart Complexity Caps ────────────────────────────────────────────
+// Hard limits to prevent decks that exhaust PowerPoint's rendering budget.
+
+/** Maximum charts per presentation deck. */
+export const MAX_CHARTS_PER_DECK = 50;
+
+/** Maximum data series per chart (Excel column reference limit B–Y). */
+export const MAX_SERIES_PER_CHART = 24;
+
+/** Maximum categories (X-axis labels) per chart. */
+export const MAX_CATEGORIES_PER_CHART = 100;
+
 // ── Namespace Constants ──────────────────────────────────────────────
 const NS_C = "http://schemas.openxmlformats.org/drawingml/2006/chart";
 const NS_A = "http://schemas.openxmlformats.org/drawingml/2006/main";
@@ -99,7 +113,11 @@ function seriesXml(
   // Series name is REQUIRED — charts with unnamed series produce meaningless legends.
   requireString(series.name, `series[${index}].name`);
   // Series values are REQUIRED and must be a non-empty array of numbers.
-  if (!series.values || !Array.isArray(series.values) || series.values.length === 0) {
+  if (
+    !series.values ||
+    !Array.isArray(series.values) ||
+    series.values.length === 0
+  ) {
     throw new Error(
       `series[${index}].values: array must not be empty. ` +
         `This often happens when fetched data is empty. ` +
@@ -327,6 +345,18 @@ export function barChart(opts: BarChartOptions): ChartResult {
   requireArray(opts.categories || [], "barChart.categories");
   requireArray(opts.series || [], "barChart.series", { nonEmpty: true });
   if (opts.textColor) requireHex(opts.textColor, "barChart.textColor");
+  // Enforce complexity caps
+  if ((opts.categories || []).length > MAX_CATEGORIES_PER_CHART) {
+    throw new Error(
+      `barChart: ${(opts.categories || []).length} categories exceeds the maximum of ${MAX_CATEGORIES_PER_CHART}. ` +
+        `Reduce category count or aggregate data.`,
+    );
+  }
+  if ((opts.series || []).length > MAX_SERIES_PER_CHART) {
+    throw new Error(
+      `barChart: ${(opts.series || []).length} series exceeds the maximum of ${MAX_SERIES_PER_CHART}.`,
+    );
+  }
 
   const dir = opts.horizontal ? "bar" : "col";
   const grouping = opts.stacked ? "stacked" : "clustered";
@@ -345,7 +375,10 @@ ${seriesXmls}
 ${axisXml(1, 2, opts.horizontal ? "l" : "b", true, tc)}
 ${axisXml(2, 1, opts.horizontal ? "b" : "l", false, tc)}`;
 
-  return chartResult("bar", chartXml(plotArea, opts.title, opts.showLegend, tc));
+  return chartResult(
+    "bar",
+    chartXml(plotArea, opts.title, opts.showLegend, tc),
+  );
 }
 
 export interface PieChartOptions {
@@ -420,6 +453,13 @@ export function pieChart(opts: PieChartOptions): ChartResult {
         `has ${values.length}. They must have the same length — one label per slice.`,
     );
   }
+  // Enforce complexity caps
+  if (labels.length > MAX_CATEGORIES_PER_CHART) {
+    throw new Error(
+      `pieChart: ${labels.length} slices exceeds the maximum of ${MAX_CATEGORIES_PER_CHART}. ` +
+        `Group smaller values into an "Other" slice.`,
+    );
+  }
   // Validate each value is a finite number
   values.forEach((v, i) => {
     if (typeof v !== "number" || !Number.isFinite(v)) {
@@ -519,7 +559,10 @@ ${seriesDataLabels}
 ${holeSize}
 </${chartTag}>`;
 
-  return chartResult("pie", chartXml(plotArea, opts.title, effectiveShowLegend, tc));
+  return chartResult(
+    "pie",
+    chartXml(plotArea, opts.title, effectiveShowLegend, tc),
+  );
 }
 
 export interface LineChartOptions {
@@ -566,6 +609,17 @@ export function lineChart(opts: LineChartOptions): ChartResult {
   requireArray(opts.categories || [], "lineChart.categories");
   requireArray(opts.series || [], "lineChart.series", { nonEmpty: true });
   if (opts.textColor) requireHex(opts.textColor, "lineChart.textColor");
+  // Enforce complexity caps
+  if ((opts.categories || []).length > MAX_CATEGORIES_PER_CHART) {
+    throw new Error(
+      `lineChart: ${(opts.categories || []).length} categories exceeds the maximum of ${MAX_CATEGORIES_PER_CHART}.`,
+    );
+  }
+  if ((opts.series || []).length > MAX_SERIES_PER_CHART) {
+    throw new Error(
+      `lineChart: ${(opts.series || []).length} series exceeds the maximum of ${MAX_SERIES_PER_CHART}.`,
+    );
+  }
 
   const chartTag = opts.area ? "c:areaChart" : "c:lineChart";
   const grouping = "standard";
@@ -650,7 +704,10 @@ ${seriesXmls}
 ${axisXml(1, 2, "b", true, tc)}
 ${axisXml(2, 1, "l", false, tc)}`;
 
-  return chartResult(opts.area ? "area" : "line", chartXml(plotArea, opts.title, opts.showLegend, tc));
+  return chartResult(
+    opts.area ? "area" : "line",
+    chartXml(plotArea, opts.title, opts.showLegend, tc),
+  );
 }
 
 export interface ComboChartOptions {
@@ -691,6 +748,12 @@ export interface ComboChartOptions {
 export function comboChart(opts: ComboChartOptions): ChartResult {
   // ── Input validation ──────────────────────────────────────────────
   requireArray(opts.categories || [], "comboChart.categories");
+  // Enforce complexity caps
+  if ((opts.categories || []).length > MAX_CATEGORIES_PER_CHART) {
+    throw new Error(
+      `comboChart: ${(opts.categories || []).length} categories exceeds the maximum of ${MAX_CATEGORIES_PER_CHART}.`,
+    );
+  }
   const barSeries = opts.barSeries || [];
   const lineSeries = opts.lineSeries || [];
   requireArray(barSeries, "comboChart.barSeries");
@@ -770,7 +833,10 @@ ${lineXmls}
 ${axisXml(1, 2, "b", true, tc)}
 ${axisXml(2, 1, "l", false, tc)}`;
 
-  return chartResult("combo", chartXml(plotArea, opts.title, opts.showLegend, tc));
+  return chartResult(
+    "combo",
+    chartXml(plotArea, opts.title, opts.showLegend, tc),
+  );
 }
 
 // ── Chart Embedding into PPTX Slides ─────────────────────────────────
@@ -783,11 +849,14 @@ export interface ChartPosition {
 }
 
 export interface EmbedChartResult {
+  /** ShapeFragment for use in customSlide shapes array. */
+  shape: ShapeFragment;
+  /** @internal Raw shape XML string (kept for internal compatibility). */
   shapeXml: string;
   zipEntries: Array<{ name: string; data: string }>;
   chartRelId: string;
   chartIndex: number;
-  /** Returns shapeXml when converted to string (e.g., in string concatenation). */
+  /** @deprecated Throws error — use .shape instead. */
   toString(): string;
 }
 
@@ -834,6 +903,14 @@ export function embedChart(
         "Pass the object returned by createPresentation().",
     );
   }
+  // Enforce deck-level chart cap
+  const currentChartCount = (pres._charts || []).length;
+  if (currentChartCount >= MAX_CHARTS_PER_DECK) {
+    throw new Error(
+      `embedChart: deck already has ${currentChartCount} charts — max ${MAX_CHARTS_PER_DECK}. ` +
+        `Reduce chart count or split into multiple presentations.`,
+    );
+  }
   if (chart == null || chart.type !== "chart") {
     throw new Error(
       "embedChart: 'chart' must be a chart object from barChart/pieChart/lineChart/comboChart. " +
@@ -908,15 +985,21 @@ export function embedChart(
     pres._chartEntries.push(entry);
   }
 
-  // Return an object that stringifies to shapeXml for easy use in shape concatenation.
-  // This allows: shapes: textBox(...) + embedChart(pres, chart, pos) + rect(...)
-  // Instead of requiring: embedChart(...).shapeXml
+  // Return structured result — use .shape for customSlide arrays.
+  // toString() now THROWS to prevent accidental XML concatenation.
   const result: EmbedChartResult = {
+    shape: _createShapeFragment(shapeXml),
     shapeXml,
     zipEntries,
     chartRelId: relId,
     chartIndex: idx,
-    toString: () => shapeXml,
+    toString(): string {
+      throw new Error(
+        "Cannot concatenate embedChart result directly into shapes. " +
+          "Use the .shape property in your shapes array: " +
+          "customSlide(pres, { shapes: [textBox(...), chart.shape, rect(...)] })",
+      );
+    },
   };
   return result;
 }