feat: add SSH/systemd remote agent backend - #6052
Closed
jonssfredrik wants to merge 14 commits into
Closed
Conversation
The reference implementation of the `BackendKind::Provider` seam that `managed_agents/backend.rs` already describes and block#2859 says has no OSS implementation. One process per op, no daemon, no state: the desktop writes one JSON request to stdin and reads one JSON response from stdout, and this crate turns that into ssh. Six modules, one protocol implementation: protocol the wire types and the `Secret` that renders `[REDACTED]` ssh the transport; every secret rides the stdin channel tailscale peer enumeration and the re-auth recovery URL discover `check`, `discover_harnesses`, `probe_models` install resolve-or-install for `buzz-acp` and the `buzz` CLI deploy the remote script, the env file, the systemd unit They are one commit because they are one protocol implementation, and because they do not separate: `deploy.rs` reaches into `install` at 43 call sites and `include_str!`s the unit template, and `discover` and `deploy` are mutually dependent (`string_list` one way, `env_map` and `is_well_formed_env_key` the other). Any smaller unit would be a file state that never compiled. Three invariants hold across every op, stated in `main.rs`'s module doc and enforced at named sites: 1. Secrets never reach a log, an error string, a `Debug` rendering, or an argv. They travel only inside the script body on the SSH stdin channel, because the remote `ps` is world-readable. 2. A deploy without the desktop-minted `private_key_nsec` fails closed, inside `Agent::from_request` so no caller can route around it. Env values are written through a control-character guard, because a newline would split the assignment into a second, attacker-chosen line. 3. Tailscale is an enhancement, never a dependency: absent, logged out, or empty, the `info` schema is byte-identical to the plain one. Trust-on-first-use is allowed for exactly one class of address — a device this machine's own Tailscale daemon already lists as a peer, which required a WireGuard-authenticated tunnel to reach. For anything the user typed, host-key checking stays strict. The re-auth URL is constructed, never parsed out of host output, so a compromised host cannot make the desktop open an arbitrary URL. Harness discovery extends upstream's `hermes` preset rather than replacing it. Because a host commonly runs several Hermes profiles out of one install, `discover_harnesses` fans that single preset out into `hermes-<name>` entries carrying `--profile` args, so each configuration appears as its own harness. Profile names are untrusted input on their way into a catalog id and then an argument vector, so they are filtered against Hermes's own charset and capped — enforced twice, once in the script so the bytes are never sent and once on the way back in, because a host is free to ignore the script it was handed. The crate is deliberately NOT bundled with the desktop app. Provider discovery prepends the app bundle's own directory to the search path, so shipping it inside the bundle would give every install an auto-discovered SSH-deploy capability and quietly undermine the "Only use providers from trusted sources" warning the desktop already shows. It installs to `~/.local/bin`, which discovery already covers. 115 tests, no `unsafe`, no `unwrap`/`expect` on a production path. `base64` inherits from the workspace, matching the `sha2` line beside it and the comment above both, which already claimed the inheritance. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
The host contract in docs/remote-agents.md is short, but two of its items fail in ways that do not look like misconfiguration. Without `loginctl enable-linger` the user manager is torn down when the last session ends — so the agent is killed the moment the deploy's own SSH session closes, and the symptom is "the agent keeps dying", not "lingering is off". Without lingering there is also no /run/user/$(id -u), so every `systemctl --user` call fails at the bus before it reaches a unit. This script turns both into a row in a table you read before deploying instead of a mystery you debug after. It fixes what it safely can (enables lingering, creates ~/.local/bin, appends the PATH export to ~/.profile only when absent) and installs nothing. Harness CLIs carry their own installers and their own interactive logins, which cannot run over a non-interactive SSH deploy; buzz-acp and the buzz CLI the deploy op itself resolves or pushes. Reporting them is useful, installing them here would duplicate a code path that already exists and is tested. Only three checks decide the exit code — lingering, ~/.local/bin on PATH, and a reachable systemd --user bus — because only those three make a deploy fail. Everything else, Tailscale included, is a note: Tailscale buys the desktop's device picker, and plain SSH is the unchanged fallback, so a host without it is correctly configured, not degraded. Three details are deliberate and easy to "clean up" wrongly. It refuses to run as root: the env file holding the minted nsec must land owned by the user whose home already holds the harness credentials, and a root run would provision the wrong home. It checks the systemd bus under the same missing-XDG_RUNTIME_DIR assumption the deploy makes, since a check that only passes in a login shell would pass on hosts the deploy then fails on. And it reads Tailscale's SSH state from prefs rather than from `Self.sshHostKeys` in the status document — that field is populated for peers and reads null when a node looks at itself, so the obvious source reports a false negative on an SSH-enabled host. POSIX sh, no jq, no bashisms: it has to run on a bare host, which is the one place none of those are guaranteed. Idempotent — a provisioned host is a no-op. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
The systemd instance, env-file path and returned `backend_agent_id` were all derived from the agent's display name. A name is not unique: two agents called the same thing on one SSH account collapsed onto one unit and one env file, so the second deploy overwrote the first agent's minted nsec and starting either record drove whichever identity was written last. Key them on the agent's pubkey instead — the record's own primary key, and the only stable identifier the payload can carry. The slug keeps the name as its readable stem and appends a 12-hex fragment of the key, so `systemctl --user status` still reads as the agent a human recognizes. The payload field is required, not optional: falling back to the name on an absent `pubkey` would reintroduce the collision silently, so deploy refuses a payload without a 64-character hex key. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
ExecStart= splits an unquoted value on whitespace, so a "buzz-acp path on the server" naming a directory with a space in it — which the config schema accepts — made systemd run the first word with the rest as arguments, and the unit never started. The SSH resolution code already shell-quotes such a path; systemd's command-line syntax is a different question. Write the substituted path double-quoted, escaping backslash and double quote first since systemd unquotes C-style escapes inside double quotes. The escape pass filters through sed rather than editing in place, so the portability argument that kept sed -i out still holds. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
A remote agent runs arbitrary code by design, so the SSH user's own privileges are the intended ceiling. Without NoNewPrivileges the harness can climb past them through any setuid/setgid binary on the host, or through passwordless sudo granted to that user. A VPS pilot exercised the harness, the Buzz CLI, NIP-OA owner-reviewed draft creation and repository branch pushes with it set; all remained functional. Deliberately the whole hardening delta. ProtectSystem/ProtectHome belong in this unit too, but an agent has no modeled workspace yet, so until writable paths are something the protocol states those directives would be guessing at which of the user's home an agent legitimately needs. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
git-credential-nostr had no preflight row of its own. It is a third tool with a third policy: deploy resolves it, never installs it, and writes the agent's GIT_CONFIG_* block only when the host already has it — so a remote agent on a host without it cannot push to a Buzz repository, with no deploy-time warning and no error until the agent tries. Folding that into the non-fatal buzz CLI check hid a different failure behind a different tool's status. Give it its own row, still reported and still never mandatory. Record the workspace gap as a known limitation while it is one. A local managed agent runs inside Desktop's REPOS workspace; the unit here has no WorkingDirectory at all, so a remote agent has no Buzz-native project checkout and in practice operates in a separately provisioned one. Closing it is a protocol addition — a workspace field the desktop states, rather than the provider guessing which project to clone. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
…ctive SSH A non-interactive `ssh host sh -s` reads no profile. On stock Debian that leaves PATH at `/usr/local/bin:/usr/bin:/bin:/usr/games`, which does not contain `~/.local/bin` — where every Buzz tool installs, and where `pipx` and `npm --prefix=~/.local` put the harness adapters. Every lookup that went through a bare `command -v` therefore answered for the wrong PATH. On a host with all twelve adapters installed, `discover` reported the entire catalog as `available: false`, and the deploy that followed refused the operator's pin with exit 91, "harness not found". The env file already pinned `PATH="$HOME/.local/bin:$PATH"` for the unit, so the contract was only half-applied: the spawned harness could see the install destination, but the scripts that resolved the harness could not. Prepend it in the scripts too, via one shared `install::PATH_PREAMBLE`, so both sides ask the same question. That also simplifies the env file's own PATH line. It now writes the deploy shell's `$PATH` verbatim, which already leads with the install destination — one prepend, in one place, instead of a second one that could drift from the first. `resolve` is unchanged: it tests `-x ~/.local/bin/<tool>` explicitly because it must also answer for an absolute configured path. Covered by two tests that run the generated scripts against a real `/bin/sh` with the stock Debian PATH and an adapter installed only to `~/.local/bin` — the reported case, in both the discover and the deploy direction. Both fail without the preamble. The provisioning preflight gets the same prepend for the same reason, so it stops reporting a tool MISSING on a host where the deploy finds it. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
The `~/.local/bin` row was mandatory and asked whether the directory was on the PATH of the process running the check. Over a non-interactive `ssh host /path/to/provision-buzz-host.sh` it never can be: ~/.profile is read by a login shell, so the check always landed in the NOTE arm and a fully provisioned host exited 1 against its own preflight. The strict form was not load-bearing. Nothing in the deploy path consults the login PATH — the deploy composes the unit's PATH itself, and both remote scripts prepend the install destination before they resolve anything. What is worth requiring is that the directory exists and that a login shell on the host is configured to find it, which the ~/.profile arms already guarantee. So those arms report OK and carry the difference in the ACTION column: whether a re-login is still owed. A host with neither the PATH entry nor the ~/.profile line still gets the line appended and still says so. Exit semantics are otherwise unchanged: a genuinely unsatisfied mandatory row — no lingering, no user bus — still exits 1. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
docs/remote-agents.md §Info fixes the provider contract at protocol_version 1 and requires an integer. §Discovery makes it load-bearing rather than informational: the desktop's pre-secret negotiation gate stages the provider binary, calls info on the staged bytes, and must validate an explicit supported protocol_version before it sends a request carrying private_key_nsec. Absence is an error there, not a presumed 1 — so a provider that omits the field is not degraded, it is undeployable. Emit it as protocol::PROTOCOL_VERSION, distinct from CARGO_PKG_VERSION: the software version says nothing about wire compatibility. The test asserts the JSON type as well as the value, because a string "1" would fail the gate exactly like an absent field. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
…t the payload docs/remote-agents.md §Deploy, Step 0: "the provider MUST parse private_key_nsec and derive the public key from it; a malformed or undecodable key is an immediate in-band error. Every selector, name, and comparison below uses the *derived* pubkey — never a caller-supplied one." The provider was taking the pubkey straight off the payload, which is the caller-supplied identity the spec rules out. It matters here because that value is not decorative: the first 12 hex characters of it name the systemd unit, the env file and backend_agent_id. A payload whose pubkey and nsec disagreed would name every host-side artifact for one identity while the harness authenticated to the relay as another — an agent that looks deployed and is permanently unreachable, which is the exact failure the fail-closed nsec check already exists to prevent. New identity module: bech32-decode the nsec (rejecting a wrong hrp, so an npub in the private key's field is caught by name), take the secp256k1 x-only public key, and reconcile against the payload's pubkey if it carried one. The derived value always wins and is never replaced by the assertion; a well-formed assertion for a different key is a fatal in-band error. Decoded key bytes are Zeroizing, and no error string echoes any part of a key — both pubkeys are truncated to a 12-character fragment, since the response is persisted in the desktop's last_error. Test fixtures now use real nsec/pubkey pairs, because an arbitrary pubkey beside an unrelated nsec no longer parses at all — which is the point. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
Requested on PR block#3449: a deploying user whose host keys do not live at ~/.ssh/known_hosts — a shared or generated file, or one kept per-fleet rather than per-user — had no way to point the provider at it, and copying entries across is not a fix. Optional ssh_known_hosts_file in SshConfig and the info config_schema, forwarded as `-o UserKnownHostsFile=<path>`. It selects which file the host key is checked against and nothing else: StrictHostKeyChecking is untouched, so a key missing from the named file fails exactly as one missing from the default would. This is not a way to relax host-key trust. Left unset, the option is not passed at all — the test asserts the argv is byte-identical to the pre-existing one, since an optional field that perturbs the default connection is not optional. Named to survive the desktop's validate_provider_config, which word-splits a config key and silently drops anything containing `key`. The obvious ssh_host_key_file spelling would arrive as absent with no error anywhere, the same trap that made the identity field ssh_identity_file rather than ssh_key_path; both names are now pinned by the schema test. Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
…e the key
Self-review of the Step 0 work found three things worth correcting.
The length check was `if bytes.len() != 32` followed by
`try_from(...).expect("length checked above")` — a panic in a production path,
which this repo forbids, and one whose safety depended on two separate
statements staying in sync. Taking `try_from` as the fallible conversion makes
the panic-free property structural instead of asserted.
secp256k1::SecretKey does not zeroize on drop; it exposes an opt-in
`non_secure_erase`, which this now calls. The module doc previously implied
guaranteed erasure — it is corrected to what the code actually achieves, which
is best-effort hygiene on a value that lives for microseconds. Overclaiming in
a comment about key handling is worse than not commenting.
`fragment` slices by byte index and is only safe because `char_indices` yields
char boundaries. It is reached with hex today, but a helper must not depend on
its caller's validation to avoid panicking, so a test pins it against
multi-byte input.
Signed-off-by: Troy Hoffman <troy.hoffman@icloud.com>
Signed-off-by: Fredrik Jonsson <jonssfredrik@gmail.com>
Signed-off-by: Fredrik Jonsson <jonssfredrik@gmail.com>
Author
|
Synced this branch with current |
themiguelamador
suggested changes
Aug 16, 2026
themiguelamador
left a comment
themiguelamador
left a comment
There was a problem hiding this comment.
I found several release-blocking correctness and security issues. I fixed all of them on Complear:review/pr-6052-fix at 3d858fd7a1efdc46919229a5799588b83549bc58.
Findings:
- P0 — every current Desktop launch payload is rejected. Desktop always emits reserved
BUZZ_ACP_RELAY_OBSERVERand may emitBUZZ_ACP_DISPLAY_NAMEinlaunch.policy_env, while the provider allowed onlyBUZZ_ACP_AGENTS. A normal deploy therefore failed before opening SSH. The fix distinguishes Desktop-owned policy output from user env while continuing to reject identity/access-key forgery. - P1 — documented binary push was never wired from Desktop. The provider and docs consume
agent.buzz_acp_binary/agent.buzz_cli_binary, butbuild_deploy_payloadnever serialized the documentedBUZZ_ACP_PUSH_BINARY/BUZZ_CLI_PUSH_BINARYvalues. Fresh-host dogfood deploys therefore always failed at missingbuzz-acp. The fix wires both paths at the deploy boundary with an environment-independent unit test. - P1 — clean shutdown and inactivity expiry were undone by systemd.
Restart=alwaysresurrected an owner!shutdownor clean inactivity exit after five seconds. The unit now usesRestart=on-failure, preserving crash recovery and clean lifecycle controls. - P1 — one host-wide unit template caused cross-agent executable mutation. Two provider definitions targeting the same host with different
buzz_acp_pathvalues rewrote the samebuzz-acp@.service; a later restart silently moved an existing agent to the other definition's executable. Deploy now materializes one concrete unit per agent, with a two-agent regression test. - P1 — concurrent first installs could overwrite the shared fleet binary. The old check-then-
mvsequence let racing desktops install different payloads to the same path, with the last writer replacing the first. Installation now uses an atomic same-filesystem hard-link create that cannot overwrite a winner, and rejects directories that merely satisfy-x. - P1 — concurrent starts of one record shared
.newfiles. Env and unit writes could interleave, including the nsec-bearing env file. PID-scoped temporary names, cleanup traps, and atomic renames make concurrent deploys complete-file last-writer-wins updates. - P1 — host-resolved paths could corrupt env/unit syntax. Harness, PATH, credential-helper, Claude CLI, and
buzz-acppaths were written without EnvironmentFile escaping;ExecStartalso failed to escape systemd%specifiers. Control characters now fail before the secret directory is created, quoted values are escaped, and literal%survives systemd parsing. - P2 — 48-bit unit identity was unnecessarily collision-prone. A 12-hex pubkey prefix selected the env file containing an nsec. The complete 64-character derived pubkey now identifies the unit and fits comfortably within systemd's limit.
- P2 — malformed wire values silently fell back or disappeared. Non-object
launch, missing resolved commands, malformed args/env/policy maps, invalid numbers, ports, access modes/pubkeys, config strings, and model-probe values were variously dropped or replaced with defaults. The provider now fails closed while preserving argument bytes exactly; surrounding nsec whitespace is canonicalized consistently for derivation and runtime emission. - P2 — local Tailscale discovery could consume the outer operation budget.
tailscale status --jsonhad no timeout, so a wedged daemon could hang aninfoor deploy call. It now has an internal two-second kill budget and bounded output handling. - P2 — the Tailscale picker offered guaranteed-failure hosts. macOS, Windows, phone, TV, and unknown peers were offered even though this binding always provisions
systemd --user. Only Linux peers are now offered; manual SSH remains unchanged. - P2 — the claimed hermetic test suite failed on macOS. Generated-shell tests assumed Linux
sha256sum; the fake host now supplies the Linux spelling through macOSshasum. - Minor — provisioning could treat a comment mentioning
.local/binas PATH configuration. The preflight now requires an active PATH assignment. Installation and lifecycle documentation was updated to match the implemented behavior and remove stale limitations already solved by the resolved launch block.
Verification:
cargo test -p buzz-backend-ssh— 142 passedcargo clippy -p buzz-backend-ssh --all-targets -- -D warnings— passedbash -n scripts/provision-buzz-host.sh— passedgit diff --check— passedjust test-unit— seven suites passed (buzz-core,buzz-auth,buzz-cli,buzz-db,buzz-conformance,buzz-push-gateway,buzz-backend-kubernetes);buzz-voicecould not build becausesherpa-onnx-systimed out downloading its macOS archive from GitHub.- Focused Desktop Rust test — blocked before crate compilation by the same
sherpa-onnx-sysarchive download; an earlier attempt also reached the checkout's missing required Tauri sidecar resource (desktop/src-tauri/binaries/buzz-acp-aarch64-apple-darwin). The edited Rust file was formatted by the repository pre-commit hook.
Please incorporate the fix commit (or equivalent changes) before merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
buzz-backend-sshprovider for SSH deployment into per-agent systemd user unitslaunchblock so remote command, args, env precedence, owner fallback, and effective parallelism match local semanticsVerification
cargo test -p buzz-backend-ssh— 133 passedcargo clippy -p buzz-backend-ssh --all-targets -- -D warningsbash -n scripts/provision-buzz-host.shgit diff --checkDesktop Rust tests were attempted but this VPS lacks the GLib development package required by
glib-sys; compilation stopped before project tests. No system packages were installed.Based on the field-tested SSH provider work from #3449, ported as a focused successor onto current
main.