mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): enforce curve validation on locked-card pubkeys + plain-byte compat vector
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 <tlongwell@block.xyz>
Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
This commit is contained in:
co-authored by
Tyler Longwell
parent
4434d39766
commit
64f819dc86
@@ -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());
|
||||
}
|
||||
|
||||
@@ -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<PublicKey, String> {
|
||||
/// 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<PublicKey, String> {
|
||||
if value.len() != 64
|
||||
|| !value
|
||||
.chars()
|
||||
@@ -113,7 +116,12 @@ fn parse_canonical_pubkey(field: &str, value: &str) -> Result<PublicKey, String>
|
||||
"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();
|
||||
|
||||
@@ -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<Vec<u8>, 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(
|
||||
|
||||
Reference in New Issue
Block a user