diff --git a/.github/workflows/branch-e2e.yml b/.github/workflows/branch-e2e.yml index cd3821c05a..4efff1f6c8 100644 --- a/.github/workflows/branch-e2e.yml +++ b/.github/workflows/branch-e2e.yml @@ -260,6 +260,7 @@ jobs: integration-inputs-artifact-id: ${{ needs.prepare-integration.outputs.integration_inputs_artifact_id }} test-matrix: >- [ + {"environment":"fedora-podman-rootful","installer":"binaries","testsuite":"driver-podman"}, {"environment":"fedora-podman-rootless","installer":"binaries","testsuite":"driver-podman"} ] diff --git a/CI.md b/CI.md index 31d0b097b3..538c33cc04 100644 --- a/CI.md +++ b/CI.md @@ -40,6 +40,21 @@ The GitHub ruleset should require the `OpenShell / ...` statuses published by `Required CI Gates` plus the direct `OpenShell / Trivy Changes` result, not the push-triggered workflow jobs themselves. +### Run only the policy advisor conformance tests + +Manually dispatch `Integration Tests` on the candidate branch with an +`artifact-run-id` from a build of the same commit. Set `category` to +`policy-advisor` and `test-matrix` to: + +```json +[{"environment":"ubuntu-docker-rootful","installer":"binaries","testsuite":"policy-advisor"}] +``` + +This runs the `mechanistic-proposal` and `policy-local` conformance tests in the +installed-artifact suite. The artifact run must contain the candidate CLI and +gateway binaries and runtime images. This manual run does not replace the +required PR E2E gate. + ## Informational security reports Security workflow compute runs directly on GitHub-hosted runners instead of diff --git a/Cargo.lock b/Cargo.lock index 53a3654ebf..c6330b8897 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4016,6 +4016,7 @@ dependencies = [ "openshell-server", "openshell-tui", "owo-colors", + "prost", "prost-types", "rcgen", "reqwest 0.12.28", diff --git a/TESTING.md b/TESTING.md index 9088be5261..7a19ab73c0 100644 --- a/TESTING.md +++ b/TESTING.md @@ -181,8 +181,11 @@ lifecycle management, output parsing, and cleanup. Suites: - Common suite (`--features e2e`) - driver-neutral CLI behavior, sandbox lifecycle, sync, port forwarding, policy, and provider tests. -- CLI conformance (`openshell-conformance`) - the portable deployment smoke - scenario plus focused tests for its reusable command runner. +- CLI conformance (`openshell-conformance`) - named scenarios for lifecycle, + mechanistic drafts, and the sandbox-local API, including agent-authored + permission requests. Driver E2E wrappers run every scenario. The + installed-artifact conformance suite runs all scenarios and offers a focused + `policy-advisor` testsuite for manual integration runs. - Driver suites (`--features e2e-docker`, `e2e-podman`, `e2e-kubernetes`, or `e2e-vm`) - CLI conformance plus the common and driver-specific coverage for the selected deployment. diff --git a/architecture/gateway.md b/architecture/gateway.md index 2f626286e6..19ba751e36 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -272,7 +272,7 @@ IDs fail instead of creating source precedence. The gateway treats configured interceptors as trusted sources and does not verify signature annotations in their profile payloads. -The CLI exposes reusable profile definitions through `openshell profile`, with `list` and `describe` reading the same effective catalog used by provider creation. Export, import, update, lint, and delete share that top-level command group. Workspace selection and explicit platform scope apply at the existing profile API boundary; `openshell provider` manages credential-bearing instances. +The CLI exposes reusable profile definitions through `openshell profile`, with `list` and `describe` reading the same effective catalog used by provider creation. Export, import, update, lint, and delete share that top-level command group. Import and lint accept local files, local directories, or a single HTTP or HTTPS URL; the CLI fetches and parses remote content before submitting it to the gateway. Workspace selection and explicit platform scope apply at the existing profile API boundary; `openshell provider` manages credential-bearing instances. Each logical gateway request captures the selected sources into one validated, immutable effective catalog before deriving provider behavior. Policy layers, @@ -361,7 +361,8 @@ successor removes that retry path across every gateway replica. Short 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 +`exp = 0`, and supervisors skip periodic renewal of those session tokens. +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. @@ -1097,6 +1098,11 @@ stdout is redirected to a file), so the exec is never ended on output-idle alone — instead an unanswered keepalive on a wedged or orphaned relay closes the channel and returns the exec with an error. Once a command reports its exit status, the gateway also bounds how long it waits for the trailing channel close. +An exec relay reports success only after receiving both the SSH exit status and +channel close; a missing close is a transport error even when the exit status is zero. +The supervisor derives that SSH exit status from the boundary output stream's +terminal frame after draining stdout and stderr, so an output delivery failure +cannot be masked by a successful process wait. Interactive exec treats normal request-stream EOF as the end of stdin and resize input. The gateway sends SSH EOF while keeping the output channel open until @@ -1105,6 +1111,11 @@ as normal EOF. The input and output pumps are owned by the exec operation, so timeout or response abandonment cannot leave a detached stdin task behind. The pumps share polling fairly, and request processing yields cooperatively even for ignored resize messages, so sustained input cannot monopolize the operation. +The CLI keeps piped input in the unary request when the complete encoded +request fits the gateway's decoder limit, preserving compatibility with older +gateways. Input up to the CLI's 4 MiB cap uses this stream with bounded frames +when the unary message would exceed the decoder limit, with or without a PTY. +The CLI closes the input side at pipe EOF. Go and TypeScript interactive-exec helpers distinguish process exit from stream completion. They consume the final gRPC status before reporting success and retain diff --git a/architecture/sandbox.md b/architecture/sandbox.md index 7d5fdb23bc..b9a2b88603 100644 --- a/architecture/sandbox.md +++ b/architecture/sandbox.md @@ -140,6 +140,16 @@ While an exec handle is retained, independent waits return its stable exit or signal status, whether or not an output attachment is open or the main process has exited. Waiting never holds the exec registry lock, so other operations can still signal or attach to the process. +Exec output uses a bounded queue that backpressures the process reader until its +attachment consumes the bytes. An attached reader can apply backpressure +without losing bytes; an absent reader that leaves the queue full eventually +fails the exec and terminates its process. The final stream status carries +delivery failures separately from the process handle's stable wait result. +After the exec parent exits, output drains until its pipes close. If a +descendant keeps a pipe open beyond 30 seconds, the exec reports an output +delivery failure and drains later writes without retaining them. This avoids +guessing which bytes belong to the parent. +Canonical main-process output retains its bounded replay log. ## Isolation Layers @@ -321,6 +331,16 @@ captured before the bypass fence, mapped back to its workload process, authorize through the same egress pipeline, and dialed only through the pinned addresses. Omitted protocol endpoints retain explicit-proxy behavior. +Policy DNS reserves `policy.local` inside each sandbox without requiring an +authored network endpoint or a trusted external lookup. The source-backed TCP +boundary routes plain HTTP on the reserved address and port 80 to the +supervisor's sandbox-local policy API; it does not open an upstream connection. +The API checks the effective agent proposal setting for every request. Policy +denials at the staged TCP authorization gate enter the supervisor's denial +aggregator with the resolved binary and destination, so the mechanistic mapper +can propose a scoped rule. Invalid mappings and destination validation failures +do not become policy proposals. + Provider credential placeholders are resolved through the live provider state for each HTTP request, after destination and L7 policy admission. A static credential resolves only when the request host, port, and path match an endpoint diff --git a/crates/openshell-cli/Cargo.toml b/crates/openshell-cli/Cargo.toml index 94d69063c2..f097bb856f 100644 --- a/crates/openshell-cli/Cargo.toml +++ b/crates/openshell-cli/Cargo.toml @@ -24,6 +24,7 @@ openshell-tui = { path = "../openshell-tui" } serde = { workspace = true } serde_json = { workspace = true } serde_yml = { workspace = true } +prost = { workspace = true } prost-types = { workspace = true } # Async runtime diff --git a/crates/openshell-cli/src/commands/provider.rs b/crates/openshell-cli/src/commands/provider.rs index 3e889e025c..fca1d16f3f 100644 --- a/crates/openshell-cli/src/commands/provider.rs +++ b/crates/openshell-cli/src/commands/provider.rs @@ -1702,10 +1702,11 @@ pub async fn provider_profile_import( server: &str, file: Option<&Path>, from: Option<&Path>, + url: Option<&str>, workspace: &str, tls: &TlsOptions, ) -> Result<()> { - let (items, mut diagnostics) = load_profile_import_items(file, from)?; + let (items, mut diagnostics) = load_profile_import_items_with_url(file, from, url).await?; if items.is_empty() && diagnostics.is_empty() { return Err(miette!("no provider profile files found")); } @@ -1792,10 +1793,11 @@ pub async fn provider_profile_lint( server: &str, file: Option<&Path>, from: Option<&Path>, + url: Option<&str>, workspace: &str, tls: &TlsOptions, ) -> Result<()> { - let (items, mut diagnostics) = load_profile_import_items(file, from)?; + let (items, mut diagnostics) = load_profile_import_items_with_url(file, from, url).await?; if items.is_empty() && diagnostics.is_empty() { return Err(miette!("no provider profile files found")); } @@ -2137,6 +2139,92 @@ fn load_profile_import_items( Ok((items, diagnostics)) } +const MAX_REMOTE_PROFILE_BYTES: usize = 1024 * 1024; + +async fn load_profile_import_items_with_url( + file: Option<&Path>, + from: Option<&Path>, + url: Option<&str>, +) -> Result<( + Vec, + Vec, +)> { + let Some(source) = url else { + return load_profile_import_items(file, from); + }; + let parsed = reqwest::Url::parse(source) + .into_diagnostic() + .wrap_err("invalid provider profile URL")?; + if !matches!(parsed.scheme(), "http" | "https") { + return Err(miette!("provider profile URL must use http or https")); + } + if !parsed.username().is_empty() || parsed.password().is_some() { + return Err(miette!("provider profile URL must not contain userinfo")); + } + let mut display_url = parsed.clone(); + display_url.set_query(None); + display_url.set_fragment(None); + let path = parsed.path().to_owned(); + let format = Path::new(&path).extension().and_then(|ext| ext.to_str()); + if !matches!(format, Some("yaml" | "yml" | "json")) { + return Err(miette!( + "provider profile URL must end in .yaml, .yml, or .json" + )); + } + let _ = rustls::crypto::aws_lc_rs::default_provider().install_default(); + let client = reqwest::Client::builder() + .timeout(std::time::Duration::from_secs(15)) + .build() + .into_diagnostic()?; + let mut response = client + .get(parsed) + .send() + .await + .map_err(|err| { + miette!( + "failed to fetch provider profile from {display_url}: {}", + err.without_url() + ) + })? + .error_for_status() + .map_err(|err| { + miette!( + "failed to fetch provider profile from {display_url}: {}", + err.without_url() + ) + })?; + if response + .content_length() + .is_some_and(|len| len > MAX_REMOTE_PROFILE_BYTES as u64) + { + return Err(miette!( + "provider profile at {display_url} exceeds the 1 MiB download limit" + )); + } + let mut bytes = Vec::new(); + while let Some(chunk) = response.chunk().await.map_err(|err| { + miette!( + "failed to read provider profile from {display_url}: {}", + err.without_url() + ) + })? { + if bytes.len().saturating_add(chunk.len()) > MAX_REMOTE_PROFILE_BYTES { + return Err(miette!( + "provider profile at {display_url} exceeds the 1 MiB download limit" + )); + } + bytes.extend_from_slice(&chunk); + } + let input = std::str::from_utf8(&bytes) + .into_diagnostic() + .wrap_err_with(|| format!("provider profile at {display_url} is not UTF-8"))?; + let item = parse_profile_import_item(display_url.as_str(), input, format); + Ok(match item { + Ok(item) => (vec![item], Vec::new()), + Err(diagnostic) => (Vec::new(), vec![diagnostic]), + }) +} + fn profile_source_paths(file: Option<&Path>, from: Option<&Path>) -> Result> { if let Some(file) = file { return Ok(vec![file.to_path_buf()]); @@ -2176,19 +2264,31 @@ fn load_profile_import_item( format!("failed to read provider profile file: {err}"), ) })?; - let profile = match path.extension().and_then(|ext| ext.to_str()) { - Some("yaml" | "yml") => parse_profile_yaml(&input), - Some("json") => parse_profile_json(&input), + parse_profile_import_item( + &source, + &input, + path.extension().and_then(|ext| ext.to_str()), + ) +} + +fn parse_profile_import_item( + source: &str, + input: &str, + format: Option<&str>, +) -> Result { + let profile = match format { + Some("yaml" | "yml") => parse_profile_yaml(input), + Some("json") => parse_profile_json(input), _ => { return Err(profile_file_diagnostic( - &source, + source, "unsupported provider profile file format".to_string(), )); } } - .map_err(|err| profile_file_diagnostic(&source, err.to_string()))?; + .map_err(|err| profile_file_diagnostic(source, err.to_string()))?; - let pre_lower = profile.validate_before_lowering(&source); + let pre_lower = profile.validate_before_lowering(source); if let Some(diag) = pre_lower.into_iter().find(|d| d.severity == "error") { return Err(ProviderProfileDiagnostic { source: diag.source, @@ -2201,7 +2301,7 @@ fn load_profile_import_item( Ok(ProviderProfileImportItem { profile: Some(profile.to_proto()), - source, + source: source.to_string(), }) } diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index eb52e3f253..e5f105919c 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1105,8 +1105,8 @@ enum ProfileCommands { global: bool, }, - /// Import provider profiles from a file or directory. - #[command(group = clap::ArgGroup::new("source").required(true).args(["file", "from"]), help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] + /// Import provider profiles from a file, directory, or HTTP URL. + #[command(group = clap::ArgGroup::new("source").required(true).args(["file", "from", "url"]), help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] Import { /// Profile file to import. #[arg(short = 'f', long = "file", value_hint = ValueHint::FilePath)] @@ -1116,6 +1116,10 @@ enum ProfileCommands { #[arg(long = "from", value_hint = ValueHint::DirPath)] from: Option, + /// HTTP or HTTPS URL of one YAML or JSON profile. + #[arg(long)] + url: Option, + /// Import as platform-scoped profiles (ignores --workspace). #[arg(long)] global: bool, @@ -1137,7 +1141,7 @@ enum ProfileCommands { }, /// Validate provider profile files without registering them. - #[command(group = clap::ArgGroup::new("source").required(true).args(["file", "from"]), help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] + #[command(group = clap::ArgGroup::new("source").required(true).args(["file", "from", "url"]), help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] Lint { /// Profile file to lint. #[arg(short = 'f', long = "file", value_hint = ValueHint::FilePath)] @@ -1147,6 +1151,10 @@ enum ProfileCommands { #[arg(long = "from", value_hint = ValueHint::DirPath)] from: Option, + /// HTTP or HTTPS URL of one YAML or JSON profile. + #[arg(long)] + url: Option, + /// Lint against platform scope (ignores --workspace). #[arg(long)] global: bool, @@ -1209,11 +1217,17 @@ impl ProfileCommands { ) .await?; } - Self::Import { file, from, global } => { + Self::Import { + file, + from, + url, + global, + } => { run::provider_profile_import( endpoint, file.as_deref(), from.as_deref(), + url.as_deref(), profile_workspace(global), tls, ) @@ -1223,11 +1237,17 @@ impl ProfileCommands { run::provider_profile_update(endpoint, &id, &file, profile_workspace(global), tls) .await?; } - Self::Lint { file, from, global } => { + Self::Lint { + file, + from, + url, + global, + } => { run::provider_profile_lint( endpoint, file.as_deref(), from.as_deref(), + url.as_deref(), profile_workspace(global), tls, ) @@ -5022,7 +5042,7 @@ mod tests { #[test] fn profile_import_and_lint_require_exactly_one_source() { for verb in ["import", "lint"] { - for source in ["-f", "--from"] { + for source in ["-f", "--from", "--url"] { let cli = Cli::try_parse_from([ "openshell", "profile", @@ -5032,19 +5052,30 @@ mod tests { "--global", ]) .expect("profile source should parse"); - let (file, from, global) = match cli.command { + let (file, from, url, global) = match cli.command { Some(Commands::Profile { command: Some( - ProfileCommands::Import { file, from, global } - | ProfileCommands::Lint { file, from, global }, + ProfileCommands::Import { + file, + from, + url, + global, + } + | ProfileCommands::Lint { + file, + from, + url, + global, + }, ), - }) => (file, from, global), + }) => (file, from, url, global), other => panic!("unexpected profile command: {other:?}"), }; assert!(global); assert_eq!(file.is_some(), source == "-f"); assert_eq!(from.is_some(), source == "--from"); + assert_eq!(url.is_some(), source == "--url"); } assert!(Cli::try_parse_from(["openshell", "profile", verb]).is_err()); assert!( diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 74d92351ff..15d91b5891 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -64,6 +64,7 @@ use openshell_core::proto::{ }; use openshell_core::settings; use openshell_core::{ObjectId, ObjectName, ObjectWorkspace}; +use prost::Message; use std::borrow::Cow; use std::collections::HashMap; use std::io::{ErrorKind, IsTerminal, Read, Write}; @@ -1794,16 +1795,15 @@ where Ok(()) } -/// Maximum stdin payload size (4 MiB). Prevents the CLI from reading unbounded -/// data into memory before the server rejects an oversized message. -const MAX_STDIN_PAYLOAD: usize = 4 * 1024 * 1024; - fn local_terminal_size() -> Option<(u32, u32)> { crossterm::terminal::size() .ok() .map(|(cols, rows)| (u32::from(cols), u32::from(rows))) } +const MAX_EXEC_REQUEST_BYTES: usize = 1024 * 1024; +const MAX_EXEC_STDIN_BYTES: usize = 4 * 1024 * 1024; + /// Execute a command in a running sandbox via gRPC, streaming output to the terminal. /// /// Returns the remote command's exit code, or an error if the event stream @@ -1846,37 +1846,66 @@ pub async fn sandbox_exec_grpc( )); } - // Read stdin if piped (not a TTY), using spawn_blocking to avoid blocking - // the async runtime. Cap the read at MAX_STDIN_PAYLOAD + 1 so we never - // buffer more than the limit into memory. - let stdin_payload = if std::io::stdin().is_terminal() { + // Resolve TTY mode: explicit --tty / --no-tty wins, otherwise auto-detect. + let stdin_is_terminal = std::io::stdin().is_terminal(); + let tty = tty_override.unwrap_or_else(|| stdin_is_terminal && std::io::stdout().is_terminal()); + + // Preserve unary exec for small pipes, including older gateways whose + // interactive RPC closes the SSH channel when stdin reaches EOF. Retain + // the existing 4 MiB input cap because the supervisor's process stdin + // queue is unbounded; larger input should use file upload instead. + let stdin_prefix = if stdin_is_terminal { Vec::new() } else { tokio::task::spawn_blocking(|| { - let limit = (MAX_STDIN_PAYLOAD + 1) as u64; - let mut buf = Vec::new(); + let mut prefix = Vec::new(); std::io::stdin() - .take(limit) - .read_to_end(&mut buf) + .take((MAX_EXEC_STDIN_BYTES + 1) as u64) + .read_to_end(&mut prefix) .into_diagnostic()?; - if buf.len() > MAX_STDIN_PAYLOAD { + if prefix.len() > MAX_EXEC_STDIN_BYTES { return Err(miette::miette!( - "stdin payload exceeds {} byte limit; pipe smaller inputs or use `sandbox upload`", - MAX_STDIN_PAYLOAD + "piped stdin exceeds the 4 MiB limit; use `sandbox upload` for larger input" )); } - Ok(buf) + Ok::<_, miette::Report>(prefix) }) .await - .into_diagnostic()?? // first ? unwraps JoinError, second ? unwraps Result + .into_diagnostic()?? }; - // Resolve TTY mode: explicit --tty / --no-tty wins, otherwise auto-detect. - let tty = tty_override - .unwrap_or_else(|| std::io::stdin().is_terminal() && std::io::stdout().is_terminal()); + let (cols, rows) = if tty { + local_terminal_size().unwrap_or((80, 24)) + } else { + (0, 0) + }; + let mut request = ExecSandboxRequest { + request_id: String::new(), + sandbox: name.to_string(), + workspace_scope: Some(openshell_core::proto::workspace_selector( + workspace.to_string(), + )), + command: command.to_vec(), + workdir: workdir.unwrap_or_default().to_string(), + environment: environment.clone(), + execution_timeout: proto_execution_timeout(timeout_seconds)?, + stdin: stdin_prefix, + tty, + cols, + rows, + no_login_shell, + }; - if tty && std::io::stdin().is_terminal() { - return sandbox_exec_interactive_grpc( + let stdin_for_size_check = std::mem::take(&mut request.stdin); + let start_request_bytes = request.encoded_len(); + request.stdin = stdin_for_size_check; + if start_request_bytes > MAX_EXEC_REQUEST_BYTES { + return Err(miette::miette!( + "exec command or environment exceeds the gateway's 1 MiB message limit" + )); + } + if (tty && stdin_is_terminal) || request.encoded_len() > MAX_EXEC_REQUEST_BYTES { + return sandbox_exec_streaming_grpc( client, &sandbox, command, @@ -1884,34 +1913,16 @@ pub async fn sandbox_exec_grpc( timeout_seconds, environment, no_login_shell, + tty, + stdin_is_terminal, + std::mem::take(&mut request.stdin), ) .await; } - let (cols, rows) = if tty { - local_terminal_size().unwrap_or_default() - } else { - (0, 0) - }; - // Make the streaming gRPC call. let mut stream = client - .exec_sandbox(ExecSandboxRequest { - request_id: String::new(), - sandbox: name.to_string(), - workspace_scope: Some(openshell_core::proto::workspace_selector( - workspace.to_string(), - )), - command: command.to_vec(), - workdir: workdir.unwrap_or_default().to_string(), - environment: environment.clone(), - execution_timeout: proto_execution_timeout(timeout_seconds)?, - stdin: stdin_payload, - tty, - cols, - rows, - no_login_shell, - }) + .exec_sandbox(request) .await .into_diagnostic()? .into_inner(); @@ -2279,7 +2290,8 @@ impl Drop for TaskGuard { } } -async fn sandbox_exec_interactive_grpc( +#[allow(clippy::too_many_arguments)] +async fn sandbox_exec_streaming_grpc( mut client: crate::tls::GrpcClient, sandbox: &Sandbox, command: &[String], @@ -2287,15 +2299,22 @@ async fn sandbox_exec_interactive_grpc( timeout_seconds: u32, environment: &HashMap, no_login_shell: bool, + tty: bool, + stdin_is_terminal: bool, + stdin_prefix: Vec, ) -> Result { #[cfg(unix)] use openshell_core::proto::ExecSandboxWindowResize; use openshell_core::proto::{ExecSandboxInput, exec_sandbox_input}; use tokio_stream::wrappers::ReceiverStream; - let (cols, rows) = local_terminal_size().unwrap_or((80, 24)); + let (cols, rows) = if tty { + local_terminal_size().unwrap_or((80, 24)) + } else { + (0, 0) + }; - let (input_tx, input_rx) = tokio::sync::mpsc::channel::(4096); + let (input_tx, input_rx) = tokio::sync::mpsc::channel::(64); // Send the start message with exec metadata. input_tx @@ -2312,7 +2331,7 @@ async fn sandbox_exec_interactive_grpc( no_login_shell, execution_timeout: proto_execution_timeout(timeout_seconds)?, stdin: Vec::new(), - tty: true, + tty, cols, rows, })), @@ -2326,40 +2345,62 @@ async fn sandbox_exec_interactive_grpc( .into_diagnostic()? .into_inner(); - // Enable raw mode so keystrokes are forwarded immediately. - crossterm::terminal::enable_raw_mode().into_diagnostic()?; - let raw_guard = RawModeGuard; + // Raw mode is only appropriate for an interactive terminal, not a pipe. + let raw_guard = if tty && stdin_is_terminal { + crossterm::terminal::enable_raw_mode().into_diagnostic()?; + Some(RawModeGuard) + } else { + None + }; // Stdin reader on a detached OS thread. Using std::thread (not // spawn_blocking) so the tokio runtime shutdown doesn't wait for a // thread blocked on stdin.read(). The thread exits when the channel // closes (blocking_send returns Err) or stdin hits EOF. let stdin_tx = input_tx.clone(); + let (stdin_result_tx, mut stdin_result_rx) = tokio::sync::oneshot::channel(); std::thread::spawn(move || { let mut stdin = std::io::stdin().lock(); let mut buf = [0u8; 4096]; - loop { - match stdin.read(&mut buf) { - Ok(0) | Err(_) => break, - Ok(n) => { - if stdin_tx - .blocking_send(ExecSandboxInput { - payload: Some(exec_sandbox_input::Payload::Stdin(buf[..n].to_vec())), - }) - .is_err() - { - break; + let result = (|| { + for chunk in stdin_prefix.chunks(buf.len()) { + if stdin_tx + .blocking_send(ExecSandboxInput { + payload: Some(exec_sandbox_input::Payload::Stdin(chunk.to_vec())), + }) + .is_err() + { + return Ok(()); + } + } + loop { + match stdin.read(&mut buf) { + Ok(0) => return Ok(()), + Err(error) if error.kind() == ErrorKind::Interrupted => {} + Err(error) => return Err(error), + Ok(n) => { + if stdin_tx + .blocking_send(ExecSandboxInput { + payload: Some(exec_sandbox_input::Payload::Stdin( + buf[..n].to_vec(), + )), + }) + .is_err() + { + return Ok(()); + } } } } - } + })(); + let _ = stdin_result_tx.send(result); }); // SIGWINCH handler: forward terminal resize events. #[cfg(unix)] - let resize_task = { + let resize_task = if tty && stdin_is_terminal { let resize_tx = input_tx.clone(); - tokio::spawn(async move { + Some(tokio::spawn(async move { let mut sig = tokio::signal::unix::signal(tokio::signal::unix::SignalKind::window_change()) .expect("failed to register SIGWINCH handler"); @@ -2375,17 +2416,56 @@ async fn sandbox_exec_interactive_grpc( } } } - }) + })) + } else { + None }; #[cfg(unix)] - let _resize_guard = TaskGuard(resize_task); + let _resize_guard = resize_task.map(TaskGuard); + + // Keep a sender until the reader confirms clean EOF. On a read error, + // cancel the response stream before the gateway can treat channel EOF as + // successful completion of a partial command. + let mut pipe_input_tx = Some(input_tx); let mut exit_code = 0i32; let mut exit_seen = false; let stdout = std::io::stdout(); let stderr = std::io::stderr(); - while let Some(event) = stream.next().await { + let mut stdin_reader_done = false; + loop { + let event = tokio::select! { + result = &mut stdin_result_rx, if !stdin_reader_done => { + stdin_reader_done = true; + match result.into_diagnostic()? { + Ok(()) => { + let sender = pipe_input_tx.take().expect("stdin sender is held until EOF"); + drop(sender); + } + Err(error) => { + let sender = pipe_input_tx.take().expect("stdin sender is held until EOF"); + // A clean request EOF would make the gateway execute + // the truncated input. An invalid frame makes the + // gateway abort the command instead. + let abort = ExecSandboxInput { payload: None }; + if tokio::time::timeout(Duration::from_secs(5), sender.send(abort)) + .await + .is_err() + { + // Keep the request body open if a blocked remote + // stdin prevents delivery of the abort frame. + std::mem::forget(sender); + } + drop(stream); + return Err(error).into_diagnostic(); + } + } + continue; + } + event = stream.next() => event, + }; + let Some(event) = event else { break }; let event = event.into_diagnostic()?; match event.payload { Some(exec_sandbox_event::Payload::Stdout(out)) => { @@ -2407,11 +2487,13 @@ async fn sandbox_exec_interactive_grpc( } } - drop(input_tx); - // Drop the raw mode guard to restore the terminal before returning. drop(raw_guard); + if !stdin_reader_done && let Ok(result) = stdin_result_rx.try_recv() { + result.into_diagnostic()?; + } + // A stream that closes without an Exit event means we never observed the // command's outcome. Treat it as a relay failure rather than reporting a // successful (0) exit. diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index 9e8782b878..2cbeaffdf6 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -4367,12 +4367,26 @@ binaries: [/usr/bin/custom] ) .unwrap(); - run::provider_profile_lint(&ts.endpoint, Some(&profile_path), None, "default", &ts.tls) - .await - .expect("profile lint"); - run::provider_profile_import(&ts.endpoint, Some(&profile_path), None, "default", &ts.tls) - .await - .expect("profile import"); + run::provider_profile_lint( + &ts.endpoint, + Some(&profile_path), + None, + None, + "default", + &ts.tls, + ) + .await + .expect("profile lint"); + run::provider_profile_import( + &ts.endpoint, + Some(&profile_path), + None, + None, + "default", + &ts.tls, + ) + .await + .expect("profile import"); let exported_yaml = run::provider_profile_export_text(&ts.endpoint, "custom-api", "yaml", "default", &ts.tls) .await @@ -4938,9 +4952,16 @@ binaries: [/usr/bin/yaml-client] .unwrap(); std::fs::write(dir.path().join("notes.txt"), "ignored").unwrap(); - run::provider_profile_import(&ts.endpoint, None, Some(dir.path()), "default", &ts.tls) - .await - .expect("profile import --from"); + run::provider_profile_import( + &ts.endpoint, + None, + Some(dir.path()), + None, + "default", + &ts.tls, + ) + .await + .expect("profile import --from"); run::provider_profile_export(&ts.endpoint, "custom-yaml", "yaml", "default", &ts.tls) .await @@ -4950,6 +4971,108 @@ binaries: [/usr/bin/yaml-client] .expect("custom-json should be imported"); } +#[tokio::test] +async fn provider_profile_lint_and_import_from_http_url() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let ts = run_server().await; + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = format!( + "http://{}/remote.yaml?revision=1", + listener.local_addr().unwrap() + ); + tokio::spawn(async move { + let body = "id: remote-api\ndisplay_name: Remote API\ncategory: other\n"; + for _ in 0..2 { + let (mut stream, _) = listener.accept().await.unwrap(); + let mut request = [0_u8; 1024]; + assert!(stream.read(&mut request).await.unwrap() > 0); + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ); + stream.write_all(response.as_bytes()).await.unwrap(); + } + }); + + run::provider_profile_lint(&ts.endpoint, None, None, Some(&url), "default", &ts.tls) + .await + .expect("remote profile lint"); + run::provider_profile_import(&ts.endpoint, None, None, Some(&url), "default", &ts.tls) + .await + .expect("remote profile import"); + run::provider_profile_export(&ts.endpoint, "remote-api", "yaml", "default", &ts.tls) + .await + .expect("remote profile should be imported"); +} + +#[tokio::test] +async fn provider_profile_import_rejects_unsupported_remote_url() { + let ts = run_server().await; + let err = run::provider_profile_import( + &ts.endpoint, + None, + None, + Some("file:///tmp/profile.yaml"), + "default", + &ts.tls, + ) + .await + .expect_err("file URL must fail"); + assert!(err.to_string().contains("http or https")); +} + +#[tokio::test] +async fn provider_profile_import_rejects_oversized_http_response() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let ts = run_server().await; + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = format!("http://{}/large.yaml", listener.local_addr().unwrap()); + tokio::spawn(async move { + let (mut stream, _) = listener.accept().await.unwrap(); + let mut request = [0_u8; 1024]; + assert!(stream.read(&mut request).await.unwrap() > 0); + stream + .write_all(b"HTTP/1.1 200 OK\r\nContent-Length: 1048577\r\nConnection: close\r\n\r\n") + .await + .unwrap(); + }); + let err = + run::provider_profile_import(&ts.endpoint, None, None, Some(&url), "default", &ts.tls) + .await + .expect_err("oversized profile must fail"); + assert!(err.to_string().contains("1 MiB download limit")); +} + +#[tokio::test] +async fn provider_profile_import_redacts_url_query_from_http_errors() { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let ts = run_server().await; + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = format!( + "http://{}/missing.yaml?token=private-value", + listener.local_addr().unwrap() + ); + tokio::spawn(async move { + let (mut stream, _) = listener.accept().await.unwrap(); + let mut request = [0_u8; 1024]; + assert!(stream.read(&mut request).await.unwrap() > 0); + stream + .write_all(b"HTTP/1.1 404 Not Found\r\nContent-Length: 0\r\n\r\n") + .await + .unwrap(); + }); + let err = + run::provider_profile_import(&ts.endpoint, None, None, Some(&url), "default", &ts.tls) + .await + .expect_err("missing remote profile must fail"); + let message = err.to_string(); + assert!(message.contains("404")); + assert!(!message.contains("private-value")); +} + #[tokio::test] async fn provider_profile_import_preserves_advanced_network_policy_fields() { let ts = run_server().await; @@ -4982,9 +5105,16 @@ binaries: ) .unwrap(); - run::provider_profile_import(&ts.endpoint, Some(&profile_path), None, "default", &ts.tls) - .await - .expect("profile import"); + run::provider_profile_import( + &ts.endpoint, + Some(&profile_path), + None, + None, + "default", + &ts.tls, + ) + .await + .expect("profile import"); let mut client = openshell_cli::tls::grpc_client(&ts.endpoint, &ts.tls) .await @@ -5027,10 +5157,16 @@ endpoints: .unwrap(); std::fs::write(dir.path().join("broken.yaml"), "id: [\n").unwrap(); - let err = - run::provider_profile_import(&ts.endpoint, None, Some(dir.path()), "default", &ts.tls) - .await - .expect_err("profile import --from should fail on parse errors"); + let err = run::provider_profile_import( + &ts.endpoint, + None, + Some(dir.path()), + None, + "default", + &ts.tls, + ) + .await + .expect_err("profile import --from should fail on parse errors"); assert!( err.to_string().contains("provider profile import failed"), "unexpected error: {err}" @@ -5059,9 +5195,16 @@ endpoints: .unwrap(); std::fs::write(dir.path().join("broken.yaml"), "id: [\n").unwrap(); - let err = run::provider_profile_lint(&ts.endpoint, None, Some(dir.path()), "default", &ts.tls) - .await - .expect_err("profile lint --from should fail on parse errors"); + let err = run::provider_profile_lint( + &ts.endpoint, + None, + Some(dir.path()), + None, + "default", + &ts.tls, + ) + .await + .expect_err("profile lint --from should fail on parse errors"); assert!( err.to_string().contains("provider profile lint failed"), "unexpected error: {err}" diff --git a/crates/openshell-conformance-cli/src/main.rs b/crates/openshell-conformance-cli/src/main.rs index c9c0beb5e0..81ae17257b 100644 --- a/crates/openshell-conformance-cli/src/main.rs +++ b/crates/openshell-conformance-cli/src/main.rs @@ -91,7 +91,7 @@ fn list(output: OutputFormat) -> Result<(), String> { match output { OutputFormat::Text => { for candidate in scenarios() { - println!("{:<16} {}", candidate.name, candidate.description); + println!("{:<24} {}", candidate.name, candidate.description); } } OutputFormat::Json => { @@ -231,6 +231,22 @@ mod tests { assert!(error.contains("openshell-conformance list")); } + #[test] + fn selects_named_policy_scenarios() { + let selected = select_scenarios(&[ + "mechanistic-proposal".to_string(), + "policy-local".to_string(), + ]) + .unwrap(); + assert_eq!( + selected + .iter() + .map(|scenario| scenario.name) + .collect::>(), + ["mechanistic-proposal", "policy-local"] + ); + } + #[test] fn parses_binary_override_and_json_output() { let cli = Cli::try_parse_from([ diff --git a/crates/openshell-conformance/Cargo.toml b/crates/openshell-conformance/Cargo.toml index 7b67258045..fbe777a7aa 100644 --- a/crates/openshell-conformance/Cargo.toml +++ b/crates/openshell-conformance/Cargo.toml @@ -14,10 +14,8 @@ repository.workspace = true rand.workspace = true serde.workspace = true serde_json.workspace = true -tokio.workspace = true - -[dev-dependencies] tempfile = "3" +tokio.workspace = true [lints] workspace = true diff --git a/crates/openshell-conformance/src/executor.rs b/crates/openshell-conformance/src/executor.rs index 032edf1e9f..5c5e5844c9 100644 --- a/crates/openshell-conformance/src/executor.rs +++ b/crates/openshell-conformance/src/executor.rs @@ -50,6 +50,7 @@ impl CliExecutor for ProcessCli { process .args(&args) .envs(environment) + .env("NO_COLOR", "1") .stdout(Stdio::piped()) .stderr(Stdio::piped()) .kill_on_drop(true); diff --git a/crates/openshell-conformance/src/lib.rs b/crates/openshell-conformance/src/lib.rs index 46f385ef71..189e41c4fe 100644 --- a/crates/openshell-conformance/src/lib.rs +++ b/crates/openshell-conformance/src/lib.rs @@ -23,7 +23,10 @@ use tokio::time::sleep; use self::executor::{CliExecutionError, CliExecutor, ProcessCli}; -pub use scenarios::{SANDBOX_LIFECYCLE_SCENARIO, SMOKE_SCENARIO}; +pub use scenarios::{ + MECHANISTIC_PROPOSAL_SCENARIO, POLICY_LOCAL_SCENARIO, SANDBOX_LIFECYCLE_SCENARIO, + SMOKE_SCENARIO, +}; /// An installed conformance scenario. #[derive(Debug)] @@ -41,7 +44,12 @@ impl Scenario { } } -const SCENARIOS: &[Scenario] = &[SMOKE_SCENARIO, SANDBOX_LIFECYCLE_SCENARIO]; +const SCENARIOS: &[Scenario] = &[ + SMOKE_SCENARIO, + SANDBOX_LIFECYCLE_SCENARIO, + MECHANISTIC_PROPOSAL_SCENARIO, + POLICY_LOCAL_SCENARIO, +]; /// Returns every scenario compiled into this distribution. pub fn scenarios() -> &'static [Scenario] { diff --git a/crates/openshell-conformance/src/scenarios/mod.rs b/crates/openshell-conformance/src/scenarios/mod.rs index a7bf2781f1..3802d49de5 100644 --- a/crates/openshell-conformance/src/scenarios/mod.rs +++ b/crates/openshell-conformance/src/scenarios/mod.rs @@ -3,8 +3,10 @@ //! Registered, portable conformance scenarios. +mod policy_behavior; mod sandbox_lifecycle; mod smoke; +pub use policy_behavior::{MECHANISTIC_PROPOSAL_SCENARIO, POLICY_LOCAL_SCENARIO}; pub use sandbox_lifecycle::SANDBOX_LIFECYCLE_SCENARIO; pub use smoke::SMOKE_SCENARIO; diff --git a/crates/openshell-conformance/src/scenarios/policy_behavior.rs b/crates/openshell-conformance/src/scenarios/policy_behavior.rs new file mode 100644 index 0000000000..13cb9574a5 --- /dev/null +++ b/crates/openshell-conformance/src/scenarios/policy_behavior.rs @@ -0,0 +1,472 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Portable policy-local and mechanistic proposal checks. + +use std::io::Write as _; +use std::time::{Duration, Instant}; + +use serde::Deserialize; +use serde_json::Value; +use tempfile::NamedTempFile; +use tokio::time::sleep; + +use crate::{OpenShellRunner, Scenario, ScenarioFuture}; + +const CREATE_TIMEOUT: Duration = Duration::from_mins(10); +const COMMAND_TIMEOUT: Duration = Duration::from_secs(45); +const READY_TIMEOUT: Duration = Duration::from_mins(4); +const POLL_INTERVAL: Duration = Duration::from_secs(2); +const PROPOSAL_TIMEOUT: Duration = Duration::from_secs(90); + +#[derive(Deserialize)] +struct SandboxState { + name: String, + phase: String, +} + +pub const POLICY_LOCAL_SCENARIO: Scenario = Scenario { + name: "policy-local", + description: "Read and request a rule through the sandbox-local policy HTTP API.", + run: run_policy_local, +}; + +pub const MECHANISTIC_PROPOSAL_SCENARIO: Scenario = Scenario { + name: "mechanistic-proposal", + description: "Turn a denied transparent TCP open into a scoped policy draft.", + run: run_mechanistic_proposal, +}; + +fn run_policy_local(runner: &mut OpenShellRunner) -> ScenarioFuture<'_> { + Box::pin(async move { + let name = format!("ct-{}-pl", runner.id()); + create_sandbox(runner, &name, None).await?; + enable_proposals(runner, &name).await?; + let binary = sandbox_bash_path(runner, &name).await?; + + let started = Instant::now(); + let readiness_path = format!("/v1/proposals/ct-{}-readiness", runner.id()); + loop { + match request_policy_local(runner, &name, "/v1/policy/current").await { + Ok(response) + if response["format"] == "yaml" + && response["policy_yaml"] + .as_str() + .is_some_and(|yaml| yaml.contains("version: 1")) => + { + // The current-policy route is local; proposal submission also + // needs the supervisor's workspace and gateway lookup session. + match request_policy_local_http(runner, &name, "GET", &readiness_path, "", 404) + .await + { + Ok(lookup) if lookup["error"] == "chunk_not_found" => break, + Ok(lookup) => { + return Err(format!( + "policy.local proposal lookup returned an invalid readiness response: {lookup}" + )); + } + Err(error) if started.elapsed() >= READY_TIMEOUT => return Err(error), + Err(_) => {} + } + } + Ok(response) => { + return Err(format!( + "policy.local returned an invalid current policy: {response}" + )); + } + Err(error) => { + if started.elapsed() >= READY_TIMEOUT { + return Err(error); + } + } + } + sleep(POLL_INTERVAL).await; + } + let denials = request_policy_local(runner, &name, "/v1/denials?last=1").await?; + if !denials["denials"].is_array() || !denials["log_available"].is_boolean() { + return Err(format!( + "policy.local returned an invalid denials response: {denials}" + )); + } + + let rule_name = format!("conformance_local_{}", runner.id()); + let payload = serde_json::json!({ + "intent_summary": "Allow Bash to read the conformance path on example.invalid.", + "operations": [{ + "addRule": { + "ruleName": &rule_name, + "rule": { + "name": &rule_name, + "endpoints": [{ + "host": "example.invalid", + "port": 443, + "protocol": "rest", + "enforcement": "enforce", + "rules": [{"allow": {"method": "GET", "path": "/conformance"}}] + }], + "binaries": [{"path": &binary}] + } + } + }] + }) + .to_string(); + let submitted = + request_policy_local_http(runner, &name, "POST", "/v1/proposals", &payload, 202) + .await?; + let chunk_id = submitted["accepted_chunk_ids"] + .as_array() + .filter(|ids| ids.len() == 1) + .and_then(|ids| ids[0].as_str()) + .filter(|id| !id.is_empty()) + .ok_or_else(|| format!("policy.local did not accept one proposal: {submitted}"))?; + if submitted["status"] != "submitted" + || submitted["accepted_chunks"] != 1 + || submitted["rejected_chunks"] != 0 + { + return Err(format!("policy.local did not submit one rule: {submitted}")); + } + + let state = + request_policy_local(runner, &name, &format!("/v1/proposals/{chunk_id}")).await?; + if state["chunk_id"] != chunk_id + || state["rule_name"] != rule_name + || state["binary"] != binary + || !matches!(state["status"].as_str(), Some("pending" | "approved")) + { + return Err(format!("policy.local returned the wrong proposal: {state}")); + } + + let review = runner + .step("reviewer-inbox") + .description("the requested rule is visible to the reviewer") + .with_timeout(COMMAND_TIMEOUT) + .run(&["rule", "get", &name]) + .await + .map_err(|error| error.to_string())?; + review.require_success()?; + if !review.stdout().contains(&format!("Chunk: {chunk_id}")) + || !review.stdout().contains(&format!("Rule: {rule_name}")) + { + return Err(review.failure_diagnostic("the submitted rule is in the reviewer inbox")); + } + Ok(()) + }) +} + +async fn sandbox_bash_path(runner: &OpenShellRunner, name: &str) -> Result { + let result = runner + .step("bash-binary") + .description("the sandbox's Bash executable has a canonical path") + .with_timeout(COMMAND_TIMEOUT) + .run(&[ + "sandbox", + "exec", + "--name", + name, + "--no-tty", + "--", + "bash", + "-c", + "printf '%s\\n' \"$(readlink -f /proc/$$/exe)\"", + ]) + .await + .map_err(|error| error.to_string())?; + result.require_success()?; + let binary = result.stdout().trim(); + if !binary.starts_with('/') + || binary.contains('\n') + || binary.rsplit('/').next() != Some("bash") + { + return Err(result.failure_diagnostic("one absolute Bash executable path ending in /bash")); + } + Ok(binary.to_string()) +} + +async fn enable_proposals(runner: &OpenShellRunner, name: &str) -> Result<(), String> { + let set = runner + .step("enable-policy-advisor") + .description("sandbox-scoped policy advisor setting is enabled") + .with_timeout(COMMAND_TIMEOUT) + .run(&[ + "settings", + "set", + name, + "--key", + "agent_policy_proposals_enabled", + "--value", + "true", + ]) + .await + .map_err(|error| error.to_string())?; + set.require_success()?; + + let settings = runner + .step("effective-settings") + .description("policy advisor setting is effectively enabled") + .with_timeout(COMMAND_TIMEOUT) + .run(&["settings", "get", name, "--json"]) + .await + .map_err(|error| error.to_string())?; + settings.require_success()?; + let value: Value = settings.json().map_err(|error| error.to_string())?; + if value["settings"]["agent_policy_proposals_enabled"]["value"] != "true" { + return Err(settings.failure_diagnostic( + "effective agent_policy_proposals_enabled is true; check for a global override", + )); + } + Ok(()) +} + +async fn request_policy_local( + runner: &OpenShellRunner, + sandbox: &str, + path: &str, +) -> Result { + request_policy_local_http(runner, sandbox, "GET", path, "", 200).await +} + +async fn request_policy_local_http( + runner: &OpenShellRunner, + sandbox: &str, + method: &str, + path: &str, + body: &str, + expected_status: u16, +) -> Result { + let script = "method=$1; path=$2; body=$3; exec 3<>/dev/tcp/policy.local/80 || exit 1; printf '%s %s HTTP/1.1\\r\\nHost: policy.local\\r\\nContent-Type: application/json\\r\\nContent-Length: %s\\r\\nConnection: close\\r\\n\\r\\n' \"$method\" \"$path\" \"${#body}\" >&3; printf '%s' \"$body\" >&3; cat <&3"; + let result = runner + .step(format!("policy-local-{method}{path}")) + .description(format!( + "{method} http://policy.local{path} succeeds from the sandbox" + )) + .with_timeout(COMMAND_TIMEOUT) + .run(&[ + "sandbox", + "exec", + "--name", + sandbox, + "--no-tty", + "--", + "bash", + "-c", + script, + "policy-local-http", + method, + path, + body, + ]) + .await + .map_err(|error| error.to_string())?; + result.require_success()?; + let (headers, body) = result.stdout().split_once("\r\n\r\n").ok_or_else(|| { + result.failure_diagnostic("a complete HTTP response with headers and JSON body") + })?; + if !headers.starts_with(&format!("HTTP/1.1 {expected_status} ")) + || !headers.lines().any(|line| { + line.to_ascii_lowercase() + .starts_with("content-type: application/json") + }) + { + return Err( + result.failure_diagnostic(&format!("HTTP {expected_status} with JSON Content-Type")) + ); + } + serde_json::from_str(body) + .map_err(|error| result.failure_diagnostic(&format!("valid JSON body: {error}"))) +} + +fn run_mechanistic_proposal(runner: &mut OpenShellRunner) -> ScenarioFuture<'_> { + Box::pin(async move { + let mut policy = NamedTempFile::new().map_err(|error| error.to_string())?; + policy + .write_all( + br"version: 1 +filesystem_policy: + include_workdir: true + read_only: [/usr, /bin, /lib, /lib64, /proc, /dev/urandom, /app, /etc, /var/log] + read_write: [/sandbox, /tmp, /dev/null] +landlock: { compatibility: best_effort } +network_policies: {} +", + ) + .map_err(|error| error.to_string())?; + let policy_path = policy + .path() + .to_str() + .ok_or("temporary policy path is not UTF-8")?; + let name = format!("ct-{}-mp", runner.id()); + create_sandbox(runner, &name, Some(policy_path)).await?; + enable_proposals(runner, &name).await?; + + let effective = runner + .step("effective-policy") + .description("sandbox has no network allow rules before the probe") + .with_timeout(COMMAND_TIMEOUT) + .run(&["policy", "get", &name, "--full", "--output", "json"]) + .await + .map_err(|error| error.to_string())?; + effective.require_success()?; + let value: Value = effective.json().map_err(|error| error.to_string())?; + let network_rules = &value["policy"]["network_policies"]; + if !network_rules.is_null() + && !network_rules + .as_object() + .is_some_and(serde_json::Map::is_empty) + { + return Err(effective.failure_diagnostic("effective network_policies is empty")); + } + + let probe = runner + .step("denied-tcp-open") + .description("one Bash TCP open to 1.1.1.1:443 is denied by policy") + .with_timeout(COMMAND_TIMEOUT) + .run(&[ + "sandbox", + "exec", + "--name", + &name, + "--no-tty", + "--", + "bash", + "-c", + "printf 'BINARY=%s\\n' \"$(readlink -f /proc/$$/exe)\"; if exec 3<>/dev/tcp/1.1.1.1/443; then echo UNEXPECTED_ALLOWED; exit 1; else echo DENIED; fi", + ]) + .await + .map_err(|error| error.to_string())?; + probe.require_success()?; + let binary = probe + .stdout() + .lines() + .find_map(|line| line.strip_prefix("BINARY=")) + .filter(|binary| binary.starts_with('/') && binary.rsplit('/').next() == Some("bash")) + .ok_or_else(|| probe.failure_diagnostic("canonical Bash executable path is reported"))? + .to_string(); + if !probe.stdout().lines().any(|line| line == "DENIED") { + return Err(probe.failure_diagnostic("TCP open is denied before any upstream dial")); + } + + let started = Instant::now(); + loop { + let draft = runner + .step("mechanistic-draft") + .description("a single scoped mechanistic draft appears") + .with_timeout(COMMAND_TIMEOUT) + .run(&["rule", "get", &name]) + .await + .map_err(|error| error.to_string())?; + if !draft.success() { + if started.elapsed() >= PROPOSAL_TIMEOUT { + return Err(draft.failure_diagnostic("the reviewer inbox is readable")); + } + sleep(POLL_INTERVAL).await; + continue; + } + if !draft.stdout().contains("Chunk:") { + if started.elapsed() >= PROPOSAL_TIMEOUT { + return Err(draft.failure_diagnostic(&format!( + "one mechanistic draft for 1.1.1.1:443 and {binary}; probe stderr:\n{}", + probe.stderr() + ))); + } + sleep(POLL_INTERVAL).await; + continue; + } + assert_mechanistic_draft(draft.stdout(), &binary) + .map_err(|error| draft.failure_diagnostic(&error))?; + return Ok(()); + } + }) +} + +fn assert_mechanistic_draft(output: &str, binary: &str) -> Result<(), String> { + let fields = output.lines().map(str::trim).collect::>(); + let field = |name: &str| { + fields + .iter() + .find_map(|line| line.strip_prefix(name).map(str::trim)) + }; + if fields + .iter() + .filter(|line| line.starts_with("Chunk:")) + .count() + != 1 + || !matches!(field("Status:"), Some("pending" | "approved")) + || field("Rule:") != Some("allow_1_1_1_1_443") + || field("Binary:") != Some(binary) + || field("Binaries:") != Some(binary) + || field("Endpoints:") != Some("1.1.1.1:443 [L4]") + || !field("Rationale:").is_some_and(|value| value.contains("1.1.1.1:443")) + { + return Err(format!( + "expected one pending or approved L4 mechanistic draft scoped to {binary} and 1.1.1.1:443" + )); + } + Ok(()) +} + +async fn create_sandbox( + runner: &mut OpenShellRunner, + name: &str, + policy_path: Option<&str>, +) -> Result<(), String> { + runner.track_sandbox(name); + let mut args = vec![ + "sandbox", + "create", + "--name", + name, + "--detach", + "--no-tty", + "--no-auto-providers", + ]; + if let Some(path) = policy_path { + args.extend(["--policy", path]); + } + args.extend(["--", "sh", "-c", "exec sleep infinity"]); + let create = runner + .step("create") + .description(format!("sandbox '{name}' is created")) + .with_timeout(CREATE_TIMEOUT) + .run(&args) + .await + .map_err(|error| error.to_string())?; + create.require_success()?; + + let started = Instant::now(); + loop { + let get = runner + .step("ready") + .description(format!("sandbox '{name}' reaches Ready")) + .with_timeout(COMMAND_TIMEOUT) + .run(&["sandbox", "get", name, "--output", "json"]) + .await + .map_err(|error| error.to_string())?; + if get.success() { + let state: SandboxState = get.json().map_err(|error| error.to_string())?; + if state.name != name { + return Err(get.failure_diagnostic("sandbox get returns the created name")); + } + if state.phase == "Ready" { + return Ok(()); + } + if state.phase == "Failed" { + return Err(get.failure_diagnostic("sandbox reaches Ready instead of Failed")); + } + } + if started.elapsed() >= READY_TIMEOUT { + return Err(get.failure_diagnostic("sandbox reaches Ready before timeout")); + } + sleep(POLL_INTERVAL).await; + } +} + +#[cfg(test)] +mod tests { + use super::assert_mechanistic_draft; + + #[test] + fn draft_assertion_rejects_unrelated_binary() { + let draft = "Chunk: id\nStatus: pending\nRule: allow_1_1_1_1_443\nBinary: /usr/bin/sh\nRationale: Allow sh to connect to 1.1.1.1:443.\nEndpoints: 1.1.1.1:443 [L4]\nBinaries: /usr/bin/sh\n"; + assert!(assert_mechanistic_draft(draft, "/usr/bin/bash").is_err()); + } +} diff --git a/crates/openshell-core/src/grpc_client.rs b/crates/openshell-core/src/grpc_client.rs index 46b2b0dd56..f49466b0e2 100644 --- a/crates/openshell-core/src/grpc_client.rs +++ b/crates/openshell-core/src/grpc_client.rs @@ -447,7 +447,10 @@ async fn refresh_token_loop( ) { let mut client = OpenShellClient::new(channel); loop { - let sleep = compute_refresh_delay(&slot); + let Some(sleep) = compute_refresh_delay(&slot) else { + debug!("gateway sandbox JWT does not expire; stopping periodic renewal"); + return; + }; tokio::time::sleep(sleep).await; match client .refresh_sandbox_token(RefreshSandboxTokenRequest { @@ -653,9 +656,10 @@ async fn refresh_extension_credentials_with_client( /// Compute the next refresh delay: 80 % of the time remaining until the /// current token's `exp`, plus up to 10 % jitter, with a small lower bound -/// for already-expired tokens and capped at 12 h. If the token can't be parsed -/// (for example, an opaque bootstrap bearer), default to 6 h. -fn compute_refresh_delay(slot: &TokenSlot) -> Duration { +/// for already-expired tokens and capped at 12 h. An `exp` of zero denotes a +/// non-expiring session credential and needs no periodic refresh. If the token +/// can't be parsed (for example, an opaque bootstrap bearer), default to 6 h. +fn compute_refresh_delay(slot: &TokenSlot) -> Option { let token = slot .read() .ok() @@ -668,7 +672,11 @@ fn compute_refresh_delay(slot: &TokenSlot) -> Duration { .map_or(0, |d| d.as_millis()), ) .unwrap_or(i64::MAX); - let mut delay_ms = parse_jwt_exp_ms(bearer).map_or(21_600_000, |exp| { + let expires_at = parse_jwt_exp_ms(bearer); + if expires_at == Some(0) { + return None; + } + let mut delay_ms = expires_at.map_or(21_600_000, |exp| { let remaining_ms = exp - now_ms; if remaining_ms <= 0 { 1_000 @@ -681,7 +689,7 @@ fn compute_refresh_delay(slot: &TokenSlot) -> Duration { let jitter_pct = (token.len() % 10) as u64; let jitter_ms = (u64::try_from(delay_ms).unwrap_or(0) * jitter_pct) / 100; delay_ms = delay_ms.saturating_add(i64::try_from(jitter_ms).unwrap_or(0)); - Duration::from_millis(u64::try_from(delay_ms).unwrap_or(0)) + Some(Duration::from_millis(u64::try_from(delay_ms).unwrap_or(0))) } /// Decode the `exp` claim from a JWT without verifying its signature. @@ -794,7 +802,7 @@ mod auth_tests { let token = format!("h.{payload}.s"); let bearer = AsciiMetadataValue::try_from(format!("Bearer {token}")).unwrap(); let slot: TokenSlot = Arc::new(RwLock::new(bearer)); - let delay = compute_refresh_delay(&slot); + let delay = compute_refresh_delay(&slot).expect("expiring token needs refresh"); // 800 s baseline + up to 10 % jitter → 800..=880 s, with some slack // for the 1-second resolution of the exp claim. let secs = delay.as_secs(); @@ -815,19 +823,18 @@ mod auth_tests { let token = format!("h.{payload}.s"); let bearer = AsciiMetadataValue::try_from(format!("Bearer {token}")).unwrap(); let slot: TokenSlot = Arc::new(RwLock::new(bearer)); - let delay = compute_refresh_delay(&slot); + let delay = compute_refresh_delay(&slot).expect("expired token needs refresh"); assert!((1..60).contains(&delay.as_secs())); } #[test] - fn compute_refresh_delay_treats_exp_zero_as_expired() { + fn compute_refresh_delay_skips_non_expiring_token() { use base64::Engine as _; let payload = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(r#"{"exp":0}"#); let token = format!("h.{payload}.s"); let bearer = AsciiMetadataValue::try_from(format!("Bearer {token}")).unwrap(); let slot: TokenSlot = Arc::new(RwLock::new(bearer)); - let delay = compute_refresh_delay(&slot); - assert!((1..60).contains(&delay.as_secs())); + assert_eq!(compute_refresh_delay(&slot), None); } #[test] @@ -843,7 +850,7 @@ mod auth_tests { let token = format!("h.{payload}.s"); let bearer = AsciiMetadataValue::try_from(format!("Bearer {token}")).unwrap(); let slot: TokenSlot = Arc::new(RwLock::new(bearer)); - let delay = compute_refresh_delay(&slot); + let delay = compute_refresh_delay(&slot).expect("expiring token needs refresh"); assert!( delay.as_secs() < 30, "expected refresh before 30s expiry, got {delay:?}", diff --git a/crates/openshell-isolation-interface/src/contract.rs b/crates/openshell-isolation-interface/src/contract.rs index da40e92cb5..ab3e10752f 100644 --- a/crates/openshell-isolation-interface/src/contract.rs +++ b/crates/openshell-isolation-interface/src/contract.rs @@ -716,6 +716,9 @@ pub struct ExecSession { pub stderr: Option, /// PTY control, present when a terminal was requested. pub terminal: Option>, + /// Final status of the attached output stream, including delivery failure. + /// The process handle's stable wait status remains independent of attachment. + pub output_status: Option>, } /// What to run inside the boundary via [`BoundaryExec`]. diff --git a/crates/openshell-isolation-interface/tests/backend_conformance.rs b/crates/openshell-isolation-interface/tests/backend_conformance.rs index f40cdcd45a..2fb1f3fdba 100644 --- a/crates/openshell-isolation-interface/tests/backend_conformance.rs +++ b/crates/openshell-isolation-interface/tests/backend_conformance.rs @@ -193,6 +193,7 @@ impl BoundaryExec for MockExec { stdout: Box::new(out_r), stderr: (!spec.pty).then_some(stderr), terminal: spec.pty.then_some(terminal), + output_status: None, }) } } diff --git a/crates/openshell-sandbox-backend/src/runtime.rs b/crates/openshell-sandbox-backend/src/runtime.rs index 3ed8ed6533..f073c6d7ce 100644 --- a/crates/openshell-sandbox-backend/src/runtime.rs +++ b/crates/openshell-sandbox-backend/src/runtime.rs @@ -35,8 +35,8 @@ use tokio::net::UnixStream; use tokio_stream::wrappers::ReceiverStream; use crate::boundary_protocol::{ - AgentSpecWire, DnsQueryResultWire, ExecSpecWire, MAX_CONTROL_FRAME_BYTES, Request, - RequestEnvelope, Response, ResponseEnvelope, STREAM_EXIT, STREAM_STDERR, STREAM_STDIN, + AgentSpecWire, DnsQueryResultWire, ExecSpecWire, ExitStatusWire, MAX_CONTROL_FRAME_BYTES, + Request, RequestEnvelope, Response, ResponseEnvelope, STREAM_EXIT, STREAM_STDERR, STREAM_STDIN, STREAM_STDIN_CLOSED, STREAM_STDOUT, SandboxPolicyWire, SandboxRuntimeDescriptor, SandboxTlsClientConfig, SandboxTransport, SignalWire, decode_frame, encode_frame, read_stream_frame, validate_resource_claims, write_stream_frame, @@ -549,6 +549,7 @@ async fn open_process_attachment( network_reader, stdout_pump, stderr_pump, + None, )); let terminal: Option> = if has_terminal { let terminal: Arc = Arc::new(RemoteTerminal { client, process_id }); @@ -574,22 +575,29 @@ async fn pump_process_responses( mut network: tokio::io::ReadHalf, mut stdout: tokio::io::DuplexStream, mut stderr: tokio::io::DuplexStream, + completion: Option>, ) { - loop { + let status = loop { match read_stream_frame(&mut network).await { Ok(Some((STREAM_STDOUT, payload))) => { if stdout.write_all(&payload).await.is_err() { - return; + break BoundaryExitStatus::Exited(74); } } Ok(Some((STREAM_STDERR, payload))) => { if stderr.write_all(&payload).await.is_err() { - return; + break BoundaryExitStatus::Exited(74); } } - Ok(Some((STREAM_EXIT, _)) | None) | Err(_) => return, - Ok(Some((_channel, _))) => return, + Ok(Some((STREAM_EXIT, payload))) => { + break serde_json::from_slice::(&payload) + .map_or(BoundaryExitStatus::Exited(74), Into::into); + } + Ok(None | Some(_)) | Err(_) => break BoundaryExitStatus::Exited(74), } + }; + if let Some(completion) = completion { + let _ = completion.send(status); } } @@ -770,11 +778,13 @@ async fn open_exec_session( let (stdin, stdin_pump) = tokio::io::duplex(64 * 1024); let (stdout, stdout_pump) = tokio::io::duplex(64 * 1024); let (stderr, stderr_pump) = tokio::io::duplex(64 * 1024); + let (output_status_tx, output_status_rx) = tokio::sync::oneshot::channel(); tokio::spawn(pump_exec_input(stdin_pump, network_writer)); tokio::spawn(pump_process_responses( network_reader, stdout_pump, stderr_pump, + Some(output_status_tx), )); let process: Arc = Arc::new(RemoteExecProcess { @@ -795,6 +805,7 @@ async fn open_exec_session( stdout, stderr, terminal, + output_status: Some(output_status_rx), }) } @@ -3139,7 +3150,7 @@ mod tests { tls_runtime_descriptor(address, certificate.client_tls), test_bearer(&"a".repeat(32)), )); - let session = open_exec_session( + let mut session = open_exec_session( client, ExecSpec { program: "/bin/true".to_string(), @@ -3155,9 +3166,15 @@ mod tests { .unwrap(); // The test peer closes its I/O stream without an exit frame. Neither // that loss nor a dropped reader can invalidate the process handle. + let output_status = session.output_status.take().unwrap(); drop(session.stdin); drop(session.stdout); drop(session.stderr); + assert_eq!( + output_status.await.unwrap(), + BoundaryExitStatus::Exited(74), + "missing stream exit must fail even when the process exits cleanly" + ); let attachment = session.process.attach().await.unwrap(); drop(attachment); for _ in 0..2 { @@ -3169,6 +3186,34 @@ mod tests { server.abort(); } + #[tokio::test] + async fn exec_output_completion_preserves_failure_status() { + let (network, mut peer) = tokio::io::duplex(1024); + let network: BoundaryDuplexStream = Box::new(network); + let (reader, _writer) = tokio::io::split(network); + let (stdout, mut output) = tokio::io::duplex(1024); + let (stderr, _error_output) = tokio::io::duplex(1024); + let (completion_tx, completion_rx) = tokio::sync::oneshot::channel(); + let pump = tokio::spawn(pump_process_responses( + reader, + stdout, + stderr, + Some(completion_tx), + )); + write_stream_frame(&mut peer, STREAM_STDOUT, b"hello") + .await + .unwrap(); + let status = serde_json::to_vec(&ExitStatusWire::Exited(74)).unwrap(); + write_stream_frame(&mut peer, STREAM_EXIT, &status) + .await + .unwrap(); + let mut bytes = [0; 5]; + output.read_exact(&mut bytes).await.unwrap(); + assert_eq!(&bytes, b"hello"); + assert_eq!(completion_rx.await.unwrap(), BoundaryExitStatus::Exited(74)); + pump.await.unwrap(); + } + struct TestTlsIo(BoundaryDuplexStream); impl tokio::io::AsyncRead for TestTlsIo { diff --git a/crates/openshell-sandbox/src/boundary_exec.rs b/crates/openshell-sandbox/src/boundary_exec.rs index f4f5a13f3f..8d0a3326ed 100644 --- a/crates/openshell-sandbox/src/boundary_exec.rs +++ b/crates/openshell-sandbox/src/boundary_exec.rs @@ -117,6 +117,7 @@ impl LocalBoundaryExec { stdout: Box::new(stdout), stderr: Some(Box::new(stderr)), terminal: None, + output_status: None, }) } @@ -290,6 +291,7 @@ impl LocalBoundaryExec { stdout, stderr, terminal: None, + output_status: None, }), process, armed: true, @@ -386,6 +388,7 @@ impl LocalBoundaryExec { stdout: Box::new(tokio::fs::File::from_std(output)), stderr: None, terminal: Some(terminal), + output_status: None, }), process, armed: true, diff --git a/crates/openshell-sandbox/src/boundary_server.rs b/crates/openshell-sandbox/src/boundary_server.rs index e6dfba1d92..7a2a73f731 100644 --- a/crates/openshell-sandbox/src/boundary_server.rs +++ b/crates/openshell-sandbox/src/boundary_server.rs @@ -1448,6 +1448,9 @@ mod linux { impl MainAttachment { fn exit_status(&self, fallback_code: i32) -> ExitStatusWire { + if self.session.output_failed() { + return ExitStatusWire::Exited(74); + } match &self.status { AttachmentStatus::Main(process) => process .exit_status() @@ -2076,6 +2079,7 @@ mod linux { stdout, stderr, terminal, + output_status: _, } = session; let Some(stdin) = stdin else { return Err(guest_error( @@ -3002,15 +3006,14 @@ mod linux { while let Some((channel, payload)) = read_stream_frame(&mut reader).await? { match channel { STREAM_STDIN => { - let Some(input) = input.as_ref() else { - return Err(io::Error::new( - io::ErrorKind::BrokenPipe, - "main process stdin already closed", - )); - }; - input.send(payload).await.map_err(|_| { - io::Error::new(io::ErrorKind::BrokenPipe, "main process stdin closed") - })?; + // A process may close stdin before the client has + // finished sending it. Keep relaying stdout, stderr, + // and the final status after that write fails. + if let Some(sender) = input.as_ref() + && sender.send(payload).await.is_err() + { + input.take(); + } } // Keep reading after stdin closes so transport EOF still // releases this control process's attachment lease. @@ -3052,6 +3055,21 @@ mod linux { .map_err(|error| format!("write main process exit: {error}")); } Err(error) => { + if matches!(&attachment.status, AttachmentStatus::Exec(_)) { + tracing::warn!( + skipped_chunks = error.skipped, + "exec output could not be delivered intact" + ); + let message = b"openshell: exec output could not be delivered intact\n"; + let _ = + write_stream_frame(&mut *writer.lock().await, STREAM_STDERR, message) + .await; + let status = serde_json::to_vec(&ExitStatusWire::Exited(74)) + .map_err(|error| format!("encode exec output failure: {error}"))?; + break write_stream_frame(&mut *writer.lock().await, STREAM_EXIT, &status) + .await + .map_err(|error| format!("write exec output failure: {error}")); + } tracing::warn!( skipped_chunks = error.skipped, "main process attachment resumed after dropping retained output" diff --git a/crates/openshell-sandbox/src/main_session.rs b/crates/openshell-sandbox/src/main_session.rs index 898cfeaed3..334c6e7223 100644 --- a/crates/openshell-sandbox/src/main_session.rs +++ b/crates/openshell-sandbox/src/main_session.rs @@ -52,26 +52,44 @@ struct OutputLogState { events: VecDeque, retained_bytes: usize, next_sequence: u64, + consumed_through: u64, } #[derive(Debug)] struct OutputLog { state: Mutex, version: watch::Sender, + /// Exec output must not discard bytes while its consumer is slow. + lossless: bool, + space_version: watch::Sender, + failed: AtomicBool, + post_exit: AtomicBool, + subscribers: AtomicUsize, terminal_reported: AtomicBool, terminal_reported_notify: Notify, } impl OutputLog { fn new() -> Arc { + Self::with_lossless(false) + } + + fn with_lossless(lossless: bool) -> Arc { let (version, _) = watch::channel(0); + let (space_version, _) = watch::channel(0); Arc::new(Self { state: Mutex::new(OutputLogState { events: VecDeque::new(), retained_bytes: 0, next_sequence: 0, + consumed_through: 0, }), version, + lossless, + space_version, + failed: AtomicBool::new(false), + post_exit: AtomicBool::new(false), + subscribers: AtomicUsize::new(0), terminal_reported: AtomicBool::new(false), terminal_reported_notify: Notify::new(), }) @@ -98,14 +116,107 @@ impl OutputLog { self.version.send_replace(version); } + /// Bound memory by making the process reader wait for the attachment. + /// A stalled or absent attachment eventually fails the exec rather than + /// allowing a successful result with missing output. + async fn publish_lossless(&self, event: MainOutput) -> bool { + self.publish_lossless_with_timeout(event, Self::STALL_TIMEOUT) + .await + } + + const STALL_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); + + fn fail(&self) { + self.failed.store(true, Ordering::Release); + self.space_version.send_modify(|version| *version += 1); + } + + fn stop_retaining_after_exit(&self) { + let state = self.state.lock().expect("main output log lock poisoned"); + let changed = !self.post_exit.swap(true, Ordering::AcqRel); + drop(state); + if changed { + self.space_version.send_modify(|version| *version += 1); + } + } + + async fn publish_lossless_with_timeout( + &self, + event: MainOutput, + stall_timeout: std::time::Duration, + ) -> bool { + // A descendant may keep the inherited pipe open after its parent + // exits. Drain that pipe without retaining unbounded post-exit output. + if self.post_exit.load(Ordering::Acquire) { + return true; + } + let mut space = self.space_version.subscribe(); + loop { + let _ = space.borrow_and_update(); + if self.failed.load(Ordering::Acquire) { + return false; + } + let published = { + let mut state = self.state.lock().expect("main output log lock poisoned"); + while state.retained_bytes + event.len() > OUTPUT_BUFFER_BYTES + && state + .events + .front() + .is_some_and(|front| front.sequence < state.consumed_through) + { + let removed = state.events.pop_front().expect("front was checked"); + state.retained_bytes -= removed.event.len(); + } + if state.retained_bytes + event.len() > OUTPUT_BUFFER_BYTES { + None + } else { + let sequence = state.next_sequence; + state.next_sequence = state + .next_sequence + .checked_add(1) + .expect("main output sequence exhausted"); + state.retained_bytes += event.len(); + state.events.push_back(SequencedOutput { + sequence, + event: event.clone(), + }); + Some(state.next_sequence) + } + }; + if let Some(version) = published { + self.version.send_replace(version); + return true; + } + let timed_out = if self.subscribers.load(Ordering::Acquire) == 0 { + tokio::time::timeout(stall_timeout, space.changed()) + .await + .is_err() + } else { + let _ = space.changed().await; + false + }; + if timed_out && self.subscribers.load(Ordering::Acquire) == 0 { + self.fail(); + return false; + } + } + } + fn subscribe(self: &Arc) -> MainOutputCursor { let version = self.version.subscribe(); - let state = self.state.lock().expect("main output log lock poisoned"); + let mut state = self.state.lock().expect("main output log lock poisoned"); let next_sequence = state .events .front() .map_or(state.next_sequence, |retained| retained.sequence); + if self.lossless { + state.consumed_through = next_sequence; + self.subscribers.fetch_add(1, Ordering::AcqRel); + } drop(state); + if self.lossless { + self.space_version.send_modify(|version| *version += 1); + } MainOutputCursor { output: Arc::clone(self), next_sequence, @@ -139,11 +250,25 @@ pub struct MainOutputCursor { version: watch::Receiver, } +impl Drop for MainOutputCursor { + fn drop(&mut self) { + if self.output.lossless { + self.output.subscribers.fetch_sub(1, Ordering::AcqRel); + self.output + .space_version + .send_modify(|version| *version += 1); + } + } +} + impl MainOutputCursor { pub async fn recv(&mut self) -> Result { loop { + if self.output.lossless && self.output.failed.load(Ordering::Acquire) { + return Err(MainOutputLagged { skipped: 0 }); + } let next = { - let state = self + let mut state = self .output .state .lock() @@ -169,6 +294,12 @@ impl MainOutputCursor { .event .clone(); self.next_sequence += 1; + if self.output.lossless { + state.consumed_through = self.next_sequence; + self.output + .space_version + .send_modify(|version| *version += 1); + } Some(event) } }; @@ -224,7 +355,7 @@ pub struct MainSession { } impl MainSession { - const REMOTE_OUTPUT_DRAIN_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(2); + const REMOTE_OUTPUT_DRAIN_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); #[cfg(test)] pub fn inert() -> Arc { let (input, _input_rx) = tokio::sync::mpsc::channel(64); @@ -327,7 +458,7 @@ impl MainSession { pid: 0, terminal: terminal_mode, input: MainInputSender { sender: input }, - output: OutputLog::new(), + output: OutputLog::with_lossless(true), input_owner: Mutex::new(None), input_closed: AtomicBool::new(false), next_owner: AtomicU64::new(1), @@ -351,8 +482,21 @@ impl MainSession { loop { match stdout.read(&mut buffer).await { Ok(0) | Err(_) => break, - Ok(read) => stdout_session - .publish(MainOutput::Stdout(Bytes::copy_from_slice(&buffer[..read]))), + Ok(read) => { + if !stdout_session + .output + .publish_lossless(MainOutput::Stdout(Bytes::copy_from_slice( + &buffer[..read], + ))) + .await + && !stdout_session.output.post_exit.load(Ordering::Acquire) + { + if let Some(process) = stdout_session.boundary_process.as_ref() { + let _ = process.terminate().await; + } + break; + } + } } } stdout_session.reader_finished(); @@ -364,8 +508,21 @@ impl MainSession { loop { match stderr.read(&mut buffer).await { Ok(0) | Err(_) => break, - Ok(read) => stderr_session - .publish(MainOutput::Stderr(Bytes::copy_from_slice(&buffer[..read]))), + Ok(read) => { + if !stderr_session + .output + .publish_lossless(MainOutput::Stderr(Bytes::copy_from_slice( + &buffer[..read], + ))) + .await + && !stderr_session.output.post_exit.load(Ordering::Acquire) + { + if let Some(process) = stderr_session.boundary_process.as_ref() { + let _ = process.terminate().await; + } + break; + } + } } } stderr_session.reader_finished(); @@ -527,12 +684,37 @@ impl MainSession { attachment_expected: bool, timeout: std::time::Duration, ) -> bool { - let _ = tokio::time::timeout(timeout, self.wait_for_output_readers()).await; - self.complete_finish(exit_code, attachment_expected) + if self.output.lossless { + // The pipe can contain parent bytes after the process exits. If + // descendants keep it open beyond the drain deadline, report a + // delivery failure rather than guessing where parent output ends. + if tokio::time::timeout(timeout, self.wait_for_output_readers()) + .await + .is_err() + { + self.output.stop_retaining_after_exit(); + self.output.fail(); + } + } else { + let _ = tokio::time::timeout(timeout, self.wait_for_output_readers()).await; + } + let code = if self.output.failed.load(Ordering::Acquire) { + 74 + } else { + exit_code + }; + self.complete_finish(code, attachment_expected) + } + + #[must_use] + pub fn output_failed(&self) -> bool { + self.output.failed.load(Ordering::Acquire) } async fn wait_for_output_readers(&self) { let notified = self.readers_done.notified(); + tokio::pin!(notified); + notified.as_mut().enable(); if self.readers_remaining.load(Ordering::Acquire) != 0 { notified.await; } @@ -968,6 +1150,171 @@ mod tests { )); } + #[tokio::test] + async fn lossless_finish_keeps_parent_status_when_output_pipe_closes() { + let mut session = MainSession::inert(); + let inner = Arc::get_mut(&mut session).expect("sole test session reference"); + inner.output = OutputLog::with_lossless(true); + inner.readers_remaining.store(1, Ordering::Release); + let mut output = session.subscribe(); + assert!( + session + .output + .publish_lossless(MainOutput::Stdout(Bytes::from_static(b"ok\n"))) + .await + ); + assert!(matches!( + output.recv().await.unwrap(), + MainOutput::Stdout(data) if data == b"ok\n"[..] + )); + session.reader_finished(); + + session + .finish_remote_with_timeout(0, false, std::time::Duration::from_millis(10)) + .await; + assert!(!session.output_failed()); + assert!(matches!(output.recv().await.unwrap(), MainOutput::Exit(0))); + } + + #[tokio::test] + async fn lossless_finish_preserves_pending_output_for_paused_reader() { + let mut session = MainSession::inert(); + let inner = Arc::get_mut(&mut session).expect("sole test session reference"); + inner.output = OutputLog::with_lossless(true); + inner.readers_remaining.store(1, Ordering::Release); + let mut output = session.subscribe(); + assert!( + session + .output + .publish_lossless(MainOutput::Stdout(Bytes::from_static(b"pending"))) + .await + ); + session.reader_finished(); + let wait_session = session.clone(); + let finish = tokio::spawn(async move { + wait_session + .finish_remote_with_timeout(0, false, std::time::Duration::from_millis(10)) + .await; + }); + tokio::time::timeout(std::time::Duration::from_secs(1), finish) + .await + .expect("paused reader must not extend the drain indefinitely") + .unwrap(); + assert!(matches!( + output.recv().await.unwrap(), + MainOutput::Stdout(data) if data == b"pending"[..] + )); + assert!(!session.output_failed()); + assert!(matches!(output.recv().await.unwrap(), MainOutput::Exit(0))); + } + + #[tokio::test] + async fn lossless_finish_fails_when_descendant_keeps_writing() { + let mut session = MainSession::inert(); + let inner = Arc::get_mut(&mut session).expect("sole test session reference"); + inner.output = OutputLog::with_lossless(true); + inner.readers_remaining.store(1, Ordering::Release); + let mut output = session.subscribe(); + let writer_log = session.output.clone(); + let writer = tokio::spawn(async move { + loop { + assert!( + writer_log + .publish_lossless(MainOutput::Stdout(Bytes::from_static(b"tick\n"))) + .await + ); + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + } + }); + let reader = tokio::spawn(async move { + loop { + match output.recv().await { + Ok(MainOutput::Exit(code)) => return code, + Err(_) => return 74, + Ok(_) => {} + } + } + }); + + tokio::time::timeout( + std::time::Duration::from_secs(1), + session.finish_remote_with_timeout(0, false, std::time::Duration::from_millis(30)), + ) + .await + .expect("descendant output must not extend the drain deadline"); + assert_eq!(reader.await.unwrap(), 74); + writer.abort(); + assert!(session.output_failed()); + } + + #[tokio::test] + async fn lossless_failed_exec_ignores_later_descendant_output() { + let mut session = MainSession::inert(); + let inner = Arc::get_mut(&mut session).expect("sole test session reference"); + inner.output = OutputLog::with_lossless(true); + inner.readers_remaining.store(1, Ordering::Release); + let mut output = session.subscribe(); + session + .finish_remote_with_timeout(0, false, std::time::Duration::from_millis(10)) + .await; + assert!(output.recv().await.is_err()); + drop(output); + let window = session.output_window(); + for _ in 0..(2 * OUTPUT_BUFFER_BYTES / 4096) { + assert!( + session + .output + .publish_lossless(MainOutput::Stdout(Bytes::from(vec![b'x'; 4096]))) + .await + ); + } + assert_eq!(session.output_window(), window); + assert!(session.output_failed()); + } + + #[tokio::test] + async fn lossless_finish_drains_large_output_before_success() { + let mut session = MainSession::inert(); + let inner = Arc::get_mut(&mut session).expect("sole test session reference"); + inner.output = OutputLog::with_lossless(true); + inner.readers_remaining.store(1, Ordering::Release); + let mut output = session.subscribe(); + let writer_log = session.output.clone(); + let writer_session = session.clone(); + let writer = tokio::spawn(async move { + for _ in 0..(2 * OUTPUT_BUFFER_BYTES / 4096) { + assert!( + writer_log + .publish_lossless(MainOutput::Stdout(Bytes::from(vec![b'x'; 4096]))) + .await + ); + } + writer_session.reader_finished(); + }); + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + let finish_session = session.clone(); + let finish = tokio::spawn(async move { + finish_session + .finish_remote_with_timeout(0, false, std::time::Duration::from_secs(5)) + .await; + }); + let mut bytes = 0; + loop { + match output.recv().await.unwrap() { + MainOutput::Stdout(data) => { + bytes += data.len(); + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + } + MainOutput::Exit(0) => break, + other => panic!("unexpected output: {other:?}"), + } + } + writer.await.unwrap(); + finish.await.unwrap(); + assert_eq!(bytes, 2 * OUTPUT_BUFFER_BYTES); + assert!(!session.output_failed()); + } + #[tokio::test] async fn declared_attachment_waits_for_connection_then_natural_close() { let session = MainSession::inert(); @@ -1024,6 +1371,72 @@ mod tests { )); } + #[tokio::test] + async fn exec_output_waits_for_a_slow_subscriber_without_dropping_bytes() { + let log = OutputLog::with_lossless(true); + let chunk = Bytes::from(vec![b'x'; 4096]); + let writer_log = log.clone(); + let writer = tokio::spawn(async move { + for _ in 0..(4 * OUTPUT_BUFFER_BYTES / chunk.len()) { + assert!( + writer_log + .publish_lossless(MainOutput::Stdout(chunk.clone())) + .await + ); + } + }); + + // Let the producer fill the bounded queue before attaching a reader. + tokio::time::sleep(std::time::Duration::from_millis(20)).await; + assert!(!writer.is_finished()); + let mut reader = log.subscribe(); + let read = async { + let mut total = 0; + while total < 4 * OUTPUT_BUFFER_BYTES { + match reader + .recv() + .await + .expect("exec output must not be skipped") + { + MainOutput::Stdout(data) => { + assert!(data.iter().all(|byte| *byte == b'x')); + total += data.len(); + tokio::task::yield_now().await; + } + other => panic!("unexpected output: {other:?}"), + } + } + total + }; + let bytes = tokio::time::timeout(std::time::Duration::from_secs(5), read) + .await + .expect("lossless output stalled"); + writer.await.expect("writer failed"); + assert_eq!(bytes, 4 * OUTPUT_BUFFER_BYTES); + assert!(!log.failed.load(Ordering::Acquire)); + let mut replay = log.subscribe(); + assert!(matches!( + replay.recv().await.expect("recent exec output remains available"), + MainOutput::Stdout(data) if data == b"x".repeat(4096) + )); + } + + #[tokio::test] + async fn exec_output_stall_is_reported_as_failure() { + let log = OutputLog::with_lossless(true); + let event = MainOutput::Stdout(Bytes::from(vec![b'x'; 4096])); + for _ in 0..(OUTPUT_BUFFER_BYTES / 4096) { + assert!(log.publish_lossless(event.clone()).await); + } + // An attachment that never consumes output must not permit a success. + assert!( + !log.publish_lossless_with_timeout(event, std::time::Duration::from_millis(10)) + .await + ); + let mut reader = log.subscribe(); + assert!(reader.recv().await.is_err()); + } + #[tokio::test] async fn terminal_pump_reads_output_and_writes_input() { let (session, mut slave) = MainSession::terminal_for_test(); diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index bc0f1d32e0..d8263e2eb4 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -3218,16 +3218,17 @@ fn exec_ssh_client_config() -> russh::client::Config { } } -/// Treat channel EOF before an exit status as relay failure, not exit code 1. -fn exec_loop_result(exit_code: Option) -> Result { - exit_code.map_or_else( - || { - Err(Status::unavailable( - "exec relay closed before the command reported an exit status", - )) - }, - Ok, - ) +/// Require both the command status and channel close before reporting success. +fn exec_loop_result(exit_code: Option, close_seen: bool) -> Result { + match (exit_code, close_seen) { + (Some(code), true) => Ok(code), + (None, _) => Err(Status::unavailable( + "exec relay closed before the command reported an exit status", + )), + (Some(_), false) => Err(Status::unavailable( + "exec relay closed before the SSH channel finished", + )), + } } fn build_remote_exec_command(req: &ExecSandboxRequest) -> Result { @@ -3576,6 +3577,7 @@ async fn run_interactive_exec_with_russh( let output = async { let mut exit_code: Option = None; + let mut close_seen = false; loop { // Bound the post-ExitStatus wait against a lost Close. let msg = if exit_code.is_some() { @@ -3618,12 +3620,15 @@ async fn run_interactive_exec_with_russh( let converted = i32::try_from(exit_status).unwrap_or(i32::MAX); exit_code = Some(converted); } - ChannelMsg::Close => break, + ChannelMsg::Close => { + close_seen = true; + break; + } _ => {} } } - exec_loop_result(exit_code) + exec_loop_result(exit_code, close_seen) }; let result = { @@ -3802,6 +3807,7 @@ async fn run_exec_with_russh( .map_err(|e| Status::internal(format!("failed to close ssh stdin: {e}")))?; let mut exit_code: Option = None; + let mut close_seen = false; loop { // Bound the post-ExitStatus wait against a lost Close. let msg = if exit_code.is_some() { @@ -3842,7 +3848,10 @@ async fn run_exec_with_russh( let converted = i32::try_from(exit_status).unwrap_or(i32::MAX); exit_code = Some(converted); } - ChannelMsg::Close => break, + ChannelMsg::Close => { + close_seen = true; + break; + } _ => {} } } @@ -3852,7 +3861,7 @@ async fn run_exec_with_russh( .disconnect(russh::Disconnect::ByApplication, "exec complete", "en") .await; - exec_loop_result(exit_code) + exec_loop_result(exit_code, close_seen) } // --------------------------------------------------------------------------- @@ -3863,6 +3872,13 @@ async fn run_exec_with_russh( mod tests { use super::*; use crate::compute::NoopTestDriver; + + #[test] + fn exec_requires_ssh_channel_close_after_exit_status() { + assert_eq!(exec_loop_result(Some(0), true).unwrap(), 0); + assert!(exec_loop_result(Some(0), false).is_err()); + assert!(exec_loop_result(None, true).is_err()); + } use crate::grpc::test_support::{ authed_request, test_server_state, test_server_state_with_compute_driver, test_server_state_with_driver, diff --git a/crates/openshell-supervisor-network/src/policy_dns/mod.rs b/crates/openshell-supervisor-network/src/policy_dns/mod.rs index 7033e6d6fc..a57caec2e7 100644 --- a/crates/openshell-supervisor-network/src/policy_dns/mod.rs +++ b/crates/openshell-supervisor-network/src/policy_dns/mod.rs @@ -46,6 +46,9 @@ use std::time::{Duration, Instant}; pub(crate) const MIN_MAPPING_TTL: Duration = Duration::from_secs(1); pub(crate) const MAX_MAPPING_TTL: Duration = Duration::from_secs(30); +/// Reserved within the first synthetic IPv4 pool for the sandbox-local API. +/// It is never published as an external endpoint mapping. +pub(crate) const POLICY_LOCAL_ADDRESS: std::net::Ipv4Addr = std::net::Ipv4Addr::new(198, 18, 0, 1); #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct SyntheticAnswer { @@ -110,6 +113,19 @@ impl PolicyDnsService { ) -> Result { let normalized_name = NormalizedName::parse(raw_name).map_err(|_| PolicyDnsError::InvalidName)?; + if normalized_name.as_str() == crate::policy_local::POLICY_LOCAL_HOST { + return if family == AddressFamily::Ipv4 { + Ok(SyntheticAnswer { + address: POLICY_LOCAL_ADDRESS.into(), + ttl: MAX_MAPPING_TTL, + mapping_id: uuid::Uuid::nil(), + mapping_generation: 0, + policy_generation: self.policy.current_generation(), + }) + } else { + Err(PolicyDnsError::Resolver(resolver::ResolveError::NoData)) + }; + } if is_host_gateway_alias(normalized_name.as_str()) && self.trusted_host_gateway.is_none() { emit_dns_denial( &normalized_name, @@ -602,6 +618,23 @@ mod tests { ) } + #[tokio::test] + async fn policy_local_resolves_without_an_authored_rule_or_external_lookup() { + let service = service("network_policies: {}\n", vec![]); + let answer = service + .answer_query("PoLiCy.LoCaL.", AddressFamily::Ipv4, Instant::now()) + .await + .expect("sandbox-local address"); + assert_eq!(answer.address, IpAddr::V4(POLICY_LOCAL_ADDRESS)); + assert_eq!(service.resolver.calls.load(Ordering::SeqCst), 0); + assert!(matches!( + service + .answer_query("policy.local", AddressFamily::Ipv6, Instant::now()) + .await, + Err(PolicyDnsError::Resolver(resolver::ResolveError::NoData)) + )); + } + const BASE_POLICY: &str = r" network_policies: database: diff --git a/crates/openshell-supervisor-network/src/policy_dns/runtime.rs b/crates/openshell-supervisor-network/src/policy_dns/runtime.rs index 23b8a6c0b5..6463792a1e 100644 --- a/crates/openshell-supervisor-network/src/policy_dns/runtime.rs +++ b/crates/openshell-supervisor-network/src/policy_dns/runtime.rs @@ -6,7 +6,7 @@ use crate::opa::OpaEngine; use crate::policy_dns::resolver::MAX_DNS_MESSAGE_BYTES; use crate::policy_dns::store::{ResolvedEndpointStore, StoreConfig, SyntheticPools}; -use crate::policy_dns::{PolicyDnsService, SocketTrustedResolver, wire}; +use crate::policy_dns::{POLICY_LOCAL_ADDRESS, PolicyDnsService, SocketTrustedResolver, wire}; use futures::{FutureExt as _, StreamExt as _, stream::FuturesUnordered}; use miette::{IntoDiagnostic, Result, WrapErr}; use openshell_core::net::set_tcp_nodelay_best_effort; @@ -65,8 +65,14 @@ impl PolicyDnsRuntimeConfig { let ipv6_cidr = ipnet::Ipv6Net::new(Ipv6Addr::from(ipv6_start), IPV6_POOL_PREFIX) .map_err(|error| miette::miette!(error.to_string()))?; + // Keep the sandbox-local API address out of the external mapping pool. + let ipv4_pool_start = if ipv4_cidr.contains(&POLICY_LOCAL_ADDRESS) { + Ipv4Addr::from(u32::from(POLICY_LOCAL_ADDRESS) + 1) + } else { + ipv4_cidr.network() + }; let pools = SyntheticPools::new( - ipv4_cidr.network()..=ipv4_cidr.broadcast(), + ipv4_pool_start..=ipv4_cidr.broadcast(), ipv6_cidr.network()..=ipv6_cidr.broadcast(), ) .map_err(|error| miette::miette!(error.to_string()))?; diff --git a/crates/openshell-supervisor-network/src/proxy.rs b/crates/openshell-supervisor-network/src/proxy.rs index b7f3e3b920..f23d8138af 100644 --- a/crates/openshell-supervisor-network/src/proxy.rs +++ b/crates/openshell-supervisor-network/src/proxy.rs @@ -435,6 +435,8 @@ impl ProxyHandle { let cache = identity_cache.clone(); let backend_gateway = *backend_host_gateway; let trusted_gateway = *trusted_host_gateway; + let dtx = denial_tx.clone(); + let has_policy_local = policy_local_ctx.is_some(); tokio::spawn(async move { if let Some(connection) = preauthorize_transparent_open( connection, @@ -443,6 +445,8 @@ impl ProxyHandle { &cache, backend_gateway, trusted_gateway, + has_policy_local, + dtx.as_ref(), ) .await { @@ -582,6 +586,7 @@ impl ProxyHandle { } } +#[allow(clippy::too_many_arguments)] async fn preauthorize_transparent_open( connection: PendingTcpOpen, policy_dns_store: Option<&Arc>, @@ -589,6 +594,8 @@ async fn preauthorize_transparent_open( identity_cache: &BinaryIdentityCache, backend_host_gateway: Option, trusted_host_gateway: Option, + has_policy_local: bool, + denial_tx: Option<&mpsc::UnboundedSender>, ) -> Option { let PendingTcpOpen { stream, @@ -603,6 +610,53 @@ async fn preauthorize_transparent_open( timing, operation: "tcp", }; + if destination.ip() == IpAddr::V4(crate::policy_dns::POLICY_LOCAL_ADDRESS) { + if destination.port() != 80 || !has_policy_local { + emit_staged_transparent_denial( + destination, + &binary_identity, + "sandbox-local policy API requires port 80 and an active context", + "transparent_tcp_policy_local_invalid_destination", + ); + let _ = completion.send(TcpOpenDecision::Denied(TcpOpenDenial::InvalidDestination)); + return None; + } + let identity_check = binary_identity + .as_ref() + .map_err(|_| TcpOpenDenial::IdentityUnavailable) + .and_then(|identity| { + identity_cache + .verify_or_cache_supplied_identity(identity) + .map_err(|error| match error { + SuppliedIdentityError::Unavailable(_) => TcpOpenDenial::IdentityUnavailable, + SuppliedIdentityError::CapacityExhausted => { + TcpOpenDenial::ResourceExhausted + } + }) + }); + if let Err(denial) = identity_check { + emit_staged_transparent_denial( + destination, + &binary_identity, + "sandbox-local policy API requires a verified workload identity", + "transparent_tcp_policy_local_identity_unavailable", + ); + let _ = completion.send(TcpOpenDecision::Denied(denial)); + return None; + } + if completion.send(TcpOpenDecision::RelayReady).is_err() { + return None; + } + return Some(( + stream, + Some(binary_identity), + None, + Some(TransparentOpen { + destination, + authorization: None, + }), + )); + } let host = match transparent_destination_host(destination, policy_dns_store, opa_engine) { Ok(host) => host, Err(error) => { @@ -640,6 +694,20 @@ async fn preauthorize_transparent_open( ); warn!(%destination, %reason, "Denied staged transparent connection"); emit_staged_transparent_denial(destination, &binary_identity, reason, status_detail); + if supplied_authorization.denial.is_none() + && !is_always_blocked_ip(destination.ip()) + && let Some(binary) = decision.binary.as_ref() + { + emit_denial_simple( + denial_tx, + &host, + destination.port(), + &binary.to_string_lossy(), + &decision, + reason, + "transparent_tcp_connect", + ); + } let _ = completion.send(TcpOpenDecision::Denied(denial)); return None; } @@ -781,6 +849,20 @@ fn transparent_destination_host( } } +fn valid_policy_local_request(method: &str, target: &str, request_headers: &str) -> bool { + if method == "CONNECT" || !target.starts_with('/') { + return false; + } + let hosts = request_headers + .split("\r\n") + .filter_map(|line| line.split_once(':')) + .filter(|(name, _)| name.eq_ignore_ascii_case("host")) + .map(|(_, value)| value.trim()) + .collect::>(); + matches!(hosts.as_slice(), [host] if host.eq_ignore_ascii_case(POLICY_LOCAL_HOST) + || host.eq_ignore_ascii_case("policy.local:80")) +} + impl Drop for ProxyHandle { fn drop(&mut self) { self.join.abort(); @@ -2221,20 +2303,29 @@ async fn handle_mediated_connection( let endpoint_observation_context = endpoint_observation_tx .as_ref() .and_then(EndpointObservationSender::capture); + let mut policy_local_transparent = false; let (mut preauthorized_decision, prevalidated_connector) = if let Some(transparent) = transparent_open { let destination = transparent.destination; - let host = - transparent_destination_host(destination, policy_dns_store.as_ref(), &opa_engine)?; - let (decision, connector) = transparent - .authorization - .map_or((None, None), |(decision, connector)| { - (Some(decision), Some(connector)) - }); - let authority = format!("{host}:{}", destination.port()); - client = tokio::io::BufReader::new(virtual_connect_stream(client.into_inner(), authority)); - (decision, connector) + if destination.ip() == IpAddr::V4(crate::policy_dns::POLICY_LOCAL_ADDRESS) + && destination.port() == 80 + { + policy_local_transparent = true; + (None, None) + } else { + let host = + transparent_destination_host(destination, policy_dns_store.as_ref(), &opa_engine)?; + let (decision, connector) = transparent + .authorization + .map_or((None, None), |(decision, connector)| { + (Some(decision), Some(connector)) + }); + let authority = format!("{host}:{}", destination.port()); + client = + tokio::io::BufReader::new(virtual_connect_stream(client.into_inner(), authority)); + (decision, connector) + } } else { (None, None) }; @@ -2283,6 +2374,24 @@ async fn handle_mediated_connection( let method = parts.next().unwrap_or(""); let target = parts.next().unwrap_or(""); + if policy_local_transparent { + if !valid_policy_local_request(method, target, request) { + respond(&mut client, b"HTTP/1.1 400 Bad Request\r\n\r\n").await?; + return Ok(()); + } + let ctx = policy_local_ctx + .as_ref() + .ok_or_else(|| miette::miette!("sandbox-local policy context is unavailable"))?; + return crate::policy_local::handle_forward_request( + ctx, + method, + target, + &buf[..used], + &mut client, + ) + .await; + } + if method != "CONNECT" { return Box::pin(handle_forward_proxy( method, @@ -6892,6 +7001,7 @@ process: ) .unwrap(); let identity_cache = BinaryIdentityCache::new(); + let (denial_tx, mut denial_rx) = mpsc::unbounded_channel(); let identity = || { Ok(ContractBinaryIdentity { executable: ContractExecutableIdentity { @@ -6925,9 +7035,18 @@ process: let (allowed, allowed_result) = pending("203.0.113.7:443"); assert!( - preauthorize_transparent_open(allowed, None, &engine, &identity_cache, None, None) - .await - .is_some() + preauthorize_transparent_open( + allowed, + None, + &engine, + &identity_cache, + None, + None, + false, + Some(&denial_tx) + ) + .await + .is_some() ); assert_eq!(allowed_result.await.unwrap(), TcpOpenDecision::RelayReady); @@ -6940,6 +7059,8 @@ process: &identity_cache, None, None, + false, + Some(&denial_tx), ) .await .is_none() @@ -6948,17 +7069,154 @@ process: unsafe_result.await.unwrap(), TcpOpenDecision::Denied(TcpOpenDenial::InvalidDestination) ); + assert!( + denial_rx.try_recv().is_err(), + "destination failures are not policy proposals" + ); let (denied, denied_result) = pending("203.0.113.8:443"); assert!( - preauthorize_transparent_open(denied, None, &engine, &identity_cache, None, None) - .await - .is_none() + preauthorize_transparent_open( + denied, + None, + &engine, + &identity_cache, + None, + None, + false, + Some(&denial_tx) + ) + .await + .is_none() ); assert_eq!( denied_result.await.unwrap(), TcpOpenDecision::Denied(TcpOpenDenial::PolicyDenied) ); + let event = denial_rx + .try_recv() + .expect("policy denial is sent to mapper"); + assert_eq!(event.host, "203.0.113.8"); + assert_eq!(event.port, 443); + assert_eq!(event.binary, "/usr/bin/curl"); + assert_eq!(event.denial_stage, "transparent_tcp_connect"); + assert!(denial_rx.try_recv().is_err(), "exactly one mapper event"); + } + + #[tokio::test] + async fn staged_policy_local_open_reaches_the_sandbox_scoped_api() { + let engine = Arc::new( + OpaEngine::from_strings( + include_str!("../data/sandbox-policy.rego"), + "network_policies: {}\n", + ) + .unwrap(), + ); + let cache = Arc::new(BinaryIdentityCache::new()); + let identity = ContractBinaryIdentity { + executable: ContractExecutableIdentity { + path: PathBuf::from("/usr/bin/bash"), + digest: Some("44".repeat(32).parse().unwrap()), + }, + ancestors: Vec::new(), + cmdline_paths: Vec::new(), + }; + let (stream, mut workload) = tokio::io::duplex(4096); + let (decision, completion) = tokio::sync::oneshot::channel(); + let pending = PendingTcpOpen { + stream: Box::new(stream), + binary_identity: Ok(identity.clone()), + destination: SocketAddr::from((crate::policy_dns::POLICY_LOCAL_ADDRESS, 80)), + socket: openshell_isolation_interface::contract::NetworkSocketMetadata { + socket_cookie: 7, + nonblocking: false, + process_generation: 1, + }, + policy_generation: engine.current_generation(), + timing: MediationTiming::default(), + decision, + }; + let (stream, supplied_identity, socket_addrs, transparent) = + preauthorize_transparent_open(pending, None, &engine, &cache, None, None, true, None) + .await + .expect("sandbox-local open is admitted"); + assert_eq!(completion.await.unwrap(), TcpOpenDecision::RelayReady); + + let proposals = AgentProposals::new(true); + let (_workspace_tx, workspace_rx) = tokio::sync::watch::channel("default".to_string()); + let context = Arc::new(PolicyLocalContext::new( + Some(openshell_core::proto::SandboxPolicy { + version: 1, + ..Default::default() + }), + None, + Some("test-sandbox".to_string()), + proposals.clone(), + workspace_rx, + )); + let handler = tokio::spawn(handle_mediated_connection( + tokio::io::BufReader::new(stream), + supplied_identity, + socket_addrs, + transparent, + None, + engine, + cache, + Arc::new(AtomicU32::new(0)), + None, + Some(context), + proposals, + Arc::new(None), + Arc::new(None), + Arc::new(None), + None, + None, + None, + None, + None, + None, + )); + workload + .write_all(b"GET /v1/policy/current HTTP/1.1\r\nHost: policy.local\r\nConnection: close\r\n\r\n") + .await + .unwrap(); + let mut response = Vec::new(); + tokio::time::timeout( + std::time::Duration::from_secs(5), + workload.read_to_end(&mut response), + ) + .await + .expect("policy.local response timed out") + .unwrap(); + handler.await.unwrap().unwrap(); + let response = String::from_utf8(response).unwrap(); + assert!(response.starts_with("HTTP/1.1 200 "), "{response}"); + assert!(response.contains("\"format\":\"yaml\""), "{response}"); + } + + #[test] + fn policy_local_route_requires_one_exact_host_and_an_origin_form_target() { + assert!(valid_policy_local_request( + "GET", + "/v1/policy/current", + "GET /v1/policy/current HTTP/1.1\r\nHost: policy.local:80\r\n" + )); + for (method, target, headers) in [ + ("CONNECT", "/v1/policy/current", "Host: policy.local\r\n"), + ( + "GET", + "http://policy.local/v1/policy/current", + "Host: policy.local\r\n", + ), + ("GET", "/v1/policy/current", "Host: external.example\r\n"), + ( + "GET", + "/v1/policy/current", + "Host: policy.local\r\nHost: external.example\r\n", + ), + ] { + assert!(!valid_policy_local_request(method, target, headers)); + } } #[tokio::test] @@ -7011,9 +7269,18 @@ process: }; assert!( - preauthorize_transparent_open(pending, None, &engine, &identity_cache, None, None) - .await - .is_none() + preauthorize_transparent_open( + pending, + None, + &engine, + &identity_cache, + None, + None, + false, + None + ) + .await + .is_none() ); assert_eq!( completion.await.unwrap(), @@ -7083,9 +7350,18 @@ process: }; assert!( - preauthorize_transparent_open(pending, None, &engine, &identity_cache, None, None) - .await - .is_none() + preauthorize_transparent_open( + pending, + None, + &engine, + &identity_cache, + None, + None, + false, + None + ) + .await + .is_none() ); assert_eq!( completion.await.unwrap(), diff --git a/crates/openshell-supervisor-process/src/ssh.rs b/crates/openshell-supervisor-process/src/ssh.rs index 911cf0a072..01be34bf75 100644 --- a/crates/openshell-supervisor-process/src/ssh.rs +++ b/crates/openshell-supervisor-process/src/ssh.rs @@ -986,6 +986,7 @@ impl SshHandler { .ok_or_else(|| anyhow::anyhow!("exec on unknown channel {channel:?}"))?; state.process = Some(exec.process.clone()); state.terminal = exec.terminal.take(); + let output_status = exec.output_status.take(); if let Some(mut stdin) = exec.stdin.take() { let (sender, receiver) = mpsc::channel::>(); @@ -1030,19 +1031,30 @@ impl SshHandler { }) }); tokio::spawn(async move { - let status = exec.process.wait().await; - let _ = stdout_task.await; - if let Some(task) = stderr_task { - let _ = task.await; - } + let status = if let Some(output_status) = output_status { + // The boundary stream's final status includes output delivery + // failure. Process wait alone reports only the child's exit. + let _ = stdout_task.await; + if let Some(task) = stderr_task { + let _ = task.await; + } + output_status.await.ok() + } else { + let status = exec.process.wait().await.ok(); + let _ = stdout_task.await; + if let Some(task) = stderr_task { + let _ = task.await; + } + status + }; let code = match status { - Ok(openshell_isolation_interface::contract::BoundaryExitStatus::Exited(code)) => { + Some(openshell_isolation_interface::contract::BoundaryExitStatus::Exited(code)) => { code.max(0).cast_unsigned() } - Ok(openshell_isolation_interface::contract::BoundaryExitStatus::Signaled( + Some(openshell_isolation_interface::contract::BoundaryExitStatus::Signaled( signal, )) => (128_i32.saturating_add(signal)).max(0).cast_unsigned(), - Err(_) => 1, + None => 74, }; let _ = handle.eof(channel).await; let _ = handle.exit_status_request(channel, code).await; diff --git a/docs/providers/profiles.mdx b/docs/providers/profiles.mdx index 446715a5c6..df39087d37 100644 --- a/docs/providers/profiles.mdx +++ b/docs/providers/profiles.mdx @@ -277,6 +277,19 @@ Import one profile file at platform scope: openshell profile import -f providers/github.yaml --global ``` +Import a published YAML or JSON profile directly from an HTTP or HTTPS URL: + +```shell +openshell profile lint --url https://example.com/profiles/github.yaml +openshell profile import --url https://example.com/profiles/github.yaml --global +``` + +Review profiles from remote sources before importing them. A profile can grant +network access and bind credentials to the endpoints and binaries it declares. +The URL path must end in `.yaml`, `.yml`, or `.json`; query parameters are allowed. +Remote downloads have a 1 MiB size limit and a 15 second timeout. `--url` is +mutually exclusive with `-f` and `--from`. + Import all non-recursive `*.yaml`, `*.yml`, and `*.json` files from a directory: ```shell diff --git a/docs/reference/gateway-config.mdx b/docs/reference/gateway-config.mdx index d9b3e489c0..79a986058d 100644 --- a/docs/reference/gateway-config.mdx +++ b/docs/reference/gateway-config.mdx @@ -270,7 +270,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 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.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`, supervisors skip periodic session-token renewal, 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 178ab92630..a475eb57de 100644 --- a/docs/sandboxes/manage-sandboxes.mdx +++ b/docs/sandboxes/manage-sandboxes.mdx @@ -357,7 +357,19 @@ Pipe stdin into the command: echo "hello" | openshell sandbox exec -n my-sandbox -- cat ``` +The CLI sends small piped input in one request for compatibility with older +gateways. It streams larger input in bounded frames, including for commands +without a TTY, so input is not limited by the gateway's per-message request +size. Piped input is limited to 4 MiB; use `sandbox upload` for larger files. +The CLI closes remote stdin when the pipe reaches EOF and continues +reading command output until the command finishes. + The command's exit code is propagated to the CLI, so `exec` works in scripts that check return codes. +Large stdout and stderr streams are delivered before a successful exit. A slow +reader backpressures the command. If output delivery fails, `exec` returns a +failure instead of reporting success with incomplete output. +If a background process keeps stdout or stderr open for more than 30 seconds +after the command exits, `exec` reports an output delivery failure. Run an interactive shell with a TTY: diff --git a/docs/sandboxes/policy-advisor.mdx b/docs/sandboxes/policy-advisor.mdx index 938bf93bd1..83413e21e1 100644 --- a/docs/sandboxes/policy-advisor.mdx +++ b/docs/sandboxes/policy-advisor.mdx @@ -254,7 +254,9 @@ Reviewers see the same guidance in the terminal UI. Run `openshell term`, open t ## Agent API -`policy.local` is available only inside the sandbox and uses plain HTTP: +`policy.local` is available only inside the sandbox and uses plain HTTP. It +resolves through sandbox-local DNS and does not need a proxy environment variable +or a network policy rule: | Endpoint | Purpose | |---|---| diff --git a/e2e/policy-advisor/README.md b/e2e/policy-advisor/README.md index 71c3f9d1dd..9e29d1ec5e 100644 --- a/e2e/policy-advisor/README.md +++ b/e2e/policy-advisor/README.md @@ -53,21 +53,29 @@ Requires Docker, `agent_policy_proposals_enabled=true`, and a GitHub token with contents write on the repository. The test auto-resolves the token from `DEMO_GITHUB_TOKEN`, `GITHUB_TOKEN`, `GH_TOKEN`, or `gh auth token`. -## Mechanistic smoke +## Conformance coverage -Lightweight regression for the L4 CONNECT deny → mechanistic chunk pipeline. -No GitHub token or LLM required. +The `mechanistic-proposal` and `policy-local` conformance scenarios check draft +generation and use `policy.local` to inspect policy, submit a narrow permission +request, and read the resulting proposal. Run them against a configured gateway +with `--openshell-bin` pointing to the CLI under test: ```bash -mise run e2e:mechanistic-smoke +openshell-conformance run mechanistic-proposal policy-local --openshell-bin target/debug/openshell ``` -Or manually against a running gateway with `agent_policy_proposals_enabled=true`: +Run `openshell-conformance list` to see all scenario names. A manual +`Integration Tests` workflow run can select the `policy-advisor` testsuite to +run only these two scenarios against an installed candidate. Set +`artifact-run-id` to the candidate build's workflow run ID and `test-matrix` to: -```bash -OPENSHELL_BIN=target/debug/openshell bash e2e/policy-advisor/mechanistic-smoke.sh +```json +[{"environment":"ubuntu-docker-rootful","installer":"binaries","testsuite":"policy-advisor"}] ``` +The GitHub write test above and the regressions below still exercise distinct +proposal review, approval, and hot-reload behavior. + The #2821 regression additionally verifies that a denial on an existing inspected endpoint becomes a binary expansion, auto-approves, hot-reloads, and does not downgrade the endpoint to L4: diff --git a/e2e/policy-advisor/mechanistic-smoke.sh b/e2e/policy-advisor/mechanistic-smoke.sh deleted file mode 100755 index 4ba780856f..0000000000 --- a/e2e/policy-advisor/mechanistic-smoke.sh +++ /dev/null @@ -1,130 +0,0 @@ -#!/usr/bin/env bash - -# SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. -# SPDX-License-Identifier: Apache-2.0 - -# Regression smoke for the mechanistic policy mapper. -# -# Triggers an L4 CONNECT deny from inside a sandbox, waits for the denial -# aggregator to flush, and asserts that a pending mechanistic chunk appears -# under `openshell rule get --status pending`. -# -# This is deliberately L4-only. L7 denials (method/path 403s) are the agent -# loop's job; the mechanistic mapper only covers L4 CONNECT denials. See #1333. -# -# Prereqs: a running gateway with agent_policy_proposals_enabled=true. - -set -euo pipefail - -SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -REPO_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)" - -if [[ -z "${OPENSHELL_BIN:-}" ]]; then - if [[ -x "${REPO_ROOT}/target/debug/openshell" ]]; then - OPENSHELL_BIN="${REPO_ROOT}/target/debug/openshell" - else - OPENSHELL_BIN="openshell" - fi -fi - -RUN_ID="${RUN_ID:-$(date +%Y%m%d-%H%M%S)}" -SANDBOX="${SANDBOX:-ms-${RUN_ID}}" -KEEP_SANDBOX="${KEEP_SANDBOX:-0}" -# Allow override so CI can set a shorter interval via OPENSHELL_DENIAL_FLUSH_INTERVAL_SECS. -FLUSH_WAIT="${FLUSH_WAIT:-15}" - -BOLD='\033[1m' -CYAN='\033[36m' -GREEN='\033[32m' -RED='\033[31m' -RESET='\033[0m' - -step() { printf "\n${BOLD}${CYAN}==> %s${RESET}\n\n" "$1"; } -ok() { printf " ${GREEN}✓${RESET} %s\n" "$*"; } -fail() { printf "\n${RED}FAIL:${RESET} %s\n" "$*" >&2; exit 1; } - -TMP_DIR="" - -cleanup() { - if [[ "$KEEP_SANDBOX" != "1" ]]; then - "$OPENSHELL_BIN" sandbox delete "$SANDBOX" >/dev/null 2>&1 || true - fi - [[ -z "$TMP_DIR" ]] || rm -rf "$TMP_DIR" -} -trap cleanup EXIT - -preflight() { - step "Preflight" - local raw_settings - if ! raw_settings="$("$OPENSHELL_BIN" settings get --global --json 2>&1)"; then - fail "cannot reach gateway: ${raw_settings}" - fi - local enabled - enabled="$(printf '%s' "$raw_settings" \ - | jq -r '.settings.agent_policy_proposals_enabled // ""')" - [[ "$enabled" == "true" ]] \ - || fail "set agent_policy_proposals_enabled=true first: - $OPENSHELL_BIN settings set --global --key agent_policy_proposals_enabled --value true --yes" - ok "agent_policy_proposals_enabled=true" -} - -create_sandbox() { - step "Creating sandbox '${SANDBOX}' (no network policy)" - TMP_DIR="$(mktemp -d)" - SSH_CONFIG="${TMP_DIR}/ssh_config" - - "$OPENSHELL_BIN" sandbox delete "$SANDBOX" >/dev/null 2>&1 || true - "$OPENSHELL_BIN" sandbox create \ - --name "$SANDBOX" \ - --detach \ - --no-auto-providers \ - --no-tty \ - -- sh -c "exec sleep infinity" \ - | sed 's/^/ /' - - "$OPENSHELL_BIN" sandbox ssh-config "$SANDBOX" > "$SSH_CONFIG" - SSH_HOST="$(awk '/^Host / { print $2; exit }' "$SSH_CONFIG")" - [[ -n "$SSH_HOST" ]] || fail "could not parse SSH host" - - for _i in $(seq 1 30); do - ssh -F "$SSH_CONFIG" "$SSH_HOST" true >/dev/null 2>&1 && { ok "SSH up"; return; } - sleep 2 - done - fail "SSH timed out" -} - -trigger_l4_deny() { - step "Triggering L4 CONNECT deny from inside sandbox" - # blocked.invalid is guaranteed unroutable and not in any policy. - ssh -F "$SSH_CONFIG" "$SSH_HOST" \ - "curl -sf --max-time 5 https://blocked.invalid/ || true" >/dev/null 2>&1 || true - ok "curl attempted (deny expected)" -} - -assert_pending_chunk() { - step "Waiting up to ${FLUSH_WAIT}s for pending chunk" - local output="" - local _i - for _i in $(seq 1 "$FLUSH_WAIT"); do - output="$("$OPENSHELL_BIN" rule get "$SANDBOX" --status pending 2>&1)" - if printf '%s\n' "$output" | grep -qi "blocked.invalid"; then - printf '%s\n' "$output" | sed 's/^/ /' - ok "pending mechanistic chunk present for blocked.invalid" - return - fi - sleep 1 - done - printf '%s\n' "$output" | sed 's/^/ /' - fail "no pending chunk for blocked.invalid after ${FLUSH_WAIT}s" -} - -main() { - command -v jq >/dev/null || fail "jq is required" - preflight - create_sandbox - trigger_l4_deny - assert_pending_chunk - step "Smoke pass" -} - -main "$@" diff --git a/e2e/rust/Cargo.toml b/e2e/rust/Cargo.toml index 102f70c147..402fc5a699 100644 --- a/e2e/rust/Cargo.toml +++ b/e2e/rust/Cargo.toml @@ -93,6 +93,11 @@ name = "podman_host_gateway" path = "tests/podman_host_gateway.rs" required-features = ["e2e-podman"] +[[test]] +name = "podman_preflight" +path = "tests/podman_preflight.rs" +required-features = ["e2e-podman"] + [[test]] name = "podman_corporate_proxy" path = "tests/podman_corporate_proxy.rs" @@ -103,6 +108,11 @@ name = "podman_oci_identity" path = "tests/podman_oci_identity.rs" required-features = ["e2e-podman"] +[[test]] +name = "podman_resource_limits" +path = "tests/podman_resource_limits.rs" +required-features = ["e2e-podman"] + [[test]] name = "podman_userns" path = "tests/podman_userns.rs" diff --git a/e2e/rust/tests/local_driver_token_restart.rs b/e2e/rust/tests/local_driver_token_restart.rs index 55dbf91821..16ac292fa4 100644 --- a/e2e/rust/tests/local_driver_token_restart.rs +++ b/e2e/rust/tests/local_driver_token_restart.rs @@ -74,7 +74,7 @@ impl LocalDriver { ], Self::Podman => vec![ "label=openshell.managed=true".to_string(), - format!("label=openshell.io/isolation-role={role}"), + format!("label=openshell.ai/isolation-role={role}"), format!("label=openshell.ai/sandbox-name={sandbox_name}"), ], Self::Vm => Vec::new(), @@ -279,7 +279,8 @@ fn require_unexpired_token(token: &str, context: &str) -> Result<(), String> { .as_secs(), ) .map_err(|_| "current Unix timestamp does not fit in i64".to_string())?; - if exp <= now { + // Sandbox session tokens use exp=0 when no TTL is configured. + if exp != 0 && exp <= now { return Err(format!( "{context} should expire in the future, got exp={exp}, now={now}" )); diff --git a/e2e/rust/tests/podman_preflight.rs b/e2e/rust/tests/podman_preflight.rs new file mode 100644 index 0000000000..afb3bc1c38 --- /dev/null +++ b/e2e/rust/tests/podman_preflight.rs @@ -0,0 +1,116 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "e2e-podman")] + +//! Podman driver daemon-unavailable e2e tests. +//! +//! These tests verify that `openshell-driver-podman` fails fast with an +//! actionable error when it cannot reach a Podman API socket, instead of +//! hanging or silently serving gRPC against a dead connection. +//! +//! The tests do NOT require a running Podman daemon or gateway — they point +//! `--podman-socket` at a path that is guaranteed not to exist to simulate +//! the daemon being unavailable. + +use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant}; + +use openshell_e2e::harness::output::strip_ansi; + +/// Locate the workspace root by walking up from this crate's manifest directory. +fn workspace_root() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .ancestors() + .nth(2) + .expect("failed to resolve workspace root from CARGO_MANIFEST_DIR") + .to_path_buf() +} + +/// Return the path to the `openshell-driver-podman` binary. +/// +/// Uses `OPENSHELL_EXTERNAL_DRIVER_BIN` when set (the same env var the shell +/// e2e harness uses for prebuilt standalone driver artifacts), otherwise +/// expects the binary at `/target/debug/openshell-driver-podman`. +fn driver_podman_bin() -> PathBuf { + let bin = std::env::var_os("OPENSHELL_EXTERNAL_DRIVER_BIN").map_or_else( + || workspace_root().join("target/debug/openshell-driver-podman"), + PathBuf::from, + ); + assert!( + bin.is_file(), + "openshell-driver-podman binary not found at {} — set OPENSHELL_EXTERNAL_DRIVER_BIN \ + or run `cargo build -p openshell-driver-podman` first", + bin.display() + ); + bin +} + +/// Run `openshell-driver-podman` pointed at a Podman socket that does not +/// exist, and wait for it to exit. +/// +/// The driver retries a handful of times before giving up (to tolerate the +/// socket briefly re-activating), so this can take several seconds. +async fn run_with_unreachable_podman_socket() -> (String, i32, Duration, PathBuf) { + let tmpdir = tempfile::tempdir().expect("create isolated socket dir"); + let missing_socket = tmpdir.path().join("openshell-e2e-nonexistent-podman.sock"); + + let start = Instant::now(); + let mut cmd = tokio::process::Command::new(driver_podman_bin()); + cmd.arg("--podman-socket") + .arg(&missing_socket) + .kill_on_drop(true) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()); + + let output = tokio::time::timeout(Duration::from_secs(60), cmd.output()) + .await + .expect("openshell-driver-podman should exit instead of hanging") + .expect("spawn openshell-driver-podman"); + let elapsed = start.elapsed(); + let stdout = String::from_utf8_lossy(&output.stdout).to_string(); + let stderr = String::from_utf8_lossy(&output.stderr).to_string(); + let combined = format!("{stdout}{stderr}"); + let code = output.status.code().unwrap_or(-1); + (combined, code, elapsed, missing_socket) +} + +/// `openshell-driver-podman` should exit non-zero, not hang, when its +/// configured Podman socket does not exist. +#[tokio::test] +async fn driver_exits_when_podman_socket_unreachable() { + let (output, code, elapsed, _) = run_with_unreachable_podman_socket().await; + + assert_ne!( + code, 0, + "driver should exit non-zero when Podman is unreachable, output:\n{output}" + ); + + assert!( + elapsed < Duration::from_secs(30), + "driver should give up retrying and exit within its bounded retry \ + window (took {}s), output:\n{output}", + elapsed.as_secs() + ); +} + +/// The error surfaced when the Podman socket is unreachable should name the +/// configured socket path and describe a connection failure, not a generic +/// panic or timeout with no actionable detail. +#[tokio::test] +async fn driver_error_names_unreachable_socket() { + let (output, code, _, missing_socket) = run_with_unreachable_podman_socket().await; + + assert_ne!(code, 0); + let clean = strip_ansi(&output); + + assert!( + clean.contains("connection error"), + "driver error should describe a connection failure:\n{clean}" + ); + assert!( + clean.contains(missing_socket.to_str().expect("socket path is utf-8")), + "driver error should name the unreachable socket path {}:\n{clean}", + missing_socket.display() + ); +} diff --git a/e2e/rust/tests/podman_resource_limits.rs b/e2e/rust/tests/podman_resource_limits.rs new file mode 100644 index 0000000000..a0282f0e09 --- /dev/null +++ b/e2e/rust/tests/podman_resource_limits.rs @@ -0,0 +1,89 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +#![cfg(feature = "e2e-podman")] + +//! Podman-specific E2E coverage verifying that declared sandbox CPU/memory +//! resource limits are actually enforced by cgroups inside the workload, not +//! just echoed back by the API/template layer. +//! +//! `e2e/rust/tests/sandbox_templates.rs` already verifies that +//! `sandbox template create --cpu ... --memory ...` is stored and returned +//! correctly, but it never inspects the resulting container's real resource +//! state. This test creates a sandbox with those flags directly and reads the +//! cgroup v2 interface files from inside the sandbox itself, so it exercises +//! the actual enforcement boundary the workload experiences. + +use openshell_e2e::harness::sandbox::SandboxGuard; + +const CGROUP_READ_POLICY: &str = r"version: 1 +filesystem_policy: + include_workdir: true + read_only: [/usr, /lib, /lib64, /proc, /etc, /sys/fs/cgroup] + read_write: [/sandbox, /tmp, /dev/null] +landlock: + compatibility: best_effort +network_policies: {} +"; + +const CPU_REQUEST: &str = "500m"; +const MEMORY_REQUEST: &str = "512Mi"; + +// "500m" (500 millicores) becomes a 50000us quota over Podman's 100000us +// (100ms) CFS period — see +// crates/openshell-driver-podman/src/container.rs's parse_cpu_to_microseconds. +// Verified directly against a real `podman run --cpus=0.5` container, whose +// cpu.max reads "50000 100000". +const EXPECTED_CPU_MAX: &str = "50000 100000"; + +// "512Mi" (mebibytes) becomes exactly 512 * 1024 * 1024 bytes — see +// crates/openshell-driver-podman/src/container.rs's parse_memory_to_bytes. +// Verified directly against a real `podman run --memory=512m` container, +// whose memory.max reads "536870912". +const EXPECTED_MEMORY_MAX: &str = "536870912"; + +#[tokio::test] +async fn sandbox_resource_limits_are_enforced_via_cgroups() { + if std::env::var("OPENSHELL_E2E_DRIVER").as_deref() != Ok("podman") { + eprintln!("Skipping Podman resource-limit test: e2e driver is not podman"); + return; + } + + let policy = tempfile::NamedTempFile::new().expect("create cgroup read policy file"); + std::fs::write(policy.path(), CGROUP_READ_POLICY).expect("write cgroup read policy"); + let policy_path = policy.path().to_str().expect("policy path is UTF-8"); + let mut sandbox = SandboxGuard::create(&[ + "--cpu", + CPU_REQUEST, + "--memory", + MEMORY_REQUEST, + "--policy", + policy_path, + ]) + .await + .expect("sandbox create with resource limits should succeed"); + + let memory_max = sandbox + .exec(&["cat", "/sys/fs/cgroup/memory.max"]) + .await + .expect("read memory.max from sandbox cgroup") + .trim() + .to_string(); + assert_eq!( + memory_max, EXPECTED_MEMORY_MAX, + "sandbox cgroup should enforce the declared {MEMORY_REQUEST} memory limit, got {memory_max}" + ); + + let cpu_max = sandbox + .exec(&["cat", "/sys/fs/cgroup/cpu.max"]) + .await + .expect("read cpu.max from sandbox cgroup") + .trim() + .to_string(); + assert_eq!( + cpu_max, EXPECTED_CPU_MAX, + "sandbox cgroup should enforce the declared {CPU_REQUEST} CPU limit, got {cpu_max}" + ); + + sandbox.cleanup().await; +} diff --git a/e2e/rust/tests/sandbox_lifecycle.rs b/e2e/rust/tests/sandbox_lifecycle.rs index 3ccd2d73c5..4df9f24eba 100644 --- a/e2e/rust/tests/sandbox_lifecycle.rs +++ b/e2e/rust/tests/sandbox_lifecycle.rs @@ -111,6 +111,257 @@ async fn delete_sandbox(name: &str) { let _ = cmd.status().await; } +#[tokio::test] +#[serial(sandbox_lifecycle)] +async fn sandbox_exec_large_output_is_complete() { + const BYTES: usize = 8 * 1024 * 1024; + let mut sandbox = SandboxGuard::create(&[]) + .await + .expect("create sandbox for large output"); + + for stderr in [false, true] { + let script = if stderr { + format!("yes A | head -c {BYTES} >&2") + } else { + format!("yes A | head -c {BYTES}") + }; + let mut command = openshell_cmd(); + command + .args([ + "sandbox", + "exec", + "--name", + &sandbox.name, + "--no-tty", + "--no-login-shell", + "--", + "sh", + "-c", + &script, + ]) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let output = tokio::time::timeout(Duration::from_secs(30), command.output()) + .await + .expect("large exec output timed out") + .expect("run large exec output"); + assert!(output.status.success(), "large exec exited unsuccessfully"); + let bytes = if stderr { output.stderr } else { output.stdout }; + assert_eq!(bytes.len(), BYTES, "exec output was truncated"); + assert!(bytes.chunks_exact(2).all(|pair| pair == b"A\n")); + } + + let mut early_exit = openshell_cmd(); + let mut early_child = early_exit + .args([ + "sandbox", + "exec", + "--name", + &sandbox.name, + "--no-tty", + "--no-login-shell", + "--", + "head", + "-n1", + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("start early-exit exec"); + let mut input = b"first\n".to_vec(); + input.extend(vec![b'x'; 128 * 1024]); + let write_result = early_child + .stdin + .take() + .expect("early-exit stdin") + .write_all(&input) + .await; + if let Err(error) = write_result { + assert_eq!(error.kind(), std::io::ErrorKind::BrokenPipe); + } + let early_output = + tokio::time::timeout(Duration::from_secs(30), early_child.wait_with_output()) + .await + .expect("early-exit exec timed out") + .expect("wait for early-exit exec"); + assert!(early_output.status.success(), "early-exit exec failed"); + assert_eq!(early_output.stdout, b"first\n"); + + let mut command = openshell_cmd(); + let output = tokio::time::timeout( + Duration::from_secs(10), + command + .args([ + "sandbox", + "exec", + "--name", + &sandbox.name, + "--no-tty", + "--no-login-shell", + "--", + "sh", + "-c", + "sleep 5 & echo ok", + ]) + .stdin(Stdio::null()) + .output(), + ) + .await + .expect("exec with background pipe holder timed out") + .expect("run exec with background pipe holder"); + assert!(output.status.success(), "unexpected output failure"); + assert_eq!(output.stdout, b"ok\n"); + + sandbox.cleanup().await; +} + +#[tokio::test] +#[serial(sandbox_lifecycle)] +async fn piped_exec_stdin_crosses_grpc_message_limit() { + let mut sandbox = SandboxGuard::create(&[]) + .await + .expect("create sandbox for streamed stdin"); + + for size in [5, 1_048_576, 4_194_304] { + let mut command = openshell_cmd(); + command + .args([ + "sandbox", + "exec", + "--name", + &sandbox.name, + "--no-tty", + "--no-login-shell", + "--", + "wc", + "-c", + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let mut child = command.spawn().expect("spawn sandbox exec"); + let mut input = child.stdin.take().expect("piped stdin"); + input + .write_all(&vec![b'x'; size]) + .await + .expect("write piped stdin"); + drop(input); + let output = tokio::time::timeout(Duration::from_secs(30), child.wait_with_output()) + .await + .expect("streamed exec timed out") + .expect("wait for streamed exec"); + assert!( + output.status.success(), + "streamed exec failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!( + String::from_utf8_lossy(&output.stdout).trim(), + size.to_string() + ); + } + + let mut oversized = openshell_cmd(); + let mut oversized_child = oversized + .args([ + "sandbox", + "exec", + "--name", + &sandbox.name, + "--no-tty", + "--no-login-shell", + "--", + "wc", + "-c", + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn oversized stdin exec"); + oversized_child + .stdin + .take() + .expect("oversized stdin pipe") + .write_all(&vec![b'x'; 4_194_305]) + .await + .expect("write oversized stdin"); + let oversized_output = oversized_child + .wait_with_output() + .await + .expect("wait for oversized stdin error"); + assert!(!oversized_output.status.success()); + assert!(oversized_output.stdout.is_empty()); + assert!(String::from_utf8_lossy(&oversized_output.stderr).contains("4 MiB limit")); + + // A forced PTY with a small pipe must keep using the unary RPC for + // compatibility with older gateways. + let mut tty_command = openshell_cmd(); + let mut tty_child = tty_command + .args([ + "sandbox", + "exec", + "--name", + &sandbox.name, + "--tty", + "--no-login-shell", + "--", + "sh", + "-c", + "printf tty-ok", + ]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .expect("spawn forced TTY exec"); + tty_child + .stdin + .take() + .expect("piped TTY stdin") + .write_all(b"x") + .await + .expect("write TTY stdin"); + let tty_output = tokio::time::timeout(Duration::from_secs(30), tty_child.wait_with_output()) + .await + .expect("forced TTY exec timed out") + .expect("wait for forced TTY exec"); + assert!( + tty_output.status.success(), + "forced TTY exec failed: {}", + String::from_utf8_lossy(&tty_output.stderr) + ); + assert!(String::from_utf8_lossy(&tty_output.stdout).contains("tty-ok")); + + #[cfg(unix)] + { + let directory = std::fs::File::open("/").expect("open directory as stdin"); + let mut command = openshell_cmd(); + let output = command + .args([ + "sandbox", + "exec", + "--name", + &sandbox.name, + "--no-tty", + "--no-login-shell", + "--", + "wc", + "-c", + ]) + .stdin(Stdio::from(directory)) + .output() + .await + .expect("run exec with unreadable stdin"); + assert!(!output.status.success(), "stdin read error was ignored"); + assert!(output.stdout.is_empty()); + } + + sandbox.cleanup().await; +} + async fn run_sandbox_lifecycle_command(operation: &str, name: &str) -> String { let mut cmd = openshell_cmd(); cmd.args(["sandbox", operation, name]) diff --git a/skills/openshell-cli/SKILL.md b/skills/openshell-cli/SKILL.md index 5ae5703810..c6e166f230 100644 --- a/skills/openshell-cli/SKILL.md +++ b/skills/openshell-cli/SKILL.md @@ -82,10 +82,13 @@ attaches your terminal to that retained process. Add `--detach` to return after the sandbox becomes ready without attaching. An explicit trailing command is foreground even when stdin or stdout is not a -terminal. The CLI streams its stdout and stderr and returns its exact exit -status. Exit code 0 leaves a retained sandbox in `Completed`; nonzero leaves it -in `Error` with `MainProcessFailed`. Use `--no-keep` to delete either result -after output drains, or `--detach` for a long-running service. Combine +terminal. The CLI streams its stdout and stderr and reports the command's exit +status after output drains. A failure to deliver output makes the CLI report a +failure even if the command itself exited successfully; check the sandbox's +state before retrying work that might have side effects. Exit code 0 leaves a +retained sandbox in `Completed`; nonzero leaves it in `Error` with +`MainProcessFailed`. Use `--no-keep` to delete either result after output +drains, or `--detach` for a long-running service. Combine `--detach --no-keep` when the gateway should run the service without a host attachment and delete its sandbox after the service exits. @@ -165,8 +168,14 @@ openshell profile describe github openshell profile export github --output yaml openshell profile lint --file ./my-profile.yaml openshell profile import --file ./my-profile.yaml +openshell profile lint --url https://example.com/profiles/my-profile.yaml +openshell profile import --url https://example.com/profiles/my-profile.yaml ``` +`--url` accepts one HTTP or HTTPS YAML or JSON profile. Review its endpoint and +binary grants before importing it. The URL path must end in `.yaml`, `.yml`, or +`.json`; downloads are limited to 1 MiB and 15 seconds. + Use `profile describe` to inspect a definition's credential metadata, endpoints, TLS handling, MCP access settings, rule counts, binaries, source, and scope before creating a provider. Check for `tls: skip` and the uninspected-credential opt-in before relying on displayed L7 rules. List and describe accept table, JSON, and YAML output; use structured output for complete rule definitions, `--workspace` for a workspace catalog, or `--global` for platform scope. Use `profile export` when preparing an editable definition, `profile update --file ` to replace an existing custom profile with its current resource version, and `profile delete ...` to remove custom profiles. Provider instances remain under `provider`. Existing scripts can continue using `provider list-profiles` and `provider profile export/import/update/lint/delete`. These commands share the top-level handlers and preserve their arguments, output options, and workspace/global flags. Prefer `profile` when writing new commands. @@ -399,9 +408,13 @@ openshell sandbox exec --name my-sandbox --workdir /workspace -- ls -la openshell sandbox exec --name my-sandbox --env MODE=test -- cargo test ``` -`sandbox exec` starts an independent sibling process, streams output, and exits -with the remote command's exit code. Use `sandbox connect` to attach to the -canonical main process. +`sandbox exec` starts an independent sibling process and streams output. After +stdout and stderr drain, it returns the remote command's exit code if delivery +succeeds. Output delivery failure instead returns exit code 74, even when the +command exited successfully. A descendant that keeps an inherited output pipe +open for more than 30 seconds after the command exits triggers that failure. +Check whether the command ran before retrying work with side effects. Use +`sandbox connect` to attach to the canonical main process. Use `--env` only for non-secret values. Attach credentials to the sandbox with a provider instead of passing API keys, tokens, or other secrets to `sandbox exec`. diff --git a/tasks/test.toml b/tasks/test.toml index fd516ae256..39dd070204 100644 --- a/tasks/test.toml +++ b/tasks/test.toml @@ -285,13 +285,6 @@ description = "Run Docker conformance and Rust e2e tests against a standalone ga depends = ["e2e:conformance:build"] run = "OPENSHELL_CONFORMANCE_BIN=\"${OPENSHELL_CONFORMANCE_BIN:-$PWD/target/debug/openshell-conformance}\" e2e/rust/e2e-docker.sh" -["e2e:mechanistic-smoke"] -description = "Run mechanistic L4 smoke against a Docker-backed gateway" -run = [ - "cargo build -p openshell-cli", - "e2e/with-docker-gateway.sh bash -lc 'target/debug/openshell settings set --global --key agent_policy_proposals_enabled --value true --yes && OPENSHELL_BIN=$PWD/target/debug/openshell bash e2e/policy-advisor/mechanistic-smoke.sh'", -] - ["e2e:mechanistic-existing-endpoint"] description = "Run #2821 existing inspected-endpoint auto-approval regression" run = [ diff --git a/tests/ansible/playbooks/conformance/cli.yaml b/tests/ansible/playbooks/conformance/cli.yaml index 759cc53fca..d21a60a35b 100644 --- a/tests/ansible/playbooks/conformance/cli.yaml +++ b/tests/ansible/playbooks/conformance/cli.yaml @@ -59,6 +59,11 @@ - --workspace-remap - /var/lib/openshell-conformance/tests - --no-capture + # The guest has 4 GiB; keep sandbox creates from competing for it. + - --test-threads + - "1" + - --filterset + - "{{ conformance_filter | default('all()') }}" environment: OPENSHELL_BIN: "{{ openshell_cli.stdout }}" register: conformance_result diff --git a/tests/ansible/playbooks/conformance/policy-advisor.yaml b/tests/ansible/playbooks/conformance/policy-advisor.yaml new file mode 100644 index 0000000000..f9a4cfbf8c --- /dev/null +++ b/tests/ansible/playbooks/conformance/policy-advisor.yaml @@ -0,0 +1,7 @@ +# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 + +--- +- import_playbook: cli.yaml + vars: + conformance_filter: binary(policy_advisor) diff --git a/tests/ansible/playbooks/drivers/podman/default-userns-baseline.yaml b/tests/ansible/playbooks/drivers/podman/default-userns-baseline.yaml index 4980bebdc4..7356ae3c5e 100644 --- a/tests/ansible/playbooks/drivers/podman/default-userns-baseline.yaml +++ b/tests/ansible/playbooks/drivers/podman/default-userns-baseline.yaml @@ -2,7 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 --- -- name: Capture the default rootless Podman user-namespace mapping +- name: Capture the default Podman user-namespace mapping hosts: all gather_facts: false vars: @@ -12,26 +12,28 @@ - name: Wait for SSH ansible.builtin.wait_for_connection: - - name: Read OpenShell gateway service user - become: true - ansible.builtin.command: - argv: - - systemctl - - show - - openshell-gateway.service - - --property - - User - - --value - changed_when: false - register: openshell_gateway_service_user + - name: Detect tmachine container runtime + ansible.builtin.include_role: + name: tmachine_container_runtime - - name: Require the rootless Podman gateway user + - name: Require a Podman gateway ansible.builtin.assert: that: - - openshell_gateway_service_user.stdout == "tmachine" + - tmachine_container_runtime_name == "podman" fail_msg: >- - The Podman default-userns suite requires a rootless Podman gateway - running as tmachine, not {{ openshell_gateway_service_user.stdout | default("unknown") }} + The Podman default-userns suite requires a Podman gateway, not + {{ tmachine_container_runtime_name }} + + # podman_reference_user only selects who runs the direct `podman` + # reference commands below; it must match the daemon's rootful/rootless + # mode. The captured reference file itself stays owned by tmachine + # (the inventory's ansible_user), since the archived test binary that + # later reads it always runs unprivileged as tmachine, in both modes. + - name: Resolve the Podman reference execution context + ansible.builtin.set_fact: + podman_reference_user: "{{ 'tmachine' if tmachine_container_runtime_is_rootless else 'root' }}" + podman_reference_home: "{{ '/home/tmachine' if tmachine_container_runtime_is_rootless else '/root' }}" + podman_reference_uid: "{{ tmachine_container_runtime_tmachine_uid.stdout if tmachine_container_runtime_is_rootless else '0' }}" - name: Read OpenShell gateway configuration become: true @@ -49,15 +51,6 @@ The Podman default-userns suite requires gateway.toml to omit userns, uidmap, and gidmap. - - name: Resolve tmachine UID - ansible.builtin.command: - argv: - - id - - -u - - tmachine - changed_when: false - register: tmachine_uid - - name: Create Podman test-input directory become: true ansible.builtin.file: @@ -69,17 +62,17 @@ - name: Pull the Podman reference image become: true - become_user: tmachine + become_user: "{{ podman_reference_user }}" ansible.builtin.command: argv: [podman, pull, "{{ podman_reference_image }}"] environment: - HOME: /home/tmachine - XDG_RUNTIME_DIR: "/run/user/{{ tmachine_uid.stdout }}" + HOME: "{{ podman_reference_home }}" + XDG_RUNTIME_DIR: "/run/user/{{ podman_reference_uid }}" changed_when: false - name: Capture direct Podman default UID mapping become: true - become_user: tmachine + become_user: "{{ podman_reference_user }}" ansible.builtin.command: argv: - podman @@ -92,8 +85,8 @@ - "{{ podman_reference_image }}" - /proc/self/uid_map environment: - HOME: /home/tmachine - XDG_RUNTIME_DIR: "/run/user/{{ tmachine_uid.stdout }}" + HOME: "{{ podman_reference_home }}" + XDG_RUNTIME_DIR: "/run/user/{{ podman_reference_uid }}" changed_when: false register: podman_default_uid_map @@ -101,7 +94,7 @@ ansible.builtin.assert: that: - podman_default_uid_map.stdout | trim | length > 0 - fail_msg: Direct rootless Podman reference container returned no UID mapping + fail_msg: Direct Podman reference container returned no UID mapping - name: Store direct Podman default UID mapping become: true diff --git a/tests/ansible/playbooks/drivers/podman/tests.yaml b/tests/ansible/playbooks/drivers/podman/tests.yaml index abd05bea70..abe2f6e553 100644 --- a/tests/ansible/playbooks/drivers/podman/tests.yaml +++ b/tests/ansible/playbooks/drivers/podman/tests.yaml @@ -12,6 +12,16 @@ - name: Wait for SSH ansible.builtin.wait_for_connection: + - name: Detect tmachine container runtime + ansible.builtin.include_role: + name: tmachine_container_runtime + + - name: Resolve the Podman reference execution context + ansible.builtin.set_fact: + podman_reference_user: "{{ 'tmachine' if tmachine_container_runtime_is_rootless else 'root' }}" + podman_reference_home: "{{ '/home/tmachine' if tmachine_container_runtime_is_rootless else '/root' }}" + podman_reference_uid: "{{ tmachine_container_runtime_tmachine_uid.stdout if tmachine_container_runtime_is_rootless else '0' }}" + - name: Create Podman test directory become: true ansible.builtin.file: @@ -78,52 +88,47 @@ var: podman_gateway_journal.stdout_lines when: podman_test_result.rc != 0 - - name: Resolve tmachine UID after Podman test failure - ansible.builtin.command: - argv: [id, --user, tmachine] - changed_when: false - when: podman_test_result.rc != 0 - register: podman_tmachine_uid - - name: Discover Podman containers after test failure become: true - become_user: tmachine + become_user: "{{ podman_reference_user }}" ansible.builtin.command: argv: [timeout, "15", podman, ps, --all, --quiet, --no-trunc] environment: - HOME: /home/tmachine - XDG_RUNTIME_DIR: "/run/user/{{ podman_tmachine_uid.stdout }}" + HOME: "{{ podman_reference_home }}" + XDG_RUNTIME_DIR: "/run/user/{{ podman_reference_uid }}" changed_when: false failed_when: false when: podman_test_result.rc != 0 register: podman_failed_containers - - name: Capture rootless Podman journal after test failure + - name: Capture Podman journal after test failure become: true - become_user: tmachine + become_user: "{{ podman_reference_user }}" ansible.builtin.command: - argv: [journalctl, --user, --unit, podman.service, --no-pager, --lines, "200"] + argv: >- + {{ ['journalctl'] + (['--user'] if tmachine_container_runtime_is_rootless else []) + + ['--unit', 'podman.service', '--no-pager', '--lines', '200'] }} environment: - HOME: /home/tmachine - XDG_RUNTIME_DIR: "/run/user/{{ podman_tmachine_uid.stdout }}" + HOME: "{{ podman_reference_home }}" + XDG_RUNTIME_DIR: "/run/user/{{ podman_reference_uid }}" changed_when: false failed_when: false when: podman_test_result.rc != 0 register: podman_service_journal - - name: Show rootless Podman journal after test failure + - name: Show Podman journal after test failure ansible.builtin.debug: var: podman_service_journal.stdout_lines when: podman_test_result.rc != 0 - name: Capture Podman container inspection after test failure become: true - become_user: tmachine + become_user: "{{ podman_reference_user }}" ansible.builtin.command: argv: [timeout, "15", podman, inspect, "{{ item }}"] environment: - HOME: /home/tmachine - XDG_RUNTIME_DIR: "/run/user/{{ podman_tmachine_uid.stdout }}" + HOME: "{{ podman_reference_home }}" + XDG_RUNTIME_DIR: "/run/user/{{ podman_reference_uid }}" changed_when: false failed_when: false when: podman_test_result.rc != 0 @@ -137,12 +142,12 @@ - name: Capture Podman container logs after test failure become: true - become_user: tmachine + become_user: "{{ podman_reference_user }}" ansible.builtin.command: argv: [timeout, "15", podman, logs, "{{ item }}"] environment: - HOME: /home/tmachine - XDG_RUNTIME_DIR: "/run/user/{{ podman_tmachine_uid.stdout }}" + HOME: "{{ podman_reference_home }}" + XDG_RUNTIME_DIR: "/run/user/{{ podman_reference_uid }}" changed_when: false failed_when: false when: podman_test_result.rc != 0 diff --git a/tests/ansible/playbooks/drivers/podman/userns-profile.yaml b/tests/ansible/playbooks/drivers/podman/userns-profile.yaml index ba5efc0f59..a3071c2833 100644 --- a/tests/ansible/playbooks/drivers/podman/userns-profile.yaml +++ b/tests/ansible/playbooks/drivers/podman/userns-profile.yaml @@ -12,26 +12,28 @@ - name: Wait for SSH ansible.builtin.wait_for_connection: - - name: Require the rootless Podman gateway user - become: true - ansible.builtin.command: - argv: - - systemctl - - show - - openshell-gateway.service - - --property - - User - - --value - changed_when: false - register: openshell_gateway_service_user + - name: Detect tmachine container runtime + ansible.builtin.include_role: + name: tmachine_container_runtime - - name: Assert the rootless Podman gateway user + - name: Require a Podman gateway ansible.builtin.assert: that: - - openshell_gateway_service_user.stdout == "tmachine" + - tmachine_container_runtime_name == "podman" fail_msg: >- - The Podman {{ podman_userns_profile }} suite requires a rootless Podman gateway - running as tmachine, not {{ openshell_gateway_service_user.stdout | default("unknown") }} + The Podman {{ podman_userns_profile }} suite requires a Podman gateway, not + {{ tmachine_container_runtime_name }} + + # podman_reference_user only selects who runs the direct `podman` + # reference command below; it must match the daemon's rootful/rootless + # mode. The captured reference file itself stays owned by tmachine + # (the inventory's ansible_user), since the archived test binary that + # later reads it always runs unprivileged as tmachine, in both modes. + - name: Resolve the Podman reference execution context + ansible.builtin.set_fact: + podman_reference_user: "{{ 'tmachine' if tmachine_container_runtime_is_rootless else 'root' }}" + podman_reference_home: "{{ '/home/tmachine' if tmachine_container_runtime_is_rootless else '/root' }}" + podman_reference_uid: "{{ tmachine_container_runtime_tmachine_uid.stdout if tmachine_container_runtime_is_rootless else '0' }}" - name: Apply the Podman user-namespace fixture become: true @@ -52,23 +54,14 @@ port: 17670 timeout: 60 - - name: Resolve tmachine UID - ansible.builtin.command: - argv: - - id - - -u - - tmachine - changed_when: false - register: tmachine_uid - - name: Capture direct Podman user-namespace mapping become: true - become_user: tmachine + become_user: "{{ podman_reference_user }}" ansible.builtin.command: argv: "{{ [\"podman\", \"run\", \"--rm\", \"--pull\", \"never\"] + podman_userns_reference_args + [\"--entrypoint\", \"/bin/cat\", podman_reference_image, \"/proc/self/uid_map\"] }}" environment: - HOME: /home/tmachine - XDG_RUNTIME_DIR: "/run/user/{{ tmachine_uid.stdout }}" + HOME: "{{ podman_reference_home }}" + XDG_RUNTIME_DIR: "/run/user/{{ podman_reference_uid }}" changed_when: false register: podman_userns_uid_map @@ -77,7 +70,7 @@ that: - podman_userns_uid_map.stdout | trim | length > 0 fail_msg: >- - Direct rootless Podman {{ podman_userns_profile }} reference container + Direct Podman {{ podman_userns_profile }} reference container returned no UID mapping - name: Create Podman test-input directory diff --git a/tests/ansible/playbooks/podman-rootful.yaml b/tests/ansible/playbooks/podman-rootful.yaml index d74a8bef9d..05eee89581 100644 --- a/tests/ansible/playbooks/podman-rootful.yaml +++ b/tests/ansible/playbooks/podman-rootful.yaml @@ -9,6 +9,18 @@ - name: Wait for SSH ansible.builtin.wait_for_connection: + # Rootful Podman's --userns auto allocates IDs from the "containers" + # subordinate ranges, which the Fedora cloud image does not define. + - name: Configure rootful Podman subordinate IDs + become: true + ansible.builtin.lineinfile: + path: "{{ item }}" + regexp: '^containers:' + line: 'containers:1000000:65536' + loop: + - /etc/subuid + - /etc/subgid + - name: Enable rootful Podman socket become: true ansible.builtin.systemd_service: diff --git a/tests/config.nix b/tests/config.nix index 4b5d5b7d96..0578f898fb 100644 --- a/tests/config.nix +++ b/tests/config.nix @@ -118,6 +118,13 @@ let openshell_conformance_test_bundle = "../artifacts/test-archives/${muslTarget}/openshell-conformance-tests.tar"; }; } + { + name = "policy-advisor"; + playbooks = [ "ansible/playbooks/conformance/policy-advisor.yaml" ]; + inputs = { + openshell_conformance_test_bundle = "../artifacts/test-archives/${muslTarget}/openshell-conformance-tests.tar"; + }; + } { name = "provider-refresh"; playbooks = [ "ansible/playbooks/features/provider-refresh/keycloak.yaml" ]; diff --git a/tests/suites/conformance/Cargo.lock b/tests/suites/conformance/Cargo.lock index 40d70e1686..89d8c0d392 100644 --- a/tests/suites/conformance/Cargo.lock +++ b/tests/suites/conformance/Cargo.lock @@ -30,6 +30,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "fastrand" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" + [[package]] name = "getrandom" version = "0.3.4" @@ -54,6 +60,12 @@ version = "0.2.189" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" +[[package]] +name = "linux-raw-sys" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" + [[package]] name = "lock_api" version = "0.4.14" @@ -80,6 +92,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "once_cell" +version = "1.21.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" + [[package]] name = "openshell-conformance" version = "0.0.0" @@ -87,6 +105,7 @@ dependencies = [ "rand", "serde", "serde_json", + "tempfile", "tokio", ] @@ -198,6 +217,19 @@ dependencies = [ "bitflags", ] +[[package]] +name = "rustix" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "891efababe418670775f199f0d233d84843c227a0949a883ce15b37c78d6629d" +dependencies = [ + "bitflags", + "errno", + "libc", + "linux-raw-sys", + "windows-sys", +] + [[package]] name = "scopeguard" version = "1.2.0" @@ -295,6 +327,19 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "tempfile" +version = "3.27.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" +dependencies = [ + "fastrand", + "getrandom", + "once_cell", + "rustix", + "windows-sys", +] + [[package]] name = "tokio" version = "1.53.1" diff --git a/tests/suites/conformance/cli/tests/policy_advisor.rs b/tests/suites/conformance/cli/tests/policy_advisor.rs new file mode 100644 index 0000000000..56bf18ba87 --- /dev/null +++ b/tests/suites/conformance/cli/tests/policy_advisor.rs @@ -0,0 +1,31 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Installed-artifact policy advisor conformance. + +use openshell_conformance::{ + MECHANISTIC_PROPOSAL_SCENARIO, OpenShellRunner, POLICY_LOCAL_SCENARIO, Scenario, +}; + +async fn run(scenario: &'static Scenario) { + let mut runner = + OpenShellRunner::from_env(scenario.name).expect("candidate openshell CLI is available"); + let result = async { + runner.check_gateway_status().await?; + scenario.run(&mut runner).await + } + .await; + if let Err(error) = runner.finish(result).await { + panic!("{} conformance scenario failed:\n{error}", scenario.name); + } +} + +#[tokio::test] +async fn mechanistic_proposal() { + run(&MECHANISTIC_PROPOSAL_SCENARIO).await; +} + +#[tokio::test] +async fn policy_local() { + run(&POLICY_LOCAL_SCENARIO).await; +} diff --git a/tests/suites/drivers/Cargo.lock b/tests/suites/drivers/Cargo.lock index ad7a993fa9..6b013452fe 100644 --- a/tests/suites/drivers/Cargo.lock +++ b/tests/suites/drivers/Cargo.lock @@ -30,6 +30,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "fastrand" +version = "2.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "da7c62ceae207dd37ea5b845da6a0696c799f85e97da1ab5b7910be3c1c80223" + [[package]] name = "getrandom" version = "0.3.4" @@ -54,6 +60,12 @@ version = "0.2.189" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" +[[package]] +name = "linux-raw-sys" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32a66949e030da00e8c7d4434b251670a91556f4144941d37452769c25d58a53" + [[package]] name = "lock_api" version = "0.4.14" @@ -80,6 +92,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "once_cell" +version = "1.21.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" + [[package]] name = "openshell-conformance" version = "0.0.0" @@ -87,6 +105,7 @@ dependencies = [ "rand", "serde", "serde_json", + "tempfile", "tokio", ] @@ -199,6 +218,19 @@ dependencies = [ "bitflags", ] +[[package]] +name = "rustix" +version = "1.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "891efababe418670775f199f0d233d84843c227a0949a883ce15b37c78d6629d" +dependencies = [ + "bitflags", + "errno", + "libc", + "linux-raw-sys", + "windows-sys", +] + [[package]] name = "scopeguard" version = "1.2.0" @@ -296,6 +328,19 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "tempfile" +version = "3.27.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" +dependencies = [ + "fastrand", + "getrandom", + "once_cell", + "rustix", + "windows-sys", +] + [[package]] name = "tokio" version = "1.53.1" diff --git a/tests/suites/drivers/podman/tests/default_userns.rs b/tests/suites/drivers/podman/tests/default_userns.rs index 7618e643af..f9aaf4354c 100644 --- a/tests/suites/drivers/podman/tests/default_userns.rs +++ b/tests/suites/drivers/podman/tests/default_userns.rs @@ -86,16 +86,38 @@ async fn configured_userns_matches_podman_reference() { } fn normalize_uid_map(value: &str) -> Option { - let mappings = value - .lines() - .filter_map(|line| { - let fields = line.split_whitespace().collect::>(); - (fields.len() == 3 - && fields - .iter() - .all(|field| field.bytes().all(|byte| byte.is_ascii_digit()))) - .then(|| fields.join(" ")) - }) - .collect::>(); - (!mappings.is_empty()).then(|| mappings.join("\n")) + let mut mappings: Vec<(u64, u64, u64)> = Vec::new(); + for line in value.lines() { + let fields = line + .split_whitespace() + .map(str::parse::) + .collect::, _>>(); + let Ok(fields) = fields else { continue }; + let [inside, outside, length] = fields.as_slice() else { + continue; + }; + if let Some(previous) = mappings.last_mut() + && previous.0.checked_add(previous.2) == Some(*inside) + && previous.1.checked_add(previous.2) == Some(*outside) + { + previous.2 += *length; + } else { + mappings.push((*inside, *outside, *length)); + } + } + (!mappings.is_empty()).then(|| { + mappings + .iter() + .map(|(inside, outside, length)| format!("{inside} {outside} {length}")) + .collect::>() + .join("\n") + }) +} + +#[test] +fn adjacent_uid_ranges_match_a_combined_mapping() { + assert_eq!( + normalize_uid_map("0 0 1\n1 1 65535\n"), + normalize_uid_map("0 0 65536\n") + ); } diff --git a/tests/suites/features/Cargo.lock b/tests/suites/features/Cargo.lock index 20ebe58016..b952a45ab1 100644 --- a/tests/suites/features/Cargo.lock +++ b/tests/suites/features/Cargo.lock @@ -882,6 +882,7 @@ dependencies = [ "rand", "serde", "serde_json", + "tempfile", "tokio", ]