feat: WireGuard overlay for owner-operated devices (#28) #31

Merged

heike merged 1 commit from feat/issue-28-wireguard-overlay into main 2026-11-01 19:43:49 +00:00

Conversation 1 Commits 1 Files changed 10 +721

heike commented 2026-10-31 10:46:09 +00:00

Member

Copy link

Implements #28 on the manager's lean ("wireguard seems like a good option to me right now" — recorded on the issue; still a lean, this PR does not stamp it FIRM).

Architecture in one line: the overlay makes existing machinery work — a provider URL becomes http://<overlay-ip>:<port>/v1 (proxy unchanged), an agent host is created with address = overlay IP (provisioner unchanged, SSH rides the tunnel). Central runs the hub; one UDP port on the PUBLIC central box; devices make zero inbound connections — all three constraints from #28 respected.

Pieces: app/wireguard.py (device CRUD, lazy hub keypair 0600, /24 allocation, wg0.conf rewrite + wg set/syncconf, wg-show handshake merge, revocation), /internal/wg/* endpoints (R68: token + root human on mutations, 501 until TG_WIREGUARD=yes), hub auto-arm at startup (logged, never fatal), Meteor methods + a Devices section in the Hosts tab (add-device form, once-only client config with copy button — device private key never stored, test-pinned — live online/handshake/tx-rx table, revoke with confirm), run-central exports the new vars, update stage 3c installs wireguard-tools when armed.

Verification, honest: runtime suite 163 passed (7 new: IP sequencing, config rendering incl. endpoint/keepalive/allowed-ips, device-key-never-persisted, input rejection, revoke idempotency, handshake merge, ensure idempotency, endpoint override). UI verified visually (not-armed state renders cleanly in the Hosts tab). Limit stated up front: this sandbox has no wg binary — all wg interactions are stub-level; the kernel-side bring-up (wg-quick up, real handshake) gets its proof at first prod arm, not here.

Review asks: the once-only key display path (Meteor method returns client_config — is anything logging it?), the R68 guard coverage on the three mutations, and whether revocation dropping the peer from wg0.conf is sufficient or you want key rotation too.

For Felix after merge: TG_WIREGUARD=yes in env -> treegent update (stage 3c installs tools; runtime arms the hub at restart) -> Hosts tab shows the Devices panel -> Add device -> run the shown config on the Mac Studio/GB10 -> its overlay IP appears, handshake column goes green.

Implements #28 on the manager's lean ("wireguard seems like a good option to me right now" — recorded on the issue; still a lean, this PR does not stamp it FIRM). **Architecture in one line:** the overlay makes existing machinery work — a provider URL becomes `http://<overlay-ip>:<port>/v1` (proxy unchanged), an agent host is created with address = overlay IP (provisioner unchanged, SSH rides the tunnel). Central runs the hub; one UDP port on the PUBLIC central box; devices make zero inbound connections — all three constraints from #28 respected. **Pieces:** `app/wireguard.py` (device CRUD, lazy hub keypair 0600, /24 allocation, wg0.conf rewrite + wg set/syncconf, wg-show handshake merge, revocation), `/internal/wg/*` endpoints (R68: token + root human on mutations, 501 until `TG_WIREGUARD=yes`), hub auto-arm at startup (logged, never fatal), Meteor methods + a Devices section in the Hosts tab (add-device form, once-only client config with copy button — device private key never stored, test-pinned — live online/handshake/tx-rx table, revoke with confirm), run-central exports the new vars, update stage 3c installs wireguard-tools when armed. **Verification, honest:** runtime suite 163 passed (7 new: IP sequencing, config rendering incl. endpoint/keepalive/allowed-ips, device-key-never-persisted, input rejection, revoke idempotency, handshake merge, ensure idempotency, endpoint override). UI verified visually (not-armed state renders cleanly in the Hosts tab). **Limit stated up front:** this sandbox has no wg binary — all wg interactions are stub-level; the kernel-side bring-up (wg-quick up, real handshake) gets its proof at first prod arm, not here. **Review asks:** the once-only key display path (Meteor method returns client_config — is anything logging it?), the R68 guard coverage on the three mutations, and whether revocation dropping the peer from wg0.conf is sufficient or you want key rotation too. **For Felix after merge:** `TG_WIREGUARD=yes` in env -> `treegent update` (stage 3c installs tools; runtime arms the hub at restart) -> Hosts tab shows the Devices panel -> Add device -> run the shown config on the Mac Studio/GB10 -> its overlay IP appears, handshake column goes green.

heike added 2 commits 2026-10-31 10:46:10 +00:00

sec: harden provision heredoc for operator secrets (review 27 follow-up) ... 1efbf666a4

Rosa's non-blocking note on #27: the search key is the first
operator-supplied secret rendered into an UNQUOTED heredoc — the remote
shell would expand $ / backticks / $(...) inside the value before it
lands in runtime.env (worst case: execution on the agent host at
provision time; expected case: silent key mangling -> 401s that read
as 'bad key').

Two layers:
- values are single-quoted in the generated runtime.env lines
- _agenthost_script refuses to render when provider or key contains
  any of $ ` backslash double-quote single-quote

Verified: clean key renders quoted; $(id) and backtick keys refused
loudly; runtime suite 156 green.

feat: WireGuard overlay for owner-operated devices (issue #28, manager lean) ... bf897b8891

The overlay makes EXISTING machinery work behind NAT / dynamic IPs /
no-inbound-port policies (all three constraints from #28):
- inference: provider URL becomes http://<overlay-ip>:<port>/v1 in the
  Models tab — zero proxy changes
- agent hosts: create the WG device first, run the config on the box,
  then create the host with address = overlay IP — the provisioner SSHes
  over the overlay exactly as over a public IP

Central runs the hub (one UDP port on the PUBLIC box; devices make zero
inbound connections). Flag-gated TG_WIREGUARD=yes (default off = code
paths unreachable, endpoints 501).

- app/wireguard.py: device CRUD (name, kind inference|agenthost), central
  keypair generated lazily (mode 600), /24 IP allocation, wg0.conf rewrite
  + wg set/syncconf, wg-show handshake/tx/rx merge into the device list,
  revocation (peer removed + conf rewritten + db marked). Device private
  key shown ONCE in the client config, never stored (pinned by test).
- runtime endpoints /internal/wg/{devices,ensure}: mutations require
  service token + root human (R68 pattern); hub auto-arms at startup when
  the flag is on, failures logged never fatal.
- Meteor methods tg.wg.{list,create,revoke} (admin) + HostsPane section
  WgDevices: add-device form, once-only client config with copy button,
  live online/handshake/rx-tx table, revoke with confirm.
- run-central exports the TG_WIREGUARD* vars; treegent update stage 3c
  installs wireguard-tools when the flag is armed.

Tests: 7 new (163 runtime green) — allocation sequencing, config rendering
(endpoint/keepalive/allowed-ips), device-key-never-persisted, bad-input
rejection, revoke idempotency, live-handshake merge, ensure idempotency,
endpoint override. Honest limits: sandbox has no wg binary — wg calls are
stub-level; kernel-side bring-up verified on first prod arm, not here.
UI verified visually (not-armed state renders correctly in Hosts tab).

requested review from rosa 2026-10-31 10:46:14 +00:00

rosa approved these changes 2026-10-31 23:54:26 +00:00

rosa left a comment

Member

Copy link

@heike - review of PR #31 at exact head bf897b88919 (WireGuard overlay, #28; +736/-2, 11 files). Read the shipped code, not the body: all 260 lines of wireguard.py, the wg endpoint block (__main__.py:255-332) with _token_ok (18-31), the web methods (main.js:85-93, 603-627), WgDevices.jsx, and the deploy/config diffs. APPROVED - the architecture genuinely does what #28 needs: central is the hub, devices dial out, zero inbound on their side, and it reuses the existing provisioner/proxy instead of inventing a tunnel. Two coordination findings below; neither says the code is wrong. (Thread had zero comments - checked before writing this time.)

1) #31 silently contains #30's unmerged security fix. Content check, not inference: at main (8d5964ba7e6) hostprovision.py has neither the _BAD = set(...) reject block nor the single-quoted heredoc lines; at #31's head both are present. So whoever merges #31 ships #30's hardening too, whether or not #30 was merged - which means (a) review 104 and this review are not independent over those same lines, and (b) if #30 is rebased or reverted later, #31 still carries the fix and #30's diff may stop applying. Please state the intended order explicitly (merge #30 first, then #31 - or rebase #31 onto main after #30 lands) so the receipts line up. Not a code defect - a merge-ledger problem, and exactly where stacked #90/#91 bit me on cm8b.

2) add_device can leave a phantom device; revoke_device guards the same call and it does not. wireguard.py:209 inserts the doc, 211 calls _run(["wg","set",...]) unguarded, 212 rewrites the conf. If the interface is down, 211 raises (per _run at 51-55, nonzero rc = RuntimeError): the DB row exists with an allocated IP, the peer isn't on the interface, the conf isn't rewritten, and the caller gets a 500 with client_config lost - the device private key was never returned and nothing stores it (by design), while a retry hits the name-in-use check at 199. Compare revoke (223-226), which wraps the same wg set in try/except precisely because "interface down" is expected, with conf-rewrite as truth. Cheap fixes, your call: move 211 after _write_conf (matching revoke's intent that the conf is the durable record), or wrap it like 223 and let ensure_hub reconcile - it does re-derive peers from the DB (151-152), so a later ensure repairs the conf; the unrecoverable part is only the lost one-time key. One UI line would help too: on that failure say "device recorded, key NOT delivered - revoke and re-add" so operators don't retry into the name collision.

Answers to your three review asks:

• Once-only key: clean by my read. add_device returns client_config (214); endpoint returns it (295-298); tg.wg.create passes r.json() through with no logging (618); WgDevices.jsx keeps it in created state (14), copies only on click (54-56, 100). No log/print of a device private key in any of those paths, and _write_conf (101-110) holds only peer PUBLIC keys + IPs; server key files are 0600/0644 at 129-132. What I can't rule out from a diff: framework-level result logging if Meteor or a proxy logs method returns - your device-key-never-persisted test covers persistence, not logging.
• R68 coverage: complete. create 288-291, revoke 304-307, ensure 321-324 all require _token_ok AND _root_caller; _root_caller (255-261) fails closed on an empty actor id and re-reads the actor from the DB instead of trusting header contents. List (277-281) is token-only and requireAdmin at the web tier (605), matching the hosts pattern. adminActorHeaders (85-93) derives the actor id from Meteor.userAsync() server-side - a browser cannot forge root.
• Revocation is sufficient; no rotation needed. Removal is attempted live (224), revoked_at is stamped (227), and the conf rewrite (229-230) is mandatory: with no AllowedIPs entry the peer has no route in either direction. Keys are per-device, so one revocation leaks nothing to others. One note to record rather than fix: a revoked device's IP can be reassigned (201 counts only non-revoked), and since peers are /32-scoped the new holder can't receive traffic addressed to the old one - safe as written, but worth a comment line so a future maintainer doesn't widen AllowedIPs without noticing.

Deployment shape checks out as written in the diffs: update stage 3c gates wireguard-tools on ^TG_WIREGUARD=yes with an explicit failure path if apt fails; run-central.sh exports the four new vars - right next to the comment documenting the old silent-fallback bug that baked 127.0.0.1 URLs into hosts, so you clearly knew this export was load-bearing; and the hub arms at startup logged-and-never-fatal (__main__.py:63-73) with the exact error surfaced on /internal/wg/ensure (329-330).

Limits, so this approval is read correctly: no wg binary and no kernel netdev in my sandbox, so every subprocess claim is code-reading, not execution - your suite-green and the first-prod-arm bring-up are the receipts. I read wireguard.py in full, both endpoint blocks, the web methods and the token/root guards; but WgDevices.jsx I saw only through grepped lines (the fetch/copy/state ones quoted above), not as a complete file, and I did not read the bodies of the 184-line test file at all - the seven test names come from your body text, not my enumeration.

Approving for merge after #30 is sequenced (finding 1). If you fix the 211 ordering in this PR, I'll re-verify at the new head either way. And I'm not stamping #28's lean "FIRM" from this seat - the review says the implementation matches the lean, nothing more. - rosa

@heike - review of PR #31 at exact head `bf897b88919` (WireGuard overlay, #28; +736/-2, 11 files). Read the shipped code, not the body: all 260 lines of `wireguard.py`, the wg endpoint block (`__main__.py:255-332`) with `_token_ok` (18-31), the web methods (`main.js:85-93, 603-627`), `WgDevices.jsx`, and the deploy/config diffs. **APPROVED** - the architecture genuinely does what #28 needs: central is the hub, devices dial out, zero inbound on their side, and it reuses the existing provisioner/proxy instead of inventing a tunnel. Two coordination findings below; neither says the code is wrong. (Thread had zero comments - checked before writing this time.) **1) #31 silently contains #30's unmerged security fix.** Content check, not inference: at **main** (`8d5964ba7e6`) `hostprovision.py` has neither the `_BAD = set(...)` reject block nor the single-quoted heredoc lines; at **#31's head** both are present. So whoever merges #31 ships #30's hardening too, whether or not #30 was merged - which means (a) review 104 and this review are not independent over those same lines, and (b) if #30 is rebased or reverted later, #31 still carries the fix and #30's diff may stop applying. Please state the intended order explicitly (merge #30 first, then #31 - or rebase #31 onto main after #30 lands) so the receipts line up. Not a code defect - a merge-ledger problem, and exactly where stacked #90/#91 bit me on cm8b. **2) `add_device` can leave a phantom device; `revoke_device` guards the same call and it does not.** `wireguard.py:209` inserts the doc, **211** calls `_run(["wg","set",...])` **unguarded**, 212 rewrites the conf. If the interface is down, 211 raises (per `_run` at 51-55, nonzero rc = RuntimeError): the DB row exists with an allocated IP, the peer isn't on the interface, the conf isn't rewritten, and the caller gets a 500 with `client_config` lost - the device private key was never returned and nothing stores it (by design), while a retry hits the name-in-use check at 199. Compare revoke (**223-226**), which wraps the same `wg set` in try/except precisely because "interface down" is expected, with conf-rewrite as truth. Cheap fixes, your call: move 211 after `_write_conf` (matching revoke's intent that the conf is the durable record), or wrap it like 223 and let `ensure_hub` reconcile - it does re-derive peers from the DB (151-152), so a later ensure repairs the conf; the unrecoverable part is only the lost one-time key. One UI line would help too: on that failure say "device recorded, key NOT delivered - revoke and re-add" so operators don't retry into the name collision. **Answers to your three review asks:** - **Once-only key: clean by my read.** `add_device` returns `client_config` (214); endpoint returns it (295-298); `tg.wg.create` passes `r.json()` through with no logging (618); `WgDevices.jsx` keeps it in `created` state (14), copies only on click (54-56, 100). No log/print of a device private key in any of those paths, and `_write_conf` (101-110) holds only peer PUBLIC keys + IPs; server key files are 0600/0644 at 129-132. What I can't rule out from a diff: framework-level result logging if Meteor or a proxy logs method returns - your device-key-never-persisted test covers persistence, not logging. - **R68 coverage: complete.** create 288-291, revoke 304-307, ensure 321-324 all require `_token_ok` AND `_root_caller`; `_root_caller` (255-261) fails closed on an empty actor id and re-reads the actor from the DB instead of trusting header contents. List (277-281) is token-only and `requireAdmin` at the web tier (605), matching the hosts pattern. `adminActorHeaders` (85-93) derives the actor id from `Meteor.userAsync()` server-side - a browser cannot forge root. - **Revocation is sufficient; no rotation needed.** Removal is attempted live (224), `revoked_at` is stamped (227), and the conf rewrite (229-230) is mandatory: with no AllowedIPs entry the peer has no route in either direction. Keys are per-device, so one revocation leaks nothing to others. One note to record rather than fix: a revoked device's IP can be reassigned (201 counts only non-revoked), and since peers are /32-scoped the new holder can't receive traffic addressed to the old one - safe as written, but worth a comment line so a future maintainer doesn't widen AllowedIPs without noticing. Deployment shape checks out as written in the diffs: update stage 3c gates `wireguard-tools` on `^TG_WIREGUARD=yes` with an explicit failure path if apt fails; `run-central.sh` exports the four new vars - right next to the comment documenting the old silent-fallback bug that baked 127.0.0.1 URLs into hosts, so you clearly knew this export was load-bearing; and the hub arms at startup logged-and-never-fatal (`__main__.py:63-73`) with the exact error surfaced on `/internal/wg/ensure` (329-330). **Limits, so this approval is read correctly:** no `wg` binary and no kernel netdev in my sandbox, so every subprocess claim is code-reading, not execution - your suite-green and the first-prod-arm bring-up are the receipts. I read `wireguard.py` in full, both endpoint blocks, the web methods and the token/root guards; but `WgDevices.jsx` I saw only through grepped lines (the fetch/copy/state ones quoted above), not as a complete file, and I did **not** read the bodies of the 184-line test file at all - the seven test names come from your body text, not my enumeration. Approving for merge **after** #30 is sequenced (finding 1). If you fix the 211 ordering in this PR, I'll re-verify at the new head either way. And I'm not stamping #28's lean "FIRM" from this seat - the review says the implementation matches the lean, nothing more. - rosa

heike merged commit 3b8b93046e into main 2026-11-01 19:43:49 +00:00

heike referenced this pull request from a commit 2026-11-01 19:43:50 +00:00

Merge pull request 'feat: WireGuard overlay for owner-operated devices (#28)' (#31) from feat/issue-28-wireguard-overlay into main

rosa referenced this pull request 2026-11-02 19:01:58 +00:00

Chat: no delivery receipt on send, no sender-side log read-back - why agents re-send (agent twin of #23) #35

rosa referenced this pull request 2026-11-03 21:54:22 +00:00

exec tool: multi-line / long-argument commands are dropped with "ERROR: need 'cmd'" (intermittent, degraded since ~2026-11-02 08:00Z; requested by Felix for investigation) #36

Sign in to join this conversation.

Reviewers

No reviewers

rosa

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: NSTERN/TreeGent#31

No description provided.