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

docs/configuration.md

@@ -41,7 +41,7 @@ Tailscale IP, a VPN address, a bastion.
 | `server.named_token_ttl` | Lifetime of named API tokens, in seconds. Default `2592000` (30 days); `0` means never expires, revoke-only. OAuth access tokens and dashboard session bearers keep their fixed 24 h TTL either way. Env override: `BEACONMCP_NAMED_TOKEN_TTL`. |
 | `server.refresh_token_ttl` | Idle lifetime of OAuth refresh tokens (issued to `authorization_code` clients such as claude.ai, ChatGPT or Le Chat), in seconds. Default and maximum `86400` (24 h); `0` is rejected. Every refresh rotates the token and restarts this clock, but no sign-in outlives 24 h: every refresh token of a sign-in expires 24 h after the authorization, whatever this value says, and the client goes back through the TOTP page. A shorter value only adds an idle timeout inside that day. Env override: `BEACONMCP_REFRESH_TOKEN_TTL`. |
 | `server.audit_log` | JSON-lines audit log covering tool calls, dashboard logins, OAuth authorize and client revokes. Created owner-only (0600). Default `/opt/beaconmcp/audit.log`; `-` keeps stderr only. Env override: `BEACONMCP_AUDIT_LOG`. |
-| `server.transfers_max_mb` | Size cap for `proxmox_upload_file` / `proxmox_download_file` / `proxmox_upload_link` / `proxmox_fetch_url`. Default 500. |
+| `server.transfers_max_mb` | Size cap for `proxmox_upload_file` / `proxmox_download_file` / `proxmox_upload_link` / `proxmox_fetch_url`. Default 500. Behind Cloudflare, an upload link request body is also capped by Cloudflare (100 MB on the free plan, 413 from Cloudflare above that); `proxmox_fetch_url` is not affected. |
 
 ## Proxmox
 

docs/tools.md

@@ -123,7 +123,7 @@ The token in the URL is the credential, so the route needs no bearer. A link is
 node, VMID and path chosen when it was minted, works once, expires after `expires_in` seconds
 (default 600, at most 3600) and is forgotten on restart. The base of the URL is
 `features.dashboard.public_url` when set, otherwise the address the MCP request came in on.
-Behind Cloudflare, uploads are limited to 100 MB per request on the free plan.
+Behind Cloudflare, uploads are limited to 100 MB per request on the free plan. The link needs a running guest: minting one for a stopped VM/CT is refused, and an upload to a guest stopped since then answers 409.
 
 ## SSH (2)
 

docs/updates.md

@@ -36,7 +36,7 @@ Detection is not a guess about how you *should* have installed it:
 
 | Detected | What you are told |
 |----------|-------------------|
-| git checkout | `cd <root>` → `git pull --ff-only` (release channel: `git fetch --tags origin` → `git merge --ff-only <tag>`) → `<venv>/bin/pip install -e . -c constraints.txt` (the real venv path, when there is one) → `systemctl restart beaconmcp` if a unit file exists |
+| git checkout | `cd <root>` → `git pull --ff-only` (release channel: `git fetch --tags origin` → `git merge --ff-only <tag>`; the tool itself mirrors the tags into `refs/beaconmcp/tags/`, so a re-pointed upstream tag never blocks it) → `<venv>/bin/pip install -e . -c constraints.txt` (the real venv path, when there is one) → `systemctl restart beaconmcp` if a unit file exists |
 | container | `docker compose pull` → `docker compose up -d` |
 | pip distribution | `pip install --upgrade 'beaconmcp @ git+https://github.com/Showdown76py/BeaconMCP.git'` |
 | unknown | Re-run `deploy/install.sh` from a checkout |
@@ -79,6 +79,8 @@ In order, stopping at the first failure:
    *actual* configuration, imports every module, builds the HTTP app and answers `/health`, against
    scratch copies of the token store and dashboard database.
 7. **Restart**: `systemctl restart`, deferred a few seconds so the response reaches you first.
+   Only when the server actually runs under the unit: a `beaconmcp serve` started by hand is told
+   to restart itself instead.
 
 Step 6 is a hard gate before the restart. If the new revision cannot start with your config (a
 setting was renamed, a new one is required, an import is broken) the checkout is reset to exactly
