From 0518bd4c83d51b77692f8386eef4a4a581806632 Mon Sep 17 00:00:00 2001 From: Drew Newberry Date: Thu, 24 Sep 2026 17:22:30 +0000 Subject: [PATCH] fix(sandbox): preserve local sessions across host sleep (#3573) * fix(cli): recover sandbox connect transport Signed-off-by: Drew Newberry * fix(auth): propagate non-expiring local sandbox sessions Signed-off-by: Drew Newberry * fix(cli): distinguish main exit from transport loss Signed-off-by: Drew Newberry * fix(cli): bound sandbox connect recovery Signed-off-by: Drew Newberry --------- Signed-off-by: Drew Newberry --- architecture/gateway.md | 8 +- architecture/sandbox.md | 12 + crates/openshell-cli/src/ssh.rs | 502 +++++++++++++++++- crates/openshell-core/src/config.rs | 14 +- crates/openshell-core/src/grpc_client.rs | 24 +- crates/openshell-core/src/jwt.rs | 102 +++- .../src/sandbox_auth.rs | 2 +- .../openshell-sandbox/src/boundary_server.rs | 34 +- .../openshell-server/src/auth/sandbox_jwt.rs | 2 +- crates/openshell-server/src/compute/mod.rs | 2 +- crates/openshell-server/src/grpc/auth_rpc.rs | 43 +- crates/openshell-server/src/lib.rs | 2 +- crates/openshell-server/src/multiplex.rs | 2 +- docs/reference/gateway-config.mdx | 5 +- docs/sandboxes/manage-sandboxes.mdx | 11 +- e2e/rust/Cargo.toml | 2 +- e2e/rust/tests/sandbox_lifecycle.rs | 291 ++++++++++ proto/openshell.proto | 2 +- sdk/go/proto/openshellv1/openshell.pb.go | 2 +- skills/openshell-cli/SKILL.md | 15 +- tasks/scripts/gateway-podman.sh | 1 - 21 files changed, 986 insertions(+), 92 deletions(-) diff --git a/architecture/gateway.md b/architecture/gateway.md index 689bd4b49..97ba7c506 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -358,8 +358,12 @@ can recover that same successor for 30 seconds when the request matches, but it cannot authorize ordinary RPCs or choose another successor. Advancing the successor removes that retry path across every gateway replica. Short `gateway_jwt.ttl_secs` lifetimes still bound the exposure of a current bearer -that has not yet been refreshed. Omitting `gateway_jwt.ttl_secs` uses a -900-second lifetime. Explicit zero is rejected. +that has not yet been refreshed. Omitting `gateway_jwt.ttl_secs` selects +non-expiring launch-scoped gateway and Sandbox Protocol tokens for local +single-player Docker, Podman, and VM gateways; both token profiles carry +`exp = 0`. Typed extension JWTs retain a 900-second default when the field is +omitted. Kubernetes and other shared deployments should set a positive TTL. +Explicit zero is rejected. Gateway JWT signing-key rotation is currently an offline operator action. The runtime loads one active signing key and one matching public verification key diff --git a/architecture/sandbox.md b/architecture/sandbox.md index c2de9a1d6..a692bf07d 100644 --- a/architecture/sandbox.md +++ b/architecture/sandbox.md @@ -55,6 +55,12 @@ TCP mediation accepts use the same authenticated transport recovery as process w A renewed Sandbox Protocol bearer is authenticated even when its credential epoch is unchanged. The supervisor confirms that bearer on the active physical connection and records its fingerprint only after confirmation succeeds, preserving pending streams and the mediation session. Changing the credential epoch still requires an authenticated replacement connection. +Local single-player Docker, Podman, and VM gateways propagate an omitted +`gateway_jwt.ttl_secs` value to both launch-scoped credential profiles. Those +credentials use `exp = 0`, so host suspension cannot strand the supervisor +after a refresh deadline passes. Shared deployments retain expiring credentials +and a durable compute-platform bootstrap identity. + Unauthenticated TLS handshakes have a separate bounded asynchronous pool and five-second deadline, never consuming authenticated control slots or threads. The socket broker reserves the TCP control-listener port against workload @@ -669,6 +675,12 @@ sandbox workload directly. The relay supports: buffer, and a single stdin lease across client disconnects. Ctrl-C interrupts the foreground process. For read-only attachments, Ctrl-C only exits the current viewer. +- Supervised CLI attachment. After an established SSH transport fails, the CLI + remains alive, requests a fresh SSH session from the gateway, and reattaches + to the same canonical main process within a bounded recovery window. It does + not stop or restart the sandbox to recover the client connection. The same + deadline bounds replacement-session RPCs. Process-targeted termination is + forwarded to the SSH child, which the CLI reaps before exiting. - Independent interactive shell sessions. - Command execution. Commands run through a login shell (`bash -lc`) by default, so the first of the user's `.bash_profile`, `.bash_login`, or `.profile` is diff --git a/crates/openshell-cli/src/ssh.rs b/crates/openshell-cli/src/ssh.rs index 947c84ced..204c8f636 100644 --- a/crates/openshell-cli/src/ssh.rs +++ b/crates/openshell-cli/src/ssh.rs @@ -14,8 +14,8 @@ use openshell_core::forward::{ validate_ssh_session_response, write_forward_pid, }; use openshell_core::proto::{ - CreateSshSessionRequest, GetSandboxRequest, SshRelayTarget, TcpForwardFrame, TcpForwardInit, - tcp_forward_init, + CreateSshSessionRequest, GetSandboxRequest, SandboxPhase, SshRelayTarget, TcpForwardFrame, + TcpForwardInit, tcp_forward_init, }; use std::fs; use std::future::Future; @@ -23,8 +23,8 @@ use std::io::{IsTerminal, Write}; #[cfg(unix)] use std::os::unix::process::CommandExt; use std::path::{Path, PathBuf}; -use std::process::{Command, Stdio}; -use std::time::Duration; +use std::process::{Command, ExitStatus, Stdio}; +use std::time::{Duration, Instant}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::TcpStream; use tokio::process::{Child, Command as TokioCommand}; @@ -44,6 +44,20 @@ const FORWARD_LISTENER_CONNECT_TIMEOUT: Duration = Duration::from_millis(200); /// command has already reported its terminal result. const TERMINAL_RELAY_REGISTRATION_TIMEOUT: Duration = Duration::from_secs(5); const TERMINAL_RELAY_REGISTRATION_INTERVAL: Duration = Duration::from_millis(50); +/// An SSH client that remained alive for this long was attached successfully, +/// rather than failing during initial authentication or setup. +const CONNECT_ESTABLISHED_DURATION: Duration = Duration::from_secs(2); +/// Briefly wait for the gateway lifecycle projection to catch up with an SSH +/// exit status before treating status 255 as a transport failure. +const CONNECT_TERMINAL_STATUS_TIMEOUT: Duration = Duration::from_millis(500); +const CONNECT_TERMINAL_STATUS_INTERVAL: Duration = Duration::from_millis(50); +/// Time allowed to restore an established canonical-main attachment after its +/// transport is lost. +const CONNECT_RECOVERY_TIMEOUT: Duration = Duration::from_mins(1); +const CONNECT_RETRY_INITIAL_DELAY: Duration = Duration::from_millis(250); +const CONNECT_RETRY_MAX_DELAY: Duration = Duration::from_secs(5); +const CONNECT_CHILD_TERMINATION_TIMEOUT: Duration = Duration::from_secs(5); +const SSH_TRANSPORT_FAILURE_EXIT_CODE: i32 = 255; const SYNC_RETRY_ATTEMPTS: usize = 4; const SYNC_RETRY_DELAY: Duration = Duration::from_secs(2); @@ -281,6 +295,377 @@ fn exec_or_wait(mut command: Command, replace_process: bool) -> Result { Ok(status.code().unwrap_or(1)) } +fn main_attach_command(session: &SshSessionConfig) -> Command { + let mut command = ssh_base_command(&session.proxy_command); + if session.main_terminal { + command.arg("-tt").arg("-o").arg("RequestTTY=force"); + } else { + command.arg("-T"); + } + command + .arg("-o") + .arg("SetEnv=TERM=xterm-256color") + .arg("-s") + .arg("sandbox") + .arg("openshell-main") + .stdin(Stdio::inherit()) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()); + command +} + +async fn run_main_attach(session: &SshSessionConfig, replace_process: bool) -> Result { + let command = main_attach_command(session); + tokio::task::spawn_blocking(move || exec_or_wait(command, replace_process)) + .await + .into_diagnostic()? +} + +fn process_exit_code(status: ExitStatus) -> i32 { + status.code().unwrap_or(1) +} + +#[cfg(unix)] +struct TerminationSignals { + interrupt: tokio::signal::unix::Signal, + quit: tokio::signal::unix::Signal, + terminate: tokio::signal::unix::Signal, +} + +#[cfg(unix)] +impl TerminationSignals { + fn new() -> Result { + use tokio::signal::unix::{SignalKind, signal}; + + Ok(Self { + interrupt: signal(SignalKind::interrupt()).into_diagnostic()?, + quit: signal(SignalKind::quit()).into_diagnostic()?, + terminate: signal(SignalKind::terminate()).into_diagnostic()?, + }) + } + + async fn recv(&mut self) -> Signal { + tokio::select! { + _ = self.interrupt.recv() => Signal::SIGINT, + _ = self.quit.recv() => Signal::SIGQUIT, + _ = self.terminate.recv() => Signal::SIGTERM, + } + } +} + +struct ConnectCancellation { + #[cfg(unix)] + signals: TerminationSignals, +} + +impl ConnectCancellation { + fn new() -> Result { + Ok(Self { + #[cfg(unix)] + signals: TerminationSignals::new()?, + }) + } + + async fn wait(&mut self, future: F) -> std::result::Result + where + F: Future, + { + #[cfg(unix)] + { + tokio::select! { + biased; + result = future => Ok(result), + signal = self.signals.recv() => Err(128 + signal as i32), + } + } + + #[cfg(not(unix))] + { + Ok(future.await) + } + } +} + +#[cfg(unix)] +fn forward_signal_to_child(child: &Child, signal: Signal) -> Result<()> { + let Some(pid) = child.id() else { + return Ok(()); + }; + let pid = i32::try_from(pid).into_diagnostic()?; + match nix::sys::signal::kill(nix::unistd::Pid::from_raw(pid), signal) { + Ok(()) | Err(nix::errno::Errno::ESRCH) => Ok(()), + Err(error) => Err(error).into_diagnostic(), + } +} + +#[cfg(unix)] +async fn terminate_and_reap_child(child: &mut Child, signal: Signal) -> Result { + forward_signal_to_child(child, signal)?; + if let Ok(status) = tokio::time::timeout(CONNECT_CHILD_TERMINATION_TIMEOUT, child.wait()).await + { + status.into_diagnostic()?; + } else { + child.kill().await.into_diagnostic()?; + child.wait().await.into_diagnostic()?; + } + Ok(128 + signal as i32) +} + +async fn run_main_attach_supervised( + session: &SshSessionConfig, + cancellation: &mut ConnectCancellation, +) -> Result { + #[cfg(not(unix))] + let _ = cancellation; + + let mut command = TokioCommand::from(main_attach_command(session)); + command.kill_on_drop(true); + + let mut child = command.spawn().into_diagnostic()?; + + #[cfg(unix)] + { + tokio::select! { + biased; + status = child.wait() => Ok(process_exit_code(status.into_diagnostic()?)), + signal = cancellation.signals.recv() => terminate_and_reap_child(&mut child, signal).await, + } + } + + #[cfg(not(unix))] + { + Ok(process_exit_code(child.wait().await.into_diagnostic()?)) + } +} + +async fn await_before_recovery_deadline( + deadline: Option, + future: F, +) -> std::result::Result +where + F: Future, +{ + let Some(deadline) = deadline else { + return Ok(future.await); + }; + tokio::time::timeout_at(tokio::time::Instant::from_std(deadline), future) + .await + .map_err(|_| ()) +} + +fn print_connect_recovery_timeout() { + eprintln!( + "Unable to restore the sandbox connection within {} seconds.", + CONNECT_RECOVERY_TIMEOUT.as_secs() + ); +} + +fn should_recover_connect( + exit_code: i32, + attached_for: Duration, + recovery_active: bool, + main_has_terminal_result: bool, +) -> bool { + !main_has_terminal_result + && exit_code == SSH_TRANSPORT_FAILURE_EXIT_CODE + && (recovery_active || attached_for >= CONNECT_ESTABLISHED_DURATION) +} + +async fn canonical_main_has_terminal_result( + server: &str, + name: &str, + tls: &TlsOptions, + workspace: &str, +) -> bool { + let probe = async { + let Ok(mut client) = grpc_client(server, tls).await else { + return false; + }; + loop { + let Ok(response) = client + .get_sandbox(GetSandboxRequest { + name: name.to_string(), + workspace_scope: Some(openshell_core::proto::workspace_selector( + workspace.to_string(), + )), + }) + .await + else { + return false; + }; + let Some(sandbox) = response.into_inner().sandbox else { + return false; + }; + let phase = SandboxPhase::try_from(sandbox.phase()).unwrap_or(SandboxPhase::Unknown); + if sandbox + .status + .as_ref() + .is_some_and(|status| status.exit_code.is_some()) + || matches!( + phase, + SandboxPhase::Completed + | SandboxPhase::Error + | SandboxPhase::Stopping + | SandboxPhase::Stopped + | SandboxPhase::Deleting + ) + { + return true; + } + tokio::time::sleep(CONNECT_TERMINAL_STATUS_INTERVAL).await; + } + }; + + tokio::time::timeout(CONNECT_TERMINAL_STATUS_TIMEOUT, probe) + .await + .unwrap_or(false) +} + +fn should_start_connect_recovery_window(attached_for: Duration, recovery_active: bool) -> bool { + !recovery_active || attached_for >= CONNECT_RECOVERY_TIMEOUT +} + +fn next_connect_retry_delay(delay: Duration) -> Duration { + std::cmp::min(delay.saturating_mul(2), CONNECT_RETRY_MAX_DELAY) +} + +fn connect_error_is_retryable(error: &Report) -> bool { + let message = format!("{error:?}").to_ascii_lowercase(); + [ + "broken pipe", + "connection aborted", + "connection closed", + "connection refused", + "connection reset", + "deadline exceeded", + "h2 protocol", + "http2", + "reset before headers", + "service is currently unavailable", + "timed out", + "timeout", + "transport error", + "unexpected eof", + "unavailable", + "upstream connect error", + ] + .iter() + .any(|needle| message.contains(needle)) +} + +async fn sandbox_connect_supervised( + server: &str, + name: &str, + tls: &TlsOptions, + workspace: &str, +) -> Result { + let mut recovery_deadline = None; + let mut retry_delay = CONNECT_RETRY_INITIAL_DELAY; + let mut cancellation = ConnectCancellation::new()?; + + loop { + if recovery_deadline.is_some_and(|deadline| Instant::now() >= deadline) { + print_connect_recovery_timeout(); + return Ok(SSH_TRANSPORT_FAILURE_EXIT_CODE); + } + + let session_attempt = match Box::pin(cancellation.wait(await_before_recovery_deadline( + recovery_deadline, + ssh_session_config(server, name, tls, workspace, None), + ))) + .await + { + Ok(attempt) => attempt, + Err(exit_code) => return Ok(exit_code), + }; + let session = match session_attempt { + Err(()) => { + print_connect_recovery_timeout(); + return Ok(SSH_TRANSPORT_FAILURE_EXIT_CODE); + } + Ok(Ok(session)) => session, + Ok(Err(error)) + if recovery_deadline.is_some_and(|deadline| Instant::now() < deadline) + && connect_error_is_retryable(&error) => + { + tracing::warn!( + sandbox = name, + error = %error, + "failed to create replacement SSH session; retrying" + ); + let now = Instant::now(); + let delay = recovery_deadline + .map_or(retry_delay, |deadline| retry_delay.min(deadline - now)); + if let Err(exit_code) = cancellation.wait(tokio::time::sleep(delay)).await { + return Ok(exit_code); + } + retry_delay = next_connect_retry_delay(retry_delay); + continue; + } + Ok(Err(error)) => return Err(error), + }; + + let attach_started = Instant::now(); + let exit_code = run_main_attach_supervised(&session, &mut cancellation).await?; + let attached_for = attach_started.elapsed(); + let recovery_active = recovery_deadline.is_some(); + let main_has_terminal_result = if exit_code == SSH_TRANSPORT_FAILURE_EXIT_CODE { + match cancellation + .wait(canonical_main_has_terminal_result( + server, name, tls, workspace, + )) + .await + { + Ok(has_result) => has_result, + Err(exit_code) => return Ok(exit_code), + } + } else { + false + }; + + if !should_recover_connect( + exit_code, + attached_for, + recovery_active, + main_has_terminal_result, + ) { + return Ok(exit_code); + } + + // Once a replacement attachment survives the full recovery interval, + // treat a later failure as a new interruption. Keeping the existing + // deadline for shorter attempts prevents SSH connect timeouts from + // extending recovery forever. + if should_start_connect_recovery_window(attached_for, recovery_active) { + recovery_deadline = Some(Instant::now() + CONNECT_RECOVERY_TIMEOUT); + retry_delay = CONNECT_RETRY_INITIAL_DELAY; + eprintln!( + "Connection to sandbox lost; reconnecting. To disconnect, press Ctrl-P then Ctrl-Q after reattachment; press Ctrl-C while retrying to cancel." + ); + } + + let Some(deadline) = recovery_deadline else { + return Ok(exit_code); + }; + let now = Instant::now(); + if now >= deadline { + print_connect_recovery_timeout(); + return Ok(exit_code); + } + + if let Err(exit_code) = cancellation + .wait(tokio::time::sleep(std::cmp::min( + retry_delay, + deadline - now, + ))) + .await + { + return Ok(exit_code); + } + retry_delay = next_connect_retry_delay(retry_delay); + } +} + async fn sandbox_connect_with_mode( server: &str, name: &str, @@ -298,27 +683,7 @@ async fn sandbox_connect_with_mode( ) .await?; - let mut command = ssh_base_command(&session.proxy_command); - if session.main_terminal { - command.arg("-tt").arg("-o").arg("RequestTTY=force"); - } else { - command.arg("-T"); - } - command - .arg("-o") - .arg("SetEnv=TERM=xterm-256color") - .arg("-s") - .arg("sandbox") - .arg("openshell-main") - .stdin(Stdio::inherit()) - .stdout(Stdio::inherit()) - .stderr(Stdio::inherit()); - - let exit_code = tokio::task::spawn_blocking(move || exec_or_wait(command, replace_process)) - .await - .into_diagnostic()??; - - Ok(exit_code) + run_main_attach(&session, replace_process).await } /// Connect to a sandbox via SSH. @@ -328,7 +693,7 @@ pub async fn sandbox_connect( tls: &TlsOptions, workspace: &str, ) -> Result { - sandbox_connect_with_mode(server, name, tls, true, workspace, None).await + sandbox_connect_supervised(server, name, tls, workspace).await } pub(crate) async fn sandbox_connect_without_exec( @@ -1868,6 +2233,91 @@ mod tests { assert!(!sync_error_is_retryable(&error)); } + #[test] + fn connect_recovery_starts_only_for_established_transport_failures() { + assert!(should_recover_connect( + SSH_TRANSPORT_FAILURE_EXIT_CODE, + CONNECT_ESTABLISHED_DURATION, + false, + false + )); + assert!(!should_recover_connect( + SSH_TRANSPORT_FAILURE_EXIT_CODE, + CONNECT_ESTABLISHED_DURATION + .checked_sub(Duration::from_millis(1)) + .unwrap(), + false, + false + )); + assert!(!should_recover_connect( + 0, + CONNECT_ESTABLISHED_DURATION, + false, + false + )); + assert!(!should_recover_connect( + SSH_TRANSPORT_FAILURE_EXIT_CODE, + CONNECT_ESTABLISHED_DURATION, + false, + true + )); + } + + #[test] + fn connect_recovery_continues_after_fast_replacement_failure() { + assert!(should_recover_connect( + SSH_TRANSPORT_FAILURE_EXIT_CODE, + Duration::ZERO, + true, + false + )); + assert!(!should_start_connect_recovery_window( + CONNECT_ESTABLISHED_DURATION, + true + )); + assert!(should_start_connect_recovery_window( + CONNECT_RECOVERY_TIMEOUT, + true + )); + } + + #[test] + fn connect_retry_delay_is_capped() { + assert_eq!( + next_connect_retry_delay(CONNECT_RETRY_INITIAL_DELAY), + Duration::from_millis(500) + ); + assert_eq!( + next_connect_retry_delay(CONNECT_RETRY_MAX_DELAY), + CONNECT_RETRY_MAX_DELAY + ); + } + + #[test] + fn connect_retry_filter_rejects_authentication_failures() { + let transient = miette::miette!("transport error: connection reset by peer"); + assert!(connect_error_is_retryable(&transient)); + + let authentication = miette::miette!("status: Unauthenticated"); + assert!(!connect_error_is_retryable(&authentication)); + + let lifecycle = miette::miette!("sandbox is not ready"); + assert!(!connect_error_is_retryable(&lifecycle)); + } + + #[tokio::test] + async fn connect_recovery_deadline_bounds_stalled_session_creation() { + let deadline = Instant::now() + Duration::from_millis(20); + let result = await_before_recovery_deadline( + Some(deadline), + std::future::pending::>(), + ) + .await; + + assert!(result.is_err()); + assert!(Instant::now() >= deadline); + } + #[test] #[allow(unsafe_code)] // Test-only: env vars require unsafe in Rust 2024. fn install_ssh_config_adds_include_once_and_updates_managed_file() { diff --git a/crates/openshell-core/src/config.rs b/crates/openshell-core/src/config.rs index 1c844864a..0d4a9f432 100644 --- a/crates/openshell-core/src/config.rs +++ b/crates/openshell-core/src/config.rs @@ -800,19 +800,25 @@ pub struct GatewayJwtConfig { /// `openshell`. #[serde(default = "default_gateway_id")] pub gateway_id: String, - /// Token lifetime in seconds. Defaults to 15 minutes when omitted. + /// Token lifetime in seconds. Omission selects non-expiring sandbox + /// session credentials and the default lifetime for extension tokens. /// Explicit zero is invalid. #[serde(default, skip_serializing_if = "Option::is_none")] pub ttl_secs: Option, } impl GatewayJwtConfig { - /// Effective token lifetime. + /// Effective typed extension-token lifetime. pub fn token_ttl(&self) -> Duration { self.ttl_secs.map_or(Duration::from_mins(15), |ttl| { Duration::from_secs(ttl.get()) }) } + + /// Effective sandbox session-token lifetime. `None` is non-expiring. + pub fn sandbox_token_ttl(&self) -> Option { + self.ttl_secs.map(|ttl| Duration::from_secs(ttl.get())) + } } fn default_gateway_id() -> String { @@ -1215,7 +1221,7 @@ mod tests { } #[test] - fn gateway_jwt_ttl_defaults_to_fifteen_minutes() { + fn gateway_jwt_omitted_ttl_defaults_extension_and_nonexpiring_session_tokens() { let cfg: GatewayJwtConfig = serde_json::from_value(serde_json::json!({ "signing_key_path": "/tmp/signing.pem", "public_key_path": "/tmp/public.pem", @@ -1225,6 +1231,7 @@ mod tests { assert_eq!(cfg.ttl_secs, None); assert_eq!(cfg.token_ttl(), Duration::from_mins(15)); + assert_eq!(cfg.sandbox_token_ttl(), None); let serialized = serde_json::to_value(&cfg).expect("gateway JWT config serializes"); assert!(serialized.get("ttl_secs").is_none()); @@ -1241,6 +1248,7 @@ mod tests { .expect("gateway JWT config should deserialize with positive ttl"); assert_eq!(cfg.token_ttl(), Duration::from_hours(1)); + assert_eq!(cfg.sandbox_token_ttl(), Some(Duration::from_hours(1))); let serialized = serde_json::to_value(&cfg).expect("gateway JWT config serializes"); assert_eq!(serialized["ttl_secs"], 3600); } diff --git a/crates/openshell-core/src/grpc_client.rs b/crates/openshell-core/src/grpc_client.rs index cbd6de567..46b2b0dd5 100644 --- a/crates/openshell-core/src/grpc_client.rs +++ b/crates/openshell-core/src/grpc_client.rs @@ -133,13 +133,19 @@ fn validate_sandbox_refresh( ) -> std::result::Result { let token = crate::jwt::SecretJwt::parse(response.sandbox_token.clone())?; let credential_epoch = crate::jwt::CredentialEpoch::new(response.credential_epoch)?; - let expiration_time = response + let expires_at = response .sandbox_expiration_time .as_ref() - .ok_or(crate::jwt::SessionJwtError::InvalidLifetime)?; - crate::time::validate_timestamp(expiration_time) - .map_err(|_| crate::jwt::SessionJwtError::InvalidLifetime)?; - let expires_at = expiration_time.seconds; + .map(|expiration_time| { + crate::time::validate_timestamp(expiration_time) + .map_err(|_| crate::jwt::SessionJwtError::InvalidLifetime)?; + if expiration_time.seconds == 0 { + return Err(crate::jwt::SessionJwtError::InvalidLifetime); + } + Ok(expiration_time.seconds) + }) + .transpose()? + .unwrap_or(0); crate::jwt::SessionBearerTokenSlot::new(token.clone(), expires_at, credential_epoch)?; Ok(ValidatedSandboxRefresh { token, @@ -710,17 +716,15 @@ mod auth_tests { #[cfg(feature = "jwt")] #[test] - fn sandbox_refresh_validation_rejects_missing_expiration() { + fn sandbox_refresh_validation_accepts_missing_expiration_as_non_expiring() { let response = crate::proto::RefreshSandboxTokenResponse { sandbox_token: "sandbox-token".to_string(), credential_epoch: 2, ..Default::default() }; - assert_eq!( - validate_sandbox_refresh(&response).err(), - Some(crate::jwt::SessionJwtError::InvalidLifetime) - ); + let refresh = validate_sandbox_refresh(&response).expect("non-expiring refresh"); + assert_eq!(refresh.expires_at, 0); } #[cfg(feature = "jwt")] diff --git a/crates/openshell-core/src/jwt.rs b/crates/openshell-core/src/jwt.rs index 5cd8c6043..d1b8d2222 100644 --- a/crates/openshell-core/src/jwt.rs +++ b/crates/openshell-core/src/jwt.rs @@ -299,7 +299,7 @@ mod session { impl SupervisorAuthBundle { pub fn validate(&self) -> Result<(), SessionJwtError> { - if self.gateway_expires_at <= 0 || self.sandbox_expires_at <= 0 { + if self.gateway_expires_at < 0 || self.sandbox_expires_at < 0 { return Err(SessionJwtError::InvalidLifetime); } SandboxGenerationId::parse(self.runtime_generation.to_string()) @@ -385,7 +385,7 @@ mod session { expires_at: i64, credential_epoch: CredentialEpoch, ) -> Result<(), SessionJwtError> { - if expires_at <= 0 { + if expires_at < 0 { return Err(SessionJwtError::InvalidLifetime); } let mut stored = self @@ -439,7 +439,7 @@ mod session { .read() .unwrap_or_else(std::sync::PoisonError::into_inner); let stored = stored.as_ref().ok_or(SessionJwtError::TokenUnavailable)?; - if stored.expires_at <= SystemJwtClock.now_unix_seconds() { + if stored.expires_at > 0 && stored.expires_at <= SystemJwtClock.now_unix_seconds() { return Err(SessionJwtError::Expired); } format!("Bearer {}", stored.token.expose_secret()) @@ -487,7 +487,7 @@ mod session { encoding_key: EncodingKey, key_id: String, issuer: String, - ttl: Duration, + ttl: Option, clock: Arc, } @@ -507,11 +507,13 @@ mod session { signing_key_pem: &[u8], key_id: impl Into, gateway_id: &str, - ttl: Duration, + ttl: Option, clock: Arc, ) -> Result { install_crypto_provider(); - validate_ttl(ttl)?; + if let Some(ttl) = ttl { + validate_ttl(ttl)?; + } let key_id = validate_key_id(key_id.into())?; let gateway_id = validate_gateway_id(gateway_id)?; let encoding_key = EncodingKey::from_ed_pem(signing_key_pem) @@ -583,9 +585,9 @@ mod session { token_id: Uuid, issued_at: i64, ) -> Result { - let expires_at = issued_at.saturating_add( - i64::try_from(self.ttl.as_secs()).map_err(|_| SessionJwtError::InvalidLifetime)?, - ); + let expires_at = self.ttl.map_or(0, |ttl| { + issued_at.saturating_add(i64::try_from(ttl.as_secs()).unwrap_or(i64::MAX)) + }); let claims = SessionClaims { iss: self.issuer.clone(), sub: format!("{SANDBOX_SUBJECT_PREFIX}{}", identity.sandbox_id), @@ -718,20 +720,22 @@ mod session { SandboxGenerationId::parse(claims.runtime_generation.to_string()) .map_err(|_| SessionJwtError::InvalidRuntimeIdentity)?; let token_id = Uuid::parse_str(&claims.jti).map_err(|_| SessionJwtError::InvalidJti)?; - if claims.exp <= claims.iat { - return Err(SessionJwtError::InvalidLifetime); - } - let lifetime = claims.exp.saturating_sub(claims.iat); - if lifetime > i64::try_from(MAX_SESSION_TOKEN_TTL.as_secs()).unwrap_or(i64::MAX) { - return Err(SessionJwtError::InvalidLifetime); - } let now = self.clock.now_unix_seconds(); let leeway = i64::try_from(MAX_SESSION_CLOCK_LEEWAY.as_secs()).unwrap_or(30); if claims.iat > now.saturating_add(leeway) { return Err(SessionJwtError::IssuedInFuture); } - if claims.exp < now.saturating_sub(leeway) { - return Err(SessionJwtError::Expired); + if claims.exp != 0 { + if claims.exp <= claims.iat { + return Err(SessionJwtError::InvalidLifetime); + } + let lifetime = claims.exp.saturating_sub(claims.iat); + if lifetime > i64::try_from(MAX_SESSION_TOKEN_TTL.as_secs()).unwrap_or(i64::MAX) { + return Err(SessionJwtError::InvalidLifetime); + } + if claims.exp < now.saturating_sub(leeway) { + return Err(SessionJwtError::Expired); + } } Ok(AuthenticatedSandboxSession { sandbox_id: claims.sandbox_id, @@ -772,7 +776,7 @@ mod session { InvalidSigningKey, #[error("Ed25519 verification key is invalid")] InvalidVerificationKey, - #[error("session token lifetime must be between 60 and 3600 seconds")] + #[error("session token lifetime must be non-expiring or between 60 and 3600 seconds")] InvalidLifetime, #[error("session token profile does not match its claims")] ProfileMismatch, @@ -924,7 +928,7 @@ mod tests { key.serialize_pem().as_bytes(), "current", "test", - DEFAULT_SESSION_TOKEN_TTL, + Some(DEFAULT_SESSION_TOKEN_TTL), clock.clone(), ) .expect("issuer"); @@ -976,6 +980,64 @@ mod tests { ); } + #[test] + fn non_expiring_session_tokens_remain_usable() { + let key = KeyPair::generate_for(&PKCS_ED25519).expect("generate Ed25519 key"); + let public_key_pem = key.public_key_pem().into_bytes(); + let issuer = SessionJwtIssuer::from_ed25519_pem( + key.serialize_pem().as_bytes(), + "current", + "test", + None, + Arc::new(FixedClock(1_900_000_000)), + ) + .expect("issuer"); + let verifier = SessionJwtVerifier::new( + "test", + SessionTokenProfile::Sandbox, + [SessionVerificationKey { + key_id: "current".to_string(), + public_key_pem, + }], + Arc::new(FixedClock(2_000_000_000)), + ) + .expect("verifier"); + let identity = SandboxRuntimeIdentity { + sandbox_id: SandboxId::parse("sandbox-a").expect("sandbox ID"), + runtime_generation: SandboxGenerationId::parse("generation-1") + .expect("runtime generation"), + auth_epoch: CredentialEpoch::new(1).expect("auth epoch"), + }; + let pair = issuer.mint_pair(&identity).expect("token pair"); + + assert_eq!(pair.gateway.expires_at, 0); + assert_eq!(pair.sandbox.expires_at, 0); + verifier + .verify(pair.sandbox.token.expose_secret()) + .expect("non-expiring token remains valid"); + + let slot = SessionBearerTokenSlot::new( + pair.sandbox.token.clone(), + pair.sandbox.expires_at, + pair.auth_epoch, + ) + .expect("non-expiring bearer slot"); + slot.authorization_metadata() + .expect("non-expiring bearer remains available"); + + let bundle = SupervisorAuthBundle { + session_id: SandboxSessionId::new(), + session_rotation: SessionRotation::new(1).expect("session rotation"), + runtime_generation: identity.runtime_generation, + auth_epoch: pair.auth_epoch, + gateway_token: pair.gateway.token, + gateway_expires_at: pair.gateway.expires_at, + sandbox_token: pair.sandbox.token, + sandbox_expires_at: pair.sandbox.expires_at, + }; + bundle.validate().expect("non-expiring auth bundle"); + } + #[test] fn token_debug_is_redacted() { let (issuer, _gateway, _sandbox, identity) = fixture(); diff --git a/crates/openshell-sandbox-backend/src/sandbox_auth.rs b/crates/openshell-sandbox-backend/src/sandbox_auth.rs index e9a72d5fd..142d50c39 100644 --- a/crates/openshell-sandbox-backend/src/sandbox_auth.rs +++ b/crates/openshell-sandbox-backend/src/sandbox_auth.rs @@ -389,7 +389,7 @@ mod tests { key.serialize_pem().as_bytes(), "current", "test", - DEFAULT_SESSION_TOKEN_TTL, + Some(DEFAULT_SESSION_TOKEN_TTL), clock.clone(), ) .expect("issuer"); diff --git a/crates/openshell-sandbox/src/boundary_server.rs b/crates/openshell-sandbox/src/boundary_server.rs index 3d85d730a..e6dfba1d9 100644 --- a/crates/openshell-sandbox/src/boundary_server.rs +++ b/crates/openshell-sandbox/src/boundary_server.rs @@ -700,6 +700,10 @@ mod linux { } fn update(&self, expires_at: i64) { + if expires_at == 0 { + self.set_deadline(None); + return; + } let now = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) .map_or(0, |duration| duration.as_secs()); @@ -710,11 +714,15 @@ mod linux { } fn update_deadline(&self, deadline: tokio::time::Instant) { + self.set_deadline(Some(deadline)); + } + + fn set_deadline(&self, deadline: Option) { let _ = self.deadline.send_if_modified(|current| { - if *current == Some(deadline) { + if *current == deadline { false } else { - *current = Some(deadline); + *current = deadline; true } }); @@ -3678,7 +3686,7 @@ mod linux { key.serialize_pem().as_bytes(), "test-key", "test-gateway", - DEFAULT_SESSION_TOKEN_TTL, + Some(DEFAULT_SESSION_TOKEN_TTL), Arc::new(SystemJwtClock), ) .expect("test session issuer"); @@ -3991,6 +3999,26 @@ mod linux { .expect("expiry worker must keep the shutdown channel open"); } + #[tokio::test] + async fn non_expiring_connection_has_no_deadline() { + let (shutdown, mut closed) = tokio::sync::watch::channel(()); + let expiry = ConnectionExpiry::new(shutdown); + expiry.update_deadline(tokio::time::Instant::now() + Duration::from_millis(20)); + expiry.update(0); + + assert!( + tokio::time::timeout(Duration::from_millis(80), closed.changed()) + .await + .is_err(), + "non-expiring credentials must clear the connection deadline" + ); + expiry.update_deadline(tokio::time::Instant::now() + Duration::from_millis(20)); + tokio::time::timeout(Duration::from_millis(80), closed.changed()) + .await + .expect("replacement connection deadline must fire") + .expect("expiry worker must keep the shutdown channel open"); + } + #[tokio::test(flavor = "multi_thread")] async fn idle_uds_and_tcp_handshakes_do_not_consume_authenticated_slots() { let directory = tempfile::tempdir().unwrap(); diff --git a/crates/openshell-server/src/auth/sandbox_jwt.rs b/crates/openshell-server/src/auth/sandbox_jwt.rs index d5f1cafa1..a86eca1fd 100644 --- a/crates/openshell-server/src/auth/sandbox_jwt.rs +++ b/crates/openshell-server/src/auth/sandbox_jwt.rs @@ -121,7 +121,7 @@ impl SandboxSessionJwtAuthority { public_key_pem: &[u8], key_id: String, gateway_id: &str, - ttl: Duration, + ttl: Option, ) -> Result { let clock = Arc::new(SystemJwtClock); let issuer = SessionJwtIssuer::from_ed25519_pem( diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index b57833356..4a38361e1 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -7828,7 +7828,7 @@ mod tests { material.public_key_pem.as_bytes(), material.kid, "test-gateway", - Duration::from_mins(15), + Some(Duration::from_mins(15)), ) .expect("test session authority") } diff --git a/crates/openshell-server/src/grpc/auth_rpc.rs b/crates/openshell-server/src/grpc/auth_rpc.rs index 90ebd4a30..83737fc81 100644 --- a/crates/openshell-server/src/grpc/auth_rpc.rs +++ b/crates/openshell-server/src/grpc/auth_rpc.rs @@ -304,15 +304,13 @@ pub async fn handle_refresh_sandbox_token( .sandbox_token .expose_secret() .to_string(), - sandbox_expiration_time: Some( - openshell_core::time::timestamp_from_millis( - authentication - .supervisor - .sandbox_expires_at - .saturating_mul(1000), - ) - .map_err(|error| Status::internal(error.to_string()))?, - ), + sandbox_expiration_time: openshell_core::time::optional_timestamp_from_legacy_millis( + authentication + .supervisor + .sandbox_expires_at + .saturating_mul(1000), + ) + .map_err(|error| Status::internal(error.to_string()))?, session_id: authentication.supervisor.runtime_generation.to_string(), credential_epoch: authentication.supervisor.auth_epoch.get(), })) @@ -447,7 +445,7 @@ mod tests { use std::collections::HashMap; use std::time::Duration; - async fn state_with_issuer() -> Arc { + async fn state_with_ttl(ttl: Option) -> Arc { let mat = generate_jwt_key().expect("jwt key"); let store = Arc::new( Store::connect("sqlite::memory:?cache=shared") @@ -483,7 +481,7 @@ mod tests { mat.public_key_pem.as_bytes(), mat.kid, "test-gateway", - Duration::from_hours(1), + ttl, ) .expect("session authority"), ); @@ -495,6 +493,10 @@ mod tests { state } + async fn state_with_issuer() -> Arc { + state_with_ttl(Some(Duration::from_hours(1))).await + } + async fn insert_sandbox( state: &Arc, sandbox_id: &str, @@ -619,6 +621,25 @@ mod tests { assert!(resp.sandbox_expiration_time.is_some()); } + #[tokio::test] + async fn refresh_propagates_non_expiring_session_credentials() { + let state = state_with_ttl(None).await; + let mut req = Request::new(RefreshSandboxTokenRequest { + extension_service_names: Vec::new(), + }); + req.extensions_mut().insert(sandbox_principal("sandbox-a")); + let _ = authorize_refresh(&state, &mut req).await; + let resp = handle_refresh_sandbox_token(&state, req) + .await + .expect("refresh OK") + .into_inner(); + + assert!(!resp.token.is_empty()); + assert!(!resp.sandbox_token.is_empty()); + assert!(resp.expiration_time.is_none()); + assert!(resp.sandbox_expiration_time.is_none()); + } + #[tokio::test] async fn refresh_replays_one_successor_then_rejects_older_bearers() { let state = state_with_issuer().await; diff --git a/crates/openshell-server/src/lib.rs b/crates/openshell-server/src/lib.rs index ff5c160be..a51e17a9a 100644 --- a/crates/openshell-server/src/lib.rs +++ b/crates/openshell-server/src/lib.rs @@ -577,7 +577,7 @@ pub(crate) async fn run_server( &public_pem, kid, &jwt.gateway_id, - jwt.token_ttl(), + jwt.sandbox_token_ttl(), ) .map_err(Error::config)?, ); diff --git a/crates/openshell-server/src/multiplex.rs b/crates/openshell-server/src/multiplex.rs index 2dfaa4ede..8988542b1 100644 --- a/crates/openshell-server/src/multiplex.rs +++ b/crates/openshell-server/src/multiplex.rs @@ -2963,7 +2963,7 @@ mod tests { material.public_key_pem.as_bytes(), material.kid.clone(), "test", - Duration::from_mins(15), + Some(Duration::from_mins(15)), ) .expect("session authority"), ); diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index ec4354aa1..38b731a69 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -90,7 +90,8 @@ future version. To migrate an existing file: 5. Use canonical image pull policies: `always`, `if_not_present`, `never`, or Podman-only `newer`. Kubernetes-style capitalization and Podman's `missing` spelling are rejected. -6. Remove zero sentinels. Omit `gateway_jwt.ttl_secs` to use the 900-second default, +6. Remove zero sentinels. Omit `gateway_jwt.ttl_secs` to use non-expiring local + sandbox session credentials and the 900-second typed extension-token default, omit Docker or Podman `sandbox_pids_limit` to use OpenShell's default limit of 2048, and omit Podman `health_check_interval_secs` to disable health checks. Explicit zero values are invalid. @@ -267,7 +268,7 @@ The client-certificate handshake policy is derived and has no `require_client_au `[openshell.gateway] policy_validation_failure_mode` controls what sandbox supervisors do when a complete candidate policy fails runtime validation. The default, `fail_closed`, deactivates the previous network policy, closes relays pinned to it, and denies new egress until a valid generation loads. `retain_last_valid` leaves the previous valid generation active. Both modes reject the candidate atomically; startup keeps the workload unstarted until the effective policy and matching provider configuration pass admission. A rejected startup exposes `ConfigurationInvalid` and remains available for policy/provider repair in either mode. Gateway mutation paths that can preflight a known effective scope reject invalid candidates before persistence and leave the active policy unchanged regardless of this setting. Changing the value requires restarting the gateway so it can reload `gateway.toml` and distribute the new posture to sandbox supervisors. -`[openshell.gateway.gateway_jwt] ttl_secs` controls the lifetime of gateway-minted, generation-bound sandbox session JWTs and typed extension JWTs. Omit it to use the 900-second default. Explicit `0` is invalid. Helm renders `3600` seconds by default. +`[openshell.gateway.gateway_jwt] ttl_secs` controls generation-bound gateway-facing and Sandbox Protocol credentials minted for a sandbox session, plus typed extension JWTs. Omit it for non-expiring local sandbox session credentials: both session tokens carry `exp = 0`, and refresh responses omit their expiration timestamps. Typed extension JWTs retain a 900-second default when the field is omitted. Use omission only for local single-player Docker, Podman, or VM gateways. Explicit `0` is invalid. Kubernetes and other shared deployments should set a positive TTL; Helm renders `3600` seconds by default, and the gateway logs a warning when a Kubernetes gateway omits the field. `[openshell.gateway.auth] allow_unauthenticated_users = true` is an unsafe local-development and trusted-proxy escape hatch. It accepts user-facing CLI/API calls without OIDC or mTLS credentials while sandbox supervisors still authenticate with gateway-minted sandbox JWTs. Leave it false for shared and production gateways. diff --git a/docs/sandboxes/manage-sandboxes.mdx b/docs/sandboxes/manage-sandboxes.mdx index a25eed30f..178ab9263 100644 --- a/docs/sandboxes/manage-sandboxes.mdx +++ b/docs/sandboxes/manage-sandboxes.mdx @@ -312,10 +312,19 @@ attaches to the same process instance and replays up to 1 MiB of recent output. One attachment owns stdin at a time. Use `sandbox exec --tty -- /bin/bash -l` when you want a new independent shell instead. +If an established connection is interrupted, for example when a laptop sleeps +and wakes, the CLI obtains a new SSH session and reattaches to the same main +process. It retries transient transport failures for up to 60 seconds. Initial +authentication failures, sandbox lifecycle changes, and clean SSH exits are +not retried. + Press `Ctrl-P`, then `Ctrl-Q` to disconnect without terminating the main process. `Ctrl-C` retains its normal terminal behavior and interrupts the foreground process. For read-only attachments, `Ctrl-C` only exits the -current viewer. +current viewer. OpenSSH's `~.` escape reports the same status as a broken +transport, so it starts automatic recovery instead of exiting. After `~.`, use +`Ctrl-P`, then `Ctrl-Q` once OpenShell reattaches, or press `Ctrl-C` while the +CLI is between retry attempts to cancel recovery. Launch VS Code or Cursor directly into the sandbox workspace: diff --git a/e2e/rust/Cargo.toml b/e2e/rust/Cargo.toml index 5bfa0a78f..4dc1be640 100644 --- a/e2e/rust/Cargo.toml +++ b/e2e/rust/Cargo.toml @@ -232,7 +232,7 @@ tonic = { version = "0.14", features = ["transport"] } tonic-prost = "0.14" tower = "0.5" url = "2" -nix = { version = "0.29", features = ["user"] } +nix = { version = "0.29", features = ["process", "signal", "term", "user"] } [dev-dependencies] serial_test = "3" diff --git a/e2e/rust/tests/sandbox_lifecycle.rs b/e2e/rust/tests/sandbox_lifecycle.rs index de50bc2d3..3ccd2d73c 100644 --- a/e2e/rust/tests/sandbox_lifecycle.rs +++ b/e2e/rust/tests/sandbox_lifecycle.rs @@ -3,6 +3,8 @@ #![cfg(feature = "e2e")] +#[cfg(target_os = "linux")] +use std::fs; use std::process::Stdio; use std::time::Duration; @@ -211,6 +213,64 @@ async fn reconnect_with_input_ownership( } } +#[cfg(target_os = "linux")] +fn find_process_with_args(expected_args: &[&str]) -> Option { + for entry in fs::read_dir("/proc").ok()?.filter_map(Result::ok) { + let Ok(pid) = entry.file_name().to_string_lossy().parse::() else { + continue; + }; + let cmdline = fs::read(entry.path().join("cmdline")).unwrap_or_default(); + let args = cmdline.split(|byte| *byte == 0).collect::>(); + if expected_args + .iter() + .all(|expected| args.contains(&expected.as_bytes())) + { + return Some(pid); + } + } + None +} + +#[cfg(target_os = "linux")] +fn find_child_process_with_args(parent_pid: u32, expected_args: &[&str]) -> Option { + for entry in fs::read_dir("/proc").ok()?.filter_map(Result::ok) { + let Ok(pid) = entry.file_name().to_string_lossy().parse::() else { + continue; + }; + let status = fs::read_to_string(entry.path().join("status")).unwrap_or_default(); + let process_parent = status.lines().find_map(|line| { + line.strip_prefix("PPid:") + .and_then(|value| value.trim().parse::().ok()) + }); + if process_parent != Some(parent_pid) { + continue; + } + let cmdline = fs::read(entry.path().join("cmdline")).unwrap_or_default(); + let args = cmdline.split(|byte| *byte == 0).collect::>(); + if expected_args + .iter() + .all(|expected| args.contains(&expected.as_bytes())) + { + return Some(pid); + } + } + None +} + +#[cfg(target_os = "linux")] +async fn wait_for_process_with_args(expected_args: &[&str]) -> u32 { + tokio::time::timeout(Duration::from_secs(10), async { + loop { + if let Some(pid) = find_process_with_args(expected_args) { + return pid; + } + sleep(Duration::from_millis(50)).await; + } + }) + .await + .unwrap_or_else(|_| panic!("process with arguments {expected_args:?} did not start")) +} + #[tokio::test] #[serial(sandbox_lifecycle)] async fn sandbox_stop_start_preserves_workspace() { @@ -722,6 +782,237 @@ async fn canonical_main_disconnect_reconnect_replays_history_for_same_process() sandbox.cleanup().await; } +#[cfg(target_os = "linux")] +#[tokio::test] +#[serial(sandbox_lifecycle)] +async fn canonical_main_connect_recovers_its_ssh_transport() { + let script = r#"trap 'kill "$writer" 2>/dev/null || true' EXIT; (n=1; while true; do printf 'transport_pid=%s sequence=%04d\n' "$$" "$n"; n=$((n + 1)); sleep 0.2; done) & writer=$!; while IFS= read -r line; do printf 'transport_pid=%s input=%s\n' "$$" "$line"; done"#; + let mut sandbox = SandboxGuard::create_detached_main(&["sh", "-lc", script]) + .await + .expect("create retained canonical main process"); + + let mut connect_cmd = openshell_cmd(); + connect_cmd + .args(["sandbox", "connect", &sandbox.name]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let mut connect = connect_cmd.spawn().expect("spawn supervised attachment"); + let mut connect_stdin = connect.stdin.take().expect("connect stdin"); + let connect_stdout = connect.stdout.take().expect("connect stdout"); + let mut connect_lines = BufReader::new(connect_stdout).lines(); + let connect_stderr = connect.stderr.take().expect("connect stderr"); + let mut connect_errors = BufReader::new(connect_stderr).lines(); + + let initial_line = tokio::time::timeout(Duration::from_secs(30), connect_lines.next_line()) + .await + .expect("initial attachment output timeout") + .expect("read initial attachment output") + .expect("initial attachment output should remain open"); + assert!( + initial_line.contains("transport_pid="), + "unexpected initial attachment output: {initial_line}" + ); + + // Recovery intentionally starts only for an established attachment, so + // let the initial SSH process live beyond that setup guard before killing + // only its ProxyCommand transport. + sleep(Duration::from_secs(3)).await; + let proxy_pid = wait_for_process_with_args(&["ssh-proxy", "--sandbox", &sandbox.name]).await; + let kill_status = tokio::process::Command::new("kill") + .args(["-TERM", &proxy_pid.to_string()]) + .status() + .await + .expect("terminate SSH proxy transport"); + assert!(kill_status.success(), "terminate SSH proxy transport"); + + tokio::time::timeout(Duration::from_secs(30), async { + loop { + let line = connect_errors + .next_line() + .await + .expect("read supervised attachment diagnostics") + .expect("supervised attachment diagnostics should remain open"); + if normalize_output(&line).contains("Connection to sandbox lost; reconnecting") { + break; + } + } + }) + .await + .expect("supervised attachment did not enter recovery"); + + let input_token = format!("after-transport-recovery-{:x}", rand::random::()); + connect_stdin + .write_all(format!("{input_token}\n").as_bytes()) + .await + .expect("write input after transport recovery"); + connect_stdin + .flush() + .await + .expect("flush input after transport recovery"); + + tokio::time::timeout(Duration::from_secs(30), async { + loop { + let line = connect_lines + .next_line() + .await + .expect("read recovered attachment output") + .expect("recovered attachment output should remain open"); + if line.contains(&format!("input={input_token}")) { + break; + } + } + }) + .await + .expect("replacement SSH session did not reattach to canonical main"); + assert!( + connect + .try_wait() + .expect("inspect supervised attachment") + .is_none(), + "sandbox connect parent should remain alive after recovery" + ); + + connect + .kill() + .await + .expect("disconnect recovered attachment"); + connect + .wait() + .await + .expect("wait for recovered attachment disconnect"); + sandbox.cleanup().await; +} + +#[cfg(target_os = "linux")] +#[tokio::test] +#[serial(sandbox_lifecycle)] +async fn canonical_main_connect_forwards_pid_targeted_termination_and_reaps_ssh() { + use std::os::fd::OwnedFd; + + let mut sandbox = SandboxGuard::create_detached_main(&["sh", "-lc", "exec sleep infinity"]) + .await + .expect("create retained canonical main process"); + let pty = nix::pty::openpty(None, None).expect("open pseudo-terminal"); + let controller: OwnedFd = pty.master; + let follower: OwnedFd = pty.slave; + + let mut connect_cmd = openshell_cmd(); + connect_cmd + .args(["sandbox", "connect", &sandbox.name]) + .stdin( + follower + .try_clone() + .expect("duplicate PTY follower for stdin"), + ) + .stdout( + follower + .try_clone() + .expect("duplicate PTY follower for stdout"), + ) + .stderr( + follower + .try_clone() + .expect("duplicate PTY follower for stderr"), + ); + let mut connect = connect_cmd + .spawn() + .expect("spawn supervised PTY attachment"); + let connect_pid = connect.id().expect("connect process ID"); + drop(follower); + + let ssh_pid = tokio::time::timeout(Duration::from_secs(30), async { + loop { + if let Some(pid) = + find_child_process_with_args(connect_pid, &["-s", "sandbox", "openshell-main"]) + { + return pid; + } + sleep(Duration::from_millis(50)).await; + } + }) + .await + .expect("supervised SSH child did not start"); + + nix::sys::signal::kill( + nix::unistd::Pid::from_raw(i32::try_from(connect_pid).expect("connect PID fits i32")), + nix::sys::signal::Signal::SIGTERM, + ) + .expect("send SIGTERM to only the OpenShell parent"); + + let status = tokio::time::timeout(Duration::from_secs(10), connect.wait()) + .await + .expect("OpenShell parent did not terminate") + .expect("wait for OpenShell parent"); + assert_eq!(status.code(), Some(143)); + tokio::time::timeout(Duration::from_secs(5), async { + while fs::metadata(format!("/proc/{ssh_pid}")).is_ok() { + sleep(Duration::from_millis(25)).await; + } + }) + .await + .expect("SSH child was not reaped after parent termination"); + + drop(controller); + sandbox.cleanup().await; +} + +#[tokio::test] +#[serial(sandbox_lifecycle)] +async fn canonical_main_exit_255_is_not_retried_as_transport_failure() { + const READY_MARKER: &str = "exit-255-ready"; + const RELEASE_PATH: &str = "/sandbox/.openshell-exit-255-release"; + let script = format!( + "echo {READY_MARKER}; while [ ! -e '{RELEASE_PATH}' ]; do sleep 0.05; done; exit 255" + ); + let mut sandbox = SandboxGuard::create_detached_main(&["sh", "-c", &script]) + .await + .expect("create retained canonical main process"); + + let mut connect_cmd = openshell_cmd(); + connect_cmd + .args(["sandbox", "connect", &sandbox.name]) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let mut connect = connect_cmd.spawn().expect("spawn supervised attachment"); + let connect_stdout = connect.stdout.take().expect("connect stdout"); + let mut connect_lines = BufReader::new(connect_stdout).lines(); + + tokio::time::timeout(Duration::from_secs(30), async { + loop { + let line = connect_lines + .next_line() + .await + .expect("read attachment output") + .expect("attachment output should remain open"); + if line.contains(READY_MARKER) { + break; + } + } + }) + .await + .expect("attachment did not observe the canonical main process"); + + // Keep the attachment alive beyond the setup guard used to distinguish + // initial SSH failures from established transport failures. + sleep(Duration::from_secs(3)).await; + sandbox + .exec(&["touch", RELEASE_PATH]) + .await + .expect("release canonical main process"); + + let status = tokio::time::timeout(Duration::from_secs(10), connect.wait()).await; + if status.is_err() { + connect.kill().await.expect("stop stuck attachment"); + } + sandbox.cleanup().await; + let status = status + .expect("exit status 255 must not enter the transport recovery loop") + .expect("wait for canonical main attachment"); + assert_eq!(status.code(), Some(255)); +} + #[tokio::test] #[serial(sandbox_lifecycle)] async fn sandbox_create_with_no_keep_cleans_up_after_tty_command() { diff --git a/proto/openshell.proto b/proto/openshell.proto index f90e5335b..a836e169b 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -859,7 +859,7 @@ message RefreshSandboxTokenResponse { repeated ExtensionServiceCredential extension_credentials = 3; // Fresh Sandbox Protocol bearer token from the same atomic refresh. string sandbox_token = 4 [(openshell.options.v1.secret) = true]; - // Absolute Sandbox Protocol token expiry. Required when sandbox_token is set. + // Absolute Sandbox Protocol token expiry. Absence means the token is non-expiring. google.protobuf.Timestamp sandbox_expiration_time = 105; // Launch generation to which both refreshed credentials are bound. string session_id = 6; diff --git a/sdk/go/proto/openshellv1/openshell.pb.go b/sdk/go/proto/openshellv1/openshell.pb.go index 66c1bb0c1..18d0b9bcd 100644 --- a/sdk/go/proto/openshellv1/openshell.pb.go +++ b/sdk/go/proto/openshellv1/openshell.pb.go @@ -1303,7 +1303,7 @@ type RefreshSandboxTokenResponse struct { ExtensionCredentials []*ExtensionServiceCredential `protobuf:"bytes,3,rep,name=extension_credentials,json=extensionCredentials,proto3" json:"extension_credentials,omitempty"` // Fresh Sandbox Protocol bearer token from the same atomic refresh. SandboxToken string `protobuf:"bytes,4,opt,name=sandbox_token,json=sandboxToken,proto3" json:"sandbox_token,omitempty"` - // Absolute Sandbox Protocol token expiry. Required when sandbox_token is set. + // Absolute Sandbox Protocol token expiry. Absence means the token is non-expiring. SandboxExpirationTime *timestamppb.Timestamp `protobuf:"bytes,105,opt,name=sandbox_expiration_time,json=sandboxExpirationTime,proto3" json:"sandbox_expiration_time,omitempty"` // Launch generation to which both refreshed credentials are bound. SessionId string `protobuf:"bytes,6,opt,name=session_id,json=sessionId,proto3" json:"session_id,omitempty"` diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index eea2867a2..5ae570381 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -352,11 +352,16 @@ openshell sandbox connect my-sandbox --editor vscode Attaches to the sandbox's existing canonical main process. Disconnecting leaves that process running; reconnecting targets the same process instance and replays -recent output. Use `sandbox exec --tty -- /bin/bash -l` for a new shell. Press -`Ctrl-P`, then `Ctrl-Q` to disconnect without terminating main. When you own -stdin, `Ctrl-C` interrupts the foreground process. In a read-only attachment, -`Ctrl-C` exits the viewer and leaves main and other attachments running. -Configure VS Code Remote-SSH with: +recent output. If an established SSH transport is interrupted, such as when a +laptop sleeps and wakes, the CLI retries transient failures for up to 60 seconds +and reattaches to that same process. Use `sandbox exec --tty -- /bin/bash -l` +for a new shell. Press `Ctrl-P`, then `Ctrl-Q` to disconnect without terminating +main. OpenSSH's `~.` escape looks like transport loss and therefore starts +automatic recovery; after it reattaches, use `Ctrl-P`, then `Ctrl-Q` to exit, or +press `Ctrl-C` between retry attempts to cancel recovery. When you own stdin, +`Ctrl-C` interrupts the foreground process. In a read-only attachment, `Ctrl-C` +exits the viewer and leaves main and other attachments running. Configure VS +Code Remote-SSH with: ```bash openshell sandbox ssh-config my-sandbox >> ~/.ssh/config diff --git a/tasks/scripts/gateway-podman.sh b/tasks/scripts/gateway-podman.sh index d2b5397fb..8f3cda7e4 100644 --- a/tasks/scripts/gateway-podman.sh +++ b/tasks/scripts/gateway-podman.sh @@ -214,7 +214,6 @@ signing_key_path = "${TLS_DIR}/jwt/signing.pem" public_key_path = "${TLS_DIR}/jwt/public.pem" kid_path = "${TLS_DIR}/jwt/kid" gateway_id = "${GATEWAY_NAME}" -ttl_secs = 3600 [openshell.drivers.podman] default_image = "${SANDBOX_IMAGE}"