Skip to content

Commit 031d8a7

Browse files
ammarioammario
authored
Merge branch 'main' into add-requester-ip
2 parents 4e414de + 831fcd4 commit 031d8a7

File tree

6 files changed

+211
-158
lines changed

6 files changed

+211
-158
lines changed

‎.github/workflows/tests.yml‎

Lines changed: 30 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -30,25 +30,18 @@ jobs:
3030
shared-key: ${{ runner.os }}
3131

3232
- name: Install nextest
33-
uses: taiki-e/install-action@nextest
33+
run: cargo install cargo-nextest --locked
3434

35-
- name: Build
36-
run: cargo build --verbose
37-
38-
- name: Run unit tests
39-
run: cargo nextest run --profile ci --lib --verbose
40-
41-
- name: Run smoke tests
42-
run: cargo nextest run --profile ci --test smoke_test --verbose
43-
44-
- name: Run script integration tests
45-
run: cargo nextest run --profile ci --test script_integration --verbose
46-
47-
- name: Run weak mode integration tests
35+
- name: Build httpjail binary
4836
run: |
49-
# On macOS, we only support weak mode due to PF limitations
50-
# (PF translation rules cannot match on user/group)
51-
cargo nextest run --profile ci --test weak_integration --verbose
37+
cargo build --bin httpjail --target-dir target
38+
export HTTPJAIL_BIN="$(pwd)/target/debug/httpjail"
39+
echo "Binary built at: ${HTTPJAIL_BIN}"
40+
ls -la "${HTTPJAIL_BIN}"
41+
echo "HTTPJAIL_BIN=${HTTPJAIL_BIN}" >> $GITHUB_ENV
42+
43+
- name: Run all tests
44+
run: cargo nextest run --profile ci
5245

5346
test-linux:
5447
name: Linux Tests
@@ -92,76 +85,43 @@ jobs:
9285
- name: Setup Rust environment and install nextest
9386
run: |
9487
source ~/.cargo/env
88+
rustup default stable
89+
cargo install cargo-nextest || true
9590
96-
# Install nextest if not already present
97-
if ! command -v cargo-nextest &> /dev/null; then
98-
cargo install cargo-nextest --locked
91+
- name: Fix target directory permissions from previous runs
92+
run: |
93+
if [ -d target ]; then
94+
sudo chown -R ci:ci target || true
9995
fi
10096
101-
- name: Build
97+
- name: Build httpjail binary
10298
run: |
10399
source ~/.cargo/env
104-
# Use incremental compilation for faster builds
105-
export CARGO_INCREMENTAL=1
106-
cargo build --verbose
100+
cargo build --bin httpjail --target-dir target
101+
export HTTPJAIL_BIN="$(pwd)/target/debug/httpjail"
102+
echo "Binary built at: ${HTTPJAIL_BIN}"
103+
ls -la "${HTTPJAIL_BIN}"
104+
echo "HTTPJAIL_BIN=${HTTPJAIL_BIN}" >> $GITHUB_ENV
107105
108-
- name: Run unit tests
106+
- name: Run all tests (non-root)
109107
run: |
110108
source ~/.cargo/env
111-
cargo nextest run --profile ci --lib --verbose
109+
cargo nextest run --profile ci --verbose -E 'not (binary(linux_integration) or binary(weak_integration))'
112110
113-
- name: Run smoke tests
114-
run: |
115-
source ~/.cargo/env
116-
cargo nextest run --profile ci --test smoke_test --verbose
111+
- name: Install dependencies for weak mode (curl)
112+
run: sudo apt-get update && sudo apt-get install -y curl
117113

