diff --git a/Cargo.lock b/Cargo.lock index c18df7e3c..21cd96b90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1319,6 +1319,7 @@ name = "grab" version = "6.4.5" dependencies = [ "axum", + "base64", "futures-util", "getrandom 0.4.3", "gettext-rs", diff --git a/Cargo.toml b/Cargo.toml index 9da984b70..7ea31c0e9 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,6 +7,8 @@ description = "A GNOME download manager (GTK 4 / libadwaita, reqwest backend)" license = "GPL-3.0-only" [dependencies] +# Encode the per-run guarded-proxy credential for HTTP Proxy-Authorization. +base64 = "0.23.1" # Version features must match the system libs (GTK 4.22+ / Adw 1.9+, GNOME 51 runtime). gtk4 = { version = "0.11", features = ["v4_10", "v4_12", "v4_14", "v4_16", "v4_18", "v4_20", "v4_22"] } libadwaita = { version = "0.9", features = ["v1_4", "v1_5", "v1_6", "v1_7", "v1_8", "v1_9"] } diff --git a/src/cookies.rs b/src/cookies.rs index 67c749356..1536b00c5 100644 --- a/src/cookies.rs +++ b/src/cookies.rs @@ -277,9 +277,8 @@ async fn stage_cookie_file( // Both cookie sources are intentional here: `--cookies` is this export's // output file and `--cookies-from-browser` is what fills it, so this spawn // must not use the handoff-aware ytdlp_spawn_identity. - let mut identity_args = - crate::video_tools::ytdlp_identity_args(cookies_browser, None, page_url); - let guarded_proxy = match crate::video_spawn::guard_untrusted_args(&mut identity_args) { + let identity_args = crate::video_tools::ytdlp_identity_args(cookies_browser, None, page_url); + let guarded_proxy = match crate::video_spawn::guard_untrusted_args(&identity_args) { Ok(guarded) => guarded, Err(e) => { tracing::warn!(error = %e, "refusing cookie export without guarded extractor networking"); diff --git a/src/gallery_probe.rs b/src/gallery_probe.rs index 420c4e375..ea7d184b8 100644 --- a/src/gallery_probe.rs +++ b/src/gallery_probe.rs @@ -303,13 +303,7 @@ fn apply_guarded_proxy( )); } let proxy = crate::subprocess_proxy::ensure_started().map_err(GalleryError::probe)?; - cmd.arg("--proxy").arg(&proxy); - cmd.env("NO_PROXY", "") - .env("no_proxy", "") - .env("HTTP_PROXY", &proxy) - .env("http_proxy", &proxy) - .env("HTTPS_PROXY", &proxy) - .env("https_proxy", &proxy); + crate::video_spawn::apply_guarded_proxy_env(cmd, Some(&proxy)); Ok(true) } @@ -423,9 +417,6 @@ async fn run_to_completion( .spawn() .map_err(|e| GalleryError::install(format!("couldn't spawn gallery-dl: {e}")))?; let mut guard = ProcessGroupGuard::new(&child); - // Recorded before the wait so the drain-timeout branch can still signal - // the group after the reap disarms the guard. - let pgid = guard.pgid(); let stdout = child .stdout .take() @@ -502,26 +493,12 @@ async fn run_to_completion( ))); } Err(_) => { - // EOF never came; the length caps already watch the bytes, so - // this is a pipe inherited by a surviving descendant. Abort the - // drains, then kill the group by the recorded PGID (the guard - // disarmed at reap) and fail the probe instead of hanging it. - // - // The reap is up to 60 s old (DRAIN_AFTER_EXIT_TIMEOUT), so the - // PGID may be free or recycled: this is a best-effort signal for - // a descendant still in the group, not a guarantee. One that - // setsid'd away is unreachable and survives (now without pipe - // readers). + // EOF never came: a descendant may have inherited a pipe after + // the direct child was reaped. Abort the drains and fail the probe, + // but do not signal the old PGID: it may have been recycled after + // the leader exited, so killpg could terminate unrelated work. out_abort.abort(); err_abort.abort(); - #[cfg(unix)] - if let Some(pgid) = pgid { - // SAFETY: constant signal number; ESRCH (already dead) is - // harmless. - unsafe { - libc::killpg(pgid, libc::SIGKILL); - } - } return Err(GalleryError::probe(format!( "{what} didn't close its pipes after exiting" ))); @@ -1133,7 +1110,7 @@ pub async fn count_gallery_files_with_proxy( if untrusted_handoff && proxy.is_some() { return None; } - let mut argv = crate::gallery_argv::count_argv_with_proxy( + let argv = crate::gallery_argv::count_argv_with_proxy( tool, url, cookies_file, @@ -1143,9 +1120,8 @@ pub async fn count_gallery_files_with_proxy( proxy, ); let guarded_proxy = if untrusted_handoff { - let proxy = crate::subprocess_proxy::ensure_started().ok()?; - crate::video_spawn::insert_guarded_proxy_arg(&mut argv, &proxy).ok()?; - Some(proxy) + crate::video_spawn::validate_guarded_proxy_args(&argv).ok()?; + Some(crate::subprocess_proxy::ensure_started().ok()?) } else { None }; diff --git a/src/gallery_runner.rs b/src/gallery_runner.rs index f379a1c07..5cb2af1b5 100644 --- a/src/gallery_runner.rs +++ b/src/gallery_runner.rs @@ -129,7 +129,7 @@ pub async fn run_gallery_download_with_proxy( "Cannot safely use a configured proxy for an untrusted browser handoff".to_string(), )); } - let mut argv = download_argv_with_proxy( + let argv = download_argv_with_proxy( tool, &job.page_url, &job.dest_dir, @@ -141,10 +141,8 @@ pub async fn run_gallery_download_with_proxy( proxy, ); let guarded_proxy = if untrusted_handoff { - let proxy = crate::subprocess_proxy::ensure_started().map_err(GalleryError::probe)?; - crate::video_spawn::insert_guarded_proxy_arg(&mut argv, &proxy) - .map_err(GalleryError::probe)?; - Some(proxy) + crate::video_spawn::validate_guarded_proxy_args(&argv).map_err(GalleryError::probe)?; + Some(crate::subprocess_proxy::ensure_started().map_err(GalleryError::probe)?) } else { None }; diff --git a/src/subprocess_proxy.rs b/src/subprocess_proxy.rs index 423c3784d..fbd66e79a 100644 --- a/src/subprocess_proxy.rs +++ b/src/subprocess_proxy.rs @@ -7,6 +7,8 @@ //! HTTP proxy requests. It is only enabled for browser-handoff hosts whose //! public-resolution policy is active. +use base64::{Engine as _, engine::general_purpose::STANDARD}; +use std::fmt::Write as _; use std::io; use std::net::{SocketAddr, TcpListener as StdTcpListener}; use std::pin::Pin; @@ -18,7 +20,64 @@ use tokio::io::{AsyncRead, AsyncReadExt as _, AsyncWrite, AsyncWriteExt as _, Re use tokio::net::{TcpListener, TcpStream}; use tokio::sync::Semaphore; -static PROXY_ADDR: OnceLock = OnceLock::new(); +static PROXY_STATE: OnceLock = OnceLock::new(); + +#[derive(Clone)] +struct ProxyState { + addr: SocketAddr, + token: String, + expected_authorization: String, +} + +impl ProxyState { + fn proxy_url(&self) -> String { + format!("http://grab:{}@{}", self.token, self.addr) + } +} + +fn proxy_authorization_header(token: &str) -> String { + format!("Basic {}", STANDARD.encode(format!("grab:{token}"))) +} + +fn random_proxy_token() -> Result { + let mut bytes = [0u8; 32]; + getrandom::fill(&mut bytes) + .map_err(|error| format!("couldn't generate guarded proxy token: {error}"))?; + let mut token = String::with_capacity(bytes.len() * 2); + for byte in bytes { + write!(&mut token, "{byte:02x}").expect("writing to a String cannot fail"); + } + Ok(token) +} + +fn constant_time_equal(left: &[u8], right: &[u8]) -> bool { + if left.len() != right.len() { + return false; + } + let mut difference = 0u8; + for (&a, &b) in left.iter().zip(right) { + difference |= a ^ b; + } + difference == 0 +} + +/// Match exactly one Proxy-Authorization header; duplicated headers are rejected. +fn proxy_authorization_matches(header: &[u8], expected: &str) -> bool { + let text = String::from_utf8_lossy(header); + let mut provided = None; + for line in text.split("\r\n").skip(1) { + let Some((name, value)) = line.split_once(':') else { + continue; + }; + if name.trim().eq_ignore_ascii_case("proxy-authorization") { + if provided.is_some() { + return false; + } + provided = Some(value.trim()); + } + } + provided.is_some_and(|value| constant_time_equal(value.as_bytes(), expected.as_bytes())) +} const MAX_HEADER_BYTES: usize = 64 * 1024; const HEADER_TIMEOUT: Duration = Duration::from_secs(10); /// Bound DNS resolution and sequential public-address connection attempts too: @@ -148,22 +207,24 @@ pub(crate) fn validate_proxy_compatibility() -> Result<(), String> { Ok(()) } -/// Start the proxy once and return its HTTP proxy URL. +/// Start the proxy once and return an authentication-bearing URL for the +/// child environment. The random credential is created once per Grab process. pub(crate) fn ensure_started() -> Result { validate_proxy_compatibility()?; - if let Some(addr) = PROXY_ADDR.get() { - return Ok(format!("http://{addr}")); + if let Some(state) = PROXY_STATE.get() { + return Ok(state.proxy_url()); } let listener = StdTcpListener::bind(("127.0.0.1", 0)) .map_err(|e| format!("couldn't start the guarded extractor proxy: {e}"))?; - start_proxy_listener(listener, &PROXY_ADDR) + start_proxy_listener(listener, &PROXY_STATE, random_proxy_token()?) } /// Register and serve a listener under Grab's process-lifetime runtime. The /// state parameter lets tests exercise isolated initialization state too. fn start_proxy_listener( listener: StdTcpListener, - state: &OnceLock, + state: &OnceLock, + token: String, ) -> Result { listener .set_nonblocking(true) @@ -171,6 +232,7 @@ fn start_proxy_listener( let addr = listener .local_addr() .map_err(|e| format!("couldn't read guarded proxy address: {e}"))?; + let expected_authorization = proxy_authorization_header(&token); let runtime = crate::runtime::tokio_rt(); let listener = { let _entered = runtime.enter(); @@ -178,8 +240,16 @@ fn start_proxy_listener( .map_err(|e| format!("couldn't start the guarded extractor proxy: {e}"))? }; - match state.set(addr) { + match state.set(ProxyState { + addr, + token, + expected_authorization, + }) { Ok(()) => { + let expected_authorization = state + .get() + .map(|state| state.expected_authorization.clone()) + .ok_or_else(|| "guarded proxy state was not initialized".to_string())?; runtime.spawn(async move { loop { let permit = match Arc::clone(&PROXY_CONNECTIONS).acquire_owned().await { @@ -188,9 +258,12 @@ fn start_proxy_listener( }; match listener.accept().await { Ok((stream, _peer)) => { + let expected_authorization = expected_authorization.clone(); tokio::spawn(async move { let _permit = permit; - if let Err(e) = serve_connection(stream).await { + if let Err(e) = + serve_connection(stream, &expected_authorization).await + { tracing::debug!(error = %e, "guarded extractor proxy request failed"); } }); @@ -203,12 +276,15 @@ fn start_proxy_listener( } } }); - Ok(format!("http://{addr}")) + state + .get() + .map(ProxyState::proxy_url) + .ok_or_else(|| "guarded proxy state was not initialized".to_string()) } - Err(_) => Ok(format!( - "http://{}", - state.get().expect("proxy address initialized") - )), + Err(_) => state + .get() + .map(ProxyState::proxy_url) + .ok_or_else(|| "guarded proxy state was not initialized".to_string()), } } @@ -360,7 +436,18 @@ async fn reply_status(stream: &mut TcpStream, code: u16, reason: &str) { let _ = stream.write_all(response.as_bytes()).await; } -async fn serve_connection(mut client: TcpStream) -> std::io::Result<()> { +async fn reply_proxy_auth_required(stream: &mut TcpStream) -> io::Result<()> { + stream + .write_all( + b"HTTP/1.1 407 Proxy Authentication Required\r\nProxy-Authenticate: Basic realm=\"Grab guarded extractor proxy\"\r\nConnection: close\r\nContent-Length: 0\r\n\r\n", + ) + .await +} + +async fn serve_connection( + mut client: TcpStream, + expected_authorization: &str, +) -> std::io::Result<()> { let (header, trailing) = match read_header_with_timeout(&mut client, HEADER_TIMEOUT).await { Ok(parts) => parts, Err(error) => { @@ -372,6 +459,10 @@ async fn serve_connection(mut client: TcpStream) -> std::io::Result<()> { return Err(error); } }; + if !proxy_authorization_matches(&header, expected_authorization) { + reply_proxy_auth_required(&mut client).await?; + return Ok(()); + } let text = String::from_utf8_lossy(&header); let mut lines = text.split("\r\n"); let Some(request_line) = lines.next() else { @@ -485,17 +576,22 @@ mod tests { caller_runtime .block_on(async { let listener = StdTcpListener::bind(("127.0.0.1", 0)).unwrap(); - start_proxy_listener(listener, &state) + start_proxy_listener(listener, &state, "test-token".to_string()) }) .expect("start guarded proxy") }) .join() .expect("caller thread"); - let address: SocketAddr = proxy_url - .strip_prefix("http://") - .expect("proxy URL scheme") - .parse() - .expect("proxy address"); + let parsed = url::Url::parse(&proxy_url).expect("proxy URL"); + assert_eq!(parsed.username(), "grab"); + assert_eq!(parsed.password(), Some("test-token")); + let address: SocketAddr = format!( + "{}:{}", + parsed.host_str().expect("proxy host"), + parsed.port().expect("proxy port") + ) + .parse() + .expect("proxy address"); let response = crate::runtime::tokio_rt() .block_on(async { @@ -514,8 +610,8 @@ mod tests { .expect("proxy connection and request"); assert!( - String::from_utf8_lossy(&response).starts_with("HTTP/1.1 403"), - "proxy should survive caller runtime shutdown: {}", + String::from_utf8_lossy(&response).starts_with("HTTP/1.1 407"), + "proxy should require authentication and survive caller runtime shutdown: {}", String::from_utf8_lossy(&response) ); } @@ -613,16 +709,20 @@ mod tests { async fn proxy_rejects_private_connect_destinations() { let listener = TcpListener::bind(("127.0.0.1", 0)).await.unwrap(); let proxy_addr = listener.local_addr().unwrap(); + let expected_authorization = proxy_authorization_header("test-token"); let server = tokio::spawn(async move { let (stream, _) = listener.accept().await.unwrap(); - serve_connection(stream).await.unwrap(); + serve_connection(stream, &expected_authorization) + .await + .unwrap(); }); let mut client = TcpStream::connect(proxy_addr).await.unwrap(); - client - .write_all(b"CONNECT 127.0.0.1:9 HTTP/1.1\r\nHost: 127.0.0.1:9\r\n\r\n") - .await - .unwrap(); + let request = format!( + "CONNECT 127.0.0.1:9 HTTP/1.1\r\nHost: 127.0.0.1:9\r\nProxy-Authorization: {}\r\n\r\n", + proxy_authorization_header("test-token") + ); + client.write_all(request.as_bytes()).await.unwrap(); let mut response = Vec::new(); client.read_to_end(&mut response).await.unwrap(); assert!( diff --git a/src/video_runner.rs b/src/video_runner.rs index 72d1e24d2..dbda531b8 100644 --- a/src/video_runner.rs +++ b/src/video_runner.rs @@ -738,8 +738,8 @@ async fn run_ytdlp_attempt( timeout: Duration, ) -> Result<(Option<()>, Option, Option), VideoError> { use tokio::io::AsyncBufReadExt as _; - let mut guarded_argv = argv.to_vec(); - let guarded_proxy = crate::video_spawn::guard_untrusted_args(&mut guarded_argv)?; + let guarded_argv = argv.to_vec(); + let guarded_proxy = crate::video_spawn::guard_untrusted_args(&guarded_argv)?; let mut cmd = ytdlp_command(youtube_bin).await; cmd.args(&guarded_argv); apply_proxy_env(&mut cmd, proxy); @@ -1284,7 +1284,7 @@ pub(crate) async fn resolve_subtitle_lang( &job.cookies_browser, &job.page_url, )); - let guarded_proxy = match crate::video_spawn::guard_untrusted_args(&mut argv) { + let guarded_proxy = match crate::video_spawn::guard_untrusted_args(&argv) { Ok(proxy) => proxy, Err(e) => { tracing::warn!(error = %e, "subtitle probe refused without guarded extractor networking"); @@ -1723,7 +1723,7 @@ pub(crate) async fn run_live_ytdlp( extension_picky_supported, ); splice_before_terminator(&mut argv, live_info_json_args(live_info_path.as_deref())); - let guarded_proxy = crate::video_spawn::guard_untrusted_args(&mut argv)?; + let guarded_proxy = crate::video_spawn::guard_untrusted_args(&argv)?; let mut cmd = ytdlp_command(youtube_bin).await; cmd.args(&argv); apply_proxy_env(&mut cmd, job.proxy.as_ref()); @@ -2125,8 +2125,8 @@ pub(crate) async fn run_hls_ytdlp( Ok(lang) => lang, Err(()) => return Ok(None), }; - let mut argv = hls_download_argv(&job, hls_format_id, ffmpeg_bin, &job.dest, playlist_index); - let guarded_proxy = crate::video_spawn::guard_untrusted_args(&mut argv)?; + let argv = hls_download_argv(&job, hls_format_id, ffmpeg_bin, &job.dest, playlist_index); + let guarded_proxy = crate::video_spawn::guard_untrusted_args(&argv)?; let mut cmd = ytdlp_command(youtube_bin).await; cmd.args(&argv); apply_proxy_env(&mut cmd, job.proxy.as_ref()); diff --git a/src/video_spawn.rs b/src/video_spawn.rs index bf167b8d8..b1641e207 100644 --- a/src/video_spawn.rs +++ b/src/video_spawn.rs @@ -93,7 +93,7 @@ pub(crate) async fn fetch_raw_dump_json( cookies_browser, url, )); - let guarded_proxy = guard_untrusted_args(&mut args)?; + let guarded_proxy = guard_untrusted_args(&args)?; // Direct spawn with the same semantics (concurrent drain, timeout kill, stderr detail on failure). let mut cmd = ytdlp_command(youtube_bin).await; // Belt-and-braces with the ProcessGroupGuard below (which SIGKILLs the @@ -363,27 +363,24 @@ pub(crate) async fn ytdlp_command(youtube_bin: &Path) -> tokio::process::Command cmd } -/// Insert the guarded proxy before yt-dlp/gallery-dl's positional URL terminator. -/// Shared by both runners so option placement and conflict checks stay identical. -pub(crate) fn insert_guarded_proxy_arg( - args: &mut Vec, - proxy: &str, -) -> Result<(), &'static str> { +/// Validate that extractor argv has a URL and no explicit proxy override. +/// The authenticated proxy URL must stay out of argv because debug logging +/// records arguments; the child receives it through standard proxy variables. +pub(crate) fn validate_guarded_proxy_args(args: &[String]) -> Result<(), &'static str> { let Some(separator) = args.iter().position(|arg| arg == "--") else { return Err("Extractor arguments are missing the URL terminator"); }; + if args.get(separator + 1).is_none() { + return Err("Extractor arguments are missing the URL after the terminator"); + } if args[..separator].iter().any(|arg| arg == "--proxy") { return Err("Cannot safely enforce public-address checks through a configured proxy"); } - args.splice( - separator..separator, - ["--proxy".to_string(), proxy.to_string()], - ); Ok(()) } /// Apply the loopback proxy when the initial page URL is a marked browser handoff. -pub(crate) fn guard_untrusted_args(args: &mut Vec) -> Result, VideoError> { +pub(crate) fn guard_untrusted_args(args: &[String]) -> Result, VideoError> { let Some(separator) = args.iter().position(|arg| arg == "--") else { return Err(VideoError::fetch( "Extractor arguments are missing the URL terminator", @@ -403,13 +400,13 @@ pub(crate) fn guard_untrusted_args(args: &mut Vec) -> Result) { if let Some(proxy) = proxy { cmd.env("NO_PROXY", "") @@ -667,33 +664,31 @@ mod tests { use super::*; #[test] - fn guarded_proxy_arg_precedes_url_terminator_and_rejects_existing_proxy() { + fn guarded_proxy_validation_leaves_credentials_out_of_argv_and_rejects_overrides() { let target = "https://video.example.invalid/watch"; - let mut args = vec![ + let args = vec![ "--ignore-config".to_string(), "--".to_string(), target.to_string(), ]; - insert_guarded_proxy_arg(&mut args, "http://127.0.0.1:43123") - .expect("guarded proxy should be inserted"); - let separator = args.iter().position(|arg| arg == "--").unwrap(); - assert_eq!(args[separator - 2], "--proxy"); - assert_eq!(args[separator - 1], "http://127.0.0.1:43123"); - assert_eq!(args[separator + 1], target); - - let mut conflict = vec![ + let original = args.clone(); + validate_guarded_proxy_args(&args).expect("argv can use the guarded proxy"); + assert_eq!(args, original, "credentials must stay out of argv"); + + let conflict = vec![ "--proxy".to_string(), "http://example-proxy.invalid".to_string(), "--".to_string(), target.to_string(), ]; - assert!(insert_guarded_proxy_arg(&mut conflict, "http://127.0.0.1:43123").is_err()); + assert!(validate_guarded_proxy_args(&conflict).is_err()); + assert!(validate_guarded_proxy_args(&["--ignore-config".to_string()]).is_err()); } #[test] fn guarded_argv_without_url_terminator_fails_closed() { - let mut args = vec!["--ignore-config".to_string()]; - assert!(guard_untrusted_args(&mut args).is_err()); + let args = vec!["--ignore-config".to_string()]; + assert!(guard_untrusted_args(&args).is_err()); } #[test] @@ -702,10 +697,10 @@ mod tests { "https://user-entered-{}.example.invalid/watch", std::process::id() ); - let mut args = vec!["--ignore-config".to_string(), "--".to_string(), url]; + let args = vec!["--ignore-config".to_string(), "--".to_string(), url]; let original = args.clone(); assert!( - guard_untrusted_args(&mut args) + guard_untrusted_args(&args) .expect("ordinary user-entered URLs need no guarded proxy") .is_none() ); @@ -716,7 +711,7 @@ mod tests { fn guarded_proxy_environment_clears_bypasses_and_covers_external_downloaders() { use std::ffi::OsStr; - let proxy = "http://127.0.0.1:43123"; + let proxy = "http://grab:0123456789abcdef@127.0.0.1:43123"; let mut command = tokio::process::Command::new("ffmpeg"); apply_guarded_proxy_env(&mut command, Some(proxy)); let envs: Vec<_> = command diff --git a/src/video_tools.rs b/src/video_tools.rs index 744514986..bbf627889 100644 --- a/src/video_tools.rs +++ b/src/video_tools.rs @@ -589,6 +589,11 @@ async fn ytdlp_supports_impersonation_cached(key: PathBuf) -> bool { /// draining are asynchronous. Timeout cleanup signals the whole process group. async fn probe_impersonate_support(youtube_bin: &Path) -> Result { use std::process::Stdio; + + const PROBE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(15); + const OUTPUT_DRAIN_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(2); + const MAX_OUTPUT_BYTES: u64 = 1024 * 1024; + let mut command = tokio::process::Command::new(youtube_bin); command .arg("--list-impersonate-targets") @@ -601,29 +606,87 @@ async fn probe_impersonate_support(youtube_bin: &Path) -> Result { - // The child has exited, regardless of its exit code. Do not let - // the drop guard signal a process group whose PID may be reused. + let stdout = child + .stdout + .take() + .ok_or_else(|| ToolProbeError::Wait("yt-dlp gave no output pipe".to_string()))?; + let stdout_task = tokio::spawn(read_probe_stdout_bounded(stdout, MAX_OUTPUT_BYTES)); + let stdout_abort = stdout_task.abort_handle(); + + let status = match tokio::time::timeout(PROBE_TIMEOUT, child.wait()).await { + Ok(Ok(status)) => { + // Reaped: this process-group ID is no longer safe to signal. group.disarm(); - if !output.status.success() { - return Err(ToolProbeError::FailedExit); - } - let out = String::from_utf8_lossy(&output.stdout); - let supported = out.lines().any(|line| { - let line = line.to_ascii_lowercase(); - line.contains("chrome") && !line.contains("unavailable") - }); - Ok(supported) + status + } + Ok(Err(error)) => { + // The reap state is uncertain; leave process-group signaling off. + // Child::drop still has kill_on_drop enabled for the direct process. + group.disarm(); + stdout_abort.abort(); + return Err(ToolProbeError::Wait(error.to_string())); } - Ok(Err(error)) => Err(ToolProbeError::Wait(error.to_string())), - Err(_) => Err(ToolProbeError::Timeout), + Err(_) => { + // child.wait() was cancelled while the owned Child remains alive. + // Kill its group before dropping the child, then disarm the guard. + group.kill_now(); + stdout_abort.abort(); + return Err(ToolProbeError::Timeout); + } + }; + + let output = match tokio::time::timeout(OUTPUT_DRAIN_TIMEOUT, stdout_task).await { + Ok(Ok(Ok(output))) => output, + Ok(Ok(Err(error))) => return Err(ToolProbeError::Wait(error.to_string())), + Ok(Err(error)) => return Err(ToolProbeError::Wait(error.to_string())), + Err(_) => { + // A descendant may keep stdout open after the leader exits. Abort + // the drain but do not signal a potentially recycled process group. + stdout_abort.abort(); + return Err(ToolProbeError::Timeout); + } + }; + + if !status.success() { + return Err(ToolProbeError::FailedExit); } + let out = String::from_utf8_lossy(&output); + let supported = out.lines().any(|line| { + let line = line.to_ascii_lowercase(); + line.contains("chrome") && !line.contains("unavailable") + }); + Ok(supported) +} + +/// Drain the impersonation probe's stdout into a bounded buffer. Always drains +/// excess bytes to EOF so the child cannot block on a full pipe while being waited. +async fn read_probe_stdout_bounded( + stream: impl tokio::io::AsyncRead + Unpin, + limit: u64, +) -> std::io::Result> { + use tokio::io::AsyncReadExt as _; + + let mut output = Vec::new(); + let mut reader = tokio::io::BufReader::new(stream); + (&mut reader) + .take(limit + 1) + .read_to_end(&mut output) + .await?; + if output.len() as u64 <= limit { + return Ok(output); + } + + let mut discard = [0u8; 8192]; + while reader.read(&mut discard).await? != 0 {} + Err(std::io::Error::new( + std::io::ErrorKind::QuotaExceeded, + "impersonation probe output exceeded the size limit", + )) } /// Best-effort process-group cleanup when a probe is cancelled or times out. @@ -641,16 +704,22 @@ impl ProbeProcessGroup { fn disarm(&mut self) { self.armed = false; } -} -impl Drop for ProbeProcessGroup { - fn drop(&mut self) { + /// Signal only while the owned child is still unreaped. + fn kill_now(&mut self) { if self.armed { kill_probe_group(self.pid); + self.armed = false; } } } +impl Drop for ProbeProcessGroup { + fn drop(&mut self) { + self.kill_now(); + } +} + fn kill_probe_group(pid: u32) { #[cfg(unix)] if pid != 0 {