Harden snapshot store admission

Author: eldryoth

CHANGELOG.md

@@ -54,6 +54,13 @@ behavior when the change improves security or project direction.
   for every byte received from a trusted proxy.
 - Refresh stream idle deadlines after every successful partial write so active
   backpressured connections are not terminated between write progress events.
+- Reject filesystem roots and pre-existing non-private snapshot directories
+  without changing their permissions; only newly created dedicated snapshot
+  directories are initialized as `0700`.
+- Reject serialized snapshots above the reader's 16 MiB limit before store
+  locking, generation allocation, or publication.
+- Bound persisted self-healing state to 64 KiB and its variable diagnostics to
+  4 KiB without control characters, using the same limit for reads and writes.
 
 ## 1.7.11 - 2026-07-14
 

crates/fluxheim-config/src/config_admin.rs

@@ -14,7 +14,9 @@ pub use crate::config_admin_transport::{
     AdminClientCertificateConfig, AdminClientCertificateConfigFragment, AdminRemoteTransportMode,
     AdminTransportConfig, AdminTransportConfigFragment,
 };
-use crate::config_path::{validate_non_world_writable_parent, validate_path};
+use crate::config_path::{
+    validate_non_world_writable_parent, validate_path, validate_private_state_directory,
+};
 
 #[derive(Debug, Clone, Eq, PartialEq, Deserialize, Serialize)]
 #[serde(deny_unknown_fields)]
@@ -167,6 +169,7 @@ impl AdminConfig {
         )?;
         validate_path("admin.token_file", self.token_file.as_deref())?;
         validate_path("admin.snapshot_store", self.snapshot_store.as_deref())?;
+        validate_private_state_directory("admin.snapshot_store", self.snapshot_store.as_deref())?;
         validate_path(
             "admin.snapshot_integrity_key_file",
             self.snapshot_integrity_key_file.as_deref(),

crates/fluxheim-config/src/config_path.rs

@@ -1,3 +1,4 @@
+use std::path::Component;
 use std::path::{Path, PathBuf};
 
 use crate::config::ConfigError;
@@ -63,6 +64,56 @@ pub fn validate_non_world_writable_parent(
     Ok(())
 }
 
+pub fn validate_private_state_directory(
+    field: impl Into<String>,
+    path: Option<&Path>,
+) -> Result<(), ConfigError> {
+    let field = field.into();
+    let Some(path) = path else {
+        return Ok(());
+    };
+    if path_is_filesystem_root(path) {
+        return Err(ConfigError::UnsafePath {
+            field,
+            path: path.to_path_buf(),
+        });
+    }
+
+    let metadata = match path.symlink_metadata() {
+        Ok(metadata) => metadata,
+        Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(()),
+        Err(error) => return Err(path_inspection_failed(field, path, error)),
+    };
+    if metadata.file_type().is_symlink() || !metadata.is_dir() {
+        return Err(ConfigError::UnsafePath {
+            field,
+            path: path.to_path_buf(),
+        });
+    }
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt as _;
+        if metadata.permissions().mode() & 0o077 != 0 {
+            return Err(ConfigError::UnsafePath {
+                field,
+                path: path.to_path_buf(),
+            });
+        }
+    }
+    Ok(())
+}
+
+fn path_is_filesystem_root(path: &Path) -> bool {
+    let mut components = path.components();
+    match components.next() {
+        Some(Component::RootDir) => components.next().is_none(),
+        Some(Component::Prefix(_)) => {
+            matches!(components.next(), Some(Component::RootDir)) && components.next().is_none()
+        }
+        _ => false,
+    }
+}
+
 pub fn validate_optional_process_path(
     field: &'static str,
     path: Option<&Path>,

crates/fluxheim-config/src/config_tests_admin_security.rs

@@ -148,6 +148,38 @@ fn rejects_admin_paths_under_world_writable_parent() {
     ));
 }
 
