fix(store): harden DB open against path injection (CodeQL #15)

crates/kindling-store/src/db.rs

@@ -9,7 +9,7 @@
 //! whose `PRAGMA user_version` falls outside the compatibility window in
 //! `schema/version.json` (see `schema/README.md`).
 
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::time::Duration;
 
 use rusqlite::{Connection, OpenFlags};
@@ -44,7 +44,14 @@ pub struct StoreOptions {
 /// Creates parent directories as needed (read-write mode only), configures
 /// the connection, applies the canonical schema to fresh databases, and
 /// verifies schema-version compatibility on existing ones.
+///
+/// `path` is validated by [`validate_db_path`] before any directory is created
+/// or SQLite is opened. The store still accepts any local filesystem location
+/// (CLI `--db`, `KINDLING_DB_PATH`, tests) — it does not confine opens to a
+/// single root — but it rejects traversal segments, NUL, and SQLite URI names.
 pub fn open_database(path: &Path, options: &StoreOptions) -> StoreResult<Connection> {
+    let path = validate_db_path(path)?;
+
     if !options.readonly {
         if let Some(parent) = path.parent() {
             if !parent.as_os_str().is_empty() {
@@ -53,20 +60,84 @@ pub fn open_database(path: &Path, options: &StoreOptions) -> StoreResult<Connect
         }
     }
 
-    let flags = if options.readonly {
-        OpenFlags::SQLITE_OPEN_READ_ONLY
-            | OpenFlags::SQLITE_OPEN_URI
-            | OpenFlags::SQLITE_OPEN_NO_MUTEX
-    } else {
-        OpenFlags::default()
-    };
-
-    let conn = Connection::open_with_flags(path, flags)?;
+    let conn = Connection::open_with_flags(&path, open_flags(options.readonly))?;
     configure(&conn, options.readonly)?;
     ensure_schema(&conn, options.readonly)?;
     Ok(conn)
 }
 
+/// Open flags for a filesystem database file.
+///
+/// Explicitly omits `SQLITE_OPEN_URI` (present on [`OpenFlags::default`]) so a
+/// path cannot be reinterpreted as a SQLite URI (`file:…?mode=…`). `NOFOLLOW`
+/// refuses to open when the final path component is a symlink.
+fn open_flags(readonly: bool) -> OpenFlags {
+    let flags = OpenFlags::SQLITE_OPEN_NO_MUTEX | OpenFlags::SQLITE_OPEN_NOFOLLOW;
+    if readonly {
+        flags | OpenFlags::SQLITE_OPEN_READ_ONLY
+    } else {
+        flags | OpenFlags::SQLITE_OPEN_READ_WRITE | OpenFlags::SQLITE_OPEN_CREATE
+    }
+}
+
+/// Validate a kindling database path before creating directories or opening
+/// SQLite.
+///
+/// Kindling is local-first: `--db` and `KINDLING_DB_PATH` are operator
+/// configuration and may point at any filesystem location. This helper does
+/// **not** confine the path to a single root (that would break legitimate
+/// local config and temp-dir tests). It rejects the cases that turn a path
+/// into an injection:
+///
+/// - empty / non-UTF-8 / embedded NUL
+/// - SQLite URI filenames (`file:…`) and special names (`:memory:`, `:temp:`)
+/// - `..` segments — the CodeQL `rust/path-injection` sanitizer guard
+///
+/// Daemon routing already confines per-project DBs: `X-Kindling-Project` is
+/// hashed to a 12-hex component under `kindling_home/projects/<id>/`.
+pub fn validate_db_path(path: &Path) -> StoreResult<PathBuf> {
+    let Some(raw) = path.to_str() else {
+        return Err(StoreError::InvalidDbPath {
+            path: path.display().to_string(),
+            reason: "path is not valid UTF-8",
+        });
+    };
+    if raw.is_empty() {
+        return Err(StoreError::InvalidDbPath {
+            path: raw.to_string(),
+            reason: "path is empty",
+        });
+    }
+    if raw.contains('\0') {
+        return Err(StoreError::InvalidDbPath {
+            path: raw.to_string(),
+            reason: "path contains a NUL byte",
+        });
+    }
+    if is_sqlite_special_filename(raw) {
+        return Err(StoreError::InvalidDbPath {
+            path: raw.to_string(),
+            reason: "SQLite URI and special filenames are not allowed",
+        });
+    }
+    // Sanitizer guard recognized by CodeQL rust/path-injection (DotDotCheck).
+    // Reconstruct the PathBuf from this checked string so the sink cannot
+    // observe the pre-check value.
+    if raw.contains("..") {
+        return Err(StoreError::InvalidDbPath {
+            path: raw.to_string(),
+            reason: "path must not contain '..' segments",
+        });
+    }
+    Ok(PathBuf::from(raw))
+}
+
+fn is_sqlite_special_filename(raw: &str) -> bool {
+    let trimmed = raw.trim();
+    let lower = trimmed.to_ascii_lowercase();
+    lower.starts_with("file:") || lower == ":memory:" || lower == ":temp:"
+}
+
 /// Open an in-memory database with the full schema applied. Test helper and
 /// scratch-space convenience; never version-gated because it is always fresh.
 pub fn open_in_memory() -> StoreResult<Connection> {

crates/kindling-store/src/error.rs

@@ -39,6 +39,9 @@ pub enum StoreError {
     #[error("database has no schema and the connection is read-only")]
     UninitializedDatabase,
 
+    #[error("invalid database path {path:?}: {reason}")]
+    InvalidDbPath { path: String, reason: &'static str },
+
     #[error("unexpected value {value:?} in column {column}")]
     UnexpectedRowValue { column: &'static str, value: String },
 }

crates/kindling-store/src/lib.rs

@@ -14,7 +14,7 @@ mod paths;
 mod schema;
 mod store;
 
-pub use db::{open_database, open_in_memory, StoreOptions};
+pub use db::{open_database, open_in_memory, validate_db_path, StoreOptions};
 pub use error::{StoreError, StoreResult};
 pub use paths::{default_kindling_home, project_db_path, project_id, resolve_db_path};
 pub use schema::{schema_version, SchemaVersion, SCHEMA_SQL};

crates/kindling-store/src/paths.rs

@@ -23,11 +23,26 @@ pub fn project_id(project_root: &str) -> String {
 
 /// Database path for a project under the given kindling home directory:
 /// `<kindling_home>/projects/<project_id>/kindling.db`.
+///
+/// `project_root` is never used as a path component. It is hashed to a
+/// 12-hex-char id, so values such as `../../../etc/passwd` cannot escape
+/// `kindling_home`.
 pub fn project_db_path(kindling_home: &Path, project_root: &str) -> PathBuf {
-    kindling_home
-        .join("projects")
-        .join(project_id(project_root))
-        .join("kindling.db")
+    let id = project_id(project_root);
+    debug_assert!(
+        is_safe_project_id(&id),
+        "project_id must be a single hex path component, got {id:?}"
+    );
+    kindling_home.join("projects").join(id).join("kindling.db")
+}
+
+/// True when `id` is a single 12-hex path component (no separators, no `..`).
+fn is_safe_project_id(id: &str) -> bool {
+    id.len() == 12
+        && id.bytes().all(|b| b.is_ascii_hexdigit())
+        && !id.contains("..")
+        && !id.contains('/')
+        && !id.contains('\\')
 }
 
 /// Default kindling home (`~/.kindling`), or `None` if no home directory can
@@ -74,6 +89,35 @@ mod tests {
         assert_eq!(path, PathBuf::from(expected));
     }
 
+    #[test]
+    fn project_id_is_a_single_hex_component() {
+        let id = project_id("../../../etc/passwd");
+        assert!(is_safe_project_id(&id), "{id}");
+        assert_ne!(id, project_id("/etc/passwd"));
+    }
+
+    #[test]
+    fn project_db_path_confines_traversal_like_roots() {
+        let home = Path::new("/home/u/.kindling");
+        for root in [
+            "../../../etc/passwd",
+            "/etc/passwd",
+            "foo/../../bar",
+            r"foo\..\bar",
+            "file:/tmp/evil",
+            ":memory:",
+        ] {
+            let path = project_db_path(home, root);
+            assert!(
+                path.starts_with(home),
+                "project root {root:?} escaped {home:?}: {path:?}"
+            );
+            let id = project_id(root);
+            assert!(is_safe_project_id(&id), "{id}");
+            assert_eq!(path, home.join("projects").join(id).join("kindling.db"));
+        }
+    }
+
     /// Reference value produced by the Node.js implementation.
     const NODE_HASH_TMP_EXAMPLE: &str = "f33aa9244af5";
 }

crates/kindling-store/tests/store_integration.rs

@@ -728,6 +728,73 @@ fn concurrent_connections_share_a_wal_database() {
     assert!(writer_b.get_observation_by_id("obs-a").unwrap().is_some());
 }
 
+#[test]
+fn open_creates_nested_parent_directories() {
+    let dir = tempfile::tempdir().unwrap();
+    let path = dir.path().join("nested").join("dir").join("kindling.db");
+    let store = SqliteKindlingStore::open(&path).unwrap();
+    store
+        .insert_observation(&observation("obs-1", "hello", 1000, "sess-1"))
+        .unwrap();
+    assert!(path.exists());
+}
+
+#[test]
+fn open_rejects_parent_dir_segments() {
+    let dir = tempfile::tempdir().unwrap();
+    let traversal = dir.path().join("..").join("outside").join("kindling.db");
+    assert!(
+        traversal.to_string_lossy().contains(".."),
+        "test path must retain a '..' segment: {traversal:?}"
+    );
+    assert!(matches!(
+        SqliteKindlingStore::open(&traversal),
+        Err(StoreError::InvalidDbPath { .. })
+    ));
+    assert!(!dir.path().join("outside").exists());
+}
+
+#[test]
+fn open_rejects_sqlite_uri_and_special_names() {
+    for raw in [
+        "file:/tmp/kindling.db",
+        "file:///tmp/kindling.db",
+        "FILE:kindling.db",
+        "file:kindling.db?mode=rwc",
+        ":memory:",
+        ":MEMORY:",
+        ":temp:",
+    ] {
+        assert!(
+            matches!(
+                SqliteKindlingStore::open(Path::new(raw)),
+                Err(StoreError::InvalidDbPath { .. })
+            ),
+            "expected InvalidDbPath for {raw:?}"
+        );
+    }
+}
+
+#[test]
+fn open_rejects_empty_and_nul_paths() {
+    assert!(matches!(
+        SqliteKindlingStore::open(Path::new("")),
+        Err(StoreError::InvalidDbPath { .. })
+    ));
+    assert!(matches!(
+        SqliteKindlingStore::open(Path::new("kindling\0.db")),
+        Err(StoreError::InvalidDbPath { .. })
+    ));
+}
+
+#[test]
+fn validate_db_path_accepts_ordinary_local_paths() {
+    let dir = tempfile::tempdir().unwrap();
+    let path = dir.path().join("kindling.db");
+    let checked = kindling_store::validate_db_path(&path).unwrap();
+    assert_eq!(checked, path);
+}
+
 #[test]
 fn per_project_paths_isolate_databases() {
     let dir = tempfile::tempdir().unwrap();

crates/kindling/src/commands/init.rs

@@ -53,6 +53,9 @@ struct ClaudeCodeResult {
 
 pub fn run(args: InitArgs) -> CliResult {
     let db_path = resolve_db_path(args.db.as_deref())?;
+    // Validate before mkdir: `--skip-db` never opens the store, but must not
+    // create parent directories from a traversal / SQLite-URI path.
+    kindling_store::validate_db_path(&db_path)?;
     let kindling_dir = db_path
         .parent()
         .map(PathBuf::from)

crates/kindling/src/lib.rs

@@ -95,15 +95,12 @@ pub fn resolve_db_path(explicit: Option<&str>) -> Result<PathBuf, CliError> {
     })
 }
 
-/// Open an in-process service at the resolved DB path, creating the parent
-/// directory if needed.
+/// Open an in-process service at the resolved DB path.
+///
+/// Parent directories are created by the store after [`kindling_store::validate_db_path`],
+/// so a `--db` / `KINDLING_DB_PATH` value cannot mkdir through `..` first.
 pub fn open_service(explicit_db: Option<&str>) -> Result<(KindlingService, PathBuf), CliError> {
     let path = resolve_db_path(explicit_db)?;
-    if let Some(parent) = path.parent() {
-        if !parent.as_os_str().is_empty() {
-            std::fs::create_dir_all(parent)?;
-        }
-    }
     let service = KindlingService::open(&path)?;
     Ok((service, path))
 }