From 64f819dc86e9bfd17b0247bb16022877d1e78b5f Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Date: Tue, 28 Jul 2026 01:56:58 -0400 Subject: [PATCH] fix(desktop): enforce curve validation on locked-card pubkeys + plain-byte compat vector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes both findings from Wren's cross-review at 4434d3976: 1. parse_canonical_pubkey now requires PublicKey::xonly() to succeed — nostr 0.44's from_hex only decodes 32 bytes and defers lift-x validation, so a non-point like "f"*64 previously passed structural transit/save validation and failed only at unlock. Non-points are now rejected structurally, per the agreed wire contract; the malformed-pubkeys vector asserts structural rejection instead of pinning the deferred-failure behavior. mint_agent_card uses the same canonical check on record.pubkey so a non-point fails BEFORE the API spend. 2. Added the plain-byte compatibility vector that the review claim referenced: plain_encoder_bytes_identical_to_pre_envelope_encoder reimplements the pre-refactor encode_snapshot_png body verbatim and asserts byte-identical output across all three composition paths — placeholder, PNG-avatar (chunk injection ordering), and JPEG transcode. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- .../src-tauri/src/commands/personas/card.rs | 12 +++- .../managed_agents/agent_snapshot_envelope.rs | 30 ++++---- .../managed_agents/agent_snapshot_tests.rs | 72 +++++++++++++++++++ 3 files changed, 100 insertions(+), 14 deletions(-) diff --git a/desktop/src-tauri/src/commands/personas/card.rs b/desktop/src-tauri/src/commands/personas/card.rs index bfdadc88f..106389c25 100644 --- a/desktop/src-tauri/src/commands/personas/card.rs +++ b/desktop/src-tauri/src/commands/personas/card.rs @@ -264,8 +264,16 @@ pub async fn mint_agent_card( ); } let owner_keys = state.signing_keys()?; - let agent_pubkey = nostr::PublicKey::from_hex(&record.pubkey) - .map_err(|e| format!("Agent record has an invalid pubkey: {e}"))?; + // Same canonical check the envelope decoder enforces (incl. curve + // validation) — a non-point record pubkey must fail BEFORE the API + // spend, not at post-mint encryption. + let agent_pubkey = crate::managed_agents::agent_snapshot_envelope::parse_canonical_pubkey( + "agentPubkey", + &record.pubkey, + ) + .map_err(|_| { + "Agent record has an invalid pubkey (not a canonical x-only key).".to_string() + })?; if owner_keys.public_key() == agent_pubkey { return Err("Cannot lock a card to itself: owner and agent keys match.".to_string()); } diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs index 8c0f0c351..bed3aad08 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot_envelope.rs @@ -102,8 +102,11 @@ struct FormatProbe { /// Canonical pubkey check: exactly 64 lowercase hex chars that parse as a /// valid x-only pubkey. Lowercase is required so string comparisons against -/// record pubkeys (always `to_hex()` output) stay sound. -fn parse_canonical_pubkey(field: &str, value: &str) -> Result { +/// record pubkeys (always `to_hex()` output) stay sound. Curve validation is +/// explicit: nostr's `PublicKey::from_hex` only decodes 32 bytes and defers +/// lift-x validation to `xonly()`, so a non-point like `"f" * 64` would +/// otherwise pass structurally and fail only at decrypt time. +pub(crate) fn parse_canonical_pubkey(field: &str, value: &str) -> Result { if value.len() != 64 || !value .chars() @@ -113,7 +116,12 @@ fn parse_canonical_pubkey(field: &str, value: &str) -> Result "Locked card envelope has a malformed {field} (expected 64 lowercase hex chars)." )); } - PublicKey::from_hex(value).map_err(|_| format!("Locked card envelope has an invalid {field}.")) + let pubkey = PublicKey::from_hex(value) + .map_err(|_| format!("Locked card envelope has an invalid {field}."))?; + pubkey + .xonly() + .map_err(|_| format!("Locked card envelope has an invalid {field} (not a curve point)."))?; + Ok(pubkey) } /// Structural validation of a locked envelope: exact version + scheme, @@ -483,7 +491,7 @@ mod tests { #[test] fn malformed_pubkeys_rejected_structurally() { - let (env, owner, _agent) = locked_envelope(); + let (env, _owner, _agent) = locked_envelope(); let mut short = env.clone(); short.encryption.owner_pubkey = "abc123".to_string(); @@ -493,16 +501,14 @@ mod tests { upper.encryption.agent_pubkey = upper.encryption.agent_pubkey.to_uppercase(); assert!(validate_envelope(&upper).unwrap_err().contains("malformed")); - // A 64-hex string that is not a curve point passes the string check - // (nostr's PublicKey defers lift-x validation) but can never decrypt: - // the owner still selects it as counterparty, NIP-44 derivation/MAC - // fails, and only the refusal surfaces. + // A 64-hex string that is not a curve point (lift-x fails for + // x = p-1... all-f) must be rejected STRUCTURALLY — before any key + // lookup or decrypt work — per the wire contract. let mut not_a_point = env.clone(); not_a_point.encryption.agent_pubkey = "f".repeat(64); - assert_eq!( - decrypt_envelope(¬_a_point, owner.secret_key()).unwrap_err(), - LOCKED_CARD_REFUSAL - ); + assert!(validate_envelope(¬_a_point) + .unwrap_err() + .contains("not a curve point")); let mut same = env; same.encryption.agent_pubkey = same.encryption.owner_pubkey.clone(); diff --git a/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs b/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs index 4aa98b896..14c9ff5f9 100644 --- a/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs +++ b/desktop/src-tauri/src/managed_agents/agent_snapshot_tests.rs @@ -133,6 +133,78 @@ fn png_round_trip_with_avatar_png() { assert_eq!(parsed.definition.name, snapshot.definition.name); } +/// Plain-card byte compatibility: `encode_snapshot_png` was refactored +/// through the shared `encode_chunk_payload_png` when locked cards were +/// added. Plain cards must emit byte-identical PNGs to the pre-envelope +/// encoder. This vector reimplements the legacy encoder body verbatim and +/// asserts equality on all three composition paths: placeholder (no avatar), +/// PNG-avatar (where tEXt chunk injection ordering matters), and +/// JPEG-avatar transcode. +#[test] +fn plain_encoder_bytes_identical_to_pre_envelope_encoder() { + // Verbatim pre-refactor `encode_snapshot_png` body (post memory guard). + fn legacy_encode( + snapshot: &AgentSnapshot, + avatar_bytes: Option<&[u8]>, + ) -> Result, String> { + let json_bytes = encode_snapshot_json(snapshot)?; + let chunk_text = STANDARD.encode(&json_bytes); + let png_bytes = match avatar_bytes.filter(|bytes| !bytes.is_empty()) { + Some(bytes) => { + let encoded_avatar = if bytes.starts_with(b"\x89PNG") { + inject_text_chunk(bytes, PNG_CHUNK_KEYWORD, &chunk_text).or_else(|_| { + transcode_avatar_to_png_with_text(bytes, PNG_CHUNK_KEYWORD, &chunk_text) + }) + } else { + transcode_avatar_to_png_with_text(bytes, PNG_CHUNK_KEYWORD, &chunk_text) + }; + match encoded_avatar { + Ok(png_bytes) => png_bytes, + Err(_) => make_png_with_text(PNG_CHUNK_KEYWORD, &chunk_text)?, + } + } + None => make_png_with_text(PNG_CHUNK_KEYWORD, &chunk_text)?, + }; + Ok(png_bytes) + } + + let record = minimal_record(); + + // Placeholder path (no avatar). + let snapshot = build_snapshot(&record, MemoryLevel::None, vec![], None); + assert_eq!( + encode_snapshot_png(&snapshot, None).unwrap(), + legacy_encode(&snapshot, None).unwrap(), + "placeholder-path plain PNG bytes must match the pre-envelope encoder" + ); + + // PNG-avatar path: chunk injected into the avatar image body. + let avatar = make_png_with_text("dummy", "value").unwrap(); + let snapshot = build_snapshot(&record, MemoryLevel::None, vec![], Some(&avatar)); + assert_eq!( + encode_snapshot_png(&snapshot, Some(&avatar)).unwrap(), + legacy_encode(&snapshot, Some(&avatar)).unwrap(), + "avatar-path plain PNG bytes must match the pre-envelope encoder" + ); + + // JPEG-avatar path: transcode-to-PNG composition. + let jpeg_avatar = image::DynamicImage::ImageRgb8(image::RgbImage::from_pixel( + 4, + 4, + image::Rgb([0x10, 0x20, 0x30]), + )); + let mut jpeg_bytes = Vec::new(); + jpeg_avatar + .write_to(&mut Cursor::new(&mut jpeg_bytes), image::ImageFormat::Jpeg) + .unwrap(); + let snapshot = build_snapshot(&record, MemoryLevel::None, vec![], Some(&jpeg_bytes)); + assert_eq!( + encode_snapshot_png(&snapshot, Some(&jpeg_bytes)).unwrap(), + legacy_encode(&snapshot, Some(&jpeg_bytes)).unwrap(), + "transcode-path plain PNG bytes must match the pre-envelope encoder" + ); +} + #[test] fn png_snapshot_transcodes_jpeg_avatar_into_image_body() { let avatar = image::DynamicImage::ImageRgb8(image::RgbImage::from_pixel(