+#[cfg(unix)]
+#[test]
+fn rejects_existing_non_private_or_root_snapshot_store() {
+    use std::os::unix::fs::PermissionsExt as _;
+
+    let snapshot_store = secure_test_dir("config-admin-snapshot-non-private");
+    std::fs::set_permissions(&snapshot_store, std::fs::Permissions::from_mode(0o755)).unwrap();
+    let config = Config {
+        admin: AdminConfig {
+            snapshot_store: Some(snapshot_store),
+            ..AdminConfig::default()
+        },
+        ..Config::default()
+    };
+    assert!(matches!(
+        config.validate(),
+        Err(ConfigError::UnsafePath { field, .. }) if field == "admin.snapshot_store"
+    ));
+
+    let root = Config {
+        admin: AdminConfig {
+            snapshot_store: Some("/".into()),
+            ..AdminConfig::default()
+        },
+        ..Config::default()
+    };
+    assert!(matches!(
+        root.validate(),
+        Err(ConfigError::UnsafePath { field, .. }) if field == "admin.snapshot_store"
+    ));
+}
+
 #[test]
 fn rejects_invalid_admin_self_healing_window() {
     let config = Config {

crates/fluxheim-snapshot/src/operations_tests.rs

@@ -2,6 +2,7 @@ use fluxheim_common::test_support::{safe_child_path, unique_temp_path};
 use fluxheim_config::{Config, ProxyConfig};
 use std::sync::{Arc, Barrier};
 
+use crate::recovery::{MAX_RUNTIME_DIAGNOSTIC_BYTES, MAX_RUNTIME_STATE_BYTES};
 use crate::store::MAX_SNAPSHOT_STORE_ENTRIES;
 #[cfg(unix)]
 use crate::store_fs::set_open_private_file_mode_for_test;
@@ -50,6 +51,7 @@ fn concurrent_snapshot_creation_cannot_exceed_capacity() {
     let dir = TestDir::new("snapshot-concurrent-capacity");
     let configs = dir.child("configs");
     std::fs::create_dir(&configs).unwrap();
+    set_private_test_directory(&configs);
     for index in 0..MAX_SNAPSHOT_STORE_ENTRIES - 1 {
         std::fs::write(
             safe_child_path(&configs, &format!("legacy-{index:04}.toml")),
@@ -378,6 +380,61 @@ fn self_healing_state_persists_pending_and_failed_rollback() {
     assert!(restored_pending.last_rollback_failure.is_some());
 }
 
+#[test]
+fn recovery_diagnostic_limits_preserve_previous_state() {
+    let dir = TestDir::new("snapshot-recovery-diagnostic-limit");
+    let store = SnapshotStore::new(dir.path());
+    let snapshot = store.snapshot_config(&Config::default(), None).unwrap();
+    let valid = SnapshotRuntimeState {
+        runtime_snapshot: Some(snapshot.id.clone()),
+        known_good_snapshot: Some(snapshot.id.clone()),
+        pending_validation: None,
+    };
+    store.save_runtime_state(&valid).unwrap();
+    let state_path = store.runtime_state_path();
+    let before = std::fs::read(&state_path).unwrap();
+
+    for (impact, failure) in [
+        ("x".repeat(MAX_RUNTIME_DIAGNOSTIC_BYTES + 1), None),
+        ("invalid\nimpact".to_owned(), None),
+        (
+            "snapshot".to_owned(),
+            Some("x".repeat(MAX_RUNTIME_DIAGNOSTIC_BYTES + 1)),
+        ),
+    ] {
+        let invalid = SnapshotRuntimeState {
+            runtime_snapshot: Some(snapshot.id.clone()),
+            known_good_snapshot: Some(snapshot.id.clone()),
+            pending_validation: Some(PendingValidation {
+                target_snapshot: snapshot.id.clone(),
+                previous_snapshot: Some(snapshot.id.clone()),
+                impact,
+                expires_unix_secs: 100,
+                successful_checks: 0,
+                failed_checks: 0,
+                rollback_attempts: 0,
+                last_rollback_failure: failure,
+            }),
+        };
+        assert!(store.save_runtime_state(&invalid).is_err());
+        assert_eq!(std::fs::read(&state_path).unwrap(), before);
+    }
+}
+
+#[test]
+fn recovery_reader_rejects_file_above_dedicated_limit() {
+    let dir = TestDir::new("snapshot-recovery-file-limit");
+    let store = SnapshotStore::new(dir.path());
+    store.snapshot_config(&Config::default(), None).unwrap();
+    write_atomically(
+        &store.runtime_state_path(),
+        &vec![b'x'; (MAX_RUNTIME_STATE_BYTES + 1) as usize],
+    )
+    .unwrap();
+
+    assert!(store.load_runtime_state().is_err());
+}
+
 #[test]
 fn failed_create_new_atomic_write_removes_temporary_file() {
     let dir = TestDir::new("snapshot-temp-cleanup");
@@ -451,6 +508,7 @@ impl TestDir {
     fn new(name: &str) -> Self {
         let path = unique_temp_path(name);
         std::fs::create_dir(&path).unwrap();
+        set_private_test_directory(&path);
         Self { path }
     }
 
@@ -463,6 +521,16 @@ impl TestDir {
     }
 }
 
+fn set_private_test_directory(path: &std::path::Path) {
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt as _;
+        std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o700)).unwrap();
+    }
+    #[cfg(not(unix))]
+    let _ = path;
+}
+
 impl Drop for TestDir {
     fn drop(&mut self) {
         let _ = std::fs::remove_dir_all(&self.path);

crates/fluxheim-snapshot/src/recovery.rs

@@ -1,9 +1,12 @@
 use crate::metadata::validate_snapshot_id;
 use crate::state::SnapshotRuntimeState;
 use crate::store::{SnapshotError, SnapshotStore};
-use crate::store_fs::{read_optional_regular_file_to_string, write_atomically};
+use crate::store_fs::{read_optional_regular_file_to_string_with_limit, write_atomically};
 use serde::{Deserialize, Serialize};
 
+pub(crate) const MAX_RUNTIME_STATE_BYTES: u64 = 64 * 1024;
+pub(crate) const MAX_RUNTIME_DIAGNOSTIC_BYTES: usize = 4096;
+
 #[derive(Debug, Deserialize, Serialize)]
 #[serde(deny_unknown_fields)]
 struct PersistedRuntimeState {
@@ -17,11 +20,16 @@ struct PersistedRuntimeState {
 impl SnapshotStore {
     pub fn load_runtime_state(&self) -> Result<Option<SnapshotRuntimeState>, SnapshotError> {
         self.validate_for_recovery()?;
-        let Some(raw) = read_optional_regular_file_to_string(&self.runtime_state_path())? else {
+        let Some(raw) = read_optional_regular_file_to_string_with_limit(
+            &self.runtime_state_path(),
+            MAX_RUNTIME_STATE_BYTES,
+        )?
+        else {
             return Ok(None);
         };
         let persisted: PersistedRuntimeState =
             toml::from_str(&raw).map_err(SnapshotError::Decode)?;
+        require_runtime_state_size(&persisted.state_toml)?;
         match self.integrity.as_deref() {
             Some(key)
                 if persisted.key_id.as_deref() == Some(key.key_id())
@@ -41,6 +49,7 @@ impl SnapshotStore {
     pub fn save_runtime_state(&self, state: &SnapshotRuntimeState) -> Result<(), SnapshotError> {
         validate_runtime_state(state)?;
         let state_toml = toml::to_string_pretty(state).map_err(SnapshotError::Encode)?;
+        require_runtime_state_size(&state_toml)?;
         let persisted = match self.integrity.as_deref() {
             Some(key) => PersistedRuntimeState {
                 hmac_sha256: Some(key.sign_recovery(state_toml.as_bytes())?),
@@ -54,6 +63,7 @@ impl SnapshotStore {
             },
         };
         let raw = toml::to_string_pretty(&persisted).map_err(SnapshotError::Encode)?;
+        require_runtime_state_size(&raw)?;
         self.with_store_lock(|| write_atomically(&self.runtime_state_path(), raw.as_bytes()))
     }
 
@@ -84,5 +94,37 @@ fn validate_runtime_state(state: &SnapshotRuntimeState) -> Result<(), SnapshotEr
     {
         validate_snapshot_id(id)?;
     }
+    if let Some(pending) = state.pending_validation.as_ref() {
+        validate_runtime_diagnostic(&pending.impact)?;
+        if let Some(failure) = pending.last_rollback_failure.as_deref() {
+            validate_runtime_diagnostic(failure)?;
+        }
+    }
+    Ok(())
+}
+
+fn validate_runtime_diagnostic(value: &str) -> Result<(), SnapshotError> {
+    if value.len() > MAX_RUNTIME_DIAGNOSTIC_BYTES || value.chars().any(char::is_control) {
+        return Err(SnapshotError::Io(std::io::Error::new(
+            std::io::ErrorKind::InvalidInput,
+            "snapshot recovery diagnostic is invalid or exceeds its configured limit",
+        )));
+    }
+    Ok(())
+}
+
+fn require_runtime_state_size(raw: &str) -> Result<(), SnapshotError> {
+    let length = u64::try_from(raw.len()).map_err(|_| {
+        SnapshotError::Io(std::io::Error::new(
+            std::io::ErrorKind::InvalidInput,
+            "serialized snapshot recovery state length overflowed",
+        ))
+    })?;
+    if length > MAX_RUNTIME_STATE_BYTES {
+        return Err(SnapshotError::Io(std::io::Error::new(
+            std::io::ErrorKind::InvalidInput,
+            format!("serialized snapshot recovery state exceeds {MAX_RUNTIME_STATE_BYTES} bytes"),
+        )));
+    }
     Ok(())
 }

crates/fluxheim-snapshot/src/snapshot_hardening_tests.rs

@@ -461,6 +461,7 @@ impl TestDir {
     fn new(name: &str) -> Self {
         let path = unique_temp_path(name);
         std::fs::create_dir(&path).unwrap();
+        set_private_test_directory(&path);
         Self(path)
     }
 
@@ -469,6 +470,16 @@ impl TestDir {
     }
 }
 
+fn set_private_test_directory(path: &std::path::Path) {
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt as _;
+        std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o700)).unwrap();
+    }
+    #[cfg(not(unix))]
+    let _ = path;
+}
+
 impl Drop for TestDir {
     fn drop(&mut self) {
         let _ = std::fs::remove_dir_all(&self.0);

crates/fluxheim-snapshot/src/snapshot_upgrade_tests.rs

@@ -142,6 +142,7 @@ impl TestDir {
     fn new(name: &str) -> Self {
         let path = unique_temp_path(name);
         std::fs::create_dir(&path).unwrap();
+        set_private_test_directory(&path);
         Self(path)
     }
 
@@ -150,6 +151,16 @@ impl TestDir {
     }
 }
 
+fn set_private_test_directory(path: &std::path::Path) {
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt as _;
+        std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o700)).unwrap();
+    }
+    #[cfg(not(unix))]
+    let _ = path;
+}
+
 impl Drop for TestDir {
     fn drop(&mut self) {
         let _ = std::fs::remove_dir_all(&self.0);