fix(make-pdf): read local images only from the document's directory, and only image files (GST-1747) (#4113)

* fix(make-pdf): read local images only from the document's directory, and only image files (GST-1747)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(make-pdf): inline a local file only when its bytes match its image format

A file with an image extension now needs that format's signature (or a recognized size header) before it is inlined; anything else gets the blocked placeholder, and --strict throws. The docx ICNS test now sees the local file blocked by the prepass and the inline data URI dropped by the docx guard.

Review-Finding: p1/specialist-batched#1 reader=security
Review-Finding: p1/specialist-batched#2 reader=testing
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix(make-pdf): compare image paths with path.relative so a root-level document keeps its images

A markdown file at the filesystem root made the containment prefix '//' (or 'C:\\'), which no path matched.

Review-Finding: v1/adversarial-native#5 reader=adversarial-native
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

changelog.d/capy-gst-1747-make-pdf-local-images.md

@@ -0,0 +1,4 @@
+---
+important: security
+---
+- **make-pdf reads local images only from the markdown's own directory, and only image files.** An image reference that resolves outside the input markdown's directory (a `../` path, an absolute path, a `file:` URL or a symlink pointing out) is no longer read: it renders as a visible `[image blocked (outside the document directory): …]` placeholder with a warning, and `--strict` still fails the run. A local file that is not a png, jpeg, gif, webp or svg image renders as an `[image blocked (not an image): …]` placeholder instead of being inlined, and fails the run under `--strict`. The make-pdf skill and docs/howto-diagrams-and-formats.md describe the new defaults. (GST-1747)

docs/howto-diagrams-and-formats.md

@@ -59,9 +59,11 @@ so a phone photo doesn't bloat the document.
 
 Image safety defaults: remote (http/https) images are **blocked with a
 visible placeholder** unless you pass `--allow-network`. An image path that
-resolves outside the markdown's directory (even through a symlink) still
-inlines but warns loudly. Files over 64MB and non-regular files (fifos,
-devices) degrade to a placeholder instead of hanging the render.
+resolves outside the markdown's directory (even through a symlink) is never
+read: it renders as a visible placeholder with a warning. Only image files
+(png, jpeg, gif, webp, svg) inline; other files, files over 64MB and
+non-regular files (fifos, devices) degrade to a placeholder instead of
+hanging the render.
 
 Per-image directives go immediately after the image:
 
@@ -130,9 +132,9 @@ make-pdf generate docs.md --strict
 ```
 
 Missing local images, blocked remote images, out-of-tree image reads (a path
-or symlink resolving outside the markdown's directory), oversized files
-(>64MB), and non-regular files all exit non-zero instead of degrading to a
-warning or placeholder — for docs pipelines where a broken image should
+or symlink resolving outside the markdown's directory), non-image files,
+oversized files (>64MB), and non-regular files all exit non-zero instead of
+degrading to a warning or placeholder — for docs pipelines where a broken image should
 break the build.
 
 ## Troubleshooting

make-pdf/SKILL.md

@@ -285,10 +285,11 @@ small with no visible quality loss.
 
 Remote (http/https) images are **blocked with a visible placeholder** by
 default — offline posture; pass `--allow-network` to fetch them. An image
-that resolves outside the markdown's directory (even via symlink) still
-inlines, but warns loudly; `--strict` makes it fatal. Files over 64MB or
-non-regular files (fifos, devices) degrade to a placeholder instead of
-hanging the run.
+that resolves outside the markdown's directory (even via symlink) is never
+read: it renders as a visible placeholder with a warning, and `--strict`
+makes it fatal. Only image files (png, jpeg, gif, webp, svg) inline; other
+files, files over 64MB and non-regular files (fifos, devices) degrade to a
+placeholder instead of hanging the run.
 
 Per-image directives, written immediately after the image:
 
@@ -322,8 +323,8 @@ promoted page is vertically centered. When the heuristic guesses wrong,
 ### CI mode — fail loud on missing assets
 
 ```bash
