diff --git a/Cargo.lock b/Cargo.lock index 0a5b368fb..525e7b489 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -698,6 +698,8 @@ dependencies = [ "libc", "once_cell", "regex", + "ring", + "serde", "serde_json", "sha2", "ureq", diff --git a/crates/skills/Cargo.toml b/crates/skills/Cargo.toml index 7a8d76dbd..0ac1095ae 100644 --- a/crates/skills/Cargo.toml +++ b/crates/skills/Cargo.toml @@ -10,9 +10,11 @@ impeccable-common = { path = "../common" } impeccable-context = { path = "../context" } impeccable-detect = { path = "../detect" } serde_json = { workspace = true } +serde = { workspace = true } regex = { workspace = true } once_cell = { workspace = true } sha2 = "0.10" +ring = "0.17.14" ureq = { version = "2", default-features = false, features = ["tls", "json"] } url = "2" zip = { version = "2", default-features = false, features = ["deflate"] } diff --git a/crates/skills/src/bundle.rs b/crates/skills/src/bundle.rs index 7eb476b31..3c388ee73 100644 --- a/crates/skills/src/bundle.rs +++ b/crates/skills/src/bundle.rs @@ -17,6 +17,7 @@ use crate::providers::{ opencode_global_config_dir, provider_display_name, Scope, Sys, API_BASE, PROVIDER_DIRS, }; use crate::util::{self, jsp}; +use crate::bundle_signature::{self, TrustedKeys, MAX_SIGNATURE_BYTES}; /// Ceiling on any single download this crate performs (triage C4). The /// launcher-only universal bundle is under 25 MB (the Cloudflare Pages file @@ -80,6 +81,7 @@ pub struct FetchResponse { fn ureq_fetch(url: &str) -> Result { let agent = ureq::AgentBuilder::new() .timeout_connect(std::time::Duration::from_secs(30)) + .timeout(std::time::Duration::from_secs(120)) .redirects(0) .build(); match agent.get(url).call() { @@ -293,13 +295,48 @@ pub fn download_and_extract_bundle(sys: &Sys) -> Result { if let Some(local) = sys.env.get("IMPECCABLE_BUNDLE_PATH").filter(|v| !v.is_empty()) { return copy_or_extract_local_bundle(sys, local); } + download_remote_bundle(sys, &mut ureq_fetch, bundle_signature::trusted_keys()) +} + +fn download_remote_bundle( + sys: &Sys, + fetch: &mut dyn FnMut(&str) -> Result, + keys: Result, +) -> Result { + keys.and_then(|keys| download_and_extract_signed_bundle(sys, fetch, &keys)) + .map_err(|e| format!("{}{e}. Nothing was installed; retry or update the CLI. If this persists, report it at https://github.com/pbakaus/impeccable/issues/479", bundle_signature::ERROR_PREFIX)) +} + +fn download_and_extract_signed_bundle( + sys: &Sys, + fetch: &mut dyn FnMut(&str) -> Result, + keys: &TrustedKeys, +) -> Result { let tmp = util::tmpdir(&sys.env); let staging = util::mkdtemp(&jsp::join(&[&tmp, "impeccable-update-"]))?; let tmp_zip = jsp::join(&[&staging, "bundle.zip"]); + let tmp_signature = jsp::join(&[&staging, "bundle.sig.json"]); let result = (|| -> Result<(), String> { - download_file(&format!("{API_BASE}/api/download/bundle/universal"), &tmp_zip)?; - extract_zip_file(&tmp_zip, &staging, &sys.cwd)?; + // Resolve once, then request both assets from that exact release. Never + // pair a latest-version lookup with a independently changing ZIP URL. + let response = fetch(&format!("{API_BASE}/api/download/bundle/universal"))?; + if !matches!(response.status, 301 | 302 | 303 | 307 | 308) { + return Err(format!("Expected a signed bundle release redirect (HTTP {})", response.status)); + } + let location = response.location.ok_or("Missing bundle release redirect")?; + let version = bundle_signature::release_version(&location)?; + download_file_capped(&format!("{location}.sig.json"), &tmp_signature, fetch, MAX_SIGNATURE_BYTES)?; + download_file_with(&location, &tmp_zip, fetch)?; + let signature = std::fs::read(&tmp_signature).map_err(|e| e.to_string())?; + let file = std::fs::File::open(&tmp_zip).map_err(|e| e.to_string())?; + let mut reader = std::io::BufReader::new(file); + bundle_signature::verify_reader(&mut reader, &signature, &version, keys)?; + // Reuse the verified file handle rather than reopening by pathname. + use std::io::Seek; + reader.rewind().map_err(|e| e.to_string())?; + extract_zip_from(reader, &staging, &sys.cwd)?; util::rm_rf(&tmp_zip); + util::rm_rf(&tmp_signature); Ok(()) })(); match result { @@ -931,4 +968,109 @@ mod tests { assert_eq!(normalize_for_hash("x .claude/skills/y .trae-cn/skills/z .agent/skills/"), "x .PROVIDER/skills/y .PROVIDER/skills/z .PROVIDER/skills/"); assert_eq!(normalize_for_hash(".other/skills/"), ".other/skills/"); } + + #[test] + fn keyring_load_failure_is_fatal_before_any_download() { + let sys = Sys::new(Default::default(), "/".into()); + let mut fetch = |_: &str| -> Result { + panic!("A failed keyring must never reach the network"); + }; + let error = download_remote_bundle(&sys, &mut fetch, Err("Invalid compiled bundle signing keyring".into())).unwrap_err(); + assert!(error.starts_with(bundle_signature::ERROR_PREFIX), "{error}"); + assert!(error.contains("Invalid compiled bundle signing keyring"), "{error}"); + } + + #[test] + fn release_resolution_accepts_standard_redirects_only() { + for status in [200, 300, 301, 302, 303, 304, 305, 306, 307, 308, 404] { + let root = tmp_dir(&format!("redirect-{status}")); + let sys = Sys::new([("TMPDIR".into(), root.clone()), ("TEMP".into(), root.clone())].into(), root.clone()); + let mut requests = 0; + let mut fetch = |_: &str| -> Result { + requests += 1; + if requests > 1 { return Err("reached signature download".into()); } + Ok(FetchResponse { + status, + location: Some("https://github.com/pbakaus/impeccable/releases/download/skill-v4.2.0/universal.zip".into()), + body: Box::new(std::io::empty()), + }) + }; + let error = download_and_extract_signed_bundle(&sys, &mut fetch, &Default::default()).unwrap_err(); + if matches!(status, 301 | 302 | 303 | 307 | 308) { + assert_eq!(error, "reached signature download", "HTTP {status}"); + assert_eq!(requests, 2); + } else { + assert!(error.contains("Expected a signed bundle release redirect"), "{error}"); + assert_eq!(requests, 1); + } + assert_eq!(std::fs::read_dir(&root).unwrap().count(), 0); + util::rm_rf(&root); + } + } + + #[test] + fn signed_download_verifies_before_extraction_and_cleans_all_failures() { + use ring::signature::{Ed25519KeyPair, KeyPair}; + let key = Ed25519KeyPair::from_seed_unchecked(&[7; 32]).unwrap(); + let hex = |bytes: &[u8]| bytes.iter().map(|b| format!("{b:02x}")).collect::(); + let keys = [("test-only".into(), hex(key.public_key().as_ref()))].into(); + let zip = zip_bytes(&[(".claude/skills/impeccable/SKILL.md", b"verified skill")]); + let digest = format!("{:x}", Sha256::digest(&zip)); + let payload = format!("impeccable-skill-bundle-v1\ntest-only\nskill-v4.2.0\nuniversal.zip\n{}\n{digest}\n", zip.len()); + let signature = serde_json::to_vec(&serde_json::json!({ + "schema": 1, "keyId": "test-only", "version": "4.2.0", "artifact": "universal.zip", + "size": zip.len(), "sha256": digest, "signature": hex(key.sign(payload.as_bytes()).as_ref()), + })).unwrap(); + let release = "https://github.com/pbakaus/impeccable/releases/download/skill-v4.2.0/universal.zip"; + for case in ["valid", "tampered", "missing", "oversized", "downgrade", "malformed-zip", "invalid-signature"] { + let root = tmp_dir(case); + let temp = format!("{root}/temp"); + std::fs::create_dir(&temp).unwrap(); + let installed = format!("{root}/existing-skill.md"); + std::fs::write(&installed, "user's existing skill").unwrap(); + let sys = Sys::new([("TMPDIR".into(), temp.clone()), ("TEMP".into(), temp.clone())].into(), root.clone()); + let mut requested = Vec::new(); + let mut fetch = |url: &str| -> Result { + requested.push(url.to_string()); + let mut res = FetchResponse { status: 200, location: None, body: Box::new(std::io::Cursor::new(Vec::new())) }; + if url.ends_with("/api/download/bundle/universal") { + res.status = 302; + res.location = Some(release.into()); + } else if url == format!("{release}.sig.json") { + res.body = Box::new(std::io::Cursor::new(signature.clone())); + match case { + "missing" => res.status = 404, + "oversized" => res.body = Box::new(std::io::repeat(b' ')), + "downgrade" => { res.status = 302; res.location = Some("http://unsafe.test/sig".into()); } + "invalid-signature" => res.body = Box::new(std::io::Cursor::new(b"{}".to_vec())), + _ => {} + } + } else if url == release { + let mut bytes = zip.clone(); + if case == "tampered" { bytes[0] ^= 1; } + if case == "malformed-zip" { bytes = b"not even a ZIP".to_vec(); } + res.body = Box::new(std::io::Cursor::new(bytes)); + } else { panic!("Unexpected URL: {url}"); } + Ok(res) + }; + let result = download_and_extract_signed_bundle(&sys, &mut fetch, &keys); + if case == "valid" { + let staging = result.unwrap(); + assert_eq!(std::fs::read_to_string(format!("{staging}/.claude/skills/impeccable/SKILL.md")).unwrap(), "verified skill"); + assert!(!util::exists(&format!("{staging}/bundle.zip"))); + assert!(!util::exists(&format!("{staging}/bundle.sig.json"))); + util::rm_rf(&staging); + } else { + let error = result.unwrap_err(); + if case == "malformed-zip" { + assert!(error.contains("size"), "must reject before ZIP parsing: {error}"); + } + } + assert_eq!(std::fs::read_to_string(&installed).unwrap(), "user's existing skill"); + assert_eq!(std::fs::read_dir(&temp).unwrap().count(), 0, "staging leak in {case}"); + assert_eq!(requested[0], format!("{API_BASE}/api/download/bundle/universal")); + assert_eq!(requested[1], format!("{release}.sig.json")); + util::rm_rf(&root); + } + } } diff --git a/crates/skills/src/bundle_signature.rs b/crates/skills/src/bundle_signature.rs new file mode 100644 index 000000000..7cf37ab03 --- /dev/null +++ b/crates/skills/src/bundle_signature.rs @@ -0,0 +1,232 @@ +//! Authenticity gate for remote skill bundles. The only trust roots are the +//! public keys compiled into this binary, never anything in a download. +use once_cell::sync::Lazy; +use regex::Regex; +use ring::signature::{UnparsedPublicKey, ED25519}; +use serde::Deserialize; +use sha2::{Digest, Sha256}; +use std::{collections::BTreeMap, io::Read}; + +pub(crate) const MAX_SIGNATURE_BYTES: u64 = 16 * 1024; +pub(crate) const ERROR_PREFIX: &str = "Could not verify skill bundle: "; +pub(crate) type TrustedKeys = BTreeMap; +static VERSION: Lazy = Lazy::new(|| { + Regex::new( + r"^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?$" +).unwrap() +}); + +pub(crate) fn trusted_keys() -> Result { + serde_json::from_str(include_str!("../../../scripts/bundle-signing-keys.json")) + .map_err(|_| "Invalid compiled bundle signing keyring".into()) +} + +pub(crate) fn release_version(location: &str) -> Result { + let version = location + .strip_prefix("https://github.com/pbakaus/impeccable/releases/download/skill-v") + .and_then(|s| s.strip_suffix("/universal.zip")) + .filter(|v| v.len() <= 128 && VERSION.is_match(v)); + version.map(str::to_string).ok_or_else(|| { + "Bundle download must redirect to a versioned Impeccable GitHub release".into() + }) +} + +#[derive(Deserialize)] +#[serde(rename_all = "camelCase", deny_unknown_fields)] +struct Envelope { + schema: u32, + key_id: String, + version: String, + artifact: String, + size: u64, + sha256: String, + signature: String, +} + +fn decode_hex(value: &str, size: usize) -> Result, String> { + if value.len() != size * 2 + || !value + .bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) + { + return Err("Invalid bundle signature encoding".into()); + } + (0..value.len()) + .step_by(2) + .map(|i| u8::from_str_radix(&value[i..i + 2], 16).map_err(|_| "Invalid hex".into())) + .collect() +} + +pub(crate) fn verify_reader( + reader: &mut dyn Read, + signature: &[u8], + version: &str, + keys: &TrustedKeys, +) -> Result<(), String> { + if signature.len() as u64 > MAX_SIGNATURE_BYTES { + return Err("Bundle signature is too large".into()); + } + let envelope: Envelope = serde_json::from_slice(signature) + .map_err(|_| "Missing or malformed bundle signature".to_string())?; + if envelope.schema != 1 + || envelope.version != version + || !VERSION.is_match(version) + || envelope.artifact != "universal.zip" + || envelope.size == 0 + || envelope.size > crate::bundle::MAX_DOWNLOAD_BYTES + { + return Err("Bundle signature metadata does not match the requested release".into()); + } + let public_key = keys + .get(&envelope.key_id) + .ok_or("Unknown bundle signing key; update the Impeccable CLI and retry")?; + let public_key = decode_hex(public_key, 32)?; + let signature = decode_hex(&envelope.signature, 64)?; + decode_hex(&envelope.sha256, 32)?; + let payload = format!( + "impeccable-skill-bundle-v1\n{}\nskill-v{}\n{}\n{}\n{}\n", + envelope.key_id, envelope.version, envelope.artifact, envelope.size, envelope.sha256 + ); + UnparsedPublicKey::new(&ED25519, public_key) + .verify(payload.as_bytes(), &signature) + .map_err(|_| "Bundle signature verification failed".to_string())?; + + let mut hash = Sha256::new(); + let mut size = 0u64; + let mut buffer = [0u8; 64 * 1024]; + loop { + let count = reader.read(&mut buffer).map_err(|e| e.to_string())?; + if count == 0 { + break; + } + size += count as u64; + if size > envelope.size { + return Err("Bundle size does not match its signature".into()); + } + hash.update(&buffer[..count]); + } + if size != envelope.size || format!("{:x}", hash.finalize()) != envelope.sha256 { + return Err("Bundle digest or size does not match its signature".into()); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use ring::signature::{Ed25519KeyPair, KeyPair}; + + #[test] + #[ignore = "Set IMPECCABLE_VERIFY_BUNDLE and IMPECCABLE_VERIFY_BUNDLE_VERSION to a reviewed release ZIP"] + fn verifies_reviewed_release_with_production_keyring() { + let path = std::env::var("IMPECCABLE_VERIFY_BUNDLE").unwrap(); + let version = std::env::var("IMPECCABLE_VERIFY_BUNDLE_VERSION").unwrap(); + let signature = std::fs::read(format!("{path}.sig.json")).unwrap(); + let mut file = std::fs::File::open(path).unwrap(); + verify_reader(&mut file, &signature, &version, &trusted_keys().unwrap()).unwrap(); + } + + #[test] + fn verifies_node_interoperability_vector() { + let fixture: serde_json::Value = serde_json::from_str(include_str!( + "../../../tests/fixtures/bundle-signature.json" + )) + .unwrap(); + let bundle = fixture["bundle"].as_str().unwrap().as_bytes(); + let envelope = serde_json::to_vec(&fixture["envelope"]).unwrap(); + let keys = serde_json::from_value(fixture["keys"].clone()).unwrap(); + verify_reader(&mut &bundle[..], &envelope, "4.2.0", &keys).unwrap(); + } + + fn fixture() -> (Vec, Vec, std::collections::BTreeMap) { + // A public, deterministic TEST key. Never present in the production keyring. + let key = Ed25519KeyPair::from_seed_unchecked(&[7; 32]).unwrap(); + let bundle = b"test bundle".to_vec(); + let digest = format!("{:x}", Sha256::digest(&bundle)); + let payload = format!( + "impeccable-skill-bundle-v1\ntest-only\nskill-v4.2.0\nuniversal.zip\n11\n{digest}\n" + ); + let hex = |bytes: &[u8]| bytes.iter().map(|b| format!("{b:02x}")).collect::(); + let envelope = serde_json::json!({ + "schema": 1, "keyId": "test-only", "version": "4.2.0", + "artifact": "universal.zip", "size": 11, "sha256": digest, + "signature": hex(key.sign(payload.as_bytes()).as_ref()), + }); + ( + bundle, + serde_json::to_vec(&envelope).unwrap(), + [("test-only".into(), hex(key.public_key().as_ref()))].into(), + ) + } + + #[test] + fn accepts_signed_bytes_and_rejects_tampering() { + let (bundle, envelope, keys) = fixture(); + verify_reader(&mut &bundle[..], &envelope, "4.2.0", &keys).unwrap(); + for tampered in [b"Test bundle".as_slice(), b"test bundle extra", b"test"] { + assert!(verify_reader(&mut &tampered[..], &envelope, "4.2.0", &keys).is_err()); + } + assert!(verify_reader(&mut &bundle[..], &envelope, "4.2.1", &keys).is_err()); + assert!(verify_reader(&mut &bundle[..], &envelope, "4.2.0", &Default::default()).is_err()); + } + + #[test] + fn rejects_changed_metadata_bad_encodings_and_unsigned_bundles() { + let (bundle, envelope, keys) = fixture(); + for (field, value) in [ + ("schema", serde_json::json!(2)), + ("keyId", serde_json::json!("attacker")), + ("version", serde_json::json!("4.2.1")), + ("artifact", serde_json::json!("other.zip")), + ("size", serde_json::json!(10)), + ("sha256", serde_json::json!("0".repeat(64))), + ("signature", serde_json::json!("0".repeat(128))), + ("signature", serde_json::json!("ff")), + ("sha256", serde_json::json!("g".repeat(64))), + ( + "publicKey", + serde_json::json!("never trust an embedded key"), + ), + ] { + let mut bad: serde_json::Value = serde_json::from_slice(&envelope).unwrap(); + bad[field] = value; + assert!( + verify_reader( + &mut &bundle[..], + &serde_json::to_vec(&bad).unwrap(), + "4.2.0", + &keys + ) + .is_err(), + "{field}" + ); + } + for malformed in [b"".as_slice(), b"{}", b"not json"] { + assert!(verify_reader(&mut &bundle[..], malformed, "4.2.0", &keys).is_err()); + } + let duplicate = String::from_utf8(envelope) + .unwrap() + .replacen('{', "{\"schema\":1,", 1); + assert!(verify_reader(&mut &bundle[..], duplicate.as_bytes(), "4.2.0", &keys).is_err()); + } + + #[test] + fn release_location_is_exact_and_versioned() { + let prefix = "https://github.com/pbakaus/impeccable/releases/download/"; + assert_eq!( + release_version(&format!("{prefix}skill-v4.2.0/universal.zip")).unwrap(), + "4.2.0" + ); + for bad in [ + format!("{prefix}skill-v4.2.0/other.zip"), + format!("{prefix}skill-v4.2.0/universal.zip?key=x"), + format!("{prefix}skill-v4.2.0/universal.zip#x"), + format!("{prefix}skill-v04.2.0/universal.zip"), + "http://github.com/pbakaus/impeccable/releases/download/skill-v4.2.0/universal.zip".into(), + "https://github.com/attacker/impeccable/releases/download/skill-v4.2.0/universal.zip".into(), + "https://github.com.evil.test/pbakaus/impeccable/releases/download/skill-v4.2.0/universal.zip".into(), + ] { + assert!(release_version(&bad).is_err(), "{bad}"); + } + } +} diff --git a/crates/skills/src/commands.rs b/crates/skills/src/commands.rs index 5a9313054..6e3fdfc99 100644 --- a/crates/skills/src/commands.rs +++ b/crates/skills/src/commands.rs @@ -588,7 +588,8 @@ fn install(flags: &[String], io: &mut Io) -> R<()> { match bundle::download_and_extract_bundle(&sys) { Ok(dir) => bundle_dir = Some(dir), Err(e) => { - if !missing_hook_targets.is_empty() || !missing_selected_targets.is_empty() { + if e.starts_with(crate::bundle_signature::ERROR_PREFIX) + || !missing_hook_targets.is_empty() || !missing_selected_targets.is_empty() { return Err(e); } update_check_skipped = true; diff --git a/crates/skills/src/lib.rs b/crates/skills/src/lib.rs index a24c69293..be31c3f36 100644 --- a/crates/skills/src/lib.rs +++ b/crates/skills/src/lib.rs @@ -3,8 +3,7 @@ //! `cli/bin/commands/skills.mjs` (plus the slice of `cli/lib/impeccable-config.mjs` //! it imports). //! -//! Two deliberate departures from the JS, both part of the release that -//! replaces the Node scripts with the binary: +//! Deliberate departures from the original JS behavior: //! //! 1. After a skill directory is written (fresh install, refresh, update), if //! its `scripts/VERSION` exists and `scripts/bin/-/impeccable` @@ -19,11 +18,16 @@ //! (`impeccable_hook::admin`) writes, and both paths recognize a manifest //! entry as ours through `impeccable_context::hook_markers`, so the two //! never drift on detection. See `hook_manifest`. +//! 3. Remote skill ZIPs require an Ed25519 signature from a compiled-in key +//! before extraction. Failure is fatal even when an existing install is +//! present. Explicit local bundle overrides remain unsigned development +//! inputs. See `bundle_signature` and docs/BUNDLE-SIGNING.md. //! //! Everything else (messages, exit codes, endpoints, flags, prompts, file //! layout) follows the JS byte for byte. pub mod bundle; +mod bundle_signature; pub mod commands; pub mod engine_binary; pub mod hook_manifest; diff --git a/docs/BUNDLE-SIGNING.md b/docs/BUNDLE-SIGNING.md new file mode 100644 index 000000000..0bab7ede2 --- /dev/null +++ b/docs/BUNDLE-SIGNING.md @@ -0,0 +1,117 @@ +# Skill bundle signatures + +`impeccable install`, `update`, and `check` authenticate a remote skill ZIP +before extracting it. `universal.zip.sig.json` is an Ed25519 signature over +the ZIP's SHA-256 digest, byte length, release version, artifact name, and key +ID. The engine trusts only `scripts/bundle-signing-keys.json`, compiled into +the binary. A signature cannot introduce a new trusted key. + +The download endpoint on impeccable.style redirects to a versioned GitHub +release. The installer resolves that redirect once and downloads the ZIP and +its signature from that same release. Every subsequent redirect must use +HTTPS. Missing signatures, unknown keys, changed metadata, and changed ZIP +bytes stop the operation before extraction or writes to installed skills. +The temporary download directory is removed on failure. + +## Sign a release + +Install the 1Password CLI and enable its desktop app integration. The signing +item holds the PKCS#8 Ed25519 private key in a concealed `private-key` field. +Set references, not key material, in your shell: + +```sh +export OP_ACCOUNT='' +export IMPECCABLE_SIGNING_KEY_REF='op:////private-key' +bun run release:skill +``` + +For a persistent setup on your machine, use local Git settings instead: + +```sh +git config --local impeccable.signingAccount '' +git config --local impeccable.signingKeyRef 'op:////private-key' +``` + +Those values stay in `.git/config`, outside version control. Environment +variables take precedence. Neither setting contains the private key. + +The release command rebuilds the ZIP, reads the key through `op read`, checks +that its public key is trusted, and writes the sidecar before creating any +tag or release. The ZIP and sidecar are uploaded together. The key is never +passed as a command argument, written to a temporary file, or printed. It +does exist briefly in the local signing process's memory. 1Password failures +are reported without forwarding child-process output. + +`--dry-run` does not access 1Password or create a signature. It checks the +usual release prerequisites and shows both assets in the upload plan; it +does not prove that signing credentials work. + +To sign an already-published release for the initial rollout, download and +review the exact released `universal.zip`, then run: + +```sh +node scripts/sign-bundle.mjs 4.2.0 /path/to/universal.zip +``` + +Check the resulting sidecar against the Rust verifier and compiled public key: + +```sh +IMPECCABLE_VERIFY_BUNDLE=/path/to/universal.zip \ +IMPECCABLE_VERIFY_BUNDLE_VERSION=4.2.0 \ +cargo test -p impeccable-skills verifies_reviewed_release_with_production_keyring -- --ignored +``` + +This creates only the local sidecar. It neither uploads it nor replaces the +ZIP. Never regenerate an old ZIP and sign those different bytes as the old +release. Uploading the sidecar is a separate maintainer approval step. + +## Rollout and rotation + +Before shipping the enforcing engine, publish a valid signature beside the +exact ZIP currently served by impeccable.style. Verify the pair using a +locally built engine, then release the engine, its npm platform packages, and +the CLI/skill pins. Keep the existing release available throughout. Do not +release an enforcing engine with an empty keyring or an unsigned served ZIP. + +For planned rotation, ship an engine trusting both the old and new public +keys before signing with the new key. Older engines that do not know the new +key will refuse the download and ask for a CLI update. A compromised key +requires an engine update removing that public key; removing it from a +website does not revoke trust in already-installed binaries. Keep the +dedicated signing item separate from GitHub and deployment credentials. + +## Scope + +This protects against bundle substitution when an attacker can change the +download endpoint, release asset, or both, but cannot use the signing key or +replace the trusted engine. It is not a freshness protocol: a previously +signed release can still be replayed. Signed timestamp metadata and rollback +state are separate work. Signatures do not establish that authored skill +content is safe, and do not authenticate separately downloaded engine +binaries (those currently use their existing SHA-256 sidecars). + +`IMPECCABLE_BUNDLE_PATH` and `impeccable link` are explicit local-development +trust paths. They continue to accept unsigned local files/directories. Do not +use those overrides to get around a failed remote verification. There is no +unsigned-network fallback or skip-signature flag. + +## Wire format + +JSON sidecar fields: `schema` (1), `keyId`, `version`, `artifact` +(`universal.zip`), `size`, `sha256`, `signature`. Hex strings are lowercase; +the public key is 32 bytes and the signature is 64 bytes. Unknown or repeated +fields are rejected. The signature payload is UTF-8 with LF line endings +and a final LF: + +```text +impeccable-skill-bundle-v1 + +skill-v +universal.zip + + +``` + +The Node signer and Rust verifier share a fixed test vector under +`tests/fixtures/bundle-signature.json`. Its deterministic test key must never +be added to the production keyring. diff --git a/docs/CLI-CONTRACT.md b/docs/CLI-CONTRACT.md index b414ba53d..35651349a 100644 --- a/docs/CLI-CONTRACT.md +++ b/docs/CLI-CONTRACT.md @@ -342,6 +342,16 @@ Optional keys added later by engines (appended after the above): `ignoreValue` ( #### `impeccable help|install|link|update|check` and `impeccable skills ` (`cli/bin/commands/skills.mjs`) +**Rust authenticity addition (#479):** the historical JS bundle flow below +is superseded for remote downloads. The Rust installer resolves the site's +redirect (301/302/303/307/308) to a versioned Impeccable GitHub release and verifies +`universal.zip.sig.json` against a compiled-in Ed25519 public key before ZIP +extraction. Missing/invalid signatures, unknown keys, mismatched metadata or +content, and failures fetching either asset exit nonzero, including when +`install` finds an existing installation. No downloaded content reaches the +installed skill or hook files. Explicit `IMPECCABLE_BUNDLE_PATH` and `link` +retain their local-development trust behavior. See [bundle signing](BUNDLE-SIGNING.md). + - **Invoked from**: README.md ("npx impeccable install / update"), README.npm.md Quick Start (`npx impeccable skills install`, `... install -y --providers=claude,codex --scope=project`, `... update`, `... install --no-hooks`, `... link --source=.impeccable --providers=claude,cursor`, `... skills help`), `README.md:360` (hook consent explanation). - `run(args)`: `args[0]` ∈ `undefined|help|--help|-h` → `showHelp()`; `install` → `install(rest)`; `link`; `update`; `check` (ignores flags); else `stderr> Unknown skills command: ${sub}` + `Run 'impeccable --help' for available commands.`, `exit 1`. - Constants: `API_BASE = 'https://impeccable.style'`; `PROVIDER_DIRS = ['.claude','.cursor','.gemini','.agents','.agent','.github','.grok','.hermes','.kiro','.opencode','.pi','.qoder','.trae','.trae-cn','.rovodev','.vibe']`; aliases (`agent`→`.agent`, `agents`/`codex`→`.agents`, `antigravity`→`.agent`, `claude`/`claude-code`→`.claude`, `copilot`/`github`→`.github`, `cursor`, `gemini`, `grok`/`grok-build`/`xai`→`.grok`, `hermes`, `kiro`, `opencode`, `pi`, `qoder`, `rovo-dev`/`rovodev`→`.rovodev`, `trae`, `trae-cn`, `vibe`); leading `.` stripped and lowercased before alias lookup; a literal PROVIDER_DIR value is accepted as-is. `DEFAULT_TARGETS = ['.claude','.agents']`. User-scope skill dir overrides: `.agent`→`~/.gemini/config/skills`, `.hermes`→`$HERMES_HOME/skills` (only when HERMES_HOME under home) else `~/.hermes/skills`, `.pi`→`~/.pi/agent/skills`, `.opencode`→`$OPENCODE_CONFIG_DIR|$XDG_CONFIG_HOME/opencode|~/.config/opencode` + `/skills`; others `~//skills`. Project scope: `//skills`. diff --git a/docs/ENGINE.md b/docs/ENGINE.md index 1ab4351eb..9a44ad42a 100644 --- a/docs/ENGINE.md +++ b/docs/ENGINE.md @@ -232,6 +232,10 @@ this feature, replacing the `detectText` call it makes into the npm ## Releases +Remote skill ZIPs require a pinned-key signature before extraction. See +[bundle signing](BUNDLE-SIGNING.md) for the 1Password setup and the required +signature-first rollout order. + Two release kinds touch the runtime, in this order: 1. **Engine** (`engine-v`): `bun run release:engine` verifies diff --git a/scripts/bundle-signing-keys.json b/scripts/bundle-signing-keys.json new file mode 100644 index 000000000..b257d6121 --- /dev/null +++ b/scripts/bundle-signing-keys.json @@ -0,0 +1,3 @@ +{ + "release-2026-09": "7433133bb92da2c0da4186925f36dbd36219c11192451df56f25fd6a23dd7db9" +} diff --git a/scripts/release.mjs b/scripts/release.mjs index 7e84414a6..1b6f6a92a 100755 --- a/scripts/release.mjs +++ b/scripts/release.mjs @@ -19,6 +19,7 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { checkEngineRelease } from './check-engine-release.mjs'; import { readEngineVersion } from './fetch-engine.mjs'; +import { signReleaseBundle } from './sign-bundle.mjs'; const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); @@ -255,6 +256,25 @@ for (const artifact of cfg.artifacts) { ok(artifact); } +// Sign the final rebuilt bytes before any tag or upload. Dry runs do not +// unlock 1Password or write a signature; they only show the publishing plan. +if (component === 'skill') { + const signatureArtifact = 'dist/universal.zip.sig.json'; + step('Signing universal.zip with the trusted 1Password release key'); + if (dryRun) { + console.log(' [dry-run] Sign dist/universal.zip (1Password is not accessed)'); + } else { + try { + signReleaseBundle({ zipPath: path.join(repoRoot, 'dist/universal.zip'), version }); + } catch (error) { + fail(error.message); + } + if (!existsSync(path.join(repoRoot, signatureArtifact))) fail(`Missing artifact: ${signatureArtifact}`); + ok('signature verified locally'); + } + cfg.artifacts.push(signatureArtifact); +} + console.log('\n--- Release notes preview ---'); console.log(notes); console.log('--- end preview ---\n'); diff --git a/scripts/sign-bundle.mjs b/scripts/sign-bundle.mjs new file mode 100644 index 000000000..4e76d675d --- /dev/null +++ b/scripts/sign-bundle.mjs @@ -0,0 +1,105 @@ +#!/usr/bin/env node +// Local release signing. Private key material travels from op through a pipe +// into crypto, never through argv, environment values, logs or temporary files. +import { createHash, createPrivateKey, createPublicKey, sign, verify } from 'node:crypto'; +import { execFileSync } from 'node:child_process'; +import { readFileSync, writeFileSync, statSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import path from 'node:path'; + +export const VERSION_PATTERN = /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?(?:\+[0-9A-Za-z-]+(?:\.[0-9A-Za-z-]+)*)?$/; +const MAX_BUNDLE_BYTES = 256 * 1024 * 1024; +const repoRoot = fileURLToPath(new URL('../', import.meta.url)); + +function localSetting(name) { + try { + return execFileSync('git', ['config', '--local', '--get', name], { + cwd: repoRoot, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], + }).trim(); + } catch { return undefined; } +} + +export function publicKeyHex(key) { + if (key.asymmetricKeyType !== 'ed25519') throw new Error('Signing requires an Ed25519 key.'); + const jwk = key.export({ format: 'jwk' }); + return Buffer.from(jwk.x, 'base64url').toString('hex'); +} + +// Shared wire format with crates/skills/src/bundle_signature.rs. UTF-8, LF, +// trailing LF. Sign fields explicitly so JSON whitespace/order is irrelevant. +export function signaturePayload({ keyId, version, artifact, size, sha256 }) { + return Buffer.from(`impeccable-skill-bundle-v1\n${keyId}\nskill-v${version}\n${artifact}\n${size}\n${sha256}\n`); +} + +export function readTrustedKeys() { + return JSON.parse(readFileSync(new URL('./bundle-signing-keys.json', import.meta.url), 'utf8')); +} + +export function signBundle(bytes, version, privateKey, trustedKeys) { + if (typeof version !== 'string' || version.length > 128 || !VERSION_PATTERN.test(version)) throw new Error('Invalid skill version for signing.'); + if (!bytes.length || bytes.length > MAX_BUNDLE_BYTES) throw new Error('Invalid bundle size for signing.'); + const publicKey = createPublicKey(privateKey); + const publicHex = publicKeyHex(publicKey); + const keyId = Object.keys(trustedKeys).find(id => trustedKeys[id] === publicHex); + if (!keyId || !/^[a-z0-9-]{1,64}$/.test(keyId)) { + throw new Error('The signing key is not in the trusted bundle keyring.'); + } + const envelope = { + schema: 1, keyId, version, artifact: 'universal.zip', size: bytes.length, + sha256: createHash('sha256').update(bytes).digest('hex'), + }; + const payload = signaturePayload(envelope); + const signature = sign(null, payload, privateKey); + if (!verify(null, payload, publicKey, signature)) throw new Error('Signature self-check failed.'); + return { ...envelope, signature: signature.toString('hex') }; +} + +function readFrom1Password(reference, account) { + return execFileSync('op', ['read', reference, '--no-newline', ...(account ? ['--account', account] : [])], { + stdio: ['ignore', 'pipe', 'pipe'], timeout: 120000, maxBuffer: 16384, + }); +} + +export function signReleaseBundle({ zipPath, version, trustedKeys = readTrustedKeys(), + secretReference = process.env.IMPECCABLE_SIGNING_KEY_REF ?? localSetting('impeccable.signingKeyRef'), + account = process.env.OP_ACCOUNT ?? localSetting('impeccable.signingAccount'), readSecret = readFrom1Password }) { + if (!secretReference?.startsWith('op://')) { + throw new Error('Set IMPECCABLE_SIGNING_KEY_REF to the 1Password private-key reference (op://vault/item/field).'); + } + if (typeof version !== 'string' || version.length > 128 || !VERSION_PATTERN.test(version)) throw new Error('Invalid skill version for signing.'); + if (statSync(zipPath).size > MAX_BUNDLE_BYTES) throw new Error('Invalid bundle size for signing.'); + const bytes = readFileSync(zipPath); + let pem; + try { + const secret = readSecret(secretReference, account); + pem = Buffer.isBuffer(secret) ? secret : Buffer.from(secret); + } catch { + // Child-process exceptions can contain stdout/stderr. Never propagate them. + throw new Error('Could not read the signing key from 1Password. Check CLI integration and unlock the vault.'); + } + let privateKey; + try { + privateKey = createPrivateKey(pem); + } catch { + throw new Error('The 1Password field is not a valid PKCS#8 private key.'); + } finally { + pem.fill(0); + } + const envelope = signBundle(bytes, version, privateKey, trustedKeys); + const output = `${zipPath}.sig.json`; + writeFileSync(output, `${JSON.stringify(envelope, null, 2)}\n`); + return output; +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + try { + const [version, zipPath, ...extra] = process.argv.slice(2); + if (!zipPath || extra.length || path.basename(zipPath) !== 'universal.zip') { + throw new Error('Usage: node scripts/sign-bundle.mjs '); + } + console.log(`Signed ${signReleaseBundle({ zipPath, version })}`); + } catch (error) { + console.error(error.message); + process.exitCode = 1; + } +} diff --git a/scripts/test-suites.mjs b/scripts/test-suites.mjs index 485586d67..84226c04a 100644 --- a/scripts/test-suites.mjs +++ b/scripts/test-suites.mjs @@ -70,6 +70,7 @@ export const SUITES = { 'tests/openai-plugin.test.mjs', 'tests/process-group.test.mjs', 'tests/release.test.mjs', + 'tests/bundle-signing.test.mjs', 'tests/skill-reference.test.mjs', 'tests/readme-gitignore.test.mjs', 'tests/test-suites.test.mjs', diff --git a/tests/bundle-signing.test.mjs b/tests/bundle-signing.test.mjs new file mode 100644 index 000000000..6db8c667a --- /dev/null +++ b/tests/bundle-signing.test.mjs @@ -0,0 +1,85 @@ +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { generateKeyPairSync, createPrivateKey, createPublicKey, verify } from 'node:crypto'; +import { mkdtempSync, writeFileSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { signBundle, signaturePayload, publicKeyHex, signReleaseBundle, readTrustedKeys } from '../scripts/sign-bundle.mjs'; + +const { privateKey, publicKey } = generateKeyPairSync('ed25519'); +const trustedKeys = { 'test-only': publicKeyHex(publicKey) }; + +test('production keyring is populated and excludes the public test key', () => { + const keys = readTrustedKeys(); + const fixture = JSON.parse(readFileSync(new URL('./fixtures/bundle-signature.json', import.meta.url))); + assert.ok(Object.keys(keys).length > 0); + for (const [id, key] of Object.entries(keys)) { + assert.match(id, /^[a-z0-9-]{1,64}$/); + assert.match(key, /^[0-9a-f]{64}$/); + assert.notEqual(key, fixture.keys['test-only']); + } +}); + +test('matches the shared Node/Rust interoperability vector (public test seed)', () => { + const fixture = JSON.parse(readFileSync(new URL('./fixtures/bundle-signature.json', import.meta.url))); + const key = createPrivateKey({ + key: Buffer.concat([Buffer.from('302e020100300506032b657004220420', 'hex'), Buffer.alloc(32, 7)]), + format: 'der', type: 'pkcs8', + }); + assert.deepEqual(signBundle(Buffer.from(fixture.bundle), '4.2.0', key, fixture.keys), fixture.envelope); +}); + +test('signs the exact bundle with version, size, digest, artifact and domain bound', () => { + const bundle = Buffer.from('test bundle'); + const envelope = signBundle(bundle, '4.2.0', privateKey, trustedKeys); + assert.equal(envelope.keyId, 'test-only'); + assert.equal(envelope.version, '4.2.0'); + assert.equal(envelope.size, bundle.length); + assert.equal(envelope.artifact, 'universal.zip'); + assert.equal(envelope.schema, 1); + assert.match(signaturePayload(envelope).toString(), /^impeccable-skill-bundle-v1\n/); + assert.ok(verify(null, signaturePayload(envelope), publicKey, Buffer.from(envelope.signature, 'hex'))); + for (const changed of [ + { version: '4.2.1' }, { size: 1 }, { sha256: '0'.repeat(64) }, + { artifact: 'other.zip' }, { keyId: 'other-key' }, + ]) { + assert.equal(verify(null, signaturePayload({ ...envelope, ...changed }), publicKey, + Buffer.from(envelope.signature, 'hex')), false); + } +}); + +test('rejects unknown or non-Ed25519 keys and invalid versions', () => { + assert.throws(() => signBundle(Buffer.from('zip'), '4.2.0', privateKey, {}), /trusted/); + for (const version of ['4.2.0\nother', '../4.2.0', '', '04.2.0', '4.2']) { + assert.throws(() => signBundle(Buffer.from('zip'), version, privateKey, trustedKeys), /version/); + } + const rsa = generateKeyPairSync('rsa', { modulusLength: 2048 }); + assert.throws(() => publicKeyHex(rsa.publicKey), /Ed25519/); +}); + +test('1Password read uses a pipe, checks the pinned key, writes only a public signature', () => { + const root = mkdtempSync(path.join(tmpdir(), 'impeccable-sign-test-')); + try { + const zipPath = path.join(root, 'universal.zip'); + writeFileSync(zipPath, 'test bundle'); + const secretReference = 'op://test-vault/test-item/private-key'; + let called = false; + signReleaseBundle({ zipPath, version: '4.2.0', trustedKeys, secretReference, + readSecret(reference) { + called = true; + assert.equal(reference, secretReference); + return privateKey.export({ type: 'pkcs8', format: 'pem' }); + }, + }); + assert.ok(called); + const envelope = JSON.parse(readFileSync(`${zipPath}.sig.json`, 'utf8')); + assert.ok(verify(null, signaturePayload(envelope), createPublicKey(privateKey), + Buffer.from(envelope.signature, 'hex'))); + assert.doesNotMatch(readFileSync(`${zipPath}.sig.json`, 'utf8'), /PRIVATE KEY/); + assert.throws(() => signReleaseBundle({ zipPath, version: '4.2.0', trustedKeys, + secretReference, readSecret() { throw new Error('SECRET that must not leak'); }, + }), /Could not read.*1Password/); + } finally { + rmSync(root, { recursive: true, force: true }); + } +}); diff --git a/tests/fixtures/bundle-signature.json b/tests/fixtures/bundle-signature.json new file mode 100644 index 000000000..79fd259cb --- /dev/null +++ b/tests/fixtures/bundle-signature.json @@ -0,0 +1,15 @@ +{ + "bundle": "test bundle", + "keys": { + "test-only": "ea4a6c63e29c520abef5507b132ec5f9954776aebebe7b92421eea691446d22c" + }, + "envelope": { + "schema": 1, + "keyId": "test-only", + "version": "4.2.0", + "artifact": "universal.zip", + "size": 11, + "sha256": "9df2a47bee5f48b9752b2cbd2d6075076556ee293adc97f66f0e0a916e4f6471", + "signature": "35dfe147573341b1fdcb4bc4054b0e96c5d07b9069bd1257f745d6ad6c3eca72f52876e7d0403cffeac2e60ef2dea41879ca4cb9cecaebb463332aaf9d15620b" + } +} diff --git a/tests/release.test.mjs b/tests/release.test.mjs index 8e006e8b3..3e249d4f7 100644 --- a/tests/release.test.mjs +++ b/tests/release.test.mjs @@ -88,7 +88,7 @@ describe('release.mjs guards', () => { // (and check-engine-release.mjs imports fetch-engine.mjs), so stage them // too or the dry runs fail to resolve the modules instead of exercising // the guard. - for (const dep of ['check-engine-release.mjs', 'fetch-engine.mjs']) { + for (const dep of ['check-engine-release.mjs', 'fetch-engine.mjs', 'sign-bundle.mjs', 'bundle-signing-keys.json']) { fs.copyFileSync(path.join(REPO_ROOT, 'scripts', dep), path.join(workDir, 'scripts', dep)); } write('.claude-plugin/plugin.json', JSON.stringify({ name: 'impeccable', version: '1.2.3' })); @@ -171,6 +171,57 @@ describe('release.mjs guards', () => { assert.match(stdout, /tag is free/); assert.match(stdout, /\[dry-run\] git tag -a skill-v1\.2\.3/); assert.match(stdout, /\[dry-run\] gh release create skill-v1\.2\.3/); + assert.match(stdout, /1Password is not accessed/); + assert.match(stdout, /gh release create[^\n]+universal\.zip\.sig\.json/); + assert.equal(fs.existsSync(path.join(workDir, 'dist/universal.zip.sig.json')), false); + }); + + it('refuses a real release before tagging when signing is not configured', () => { + const pkg = JSON.parse(fs.readFileSync(path.join(workDir, 'package.json'), 'utf8')); + pkg.scripts = { 'build:release': 'node -e "process.exit(0)"' }; + write('package.json', JSON.stringify(pkg)); + git(workDir, 'add', 'package.json'); + git(workDir, 'commit', '-m', 'fixture build command'); + git(workDir, 'push', 'origin', 'main'); + assert.throws(() => execFileSync(process.execPath, ['scripts/release.mjs', 'skill'], { + cwd: workDir, encoding: 'utf8', stdio: 'pipe', + env: { ...process.env, IMPECCABLE_SKIP_ENGINE_CHECK: '1', IMPECCABLE_SIGNING_KEY_REF: '' }, + }), error => { + assert.match(error.stderr, /Set IMPECCABLE_SIGNING_KEY_REF/); + assert.doesNotMatch(error.stdout, /Creating annotated tag|Creating GitHub release/); + return true; + }); + assert.equal(git(workDir, 'tag'), ''); + assert.equal(git(workDir, 'ls-remote', '--tags', 'origin'), ''); + }); + + it('refuses before tagging when the signer returns without creating the sidecar', () => { + const pkg = JSON.parse(fs.readFileSync(path.join(workDir, 'package.json'), 'utf8')); + pkg.scripts = { 'build:release': 'node -e "process.exit(0)"' }; + write('package.json', JSON.stringify(pkg)); + // Stub only inside this disposable repository. No 1Password access, tags, + // or real GitHub publication can occur even if the assertion regresses. + write('scripts/sign-bundle.mjs', 'export function signReleaseBundle() {}\n'); + const releaseSource = fs.readFileSync(RELEASE_SCRIPT, 'utf8'); + const tagStep = 'step(`Creating annotated tag ${tag}`);'; + assert.ok(releaseSource.includes(tagStep), 'fixture must intercept the tag step'); + write('scripts/release.mjs', releaseSource.replace( + tagStep, + 'throw new Error("UNEXPECTED_TAG_STEP");' + )); + git(workDir, 'add', 'package.json', 'scripts/sign-bundle.mjs', 'scripts/release.mjs'); + git(workDir, 'commit', '-m', 'fixture signer with missing output'); + git(workDir, 'push', 'origin', 'main'); + assert.throws(() => execFileSync(process.execPath, ['scripts/release.mjs', 'skill'], { + cwd: workDir, encoding: 'utf8', stdio: 'pipe', + env: { ...process.env, IMPECCABLE_SKIP_ENGINE_CHECK: '1' }, + }), error => { + assert.match(error.stderr, /Missing artifact: dist\/universal\.zip\.sig\.json/); + assert.doesNotMatch(error.stderr, /UNEXPECTED_TAG_STEP/); + return true; + }); + assert.equal(git(workDir, 'tag'), ''); + assert.equal(git(workDir, 'ls-remote', '--tags', 'origin'), ''); }); it('converts the changelog entry to markdown release notes', () => {