engine fix: synch user assets folder better

runtime/engines/reveal.ts

@@ -1,11 +1,117 @@
-import { mkdir, readFile, writeFile } from 'node:fs/promises';
-import { dirname, join } from 'node:path';
+import { mkdir, readFile, writeFile, readdir, cp, lstat, rm } from 'node:fs/promises';
+import type { Dirent } from 'node:fs';
+import { dirname, join, basename, resolve, sep } from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { marked } from 'marked';
 
-const REVEAL_PREFIX = '/vendor/reveal';
+// Relative (no leading slash) so the emitted HTML works both as served
+// from the local review server AND as a standalone file:// open after
+// publish_deck multi-file mode. The dev server's /vendor/reveal/* route
+// still matches because the browser resolves the relative path against
+// the page URL, which is the server root '/'.
+const REVEAL_PREFIX = 'vendor/reveal';
 const __dirname = dirname(fileURLToPath(import.meta.url));
 
+/**
+ * Names at the deck root that are deckmark/agent internals, NOT user assets.
+ * Everything else (images/, fonts/, etc.) gets copied into build/ so the
+ * rendered deck can reference assets via relative paths like
+ * `<img src="images/foo.jpg">` and the local server can serve them.
+ *
+ * `vendor` is excluded so a user-provided `vendor/` folder cannot collide
+ * with — or overwrite — the official reveal.js dist that publish_deck
+ * (multi-file mode) places at `<outDir>/vendor/reveal/`.
+ */
+const EXCLUDED_FROM_ASSET_SYNC = new Set([
+  // The content markdown is excluded dynamically by basename(opts.contentPath)
+  // — it may be named anything (slides.md, talk.md, etc.), and the MCP layer
+  // lets callers override it. 'content.md' stays here as a belt-and-suspenders
+  // default for the common case.
+  'content.md',
+  'deckmark.config.json',
+  'annotations',
+  'build',
+  'published',
+  'vendor',
+  'node_modules',
+  '.git',
+  '.github',
+  '.gitignore',
+  '.claude-plugin',
+  '.mcp.json',
+  'AGENTS.md',
+  'CLAUDE.md',
+  'GEMINI.md',
+  '.codex',
+  '.cursor'
+]);
+
+/**
+ * cp() filter that rejects symlinks at any depth. Required because
+ * `cp(..., { recursive: true })` without this would copy nested symlinks
+ * verbatim (e.g. `images/secret -> /etc/passwd`); the static server then
+ * follows them via stat()/readFile(), exposing arbitrary local files.
+ *
+ * `lstat` (not `stat`) is mandatory here — `stat` follows symlinks and
+ * would report the target's type, defeating the check.
+ */
+async function rejectSymlink(src: string): Promise<boolean> {
+  try {
+    const st = await lstat(src);
+    return !st.isSymbolicLink();
+  } catch {
+    return false;
+  }
+}
+
+/**
+ * Mirror non-internal files from the deck folder into build/ so the rendered
+ * HTML can reference user assets (images, fonts, etc.) via stable relative
+ * paths. Re-running overwrites matching files in build/. In the normal build
+ * flow, removed assets do not linger because `buildDeck` clears the output
+ * directory before syncing user assets.
+ *
+ * Symlinks are skipped at every depth (top-level entries + the `filter`
+ * passed to cp()) to keep links like `images/secret -> /etc/passwd` out of
+ * the published output. The static server has its own containment check,
+ * but this is defense in depth.
+ */
+async function syncUserAssetsToBuild(
+  deckDir: string,
+  buildDir: string,
+  contentBase: string
+): Promise<void> {
+  let entries: Dirent[];
+  try {
+    entries = await readdir(deckDir, { withFileTypes: true });
+  } catch {
+    return;
+  }
+  // Compare resolved absolute paths — not basenames — so a deck that happens
+  // to contain a folder whose name matches the buildDir basename (e.g. an
+  // `output/` assets folder when outDir=/tmp/out/output/) doesn't get
+  // accidentally skipped. Only the literal buildDir is excluded.
+  const buildResolved = resolve(buildDir);
+  for (const e of entries) {
+    // Hidden / dotfiles never sync.
+    if (e.name.startsWith('.')) continue;
+    if (EXCLUDED_FROM_ASSET_SYNC.has(e.name)) continue;
+    // Skip the content markdown (any filename — caller passes its basename).
+    if (e.name === contentBase) continue;
+    // Don't sync the build dir itself, even if it sits inside deckDir.
+    if (resolve(join(deckDir, e.name)) === buildResolved) continue;
+    // Top-level .html and .tgz files are likely publish artifacts; skip.
+    if (!e.isDirectory() && (e.name.endsWith('.html') || e.name.endsWith('.tgz'))) continue;
+    // Fast path: skip top-level symlinks before recursing.
+    if (e.isSymbolicLink()) continue;
+    const src = join(deckDir, e.name);
+    const dst = join(buildDir, e.name);
+    // `filter` is applied to every src path cp() visits during recursion,
+    // so nested symlinks are rejected too — not just top-level ones.
+    await cp(src, dst, { recursive: true, force: true, filter: rejectSymlink });
+  }
+}
+
 export type DeckStyle = 'professional' | 'academic' | 'fashion' | 'technical' | 'fun';
 export type DeckMode = 'light' | 'dark';
 export type DeckMotion = 'slide-transitions' | 'fragment-reveals' | 'auto-animate';
