[bradseiler]

Summary

Repair historical v2 invitation revocation after the schema-only migration merged in #8237. The operator script revokes affected legacy invitations and clears the legacy marker on examined active-community rows, including spared invitations. Later timeouts remain temporary; active bans permanently revoke invitations.

Related issue

Companion to #8237. First layer of the community security stack: #8529 → #8241 → #8239 → #8235. #8237 and #8242 are already merged.

Operation

Use schema 0075, with the populated upgrade fixture starting through 0074. Any shared or staging database that applied an older invitation candidate needs its migration history and schema reconciled by its owner first. Never reset a human or shared database.

Follow TESTING.md. Prevent invitation mint/claim, ban lifts and new timeout/mute writes across every writer. If existing controls cannot enforce this, stop all relay instances and other writers and drain their work.

Before execution, capture an operator-approved snapshot of candidate invitation IDs, community IDs, revoked_at and legacy_admission. Revocation is permanent; retain the snapshot for audit and any separately reviewed recovery. After schema 0075, run psql -X -v ON_ERROR_STOP=1 -f scripts/maintenance/backfill_invite_revocations.sql through the authorized operator connection. Run at top level, outside a caller transaction. The two-minute default timeout covers the whole invocation. A lock timeout also preserves committed batches; resolve contention and rerun under the same writer controls. For a statement timeout, committed batches survive; the current batch rolls back and the zero check is skipped. Keep writer controls in place and rerun. If the repaired prefix consumes the budget, choose a larger finite budget with -v invite_repair_timeout=10min. Require zero unrepaired invitations in active communities, then rerun and confirm unchanged markers and timestamps. Keep skipped or fenced communities closed until repaired. Reopen only on the new code after all checks pass.

Testing

Open review finding — invite-batch scaling: discussion_r4519323027 remains unresolved. McKeon independently confirmed at 4a3ee15016 that each 500-row batch rescans the repaired prefix within a community, producing quadratic growth. His real-schema PostgreSQL 16 reproduction measured 0.29s / 3.87s / 12.95s at 10k / 50k / 100k invitations. He recommends fixing the per-community invite cursor before approving the repair to run. A larger finite timeout is a documented workaround, but the scaling defect remains open. Luke's community-cursor fix addresses a separate outer loop. Brad retains ref ownership; no implementation, branch change or thread resolution accompanies this note.

Current head 4a3ee15016b2ba5e6561e6f0819600ddf1d795b8 in native stack 8725, rebased by Brad through GitHub onto main 44b3c4823. Brad owns further ref updates.

Luke's four review items are addressed in separate commits: indexable successor lookups, a failing exit status for nonzero validation, lock-timeout recovery documentation, and async psql processes using the connection URI. A follow-up restores valid random IDs in the 12-community fixture; the schema rejects nil UUIDs.

Historical independent validation at exact 53385dc791 passed all 34 relay_invite:: cases against the real PostgreSQL 16 schema. Buzz-db clippy passed with all targets and warnings denied. A mutation that skips repair fails eight tests through the process exit-status assertion. The preceding head 04f539b085 failed four fixture inserts and remains a failed result.

At current 4a3ee15016, McKeon additionally passed 35/35 real-schema tests covering relay_invite:: and the masked-activity fence test using a postgres:// URI. This does not turn the historical full PostgreSQL run into a pass.

A separate minimal UUID-table benchmark reproduced the old query's generic plan without an Index Cond. After six prepared executions, the successor query uses Index Cond: (id > $1). The old/split timings were 4,215/24 ms at 10k communities and 17,410/50 ms at 20k. This benchmark and the simplified script harness do not substitute for real-schema package tests.

Normal pre-push hooks passed on integrated head 7ee3cf92f59a96349099d48fc115e5b8bb6a9716, including the branch-skew guard, unit tests, desktop checks/tests, Tauri checks, mobile checks/tests and file-size checks. These hooks validate the checked-out top tree; hosted CI remains the gate for each individual PR head.

Current-head hosted CI is terminal and green, including runtime unit and PostgreSQL jobs. McKeon verified unchanged carried patches (8/20/21/6), with no overlap between the newer main delta and stack files. Luke's four fixes and the child PR clearances remain valid; overall #8529 clearance is superseded by the open invite-batch scaling finding below.

