feat(skills): #140 community-skill installer module

Add `crates/tui/src/skills/install.rs` — async installer that pulls
user-authored skills from GitHub repos, raw tarball URLs, or a curated
`index.json` registry. The whole pipeline is gated by the per-domain
`NetworkPolicy` (#135), validated against path-traversal / size / symlink
attacks before any bytes hit the destination, and atomic-renamed into place
so a half-installed skill cannot survive a failure mid-extract.

Public surface:
- `InstallSource::{GitHubRepo,DirectUrl,Registry}` with `parse(spec)`.
- `install` / `install_with_registry` returning
  `InstallOutcome::{Installed,NeedsApproval,NetworkDenied}`.
- `update` / `update_with_registry` returning
  `UpdateResult::{NoChange,Updated,NeedsApproval,NetworkDenied}` — uses a
  SHA-256 over the downloaded tarball to short-circuit no-op fetches.
- `uninstall` / `trust` — both refuse to touch directories without an
  `.installed-from` marker, so the bundled `skill-creator` system skill is
  protected.
- `fetch_registry` — typed loader for the curated `index.json`.

Validation hard rules (each covered by an integration test):
- `..` segments and absolute paths in tar entries are rejected.
- Symlinks / hardlinks in tar entries are rejected outright.
- Uncompressed total size is bounded by `max_size` (default 5 MiB).
- SKILL.md must exist at the archive root or under `skills/<name>/`.
- Frontmatter must carry both `name` and `description`.
- `install` with an existing destination requires `update = true`.
- `update` re-fetches and only replaces the on-disk install when the
  checksum changes; no-change paths skip the rename entirely.

Adds `tar`, `flate2`, and `sha2` to `crates/tui/Cargo.toml` and propagates
the resulting lockfile drift to `Cargo.lock`.

Tests: 11 colocated unit tests in `install.rs` + 11 integration tests in
`crates/tui/tests/skill_install.rs` driving a `tiny_http`-based server so
the network gate, download cap, validation pipeline, and atomic rename
all run end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Hunter Bown
2026-04-28 00:29:48 -05:00
parent 87ac2e9bdc
commit ad78466ba0
5 changed files with 1698 additions and 1 deletions
Generated
+46 -1
View File
@@ -1171,6 +1171,7 @@ dependencies = [
"deepseek-secrets",
"dirs",
"dotenvy",
"flate2",
"futures-util",
"ignore",
"image",
@@ -1186,9 +1187,11 @@ dependencies = [
"rustyline 15.0.0",
"serde",
"serde_json",
"sha2",
"shellexpand",
"shlex",
"starlark",
"tar",
"tempfile",
"thiserror 2.0.17",
"tiny_http",
@@ -1597,6 +1600,17 @@ dependencies = [
"winapi",
]
[[package]]
name = "filetime"
version = "0.2.27"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f98844151eee8917efc50bd9e8318cb963ae8b297431495d3f758616ea5c57db"
dependencies = [
"cfg-if",
"libc",
"libredox",
]
[[package]]
name = "find-msvc-tools"
version = "0.1.7"
@@ -2484,6 +2498,7 @@ checksum = "3d0b95e02c851351f877147b7deea7b1afb1df71b63aa5f8270716e0c5720616"
dependencies = [
"bitflags 2.10.0",
"libc",
"redox_syscall 0.7.4",
]
[[package]]
@@ -3063,7 +3078,7 @@ checksum = "2621685985a2ebf1c516881c026032ac7deafcda1a2c9b7850dc81e3dfcb64c1"
dependencies = [
"cfg-if",
"libc",
"redox_syscall",
"redox_syscall 0.5.18",
"smallvec",
"windows-link 0.2.1",
]
@@ -3458,6 +3473,15 @@ dependencies = [
"bitflags 2.10.0",
]
[[package]]
name = "redox_syscall"
version = "0.7.4"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f450ad9c3b1da563fb6948a8e0fb0fb9269711c9c73d9ea1de5058c79c8d643a"
dependencies = [
"bitflags 2.10.0",
]
[[package]]
name = "redox_users"
version = "0.4.6"
@@ -4335,6 +4359,17 @@ dependencies = [
"syn 2.0.114",
]
[[package]]
name = "tar"
version = "0.4.45"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "22692a6476a21fa75fdfc11d452fda482af402c008cdbaf3476414e122040973"
dependencies = [
"filetime",
"libc",
"xattr",
]
[[package]]
name = "tempfile"
version = "3.24.0"
@@ -5487,6 +5522,16 @@ version = "0.13.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ea6fc2961e4ef194dcbfe56bb845534d0dc8098940c7e5c012a258bfec6701bd"
[[package]]
name = "xattr"
version = "1.6.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "32e45ad4206f6d2479085147f02bc2ef834ac85886624a23575ae137c8aa8156"
dependencies = [
"libc",
"rustix 1.1.3",
]
[[package]]
name = "xdg-home"
version = "1.3.0"
+3
View File
@@ -57,6 +57,9 @@ zeroize = "1.8.2"
ignore = "0.4"
image = { version = "0.25", default-features = false, features = ["png"] }
pdf-extract = "0.7"
tar = "0.4"
flate2 = "1.1"
sha2 = "0.10"
[dev-dependencies]
wiremock = "0.6"
File diff suppressed because it is too large Load Diff
+11
View File
@@ -1,6 +1,17 @@
//! Skill discovery and registry for local SKILL.md files.
pub mod install;
mod system;
// Re-exports kept for documentation parity and downstream consumers; the
// binary itself imports directly from `skills::install`. `#[allow(...)]`
// silences the dead-code warning that fires because no `bin` source path
// references these names through `skills::*`.
#[allow(unused_imports)]
pub use install::{
DEFAULT_MAX_SIZE_BYTES, DEFAULT_REGISTRY_URL, INSTALLED_FROM_MARKER, InstallOutcome,
InstallSource, InstalledSkill, RegistryDocument, RegistryEntry, RegistryFetchResult,
UpdateResult,
};
pub use system::install_system_skills;
use std::fs;
+522
View File
@@ -0,0 +1,522 @@
//! Integration tests for the community-skill installer (#140).
//!
//! These tests exercise the full validation pipeline against a tiny in-process
//! HTTP server, so the network gate, download cap, tarball validation, atomic
//! rename, and `.installed-from` marker all run end-to-end. The module is
//! pulled in via `#[path]` includes (matching `integration_mock_llm.rs`) so we
//! get access to private helpers without a separate library crate.
use std::io::Write;
use std::path::Path;
use flate2::Compression;
use flate2::write::GzEncoder;
use tempfile::TempDir;
use tiny_http::{Method, Response, Server};
// Pull the production source files into this test binary so the test can
// reach `install`'s public surface without a dedicated library crate.
//
// `install.rs` only references `crate::network_policy` so we just need that
// one helper module alongside `install` itself.
#[path = "../src/network_policy.rs"]
mod network_policy;
#[path = "../src/skills/install.rs"]
#[allow(dead_code)]
mod install;
use crate::install::{InstallOutcome, InstallSource, UpdateResult};
use crate::network_policy::{DecisionToml, NetworkPolicy};
/// Construct a gzipped tarball from `(path, body)` pairs. Permissions are set
/// to 0o644 so umask differences across platforms don't perturb the bytes.
fn make_tarball(entries: &[(&str, &[u8])]) -> Vec<u8> {
let mut gz = GzEncoder::new(Vec::new(), Compression::default());
{
let mut builder = tar::Builder::new(&mut gz);
for (path, body) in entries {
let mut header = tar::Header::new_gnu();
header.set_size(body.len() as u64);
header.set_mode(0o644);
header.set_cksum();
builder
.append_data(&mut header, path, *body)
.expect("append_data");
}
builder.finish().expect("finish tar");
}
gz.finish().expect("finish gz")
}
fn skill_md(name: &str, description: &str) -> Vec<u8> {
format!(
"---\nname: {name}\ndescription: {description}\n---\n# {name}\n\nThis is a test skill.\n"
)
.into_bytes()
}
fn allow_all_policy() -> NetworkPolicy {
NetworkPolicy {
default: DecisionToml::Allow,
allow: Vec::new(),
deny: Vec::new(),
audit: false,
}
}
fn deny_all_policy() -> NetworkPolicy {
NetworkPolicy {
default: DecisionToml::Deny,
allow: Vec::new(),
deny: Vec::new(),
audit: false,
}
}
fn prompt_all_policy() -> NetworkPolicy {
NetworkPolicy {
default: DecisionToml::Prompt,
allow: Vec::new(),
deny: Vec::new(),
audit: false,
}
}
/// Spawn a tiny HTTP server that serves `bytes` at any path with 200 OK and
/// returns the bound URL. The server replies to *every* request (we re-use it
/// across multiple installs in the same test).
fn spawn_tarball_server(
bytes: Vec<u8>,
) -> (
String,
std::sync::mpsc::Sender<()>,
std::thread::JoinHandle<()>,
) {
let server = Server::http("127.0.0.1:0").expect("bind ephemeral port");
let url = format!(
"http://{}/skill.tar.gz",
server.server_addr().to_ip().expect("ip addr")
);
let (shutdown_tx, shutdown_rx) = std::sync::mpsc::channel::<()>();
let handle = std::thread::spawn(move || {
loop {
// Poll-style with a small recv timeout so we can break out cleanly.
match server.recv_timeout(std::time::Duration::from_millis(100)) {
Ok(Some(req)) => {
if req.method() != &Method::Get {
continue;
}
let response = Response::from_data(bytes.clone());
let _ = req.respond(response);
}
Ok(None) => {}
Err(_) => break,
}
if shutdown_rx.try_recv().is_ok() {
break;
}
}
});
(url, shutdown_tx, handle)
}
fn shutdown(tx: std::sync::mpsc::Sender<()>, handle: std::thread::JoinHandle<()>) {
let _ = tx.send(());
let _ = handle.join();
}
#[tokio::test]
async fn install_happy_path_writes_skill_and_marker() {
let tarball = make_tarball(&[
(
"test-skill-main/SKILL.md",
&skill_md("test-skill", "Test skill"),
),
("test-skill-main/notes.txt", b"hello world"),
]);
let (url, tx, handle) = spawn_tarball_server(tarball);
let tmp = TempDir::new().unwrap();
let policy = allow_all_policy();
let outcome = install::install(
InstallSource::DirectUrl(url),
tmp.path(),
install::DEFAULT_MAX_SIZE_BYTES,
&policy,
false,
)
.await
.expect("install ok");
let installed = match outcome {
InstallOutcome::Installed(s) => s,
other => panic!("expected Installed, got {other:?}"),
};
assert_eq!(installed.name, "test-skill");
let installed_dir = tmp.path().join("test-skill");
assert!(installed_dir.is_dir(), "skill dir created");
assert!(installed_dir.join("SKILL.md").is_file(), "SKILL.md present");
assert!(
installed_dir.join("notes.txt").is_file(),
"extra file present"
);
assert!(
installed_dir.join(install::INSTALLED_FROM_MARKER).is_file(),
".installed-from marker present"
);
shutdown(tx, handle);
}
#[tokio::test]
async fn install_rejects_path_traversal() {
// `tar::Builder::append_data` rejects `..` itself, so we craft the bad
// entry by writing the raw header bytes via `append`.
let mut gz = GzEncoder::new(Vec::new(), Compression::default());
{
let mut builder = tar::Builder::new(&mut gz);
let body = skill_md("test-skill", "T");
let mut hdr = tar::Header::new_gnu();
hdr.set_size(body.len() as u64);
hdr.set_mode(0o644);
hdr.set_cksum();
builder
.append_data(&mut hdr, "test-skill-main/SKILL.md", body.as_slice())
.unwrap();
// Path-traversal entry. The `tar` crate's `set_path` rejects `..`
// itself, so we patch the raw 100-byte name field in the header.
let evil_body: &[u8] = b"not gonna happen";
let mut evil_hdr = tar::Header::new_gnu();
evil_hdr.set_size(evil_body.len() as u64);
evil_hdr.set_mode(0o644);
// Write a name with a `..` directly into the legacy "name" field.
let bytes = evil_hdr.as_old_mut();
let evil_name = b"../etc/passwd";
bytes.name[..evil_name.len()].copy_from_slice(evil_name);
evil_hdr.set_cksum();
builder.append(&evil_hdr, evil_body).unwrap();
builder.finish().unwrap();
}
let tarball = gz.finish().unwrap();
let (url, tx, handle) = spawn_tarball_server(tarball);
let tmp = TempDir::new().unwrap();
let policy = allow_all_policy();
let err = install::install(
InstallSource::DirectUrl(url),
tmp.path(),
install::DEFAULT_MAX_SIZE_BYTES,
&policy,
false,
)
.await
.expect_err("path traversal must be rejected");
let msg = format!("{err:#}");
assert!(
msg.contains("escapes destination"),
"expected path-traversal error, got: {msg}"
);
shutdown(tx, handle);
}
#[tokio::test]
async fn install_rejects_oversized_tarball() {
let big = vec![b'a'; 256 * 1024]; // 256 KiB per file
let mut entries: Vec<(String, Vec<u8>)> = Vec::new();
entries.push((
"test-skill-main/SKILL.md".to_string(),
skill_md("test-skill", "T"),
));
for i in 0..50 {
entries.push((format!("test-skill-main/big-{i}.bin"), big.clone()));
}
let entry_refs: Vec<(&str, &[u8])> = entries
.iter()
.map(|(p, b)| (p.as_str(), b.as_slice()))
.collect();
let tarball = make_tarball(&entry_refs);
let (url, tx, handle) = spawn_tarball_server(tarball);
let tmp = TempDir::new().unwrap();
let policy = allow_all_policy();
let small_cap = 1024 * 1024;
let err = install::install(
InstallSource::DirectUrl(url),
tmp.path(),
small_cap,
&policy,
false,
)
.await
.expect_err("oversized must be rejected");
let msg = format!("{err:#}");
assert!(
msg.contains("too large") || msg.contains("exceed"),
"expected size cap error, got: {msg}"
);
shutdown(tx, handle);
}
#[tokio::test]
async fn install_rejects_missing_skill_md() {
let tarball = make_tarball(&[("repo-main/README.md", b"not a skill")]);
let (url, tx, handle) = spawn_tarball_server(tarball);
let tmp = TempDir::new().unwrap();
let policy = allow_all_policy();
let err = install::install(
InstallSource::DirectUrl(url),
tmp.path(),
install::DEFAULT_MAX_SIZE_BYTES,
&policy,
false,
)
.await
.expect_err("missing SKILL.md must be rejected");
assert!(format!("{err:#}").contains("missing SKILL.md"), "{err:#}");
shutdown(tx, handle);
}
#[tokio::test]
async fn install_rejects_missing_required_frontmatter() {
let tarball = make_tarball(&[("repo-main/SKILL.md", b"---\nname: test\n---\nbody\n")]);
let (url, tx, handle) = spawn_tarball_server(tarball);
let tmp = TempDir::new().unwrap();
let policy = allow_all_policy();
let err = install::install(
InstallSource::DirectUrl(url),
tmp.path(),
install::DEFAULT_MAX_SIZE_BYTES,
&policy,
false,
)
.await
.expect_err("missing description must be rejected");
assert!(format!("{err:#}").contains("description"), "{err:#}");
shutdown(tx, handle);
}
#[tokio::test]
async fn install_idempotent_then_uninstall_then_reinstall() {
let tarball_bytes =
make_tarball(&[("repo-main/SKILL.md", &skill_md("idem-skill", "Idempotent"))]);
let (url, tx, handle) = spawn_tarball_server(tarball_bytes);
let tmp = TempDir::new().unwrap();
let policy = allow_all_policy();
install::install(
InstallSource::DirectUrl(url.clone()),
tmp.path(),
install::DEFAULT_MAX_SIZE_BYTES,
&policy,
false,
)
.await
.expect("first install ok");
// Second install with `update = false` must reject.
let err = install::install(
InstallSource::DirectUrl(url.clone()),
tmp.path(),
install::DEFAULT_MAX_SIZE_BYTES,
&policy,
false,
)
.await
.expect_err("second install must reject");
let msg = format!("{err:#}");
assert!(
msg.contains("already installed"),
"expected already-installed error, got: {msg}"
);
// Uninstall then reinstall.
install::uninstall("idem-skill", tmp.path()).expect("uninstall ok");
assert!(!tmp.path().join("idem-skill").exists());
install::install(
InstallSource::DirectUrl(url),
tmp.path(),
install::DEFAULT_MAX_SIZE_BYTES,
&policy,
false,
)
.await
.expect("reinstall ok");
assert!(tmp.path().join("idem-skill").join("SKILL.md").is_file());
shutdown(tx, handle);
}
#[tokio::test]
async fn update_no_change_returns_nochange_without_overwriting() {
let tarball_bytes =
make_tarball(&[("repo-main/SKILL.md", &skill_md("upd-skill", "Update test"))]);
let (url, tx, handle) = spawn_tarball_server(tarball_bytes);
let tmp = TempDir::new().unwrap();
let policy = allow_all_policy();
install::install(
InstallSource::DirectUrl(url.clone()),
tmp.path(),
install::DEFAULT_MAX_SIZE_BYTES,
&policy,
false,
)
.await
.unwrap();
// Patch the marker so update() re-fetches the same URL.
let marker_path = tmp
.path()
.join("upd-skill")
.join(install::INSTALLED_FROM_MARKER);
let marker_body = std::fs::read_to_string(&marker_path).unwrap();
let mut marker_json: serde_json::Value = serde_json::from_str(&marker_body).unwrap();
marker_json["spec"] = serde_json::Value::String(url);
std::fs::write(&marker_path, marker_json.to_string()).unwrap();
// Capture mtime so we can confirm SKILL.md wasn't rewritten.
let skill_md_path = tmp.path().join("upd-skill").join("SKILL.md");
let mtime_before = std::fs::metadata(&skill_md_path)
.unwrap()
.modified()
.unwrap();
let result = install::update(
"upd-skill",
tmp.path(),
install::DEFAULT_MAX_SIZE_BYTES,
&policy,
)
.await
.expect("update ok");
assert!(matches!(result, UpdateResult::NoChange));
let mtime_after = std::fs::metadata(&skill_md_path)
.unwrap()
.modified()
.unwrap();
assert_eq!(mtime_before, mtime_after, "SKILL.md must not be rewritten");
shutdown(tx, handle);
}
#[tokio::test]
async fn install_with_deny_policy_returns_network_denied() {
let tmp = TempDir::new().unwrap();
let policy = deny_all_policy();
let outcome = install::install(
InstallSource::DirectUrl("https://example.invalid/skill.tar.gz".to_string()),
tmp.path(),
install::DEFAULT_MAX_SIZE_BYTES,
&policy,
false,
)
.await
.expect("policy outcome should be Ok");
match outcome {
InstallOutcome::NetworkDenied(host) => {
assert!(host.contains("example.invalid"), "got host {host}");
}
other => panic!("expected NetworkDenied, got {other:?}"),
}
// Verify the temp dir is untouched.
assert!(
std::fs::read_dir(tmp.path()).unwrap().next().is_none(),
"temp dir must be untouched"
);
}
#[tokio::test]
async fn install_with_prompt_policy_returns_needs_approval() {
let tmp = TempDir::new().unwrap();
let policy = prompt_all_policy();
let outcome = install::install(
InstallSource::DirectUrl("https://example.invalid/skill.tar.gz".to_string()),
tmp.path(),
install::DEFAULT_MAX_SIZE_BYTES,
&policy,
false,
)
.await
.expect("policy outcome should be Ok");
match outcome {
InstallOutcome::NeedsApproval(host) => {
assert!(host.contains("example.invalid"), "got host {host}");
}
other => panic!("expected NeedsApproval, got {other:?}"),
}
assert!(
std::fs::read_dir(tmp.path()).unwrap().next().is_none(),
"temp dir must be untouched on prompt"
);
}
#[tokio::test]
async fn install_rejects_symlink_entry() {
let mut gz = GzEncoder::new(Vec::new(), Compression::default());
{
let mut builder = tar::Builder::new(&mut gz);
let body = skill_md("link-skill", "x");
let mut hdr = tar::Header::new_gnu();
hdr.set_size(body.len() as u64);
hdr.set_mode(0o644);
hdr.set_cksum();
builder
.append_data(&mut hdr, "repo-main/SKILL.md", body.as_slice())
.unwrap();
let mut link_hdr = tar::Header::new_gnu();
link_hdr.set_entry_type(tar::EntryType::Symlink);
link_hdr.set_size(0);
link_hdr.set_mode(0o777);
builder
.append_link(&mut link_hdr, "repo-main/escape", Path::new("/etc/passwd"))
.unwrap();
builder.finish().unwrap();
}
let tarball = gz.finish().unwrap();
let (url, tx, handle) = spawn_tarball_server(tarball);
let tmp = TempDir::new().unwrap();
let policy = allow_all_policy();
let err = install::install(
InstallSource::DirectUrl(url),
tmp.path(),
install::DEFAULT_MAX_SIZE_BYTES,
&policy,
false,
)
.await
.expect_err("symlinks must be rejected");
assert!(format!("{err:#}").contains("symlink"), "{err:#}");
shutdown(tx, handle);
}
#[test]
fn uninstall_refuses_system_skill() {
let tmp = TempDir::new().unwrap();
let dir = tmp.path().join("system-skill");
std::fs::create_dir_all(&dir).unwrap();
let mut f = std::fs::File::create(dir.join("SKILL.md")).unwrap();
f.write_all(b"---\nname: system-skill\ndescription: x\n---\n")
.unwrap();
// No `.installed-from` marker — looks like a system skill.
let err = install::uninstall("system-skill", tmp.path()).expect_err("must refuse");
assert!(format!("{err:#}").contains("not installed via"));
assert!(dir.exists(), "directory must be left alone");
}