@@ -92,7 +94,9 @@ breaks at runtime). So the restart itself is done by a watchdog, started as its
 with `systemd-run` (a plain child would be killed with the service's cgroup). It:
 
 1. restarts the service;
-2. waits up to 90 seconds for `/health` to answer, and to still answer 10 seconds later;
+2. waits up to 90 seconds for `/health` to answer, then requires it to keep answering for 30
+   seconds with the same main PID and restart count (a process that crashes and is restarted by
+   systemd every few seconds can answer every single probe);
 3. otherwise runs `git reset --hard <previous revision>`, reinstalls the previous pinned
    dependencies and restarts again.
 
@@ -126,8 +130,13 @@ features:
 - `require_signed: true` makes `git verify-tag` (release) or `git verify-commit` (branch) a hard
   gate. Git checks the signature against the keys the service user trusts: import the maintainers'
   GPG keys, or for SSH signatures set `gpg.ssh.allowedSignersFile` in that user's git config.
+  **Every** public key in that user's keyring is accepted, trusted or not, so keep it to the
+  maintainers' keys (or point `GNUPGHOME` at a dedicated keyring in the unit). A release tag must
+  also carry its own name: a signed `v2.1.0-rc1` republished as `v2.1.0` is refused.
   On the branch channel this needs a signed tip commit; merges made through GitHub's web UI are
-  signed by GitHub's key, which you then have to trust.
+  signed by GitHub's key, which you then have to trust. That key signs every web-UI commit of every
+  GitHub user, so on the branch channel the gate only proves the commit went through github.com:
+  it does not stop anyone who can push or merge to the branch.
 
 Only one update runs at a time. The dashboard button and the MCP tool reach the same code, and two
 `git pull` / `pip install` runs in one checkout would fight over `index.lock`. A second caller is

src/beaconmcp/__main__.py

@@ -1212,6 +1212,10 @@ def _run_http(mcp, host: str, port: int, *, serve: bool = True):
     )
     from .ratelimit import RateLimiter, TotpLockout, client_ip, forwarded_host
     from .server import config, transport_security
+    # The same list the SDK enforces for CORS, including its built-in default
+    # when beaconmcp.yaml leaves server.allowed_origins empty: OAuth
+    # redirects to claude.ai must keep working out of the box.
+    redirect_origins = list(transport_security.allowed_origins or [])
 
     from .metrics import REGISTRY, auth_events, http_requests
 
@@ -1343,9 +1347,24 @@ async def dispatch(self, request: Request, call_next):
     # client_id alone, anyone who knew a client_id could lock its owner out
     # of every 2FA path.
     totp_lockout = TotpLockout(per_source=5, per_client=10, lockout_seconds=300)
-    totp_locked = totp_lockout.locked
-    totp_record_failure = totp_lockout.record_failure
-    totp_record_success = totp_lockout.record_success
+
+    # A derived (DCR) client checks codes against its owner's seed, so the
+    # lockout is keyed on the seed owner: otherwise every derived client_id
+    # would get its own guess budget against the same seed.
+    def _seed_owner(client_id: str) -> str:
+        client = client_store.get(client_id)
+        if client is not None and client.owner_client_id:
+            return client.owner_client_id
+        return client_id
+
+    def totp_locked(client_id: str, ip: str | None = None) -> bool:
+        return totp_lockout.locked(_seed_owner(client_id), ip)
+
+    def totp_record_failure(client_id: str, ip: str | None = None) -> None:
+        totp_lockout.record_failure(_seed_owner(client_id), ip)
+
+    def totp_record_success(client_id: str, ip: str | None = None) -> None:
+        totp_lockout.record_success(_seed_owner(client_id), ip)
 
     def _ip(request: Request) -> str:
         return client_ip(request, tuple(config.server.trusted_proxies))
