Fourth batch of FreeBSD changes for the compliance test suite (#915)

test-framework/e2e-tests/src/pty.rs

@@ -37,7 +37,7 @@ fn do_test(binary: Binary, not_use_pty: bool, user_tty: bool, expected: ExecMode
     let mut cmd = match binary {
         Binary::Su => {
             let mut cmd = Command::new("su");
-            cmd.args(["-c", "sh -c 'touch /tmp/barrier; sleep 3; true'"]);
+            cmd.args(["-c", "touch /tmp/barrier; sleep 3; true"]);
             cmd
         }
 

test-framework/e2e-tests/src/su/flag_pty.rs

@@ -17,7 +17,7 @@ fn fixture() -> Result<Processes> {
 
     let child = Command::new("su")
         .args(["--pty", "-c"])
-        .arg("sh -c 'touch /tmp/barrier; sleep 3; true'")
+        .arg("touch /tmp/barrier; sleep 3")
         .tty(true)
         .spawn(&env)?;
 
@@ -35,9 +35,7 @@ fn fixture() -> Result<Processes> {
 
     let mut su_related_processes = entries
         .into_iter()
-        .filter(|entry| {
-            entry.command.contains("sh -c 'touch") | entry.command.starts_with("sh -c touch")
-        })
+        .filter(|entry| entry.command.contains("touch"))
         .collect::<Vec<_>>();
 
     su_related_processes.sort_by_key(|entry| entry.pid);

test-framework/e2e-tests/src/su/signal_handling/mod.rs

@@ -37,7 +37,7 @@ fn signal_is_forwarded_to_child() -> Result<()> {
 
     let child = Command::new("su")
         .arg("-c")
-        .arg(format!("sh {expects_signal} {signal}"))
+        .arg(format!("exec sh {expects_signal} {signal}"))
         .spawn(&env)?;
 
     Command::new("sh")
@@ -60,7 +60,7 @@ fn child_terminated_by_signal() -> Result<()> {
     // child process sends SIGTERM to itself
     let output = Command::new("su")
         .arg("-c")
-        .arg("sh -c 'kill $$'")
+        .arg("kill $$")
         .output(&env)?;
 
     assert_eq!(Some(143), output.status().code());
@@ -122,7 +122,7 @@ fn sigalrm_terminates_command() -> Result<()> {
 
     let child = Command::new("su")
         .arg("-c")
-        .arg(format!("sh {expects_signal} HUP TERM"))
+        .arg(format!("exec sh {expects_signal} HUP TERM"))
         .spawn(&env)?;
 
     Command::new("sh")

test-framework/e2e-tests/src/su/signal_handling/sigtstp.bash

@@ -4,7 +4,7 @@ set -e
 # enable 'job control' to make `fg` work
 set -m
 
-su -c "sh -c 'for i in "'$(seq 1 5)'"; do date +%s; sleep 1; done'" &
+su -c "exec sh -c 'for i in "'$(seq 1 5)'"; do date +%s; sleep 1; done'" &
 sleep 2
 kill -TSTP $!
 sleep 5

test-framework/sudo-compliance-tests/src/su/cli.rs

@@ -56,7 +56,7 @@ echo "${@}""#;
 
 #[test]
 fn flag_after_positional_argument() -> Result<()> {
-    let expected = "-bash";
+    let expected = "-sh";
     let env = Env("").build()?;
     let stdout = Command::new("env")
         .args(["su", "-c", "echo $0", "root", "-l"])

test-framework/sudo-compliance-tests/src/su/flag_login.rs

@@ -14,7 +14,7 @@ fn it_works() -> Result<()> {
         let actual = Command::new("su").args(args).output(&env)?.stdout()?;
 
         // argv[0] is prefixed with '-' to invoke the shell as a login shell
-        assert_eq!("-bash", actual);
+        assert_eq!("-sh", actual);
     }
 
     Ok(())
@@ -151,7 +151,7 @@ fn may_be_specified_more_than_once_without_change_in_semantics() -> Result<()> {
         let actual = Command::new("su").args(args).output(&env)?.stdout()?;
 
         // argv[0] is prefixed with '-' to invoke the shell as a login shell
-        assert_eq!("-bash", actual);
+        assert_eq!("-sh", actual);
     }
 
     Ok(())

test-framework/sudo-compliance-tests/src/su/flag_shell.rs

@@ -89,7 +89,7 @@ fn ignores_shell_env_var_when_flag_preserve_environment_is_absent() -> Result<()
         .output(&env)?
         .stdout()?;
 
-    assert_eq!("/bin/bash", stdout);
+    assert_eq!("/bin/sh", stdout);
 
     Ok(())
 }

test-framework/sudo-compliance-tests/src/sudo/env_reset.rs

@@ -56,7 +56,7 @@ fn some_vars_are_set() -> Result<()> {
 
     // # man sudoers
     // "The HOME, MAIL, SHELL, LOGNAME and USER environment variables are initialized based on the target user"
-    assert_eq!(Some("/bin/bash"), sudo_env.remove("SHELL"));
+    assert_eq!(Some("/bin/sh"), sudo_env.remove("SHELL"));
 
     // "If the PATH and TERM variables are not preserved from the user's environment, they will be set to default values."
     let sudo_path = sudo_env.remove("PATH").expect("PATH not set");
@@ -199,7 +199,7 @@ fn some_vars_are_preserved() -> Result<()> {
     // not preserved
     assert_eq!(Some("/root"), sudo_env.remove("HOME"));
     assert_eq!(Some("/var/mail/root"), sudo_env.remove("MAIL"));
-    assert_eq!(Some("/bin/bash"), sudo_env.remove("SHELL"));
+    assert_eq!(Some("/bin/sh"), sudo_env.remove("SHELL"));
     assert_eq!(Some("root"), sudo_env.remove("LOGNAME"));
     assert_eq!(Some("root"), sudo_env.remove("USER"));
     assert_eq!(

test-framework/sudo-compliance-tests/src/sudo/flag_chdir.rs

@@ -191,7 +191,7 @@ fn target_user_has_insufficient_perms() -> Result<()> {
 
 #[test]
 fn flag_login_is_respected() -> Result<()> {
-    let expected = "-bash";
+    let expected = "-sh";
     let env = Env("ALL ALL=(ALL:ALL) CWD=* ALL").build()?;
 
     let output = Command::new("sh")

test-framework/sudo-compliance-tests/src/sudo/flag_list.rs

@@ -261,7 +261,7 @@ Sudoers entry:
 
 #[test]
 fn when_command_is_specified_the_fully_qualified_path_is_displayed() -> Result<()> {
-    let env = Env("ALL ALL=(ALL:ALL) NOPASSWD: /bin/true")
+    let env = Env("ALL ALL=(ALL:ALL) NOPASSWD: /usr/bin/true")
         .user(USERNAME)
         .build()?;
 
@@ -282,7 +282,7 @@ fn when_command_is_specified_the_fully_qualified_path_is_displayed() -> Result<(
 
 #[test]
 fn when_several_commands_specified_only_first_displayed_with_fully_qualified_path() -> Result<()> {
-    let env = Env("ALL ALL=(ALL:ALL) NOPASSWD: /bin/true, /bin/ls")
+    let env = Env("ALL ALL=(ALL:ALL) NOPASSWD: /usr/bin/true, /bin/ls")
         .user(USERNAME)
         .build()?;
 

test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/mod.rs

@@ -5,6 +5,7 @@ use crate::{Result, HOSTNAME};
 macro_rules! assert_snapshot {
     ($($tt:tt)*) => {
         insta::with_settings!({
+            filters => vec![(BIN_LS, "<BIN_LS>")],
             prepend_module_to_snapshot => false,
         }, {
             insta::assert_snapshot!($($tt)*)

test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/command_alias.snap

@@ -1,13 +1,12 @@
 ---
 source: sudo-compliance-tests/src/sudo/flag_list/long_format/mod.rs
-assertion_line: 131
 expression: stdout
 ---
 User root may run the following commands on container:
 
 Sudoers entry:
     RunAsUsers: root
     Commands:
-	/usr/bin/ls
+	<BIN_LS>
 	/usr/bin/true
 	/usr/bin/false

test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/cwd_override_across_runas_groups.snap

@@ -1,6 +1,5 @@
 ---
 source: sudo-compliance-tests/src/sudo/flag_list/long_format/mod.rs
-assertion_line: 227
 expression: stdout
 ---
 User root may run the following commands on container:
@@ -21,4 +20,4 @@ Sudoers entry:
     RunAsUsers: ferris
     Cwd: /home
     Commands:
-	/usr/bin/ls
+	<BIN_LS>

test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/multiple_lines.snap

@@ -1,6 +1,5 @@
 ---
 source: sudo-compliance-tests/src/sudo/flag_list/long_format/mod.rs
-assertion_line: 302
 expression: stdout
 ---
 User root may run the following commands on container:
@@ -14,4 +13,4 @@ Sudoers entry:
 Sudoers entry:
     RunAsUsers: root
     Commands:
-	/usr/bin/ls
+	<BIN_LS>

test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/negated_command_alias.snap

@@ -1,13 +1,12 @@
 ---
 source: sudo-compliance-tests/src/sudo/flag_list/long_format/mod.rs
-assertion_line: 141
 expression: stdout
 ---
 User root may run the following commands on container:
 
 Sudoers entry:
     RunAsUsers: root
     Commands:
-	/usr/bin/ls
+	<BIN_LS>
 	!/usr/bin/true
 	/usr/bin/false

test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/nopasswd_passwd_override_across_runas_groups.snap

@@ -1,6 +1,5 @@
 ---
 source: sudo-compliance-tests/src/sudo/flag_list/long_format/mod.rs
-assertion_line: 285
 expression: stdout
 ---
 User root may run the following commands on container:
@@ -21,4 +20,4 @@ Sudoers entry:
     RunAsUsers: ferris
     Options: authenticate
     Commands:
-	/usr/bin/ls
+	<BIN_LS>

test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/mod.rs

@@ -5,6 +5,7 @@ use crate::{Result, HOSTNAME};
 macro_rules! assert_snapshot {
     ($($tt:tt)*) => {
         insta::with_settings!({
+            filters => vec![(BIN_LS, "<BIN_LS>")],
             prepend_module_to_snapshot => false,
         }, {
             insta::assert_snapshot!($($tt)*)

test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/command_alias.snap

@@ -1,7 +1,6 @@
 ---
 source: sudo-compliance-tests/src/sudo/flag_list/short_format/mod.rs
-assertion_line: 128
 expression: stdout
 ---
 User root may run the following commands on container:
-    (root) /usr/bin/ls, /usr/bin/true, /usr/bin/false
+    (root) <BIN_LS>, /usr/bin/true, /usr/bin/false

test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/cwd_override_across_runas_groups.snap

@@ -1,8 +1,7 @@
 ---
 source: sudo-compliance-tests/src/sudo/flag_list/short_format/mod.rs
-assertion_line: 169
 expression: stdout
 ---
 User root may run the following commands on container:
     (root) CWD=* /usr/bin/true
-    (ferris) CWD=* /usr/bin/false, CWD=/home /usr/bin/ls
+    (ferris) CWD=* /usr/bin/false, CWD=/home <BIN_LS>

test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/multiple_lines.snap

@@ -1,8 +1,7 @@
 ---
 source: sudo-compliance-tests/src/sudo/flag_list/short_format/mod.rs
-assertion_line: 245
 expression: stdout
 ---
 User root may run the following commands on container:
     (root) /usr/bin/true, /usr/bin/false
-    (root) /usr/bin/ls
+    (root) <BIN_LS>

test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/negated_command_alias.snap

@@ -1,7 +1,6 @@
 ---
 source: sudo-compliance-tests/src/sudo/flag_list/short_format/mod.rs
-assertion_line: 138
 expression: stdout
 ---
 User root may run the following commands on container:
-    (root) /usr/bin/ls, !/usr/bin/true, /usr/bin/false
+    (root) <BIN_LS>, !/usr/bin/true, /usr/bin/false

test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/nopasswd_passwd_override_across_runas_groups.snap

@@ -1,8 +1,7 @@
 ---
 source: sudo-compliance-tests/src/sudo/flag_list/short_format/mod.rs
-assertion_line: 212
 expression: stdout
 ---
 User root may run the following commands on container:
     (root) NOPASSWD: /usr/bin/true
-    (ferris) NOPASSWD: /usr/bin/false, PASSWD: /usr/bin/ls
+    (ferris) NOPASSWD: /usr/bin/false, PASSWD: <BIN_LS>

test-framework/sudo-compliance-tests/src/sudo/flag_shell.rs

@@ -20,6 +20,12 @@ fn if_shell_env_var_is_not_set_then_uses_the_invoking_users_shell_in_passwd_data
 {
     let env = Env(SUDOERS_ALL_ALL_NOPASSWD).user(USERNAME).build()?;
 
+    // Make sure the shell of root doesn't match the shell of ferris
+    let output = Command::new("chsh")
+        .args(["-s", "/non/existent"])
+        .output(&env)?;
+    assert!(output.status().success());
+
     let getent_passwd = Command::new("getent")
         .arg("passwd")
         .output(&env)?
@@ -28,7 +34,6 @@ fn if_shell_env_var_is_not_set_then_uses_the_invoking_users_shell_in_passwd_data
     let target_users_shell = user_to_shell["root"];
     let invoking_users_shell = user_to_shell["ferris"];
 
-    // XXX a bit brittle. it would be better to set ferris' shell when creating the user
     assert_ne!(target_users_shell, invoking_users_shell);
 
     let output = Command::new("env")

test-framework/sudo-compliance-tests/src/sudo/path_search.rs

@@ -126,8 +126,12 @@ fn arg0_native_is_passed_from_commandline() -> Result<()> {
         ])
         .output(&env)?;
 
-    let stderr = output.stderr();
-    assert_starts_with!(stderr, "/bin/foo: unrecognized option");
+    let mut stderr = output.stderr();
+    // On GNU, this will report "/bin/foo: ...". but FreeBSD's `ls` does not print its full invoked path in error
+    if stderr.starts_with("/bin/") {
+        stderr = &stderr[5..]
+    }
+    assert_starts_with!(stderr, "foo: unrecognized option");
 
     Ok(())
 }
@@ -175,7 +179,7 @@ fn arg0_script_is_resolved_from_commandline() -> Result<()> {
         .build()?;
 
     let output = Command::new("sh")
-        .args(["-c", &format!("ln -s {path} /bin/foo; sudo foo")])
+        .args(["-c", &format!("ln -s {path} /usr/bin/foo; sudo foo")])
         .output(&env)?;
 
     let stdout = output.stdout()?;

test-framework/sudo-compliance-tests/src/sudo/sudo_ps1.rs

@@ -27,28 +27,6 @@ fn ps1_env_var_is_set_when_sudo_ps1_is_set() -> Result<()> {
     Ok(())
 }
 
-#[test]
-fn ps1_env_var_is_not_set_when_sudo_ps1_is_set_and_flag_login_is_used() -> Result<()> {
-    let env = Env(SUDOERS_ROOT_ALL_NOPASSWD).build()?;
-
-    let sudo_abs_path = Command::new("which").arg("sudo").output(&env)?.stdout()?;
-    let env_abs_path = Command::new("which").arg("env").output(&env)?.stdout()?;
-
-    // run sudo in an empty environment
-    let stdout = Command::new("env")
-        .args(["-i", SUDO_RS_IS_UNSTABLE])
-        .arg("SUDO_PS1=abc")
-        .args([&sudo_abs_path, "-i", &env_abs_path])
-        .output(&env)?
-        .stdout()?;
-    let sudo_env = helpers::parse_env_output(&stdout)?;
-
-    assert!(!sudo_env.contains_key("PS1"));
-    assert!(!sudo_env.contains_key("SUDO_PS1"));
-
-    Ok(())
-}
-
 // sudo removes env vars whose values start with `()` but that does not affect the SUDO_PS1 feature
 #[test]
 fn can_start_with_parentheses() -> Result<()> {

test-framework/sudo-test/src/lib.rs

@@ -484,7 +484,8 @@ impl From<String> for User {
             id: None,
             name,
             password: None,
-            shell: None,
+            // Keep the shell that is used consistent across OSes
+            shell: Some("/bin/sh".to_owned()),
         }
     }
 }

test-framework/sudo-test/src/ours.freebsd.Dockerfile

@@ -8,6 +8,8 @@ RUN install -m 4755 build/sudo /usr/bin/sudo && \
     install -m 755 build/visudo /usr/sbin/visudo
 # `apt-get install sudo` creates this directory; creating it in the image saves us the work of creating it in each compliance test
 RUN mkdir -p /usr/local/etc/sudoers.d
+# Ensure we use the same shell across OSes
+RUN chsh -s /bin/sh
 # set the default working directory to somewhere world writable so sudo / su can create .profraw files there
 WORKDIR /tmp
 # Makes sure our sudo implementation actually runs

test-framework/sudo-test/src/ours.linux.Dockerfile

@@ -12,6 +12,8 @@ RUN install -m 4755 build/sudo /usr/bin/sudo && \
     install -m 755 build/visudo /usr/sbin/visudo
 # `apt-get install sudo` creates this directory; creating it in the image saves us the work of creating it in each compliance test
 RUN mkdir -p /etc/sudoers.d
+# Ensure we use the same shell across OSes
+RUN chsh -s /bin/sh
 # remove build dependencies
 RUN apt-get autoremove -y clang libclang-dev
 # set the default working directory to somewhere world writable so sudo / su can create .profraw files there

test-framework/sudo-test/src/theirs.freebsd.Dockerfile

@@ -1,5 +1,7 @@
 FROM dougrabson/freebsd14.1-small:latest
 RUN IGNORE_OSVERSION=yes pkg install -y sudo pidof sshpass rsyslog bash vim dash FreeBSD-libbsm && \
     rm /usr/local/etc/sudoers
+# Ensure we use the same shell across OSes
+RUN chsh -s /bin/sh
 # just to match `ours.Dockerfile`
 WORKDIR /tmp

test-framework/sudo-test/src/theirs.linux.Dockerfile

@@ -2,5 +2,7 @@ FROM debian:bookworm-slim
 RUN apt-get update && \
     apt-get install -y --no-install-recommends sudo procps sshpass rsyslog && \
     rm /etc/sudoers
+# Ensure we use the same shell across OSes
+RUN chsh -s /bin/sh
 # just to match `ours.Dockerfile`
 WORKDIR /tmp