@@ -133,7 +239,65 @@ ${sections.join('\n')}
 </body>
 </html>`;
 
+  // Clean the build dir before each build so stale entries (especially any
+  // pre-existing symlinks) can't survive a rebuild. Because rm() is
+  // destructive, we ratchet through a layered guard:
+  //
+  //   (a) filesystem root → throw (would wipe a drive)
+  //   (b) outDir contains the deck source → throw (would wipe content.md)
+  //   (c) outDir exists, is non-empty, has no .deckmark-build marker → throw
+  //       (it's a user-owned dir; refusing to clean prevents catastrophic
+  //       data loss if outDir is mis-specified, e.g. pointed at ~/Documents)
+  //
+  // First-build flow: outDir doesn't exist OR is empty → skip rm, mkdir,
+  // drop marker. Subsequent builds find the marker (we wrote it last time)
+  // and can rm safely.
+  const resolvedOutDir = resolve(opts.outDir);
+  const resolvedContent = resolve(opts.contentPath);
+  if (dirname(resolvedOutDir) === resolvedOutDir) {
+    throw new Error(
+      `buildDeck: refusing to clean filesystem root as outDir: ${resolvedOutDir}`
+    );
+  }
+  const outDirWithSep = resolvedOutDir.endsWith(sep) ? resolvedOutDir : resolvedOutDir + sep;
+  if (resolvedContent === resolvedOutDir || resolvedContent.startsWith(outDirWithSep)) {
+    throw new Error(
+      `buildDeck: refusing to clean outDir ${resolvedOutDir} — it contains the deck source ${resolvedContent}`
+    );
+  }
+  const markerPath = join(opts.outDir, '.deckmark-build');
+  let existingEntries: string[] | null = null;
+  try {
+    existingEntries = await readdir(opts.outDir);
+  } catch (err: unknown) {
+    const e = err as NodeJS.ErrnoException;
+    if (e.code !== 'ENOENT') throw err;
+    // ENOENT → outDir doesn't exist yet; we'll create it below.
+  }
+  if (existingEntries !== null && existingEntries.length > 0) {
+    const hasMarker = existingEntries.includes('.deckmark-build');
+    if (!hasMarker) {
+      throw new Error(
+        `buildDeck: refusing to clean ${resolvedOutDir} — directory is non-empty ` +
+        `and has no .deckmark-build marker, so it doesn't look like a deckmark ` +
+        `build output. Pass an empty dir or one previously built by deckmark.`
+      );
+    }
+    await rm(opts.outDir, { recursive: true, force: true });
+  }
   await mkdir(opts.outDir, { recursive: true });
+  // Drop the marker first so an interrupted build still leaves the dir
+  // recognizable on next run.
+  await writeFile(markerPath, '');
+  // Mirror user assets (images/, fonts/, etc.) from the deck folder into
+  // build/ before writing index.html, so the rendered deck can reference
+  // them via relative paths and so publish_deck (which reads from build/)
+  // can find them for inlining or for multi-file deploy.
+  await syncUserAssetsToBuild(
+    dirname(opts.contentPath),
+    opts.outDir,
+    basename(opts.contentPath)
+  );
   await writeFile(join(opts.outDir, 'index.html'), htmlDocument, 'utf8');
   return { outDir: opts.outDir, slideCount: sections.length, style, mode, motion, slideNumbers: slideNumberValue };
 }

