fix(config): add config validation + deny_unknown_fields (#334 P3-core)

CHANGELOG.md

@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed — Config Validation & Best Practices (#334)
+
+- **Quality gate no longer fails when disabled (#58).** `evaluate()` forced `GateResult::Pass` when `enabled: false`; it now short-circuits before applying thresholds so a disabled gate never blocks a merge.
+- **Category actions are now validated enums (#57).** `quality_gate.categories.*.action` accepted any string, so a typo like `blok` silently became blocking. It is now a case-insensitive `CategoryAction` enum (`block`/`warn`/`ignore`); unknown values fail loudly at config load.
+- **Config values are validated (#94).** `Config::validate()` now rejects out-of-range/unsupported values at load time: `temperature` (0.0–2.0), `max_tokens`/`timeout` (≥1), `max_tokens_param`, `response_format`, `output.format`, `hook.mode`/`on_violation`/`min_severity`, and `provider.base_url` scheme. Multiple errors are aggregated into one message.
+- **Profile values are validated (#81).** Focus area `weight` must be 1–10, and `action`/`tone`/`detail_level` must be recognized values; all built-in profiles pass.
+
+### Added — Config Safety
+
+- **`deny_unknown_fields` on all config sections (#80).** Misspelled YAML keys (e.g. `quailty_gate`, `temprature`) are now rejected at parse time instead of being silently dropped.
+
 ## [0.6.2] - 2026-06-21
 
 ### Fixed — Token Usage Tracking

docs/changelog.md

@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed — Config Validation & Best Practices (#334)
+
+- **Quality gate no longer fails when disabled (#58).** `evaluate()` now forces `GateResult::Pass` when `enabled: false` instead of applying thresholds.
+- **Category actions are now validated enums (#57).** `quality_gate.categories.*.action` is a case-insensitive `CategoryAction` enum (`block`/`warn`/`ignore`); unknown values fail loudly at config load instead of silently becoming blocking.
+- **Config values are validated (#94).** `Config::validate()` rejects out-of-range/unsupported values at load: `temperature` (0.0–2.0), `max_tokens`/`timeout` (≥1), `max_tokens_param`, `response_format`, `output.format`, `hook.*`, and `provider.base_url`.
+- **Profile values are validated (#81).** Focus `weight` must be 1–10; `action`/`tone`/`detail_level` must be recognized.
+- **`deny_unknown_fields` on all config sections (#80).** Misspelled YAML keys are rejected at parse time.
+
 ## [0.6.2] - 2026-06-21
 
 ### Fixed — Token Usage Tracking

src/config/loader.rs

@@ -299,6 +299,10 @@ pub fn load_config(
         config.output.color = false;
     }
 
+    // #94: validate semantic constraints after all sources are merged so typos
+    // and out-of-range values fail loudly at load instead of at runtime.
+    config.validate()?;
+
     Ok(config)
 }
 

src/config/schema.rs

@@ -148,8 +148,121 @@ impl HookConfig {
     }
 }
 
+impl Config {
+    /// Validate semantic constraints on resolved config values (#94).
+    ///
+    /// Catches typos and out-of-range values that would otherwise propagate
+    /// silently to runtime (e.g. `temperature: 5.0`, an unsupported output
+    /// format, or a misspelled `max_tokens_param`). Called after merging all
+    /// config sources in `loader::load_config`.
+    pub fn validate(&self) -> std::result::Result<(), CoraError> {
+        let mut errs: Vec<String> = Vec::new();
+
+        // ── provider ──
+        if self.provider.provider.trim().is_empty() {
+            errs.push("provider.provider must not be empty".into());
+        }
+        let base = self.provider.base_url.trim();
+        let valid_scheme = base.is_empty()
+            || base.starts_with("http://")
+            || base.starts_with("https://")
+            || base.starts_with("ws://")
+            || base.starts_with("unix:");
+        if !valid_scheme {
+            errs.push(format!(
+                "provider.base_url must be an http(s) URL, got: {}",
+                self.provider.base_url
+            ));
+        }
+
+        // ── llm ──
+        if !(0.0..=2.0).contains(&self.temperature) {
+            errs.push(format!(
+                "llm.temperature must be between 0.0 and 2.0, got: {}",
+                self.temperature
+            ));
+        }
+        if self.max_tokens == 0 {
+            errs.push("llm.max_tokens must be at least 1".into());
+        }
+        if self.timeout == 0 {
+            errs.push("llm.timeout must be at least 1 second".into());
+        }
+        const VALID_TOKEN_PARAMS: &[&str] = &[
+            "auto",
+            "max_tokens",
+            "max_output_tokens",
+            "max_completion_tokens",
+        ];
+        if !VALID_TOKEN_PARAMS.contains(&self.max_tokens_param.as_str()) {
+            errs.push(format!(
+                "llm.max_tokens_param must be one of {:?}, got: {}",
+                VALID_TOKEN_PARAMS, self.max_tokens_param
+            ));
+        }
+
+        // ── response format ──
+        const VALID_RESPONSE_FORMATS: &[&str] = &["none", "json_object"];
+        if !VALID_RESPONSE_FORMATS.contains(&self.response_format.as_str()) {
+            errs.push(format!(
+                "review.response_format must be one of {:?}, got: {}",
+                VALID_RESPONSE_FORMATS, self.response_format
+            ));
+        }
+
+        // ── output ──
+        const VALID_OUTPUT_FORMATS: &[&str] = &["pretty", "json", "compact", "sarif"];
+        if !VALID_OUTPUT_FORMATS.contains(&self.output.format.as_str()) {
+            errs.push(format!(
+                "output.format must be one of {:?}, got: {}",
+                VALID_OUTPUT_FORMATS, self.output.format
+            ));
+        }
+
+        // ── hook ──
+        const VALID_HOOK_MODES: &[&str] = &["warn", "block"];
+        if !VALID_HOOK_MODES.contains(&self.hook.mode.as_str()) {
+            errs.push(format!(
+                "hook.mode must be one of {:?}, got: {}",
+                VALID_HOOK_MODES, self.hook.mode
+            ));
+        }
+        const VALID_VIOLATION: &[&str] = &["warn", "disallow"];
+        if !VALID_VIOLATION.contains(&self.hook.on_violation.as_str()) {
+            errs.push(format!(
+                "hook.on_violation must be one of {:?}, got: {}",
+                VALID_VIOLATION, self.hook.on_violation
+            ));
+        }
+        const VALID_SEVERITIES: &[&str] = &["critical", "major", "minor", "info"];
+        if !VALID_SEVERITIES.contains(&self.hook.min_severity.to_lowercase().as_str()) {
+            errs.push(format!(
+                "hook.min_severity must be one of {:?}, got: {}",
+                VALID_SEVERITIES, self.hook.min_severity
+            ));
+        }
+
+        // ── profile (#81) ──
+        if let Some(profile) = &self.profile {
+            if let Err(pe) = profile.validate() {
+                errs.push(pe);
+            }
+        }
+
+        if errs.is_empty() {
+            Ok(())
+        } else {
+            Err(CoraError::ConfigParse(format!(
+                "invalid configuration:\n  - {}",
+                errs.join("\n  - ")
+            )))
+        }
+    }
+}
+
 /// Serde-compatible schema for the `.cora.yaml` configuration file.
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(deny_unknown_fields)]
 pub struct CoraFile {
     #[serde(
         default,
@@ -190,6 +303,7 @@ pub struct CoraFile {
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(deny_unknown_fields)]
 pub struct ProviderSection {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub provider: Option<String>,
@@ -223,6 +337,7 @@ where
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(deny_unknown_fields)]
 pub struct IgnoreSection {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub files: Option<Vec<String>>,
@@ -231,6 +346,7 @@ pub struct IgnoreSection {
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(deny_unknown_fields)]
 pub struct HookSection {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub mode: Option<String>,
@@ -243,6 +359,7 @@ pub struct HookSection {
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(deny_unknown_fields)]
 pub struct OutputSection {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub format: Option<String>,
@@ -252,6 +369,7 @@ pub struct OutputSection {
 
 /// Review-specific configuration section.
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(deny_unknown_fields)]
 pub struct ReviewSection {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub response_format: Option<String>,
@@ -286,6 +404,7 @@ fn is_default<T: Default + PartialEq>(val: &T) -> bool {
 
 /// Scan-specific configuration section.
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(deny_unknown_fields)]
 pub struct ScanSection {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub system_prompt: Option<String>,
@@ -295,6 +414,7 @@ pub struct ScanSection {
 
 /// LLM-specific configuration section (temperature, `max_tokens`, timeout, `cache_ttl`).
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(deny_unknown_fields)]
 pub struct LlmSection {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub temperature: Option<f32>,
@@ -317,6 +437,7 @@ fn default_max_findings() -> usize {
 
 /// Rule engine configuration section for `.cora.yaml`.
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(deny_unknown_fields)]
 pub struct RulesSection {
     #[serde(default, skip_serializing_if = "is_default")]
     pub enabled: bool,
@@ -328,6 +449,7 @@ pub struct RulesSection {
 
 /// File bundling configuration section for `.cora.yaml`.
 #[derive(Debug, Clone, Serialize, Deserialize, Default)]
+#[serde(deny_unknown_fields)]
 pub struct BundlingSection {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub max_chars_per_group: Option<usize>,
@@ -1345,4 +1467,80 @@ bundling:
         assert_eq!(back.max_tokens_param, Some("max_output_tokens".to_string()));
         assert_eq!(back.max_tokens, Some(8192));
     }
+
+    // ─── #94: Config::validate ───
+
+    #[test]
+    fn validate_accepts_default_config() {
+        assert!(Config::default().validate().is_ok());
+    }
+
+    #[test]
+    fn validate_rejects_out_of_range_temperature() {
+        let cfg = Config {
+            temperature: 5.0,
+            ..Default::default()
+        };
+        let err = cfg.validate().unwrap_err().to_string();
+        assert!(err.contains("temperature"), "err: {err}");
+    }
+
+    #[test]
+    fn validate_rejects_invalid_output_format() {
+        let mut cfg = Config::default();
+        cfg.output.format = "prety".to_string(); // typo
+        let err = cfg.validate().unwrap_err().to_string();
+        assert!(err.contains("output.format"), "err: {err}");
+    }
+
+    #[test]
+    fn validate_rejects_invalid_max_tokens_param() {
+        let cfg = Config {
+            max_tokens_param: "tokens".to_string(),
+            ..Default::default()
+        };
+        assert!(cfg.validate().is_err());
+    }
+
+    #[test]
+    fn validate_rejects_invalid_base_url() {
+        let mut cfg = Config::default();
+        cfg.provider.base_url = "api.openai.com".to_string(); // missing scheme
+        let err = cfg.validate().unwrap_err().to_string();
+        assert!(err.contains("base_url"), "err: {err}");
+    }
+
+    #[test]
+    fn validate_aggregates_multiple_errors() {
+        let cfg = Config {
+            temperature: 9.0,
+            timeout: 0,
+            ..Default::default()
+        };
+        let err = cfg.validate().unwrap_err().to_string();
+        assert!(err.contains("temperature"), "err: {err}");
+        assert!(err.contains("timeout"), "err: {err}");
+    }
+
+    // ─── #80: deny_unknown_fields ───
+
+    #[test]
+    fn deny_unknown_fields_rejects_top_level_typo() {
+        let yaml = "\nquailty_gate:\n  enabled: true\n";
+        let result = CoraFile::from_str(yaml);
+        assert!(result.is_err(), "misspelled top-level key must be rejected");
+    }
+
+    #[test]
+    fn deny_unknown_fields_rejects_section_typo() {
+        let yaml = "\nllm:\n  temprature: 0.5\n";
+        let result = CoraFile::from_str(yaml);
+        assert!(result.is_err(), "misspelled section key must be rejected");
+    }
+
+    #[test]
+    fn deny_unknown_fields_allows_valid_keys() {
+        let yaml = "\nllm:\n  temperature: 0.5\n  max_tokens: 8192\n";
+        assert!(CoraFile::from_str(yaml).is_ok());
+    }
 }