-"$P" generate docs.md --strict     # missing, remote, out-of-tree, oversized,
-                                 # and non-regular-file images exit non-zero
+"$P" generate docs.md --strict     # missing, remote, out-of-tree, non-image,
+                                 # oversized and non-regular-file images exit non-zero
                                  # instead of warn + placeholder
 ```
 
@@ -348,8 +349,9 @@ Branding:
 Output:
   --to pdf|html|docx         Output format (default: pdf). html = single
                              self-contained file; docx = content fidelity.
-  --strict                   Missing, remote, out-of-tree, oversized, or
-                             non-regular-file images fail the run (CI mode).
+  --strict                   Missing, remote, out-of-tree, non-image,
+                             oversized, or non-regular-file images fail the
+                             run (CI mode).
   --page-numbers             "N of M" footer (default on)
   --tagged                   Accessible PDF (default on)
   --outline                  PDF bookmarks from headings (default on)

make-pdf/SKILL.md.tmpl

@@ -133,10 +133,11 @@ small with no visible quality loss.
 
 Remote (http/https) images are **blocked with a visible placeholder** by
 default — offline posture; pass `--allow-network` to fetch them. An image
-that resolves outside the markdown's directory (even via symlink) still
-inlines, but warns loudly; `--strict` makes it fatal. Files over 64MB or
-non-regular files (fifos, devices) degrade to a placeholder instead of
-hanging the run.
+that resolves outside the markdown's directory (even via symlink) is never
+read: it renders as a visible placeholder with a warning, and `--strict`
+makes it fatal. Only image files (png, jpeg, gif, webp, svg) inline; other
+files, files over 64MB and non-regular files (fifos, devices) degrade to a
+placeholder instead of hanging the run.
 
 Per-image directives, written immediately after the image:
 
@@ -170,8 +171,8 @@ promoted page is vertically centered. When the heuristic guesses wrong,
 ### CI mode — fail loud on missing assets
 
 ```bash
