fix(evolution): constrain pending skill path operations

crates/skilllite-commands/src/evolution_desktop.rs

@@ -149,6 +149,7 @@ pub fn list_pending_skills(workspace: &str) -> Result<Vec<PendingSkillSnapshot>>
 }
 
 pub fn read_pending_skill_md(workspace: &str, skill_name: &str) -> Result<String> {
+    let skill_name = skilllite_evolution::skill_synth::validate_pending_skill_name(skill_name)?;
     let skills_root = resolve_skills_root(workspace)?;
     let path = skills_root
         .join("_evolved")

crates/skilllite-evolution/src/skill_synth/mod.rs

@@ -26,7 +26,7 @@ mod scan;
 mod validate;
 
 use std::collections::HashSet;
-use std::path::Path;
+use std::path::{Component, Path};
 
 use rusqlite::Connection;
 
@@ -242,7 +242,20 @@ pub fn list_pending_skills_with_review(skills_root: &Path) -> Vec<(String, bool)
         .collect()
 }
 
+pub fn validate_pending_skill_name(skill_name: &str) -> Result<&str> {
+    if skill_name.trim().is_empty() || skill_name.chars().any(|c| matches!(c, '/' | '\\' | '\0')) {
+        bail!("invalid pending skill name: {}", skill_name);
+    }
+
+    let mut components = Path::new(skill_name).components();
+    match (components.next(), components.next()) {
+        (Some(Component::Normal(_)), None) => Ok(skill_name),
+        _ => bail!("invalid pending skill name: {}", skill_name),
+    }
+}
+
 pub fn confirm_pending_skill(skills_root: &Path, skill_name: &str) -> Result<()> {
+    let skill_name = validate_pending_skill_name(skill_name)?;
     let pending_dir = skills_root.join("_evolved").join("_pending");
     let evolved_dir = skills_root.join("_evolved");
     let src = pending_dir.join(skill_name);
@@ -261,6 +274,7 @@ pub fn confirm_pending_skill(skills_root: &Path, skill_name: &str) -> Result<()>
 }
 
 pub fn reject_pending_skill(skills_root: &Path, skill_name: &str) -> Result<()> {
+    let skill_name = validate_pending_skill_name(skill_name)?;
     let pending_dir = skills_root.join("_evolved").join("_pending");
     let src = pending_dir.join(skill_name);
 
@@ -278,3 +292,64 @@ pub fn reject_pending_skill(skills_root: &Path, skill_name: &str) -> Result<()>
 pub use repair::{repair_one_skill, repair_skills};
 pub use scan::track_skill_usage;
 pub use validate::{validate_skills, SkillValidation};
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn create_pending_skill(skills_root: &Path, name: &str) {
+        let skill_dir = skills_root.join("_evolved").join("_pending").join(name);
+        std::fs::create_dir_all(&skill_dir).unwrap();
+        std::fs::write(skill_dir.join("SKILL.md"), "# Test skill\n").unwrap();
+    }
+
+    #[test]
+    fn reject_pending_skill_rejects_absolute_path_without_deleting_escape_target() {
+        let temp = tempfile::tempdir().unwrap();
+        let skills_root = temp.path().join("skills");
+        let escape_target = temp.path().join("keep-me");
+        std::fs::create_dir_all(skills_root.join("_evolved").join("_pending")).unwrap();
+        std::fs::create_dir_all(&escape_target).unwrap();
+        std::fs::write(escape_target.join("sentinel.txt"), "still here").unwrap();
+
+        let err = reject_pending_skill(&skills_root, escape_target.to_str().unwrap()).unwrap_err();
+
+        assert!(err.to_string().contains("invalid pending skill name"));
+        assert!(escape_target.join("sentinel.txt").is_file());
+    }
+
+    #[test]
+    fn confirm_pending_skill_rejects_path_traversal_without_moving_escape_target() {
+        let temp = tempfile::tempdir().unwrap();
+        let skills_root = temp.path().join("skills");
+        std::fs::create_dir_all(skills_root.join("_evolved").join("_pending")).unwrap();
+        let escape_source = skills_root.join("external-skill");
+        let escape_destination = temp.path().join("external-skill");
+        std::fs::create_dir_all(&escape_source).unwrap();
+
+        let err = confirm_pending_skill(&skills_root, "../../external-skill").unwrap_err();
+
+        assert!(err.to_string().contains("invalid pending skill name"));
+        assert!(escape_source.is_dir());
+        assert!(!escape_destination.exists());
+    }
+
+    #[test]
+    fn confirm_and_reject_pending_skill_accept_safe_names() {
+        let temp = tempfile::tempdir().unwrap();
+        let skills_root = temp.path().join("skills");
+        std::fs::create_dir_all(skills_root.join("_evolved").join("_pending")).unwrap();
+        create_pending_skill(&skills_root, "safe-skill");
+        create_pending_skill(&skills_root, "other-safe-skill");
+
+        confirm_pending_skill(&skills_root, "safe-skill").unwrap();
+        reject_pending_skill(&skills_root, "other-safe-skill").unwrap();
+
+        assert!(skills_root.join("_evolved").join("safe-skill").is_dir());
+        assert!(!skills_root
+            .join("_evolved")
+            .join("_pending")
+            .join("other-safe-skill")
+            .exists());
+    }
+}

