fixes PR004
This commit is contained in:
parent
384587bb04
commit
5ec270dd43
@ -5,6 +5,7 @@ pub enum FsError {
|
||||
NotFound,
|
||||
AlreadyExists,
|
||||
PermissionDenied,
|
||||
InvalidPath(String),
|
||||
NotMounted,
|
||||
IOError(String),
|
||||
Other(String),
|
||||
@ -16,6 +17,7 @@ impl fmt::Display for FsError {
|
||||
FsError::NotFound => write!(f, "File or directory not found"),
|
||||
FsError::AlreadyExists => write!(f, "Already exists"),
|
||||
FsError::PermissionDenied => write!(f, "Permission denied"),
|
||||
FsError::InvalidPath(s) => write!(f, "Invalid path: {}", s),
|
||||
FsError::NotMounted => write!(f, "Filesystem not mounted"),
|
||||
FsError::IOError(s) => write!(f, "IO Error: {}", s),
|
||||
FsError::Other(s) => write!(f, "Error: {}", s),
|
||||
|
||||
@ -34,40 +34,63 @@ impl VirtualFS {
|
||||
self.backend.is_some()
|
||||
}
|
||||
|
||||
fn normalize_path(&self, path: &str) -> String {
|
||||
fn normalize_path(path: &str) -> Result<String, FsError> {
|
||||
let mut normalized = path.replace('\\', "/");
|
||||
if !normalized.starts_with('/') {
|
||||
normalized = format!("/{}", normalized);
|
||||
}
|
||||
normalized
|
||||
|
||||
let mut segments = Vec::new();
|
||||
for segment in normalized.split('/') {
|
||||
match segment {
|
||||
"" | "." => continue,
|
||||
".." => {
|
||||
return Err(FsError::InvalidPath(
|
||||
"parent traversal '..' is not allowed".to_string(),
|
||||
));
|
||||
}
|
||||
_ => segments.push(segment),
|
||||
}
|
||||
}
|
||||
|
||||
if segments.is_empty() {
|
||||
Ok("/".to_string())
|
||||
} else {
|
||||
Ok(format!("/{}", segments.join("/")))
|
||||
}
|
||||
}
|
||||
|
||||
pub fn list_dir(&self, path: &str) -> Result<Vec<FsEntry>, FsError> {
|
||||
let normalized = self.normalize_path(path);
|
||||
let normalized = Self::normalize_path(path)?;
|
||||
let backend = self.backend.as_ref().ok_or(FsError::NotMounted)?;
|
||||
backend.list_dir(&normalized)
|
||||
}
|
||||
|
||||
pub fn read_file(&self, path: &str) -> Result<Vec<u8>, FsError> {
|
||||
let normalized = self.normalize_path(path);
|
||||
let normalized = Self::normalize_path(path)?;
|
||||
let backend = self.backend.as_ref().ok_or(FsError::NotMounted)?;
|
||||
backend.read_file(&normalized)
|
||||
}
|
||||
|
||||
pub fn write_file(&mut self, path: &str, data: &[u8]) -> Result<(), FsError> {
|
||||
let normalized = self.normalize_path(path);
|
||||
let normalized = Self::normalize_path(path)?;
|
||||
let backend = self.backend.as_mut().ok_or(FsError::NotMounted)?;
|
||||
backend.write_file(&normalized, data)
|
||||
}
|
||||
|
||||
pub fn delete(&mut self, path: &str) -> Result<(), FsError> {
|
||||
let normalized = self.normalize_path(path);
|
||||
let normalized = Self::normalize_path(path)?;
|
||||
if normalized == "/" {
|
||||
return Err(FsError::PermissionDenied);
|
||||
}
|
||||
let backend = self.backend.as_mut().ok_or(FsError::NotMounted)?;
|
||||
backend.delete(&normalized)
|
||||
}
|
||||
|
||||
pub fn exists(&self, path: &str) -> bool {
|
||||
let normalized = self.normalize_path(path);
|
||||
let Ok(normalized) = Self::normalize_path(path) else {
|
||||
return false;
|
||||
};
|
||||
self.backend.as_ref().map(|b| b.exists(&normalized)).unwrap_or(false)
|
||||
}
|
||||
|
||||
@ -80,15 +103,31 @@ impl VirtualFS {
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::collections::HashMap;
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
use std::sync::Arc;
|
||||
|
||||
#[derive(Default)]
|
||||
struct CallCounters {
|
||||
list_dir: AtomicUsize,
|
||||
read_file: AtomicUsize,
|
||||
write_file: AtomicUsize,
|
||||
delete: AtomicUsize,
|
||||
exists: AtomicUsize,
|
||||
}
|
||||
|
||||
struct MockBackend {
|
||||
files: HashMap<String, Vec<u8>>,
|
||||
healthy: bool,
|
||||
calls: Arc<CallCounters>,
|
||||
}
|
||||
|
||||
impl MockBackend {
|
||||
fn new() -> Self {
|
||||
Self { files: HashMap::new(), healthy: true }
|
||||
Self::with_calls(Arc::new(CallCounters::default()))
|
||||
}
|
||||
|
||||
fn with_calls(calls: Arc<CallCounters>) -> Self {
|
||||
Self { files: HashMap::new(), healthy: true, calls }
|
||||
}
|
||||
}
|
||||
|
||||
@ -98,6 +137,7 @@ mod tests {
|
||||
}
|
||||
fn unmount(&mut self) {}
|
||||
fn list_dir(&self, _path: &str) -> Result<Vec<FsEntry>, FsError> {
|
||||
self.calls.list_dir.fetch_add(1, Ordering::Relaxed);
|
||||
Ok(self
|
||||
.files
|
||||
.keys()
|
||||
@ -105,17 +145,21 @@ mod tests {
|
||||
.collect())
|
||||
}
|
||||
fn read_file(&self, path: &str) -> Result<Vec<u8>, FsError> {
|
||||
self.calls.read_file.fetch_add(1, Ordering::Relaxed);
|
||||
self.files.get(path).cloned().ok_or(FsError::NotFound)
|
||||
}
|
||||
fn write_file(&mut self, path: &str, data: &[u8]) -> Result<(), FsError> {
|
||||
self.calls.write_file.fetch_add(1, Ordering::Relaxed);
|
||||
self.files.insert(path.to_string(), data.to_vec());
|
||||
Ok(())
|
||||
}
|
||||
fn delete(&mut self, path: &str) -> Result<(), FsError> {
|
||||
self.calls.delete.fetch_add(1, Ordering::Relaxed);
|
||||
self.files.remove(path);
|
||||
Ok(())
|
||||
}
|
||||
fn exists(&self, path: &str) -> bool {
|
||||
self.calls.exists.fetch_add(1, Ordering::Relaxed);
|
||||
self.files.contains_key(path)
|
||||
}
|
||||
fn is_healthy(&self) -> bool {
|
||||
@ -152,4 +196,44 @@ mod tests {
|
||||
vfs.mount(Box::new(backend)).unwrap();
|
||||
assert!(!vfs.is_healthy());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_normalize_path_rejects_parent_traversal() {
|
||||
for path in ["../x", "/../x", "/user/../../x", "\\user\\..\\..\\x"] {
|
||||
let error = VirtualFS::normalize_path(path).unwrap_err();
|
||||
assert!(matches!(error, FsError::InvalidPath(_)));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_invalid_paths_never_reach_backend() {
|
||||
let calls = Arc::new(CallCounters::default());
|
||||
let mut vfs = VirtualFS::new();
|
||||
vfs.mount(Box::new(MockBackend::with_calls(calls.clone()))).unwrap();
|
||||
|
||||
assert!(matches!(vfs.read_file("../secret.txt"), Err(FsError::InvalidPath(_))));
|
||||
assert!(matches!(
|
||||
vfs.write_file("/user/../../secret.txt", b"nope"),
|
||||
Err(FsError::InvalidPath(_))
|
||||
));
|
||||
assert!(matches!(vfs.delete("\\user\\..\\secret.txt"), Err(FsError::InvalidPath(_))));
|
||||
assert!(matches!(vfs.list_dir("/../"), Err(FsError::InvalidPath(_))));
|
||||
assert!(!vfs.exists("../x"));
|
||||
|
||||
assert_eq!(calls.read_file.load(Ordering::Relaxed), 0);
|
||||
assert_eq!(calls.write_file.load(Ordering::Relaxed), 0);
|
||||
assert_eq!(calls.delete.load(Ordering::Relaxed), 0);
|
||||
assert_eq!(calls.list_dir.load(Ordering::Relaxed), 0);
|
||||
assert_eq!(calls.exists.load(Ordering::Relaxed), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_delete_root_is_rejected_before_backend() {
|
||||
let calls = Arc::new(CallCounters::default());
|
||||
let mut vfs = VirtualFS::new();
|
||||
vfs.mount(Box::new(MockBackend::with_calls(calls.clone()))).unwrap();
|
||||
|
||||
assert!(matches!(vfs.delete("/"), Err(FsError::PermissionDenied)));
|
||||
assert_eq!(calls.delete.load(Ordering::Relaxed), 0);
|
||||
}
|
||||
}
|
||||
|
||||
@ -1,6 +1,6 @@
|
||||
use prometeu_system::fs::{FsBackend, FsEntry, FsError};
|
||||
use std::fs;
|
||||
use std::path::PathBuf;
|
||||
use std::path::{Component, Path, PathBuf};
|
||||
|
||||
pub struct HostDirBackend {
|
||||
root: PathBuf,
|
||||
@ -11,9 +11,41 @@ impl HostDirBackend {
|
||||
Self { root: root.into() }
|
||||
}
|
||||
|
||||
fn resolve(&self, path: &str) -> PathBuf {
|
||||
let path = path.trim_start_matches('/');
|
||||
self.root.join(path)
|
||||
fn resolve(&self, path: &str) -> Result<PathBuf, FsError> {
|
||||
let normalized = path.replace('\\', "/");
|
||||
let relative = normalized.strip_prefix('/').unwrap_or(&normalized);
|
||||
let mut resolved = self.root.clone();
|
||||
|
||||
for component in Path::new(relative).components() {
|
||||
match component {
|
||||
Component::Normal(segment) => resolved.push(segment),
|
||||
Component::ParentDir => {
|
||||
return Err(FsError::InvalidPath(
|
||||
"parent traversal '..' is not allowed".to_string(),
|
||||
));
|
||||
}
|
||||
Component::CurDir => {
|
||||
return Err(FsError::InvalidPath(
|
||||
"current directory '.' is not allowed".to_string(),
|
||||
));
|
||||
}
|
||||
Component::RootDir => {
|
||||
return Err(FsError::InvalidPath("unexpected root component".to_string()));
|
||||
}
|
||||
Component::Prefix(prefix) => {
|
||||
return Err(FsError::InvalidPath(format!(
|
||||
"unexpected platform path prefix: {:?}",
|
||||
prefix
|
||||
)));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if !resolved.starts_with(&self.root) {
|
||||
return Err(FsError::InvalidPath("resolved path escaped the mounted root".to_string()));
|
||||
}
|
||||
|
||||
Ok(resolved)
|
||||
}
|
||||
}
|
||||
|
||||
@ -36,7 +68,7 @@ impl FsBackend for HostDirBackend {
|
||||
fn unmount(&mut self) {}
|
||||
|
||||
fn list_dir(&self, path: &str) -> Result<Vec<FsEntry>, FsError> {
|
||||
let full_path = self.resolve(path);
|
||||
let full_path = self.resolve(path)?;
|
||||
let entries = fs::read_dir(full_path).map_err(|e| FsError::IOError(e.to_string()))?;
|
||||
|
||||
let mut result = Vec::new();
|
||||
@ -53,7 +85,7 @@ impl FsBackend for HostDirBackend {
|
||||
}
|
||||
|
||||
fn read_file(&self, path: &str) -> Result<Vec<u8>, FsError> {
|
||||
let full_path = self.resolve(path);
|
||||
let full_path = self.resolve(path)?;
|
||||
fs::read(full_path).map_err(|e| match e.kind() {
|
||||
std::io::ErrorKind::NotFound => FsError::NotFound,
|
||||
_ => FsError::IOError(e.to_string()),
|
||||
@ -61,7 +93,7 @@ impl FsBackend for HostDirBackend {
|
||||
}
|
||||
|
||||
fn write_file(&mut self, path: &str, data: &[u8]) -> Result<(), FsError> {
|
||||
let full_path = self.resolve(path);
|
||||
let full_path = self.resolve(path)?;
|
||||
if let Some(parent) = full_path.parent() {
|
||||
fs::create_dir_all(parent).map_err(|e| FsError::IOError(e.to_string()))?;
|
||||
}
|
||||
@ -69,7 +101,10 @@ impl FsBackend for HostDirBackend {
|
||||
}
|
||||
|
||||
fn delete(&mut self, path: &str) -> Result<(), FsError> {
|
||||
let full_path = self.resolve(path);
|
||||
let full_path = self.resolve(path)?;
|
||||
if full_path == self.root {
|
||||
return Err(FsError::PermissionDenied);
|
||||
}
|
||||
if full_path.is_dir() {
|
||||
fs::remove_dir_all(full_path).map_err(|e| FsError::IOError(e.to_string()))
|
||||
} else {
|
||||
@ -78,7 +113,7 @@ impl FsBackend for HostDirBackend {
|
||||
}
|
||||
|
||||
fn exists(&self, path: &str) -> bool {
|
||||
self.resolve(path).exists()
|
||||
self.resolve(path).map(|resolved| resolved.exists()).unwrap_or(false)
|
||||
}
|
||||
}
|
||||
|
||||
@ -113,4 +148,72 @@ mod tests {
|
||||
|
||||
let _ = fs::remove_dir_all(root);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_host_dir_backend_round_trip() {
|
||||
let root = get_temp_dir("round_trip");
|
||||
let mut backend = HostDirBackend::new(root.clone());
|
||||
backend.mount().unwrap();
|
||||
|
||||
let path = "/user/test.txt";
|
||||
let content = b"hello world";
|
||||
|
||||
backend.write_file(path, content).unwrap();
|
||||
assert!(backend.exists(path));
|
||||
assert_eq!(backend.read_file(path).unwrap(), content);
|
||||
|
||||
backend.delete(path).unwrap();
|
||||
assert!(!backend.exists(path));
|
||||
|
||||
let _ = fs::remove_dir_all(root);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_host_dir_backend_rejects_traversal_write_and_read() {
|
||||
let root = get_temp_dir("rejects_traversal");
|
||||
let mut backend = HostDirBackend::new(root.clone());
|
||||
backend.mount().unwrap();
|
||||
|
||||
let outside_name = format!(
|
||||
"outside_{}.txt",
|
||||
std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).unwrap().as_nanos()
|
||||
);
|
||||
let traversal_path = format!("/user/../../{}", outside_name);
|
||||
let outside_path = root.parent().unwrap().join(&outside_name);
|
||||
|
||||
assert!(matches!(
|
||||
backend.write_file(&traversal_path, b"escape"),
|
||||
Err(FsError::InvalidPath(_))
|
||||
));
|
||||
assert!(matches!(backend.read_file(&traversal_path), Err(FsError::InvalidPath(_))));
|
||||
assert!(!outside_path.exists());
|
||||
|
||||
let _ = fs::remove_dir_all(root);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_host_dir_backend_rejects_traversal_for_exists_and_list_dir() {
|
||||
let root = get_temp_dir("rejects_bool_and_list");
|
||||
let mut backend = HostDirBackend::new(root.clone());
|
||||
backend.mount().unwrap();
|
||||
|
||||
let traversal_path = "/user/../../outside";
|
||||
|
||||
assert!(!backend.exists(traversal_path));
|
||||
assert!(matches!(backend.list_dir(traversal_path), Err(FsError::InvalidPath(_))));
|
||||
|
||||
let _ = fs::remove_dir_all(root);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_host_dir_backend_rejects_delete_root() {
|
||||
let root = get_temp_dir("rejects_delete_root");
|
||||
let mut backend = HostDirBackend::new(root.clone());
|
||||
backend.mount().unwrap();
|
||||
|
||||
assert!(matches!(backend.delete("/"), Err(FsError::PermissionDenied)));
|
||||
assert!(root.exists());
|
||||
|
||||
let _ = fs::remove_dir_all(root);
|
||||
}
|
||||
}
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user