@@ -1409,7 +1428,7 @@ def _validate_authorize_params(
         # allowlist. Prevents authorization-code exfiltration via a
         # typo-squat or attacker-controlled client that somehow got a
         # valid client_id.
-        if not auth.is_trusted_redirect_uri(redirect_uri, config.server.allowed_origins):
+        if not auth.is_trusted_redirect_uri(redirect_uri, redirect_origins):
             return {}, JSONResponse(
                 {"error": "invalid_request",
                  "error_description": (
@@ -2265,7 +2284,7 @@ async def dcr_register(request: Request) -> Response:
                 status_code=400,
             )
         bad = [u for u in redirect_uris_raw
-               if not auth.is_trusted_redirect_uri(u, config.server.allowed_origins)]
+               if not auth.is_trusted_redirect_uri(u, redirect_origins)]
         if bad:
             return JSONResponse(
                 {"error": "invalid_redirect_uri",

src/beaconmcp/auth.py

@@ -992,6 +992,17 @@ def refresh(self, refresh_token: str, client_id: str) -> OAuthGrant:
             rt = self._refresh_tokens.get(_hash_secret(refresh_token))
             if rt is None or rt.client_id != client_id:
                 raise RefreshTokenError("invalid")
+            # Reuse is checked before the token's own idle expiry: a
+            # rotated-out token replayed after its idle TTL is still a
+            # stolen copy, and must still take the family down.
+            if rt.used and now < rt.family_expires_at:
+                if (
+                    rt.replay is not None
+                    and now - rt.used_at <= self.REFRESH_REUSE_GRACE
+                ):
+                    return rt.replay
+                self.revoke_family(rt.family_id)
+                raise RefreshTokenError("reused", family_id=rt.family_id)
             if now > rt.expires_at or now >= rt.family_expires_at:
                 # The whole sign-in is over with the family end; a token
                 # that idled past its own TTL only takes itself out.
@@ -1000,16 +1011,12 @@ def refresh(self, refresh_token: str, client_id: str) -> OAuthGrant:
                 else:
                     self._drop_refresh(rt.token_hash)
                 raise RefreshTokenError("expired", family_id=rt.family_id)
-            if rt.used:
-                if (
-                    rt.replay is not None
-                    and now - rt.used_at <= self.REFRESH_REUSE_GRACE
-                ):
-                    return rt.replay
-                self.revoke_family(rt.family_id)
-                raise RefreshTokenError("reused", family_id=rt.family_id)
             rt.used = True
             rt.used_at = now
+            # Keep the rotated-out token until the family ends (not its own
+            # idle expiry, shorter with a custom refresh_token_ttl) so a
+            # replay is still recognised as reuse.
+            rt.expires_at = rt.family_expires_at
             self._persist_refresh(rt)
             grant = self._mint_oauth(client_id, rt.family_id, rt.family_expires_at)
             rt.replay = grant

src/beaconmcp/dashboard/mcp_bridge.py

@@ -25,7 +25,6 @@
 import httpx2
 from mcp.client.session import ClientSession
 from mcp.client.streamable_http import streamable_http_client
-from pydantic import AnyUrl
 
 # Extension identifier and content type from the MCP Apps spec (SEP-1865).
 UI_EXTENSION_ID = "io.modelcontextprotocol/ui"
@@ -144,7 +143,8 @@ async def read_ui_resource(session: ClientSession, uri: str) -> str:
     if not uri.startswith("ui://"):
         raise UiResourceError("not a ui:// resource")
     try:
-        result = await session.read_resource(AnyUrl(uri))
+        # mcp 2.x takes a plain str: an AnyUrl fails request validation.
+        result = await session.read_resource(uri)
     except Exception as exc:  # noqa: BLE001
         raise UiResourceError(str(exc)) from exc
 

src/beaconmcp/proxmox/client.py

@@ -10,7 +10,8 @@
 
 from proxmoxer import ProxmoxAPI
 from requests.adapters import HTTPAdapter
-from requests.exceptions import ConnectionError, Timeout
+from requests.exceptions import ConnectionError, ConnectTimeout, Timeout
+from urllib3.exceptions import ConnectTimeoutError, NewConnectionError
 
 from ..config import Config
 
@@ -98,6 +99,14 @@ def _trust_ca_bundle(conn: ProxmoxAPI, ca_path: str) -> None:
     session.mount("https://", _CABundleAdapter(ca_path))
 
 
+def _never_sent(exc: Exception) -> bool:
+    """True when the TCP connection itself failed, so no request went out."""
+    if isinstance(exc, ConnectTimeout):
+        return True
+    reason = getattr(exc.args[0], "reason", None) if exc.args else None
+    return isinstance(reason, (NewConnectionError, ConnectTimeoutError))
+
+
 class ProxmoxClient:
     """Manages connections to one or more Proxmox VE nodes via API tokens."""
 
@@ -166,6 +175,17 @@ def api_call(self, node_name: str, method: str, path: str, **kwargs: Any) -> Any
                 raise
             except (ConnectionError, Timeout) as e:
                 last_exc = e
+                # A GET is safe to send twice. Anything else is retried only
+                # when the request provably never reached the node: a read
+                # timeout on vm_clone or snapshot_create may mean the task is
+                # already running, and a second POST would start it again.
+                if method != "get" and not _never_sent(e):
+                    self._cache().pop(node_name, None)
+                    return {
+                        "error": f"Node '{node_name}' did not answer in time: {e}. "
+                        "The request may still have been applied; check "
+                        "proxmox_get_tasks before retrying."
+                    }
                 # Drop this thread's cached connection so the retry rebuilds
                 # TLS state rather than re-using a half-broken socket. Other
                 # threads keep their own healthy sessions.