Automated security review (HIGH) caught a jail-escape my own review
missed: copy_recursive used fs::metadata (follows symlinks). A symlink
inside the jail pointing to e.g. /etc, then a 'copy' of its parent dir,
would dereference it and pull external content INTO the jail where it
could be read — a read-escape exfiltration. jail() validates only the
top-level src/dest; the recursive walk reintroduced the escape.
Fix: copy_recursive uses symlink_metadata and refuses any symlink
('symlinks are not followed across the jail boundary'). list() likewise
switched to symlink_metadata so it reports the link, never the
dereferenced target's size/type (info leak). Two regression tests added:
copy-symlink-exfil (asserts no external content lands inside) and
list-no-deref. 44/44 tests green. Rolled forward to alpha.4 (vulnerable
alpha.3 superseded).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
462 lines
16 KiB
Rust
462 lines
16 KiB
Rust
//! Integration tests for the jailed file manager.
|
|
//!
|
|
//! Each test runs in a real tempdir on the host filesystem. The jail-escape
|
|
//! tests are the security-critical section: any path that resolves outside the
|
|
//! instance root MUST be rejected regardless of how the escape is attempted.
|
|
//!
|
|
//! Coverage:
|
|
//! - Functional: list, write, read roundtrip, mkdir, rename, delete
|
|
//! - Security: dotdot traversal, absolute path injection, symlink escape
|
|
//! (POSIX symlinks only — `#[cfg(unix)]`)
|
|
|
|
use corrosion_host_agent::filemanager;
|
|
use std::path::Path;
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Helpers
|
|
// ---------------------------------------------------------------------------
|
|
|
|
/// Create a temporary directory and return its path. The directory is
|
|
/// automatically cleaned up when the `TempDir` is dropped.
|
|
fn tempdir() -> tempfile::TempDir {
|
|
tempfile::tempdir().expect("create tempdir")
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Functional tests
|
|
// ---------------------------------------------------------------------------
|
|
|
|
#[test]
|
|
fn write_read_roundtrip() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
let content = "hello from the file manager\nline 2\n";
|
|
filemanager::write(root, "test.txt", content).expect("write should succeed");
|
|
|
|
let got = filemanager::read(root, "test.txt").expect("read should succeed");
|
|
assert_eq!(got, content);
|
|
}
|
|
|
|
#[test]
|
|
fn list_returns_written_file() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
filemanager::write(root, "server.cfg", "hostname MyServer\n").expect("write");
|
|
|
|
let entries = filemanager::list(root, "").expect("list root");
|
|
let names: Vec<&str> = entries.iter().map(|e| e.name.as_str()).collect();
|
|
assert!(names.contains(&"server.cfg"), "expected 'server.cfg' in listing, got {names:?}");
|
|
}
|
|
|
|
#[test]
|
|
fn list_empty_root_is_empty() {
|
|
let dir = tempdir();
|
|
let entries = filemanager::list(dir.path(), "").expect("list empty root");
|
|
assert!(entries.is_empty(), "fresh tempdir should have no entries");
|
|
}
|
|
|
|
#[test]
|
|
fn mkdir_creates_directory() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
filemanager::mkdir(root, "cfg/custom").expect("mkdir should succeed");
|
|
|
|
assert!(root.join("cfg/custom").is_dir(), "directory should exist after mkdir");
|
|
}
|
|
|
|
#[test]
|
|
fn mkdir_creates_nested_dirs() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
filemanager::mkdir(root, "a/b/c/d").expect("mkdir nested");
|
|
assert!(root.join("a/b/c/d").is_dir());
|
|
}
|
|
|
|
#[test]
|
|
fn write_creates_parent_dirs() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
filemanager::write(root, "subdir/deep/file.txt", "data").expect("write with auto-mkdir");
|
|
let content = filemanager::read(root, "subdir/deep/file.txt").expect("read");
|
|
assert_eq!(content, "data");
|
|
}
|
|
|
|
#[test]
|
|
fn rename_file() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
filemanager::write(root, "old.txt", "content").expect("write");
|
|
filemanager::rename(root, "old.txt", "new.txt").expect("rename");
|
|
|
|
assert!(!root.join("old.txt").exists(), "old.txt should be gone");
|
|
assert!(root.join("new.txt").exists(), "new.txt should exist");
|
|
|
|
let content = filemanager::read(root, "new.txt").expect("read renamed");
|
|
assert_eq!(content, "content");
|
|
}
|
|
|
|
#[test]
|
|
fn rename_rejects_separator_in_new_name() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
filemanager::write(root, "file.txt", "data").expect("write");
|
|
|
|
let err = filemanager::rename(root, "file.txt", "subdir/escape.txt")
|
|
.expect_err("rename with path separator must fail");
|
|
assert!(
|
|
err.to_string().contains("separator"),
|
|
"error should mention separator: {err}"
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn delete_file() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
filemanager::write(root, "todelete.txt", "bye").expect("write");
|
|
assert!(root.join("todelete.txt").exists());
|
|
|
|
filemanager::delete(root, "todelete.txt").expect("delete");
|
|
assert!(!root.join("todelete.txt").exists());
|
|
}
|
|
|
|
#[test]
|
|
fn delete_directory_recursive() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
filemanager::mkdir(root, "tree/sub").expect("mkdir");
|
|
filemanager::write(root, "tree/sub/file.txt", "x").expect("write");
|
|
assert!(root.join("tree").is_dir());
|
|
|
|
filemanager::delete(root, "tree").expect("delete tree");
|
|
assert!(!root.join("tree").exists(), "directory tree should be deleted");
|
|
}
|
|
|
|
#[test]
|
|
fn mkfile_creates_empty_file() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
filemanager::mkfile(root, "empty.txt").expect("mkfile");
|
|
let content = filemanager::read(root, "empty.txt").expect("read empty file");
|
|
assert_eq!(content, "");
|
|
}
|
|
|
|
#[test]
|
|
fn copy_file() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
filemanager::write(root, "source.txt", "original").expect("write source");
|
|
filemanager::copy(root, "source.txt", "dest.txt").expect("copy");
|
|
|
|
let src = filemanager::read(root, "source.txt").expect("read source after copy");
|
|
let dst = filemanager::read(root, "dest.txt").expect("read destination");
|
|
assert_eq!(src, "original");
|
|
assert_eq!(dst, "original");
|
|
}
|
|
|
|
#[test]
|
|
fn move_file() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
filemanager::write(root, "moveme.txt", "payload").expect("write");
|
|
filemanager::move_path(root, "moveme.txt", "moved.txt").expect("move");
|
|
|
|
assert!(!root.join("moveme.txt").exists(), "source should be gone");
|
|
let content = filemanager::read(root, "moved.txt").expect("read after move");
|
|
assert_eq!(content, "payload");
|
|
}
|
|
|
|
#[test]
|
|
fn list_entry_fields_are_populated() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
filemanager::write(root, "check.txt", "abcde").expect("write");
|
|
filemanager::mkdir(root, "subdir").expect("mkdir");
|
|
|
|
let entries = filemanager::list(root, "").expect("list");
|
|
// Dirs sort before files.
|
|
let dir_entry = entries.iter().find(|e| e.name == "subdir").expect("subdir entry");
|
|
assert!(dir_entry.is_dir);
|
|
assert_eq!(dir_entry.size, 0);
|
|
assert!(!dir_entry.modified.is_empty(), "modified should be set");
|
|
|
|
let file_entry = entries.iter().find(|e| e.name == "check.txt").expect("file entry");
|
|
assert!(!file_entry.is_dir);
|
|
assert_eq!(file_entry.size, 5, "size should match byte count");
|
|
// path should be relative and use forward slashes.
|
|
assert!(!file_entry.path.starts_with('/'), "path should be relative");
|
|
assert!(!file_entry.path.contains('\\'), "path should use forward slashes");
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Security: jail-escape tests
|
|
// CRITICAL — these are the whole point of the jail abstraction.
|
|
// ---------------------------------------------------------------------------
|
|
|
|
/// `../../etc/passwd` must never resolve outside the instance root.
|
|
#[test]
|
|
fn jail_rejects_dotdot_traversal() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
let err = filemanager::read(root, "../../etc/passwd")
|
|
.expect_err("dotdot traversal must be rejected");
|
|
// Verify the error is security-related and not just "file not found".
|
|
let msg = err.to_string();
|
|
assert!(
|
|
msg.contains("outside") || msg.contains("escapes") || msg.contains("escape"),
|
|
"error should mention jail escape for dotdot traversal, got: {msg}"
|
|
);
|
|
}
|
|
|
|
/// A deeply nested `../` chain must also be stopped.
|
|
#[test]
|
|
fn jail_rejects_deep_dotdot_traversal() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
let err = filemanager::read(root, "a/b/c/../../../../../../../../etc/shadow")
|
|
.expect_err("deep dotdot traversal must be rejected");
|
|
let msg = err.to_string();
|
|
assert!(
|
|
msg.contains("outside") || msg.contains("escapes") || msg.contains("escape") || msg.contains("absolute"),
|
|
"error should mention jail escape for deep traversal, got: {msg}"
|
|
);
|
|
}
|
|
|
|
/// An absolute path (e.g. `/etc/passwd`) must be rejected immediately — it
|
|
/// completely bypasses relative joining and should never be accepted.
|
|
#[test]
|
|
fn jail_rejects_absolute_path() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
let err = filemanager::read(root, "/etc/passwd")
|
|
.expect_err("absolute path must be rejected");
|
|
let msg = err.to_string();
|
|
assert!(
|
|
msg.contains("absolute") || msg.contains("outside") || msg.contains("escapes") || msg.contains("escape"),
|
|
"error should mention the absolute-path rejection, got: {msg}"
|
|
);
|
|
}
|
|
|
|
/// An absolute path to a Windows-style location must also be rejected.
|
|
#[test]
|
|
fn jail_rejects_absolute_windows_style_path() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
// On POSIX this is just treated as an absolute path starting with `/`.
|
|
// The test is intentionally platform-portable: any absolute path is bad.
|
|
let err = filemanager::read(root, "/tmp/evil")
|
|
.expect_err("absolute /tmp/evil must be rejected");
|
|
let msg = err.to_string();
|
|
assert!(
|
|
msg.contains("absolute") || msg.contains("outside") || msg.contains("escapes") || msg.contains("escape"),
|
|
"got: {msg}"
|
|
);
|
|
}
|
|
|
|
/// A symlink inside the root that points to a path outside the root must not
|
|
/// be followed. This is the critical symlink-escape vector.
|
|
#[cfg(unix)]
|
|
#[test]
|
|
fn jail_rejects_symlink_escape() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
// Create a directory outside the root to be the symlink target.
|
|
let outside = tempdir();
|
|
let outside_file = outside.path().join("secret.txt");
|
|
std::fs::write(&outside_file, "secret data").expect("write outside file");
|
|
|
|
// Plant a symlink inside the root pointing to the outside directory.
|
|
let link_path = root.join("evil_link");
|
|
std::os::unix::fs::symlink(outside.path(), &link_path)
|
|
.expect("create symlink inside root");
|
|
|
|
// Attempt to read through the symlink.
|
|
let err = filemanager::read(root, "evil_link/secret.txt")
|
|
.expect_err("symlink escape must be rejected");
|
|
let msg = err.to_string();
|
|
assert!(
|
|
msg.contains("outside") || msg.contains("escapes") || msg.contains("escape"),
|
|
"error should mention jail escape for symlink traversal, got: {msg}"
|
|
);
|
|
}
|
|
|
|
/// A symlink directly inside the root pointing to a file outside must be
|
|
/// rejected even when the path looks like a normal relative reference.
|
|
#[cfg(unix)]
|
|
#[test]
|
|
fn jail_rejects_symlink_pointing_directly_outside() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
// Symlink to /etc/passwd itself (or any outside path that exists or not).
|
|
let link_path = root.join("passwd_link");
|
|
std::os::unix::fs::symlink(Path::new("/etc/passwd"), &link_path)
|
|
.expect("create symlink to /etc/passwd");
|
|
|
|
let err = filemanager::read(root, "passwd_link")
|
|
.expect_err("direct symlink outside root must be rejected");
|
|
let msg = err.to_string();
|
|
assert!(
|
|
msg.contains("outside") || msg.contains("escapes") || msg.contains("escape"),
|
|
"error should mention jail escape, got: {msg}"
|
|
);
|
|
}
|
|
|
|
/// A symlink chain (symlink → symlink → outside) must also be caught.
|
|
#[cfg(unix)]
|
|
#[test]
|
|
fn jail_rejects_chained_symlink_escape() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
let outside = tempdir();
|
|
|
|
// Chain: root/link1 → root/link2 → outside/
|
|
let link2_path = root.join("link2");
|
|
std::os::unix::fs::symlink(outside.path(), &link2_path)
|
|
.expect("create link2");
|
|
|
|
let link1_path = root.join("link1");
|
|
std::os::unix::fs::symlink(&link2_path, &link1_path)
|
|
.expect("create link1");
|
|
|
|
let err = filemanager::read(root, "link1")
|
|
.expect_err("chained symlink escape must be rejected");
|
|
let msg = err.to_string();
|
|
assert!(
|
|
msg.contains("outside") || msg.contains("escapes") || msg.contains("escape"),
|
|
"chained symlink should be caught, got: {msg}"
|
|
);
|
|
}
|
|
|
|
/// SECURITY REGRESSION: copying a directory that contains a symlink pointing
|
|
/// OUTSIDE the jail must NOT dereference it and pull external content inside.
|
|
/// jail() validates only the top-level src/dest; the recursive copy must
|
|
/// refuse symlinks itself or it becomes a read-escape exfiltration path.
|
|
#[cfg(unix)]
|
|
#[test]
|
|
fn copy_refuses_to_follow_symlink_out_of_jail() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
let outside = tempdir();
|
|
std::fs::write(outside.path().join("secret.txt"), "TOP SECRET")
|
|
.expect("write external secret");
|
|
|
|
// A directory inside the jail containing a symlink to the outside dir.
|
|
std::fs::create_dir(root.join("src")).expect("mkdir src");
|
|
std::os::unix::fs::symlink(outside.path(), root.join("src").join("escape"))
|
|
.expect("plant symlink to outside");
|
|
|
|
// Attempt to copy src -> dest (both inside the jail).
|
|
let err = filemanager::copy(root, "src", "dest")
|
|
.expect_err("copy must refuse the embedded symlink");
|
|
assert!(
|
|
format!("{err:#}").contains("symlink"),
|
|
"error should name the refused symlink, got: {err:#}"
|
|
);
|
|
|
|
// The external secret must NOT have landed inside the jail.
|
|
assert!(
|
|
!root.join("dest").join("escape").join("secret.txt").exists(),
|
|
"external content leaked into the jail via symlink-following copy",
|
|
);
|
|
}
|
|
|
|
/// `list` must report a symlink as the link itself, never the dereferenced
|
|
/// target — otherwise it leaks the size/type of files outside the jail.
|
|
#[cfg(unix)]
|
|
#[test]
|
|
fn list_does_not_dereference_symlink_metadata() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
std::os::unix::fs::symlink(Path::new("/etc/passwd"), root.join("leak"))
|
|
.expect("plant symlink");
|
|
|
|
let entries = filemanager::list(root, "").expect("list root");
|
|
let leak = entries.iter().find(|e| e.name == "leak").expect("symlink listed");
|
|
// /etc/passwd is a regular file; if we followed the link, is_dir would
|
|
// reflect the target. We must report the link, which is not a directory,
|
|
// and must NOT expose the target's byte size.
|
|
assert!(!leak.is_dir, "symlink must not be reported as a directory");
|
|
let target_size = std::fs::metadata("/etc/passwd").map(|m| m.len()).unwrap_or(0);
|
|
assert!(
|
|
leak.size != target_size || target_size == 0,
|
|
"list leaked the symlink target's size ({target_size} bytes)"
|
|
);
|
|
}
|
|
|
|
// ---------------------------------------------------------------------------
|
|
// Dispatch layer tests
|
|
// ---------------------------------------------------------------------------
|
|
|
|
#[test]
|
|
fn dispatch_list_returns_success() {
|
|
let dir = tempdir();
|
|
let root = dir.path();
|
|
|
|
filemanager::write(root, "a.txt", "a").expect("write");
|
|
|
|
let req = filemanager::FileRequest {
|
|
op: "list".to_string(),
|
|
path: String::new(),
|
|
dest: None,
|
|
content: None,
|
|
name: None,
|
|
};
|
|
let resp = filemanager::dispatch(root, &req);
|
|
assert_eq!(resp["status"], "success");
|
|
assert!(resp["data"]["entries"].is_array());
|
|
}
|
|
|
|
#[test]
|
|
fn dispatch_unknown_op_returns_error() {
|
|
let dir = tempdir();
|
|
let req = filemanager::FileRequest {
|
|
op: "explode".to_string(),
|
|
path: String::new(),
|
|
dest: None,
|
|
content: None,
|
|
name: None,
|
|
};
|
|
let resp = filemanager::dispatch(dir.path(), &req);
|
|
assert_eq!(resp["status"], "error");
|
|
assert!(resp["message"].as_str().unwrap().contains("unknown op"));
|
|
}
|
|
|
|
#[test]
|
|
fn dispatch_escape_attempt_returns_error_not_panic() {
|
|
let dir = tempdir();
|
|
let req = filemanager::FileRequest {
|
|
op: "read".to_string(),
|
|
path: "../../etc/passwd".to_string(),
|
|
dest: None,
|
|
content: None,
|
|
name: None,
|
|
};
|
|
let resp = filemanager::dispatch(dir.path(), &req);
|
|
// Must return an error response, not panic or expose the file.
|
|
assert_eq!(resp["status"], "error", "escape attempt should return error status");
|
|
assert!(
|
|
resp["message"].as_str().is_some(),
|
|
"error response must have a message"
|
|
);
|
|
}
|