Skip to content

Commit

Permalink
refactor(monofs): improve path handling and cleanup dependencies (#100)
Browse files Browse the repository at this point in the history
- Add path normalization to handle . and .. components safely
- Add tests for path traversal security
- Remove deprecated monoutils-nfs package
- Remove experimental overlayfs feature flag
- Remove unused monocore modules (oci, orchestration, server)
- Clean up filesystem entity type handling
- Update monoutils-raft version to 0.1.0
  • Loading branch information
appcypher authored Jan 8, 2025
1 parent 439ea2d commit 445a0c0
Show file tree
Hide file tree
Showing 8 changed files with 134 additions and 125 deletions.
6 changes: 1 addition & 5 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ members = [
"monopacks",
"monoutils",
"monoutils-did",
"monoutils-nfs",
"monoutils-raft",
"monoutils-store",
"monoutils-ucan",
Expand Down
39 changes: 1 addition & 38 deletions monocore/lib/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,51 +30,14 @@
//! # Usage Example
//!
//! ```rust,no_run
//! use monocore::{
//! config::{Group, Monocore, Service},
//! orchestration::Orchestrator,
//! };
//!
//! #[tokio::main]
//! async fn main() -> anyhow::Result<()> {
//! // Configure a service
//! let service = Service::builder()
//! .name("ai-agent")
//! .base("alpine:latest")
//! .ram(512)
//! .build();
//!
//! // Create monocore config
//! let config = Monocore::builder()
//! .services(vec![service])
//! .groups(vec![Group::builder().name("agents").build()])
//! .build()?;
//!
//! // Start orchestrator
//! let mut orchestrator = Orchestrator::new("/path/to/home_dir", "/path/to/supervisor").await?;
//! orchestrator.up(config).await?;
//!
//! Ok(())
//! }
//! // TODO
//! ```
//!
//! # Feature Flags
//!
//! - `overlayfs` - Enables experimental overlayfs support on Linux
//! - Not recommended for production use
//! - Does not support OCI whiteout files
//! - May have permission issues
//! - Falls back to copy-based merge on failure
//! - Will be replaced by monofs in the future for a more robust solution
//!
//! # Modules
//!
//! - [`cli`] - Command-line interface and argument parsing
//! - [`config`] - Configuration types and validation
//! - [`oci`] - OCI image handling and distribution
//! - [`orchestration`] - Service lifecycle management
//! - [`runtime`] - Process supervision and monitoring
//! - [`server`] - REST API server implementation
//! - [`utils`] - Common utilities and helpers
//! - [`vm`] - MicroVM configuration and control
//!
Expand Down
2 changes: 1 addition & 1 deletion monocore/lib/log/rotating.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use tokio::{
/// # Example
///
/// ```no_run
/// use monocore::runtime::RotatingLog;
/// use monocore::RotatingLog;
///
/// #[tokio::main]
/// async fn main() -> std::io::Result<()> {
Expand Down
16 changes: 4 additions & 12 deletions monocore/tests/cli/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ fn verify_init_results(dir_path: &std::path::Path) {
assert!(log_path.exists(), "log directory should exist");
assert!(log_path.is_dir(), "log should be a directory");

// Verify state.db was created
let db_path = menv_path.join("state.db");
assert!(db_path.exists(), "state.db should exist");
assert!(db_path.is_file(), "state.db should be a file");
// Verify active.db was created
let db_path = menv_path.join("active.db");
assert!(db_path.exists(), "active.db should exist");
assert!(db_path.is_file(), "active.db should be a file");

// Verify monocore.yaml was created
let config_path = dir_path.join("monocore.yaml");
Expand All @@ -32,18 +32,10 @@ fn verify_init_results(dir_path: &std::path::Path) {

// Verify monocore.yaml contents
let config_contents = fs::read_to_string(config_path).unwrap();
assert!(
config_contents.contains("meta:"),
"config should have meta section"
);
assert!(
config_contents.contains("sandboxes:"),
"config should have sandboxes section"
);
assert!(
config_contents.contains("groups:"),
"config should have groups section"
);
}

//--------------------------------------------------------------------------------------------------
Expand Down
27 changes: 0 additions & 27 deletions monofs/examples/test.rs

This file was deleted.

148 changes: 118 additions & 30 deletions monofs/lib/filesystem/dir/find.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,16 @@ pub async fn find_dir<S>(mut dir: &Dir<S>, path: impl AsRef<str>) -> FsResult<Fi
where
S: IpldStore + Send + Sync,
{
let path = Utf8UnixPath::new(path.as_ref());
// Normalize the path first - this will handle . and .. components and validate the path
let normalized_path =
monoutils::normalize_path(path.as_ref(), monoutils::SupportedPathType::Relative)
.map_err(|_| FsError::InvalidSearchPath(path.as_ref().to_string()))?;

// Convert path components to Utf8UnixPathSegment and collect them
let components = path
let components = Utf8UnixPath::new(&normalized_path)
.components()
.map(|ref c| match c {
Utf8UnixComponent::RootDir => Err(FsError::InvalidSearchPath(path.to_string())),
Utf8UnixComponent::CurDir => Err(FsError::InvalidSearchPath(path.to_string())),
Utf8UnixComponent::ParentDir => Err(FsError::InvalidSearchPath(path.to_string())),
_ => Ok(Utf8UnixPathSegment::try_from(c)?),
.filter_map(|c| match c {
Utf8UnixComponent::Normal(s) => Some(Utf8UnixPathSegment::try_from(s)),
_ => None, // Skip any . or .. since they were handled by normalize_path
})
.collect::<Result<Vec<_>, _>>()?;

Expand All @@ -101,7 +101,7 @@ where
}
Some(Entity::SymCidLink(_)) => {
// SymCidLinks are not supported yet, so we return an error
return Err(FsError::SymCidLinkNotSupportedYet(components.clone()));
return Err(FsError::SymCidLinkNotSupportedYet(components));
}
Some(_) => {
// If we encounter a non-directory entity in the middle of the path,
Expand Down Expand Up @@ -156,16 +156,16 @@ pub async fn find_dir_mut<S>(
where
S: IpldStore + Send + Sync,
{
let path = Utf8UnixPath::new(path.as_ref());
// Normalize the path first - this will handle . and .. components and validate the path
let normalized_path =
monoutils::normalize_path(path.as_ref(), monoutils::SupportedPathType::Relative)
.map_err(|_| FsError::InvalidSearchPath(path.as_ref().to_string()))?;

// Convert path components to Utf8UnixPathSegment and collect them
let components = path
let components = Utf8UnixPath::new(&normalized_path)
.components()
.map(|ref c| match c {
Utf8UnixComponent::RootDir => Err(FsError::InvalidSearchPath(path.to_string())),
Utf8UnixComponent::CurDir => Err(FsError::InvalidSearchPath(path.to_string())),
Utf8UnixComponent::ParentDir => Err(FsError::InvalidSearchPath(path.to_string())),
_ => Ok(Utf8UnixPathSegment::try_from(c)?),
.filter_map(|c| match c {
Utf8UnixComponent::Normal(s) => Some(Utf8UnixPathSegment::try_from(s)),
_ => None, // Skip any . or .. since they were handled by normalize_path
})
.collect::<Result<Vec<_>, _>>()?;

Expand All @@ -178,7 +178,7 @@ where
}
Some(Entity::SymCidLink(_)) => {
// SymCidLinks are not supported yet, so we return an error
return Err(FsError::SymCidLinkNotSupportedYet(components.clone()));
return Err(FsError::SymCidLinkNotSupportedYet(components));
}
Some(_) => {
// If we encounter a non-directory entity in the middle of the path,
Expand Down Expand Up @@ -221,26 +221,42 @@ pub async fn find_or_create_dir<S>(dir: &mut Dir<S>, path: impl AsRef<str>) -> F
where
S: IpldStore + Send + Sync,
{
let path = Utf8UnixPath::new(path.as_ref());

match find_dir_mut(dir, path).await {
match find_dir_mut(dir, path.as_ref()).await {
Ok(FindResult::Found { dir }) => Ok(dir),
Ok(FindResult::NotFound { mut dir, depth }) => {
for component in path.components().skip(depth) {
let new_dir = Dir::new(dir.get_store().clone());
let segment = Utf8UnixPathSegment::try_from(&component)?;
let normalized_path =
monoutils::normalize_path(path.as_ref(), monoutils::SupportedPathType::Relative)
.map_err(|_| FsError::InvalidSearchPath(path.as_ref().to_string()))?;

let components = Utf8UnixPath::new(&normalized_path)
.components()
.skip(depth)
.filter_map(|c| match c {
Utf8UnixComponent::Normal(s) => Some(Utf8UnixPathSegment::try_from(s)),
_ => None, // Skip any . or .. since they were handled by normalize_path
})
.collect::<Result<Vec<_>, _>>()?;

for segment in components {
let new_dir = Dir::new(dir.get_store().clone());
dir.put_dir(segment.clone(), new_dir)?;
dir = dir.get_dir_mut(&segment).await?.unwrap();
}

Ok(dir)
}
Ok(FindResult::NotADir { depth }) => {
let components = path
let normalized_path =
monoutils::normalize_path(path.as_ref(), monoutils::SupportedPathType::Relative)
.map_err(|_| FsError::InvalidSearchPath(path.as_ref().to_string()))?;

let components = Utf8UnixPath::new(&normalized_path)
.components()
.take(depth + 1)
.map(|c| c.to_string())
.filter_map(|c| match c {
Utf8UnixComponent::Normal(s) => Some(s.to_string()),
_ => None,
})
.collect::<Vec<_>>();

Err(FsError::NotADirectory(components.join("/")))
Expand Down Expand Up @@ -303,6 +319,16 @@ mod tests {
let result = find_dir(&root, "subdir1/subdir2").await?;
assert!(matches!(result, FindResult::Found { .. }));

// Test with . and .. components that resolve within bounds
let result = find_dir(&root, "subdir1/./subdir2").await?;
assert!(matches!(result, FindResult::Found { .. }));

let result = find_dir(&root, "subdir1/subdir2/../subdir2").await?;
assert!(matches!(result, FindResult::Found { .. }));

let result = find_dir(&root, "./subdir1").await?;
assert!(matches!(result, FindResult::Found { .. }));

// Test finding non-existent directories
let result = find_dir(&root, "nonexistent").await?;
assert!(matches!(result, FindResult::NotFound { depth: 0, .. }));
Expand All @@ -314,14 +340,21 @@ mod tests {
let result = find_dir(&root, "subdir1/file1.txt/invalid").await?;
assert!(matches!(result, FindResult::NotADir { depth: 1 }));

// Test invalid paths
let result = find_dir(&root, "/invalid/path").await;
// Test path escape attempts - these should fail
let result = find_dir(&root, "..").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

let result = find_dir(&root, "invalid/../path").await;
let result = find_dir(&root, "../escaped").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

let result = find_dir(&root, "./invalid/path").await;
let result = find_dir(&root, "subdir1/../../escaped").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

// Test invalid paths
let result = find_dir(&root, "/invalid/path").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

let result = find_dir(&root, "").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

Ok(())
Expand All @@ -338,6 +371,16 @@ mod tests {
let result = find_dir_mut(&mut root, "subdir1/subdir2").await?;
assert!(matches!(result, FindResult::Found { .. }));

// Test with . and .. components that resolve within bounds
let result = find_dir_mut(&mut root, "subdir1/./subdir2").await?;
assert!(matches!(result, FindResult::Found { .. }));

let result = find_dir_mut(&mut root, "subdir1/subdir2/../subdir2").await?;
assert!(matches!(result, FindResult::Found { .. }));

let result = find_dir_mut(&mut root, "./subdir1").await?;
assert!(matches!(result, FindResult::Found { .. }));

// Test finding non-existent directories
let result = find_dir_mut(&mut root, "nonexistent").await?;
assert!(matches!(result, FindResult::NotFound { depth: 0, .. }));
Expand All @@ -349,6 +392,23 @@ mod tests {
let result = find_dir_mut(&mut root, "subdir1/file1.txt/invalid").await?;
assert!(matches!(result, FindResult::NotADir { depth: 1 }));

// Test path escape attempts - these should fail
let result = find_dir_mut(&mut root, "..").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

let result = find_dir_mut(&mut root, "../escaped").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

let result = find_dir_mut(&mut root, "subdir1/../../escaped").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

// Test invalid paths
let result = find_dir_mut(&mut root, "/invalid/path").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

let result = find_dir_mut(&mut root, "").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

Ok(())
}

Expand All @@ -372,6 +432,17 @@ mod tests {
let result = find_dir(&root, "parent/child/grandchild").await?;
assert!(matches!(result, FindResult::Found { .. }));

// Test with . and .. components that resolve within bounds
let dir1 = find_or_create_dir(&mut root, "test/./dir1").await?;
assert!(dir1.is_empty());
let result = find_dir(&root, "test/dir1").await?;
assert!(matches!(result, FindResult::Found { .. }));

let dir2 = find_or_create_dir(&mut root, "test/temp/../dir2").await?;
assert!(dir2.is_empty());
let result = find_dir(&root, "test/dir2").await?;
assert!(matches!(result, FindResult::Found { .. }));

// Test getting an existing directory
let existing_dir = find_or_create_dir(&mut root, "subdir1").await?;
assert!(!existing_dir.is_empty());
Expand All @@ -380,6 +451,23 @@ mod tests {
let result = find_or_create_dir(&mut root, "subdir1/file1.txt").await;
assert!(matches!(result, Err(FsError::NotADirectory(_))));

// Test path escape attempts - these should fail
let result = find_or_create_dir(&mut root, "..").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

let result = find_or_create_dir(&mut root, "../escaped").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

let result = find_or_create_dir(&mut root, "test/../../escaped").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

// Test invalid paths
let result = find_or_create_dir(&mut root, "/invalid/path").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

let result = find_or_create_dir(&mut root, "").await;
assert!(matches!(result, Err(FsError::InvalidSearchPath(_))));

Ok(())
}
}
Loading

0 comments on commit 445a0c0

Please sign in to comment.