Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions make-pdf/src/browseClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
35 changes: 35 additions & 0 deletions make-pdf/test/browseClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down