fix(sdk): allow self-tagging in build_add_member so self-targeted kind 9000 keeps its p tag - #4484
Open
Joi wants to merge 1 commit into
Open
fix(sdk): allow self-tagging in build_add_member so self-targeted kind 9000 keeps its p tag#4484Joi wants to merge 1 commit into
Joi wants to merge 1 commit into
Conversation
…d 9000 keeps its p tag nostr 0.44 strips p tags matching the signer's own pubkey at signing time, so a self-targeted add-member (actor == target, e.g. an owner adjusting their own role or an admin agent adding itself with an explicit role) went on the wire without its p tag and was rejected by the relay with "missing p tag". The NIP-IA builders already apply .allow_self_tagging() for their self paths with a comment documenting the scrub, and the e2e self-targeted PUT_USER probe hand-builds its kind 9000 with .allow_self_tagging() instead of using build_add_member. This applies the same fix to build_add_member itself. Non-self adds are unaffected. Adds add_member_self_target_keeps_p_tag pinning the fixed wire form, mirroring unarchive_request_layout_self_path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Joi Ito <joi@ito.com>
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
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.
Summary
buzz_sdk::builders::build_add_memberbuilds kind:9000 without.allow_self_tagging(), so a self-targeted add-member (actor == target — e.g. an owner adjusting their own role, or an admin agent adding itself with an explicit role) silently loses its["p", target]tag at signing time: nostr 0.44 stripsptags that match the signer's own pubkey by default (verified against the resolvednostr-0.44.7,builder.rs:435-449). The event then reaches the relay without aptag and is rejected byhandlers/side_effects.rswithinvalid: missing p tag.The codebase already recognizes this exact interaction in two places:
.allow_self_tagging()for their self paths, with a comment documenting the nostr 0.44 scrub (crates/buzz-sdk/src/builders.rs—build_archive_identity_request/build_unarchive_identity_request)..allow_self_tagging()instead of usingbuild_add_member, with a comment noting the builder would otherwise drop the tag (crates/buzz-test-client/tests/e2e_relay.rs, "The probe: owner_b sends a bare self-targeted PUT_USER").This PR applies the same one-line fix to
build_add_memberitself, so SDK/CLI callers get the correct wire form. Non-self adds are unaffected (allow_self_taggingonly changes behavior when aptag matches the signer).Observed in production: a relay-admin agent running the CLI's
add-memberagainst its own pubkey (to set an explicitbotrole) was rejected withinvalid: missing p tag; adding a different pubkey with the same CLI worked fine.Note:
build_remove_member(9001) andbuild_dm_add_member(41011) share the same shape and would also scrub a selfptag — and the relay does support guarded self-removal (handle_remove_user: self-remove allowed unless the actor is the last owner). I left them unchanged only to keep this PR scoped to the observed add-member failure — happy to extend it to the whole class if you prefer.Related issue
None found (searched open issues/PRs for
allow_self_tagging/ "missing p tag" / add-member self).Testing
add_member_self_target_keeps_p_tagpins the fixed wire form (signs a self-targeted build_add_member and asserts thepandroletags survive), mirroring the existingunarchive_request_layout_self_pathpin. Verified red without the fix: it fails on exactlyassertion failed: has_tag(&ev, "p", &self_pk).add_member_with_role/add_member_without_roletests still pass (non-self path unchanged).cargo test -p buzz-sdkgreen (253 tests).just cirun locally and green:cargo fmt --all -- --check,cargo clippy --workspace --all-targets -- -D warnings, and the fulltest-unitpackage set (buzz-core, buzz-auth, buzz-voice, buzz-cli, buzz-db --lib, buzz-conformance, buzz-push-gateway, buzz-backend-kubernetes) — 0 failures. The desktop/web/mobile legs weren't run locally (toolchains not installed here); the change is confined tocrates/buzz-sdk, so deferring those to PR CI.channels add-member --role adminis accepted andchannels membersshows the signer's pubkey at the requested role.invalid: missing p tag(wire-form, before the relay can even evaluate the request) toinvalid: only owners/admins may change an active member's role(authorization) — i.e. theptag now reaches the relay and the request is judged on its merits.