feat(comms): UpdatePinnedBoard handler + pin relay arm (SEA-1723) - #165
Merged
Conversation
seal-agent
added a commit
that referenced
this pull request
Aug 5, 2026
Review-fix on T6 (PR #165), applying Matt's in-transaction ruling plus grounded auto-fixes from the mandatory adversarial review. Board-mutation authz TOCTOU (Matt-ruled: close it in-tx). UpdatePinnedBoard decided authz on a non-transactional GetChannel snapshot at the handler edge, then called the store pin op — whose transaction re-checked nothing but the channels-row lock and message membership. A membership revocation or policy flip committing between the edge read and the store tx let a just-removed member or a now-unauthorized non-owner still mutate the board. Authz now rides the store's board transaction. lockChannelForPins folds the post_policy/owner read into its existing FOR UPDATE SELECT (no extra round-trip), and a shared requireBoardMutator enforces the same two-gate no-oracle check PostMessage uses — the actor must be a member, and on OWNER_ONLY the owner — under the row lock, before the pin/unpin write. So the who-may-act decision is serialized against a concurrent membership/policy change. UnpinMessage gains the actor parameter (PinMessage already carried it for pinned_by attribution and now reuses it as the authz principal); the handler's redundant edge gate and the now-dead mayMutateBoard helper are removed, leaving one authoritative in-tx authz site. The handler godoc's false "races nothing it must serialize" claim is corrected, and a stale messages.go citation fixed. Tests: TestUpdatePinnedBoardMembershipRevokedMidFlightIsNotFound and TestUpdatePinnedBoardOwnerOnlyNonOwnerInTxIsNotFound pin the in-tx membership and post_policy gates (both verified red-first by neutering requireBoardMutator); TestUpdatePinnedBoardNonOwnerMemberOnOpenSucceeds pins the OPEN half (any member may mutate), verified red-first against an owner-only narrowing. Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
seal-agent
added a commit
that referenced
this pull request
Aug 5, 2026
Review-fix on T6 (PR #165), applying Matt's in-transaction ruling plus grounded auto-fixes from the mandatory adversarial review. Board-mutation authz TOCTOU (Matt-ruled: close it in-tx). UpdatePinnedBoard decided authz on a non-transactional GetChannel snapshot at the handler edge, then called the store pin op — whose transaction re-checked nothing but the channels-row lock and message membership. A membership revocation or policy flip committing between the edge read and the store tx let a just-removed member or a now-unauthorized non-owner still mutate the board. Authz now rides the store's board transaction. lockChannelForPins folds the post_policy/owner read into its existing FOR UPDATE SELECT (no extra round-trip), and a shared requireBoardMutator enforces the same two-gate no-oracle check PostMessage uses — the actor must be a member, and on OWNER_ONLY the owner — under the row lock, before the pin/unpin write. So the who-may-act decision is serialized against a concurrent membership/policy change. UnpinMessage gains the actor parameter (PinMessage already carried it for pinned_by attribution and now reuses it as the authz principal); the handler's redundant edge gate and the now-dead mayMutateBoard helper are removed, leaving one authoritative in-tx authz site. The handler godoc's false "races nothing it must serialize" claim is corrected, and a stale messages.go citation fixed. Tests: TestUpdatePinnedBoardMembershipRevokedMidFlightIsNotFound and TestUpdatePinnedBoardOwnerOnlyNonOwnerInTxIsNotFound pin the in-tx membership and post_policy gates (both verified red-first by neutering requireBoardMutator); TestUpdatePinnedBoardNonOwnerMemberOnOpenSucceeds pins the OPEN half (any member may mutate), verified red-first against an owner-only narrowing. Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
seal-agent
force-pushed
the
compass-comms-1723-t6-pinned-board
branch
2 times, most recently
from
August 5, 2026 21:14
4567c9c to
bced164
Compare
seal-agent
added a commit
that referenced
this pull request
Aug 5, 2026
Review-fix on T6 (PR #165), applying Matt's in-transaction ruling plus grounded auto-fixes from the mandatory adversarial review. Board-mutation authz TOCTOU (Matt-ruled: close it in-tx). UpdatePinnedBoard decided authz on a non-transactional GetChannel snapshot at the handler edge, then called the store pin op — whose transaction re-checked nothing but the channels-row lock and message membership. A membership revocation or policy flip committing between the edge read and the store tx let a just-removed member or a now-unauthorized non-owner still mutate the board. Authz now rides the store's board transaction. lockChannelForPins folds the post_policy/owner read into its existing FOR UPDATE SELECT (no extra round-trip), and a shared requireBoardMutator enforces the same two-gate no-oracle check PostMessage uses — the actor must be a member, and on OWNER_ONLY the owner — under the row lock, before the pin/unpin write. So the who-may-act decision is serialized against a concurrent membership/policy change. UnpinMessage gains the actor parameter (PinMessage already carried it for pinned_by attribution and now reuses it as the authz principal); the handler's redundant edge gate and the now-dead mayMutateBoard helper are removed, leaving one authoritative in-tx authz site. The handler godoc's false "races nothing it must serialize" claim is corrected, and a stale messages.go citation fixed. Tests: TestUpdatePinnedBoardMembershipRevokedMidFlightIsNotFound and TestUpdatePinnedBoardOwnerOnlyNonOwnerInTxIsNotFound pin the in-tx membership and post_policy gates (both verified red-first by neutering requireBoardMutator); TestUpdatePinnedBoardNonOwnerMemberOnOpenSucceeds pins the OPEN half (any member may mutate), verified red-first against an owner-only narrowing. Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
seal-agent
force-pushed
the
compass-comms-1722-t4-channel-policy
branch
from
August 5, 2026 21:14
4074868 to
bfccd5f
Compare
seal-agent
added a commit
that referenced
this pull request
Aug 6, 2026
Review-fix on T6 (PR #165), applying Matt's in-transaction ruling plus grounded auto-fixes from the mandatory adversarial review. Board-mutation authz TOCTOU (Matt-ruled: close it in-tx). UpdatePinnedBoard decided authz on a non-transactional GetChannel snapshot at the handler edge, then called the store pin op — whose transaction re-checked nothing but the channels-row lock and message membership. A membership revocation or policy flip committing between the edge read and the store tx let a just-removed member or a now-unauthorized non-owner still mutate the board. Authz now rides the store's board transaction. lockChannelForPins folds the post_policy/owner read into its existing FOR UPDATE SELECT (no extra round-trip), and a shared requireBoardMutator enforces the same two-gate no-oracle check PostMessage uses — the actor must be a member, and on OWNER_ONLY the owner — under the row lock, before the pin/unpin write. So the who-may-act decision is serialized against a concurrent membership/policy change. UnpinMessage gains the actor parameter (PinMessage already carried it for pinned_by attribution and now reuses it as the authz principal); the handler's redundant edge gate and the now-dead mayMutateBoard helper are removed, leaving one authoritative in-tx authz site. The handler godoc's false "races nothing it must serialize" claim is corrected, and a stale messages.go citation fixed. Tests: TestUpdatePinnedBoardMembershipRevokedMidFlightIsNotFound and TestUpdatePinnedBoardOwnerOnlyNonOwnerInTxIsNotFound pin the in-tx membership and post_policy gates (both verified red-first by neutering requireBoardMutator); TestUpdatePinnedBoardNonOwnerMemberOnOpenSucceeds pins the OPEN half (any member may mutate), verified red-first against an owner-only narrowing. Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
seal-agent
force-pushed
the
compass-comms-1723-t6-pinned-board
branch
from
August 6, 2026 00:42
bced164 to
fab620a
Compare
Manager-comms substrate T6 (handler half). Replaces the T1 UpdatePinnedBoard CodeUnimplemented stub with the real handler and wires the agent-initiated pin relay arm, over the already-merged channel_pins store (#152). - comms: UpdatePinnedBoard resolves the channel (GetChannel), authorizes at the edge via mayMutateBoard (post_policy: any member on OPEN, owner-only on OWNER_ONLY), applies the op via applyBoardOp (pin/CAS-repoint via PinMessage, unpin via UnpinMessage; unset oneof -> CodeInvalidArgument), then re-reads and emits ChannelChanged carrying the updated board. A non-member OR a non-owner on an OWNER_ONLY channel collapses to the SAME in-band CodeNotFound — the no-oracle not-found/forbidden merge, mirroring the store's PostMessage OWNER_ONLY enforcement (store/messages.go:79-81) so the board leaks no existence signal. - comms: pinnedEntriesToWire maps store-native []PinnedEntry -> wire, preserving position order (channelToWire does not project the board, so the event and the response are published from the same board-carrying wire). - comms: UpdatePinnedBoardAsAccount (WithActor + shared handler path), the agent-initiated leg mirroring PostAsAccount. - runnerhub: executeCall gains the pin arm (CommsCallRequest_Pin -> UpdatePinnedBoardAsAccount, wraps CommsCallResult_Pin); CommsCaller interface + fakeCommsCaller extended. The fail-closed default is untouched. - store: exported GetChannel(ctx, id) over the existing unexported getChannel — the id-addressed read (members + policy) the edge authz needs. Red-first (9 pgtest handler cases, watched fail on a disabled mayMutateBoard guard): pin of a message from another channel -> in-band CodeNotFound; repoint swaps atomically (old gone / new present / position preserved); stale CAS -> CodeAlreadyExists; cap+1 -> CodeFailedPrecondition; non-owner on OWNER_ONLY and non-member both -> no-oracle CodeNotFound (owner positive path asserted); unpin removes the entry; unset op -> CodeInvalidArgument; ChannelChanged carries the board. The store FOR-UPDATE two-concurrent-cap-edge race is covered at the store layer (TestPinMessageConcurrentCapEdge). Refs SEA-1723 Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
Review-fix on T6 (PR #165), applying Matt's in-transaction ruling plus grounded auto-fixes from the mandatory adversarial review. Board-mutation authz TOCTOU (Matt-ruled: close it in-tx). UpdatePinnedBoard decided authz on a non-transactional GetChannel snapshot at the handler edge, then called the store pin op — whose transaction re-checked nothing but the channels-row lock and message membership. A membership revocation or policy flip committing between the edge read and the store tx let a just-removed member or a now-unauthorized non-owner still mutate the board. Authz now rides the store's board transaction. lockChannelForPins folds the post_policy/owner read into its existing FOR UPDATE SELECT (no extra round-trip), and a shared requireBoardMutator enforces the same two-gate no-oracle check PostMessage uses — the actor must be a member, and on OWNER_ONLY the owner — under the row lock, before the pin/unpin write. So the who-may-act decision is serialized against a concurrent membership/policy change. UnpinMessage gains the actor parameter (PinMessage already carried it for pinned_by attribution and now reuses it as the authz principal); the handler's redundant edge gate and the now-dead mayMutateBoard helper are removed, leaving one authoritative in-tx authz site. The handler godoc's false "races nothing it must serialize" claim is corrected, and a stale messages.go citation fixed. Tests: TestUpdatePinnedBoardMembershipRevokedMidFlightIsNotFound and TestUpdatePinnedBoardOwnerOnlyNonOwnerInTxIsNotFound pin the in-tx membership and post_policy gates (both verified red-first by neutering requireBoardMutator); TestUpdatePinnedBoardNonOwnerMemberOnOpenSucceeds pins the OPEN half (any member may mutate), verified red-first against an owner-only narrowing. Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
seal-agent
force-pushed
the
compass-comms-1723-t6-pinned-board
branch
from
August 6, 2026 01:11
fab620a to
5666fb2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Manager-comms substrate T6 (handler half, SEA-1723): the
UpdatePinnedBoardhandler + the agent-initiated pin relay arm, over the already-mergedchannel_pinsstore (#152). Design record section T6-handler (design.md:594-637).Stacked on #161 (T4,
compass-comms-1722-t4-channel-policy) — base is the T4 branch, notmain. The board authz readsChannel.Policy.PostPolicy/OwnerAccountID, which T4 adds. Merge #161 first.Changes
comms.UpdatePinnedBoardresolves the channel (GetChannel), authorizes at the edge viamayMutateBoard(post_policy: any member on OPEN, owner-only on OWNER_ONLY), applies the op viaapplyBoardOp(pin / CAS-repoint →PinMessage; unpin →UnpinMessage; unset oneof →CodeInvalidArgument), re-reads, and emitsChannelChangedcarrying the updated board.CodeNotFound, mirroring the store'sPostMessageOWNER_ONLY enforcement (store/messages.go:79-81) so the board leaks no existence signal.pinnedEntriesToWiremaps store-native[]PinnedEntry→ wire, position order preserved (channelToWiredoes not project the board; the event and response ship from the same board-carrying wire).UpdatePinnedBoardAsAccount(WithActor + shared handler path), mirroringPostAsAccount.executeCallgains the pin arm (CommsCallRequest_Pin→UpdatePinnedBoardAsAccount, wrapsCommsCallResult_Pin);CommsCalleriface +fakeCommsCallerextended. Fail-closed default untouched.GetChannel(ctx, id)over the existing unexportedgetChannel— the id-addressed read (members + policy) the edge authz needs.Tests (red-first, 9 pgtest handler cases)
Watched fail on a disabled
mayMutateBoardguard (if false && …→ both authz testsgot nil error, want not_found; guard restored, green). Cases: pin of a message from another channel → in-bandCodeNotFound; repoint swaps atomically (old gone / new present / position preserved); stale CAS →CodeAlreadyExists; cap+1 →CodeFailedPrecondition; non-owner on OWNER_ONLY and non-member both → no-oracleCodeNotFound(owner positive path asserted); unpin removes the entry; unset op →CodeInvalidArgument;ChannelChangedcarries the board. The store FOR-UPDATE two-concurrent-cap-edge race is covered at the store layer (TestPinMessageConcurrentCapEdge, #152).Verification
go build ./...,go vet, CI-exactgolangci-lint run --config .golangci.yml ./...all clean (0 issues);internal/comms(28.2s),internal/runnerhub(17.6s),internal/store(85.5s, migrations 0009→0013 contiguous) pgtest suites green under-race.Merge-fold note (for the collector)
T6 wires the
pinrelay arm; #163 (T2) wiresroster+set_statusin a separate workspace. TheCommsCalleriface,fakeCommsCaller, andexecuteCallswitch each gain additive entries in both (T6's comms executors inagent_caller.go, T2's inroster.go— disjoint), so whichever lands second gets a clean 3-spot union. T6 leavesexecuteCall's fail-closed default at its original"post/list"text (its base predates #163); at union, take #163's reworded default and appendpinso it names all five recognized variants (post/list/roster/set_status/pin) — cosmetic, behavior unchanged.Refs SEA-1723