tasks/TASK-2026-067-pending-skill-path-safety/CONTEXT.md

@@ -0,0 +1,40 @@
+# Technical Context
+
+## Current State
+
+- Relevant crates/files: `crates/skilllite-evolution/src/skill_synth/mod.rs`
+  owns pending skill confirm/reject; `crates/skilllite-commands/src/evolution_desktop.rs`
+  owns desktop pending skill reads and delegates confirm/reject; `skilllite/src/dispatch/mod.rs`
+  exposes CLI dispatch.
+- Current behavior: callers provide a `skill_name` string that is joined onto
+  `_evolved/_pending` without checking whether it is a single safe path segment.
+
+## Architecture Fit
+
+- Layer boundaries involved: entry CLI -> commands -> evolution library. The
+  invariant belongs in `skilllite-evolution` for mutating operations and in
+  `skilllite-commands` for the read helper that constructs its own path.
+- Interfaces to preserve: command names, JSON output shape, and successful
+  operation semantics for safe pending skill directory names.
+
+## Dependency and Compatibility
+
+- New dependencies: none.
+- Backward compatibility notes: names containing path separators were never
+  valid entries returned by pending skill listing, so rejecting them closes an
+  unsafe input path without changing listed pending skill behavior.
+
+## Design Decisions
+
+- Decision: validate identifiers as single normal path components before any
+  pending skill filesystem operation.
+  - Rationale: this directly addresses absolute-path and `..` traversal while
+    keeping the accepted identifier model aligned with `read_dir` listing.
+  - Alternatives considered: canonicalize after join and check ancestors.
+  - Why rejected: canonicalization requires the path to exist and still benefits
+    from segment validation; a single-component invariant is simpler and
+    sufficient for pending skill IDs.
+
+## Open Questions
+
+- [x] No open questions for the minimal security fix.

tasks/TASK-2026-067-pending-skill-path-safety/PRD.md

