diff --git a/crates/server/src/devguard.rs b/crates/server/src/devguard.rs index 6d1a1d9..0da103b 100644 --- a/crates/server/src/devguard.rs +++ b/crates/server/src/devguard.rs @@ -918,32 +918,65 @@ pub(crate) mod tests { assert!(!logs.contains(SECRET), "{logs}"); } - /// What a child reports about its descriptors above 2 and its environment. - #[derive(Debug, Default, PartialEq, Eq)] + /// What a child reports about its descriptors above 2 and its environment. Its `Debug` + /// leaves the environment out: failure messages print reports, and a child's environment is + /// this process's, with whatever credentials the test run was started with. + #[derive(Default, PartialEq, Eq)] pub(crate) struct ChildReport { pub(crate) descriptors: BTreeSet, pub(crate) sockets: BTreeSet, pub(crate) environment: String, } - /// A script that lists the child's descriptors 3..128 and sockets among them, then its - /// environment. The descriptor numbers the parent can hold stay below 128 here. + impl std::fmt::Debug for ChildReport { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ChildReport") + .field("descriptors", &self.descriptors) + .field("sockets", &self.sockets) + .finish_non_exhaustive() + } + } + + /// A script that lists the child's descriptors 3..128 and sockets among them, then an `env:` + /// line and its environment. The descriptor numbers the parent can hold stay below 128 here. pub(crate) fn report_script() -> String { + format!("{}; {ENVIRONMENT_SCRIPT}", descriptor_script()) + } + + /// The part of `report_script` before the environment. + fn descriptor_script() -> String { let numbers: Vec = (3..128).map(|n| n.to_string()).collect(); format!( "fds=; sockets=; for n in {}; do if [ -e /dev/fd/$n ]; then fds=\"$fds $n\"; \ if [ -S /dev/fd/$n ]; then sockets=\"$sockets $n\"; fi; fi; done; \ - echo \"fds:$fds\"; echo \"sockets:$sockets\"; env", + echo \"fds:$fds\"; echo \"sockets:$sockets\"", numbers.join(" ") ) } + /// Prints an `env:` line, then the environment. Every report prints it last, because a + /// failure message prints a report only up to that line (see `before_environment`). + const ENVIRONMENT_SCRIPT: &str = "echo env:; env"; + + /// `output` up to its `env:` line, which is what a failure message may print of a child's + /// report; all of `output` if it has no such line. + pub(crate) fn before_environment(output: &str) -> &str { + let mut end = 0; + for line in output.split_inclusive('\n') { + if line.trim_end() == "env:" { + return &output[..end]; + } + end += line.len(); + } + output + } + pub(crate) fn parse(output: &str) -> ChildReport { let numbers = |prefix: &str| -> BTreeSet { output .lines() .find_map(|line| line.trim_end_matches('\r').strip_prefix(prefix)) - .unwrap_or_else(|| panic!("no {prefix} line in {output:?}")) + .unwrap_or_else(|| panic!("no {prefix} line in {:?}", before_environment(output))) .split_whitespace() .map(|n| n.parse().unwrap()) .collect() @@ -955,6 +988,42 @@ pub(crate) mod tests { } } + #[test] + fn failure_messages_leave_a_childs_environment_out() { + // A PTY child's lines end with `\r\n`. + for newline in ["\n", "\r\n"] { + let output = [ + "fds: 10", + "sockets:", + "env:", + "TOKEN=only-in-the-environment", + "", + ] + .join(newline); + let report = parse(&output); + assert!(report.environment.contains("only-in-the-environment")); + let message = format!("{report:?}"); + assert!(!message.contains("only-in-the-environment"), "{message}"); + assert_eq!( + before_environment(&output), + ["fds: 10", "sockets:", ""].join(newline) + ); + } + assert_eq!(before_environment("sh: not found\n"), "sh: not found\n"); + + // The real script prints the environment after everything a message shows. + let output = std::process::Command::new("/bin/sh") + .arg("-c") + .arg(report_script()) + .env("CODESPACE_ONLY_IN_THE_ENVIRONMENT", "1") + .output() + .unwrap(); + let output = String::from_utf8(output.stdout).unwrap(); + assert!(output.contains("CODESPACE_ONLY_IN_THE_ENVIRONMENT=1")); + assert!(before_environment(&output).starts_with("fds:")); + assert!(!before_environment(&output).contains("CODESPACE_ONLY_IN_THE_ENVIRONMENT")); + } + /// CodeSpace's spawners, and an unguarded spawn as the control. #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum Spawner { @@ -1391,8 +1460,8 @@ pub(crate) mod tests { text } - /// A worker script that reports its arguments, its descriptors and environment, and how - /// many bytes the descriptor it was handed carries. + /// A worker script that reports its arguments, its descriptors, how many bytes the + /// descriptor it was handed carries, and its environment. fn reporting_worker(dir: &Path, report: &Path) -> PathBuf { let script = dir.join("worker.sh"); std::fs::write( @@ -1402,8 +1471,9 @@ pub(crate) mod tests { if [ \"$previous\" = --devguard-credential-fd ]; then fd=$argument; fi; \ previous=$argument; done; \ {{ echo \"args: $*\"; {}; \ - if [ -n \"$fd\" ]; then echo \"carried: $(wc -c <&$fd | tr -d ' ')\"; fi; }} > '{}'\n", - report_script(), + if [ -n \"$fd\" ]; then echo \"carried: $(wc -c <&$fd | tr -d ' ')\"; fi; \ + {ENVIRONMENT_SCRIPT}; }} > '{}'\n", + descriptor_script(), report.display() ), ) @@ -1442,7 +1512,7 @@ pub(crate) mod tests { || start_reporting_worker(&worker, dir.path()), ) .await; - assert!(!output.contains(SECRET), "{output}"); + assert!(!output.contains(SECRET), "{}", before_environment(&output)); let args = output .lines() .find_map(|line| line.strip_prefix("args: ")) @@ -1465,7 +1535,11 @@ pub(crate) mod tests { // Its one socket is the carrier, holding the secret. (Its other descriptors are the // shell's own, whose numbers depend on which ones the carrier left free.) assert_eq!(parse(&output).sockets, BTreeSet::from([fd])); - assert!(output.contains("carried: 64"), "{output}"); + assert!( + output.contains("carried: 64"), + "{}", + before_environment(&output) + ); } #[tokio::test] @@ -1475,9 +1549,10 @@ pub(crate) mod tests { unusable.credential_file = dir.path().join("missing.secret"); let worker = WorkerSettings::from_settings(unusable); let output = start_reporting_worker(&worker, dir.path()).await; - assert!(output.contains("--devguard-socket"), "{output}"); - assert!(!output.contains("--devguard-credential-fd"), "{output}"); - assert!(!output.contains("carried:"), "{output}"); + let message = before_environment(&output); + assert!(output.contains("--devguard-socket"), "{message}"); + assert!(!output.contains("--devguard-credential-fd"), "{message}"); + assert!(!output.contains("carried:"), "{message}"); assert_eq!(parse(&output).sockets, BTreeSet::new()); } diff --git a/crates/server/src/mcp.rs b/crates/server/src/mcp.rs index 944ea48..df07f8b 100644 --- a/crates/server/src/mcp.rs +++ b/crates/server/src/mcp.rs @@ -2206,8 +2206,8 @@ mod devguard_tests { use super::tests::{assert_required_starts_nothing, exec_params, participation_registry}; use super::*; use crate::devguard::tests::{ - devguard_worker, execute_report, parse, past_the_credential, settings, short_directory, - Endpoint, FixtureAuthority, SECRET, + before_environment, devguard_worker, execute_report, parse, past_the_credential, settings, + short_directory, Endpoint, FixtureAuthority, SECRET, }; use crate::devguard::{ResourceAuthority, WorkerSettings}; use crate::runtime::RuntimeProcess; @@ -3213,7 +3213,10 @@ mod devguard_tests { } tokio::time::sleep(std::time::Duration::from_millis(10)).await; } - panic!("{process_id:?} did not exit: {output:?}"); + panic!( + "{process_id:?} did not exit: {:?}", + before_environment(&output) + ); } /// `operation_resume`, again while it stops at the credential (see `past_the_credential`): @@ -3939,17 +3942,19 @@ mod devguard_tests { // DevGuard's connect window on macOS. let (control, _peer) = crate::devguard::tests::inheritable_socket(); let control_fd = control.as_raw_fd(); - // Every descriptor 3..1024 a child holds, and which are sockets or pipes. + // Every descriptor 3..1024 a child holds and which are sockets or pipes, then an `env:` + // line and its environment. let probe = "fds=; sockets=; pipes=; n=3; while [ $n -lt 1024 ]; do \ if [ -e /dev/fd/$n ]; then fds=\"$fds $n\"; \ if [ -S /dev/fd/$n ]; then sockets=\"$sockets $n\"; fi; \ if [ -p /dev/fd/$n ]; then pipes=\"$pipes $n\"; fi; fi; n=$((n + 1)); done; \ - echo \"fds:$fds\"; echo \"sockets:$sockets\"; echo \"pipes:$pipes\"; env"; + echo \"fds:$fds\"; echo \"sockets:$sockets\"; echo \"pipes:$pipes\"; \ + echo env:; env"; let field = |output: &str, name: &str| -> String { output .lines() .find_map(|line| line.strip_prefix(name)) - .unwrap_or_else(|| panic!("no {name} in {output:?}")) + .unwrap_or_else(|| panic!("no {name} in {:?}", before_environment(output))) .trim() .to_owned() }; @@ -4002,7 +4007,8 @@ mod devguard_tests { field(&output, "pipes:") ), (String::new(), String::new(), String::new()), - "{workspace} tty={tty}: {output}" + "{workspace} tty={tty}: {}", + before_environment(&output) ); if workspace.starts_with("gov") { governed += 1;