runtime/publish/inline-html.ts

@@ -1,6 +1,6 @@
 // runtime/publish/inline-html.ts
 import { readFile, writeFile, mkdir } from 'node:fs/promises';
-import { resolve, dirname, extname } from 'node:path';
+import { resolve, dirname, extname, sep } from 'node:path';
 import { createRequire } from 'node:module';
 import { existsSync } from 'node:fs';
 
@@ -14,6 +14,19 @@ function dbg(msg: string): void {
 // node_modules when deckmark is installed via npx. See static-overlay.ts.
 const REVEAL_DIST = dirname(require.resolve('reveal.js/dist/reveal.js'));
 
+/**
+ * True only when `candidate` resolves to a path equal to, or inside, `root`.
+ * Guards every inliner sink from path-traversal references that, after a
+ * `resolve(root, file)`, would land outside `root` and let the inliner
+ * read/embed arbitrary local files into the single-file HTML.
+ *
+ * Both reveal asset inlining (root = REVEAL_DIST) and image inlining
+ * (root = buildDir) use this — different sinks, same attack class.
+ */
+function isUnder(root: string, candidate: string): boolean {
+  return candidate === root || candidate.startsWith(root + sep);
+}
+
 function mimeFor(ext: string): string {
   switch (ext.toLowerCase()) {
     case '.png': return 'image/png';
@@ -57,9 +70,13 @@ async function replaceLinkStylesheets(html: string): Promise<string> {
   dbg(`  link matches: ${matches.length}`);
   for (const m of matches) {
     const href = m[1];
-    if (!href.startsWith('/vendor/reveal/')) continue;
-    const file = href.replace('/vendor/reveal/', '');
+    // Accept both relative ('vendor/reveal/...') and absolute ('/vendor/reveal/...')
+    // forms — engine now emits relative but old-published HTML may have absolute.
+    const VENDOR_RE = /^\/?vendor\/reveal\//;
+    if (!VENDOR_RE.test(href)) continue;
+    const file = href.replace(VENDOR_RE, '');
     const src = resolve(REVEAL_DIST, file);
+    if (!isUnder(REVEAL_DIST, src)) { dbg(`  skip traversal: ${src}`); continue; }
     if (!existsSync(src)) { dbg(`  skip missing: ${src}`); continue; }
     const css = await readFile(src, 'utf8');
     html = html.replace(m[0], `<style data-deckmark-inlined="${href}">\n${css}\n</style>`);
@@ -74,9 +91,11 @@ async function replaceScripts(html: string): Promise<string> {
   dbg(`  script matches: ${matches.length}`);
   for (const m of matches) {
     const src = m[1];
-    if (!src.startsWith('/vendor/reveal/')) continue;
-    const file = src.replace('/vendor/reveal/', '');
+    const VENDOR_RE = /^\/?vendor\/reveal\//;
+    if (!VENDOR_RE.test(src)) continue;
+    const file = src.replace(VENDOR_RE, '');
     const filePath = resolve(REVEAL_DIST, file);
+    if (!isUnder(REVEAL_DIST, filePath)) { dbg(`  skip traversal: ${filePath}`); continue; }
     if (!existsSync(filePath)) { dbg(`  skip missing: ${filePath}`); continue; }
     const js = await readFile(filePath, 'utf8');
     html = html.replace(m[0], `<script data-deckmark-inlined="${src}">\n${js}\n</script>`);
@@ -97,6 +116,11 @@ async function replaceImages(html: string, buildDir: string): Promise<string> {
     }
     const file = src.replace(/^[/.]+/, '');
     const filePath = resolve(buildDir, file);
+    // Containment guard: stripping leading '/' and '.' chars does NOT remove
+    // internal `..` segments, so `images/../../../etc/passwd` would still
+    // escape buildDir after resolve(). Reject anything that lands outside.
+    const buildResolved = resolve(buildDir);
+    if (!isUnder(buildResolved, filePath)) { dbg(`  skip traversal: ${filePath}`); continue; }
     if (!existsSync(filePath)) { dbg(`  skip missing local img: ${filePath}`); continue; }
     const ext = extname(filePath);
     const buf = await readFile(filePath);

runtime/publish/multi-file.ts

@@ -1,6 +1,6 @@
 // runtime/publish/multi-file.ts
-import { mkdir, cp } from 'node:fs/promises';
-import { dirname, join } from 'node:path';
+import { mkdir, cp, lstat } from 'node:fs/promises';
+import { dirname, join, basename } from 'node:path';
 import { createRequire } from 'node:module';
 
 const require = createRequire(import.meta.url);
@@ -9,19 +9,78 @@ const require = createRequire(import.meta.url);
 // node_modules when deckmark is installed via npx. See static-overlay.ts.
 const REVEAL_DIST = dirname(require.resolve('reveal.js/dist/reveal.js'));
 
+/**
+ * cp() filter that rejects symlinks at every depth. The engine's asset sync
+ * already rejects symlinks before they reach buildDir, but multi-file may
+ * also be invoked with a buildDir produced by other tooling. A
+ * symlink-following web server (Apache/nginx default) would expose whatever
+ * the symlink points at, so we belt-and-suspenders it here too.
+ */
+async function rejectSymlink(src: string): Promise<boolean> {
+  try {
+    const st = await lstat(src);
+    return !st.isSymbolicLink();
+  } catch {
+    return false;
+  }
+}
+
+/**
+ * cp() filter for buildDir → outDir: rejects symlinks AND the internal
+ * .deckmark-build marker so the published folder doesn't ship deckmark's
+ * "this dir is ours to clean" signal.
+ */
+async function rejectSymlinkAndMarker(src: string): Promise<boolean> {
+  if (basename(src) === '.deckmark-build') return false;
+  return rejectSymlink(src);
+}
+
+/**
+ * Throw a clear error if `path` exists and is not a directory. Used before
+ * `mkdir({ recursive: true })` calls that would otherwise fail with ENOTDIR
+ * deep inside Node's internals — leaving the caller to figure out which
+ * path was the problem.
+ */
+async function assertDirOrAbsent(path: string): Promise<void> {
+  try {
+    const st = await lstat(path);
+    if (!st.isDirectory()) {
+      throw new Error(
+        `multiFile: ${path} exists but is not a directory; refusing to overwrite (remove it manually if intentional)`
+      );
+    }
+  } catch (err: unknown) {
+    const e = err as NodeJS.ErrnoException;
+    if (e.code === 'ENOENT') return;
+    throw err;
+  }
+}
+
 export interface MultiFileOpts {
   buildDir: string;
   outDir: string;
 }
 
 export async function multiFile(opts: MultiFileOpts): Promise<{ outDir: string; files: string[] }> {
   await mkdir(opts.outDir, { recursive: true });
-  // Copy reveal.js dist contents to outDir/vendor/reveal/
-  await mkdir(join(opts.outDir, 'vendor', 'reveal'), { recursive: true });
-  await cp(REVEAL_DIST, join(opts.outDir, 'vendor', 'reveal'), { recursive: true });
 
-  // Copy build/ contents (index.html and any embedded images) to outDir/
-  await cp(opts.buildDir, opts.outDir, { recursive: true, force: true });
+  // Order matters: copy buildDir FIRST (user assets, including any
+  // user-named vendor/ that may have made it in), then overlay the
+  // official reveal.js dist LAST. Whatever path collisions exist,
+  // /vendor/reveal/* always reflects the real reveal.js — never user
+  // content masquerading under the same path.
+  await cp(opts.buildDir, opts.outDir, { recursive: true, force: true, filter: rejectSymlinkAndMarker });
+
+  // After the buildDir copy, the reveal dist is overlaid at <outDir>/vendor/reveal/.
+  // If buildDir happened to contain a *file* (not a directory) at either
+  // `vendor` or `vendor/reveal`, the subsequent mkdir({ recursive: true })
+  // would throw ENOTDIR with no context. The engine's asset sync already
+  // excludes `vendor`, but multiFile is callable with any buildDir, so
+  // fail with a clear message instead of letting users debug a stat error.
+  await assertDirOrAbsent(join(opts.outDir, 'vendor'));
+  await assertDirOrAbsent(join(opts.outDir, 'vendor', 'reveal'));
+  await mkdir(join(opts.outDir, 'vendor', 'reveal'), { recursive: true });
+  await cp(REVEAL_DIST, join(opts.outDir, 'vendor', 'reveal'), { recursive: true, force: true, filter: rejectSymlink });
 
   // Walk the output to list what landed
   const { readdir } = await import('node:fs/promises');