Skip to content

feat(relay): add FI default for new communities - #9172

Closed
bradseiler wants to merge 5 commits into
mainfrom
honor/nipfi-new-communities
Closed

bradseiler wants to merge 5 commits into
mainfrom
honor/nipfi-new-communities

Conversation

@bradseiler

@bradseiler bradseiler commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Add BUZZ_NIP_FI_REQUIRE_NEW_COMMUNITIES, default false, so operators can require Federated Identity (NIP-FI) for new tenant communities without upgrading existing ones. Both operator creation modes persist the requirement at generation 1 in the same transaction as issuer bindings. Existing rows and idempotent retries retain their policy and bindings.

Enabling this setting requires the database community source and a valid provision binding. New unbindable hosts return a coded HTTP 400, including in Off mode. Deployment/control-host startup bootstrap keeps its existing behavior, including an unrequired, unbound reserved control host. The creation setting is independent of BUZZ_NIP_FI_UPGRADES_ENABLED, which gates upgrades of existing communities.

Stage the database source and issuer configuration first, then enable this setting when the issuer can sign users into new communities. In Shadow, verify a new community's NIP-11 requires_nip_fi and operator issuer binding, while an existing optional community stays optional. Disabling the setting affects future creations only; born-required communities remain required, with no per-community rollback endpoint. Mode semantics stay unchanged: Off bypasses FI relay-wide, Shadow enforces stored requirements, and Enforce requires FI everywhere.

Related issue

No duplicate creation-default change found. Related: merged #8991 handles existing-session closure after policy upgrades. This change adds a creation default; it adds no schema migration or retrofit of existing communities.

Testing

A local relay passed 16 signed HTTP scenarios across restarts with the setting absent, enabled, and disabled. Both operator modes preserve existing rows, create required rows with issuer bindings, and return coded 400 refusals for unbindable new hosts. Reserved control-host bootstrap stays unrequired and unbound. Disabling the setting preserves stored requirements; NIP-11 still reports the Off-mode bypass.

At b3d067e87877995be6a029e440dd2fb81dfa9fa4: 1,967 relay/db package tests and 1,339 PostgreSQL tests passed, as did all-target Clippy, the standalone build, and normal pre-push hooks. The full repository gate passed at the earlier integration head 9d8d18f7; unrelated desktop/mobile checks were not repeated after the final relay-only merge.

To reproduce the database journeys with local PostgreSQL and Redis configured per TESTING.md, run scripts/postgres-test-run.sh --locked -p buzz-db -p buzz-relay --lib --tests. The new journeys cover creation, retries, binding preservation, coded refusals and bootstrap.

Independent source, final merge and PR description review found no remaining blockers. The three regression tests passed their baseline; all ten deliberate mutations failed assertions, including issuer replacement on retry and each creation/bootstrap/config guard. Human runtime confirmation remains pending; this PR stays draft.

Generated with Codex

bradseiler and others added 5 commits November 5, 2026 19:38
Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz>
Signed-off-by: Brad Seiler <seiler@squareup.com>
Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz>
Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz>
Signed-off-by: Brad Seiler <seiler@squareup.com>
…pfi-new-communities

