fix(secret-rotator): correct proxmox TLS rationale + log DELETE failures

Addresses the advisory review on the proxmox-token mode.

The code claimed the PVE cert is self-signed. It is not — the in-cluster
Service presents a VALID Let's Encrypt cert (verified live: CN=neptune.renner.uno,
issuer Let's Encrypt). Verification fails only on HOSTNAME mismatch: the job
connects to the pve.apps.svc.cluster.local Service (for 3-node failover) while
the cert is for a node FQDN. Corrected the module header and the insecure_agent
comment to say this accurately, and spelled out the real tradeoff — ureq 3 has
no skip-hostname-only option, so it's disable-verification-with-failover (this,
matching the python's prior CERT_NONE) vs pin-to-one-node-FQDN-with-full-verify
-but-SPOF. Noted the deferred improvement (a custom rustls verifier that
validates the LE chain but skips hostname).

Also from the review:
- log a warning when the DELETE result is an Err, so a re-run needn't
  rediscover why a first attempt behaved oddly (still non-fatal — a leftover
  token makes the POST fail loud).
- documented why the raw URL-path interpolation is safe (fixed operator config,
  never untrusted; percent-encoding @ to %40 would break PVE's userid lookup).

No behaviour change to the rotation itself. clippy -D warnings clean; 13 tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

secret-rotator/src/proxmox.rs

@@ -6,11 +6,21 @@
 //! new value. Inherent to Proxmox; acceptable on an off-hours quarterly
 //! schedule.
 //!
-//! TLS verification is DISABLED for these calls: the PVE API is reached via the
-//! in-cluster Service ClusterIP, whose cert never matches
-//! `*.svc.cluster.local`. This is the only mode that turns verification off,
-//! and it is scoped to a dedicated agent used ONLY for the Proxmox calls —
-//! OpenBao still goes over the normal verified/plain-http agent.
+//! TLS verification is DISABLED for these calls — matching the CERT_NONE the
+//! python rotator used, so this preserves the existing posture rather than
+//! changing it. The nodes actually present VALID Let's Encrypt certs (e.g.
+//! CN=neptune.renner.uno); the only reason verification fails is the HOSTNAME
+//! mismatch — the job connects to the pve.apps.svc.cluster.local Service (for
+//! 3-node failover) while the cert is for a node FQDN. ureq 3 offers no
+//! skip-hostname-only option (only all-or-nothing disable_verification), so the
+//! choice is: disable verification and keep failover (this), or pin api-url to
+//! one node FQDN for full verification at the cost of a single-node SPOF.
+//! Disabling is scoped to a dedicated agent used ONLY for the Proxmox calls;
+//! OpenBao still goes over the normal verified/plain-http agent. The path is
+//! pod -> ClusterIP -> node, entirely inside the cluster. A future improvement
+//! (a custom rustls verifier that validates the LE chain but skips hostname)
+//! would restore MITM protection without the SPOF — deferred as it means
+//! dropping ureq for this one call.
 
 use std::error::Error;
 use std::fmt;
@@ -29,7 +39,8 @@ fn err<T>(msg: impl Into<String>) -> Result<T, Box<dyn Error>> {
     Err(Box::new(PveError(msg.into())))
 }
 
-/// An agent that does NOT verify TLS — for the self-signed PVE API only.
+/// An agent that does NOT verify TLS — for the PVE Service endpoint only,
+/// where a valid LE cert fails on hostname mismatch (see the module header).
 pub fn insecure_agent() -> ureq::Agent {
     let tls = ureq::tls::TlsConfig::builder()
         .disable_verification(true)
@@ -55,6 +66,10 @@ pub fn recreate_token(
     user: &str,
     token_id: &str,
 ) -> Result<String, Box<dyn Error>> {
+    // `user` and `token_id` are interpolated raw. They are fixed operator
+    // config (pve-exporter@pve / monitoring), never untrusted input; `@` is a
+    // valid path char that PVE matches literally (percent-encoding it to %40
+    // would break the userid lookup), and neither can contain `/` or `%`.
     let base = format!(
         "{}/api2/json/access/users/{}/token/{}",
         api_url.trim_end_matches('/'),
@@ -63,12 +78,15 @@ pub fn recreate_token(
     );
     let hdr = format!("PVEAPIToken={auth}");
 
-    // DELETE first. Ignore its outcome: if the token doesn't exist yet the
-    // delete is a no-op, and any real auth/permission problem surfaces at the
-    // POST below (where it is fatal). Ignoring it here does NOT mask a broken
-    // rotation — a failed delete that leaves the token in place makes the POST
-    // fail with "already exists", which is loud.
-    let _ = agent.delete(&base).header("Authorization", &hdr).call();
+    // DELETE first. Don't fail on its outcome: if the token doesn't exist yet
+    // the delete is a no-op, and any real auth/permission problem surfaces at
+    // the POST below (where it is fatal). A failed delete that leaves the token
+    // in place makes the POST fail loud with "already exists", so this does not
+    // mask a broken rotation. Log a warning though, so a re-run doesn't have to
+    // rediscover why a first attempt behaved oddly.
+    if let Err(e) = agent.delete(&base).header("Authorization", &hdr).call() {
+        eprintln!("warning: DELETE of token {user}!{token_id} did not succeed (continuing to recreate): {e}");
+    }
 
     // Recreate with privsep=0. Form-encoded body, as the PVE API expects.
     let mut resp = agent