Repository navigation
Conversation
🔐 Codex Security Review
|
0cd4693 to
65168e4
Compare
joahg
left a comment
There was a problem hiding this comment.
🤖 Leaving comments rather than approving. This changes who can wake every agent, so it needs a sign-off from a Buzz maintainer. Two things to settle first:
- The default changes.
RespondTo::default()isOwnerOnly, so once this merges, every agent on default settings stops waking for workflows. That reverses the default #6953 chose on purpose. It needs a maintainer's call, plus a release note or migration for operators who use workflows today. - The opt-in grants more than attribution. Listing the relay key in
BUZZ_ACP_RESPOND_TO_ALLOWLISTalso makes the relay an allowed author. That's becauseauthor_allowedchecksallowlist.contains(author).effective_prompt_authoralso falls back to the raw signer wheneververified_workflow_ownerreturnsNone. So with the opt-in on, any relay-signed event without verified attribution wakes the agent. That includes the cases where NIP-11 is unavailable and where the key was rotated away (if the old key is still listed).- The tests for those two cases used to assert
!allowed. They now assert only oneffective_author, which hides the change. - Suggestion: keep "trust relay attribution" separate from "allow this author". For example, pass
author_allowedan allowlist without the relay key, or use a dedicated setting like the one #4068 proposed. Then restore the!allowedassertions.
- The tests for those two cases used to assert
The gate change itself, filtering relay_self through the allowlist, is small and correct for the owner-only case.
The relay signs workflow messages and names their owner in a tag, and the author gate took that tag as the owner speaking. Anyone holding the relay key could therefore prompt an agent as its owner. Attribution now needs --trust-relay-workflow-attribution (BUZZ_ACP_TRUST_RELAY_WORKFLOW_ATTRIBUTION), off by default. It is separate from the author allowlist, so opting in no longer makes the relay an allowed author: relay-signed events without verified attribution are still judged as the relay. Tests cover the default-off gate and a substituted mention that carries a p tag but no buzz:workflow-mention tag. Opus 5.5 helped authoring this commit. Signed-off-by: Leo Wandersleb <leo@leowandersleb.de>
65168e4 to
fe8bdcd
Compare
|
Thanks @joahg, both points were right. I pushed a revision:
That leaves your first point, which I can't settle in code: what's the intended trust model for the relay key? As I read #6953, a message signed by the relay's NIP-11 If that's the intended model ("the relay operator is trusted to speak for owners"), I'd flip the default back to on and keep the flag as a way to opt out, with a line in the docs. If it isn't, default-off needs a release note, and Desktop would need to pass the flag for agents that use workflows, since today it doesn't. Longer term, the owner-signed approach in #2737 or a pinned relay key (#4068) would remove the trade-off. Happy to go either way once a maintainer picks the default. |
joahg
left a comment
There was a problem hiding this comment.
🤖 Thanks, both code points are fixed on this head.
--trust-relay-workflow-attributionnow gates only the attribution.author_allowedgets the operator's allowlist unchanged, and the new test shows that listing the relay key doesn't turn attribution on.- The NIP-11-unavailable and rotated-key tests are back to main's
!allowedassertions. - The substituted-mention test covers the webhook
@agentcase.
On your question: the default is a call about the relay's trust model, and it belongs to a Buzz maintainer, so I'm leaving it to them rather than approving. Whichever way it goes, please ship it together with the matching Desktop change. Desktop doesn't pass this flag today, so default-off on its own would quietly stop workflows from waking every Desktop agent. That needs a release note. If maintainers choose default-on, keeping the flag as an explicit opt-out seems right.
Summary
The relay signs workflow messages and names the workflow owner in a tag. Since #6953 the respond-to gate takes that tag as the owner speaking, so whoever holds the relay key can prompt an agent with any text, as its owner.
This makes that trust an explicit operator opt-in with its own flag,
--trust-relay-workflow-attribution(BUZZ_ACP_TRUST_RELAY_WORKFLOW_ATTRIBUTION), default off. Without it, a relay-signed message is judged by its raw signer, as for any other author. The flag does not touch the author allowlist, so opting in does not make the relay an allowed author.Behaviour change: operators relying on workflows to wake agents must set the flag. Desktop does not pass it today.
Related
#4068 proposed the same opt-in (
--honor-workflow-attribution) but conflicts with main and has been stale since 2026-08-19. #6953 made the trust the default. #3858 is the "workflows don't wake agents" side. #2737 is the owner-signed alternative.Testing
cargo test -p buzz-acp(1091 passed),cargo clippy -p buzz-acp --all-targets -- -D warnings,cargo fmt --check. New tests: default-off (owner-only, and relay listed in the allowlist), and a relay-signed message with aptag but nobuzz:workflow-mentionstays dropped even with the flag on.🤖 Generated with Claude Code