Historical local validation at integrated head 7ee3cf92f5: buzz-db/buzz-relay clippy passed with all targets and warnings denied; package tests passed 2,035/2,035; desktop build, Tauri check/tests, web build and mobile tests passed. The full PostgreSQL lane finished 1,503/1,505, with two failures still unresolved in that run: cluster_global_sample_writer_fails_closed_when_activity_is_masked and hanging_redis_peer_times_out_before_the_health_listener_binds. The runner constructed a postgresql:// URI; the fence test requires literal postgres://, explaining its prefix panic. The Redis test exceeded its scheduling bound at 8.03 seconds; that local failure remains unexplained. No full local just ci pass is claimed. These receipts remain attributed to the historical head.

Human review and verification remain required. Existing review decisions are preserved. No merge, deployment, production repair or human acceptance was performed.

Generated with Codex

[github-actions]

🔐 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: 44b3c4823f0cf1f7715f85dbf5566cb64e848162...4a3ee15016b2ba5e6561e6f0819600ddf1d795b8
• 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.

Findings

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

Notes

• Read-only inspection; scripts, builds, and tests were not executed.
• Repair safety depends on the documented writer drain and admission restrictions remaining enforced throughout repair and validation.

---

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

[wpfleger96]

🤖 Automated review of 425d89b4 — no blocking findings.

The repair script meets the docs/database.md Rule 3 requirements for a pre-framework one-off repair: it is idempotent, works in batches of 500 that each commit from a top-level DO block, takes the shared community deletion lock and re-reads the community's state inside the same transaction, and ends with a validation query that expects 0. Its revocation rule matches the claim-time has_revoking_restriction and is_canonical_invite_pubkey checks in relay_invite.rs. Its lock order (shared deletion lock, then invitation-admission lock, then invite rows) matches the claim, ban and owner-link writers. Run inside a caller transaction, the first internal COMMIT errors and nothing persists.

The PostgreSQL CI lane on this head passed every new and changed repair test. A separate hands-on run against a fresh disposable PostgreSQL 17 database (migrated through 0074, then 0075) seeded 1,226 invitations, 1,224 of them in one community so the repair crossed two batch boundaries. The checked-in script then ran twice through psql. Both runs reported zero unrepaired invites, and the second run left every invitation row byte-identical. Legacy restriction history, malformed issuers, owner restrictions, native active bans, timeout-only history, other tenants, existing revocation timestamps and a fenced community all came out as expected.

Minor findings:

1. scripts/maintenance/backfill_invite_revocations.sql:17: SET statement_timeout = '2min' limits the whole DO block across every community, not each batch. The internal COMMITs do not reset the timer: a 1-second timeout around three 0.6-second iterations that commit between them is cancelled at 1.0 s on PostgreSQL 17. On a large relay the script would stop before its validation query and need repeated reruns. Either drop the statement timeout, since lock_timeout already limits lock waits and batches are capped at 500, or document in TESTING.md that a timeout means rerun until the validation query runs.
2. scripts/maintenance/backfill_invite_revocations.sql:71, :85, :150: the same issuer-or-current-owner lookup against community_bans is written out three times (has_retained_restriction, has_active_ban, and the validation query). Use one lateral over community_bans WHERE community_id = … AND pubkey IN (issuer, owner) with bool_or(true) / bool_or(banned AND …), the shape principal_restricted uses, so the copies can't drift from each other or from the application predicate.
3. scripts/maintenance/backfill_invite_revocations.sql:1 and TESTING.md:225: "This companion must be separately approved before core deployment" and "The separately approved companion provides the operator command" describe the release process rather than the script, and "companion" isn't defined for a reader who arrives at either file. Name it as the historical invitation repair script. The TESTING.md paragraph also has a stray line break at "lift the first restriction before / upgrade" (:242).
4. crates/buzz-db/src/store/relay_invite/upgrade_postgres_tests.rs:388: start_invite_repair_script copies the connection-parameter parsing (host, port, user, hand-split password, current_database()) from apply_pgschema_to_existing_database (:558). Extract one shared helper. The hand-split password isn't percent-decoded, so that helper is also the one place to fix encoded passwords.
5. crates/buzz-db/src/store/relay_invite/upgrade_postgres_tests.rs:618: the automated repair fixtures never exceed 500 invites in one community and never fail a run after an earlier batch committed. Extend an existing repair test with more than 500 invites in one community, including spared and already-revoked rows, and check that a rerun after a later-batch failure completes and preserves the earlier timestamps.

[wpfleger96]

🤖 Automated review of 96a2686b — no blocking findings.