118-
- name: Run script integration tests
114+
- name: Run weak mode integration tests (Linux)
119115
run: |
120116
source ~/.cargo/env
121-
cargo nextest run --profile ci --test script_integration --verbose
117+
cargo nextest run --profile ci --test weak_integration
122118
123-
- name: Run Linux jail integration tests
119+
- name: Run Linux jail integration tests (sudo)
124120
run: |
125121
source ~/.cargo/env
126-
# Run all tests without CI workarounds since this is a self-hosted runner
122+
# Run Linux-specific jail tests with sudo to satisfy root requirements
127123
sudo -E $(which cargo) nextest run --profile ci --test linux_integration --verbose
128124
129-
- name: Run isolated cleanup tests
130-
run: |
131-
source ~/.cargo/env
132-
# Run only the comprehensive cleanup and sigint tests with the feature flag
133-
# These tests need to run in isolation from other tests
134-
sudo -E $(which cargo) test --test linux_integration --features isolated-cleanup-tests -- test_comprehensive_resource_cleanup test_cleanup_after_sigint
135-
136-
test-weak:
137-
name: Weak Mode Integration Tests (Linux)
138-
runs-on: ubuntu-latest-8-cores
139-
140-
steps:
141-
- uses: actions/checkout@v4
142-
143-
- name: Install Rust
144-
uses: dtolnay/rust-toolchain@stable
145-
with:
146-
toolchain: stable
147-
148-
- name: Setup Rust cache
149-
uses: Swatinem/rust-cache@v2
150-
with:
151-
shared-key: ${{ runner.os }}
152-
153-
- name: Install nextest
154-
uses: taiki-e/install-action@nextest
155-
156-
- name: Build
157-
run: cargo build --verbose
158-
159-
- name: Run script integration tests
160-
run: cargo nextest run --profile ci --test script_integration --verbose
161-
162-
- name: Run weak mode integration tests
163-
run: cargo nextest run --profile ci --test weak_integration --verbose
164-
165125
clippy:
166126
name: Clippy (${{ matrix.os }})
167127
runs-on: ${{ matrix.os }}

‎src/jail/linux/mod.rs‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
mod nftables;
22
mod resources;
33

4-
use super::{Jail, JailConfig};
4+
use super::Jail;
5+
use super::JailConfig;
56
use crate::sys_resource::ManagedResource;
67
use anyhow::{Context, Result};
78
use resources::{NFTable, NamespaceConfig, NetworkNamespace, VethPair};
@@ -492,15 +493,16 @@ nameserver 8.8.4.4\n",
492493
namespace_name
493494
);
494495

495-
// Create a temporary resolv.conf with public DNS
496-
let temp_resolv = format!("/tmp/httpjail_resolv_{}.conf", &namespace_name);
497-
std::fs::write(
498-
&temp_resolv,
499-
"# Temporary DNS for httpjail namespace\n\
500-
nameserver 8.8.8.8\n\
501-
nameserver 8.8.4.4\n\
502-
nameserver 1.1.1.1\n",
503-
)?;
496+
// Setup DNS for the namespace
497+
// Create a temporary resolv.conf before running the nsenter command
498+
let temp_dir = crate::jail::get_temp_dir();
499+
std::fs::create_dir_all(&temp_dir).ok();
500+
let temp_resolv = temp_dir
501+
.join(format!("httpjail_resolv_{}.conf", &namespace_name))
502+
.to_string_lossy()
503+
.to_string();
504+
std::fs::write(&temp_resolv, "nameserver 1.1.1.1\nnameserver 8.8.8.8\n")
505+
.with_context(|| format!("Failed to create temp resolv.conf: {}", temp_resolv))?;
504506

505507
// First, try to directly write to /etc/resolv.conf in the namespace using echo
506508
let write_cmd = Command::new("ip")

‎src/jail/managed.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@ use std::thread::{self, JoinHandle};
99
use std::time::{Duration, SystemTime};
1010
use tracing::{debug, error, info, warn};
1111

