nohup: added a bunch of nohup fixes based on the failures in the GNU tests

src/uu/nohup/Cargo.toml

@@ -20,6 +20,7 @@ path = "src/nohup.rs"
 [dependencies]
 clap = { workspace = true }
 libc = { workspace = true }
+nix = { workspace = true, features = ["fs"] }
 uucore = { workspace = true, features = ["fs"] }
 thiserror = { workspace = true }
 fluent = { workspace = true }

src/uu/nohup/locales/en-US.ftl

@@ -14,4 +14,6 @@ nohup-error-open-failed-both = failed to open { $first_path }: { $first_err }
  failed to open { $second_path }: { $second_err }
 
 # Status messages
+nohup-ignoring-input = ignoring input
+nohup-appending-output = appending output to { $path }
 nohup-ignoring-input-appending-output = ignoring input and appending output to { $path }

src/uu/nohup/src/nohup.rs

@@ -7,18 +7,20 @@
 
 use clap::{Arg, ArgAction, Command};
 use libc::{SIG_IGN, SIGHUP, dup2, signal};
+use nix::sys::stat::{Mode, umask};
 use std::env;
 use std::fs::{File, OpenOptions};
-use std::io::{Error, ErrorKind, IsTerminal};
+use std::io::{Error, ErrorKind, IsTerminal, Write};
+use std::os::unix::fs::OpenOptionsExt;
 use std::os::unix::prelude::*;
 use std::os::unix::process::CommandExt;
 use std::path::{Path, PathBuf};
 use std::process;
 use thiserror::Error;
 use uucore::display::Quotable;
-use uucore::error::{UError, UResult, set_exit_code};
+use uucore::error::{ExitCode, UError, UResult, set_exit_code};
+use uucore::format_usage;
 use uucore::translate;
-use uucore::{format_usage, show_error};
 
 static NOHUP_OUT: &str = "nohup.out";
 // exit codes that match the GNU implementation
@@ -55,10 +57,31 @@ impl UError for NohupError {
  }
 }
 
+fn failure_code() -> i32 {
+ match env::var("POSIXLY_CORRECT") {
+ Ok(_) => POSIX_NOHUP_FAILURE,
+ Err(_) => EXIT_CANCELED,
+ }
+}
+
+/// We are unable to use the regular show_error because we need to detect if stderr
+/// is unavailable because GNU nohup exits with 125 if it can't write to stderr.
+/// When stderr is unavailable, we use ExitCode to exit silently with the appropriate code.
+fn write_stderr(msg: &str) -> UResult<()> {
+ let mut stderr = std::io::stderr();
+ if writeln!(stderr, "nohup: {msg}").is_err() || stderr.flush().is_err() {
+ return Err(ExitCode(failure_code()).into());
+ }
+ Ok(())
+}
+
 #[uucore::main]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
- let matches =
- uucore::clap_localization::handle_clap_result_with_exit_code(uu_app(), args, 125)?;
+ let matches = uucore::clap_localization::handle_clap_result_with_exit_code(
+ uu_app(),
+ args,
+ failure_code(),
+ )?;
 
  replace_fds()?;
 
