From 4c496be02f80e6dce05dc4efd36a5e23b7af994e Mon Sep 17 00:00:00 2001 From: jaberjaber23 Date: Tue, 12 May 2026 16:13:08 +0300 Subject: [PATCH] integration fixes --- Cargo.toml | 2 +- README.md | 4 +- crates/openfang-channels/src/discord.rs | 18 +++--- crates/openfang-desktop/tauri.conf.json | 2 +- crates/openfang-kernel/src/kernel.rs | 32 +++++++--- crates/openfang-skills/src/installer.rs | 84 +++++++++++++++++++++++++ 6 files changed, 119 insertions(+), 23 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 77f479d2..73e7f288 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,7 @@ members = [ ] [workspace.package] -version = "0.6.7" +version = "0.6.8" edition = "2021" license = "Apache-2.0 OR MIT" repository = "https://github.com/RightNow-AI/openfang" diff --git a/README.md b/README.md index eee27bac..460a5c01 100644 --- a/README.md +++ b/README.md @@ -19,8 +19,8 @@

Rust MIT - v0.6.7 - Tests + v0.6.8 + Tests Clippy Buy Me A Coffee

diff --git a/crates/openfang-channels/src/discord.rs b/crates/openfang-channels/src/discord.rs index bc487364..8b5ff077 100644 --- a/crates/openfang-channels/src/discord.rs +++ b/crates/openfang-channels/src/discord.rs @@ -1457,7 +1457,7 @@ mod tests { async fn test_parse_image_only_no_caption() { let bot_id = Arc::new(RwLock::new(Some("bot123".to_string()))); let d = payload_with("", vec![att("photo.png", Some("image/png"), 100_000)]); - let msg = parse_discord_message(&d, &bot_id, &[], &[], true) + let msg = parse_discord_message(&d, &bot_id, &[], &[], true, &empty_threads()) .await .unwrap(); match msg.content { @@ -1480,7 +1480,7 @@ mod tests { "look at this", vec![att("photo.jpg", Some("image/jpeg"), 50_000)], ); - let msg = parse_discord_message(&d, &bot_id, &[], &[], true) + let msg = parse_discord_message(&d, &bot_id, &[], &[], true, &empty_threads()) .await .unwrap(); match msg.content { @@ -1512,7 +1512,7 @@ mod tests { att("b.png", Some("image/png"), 20_000), ], ); - let msg = parse_discord_message(&d, &bot_id, &[], &[], true) + let msg = parse_discord_message(&d, &bot_id, &[], &[], true, &empty_threads()) .await .unwrap(); match msg.content { @@ -1536,7 +1536,7 @@ mod tests { att("b.png", Some("image/png"), 20_000), ], ); - let msg = parse_discord_message(&d, &bot_id, &[], &[], true) + let msg = parse_discord_message(&d, &bot_id, &[], &[], true, &empty_threads()) .await .unwrap(); match msg.content { @@ -1555,7 +1555,7 @@ mod tests { async fn test_parse_heic_falls_to_file() { let bot_id = Arc::new(RwLock::new(Some("bot123".to_string()))); let d = payload_with("", vec![att("photo.heic", Some("image/heic"), 100_000)]); - let msg = parse_discord_message(&d, &bot_id, &[], &[], true) + let msg = parse_discord_message(&d, &bot_id, &[], &[], true, &empty_threads()) .await .unwrap(); match msg.content { @@ -1575,7 +1575,7 @@ mod tests { "", vec![att("huge.png", Some("image/png"), 6 * 1024 * 1024)], ); - let msg = parse_discord_message(&d, &bot_id, &[], &[], true) + let msg = parse_discord_message(&d, &bot_id, &[], &[], true, &empty_threads()) .await .unwrap(); match msg.content { @@ -1600,7 +1600,7 @@ mod tests { "see attached", vec![att("doc.pdf", Some("application/pdf"), 200_000)], ); - let msg = parse_discord_message(&d, &bot_id, &[], &[], true) + let msg = parse_discord_message(&d, &bot_id, &[], &[], true, &empty_threads()) .await .unwrap(); match msg.content { @@ -1619,7 +1619,7 @@ mod tests { // we should fall back to the filename extension. let bot_id = Arc::new(RwLock::new(Some("bot123".to_string()))); let d = payload_with("", vec![att("pic.png", None, 50_000)]); - let msg = parse_discord_message(&d, &bot_id, &[], &[], true) + let msg = parse_discord_message(&d, &bot_id, &[], &[], true, &empty_threads()) .await .unwrap(); assert!(matches!(msg.content, ChannelContent::Image { .. })); @@ -1629,7 +1629,7 @@ mod tests { async fn test_parse_empty_message_with_no_attachments_returns_none() { let bot_id = Arc::new(RwLock::new(Some("bot123".to_string()))); let d = payload_with("", vec![]); - let msg = parse_discord_message(&d, &bot_id, &[], &[], true).await; + let msg = parse_discord_message(&d, &bot_id, &[], &[], true, &empty_threads()).await; assert!(msg.is_none()); } } diff --git a/crates/openfang-desktop/tauri.conf.json b/crates/openfang-desktop/tauri.conf.json index 652bff6c..7948bb45 100644 --- a/crates/openfang-desktop/tauri.conf.json +++ b/crates/openfang-desktop/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "OpenFang", - "version": "0.6.7", + "version": "0.6.8", "identifier": "ai.openfang.desktop", "build": {}, "app": { diff --git a/crates/openfang-kernel/src/kernel.rs b/crates/openfang-kernel/src/kernel.rs index 60e6cdeb..980690d2 100644 --- a/crates/openfang-kernel/src/kernel.rs +++ b/crates/openfang-kernel/src/kernel.rs @@ -5249,12 +5249,20 @@ impl OpenFangKernel { .iter() .map(|p| p.id.clone()) .collect(); - let env_map: std::collections::HashMap = catalog - .list_providers() - .iter() - .filter(|p| !p.api_key_env.is_empty()) - .map(|p| (p.api_key_env.to_ascii_uppercase(), p.id.clone())) - .collect(); + // Multi-valued: several providers may share the same api_key_env + // (e.g. both `openai` and `codex` use OPENAI_API_KEY). Using a + // plain HashMap silently dropped earlier providers — broke #1188. + let mut env_map: std::collections::HashMap> = + std::collections::HashMap::new(); + for p in catalog.list_providers() { + if p.api_key_env.is_empty() { + continue; + } + env_map + .entry(p.api_key_env.to_ascii_uppercase()) + .or_default() + .push(p.id.clone()); + } (ids, env_map) }; @@ -5407,10 +5415,12 @@ impl OpenFangKernel { } for var in skill.manifest.config.values() { if let Some(env_name) = var.env.as_deref() { - if let Some(provider) = + if let Some(providers) = env_to_provider.get(&env_name.to_ascii_uppercase()) { - set.insert(provider.clone()); + for provider in providers { + set.insert(provider.clone()); + } } } } @@ -5422,10 +5432,12 @@ impl OpenFangKernel { // wired that provider into their MCP server. for server in &self.config.mcp_servers { for env_name in &server.env { - if let Some(provider) = + if let Some(providers) = env_to_provider.get(&env_name.to_ascii_uppercase()) { - set.insert(provider.clone()); + for provider in providers { + set.insert(provider.clone()); + } } } } diff --git a/crates/openfang-skills/src/installer.rs b/crates/openfang-skills/src/installer.rs index dfd74131..5b8e25ab 100644 --- a/crates/openfang-skills/src/installer.rs +++ b/crates/openfang-skills/src/installer.rs @@ -124,6 +124,45 @@ pub fn enforce_require_signed( ))); } + // Bind the signature to the installed bytes. + // + // envelope.verify() only proves the envelope's signature matches its own + // embedded `manifest` text. Without comparing that text to the actual + // skill.toml / SKILL.md on disk, an attacker could ship a benign signed + // envelope alongside malicious skill files and pass the check. + // + // Read every candidate manifest file in the installed dir and require + // that at least one byte-matches envelope.manifest. + const MANIFEST_CANDIDATES: &[&str] = &["skill.toml", "SKILL.md", "skill.md"]; + let mut bound = false; + for name in MANIFEST_CANDIDATES { + let path = skill_dir.join(name); + if !path.exists() { + continue; + } + match std::fs::read_to_string(&path) { + Ok(actual) if actual == envelope.manifest => { + bound = true; + break; + } + Ok(_) => {} + Err(e) => { + return Err(SkillError::SecurityBlocked(format!( + "require_signed: failed to read {} for binding check: {e}", + path.display() + ))); + } + } + } + if !bound { + return Err(SkillError::SecurityBlocked(format!( + "require_signed: signed envelope content does not match any \ + installed manifest file in {} (signature was valid but the \ + skill payload on disk differs from what was signed)", + skill_dir.display() + ))); + } + if !opts.allowed_signer_keys.is_empty() { let actual = hex::encode(&envelope.signer_public_key); let actual_lower = actual.to_lowercase(); @@ -278,6 +317,51 @@ entry = "main.py" } } + /// Critical: a valid signature for a different manifest must NOT pass + /// when the on-disk skill.toml differs. Without binding the envelope to + /// the installed bytes, an attacker could ship a benign signed envelope + /// next to malicious skill files. + #[test] + fn require_signed_on_rejects_signature_unbound_to_disk() { + let dir = TempDir::new().unwrap(); + // Sign a BENIGN manifest body but never write that text to disk. + let benign_toml = r#"name = "benign" +version = "0.1.0" +description = "Looks fine." + +[runtime] +type = "python" +entry = "main.py" +"#; + let signing_key = SigningKey::generate(&mut OsRng); + let envelope = + SignedManifest::sign(benign_toml.to_string(), &signing_key, "trusted-signer"); + write_signature(dir.path(), &envelope, "signature.json"); + + // Write a DIFFERENT (malicious) skill.toml on disk. + let evil_toml = r#"name = "evil" +version = "0.1.0" +description = "Backdoor." + +[runtime] +type = "python" +entry = "rm-rf.py" +"#; + std::fs::write(dir.path().join("skill.toml"), evil_toml).unwrap(); + + let opts = InstallOptions::require_signed(); + let err = enforce_require_signed(dir.path(), &opts).unwrap_err(); + match err { + SkillError::SecurityBlocked(msg) => { + assert!( + msg.contains("does not match any") || msg.contains("payload on disk differs"), + "got: {msg}" + ); + } + other => panic!("expected SecurityBlocked, got {other:?}"), + } + } + #[test] fn load_signature_returns_none_when_absent() { let dir = TempDir::new().unwrap();