From d5cd65bc4f84de47835a838f66afec8709157699 Mon Sep 17 00:00:00 2001 From: Chummy Date: Wed, 25 Feb 2026 18:11:17 +0800 Subject: [PATCH] hardening: tighten gateway auth and secret lifecycle handling --- src/config/schema.rs | 669 ++++++++++++++++++++++++++++++++++++++-- src/gateway/mod.rs | 208 ++++++++++++- src/providers/mod.rs | 20 +- src/security/otp.rs | 65 +++- src/security/pairing.rs | 10 +- src/security/secrets.rs | 41 ++- 6 files changed, 960 insertions(+), 53 deletions(-) diff --git a/src/config/schema.rs b/src/config/schema.rs index eecf15adc..b6ff23e46 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -86,7 +86,7 @@ impl ProviderApiMode { /// Top-level ZeroClaw configuration, loaded from `config.toml`. /// /// Resolution order: `ZEROCLAW_WORKSPACE` env → `active_workspace.toml` marker → `~/.zeroclaw/config.toml`. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Clone, Serialize, Deserialize, JsonSchema)] pub struct Config { /// Workspace directory - computed from home, not serialized #[serde(skip)] @@ -293,7 +293,7 @@ pub struct ProviderConfig { // ── Delegate Agents ────────────────────────────────────────────── /// Configuration for a delegate sub-agent used by the `delegate` tool. -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] +#[derive(Clone, Serialize, Deserialize, JsonSchema)] pub struct DelegateAgentConfig { /// Provider name (e.g. "ollama", "openrouter", "anthropic") pub provider: String, @@ -330,6 +330,79 @@ fn default_max_tool_iterations() -> usize { 10 } +impl std::fmt::Debug for DelegateAgentConfig { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("DelegateAgentConfig") + .field("provider", &self.provider) + .field("model", &self.model) + .field("system_prompt", &self.system_prompt) + .field( + "api_key", + &self.api_key.as_ref().map(|_| "***REDACTED***".to_string()), + ) + .field("temperature", &self.temperature) + .field("max_depth", &self.max_depth) + .field("agentic", &self.agentic) + .field("allowed_tools", &self.allowed_tools) + .field("max_iterations", &self.max_iterations) + .finish() + } +} + +impl std::fmt::Debug for Config { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let model_provider_ids: Vec<&str> = + self.model_providers.keys().map(String::as_str).collect(); + let delegate_agent_ids: Vec<&str> = self.agents.keys().map(String::as_str).collect(); + let enabled_channel_count = [ + self.channels_config.telegram.is_some(), + self.channels_config.discord.is_some(), + self.channels_config.slack.is_some(), + self.channels_config.mattermost.is_some(), + self.channels_config.webhook.is_some(), + self.channels_config.imessage.is_some(), + self.channels_config.matrix.is_some(), + self.channels_config.signal.is_some(), + self.channels_config.whatsapp.is_some(), + self.channels_config.linq.is_some(), + self.channels_config.wati.is_some(), + self.channels_config.nextcloud_talk.is_some(), + self.channels_config.email.is_some(), + self.channels_config.irc.is_some(), + self.channels_config.lark.is_some(), + self.channels_config.feishu.is_some(), + self.channels_config.dingtalk.is_some(), + self.channels_config.qq.is_some(), + self.channels_config.nostr.is_some(), + self.channels_config.clawdtalk.is_some(), + ] + .into_iter() + .filter(|enabled| *enabled) + .count(); + + f.debug_struct("Config") + .field("workspace_dir", &self.workspace_dir) + .field("config_path", &self.config_path) + .field( + "api_key", + &self.api_key.as_ref().map(|_| "***REDACTED***".to_string()), + ) + .field("api_url_configured", &self.api_url.is_some()) + .field("default_provider", &self.default_provider) + .field("provider_api", &self.provider_api) + .field("default_model", &self.default_model) + .field("model_providers", &model_provider_ids) + .field("default_temperature", &self.default_temperature) + .field("model_routes_count", &self.model_routes.len()) + .field("embedding_routes_count", &self.embedding_routes.len()) + .field("delegate_agents", &delegate_agent_ids) + .field("cli_channel_enabled", &self.channels_config.cli) + .field("enabled_channels_count", &enabled_channel_count) + .field("sensitive_sections", &"***REDACTED***") + .finish_non_exhaustive() + } +} + // ── Hardware Config (wizard-driven) ───────────────────────────── /// Hardware transport mode. @@ -2139,7 +2212,7 @@ pub struct AutonomyConfig { /// /// When a tool is listed here, non-CLI channels will not expose it to the /// model in tool specs. - #[serde(default)] + #[serde(default = "default_non_cli_excluded_tools")] pub non_cli_excluded_tools: Vec, } @@ -2151,6 +2224,35 @@ fn default_always_ask() -> Vec { vec![] } +fn default_non_cli_excluded_tools() -> Vec { + [ + "shell", + "file_write", + "file_edit", + "git_operations", + "browser", + "browser_open", + "http_request", + "schedule", + "cron_add", + "cron_remove", + "cron_update", + "cron_run", + "memory_store", + "memory_forget", + "proxy_config", + "model_routing_config", + "pushover", + "composio", + "delegate", + "screenshot", + "image_info", + ] + .into_iter() + .map(std::string::ToString::to_string) + .collect() +} + fn is_valid_env_var_name(name: &str) -> bool { let mut chars = name.chars(); match chars.next() { @@ -2208,7 +2310,7 @@ impl Default for AutonomyConfig { auto_approve: default_auto_approve(), always_ask: default_always_ask(), allowed_roots: Vec::new(), - non_cli_excluded_tools: Vec::new(), + non_cli_excluded_tools: default_non_cli_excluded_tools(), } } } @@ -4177,6 +4279,21 @@ fn decrypt_secret( Ok(()) } +fn decrypt_vec_secrets( + store: &crate::security::SecretStore, + values: &mut [String], + field_name: &str, +) -> Result<()> { + for (idx, value) in values.iter_mut().enumerate() { + if crate::security::SecretStore::is_encrypted(value) { + *value = store + .decrypt(value) + .with_context(|| format!("Failed to decrypt {field_name}[{idx}]"))?; + } + } + Ok(()) +} + fn encrypt_optional_secret( store: &crate::security::SecretStore, value: &mut Option, @@ -4207,6 +4324,345 @@ fn encrypt_secret( Ok(()) } +fn encrypt_vec_secrets( + store: &crate::security::SecretStore, + values: &mut [String], + field_name: &str, +) -> Result<()> { + for (idx, value) in values.iter_mut().enumerate() { + if !crate::security::SecretStore::is_encrypted(value) { + *value = store + .encrypt(value) + .with_context(|| format!("Failed to encrypt {field_name}[{idx}]"))?; + } + } + Ok(()) +} + +fn decrypt_channel_secrets( + store: &crate::security::SecretStore, + channels: &mut ChannelsConfig, +) -> Result<()> { + if let Some(ref mut telegram) = channels.telegram { + decrypt_secret( + store, + &mut telegram.bot_token, + "config.channels_config.telegram.bot_token", + )?; + } + if let Some(ref mut discord) = channels.discord { + decrypt_secret( + store, + &mut discord.bot_token, + "config.channels_config.discord.bot_token", + )?; + } + if let Some(ref mut slack) = channels.slack { + decrypt_secret( + store, + &mut slack.bot_token, + "config.channels_config.slack.bot_token", + )?; + decrypt_optional_secret( + store, + &mut slack.app_token, + "config.channels_config.slack.app_token", + )?; + } + if let Some(ref mut mattermost) = channels.mattermost { + decrypt_secret( + store, + &mut mattermost.bot_token, + "config.channels_config.mattermost.bot_token", + )?; + } + if let Some(ref mut webhook) = channels.webhook { + decrypt_optional_secret( + store, + &mut webhook.secret, + "config.channels_config.webhook.secret", + )?; + } + if let Some(ref mut matrix) = channels.matrix { + decrypt_secret( + store, + &mut matrix.access_token, + "config.channels_config.matrix.access_token", + )?; + } + if let Some(ref mut whatsapp) = channels.whatsapp { + decrypt_optional_secret( + store, + &mut whatsapp.access_token, + "config.channels_config.whatsapp.access_token", + )?; + decrypt_optional_secret( + store, + &mut whatsapp.app_secret, + "config.channels_config.whatsapp.app_secret", + )?; + decrypt_optional_secret( + store, + &mut whatsapp.verify_token, + "config.channels_config.whatsapp.verify_token", + )?; + } + if let Some(ref mut linq) = channels.linq { + decrypt_secret( + store, + &mut linq.api_token, + "config.channels_config.linq.api_token", + )?; + decrypt_optional_secret( + store, + &mut linq.signing_secret, + "config.channels_config.linq.signing_secret", + )?; + } + if let Some(ref mut nextcloud) = channels.nextcloud_talk { + decrypt_secret( + store, + &mut nextcloud.app_token, + "config.channels_config.nextcloud_talk.app_token", + )?; + decrypt_optional_secret( + store, + &mut nextcloud.webhook_secret, + "config.channels_config.nextcloud_talk.webhook_secret", + )?; + } + if let Some(ref mut irc) = channels.irc { + decrypt_optional_secret( + store, + &mut irc.server_password, + "config.channels_config.irc.server_password", + )?; + decrypt_optional_secret( + store, + &mut irc.nickserv_password, + "config.channels_config.irc.nickserv_password", + )?; + decrypt_optional_secret( + store, + &mut irc.sasl_password, + "config.channels_config.irc.sasl_password", + )?; + } + if let Some(ref mut lark) = channels.lark { + decrypt_secret( + store, + &mut lark.app_secret, + "config.channels_config.lark.app_secret", + )?; + decrypt_optional_secret( + store, + &mut lark.encrypt_key, + "config.channels_config.lark.encrypt_key", + )?; + decrypt_optional_secret( + store, + &mut lark.verification_token, + "config.channels_config.lark.verification_token", + )?; + } + if let Some(ref mut dingtalk) = channels.dingtalk { + decrypt_secret( + store, + &mut dingtalk.client_secret, + "config.channels_config.dingtalk.client_secret", + )?; + } + if let Some(ref mut qq) = channels.qq { + decrypt_secret( + store, + &mut qq.app_secret, + "config.channels_config.qq.app_secret", + )?; + } + if let Some(ref mut nostr) = channels.nostr { + decrypt_secret( + store, + &mut nostr.private_key, + "config.channels_config.nostr.private_key", + )?; + } + if let Some(ref mut clawdtalk) = channels.clawdtalk { + decrypt_secret( + store, + &mut clawdtalk.api_key, + "config.channels_config.clawdtalk.api_key", + )?; + decrypt_optional_secret( + store, + &mut clawdtalk.webhook_secret, + "config.channels_config.clawdtalk.webhook_secret", + )?; + } + Ok(()) +} + +fn encrypt_channel_secrets( + store: &crate::security::SecretStore, + channels: &mut ChannelsConfig, +) -> Result<()> { + if let Some(ref mut telegram) = channels.telegram { + encrypt_secret( + store, + &mut telegram.bot_token, + "config.channels_config.telegram.bot_token", + )?; + } + if let Some(ref mut discord) = channels.discord { + encrypt_secret( + store, + &mut discord.bot_token, + "config.channels_config.discord.bot_token", + )?; + } + if let Some(ref mut slack) = channels.slack { + encrypt_secret( + store, + &mut slack.bot_token, + "config.channels_config.slack.bot_token", + )?; + encrypt_optional_secret( + store, + &mut slack.app_token, + "config.channels_config.slack.app_token", + )?; + } + if let Some(ref mut mattermost) = channels.mattermost { + encrypt_secret( + store, + &mut mattermost.bot_token, + "config.channels_config.mattermost.bot_token", + )?; + } + if let Some(ref mut webhook) = channels.webhook { + encrypt_optional_secret( + store, + &mut webhook.secret, + "config.channels_config.webhook.secret", + )?; + } + if let Some(ref mut matrix) = channels.matrix { + encrypt_secret( + store, + &mut matrix.access_token, + "config.channels_config.matrix.access_token", + )?; + } + if let Some(ref mut whatsapp) = channels.whatsapp { + encrypt_optional_secret( + store, + &mut whatsapp.access_token, + "config.channels_config.whatsapp.access_token", + )?; + encrypt_optional_secret( + store, + &mut whatsapp.app_secret, + "config.channels_config.whatsapp.app_secret", + )?; + encrypt_optional_secret( + store, + &mut whatsapp.verify_token, + "config.channels_config.whatsapp.verify_token", + )?; + } + if let Some(ref mut linq) = channels.linq { + encrypt_secret( + store, + &mut linq.api_token, + "config.channels_config.linq.api_token", + )?; + encrypt_optional_secret( + store, + &mut linq.signing_secret, + "config.channels_config.linq.signing_secret", + )?; + } + if let Some(ref mut nextcloud) = channels.nextcloud_talk { + encrypt_secret( + store, + &mut nextcloud.app_token, + "config.channels_config.nextcloud_talk.app_token", + )?; + encrypt_optional_secret( + store, + &mut nextcloud.webhook_secret, + "config.channels_config.nextcloud_talk.webhook_secret", + )?; + } + if let Some(ref mut irc) = channels.irc { + encrypt_optional_secret( + store, + &mut irc.server_password, + "config.channels_config.irc.server_password", + )?; + encrypt_optional_secret( + store, + &mut irc.nickserv_password, + "config.channels_config.irc.nickserv_password", + )?; + encrypt_optional_secret( + store, + &mut irc.sasl_password, + "config.channels_config.irc.sasl_password", + )?; + } + if let Some(ref mut lark) = channels.lark { + encrypt_secret( + store, + &mut lark.app_secret, + "config.channels_config.lark.app_secret", + )?; + encrypt_optional_secret( + store, + &mut lark.encrypt_key, + "config.channels_config.lark.encrypt_key", + )?; + encrypt_optional_secret( + store, + &mut lark.verification_token, + "config.channels_config.lark.verification_token", + )?; + } + if let Some(ref mut dingtalk) = channels.dingtalk { + encrypt_secret( + store, + &mut dingtalk.client_secret, + "config.channels_config.dingtalk.client_secret", + )?; + } + if let Some(ref mut qq) = channels.qq { + encrypt_secret( + store, + &mut qq.app_secret, + "config.channels_config.qq.app_secret", + )?; + } + if let Some(ref mut nostr) = channels.nostr { + encrypt_secret( + store, + &mut nostr.private_key, + "config.channels_config.nostr.private_key", + )?; + } + if let Some(ref mut clawdtalk) = channels.clawdtalk { + encrypt_secret( + store, + &mut clawdtalk.api_key, + "config.channels_config.clawdtalk.api_key", + )?; + encrypt_optional_secret( + store, + &mut clawdtalk.webhook_secret, + "config.channels_config.clawdtalk.webhook_secret", + )?; + } + Ok(()) +} + fn config_dir_creation_error(path: &Path) -> String { format!( "Failed to create config directory: {}. If running as an OpenRC service, \ @@ -4351,18 +4807,22 @@ impl Config { &mut config.storage.provider.config.db_url, "config.storage.provider.config.db_url", )?; + decrypt_vec_secrets( + &store, + &mut config.reliability.api_keys, + "config.reliability.api_keys", + )?; + decrypt_vec_secrets( + &store, + &mut config.gateway.paired_tokens, + "config.gateway.paired_tokens", + )?; for agent in config.agents.values_mut() { decrypt_optional_secret(&store, &mut agent.api_key, "config.agents.*.api_key")?; } - if let Some(ref mut ns) = config.channels_config.nostr { - decrypt_secret( - &store, - &mut ns.private_key, - "config.channels_config.nostr.private_key", - )?; - } + decrypt_channel_secrets(&store, &mut config.channels_config)?; config.apply_env_overrides(); config.validate()?; @@ -4505,6 +4965,26 @@ impl Config { ); } } + let mut seen_non_cli_excluded = std::collections::HashSet::new(); + for (i, tool_name) in self.autonomy.non_cli_excluded_tools.iter().enumerate() { + let normalized = tool_name.trim(); + if normalized.is_empty() { + anyhow::bail!("autonomy.non_cli_excluded_tools[{i}] must not be empty"); + } + if !normalized + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-') + { + anyhow::bail!( + "autonomy.non_cli_excluded_tools[{i}] contains invalid characters: {normalized}" + ); + } + if !seen_non_cli_excluded.insert(normalized.to_string()) { + anyhow::bail!( + "autonomy.non_cli_excluded_tools contains duplicate entry: {normalized}" + ); + } + } // Security OTP / estop if self.security.otp.token_ttl_secs == 0 { @@ -4999,18 +5479,22 @@ impl Config { &mut config_to_save.storage.provider.config.db_url, "config.storage.provider.config.db_url", )?; + encrypt_vec_secrets( + &store, + &mut config_to_save.reliability.api_keys, + "config.reliability.api_keys", + )?; + encrypt_vec_secrets( + &store, + &mut config_to_save.gateway.paired_tokens, + "config.gateway.paired_tokens", + )?; for agent in config_to_save.agents.values_mut() { encrypt_optional_secret(&store, &mut agent.api_key, "config.agents.*.api_key")?; } - if let Some(ref mut ns) = config_to_save.channels_config.nostr { - encrypt_secret( - &store, - &mut ns.private_key, - "config.channels_config.nostr.private_key", - )?; - } + encrypt_channel_secrets(&store, &mut config_to_save.channels_config)?; let toml_str = toml::to_string_pretty(&config_to_save).context("Failed to serialize config")?; @@ -5046,6 +5530,18 @@ impl Config { temp_path.display() ) })?; + #[cfg(unix)] + { + use std::{fs::Permissions, os::unix::fs::PermissionsExt}; + fs::set_permissions(&temp_path, Permissions::from_mode(0o600)) + .await + .with_context(|| { + format!( + "Failed to set secure permissions on temporary config file: {}", + temp_path.display() + ) + })?; + } temp_file .write_all(toml_str.as_bytes()) .await @@ -5081,15 +5577,14 @@ impl Config { #[cfg(unix)] { use std::{fs::Permissions, os::unix::fs::PermissionsExt}; - if let Err(err) = - fs::set_permissions(&self.config_path, Permissions::from_mode(0o600)).await - { - tracing::warn!( - "Failed to harden config permissions to 0600 at {}: {}", - self.config_path.display(), - err - ); - } + fs::set_permissions(&self.config_path, Permissions::from_mode(0o600)) + .await + .with_context(|| { + format!( + "Failed to enforce secure permissions on config file: {}", + self.config_path.display() + ) + })?; } sync_directory(parent_dir).await?; @@ -5160,6 +5655,60 @@ mod tests { assert!(c.config_path.to_string_lossy().contains("config.toml")); } + #[test] + async fn config_debug_redacts_sensitive_values() { + let mut config = Config::default(); + config.workspace_dir = PathBuf::from("/tmp/workspace"); + config.config_path = PathBuf::from("/tmp/config.toml"); + config.api_key = Some("root-credential".into()); + config.storage.provider.config.db_url = Some("postgres://user:pw@host/db".into()); + config.browser.computer_use.api_key = Some("browser-credential".into()); + config.gateway.paired_tokens = vec!["zc_0123456789abcdef".into()]; + config.channels_config.telegram = Some(TelegramConfig { + bot_token: "telegram-credential".into(), + allowed_users: Vec::new(), + stream_mode: StreamMode::Off, + draft_update_interval_ms: 1000, + interrupt_on_new_message: false, + mention_only: false, + }); + config.agents.insert( + "worker".into(), + DelegateAgentConfig { + provider: "openrouter".into(), + model: "model-test".into(), + system_prompt: None, + api_key: Some("agent-credential".into()), + temperature: None, + max_depth: 3, + agentic: false, + allowed_tools: Vec::new(), + max_iterations: 10, + }, + ); + + let debug_output = format!("{config:?}"); + assert!(debug_output.contains("***REDACTED***")); + + for secret in [ + "root-credential", + "postgres://user:pw@host/db", + "browser-credential", + "zc_0123456789abcdef", + "telegram-credential", + "agent-credential", + ] { + assert!( + !debug_output.contains(secret), + "debug output leaked secret value: {secret}" + ); + } + + assert!(!debug_output.contains("paired_tokens")); + assert!(!debug_output.contains("bot_token")); + assert!(!debug_output.contains("db_url")); + } + #[test] async fn config_dir_creation_error_mentions_openrc_and_path() { let msg = config_dir_creation_error(Path::new("/etc/zeroclaw")); @@ -5244,6 +5793,41 @@ mod tests { assert!(a.require_approval_for_medium_risk); assert!(a.block_high_risk_commands); assert!(a.shell_env_passthrough.is_empty()); + assert!(a.non_cli_excluded_tools.contains(&"shell".to_string())); + assert!(a.non_cli_excluded_tools.contains(&"delegate".to_string())); + } + + #[test] + async fn autonomy_config_serde_defaults_non_cli_excluded_tools() { + let raw = r#" +level = "supervised" +workspace_only = true +allowed_commands = ["git"] +forbidden_paths = ["/etc"] +max_actions_per_hour = 20 +max_cost_per_day_cents = 500 +require_approval_for_medium_risk = true +block_high_risk_commands = true +shell_env_passthrough = [] +auto_approve = ["file_read"] +always_ask = [] +allowed_roots = [] +"#; + let parsed: AutonomyConfig = toml::from_str(raw).unwrap(); + assert!(parsed.non_cli_excluded_tools.contains(&"shell".to_string())); + assert!(parsed + .non_cli_excluded_tools + .contains(&"browser".to_string())); + } + + #[test] + async fn config_validate_rejects_duplicate_non_cli_excluded_tools() { + let mut cfg = Config::default(); + cfg.autonomy.non_cli_excluded_tools = vec!["shell".into(), "shell".into()]; + let err = cfg.validate().unwrap_err(); + assert!(err + .to_string() + .contains("autonomy.non_cli_excluded_tools contains duplicate entry")); } #[test] @@ -5708,6 +6292,16 @@ tool_dispatcher = "xml" config.browser.computer_use.api_key = Some("browser-credential".into()); config.web_search.brave_api_key = Some("brave-credential".into()); config.storage.provider.config.db_url = Some("postgres://user:pw@host/db".into()); + config.reliability.api_keys = vec!["backup-credential".into()]; + config.gateway.paired_tokens = vec!["zc_0123456789abcdef".into()]; + config.channels_config.telegram = Some(TelegramConfig { + bot_token: "telegram-credential".into(), + allowed_users: Vec::new(), + stream_mode: StreamMode::Off, + draft_update_interval_ms: 1000, + interrupt_on_new_message: false, + mention_only: false, + }); config.agents.insert( "worker".into(), @@ -5775,6 +6369,27 @@ tool_dispatcher = "xml" "postgres://user:pw@host/db" ); + let reliability_key = &stored.reliability.api_keys[0]; + assert!(crate::security::SecretStore::is_encrypted(reliability_key)); + assert_eq!(store.decrypt(reliability_key).unwrap(), "backup-credential"); + + let paired_token = &stored.gateway.paired_tokens[0]; + assert!(crate::security::SecretStore::is_encrypted(paired_token)); + assert_eq!(store.decrypt(paired_token).unwrap(), "zc_0123456789abcdef"); + + let telegram_token = stored + .channels_config + .telegram + .as_ref() + .unwrap() + .bot_token + .clone(); + assert!(crate::security::SecretStore::is_encrypted(&telegram_token)); + assert_eq!( + store.decrypt(&telegram_token).unwrap(), + "telegram-credential" + ); + let _ = fs::remove_dir_all(&dir).await; } diff --git a/src/gateway/mod.rs b/src/gateway/mod.rs index f9dba8617..6e237bcb9 100644 --- a/src/gateway/mod.rs +++ b/src/gateway/mod.rs @@ -793,7 +793,36 @@ async fn handle_health(State(state): State) -> impl IntoResponse { const PROMETHEUS_CONTENT_TYPE: &str = "text/plain; version=0.0.4; charset=utf-8"; /// GET /metrics — Prometheus text exposition format -async fn handle_metrics(State(state): State) -> impl IntoResponse { +async fn handle_metrics( + State(state): State, + ConnectInfo(peer_addr): ConnectInfo, + headers: HeaderMap, +) -> impl IntoResponse { + if state.pairing.require_pairing() { + let auth = headers + .get(header::AUTHORIZATION) + .and_then(|v| v.to_str().ok()) + .unwrap_or(""); + let token = auth.strip_prefix("Bearer ").unwrap_or("").trim(); + if !state.pairing.is_authenticated(token) { + return ( + StatusCode::UNAUTHORIZED, + [(header::CONTENT_TYPE, PROMETHEUS_CONTENT_TYPE)], + String::from( + "# unauthorized: provide Authorization: Bearer for /metrics\n", + ), + ); + } + } else if !peer_addr.ip().is_loopback() { + return ( + StatusCode::FORBIDDEN, + [(header::CONTENT_TYPE, PROMETHEUS_CONTENT_TYPE)], + String::from( + "# metrics disabled for non-loopback clients when pairing is not required\n", + ), + ); + } + let body = if let Some(prom) = state .observer .as_ref() @@ -1136,6 +1165,20 @@ async fn handle_webhook( return (StatusCode::TOO_MANY_REQUESTS, Json(err)); } + // Require at least one auth layer for non-loopback traffic. + if !state.pairing.require_pairing() + && state.webhook_secret_hash.is_none() + && !peer_addr.ip().is_loopback() + { + tracing::warn!( + "Webhook: rejected unauthenticated non-loopback request (pairing disabled and no webhook secret configured)" + ); + let err = serde_json::json!({ + "error": "Unauthorized — configure pairing or X-Webhook-Secret for non-local webhook access" + }); + return (StatusCode::UNAUTHORIZED, Json(err)); + } + // ── Bearer token auth (pairing) ── if state.pairing.require_pairing() { let auth = headers @@ -1961,7 +2004,9 @@ mod tests { event_tx: tokio::sync::broadcast::channel(16).0, }; - let response = handle_metrics(State(state)).await.into_response(); + let response = handle_metrics(State(state), test_connect_info(), HeaderMap::new()) + .await + .into_response(); assert_eq!(response.status(), StatusCode::OK); assert_eq!( response @@ -2015,7 +2060,9 @@ mod tests { event_tx: tokio::sync::broadcast::channel(16).0, }; - let response = handle_metrics(State(state)).await.into_response(); + let response = handle_metrics(State(state), test_connect_info(), HeaderMap::new()) + .await + .into_response(); assert_eq!(response.status(), StatusCode::OK); let body = response.into_body().collect().await.unwrap().to_bytes(); @@ -2023,6 +2070,98 @@ mod tests { assert!(text.contains("zeroclaw_heartbeat_ticks_total 1")); } + #[tokio::test] + async fn metrics_endpoint_rejects_public_clients_when_pairing_is_disabled() { + let state = AppState { + config: Arc::new(Mutex::new(Config::default())), + provider: Arc::new(MockProvider::default()), + model: "test-model".into(), + temperature: 0.0, + mem: Arc::new(MockMemory), + auto_save: false, + webhook_secret_hash: None, + pairing: Arc::new(PairingGuard::new(false, &[])), + trust_forwarded_headers: false, + rate_limiter: Arc::new(GatewayRateLimiter::new(100, 100, 100)), + idempotency_store: Arc::new(IdempotencyStore::new(Duration::from_secs(300), 1000)), + whatsapp: None, + whatsapp_app_secret: None, + linq: None, + linq_signing_secret: None, + nextcloud_talk: None, + nextcloud_talk_webhook_secret: None, + wati: None, + qq: None, + qq_webhook_enabled: false, + observer: Arc::new(crate::observability::NoopObserver), + tools_registry: Arc::new(Vec::new()), + tools_registry_exec: Arc::new(Vec::new()), + multimodal: crate::config::MultimodalConfig::default(), + max_tool_iterations: 10, + cost_tracker: None, + event_tx: tokio::sync::broadcast::channel(16).0, + }; + + let response = handle_metrics(State(state), test_public_connect_info(), HeaderMap::new()) + .await + .into_response(); + assert_eq!(response.status(), StatusCode::FORBIDDEN); + + let body = response.into_body().collect().await.unwrap().to_bytes(); + let text = String::from_utf8(body.to_vec()).unwrap(); + assert!(text.contains("non-loopback")); + } + + #[tokio::test] + async fn metrics_endpoint_requires_bearer_token_when_pairing_is_enabled() { + let paired_token = "zc_test_token".to_string(); + let state = AppState { + config: Arc::new(Mutex::new(Config::default())), + provider: Arc::new(MockProvider::default()), + model: "test-model".into(), + temperature: 0.0, + mem: Arc::new(MockMemory), + auto_save: false, + webhook_secret_hash: None, + pairing: Arc::new(PairingGuard::new(true, std::slice::from_ref(&paired_token))), + trust_forwarded_headers: false, + rate_limiter: Arc::new(GatewayRateLimiter::new(100, 100, 100)), + idempotency_store: Arc::new(IdempotencyStore::new(Duration::from_secs(300), 1000)), + whatsapp: None, + whatsapp_app_secret: None, + linq: None, + linq_signing_secret: None, + nextcloud_talk: None, + nextcloud_talk_webhook_secret: None, + wati: None, + qq: None, + qq_webhook_enabled: false, + observer: Arc::new(crate::observability::NoopObserver), + tools_registry: Arc::new(Vec::new()), + tools_registry_exec: Arc::new(Vec::new()), + multimodal: crate::config::MultimodalConfig::default(), + max_tool_iterations: 10, + cost_tracker: None, + event_tx: tokio::sync::broadcast::channel(16).0, + }; + + let unauthorized = + handle_metrics(State(state.clone()), test_connect_info(), HeaderMap::new()) + .await + .into_response(); + assert_eq!(unauthorized.status(), StatusCode::UNAUTHORIZED); + + let mut headers = HeaderMap::new(); + headers.insert( + header::AUTHORIZATION, + HeaderValue::from_str(&format!("Bearer {paired_token}")).unwrap(), + ); + let authorized = handle_metrics(State(state), test_connect_info(), headers) + .await + .into_response(); + assert_eq!(authorized.status(), StatusCode::OK); + } + #[test] fn gateway_rate_limiter_blocks_after_limit() { let limiter = GatewayRateLimiter::new(2, 2, 100); @@ -2183,12 +2322,15 @@ mod tests { let parsed: Config = toml::from_str(&saved).unwrap(); assert_eq!(parsed.gateway.paired_tokens.len(), 1); let persisted = &parsed.gateway.paired_tokens[0]; - assert_eq!(persisted.len(), 64); - assert!(persisted.chars().all(|c| c.is_ascii_hexdigit())); + assert!(crate::security::SecretStore::is_encrypted(persisted)); + let store = crate::security::SecretStore::new(temp.path(), true); + let decrypted = store.decrypt(persisted).unwrap(); + assert_eq!(decrypted.len(), 64); + assert!(decrypted.chars().all(|c| c.is_ascii_hexdigit())); let in_memory = shared_config.lock(); assert_eq!(in_memory.gateway.paired_tokens.len(), 1); - assert_eq!(&in_memory.gateway.paired_tokens[0], persisted); + assert_eq!(&in_memory.gateway.paired_tokens[0], &decrypted); } #[test] @@ -2366,6 +2508,10 @@ mod tests { ConnectInfo(SocketAddr::from(([127, 0, 0, 1], 30_300))) } + fn test_public_connect_info() -> ConnectInfo { + ConnectInfo(SocketAddr::from(([203, 0, 113, 10], 30_300))) + } + #[tokio::test] async fn webhook_idempotency_skips_duplicate_provider_calls() { let provider_impl = Arc::new(MockProvider::default()); @@ -2433,6 +2579,56 @@ mod tests { assert_eq!(provider_impl.calls.load(Ordering::SeqCst), 1); } + #[tokio::test] + async fn webhook_rejects_public_traffic_without_auth_layers() { + let provider_impl = Arc::new(MockProvider::default()); + let provider: Arc = provider_impl; + let memory: Arc = Arc::new(MockMemory); + + let state = AppState { + config: Arc::new(Mutex::new(Config::default())), + provider, + model: "test-model".into(), + temperature: 0.0, + mem: memory, + auto_save: false, + webhook_secret_hash: None, + pairing: Arc::new(PairingGuard::new(false, &[])), + trust_forwarded_headers: false, + rate_limiter: Arc::new(GatewayRateLimiter::new(100, 100, 100)), + idempotency_store: Arc::new(IdempotencyStore::new(Duration::from_secs(300), 1000)), + whatsapp: None, + whatsapp_app_secret: None, + linq: None, + linq_signing_secret: None, + nextcloud_talk: None, + nextcloud_talk_webhook_secret: None, + wati: None, + qq: None, + qq_webhook_enabled: false, + observer: Arc::new(crate::observability::NoopObserver), + tools_registry: Arc::new(Vec::new()), + tools_registry_exec: Arc::new(Vec::new()), + multimodal: crate::config::MultimodalConfig::default(), + max_tool_iterations: 10, + cost_tracker: None, + event_tx: tokio::sync::broadcast::channel(16).0, + }; + + let response = handle_webhook( + State(state), + test_public_connect_info(), + HeaderMap::new(), + Ok(Json(WebhookBody { + message: "hello".into(), + })), + ) + .await + .into_response(); + + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + } + #[tokio::test] async fn node_control_returns_not_found_when_disabled() { let provider: Arc = Arc::new(MockProvider::default()); diff --git a/src/providers/mod.rs b/src/providers/mod.rs index 177df47dc..cb39fed73 100644 --- a/src/providers/mod.rs +++ b/src/providers/mod.rs @@ -719,9 +719,9 @@ fn token_end(input: &str, from: usize) -> usize { /// Scrub known secret-like token prefixes from provider error strings. /// /// Redacts tokens with prefixes like `sk-`, `xoxb-`, `xoxp-`, `ghp_`, `gho_`, -/// `ghu_`, and `github_pat_`. +/// `ghu_`, `github_pat_`, `AIza`, and `AKIA`. pub fn scrub_secret_patterns(input: &str) -> String { - const PREFIXES: [&str; 7] = [ + const PREFIXES: [&str; 9] = [ "sk-", "xoxb-", "xoxp-", @@ -729,6 +729,8 @@ pub fn scrub_secret_patterns(input: &str) -> String { "gho_", "ghu_", "github_pat_", + "AIza", + "AKIA", ]; let mut scrubbed = input.to_string(); @@ -2865,6 +2867,20 @@ mod tests { assert_eq!(result, "failed: [REDACTED]"); } + #[test] + fn scrub_google_api_key_prefix() { + let input = "upstream returned key AIzaSyA8exampleToken123456"; + let result = scrub_secret_patterns(input); + assert_eq!(result, "upstream returned key [REDACTED]"); + } + + #[test] + fn scrub_aws_access_key_prefix() { + let input = "credential leak AKIAIOSFODNN7EXAMPLE"; + let result = scrub_secret_patterns(input); + assert_eq!(result, "credential leak [REDACTED]"); + } + #[test] fn routed_provider_accepts_per_route_max_tokens() { let reliability = crate::config::ReliabilityConfig::default(); diff --git a/src/security/otp.rs b/src/security/otp.rs index 2ab6913fb..7689a28e8 100644 --- a/src/security/otp.rs +++ b/src/security/otp.rs @@ -5,6 +5,7 @@ use parking_lot::Mutex; use ring::hmac; use std::collections::HashMap; use std::fs; +use std::io::Write; use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; @@ -76,7 +77,7 @@ impl OtpValidator { .get(normalized) .is_some_and(|expiry| *expiry >= now_secs) { - return Ok(true); + return Ok(false); } } @@ -132,18 +133,43 @@ fn write_secret_file(path: &Path, value: &str) -> Result<()> { } let temp_path = path.with_extension(format!("tmp-{}", uuid::Uuid::new_v4())); - fs::write(&temp_path, value).with_context(|| { + let mut temp_file = fs::OpenOptions::new() + .create_new(true) + .write(true) + .open(&temp_path) + .with_context(|| { + format!( + "Failed to create temporary OTP secret {}", + temp_path.display() + ) + })?; + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + temp_file + .set_permissions(fs::Permissions::from_mode(0o600)) + .with_context(|| { + format!( + "Failed to set permissions on temporary OTP secret {}", + temp_path.display() + ) + })?; + } + + temp_file.write_all(value.as_bytes()).with_context(|| { format!( "Failed to write temporary OTP secret {}", temp_path.display() ) })?; - - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - let _ = fs::set_permissions(&temp_path, fs::Permissions::from_mode(0o600)); - } + temp_file.sync_all().with_context(|| { + format!( + "Failed to fsync temporary OTP secret {}", + temp_path.display() + ) + })?; + drop(temp_file); fs::rename(&temp_path, path).with_context(|| { format!( @@ -151,6 +177,17 @@ fn write_secret_file(path: &Path, value: &str) -> Result<()> { path.display() ) })?; + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(path, fs::Permissions::from_mode(0o600)).with_context(|| { + format!( + "Failed to enforce permissions on OTP secret file {}", + path.display() + ) + })?; + } Ok(()) } @@ -275,6 +312,18 @@ mod tests { assert!(validator.validate_at(&code, now).unwrap()); } + #[test] + fn replayed_totp_code_is_rejected() { + let dir = tempdir().unwrap(); + let store = SecretStore::new(dir.path(), true); + let (validator, _) = OtpValidator::from_config(&test_config(), dir.path(), &store).unwrap(); + + let now = 1_700_000_000u64; + let code = validator.code_for_timestamp(now); + assert!(validator.validate_at(&code, now).unwrap()); + assert!(!validator.validate_at(&code, now).unwrap()); + } + #[test] fn expired_totp_code_is_rejected() { let dir = tempdir().unwrap(); diff --git a/src/security/pairing.rs b/src/security/pairing.rs index 95eeb5310..b97f8d700 100644 --- a/src/security/pairing.rs +++ b/src/security/pairing.rs @@ -190,9 +190,13 @@ impl PairingGuard { // TODO: make this function the main one without spawning a task let handle = tokio::task::spawn_blocking(move || this.try_pair_blocking(&code, &client_id)); - handle - .await - .expect("failed to spawn blocking task this should not happen") + match handle.await { + Ok(result) => result, + Err(err) => { + tracing::error!("pairing worker task failed: {err}"); + Ok(None) + } + } } /// Check if a bearer token is valid (compares against stored hashes). diff --git a/src/security/secrets.rs b/src/security/secrets.rs index 663112c40..68a647a8e 100644 --- a/src/security/secrets.rs +++ b/src/security/secrets.rs @@ -24,6 +24,7 @@ use anyhow::{Context, Result}; use chacha20poly1305::aead::{Aead, KeyInit, OsRng}; use chacha20poly1305::{AeadCore, ChaCha20Poly1305, Key, Nonce}; use std::fs; +use std::io::Write; use std::path::{Path, PathBuf}; /// Length of the random encryption key in bytes (256-bit, matches `ChaCha20`). @@ -178,16 +179,42 @@ impl SecretStore { if let Some(parent) = self.key_path.parent() { fs::create_dir_all(parent)?; } - fs::write(&self.key_path, hex_encode(&key)) - .context("Failed to write secret key file")?; - // Set restrictive permissions - #[cfg(unix)] + let key_hex = hex_encode(&key); + match fs::OpenOptions::new() + .create_new(true) + .write(true) + .open(&self.key_path) { - use std::os::unix::fs::PermissionsExt; - fs::set_permissions(&self.key_path, fs::Permissions::from_mode(0o600)) - .context("Failed to set key file permissions")?; + Ok(mut key_file) => { + // Set restrictive permissions before writing key bytes. + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + key_file + .set_permissions(fs::Permissions::from_mode(0o600)) + .context("Failed to set key file permissions")?; + } + + key_file + .write_all(key_hex.as_bytes()) + .context("Failed to write secret key file")?; + key_file + .sync_all() + .context("Failed to fsync secret key file")?; + } + Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => { + // Concurrent creator won the race; read the existing key. + let hex_key = fs::read_to_string(&self.key_path) + .context("Failed to read concurrently created secret key file")?; + return hex_decode(hex_key.trim()) + .context("Secret key file is corrupt after concurrent create"); + } + Err(err) => { + return Err(err).context("Failed to create secret key file"); + } } + #[cfg(windows)] { // On Windows, use icacls to restrict permissions to current user only