From 95753667f93889a243bbc5aa5e8b2921e36714af Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:19:37 +0100 Subject: [PATCH 1/8] Remove broken test This removes a test that seemed like it checks that sudo doesn't set PS1 even when SUDO_PS1 is set in the case of a login shell. As it turns out however, this behavior is caused by bash not inheriting PS1 to it's children. If another shell is used, PS1 will be shown to be passed by sudo to the elevated process even with og sudo. --- .../src/sudo/sudo_ps1.rs | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/test-framework/sudo-compliance-tests/src/sudo/sudo_ps1.rs b/test-framework/sudo-compliance-tests/src/sudo/sudo_ps1.rs index 826942875..9b08e90bc 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/sudo_ps1.rs +++ b/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<()> { From bc74bb61925c1c5ec93a06c7bd96b5f047c733a4 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:47:36 +0100 Subject: [PATCH 2/8] Handle FreeBSD ls not using the full path as command name --- .../sudo-compliance-tests/src/sudo/path_search.rs | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/test-framework/sudo-compliance-tests/src/sudo/path_search.rs b/test-framework/sudo-compliance-tests/src/sudo/path_search.rs index e3151e925..c11a8c052 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/path_search.rs +++ b/test-framework/sudo-compliance-tests/src/sudo/path_search.rs @@ -117,17 +117,14 @@ fn paths_are_matched_using_realpath_in_arguments() -> Result<()> { #[test] fn arg0_native_is_passed_from_commandline() -> Result<()> { - let env = Env(SUDOERS_ALL_ALL_NOPASSWD).build()?; + let env = Env(SUDOERS_ALL_ALL_NOPASSWD) + .file("/usr/bin/foo", TextFile("#!/bin/sh\necho $0").chmod("755")) + .build()?; - let output = Command::new("sh") - .args([ - "-c", - "ln -s /bin/ls /bin/foo; sudo /bin/foo --invalid-flag; true", - ]) - .output(&env)?; + let output = Command::new("sudo").arg("/usr/bin/foo").output(&env)?; - let stderr = output.stderr(); - assert_starts_with!(stderr, "/bin/foo: unrecognized option"); + let stdout = output.stdout().unwrap(); + assert_starts_with!(stdout, "/usr/bin/foo"); Ok(()) } From b08a59148db2e5c1aee882b58e0f4421dc5073b5 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:48:12 +0100 Subject: [PATCH 3/8] Handle FreeBSD not symlinking /bin to /usr/bin in a couple of tests --- test-framework/sudo-compliance-tests/src/sudo/flag_list.rs | 4 ++-- test-framework/sudo-compliance-tests/src/sudo/path_search.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list.rs b/test-framework/sudo-compliance-tests/src/sudo/flag_list.rs index 7a20524ca..7f8da7cd7 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_list.rs +++ b/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()?; diff --git a/test-framework/sudo-compliance-tests/src/sudo/path_search.rs b/test-framework/sudo-compliance-tests/src/sudo/path_search.rs index c11a8c052..066855c55 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/path_search.rs +++ b/test-framework/sudo-compliance-tests/src/sudo/path_search.rs @@ -172,7 +172,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()?; From c5226844c81df368c0ad7b0876acafc59880f3c2 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 25 Nov 2024 16:19:54 +0100 Subject: [PATCH 4/8] Use a consistent shell across OSes /bin/sh is the only shell that exists by default on both Linux and FreeBSD. And also out of the installable shells the only shell which has the same installation location on both OSes. --- test-framework/sudo-compliance-tests/src/su/cli.rs | 2 +- test-framework/sudo-compliance-tests/src/su/flag_login.rs | 4 ++-- test-framework/sudo-compliance-tests/src/su/flag_shell.rs | 2 +- test-framework/sudo-compliance-tests/src/sudo/env_reset.rs | 4 ++-- .../sudo-compliance-tests/src/sudo/flag_chdir.rs | 2 +- .../sudo-compliance-tests/src/sudo/flag_shell.rs | 7 ++++++- test-framework/sudo-test/src/lib.rs | 3 ++- test-framework/sudo-test/src/ours.freebsd.Dockerfile | 2 ++ test-framework/sudo-test/src/ours.linux.Dockerfile | 2 ++ test-framework/sudo-test/src/theirs.freebsd.Dockerfile | 2 ++ test-framework/sudo-test/src/theirs.linux.Dockerfile | 2 ++ 11 files changed, 23 insertions(+), 9 deletions(-) diff --git a/test-framework/sudo-compliance-tests/src/su/cli.rs b/test-framework/sudo-compliance-tests/src/su/cli.rs index eab8ae8ce..6bc31983b 100644 --- a/test-framework/sudo-compliance-tests/src/su/cli.rs +++ b/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"]) diff --git a/test-framework/sudo-compliance-tests/src/su/flag_login.rs b/test-framework/sudo-compliance-tests/src/su/flag_login.rs index ed5bdaa76..3f7eb62b5 100644 --- a/test-framework/sudo-compliance-tests/src/su/flag_login.rs +++ b/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(()) diff --git a/test-framework/sudo-compliance-tests/src/su/flag_shell.rs b/test-framework/sudo-compliance-tests/src/su/flag_shell.rs index be098aa7b..4fb28c08e 100644 --- a/test-framework/sudo-compliance-tests/src/su/flag_shell.rs +++ b/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(()) } diff --git a/test-framework/sudo-compliance-tests/src/sudo/env_reset.rs b/test-framework/sudo-compliance-tests/src/sudo/env_reset.rs index a7191c042..8a455b5dc 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/env_reset.rs +++ b/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!( diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_chdir.rs b/test-framework/sudo-compliance-tests/src/sudo/flag_chdir.rs index 17fab7f9e..caa929167 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_chdir.rs +++ b/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") diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_shell.rs b/test-framework/sudo-compliance-tests/src/sudo/flag_shell.rs index 8e0fddb09..100a26cbd 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_shell.rs +++ b/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") diff --git a/test-framework/sudo-test/src/lib.rs b/test-framework/sudo-test/src/lib.rs index 7e9be7f62..0d2f05896 100644 --- a/test-framework/sudo-test/src/lib.rs +++ b/test-framework/sudo-test/src/lib.rs @@ -484,7 +484,8 @@ impl From for User { id: None, name, password: None, - shell: None, + // Keep the shell that is used consistent across OSes + shell: Some("/bin/sh".to_owned()), } } } diff --git a/test-framework/sudo-test/src/ours.freebsd.Dockerfile b/test-framework/sudo-test/src/ours.freebsd.Dockerfile index 715515d4c..ee0c863e5 100644 --- a/test-framework/sudo-test/src/ours.freebsd.Dockerfile +++ b/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 diff --git a/test-framework/sudo-test/src/ours.linux.Dockerfile b/test-framework/sudo-test/src/ours.linux.Dockerfile index b7780d395..b9b5e1cd8 100644 --- a/test-framework/sudo-test/src/ours.linux.Dockerfile +++ b/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 diff --git a/test-framework/sudo-test/src/theirs.freebsd.Dockerfile b/test-framework/sudo-test/src/theirs.freebsd.Dockerfile index 80a99ccc3..cda989b9b 100644 --- a/test-framework/sudo-test/src/theirs.freebsd.Dockerfile +++ b/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 diff --git a/test-framework/sudo-test/src/theirs.linux.Dockerfile b/test-framework/sudo-test/src/theirs.linux.Dockerfile index 0c4feceaf..084979596 100644 --- a/test-framework/sudo-test/src/theirs.linux.Dockerfile +++ b/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 From d5c0f78ee0547cecb2911c0c9b90f0aa06426d00 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:31:40 +0100 Subject: [PATCH 5/8] Handle non-existent signal propagation of dash If the dash shell receives a signal, it doesn't kill it's children. To handle this, make sure the sh spawned by su execs the program that should receive the signal. --- test-framework/e2e-tests/src/su/signal_handling/mod.rs | 6 +++--- .../e2e-tests/src/su/signal_handling/sigtstp.bash | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test-framework/e2e-tests/src/su/signal_handling/mod.rs b/test-framework/e2e-tests/src/su/signal_handling/mod.rs index 13ea2f457..8298ed096 100644 --- a/test-framework/e2e-tests/src/su/signal_handling/mod.rs +++ b/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") diff --git a/test-framework/e2e-tests/src/su/signal_handling/sigtstp.bash b/test-framework/e2e-tests/src/su/signal_handling/sigtstp.bash index 546f7e3b6..fa1648f60 100644 --- a/test-framework/e2e-tests/src/su/signal_handling/sigtstp.bash +++ b/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 From f664a1720a5e539109455de1b14953f188750f5b Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 26 Nov 2024 14:32:08 +0100 Subject: [PATCH 6/8] Handle differing cases where bash and dash exec the tail program --- test-framework/e2e-tests/src/pty.rs | 2 +- test-framework/e2e-tests/src/su/flag_pty.rs | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/test-framework/e2e-tests/src/pty.rs b/test-framework/e2e-tests/src/pty.rs index d0d5ef2da..0b232abc2 100644 --- a/test-framework/e2e-tests/src/pty.rs +++ b/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 } diff --git a/test-framework/e2e-tests/src/su/flag_pty.rs b/test-framework/e2e-tests/src/su/flag_pty.rs index ebe0f8b11..678b27be0 100644 --- a/test-framework/e2e-tests/src/su/flag_pty.rs +++ b/test-framework/e2e-tests/src/su/flag_pty.rs @@ -17,7 +17,7 @@ fn fixture() -> Result { 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 { 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::>(); su_related_processes.sort_by_key(|entry| entry.pid); From 2e73b8d8270f90cac2be11658ede0646f4e0e75a Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Tue, 26 Nov 2024 15:57:36 +0100 Subject: [PATCH 7/8] Ensure snapshots are consistent independent of the location of ls --- .../src/sudo/flag_list/long_format/mod.rs | 1 + .../sudo/flag_list/long_format/snapshots/command_alias.snap | 3 +-- .../snapshots/cwd_override_across_runas_groups.snap | 3 +-- .../sudo/flag_list/long_format/snapshots/multiple_lines.snap | 3 +-- .../flag_list/long_format/snapshots/negated_command_alias.snap | 3 +-- .../nopasswd_passwd_override_across_runas_groups.snap | 3 +-- .../src/sudo/flag_list/short_format/mod.rs | 1 + .../sudo/flag_list/short_format/snapshots/command_alias.snap | 3 +-- .../snapshots/cwd_override_across_runas_groups.snap | 3 +-- .../sudo/flag_list/short_format/snapshots/multiple_lines.snap | 3 +-- .../short_format/snapshots/negated_command_alias.snap | 3 +-- .../nopasswd_passwd_override_across_runas_groups.snap | 3 +-- 12 files changed, 12 insertions(+), 20 deletions(-) diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/mod.rs b/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/mod.rs index 479fe7edc..5b52b088b 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/mod.rs +++ b/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, "")], prepend_module_to_snapshot => false, }, { insta::assert_snapshot!($($tt)*) diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/command_alias.snap b/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/command_alias.snap index efa7694c1..a5b885feb 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/command_alias.snap +++ b/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/command_alias.snap @@ -1,6 +1,5 @@ --- 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: @@ -8,6 +7,6 @@ User root may run the following commands on container: Sudoers entry: RunAsUsers: root Commands: - /usr/bin/ls + /usr/bin/true /usr/bin/false diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/cwd_override_across_runas_groups.snap b/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/cwd_override_across_runas_groups.snap index fb3f8c333..de2aa6567 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/cwd_override_across_runas_groups.snap +++ b/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 + diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/multiple_lines.snap b/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/multiple_lines.snap index 88adedeac..0b14d1148 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/multiple_lines.snap +++ b/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 + diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/negated_command_alias.snap b/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/negated_command_alias.snap index 4ffc9c490..e8c3a7188 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/negated_command_alias.snap +++ b/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/negated_command_alias.snap @@ -1,6 +1,5 @@ --- 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: @@ -8,6 +7,6 @@ User root may run the following commands on container: Sudoers entry: RunAsUsers: root Commands: - /usr/bin/ls + !/usr/bin/true /usr/bin/false diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/nopasswd_passwd_override_across_runas_groups.snap b/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/nopasswd_passwd_override_across_runas_groups.snap index 9bd2f9117..05c8a7441 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_list/long_format/snapshots/nopasswd_passwd_override_across_runas_groups.snap +++ b/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 + diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/mod.rs b/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/mod.rs index 2ffee78d4..b9d592abd 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/mod.rs +++ b/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, "")], prepend_module_to_snapshot => false, }, { insta::assert_snapshot!($($tt)*) diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/command_alias.snap b/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/command_alias.snap index 8000639a3..47622dc19 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/command_alias.snap +++ b/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) , /usr/bin/true, /usr/bin/false diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/cwd_override_across_runas_groups.snap b/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/cwd_override_across_runas_groups.snap index d45292ab5..f6a2205e9 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/cwd_override_across_runas_groups.snap +++ b/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 diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/multiple_lines.snap b/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/multiple_lines.snap index cb8e0eaca..760f237a3 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/multiple_lines.snap +++ b/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) diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/negated_command_alias.snap b/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/negated_command_alias.snap index ffa2a863e..3aa616346 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/negated_command_alias.snap +++ b/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) , !/usr/bin/true, /usr/bin/false diff --git a/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/nopasswd_passwd_override_across_runas_groups.snap b/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/nopasswd_passwd_override_across_runas_groups.snap index ef23ccfe0..84239a02e 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/flag_list/short_format/snapshots/nopasswd_passwd_override_across_runas_groups.snap +++ b/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: From c0f92e70697e4a376f1b6a0140166920259462de Mon Sep 17 00:00:00 2001 From: Marc Schoolderman Date: Thu, 28 Nov 2024 14:07:29 +0100 Subject: [PATCH 8/8] different approach to making this test crossplatform --- .../src/sudo/path_search.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/test-framework/sudo-compliance-tests/src/sudo/path_search.rs b/test-framework/sudo-compliance-tests/src/sudo/path_search.rs index 066855c55..264bab8b2 100644 --- a/test-framework/sudo-compliance-tests/src/sudo/path_search.rs +++ b/test-framework/sudo-compliance-tests/src/sudo/path_search.rs @@ -117,14 +117,21 @@ fn paths_are_matched_using_realpath_in_arguments() -> Result<()> { #[test] fn arg0_native_is_passed_from_commandline() -> Result<()> { - let env = Env(SUDOERS_ALL_ALL_NOPASSWD) - .file("/usr/bin/foo", TextFile("#!/bin/sh\necho $0").chmod("755")) - .build()?; + let env = Env(SUDOERS_ALL_ALL_NOPASSWD).build()?; - let output = Command::new("sudo").arg("/usr/bin/foo").output(&env)?; + let output = Command::new("sh") + .args([ + "-c", + "ln -s /bin/ls /bin/foo; sudo /bin/foo --invalid-flag; true", + ]) + .output(&env)?; - let stdout = output.stdout().unwrap(); - assert_starts_with!(stdout, "/usr/bin/foo"); + 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(()) }