From 6a1ce40d86c961f69156714081ff2fbdd3f4d91c Mon Sep 17 00:00:00 2001 From: jaberjaber23 Date: Tue, 12 May 2026 15:34:01 +0300 Subject: [PATCH] require signed --- crates/openfang-skills/Cargo.toml | 2 + crates/openfang-skills/src/clawhub.rs | 31 ++- crates/openfang-skills/src/installer.rs | 299 ++++++++++++++++++++++ crates/openfang-skills/src/lib.rs | 1 + crates/openfang-skills/src/marketplace.rs | 34 ++- 5 files changed, 364 insertions(+), 3 deletions(-) create mode 100644 crates/openfang-skills/src/installer.rs diff --git a/crates/openfang-skills/Cargo.toml b/crates/openfang-skills/Cargo.toml index b1afdbfe..d2070809 100644 --- a/crates/openfang-skills/Cargo.toml +++ b/crates/openfang-skills/Cargo.toml @@ -25,3 +25,5 @@ zip = { workspace = true } [dev-dependencies] tempfile = { workspace = true } tokio-test = { workspace = true } +ed25519-dalek = { workspace = true } +rand = { workspace = true } diff --git a/crates/openfang-skills/src/clawhub.rs b/crates/openfang-skills/src/clawhub.rs index 3f53a8f2..46d681b1 100644 --- a/crates/openfang-skills/src/clawhub.rs +++ b/crates/openfang-skills/src/clawhub.rs @@ -11,6 +11,7 @@ //! - Download: `GET /api/v1/download?slug=...` //! - File: `GET /api/v1/skills/{slug}/file?path=SKILL.md` +use crate::installer::{enforce_require_signed, InstallOptions}; use crate::openclaw_compat; use crate::verify::{SkillVerifier, SkillWarning, WarningSeverity}; use crate::SkillError; @@ -491,6 +492,25 @@ impl ClawHubClient { /// Install a skill from ClawHub into the target directory. /// + /// Convenience wrapper around [`Self::install_with_options`] using the + /// default (permissive) options. Existing callers behave exactly as + /// before. + pub async fn install( + &self, + slug: &str, + target_dir: &Path, + ) -> Result { + self.install_with_options(slug, target_dir, &InstallOptions::default()) + .await + } + + /// Install a skill from ClawHub with explicit enforcement options. + /// + /// When `opts.require_signed` is true, the skill must ship with a valid + /// Ed25519 `SignedManifest` envelope (see [`crate::installer`] for the + /// well-known filenames). Skills failing the gate are removed from disk + /// and a `SkillError::SecurityBlocked` is returned. + /// /// Security pipeline: /// 1. Download skill zip and compute SHA256 /// 2. Detect format (SKILL.md vs package.json) @@ -499,10 +519,12 @@ impl ClawHubClient { /// 5. If prompt-only: run prompt injection scan /// 6. Check binary dependencies /// 7. Write skill.toml with `verified: false` - pub async fn install( + /// 8. Enforce `require_signed` if requested. + pub async fn install_with_options( &self, slug: &str, target_dir: &Path, + opts: &InstallOptions, ) -> Result { // Use /api/v1/download?slug=... endpoint let url = format!("{}/download?slug={}", self.base_url, urlencoded(slug)); @@ -637,6 +659,12 @@ impl ClawHubClient { // Step 7: Write skill.toml openclaw_compat::write_openfang_manifest(&skill_dir, &manifest)?; + // Step 8: Enforce --require-signed gate, if requested. + if let Err(e) = enforce_require_signed(&skill_dir, opts) { + let _ = std::fs::remove_dir_all(&skill_dir); + return Err(e); + } + let result = ClawHubInstallResult { skill_name: manifest.skill.name.clone(), version: manifest.skill.version.clone(), @@ -650,6 +678,7 @@ impl ClawHubClient { slug, skill_name = %result.skill_name, warnings = result.warnings.len(), + require_signed = opts.require_signed, "Installed skill from ClawHub" ); diff --git a/crates/openfang-skills/src/installer.rs b/crates/openfang-skills/src/installer.rs new file mode 100644 index 00000000..dfd74131 --- /dev/null +++ b/crates/openfang-skills/src/installer.rs @@ -0,0 +1,299 @@ +//! Skill install enforcement options. +//! +//! Wraps the per-source install clients (FangHub `marketplace`, ClawHub) with +//! optional supply-chain gates. The flagship gate is `require_signed`: when +//! true, an Ed25519 `SignedManifest` envelope must sit alongside the skill +//! payload and verify cleanly before the install is considered complete. +//! +//! The signature envelope is a JSON serialisation of +//! [`openfang_types::manifest_signing::SignedManifest`]. The installer looks +//! for it at one of these well-known names inside the freshly written skill +//! directory: +//! +//! - `signature.json` +//! - `skill.toml.sig.json` +//! - `SKILL.md.sig.json` +//! +//! On a `require_signed` failure the skill directory is removed and a +//! `SkillError::SecurityBlocked` is returned, matching the existing +//! prompt-injection-blocked path in `clawhub.rs`. + +use crate::SkillError; +use openfang_types::manifest_signing::SignedManifest; +use std::path::Path; + +/// Options controlling enforcement during skill install. +/// +/// Defaults are permissive — `require_signed` is `false` so existing +/// callers (`Installer::install`, `Installer::install` on the marketplace) +/// behave exactly as before. +#[derive(Debug, Clone, Default)] +pub struct InstallOptions { + /// When true, reject any skill that does not ship with a valid Ed25519 + /// `SignedManifest` envelope. The `--require-signed` CLI flag maps here. + pub require_signed: bool, + /// Optional allow-list of acceptable signer public keys (hex-encoded, + /// 32 bytes / 64 hex chars). When non-empty, the envelope's + /// `signer_public_key` must match one of these entries in addition to + /// passing cryptographic verification. Empty = any valid signature + /// accepted (TOFU mode). + pub allowed_signer_keys: Vec, +} + +impl InstallOptions { + /// Convenience: `require_signed = true`, no key pinning. + pub fn require_signed() -> Self { + Self { + require_signed: true, + allowed_signer_keys: Vec::new(), + } + } + + /// Convenience: `require_signed = true` with a pinned signer key. + pub fn require_signed_by(pubkey_hex: impl Into) -> Self { + Self { + require_signed: true, + allowed_signer_keys: vec![pubkey_hex.into()], + } + } +} + +/// Well-known filenames the installer searches for a detached signature +/// envelope, in priority order. +const SIGNATURE_CANDIDATES: &[&str] = &[ + "signature.json", + "skill.toml.sig.json", + "SKILL.md.sig.json", +]; + +/// Locate a `SignedManifest` envelope inside `skill_dir`, if any. +/// +/// Returns the parsed envelope on the first candidate that exists and parses +/// successfully. Files that exist but fail to parse return an error — a +/// malformed envelope is a stronger signal than an absent one. +pub fn load_signature(skill_dir: &Path) -> Result, SkillError> { + for name in SIGNATURE_CANDIDATES { + let path = skill_dir.join(name); + if !path.exists() { + continue; + } + let raw = std::fs::read_to_string(&path)?; + let envelope: SignedManifest = serde_json::from_str(&raw).map_err(|e| { + SkillError::InvalidManifest(format!( + "Signature envelope at {} is not valid JSON: {e}", + path.display() + )) + })?; + return Ok(Some(envelope)); + } + Ok(None) +} + +/// Enforce `require_signed` against a freshly installed skill directory. +/// +/// Returns `Ok(())` when: +/// - `opts.require_signed` is false (no enforcement); or +/// - a `SignedManifest` envelope is found, `verify()` passes, and (when +/// `allowed_signer_keys` is non-empty) the signer key is allow-listed. +/// +/// Returns `SkillError::SecurityBlocked` when enforcement is on and the +/// skill fails any of those checks. On failure the caller is expected to +/// remove `skill_dir` to keep the skills directory clean. +pub fn enforce_require_signed( + skill_dir: &Path, + opts: &InstallOptions, +) -> Result<(), SkillError> { + if !opts.require_signed { + return Ok(()); + } + + let envelope = match load_signature(skill_dir)? { + Some(e) => e, + None => { + return Err(SkillError::SecurityBlocked(format!( + "require_signed: no signature envelope found in {} \ + (looked for signature.json / skill.toml.sig.json / SKILL.md.sig.json)", + skill_dir.display() + ))) + } + }; + + if let Err(e) = envelope.verify() { + return Err(SkillError::SecurityBlocked(format!( + "require_signed: signature verification failed: {e}" + ))); + } + + if !opts.allowed_signer_keys.is_empty() { + let actual = hex::encode(&envelope.signer_public_key); + let actual_lower = actual.to_lowercase(); + let matched = opts + .allowed_signer_keys + .iter() + .any(|k| k.trim().to_lowercase() == actual_lower); + if !matched { + return Err(SkillError::SecurityBlocked(format!( + "require_signed: signer key {actual} not in allow-list \ + (signer_id = {:?})", + envelope.signer_id + ))); + } + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use ed25519_dalek::SigningKey; + use rand::rngs::OsRng; + use tempfile::TempDir; + + fn write_skill_toml(dir: &Path) -> String { + let toml = r#" +[skill] +name = "signed-skill" +version = "0.1.0" +description = "A signed skill" + +[runtime] +type = "python" +entry = "main.py" +"#; + std::fs::write(dir.join("skill.toml"), toml).unwrap(); + toml.to_string() + } + + fn write_signature(dir: &Path, envelope: &SignedManifest, name: &str) { + let json = serde_json::to_string_pretty(envelope).unwrap(); + std::fs::write(dir.join(name), json).unwrap(); + } + + #[test] + fn require_signed_off_passes_unsigned() { + let dir = TempDir::new().unwrap(); + write_skill_toml(dir.path()); + let opts = InstallOptions::default(); + assert!(enforce_require_signed(dir.path(), &opts).is_ok()); + } + + #[test] + fn require_signed_on_rejects_missing_signature() { + let dir = TempDir::new().unwrap(); + write_skill_toml(dir.path()); + let opts = InstallOptions::require_signed(); + let err = enforce_require_signed(dir.path(), &opts).unwrap_err(); + match err { + SkillError::SecurityBlocked(msg) => { + assert!(msg.contains("no signature envelope"), "got: {msg}"); + } + other => panic!("expected SecurityBlocked, got {other:?}"), + } + } + + #[test] + fn require_signed_on_accepts_valid_signature() { + let dir = TempDir::new().unwrap(); + let toml = write_skill_toml(dir.path()); + let signing_key = SigningKey::generate(&mut OsRng); + let envelope = SignedManifest::sign(toml, &signing_key, "test-signer"); + write_signature(dir.path(), &envelope, "signature.json"); + + let opts = InstallOptions::require_signed(); + assert!(enforce_require_signed(dir.path(), &opts).is_ok()); + } + + #[test] + fn require_signed_on_rejects_tampered_envelope() { + let dir = TempDir::new().unwrap(); + let toml = write_skill_toml(dir.path()); + let signing_key = SigningKey::generate(&mut OsRng); + let mut envelope = SignedManifest::sign(toml, &signing_key, "test-signer"); + // Tamper with the manifest body — content_hash will no longer match. + envelope.manifest.push_str("\n# evil append\n"); + write_signature(dir.path(), &envelope, "signature.json"); + + let opts = InstallOptions::require_signed(); + let err = enforce_require_signed(dir.path(), &opts).unwrap_err(); + match err { + SkillError::SecurityBlocked(msg) => { + assert!( + msg.contains("signature verification failed") + || msg.contains("content hash mismatch"), + "got: {msg}" + ); + } + other => panic!("expected SecurityBlocked, got {other:?}"), + } + } + + #[test] + fn require_signed_rejects_malformed_envelope() { + let dir = TempDir::new().unwrap(); + write_skill_toml(dir.path()); + std::fs::write(dir.path().join("signature.json"), "{not valid json").unwrap(); + + let opts = InstallOptions::require_signed(); + let err = enforce_require_signed(dir.path(), &opts).unwrap_err(); + match err { + SkillError::InvalidManifest(msg) => { + assert!(msg.contains("Signature envelope"), "got: {msg}"); + } + other => panic!("expected InvalidManifest, got {other:?}"), + } + } + + #[test] + fn require_signed_with_allowed_keys_accepts_listed_key() { + let dir = TempDir::new().unwrap(); + let toml = write_skill_toml(dir.path()); + let signing_key = SigningKey::generate(&mut OsRng); + let envelope = SignedManifest::sign(toml, &signing_key, "test-signer"); + let pk_hex = hex::encode(&envelope.signer_public_key); + write_signature(dir.path(), &envelope, "signature.json"); + + let opts = InstallOptions::require_signed_by(pk_hex); + assert!(enforce_require_signed(dir.path(), &opts).is_ok()); + } + + #[test] + fn require_signed_with_allowed_keys_rejects_unlisted_key() { + let dir = TempDir::new().unwrap(); + let toml = write_skill_toml(dir.path()); + let signing_key = SigningKey::generate(&mut OsRng); + let envelope = SignedManifest::sign(toml, &signing_key, "evil-signer"); + write_signature(dir.path(), &envelope, "signature.json"); + + // Allow only a different key. + let other_key = SigningKey::generate(&mut OsRng); + let other_hex = hex::encode(other_key.verifying_key().to_bytes()); + let opts = InstallOptions::require_signed_by(other_hex); + let err = enforce_require_signed(dir.path(), &opts).unwrap_err(); + match err { + SkillError::SecurityBlocked(msg) => { + assert!(msg.contains("not in allow-list"), "got: {msg}"); + } + other => panic!("expected SecurityBlocked, got {other:?}"), + } + } + + #[test] + fn load_signature_returns_none_when_absent() { + let dir = TempDir::new().unwrap(); + write_skill_toml(dir.path()); + assert!(load_signature(dir.path()).unwrap().is_none()); + } + + #[test] + fn load_signature_finds_alternate_filename() { + let dir = TempDir::new().unwrap(); + let toml = write_skill_toml(dir.path()); + let signing_key = SigningKey::generate(&mut OsRng); + let envelope = SignedManifest::sign(toml, &signing_key, "alt-name-signer"); + write_signature(dir.path(), &envelope, "skill.toml.sig.json"); + + let loaded = load_signature(dir.path()).unwrap().unwrap(); + assert_eq!(loaded.signer_id, "alt-name-signer"); + } +} diff --git a/crates/openfang-skills/src/lib.rs b/crates/openfang-skills/src/lib.rs index 470c0b7e..c349d484 100644 --- a/crates/openfang-skills/src/lib.rs +++ b/crates/openfang-skills/src/lib.rs @@ -10,6 +10,7 @@ pub mod bundled; pub mod clawhub; pub mod config_injection; +pub mod installer; pub mod loader; pub mod marketplace; pub mod openclaw_compat; diff --git a/crates/openfang-skills/src/marketplace.rs b/crates/openfang-skills/src/marketplace.rs index 91f4b4eb..bbb316bb 100644 --- a/crates/openfang-skills/src/marketplace.rs +++ b/crates/openfang-skills/src/marketplace.rs @@ -3,6 +3,7 @@ //! For Phase 1, uses GitHub releases as the registry backend. //! Each skill is a GitHub repo with releases containing the skill bundle. +use crate::installer::{enforce_require_signed, InstallOptions}; use crate::SkillError; use std::path::Path; use tracing::info; @@ -90,8 +91,28 @@ impl MarketplaceClient { /// Install a skill from a GitHub repo by name. /// - /// Downloads the latest release tarball and extracts it to the target directory. + /// Convenience wrapper around [`Self::install_with_options`] using the + /// default (permissive) options. Existing callers behave exactly as + /// before. pub async fn install(&self, skill_name: &str, target_dir: &Path) -> Result { + self.install_with_options(skill_name, target_dir, &InstallOptions::default()) + .await + } + + /// Install a skill from a GitHub repo with explicit enforcement options. + /// + /// When `opts.require_signed` is true, the installed bundle must contain + /// a valid Ed25519 `SignedManifest` envelope. Skills failing the gate + /// are removed from disk and a `SkillError::SecurityBlocked` is + /// returned. + /// + /// Downloads the latest release tarball and extracts it to the target directory. + pub async fn install_with_options( + &self, + skill_name: &str, + target_dir: &Path, + opts: &InstallOptions, + ) -> Result { let repo = format!("{}/{}", self.config.github_org, skill_name); let url = format!( "{}/repos/{}/releases/latest", @@ -163,7 +184,16 @@ impl MarketplaceClient { serde_json::to_string_pretty(&meta).unwrap_or_default(), )?; - info!("Installed skill: {skill_name} {version}"); + // Enforce --require-signed gate, if requested. + if let Err(e) = enforce_require_signed(&skill_dir, opts) { + let _ = std::fs::remove_dir_all(&skill_dir); + return Err(e); + } + + info!( + "Installed skill: {skill_name} {version} (require_signed={})", + opts.require_signed + ); Ok(version) } }