From 3f970252d527ae93738b4b6c094ee1654b33c235 Mon Sep 17 00:00:00 2001 From: Zaki Date: Sat, 28 Mar 2026 11:10:35 -0700 Subject: [PATCH] fix(security): address review feedback on cross-channel approval checks 1. thread_ops.rs: Remove .or(Some(&*message.channel)) fallback in maybe_hydrate_thread() so that when source_channel is NULL in the DB, it stays None rather than being stamped with the requesting channel. This preserves the fail-closed behavior of is_approval_authorized(). 2. libsql_migrations.rs: Remove source_channel from base SCHEMA to eliminate duplicate column definition. The column is now added solely by V14 migration, preventing fresh databases from failing on startup. 3. wasm/setup.rs: Expand RESERVED_CHANNEL_NAMES to cover all built-in channels (http, signal, slack-relay, secret_save) and add a dynamic collision check against already-registered channel names passed from the startup sequence. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/agent/thread_ops.rs | 2 +- src/channels/wasm/setup.rs | 28 +++++++++++++++++++++++++++- src/db/libsql_migrations.rs | 3 +-- src/main.rs | 1 + 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/agent/thread_ops.rs b/src/agent/thread_ops.rs index 60396e31..c53d37eb 100644 --- a/src/agent/thread_ops.rs +++ b/src/agent/thread_ops.rs @@ -146,7 +146,7 @@ impl Agent { } else { None }; - let effective_source_channel = db_source_channel.as_deref().or(Some(&*message.channel)); + let effective_source_channel = db_source_channel.as_deref(); let session_id = { let sess = session.lock().await; diff --git a/src/channels/wasm/setup.rs b/src/channels/wasm/setup.rs index fdda89e2..bf9b084d 100644 --- a/src/channels/wasm/setup.rs +++ b/src/channels/wasm/setup.rs @@ -34,6 +34,7 @@ pub async fn setup_wasm_channels( secrets_store: &Option>, extension_manager: Option<&Arc>, database: Option<&Arc>, + registered_channel_names: &[String], ) -> Option { let runtime = match WasmChannelRuntime::new(WasmChannelRuntimeConfig::default()) { Ok(r) => Arc::new(r), @@ -74,7 +75,19 @@ pub async fn setup_wasm_channels( // Reserved channel names that WASM modules must not claim. // A malicious module could otherwise register as a trusted built-in // channel and bypass cross-channel authorization checks. - const RESERVED_CHANNEL_NAMES: &[&str] = &["web", "gateway", "cli", "repl"]; + // This list must cover every built-in channel name to prevent a WASM + // module from impersonating a built-in and satisfying same-channel + // approval checks. + const RESERVED_CHANNEL_NAMES: &[&str] = &[ + "web", + "gateway", + "cli", + "repl", + "http", + "signal", + "slack-relay", + "secret_save", + ]; for loaded in results.loaded { let name_lower = loaded.name().to_ascii_lowercase(); @@ -85,6 +98,19 @@ pub async fn setup_wasm_channels( ); continue; } + // Also reject any name that collides with an already-registered + // channel to prevent a WASM module from shadowing a channel that + // was registered earlier in the startup sequence. + if registered_channel_names + .iter() + .any(|n| n.to_ascii_lowercase() == name_lower) + { + tracing::warn!( + channel = %loaded.name(), + "Rejected WASM channel that collides with already-registered channel" + ); + continue; + } let (name, channel) = register_channel( loaded, diff --git a/src/db/libsql_migrations.rs b/src/db/libsql_migrations.rs index 4dab6222..c292edba 100644 --- a/src/db/libsql_migrations.rs +++ b/src/db/libsql_migrations.rs @@ -38,8 +38,7 @@ CREATE TABLE IF NOT EXISTS conversations ( thread_id TEXT, started_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), last_activity TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')), - metadata TEXT NOT NULL DEFAULT '{}', - source_channel TEXT + metadata TEXT NOT NULL DEFAULT '{}' ); CREATE INDEX IF NOT EXISTS idx_conversations_channel ON conversations(channel); diff --git a/src/main.rs b/src/main.rs index 88bf76c5..7362588d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -449,6 +449,7 @@ async fn async_main() -> anyhow::Result<()> { &components.secrets_store, components.extension_manager.as_ref(), components.db.as_ref(), + &channel_names, ) .await;