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::format_usage;
 use uucore::translate;
-use uucore::{format_usage, show_error};
 
 static NOHUP_OUT: &str = "nohup.out";
 // exit codes that match the GNU implementation
@@ -44,21 +46,46 @@ enum NohupError {
 
     #[error("{}", translate!("nohup-error-open-failed-both", "first_path" => NOHUP_OUT.quote(), "first_err" => _1, "second_path" => _2.quote(), "second_err" => _3))]
     OpenFailed2(i32, #[source] Error, String, Error),
+
+    #[error("")]
+    StderrWriteFailed(i32),
 }
 
 impl UError for NohupError {
     fn code(&self) -> i32 {
         match self {
-            Self::OpenFailed(code, _) | Self::OpenFailed2(code, _, _, _) => *code,
+            Self::OpenFailed(code, _)
+            | Self::OpenFailed2(code, _, _, _)
+            | Self::StderrWriteFailed(code) => *code,
             _ => 2,
         }
     }
 }
 
+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.
+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(NohupError::StderrWriteFailed(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 +127,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 +167,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}"
+        );
+    }
+}