Repository navigation
feat(transfers): one-shot upload links and fetch-by-URL into a VM/CT - #49
Merged
Merged
Conversation
MCP tool arguments are JSON, so a file could only reach a guest as text
(proxmox_write_file, 1 MB) or by being staged on the node out of band.
- proxmox_upload_link mints a single-use, short-lived URL bound to one
node/vmid/dest. `curl -T <file> <url>` streams the raw bytes to
PUT|POST /transfers/upload/{token}; the server stages them and pushes
them into the guest through the proxmox_upload_file path (SHA-256
checked), then answers with the result. link_id reads the outcome.
- proxmox_fetch_url downloads an http(s) URL on the server and pushes it
into the guest the same way.
- The push half of proxmox_upload_file is now a shared _push_staged().
- The upload route skips bearer auth (the token is the credential) and is
collapsed to one label in /metrics so tokens never land there.
- Both tools need confirmation in the dashboard chat.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01HpjwiAg4NUssQYBz8D8Rbz
Showdown76py
approved these changes
Oct 11, 2026
Showdown76py
left a comment
Owner
There was a problem hiding this comment.
Tested end to end on a real beaconmcp serve (fake PVE over HTTPS, a real SSH/SFTP server with a stub pct, 5 MB transfer cap): 49/49 checks pass, plus pytest 657 and ruff.
What I checked:
- 3 MB binary PUT into an LXC and 2 MB POST into a QEMU guest arrive with matching SHA-256; staging is empty afterwards.
- Single use holds: reuse is 404, 5 concurrent PUTs on one link give exactly one 200, a restart voids pending links, the status view never shows the token.
- Cap enforced with Content-Length and with chunked bodies (413, link spent, no partial file). Multipart 415, empty body 400, TTL clamped to 60 s, unknown token 404.
- A
destof/root/$(touch PWNED);x y.binlands as that literal filename, nothing executed. - No auth bypass through the unauthenticated prefix:
../../mcp,%2e%2e, an extra segment,../../app/tokensand an empty token all return 404. /metricscollapses the route, thetransfer.uploadaudit event carries the IP and not the token.- fetch_url: success with matching SHA, http redirects followed, 404 and connection refused give clean errors,
file://refused even behind a redirect, cap enforced with and without Content-Length. - Both tools are behind the chat confirmation gate and refused by the panel relay.
Non-blocking notes for a follow-up if you want:
- fetch_url can reach loopback and link-local services on the BeaconMCP host (it fetched our own
/metricsinto a guest). Not an escalation givenproxmox_run, but worth a sentence in the docs, or an opt-in block on private ranges. - fetch_url downloads the whole file before checking that the vmid exists; validating the target first would save a pointless download.
- Reading a link's status with
link_idalone also hits the chat confirmation modal; that call is read-only.
Merging.
Generated by Claude Code
Showdown76py
pushed a commit
that referenced
this pull request
Oct 11, 2026
- proxmox_fetch_url refuses URLs that resolve to loopback, link-local, unspecified or multicast addresses, on every redirect hop, so it can no longer read services that only listen on the BeaconMCP host (its own /metrics, the Proxmox API on 127.0.0.1) or a cloud metadata endpoint into a guest. Private LAN addresses stay allowed. - proxmox_fetch_url checks that the VM/CT exists before downloading. - proxmox_upload_link(link_id=...) is a read and no longer opens the dashboard chat's confirmation modal. Claude-Session: https://claude.ai/code/session_01HpjwiAg4NUssQYBz8D8Rbz Co-authored-by: Claude <[email protected]>
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
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.
Why
MCP tool arguments are JSON, so the only way to get a file into a guest was as text (
proxmox_write_file, 1 MB, UTF-8) or by putting it in the staging dir out of band (scp) beforeproxmox_upload_file. A model holding a real file, binary included, had no way to send it.What
proxmox_upload_link(node, vmid, dest)mints a single-use, short-lived URL bound to one target.curl -T <file> <url>streams the raw bytes toPUT|POST /transfers/upload/{token}; the server stages them and pushes them into the guest through the existingproxmox_upload_filepath (SHA-256 checked), then answers with the transfer result.proxmox_upload_link(link_id=...)reads the outcome if the HTTP reply got lost.proxmox_fetch_url(node, vmid, url, dest): the server downloads an http(s) URL and pushes it into the guest the same way.proxmox_upload_fileis now a shared_push_staged(); its behaviour is unchanged.Security
secrets.token_urlsafe(32)) is the credential, so the route skips the bearer middleware, like the slug-scoped DCR endpoints. A token is spent the moment a request presents it, is bound to the node/vmid/dest chosen at mint time, expires afterexpires_in(60-3600 s, default 600), and lives in memory only (a restart voids pending links). At most 50 pending links.server.transfers_max_mb, checked onContent-Lengthand again while streaming. Multipart bodies are refused./metricscollapses the route to a single label, so a token never shows up there.transfer.uploadaudit event.Public URL
The link uses
features.dashboard.public_urlwhen set, otherwise the issuer of the in-flight MCP request (a newtransfers.current_public_baseContextVar set by the auth middleware).Tests
tests/test_upload_links.py(15 tests): store semantics (single use, expiry, TTL clamp, cap, no token in the status view), the full mint → PUT → guest flow with fake Proxmox/SSH clients (binary payload, checksum verified, staging cleaned, reuse 404, outcome readable), the oversize 413, multipart 415 and unknown-token 404 cases, andfetch_url(success, HTTP error, non-http scheme).tests/test_mcp_http_stack.py: on the real app stack the route skips bearer auth, the body streams through bothBaseHTTPMiddlewarelayers, and the token is absent from/metrics.pytest: 657 passed,ruff check src testsclean.sftp+pct_push,checksum_verified: true), reusing the link returned 404,proxmox_fetch_urlpulled a raw GitHub file in, and the staging dir was empty afterwards.Notes
docs/tools.md.🤖 Generated with Claude Code
https://claude.ai/code/session_01HpjwiAg4NUssQYBz8D8Rbz
Generated by Claude Code