From 7bd694214b22b3ad22cfb1f45be03e5ca2fd4d46 Mon Sep 17 00:00:00 2001 From: Rex Raphael Date: Thu, 3 Sep 2026 08:43:33 -0500 Subject: [PATCH] fix(a2aremote): drop an unrecognised inReplyTo instead of the message The CodeQL autofix made an inbound message fail outright when its inReplyTo is not one of our TypeIDs. A peer's message ids are the peer's business: a UUID there is perfectly conformant A2A, it simply cannot match a token we issued. The bus already treats an in-reply-to matching no pending ask as ordinary mail, so refusing the request turned a message we can read into an error the peer cannot act on. The token is still validated before it travels any further, which is what the alert was pointing at, so what changes is only what happens when validation fails. Worth saying about the alert itself: the sink is Where("reply_with = ?", replyWith) on sqlite and postgres and a bson value on mongo, so there was no injection to fix. Validating is cheap and worth keeping regardless; refusing conformant peers was the part to undo. --- a2aremote/mapping.go | 18 +++++++++++--- a2aremote/mapping_test.go | 52 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 4 deletions(-) diff --git a/a2aremote/mapping.go b/a2aremote/mapping.go index 8460492..4ab3962 100644 --- a/a2aremote/mapping.go +++ b/a2aremote/mapping.go @@ -96,12 +96,22 @@ func EnvelopeParamsFromMessage(m Message, sender, receiver a2a.Address) (a2a.Sen Encoding: meta.Encoding, ReplyWith: meta.ReplyWith, } + // The correlation token is validated before it is carried any + // further, and an unrecognised one is DROPPED rather than refused. + // + // Both halves matter. Validating bounds what reaches the pending-ask + // lookup to a shape we minted, which is worth doing even though that + // lookup is a parameterised query and not injectable. Dropping + // rather than refusing keeps cortex able to talk to peers whose + // message ids are not TypeIDs: a UUID in inReplyTo is perfectly + // conformant A2A, it simply cannot match a token we issued, and the + // bus already treats an in-reply-to matching no pending ask as + // ordinary mail. Refusing the request instead would turn a message + // we can read into an error the peer cannot act on. if meta.InReplyTo != "" { - inReplyTo, inReplyToErr := id.ParseWithPrefix(meta.InReplyTo, id.PrefixMessage) - if inReplyToErr != nil { - return a2a.SendParams{}, ErrInvalidParams("inReplyTo is not a message id: " + inReplyToErr.Error()) + if inReplyTo, parseErr := id.ParseWithPrefix(meta.InReplyTo, id.PrefixMessage); parseErr == nil { + params.InReplyTo = inReplyTo.String() } - params.InReplyTo = inReplyTo.String() } if m.ContextID != "" { convID, convErr := id.ParseWithPrefix(m.ContextID, id.PrefixConversation) diff --git a/a2aremote/mapping_test.go b/a2aremote/mapping_test.go index a1de32e..60e9bfc 100644 --- a/a2aremote/mapping_test.go +++ b/a2aremote/mapping_test.go @@ -190,3 +190,55 @@ func TestRunningRunHasNoArtifactsYet(t *testing.T) { t.Fatalf("task = %+v", task) } } + +// A peer's message id is the peer's business. Ours are TypeIDs; a +// perfectly conformant A2A implementation might use UUIDs, or a +// sequence, or anything else, and a message that quotes one of those in +// inReplyTo is still a valid message. +// +// So an unrecognised token drops the correlation rather than the +// message. The bus already treats an in-reply-to that matches no +// pending ask as ordinary mail, which is the same outcome by a different +// route, and refusing the whole request instead would make cortex +// unable to talk to any peer whose ids are not shaped like ours. +func TestAnUnrecognisedInReplyToDropsTheCorrelationNotTheMessage(t *testing.T) { + m := Message{ + MessageID: "not-a-typeid-either", Role: RoleUser, Parts: []Part{{Text: "answering you"}}, + Metadata: map[string]any{FIPAExtensionURI: map[string]any{ + "performative": "inform", + "inReplyTo": "9f8b2c1e-4a7d-11ef-9c3d-0242ac120002", + }}, + } + + params, err := EnvelopeParamsFromMessage(m, a2a.Address{Agent: "them", Node: "peer.example"}, a2a.Address{Agent: "worker"}) + if err != nil { + t.Fatalf("a valid A2A message was refused over a token shape: %v", err) + } + if params.Content != "answering you" { + t.Fatalf("content = %q", params.Content) + } + if params.InReplyTo != "" { + t.Fatalf("InReplyTo = %q, want it dropped: it can never match a token we minted", params.InReplyTo) + } +} + +// A token we minted correlates, which is the case that matters: it is +// the only shape a reply to one of our own asks can carry. +func TestOurOwnInReplyToSurvives(t *testing.T) { + ours := id.NewMessageID().String() + m := Message{ + MessageID: "m1", Role: RoleUser, Parts: []Part{{Text: "answering you"}}, + Metadata: map[string]any{FIPAExtensionURI: map[string]any{ + "performative": "inform", + "inReplyTo": ours, + }}, + } + + params, err := EnvelopeParamsFromMessage(m, a2a.Address{Agent: "them", Node: "peer.example"}, a2a.Address{Agent: "worker"}) + if err != nil { + t.Fatalf("EnvelopeParamsFromMessage: %v", err) + } + if params.InReplyTo != ours { + t.Fatalf("InReplyTo = %q, want %q", params.InReplyTo, ours) + } +}