Skip to content

agent-connector: send_final idempotency fingerprint is computed over the un-normalized group_id_hex — a differently-cased retry double-posts an unrecallable message #897

Description

@erskingardner

Severity: LOW
Component: crates/agent-connector

Summary

send_final_response (and its sibling media/delete handlers) decode group_id_hex with a raw hex::decode(group_id_hex) instead of normalize_hex(group_id_hex)?, unlike the other app-event handlers in the same file. For send_final specifically, the anti-double-post idempotency fingerprint is computed over the raw, un-normalized group_id_hex, so two retries under the same idempotency key that differ only in group-id hex casing produce different fingerprints — the second is a cache miss and the durable message is re-sent.

Location

  • crates/agent-connector/src/messaging.rs:115-120send_final_fingerprint(account_id_hex, group_id_hex, &text, reply_to...) computed over the raw group_id_hex.
  • crates/agent-connector/src/messaging.rs:132GroupId::new(hex::decode(group_id_hex)?) (raw decode at send time).
  • Same raw-decode pattern (no normalize_hex) in delete_message_response (:163), send_media (~:387), and download_media (~:427).
  • Contrast: send_agent_activity (:272-273), send_agent_operation_event (:313-314), send_group_system_event (:354-355) all call normalize_hex(group_id_hex)? first.
  • crates/agent-connector/src/validation.rs:216-218normalize_hex(v) = hex::encode(hex::decode(v)?), i.e. it validates and lowercases; account ids are already canonical (matched exactly by local_account_for_account_id), so only the group component is exposed to casing divergence.

Concrete failure scenario

A connector client (the threat model explicitly includes a prompt-injected/compromised gateway) issues send_final with idempotency key K and group_id_hex in uppercase, times out after the durable write, and retries with K and the same group in lowercase (or vice versa). send_final_fingerprint differs between the two, so self.idempotency.get(key, &fingerprint) misses and the message is sent a second time — defeating the idempotency guarantee whose entire purpose (per the in-code comment) is that "a retry after a post-write timeout cannot double-post an unrecallable message."

Impact

Duplicate, unrecallable durable message on a differently-cased retry. Low severity because a client that retries with byte-identical frames is unaffected, but it is a real correctness gap in the idempotency contract and a consistency deviation from the crate's own normalize_hex discipline.

Suggested fix

Normalize group_id_hex via normalize_hex at the top of these handlers (as the sibling app-event handlers do) so both the fingerprint and the GroupId derive from the canonical form.

Dedup

No existing issue covers this handler set. Distinct from the FFI/storage checked-conversion issues (#871/#821) — this is normalization, not integer casting.


Filed from a meticulous automated code-review pass; verified against source and deduplicated against the existing open/closed issue set.

Metadata

Metadata

Assignees

No one assigned

    Labels

    LOWbugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions