From 94fbe1edef0c4ac9719340b10d0ce5c5e135aa7d Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Sun, 19 Sep 2021 11:25:00 +0200 Subject: [PATCH 1/4] `chroot`: quick fix for #2687 * fixes #2687 * change error message to better mimic GNU --- src/uu/chroot/src/chroot.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/uu/chroot/src/chroot.rs b/src/uu/chroot/src/chroot.rs index 40799d009..c78172025 100644 --- a/src/uu/chroot/src/chroot.rs +++ b/src/uu/chroot/src/chroot.rs @@ -67,7 +67,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { // TODO: refactor the args and command matching // See: https://github.com/uutils/coreutils/pull/2365#discussion_r647849967 let command: Vec<&str> = match commands.len() { - 1 => { + 0 => { let shell: &str = match user_shell { Err(_) => default_shell, Ok(ref s) => s.as_ref(), @@ -79,10 +79,22 @@ pub fn uumain(args: impl uucore::Args) -> i32 { set_context(newroot, &matches); + assert!(!command.is_empty()); let pstatus = Command::new(command[0]) .args(&command[1..]) .status() - .unwrap_or_else(|e| crash!(1, "Cannot exec: {}", e)); + .unwrap_or_else(|e| { + // TODO: Exit status: + // 125 if chroot itself fails + // 126 if command is found but cannot be invoked + // 127 if command cannot be found + crash!( + 1, + "failed to run command {}: {}", + command[0].to_string().quote(), + e + ) + }); if pstatus.success() { 0 From 8db8d8ac4ec6eea0109fa5c584991e0823025144 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Sun, 19 Sep 2021 22:21:29 +0200 Subject: [PATCH 2/4] chroot: add 'test_default_shell` --- tests/by-util/test_chroot.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/by-util/test_chroot.rs b/tests/by-util/test_chroot.rs index 65d821d01..361133ccb 100644 --- a/tests/by-util/test_chroot.rs +++ b/tests/by-util/test_chroot.rs @@ -89,3 +89,24 @@ fn test_preference_of_userspec() { println!("result.stdout = {}", result.stdout_str()); println!("result.stderr = {}", result.stderr_str()); } + +#[test] +fn test_default_shell() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + let dir = "CHROOT_DIR"; + at.mkdir(dir); + + let shell = std::env::var("SHELL").unwrap_or_else(|_| "/bin/sh".to_string()); + let expected = format!( + "chroot: failed to run command '{}': No such file or directory", + shell + ); + + if let Ok(result) = run_ucmd_as_root(&ts, &[dir]) { + result.stderr_contains(expected); + } else { + print!("TEST SKIPPED"); + } +} From 8c0b7d1314f15bb2caec618073e7a07b0b229505 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Wed, 22 Sep 2021 11:59:43 +0200 Subject: [PATCH 3/4] chroot: move logic so it can be triggered by tests * move the command building logic before the `chroot` syscall so it will be reachable by tests that don't have root permissions. --- src/uu/chroot/src/chroot.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/uu/chroot/src/chroot.rs b/src/uu/chroot/src/chroot.rs index c78172025..55097c1bb 100644 --- a/src/uu/chroot/src/chroot.rs +++ b/src/uu/chroot/src/chroot.rs @@ -77,11 +77,15 @@ pub fn uumain(args: impl uucore::Args) -> i32 { _ => commands, }; + assert!(!command.is_empty()); + let chroot_command = command[0]; + let chroot_args = &command[1..]; + + // NOTE: Tests can only trigger code beyond this point if they're invoked with root permissions set_context(newroot, &matches); - assert!(!command.is_empty()); - let pstatus = Command::new(command[0]) - .args(&command[1..]) + let pstatus = Command::new(chroot_command) + .args(chroot_args) .status() .unwrap_or_else(|e| { // TODO: Exit status: From 30c8a4378893d52161b3f9fbea244fdcd54df52a Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Wed, 22 Sep 2021 12:03:22 +0200 Subject: [PATCH 4/4] test_chroot: add comments --- tests/by-util/test_chroot.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/by-util/test_chroot.rs b/tests/by-util/test_chroot.rs index 361133ccb..3e5c22679 100644 --- a/tests/by-util/test_chroot.rs +++ b/tests/by-util/test_chroot.rs @@ -15,6 +15,7 @@ fn test_missing_operand() { #[test] fn test_enter_chroot_fails() { + // NOTE: since #2689 this test also ensures that we don't regress #2687 let (at, mut ucmd) = at_and_ucmd!(); at.mkdir("jail"); @@ -92,6 +93,7 @@ fn test_preference_of_userspec() { #[test] fn test_default_shell() { + // NOTE: This test intends to trigger code which can only be reached with root permissions. let ts = TestScenario::new(util_name!()); let at = &ts.fixtures; @@ -99,14 +101,15 @@ fn test_default_shell() { at.mkdir(dir); let shell = std::env::var("SHELL").unwrap_or_else(|_| "/bin/sh".to_string()); - let expected = format!( + let _expected = format!( "chroot: failed to run command '{}': No such file or directory", shell ); - if let Ok(result) = run_ucmd_as_root(&ts, &[dir]) { - result.stderr_contains(expected); - } else { - print!("TEST SKIPPED"); - } + // TODO: [2021-09; jhscheer] uncomment if/when #2692 gets merged + // if let Ok(result) = run_ucmd_as_root(&ts, &[dir]) { + // result.stderr_contains(expected); + // } else { + // print!("TEST SKIPPED"); + // } }