From 32a4a2ddc98c5153fe324e6b201812c54e123147 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 07:08:28 +0000 Subject: [PATCH 01/22] feat(scan): add restricted mode that ignores agent-editable config reach --- src/handlers/scans/commits.rs | 7 ++++- src/handlers/scans/files.rs | 7 ++++- src/models/scans.rs | 50 +++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/handlers/scans/commits.rs b/src/handlers/scans/commits.rs index 09ab5363..1b752759 100644 --- a/src/handlers/scans/commits.rs +++ b/src/handlers/scans/commits.rs @@ -11,7 +11,7 @@ use crate::{ scans::{ CommitChanges, CommitsScanResponse, DiffHunk, DiffProcessingState, FileHunks, IgnoredSecretsPayload, MatchConfigPayload, ProjectContextConfigPayload, - ScanCommitChangesPayload, ScanConfig, ScanOutputJson, + ScanCommitChangesPayload, ScanConfig, ScanOutputJson, SCAN_RESTRICTED_ENV, }, validation::{InputValidationError, ScanInputValidationError}, }, @@ -84,6 +84,11 @@ pub async fn handle_scan_unpushed_commit_hunks( }, None => ScanConfig::default(), }; + let config = if std::env::var(SCAN_RESTRICTED_ENV).as_deref() == Ok("1") { + config.restricted_for_broker() + } else { + config + }; let exclude = default_scan_exclude_patterns() .into_iter() diff --git a/src/handlers/scans/files.rs b/src/handlers/scans/files.rs index 43526aed..3cb710e7 100644 --- a/src/handlers/scans/files.rs +++ b/src/handlers/scans/files.rs @@ -6,7 +6,7 @@ use crate::{ scans::{ DiffHunk, DiffProcessingState, FileChangesScanResponse, FileHunks, IgnoredSecretsPayload, MatchConfigPayload, ProjectContextConfigPayload, ScanConfig, - ScanFileChangesPayload, ScanOutputJson, + ScanFileChangesPayload, ScanOutputJson, SCAN_RESTRICTED_ENV, }, validation::{InputValidationError, ScanInputValidationError}, }, @@ -145,6 +145,11 @@ async fn handle_scan_file_hunks( }, None => ScanConfig::default(), }; + let config = if std::env::var(SCAN_RESTRICTED_ENV).as_deref() == Ok("1") { + config.restricted_for_broker() + } else { + config + }; let exclude = default_scan_exclude_patterns() .into_iter() diff --git a/src/models/scans.rs b/src/models/scans.rs index 4a5473ef..3ce1f805 100644 --- a/src/models/scans.rs +++ b/src/models/scans.rs @@ -62,7 +62,22 @@ impl Default for ScanConfig { } } +/// Set by the Agent Proxy on the host-side scan it runs for a sandboxed +/// agent's git hook. The scan config lives in the repo, which the agent can +/// edit, so options that reach beyond narrowing results are ignored. +pub const SCAN_RESTRICTED_ENV: &str = "STASHBASE_SCAN_RESTRICTED"; + impl ScanConfig { + /// Drops `match_config` (matching against stored secrets and reading + /// arbitrary host files) and `output_dir` (writing outside the repo). + pub fn restricted_for_broker(self) -> Self { + Self { + match_config: None, + output_dir: None, + ..self + } + } + pub fn load_from_file(config_path: &str) -> Result { let file = std::fs::File::open(config_path).map_err(|e| { if e.kind() == std::io::ErrorKind::NotFound { @@ -721,3 +736,38 @@ impl ScanOutputJson { Ok(pretty_json) } } + +#[cfg(test)] +mod restricted_tests { + use super::ScanConfig; + + #[test] + fn restricted_config_drops_agent_controlled_reach_but_keeps_narrowing() { + let yaml = r#" +excluded-files: ["fixtures/**"] +output-dir: "/tmp/anywhere" +ignored-secrets: + hashes: ["aa"] + regexes: ["^test_"] +match: + project: + identifier: "prod" + environments: ["production"] + files: ["~/.aws/credentials"] +"#; + let config: ScanConfig = serde_yaml::from_str(yaml).unwrap(); + assert!(config.match_config.is_some() && config.output_dir.is_some()); + + let restricted = config.restricted_for_broker(); + + assert!(restricted.match_config.is_none()); + assert!(restricted.output_dir.is_none()); + assert_eq!( + restricted.excluded_files, + Some(vec!["fixtures/**".to_owned()]) + ); + let ignored = restricted.ignored_secrets.unwrap(); + assert_eq!(ignored.hashes, Some(vec!["aa".to_owned()])); + assert_eq!(ignored.regexes, Some(vec!["^test_".to_owned()])); + } +} From 6052061dcfb4aec48b8360b074c31e83147ecfac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 07:10:25 +0000 Subject: [PATCH 02/22] feat(agent): broker secret scans on the host through the agent proxy --- src/handlers/run/entry.rs | 21 +- src/handlers/run/proxy.rs | 466 ++++++++++++++++++++++++++++++++++---- 2 files changed, 435 insertions(+), 52 deletions(-) diff --git a/src/handlers/run/entry.rs b/src/handlers/run/entry.rs index 955704aa..fc2c580c 100644 --- a/src/handlers/run/entry.rs +++ b/src/handlers/run/entry.rs @@ -442,13 +442,18 @@ pub async fn handle_remote_agent_run( )?; (None, String::new()) }; + let remote_hooks = hooks_enabled.then(|| super::proxy::HookBrokerConfig { + api_key, + dependency_check: true, + secret_scan: None, + }); let proxy_start_result = if let Some(network) = &docker_network { super::proxy::Proxy::start_remote_with_hook_and_bind_host( remote, policy, audit_log, proxy_port, - hooks_enabled.then_some(api_key), + remote_hooks, &super::docker_sandbox::proxy_bind_host(network), ) .await @@ -458,7 +463,7 @@ pub async fn handle_remote_agent_run( policy, audit_log, proxy_port, - hooks_enabled.then_some(api_key), + remote_hooks, ) .await }; @@ -1701,13 +1706,21 @@ async fn handle_run( )?; (None, String::new()) }; + let local_hooks = dependency_hooks + .then_some(hook_api_key) + .flatten() + .map(|api_key| super::proxy::HookBrokerConfig { + api_key, + dependency_check: true, + secret_scan: None, + }); let proxy_start_result = if let Some(network) = &docker_network { super::proxy::Proxy::start_with_hook_and_bind_host( secrets_hash_map, proxy_policy.unwrap_or_else(super::proxy::ProxyPolicy::permissive), audit_log, proxy_port, - dependency_hooks.then_some(hook_api_key).flatten(), + local_hooks, &super::docker_sandbox::proxy_bind_host(network), ) .await @@ -1717,7 +1730,7 @@ async fn handle_run( proxy_policy.unwrap_or_else(super::proxy::ProxyPolicy::permissive), audit_log, proxy_port, - dependency_hooks.then_some(hook_api_key).flatten(), + local_hooks, ) .await }; diff --git a/src/handlers/run/proxy.rs b/src/handlers/run/proxy.rs index b2fea6bf..82a9d18f 100644 --- a/src/handlers/run/proxy.rs +++ b/src/handlers/run/proxy.rs @@ -84,6 +84,49 @@ const AUDIT_LOG_RETENTION: Duration = Duration::from_secs(30 * 24 * 60 * 60); const AUDIT_LOG_MAX_FILES: usize = 1_000; const MCP_INSPECTION_HEADER: &str = "x-stashbase-mcp-inspection"; const DEPENDENCY_HOOK_PATH: &str = "/__stashbase/dependency-check"; +const SECRET_SCAN_HOOK_PATH: &str = "/__stashbase/scan"; +/// Where a sandboxed git hook asks the proxy to run `stashbase scan` on the host. +pub const SCAN_BROKER_URL_ENV: &str = "STASHBASE_SCAN_BROKER_URL"; +const SECRET_SCAN_BODY_LIMIT: usize = 1024 * 1024; + +/// Authenticated hooks the proxy serves on the parent's behalf, so the child +/// never holds the Stashbase API key. +pub struct HookBrokerConfig { + pub api_key: String, + pub dependency_check: bool, + /// Enables the secret-scan route. + pub secret_scan: Option, +} + +pub struct SecretScanConfig { + /// The run's working directory, scanned on the host. + pub workdir: PathBuf, + /// Run as ` scan --json --silent`. + pub exe: PathBuf, + pub timeout: Duration, +} + +struct HookBroker { + token: String, + client: reqwest::Client, + api_key: String, + dependency_check: bool, + secret_scan: Option, + // One scan at a time: concurrent hooks would race on the same index. + scan_lock: tokio::sync::Mutex<()>, +} + +impl HookBroker { + fn authorized(&self, request: &Request) -> bool { + request.method() == Method::POST + && request.uri().query().is_none() + && request + .headers() + .get("authorization") + .and_then(|value| value.to_str().ok()) + .is_some_and(|value| value == format!("Bearer {}", self.token)) + } +} /// One metadata-only event emitted by the local proxy audit log. #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, Hash)] @@ -1014,8 +1057,7 @@ struct ProxyState { connections: Arc, remote: Option, mcp_inspection_token: String, - dependency_hook_token: Option, - dependency_hook_client: Option, + hook_broker: Option>, revocation_path: Arc>>, } @@ -1117,7 +1159,7 @@ impl Proxy { policy: ProxyPolicy, audit_log: Option, proxy_port: Option, - api_key: Option, + hooks: Option, ) -> Result { Self::start_inner( secrets, @@ -1125,7 +1167,7 @@ impl Proxy { audit_log, proxy_port, None, - api_key, + hooks, true, "127.0.0.1", ) @@ -1141,11 +1183,11 @@ impl Proxy { policy: ProxyPolicy, audit_log: Option, proxy_port: Option, - api_key: Option, + hooks: Option, bind_host: &str, ) -> Result { Self::start_inner( - secrets, policy, audit_log, proxy_port, None, api_key, true, bind_host, + secrets, policy, audit_log, proxy_port, None, hooks, true, bind_host, ) .await } @@ -1175,7 +1217,7 @@ impl Proxy { policy: ProxyPolicy, audit_log: Option, proxy_port: Option, - api_key: Option, + hooks: Option, ) -> Result { let placeholders = remote.placeholders.clone(); Self::start_inner( @@ -1184,7 +1226,7 @@ impl Proxy { audit_log, proxy_port, Some(remote), - api_key, + hooks, true, "127.0.0.1", ) @@ -1198,7 +1240,7 @@ impl Proxy { policy: ProxyPolicy, audit_log: Option, proxy_port: Option, - api_key: Option, + hooks: Option, bind_host: &str, ) -> Result { let placeholders = remote.placeholders.clone(); @@ -1208,7 +1250,7 @@ impl Proxy { audit_log, proxy_port, Some(remote), - api_key, + hooks, true, bind_host, ) @@ -1221,7 +1263,7 @@ impl Proxy { audit_log: Option, proxy_port: Option, remote: Option, - hook_api_key: Option, + hooks: Option, hook_mode_set: bool, bind_host: &str, ) -> Result { @@ -1333,16 +1375,22 @@ impl Proxy { connections: connections.clone(), remote, mcp_inspection_token: Uuid::new_v4().to_string(), - dependency_hook_token: hook_api_key.as_ref().map(|_| Uuid::new_v4().to_string()), - dependency_hook_client: hook_api_key.as_ref().map(|api_key| { - reqwest::Client::builder() - .no_proxy() - .default_headers(reqwest::header::HeaderMap::from_iter([( - reqwest::header::AUTHORIZATION, - format!("Bearer {api_key}").parse().unwrap(), - )])) - .build() - .expect("dependency hook client must build") + hook_broker: hooks.map(|hooks| { + Arc::new(HookBroker { + token: Uuid::new_v4().to_string(), + client: reqwest::Client::builder() + .no_proxy() + .default_headers(reqwest::header::HeaderMap::from_iter([( + reqwest::header::AUTHORIZATION, + format!("Bearer {}", hooks.api_key).parse().unwrap(), + )])) + .build() + .expect("hook broker client must build"), + api_key: hooks.api_key, + dependency_check: hooks.dependency_check, + secret_scan: hooks.secret_scan, + scan_lock: tokio::sync::Mutex::new(()), + }) }), revocation_path: revocation_path.clone(), }; @@ -1386,21 +1434,29 @@ impl Proxy { if hook_mode_set { child_env.insert( crate::api::dependencies::HOOK_MODE_ENV.to_owned(), - if hook_api_key.is_some() { + if state.hook_broker.is_some() { "broker" } else { "disabled" } .to_owned(), ); - if let (Some(token), Some(_)) = (&state.dependency_hook_token, &hook_api_key) { - child_env.insert( - crate::api::dependencies::HOOK_BROKER_URL_ENV.to_owned(), - format!("http://{address}{DEPENDENCY_HOOK_PATH}"), - ); + if let Some(broker) = &state.hook_broker { + if broker.dependency_check { + child_env.insert( + crate::api::dependencies::HOOK_BROKER_URL_ENV.to_owned(), + format!("http://{address}{DEPENDENCY_HOOK_PATH}"), + ); + } + if broker.secret_scan.is_some() { + child_env.insert( + SCAN_BROKER_URL_ENV.to_owned(), + format!("http://{address}{SECRET_SCAN_HOOK_PATH}"), + ); + } child_env.insert( crate::api::dependencies::HOOK_BROKER_TOKEN_ENV.to_owned(), - token.clone(), + broker.token.clone(), ); } } @@ -1826,6 +1882,14 @@ fn proxy_request( if request.uri().path() == DEPENDENCY_HOOK_PATH { return Ok(handle_dependency_hook(request, &state).await); } + if let Some(mode) = request + .uri() + .path() + .strip_prefix(SECRET_SCAN_HOOK_PATH) + .map(str::to_owned) + { + return Ok(handle_secret_scan_hook(request, &mode, &state, started).await); + } let request_id = new_local_request_id(); let host = request_host(&request, connect_authority.as_deref()); @@ -2350,29 +2414,18 @@ async fn handle_dependency_hook( request: Request, state: &ProxyState, ) -> Response { - let authorized = request.method() == Method::POST - && request.uri().query().is_none() - && state.dependency_hook_token.as_ref().is_some_and(|token| { - request - .headers() - .get("authorization") - .and_then(|value| value.to_str().ok()) - .is_some_and(|value| value == format!("Bearer {token}")) - }); - if !authorized { - return proxy_error_response( - StatusCode::FORBIDDEN, - "proxy.dependency_hook_not_allowed", - "Dependency hook route is not enabled for this run", - ); - } - let Some(client) = &state.dependency_hook_client else { + let Some(broker) = state + .hook_broker + .as_ref() + .filter(|broker| broker.dependency_check && broker.authorized(&request)) + else { return proxy_error_response( StatusCode::FORBIDDEN, "proxy.dependency_hook_not_allowed", "Dependency hook route is not enabled for this run", ); }; + let client = &broker.client; let body = match request.into_body().collect().await { Ok(body) => body.to_bytes(), Err(_) => { @@ -2421,6 +2474,95 @@ async fn handle_dependency_hook( builder.body(full_body(body)).unwrap() } +/// Runs the host's own `stashbase scan` for a sandboxed git hook. The hook +/// sends only the mode; what gets scanned, with which key and which config +/// options, is decided here. +async fn handle_secret_scan_hook( + request: Request, + mode: &str, + state: &ProxyState, + started: Instant, +) -> Response { + let request_id = new_local_request_id(); + let Some((broker, scan)) = state + .hook_broker + .as_ref() + .filter(|broker| broker.authorized(&request)) + .and_then(|broker| broker.secret_scan.as_ref().map(|scan| (broker, scan))) + else { + return proxy_error_response( + StatusCode::FORBIDDEN, + "proxy.secret_scan_not_allowed", + "Secret scan hook is not enabled for this run; add \"secret_scan\" to allow_hooks", + ); + }; + let mode = match mode { + "/staged" => "staged", + "/unpushed" => "unpushed", + _ => { + return proxy_error_response( + StatusCode::NOT_FOUND, + "proxy.secret_scan_unknown_mode", + "Unknown scan mode; expected staged or unpushed", + ) + } + }; + + let _guard = broker.scan_lock.lock().await; + let mut command = tokio::process::Command::new(&scan.exe); + command + .args(["scan", mode, "--json", "--silent"]) + .current_dir(&scan.workdir) + .env("STASHBASE_API_KEY", &broker.api_key) + .env(crate::models::scans::SCAN_RESTRICTED_ENV, "1") + // Keeps the scan's telemetry off, like everything else in the session. + .env("STASHBASE_SANDBOX", "1") + .env_remove(crate::api::dependencies::HOOK_MODE_ENV) + .env_remove(crate::api::dependencies::HOOK_BROKER_URL_ENV) + .env_remove(crate::api::dependencies::HOOK_BROKER_TOKEN_ENV) + .env_remove(SCAN_BROKER_URL_ENV) + .stdin(std::process::Stdio::null()) + .kill_on_drop(true); + + let response = match tokio::time::timeout(scan.timeout, command.output()).await { + Err(_) => proxy_error_response( + StatusCode::GATEWAY_TIMEOUT, + "proxy.secret_scan_timeout", + "Secret scan did not finish in time", + ), + Ok(Err(_)) => proxy_error_response( + StatusCode::BAD_GATEWAY, + "proxy.secret_scan_failed", + "Secret scan could not be started", + ), + Ok(Ok(output)) => { + let status = match output.status.code() { + Some(0) => StatusCode::OK, + Some(1) => StatusCode::UNPROCESSABLE_ENTITY, + _ => StatusCode::BAD_GATEWAY, + }; + let mut body = output.stdout; + body.extend_from_slice(&output.stderr); + body.truncate(SECRET_SCAN_BODY_LIMIT); + Response::builder() + .status(status) + .header("content-type", "text/plain; charset=utf-8") + .body(full_body(Bytes::from(body))) + .unwrap() + } + }; + state.record_audit_with_request( + &request_id, + "secret_scan_hook", + None, + Some(&Method::POST), + None, + Some(response.status()), + Some(started.elapsed()), + ); + response +} + /// Standard remote-proxy requests are built per request so a new connection /// always observes the latest rotated token. Existing response streams retain /// the client and session that opened them. @@ -5270,7 +5412,11 @@ mod tests { ProxyPolicy::permissive(), None, None, - Some("parent-api-key".to_owned()), + Some(HookBrokerConfig { + api_key: "parent-api-key".to_owned(), + dependency_check: true, + secret_scan: None, + }), ) .await .unwrap(); @@ -5300,6 +5446,231 @@ mod tests { disabled.stop().await; } + #[cfg(unix)] + fn scan_test_dir() -> PathBuf { + let dir = std::env::temp_dir().join(format!("stashbase-scan-hook-{}", Uuid::new_v4())); + std::fs::create_dir_all(&dir).unwrap(); + dir.canonicalize().unwrap() + } + + #[cfg(unix)] + fn fake_scan_exe(dir: &std::path::Path, exit_code: i32, sleep_secs: u32) -> PathBuf { + let path = dir.join("fake-stashbase"); + std::fs::write( + &path, + format!( + "#!/bin/sh\nsleep {sleep_secs}\n\ + echo \"args=$*\"\necho \"cwd=$(pwd -P)\"\n\ + echo \"key=$STASHBASE_API_KEY\"\necho \"restricted=$STASHBASE_SCAN_RESTRICTED\"\n\ + echo \"hook_token=${{STASHBASE_HOOK_BROKER_TOKEN:-unset}}\"\n\ + echo 'finding on stderr' >&2\nexit {exit_code}\n" + ), + ) + .unwrap(); + std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o755)).unwrap(); + path + } + + #[cfg(unix)] + async fn start_scan_proxy(exe: PathBuf, workdir: PathBuf, timeout: Duration) -> Proxy { + Proxy::start_with_hook( + HashMap::new(), + ProxyPolicy::permissive(), + None, + None, + Some(HookBrokerConfig { + api_key: "parent-api-key".to_owned(), + dependency_check: false, + secret_scan: Some(SecretScanConfig { + workdir, + exe, + timeout, + }), + }), + ) + .await + .unwrap() + } + + fn hook_token(proxy: &Proxy) -> String { + proxy.child_env()[crate::api::dependencies::HOOK_BROKER_TOKEN_ENV].clone() + } + + async fn post_to(url: String, token: Option<&str>) -> (u16, String) { + let client = reqwest::Client::builder().no_proxy().build().unwrap(); + let mut request = client.post(url); + if let Some(token) = token { + request = request.bearer_auth(token); + } + let response = request.send().await.unwrap(); + (response.status().as_u16(), response.text().await.unwrap()) + } + + #[cfg(unix)] + async fn post_scan(proxy: &Proxy, mode: &str, token: Option<&str>) -> (u16, String) { + let url = format!("{}/{mode}", proxy.child_env()[SCAN_BROKER_URL_ENV]); + post_to(url, token).await + } + + #[cfg(unix)] + #[tokio::test] + async fn secret_scan_hook_runs_host_scan_in_workdir_with_parent_key_restricted() { + let dir = scan_test_dir(); + let proxy = start_scan_proxy( + fake_scan_exe(&dir, 0, 0), + dir.clone(), + Duration::from_secs(10), + ) + .await; + + let (status, body) = post_scan(&proxy, "staged", Some(&hook_token(&proxy))).await; + + assert_eq!(status, 200, "{body}"); + assert!(body.contains("args=scan staged --json --silent"), "{body}"); + assert!(body.contains(&format!("cwd={}", dir.display())), "{body}"); + assert!(body.contains("key=parent-api-key"), "{body}"); + assert!(body.contains("restricted=1"), "{body}"); + assert!(body.contains("hook_token=unset"), "{body}"); + assert!(body.contains("finding on stderr"), "{body}"); + assert!(!proxy.child_env().contains_key("STASHBASE_API_KEY")); + assert!(!proxy + .child_env() + .contains_key(crate::api::dependencies::HOOK_BROKER_URL_ENV)); + proxy.stop().await; + let _ = std::fs::remove_dir_all(dir); + } + + #[cfg(unix)] + #[tokio::test] + async fn secret_scan_hook_maps_findings_to_422_and_supports_unpushed() { + let dir = scan_test_dir(); + let proxy = start_scan_proxy( + fake_scan_exe(&dir, 1, 0), + dir.clone(), + Duration::from_secs(10), + ) + .await; + + let (status, body) = post_scan(&proxy, "unpushed", Some(&hook_token(&proxy))).await; + + assert_eq!(status, 422, "{body}"); + assert!(body.contains("args=scan unpushed --json --silent"), "{body}"); + proxy.stop().await; + let _ = std::fs::remove_dir_all(dir); + } + + #[cfg(unix)] + #[tokio::test] + async fn secret_scan_hook_rejects_bad_token_and_unknown_mode() { + let dir = scan_test_dir(); + let proxy = start_scan_proxy( + fake_scan_exe(&dir, 0, 0), + dir.clone(), + Duration::from_secs(10), + ) + .await; + let token = hook_token(&proxy); + + assert_eq!(post_scan(&proxy, "staged", None).await.0, 403); + assert_eq!(post_scan(&proxy, "staged", Some("wrong")).await.0, 403); + assert_eq!(post_scan(&proxy, "changes", Some(&token)).await.0, 404); + proxy.stop().await; + let _ = std::fs::remove_dir_all(dir); + } + + #[tokio::test] + async fn secret_scan_route_is_not_served_when_only_dependency_check_is_allowed() { + let proxy = Proxy::start_with_hook( + HashMap::new(), + ProxyPolicy::permissive(), + None, + None, + Some(HookBrokerConfig { + api_key: "parent-api-key".to_owned(), + dependency_check: true, + secret_scan: None, + }), + ) + .await + .unwrap(); + assert!(!proxy.child_env().contains_key(SCAN_BROKER_URL_ENV)); + let url = proxy.child_env()[crate::api::dependencies::HOOK_BROKER_URL_ENV].replace( + DEPENDENCY_HOOK_PATH, + &format!("{SECRET_SCAN_HOOK_PATH}/staged"), + ); + + let (status, _) = post_to(url, Some(&hook_token(&proxy))).await; + + assert_eq!(status, 403); + proxy.stop().await; + } + + #[cfg(unix)] + #[tokio::test] + async fn dependency_route_is_not_served_when_only_secret_scan_is_allowed() { + let dir = scan_test_dir(); + let proxy = start_scan_proxy( + fake_scan_exe(&dir, 0, 0), + dir.clone(), + Duration::from_secs(10), + ) + .await; + let url = proxy.child_env()[SCAN_BROKER_URL_ENV] + .replace(SECRET_SCAN_HOOK_PATH, DEPENDENCY_HOOK_PATH); + + let (status, _) = post_to(url, Some(&hook_token(&proxy))).await; + + assert_eq!(status, 403); + proxy.stop().await; + let _ = std::fs::remove_dir_all(dir); + } + + #[cfg(unix)] + #[tokio::test] + async fn secret_scan_hook_times_out_with_504() { + let dir = scan_test_dir(); + let proxy = start_scan_proxy( + fake_scan_exe(&dir, 0, 5), + dir.clone(), + Duration::from_millis(300), + ) + .await; + + let (status, body) = post_scan(&proxy, "staged", Some(&hook_token(&proxy))).await; + + assert_eq!(status, 504, "{body}"); + assert!(body.contains("proxy.secret_scan_timeout"), "{body}"); + proxy.stop().await; + let _ = std::fs::remove_dir_all(dir); + } + + #[cfg(unix)] + #[tokio::test] + async fn secret_scan_hooks_are_serialized() { + let dir = scan_test_dir(); + let proxy = start_scan_proxy( + fake_scan_exe(&dir, 0, 1), + dir.clone(), + Duration::from_secs(10), + ) + .await; + let token = hook_token(&proxy); + + let started = std::time::Instant::now(); + let (a, b) = tokio::join!( + post_scan(&proxy, "staged", Some(&token)), + post_scan(&proxy, "staged", Some(&token)), + ); + + assert_eq!((a.0, b.0), (200, 200)); + assert!( + started.elapsed() >= Duration::from_secs(2), + "scans ran concurrently" + ); + proxy.stop().await; + let _ = std::fs::remove_dir_all(dir); + } + #[tokio::test] async fn child_environment_uses_the_binding_name_for_its_default_placeholder() { let proxy = Proxy::start( @@ -5500,8 +5871,7 @@ mod tests { connections: Arc::new(ActiveConnections::default()), remote: None, mcp_inspection_token: "inspection-token".to_owned(), - dependency_hook_token: None, - dependency_hook_client: None, + hook_broker: None, revocation_path: Arc::new(RwLock::new(None)), }; From 4e8d0170d8a44ac68dbcf413f0266d74363c9143 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 07:10:32 +0000 Subject: [PATCH 03/22] fix(docker): rewrite the scan broker URL for the container --- src/handlers/run/docker_sandbox.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/handlers/run/docker_sandbox.rs b/src/handlers/run/docker_sandbox.rs index 99b59161..107007e6 100644 --- a/src/handlers/run/docker_sandbox.rs +++ b/src/handlers/run/docker_sandbox.rs @@ -512,6 +512,7 @@ const PROXY_URL_ENV_KEYS: &[&str] = &[ "http_proxy", "https_proxy", crate::api::dependencies::HOOK_BROKER_URL_ENV, + crate::handlers::run::proxy::SCAN_BROKER_URL_ENV, ]; /// Rewrites the proxy's child-process env vars so the container reaches the @@ -1694,6 +1695,23 @@ mod tests { assert_eq!(rewritten["STASHBASE_GH_TOKEN"], "127.0.0.1"); } + #[test] + fn rewrite_proxy_urls_rewrites_the_scan_broker_url() { + let mut env_vars = std::collections::HashMap::new(); + env_vars.insert( + crate::handlers::run::proxy::SCAN_BROKER_URL_ENV.to_owned(), + "http://127.0.0.1:9999/__stashbase/scan".to_owned(), + ); + + let rewritten = + rewrite_proxy_urls_for_container(&env_vars, "127.0.0.1", "host.docker.internal"); + + assert_eq!( + rewritten[crate::handlers::run::proxy::SCAN_BROKER_URL_ENV], + "http://host.docker.internal:9999/__stashbase/scan" + ); + } + #[test] fn rewrite_proxy_urls_is_a_no_op_when_hosts_match() { let mut env_vars = std::collections::HashMap::new(); From 2f4f0ba429477b9905a015bb347e75c08b337eff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 07:11:53 +0000 Subject: [PATCH 04/22] feat(agent): enable the secret scan hook with allow_hooks = ["secret_scan"] --- src/handlers/agent/init.rs | 6 +- src/handlers/agent/validate.rs | 24 +++++++- src/handlers/entry/root.rs | 72 +++++++++++++--------- src/handlers/run/entry.rs | 105 +++++++++++++++++++++++++-------- src/models/agent.rs | 39 ++++++++++++ 5 files changed, 192 insertions(+), 54 deletions(-) diff --git a/src/handlers/agent/init.rs b/src/handlers/agent/init.rs index b9dc027a..7a25da3d 100644 --- a/src/handlers/agent/init.rs +++ b/src/handlers/agent/init.rs @@ -14,8 +14,8 @@ const PROFILE_TEMPLATE: &str = r#"# Stashbase Agent Proxy profile. # Add only destinations the agent genuinely needs to contact. egress_hosts = [] -# Enable installed dependency hooks for this profile. -# allow_hooks = ["dependency_check"] +# Enable installed dependency and secret-scan hooks for this profile. +# allow_hooks = ["dependency_check", "secret_scan"] # Optional: allow local test servers and Unix-socket IPC (macOS only). # Linux keeps loopback available for the embedded proxy; this setting has no effect there. @@ -126,7 +126,7 @@ mod tests { fn template_starts_closed_and_includes_a_generic_rule() { assert!(PROFILE_TEMPLATE.contains("egress_hosts = []")); assert!(PROFILE_TEMPLATE.contains("allow_network_listeners = true")); - assert!(PROFILE_TEMPLATE.contains("# allow_hooks = [\"dependency_check\"]")); + assert!(PROFILE_TEMPLATE.contains("# allow_hooks = [\"dependency_check\", \"secret_scan\"]")); assert!(PROFILE_TEMPLATE.contains("[secrets.SECRET_NAME]")); assert!(PROFILE_TEMPLATE.contains("[[secrets.SECRET_NAME.rules]]")); } diff --git a/src/handlers/agent/validate.rs b/src/handlers/agent/validate.rs index 44f6e6de..45e5af23 100644 --- a/src/handlers/agent/validate.rs +++ b/src/handlers/agent/validate.rs @@ -696,7 +696,7 @@ fn validate_hook_capabilities(profile: &AgentProfile) -> Vec { let unsupported = profile .allow_hooks .iter() - .filter(|hook| hook.as_str() != "dependency_check") + .filter(|hook| !matches!(hook.as_str(), "dependency_check" | "secret_scan")) .collect::>(); if !unsupported.is_empty() { return unsupported @@ -1132,6 +1132,28 @@ mod tests { .contains("Unsupported hook capability 'anything_else'")); } + #[test] + fn secret_scan_is_a_known_hook() { + let profile = AgentProfile { + file: None, + egress_hosts: None, + allow_network_listeners: false, + deny_hosts: None, + filesystem: Default::default(), + sandbox: Default::default(), + workspace: Default::default(), + mcp_servers: HashMap::new(), + secrets: HashMap::new().into(), + personal_credentials: HashMap::new(), + policy_tests: Vec::new(), + allow_hooks: vec!["dependency_check".to_owned(), "secret_scan".to_owned()], + }; + + let checks = validate_hook_capabilities(&profile); + + assert!(checks.iter().all(|check| check.status != Status::Fail)); + } + #[test] fn docker_backend_profile_gets_a_docker_runtime_check_not_native_ones() { // A Docker-backend profile never touches Seatbelt/systemd-run/ diff --git a/src/handlers/entry/root.rs b/src/handlers/entry/root.rs index ad779ca8..fcd8512f 100644 --- a/src/handlers/entry/root.rs +++ b/src/handlers/entry/root.rs @@ -878,11 +878,8 @@ pub async fn handle_cli(args: Cli) -> Exit { } } - let dependency_hooks_requested = profile - .allow_hooks - .iter() - .any(|hook| hook == "dependency_check"); - let dependency_hooks = dependency_hooks_enabled(&profile, &api_key); + let requested_hooks = profile.requested_hooks(); + let hooks = enabled_hooks(&profile, &api_key); if !silent { eprintln!("Network sandbox: enabled"); if profile.sandbox.backend @@ -891,17 +888,10 @@ pub async fn handle_cli(args: Cli) -> Exit { eprintln!("Sandbox backend: Docker (container-isolated)"); } print_agent_egress_warnings(&profile); - eprintln!( - "API hook broker: {}", - if dependency_hooks { - "enabled (dependency_check)" - } else { - "disabled" - } - ); - if dependency_hooks_requested && !dependency_hooks { + eprintln!("API hook broker: {}", hooks.label()); + if requested_hooks.any() && !hooks.any() { eprintln!( - "Warning: dependency_check is configured but no API key is available; dependency checks are disabled." + "Warning: allow_hooks is configured but no API key is available; API hooks are disabled." ); } } @@ -1314,7 +1304,7 @@ pub async fn handle_cli(args: Cli) -> Exit { ); let result = handle_remote_agent_run( api_key.clone(), - dependency_hooks, + hooks, command, policy, crate::handlers::run::proxy::RemoteProxyConfig { proxy_url, session: remote_session.clone(), placeholders, child_env, protocol, ca_file: remote_ca_file, routing }, @@ -1362,7 +1352,7 @@ pub async fn handle_cli(args: Cli) -> Exit { set_comments: Vec::new(), print_secrets: None, no_print_secrets: true, - dependency_hooks, + hooks, local_session, config_file: None, file: profile.file, @@ -1422,7 +1412,7 @@ pub async fn handle_cli(args: Cli) -> Exit { json_format: raw_output, silent, scope: run_cmd.scope, - dependency_hooks: false, + hooks: crate::models::agent::EnabledHooks::default(), local_session: None, }; @@ -1590,12 +1580,15 @@ fn uses_local_dependency_hook_broker(entity_type: &EntityType) -> bool { ) } -fn dependency_hooks_enabled(profile: &crate::models::agent::AgentProfile, api_key: &str) -> bool { - !api_key.is_empty() - && profile - .allow_hooks - .iter() - .any(|hook| hook == "dependency_check") +fn enabled_hooks( + profile: &crate::models::agent::AgentProfile, + api_key: &str, +) -> crate::models::agent::EnabledHooks { + if api_key.is_empty() { + crate::models::agent::EnabledHooks::default() + } else { + profile.requested_hooks() + } } fn uses_local_dependency_hook_broker_mode(entity_type: &EntityType, mode: Option<&str>) -> bool { @@ -2379,7 +2372,7 @@ fn spawn_remote_session_rotation( mod tests { use super::{ audit_binding_sources, codex_mcp_binding_header_overrides, configured_host_matches, - dependency_hooks_enabled, directory_profile_git_warning, + directory_profile_git_warning, enabled_hooks, ensure_replacement_session_is_compatible, exit_for, infer_remote_agent_type, is_bare_agent_hook, remote_bindings, remote_codex_command_with_mcp_binding_headers, remote_session_rotation_delay_for, remote_session_started_message, @@ -2488,8 +2481,33 @@ mod tests { "allow_hooks": ["dependency_check"] })) .unwrap(); - assert!(!dependency_hooks_enabled(&profile, "")); - assert!(dependency_hooks_enabled(&profile, "key")); + assert!(!enabled_hooks(&profile, "").dependency_check); + assert!(enabled_hooks(&profile, "key").dependency_check); + } + + #[test] + fn enabled_hooks_reads_both_hook_kinds_and_needs_an_api_key() { + let profile_with = |hooks: &[&str]| -> AgentProfile { + serde_json::from_value(serde_json::json!({ + "file": null, + "egress_hosts": null, + "deny_hosts": null, + "allow_hooks": hooks + })) + .unwrap() + }; + + let hooks = enabled_hooks(&profile_with(&["secret_scan"]), "key"); + assert!(hooks.secret_scan && !hooks.dependency_check); + assert_eq!(hooks.label(), "enabled (secret_scan)"); + assert_eq!( + enabled_hooks(&profile_with(&["dependency_check", "secret_scan"]), "key").label(), + "enabled (dependency_check, secret_scan)" + ); + assert_eq!( + enabled_hooks(&profile_with(&["secret_scan"]), "").label(), + "disabled" + ); } #[test] diff --git a/src/handlers/run/entry.rs b/src/handlers/run/entry.rs index fc2c580c..bf296342 100644 --- a/src/handlers/run/entry.rs +++ b/src/handlers/run/entry.rs @@ -380,11 +380,37 @@ fn finish_run_worktree(worktree: &super::worktree::RunWorktree, docker: bool, si } } +/// What the proxy's hook broker serves for this run, or `None` when no hook +/// is enabled or there is no API key to call Stashbase with. +fn hook_broker_config( + hooks: crate::models::agent::EnabledHooks, + api_key: Option, + workdir: &Path, +) -> Option { + if !hooks.any() { + return None; + } + Some(super::proxy::HookBrokerConfig { + api_key: api_key.filter(|key| !key.is_empty())?, + dependency_check: hooks.dependency_check, + // The scan runs the host's own CLI in the directory the agent works in. + secret_scan: hooks + .secret_scan + .then(|| std::env::current_exe().ok()) + .flatten() + .map(|exe| super::proxy::SecretScanConfig { + workdir: workdir.to_owned(), + exe, + timeout: std::time::Duration::from_secs(120), + }), + }) +} + /// Runs an agent through the localhost relay while credentials stay in the /// control-plane's short-lived remote agent-proxy session. pub async fn handle_remote_agent_run( api_key: String, - hooks_enabled: bool, + hooks: crate::models::agent::EnabledHooks, command: Vec, policy: super::proxy::ProxyPolicy, remote: super::proxy::RemoteProxyConfig, @@ -442,11 +468,11 @@ pub async fn handle_remote_agent_run( )?; (None, String::new()) }; - let remote_hooks = hooks_enabled.then(|| super::proxy::HookBrokerConfig { - api_key, - dependency_check: true, - secret_scan: None, - }); + let scan_workdir = match run_worktree.workdir() { + Some(workdir) => workdir.to_owned(), + None => std::env::current_dir()?, + }; + let remote_hooks = hook_broker_config(hooks, Some(api_key), &scan_workdir); let proxy_start_result = if let Some(network) = &docker_network { super::proxy::Proxy::start_remote_with_hook_and_bind_host( remote, @@ -625,7 +651,7 @@ pub struct HandleRunArgs { pub json_format: bool, pub silent: bool, pub scope: Option, - pub dependency_hooks: bool, + pub hooks: crate::models::agent::EnabledHooks, pub local_session: Option, } @@ -660,7 +686,7 @@ pub async fn handle_load_env_run(args: HandleRunArgs) -> anyhow::Result<()> { json_format, silent, scope, - dependency_hooks, + hooks, local_session, } = args; @@ -1171,7 +1197,7 @@ pub async fn handle_load_env_run(args: HandleRunArgs) -> anyhow::Result<()> { is_from_file, silent, json_format, - dependency_hooks, + hooks, Some(api_key.clone()), local_session, ) @@ -1214,7 +1240,7 @@ pub async fn handle_load_env_run(args: HandleRunArgs) -> anyhow::Result<()> { false, silent, json_format, - dependency_hooks, + hooks, Some(api_key.clone()), local_session, ) @@ -1374,7 +1400,7 @@ pub async fn handle_load_env_run(args: HandleRunArgs) -> anyhow::Result<()> { is_from_file, silent, json_format, - dependency_hooks, + hooks, Some(api_key.clone()), local_session, ) @@ -1409,7 +1435,7 @@ pub async fn handle_load_env_run(args: HandleRunArgs) -> anyhow::Result<()> { is_from_file, silent, json_format, - dependency_hooks, + hooks, Some(api_key.clone()), local_session, ) @@ -1522,7 +1548,7 @@ async fn handle_run( is_from_file: bool, silent: bool, json_format: bool, - dependency_hooks: bool, + hooks: crate::models::agent::EnabledHooks, hook_api_key: Option, local_session: Option, ) -> anyhow::Result<()> { @@ -1706,14 +1732,11 @@ async fn handle_run( )?; (None, String::new()) }; - let local_hooks = dependency_hooks - .then_some(hook_api_key) - .flatten() - .map(|api_key| super::proxy::HookBrokerConfig { - api_key, - dependency_check: true, - secret_scan: None, - }); + let scan_workdir = match run_worktree.workdir() { + Some(workdir) => workdir.to_owned(), + None => std::env::current_dir()?, + }; + let local_hooks = hook_broker_config(hooks, hook_api_key, &scan_workdir); let proxy_start_result = if let Some(network) = &docker_network { super::proxy::Proxy::start_with_hook_and_bind_host( secrets_hash_map, @@ -1994,11 +2017,11 @@ fn missing_secret_labels( mod tests { use super::{ apply_secret_bindings, can_prompt_for_image_build, declined_image_build_error, - load_run_secrets_from_file, loaded_message, loading_message, + hook_broker_config, load_run_secrets_from_file, loaded_message, loading_message, merge_remote_and_local_secrets, missing_image_build_command, missing_image_error, missing_secret_labels, needs_remote_fetch, prepare_local_run_secrets, shell_quote, }; - use crate::models::secrets::SecretWithoutComment; + use crate::models::{agent::EnabledHooks, secrets::SecretWithoutComment}; use std::{ collections::HashMap, fs, @@ -2006,6 +2029,42 @@ mod tests { time::{SystemTime, UNIX_EPOCH}, }; + #[test] + fn hook_broker_config_scans_the_run_workdir() { + let workdir = std::path::Path::new("/repo/.stashbase/worktrees/feat"); + let scan_only = EnabledHooks { + dependency_check: false, + secret_scan: true, + }; + + let config = hook_broker_config(scan_only, Some("k".to_owned()), workdir).unwrap(); + + assert!(!config.dependency_check); + let scan = config.secret_scan.unwrap(); + assert_eq!(scan.workdir, workdir); + assert_eq!(scan.exe, std::env::current_exe().unwrap()); + assert_eq!(scan.timeout, std::time::Duration::from_secs(120)); + } + + #[test] + fn hook_broker_config_needs_a_hook_and_an_api_key() { + let workdir = std::path::Path::new("/repo"); + let both = EnabledHooks { + dependency_check: true, + secret_scan: true, + }; + + assert!(hook_broker_config(EnabledHooks::default(), Some("k".to_owned()), workdir).is_none()); + assert!(hook_broker_config(both, None, workdir).is_none()); + assert!(hook_broker_config(both, Some(String::new()), workdir).is_none()); + let deps_only = EnabledHooks { + dependency_check: true, + secret_scan: false, + }; + let config = hook_broker_config(deps_only, Some("k".to_owned()), workdir).unwrap(); + assert!(config.dependency_check && config.secret_scan.is_none()); + } + #[test] fn missing_default_image_points_to_agent_docker_build() { let source = crate::handlers::run::docker_sandbox::AgentImageSource::Default; diff --git a/src/models/agent.rs b/src/models/agent.rs index f3d5b834..290a74be 100644 --- a/src/models/agent.rs +++ b/src/models/agent.rs @@ -42,6 +42,45 @@ pub struct AgentProfile { pub allow_hooks: Vec, } +/// Authenticated hooks the run broker serves on the agent's behalf. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct EnabledHooks { + pub dependency_check: bool, + pub secret_scan: bool, +} + +impl EnabledHooks { + pub fn any(self) -> bool { + self.dependency_check || self.secret_scan + } + + /// The startup line's description, e.g. `enabled (dependency_check, secret_scan)`. + pub fn label(self) -> String { + let names = [ + (self.dependency_check, "dependency_check"), + (self.secret_scan, "secret_scan"), + ] + .into_iter() + .filter_map(|(enabled, name)| enabled.then_some(name)) + .collect::>(); + if names.is_empty() { + "disabled".to_owned() + } else { + format!("enabled ({})", names.join(", ")) + } + } +} + +impl AgentProfile { + /// The hooks `allow_hooks` asks for, before checking an API key exists. + pub fn requested_hooks(&self) -> EnabledHooks { + EnabledHooks { + dependency_check: self.allow_hooks.iter().any(|hook| hook == "dependency_check"), + secret_scan: self.allow_hooks.iter().any(|hook| hook == "secret_scan"), + } + } +} + /// An MCP endpoint rule. `hosts` and `paths` identify the endpoint, while /// `tools` limits the tools exposed through it. #[derive(Debug, Clone, Serialize, Deserialize)] From 8c5247cc0c64943b8bc58e009d08e13335079f63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 07:13:00 +0000 Subject: [PATCH 05/22] feat(scan): route sandboxed git hooks through the agent proxy broker --- docs/agent-profiles.md | 10 +-- docs/sandboxing.md | 4 ++ src/handlers/scans/install.rs | 130 ++++++++++++++++++++++++++++++++-- 3 files changed, 133 insertions(+), 11 deletions(-) diff --git a/docs/agent-profiles.md b/docs/agent-profiles.md index 29a7fd06..7fc9f161 100644 --- a/docs/agent-profiles.md +++ b/docs/agent-profiles.md @@ -165,15 +165,17 @@ allow_tools = ["search_issues", "get_issue"] The HTTP rule controls credential injection at the endpoint. The MCP rule controls which tools the agent may call. Omit `allow_tools` or leave it empty to deny all tools by default. Use `allow_tools = ["*"]` to allow all tools (a matching `deny_tools` still takes precedence). -## Dependency Hooks +## API Hooks -If your profile enables authenticated hooks (e.g., dependency checking), grant capability explicitly: +Hooks that call the Stashbase API (dependency checking, secret scanning) must be granted explicitly: ```toml -allow_hooks = ["dependency_check"] +allow_hooks = ["dependency_check", "secret_scan"] ``` -The child receives only a scoped local broker token. Hooks are disabled by default. +The child receives only a scoped local broker token, never your API key. Hooks are disabled by default. + +`secret_scan` lets the git hooks from `stashbase scan install` run inside the sandbox. The hook sends only a token to the Agent Proxy, which runs `stashbase scan staged` (or `unpushed`) on the host, in the run's working directory, with your key. Because the agent can edit the repo's scan config, these scans ignore its `match` and `output-dir` settings; `excluded-files` and `ignored-secrets` still apply. The hook needs `curl` in the sandbox (the default Docker image has it). Hooks installed before this existed need `stashbase scan install` once more to pick up the sandbox-aware block. Without `secret_scan`, a scan hook inside the sandbox fails with a message pointing here. ## Audit Logs and Session Revocation diff --git a/docs/sandboxing.md b/docs/sandboxing.md index fc399807..9c5bf6db 100644 --- a/docs/sandboxing.md +++ b/docs/sandboxing.md @@ -111,6 +111,10 @@ stashbase agent run --profile coding --docker-image node:22-alpine -- claude Your global `git config user.name` and `user.email` (if configured on the host) are forwarded into the container as `GIT_AUTHOR_NAME`, `GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_NAME`, and `GIT_COMMITTER_EMAIL`. This is the one piece of host configuration deliberately forwarded despite the filesystem allow-list, since it's authorship metadata, not a credential — without it, `git commit` inside the sandbox fails with no identity configured. It does not grant push access: `git push` (or any other authenticated git operation) still needs a real credential, wired through `[secrets]` like `GITHUB_TOKEN`, or run from outside the sandbox. Raw SSH keys are never forwarded. A profile that explicitly sets one of these four env vars itself takes precedence over the forwarded host value. +### Secret scan hooks + +Git hooks installed with `stashbase scan install` work in the sandbox when the profile sets `allow_hooks = ["secret_scan"]`. The sandbox has neither the Stashbase CLI nor your API key, so the hook asks the Agent Proxy to run the scan on the host against the same working directory, with `curl` and a per-run token. Findings come back to the agent, and the commit or push is blocked, exactly as outside the sandbox. Like any git hook, it is skipped by `git commit --no-verify` — a safety net, not an enforcement boundary. See [Agent Profiles](agent-profiles.md#api-hooks). + ### Notifications, herdr and cmux Agent notifications work in the sandbox with no setup. Your terminal's identity (`TERM`, `COLORTERM`, `TERM_PROGRAM`, `TERM_PROGRAM_VERSION`, `LC_TERMINAL`) is forwarded into the container, so Claude Code and Codex send the same notifications they would outside it (OSC 9/777/99 or the bell) when a turn finishes or they need input. Ghostty, iTerm2, kitty, [cmux](https://cmux.com), [herdr](https://herdr.dev), tmux and other terminals and multiplexers pick them up as usual. Only the terminal's name and version are forwarded; nothing else about your terminal or session reaches the container. diff --git a/src/handlers/scans/install.rs b/src/handlers/scans/install.rs index d52acaaa..65fc2abe 100644 --- a/src/handlers/scans/install.rs +++ b/src/handlers/scans/install.rs @@ -39,6 +39,38 @@ impl HookType { } } +/// Inside an agent sandbox the hook asks the Agent Proxy to run the scan on +/// the host, since the sandbox has neither the CLI nor an API key. +fn hook_block(hook_type: HookType) -> String { + format!( + r#"{start} +if [ -n "$STASHBASE_SCAN_BROKER_URL" ]; then + command -v curl >/dev/null 2>&1 || {{ + echo "curl not found in the agent sandbox. Cannot run the Stashbase scan." + exit 1 + }} + curl -sS --fail-with-body --noproxy '*' -X POST \ + -H "Authorization: Bearer $STASHBASE_HOOK_BROKER_TOKEN" \ + "$STASHBASE_SCAN_BROKER_URL/{mode}" || exit 1 +elif [ "$STASHBASE_SANDBOX" = "1" ]; then + echo "Stashbase scan hook is not enabled for this agent run. Add allow_hooks = [\"secret_scan\"] to the agent profile." + exit 1 +else + command -v stashbase >/dev/null 2>&1 || {{ + echo "stashbase CLI not found. Skipping scan." + exit 1 + }} + + stashbase scan {mode} --silent --json || exit 1 +fi +{end} +"#, + start = STASHBASE_SCAN_START_MARKER, + mode = hook_type.scan_mode(), + end = STASHBASE_SCAN_END_MARKER, + ) +} + pub fn install_scan_hook( hook_type: HookType, file_path: Option<&str>, @@ -61,12 +93,7 @@ pub fn install_scan_hook( ) })?; - let hook_block = format!( - "{start}\ncommand -v stashbase >/dev/null 2>&1 || {{\n echo \"stashbase CLI not found. Skipping scan.\"\n exit 1\n}}\n\nstashbase scan {mode} --silent --json || exit 1\n{end}\n", - start = STASHBASE_SCAN_START_MARKER, - mode = hook_type.scan_mode(), - end = STASHBASE_SCAN_END_MARKER, - ); + let hook_block = hook_block(hook_type); let mut was_already_installed = false; @@ -289,7 +316,7 @@ fn normalize_after_uninstall(content: String) -> String { #[cfg(test)] mod tests { - use super::{install_scan_hook, uninstall_scan_hook, HookType}; + use super::{hook_block, install_scan_hook, uninstall_scan_hook, HookType}; use once_cell::sync::Lazy; use std::{ env, fs, @@ -341,6 +368,95 @@ mod tests { assert!(status.success(), "git init failed"); } + #[cfg(unix)] + fn run_block(hook_type: HookType, env: &[(&str, &str)], with_curl: bool) -> (i32, String) { + use std::os::unix::fs::PermissionsExt; + + let bin = temp_dir().join("bin"); + fs::create_dir_all(&bin).expect("failed to create bin dir"); + let stub = |name: &str, body: &str| { + let path = bin.join(name); + fs::write(&path, format!("#!/bin/sh\n{body}\n")).expect("stub write failed"); + fs::set_permissions(&path, fs::Permissions::from_mode(0o755)).expect("chmod failed"); + }; + if with_curl { + stub("curl", "echo \"curl $*\"; exit ${FAKE_CURL_EXIT:-0}"); + } + stub("stashbase", "echo \"cli $*\""); + + // PATH holds only the stubs, so a real curl can never stand in for a + // missing one; everything else the block uses is a shell builtin. + let output = Command::new("/bin/sh") + .arg("-c") + .arg(hook_block(hook_type)) + .env_clear() + .env("PATH", &bin) + .envs(env.iter().copied()) + .output() + .expect("failed to run hook block"); + let text = String::from_utf8_lossy(&output.stdout).into_owned() + + &String::from_utf8_lossy(&output.stderr); + (output.status.code().expect("hook was killed"), text) + } + + #[cfg(unix)] + const BROKER_ENV: [(&str, &str); 3] = [ + ("STASHBASE_SANDBOX", "1"), + ("STASHBASE_SCAN_BROKER_URL", "http://h:1/__stashbase/scan"), + ("STASHBASE_HOOK_BROKER_TOKEN", "t"), + ]; + + #[cfg(unix)] + #[test] + fn hook_uses_the_broker_inside_the_sandbox() { + let (code, out) = run_block(HookType::PrePush, &BROKER_ENV, true); + + assert_eq!(code, 0, "{out}"); + assert!(out.contains("http://h:1/__stashbase/scan/unpushed"), "{out}"); + assert!(out.contains("Authorization: Bearer t"), "{out}"); + assert!(!out.contains("cli "), "{out}"); + } + + #[cfg(unix)] + #[test] + fn hook_fails_when_the_broker_reports_findings() { + let mut env = BROKER_ENV.to_vec(); + env.push(("FAKE_CURL_EXIT", "22")); + + let (code, out) = run_block(HookType::PreCommit, &env, true); + + assert_eq!(code, 1, "{out}"); + assert!(out.contains("http://h:1/__stashbase/scan/staged"), "{out}"); + } + + #[cfg(unix)] + #[test] + fn hook_in_sandbox_without_broker_names_the_profile_fix() { + let (code, out) = run_block(HookType::PreCommit, &[("STASHBASE_SANDBOX", "1")], true); + + assert_eq!(code, 1, "{out}"); + assert!(out.contains("allow_hooks = [\"secret_scan\"]"), "{out}"); + assert!(!out.contains("stashbase CLI not found"), "{out}"); + } + + #[cfg(unix)] + #[test] + fn hook_in_sandbox_without_curl_says_so() { + let (code, out) = run_block(HookType::PreCommit, &BROKER_ENV, false); + + assert_eq!(code, 1, "{out}"); + assert!(out.contains("curl not found"), "{out}"); + } + + #[cfg(unix)] + #[test] + fn hook_outside_sandbox_runs_the_cli_as_before() { + let (code, out) = run_block(HookType::PreCommit, &[], true); + + assert_eq!(code, 0, "{out}"); + assert!(out.contains("cli scan staged --silent --json"), "{out}"); + } + #[test] fn creates_new_pre_commit_hook() { let _lock = test_lock(); From a22f58561e1b8dccfa55e677e8c74c9f4b0f5e73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 07:19:15 +0000 Subject: [PATCH 06/22] fix(run): pass EnabledHooks on the egress-only run path --- src/handlers/run/entry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/handlers/run/entry.rs b/src/handlers/run/entry.rs index bf296342..d634d041 100644 --- a/src/handlers/run/entry.rs +++ b/src/handlers/run/entry.rs @@ -746,7 +746,7 @@ pub async fn handle_load_env_run(args: HandleRunArgs) -> anyhow::Result<()> { false, silent, json_format, - false, + crate::models::agent::EnabledHooks::default(), None, local_session, ) From 972f3c0afc3565781b1a00c1ec39e40b3c7044f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 07:25:07 +0000 Subject: [PATCH 07/22] fix(run): serve API hooks for egress-only agent profiles --- src/handlers/run/entry.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/handlers/run/entry.rs b/src/handlers/run/entry.rs index d634d041..052295e1 100644 --- a/src/handlers/run/entry.rs +++ b/src/handlers/run/entry.rs @@ -746,8 +746,10 @@ pub async fn handle_load_env_run(args: HandleRunArgs) -> anyhow::Result<()> { false, silent, json_format, - crate::models::agent::EnabledHooks::default(), - None, + // No secrets to fetch, but the hook broker still calls Stashbase + // on the host with the parent's key. + hooks, + Some(api_key.clone()), local_session, ) .await; From d66fa0392d072f48954b089efe671e28dd7b204b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 07:43:23 +0000 Subject: [PATCH 08/22] fix(scan): cap brokered scan output while reading and keep the hook set -u safe --- src/handlers/run/proxy.rs | 59 ++++++++++++++++++++++++++++++++--- src/handlers/scans/install.rs | 28 ++++++++++++++--- 2 files changed, 78 insertions(+), 9 deletions(-) diff --git a/src/handlers/run/proxy.rs b/src/handlers/run/proxy.rs index 82a9d18f..05c00bd1 100644 --- a/src/handlers/run/proxy.rs +++ b/src/handlers/run/proxy.rs @@ -2474,6 +2474,18 @@ async fn handle_dependency_hook( builder.body(full_body(body)).unwrap() } +/// Keeps at most `limit` bytes of `reader`, discarding the rest as it +/// arrives. The scan's output size is agent-controlled, and draining (rather +/// than stopping) keeps the child from blocking on a full pipe. +async fn read_capped(mut reader: impl tokio::io::AsyncRead + Unpin, limit: usize) -> Vec { + use tokio::io::AsyncReadExt; + + let mut kept = Vec::new(); + let _ = (&mut reader).take(limit as u64).read_to_end(&mut kept).await; + let _ = tokio::io::copy(&mut reader, &mut tokio::io::sink()).await; + kept +} + /// Runs the host's own `stashbase scan` for a sandboxed git hook. The hook /// sends only the mode; what gets scanned, with which key and which config /// options, is decided here. @@ -2522,9 +2534,23 @@ async fn handle_secret_scan_hook( .env_remove(crate::api::dependencies::HOOK_BROKER_TOKEN_ENV) .env_remove(SCAN_BROKER_URL_ENV) .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) .kill_on_drop(true); - let response = match tokio::time::timeout(scan.timeout, command.output()).await { + let run = async move { + let mut child = command.spawn()?; + let stdout = child.stdout.take().expect("stdout is piped"); + let stderr = child.stderr.take().expect("stderr is piped"); + let (stdout, stderr, status) = tokio::join!( + read_capped(stdout, SECRET_SCAN_BODY_LIMIT), + read_capped(stderr, SECRET_SCAN_BODY_LIMIT), + child.wait(), + ); + status.map(|status| (status, stdout, stderr)) + }; + // On timeout `run` is dropped with the child, which `kill_on_drop` ends. + let response = match tokio::time::timeout(scan.timeout, run).await { Err(_) => proxy_error_response( StatusCode::GATEWAY_TIMEOUT, "proxy.secret_scan_timeout", @@ -2535,14 +2561,14 @@ async fn handle_secret_scan_hook( "proxy.secret_scan_failed", "Secret scan could not be started", ), - Ok(Ok(output)) => { - let status = match output.status.code() { + Ok(Ok((exit, stdout, stderr))) => { + let status = match exit.code() { Some(0) => StatusCode::OK, Some(1) => StatusCode::UNPROCESSABLE_ENTITY, _ => StatusCode::BAD_GATEWAY, }; - let mut body = output.stdout; - body.extend_from_slice(&output.stderr); + let mut body = stdout; + body.extend_from_slice(&stderr); body.truncate(SECRET_SCAN_BODY_LIMIT); Response::builder() .status(status) @@ -5644,6 +5670,29 @@ mod tests { let _ = std::fs::remove_dir_all(dir); } + #[cfg(unix)] + #[tokio::test] + async fn secret_scan_hook_caps_huge_output_and_still_finishes() { + let dir = scan_test_dir(); + let exe = dir.join("loud-stashbase"); + std::fs::write( + &exe, + "#!/bin/sh\nhead -c 5242880 /dev/zero | tr '\\0' a\nhead -c 5242880 /dev/zero | tr '\\0' b >&2\nexit 1\n", + ) + .unwrap(); + std::fs::set_permissions(&exe, std::fs::Permissions::from_mode(0o755)).unwrap(); + let proxy = start_scan_proxy(exe, dir.clone(), Duration::from_secs(10)).await; + + let (status, body) = post_scan(&proxy, "staged", Some(&hook_token(&proxy))).await; + + // 422 rather than 504: the excess was drained, so the scan exited. + assert_eq!(status, 422); + assert!(body.len() <= SECRET_SCAN_BODY_LIMIT, "{}", body.len()); + assert!(body.starts_with('a')); + proxy.stop().await; + let _ = std::fs::remove_dir_all(dir); + } + #[cfg(unix)] #[tokio::test] async fn secret_scan_hooks_are_serialized() { diff --git a/src/handlers/scans/install.rs b/src/handlers/scans/install.rs index 65fc2abe..c77fce0e 100644 --- a/src/handlers/scans/install.rs +++ b/src/handlers/scans/install.rs @@ -44,15 +44,15 @@ impl HookType { fn hook_block(hook_type: HookType) -> String { format!( r#"{start} -if [ -n "$STASHBASE_SCAN_BROKER_URL" ]; then +if [ -n "${STASHBASE_SCAN_BROKER_URL:-}" ]; then command -v curl >/dev/null 2>&1 || {{ echo "curl not found in the agent sandbox. Cannot run the Stashbase scan." exit 1 }} curl -sS --fail-with-body --noproxy '*' -X POST \ - -H "Authorization: Bearer $STASHBASE_HOOK_BROKER_TOKEN" \ + -H "Authorization: Bearer ${STASHBASE_HOOK_BROKER_TOKEN:-}" \ "$STASHBASE_SCAN_BROKER_URL/{mode}" || exit 1 -elif [ "$STASHBASE_SANDBOX" = "1" ]; then +elif [ "${STASHBASE_SANDBOX:-}" = "1" ]; then echo "Stashbase scan hook is not enabled for this agent run. Add allow_hooks = [\"secret_scan\"] to the agent profile." exit 1 else @@ -370,6 +370,11 @@ mod tests { #[cfg(unix)] fn run_block(hook_type: HookType, env: &[(&str, &str)], with_curl: bool) -> (i32, String) { + run_script(hook_block(hook_type), env, with_curl) + } + + #[cfg(unix)] + fn run_script(script: String, env: &[(&str, &str)], with_curl: bool) -> (i32, String) { use std::os::unix::fs::PermissionsExt; let bin = temp_dir().join("bin"); @@ -388,7 +393,7 @@ mod tests { // missing one; everything else the block uses is a shell builtin. let output = Command::new("/bin/sh") .arg("-c") - .arg(hook_block(hook_type)) + .arg(script) .env_clear() .env("PATH", &bin) .envs(env.iter().copied()) @@ -448,6 +453,21 @@ mod tests { assert!(out.contains("curl not found"), "{out}"); } + #[cfg(unix)] + #[test] + fn hook_works_inside_an_existing_set_u_hook() { + // `scan install` appends to existing hooks, which may use `set -u`. + let script = format!("set -u\n{}", hook_block(HookType::PreCommit)); + + let (code, out) = run_script(script.clone(), &[], true); + assert_eq!(code, 0, "{out}"); + assert!(out.contains("cli scan staged --silent --json"), "{out}"); + + let (code, out) = run_script(script, &BROKER_ENV, true); + assert_eq!(code, 0, "{out}"); + assert!(out.contains("http://h:1/__stashbase/scan/staged"), "{out}"); + } + #[cfg(unix)] #[test] fn hook_outside_sandbox_runs_the_cli_as_before() { From 815e26a72ff8371c8850849bbbf03d42ce0bedf9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 07:46:45 +0000 Subject: [PATCH 09/22] fix(scan): escape shell parameter braces in the hook format string --- src/handlers/scans/install.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/handlers/scans/install.rs b/src/handlers/scans/install.rs index c77fce0e..de03df6d 100644 --- a/src/handlers/scans/install.rs +++ b/src/handlers/scans/install.rs @@ -44,15 +44,15 @@ impl HookType { fn hook_block(hook_type: HookType) -> String { format!( r#"{start} -if [ -n "${STASHBASE_SCAN_BROKER_URL:-}" ]; then +if [ -n "${{STASHBASE_SCAN_BROKER_URL:-}}" ]; then command -v curl >/dev/null 2>&1 || {{ echo "curl not found in the agent sandbox. Cannot run the Stashbase scan." exit 1 }} curl -sS --fail-with-body --noproxy '*' -X POST \ - -H "Authorization: Bearer ${STASHBASE_HOOK_BROKER_TOKEN:-}" \ + -H "Authorization: Bearer ${{STASHBASE_HOOK_BROKER_TOKEN:-}}" \ "$STASHBASE_SCAN_BROKER_URL/{mode}" || exit 1 -elif [ "${STASHBASE_SANDBOX:-}" = "1" ]; then +elif [ "${{STASHBASE_SANDBOX:-}}" = "1" ]; then echo "Stashbase scan hook is not enabled for this agent run. Add allow_hooks = [\"secret_scan\"] to the agent profile." exit 1 else From 51a45574928c79992423437f8490ac8af88d46fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 07:47:26 +0000 Subject: [PATCH 10/22] style: cargo fmt --- src/handlers/agent/init.rs | 4 +++- src/handlers/entry/root.rs | 11 +++++------ src/handlers/run/entry.rs | 4 +++- src/handlers/run/proxy.rs | 10 ++++++++-- src/handlers/scans/install.rs | 5 ++++- src/models/agent.rs | 5 ++++- 6 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/handlers/agent/init.rs b/src/handlers/agent/init.rs index 7a25da3d..05998008 100644 --- a/src/handlers/agent/init.rs +++ b/src/handlers/agent/init.rs @@ -126,7 +126,9 @@ mod tests { fn template_starts_closed_and_includes_a_generic_rule() { assert!(PROFILE_TEMPLATE.contains("egress_hosts = []")); assert!(PROFILE_TEMPLATE.contains("allow_network_listeners = true")); - assert!(PROFILE_TEMPLATE.contains("# allow_hooks = [\"dependency_check\", \"secret_scan\"]")); + assert!( + PROFILE_TEMPLATE.contains("# allow_hooks = [\"dependency_check\", \"secret_scan\"]") + ); assert!(PROFILE_TEMPLATE.contains("[secrets.SECRET_NAME]")); assert!(PROFILE_TEMPLATE.contains("[[secrets.SECRET_NAME.rules]]")); } diff --git a/src/handlers/entry/root.rs b/src/handlers/entry/root.rs index fcd8512f..ad976c27 100644 --- a/src/handlers/entry/root.rs +++ b/src/handlers/entry/root.rs @@ -2372,12 +2372,11 @@ fn spawn_remote_session_rotation( mod tests { use super::{ audit_binding_sources, codex_mcp_binding_header_overrides, configured_host_matches, - directory_profile_git_warning, enabled_hooks, - ensure_replacement_session_is_compatible, exit_for, infer_remote_agent_type, - is_bare_agent_hook, remote_bindings, remote_codex_command_with_mcp_binding_headers, - remote_session_rotation_delay_for, remote_session_started_message, - remote_session_transport_identity, remote_source_env_names, secret_child_name, - summarize_audit_events, uses_local_dependency_hook_broker_mode, + directory_profile_git_warning, enabled_hooks, ensure_replacement_session_is_compatible, + exit_for, infer_remote_agent_type, is_bare_agent_hook, remote_bindings, + remote_codex_command_with_mcp_binding_headers, remote_session_rotation_delay_for, + remote_session_started_message, remote_session_transport_identity, remote_source_env_names, + secret_child_name, summarize_audit_events, uses_local_dependency_hook_broker_mode, }; use crate::api::remote_proxy::{RemoteBinding, RemoteBindingSource}; use crate::cmd::root::Cli; diff --git a/src/handlers/run/entry.rs b/src/handlers/run/entry.rs index 052295e1..c0c0a451 100644 --- a/src/handlers/run/entry.rs +++ b/src/handlers/run/entry.rs @@ -2056,7 +2056,9 @@ mod tests { secret_scan: true, }; - assert!(hook_broker_config(EnabledHooks::default(), Some("k".to_owned()), workdir).is_none()); + assert!( + hook_broker_config(EnabledHooks::default(), Some("k".to_owned()), workdir).is_none() + ); assert!(hook_broker_config(both, None, workdir).is_none()); assert!(hook_broker_config(both, Some(String::new()), workdir).is_none()); let deps_only = EnabledHooks { diff --git a/src/handlers/run/proxy.rs b/src/handlers/run/proxy.rs index 05c00bd1..e4814496 100644 --- a/src/handlers/run/proxy.rs +++ b/src/handlers/run/proxy.rs @@ -2481,7 +2481,10 @@ async fn read_capped(mut reader: impl tokio::io::AsyncRead + Unpin, limit: usize use tokio::io::AsyncReadExt; let mut kept = Vec::new(); - let _ = (&mut reader).take(limit as u64).read_to_end(&mut kept).await; + let _ = (&mut reader) + .take(limit as u64) + .read_to_end(&mut kept) + .await; let _ = tokio::io::copy(&mut reader, &mut tokio::io::sink()).await; kept } @@ -5580,7 +5583,10 @@ mod tests { let (status, body) = post_scan(&proxy, "unpushed", Some(&hook_token(&proxy))).await; assert_eq!(status, 422, "{body}"); - assert!(body.contains("args=scan unpushed --json --silent"), "{body}"); + assert!( + body.contains("args=scan unpushed --json --silent"), + "{body}" + ); proxy.stop().await; let _ = std::fs::remove_dir_all(dir); } diff --git a/src/handlers/scans/install.rs b/src/handlers/scans/install.rs index de03df6d..01fcbf8b 100644 --- a/src/handlers/scans/install.rs +++ b/src/handlers/scans/install.rs @@ -417,7 +417,10 @@ mod tests { let (code, out) = run_block(HookType::PrePush, &BROKER_ENV, true); assert_eq!(code, 0, "{out}"); - assert!(out.contains("http://h:1/__stashbase/scan/unpushed"), "{out}"); + assert!( + out.contains("http://h:1/__stashbase/scan/unpushed"), + "{out}" + ); assert!(out.contains("Authorization: Bearer t"), "{out}"); assert!(!out.contains("cli "), "{out}"); } diff --git a/src/models/agent.rs b/src/models/agent.rs index 290a74be..55268de8 100644 --- a/src/models/agent.rs +++ b/src/models/agent.rs @@ -75,7 +75,10 @@ impl AgentProfile { /// The hooks `allow_hooks` asks for, before checking an API key exists. pub fn requested_hooks(&self) -> EnabledHooks { EnabledHooks { - dependency_check: self.allow_hooks.iter().any(|hook| hook == "dependency_check"), + dependency_check: self + .allow_hooks + .iter() + .any(|hook| hook == "dependency_check"), secret_scan: self.allow_hooks.iter().any(|hook| hook == "secret_scan"), } } From cadcc3321685b9f514a400dd8b3f144b44f762f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 08:13:14 +0000 Subject: [PATCH 11/22] fix(scan): cap restricted scan input and ignore a symlinked scan config --- src/handlers/scans/commits.rs | 9 ++ src/handlers/scans/config.rs | 61 ++++++++++- src/handlers/scans/files.rs | 8 ++ src/models/validation.rs | 8 ++ src/utils/scans.rs | 198 ++++++++++++++++++++++++++++++++++ 5 files changed, 279 insertions(+), 5 deletions(-) diff --git a/src/handlers/scans/commits.rs b/src/handlers/scans/commits.rs index 1b752759..4f2bdc62 100644 --- a/src/handlers/scans/commits.rs +++ b/src/handlers/scans/commits.rs @@ -21,6 +21,7 @@ use crate::{ default_scan_exclude_patterns, file_content_equals, filter_new_findings, get_file_matches, get_latest_scan_file, is_binary_file, is_valid_sha256_hash, load_baseline_results, process_diff_line, save_scan_results, should_exclude_file, + RestrictedScanBudget, update_findings_with_file_matches, SCAN_CONTEXT_LINES, SCAN_IGNORE_LINE_COMMENT, }, spinner::new_spinner, @@ -787,6 +788,7 @@ pub fn get_unpushed_commit_hunks( let mut all_commit_changes = Vec::new(); let mut current = local_commit; + let mut budget = RestrictedScanBudget::for_current_scan(); // Check all commits that would be pushed (new commits since merge base) loop { @@ -811,6 +813,10 @@ pub fn get_unpushed_commit_hunks( let mut diff_opts = git2::DiffOptions::new(); diff_opts.context_lines(context_lines as u32); diff_opts.show_binary(false); + if let Some(budget) = budget.as_mut() { + budget.charge_commit()?; + RestrictedScanBudget::limit_diff_options(&mut diff_opts); + } let diff = repo .diff_tree_to_tree( @@ -821,6 +827,9 @@ pub fn get_unpushed_commit_hunks( .map_err(|e| ScanInputValidationError::GitDiffGeneration { message: e.message().to_string(), })?; + if let Some(budget) = budget.as_mut() { + budget.charge_diff(&repo, &diff)?; + } let state = Rc::new(RefCell::new(DiffProcessingState::new())); let ignore_line_comment = ignore_line_comment.to_string(); diff --git a/src/handlers/scans/config.rs b/src/handlers/scans/config.rs index 91010a9a..57d3c003 100644 --- a/src/handlers/scans/config.rs +++ b/src/handlers/scans/config.rs @@ -175,12 +175,63 @@ pub fn resolve_scan_config_path(config_file_path: Option) -> Option Option { + let default_config = workdir.join(DEFAULT_SCAN_CONFIG_PATH); + let usable = if restricted { + fs::symlink_metadata(&default_config).is_ok_and(|metadata| metadata.is_file()) } else { - None + default_config.exists() + }; + usable.then_some(default_config) +} + +#[cfg(all(test, unix))] +mod restricted_config_tests { + use super::{default_config_in, DEFAULT_SCAN_CONFIG_PATH}; + use std::fs; + + fn temp_dir() -> std::path::PathBuf { + let dir = std::env::temp_dir().join(format!( + "stashbase-scan-config-{}", + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + fs::create_dir_all(&dir).unwrap(); + dir + } + + #[test] + fn restricted_scan_ignores_a_symlinked_config() { + let dir = temp_dir(); + let outside = dir.join("outside-secret"); + fs::write(&outside, "excluded-files: []\n").unwrap(); + let workdir = dir.join("repo"); + fs::create_dir_all(&workdir).unwrap(); + std::os::unix::fs::symlink(&outside, workdir.join(DEFAULT_SCAN_CONFIG_PATH)).unwrap(); + + assert!(default_config_in(&workdir, true).is_none()); + assert!(default_config_in(&workdir, false).is_some()); + let _ = fs::remove_dir_all(dir); + } + + #[test] + fn restricted_scan_uses_a_regular_config() { + let dir = temp_dir(); + fs::write(dir.join(DEFAULT_SCAN_CONFIG_PATH), "excluded-files: []\n").unwrap(); + + assert_eq!( + default_config_in(&dir, true), + Some(dir.join(DEFAULT_SCAN_CONFIG_PATH)) + ); + let _ = fs::remove_dir_all(dir); } } diff --git a/src/handlers/scans/files.rs b/src/handlers/scans/files.rs index 3cb710e7..78d95da8 100644 --- a/src/handlers/scans/files.rs +++ b/src/handlers/scans/files.rs @@ -16,6 +16,7 @@ use crate::{ default_scan_exclude_patterns, file_content_equals, filter_new_findings, get_file_matches, get_latest_scan_file, is_binary_file, is_valid_sha256_hash, load_baseline_results, process_diff_line, save_scan_results, should_exclude_file, + RestrictedScanBudget, update_findings_with_file_matches, SCAN_CONTEXT_LINES, SCAN_IGNORE_LINE_COMMENT, }, spinner::new_spinner, @@ -782,9 +783,13 @@ fn get_file_hunks( } }; + let mut budget = RestrictedScanBudget::for_current_scan(); let mut diff_opts = git2::DiffOptions::new(); diff_opts.context_lines(context_lines as u32); diff_opts.show_binary(false); + if budget.is_some() { + RestrictedScanBudget::limit_diff_options(&mut diff_opts); + } if matches!(mode, FileScanMode::Changed) { diff_opts.include_untracked(true); diff_opts.recurse_untracked_dirs(true); @@ -801,6 +806,9 @@ fn get_file_hunks( .map_err(|e| ScanInputValidationError::GitDiffGeneration { message: e.message().to_string(), })?; + if let Some(budget) = budget.as_mut() { + budget.charge_diff(&repo, &diff)?; + } let state = Rc::new(RefCell::new(DiffProcessingState::new())); diff --git a/src/models/validation.rs b/src/models/validation.rs index 809f0c44..9afcdac2 100644 --- a/src/models/validation.rs +++ b/src/models/validation.rs @@ -194,6 +194,7 @@ pub enum ScanInputValidationError { ConfigFileParse { path: String, message: String }, InvalidIgnoreSecretRegex { regex: String, message: String }, InvalidIgnoreSecretHash { hash: String }, + RestrictedScanTooLarge { detail: String }, } #[derive(Debug, Serialize)] @@ -1141,6 +1142,13 @@ impl ScanInputValidationError { "Invalid ignore secret hash.", Some(Box::leak(format!("Hash: {}", hash).into_boxed_str())), ), + ScanInputValidationError::RestrictedScanTooLarge { detail } => ( + "Changes are too large for a sandboxed scan.", + Some(Box::leak( + format!("{detail}. Commit or push them from outside the agent sandbox.") + .into_boxed_str(), + )), + ), } } } diff --git a/src/utils/scans.rs b/src/utils/scans.rs index 7f1789f1..2d8d977f 100644 --- a/src/utils/scans.rs +++ b/src/utils/scans.rs @@ -612,3 +612,201 @@ pub fn update_findings_with_file_matches( } } } + +pub const RESTRICTED_SCAN_MAX_FILE_BYTES: u64 = 1024 * 1024; +pub const RESTRICTED_SCAN_MAX_TOTAL_BYTES: u64 = 32 * 1024 * 1024; +pub const RESTRICTED_SCAN_MAX_FILES: usize = 10_000; +pub const RESTRICTED_SCAN_MAX_COMMITS: usize = 1_000; + +/// Caps what a restricted scan reads. The Agent Proxy runs it on the host for +/// a sandboxed agent, which controls the staged content and the history, so +/// without a cap it could make the host load arbitrarily large blobs. +pub struct RestrictedScanBudget { + remaining_bytes: u64, + remaining_files: usize, + remaining_commits: usize, +} + +impl RestrictedScanBudget { + pub fn new() -> Self { + Self { + remaining_bytes: RESTRICTED_SCAN_MAX_TOTAL_BYTES, + remaining_files: RESTRICTED_SCAN_MAX_FILES, + remaining_commits: RESTRICTED_SCAN_MAX_COMMITS, + } + } + + /// `Some` when this process is a restricted scan. + pub fn for_current_scan() -> Option { + (std::env::var(crate::models::scans::SCAN_RESTRICTED_ENV).as_deref() == Ok("1")) + .then(Self::new) + } + + /// Keeps libgit2 from loading a blob above the per-file cap, as a second + /// line behind `charge_diff`. + pub fn limit_diff_options(options: &mut git2::DiffOptions) { + options.max_size(RESTRICTED_SCAN_MAX_FILE_BYTES as i64); + } + + pub fn charge_commit(&mut self) -> Result<(), ScanInputValidationError> { + if self.remaining_commits == 0 { + return Err(too_large(format!( + "more than {RESTRICTED_SCAN_MAX_COMMITS} commits to scan" + ))); + } + self.remaining_commits -= 1; + Ok(()) + } + + /// Charges every changed file in `diff` before any content is loaded. + /// Sizes come from the object headers, which the agent cannot misstate + /// the way it can an index entry. + pub fn charge_diff( + &mut self, + repo: &git2::Repository, + diff: &git2::Diff, + ) -> Result<(), ScanInputValidationError> { + let odb = repo + .odb() + .map_err(|e| ScanInputValidationError::GitDiffProcessing { + message: e.message().to_string(), + })?; + for delta in diff.deltas() { + if delta.status() == git2::Delta::Deleted { + continue; + } + if self.remaining_files == 0 { + return Err(too_large(format!( + "more than {RESTRICTED_SCAN_MAX_FILES} changed files" + ))); + } + self.remaining_files -= 1; + + let file = delta.new_file(); + if file.id().is_zero() { + continue; + } + let (size, _) = odb.read_header(file.id()).map_err(|e| { + ScanInputValidationError::GitDiffProcessing { + message: e.message().to_string(), + } + })?; + let size = size as u64; + if size > RESTRICTED_SCAN_MAX_FILE_BYTES { + let path = file + .path() + .map(|path| path.to_string_lossy().into_owned()) + .unwrap_or_default(); + return Err(too_large(format!("'{path}' is larger than 1 MiB"))); + } + if size > self.remaining_bytes { + return Err(too_large("the changes total more than 32 MiB".to_owned())); + } + self.remaining_bytes -= size; + } + Ok(()) + } +} + +fn too_large(detail: String) -> ScanInputValidationError { + ScanInputValidationError::RestrictedScanTooLarge { detail } +} + +#[cfg(test)] +mod restricted_budget_tests { + use super::*; + + fn temp_repo() -> (std::path::PathBuf, git2::Repository) { + let dir = std::env::temp_dir().join(format!( + "stashbase-scan-budget-{}", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + let repo = git2::Repository::init(&dir).unwrap(); + (dir, repo) + } + + fn diff_adding(repo: &git2::Repository, files: &[(&str, usize)]) -> git2::Diff<'_> { + let mut builder = repo.treebuilder(None).unwrap(); + for (name, size) in files { + let blob = repo.blob(&vec![b'a'; *size]).unwrap(); + builder.insert(name, blob, 0o100644).unwrap(); + } + let tree = repo.find_tree(builder.write().unwrap()).unwrap(); + repo.diff_tree_to_tree(None, Some(&tree), None).unwrap() + } + + #[test] + fn budget_accepts_ordinary_changes() { + let (dir, repo) = temp_repo(); + let diff = diff_adding(&repo, &[("a.txt", 10), ("b.txt", 1024)]); + + assert!(RestrictedScanBudget::new().charge_diff(&repo, &diff).is_ok()); + let _ = fs::remove_dir_all(dir); + } + + #[test] + fn budget_rejects_a_file_over_the_per_file_cap() { + let (dir, repo) = temp_repo(); + let size = RESTRICTED_SCAN_MAX_FILE_BYTES as usize + 1; + let diff = diff_adding(&repo, &[("big.txt", size)]); + + let error = RestrictedScanBudget::new() + .charge_diff(&repo, &diff) + .unwrap_err(); + + assert!( + matches!(&error, ScanInputValidationError::RestrictedScanTooLarge { detail } if detail.contains("big.txt")), + "{error:?}" + ); + let _ = fs::remove_dir_all(dir); + } + + #[test] + fn budget_rejects_changes_over_the_total_cap_across_diffs() { + let (dir, repo) = temp_repo(); + let per_file = RESTRICTED_SCAN_MAX_FILE_BYTES as usize; + let files = (0..20) + .map(|index| (format!("f{index}.txt"), per_file)) + .collect::>(); + let files = files + .iter() + .map(|(name, size)| (name.as_str(), *size)) + .collect::>(); + let (first, second) = files.split_at(16); + let mut budget = RestrictedScanBudget::new(); + + // 16 MiB and then 4 MiB fit; another 16 MiB, as in a later + // commit's diff, crosses the 32 MiB total. + assert!(budget + .charge_diff(&repo, &diff_adding(&repo, first)) + .is_ok()); + assert!(budget + .charge_diff(&repo, &diff_adding(&repo, second)) + .is_ok()); + let error = budget + .charge_diff(&repo, &diff_adding(&repo, first)) + .unwrap_err(); + + assert!( + matches!(&error, ScanInputValidationError::RestrictedScanTooLarge { detail } if detail.contains("32 MiB")), + "{error:?}" + ); + let _ = fs::remove_dir_all(dir); + } + + #[test] + fn budget_caps_the_number_of_commits() { + let mut budget = RestrictedScanBudget::new(); + for _ in 0..RESTRICTED_SCAN_MAX_COMMITS { + budget.charge_commit().unwrap(); + } + + assert!(matches!( + budget.charge_commit(), + Err(ScanInputValidationError::RestrictedScanTooLarge { .. }) + )); + } +} From 4a6927e99a1093a7a90a01da648b7668ad67e883 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 08:17:55 +0000 Subject: [PATCH 12/22] fix(agent): confine the host-side secret scan to the agent's view of the filesystem --- docs/agent-profiles.md | 2 +- docs/sandboxing.md | 2 +- src/handlers/agent/validate.rs | 11 +- src/handlers/run/entry.rs | 94 ++++++-- src/handlers/run/mod.rs | 1 + src/handlers/run/proxy.rs | 60 ++++- src/handlers/run/scan_sandbox.rs | 365 +++++++++++++++++++++++++++++++ src/handlers/run/subprocess.rs | 4 +- 8 files changed, 509 insertions(+), 30 deletions(-) create mode 100644 src/handlers/run/scan_sandbox.rs diff --git a/docs/agent-profiles.md b/docs/agent-profiles.md index 7fc9f161..4d36c2ee 100644 --- a/docs/agent-profiles.md +++ b/docs/agent-profiles.md @@ -175,7 +175,7 @@ allow_hooks = ["dependency_check", "secret_scan"] The child receives only a scoped local broker token, never your API key. Hooks are disabled by default. -`secret_scan` lets the git hooks from `stashbase scan install` run inside the sandbox. The hook sends only a token to the Agent Proxy, which runs `stashbase scan staged` (or `unpushed`) on the host, in the run's working directory, with your key. Because the agent can edit the repo's scan config, these scans ignore its `match` and `output-dir` settings; `excluded-files` and `ignored-secrets` still apply. The hook needs `curl` in the sandbox (the default Docker image has it). Hooks installed before this existed need `stashbase scan install` once more to pick up the sandbox-aware block. Without `secret_scan`, a scan hook inside the sandbox fails with a message pointing here. +`secret_scan` lets the git hooks from `stashbase scan install` run inside the sandbox. The hook sends only a token to the Agent Proxy, which runs `stashbase scan staged` (or `unpushed`) on the host, in the run's working directory, with your key. Because the agent can edit the repo's scan config, these scans ignore its `match` and `output-dir` settings; `excluded-files` and `ignored-secrets` still apply. The host-side scan is confined to roughly the agent's view of your files: home directories (and `/Volumes` on macOS, `/mnt`, `/media` and `/srv` on Linux) are hidden except the run's working tree and git directories, and the profile's `deny_read` entries stay denied, so a symlink or a `.git` redirect planted by the agent can't point it at other files. This uses Seatbelt on macOS and bubblewrap on Linux; where neither is available (including Windows), a profile with `secret_scan` refuses to start. Restricted scans also stop at 1 MiB per changed file, 32 MiB in total, 10,000 files or 1,000 commits; larger changes have to be committed from outside the sandbox. The hook needs `curl` in the sandbox (the default Docker image has it). Hooks installed before this existed need `stashbase scan install` once more to pick up the sandbox-aware block. Without `secret_scan`, a scan hook inside the sandbox fails with a message pointing here. ## Audit Logs and Session Revocation diff --git a/docs/sandboxing.md b/docs/sandboxing.md index 9c5bf6db..150eae04 100644 --- a/docs/sandboxing.md +++ b/docs/sandboxing.md @@ -113,7 +113,7 @@ Your global `git config user.name` and `user.email` (if configured on the host) ### Secret scan hooks -Git hooks installed with `stashbase scan install` work in the sandbox when the profile sets `allow_hooks = ["secret_scan"]`. The sandbox has neither the Stashbase CLI nor your API key, so the hook asks the Agent Proxy to run the scan on the host against the same working directory, with `curl` and a per-run token. Findings come back to the agent, and the commit or push is blocked, exactly as outside the sandbox. Like any git hook, it is skipped by `git commit --no-verify` — a safety net, not an enforcement boundary. See [Agent Profiles](agent-profiles.md#api-hooks). +Git hooks installed with `stashbase scan install` work in the sandbox when the profile sets `allow_hooks = ["secret_scan"]`. The sandbox has neither the Stashbase CLI nor your API key, so the hook asks the Agent Proxy to run the scan on the host against the same working directory, with `curl` and a per-run token. Findings come back to the agent, and the commit or push is blocked, exactly as outside the sandbox. The scan itself runs confined (Seatbelt on macOS, bubblewrap on Linux) so it can only read the run's working tree and git directories, never files the agent points it at through symlinks or `.git` redirects; `secret_scan` is unavailable on Windows for that reason. Like any git hook, it is skipped by `git commit --no-verify` — a safety net, not an enforcement boundary. See [Agent Profiles](agent-profiles.md#api-hooks). ### Notifications, herdr and cmux diff --git a/src/handlers/agent/validate.rs b/src/handlers/agent/validate.rs index 45e5af23..73d531f1 100644 --- a/src/handlers/agent/validate.rs +++ b/src/handlers/agent/validate.rs @@ -709,6 +709,11 @@ fn validate_hook_capabilities(profile: &AgentProfile) -> Vec { }) .collect(); } + if profile.allow_hooks.iter().any(|hook| hook == "secret_scan") { + if let Some(reason) = crate::handlers::run::scan_sandbox::unavailable_reason() { + return vec![fail("Hook capability", format!("{reason}."))]; + } + } vec![ok( "Hook capabilities", if profile.allow_hooks.is_empty() { @@ -1151,7 +1156,11 @@ mod tests { let checks = validate_hook_capabilities(&profile); - assert!(checks.iter().all(|check| check.status != Status::Fail)); + let confinable = crate::handlers::run::scan_sandbox::unavailable_reason().is_none(); + assert_eq!( + checks.iter().all(|check| check.status != Status::Fail), + confinable + ); } #[test] diff --git a/src/handlers/run/entry.rs b/src/handlers/run/entry.rs index c0c0a451..715d0a22 100644 --- a/src/handlers/run/entry.rs +++ b/src/handlers/run/entry.rs @@ -381,29 +381,55 @@ fn finish_run_worktree(worktree: &super::worktree::RunWorktree, docker: bool, si } /// What the proxy's hook broker serves for this run, or `None` when no hook -/// is enabled or there is no API key to call Stashbase with. +/// is enabled or there is no API key to call Stashbase with. Fails when +/// `secret_scan` is enabled but its host-side scan can't be confined here. fn hook_broker_config( hooks: crate::models::agent::EnabledHooks, api_key: Option, workdir: &Path, -) -> Option { + denied_read: &[String], +) -> anyhow::Result> { + let Some(api_key) = api_key.filter(|key| !key.is_empty()) else { + return Ok(None); + }; if !hooks.any() { - return None; + return Ok(None); } - Some(super::proxy::HookBrokerConfig { - api_key: api_key.filter(|key| !key.is_empty())?, + // The scan runs the host's own CLI in the directory the agent works in. + let secret_scan = if hooks.secret_scan { + if let Some(reason) = super::scan_sandbox::unavailable_reason() { + anyhow::bail!(reason); + } + let exe = std::env::current_exe() + .map_err(|error| anyhow::anyhow!("secret_scan cannot locate the stashbase binary: {error}"))?; + Some(super::proxy::SecretScanConfig { + workdir: workdir.to_owned(), + isolation: super::proxy::ScanIsolation::Confined( + super::scan_sandbox::ScanConfinement::for_run(workdir, &exe, denied_read), + ), + exe, + timeout: std::time::Duration::from_secs(120), + }) + } else { + None + }; + Ok(Some(super::proxy::HookBrokerConfig { + api_key, dependency_check: hooks.dependency_check, - // The scan runs the host's own CLI in the directory the agent works in. - secret_scan: hooks - .secret_scan - .then(|| std::env::current_exe().ok()) - .flatten() - .map(|exe| super::proxy::SecretScanConfig { - workdir: workdir.to_owned(), - exe, - timeout: std::time::Duration::from_secs(120), - }), - }) + secret_scan, + })) +} + +/// Checked before any sandbox setup, so a refusal leaves nothing to clean up. +fn ensure_secret_scan_can_be_confined( + hooks: crate::models::agent::EnabledHooks, +) -> anyhow::Result<()> { + if hooks.secret_scan { + if let Some(reason) = super::scan_sandbox::unavailable_reason() { + anyhow::bail!(reason); + } + } + Ok(()) } /// Runs an agent through the localhost relay while credentials stay in the @@ -421,6 +447,7 @@ pub async fn handle_remote_agent_run( source_env_names: Vec, silent: bool, ) -> anyhow::Result<()> { + ensure_secret_scan_can_be_confined(hooks)?; let cmd = command.first().context("no command provided")?.clone(); let args = command.into_iter().skip(1).collect(); let denied_read_paths = policy.denied_read_paths.clone(); @@ -472,7 +499,8 @@ pub async fn handle_remote_agent_run( Some(workdir) => workdir.to_owned(), None => std::env::current_dir()?, }; - let remote_hooks = hook_broker_config(hooks, Some(api_key), &scan_workdir); + let remote_hooks = + hook_broker_config(hooks, Some(api_key), &scan_workdir, &denied_read_paths)?; let proxy_start_result = if let Some(network) = &docker_network { super::proxy::Proxy::start_remote_with_hook_and_bind_host( remote, @@ -1554,6 +1582,7 @@ async fn handle_run( hook_api_key: Option, local_session: Option, ) -> anyhow::Result<()> { + ensure_secret_scan_can_be_confined(hooks)?; apply_secret_bindings(&mut secrets, secret_bindings); let secrets_hash_map = if secret_bindings.is_empty() { env::expand_and_inject_env(&mut secrets) @@ -1738,7 +1767,8 @@ async fn handle_run( Some(workdir) => workdir.to_owned(), None => std::env::current_dir()?, }; - let local_hooks = hook_broker_config(hooks, hook_api_key, &scan_workdir); + let local_hooks = + hook_broker_config(hooks, hook_api_key, &scan_workdir, &denied_read_paths)?; let proxy_start_result = if let Some(network) = &docker_network { super::proxy::Proxy::start_with_hook_and_bind_host( secrets_hash_map, @@ -2039,13 +2069,23 @@ mod tests { secret_scan: true, }; - let config = hook_broker_config(scan_only, Some("k".to_owned()), workdir).unwrap(); + let result = hook_broker_config(scan_only, Some("k".to_owned()), workdir, &[]); + if let Some(reason) = crate::handlers::run::scan_sandbox::unavailable_reason() { + // Refused up front rather than run unconfined. + assert_eq!(result.unwrap_err().to_string(), reason); + return; + } + let config = result.unwrap().unwrap(); assert!(!config.dependency_check); let scan = config.secret_scan.unwrap(); assert_eq!(scan.workdir, workdir); assert_eq!(scan.exe, std::env::current_exe().unwrap()); assert_eq!(scan.timeout, std::time::Duration::from_secs(120)); + assert!(matches!( + scan.isolation, + crate::handlers::run::proxy::ScanIsolation::Confined(_) + )); } #[test] @@ -2057,15 +2097,23 @@ mod tests { }; assert!( - hook_broker_config(EnabledHooks::default(), Some("k".to_owned()), workdir).is_none() + hook_broker_config(EnabledHooks::default(), Some("k".to_owned()), workdir, &[]) + .unwrap() + .is_none() ); - assert!(hook_broker_config(both, None, workdir).is_none()); - assert!(hook_broker_config(both, Some(String::new()), workdir).is_none()); + assert!(hook_broker_config(both, None, workdir, &[]) + .unwrap() + .is_none()); + assert!(hook_broker_config(both, Some(String::new()), workdir, &[]) + .unwrap() + .is_none()); let deps_only = EnabledHooks { dependency_check: true, secret_scan: false, }; - let config = hook_broker_config(deps_only, Some("k".to_owned()), workdir).unwrap(); + let config = hook_broker_config(deps_only, Some("k".to_owned()), workdir, &[]) + .unwrap() + .unwrap(); assert!(config.dependency_check && config.secret_scan.is_none()); } diff --git a/src/handlers/run/mod.rs b/src/handlers/run/mod.rs index c1ab0a0b..0c0a9069 100644 --- a/src/handlers/run/mod.rs +++ b/src/handlers/run/mod.rs @@ -4,6 +4,7 @@ pub mod format; pub mod fs_rules; pub mod proxy; pub mod routing; +pub mod scan_sandbox; pub mod subprocess; pub mod trust; pub mod worktree; diff --git a/src/handlers/run/proxy.rs b/src/handlers/run/proxy.rs index e4814496..c6c8e222 100644 --- a/src/handlers/run/proxy.rs +++ b/src/handlers/run/proxy.rs @@ -104,6 +104,34 @@ pub struct SecretScanConfig { /// Run as ` scan --json --silent`. pub exe: PathBuf, pub timeout: Duration, + pub isolation: ScanIsolation, +} + +pub enum ScanIsolation { + /// The scan reads an agent-controlled repository, so it runs with roughly + /// the agent's own view of the host filesystem. + Confined(super::scan_sandbox::ScanConfinement), + /// Runs `exe` directly, for tests of the route itself. + #[cfg(test)] + Unconfined, +} + +/// An empty home for one scan, so it reads none of the user's dotfiles +/// (git's global config, the CLI's own config) and can still write. +struct ScratchHome(PathBuf); + +impl ScratchHome { + fn create() -> std::io::Result { + let path = std::env::temp_dir().join(format!("stashbase-scan-home-{}", Uuid::new_v4())); + std::fs::create_dir_all(&path)?; + Ok(Self(path)) + } +} + +impl Drop for ScratchHome { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } } struct HookBroker { @@ -2524,10 +2552,37 @@ async fn handle_secret_scan_hook( }; let _guard = broker.scan_lock.lock().await; - let mut command = tokio::process::Command::new(&scan.exe); + let scan_failed = || { + proxy_error_response( + StatusCode::BAD_GATEWAY, + "proxy.secret_scan_failed", + "Secret scan could not be started", + ) + }; + let Ok(home) = ScratchHome::create() else { + return scan_failed(); + }; + let scan_args = ["scan", mode, "--json", "--silent"]; + let (program, args) = match &scan.isolation { + ScanIsolation::Confined(confinement) => match confinement.wrap(&scan_args, &home.0) { + Ok(command) => command, + Err(_) => return scan_failed(), + }, + #[cfg(test)] + ScanIsolation::Unconfined => ( + scan.exe.to_string_lossy().into_owned(), + scan_args.iter().map(|arg| (*arg).to_owned()).collect(), + ), + }; + let mut command = tokio::process::Command::new(program); command - .args(["scan", mode, "--json", "--silent"]) + .args(args) .current_dir(&scan.workdir) + .env("HOME", &home.0) + .env_remove("XDG_CONFIG_HOME") + .env_remove("XDG_CACHE_HOME") + .env_remove("XDG_DATA_HOME") + .env_remove("XDG_STATE_HOME") .env("STASHBASE_API_KEY", &broker.api_key) .env(crate::models::scans::SCAN_RESTRICTED_ENV, "1") // Keeps the scan's telemetry off, like everything else in the session. @@ -5514,6 +5569,7 @@ mod tests { workdir, exe, timeout, + isolation: ScanIsolation::Unconfined, }), }), ) diff --git a/src/handlers/run/scan_sandbox.rs b/src/handlers/run/scan_sandbox.rs new file mode 100644 index 00000000..6b09f85c --- /dev/null +++ b/src/handlers/run/scan_sandbox.rs @@ -0,0 +1,365 @@ +//! Confines the host-side secret scan the Agent Proxy runs for a sandboxed +//! agent. The scan reads a repository the agent controls, so a symlink, a +//! `gitdir:` file, `objects/info/alternates` or `include.path` could +//! otherwise point it at any file on the host. Under this confinement the +//! scan sees roughly what the agent sees: home directories are hidden except +//! the run's working tree, its git directories and the CLI binary, and the +//! profile's `deny_read` entries stay denied. + +use anyhow::Result; +use std::path::{Path, PathBuf}; + +pub struct ScanConfinement { + workdir: PathBuf, + git_dirs: Vec, + exe: PathBuf, + denied_read: Vec, +} + +impl ScanConfinement { + /// Captures the run's git directories now, before the agent can point + /// `.git` anywhere else. + pub fn for_run(workdir: &Path, exe: &Path, denied_read: &[String]) -> Self { + let workdir = canonical(workdir); + let mut git_dirs = Vec::new(); + if let Ok(repo) = git2::Repository::discover(&workdir) { + for dir in [repo.path(), repo.commondir()] { + let dir = canonical(dir); + if !dir.starts_with(&workdir) && !git_dirs.contains(&dir) { + git_dirs.push(dir); + } + } + } + Self { + workdir, + git_dirs, + exe: canonical(exe), + denied_read: denied_read.to_vec(), + } + } + + /// The program and arguments that run `exe args` confined, with `home` + /// (an empty, writable directory) standing in for the user's home. + pub fn wrap(&self, args: &[&str], home: &Path) -> Result<(String, Vec)> { + platform::wrap(self, args, home) + } + + #[cfg_attr(not(any(target_os = "macos", target_os = "linux")), allow(dead_code))] + fn readable(&self, home: &Path) -> Vec { + let mut paths = vec![self.workdir.clone(), self.exe.clone(), canonical(home)]; + paths.extend(self.git_dirs.iter().cloned()); + paths + } +} + +/// Why secret scans can't be confined on this machine, if they can't. +pub fn unavailable_reason() -> Option { + platform::unavailable_reason() +} + +fn canonical(path: &Path) -> PathBuf { + path.canonicalize().unwrap_or_else(|_| path.to_owned()) +} + +/// Roots that hold users' files, hidden from the scan. +#[cfg_attr(not(any(target_os = "macos", target_os = "linux")), allow(dead_code))] +fn hidden_roots(candidates: &[&str]) -> Vec { + let mut roots = candidates + .iter() + .map(PathBuf::from) + .chain(std::env::var_os("HOME").map(PathBuf::from)) + .filter(|path| path.exists()) + .map(|path| canonical(&path)) + .collect::>(); + roots.sort(); + roots.dedup(); + // A root inside another root adds nothing. + let all = roots.clone(); + roots.retain(|root| !all.iter().any(|other| other != root && root.starts_with(other))); + roots +} + +#[cfg(target_os = "macos")] +mod platform { + use super::{hidden_roots, ScanConfinement}; + use crate::handlers::run::subprocess::{denied_file_rules, escape_sbpl_path}; + use anyhow::Result; + use std::path::{Path, PathBuf}; + + pub fn unavailable_reason() -> Option { + None + } + + pub fn wrap( + confinement: &ScanConfinement, + args: &[&str], + home: &Path, + ) -> Result<(String, Vec)> { + let mut command = vec!["-p".to_owned(), profile(confinement, home)?]; + command.push(confinement.exe.to_string_lossy().into_owned()); + command.extend(args.iter().map(|arg| (*arg).to_owned())); + Ok(("/usr/bin/sandbox-exec".to_owned(), command)) + } + + pub(super) fn profile(confinement: &ScanConfinement, home: &Path) -> Result { + let readable = confinement.readable(home); + let quote = |path: &Path| format!("\"{}\"", escape_sbpl_path(&path.to_string_lossy())); + + let mut rules = vec!["(version 1)".to_owned(), "(allow default)".to_owned()]; + for root in hidden_roots(&["/Users", "/Volumes", "/private/var/root"]) { + rules.push(format!("(deny file-read* (subpath {}))", quote(&root))); + } + // Path resolution (realpath, libgit2's repository lookup) stats each + // ancestor of the paths the scan needs; that metadata is all it gets. + let mut ancestors = readable + .iter() + .flat_map(|path| path.ancestors().skip(1).map(PathBuf::from)) + .collect::>(); + ancestors.sort(); + ancestors.dedup(); + for ancestor in ancestors { + rules.push(format!( + "(allow file-read-metadata (literal {}))", + quote(&ancestor) + )); + } + for path in &readable { + let filter = if path.is_file() { "literal" } else { "subpath" }; + rules.push(format!("(allow file-read* ({filter} {}))", quote(path))); + } + // Last, so the profile's own denials win inside the working tree. + rules.push(denied_file_rules( + &confinement.denied_read, + &[], + &confinement.workdir, + )?); + Ok(rules.join("\n")) + } +} + +#[cfg(target_os = "linux")] +mod platform { + use super::{hidden_roots, ScanConfinement}; + use crate::handlers::run::{docker_sandbox::empty_shadow_file_path, fs_rules}; + use anyhow::Result; + use std::path::{Path, PathBuf}; + + fn executable() -> Option<&'static str> { + ["bwrap", "bubblewrap"].into_iter().find(|name| { + std::env::var_os("PATH").is_some_and(|path| { + std::env::split_paths(&path).any(|directory| directory.join(name).is_file()) + }) + }) + } + + pub fn unavailable_reason() -> Option { + let Some(executable) = executable() else { + return Some( + "secret_scan needs bubblewrap (`bwrap`) on Linux to confine the host-side scan" + .to_owned(), + ); + }; + let probe = std::process::Command::new(executable) + .args([ + "--die-with-parent", + "--ro-bind", + "/", + "/", + "--tmpfs", + "/tmp", + "--", + "/bin/true", + ]) + .output(); + match probe { + Ok(output) if output.status.success() => None, + Ok(output) => Some(format!( + "secret_scan needs a working bubblewrap to confine the host-side scan: {}", + String::from_utf8_lossy(&output.stderr).trim() + )), + Err(error) => Some(format!( + "secret_scan needs a working bubblewrap to confine the host-side scan: {error}" + )), + } + } + + pub fn wrap( + confinement: &ScanConfinement, + args: &[&str], + home: &Path, + ) -> Result<(String, Vec)> { + let executable = executable() + .ok_or_else(|| anyhow::anyhow!("bubblewrap is not available"))? + .to_owned(); + let mut command = bwrap_args(confinement, home).map_err(anyhow::Error::msg)?; + command.push("--".to_owned()); + command.push(confinement.exe.to_string_lossy().into_owned()); + command.extend(args.iter().map(|arg| (*arg).to_owned())); + Ok((executable, command)) + } + + pub(super) fn bwrap_args( + confinement: &ScanConfinement, + home: &Path, + ) -> Result, String> { + let text = |path: &Path| path.to_string_lossy().into_owned(); + let mut args = vec![ + "--die-with-parent".to_owned(), + "--ro-bind".to_owned(), + "/".to_owned(), + "/".to_owned(), + "--proc".to_owned(), + "/proc".to_owned(), + "--dev".to_owned(), + "/dev".to_owned(), + ]; + for root in hidden_roots(&["/home", "/root", "/mnt", "/media", "/srv"]) { + args.extend(["--tmpfs".to_owned(), text(&root)]); + } + let home = super::canonical(home); + for path in confinement.readable(&home) { + let mode = if path == home { "--bind" } else { "--ro-bind" }; + args.extend([mode.to_owned(), text(&path), text(&path)]); + } + for path in fs_rules::expand_policy_entries( + &confinement.denied_read, + &confinement.workdir, + None, + )? { + if PathBuf::from(&path).is_dir() { + args.extend(["--tmpfs".to_owned(), path]); + } else { + args.extend(["--ro-bind".to_owned(), empty_shadow_file_path()?, path]); + } + } + args.extend(["--chdir".to_owned(), text(&confinement.workdir)]); + Ok(args) + } +} + +#[cfg(not(any(target_os = "macos", target_os = "linux")))] +mod platform { + use super::ScanConfinement; + use anyhow::Result; + use std::path::Path; + + pub fn unavailable_reason() -> Option { + Some("secret_scan is supported only on macOS and Linux, where the host-side scan can be confined".to_owned()) + } + + pub fn wrap(_: &ScanConfinement, _: &[&str], _: &Path) -> Result<(String, Vec)> { + anyhow::bail!(unavailable_reason().unwrap_or_default()) + } +} + +#[cfg(all(test, any(target_os = "macos", target_os = "linux")))] +mod tests { + use super::*; + use std::fs; + + struct Layout { + root: PathBuf, + workdir: PathBuf, + home: PathBuf, + outside: PathBuf, + } + + /// A repo, a scratch home, and a secret outside both, all inside a + /// directory under the real home so the hidden roots cover it. + fn layout() -> Option { + let base = std::env::var_os("HOME").map(PathBuf::from)?; + let root = base.join(format!(".stashbase-scan-sandbox-test-{}", uuid::Uuid::new_v4())); + let workdir = root.join("repo"); + let home = std::env::temp_dir().join(format!("stashbase-scan-home-{}", uuid::Uuid::new_v4())); + fs::create_dir_all(&workdir).ok()?; + fs::create_dir_all(&home).ok()?; + git2::Repository::init(&workdir).ok()?; + fs::write(workdir.join("inside.txt"), "inside").ok()?; + let outside = root.join("outside-secret.txt"); + fs::write(&outside, "outside").ok()?; + std::os::unix::fs::symlink(&outside, workdir.join("link.txt")).ok()?; + Some(Layout { + root: canonical(&root), + workdir: canonical(&workdir), + home, + outside: canonical(&outside), + }) + } + + fn run_cat(confinement: &ScanConfinement, home: &Path, file: &Path) -> std::process::Output { + let (program, args) = confinement + .wrap(&[&file.to_string_lossy()], home) + .unwrap(); + std::process::Command::new(program) + .args(args) + .current_dir(&confinement.workdir) + .output() + .unwrap() + } + + #[test] + fn confined_scan_reads_the_worktree_but_not_host_files_it_links_to() { + if let Some(reason) = unavailable_reason() { + eprintln!("skipping: {reason}"); + return; + } + let Some(layout) = layout() else { + eprintln!("skipping: no writable HOME"); + return; + }; + // `cat` stands in for the CLI binary: the confinement is about which + // files the process can read, not what it does with them. + let cat = canonical(Path::new("/bin/cat")); + let confinement = ScanConfinement::for_run(&layout.workdir, &cat, &[]); + + let inside = run_cat(&confinement, &layout.home, &layout.workdir.join("inside.txt")); + let via_link = run_cat(&confinement, &layout.home, &layout.workdir.join("link.txt")); + let direct = run_cat(&confinement, &layout.home, &layout.outside); + + assert_eq!(String::from_utf8_lossy(&inside.stdout), "inside"); + assert!(!via_link.status.success(), "followed a symlink out of the worktree"); + assert!(!direct.status.success(), "read a host file outside the worktree"); + let _ = fs::remove_dir_all(&layout.root); + let _ = fs::remove_dir_all(&layout.home); + } + + #[test] + fn confined_scan_keeps_profile_deny_read_inside_the_worktree() { + if let Some(reason) = unavailable_reason() { + eprintln!("skipping: {reason}"); + return; + } + let Some(layout) = layout() else { + eprintln!("skipping: no writable HOME"); + return; + }; + fs::write(layout.workdir.join(".env"), "SECRET=1").unwrap(); + let cat = canonical(Path::new("/bin/cat")); + let confinement = ScanConfinement::for_run(&layout.workdir, &cat, &[".env".to_owned()]); + + let denied = run_cat(&confinement, &layout.home, &layout.workdir.join(".env")); + + assert!( + !String::from_utf8_lossy(&denied.stdout).contains("SECRET"), + "read a deny_read file" + ); + let _ = fs::remove_dir_all(&layout.root); + let _ = fs::remove_dir_all(&layout.home); + } + + #[test] + fn for_run_captures_git_dirs_outside_the_worktree() { + let Some(layout) = layout() else { + return; + }; + let cat = canonical(Path::new("/bin/cat")); + + let confinement = ScanConfinement::for_run(&layout.workdir, &cat, &[]); + + // A plain repo's `.git` is inside the worktree, so nothing extra. + assert!(confinement.git_dirs.is_empty()); + assert_eq!(confinement.workdir, layout.workdir); + let _ = fs::remove_dir_all(&layout.root); + let _ = fs::remove_dir_all(&layout.home); + } +} diff --git a/src/handlers/run/subprocess.rs b/src/handlers/run/subprocess.rs index 29642799..eb985919 100644 --- a/src/handlers/run/subprocess.rs +++ b/src/handlers/run/subprocess.rs @@ -831,7 +831,7 @@ fn codex_workspace_rules(boundary: CodexSandboxBoundary, current_dir: &Path) -> } #[cfg(target_os = "macos")] -fn denied_file_rules( +pub(crate) fn denied_file_rules( deny_read: &[String], deny_write: &[String], current_dir: &Path, @@ -967,7 +967,7 @@ fn filesystem_denial_from_line( } #[cfg(target_os = "macos")] -fn escape_sbpl_path(path: &str) -> String { +pub(crate) fn escape_sbpl_path(path: &str) -> String { path.replace('\\', "\\\\").replace('"', "\\\"") } From c36db518656b6f263d44354728260dd23d654acd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 08:19:48 +0000 Subject: [PATCH 13/22] fix: compile errors in the scan confinement and budget tests --- src/handlers/run/entry.rs | 15 ++++++------ src/handlers/run/proxy.rs | 11 ++++----- src/handlers/run/scan_sandbox.rs | 41 +++++++++++++++++++++----------- src/handlers/scans/commits.rs | 4 ++-- src/handlers/scans/config.rs | 3 +-- src/handlers/scans/files.rs | 4 ++-- src/utils/scans.rs | 9 +++++-- 7 files changed, 51 insertions(+), 36 deletions(-) diff --git a/src/handlers/run/entry.rs b/src/handlers/run/entry.rs index 715d0a22..42cc9760 100644 --- a/src/handlers/run/entry.rs +++ b/src/handlers/run/entry.rs @@ -400,14 +400,14 @@ fn hook_broker_config( if let Some(reason) = super::scan_sandbox::unavailable_reason() { anyhow::bail!(reason); } - let exe = std::env::current_exe() - .map_err(|error| anyhow::anyhow!("secret_scan cannot locate the stashbase binary: {error}"))?; + let exe = std::env::current_exe().map_err(|error| { + anyhow::anyhow!("secret_scan cannot locate the stashbase binary: {error}") + })?; Some(super::proxy::SecretScanConfig { workdir: workdir.to_owned(), isolation: super::proxy::ScanIsolation::Confined( super::scan_sandbox::ScanConfinement::for_run(workdir, &exe, denied_read), ), - exe, timeout: std::time::Duration::from_secs(120), }) } else { @@ -499,8 +499,7 @@ pub async fn handle_remote_agent_run( Some(workdir) => workdir.to_owned(), None => std::env::current_dir()?, }; - let remote_hooks = - hook_broker_config(hooks, Some(api_key), &scan_workdir, &denied_read_paths)?; + let remote_hooks = hook_broker_config(hooks, Some(api_key), &scan_workdir, &denied_read_paths)?; let proxy_start_result = if let Some(network) = &docker_network { super::proxy::Proxy::start_remote_with_hook_and_bind_host( remote, @@ -2073,14 +2072,16 @@ mod tests { if let Some(reason) = crate::handlers::run::scan_sandbox::unavailable_reason() { // Refused up front rather than run unconfined. - assert_eq!(result.unwrap_err().to_string(), reason); + let Err(error) = result else { + panic!("secret_scan ran without confinement"); + }; + assert_eq!(error.to_string(), reason); return; } let config = result.unwrap().unwrap(); assert!(!config.dependency_check); let scan = config.secret_scan.unwrap(); assert_eq!(scan.workdir, workdir); - assert_eq!(scan.exe, std::env::current_exe().unwrap()); assert_eq!(scan.timeout, std::time::Duration::from_secs(120)); assert!(matches!( scan.isolation, diff --git a/src/handlers/run/proxy.rs b/src/handlers/run/proxy.rs index c6c8e222..c5151857 100644 --- a/src/handlers/run/proxy.rs +++ b/src/handlers/run/proxy.rs @@ -101,8 +101,6 @@ pub struct HookBrokerConfig { pub struct SecretScanConfig { /// The run's working directory, scanned on the host. pub workdir: PathBuf, - /// Run as ` scan --json --silent`. - pub exe: PathBuf, pub timeout: Duration, pub isolation: ScanIsolation, } @@ -113,7 +111,7 @@ pub enum ScanIsolation { Confined(super::scan_sandbox::ScanConfinement), /// Runs `exe` directly, for tests of the route itself. #[cfg(test)] - Unconfined, + Unconfined { exe: PathBuf }, } /// An empty home for one scan, so it reads none of the user's dotfiles @@ -2569,8 +2567,8 @@ async fn handle_secret_scan_hook( Err(_) => return scan_failed(), }, #[cfg(test)] - ScanIsolation::Unconfined => ( - scan.exe.to_string_lossy().into_owned(), + ScanIsolation::Unconfined { exe } => ( + exe.to_string_lossy().into_owned(), scan_args.iter().map(|arg| (*arg).to_owned()).collect(), ), }; @@ -5567,9 +5565,8 @@ mod tests { dependency_check: false, secret_scan: Some(SecretScanConfig { workdir, - exe, timeout, - isolation: ScanIsolation::Unconfined, + isolation: ScanIsolation::Unconfined { exe }, }), }), ) diff --git a/src/handlers/run/scan_sandbox.rs b/src/handlers/run/scan_sandbox.rs index 6b09f85c..fa66ba82 100644 --- a/src/handlers/run/scan_sandbox.rs +++ b/src/handlers/run/scan_sandbox.rs @@ -75,7 +75,10 @@ fn hidden_roots(candidates: &[&str]) -> Vec { roots.dedup(); // A root inside another root adds nothing. let all = roots.clone(); - roots.retain(|root| !all.iter().any(|other| other != root && root.starts_with(other))); + roots.retain(|root| { + !all.iter() + .any(|other| other != root && root.starts_with(other)) + }); roots } @@ -221,11 +224,9 @@ mod platform { let mode = if path == home { "--bind" } else { "--ro-bind" }; args.extend([mode.to_owned(), text(&path), text(&path)]); } - for path in fs_rules::expand_policy_entries( - &confinement.denied_read, - &confinement.workdir, - None, - )? { + for path in + fs_rules::expand_policy_entries(&confinement.denied_read, &confinement.workdir, None)? + { if PathBuf::from(&path).is_dir() { args.extend(["--tmpfs".to_owned(), path]); } else { @@ -268,9 +269,13 @@ mod tests { /// directory under the real home so the hidden roots cover it. fn layout() -> Option { let base = std::env::var_os("HOME").map(PathBuf::from)?; - let root = base.join(format!(".stashbase-scan-sandbox-test-{}", uuid::Uuid::new_v4())); + let root = base.join(format!( + ".stashbase-scan-sandbox-test-{}", + uuid::Uuid::new_v4() + )); let workdir = root.join("repo"); - let home = std::env::temp_dir().join(format!("stashbase-scan-home-{}", uuid::Uuid::new_v4())); + let home = + std::env::temp_dir().join(format!("stashbase-scan-home-{}", uuid::Uuid::new_v4())); fs::create_dir_all(&workdir).ok()?; fs::create_dir_all(&home).ok()?; git2::Repository::init(&workdir).ok()?; @@ -287,9 +292,7 @@ mod tests { } fn run_cat(confinement: &ScanConfinement, home: &Path, file: &Path) -> std::process::Output { - let (program, args) = confinement - .wrap(&[&file.to_string_lossy()], home) - .unwrap(); + let (program, args) = confinement.wrap(&[&file.to_string_lossy()], home).unwrap(); std::process::Command::new(program) .args(args) .current_dir(&confinement.workdir) @@ -312,13 +315,23 @@ mod tests { let cat = canonical(Path::new("/bin/cat")); let confinement = ScanConfinement::for_run(&layout.workdir, &cat, &[]); - let inside = run_cat(&confinement, &layout.home, &layout.workdir.join("inside.txt")); + let inside = run_cat( + &confinement, + &layout.home, + &layout.workdir.join("inside.txt"), + ); let via_link = run_cat(&confinement, &layout.home, &layout.workdir.join("link.txt")); let direct = run_cat(&confinement, &layout.home, &layout.outside); assert_eq!(String::from_utf8_lossy(&inside.stdout), "inside"); - assert!(!via_link.status.success(), "followed a symlink out of the worktree"); - assert!(!direct.status.success(), "read a host file outside the worktree"); + assert!( + !via_link.status.success(), + "followed a symlink out of the worktree" + ); + assert!( + !direct.status.success(), + "read a host file outside the worktree" + ); let _ = fs::remove_dir_all(&layout.root); let _ = fs::remove_dir_all(&layout.home); } diff --git a/src/handlers/scans/commits.rs b/src/handlers/scans/commits.rs index 4f2bdc62..572a5029 100644 --- a/src/handlers/scans/commits.rs +++ b/src/handlers/scans/commits.rs @@ -21,8 +21,8 @@ use crate::{ default_scan_exclude_patterns, file_content_equals, filter_new_findings, get_file_matches, get_latest_scan_file, is_binary_file, is_valid_sha256_hash, load_baseline_results, process_diff_line, save_scan_results, should_exclude_file, - RestrictedScanBudget, - update_findings_with_file_matches, SCAN_CONTEXT_LINES, SCAN_IGNORE_LINE_COMMENT, + update_findings_with_file_matches, RestrictedScanBudget, SCAN_CONTEXT_LINES, + SCAN_IGNORE_LINE_COMMENT, }, spinner::new_spinner, validation::validate_project_identifier, diff --git a/src/handlers/scans/config.rs b/src/handlers/scans/config.rs index 57d3c003..a24f4403 100644 --- a/src/handlers/scans/config.rs +++ b/src/handlers/scans/config.rs @@ -176,8 +176,7 @@ pub fn resolve_scan_config_path(config_file_path: Option) -> Option git2::Diff<'_> { + fn diff_adding<'repo>( + repo: &'repo git2::Repository, + files: &[(&str, usize)], + ) -> git2::Diff<'repo> { let mut builder = repo.treebuilder(None).unwrap(); for (name, size) in files { let blob = repo.blob(&vec![b'a'; *size]).unwrap(); @@ -743,7 +746,9 @@ mod restricted_budget_tests { let (dir, repo) = temp_repo(); let diff = diff_adding(&repo, &[("a.txt", 10), ("b.txt", 1024)]); - assert!(RestrictedScanBudget::new().charge_diff(&repo, &diff).is_ok()); + assert!(RestrictedScanBudget::new() + .charge_diff(&repo, &diff) + .is_ok()); let _ = fs::remove_dir_all(dir); } From a4805dcf869620df6440a1119fb02416a61b1899 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 08:23:34 +0000 Subject: [PATCH 14/22] fix(agent): deny the host-side scan file reads by default, allowing only the run's paths and system files --- docs/agent-profiles.md | 2 +- docs/sandboxing.md | 2 +- src/handlers/run/proxy.rs | 1 + src/handlers/run/scan_sandbox.rs | 236 ++++++++++++++++++------------- 4 files changed, 143 insertions(+), 98 deletions(-) diff --git a/docs/agent-profiles.md b/docs/agent-profiles.md index 4d36c2ee..042a8185 100644 --- a/docs/agent-profiles.md +++ b/docs/agent-profiles.md @@ -175,7 +175,7 @@ allow_hooks = ["dependency_check", "secret_scan"] The child receives only a scoped local broker token, never your API key. Hooks are disabled by default. -`secret_scan` lets the git hooks from `stashbase scan install` run inside the sandbox. The hook sends only a token to the Agent Proxy, which runs `stashbase scan staged` (or `unpushed`) on the host, in the run's working directory, with your key. Because the agent can edit the repo's scan config, these scans ignore its `match` and `output-dir` settings; `excluded-files` and `ignored-secrets` still apply. The host-side scan is confined to roughly the agent's view of your files: home directories (and `/Volumes` on macOS, `/mnt`, `/media` and `/srv` on Linux) are hidden except the run's working tree and git directories, and the profile's `deny_read` entries stay denied, so a symlink or a `.git` redirect planted by the agent can't point it at other files. This uses Seatbelt on macOS and bubblewrap on Linux; where neither is available (including Windows), a profile with `secret_scan` refuses to start. Restricted scans also stop at 1 MiB per changed file, 32 MiB in total, 10,000 files or 1,000 commits; larger changes have to be committed from outside the sandbox. The hook needs `curl` in the sandbox (the default Docker image has it). Hooks installed before this existed need `stashbase scan install` once more to pick up the sandbox-aware block. Without `secret_scan`, a scan hook inside the sandbox fails with a message pointing here. +`secret_scan` lets the git hooks from `stashbase scan install` run inside the sandbox. The hook sends only a token to the Agent Proxy, which runs `stashbase scan staged` (or `unpushed`) on the host, in the run's working directory, with your key. Because the agent can edit the repo's scan config, these scans ignore its `match` and `output-dir` settings; `excluded-files` and `ignored-secrets` still apply. The host-side scan is confined: it can read file contents only from the run's working tree, its git directories, the CLI binary, an empty scratch home and the operating system's own files (system libraries, `/etc`, TLS roots), and the profile's `deny_read` entries stay denied, so a symlink or a `.git` redirect planted by the agent can't point it at your other files. On macOS, file names and sizes elsewhere remain visible to it, since path lookup needs them; contents do not. This uses Seatbelt on macOS and bubblewrap on Linux; where neither is available (including Windows), a profile with `secret_scan` refuses to start. Restricted scans also stop at 1 MiB per changed file, 32 MiB in total, 10,000 files or 1,000 commits; larger changes have to be committed from outside the sandbox. The hook needs `curl` in the sandbox (the default Docker image has it). Hooks installed before this existed need `stashbase scan install` once more to pick up the sandbox-aware block. Without `secret_scan`, a scan hook inside the sandbox fails with a message pointing here. ## Audit Logs and Session Revocation diff --git a/docs/sandboxing.md b/docs/sandboxing.md index 150eae04..bf88201a 100644 --- a/docs/sandboxing.md +++ b/docs/sandboxing.md @@ -113,7 +113,7 @@ Your global `git config user.name` and `user.email` (if configured on the host) ### Secret scan hooks -Git hooks installed with `stashbase scan install` work in the sandbox when the profile sets `allow_hooks = ["secret_scan"]`. The sandbox has neither the Stashbase CLI nor your API key, so the hook asks the Agent Proxy to run the scan on the host against the same working directory, with `curl` and a per-run token. Findings come back to the agent, and the commit or push is blocked, exactly as outside the sandbox. The scan itself runs confined (Seatbelt on macOS, bubblewrap on Linux) so it can only read the run's working tree and git directories, never files the agent points it at through symlinks or `.git` redirects; `secret_scan` is unavailable on Windows for that reason. Like any git hook, it is skipped by `git commit --no-verify` — a safety net, not an enforcement boundary. See [Agent Profiles](agent-profiles.md#api-hooks). +Git hooks installed with `stashbase scan install` work in the sandbox when the profile sets `allow_hooks = ["secret_scan"]`. The sandbox has neither the Stashbase CLI nor your API key, so the hook asks the Agent Proxy to run the scan on the host against the same working directory, with `curl` and a per-run token. Findings come back to the agent, and the commit or push is blocked, exactly as outside the sandbox. The scan itself runs confined (Seatbelt on macOS, bubblewrap on Linux): beyond the operating system's own files, it can read only the run's working tree, its git directories and the CLI binary, never files the agent points it at through symlinks or `.git` redirects; `secret_scan` is unavailable on Windows for that reason. Like any git hook, it is skipped by `git commit --no-verify` — a safety net, not an enforcement boundary. See [Agent Profiles](agent-profiles.md#api-hooks). ### Notifications, herdr and cmux diff --git a/src/handlers/run/proxy.rs b/src/handlers/run/proxy.rs index c5151857..2c59689d 100644 --- a/src/handlers/run/proxy.rs +++ b/src/handlers/run/proxy.rs @@ -2577,6 +2577,7 @@ async fn handle_secret_scan_hook( .args(args) .current_dir(&scan.workdir) .env("HOME", &home.0) + .env("TMPDIR", &home.0) .env_remove("XDG_CONFIG_HOME") .env_remove("XDG_CACHE_HOME") .env_remove("XDG_DATA_HOME") diff --git a/src/handlers/run/scan_sandbox.rs b/src/handlers/run/scan_sandbox.rs index fa66ba82..c4ec6e64 100644 --- a/src/handlers/run/scan_sandbox.rs +++ b/src/handlers/run/scan_sandbox.rs @@ -1,10 +1,10 @@ //! Confines the host-side secret scan the Agent Proxy runs for a sandboxed //! agent. The scan reads a repository the agent controls, so a symlink, a //! `gitdir:` file, `objects/info/alternates` or `include.path` could -//! otherwise point it at any file on the host. Under this confinement the -//! scan sees roughly what the agent sees: home directories are hidden except -//! the run's working tree, its git directories and the CLI binary, and the -//! profile's `deny_read` entries stay denied. +//! otherwise point it at any file on the host. Reading file contents is +//! denied by default; the scan can read only the run's working tree, its git +//! directories, the CLI binary, a scratch home and the operating system's +//! own files, and the profile's `deny_read` entries stay denied. use anyhow::Result; use std::path::{Path, PathBuf}; @@ -39,12 +39,16 @@ impl ScanConfinement { } /// The program and arguments that run `exe args` confined, with `home` - /// (an empty, writable directory) standing in for the user's home. + /// (an empty, writable directory) standing in for the user's home and + /// temporary directory. pub fn wrap(&self, args: &[&str], home: &Path) -> Result<(String, Vec)> { platform::wrap(self, args, home) } - #[cfg_attr(not(any(target_os = "macos", target_os = "linux")), allow(dead_code))] + #[cfg_attr( + not(any(target_os = "macos", target_os = "linux")), + allow(dead_code) + )] fn readable(&self, home: &Path) -> Vec { let mut paths = vec![self.workdir.clone(), self.exe.clone(), canonical(home)]; paths.extend(self.git_dirs.iter().cloned()); @@ -61,33 +65,31 @@ fn canonical(path: &Path) -> PathBuf { path.canonicalize().unwrap_or_else(|_| path.to_owned()) } -/// Roots that hold users' files, hidden from the scan. -#[cfg_attr(not(any(target_os = "macos", target_os = "linux")), allow(dead_code))] -fn hidden_roots(candidates: &[&str]) -> Vec { - let mut roots = candidates - .iter() - .map(PathBuf::from) - .chain(std::env::var_os("HOME").map(PathBuf::from)) - .filter(|path| path.exists()) - .map(|path| canonical(&path)) - .collect::>(); - roots.sort(); - roots.dedup(); - // A root inside another root adds nothing. - let all = roots.clone(); - roots.retain(|root| { - !all.iter() - .any(|other| other != root && root.starts_with(other)) - }); - roots -} - #[cfg(target_os = "macos")] mod platform { - use super::{hidden_roots, ScanConfinement}; + use super::ScanConfinement; use crate::handlers::run::subprocess::{denied_file_rules, escape_sbpl_path}; use anyhow::Result; - use std::path::{Path, PathBuf}; + use std::path::Path; + + /// What the CLI needs from the OS itself: dyld and system frameworks, + /// TLS trust roots, time zones, `/etc` (DNS, hosts) and devices. + const SYSTEM_READABLE: &[&str] = &[ + "/System", + "/usr", + "/bin", + "/sbin", + "/Library/Apple", + "/Library/Keychains", + "/Library/Preferences", + "/Library/Security", + "/private/etc", + "/private/var/db/dyld", + "/private/var/db/mds", + "/private/var/db/timezone", + "/private/var/run/resolv.conf", + "/dev", + ]; pub fn unavailable_reason() -> Option { None @@ -105,31 +107,22 @@ mod platform { } pub(super) fn profile(confinement: &ScanConfinement, home: &Path) -> Result { - let readable = confinement.readable(home); let quote = |path: &Path| format!("\"{}\"", escape_sbpl_path(&path.to_string_lossy())); - - let mut rules = vec!["(version 1)".to_owned(), "(allow default)".to_owned()]; - for root in hidden_roots(&["/Users", "/Volumes", "/private/var/root"]) { - rules.push(format!("(deny file-read* (subpath {}))", quote(&root))); - } - // Path resolution (realpath, libgit2's repository lookup) stats each - // ancestor of the paths the scan needs; that metadata is all it gets. - let mut ancestors = readable - .iter() - .flat_map(|path| path.ancestors().skip(1).map(PathBuf::from)) - .collect::>(); - ancestors.sort(); - ancestors.dedup(); - for ancestor in ancestors { + // Metadata (stat, realpath, libgit2's repository lookup) stays + // readable everywhere; contents are denied unless allowed below. + let mut rules = vec![ + "(version 1)".to_owned(), + "(allow default)".to_owned(), + "(deny file-read-data file-read-xattr)".to_owned(), + ]; + let system = SYSTEM_READABLE.iter().map(Path::new).map(Path::to_path_buf); + for path in system.chain(confinement.readable(home)) { + let filter = if path.is_file() { "literal" } else { "subpath" }; rules.push(format!( - "(allow file-read-metadata (literal {}))", - quote(&ancestor) + "(allow file-read-data file-read-xattr ({filter} {}))", + quote(&path) )); } - for path in &readable { - let filter = if path.is_file() { "literal" } else { "subpath" }; - rules.push(format!("(allow file-read* ({filter} {}))", quote(path))); - } // Last, so the profile's own denials win inside the working tree. rules.push(denied_file_rules( &confinement.denied_read, @@ -142,11 +135,34 @@ mod platform { #[cfg(target_os = "linux")] mod platform { - use super::{hidden_roots, ScanConfinement}; + use super::ScanConfinement; use crate::handlers::run::{docker_sandbox::empty_shadow_file_path, fs_rules}; use anyhow::Result; use std::path::{Path, PathBuf}; + /// Top-level directories that are usually symlinks into `/usr`. + const ROOT_LINKS: &[&str] = &["/bin", "/sbin", "/lib", "/lib32", "/lib64", "/libx32"]; + + /// What the CLI needs from `/etc` and `/run`: DNS, TLS trust roots, user + /// lookup, time zone and the dynamic linker cache. + const SYSTEM_FILES: &[&str] = &[ + "/etc/resolv.conf", + "/etc/hosts", + "/etc/nsswitch.conf", + "/etc/host.conf", + "/etc/gai.conf", + "/etc/ssl", + "/etc/ca-certificates", + "/etc/pki", + "/etc/passwd", + "/etc/group", + "/etc/localtime", + "/etc/ld.so.cache", + "/etc/ld.so.conf", + "/etc/ld.so.conf.d", + "/run/systemd/resolve", + ]; + fn executable() -> Option<&'static str> { ["bwrap", "bubblewrap"].into_iter().find(|name| { std::env::var_os("PATH").is_some_and(|path| { @@ -201,6 +217,8 @@ mod platform { Ok((executable, command)) } + /// Builds the scan's root from nothing: unlike the agent's own Linux + /// sandbox, `/` is not bound, so only what is listed here exists. pub(super) fn bwrap_args( confinement: &ScanConfinement, home: &Path, @@ -208,25 +226,39 @@ mod platform { let text = |path: &Path| path.to_string_lossy().into_owned(); let mut args = vec![ "--die-with-parent".to_owned(), - "--ro-bind".to_owned(), - "/".to_owned(), - "/".to_owned(), "--proc".to_owned(), "/proc".to_owned(), "--dev".to_owned(), "/dev".to_owned(), + "--tmpfs".to_owned(), + "/tmp".to_owned(), + "--ro-bind".to_owned(), + "/usr".to_owned(), + "/usr".to_owned(), ]; - for root in hidden_roots(&["/home", "/root", "/mnt", "/media", "/srv"]) { - args.extend(["--tmpfs".to_owned(), text(&root)]); + for link in ROOT_LINKS { + let path = Path::new(link); + match std::fs::read_link(path) { + Ok(target) => args.extend(["--symlink".to_owned(), text(&target), text(path)]), + Err(_) if path.is_dir() => { + args.extend(["--ro-bind".to_owned(), text(path), text(path)]) + } + Err(_) => {} + } + } + for file in SYSTEM_FILES { + args.extend(["--ro-bind-try".to_owned(), (*file).to_owned(), (*file).to_owned()]); } let home = super::canonical(home); for path in confinement.readable(&home) { let mode = if path == home { "--bind" } else { "--ro-bind" }; args.extend([mode.to_owned(), text(&path), text(&path)]); } - for path in - fs_rules::expand_policy_entries(&confinement.denied_read, &confinement.workdir, None)? - { + for path in fs_rules::expand_policy_entries( + &confinement.denied_read, + &confinement.workdir, + None, + )? { if PathBuf::from(&path).is_dir() { args.extend(["--tmpfs".to_owned(), path]); } else { @@ -265,14 +297,17 @@ mod tests { outside: PathBuf, } - /// A repo, a scratch home, and a secret outside both, all inside a - /// directory under the real home so the hidden roots cover it. - fn layout() -> Option { - let base = std::env::var_os("HOME").map(PathBuf::from)?; - let root = base.join(format!( - ".stashbase-scan-sandbox-test-{}", - uuid::Uuid::new_v4() - )); + impl Drop for Layout { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.root); + let _ = fs::remove_dir_all(&self.home); + } + } + + /// A repo under `base`, with a secret next to it (outside the worktree) + /// and a symlink from the worktree to that secret. + fn layout(base: &Path) -> Option { + let root = base.join(format!(".stashbase-scan-sandbox-test-{}", uuid::Uuid::new_v4())); let workdir = root.join("repo"); let home = std::env::temp_dir().join(format!("stashbase-scan-home-{}", uuid::Uuid::new_v4())); @@ -292,7 +327,9 @@ mod tests { } fn run_cat(confinement: &ScanConfinement, home: &Path, file: &Path) -> std::process::Output { - let (program, args) = confinement.wrap(&[&file.to_string_lossy()], home).unwrap(); + let (program, args) = confinement + .wrap(&[&file.to_string_lossy()], home) + .unwrap(); std::process::Command::new(program) .args(args) .current_dir(&confinement.workdir) @@ -300,40 +337,52 @@ mod tests { .unwrap() } - #[test] - fn confined_scan_reads_the_worktree_but_not_host_files_it_links_to() { - if let Some(reason) = unavailable_reason() { - eprintln!("skipping: {reason}"); - return; - } - let Some(layout) = layout() else { - eprintln!("skipping: no writable HOME"); + /// `cat` stands in for the CLI binary: the confinement is about which + /// files the process can read, not what it does with them. + fn assert_reads_only_the_worktree(base: &Path) { + let Some(layout) = layout(base) else { + eprintln!("skipping: cannot create a test layout under {}", base.display()); return; }; - // `cat` stands in for the CLI binary: the confinement is about which - // files the process can read, not what it does with them. let cat = canonical(Path::new("/bin/cat")); let confinement = ScanConfinement::for_run(&layout.workdir, &cat, &[]); - let inside = run_cat( - &confinement, - &layout.home, - &layout.workdir.join("inside.txt"), - ); + let inside = run_cat(&confinement, &layout.home, &layout.workdir.join("inside.txt")); let via_link = run_cat(&confinement, &layout.home, &layout.workdir.join("link.txt")); let direct = run_cat(&confinement, &layout.home, &layout.outside); assert_eq!(String::from_utf8_lossy(&inside.stdout), "inside"); assert!( - !via_link.status.success(), - "followed a symlink out of the worktree" + !String::from_utf8_lossy(&via_link.stdout).contains("outside"), + "followed a symlink out of the worktree under {}", + base.display() ); assert!( - !direct.status.success(), - "read a host file outside the worktree" + !String::from_utf8_lossy(&direct.stdout).contains("outside"), + "read a host file outside the worktree under {}", + base.display() ); - let _ = fs::remove_dir_all(&layout.root); - let _ = fs::remove_dir_all(&layout.home); + } + + #[test] + fn confined_scan_cannot_read_outside_the_worktree_under_home() { + if let Some(reason) = unavailable_reason() { + eprintln!("skipping: {reason}"); + return; + } + if let Some(home) = std::env::var_os("HOME") { + assert_reads_only_the_worktree(Path::new(&home)); + } + } + + #[test] + fn confined_scan_cannot_read_outside_the_worktree_under_tmp() { + if let Some(reason) = unavailable_reason() { + eprintln!("skipping: {reason}"); + return; + } + assert_reads_only_the_worktree(Path::new("/tmp")); + assert_reads_only_the_worktree(&std::env::temp_dir()); } #[test] @@ -342,8 +391,7 @@ mod tests { eprintln!("skipping: {reason}"); return; } - let Some(layout) = layout() else { - eprintln!("skipping: no writable HOME"); + let Some(layout) = layout(&std::env::temp_dir()) else { return; }; fs::write(layout.workdir.join(".env"), "SECRET=1").unwrap(); @@ -356,13 +404,11 @@ mod tests { !String::from_utf8_lossy(&denied.stdout).contains("SECRET"), "read a deny_read file" ); - let _ = fs::remove_dir_all(&layout.root); - let _ = fs::remove_dir_all(&layout.home); } #[test] fn for_run_captures_git_dirs_outside_the_worktree() { - let Some(layout) = layout() else { + let Some(layout) = layout(&std::env::temp_dir()) else { return; }; let cat = canonical(Path::new("/bin/cat")); @@ -372,7 +418,5 @@ mod tests { // A plain repo's `.git` is inside the worktree, so nothing extra. assert!(confinement.git_dirs.is_empty()); assert_eq!(confinement.workdir, layout.workdir); - let _ = fs::remove_dir_all(&layout.root); - let _ = fs::remove_dir_all(&layout.home); } } From d30540696c6fe883ce164a48c83d6877ce125a31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 08:25:10 +0000 Subject: [PATCH 15/22] fix(agent): express the scan's read denial as one deny with exclusions --- src/handlers/run/scan_sandbox.rs | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/handlers/run/scan_sandbox.rs b/src/handlers/run/scan_sandbox.rs index c4ec6e64..35e17787 100644 --- a/src/handlers/run/scan_sandbox.rs +++ b/src/handlers/run/scan_sandbox.rs @@ -109,20 +109,23 @@ mod platform { pub(super) fn profile(confinement: &ScanConfinement, home: &Path) -> Result { let quote = |path: &Path| format!("\"{}\"", escape_sbpl_path(&path.to_string_lossy())); // Metadata (stat, realpath, libgit2's repository lookup) stays - // readable everywhere; contents are denied unless allowed below. + // readable everywhere; contents are denied outside the allowed + // paths. One deny with exclusions, since a later allow does not + // override an earlier deny for the same operation. + let system = SYSTEM_READABLE.iter().map(Path::new).map(Path::to_path_buf); + let allowed = system + .chain(confinement.readable(home)) + .map(|path| { + let filter = if path.is_file() { "literal" } else { "subpath" }; + format!("({filter} {})", quote(&path)) + }) + .collect::>() + .join(" "); let mut rules = vec![ "(version 1)".to_owned(), "(allow default)".to_owned(), - "(deny file-read-data file-read-xattr)".to_owned(), + format!("(deny file-read-data file-read-xattr (require-not (require-any {allowed})))"), ]; - let system = SYSTEM_READABLE.iter().map(Path::new).map(Path::to_path_buf); - for path in system.chain(confinement.readable(home)) { - let filter = if path.is_file() { "literal" } else { "subpath" }; - rules.push(format!( - "(allow file-read-data file-read-xattr ({filter} {}))", - quote(&path) - )); - } // Last, so the profile's own denials win inside the working tree. rules.push(denied_file_rules( &confinement.denied_read, From 1d4f9f9a357e2637c82ce6b0255e20ec556a84a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 08:51:23 +0000 Subject: [PATCH 16/22] fix(agent): give the brokered scan the run's resolved API URL --- src/api/client.rs | 2 +- src/handlers/run/proxy.rs | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/api/client.rs b/src/api/client.rs index 2d638262..7f4187f8 100644 --- a/src/api/client.rs +++ b/src/api/client.rs @@ -19,7 +19,7 @@ use crate::models::api_client::{ use crate::{REQUEST_ABORTED, REQUEST_TIMEOUT_SECS}; const DEFAULT_API_URL: &str = "https://api.stashbase.dev"; -const API_URL_ENV_VAR: &str = "STASHBASE_API_URL"; +pub(crate) const API_URL_ENV_VAR: &str = "STASHBASE_API_URL"; const BUILD_TIME_API_URL: Option<&str> = option_env!("STASHBASE_API_URL"); pub const CLI_USER_AGENT: &str = concat!("stashbase/cli/", env!("CARGO_PKG_VERSION")); diff --git a/src/handlers/run/proxy.rs b/src/handlers/run/proxy.rs index 2c59689d..50f86c21 100644 --- a/src/handlers/run/proxy.rs +++ b/src/handlers/run/proxy.rs @@ -2577,6 +2577,12 @@ async fn handle_secret_scan_hook( .args(args) .current_dir(&scan.workdir) .env("HOME", &home.0) + // The API this run uses, whether it came from the environment or was + // built in, so the scan never falls back to a different default. + .env( + crate::api::client::API_URL_ENV_VAR, + crate::api::client::get_api_url(), + ) .env("TMPDIR", &home.0) .env_remove("XDG_CONFIG_HOME") .env_remove("XDG_CACHE_HOME") @@ -5544,7 +5550,7 @@ mod tests { format!( "#!/bin/sh\nsleep {sleep_secs}\n\ echo \"args=$*\"\necho \"cwd=$(pwd -P)\"\n\ - echo \"key=$STASHBASE_API_KEY\"\necho \"restricted=$STASHBASE_SCAN_RESTRICTED\"\n\ + echo \"key=$STASHBASE_API_KEY\"\necho \"api_url=$STASHBASE_API_URL\"\necho \"restricted=$STASHBASE_SCAN_RESTRICTED\"\n\ echo \"hook_token=${{STASHBASE_HOOK_BROKER_TOKEN:-unset}}\"\n\ echo 'finding on stderr' >&2\nexit {exit_code}\n" ), @@ -5612,6 +5618,13 @@ mod tests { assert!(body.contains("args=scan staged --json --silent"), "{body}"); assert!(body.contains(&format!("cwd={}", dir.display())), "{body}"); assert!(body.contains("key=parent-api-key"), "{body}"); + assert!( + body.contains(&format!( + "api_url={}", + crate::api::client::get_api_url() + )), + "{body}" + ); assert!(body.contains("restricted=1"), "{body}"); assert!(body.contains("hook_token=unset"), "{body}"); assert!(body.contains("finding on stderr"), "{body}"); From 923601bc0b00418913d1674bc0dc82b760aeb220 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 08:55:59 +0000 Subject: [PATCH 17/22] fix(agent): allow the dyld cache's real path and report how a failed scan exited --- src/handlers/run/proxy.rs | 5 +++++ src/handlers/run/scan_sandbox.rs | 3 +++ 2 files changed, 8 insertions(+) diff --git a/src/handlers/run/proxy.rs b/src/handlers/run/proxy.rs index 50f86c21..8318ace9 100644 --- a/src/handlers/run/proxy.rs +++ b/src/handlers/run/proxy.rs @@ -2632,6 +2632,11 @@ async fn handle_secret_scan_hook( }; let mut body = stdout; body.extend_from_slice(&stderr); + if status == StatusCode::BAD_GATEWAY { + // A scan killed before printing anything (e.g. by the + // confinement) would otherwise leave an empty body. + body.extend_from_slice(format!("\nstashbase scan exited with {exit}\n").as_bytes()); + } body.truncate(SECRET_SCAN_BODY_LIMIT); Response::builder() .status(status) diff --git a/src/handlers/run/scan_sandbox.rs b/src/handlers/run/scan_sandbox.rs index 35e17787..a0db0f29 100644 --- a/src/handlers/run/scan_sandbox.rs +++ b/src/handlers/run/scan_sandbox.rs @@ -76,6 +76,9 @@ mod platform { /// TLS trust roots, time zones, `/etc` (DNS, hosts) and devices. const SYSTEM_READABLE: &[&str] = &[ "/System", + // The dyld shared cache's real location since macOS 13; + // `/System/Volumes/Preboot` is only a firmlink to it. + "/private/preboot", "/usr", "/bin", "/sbin", From 0c4ad29c51c81f78d895fdcb4ebe7c014bf74dde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 08:58:50 +0000 Subject: [PATCH 18/22] fix(agent): deny the scan only users' data locations on macOS so the CLI still loads --- docs/agent-profiles.md | 2 +- docs/sandboxing.md | 2 +- src/handlers/run/scan_sandbox.rs | 61 ++++++++++++++++---------------- 3 files changed, 32 insertions(+), 33 deletions(-) diff --git a/docs/agent-profiles.md b/docs/agent-profiles.md index 042a8185..1653beb4 100644 --- a/docs/agent-profiles.md +++ b/docs/agent-profiles.md @@ -175,7 +175,7 @@ allow_hooks = ["dependency_check", "secret_scan"] The child receives only a scoped local broker token, never your API key. Hooks are disabled by default. -`secret_scan` lets the git hooks from `stashbase scan install` run inside the sandbox. The hook sends only a token to the Agent Proxy, which runs `stashbase scan staged` (or `unpushed`) on the host, in the run's working directory, with your key. Because the agent can edit the repo's scan config, these scans ignore its `match` and `output-dir` settings; `excluded-files` and `ignored-secrets` still apply. The host-side scan is confined: it can read file contents only from the run's working tree, its git directories, the CLI binary, an empty scratch home and the operating system's own files (system libraries, `/etc`, TLS roots), and the profile's `deny_read` entries stay denied, so a symlink or a `.git` redirect planted by the agent can't point it at your other files. On macOS, file names and sizes elsewhere remain visible to it, since path lookup needs them; contents do not. This uses Seatbelt on macOS and bubblewrap on Linux; where neither is available (including Windows), a profile with `secret_scan` refuses to start. Restricted scans also stop at 1 MiB per changed file, 32 MiB in total, 10,000 files or 1,000 commits; larger changes have to be committed from outside the sandbox. The hook needs `curl` in the sandbox (the default Docker image has it). Hooks installed before this existed need `stashbase scan install` once more to pick up the sandbox-aware block. Without `secret_scan`, a scan hook inside the sandbox fails with a message pointing here. +`secret_scan` lets the git hooks from `stashbase scan install` run inside the sandbox. The hook sends only a token to the Agent Proxy, which runs `stashbase scan staged` (or `unpushed`) on the host, in the run's working directory, with your key. Because the agent can edit the repo's scan config, these scans ignore its `match` and `output-dir` settings; `excluded-files` and `ignored-secrets` still apply. The host-side scan is confined, so a symlink or a `.git` redirect planted by the agent can't point it at your other files. On Linux it sees only system files plus the run's working tree, git directories and the CLI binary. On macOS it can't read file contents anywhere users' data lives (home directories, `/Volumes`, `/tmp`, `/var/folders`, `/opt`) except those same run paths; system files stay readable, and file names and sizes stay visible because path lookup needs them. The profile's `deny_read` entries stay denied on both. This uses Seatbelt on macOS and bubblewrap on Linux; where neither is available (including Windows), a profile with `secret_scan` refuses to start. Restricted scans also stop at 1 MiB per changed file, 32 MiB in total, 10,000 files or 1,000 commits; larger changes have to be committed from outside the sandbox. The hook needs `curl` in the sandbox (the default Docker image has it). Hooks installed before this existed need `stashbase scan install` once more to pick up the sandbox-aware block. Without `secret_scan`, a scan hook inside the sandbox fails with a message pointing here. ## Audit Logs and Session Revocation diff --git a/docs/sandboxing.md b/docs/sandboxing.md index bf88201a..512ececd 100644 --- a/docs/sandboxing.md +++ b/docs/sandboxing.md @@ -113,7 +113,7 @@ Your global `git config user.name` and `user.email` (if configured on the host) ### Secret scan hooks -Git hooks installed with `stashbase scan install` work in the sandbox when the profile sets `allow_hooks = ["secret_scan"]`. The sandbox has neither the Stashbase CLI nor your API key, so the hook asks the Agent Proxy to run the scan on the host against the same working directory, with `curl` and a per-run token. Findings come back to the agent, and the commit or push is blocked, exactly as outside the sandbox. The scan itself runs confined (Seatbelt on macOS, bubblewrap on Linux): beyond the operating system's own files, it can read only the run's working tree, its git directories and the CLI binary, never files the agent points it at through symlinks or `.git` redirects; `secret_scan` is unavailable on Windows for that reason. Like any git hook, it is skipped by `git commit --no-verify` — a safety net, not an enforcement boundary. See [Agent Profiles](agent-profiles.md#api-hooks). +Git hooks installed with `stashbase scan install` work in the sandbox when the profile sets `allow_hooks = ["secret_scan"]`. The sandbox has neither the Stashbase CLI nor your API key, so the hook asks the Agent Proxy to run the scan on the host against the same working directory, with `curl` and a per-run token. Findings come back to the agent, and the commit or push is blocked, exactly as outside the sandbox. The scan itself runs confined (Seatbelt on macOS, bubblewrap on Linux): outside system files, it can read only the run's working tree, its git directories and the CLI binary, never your other files that the agent points it at through symlinks or `.git` redirects; `secret_scan` is unavailable on Windows for that reason. Like any git hook, it is skipped by `git commit --no-verify` — a safety net, not an enforcement boundary. See [Agent Profiles](agent-profiles.md#api-hooks). ### Notifications, herdr and cmux diff --git a/src/handlers/run/scan_sandbox.rs b/src/handlers/run/scan_sandbox.rs index a0db0f29..a35a6fe7 100644 --- a/src/handlers/run/scan_sandbox.rs +++ b/src/handlers/run/scan_sandbox.rs @@ -1,10 +1,13 @@ //! Confines the host-side secret scan the Agent Proxy runs for a sandboxed //! agent. The scan reads a repository the agent controls, so a symlink, a //! `gitdir:` file, `objects/info/alternates` or `include.path` could -//! otherwise point it at any file on the host. Reading file contents is -//! denied by default; the scan can read only the run's working tree, its git -//! directories, the CLI binary, a scratch home and the operating system's -//! own files, and the profile's `deny_read` entries stay denied. +//! otherwise point it at any file on the host. On Linux the scan's root is +//! built from nothing but system files and the run's paths. On macOS, where +//! the loader needs too many system paths to list reliably, file contents +//! under every location that holds users' data (homes, volumes, temporary +//! directories, `/opt`) are denied except the run's working tree, its git +//! directories, the CLI binary and a scratch home. On both, the profile's +//! `deny_read` entries stay denied. use anyhow::Result; use std::path::{Path, PathBuf}; @@ -72,26 +75,16 @@ mod platform { use anyhow::Result; use std::path::Path; - /// What the CLI needs from the OS itself: dyld and system frameworks, - /// TLS trust roots, time zones, `/etc` (DNS, hosts) and devices. - const SYSTEM_READABLE: &[&str] = &[ - "/System", - // The dyld shared cache's real location since macOS 13; - // `/System/Volumes/Preboot` is only a firmlink to it. - "/private/preboot", - "/usr", - "/bin", - "/sbin", - "/Library/Apple", - "/Library/Keychains", - "/Library/Preferences", - "/Library/Security", - "/private/etc", - "/private/var/db/dyld", - "/private/var/db/mds", - "/private/var/db/timezone", - "/private/var/run/resolv.conf", - "/dev", + /// Where users' files live on macOS. Everything else on the system + /// (libraries, frameworks, `/etc`) stays readable, so the CLI loads + /// normally; these are denied except for the run's own paths. + const USER_DATA_ROOTS: &[&str] = &[ + "/Users", + "/Volumes", + "/private/tmp", + "/private/var/folders", + "/private/var/root", + "/opt", ]; pub fn unavailable_reason() -> Option { @@ -112,12 +105,16 @@ mod platform { pub(super) fn profile(confinement: &ScanConfinement, home: &Path) -> Result { let quote = |path: &Path| format!("\"{}\"", escape_sbpl_path(&path.to_string_lossy())); // Metadata (stat, realpath, libgit2's repository lookup) stays - // readable everywhere; contents are denied outside the allowed - // paths. One deny with exclusions, since a later allow does not - // override an earlier deny for the same operation. - let system = SYSTEM_READABLE.iter().map(Path::new).map(Path::to_path_buf); - let allowed = system - .chain(confinement.readable(home)) + // readable everywhere. One deny with exclusions, since a later allow + // does not override an earlier deny for the same operation. + let roots = USER_DATA_ROOTS + .iter() + .map(|root| format!("(subpath {})", quote(Path::new(root)))) + .collect::>() + .join(" "); + let allowed = confinement + .readable(home) + .into_iter() .map(|path| { let filter = if path.is_file() { "literal" } else { "subpath" }; format!("({filter} {})", quote(&path)) @@ -127,7 +124,9 @@ mod platform { let mut rules = vec![ "(version 1)".to_owned(), "(allow default)".to_owned(), - format!("(deny file-read-data file-read-xattr (require-not (require-any {allowed})))"), + format!( + "(deny file-read-data file-read-xattr (require-all (require-any {roots}) (require-not (require-any {allowed}))))" + ), ]; // Last, so the profile's own denials win inside the working tree. rules.push(denied_file_rules( From 902d1dbfc8fd7aa7563bd906f0338283084afa7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 09:09:16 +0000 Subject: [PATCH 19/22] ci: install bubblewrap so the Linux scan confinement tests run --- .github/workflows/ci.yaml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 3b3c5039..707e7919 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -26,5 +26,10 @@ jobs: if: runner.os == 'Linux' run: cargo fmt --check + # The secret-scan confinement tests skip without a working bubblewrap. + - name: Install bubblewrap + if: runner.os == 'Linux' + run: sudo apt-get update && sudo apt-get install -y bubblewrap + - name: Run tests run: cargo test --locked From a7fc1841a2d1ae43d3535d46e1a4646539257db5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 09:09:41 +0000 Subject: [PATCH 20/22] test: fail the scan confinement tests in CI instead of skipping --- src/handlers/run/scan_sandbox.rs | 24 ++++++++++++++++++------ src/utils/scans.rs | 9 ++------- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/handlers/run/scan_sandbox.rs b/src/handlers/run/scan_sandbox.rs index a35a6fe7..62c2ca0d 100644 --- a/src/handlers/run/scan_sandbox.rs +++ b/src/handlers/run/scan_sandbox.rs @@ -342,6 +342,21 @@ mod tests { .unwrap() } + /// Skips locally when this machine can't confine the scan, but fails in + /// CI, where a skip would hide that the confinement never ran. + fn confinement_available() -> bool { + match unavailable_reason() { + None => true, + Some(reason) if std::env::var_os("CI").is_some() => { + panic!("scan confinement must run in CI: {reason}") + } + Some(reason) => { + eprintln!("skipping: {reason}"); + false + } + } + } + /// `cat` stands in for the CLI binary: the confinement is about which /// files the process can read, not what it does with them. fn assert_reads_only_the_worktree(base: &Path) { @@ -371,8 +386,7 @@ mod tests { #[test] fn confined_scan_cannot_read_outside_the_worktree_under_home() { - if let Some(reason) = unavailable_reason() { - eprintln!("skipping: {reason}"); + if !confinement_available() { return; } if let Some(home) = std::env::var_os("HOME") { @@ -382,8 +396,7 @@ mod tests { #[test] fn confined_scan_cannot_read_outside_the_worktree_under_tmp() { - if let Some(reason) = unavailable_reason() { - eprintln!("skipping: {reason}"); + if !confinement_available() { return; } assert_reads_only_the_worktree(Path::new("/tmp")); @@ -392,8 +405,7 @@ mod tests { #[test] fn confined_scan_keeps_profile_deny_read_inside_the_worktree() { - if let Some(reason) = unavailable_reason() { - eprintln!("skipping: {reason}"); + if !confinement_available() { return; } let Some(layout) = layout(&std::env::temp_dir()) else { diff --git a/src/utils/scans.rs b/src/utils/scans.rs index 4fd8d2b9..e6e96eb1 100644 --- a/src/utils/scans.rs +++ b/src/utils/scans.rs @@ -717,13 +717,8 @@ mod restricted_budget_tests { use super::*; fn temp_repo() -> (std::path::PathBuf, git2::Repository) { - let dir = std::env::temp_dir().join(format!( - "stashbase-scan-budget-{}", - SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_nanos() - )); + let dir = + std::env::temp_dir().join(format!("stashbase-scan-budget-{}", uuid::Uuid::new_v4())); let repo = git2::Repository::init(&dir).unwrap(); (dir, repo) } From 9396583c28faeb9fe31ee1f3d5cef24e2c8e0858 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 11:15:38 +0200 Subject: [PATCH 21/22] style: format scan sandbox changes --- src/handlers/run/proxy.rs | 5 +--- src/handlers/run/scan_sandbox.rs | 39 +++++++++++++++++++------------- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/handlers/run/proxy.rs b/src/handlers/run/proxy.rs index 8318ace9..0371273b 100644 --- a/src/handlers/run/proxy.rs +++ b/src/handlers/run/proxy.rs @@ -5624,10 +5624,7 @@ mod tests { assert!(body.contains(&format!("cwd={}", dir.display())), "{body}"); assert!(body.contains("key=parent-api-key"), "{body}"); assert!( - body.contains(&format!( - "api_url={}", - crate::api::client::get_api_url() - )), + body.contains(&format!("api_url={}", crate::api::client::get_api_url())), "{body}" ); assert!(body.contains("restricted=1"), "{body}"); diff --git a/src/handlers/run/scan_sandbox.rs b/src/handlers/run/scan_sandbox.rs index 62c2ca0d..1db5fedb 100644 --- a/src/handlers/run/scan_sandbox.rs +++ b/src/handlers/run/scan_sandbox.rs @@ -48,10 +48,7 @@ impl ScanConfinement { platform::wrap(self, args, home) } - #[cfg_attr( - not(any(target_os = "macos", target_os = "linux")), - allow(dead_code) - )] + #[cfg_attr(not(any(target_os = "macos", target_os = "linux")), allow(dead_code))] fn readable(&self, home: &Path) -> Vec { let mut paths = vec![self.workdir.clone(), self.exe.clone(), canonical(home)]; paths.extend(self.git_dirs.iter().cloned()); @@ -252,18 +249,20 @@ mod platform { } } for file in SYSTEM_FILES { - args.extend(["--ro-bind-try".to_owned(), (*file).to_owned(), (*file).to_owned()]); + args.extend([ + "--ro-bind-try".to_owned(), + (*file).to_owned(), + (*file).to_owned(), + ]); } let home = super::canonical(home); for path in confinement.readable(&home) { let mode = if path == home { "--bind" } else { "--ro-bind" }; args.extend([mode.to_owned(), text(&path), text(&path)]); } - for path in fs_rules::expand_policy_entries( - &confinement.denied_read, - &confinement.workdir, - None, - )? { + for path in + fs_rules::expand_policy_entries(&confinement.denied_read, &confinement.workdir, None)? + { if PathBuf::from(&path).is_dir() { args.extend(["--tmpfs".to_owned(), path]); } else { @@ -312,7 +311,10 @@ mod tests { /// A repo under `base`, with a secret next to it (outside the worktree) /// and a symlink from the worktree to that secret. fn layout(base: &Path) -> Option { - let root = base.join(format!(".stashbase-scan-sandbox-test-{}", uuid::Uuid::new_v4())); + let root = base.join(format!( + ".stashbase-scan-sandbox-test-{}", + uuid::Uuid::new_v4() + )); let workdir = root.join("repo"); let home = std::env::temp_dir().join(format!("stashbase-scan-home-{}", uuid::Uuid::new_v4())); @@ -332,9 +334,7 @@ mod tests { } fn run_cat(confinement: &ScanConfinement, home: &Path, file: &Path) -> std::process::Output { - let (program, args) = confinement - .wrap(&[&file.to_string_lossy()], home) - .unwrap(); + let (program, args) = confinement.wrap(&[&file.to_string_lossy()], home).unwrap(); std::process::Command::new(program) .args(args) .current_dir(&confinement.workdir) @@ -361,13 +361,20 @@ mod tests { /// files the process can read, not what it does with them. fn assert_reads_only_the_worktree(base: &Path) { let Some(layout) = layout(base) else { - eprintln!("skipping: cannot create a test layout under {}", base.display()); + eprintln!( + "skipping: cannot create a test layout under {}", + base.display() + ); return; }; let cat = canonical(Path::new("/bin/cat")); let confinement = ScanConfinement::for_run(&layout.workdir, &cat, &[]); - let inside = run_cat(&confinement, &layout.home, &layout.workdir.join("inside.txt")); + let inside = run_cat( + &confinement, + &layout.home, + &layout.workdir.join("inside.txt"), + ); let via_link = run_cat(&confinement, &layout.home, &layout.workdir.join("link.txt")); let direct = run_cat(&confinement, &layout.home, &layout.outside); From b9f92c3210c4299e41f83a2154c69ef81b94b073 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radim=20H=C3=B6fer?= Date: Fri, 9 Oct 2026 11:23:45 +0200 Subject: [PATCH 22/22] ci: allow bubblewrap user namespaces --- .github/workflows/ci.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 707e7919..c2be4da3 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -29,7 +29,10 @@ jobs: # The secret-scan confinement tests skip without a working bubblewrap. - name: Install bubblewrap if: runner.os == 'Linux' - run: sudo apt-get update && sudo apt-get install -y bubblewrap + run: | + sudo apt-get update + sudo apt-get install -y bubblewrap apparmor-profiles + sudo apparmor_parser -r /usr/share/apparmor/extra-profiles/bwrap-userns-restrict - name: Run tests run: cargo test --locked