Skip to content

fix: preserve self-referential p tags in channel member builders - #4338

Open
iroiro147 wants to merge 2 commits into
block:mainfrom
iroiro147:fix/4326-self-tag-p-stripped
Open

fix: preserve self-referential p tags in channel member builders#4338
iroiro147 wants to merge 2 commits into
block:mainfrom
iroiro147:fix/4326-self-tag-p-stripped

Conversation

@iroiro147

Copy link
Copy Markdown
Contributor

Summary

Fixes #4326: buzz channels remove-member --pubkey <your-own-key> fails with 400 missing p tag.

Root cause

It's in nostr 0.44's EventBuilder::build, not buzz code. From nostr/src/event/builder.rs:430:

By default, this method removes any `p` tags that match the author's public key.
To allow self-tagging, call [`EventBuilder::allow_self_tagging`] first.

The SDK builders construct the p tag correctly (builders.rs:582-592), but sign_with_keys strips it when the target equals the signer. The existing tests only exercised non-self targets, which is why the bug shipped.

Fix

Chain .allow_self_tagging() on both build_add_member (kind 9000) and build_remove_member (kind 9001), matching the existing convention in the NIP-IA builders elsewhere in this file (lines 1816, 1837).

Tests added

Two regression tests that sign with the same key as the target pubkey:

  • remove_member_self_referential_preserves_p_tag
  • add_member_self_referential_preserves_p_tag

Both assert the p tag survives signing. Verified with cargo test -p buzz-sdk --lib (243/243 passing).

Closes #4326.

iroiro147 and others added 2 commits August 31, 2026 07:44
nostr 0.44's EventBuilder::build strips p tags whose value equals the
author's public key by default (see allow_self_tagging flag). The
build_add_member (kind 9000) and build_remove_member (kind 9001)
builders constructed the p tag correctly, but sign_with_keys would
then strip it when target_pubkey == signer pubkey, causing the relay
to reply 400 "missing p tag" for self-removal/self-add operations.

Fix by chaining .allow_self_tagging() on both builders, matching the
existing convention in NIP-IA builders (builders.rs:1816,1837).

Add regression tests that sign with the same key as the target
pubkey and assert the p tag survives — the existing happy-path tests
only exercised the non-self case, which is why the bug shipped.

Closes block#4326.

Signed-off-by: Sarthak Singh <sarthak.singh@juspay.in>
Signed-off-by: iroiro147 <sarthak.singh@mastersunion.org>
@sumit-m found a third builder with the same block#4326 defect, reachable
from a visible Desktop control rather than the CLI: 'Assign to me' and
'Unassign' on yourself report success and write the history comment
while the Assignees row never changes.

An assignment note's p tags ARE the assignees, so a self-assignment has
target == signer, and nostr 0.44 strips matching p tags unless
allow_self_tagging() is set -- the same root cause this PR already
fixed for kinds 9000 and 9001.

Also merges current main (this branch was 449 commits behind, and
build_git_issue_assignee_operation did not exist when it was opened).

self_assignment_preserves_p_tag fails without the change and passes
with it; 192 builder tests green.

Signed-off-by: iroiro147 <sarthak.singh@mastersunion.org>
@iroiro147
iroiro147 force-pushed the fix/4326-self-tag-p-stripped branch from e177028 to f3ec833 Compare August 31, 2026 02:15
@github-actions

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

The current range is c3132c3ee982d194cd0198ad07b57ec8bd726e4e...f3ec833269d74c23006409d29aedd24c2b23eadf.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review f3ec833269d74c23006409d29aedd24c2b23eadf to authorize a new review.
Any previous review applies only to its recorded range.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Self-referential p tag is stripped at signing: channels remove-member/add-member targeting your own key always fails with 400 missing p tag

1 participant