Skip to content

feat(agent): add keyless broker runtime - #6967

Open
jrobotham-square wants to merge 13 commits into
jrobotham/broker-storage-rwfrom
jrobotham/keyless-client-slice
Open

feat(agent): add keyless broker runtime#6967
jrobotham-square wants to merge 13 commits into
jrobotham/broker-storage-rwfrom
jrobotham/keyless-client-slice

Conversation

@jrobotham-square

@jrobotham-square jrobotham-square commented Aug 28, 2026

Copy link
Copy Markdown

Stack Info: Stacked on #6922

Why

Allow agents to run without a signing key or direct relay access by delegating authorized operations to a broker host.

What

  • Add a shared HTTP broker transport with explicit local and broker modes.
  • Route messages, reactions, profiles, storage, presence, typing, telemetry, and liveness through broker actions.
  • Run buzz-acp through broker-polled reads with bounded replay deduplication and safe child provisioning.
  • Redact all MCP environment values from observer telemetry and ACP wire logs in both local and broker modes; child provisioning remains unchanged.
  • Require TLS outside loopback, provision explicit relay identity, and apply action-aware response bounds.

Risk Assessment

Low — broker mode is opt-in and local mode remains covered by existing tests. Basic end-to-end validation has exercised the keyless path through a signing broker against a real Buzz relay.

References


Update Aug 29, 10:11: Hardened broker boundaries following review.

  • Centralize redacted outbound ACP diagnostics, including session/new.
  • Force plaintext loopback broker traffic to bypass proxies.
  • Size channel.read response buffering from the validated page limit.
  • Account for outer JSON escaping when trimming broker observer frames.

Generated with Codex

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown

🔐 Codex Security Review

Status: review required for the current range.

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

@jrobotham-square
jrobotham-square force-pushed the jrobotham/keyless-client-slice branch from a101b80 to 1087586 Compare August 28, 2026 05:31
@jrobotham-square
jrobotham-square changed the base branch from main to jrobotham/broker-storage-rw August 28, 2026 05:32
@jrobotham-square jrobotham-square changed the title Keyless buzz client implementation feat(agent): add keyless broker client and runtime Aug 28, 2026
@jrobotham-square jrobotham-square changed the title feat(agent): add keyless broker client and runtime feat(agent): add keyless broker runtime Aug 28, 2026
@jrobotham-square
jrobotham-square marked this pull request as ready for review August 28, 2026 07:11
@jrobotham-square
jrobotham-square requested a review from a team as a code owner August 28, 2026 07:11

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Two credential-handling defects make the keyless boundary unsafe, and one request-bound bug makes a compromised broker unnecessarily expensive to survive:

  1. High: session/new writes the broker bearer credential to ACP debug logs. Broker mode places BUZZ_BROKER_CREDENTIAL in the MCP server env (crates/buzz-acp/src/lib.rs:5231-5248), which session_new_full embeds under params.mcpServers (crates/buzz-acp/src/acp.rs:699-724). Generic send_request then logs the original message at acp::wire=debug (acp.rs:1145-1166). The new redactor only protects observer telemetry in write_ndjson and the separate prompt path, so it does not protect this log. Apply the redactor to every outbound wire diagnostic, especially send_request, and pin session/new with a log-capture regression test.

  2. High: loopback HTTP can send the bearer through a system proxy in plaintext. HttpBrokerClient permits http://localhost and loopback literals, but its default reqwest client is created without .no_proxy() (crates/buzz-broker-client/src/lib.rs:52-95). Reqwest enables system/environment proxy discovery by default, so a machine with HTTP_PROXY/ALL_PROXY and no matching NO_PROXY routes the request, including the Authorization: Bearer header at lines 134-143, through that proxy. For plaintext loopback, structurally force a proxy-free client (and prevent with_client from weakening that invariant), or require HTTPS universally. Add a capture-proxy regression proving a loopback credential never reaches the proxy.

  3. Medium: channel.read buffers for the protocol-wide maximum instead of the requested page size. max_response_bytes grants every read 64 MiB solely from its action (crates/buzz-broker-client/src/lib.rs:14-25,128-169), while request-specific enforcement happens only after the entire body is parsed (crates/buzz-sdk/src/broker/mod.rs:731-765). The ACP poll always asks for 100 events (crates/buzz-acp/src/runtime_transport.rs:297-307), so a hostile or faulty broker can force repeated 64 MiB allocations for pages that will ultimately be rejected. Derive the transport cap from the validated request limit and test the limit-100 case.

The explicit-channel ACP polling path itself looks sound: signed-event verification, h-tag isolation, rotating dedup, terminal unauthenticated handling, and fail-closed unknown channel metadata are wired consistently. CI is green, and focused source/contract review was against exact head 937e446cf8fce5d1b7db5de4a78b87e6b6524370 and stacked base fcf2e9bc753e9ce047b21710c4262e106c2f706b. git diff --check passed. I did not run the external broker harness; the submitted head does not include one.

