Skip to content

security: fix audit findings across fs sandbox, IPC, grants, plugins, storage #30

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MerhiOPS wants to merge 6 commits into from
Closed

security: fix audit findings across fs sandbox, IPC, grants, plugins, storage #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f36163c
security(fs): add post-open path verification to close TOCTOU races
MerhiOPS Mar 16, 2026
65a51be
security: fix medium-severity audit findings across IPC, grants, plug…
MerhiOPS Mar 16, 2026
b5820c6
security: fix low-severity audit findings (rate limit, reservations, …
MerhiOPS Mar 16, 2026
30ed7ab
fix: resolve compilation errors in audit hardening
MerhiOPS Mar 16, 2026
59a220a
fix: split fs module for 300-line cap and fix init script test assertion
MerhiOPS Mar 16, 2026
c047c1e
ci: trigger checks
MerhiOPS Mar 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
225 changes: 58 additions & 167 deletions crates/volt-core/src/fs/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
mod resolve;

use std::fs;
use std::io::Write;
use std::path::{Path, PathBuf};
use std::path::Path;
use std::time::UNIX_EPOCH;
use thiserror::Error;

use crate::security::validate_path;
pub use resolve::{safe_resolve, safe_resolve_for_create};
use resolve::{canonical_base, ensure_not_symlink, ensure_scoped_directory, verify_opened_path};

#[derive(Error, Debug)]
pub enum FsError {
Expand All @@ -18,113 +21,70 @@ pub enum FsError {
OutOfScope,
}

/// Safely resolve a user-provided relative path against a base directory.
/// Rejects absolute paths, path traversal, and ensures the result is under base.
pub fn safe_resolve(base: &Path, user_path: &str) -> Result<PathBuf, FsError> {
// Validate the path for traversal attacks and reserved names
validate_path(user_path).map_err(FsError::Security)?;

let resolved = base.join(user_path);

// Canonicalize both paths and verify the resolved path is under base.
// If the file doesn't exist yet, canonicalize the parent.
let canonical_base = base
.canonicalize()
.map_err(|_| FsError::Security("Base directory does not exist".to_string()))?;

// Try to canonicalize the full path first (works if file exists)
let canonical_resolved = if resolved.exists() {
resolved.canonicalize()?
} else {
// If the file doesn't exist, canonicalize the parent directory
let parent = resolved
.parent()
.ok_or_else(|| FsError::Security("Cannot resolve parent directory".to_string()))?;

if !parent.exists() {
// Parent also doesn't exist - walk up to find nearest existing ancestor
// Find the nearest existing ancestor and canonicalize from there
let mut ancestor = parent.to_path_buf();
let mut trailing_components = Vec::new();
while !ancestor.exists() {
if let Some(name) = ancestor.file_name() {
trailing_components.push(name.to_os_string());
} else {
return Err(FsError::Security(
"Cannot resolve path ancestor".to_string(),
));
}
ancestor = ancestor
.parent()
.ok_or_else(|| FsError::Security("Cannot resolve path ancestor".to_string()))?
.to_path_buf();
}
let mut canonical = ancestor.canonicalize()?;
for component in trailing_components.into_iter().rev() {
canonical.push(component);
}
if let Some(file_name) = resolved.file_name() {
canonical.push(file_name);
}
canonical
} else {
let canonical_parent = parent.canonicalize()?;
let file_name = resolved
.file_name()
.ok_or_else(|| FsError::Security("Invalid file name".to_string()))?;
canonical_parent.join(file_name)
}
};

// Verify the resolved path starts with the base
if !canonical_resolved.starts_with(&canonical_base) {
return Err(FsError::OutOfScope);
}

Ok(canonical_resolved)
}

/// Resolve a path for create/write flows while securely materializing any
/// missing parent directories inside the scoped base directory.
pub fn safe_resolve_for_create(base: &Path, user_path: &str) -> Result<PathBuf, FsError> {
let resolved = safe_resolve(base, user_path)?;
ensure_scoped_parent_dirs(base, &resolved)?;
ensure_not_symlink(&resolved)?;
Ok(resolved)
/// File metadata info returned by stat().
#[derive(Debug, Clone)]
pub struct FileInfo {
pub size: u64,
pub is_file: bool,
pub is_dir: bool,
pub readonly: bool,
/// Last modification time as milliseconds since Unix epoch.
pub modified_ms: f64,
/// Creation time as milliseconds since Unix epoch.
/// `None` on platforms/filesystems that do not support birth time.
pub created_ms: Option<f64>,
}

/// Read a file's contents as bytes.
/// Post-open verification ensures no TOCTOU symlink swap can escape the sandbox.
pub fn read_file(base: &Path, path: &str) -> Result<Vec<u8>, FsError> {
let resolved = safe_resolve(base, path)?;
Ok(fs::read(resolved)?)
let cb = canonical_base(base)?;
verify_opened_path(&resolved, &cb)?;
let data = fs::read(&resolved)?;
verify_opened_path(&resolved, &cb)?;
Ok(data)
}

/// Read a file's contents as a UTF-8 string.
/// Post-open verification ensures no TOCTOU symlink swap can escape the sandbox.
pub fn read_file_text(base: &Path, path: &str) -> Result<String, FsError> {
let resolved = safe_resolve(base, path)?;
Ok(fs::read_to_string(resolved)?)
let cb = canonical_base(base)?;
verify_opened_path(&resolved, &cb)?;
let data = fs::read_to_string(&resolved)?;
verify_opened_path(&resolved, &cb)?;
Ok(data)
}

/// Write data to a file, creating it if it doesn't exist.
/// For existing files, a post-open symlink check prevents TOCTOU escapes.
/// For new files, `create_new(true)` fails if a symlink appears at the path.
pub fn write_file(base: &Path, path: &str, data: &[u8]) -> Result<(), FsError> {
let resolved = safe_resolve_for_create(base, path)?;
let cb = canonical_base(base)?;

if resolved.exists() {
fs::write(resolved, data)?;
ensure_not_symlink(&resolved)?;
verify_opened_path(&resolved, &cb)?;
fs::write(&resolved, data)?;
verify_opened_path(&resolved, &cb)?;
return Ok(());
}

let mut file = fs::OpenOptions::new()
.write(true)
.create_new(true)
.open(resolved)?;
.open(&resolved)?;
file.write_all(data)?;
Ok(())
}

/// List entries in a directory.
pub fn read_dir(base: &Path, path: &str) -> Result<Vec<String>, FsError> {
let resolved = safe_resolve(base, path)?;
let cb = canonical_base(base)?;
verify_opened_path(&resolved, &cb)?;
let mut entries = Vec::new();
for entry in fs::read_dir(resolved)? {
let entry = entry?;
Expand All @@ -138,6 +98,8 @@ pub fn read_dir(base: &Path, path: &str) -> Result<Vec<String>, FsError> {
/// Get metadata for a path.
pub fn stat(base: &Path, path: &str) -> Result<FileInfo, FsError> {
let resolved = safe_resolve(base, path)?;
let cb = canonical_base(base)?;
verify_opened_path(&resolved, &cb)?;
let meta = fs::metadata(resolved)?;

let modified_ms = meta
Expand Down Expand Up @@ -166,21 +128,12 @@ pub fn stat(base: &Path, path: &str) -> Result<FileInfo, FsError> {
/// Check whether a path exists within the scoped base directory.
pub fn exists(base: &Path, path: &str) -> Result<bool, FsError> {
let resolved = safe_resolve(base, path)?;
Ok(resolved.exists())
}

/// File metadata info returned by stat().
#[derive(Debug, Clone)]
pub struct FileInfo {
pub size: u64,
pub is_file: bool,
pub is_dir: bool,
pub readonly: bool,
/// Last modification time as milliseconds since Unix epoch.
pub modified_ms: f64,
/// Creation time as milliseconds since Unix epoch.
/// `None` on platforms/filesystems that do not support birth time.
pub created_ms: Option<f64>,
if !resolved.exists() {
return Ok(false);
}
let cb = canonical_base(base)?;
verify_opened_path(&resolved, &cb)?;
Ok(true)
}

/// Create a directory (and parents if needed).
Expand All @@ -191,31 +144,34 @@ pub fn mkdir(base: &Path, path: &str) -> Result<(), FsError> {

/// Remove a file or directory.
/// If the path is a directory, removal is recursive.
/// Double symlink check (before and after `is_dir`) narrows the TOCTOU window.
pub fn remove(base: &Path, path: &str) -> Result<(), FsError> {
let resolved = safe_resolve(base, path)?;
ensure_not_symlink(&resolved)?;
let canonical_base = base
.canonicalize()
.map_err(|_| FsError::Security("Base directory does not exist".to_string()))?;
if resolved == canonical_base {
let cb = canonical_base(base)?;
if resolved == cb {
return Err(FsError::Security(
"Refusing to remove the base directory".to_string(),
));
}

if resolved.is_dir() {
ensure_not_symlink(&resolved)?;
verify_opened_path(&resolved, &cb)?;
Ok(fs::remove_dir_all(resolved)?)
} else {
ensure_not_symlink(&resolved)?;
verify_opened_path(&resolved, &cb)?;
Ok(fs::remove_file(resolved)?)
}
}

/// Rename (move) a file or directory within the scope.
/// Both `from` and `to` must be within the base scope.
/// Uses `std::fs::rename` which is atomic on same-filesystem.
pub fn rename(base: &Path, from: &str, to: &str) -> Result<(), FsError> {
let resolved_from = safe_resolve(base, from)?;
let resolved_to = safe_resolve_for_create(base, to)?;
let cb = canonical_base(base)?;
verify_opened_path(&resolved_from, &cb)?;

if !resolved_from.exists() {
return Err(FsError::Io(std::io::Error::new(
Expand All @@ -235,11 +191,12 @@ pub fn rename(base: &Path, from: &str, to: &str) -> Result<(), FsError> {
}

/// Copy a file within the scope.
/// Both `from` and `to` must be within the base scope.
/// Only files can be copied; use mkdir + recursive copy for directories.
pub fn copy(base: &Path, from: &str, to: &str) -> Result<(), FsError> {
let resolved_from = safe_resolve(base, from)?;
let resolved_to = safe_resolve_for_create(base, to)?;
let cb = canonical_base(base)?;
verify_opened_path(&resolved_from, &cb)?;

if !resolved_from.exists() {
return Err(FsError::Io(std::io::Error::new(
Expand All @@ -264,71 +221,5 @@ pub fn copy(base: &Path, from: &str, to: &str) -> Result<(), FsError> {
Ok(())
}

fn ensure_scoped_directory(base: &Path, directory: &Path) -> Result<(), FsError> {
let canonical_base = canonical_base_dir(base)?;
let relative = directory
.strip_prefix(&canonical_base)
.map_err(|_| FsError::OutOfScope)?;
let mut current = canonical_base.clone();

for component in relative.components() {
current.push(component.as_os_str());
match fs::symlink_metadata(&current) {
Ok(metadata) => {
if metadata.file_type().is_symlink() {
return Err(FsError::Security(format!(
"symlink component is not allowed: '{}'",
current.display()
)));
}
if !metadata.is_dir() {
return Err(FsError::Security(format!(
"path component is not a directory: '{}'",
current.display()
)));
}
}
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
fs::create_dir(&current)?;
}
Err(error) => return Err(FsError::Io(error)),
}

let canonical_current = current.canonicalize()?;
if !canonical_current.starts_with(&canonical_base) {
return Err(FsError::OutOfScope);
}
current = canonical_current;
}

Ok(())
}

fn ensure_scoped_parent_dirs(base: &Path, resolved: &Path) -> Result<(), FsError> {
let Some(parent) = resolved.parent() else {
return Err(FsError::Security(
"Cannot resolve parent directory".to_string(),
));
};
ensure_scoped_directory(base, parent)
}

fn ensure_not_symlink(path: &Path) -> Result<(), FsError> {
match fs::symlink_metadata(path) {
Ok(metadata) if metadata.file_type().is_symlink() => Err(FsError::Security(format!(
"symlink targets are not allowed: '{}'",
path.display()
))),
Ok(_) => Ok(()),
Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()),
Err(error) => Err(FsError::Io(error)),
}
}

fn canonical_base_dir(base: &Path) -> Result<PathBuf, FsError> {
base.canonicalize()
.map_err(|_| FsError::Security("Base directory does not exist".to_string()))
}

#[cfg(test)]
mod tests;
Loading
Loading