This head rebases the change onto f3a7bab0 without altering it. The repair script, both testing documents and revocation_postgres_tests.rs are byte-identical to 425d89b4. The only change is in upgrade_postgres_tests.rs, which picks up main's correction: the test now checks that migration 75 succeeded, not that it is the latest. Migrations 0076–0078 add artifact indexes and timestamps, channel_members.sync_modified_at and communities.artifact_policy_modified_at. The script reads communities, relay_invites, community_bans and users and updates only relay_invites, so none of those migrations touches anything it depends on. The revocation rule still matches the claim-time checks in relay_invite.rs, and the lock order (shared deletion lock, then the invitation-admission lock, then invite rows) still matches the claim, ban, timeout and owner-link writers.

The PostgreSQL CI lane passed every upgrade and repair test on this head. A separate hands-on run used a fresh disposable PostgreSQL 17.10 database, seeded at 0074 and then migrated through 0078. It held 1,226 invitations, 1,224 of them in one community with spared, already-revoked and to-be-revoked rows interleaved, so the repair committed batches of 500, 500 and 20. The checked-in script ran twice through psql -X -v ON_ERROR_STOP=1. Both runs reported zero unrepaired invites, and both full invitation-row snapshots had the same SHA-256. Existing revocation timestamps, other tenants' rows and the fenced-community skip matched independently computed expectations. The eight upgrade_postgres_tests tests also passed against that instance.

Minor findings. The first five carry over unchanged from the review of 425d89b4:

1. scripts/maintenance/backfill_invite_revocations.sql:17: SET statement_timeout = '2min' limits the whole DO block across every community, not each batch, because the internal COMMITs do not reset the timer. On a large relay the script can stop before its validation query. Either drop the statement timeout, since lock_timeout already bounds lock waits and batches are capped at 500, or document in TESTING.md that a timeout means rerunning until the validation query runs.
2. scripts/maintenance/backfill_invite_revocations.sql:71–101 and :150–165: the issuer-or-current-owner lookup against community_bans is written out three times (has_retained_restriction, has_active_ban and the validation query). Collapse each into one lookup over community_bans WHERE community_id = … AND pubkey IN (issuer, owner), the shape principal_restricted uses at relay_invite.rs:232–238, so the copies cannot drift from each other or from the application predicate. This corrects the earlier review, which described principal_restricted as using bool_or. It is a single EXISTS … pubkey IN (…) query.
3. scripts/maintenance/backfill_invite_revocations.sql:1 and TESTING.md:196, :225: "companion" and "separately approved" describe the release process rather than the script, and a reader who arrives at either file has no definition for "companion". Call it the historical invitation repair script. There is also a stray line break at "lift the first restriction before / upgrade" (TESTING.md:242–243).
4. crates/buzz-db/src/store/relay_invite/upgrade_postgres_tests.rs:392–407 and :558–573: start_invite_repair_script copies the connection-parameter parsing from apply_pgschema_to_existing_database. Extract one shared helper. The hand-split password isn't percent-decoded, so that helper is also the one place to fix encoded passwords.
5. crates/buzz-db/src/store/relay_invite/upgrade_postgres_tests.rs:618–686: the automated repair fixtures insert one invitation per community, so no test crosses a 500-row batch inside a community or fails after an earlier batch has committed. Extend an existing repair test with more than 500 mixed invites in one community, and check that a rerun after a later-batch failure completes and preserves the earlier timestamps.
6. TESTING.md:210–214 and :237–248: the steps say to apply schema 0075, but 0075 is only the invitation migration. The relay at this head also needs 0076–0078 (for example, channel_members.rs:936 writes sync_modified_at from 0077), and those migrations have their own index-prebuild procedure for populated tables in docs/artifact-sync-deployment.md. Add a sentence that separates "the repair requires 0075" from "a current-relay upgrade applies through 0078", and link that procedure, so an operator doesn't take "apply 0075" as the complete upgrade recipe.

[wpfleger96]

🤖 Automated review of b7d7d61c: no blocking findings.

This head rebases the change onto 5b738eb6 and leaves it unchanged. The PR's patch ID matches 96a2686b, and the repair script, root TESTING.md and both changed Rust files are byte-identical. Main has changed underneath it since f3a7bab0, though, so this review checked that the script still agrees with the application:

• Revocation rule. has_revoking_restriction, is_canonical_invite_pubkey and principal_restricted in relay_invite.rs are unchanged; main's edits to that file only touch test scaffolding (the move to ScratchDb). moderation.rs adds an own_banned field and a read-time wrapper to restriction_state without changing the ban predicate. The claim path in api/invites.rs still ends in claim_relay_invite_with_owner, and claim_auth.rs only adds access reporting.
• Lock order. Mint/claim, ban/timeout, admin enforcement and first-owner writes still take the shared deletion lock, then invitation admission, then invite rows. The new auto-join path runs in separate channel-membership transactions only after the claim commits, so it holds no invite locks and writes no invitations, restrictions or owner links. The new verified-profile publish reads community_bans but writes nothing the script depends on.
• Migrations 0079–0081 add nip_fi_profile_heads, channels.auto_join and a profile-name index. None touches communities, relay_invites, users or community_bans.

The PostgreSQL CI lane on this head passed every upgrade and repair test. A separate hands-on run used a fresh disposable PostgreSQL 17.10 database, seeded at 0074 and then migrated through 0081. It held 1,226 invitations, 1,224 of them in one community, so the repair committed batches of 500, 500 and 20. The checked-in script ran twice through psql -X -v ON_ERROR_STOP=1. Both runs reported zero unrepaired invites, and both full invitation-row snapshots had the same SHA-256. Every case matched independently computed expectations: legacy restriction history, malformed and uppercase issuers, current-owner restrictions, native active bans, lifted bans, timeout-only history, existing revocation timestamps, another tenant and a fenced community. Users, bans, channels and communities were unchanged, including auto_join and guest join-role controls. The eight upgrade_postgres_tests tests also passed against that instance.

Minor findings. The first five carry over unchanged from the earlier reviews; the sixth is updated for the new main:

1. scripts/maintenance/backfill_invite_revocations.sql:17: SET statement_timeout = '2min' limits the whole DO block across every community, not each batch, because the internal COMMITs do not reset the timer. On a large relay the script can stop before its validation query. Either drop the statement timeout, since lock_timeout already bounds lock waits and batches are capped at 500, or document in TESTING.md that a timeout means rerunning until the validation query runs.
2. scripts/maintenance/backfill_invite_revocations.sql:71–101 and :150–165: the issuer-or-current-owner lookup against community_bans is written out three times (has_retained_restriction, has_active_ban and the validation query). Collapse each into one EXISTS over community_bans WHERE community_id = … AND pubkey IN (issuer, owner), the shape principal_restricted uses in relay_invite.rs, so the copies cannot drift from each other or from the application predicate.
3. scripts/maintenance/backfill_invite_revocations.sql:1 and TESTING.md:196, :225: "companion" and "separately approved" describe the release process rather than the script, and a reader who arrives at either file has no definition for "companion". Call it the historical invitation repair script. There is also a stray line break at "lift the first restriction before / upgrade" (TESTING.md:242–243).
4. crates/buzz-db/src/store/relay_invite/upgrade_postgres_tests.rs:392–407 and :558–573: start_invite_repair_script copies the connection-parameter parsing from apply_pgschema_to_existing_database. Extract one shared helper. The hand-split password isn't percent-decoded, so that helper is also the one place to fix encoded passwords.
5. crates/buzz-db/src/store/relay_invite/upgrade_postgres_tests.rs:618–686: the automated repair fixtures insert one invitation per community, so no test crosses a 500-row batch inside a community or fails after an earlier batch has committed. Extend an existing repair test with more than 500 mixed invites in one community, and check that a rerun after a later-batch failure completes and preserves the earlier timestamps.
6. TESTING.md:210–214 and :237–248: the steps say to apply schema 0075, but a current relay now needs migrations through 0081. Migrations 0076 and 0081 each need an index prebuilt on populated databases (docs/artifact-sync-deployment.md and the Deployment section of docs/profile-search.md), and the auto-join reads need 0080. Add a sentence that separates "the repair requires 0075" from "a current-relay upgrade applies through the latest migration", and link both procedures, so an operator doesn't take "apply 0075" as the complete upgrade recipe.

[wpfleger96]

🤖 Automated review of 988a9b4d: no blocking findings.

This head rebases the change onto ec21753e and leaves it unchanged. git range-diff shows both commits identical to b7d7d61c, and the repair script, both TESTING.md files and both changed Rust test files are byte-identical. Main has one database change since 5b738eb6 that could matter: migration 0082 and the new community NIP-FI auth upgrade writer in community_auth.rs.

