diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index 790293423f..53607ecd2b 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -19,7 +19,7 @@ use std::fmt::Debug; use std::fs::{self, metadata}; use std::fs::{File, OpenOptions}; use std::io::{Write, stdout}; -use std::os::unix::fs::MetadataExt; +use std::os::unix::fs::{DirBuilderExt, MetadataExt}; use std::os::unix::prelude::OsStrExt; use std::path::{MAIN_SEPARATOR, Path, PathBuf}; use std::process; @@ -210,10 +210,12 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let behavior = behavior(&matches, diag_args.as_deref())?; - match behavior.main_function { + // Like GNU, work with the umask cleared: new ancestor directories get + // exactly DEFAULT_MODE and the strip program sees a umask of 0. + uucore::mode::with_umask(0, || match behavior.main_function { MainFunction::Directory => directory(&paths, &behavior), MainFunction::Standard => standard(paths, &behavior), - } + }) } pub fn uu_app() -> Command { @@ -520,11 +522,11 @@ fn directory(paths: &[OsString], b: &Behavior) -> UResult<()> { // create all ancestors (or components) of a directory // regardless of the presence of the "-D" flag. // - // NOTE: the GNU "install" sets the expected mode only for the - // target directory. All created ancestor directories will have - // the default mode. Hence it is safe to use fs::create_dir_all - // and then only modify the target's dir mode. - if let Err(e) = fs::create_dir_all(&path_to_create).map_err_context( + // Only the target gets the requested mode; newly created ancestors + // use DEFAULT_MODE, which create_dir_all would request as 0777. + let mut builder = fs::DirBuilder::new(); + builder.recursive(true).mode(DEFAULT_MODE); + if let Err(e) = builder.create(&path_to_create).map_err_context( || translate!("install-error-create-dir-failed", "path" => path_to_create.quote()), ) { show!(e); @@ -727,8 +729,6 @@ fn standard(mut paths: Vec, b: &Behavior) -> UResult<()> { #[cfg(not(target_os = "redox"))] { - // Use DEFAULT_MODE (0o755) for created directories - this matches GNU install - // behavior. The actual mode will be modified by umask at the kernel level. match create_dir_all_safe(to_create, DEFAULT_MODE) { Ok(dir_fd) => { if b.target_dir.is_none() @@ -769,7 +769,10 @@ fn standard(mut paths: Vec, b: &Behavior) -> UResult<()> { } } #[cfg(target_os = "redox")] - fs::create_dir_all(to_create) + fs::DirBuilder::new() + .recursive(true) + .mode(DEFAULT_MODE) + .create(to_create) .map_err(|e| InstallError::CreateDirFailed(to_create.to_path_buf(), e))?; } } diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index 8bcf4f656b..038450c070 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -113,12 +113,9 @@ fn test_install_ancestors_mode_directories() { let target_dir = "ancestor1/ancestor2/target_dir"; let directories_arg = "-d"; let mode_arg = "--mode=200"; - let probe = "probe"; - at.mkdir(probe); - let default_perms = at.metadata(probe).permissions().mode(); - - ucmd.args(&[mode_arg, directories_arg, target_dir]) + ucmd.umask(0o077) + .args(&[mode_arg, directories_arg, target_dir]) .succeeds() .no_stderr(); @@ -126,11 +123,10 @@ fn test_install_ancestors_mode_directories() { assert!(at.dir_exists(ancestor2)); assert!(at.dir_exists(target_dir)); - assert_eq!(default_perms, at.metadata(ancestor1).permissions().mode()); - assert_eq!(default_perms, at.metadata(ancestor2).permissions().mode()); - - // Expected mode only on the target_dir. - assert_eq!(0o40_200_u32, at.metadata(target_dir).permissions().mode()); + // Ancestors use the default mode; --mode applies only to the target. + assert_eq!(0o755, at.metadata(ancestor1).permissions().mode() & 0o777); + assert_eq!(0o755, at.metadata(ancestor2).permissions().mode() & 0o777); + assert_eq!(0o200, at.metadata(target_dir).permissions().mode() & 0o777); } #[test] @@ -142,14 +138,11 @@ fn test_install_ancestors_mode_directories_with_file() { let directories_arg = "-D"; let mode_arg = "--mode=200"; let file = "file"; - let probe = "probe"; - - at.mkdir(probe); - let default_perms = at.metadata(probe).permissions().mode(); at.touch(file); - ucmd.args(&[mode_arg, directories_arg, file, target_file]) + ucmd.umask(0o077) + .args(&[mode_arg, directories_arg, file, target_file]) .succeeds() .no_stderr(); @@ -157,11 +150,10 @@ fn test_install_ancestors_mode_directories_with_file() { assert!(at.dir_exists(ancestor2)); assert!(at.file_exists(target_file)); - assert_eq!(default_perms, at.metadata(ancestor1).permissions().mode()); - assert_eq!(default_perms, at.metadata(ancestor2).permissions().mode()); - - // Expected mode only on the target_file. - assert_eq!(0o100_200_u32, at.metadata(target_file).permissions().mode()); + // Ancestors use the default mode; --mode applies only to the target. + assert_eq!(0o755, at.metadata(ancestor1).permissions().mode() & 0o777); + assert_eq!(0o755, at.metadata(ancestor2).permissions().mode() & 0o777); + assert_eq!(0o200, at.metadata(target_file).permissions().mode() & 0o777); } #[test] @@ -846,6 +838,7 @@ fn test_install_and_strip() { scene .ucmd() + .umask(0o777) .env("PATH", path) .arg("-s") .arg("source") @@ -856,6 +849,69 @@ fn test_install_and_strip() { assert_eq!(at.metadata(STRIP_TARGET_FILE).len(), 0); } +#[test] +fn test_install_strip_program_sees_zeroed_umask() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + // Write in a child to avoid inherited writable script fds (see write_strip_program). + scene + .cmd("sh") + .env("STRIP_SCRIPT", "#!/bin/sh\numask > \"$1\"\n") + .arg("-c") + .arg("printf '%s' \"$STRIP_SCRIPT\" > probe && chmod 755 probe") + .succeeds(); + at.write("source", "file contents"); + + scene + .ucmd() + .umask(0o077) + .args(&[ + "-s", + "--strip-program", + "./probe", + "source", + STRIP_TARGET_FILE, + ]) + .succeeds() + .no_stderr(); + + // Follow GNU's behavior: the strip program sees a zeroed umask. Shells + // differ on whether they print it as 000 or 0000, so compare the value. + let reported = at.read(STRIP_TARGET_FILE); + let reported = reported.trim(); + assert_eq!( + u32::from_str_radix(reported, 8).unwrap(), + 0, + "strip program saw umask {reported}" + ); +} + +#[test] +fn test_install_and_strip_with_ancestor_directories() { + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + write_strip_program(&scene, "strip"); + at.write("source", "file contents"); + let path = format!( + "{}:{}", + at.plus(".").display(), + std::env::var("PATH").unwrap_or_default() + ); + let target = "ancestor/helloworld_installed"; + + scene + .ucmd() + .umask(0o777) + .env("PATH", path) + .args(&["-s", "-D", "source", target]) + .succeeds() + .no_stderr(); + + assert_eq!(at.metadata(target).len(), 0); +} + #[test] fn test_install_no_strip_with_program() { let source = current_exe().unwrap();