jrobotham-square and others added 13 commits August 29, 2026 10:06
Client-side [C1] of the agent broker (#6790): lets the buzz
CLI run without an agent nsec, routing reads and writes through a host
so relay traffic looks like a normal Buzz client.

- broker_client.rs: HttpBrokerClient implements the BrokerClient
  transport primitive the contract crate omits — POST /v1/action with an
  opaque bearer credential, parse an envelope regardless of HTTP status,
  never interpret a verdict. Correlation stays in execute().
- backend.rs: AgentBackend trait spoken in broker vocabulary, with two
  impls behind a Backend enum — BrokerBackend (keyless) and LocalBackend
  (nsec + relay, today's path). LocalBackend reuses the shared
  buzz_sdk::build_message builder, so there is no parallel message path.
- lib.rs: register the two modules.

Not yet wired into the command surface or provisioning. 8/8 unit tests
pass (4 transport, 4 backend); clippy -D warnings clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Joel Robotham <jrobotham@squareup.com>
Add the mode toggle for the keyless client. --agent-mode (env
BUZZ_AGENT_MODE, default "local") picks the backend; "broker" diverges
before any key is read and routes the wake->reply slice through the host:

- lib.rs: AgentMode enum + --agent-mode / --broker-url / --broker-credential
  flags; run() branches to run_broker(), which builds Backend::broker from
  the endpoint + credential. Broker mode fails closed if a private key is
  present (supplying one is a provisioning error, not silently ignored).
  --mentions-only added to `messages get` for the wake path.
- messages.rs: dispatch_broker maps `messages get` -> channel.read and
  `messages send`/reply -> message.post/message.reply, taking explicit
  pubkey mentions. Relay-coupled extras (auto @mention resolution, file
  upload, forum kinds, broadcast, time/kind windowing) are refused in
  broker mode rather than silently dropped; the local path is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Joel Robotham <jrobotham@squareup.com>
- examples/mock_broker.rs: a throwaway host that speaks just enough of the
  contract (POST /v1/action, echoes requestId/action, canned outcomes) to
  exercise the keyless CLI end to end before a real broker exists. Signs
  nothing, touches no relay.
- KEYLESS.md: brief instructions — build, run against the mock, and point
  the CLI at your own broker.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Joel Robotham <jrobotham@squareup.com>
Extend the AgentBackend seam with reaction_add / profile_set and route
two more command groups through it in keyless mode:

- reactions add  -> reaction.add  (requires --channel, the host's scope;
  refuses --emoji-url, which is host-owned custom-emoji handling)
- users set-profile -> profile.set (maps name/about/avatar; refuses
  --nip05, absent from the contract; requires >=1 field)

BrokerBackend maps each to its ActionArgs and unwraps the EventPublished
outcome, same as message.post. LocalBackend implements both for parity
(reaction via build_reaction; profile as read-merge-write over the
current kind:0, emulating the contract's 'absent fields left as-is').
Local command paths are untouched. Mock host and KEYLESS.md gain the two
actions; two BrokerBackend round-trip tests added.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Joel Robotham <jrobotham@squareup.com>
Add an addressing-only bridge for encrypted memory while the end-to-end runtime storage semantics remain deferred.

- add mem address <slug> and normalize shorthand to the NIP-AE slug
- route storage.address through AgentBackend in broker mode and print its validated JSON outcome
- derive the same address locally for command parity
- extend the mock host, backend round-trip coverage, and keyless docs

The returned coordinates identify a record but do not yet fetch, decrypt, encrypt, or publish it; KEYLESS.md records that deliberate temporary limit.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Joel Robotham <jrobotham@squareup.com>
Promote the HTTP broker transport into a shared crate and add an explicit runtime transport seam to buzz-acp. Broker mode rejects local keys, derives its identity through storage.address, polls configured channels through channel.read, validates returned events, and provisions agent subprocesses with broker-only credentials.

Relay-only housekeeping and enrichment stay disabled until the frozen contract grows an agreed host-owned path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Signed-off-by: Joel Robotham <jrobotham@squareup.com>
Remove empty local-credential tombstones from spawned agent environments and defensively treat an empty CLI private-key value as absent, while continuing to reject all real key material.

Bound broker HTTP actions, terminate polling on rejected credentials, and document the cursor, restart, and thread-context limits that remain for real-host integration.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Joel Robotham <jrobotham@squareup.com>
Signed-off-by: Joel Robotham <jrobotham@squareup.com>
Signed-off-by: Joel Robotham <jrobotham@squareup.com>
Signed-off-by: Joel Robotham <jrobotham@squareup.com>
Signed-off-by: Joel Robotham <jrobotham@squareup.com>
Signed-off-by: Joel Robotham <jrobotham@squareup.com>
Signed-off-by: Joel Robotham <jrobotham@squareup.com>
@jrobotham-square
jrobotham-square force-pushed the jrobotham/keyless-client-slice branch from 937e446 to f42a4a3 Compare August 29, 2026 00:11
@jrobotham-square

Copy link
Copy Markdown
Author

🤖 Updated by Codex in f42a4a3c1 — all three findings are addressed:

  1. Outbound ACP wire logging is now centralized through the MCP environment redactor, with a session/new log-capture regression covering the broker credential.
  2. Plaintext loopback broker clients now structurally disable proxies, and a capture-proxy regression verifies the bearer reaches only the broker.
  3. channel.read response buffering now derives its cap from the validated request limit, with coverage for the ACP limit of 100.

The rebase also accounts for the updated observer payload contract by including outer JSON escaping in the broker-frame budget. Local pre-push checks and CI are green on the new head. Ready for re-review.

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.

3 participants