SQLPage decides "may an untrusted HTTP request reach this file?" in one function, validate_unprivileged_path, but that single security decision is invoked from three independent call sites, trust is carried by a misspelled boolean threaded through six signatures, and FileCache exposes a public FileStore::contains that performs no validation at all. The guard is correct today and I know of no bypass. The problem is structural: the same check enforced in several places, plus one unguarded variant, is exactly the shape that becomes a path-traversal or reserved-prefix bypass the day someone adds a new resolution path and forgets one of the sites.
Evidence (HEAD 162f996)
The guard, with a doc comment that itself warns it must be repeated everywhere: src/filesystem.rs#L204.
It is invoked from three places, each with a "remember to also call this here" comment:
A second, public FileStore impl that does NOT validate (only map lookups): src/file_cache.rs#L73-L77. Nothing stops a future caller from routing an untrusted path through it directly.
Trust is a boolean, threaded through six signatures and misspelled (priviledged) in the filesystem layer while spelled correctly (privileged) in the cache layer, an easy place to pass the wrong value:
read_file, read_to_string, modified_since, safe_local_path, get_with_privilege.
Proposed change
A small Copy token with private fields, so that holding an unprivileged instance is proof the guard already ran:
#[derive(Clone, Copy)]
#[must_use]
pub struct FileAccess<'a> {
path: &'a Path,
privileged: bool,
}
impl<'a> FileAccess<'a> {
/// Untrusted, HTTP-facing access. Runs the reserved/dotfile/traversal guard.
pub fn unprivileged(path: &'a Path) -> anyhow::Result<Self> {
validate_unprivileged_path(path)?;
Ok(Self { path, privileged: false })
}
/// Trusted internal access (templates, run_sql includes, read_file builtin).
pub fn privileged(path: &'a Path) -> Self {
Self { path, privileged: true }
}
pub fn path(&self) -> &Path { self.path }
}
Then make validate_unprivileged_path module-private, and change every FS/cache API from (path: &Path, privileged: bool) to take FileAccess by value. The guard runs exactly once, at construction. safe_local_path becomes infallible (its only error was the guard). FileCache::contains is now provably safe because a caller cannot build a FileAccess for an untrusted path without passing the guard. The misspelled boolean disappears.
Trust at the call sites becomes self-documenting: routing and the two http.rs entry points build FileAccess::unprivileged(path)?; the run_sql include, template loads, and the read_file builtin build FileAccess::privileged(path).
Risk and mitigation
FileStore::contains is a public trait method, so its signature changes, but there are only three in-tree impls (AppFileStore, FileCache, and the routing test Store). Behavior is unchanged: same guard, same per-candidate validation in routing, same FORBIDDEN propagation point, empty path (GET /) still passes. Two implementation gotchas worth stating up front: at the two http.rs construction sites keep the existing .map_err(|e| anyhow_err_to_actix(e, state))? so a guard failure stays a 403 rather than a 500; and let-bind the derived candidate PathBufs in routing so the borrow lives across the .await.
Expected win
One definition of "what path an untrusted request may reach," enforced by the type system. It becomes impossible to add a file-resolution path that skips the guard, the unguarded public contains stops being a footgun, and the misspelled trust boolean is gone. This is the concrete fix for the "same security decision implemented in more than one way" risk in the path-handling code.
SQLPage decides "may an untrusted HTTP request reach this file?" in one function,
validate_unprivileged_path, but that single security decision is invoked from three independent call sites, trust is carried by a misspelled boolean threaded through six signatures, andFileCacheexposes a publicFileStore::containsthat performs no validation at all. The guard is correct today and I know of no bypass. The problem is structural: the same check enforced in several places, plus one unguarded variant, is exactly the shape that becomes a path-traversal or reserved-prefix bypass the day someone adds a new resolution path and forgets one of the sites.Evidence (HEAD 162f996)
The guard, with a doc comment that itself warns it must be repeated everywhere: src/filesystem.rs#L204.
It is invoked from three places, each with a "remember to also call this here" comment:
safe_local_path: src/filesystem.rs#L156A second, public
FileStoreimpl that does NOT validate (only map lookups): src/file_cache.rs#L73-L77. Nothing stops a future caller from routing an untrusted path through it directly.Trust is a boolean, threaded through six signatures and misspelled (
priviledged) in the filesystem layer while spelled correctly (privileged) in the cache layer, an easy place to pass the wrong value:read_file,read_to_string,modified_since,safe_local_path,get_with_privilege.Proposed change
A small
Copytoken with private fields, so that holding an unprivileged instance is proof the guard already ran:Then make
validate_unprivileged_pathmodule-private, and change every FS/cache API from(path: &Path, privileged: bool)to takeFileAccessby value. The guard runs exactly once, at construction.safe_local_pathbecomes infallible (its only error was the guard).FileCache::containsis now provably safe because a caller cannot build aFileAccessfor an untrusted path without passing the guard. The misspelled boolean disappears.Trust at the call sites becomes self-documenting: routing and the two
http.rsentry points buildFileAccess::unprivileged(path)?; therun_sqlinclude, template loads, and theread_filebuiltin buildFileAccess::privileged(path).Risk and mitigation
FileStore::containsis a public trait method, so its signature changes, but there are only three in-tree impls (AppFileStore,FileCache, and the routing testStore). Behavior is unchanged: same guard, same per-candidate validation in routing, same FORBIDDEN propagation point, empty path (GET /) still passes. Two implementation gotchas worth stating up front: at the twohttp.rsconstruction sites keep the existing.map_err(|e| anyhow_err_to_actix(e, state))?so a guard failure stays a403rather than a500; andlet-bind the derived candidatePathBufs in routing so the borrow lives across the.await.Expected win
One definition of "what path an untrusted request may reach," enforced by the type system. It becomes impossible to add a file-resolution path that skips the guard, the unguarded public
containsstops being a footgun, and the misspelled trust boolean is gone. This is the concrete fix for the "same security decision implemented in more than one way" risk in the path-handling code.