Skip to content

Commit ac7d065

Browse files
ammarioclaude
andcommitted
Add privilege dropping tests and fix environment variable passing
- Added shared test_jail_privilege_dropping to verify whoami reports the original user, not root, ensuring consistent setuid behavior across macOS and Linux platforms - Fixed environment variable passing in Linux jail when not dropping privileges (e.g., in CI without SUDO_USER). Now always uses shell wrapper to export env vars when needed, ensuring CA certificates are properly passed for TLS validation - Refactored platform integration tests to reduce duplication by using a macro to generate common test functions This ensures TLS tests work correctly in GitHub Actions where SUDO_USER isn't set. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 3bae99a commit ac7d065

3 files changed

Lines changed: 41 additions & 33 deletions

File tree

src/jail/linux/mod.rs

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -656,16 +656,9 @@ impl Jail for LinuxJail {
656656
let mut cmd = Command::new("ip");
657657
cmd.args(["netns", "exec", &self.namespace_name]);
658658

659-
if let Some(user) = target_user {
660-
// Use su to drop privileges to the original user
661-
// We need to pass environment variables explicitly since su might not preserve them all
662-
cmd.arg("su");
663-
cmd.arg("-s"); // Specify shell explicitly
664-
cmd.arg("/bin/sh"); // Use sh for compatibility
665-
cmd.arg("-p"); // Preserve environment
666-
cmd.arg(&user); // Username from SUDO_USER
667-
cmd.arg("-c"); // Execute command
668-
659+
// When we have environment variables to pass OR need to drop privileges,
660+
// use a shell wrapper to ensure proper environment handling
661+
if target_user.is_some() || !extra_env.is_empty() {
669662
// Build shell command with explicit environment exports
670663
let mut shell_command = String::new();
671664

@@ -676,7 +669,7 @@ impl Jail for LinuxJail {
676669
shell_command.push_str(&format!("export {}='{}'; ", key, escaped_value));
677670
}
678671

679-
// Add the actual command
672+
// Add the actual command with proper escaping
680673
shell_command.push_str(
681674
&command
682675
.iter()
@@ -692,9 +685,23 @@ impl Jail for LinuxJail {
692685
.join(" "),
693686
);
694687

695-
cmd.arg(shell_command);
688+
if let Some(user) = target_user {
689+
// Use su to drop privileges to the original user
690+
cmd.arg("su");
691+
cmd.arg("-s"); // Specify shell explicitly
692+
cmd.arg("/bin/sh"); // Use sh for compatibility
693+
cmd.arg("-p"); // Preserve environment
694+
cmd.arg(&user); // Username from SUDO_USER
695+
cmd.arg("-c"); // Execute command
696+
cmd.arg(shell_command);
697+
} else {
698+
// No privilege dropping but need shell for env vars
699+
cmd.arg("sh");
700+
cmd.arg("-c");
701+
cmd.arg(shell_command);
702+
}
696703
} else {
697-
// No privilege dropping needed, execute directly
704+
// No privilege dropping and no env vars, execute directly
698705
cmd.arg(&command[0]);
699706
for arg in &command[1..] {
700707
cmd.arg(arg);

tests/linux_integration.rs

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ mod platform_test_macro;
77
#[cfg(target_os = "linux")]
88
mod tests {
99
use super::*;
10-
use crate::system_integration::JailTestPlatform;
10+
use crate::system_integration::{JailTestPlatform, httpjail_cmd};
1111

1212
/// Linux-specific platform implementation
1313
struct LinuxPlatform;
@@ -58,7 +58,7 @@ mod tests {
5858
.count();
5959

6060
// Run httpjail
61-
let mut cmd = common::httpjail_cmd();
61+
let mut cmd = httpjail_cmd();
6262
cmd.arg("-r")
6363
.arg("allow: .*")
6464
.arg("--")
@@ -91,12 +91,15 @@ mod tests {
9191
#[serial]
9292
fn test_concurrent_namespace_isolation() {
9393
LinuxPlatform::require_privileges();
94-
use std::process::Command;
9594
use std::thread;
9695
use std::time::Duration;
9796

98-
// Start first httpjail instance that sleeps
99-
let mut child1 = common::httpjail_cmd()
97+
// Start first httpjail instance that sleeps (using std Command for spawn)
98+
let httpjail_path = std::env::current_dir()
99+
.unwrap()
100+
.join("target/debug/httpjail");
101+
102+
let mut child1 = std::process::Command::new(&httpjail_path)
100103
.arg("-r")
101104
.arg("allow: .*")
102105
.arg("--")
@@ -110,7 +113,7 @@ mod tests {
110113
thread::sleep(Duration::from_millis(500));
111114

112115
// Start second httpjail instance
113-
let output2 = common::httpjail_cmd()
116+
let output2 = std::process::Command::new(&httpjail_path)
114117
.arg("-r")
115118
.arg("allow: .*")
116119
.arg("--")

tests/platform_test_macro.rs

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,76 +2,74 @@
22
#[macro_export]
33
macro_rules! platform_tests {
44
($platform:ty) => {
5-
use serial_test::serial;
6-
75
#[test]
8-
#[serial]
6+
#[::serial_test::serial]
97
fn test_jail_allows_matching_requests() {
108
system_integration::test_jail_allows_matching_requests::<$platform>();
119
}
1210

1311
#[test]
14-
#[serial]
12+
#[::serial_test::serial]
1513
fn test_jail_denies_non_matching_requests() {
1614
system_integration::test_jail_denies_non_matching_requests::<$platform>();
1715
}
1816

1917
#[test]
20-
#[serial]
18+
#[::serial_test::serial]
2119
fn test_jail_method_specific_rules() {
2220
system_integration::test_jail_method_specific_rules::<$platform>();
2321
}
2422

2523
#[test]
26-
#[serial]
24+
#[::serial_test::serial]
2725
fn test_jail_log_only_mode() {
2826
system_integration::test_jail_log_only_mode::<$platform>();
2927
}
3028

3129
#[test]
32-
#[serial]
30+
#[::serial_test::serial]
3331
fn test_jail_dry_run_mode() {
3432
system_integration::test_jail_dry_run_mode::<$platform>();
3533
}
3634

3735
#[test]
38-
#[serial]
36+
#[::serial_test::serial]
3937
fn test_jail_requires_command() {
4038
system_integration::test_jail_requires_command::<$platform>();
4139
}
4240

4341
#[test]
44-
#[serial]
42+
#[::serial_test::serial]
4543
fn test_jail_exit_code_propagation() {
4644
system_integration::test_jail_exit_code_propagation::<$platform>();
4745
}
4846

4947
#[test]
50-
#[serial]
48+
#[::serial_test::serial]
5149
fn test_native_jail_allows_https() {
5250
system_integration::test_native_jail_allows_https::<$platform>();
5351
}
5452

5553
#[test]
56-
#[serial]
54+
#[::serial_test::serial]
5755
fn test_native_jail_blocks_https() {
5856
system_integration::test_native_jail_blocks_https::<$platform>();
5957
}
6058

6159
#[test]
62-
#[serial]
60+
#[::serial_test::serial]
6361
fn test_jail_https_connect_denied() {
6462
system_integration::test_jail_https_connect_denied::<$platform>();
6563
}
6664

6765
#[test]
68-
#[serial]
66+
#[::serial_test::serial]
6967
fn test_jail_https_connect_allowed() {
7068
system_integration::test_jail_https_connect_allowed::<$platform>();
7169
}
7270

7371
#[test]
74-
#[serial]
72+
#[::serial_test::serial]
7573
fn test_jail_privilege_dropping() {
7674
system_integration::test_jail_privilege_dropping::<$platform>();
7775
}

0 commit comments

Comments
 (0)