@@ -0,0 +1,51 @@
+# PRD
+
+## Background
+
+Recent evolution L2 CLI work exposed pending skill confirmation and rejection
+through desktop-facing commands. Those commands pass `skill_name` through to
+filesystem operations that currently accept path separators and absolute paths.
+This creates an arbitrary directory delete/move risk in a trusted local CLI and
+desktop process.
+
+## Objective
+
+Constrain pending skill operations so the identifier can only name one pending
+skill directory under `_evolved/_pending`, and preserve the existing valid
+confirm/reject workflow.
+
+## Functional Requirements
+
+- FR-1: Reject pending skill identifiers containing path separators, absolute
+  path semantics, `.` or `..` segments, or empty/whitespace-only names.
+- FR-2: Apply the same validation to pending skill read, confirm, and reject
+  paths.
+- FR-3: Keep successful confirm/reject behavior unchanged for safe skill names.
+
+## Non-Functional Requirements
+
+- Security: pending skill operations must remain confined to the pending skill
+  directory and must not delete or move arbitrary filesystem paths.
+- Performance: validation should be constant-time over a short identifier and
+  add no meaningful overhead.
+- Compatibility: generated/listed pending skill names that are normal directory
+  names remain accepted.
+
+## Constraints
+
+- Technical: avoid new dependencies and keep the fix in shared lower-level code
+  where CLI and desktop callers both benefit.
+- Timeline: immediate P0 bug fix; avoid broad refactors.
+
+## Success Metrics
+
+- Metric: traversal regression tests.
+- Baseline: unsafe names can be joined outside `_pending`.
+- Target: unsafe names return validation errors and out-of-scope directories are
+  left intact.
+
+## Rollout
+
+- Rollout plan: ship as a patch release / normal merge once tests pass.
+- Rollback plan: revert the small validation change if it blocks legitimate
+  generated skill names; no data migration is involved.

tasks/TASK-2026-067-pending-skill-path-safety/REVIEW.md

@@ -0,0 +1,45 @@
+# Review Report
+
+## Scope Reviewed
+
+- Files/modules: `crates/skilllite-evolution/src/skill_synth/mod.rs`,
+  `crates/skilllite-commands/src/evolution_desktop.rs`, task artifacts.
+- Commits/changes: recent evolution L2 CLI bridge paths for pending skill
+  read/confirm/reject.
+
+## Findings
+
+- Critical: `skill_name` was previously joined directly onto
+  `_evolved/_pending`; absolute paths could make `reject_pending_skill` delete
+  arbitrary directories and `..` paths could make `confirm_pending_skill` move
+  non-pending directories.
+- Major: None.
+- Minor: None.
+
+## Quality Gates
+
+- Architecture boundary checks: `pass`
+- Security invariants: `pass`
+- Required tests executed: `pass`
+- Docs sync (EN/ZH): `pass` (no user-facing command, env, or policy wording
+  change; validation closes unsafe inputs only)
+
+## Test Evidence
+
+- Commands run: `cargo test -p skilllite-evolution pending_skill -- --nocapture`;
+  `cargo test -p skilllite-evolution`; `cargo test -p skilllite-commands`;
+  `cargo fmt --check`; `cargo clippy --all-targets -- -D warnings`;
+  `cargo test`; `python3 scripts/validate_tasks.py`.
+- Key outputs: focused tests passed with
+  `3 passed; 0 failed; 94 filtered out`; `skilllite-evolution` passed
+  `97 passed; 0 failed`; `skilllite-commands` passed `23 passed; 0 failed`;
+  full workspace `cargo test` completed successfully; task validation passed
+  `67 task directories checked`.
+- Environment note: the first test attempt failed before compilation because
+  Cargo 1.83 does not support edition 2024 dependencies; stable was updated to
+  Rust/Cargo 1.96 per repository instructions, then validation passed.
+
+## Decision
+
+- Merge readiness: `ready`
+- Follow-up actions: None required for this PR.

tasks/TASK-2026-067-pending-skill-path-safety/STATUS.md

@@ -0,0 +1,32 @@
+# Status Journal
+
+## Timeline
+
+- 2026-06-03:
+  - Progress: Created task artifacts for a P0 pending skill path traversal
+    fix. Confirmed the vulnerable operations and defined validation/test plan.
+  - Blockers: None.
+  - Next step: Implement shared pending skill name validation and regression
+    tests.
+- 2026-06-03:
+  - Progress: Implemented shared pending skill name validation in
+    `skilllite-evolution`, applied it to desktop pending skill reads, and added
+    regression tests for absolute-path deletion, `..` movement, and safe-name
+    compatibility.
+  - Blockers: Initial validation hit Rust 1.83 / Cargo edition-2024 support;
+    updated stable toolchain to Rust 1.96 per `AGENTS.md`.
+  - Next step: Open PR after final artifact/board verification.
+- 2026-06-03:
+  - Progress: Completed final task artifact updates and board transition to
+    Done.
+  - Blockers: None.
+  - Next step: Open PR.
+
+## Checkpoints
+
+- [x] PRD drafted before implementation (or `N/A` recorded)
+- [x] Context drafted before implementation (or `N/A` recorded)
+- [x] Implementation complete
+- [x] Tests passed
+- [x] Review complete
+- [x] Board updated