-"$P" generate docs.md --strict     # missing, remote, out-of-tree, oversized,
-                                 # and non-regular-file images exit non-zero
+"$P" generate docs.md --strict     # missing, remote, out-of-tree, non-image,
+                                 # oversized and non-regular-file images exit non-zero
                                  # instead of warn + placeholder
 ```
 
@@ -196,8 +197,9 @@ Branding:
 Output:
   --to pdf|html|docx         Output format (default: pdf). html = single
                              self-contained file; docx = content fidelity.
-  --strict                   Missing, remote, out-of-tree, oversized, or
-                             non-regular-file images fail the run (CI mode).
+  --strict                   Missing, remote, out-of-tree, non-image,
+                             oversized, or non-regular-file images fail the
+                             run (CI mode).
   --page-numbers             "N of M" footer (default on)
   --tagged                   Accessible PDF (default on)
   --outline                  PDF bookmarks from headings (default on)

make-pdf/src/diagram-prepass.ts

@@ -609,19 +609,20 @@ export async function inlineLocalImages(html: string, opts: PrepassImageOptions)
       return buildMissingImagePlaceholder(src);
     }
 
-    // Out-of-tree reads are legal (local CLI semantics — like pandoc) but
-    // never silent: an agent PDF-ing untrusted markdown should not quietly
-    // embed ~/.ssh/config into a shareable document. --strict makes it fatal.
+    // Out-of-tree files are never read: rendering untrusted markdown must not
+    // copy files from elsewhere on disk into a shareable document. Visible
+    // placeholder by default; --strict makes it fatal.
     // Compare REAL paths — a symlink inside the input dir pointing outside
     // would otherwise pass a string-prefix check (Codex adversarial finding).
     // Runs after the existence check: realpath of a missing file can't
     // resolve, and on macOS /var vs /private/var would false-positive.
-    const inputRoot = safeRealpath(path.resolve(opts.inputDir)) + path.sep;
+    const inputRoot = safeRealpath(path.resolve(opts.inputDir));
     const realFilePath = safeRealpath(filePath);
-    if (!realFilePath.startsWith(inputRoot)) {
+    if (!isInsideDir(realFilePath, inputRoot)) {
       const msg = `image resolves OUTSIDE the input directory: ${src} → ${realFilePath}`;
-      if (opts.strict) throw new StrictModeError(msg + " — move it under the markdown's directory or drop --strict");
+      if (opts.strict) throw new StrictModeError(msg + " — move it under the markdown's directory");
       opts.warn(msg);
+      return buildBlockedLocalPlaceholder("outside the document directory", src);
     }
 
     // Bound the read BEFORE reading: a markdown image pointing at a special
@@ -649,7 +650,13 @@ export async function inlineLocalImages(html: string, opts: PrepassImageOptions)
 
     const buf = fs.readFileSync(filePath);
     const dims = imageDims(buf);
-    const mime = dims?.mime ?? mimeFromExtension(filePath);
+    const mime = dims?.mime ?? signedExtensionMime(buf, filePath);
+    if (mime === "application/octet-stream") {
+      const msg = `not a recognized image (png, jpeg, gif, webp, svg): ${src}`;
+      if (opts.strict) throw new StrictModeError(msg);
+      opts.warn(msg);
+      return buildBlockedLocalPlaceholder("not an image", src);
+    }
 
     // Print-resolution normalization (D4): rasters only — SVG scales free.
     if (dims && mime !== "image/svg+xml" && dims.width > maxPx && opts.run) {
@@ -735,6 +742,18 @@ function buildBlockedRemotePlaceholder(src: string): string {
   );
 }
 
+function buildBlockedLocalPlaceholder(reason: string, src: string): string {
+  return (
+    `<span class="image-missing" role="img" aria-label="image blocked">` +
+    `[image blocked (${reason}): ${escapeHtml(src)}]</span>`
+  );
+}
+
+function isInsideDir(p: string, dir: string): boolean {
+  const rel = path.relative(dir, p);
+  return rel !== "" && rel !== ".." && !rel.startsWith(".." + path.sep) && !path.isAbsolute(rel);
+}
+
 /** realpath that degrades to the input path when resolution fails. */
 function safeRealpath(p: string): string {
   try {
@@ -744,6 +763,20 @@ function safeRealpath(p: string): string {
   }
 }
 
+const IMAGE_SIGNATURES: Record<string, (b: Buffer) => boolean> = {
+  "image/png": (b) => b.length >= 8 && b.readUInt32BE(0) === 0x89504e47 && b.readUInt32BE(4) === 0x0d0a1a0a,
+  "image/jpeg": (b) => b.length >= 3 && b[0] === 0xff && b[1] === 0xd8 && b[2] === 0xff,
+  "image/gif": (b) => /^GIF8[79]a/.test(b.toString("ascii", 0, 6)),
+  "image/webp": (b) => b.toString("ascii", 0, 4) === "RIFF" && b.toString("ascii", 8, 12) === "WEBP",
+  "image/svg+xml": (b) => /<svg[\s>]/i.test(b.toString("utf8", 0, 65536)),
+};
+
+/** The extension's image MIME, only when the bytes carry that format's signature. */
+function signedExtensionMime(buf: Buffer, p: string): string {
+  const mime = mimeFromExtension(p);
+  return IMAGE_SIGNATURES[mime]?.(buf) ? mime : "application/octet-stream";
+}
+
 function mimeFromExtension(p: string): string {
   switch (path.extname(p).toLowerCase()) {
     case ".png": return "image/png";

make-pdf/test/diagram-prepass.test.ts

@@ -348,21 +348,89 @@ describe("inlineLocalImages", () => {
     expect(out).toContain('data-gstack-px-height="44"');
   });
 
-  test("out-of-tree image reads warn (never silent) and still inline", async () => {
+  test("out-of-tree image is not inlined: visible placeholder + warning", async () => {
     const outside = fs.mkdtempSync(path.join(os.tmpdir(), "prepass-outside-"));
-    fs.writeFileSync(path.join(outside, "ext.png"), tinyPng(10, 10));
+    const png = tinyPng(10, 10);
+    fs.writeFileSync(path.join(outside, "ext.png"), png);
     try {
       const warnings: string[] = [];
       const out = await inlineLocalImages(`<img src="${path.join(outside, "ext.png")}">`, {
         ...base, warn: (m) => warnings.push(m),
       });
-      expect(out).toContain("data:image/png;base64,");
+      expect(out).not.toContain(png.toString("base64"));
+      expect(out).not.toContain("data:image/png;base64,");
+      expect(out).toContain("image-missing");
       expect(warnings.some((w) => w.includes("OUTSIDE the input directory"))).toBe(true);
     } finally {
       fs.rmSync(outside, { recursive: true, force: true });
     }
   });
 
+  test("relative reference to a file outside the document directory leaves its bytes out of the output", async () => {
+    const root = fs.mkdtempSync(path.join(os.tmpdir(), "prepass-tree-"));
+    const docDir = path.join(root, "doc");
+    const outsideDir = path.join(root, "outside");
+    fs.mkdirSync(docDir);
+    fs.mkdirSync(outsideDir);
+    const secret = Buffer.from("PRIVATE-MARKER-7f3c9a\n");
+    fs.writeFileSync(path.join(outsideDir, "secret.txt"), secret);
+    try {
+      const warnings: string[] = [];
+      const out = await inlineLocalImages(`<img src="../outside/secret.txt" alt="x">`, {
+        ...base, inputDir: docDir, warn: (m) => warnings.push(m),
+      });
+      expect(out).not.toContain(secret.toString("base64"));
+      expect(out).not.toContain("PRIVATE-MARKER-7f3c9a");
+      expect(out).not.toContain("data:");
+      expect(out).toContain("image-missing");
+      expect(warnings.some((w) => w.includes("OUTSIDE the input directory"))).toBe(true);
+    } finally {
+      fs.rmSync(root, { recursive: true, force: true });
+    }
+  });
+
+  test("non-image file inside the input dir is not inlined", async () => {
+    const text = Buffer.from("plain text, not an image\n");
+    fs.writeFileSync(path.join(dir, "notes.txt"), text);
+    const warnings: string[] = [];
+    const out = await inlineLocalImages(`<img src="notes.txt">`, { ...base, warn: (m) => warnings.push(m) });
+    expect(out).not.toContain(text.toString("base64"));
+    expect(out).not.toContain("data:");
+    expect(out).toContain("image-missing");
+    expect(warnings.some((w) => w.includes("not a recognized image"))).toBe(true);
+  });
+
+  test("markdown at the filesystem root still inlines images beneath it", async () => {
+    const file = path.join(dir, "rooted.png");
+    fs.writeFileSync(file, tinyPng(12, 12));
+    const out = await inlineLocalImages(`<img src="${file}">`, { ...base, inputDir: path.parse(dir).root, warn: () => {} });
+    expect(out).toContain("data:image/png;base64,");
+  });
+
+  test("non-image bytes under an image extension are not inlined", async () => {
+    const text = Buffer.from("plain text named like an image\n");
+    fs.writeFileSync(path.join(dir, "fake.png"), text);
+    fs.writeFileSync(path.join(dir, "fake.svg"), text);
+    for (const name of ["fake.png", "fake.svg"]) {
+      const warnings: string[] = [];
+      const out = await inlineLocalImages(`<img src="${name}">`, { ...base, warn: (m) => warnings.push(m) });
+      expect(out).not.toContain(text.toString("base64"));
+      expect(out).not.toContain("data:");
+      expect(out).toContain("image-missing");
+      expect(warnings.some((w) => w.includes("not a recognized image"))).toBe(true);
+      await expect(
+        inlineLocalImages(`<img src="${name}">`, { ...base, strict: true, warn: () => {} }),
+      ).rejects.toThrow(StrictModeError);
+    }
+  });
+
+  test("non-image file + --strict → StrictModeError", async () => {
+    fs.writeFileSync(path.join(dir, "notes2.txt"), "plain text\n");
+    await expect(
+      inlineLocalImages(`<img src="notes2.txt">`, { ...base, strict: true, warn: () => {} }),
+    ).rejects.toThrow(StrictModeError);
+  });
+
   test("out-of-tree image + --strict → StrictModeError", async () => {
     const outside = fs.mkdtempSync(path.join(os.tmpdir(), "prepass-outside-"));
     fs.writeFileSync(path.join(outside, "ext.png"), tinyPng(10, 10));

make-pdf/test/docx-image-size.test.ts

@@ -104,7 +104,8 @@ await generate({ input: ${JSON.stringify(path.join(workDir, "doc.md"))}, output:
       const run = Bun.spawnSync([process.execPath, "-e", script], { timeout: 30_000, stdout: "pipe", stderr: "pipe" });
       const stderr = run.stderr.toString();
       expect(run.exitCode).toBe(0);
-      expect(stderr.match(/docx: dropped an ICNS image/g)).toHaveLength(2);
+      expect(stderr).toContain("not a recognized image (png, jpeg, gif, webp, svg): evil.png");
+      expect(stderr.match(/docx: dropped an ICNS image/g)).toHaveLength(1);
       expect(unzip(["-p", out, "word/document.xml"]).toString("utf8")).not.toContain("<w:drawing>");
     } finally {
       fs.rmSync(workDir, { recursive: true, force: true });