🐛 fix(config): reject negative cache timing (#1785)

Author: gaborbernat

crates/peryx/src/config/mod.rs

@@ -92,4 +92,6 @@ pub enum ConfigError {
     Tls { reason: &'static str },
     #[error("invalid environment variable {var}: {reason}")]
     Env { var: &'static str, reason: String },
+    #[error("`{field}` must be non-negative, got {value}")]
+    CacheTiming { field: &'static str, value: i64 },
 }

crates/peryx/src/config/model.rs

@@ -365,9 +365,17 @@ impl Config {
     }
 
     /// # Errors
-    /// Returns an error when plugin authentication, identity providers, jobs, durability, or writer
-    /// identity conflict with the resolved configuration.
+    /// Returns an error when cache timing, plugin authentication, identity providers, jobs,
+    /// durability, or writer identity conflict with the resolved configuration.
     pub fn validate_with_plugins(&self, plugins: &peryx_plugin_registry::PluginRegistry) -> Result<(), ConfigError> {
+        for (field, value) in [
+            ("cache_ttl_secs", self.cache_ttl_secs),
+            ("max_stale_secs", self.max_stale_secs),
+        ] {
+            if value < 0 {
+                return Err(ConfigError::CacheTiming { field, value });
+            }
+        }
         let plugin_indexes = self
             .indexes
             .iter()

crates/peryx/tests/cli_entrypoint.rs

@@ -370,6 +370,21 @@ fn runtime_command_rejects_an_invalid_host(#[case] command: &[&str]) {
     assert_failure(&output, "`host` \"not a host\"");
 }
 
+#[rstest]
+#[case::serve_freshness(&["serve"], "PERYX_CACHE_TTL_SECS", "cache_ttl_secs")]
+#[case::serve_stale_bound(&["serve"], "PERYX_MAX_STALE_SECS", "max_stale_secs")]
+#[case::check_freshness(&["config", "check"], "PERYX_CACHE_TTL_SECS", "cache_ttl_secs")]
+#[case::check_stale_bound(&["config", "check"], "PERYX_MAX_STALE_SECS", "max_stale_secs")]
+fn runtime_commands_reject_negative_cache_timing(
+    #[case] command: &[&str],
+    #[case] variable: &str,
+    #[case] field: &str,
+) {
+    let output = peryx().args(command).env(variable, "-1").output().unwrap();
+
+    assert_failure(&output, &format!("`{field}` must be non-negative, got -1"));
+}
+
 #[test]
 fn runtime_arguments_override_environment_and_selected_config() {
     let dir = tempfile::tempdir().unwrap();

crates/peryx/tests/unit/tests/config/model_tests.rs

@@ -88,6 +88,36 @@ fn test_default_config() {
     assert_eq!(c.rate_limit, RateLimitConfig::default());
 }
 
+#[rstest::rstest]
+#[case::freshness(-1, 0, "`cache_ttl_secs` must be non-negative, got -1")]
+#[case::stale_bound(0, -2, "`max_stale_secs` must be non-negative, got -2")]
+fn test_config_rejects_negative_cache_timing(
+    #[case] cache_ttl_secs: i64,
+    #[case] max_stale_secs: i64,
+    #[case] expected: &str,
+) {
+    let config = Config {
+        cache_ttl_secs,
+        max_stale_secs,
+        ..Config::default()
+    };
+
+    assert_eq!(config.validate().unwrap_err().to_string(), expected);
+}
+
+#[rstest::rstest]
+#[case::zero(0)]
+#[case::positive(1)]
+fn test_config_accepts_non_negative_cache_timing(#[case] value: i64) {
+    Config {
+        cache_ttl_secs: value,
+        max_stale_secs: value,
+        ..Config::default()
+    }
+    .validate()
+    .unwrap();
+}
+
 #[rstest::rstest]
 #[case::ipv4("127.0.0.1", "127.0.0.1:4433")]
 #[case::ipv6_wildcard("::", "[::]:4433")]

site/content/core/operations/configuration.md

@@ -47,7 +47,8 @@ selected socket address. `serve` passes its selected address to the listener wit
 
 The ceiling limits how long peryx trusts an upstream `Cache-Control` value. An upstream or CDN that answers
 `max-age=31536000` would otherwise pin a page in the cache for a year with no revalidation. Raise `cache_ttl_secs` if
-you want to trust a long upstream lifetime; lower it to revalidate sooner than the upstream asks.
+you want to trust a long upstream lifetime; lower it to revalidate sooner than the upstream asks. Set it to `0` to
+revalidate every request. `cache_ttl_secs` must be non-negative.
 
 Artifacts never expire; they are content-addressed by sha256, so a changed upstream file is a new entry on the page
 rather than a mutation.
@@ -56,7 +57,7 @@ rather than a mutation.
 last page it fetched, but only for this long past the page's freshness window. Beyond it the upstream failure surfaces
 instead, because a cache that answers with whatever it last saw, forever, has stopped being a cache and become a fork.
 Set it to `0` to serve stale without limit, which is what mirroring a knowingly unreliable upstream asks for;
-`offline = true` below is the unconditional form.
+`offline = true` below is the unconditional form. `max_stale_secs` must be non-negative.
 
 `usage_retention_days` bounds the durable
 [daily group and source usage](@/core/operations/monitor.md#daily-group-and-source-usage) aggregate: buckets older than