fix(node): Implement fs.lchown (and process.getegid) (#24418)

Closes #21260. Part of #18218. Implements `node:fs.lchown`, and enables the node_compat test for it. The test uses `process.getegid`, which we didn't have implemented, so I went ahead and implemented that as well to get the test working.
denoland · Jul 5, 2024 · d4d3a3c · d4d3a3c
1 parent 28d2ff7
commit d4d3a3c
Show file tree

Hide file tree

Showing 14 changed files with 310 additions and 5 deletions.
diff --git a/cli/standalone/file_system.rs b/cli/standalone/file_system.rs
@@ -145,6 145,26 @@ impl FileSystem for DenoCompileFileSystem {
  RealFs.chown_async(path, uid, gid).await
  }
 
  fn lchown_sync(
  &self,
  path: &Path,
  uid: Option<u32>,
  gid: Option<u32>,
  ) -> FsResult<()> {
  self.error_if_in_vfs(path)?;
  RealFs.lchown_sync(path, uid, gid)
  }
 
  async fn lchown_async(
  &self,
  path: PathBuf,
  uid: Option<u32>,
  gid: Option<u32>,
  ) -> FsResult<()> {
  self.error_if_in_vfs(&path)?;
  RealFs.lchown_async(path, uid, gid).await
  }
 
  fn remove_sync(&self, path: &Path, recursive: bool) -> FsResult<()> {
  self.error_if_in_vfs(path)?;
  RealFs.remove_sync(path, recursive)

diff --git a/ext/fs/in_memory_fs.rs b/ext/fs/in_memory_fs.rs
@@ -178,6 178,24 @@ impl FileSystem for InMemoryFs {
  self.chown_sync(&path, uid, gid)
  }
 
  fn lchown_sync(
  &self,
  _path: &Path,
  _uid: Option<u32>,
  _gid: Option<u32>,
  ) -> FsResult<()> {
  Err(FsError::NotSupported)
  }
 
  async fn lchown_async(
  &self,
  path: PathBuf,
  uid: Option<u32>,
  gid: Option<u32>,
  ) -> FsResult<()> {
  self.lchown_sync(&path, uid, gid)
  }
 
  fn remove_sync(&self, _path: &Path, _recursive: bool) -> FsResult<()> {
  Err(FsError::NotSupported)
  }

diff --git a/ext/fs/interface.rs b/ext/fs/interface.rs
@@ -146,6 146,19 @@ pub trait FileSystem: std::fmt::Debug MaybeSend MaybeSync {
  gid: Option<u32>,
  ) -> FsResult<()>;
 
  fn lchown_sync(
  &self,
  path: &Path,
  uid: Option<u32>,
  gid: Option<u32>,
  ) -> FsResult<()>;
  async fn lchown_async(
  &self,
  path: PathBuf,
  uid: Option<u32>,
  gid: Option<u32>,
  ) -> FsResult<()>;
 
  fn remove_sync(&self, path: &Path, recursive: bool) -> FsResult<()>;
  async fn remove_async(&self, path: PathBuf, recursive: bool) -> FsResult<()>;
 

diff --git a/ext/fs/std_fs.rs b/ext/fs/std_fs.rs
@@ -303,6 303,24 @@ impl FileSystem for RealFs {
  .await?
  }
 
  fn lchown_sync(
  &self,
  path: &Path,
  uid: Option<u32>,
  gid: Option<u32>,
  ) -> FsResult<()> {
  lchown(path, uid, gid)
  }
 
  async fn lchown_async(
  &self,
  path: PathBuf,
  uid: Option<u32>,
  gid: Option<u32>,
  ) -> FsResult<()> {
  spawn_blocking(move || lchown(&path, uid, gid)).await?
  }
 
  fn write_file_sync(
  &self,
  path: &Path,
@@ -431,6 449,31 @@ fn chown(_path: &Path, _uid: Option<u32>, _gid: Option<u32>) -> FsResult<()> {
  Err(FsError::NotSupported)
 }
 
 #[cfg(unix)]
 fn lchown(path: &Path, uid: Option<u32>, gid: Option<u32>) -> FsResult<()> {
  use std::os::unix::ffi::OsStrExt;
  let c_path = std::ffi::CString::new(path.as_os_str().as_bytes()).unwrap();
  // -1 = leave unchanged
  let uid = uid
  .map(|uid| uid as libc::uid_t)
  .unwrap_or(-1i32 as libc::uid_t);
  let gid = gid
  .map(|gid| gid as libc::gid_t)
  .unwrap_or(-1i32 as libc::gid_t);
  // SAFETY: `c_path` is a valid C string and lives throughout this function call.
  let result = unsafe { libc::lchown(c_path.as_ptr(), uid, gid) };
  if result != 0 {
  return Err(io::Error::last_os_error().into());
  }
  Ok(())
 }
 
 // TODO: implement lchown for Windows
 #[cfg(not(unix))]
 fn lchown(_path: &Path, _uid: Option<u32>, _gid: Option<u32>) -> FsResult<()> {
  Err(FsError::NotSupported)
 }
 
 fn remove(path: &Path, recursive: bool) -> FsResult<()> {
  // TODO: this is racy. This should open fds, and then `unlink` those.
  let metadata = fs::symlink_metadata(path)?;

diff --git a/ext/node/lib.rs b/ext/node/lib.rs
@@ -315,6 315,8 @@ deno_core::extension!(deno_node,
  ops::fs::op_node_fs_exists_sync<P>,
  ops::fs::op_node_cp_sync<P>,
  ops::fs::op_node_cp<P>,
  ops::fs::op_node_lchown_sync<P>,
  ops::fs::op_node_lchown<P>,
  ops::fs::op_node_lutimes_sync<P>,
  ops::fs::op_node_lutimes<P>,
  ops::fs::op_node_statfs<P>,
@@ -365,6 367,7 @@ deno_core::extension!(deno_node,
  ops::os::op_node_os_set_priority<P>,
  ops::os::op_node_os_username<P>,
  ops::os::op_geteuid<P>,
  ops::os::op_getegid<P>,
  ops::os::op_cpus<P>,
  ops::os::op_homedir<P>,
  op_node_build_os,
@@ -426,6 429,7 @@ deno_core::extension!(deno_node,
  "_fs/_fs_fsync.ts",
  "_fs/_fs_ftruncate.ts",
  "_fs/_fs_futimes.ts",
  "_fs/_fs_lchown.ts",
  "_fs/_fs_link.ts",
  "_fs/_fs_lstat.ts",
  "_fs/_fs_lutimes.ts",

diff --git a/ext/node/ops/fs.rs b/ext/node/ops/fs.rs
@@ -273,3 273,44 @@ where
 
  Ok(())
 }
 
 #[op2]
 pub fn op_node_lchown_sync<P>(
  state: &mut OpState,
  #[string] path: String,
  uid: Option<u32>,
  gid: Option<u32>,
 ) -> Result<(), AnyError>
 where
  P: NodePermissions 'static,
 {
  let path = PathBuf::from(path);
  state
  .borrow_mut::<P>()
  .check_write_with_api_name(&path, Some("node:fs.lchownSync"))?;
  let fs = state.borrow::<FileSystemRc>();
  fs.lchown_sync(&path, uid, gid)?;
  Ok(())
 }
 
 #[op2(async)]
 pub async fn op_node_lchown<P>(
  state: Rc<RefCell<OpState>>,
  #[string] path: String,
  uid: Option<u32>,
  gid: Option<u32>,
 ) -> Result<(), AnyError>
 where
  P: NodePermissions 'static,
 {
  let path = PathBuf::from(path);
  let fs = {
  let mut state = state.borrow_mut();
  state
  .borrow_mut::<P>()
  .check_write_with_api_name(&path, Some("node:fs.lchown"))?;
  state.borrow::<FileSystemRc>().clone()
  };
  fs.lchown_async(path, uid, gid).await?;
  Ok(())
 }
diff --git a/ext/node/ops/os/mod.rs b/ext/node/ops/os/mod.rs
@@ -75,6 75,25 @@ where
  Ok(euid)
 }
 
 #[op2(fast)]
 pub fn op_getegid<P>(state: &mut OpState) -> Result<u32, AnyError>
 where
  P: NodePermissions 'static,
 {
  {
  let permissions = state.borrow_mut::<P>();
  permissions.check_sys("getegid", "node:os.getegid()")?;
  }
 
  #[cfg(windows)]
  let egid = 0;
  #[cfg(unix)]
  // SAFETY: Call to libc getegid.
  let egid = unsafe { libc::getegid() };
 
  Ok(egid)
 }
 
 #[op2]
 #[serde]
 pub fn op_cpus<P>(state: &mut OpState) -> Result<Vec<cpus::CpuInfo>, AnyError>

diff --git a/ext/node/polyfills/_fs/_fs_lchown.ts b/ext/node/polyfills/_fs/_fs_lchown.ts
@@ -0,0 1,61 @@
 // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
 
 // TODO(petamoriken): enable prefer-primordials for node polyfills
 // deno-lint-ignore-file prefer-primordials
 
 import {
  type CallbackWithError,
  makeCallback,
 } from "ext:deno_node/_fs/_fs_common.ts";
 import {
  getValidatedPath,
  kMaxUserId,
 } from "ext:deno_node/internal/fs/utils.mjs";
 import * as pathModule from "node:path";
 import { validateInteger } from "ext:deno_node/internal/validators.mjs";
 import type { Buffer } from "node:buffer";
 import { promisify } from "ext:deno_node/internal/util.mjs";
 import { op_node_lchown, op_node_lchown_sync } from "ext:core/ops";
 
 /**
  * Asynchronously changes the owner and group
  * of a file, without following symlinks.
  */
 export function lchown(
  path: string | Buffer | URL,
  uid: number,
  gid: number,
  callback: CallbackWithError,
 ) {
  callback = makeCallback(callback);
  path = getValidatedPath(path).toString();
  validateInteger(uid, "uid", -1, kMaxUserId);
  validateInteger(gid, "gid", -1, kMaxUserId);
 
  op_node_lchown(pathModule.toNamespacedPath(path), uid, gid).then(
  () => callback(null),
  callback,
  );
 }
 
 export const lchownPromise = promisify(lchown) as (
  path: string | Buffer | URL,
  uid: number,
  gid: number,
 ) => Promise<void>;
 
 /**
  * Synchronously changes the owner and group
  * of a file, without following symlinks.
  */
 export function lchownSync(
  path: string | Buffer | URL,
  uid: number,
  gid: number,
 ) {
  path = getValidatedPath(path).toString();
  validateInteger(uid, "uid", -1, kMaxUserId);
  validateInteger(gid, "gid", -1, kMaxUserId);
 
  op_node_lchown_sync(pathModule.toNamespacedPath(path), uid, gid);
 }
diff --git a/ext/node/polyfills/fs.ts b/ext/node/polyfills/fs.ts
@@ -27,6 27,11 @@ import { fstat, fstatSync } from "ext:deno_node/_fs/_fs_fstat.ts";
 import { fsync, fsyncSync } from "ext:deno_node/_fs/_fs_fsync.ts";
 import { ftruncate, ftruncateSync } from "ext:deno_node/_fs/_fs_ftruncate.ts";
 import { futimes, futimesSync } from "ext:deno_node/_fs/_fs_futimes.ts";
 import {
  lchown,
  lchownPromise,
  lchownSync,
 } from "ext:deno_node/_fs/_fs_lchown.ts";
 import { link, linkPromise, linkSync } from "ext:deno_node/_fs/_fs_link.ts";
 import { lstat, lstatPromise, lstatSync } from "ext:deno_node/_fs/_fs_lstat.ts";
 import {
@@ -173,7 178,7 @@ const promises = {
  unlink: unlinkPromise,
  chmod: chmodPromise,
  // lchmod: promisify(lchmod),
- // lchown: promisify(lchown),
  lchown: lchownPromise,
  chown: chownPromise,
  utimes: utimesPromise,
  lutimes: lutimesPromise,
@@ -218,6 223,8 @@ export default {
  ftruncateSync,
  futimes,
  futimesSync,
  lchown,
  lchownSync,
  link,
  linkSync,
  lstat,

diff --git a/ext/node/polyfills/process.ts b/ext/node/polyfills/process.ts
@@ -7,6 7,7 @@
 import { core, internals } from "ext:core/mod.js";
 import { initializeDebugEnv } from "ext:deno_node/internal/util/debuglog.ts";
 import {
  op_getegid,
  op_geteuid,
  op_node_process_kill,
  op_process_abort,
@@ -309,15 310,16 @@ export function kill(pid: number, sig: string | number = "SIGTERM") {
  return true;
 }
 
-let getgid, getuid, geteuid;
 let getgid, getuid, getegid, geteuid;
 
 if (!isWindows) {
  getgid = () => Deno.gid();
  getuid = () => Deno.uid();
  getegid = () => op_getegid();
  geteuid = () => op_geteuid();
 }
 
-export { geteuid, getgid, getuid };
 export { getegid, geteuid, getgid, getuid };
 
 const ALLOWED_FLAGS = buildAllowedFlags();
 
@@ -685,6 687,9 @@ Process.prototype.getgid = getgid;
 /** This method is removed on Windows */
 Process.prototype.getuid = getuid;
 
 /** This method is removed on Windows */
 Process.prototype.getegid = getegid;
 
 /** This method is removed on Windows */
 Process.prototype.geteuid = geteuid;
 
@@ -726,6 731,7 @@ Process.prototype.noDeprecation = false;
 if (isWindows) {
  delete Process.prototype.getgid;
  delete Process.prototype.getuid;
  delete Process.prototype.getegid;
  delete Process.prototype.geteuid;
 }
 

diff --git a/runtime/permissions/lib.rs b/runtime/permissions/lib.rs
@@ -915,7 915,9 @@ impl Descriptor for SysDescriptor {
 pub fn parse_sys_kind(kind: &str) -> Result<&str, AnyError> {
  match kind {
  "hostname" | "osRelease" | "osUptime" | "loadavg" | "networkInterfaces"
- | "systemMemoryInfo" | "uid" | "gid" | "cpus" | "homedir" => Ok(kind),
  | "systemMemoryInfo" | "uid" | "gid" | "cpus" | "homedir" | "getegid" => {
  Ok(kind)
  }
  _ => Err(type_error(format!("unknown system info kind \"{kind}\""))),
  }
 }

diff --git a/tests/node_compat/config.jsonc b/tests/node_compat/config.jsonc
@@ -327,6 327,7 @@
  "test-fs-chown-type-check.js",
  "test-fs-copyfile.js",
  "test-fs-empty-readStream.js",
  "test-fs-lchown.js",
  "test-fs-mkdir.js",
  "test-fs-open-flags.js",
  "test-fs-open-mode-mask.js",

diff --git a/tests/node_compat/runner/TODO.md b/tests/node_compat/runner/TODO.md
@@ -794,7 794,6 @@ NOTE: This file should not be manually edited. Please edit `tests/node_compat/co
 - [parallel/test-fs-fmap.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-fmap.js)
 - [parallel/test-fs-fsync.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-fsync.js)
 - [parallel/test-fs-lchmod.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-lchmod.js)
-- [parallel/test-fs-lchown.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-lchown.js)
 - [parallel/test-fs-link.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-link.js)
 - [parallel/test-fs-long-path.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-long-path.js)
 - [parallel/test-fs-make-callback.js](https://github.com/nodejs/node/tree/v18.12.1/test/parallel/test-fs-make-callback.js)