phase 7: TOFU SSH + xpc trace/dbg/snap/daemon + tun -R stub - #4
Conversation
… tun -R stub
Closes the remaining deferred items from MASTER.md §10. Each command is
contained and useful in v0 without further user input.
* TOFU SSH host-key (internal/sshlife/ssh.go + tofu_test.go):
Dial() now defaults to TOFUHostKey(~/.xpc/known_hosts) -- writes
<host> <key-type> <base64-key> on first contact, byte-matches on
subsequent contacts, refuses on key change with a clear MITM warning.
4 unit tests cover the four edge cases.
* xpc trace start/stop/pull (internal/cli/trace.go):
Sysinternals procmon.exe wrapper. start /accepteula /quiet /minimized
/backingfile <out> [/runtime N] via the same DETACHED_PROCESS spawn
trick used by xpc bootstrap. stop calls procmon.exe /Terminate via a
python subprocess (no cmd.exe quirks). pull is an alias for xpc cp.
* xpc dbg run/analyze (internal/cli/dbg.go):
One-shot cdb wrappers. run <target> [--command] auto-detects .dmp
files (uses -z) and appends ;q so cdb exits cleanly. analyze is the
shorthand for !analyze -v against a minidump.
* xpc snap list/create/restore/delete (internal/cli/snap.go):
Proxmox PVE HTTP API client at https://<host>:8006/api2/json/ with
PVEAPIToken auth. Profile fields proxmox_host / proxmox_user are
honored as defaults; the secret is expected via --proxmox-token or
$XPC_PROXMOX_TOKEN. --proxmox-{host,user,token,node,vmid,insecure}
flags layer on top.
* xpc daemon start/stop/status/exec (internal/cli/daemon.go):
Long-lived host-side process holding warm TLS+session connections per
profile. IPC over ~/.xpc/run/daemon.sock with one-line JSON requests
and stdout_b64/stderr_b64 chunked responses. Smoke verified end-to-
end: 'xpc daemon exec ver' through the warm session prints
'Microsoft Windows XP [Version 5.1.2600]'. The CLI doesn't auto-route
through it yet; opt-in for now.
* xpc tun -R: stub. Returns a clear 'not yet implemented' error
pointing at TASKS.md. Real reverse forwarding needs an agent->host
tool.invoke primitive; tracked as deferred.
CI fix: drop -coverprofile from go test (covdata tool missing on the
setup-go@v5 runners) -- already merged via PR #3 commit 3d58226.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Free Run ID: 📒 Files selected for processing (9)
Note 🎁 Summarized by CodeRabbit FreeYour organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login. Comment |
![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: fb68223994
ℹ️ 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".
| if err := arcp.WriteFrame(sess.conn, invoke); err != nil { | ||
| // Session likely dead; drop and let the next call reopen. | ||
| d.dropSession(profileName) | ||
| return wrapConnection(err) | ||
| } | ||
|
|
||
| streamChannels := map[string]string{} | ||
| for { | ||
| env, err := arcp.ReadFrame(sess.conn) |
There was a problem hiding this comment.
handleExec writes a request to sess.conn and then immediately starts reading frames from that same shared connection without any per-session lock or correlation filter. Because serve handles each client in its own goroutine, two xpc daemon exec calls against the same profile can run concurrently and interleave WriteFrame/ReadFrame on one socket, causing one caller to consume the other caller’s stream/result or hang waiting for frames that were already read by a different goroutine. This breaks correctness under concurrent daemon use.
Useful? React with 👍 / 👎.
| if idx := strings.LastIndex(hostname, ":"); idx > 0 { | ||
| // Make sure it isn't an IPv6 literal "[::1]:22". | ||
| if !strings.Contains(hostname[:idx], "]") || strings.HasPrefix(hostname, "[") { | ||
| return hostname[:idx] | ||
| } |
There was a problem hiding this comment.
The TOFU canonicalization strips the port unconditionally, so connections to host:22 and host:2222 are treated as the same known-host entry. In environments where multiple SSH endpoints are exposed on different ports of one host/IP, the first key learned will cause later valid connections on another port to be rejected as “host key changed” (or incorrectly matched). The host identity should retain port information for non-default ports (OpenSSH-style [host]:port).
Useful? React with 👍 / 👎.
1 participant
Summary
Closes the remaining deferred items from MASTER.md §10.
internal/sshlife): defaultDial()now usesTOFU against
~/.xpc/known_hosts. First contact writes the entry;subsequent dials byte-match; key changes refuse with a clear
MITM warning. 4 unit tests.
xpc trace start | stop | pull: Sysinternals procmon wrapperwith detached spawn +
procmon.exe /Terminate+xpc cpfor the.pml file.
xpc dbg run | analyze: one-shot cdb wrappers. Auto-detect.dmpfiles (uses-z); append;qso cdb exits cleanly.analyzeis shorthand for
!analyze -v.xpc snap list | create | restore | delete: full Proxmox PVEHTTP API client with
PVEAPITokenauth. Configuration via flags orXPC_PROXMOX_*env vars; profile carriesproxmox_host/proxmox_useras defaults.xpc daemon start | stop | status | exec: long-lived hostprocess holding warm TLS+session connections per profile. Unix-socket
IPC at
~/.xpc/run/daemon.sock; smoke-verified end-to-end (xpc daemon exec verreturned Windows XP version through the warmsession). Opt-in; CLI doesn't auto-route yet.
xpc tun -R: stub. Returns a clear "not yet implemented" errorpointing at the agent->host
tool.invokeprimitive needed to land it.Test plan
go test -race ./...greengolangci-lint runclean (0 issues)xpc daemon start+xpc daemon exec ver+xpc daemon stopxpc trace start --dry-run,xpc dbg analyze --dry-run,xpc snap list(errors cleanly without Proxmox config)🤖 Generated with Claude Code