diff --git a/docs/bwrap-support/bubblewrap-backend.md b/docs/bwrap-support/bubblewrap-backend.md index c1851c7c0..6232845d8 100644 --- a/docs/bwrap-support/bubblewrap-backend.md +++ b/docs/bwrap-support/bubblewrap-backend.md @@ -260,10 +260,10 @@ Bubblewrap because it requires **no root and no `CAP_NET_ADMIN`**. supply their own proxy via `localhost: ` or `url: `. 2. The sandbox is then started **without** `--unshare-net` so the sandbox shares the host network namespace and can reach the loopback proxy. -3. The command builder sets `HTTP_PROXY`, `HTTPS_PROXY`, `http_proxy`, and - `https_proxy` inside the sandbox via `bwrap --setenv` (any - caller-supplied values for these keys, including `NO_PROXY` / - `no_proxy`, are stripped before injection). The runner deliberately +3. The command builder sets `HTTP_PROXY`, `HTTPS_PROXY`, `ALL_PROXY`, + `FTP_PROXY`, and their lowercase variants inside the sandbox via + `bwrap --setenv` (caller-supplied values for these keys, including + `NO_PROXY` / `no_proxy`, are stripped before injection). The runner deliberately does **not** set `NO_PROXY`: since the sandbox shares the host netns, a `NO_PROXY=localhost,127.0.0.1` entry would let cooperating clients bypass the proxy for host-loopback destinations, defeating diff --git a/src/backends/bubblewrap/common/src/bwrap_command.rs b/src/backends/bubblewrap/common/src/bwrap_command.rs index 770a480fc..8fed24071 100644 --- a/src/backends/bubblewrap/common/src/bwrap_command.rs +++ b/src/backends/bubblewrap/common/src/bwrap_command.rs @@ -167,7 +167,7 @@ pub fn build_args(request: &ExecutionRequest, proxy_address: Option<&ProxyAddres /// - drops `--unshare-net` (the sandbox needs to reach the loopback proxy on /// the host's network namespace), /// - strips any caller-supplied `HTTP_PROXY` / `HTTPS_PROXY` / `ALL_PROXY` / -/// `NO_PROXY` entries from `request.env`, +/// `FTP_PROXY` / `NO_PROXY` entries from `request.env`, /// - emits `--setenv` for the proxy keys (all but `NO_PROXY`) pointing at the /// proxy URL. /// @@ -731,6 +731,9 @@ mod tests { "HTTP_PROXY=http://attacker.example:9999".into(), "https_proxy=http://attacker.example:9999".into(), "ALL_PROXY=http://attacker.example:9999".into(), + "FTP_PROXY=http://attacker.example:9999".into(), + "ftp_proxy=http://attacker.example:9999".into(), + "NO_PROXY=*".into(), "PATH=/usr/bin".into(), ]; let addr = ProxyAddress::new("127.0.0.1".into(), 9000); @@ -749,6 +752,25 @@ mod tests { // The proxy URL is the one we set, not the attacker's. let http_pos = args.iter().position(|a| a == "HTTP_PROXY").unwrap(); assert_eq!(args[http_pos + 1], "http://127.0.0.1:9000"); + + // FTP variables point at the configured proxy rather than a + // caller-controlled alternative. + for key in ["FTP_PROXY", "ftp_proxy"] { + let pos = args + .iter() + .position(|arg| arg == key) + .unwrap_or_else(|| panic!("missing --setenv {key} in {args:?}")); + assert_eq!(args[pos + 1], "http://127.0.0.1:9000"); + } + + // Bypass variables remain absent after clearing the caller's + // environment. + for key in ["NO_PROXY", "no_proxy"] { + assert!( + !args.iter().any(|arg| arg == key), + "proxy mode must not emit caller-controlled {key}: {args:?}" + ); + } } #[test] diff --git a/src/core/wxc_common/src/proxy_env.rs b/src/core/wxc_common/src/proxy_env.rs index 484a1a698..8ff00eba2 100644 --- a/src/core/wxc_common/src/proxy_env.rs +++ b/src/core/wxc_common/src/proxy_env.rs @@ -15,7 +15,7 @@ //! workload cannot pre-disable the proxy via its own `HTTP_PROXY` (or a //! `NO_PROXY` exemption). This only sanitizes the *initial* env; the model //! is cooperative, so a workload can still mutate its own env at runtime. -//! 2. **Set** the HTTP/HTTPS/ALL proxy keys ([`PROXY_SET_KEYS`]) to the +//! 2. **Set** the HTTP/HTTPS/ALL/FTP proxy keys ([`PROXY_SET_KEYS`]) to the //! configured URL — never `NO_PROXY` (a host exemption list, not a target). //! //! `NO_PROXY` is kept out of [`PROXY_SET_KEYS`] and handled per-backend: @@ -35,25 +35,29 @@ pub const PROXY_ENV_KEYS: &[&str] = &[ "HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", + "FTP_PROXY", "http_proxy", "https_proxy", "all_proxy", + "ftp_proxy", "NO_PROXY", "no_proxy", ]; /// Proxy env var keys that are actively *set* to the configured proxy URL. /// -/// The HTTP/HTTPS/ALL keys (upper- and lower-case) are set. `NO_PROXY` is +/// The HTTP/HTTPS/ALL/FTP keys (upper- and lower-case) are set. `NO_PROXY` is /// deliberately omitted (it is a host-exemption list, not a proxy target; see /// module docs and [`PROXY_NEUTRALIZE_KEYS`]). pub const PROXY_SET_KEYS: &[&str] = &[ "HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", + "FTP_PROXY", "http_proxy", "https_proxy", "all_proxy", + "ftp_proxy", ]; /// Proxy env var keys that [`apply_cooperative_proxy_env`] sets to the *empty @@ -129,7 +133,7 @@ mod tests { use super::*; #[test] - fn sets_all_http_https_proxy_keys_to_url() { + fn sets_all_proxy_keys_to_url() { let env = apply_cooperative_proxy_env(&[], "http://127.0.0.1:8080"); for key in PROXY_SET_KEYS { assert!( @@ -139,6 +143,15 @@ mod tests { } } + #[test] + fn sets_ftp_proxy_to_override_image_environment() { + // WSLc merges these entries over image ENV, so emitting both spellings + // is what overrides an image-baked FTP proxy. + let env = apply_cooperative_proxy_env(&[], "http://127.0.0.1:8080"); + assert!(env.contains(&"FTP_PROXY=http://127.0.0.1:8080".to_string())); + assert!(env.contains(&"ftp_proxy=http://127.0.0.1:8080".to_string())); + } + #[test] fn sets_no_proxy_empty() { // NO_PROXY / no_proxy are forced to the empty string (never a value), @@ -166,6 +179,8 @@ mod tests { "FOO=bar".to_string(), "HTTP_PROXY=http://attacker.example:9999".to_string(), "https_proxy=http://attacker.example:9999".to_string(), + "FTP_PROXY=http://attacker.example:9999".to_string(), + "ftp_proxy=http://attacker.example:9999".to_string(), "NO_PROXY=example.com".to_string(), "PATH=/usr/bin".to_string(), ];