acpmux: the Mac runs an added machine's GitHub action with its own gh login

Merged
TE
8d ago
+775-40

Leo's ask (cmuxterm-hq#4770): "you can just not choose a project ... you just get a little green dot ... as if you were working on your own computer, but now it's remote." On an added machine, that includes GitHub working with no gh auth login there.

Summary

This is part 1 of 3, the Mac side. It builds on #26917 (merged), the broker core (github_broker.rs and its unit tests), split out to keep each PR under about 800 lines; this PR wires it into the peer link. When the Mac lends GitHub to a peer, it answers that peer's _acpmux/github_action request by running one fixed gh command with its own keyring login. It sends back the bounded output and no token.

  • PeerConfig.githubBroker (default off, omitted from the file when off). peer_add takes githubBroker, a re-add at the same URL keeps the old value, the peers listing shows it, and a config reload applies it.
  • The Mac's read loop (peer.rs) used to refuse every request from a peer with method_not_found. It now serves _acpmux/github_action when the flag is on, and still refuses every other request. Admission (flag on and one of 4 slots free) happens before anything is spawned.
  • When the flag is on, the Mac's initialize says _meta.acpmux.githubBroker: true, so the peer knows which link to ask (part 2).
  • github_broker.rs (in #26917) holds the vocabulary, the validation, the fixed gh arguments, bounded and redacted output and the audit row.
  • An action already running stops when lending ends (githubBroker turned off, the peer removed, its link ended): its gh is killed and a link still up is told github.denied.
  • A refusal of a peer's request is sent within the same time budget as every other frame, so a peer that asks and never reads loses the link instead of holding it.

Part 2 (the added machine asks, #26784) and part 3 (Add Machine turns this on and acpmux gh, #26785) are stacked on this branch, then #26914, #26909 and #26910 (App tokens when the Mac has no gh login). Each one lands on its own.

Why the design changed

The earlier design here had the added machine forward to the Mac over a separate link, with shared HMAC secrets and an exact-session policy file the person had to write. An Add Machine peer has none of those, so it could never be on by default. This version runs over the link the Mac already opened, and the Mac's own config flag is the only grant. The replay guard, policy file and secrets are gone because nothing needs them now: requests arrive on the Mac's own authenticated link. Push is dropped. The webview row is dropped too, because the event is stored and shown as a plain event.

Threat model (the whole feature: this PR and its two stacked follow-ups)

  • What is lent: the person's gh keyring login on their Mac. It never leaves the Mac. The Mac runs gh itself and sends back only the command's output, capped at 64 KiB. No token goes into a request, a reply, the peer's env, argv, disk or logs. The gh that runs is the Mac's own, with null stdin, GH_PROMPT_DISABLED=1, a 60 s deadline and kill_on_drop.
  • Who may ask: on the Mac, only a peer whose config has githubBroker: true, and only on a link where the peer has answered the handshake's watch while serving the Mac as a peer (not as Web). Anything sent before that, or on any other link, is refused. Two things set the flag. One is acpmux host enroll, which is what Add Machine runs. The other is a peer_add that names it, or the config file. Both go over the Mac's unix socket (0600, in a 0700 dir), so only the person's own processes can reach them, and those can already run gh themselves. host doctor --fix, a plain peer_add, a lease and a pairing never set it. Every other peer gets github.denied before anything is spawned. On the added machine, only a caller-bound agent session on the local unix socket may ask. Web, LocalApp and Peer origins are refused in remote_guard. A request goes up only a link that the Mac opened with a PeerToken and that said githubBroker: true.
  • What may be asked: five fixed gh commands: repo view, pr view N, issue create, issue comment N and pr create --draft. Every caller value goes in --flag=value form, so a title like --repo=evil/x stays a title. The repo is owner/name with no leading - and no path parts. The branch has no : (a fork's branch), no .., no @{ and no leading -. Unknown fields are refused (deny_unknown_fields), not ignored. The Mac checks every request again itself and does not trust the added machine's check. There is no merge, close, delete, api, auth, push or workflow action.
  • What an attacker on the added machine gets: root or any agent there can ask for those five actions as the person, for as long as the Mac keeps the link open. This is the same trust as ssh-agent forwarding to a machine you added. It cannot get the token, run any other gh command, or reach repos the person's login cannot.
  • Limits: 4 actions at a time per peer (github.busy past that), 60 s each on the Mac, 64 KiB of output and of each body (acpmux gh reads no more of a body file than that). Each request carries a deadlineMs that the Mac checks before it runs anything (github.expired once passed), the added machine waits at most 95 s, and at most 8 requests wait on one link. At most 16 lending links are remembered.
  • Records: each action appends a github_broker_action event to the asking session on the added machine. It holds the action, repo, branch, issue number, status and error code, and never the title, body or output. The Mac logs one tracing line per action with the same fields.
  • Transport: lending needs an ssh:// link (what Add Machine saves), wss://, or ws:// on loopback. Over plain ws:// to another host, anything on the path could answer the handshake as the peer, so the flag is ignored there.
  • Revocation: turning lending off, removing the peer or losing its link stops the actions still running for it at once.
  • gh aliases cannot replace the five commands: gh refuses an alias whose name is a built-in command, and repo view, pr view, issue create, issue comment and pr create all are. The Mac's gh config is the person's own; the added machine cannot write it.
  • Residual: gh's stderr is relayed back, bounded and with any GitHub token cut out, so a failed action can show a path from the Mac. If two Macs lend to one machine, the newest link is used. git push of local commits is not brokered: the agent's branch must reach GitHub some other way before a draft PR can name it.

Tests (cmux-ci, Linux)

  • Before (base f2321aa with only this PR's first tests/github_broker.rs): the_mac_runs_an_added_machines_action_with_its_own_gh_and_no_other_peers FAILED. The Mac answered {"code":-32601,"message":"Method not found: client-side request"}. Run: https://github.com/manaflow-ai/cmux/actions/runs/40651696477
  • Before, for the review fixes (the first head 758863e with this head's tests/github_broker.rs): two tests FAILED. the_mac_lends_only_on_a_link_whose_handshake_made_it_a_peer got a result for a request sent before the handshake finished. lending_to_a_peer_added_before_keeps_its_tokens got (None, None) for the saved tokens. Run: https://github.com/manaflow-ai/cmux/actions/runs/40656445480
  • After, at 8d242fe on the older base: cargo clippy --locked -p acpmux --all-targets -- -D warnings was clean. The unit tests github_broker/peer_lease/catalog_reload passed (14), and so did the integration tests github_broker (3), peer_lease, web_token_exposure, remote_guard and peer_integration (50). Run: https://github.com/manaflow-ai/cmux/actions/runs/40656411175
  • After the rebase onto #26756, at d703168: #26756 changed the peer handshake (link_request_on) and the outbound queue (Outbound), so the initialize _meta and the reply frame were updated to match. Clippy was clean, and the unit tests and the integration tests github_broker, remote_guard, remote_guard_peer, peer_integration, web_token_exposure, pairing and peer_lease passed. Run: https://github.com/manaflow-ai/cmux/actions/runs/40667211363
  • Before, for review round 3 (reload): d703168 with a test that reloads m as read at /old with lending on, onto a live m at /new without it. a_reload_lends_only_to_the_machine_it_read FAILED (the /new machine got lending). Run: https://github.com/manaflow-ai/cmux/actions/runs/40675851865
  • Before, for review round 4 (same-url revocation): 828f855 with a test that adds m lending, reads the file, then re-adds m at the same url without lending and applies the reload. a_reload_read_before_an_add_leaves_what_the_add_did FAILED: the reload gave lending back. Run: https://github.com/manaflow-ai/cmux/actions/runs/40693352576
  • Before, for review round 6: 4dfbc25 with this head's tests/github_broker.rs. the_mac_lends_only_on_a_link_whose_handshake_made_it_a_peer FAILED: an action sent right after the initialize reply, before the watch, ran gh. Run: https://github.com/manaflow-ai/cmux/actions/runs/40718970639
  • After, at this head 30e24d1: clippy was clean. The unit tests passed, including a_reload_lends_only_to_the_machine_and_the_add_it_read and a_reload_or_reconnect_waits_for_a_peer_change_in_progress. The integration tests passed too, including no_page_pane_or_peer_asks_for_a_github_action and the handshake test above, as did remote_guard, remote_guard_peer, peer_integration, web_token_exposure, pairing, peer_lease and start_mode. Run: https://github.com/manaflow-ai/cmux/actions/runs/40718967152
    • The round-5 lock-scope fixes (the reload reads the add count under the config lock, and a reconnect holds that lock through starting its peer) cannot be made to fail deterministically on the old code without a hook mid-function, so there is no before-run for them. The test holds a change mid-way and checks that neither proceeds until it is saved.
  • Before, for review round 7: 30e24d1 with this head's tests. a_reconnect_shares_its_github_slots_with_the_actions_still_running and a_removed_peer_is_never_started_again_by_a_reconnect_in_progress FAILED. Run: https://github.com/manaflow-ai/cmux/actions/runs/40726807643
  • Before, for review round 8 (redaction, now in #26917): no_token_gh_prints_goes_back_to_the_machine FAILED: a gho_ token on stderr and a github_pat_ token on stdout went back. Run: cmux-ci fleet job 7f032bd11937b8783cb7ebda.
  • Before, for review round 9: 946c3d5 (the previous head on #26917) with this head's tests. an_action_running_when_lending_ends_is_stopped FAILED (the stand-in gh finished 4 s after the peer was removed), and a_peer_that_asks_and_never_reads_loses_the_link FAILED (the link's loop hung on the refusal past 10 s). Run: cmux-ci fleet job 8ee0802b9ca24cba7551359e.
  • Before, for review round 10: 45ab6b1 with this head's tests. github_is_lent_only_over_ssh_tls_or_loopback FAILED (a plain ws://10.0.0.5 peer was lent to), and the_mac_runs_an_added_machines_action_with_its_own_gh_and_no_other_peers FAILED (with a decoy GITHUB_TOKEN in the daemon's env, the stand-in gh saw a token and no GH_HOST). Run: cmux-ci fleet job 39ec024a998fbd5ccc41789a.
  • After, at 45ab6b1: cargo fmt --check and clippy -D warnings were clean. Unit tests github_broker github_app_token github_owner gh_shim host_peering client_request remote_guard peer_lease catalog_reload list_reply hub::peers peer::link_epoch_tests server::outbound_queue_tests passed, and so did the integration tests github_broker (6), remote_guard, remote_guard_peer, peer_integration, web_token_exposure, pairing, peer_lease and start_mode. Run: cmux-ci fleet job 33cb6f56ce0628dfa10f7e60.
  • After, at ee07ea5: the same checks passed. Run: cmux-ci fleet job 36db97b3683efd2b02a740df.
  • Before, for review round 11: the round-11 head with this head's peer.rs tests. peer::link_epoch_tests::an_action_admitted_before_a_stop_never_runs FAILED: an action admitted just before stop() still started gh. Run: cmux-ci fleet job 4aaaae033db3eaa391b224fb.
  • After, at 9d7fd4d: the same checks passed. Run: cmux-ci fleet job fe6aacddd9ed2fef4f778539.
  • After, at d9d4dff (9d7fd4d with feat-cmux-next merged in, after #26917 landed; the PR's diff is unchanged): cargo fmt --check, clippy -D warnings, the unit tests and the integration tests above all passed. Run: https://github.com/manaflow-ai/cmux/actions/runs/40773721227
  • After, at this head 3a490fe (feat-cmux-next 8e36e52 merged in, with the hub/peers.rs conflict resolved): the same checks passed. Run: https://github.com/manaflow-ai/cmux/actions/runs/40785371293
  • PR CI at b93f3a3: the fleet Release compile was green. Its reds were ActionSurfaceParityTests (voiceRoom.open surfaces) with its generated-files verdict, PaneBurstMotionTests and "Package dependency lists sorted". This PR touches no Swift or Package.swift, and the same reds show on other open PRs (for example run 40697422837), so they are base reds.
  • The integration tests use a fake gh on PATH to check five things:
    • a lending Mac runs gh repo view o/r --json=... and gh issue comment 7 --body=hi --repo=o/r with no GH_TOKEN/GITHUB_TOKEN in the env;
    • a merge is refused with -32602;
    • a peer that is not lent to gets github.denied;
    • a request sent before the handshake finishes, or on a link where the peer served the Mac as Web, gets github.denied;
    • lending to a peer added before keeps its saved tokens;
    • an action running when the peer is removed is stopped.

Review

Independent Codex gpt-6-astra reviews (high, fast, read-only):

  • Round 1, at 758863e: BLOCK, two findings.
    • The Mac served a lent peer's request before the handshake finished, and on a link where the peer served it as Web. Fixed: github_ready takes a request only on the link whose own handshake said origin: peer.
    • peer_add takes githubBroker from any unix-socket caller. Kept, with the reason in the threat model: only the person's own processes can reach that socket, and they can already run gh.
  • Round 1 of part 3 also found that a re-add for lending dropped a peer's saved tokens. Fixed here in add_peer_lending.
  • Round 2, at 8d242fe: SHIP.
  • Round 3, at d703168 after the rebase onto #26756: BLOCK, two findings, both fixed in 828f855.
    • A reload applied the lending it read to whatever live peer had that name by then. Now it applies only to the peer at the url it read.
    • A reply task dropped its slot before an unbounded enqueue. Now it holds the slot until the reply is queued, waiting at most the link's request budget.
  • Round 4, at 828f855: BLOCK. A reload whose file read came before a same-url peer_add brought back what that add took away. Fixed: the reload now applies saved peers' flags inside the config lock, and peer_add holds that lock through starting its peer. A generation that every peer_add moves makes a reload that read before one leave the saved peers alone.
  • Round 5, at b93f3a3: BLOCK. The reload read the add count outside the config lock, and a reconnect started a peer from a config read it had already let go. Both fixed as above.
  • Round 6, at 4dfbc25: BLOCK, two findings.
    • This part did not yet keep _acpmux/github_action from being forwarded or asked from a WebSocket origin. It now does both, where part 2 used to.
    • The gate opened at initialize, before the watch. It now opens when the reader gets this link's watch reply, and the added machine counts the Mac only after it has queued that reply.
  • Round 7, at 30e24d1: BLOCK. A removal raced a reconnect in progress, and each reconnect got slots of its own while the old actions still ran. Both fixed (remove_peer holds the config lock through stop and save; slots are per peer name, owner_slots).
  • Round 8, at c43d357: BLOCK. gh's raw output went back, so a token it printed would too. Fixed (redaction, now in #26917). It also restated the peer_add provenance point, kept as in round 1.
  • Round 9, at f2e68a4: BLOCK, three findings. Actions ran on after revocation, and refusals were sent with no time limit; both fixed above. It also said gh aliases could replace the fixed commands; not changed, because gh refuses an alias named like a built-in command (threat model above).
  • Round 10, at 45ab6b1: BLOCK. A plain ws:// peer on another host could be lent to; fixed (transport, above). Round 1 of #26917 also found the env gap, fixed there.
  • Round 11, at ee07ea5: BLOCK. An action admitted just before a removal's stop() could still start gh after it. Fixed: the handler takes the link's own epoch and checks github_ready(link) (not stopped, same link) before it runs, and the revocation listener is registered first; test an_action_admitted_before_a_stop_never_runs.
  • Round 12, at 9d7fd4d: SHIP.
  • Round 13, at this head d9d4dff (feat-cmux-next merged in): BLOCK, one finding: host enroll does not set githubBroker. That is by design for this part, which ships the broker off; part 3 (#26785) turns it on for Add Machine, with an enrollment test.
  • Round 14, at d9d4dff, with that scope stated: SHIP.
  • Round 15, at this head 3a490fe: feat-cmux-next moved on and #27030 conflicted in hub/peers.rs. reconnect_peer now goes through #27030's reconnect_peer_with(force: true), which still holds the config lock through start_peer, and drops it before waiting on a link it leaves alone.

Review (codex gpt-6-astra, high, fast): SHIP at 3a490fef884abb06dd7be8f309ce5f62f4ed3122. "No findings."

Size: about 770 changed lines; the broker core is #26917.

Changelog

none (off unless a peer has githubBroker: true; part 3 turns it on for Add Machine)

No capture: nothing is drawn. The one webview file is the generated methods.ts method list (two names). No Packages/macOS file changes.

Risk

risk:high (socket and relay policy). Evidence: the SHIP above at this head, and the green clippy and tests at this head.

Agent session: agent session

šŸ¤– Generated with Claude Code