From b49f84c7343ff6f3b272f46092dfee692779b12d Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Thu, 7 May 2026 10:30:33 -0700 Subject: [PATCH 1/5] feat(providers): support sandbox provider attach lifecycle Closes #1171 Adds sandbox provider list, attach, and detach API/CLI support while keeping provider policy and credential resolution derived from current sandbox attachments. --- crates/openshell-cli/src/main.rs | 77 ++++ crates/openshell-cli/src/run.rs | 127 ++++- .../tests/ensure_providers_integration.rs | 43 +- .../openshell-cli/tests/mtls_integration.rs | 27 ++ .../tests/provider_commands_integration.rs | 43 +- .../sandbox_create_lifecycle_integration.rs | 44 +- .../sandbox_name_fallback_integration.rs | 42 +- crates/openshell-server/src/auth/authz.rs | 12 + crates/openshell-server/src/grpc/mod.rs | 55 ++- crates/openshell-server/src/grpc/policy.rs | 97 ++++ crates/openshell-server/src/grpc/sandbox.rs | 436 +++++++++++++++++- .../tests/auth_endpoint_integration.rs | 30 ++ .../tests/edge_tunnel_auth.rs | 27 ++ .../tests/multiplex_integration.rs | 27 ++ .../tests/multiplex_tls_integration.rs | 27 ++ .../tests/supervisor_relay_integration.rs | 18 + .../tests/ws_tunnel_integration.rs | 27 ++ proto/openshell.proto | 53 +++ 18 files changed, 1141 insertions(+), 71 deletions(-) diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index cd14568ef..25fa07cf2 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -1260,6 +1260,45 @@ enum SandboxCommands { #[arg(add = ArgValueCompleter::new(completers::complete_sandbox_names))] name: Option, }, + + /// Manage providers attached to a sandbox. + #[command(subcommand)] + Provider(SandboxProviderCommands), +} + +#[derive(Subcommand, Debug)] +enum SandboxProviderCommands { + /// List providers attached to a sandbox. + #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] + List { + /// Sandbox name (defaults to last-used sandbox). + #[arg(add = ArgValueCompleter::new(completers::complete_sandbox_names))] + name: Option, + }, + + /// Attach a provider to a sandbox. + #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] + Attach { + /// Sandbox name. + #[arg(add = ArgValueCompleter::new(completers::complete_sandbox_names))] + name: String, + + /// Provider name to attach. + #[arg(add = ArgValueCompleter::new(completers::complete_provider_names))] + provider: String, + }, + + /// Detach a provider from a sandbox. + #[command(help_template = LEAF_HELP_TEMPLATE, next_help_heading = "FLAGS")] + Detach { + /// Sandbox name. + #[arg(add = ArgValueCompleter::new(completers::complete_sandbox_names))] + name: String, + + /// Provider name to detach. + #[arg(add = ArgValueCompleter::new(completers::complete_provider_names))] + provider: String, + }, } #[derive(Subcommand, Debug)] @@ -2385,6 +2424,20 @@ async fn main() -> Result<()> { let name = resolve_sandbox_name(name, &ctx.name)?; run::print_ssh_config(&ctx.name, &name); } + SandboxCommands::Provider(command) => match command { + SandboxProviderCommands::List { name } => { + let name = resolve_sandbox_name(name, &ctx.name)?; + run::sandbox_provider_list(endpoint, &name, &tls).await?; + } + SandboxProviderCommands::Attach { name, provider } => { + run::sandbox_provider_attach(endpoint, &name, &provider, &tls) + .await?; + } + SandboxProviderCommands::Detach { name, provider } => { + run::sandbox_provider_detach(endpoint, &name, &provider, &tls) + .await?; + } + }, } } } @@ -2721,6 +2774,30 @@ mod tests { ); } + #[test] + fn sandbox_provider_subcommands_parse() { + let cli = Cli::try_parse_from([ + "openshell", + "sandbox", + "provider", + "attach", + "work-sandbox", + "work-github", + ]) + .expect("sandbox provider attach should parse"); + + let Some(Commands::Sandbox { + command: + Some(SandboxCommands::Provider(SandboxProviderCommands::Attach { name, provider })), + }) = cli.command + else { + panic!("expected sandbox provider attach command"); + }; + + assert_eq!(name, "work-sandbox"); + assert_eq!(provider, "work-github"); + } + #[test] fn completions_policy_flag_falls_back_to_file_paths() { let temp = tempfile::tempdir().expect("failed to create tempdir"); diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index 102bc87ab..df6b3382e 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -25,18 +25,19 @@ use openshell_bootstrap::{ }; use openshell_core::proto::ProviderProfileCategory; use openshell_core::proto::{ - ApproveAllDraftChunksRequest, ApproveDraftChunkRequest, ClearDraftChunksRequest, - CreateProviderRequest, CreateSandboxRequest, DeleteProviderProfileRequest, - DeleteProviderRequest, DeleteSandboxRequest, ExecSandboxRequest, GetClusterInferenceRequest, + ApproveAllDraftChunksRequest, ApproveDraftChunkRequest, AttachSandboxProviderRequest, + ClearDraftChunksRequest, CreateProviderRequest, CreateSandboxRequest, + DeleteProviderProfileRequest, DeleteProviderRequest, DeleteSandboxRequest, + DetachSandboxProviderRequest, ExecSandboxRequest, GetClusterInferenceRequest, GetDraftHistoryRequest, GetDraftPolicyRequest, GetGatewayConfigRequest, GetProviderProfileRequest, GetProviderRequest, GetSandboxConfigRequest, GetSandboxLogsRequest, GetSandboxPolicyStatusRequest, GetSandboxRequest, HealthRequest, ImportProviderProfilesRequest, LintProviderProfilesRequest, ListProviderProfilesRequest, ListProvidersRequest, - ListSandboxPoliciesRequest, ListSandboxesRequest, PolicySource, PolicyStatus, Provider, - ProviderProfile, ProviderProfileDiagnostic, ProviderProfileImportItem, RejectDraftChunkRequest, - Sandbox, SandboxPhase, SandboxPolicy, SandboxSpec, SandboxTemplate, SetClusterInferenceRequest, - SettingScope, SettingValue, UpdateConfigRequest, UpdateProviderRequest, WatchSandboxRequest, - exec_sandbox_event, setting_value, + ListSandboxPoliciesRequest, ListSandboxProvidersRequest, ListSandboxesRequest, PolicySource, + PolicyStatus, Provider, ProviderProfile, ProviderProfileDiagnostic, ProviderProfileImportItem, + RejectDraftChunkRequest, Sandbox, SandboxPhase, SandboxPolicy, SandboxSpec, SandboxTemplate, + SetClusterInferenceRequest, SettingScope, SettingValue, UpdateConfigRequest, + UpdateProviderRequest, WatchSandboxRequest, exec_sandbox_event, setting_value, }; use openshell_core::settings::{self, SettingValueKind}; use openshell_core::{ObjectId, ObjectName}; @@ -2512,6 +2513,116 @@ pub async fn sandbox_list( Ok(()) } +pub async fn sandbox_provider_list(server: &str, name: &str, tls: &TlsOptions) -> Result<()> { + let mut client = grpc_client(server, tls).await?; + let response = client + .list_sandbox_providers(ListSandboxProvidersRequest { + sandbox_name: name.to_string(), + }) + .await + .into_diagnostic()?; + let providers = response.into_inner().providers; + + if providers.is_empty() { + println!("No providers attached to sandbox {name}."); + return Ok(()); + } + + print_provider_attachment_table(&providers); + Ok(()) +} + +pub async fn sandbox_provider_attach( + server: &str, + name: &str, + provider: &str, + tls: &TlsOptions, +) -> Result<()> { + let mut client = grpc_client(server, tls).await?; + let response = client + .attach_sandbox_provider(AttachSandboxProviderRequest { + sandbox_name: name.to_string(), + provider_name: provider.to_string(), + }) + .await + .into_diagnostic()? + .into_inner(); + + if response.attached { + println!( + "{} Attached provider {} to sandbox {}", + "✓".green().bold(), + provider, + name + ); + } else { + println!("Provider {provider} is already attached to sandbox {name}."); + } + Ok(()) +} + +pub async fn sandbox_provider_detach( + server: &str, + name: &str, + provider: &str, + tls: &TlsOptions, +) -> Result<()> { + let mut client = grpc_client(server, tls).await?; + let response = client + .detach_sandbox_provider(DetachSandboxProviderRequest { + sandbox_name: name.to_string(), + provider_name: provider.to_string(), + }) + .await + .into_diagnostic()? + .into_inner(); + + if response.detached { + println!( + "{} Detached provider {} from sandbox {}", + "✓".green().bold(), + provider, + name + ); + } else { + println!("Provider {provider} was not attached to sandbox {name}."); + } + Ok(()) +} + +fn print_provider_attachment_table(providers: &[Provider]) { + let name_width = providers + .iter() + .map(|provider| provider.object_name().len()) + .max() + .unwrap_or(4) + .max(4); + let type_width = providers + .iter() + .map(|provider| provider.r#type.len()) + .max() + .unwrap_or(4) + .max(4); + + println!( + "{:, + ) -> Result, Status> { + Ok(Response::new(ListSandboxProvidersResponse::default())) + } + + async fn attach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new(AttachSandboxProviderResponse::default())) + } + + async fn detach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new(DetachSandboxProviderResponse::default())) + } + async fn delete_sandbox( &self, _request: tonic::Request, diff --git a/crates/openshell-cli/tests/mtls_integration.rs b/crates/openshell-cli/tests/mtls_integration.rs index 866048a81..e833e7af9 100644 --- a/crates/openshell-cli/tests/mtls_integration.rs +++ b/crates/openshell-cli/tests/mtls_integration.rs @@ -99,6 +99,33 @@ impl OpenShell for TestOpenShell { )) } + async fn list_sandbox_providers( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::ListSandboxProvidersResponse::default(), + )) + } + + async fn attach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::AttachSandboxProviderResponse::default(), + )) + } + + async fn detach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::DetachSandboxProviderResponse::default(), + )) + } + async fn delete_sandbox( &self, _request: tonic::Request, diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index 55ed69500..54de10511 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -5,16 +5,18 @@ use openshell_cli::run; use openshell_cli::tls::TlsOptions; use openshell_core::proto::open_shell_server::{OpenShell, OpenShellServer}; use openshell_core::proto::{ - CreateProviderRequest, CreateSandboxRequest, CreateSshSessionRequest, CreateSshSessionResponse, - DeleteProviderRequest, DeleteProviderResponse, DeleteSandboxRequest, DeleteSandboxResponse, - ExecSandboxEvent, ExecSandboxRequest, GatewayMessage, GetGatewayConfigRequest, - GetGatewayConfigResponse, GetProviderRequest, GetSandboxConfigRequest, - GetSandboxConfigResponse, GetSandboxProviderEnvironmentRequest, - GetSandboxProviderEnvironmentResponse, GetSandboxRequest, HealthRequest, HealthResponse, - ListProvidersRequest, ListProvidersResponse, ListSandboxesRequest, ListSandboxesResponse, - Provider, ProviderProfile, ProviderResponse, RevokeSshSessionRequest, RevokeSshSessionResponse, - SandboxResponse, SandboxStreamEvent, ServiceStatus, SupervisorMessage, UpdateProviderRequest, - WatchSandboxRequest, + AttachSandboxProviderRequest, AttachSandboxProviderResponse, CreateProviderRequest, + CreateSandboxRequest, CreateSshSessionRequest, CreateSshSessionResponse, DeleteProviderRequest, + DeleteProviderResponse, DeleteSandboxRequest, DeleteSandboxResponse, + DetachSandboxProviderRequest, DetachSandboxProviderResponse, ExecSandboxEvent, + ExecSandboxRequest, GatewayMessage, GetGatewayConfigRequest, GetGatewayConfigResponse, + GetProviderRequest, GetSandboxConfigRequest, GetSandboxConfigResponse, + GetSandboxProviderEnvironmentRequest, GetSandboxProviderEnvironmentResponse, GetSandboxRequest, + HealthRequest, HealthResponse, ListProvidersRequest, ListProvidersResponse, + ListSandboxProvidersRequest, ListSandboxProvidersResponse, ListSandboxesRequest, + ListSandboxesResponse, Provider, ProviderProfile, ProviderResponse, RevokeSshSessionRequest, + RevokeSshSessionResponse, SandboxResponse, SandboxStreamEvent, ServiceStatus, + SupervisorMessage, UpdateProviderRequest, WatchSandboxRequest, }; use openshell_core::{ObjectId, ObjectName}; use rcgen::{ @@ -104,6 +106,27 @@ impl OpenShell for TestOpenShell { Ok(Response::new(ListSandboxesResponse::default())) } + async fn list_sandbox_providers( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new(ListSandboxProvidersResponse::default())) + } + + async fn attach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new(AttachSandboxProviderResponse::default())) + } + + async fn detach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new(DetachSandboxProviderResponse::default())) + } + async fn delete_sandbox( &self, _request: tonic::Request, diff --git a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs index a8e359d54..eb28a18b3 100644 --- a/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs +++ b/crates/openshell-cli/tests/sandbox_create_lifecycle_integration.rs @@ -6,16 +6,19 @@ use openshell_cli::run; use openshell_cli::tls::TlsOptions; use openshell_core::proto::open_shell_server::{OpenShell, OpenShellServer}; use openshell_core::proto::{ - CreateProviderRequest, CreateSandboxRequest, CreateSshSessionRequest, CreateSshSessionResponse, - DeleteProviderRequest, DeleteProviderResponse, DeleteSandboxRequest, DeleteSandboxResponse, - ExecSandboxEvent, ExecSandboxRequest, GatewayMessage, GetGatewayConfigRequest, - GetGatewayConfigResponse, GetProviderRequest, GetSandboxConfigRequest, - GetSandboxConfigResponse, GetSandboxProviderEnvironmentRequest, - GetSandboxProviderEnvironmentResponse, GetSandboxRequest, HealthRequest, HealthResponse, - ListProvidersRequest, ListProvidersResponse, ListSandboxesRequest, ListSandboxesResponse, - PlatformEvent, ProviderResponse, RevokeSshSessionRequest, RevokeSshSessionResponse, Sandbox, - SandboxPhase, SandboxResponse, SandboxStreamEvent, ServiceStatus, SupervisorMessage, - UpdateProviderRequest, WatchSandboxRequest, sandbox_stream_event, + AttachSandboxProviderRequest, AttachSandboxProviderResponse, CreateProviderRequest, + CreateSandboxRequest, CreateSshSessionRequest, CreateSshSessionResponse, DeleteProviderRequest, + DeleteProviderResponse, DeleteSandboxRequest, DeleteSandboxResponse, + DetachSandboxProviderRequest, DetachSandboxProviderResponse, ExecSandboxEvent, + ExecSandboxRequest, GatewayMessage, GetGatewayConfigRequest, GetGatewayConfigResponse, + GetProviderRequest, GetSandboxConfigRequest, GetSandboxConfigResponse, + GetSandboxProviderEnvironmentRequest, GetSandboxProviderEnvironmentResponse, GetSandboxRequest, + HealthRequest, HealthResponse, ListProvidersRequest, ListProvidersResponse, + ListSandboxProvidersRequest, ListSandboxProvidersResponse, ListSandboxesRequest, + ListSandboxesResponse, PlatformEvent, ProviderResponse, RevokeSshSessionRequest, + RevokeSshSessionResponse, Sandbox, SandboxPhase, SandboxResponse, SandboxStreamEvent, + ServiceStatus, SupervisorMessage, UpdateProviderRequest, WatchSandboxRequest, + sandbox_stream_event, }; use rcgen::{ BasicConstraints, Certificate, CertificateParams, ExtendedKeyUsagePurpose, IsCa, KeyPair, @@ -151,6 +154,27 @@ impl OpenShell for TestOpenShell { Ok(Response::new(ListSandboxesResponse::default())) } + async fn list_sandbox_providers( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new(ListSandboxProvidersResponse::default())) + } + + async fn attach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new(AttachSandboxProviderResponse::default())) + } + + async fn detach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new(DetachSandboxProviderResponse::default())) + } + async fn delete_sandbox( &self, request: tonic::Request, diff --git a/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs b/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs index ac1ff37c6..7e6ea68b8 100644 --- a/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs +++ b/crates/openshell-cli/tests/sandbox_name_fallback_integration.rs @@ -6,15 +6,18 @@ use openshell_cli::run; use openshell_cli::tls::TlsOptions; use openshell_core::proto::open_shell_server::{OpenShell, OpenShellServer}; use openshell_core::proto::{ - CreateProviderRequest, CreateSandboxRequest, CreateSshSessionRequest, CreateSshSessionResponse, - DeleteProviderRequest, DeleteProviderResponse, DeleteSandboxRequest, DeleteSandboxResponse, - ExecSandboxEvent, ExecSandboxRequest, GatewayMessage, GetGatewayConfigRequest, - GetGatewayConfigResponse, GetProviderRequest, GetSandboxConfigRequest, - GetSandboxConfigResponse, GetSandboxProviderEnvironmentRequest, - GetSandboxProviderEnvironmentResponse, GetSandboxRequest, HealthRequest, HealthResponse, - ListProvidersRequest, ListProvidersResponse, ListSandboxesRequest, ListSandboxesResponse, - ProviderResponse, Sandbox, SandboxPolicy, SandboxResponse, SandboxStreamEvent, ServiceStatus, - SupervisorMessage, UpdateProviderRequest, WatchSandboxRequest, + AttachSandboxProviderRequest, AttachSandboxProviderResponse, CreateProviderRequest, + CreateSandboxRequest, CreateSshSessionRequest, CreateSshSessionResponse, DeleteProviderRequest, + DeleteProviderResponse, DeleteSandboxRequest, DeleteSandboxResponse, + DetachSandboxProviderRequest, DetachSandboxProviderResponse, ExecSandboxEvent, + ExecSandboxRequest, GatewayMessage, GetGatewayConfigRequest, GetGatewayConfigResponse, + GetProviderRequest, GetSandboxConfigRequest, GetSandboxConfigResponse, + GetSandboxProviderEnvironmentRequest, GetSandboxProviderEnvironmentResponse, GetSandboxRequest, + HealthRequest, HealthResponse, ListProvidersRequest, ListProvidersResponse, + ListSandboxProvidersRequest, ListSandboxProvidersResponse, ListSandboxesRequest, + ListSandboxesResponse, ProviderResponse, Sandbox, SandboxPolicy, SandboxResponse, + SandboxStreamEvent, ServiceStatus, SupervisorMessage, UpdateProviderRequest, + WatchSandboxRequest, }; use rcgen::{ BasicConstraints, Certificate, CertificateParams, ExtendedKeyUsagePurpose, IsCa, KeyPair, @@ -129,6 +132,27 @@ impl OpenShell for TestOpenShell { Ok(Response::new(ListSandboxesResponse::default())) } + async fn list_sandbox_providers( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new(ListSandboxProvidersResponse::default())) + } + + async fn attach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new(AttachSandboxProviderResponse::default())) + } + + async fn detach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new(DetachSandboxProviderResponse::default())) + } + async fn delete_sandbox( &self, _request: tonic::Request, diff --git a/crates/openshell-server/src/auth/authz.rs b/crates/openshell-server/src/auth/authz.rs index 05ac19354..a3f73117b 100644 --- a/crates/openshell-server/src/auth/authz.rs +++ b/crates/openshell-server/src/auth/authz.rs @@ -41,6 +41,10 @@ const SCOPED_METHODS: &[(&str, &str)] = &[ // sandbox:read ("/openshell.v1.OpenShell/GetSandbox", "sandbox:read"), ("/openshell.v1.OpenShell/ListSandboxes", "sandbox:read"), + ( + "/openshell.v1.OpenShell/ListSandboxProviders", + "sandbox:read", + ), ("/openshell.v1.OpenShell/WatchSandbox", "sandbox:read"), ("/openshell.v1.OpenShell/GetSandboxLogs", "sandbox:read"), ( @@ -57,6 +61,14 @@ const SCOPED_METHODS: &[(&str, &str)] = &[ ("/openshell.v1.OpenShell/ExecSandbox", "sandbox:write"), ("/openshell.v1.OpenShell/CreateSshSession", "sandbox:write"), ("/openshell.v1.OpenShell/RevokeSshSession", "sandbox:write"), + ( + "/openshell.v1.OpenShell/AttachSandboxProvider", + "sandbox:write", + ), + ( + "/openshell.v1.OpenShell/DetachSandboxProvider", + "sandbox:write", + ), // provider:read ("/openshell.v1.OpenShell/GetProvider", "provider:read"), ("/openshell.v1.OpenShell/ListProviders", "provider:read"), diff --git a/crates/openshell-server/src/grpc/mod.rs b/crates/openshell-server/src/grpc/mod.rs index 87af948ed..ebb8b1021 100644 --- a/crates/openshell-server/src/grpc/mod.rs +++ b/crates/openshell-server/src/grpc/mod.rs @@ -10,27 +10,29 @@ mod validation; use openshell_core::proto::{ ApproveAllDraftChunksRequest, ApproveAllDraftChunksResponse, ApproveDraftChunkRequest, - ApproveDraftChunkResponse, ClearDraftChunksRequest, ClearDraftChunksResponse, - CreateProviderRequest, CreateSandboxRequest, CreateSshSessionRequest, CreateSshSessionResponse, - DeleteProviderProfileRequest, DeleteProviderProfileResponse, DeleteProviderRequest, - DeleteProviderResponse, DeleteSandboxRequest, DeleteSandboxResponse, EditDraftChunkRequest, - EditDraftChunkResponse, ExecSandboxEvent, ExecSandboxRequest, GatewayMessage, - GetDraftHistoryRequest, GetDraftHistoryResponse, GetDraftPolicyRequest, GetDraftPolicyResponse, - GetGatewayConfigRequest, GetGatewayConfigResponse, GetProviderProfileRequest, - GetProviderRequest, GetSandboxConfigRequest, GetSandboxConfigResponse, GetSandboxLogsRequest, + ApproveDraftChunkResponse, AttachSandboxProviderRequest, AttachSandboxProviderResponse, + ClearDraftChunksRequest, ClearDraftChunksResponse, CreateProviderRequest, CreateSandboxRequest, + CreateSshSessionRequest, CreateSshSessionResponse, DeleteProviderProfileRequest, + DeleteProviderProfileResponse, DeleteProviderRequest, DeleteProviderResponse, + DeleteSandboxRequest, DeleteSandboxResponse, DetachSandboxProviderRequest, + DetachSandboxProviderResponse, EditDraftChunkRequest, EditDraftChunkResponse, ExecSandboxEvent, + ExecSandboxRequest, GatewayMessage, GetDraftHistoryRequest, GetDraftHistoryResponse, + GetDraftPolicyRequest, GetDraftPolicyResponse, GetGatewayConfigRequest, + GetGatewayConfigResponse, GetProviderProfileRequest, GetProviderRequest, + GetSandboxConfigRequest, GetSandboxConfigResponse, GetSandboxLogsRequest, GetSandboxLogsResponse, GetSandboxPolicyStatusRequest, GetSandboxPolicyStatusResponse, GetSandboxProviderEnvironmentRequest, GetSandboxProviderEnvironmentResponse, GetSandboxRequest, HealthRequest, HealthResponse, ImportProviderProfilesRequest, ImportProviderProfilesResponse, LintProviderProfilesRequest, LintProviderProfilesResponse, ListProviderProfilesRequest, ListProviderProfilesResponse, ListProvidersRequest, ListProvidersResponse, - ListSandboxPoliciesRequest, ListSandboxPoliciesResponse, ListSandboxesRequest, - ListSandboxesResponse, ProviderProfileResponse, ProviderResponse, PushSandboxLogsRequest, - PushSandboxLogsResponse, RejectDraftChunkRequest, RejectDraftChunkResponse, RelayFrame, - ReportPolicyStatusRequest, ReportPolicyStatusResponse, RevokeSshSessionRequest, - RevokeSshSessionResponse, SandboxResponse, SandboxStreamEvent, ServiceStatus, - SubmitPolicyAnalysisRequest, SubmitPolicyAnalysisResponse, SupervisorMessage, - UndoDraftChunkRequest, UndoDraftChunkResponse, UpdateConfigRequest, UpdateConfigResponse, - UpdateProviderRequest, WatchSandboxRequest, open_shell_server::OpenShell, + ListSandboxPoliciesRequest, ListSandboxPoliciesResponse, ListSandboxProvidersRequest, + ListSandboxProvidersResponse, ListSandboxesRequest, ListSandboxesResponse, + ProviderProfileResponse, ProviderResponse, PushSandboxLogsRequest, PushSandboxLogsResponse, + RejectDraftChunkRequest, RejectDraftChunkResponse, RelayFrame, ReportPolicyStatusRequest, + ReportPolicyStatusResponse, RevokeSshSessionRequest, RevokeSshSessionResponse, SandboxResponse, + SandboxStreamEvent, ServiceStatus, SubmitPolicyAnalysisRequest, SubmitPolicyAnalysisResponse, + SupervisorMessage, UndoDraftChunkRequest, UndoDraftChunkResponse, UpdateConfigRequest, + UpdateConfigResponse, UpdateProviderRequest, WatchSandboxRequest, open_shell_server::OpenShell, }; use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; @@ -199,6 +201,27 @@ impl OpenShell for OpenShellService { sandbox::handle_list_sandboxes(&self.state, request).await } + async fn list_sandbox_providers( + &self, + request: Request, + ) -> Result, Status> { + sandbox::handle_list_sandbox_providers(&self.state, request).await + } + + async fn attach_sandbox_provider( + &self, + request: Request, + ) -> Result, Status> { + sandbox::handle_attach_sandbox_provider(&self.state, request).await + } + + async fn detach_sandbox_provider( + &self, + request: Request, + ) -> Result, Status> { + sandbox::handle_detach_sandbox_provider(&self.state, request).await + } + async fn delete_sandbox( &self, request: Request, diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 2c62c930a..56fe8f199 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -3322,6 +3322,103 @@ mod tests { assert_eq!(v2_env.get("GITHUB_TOKEN"), Some(&"ghp-test".to_string())); } + #[tokio::test] + async fn sandbox_config_and_provider_env_follow_attached_provider_lifecycle() { + use crate::grpc::sandbox::{ + handle_attach_sandbox_provider, handle_detach_sandbox_provider, + }; + use openshell_core::proto::{ + AttachSandboxProviderRequest, DetachSandboxProviderRequest, + GetSandboxProviderEnvironmentRequest, + }; + + let state = test_server_state().await; + enable_providers_v2(&state).await; + state + .store + .put_message(&test_provider("work-github", "github")) + .await + .unwrap(); + state + .store + .put_message(&test_sandbox( + "sb-attach-lifecycle", + "attach-lifecycle", + test_policy_with_rule("sandbox_only", "sandbox.example.com"), + Vec::new(), + )) + .await + .unwrap(); + + let baseline_policy = get_sandbox_policy(&state, "sb-attach-lifecycle").await; + assert!( + !baseline_policy + .network_policies + .contains_key("_provider_work_github") + ); + + handle_attach_sandbox_provider( + &state, + Request::new(AttachSandboxProviderRequest { + sandbox_name: "attach-lifecycle".to_string(), + provider_name: "work-github".to_string(), + }), + ) + .await + .unwrap(); + + let attached_policy = get_sandbox_policy(&state, "sb-attach-lifecycle").await; + assert!( + attached_policy + .network_policies + .contains_key("_provider_work_github") + ); + + let attached_env = handle_get_sandbox_provider_environment( + &state, + Request::new(GetSandboxProviderEnvironmentRequest { + sandbox_id: "sb-attach-lifecycle".to_string(), + }), + ) + .await + .unwrap() + .into_inner() + .environment; + assert_eq!( + attached_env.get("GITHUB_TOKEN"), + Some(&"ghp-test".to_string()) + ); + + handle_detach_sandbox_provider( + &state, + Request::new(DetachSandboxProviderRequest { + sandbox_name: "attach-lifecycle".to_string(), + provider_name: "work-github".to_string(), + }), + ) + .await + .unwrap(); + + let detached_policy = get_sandbox_policy(&state, "sb-attach-lifecycle").await; + assert!( + !detached_policy + .network_policies + .contains_key("_provider_work_github") + ); + + let detached_env = handle_get_sandbox_provider_environment( + &state, + Request::new(GetSandboxProviderEnvironmentRequest { + sandbox_id: "sb-attach-lifecycle".to_string(), + }), + ) + .await + .unwrap() + .into_inner() + .environment; + assert!(!detached_env.contains_key("GITHUB_TOKEN")); + } + #[tokio::test] async fn global_policy_suppresses_provider_profile_layers_when_v2_enabled() { use openshell_core::proto::{ diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index ed1b4cdfc..58220a03b 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -13,11 +13,13 @@ use crate::ServerState; use crate::persistence::{ObjectType, generate_name}; use futures::future; use openshell_core::proto::{ - CreateSandboxRequest, CreateSshSessionRequest, CreateSshSessionResponse, DeleteSandboxRequest, - DeleteSandboxResponse, ExecSandboxEvent, ExecSandboxExit, ExecSandboxRequest, - ExecSandboxStderr, ExecSandboxStdout, GetSandboxRequest, ListSandboxesRequest, - ListSandboxesResponse, RevokeSshSessionRequest, RevokeSshSessionResponse, SandboxResponse, - SandboxStreamEvent, WatchSandboxRequest, + AttachSandboxProviderRequest, AttachSandboxProviderResponse, CreateSandboxRequest, + CreateSshSessionRequest, CreateSshSessionResponse, DeleteSandboxRequest, DeleteSandboxResponse, + DetachSandboxProviderRequest, DetachSandboxProviderResponse, ExecSandboxEvent, ExecSandboxExit, + ExecSandboxRequest, ExecSandboxStderr, ExecSandboxStdout, GetSandboxRequest, + ListSandboxProvidersRequest, ListSandboxProvidersResponse, ListSandboxesRequest, + ListSandboxesResponse, Provider, RevokeSshSessionRequest, RevokeSshSessionResponse, + SandboxResponse, SandboxStreamEvent, WatchSandboxRequest, }; use openshell_core::proto::{Sandbox, SandboxPhase, SandboxTemplate, SshSession}; use prost::Message; @@ -31,12 +33,12 @@ use tracing::{info, warn}; use russh::ChannelMsg; use russh::client::AuthResult; -use super::provider::is_valid_env_key; +use super::provider::{get_provider_record, is_valid_env_key}; use super::validation::{ level_matches, source_matches, validate_exec_request_fields, validate_policy_safety, validate_sandbox_spec, }; -use super::{MAX_PAGE_SIZE, clamp_limit, current_time_ms}; +use super::{MAX_PAGE_SIZE, MAX_PROVIDERS, clamp_limit, current_time_ms}; // --------------------------------------------------------------------------- // Sandbox lifecycle handlers @@ -66,7 +68,7 @@ pub(super) async fn handle_create_sandbox( for name in &spec.providers { state .store - .get_message_by_name::(name) + .get_message_by_name::(name) .await .map_err(|e| Status::internal(format!("fetch provider failed: {e}")))? .ok_or_else(|| Status::failed_precondition(format!("provider '{name}' not found")))?; @@ -192,6 +194,128 @@ pub(super) async fn handle_list_sandboxes( Ok(Response::new(ListSandboxesResponse { sandboxes })) } +pub(super) async fn handle_list_sandbox_providers( + state: &Arc, + request: Request, +) -> Result, Status> { + let sandbox = sandbox_by_name(state, &request.into_inner().sandbox_name).await?; + let providers = providers_for_sandbox(state, &sandbox).await?; + Ok(Response::new(ListSandboxProvidersResponse { providers })) +} + +pub(super) async fn handle_attach_sandbox_provider( + state: &Arc, + request: Request, +) -> Result, Status> { + let request = request.into_inner(); + if request.provider_name.is_empty() { + return Err(Status::invalid_argument("provider_name is required")); + } + + get_provider_record(state.store.as_ref(), &request.provider_name) + .await + .map_err(|err| { + if err.code() == tonic::Code::NotFound { + Status::failed_precondition(format!( + "provider '{}' not found", + request.provider_name + )) + } else { + err + } + })?; + + let mut sandbox = sandbox_by_name(state, &request.sandbox_name).await?; + let sandbox_name = sandbox + .metadata + .as_ref() + .map_or_else(String::new, |metadata| metadata.name.clone()); + let spec = sandbox + .spec + .as_mut() + .ok_or_else(|| Status::failed_precondition("sandbox spec is missing"))?; + + dedupe_provider_names(&mut spec.providers); + let attached = if spec + .providers + .iter() + .any(|name| name == &request.provider_name) + { + false + } else { + if spec.providers.len() >= MAX_PROVIDERS { + return Err(Status::invalid_argument(format!( + "providers list exceeds maximum ({MAX_PROVIDERS})" + ))); + } + spec.providers.push(request.provider_name.clone()); + true + }; + validate_sandbox_spec(&sandbox_name, spec)?; + + state + .store + .put_message(&sandbox) + .await + .map_err(|e| Status::internal(format!("persist sandbox failed: {e}")))?; + + info!( + sandbox_name = %request.sandbox_name, + provider_name = %request.provider_name, + attached, + "AttachSandboxProvider request completed successfully" + ); + + Ok(Response::new(AttachSandboxProviderResponse { + sandbox: Some(sandbox), + attached, + })) +} + +pub(super) async fn handle_detach_sandbox_provider( + state: &Arc, + request: Request, +) -> Result, Status> { + let request = request.into_inner(); + if request.provider_name.is_empty() { + return Err(Status::invalid_argument("provider_name is required")); + } + + let mut sandbox = sandbox_by_name(state, &request.sandbox_name).await?; + let sandbox_name = sandbox + .metadata + .as_ref() + .map_or_else(String::new, |metadata| metadata.name.clone()); + let spec = sandbox + .spec + .as_mut() + .ok_or_else(|| Status::failed_precondition("sandbox spec is missing"))?; + + let before_len = spec.providers.len(); + spec.providers.retain(|name| name != &request.provider_name); + let detached = spec.providers.len() != before_len; + dedupe_provider_names(&mut spec.providers); + validate_sandbox_spec(&sandbox_name, spec)?; + + state + .store + .put_message(&sandbox) + .await + .map_err(|e| Status::internal(format!("persist sandbox failed: {e}")))?; + + info!( + sandbox_name = %request.sandbox_name, + provider_name = %request.provider_name, + detached, + "DetachSandboxProvider request completed successfully" + ); + + Ok(Response::new(DetachSandboxProviderResponse { + sandbox: Some(sandbox), + detached, + })) +} + pub(super) async fn handle_delete_sandbox( state: &Arc, request: Request, @@ -206,6 +330,56 @@ pub(super) async fn handle_delete_sandbox( Ok(Response::new(DeleteSandboxResponse { deleted })) } +async fn sandbox_by_name(state: &Arc, name: &str) -> Result { + if name.is_empty() { + return Err(Status::invalid_argument("sandbox_name is required")); + } + + state + .store + .get_message_by_name::(name) + .await + .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? + .ok_or_else(|| Status::not_found("sandbox not found")) +} + +async fn providers_for_sandbox( + state: &Arc, + sandbox: &Sandbox, +) -> Result, Status> { + let provider_names = sandbox + .spec + .as_ref() + .map(|spec| spec.providers.as_slice()) + .ok_or_else(|| Status::failed_precondition("sandbox spec is missing"))?; + + let mut providers = Vec::with_capacity(provider_names.len()); + for name in provider_names { + let provider = get_provider_record(state.store.as_ref(), name) + .await + .map_err(|err| { + if err.code() == tonic::Code::NotFound { + Status::failed_precondition(format!("provider '{name}' not found")) + } else { + err + } + })?; + providers.push(provider); + } + Ok(providers) +} + +fn dedupe_provider_names(provider_names: &mut Vec) { + let mut index = 0; + while index < provider_names.len() { + if provider_names[..index].contains(&provider_names[index]) { + provider_names.remove(index); + } else { + index += 1; + } + } +} + // --------------------------------------------------------------------------- // Watch handler // --------------------------------------------------------------------------- @@ -938,6 +1112,15 @@ async fn run_exec_with_russh( #[cfg(test)] mod tests { use super::*; + use crate::compute::new_test_runtime; + use crate::persistence::Store; + use crate::sandbox_index::SandboxIndex; + use crate::sandbox_watch::SandboxWatchBus; + use crate::supervisor_session::SupervisorSessionRegistry; + use crate::tracing_bus::TracingLogBus; + use openshell_core::Config; + use openshell_core::proto::datamodel::v1::ObjectMeta; + use std::collections::HashMap; // ---- shell_escape ---- @@ -1066,4 +1249,241 @@ mod tests { ); } } + + async fn test_server_state() -> Arc { + let store = Arc::new(Store::connect("sqlite::memory:").await.unwrap()); + let compute = new_test_runtime(store.clone()).await; + Arc::new(ServerState::new( + Config::new(None) + .with_database_url("sqlite::memory:") + .with_ssh_handshake_secret("test-secret"), + store, + compute, + SandboxIndex::new(), + SandboxWatchBus::new(), + TracingLogBus::new(), + Arc::new(SupervisorSessionRegistry::new()), + None, + )) + } + + fn test_provider(name: &str, provider_type: &str) -> Provider { + Provider { + metadata: Some(ObjectMeta { + id: format!("provider-{name}"), + name: name.to_string(), + created_at_ms: 1_000_000, + labels: HashMap::new(), + }), + r#type: provider_type.to_string(), + credentials: std::iter::once(("TOKEN".to_string(), "secret".to_string())).collect(), + config: HashMap::new(), + } + } + + fn test_sandbox(name: &str, providers: Vec) -> Sandbox { + Sandbox { + metadata: Some(ObjectMeta { + id: format!("sandbox-{name}"), + name: name.to_string(), + created_at_ms: 1_000_000, + labels: std::iter::once(("team".to_string(), "agents".to_string())).collect(), + }), + spec: Some(openshell_core::proto::SandboxSpec { + log_level: "debug".to_string(), + policy: Some(openshell_core::proto::SandboxPolicy::default()), + providers, + ..Default::default() + }), + phase: SandboxPhase::Ready as i32, + current_policy_version: 7, + ..Default::default() + } + } + + #[tokio::test] + async fn attach_sandbox_provider_persists_current_provider_list() { + let state = test_server_state().await; + state + .store + .put_message(&test_provider("work-github", "github")) + .await + .unwrap(); + state + .store + .put_message(&test_sandbox("work", Vec::new())) + .await + .unwrap(); + + let response = handle_attach_sandbox_provider( + &state, + Request::new(AttachSandboxProviderRequest { + sandbox_name: "work".to_string(), + provider_name: "work-github".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + + assert!(response.attached); + let sandbox = state + .store + .get_message_by_name::("work") + .await + .unwrap() + .unwrap(); + let spec = sandbox.spec.unwrap(); + assert_eq!(spec.providers, vec!["work-github"]); + assert_eq!(spec.log_level, "debug"); + assert_eq!(sandbox.phase, SandboxPhase::Ready as i32); + assert_eq!(sandbox.current_policy_version, 7); + } + + #[tokio::test] + async fn attach_sandbox_provider_is_idempotent_and_avoids_duplicates() { + let state = test_server_state().await; + state + .store + .put_message(&test_provider("work-github", "github")) + .await + .unwrap(); + state + .store + .put_message(&test_sandbox( + "work", + vec!["work-github".to_string(), "work-github".to_string()], + )) + .await + .unwrap(); + + let response = handle_attach_sandbox_provider( + &state, + Request::new(AttachSandboxProviderRequest { + sandbox_name: "work".to_string(), + provider_name: "work-github".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + + assert!(!response.attached); + let providers = state + .store + .get_message_by_name::("work") + .await + .unwrap() + .unwrap() + .spec + .unwrap() + .providers; + assert_eq!(providers, vec!["work-github"]); + } + + #[tokio::test] + async fn detach_sandbox_provider_is_idempotent_and_removes_all_matches() { + let state = test_server_state().await; + state + .store + .put_message(&test_sandbox( + "work", + vec![ + "work-github".to_string(), + "other".to_string(), + "work-github".to_string(), + ], + )) + .await + .unwrap(); + + let response = handle_detach_sandbox_provider( + &state, + Request::new(DetachSandboxProviderRequest { + sandbox_name: "work".to_string(), + provider_name: "work-github".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + + assert!(response.detached); + let providers = state + .store + .get_message_by_name::("work") + .await + .unwrap() + .unwrap() + .spec + .unwrap() + .providers; + assert_eq!(providers, vec!["other"]); + + let response = handle_detach_sandbox_provider( + &state, + Request::new(DetachSandboxProviderRequest { + sandbox_name: "work".to_string(), + provider_name: "work-github".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert!(!response.detached); + } + + #[tokio::test] + async fn list_sandbox_providers_returns_attached_provider_records() { + let state = test_server_state().await; + state + .store + .put_message(&test_provider("work-github", "github")) + .await + .unwrap(); + state + .store + .put_message(&test_sandbox("work", vec!["work-github".to_string()])) + .await + .unwrap(); + + let response = handle_list_sandbox_providers( + &state, + Request::new(ListSandboxProvidersRequest { + sandbox_name: "work".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + + assert_eq!(response.providers.len(), 1); + assert_eq!(response.providers[0].r#type, "github"); + assert_eq!( + response.providers[0].credentials.get("TOKEN"), + Some(&"REDACTED".to_string()) + ); + } + + #[tokio::test] + async fn attach_sandbox_provider_validates_provider_exists() { + let state = test_server_state().await; + state + .store + .put_message(&test_sandbox("work", Vec::new())) + .await + .unwrap(); + + let err = handle_attach_sandbox_provider( + &state, + Request::new(AttachSandboxProviderRequest { + sandbox_name: "work".to_string(), + provider_name: "missing".to_string(), + }), + ) + .await + .unwrap_err(); + + assert_eq!(err.code(), tonic::Code::FailedPrecondition); + } } diff --git a/crates/openshell-server/tests/auth_endpoint_integration.rs b/crates/openshell-server/tests/auth_endpoint_integration.rs index 7b16ee991..c66f2ad6b 100644 --- a/crates/openshell-server/tests/auth_endpoint_integration.rs +++ b/crates/openshell-server/tests/auth_endpoint_integration.rs @@ -426,6 +426,36 @@ impl openshell_core::proto::open_shell_server::OpenShell for TestOpenShell { )) } + async fn list_sandbox_providers( + &self, + _: tonic::Request, + ) -> Result, tonic::Status> + { + Ok(tonic::Response::new( + openshell_core::proto::ListSandboxProvidersResponse::default(), + )) + } + + async fn attach_sandbox_provider( + &self, + _: tonic::Request, + ) -> Result, tonic::Status> + { + Ok(tonic::Response::new( + openshell_core::proto::AttachSandboxProviderResponse::default(), + )) + } + + async fn detach_sandbox_provider( + &self, + _: tonic::Request, + ) -> Result, tonic::Status> + { + Ok(tonic::Response::new( + openshell_core::proto::DetachSandboxProviderResponse::default(), + )) + } + async fn delete_sandbox( &self, _: tonic::Request, diff --git a/crates/openshell-server/tests/edge_tunnel_auth.rs b/crates/openshell-server/tests/edge_tunnel_auth.rs index 39df0819f..706967d1f 100644 --- a/crates/openshell-server/tests/edge_tunnel_auth.rs +++ b/crates/openshell-server/tests/edge_tunnel_auth.rs @@ -106,6 +106,33 @@ impl OpenShell for TestOpenShell { Ok(Response::new(ListSandboxesResponse::default())) } + async fn list_sandbox_providers( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::ListSandboxProvidersResponse::default(), + )) + } + + async fn attach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::AttachSandboxProviderResponse::default(), + )) + } + + async fn detach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::DetachSandboxProviderResponse::default(), + )) + } + async fn delete_sandbox( &self, _request: tonic::Request, diff --git a/crates/openshell-server/tests/multiplex_integration.rs b/crates/openshell-server/tests/multiplex_integration.rs index 49c6f9c92..d5631319d 100644 --- a/crates/openshell-server/tests/multiplex_integration.rs +++ b/crates/openshell-server/tests/multiplex_integration.rs @@ -64,6 +64,33 @@ impl OpenShell for TestOpenShell { Ok(Response::new(ListSandboxesResponse::default())) } + async fn list_sandbox_providers( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::ListSandboxProvidersResponse::default(), + )) + } + + async fn attach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::AttachSandboxProviderResponse::default(), + )) + } + + async fn detach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::DetachSandboxProviderResponse::default(), + )) + } + async fn delete_sandbox( &self, _request: tonic::Request, diff --git a/crates/openshell-server/tests/multiplex_tls_integration.rs b/crates/openshell-server/tests/multiplex_tls_integration.rs index d6a244e49..c4f68eaf4 100644 --- a/crates/openshell-server/tests/multiplex_tls_integration.rs +++ b/crates/openshell-server/tests/multiplex_tls_integration.rs @@ -77,6 +77,33 @@ impl OpenShell for TestOpenShell { Ok(Response::new(ListSandboxesResponse::default())) } + async fn list_sandbox_providers( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::ListSandboxProvidersResponse::default(), + )) + } + + async fn attach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::AttachSandboxProviderResponse::default(), + )) + } + + async fn detach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::DetachSandboxProviderResponse::default(), + )) + } + async fn delete_sandbox( &self, _request: tonic::Request, diff --git a/crates/openshell-server/tests/supervisor_relay_integration.rs b/crates/openshell-server/tests/supervisor_relay_integration.rs index f8519cdc7..8f5cac03a 100644 --- a/crates/openshell-server/tests/supervisor_relay_integration.rs +++ b/crates/openshell-server/tests/supervisor_relay_integration.rs @@ -111,6 +111,24 @@ impl OpenShell for RelayGateway { ) -> Result, Status> { Err(Status::unimplemented("unused")) } + async fn list_sandbox_providers( + &self, + _: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("unused")) + } + async fn attach_sandbox_provider( + &self, + _: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("unused")) + } + async fn detach_sandbox_provider( + &self, + _: tonic::Request, + ) -> Result, Status> { + Err(Status::unimplemented("unused")) + } async fn delete_sandbox( &self, _: tonic::Request, diff --git a/crates/openshell-server/tests/ws_tunnel_integration.rs b/crates/openshell-server/tests/ws_tunnel_integration.rs index 173a7225d..8212b1085 100644 --- a/crates/openshell-server/tests/ws_tunnel_integration.rs +++ b/crates/openshell-server/tests/ws_tunnel_integration.rs @@ -100,6 +100,33 @@ impl OpenShell for TestOpenShell { Ok(Response::new(ListSandboxesResponse::default())) } + async fn list_sandbox_providers( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::ListSandboxProvidersResponse::default(), + )) + } + + async fn attach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::AttachSandboxProviderResponse::default(), + )) + } + + async fn detach_sandbox_provider( + &self, + _request: tonic::Request, + ) -> Result, Status> { + Ok(Response::new( + openshell_core::proto::DetachSandboxProviderResponse::default(), + )) + } + async fn delete_sandbox( &self, _request: tonic::Request, diff --git a/proto/openshell.proto b/proto/openshell.proto index a4a18ce82..85c8d79fa 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -30,6 +30,18 @@ service OpenShell { // List sandboxes. rpc ListSandboxes(ListSandboxesRequest) returns (ListSandboxesResponse); + // List provider records attached to a sandbox. + rpc ListSandboxProviders(ListSandboxProvidersRequest) + returns (ListSandboxProvidersResponse); + + // Attach a provider record to an existing sandbox. + rpc AttachSandboxProvider(AttachSandboxProviderRequest) + returns (AttachSandboxProviderResponse); + + // Detach a provider record from an existing sandbox. + rpc DetachSandboxProvider(DetachSandboxProviderRequest) + returns (DetachSandboxProviderResponse); + // Delete a sandbox by name. rpc DeleteSandbox(DeleteSandboxRequest) returns (DeleteSandboxResponse); @@ -333,6 +345,28 @@ message ListSandboxesRequest { string label_selector = 3; } +// List providers attached to a sandbox request. +message ListSandboxProvidersRequest { + // Sandbox name (canonical lookup key). + string sandbox_name = 1; +} + +// Attach provider to sandbox request. +message AttachSandboxProviderRequest { + // Sandbox name (canonical lookup key). + string sandbox_name = 1; + // Provider name to attach. + string provider_name = 2; +} + +// Detach provider from sandbox request. +message DetachSandboxProviderRequest { + // Sandbox name (canonical lookup key). + string sandbox_name = 1; + // Provider name to detach. + string provider_name = 2; +} + // Delete sandbox request. message DeleteSandboxRequest { // Sandbox name (canonical lookup key). @@ -349,6 +383,25 @@ message ListSandboxesResponse { repeated Sandbox sandboxes = 1; } +// List providers attached to a sandbox response. +message ListSandboxProvidersResponse { + repeated openshell.datamodel.v1.Provider providers = 1; +} + +// Attach provider to sandbox response. +message AttachSandboxProviderResponse { + Sandbox sandbox = 1; + // True when the provider was newly attached. False means it was already attached. + bool attached = 2; +} + +// Detach provider from sandbox response. +message DetachSandboxProviderResponse { + Sandbox sandbox = 1; + // True when the provider was removed. False means it was not attached. + bool detached = 2; +} + // Delete sandbox response. message DeleteSandboxResponse { bool deleted = 1; From 6651c78011bb5716994139f98a4703b749206f6f Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Thu, 7 May 2026 14:38:08 -0700 Subject: [PATCH 2/5] fix(providers): refresh sandbox provider credentials Adds provider environment revisions and generation-scoped sandbox credential snapshots so future SSH and exec launches pick up provider attach, detach, and credential updates without mutating already-running processes. Also blocks provider deletion while attached to prevent stale sandbox provider references. --- crates/openshell-sandbox/src/grpc_client.rs | 15 +- crates/openshell-sandbox/src/lib.rs | 123 ++++++++++----- .../src/provider_credentials.rs | 143 ++++++++++++++++++ crates/openshell-sandbox/src/proxy.rs | 9 +- crates/openshell-sandbox/src/secrets.rs | 30 +++- crates/openshell-sandbox/src/ssh.rs | 24 +-- crates/openshell-server/src/auth/authz.rs | 17 ++- crates/openshell-server/src/grpc/policy.rs | 138 +++++++++++++++-- crates/openshell-server/src/grpc/provider.rs | 88 ++++++++++- proto/openshell.proto | 2 + proto/sandbox.proto | 3 + 11 files changed, 522 insertions(+), 70 deletions(-) create mode 100644 crates/openshell-sandbox/src/provider_credentials.rs diff --git a/crates/openshell-sandbox/src/grpc_client.rs b/crates/openshell-sandbox/src/grpc_client.rs index 44f372355..cc35f67b5 100644 --- a/crates/openshell-sandbox/src/grpc_client.rs +++ b/crates/openshell-sandbox/src/grpc_client.rs @@ -244,7 +244,7 @@ pub async fn sync_policy(endpoint: &str, sandbox: &str, policy: &ProtoSandboxPol pub async fn fetch_provider_environment( endpoint: &str, sandbox_id: &str, -) -> Result> { +) -> Result { debug!(endpoint = %endpoint, sandbox_id = %sandbox_id, "Fetching provider environment"); let mut client = connect(endpoint).await?; @@ -256,7 +256,11 @@ pub async fn fetch_provider_environment( .await .into_diagnostic()?; - Ok(response.into_inner().environment) + let inner = response.into_inner(); + Ok(ProviderEnvironmentResult { + environment: inner.environment, + provider_env_revision: inner.provider_env_revision, + }) } /// A reusable gRPC client for the `OpenShell` service. @@ -279,6 +283,12 @@ pub struct SettingsPollResult { pub settings: HashMap, /// When `policy_source` is `Global`, the version of the global policy revision. pub global_policy_version: u32, + pub provider_env_revision: u64, +} + +pub struct ProviderEnvironmentResult { + pub environment: HashMap, + pub provider_env_revision: u64, } impl CachedOpenShellClient { @@ -315,6 +325,7 @@ impl CachedOpenShellClient { .unwrap_or(PolicySource::Unspecified), settings: inner.settings, global_policy_version: inner.global_policy_version, + provider_env_revision: inner.provider_env_revision, }) } diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index 19424bd2b..abbf7eb65 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -17,6 +17,7 @@ pub mod opa; mod policy; mod process; pub mod procfs; +mod provider_credentials; pub mod proxy; mod sandbox; mod secrets; @@ -97,7 +98,6 @@ use crate::policy::{NetworkMode, NetworkPolicy, ProxyPolicy, SandboxPolicy}; use crate::proxy::ProxyHandle; #[cfg(target_os = "linux")] use crate::sandbox::linux::netns::NetworkNamespace; -use crate::secrets::SecretResolver; pub use process::{ProcessHandle, ProcessStatus}; pub use sandbox::apply_supervisor_startup_hardening; @@ -269,42 +269,46 @@ pub async fn run_sandbox( // Fetch provider environment variables from the server. // This is done after loading the policy so the sandbox can still start // even if provider env fetch fails (graceful degradation). - let provider_env = if let (Some(id), Some(endpoint)) = (&sandbox_id, &openshell_endpoint) { - match grpc_client::fetch_provider_environment(endpoint, id).await { - Ok(env) => { - ocsf_emit!( - ConfigStateChangeBuilder::new(ocsf_ctx()) - .severity(SeverityId::Informational) - .status(StatusId::Success) - .state(StateId::Enabled, "loaded") - .message(format!( - "Fetched provider environment [env_count:{}]", - env.len() - )) - .build() - ); - env - } - Err(e) => { - ocsf_emit!( - ConfigStateChangeBuilder::new(ocsf_ctx()) - .severity(SeverityId::Medium) - .status(StatusId::Failure) - .state(StateId::Other, "degraded") - .message(format!( - "Failed to fetch provider environment, continuing without: {e}" - )) - .build() - ); - std::collections::HashMap::new() + let (provider_env_revision, provider_env) = + if let (Some(id), Some(endpoint)) = (&sandbox_id, &openshell_endpoint) { + match grpc_client::fetch_provider_environment(endpoint, id).await { + Ok(result) => { + ocsf_emit!( + ConfigStateChangeBuilder::new(ocsf_ctx()) + .severity(SeverityId::Informational) + .status(StatusId::Success) + .state(StateId::Enabled, "loaded") + .message(format!( + "Fetched provider environment [env_count:{}]", + result.environment.len() + )) + .build() + ); + (result.provider_env_revision, result.environment) + } + Err(e) => { + ocsf_emit!( + ConfigStateChangeBuilder::new(ocsf_ctx()) + .severity(SeverityId::Medium) + .status(StatusId::Failure) + .state(StateId::Other, "degraded") + .message(format!( + "Failed to fetch provider environment, continuing without: {e}" + )) + .build() + ); + (0, std::collections::HashMap::new()) + } } - } - } else { - std::collections::HashMap::new() - }; + } else { + (0, std::collections::HashMap::new()) + }; - let (provider_env, secret_resolver) = SecretResolver::from_provider_env(provider_env); - let secret_resolver = secret_resolver.map(Arc::new); + let provider_credentials = provider_credentials::ProviderCredentialState::from_environment( + provider_env_revision, + provider_env, + ); + let provider_env = provider_credentials.snapshot().child_env.clone(); // Create identity cache for SHA256 TOFU when OPA is active let identity_cache = opa_engine @@ -480,7 +484,7 @@ pub async fn run_sandbox( entrypoint_pid.clone(), tls_state, inference_ctx, - secret_resolver.clone(), + Some(provider_credentials.clone()), denial_tx, ) .await?; @@ -619,7 +623,7 @@ pub async fn run_sandbox( let proxy_url = ssh_proxy_url; let netns_fd = ssh_netns_fd; let ca_paths = ca_file_paths.clone(); - let provider_env_clone = provider_env.clone(); + let provider_credentials_clone = provider_credentials.clone(); let (ssh_ready_tx, ssh_ready_rx) = tokio::sync::oneshot::channel(); @@ -632,7 +636,7 @@ pub async fn run_sandbox( netns_fd, proxy_url, ca_paths, - provider_env_clone, + provider_credentials_clone, ) .await { @@ -796,6 +800,7 @@ pub async fn run_sandbox( let poll_engine = engine.clone(); let poll_ocsf_enabled = ocsf_enabled.clone(); let poll_pid = entrypoint_pid.clone(); + let poll_provider_credentials = provider_credentials.clone(); let poll_interval_secs: u64 = std::env::var("OPENSHELL_POLICY_POLL_INTERVAL_SECS") .ok() .and_then(|v| v.parse().ok()) @@ -809,6 +814,7 @@ pub async fn run_sandbox( &poll_pid, poll_interval_secs, &poll_ocsf_enabled, + poll_provider_credentials, ) .await { @@ -2152,6 +2158,7 @@ async fn run_policy_poll_loop( entrypoint_pid: &Arc, interval_secs: u64, ocsf_enabled: &std::sync::atomic::AtomicBool, + provider_credentials: provider_credentials::ProviderCredentialState, ) -> Result<()> { use crate::grpc_client::CachedOpenShellClient; use openshell_core::proto::PolicySource; @@ -2159,6 +2166,7 @@ async fn run_policy_poll_loop( let client = CachedOpenShellClient::connect(endpoint).await?; let mut current_config_revision: u64 = 0; + let mut current_provider_env_revision: u64 = provider_credentials.snapshot().revision; let mut current_policy_hash = String::new(); let mut current_settings: std::collections::HashMap< String, @@ -2193,7 +2201,8 @@ async fn run_policy_poll_loop( } }; - if result.config_revision == current_config_revision { + let provider_env_changed = result.provider_env_revision != current_provider_env_revision; + if result.config_revision == current_config_revision && !provider_env_changed { continue; } @@ -2209,12 +2218,46 @@ async fn run_policy_poll_loop( .unmapped("old_config_revision", serde_json::json!(current_config_revision)) .unmapped("new_config_revision", serde_json::json!(result.config_revision)) .unmapped("policy_changed", serde_json::json!(policy_changed)) + .unmapped("provider_env_changed", serde_json::json!(provider_env_changed)) .message(format!( - "Settings poll: config change detected [old_revision:{current_config_revision} new_revision:{} policy_changed:{policy_changed}]", + "Settings poll: config change detected [old_revision:{current_config_revision} new_revision:{} policy_changed:{policy_changed} provider_env_changed:{provider_env_changed}]", result.config_revision )) .build()); + if provider_env_changed { + match grpc_client::fetch_provider_environment(endpoint, sandbox_id).await { + Ok(env_result) => { + let env_count = provider_credentials.install_environment( + env_result.provider_env_revision, + env_result.environment, + ); + current_provider_env_revision = env_result.provider_env_revision; + ocsf_emit!( + ConfigStateChangeBuilder::new(ocsf_ctx()) + .severity(SeverityId::Informational) + .status(StatusId::Success) + .state(StateId::Enabled, "loaded") + .unmapped( + "provider_env_revision", + serde_json::json!(current_provider_env_revision) + ) + .message(format!( + "Provider environment refreshed [revision:{current_provider_env_revision} env_count:{env_count}]" + )) + .build() + ); + } + Err(e) => { + warn!( + error = %e, + provider_env_revision = result.provider_env_revision, + "Settings poll: failed to refresh provider environment" + ); + } + } + } + // Only reload OPA when the policy payload actually changed. if policy_changed { let Some(policy) = result.policy.as_ref() else { diff --git a/crates/openshell-sandbox/src/provider_credentials.rs b/crates/openshell-sandbox/src/provider_credentials.rs new file mode 100644 index 000000000..bd28824ae --- /dev/null +++ b/crates/openshell-sandbox/src/provider_credentials.rs @@ -0,0 +1,143 @@ +// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +//! Runtime provider credential snapshots. + +use crate::secrets::SecretResolver; +use std::collections::{HashMap, VecDeque}; +use std::sync::{Arc, RwLock}; + +const MAX_RETAINED_CREDENTIAL_GENERATIONS: usize = 8; + +#[derive(Debug, Clone, Default)] +pub struct ProviderCredentialSnapshot { + pub revision: u64, + pub child_env: HashMap, +} + +#[derive(Debug)] +struct ProviderCredentialStateInner { + current: Arc, + generations: VecDeque>, + combined_resolver: Option>, +} + +#[derive(Debug, Clone)] +pub struct ProviderCredentialState { + inner: Arc>, +} + +impl ProviderCredentialState { + pub fn from_environment(revision: u64, env: HashMap) -> Self { + let (child_env, resolver) = SecretResolver::from_provider_env_for_revision(env, revision); + let snapshot = Arc::new(ProviderCredentialSnapshot { + revision, + child_env, + }); + let generations: VecDeque<_> = resolver.map(Arc::new).into_iter().collect(); + let combined_resolver = + SecretResolver::merge(generations.iter().map(Arc::as_ref)).map(Arc::new); + + Self { + inner: Arc::new(RwLock::new(ProviderCredentialStateInner { + current: snapshot, + generations, + combined_resolver, + })), + } + } + + pub fn snapshot(&self) -> Arc { + self.inner + .read() + .expect("provider credential state poisoned") + .current + .clone() + } + + pub fn resolver(&self) -> Option> { + self.inner + .read() + .expect("provider credential state poisoned") + .combined_resolver + .clone() + } + + pub fn install_environment(&self, revision: u64, env: HashMap) -> usize { + let (child_env, resolver) = SecretResolver::from_provider_env_for_revision(env, revision); + let mut inner = self + .inner + .write() + .expect("provider credential state poisoned"); + + inner.current = Arc::new(ProviderCredentialSnapshot { + revision, + child_env, + }); + + if let Some(resolver) = resolver { + inner.generations.push_back(Arc::new(resolver)); + while inner.generations.len() > MAX_RETAINED_CREDENTIAL_GENERATIONS { + inner.generations.pop_front(); + } + } + inner.combined_resolver = + SecretResolver::merge(inner.generations.iter().map(Arc::as_ref)).map(Arc::new); + inner.current.child_env.len() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn snapshots_use_revision_scoped_placeholders() { + let state = ProviderCredentialState::from_environment( + 10, + HashMap::from([("GITHUB_TOKEN".to_string(), "old".to_string())]), + ); + let first = state.snapshot(); + assert_eq!( + first.child_env.get("GITHUB_TOKEN").map(String::as_str), + Some("openshell:resolve:env:v10_GITHUB_TOKEN") + ); + + state.install_environment( + 11, + HashMap::from([("GITHUB_TOKEN".to_string(), "new".to_string())]), + ); + let second = state.snapshot(); + assert_eq!( + second.child_env.get("GITHUB_TOKEN").map(String::as_str), + Some("openshell:resolve:env:v11_GITHUB_TOKEN") + ); + + let resolver = state.resolver().expect("resolver"); + assert_eq!( + resolver.resolve_placeholder("openshell:resolve:env:v10_GITHUB_TOKEN"), + Some("old") + ); + assert_eq!( + resolver.resolve_placeholder("openshell:resolve:env:v11_GITHUB_TOKEN"), + Some("new") + ); + } + + #[test] + fn empty_refresh_removes_env_from_new_snapshots_but_retains_old_resolver() { + let state = ProviderCredentialState::from_environment( + 10, + HashMap::from([("GITHUB_TOKEN".to_string(), "old".to_string())]), + ); + + state.install_environment(11, HashMap::new()); + + assert!(state.snapshot().child_env.is_empty()); + let resolver = state.resolver().expect("old resolver retained"); + assert_eq!( + resolver.resolve_placeholder("openshell:resolve:env:v10_GITHUB_TOKEN"), + Some("old") + ); + } +} diff --git a/crates/openshell-sandbox/src/proxy.rs b/crates/openshell-sandbox/src/proxy.rs index 5344374ac..179576d82 100644 --- a/crates/openshell-sandbox/src/proxy.rs +++ b/crates/openshell-sandbox/src/proxy.rs @@ -8,6 +8,7 @@ use crate::identity::BinaryIdentityCache; use crate::l7::tls::ProxyTlsState; use crate::opa::{NetworkAction, OpaEngine, PolicyGenerationGuard}; use crate::policy::ProxyPolicy; +use crate::provider_credentials::ProviderCredentialState; use crate::secrets::{SecretResolver, rewrite_header_line}; use miette::{IntoDiagnostic, Result}; use openshell_core::net::{is_always_blocked_ip, is_internal_ip}; @@ -147,7 +148,7 @@ impl ProxyHandle { /// The proxy uses OPA for network decisions with process-identity binding /// via `/proc/net/tcp`. All connections are evaluated through OPA policy. #[allow(clippy::too_many_arguments)] - pub async fn start_with_bind_addr( + pub(crate) async fn start_with_bind_addr( policy: &ProxyPolicy, bind_addr: Option, opa_engine: Arc, @@ -155,7 +156,7 @@ impl ProxyHandle { entrypoint_pid: Arc, tls_state: Option>, inference_ctx: Option>, - secret_resolver: Option>, + provider_credentials: Option, denial_tx: Option>, ) -> Result { // Use override bind_addr, fall back to policy http_addr, then default @@ -194,7 +195,9 @@ impl ProxyHandle { let spid = entrypoint_pid.clone(); let tls = tls_state.clone(); let inf = inference_ctx.clone(); - let resolver = secret_resolver.clone(); + let resolver = provider_credentials + .as_ref() + .and_then(ProviderCredentialState::resolver); let dtx = denial_tx.clone(); tokio::spawn(async move { if let Err(err) = handle_tcp_connection( diff --git a/crates/openshell-sandbox/src/secrets.rs b/crates/openshell-sandbox/src/secrets.rs index 63e253e50..d645e1482 100644 --- a/crates/openshell-sandbox/src/secrets.rs +++ b/crates/openshell-sandbox/src/secrets.rs @@ -70,8 +70,16 @@ pub struct SecretResolver { } impl SecretResolver { + #[cfg_attr(not(test), allow(dead_code))] pub(crate) fn from_provider_env( provider_env: HashMap, + ) -> (HashMap, Option) { + Self::from_provider_env_for_revision(provider_env, 0) + } + + pub(crate) fn from_provider_env_for_revision( + provider_env: HashMap, + revision: u64, ) -> (HashMap, Option) { if provider_env.is_empty() { return (HashMap::new(), None); @@ -81,7 +89,7 @@ impl SecretResolver { let mut by_placeholder = HashMap::with_capacity(provider_env.len()); for (key, value) in provider_env { - let placeholder = placeholder_for_env_key(&key); + let placeholder = placeholder_for_env_key_for_revision(&key, revision); child_env.insert(key, placeholder.clone()); by_placeholder.insert(placeholder, value); } @@ -89,6 +97,18 @@ impl SecretResolver { (child_env, Some(Self { by_placeholder })) } + pub(crate) fn merge<'a>(resolvers: impl IntoIterator) -> Option { + let mut by_placeholder = HashMap::new(); + for resolver in resolvers { + by_placeholder.extend(resolver.by_placeholder.clone()); + } + if by_placeholder.is_empty() { + None + } else { + Some(Self { by_placeholder }) + } + } + /// Resolve a placeholder string to the real secret value. /// /// Returns `None` if the placeholder is unknown or the resolved value @@ -178,6 +198,14 @@ pub fn placeholder_for_env_key(key: &str) -> String { format!("{PLACEHOLDER_PREFIX}{key}") } +pub fn placeholder_for_env_key_for_revision(key: &str, revision: u64) -> String { + if revision == 0 { + placeholder_for_env_key(key) + } else { + format!("{PLACEHOLDER_PREFIX}v{revision}_{key}") + } +} + // --------------------------------------------------------------------------- // Secret validation (F1 — CWE-113) // --------------------------------------------------------------------------- diff --git a/crates/openshell-sandbox/src/ssh.rs b/crates/openshell-sandbox/src/ssh.rs index 9434d0a16..355fdc037 100644 --- a/crates/openshell-sandbox/src/ssh.rs +++ b/crates/openshell-sandbox/src/ssh.rs @@ -6,6 +6,7 @@ use crate::child_env; use crate::policy::SandboxPolicy; use crate::process::drop_privileges; +use crate::provider_credentials::ProviderCredentialState; use crate::sandbox; #[cfg(target_os = "linux")] use crate::{register_managed_child, unregister_managed_child}; @@ -105,7 +106,7 @@ pub async fn run_ssh_server( netns_fd: Option, proxy_url: Option, ca_file_paths: Option<(PathBuf, PathBuf)>, - provider_env: HashMap, + provider_credentials: ProviderCredentialState, ) -> Result<()> { let (listener, config, ca_paths) = match ssh_server_init(&listen_path, &ca_file_paths) { Ok(v) => { @@ -129,7 +130,7 @@ pub async fn run_ssh_server( let workdir = workdir.clone(); let proxy_url = proxy_url.clone(); let ca_paths = ca_paths.clone(); - let provider_env = provider_env.clone(); + let provider_credentials = provider_credentials.clone(); tokio::spawn(async move { if let Err(err) = handle_connection( @@ -140,7 +141,7 @@ pub async fn run_ssh_server( netns_fd, proxy_url, ca_paths, - provider_env, + provider_credentials, ) .await { @@ -166,7 +167,7 @@ async fn handle_connection( netns_fd: Option, proxy_url: Option, ca_file_paths: Option>, - provider_env: HashMap, + provider_credentials: ProviderCredentialState, ) -> Result<()> { // Access is gated by the Unix-socket filesystem permissions (root-only), // not by an application-level preface. The supervisor bridges the @@ -188,7 +189,7 @@ async fn handle_connection( netns_fd, proxy_url, ca_file_paths, - provider_env, + provider_credentials, ); russh::server::run_stream(config, stream, handler) .await @@ -215,7 +216,7 @@ struct SshHandler { netns_fd: Option, proxy_url: Option, ca_file_paths: Option>, - provider_env: HashMap, + provider_credentials: ProviderCredentialState, channels: HashMap, } @@ -226,7 +227,7 @@ impl SshHandler { netns_fd: Option, proxy_url: Option, ca_file_paths: Option>, - provider_env: HashMap, + provider_credentials: ProviderCredentialState, ) -> Self { Self { policy, @@ -234,7 +235,7 @@ impl SshHandler { netns_fd, proxy_url, ca_file_paths, - provider_env, + provider_credentials, channels: HashMap::new(), } } @@ -456,7 +457,7 @@ impl russh::server::Handler for SshHandler { self.netns_fd, self.proxy_url.clone(), self.ca_file_paths.clone(), - &self.provider_env, + &self.provider_credentials.snapshot().child_env, )?; let state = self.channels.get_mut(&channel).ok_or_else(|| { anyhow::anyhow!("subsystem_request on unknown channel {channel:?}") @@ -533,6 +534,7 @@ impl SshHandler { handle: Handle, command: Option, ) -> anyhow::Result<()> { + let provider_snapshot = self.provider_credentials.snapshot(); let state = self .channels .get_mut(&channel) @@ -550,7 +552,7 @@ impl SshHandler { self.netns_fd, self.proxy_url.clone(), self.ca_file_paths.clone(), - &self.provider_env, + &provider_snapshot.child_env, )?; state.pty_master = Some(pty_master); state.input_sender = Some(input_sender); @@ -567,7 +569,7 @@ impl SshHandler { self.netns_fd, self.proxy_url.clone(), self.ca_file_paths.clone(), - &self.provider_env, + &provider_snapshot.child_env, )?; state.input_sender = Some(input_sender); } diff --git a/crates/openshell-server/src/auth/authz.rs b/crates/openshell-server/src/auth/authz.rs index a3f73117b..7e69b1cd8 100644 --- a/crates/openshell-server/src/auth/authz.rs +++ b/crates/openshell-server/src/auth/authz.rs @@ -410,11 +410,26 @@ mod tests { .check(&id, "/openshell.v1.OpenShell/ListSandboxes") .is_ok() ); + assert!( + policy + .check(&id, "/openshell.v1.OpenShell/ListSandboxProviders") + .is_ok() + ); assert!( policy .check(&id, "/openshell.v1.OpenShell/CreateSandbox") .is_ok() ); + assert!( + policy + .check(&id, "/openshell.v1.OpenShell/AttachSandboxProvider") + .is_ok() + ); + assert!( + policy + .check(&id, "/openshell.v1.OpenShell/DetachSandboxProvider") + .is_ok() + ); } #[test] @@ -427,7 +442,7 @@ mod tests { .is_ok() ); let err = policy - .check(&id, "/openshell.v1.OpenShell/CreateSandbox") + .check(&id, "/openshell.v1.OpenShell/AttachSandboxProvider") .unwrap_err(); assert_eq!(err.code(), tonic::Code::PermissionDenied); assert!(err.message().contains("sandbox:write")); diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 56fe8f199..8c843875f 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -10,7 +10,7 @@ #![allow(clippy::cast_precision_loss)] // f64->f32 for confidence scores #![allow(clippy::items_after_statements)] // DB_PORTS const inside function -use crate::persistence::{DraftChunkRecord, ObjectId, ObjectName, PolicyRecord, Store}; +use crate::persistence::{DraftChunkRecord, ObjectId, ObjectName, ObjectType, PolicyRecord, Store}; use crate::policy_store::PolicyStoreExt; use crate::{ServerState, auth::oidc}; use openshell_core::proto::policy_merge_operation; @@ -472,6 +472,8 @@ pub(super) async fn handle_get_sandbox_config( let settings = merge_effective_settings(&global_settings, &sandbox_settings)?; let config_revision = compute_config_revision(policy.as_ref(), &settings, policy_source); + let provider_env_revision = + compute_provider_env_revision(state.store.as_ref(), &sandbox_provider_names).await?; Ok(Response::new(GetSandboxConfigResponse { policy, @@ -481,9 +483,52 @@ pub(super) async fn handle_get_sandbox_config( config_revision, policy_source: policy_source.into(), global_policy_version, + provider_env_revision, })) } +pub(super) async fn compute_provider_env_revision( + store: &Store, + provider_names: &[String], +) -> Result { + let mut hasher = Sha256::new(); + hasher.update(b"openshell-provider-env-revision-v1"); + + for provider_name in provider_names { + hasher.update(provider_name.as_bytes()); + match store + .get_by_name(Provider::object_type(), provider_name) + .await + .map_err(|e| { + Status::internal(format!("fetch provider '{provider_name}' failed: {e}")) + })? { + Some(record) => { + hasher.update(record.id.as_bytes()); + hasher.update(record.updated_at_ms.to_le_bytes()); + + let provider = Provider::decode(record.payload.as_slice()).map_err(|e| { + Status::internal(format!("decode provider '{provider_name}' failed: {e}")) + })?; + hasher.update(provider.r#type.as_bytes()); + + let mut credential_keys: Vec<_> = provider.credentials.keys().collect(); + credential_keys.sort(); + for key in credential_keys { + hasher.update(key.as_bytes()); + } + } + None => { + hasher.update(b"missing"); + } + } + } + + let digest = hasher.finalize(); + Ok(u64::from_le_bytes(digest[..8].try_into().map_err( + |_| Status::internal("provider env revision digest too short"), + )?)) +} + async fn profile_provider_policy_layers( store: &Store, provider_names: &[String], @@ -571,19 +616,24 @@ pub(super) async fn handle_get_sandbox_provider_environment( .spec .ok_or_else(|| Status::internal("sandbox has no spec"))?; + let provider_names = spec.providers; + let provider_env_revision = + compute_provider_env_revision(state.store.as_ref(), &provider_names).await?; let environment = - super::provider::resolve_provider_environment(state.store.as_ref(), &spec.providers) + super::provider::resolve_provider_environment(state.store.as_ref(), &provider_names) .await?; info!( sandbox_id = %sandbox_id, - provider_count = spec.providers.len(), + provider_count = provider_names.len(), env_count = environment.len(), + provider_env_revision, "GetSandboxProviderEnvironment request completed successfully" ); Ok(Response::new(GetSandboxProviderEnvironmentResponse { environment, + provider_env_revision, })) } @@ -3322,6 +3372,61 @@ mod tests { assert_eq!(v2_env.get("GITHUB_TOKEN"), Some(&"ghp-test".to_string())); } + #[tokio::test] + async fn provider_env_revision_changes_when_attached_provider_record_changes() { + use openshell_core::proto::GetSandboxProviderEnvironmentRequest; + use std::time::Duration; + + let state = test_server_state().await; + let mut provider = test_provider("work-github", "github"); + state.store.put_message(&provider).await.unwrap(); + state + .store + .put_message(&test_sandbox( + "sb-provider-revision", + "provider-revision", + test_policy_with_rule("sandbox_only", "sandbox.example.com"), + vec!["work-github".to_string()], + )) + .await + .unwrap(); + + let first = handle_get_sandbox_provider_environment( + &state, + Request::new(GetSandboxProviderEnvironmentRequest { + sandbox_id: "sb-provider-revision".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + + tokio::time::sleep(Duration::from_millis(2)).await; + provider + .credentials + .insert("GITHUB_TOKEN".to_string(), "rotated".to_string()); + state.store.put_message(&provider).await.unwrap(); + + let second = handle_get_sandbox_provider_environment( + &state, + Request::new(GetSandboxProviderEnvironmentRequest { + sandbox_id: "sb-provider-revision".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + + assert_ne!( + first.provider_env_revision, second.provider_env_revision, + "provider object updates must trigger sandbox credential refresh" + ); + assert_eq!( + second.environment.get("GITHUB_TOKEN"), + Some(&"rotated".to_string()) + ); + } + #[tokio::test] async fn sandbox_config_and_provider_env_follow_attached_provider_lifecycle() { use crate::grpc::sandbox::{ @@ -3356,6 +3461,15 @@ mod tests { .network_policies .contains_key("_provider_work_github") ); + let baseline_env = handle_get_sandbox_provider_environment( + &state, + Request::new(GetSandboxProviderEnvironmentRequest { + sandbox_id: "sb-attach-lifecycle".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); handle_attach_sandbox_provider( &state, @@ -3382,10 +3496,13 @@ mod tests { ) .await .unwrap() - .into_inner() - .environment; + .into_inner(); + assert_ne!( + baseline_env.provider_env_revision, + attached_env.provider_env_revision + ); assert_eq!( - attached_env.get("GITHUB_TOKEN"), + attached_env.environment.get("GITHUB_TOKEN"), Some(&"ghp-test".to_string()) ); @@ -3414,9 +3531,12 @@ mod tests { ) .await .unwrap() - .into_inner() - .environment; - assert!(!detached_env.contains_key("GITHUB_TOKEN")); + .into_inner(); + assert_ne!( + attached_env.provider_env_revision, + detached_env.provider_env_revision + ); + assert!(!detached_env.environment.contains_key("GITHUB_TOKEN")); } #[tokio::test] diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 2f4893073..2ed4d439d 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -6,7 +6,7 @@ #![allow(clippy::result_large_err)] // gRPC handlers return Result, Status> use crate::persistence::{ObjectName, ObjectType, Store, generate_name}; -use openshell_core::proto::Provider; +use openshell_core::proto::{Provider, Sandbox}; use prost::Message; use tonic::Status; use tracing::warn; @@ -175,12 +175,57 @@ pub(super) async fn delete_provider_record(store: &Store, name: &str) -> Result< return Err(Status::invalid_argument("name is required")); } + let blocking_sandboxes = sandboxes_using_provider(store, name).await?; + if !blocking_sandboxes.is_empty() { + return Err(Status::failed_precondition(format!( + "provider '{name}' is attached to sandbox(es): {}", + blocking_sandboxes.join(", ") + ))); + } + store .delete_by_name(Provider::object_type(), name) .await .map_err(|e| Status::internal(format!("delete provider failed: {e}"))) } +async fn sandboxes_using_provider( + store: &Store, + provider_name: &str, +) -> Result, Status> { + let mut blocking = Vec::new(); + let mut offset = 0; + loop { + let records = store + .list(Sandbox::object_type(), 1000, offset) + .await + .map_err(|e| Status::internal(format!("list sandboxes failed: {e}")))?; + if records.is_empty() { + break; + } + offset = offset + .checked_add( + u32::try_from(records.len()) + .map_err(|_| Status::internal("sandbox page size exceeded u32"))?, + ) + .ok_or_else(|| Status::internal("sandbox pagination offset overflow"))?; + + for record in records { + let sandbox = Sandbox::decode(record.payload.as_slice()) + .map_err(|e| Status::internal(format!("decode sandbox failed: {e}")))?; + let Some(spec) = sandbox.spec.as_ref() else { + continue; + }; + if spec.providers.iter().any(|name| name == provider_name) { + blocking.push(sandbox.object_name().to_string()); + } + } + } + blocking.sort(); + blocking.dedup(); + Ok(blocking) +} + /// Merge an incoming map into an existing map. /// /// - If `incoming` is empty, return `existing` unchanged (no-op). @@ -278,8 +323,8 @@ use openshell_core::proto::{ ImportProviderProfilesRequest, ImportProviderProfilesResponse, LintProviderProfilesRequest, LintProviderProfilesResponse, ListProviderProfilesRequest, ListProviderProfilesResponse, ListProvidersRequest, ListProvidersResponse, ProviderProfile, ProviderProfileDiagnostic, - ProviderProfileImportItem, ProviderProfileResponse, ProviderResponse, Sandbox, - StoredProviderProfile, UpdateProviderRequest, + ProviderProfileImportItem, ProviderProfileResponse, ProviderResponse, StoredProviderProfile, + UpdateProviderRequest, }; use openshell_providers::{ ProfileValidationDiagnostic, ProviderTypeProfile, default_profiles, get_default_profile, @@ -1414,6 +1459,43 @@ mod tests { assert_eq!(missing.code(), Code::NotFound); } + #[tokio::test] + async fn delete_provider_rejects_attached_provider() { + let store = Store::connect("sqlite::memory:?cache=shared") + .await + .unwrap(); + + create_provider_record(&store, provider_with_values("gitlab-local", "gitlab")) + .await + .unwrap(); + store + .put_message(&Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: "sandbox-id".to_string(), + name: "attached-sandbox".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + }), + spec: Some(SandboxSpec { + providers: vec!["gitlab-local".to_string()], + ..Default::default() + }), + ..Default::default() + }) + .await + .unwrap(); + + let err = delete_provider_record(&store, "gitlab-local") + .await + .unwrap_err(); + assert_eq!(err.code(), Code::FailedPrecondition); + assert!( + err.message().contains("attached-sandbox"), + "error should identify blocking sandbox: {}", + err.message() + ); + } + #[tokio::test] async fn provider_validation_errors() { let store = Store::connect("sqlite::memory:?cache=shared") diff --git a/proto/openshell.proto b/proto/openshell.proto index 85c8d79fa..e5ee40c98 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -760,6 +760,8 @@ message GetSandboxProviderEnvironmentRequest { message GetSandboxProviderEnvironmentResponse { // Provider credential environment variables. map environment = 1; + // Fingerprint for the provider credential inputs that produced environment. + uint64 provider_env_revision = 2; } // --------------------------------------------------------------------------- diff --git a/proto/sandbox.proto b/proto/sandbox.proto index 2ea8659da..f7df5945e 100644 --- a/proto/sandbox.proto +++ b/proto/sandbox.proto @@ -257,4 +257,7 @@ message GetSandboxConfigResponse { // When policy_source is GLOBAL, the version of the global policy revision. // Zero when no global policy is active or when policy_source is SANDBOX. uint32 global_policy_version = 7; + // Fingerprint for provider credential inputs attached to this sandbox. + // Changes when attached provider names or attached provider records change. + uint64 provider_env_revision = 8; } From cfbe999adadd2b034c40bdc8719c213b1b7f7e6b Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Thu, 7 May 2026 15:08:41 -0700 Subject: [PATCH 3/5] fix(providers): serialize sandbox object mutations --- crates/openshell-server/src/compute/mod.rs | 10 ++++++ crates/openshell-server/src/grpc/policy.rs | 36 ++++++++++++++------- crates/openshell-server/src/grpc/sandbox.rs | 2 ++ 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/crates/openshell-server/src/compute/mod.rs b/crates/openshell-server/src/compute/mod.rs index 2d6351637..8bfa87e8c 100644 --- a/crates/openshell-server/src/compute/mod.rs +++ b/crates/openshell-server/src/compute/mod.rs @@ -275,6 +275,16 @@ impl ComputeRuntime { }) } + /// Serializes sandbox object read-modify-write operations within this + /// gateway process. + /// + /// This is a temporary single-gateway guard for full-object sandbox writes. + /// It is not HA-safe; replace it with DB-backed CAS/resource-version writes + /// tracked by #1255 before enabling multiple gateway writers. + pub(crate) async fn sandbox_sync_guard(&self) -> tokio::sync::OwnedMutexGuard<()> { + self.sync_lock.clone().lock_owned().await + } + pub async fn new_docker( config: openshell_core::Config, docker_config: DockerComputeConfig, diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 8c843875f..5fd660061 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -1000,19 +1000,32 @@ pub(super) async fn handle_update_config( validate_static_fields_unchanged(baseline_policy, &new_policy)?; validate_policy_safety(&new_policy)?; } else { - let mut sandbox = sandbox; - if let Some(ref mut spec) = sandbox.spec { - spec.policy = Some(new_policy.clone()); - } - state + let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; + let mut sandbox = state .store - .put_message(&sandbox) + .get_message::(&sandbox_id) .await - .map_err(|e| Status::internal(format!("backfill spec.policy failed: {e}")))?; - info!( - sandbox_id = %sandbox_id, - "UpdateConfig: backfilled spec.policy from sandbox-discovered policy" - ); + .map_err(|e| Status::internal(format!("fetch sandbox failed: {e}")))? + .ok_or_else(|| Status::not_found("sandbox not found"))?; + let spec = sandbox + .spec + .as_mut() + .ok_or_else(|| Status::internal("sandbox has no spec"))?; + if let Some(baseline_policy) = spec.policy.as_ref() { + validate_static_fields_unchanged(baseline_policy, &new_policy)?; + validate_policy_safety(&new_policy)?; + } else { + spec.policy = Some(new_policy.clone()); + state + .store + .put_message(&sandbox) + .await + .map_err(|e| Status::internal(format!("backfill spec.policy failed: {e}")))?; + info!( + sandbox_id = %sandbox_id, + "UpdateConfig: backfilled spec.policy from sandbox-discovered policy" + ); + } } let latest = state @@ -1209,6 +1222,7 @@ pub(super) async fn handle_report_policy_status( .store .supersede_older_policies(&req.sandbox_id, version) .await; + let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; if let Ok(Some(mut sandbox)) = state.store.get_message::(&req.sandbox_id).await { sandbox.current_policy_version = req.version; let _ = state.store.put_message(&sandbox).await; diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 58220a03b..65ac69acb 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -225,6 +225,7 @@ pub(super) async fn handle_attach_sandbox_provider( } })?; + let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; let mut sandbox = sandbox_by_name(state, &request.sandbox_name).await?; let sandbox_name = sandbox .metadata @@ -281,6 +282,7 @@ pub(super) async fn handle_detach_sandbox_provider( return Err(Status::invalid_argument("provider_name is required")); } + let _sandbox_sync_guard = state.compute.sandbox_sync_guard().await; let mut sandbox = sandbox_by_name(state, &request.sandbox_name).await?; let sandbox_name = sandbox .metadata From cbeebc860947fae51c416a3f1a5fc335a77cdd37 Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Thu, 7 May 2026 15:24:54 -0700 Subject: [PATCH 4/5] test(providers): cover sandbox provider attach lifecycle --- crates/openshell-cli/src/run.rs | 100 +++++++-- .../tests/provider_commands_integration.rs | 206 +++++++++++++++++- crates/openshell-server/src/grpc/policy.rs | 160 ++++++++++++++ e2e/python/test_sandbox_providers.py | 72 ++++++ 4 files changed, 514 insertions(+), 24 deletions(-) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index df6b3382e..dd1b6721d 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -2591,6 +2591,12 @@ pub async fn sandbox_provider_detach( } fn print_provider_attachment_table(providers: &[Provider]) { + print!("{}", format_provider_attachment_table(providers, true)); +} + +fn format_provider_attachment_table(providers: &[Provider], color: bool) -> String { + use std::fmt::Write as _; + let name_width = providers .iter() .map(|provider| provider.object_name().len()) @@ -2604,23 +2610,44 @@ fn print_provider_attachment_table(providers: &[Provider]) { .unwrap_or(4) .max(4); - println!( - "{: String { mod tests { use super::{ TlsOptions, dockerfile_sources_supported_for_gateway, format_gateway_select_header, - format_gateway_select_items, gateway_add, gateway_auth_label, gateway_env_override_warning, - gateway_select_with, gateway_type_label, git_sync_files, http_health_check, - image_requests_gpu, inferred_provider_type, parse_cli_setting_value, - parse_credential_pairs, plaintext_gateway_is_remote, provisioning_timeout_message, - ready_false_condition_message, resolve_from, sandbox_should_persist, + format_gateway_select_items, format_provider_attachment_table, gateway_add, + gateway_auth_label, gateway_env_override_warning, gateway_select_with, gateway_type_label, + git_sync_files, http_health_check, image_requests_gpu, inferred_provider_type, + parse_cli_setting_value, parse_credential_pairs, plaintext_gateway_is_remote, + provisioning_timeout_message, ready_false_condition_message, resolve_from, + sandbox_should_persist, }; use crate::TEST_ENV_LOCK; use hyper::StatusCode; @@ -5379,7 +5407,9 @@ mod tests { use std::thread; use openshell_bootstrap::GatewayMetadata; - use openshell_core::proto::{SandboxCondition, SandboxStatus}; + use openshell_core::proto::{ + Provider, SandboxCondition, SandboxStatus, datamodel::v1::ObjectMeta, + }; struct EnvVarGuard { key: &'static str, @@ -5482,6 +5512,40 @@ mod tests { )); } + #[test] + fn provider_attachment_table_formats_provider_counts() { + let output = format_provider_attachment_table( + &[Provider { + metadata: Some(ObjectMeta { + name: "work-custom".to_string(), + ..Default::default() + }), + r#type: "custom-api".to_string(), + credentials: [ + ("CUSTOM_API_KEY".to_string(), "REDACTED".to_string()), + ("CUSTOM_API_SECRET".to_string(), "REDACTED".to_string()), + ] + .into_iter() + .collect(), + config: std::iter::once(( + "BASE_URL".to_string(), + "https://api.custom.example".to_string(), + )) + .collect(), + }], + false, + ); + + assert!(output.contains("NAME")); + assert!(output.contains("TYPE")); + assert!(output.contains("CREDENTIAL_KEYS")); + assert!(output.contains("CONFIG_KEYS")); + assert!(output.contains("work-custom")); + assert!(output.contains("custom-api")); + assert!(output.contains('2')); + assert!(output.contains('1')); + } + #[cfg(feature = "dev-settings")] #[test] fn parse_cli_setting_value_parses_bool_aliases() { diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index 54de10511..3902bda34 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -66,6 +66,23 @@ impl Drop for EnvVarGuard { struct ProviderState { providers: Arc>>, profiles: Arc>>, + sandbox_providers: Arc>>>, + sandbox_provider_requests: Arc>>, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +enum SandboxProviderRequestLog { + List { + sandbox_name: String, + }, + Attach { + sandbox_name: String, + provider_name: String, + }, + Detach { + sandbox_name: String, + provider_name: String, + }, } #[derive(Clone, Default)] @@ -108,23 +125,116 @@ impl OpenShell for TestOpenShell { async fn list_sandbox_providers( &self, - _request: tonic::Request, + request: tonic::Request, ) -> Result, Status> { - Ok(Response::new(ListSandboxProvidersResponse::default())) + let sandbox_name = request.into_inner().sandbox_name; + self.state + .sandbox_provider_requests + .lock() + .await + .push(SandboxProviderRequestLog::List { + sandbox_name: sandbox_name.clone(), + }); + let provider_names = self + .state + .sandbox_providers + .lock() + .await + .get(&sandbox_name) + .cloned() + .unwrap_or_default(); + let providers_by_name = self.state.providers.lock().await; + let providers = provider_names + .iter() + .filter_map(|name| providers_by_name.get(name).cloned()) + .collect(); + Ok(Response::new(ListSandboxProvidersResponse { providers })) } async fn attach_sandbox_provider( &self, - _request: tonic::Request, + request: tonic::Request, ) -> Result, Status> { - Ok(Response::new(AttachSandboxProviderResponse::default())) + let request = request.into_inner(); + self.state + .sandbox_provider_requests + .lock() + .await + .push(SandboxProviderRequestLog::Attach { + sandbox_name: request.sandbox_name.clone(), + provider_name: request.provider_name.clone(), + }); + if !self + .state + .providers + .lock() + .await + .contains_key(&request.provider_name) + { + return Err(Status::failed_precondition("provider not found")); + } + let mut sandbox_providers = self.state.sandbox_providers.lock().await; + let providers = sandbox_providers + .entry(request.sandbox_name.clone()) + .or_default(); + let attached = if providers.contains(&request.provider_name) { + false + } else { + providers.push(request.provider_name.clone()); + true + }; + let sandbox = openshell_core::proto::Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + name: request.sandbox_name, + ..Default::default() + }), + spec: Some(openshell_core::proto::SandboxSpec { + providers: providers.clone(), + ..Default::default() + }), + ..Default::default() + }; + Ok(Response::new(AttachSandboxProviderResponse { + sandbox: Some(sandbox), + attached, + })) } async fn detach_sandbox_provider( &self, - _request: tonic::Request, + request: tonic::Request, ) -> Result, Status> { - Ok(Response::new(DetachSandboxProviderResponse::default())) + let request = request.into_inner(); + self.state + .sandbox_provider_requests + .lock() + .await + .push(SandboxProviderRequestLog::Detach { + sandbox_name: request.sandbox_name.clone(), + provider_name: request.provider_name.clone(), + }); + let mut sandbox_providers = self.state.sandbox_providers.lock().await; + let providers = sandbox_providers + .entry(request.sandbox_name.clone()) + .or_default(); + let before_len = providers.len(); + providers.retain(|name| name != &request.provider_name); + let detached = providers.len() != before_len; + let sandbox = openshell_core::proto::Sandbox { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + name: request.sandbox_name, + ..Default::default() + }), + spec: Some(openshell_core::proto::SandboxSpec { + providers: providers.clone(), + ..Default::default() + }), + ..Default::default() + }; + Ok(Response::new(DetachSandboxProviderResponse { + sandbox: Some(sandbox), + detached, + })) } async fn delete_sandbox( @@ -651,6 +761,90 @@ async fn provider_list_profiles_cli_uses_profile_browsing_rpc() { .expect("provider list-profiles"); } +#[tokio::test] +async fn sandbox_provider_cli_run_functions_wire_requests_and_idempotent_results() { + let ts = run_server().await; + + run::provider_create( + &ts.endpoint, + "work-github", + "github", + false, + &["GITHUB_TOKEN=ghp-test".to_string()], + &[], + &ts.tls, + ) + .await + .expect("provider create"); + + run::sandbox_provider_attach(&ts.endpoint, "dev-sandbox", "work-github", &ts.tls) + .await + .expect("sandbox provider attach"); + run::sandbox_provider_attach(&ts.endpoint, "dev-sandbox", "work-github", &ts.tls) + .await + .expect("sandbox provider attach is idempotent"); + run::sandbox_provider_list(&ts.endpoint, "dev-sandbox", &ts.tls) + .await + .expect("sandbox provider list"); + run::sandbox_provider_detach(&ts.endpoint, "dev-sandbox", "work-github", &ts.tls) + .await + .expect("sandbox provider detach"); + run::sandbox_provider_detach(&ts.endpoint, "dev-sandbox", "work-github", &ts.tls) + .await + .expect("sandbox provider detach is idempotent"); + + let requests = ts.state.sandbox_provider_requests.lock().await.clone(); + assert_eq!( + requests, + vec![ + SandboxProviderRequestLog::Attach { + sandbox_name: "dev-sandbox".to_string(), + provider_name: "work-github".to_string(), + }, + SandboxProviderRequestLog::Attach { + sandbox_name: "dev-sandbox".to_string(), + provider_name: "work-github".to_string(), + }, + SandboxProviderRequestLog::List { + sandbox_name: "dev-sandbox".to_string(), + }, + SandboxProviderRequestLog::Detach { + sandbox_name: "dev-sandbox".to_string(), + provider_name: "work-github".to_string(), + }, + SandboxProviderRequestLog::Detach { + sandbox_name: "dev-sandbox".to_string(), + provider_name: "work-github".to_string(), + }, + ] + ); + + let providers = ts.state.sandbox_providers.lock().await; + assert!(providers.get("dev-sandbox").is_none_or(Vec::is_empty)); +} + +#[tokio::test] +async fn sandbox_provider_attach_cli_surfaces_server_errors() { + let ts = run_server().await; + + let err = + run::sandbox_provider_attach(&ts.endpoint, "dev-sandbox", "missing-provider", &ts.tls) + .await + .expect_err("missing provider should fail"); + + assert!( + err.to_string().contains("provider not found"), + "unexpected error: {err}" + ); + assert_eq!( + ts.state.sandbox_provider_requests.lock().await.as_slice(), + [SandboxProviderRequestLog::Attach { + sandbox_name: "dev-sandbox".to_string(), + provider_name: "missing-provider".to_string(), + }] + ); +} + #[tokio::test] async fn provider_profile_cli_run_functions_support_custom_profiles() { let ts = run_server().await; diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index 5fd660061..d5a47bcba 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -3553,6 +3553,166 @@ mod tests { assert!(!detached_env.environment.contains_key("GITHUB_TOKEN")); } + #[tokio::test] + #[allow(deprecated)] + async fn custom_imported_profile_policy_and_env_follow_attach_detach_lifecycle() { + use crate::grpc::provider::handle_import_provider_profiles; + use crate::grpc::sandbox::{ + handle_attach_sandbox_provider, handle_detach_sandbox_provider, + }; + use openshell_core::proto::{ + AttachSandboxProviderRequest, DetachSandboxProviderRequest, + GetSandboxProviderEnvironmentRequest, ImportProviderProfilesRequest, NetworkBinary, + ProviderProfile, ProviderProfileCategory, ProviderProfileCredential, + ProviderProfileImportItem, + }; + + let state = test_server_state().await; + enable_providers_v2(&state).await; + handle_import_provider_profiles( + &state, + Request::new(ImportProviderProfilesRequest { + profiles: vec![ProviderProfileImportItem { + source: "custom-api.yaml".to_string(), + profile: Some(ProviderProfile { + id: "custom-api".to_string(), + display_name: "Custom API".to_string(), + description: String::new(), + category: ProviderProfileCategory::Other as i32, + credentials: vec![ProviderProfileCredential { + name: "api_key".to_string(), + env_vars: vec!["CUSTOM_API_KEY".to_string()], + auth_style: "bearer".to_string(), + header_name: "authorization".to_string(), + required: true, + ..Default::default() + }], + endpoints: vec![NetworkEndpoint { + host: "api.custom.example".to_string(), + port: 443, + protocol: "rest".to_string(), + rules: vec![L7Rule { + allow: Some(openshell_core::proto::L7Allow { + method: "GET".to_string(), + path: "/v1/**".to_string(), + ..Default::default() + }), + }], + ..Default::default() + }], + binaries: vec![NetworkBinary { + path: "/usr/bin/custom".to_string(), + harness: true, + }], + inference_capable: false, + }), + }], + }), + ) + .await + .unwrap(); + + let mut provider = test_provider("work-custom", "custom-api"); + provider.credentials = + std::iter::once(("CUSTOM_API_KEY".to_string(), "custom-secret".to_string())).collect(); + state.store.put_message(&provider).await.unwrap(); + state + .store + .put_message(&test_sandbox( + "sb-custom-attach-lifecycle", + "custom-attach-lifecycle", + test_policy_with_rule("sandbox_only", "sandbox.example.com"), + Vec::new(), + )) + .await + .unwrap(); + + let baseline_policy = get_sandbox_policy(&state, "sb-custom-attach-lifecycle").await; + assert!( + !baseline_policy + .network_policies + .contains_key("_provider_work_custom") + ); + let baseline_env = handle_get_sandbox_provider_environment( + &state, + Request::new(GetSandboxProviderEnvironmentRequest { + sandbox_id: "sb-custom-attach-lifecycle".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + + handle_attach_sandbox_provider( + &state, + Request::new(AttachSandboxProviderRequest { + sandbox_name: "custom-attach-lifecycle".to_string(), + provider_name: "work-custom".to_string(), + }), + ) + .await + .unwrap(); + + let attached_policy = get_sandbox_policy(&state, "sb-custom-attach-lifecycle").await; + let custom_rule = attached_policy + .network_policies + .get("_provider_work_custom") + .expect("custom provider rule should be composed after attach"); + assert_eq!(custom_rule.endpoints[0].host, "api.custom.example"); + assert_eq!(custom_rule.endpoints[0].protocol, "rest"); + assert_eq!(custom_rule.endpoints[0].rules.len(), 1); + assert_eq!(custom_rule.binaries[0].path, "/usr/bin/custom"); + + let attached_env = handle_get_sandbox_provider_environment( + &state, + Request::new(GetSandboxProviderEnvironmentRequest { + sandbox_id: "sb-custom-attach-lifecycle".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert_ne!( + baseline_env.provider_env_revision, + attached_env.provider_env_revision + ); + assert_eq!( + attached_env.environment.get("CUSTOM_API_KEY"), + Some(&"custom-secret".to_string()) + ); + + handle_detach_sandbox_provider( + &state, + Request::new(DetachSandboxProviderRequest { + sandbox_name: "custom-attach-lifecycle".to_string(), + provider_name: "work-custom".to_string(), + }), + ) + .await + .unwrap(); + + let detached_policy = get_sandbox_policy(&state, "sb-custom-attach-lifecycle").await; + assert!( + !detached_policy + .network_policies + .contains_key("_provider_work_custom") + ); + let detached_env = handle_get_sandbox_provider_environment( + &state, + Request::new(GetSandboxProviderEnvironmentRequest { + sandbox_id: "sb-custom-attach-lifecycle".to_string(), + }), + ) + .await + .unwrap() + .into_inner(); + assert_ne!( + attached_env.provider_env_revision, + detached_env.provider_env_revision + ); + assert!(!detached_env.environment.contains_key("CUSTOM_API_KEY")); + } + #[tokio::test] async fn global_policy_suppresses_provider_profile_layers_when_v2_enabled() { use openshell_core::proto::{ diff --git a/e2e/python/test_sandbox_providers.py b/e2e/python/test_sandbox_providers.py index 0e9349dc0..094e5d7a3 100644 --- a/e2e/python/test_sandbox_providers.py +++ b/e2e/python/test_sandbox_providers.py @@ -11,6 +11,7 @@ from __future__ import annotations +import time from contextlib import contextmanager from typing import TYPE_CHECKING @@ -183,6 +184,77 @@ def read_nvidia_key() -> str: assert result.stdout.strip() == "openshell:resolve:env:NVIDIA_API_KEY" +def test_attach_detach_updates_credentials_for_later_exec_launches( + sandbox: Callable[..., Sandbox], + sandbox_client: SandboxClient, +) -> None: + """Later exec launches see provider attach/detach credential changes.""" + stub = sandbox_client._stub + provider_name = "e2e-test-attach-detach-env" + + with provider( + stub, + name=provider_name, + provider_type="generic", + credentials={"CUSTOM_ATTACH_TOKEN": "token-attach-detach"}, + ): + spec = datamodel_pb2.SandboxSpec(policy=_default_policy(), providers=[]) + + def read_attach_token() -> str: + import os + + return os.environ.get("CUSTOM_ATTACH_TOKEN", "NOT_SET") + + def exec_token(sb: Sandbox) -> str: + result = sb.exec_python(read_attach_token) + assert result.exit_code == 0, result.stderr + return result.stdout.strip() + + def wait_for_token(sb: Sandbox, expected: str) -> None: + deadline = time.monotonic() + 35 + last = None + while time.monotonic() < deadline: + last = exec_token(sb) + if last == expected: + return + time.sleep(2) + pytest.fail(f"expected {expected!r}, last exec saw {last!r}") + + with sandbox(spec=spec, delete_on_exit=True) as sb: + assert exec_token(sb) == "NOT_SET" + + try: + stub.AttachSandboxProvider( + openshell_pb2.AttachSandboxProviderRequest( + sandbox_name=sb.sandbox.name, + provider_name=provider_name, + ) + ) + wait_for_token( + sb, + "openshell:resolve:env:CUSTOM_ATTACH_TOKEN", + ) + + stub.DetachSandboxProvider( + openshell_pb2.DetachSandboxProviderRequest( + sandbox_name=sb.sandbox.name, + provider_name=provider_name, + ) + ) + wait_for_token(sb, "NOT_SET") + finally: + try: + stub.DetachSandboxProvider( + openshell_pb2.DetachSandboxProviderRequest( + sandbox_name=sb.sandbox.name, + provider_name=provider_name, + ) + ) + except grpc.RpcError as exc: + if exc.code() != grpc.StatusCode.NOT_FOUND: + raise + + # =========================================================================== # Tests: security & edge cases # =========================================================================== From dfbc35a86fd985681de4c52b1d5f133d46e08c77 Mon Sep 17 00:00:00 2001 From: John Myers <9696606+johntmyers@users.noreply.github.com> Date: Thu, 7 May 2026 16:16:46 -0700 Subject: [PATCH 5/5] test(providers): accept versioned credential placeholders --- e2e/python/test_sandbox_providers.py | 30 ++++++++++++++++++++------ e2e/rust/tests/provider_auto_create.rs | 12 +++++++++-- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/e2e/python/test_sandbox_providers.py b/e2e/python/test_sandbox_providers.py index 094e5d7a3..337e3db8b 100644 --- a/e2e/python/test_sandbox_providers.py +++ b/e2e/python/test_sandbox_providers.py @@ -31,6 +31,17 @@ # --------------------------------------------------------------------------- +def _is_placeholder_for_env_key(value: str, key: str) -> bool: + """Return true when value is an OpenShell credential placeholder for key.""" + prefix = "openshell:resolve:env:" + if value == f"{prefix}{key}": + return True + token = value.removeprefix(prefix) + if token == value: + return False + return token.startswith("v") and token.endswith(f"_{key}") + + def _default_policy() -> sandbox_pb2.SandboxPolicy: """Build a sandbox policy with standard filesystem/process/landlock settings.""" return sandbox_pb2.SandboxPolicy( @@ -118,7 +129,7 @@ def read_env_var() -> str: result = sb.exec_python(read_env_var) assert result.exit_code == 0, result.stderr value = result.stdout.strip() - assert value == "openshell:resolve:env:ANTHROPIC_API_KEY" + assert _is_placeholder_for_env_key(value, "ANTHROPIC_API_KEY") assert value != "sk-e2e-test-key-12345" @@ -151,10 +162,9 @@ def read_generic_env_vars() -> str: with sandbox(spec=spec, delete_on_exit=True) as sb: result = sb.exec_python(read_generic_env_vars) assert result.exit_code == 0, result.stderr - assert ( - result.stdout.strip() - == "openshell:resolve:env:CUSTOM_SERVICE_TOKEN|openshell:resolve:env:CUSTOM_SERVICE_URL" - ) + token, url = result.stdout.strip().split("|") + assert _is_placeholder_for_env_key(token, "CUSTOM_SERVICE_TOKEN") + assert _is_placeholder_for_env_key(url, "CUSTOM_SERVICE_URL") def test_nvidia_provider_injects_nvidia_api_key_env_var( @@ -181,7 +191,9 @@ def read_nvidia_key() -> str: with sandbox(spec=spec, delete_on_exit=True) as sb: result = sb.exec_python(read_nvidia_key) assert result.exit_code == 0, result.stderr - assert result.stdout.strip() == "openshell:resolve:env:NVIDIA_API_KEY" + assert _is_placeholder_for_env_key( + result.stdout.strip(), "NVIDIA_API_KEY" + ) def test_attach_detach_updates_credentials_for_later_exec_launches( @@ -215,7 +227,11 @@ def wait_for_token(sb: Sandbox, expected: str) -> None: last = None while time.monotonic() < deadline: last = exec_token(sb) - if last == expected: + if expected == "NOT_SET": + matched = last == expected + else: + matched = _is_placeholder_for_env_key(last, "CUSTOM_ATTACH_TOKEN") + if matched: return time.sleep(2) pytest.fail(f"expected {expected!r}, last exec saw {last!r}") diff --git a/e2e/rust/tests/provider_auto_create.rs b/e2e/rust/tests/provider_auto_create.rs index 46ccb7999..c678c46c4 100644 --- a/e2e/rust/tests/provider_auto_create.rs +++ b/e2e/rust/tests/provider_auto_create.rs @@ -24,9 +24,17 @@ use openshell_e2e::harness::binary::openshell_cmd; use openshell_e2e::harness::output::{extract_field, strip_ansi}; const TEST_API_KEY: &str = "sk-e2e-auto-provider-test-key"; -const TEST_API_KEY_PLACEHOLDER: &str = "openshell:resolve:env:ANTHROPIC_API_KEY"; static CLAUDE_PROVIDER_LOCK: Mutex<()> = Mutex::new(()); +fn contains_placeholder_for_env_key(output: &str, key: &str) -> bool { + let legacy = format!("openshell:resolve:env:{key}"); + let revision_prefix = "openshell:resolve:env:v"; + let revision_suffix = format!("_{key}"); + output.split_whitespace().any(|token| { + token == legacy || (token.starts_with(revision_prefix) && token.ends_with(&revision_suffix)) + }) +} + /// Helper: delete a provider by name, ignoring errors. async fn delete_provider(name: &str) { let mut cmd = openshell_cmd(); @@ -123,7 +131,7 @@ async fn auto_created_provider_credential_available_in_sandbox() { ); assert!( - clean.contains(TEST_API_KEY_PLACEHOLDER), + contains_placeholder_for_env_key(&clean, "ANTHROPIC_API_KEY"), "sandbox should have placeholder ANTHROPIC_API_KEY in its environment:\n{clean}" );