From 8fc4796ed5f765a8b55cf08b1802af6774f2d1a1 Mon Sep 17 00:00:00 2001
From: Filip Stevanovic <62512535+filipstev@users.noreply.github.com>
Date: Wed, 18 Dec 2024 23:52:37 +0100
Subject: [PATCH] fix(ext/node): Fix `fs.access`/`fs.promises.access` with
 `X_OK` mode parameter on Windows (#27407)

- Fixes an issue on Windows where the `fs.constants.X_OK` flag caused
`fs.access` and `fs.promises.access` to incorrectly throw a "permission
denied" error
- Introduced formatting changes due to the formatting tool
- Fixed the issue by always removing the `X_OK` bit from the mode
variable m (not sure if it's necessary to check for the presence of it
in the `mode` param first?)
- Updated unit tests to handle the mentioned constant
- `X_OK` bit is ignored in the Node implementation and should behave
like `F_OK`

fs constants Node documentation:
https://nodejs.org/api/fs.html#fsconstants

fixes https://github.com/denoland/deno/issues/27405
---
 ext/node/polyfills/_fs/_fs_access.ts   | 102 ++++++++++++++-----------
 tests/unit_node/_fs/_fs_access_test.ts |   4 +
 2 files changed, 60 insertions(+), 46 deletions(-)

diff --git a/ext/node/polyfills/_fs/_fs_access.ts b/ext/node/polyfills/_fs/_fs_access.ts
index b501bcbcae..824386e64b 100644
--- a/ext/node/polyfills/_fs/_fs_access.ts
+++ b/ext/node/polyfills/_fs/_fs_access.ts
@@ -30,50 +30,58 @@ export function access(
   mode = getValidMode(mode, "access");
   const cb = makeCallback(callback);
 
-  Deno.lstat(path).then((info) => {
-    if (info.mode === null) {
-      // If the file mode is unavailable, we pretend it has
-      // the permission
-      cb(null);
-      return;
-    }
-    const m = +mode || 0;
-    let fileMode = +info.mode || 0;
-    if (Deno.build.os !== "windows" && info.uid === Deno.uid()) {
-      // If the user is the owner of the file, then use the owner bits of
-      // the file permission
-      fileMode >>= 6;
-    }
-    // TODO(kt3k): Also check the case when the user belong to the group
-    // of the file
-    if ((m & fileMode) === m) {
-      // all required flags exist
-      cb(null);
-    } else {
-      // some required flags don't
-      // deno-lint-ignore no-explicit-any
-      const e: any = new Error(`EACCES: permission denied, access '${path}'`);
-      e.path = path;
-      e.syscall = "access";
-      e.errno = codeMap.get("EACCES");
-      e.code = "EACCES";
-      cb(e);
-    }
-  }, (err) => {
-    if (err instanceof Deno.errors.NotFound) {
-      // deno-lint-ignore no-explicit-any
-      const e: any = new Error(
-        `ENOENT: no such file or directory, access '${path}'`,
-      );
-      e.path = path;
-      e.syscall = "access";
-      e.errno = codeMap.get("ENOENT");
-      e.code = "ENOENT";
-      cb(e);
-    } else {
-      cb(err);
-    }
-  });
+  Deno.lstat(path).then(
+    (info) => {
+      if (info.mode === null) {
+        // If the file mode is unavailable, we pretend it has
+        // the permission
+        cb(null);
+        return;
+      }
+      let m = +mode || 0;
+      let fileMode = +info.mode || 0;
+
+      if (Deno.build.os === "windows") {
+        m &= ~fs.X_OK; // Ignore the X_OK bit on Windows
+      } else if (info.uid === Deno.uid()) {
+        // If the user is the owner of the file, then use the owner bits of
+        // the file permission
+        fileMode >>= 6;
+      }
+
+      // TODO(kt3k): Also check the case when the user belong to the group
+      // of the file
+
+      if ((m & fileMode) === m) {
+        // all required flags exist
+        cb(null);
+      } else {
+        // some required flags don't
+        // deno-lint-ignore no-explicit-any
+        const e: any = new Error(`EACCES: permission denied, access '${path}'`);
+        e.path = path;
+        e.syscall = "access";
+        e.errno = codeMap.get("EACCES");
+        e.code = "EACCES";
+        cb(e);
+      }
+    },
+    (err) => {
+      if (err instanceof Deno.errors.NotFound) {
+        // deno-lint-ignore no-explicit-any
+        const e: any = new Error(
+          `ENOENT: no such file or directory, access '${path}'`,
+        );
+        e.path = path;
+        e.syscall = "access";
+        e.errno = codeMap.get("ENOENT");
+        e.code = "ENOENT";
+        cb(e);
+      } else {
+        cb(err);
+      }
+    },
+  );
 }
 
 export const accessPromise = promisify(access) as (
@@ -91,9 +99,11 @@ export function accessSync(path: string | Buffer | URL, mode?: number) {
       // the permission
       return;
     }
-    const m = +mode! || 0;
+    let m = +mode! || 0;
     let fileMode = +info.mode! || 0;
-    if (Deno.build.os !== "windows" && info.uid === Deno.uid()) {
+    if (Deno.build.os === "windows") {
+      m &= ~fs.X_OK; // Ignore the X_OK bit on Windows
+    } else if (info.uid === Deno.uid()) {
       // If the user is the owner of the file, then use the owner bits of
       // the file permission
       fileMode >>= 6;
diff --git a/tests/unit_node/_fs/_fs_access_test.ts b/tests/unit_node/_fs/_fs_access_test.ts
index f8010b0b8b..0881769f2c 100644
--- a/tests/unit_node/_fs/_fs_access_test.ts
+++ b/tests/unit_node/_fs/_fs_access_test.ts
@@ -28,6 +28,8 @@ Deno.test(
     try {
       await fs.promises.access(file, fs.constants.R_OK);
       await fs.promises.access(file, fs.constants.W_OK);
+      await fs.promises.access(file, fs.constants.X_OK);
+      await fs.promises.access(file, fs.constants.F_OK);
     } finally {
       await Deno.remove(file);
     }
@@ -60,6 +62,8 @@ Deno.test(
     try {
       fs.accessSync(file, fs.constants.R_OK);
       fs.accessSync(file, fs.constants.W_OK);
+      fs.accessSync(file, fs.constants.X_OK);
+      fs.accessSync(file, fs.constants.F_OK);
     } finally {
       Deno.removeSync(file);
     }