• Tables. 0082 adds communities.requires_nip_fi and communities.policy_generation and creates community_auth_upgrades. The upgrade writer updates only those columns and inserts receipts. It never writes relay_invites, community_bans, users or community owner rows, so nothing the script reads or writes changes meaning.
• Locks. The upgrade takes its own advisory lock, then the shared deletion lock, then communities ... FOR NO KEY UPDATE. The script takes the shared deletion lock and the invitation-admission lock, then reads communities without a row lock. The two never wait on each other's row locks, and the upgrade never takes the invitation-admission lock, so no new lock cycle appears.
• Revocation rule. The claim path, relay_invite.rs and the moderation predicates are unchanged on main since the last review, and no claim or ban code reads the new columns.

CI passes at this head, including the PostgreSQL upgrade and repair tests.

Minor findings. The first five carry over unchanged from the earlier reviews; the sixth is updated for the new main:

1. scripts/maintenance/backfill_invite_revocations.sql:17: SET statement_timeout = '2min' limits the whole DO block across every community, not each batch, because the internal COMMITs do not reset the timer. On a large relay the script can stop before its validation query. Either drop the statement timeout, since lock_timeout already bounds lock waits and batches are capped at 500, or document in TESTING.md that a timeout means rerunning until the validation query runs.
2. scripts/maintenance/backfill_invite_revocations.sql:71–101 and :150–165: the issuer-or-current-owner lookup against community_bans is written out three times (has_retained_restriction, has_active_ban and the validation query). Collapse each into one EXISTS over community_bans WHERE community_id = … AND pubkey IN (issuer, owner), the shape principal_restricted uses in relay_invite.rs, so the copies cannot drift from each other or from the application predicate.
3. scripts/maintenance/backfill_invite_revocations.sql:1 and TESTING.md:196, :225: "companion" and "separately approved" describe the release process rather than the script, and a reader who arrives at either file has no definition for "companion". Call it the historical invitation repair script. There is also a stray line break at "lift the first restriction before / upgrade" (TESTING.md:242–243).
4. crates/buzz-db/src/store/relay_invite/upgrade_postgres_tests.rs:392–407 and :558–573: start_invite_repair_script copies the connection-parameter parsing from apply_pgschema_to_existing_database. Extract one shared helper. The hand-split password isn't percent-decoded, so that helper is also the one place to fix encoded passwords.
5. crates/buzz-db/src/store/relay_invite/upgrade_postgres_tests.rs:618–686: the automated repair fixtures insert one invitation per community, so no test crosses a 500-row batch inside a community or fails after an earlier batch has committed. Extend an existing repair test with more than 500 mixed invites in one community, and check that a rerun after a later-batch failure completes and preserves the earlier timestamps.
6. TESTING.md:210–214 and :237–248: the steps say to apply schema 0075, but a current relay now needs migrations through 0082. Migrations 0076 and 0081 each need an index prebuilt on populated databases (docs/artifact-sync-deployment.md and the Deployment section of docs/profile-search.md), and the auto-join reads need 0080. Add a sentence that separates "the repair requires 0075" from "a current-relay upgrade applies through the latest migration", and link both procedures, so an operator doesn't take "apply 0075" as the complete upgrade recipe.

[TheSentinel454]

🤖 Reviewed at 20ef0f3d: approach looks good and no blockers. One P1 performance fix and three optional notes inline.

[wpfleger96]

🤖 Reviewed at 20ef0f3d. The revocation rule still matches the claim path in relay_invite.rs and the lock order matches the application writers. One finding not covered by the existing comments, inline.

[wpfleger96]

🤖 Separate from the community-walk comment on line 39: each 500-row batch inside one community also restarts at that community's first invite. ORDER BY invite.id LIMIT 500 walks the (community_id, id) primary key from the beginning, and rows repaired by earlier batches are filtered out rather than skipped, so a community with M legacy invites costs about M²/1000 row visits, each also evaluating the restriction lookups. On a throwaway PostgreSQL 16 with one community of unrestricted legacy invites, this script took 9.6s at 50k invites and 35.9s at 100k, so the default 2min budget runs out at roughly 180k invites in a single community.

Keeping a per-community cursor fixes it: reset last_invite for each community, add AND invite.id > last_invite, and set it to the highest invite.id the batch updated (for example via UPDATE ... RETURNING invite.id). With that change the same 100k community took 1.05s and ended in the same state (no legacy markers left, zero check passing). Seed or branch the first batch the same way as the community-walk fix so the predicate stays indexable under the cached generic plan.