diff --git a/architecture/sandbox.md b/architecture/sandbox.md index 4bc6803eb..c7cffdb03 100644 --- a/architecture/sandbox.md +++ b/architecture/sandbox.md @@ -74,6 +74,33 @@ Credential placeholders in proxied HTTP requests can be resolved by the proxy when policy allows the target endpoint. Secrets must not be logged in OCSF or plain tracing output. +### Selective Passthrough + +The canonical placeholder model breaks for credentials that an SDK validates +in-process before any network call (Slack `xoxb-`/`xapp-`, JWT structure, +AWS access-key format), are sent over transports the L7 proxy cannot rewrite +(WebSocket payloads after `101 Switching Protocols`), or are consumed by +in-process crypto (HMAC signing, signature verification). + +For these cases an operator can opt specific credential keys into passthrough +via `Provider.passthrough_credentials`. The supervisor injects the real value +into the agent's environment instead of the canonical +`openshell:resolve:env:` placeholder, and the resolver does not register +that key, so the L7 proxy performs no substitution for it. Each entry must be +a valid env-var name, present in `Provider.credentials`, and unique. On update, +an empty incoming list preserves the existing list while auto-pruning entries +whose credential was deleted in the same update; a non-empty list replaces the +stored list verbatim; setting `UpdateProviderRequest.clear_passthrough_credentials = true` +revokes passthrough for every credential by replacing the list with the empty +list (mutually exclusive with a non-empty incoming list). When two providers +declare the same credential key, the first provider's value wins and the +passthrough flag follows the winning value. + +Passthrough drops the "agent never sees the real secret" invariant for the +listed keys: the real value is at rest in `/proc//environ` and any +descendant inherits it. Prefer canonical placeholders; only opt in keys whose +consumer demonstrably fails with the placeholder. + ## Connect and Logs The supervisor runs an SSH server on a Unix socket inside the sandbox. The diff --git a/crates/openshell-cli/src/main.rs b/crates/openshell-cli/src/main.rs index 25c677986..6575fb5c8 100644 --- a/crates/openshell-cli/src/main.rs +++ b/crates/openshell-cli/src/main.rs @@ -741,6 +741,15 @@ enum ProviderCommands { /// Provider config key/value pair. #[arg(long = "config", value_name = "KEY=VALUE")] config: Vec, + + /// Credential key whose real value is injected directly into the + /// agent process, bypassing the canonical placeholder substitution + /// and L7 proxy rewriting. Each key must also appear in `--credential` + /// (or be discovered via `--from-existing`). Drops the "agent never + /// sees the real secret" invariant for that key — see provider docs + /// for the security trade-off. + #[arg(long = "passthrough", value_name = "KEY")] + passthrough: Vec, }, /// Manage provider credential refresh. @@ -806,6 +815,22 @@ enum ProviderCommands { #[arg(long = "config", value_name = "KEY=VALUE")] config: Vec, + /// Credential key whose real value is injected directly into the + /// agent process, bypassing the canonical placeholder substitution + /// and L7 proxy rewriting. Replaces the existing passthrough list + /// when non-empty. Each key must also be present in the merged + /// credentials. Drops the "agent never sees the real secret" + /// invariant for that key. + #[arg(long = "passthrough", value_name = "KEY")] + passthrough: Vec, + + /// Revoke passthrough for every credential by replacing the + /// passthrough list with the empty list. Mutually exclusive with + /// `--passthrough` because passing both would be ambiguous (the + /// server cannot apply "clear and then set" in one request). + #[arg(long = "clear-passthrough", conflicts_with = "passthrough")] + clear_passthrough: bool, + /// Credential expiry (`KEY=TIMESTAMP`). Accepts epoch milliseconds or RFC3339. A zero timestamp clears expiry. #[arg(long = "credential-expires-at", value_name = "KEY=TIMESTAMP")] credential_expires_at: Vec, @@ -2768,6 +2793,7 @@ async fn main() -> Result<()> { from_existing, credentials, config, + passthrough, } => { run::provider_create( endpoint, @@ -2776,6 +2802,7 @@ async fn main() -> Result<()> { from_existing, &credentials, &config, + &passthrough, &tls, ) .await?; @@ -2873,6 +2900,8 @@ async fn main() -> Result<()> { from_existing, credentials, config, + passthrough, + clear_passthrough, credential_expires_at, } => { run::provider_update( @@ -2881,6 +2910,8 @@ async fn main() -> Result<()> { from_existing, &credentials, &config, + &passthrough, + clear_passthrough, &credential_expires_at, &tls, ) diff --git a/crates/openshell-cli/src/run.rs b/crates/openshell-cli/src/run.rs index f1a44ad31..8445db4cb 100644 --- a/crates/openshell-cli/src/run.rs +++ b/crates/openshell-cli/src/run.rs @@ -3679,6 +3679,7 @@ async fn auto_create_provider( credentials: discovered.credentials.clone(), config: discovered.config.clone(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }), }; @@ -3721,6 +3722,7 @@ async fn auto_create_provider( credentials: discovered.credentials.clone(), config: discovered.config.clone(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }), }; @@ -3781,6 +3783,50 @@ fn parse_key_value_pairs(items: &[String], flag: &str) -> Result Result> { + let mut seen = HashSet::new(); + let mut out = Vec::with_capacity(items.len()); + for item in items { + let key = item.trim(); + if key.is_empty() { + return Err(miette::miette!("--passthrough key cannot be empty")); + } + if !seen.insert(key.to_string()) { + return Err(miette::miette!( + "--passthrough key '{key}' was supplied more than once" + )); + } + out.push(key.to_string()); + } + Ok(out) +} + +/// Reject any passthrough key that is not present in `credential_map`. The +/// caller is responsible for assembling the full local credential set before +/// invoking — typically explicit `--credential` entries merged with anything +/// `--from-existing` discovered. +fn ensure_passthrough_in_credentials( + passthrough: &[String], + credential_map: &HashMap, +) -> Result<()> { + for key in passthrough { + if !credential_map.contains_key(key) { + return Err(miette::miette!( + "--passthrough '{key}' is not present in resolved credentials" + )); + } + } + Ok(()) +} + fn parse_credential_pairs(items: &[String]) -> Result> { let mut map = HashMap::new(); @@ -4175,6 +4221,7 @@ async fn discover_existing_provider_data( } } +#[allow(clippy::too_many_arguments)] // user-facing CLI command pub async fn provider_create( server: &str, name: &str, @@ -4182,6 +4229,7 @@ pub async fn provider_create( from_existing: bool, credentials: &[String], config: &[String], + passthrough: &[String], tls: &TlsOptions, ) -> Result<()> { if from_existing && !credentials.is_empty() { @@ -4222,6 +4270,7 @@ pub async fn provider_create( let mut credential_map = parse_credential_pairs(credentials)?; let mut config_map = parse_key_value_pairs(config, "--config")?; + let passthrough_credentials = parse_passthrough_keys(passthrough)?; if from_existing { let discovered = discover_existing_provider_data(&mut client, &provider_type).await?; @@ -4252,6 +4301,11 @@ pub async fn provider_create( } } + // The full credential set is known locally on create (--credential plus + // anything --from-existing discovered), so cross-check passthrough keys + // here for fast feedback. The server re-validates regardless. + ensure_passthrough_in_credentials(&passthrough_credentials, &credential_map)?; + let response = client .create_provider(CreateProviderRequest { provider: Some(Provider { @@ -4266,6 +4320,7 @@ pub async fn provider_create( credentials: credential_map, config: config_map, credential_expires_at_ms: HashMap::new(), + passthrough_credentials, }), }) .await @@ -4322,8 +4377,25 @@ pub async fn provider_get(server: &str, name: &str, tls: &TlsOptions) -> Result< .provider .ok_or_else(|| miette::miette!("provider missing from response"))?; - let credential_keys = provider.credentials.keys().cloned().collect::>(); - let config_keys = provider.config.keys().cloned().collect::>(); + let passthrough: HashSet<&str> = provider + .passthrough_credentials + .iter() + .map(String::as_str) + .collect(); + let mut credential_keys = provider + .credentials + .keys() + .map(|k| { + if passthrough.contains(k.as_str()) { + format!("{k} (passthrough)") + } else { + k.clone() + } + }) + .collect::>(); + credential_keys.sort(); + let mut config_keys = provider.config.keys().cloned().collect::>(); + config_keys.sort(); println!("{}", "Provider:".cyan().bold()); println!(); @@ -4399,19 +4471,21 @@ pub async fn provider_list( .max(4); println!( - "{: Result<()> { @@ -4970,6 +5046,11 @@ pub async fn provider_update( let mut credential_map = parse_credential_pairs(credentials)?; let mut config_map = parse_key_value_pairs(config, "--config")?; + // On update the local --credential set may legitimately be empty (the + // operator is only changing the passthrough list against existing stored + // credentials), so the cross-check against credentials is delegated to + // server-side validation, which sees the post-merge state. + let passthrough_credentials = parse_passthrough_keys(passthrough)?; let credential_expires_at_ms = parse_credential_expiry_pairs(credential_expires_at)?; if from_existing { @@ -5014,8 +5095,10 @@ pub async fn provider_update( credentials: credential_map, config: config_map, credential_expires_at_ms: HashMap::new(), + passthrough_credentials, }), credential_expires_at_ms, + clear_passthrough_credentials: clear_passthrough, }) .await .into_diagnostic()?; @@ -7147,6 +7230,7 @@ mod tests { )) .collect(), credential_expires_at_ms: std::collections::HashMap::new(), + passthrough_credentials: Vec::new(), }], false, ); diff --git a/crates/openshell-cli/tests/ensure_providers_integration.rs b/crates/openshell-cli/tests/ensure_providers_integration.rs index ea2d5a465..70ba80e33 100644 --- a/crates/openshell-cli/tests/ensure_providers_integration.rs +++ b/crates/openshell-cli/tests/ensure_providers_integration.rs @@ -68,6 +68,7 @@ impl TestOpenShell { credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ); } @@ -350,6 +351,11 @@ impl OpenShell for TestOpenShell { existing.credential_expires_at_ms, provider.credential_expires_at_ms, ), + passthrough_credentials: if provider.passthrough_credentials.is_empty() { + existing.passthrough_credentials + } else { + provider.passthrough_credentials + }, }; let updated_name = updated.object_name().to_string(); providers.insert(updated_name, updated.clone()); diff --git a/crates/openshell-cli/tests/provider_commands_integration.rs b/crates/openshell-cli/tests/provider_commands_integration.rs index 090097a20..3f61a270d 100644 --- a/crates/openshell-cli/tests/provider_commands_integration.rs +++ b/crates/openshell-cli/tests/provider_commands_integration.rs @@ -516,6 +516,11 @@ impl OpenShell for TestOpenShell { existing.credential_expires_at_ms, provider.credential_expires_at_ms, ), + passthrough_credentials: if provider.passthrough_credentials.is_empty() { + existing.passthrough_credentials + } else { + provider.passthrough_credentials + }, }; let updated_name = updated.object_name().to_string(); providers.insert(updated_name, updated.clone()); @@ -925,6 +930,7 @@ async fn provider_cli_run_functions_support_full_crud_flow() { false, &["API_KEY=abc".to_string()], &["profile=dev".to_string()], + &[], &ts.tls, ) .await @@ -944,6 +950,8 @@ async fn provider_cli_run_functions_support_full_crud_flow() { &["API_KEY=rotated".to_string()], &["profile=prod".to_string()], &[], + false, + &[], &ts.tls, ) .await @@ -974,6 +982,7 @@ async fn provider_refresh_cli_run_functions_wire_requests() { false, &["MS_GRAPH_ACCESS_TOKEN=token".to_string()], &[], + &[], &ts.tls, ) .await @@ -1061,6 +1070,7 @@ async fn provider_create_allows_empty_credentials_for_gateway_refresh_profiles() false, &[], &[], + &[], &ts.tls, ) .await @@ -1083,6 +1093,7 @@ async fn sandbox_provider_cli_run_functions_wire_requests_and_idempotent_results false, &["GITHUB_TOKEN=ghp-test".to_string()], &[], + &[], &ts.tls, ) .await @@ -1201,6 +1212,7 @@ binaries: [/usr/bin/custom] false, &["CUSTOM_API_KEY=abc".to_string()], &[], + &[], &ts.tls, ) .await @@ -1254,6 +1266,7 @@ async fn provider_create_from_existing_uses_profile_discovery_when_v2_enabled() true, &[], &[], + &[], &ts.tls, ) .await @@ -1286,6 +1299,7 @@ async fn provider_create_from_existing_uses_registry_discovery_when_v2_disabled( true, &[], &[], + &[], &ts.tls, ) .await @@ -1312,9 +1326,18 @@ async fn provider_create_from_existing_requires_profile_when_v2_enabled() { enable_providers_v2(&ts).await; let _env = EnvVarGuard::set(&[("OPENAI_API_KEY", "legacy-openai-secret")]); - let err = run::provider_create(&ts.endpoint, "v2-openai", "openai", true, &[], &[], &ts.tls) - .await - .expect_err("v2 discovery without a profile should fail"); + let err = run::provider_create( + &ts.endpoint, + "v2-openai", + "openai", + true, + &[], + &[], + &[], + &ts.tls, + ) + .await + .expect_err("v2 discovery without a profile should fail"); assert!( err.to_string() @@ -1353,6 +1376,7 @@ async fn provider_create_from_existing_fails_when_profile_discovery_finds_nothin true, &[], &[], + &[], &ts.tls, ) .await @@ -1405,13 +1429,24 @@ async fn provider_update_from_existing_uses_profile_discovery_when_v2_enabled() credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ); let _env = EnvVarGuard::set(&[("CUSTOM_UPDATE_DISCOVERY_API_KEY", "updated-profile-secret")]); - run::provider_update(&ts.endpoint, "custom-update", true, &[], &[], &[], &ts.tls) - .await - .expect("profile-backed provider update --from-existing"); + run::provider_update( + &ts.endpoint, + "custom-update", + true, + &[], + &[], + &[], + false, + &[], + &ts.tls, + ) + .await + .expect("profile-backed provider update --from-existing"); let provider = ts .state @@ -1606,6 +1641,7 @@ async fn provider_create_rejects_key_only_credentials_without_local_env_value() false, &["INVALID_PAIR".to_string()], &[], + &[], &ts.tls, ) .await @@ -1630,6 +1666,7 @@ async fn provider_create_supports_generic_type_and_env_lookup_credentials() { false, &["NAV_GENERIC_TEST_KEY".to_string()], &[], + &[], &ts.tls, ) .await @@ -1664,6 +1701,7 @@ async fn provider_create_rejects_combined_from_existing_and_credentials() { true, &["API_KEY=abc".to_string()], &[], + &[], &ts.tls, ) .await @@ -1688,6 +1726,7 @@ async fn provider_create_rejects_empty_env_var_for_key_only_credential() { false, &["NAV_EMPTY_ENV_KEY".to_string()], &[], + &[], &ts.tls, ) .await @@ -1712,6 +1751,7 @@ async fn provider_create_supports_nvidia_type_with_nvidia_api_key() { false, &["NVIDIA_API_KEY".to_string()], &[], + &[], &ts.tls, ) .await diff --git a/crates/openshell-sandbox/src/grpc_client.rs b/crates/openshell-sandbox/src/grpc_client.rs index 14a6808c1..66e580af4 100644 --- a/crates/openshell-sandbox/src/grpc_client.rs +++ b/crates/openshell-sandbox/src/grpc_client.rs @@ -631,13 +631,29 @@ pub async fn sync_policy(endpoint: &str, sandbox: &str, policy: &ProtoSandboxPol /// Fetch provider environment variables for a sandbox from `OpenShell` server via gRPC. /// -/// Returns a map of environment variable names to values derived from provider -/// credentials configured on the sandbox. Returns an empty map if the sandbox -/// has no providers or the call fails. +/// Resolved provider environment for the sandbox. +#[derive(Debug, Default, Clone)] +pub struct ProviderEnvironment { + /// Credential environment variables (canonical and passthrough mixed). + pub environment: HashMap, + /// Subset of `environment` keys whose value is the real credential and + /// must be injected directly into the agent process — bypassing the + /// canonical placeholder substitution and L7 proxy rewriting. + pub passthrough_keys: Vec, + /// Fingerprint for the provider credential inputs that produced + /// `environment`; used by the supervisor to detect rotation. + pub provider_env_revision: u64, + /// Expiration timestamps for entries in `environment`. + pub credential_expires_at_ms: HashMap, +} + +/// Returns the env map and passthrough keys derived from provider credentials +/// configured on the sandbox. Returns an empty result if the sandbox has no +/// providers or the call fails. 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?; @@ -650,8 +666,9 @@ pub async fn fetch_provider_environment( .into_diagnostic()?; let inner = response.into_inner(); - Ok(ProviderEnvironmentResult { + Ok(ProviderEnvironment { environment: inner.environment, + passthrough_keys: inner.passthrough_keys, provider_env_revision: inner.provider_env_revision, credential_expires_at_ms: inner.credential_expires_at_ms, }) @@ -680,12 +697,6 @@ pub struct SettingsPollResult { pub provider_env_revision: u64, } -pub struct ProviderEnvironmentResult { - pub environment: HashMap, - pub provider_env_revision: u64, - pub credential_expires_at_ms: HashMap, -} - impl CachedOpenShellClient { pub async fn connect(endpoint: &str) -> Result { debug!(endpoint = %endpoint, "Connecting openshell gRPC client for policy polling"); diff --git a/crates/openshell-sandbox/src/lib.rs b/crates/openshell-sandbox/src/lib.rs index b83125f12..a4fd57817 100644 --- a/crates/openshell-sandbox/src/lib.rs +++ b/crates/openshell-sandbox/src/lib.rs @@ -352,57 +352,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_revision, provider_env, provider_credential_expires_at_ms) = - 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, - result.credential_expires_at_ms, - ) - } - 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(), - std::collections::HashMap::new(), - ) - } + let resolved = if let (Some(id), Some(endpoint)) = (&sandbox_id, &openshell_endpoint) { + match grpc_client::fetch_provider_environment(endpoint, id).await { + Ok(resolved) => { + ocsf_emit!( + ConfigStateChangeBuilder::new(ocsf_ctx()) + .severity(SeverityId::Informational) + .status(StatusId::Success) + .state(StateId::Enabled, "loaded") + .message(format!( + "Fetched provider environment [env_count:{} passthrough_count:{}]", + resolved.environment.len(), + resolved.passthrough_keys.len() + )) + .build() + ); + resolved } - } else { - ( - 0, - std::collections::HashMap::new(), - std::collections::HashMap::new(), - ) - }; + 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() + ); + grpc_client::ProviderEnvironment::default() + } + } + } else { + grpc_client::ProviderEnvironment::default() + }; let provider_credentials = provider_credentials::ProviderCredentialState::from_environment( - provider_env_revision, - provider_env, - provider_credential_expires_at_ms, + resolved.provider_env_revision, + resolved.environment, + &resolved.passthrough_keys, + resolved.credential_expires_at_ms, ); let provider_env = provider_credentials.snapshot().child_env.clone(); @@ -2373,6 +2362,7 @@ async fn run_policy_poll_loop(ctx: PolicyPollLoopContext) -> Result<()> { let env_count = ctx.provider_credentials.install_environment( env_result.provider_env_revision, env_result.environment, + &env_result.passthrough_keys, env_result.credential_expires_at_ms, ); current_provider_env_revision = env_result.provider_env_revision; diff --git a/crates/openshell-sandbox/src/provider_credentials.rs b/crates/openshell-sandbox/src/provider_credentials.rs index ae91e8d6e..9b828d6b9 100644 --- a/crates/openshell-sandbox/src/provider_credentials.rs +++ b/crates/openshell-sandbox/src/provider_credentials.rs @@ -32,11 +32,13 @@ impl ProviderCredentialState { pub fn from_environment( revision: u64, env: HashMap, + passthrough_keys: &[String], credential_expires_at_ms: HashMap, ) -> Self { let (child_env, generation_resolver, current_resolver) = SecretResolver::from_provider_env_for_current_revision( env, + passthrough_keys, credential_expires_at_ms, revision, ); @@ -78,11 +80,13 @@ impl ProviderCredentialState { &self, revision: u64, env: HashMap, + passthrough_keys: &[String], credential_expires_at_ms: HashMap, ) -> usize { let (child_env, generation_resolver, current_resolver) = SecretResolver::from_provider_env_for_current_revision( env, + passthrough_keys, credential_expires_at_ms, revision, ); @@ -131,6 +135,7 @@ mod tests { let state = ProviderCredentialState::from_environment( 10, HashMap::from([("GITHUB_TOKEN".to_string(), "old".to_string())]), + &[], HashMap::new(), ); let first = state.snapshot(); @@ -142,6 +147,7 @@ mod tests { state.install_environment( 11, HashMap::from([("GITHUB_TOKEN".to_string(), "new".to_string())]), + &[], HashMap::new(), ); let second = state.snapshot(); @@ -174,10 +180,11 @@ mod tests { let state = ProviderCredentialState::from_environment( 10, HashMap::from([("GITHUB_TOKEN".to_string(), "old".to_string())]), + &[], HashMap::new(), ); - state.install_environment(11, HashMap::new(), HashMap::new()); + state.install_environment(11, HashMap::new(), &[], HashMap::new()); assert!(state.snapshot().child_env.is_empty()); let resolver = state.resolver().expect("old resolver retained"); @@ -195,6 +202,35 @@ mod tests { ); } + #[test] + fn passthrough_keys_skip_placeholder_substitution() { + let state = ProviderCredentialState::from_environment( + 7, + HashMap::from([ + ("GITHUB_TOKEN".to_string(), "tok".to_string()), + ("CUSTOM_RAW_KEY".to_string(), "raw-value".to_string()), + ]), + &["CUSTOM_RAW_KEY".to_string()], + HashMap::new(), + ); + let snap = state.snapshot(); + assert_eq!( + snap.child_env.get("GITHUB_TOKEN").map(String::as_str), + Some("openshell:resolve:env:v7_GITHUB_TOKEN") + ); + assert_eq!( + snap.child_env.get("CUSTOM_RAW_KEY").map(String::as_str), + Some("raw-value") + ); + + let resolver = state.resolver().expect("resolver"); + assert_eq!( + resolver.resolve_placeholder("openshell:resolve:env:v7_GITHUB_TOKEN"), + Some("tok") + ); + assert_eq!(resolver.resolve_placeholder("raw-value"), None); + } + #[test] fn expired_retained_generation_does_not_resolve() { let now_ms = i64::try_from( @@ -207,12 +243,14 @@ mod tests { let state = ProviderCredentialState::from_environment( 10, HashMap::from([("GITHUB_TOKEN".to_string(), "old".to_string())]), + &[], HashMap::from([("GITHUB_TOKEN".to_string(), now_ms - 1_000)]), ); state.install_environment( 11, HashMap::from([("GITHUB_TOKEN".to_string(), "new".to_string())]), + &[], HashMap::from([("GITHUB_TOKEN".to_string(), now_ms + 60_000)]), ); diff --git a/crates/openshell-sandbox/src/secrets.rs b/crates/openshell-sandbox/src/secrets.rs index de7804393..ea1d1d53a 100644 --- a/crates/openshell-sandbox/src/secrets.rs +++ b/crates/openshell-sandbox/src/secrets.rs @@ -116,20 +116,44 @@ impl fmt::Debug for SecretResolver { } impl SecretResolver { + /// Test helper: build a `SecretResolver` from the provider env map with no + /// passthrough keys and revision 0. Production callers must use + /// [`Self::from_provider_env_for_revision`] so passthrough and rotation + /// metadata are honoured. #[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, HashMap::new(), 0) + Self::from_provider_env_for_revision(provider_env, &[], HashMap::new(), 0) } + /// Build a `SecretResolver` and the corresponding child-process environment + /// from the provider env map, the set of passthrough keys, and the + /// credential revision. + /// + /// For each entry in `provider_env`: + /// - If the key appears in `passthrough_keys`, the child environment receives + /// the **real** value directly. The resolver does not learn it; the L7 + /// proxy will not rewrite anything for that key. This drops the "agent + /// never sees the real secret" invariant for that key — required for + /// credentials consumed by SDKs that validate format in-process or by + /// transports the proxy cannot rewrite. + /// - Otherwise the child environment receives a revisioned placeholder + /// (`openshell:resolve:env:[v_]KEY`) and the resolver maps that + /// placeholder back to the real value for on-the-wire substitution. + /// + /// Returns `(child_env, resolver)`. The resolver is `None` only when there + /// are no canonical placeholders to register (every key was passthrough or + /// `provider_env` was empty). pub(crate) fn from_provider_env_for_revision( provider_env: HashMap, + passthrough_keys: &[String], credential_expires_at_ms: HashMap, revision: u64, ) -> (HashMap, Option) { Self::from_provider_env_for_revision_with_current_aliases( provider_env, + passthrough_keys, credential_expires_at_ms, revision, false, @@ -138,6 +162,7 @@ impl SecretResolver { pub(crate) fn from_provider_env_for_current_revision( provider_env: HashMap, + passthrough_keys: &[String], credential_expires_at_ms: HashMap, revision: u64, ) -> (HashMap, Option, Option) { @@ -145,6 +170,7 @@ impl SecretResolver { let (child_env, current_resolver) = Self::from_provider_env_for_revision_with_current_aliases( provider_env, + passthrough_keys, credential_expires_at_ms, 0, true, @@ -156,12 +182,14 @@ impl SecretResolver { let (child_env, revision_resolver) = Self::from_provider_env_for_revision_with_current_aliases( provider_env, + passthrough_keys, credential_expires_at_ms, revision, false, ); let (_, current_resolver) = Self::from_provider_env_for_revision_with_current_aliases( provider_env_for_current, + passthrough_keys, credential_expires_at_ms_for_current, revision, true, @@ -171,6 +199,7 @@ impl SecretResolver { fn from_provider_env_for_revision_with_current_aliases( provider_env: HashMap, + passthrough_keys: &[String], credential_expires_at_ms: HashMap, revision: u64, include_current_aliases: bool, @@ -179,10 +208,17 @@ impl SecretResolver { return (HashMap::new(), None); } + let passthrough: std::collections::HashSet<&str> = + passthrough_keys.iter().map(String::as_str).collect(); + let mut child_env = HashMap::with_capacity(provider_env.len()); let mut by_placeholder = HashMap::with_capacity(provider_env.len()); for (key, value) in provider_env { + if passthrough.contains(key.as_str()) { + child_env.insert(key, value); + continue; + } let placeholder = placeholder_for_env_key_for_revision(&key, revision); let secret = SecretValue { value, @@ -198,7 +234,12 @@ impl SecretResolver { } } - (child_env, Some(Self { by_placeholder })) + let resolver = if by_placeholder.is_empty() { + None + } else { + Some(Self { by_placeholder }) + }; + (child_env, resolver) } pub(crate) fn merge<'a>(resolvers: impl IntoIterator) -> Option { @@ -1027,6 +1068,102 @@ pub fn rewrite_target_for_eval( mod tests { use super::*; + // === Passthrough credential tests === + + #[test] + fn passthrough_key_receives_real_value_in_child_env() { + let provider_env: HashMap = [ + ("SLACK_BOT_TOKEN".to_string(), "xoxb-real".to_string()), + ("CLAUDE_API_KEY".to_string(), "sk-real".to_string()), + ] + .into_iter() + .collect(); + let (child_env, resolver) = SecretResolver::from_provider_env_for_revision( + provider_env, + &["SLACK_BOT_TOKEN".to_string()], + HashMap::new(), + 0, + ); + + // Passthrough key: real value in child env. + assert_eq!( + child_env.get("SLACK_BOT_TOKEN"), + Some(&"xoxb-real".to_string()) + ); + // Non-passthrough key: canonical placeholder in child env (revision 0 + // collapses to the unrevisioned form). + assert_eq!( + child_env.get("CLAUDE_API_KEY"), + Some(&"openshell:resolve:env:CLAUDE_API_KEY".to_string()) + ); + // Resolver only knows about the canonical placeholder. + let resolver = resolver.expect("resolver should exist for non-passthrough key"); + assert_eq!( + resolver.resolve_placeholder("openshell:resolve:env:CLAUDE_API_KEY"), + Some("sk-real") + ); + // Resolver must not learn the real value of the passthrough key. + assert!( + resolver + .resolve_placeholder("openshell:resolve:env:SLACK_BOT_TOKEN") + .is_none() + ); + } + + #[test] + fn all_passthrough_produces_no_resolver() { + let provider_env: HashMap = [ + ("SLACK_BOT_TOKEN".to_string(), "xoxb-real".to_string()), + ("SLACK_SIGNING_SECRET".to_string(), "sig-real".to_string()), + ] + .into_iter() + .collect(); + let (child_env, resolver) = SecretResolver::from_provider_env_for_revision( + provider_env, + &[ + "SLACK_BOT_TOKEN".to_string(), + "SLACK_SIGNING_SECRET".to_string(), + ], + HashMap::new(), + 0, + ); + + assert_eq!( + child_env.get("SLACK_BOT_TOKEN"), + Some(&"xoxb-real".to_string()) + ); + assert_eq!( + child_env.get("SLACK_SIGNING_SECRET"), + Some(&"sig-real".to_string()) + ); + assert!( + resolver.is_none(), + "resolver should be None when every key is passthrough" + ); + } + + #[test] + fn passthrough_entry_for_unrelated_key_is_silently_ignored() { + // A passthrough key not present in provider_env is a no-op (the server + // already validates this on Provider creation; the supervisor must not + // crash if it ever receives a stale list). + let provider_env: HashMap = [("API_KEY".to_string(), "real".to_string())] + .into_iter() + .collect(); + let (child_env, resolver) = SecretResolver::from_provider_env_for_revision( + provider_env, + &["UNRELATED_KEY".to_string()], + HashMap::new(), + 0, + ); + + assert_eq!( + child_env.get("API_KEY"), + Some(&"openshell:resolve:env:API_KEY".to_string()) + ); + assert!(resolver.is_some()); + } + // === Existing tests (preserved) === #[test] diff --git a/crates/openshell-server/src/grpc/policy.rs b/crates/openshell-server/src/grpc/policy.rs index bdc96d862..d84eebdf6 100644 --- a/crates/openshell-server/src/grpc/policy.rs +++ b/crates/openshell-server/src/grpc/policy.rs @@ -683,12 +683,14 @@ pub(super) async fn handle_get_sandbox_provider_environment( sandbox_id = %sandbox_id, provider_count = provider_names.len(), env_count = provider_environment.environment.len(), + passthrough_count = provider_environment.passthrough_keys.len(), provider_env_revision, "GetSandboxProviderEnvironment request completed successfully" ); Ok(Response::new(GetSandboxProviderEnvironmentResponse { environment: provider_environment.environment, + passthrough_keys: provider_environment.passthrough_keys, provider_env_revision, credential_expires_at_ms: provider_environment.credential_expires_at_ms, })) @@ -3330,6 +3332,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), } } diff --git a/crates/openshell-server/src/grpc/provider.rs b/crates/openshell-server/src/grpc/provider.rs index 3ddaae037..83cf7f53d 100644 --- a/crates/openshell-server/src/grpc/provider.rs +++ b/crates/openshell-server/src/grpc/provider.rs @@ -33,10 +33,17 @@ fn redact_provider_credentials(mut provider: Provider) -> Provider { provider } +/// Resolved provider environment for a sandbox. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub(super) struct ProviderEnvironment { + /// Credential environment variables (canonical and passthrough mixed). pub environment: std::collections::HashMap, + /// Expiration timestamps for entries in `environment`. pub credential_expires_at_ms: std::collections::HashMap, + /// Subset of `environment` keys whose value is the real credential and + /// must be injected directly into the agent process — bypassing the + /// canonical placeholder substitution and L7 proxy rewriting. + pub passthrough_keys: Vec, } impl ProviderEnvironment { @@ -169,6 +176,22 @@ pub(super) async fn list_provider_records( pub(super) async fn update_provider_record( store: &Store, provider: Provider, +) -> Result { + update_provider_record_inner(store, provider, false).await +} + +pub(super) async fn update_provider_record_with_options( + store: &Store, + provider: Provider, + clear_passthrough_credentials: bool, +) -> Result { + update_provider_record_inner(store, provider, clear_passthrough_credentials).await +} + +async fn update_provider_record_inner( + store: &Store, + provider: Provider, + clear_passthrough_credentials: bool, ) -> Result { use crate::persistence::{ObjectId, ObjectName}; @@ -214,6 +237,29 @@ pub(super) async fn update_provider_record( candidate.credential_expires_at_ms, provider.credential_expires_at_ms, ); + // Passthrough merge: + // - `clear_passthrough_credentials=true` wipes the list to empty, regardless + // of `provider.passthrough_credentials`. This is the only way to revoke + // passthrough for every key, because proto3 cannot distinguish "absent" + // from "empty list" on a repeated field. + // - When the caller does not declare a new passthrough list (empty incoming + // = preserve), drop any entries whose credential was deleted in this same + // update. Without this, deleting a credential that was marked passthrough + // leaves the provider in a state that fails validation, and the operator + // cannot recover without recreating the provider. + // - When the caller declares a non-empty list, replace verbatim and keep + // strict validation — an explicit orphan is a caller bug and must surface. + let merged_passthrough = if clear_passthrough_credentials { + Vec::new() + } else if provider.passthrough_credentials.is_empty() { + std::mem::take(&mut candidate.passthrough_credentials) + .into_iter() + .filter(|key| candidate.credentials.contains_key(key)) + .collect() + } else { + provider.passthrough_credentials + }; + candidate.passthrough_credentials = merged_passthrough; // Validate BEFORE writing to prevent persisting invalid state super::validation::validate_object_metadata(candidate.metadata.as_ref(), "provider")?; @@ -416,9 +462,12 @@ fn merge_i64_map( /// Resolve provider credentials into environment variables. /// /// For each provider name in the list, fetches the provider from the store and -/// collects credential key-value pairs. Returns a map of environment variables -/// to inject into the sandbox. Credential keys must be unique across attached -/// providers so one provider cannot silently overwrite another provider's token. +/// collects credential key-value pairs and the passthrough opt-in list. Returns +/// a [`ProviderEnvironment`] describing the env map, expiry timestamps, and +/// which keys are passthrough. Credential keys must be unique across attached +/// providers so one provider cannot silently overwrite another provider's +/// token. Duplicate credential keys across providers are rejected; a key is +/// treated as passthrough if its owning provider lists it as passthrough. pub(super) async fn resolve_provider_environment( store: &Store, provider_names: &[String], @@ -429,6 +478,7 @@ pub(super) async fn resolve_provider_environment( let mut env = std::collections::HashMap::new(); let mut expires = std::collections::HashMap::new(); + let mut passthrough = std::collections::HashSet::new(); let now_ms = crate::persistence::current_time_ms(); validate_provider_environment_keys_unique_at(store, provider_names, None, now_ms).await?; @@ -439,39 +489,54 @@ pub(super) async fn resolve_provider_environment( .map_err(|e| Status::internal(format!("failed to fetch provider '{name}': {e}")))? .ok_or_else(|| Status::failed_precondition(format!("provider '{name}' not found")))?; + let provider_passthrough: std::collections::HashSet<&str> = provider + .passthrough_credentials + .iter() + .map(String::as_str) + .collect(); + for (key, value) in &provider.credentials { - if is_valid_env_key(key) { - let expires_at_ms = provider - .credential_expires_at_ms - .get(key) - .copied() - .unwrap_or_default(); - if expires_at_ms > 0 && expires_at_ms <= now_ms { - warn!( - provider_name = %name, - key = %key, - expires_at_ms, - "skipping expired provider credential" - ); - continue; - } - if expires_at_ms > 0 { - expires.entry(key.clone()).or_insert(expires_at_ms); - } - env.entry(key.clone()).or_insert_with(|| value.clone()); - } else { + if !is_valid_env_key(key) { warn!( provider_name = %name, key = %key, "skipping credential with invalid env var key" ); + continue; + } + let expires_at_ms = provider + .credential_expires_at_ms + .get(key) + .copied() + .unwrap_or_default(); + if expires_at_ms > 0 && expires_at_ms <= now_ms { + warn!( + provider_name = %name, + key = %key, + expires_at_ms, + "skipping expired provider credential" + ); + continue; + } + if !env.contains_key(key) { + env.insert(key.clone(), value.clone()); + if expires_at_ms > 0 { + expires.insert(key.clone(), expires_at_ms); + } + if provider_passthrough.contains(key.as_str()) { + passthrough.insert(key.clone()); + } } } } + let mut passthrough_keys: Vec = passthrough.into_iter().collect(); + passthrough_keys.sort(); + Ok(ProviderEnvironment { environment: env, credential_expires_at_ms: expires, + passthrough_keys, }) } @@ -1085,10 +1150,20 @@ pub(super) async fn handle_update_provider( let mut provider = req .provider .ok_or_else(|| Status::invalid_argument("provider is required"))?; + if req.clear_passthrough_credentials && !provider.passthrough_credentials.is_empty() { + return Err(Status::invalid_argument( + "clear_passthrough_credentials and a non-empty passthrough_credentials list are mutually exclusive", + )); + } provider .credential_expires_at_ms .extend(req.credential_expires_at_ms); - let provider = update_provider_record(state.store.as_ref(), provider).await?; + let provider = update_provider_record_with_options( + state.store.as_ref(), + provider, + req.clear_passthrough_credentials, + ) + .await?; Ok(Response::new(ProviderResponse { provider: Some(provider), @@ -1322,6 +1397,7 @@ pub(super) async fn handle_configure_provider_refresh( credential_key.to_string(), expires_at_ms, )]), + passthrough_credentials: Vec::new(), }; update_provider_record(state.store.as_ref(), updated).await?; } @@ -1417,6 +1493,7 @@ pub(super) async fn handle_delete_provider_refresh( credential_key.to_string(), 0, )]), + passthrough_credentials: Vec::new(), }; update_provider_record(state.store.as_ref(), updated).await?; } @@ -1502,6 +1579,7 @@ mod tests { .into_iter() .collect(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), } } @@ -2093,6 +2171,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -2206,6 +2285,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -2248,6 +2328,7 @@ mod tests { "MS_GRAPH_ACCESS_TOKEN".to_string(), manual_expires_at_ms, )]), + passthrough_credentials: Vec::new(), }, ) .await @@ -2301,6 +2382,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -2320,6 +2402,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -2389,6 +2472,7 @@ mod tests { credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -2478,6 +2562,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -2545,6 +2630,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -2660,6 +2746,7 @@ mod tests { config: std::iter::once(("endpoint".to_string(), "https://gitlab.com".to_string())) .collect(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -2825,6 +2912,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -2854,6 +2942,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -2884,6 +2973,7 @@ mod tests { credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -2904,6 +2994,7 @@ mod tests { credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -2975,6 +3066,7 @@ mod tests { credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3011,6 +3103,7 @@ mod tests { credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3047,6 +3140,7 @@ mod tests { credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3073,6 +3167,7 @@ mod tests { credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3101,6 +3196,7 @@ mod tests { credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3148,6 +3244,7 @@ mod tests { credentials: std::iter::once(("SECONDARY".to_string(), String::new())).collect(), config: std::iter::once(("region".to_string(), String::new())).collect(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3199,6 +3296,7 @@ mod tests { credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3228,6 +3326,7 @@ mod tests { credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3259,6 +3358,7 @@ mod tests { credentials: std::iter::once((oversized_key, "value".to_string())).collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3267,11 +3367,242 @@ mod tests { assert_eq!(err.code(), Code::InvalidArgument); } + #[tokio::test] + async fn update_provider_prunes_passthrough_for_deleted_credential() { + let store = Store::connect("sqlite::memory:?cache=shared") + .await + .unwrap(); + + create_provider_record( + &store, + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "slack".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "slack".to_string(), + credentials: [ + ("SLACK_BOT_TOKEN".to_string(), "xoxb".to_string()), + ("SLACK_SIGNING_SECRET".to_string(), "sig".to_string()), + ] + .into_iter() + .collect(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + passthrough_credentials: vec![ + "SLACK_BOT_TOKEN".to_string(), + "SLACK_SIGNING_SECRET".to_string(), + ], + }, + ) + .await + .unwrap(); + + // Caller deletes the bot token without touching the passthrough list. + // The orphaned passthrough entry must be pruned, not block the update. + let updated = update_provider_record( + &store, + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "slack".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: String::new(), + credentials: std::iter::once(("SLACK_BOT_TOKEN".to_string(), String::new())) + .collect(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), + }, + ) + .await + .unwrap(); + + assert!(!updated.credentials.contains_key("SLACK_BOT_TOKEN")); + assert!(updated.credentials.contains_key("SLACK_SIGNING_SECRET")); + assert_eq!( + updated.passthrough_credentials, + vec!["SLACK_SIGNING_SECRET".to_string()] + ); + } + + #[tokio::test] + async fn update_provider_rejects_explicit_passthrough_orphan() { + // When the caller declares a non-empty passthrough list, an entry that + // names a non-existent credential is a caller bug and must surface. + let store = Store::connect("sqlite::memory:?cache=shared") + .await + .unwrap(); + + create_provider_record( + &store, + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "slack-explicit".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "slack".to_string(), + credentials: std::iter::once(("API_KEY".to_string(), "v".to_string())).collect(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), + }, + ) + .await + .unwrap(); + + let err = update_provider_record( + &store, + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "slack-explicit".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: String::new(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + passthrough_credentials: vec!["UNKNOWN_KEY".to_string()], + }, + ) + .await + .unwrap_err(); + + assert_eq!(err.code(), Code::InvalidArgument); + assert!(err.message().contains("not present in credentials")); + } + + #[tokio::test] + async fn update_provider_clear_passthrough_credentials_wipes_list() { + // `clear_passthrough_credentials=true` revokes passthrough for every + // credential. The proto3 repeated field cannot distinguish "absent" + // from "empty list", so without this flag an empty incoming list is + // preserved. + let store = test_store().await; + + create_provider_record( + &store, + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "slack-clear".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "slack".to_string(), + credentials: [ + ("SLACK_BOT_TOKEN".to_string(), "xoxb".to_string()), + ("SLACK_SIGNING_SECRET".to_string(), "sig".to_string()), + ] + .into_iter() + .collect(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + passthrough_credentials: vec![ + "SLACK_BOT_TOKEN".to_string(), + "SLACK_SIGNING_SECRET".to_string(), + ], + }, + ) + .await + .unwrap(); + + let cleared = update_provider_record_with_options( + &store, + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "slack-clear".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: String::new(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), + }, + true, + ) + .await + .unwrap(); + + assert!(cleared.passthrough_credentials.is_empty()); + assert!(cleared.credentials.contains_key("SLACK_BOT_TOKEN")); + assert!(cleared.credentials.contains_key("SLACK_SIGNING_SECRET")); + } + + #[tokio::test] + async fn handle_update_provider_rejects_clear_combined_with_non_empty_passthrough() { + let state = test_server_state().await; + create_provider_record( + state.store.as_ref(), + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "slack-clear-conflict".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "slack".to_string(), + credentials: std::iter::once(("API_KEY".to_string(), "v".to_string())).collect(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), + }, + ) + .await + .unwrap(); + + let err = handle_update_provider( + &state, + Request::new(UpdateProviderRequest { + provider: Some(Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "slack-clear-conflict".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: String::new(), + credentials: HashMap::new(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + passthrough_credentials: vec!["API_KEY".to_string()], + }), + credential_expires_at_ms: HashMap::new(), + clear_passthrough_credentials: true, + }), + ) + .await + .unwrap_err(); + + assert_eq!(err.code(), Code::InvalidArgument); + assert!(err.message().contains("mutually exclusive")); + } + #[tokio::test] async fn resolve_provider_env_empty_list_returns_empty() { let store = test_store().await; let result = resolve_provider_environment(&store, &[]).await.unwrap(); - assert!(result.is_empty()); + assert!(result.environment.is_empty()); + assert!(result.passthrough_keys.is_empty()); } #[tokio::test] @@ -3298,15 +3629,23 @@ mod tests { )) .collect(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }; create_provider_record(&store, provider).await.unwrap(); let result = resolve_provider_environment(&store, &["claude-local".to_string()]) .await .unwrap(); - assert_eq!(result.get("ANTHROPIC_API_KEY"), Some(&"sk-abc".to_string())); - assert_eq!(result.get("CLAUDE_API_KEY"), Some(&"sk-abc".to_string())); - assert!(!result.contains_key("endpoint")); + assert_eq!( + result.environment.get("ANTHROPIC_API_KEY"), + Some(&"sk-abc".to_string()) + ); + assert_eq!( + result.environment.get("CLAUDE_API_KEY"), + Some(&"sk-abc".to_string()) + ); + assert!(!result.environment.contains_key("endpoint")); + assert!(result.passthrough_keys.is_empty()); } #[tokio::test] @@ -3335,6 +3674,7 @@ mod tests { ] .into_iter() .collect(), + passthrough_credentials: Vec::new(), }; create_provider_record(&store, provider).await.unwrap(); @@ -3380,15 +3720,19 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }; create_provider_record(&store, provider).await.unwrap(); let result = resolve_provider_environment(&store, &["test-provider".to_string()]) .await .unwrap(); - assert_eq!(result.get("VALID_KEY"), Some(&"value".to_string())); - assert!(!result.contains_key("nested.api_key")); - assert!(!result.contains_key("bad-key")); + assert_eq!( + result.environment.get("VALID_KEY"), + Some(&"value".to_string()) + ); + assert!(!result.environment.contains_key("nested.api_key")); + assert!(!result.environment.contains_key("bad-key")); } #[tokio::test] @@ -3412,6 +3756,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3431,6 +3776,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3442,8 +3788,14 @@ mod tests { ) .await .unwrap(); - assert_eq!(result.get("ANTHROPIC_API_KEY"), Some(&"sk-abc".to_string())); - assert_eq!(result.get("GITLAB_TOKEN"), Some(&"glpat-xyz".to_string())); + assert_eq!( + result.environment.get("ANTHROPIC_API_KEY"), + Some(&"sk-abc".to_string()) + ); + assert_eq!( + result.environment.get("GITLAB_TOKEN"), + Some(&"glpat-xyz".to_string()) + ); } #[tokio::test] @@ -3464,6 +3816,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3486,6 +3839,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3524,6 +3878,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3546,6 +3901,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3584,6 +3940,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3594,6 +3951,55 @@ mod tests { assert!(err.message().contains("MS_GRAPH_ACCESS_TOKEN")); } + #[tokio::test] + async fn resolve_provider_env_propagates_passthrough_keys() { + let store = test_store().await; + create_provider_record( + &store, + Provider { + metadata: Some(openshell_core::proto::datamodel::v1::ObjectMeta { + id: String::new(), + name: "slack".to_string(), + created_at_ms: 0, + labels: HashMap::new(), + resource_version: 0, + }), + r#type: "slack".to_string(), + credentials: [ + ("SLACK_BOT_TOKEN".to_string(), "xoxb-real".to_string()), + ("SLACK_SIGNING_SECRET".to_string(), "sig-real".to_string()), + ] + .into_iter() + .collect(), + config: HashMap::new(), + credential_expires_at_ms: HashMap::new(), + passthrough_credentials: vec![ + "SLACK_BOT_TOKEN".to_string(), + "SLACK_SIGNING_SECRET".to_string(), + ], + }, + ) + .await + .unwrap(); + + let result = resolve_provider_environment(&store, &["slack".to_string()]) + .await + .unwrap(); + let mut keys = result.passthrough_keys.clone(); + keys.sort(); + assert_eq!( + keys, + vec![ + "SLACK_BOT_TOKEN".to_string(), + "SLACK_SIGNING_SECRET".to_string(), + ] + ); + assert_eq!( + result.environment.get("SLACK_BOT_TOKEN"), + Some(&"xoxb-real".to_string()) + ); + } + #[tokio::test] async fn handler_flow_resolves_credentials_from_sandbox_providers() { use openshell_core::proto::{Sandbox, SandboxPhase, SandboxSpec}; @@ -3618,6 +4024,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }, ) .await @@ -3647,11 +4054,14 @@ mod tests { .unwrap() .unwrap(); let spec = loaded.spec.unwrap(); - let env = resolve_provider_environment(&store, &spec.providers) + let resolved = resolve_provider_environment(&store, &spec.providers) .await .unwrap(); - assert_eq!(env.get("ANTHROPIC_API_KEY"), Some(&"sk-test".to_string())); + assert_eq!( + resolved.environment.get("ANTHROPIC_API_KEY"), + Some(&"sk-test".to_string()) + ); } #[tokio::test] @@ -3681,11 +4091,12 @@ mod tests { .unwrap() .unwrap(); let spec = loaded.spec.unwrap(); - let env = resolve_provider_environment(&store, &spec.providers) + let resolved = resolve_provider_environment(&store, &spec.providers) .await .unwrap(); - assert!(env.is_empty()); + assert!(resolved.environment.is_empty()); + assert!(resolved.passthrough_keys.is_empty()); } #[tokio::test] @@ -3720,6 +4131,7 @@ mod tests { credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), }; // Attempt to update with an oversized credential key (exceeds MAX_MAP_KEY_LEN) @@ -3868,6 +4280,7 @@ mod tests { Request::new(UpdateProviderRequest { provider: Some(updated_provider.clone()), credential_expires_at_ms: HashMap::new(), + clear_passthrough_credentials: false, }), ) .await @@ -3936,6 +4349,7 @@ mod tests { Request::new(UpdateProviderRequest { provider: Some(stale_provider), credential_expires_at_ms: HashMap::new(), + clear_passthrough_credentials: false, }), ) .await @@ -4006,6 +4420,7 @@ mod tests { Request::new(UpdateProviderRequest { provider: Some(updated), credential_expires_at_ms: HashMap::new(), + clear_passthrough_credentials: false, }), ) .await diff --git a/crates/openshell-server/src/grpc/sandbox.rs b/crates/openshell-server/src/grpc/sandbox.rs index 1855972d7..1d41a5545 100644 --- a/crates/openshell-server/src/grpc/sandbox.rs +++ b/crates/openshell-server/src/grpc/sandbox.rs @@ -2087,6 +2087,7 @@ mod tests { .collect(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), } } diff --git a/crates/openshell-server/src/grpc/validation.rs b/crates/openshell-server/src/grpc/validation.rs index 53f292053..5b3f2b756 100644 --- a/crates/openshell-server/src/grpc/validation.rs +++ b/crates/openshell-server/src/grpc/validation.rs @@ -286,6 +286,44 @@ pub(super) fn validate_provider_fields(provider: &Provider) -> Result<(), Status )); } } + validate_passthrough_credentials(provider)?; + Ok(()) +} + +/// Validate the `passthrough_credentials` field: each entry must be a valid +/// environment variable name, must reference a key present in `credentials`, +/// and the list must not contain duplicates. +fn validate_passthrough_credentials(provider: &Provider) -> Result<(), Status> { + if provider.passthrough_credentials.len() > MAX_PROVIDER_CREDENTIALS_ENTRIES { + return Err(Status::invalid_argument(format!( + "provider.passthrough_credentials exceeds maximum entries ({} > {MAX_PROVIDER_CREDENTIALS_ENTRIES})", + provider.passthrough_credentials.len() + ))); + } + let mut seen = std::collections::HashSet::with_capacity(provider.passthrough_credentials.len()); + for key in &provider.passthrough_credentials { + if key.len() > MAX_MAP_KEY_LEN { + return Err(Status::invalid_argument(format!( + "provider.passthrough_credentials key exceeds maximum length ({} > {MAX_MAP_KEY_LEN})", + key.len() + ))); + } + if !super::provider::is_valid_env_key(key) { + return Err(Status::invalid_argument(format!( + "provider.passthrough_credentials contains invalid env var name: '{key}'" + ))); + } + if !provider.credentials.contains_key(key) { + return Err(Status::invalid_argument(format!( + "provider.passthrough_credentials key '{key}' is not present in credentials" + ))); + } + if !seen.insert(key.as_str()) { + return Err(Status::invalid_argument(format!( + "provider.passthrough_credentials contains duplicate entry: '{key}'" + ))); + } + } Ok(()) } @@ -899,6 +937,7 @@ mod tests { credentials, config, credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), } } @@ -982,6 +1021,54 @@ mod tests { assert!(err.message().contains("key")); } + #[test] + fn validate_provider_fields_accepts_passthrough_subset_of_credentials() { + let creds: HashMap = [ + ("API_KEY".to_string(), "v1".to_string()), + ("SIGNING_SECRET".to_string(), "v2".to_string()), + ] + .into_iter() + .collect(); + let mut provider = make_test_provider("ok", "claude", creds, HashMap::new()); + provider.passthrough_credentials = vec!["SIGNING_SECRET".to_string()]; + assert!(validate_provider_fields(&provider).is_ok()); + } + + #[test] + fn validate_provider_fields_accepts_empty_passthrough() { + let provider = make_test_provider("ok", "claude", one_credential(), HashMap::new()); + assert!(validate_provider_fields(&provider).is_ok()); + } + + #[test] + fn validate_provider_fields_rejects_passthrough_key_absent_from_credentials() { + let mut provider = make_test_provider("ok", "claude", one_credential(), HashMap::new()); + provider.passthrough_credentials = vec!["MISSING".to_string()]; + let err = validate_provider_fields(&provider).unwrap_err(); + assert_eq!(err.code(), Code::InvalidArgument); + assert!(err.message().contains("not present in credentials")); + } + + #[test] + fn validate_provider_fields_rejects_invalid_passthrough_env_var_name() { + let mut creds = one_credential(); + creds.insert("bad-key".to_string(), "v".to_string()); + let mut provider = make_test_provider("ok", "claude", creds, HashMap::new()); + provider.passthrough_credentials = vec!["bad-key".to_string()]; + let err = validate_provider_fields(&provider).unwrap_err(); + assert_eq!(err.code(), Code::InvalidArgument); + assert!(err.message().contains("invalid env var name")); + } + + #[test] + fn validate_provider_fields_rejects_duplicate_passthrough_entries() { + let mut provider = make_test_provider("ok", "claude", one_credential(), HashMap::new()); + provider.passthrough_credentials = vec!["KEY".to_string(), "KEY".to_string()]; + let err = validate_provider_fields(&provider).unwrap_err(); + assert_eq!(err.code(), Code::InvalidArgument); + assert!(err.message().contains("duplicate")); + } + #[test] fn validate_provider_fields_rejects_oversized_config_value() { let mut config = HashMap::new(); diff --git a/crates/openshell-server/src/inference.rs b/crates/openshell-server/src/inference.rs index 183a80e74..5bc1f4137 100644 --- a/crates/openshell-server/src/inference.rs +++ b/crates/openshell-server/src/inference.rs @@ -584,6 +584,7 @@ mod tests { credentials: std::iter::once((key_name.to_string(), key_value.to_string())).collect(), config: std::collections::HashMap::new(), credential_expires_at_ms: std::collections::HashMap::new(), + passthrough_credentials: Vec::new(), } } @@ -751,6 +752,7 @@ mod tests { )) .collect(), credential_expires_at_ms: std::collections::HashMap::new(), + passthrough_credentials: Vec::new(), }; store .put_message(&provider) @@ -825,6 +827,7 @@ mod tests { .collect(), config: provider.config.clone(), credential_expires_at_ms: provider.credential_expires_at_ms.clone(), + passthrough_credentials: provider.passthrough_credentials.clone(), }; store .put_message(&rotated_provider) diff --git a/crates/openshell-server/src/provider_refresh.rs b/crates/openshell-server/src/provider_refresh.rs index 161daeb7f..9deeb17db 100644 --- a/crates/openshell-server/src/provider_refresh.rs +++ b/crates/openshell-server/src/provider_refresh.rs @@ -1183,6 +1183,7 @@ mod tests { credentials: HashMap::new(), config: HashMap::new(), credential_expires_at_ms: HashMap::new(), + passthrough_credentials: Vec::new(), } } diff --git a/crates/openshell-tui/src/lib.rs b/crates/openshell-tui/src/lib.rs index 1969715ce..6943621b0 100644 --- a/crates/openshell-tui/src/lib.rs +++ b/crates/openshell-tui/src/lib.rs @@ -1633,6 +1633,7 @@ fn spawn_create_provider(app: &App, tx: mpsc::UnboundedSender) { credentials: credentials.clone(), config: HashMap::default(), credential_expires_at_ms: HashMap::default(), + passthrough_credentials: Vec::new(), }), }; @@ -1725,8 +1726,10 @@ fn spawn_update_provider(app: &App, tx: mpsc::UnboundedSender) { credentials, config: HashMap::default(), credential_expires_at_ms: HashMap::default(), + passthrough_credentials: Vec::new(), }), credential_expires_at_ms: HashMap::default(), + clear_passthrough_credentials: false, }; match tokio::time::timeout(Duration::from_secs(5), client.update_provider(req)).await { diff --git a/docs/sandboxes/manage-providers.mdx b/docs/sandboxes/manage-providers.mdx index 7bc1f977f..f3edd11b2 100644 --- a/docs/sandboxes/manage-providers.mdx +++ b/docs/sandboxes/manage-providers.mdx @@ -199,7 +199,7 @@ provider instance, and pass `--provider ` explicitly. ## How Credential Injection Works -The agent process inside the sandbox never sees real credential values. At startup, the proxy replaces each credential with an opaque placeholder token in the agent's environment. When the agent sends an HTTP request containing a placeholder, the proxy resolves it to the real credential before forwarding upstream. +By default, the agent process inside the sandbox never sees real credential values. At startup, the proxy replaces each credential with an opaque placeholder token in the agent's environment. When the agent sends an HTTP request containing a placeholder, the proxy resolves it to the real credential before forwarding upstream. This default is intentionally narrowed by the [selective passthrough](#selective-passthrough) opt-in below, which is required for credentials that an SDK validates in-process or sends over transports the proxy cannot rewrite. This resolution requires the proxy to see plaintext HTTP. Endpoints must use `protocol: rest` in the policy (which auto-terminates TLS) or explicit `tls: terminate`. Endpoints without TLS termination pass traffic through as an opaque stream, and credential placeholders are forwarded unresolved. @@ -238,6 +238,40 @@ openshell provider create --name google --type generic --credential YOUTUBE_API_ The agent sends `GET /youtube/v3/search?part=snippet&key=`. The proxy resolves the placeholder in the query parameter value and percent-encodes the result before forwarding. +## Selective Passthrough + +Some SDKs reject the placeholder before any HTTP request is made — for example Slack's `@slack/web-api` checks the `xoxb-` prefix on bot tokens, and OAuth libraries parse JWT structure in process. Other credentials never reach the wire at all (Slack signing-secret HMAC, Discord interaction-signature verification) or flow through transports the proxy cannot rewrite (WebSocket payloads after HTTP `101 Switching Protocols`). + +For these cases, mark a credential key as **passthrough**: the supervisor injects the real value directly into the agent process instead of a placeholder. + +```shell +openshell provider create \ + --name slack \ + --type generic \ + --credential SLACK_BOT_TOKEN=xoxb-real-value \ + --credential SLACK_SIGNING_SECRET=real-signing-secret \ + --passthrough SLACK_BOT_TOKEN \ + --passthrough SLACK_SIGNING_SECRET +``` + +`--passthrough` is repeatable, and the same flag is available on `openshell provider update`. Each key must exist in the resolved provider credentials — supplied via `--credential`, discovered through `--from-existing`, or, on `update`, already stored against the provider. On `update`, a non-empty `--passthrough KEY ...` list replaces the existing list verbatim; an empty list (i.e. omitting the flag) preserves the stored list and prunes any entries whose credential was deleted in the same update. + + +Passthrough drops the "agent never sees the real secret" invariant for the listed keys. The real value sits in `/proc//environ` and any descendant process inherits it. Use it only when the consumer demonstrably fails with the canonical placeholder — prefer the placeholder model whenever the consumer permits it. + + +### Revoking passthrough + +To revoke passthrough for every credential on a provider in one operation, use `--clear-passthrough` on `openshell provider update`: + +```shell +openshell provider update --name slack --clear-passthrough +``` + +`--clear-passthrough` is mutually exclusive with `--passthrough` because passing both would be ambiguous (the server cannot apply "clear and then set" in one request — issue two updates instead). After the update, any sandbox launched (or relaunched) against the provider receives canonical placeholders for every credential. Agent processes that are already running keep the real values that were injected into their environment at launch until they are restarted; the gateway does not mutate the env of a live process. + +`openshell provider get ` annotates passthrough keys in its output, and `openshell provider list` shows a `PASSTHROUGH` column with the per-provider count. + ## Supported Provider Types The following provider types are supported. diff --git a/e2e/python/test_sandbox_providers.py b/e2e/python/test_sandbox_providers.py index 083a42492..e8273c5ce 100644 --- a/e2e/python/test_sandbox_providers.py +++ b/e2e/python/test_sandbox_providers.py @@ -70,6 +70,7 @@ def provider( name: str, provider_type: str, credentials: dict[str, str], + passthrough_credentials: list[str] | None = None, ) -> Iterator[str]: """Create a provider for the duration of the block, then delete it.""" _delete_provider(stub, name) @@ -79,6 +80,7 @@ def provider( metadata=datamodel_pb2.ObjectMeta(name=name), type=provider_type, credentials=credentials, + passthrough_credentials=passthrough_credentials or [], ) ) ) @@ -271,6 +273,81 @@ def wait_for_token(sb: Sandbox, expected: str) -> None: raise +# =========================================================================== +# Tests: selective passthrough +# =========================================================================== + + +def test_passthrough_credential_visible_as_real_value( + sandbox: Callable[..., Sandbox], + sandbox_client: SandboxClient, +) -> None: + """A credential listed in passthrough_credentials is injected as the real + value, not as the canonical openshell:resolve:env:* placeholder.""" + real_value = "xoxb-e2e-passthrough-real-value-for-slack" + with provider( + sandbox_client._stub, + name="e2e-test-passthrough-real-value", + provider_type="generic", + credentials={"SLACK_BOT_TOKEN": real_value}, + passthrough_credentials=["SLACK_BOT_TOKEN"], + ) as provider_name: + spec = datamodel_pb2.SandboxSpec( + policy=_default_policy(), + providers=[provider_name], + ) + + def read_slack_token() -> str: + import os + + return os.environ.get("SLACK_BOT_TOKEN", "NOT_SET") + + with sandbox(spec=spec, delete_on_exit=True) as sb: + result = sb.exec_python(read_slack_token) + assert result.exit_code == 0, result.stderr + value = result.stdout.strip() + assert value == real_value + assert value != "openshell:resolve:env:SLACK_BOT_TOKEN" + + +def test_passthrough_and_canonical_credentials_coexist( + sandbox: Callable[..., Sandbox], + sandbox_client: SandboxClient, +) -> None: + """A provider with one passthrough and one canonical credential injects + each in its own mode: real value for the passthrough key, canonical + placeholder for the other.""" + real_signing_secret = "e2e-passthrough-real-signing-secret" + with provider( + sandbox_client._stub, + name="e2e-test-passthrough-mixed", + provider_type="generic", + credentials={ + "SLACK_SIGNING_SECRET": real_signing_secret, + "SLACK_API_KEY": "irrelevant-canonical-value", + }, + passthrough_credentials=["SLACK_SIGNING_SECRET"], + ) as provider_name: + spec = datamodel_pb2.SandboxSpec( + policy=_default_policy(), + providers=[provider_name], + ) + + def read_both_envs() -> str: + import os + + signing = os.environ.get("SLACK_SIGNING_SECRET", "NOT_SET") + api = os.environ.get("SLACK_API_KEY", "NOT_SET") + return f"{signing}|{api}" + + with sandbox(spec=spec, delete_on_exit=True) as sb: + result = sb.exec_python(read_both_envs) + assert result.exit_code == 0, result.stderr + signing, api = result.stdout.strip().split("|", 1) + assert signing == real_signing_secret + assert _is_placeholder_for_env_key(api, "SLACK_API_KEY") + + # =========================================================================== # Tests: security & edge cases # =========================================================================== diff --git a/proto/datamodel.proto b/proto/datamodel.proto index f92d7b7a3..97e08c63b 100644 --- a/proto/datamodel.proto +++ b/proto/datamodel.proto @@ -41,4 +41,18 @@ message Provider { // Expiration timestamps for credential values, keyed by credential/env var // name. A zero or missing value means the credential does not expire. map credential_expires_at_ms = 5; + // Subset of `credentials` keys whose real values should be injected directly + // into the agent process environment instead of the canonical + // `openshell:resolve:env:` placeholder. The L7 proxy does not rewrite + // these on egress because the real value is already on the client side. + // + // Use sparingly: this drops the "agent never sees the real secret" invariant + // for the listed keys. Required for SDKs that validate credential format + // in-process (Slack `xoxb-*`/`xapp-*`), credentials consumed in in-process + // crypto (HMAC signing secrets, signature verification), and transports the + // proxy cannot rewrite (WebSocket payloads after HTTP 101 upgrade). + // + // Each entry must be a valid environment variable name and present in + // `credentials`. + repeated string passthrough_credentials = 6; } diff --git a/proto/openshell.proto b/proto/openshell.proto index 90d1594f7..1caaa05e9 100644 --- a/proto/openshell.proto +++ b/proto/openshell.proto @@ -834,6 +834,11 @@ message UpdateProviderRequest { // Optional per-credential expiry timestamps to merge into the provider. // A zero value removes the expiry for that credential. map credential_expires_at_ms = 2; + // When true, replace the provider's `passthrough_credentials` list with the + // empty list, removing every entry. Use this to revoke passthrough for all + // keys in one operation — an empty `provider.passthrough_credentials` field + // alone is treated as "preserve existing list" and cannot express clear. + bool clear_passthrough_credentials = 3; } // Delete provider request. @@ -1084,6 +1089,11 @@ message GetSandboxProviderEnvironmentResponse { uint64 provider_env_revision = 2; // Expiration timestamps for returned environment variables. map credential_expires_at_ms = 3; + // Subset of keys in `environment` whose value is the real credential and + // must be injected directly into the agent process — bypassing the + // canonical placeholder substitution and L7 proxy rewriting. Sourced from + // each provider's `passthrough_credentials` list. + repeated string passthrough_keys = 4; } // ---------------------------------------------------------------------------