Repository navigation
Conversation
b5f4099 to
141aa01
Compare
github-actions
Bot
added
the
codex-security-review-current
🔐 Codex Security Review
Review SummaryOverall Risk: NONE
FindingsNo concrete security, correctness, or reliability findings were identified. Notes
Generated by Codex Security Review | |
`list_auto_join_channels` stopped at 32 rows while kind:9002 kept accepting `auto_join=true` on any number of channels, so with 33 flagged channels the newest one carried the kind:39000 `auto_join` tag but no claim or kind:9030 add ever joined anyone to it, and nothing retried because the person was already a relay member (Codex review on #8594). Drop the cap rather than enforce it at flag time: the kind:9002 transaction holds only the shared community admission lock and the one channel's row lock, so a count-based refusal would need a new community-wide lock to be race-free, and a private or archive flip on a flagged channel would still let the stored count drift past it. The loader now returns every effective channel, oldest first; the set is bounded by the community's channels, each flagged by a relay owner or admin, and NOSTR.md says so. Test: `every_default_channel_is_listed` (Postgres lane) flags 33 channels and asserts the loader lists all of them in creation order; putting the `LIMIT` back fails it (verified by mutation). Verified: fmt; clippy -D warnings on buzz-db and buzz-relay (all targets); the four auto_join Postgres tests. `auto_join_is_relay_admin_only_...` failed once at its first query with the known `PoolTimedOut` lane flake and passed on a solo rerun. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
github-actions
Bot
removed
the
codex-security-review-current
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
🤖 :bot: Jude’s code review agent
Verdict: REQUEST CHANGES
Reviewed: d676c7b8db30ccc15611b9ef51ed9c6f2e8a954a..502d01607ca77d77e175a997cdc16aff98ce674a (exact head 502d01607ca77d77e175a997cdc16aff98ce674a)
Risk: high — this changes relay-enforced community membership behavior and the persisted channels schema that drives default-channel admission.
Behavior/contracts traced: schema migration + desired-state schema for channels.auto_join; kind:9002 authorization/scope for auto_join; row-lock enforcement that only effective open/unarchived stream channels can be flagged; kind:39000 effective-state auto_join advertising; roster-insert call sites for v2 invite claim, v1 invite claim, and kind:9030 add; shared self-join side effects and leave/re-claim behavior.
Findings
P1 — channels table is missing required desired-state-vs-migration catalog parity coverage
This PR changes the channels table in both schema paths:
migrations/0069_channel_auto_join.sql:10-11addsauto_join BOOLEAN NOT NULL DEFAULT false.schema/tables/public/channels.sql:39-41declares the desired-state column.
The repository schema rules require more than editing both files. For tables/indexes touched by a schema change, the PR must add or extend a live catalog parity assertion comparing a desired-state bootstrap with a migrated database (migrations/AGENTS.md:12-15, schema/AGENTS.md:1-4, docs/database.md:267-273). The current parity test does not cover channels: admin_schema_parity_between_desired_state_and_migrations only loops over relay_admin_actions, relay_admin_outbox, relay_operator_audit, and entity_deletion_requests for column/index parity (crates/buzz-db/src/runtime/migration.rs:2891-2911). A separate migrated-schema smoke check only verifies that the channels table exists; it does not compare migrated vs desired column defaults/nullability/index shape.
Consequence: a drift in channels.auto_join shape — default, nullability, or a future related catalog shape — can pass this PR’s current tests while fresh desired-state databases and migrated databases behave differently. For a feature that controls automatic channel membership and visibility for new community members, that is the schema-path split the repo rule exists to catch.
Author action: add channels to admin_schema_parity_between_desired_state_and_migrations (or add an equivalent Postgres-gated catalog parity test) so the touched table’s columns and index shapes are compared between desired-state bootstrap and migrations. Include constraints too if the final fix touches them. Then run the relevant PostgreSQL migration/parity lane.
Verification owner: author for the test/change; CI/reviewer for the rerun after the fix.
Non-blocking observations
I did not find an additional blocking access/consent defect in the default-channel behavior itself. The reachable implementation is bounded in the right places: auto_join is a relay owner/admin setting rather than a channel-owner-only setting; setting it to true is rejected unless the locked resulting channel state is open and unarchived; kind:39000 advertises the tag only while effective; and auto-join runs only at roster-insert sites, so a later leave is respected.
Current GitHub CI has one Desktop/Tauri failure caused by repeated apt-get install timeouts while fetching libwebkit2gtk-4.1-0, plus a cancelled Codex Security Review. I am treating those as CI/tooling confidence gaps, not author rework for this PR.
Validation
Local exact-head checks, from a clean worktree:
git rev-parse HEAD
# 502d01607ca77d77e175a997cdc16aff98ce674a
git status --short
# clean
cargo test -p buzz-db group_metadata_tag_tests --lib
# PASS — 6 passed; 0 failed
cargo test -p buzz-sdk update_channel_auto_join_is_always_explicit --lib
# PASS — 1 passed; 0 failed
cargo test -p buzz-db embedded_migrator_contains_consolidated_initial_schema --lib
# PASS — 1 passed; 0 failed
CI status reviewed at the same live head: PostgreSQL Tests, Relay E2E, Backend Integration, Desktop E2E Integration, Rust lint/unit, Docker image builds, Desktop Release Candidate, and macOS Desktop Build were green. The Desktop Tauri/core failure occurred before code execution during dependency installation (apt-get timed out three times on the webkit package download).
Manual/native evidence: not applicable for this blocker; it is relay/schema catalog coverage. I did not run the full Postgres live-relay suite locally.
Residual risk: the PR currently reports mergeStateStatus: DIRTY; conflict resolution or any other head change expires this review and needs a fresh pass.
`list_auto_join_channels` stopped at 32 rows while kind:9002 kept accepting `auto_join=true` on any number of channels, so with 33 flagged channels the newest one carried the kind:39000 `auto_join` tag but no claim or kind:9030 add ever joined anyone to it, and nothing retried because the person was already a relay member (Codex review on #8594). Drop the cap rather than enforce it at flag time: the kind:9002 transaction holds only the shared community admission lock and the one channel's row lock, so a count-based refusal would need a new community-wide lock to be race-free, and a private or archive flip on a flagged channel would still let the stored count drift past it. The loader now returns every effective channel, oldest first; the set is bounded by the community's channels, each flagged by a relay owner or admin, and NOSTR.md says so. Test: `every_default_channel_is_listed` (Postgres lane) flags 33 channels and asserts the loader lists all of them in creation order; putting the `LIMIT` back fails it (verified by mutation). Verified: fmt; clippy -D warnings on buzz-db and buzz-relay (all targets); the four auto_join Postgres tests. `auto_join_is_relay_admin_only_...` failed once at its first query with the known `PoolTimedOut` lane flake and passed on a solo rerun. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
502d016 to
05d4310
Compare
github-actions
Bot
added
the
codex-security-review-current
|
🤖 Rebased onto Parity coverage. Rebase. Defect found by the merge. Verified locally on this head: Pre-push lanes (lhm, 1.95.0): everything passed except Head is now |
jedwards27
left a comment
jedwards27
left a comment
There was a problem hiding this comment.
🤖 :bot: Jude's code review agent review
Verdict: request changes recommended on exact head 05d431094c3b5c54968c8e10c4872329787c8372 (base 8216db1c712cefdc0e2af7191d501adae03ba392). Per the repo's agent-review policy, I am posting this as a comment review; the recommendation is still to fix the P1 below before merging.
P1: auto-join can add a new community member to a channel after it has been archived
The new auto-join path first snapshots effective defaults in list_auto_join_channels, filtering for auto_join, stream, open, archived_at IS NULL, and deleted_at IS NULL (crates/buzz-db/src/store/channel.rs:628-649). It then joins each selected channel through join_channel_as_self (crates/buzz-relay/src/handlers/side_effects.rs:2744-2763). That helper performs an archive precheck before the write (crates/buzz-relay/src/handlers/side_effects.rs:2670-2681), but the authoritative membership write is put_member/put_members.
Inside put_members, the transaction takes the channel membership lock, reads the channel, and derives admission from only visibility and join_role (crates/buzz-db/src/store/channel_members.rs:809-815, crates/buzz-db/src/store/channel_members.rs:859-867). It never refuses archived_at before inserting/reactivating rows (crates/buzz-db/src/store/channel_members.rs:923-941). Meanwhile, archive_channel and unarchive_channel call lock_channel_for_settings_write(..., ChannelUpdate::default()) (crates/buzz-db/src/store/channel.rs:1249-1284), and ChannelUpdate::changes_admission() only covers visibility/join_role (crates/buzz-db/src/store/channel.rs:862-866), so archive/unarchive do not take the membership lock; lock_channel_row only takes that lock when its admission argument is true (crates/buzz-db/src/store/channel.rs:1021-1035).
Failure scenario: a relay owner/admin marks #starter as a default channel. A new member claims an invite, and auto_join_default_channels lists #starter while it is open/unarchived. Before put_member inserts the membership, an admin or TTL/archive path archives the channel. Because archive and membership insertion do not share a serialization point, put_member can still create/reactivate the membership. join_channel_as_self then invalidates caches and emits the member_joined system message, group discovery events, and the member-added notification (crates/buzz-relay/src/handlers/side_effects.rs:2700-2733) for a channel that is no longer an effective default.
That violates the feature contract documented in NOSTR.md: new members are joined to every effective default channel, and kind:39000 only carries auto_join while the setting is effective (NOSTR.md:219-233, NOSTR.md:205-211). It also creates surprising membership state that can become visible/actionable later if the channel is unarchived or made private.
Author action: make archived-state admission authoritative at the membership write seam, not only in the caller precheck. The smallest safe shape is to treat archive/unarchive as admission-changing so they take the established channel membership lock in the existing lock order, and make put_members refuse new/reactivated membership when the locked channel state is archived. A stale auto-join list should become a logged/skipped failure, not a membership row or notifications.
Verification owner: author adds the fix and a deterministic Postgres regression covering default channel listed/open -> archive wins before auto-join membership insert -> no channel_members row and no 40099/39002/44100 side effects. A simpler DB guard test for put_member against an already archived channel is useful but is not sufficient unless the archive/membership race is also serialized.
Other review evidence
- The prior schema-parity blocker appears addressed:
channels.auto_joinis present in both schema paths, and the parity test now compareschannelscolumns between desired-state and migrated schemas (crates/buzz-db/src/runtime/migration.rs:2926-2934), while intentionally excluding the pre-existingidx_channels_id_live INCLUDE (community_id)drift. - Exact-head focused validation reviewed for this pass included:
cargo test -p buzz-db embedded_migrator_contains_consolidated_initial_schema --lib,cargo test -p buzz-db group_metadata_tag_tests --lib,cargo test -p buzz-sdk update_channel_auto_join_is_always_explicit --lib,cargo test -p buzz-cli update_channel_fields --lib, andscripts/test-postgres-test-discovery.shpassing at05d431094c3b5c54968c8e10c4872329787c8372with clean worktrees. - Fresh GitHub state checked before posting: PR head was still
05d431094c3b5c54968c8e10c4872329787c8372; relay/Postgres-relevant checks were green. Desktop aggregate/compiled-flags failures and one Windows Rust check were still present/in progress, but this review is blocked by the PR-caused membership/archive race above regardless of those CI states.
Add `channels.auto_join` (migration 0076, constant default false) so a community's relay owners and admins can mark the channels the relay should join new community members into. This is the contract half (R1): the setting, who may change it, and how it is published. The auto-add at the join sites lands separately (R2). - kind:9002 accepts `["auto_join", "true"|"false"]`. A new authorization tier in the 9002 validator takes the actor's relay roster role (owner|admin) rather than a channel role, so a relay admin who is not a channel member can set it, and an event carrying only that tag needs neither channel-owner nor member authority. Only stream channels qualify. API tokens need `AdminChannels` for it, like `archived`. - `apply_channel_metadata_event` refuses `auto_join=true` unless the state the event leaves behind is open and unarchived, decided under the channel row lock: one event may open and flag a channel, and a concurrent archive cannot slip past a pre-lock check. Every change is announced by one `auto_join_changed` kind:40099 notice. - kind:39000 carries a bare `auto_join` tag only while the setting is effective (set, open, unarchived), so what the event says is what the relay will do, with no cascade logic on visibility flips or archive. - `Db::list_auto_join_channels` (effective channels, oldest first, capped at 32) is the loader R2 will call. - SDK `build_update_channel(.., auto_join)`, `buzz channels update --auto-join <true|false>`, a NOSTR.md "Default channels" section and the NIP-CL tag table row. Tests: a Postgres-lane journey in edit_metadata_postgres_tests covering the relay-role tier, the 40099 notice, the effective-state tag across a flip to private, the combined open+flag event, and the private, archived and forum refusals; group_metadata_tags unit rows; SDK and CLI builder cases; and auto_join assertions in the announce-channel e2e wire test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Join every new community member into the channels a relay owner or admin flagged with `channels.auto_join` (R1, d65bf98), at the moment they become a member. This is the behaviour half (R2) of default channels: clients see the flagged channels in their sidebar, unread state and push with no client work, because the relay runs the kind:9021 body for them. - `join_channel_as_self` is kind:9021's body extracted from `handle_join_request` and returns `Joined | AlreadyMember`: open and unarchived checks, skip-if-member, then `put_member(.., None, None)` so the channel's `join_role` applies, cache invalidation, the `member_joined` kind:40099, 39000/39002 and the kind:44100. The handler keeps its exact external behaviour; the archived guard is new in the shared body because only the 9021 validator held it before. - `auto_join_default_channels` iterates `Db::list_auto_join_channels` (effective channels only) and calls the body per channel, best effort: a failure is logged and counted (`buzz_auto_join_failures_total`), a join counted (`buzz_auto_join_channels_total`), and the caller never fails, because the relay membership has already committed. No retry after that commit: re-running at any later point would resurrect kind:9022 leaves, and the kind:39000 `auto_join` tag is the record a client reconciles against. This is an accepted Review-Proven Rule 1 deviation, documented in NOSTR.md. - Called inline right after `publish_nip43_member_added` at the three user-facing join sites: the v2 invite claim `Joined` arm, the v1 claim `was_inserted` branch, and kind:9030 when the roster row was inserted. Never at login or reconnect. Inline means the kind:39002 roster already lists the member when the claim response returns. - `buzz-admin add-member` is unchanged except for help text noting it writes the roster row only; it has no `AppState` to sign 39002/44100. Tests: `claim_auto_joins_default_channels_once` (Postgres lane) covers a v2 claim against a flagged, an archived-after-flagging, a private-after-flagging and an unflagged channel: membership and 39002 in the flagged one only, one 44100 and one `member_joined` 40099, then a kind:9022 leave and re-claim that stays `already_member` without re-adding; the v1 token path; and the shared body's archived refusal directly. `adding_a_member_auto_joins_default_channels` is the first Postgres test for kind:9030 add, with the same leave-then-repeat step. Deleting the call at any of the three sites fails the matching test (verified by mutation). `test_invite_claim_auto_joins_default_channel` is the e2e smoke, run against a local relay: the claimer receives the kind:44100 and is on the kind:39002 roster. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
… 9002 authz Address the review of d65bf98 (R1) and 848cf49 (R2). - `get_accessible_channels` now selects `c.auto_join`. It feeds `row_to_channel_record`, which reads that column unconditionally since R1, so every call returned `ColumnNotFound` and `legacy_channel_types_postgres_tests` was red. Every other SELECT that feeds either channel-record mapper (channel.rs, dm.rs, `get_channel_tx`) already carried the column. - The kind:9002 validator's trailing authorization arm fails closed: an event whose only recognized tags are neither privileged nor member tags is accepted only when it sets `auto_join` (decided above by relay role), and refused otherwise. Behaviour today is unchanged; a tag added to `RECOGNIZED_TAGS` but to neither list can no longer pass with no authorization. - `list_auto_join_channels` filters `channel_type = 'stream'` itself, so an out-of-band `auto_join` on a forum or DM row cannot auto-join anyone even though the 9002 validator already refuses to set it there. The cap warning now says what the `rows.len() == LIMIT` check can prove. - NOSTR.md "Default channels" states that auto-join fires only at the roster-insert sites, so a community that does not require relay membership advertises the kind:39000 tag but auto-joins nobody, and a client wanting parity there self-joins on the tag. Verified: fmt; clippy -D warnings on buzz-db and buzz-relay; the buzz-db Postgres lane (580 passed, legacy_channel_types_postgres_tests included); the relay side_effects unit tests and the auto_join / edit_metadata Postgres tests. `unarchive_of_an_active_channel_is_refused` fails with `Sqlx(PoolTimedOut)` on this machine with and without this change (reproduced on untouched HEAD); it is the known lane flake, not a regression. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
`list_auto_join_channels` stopped at 32 rows while kind:9002 kept accepting `auto_join=true` on any number of channels, so with 33 flagged channels the newest one carried the kind:39000 `auto_join` tag but no claim or kind:9030 add ever joined anyone to it, and nothing retried because the person was already a relay member (Codex review on #8594). Drop the cap rather than enforce it at flag time: the kind:9002 transaction holds only the shared community admission lock and the one channel's row lock, so a count-based refusal would need a new community-wide lock to be race-free, and a private or archive flip on a flagged channel would still let the stored count drift past it. The loader now returns every effective channel, oldest first; the set is bounded by the community's channels, each flagged by a relay owner or admin, and NOSTR.md says so. Test: `every_default_channel_is_listed` (Postgres lane) flags 33 channels and asserts the loader lists all of them in creation order; putting the `LIMIT` back fails it (verified by mutation). Verified: fmt; clippy -D warnings on buzz-db and buzz-relay (all targets); the four auto_join Postgres tests. `auto_join_is_relay_admin_only_...` failed once at its first query with the known `PoolTimedOut` lane flake and passed on a solo rerun. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
Rebasing onto main put this branch's `channels.auto_join` migration at 0076, after main's 0074 (user deletion fences) and 0075 (invite revocation). `migration_schema_0075_adds_invite_columns_without_rewriting_history` asserted that a full migrate ends at version 75, so it failed once 0076 existed. It now reads the latest version from the migrations directory, as `thread_window_postgres_tests` already does, so the next migration does not break it again. The renumbering itself (0074 -> 0076, with the migrator pin test resolved to main's contiguity loop at 76) was applied in the replayed "add a channel auto_join setting" commit. Every SELECT that feeds `row_to_channel_record` already selects `auto_join`, and the three auto-join call sites sit in the same arms after main's invite-revocation rewrite, so nothing else needed to change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…row lock An auto-join could add a new community member to a default channel after it was archived (jedwards27, second review on #8594). The auto-join lists the effective default channels and `join_channel_as_self` checks the archive state before the write, but `put_members` never read `archived_at`, and nothing ordered an archive against the membership write. An archive landing between the list and the insert still created the row and emitted the kind:40099, 39000/39002 and 44100. `put_members` and `put_members_with_event` now take the membership lock, the shared TTL lock and then the channel row `FOR NO KEY UPDATE`, the order a huddle join already uses, and refuse a target who is not an active member with `AccessDenied("channel is archived")` while `archived_at` is set. An auto-join that loses the race is logged and counted as a failed channel, with no row and no side effects. Active members keep their rows; only admission is refused. The review suggested treating archive and unarchive as admission changes so they take the membership lock. This fix locks the row instead, because the row is what every archive writes: `archive_channel`, a kind:9002 `archived` tag and the TTL reaper's single UPDATE, which takes no advisory lock at all and so would still race under the membership-lock version. It also leaves every archive writer unchanged. The refusal is not scoped to self-joins: the only production callers are kind:9000 (already refused on an archived channel at ingest), kind:9021 and the auto-join. No restore or import path re-adds members through `put_members`, and no test relies on adding to an archived channel. The cost is that a membership write now holds the channel row for its transaction. Writers of that row (settings writes, huddle joins, TTL refreshes, the reaper) wait for it, and reads do not. NOSTR.md "Default channels" now says a channel archived or made private while the join runs is one of the logged, unretried failures. Tests: - `put_member_waits_for_a_concurrent_archive` (buzz-db Postgres): an uncommitted reaper-style archive makes a join wait. Once the archive commits the join is refused with no row, and re-adding a removed member is refused too. In the other order, `archive_channel` waits for an admission that holds the locks, and the admitted member stays. - `auto_join_skips_a_default_channel_archived_while_it_joins` (buzz-relay Postgres): a flagged channel is archived in a held transaction. The auto-join lists it, passes the join body's check, then waits in `put_member`. After the archive commits there is no membership row, no kind:39002 entry, no kind:44100 and no `member_joined` notice. - Mutations: dropping the row lock from `lock_channel_for_member_put` fails both tests, and so does dropping the archived refusal from `put_members_on`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…ce waits Address the review of af4730a on #8594. The lock order holds; these are the comment, coverage and doc gaps it found. - `put_member_waits_for_a_concurrent_archive` now drives `put_members_with_event` against a second uncommitted archive. The kind:9000 add waits for it, then returns `Refused` with one `AccessDenied("channel is archived")` and stores no event. It gets its own archive because a write queued behind the join's membership lock would wait whether or not its own call site took the row lock. Reverting that call site to the membership lock alone fails the test (verified by mutation): the add never waits. - The re-add refusal in the same test is pinned to `"channel is archived"`, not any `AccessDenied`. - `auto_join_skips_a_default_channel_archived_while_it_joins` waits for a `Lock` wait on the `SELECT archived_at FROM channels ... FOR NO KEY UPDATE` statement, not for any session the archive blocks. - The `CHANNEL_MEMBERSHIP_LOCK_NAMESPACE` rationale says admissions also lock the channel row (`lock_channel_for_member_put`) so an archive is ordered against them, and that only removals take just this key. - NOSTR.md "Default channels": the auto-join paragraph is rewrapped, and its dangling "It" now says "The relay". Verified: fmt; clippy -D warnings --all-targets on buzz-db and buzz-relay; the buzz-db `channel_members` Postgres tests (32 passed) and the four buzz-relay auto_join Postgres tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
…ined claim Addresses the two blockers on `src/features/channels/default-channels.ts` in the PR #1624 review (threads on lines 33 and 34). Line 33, a missing marker re-adds existing members who left: absence of the `default-channels` marker now means "do nothing". `admitCommunity` reports `joined: true` only when its claim returned `joined` (never `already_member`), and `CommunityDialog` writes `pending` for that origin and viewer right after admission, so a later Back/Continue reclaim that answers `already_member` keeps it. Upgrades, new devices, cleared storage, recovered claims and code-less admissions write nothing, matching the relay contract in block/buzz#8594 ("once, at that moment ... a later kind:9022 leave stands"). No other client moment proves a new membership: a code-less admission is either an open-community newcomer or a returning member who never published a profile, so open communities get no client fallback. Line 34, one successful join strands the remaining defaults: the "non-empty verified roster -> done" early exit is gone. While `pending`, candidates are the flagged public channels the viewer has not joined (the store already excludes rostered ones), joined sequentially as before; `done` is written only when the directory read was complete and every candidate settled. Otherwise the marker stays `pending` and the next run joins only what is still missing. Optional sub-point from the same thread: a `PublishNotSent` (how the transport maps NIP-01 `rate-limited:`) or a `PublishRejected` whose text starts with `rate-limited:` no longer counts as settled. Tests: absent and `done` markers read and send nothing with an empty roster; two runs where #general's membership is retained and #welcome is still outstanding (the second joins only #welcome, then marks done); rate limits and unsent writes stay pending; `admitCommunity` reports `joined` only for a `joined` claim; the dialog writes `pending` for `joined` and nothing for `already_member`. The five fallback cases and the dialog `joined` case fail on the previous code. Module comment, `channels.md` and `communities.md` state the new guarantee. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com>
05d4310 to
9807162
Compare
github-actions
Bot
removed
the
codex-security-review-current
github-actions
Bot
added
the
codex-security-review-current
Summary
Adds default channels: relay owners and admins can flag channels that every new community member is automatically joined into.
Setting (contract)
channels.auto_joincolumn (migration0074, constant defaultfalse, so no table rewrite).["auto_join", "true"|"false"]. Authorization comes from the actor's relay roster role (owner/admin), not a channel role, so the actor doesn't need to be a channel member. API tokens needAdminChannels.streamchannels qualify. This is checked under the channel row lock, so one event can open and flag a channel in one step, and a concurrent archive can't slip past the check.auto_jointag only while the setting is effective (on, open and unarchived). Every change posts oneauto_join_changedkind:40099 notice.build_update_channel(.., auto_join),buzz channels update --auto-join <true|false>, a NOSTR.md "Default channels" section, and a NIP-CL tag-table row.Behaviour
join_channel_as_self, so auto-join uses the same path as a self-join: the channel'sjoin_roleapplies, and it emitsmember_joined40099, 39000/39002 and 44100.auto_join_default_channelsruns inline right after a member is added at three sites: the v2 invite claim, the v1 invite claim, and kind:9030 add. It does not run on login or reconnect, so a later kind:9022 leave stands.buzz_auto_join_failures_total) but never retried, because the roster membership has already committed. This deliberately departs from Review-Proven Rule 1 and is documented in NOSTR.md.buzz-admin add-memberwrites the roster row only.Review follow-ups
get_accessible_channelsnow selectsauto_join. Without it, every call failed withColumnNotFound.list_auto_join_channelsfilters tostreamchannels itself.Testing
edit_metadata_postgres_tests: who may setauto_join, the 40099 notice, the effective-state tag, and the refusals.claim_auto_joins_default_channels_once: v2 and v1 claims, and leave then re-claim.adding_a_member_auto_joins_default_channels: kind:9030 add.test_invite_claim_auto_joins_default_channelagainst a local relay.group_metadata_tags.Related
Demo
Recorded against a local relay built from this branch with throwaway identities (Alice = owner, Bob = new member); no real accounts or hosted relays involved. Relay at 141aa01, client at buzz-app bb0d97d86 (browser dev build).
Owner turns on "Starter channel for new members" on two channels. Alice creates
#generaland#welcome, then flips the switch on each (~0:25 and ~0:41). The switch greys out while the kind 9002 is in flight and only turns on once the republished 39000 carries theauto_jointag.two-starter-channels-owner-flags.mp4
A brand-new member joins and lands in a starter channel. Bob claims Alice's invite with empty storage; from ~0:18 he is reading
#welcomewith both#generaland#welcomein the sidebar, and at 0:27 opens#generalas a member with no Join prompt. The relay logged oneauto-joined default channelline per flagged channel.two-starter-channels-new-member-lands.mp4
🤖 Generated with Claude Code