diff --git a/crates/nono-cli/src/command_runtime.rs b/crates/nono-cli/src/command_runtime.rs index 3806c40e1..5ead075b7 100644 --- a/crates/nono-cli/src/command_runtime.rs +++ b/crates/nono-cli/src/command_runtime.rs @@ -11,8 +11,7 @@ use crate::profile; use crate::proxy_runtime::prepare_proxy_launch_options; use crate::sandbox_prepare::{ prepare_sandbox, print_allow_gpu_warning, print_allow_launch_services_warning, - should_auto_enable_claude_launch_services, validate_block_net_conflicts, - validate_external_proxy_bypass, + should_auto_enable_claude_launch_services, validate_proxy_conflicts, }; use crate::theme; use nono::{CapabilitySet, NonoError, Result}; @@ -212,8 +211,7 @@ pub(crate) fn run_sandbox(mut run_args: RunArgs, silent: bool) -> Result<()> { if args.dry_run { let prepared = prepare_sandbox(&args, silent)?; - validate_block_net_conflicts(&args, &prepared)?; - validate_external_proxy_bypass(&args, &prepared)?; + validate_proxy_conflicts(&args, &prepared)?; if !prepared.secrets.is_empty() && !silent { eprintln!( " Would inject {} credential(s) as environment variables", @@ -242,6 +240,7 @@ pub(crate) fn run_shell(args: ShellArgs, silent: bool) -> Result<()> { if args.sandbox.dry_run { let prepared = prepare_sandbox(&args.sandbox, silent)?; + validate_proxy_conflicts(&args.sandbox, &prepared)?; reject_run_only_sandbox_policy("shell", &args.sandbox, &prepared)?; if !prepared.secrets.is_empty() && !silent { eprintln!( @@ -255,6 +254,7 @@ pub(crate) fn run_shell(args: ShellArgs, silent: bool) -> Result<()> { } let prepared = prepare_sandbox(&args.sandbox, silent)?; + validate_proxy_conflicts(&args.sandbox, &prepared)?; reject_run_only_sandbox_policy("shell", &args.sandbox, &prepared)?; if prepared.allow_launch_services_active { @@ -420,8 +420,36 @@ pub(crate) fn run_wrap(wrap_args: WrapArgs, silent: bool) -> Result<()> { #[cfg(test)] mod tests { - use super::reject_resource_limits_under_wrap; + use super::{reject_resource_limits_under_wrap, run_shell}; + use crate::cli::{SandboxArgs, ShellArgs}; use nono::{CapabilitySet, ResourceLimits}; + use std::path::PathBuf; + + #[test] + fn shell_dry_run_rejects_block_net_with_upstream_proxy() { + let args = ShellArgs { + sandbox: SandboxArgs { + dry_run: true, + allow_cwd: true, + block_net: true, + external_proxy: Some("squid.corp:3128".to_string()), + ..SandboxArgs::default() + }, + shell: Some(PathBuf::from("/bin/sh")), + name: None, + startup_timeout_secs: None, + help: None, + }; + + let result = run_shell(args, true); + let Err(err) = result else { + panic!("expected shell dry-run to reject --block-net + --upstream-proxy"); + }; + assert!( + err.to_string().contains("--block-net") && err.to_string().contains("--upstream-proxy"), + "unexpected error: {err}" + ); + } #[test] fn wrap_rejects_caps_carrying_a_memory_limit() { diff --git a/crates/nono-cli/src/launch_runtime.rs b/crates/nono-cli/src/launch_runtime.rs index 40e50bc1a..e086e8701 100644 --- a/crates/nono-cli/src/launch_runtime.rs +++ b/crates/nono-cli/src/launch_runtime.rs @@ -3,7 +3,7 @@ use crate::config; use crate::proxy_runtime::prepare_proxy_launch_options; use crate::sandbox_prepare::{ PreparedSandbox, prepare_sandbox, print_allow_gpu_warning, print_allow_launch_services_warning, - validate_block_net_conflicts, + validate_proxy_conflicts, }; use crate::{exec_strategy, instruction_deny, profile, trust_scan}; use colored::Colorize; @@ -346,7 +346,7 @@ pub(crate) fn prepare_run_launch_plan( } let mut prepared = prepare_sandbox(&args, silent)?; - validate_block_net_conflicts(&args, &prepared)?; + validate_proxy_conflicts(&args, &prepared)?; validate_rollback_destination(run_args.rollback_dest.as_ref(), &prepared)?; if prepared.allow_launch_services_active { @@ -641,3 +641,87 @@ pub(crate) fn select_threading_context( exec_strategy::ThreadingContext::Strict } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::cli::SandboxArgs; + + fn run_args_with_sandbox(sandbox: SandboxArgs) -> RunArgs { + RunArgs { + sandbox, + detached: false, + detach_timeout_secs: None, + rollback: false, + no_rollback_prompt: false, + no_rollback: false, + rollback_exclude: Vec::new(), + rollback_include: Vec::new(), + rollback_all: false, + skip_dir: Vec::new(), + rollback_dest: None, + no_diagnostics: false, + diagnostics_json: false, + startup_timeout_secs: None, + no_audit: false, + no_audit_integrity: false, + audit_integrity: false, + audit_sign_key: None, + trust_override: false, + name: None, + capability_elevation: false, + command: Vec::new(), + help: None, + } + } + + #[test] + fn run_launch_plan_rejects_block_net_with_upstream_proxy() { + let _env_lock = crate::test_env::ENV_LOCK.lock().expect("env lock"); + let test_root = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("..") + .join("..") + .join("target") + .join("test-env") + .join(format!( + "run-launch-block-net-upstream-proxy-{}", + std::process::id() + )); + let home = test_root.join("home"); + let state = test_root.join("state"); + let config = test_root.join("config"); + let workdir = test_root.join("workdir"); + std::fs::create_dir_all(&home).expect("create test home"); + std::fs::create_dir_all(&state).expect("create test state"); + std::fs::create_dir_all(&config).expect("create test config"); + std::fs::create_dir_all(&workdir).expect("create test workdir"); + let _env = crate::test_env::EnvVarGuard::set_all(&[ + ("HOME", home.to_str().expect("home path is utf-8")), + ( + "XDG_STATE_HOME", + state.to_str().expect("state path is utf-8"), + ), + ( + "XDG_CONFIG_HOME", + config.to_str().expect("config path is utf-8"), + ), + ]); + + let run_args = run_args_with_sandbox(SandboxArgs { + allow_cwd: true, + workdir: Some(workdir), + block_net: true, + external_proxy: Some("squid.corp:3128".to_string()), + ..SandboxArgs::default() + }); + + let result = prepare_run_launch_plan(run_args, OsString::from("/bin/echo"), vec![], true); + let Err(err) = result else { + panic!("expected run launch plan to reject --block-net + --upstream-proxy"); + }; + assert!( + err.to_string().contains("--block-net") && err.to_string().contains("--upstream-proxy"), + "unexpected error: {err}" + ); + } +} diff --git a/crates/nono-cli/src/proxy_runtime.rs b/crates/nono-cli/src/proxy_runtime.rs index b296b344e..9aa16444a 100644 --- a/crates/nono-cli/src/proxy_runtime.rs +++ b/crates/nono-cli/src/proxy_runtime.rs @@ -1462,7 +1462,8 @@ pub(crate) fn prepare_proxy_launch_options( let has_credentials = !credentials.is_empty(); let would_activate = has_domain_filter || has_credentials || upstream_proxy_addr.is_some(); - // --block-net always wins; profile network.block yields to any proxy config. + // Normal launch validation rejects hard-block plus proxy-mode inputs before + // this fallback can choose BlockAll or strict filtering. let block_wins = args.block_net || (prepared.profile_network_block && !would_activate); if block_wins { if would_activate { @@ -1480,7 +1481,6 @@ pub(crate) fn prepare_proxy_launch_options( return Ok(NetworkIntent::BlockAll); } - // Profile network.block + proxy flags → strict mode: deny unlisted hosts. let strict_filter = prepared.profile_network_block; let (plain_entries, endpoint_entries): (Vec<_>, Vec<_>) = allow_domain diff --git a/crates/nono-cli/src/sandbox_prepare.rs b/crates/nono-cli/src/sandbox_prepare.rs index 8521d6d36..161f9d4d2 100644 --- a/crates/nono-cli/src/sandbox_prepare.rs +++ b/crates/nono-cli/src/sandbox_prepare.rs @@ -771,14 +771,17 @@ fn has_proxy_intent(args: &SandboxArgs, prepared: &PreparedSandbox) -> bool { || prepared.upstream_proxy.is_some() } +fn has_upstream_proxy(args: &SandboxArgs, prepared: &PreparedSandbox) -> bool { + args.external_proxy.is_some() || prepared.upstream_proxy.is_some() +} + pub(crate) fn validate_external_proxy_bypass( args: &SandboxArgs, prepared: &PreparedSandbox, ) -> Result<()> { let has_bypass = !args.external_proxy_bypass.is_empty() || !prepared.upstream_bypass.is_empty(); - let has_external_proxy = args.external_proxy.is_some() || prepared.upstream_proxy.is_some(); - if has_bypass && !has_external_proxy { + if has_bypass && !has_upstream_proxy(args, prepared) { return Err(NonoError::ConfigParse( "--upstream-bypass requires --upstream-proxy \ (or upstream_proxy in profile network config)" @@ -788,6 +791,14 @@ pub(crate) fn validate_external_proxy_bypass( Ok(()) } +pub(crate) fn validate_proxy_conflicts( + args: &SandboxArgs, + prepared: &PreparedSandbox, +) -> Result<()> { + validate_block_net_conflicts(args, prepared)?; + validate_external_proxy_bypass(args, prepared) +} + /// Validate that `--block-net` is not combined with flags that imply proxy /// mode, and that `--allow-endpoint` always has a matching credential. /// @@ -830,6 +841,14 @@ pub(crate) fn validate_block_net_conflicts( .to_string(), )); } + + if has_upstream_proxy(args, prepared) { + return Err(NonoError::ConfigParse( + "--block-net and --upstream-proxy are contradictory: \ + upstream proxy routing requires proxy mode" + .to_string(), + )); + } } // --allow-endpoint without any credential is a no-op (and almost certainly @@ -2486,6 +2505,24 @@ mod tests { ); } + #[test] + fn block_net_with_upstream_proxy_errors() { + let args = SandboxArgs { + block_net: true, + external_proxy: Some("squid.corp:3128".to_string()), + ..Default::default() + }; + let prepared = empty_prepared(); + let result = validate_block_net_conflicts(&args, &prepared); + let Err(err) = result else { + panic!("expected error for --block-net + --upstream-proxy"); + }; + assert!( + err.to_string().contains("--block-net") && err.to_string().contains("--upstream-proxy"), + "unexpected error: {err}" + ); + } + #[test] fn block_net_alone_is_valid() { let args = SandboxArgs { @@ -2510,6 +2547,22 @@ mod tests { ); } + #[test] + fn profile_network_block_with_upstream_proxy_from_profile_errors() { + let args = SandboxArgs::default(); + let mut prepared = empty_prepared(); + prepared.profile_network_block = true; + prepared.upstream_proxy = Some("squid.corp:3128".to_string()); + let result = validate_block_net_conflicts(&args, &prepared); + let Err(err) = result else { + panic!("expected error for profile network block + profile upstream proxy"); + }; + assert!( + err.to_string().contains("--block-net") && err.to_string().contains("--upstream-proxy"), + "unexpected error: {err}" + ); + } + #[test] fn allow_endpoint_without_credential_errors() { let args = SandboxArgs {