* commit '4e28176f13957f4bb77f58c5b4aa0ad137017423':
  fix(relay): close legacy sockets after community FI upgrades (#8991)

Signed-off-by: Brad Seiler <seiler@squareup.com>
@bradseiler
bradseiler marked this pull request as ready for review November 6, 2026 17:01
@bradseiler
bradseiler requested a review from a team as a code owner November 6, 2026 17:01
@bradseiler
bradseiler deployed to codex-review November 6, 2026 17:01 — with GitHub Actions Active
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Nov 6, 2026
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Note: This is an automated, security-focused review generated by Codex.
Use it as a supplement to human review; false positives are possible.

Scope

  • Exact PR diff: 1bb2ad443d4ee4512dd0927888ad21a4a1c041bc...b3d067e87877995be6a029e440dd2fb81dfa9fa4
  • Model: gpt-6.1-sol

💡 Click "edited" above to see earlier reviews for this PR.


Review Summary

Overall Risk: NONE

No concrete security, correctness, or reliability regressions found in the authorized PR range after tracing provisioning authorization, transactional policy creation, retries, issuer bindings, caching, and admission enforcement.

Findings

No concrete security, correctness, or reliability findings were identified.

Notes

  • Review used read-only source and history inspection. Builds and tests were not executed, as instructed.

Generated by Codex Security Review |
Requested by: @bradseiler |
Workflow run

@bradseiler bradseiler closed this Nov 6, 2026
bradseiler added a commit that referenced this pull request Nov 6, 2026
Trusted operators can choose `initial_requires_nip_fi` on create-only
community provisioning. True commits required policy (generation 1),
validated issuer bindings and the owner with the new community in one
transaction. Omitted, null or false preserves optional creation; a
non-null field in legacy ensure mode returns coded 400. Missing
effective bindings refuses required insertion without leaving a row.

The response reports policy read from the stored row. Same-owner
create-only replay now accurately reports `existed` instead of
`created`; it stays successful and read-only, including after policy
disable or issuer-config changes. Different-owner conflict, legacy
ensure, bootstrap and the operator authorization gate retain their
behavior. This adds no schema changes or server-wide default flag.

Upgrade the whole serving relay fleet before callers request true. An
old writer may ignore the field and insert an optional row: response
checks detect it but cannot undo it, and later lookup remains read-only.
Turn caller targeting off before relay rollback. Mode off stores policy
but bypasses enforcement, independently of the upgrade gate.

### Related issue

Replaces closed #9172. The separate lifecycle API remains in #9176.

### Validation

Draft head: `bf16a1e0aba102c3be86f9621bd53d2cae98661c`. Independent
source review covers this head. Formatting, diff sanity, Clippy, build,
all 14 signed HTTP scenarios and full local `just ci` pass at this head.
GitHub reports 59 successful and 38 skipped checks, with none pending or
failing in the collected snapshot.

At parent `b9d9423a6058387f42c310f5a7e4dbe0c8d8b40f`, all 1,978 package
tests and 1,346 PostgreSQL tests passed (PostgreSQL concurrency 2). Four
deliberate mutations were detected and restored. Clippy then found seven
needless borrows in the new HTTP test; this head removes those borrows
only. Earlier validation passed 1,344/1,346 PostgreSQL tests at
concurrency 8; the replay assertion was corrected and the timeout
disappeared at concurrency 2. Failed-run evidence is retained.

At this head, the first mandatory push-hook attempt failed all nine
boot-lifecycle tests on child startup deadlines. A subsequent serial run
passed all nine (seven PostgreSQL-only cases skipped), with unchanged
deadlines and source. The first diagnostic was interrupted during
compilation without a result; all evidence is retained. A normal-hook
retry was stopped when the author explicitly authorized one push with
`--no-verify` for this exact commit. That exception skips local pre-push
hooks for publication only; failed hooks and remaining validation still
gate readiness.

Explicit human runtime confirmation remains a separate acceptance step;
agents have not attested it.

Generated with Codex


Validation update at the same draft head: Clippy, build and all 14
signed HTTP workflow scenarios passed, including atomic required
creation, stored-policy replay after disable without a current binding,
and refusal without leaving a row. The first live harness run stopped
after 12 checks because its SQL helper fetched rows after an UPDATE
without RETURNING; that harness query was corrected and the complete
workflow passed. Relay source stayed unchanged.

The full local `just ci` run failed in `mobile-check`: Dart formatting
reported 266 changes among 1,068 files, with unresolved `flutter_lints`
configuration warnings. The format check used `--output=none`; the
worktree remains clean. Repository validation is therefore not green.
That failure is retained as historical evidence; see the successful
setup-corrected rerun below. Explicit human runtime confirmation remains
separate.


Final local validation: after `just mobile-install`, `mobile-check`
passed without formatting edits, and full `just ci` completed
successfully at `bf16a1e0aba102c3be86f9621bd53d2cae98661c`. Tracked
files remained clean after installation and after the complete run. This
confirms the earlier mobile formatting failure resulted from missing
worktree dependencies. All failed runs remain preserved. GitHub's
current ready-for-review state is preserved; no agent added
`buzz-review-completed` or attested human runtime acceptance.

---------

Signed-off-by: Brad Seiler <seiler@squareup.com>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@buzz.block.builderlab.xyz>
Co-authored-by: Honor <e0bf479eeebe47dde1e35705212a4df70616a7bc834d907ddfb18cf0405f8b1b@seiler-test.communities.public.builderlabstaging.xyz>

This branch was successfully deployed

1 active deployment
codex-review — b3d067e8 Deployed Nov 6, 2026 by bradseiler via Run Codex Security Review #13504
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex-security-review-current The posted Codex security review matches its recorded range.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant