Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
229 changes: 191 additions & 38 deletions src/channels/wasm/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ pub async fn setup_wasm_channels(
extension_manager: Option<&Arc<ExtensionManager>>,
database: Option<&Arc<dyn Database>>,
registered_channel_names: &[String],
startup_active_channel_names: Option<&HashSet<String>>,
ownership_cache: Arc<crate::ownership::OwnershipCache>,
) -> Option<WasmChannelSetup> {
let runtime = match WasmChannelRuntime::new(WasmChannelRuntimeConfig::default()) {
Expand Down Expand Up @@ -93,18 +94,99 @@ pub async fn setup_wasm_channels(
loader = loader.with_secrets_store(Arc::clone(secrets));
}

let results = match loader
.load_from_dir(&config.channels.wasm_channels_dir)
.await
{
Ok(r) => r,
Err(e) => {
tracing::warn!("Failed to scan WASM channels directory: {}", e);
return None;
}
let discovered_channels =
match crate::channels::wasm::discover_channels(&config.channels.wasm_channels_dir).await {
Ok(channels) => channels,
Err(e) => {
tracing::warn!("Failed to scan WASM channels directory: {}", e);
return None;
}
};

let startup_entries: Vec<(String, std::path::PathBuf, Option<std::path::PathBuf>)> =
discovered_channels
.into_iter()
.filter_map(|(name, discovered)| {
startup_active_channel_names
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Severity — Normalized-vs-raw name mismatch in startup filter

startup_active_channel_names contains normalized names (hyphens → underscores, via normalize_persisted_wasm_channel_names in main.rs), but discover_channels returns raw filesystem names. The filter here compares a raw discovered name against the normalized set:

startup_active_channel_names
    .is_none_or(|active_names| active_names.contains(&name))

If a WASM file is named with hyphens (e.g. my-channel.wasm), discovery returns "my-channel" but the normalized set contains "my_channel" — the channel won't be loaded at startup despite being persisted-active. The same mismatch recurs in register_startup_loaded_channels at the second filter (line 203).

Suggested fix: Normalize the discovered name before comparison:

startup_active_channel_names
    .is_none_or(|active_names| active_names.contains(&name.replace('-', "_")))

Or better — apply a shared canonicalize_channel_name() once at discovery and once at persistence to eliminate this class of mismatch entirely.

.is_none_or(|active_names| active_names.contains(&name))
.then_some((name, discovered.wasm_path, discovered.capabilities_path))
})
.collect();

let load_futures = startup_entries.iter().map(|(name, wasm_path, cap_path)| {
loader.load_from_files(name, wasm_path, cap_path.as_deref())
});
let load_results = futures::future::join_all(load_futures).await;

let mut loaded_channels = Vec::new();
let startup_load_error_message = if startup_active_channel_names.is_some() {
"Failed to load persisted-active WASM channel at startup"
} else {
"Failed to load WASM channel at startup"
};
for ((name, wasm_path, _), result) in startup_entries.into_iter().zip(load_results) {
match result {
Ok(loaded) => loaded_channels.push(loaded),
Err(err) => {
tracing::warn!(
channel = %name,
path = %wasm_path.display(),
error = %err,
"{startup_load_error_message}"
);
}
}
}

let wasm_router = Arc::new(WasmChannelRouter::new());
let registration_context = StartupChannelRegistrationContext {
registered_channel_names,
startup_active_channel_names,
config,
secrets_store,
settings_store: settings_store.as_ref(),
pairing_store: &pairing_store,
wasm_router: &wasm_router,
};
let (channels, channel_names) =
register_startup_loaded_channels(loaded_channels, &registration_context).await;

// Always create webhook routes (even with no channels loaded) so that
// channels hot-added at runtime can receive webhooks without a restart.
let webhook_routes = {
Some(create_wasm_channel_router(
Arc::clone(&wasm_router),
extension_manager.map(Arc::clone),
))
};

Some(WasmChannelSetup {
channels,
channel_names,
webhook_routes,
wasm_channel_runtime: runtime,
pairing_store,
wasm_channel_router: wasm_router,
})
}

struct StartupChannelRegistrationContext<'a> {
registered_channel_names: &'a [String],
startup_active_channel_names: Option<&'a HashSet<String>>,
config: &'a Config,
secrets_store: &'a Option<Arc<dyn SecretsStore + Send + Sync>>,
settings_store: Option<&'a Arc<dyn crate::db::SettingsStore>>,
pairing_store: &'a Arc<PairingStore>,
wasm_router: &'a Arc<WasmChannelRouter>,
}

async fn register_startup_loaded_channels(
loaded_channels: Vec<LoadedChannel>,
context: &StartupChannelRegistrationContext<'_>,
) -> (
Vec<(String, Box<dyn crate::channels::Channel>)>,
Vec<String>,
) {
let mut channels: Vec<(String, Box<dyn crate::channels::Channel>)> = Vec::new();
let mut channel_names: Vec<String> = Vec::new();

Expand All @@ -116,7 +198,14 @@ pub async fn setup_wasm_channels(
// - All native/built-in channel names (prevent impersonation)
// - Trusted approval channels from session::TRUSTED_APPROVAL_CHANNELS
// - The bootstrap sentinel (universal approval wildcard)
for loaded in results.loaded {
for loaded in loaded_channels {
if !context
.startup_active_channel_names
.is_none_or(|active_names| active_names.contains(loaded.name()))
{
continue;
}
Comment on lines +202 to +207
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This check is redundant. The loaded_channels vector is derived from startup_entries, which is already filtered by startup_active_channel_names in the caller function setup_wasm_channels (lines 106-114). Removing this redundant check simplifies the loop logic.


let name_lower = loaded.name().to_ascii_lowercase();
if is_reserved_wasm_channel_name(&name_lower) {
tracing::warn!(
Expand All @@ -128,7 +217,8 @@ pub async fn setup_wasm_channels(
// 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
if context
.registered_channel_names
.iter()
.any(|n| n.to_ascii_lowercase() == name_lower)
{
Expand All @@ -141,38 +231,18 @@ pub async fn setup_wasm_channels(

let (name, channel) = register_channel(
loaded,
config,
secrets_store,
settings_store.as_ref(),
&pairing_store,
&wasm_router,
context.config,
context.secrets_store,
context.settings_store,
context.pairing_store,
context.wasm_router,
)
.await;
channel_names.push(name.clone());
channels.push((name, channel));
}

for (path, err) in &results.errors {
tracing::warn!("Failed to load WASM channel {}: {}", path.display(), err);
}

// Always create webhook routes (even with no channels loaded) so that
// channels hot-added at runtime can receive webhooks without a restart.
let webhook_routes = {
Some(create_wasm_channel_router(
Arc::clone(&wasm_router),
extension_manager.map(Arc::clone),
))
};

Some(WasmChannelSetup {
channels,
channel_names,
webhook_routes,
wasm_channel_runtime: runtime,
pairing_store,
wasm_channel_router: wasm_router,
})
(channels, channel_names)
}

/// Process a single loaded WASM channel: retrieve secrets, inject config,
Expand Down Expand Up @@ -593,7 +663,7 @@ async fn inject_channel_secrets_into_config(

#[cfg(test)]
mod tests {
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::sync::Arc;

use super::reserved_wasm_channel_names;
Expand Down Expand Up @@ -809,6 +879,89 @@ mod tests {
);
}

#[tokio::test]
async fn register_startup_loaded_channels_only_restores_persisted_active_channels() {
let (config, _temp_dir) = test_config();
let wasm_router = Arc::new(WasmChannelRouter::new());
let pairing_store = Arc::new(PairingStore::new_noop());
let startup_active = HashSet::from(["telegram".to_string()]);
let context = super::StartupChannelRegistrationContext {
registered_channel_names: &[],
startup_active_channel_names: Some(&startup_active),
config: &config,
secrets_store: &None,
settings_store: None,
pairing_store: &pairing_store,
wasm_router: &wasm_router,
};

let (channels, channel_names) = super::register_startup_loaded_channels(
vec![
test_loaded_channel("telegram", serde_json::json!({ "owner_id": 12345 })),
test_loaded_channel("slack", serde_json::json!({ "owner_id": 67890 })),
],
&context,
)
.await;

assert_eq!(channel_names, vec!["telegram".to_string()]);
assert_eq!(channels.len(), 1);
assert!(
wasm_router
.get_channel_for_path("/webhook/telegram")
.await
.is_some(),
"persisted-active channel should be registered on the router"
);
assert!(
wasm_router
.get_channel_for_path("/webhook/slack")
.await
.is_none(),
"installed-but-inactive channel must not be registered on the router"
);
}

#[tokio::test]
async fn register_startup_loaded_channels_without_persistence_restores_all_channels() {
let (config, _temp_dir) = test_config();
let wasm_router = Arc::new(WasmChannelRouter::new());
let pairing_store = Arc::new(PairingStore::new_noop());
let context = super::StartupChannelRegistrationContext {
registered_channel_names: &[],
startup_active_channel_names: None,
config: &config,
secrets_store: &None,
settings_store: None,
pairing_store: &pairing_store,
wasm_router: &wasm_router,
};

let (channels, channel_names) = super::register_startup_loaded_channels(
vec![
test_loaded_channel("telegram", serde_json::json!({ "owner_id": 12345 })),
test_loaded_channel("slack", serde_json::json!({ "owner_id": 67890 })),
],
&context,
)
.await;

assert_eq!(channels.len(), 2);
assert_eq!(channel_names.len(), 2);
assert!(
wasm_router
.get_channel_for_path("/webhook/telegram")
.await
.is_some()
);
assert!(
wasm_router
.get_channel_for_path("/webhook/slack")
.await
.is_some()
);
}

#[tokio::test]
async fn register_channel_routes_capabilities_owner_id_to_wasm_channel() {
let (config, _temp_dir) = test_config();
Expand Down
85 changes: 85 additions & 0 deletions src/channels/web/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2896,6 +2896,20 @@ async fn pending_gate_extension_name(
);
}

if matches!(
tool_name,
"tool_install"
| "tool-install"
| "tool_activate"
| "tool-activate"
| "tool_auth"
| "tool-auth"
) && let Some(name) = parsed_parameters.get("name").and_then(|v| v.as_str())
&& !name.trim().is_empty()
{
return Some(name.to_string());
}

if let Some(tools) = state.tool_registry.as_ref()
&& let Some(name) = tools.provider_extension_for_tool(tool_name).await
{
Expand Down Expand Up @@ -4633,6 +4647,26 @@ mod tests {
assert_eq!(extension_name.as_deref(), Some("telegram"));
}

#[tokio::test]
async fn pending_gate_extension_name_uses_install_parameters_for_hyphenated_activate_tool() {
let state = test_gateway_state(None);

let extension_name = pending_gate_extension_name(
&state,
"test-user",
"tool-activate",
r#"{"name":"telegram"}"#,
&ironclaw_engine::ResumeKind::Authentication {
credential_name: "telegram_bot_token".to_string(),
instructions: "paste token".to_string(),
auth_url: None,
},
)
.await;

assert_eq!(extension_name.as_deref(), Some("telegram"));
}

#[tokio::test]
async fn pending_gate_extension_name_falls_back_to_provider_extension() {
struct ProviderTool;
Expand Down Expand Up @@ -5745,6 +5779,57 @@ mod tests {
);
}

#[tokio::test]
async fn test_extensions_list_reports_installed_inactive_wasm_channel_as_inactive() {
use axum::body::Body;
use tower::ServiceExt;

let secrets = test_secrets_store();
let (ext_mgr, _wasm_tools_dir, wasm_channels_dir) = test_ext_mgr(secrets);
let channel_name = "telegram";
std::fs::write(
wasm_channels_dir
.path()
.join(format!("{channel_name}.wasm")),
b"\0asm fake",
)
.expect("write fake wasm");

let state = test_gateway_state(Some(ext_mgr));
let app = Router::new()
.route("/api/extensions", get(extensions_list_handler))
.with_state(state);

let mut req = axum::http::Request::builder()
.method("GET")
.uri("/api/extensions")
.body(Body::empty())
.expect("request");
req.extensions_mut().insert(UserIdentity {
user_id: "test".to_string(),
role: "admin".to_string(),
workspace_read_scopes: Vec::new(),
});

let resp = ServiceExt::<axum::http::Request<Body>>::oneshot(app, req)
.await
.expect("response");
assert_eq!(resp.status(), StatusCode::OK);

let body = axum::body::to_bytes(resp.into_body(), 1024 * 64)
.await
.expect("body");
let parsed: serde_json::Value = serde_json::from_slice(&body).expect("json response");
let telegram = parsed["extensions"]
.as_array()
.and_then(|items| items.iter().find(|item| item["name"] == channel_name))
.expect("telegram extension entry");
assert_eq!(telegram["kind"], "wasm_channel");
assert_eq!(telegram["active"], false);
assert_eq!(telegram["authenticated"], false);
assert_eq!(telegram["activation_status"], "installed");
}

#[test]
fn test_extension_phase_for_web_prefers_error_then_readiness() {
let mut ext = crate::extensions::InstalledExtension {
Expand Down
Loading
Loading