store: make the database, its WAL and lease files owner-only

crates/cortexkit-lease/src/lib.rs

@@ -35,6 +35,55 @@ use std::{
 
 use fs2::FileExt;
 
+/// Force owner-only permissions on a file this process owns the lifecycle of.
+///
+/// Files created through `File::create` or `OpenOptions::create` get their mode
+/// from the process umask, which on a default system means `0644` — readable by
+/// every other account and, more to the point, by every other process running
+/// as this user. That is the exposure that actually exists on a
+/// single-account machine: every module, every worker, every tool, plus
+/// anything that copies the tree (a backup, a restore, an `install` into a
+/// shared location, a container bind-mount).
+///
+/// Applied on OPEN rather than only at creation, because a file that already
+/// exists carries whatever mode it was given — by an older build, a copy, or a
+/// restore. A creation-time-only fix protects exactly the installations with no
+/// history and leaves the ones that matter permissive forever.
+///
+/// A path that is not a regular file is REFUSED rather than adjusted. Following
+/// a symlink here would chmod a file the caller never named, which is a
+/// privilege-escalation primitive wearing a hardening step's clothes.
+///
+/// A missing file is not an error: callers pass optional sidecars (a WAL that
+/// exists only while the journal is active) and the absence of a file is
+/// nothing to protect.
+pub fn protect_file(path: &std::path::Path) -> std::io::Result<()> {
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+        let metadata = match std::fs::symlink_metadata(path) {
+            Ok(metadata) => metadata,
+            Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(()),
+            Err(error) => return Err(error),
+        };
+        if !metadata.is_file() {
+            return Err(std::io::Error::new(
+                std::io::ErrorKind::InvalidInput,
+                format!(
+                    "{} is not a regular file; refusing to change its permissions",
+                    path.display()
+                ),
+            ));
+        }
+        if metadata.permissions().mode() & 0o777 != 0o600 {
+            std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?;
+        }
+    }
+    #[cfg(not(unix))]
+    let _ = path;
+    Ok(())
+}
+
 /// Identifies the thing being single-writer-guarded, namespaced so distinct
 /// modules cannot collide on a shared lease root.
 ///
