fix(process): clear supplementary groups before changing identity

@@ -4,9 +4,15 @@ use std::borrow::Cow; use std::cell::Cell; use std::cell::RefCell; use std::collections::HashMap;+#[cfg(unix)]+use std::ffi::CStr;+#[cfg(unix)]+use std::ffi::CString; use std::ffi::OsString; use std::io::Write; #[cfg(unix)]+use std::os::unix::ffi::OsStrExt;+#[cfg(unix)] use std::os::unix::io::FromRawFd; #[cfg(unix)] use std::os::unix::prelude::ExitStatusExt;@@ -173,6 +179,104 @@ impl<'de> Deserialize<'de> for StdioOrFd {   } } +#[cfg(unix)]+fn should_clear_supplementary_groups(+  uid: Option<u32>,+  gid: Option<u32>,+) -> bool {+  uid.is_some() || gid.is_some()+}++#[cfg(unix)]+fn should_change_cwd_in_pre_exec(+  uid: Option<u32>,+  gid: Option<u32>,+  set_cwd_on_command: bool,+) -> bool {+  set_cwd_on_command && should_clear_supplementary_groups(uid, gid)+}++#[cfg(unix)]+fn unix_child_effective_identity_will_change(+  uid: Option<u32>,+  gid: Option<u32>,+) -> bool {+  // SAFETY: getegid and geteuid are async-signal-safe and cannot fail.+  let (current_uid, current_gid) =+    unsafe { (libc::geteuid(), libc::getegid()) };+  uid.is_some_and(|uid| uid as libc::uid_t != current_uid)+    || gid.is_some_and(|gid| gid as libc::gid_t != current_gid)+}++#[cfg(unix)]+fn check_identity_syscall_result(result: libc::c_int) -> std::io::Result<()> {+  if result == -1 {+    Err(std::io::Error::last_os_error())+  } else {+    Ok(())+  }+}++#[cfg(unix)]+#[derive(Debug, Clone, Copy, PartialEq, Eq)]+enum UnixChildSetupStep<'a> {+  ClearSupplementaryGroups,+  SetPrimaryGroupId(u32),+  SetUserId(u32),+  ChangeCurrentDirectory(&'a CStr),+}++#[cfg(unix)]+fn run_unix_child_setup_step(step: UnixChildSetupStep) -> std::io::Result<()> {+  // SAFETY: these calls are async-signal-safe and run from pre_exec. The cwd+  // is converted to a C string before spawning, and a null groups pointer is+  // valid when the group count is zero.+  let result = unsafe {+    match step {+      UnixChildSetupStep::ClearSupplementaryGroups => {+        libc::setgroups(0, std::ptr::null())+      }+      UnixChildSetupStep::SetPrimaryGroupId(gid) => {+        libc::setgid(gid as libc::gid_t)+      }+      UnixChildSetupStep::SetUserId(uid) => libc::setuid(uid as libc::uid_t),+      UnixChildSetupStep::ChangeCurrentDirectory(cwd) => {+        libc::chdir(cwd.as_ptr())+      }+    }+  };+  check_identity_syscall_result(result)+}++#[cfg(unix)]+fn apply_unix_child_setup<'a>(+  uid: Option<u32>,+  gid: Option<u32>,+  cwd: Option<&'a CStr>,+  effective_identity_will_change: bool,+  mut run_step: impl FnMut(UnixChildSetupStep<'a>) -> std::io::Result<()>,+) -> std::io::Result<()> {+  // Preserve same-ID spawns for unprivileged callers, which can reapply their+  // effective IDs even though they cannot alter supplementary groups. Never+  // ignore a group-clearing failure before an effective identity transition.+  if let Err(err) = run_step(UnixChildSetupStep::ClearSupplementaryGroups)+    && (effective_identity_will_change+      || err.raw_os_error() != Some(libc::EPERM))+  {+    return Err(err);+  }+  if let Some(gid) = gid {+    run_step(UnixChildSetupStep::SetPrimaryGroupId(gid))?;+  }+  if let Some(uid) = uid {+    run_step(UnixChildSetupStep::SetUserId(uid))?;+  }+  if let Some(cwd) = cwd {+    run_step(UnixChildSetupStep::ChangeCurrentDirectory(cwd))?;+  }+  Ok(())+}+ impl StdioOrFd {   pub fn as_stdio(&self) -> Result<StdStdio, ProcessError> {     match &self {@@ -661,21 +765,41 @@ fn create_command(     command.args(args.args);   } +  #[cfg(unix)]+  let uid = args.uid;+  #[cfg(unix)]+  let gid = args.gid;+  #[cfg(unix)]+  let move_cwd_to_pre_exec =+    should_change_cwd_in_pre_exec(uid, gid, run_env.set_cwd_on_command);+  // Rust applies built-in uid/gid changes before current_dir, but applies+  // current_dir before user pre_exec callbacks. Defer it into our callback so+  // replacing the built-in identity setup preserves uid/gid-before-cwd order.+  #[cfg(unix)]+  let pre_exec_cwd = if move_cwd_to_pre_exec {+    Some(+      CString::new(run_env.cwd.as_os_str().as_bytes()).map_err(|err| {+        ProcessError::Io(std::io::Error::new(+          std::io::ErrorKind::InvalidInput,+          err,+        ))+      })?,+    )+  } else {+    None+  };++  #[cfg(unix)]+  if run_env.set_cwd_on_command && !move_cwd_to_pre_exec {+    command.current_dir(&run_env.cwd);+  }+  #[cfg(windows)]   if run_env.set_cwd_on_command {     command.current_dir(&run_env.cwd);   }   command.env_clear();   command.envs(run_env.envs.into_iter().map(|(k, v)| (k.into_inner(), v))); -  #[cfg(unix)]-  if let Some(gid) = args.gid {-    command.gid(gid);-  }-  #[cfg(unix)]-  if let Some(uid) = args.uid {-    command.uid(uid);-  }-   if args.stdio.stdin.is_ipc() {     args.ipc = Some(0);   } else if args.input.is_some() {@@ -777,8 +901,22 @@ fn create_command(     }      let detached = args.detached;-    if detached || !fds_to_dup.is_empty() || args.gid.is_some() {+    let should_clear_groups = should_clear_supplementary_groups(uid, gid);+    if detached || !fds_to_dup.is_empty() || should_clear_groups {       command.pre_exec(move || {+        if should_clear_groups {+          // Keep the identity change in one checked sequence. Command::uid+          // clears groups itself, but does not report every failure.+          let effective_identity_will_change =+            unix_child_effective_identity_will_change(uid, gid);+          apply_unix_child_setup(+            uid,+            gid,+            pre_exec_cwd.as_deref(),+            effective_identity_will_change,+            run_unix_child_setup_step,+          )?;+        }         if detached {           libc::setsid();         }@@ -801,7 +939,6 @@ fn create_command(             }           }         }-        libc::setgroups(0, std::ptr::null());         Ok(())       });     }@@ -1994,19 +2131,6 @@ mod deprecated {       c.env(key.inner, value);     } -    #[cfg(unix)]-    // TODO(bartlomieju):-    #[allow(-      clippy::undocumented_unsafe_blocks,-      reason = "TODO: add safety comment"-    )]-    unsafe {-      c.pre_exec(|| {-        libc::setgroups(0, std::ptr::null());-        Ok(())-      });-    }-     // TODO: make this work with other resources, eg. sockets     c.stdin(run_args.stdin.as_stdio()?);     c.stdout(match run_args.stdout {@@ -2270,3 +2394,158 @@ mod deprecated {     kill(pid, &signal)   } }++#[cfg(all(test, unix))]+mod tests {+  use std::ffi::CString;++  use super::UnixChildSetupStep;+  use super::apply_unix_child_setup;+  use super::check_identity_syscall_result;+  use super::should_change_cwd_in_pre_exec;+  use super::should_clear_supplementary_groups;+  use super::unix_child_effective_identity_will_change;++  #[test]+  fn supplementary_groups_are_only_cleared_for_identity_options() {+    assert!(!should_clear_supplementary_groups(None, None));+    assert!(should_clear_supplementary_groups(Some(1000), None));+    assert!(should_clear_supplementary_groups(None, Some(1000)));+    assert!(should_clear_supplementary_groups(Some(1000), Some(1000)));+    assert!(!should_change_cwd_in_pre_exec(None, None, true));+    assert!(!should_change_cwd_in_pre_exec(Some(1000), None, false));+    assert!(should_change_cwd_in_pre_exec(Some(1000), None, true));+    assert!(should_change_cwd_in_pre_exec(None, Some(1000), true));++    // SAFETY: getegid and geteuid are async-signal-safe and cannot fail.+    let (current_uid, current_gid) =+      unsafe { (libc::geteuid() as u32, libc::getegid() as u32) };+    assert!(!unix_child_effective_identity_will_change(+      Some(current_uid),+      Some(current_gid)+    ));+    assert!(unix_child_effective_identity_will_change(+      Some(current_uid.wrapping_add(1)),+      Some(current_gid)+    ));+    assert!(unix_child_effective_identity_will_change(+      Some(current_uid),+      Some(current_gid.wrapping_add(1))+    ));+  }++  #[test]+  fn identity_syscall_failure_returns_os_error() {+    // SAFETY: -1 is not a valid file descriptor, so this only sets errno.+    assert_eq!(unsafe { libc::close(-1) }, -1);+    let error = check_identity_syscall_result(-1).unwrap_err();+    assert_eq!(error.raw_os_error(), Some(libc::EBADF));+  }++  #[test]+  fn identity_change_clears_groups_before_setting_ids() {+    let cwd = CString::new("/work").unwrap();+    let mut steps = Vec::new();+    apply_unix_child_setup(+      Some(1000),+      Some(2000),+      Some(cwd.as_c_str()),+      true,+      |step| {+        steps.push(step);+        Ok(())+      },+    )+    .unwrap();+    assert_eq!(+      steps,+      [+        UnixChildSetupStep::ClearSupplementaryGroups,+        UnixChildSetupStep::SetPrimaryGroupId(2000),+        UnixChildSetupStep::SetUserId(1000),+        UnixChildSetupStep::ChangeCurrentDirectory(cwd.as_c_str()),+      ]+    );+  }++  #[test]+  fn identity_transition_stops_when_group_clear_fails() {+    let mut steps = Vec::new();+    let error =+      apply_unix_child_setup(Some(1000), Some(2000), None, true, |step| {+        steps.push(step);+        Err(std::io::Error::from_raw_os_error(libc::EPERM))+      })+      .unwrap_err();+    assert_eq!(steps, [UnixChildSetupStep::ClearSupplementaryGroups]);+    assert_eq!(error.raw_os_error(), Some(libc::EPERM));+  }++  #[test]+  fn unchanged_identity_tolerates_group_clear_permission_denied() {+    let mut steps = Vec::new();+    apply_unix_child_setup(Some(1000), Some(2000), None, false, |step| {+      steps.push(step);+      if matches!(step, UnixChildSetupStep::ClearSupplementaryGroups) {+        Err(std::io::Error::from_raw_os_error(libc::EPERM))+      } else {+        Ok(())+      }+    })+    .unwrap();+    assert_eq!(+      steps,+      [+        UnixChildSetupStep::ClearSupplementaryGroups,+        UnixChildSetupStep::SetPrimaryGroupId(2000),+        UnixChildSetupStep::SetUserId(1000),+      ]+    );+  }++  #[test]+  fn unchanged_identity_stops_on_other_group_clear_errors() {+    let mut steps = Vec::new();+    let error =+      apply_unix_child_setup(Some(1000), Some(2000), None, false, |step| {+        steps.push(step);+        Err(std::io::Error::from_raw_os_error(libc::EIO))+      })+      .unwrap_err();+    assert_eq!(steps, [UnixChildSetupStep::ClearSupplementaryGroups]);+    assert_eq!(error.raw_os_error(), Some(libc::EIO));+  }++  #[test]+  fn identity_change_returns_chdir_error_after_setting_ids() {+    let cwd = CString::new("/missing").unwrap();+    let mut steps = Vec::new();+    let error = apply_unix_child_setup(+      Some(1000),+      Some(2000),+      Some(cwd.as_c_str()),+      true,+      |step| {+        let is_chdir =+          matches!(step, UnixChildSetupStep::ChangeCurrentDirectory(_));+        steps.push(step);+        if is_chdir {+          Err(std::io::Error::from_raw_os_error(libc::ENOENT))+        } else {+          Ok(())+        }+      },+    )+    .unwrap_err();+    assert_eq!(+      steps,+      [+        UnixChildSetupStep::ClearSupplementaryGroups,+        UnixChildSetupStep::SetPrimaryGroupId(2000),+        UnixChildSetupStep::SetUserId(1000),+        UnixChildSetupStep::ChangeCurrentDirectory(cwd.as_c_str()),+      ]+    );+    assert_eq!(error.raw_os_error(), Some(libc::ENOENT));+  }+}