tasks/TASK-2026-067-pending-skill-path-safety/TASK.md

@@ -0,0 +1,68 @@
+# TASK Card
+
+## Metadata
+
+- Task ID: `TASK-2026-067`
+- Title: Constrain pending skill path operations
+- Status: `done`
+- Priority: `P0`
+- Owner: `agent`
+- Contributors: Cursor automation
+- Created: `2026-06-03`
+- Target milestone: critical bug fix
+
+## Problem
+
+Pending skill operations accept `skill_name` values and join them directly under
+`<skills>/_evolved/_pending`. Absolute paths and `..` segments can escape that
+directory; `reject_pending_skill` then calls `remove_dir_all` on the escaped
+path, and `confirm_pending_skill` can move directories outside the pending
+queue. The CLI and desktop bridge expose these operations.
+
+## Scope
+
+- In scope: validate pending skill names as safe single path segments before
+  read, confirm, or reject filesystem operations.
+- In scope: add regression tests that prove path traversal inputs are rejected
+  without deleting or moving out-of-scope directories.
+- Out of scope: redesign pending skill storage, UI contract changes, or broader
+  evolution policy changes.
+
+## Acceptance Criteria
+
+- [x] `confirm_pending_skill`, `reject_pending_skill`, and desktop pending skill
+      reads reject absolute paths and `..` escapes.
+- [x] Valid pending skill names continue to confirm and reject normally.
+- [x] Regression tests cover rejection behavior and prove out-of-scope
+      directories survive.
+
+## Risks
+
+- Risk: Overly strict validation could reject valid skill directories.
+  - Impact: Users may be unable to confirm or reject a generated pending skill.
+  - Mitigation: Match the safety boundary to existing listing behavior: only
+    single directory names are valid pending skill identifiers.
+
+## Validation Plan
+
+- Required tests: focused `skilllite-evolution` unit tests plus workspace Rust
+  format, clippy, and test commands required by repository policy.
+- Commands to run: `cargo fmt --check`; `cargo test -p skilllite-evolution`;
+  `cargo test -p skilllite-commands`; `cargo clippy --all-targets -- -D warnings`;
+  `cargo test`; `python3 scripts/validate_tasks.py`.
+- Manual checks: inspect the patched filesystem paths and task artifacts after
+  edits.
+
+## Regression Scope
+
+- Areas likely affected: evolution pending skill read/confirm/reject paths,
+  desktop evolution UI commands, CLI `skilllite evolution confirm|reject`.
+- Explicit non-goals: changing generated skill naming rules beyond path safety,
+  adding new dependencies, or changing user-facing command names.
+
+## Links
+
+- Source TODO section: N/A; discovered during critical bug automation.
+- Related PRs/issues: recent evolution L2 CLI bridge commits.
+- Related docs: `spec/security-nonnegotiables.md`,
+  `spec/rust-conventions.md`, `spec/testing-policy.md`.

tasks/board.md

@@ -1,6 +1,6 @@
 # Task Board
 
-Last updated: 2026-05-29 (TASK-2026-066 utf8 evolution log truncate done)
+Last updated: 2026-06-03 (TASK-2026-067 pending skill path safety done)
 
 ## In Progress
 
@@ -17,6 +17,7 @@ Last updated: 2026-05-29 (TASK-2026-066 utf8 evolution log truncate done)
 
 ## Done
 
+- `TASK-2026-067-pending-skill-path-safety` - Status: `done` - Owner: `agent`
 - `TASK-2026-066-utf8-evolution-log-truncate` - Status: `done` - Owner: `agent`
 - `TASK-2026-064-env-keys-single-source` - Status: `done` - Owner: `agent`
 - `TASK-2026-063-extension-tool-metadata-dispatch` - Status: `done` - Owner: `agent`