diff --git a/CLAUDE.md b/CLAUDE.md index 752b412..d2b4cd6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -344,10 +344,13 @@ docker exec stdout → tokio task → emit("terminal-output-{sessionId}") → li under a supervisor loop. **Its contract is Docker parity**: an `ExecSpec` must behave through the agent exactly as through `docker exec`. Any change to exec semantics on either side is checked by `host::agent::live_tests::the_agent_answers_exactly_as_docker_exec_does` (ignored; needs an image - with the agent — see the crate README). The token never stays in the environment: the entrypoint - moves it to a root-only file before anything runs, and the agent refuses a token file others can - read. The agent is started from an env/cwd snapshot taken at the top of the entrypoint so execs - see the container's configured env, not the script's. + with the agent — see the crate README). The token never stays in the environment and never touches the + writable layer: the entrypoint moves it to a root-only file on `/dev/shm` (tmpfs — a file in the + writable layer would ride `docker commit` into the snapshot image) before anything runs, and the + agent refuses a token file others can read. The agent is started from an env/cwd snapshot taken + at the top of the entrypoint and held **in memory only** (it contains every API key in the env), + so execs see the container's configured env, not the script's. The app treats the agent as + untrusted: no redirects, capped bodies, timeouts on every small call (`host/agent.rs`). **`/home/claude` in the image is seed-only.** It is the mount point of the named volume `triple-c-home-{projectId}`, so after a project's *first* start the image's copy of that directory diff --git a/app/src-tauri/src/host/agent.rs b/app/src-tauri/src/host/agent.rs index 0c419a9..a8f7001 100644 --- a/app/src-tauri/src/host/agent.rs +++ b/app/src-tauri/src/host/agent.rs @@ -29,6 +29,7 @@ use serde::Deserialize; use tokio::io::AsyncWrite; use tokio_tungstenite::tungstenite::client::IntoClientRequest; use tokio_tungstenite::tungstenite::http::HeaderValue; +use crate::host::OUTPUT_LIMIT_MARKER; use tokio_tungstenite::tungstenite::Message; use tokio_util::sync::PollSender; @@ -45,6 +46,20 @@ use crate::docker::exec::{ const CONNECT_TIMEOUT: Duration = Duration::from_secs(5); /// `is_running` is a liveness probe; it answers quickly or says "no". const HEALTH_TIMEOUT: Duration = Duration::from_secs(3); +/// Whole-request ceiling for the small calls (detached start, exec status, +/// resize) and for reading any small body. The agent is not trusted: one that +/// accepts a connection and then says nothing must not hang the app. One-shot +/// execs, attach and archive have *no* overall timeout — commands can +/// legitimately run long and send nothing until they finish, as with Docker. +#[cfg(not(test))] +const SMALL_REQUEST_TIMEOUT: Duration = Duration::from_secs(15); +/// Short in tests, so the hostile-agent tests finish quickly. +#[cfg(test)] +const SMALL_REQUEST_TIMEOUT: Duration = Duration::from_secs(1); +/// Largest WebSocket message either side may send; the agent enforces the same +/// (its output frames are at most 32 KiB). Stdin is split to fit. +const WS_MAX_MESSAGE: usize = 1024 * 1024; +const STDIN_FRAME_MAX: usize = 64 * 1024; /// How long an attach waits for the agent's `started` reply after the /// handshake. Spawning a process is fast; this only bounds a wedged agent. const ATTACH_START_TIMEOUT: Duration = Duration::from_secs(30); @@ -86,9 +101,15 @@ impl AgentEndpoint { } base_url.set_query(None); base_url.set_fragment(None); + let token = token.into(); + // It goes in an HTTP header; refuse anything that cannot, here, once, + // rather than failing (or silently degrading) on every request. + if token.is_empty() || !token.bytes().all(|b| b.is_ascii_graphic()) { + return Err("An agent token must be non-empty printable ASCII with no spaces".to_string()); + } Ok(Self { base_url, - token: token.into(), + token, }) } @@ -114,6 +135,15 @@ impl AgentEndpoint { fn bearer(&self) -> String { format!("Bearer {}", self.token) } + + /// The `Authorization` header, marked sensitive so it is never logged or + /// shown by `Debug` (reqwest/hyper honour the flag). + fn auth(&self) -> HeaderValue { + let mut v = HeaderValue::from_str(&self.bearer()) + .unwrap_or_else(|_| HeaderValue::from_static("Bearer invalid-token")); + v.set_sensitive(true); + v + } } // ── HTTP plumbing ──────────────────────────────────────────────────────────── @@ -152,7 +182,10 @@ const ERROR_TEXT_CAP: usize = 4 * 1024; /// Parse a small JSON body, reading at most [`SMALL_BODY_CAP`] bytes. async fn small_json(resp: reqwest::Response) -> Option { - let raw = read_capped(resp, SMALL_BODY_CAP).await.ok()??; + let raw = tokio::time::timeout(SMALL_REQUEST_TIMEOUT, read_capped(resp, SMALL_BODY_CAP)) + .await + .ok()? + .ok()??; serde_json::from_slice(&raw).ok() } @@ -254,16 +287,19 @@ async fn exec_oneshot_raw( CONTEXT, http() .post(ep.url("v1/exec")) - .header(reqwest::header::AUTHORIZATION, ep.bearer()) + .header(reqwest::header::AUTHORIZATION, ep.auth()) .json(&body), ) .await?; let status = resp.status(); if status == reqwest::StatusCode::PAYLOAD_TOO_LARGE { - // Verbatim: it starts with OUTPUT_LIMIT_MARKER, which callers match. + // Verbatim when it is the limit message (callers match on + // OUTPUT_LIMIT_MARKER); anything else the agent puts in a 413 is not + // trusted to speak for it. return Err(error_text(resp) .await + .filter(|e| e.starts_with(OUTPUT_LIMIT_MARKER)) .unwrap_or_else(|| output_limit_error(limit))); } if !status.is_success() { @@ -350,7 +386,8 @@ pub async fn exec_detached(ep: &AgentEndpoint, spec: &ExecSpec) -> Result Option { /// unknown id, a finished exec with no code, or the budget running out — /// exactly where `docker::exec::wait_for_exec_exit` says `None`. pub async fn wait_exec_exit(ep: &AgentEndpoint, exec_id: &str) -> Option { + // Docker's budget is a number of polls; bound the wall clock too, so a + // slow-answering agent cannot stretch it (each poll is also bounded). + let budget = EXEC_EXIT_POLL_INTERVAL * EXEC_EXIT_POLLS as u32 + SMALL_REQUEST_TIMEOUT; + tokio::time::timeout(budget, wait_exec_exit_polls(ep, exec_id)).await.ok()? +} + +async fn wait_exec_exit_polls(ep: &AgentEndpoint, exec_id: &str) -> Option { if let Some(code) = recorded_exit(ep.base_url.as_str(), exec_id) { return Some(code); } @@ -423,7 +467,8 @@ pub async fn wait_exec_exit(ep: &AgentEndpoint, exec_id: &str) -> Option { } let resp = http() .get(url.clone()) - .header(reqwest::header::AUTHORIZATION, ep.bearer()) + .timeout(SMALL_REQUEST_TIMEOUT) + .header(reqwest::header::AUTHORIZATION, ep.auth()) .send() .await .ok()?; @@ -445,7 +490,8 @@ pub async fn resize_exec(ep: &AgentEndpoint, exec_id: &str, cols: u16, rows: u16 CONTEXT, http() .post(ep.url(&format!("v1/exec/{}/resize", urlencode_segment(exec_id)))) - .header(reqwest::header::AUTHORIZATION, ep.bearer()) + .timeout(SMALL_REQUEST_TIMEOUT) + .header(reqwest::header::AUTHORIZATION, ep.auth()) .json(&serde_json::json!({ "cols": cols, "rows": rows })), ) .await?; @@ -490,11 +536,18 @@ async fn connect_ws(ep: &AgentEndpoint, path: &str, context: &str) -> Result Err(format!("{}: timed out connecting to the agent", context)), Ok(Err(tokio_tungstenite::tungstenite::Error::Http(resp))) if resp.status() == tokio_tungstenite::tungstenite::http::StatusCode::UNAUTHORIZED => @@ -657,10 +710,14 @@ impl AsyncWrite for AttachInput { Poll::Pending => Poll::Pending, Poll::Ready(Err(_)) => Poll::Ready(Err(broken_pipe())), Poll::Ready(Ok(())) => { + // One frame per write, at most STDIN_FRAME_MAX; `write_all` + // loops for the rest. Keeps every frame under the agent's + // WebSocket message limit. + let n = buf.len().min(STDIN_FRAME_MAX); this.tx - .send_item(StdinCmd::Data(Bytes::copy_from_slice(buf))) + .send_item(StdinCmd::Data(Bytes::copy_from_slice(&buf[..n]))) .map_err(|_| broken_pipe())?; - Poll::Ready(Ok(buf.len())) + Poll::Ready(Ok(n)) } } } @@ -742,11 +799,14 @@ async fn put_file( .append_pair("mode", &format!("{:o}", mode)) .append_pair("uid", &uid.to_string()) .append_pair("gid", &gid.to_string()); + // Generous, but finite: a minute plus 256 KiB/s for the payload. + let timeout = Duration::from_secs(60 + (data.len() as u64) / (256 * 1024)); let resp = send( CONTEXT, http() .put(url) - .header(reqwest::header::AUTHORIZATION, ep.bearer()) + .timeout(timeout) + .header(reqwest::header::AUTHORIZATION, ep.auth()) .header(reqwest::header::CONTENT_TYPE, "application/octet-stream") .body(data), ) @@ -795,7 +855,7 @@ pub fn download_archive(ep: &AgentEndpoint, path: &str) -> ArchiveStream { url.query_pairs_mut().append_pair("path", path); let req = http() .get(url) - .header(reqwest::header::AUTHORIZATION, ep.bearer()); + .header(reqwest::header::AUTHORIZATION, ep.auth()); enum State { Start(reqwest::RequestBuilder), @@ -835,7 +895,7 @@ pub fn download_archive(ep: &AgentEndpoint, path: &str) -> ArchiveStream { pub async fn is_running(ep: &AgentEndpoint) -> Result { let resp = match http() .get(ep.url("v1/health")) - .header(reqwest::header::AUTHORIZATION, ep.bearer()) + .header(reqwest::header::AUTHORIZATION, ep.auth()) .timeout(HEALTH_TIMEOUT) .send() .await diff --git a/app/src-tauri/src/host/agent/tests.rs b/app/src-tauri/src/host/agent/tests.rs index 3c09c3d..067a52d 100644 --- a/app/src-tauri/src/host/agent/tests.rs +++ b/app/src-tauri/src/host/agent/tests.rs @@ -628,3 +628,46 @@ async fn an_oversized_error_body_is_neither_buffered_nor_passed_through_whole() assert!(e.len() <= 4 * 1024 + 64, "error was {} bytes", e.len()); assert!(e.ends_with('…'), "{e}"); } + +#[tokio::test] +async fn an_agent_that_accepts_and_then_says_nothing_cannot_hang_the_small_calls() { + // Accepts every connection, reads nothing, answers nothing, holds it open. + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + tokio::spawn(async move { + let mut held = Vec::new(); + while let Ok((sock, _)) = listener.accept().await { + held.push(sock); + } + }); + let t = ContainerTarget::Agent(AgentEndpoint::loopback(port, TOKEN)); + let started = std::time::Instant::now(); + assert!(t.exec_detached(ExecSpec::new(cmd("true"))).await.is_err()); + assert!(t.resize_exec("abc", 80, 24).await.is_err()); + assert_eq!(t.wait_exec_exit("abc").await, None); + // Three calls, each bounded by SMALL_REQUEST_TIMEOUT (1 s under test). + assert!(started.elapsed() < std::time::Duration::from_secs(10), "took {:?}", started.elapsed()); +} + +#[test] +fn a_token_that_cannot_be_a_header_value_is_refused_up_front() { + let url = url::Url::parse("http://127.0.0.1:1/").unwrap(); + for bad in ["", "has space", "new\nline", "tab\there", "ünïcode"] { + assert!(AgentEndpoint::new(url.clone(), bad).is_err(), "{bad:?} accepted"); + } + assert!(AgentEndpoint::new(url, TOKEN).is_ok()); +} + +#[tokio::test] +async fn a_413_that_is_not_the_limit_message_is_not_passed_through() { + let port = serve(Router::new().fallback(|| async { + (StatusCode::PAYLOAD_TOO_LARGE, Json(json!({ "error": "click here: http://evil.example" }))) + })) + .await; + let e = ContainerTarget::Agent(AgentEndpoint::loopback(port, TOKEN)) + .exec_oneshot_limited("claude", cmd("true"), 1000) + .await + .unwrap_err(); + assert!(e.starts_with(OUTPUT_LIMIT_MARKER), "{e}"); + assert!(!e.contains("evil"), "{e}"); +} diff --git a/container/entrypoint.sh b/container/entrypoint.sh index becf08f..7cbe10c 100644 --- a/container/entrypoint.sh +++ b/container/entrypoint.sh @@ -31,19 +31,29 @@ # the snapshot (`env -i`). The agent drops its own TRIPLEC_AGENT_* settings. # >>> agent_token agent_cwd=$PWD +# The token goes on a tmpfs, never the container's writable layer: a file +# there would persist after stop and ride `docker commit` into the project's +# snapshot image (the commit's SECRET_ENV_KEYS scrub covers env, not files). +# /dev/shm is a tmpfs in Docker and on Fargate; anything else is refused. if [ -n "${TRIPLEC_AGENT_TOKEN:-}" ]; then - mkdir -p /run/triple-c - chmod 700 /run/triple-c - ( umask 077; printf '%s' "$TRIPLEC_AGENT_TOKEN" > /run/triple-c/agent-token ) + if [ "$(stat -f -c %T /dev/shm 2>/dev/null)" = tmpfs ]; then + mkdir -p /dev/shm/triple-c + chown root:root /dev/shm/triple-c + chmod 700 /dev/shm/triple-c + ( umask 077; printf '%s' "$TRIPLEC_AGENT_TOKEN" > /dev/shm/triple-c/agent-token ) + export TRIPLEC_AGENT_TOKEN_FILE=/dev/shm/triple-c/agent-token + else + echo "triple-c-agent: /dev/shm is not a tmpfs; not writing the token to disk — agent disabled" >&2 + fi unset TRIPLEC_AGENT_TOKEN - export TRIPLEC_AGENT_TOKEN_FILE=/run/triple-c/agent-token fi +# The exec environment snapshot stays in this script's memory — it holds the +# container's whole configured env, API keys included, so it must not touch +# disk either. Minus what bash itself exports (PWD, OLDPWD, SHLVL, `_`): +# those are this script's, not the container's, and `docker exec` has none. +agent_env=() if [ -n "${TRIPLEC_AGENT_TOKEN_FILE:-}" ]; then - mkdir -p /run/triple-c - chmod 700 /run/triple-c - # Minus what bash itself exports (PWD, OLDPWD, SHLVL, `_`): those are this - # script's, not the container's, and `docker exec` has none of them. - ( umask 077; env -u PWD -u OLDPWD -u SHLVL -u _ -0 > /run/triple-c/agent-env ) + mapfile -d '' agent_env < <(env -u PWD -u OLDPWD -u SHLVL -u _ -0) fi # <<< agent_token @@ -817,9 +827,11 @@ start_agent() { echo "triple-c-agent: token given but no agent binary in this image — not starting it" >&2 return 0 fi - local -a agent_env=() - [ -r /run/triple-c/agent-env ] && mapfile -d '' agent_env < /run/triple-c/agent-env ( + # Keeps the supervisor alive through a stray HUP. The agent resets + # every signal disposition and the mask in each exec it spawns, so this + # does not leak into commands (an ignored SIGHUP otherwise survives + # exec and turns every exec into `nohup`). trap '' HUP cd "$agent_cwd" || cd / while :; do