diff --git a/prek.toml b/prek.toml index 82c09caa..99598f46 100644 --- a/prek.toml +++ b/prek.toml @@ -54,25 +54,10 @@ args = ["-c", "lychee --offline --no-progress --include-fragments docs/**/*.md R types = ["markdown"] pass_filenames = false -# Tests — expensive, runs only on pre-push -[[repos.hooks]] -id = "cargo-test" -name = "cargo test" -language = "system" -entry = "cargo nextest run --profile ci" -types = ["rust"] -pass_filenames = false -stages = ["pre-push"] - -# Doc tests — not covered by nextest -[[repos.hooks]] -id = "cargo-doctest" -name = "cargo doc tests" -language = "system" -entry = "cargo test --doc" -types = ["rust"] -pass_filenames = false -stages = ["pre-push"] +# NOTE: cargo-test and cargo-doctest were removed from pre-push to keep +# the gate fast-fail. CI re-runs the full suite on every PR (see +# `.github/workflows/ci.yml` `tests` job), so duplicating ~20 min of work +# locally only delays the developer feedback loop without adding signal. # Cargo deny — license & advisory & dependency checks # Only runs when Cargo.toml/lock or Rust files change. diff --git a/src/cli/config/budget.rs b/src/cli/config/budget.rs index 414a2f26..4eaa6e74 100644 --- a/src/cli/config/budget.rs +++ b/src/cli/config/budget.rs @@ -6,6 +6,7 @@ use crate::cli::BudgetUsd; /// Budget configuration #[derive(Debug, Clone, Deserialize, Serialize, Default)] +#[serde(deny_unknown_fields)] pub struct BudgetConfig { /// Global monthly hard cap in USD (0 = unlimited) #[serde(default)] diff --git a/src/cli/config/cache.rs b/src/cli/config/cache.rs index b2e42554..201ddf66 100644 --- a/src/cli/config/cache.rs +++ b/src/cli/config/cache.rs @@ -4,6 +4,7 @@ use serde::{Deserialize, Serialize}; /// LLM response cache configuration #[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct CacheConfig { /// Enable response caching (only for temperature=0 requests) #[serde(default)] diff --git a/src/cli/config/providers.rs b/src/cli/config/providers.rs index 87c9ab77..88c240f8 100644 --- a/src/cli/config/providers.rs +++ b/src/cli/config/providers.rs @@ -28,6 +28,7 @@ pub enum AuthType { /// Provider configuration from TOML. #[derive(Debug, Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] pub struct ProviderConfig { /// Unique provider name used in routing and logging. pub name: String, @@ -119,7 +120,16 @@ pub struct ProviderConfig { } impl ProviderConfig { - /// Returns `true` if the provider is enabled (defaults to `true`). + /// Returns `true` if the provider is enabled. + /// + /// Semantics: + /// - `enabled = true` → enabled. + /// - `enabled = false` → disabled. + /// - `enabled` absent → enabled (sensible default for newly added blocks). + /// + /// Typo safety: `#[serde(deny_unknown_fields)]` on [`ProviderConfig`] + /// rejects misspelled keys (e.g. `enbaled`) at parse time, so an absent + /// `enabled` field genuinely means "not specified" rather than "typo'd". pub fn is_enabled(&self) -> bool { self.enabled.unwrap_or(true) } diff --git a/src/cli/config/routing.rs b/src/cli/config/routing.rs index d0992450..0538177f 100644 --- a/src/cli/config/routing.rs +++ b/src/cli/config/routing.rs @@ -9,6 +9,7 @@ use super::user::PresetConfig; /// Router configuration #[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct RouterConfig { /// Default model for unclassified requests pub default: String, @@ -102,6 +103,7 @@ pub struct FanOutConfig { /// Model configuration with 1:N provider mappings #[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct ModelConfig { /// External model name (used in API requests) pub name: String, @@ -203,6 +205,7 @@ pub struct TierMatchCondition { /// When the scoring heuristic classifies a request, the dispatch pipeline /// resolves providers from the matching tier instead of the default model mappings. #[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct TierConfig { /// Tier name — must match a `ComplexityTier` variant (case-insensitive). pub name: String, diff --git a/src/cli/config/security.rs b/src/cli/config/security.rs index 27583a9c..c403952e 100644 --- a/src/cli/config/security.rs +++ b/src/cli/config/security.rs @@ -8,6 +8,7 @@ use super::default_true; /// Security configuration (wired into middleware stack) #[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct SecurityConfig { /// Master switch for security middleware #[serde(default = "default_true")] diff --git a/src/features/dlp/config.rs b/src/features/dlp/config.rs index f9329651..dc81c20e 100644 --- a/src/features/dlp/config.rs +++ b/src/features/dlp/config.rs @@ -3,6 +3,7 @@ use serde::{Deserialize, Serialize}; /// Top-level DLP configuration, mapped from `[dlp]` in TOML. #[derive(Debug, Clone, Deserialize, Serialize, Default)] +#[serde(deny_unknown_fields)] pub struct DlpConfig { /// Enables the DLP pipeline globally. #[serde(default)] diff --git a/src/models/config.rs b/src/models/config.rs index 63fe5196..1c87c1b5 100644 --- a/src/models/config.rs +++ b/src/models/config.rs @@ -25,6 +25,7 @@ use crate::features::tap::TapConfig; /// Application configuration #[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] pub struct AppConfig { /// Config schema version (for forward compatibility) #[serde(default, skip_serializing_if = "Option::is_none")] diff --git a/src/routing/classify/classify.rs b/src/routing/classify/classify.rs index ebe280eb..38b70b2a 100644 --- a/src/routing/classify/classify.rs +++ b/src/routing/classify/classify.rs @@ -84,6 +84,7 @@ impl Default for ScoringThresholds { /// Scoring configuration combining weights and thresholds. #[derive(Debug, Clone, Default, serde::Deserialize, serde::Serialize)] +#[serde(deny_unknown_fields)] pub struct ScoringConfig { /// Per-signal weights. pub weights: ScoringWeights, diff --git a/src/server/config_guard.rs b/src/server/config_guard.rs index b0a8132f..58993720 100644 --- a/src/server/config_guard.rs +++ b/src/server/config_guard.rs @@ -12,29 +12,76 @@ use std::sync::Arc; use tracing::info; /// Top-level TOML sections that are never writable via any config API. -const DENIED_SECTIONS: &[&str] = &["providers", "dlp"]; +/// +/// Each entry is denied because hot-reloading it cannot be done safely +/// at runtime — either the data is sensitive (and must travel through a +/// dedicated secret API), or the code path that consumes it is set up +/// once at process start and not re-initialised on `/api/config/reload`: +/// +/// | Section | Reason | +/// |-------------|-----------------------------------------------------------------------------------------| +/// | `providers` | Contains API keys; mutate via `grob connect` / secret backend, not the config API. | +/// | `dlp` | Security policy must not be weakened by an authenticated control-plane caller. | +/// | `tee` | TEE attestation runs at startup; flipping the mode mid-flight bypasses the gate. | +/// | `fips` | FIPS mode is checked once on init; toggling at runtime gives a false sense of compliance. | +/// +/// To change any of these the operator must edit `~/.grob/config.toml` +/// and restart the daemon. +const DENIED_SECTIONS: &[&str] = &["providers", "dlp", "tee", "fips"]; /// Per-section keys that are never writable via any config API. +/// +/// These are individual fields whose host section is otherwise editable, +/// but the field itself is either credential material or wired into a +/// non-reloadable subsystem: +/// +/// | Section.Key | Reason | +/// |--------------------|---------------------------------------------------------------------------------| +/// | `router.api_key` | Credential material — never round-trip through the config API. | +/// | `budget.api_key` | Same. | +/// | `cache.api_key` | Same. | +/// | `server.tls` | TLS listener is bound at startup; rebuilding it requires a daemon restart. | +/// | `secrets.backend` | The secret backend is constructed once and shared via `Arc`; swapping it at | +/// | | runtime would orphan in-flight readers and change credential resolution semantics. | const DENIED_KEYS: &[(&str, &str)] = &[ ("router", "api_key"), ("budget", "api_key"), ("cache", "api_key"), + ("server", "tls"), + ("secrets", "backend"), ]; /// Checks whether a (section, key) pair is blocked by the deny-list. /// -/// Returns `true` when the write must be rejected: -/// - The entire `providers` section (contains API keys). -/// - The entire `dlp` section (security must not be weakened). -/// - Any `api_key` field in any section. +/// Returns `true` when the write must be rejected. See [`DENIED_SECTIONS`] +/// and [`DENIED_KEYS`] for the rationale behind every entry. A denied +/// attempt is logged at INFO so the operator sees actionable guidance +/// (restart instead of expecting a silent reload to take effect). pub fn is_section_or_key_denied(section: &str, key: &str) -> bool { if DENIED_SECTIONS.contains(§ion) { + info!( + section = %section, + "config hot-reload: section is on the deny-list; restart the daemon to apply changes" + ); return true; } if key == "api_key" { + info!( + section = %section, + key = %key, + "config hot-reload: api_key fields cannot be set via the config API; use `grob connect` or the secret backend" + ); + return true; + } + if DENIED_KEYS.iter().any(|(s, k)| *s == section && *k == key) { + info!( + section = %section, + key = %key, + "config hot-reload: key is on the deny-list; restart the daemon to apply changes" + ); return true; } - DENIED_KEYS.iter().any(|(s, k)| *s == section && *k == key) + false } /// Validates a key update against the deny-list using [`ConfigSection`]. @@ -177,6 +224,27 @@ mod tests { assert!(!is_section_or_key_denied("cache", "ttl_secs")); } + #[test] + fn deny_static_init_sections() { + // tee and fips are checked once at startup; toggling them at runtime + // would bypass the gate without the operator realising. + assert!(is_section_or_key_denied("tee", "mode")); + assert!(is_section_or_key_denied("tee", "sealed_keys")); + assert!(is_section_or_key_denied("fips", "mode")); + assert!(is_section_or_key_denied("fips", "anything")); + } + + #[test] + fn deny_static_init_keys() { + // The TLS listener and secret backend are constructed once on + // process start; both require a daemon restart to swap. + assert!(is_section_or_key_denied("server", "tls")); + assert!(is_section_or_key_denied("secrets", "backend")); + // Sibling keys in the same sections must remain editable. + assert!(!is_section_or_key_denied("server", "host")); + assert!(!is_section_or_key_denied("server", "port")); + } + #[cfg(feature = "mcp")] mod mcp_compat { use super::*;