fix: review of today's merged PRs (#40-#51)

[Ailcope]

Review of every PR merged today (#40, #41, #42, #43, #45, #46, #47, #48, #49, #50, #51; #44 shipped inside #48), plus a live test of the transfers on a real PVE 9.2 host behind a Cloudflare tunnel. Every fix below comes with a test that fails on current main and passes here. Full suite: 705 passed, ruff clean.

Transfers (#49, #50)

• Stopped guest: an upload link or fetch into a stopped CT/VM used to fail at push time with a 502, which Cloudflare replaces with its own "error code: 502" page. The guest must now be running when the link is minted and when the file is pushed. The error is clear, and the HTTP status is 409 (stopped guest) or 500, never 502.
• SSRF / DNS rebinding in proxmox_fetch_url: the hostname was checked, then resolved again by httpx at connect time. A custom httpcore network backend now resolves once, refuses local addresses and connects to the address it checked. It fails closed when resolution errors.
• Cleanup: the staged copy on the node is removed even when SFTP fails part way or the request is cancelled. The upload handler always settles the link and writes the audit entry.
• Checksums: a sha256sum that exits non-zero for any reason other than "not installed" (127) is now an error instead of a silent success.
• QEMU: a push to a directory dest is refused with a clear message.
• Public URL: a loopback request base is never used to build the upload URL.
• Docs: notes on Cloudflare's 100 MB request body limit vs transfers_max_mb.

Guest file I/O (pre-existing, confirmed against PVE's Agent.pm)

• proxmox_write_file sent raw text while PVE base64-decodes by default. It now sends encode=0 and caps content at 45 KiB, since PVE limits content to 60 KB. The error points to the upload tools.
• proxmox_read_file in binary mode returned PVE's decoded bytes as if they were base64. It now reads with decode=0 and returns real base64, and falls back on older PVE.

Auth (#40, #42, #45)

• Refresh token reuse is detected even after a short refresh TTL (reuse is checked before expiry, and the rotated token keeps the family expiry).
• The TOTP lockout is keyed on the client that owns the seed, so derived clients share the owner's counter.
• A source that recently passed TOTP is no longer locked out by another source's failures on the same client.
• The untrusted-proxy XFF warning now says which peer to add to server.trusted_proxies.

OAuth (#41)

• With server.allowed_origins left out of beaconmcp.yaml, CORS used the built-in list (claude.ai, ...) but /oauth/authorize and DCR checked the empty YAML value. The Claude connector then got a 400 on a default install. Both checks now use the list the server actually enforces.

SSH TOFU (#46)

• asyncssh retries a non-22 port without the port, so [host]:2222 was checked against the key of host:22, often another machine behind the same NAT address. Keys are now pinned and matched per host and port.

Proxmox client (pre-existing)

• A read timeout on a POST such as vm_clone or snapshot_create was retried and could start the task twice. Writes are now retried only when the TCP connection never opened.
• _prune_exec_sessions iterated a dict that other threads were writing to. It now iterates a snapshot.
• Docstring on the cmd.exe quoting limit for Windows guests (fix(proxmox): run proxmox_run commands through the guest shell on QEMU #43).

Dashboard (#48)

• MCP Apps panels failed to load with the 2.x client (read_resource now takes a str).

Self-update (#47)

• Watchdog settle check never waited in production: systemd expands ${VAR} in a transient unit's ExecStart, script text included, so ${10} reached sh empty. The script no longer contains a dollar-brace or $$, and a test enforces that.
• Crash loops: "healthy" now means /health keeps answering for 30 s with the same MainPID and NRestarts. Two samples could otherwise land on two incarnations of a crash loop.
• The rollback pip install is bounded by timeout 900.
• Release channel: tags are mirrored into refs/beaconmcp/tags/ with --force --prune. Before, one re-pointed upstream tag made every later fetch --tags fail.
• Signed releases: a signed tag must carry its own name. A signed v2.1.0-rc1 republished as v2.1.0 used to pass.
• No restart is scheduled when the server runs outside its systemd unit.
• Docs: every key in the keyring counts, and GitHub's web-flow key proves little on the branch channel.

Not changed

• Host certificates in TOFU mode (low risk).
• The git status index.lock race during a rollback (a window of milliseconds, unverified).

🤖 Generated with Claude Code

https://claude.ai/code/session_01HpjwiAg4NUssQYBz8D8Rbz

---

Generated by Claude Code

[Showdown76py]

LGTM 👍

Went through the src diff and checked the agent file I/O against Agent.pm on a PVE 9.2.10 node: count/decode on file-read, encode defaulting to 1 on file-write and the 60 KiB content cap are all as described, so the double-encoding fix and the 45 KiB limit are right. The refresh-reuse reordering, the seed-owner keyed lockout, the CORS/redirect origin alignment, the systemd ${10} fix and the tag-name check on signed releases look good too. CI green.

Non-blocking nits, fine as a follow-up:

• _agent_read_bytes legacy fallback (proxmox/system.py): the retry drops decode but still sends count. If both params came in the same qemu-server release, an older PVE rejects count too and the fallback can never succeed. Dropping count in the legacy call (and relying on the len(data) check) would make it useful.
• _GuardedNetworkBackend.connect_tcp only connects to addrs[0]. Before, anyio tried every resolved address (happy eyeballs), so a host whose first answer is unreachable (e.g. AAAA on an IPv4-only box) now fails outright. All addresses are already checked, so looping over them on OSError would keep the guarantee.
• Non-GET retry guard (proxmox/client.py): a ConnectionError from a reset on a reused keep-alive socket also lands in the "did not answer in time" branch. Skipping the retry is the right call, but the message is misleading there. Something like "the connection to node X failed mid-request" would be more accurate.
• _fetch_transport sets transport._pool, a private httpx attribute. It's pinned by the tests, but worth a comment pointing at the httpx version it was checked against.

[Ailcope]

Thanks! All four notes are addressed, each with a test, in the follow-up #53. This branch no longer accepts direct pushes, so the follow-up is stacked on this one: merge #52 first, then #53 shows a single commit.

---

Generated by Claude Code

[Showdown76py]

why the fuck are you opening new PRs? Just update this one it isnt merged yet.

[Ailcope]

Sorry for the noise. I tried to push to this branch first, and GitHub refuses it. It just did again:

GH013: Repository rule violations found for refs/heads/c/magical-turing-tqjhj2-giga
- At least 1 approving review is required by reviewers with write access.

So a repository ruleset covers every branch, this one included (https://github.com/Showdown76py/BeaconMCP/rules?ref=refs%2Fheads%2Fc%2Fmagical-turing-tqjhj2-giga), and that is why I opened #53. Two ways out:

• Exclude c/** from that ruleset, or add a bypass for it. I then push the commit (0f598e9) straight here and close fix: review notes on #52 #53.
• Approve fix: review notes on #52 #53, which now targets this branch. I merge it in, and it closes on its own.

---

Generated by Claude Code