From 7ca68da78558e736239e0bffb19522ecb8912a8a Mon Sep 17 00:00:00 2001
From: Garry Tan <garrytan@gmail.com>
Date: Fri, 14 Aug 2026 18:44:26 -0700
Subject: [PATCH] fix(make-pdf): reject directories when resolving the browse
 binary

access(X_OK) is true for directories (they carry the execute/traverse
bit on POSIX and pass the Windows existence check too), so cwd-dependent
resolution could pick the ~/.claude/skills/browse alias DIRECTORY as the
browse binary. Every browse call then exited 4 with empty stderr, which
make-pdf surfaced as "Chromium failed to launch" against a perfectly
healthy Chromium (#2156). Guard isExecutable with statSync().isFile()
so only regular files qualify.

Contributed by @jwilk-hrep (PR #2538).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 make-pdf/src/browseClient.ts       |  5 +++++
 make-pdf/test/browseClient.test.ts | 35 ++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/make-pdf/src/browseClient.ts b/make-pdf/src/browseClient.ts
index da25677e5..08213a0e5 100644
--- a/make-pdf/src/browseClient.ts
+++ b/make-pdf/src/browseClient.ts
@@ -158,6 +158,11 @@ export function resolveBrowseBin(env: NodeJS.ProcessEnv = process.env): string {
 
 function isExecutable(p: string): boolean {
   try {
+    // Must be a regular FILE. access(X_OK) alone is true for directories — they carry the
+    // execute/traverse bit on POSIX and pass the Windows check too — so discovery happily
+    // "found" ~/.claude/skills/browse, which is the skill's docs folder containing nothing
+    // but SKILL.md, and returned a directory as the browse binary.
+    if (!fs.statSync(p).isFile()) return false;
     fs.accessSync(p, fs.constants.X_OK);
     return true;
   } catch {
diff --git a/make-pdf/test/browseClient.test.ts b/make-pdf/test/browseClient.test.ts
index 072278e50..b59068e8b 100644
--- a/make-pdf/test/browseClient.test.ts
+++ b/make-pdf/test/browseClient.test.ts
@@ -59,6 +59,41 @@ describe("findExecutable", () => {
     const found = findExecutable("/nonexistent/path/to/nothing");
     expect(found).toBeNull();
   });
+
+  // access(X_OK) is TRUE for directories — they carry the execute/traverse bit — so a
+  // bare X_OK test returned ~/.claude/skills/browse, the skill's docs folder, as "the
+  // browse binary". Every browse call then failed with an empty error, which surfaced
+  // as make-pdf reporting "Chromium failed to launch".
+  test("rejects a DIRECTORY even though it passes access(X_OK)", () => {
+    const dir = fs.mkdtempSync(path.join(os.tmpdir(), "mkpdf-dir-"));
+    try {
+      // Prove the precondition: the directory really does pass the old test.
+      let passesXok = true;
+      try {
+        fs.accessSync(dir, fs.constants.X_OK);
+      } catch {
+        passesXok = false;
+      }
+      expect(passesXok).toBe(true);
+
+      expect(findExecutable(dir)).toBeNull();
+    } finally {
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
+  });
+
+  test("rejects a directory that shadows a real binary name", () => {
+    // The exact shape of the bug: a directory named like the thing being looked for.
+    const base = fs.mkdtempSync(path.join(os.tmpdir(), "mkpdf-shadow-"));
+    const shadow = path.join(base, "browse");
+    fs.mkdirSync(shadow);
+    fs.writeFileSync(path.join(shadow, "SKILL.md"), "# not a binary\n");
+    try {
+      expect(findExecutable(shadow)).toBeNull();
+    } finally {
+      fs.rmSync(base, { recursive: true, force: true });
+    }
+  });
 });
 
 describe("resolveBrowseBin", () => {