Commit Graph
7 Commits
Author SHA1 Message Date
[email protected]andClaude Opus 4.6 4d643f47c7 refactor: remove glob re-exports, fix clippy warnings, clean up duplicates
- Remove `pub use ironclaw_safety::*` from src/safety/mod.rs and migrate
  all 20+ call sites to import directly from `ironclaw_safety`
- Remove `pub use ironclaw_skills::*` from src/skills/mod.rs and migrate
  all 15+ call sites to import directly from `ironclaw_skills`
- Fix 4 clippy warnings: 2 shadow imports, 2 collapsible if-let chains
- Add missing SkillActivated arm to WASM channel StatusUpdate match
- Remove duplicate AuthRequired/AuthCompleted arms in repl.rs
- Update CLAUDE.md extracted crates guidance and prompt template rule
- Fix bench imports (safety_check, safety_pipeline)

46 files changed, zero warnings, 3836 tests passing.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-03-27 23:51:24 -07:00
Xing JiandGitHub 3f6d2ab6c2 fix(skill): treat empty url param as absent when installing skills (#1128)
LLMs sometimes pass "" for optional parameters instead of omitting them.
Previously, passing url: "" to skill_install would match the explicit-URL
branch and attempt to fetch from an empty string, producing an invalid URL
error instead of falling back to the catalog lookup.

Fix by adding .filter(|s| !s.is_empty()) so an empty url is treated the
same as a missing field.

A unit test verifies the parameter filtering behaviour directly; the full
execute path (catalog lookup + install) requires a real catalog and database
and cannot be covered at the unit level.
2026-03-15 05:50:39 +00:00
Nick PismenkovandGitHub 7d8576a464 fix: destructive actions from ambiguous user prompts (#782)
* fix: destructive actions from ambiguous user prompts

* review fixes

* review fixes
2026-03-09 18:03:39 -07:00
45ec691f4c Improve test infrastructure: StubChannel, gateway helpers, security tests, search edge cases (#623)
* feat(testing): add StubChannel test double for Channel trait

Adds StubChannel to src/testing.rs alongside StubLlm. Supports message
injection via mpsc sender, response/status capture, and configurable
health check toggling. Includes handle methods for use after ownership
transfer to ChannelManager.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat(testing): wire StubChannel into TestHarnessBuilder

Add with_stub_channel() builder method that creates a StubChannel
pre-registered in a ChannelManager. Tests can inject messages via
the sender and verify routing through the manager. The channel field
on TestHarness is Optional, defaulting to None for backward compat.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test: gate external-service tests behind integration feature flag

Replace silent try_connect() skip pattern with explicit feature gating.
cargo test now runs only self-contained tests.
cargo test --features integration runs tests requiring PostgreSQL.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test(channels): add ChannelManager unit tests using StubChannel

Cover add/start_all stream merging, respond routing, unknown channel
errors, health_check_all with mixed health, empty-channels error path,
and injection channel merging -- all via StubChannel test double.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* docs: document test tier separation (unit/integration/live)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* ci: add architecture boundary check script

Grep-based checks for three architecture boundaries:
- Direct database driver usage (tokio_postgres/libsql) outside src/db/
- .unwrap()/.expect() in production code (warning only)
- Direct std::env::var reads outside config layer (warning only)

The DB driver check is a hard violation; the other two are warnings
for gradual cleanup. Run with: bash scripts/check-boundaries.sh

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test(search): add RRF edge case tests for empty inputs, limits, and config modes

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test(security): add regression tests for skill installer ZIP and SSRF protections

Add 11 regression tests covering the security controls in skill_tools:

ZIP extraction safety:
- Valid SKILL.md extraction works correctly
- Non-SKILL.md entries are ignored (returns error)
- Path traversal entries (../../SKILL.md) do not match
- Nested path entries (subdir/SKILL.md) do not match
- Oversized entries (>1MB uncompressed) are rejected

SSRF prevention:
- Loopback addresses (127.0.0.1) are blocked
- Private ranges (10.x, 172.16.x, 192.168.x) are blocked
- Link-local addresses (169.254.x) are blocked
- Public IPs (8.8.8.8, 1.1.1.1) are allowed
- IPv4-mapped IPv6 unwrapping logic works correctly
- Metadata endpoints and .internal/.local hostnames are blocked
- Normal hostnames (github.com, clawhub.dev) are allowed

Also documents a known gap: url::Url::host_str() returns bracketed
IPv6 addresses that std::net::IpAddr cannot parse, so IPv4-mapped
IPv6 URLs currently bypass IP-based checks in validate_fetch_url.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor(testing): extract TestGatewayBuilder to eliminate gateway test duplication

Both ws_gateway_integration.rs and openai_compat_integration.rs manually
constructed GatewayState with 19+ fields. Extracted to a shared builder in
src/channels/web/test_helpers.rs that provides sensible defaults and lets
tests override only what they need.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* docs: add implementation plans for testing batches 1 and 2

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(security): close IPv6 SSRF bypass in validate_fetch_url

validate_fetch_url used host_str() which returns bracketed IPv6
(e.g. "[::ffff:7f00:1]") that IpAddr::parse() cannot handle,
silently skipping IP-based SSRF checks for all IPv6 URLs.

Switch to url::Host enum matching to extract proper IpAddr values
without string parsing. IPv4-mapped IPv6 addresses like
::ffff:127.0.0.1 are now correctly unwrapped and blocked.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test(skills): add activation criteria limits enforcement tests

Adds test_activation_criteria_enforce_limits to verify that
enforce_limits() correctly trims excess patterns (>5), keywords (>20),
and tags (>10), and filters out short keywords/tags (<3 chars).

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* test(wasm): add security regression tests for WASM tool loader

Add 6 tests covering: tool name path separator rejection, empty name
rejection, nonexistent file handling, invalid WASM bytes rejection,
dotfile discovery behavior, and subdirectory non-recursion.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* refactor: address PR review feedback

- Remove plan files from repo (ilblackdragon review)
- Replace CLAUDE.md test tier rules with pointer to check-boundaries.sh
- Add Check 4 to check-boundaries.sh: enforces integration tests are
  gated behind the 'integration' feature flag

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* ci: add try_connect silent-skip pattern check to check-boundaries.sh

Check 5 catches try_connect() and similar silent-skip patterns in
integration tests. Tests should use feature gates to fail loudly
when prerequisites are missing, not silently return.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix(security): harden skill fetch SSRF checks

* fix(scripts): use bash arrays in check-boundaries.sh tier violation check

Refactor Check 4 in check-boundaries.sh to use bash arrays and printf
instead of string concatenation with echo -e. This is more robust with
special characters in filenames and avoids portability concerns with
echo -e. [skip-regression-check]

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-03-07 08:30:47 +00:00
4e2dd76ae5 Fix skills system: enable by default, fix registry and install (#300)
* feat: add Docker detection module with platform guidance

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: add Docker sandbox step to setup wizard

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: show Docker status in boot screen

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* fix: cargo fmt + update test_builder_defaults for enabled-by-default

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: deduplicate wizard Docker status handling per review

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* feat: add /skills command and enrich search results with ClawHub metadata

- Parse /skills and /skills search <query> 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 <q> 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 <[email protected]>

* fix: cargo fmt after merge conflict resolution

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* 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 <[email protected]>

* 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 <[email protected]>

* Harden Docker detection for rootless Linux and Windows fallback

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-02-23 10:04:02 -08:00
2cdd1acb1e refactor: consolidate tool approval into single param-aware method (#274)
* refactor: consolidate tool approval into single param-aware method

Replace the two confusing approval methods (requires_approval() and
requires_approval_for()) with a single requires_approval(&self, params)
returning a 3-variant ApprovalRequirement enum (Never, UnlessAutoApproved,
Always). This enables param-aware approval decisions: HTTP calls without
auth headers now skip approval entirely, while authenticated requests
always require it. Shell tool merges its destructive-command detection
into the same method.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: add credential injection to built-in HTTP tool

Wire the WASM credential injection system into the built-in HTTP tool
so credentials are auto-injected at the boundary (zero-exposure model).

- Add SharedCredentialRegistry: thread-safe, append-only registry of
  credential mappings populated by WASM tools at registration time
- Add credential_detect module with broad auth detection for headers
  (12 exact + 5 substring matches), header values (7 auth scheme
  prefixes), and URL query params (17 exact + 5 substring matches)
- HttpTool now accepts optional credential registry + secrets store,
  auto-injects matching credentials in execute(), and uses broader
  auth detection in requires_approval()
- ToolRegistry passes credential registry to HttpTool at startup and
  populates it when WASM tools register
- Remove old hardcoded AUTH_HEADER_NAMES / has_auth_headers in favor
  of the new params_contain_manual_credentials()

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR #274 review comments (query param injection, lock poisoning, visibility)

- Fix injected query params not being sent on outbound HTTP requests by
  also calling .query() on the RequestBuilder alongside parsed_url mutation
- Recover from poisoned RwLock in SharedCredentialRegistry instead of
  silently ignoring failures, with tracing::warn for visibility
- Narrow inject_credential and host_matches_pattern to pub(crate) to
  avoid committing to them as stable public API

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-02-21 01:28:23 +00:00
bac2d75713 feat: Secure prompt-based skills system (Phases 1-4) (#51)
* feat: Add secure prompt-based skills system (Phase 1 MVP)

Implement a skills system that extends the agent with prompt-level
instructions from local directories. Skills declare activation criteria,
tool permissions, and trust tiers that determine authority attenuation.

Core security model: the minimum trust level of any active skill
determines a tool ceiling -- tools above the ceiling are removed from
the LLM's tool list entirely at the API level, preventing prompt-based
manipulation.

New modules:
- skills/mod.rs: Core types (SkillTrust, SkillManifest, LoadedSkill)
- skills/scanner.rs: Content scanner for manipulation detection
- skills/registry.rs: Filesystem discovery and manifest parsing
- skills/selector.rs: Deterministic two-phase prefilter (no LLM)
- skills/attenuation.rs: Trust-based tool filtering

Integration:
- Agent loop selects skills per-turn and applies tool attenuation
- Reasoning engine injects skill context with structural isolation
- Config supports SKILLS_ENABLED, SKILLS_DIR, SKILLS_MAX_ACTIVE,
  SKILLS_MAX_CONTEXT_TOKENS environment variables
- Disabled by default (SKILLS_ENABLED=false)

41 new tests covering all modules.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: Address all adversarial review findings for skills system

Security fixes:
- Escape skill name/version in XML attributes to prevent trust spoofing
- Escape prompt content to prevent </skill> tag breakout
- Require integrity hash for Verified/Community tier skills
- Validate skill names against [a-zA-Z0-9][a-zA-Z0-9._-]{0,63}
- Add 64 KiB file size limit on prompt.md

Bug fixes:
- Use actual SkillsConfig from AgentDeps instead of SkillsConfig::default()
- Add skills_config field to AgentDeps, wired through from main.rs

Performance:
- Pre-compile regex patterns at load time (cached on LoadedSkill)
- Selector uses pre-compiled patterns instead of recompiling per message
- Switch all std::fs to tokio::fs for non-blocking async I/O

Hardening:
- Cap keyword score at 30 points to prevent keyword stuffing attacks
- Enforce max 20 keywords and 5 patterns per skill
- Normalize line endings (CRLF/CR to LF) before hashing
- Also includes cargo fmt formatting fixes for adjacent code

Tests: 54 skills tests pass (up from 41), zero new clippy warnings.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: Address medium/low severity findings from adversarial review

Fixes all 18 medium/low severity findings identified by the security review:

- mod.rs: Add MAX_TAGS_PER_SKILL cap (10) in enforce_limits(); use
  RegexBuilder with 64 KiB size_limit to prevent ReDoS; replace
  case-enumerated escape_skill_content with regex matching all case
  variants plus whitespace/null byte injection between </ and skill;
  document allowed_patterns as unenforced until Phase 2; document
  Marketplace URL validation as Phase 3 concern

- registry.rs: Add MAX_MANIFEST_FILE_SIZE (16 KiB) check before reading;
  add symlink detection via symlink_metadata to reject symlinks in
  discover_local; add MAX_DISCOVERED_SKILLS (100) cap; validate
  prompt_hash format (sha256: + 64 hex chars); warn on name collision
  before overwriting; accept SkillSource parameter in load_skill instead
  of always using Local; add InvalidHashFormat, ManifestTooLarge,
  SymlinkDetected error variants

- selector.rs: Add MAX_TAG_SCORE (15) cap parallel to keyword cap; warn
  when declared max_context_tokens diverges >2x from actual prompt size

- scanner.rs: Add mixed-script homoglyph detection (Cyrillic, Greek,
  Armenian unicode ranges); document token-boundary bypass and semantic
  paraphrasing as known limitations

- attenuation.rs: Document READ_ONLY_TOOLS maintenance requirements

- agent_loop.rs: Surface scan warnings via structured tracing; add
  structured audit events for skill activation and tool attenuation

61 tests pass, 0 new clippy warnings.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: Address 12 findings from second adversarial security review

HIGH:
- Escape opening <skill tags in prompt content (prevents fake skill block injection)
- Scan manifest metadata fields (description, author, tags, reasons) not just prompt
- Block trust downgrade on name collision (existing Local can't be replaced by Community)

MEDIUM:
- Eliminate TOCTOU gap: read files then check size instead of metadata-then-read
- Reject file-level symlinks in load_skill (prompt.md, skill.toml)
- Truncate and filter manifest.skill.tags (prevent unlimited tag scoring)
- Cap regex pattern score at 40 (prevent 5x20=100 dominating keyword+tag)
- Add doc comment about skill_list tool exposing metadata (sanitization required)
- Move Community disclaimer inside <skill> tags (not outside structural boundary)
- Filter keywords/tags shorter than 3 chars (prevent broad matching)

LOW:
- Enforce minimum token_cost of 1 (max_context_tokens=0 can't bypass budget)
- Remove redundant try_exists checks in discover_local (let load_skill handle errors)

70 skills tests passing.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: Add HTTP endpoint scoping for skills (Phase 1)

Skills that declare an [http] section in skill.toml now have their HTTP
requests constrained to declared endpoints at runtime. This addresses
the gap where allowed_patterns was parsed but never enforced -- once the
http tool was visible via attenuation, the LLM could reach any URL.

Enforcement reuses EndpointPattern/AllowlistValidator from the WASM
capability system. Semantics: if no active skill declares [http], all
requests pass through (backward compat). If any skill declares [http],
URLs must match at least one skill's allowlist (union). Community skills'
[http] declarations are silently ignored (defense in depth).

Shell commands using curl/wget are also validated against scopes.

Scanner gains detection for known exfiltration domains (webhook.site,
ngrok.io, etc.), overly broad wildcards, and credential/host mismatches.

Closes #38

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: Apply cargo fmt to http_scoping.rs

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* style: Apply cargo fmt across codebase

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: Add parameter-level permission enforcement for skills (Phase 2)

Activates enforcement of `allowed_patterns` in skill.toml permissions.
Previously these patterns were parsed but not enforced -- a Verified skill
declaring `permissions.shell` with `allowed_patterns = [{command = "cargo *"}]`
could still run any shell command. Now the enforcer validates tool parameters
against declared glob patterns before execution.

Key changes:
- New `enforcer.rs` module with `SkillPermissionEnforcer`, `glob_to_regex()`,
  and `validate_tool_call()` with union semantics across active skills
- Typed pattern enums (`ShellPattern`, `FilePathPattern`, `MemoryTargetPattern`)
  replace the previous `Vec<serde_json::Value>` in `ToolPermissionDeclaration`
- Scanner gains `scan_permission_patterns()` detecting dangerous patterns
  (rm, sudo, curl, bare wildcards, command chaining, sensitive paths, identity files)
- Registry blocks non-Local skills with critical permission pattern warnings
- Agent loop threads enforcer into `execute_chat_tool` alongside HTTP scoping

Trust interaction: Community patterns ignored, Verified enforced, Local without
patterns unrestricted, Local with patterns enforced as guidance. Union semantics
across skills -- tool call allowed if ANY skill's patterns permit it.

34 new tests. All 818 library tests pass.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: Add worker permission enforcement and LLM behavioral analysis (Phase 3+4)

Phase 3 - Worker-side permission enforcement:
- Add SerializedToolPermission/SerializedPattern DTOs for HTTP boundary crossing
- Extend JobDescription, ContainerHandle, and orchestrator API to carry permissions
- CreateJobTool snapshots and forwards skill permissions to spawned workers
- Worker runtime builds SkillPermissionEnforcer and checks before tool execution
- Load-time token budget enforcement rejects prompts exceeding 2x declared budget
- Deduplicate enforcer construction: from_active_skills() delegates to from_serialized()

Phase 4 - LLM behavioral analysis:
- BehavioralAnalyzer with cached, LLM-based semantic content analysis
- Structured output parsing (FINDING|CATEGORY|SEVERITY|DESCRIPTION or CLEAN)
- Content-hash caching with bounded size (MAX_CACHE_ENTRIES=256)
- Graceful degradation when LLM unavailable
- Integrated into load_skill() for non-Local skills; critical findings block loading

Review fixes:
- Real cache tests with CountingLlm mock (test_cache_hit, test_cache_miss, test_cache_bounded)
- UTF-8-safe truncate() in worker runtime
- Few-shot examples in behavioral analysis prompt
- Documented max_context_tokens=0 opt-out and create_job() permission gap

848 tests passing, no new clippy warnings.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: Address review feedback from serrrfirat on skills-phase2

- Fix truncate_cmd UTF-8 panic: use char-boundary-aware slicing
- Remove redundant effective_tools branching in reasoning.rs
- Document cache eviction as known limitation (arbitrary, not LRU)
- Add safety comment on SkillTrust enum ordering (security-critical)
- Simplify active_skills selection (prefilter_skills handles empty input)

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address remaining skills review feedback

* refactor: replace skills system with OpenClaw SKILL.md format + 2-state trust

Replace the 5-gate, 3-tier trust hierarchy (scanner, behavioral analyzer,
parameter-level enforcer, HTTP endpoint scoping) with a simplified 3-layer
security model: gating -> attenuation -> Docker confinement.

Key changes:
- SKILL.md format (YAML frontmatter + markdown prompt) replaces skill.toml + prompt.md
- 2-state trust (Installed/Trusted) replaces 3-tier (Community/Verified/Local)
- New parser.rs for SKILL.md parsing with serde_yaml
- New gating.rs for requirements checking (bins/env/config)
- Simplified registry with 2-location discovery (workspace + user dirs)
- Removed scanner, behavioral_analyzer, enforcer, http_scoping (~4,100 lines)
- Removed skill_permissions propagation through job/orchestrator/worker pipeline
- Added serde_yaml dependency for YAML frontmatter parsing

Net: -5,298 lines, 59 skills tests pass, 907 total tests pass.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* feat: add in-app skill management tools and ClawHub catalog integration

Add 4 chat-callable tools (skill_list, skill_search, skill_install,
skill_remove) plus matching web gateway endpoints for managing skills
at runtime. The catalog fetches from ClawHub's public registry API
at runtime rather than bundling entries at compile time.

Key changes:
- SkillRegistry gains mutation methods (install_skill, remove_skill,
  reload, find_by_name) with Arc<RwLock> for concurrent access
- New catalog module queries ClawHub /api/v1/search with in-memory
  caching (5-min TTL, configurable via CLAWHUB_REGISTRY env var)
- skill_list and skill_search added to READ_ONLY_TOOLS for safe use
  under Installed trust ceiling
- Web gateway gets /api/skills, /api/skills/search, /api/skills/install,
  and /api/skills/{name} DELETE endpoints

Co-Authored-By: Claude Opus 4.6 <[email protected]>

* fix: address PR #51 review feedback from ilblackdragon

Security:
- Add SSRF protection to fetch_skill_content: require HTTPS, reject
  private/loopback/link-local IPs and internal hostnames, disable
  redirects. Gateway install handler now reuses the same validation.
- URL-encode slug in skill_download_url to prevent query injection.
- Require X-Confirm-Action header on gateway skill install/remove
  endpoints (equivalent to chat tool requires_approval gate).

Correctness:
- Eliminate all block_in_place/block_on usage in skill tools and
  gateway handlers. Split install into prepare_install_to_disk (static
  async, no lock) + commit_install (sync, brief write lock). Same
  pattern for remove: validate_remove + delete_skill_files + commit_remove.
- Write normalized content to disk in install_skill (was writing
  original un-normalized content, causing hash mismatch on re-read).
- Fix token estimation from 0.75 to 0.25 tokens/byte (~4 chars per
  token) in registry.rs, selector.rs, and standalone loader.

Dependencies:
- Replace deprecated serde_yaml 0.9 with serde_yml 0.0.12.
- Remove unused toml dependency.

Co-Authored-By: Claude Opus 4.6 <[email protected]>

---------

Co-authored-by: Claude Opus 4.6 <[email protected]>
2026-02-18 00:28:38 +00:00