Classify OIDC paths with routing's exact decode

is_public_path reimplemented percent-decoding (lossy UTF-8, string match),
diverging from routing::check_path on non-UTF-8 paths (Unix) and backslash
separators (Windows), allowing protected files to be served as public.
Extract routing's decode into a shared decode_percent_encoded_path and use it
for OIDC classification, comparing on path-component boundaries so auth and
routing always agree. Adds Unix non-UTF-8 and Windows backslash regression tests.

Author: lovasoa

CHANGELOG.md

@@ -4,7 +4,7 @@
 
 - **Download filenames can no longer inject extra `Content-Disposition` parameters.** The `csv` and `download` components now build the `Content-Disposition` header with a properly quoted and escaped filename instead of plain string concatenation. Before this fix, a filename containing characters such as `;`, `"`, or `=` could add a second header parameter (for example a `filename*=...` value), and some browsers prefer that injected value over the intended one. You are affected if your app puts untrusted data (such as a user-provided name or a value from the database) into the `filename` of a `csv` or `download` component. No SQL change is required: the supplied filename now always appears as a single, safely quoted `filename` value.
 - **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.
-- **Security fix: OIDC protected paths could be bypassed with percent-encoded URLs.** When `oidc_protected_paths` used a non-default prefix such as `["/protected"]`, an unauthenticated visitor could percent-encode a byte of the prefix (for example requesting `/%70rotected/`, where `%70` is `p`) to make the OIDC middleware treat the page as public and serve it without logging in, even though SQLPage then decoded the URL and ran the protected SQL file. You are affected if you rely on `oidc_protected_paths` (or `oidc_public_paths`) with prefixes other than the default `/`. The path is now percent-decoded before the protected/public rules are applied, so encoded and plain URLs are classified identically. No configuration change is required; just upgrade.
+- **Security fix: OIDC protected paths could be bypassed with percent-encoded URLs.** When `oidc_protected_paths` used a non-default prefix such as `["/protected"]`, an unauthenticated visitor could percent-encode a byte of the prefix (for example requesting `/%70rotected/`, where `%70` is `p`) to make the OIDC middleware treat the page as public and serve it without logging in, even though SQLPage then decoded the URL and ran the protected SQL file. You are affected if you rely on `oidc_protected_paths` (or `oidc_public_paths`) with prefixes other than the default `/`. SQLPage now classifies the request with the exact same path decoding the router uses to pick the file, so percent-encoded URLs, non-UTF-8 byte paths on Unix, and `\` (a path separator on Windows) are all treated identically by authentication and by routing. Matching is now on path-component boundaries, so a protected `/admin` covers `/admin` and `/admin/...` but not an unrelated `/administrator`. No configuration change is required; just upgrade.
 
 ## v0.44.0
 

src/webserver/oidc.rs

@@ -40,6 +40,7 @@ use tracing::Instrument;
 
 use super::error::anyhow_err_to_actix_resp;
 use super::http_client::make_http_client;
+use super::routing::decode_percent_encoded_path;
 
 type LocalBoxFuture<T> = Pin<Box<dyn Future<Output = T> + 'static>>;
 
@@ -140,28 +141,22 @@ impl TryFrom<&AppConfig> for OidcConfig {
 impl OidcConfig {
     #[must_use]
     pub fn is_public_path(&self, path: &str) -> bool {
-        // Percent-decode before matching, so an encoded protected prefix
-        // (e.g. "/%70rotected" == "/protected") is classified the same way the
-        // router resolves it once decoded. Both the request path AND the
-        // configured prefixes are decoded: `site_prefix` may itself contain
-        // percent-encoded characters (e.g. a space -> "%20"), so the stored
-        // prefixes can be encoded too. Comparing a decoded path against an
-        // encoded prefix would never match and would wrongly make every
-        // protected page public. Decode both sides so the comparison matches
-        // what routing actually serves.
-        fn decode(s: &str) -> std::borrow::Cow<'_, str> {
-            percent_encoding::percent_decode_str(s).decode_utf8_lossy()
-        }
-        let decoded = decode(path);
-        let path = decoded.as_ref();
-        !self
-            .protected_paths
-            .iter()
-            .any(|p| path.starts_with(decode(p).as_ref()))
-            || self
-                .public_paths
+        // Classify the request using the exact same decoding the router uses to
+        // resolve the file (`routing::decode_percent_encoded_path`), so
+        // authentication and file resolution can never disagree. This handles
+        // percent-encoded bytes, non-UTF-8 paths on Unix, and `\` as a separator
+        // on Windows identically to routing. The configured prefixes are decoded
+        // the same way (a `site_prefix` such as `/my app/` is stored encoded as
+        // `/my%20app/`), and matching is done on path-component boundaries, so a
+        // protected `/admin` covers `/admin/...` but not an unrelated
+        // `/administrator`, mirroring how routing maps URLs to files.
+        let decoded = decode_percent_encoded_path(path);
+        let matches_any = |prefixes: &[String]| {
+            prefixes
                 .iter()
-                .any(|p| path.starts_with(decode(p).as_ref()))
+                .any(|p| decoded.starts_with(decode_percent_encoded_path(p)))
+        };
+        !matches_any(&self.protected_paths) || matches_any(&self.public_paths)
     }
 
     /// Creates a custom ID token verifier that supports multiple issuers
@@ -1358,6 +1353,37 @@ mod tests {
         );
     }
 
+    #[cfg(unix)]
+    #[test]
+    fn non_utf8_path_is_classified_like_routing() {
+        // Protected root, plus a public prefix containing a non-UTF-8 byte.
+        // A request for a DIFFERENT non-UTF-8 byte must not be misclassified as
+        // public: lossy UTF-8 decoding would collapse both %FF and %FE to the
+        // same replacement char, but routing serves distinct byte paths.
+        let config =
+            test_oidc_config_with_paths(vec!["/".to_string()], vec!["/public/%FF".to_string()]);
+        assert!(
+            !config.is_public_path("/public/%FE/secret.sql"),
+            "/public/%FE/... is a different file than the public /public/%FF and must stay protected"
+        );
+        assert!(
+            config.is_public_path("/public/%FF/page.sql"),
+            "the configured public path itself must stay public"
+        );
+    }
+
+    // Windows treats `\` as a path separator, like routing's PathBuf. This runs
+    // only on Windows CI; on Unix `\` is a literal byte and the case differs.
+    #[cfg(windows)]
+    #[test]
+    fn windows_backslash_path_is_classified_like_routing() {
+        let config = test_oidc_config_with_paths(vec!["/admin/".to_string()], vec![]);
+        assert!(
+            !config.is_public_path("/admin%5Csecret.sql"),
+            "/admin\\secret.sql resolves under /admin/ on Windows and must stay protected"
+        );
+    }
+
     #[test]
     fn evicts_excess_tmp_login_flow_state_cookies() {
         let request = (0..MAX_OIDC_PARALLEL_LOGIN_FLOWS)

src/webserver/routing.rs

@@ -172,21 +172,26 @@ where
 {
     match path_and_query.path().strip_prefix(config.prefix()) {
         None => Err(Redirect(config.prefix().to_string())),
-        Some(path) => {
-            let decoded = percent_encoding::percent_decode_str(path);
-            #[cfg(unix)]
-            {
-                use std::ffi::OsString;
-                use std::os::unix::ffi::OsStringExt;
-
-                let decoded = decoded.collect::<Vec<u8>>();
-                Ok(PathBuf::from(OsString::from_vec(decoded)))
-            }
-            #[cfg(not(unix))]
-            {
-                Ok(PathBuf::from(decoded.decode_utf8_lossy().as_ref()))
-            }
-        }
+        Some(path) => Ok(decode_percent_encoded_path(path)),
+    }
+}
+
+/// Percent-decodes a request path into a `PathBuf`, preserving the exact decoded
+/// bytes on Unix and applying platform path semantics (for example `\` is a
+/// separator on Windows). OIDC path classification reuses this so authentication
+/// and file resolution always agree on which file a request targets.
+pub(crate) fn decode_percent_encoded_path(path: &str) -> PathBuf {
+    let decoded = percent_encoding::percent_decode_str(path);
+    #[cfg(unix)]
+    {
+        use std::ffi::OsString;
+        use std::os::unix::ffi::OsStringExt;
+
+        PathBuf::from(OsString::from_vec(decoded.collect::<Vec<u8>>()))
+    }
+    #[cfg(not(unix))]
+    {
+        PathBuf::from(decoded.decode_utf8_lossy().as_ref())
     }
 }