diff --git a/src/channels/wasm/wrapper.rs b/src/channels/wasm/wrapper.rs index b47bf61f23e..42bf25d8835 100644 --- a/src/channels/wasm/wrapper.rs +++ b/src/channels/wasm/wrapper.rs @@ -1077,15 +1077,11 @@ impl WasmChannel { .await; } - /// Load broadcast metadata from settings store on startup. + /// Load broadcast metadata from the owner-scoped settings store on startup. /// - /// # Legacy migration (remove after ownership model rollout — tracked in #2100) - /// - /// If no metadata is found under `self.owner_scope_id`, a second lookup - /// under `"default"` is attempted for backward compatibility with instances - /// that stored broadcast metadata before the ownership model migration. - /// Remove this fallback once all deployments have run the - /// `migrate_default_owner` bootstrap step and restarted at least once. + /// Broadcast metadata is owner-scoped runtime state. If the configured + /// owner scope has no stored value, we intentionally leave the in-memory + /// state empty rather than falling back to `default`. async fn load_broadcast_metadata(&self) { if let Some(ref store) = self.settings_store { match store @@ -1100,29 +1096,11 @@ impl WasmChannel { ); } Ok(_) => { - // LEGACY MIGRATION: remove after ownership model rollout — tracked in #2100 - if self.owner_scope_id != "default" { - match store - .get_setting("default", &self.broadcast_metadata_key()) - .await - { - Ok(Some(serde_json::Value::String(meta))) => { - *self.last_broadcast_metadata.write().await = Some(meta); - tracing::debug!( - channel = %self.name, - "Restored legacy owner broadcast metadata from default scope" - ); - } - Ok(_) => {} - Err(e) => { - tracing::warn!( - channel = %self.name, - "Failed to load legacy broadcast metadata: {}", - e - ); - } - } - } + tracing::debug!( + channel = %self.name, + owner_scope_id = %self.owner_scope_id, + "No owner-scoped broadcast metadata stored" + ); } Err(e) => { tracing::warn!( @@ -4360,6 +4338,7 @@ fn read_attachments(paths: &[String]) -> Result, St #[cfg(test)] mod tests { + use std::collections::HashMap; use std::sync::Arc; use std::time::Duration; @@ -4424,6 +4403,13 @@ mod tests { } fn create_test_channel_with_owner_scope(owner_scope_id: &str) -> WasmChannel { + create_test_channel_with_owner_scope_and_settings(owner_scope_id, None) + } + + fn create_test_channel_with_owner_scope_and_settings( + owner_scope_id: &str, + settings_store: Option>, + ) -> WasmChannel { let config = WasmChannelRuntimeConfig::for_testing(); let runtime = Arc::new(WasmChannelRuntime::new(config).unwrap()); @@ -4443,10 +4429,121 @@ mod tests { owner_scope_id, "{}".to_string(), Arc::new(PairingStore::new_noop()), - None, + settings_store, ) } + struct RecordingSettingsStore { + values: tokio::sync::RwLock>>, + lookups: tokio::sync::RwLock>, + } + + impl RecordingSettingsStore { + fn new() -> Self { + Self { + values: tokio::sync::RwLock::new(HashMap::new()), + lookups: tokio::sync::RwLock::new(Vec::new()), + } + } + + async fn seed(&self, user_id: &str, key: &str, value: serde_json::Value) { + self.values + .write() + .await + .entry(user_id.to_string()) + .or_default() + .insert(key.to_string(), value); + } + + async fn lookups(&self) -> Vec<(String, String)> { + self.lookups.read().await.clone() + } + } + + #[async_trait::async_trait] + impl crate::db::SettingsStore for RecordingSettingsStore { + async fn get_setting( + &self, + user_id: &str, + key: &str, + ) -> Result, crate::error::DatabaseError> { + self.lookups + .write() + .await + .push((user_id.to_string(), key.to_string())); + Ok(self + .values + .read() + .await + .get(user_id) + .and_then(|row| row.get(key).cloned())) + } + + async fn get_setting_full( + &self, + _user_id: &str, + _key: &str, + ) -> Result, crate::error::DatabaseError> { + Err(crate::error::DatabaseError::Query( + "RecordingSettingsStore::get_setting_full not implemented".into(), + )) + } + + async fn set_setting( + &self, + user_id: &str, + key: &str, + value: &serde_json::Value, + ) -> Result<(), crate::error::DatabaseError> { + self.seed(user_id, key, value.clone()).await; + Ok(()) + } + + async fn delete_setting( + &self, + user_id: &str, + key: &str, + ) -> Result { + let mut rows = self.values.write().await; + Ok(rows + .get_mut(user_id) + .map(|row| row.remove(key).is_some()) + .unwrap_or(false)) + } + + async fn list_settings( + &self, + _user_id: &str, + ) -> Result, crate::error::DatabaseError> { + Err(crate::error::DatabaseError::Query( + "RecordingSettingsStore::list_settings not implemented".into(), + )) + } + + async fn get_all_settings( + &self, + _user_id: &str, + ) -> Result, crate::error::DatabaseError> { + Err(crate::error::DatabaseError::Query( + "RecordingSettingsStore::get_all_settings not implemented".into(), + )) + } + + async fn set_all_settings( + &self, + _user_id: &str, + _settings: &HashMap, + ) -> Result<(), crate::error::DatabaseError> { + Err(crate::error::DatabaseError::Query( + "RecordingSettingsStore::set_all_settings not implemented".into(), + )) + } + + async fn has_settings(&self, user_id: &str) -> Result { + Ok(self.values.read().await.contains_key(user_id)) + } + } + #[test] fn test_websocket_runtime_config_reads_capability_payload() { let mut tool_capabilities = ToolCapabilities::default(); @@ -6239,6 +6336,40 @@ mod tests { assert_eq!(stored_metadata.as_deref(), Some(r#"{"chat_id":12345}"#)); // safety: test-only assertion } + #[tokio::test] + async fn test_start_does_not_fallback_to_default_broadcast_metadata_scope() { + let store = Arc::new(RecordingSettingsStore::new()); + store + .seed( + "default", + "channel_broadcast_metadata_test", + serde_json::json!(r#"{"chat_id":12345}"#), + ) + .await; + + let channel = create_test_channel_with_owner_scope_and_settings( + "owner-scope", + Some(store.clone() as Arc), + ); + + let _stream = channel.start().await.expect("Channel should start"); + + assert!( + channel.last_broadcast_metadata.read().await.is_none(), + "owner-scoped metadata should stay empty when only default scope has a value" + ); + assert_eq!( + store.lookups().await, + vec![( + "owner-scope".to_string(), + "channel_broadcast_metadata_test".to_string() + )], + "load_broadcast_metadata must not probe default scope" + ); + + channel.shutdown().await.expect("Shutdown should succeed"); + } + #[tokio::test] async fn test_dispatch_emitted_messages_guest_sender_stays_isolated() { use crate::channels::wasm::host::EmittedMessage; diff --git a/src/tools/wasm/wrapper.rs b/src/tools/wasm/wrapper.rs index 2edede2aa6e..83d9344b9a8 100644 --- a/src/tools/wasm/wrapper.rs +++ b/src/tools/wasm/wrapper.rs @@ -1255,11 +1255,11 @@ impl Tool for WasmToolWrapper { // Pre-resolve host credentials from secrets store (async, before blocking task). // This decrypts the secrets once so the sync http_request() host function // can inject them without needing async access. - let credential_user_id = &ctx.user_id; + let credential_user_id = ctx.user_id.clone(); let resolution = resolve_host_credentials( &self.capabilities, self.secrets_store.as_deref(), - credential_user_id, + &credential_user_id, self.role_lookup.as_deref(), self.oauth_refresh.as_ref(), ) @@ -1274,9 +1274,10 @@ impl Tool for WasmToolWrapper { // mapping `optional = true` in their capabilities manifest. if !resolution.missing_required.is_empty() { return Err(ToolError::ExecutionFailed(format!( - "WASM tool '{}' requires credentials that are not configured: {}. \ - Configure the missing credentials before re-running the tool.", + "WASM tool '{}' requires credentials that are not configured for user '{}': {}. \ + Configure the missing credentials with `ironclaw secrets set` and re-run the tool.", self.name(), + credential_user_id, resolution.missing_required.join(", ") ))); } @@ -1375,16 +1376,14 @@ impl std::fmt::Debug for WasmToolWrapper { /// so that the synchronous WASM host function can inject credentials /// without needing async access to the secrets store. /// -/// Silently skips credentials that can't be resolved (e.g., missing secrets). -/// The tool will get a 401/403 from the API, which is the expected UX when -/// auth hasn't been configured yet. -/// Outcome of pre-resolving WASM tool host credentials. Carries both the -/// successfully-resolved set and any *required* credentials that could not -/// be resolved. The caller is responsible for refusing to execute the tool -/// when `missing_required` is non-empty — proceeding would let the tool -/// issue requests without the credentials it declared, which a malicious -/// or misconfigured tool can use to exfiltrate user context to an -/// unauthenticated endpoint. +/// Resolves the host credentials declared by a WASM tool for the current user. +/// +/// Optional mappings may be skipped when unavailable. Required mappings are +/// tracked in `missing_required` so the caller can fail closed before +/// execution instead of letting the tool run without the credentials it +/// declared. That prevents a malicious or misconfigured tool from issuing +/// requests that silently borrow another scope's secrets or exfiltrate user +/// context to an unauthenticated endpoint. struct HostCredentialsResolution { resolved: Vec, missing_required: Vec, @@ -1479,7 +1478,7 @@ async fn resolve_host_credentials( &mapping.secret_name, role_lookup, oauth_refresh.filter(|config| config.secret_name == mapping.secret_name), - crate::auth::DefaultFallback::AdminOnly, + crate::auth::DefaultFallback::Denied, ) .await { @@ -3464,7 +3463,7 @@ mod tests { #[cfg(feature = "libsql")] #[tokio::test] - async fn test_resolve_host_credentials_fallback_to_default_for_admin_user() { + async fn test_resolve_host_credentials_denies_default_fallback_for_admin_user() { use crate::secrets::{CredentialLocation, CredentialMapping, SecretsStore}; use crate::tools::wasm::capabilities::HttpCapability; use crate::tools::wasm::wrapper::resolve_host_credentials; @@ -3504,8 +3503,9 @@ mod tests { ..Default::default() }; - // Resolve credentials for a different user (routine context) - // Should fallback to "default" and find the token + // Resolve credentials for a different user (routine context). + // The WASM tool path must fail closed and refuse to borrow the + // admin-only default scope. let result = resolve_host_credentials( &caps, Some(&store), @@ -3515,8 +3515,15 @@ mod tests { ) .await; - assert!(!result.is_empty(), "fallback to default"); // safety: test code only - assert_eq!(result[0].secret_value, "global_token_value"); // safety: test code only + assert!( + result.is_empty(), + "WASM tool credential resolution must not borrow the default scope" + ); + assert_eq!( + result.missing_required, + vec!["google_oauth_token".to_string()], + "missing required credential should still be reported" + ); } #[cfg(feature = "libsql")]