fix(cli): resolve agents from owner records - #3178
Conversation
Co-authored-by: npub1qye6rec0htgg3np8yt6plpyyg8cyffaq66emt3kmk05eylckkzhq0hnf2k <0133a1e70fbad088cc2722f41f848441f044a7a0d6b3b5c6dbb3e9927f16b0ae@buzz.block.builderlab.xyz> Signed-off-by: npub1qye6rec0htgg3np8yt6plpyyg8cyffaq66emt3kmk05eylckkzhq0hnf2k <0133a1e70fbad088cc2722f41f848441f044a7a0d6b3b5c6dbb3e9927f16b0ae@buzz.block.builderlab.xyz>
Co-authored-by: npub1qye6rec0htgg3np8yt6plpyyg8cyffaq66emt3kmk05eylckkzhq0hnf2k <0133a1e70fbad088cc2722f41f848441f044a7a0d6b3b5c6dbb3e9927f16b0ae@buzz.block.builderlab.xyz> Signed-off-by: npub1qye6rec0htgg3np8yt6plpyyg8cyffaq66emt3kmk05eylckkzhq0hnf2k <0133a1e70fbad088cc2722f41f848441f044a7a0d6b3b5c6dbb3e9927f16b0ae@buzz.block.builderlab.xyz>
wpfleger96
left a comment
There was a problem hiding this comment.
I found one correctness issue in the owner-scoped lookup. The duplicated CLI/runtime validation is non-blocking.
| #[arg(long = "name")] | ||
| name: Option<String>, | ||
| /// Scope an exact-name agent lookup to its owner (`me`, hex, or npub) | ||
| #[arg(long = "owner", requires = "name")] |
There was a problem hiding this comment.
[MINOR][Minimalism] requires = "name" makes sense here because --owner is only implemented as a scope for name-based agent resolution; it is not meaningful for --pubkey or the implicit self lookup. However, this also means the runtime owner.is_some() check in commands/users.rs:31-32 is unreachable through normal Clap dispatch. I think we should retain this parse-time constraint and remove the duplicate runtime guard unless direct calls to cmd_get_users() are an intended supported path. Non-blocking.
There was a problem hiding this comment.
I’m retaining the runtime guard so cmd_get_users enforces its own argument invariant independently of Clap. The function is public within the CLI command module and accepts owner directly, so keeping the inexpensive check prevents future direct callers or dispatch refactors from silently ignoring --owner when no name is supplied. I agree Clap is the normal path, but prefer defense-in-depth at this command boundary. Leaving this non-blocking thread unresolved because I’m declining the suggested removal rather than implementing it. — AI-generated by Fizz
Co-authored-by: npub1qye6rec0htgg3np8yt6plpyyg8cyffaq66emt3kmk05eylckkzhq0hnf2k <0133a1e70fbad088cc2722f41f848441f044a7a0d6b3b5c6dbb3e9927f16b0ae@buzz.block.builderlab.xyz> Signed-off-by: npub1qye6rec0htgg3np8yt6plpyyg8cyffaq66emt3kmk05eylckkzhq0hnf2k <0133a1e70fbad088cc2722f41f848441f044a7a0d6b3b5c6dbb3e9927f16b0ae@buzz.block.builderlab.xyz>
wpfleger96
left a comment
There was a problem hiding this comment.
Reviewed current remote head 906072bd980f77ceb311fdd7ff635d7b7e7962b9. The prior trust-boundary and disappearing-candidate findings are addressed, but one protocol-level trust defect remains.
Review
[CRITICAL][Correctness]: Condition-restricted credentials are promoted to profile ownership proof
owner_verification() calls verify_auth_tag(), which validates the signature and condition syntax but never applies the signed conditions to the profile event. NIP-OA requires verifiers to evaluate every clause and defines kind=<n> and created_at bounds against the event being verified (spec lines 64–69). The new test makes the mismatch concrete: it signs kind=9 and expects that credential to verify on a kind:0 profile (test lines 559–584).
That promotes an attestation the owner restricted to kind:9 messages into trusted owner_pubkey / owned_by_me metadata for a kind:0 profile. Because this command exists to prevent wrong-owner agent selection, widening a credential beyond its signed scope is blocking.
After the signature check, evaluate the exact conditions string against the profile event: every kind= clause must equal 0, and every created_at< / created_at> clause must accept the profile's created_at. Return a non-verified status when any clause fails, and add negative tests for kind=9 and violated time bounds. If this lookup is intentionally defining a separate standing-identity proof context like NIP-IA rather than event-level NIP-OA verification, that semantic expansion needs an explicit protocol contract and test name; the current implementation and PR description claim ordinary NIP-OA verification.
[MINOR][Correctness]: The reported Unit Tests check does not run these tests
The added buzz-cli tests pass locally, but just test-unit only selects buzz-core, buzz-auth, buzz-db, buzz-conformance, and buzz-push-gateway. Therefore the green GitHub Unit Tests job does not exercise this PR's new tests. Non-blocking for this fix because I independently ran the full package suite, but buzz-cli should be added to that CI gate or get a changed-package job.
What's Solid
- The original trust-boundary flaw is substantially fixed: ownership fields now require exactly one cryptographically valid auth tag whose owner matches the requested owner.
- Renamed profiles, missing profiles, malformed agent keys, missing/invalid/multiple auth tags, and owner mismatches remain visible without claiming ownership.
- The owner-scoped path no longer re-applies the lossy profile-name filter, so the prior disappearing-candidate defect is fixed.
- Reusing
extract_d_tag, filtering invalid author keys before the profile query, preserving unscoped compact output, and returning all duplicate matches are all correct. - All GitHub checks are green. I independently ran
cargo test -p buzz-cliat exact head906072bd980f77ceb311fdd7ff635d7b7e7962b9: 254 passed, 0 failed; the SHA was unchanged before and after the run.
Alternatives
The smallest correction is local condition evaluation after verify_auth_tag(). A more reusable design is an event-aware SDK verifier that accepts (kind, created_at) while retaining the existing signature-only primitive for explicitly documented NIP-AA/NIP-IA contexts.
Quality Gate
- Minimalism: 9/10 — the CLI-only two-query flow and explicit verification statuses are focused and load-bearing.
- Elegance: 9/10 — the owner resolution, candidate preservation, and output shaping are direct and readable.
- Correctness: 7/10 — signature verification is now present, but signed authorization conditions are ignored before trusted ownership metadata is emitted.
Verdict: Request changes. Fix the condition applicability gap, then this is ready for another pass.
Co-authored-by: npub1qye6rec0htgg3np8yt6plpyyg8cyffaq66emt3kmk05eylckkzhq0hnf2k <0133a1e70fbad088cc2722f41f848441f044a7a0d6b3b5c6dbb3e9927f16b0ae@buzz.block.builderlab.xyz> Signed-off-by: npub1qye6rec0htgg3np8yt6plpyyg8cyffaq66emt3kmk05eylckkzhq0hnf2k <0133a1e70fbad088cc2722f41f848441f044a7a0d6b3b5c6dbb3e9927f16b0ae@buzz.block.builderlab.xyz>
|
Addressed the re-review findings in
Validation at |
Co-authored-by: npub1qye6rec0htgg3np8yt6plpyyg8cyffaq66emt3kmk05eylckkzhq0hnf2k <0133a1e70fbad088cc2722f41f848441f044a7a0d6b3b5c6dbb3e9927f16b0ae@buzz.block.builderlab.xyz> Signed-off-by: npub1qye6rec0htgg3np8yt6plpyyg8cyffaq66emt3kmk05eylckkzhq0hnf2k <0133a1e70fbad088cc2722f41f848441f044a7a0d6b3b5c6dbb3e9927f16b0ae@buzz.block.builderlab.xyz>
wpfleger96
left a comment
There was a problem hiding this comment.
Re-reviewed exact remote head 4275a48d1b65cdbb590a90541bad843cd667d995. The prior blocking condition-applicability defect and the CI coverage comment are addressed.
Review
No blocking findings.
What's Solid
owner_verification()now verifies the signature first, then evaluates every signedkind,created_at<, andcreated_at>condition against the actual kind:0 profile event before emitting ownership metadata.- Negative tests cover
kind=9and both exclusive timestamp boundaries; the combined valid case verifies. - Drifted, missing, malformed, and condition-mismatched candidates remain visible without
owner_pubkeyorowned_by_me: true. buzz-cliis included in bothjust test-unitexecution paths. The green CI Unit Tests log ran 258 CLI tests, including this path.- Independent exact-SHA validation passed:
just test-unit(includingbuzz-cli: 255 passed, 0 failed) andcargo check -p buzz-cli --all-targets. HEAD remained unchanged and the worktree stayed clean.
Quality Gate
- Minimalism: 9/10 — the condition check is a focused local addition; the retained command-boundary guard is a reasonable defense-in-depth choice.
- Elegance: 9/10 — signature, owner, condition, and candidate-state decisions remain explicit and readable.
- Correctness: 9/10 — the trust boundary now applies every supported NIP-OA condition with strict timestamp semantics, and the failure paths do not assert ownership.
Verdict: Approve.
…chive * origin/main: (22 commits) feat(catalog): resolve publisher display name in catalog detail pane (#3640) feat(mesh): upgrade embedded mesh to v0.74 and harden shared compute (split 1/2 of #3467) (#3741) docs(nips): specify kind:30621 multi-repo projects (NIP-MP) (#3163) Refine agent sharing dialog (#3699) desktop: enable getUserMedia in the Linux WebKitGTK webview (#3607) fix: align responsive agent views (#3688) Add macOS agent menu-bar menu (#3565) Fix pending message feedback (#3543) fix(desktop): remove remaining Projects panel fills (#3742) feat(mobile): desktop-parity emoji and thread experience (#3485) desktop: restore direct community member adds (#3634) fix(desktop): explain open agent access (#2561) fix(cli): resolve agents from owner records (#3178) fix(desktop): remove Projects overview card fills (#3416) feat(replica): portable heartbeat-token fence with snapshot-local reader routing (#3268) fix(git): channel binding tooling + author remediation for unbound repos (#3626) feat: configure S3 URL addressing style (#3400) feat: add first-class OpenRouter provider support (#1975) feat(agent,acp): wire provider total_tokens through NIP-AM publish chain (#3593) chore(release): release Buzz Desktop version 0.5.2 (#3624) ... Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…g-pipeline * origin/main: (25 commits) Refine agent sharing dialog (#3699) desktop: enable getUserMedia in the Linux WebKitGTK webview (#3607) fix: align responsive agent views (#3688) Add macOS agent menu-bar menu (#3565) Fix pending message feedback (#3543) fix(desktop): remove remaining Projects panel fills (#3742) feat(mobile): desktop-parity emoji and thread experience (#3485) desktop: restore direct community member adds (#3634) fix(desktop): explain open agent access (#2561) fix(cli): resolve agents from owner records (#3178) fix(desktop): remove Projects overview card fills (#3416) feat(replica): portable heartbeat-token fence with snapshot-local reader routing (#3268) fix(git): channel binding tooling + author remediation for unbound repos (#3626) feat: configure S3 URL addressing style (#3400) feat: add first-class OpenRouter provider support (#1975) feat(agent,acp): wire provider total_tokens through NIP-AM publish chain (#3593) chore(release): release Buzz Desktop version 0.5.2 (#3624) docs: add Linux rendering troubleshooting guide (#3573) fix(desktop): discover bun-installed agent CLIs in ~/.bun/bin (#3343) feat(tracing): correlate trace IDs in relay logs (#3608) ... Signed-off-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…-style * origin/main: fix(acp): preserve truncated thread context (block#3340) feat(catalog): resolve publisher display name in catalog detail pane (block#3640) feat(mesh): upgrade embedded mesh to v0.74 and harden shared compute (split 1/2 of block#3467) (block#3741) docs(nips): specify kind:30621 multi-repo projects (NIP-MP) (block#3163) Refine agent sharing dialog (block#3699) desktop: enable getUserMedia in the Linux WebKitGTK webview (block#3607) fix: align responsive agent views (block#3688) Add macOS agent menu-bar menu (block#3565) Fix pending message feedback (block#3543) fix(desktop): remove remaining Projects panel fills (block#3742) feat(mobile): desktop-parity emoji and thread experience (block#3485) desktop: restore direct community member adds (block#3634) fix(desktop): explain open agent access (block#2561) fix(cli): resolve agents from owner records (block#3178) fix(desktop): remove Projects overview card fills (block#3416) feat(replica): portable heartbeat-token fence with snapshot-local reader routing (block#3268) Signed-off-by: Joah Gerstenberg <joah@squareup.com>
Context
buzz users get --name Honeysearches relay-wide profiles and can return an identically named agent owned by someone else. This caused agents from the wrong owner to be added to a channel.Summary
This bug fix scopes exact-name agent lookup to owner-authored managed-agent records, then cryptographically verifies each returned profile's NIP-OA
authtag before asserting ownership. The relay and database contracts remain unchanged.Related issue
None found.
Changes
buzz users get --name Honey --owner me|<hex>|<npub>.meto the NIP-OA owner when the CLI runs as an agent, otherwise to the CLI identity.30177managed-agent record names exactly and case-insensitively under the requested owner.authtag whose verified owner equals the requested owner and whosekindandcreated_atconditions apply to the profile event before returningowner_pubkeyorowned_by_me: true.owned_by_me: falseand an explicitverificationvalue.Testing
The reviewer-reproducible red and green commands below exercise the ownership bug against the target branch and this branch.
Screenshots
Not applicable. This is a CLI-only change.
Reviewer-reproducible examples
The lookups below were run against the live relay from
mainand this branch.Red: unscoped lookup returns the 100-profile relay-wide cap and excludes John's agents
On
main:Observed output:
{ "count": 100, "first_three": [ "20d27fc6c0ab4f50b66d1a32a64c5ca1fb985254143ce911f61ab7733333c3d7", "00644478cdd9032c563ddc712b3687d8345d948945aab3c18bab95afbf6f519a", "93c16697d0e58007bc11fb953208bc6b1cff387b2dee094abc10bf82dfee5424" ], "johns_agents": [] }mainalso rejects the owner-scoped command:Green: owner-scoped lookup distinguishes verified and unresolved records
On this branch:
Observed output:
[ { "pubkey": "0ca77314d7ac8b3fcf6c647cc8cb9c3afd840db3b2a8ff2079f09a168de1827e", "display_name": null, "owner_pubkey": null, "owned_by_me": false, "verification": "missing_profile" }, { "pubkey": "31b29bcbe69d6716fbb7ba33602b89200bfc9ddfdabcfd1ea6fbfa70b816dfc7", "display_name": "Honey", "owner_pubkey": "67252b09c31a995daa63aada26569fbc6a3d12f573113f001ce7432f870da820", "owned_by_me": true, "verification": "verified" }, { "pubkey": "4597ac725bba33fc7dd0454c1e2316a5ed770426acf667837d46f6553b3fcf54", "display_name": "Honey", "owner_pubkey": "67252b09c31a995daa63aada26569fbc6a3d12f573113f001ce7432f870da820", "owned_by_me": true, "verification": "verified" } ]Only the two profiles with valid NIP-OA proofs assert ownership. The owner-authored record whose profile is absent remains visible but cannot be selected as verified ownership.