Fix which returning directories sometimes

src/bun.js/bindings/c-bindings.cpp

@@ -1,6 +1,9 @@
 // when we don't want to use @cInclude, we can just stick wrapper functions here
 #include <sys/resource.h>
 #include <cstdint>
+#include <unistd.h>
+#include <sys/fcntl.h>
+#include <sys/stat.h>
 
 extern "C" int32_t get_process_priority(uint32_t pid)
 {
@@ -11,3 +14,23 @@ extern "C" int32_t set_process_priority(uint32_t pid, int32_t priority)
 {
     return setpriority(PRIO_PROCESS, pid, priority);
 }
+
+extern "C" bool is_executable_file(const char* path)
+{
+
+#ifdef __APPLE__
+    // O_EXEC is macOS specific
+    int fd = open(path, O_EXEC, O_CLOEXEC);
+    if (fd < 0)
+        return false;
+    close(fd);
+    return true;
+#endif
+
+    struct stat st;
+    if (stat(path, &st) != 0)
+        return false;
+
+    // regular file and user can execute
+    return S_ISREG(st.st_mode) && (st.st_mode & S_IXUSR);
+}

src/which.zig

@@ -10,11 +10,9 @@ fn isValid(buf: *[bun.MAX_PATH_BYTES]u8, segment: []const u8, bin: []const u8) ?
     return @intCast(u16, filepath.len);
 }
 
+extern "C" fn is_executable_file(path: [*:0]const u8) bool;
 fn checkPath(filepath: [:0]const u8) bool {
-    // TODO: is there a single system call for executable AND file?
-    std.os.accessZ(filepath, std.os.X_OK) catch return false;
-    std.os.accessZ(filepath, std.os.F_OK) catch return false;
-    return true;
+    return is_executable_file(filepath);
 }
 
 // Like /usr/bin/which but without needing to exec a child process

test/bun.js/which.test.ts

@@ -1,6 +1,7 @@
 import { test, expect } from "bun:test";
 
 import { which } from "bun";
+import { chmodSync, mkdirSync, unlinkSync } from "node:fs";
 
 test("which", () => {
   writeFixture("/tmp/myscript.sh");
@@ -8,6 +9,14 @@ test("which", () => {
   // Our cwd is not /tmp
   expect(which("myscript.sh")).toBe(null);
 
+  try {
+    mkdirSync("myscript.sh");
+    chmodSync("myscript.sh", "755");
+  } catch (e) {}
+
+  // directories should not be returned
+  expect(which("myscript.sh")).toBe(null);
+
   // "bun" is in our PATH
   expect(which("bun")?.length > 0).toBe(true);
 
@@ -37,6 +46,10 @@ test("which", () => {
       cwd: "/tmp",
     }),
   ).toBe("/tmp/myscript.sh");
+
+  try {
+    unlinkSync("myscript.sh");
+  } catch (e) {}
 });
 
 function writeFixture(path) {