diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 00000000..9a039e91 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,8 @@ +# Complexity guardrails for AI-assisted development quality. +# These thresholds prevent new violations while preserving existing code. +# See: https://github.com/nearai/ironclaw/issues/338 + +cognitive-complexity-threshold = 15 # default: 25 (only active when lint is enabled) +too-many-lines-threshold = 100 # default: 100 (only active when lint is enabled) +too-many-arguments-threshold = 7 # default: 7 (keep default, avoids new violations) +type-complexity-threshold = 250 # default: 250 (keep default, avoids new violations) diff --git a/src/extensions/manager.rs b/src/extensions/manager.rs index 44d598cd..e2c11077 100644 --- a/src/extensions/manager.rs +++ b/src/extensions/manager.rs @@ -544,6 +544,12 @@ impl ExtensionManager { // Unregister from tool registry self.tool_registry.unregister(name).await; + // Revoke credential mappings from the shared registry + let cap_path = self + .wasm_tools_dir + .join(format!("{}.capabilities.json", name)); + self.revoke_credential_mappings(&cap_path).await; + // Unregister hooks registered from this plugin source. let removed_hooks = self .unregister_hook_prefix(&format!("plugin.tool:{}::", name)) @@ -561,9 +567,6 @@ impl ExtensionManager { // Delete files let wasm_path = self.wasm_tools_dir.join(format!("{}.wasm", name)); - let cap_path = self - .wasm_tools_dir - .join(format!("{}.capabilities.json", name)); if wasm_path.exists() { tokio::fs::remove_file(&wasm_path) @@ -587,6 +590,9 @@ impl ExtensionManager { .wasm_channels_dir .join(format!("{}.capabilities.json", name)); + // Revoke credential mappings before deleting the capabilities file + self.revoke_credential_mappings(&cap_path).await; + if wasm_path.exists() { tokio::fs::remove_file(&wasm_path) .await @@ -671,16 +677,17 @@ impl ExtensionManager { primary_error = %primary_err, "Primary install failed, trying fallback source" ); - self.try_install_from_source(entry, fallback) - .await - .map_err(|fallback_err| { + match self.try_install_from_source(entry, fallback).await { + Ok(result) => Ok(result), + Err(fallback_err) => { tracing::error!( extension = %entry.name, fallback_error = %fallback_err, "Fallback install also failed" ); - combine_install_errors(&primary_err, fallback_err) - }) + Err(combine_install_errors(primary_err, fallback_err)) + } + } } } } @@ -2538,6 +2545,47 @@ impl ExtensionManager { } } + /// Read a capabilities.json file and revoke its credential mappings from + /// the shared credential registry, so removed extensions lose injection + /// authority immediately. + async fn revoke_credential_mappings(&self, cap_path: &std::path::Path) { + if !cap_path.exists() { + return; + } + let Ok(bytes) = tokio::fs::read(cap_path).await else { + return; + }; + // Extract secret names from the capabilities JSON. + // Structure: { "http": { "credentials": { "": { "secret_name": "..." } } } } + let Ok(json) = serde_json::from_slice::(&bytes) else { + return; + }; + let secret_names: Vec = json + .get("http") + .and_then(|h| h.get("credentials")) + .and_then(|c| c.as_object()) + .map(|creds| { + creds + .values() + .filter_map(|v| v.get("secret_name").and_then(|s| s.as_str())) + .map(String::from) + .collect() + }) + .unwrap_or_default(); + + if secret_names.is_empty() { + return; + } + + if let Some(cr) = self.tool_registry.credential_registry() { + cr.remove_mappings_for_secrets(&secret_names); + tracing::info!( + secrets = ?secret_names, + "Revoked credential mappings for removed extension" + ); + } + } + async fn unregister_hook_prefix(&self, prefix: &str) -> usize { let Some(ref hooks) = self.hooks else { return 0; @@ -2640,18 +2688,18 @@ fn fallback_decision( /// Combine primary and fallback errors into a single error. /// /// Preserves `AlreadyInstalled` from the fallback directly; otherwise wraps -/// both error messages into `ExtensionError::Other`. +/// both errors into the structured `ExtensionError::FallbackFailed` variant. fn combine_install_errors( - primary_err: &ExtensionError, + primary_err: ExtensionError, fallback_err: ExtensionError, ) -> ExtensionError { if matches!(fallback_err, ExtensionError::AlreadyInstalled(_)) { return fallback_err; } - ExtensionError::Other(format!( - "Primary install failed: {}; fallback install also failed: {}", - primary_err, fallback_err - )) + ExtensionError::FallbackFailed { + primary: Box::new(primary_err), + fallback: Box::new(fallback_err), + } } #[cfg(test)] @@ -2748,7 +2796,11 @@ mod tests { fn test_combine_errors_includes_both_messages() { let primary = ExtensionError::DownloadFailed("404 Not Found".to_string()); let fallback = ExtensionError::InstallFailed("cargo not found".to_string()); - let combined = combine_install_errors(&primary, fallback); + let combined = combine_install_errors(primary, fallback); + assert!( + matches!(combined, ExtensionError::FallbackFailed { .. }), + "Expected FallbackFailed, got: {combined:?}" + ); let msg = combined.to_string(); assert!(msg.contains("404 Not Found"), "missing primary: {msg}"); assert!(msg.contains("cargo not found"), "missing fallback: {msg}"); @@ -2758,7 +2810,7 @@ mod tests { fn test_combine_errors_forwards_already_installed_from_fallback() { let primary = ExtensionError::DownloadFailed("404".to_string()); let fallback = ExtensionError::AlreadyInstalled("test".to_string()); - let combined = combine_install_errors(&primary, fallback); + let combined = combine_install_errors(primary, fallback); assert!( matches!(combined, ExtensionError::AlreadyInstalled(ref name) if name == "test"), "Expected AlreadyInstalled, got: {combined:?}" diff --git a/src/extensions/mod.rs b/src/extensions/mod.rs index 3b166b46..15b913d9 100644 --- a/src/extensions/mod.rs +++ b/src/extensions/mod.rs @@ -244,6 +244,12 @@ pub enum ExtensionError { #[error("Config error: {0}")] Config(String), + #[error("Primary install failed: {primary}; fallback install also failed: {fallback}")] + FallbackFailed { + primary: Box, + fallback: Box, + }, + #[error("{0}")] Other(String), } diff --git a/src/sandbox/detect.rs b/src/sandbox/detect.rs index 36481ebb..e7db8819 100644 --- a/src/sandbox/detect.rs +++ b/src/sandbox/detect.rs @@ -85,7 +85,9 @@ impl Platform { /// Instructions to start the Docker daemon on this platform. pub fn start_hint(&self) -> &'static str { match self { - Platform::MacOS => "Start Docker Desktop from Applications, or run: open -a Docker", + Platform::MacOS => { + "Start Docker Desktop from Applications, or run: open -a Docker\n\n To auto-start at login: System Settings > General > Login Items > add Docker.app" + } Platform::Linux => "Start the Docker daemon: sudo systemctl start docker", Platform::Windows => "Start Docker Desktop from the Start menu", } diff --git a/src/setup/channels.rs b/src/setup/channels.rs index 920a383f..c0a48ab7 100644 --- a/src/setup/channels.rs +++ b/src/setup/channels.rs @@ -20,8 +20,8 @@ use crate::secrets::SecretsCrypto; use crate::secrets::{CreateSecretParams, SecretsStore}; use crate::settings::{Settings, TunnelSettings}; use crate::setup::prompts::{ - confirm, input, optional_input, print_error, print_info, print_success, secret_input, - select_one, + confirm, input, optional_input, print_error, print_info, print_success, print_warning, + secret_input, select_one, }; /// Typed errors for channel setup flows. @@ -38,6 +38,9 @@ pub enum ChannelSetupError { #[error("{0}")] Validation(String), + + #[error("Setup cancelled by user")] + Cancelled, } /// Context for saving secrets during setup. @@ -490,6 +493,15 @@ fn setup_tunnel_cloudflare() -> Result { } } + // Detect existing cloudflared services that may conflict + if let Some(warning) = detect_existing_cloudflared() { + print_warning(&warning); + if !confirm("Continue anyway?", true)? { + return Err(ChannelSetupError::Cancelled); + } + println!(); + } + print_info("Get your tunnel token from the Cloudflare Zero Trust dashboard:"); print_info(" https://one.dash.cloudflare.com/ > Networks > Tunnels"); println!(); @@ -529,6 +541,95 @@ fn setup_tunnel_cloudflare() -> Result { }) } +/// Detect running cloudflared processes or managed services that could conflict +/// with IronClaw's tunnel management. +fn detect_existing_cloudflared() -> Option { + let mut conflicts = Vec::new(); + + // Check for running cloudflared processes (all platforms) + #[cfg(unix)] + { + let output = std::process::Command::new("pgrep") + .args(["-x", "cloudflared"]) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .output(); + if let Ok(out) = output + && out.status.success() + { + let pids = String::from_utf8_lossy(&out.stdout); + let pids: Vec<&str> = pids.trim().lines().collect(); + if !pids.is_empty() { + conflicts.push(format!( + "Running cloudflared process(es): PID {}", + pids.join(", ") + )); + } + } + } + + // macOS: check brew services + #[cfg(target_os = "macos")] + { + let output = std::process::Command::new("brew") + .args(["services", "list"]) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .output(); + if let Ok(out) = output { + let stdout = String::from_utf8_lossy(&out.stdout); + for line in stdout.lines() { + if line.contains("cloudflared") && line.contains("started") { + conflicts.push("Homebrew service: cloudflared (started)".to_string()); + break; + } + } + } + + let output = std::process::Command::new("launchctl") + .args(["list"]) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .output(); + if let Ok(out) = output { + let stdout = String::from_utf8_lossy(&out.stdout); + for line in stdout.lines() { + if line.contains("cloudflared") { + conflicts.push("launchd service: cloudflared detected".to_string()); + break; + } + } + } + } + + // Linux: check systemd + #[cfg(target_os = "linux")] + { + let output = std::process::Command::new("systemctl") + .args(["is-active", "cloudflared"]) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .output(); + if let Ok(out) = output { + let stdout = String::from_utf8_lossy(&out.stdout); + if stdout.trim() == "active" { + conflicts.push("systemd service: cloudflared (active)".to_string()); + } + } + } + + if conflicts.is_empty() { + None + } else { + Some(format!( + "Detected existing cloudflared service(s) that may conflict:\n {}\n\ + Consider stopping them first (e.g., `brew services stop cloudflared` or \ + `sudo systemctl stop cloudflared`).", + conflicts.join("\n ") + )) + } +} + fn setup_tunnel_tailscale() -> Result { let funnel = confirm("Use Tailscale Funnel (public internet)?", true)?; let hostname = optional_input("Hostname override", Some("leave empty for auto-detect"))?; diff --git a/src/setup/prompts.rs b/src/setup/prompts.rs index 8b50af8c..df4cbbc2 100644 --- a/src/setup/prompts.rs +++ b/src/setup/prompts.rs @@ -311,6 +311,15 @@ pub fn print_error(message: &str) { eprintln!(" {}", message); } +/// Print a warning message with yellow exclamation. +pub fn print_warning(message: &str) { + let mut stdout = io::stdout(); + let _ = execute!(stdout, SetForegroundColor(Color::Yellow)); + print!("!"); + let _ = execute!(stdout, ResetColor); + println!(" {}", message); +} + /// Print an info message with blue info icon. pub fn print_info(message: &str) { let mut stdout = io::stdout(); diff --git a/src/tools/wasm/credential_injector.rs b/src/tools/wasm/credential_injector.rs index 7f735a6d..aff719c0 100644 --- a/src/tools/wasm/credential_injector.rs +++ b/src/tools/wasm/credential_injector.rs @@ -77,7 +77,7 @@ impl SharedCredentialRegistry { } } - /// Add credential mappings (called when WASM tools register). + /// Add credential mappings tagged with an extension name (called when WASM tools register). pub fn add_mappings(&self, mappings: impl IntoIterator) { match self.mappings.write() { Ok(mut guard) => { @@ -93,6 +93,23 @@ impl SharedCredentialRegistry { } } + /// Remove all credential mappings whose `secret_name` matches any of the given names. + /// + /// Called when an extension is unregistered/deactivated so its credential + /// injection authority does not outlive the extension. + pub fn remove_mappings_for_secrets(&self, secret_names: &[String]) { + let mut guard = match self.mappings.write() { + Ok(guard) => guard, + Err(poisoned) => { + tracing::warn!( + "SharedCredentialRegistry RwLock poisoned during remove_mappings_for_secrets; recovering" + ); + poisoned.into_inner() + } + }; + guard.retain(|m| !secret_names.contains(&m.secret_name)); + } + /// Check if any credential mapping matches this host (sync, for requires_approval). pub fn has_credentials_for_host(&self, host: &str) -> bool { let guard = match self.mappings.read() { @@ -564,6 +581,36 @@ mod tests { assert_eq!(found.len(), 2); } + #[test] + fn test_shared_registry_remove_mappings_for_secrets() { + let registry = SharedCredentialRegistry::new(); + registry.add_mappings(vec![ + CredentialMapping::bearer("openai_key", "api.openai.com"), + CredentialMapping::bearer("gh_token", "*.github.com"), + CredentialMapping::header("openai_org", "OpenAI-Organization", "api.openai.com"), + ]); + + assert_eq!(registry.find_for_host("api.openai.com").len(), 2); + assert!(registry.has_credentials_for_host("api.github.com")); + + // Remove only mappings for openai secrets + registry.remove_mappings_for_secrets(&["openai_key".to_string(), "openai_org".to_string()]); + + // OpenAI mappings should be gone + assert!(registry.find_for_host("api.openai.com").is_empty()); + // GitHub mapping should remain + assert!(registry.has_credentials_for_host("api.github.com")); + } + + #[test] + fn test_shared_registry_remove_nonexistent_is_noop() { + let registry = SharedCredentialRegistry::new(); + registry.add_mappings(vec![CredentialMapping::bearer("key1", "api.example.com")]); + + registry.remove_mappings_for_secrets(&["nonexistent".to_string()]); + assert_eq!(registry.find_for_host("api.example.com").len(), 1); + } + #[test] fn test_shared_registry_thread_safety() { use std::sync::Arc; diff --git a/src/workspace/mod.rs b/src/workspace/mod.rs index 71f2f666..c898a7d1 100644 --- a/src/workspace/mod.rs +++ b/src/workspace/mod.rs @@ -813,7 +813,16 @@ impl Workspace { count += 1; } Err(e) => { - tracing::warn!("Failed to embed chunk {}: {}", chunk.id, e); + tracing::warn!( + "Failed to embed chunk {}: {}{}", + chunk.id, + e, + if matches!(e, embeddings::EmbeddingError::AuthFailed) { + ". Check OPENAI_API_KEY or set EMBEDDING_PROVIDER=ollama for local embeddings" + } else { + "" + } + ); } } }