feat(comms): channel policy — post ACL, mandatory subscription, SetChannelPolicy (SEA-1722) - #161
Merged
Merged
Conversation
This was referenced Aug 5, 2026
seal-agent
added a commit
that referenced
this pull request
Aug 5, 2026
…ap (SEA-1722) Review-fix on T4 (PR #161), applying Matt's owner-only ruling plus grounded auto-fixes from the mandatory adversarial review. SetChannelPolicy privilege escalation (Matt-ruled: owner-only gate). The handler gated only on requireChannelMember and wrote the client-supplied owner_account_id with an FK check only, so any member could reassign ownership to itself and bypass the OWNER_ONLY post-gate. The policy-lock SELECT now also reads the current owner under FOR UPDATE; once an owner exists only that owner may change policy or reassign ownership (a non-owner is refused with the same no-oracle ErrNotFound a non-member gets, mirroring PostMessage). An ownerless (OPEN, empty-owner) channel still lets any member establish the first owner/policy — SetChannelPolicy is create-or-update. Fail-dangerous delivery-cursor seed gap. addOrUpdateMember seeded the cursor only when the member was subscribed, so a plain (unsubscribed) add to an already-mandatory channel minted a D1 delivery target with no cursor row — the absent-cursor fail-safe then treats it permanently caught-up and it silently never receives. The channel's mandatory_subscription flag is now read once under the tx and threaded into addOrUpdateMember; the seed fires when the member is subscribed OR the channel is mandatory. OWNER_ONLY + empty-owner coherence. Both SetChannelPolicy and CreateChannel now reject OWNER_ONLY with an empty owner as ErrInvalidArgument — otherwise the NULL owner makes the post gate's COALESCE('') reject every author (an unpostable channel with no diagnostic). Policy wire mappers converted from if/else to an exhaustive switch with an explicit default, so a future third enum value cannot silently downgrade a restrictive policy to OPEN. Tests: TestSetChannelPolicyNonOwnerOnOwnedChannelIsNotFound, TestSetChannelPolicyMemberEstablishesPolicyOnOwnerlessChannel, TestSetChannelPolicyOwnerCanUpdate, TestSetChannelPolicyOwnerOnlyEmptyOwnerRejected, TestUpdateChannelMembersSeedsUnsubscribedAddOnMandatory; the two seed tests strengthened to assert seeded-to-head (not merely row-exists). Each fail-closed test verified red-first. Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
seal-agent
added a commit
that referenced
this pull request
Aug 5, 2026
…ence (SEA-1722) Additive review-fix on T4 (PR #161) from the re-review of the owner-only SetChannelPolicy gate. The gate itself was confirmed correct; this closes a gating concurrency race in the sibling member-add path and tightens two owner-coherence gaps Matt ruled to fix now. Seed-presence race (gating). UpdateChannelMembers read mandatory_subscription without a lock, so under READ COMMITTED it could race SetChannelPolicy's FOR UPDATE mandatory flip: SetChannelPolicy seeds every current member, the concurrent add reads the stale mandatory=false and skips seeding the new member, and the new member of a now-mandatory channel is left with no delivery cursor. The absent-cursor fail-safe then coalesces to live head and treats it permanently caught-up, so it silently never receives (the D2 hazard). The read now takes FOR UPDATE so it serializes against the flip on the same channels row; whichever writer commits first is observed by the second, so the member is seeded by exactly one of them, never zero. The defending comment, which claimed the read was deliberately unlocked, is corrected to the true rationale. Owner coherence (Matt-ruled). Both SetChannelPolicy and CreateChannel now reject two incoherent owner states as ErrInvalidArgument. An owner that is not a channel member is refused: an OWNER_ONLY channel whose owner is a non-member is unpostable, since the post gate demands the author be both a member and the owner. A non-empty owner on an OPEN channel is refused: owner-empty is the only legal state when OPEN, and a named owner there would let a member silently claim the operator slot. In SetChannelPolicy both guards run after the no-oracle owner gate, so a non-owner still collapses to ErrNotFound and no InvalidArgument signal leaks channel existence to an unauthorized caller. The dead no-rows branch on the policy-lock SELECT (unreachable after requireChannelMember proves the channel exists) is kept for symmetry with messages.go and annotated as defensive/unreachable. A doc comment carrying a typographic smart quote is reworded to plain ASCII (gofmt). Tests (pgtest, each fail-closed case red-first): TestUpdateChannelMembersConcurrentFlipSeedsLateMember (deterministic, gated on pg_blocking_pids, no sleep), TestSetChannelPolicyOwnerNotMemberRejected, TestSetChannelPolicyOpenWithOwnerRejected, TestCreateChannelOwnerOnlyNonMemberOwnerRejected, TestCreateChannelOpenWithOwnerRejected. Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
…annelPolicy (SEA-1722) Manager-comms substrate T4. Adds the per-channel policy the coordination channel needs: a post ACL (OPEN / OWNER_ONLY) and a mandatory-subscription flag whose members are delivery targets regardless of their stored subscribed flag. - Migration 0013 adds channels.post_policy, owner_account_id (FK, nullable), mandatory_subscription; contiguous after 0012. - store: native ChannelPostPolicy enum (mirrors comms.proto, keeps the store free of generated code like ChannelKind; mapped at the comms edge). SetChannelPolicy is the sole post-creation mutation path and seeds the D2 delivery cursor for every member the mandatory flag newly makes a delivery target, transactionally with the flag flip (an un-seeded delivery target is the fail-dangerous D2 hazard). - enforcement: OWNER_ONLY post by a non-owner returns the same in-band not-found a non-member gets (no oracle); an explicit unsubscribe on a mandatory channel is InvalidArgument; policy fields are server-set, never client-mutable via UpdateChannelMembers. - D1 subscriber resolution gains the third disjunct (OR mandatory_subscription) so a mandatory channel's members are a delivery target read-side. Refs SEA-1722 Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
…EA-1722) CreateChannel closes the create-path half of the D2 delivery hazard the flip path already handles: a channel created with Policy.MandatorySubscription=true makes every member a delivery target through the D1 disjunct regardless of the stored subscribed flag, but the member inserts wrote subscribed=false with no cursor seed — minting un-seeded delivery targets (the fail-dangerous D2 hazard, compass-notification-delivery/design.md:293-311). Seed each agent member's delivery cursor in the create txn when the channel is born mandatory, symmetric with SetChannelPolicy's newly-mandatory seed. seedDeliveryCursor is agent-only self-guarding and idempotent, so seeding every member is safe (a human member is a no-op) and a non-mandatory create seeds nothing (its members seed at subscribe time, unchanged). This is the base the T5 coordination-channel provisioning stacks on: T5 creates its channel born mandatory, so the seed must be present or every provisioned coordination channel is born with un-seeded delivery targets. Red-first: TestCreateChannelBornMandatorySeedsCursors (red without the seed). Refs SEA-1722 Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
…ap (SEA-1722) Review-fix on T4 (PR #161), applying Matt's owner-only ruling plus grounded auto-fixes from the mandatory adversarial review. SetChannelPolicy privilege escalation (Matt-ruled: owner-only gate). The handler gated only on requireChannelMember and wrote the client-supplied owner_account_id with an FK check only, so any member could reassign ownership to itself and bypass the OWNER_ONLY post-gate. The policy-lock SELECT now also reads the current owner under FOR UPDATE; once an owner exists only that owner may change policy or reassign ownership (a non-owner is refused with the same no-oracle ErrNotFound a non-member gets, mirroring PostMessage). An ownerless (OPEN, empty-owner) channel still lets any member establish the first owner/policy — SetChannelPolicy is create-or-update. Fail-dangerous delivery-cursor seed gap. addOrUpdateMember seeded the cursor only when the member was subscribed, so a plain (unsubscribed) add to an already-mandatory channel minted a D1 delivery target with no cursor row — the absent-cursor fail-safe then treats it permanently caught-up and it silently never receives. The channel's mandatory_subscription flag is now read once under the tx and threaded into addOrUpdateMember; the seed fires when the member is subscribed OR the channel is mandatory. OWNER_ONLY + empty-owner coherence. Both SetChannelPolicy and CreateChannel now reject OWNER_ONLY with an empty owner as ErrInvalidArgument — otherwise the NULL owner makes the post gate's COALESCE('') reject every author (an unpostable channel with no diagnostic). Policy wire mappers converted from if/else to an exhaustive switch with an explicit default, so a future third enum value cannot silently downgrade a restrictive policy to OPEN. Tests: TestSetChannelPolicyNonOwnerOnOwnedChannelIsNotFound, TestSetChannelPolicyMemberEstablishesPolicyOnOwnerlessChannel, TestSetChannelPolicyOwnerCanUpdate, TestSetChannelPolicyOwnerOnlyEmptyOwnerRejected, TestUpdateChannelMembersSeedsUnsubscribedAddOnMandatory; the two seed tests strengthened to assert seeded-to-head (not merely row-exists). Each fail-closed test verified red-first. Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
…ence (SEA-1722) Additive review-fix on T4 (PR #161) from the re-review of the owner-only SetChannelPolicy gate. The gate itself was confirmed correct; this closes a gating concurrency race in the sibling member-add path and tightens two owner-coherence gaps Matt ruled to fix now. Seed-presence race (gating). UpdateChannelMembers read mandatory_subscription without a lock, so under READ COMMITTED it could race SetChannelPolicy's FOR UPDATE mandatory flip: SetChannelPolicy seeds every current member, the concurrent add reads the stale mandatory=false and skips seeding the new member, and the new member of a now-mandatory channel is left with no delivery cursor. The absent-cursor fail-safe then coalesces to live head and treats it permanently caught-up, so it silently never receives (the D2 hazard). The read now takes FOR UPDATE so it serializes against the flip on the same channels row; whichever writer commits first is observed by the second, so the member is seeded by exactly one of them, never zero. The defending comment, which claimed the read was deliberately unlocked, is corrected to the true rationale. Owner coherence (Matt-ruled). Both SetChannelPolicy and CreateChannel now reject two incoherent owner states as ErrInvalidArgument. An owner that is not a channel member is refused: an OWNER_ONLY channel whose owner is a non-member is unpostable, since the post gate demands the author be both a member and the owner. A non-empty owner on an OPEN channel is refused: owner-empty is the only legal state when OPEN, and a named owner there would let a member silently claim the operator slot. In SetChannelPolicy both guards run after the no-oracle owner gate, so a non-owner still collapses to ErrNotFound and no InvalidArgument signal leaks channel existence to an unauthorized caller. The dead no-rows branch on the policy-lock SELECT (unreachable after requireChannelMember proves the channel exists) is kept for symmetry with messages.go and annotated as defensive/unreachable. A doc comment carrying a typographic smart quote is reworded to plain ASCII (gofmt). Tests (pgtest, each fail-closed case red-first): TestUpdateChannelMembersConcurrentFlipSeedsLateMember (deterministic, gated on pg_blocking_pids, no sleep), TestSetChannelPolicyOwnerNotMemberRejected, TestSetChannelPolicyOpenWithOwnerRejected, TestCreateChannelOwnerOnlyNonMemberOwnerRejected, TestCreateChannelOpenWithOwnerRejected. Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
…drift (SEA-1722) main advanced while this PR was in review and merged 0013_issues.sql, so the merge tree this PR is tested against carried two version-13 migrations and every store.Open failed with "schema version mismatch: duplicate migration version 13" (the refuse-to-serve guard in store.go firing correctly). The channel-policy migration takes the next free slot instead. Pure rename: the DDL (ALTER TABLE channels ADD post_policy / owner_account_id / mandatory_subscription) is byte-identical and independent of 0013_issues.sql. Migration versions are filename-derived (//go:embed migrations/*.sql, highest- version computed at Open), so no code references the ordinal. 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
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 T4 (SEA-1722): per-channel policy — a post ACL (
OPEN/OWNER_ONLY) and a mandatory-subscription flag — plus theSetChannelPolicyhandler and the enforcement/read-side wiring. This is the primitive behind the manager-owned coordination channel (owner-only post, members auto-subscribed and non-togglable).Design record:
docs/designs/product/compass-manager-comms-substrate/design.md(frozen, sealed PR #1090), section T4 (design.md:488-528). Store halves T2/T6 (#151/#152) and the T1 proto delta (#157) are already onmain.Changes
0013_channel_policy.sql—channelsgainspost_policy(SMALLINT, default 0=OPEN),owner_account_id(TEXT FK → accounts, nullable, empty when OPEN),mandatory_subscription(BOOL, default false). Contiguous after merged 0012.store—ChannelPolicy{PostPolicy, OwnerAccountID, MandatorySubscription}onChannel;NewChannelcarries it.SetChannelPolicy(ctx, actor, channelID, policy)is the sole post-creation mutation path (D9 member-authz, rowFOR UPDATE); on a false→true mandatory flip it seeds the D2 delivery cursor for every member in the same txn (an un-seeded delivery target is the fail-dangerous D2 hazard,compass-notification-delivery/design.md:293-311).OWNER_ONLYpost by a non-owner → the same in-band not-found a non-member gets (no oracle); explicit unsubscribe on a mandatory channel →InvalidArgument;owner_account_id/policy fields are server-set, never client-mutable viaUpdateChannelMembers.OR ch.mandatory_subscription(SubscribedAgents+UndeliveredMessages) so a mandatory channel's members are delivery targets read-side, independent of the storedsubscribedflag.comms—SetChannelPolicyhandler (maps req → store policy underactorFromContext,publishChannelChangedwrite-through, echoes updated channel); edge mapperschannelPostPolicyToWire/FromWire.Tests (red-first)
TestPostMessageOwnerOnlyRejectsNonOwnerInBand,TestPostMessageOwnerOnlyOwnerPostLands,TestUpdateChannelMembersUnsubscribeRejectedOnMandatory,TestUndeliveredMessagesReachesUnsubscribedMandatoryMember,TestSetChannelPolicySeedsCursorsForNewlyMandatory(asserts no un-seeded delivery target after the flip).TestSetChannelPolicyUpdatesAndEchoes,TestPostMessageOwnerOnlyNonOwnerIsNotFound,TestUpdateChannelMembersUnsubscribeMandatoryIsInvalidArgument.Verification
go build ./...,go vet, and the CI-exactgolangci-lint run --config .golangci.yml ./...(store+comms) all clean; full store + comms pgtest suites green under-race(migrations 0009→0013 apply contiguously). Red-first proof: removing the mandatory disjunct fromUndeliveredMessagesturnedTestUndeliveredMessagesReachesUnsubscribedMandatoryMemberred; restoring it green.Realization notes (for the human gate)
compassv1.ChannelPostPolicy. The design's T4 interface block writescompassv1.ChannelPostPolicy, but the store deliberately depends on no generated code (store/types.gopackage doc) and mirrors wire enums natively —Channel.Kindon the same struct is store-nativeChannelKind, mapped at the comms edge. Realized the design block's type per that convention (a nativeChannelPostPolicy int32, OPEN=0/OWNER_ONLY=1, edge mappers), which keeps the store's zero-generated-code invariant true. Values/vocabulary unchanged.MemberUpdate.Unsubscribestore-input field added to distinguish an explicit unsubscribe (unsubscribe_account_ids) from a plain add for the mandatory guard — no wire/API change (the wire request already carries the four parallel lists).Refs SEA-1722