diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index d1df5aef..bf5e3e93 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -62,8 +62,9 @@ sudo ./target/release/httpjail --allow "httpbin\.org" -- curl http://httpbin.org # Test with method-specific rules sudo ./target/release/httpjail --allow-get ".*" -- curl -X POST http://httpbin.org/post -# Test log-only mode -sudo ./target/release/httpjail --log-only -- curl http://example.com +# Test request logging +sudo ./target/release/httpjail --request-log requests.log -r "allow: .*" -- curl http://example.com +# Log format: " <+/-> " (+ = allowed, - = blocked) ``` ### Test Organization diff --git a/README.md b/README.md index 583623ab..7552abe3 100644 --- a/README.md +++ b/README.md @@ -32,8 +32,9 @@ cargo install httpjail # Allow only requests to github.com httpjail -r "allow: github\.com" -r "deny: .*" -- claude -# Monitor all requests without blocking -httpjail --log-only -- npm install +# Log requests to a file +httpjail --request-log requests.log -r "allow: .*" -- npm install +# Log format: " <+/-> " (+ = allowed, - = blocked) # Block specific domains httpjail -r "deny: telemetry\..*" -r "allow: .*" -- ./my-app diff --git a/src/main.rs b/src/main.rs index 3430c057..12db7d39 100644 --- a/src/main.rs +++ b/src/main.rs @@ -3,9 +3,10 @@ use clap::Parser; use httpjail::jail::{JailConfig, create_jail}; use httpjail::proxy::ProxyServer; use httpjail::rules::{Action, Rule, RuleEngine}; +use std::fs::OpenOptions; use std::os::unix::process::ExitStatusExt; -use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, Mutex}; use tracing::{debug, info, warn}; #[derive(Parser, Debug)] @@ -28,9 +29,9 @@ struct Args { #[arg(short = 'c', long = "config", value_name = "FILE")] config: Option, - /// Monitor without filtering - #[arg(long = "log-only")] - log_only: bool, + /// Append requests to a log file + #[arg(long = "request-log", value_name = "FILE")] + request_log: Option, /// Interactive approval mode #[arg(long = "interactive")] @@ -313,7 +314,18 @@ async fn main() -> Result<()> { // Build rules from command line arguments let rules = build_rules(&args)?; - let rule_engine = RuleEngine::new(rules, args.log_only); + let request_log = if let Some(path) = &args.request_log { + Some(Arc::new(Mutex::new( + OpenOptions::new() + .create(true) + .append(true) + .open(path) + .context("Failed to open request log file")?, + ))) + } else { + None + }; + let rule_engine = RuleEngine::new(rules, request_log); // Parse bind configuration from env vars // Supports both "port" and "ip:port" formats @@ -527,7 +539,7 @@ mod tests { Rule::new(Action::Deny, r".*").unwrap(), ]; - let engine = RuleEngine::new(rules, false); + let engine = RuleEngine::new(rules, None); // Test allow rule assert!(matches!( @@ -548,19 +560,6 @@ mod tests { )); } - #[test] - fn test_log_only_mode() { - let rules = vec![Rule::new(Action::Deny, r".*").unwrap()]; - - let engine = RuleEngine::new(rules, true); - - // In log-only mode, everything should be allowed - assert!(matches!( - engine.evaluate(Method::POST, "https://example.com"), - Action::Allow - )); - } - #[test] fn test_build_rules_from_config_file() { use std::io::Write; @@ -576,7 +575,7 @@ mod tests { let args = Args { rules: vec![], config: Some(file.path().to_str().unwrap().to_string()), - log_only: false, + request_log: None, interactive: false, weak: false, verbose: 0, diff --git a/src/proxy.rs b/src/proxy.rs index f70193c5..27b0d5fe 100644 --- a/src/proxy.rs +++ b/src/proxy.rs @@ -462,7 +462,7 @@ mod tests { Rule::new(Action::Deny, r".*").unwrap(), ]; - let rule_engine = RuleEngine::new(rules, false); + let rule_engine = RuleEngine::new(rules, None); let proxy = ProxyServer::new(Some(8080), Some(8443), rule_engine, None); assert_eq!(proxy.http_port, Some(8080)); @@ -473,7 +473,7 @@ mod tests { async fn test_proxy_server_auto_port() { let rules = vec![Rule::new(Action::Allow, r".*").unwrap()]; - let rule_engine = RuleEngine::new(rules, false); + let rule_engine = RuleEngine::new(rules, None); let mut proxy = ProxyServer::new(None, None, rule_engine, None); let (http_port, https_port) = proxy.start().await.unwrap(); diff --git a/src/proxy_tls.rs b/src/proxy_tls.rs index bf9c837c..90cb1380 100644 --- a/src/proxy_tls.rs +++ b/src/proxy_tls.rs @@ -593,7 +593,7 @@ mod tests { Rule::new(Action::Deny, r".*").unwrap(), ] }; - Arc::new(RuleEngine::new(rules, false)) + Arc::new(RuleEngine::new(rules, None)) } /// Create a TLS client config that trusts any certificate (for testing) diff --git a/src/rules.rs b/src/rules.rs index f3344921..629cb559 100644 --- a/src/rules.rs +++ b/src/rules.rs @@ -1,7 +1,11 @@ use anyhow::Result; +use chrono::{SecondsFormat, Utc}; use hyper::Method; use regex::Regex; use std::collections::HashSet; +use std::fs::File; +use std::io::Write; +use std::sync::{Arc, Mutex}; use tracing::{info, warn}; #[derive(Debug, Clone)] @@ -48,22 +52,21 @@ impl Rule { #[derive(Clone)] pub struct RuleEngine { pub rules: Vec, - pub log_only: bool, + pub request_log: Option>>, } impl RuleEngine { - pub fn new(rules: Vec, log_only: bool) -> Self { - RuleEngine { rules, log_only } + pub fn new(rules: Vec, request_log: Option>>) -> Self { + RuleEngine { rules, request_log } } pub fn evaluate(&self, method: Method, url: &str) -> Action { - if self.log_only { - info!("Request: {} {}", method, url); - return Action::Allow; - } + let mut action = Action::Deny; + let mut matched = false; for rule in &self.rules { if rule.matches(method.clone(), url) { + matched = true; match &rule.action { Action::Allow => { info!( @@ -72,7 +75,7 @@ impl RuleEngine { url, rule.pattern.as_str() ); - return Action::Allow; + action = Action::Allow; } Action::Deny => { warn!( @@ -81,21 +84,39 @@ impl RuleEngine { url, rule.pattern.as_str() ); - return Action::Deny; + action = Action::Deny; } } + break; + } + } + + if !matched { + warn!("DENY: {} {} (no matching rules)", method, url); + action = Action::Deny; + } + + if let Some(log) = &self.request_log + && let Ok(mut file) = log.lock() + { + let timestamp = Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true); + let status = match &action { + Action::Allow => '+', + Action::Deny => '-', + }; + if let Err(e) = writeln!(file, "{} {} {} {}", timestamp, status, method, url) { + warn!("Failed to write to request log: {}", e); } } - // Default deny if no rules match - warn!("DENY: {} {} (no matching rules)", method, url); - Action::Deny + action } } #[cfg(test)] mod tests { use super::*; + use std::sync::{Arc, Mutex}; #[test] fn test_rule_matching() { @@ -125,7 +146,7 @@ mod tests { Rule::new(Action::Deny, r".*").unwrap(), ]; - let engine = RuleEngine::new(rules, false); + let engine = RuleEngine::new(rules, None); // Test allow rule assert!(matches!( @@ -155,7 +176,7 @@ mod tests { Rule::new(Action::Deny, r".*").unwrap(), ]; - let engine = RuleEngine::new(rules, false); + let engine = RuleEngine::new(rules, None); // GET should be allowed assert!(matches!( @@ -171,15 +192,38 @@ mod tests { } #[test] - fn test_log_only_mode() { + fn test_request_logging() { + use std::fs::OpenOptions; + + let rules = vec![Rule::new(Action::Allow, r".*").unwrap()]; + let log_file = tempfile::NamedTempFile::new().unwrap(); + let file = OpenOptions::new() + .append(true) + .open(log_file.path()) + .unwrap(); + let engine = RuleEngine::new(rules, Some(Arc::new(Mutex::new(file)))); + + engine.evaluate(Method::GET, "https://example.com"); + + let contents = std::fs::read_to_string(log_file.path()).unwrap(); + assert!(contents.contains("+ GET https://example.com")); + } + + #[test] + fn test_request_logging_denied() { + use std::fs::OpenOptions; + let rules = vec![Rule::new(Action::Deny, r".*").unwrap()]; + let log_file = tempfile::NamedTempFile::new().unwrap(); + let file = OpenOptions::new() + .append(true) + .open(log_file.path()) + .unwrap(); + let engine = RuleEngine::new(rules, Some(Arc::new(Mutex::new(file)))); - let engine = RuleEngine::new(rules, true); + engine.evaluate(Method::GET, "https://blocked.com"); - // In log-only mode, everything should be allowed - assert!(matches!( - engine.evaluate(Method::POST, "https://example.com"), - Action::Allow - )); + let contents = std::fs::read_to_string(log_file.path()).unwrap(); + assert!(contents.contains("- GET https://blocked.com")); } } diff --git a/tests/platform_test_macro.rs b/tests/platform_test_macro.rs index c693c0b9..84d7b07b 100644 --- a/tests/platform_test_macro.rs +++ b/tests/platform_test_macro.rs @@ -28,8 +28,8 @@ macro_rules! platform_tests { #[test] #[::serial_test::serial] - fn test_jail_log_only_mode() { - system_integration::test_jail_log_only_mode::<$platform>(); + fn test_jail_request_log() { + system_integration::test_jail_request_log::<$platform>(); } #[test] diff --git a/tests/system_integration.rs b/tests/system_integration.rs index 32155257..b6d57cd3 100644 --- a/tests/system_integration.rs +++ b/tests/system_integration.rs @@ -195,12 +195,19 @@ pub fn test_jail_method_specific_rules() { assert_eq!(stdout.trim(), "403", "POST request should be denied"); } -/// Test log-only mode -pub fn test_jail_log_only_mode() { +/// Test request logging +pub fn test_jail_request_log() { P::require_privileges(); + let log_file = tempfile::NamedTempFile::new().expect("Failed to create temp file"); + let log_path = log_file.path().to_str().unwrap().to_string(); + let mut cmd = httpjail_cmd(); - cmd.arg("--log-only").arg("--"); + cmd.arg("--request-log") + .arg(&log_path) + .arg("-r") + .arg("allow: .*") + .arg("--"); curl_http_status_args(&mut cmd, "http://example.com"); let output = cmd.output().expect("Failed to execute httpjail"); @@ -210,18 +217,25 @@ pub fn test_jail_log_only_mode() { if !stderr.is_empty() { eprintln!("[{}] stderr: {}", P::platform_name(), stderr); } - eprintln!("[{}] stdout: {}", P::platform_name(), stdout); - // In log-only mode, all requests should be allowed - // Due to proxy issues, we might get partial responses or timeouts - // Just verify that the request wasn't explicitly blocked (403) - assert!( - !stdout.contains("403") && !stderr.contains("403") && !stdout.contains("Request blocked"), - "Request should not be blocked in log-only mode. Got stdout: '{}', stderr: '{}', exit code: {:?}", - stdout.trim(), - stderr.trim(), - output.status.code() - ); + assert_eq!(stdout.trim(), "200", "GET request should be allowed"); + + // Run a denied request to ensure '-' is logged + let mut cmd = httpjail_cmd(); + cmd.arg("--request-log") + .arg(&log_path) + .arg("-r") + .arg("deny: .*") + .arg("--"); + curl_http_status_args(&mut cmd, "http://example.com"); + + let output = cmd.output().expect("Failed to execute httpjail"); + let stdout = String::from_utf8_lossy(&output.stdout); + assert_eq!(stdout.trim(), "403", "GET request should be denied"); + + let contents = std::fs::read_to_string(log_file.path()).expect("Failed to read request log"); + assert!(contents.contains("+ GET http://example.com")); + assert!(contents.contains("- GET http://example.com")); } /// Test that jail requires a command