From 4a293249112c51bb369d082cf7205b20dbdd3488 Mon Sep 17 00:00:00 2001 From: mintaka Date: Fri, 11 Sep 2026 21:34:42 -0400 Subject: [PATCH] test(comms): pin dmChannelName properties and DM two-party read scope (RIG-3535) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an untagged table test for `dmChannelName` and two pgtest lifecycle cases. The untagged test moves two already-proven properties to the cheapest tier: order independence, and injectivity across the `:` separator. The injectivity case is the confidentiality-relevant one — a hyphen delimiter would derive the same name for the distinct pairs {a, b--c} and {a--b, c}, cross-adding members into a same-owner private DM. The pgtest cases cover both parties posting and reading each other's messages, and a third same-owner agent reading nothing from a DM it is not a member of. The third-party case is canary-ordered: a real party is proven to read the post before the outsider's empty read is asserted, so an undelivered post cannot pass it vacuously. Mutation-verified, each against production code: - dropping the sort swap in `dmChannelName` fails the untagged test - switching the separator to `--` fails it on the injectivity assertion - weakening the ListMessages membership join to any-member fails the third-party case Co-authored-by: Matt Wilkinson --- go/internal/comms/dm_open_pgtest_test.go | 115 +++++++++++++++++++++++ go/internal/comms/dm_test.go | 35 +++++++ 2 files changed, 150 insertions(+) create mode 100644 go/internal/comms/dm_test.go diff --git a/go/internal/comms/dm_open_pgtest_test.go b/go/internal/comms/dm_open_pgtest_test.go index baaeef6eb..18915db20 100644 --- a/go/internal/comms/dm_open_pgtest_test.go +++ b/go/internal/comms/dm_open_pgtest_test.go @@ -16,6 +16,7 @@ import ( "connectrpc.com/connect" compassv1 "github.com/RigelBuild/compass/go/gen/compass/v1" + "github.com/RigelBuild/compass/go/internal/store" ) // TestOpenDMSameOwnerCreatesDMChannel: an agent opens a DM with a same-owner @@ -281,3 +282,117 @@ func TestOpenDMResumeEmitsNoChannelChanged(t *testing.T) { t.Fatalf("ChannelChanged events for the DM = %d, want exactly 1 (the create only; a resume must emit nothing)", dmChannelChanges) } } + +// Each party posts under its own actor, and each read must return BOTH posts — +// red if DM membership fails to grant a party read on the peer's turn. +func TestOpenDMPostAndReadBothParties(t *testing.T) { + svc, st := newHandler(t) + ctx := context.Background() + owner := mustUser(t, st, "owner") + alice := mustAgent(t, st, owner.ID, "alice") + bob := mustAgent(t, st, owner.ID, "bob") + + opened, err := svc.OpenDM(WithActor(ctx, alice.ID), connect.NewRequest(&compassv1.OpenDMRequest{PeerHandle: "bob"})) + if err != nil { + t.Fatalf("OpenDM(alice->bob) = %v, want success", err) + } + dmID := opened.Msg.GetChannel().GetId() + + post := func(actor store.AccountID, text string) string { + t.Helper() + resp, err := svc.PostMessage(WithActor(ctx, actor), connect.NewRequest(&compassv1.PostMessageRequest{ + Container: &compassv1.PostMessageRequest_ChannelId{ChannelId: dmID}, + Topic: &compassv1.PostMessageRequest_TopicName{TopicName: "general"}, + CreateTopic: true, + Blocks: textBlocks(text), + })) + if err != nil { + t.Fatalf("PostMessage(%s): %v", text, err) + } + return resp.Msg.GetMessage().GetId() + } + aliceMsg := post(alice.ID, "from alice") + bobMsg := post(bob.ID, "from bob") + + // Each party's read of the DM must surface BOTH posts — its own and the peer's. + readSees := func(reader store.AccountID) map[string]bool { + t.Helper() + listed, err := svc.ListMessages(WithActor(ctx, reader), connect.NewRequest(&compassv1.ListMessagesRequest{ + Container: &compassv1.ListMessagesRequest_ChannelId{ChannelId: dmID}, + })) + if err != nil { + t.Fatalf("ListMessages(%s): %v", reader, err) + } + ids := map[string]bool{} + for _, m := range listed.Msg.GetMessages() { + ids[m.GetId()] = true + } + return ids + } + for _, party := range []struct { + name string + id store.AccountID + }{{"alice", alice.ID}, {"bob", bob.ID}} { + got := readSees(party.id) + if !got[aliceMsg] || !got[bobMsg] { + t.Fatalf("%s reads DM = %v, want both alice %q and bob %q posts", party.name, got, aliceMsg, bobMsg) + } + } +} + +// Same-owner is not membership: a third agent under the same owner reads +// nothing from a two-party DM. Canary-ordered, so an undelivered post cannot +// pass it vacuously. Red if read-scoping falls back to owner scope. +func TestOpenDMThirdPartySameOwnerCannotSee(t *testing.T) { + svc, st := newHandler(t) + ctx := context.Background() + owner := mustUser(t, st, "owner") + alice := mustAgent(t, st, owner.ID, "alice") + bob := mustAgent(t, st, owner.ID, "bob") + carol := mustAgent(t, st, owner.ID, "carol") + + opened, err := svc.OpenDM(WithActor(ctx, alice.ID), connect.NewRequest(&compassv1.OpenDMRequest{PeerHandle: "bob"})) + if err != nil { + t.Fatalf("OpenDM(alice->bob) = %v, want success", err) + } + dmID := opened.Msg.GetChannel().GetId() + + posted, err := svc.PostMessage(WithActor(ctx, alice.ID), connect.NewRequest(&compassv1.PostMessageRequest{ + Container: &compassv1.PostMessageRequest_ChannelId{ChannelId: dmID}, + Topic: &compassv1.PostMessageRequest_TopicName{TopicName: "general"}, + CreateTopic: true, + Blocks: textBlocks("private to alice and bob"), + })) + if err != nil { + t.Fatalf("PostMessage(alice): %v", err) + } + msgID := posted.Msg.GetMessage().GetId() + + list := func(reader store.AccountID) []*compassv1.Message { + t.Helper() + listed, err := svc.ListMessages(WithActor(ctx, reader), connect.NewRequest(&compassv1.ListMessagesRequest{ + Container: &compassv1.ListMessagesRequest_ChannelId{ChannelId: dmID}, + })) + if err != nil { + t.Fatalf("ListMessages(%s): %v", reader, err) + } + return listed.Msg.GetMessages() + } + + // Canary: bob, a real party, DOES read the post — proves it was delivered, + // so the third-party emptiness below is a real negative, not a vacuous one. + var bobSees bool + for _, m := range list(bob.ID) { + if m.GetId() == msgID { + bobSees = true + } + } + if !bobSees { + t.Fatalf("party bob cannot read the DM post %q — canary failed, negative would be vacuous", msgID) + } + + // carol, same owner but not a DM member, reads nothing. + if got := list(carol.ID); len(got) != 0 { + t.Fatalf("third same-owner agent carol read %d messages from a DM she is not a member of, want 0", len(got)) + } +} diff --git a/go/internal/comms/dm_test.go b/go/internal/comms/dm_test.go new file mode 100644 index 000000000..2c1e30103 --- /dev/null +++ b/go/internal/comms/dm_test.go @@ -0,0 +1,35 @@ +package comms + +import "testing" + +// dmChannelName must be order-independent and injective. The `:` separator is +// load-bearing: a hyphen would derive one name for the distinct pairs +// {a, b--c} and {a--b, c}, cross-adding members into a same-owner private DM. +func TestDmChannelName(t *testing.T) { + cases := []struct { + name string + h1, h2 string + want string + }{ + // lo > hi: exercises the swap branch (bob sorts after alice). + {name: "unsorted input swaps to sorted name", h1: "bob", h2: "alice", want: "dm:alice:bob"}, + // lo <= hi: exercises the no-swap branch. + {name: "already-sorted input keeps order", h1: "alice", h2: "bob", want: "dm:alice:bob"}, + {name: "double-hyphen handle, first pair", h1: "a", h2: "b--c", want: "dm:a:b--c"}, + {name: "double-hyphen handle, second pair", h1: "a--b", h2: "c", want: "dm:a--b:c"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := dmChannelName(tc.h1, tc.h2); got != tc.want { + t.Fatalf("dmChannelName(%q, %q) = %q, want %q", tc.h1, tc.h2, got, tc.want) + } + // Order independence: the reversed pair must map to the same name. + if rev := dmChannelName(tc.h2, tc.h1); rev != tc.want { + t.Fatalf("dmChannelName(%q, %q) = %q, want %q (order-independent)", tc.h2, tc.h1, rev, tc.want) + } + }) + } + + // The two `--` rows above pin distinct names, which is the injectivity + // property; a hyphen delimiter would collide them onto `dm--a--b--c`. +}