diff --git a/agent/src/update.rs b/agent/src/update.rs index 312af41..cbcd814 100644 --- a/agent/src/update.rs +++ b/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 Result { 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 { } /// 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 { info!("Verifying checksum..."); @@ -160,6 +186,9 @@ pub fn verify_checksum(file_path: &PathBuf, expected_sha256: &str) -> Result Result { + // 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" + ); + } + } + } }