fix: enforce unprivileged path guard on fresh cache hits (#1307)

Reserved/private SQL files (sqlpage/ prefix, dotfiles, .. traversal,
absolute paths) became directly routable over HTTP while their parsed
form was fresh in sql_file_cache. A trusted page loading such a file via
sqlpage.run_sql(...) loads it with privilege and caches it; a later
direct unprivileged request hit the fresh cache entry before the path
guard ran, returning 200 and executing the private SQL instead of 403.

The unprivileged path validation is extracted into
filesystem::validate_unprivileged_path and now enforced before
consulting the cache in both HTTP routing (AppFileStore::contains) and
the unprivileged FileCache::get_with_privilege path.

Author: lovasoa

CHANGELOG.md

@@ -1,5 +1,9 @@
 # CHANGELOG.md
 
+## Unreleased
+
+- **Security fix: reserved and private files could be served directly over HTTP after a trusted page loaded them.** Files that SQLPage normally refuses to serve to direct HTTP requests (anything under the reserved `sqlpage/` prefix such as migrations, configuration, and database connection details, as well as dotfiles, parent-directory traversal paths like `../secret.sql`, and absolute paths) could briefly become reachable. This happened only after a trusted page loaded that same file with `sqlpage.run_sql(...)`, which loads files with elevated privileges and caches the parsed result. While that cache entry was fresh, a direct request such as `GET /sqlpage/secret.sql` (or the extensionless alias `GET /sqlpage/secret`) returned `200 OK` and executed the private SQL instead of returning `403 Forbidden`. Worst case, an attacker who can reach your site could read or execute internal SQL that was meant to stay private, including migration and configuration logic. You are affected if your app calls `sqlpage.run_sql()` on files inside `sqlpage/` (or on dotfiles or paths outside the web root) and is reachable by untrusted users. The fix enforces the unprivileged path guard on every direct HTTP request regardless of the cache, so these paths now always return `403 Forbidden`. Upgrade to get the fix; no configuration change is required. Legitimate `sqlpage.run_sql()` includes of such files from your own pages keep working as before.
+
 ## v0.44.0
 
 This release focuses on making production SQLPage apps easier to understand, debug, and operate. Most apps should keep working without SQL changes, but maintainers should review the notes about logging and uploaded-file permissions.

sqlpage/private_cache_bypass_test.sql

@@ -0,0 +1 @@
+select 'text' as component, 'private cache bypass secret' as contents;

src/file_cache.rs

@@ -120,6 +120,12 @@ impl<T: AsyncFromStrWithState> FileCache<T> {
         privileged: bool,
     ) -> anyhow::Result<Arc<T>> {
         log::trace!("Attempting to get from cache {}", path.display());
+        // Enforce the untrusted path guard before consulting the cache. A fresh cache
+        // entry (possibly populated by a privileged `run_sql` include) must never let an
+        // unprivileged request reach a reserved/private/traversal/absolute path.
+        if !privileged {
+            crate::filesystem::validate_unprivileged_path(path)?;
+        }
         if let Some(cached) = self.cache.read().await.get(path) {
             if !cached.needs_check(app_state.config.cache_stale_duration_ms()) {
                 log::trace!(

src/filesystem.rs

@@ -153,34 +153,7 @@ impl FileSystem {
                 return Ok(normalized);
             }
         } else {
-            for (i, component) in path.components().enumerate() {
-                if let Component::Normal(c) = component {
-                    if i == 0 && c.eq_ignore_ascii_case("sqlpage") {
-                        return Err(ErrorWithStatus {
-                            status: actix_web::http::StatusCode::FORBIDDEN,
-                        })
-                        .with_context(|| {
-                            "The /sqlpage/ path prefix is reserved for internal use. It is not public."
-                        });
-                    }
-                    if c.as_encoded_bytes().starts_with(b".") {
-                        return Err(ErrorWithStatus {
-                            status: actix_web::http::StatusCode::FORBIDDEN,
-                        })
-                        .with_context(|| "Directory traversal is not allowed");
-                    }
-                } else {
-                    return Err(ErrorWithStatus {
-                        status: actix_web::http::StatusCode::FORBIDDEN,
-                    })
-                    .with_context(|| {
-                        format!(
-                            "Unsupported path: {}. Path component '{component:?}' is not allowed.",
-                            path.display()
-                        )
-                    });
-                }
-            }
+            validate_unprivileged_path(path)?;
         }
         Ok(self.local_root.join(path))
     }
@@ -219,6 +192,47 @@ impl FileSystem {
     }
 }
 
+/// Rejects paths that an untrusted (unprivileged) HTTP request must never reach:
+/// the reserved `sqlpage/` prefix, dotfiles, parent-directory traversal and
+/// absolute/root paths.
+///
+/// This guard must be enforced on every unprivileged resolution path, including
+/// before trusting a fresh cache hit. A trusted page may legitimately load such a
+/// file with privilege via `sqlpage.run_sql(...)`, which populates the shared file
+/// cache; without this check a later direct HTTP request could be served the cached
+/// private file instead of being forbidden.
+pub fn validate_unprivileged_path(path: &Path) -> anyhow::Result<()> {
+    for (i, component) in path.components().enumerate() {
+        if let Component::Normal(c) = component {
+            if i == 0 && c.eq_ignore_ascii_case("sqlpage") {
+                return Err(ErrorWithStatus {
+                    status: actix_web::http::StatusCode::FORBIDDEN,
+                })
+                .with_context(
+                    || "The /sqlpage/ path prefix is reserved for internal use. It is not public.",
+                );
+            }
+            if c.as_encoded_bytes().starts_with(b".") {
+                return Err(ErrorWithStatus {
+                    status: actix_web::http::StatusCode::FORBIDDEN,
+                })
+                .with_context(|| "Directory traversal is not allowed");
+            }
+        } else {
+            return Err(ErrorWithStatus {
+                status: actix_web::http::StatusCode::FORBIDDEN,
+            })
+            .with_context(|| {
+                format!(
+                    "Unsupported path: {}. Path component '{component:?}' is not allowed.",
+                    path.display()
+                )
+            });
+        }
+    }
+    Ok(())
+}
+
 fn is_path_missing_error(error: &std::io::Error) -> bool {
     matches!(error.kind(), ErrorKind::NotFound | ErrorKind::NotADirectory)
 }

src/webserver/routing.rs

@@ -130,6 +130,10 @@ impl<'a> AppFileStore<'a> {
 
 impl FileStore for AppFileStore<'_> {
     async fn contains(&self, path: &Path) -> anyhow::Result<bool> {
+        // HTTP routing is always an untrusted, unprivileged operation. Enforce the path
+        // guard before consulting the cache: a fresh cache entry populated by a
+        // privileged `run_sql` include must not make a reserved/private path routable.
+        crate::filesystem::validate_unprivileged_path(path)?;
         if self.cache.contains(path).await? {
             Ok(true)
         } else {

tests/errors/mod.rs

@@ -2,11 +2,67 @@ use actix_web::{
     http::{self, StatusCode},
     test,
 };
+use sqlpage::{AppState, webserver::http::main_handler};
 
-use crate::common::req_path;
+use crate::common::{make_app_data_from_config, req_path, req_path_with_app_data, test_config};
 mod basic_auth;
 mod invalid_header;
 
+/// Sends a direct unprivileged GET request through the main handler and returns the
+/// resulting HTTP status, whether the handler returned an `Ok` response or an `Err`.
+async fn direct_request_status(path: &str, app_data: actix_web::web::Data<AppState>) -> StatusCode {
+    let req = test::TestRequest::get()
+        .uri(path)
+        .app_data(app_data)
+        .insert_header(actix_web::http::header::Accept::html())
+        .to_srv_request();
+    match main_handler(req).await {
+        Ok(resp) => resp.status(),
+        Err(e) => e.error_response().status(),
+    }
+}
+
+/// Regression test for a cache privilege-escalation bug.
+///
+/// `sqlpage.run_sql(...)` loads include files with privilege, so it is allowed to
+/// load reserved files under the `sqlpage/` prefix and stores their parsed form in
+/// the shared `sql_file_cache`. A subsequent *direct* unprivileged HTTP request for
+/// that same reserved path must still be rejected with 403, even while the cache
+/// entry is fresh. Before the fix, the fresh cache hit short-circuited the
+/// unprivileged path guard and the private SQL was executed and served.
+#[actix_web::test]
+async fn test_private_path_not_accessible_after_privileged_cache_priming() {
+    // Keep cache entries "fresh" so the bug (skipping the path guard on fresh hits) is exercised.
+    let mut config = test_config();
+    config.cache_stale_duration_ms = Some(60_000);
+    let app_data = make_app_data_from_config(config).await;
+
+    // 1. A trusted page primes the cache by loading the reserved file with privilege.
+    let prime = req_path_with_app_data("/tests/errors/prime_private_cache.sql", app_data.clone())
+        .await
+        .expect("priming page should render");
+    assert_eq!(prime.status(), StatusCode::OK);
+    let prime_body = String::from_utf8(test::read_body(prime).await.to_vec()).unwrap();
+    assert!(
+        prime_body.contains("private cache bypass secret"),
+        "priming page should have executed the private file via run_sql, got: {prime_body}"
+    );
+
+    // 2. A direct unprivileged HTTP request for the reserved path must stay forbidden.
+    for path in [
+        "/sqlpage/private_cache_bypass_test.sql",
+        // Extensionless alias: routing appends .sql and finds the fresh cache entry.
+        "/sqlpage/private_cache_bypass_test",
+    ] {
+        let status = direct_request_status(path, app_data.clone()).await;
+        assert_eq!(
+            status,
+            StatusCode::FORBIDDEN,
+            "{path} must be forbidden even after privileged cache priming, got {status}"
+        );
+    }
+}
+
 #[actix_web::test]
 async fn test_privileged_paths_are_not_accessible() {
     let resp_result = req_path("/sqlpage/migrations/0001_init.sql").await;

tests/errors/prime_private_cache.sql

@@ -0,0 +1,4 @@
+-- Trusted page: loads a reserved (sqlpage/) file via the privileged run_sql,
+-- which populates the shared sql_file_cache with the parsed private file.
+select 'dynamic' as component,
+    sqlpage.run_sql('sqlpage/private_cache_bypass_test.sql') as properties;