@@ -196,6 +245,11 @@ impl LeaseStore for FileLeaseStore {
             .truncate(false)
             .open(&path)
             .map_err(LeaseError::Io)?;
+        // The lease is the single-writer fence, so a world-WRITABLE lease file is
+        // an integrity question rather than a privacy one: anything able to write
+        // the persisted epoch can forge the fence token that readers use to
+        // detect a stale writer.
+        protect_file(&path).map_err(LeaseError::Io)?;
 
         // Liveness gate: a live holder still owns the lock, so the try-lock fails
         // with the OS "contended" error.
@@ -230,6 +284,7 @@ impl LeaseStore for FileLeaseStore {
             .truncate(false)
             .open(&path)
             .map_err(LeaseError::Io)?;
+        protect_file(&path).map_err(LeaseError::Io)?;
 
         // Shared liveness gate: only an exclusive holder contends; other shared
         // holders coexist. On unix this is flock(LOCK_SH|LOCK_NB); on Windows,
@@ -323,6 +378,105 @@ mod tests {
         (FileLeaseStore::new(&dir), dir)
     }
 
+    /// An acquired lease file is owner-only on disk, including one that already
+    /// exists with a permissive mode.
+    ///
+    /// The pre-existing half is the case that bites: every lease already on a
+    /// deployed machine was created at the umask default, so correcting only at
+    /// creation time would protect exactly the installations with no history.
+    ///
+    /// The lease is the single-writer FENCE, so the exposure here is integrity
+    /// rather than privacy — anything able to write the persisted epoch can
+    /// forge the token readers use to detect a stale writer.
+    ///
+    /// Mutation-proved: removing the `protect_file` call from `acquire` fails
+    /// this on the pre-existing case.
+    #[cfg(unix)]
+    #[test]
+    fn an_acquired_lease_file_is_owner_only() {
+        use std::os::unix::fs::PermissionsExt;
+
+        let (store, dir) = tmp_store();
+        let k = key("perm");
+
+        // Reproduce a lease left behind by an older build at the umask default.
+        std::fs::create_dir_all(&dir).expect("create lease dir");
+        let path = store.lease_path(&k);
+        std::fs::write(&path, b"").expect("pre-create lease file");
+        std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o644))
+            .expect("set permissive mode");
+
+        let guard = store.acquire(&k).expect("acquire");
+        let mode = std::fs::metadata(&path)
+            .expect("stat lease")
+            .permissions()
+            .mode()
+            & 0o777;
+        assert_eq!(
+            mode, 0o600,
+            "the lease file stayed group/world writable at {mode:o}"
+        );
+
+        drop(guard);
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
+    /// `protect_file` refuses a symlink rather than following it.
+    ///
+    /// Following one would change the mode of a file the caller never named,
+    /// which is a privilege-escalation primitive wearing a hardening step's
+    /// clothes. The assertion is that the TARGET's mode is unchanged, not
+    /// merely that an error came back — an implementation could chmod the
+    /// target and still return Err.
+    #[cfg(unix)]
+    #[test]
+    fn protect_file_refuses_a_symlink_and_leaves_its_target_untouched() {
+        use std::os::unix::fs::PermissionsExt;
+
+        let dir = std::env::temp_dir().join(format!(
+            "cortexkit-lease-symlink-{}-{}",
+            std::process::id(),
+            fnv1a_hex(&format!("{:?}", std::time::Instant::now()))
+        ));
+        std::fs::create_dir_all(&dir).expect("create dir");
+        let target = dir.join("target");
+        std::fs::write(&target, b"not mine").expect("write target");
+        std::fs::set_permissions(&target, std::fs::Permissions::from_mode(0o644))
+            .expect("set target mode");
+        let link = dir.join("link");
+        std::os::unix::fs::symlink(&target, &link).expect("symlink");
+
+        assert!(
+            protect_file(&link).is_err(),
+            "a symlink must be refused rather than followed"
+        );
+        let mode = std::fs::metadata(&target)
+            .expect("stat target")
+            .permissions()
+            .mode()
+            & 0o777;
+        assert_eq!(
+            mode, 0o644,
+            "the symlink target was chmod-ed through the link"
+        );
+
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
+    /// A path that does not exist is not an error: callers pass optional
+    /// sidecars (a WAL that exists only while the journal is active), and the
+    /// absence of a file is nothing to protect. Without this, a first open of a
+    /// fresh database would fail on its missing WAL.
+    #[test]
+    fn protect_file_ignores_a_missing_path() {
+        let missing = std::env::temp_dir().join(format!(
+            "cortexkit-lease-absent-{}-{}",
+            std::process::id(),
+            fnv1a_hex(&format!("{:?}", std::time::Instant::now()))
+        ));
+        assert!(protect_file(&missing).is_ok());
+    }
+
     #[test]
     fn acquire_then_second_holder_is_rejected() {
         let (store, dir) = tmp_store();

crates/cortexkit-store/src/lib.rs

@@ -102,7 +102,7 @@ mod sqlite_backend {
         time::{Duration, SystemTime, UNIX_EPOCH},
     };
 
-    use cortexkit_lease::{FileLeaseStore, LeaseHandle};
+    use cortexkit_lease::{protect_file, FileLeaseStore, LeaseHandle};
     use rusqlite::Connection;
 
     /// A lease-guarded, migrated sqlite store. Holds the single-writer lease for
@@ -291,6 +291,34 @@ mod sqlite_backend {
         conn.pragma_update(None, "foreign_keys", "ON")
             .map_err(|e| StoreError::Backend(e.to_string()))?;
 
+        // Owner-only, decided here rather than left to the caller's umask.
+        //
+        // SQLite sets no mode of its own, so without this the shipped default is
+        // world-readable `0644` — measured across every module store on a real
+        // deployment. A crate that already decides WAL mode, busy timeout and
+        // foreign keys has taken responsibility for how this file behaves on
+        // disk; leaving permissions to callers means the decision is made by the
+        // ambient umask, which is to say not made at all.
+        //
+        // The WAL and SHM siblings are the half that gets missed. Recently
+        // committed rows live in the WAL until a checkpoint, so protecting only
+        // the database file leaves the NEWEST data permissive while the database
+        // itself reads as correct — and on real hosts the WAL is routinely
+        // larger than the database.
+        //
+        // Ordering: after the pragma that ENABLES WAL, so the sibling files
+        // exist to be protected on a first open rather than being created
+        // unprotected immediately afterwards.
+        //
+        // A group-readable store is not a configuration this crate supports: it
+        // hands out an exclusive single-writer lease, so an out-of-band reader
+        // is already outside the contract. That need is a read replica or an
+        // export operation, not a looser file mode.
+        for suffix in ["", "-wal", "-shm"] {
+            protect_file(Path::new(&format!("{path}{suffix}")))
+                .map_err(|e| StoreError::Backend(e.to_string()))?;
+        }
+
         Ok(SqliteStore {
             conn: Mutex::new(conn),
             epoch,
@@ -371,6 +399,93 @@ pub use sqlite_backend::{open_sqlite, SqliteStore};
 mod tests {
     use super::*;
 
+    /// The database file AND its WAL sibling are owner-only on disk after
+    /// `open_sqlite`, including a database that already exists permissively.
+    ///
+    /// Two properties, asserted separately because they fail independently.
+    /// SQLite sets no mode, so without this the shipped default is `0644`.
+    /// And the WAL is the half that gets missed: recently committed rows live
+    /// there until a checkpoint, so protecting only the database leaves the
+    /// NEWEST data readable while the database file itself looks correct.
+    ///
+    /// Asserts the mode ON DISK rather than that `open_sqlite` returned Ok — a
+    /// hardening step that silently did nothing would still return Ok.
+    ///
+    /// The scenario is REOPENING an already-deployed store, which is the only
+    /// shape in which the WAL assertion means anything. A first open cannot
+    /// exercise it: SQLite creates a fresh WAL inheriting the database's mode,
+    /// so by then the database is already `0600` and the WAL follows for free.
+    /// A permissive WAL only exists because a PREVIOUS process wrote one under
+    /// the old umask — exactly the state every deployed machine is in.
+    ///
+    /// Mutation-proved, both suffixes independently: dropping `""` fails the
+    /// database assertion, dropping `"-wal"` fails the WAL assertion, and
+    /// neither is carried by the other. An earlier version of this test used a
+    /// first open and the `"-wal"` mutation SURVIVED it — the assertion was
+    /// there, it just could not fail.
+    #[cfg(unix)]
+    #[test]
+    fn reopening_a_permissive_store_protects_the_database_and_its_wal() {
+        use std::os::unix::fs::PermissionsExt;
+
+        let (root, descriptor) = tmp();
+        let StorageBackend::Sqlite { path } = &descriptor.backend else {
+            panic!("sqlite descriptor");
+        };
+        let path = std::path::PathBuf::from(path);
+        let wal = std::path::PathBuf::from(format!("{}-wal", path.display()));
+
+        // First open: create a real database, then close it.
+        {
+            let store = open_sqlite(&descriptor).expect("first open");
+            store
+                .migrate(
+                    "perm",
+                    &[Migration {
+                        version: 1,
+                        statements: "CREATE TABLE t (k TEXT);",
+                    }],
+                )
+                .expect("migrate");
+        }
+
+        // A clean close checkpoints and REMOVES the WAL, so one has to be put
+        // back deliberately. That is not artificial: a WAL surviving on disk is
+        // precisely what an unclean shutdown leaves behind, and it is the only
+        // state in which a permissive WAL can be waiting at open time.
+        std::fs::write(&wal, b"").expect("leave a WAL behind");
+
+        // Reproduce the deployed state: both files permissive, as every store
+        // created before this hardening actually is on disk.
+        for file in [&path, &wal] {
+            std::fs::set_permissions(file, std::fs::Permissions::from_mode(0o644))
+                .expect("set permissive mode");
+        }
+
+        let store = open_sqlite(&descriptor).expect("reopen");
+
+        let mode = |p: &std::path::Path| {
+            std::fs::metadata(p)
+                .unwrap_or_else(|error| panic!("stat {}: {error}", p.display()))
+                .permissions()
+                .mode()
+                & 0o777
+        };
+        assert_eq!(
+            mode(&path),
+            0o600,
+            "the database stayed group/world readable on reopen"
+        );
+        assert_eq!(
+            mode(&wal),
+            0o600,
+            "the WAL stayed group/world readable while the database looked correct"
+        );
+
+        drop(store);
+        let _ = std::fs::remove_dir_all(&root);
+    }
+
     /// A unique temp root + a descriptor whose sqlite file lives under it. The
     /// lease is derived from the db path (its parent), so no separate lease dir.
     fn tmp() -> (std::path::PathBuf, StorageDescriptor) {