fix(workflows): authorize owner-triggered agent wakes via signed doorbells - #2737
fix(workflows): authorize owner-triggered agent wakes via signed doorbells#2737loganj wants to merge 3 commits into
Conversation
d1a1a6f to
3688eda
Compare
b16fab8 to
0a1da5c
Compare
0a1da5c to
ab95d09
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab95d0924b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
81dd122 to
22ec8c8
Compare
🤖
## Summary
Workflow messages still wake the managed agent named by their
owner-signed definition, but the relay is now only a doorbell. It
carries pointers to the exact workflow revision and cause; ACP
independently verifies those signed artifacts, reconstructs trigger
input, and renders the prompt locally. Relay-authored prose is never
shown to the agent or treated as owner authority.
Before this change, a compromised relay signing key could attach
`workflow-owner` to chosen message content. After this change, ACP
grants the workflow owner principal only when the doorbell, owner-signed
kind `30620` definition, referenced step, channel, and cause all verify.
Invalid workflow-shaped deliveries are dropped rather than falling
through as ordinary relay messages.
### Doorbell behavior
- Workflow rows retain the exact kind `30620` event ID that materialized
them. Existing rows remain nullable and fail closed until the definition
is saved again.
- Relay events contain the definition event/step and one typed cause:
signed event, signed manual command, deterministic schedule slot, or
webhook cargo.
- ACP refetches and verifies the owner-signed definition. It also
refetches signed event/command causes, reconstructs their trigger
context, rechecks that the signed trigger matches the definition,
validates exact cron/interval slots, and renders `send_message` text
itself.
- Manual commands from an agent's cryptographically verified human owner
remain authorized, but command JSON deliberately no longer populates
arbitrary webhook fields. Unrelated channel members remain denied. If
parameterized manual runs are added later, those parameters must return
as explicitly untrusted cargo under the same boundary as webhooks rather
than inheriting owner authority.
- Prior workflow-step outputs cannot be independently reconstructed, so
`send_message` templates and conditions that depend on them fail closed
rather than treating unresolved placeholders as owner-authorized text.
- Mention wake targets are resolved only from the owner-signed template.
`@Name` text arriving through a signed source event or webhook field
cannot steer a different agent.
### Webhooks and replay controls
Webhooks have no signed source event. Their JSON therefore travels as
explicitly untrusted external cargo, can fill only `{{trigger.*}}` slots
declared by the owner-signed template, and is labelled as untrusted in
the prompt. ACP applies a per-definition token bucket (five immediate
turns, refilling at five per minute) and caps serialized cargo at 61,440
bytes.
Signed event, command, and schedule causes are deduplicated by
definition revision × semantic cause. Dedupe is intentionally
process-lifetime/in-memory: an ACP restart can admit a replay again.
Schedule causes must be exact authorized cron occurrences or interval
boundaries; wall-clock freshness/skew policy is deferred.
### Rollout
This is a coordinated protocol change. New ACP drops old relay workflow
messages because they lack the `doorbell-v1` pointer shape. Old ACP does
not recognize new doorbells as delegated owner messages, so owner-only
policy rejects them. Existing database rows without a definition event
ID do not fire a doorbell until the workflow is re-saved.
### Related issue
Security correction stacked directly on #2737. The owner-trigger
authorization in #5854 remains separate and will be restacked after this
correction merges into #2737.
### Testing
- `cargo test -p buzz-workflow` — 156 passed, 2 Postgres-gated ignored
- `cargo test -p buzz-acp` — 783 unit + 9 lifecycle passed
- `cargo test -p buzz-db migration --lib` — 10 passed, 6 Postgres-gated
ignored
- `cargo test -p buzz-relay --no-run`
- `cargo test -p buzz-test-client --test e2e_workflow_agent_owner
--no-run`
- `cargo clippy -p buzz-acp -p buzz-workflow -p buzz-relay -p
buzz-test-client --all-targets`
- `cargo fmt --check` and `git diff --check`
---------
Signed-off-by: Larry <8cf5a83f590ec0955b11647d1c88f796a98e088c30a492c58e0e46c3026ae7a4@buzz.block.builderlab.xyz>
Co-authored-by: Larry <8cf5a83f590ec0955b11647d1c88f796a98e088c30a492c58e0e46c3026ae7a4@buzz.block.builderlab.xyz>
|
Production follow-up (2026-08-19): the hosted Workflows overview is still empty for a user whose memberships and eight live definitions are verified, so this remains a release blocker. The PR is currently
@loganj please rebase this branch onto current |
3e3d80a to
ad6141e
Compare
56a9cfb to
f78ab0e
Compare
loganj
changed the title
Signed-off-by: loganj <loganj@squareup.com> Co-authored-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: loganj <loganj@squareup.com> Co-authored-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
Signed-off-by: loganj <loganj@squareup.com> Co-authored-by: Brother Darryl <146fb160a3266e6165bfa385f6048c975eda9e21cf65da097a0b5ea7952532a5@buzz.block.builderlab.xyz>
f78ab0e to
aa8f8f6
Compare
wesbillman
left a comment
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Requesting changes for two user-visible workflow regressions at aa8f8f6f7df5767cfa1669e33bed0fa7b8d43cf7:
-
P1: persisted workflow messages expose doorbell transport data instead of the authored message.
RelayActionSink::send_messagereceives the renderedtext, but lines 375-381 persist a kind-9 event whose content is empty for event/schedule/manual causes and the completewebhook_fieldsobject for webhook causes. Kind 9 is still a normal timeline content event, and Desktop rendersevent.contentdirectly (desktop/src/features/messages/lib/formatTimelineMessages.ts:49-62,506). Therefore an authoredsend_message.text: "Build complete"appears blank for normal triggers, while a webhook carrying{"token":"secret"}displays that payload to every channel reader. ACP's rewrite atcrates/buzz-acp/src/lib.rs:3279-3283only changes the agent process's local event after retrieval; it cannot repair the already-persisted channel event. Please preserve the authored/rendered timeline message (or introduce an explicitly hidden, non-timeline doorbell transport) and add coverage for both ordinary and webhook-triggered persisted content. -
P1: cause-level dedup suppresses distinct
send_messagesteps from the same run. The executor dispatches every step (crates/buzz-workflow/src/executor.rs:1175-1228) and each doorbell identifies its step (crates/buzz-relay/src/workflow_sink.rs:331-333), but ACP's event, command, and schedule keys use only definition revision plus cause (crates/buzz-acp/src/lib.rs:469-472,486-493,503-510). The first step inserts that key, then lines 3258-3267 drop the next step before filtering or queue insertion. A workflow with two intentional prompts to the same agent therefore delivers only the first. Include the signed step ID in the key, then test that replaying one step collapses while two distinct steps from one cause are both admitted.
I traced both paths against exact base bbd20fae75ecc3bd7a83cc12a65379fac22a2b79. Current CI is green, but its tests do not exercise these multi-step or persisted-timeline contracts.
3 participants
🤖
Summary
People could not use the workflows their own agent set up for them. Two separate failures:
This change treats a workflow message as a doorbell: a notification that something happened, carrying no authority of its own. The agent fetches the owner-signed workflow definition itself, verifies the signature, checks that the claimed cause actually happened, and renders the message text locally from the signed template. Only then does it treat the wake as coming from its owner. Text authored by the relay never inherits the owner's authority. Separately, a person may now manually run a workflow owned by an agent they own.
Details
Migration and compatibility
Migration
0032adds a nullabledefinition_event_idcolumn toworkflows, filled in whenever a workflow is saved. There is intentionally no backfill.A workflow saved before this migration has no stored revision, so it fails closed on every path until its owner saves it again — event triggers, schedules, webhooks, and manual runs alike. Workflows saved after the migration are unaffected.
That is a deliberate choice, not an oversight. The stored value points at the specific signed artifact that authorizes each wake; a backfill would have to guess which revision an existing row came from, and guessing is the exact thing this change exists to prevent. Re-saving is a single action per workflow, and the affected population is small. A backfill migration remains possible later as its own change.
Rollout
Relay and agent harness must both understand doorbells before workflow wakes are admitted. During a partial deployment, workflow wakes are dropped rather than granting relay-authored content owner authority. Ordinary messages are unaffected.
Scope
Doorbell authorization only. This PR contains no Desktop changes and no workflow listing or discovery work; existing Desktop behavior on
mainis untouched.Related issue
#3858 — relay-signed workflow output dropped before mention matching. This PR authorizes the wake for agents whose owner signed the workflow definition; it does not change behavior for an unrelated agent mentioned by someone else's workflow.
Testing
Not a UI change, so no screenshots.
f78ab0ee9: run 32271534677, 28 checks passed / 4 skipped / 0 failed.crates/buzz-acp/src/lib.rscover the accepted doorbell shape and the fail-closed cases: relay prose, malformed or duplicated provenance tags, an unverifiable prior-step reference, cause deduplication across relay replays, webhook rate limiting, and the rule thatptags never confer authority.crates/buzz-relay/src/workflow_sink.rsgains a Postgres-gated test asserting the emitted doorbell's tags, relay signature, discarded prose, and that a mention rendered out of trigger data does not become a second wake target.crates/buzz-test-client/tests/e2e_workflow_agent_owner.rs(run with--ignoredagainst a local relay) asserts the agent's human owner can trigger an agent-owned workflow while an unrelated channel member cannot.