12-
/// A jail with lifecycle management (heartbeat and orphan cleanup)
12+
use crate::jail::get_canary_dir;
13+
14+
/// Manages jail lifecycle and cleanup with automatic cleanup on drop
1315
pub struct ManagedJail<J: Jail> {
1416
jail: J,
1517

16-
// Lifecycle management fields (inlined from JailLifecycleManager)
18+
// Lifecycle management fields
1719
canary_dir: PathBuf,
1820
canary_path: PathBuf,
1921
heartbeat_interval: Duration,
@@ -26,9 +28,8 @@ pub struct ManagedJail<J: Jail> {
2628
}
2729

2830
impl<J: Jail> ManagedJail<J> {
29-
/// Create a new managed jail
3031
pub fn new(jail: J, config: &JailConfig) -> Result<Self> {
31-
let canary_dir = PathBuf::from("/tmp/httpjail");
32+
let canary_dir = get_canary_dir();
3233
let canary_path = canary_dir.join(&config.jail_id);
3334

3435
Ok(Self {

‎src/jail/mod.rs‎

Lines changed: 52 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
use anyhow::Result;
22
use rand::Rng;
33

4+
pub mod weak;
5+
6+
#[cfg(target_os = "linux")]
7+
pub mod linux;
8+
9+
#[cfg(any(target_os = "macos", target_os = "linux"))]
410
pub mod managed;
511

612
/// Trait for platform-specific jail implementations
@@ -43,8 +49,18 @@ pub trait Jail: Send + Sync {
4349
Self: Sized;
4450
}
4551

46-
/// Configuration for jail setup
47-
#[derive(Debug, Clone)]
52+
/// Get the canary directory for tracking jail lifetimes
53+
pub fn get_canary_dir() -> std::path::PathBuf {
54+
std::path::PathBuf::from("/tmp/httpjail")
55+
}
56+
57+
/// Get the directory for httpjail temporary files (like resolv.conf)
58+
pub fn get_temp_dir() -> std::path::PathBuf {
59+
std::path::PathBuf::from("/tmp/httpjail")
60+
}
61+
62+
/// Jail configuration
63+
#[derive(Clone, Debug)]
4864
pub struct JailConfig {
4965
/// Port where the HTTP proxy is listening
5066
pub http_proxy_port: u16,
@@ -102,6 +118,40 @@ impl Default for JailConfig {
102118
}
103119
}
104120

121+
/// Create a platform-specific jail implementation wrapped with lifecycle management
122+
pub fn create_jail(config: JailConfig, weak_mode: bool) -> Result<Box<dyn Jail>> {
123+
use self::weak::WeakJail;
124+
125+
// Always use weak jail on macOS due to PF limitations
126+
// (PF translation rules cannot match on user/group)
127+
#[cfg(target_os = "macos")]
128+
{
129+
let _ = weak_mode; // Suppress unused warning on macOS
130+
// WeakJail doesn't need lifecycle management since it creates no system resources
131+
Ok(Box::new(WeakJail::new(config)?))
132+
}
133+
134+
#[cfg(target_os = "linux")]
135+
{
136+
use self::linux::LinuxJail;
137+
if weak_mode {
138+
// WeakJail doesn't need lifecycle management since it creates no system resources
139+
Ok(Box::new(WeakJail::new(config)?))
140+
} else {
141+
// LinuxJail creates system resources (namespaces, iptables) that need cleanup
142+
Ok(Box::new(self::managed::ManagedJail::new(
143+
LinuxJail::new(config.clone())?,
144+
&config,
145+
)?))
146+
}
147+
}
148+
149+
#[cfg(not(any(target_os = "macos", target_os = "linux")))]
150+
{
151+
anyhow::bail!("Unsupported platform")
152+
}
153+
}
154+
105155
#[cfg(test)]
106156
mod tests {
107157
use super::*;
@@ -144,50 +194,3 @@ mod tests {
144194
assert_eq!(ids.len(), 1000);
145195
}
146196
}
147-
148-
// macOS module removed - using weak jail on macOS due to PF limitations
149-
// (PF translation rules cannot match on user/group)
150-
151-
#[cfg(target_os = "linux")]
152-
pub mod linux;
153-
154-
mod weak;
155-
156-
/// Create a platform-specific jail implementation wrapped with lifecycle management
157-
pub fn create_jail(config: JailConfig, weak_mode: bool) -> Result<Box<dyn Jail>> {
158-
use self::managed::ManagedJail;
159-
use self::weak::WeakJail;
160-
161-
// Use weak jail if requested or on macOS (since PF cannot match groups with translation rules)
162-
#[cfg(target_os = "macos")]
163-
{
164-
// Always use weak jail on macOS due to PF limitations
165-
// (PF translation rules cannot match on user/group)
166-
let _ = weak_mode; // Suppress unused warning on macOS
167-
Ok(Box::new(ManagedJail::new(
168-
WeakJail::new(config.clone())?,
169-
&config,
170-
)?))
171-
}
172-
173-
#[cfg(target_os = "linux")]
174-
{
175-
if weak_mode {
176-
Ok(Box::new(ManagedJail::new(
177-
WeakJail::new(config.clone())?,
178-
&config,
179-
)?))
180-
} else {
181-
use self::linux::LinuxJail;
182-
Ok(Box::new(ManagedJail::new(
183-
LinuxJail::new(config.clone())?,
184-
&config,
185-
)?))
186-
}
187-
}
188-
189-
#[cfg(not(any(target_os = "macos", target_os = "linux")))]
190-
{
191-
anyhow::bail!("Unsupported platform")
192-
}
193-
}

‎src/main.rs‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -140,11 +140,12 @@ fn setup_logging(verbosity: u8) {
140140
fn cleanup_orphans() -> Result<()> {
141141
use anyhow::Context;
142142
use std::fs;
143+
#[cfg(target_os = "linux")]
143144
use std::path::PathBuf;
144145
use std::time::{Duration, SystemTime};
145146
use tracing::{debug, info};
146147

147-
let canary_dir = PathBuf::from("/tmp/httpjail");
148+
let canary_dir = httpjail::jail::get_canary_dir();
148149
let orphan_timeout = Duration::from_secs(5); // Short timeout to catch recent orphans
149150

150151
debug!("Starting direct orphan cleanup scan");
@@ -344,21 +345,17 @@ async fn main() -> Result<()> {
344345
let mut parts = s.split_whitespace();
345346
match (parts.next(), parts.next()) {
346347
(Some(maybe_method), Some(url_rest)) => {
347-
let method = maybe_method
348-
.parse::<Method>()
349-
.or_else(|_| maybe_method.to_ascii_uppercase().parse::<Method>())
350-
.unwrap_or(Method::GET);
348+
let method_str = maybe_method.to_ascii_uppercase();
349+
let method = method_str.parse::<Method>().unwrap_or(Method::GET);
351350
(method, url_rest.to_string())
352351
}
353352
_ => (Method::GET, s.clone()),
354353
}
355354
} else {
356355
let maybe_method = &test_vals[0];
357356
let url = &test_vals[1];
358-
let method = maybe_method
359-
.parse::<Method>()
360-
.or_else(|_| maybe_method.to_ascii_uppercase().parse::<Method>())
361-
.unwrap_or(Method::GET);
357+
let method_str = maybe_method.to_ascii_uppercase();
358+
let method = method_str.parse::<Method>().unwrap_or(Method::GET);
362359
(method, url.clone())
363360
};
364361

@@ -448,7 +445,8 @@ async fn main() -> Result<()> {
448445
}
449446

450447
// Create jail canary dir early to reduce race with cleanup
451-
std::fs::create_dir_all("/tmp/httpjail").ok();
448+
let canary_dir = httpjail::jail::get_canary_dir();
449+
std::fs::create_dir_all(&canary_dir).ok();
452450

453451
// Configure and execute the target command inside a jail
454452
jail_config.http_proxy_port = actual_http_port;

0 commit comments

Comments
 (0)