refactor(relay): extract NIP-29 membership authorization - #7285
Merged
Conversation
…l_authz `validate_admin_event` carried the kind:9000/9001/9022 authorization policy inline in a 452-line `async fn` over `&Arc<AppState>`, so none of it could be tested without Postgres and Redis. Last-owner protection alone was restated five times across three shapes, in two of which it had drifted to a different error string. Add `handlers::channel_authz`: data-only inputs, typed decisions, no state handle. `validate_admin_event` keeps every database read and calls the pure policy — the same shell/pure split `handlers::moderation_authz` already uses. The five last-owner restatements collapse to one `is_sole_owner` predicate. Both historical phrasings are preserved as distinct `ChannelAuthzError` variants, so no client-visible message changes: the rule is defined once, the wording stays per call site. kind:9001 self-removal and kind:9022 leave were character-identical and now share `decide_self_departure`. The buzz-db last-owner guards stay as independent defence in depth. Widen the `just test-unit` nextest filter to select the relay's pure authorization-decision tests. They ran in no lane before: `test(/^api::admin::/)` never matched them and the PostgreSQL lane pairs `--run-ignored ignored-only` with a `postgres_tests::` default-filter, so 14 existing tests in `handlers::side_effects` and `handlers::moderation_authz` could go red and still ship green. Scoped to the three decision modules rather than all of `handlers::`, which is mostly Postgres-backed. Behaviour is unchanged: the full `e2e_relay` suite against a live relay is identical before and after (45 pass; `test_unarchive_emits_member_added_ notification` fails on both, a pre-existing kind:9002 failure). Refs: TheSentinel454#24 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: tornquist <tornquist@squareup.com>
🔐 Codex Security Review
|
Exercise kind 9001 and 9022 self-departure through the live relay path, pinning the exact accepted status and OK-frame message for sole owners, nonmembers, and co-owner departures. Mirror the three infra-free relay handler modules from the nextest unit expression in the cargo-test fallback so either runner covers the same authorization decisions. Signed-off-by: tornquist <tornquist@squareup.com> Co-authored-by: Codex <noreply@openai.com>
Give the six kind 9001 and 9022 departure tests one shared filter and invoke it from the required Relay E2E job. Pin the workflow selection in the existing required-context contract test. Signed-off-by: tornquist <tornquist@squareup.com> Co-authored-by: Codex <noreply@openai.com>
TheSentinel454
marked this pull request as ready for review
September 3, 2026 20:54
Contributor
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
wpfleger96
approved these changes
Sep 4, 2026
wpfleger96
left a comment
wpfleger96
left a comment
Member
There was a problem hiding this comment.
🤖 Approving after a three-lane coordinated review at head b9dec5cc7593008e3217521edcd69862caeb2f39:
- Two independent source reviews (Paul, Thufir): the extracted
channel_authz.rspolicy is branch-for-branch equivalent to the removed inline logic for kinds 9000/9001/9022 — gate ordering, guards, and all ten client-visible error strings preserved (including the intentionally distinct validator/applier last-owner wording). DB reads stay inside_effects.rs; no new abstraction. Test wiring verified end to end: nextest expression, plain-cargo fallback parity, and thenip29_departure_wireselector pinned in required Relay E2E CI. - Independent live E2E (Gurney): fresh isolated relay stack at this SHA; all 9000/9001/9022 accept/reject journeys returned the exact expected OK frames and error strings with correct persisted roster state, and the checked-in CI selector passed 6/6.
No findings at any severity.
wpfleger96
pushed a commit
that referenced
this pull request
Sep 4, 2026
* origin/main: refactor(relay): extract NIP-29 membership authorization (#7285) chore(release): release Buzz Desktop version 0.5.22 (#7308) feat(desktop): preserve mentions across copy and paste (#7228) test(desktop): await Bestie drag and profile hover endpoints (#7294) Collapse contiguous join messages (#7262) Signed-off-by: Hayt <9e1c23a3fd83f61da34420e4e88ff1b16e45cafcc0cd9019eb07d4ecfa8ca9b0@buzz.block.builderlab.xyz>
nambse
pushed a commit
to nambse/buzz
that referenced
this pull request
Sep 4, 2026
## Why NIP-29 membership authorization for kinds 9000, 9001, and 9022 was embedded in a large async handler, requiring Postgres and Redis to test and repeating the last-owner rule across five call sites. This addresses [TheSentinel454#24](TheSentinel454#24) without adding the broader state/repository abstraction suggested there because HTTP and WebSocket writes already share ingestion. ## What - Extract pure, typed membership authorization decisions while leaving database reads and mutations in `validate_admin_event`. - Preserve existing client-visible errors and independent database last-owner safeguards. - Collapse five last-owner policy restatements into one predicate and share the identical self-departure policy. - Include the relay decision modules in `just test-unit`, so these tests execute in CI. ## Risk Assessment Medium-low. This touches production relay authorization, but intentionally preserves wire behavior and database defense in depth; exhaustive decision tables and relay-backed tests cover the affected paths. ## Simplification This removes repeated policy from the orchestration path and makes the rule set directly testable without introducing a repository trait or second transport path. ## Verification Verified at `15255a090797f85874921120003c645962fefaed`: - `just test-unit` — all 10 package summaries passed. An initial run hit two unrelated timing-sensitive `buzz-acp` failures; the complete retry passed 905/905 in that package. - `cargo fmt --all -- --check` - `cargo clippy --workspace --all-targets -- -D warnings` - `just file-size-check` — 10/10 policy tests passed. - `just security-review-check` — 13/13 tests passed. - Push preflight — `push-head-scope`, `branch-skew`, `file-size-check`, `rust-tests`, and `desktop-tauri-checks` passed. - Blox Postgres relay lane — 84/84 at the byte-identical source patch; full live `e2e_relay` behavior matched the clean base (45 pass and the same pre-existing kind:9002 failure on each). Generated with Claude Code --------- Signed-off-by: tornquist <tornquist@squareup.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Codex <noreply@openai.com> (cherry picked from commit cd02b69) Signed-off-by: nambse <sefa.esendemir@gmail.com>
This was referenced Sep 5, 2026
This was referenced Sep 5, 2026
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.
Why
NIP-29 membership authorization for kinds 9000, 9001, and 9022 was embedded in a large async handler, requiring Postgres and Redis to test and repeating the last-owner rule across five call sites. This addresses TheSentinel454/buzz#24 without adding the broader state/repository abstraction suggested there because HTTP and WebSocket writes already share ingestion.
What
validate_admin_event.just test-unit, so these tests execute in CI.Risk Assessment
Medium-low. This touches production relay authorization, but intentionally preserves wire behavior and database defense in depth; exhaustive decision tables and relay-backed tests cover the affected paths.
Simplification
This removes repeated policy from the orchestration path and makes the rule set directly testable without introducing a repository trait or second transport path.
Verification
Verified at
15255a090797f85874921120003c645962fefaed:just test-unit— all 10 package summaries passed. An initial run hit two unrelated timing-sensitivebuzz-acpfailures; the complete retry passed 905/905 in that package.cargo fmt --all -- --checkcargo clippy --workspace --all-targets -- -D warningsjust file-size-check— 10/10 policy tests passed.just security-review-check— 13/13 tests passed.push-head-scope,branch-skew,file-size-check,rust-tests, anddesktop-tauri-checkspassed.e2e_relaybehavior matched the clean base (45 pass and the same pre-existing kind:9002 failure on each).Generated with Claude Code