From 4e2dd76ae50d6d097f1c8e7de25a59be8b68ae16 Mon Sep 17 00:00:00 2001 From: Zaki Manian Date: Mon, 23 Feb 2026 10:04:02 -0800 Subject: [PATCH] Fix skills system: enable by default, fix registry and install (#300) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: add Docker detection module with platform guidance Co-Authored-By: Claude Opus 4.6 * feat: add Docker sandbox step to setup wizard Co-Authored-By: Claude Opus 4.6 * feat: show Docker status in boot screen Co-Authored-By: Claude Opus 4.6 * feat: check Docker availability at startup When SANDBOX_ENABLED=true, proactively detect whether Docker is installed and running before creating the ContainerJobManager. If Docker is unavailable, log a warning with platform-specific guidance and disable the sandbox for the session. Co-Authored-By: Claude Opus 4.6 * feat: enable sandbox by default, improve wizard explanation, document detection limits - SandboxConfig defaults to enabled=true (startup check disables gracefully if Docker is unavailable) - Wizard step explains why Docker matters: isolation for LLM-generated code vs running directly on the host - Document detection confidence per platform in detect.rs module docs: high on macOS/Linux, medium on Windows (named pipe edge cases) Co-Authored-By: Claude Opus 4.6 * fix: cargo fmt + update test_builder_defaults for enabled-by-default Co-Authored-By: Claude Opus 4.6 * fix: deduplicate wizard Docker status handling per review Co-Authored-By: Claude Opus 4.6 * feat: fix skills system - enable by default, fix registry connectivity and install - Enable skills system by default (SKILLS_ENABLED no longer required) - Bypass Vercel TLS fingerprint blocking by pointing DEFAULT_REGISTRY_URL directly at the Convex backend (wry-manatee-359.convex.site) - Handle ZIP archives from ClawHub download API - the registry returns ZIP files containing SKILL.md, not raw text. Uses flate2 (existing dep) to extract SKILL.md from the archive. - Surface catalog search errors in the UI with a yellow warning banner instead of silently returning empty results - Handle both {"results":[...]} envelope and bare [...] array JSON formats from the search API - Add ClawHub links and metadata to search result cards (clickable skill names linking to clawhub.ai, relevance score, "updated X ago" recency) - Fix 3 pre-existing clippy warnings in tests/html_to_markdown.rs Co-Authored-By: Claude Opus 4.6 * fix: address security review feedback on ZIP extraction and SSRF - Cap download size to 10 MB before reading response body - Guard against ZIP bombs: cap uncompressed_size at 1 MB, wrap DeflateDecoder with .take() read limit - Use checked_add for ZIP header offset arithmetic to prevent overflow - Remove .unwrap() on try_into() -- use direct array construction - Handle IPv4-mapped IPv6 addresses (::ffff:192.168.x.x) in SSRF checks - Don't leak internal registry URLs in user-facing catalog_error messages - Fix non-ASCII panic in catalog response debug logging (use .get() instead of byte slicing) Co-Authored-By: Claude Opus 4.6 * feat: add /skills command and enrich search results with ClawHub metadata - Parse /skills and /skills search as SystemCommands in submission.rs - Add skill_catalog to AgentDeps and wire it through main.rs - Handle "skills" command in commands.rs: list installed skills and search ClawHub - Add /skills and /skills search entries to /help output - Add SkillDetail, SkillStats, SkillOwner structs to catalog.rs - Add fetch_skill_detail() calling GET /api/v1/skills/{slug} on Convex backend - Add enrich_search_results() to fetch stars/downloads/owner for top 5 results in parallel - Fix SkillDetailResponse wrapper struct to match actual API shape: {"skill":{...},"owner":{...}} - Surface stars, downloads, owner in web UI skill search cards (app.js) - Surface enriched data in skills web handler and skill_search tool output Co-Authored-By: Claude Sonnet 4.6 * fix: cargo fmt after merge conflict resolution Co-Authored-By: Claude Sonnet 4.6 * fix: separate installed_skills dir for correct trust on restart, remove duplicate handlers Trust level bug: skills installed from ClawHub were written to user_dir (~/.ironclaw/skills/) which is discovered as Trusted on restart. Now installs go to ~/.ironclaw/installed_skills/ which is discovered as Installed, matching the documented skill directory layout. Changes: - SkillsConfig: add installed_dir field (SKILLS_INSTALLED_DIR env var, default ~/.ironclaw/installed_skills/) - SkillRegistry: add with_installed_dir() builder, installed_dir()/ install_target_dir() accessors, and discover installed_dir with SkillTrust::Installed in discover_all() - All install paths (web handler, skill tool) use install_target_dir() instead of user_dir() so new installs land in the correct directory - 3 new registry tests: test_installed_dir_uses_installed_trust, test_install_target_dir_prefers_installed_dir, test_user_dir_stays_trusted_with_installed_dir Duplicate handler cleanup: handlers/skills.rs was the canonical implementation but the handlers module was never compiled (not declared in web/mod.rs), so server.rs had its own duplicate inline definitions that the router used. Wire up the handlers module, delete the 260-line duplicate in server.rs, and have server.rs import skills handlers from handlers::skills. Fix pre-existing compile error in handlers/extensions.rs (missing needs_setup field). Add #[allow(dead_code)] on not-yet-migrated handler modules to suppress warnings. Co-Authored-By: Claude Opus 4.6 * fix: probe more Docker socket paths on macOS Docker Desktop 4.13+ (stabilised in 4.18) no longer creates the /var/run/docker.sock symlink by default. The API socket lives at ~/.docker/run/docker.sock, which bollard's connect_with_local_defaults() does not try. Add a fallback probe list covering the common macOS container runtimes: - ~/.docker/run/docker.sock — Docker Desktop 4.13+ - ~/.colima/default/docker.sock — Colima - ~/.rd/docker.sock — Rancher Desktop Remove the bogus ~/.docker/desktop/docker.sock path that was added previously; it is not an API socket on any known Docker installation. Fixes the false-negative "Docker is installed but not running" warning reported by Illia on macOS with Docker Desktop 4.18+. Co-Authored-By: Claude Sonnet 4.6 * Harden Docker detection for rootless Linux and Windows fallback --------- Co-authored-by: Claude Opus 4.6 --- src/agent/agent_loop.rs | 5 + src/agent/commands.rs | 154 +++++++++++ src/agent/dispatcher.rs | 1 + src/agent/submission.rs | 47 ++++ src/app.rs | 3 +- src/boot_screen.rs | 28 +- src/channels/web/handlers/extensions.rs | 1 + src/channels/web/handlers/mod.rs | 39 +-- src/channels/web/handlers/skills.rs | 19 +- src/channels/web/mod.rs | 1 + src/channels/web/server.rs | 250 +----------------- src/channels/web/static/app.js | 335 ++++++++++++++++++++++- src/channels/web/static/index.html | 29 ++ src/channels/web/static/style.css | 76 ++++++ src/channels/web/types.rs | 3 + src/config/skills.rs | 24 +- src/main.rs | 116 +++++--- src/sandbox/config.rs | 2 +- src/sandbox/container.rs | 98 +++++-- src/sandbox/detect.rs | 233 ++++++++++++++++ src/sandbox/manager.rs | 2 +- src/sandbox/mod.rs | 2 + src/setup/wizard.rs | 94 ++++++- src/skills/catalog.rs | 338 ++++++++++++++++++++++-- src/skills/registry.rs | 116 +++++++- src/testing.rs | 1 + src/tools/builtin/skill_tools.rs | 245 +++++++++++++++-- 27 files changed, 1874 insertions(+), 388 deletions(-) create mode 100644 src/sandbox/detect.rs diff --git a/src/agent/agent_loop.rs b/src/agent/agent_loop.rs index 71108b6f..6c6fe9c8 100644 --- a/src/agent/agent_loop.rs +++ b/src/agent/agent_loop.rs @@ -68,6 +68,7 @@ pub struct AgentDeps { pub workspace: Option>, pub extension_manager: Option>, pub skill_registry: Option>>, + pub skill_catalog: Option>, pub skills_config: SkillsConfig, pub hooks: Arc, /// Cost enforcement guardrails (daily budget, hourly rate limits). @@ -174,6 +175,10 @@ impl Agent { self.deps.skill_registry.as_ref() } + pub(super) fn skill_catalog(&self) -> Option<&Arc> { + self.deps.skill_catalog.as_ref() + } + /// Select active skills for a message using deterministic prefiltering. pub(super) fn select_active_skills( &self, diff --git a/src/agent/commands.rs b/src/agent/commands.rs index 2b475727..168e9c7b 100644 --- a/src/agent/commands.rs +++ b/src/agent/commands.rs @@ -15,6 +15,17 @@ use crate::channels::{IncomingMessage, StatusUpdate}; use crate::error::Error; use crate::llm::{ChatMessage, Reasoning}; +/// Format a count with a suffix, using K/M abbreviations for large numbers. +fn format_count(n: u64, suffix: &str) -> String { + if n >= 1_000_000 { + format!("{:.1}M {}", n as f64 / 1_000_000.0, suffix) + } else if n >= 1_000 { + format!("{:.1}K {}", n as f64 / 1_000.0, suffix) + } else { + format!("{} {}", n, suffix) + } +} + impl Agent { /// Handle job-related intents without turn tracking. pub(super) async fn handle_job_or_command( @@ -373,6 +384,10 @@ impl Agent { " /thread Switch to thread\n", " /resume Resume from checkpoint\n", "\n", + "Skills:\n", + " /skills List installed skills\n", + " /skills search Search ClawHub registry\n", + "\n", "Agent:\n", " /heartbeat Run heartbeat check\n", " /summarize Summarize current thread\n", @@ -405,6 +420,22 @@ impl Agent { )) } + "skills" => { + if args.first().map(|s| s.as_str()) == Some("search") { + let query = args[1..].join(" "); + if query.is_empty() { + return Ok(SubmissionResult::error("Usage: /skills search ")); + } + self.handle_skills_search(&query).await + } else if args.is_empty() { + self.handle_skills_list().await + } else { + Ok(SubmissionResult::error( + "Usage: /skills or /skills search ", + )) + } + } + "model" => { let current = self.llm().active_model_name(); @@ -475,6 +506,129 @@ impl Agent { } } + /// List installed skills. + async fn handle_skills_list(&self) -> Result { + let Some(registry) = self.skill_registry() else { + return Ok(SubmissionResult::error("Skills system not enabled.")); + }; + + let guard = match registry.read() { + Ok(g) => g, + Err(e) => { + return Ok(SubmissionResult::error(format!( + "Skill registry lock error: {}", + e + ))); + } + }; + + let skills = guard.skills(); + if skills.is_empty() { + return Ok(SubmissionResult::response( + "No skills installed.\n\nUse /skills search to find skills on ClawHub.", + )); + } + + let mut out = String::from("Installed skills:\n\n"); + for s in skills { + let desc = if s.manifest.description.chars().count() > 60 { + let truncated: String = s.manifest.description.chars().take(57).collect(); + format!("{}...", truncated) + } else { + s.manifest.description.clone() + }; + out.push_str(&format!( + " {:<24} v{:<10} [{}] {}\n", + s.manifest.name, s.manifest.version, s.trust, desc, + )); + } + out.push_str("\nUse /skills search to find more on ClawHub."); + + Ok(SubmissionResult::response(out)) + } + + /// Search ClawHub for skills. + async fn handle_skills_search(&self, query: &str) -> Result { + let catalog = match self.skill_catalog() { + Some(c) => c, + None => { + return Ok(SubmissionResult::error("Skill catalog not available.")); + } + }; + + let outcome = catalog.search(query).await; + + // Enrich top results with detail data (stars, downloads, owner) + let mut entries = outcome.results; + catalog.enrich_search_results(&mut entries, 5).await; + + let mut out = format!("ClawHub results for \"{}\":\n\n", query); + + if entries.is_empty() { + if let Some(ref err) = outcome.error { + out.push_str(&format!(" (registry error: {})\n", err)); + } else { + out.push_str(" No results found.\n"); + } + } else { + for entry in &entries { + let owner_str = entry + .owner + .as_deref() + .map(|o| format!(" by {}", o)) + .unwrap_or_default(); + + let stats_parts: Vec = [ + entry.stars.map(|s| format!("{} stars", s)), + entry.downloads.map(|d| format_count(d, "downloads")), + ] + .into_iter() + .flatten() + .collect(); + let stats_str = if stats_parts.is_empty() { + String::new() + } else { + format!(" {}", stats_parts.join(" ")) + }; + + out.push_str(&format!( + " {:<24} v{:<10}{}{}\n", + entry.name, entry.version, owner_str, stats_str, + )); + if !entry.description.is_empty() { + out.push_str(&format!(" {}\n\n", entry.description)); + } + } + } + + // Show matching installed skills + if let Some(registry) = self.skill_registry() + && let Ok(guard) = registry.read() + { + let query_lower = query.to_lowercase(); + let matches: Vec<_> = guard + .skills() + .iter() + .filter(|s| { + s.manifest.name.to_lowercase().contains(&query_lower) + || s.manifest.description.to_lowercase().contains(&query_lower) + }) + .collect(); + + if !matches.is_empty() { + out.push_str(&format!("Installed skills matching \"{}\":\n", query)); + for s in &matches { + out.push_str(&format!( + " {:<24} v{:<10} [{}]\n", + s.manifest.name, s.manifest.version, s.trust, + )); + } + } + } + + Ok(SubmissionResult::response(out)) + } + /// Handle legacy command routing from the Router (job commands that go through /// process_user_input -> router -> handle_job_or_command -> here). pub(super) async fn handle_command( diff --git a/src/agent/dispatcher.rs b/src/agent/dispatcher.rs index 8a8c3a65..aba94458 100644 --- a/src/agent/dispatcher.rs +++ b/src/agent/dispatcher.rs @@ -960,6 +960,7 @@ mod tests { workspace: None, extension_manager: None, skill_registry: None, + skill_catalog: None, skills_config: SkillsConfig::default(), hooks: Arc::new(HookRegistry::new()), cost_guard: Arc::new(CostGuard::new(CostGuardConfig::default())), diff --git a/src/agent/submission.rs b/src/agent/submission.rs index cd1646df..87ded36d 100644 --- a/src/agent/submission.rs +++ b/src/agent/submission.rs @@ -62,6 +62,23 @@ impl SubmissionParser { args: vec![], }; } + if lower == "/skills" { + return Submission::SystemCommand { + command: "skills".to_string(), + args: vec![], + }; + } + if lower.starts_with("/skills ") { + let args: Vec = trimmed + .split_whitespace() + .skip(1) + .map(|s| s.to_string()) + .collect(); + return Submission::SystemCommand { + command: "skills".to_string(), + args, + }; + } if lower == "/ping" { return Submission::SystemCommand { command: "ping".to_string(), @@ -693,6 +710,36 @@ mod tests { assert!(!submission.starts_turn()); } + #[test] + fn test_parser_system_command_skills() { + let submission = SubmissionParser::parse("/skills"); + assert!( + matches!(submission, Submission::SystemCommand { command, args } if command == "skills" && args.is_empty()) + ); + + // Case insensitive + let submission = SubmissionParser::parse("/SKILLS"); + assert!( + matches!(submission, Submission::SystemCommand { command, .. } if command == "skills") + ); + } + + #[test] + fn test_parser_system_command_skills_search() { + let submission = SubmissionParser::parse("/skills search markdown"); + assert!( + matches!(submission, Submission::SystemCommand { command, args } + if command == "skills" && args == vec!["search", "markdown"]) + ); + + // Multiple words in query + let submission = SubmissionParser::parse("/skills search code review tools"); + assert!( + matches!(submission, Submission::SystemCommand { command, args } + if command == "skills" && args == vec!["search", "code", "review", "tools"]) + ); + } + #[test] fn test_parser_quit() { assert!(matches!(SubmissionParser::parse("/quit"), Submission::Quit)); diff --git a/src/app.rs b/src/app.rs index 1f9724a0..9ee1e200 100644 --- a/src/app.rs +++ b/src/app.rs @@ -692,7 +692,8 @@ impl AppBuilder { // Skills system let (skill_registry, skill_catalog) = if self.config.skills.enabled { - let mut registry = SkillRegistry::new(self.config.skills.local_dir.clone()); + let mut registry = SkillRegistry::new(self.config.skills.local_dir.clone()) + .with_installed_dir(self.config.skills.installed_dir.clone()); let loaded = registry.discover_all().await; if !loaded.is_empty() { tracing::info!("Loaded {} skill(s): {}", loaded.len(), loaded.join(", ")); diff --git a/src/boot_screen.rs b/src/boot_screen.rs index 881c0f1a..d9590ccc 100644 --- a/src/boot_screen.rs +++ b/src/boot_screen.rs @@ -20,8 +20,10 @@ pub struct BootInfo { pub heartbeat_enabled: bool, pub heartbeat_interval_secs: u64, pub sandbox_enabled: bool, + pub docker_status: crate::sandbox::detect::DockerStatus, pub claude_code_enabled: bool, pub routines_enabled: bool, + pub skills_enabled: bool, pub channels: Vec, /// Public URL from a managed tunnel (e.g., "https://abc.ngrok.io"). pub tunnel_url: Option, @@ -35,6 +37,7 @@ pub fn print_boot_screen(info: &BootInfo) { let bold = "\x1b[1m"; let cyan = "\x1b[36m"; let dim = "\x1b[90m"; + let yellow = "\x1b[33m"; let yellow_underline = "\x1b[33;4m"; let reset = "\x1b[0m"; @@ -90,8 +93,19 @@ pub fn print_boot_screen(info: &BootInfo) { let mins = info.heartbeat_interval_secs / 60; features.push(format!("heartbeat ({mins}m)")); } - if info.sandbox_enabled { - features.push("sandbox".to_string()); + match info.docker_status { + crate::sandbox::detect::DockerStatus::Available => { + features.push("sandbox".to_string()); + } + crate::sandbox::detect::DockerStatus::NotInstalled => { + features.push(format!("{yellow}sandbox (docker not installed){reset}")); + } + crate::sandbox::detect::DockerStatus::NotRunning => { + features.push(format!("{yellow}sandbox (docker not running){reset}")); + } + crate::sandbox::detect::DockerStatus::Disabled => { + // Don't show sandbox when disabled + } } if info.claude_code_enabled { features.push("claude-code".to_string()); @@ -99,6 +113,9 @@ pub fn print_boot_screen(info: &BootInfo) { if info.routines_enabled { features.push("routines".to_string()); } + if info.skills_enabled { + features.push("skills".to_string()); + } if !features.is_empty() { println!( " {dim}features{reset} {cyan}{}{reset}", @@ -140,6 +157,7 @@ pub fn print_boot_screen(info: &BootInfo) { #[cfg(test)] mod tests { use super::*; + use crate::sandbox::detect::DockerStatus; #[test] fn test_print_boot_screen_full() { @@ -158,8 +176,10 @@ mod tests { heartbeat_enabled: true, heartbeat_interval_secs: 1800, sandbox_enabled: true, + docker_status: DockerStatus::Available, claude_code_enabled: false, routines_enabled: true, + skills_enabled: true, channels: vec![ "repl".to_string(), "gateway".to_string(), @@ -189,8 +209,10 @@ mod tests { heartbeat_enabled: false, heartbeat_interval_secs: 0, sandbox_enabled: false, + docker_status: DockerStatus::Disabled, claude_code_enabled: false, routines_enabled: false, + skills_enabled: false, channels: vec![], tunnel_url: None, tunnel_provider: None, @@ -216,8 +238,10 @@ mod tests { heartbeat_enabled: false, heartbeat_interval_secs: 0, sandbox_enabled: false, + docker_status: DockerStatus::Disabled, claude_code_enabled: false, routines_enabled: false, + skills_enabled: false, channels: vec!["repl".to_string()], tunnel_url: None, tunnel_provider: None, diff --git a/src/channels/web/handlers/extensions.rs b/src/channels/web/handlers/extensions.rs index c2c87055..76b8321a 100644 --- a/src/channels/web/handlers/extensions.rs +++ b/src/channels/web/handlers/extensions.rs @@ -34,6 +34,7 @@ pub async fn extensions_list_handler( authenticated: ext.authenticated, active: ext.active, tools: ext.tools, + needs_setup: ext.needs_setup, }) .collect(); diff --git a/src/channels/web/handlers/mod.rs b/src/channels/web/handlers/mod.rs index 88cd3d91..0573a067 100644 --- a/src/channels/web/handlers/mod.rs +++ b/src/channels/web/handlers/mod.rs @@ -1,23 +1,28 @@ //! Handler modules for the web gateway API. //! //! Each module groups related endpoint handlers by domain. +//! +//! # Migration status +//! +//! `skills` is the canonical implementation used by `server.rs`. +//! The remaining modules are in-progress migrations from inline server.rs +//! handlers; their functions are not yet wired up, hence the `dead_code` allow. -pub mod chat; -pub mod extensions; -pub mod jobs; -pub mod memory; -pub mod routines; -pub mod settings; pub mod skills; -pub mod static_files; -// Re-export all handler functions so `server.rs` can reference them -// as `handlers::chat_send_handler`, etc. -pub use chat::*; -pub use extensions::*; -pub use jobs::*; -pub use memory::*; -pub use routines::*; -pub use settings::*; -pub use skills::*; -pub use static_files::*; +// Modules not yet wired into server.rs router -- suppress dead_code until +// they replace their inline counterparts. +#[allow(dead_code)] +pub mod chat; +#[allow(dead_code)] +pub mod extensions; +#[allow(dead_code)] +pub mod jobs; +#[allow(dead_code)] +pub mod memory; +#[allow(dead_code)] +pub mod routines; +#[allow(dead_code)] +pub mod settings; +#[allow(dead_code)] +pub mod static_files; diff --git a/src/channels/web/handlers/skills.rs b/src/channels/web/handlers/skills.rs index dc281e40..6bda411b 100644 --- a/src/channels/web/handlers/skills.rs +++ b/src/channels/web/handlers/skills.rs @@ -58,8 +58,14 @@ pub async fn skills_search_handler( ))?; // Search ClawHub catalog - let catalog_results = catalog.search(&req.query).await; - let catalog_json: Vec = catalog_results + let catalog_outcome = catalog.search(&req.query).await; + let catalog_error = catalog_outcome.error.clone(); + + // Enrich top results with detail data (stars, downloads, owner) + let mut entries = catalog_outcome.results; + catalog.enrich_search_results(&mut entries, 5).await; + + let catalog_json: Vec = entries .into_iter() .map(|e| { serde_json::json!({ @@ -68,6 +74,10 @@ pub async fn skills_search_handler( "description": e.description, "version": e.version, "score": e.score, + "updatedAt": e.updated_at, + "stars": e.stars, + "downloads": e.downloads, + "owner": e.owner, }) }) .collect(); @@ -103,6 +113,7 @@ pub async fn skills_search_handler( catalog: catalog_json, installed, registry_url: catalog.registry_url().to_string(), + catalog_error, })) } @@ -147,7 +158,7 @@ pub async fn skills_install_handler( ))); }; - // Parse, check duplicates, and get user_dir under a brief read lock. + // Parse, check duplicates, and get install_dir under a brief read lock. let (user_dir, skill_name_from_parse) = { let guard = registry.read().map_err(|e| { ( @@ -168,7 +179,7 @@ pub async fn skills_install_handler( )))); } - (guard.user_dir().to_path_buf(), skill_name) + (guard.install_target_dir().to_path_buf(), skill_name) }; // Perform async I/O (write to disk, load) with no lock held. diff --git a/src/channels/web/mod.rs b/src/channels/web/mod.rs index 9248e9a1..0c766b98 100644 --- a/src/channels/web/mod.rs +++ b/src/channels/web/mod.rs @@ -15,6 +15,7 @@ //! ``` pub mod auth; +pub(crate) mod handlers; pub mod log_layer; pub mod openai_compat; pub mod server; diff --git a/src/channels/web/server.rs b/src/channels/web/server.rs index fb4b698b..b6bcb74c 100644 --- a/src/channels/web/server.rs +++ b/src/channels/web/server.rs @@ -28,6 +28,9 @@ use uuid::Uuid; use crate::agent::SessionManager; use crate::channels::IncomingMessage; use crate::channels::web::auth::{AuthState, auth_middleware}; +use crate::channels::web::handlers::skills::{ + skills_install_handler, skills_list_handler, skills_remove_handler, skills_search_handler, +}; use crate::channels::web::log_layer::LogBroadcaster; use crate::channels::web::sse::SseManager; use crate::channels::web::types::*; @@ -2086,253 +2089,6 @@ async fn pairing_approve_handler( } } -// --- Skills handlers --- - -async fn skills_list_handler( - State(state): State>, -) -> Result, (StatusCode, String)> { - let registry = state.skill_registry.as_ref().ok_or(( - StatusCode::NOT_IMPLEMENTED, - "Skills system not enabled".to_string(), - ))?; - - let guard = registry.read().map_err(|e| { - ( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Skill registry lock poisoned: {}", e), - ) - })?; - - let skills: Vec = guard - .skills() - .iter() - .map(|s| super::types::SkillInfo { - name: s.manifest.name.clone(), - description: s.manifest.description.clone(), - version: s.manifest.version.clone(), - trust: s.trust.to_string(), - source: format!("{:?}", s.source), - keywords: s.manifest.activation.keywords.clone(), - }) - .collect(); - - let count = skills.len(); - Ok(Json(super::types::SkillListResponse { skills, count })) -} - -async fn skills_search_handler( - State(state): State>, - Json(req): Json, -) -> Result, (StatusCode, String)> { - let registry = state.skill_registry.as_ref().ok_or(( - StatusCode::NOT_IMPLEMENTED, - "Skills system not enabled".to_string(), - ))?; - - let catalog = state.skill_catalog.as_ref().ok_or(( - StatusCode::NOT_IMPLEMENTED, - "Skill catalog not available".to_string(), - ))?; - - // Search ClawHub catalog - let catalog_results = catalog.search(&req.query).await; - let catalog_json: Vec = catalog_results - .into_iter() - .map(|e| { - serde_json::json!({ - "slug": e.slug, - "name": e.name, - "description": e.description, - "version": e.version, - "score": e.score, - }) - }) - .collect(); - - // Search local skills - let query_lower = req.query.to_lowercase(); - let installed: Vec = { - let guard = registry.read().map_err(|e| { - ( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Skill registry lock poisoned: {}", e), - ) - })?; - guard - .skills() - .iter() - .filter(|s| { - s.manifest.name.to_lowercase().contains(&query_lower) - || s.manifest.description.to_lowercase().contains(&query_lower) - }) - .map(|s| super::types::SkillInfo { - name: s.manifest.name.clone(), - description: s.manifest.description.clone(), - version: s.manifest.version.clone(), - trust: s.trust.to_string(), - source: format!("{:?}", s.source), - keywords: s.manifest.activation.keywords.clone(), - }) - .collect() - }; - - Ok(Json(super::types::SkillSearchResponse { - catalog: catalog_json, - installed, - registry_url: catalog.registry_url().to_string(), - })) -} - -async fn skills_install_handler( - State(state): State>, - headers: axum::http::HeaderMap, - Json(req): Json, -) -> Result, (StatusCode, String)> { - // Require explicit confirmation header to prevent accidental installs. - // Chat tools have requires_approval(); this is the equivalent for the web API. - if headers - .get("x-confirm-action") - .and_then(|v| v.to_str().ok()) - != Some("true") - { - return Err(( - StatusCode::BAD_REQUEST, - "Skill install requires X-Confirm-Action: true header".to_string(), - )); - } - - let registry = state.skill_registry.as_ref().ok_or(( - StatusCode::NOT_IMPLEMENTED, - "Skills system not enabled".to_string(), - ))?; - - let content = if let Some(ref raw) = req.content { - raw.clone() - } else if let Some(ref url) = req.url { - // Fetch from explicit URL (with SSRF protection) - crate::tools::builtin::skill_tools::fetch_skill_content(url) - .await - .map_err(|e| (StatusCode::BAD_REQUEST, e.to_string()))? - } else if let Some(ref catalog) = state.skill_catalog { - let url = crate::skills::catalog::skill_download_url(catalog.registry_url(), &req.name); - crate::tools::builtin::skill_tools::fetch_skill_content(&url) - .await - .map_err(|e| (StatusCode::BAD_GATEWAY, e.to_string()))? - } else { - return Ok(Json(ActionResponse::fail( - "Provide 'content' or 'url' to install a skill".to_string(), - ))); - }; - - // Parse, check duplicates, and get user_dir under a brief read lock. - let (user_dir, skill_name_from_parse) = { - let guard = registry.read().map_err(|e| { - ( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Skill registry lock poisoned: {}", e), - ) - })?; - - let normalized = crate::skills::normalize_line_endings(&content); - let parsed = crate::skills::parser::parse_skill_md(&normalized) - .map_err(|e| (StatusCode::BAD_REQUEST, e.to_string()))?; - let skill_name = parsed.manifest.name.clone(); - - if guard.has(&skill_name) { - return Ok(Json(ActionResponse::fail(format!( - "Skill '{}' already exists", - skill_name - )))); - } - - (guard.user_dir().to_path_buf(), skill_name) - }; - - // Perform async I/O (write to disk, load) with no lock held. - let normalized = crate::skills::normalize_line_endings(&content); - let (skill_name, loaded_skill) = - crate::skills::registry::SkillRegistry::prepare_install_to_disk( - &user_dir, - &skill_name_from_parse, - &normalized, - ) - .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; - - // Commit: brief write lock for in-memory addition - let mut guard = registry.write().map_err(|e| { - ( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Skill registry lock poisoned: {}", e), - ) - })?; - - match guard.commit_install(&skill_name, loaded_skill) { - Ok(()) => Ok(Json(ActionResponse::ok(format!( - "Skill '{}' installed", - skill_name - )))), - Err(e) => Ok(Json(ActionResponse::fail(e.to_string()))), - } -} - -async fn skills_remove_handler( - State(state): State>, - headers: axum::http::HeaderMap, - Path(name): Path, -) -> Result, (StatusCode, String)> { - // Require explicit confirmation header to prevent accidental removals. - if headers - .get("x-confirm-action") - .and_then(|v| v.to_str().ok()) - != Some("true") - { - return Err(( - StatusCode::BAD_REQUEST, - "Skill removal requires X-Confirm-Action: true header".to_string(), - )); - } - - let registry = state.skill_registry.as_ref().ok_or(( - StatusCode::NOT_IMPLEMENTED, - "Skills system not enabled".to_string(), - ))?; - - // Validate removal under a brief read lock - let skill_path = { - let guard = registry.read().map_err(|e| { - ( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Skill registry lock poisoned: {}", e), - ) - })?; - guard - .validate_remove(&name) - .map_err(|e| (StatusCode::BAD_REQUEST, e.to_string()))? - }; - - // Delete files from disk (async I/O, no lock held) - crate::skills::registry::SkillRegistry::delete_skill_files(&skill_path) - .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; - - // Remove from in-memory registry under a brief write lock - let mut guard = registry.write().map_err(|e| { - ( - StatusCode::INTERNAL_SERVER_ERROR, - format!("Skill registry lock poisoned: {}", e), - ) - })?; - - match guard.commit_remove(&name) { - Ok(()) => Ok(Json(ActionResponse::ok(format!( - "Skill '{}' removed", - name - )))), - Err(e) => Ok(Json(ActionResponse::fail(e.to_string()))), - } -} - // --- Routines handlers --- async fn routines_list_handler( diff --git a/src/channels/web/static/app.js b/src/channels/web/static/app.js index 412808b4..ce04e621 100644 --- a/src/channels/web/static/app.js +++ b/src/channels/web/static/app.js @@ -93,7 +93,11 @@ function apiFetch(path, options) { opts.body = JSON.stringify(opts.body); } return fetch(path, opts).then((res) => { - if (!res.ok) throw new Error(res.status + ' ' + res.statusText); + if (!res.ok) { + return res.text().then(function(body) { + throw new Error(body || (res.status + ' ' + res.statusText)); + }); + } return res.json(); }); } @@ -846,6 +850,7 @@ function switchTab(tab) { if (tab === 'routines') loadRoutines(); if (tab === 'logs') applyLogFilters(); if (tab === 'extensions') loadExtensions(); + if (tab === 'skills') loadSkills(); } // --- Memory (filesystem tree) --- @@ -2713,6 +2718,328 @@ function addMcpServer() { }); } +// --- Skills --- + +function loadSkills() { + var skillsList = document.getElementById('skills-list'); + apiFetch('/api/skills').then(function(data) { + if (!data.skills || data.skills.length === 0) { + skillsList.innerHTML = '
No skills installed
'; + return; + } + skillsList.innerHTML = ''; + for (var i = 0; i < data.skills.length; i++) { + skillsList.appendChild(renderSkillCard(data.skills[i])); + } + }).catch(function(err) { + skillsList.innerHTML = '
Failed to load skills: ' + escapeHtml(err.message) + '
'; + }); +} + +function renderSkillCard(skill) { + var card = document.createElement('div'); + card.className = 'ext-card'; + + var header = document.createElement('div'); + header.className = 'ext-header'; + + var name = document.createElement('span'); + name.className = 'ext-name'; + name.textContent = skill.name; + header.appendChild(name); + + var trust = document.createElement('span'); + var trustClass = skill.trust.toLowerCase() === 'trusted' ? 'trust-trusted' : 'trust-installed'; + trust.className = 'skill-trust ' + trustClass; + trust.textContent = skill.trust; + header.appendChild(trust); + + var version = document.createElement('span'); + version.className = 'skill-version'; + version.textContent = 'v' + skill.version; + header.appendChild(version); + + card.appendChild(header); + + var desc = document.createElement('div'); + desc.className = 'ext-desc'; + desc.textContent = skill.description; + card.appendChild(desc); + + if (skill.keywords && skill.keywords.length > 0) { + var kw = document.createElement('div'); + kw.className = 'ext-keywords'; + kw.textContent = 'Activates on: ' + skill.keywords.join(', '); + card.appendChild(kw); + } + + var actions = document.createElement('div'); + actions.className = 'ext-actions'; + + // Only show Remove for registry-installed skills, not user-placed trusted skills + if (skill.trust.toLowerCase() !== 'trusted') { + var removeBtn = document.createElement('button'); + removeBtn.className = 'btn-ext remove'; + removeBtn.textContent = 'Remove'; + removeBtn.addEventListener('click', function() { removeSkill(skill.name); }); + actions.appendChild(removeBtn); + } + + card.appendChild(actions); + return card; +} + +function searchClawHub() { + var input = document.getElementById('skill-search-input'); + var query = input.value.trim(); + if (!query) return; + + var resultsDiv = document.getElementById('skill-search-results'); + resultsDiv.innerHTML = '
Searching...
'; + + apiFetch('/api/skills/search', { + method: 'POST', + body: { query: query }, + }).then(function(data) { + resultsDiv.innerHTML = ''; + + // Show registry error as a warning banner if present + if (data.catalog_error) { + var warning = document.createElement('div'); + warning.className = 'empty-state'; + warning.style.color = '#f0ad4e'; + warning.style.borderLeft = '3px solid #f0ad4e'; + warning.style.paddingLeft = '12px'; + warning.style.marginBottom = '16px'; + warning.textContent = 'Could not reach ClawHub registry: ' + data.catalog_error; + resultsDiv.appendChild(warning); + } + + // Show catalog results + if (data.catalog && data.catalog.length > 0) { + // Build a set of installed skill names for quick lookup + var installedNames = {}; + if (data.installed) { + for (var j = 0; j < data.installed.length; j++) { + installedNames[data.installed[j].name] = true; + } + } + + for (var i = 0; i < data.catalog.length; i++) { + var card = renderCatalogSkillCard(data.catalog[i], installedNames); + card.style.animationDelay = (i * 0.06) + 's'; + resultsDiv.appendChild(card); + } + } + + // Show matching installed skills too + if (data.installed && data.installed.length > 0) { + for (var k = 0; k < data.installed.length; k++) { + var installedCard = renderSkillCard(data.installed[k]); + installedCard.style.animationDelay = ((data.catalog ? data.catalog.length : 0) + k) * 0.06 + 's'; + installedCard.classList.add('skill-search-result'); + resultsDiv.appendChild(installedCard); + } + } + + if (resultsDiv.children.length === 0) { + resultsDiv.innerHTML = '
No skills found for "' + escapeHtml(query) + '"
'; + } + }).catch(function(err) { + resultsDiv.innerHTML = '
Search failed: ' + escapeHtml(err.message) + '
'; + }); +} + +function renderCatalogSkillCard(entry, installedNames) { + var card = document.createElement('div'); + card.className = 'ext-card ext-available skill-search-result'; + + var header = document.createElement('div'); + header.className = 'ext-header'; + + var name = document.createElement('a'); + name.className = 'ext-name'; + name.textContent = entry.name || entry.slug; + name.href = 'https://clawhub.ai/skills/' + encodeURIComponent(entry.slug); + name.target = '_blank'; + name.rel = 'noopener'; + name.style.textDecoration = 'none'; + name.style.color = 'inherit'; + name.title = 'View on ClawHub'; + header.appendChild(name); + + if (entry.version) { + var version = document.createElement('span'); + version.className = 'skill-version'; + version.textContent = 'v' + entry.version; + header.appendChild(version); + } + + card.appendChild(header); + + if (entry.description) { + var desc = document.createElement('div'); + desc.className = 'ext-desc'; + desc.textContent = entry.description; + card.appendChild(desc); + } + + // Metadata row: owner, stars, downloads, recency + var meta = document.createElement('div'); + meta.className = 'ext-meta'; + meta.style.fontSize = '11px'; + meta.style.color = '#888'; + meta.style.marginTop = '6px'; + + function addMetaSep() { + if (meta.children.length > 0) { + meta.appendChild(document.createTextNode(' \u00b7 ')); + } + } + + if (entry.owner) { + var ownerSpan = document.createElement('span'); + ownerSpan.textContent = 'by ' + entry.owner; + meta.appendChild(ownerSpan); + } + + if (entry.stars != null) { + addMetaSep(); + var starsSpan = document.createElement('span'); + starsSpan.textContent = entry.stars + ' stars'; + meta.appendChild(starsSpan); + } + + if (entry.downloads != null) { + addMetaSep(); + var dlSpan = document.createElement('span'); + dlSpan.textContent = formatCompactNumber(entry.downloads) + ' downloads'; + meta.appendChild(dlSpan); + } + + if (entry.updatedAt) { + var ago = formatTimeAgo(entry.updatedAt); + if (ago) { + addMetaSep(); + var updatedSpan = document.createElement('span'); + updatedSpan.textContent = 'updated ' + ago; + meta.appendChild(updatedSpan); + } + } + + if (meta.children.length > 0) { + card.appendChild(meta); + } + + var actions = document.createElement('div'); + actions.className = 'ext-actions'; + + var slug = entry.slug || entry.name; + var isInstalled = installedNames[entry.name] || installedNames[slug]; + + if (isInstalled) { + var label = document.createElement('span'); + label.className = 'ext-active-label'; + label.textContent = 'Installed'; + actions.appendChild(label); + } else { + var installBtn = document.createElement('button'); + installBtn.className = 'btn-ext install'; + installBtn.textContent = 'Install'; + installBtn.addEventListener('click', (function(s, btn) { + return function() { + if (!confirm('Install skill "' + s + '" from ClawHub?')) return; + btn.disabled = true; + btn.textContent = 'Installing...'; + installSkill(s, null, btn); + }; + })(slug, installBtn)); + actions.appendChild(installBtn); + } + + card.appendChild(actions); + return card; +} + +function formatCompactNumber(n) { + if (n >= 1000000) return (n / 1000000).toFixed(1) + 'M'; + if (n >= 1000) return (n / 1000).toFixed(1) + 'K'; + return '' + n; +} + +function formatTimeAgo(epochMs) { + var now = Date.now(); + var diff = now - epochMs; + if (diff < 0) return null; + var minutes = Math.floor(diff / 60000); + if (minutes < 60) return minutes <= 1 ? 'just now' : minutes + 'm ago'; + var hours = Math.floor(minutes / 60); + if (hours < 24) return hours + 'h ago'; + var days = Math.floor(hours / 24); + if (days < 30) return days + 'd ago'; + var months = Math.floor(days / 30); + if (months < 12) return months + 'mo ago'; + return Math.floor(months / 12) + 'y ago'; +} + +function installSkill(nameOrSlug, url, btn) { + var body = { name: nameOrSlug }; + if (url) body.url = url; + + apiFetch('/api/skills/install', { + method: 'POST', + headers: { 'X-Confirm-Action': 'true' }, + body: body, + }).then(function(res) { + if (res.success) { + showToast('Installed skill "' + nameOrSlug + '"', 'success'); + } else { + showToast('Install failed: ' + (res.message || 'unknown error'), 'error'); + } + loadSkills(); + if (btn) { btn.disabled = false; btn.textContent = 'Install'; } + }).catch(function(err) { + showToast('Install failed: ' + err.message, 'error'); + if (btn) { btn.disabled = false; btn.textContent = 'Install'; } + }); +} + +function removeSkill(name) { + if (!confirm('Remove skill "' + name + '"?')) return; + apiFetch('/api/skills/' + encodeURIComponent(name), { + method: 'DELETE', + headers: { 'X-Confirm-Action': 'true' }, + }).then(function(res) { + if (res.success) { + showToast('Removed skill "' + name + '"', 'success'); + } else { + showToast('Remove failed: ' + (res.message || 'unknown error'), 'error'); + } + loadSkills(); + }).catch(function(err) { + showToast('Remove failed: ' + err.message, 'error'); + }); +} + +function installSkillFromForm() { + var name = document.getElementById('skill-install-name').value.trim(); + if (!name) { showToast('Skill name is required', 'error'); return; } + var url = document.getElementById('skill-install-url').value.trim() || null; + if (url && !url.startsWith('https://')) { + showToast('URL must use HTTPS', 'error'); + return; + } + if (!confirm('Install skill "' + name + '"?')) return; + installSkill(name, url, null); + document.getElementById('skill-install-name').value = ''; + document.getElementById('skill-install-url').value = ''; +} + +// Wire up Enter key on search input +document.getElementById('skill-search-input').addEventListener('keydown', function(e) { + if (e.key === 'Enter') searchClawHub(); +}); + // --- Keyboard shortcuts --- document.addEventListener('keydown', (e) => { @@ -2720,10 +3047,10 @@ document.addEventListener('keydown', (e) => { const tag = (e.target.tagName || '').toLowerCase(); const inInput = tag === 'input' || tag === 'textarea'; - // Mod+1-5: switch tabs - if (mod && e.key >= '1' && e.key <= '5') { + // Mod+1-6: switch tabs + if (mod && e.key >= '1' && e.key <= '6') { e.preventDefault(); - const tabs = ['chat', 'memory', 'jobs', 'routines', 'extensions']; + const tabs = ['chat', 'memory', 'jobs', 'routines', 'extensions', 'skills']; const idx = parseInt(e.key) - 1; if (tabs[idx]) switchTab(tabs[idx]); return; diff --git a/src/channels/web/static/index.html b/src/channels/web/static/index.html index 4a2ecd16..0b87d617 100644 --- a/src/channels/web/static/index.html +++ b/src/channels/web/static/index.html @@ -42,6 +42,7 @@ +
+ + +
+
+
+

Search ClawHub

+
+ + +
+
+
+
+

Installed Skills

+
+
Loading skills...
+
+
+
+

Install Skill by URL

+
+ + + +
+
+
+
diff --git a/src/channels/web/static/style.css b/src/channels/web/static/style.css index 798505eb..87cf6e48 100644 --- a/src/channels/web/static/style.css +++ b/src/channels/web/static/style.css @@ -2809,6 +2809,82 @@ mark { transform: scale(0.98); } +/* --- Skills tab --- */ + +.skill-search-box { + display: flex; + gap: 8px; + align-items: center; + margin-bottom: 12px; +} + +.skill-search-box input { + flex: 1; + padding: 8px 12px; + background: var(--bg); + border: 1px solid var(--border); + border-radius: var(--radius); + color: var(--text); + font-size: 13px; +} + +.skill-search-box input:focus { + outline: none; + border-color: var(--accent); + box-shadow: 0 0 0 3px rgba(52, 211, 153, 0.1); +} + +.skill-search-box button { + padding: 8px 20px; + background: var(--accent); + color: #09090b; + border: none; + border-radius: var(--radius); + cursor: pointer; + font-size: 13px; + font-weight: 600; + transition: background 0.2s, transform 0.2s; +} + +.skill-search-box button:hover { + background: var(--accent-hover); + transform: translateY(-1px); +} + +.skill-trust { + font-size: 10px; + padding: 2px 6px; + border-radius: 8px; + font-weight: 500; + text-transform: uppercase; + letter-spacing: 0.3px; +} + +.skill-trust.trust-trusted { + background: rgba(52, 211, 153, 0.15); + color: var(--success); +} + +.skill-trust.trust-installed { + background: rgba(96, 165, 250, 0.15); + color: #60a5fa; +} + +.skill-version { + font-size: 11px; + color: var(--text-secondary); + font-family: var(--font-mono); +} + +@keyframes skillFadeIn { + from { opacity: 0; transform: translateY(8px); } + to { opacity: 1; transform: translateY(0); } +} + +.skill-search-result { + animation: skillFadeIn 0.3s ease-out both; +} + /* --- Activity toolbar --- */ .activity-toolbar { diff --git a/src/channels/web/types.rs b/src/channels/web/types.rs index 28ac00e9..45af9924 100644 --- a/src/channels/web/types.rs +++ b/src/channels/web/types.rs @@ -508,6 +508,9 @@ pub struct SkillSearchResponse { pub catalog: Vec, pub installed: Vec, pub registry_url: String, + /// If the catalog registry was unreachable or errored, a human-readable message. + #[serde(skip_serializing_if = "Option::is_none")] + pub catalog_error: Option, } #[derive(Debug, Deserialize)] diff --git a/src/config/skills.rs b/src/config/skills.rs index e58e41b5..f6f742b0 100644 --- a/src/config/skills.rs +++ b/src/config/skills.rs @@ -8,8 +8,12 @@ use crate::error::ConfigError; pub struct SkillsConfig { /// Whether the skills system is enabled. pub enabled: bool, - /// Directory containing local skills (default: ~/.ironclaw/skills/). + /// Directory containing user-placed skills (default: ~/.ironclaw/skills/). + /// Skills here are loaded with `Trusted` trust level. pub local_dir: PathBuf, + /// Directory containing registry-installed skills (default: ~/.ironclaw/installed_skills/). + /// Skills here are loaded with `Installed` trust level and get read-only tool access. + pub installed_dir: PathBuf, /// Maximum number of skills that can be active simultaneously. pub max_active_skills: usize, /// Maximum total context tokens allocated to skill prompts. @@ -19,15 +23,16 @@ pub struct SkillsConfig { impl Default for SkillsConfig { fn default() -> Self { Self { - enabled: false, + enabled: true, local_dir: default_skills_dir(), + installed_dir: default_installed_skills_dir(), max_active_skills: 3, max_context_tokens: 4000, } } } -/// Get the default skills directory (~/.ironclaw/skills/). +/// Get the default user skills directory (~/.ironclaw/skills/). fn default_skills_dir() -> PathBuf { dirs::home_dir() .unwrap_or_else(|| PathBuf::from(".")) @@ -35,13 +40,24 @@ fn default_skills_dir() -> PathBuf { .join("skills") } +/// Get the default installed skills directory (~/.ironclaw/installed_skills/). +fn default_installed_skills_dir() -> PathBuf { + dirs::home_dir() + .unwrap_or_else(|| PathBuf::from(".")) + .join(".ironclaw") + .join("installed_skills") +} + impl SkillsConfig { pub(crate) fn resolve() -> Result { Ok(Self { - enabled: parse_bool_env("SKILLS_ENABLED", false)?, + enabled: parse_bool_env("SKILLS_ENABLED", true)?, local_dir: optional_env("SKILLS_DIR")? .map(PathBuf::from) .unwrap_or_else(default_skills_dir), + installed_dir: optional_env("SKILLS_INSTALLED_DIR")? + .map(PathBuf::from) + .unwrap_or_else(default_installed_skills_dir), max_active_skills: parse_optional_env("SKILLS_MAX_ACTIVE", 3)?, max_context_tokens: parse_optional_env("SKILLS_MAX_CONTEXT_TOKENS", 4000)?, }) diff --git a/src/main.rs b/src/main.rs index a7103c22..0e9a3b48 100644 --- a/src/main.rs +++ b/src/main.rs @@ -220,9 +220,35 @@ async fn main() -> anyhow::Result<()> { // ── Orchestrator / container job manager ──────────────────────────── + // Proactive Docker detection + let docker_status = if config.sandbox.enabled { + let detection = ironclaw::sandbox::check_docker().await; + match detection.status { + ironclaw::sandbox::DockerStatus::Available => { + tracing::info!("Docker is available"); + } + ironclaw::sandbox::DockerStatus::NotInstalled => { + tracing::warn!( + "Docker is not installed -- sandbox disabled for this session. {}", + detection.platform.install_hint() + ); + } + ironclaw::sandbox::DockerStatus::NotRunning => { + tracing::warn!( + "Docker is installed but not running -- sandbox disabled for this session. {}", + detection.platform.start_hint() + ); + } + ironclaw::sandbox::DockerStatus::Disabled => {} + } + detection.status + } else { + ironclaw::sandbox::DockerStatus::Disabled + }; + let job_event_tx: Option< tokio::sync::broadcast::Sender<(uuid::Uuid, ironclaw::channels::web::types::SseEvent)>, - > = if config.sandbox.enabled { + > = if config.sandbox.enabled && docker_status.is_ok() { let (tx, _) = tokio::sync::broadcast::channel(256); Some(tx) } else { @@ -233,51 +259,52 @@ async fn main() -> anyhow::Result<()> { std::collections::VecDeque, >::new())); - let container_job_manager: Option> = if config.sandbox.enabled { - let token_store = TokenStore::new(); - let job_config = ContainerJobConfig { - image: config.sandbox.image.clone(), - memory_limit_mb: config.sandbox.memory_limit_mb, - cpu_shares: config.sandbox.cpu_shares, - orchestrator_port: 50051, - claude_code_api_key: std::env::var("ANTHROPIC_API_KEY").ok(), - claude_code_oauth_token: ironclaw::config::ClaudeCodeConfig::extract_oauth_token(), - claude_code_model: config.claude_code.model.clone(), - claude_code_max_turns: config.claude_code.max_turns, - claude_code_memory_limit_mb: config.claude_code.memory_limit_mb, - claude_code_allowed_tools: config.claude_code.allowed_tools.clone(), - }; - let jm = Arc::new(ContainerJobManager::new(job_config, token_store.clone())); + let container_job_manager: Option> = + if config.sandbox.enabled && docker_status.is_ok() { + let token_store = TokenStore::new(); + let job_config = ContainerJobConfig { + image: config.sandbox.image.clone(), + memory_limit_mb: config.sandbox.memory_limit_mb, + cpu_shares: config.sandbox.cpu_shares, + orchestrator_port: 50051, + claude_code_api_key: std::env::var("ANTHROPIC_API_KEY").ok(), + claude_code_oauth_token: ironclaw::config::ClaudeCodeConfig::extract_oauth_token(), + claude_code_model: config.claude_code.model.clone(), + claude_code_max_turns: config.claude_code.max_turns, + claude_code_memory_limit_mb: config.claude_code.memory_limit_mb, + claude_code_allowed_tools: config.claude_code.allowed_tools.clone(), + }; + let jm = Arc::new(ContainerJobManager::new(job_config, token_store.clone())); - // Start the orchestrator internal API in the background - let orchestrator_state = OrchestratorState { - llm: components.llm.clone(), - job_manager: Arc::clone(&jm), - token_store, - job_event_tx: job_event_tx.clone(), - prompt_queue: Arc::clone(&prompt_queue), - store: components.db.clone(), - secrets_store: components.secrets_store.clone(), - user_id: "default".to_string(), - }; + // Start the orchestrator internal API in the background + let orchestrator_state = OrchestratorState { + llm: components.llm.clone(), + job_manager: Arc::clone(&jm), + token_store, + job_event_tx: job_event_tx.clone(), + prompt_queue: Arc::clone(&prompt_queue), + store: components.db.clone(), + secrets_store: components.secrets_store.clone(), + user_id: "default".to_string(), + }; - tokio::spawn(async move { - if let Err(e) = OrchestratorApi::start(orchestrator_state, 50051).await { - tracing::error!("Orchestrator API failed: {}", e); + tokio::spawn(async move { + if let Err(e) = OrchestratorApi::start(orchestrator_state, 50051).await { + tracing::error!("Orchestrator API failed: {}", e); + } + }); + + if config.claude_code.enabled { + tracing::info!( + "Claude Code sandbox mode available (model: {}, max_turns: {})", + config.claude_code.model, + config.claude_code.max_turns + ); } - }); - - if config.claude_code.enabled { - tracing::info!( - "Claude Code sandbox mode available (model: {}, max_turns: {})", - config.claude_code.model, - config.claude_code.max_turns - ); - } - Some(jm) - } else { - None - }; + Some(jm) + } else { + None + }; // ── Channel setup ────────────────────────────────────────────────── @@ -517,8 +544,10 @@ async fn main() -> anyhow::Result<()> { heartbeat_enabled: config.heartbeat.enabled, heartbeat_interval_secs: config.heartbeat.interval_secs, sandbox_enabled: config.sandbox.enabled, + docker_status, claude_code_enabled: config.claude_code.enabled, routines_enabled: config.routines.enabled, + skills_enabled: config.skills.enabled, channels: channel_names, tunnel_url: active_tunnel .as_ref() @@ -558,6 +587,7 @@ async fn main() -> anyhow::Result<()> { workspace: components.workspace, extension_manager: components.extension_manager, skill_registry: components.skill_registry, + skill_catalog: components.skill_catalog, skills_config: config.skills.clone(), hooks: components.hooks, cost_guard: components.cost_guard, diff --git a/src/sandbox/config.rs b/src/sandbox/config.rs index 3ebeb96e..76356a3c 100644 --- a/src/sandbox/config.rs +++ b/src/sandbox/config.rs @@ -28,7 +28,7 @@ pub struct SandboxConfig { impl Default for SandboxConfig { fn default() -> Self { Self { - enabled: false, // Disabled by default until Docker is confirmed available + enabled: true, // Startup check disables gracefully if Docker unavailable policy: SandboxPolicy::ReadOnly, timeout: Duration::from_secs(120), memory_limit_mb: 2048, diff --git a/src/sandbox/container.rs b/src/sandbox/container.rs index 2160d450..196764fc 100644 --- a/src/sandbox/container.rs +++ b/src/sandbox/container.rs @@ -26,7 +26,7 @@ //! ``` use std::collections::HashMap; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::time::Duration; use bollard::Docker; @@ -490,40 +490,108 @@ impl ContainerRunner { /// /// Tries these locations in order: /// 1. `DOCKER_HOST` env var (bollard default) -/// 2. `/var/run/docker.sock` (Linux default) -/// 3. `~/.docker/run/docker.sock` (Docker Desktop on macOS) +/// 2. `/var/run/docker.sock` (Linux default; also used by OrbStack and Podman Desktop on macOS) +/// 3. `~/.docker/run/docker.sock` (Docker Desktop 4.13+ on macOS — primary user-owned socket) +/// 4. `~/.colima/default/docker.sock` (Colima — popular lightweight Docker Desktop alternative) +/// 5. `~/.rd/docker.sock` (Rancher Desktop on macOS) +/// 6. `$XDG_RUNTIME_DIR/docker.sock` (common rootless Docker socket on Linux) +/// 7. `/run/user/$UID/docker.sock` (rootless Docker fallback on Linux) pub async fn connect_docker() -> Result { - // First try bollard defaults (checks DOCKER_HOST, then /var/run/docker.sock) + // First try bollard defaults (checks DOCKER_HOST env var, then /var/run/docker.sock). + // This covers Linux, OrbStack (updates the /var/run symlink), and any user with + // DOCKER_HOST set to their runtime's socket. if let Ok(docker) = Docker::connect_with_local_defaults() && docker.ping().await.is_ok() { return Ok(docker); } - // Try Docker Desktop socket (macOS) - if let Some(home) = std::env::var_os("HOME") { - let desktop_sock = std::path::Path::new(&home).join(".docker/run/docker.sock"); - if desktop_sock.exists() { - let sock_str = desktop_sock.to_string_lossy(); - if let Ok(docker) = - Docker::connect_with_socket(&sock_str, 120, bollard::API_DEFAULT_VERSION) - && docker.ping().await.is_ok() - { - return Ok(docker); + #[cfg(unix)] + { + // Try well-known user-owned socket locations for desktop and rootless runtimes. + // Docker Desktop 4.13+ (stabilised in 4.18) stopped creating the + // /var/run/docker.sock symlink by default and moved the API socket + // to ~/.docker/run/docker.sock. + for sock in unix_socket_candidates() { + if sock.exists() { + let sock_str = sock.to_string_lossy(); + if let Ok(docker) = + Docker::connect_with_socket(&sock_str, 120, bollard::API_DEFAULT_VERSION) + && docker.ping().await.is_ok() + { + return Ok(docker); + } } } } Err(SandboxError::DockerNotAvailable { - reason: "Could not connect to Docker. Tried: default socket, ~/.docker/run/docker.sock" + reason: "Could not connect to Docker daemon. Tried: $DOCKER_HOST, \ + /var/run/docker.sock, ~/.docker/run/docker.sock, \ + ~/.colima/default/docker.sock, ~/.rd/docker.sock, \ + $XDG_RUNTIME_DIR/docker.sock, /run/user/$UID/docker.sock" .to_string(), }) } +#[cfg(unix)] +fn unix_socket_candidates() -> Vec { + unix_socket_candidates_from_env( + std::env::var_os("HOME").map(PathBuf::from), + std::env::var_os("XDG_RUNTIME_DIR").map(PathBuf::from), + std::env::var("UID").ok(), + ) +} + +#[cfg(unix)] +fn unix_socket_candidates_from_env( + home: Option, + xdg_runtime_dir: Option, + uid: Option, +) -> Vec { + let mut candidates = Vec::new(); + let mut push_unique = |path: PathBuf| { + if !candidates.iter().any(|existing| existing == &path) { + candidates.push(path); + } + }; + + if let Some(home) = home { + push_unique(home.join(".docker/run/docker.sock")); // Docker Desktop 4.13+ + push_unique(home.join(".colima/default/docker.sock")); // Colima + push_unique(home.join(".rd/docker.sock")); // Rancher Desktop + } + + if let Some(xdg_runtime_dir) = xdg_runtime_dir { + push_unique(xdg_runtime_dir.join("docker.sock")); + } + + if let Some(uid) = uid.filter(|value| !value.is_empty()) { + push_unique(PathBuf::from(format!("/run/user/{uid}/docker.sock"))); + } + + candidates +} + #[cfg(test)] mod tests { use super::*; + #[cfg(unix)] + #[test] + fn test_unix_socket_candidates_include_rootless_paths() { + let candidates = unix_socket_candidates_from_env( + Some(PathBuf::from("/home/tester")), + Some(PathBuf::from("/run/user/1000")), + Some("1000".to_string()), + ); + + assert!(candidates.contains(&PathBuf::from("/home/tester/.docker/run/docker.sock"))); + assert!(candidates.contains(&PathBuf::from("/home/tester/.colima/default/docker.sock"))); + assert!(candidates.contains(&PathBuf::from("/home/tester/.rd/docker.sock"))); + assert!(candidates.contains(&PathBuf::from("/run/user/1000/docker.sock"))); + } + #[tokio::test] async fn test_docker_connection() { // This test requires Docker to be running diff --git a/src/sandbox/detect.rs b/src/sandbox/detect.rs new file mode 100644 index 00000000..36481ebb --- /dev/null +++ b/src/sandbox/detect.rs @@ -0,0 +1,233 @@ +//! Proactive Docker detection with platform-specific guidance. +//! +//! Checks whether Docker is both installed (binary on PATH) and running +//! (daemon responding to ping), and provides platform-appropriate +//! installation or startup instructions when it is not. +//! +//! # Detection Limitations +//! +//! - **macOS**: High confidence. Detects both standard Docker Desktop socket +//! (`~/.docker/run/docker.sock`) and the default `/var/run/docker.sock`. +//! +//! - **Linux**: High confidence for standard installs. Rootless Docker uses +//! a different socket path (`/run/user/$UID/docker.sock`) which is now +//! checked by the fallback in `connect_docker()`. If `DOCKER_HOST` is set, +//! bollard's default connection still takes precedence. +//! +//! - **Windows**: Medium confidence. Binary detection uses `where.exe` which +//! works reliably. Daemon detection relies on bollard's default named pipe +//! connection (`//./pipe/docker_engine`) which works with Docker Desktop. +//! The Unix socket fallback in `connect_docker()` is a no-op on Windows, +//! so detection also probes `docker version`/`docker info` via CLI if the +//! named pipe is unavailable. + +/// Docker daemon availability status. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DockerStatus { + /// Docker binary found on PATH and daemon responding to ping. + Available, + /// `docker` binary not found on PATH. + NotInstalled, + /// Binary found but daemon not responding. + NotRunning, + /// Sandbox feature not enabled (no check performed). + Disabled, +} + +impl DockerStatus { + /// Returns true if Docker is available and ready. + pub fn is_ok(&self) -> bool { + matches!(self, DockerStatus::Available) + } + + /// Human-readable status string. + pub fn as_str(&self) -> &'static str { + match self { + DockerStatus::Available => "available", + DockerStatus::NotInstalled => "not installed", + DockerStatus::NotRunning => "not running", + DockerStatus::Disabled => "disabled", + } + } +} + +/// Host platform for install guidance. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Platform { + MacOS, + Linux, + Windows, +} + +impl Platform { + /// Detect the current platform. + pub fn current() -> Self { + match std::env::consts::OS { + "macos" => Platform::MacOS, + "windows" => Platform::Windows, + _ => Platform::Linux, + } + } + + /// Installation instructions for Docker on this platform. + pub fn install_hint(&self) -> &'static str { + match self { + Platform::MacOS => { + "Install Docker Desktop: https://docs.docker.com/desktop/install/mac-install/" + } + Platform::Linux => "Install Docker Engine: https://docs.docker.com/engine/install/", + Platform::Windows => { + "Install Docker Desktop: https://docs.docker.com/desktop/install/windows-install/" + } + } + } + + /// 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::Linux => "Start the Docker daemon: sudo systemctl start docker", + Platform::Windows => "Start Docker Desktop from the Start menu", + } + } +} + +/// Result of a Docker detection check. +pub struct DockerDetection { + pub status: DockerStatus, + pub platform: Platform, +} + +/// Check whether Docker is installed and running. +/// +/// 1. Checks if `docker` binary exists on PATH +/// 2. If found, tries to connect and ping the Docker daemon via `connect_docker()` +/// 3. Returns `Available`, `NotInstalled`, or `NotRunning` +pub async fn check_docker() -> DockerDetection { + let platform = Platform::current(); + + // Step 1: Check if docker binary is on PATH + if !docker_binary_exists() { + return DockerDetection { + status: DockerStatus::NotInstalled, + platform, + }; + } + + // Step 2: Try to connect to the daemon + if crate::sandbox::connect_docker().await.is_ok() { + return DockerDetection { + status: DockerStatus::Available, + platform, + }; + } + + // Windows fallback: if the named pipe probe fails but docker CLI can still + // reach the daemon/server, treat Docker as available. + #[cfg(windows)] + if docker_cli_daemon_reachable() { + return DockerDetection { + status: DockerStatus::Available, + platform, + }; + } + + DockerDetection { + status: DockerStatus::NotRunning, + platform, + } +} + +/// Check if the `docker` binary exists on PATH. +fn docker_binary_exists() -> bool { + #[cfg(unix)] + { + std::process::Command::new("which") + .arg("docker") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .is_ok_and(|s| s.success()) + } + #[cfg(windows)] + { + std::process::Command::new("where") + .arg("docker") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .is_ok_and(|s| s.success()) + } +} + +#[cfg(windows)] +fn docker_cli_daemon_reachable() -> bool { + let stdout = std::process::Stdio::null(); + let stderr = std::process::Stdio::null(); + + // `docker version` requires daemon reachability for server fields. + let version_ok = std::process::Command::new("docker") + .args(["version", "--format", "{{.Server.Version}}"]) + .stdout(stdout) + .stderr(stderr) + .status() + .is_ok_and(|s| s.success()); + + if version_ok { + return true; + } + + // Fallback for environments where `docker version --format` behaves differently. + std::process::Command::new("docker") + .args(["info", "--format", "{{.ServerVersion}}"]) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .is_ok_and(|s| s.success()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_detect_platform() { + let platform = Platform::current(); + match platform { + Platform::MacOS | Platform::Linux | Platform::Windows => {} + } + } + + #[test] + fn test_install_hint_not_empty() { + for platform in [Platform::MacOS, Platform::Linux, Platform::Windows] { + assert!(!platform.install_hint().is_empty()); + assert!(!platform.start_hint().is_empty()); + } + } + + #[test] + fn test_docker_status_display() { + assert_eq!(DockerStatus::Available.as_str(), "available"); + assert_eq!(DockerStatus::NotInstalled.as_str(), "not installed"); + assert_eq!(DockerStatus::NotRunning.as_str(), "not running"); + assert_eq!(DockerStatus::Disabled.as_str(), "disabled"); + } + + #[test] + fn test_docker_status_is_ok() { + assert!(DockerStatus::Available.is_ok()); + assert!(!DockerStatus::NotInstalled.is_ok()); + assert!(!DockerStatus::NotRunning.is_ok()); + assert!(!DockerStatus::Disabled.is_ok()); + } + + #[tokio::test] + async fn test_check_docker_returns_valid_status() { + let result = check_docker().await; + match result.status { + DockerStatus::Available | DockerStatus::NotInstalled | DockerStatus::NotRunning => {} + DockerStatus::Disabled => panic!("check_docker should never return Disabled"), + } + } +} diff --git a/src/sandbox/manager.rs b/src/sandbox/manager.rs index 9da7a22e..d2821f28 100644 --- a/src/sandbox/manager.rs +++ b/src/sandbox/manager.rs @@ -460,7 +460,7 @@ mod tests { #[test] fn test_builder_defaults() { let manager = SandboxManagerBuilder::new().build(); - assert!(!manager.config.enabled); // Disabled by default + assert!(manager.config.enabled); // Enabled by default (startup check disables if Docker unavailable) } #[test] diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index 73f553e7..caf24cad 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -87,12 +87,14 @@ pub mod config; pub mod container; +pub mod detect; pub mod error; pub mod manager; pub mod proxy; pub use config::{ResourceLimits, SandboxConfig, SandboxPolicy}; pub use container::{ContainerOutput, ContainerRunner, connect_docker}; +pub use detect::{DockerDetection, DockerStatus, Platform, check_docker}; pub use error::{Result, SandboxError}; pub use manager::{ExecOutput, SandboxManager, SandboxManagerBuilder}; pub use proxy::{ diff --git a/src/setup/wizard.rs b/src/setup/wizard.rs index 2000e524..0e3e6202 100644 --- a/src/setup/wizard.rs +++ b/src/setup/wizard.rs @@ -8,7 +8,8 @@ //! 5. Embeddings //! 6. Channel configuration //! 7. Extensions (tool installation from registry) -//! 8. Heartbeat (background tasks) +//! 8. Docker sandbox +//! 9. Heartbeat (background tasks) use std::collections::{HashMap, HashSet}; use std::sync::Arc; @@ -140,7 +141,7 @@ impl SetupWizard { print_step(1, 1, "Channel Configuration"); self.step_channels().await?; } else { - let total_steps = 8; + let total_steps = 9; // Step 1: Database print_step(1, total_steps, "Database Connection"); @@ -191,8 +192,13 @@ impl SetupWizard { print_step(7, total_steps, "Extensions"); self.step_extensions().await?; - // Step 8: Heartbeat - print_step(8, total_steps, "Background Tasks"); + // Step 8: Docker Sandbox + print_step(8, total_steps, "Docker Sandbox"); + self.step_docker_sandbox().await?; + self.persist_after_step().await; + + // Step 9: Heartbeat + print_step(9, total_steps, "Background Tasks"); self.step_heartbeat()?; self.persist_after_step().await; } @@ -1722,7 +1728,85 @@ impl SetupWizard { Ok(()) } - /// Step 8: Heartbeat configuration. + /// Step 8: Docker Sandbox -- check Docker installation and availability. + async fn step_docker_sandbox(&mut self) -> Result<(), SetupError> { + print_info("IronClaw can execute code, run builds, and use tools inside Docker"); + print_info("containers. This keeps your system safe -- commands from the LLM run"); + print_info("in an isolated sandbox with no access to your credentials, limited"); + print_info("filesystem access, and network traffic restricted to an allowlist."); + println!(); + print_info("Without Docker, code execution tools (shell, file write) run directly"); + print_info("on your machine with no isolation."); + println!(); + + if !confirm("Enable Docker sandbox?", false).map_err(SetupError::Io)? { + self.settings.sandbox.enabled = false; + print_info("Sandbox disabled. You can enable it later with SANDBOX_ENABLED=true."); + return Ok(()); + } + + // Check Docker availability + let detection = crate::sandbox::detect::check_docker().await; + + match detection.status { + crate::sandbox::detect::DockerStatus::Available => { + self.settings.sandbox.enabled = true; + print_success("Docker is installed and running. Sandbox enabled."); + } + crate::sandbox::detect::DockerStatus::NotInstalled + | crate::sandbox::detect::DockerStatus::NotRunning => { + println!(); + let not_installed = + detection.status == crate::sandbox::detect::DockerStatus::NotInstalled; + if not_installed { + print_error("Docker is not installed."); + print_info(detection.platform.install_hint()); + } else { + print_error("Docker is installed but not running."); + print_info(detection.platform.start_hint()); + } + println!(); + + let retry_prompt = if not_installed { + "Retry after installing Docker?" + } else { + "Retry after starting Docker?" + }; + if confirm(retry_prompt, false).map_err(SetupError::Io)? { + let retry = crate::sandbox::detect::check_docker().await; + if retry.status.is_ok() { + self.settings.sandbox.enabled = true; + print_success(if not_installed { + "Docker is now available. Sandbox enabled." + } else { + "Docker is now running. Sandbox enabled." + }); + } else { + self.settings.sandbox.enabled = false; + print_info(if not_installed { + "Docker still not available. Sandbox disabled for now." + } else { + "Docker still not responding. Sandbox disabled for now." + }); + } + } else { + self.settings.sandbox.enabled = false; + print_info(if not_installed { + "Sandbox disabled. Install Docker and set SANDBOX_ENABLED=true later." + } else { + "Sandbox disabled. Start Docker and set SANDBOX_ENABLED=true later." + }); + } + } + crate::sandbox::detect::DockerStatus::Disabled => { + self.settings.sandbox.enabled = false; + } + } + + Ok(()) + } + + /// Step 9: Heartbeat configuration. fn step_heartbeat(&mut self) -> Result<(), SetupError> { print_info("Heartbeat runs periodic background tasks (e.g., checking your calendar,"); print_info("monitoring for notifications, running scheduled workflows)."); diff --git a/src/skills/catalog.rs b/src/skills/catalog.rs index 76c8b971..2a2b69dc 100644 --- a/src/skills/catalog.rs +++ b/src/skills/catalog.rs @@ -5,7 +5,7 @@ //! up-to-date with the registry. //! //! Configuration: -//! - `CLAWHUB_REGISTRY` env var overrides the default base URL (`https://clawhub.ai`) +//! - `CLAWHUB_REGISTRY` env var overrides the default base URL use std::sync::Arc; use std::time::{Duration, Instant}; @@ -14,7 +14,10 @@ use serde::{Deserialize, Serialize}; use tokio::sync::RwLock; /// Default ClawHub registry URL. -const DEFAULT_REGISTRY_URL: &str = "https://clawhub.ai"; +/// +/// Points directly at the Convex backend, bypassing Vercel's edge which +/// rejects non-browser TLS fingerprints (JA3/JA4 filtering). +const DEFAULT_REGISTRY_URL: &str = "https://wry-manatee-359.convex.site"; /// How long cached search results remain valid (5 minutes). const CACHE_TTL: Duration = Duration::from_secs(300); @@ -25,6 +28,15 @@ const MAX_RESULTS: usize = 25; /// HTTP request timeout for catalog queries. const REQUEST_TIMEOUT: Duration = Duration::from_secs(10); +/// Result of a catalog search, carrying both results and any error that occurred. +#[derive(Debug, Clone)] +pub struct CatalogSearchOutcome { + /// Skill entries returned by the search (empty on error). + pub results: Vec, + /// If the registry was unreachable or returned an error, a human-readable message. + pub error: Option, +} + /// A skill entry from the ClawHub catalog. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct CatalogEntry { @@ -41,18 +53,102 @@ pub struct CatalogEntry { /// Relevance score from the search API. #[serde(default)] pub score: f64, + /// Last updated timestamp (epoch milliseconds from registry). + #[serde(default)] + pub updated_at: Option, + /// Star count (populated via detail enrichment). + #[serde(default)] + pub stars: Option, + /// Total download count (populated via detail enrichment). + #[serde(default)] + pub downloads: Option, + /// Current install count (populated via detail enrichment). + #[serde(default)] + pub installs_current: Option, + /// Owner handle (populated via detail enrichment). + #[serde(default)] + pub owner: Option, +} + +/// Top-level wrapper from the ClawHub `/api/v1/skills/{slug}` response. +/// +/// The API returns `{"skill": {...}, "owner": {...}, "latestVersion": {...}}`. +#[derive(Debug, Clone, Deserialize)] +struct SkillDetailResponse { + skill: SkillDetailInner, + #[serde(default)] + owner: Option, +} + +/// Inner `skill` object within `SkillDetailResponse`. +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +struct SkillDetailInner { + pub slug: String, + #[serde(default)] + pub display_name: Option, + #[serde(default)] + pub summary: Option, + #[serde(default)] + pub stats: Option, + #[serde(default)] + pub updated_at: Option, +} + +/// Detailed skill information from the ClawHub `/api/v1/skills/{slug}` endpoint. +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct SkillDetail { + pub slug: String, + #[serde(default)] + pub display_name: Option, + #[serde(default)] + pub summary: Option, + #[serde(default)] + pub version: Option, + #[serde(default)] + pub stats: Option, + #[serde(default)] + pub owner: Option, + #[serde(default)] + pub updated_at: Option, +} + +/// Statistics for a skill from the ClawHub detail endpoint. +#[derive(Debug, Clone, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct SkillStats { + #[serde(default)] + pub stars: Option, + #[serde(default)] + pub downloads: Option, + #[serde(default)] + pub installs_current: Option, + #[serde(default)] + pub installs_all_time: Option, + #[serde(default)] + pub versions: Option, +} + +/// Owner information for a skill. +#[derive(Debug, Clone, Deserialize)] +pub struct SkillOwner { + #[serde(default)] + pub handle: Option, + #[serde(default, rename = "displayName")] + pub display_name: Option, } /// Cached search result with TTL. struct CachedSearch { query: String, - results: Vec, + outcome: CatalogSearchOutcome, fetched_at: Instant, } /// Runtime skill catalog that queries ClawHub's API. pub struct SkillCatalog { - /// Base URL for the registry (e.g. `https://clawhub.ai`). + /// Base URL for the registry. registry_url: String, /// HTTP client (reused across requests). client: reqwest::Client, @@ -64,7 +160,7 @@ impl SkillCatalog { /// Create a new catalog. /// /// Reads `CLAWHUB_REGISTRY` (or legacy `CLAWDHUB_REGISTRY`) from the - /// environment, falling back to `https://clawhub.ai`. + /// environment, falling back to the Convex backend. pub fn new() -> Self { let registry_url = std::env::var("CLAWHUB_REGISTRY") .or_else(|_| std::env::var("CLAWDHUB_REGISTRY")) @@ -102,9 +198,10 @@ impl SkillCatalog { /// Search for skills in the catalog. /// /// First checks the in-memory cache. If not cached or expired, fetches - /// from the ClawHub API. Returns an empty Vec on network errors (catalog - /// search is best-effort, never blocks the agent). - pub async fn search(&self, query: &str) -> Vec { + /// from the ClawHub API. Returns a [`CatalogSearchOutcome`] that carries + /// both results and any error that occurred (catalog search is best-effort, + /// never blocks the agent). + pub async fn search(&self, query: &str) -> CatalogSearchOutcome { let query_lower = query.to_lowercase(); // Check cache @@ -113,12 +210,12 @@ impl SkillCatalog { if let Some(cached) = cache.iter().find(|c| c.query == query_lower) && cached.fetched_at.elapsed() < CACHE_TTL { - return cached.results.clone(); + return cached.outcome.clone(); } } // Fetch from API - let results = self.fetch_search(&query_lower).await; + let outcome = self.fetch_search(&query_lower).await; // Update cache { @@ -131,43 +228,75 @@ impl SkillCatalog { } cache.push(CachedSearch { query: query_lower, - results: results.clone(), + outcome: outcome.clone(), fetched_at: Instant::now(), }); } - results + outcome } /// Fetch search results from the ClawHub API. - async fn fetch_search(&self, query: &str) -> Vec { + async fn fetch_search(&self, query: &str) -> CatalogSearchOutcome { let url = format!("{}/api/v1/search", self.registry_url); let response = match self.client.get(&url).query(&[("q", query)]).send().await { Ok(resp) => resp, Err(e) => { - tracing::debug!("Catalog search failed (network): {}", e); - return Vec::new(); + tracing::warn!("Catalog search failed (network): {}", e); + return CatalogSearchOutcome { + results: Vec::new(), + error: Some("Registry unreachable".to_string()), + }; } }; if !response.status().is_success() { + let status = response.status(); tracing::debug!( "Catalog search returned status {}: {}", - response.status(), + status, response .text() .await .unwrap_or_else(|_| "(no body)".to_string()) ); - return Vec::new(); + return CatalogSearchOutcome { + results: Vec::new(), + error: Some(format!("Registry returned status {status}")), + }; } - // Parse the response -- ClawHub returns an array of results. - // We try the v1 format first (with slug, displayName, version, score), - // then fall back to a simpler format. - match response.json::>().await { - Ok(results) => results + // Parse the response body as text first so we can try multiple formats. + let body = match response.text().await { + Ok(b) => b, + Err(e) => { + tracing::debug!("Catalog search: failed to read response body: {}", e); + return CatalogSearchOutcome { + results: Vec::new(), + error: Some("Failed to read registry response".to_string()), + }; + } + }; + + // Try wrapped format first: {"results": [...]} + // Then fall back to bare array: [...] + let raw_results = if let Ok(envelope) = serde_json::from_str::(&body) + { + envelope.results + } else if let Ok(arr) = serde_json::from_str::>(&body) { + arr + } else { + let preview = body.get(..200).unwrap_or(&body); + tracing::debug!("Catalog search: failed to parse response: {}", preview); + return CatalogSearchOutcome { + results: Vec::new(), + error: Some("Invalid response from registry".to_string()), + }; + }; + + CatalogSearchOutcome { + results: raw_results .into_iter() .take(MAX_RESULTS) .map(|r| CatalogEntry { @@ -176,11 +305,78 @@ impl SkillCatalog { description: r.summary.unwrap_or_default(), version: r.version.unwrap_or_default(), score: r.score.unwrap_or(0.0), + updated_at: r.updated_at, + stars: None, + downloads: None, + installs_current: None, + owner: None, }) .collect(), - Err(e) => { - tracing::debug!("Catalog search: failed to parse response: {}", e); - Vec::new() + error: None, + } + } + + /// Fetch detailed information for a single skill by slug. + /// + /// Calls `GET /api/v1/skills/{slug}` and returns the detail if available. + /// Returns `None` on any network or parse error (best-effort). + pub async fn fetch_skill_detail(&self, slug: &str) -> Option { + let url = format!( + "{}/api/v1/skills/{}", + self.registry_url, + urlencoding::encode(slug) + ); + + let response = self.client.get(&url).send().await.ok()?; + if !response.status().is_success() { + tracing::debug!( + "Skill detail for '{}' returned status {}", + slug, + response.status() + ); + return None; + } + + let wrapper = response.json::().await.ok()?; + let inner = wrapper.skill; + Some(SkillDetail { + slug: inner.slug, + display_name: inner.display_name, + summary: inner.summary, + version: None, // not returned in detail response + stats: inner.stats, + owner: wrapper.owner, + updated_at: inner.updated_at, + }) + } + + /// Enrich catalog entries with detail data (stars, downloads, owner). + /// + /// Fetches detail for up to `max` entries in parallel. Best-effort: entries + /// that fail to enrich keep their `None` values. + pub async fn enrich_search_results(&self, entries: &mut [CatalogEntry], max: usize) { + let count = entries.len().min(max); + if count == 0 { + return; + } + + let futures: Vec<_> = entries[..count] + .iter() + .map(|e| self.fetch_skill_detail(&e.slug)) + .collect(); + + let details = futures::future::join_all(futures).await; + + for (entry, detail) in entries[..count].iter_mut().zip(details.into_iter()) { + if let Some(detail) = detail { + if let Some(ref stats) = detail.stats { + entry.stars = stats.stars; + entry.downloads = stats.downloads; + entry.installs_current = stats.installs_current; + } + if let Some(ref owner) = detail.owner { + entry.owner = owner.handle.clone().or_else(|| owner.display_name.clone()); + } } } } @@ -202,6 +398,12 @@ impl Default for SkillCatalog { } } +/// Wrapper for ClawHub's `{"results": [...]}` envelope. +#[derive(Debug, Deserialize)] +struct CatalogSearchEnvelope { + results: Vec, +} + /// Internal type matching ClawHub's `/api/v1/search` response items. #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] @@ -215,6 +417,8 @@ struct CatalogSearchResult { summary: Option, #[serde(default)] score: Option, + #[serde(default)] + updated_at: Option, } /// Construct the download URL for a skill's SKILL.md from the registry. @@ -252,11 +456,13 @@ mod tests { } #[tokio::test] - async fn test_search_returns_empty_on_network_error() { + async fn test_search_returns_error_on_network_failure() { // Point at an invalid URL to trigger a network error let catalog = SkillCatalog::with_url("http://127.0.0.1:1"); - let results = catalog.search("test").await; - assert!(results.is_empty()); + let outcome = catalog.search("test").await; + assert!(outcome.results.is_empty()); + assert!(outcome.error.is_some()); + assert!(outcome.error.unwrap().contains("Registry unreachable")); } #[tokio::test] @@ -295,6 +501,77 @@ mod tests { assert!(url.contains("slug=foo%26bar%3Dbaz%23frag")); } + #[test] + fn test_parse_wrapped_response() { + // ClawHub returns {"results": [...]} format + let json = r#"{"results":[{"slug":"markdown","displayName":"Markdown","summary":"A skill","version":"1.0.0","score":3.5}]}"#; + let envelope: CatalogSearchEnvelope = serde_json::from_str(json).unwrap(); + assert_eq!(envelope.results.len(), 1); + assert_eq!(envelope.results[0].slug, "markdown"); + assert_eq!( + envelope.results[0].display_name.as_deref(), + Some("Markdown") + ); + } + + #[test] + fn test_parse_bare_array_response() { + // Fallback: bare array format + let json = r#"[{"slug":"markdown","displayName":"Markdown","summary":"A skill","version":"1.0.0","score":3.5}]"#; + let results: Vec = serde_json::from_str(json).unwrap(); + assert_eq!(results.len(), 1); + assert_eq!(results[0].slug, "markdown"); + } + + #[test] + fn test_parse_skill_detail() { + // Response format matches the actual ClawHub API: {"skill": {...}, "owner": {...}} + let json = r#"{ + "skill": { + "slug": "steipete/markdown-writer", + "displayName": "Markdown Writer", + "summary": "Write markdown docs", + "stats": { + "stars": 142, + "downloads": 8400, + "installsCurrent": 55, + "installsAllTime": 200, + "versions": 5 + }, + "updatedAt": 1700000000000 + }, + "owner": { + "handle": "steipete", + "displayName": "Peter S." + }, + "latestVersion": { + "version": "1.2.3", + "createdAt": 1700000000000, + "changelog": "" + } + }"#; + + let wrapper: SkillDetailResponse = serde_json::from_str(json).unwrap(); + let inner = &wrapper.skill; + assert_eq!(inner.slug, "steipete/markdown-writer"); + assert_eq!(inner.display_name.as_deref(), Some("Markdown Writer")); + + let stats = inner.stats.as_ref().unwrap(); + assert_eq!(stats.stars, Some(142)); + assert_eq!(stats.downloads, Some(8400)); + assert_eq!(stats.installs_current, Some(55)); + + let owner = wrapper.owner.as_ref().unwrap(); + assert_eq!(owner.handle.as_deref(), Some("steipete")); + } + + #[tokio::test] + async fn test_fetch_skill_detail_returns_none_on_error() { + let catalog = SkillCatalog::with_url("http://127.0.0.1:1"); + let result = catalog.fetch_skill_detail("nonexistent/skill").await; + assert!(result.is_none()); + } + #[test] fn test_catalog_entry_serde() { let entry = CatalogEntry { @@ -303,6 +580,11 @@ mod tests { description: "A test".to_string(), version: "1.0.0".to_string(), score: 0.95, + updated_at: Some(1700000000000), + stars: Some(42), + downloads: Some(1000), + installs_current: None, + owner: Some("tester".to_string()), }; let json = serde_json::to_string(&entry).unwrap(); let parsed: CatalogEntry = serde_json::from_str(&json).unwrap(); diff --git a/src/skills/registry.rs b/src/skills/registry.rs index 6c5485d0..d5ad5385 100644 --- a/src/skills/registry.rs +++ b/src/skills/registry.rs @@ -68,8 +68,10 @@ pub enum SkillRegistryError { pub struct SkillRegistry { /// All loaded skills. skills: Vec, - /// User skills directory (~/.ironclaw/skills/). + /// User skills directory (~/.ironclaw/skills/). Skills here are Trusted. user_dir: PathBuf, + /// Registry-installed skills directory (~/.ironclaw/installed_skills/). Skills here are Installed. + installed_dir: Option, /// Optional workspace skills directory. workspace_dir: Option, } @@ -80,10 +82,22 @@ impl SkillRegistry { Self { skills: Vec::new(), user_dir, + installed_dir: None, workspace_dir: None, } } + /// Set the registry-installed skills directory. + /// + /// Skills installed via ClawHub or the skill tools are written here and + /// loaded with `SkillTrust::Installed` (read-only tool access). This + /// directory is separate from the user dir so that trust levels survive + /// restarts correctly. + pub fn with_installed_dir(mut self, dir: PathBuf) -> Self { + self.installed_dir = Some(dir); + self + } + /// Set a workspace skills directory. pub fn with_workspace_dir(mut self, dir: PathBuf) -> Self { self.workspace_dir = Some(dir); @@ -95,6 +109,7 @@ impl SkillRegistry { /// Discovery order (earlier wins on name collision): /// 1. Workspace skills directory (if set) -- Trusted /// 2. User skills directory -- Trusted + /// 3. Installed skills directory (if set) -- Installed pub async fn discover_all(&mut self) -> Vec { let mut loaded_names: Vec = Vec::new(); let mut seen: HashSet = HashSet::new(); @@ -129,6 +144,25 @@ impl SkillRegistry { self.skills.push(skill); } + // 3. Installed skills (registry-installed, lowest priority) + if let Some(inst_dir) = self.installed_dir.clone() { + let inst_skills = self + .discover_from_dir(&inst_dir, SkillTrust::Installed, SkillSource::User) + .await; + for (name, skill) in inst_skills { + if seen.contains(&name) { + tracing::debug!( + "Skipping installed skill '{}' (overridden by user/workspace)", + name + ); + continue; + } + seen.insert(name.clone()); + loaded_names.push(name); + self.skills.push(skill); + } + } + loaded_names } @@ -424,6 +458,20 @@ impl SkillRegistry { pub fn user_dir(&self) -> &Path { &self.user_dir } + + /// Get the installed skills directory path, if configured. + pub fn installed_dir(&self) -> Option<&Path> { + self.installed_dir.as_deref() + } + + /// Get the directory where new registry installs should be written. + /// + /// Returns the installed_dir if configured (preferred), otherwise falls + /// back to user_dir. In practice, the installed_dir is always set when + /// the app is running; the fallback exists for test registries. + pub fn install_target_dir(&self) -> &Path { + self.installed_dir.as_deref().unwrap_or(&self.user_dir) + } } /// Load and validate a single SKILL.md file from disk. @@ -948,4 +996,70 @@ mod tests { let h2 = compute_hash("world"); assert_ne!(h1, h2); } + + /// Skills in the installed_dir are discovered with SkillTrust::Installed, + /// not Trusted. This ensures registry-installed skills do not gain full + /// tool access after an agent restart. + #[tokio::test] + async fn test_installed_dir_uses_installed_trust() { + let user_dir = tempfile::tempdir().unwrap(); + let inst_dir = tempfile::tempdir().unwrap(); + + // Place a skill in the installed dir + let skill_dir = inst_dir.path().join("registry-skill"); + fs::create_dir(&skill_dir).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: registry-skill\nversion: \"1.2.3\"\n---\n\nInstalled prompt.\n", + ) + .unwrap(); + + let mut registry = SkillRegistry::new(user_dir.path().to_path_buf()) + .with_installed_dir(inst_dir.path().to_path_buf()); + let loaded = registry.discover_all().await; + + assert_eq!(loaded, vec!["registry-skill"]); + let skill = registry.find_by_name("registry-skill").unwrap(); + assert_eq!( + skill.trust, + SkillTrust::Installed, + "installed_dir skills must be Installed" + ); + assert_eq!(skill.manifest.version, "1.2.3"); + } + + /// install_target_dir() returns installed_dir when set, user_dir otherwise. + #[test] + fn test_install_target_dir_prefers_installed_dir() { + let user_dir = PathBuf::from("/tmp/user-skills"); + let inst_dir = PathBuf::from("/tmp/installed-skills"); + + let registry = SkillRegistry::new(user_dir.clone()).with_installed_dir(inst_dir.clone()); + assert_eq!(registry.install_target_dir(), inst_dir.as_path()); + + let registry_no_inst = SkillRegistry::new(user_dir.clone()); + assert_eq!(registry_no_inst.install_target_dir(), user_dir.as_path()); + } + + /// User skills (user_dir) remain Trusted even when installed_dir is set. + #[tokio::test] + async fn test_user_dir_stays_trusted_with_installed_dir() { + let user_dir = tempfile::tempdir().unwrap(); + let inst_dir = tempfile::tempdir().unwrap(); + + let skill_dir = user_dir.path().join("my-skill"); + fs::create_dir(&skill_dir).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: my-skill\n---\n\nUser prompt.\n", + ) + .unwrap(); + + let mut registry = SkillRegistry::new(user_dir.path().to_path_buf()) + .with_installed_dir(inst_dir.path().to_path_buf()); + registry.discover_all().await; + + let skill = registry.find_by_name("my-skill").unwrap(); + assert_eq!(skill.trust, SkillTrust::Trusted); + } } diff --git a/src/testing.rs b/src/testing.rs index 99e7f9dd..0e287b3b 100644 --- a/src/testing.rs +++ b/src/testing.rs @@ -289,6 +289,7 @@ impl TestHarnessBuilder { workspace: None, extension_manager: None, skill_registry: None, + skill_catalog: None, skills_config: SkillsConfig::default(), hooks, cost_guard, diff --git a/src/tools/builtin/skill_tools.rs b/src/tools/builtin/skill_tools.rs index 64d7874e..65948d27 100644 --- a/src/tools/builtin/skill_tools.rs +++ b/src/tools/builtin/skill_tools.rs @@ -155,7 +155,14 @@ impl Tool for SkillSearchTool { let query = require_str(¶ms, "query")?; // Search the ClawHub catalog (async, best-effort) - let catalog_results = self.catalog.search(query).await; + let catalog_outcome = self.catalog.search(query).await; + let catalog_error = catalog_outcome.error.clone(); + + // Enrich top results with detail data (stars, downloads, owner) + let mut catalog_entries = catalog_outcome.results; + self.catalog + .enrich_search_results(&mut catalog_entries, 5) + .await; // Search locally loaded skills let installed_names: Vec = { @@ -171,7 +178,7 @@ impl Tool for SkillSearchTool { }; // Mark catalog entries that are already installed - let catalog_json: Vec = catalog_results + let catalog_json: Vec = catalog_entries .iter() .map(|entry| { let is_installed = installed_names.iter().any(|n| { @@ -185,6 +192,9 @@ impl Tool for SkillSearchTool { "version": entry.version, "score": entry.score, "installed": is_installed, + "stars": entry.stars, + "downloads": entry.downloads, + "owner": entry.owner, }) }) .collect(); @@ -218,13 +228,16 @@ impl Tool for SkillSearchTool { .collect() }; - let output = serde_json::json!({ + let mut output = serde_json::json!({ "catalog": catalog_json, "catalog_count": catalog_json.len(), "installed": local_matches, "installed_count": local_matches.len(), "registry_url": self.catalog.registry_url(), }); + if let Some(err) = catalog_error { + output["catalog_error"] = serde_json::Value::String(err); + } Ok(ToolOutput::success(output, start.elapsed())) } @@ -298,7 +311,7 @@ impl Tool for SkillInstallTool { fetch_skill_content(&download_url).await? }; - // Check for duplicates and get user_dir under a brief read lock. + // Check for duplicates and get install_dir under a brief read lock. let (user_dir, skill_name_from_parse) = { let guard = self .registry @@ -318,7 +331,7 @@ impl Tool for SkillInstallTool { ))); } - (guard.user_dir().to_path_buf(), skill_name) + (guard.install_target_dir().to_path_buf(), skill_name) }; // Perform async I/O (write to disk, validate round-trip) with no lock held. @@ -383,14 +396,23 @@ pub fn validate_fetch_url(url_str: &str) -> Result<(), ToolError> { .host_str() .ok_or_else(|| ToolError::ExecutionFailed("URL has no host".to_string()))?; - // Check if host is an IP address and reject private ranges - if let Ok(ip) = host.parse::() - && (ip.is_loopback() || ip.is_unspecified() || is_private_ip(&ip) || is_link_local_ip(&ip)) - { - return Err(ToolError::ExecutionFailed(format!( - "URL points to a private/loopback/link-local address: {}", - host - ))); + // Check if host is an IP address and reject private ranges. + // Unwrap IPv4-mapped IPv6 addresses (e.g. ::ffff:192.168.1.1) to catch + // SSRF bypasses that encode private IPv4 addresses as IPv6. + if let Ok(raw_ip) = host.parse::() { + let ip = match raw_ip { + std::net::IpAddr::V6(v6) => v6 + .to_ipv4_mapped() + .map(std::net::IpAddr::V4) + .unwrap_or(std::net::IpAddr::V6(v6)), + other => other, + }; + if ip.is_loopback() || ip.is_unspecified() || is_private_ip(&ip) || is_link_local_ip(&ip) { + return Err(ToolError::ExecutionFailed(format!( + "URL points to a private/loopback/link-local address: {}", + host + ))); + } } // Reject common internal hostnames @@ -435,6 +457,11 @@ fn is_link_local_ip(ip: &std::net::IpAddr) -> bool { } /// Fetch SKILL.md content from a URL with SSRF protection. +/// +/// The ClawHub registry returns skill downloads as ZIP archives containing +/// `SKILL.md` and `_meta.json`. This function detects ZIP responses (by the +/// `PK\x03\x04` magic bytes) and extracts `SKILL.md` automatically. Plain +/// text responses are returned as-is. pub async fn fetch_skill_content(url: &str) -> Result { validate_fetch_url(url)?; @@ -457,10 +484,28 @@ pub async fn fetch_skill_content(url: &str) -> Result { ))); } - let content = response - .text() + // Limit download size to prevent memory exhaustion from large responses. + const MAX_DOWNLOAD_BYTES: usize = 10 * 1024 * 1024; // 10 MB + let bytes = response + .bytes() .await .map_err(|e| ToolError::ExecutionFailed(format!("Failed to read response body: {}", e)))?; + if bytes.len() > MAX_DOWNLOAD_BYTES { + return Err(ToolError::ExecutionFailed(format!( + "Response too large: {} bytes (max {} bytes)", + bytes.len(), + MAX_DOWNLOAD_BYTES + ))); + } + + // Detect ZIP archive (PK\x03\x04 magic) and extract SKILL.md + let content = if bytes.starts_with(b"PK\x03\x04") { + extract_skill_from_zip(&bytes)? + } else { + String::from_utf8(bytes.to_vec()).map_err(|e| { + ToolError::ExecutionFailed(format!("Response is not valid UTF-8: {}", e)) + })? + }; // Basic size check if content.len() as u64 > crate::skills::MAX_PROMPT_FILE_SIZE { @@ -474,6 +519,102 @@ pub async fn fetch_skill_content(url: &str) -> Result { Ok(content) } +/// Extract `SKILL.md` from a ZIP archive returned by the ClawHub download API. +/// +/// Walks ZIP local file headers looking for an entry named `SKILL.md`. +/// Supports Store (method 0) and Deflate (method 8) compression. +fn extract_skill_from_zip(data: &[u8]) -> Result { + use flate2::read::DeflateDecoder; + use std::io::Read; + + // SKILL.md files should never be larger than 1 MB. + const MAX_DECOMPRESSED: usize = 1_024 * 1_024; + + let mut offset = 0; + while offset + 30 <= data.len() { + // Local file header signature = PK\x03\x04 + if data[offset..offset + 4] != [0x50, 0x4B, 0x03, 0x04] { + break; + } + + let compression = u16::from_le_bytes([data[offset + 8], data[offset + 9]]); + let compressed_size = u32::from_le_bytes([ + data[offset + 18], + data[offset + 19], + data[offset + 20], + data[offset + 21], + ]) as usize; + let uncompressed_size = u32::from_le_bytes([ + data[offset + 22], + data[offset + 23], + data[offset + 24], + data[offset + 25], + ]) as usize; + let name_len = u16::from_le_bytes([data[offset + 26], data[offset + 27]]) as usize; + let extra_len = u16::from_le_bytes([data[offset + 28], data[offset + 29]]) as usize; + + let name_start = offset + 30; + let name_end = name_start + name_len; + if name_end > data.len() { + break; + } + let file_name = std::str::from_utf8(&data[name_start..name_end]).unwrap_or(""); + + let data_start = name_end + .checked_add(extra_len) + .ok_or_else(|| ToolError::ExecutionFailed("ZIP header offset overflow".to_string()))?; + let data_end = data_start + .checked_add(compressed_size) + .ok_or_else(|| ToolError::ExecutionFailed("ZIP header size overflow".to_string()))?; + + if file_name == "SKILL.md" { + if data_end > data.len() { + return Err(ToolError::ExecutionFailed( + "ZIP archive truncated".to_string(), + )); + } + + if uncompressed_size > MAX_DECOMPRESSED { + return Err(ToolError::ExecutionFailed( + "ZIP entry too large to decompress safely".to_string(), + )); + } + + let raw = &data[data_start..data_end]; + let decompressed = match compression { + 0 => raw.to_vec(), // Store + 8 => { + // Deflate -- wrap with a read limit to guard against ZIP bombs + // where the declared size is small but decompressed output is huge. + let mut decoder = DeflateDecoder::new(raw).take(MAX_DECOMPRESSED as u64); + let mut buf = Vec::with_capacity(uncompressed_size.min(MAX_DECOMPRESSED)); + decoder.read_to_end(&mut buf).map_err(|e| { + ToolError::ExecutionFailed(format!("Failed to decompress SKILL.md: {}", e)) + })?; + buf + } + other => { + return Err(ToolError::ExecutionFailed(format!( + "Unsupported ZIP compression method: {}", + other + ))); + } + }; + + return String::from_utf8(decompressed).map_err(|e| { + ToolError::ExecutionFailed(format!("SKILL.md in archive is not valid UTF-8: {}", e)) + }); + } + + // Skip to next entry + offset = data_end; + } + + Err(ToolError::ExecutionFailed( + "ZIP archive does not contain SKILL.md".to_string(), + )) +} + // ── skill_remove ──────────────────────────────────────────────────────── pub struct SkillRemoveTool { @@ -675,4 +816,78 @@ mod tests { let err = super::validate_fetch_url("file:///etc/passwd").unwrap_err(); assert!(err.to_string().contains("Only HTTPS")); } + + #[test] + fn test_extract_skill_from_zip_deflate() { + // Build a real ZIP with flate2 + manual header construction. + use flate2::Compression; + use flate2::write::DeflateEncoder; + use std::io::Write; + + let skill_md = b"---\nname: test\n---\n# Test Skill\n"; + let mut encoder = DeflateEncoder::new(Vec::new(), Compression::default()); + encoder.write_all(skill_md).unwrap(); + let compressed = encoder.finish().unwrap(); + + let mut zip = Vec::new(); + // Local file header + zip.extend_from_slice(&[0x50, 0x4B, 0x03, 0x04]); // signature + zip.extend_from_slice(&[0x14, 0x00]); // version needed (2.0) + zip.extend_from_slice(&[0x00, 0x00]); // flags + zip.extend_from_slice(&[0x08, 0x00]); // compression: deflate + zip.extend_from_slice(&[0x00, 0x00, 0x00, 0x00]); // mod time/date + zip.extend_from_slice(&[0x00, 0x00, 0x00, 0x00]); // crc32 (unused) + zip.extend_from_slice(&(compressed.len() as u32).to_le_bytes()); // compressed size + zip.extend_from_slice(&(skill_md.len() as u32).to_le_bytes()); // uncompressed size + zip.extend_from_slice(&8u16.to_le_bytes()); // filename length + zip.extend_from_slice(&0u16.to_le_bytes()); // extra field length + zip.extend_from_slice(b"SKILL.md"); + zip.extend_from_slice(&compressed); + + let result = super::extract_skill_from_zip(&zip).unwrap(); + assert_eq!(result, "---\nname: test\n---\n# Test Skill\n"); + } + + #[test] + fn test_extract_skill_from_zip_store() { + let skill_md = b"---\nname: stored\n---\n# Stored\n"; + + let mut zip = Vec::new(); + // Local file header + zip.extend_from_slice(&[0x50, 0x4B, 0x03, 0x04]); + zip.extend_from_slice(&[0x0A, 0x00]); // version needed (1.0) + zip.extend_from_slice(&[0x00, 0x00]); // flags + zip.extend_from_slice(&[0x00, 0x00]); // compression: store + zip.extend_from_slice(&[0x00, 0x00, 0x00, 0x00]); // mod time/date + zip.extend_from_slice(&[0x00, 0x00, 0x00, 0x00]); // crc32 + zip.extend_from_slice(&(skill_md.len() as u32).to_le_bytes()); // compressed = uncompressed + zip.extend_from_slice(&(skill_md.len() as u32).to_le_bytes()); + zip.extend_from_slice(&8u16.to_le_bytes()); // filename length + zip.extend_from_slice(&0u16.to_le_bytes()); // extra field length + zip.extend_from_slice(b"SKILL.md"); + zip.extend_from_slice(skill_md); + + let result = super::extract_skill_from_zip(&zip).unwrap(); + assert_eq!(result, "---\nname: stored\n---\n# Stored\n"); + } + + #[test] + fn test_extract_skill_from_zip_missing_skill_md() { + let mut zip = Vec::new(); + zip.extend_from_slice(&[0x50, 0x4B, 0x03, 0x04]); + zip.extend_from_slice(&[0x0A, 0x00]); // version + zip.extend_from_slice(&[0x00, 0x00]); // flags + zip.extend_from_slice(&[0x00, 0x00]); // compression: store + zip.extend_from_slice(&[0x00, 0x00, 0x00, 0x00]); // mod time/date + zip.extend_from_slice(&[0x00, 0x00, 0x00, 0x00]); // crc32 + zip.extend_from_slice(&2u32.to_le_bytes()); // compressed size + zip.extend_from_slice(&2u32.to_le_bytes()); // uncompressed size + zip.extend_from_slice(&10u16.to_le_bytes()); // filename length + zip.extend_from_slice(&0u16.to_le_bytes()); // extra field length + zip.extend_from_slice(b"_meta.json"); + zip.extend_from_slice(b"{}"); + + let err = super::extract_skill_from_zip(&zip).unwrap_err(); + assert!(err.to_string().contains("does not contain SKILL.md")); + } }