fix(security): fail closed in pairing.rs::is_authenticated when token is unset (#1919) (#2108)

Co-authored-by: Cursor <cursoragent@cursor.com>

src/openhuman/security/mod.rs

@@ -21,7 +21,10 @@ pub use detect::create_sandbox;
 pub use ops as rpc;
 pub use ops::*;
 #[allow(unused_imports)]
-pub use pairing::PairingGuard;
+pub use pairing::{
+    ensure_core_rpc_token_for_bind, is_public_bind, CoreBindTokenError, PairingGuard,
+    CORE_TOKEN_ENV_VAR,
+};
 pub use policy::validate_path_within_root;
 #[allow(unused_imports)]
 pub use policy::AutonomyLevel;

src/openhuman/security/pairing.rs

@@ -7,9 +7,17 @@
 use parking_lot::Mutex;
 use sha2::{Digest, Sha256};
 use std::collections::HashSet;
+use std::io::Write as _;
+use std::path::Path;
 use std::sync::Arc;
 use std::time::Instant;
 
+#[cfg(unix)]
+use std::os::unix::fs::{OpenOptionsExt as _, PermissionsExt as _};
+
+/// Environment variable for the core JSON-RPC bearer token (see `crate::core::auth`).
+pub const CORE_TOKEN_ENV_VAR: &str = "OPENHUMAN_CORE_TOKEN";
+
 /// Maximum failed pairing attempts before lockout.
 const MAX_PAIR_ATTEMPTS: u32 = 5;
 /// Lockout duration after too many failed pairing attempts.
@@ -156,13 +164,31 @@ impl PairingGuard {
     }
 
     /// Check if a bearer token is valid (compares against stored hashes).
+    ///
+    /// Always fails closed on empty/whitespace tokens. When pairing is not required,
+    /// configured tokens are still honored if present; with no tokens configured,
+    /// every request is rejected.
     pub fn is_authenticated(&self, token: &str) -> bool {
-        if !self.require_pairing {
-            return true;
+        if token.trim().is_empty() {
+            log::debug!("[openhuman:pairing] is_authenticated: rejected empty bearer token");
+            return false;
         }
-        let hashed = hash_token(token);
+
         let tokens = self.paired_tokens.lock();
-        tokens.contains(&hashed)
+        if tokens.is_empty() {
+            log::debug!(
+                "[openhuman:pairing] is_authenticated: no paired tokens configured (require_pairing={})",
+                self.require_pairing
+            );
+            return false;
+        }
+
+        let hashed = hash_token(token);
+        let ok = tokens.contains(&hashed);
+        if !ok {
+            log::debug!("[openhuman:pairing] is_authenticated: bearer token not in paired set");
+        }
+        ok
     }
 
     /// Returns true if pairing is satisfied (has at least one token).
@@ -252,11 +278,113 @@ pub fn constant_time_eq(a: &str, b: &str) -> bool {
 /// Check if a host string represents a non-localhost bind address.
 pub fn is_public_bind(host: &str) -> bool {
     !matches!(
-        host,
+        host.trim(),
         "127.0.0.1" | "localhost" | "::1" | "[::1]" | "0:0:0:0:0:0:0:1"
     )
 }
 
+/// Error while resolving or persisting a core RPC token for a bind address.
+#[derive(Debug, thiserror::Error)]
+pub enum CoreBindTokenError {
+    #[error(
+        "{CORE_TOKEN_ENV_VAR} must not be empty when binding on a non-loopback address ({host})"
+    )]
+    EmptyEnvToken { host: String },
+    #[error("failed to persist core RPC token at {path}: {source}")]
+    Persist {
+        path: String,
+        #[source]
+        source: std::io::Error,
+    },
+}
+
+/// Ensure a non-empty core RPC bearer token exists before binding on `host`.
+///
+/// **Loopback** (`127.0.0.1`, `localhost`, `::1`, …): returns `Ok(None)` when
+/// `env_token` is unset/empty so local dev can rely on other startup paths.
+///
+/// **Non-loopback** (`0.0.0.0`, LAN IPs, …): returns a usable token — either the
+/// trimmed `env_token` or a freshly generated 256-bit value written to
+/// `{workspace_dir}/core.token` (owner-only on Unix), matching the standalone CLI
+/// path in `crate::core::auth::init_rpc_token`.
+pub fn ensure_core_rpc_token_for_bind(
+    host: &str,
+    workspace_dir: &Path,
+    env_token: Option<&str>,
+) -> Result<Option<String>, CoreBindTokenError> {
+    let host = host.trim();
+    if let Some(raw) = env_token {
+        let trimmed = raw.trim();
+        if !trimmed.is_empty() {
+            log::info!(
+                "[openhuman:pairing] core RPC token supplied via {CORE_TOKEN_ENV_VAR} for bind host={host}"
+            );
+            return Ok(Some(trimmed.to_string()));
+        }
+        if is_public_bind(host) {
+            log::error!(
+                "[openhuman:pairing] {CORE_TOKEN_ENV_VAR} is set but empty on public bind host={host}"
+            );
+            return Err(CoreBindTokenError::EmptyEnvToken {
+                host: host.to_string(),
+            });
+        }
+    }
+
+    if !is_public_bind(host) {
+        log::debug!(
+            "[openhuman:pairing] loopback bind host={host}: no {CORE_TOKEN_ENV_VAR} configured"
+        );
+        return Ok(None);
+    }
+
+    let token = generate_core_rpc_token();
+    let token_path = workspace_dir.join("core.token");
+    write_core_token_file(&token_path, &token).map_err(|source| CoreBindTokenError::Persist {
+        path: token_path.display().to_string(),
+        source,
+    })?;
+    log::warn!(
+        "[openhuman:pairing] Public bind on {host} without {CORE_TOKEN_ENV_VAR}: \
+         generated token at {} — set {CORE_TOKEN_ENV_VAR} explicitly for stable deployments",
+        token_path.display()
+    );
+    Ok(Some(token))
+}
+
+/// Generate a 256-bit core RPC bearer token (lowercase hex, no `zc_` prefix).
+fn generate_core_rpc_token() -> String {
+    use rand::RngExt as _;
+    let mut bytes = [0u8; 32];
+    rand::rng().fill(&mut bytes);
+    hex::encode(bytes)
+}
+
+/// Write `token` to `path` with owner-only permissions on Unix (`0o600`).
+fn write_core_token_file(path: &Path, token: &str) -> std::io::Result<()> {
+    if let Some(parent) = path.parent() {
+        std::fs::create_dir_all(parent)?;
+    }
+
+    #[cfg(unix)]
+    {
+        let mut file = std::fs::OpenOptions::new()
+            .write(true)
+            .create(true)
+            .truncate(true)
+            .mode(0o600)
+            .open(path)?;
+        file.write_all(token.as_bytes())?;
+    }
+
+    #[cfg(not(unix))]
+    {
+        std::fs::write(path, token)?;
+    }
+
+    Ok(())
+}
+
 #[cfg(test)]
 #[path = "pairing_tests.rs"]
 mod tests;

src/openhuman/security/pairing_tests.rs

@@ -70,10 +70,29 @@ async fn is_authenticated_with_invalid_token() {
 }
 
 #[test]
-async fn is_authenticated_when_pairing_disabled() {
+async fn empty_token_rejected_when_require_pairing_false() {
     let (guard, _) = PairingGuard::new(false, &[]);
-    assert!(guard.is_authenticated("anything"));
-    assert!(guard.is_authenticated(""));
+    assert!(!guard.is_authenticated(""));
+    assert!(!guard.is_authenticated("   "));
+}
+
+#[test]
+async fn empty_token_rejected_when_require_pairing_true() {
+    let (guard, _) = PairingGuard::new(true, &["zc_valid".into()]);
+    assert!(!guard.is_authenticated(""));
+}
+
+#[test]
+async fn is_authenticated_rejects_unknown_when_pairing_disabled_and_no_tokens() {
+    let (guard, _) = PairingGuard::new(false, &[]);
+    assert!(!guard.is_authenticated("anything"));
+}
+
+#[test]
+async fn is_authenticated_honors_configured_tokens_when_pairing_disabled() {
+    let (guard, _) = PairingGuard::new(false, &["zc_dev".into()]);
+    assert!(guard.is_authenticated("zc_dev"));
+    assert!(!guard.is_authenticated("zc_other"));
 }
 
 #[test]
@@ -146,6 +165,54 @@ async fn real_ip_is_public() {
     assert!(is_public_bind("10.0.0.1"));
 }
 
+// ── Core RPC bind token ──────────────────────────────────
+
+#[test]
+async fn public_bind_without_env_token_auto_generates() {
+    let tmp = std::env::temp_dir().join(format!("pairing-bind-test-{}", std::process::id()));
+    std::fs::create_dir_all(&tmp).unwrap();
+    let token = ensure_core_rpc_token_for_bind("0.0.0.0", &tmp, None)
+        .expect("public bind should auto-generate a token")
+        .expect("public bind should return Some(token)");
+    assert_eq!(token.len(), 64);
+    assert!(token.chars().all(|c| c.is_ascii_hexdigit()));
+    let from_disk = std::fs::read_to_string(tmp.join("core.token")).unwrap();
+    assert_eq!(from_disk, token);
+    std::fs::remove_dir_all(&tmp).ok();
+}
+
+#[test]
+async fn public_bind_rejects_explicit_empty_env_token() {
+    let tmp = std::env::temp_dir().join(format!("pairing-bind-empty-{}", std::process::id()));
+    std::fs::create_dir_all(&tmp).unwrap();
+    let err = ensure_core_rpc_token_for_bind("0.0.0.0", &tmp, Some("   "))
+        .expect_err("empty env token on public bind must fail");
+    assert!(matches!(err, CoreBindTokenError::EmptyEnvToken { .. }));
+    std::fs::remove_dir_all(&tmp).ok();
+}
+
+#[test]
+async fn loopback_bind_without_env_token_returns_none() {
+    let tmp = std::env::temp_dir().join(format!("pairing-bind-loopback-{}", std::process::id()));
+    std::fs::create_dir_all(&tmp).unwrap();
+    let token = ensure_core_rpc_token_for_bind("127.0.0.1", &tmp, None).unwrap();
+    assert!(token.is_none());
+    assert!(!tmp.join("core.token").exists());
+    std::fs::remove_dir_all(&tmp).ok();
+}
+
+#[test]
+async fn public_bind_uses_nonempty_env_token_without_writing_file() {
+    let tmp = std::env::temp_dir().join(format!("pairing-bind-env-token-{}", std::process::id()));
+    std::fs::create_dir_all(&tmp).unwrap();
+    let token = ensure_core_rpc_token_for_bind("0.0.0.0", &tmp, Some("  abc123  "))
+        .unwrap()
+        .expect("env token should be returned");
+    assert_eq!(token, "abc123");
+    assert!(!tmp.join("core.token").exists());
+    std::fs::remove_dir_all(&tmp).ok();
+}
+
 // ── constant_time_eq ─────────────────────────────────────
 
 #[test]