fix(relay): materialize NIP-OA owner for direct members on closed relays (#4223)

[iroiro147]

What

On a closed relay (require_relay_membership = true), an agent that is already a direct relay member has its NIP-OA auth tag silently dropped. enforce_relay_membership returns Ok(None) for direct members (the Member branch), and the or_else in bridge.rs:812-818 skipped extract_nip_oa_owner when require_relay_membership was true — so agent_owner_pubkey was never materialized in users.

This made channel_add_policy: owner_only unenforceable in the intended direction (no owner on record to match against), and owner-scoped lookups (buzz users get --owner <hex>) returned nothing. The issue author's reproduction is unambiguous: removing the direct relay membership and republishing the identical event materializes the owner correctly.

Fix

The NIP-OA auth tag is cryptographically self-proving — if verify_auth_tag succeeds, the owner relationship is authentic regardless of which membership branch granted access. Hoist extract_nip_oa_owner out of the require_relay_membership conditional so it runs unconditionally when enforce_relay_membership returns Ok(None):

// Before:
Ok(owner) => owner.or_else(|| {
    if !state.config.require_relay_membership {
        super::relay_members::extract_nip_oa_owner(&pubkey_bytes, auth_tag)
    } else {
        None  // closed relay + direct member => owner never extracted
    }
}),

// After:
Ok(owner) => owner.or_else(|| {
    super::relay_members::extract_nip_oa_owner(&pubkey_bytes, auth_tag)
}),

The if !require_relay_membership guard was originally there because extract_nip_oa_owner was the open-relay fallback path. But the function itself has no dependency on the relay mode — it just verifies the auth tag signature. The guard was excluding exactly the case where it matters most.

Behavioral matrix (after fix)

Relay mode	Membership path	Auth tag	Owner materialized?
Open	(no check)	absent	No (unchanged)
Open	(no check)	valid	Yes (unchanged)
Closed	Direct member	absent	No (unchanged)
Closed	Direct member	valid	Yes (fixed)
Closed	Via owner	valid	Yes (unchanged — or_else not called)
Closed	Denied	—	Err (unchanged)

Tests

• cargo check -p buzz-relay — clean
• cargo test -p buzz-relay --lib nip_oa — 1/1 passed (existing extract_nip_oa_owner unit test)
• cargo test -p buzz-relay --lib bridge — 59/59 passed, 0 failed

The existing extract_nip_oa_owner unit tests (valid tag → Some(owner), no tag → None, invalid tag → None) already cover the function being called in the new position. The bridge handler tests that require Postgres are #[ignore] and not run here, but the 59 non-ignored bridge tests all pass.

Linked issue

Refs #4223

[rmichelena]

Heads-up rather than a competing claim: I've opened #5581, which includes this same hoist plus the handlers/auth.rs half.

Your diagnosis is right and it predates mine. What sent me to the WS path is that bridge.rs alone doesn't reach the two consequences that have no operator workaround: connection.rs:632 derives is_agent — and therefore the agent vs human rate class — from ctx.agent_owner_pubkey, and the observer-frame (kind 24200) fast path reads the same session field. Both are populated in handlers/auth.rs, which has an identical copy of the conditional you removed, so on a closed relay a direct member still authenticates with no owner in its session context. A DB repair doesn't help there either, since neither reads back from the DB.

#5581 puts the resolution in one shared pure helper so the HTTP and WS paths can't drift apart again, and adds unit tests (the module currently only tests extract_nip_oa_owner).

I'm not asking you to close this — if maintainers would rather land your smaller diff first, I'll happily reduce mine to the auth.rs half plus tests on top of it. Just flagging so nobody reviews the same conditional twice without knowing.

[iroiro147]

@rmichelena thank you for the careful heads-up — and for checking priority, which you didn't have to do.

I verified rather than took it on faith: #5581 touches bridge.rs, mod.rs and handlers/auth.rs, and your point about the WS path is the part mine misses entirely — connection.rs deriving is_agent from ctx.agent_owner_pubkey, populated only in auth.rs, means my bridge.rs-only fix leaves a closed relay authenticating direct members with no owner in session context, with no DB-repair workaround. The shared pure helper is also the right structural answer; two copies of that conditional is how this drifted in the first place.

So I'm closing this one in favour of #5581 — yours is strictly more complete and I'd rather reviewers read one PR than two. No need to reduce yours to the auth.rs half; land the whole thing.

For maintainers: #5581 supersedes this PR and is the one worth review time.