fix(agent): close auto-update TLS bypass (MITM -> RCE) [HIGH]

The auto-update path built both reqwest clients with an unconditional
danger_accept_invalid_certs(true), so a network MITM could serve an arbitrary
update .exe (checksum is no defense — same unverified channel) and gain RCE on
every managed endpoint. Replace with dev_insecure_tls() = cfg!(debug_assertions)
&& env GURUCONNECT_DEV_INSECURE_TLS: the cfg gate compiles out of release builds,
so a shipped agent ALWAYS verifies certs; dev keeps a self-signed escape hatch.
Loud warn when the insecure path is taken; verify_checksum kept + documented as
transport-integrity (not tamper) defense; TODO + follow-up for embedded-key
update signing (defense-in-depth). Release-invariant unit test added.
cargo fmt/clippy(-D warnings)/test green on GURU-5070 (90 tests). Closes the
2026-05-30 security-audit HIGH (reports/2026-05-30-gc-audit.md).

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

agent/src/update.rs

@@ -10,6 +10,25 @@ use tracing::{error, info, warn};
 
 use crate::build_info;
 
+/// Whether to disable TLS certificate verification for update traffic.
+///
+/// Returns `true` ONLY in a debug build (`cfg!(debug_assertions)`) when the
+/// `GURUCONNECT_DEV_INSECURE_TLS` environment variable is set. The `cfg!` gate
+/// is compiled out of release builds, so a shipped agent ALWAYS verifies certs
+/// regardless of environment — a MITM cannot serve a forged update binary over
+/// an unverified channel. The env var lets a developer test against a
+/// self-signed server without weakening production.
+fn dev_insecure_tls() -> bool {
+    if cfg!(debug_assertions) && std::env::var("GURUCONNECT_DEV_INSECURE_TLS").is_ok() {
+        warn!(
+            "TLS certificate verification DISABLED (dev-insecure mode) — DO NOT use in production"
+        );
+        true
+    } else {
+        false
+    }
+}
+
 /// Version information from the server
 #[derive(Debug, Clone, serde::Deserialize)]
 pub struct VersionInfo {
@@ -42,7 +61,7 @@ pub async fn check_for_update(server_base_url: &str) -> Result<Option<VersionInf
     info!("Checking for updates at {}", url);
 
     let client = reqwest::Client::builder()
-        .danger_accept_invalid_certs(true) // For self-signed certs in dev
+        .danger_accept_invalid_certs(dev_insecure_tls())
         .build()?;
 
     let response = client
@@ -108,7 +127,7 @@ pub async fn download_update(version_info: &VersionInfo) -> Result<PathBuf> {
     info!("Downloading update from {}", version_info.download_url);
 
     let client = reqwest::Client::builder()
-        .danger_accept_invalid_certs(true)
+        .danger_accept_invalid_certs(dev_insecure_tls())
         .build()?;
 
     let response = client
@@ -134,6 +153,13 @@ pub async fn download_update(version_info: &VersionInfo) -> Result<PathBuf> {
 }
 
 /// Verify downloaded file checksum
+///
+/// NOTE: This is a transport-integrity check (catches truncated/corrupted
+/// downloads), NOT a tamper defense. The expected checksum arrives over the
+/// same channel as the binary, so an attacker who can serve a forged binary
+/// can also serve a matching checksum. Tamper resistance comes from verifying
+/// the TLS certificate of the update server (see `dev_insecure_tls`) and, as a
+/// future hardening step, an embedded-public-key signature over the artifact.
 pub fn verify_checksum(file_path: &PathBuf, expected_sha256: &str) -> Result<bool> {
     info!("Verifying checksum...");
 
@@ -160,6 +186,9 @@ pub fn verify_checksum(file_path: &PathBuf, expected_sha256: &str) -> Result<boo
 /// Perform the actual update installation
 /// This renames the current executable and copies the new one in place
 pub fn install_update(temp_path: &PathBuf) -> Result<PathBuf> {
+    // TODO(security): defense-in-depth — verify an embedded-public-key signature
+    // over the update binary/manifest before install_update; see
+    // reports/2026-05-30-gc-audit.md
     info!("Installing update...");
 
     // Get current executable path
@@ -321,4 +350,31 @@ mod tests {
         assert!(!is_newer_version("0.1.0", "0.2.0"));
         assert!(is_newer_version("0.2.0-abc123", "0.1.0-def456"));
     }
+
+    /// In a release build (`debug_assertions` off), `dev_insecure_tls()` MUST
+    /// return false regardless of the env var — the shipped agent can never
+    /// accept invalid certs. In a debug build, it returns true only when
+    /// `GURUCONNECT_DEV_INSECURE_TLS` is set; we cannot assert the env-var path
+    /// here without mutating process-global state (which would race other
+    /// tests), so we only assert the invariant that holds in the current
+    /// build profile.
+    #[test]
+    fn test_dev_insecure_tls_release_is_always_false() {
+        if !cfg!(debug_assertions) {
+            // Release/test-release profile: must be false no matter the env.
+            assert!(
+                !dev_insecure_tls(),
+                "release build must never disable TLS verification"
+            );
+        } else {
+            // Debug profile: with the env var unset, must still be false.
+            // (We avoid setting it to prevent cross-test interference.)
+            if std::env::var("GURUCONNECT_DEV_INSECURE_TLS").is_err() {
+                assert!(
+                    !dev_insecure_tls(),
+                    "debug build without the env var must verify TLS"
+                );
+            }
+        }
+    }
 }