lib: unmask mode_t values with 0o777

This commit allows permission bits higher than 0o777 to go through
the API (e.g. `S_ISVTX`=`0o1000`, `S_ISGID`=`0o2000`,
`S_ISUID`=`0o4000`).

Also documents that these bits are not exposed through `fs.constants`
and their behaviors are platform-specific, so the users need to
use them on their own risk.

PR-URL: nodejs#20975
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>

doc/api/fs.md

@@ -1089,6 +1089,12 @@ For example, the octal value `0o765` means:
 * The group may read and write the file.
 * Others may read and execute the file.
 
+Note: When using raw numbers where file modes are expected,
+any value larger than `0o777` may result in platform-specific
+behaviors that are not supported to work consistently.
+Therefore constants like `S_ISVTX`, `S_ISGID` or `S_ISUID` are
+not exposed in `fs.constants`.
+
 Caveats: on Windows only the write permission can be changed, and the
 distinction among the permissions of group, owner or others is not
 implemented.

lib/fs.js

@@ -77,7 +77,7 @@ const {
 } = require('internal/constants');
 const {
   isUint32,
-  validateAndMaskMode,
+  validateMode,
   validateInteger,
   validateInt32,
   validateUint32
@@ -416,7 +416,7 @@ function open(path, flags, mode, callback) {
     callback = makeCallback(mode);
     mode = 0o666;
   } else {
-    mode = validateAndMaskMode(mode, 'mode', 0o666);
+    mode = validateMode(mode, 'mode', 0o666);
     callback = makeCallback(callback);
   }
 
@@ -434,7 +434,7 @@ function openSync(path, flags, mode) {
   path = getPathFromURL(path);
   validatePath(path);
   const flagsNumber = stringToFlags(flags);
-  mode = validateAndMaskMode(mode, 'mode', 0o666);
+  mode = validateMode(mode, 'mode', 0o666);
 
   const ctx = { path };
   const result = binding.open(pathModule.toNamespacedPath(path),
@@ -721,7 +721,7 @@ function mkdir(path, mode, callback) {
     mode = 0o777;
   } else {
     callback = makeCallback(callback);
-    mode = validateAndMaskMode(mode, 'mode', 0o777);
+    mode = validateMode(mode, 'mode', 0o777);
   }
 
   const req = new FSReqWrap();
@@ -732,7 +732,7 @@ function mkdir(path, mode, callback) {
 function mkdirSync(path, mode) {
   path = getPathFromURL(path);
   validatePath(path);
-  mode = validateAndMaskMode(mode, 'mode', 0o777);
+  mode = validateMode(mode, 'mode', 0o777);
   const ctx = { path };
   binding.mkdir(pathModule.toNamespacedPath(path), mode, undefined, ctx);
   handleErrorFromBinding(ctx);
@@ -915,7 +915,7 @@ function unlinkSync(path) {
 
 function fchmod(fd, mode, callback) {
   validateInt32(fd, 'fd', 0);
-  mode = validateAndMaskMode(mode, 'mode');
+  mode = validateMode(mode, 'mode');
   callback = makeCallback(callback);
 
   const req = new FSReqWrap();
@@ -925,7 +925,7 @@ function fchmod(fd, mode, callback) {
 
 function fchmodSync(fd, mode) {
   validateInt32(fd, 'fd', 0);
-  mode = validateAndMaskMode(mode, 'mode');
+  mode = validateMode(mode, 'mode');
   const ctx = {};
   binding.fchmod(fd, mode, undefined, ctx);
   handleErrorFromBinding(ctx);
@@ -966,7 +966,7 @@ function lchmodSync(path, mode) {
 function chmod(path, mode, callback) {
   path = getPathFromURL(path);
   validatePath(path);
-  mode = validateAndMaskMode(mode, 'mode');
+  mode = validateMode(mode, 'mode');
   callback = makeCallback(callback);
 
   const req = new FSReqWrap();
@@ -977,7 +977,7 @@ function chmod(path, mode, callback) {
 function chmodSync(path, mode) {
   path = getPathFromURL(path);
   validatePath(path);
-  mode = validateAndMaskMode(mode, 'mode');
+  mode = validateMode(mode, 'mode');
 
   const ctx = { path };
   binding.chmod(pathModule.toNamespacedPath(path), mode, undefined, ctx);

lib/internal/fs/promises.js

@@ -32,7 +32,7 @@ const {
 } = require('internal/fs/utils');
 const {
   isUint32,
-  validateAndMaskMode,
+  validateMode,
   validateInteger,
   validateUint32
 } = require('internal/validators');
@@ -192,7 +192,7 @@ async function copyFile(src, dest, flags) {
 async function open(path, flags, mode) {
   path = getPathFromURL(path);
   validatePath(path);
-  mode = validateAndMaskMode(mode, 'mode', 0o666);
+  mode = validateMode(mode, 'mode', 0o666);
   return new FileHandle(
     await binding.openFileHandle(pathModule.toNamespacedPath(path),
                                  stringToFlags(flags),
@@ -287,7 +287,7 @@ async function fsync(handle) {
 async function mkdir(path, mode) {
   path = getPathFromURL(path);
   validatePath(path);
-  mode = validateAndMaskMode(mode, 'mode', 0o777);
+  mode = validateMode(mode, 'mode', 0o777);
   return binding.mkdir(pathModule.toNamespacedPath(path), mode, kUsePromises);
 }
 
@@ -359,14 +359,14 @@ async function unlink(path) {
 
 async function fchmod(handle, mode) {
   validateFileHandle(handle);
-  mode = validateAndMaskMode(mode, 'mode');
+  mode = validateMode(mode, 'mode');
   return binding.fchmod(handle.fd, mode, kUsePromises);
 }
 
 async function chmod(path, mode) {
   path = getPathFromURL(path);
   validatePath(path);
-  mode = validateAndMaskMode(mode, 'mode');
+  mode = validateMode(mode, 'mode');
   return binding.chmod(pathModule.toNamespacedPath(path), mode, kUsePromises);
 }
 

lib/internal/validators.js

@@ -16,11 +16,22 @@ function isUint32(value) {
 
 const octalReg = /^[0-7]+$/;
 const modeDesc = 'must be a 32-bit unsigned integer or an octal string';
-// Validator for mode_t (the S_* constants). Valid numbers or octal strings
-// will be masked with 0o777 to be consistent with the behavior in POSIX APIs.
-function validateAndMaskMode(value, name, def) {
+
+/**
+ * Validate values that will be converted into mode_t (the S_* constants).
+ * Only valid numbers and octal strings are allowed. They could be converted
+ * to 32-bit unsigned integers or non-negative signed integers in the C++
+ * land, but any value higher than 0o777 will result in platform-specific
+ * behaviors.
+ *
+ * @param {*} value Values to be validated
+ * @param {string} name Name of the argument
+ * @param {number} def If specified, will be returned for invalid values
+ * @returns {number}
+ */
+function validateMode(value, name, def) {
   if (isUint32(value)) {
-    return value & 0o777;
+    return value;
   }
 
   if (typeof value === 'number') {
@@ -37,7 +48,7 @@ function validateAndMaskMode(value, name, def) {
       throw new ERR_INVALID_ARG_VALUE(name, value, modeDesc);
     }
     const parsed = parseInt(value, 8);
-    return parsed & 0o777;
+    return parsed;
   }
 
   // TODO(BridgeAR): Only return `def` in case `value == null`
@@ -106,7 +117,7 @@ function validateUint32(value, name, positive) {
 module.exports = {
   isInt32,
   isUint32,
-  validateAndMaskMode,
+  validateMode,
   validateInteger,
   validateInt32,
   validateUint32

test/parallel/test-fs-chmod-mask.js

@@ -1,6 +1,6 @@
 'use strict';
 
-// This tests that mode > 0o777 will be masked off with 0o777 in fs APIs.
+// This tests that the lower bits of mode > 0o777 still works in fs APIs.
 
 const common = require('../common');
 const assert = require('assert');

test/parallel/test-fs-mkdir-mode-mask.js

@@ -1,6 +1,6 @@
 'use strict';
 
-// This tests that mode > 0o777 will be masked off with 0o777 in fs.mkdir().
+// This tests that the lower bits of mode > 0o777 still works in fs.mkdir().
 
 const common = require('../common');
 const assert = require('assert');

test/parallel/test-fs-open-mode-mask.js

@@ -1,6 +1,6 @@
 'use strict';
 
-// This tests that mode > 0o777 will be masked off with 0o777 in fs.open().
+// This tests that the lower bits of mode > 0o777 still works in fs.open().
 
 const common = require('../common');
 const assert = require('assert');

test/parallel/test-fs-promises.js

@@ -107,9 +107,8 @@ function verifyStatObject(stat) {
     await chmod(dest, 0o666);
     await handle.chmod(0o666);
 
-    // Mode larger than 0o777 should be masked off.
-    await chmod(dest, (0o777 + 1));
-    await handle.chmod(0o777 + 1);
+    await chmod(dest, (0o10777));
+    await handle.chmod(0o10777);
 
     await utimes(dest, new Date(), new Date());
 

test/parallel/test-process-umask-mask.js

@@ -1,6 +1,6 @@
 'use strict';
 
-// This tests that mask > 0o777 will be masked off with 0o777 in
+// This tests that the lower bits of mode > 0o777 still works in
 // process.umask()
 
 const common = require('../common');