@@ -848,7 +848,12 @@ Deno.test(       args: ["-u"],     }).output(); -    const currentUid = new TextDecoder().decode(stdout);+    const currentUid = new TextDecoder().decode(stdout).trim();+    const sameUid = await new Deno.Command("id", {+      args: ["-u"],+      uid: Number(currentUid),+    }).output();+    assertEquals(new TextDecoder().decode(sameUid.stdout).trim(), currentUid);      if (currentUid !== "0") {       await assertRejects(async () => {@@ -902,7 +907,12 @@ Deno.test(       args: ["-u"],     }).outputSync(); -    const currentUid = new TextDecoder().decode(stdout);+    const currentUid = new TextDecoder().decode(stdout).trim();+    const sameUid = new Deno.Command("id", {+      args: ["-u"],+      uid: Number(currentUid),+    }).outputSync();+    assertEquals(new TextDecoder().decode(sameUid.stdout).trim(), currentUid);      if (currentUid !== "0") {       assertThrows(() => {@@ -925,7 +935,12 @@ Deno.test(       args: ["-g"],     }).output(); -    const currentGid = new TextDecoder().decode(stdout);+    const currentGid = new TextDecoder().decode(stdout).trim();+    const sameGid = await new Deno.Command("id", {+      args: ["-g"],+      gid: Number(currentGid),+    }).output();+    assertEquals(new TextDecoder().decode(sameGid.stdout).trim(), currentGid);      if (currentGid !== "0") {       await assertRejects(async () => {@@ -948,7 +963,12 @@ Deno.test(       args: ["-g"],     }).outputSync(); -    const currentGid = new TextDecoder().decode(stdout);+    const currentGid = new TextDecoder().decode(stdout).trim();+    const sameGid = new Deno.Command("id", {+      args: ["-g"],+      gid: Number(currentGid),+    }).outputSync();+    assertEquals(new TextDecoder().decode(sameGid.stdout).trim(), currentGid);      if (currentGid !== "0") {       assertThrows(() => {