fix(relay): allow open relays to set their NIP-11 workspace icon (kind:9033) (#3998)

## Problem

The desktop deliberately shows the workspace icon editor on open relays
(#2640, gate: `canEditIcon` in
`desktop/src/features/communities/ui/EditCommunityDialog.tsx`) and
defers to the relay-side kind:9033 check — which required an admin/owner
row in `relay_members`. For a community with **no admin/owner row at
all** (the `ensure_configured_community` path, which never writes an
owner), every 9033 was refused and the icon was permanently unsettable.

**Correction from review (thanks @Dawn):** the original version of this
PR claimed nobody holds a role on an open relay. That's false —
`main.rs` bootstraps `RELAY_OWNER_PUBKEY` as owner regardless of
`BUZZ_REQUIRE_RELAY_MEMBERSHIP`, so a production open relay like
bb-block *does* have an owner row, and the old gate was refusing
everyone except that owner. The first revision of this diff would have
silently widened that owner-only control to any NIP-42-authenticated
sender.

## Fix — steward-wins

`may_set_workspace_profile(sender_role, membership_enforced,
community_has_steward)`:

| Relay mode | Community has admin/owner row? | Who may set the icon |
|---|---|---|
| Closed (`require_relay_membership=true`) | any | admin or owner
(unchanged) |
| Open | yes (e.g. bb-block) | admin or owner (unchanged posture) |
| Open | no (genuinely rosterless) | any NIP-42-authenticated sender |

- New DB helper `has_admin_or_owner(community)`
(`crates/buzz-db/src/relay_members.rs`); the call site only queries it
on open relays.
- The rosterless admit logs a `warn!` with the sender pubkey — 9033
writes no audit row and publishes no announcement event (unlike
9030/9031), so this is the only durable attribution.
- Kinds 9030–9032, NIP-42 auth, `AdminUsers` scope, ban gate, and icon
validation are all untouched.
- Doc comment fixed: cited nonexistent `canEditCommunityProfile`; real
symbol is `canEditIcon`.

## Test coverage — closing the mutation gap

Dawn's mutation testing showed the original unit tests pinned only the
helper's truth table: inverting the flag at the call site or deleting
the gate entirely survived the full suite.

- Unit tests now cover the 3-arg truth table (closed
steward-independent, open-with-steward stays steward-only,
rosterless-open admits).
- Two `#[ignore]`d Postgres integration tests drive
`handle_relay_admin_event` with a real `AppState` (open rosterless admit
→ steward appears → roleless refused again; closed relay member
refused). Wired into the Backend Integration CI job as a dedicated
nextest step.
- **Both of Dawn's mutants verified killed** at this head: flag
inversion fails 1 unit test; gate deletion fails both integration tests
(`Ok(())` where `Rejected` expected).

## CI wrinkle found and fixed: pre-existing schema drift

The first Backend Integration run of the new 9033 tests failed with
`column "icon" of relation "communities" does not exist` — migration
`0003_community_icon.sql` added the column, but `schema/schema.sql` (the
desired-state file that CI job applies via pgschema) was never updated.
Pre-existing drift, invisible until a test in that job actually wrote
the column. Fixed in `297148f62` (3-line addition to
`schema/schema.sql`).

## Receipts (at `1b4b52db8` code / `297148f62` head)

- `cargo test -p buzz-relay`: 835 pass, 1 fail —
`api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo`,
pre-existing (fails identically at the old base and on clean main);
`telemetry::trace_context_lookup_does_not_enable_callsites` is a known
order-dependent flake, passes in isolation.
- `cargo test -p buzz-db`: 94 pass.
- Both ignored integration tests pass live against local Postgres.
- `cargo fmt --all -- --check`: clean.
- Live-local pass per TESTING.md at this head (release build, relay on
:3199, real WS + NIP-42 via nak):
- open rosterless: roleless key sets icon → NIP-11 serves it; `warn!`
with sender pubkey in the relay log
- open + owner row inserted: fresh roleless key refused ("must be admin
or owner"); owner sets icon
- closed relay (owner bootstrapped, `BUZZ_RELAY_PRIVATE_KEY` set): plain
member refused, owner sets icon, `javascript:` URL rejected, empty icon
clears (NIP-11 → null)

---------

Signed-off-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
Co-authored-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz>
This commit is contained in:
Tyler
2026-08-01 12:03:31 -04:00
committed by GitHub
co-authored by npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d
parent 3d7712cc36
commit 5765fc74b7
5 changed files with 343 additions and 2 deletions
+12
View File
@@ -692,6 +692,18 @@ jobs:
--run-ignored ignored-only
env:
DATABASE_URL: postgres://buzz:${{ env.BUZZ_TEST_POSTGRES_PASSWORD }}@localhost:5432/buzz
- name: Workspace profile (kind:9033) gate tests
# Call-site integration for the 9033 authorization gate: open relay
# rosterless/steward transitions and the closed-relay admin/owner rule,
# against real Postgres. #[ignore]d in the default suite, selected
# explicitly here — see handlers::relay_admin::tests.
run: |
cargo nextest run \
--archive-file target/ci/backend-integration-tests.tar.zst \
-E 'package(buzz-relay) and test(/handlers::relay_admin::tests/)' \
--run-ignored ignored-only
env:
DATABASE_URL: postgres://buzz:${{ env.BUZZ_TEST_POSTGRES_PASSWORD }}@localhost:5432/buzz
- name: NIP-ER reminder e2e
# Feature e2e for NIP-ER (Event Reminders, kind:30300): write-path
# validation, author-only read filtering, and scheduler delivery against
+6
View File
@@ -4096,6 +4096,12 @@ impl Db {
relay_members::bootstrap_owner(&self.pool, community, owner_pubkey).await
}
/// Returns `true` if any member of `community` holds the `admin` or
/// `owner` role.
pub async fn has_admin_or_owner(&self, community: CommunityId) -> Result<bool> {
relay_members::has_admin_or_owner(&self.pool, community).await
}
/// Atomically transfers ownership of `community` to `new_owner_pubkey`,
/// demoting the previous owner(s) to `member`. Verifies
/// `expected_owner_pubkey` matches the current owner inside the same
+15
View File
@@ -37,6 +37,21 @@ pub async fn is_relay_member(pool: &PgPool, community: CommunityId, pubkey: &str
Ok(row.is_some())
}
/// Returns `true` if any member of `community` holds the `admin` or `owner`
/// role. Open relays don't *enforce* the roster, but startup
/// (`bootstrap_owner`) and operator provisioning still populate it — this is
/// how the workspace-profile gate detects whether a steward exists.
pub async fn has_admin_or_owner(pool: &PgPool, community: CommunityId) -> Result<bool> {
let row = sqlx::query(
"SELECT 1 FROM relay_members \
WHERE community_id = $1 AND role IN ('admin', 'owner') LIMIT 1",
)
.bind(community.as_uuid())
.fetch_optional(pool)
.await?;
Ok(row.is_some())
}
/// Returns the relay member record for `pubkey` in `community`, or `None`.
pub async fn get_relay_member(
pool: &PgPool,
+307 -2
View File
@@ -10,7 +10,7 @@
//! | 9030 | Add member | admin or owner |
//! | 9031 | Remove member | admin or owner |
//! | 9032 | Change role | owner only |
//! | 9033 | Set workspace profile (icon) | admin or owner |
//! | 9033 | Set workspace profile (icon) | admin or owner; on an open relay whose community has no admin/owner row at all, any authenticated sender (see [`may_set_workspace_profile`]) |
use std::sync::Arc;
@@ -94,6 +94,35 @@ fn validate_workspace_icon(icon: &str) -> Result<(), String> {
Ok(())
}
/// Whether `sender_role` may set the workspace profile (kind:9033).
///
/// Closed relays (`membership_enforced == true`) require an `admin`/`owner`
/// row in `relay_members` — the enforced roster is the authority. Open relays
/// don't *enforce* the roster, but the data can still exist: startup
/// bootstraps `RELAY_OWNER_PUBKEY` as `owner` regardless of the flag
/// (`main.rs`), as does operator provisioning. So the rule is steward-wins:
///
/// - a steward (any admin/owner row) exists → admin/owner only, exactly like
/// a closed relay. An open relay with a configured owner keeps its icon
/// owner-controlled instead of last-write-wins for every authenticated key.
/// - genuinely rosterless (e.g. a community created by
/// `ensure_configured_community`, which writes no owner row) → any
/// NIP-42-authenticated sender may set the icon, mirroring how open relays
/// gate every other write. Without this the icon is permanently unsettable:
/// the desktop deliberately shows the icon editor on open relays (see
/// `canEditIcon` in `EditCommunityDialog.tsx`, #2640) and defers to this
/// relay-side check, which used to always say no.
fn may_set_workspace_profile(
sender_role: &str,
membership_enforced: bool,
community_has_steward: bool,
) -> bool {
if !membership_enforced && !community_has_steward {
return true;
}
sender_role == "admin" || sender_role == "owner"
}
/// A relay-admin command failure, carrying the *category* of the failure so
/// the ingest seam can map it to the right NIP-01 prefix and HTTP status.
///
@@ -230,9 +259,33 @@ async fn execute_relay_admin_command(
// kind:9033 — Set workspace profile (icon). Handled before p-tag
// extraction: it targets the relay itself, not a member pubkey.
if kind == RELAY_ADMIN_SET_WORKSPACE_PROFILE {
if sender_role != "admin" && sender_role != "owner" {
// Steward detection only matters on open relays (closed relays gate on
// the sender's own role either way), so skip the extra query there.
let community_has_steward = if state.config.require_relay_membership {
true
} else {
state
.db
.has_admin_or_owner(tenant.community())
.await
.map_err(|e| format!("database error: {e}"))?
};
if !may_set_workspace_profile(
sender_role,
state.config.require_relay_membership,
community_has_steward,
) {
return Err("actor not authorized: must be admin or owner".to_string());
}
if sender_role != "admin" && sender_role != "owner" {
// Rosterless-open-relay admit: 9033 writes no audit row and
// publishes no announcement event (unlike 9030/9031), so this warn
// is the only durable attribution of who changed the icon.
warn!(
sender = %sender_hex,
"workspace profile change admitted without a roster role (open relay, no steward)"
);
}
// Empty or missing icon tag clears the workspace icon.
let icon = extract_tag_value(event, "icon").unwrap_or_default();
@@ -562,6 +615,46 @@ mod tests {
assert!(validate_workspace_icon("").is_ok());
}
/// Closed relay (membership enforced): only an admin/owner row in
/// `relay_members` may set the workspace profile — a plain member, or a
/// pubkey with no row at all (empty role), must be refused. The steward
/// flag is irrelevant when membership is enforced (call sites pass `true`,
/// but the rule must not depend on it).
#[test]
fn closed_relay_requires_admin_or_owner_for_workspace_profile() {
for steward in [true, false] {
assert!(may_set_workspace_profile("owner", true, steward));
assert!(may_set_workspace_profile("admin", true, steward));
assert!(!may_set_workspace_profile("member", true, steward));
assert!(!may_set_workspace_profile("", true, steward));
}
}
/// Open relay with a steward: startup bootstraps `RELAY_OWNER_PUBKEY` as
/// `owner` regardless of `require_relay_membership`, so an open relay's
/// community can hold admin/owner rows. When one exists, the icon stays
/// steward-only — the fix must not widen an owner-controlled icon to
/// every authenticated key.
#[test]
fn open_relay_with_steward_keeps_workspace_profile_steward_only() {
assert!(may_set_workspace_profile("owner", false, true));
assert!(may_set_workspace_profile("admin", false, true));
assert!(!may_set_workspace_profile("member", false, true));
assert!(!may_set_workspace_profile("", false, true));
}
/// Open relay, genuinely rosterless (no admin/owner row anywhere): any
/// authenticated sender may set the icon — including the roleless (empty
/// role) case, which is *every* sender there. This is the bug being
/// fixed: the desktop shows the icon editor on open relays (#2640) but
/// the relay refused every 9033.
#[test]
fn rosterless_open_relay_admits_any_authenticated_sender_for_workspace_profile() {
assert!(may_set_workspace_profile("", false, false));
assert!(may_set_workspace_profile("member", false, false));
assert!(may_set_workspace_profile("owner", false, false));
}
#[test]
fn workspace_icon_https_ok() {
assert!(validate_workspace_icon("https://example.com/icon.png").is_ok());
@@ -591,4 +684,216 @@ mod tests {
let long_data = format!("data:image/png;base64,{}", "A".repeat(98_304));
assert!(validate_workspace_icon(&long_data).is_err());
}
// ─── Call-site integration: the 9033 gate wired to real config + DB ────
//
// The unit tests above pin `may_set_workspace_profile`'s truth table, but
// not its wiring: mutation-testing showed that inverting
// `state.config.require_relay_membership` at the call site — an exact
// inversion of the security contract — survives the default suite. These
// tests drive `handle_relay_admin_event` with a real `AppState` against
// Postgres, on both relay modes, so the wiring itself is pinned. Selected
// explicitly in CI's Backend Integration job; requires local Postgres
// (and hard-fails rather than skipping when it is unreachable).
const TEST_DB_URL: &str = "postgres://buzz:buzz_dev@localhost:5432/buzz"; // sadscan:disable np.postgres.1
/// Build a real `AppState` + tenant for a fresh community on `host`, with
/// `require_relay_membership` set as given. Mirrors
/// `api::invites::tests::invite_test_state`.
async fn workspace_profile_test_state(
host: &str,
require_relay_membership: bool,
) -> (Arc<AppState>, TenantContext) {
let mut config = crate::config::Config::from_env().expect("config from env");
let database_url = std::env::var("BUZZ_TEST_DATABASE_URL")
.or_else(|_| std::env::var("DATABASE_URL"))
.unwrap_or_else(|_| TEST_DB_URL.to_string());
config.database_url = database_url.clone();
config.redis_url = "redis://127.0.0.1:1".to_string();
config.relay_url = format!("wss://{host}");
config.require_relay_membership = require_relay_membership;
let pool = sqlx::PgPool::connect(&database_url)
.await
.expect("requires reachable Postgres");
let db = buzz_db::Db::from_pool(pool.clone());
let record = db
.ensure_configured_community(host)
.await
.expect("ensure community");
let tenant = TenantContext::resolved(record.id, host);
let redis_pool = deadpool_redis::Config::from_url(&config.redis_url)
.create_pool(Some(deadpool_redis::Runtime::Tokio1))
.expect("redis pool config");
let pubsub = Arc::new(
buzz_pubsub::PubSubManager::new(&config.redis_url, redis_pool.clone())
.await
.expect("pubsub manager"),
);
let audit = buzz_audit::AuditService::new(pool.clone());
let auth = buzz_auth::AuthService::new(config.auth.clone());
let search = buzz_search::SearchService::new(pool.clone());
let workflow_engine = Arc::new(buzz_workflow::WorkflowEngine::new(
db.clone(),
buzz_workflow::WorkflowConfig::default(),
));
let media_storage = buzz_media::MediaStorage::new(&config.media).expect("media storage");
let (state, _audit_shutdown) = AppState::new(
config,
db,
redis_pool,
audit,
pubsub,
auth,
search,
workflow_engine,
Keys::generate(),
media_storage,
);
(Arc::new(state), tenant)
}
/// Sign a fresh kind:9033 with `icon` and run it through the real
/// admission + command path.
async fn submit_9033(
state: &Arc<AppState>,
tenant: &TenantContext,
keys: &Keys,
icon: &str,
) -> Result<(), RelayAdminError> {
let event = EventBuilder::new(Kind::Custom(9033), "")
.tags(vec![Tag::parse(["icon", icon]).expect("icon tag")])
.sign_with_keys(keys)
.expect("sign 9033");
handle_relay_admin_event(tenant, state, &event).await
}
async fn stored_icon(state: &Arc<AppState>, tenant: &TenantContext) -> Option<String> {
state
.db
.get_community_icon(tenant.community())
.await
.expect("read icon")
}
/// Open relay (`require_relay_membership = false`): a rosterless
/// community admits any authenticated sender, but the moment a steward
/// (admin/owner row) exists the gate reverts to steward-only.
///
/// Discriminating: fails if the call site inverts or drops
/// `require_relay_membership`, or stops consulting `has_admin_or_owner`.
#[tokio::test]
#[ignore = "requires Postgres"]
async fn open_relay_9033_admits_roleless_only_until_a_steward_exists() {
let host = format!("icon-gate-open-{}.example", uuid::Uuid::new_v4().simple());
let (state, tenant) = workspace_profile_test_state(&host, false).await;
let roleless = Keys::generate();
let owner = Keys::generate();
// Rosterless: the roleless sender may set the icon.
submit_9033(&state, &tenant, &roleless, "https://example.com/open.png")
.await
.expect("rosterless open relay must admit an authenticated sender");
assert_eq!(
stored_icon(&state, &tenant).await.as_deref(),
Some("https://example.com/open.png"),
"icon must actually be stored"
);
// Seed a steward — the same roleless sender must now be refused, and
// the previously stored icon must survive the refused attempt.
state
.db
.add_relay_member(
tenant.community(),
&owner.public_key().to_hex(),
"owner",
None,
)
.await
.expect("seed owner");
let refused = submit_9033(&state, &tenant, &roleless, "https://evil.example/pwn.png").await;
assert_eq!(
refused,
Err(RelayAdminError::Rejected(
"actor not authorized: must be admin or owner".to_string()
)),
"an open relay with a steward must refuse a roleless sender"
);
assert_eq!(
stored_icon(&state, &tenant).await.as_deref(),
Some("https://example.com/open.png"),
"refused attempt must not mutate the icon"
);
// The steward still can.
submit_9033(&state, &tenant, &owner, "https://example.com/owner.png")
.await
.expect("the steward must retain icon control");
assert_eq!(
stored_icon(&state, &tenant).await.as_deref(),
Some("https://example.com/owner.png")
);
}
/// Closed relay (`require_relay_membership = true`): admin/owner only —
/// a plain member and a roleless key are refused even though the
/// community also *looks* rosterless-then-stewarded to the open-relay
/// branch. Together with the open-relay test this kills the inverted-flag
/// mutant: no assignment of the flag satisfies both.
#[tokio::test]
#[ignore = "requires Postgres"]
async fn closed_relay_9033_still_requires_admin_or_owner() {
let host = format!("icon-gate-closed-{}.example", uuid::Uuid::new_v4().simple());
let (state, tenant) = workspace_profile_test_state(&host, true).await;
let roleless = Keys::generate();
let member = Keys::generate();
let admin = Keys::generate();
state
.db
.add_relay_member(
tenant.community(),
&member.public_key().to_hex(),
"member",
None,
)
.await
.expect("seed member");
state
.db
.add_relay_member(
tenant.community(),
&admin.public_key().to_hex(),
"admin",
None,
)
.await
.expect("seed admin");
for (keys, label) in [(&roleless, "roleless"), (&member, "member")] {
let refused = submit_9033(&state, &tenant, keys, "https://evil.example/pwn.png").await;
assert_eq!(
refused,
Err(RelayAdminError::Rejected(
"actor not authorized: must be admin or owner".to_string()
)),
"closed relay must refuse a {label} sender"
);
}
assert_eq!(
stored_icon(&state, &tenant).await,
None,
"refused attempts must not set an icon"
);
submit_9033(&state, &tenant, &admin, "https://example.com/closed.png")
.await
.expect("closed-relay admin must set the icon");
assert_eq!(
stored_icon(&state, &tenant).await.as_deref(),
Some("https://example.com/closed.png")
);
}
}
+3
View File
@@ -54,6 +54,9 @@ CREATE TABLE communities (
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
host VARCHAR(255) NOT NULL,
signing_key BYTEA,
-- Per-community workspace icon (NIP-11 `icon`), set via kind:9033.
-- Added by migration 0003; kept here so desired-state applies match.
icon TEXT,
created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(),
archived_at TIMESTAMPTZ,
CONSTRAINT chk_communities_id_not_nil CHECK (id <> '00000000-0000-0000-0000-000000000000'::uuid)