@@ -100,21 +123,38 @@ pub fn uu_app() -> Command {
 }
 
 fn replace_fds() -> UResult<()> {
- if std::io::stdin().is_terminal() {
+ let stdin_is_terminal = std::io::stdin().is_terminal();
+ let stdout_is_terminal = std::io::stdout().is_terminal();
+
+ if stdin_is_terminal {
  let new_stdin = File::open(Path::new("/dev/null"))
  .map_err(|e| NohupError::CannotReplace("STDIN", e))?;
  if unsafe { dup2(new_stdin.as_raw_fd(), 0) } != 0 {
  return Err(NohupError::CannotReplace("STDIN", Error::last_os_error()).into());
  }
  }
 
- if std::io::stdout().is_terminal() {
- let new_stdout = find_stdout()?;
+ if stdout_is_terminal {
+ let (new_stdout, path) = find_stdout()?;
  let fd = new_stdout.as_raw_fd();
 
+ // Print the appropriate message based on what we're doing
+ // Use write_stderr to detect write failures (e.g., /dev/full)
+ if stdin_is_terminal {
+ write_stderr(&translate!(
+ "nohup-ignoring-input-appending-output",
+ "path" => path.quote()
+ ))?;
+ } else {
+ write_stderr(&translate!("nohup-appending-output", "path" => path.quote()))?;
+ }
+
  if unsafe { dup2(fd, 1) } != 1 {
  return Err(NohupError::CannotReplace("STDOUT", Error::last_os_error()).into());
  }
+ } else if stdin_is_terminal {
+ // Only ignoring input, not redirecting stdout
+ write_stderr(&translate!("nohup-ignoring-input"))?;
  }
 
  if std::io::stderr().is_terminal() && unsafe { dup2(1, 2) } != 2 {
@@ -123,46 +163,39 @@ fn replace_fds() -> UResult<()> {
  Ok(())
 }
 
-fn find_stdout() -> UResult<File> {
- let internal_failure_code = match env::var("POSIXLY_CORRECT") {
- Ok(_) => POSIX_NOHUP_FAILURE,
-  Err(_) => EXIT_CANCELED,
- };
+/// Open nohup.out file with mode 0o600, temporarily clearing umask.
+/// The umask is cleared to ensure the file is created with exactly 0o600 permissions.
+fn open_nohup_file(path: &Path) -> std::io::Result<File> {
+ // Clear umask (set it to 0) and save the old value
+ let old_umask = umask(Mode::from_bits_truncate(0));
 
- match OpenOptions::new()
+ let result = OpenOptions::new()
  .create(true)
  .append(true)
- .open(Path::new(NOHUP_OUT))
- {
- Ok(t) => {
- show_error!(
- "{}",
- translate!("nohup-ignoring-input-appending-output", "path" => NOHUP_OUT.quote())
- );
- Ok(t)
- }
+ .mode(0o600)
+ .open(path);
+
+ // Restore previous umask
+ umask(old_umask);
+
+ result
+}
+
+fn find_stdout() -> UResult<(File, String)> {
+ let exit_code = failure_code();
+
+ match open_nohup_file(Path::new(NOHUP_OUT)) {
+ Ok(t) => Ok((t, NOHUP_OUT.to_string())),
  Err(e1) => {
  let Ok(home) = env::var("HOME") else {
- return Err(NohupError::OpenFailed(internal_failure_code, e1).into());
+ return Err(NohupError::OpenFailed(exit_code, e1).into());
  };
  let mut homeout = PathBuf::from(home);
  homeout.push(NOHUP_OUT);
- let homeout_str = homeout.to_str().unwrap();
- match OpenOptions::new().create(true).append(true).open(&homeout) {
- Ok(t) => {
- show_error!(
- "{}",
- translate!("nohup-ignoring-input-appending-output", "path" => homeout_str.quote())
- );
- Ok(t)
- }
- Err(e2) => Err(NohupError::OpenFailed2(
- internal_failure_code,
- e1,
- homeout_str.to_string(),
- e2,
- )
- .into()),
+ let homeout_str = homeout.to_str().unwrap().to_string();
+ match open_nohup_file(&homeout) {
+ Ok(t) => Ok((t, homeout_str)),
+ Err(e2) => Err(NohupError::OpenFailed2(exit_code, e1, homeout_str, e2).into()),
  }
  }
  }

tests/by-util/test_nohup.rs

@@ -3,21 +3,18 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 // spell-checker:ignore winsize Openpty openpty xpixel ypixel ptyprocess
+use std::os::unix::fs::PermissionsExt;
 use std::thread::sleep;
 use uutests::at_and_ucmd;
 use uutests::new_ucmd;
+use uutests::util::TerminalSimulation;
 use uutests::util::TestScenario;
 use uutests::util_name;
 
 // General observation: nohup.out will not be created in tests run by cargo test
 // because stdin/stdout is not attached to a TTY.
 // All that can be tested is the side-effects.
 
-#[test]
-fn test_invalid_arg() {
- new_ucmd!().arg("--definitely-invalid").fails_with_code(125);
-}
-
 #[test]
 #[cfg(any(
  target_os = "linux",
@@ -238,3 +235,122 @@ fn test_nohup_stderr_to_stdout() {
  assert!(content.contains("stdout message"));
  assert!(content.contains("stderr message"));
 }
+
+#[test]
+fn test_nohup_file_permissions_ignore_umask_always_o600() {
+ for umask_val in [0o077, 0o000] {
+ let ts = TestScenario::new(util_name!());
+ ts.ucmd()
+ .terminal_sim_stdio(TerminalSimulation {
+ stdin: true,
+ stdout: true,
+ stderr: true,
+ size: None,
+ })
+ .umask(umask_val)
+ .args(&["echo", "test"])
+ .succeeds();
+
+ sleep(std::time::Duration::from_millis(10));
+ let mode = std::fs::metadata(ts.fixtures.plus_as_string("nohup.out"))
+ .unwrap()
+ .permissions()
+ .mode()
+ & 0o777;
+ assert_eq!(mode, 0o600, "with umask {umask_val:o}, got mode {mode:o}");
+ }
+}
+
+#[test]
+fn test_nohup_exit_codes() {
+ // No args: 125 default, 127 with POSIXLY_CORRECT
+ new_ucmd!().fails_with_code(125);
+ new_ucmd!().env("POSIXLY_CORRECT", "1").fails_with_code(127);
+
+ // Invalid arg: 125 default, 127 with POSIXLY_CORRECT
+ new_ucmd!().arg("--invalid").fails_with_code(125);
+ new_ucmd!()
+ .env("POSIXLY_CORRECT", "1")
+ .arg("--invalid")
+ .fails_with_code(127);
+}
+
+#[test]
+fn test_nohup_messages_by_terminal_state() {
+ let cases = [
+ (true, true, "ignoring input and appending output to", ""),
+ (false, true, "appending output to", "ignoring input"),
+ (true, false, "ignoring input", "appending output"),
+ ];
+
+ for (stdin_tty, stdout_tty, expected, not_expected) in cases {
+ let ts = TestScenario::new(util_name!());
+ let result = ts
+ .ucmd()
+ .terminal_sim_stdio(TerminalSimulation {
+ stdin: stdin_tty,
+ stdout: stdout_tty,
+ stderr: true,
+ size: None,
+ })
+ .args(&["echo", "test"])
+ .succeeds();
+
+ let stderr = String::from_utf8_lossy(result.stderr());
+ assert!(
+ stderr.contains(expected),
+ "stdin={stdin_tty}, stdout={stdout_tty}: expected '{expected}'"
+ );
+ if !not_expected.is_empty() {
+ assert!(
+ !stderr.contains(not_expected),
+ "stdin={stdin_tty}, stdout={stdout_tty}: unexpected '{not_expected}'"
+ );
+ }
+ }
+}
+
+#[test]
+fn test_nohup_no_message_without_tty() {
+ new_ucmd!()
+ .args(&["echo", "test"])
+ .succeeds()
+ .stderr_does_not_contain("ignoring input")
+ .stderr_does_not_contain("appending output");
+}
+
+#[test]
+#[cfg(target_os = "linux")]
+fn test_nohup_stderr_write_failure() {
+ use std::fs::OpenOptions;
+
+ if !std::path::Path::new("/dev/full").exists() {
+ return;
+ }
+
+ for (posixly_correct, expected_code) in [(false, 125), (true, 127)] {
+ let Ok(dev_full) = OpenOptions::new().write(true).open("/dev/full") else {
+ return;
+ };
+ let mut cmd = new_ucmd!();
+ if posixly_correct {
+ cmd.env("POSIXLY_CORRECT", "1");
+ }
+ let result = cmd
+ .terminal_sim_stdio(TerminalSimulation {
+ stdin: true,
+ stdout: true,
+ stderr: false,
+ size: None,
+ })
+ .set_stderr(dev_full)
+ .args(&["echo", "test"])
+ .fails();
+
+ assert_eq!(
+ result.try_exit_status().and_then(|s| s.code()),
+ Some(expected_code),
+ "POSIXLY_CORRECT={posixly_correct}"
+ );
+ }
+}