fix: bound filesystem execution and preserve guarantees
This commit is contained in:
@@ -8,14 +8,19 @@ envelopes, batching, and HTTP status mapping.
|
||||
|
||||
## Security model
|
||||
|
||||
- The effective root is opened once as a directory capability. Every request is
|
||||
resolved segment-by-segment with directory-relative, `O_NOFOLLOW` handles.
|
||||
- The effective root is opened once as a directory capability. Root path
|
||||
components must be real directories (symlink components are rejected), and the
|
||||
displayed effective root is read back from that same pinned descriptor. Every
|
||||
request is resolved segment-by-segment with directory-relative, `O_NOFOLLOW`
|
||||
handles.
|
||||
- Symlinks are projected as links and are never followed. Their stored target is
|
||||
available only through the `SymlinkInspect.read_link` application operation.
|
||||
- Regular files, directories, symlinks, and unsupported entry types have distinct
|
||||
public descriptions and capability sets.
|
||||
- WIP names are Unicode. Linux names that are not valid UTF-8 are deliberately
|
||||
not published; valid UTF-8 names map byte-for-byte and therefore cannot collide.
|
||||
Unicode paths containing backend-invalid NUL bytes are likewise treated as
|
||||
unpublished rather than as backend failures.
|
||||
- No object `ref` is issued. This avoids claiming representation identity for
|
||||
hard links or path-specific names. Regular files and symlinks receive an opaque
|
||||
validator containing filesystem identity plus protected metadata; replacing a
|
||||
@@ -122,8 +127,9 @@ standard `POST /wip/v1/batch` route. `call_operation` is never batchable.
|
||||
|
||||
`ResourceLimits` bounds request/response bytes, JSON nesting, batch items, file
|
||||
read/write bytes, tree depth/node count, pinned handles, concurrent requests, and
|
||||
a cooperative deadline. Tree traversal is breadth-first and opens only requested
|
||||
entries. Once a limit or depth boundary is reached it reads only far enough to
|
||||
a cooperative deadline. Advisory file-lock acquisition uses a nonblocking loop and
|
||||
checks that deadline/cancellation rather than allowing a contended lock to stall
|
||||
a worker. Tree traversal is breadth-first and opens only requested entries. Once a limit or depth boundary is reached it reads only far enough to
|
||||
prove whether an additional visible child exists, so `truncated` is true only
|
||||
when something was actually omitted. I/O failure and cancellation remain errors,
|
||||
not clean exhaustion.
|
||||
|
||||
@@ -18,7 +18,7 @@ mod projection;
|
||||
mod server;
|
||||
|
||||
use std::net::{IpAddr, Ipv4Addr, SocketAddr};
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
|
||||
pub use interfaces::{FileRead, FileWrite, SymlinkInspect};
|
||||
@@ -138,10 +138,6 @@ impl ServerConfig {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(crate) fn effective_root(&self) -> Result<PathBuf, std::io::Error> {
|
||||
std::fs::canonicalize(&self.root)
|
||||
}
|
||||
|
||||
pub(crate) fn endpoint(&self) -> Result<wip_https::Endpoint, ConfigError> {
|
||||
let endpoint = format!("https://localhost{}", self.endpoint_base_path);
|
||||
wip_https::Endpoint::parse(&endpoint).map_err(|_| ConfigError::InvalidBasePath)
|
||||
@@ -189,9 +185,3 @@ fn valid_base_path(path: &str) -> bool {
|
||||
.any(|segment| segment == "." || segment == "..")
|
||||
&& (path == "/" || !path.ends_with('/'))
|
||||
}
|
||||
|
||||
/// Returns whether `root` is an existing directory suitable for projection.
|
||||
#[must_use]
|
||||
pub fn is_projectable_root(root: &Path) -> bool {
|
||||
root.is_dir()
|
||||
}
|
||||
|
||||
@@ -2,6 +2,7 @@ use std::collections::VecDeque;
|
||||
use std::ffi::CStr;
|
||||
use std::mem::MaybeUninit;
|
||||
use std::os::fd::{AsRawFd, OwnedFd};
|
||||
use std::path::{Component, Path, PathBuf};
|
||||
use std::sync::atomic::{AtomicUsize, Ordering};
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
@@ -173,7 +174,6 @@ pub(crate) struct Projection {
|
||||
access: AccessMode,
|
||||
limits: ResourceLimits,
|
||||
handles: Arc<HandleCounter>,
|
||||
write_gate: Mutex<()>,
|
||||
injection: Mutex<FailureInjection>,
|
||||
}
|
||||
|
||||
@@ -183,13 +183,7 @@ impl Projection {
|
||||
access: AccessMode,
|
||||
limits: ResourceLimits,
|
||||
) -> Result<Self, std::io::Error> {
|
||||
let root = openat(
|
||||
CWD,
|
||||
root,
|
||||
OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC,
|
||||
FsMode::empty(),
|
||||
)
|
||||
.map_err(std::io::Error::from)?;
|
||||
let root = open_root(root)?;
|
||||
let mut registry = InterfaceRegistry::new();
|
||||
let read_interface = registry
|
||||
.register(FileReadDefinition, None)
|
||||
@@ -212,11 +206,14 @@ impl Projection {
|
||||
current: AtomicUsize::new(0),
|
||||
maximum: limits.max_open_handles,
|
||||
}),
|
||||
write_gate: Mutex::new(()),
|
||||
injection: Mutex::new(FailureInjection::None),
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn effective_root(&self) -> Result<PathBuf, std::io::Error> {
|
||||
std::fs::read_link(format!("/proc/self/fd/{}", self.root.as_raw_fd()))
|
||||
}
|
||||
|
||||
pub(crate) fn fetch_interface(
|
||||
&self,
|
||||
request: &FetchInterfaceRequest,
|
||||
@@ -367,6 +364,9 @@ impl Projection {
|
||||
let mut segments = path[1..].split('/').peekable();
|
||||
while let Some(segment) = segments.next() {
|
||||
check_context(context)?;
|
||||
if segment.as_bytes().contains(&0) {
|
||||
return Err(ProjectionError::NotFound);
|
||||
}
|
||||
let probe = openat(
|
||||
&parent,
|
||||
segment,
|
||||
@@ -486,15 +486,6 @@ impl ObjectProvider for Projection {
|
||||
path: &str,
|
||||
operation: impl FnOnce(&PublishedObject<Self::Object>) -> OperationAttempt,
|
||||
) -> ProviderResult<OperationAttempt> {
|
||||
let _gate = match self.write_gate.lock() {
|
||||
Ok(gate) => gate,
|
||||
Err(_) => {
|
||||
return ProviderResult::Failure(HostError::internal(
|
||||
"filesystem operation lock failed",
|
||||
EffectGuarantee::NotCommitted,
|
||||
));
|
||||
}
|
||||
};
|
||||
let entry = match self.resolve(context, path) {
|
||||
Ok(entry) => entry,
|
||||
Err(ProjectionError::NotFound) => return ProviderResult::NotFound,
|
||||
@@ -511,14 +502,23 @@ impl ObjectProvider for Projection {
|
||||
));
|
||||
}
|
||||
};
|
||||
if entry.kind == EntryKind::Regular
|
||||
&& flock(entry.fd(), FlockOperation::LockExclusive).is_err()
|
||||
{
|
||||
if entry.kind == EntryKind::Regular {
|
||||
match lock_with_context(entry.fd(), context, self.limits.request_timeout) {
|
||||
Ok(()) => {}
|
||||
Err(ProjectionError::Resource) => {
|
||||
return ProviderResult::Failure(HostError::resource_limit(
|
||||
"filesystem lock deadline exceeded",
|
||||
EffectGuarantee::NotCommitted,
|
||||
));
|
||||
}
|
||||
Err(ProjectionError::Internal | ProjectionError::NotFound) => {
|
||||
return ProviderResult::Failure(HostError::internal(
|
||||
"filesystem object could not be locked",
|
||||
EffectGuarantee::NotCommitted,
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
let publication = self.publication(entry.clone(), context.clone());
|
||||
let mut attempt = operation(&publication);
|
||||
if let OperationAttempt::Started(Ok(result)) = &mut attempt {
|
||||
@@ -661,6 +661,71 @@ impl FileWrite for FilesystemObject {
|
||||
}
|
||||
}
|
||||
|
||||
fn open_root(path: &Path) -> Result<OwnedFd, std::io::Error> {
|
||||
let start = if path.is_absolute() { "/" } else { "." };
|
||||
let mut current = openat(
|
||||
CWD,
|
||||
start,
|
||||
OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC,
|
||||
FsMode::empty(),
|
||||
)
|
||||
.map_err(std::io::Error::from)?;
|
||||
for component in path.components() {
|
||||
let name = match component {
|
||||
Component::RootDir | Component::CurDir => continue,
|
||||
Component::Normal(name) => name,
|
||||
Component::ParentDir | Component::Prefix(_) => {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::InvalidInput,
|
||||
"filesystem root must not contain parent or platform-prefix components",
|
||||
));
|
||||
}
|
||||
};
|
||||
let probe = openat(
|
||||
¤t,
|
||||
name,
|
||||
OFlags::PATH | OFlags::NOFOLLOW | OFlags::CLOEXEC,
|
||||
FsMode::empty(),
|
||||
)
|
||||
.map_err(std::io::Error::from)?;
|
||||
let stat = fstat(&probe).map_err(std::io::Error::from)?;
|
||||
if EntryKind::from_mode(stat.st_mode) != EntryKind::Directory {
|
||||
return Err(std::io::Error::new(
|
||||
std::io::ErrorKind::NotADirectory,
|
||||
"filesystem root contains a symlink or non-directory component",
|
||||
));
|
||||
}
|
||||
current = reopen(&probe, OFlags::RDONLY | OFlags::DIRECTORY)
|
||||
.map_err(|_| std::io::Error::other("filesystem root could not be pinned"))?;
|
||||
}
|
||||
Ok(current)
|
||||
}
|
||||
|
||||
fn lock_with_context(
|
||||
fd: &OwnedFd,
|
||||
context: &ExecutionContext,
|
||||
maximum_wait: std::time::Duration,
|
||||
) -> Result<(), ProjectionError> {
|
||||
let now = std::time::Instant::now();
|
||||
let fallback_deadline = now.checked_add(maximum_wait).unwrap_or(now);
|
||||
let deadline = context.deadline().map_or(fallback_deadline, |configured| {
|
||||
configured.min(fallback_deadline)
|
||||
});
|
||||
loop {
|
||||
check_context(context)?;
|
||||
if std::time::Instant::now() >= deadline {
|
||||
return Err(ProjectionError::Resource);
|
||||
}
|
||||
match flock(fd, FlockOperation::NonBlockingLockExclusive) {
|
||||
Ok(()) => return Ok(()),
|
||||
Err(rustix::io::Errno::WOULDBLOCK) => {
|
||||
std::thread::sleep(std::time::Duration::from_millis(1));
|
||||
}
|
||||
Err(_) => return Err(ProjectionError::Internal),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn reopen(fd: &OwnedFd, flags: OFlags) -> Result<OwnedFd, ProjectionError> {
|
||||
let path = format!("/proc/self/fd/{}", fd.as_raw_fd());
|
||||
openat(CWD, path, flags | OFlags::CLOEXEC, FsMode::empty()).map_err(classify_lookup)
|
||||
@@ -683,7 +748,13 @@ fn classify_lookup(error: rustix::io::Errno) -> ProjectionError {
|
||||
use rustix::io::Errno;
|
||||
if matches!(
|
||||
error,
|
||||
Errno::NOENT | Errno::NOTDIR | Errno::LOOP | Errno::ACCESS | Errno::PERM
|
||||
Errno::NOENT
|
||||
| Errno::NOTDIR
|
||||
| Errno::LOOP
|
||||
| Errno::ACCESS
|
||||
| Errno::PERM
|
||||
| Errno::INVAL
|
||||
| Errno::NAMETOOLONG
|
||||
) {
|
||||
ProjectionError::NotFound
|
||||
} else {
|
||||
@@ -740,7 +811,7 @@ mod tests {
|
||||
};
|
||||
use wip_protocol::{FetchTreeRequest, ProtocolErrorCode, Value};
|
||||
|
||||
use super::{FailureInjection, Projection};
|
||||
use super::{FailureInjection, FlockOperation, Projection};
|
||||
use crate::{AccessMode, ResourceLimits};
|
||||
|
||||
fn projection(root: &std::path::Path, access: AccessMode) -> Projection {
|
||||
@@ -1031,4 +1102,61 @@ mod tests {
|
||||
.expect_err("cancelled traversal");
|
||||
assert_eq!(error.code, ProtocolErrorCode::ResourceLimitExceeded);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filesystem_invalid_names_are_not_published() {
|
||||
let temporary = tempdir().expect("temporary root");
|
||||
let projection = projection(temporary.path(), AccessMode::ReadOnly);
|
||||
let error = projection
|
||||
.fetch("/bad\0name")
|
||||
.expect_err("NUL cannot name a backend entry");
|
||||
assert_eq!(error.kind(), CallErrorKind::NotFound);
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn configured_root_rejects_symlink_components() {
|
||||
let temporary = tempdir().expect("temporary parent");
|
||||
let actual = temporary.path().join("actual");
|
||||
fs::create_dir(&actual).expect("create actual root");
|
||||
let linked = temporary.path().join("linked");
|
||||
std::os::unix::fs::symlink(&actual, &linked).expect("create root symlink");
|
||||
assert!(
|
||||
Projection::open(&linked, AccessMode::ReadOnly, ResourceLimits::default()).is_err()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn file_lock_wait_observes_the_operation_deadline() {
|
||||
let temporary = tempdir().expect("temporary root");
|
||||
let path = temporary.path().join("file");
|
||||
fs::write(&path, b"locked").expect("write file");
|
||||
let locked = fs::OpenOptions::new()
|
||||
.read(true)
|
||||
.write(true)
|
||||
.open(&path)
|
||||
.expect("open lock owner");
|
||||
rustix::fs::flock(&locked, FlockOperation::LockExclusive).expect("hold external lock");
|
||||
|
||||
let limits = ResourceLimits {
|
||||
request_timeout: std::time::Duration::from_millis(20),
|
||||
..ResourceLimits::default()
|
||||
};
|
||||
let projection = Projection::open(temporary.path(), AccessMode::ReadOnly, limits)
|
||||
.expect("open projection");
|
||||
let observed = projection.fetch("/file").expect("fetch file");
|
||||
let started = std::time::Instant::now();
|
||||
let error = call(
|
||||
&projection,
|
||||
"/file",
|
||||
observed.validator,
|
||||
observed.interfaces[0].clone(),
|
||||
"read",
|
||||
BTreeMap::from([("length".into(), Value::Integer(1))]),
|
||||
)
|
||||
.expect_err("lock wait must time out");
|
||||
assert_eq!(error.kind(), CallErrorKind::ResourceLimitExceeded);
|
||||
assert!(started.elapsed() < std::time::Duration::from_secs(1));
|
||||
rustix::fs::flock(&locked, FlockOperation::Unlock).expect("release lock");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -34,9 +34,9 @@ impl WipFsServer {
|
||||
/// Validates configuration and opens the configured directory capability.
|
||||
pub fn new(config: ServerConfig) -> Result<Self, ServerBuildError> {
|
||||
config.validate()?;
|
||||
let effective_root = config.effective_root()?;
|
||||
let endpoint = config.endpoint()?;
|
||||
let projection = Projection::open(&effective_root, config.access, config.limits)?;
|
||||
let projection = Projection::open(&config.root, config.access, config.limits)?;
|
||||
let effective_root = projection.effective_root()?;
|
||||
Ok(Self {
|
||||
config,
|
||||
endpoint,
|
||||
@@ -161,6 +161,7 @@ impl WipFsServer {
|
||||
Ok(metadata) => metadata,
|
||||
Err(error) => return self.codec_error(ProtocolInteraction::CallOperation, error),
|
||||
};
|
||||
let may_commit = metadata.operation == "replace";
|
||||
let decoded = std::sync::Mutex::new(None);
|
||||
let execution = self.projection.call_operation_with_context_and_decoder(
|
||||
context,
|
||||
@@ -181,14 +182,15 @@ impl WipFsServer {
|
||||
.into_inner()
|
||||
.unwrap_or_else(|error| error.into_inner())
|
||||
else {
|
||||
return self.protocol_error(
|
||||
ProtocolInteraction::CallOperation,
|
||||
&ProtocolError {
|
||||
code: ProtocolErrorCode::OperationOutcomeUnknown,
|
||||
message: "operation completed without response metadata".into(),
|
||||
let error = ProtocolError {
|
||||
code: if may_commit {
|
||||
ProtocolErrorCode::OperationOutcomeUnknown
|
||||
} else {
|
||||
ProtocolErrorCode::Internal
|
||||
},
|
||||
None,
|
||||
);
|
||||
message: "operation completed without response metadata".into(),
|
||||
};
|
||||
return self.protocol_error(ProtocolInteraction::CallOperation, &error, None);
|
||||
};
|
||||
encode_call_operation_response(
|
||||
&request,
|
||||
@@ -196,9 +198,13 @@ impl WipFsServer {
|
||||
&result,
|
||||
self.config.limits.wire,
|
||||
)
|
||||
.unwrap_or_else(|_| {
|
||||
// Dispatch succeeded. A transport/result encoding failure cannot
|
||||
// prove that a write did not commit.
|
||||
.unwrap_or_else(|encode_error| {
|
||||
if !may_commit {
|
||||
return self
|
||||
.read_encode_error(ProtocolInteraction::CallOperation, encode_error);
|
||||
}
|
||||
// Replacement may already be committed, so a response
|
||||
// encoding failure cannot carry a non-commit guarantee.
|
||||
let error = ProtocolError {
|
||||
code: ProtocolErrorCode::OperationOutcomeUnknown,
|
||||
message: "operation completed but its response could not be encoded".into(),
|
||||
|
||||
@@ -10,14 +10,14 @@ use tempfile::tempdir;
|
||||
use wip_fs_server::{AccessMode, ServerConfig, TransportSecurity, WipFsServer};
|
||||
use wip_https::{
|
||||
BatchRequest, BatchRequestItem, BatchRequestKind, BatchResponse, BatchSuccess, DecodedResponse,
|
||||
Endpoint, JSON_CONTENT_TYPE, Route, decode_batch_request, decode_batch_response,
|
||||
Endpoint, JSON_CONTENT_TYPE, Limits, Route, decode_batch_request, decode_batch_response,
|
||||
decode_call_operation_response, decode_fetch_interface_response, decode_fetch_response,
|
||||
decode_fetch_tree_response, encode_batch_request, encode_call_operation_request,
|
||||
encode_fetch_interface_request, encode_fetch_request, encode_fetch_tree_request,
|
||||
};
|
||||
use wip_protocol::{
|
||||
CallOperationRequest, FetchInterfaceRequest, FetchRequest, FetchTreeRequest, InterfaceTarget,
|
||||
Target, Value,
|
||||
ProtocolErrorCode, Target, Value,
|
||||
};
|
||||
|
||||
#[test]
|
||||
@@ -256,6 +256,83 @@ fn call_argument_decoding_does_not_mask_missing_target() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn backend_invalid_unicode_path_is_not_found_not_internal() {
|
||||
let temporary = tempdir().expect("temporary root");
|
||||
let config = ServerConfig::for_root(temporary.path());
|
||||
let limits = config.limits.wire;
|
||||
let server = WipFsServer::new(config).expect("build server");
|
||||
let endpoint = Endpoint::parse("https://localhost/wip").expect("endpoint");
|
||||
let request = FetchRequest {
|
||||
path: "/bad\0name".into(),
|
||||
};
|
||||
let response = server
|
||||
.handle_http(encode_fetch_request(&endpoint, &request, limits).expect("encode fetch"));
|
||||
assert_eq!(response.status(), http::StatusCode::NOT_FOUND);
|
||||
match decode_fetch_response(&request, &response, limits).expect("decode missing response") {
|
||||
DecodedResponse::ProtocolFailure { error, .. } => {
|
||||
assert_eq!(error.code, ProtocolErrorCode::NotFound);
|
||||
}
|
||||
DecodedResponse::Success(_) => panic!("backend-invalid path must not resolve"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_response_size_failure_preserves_non_commit_guarantee() {
|
||||
let temporary = tempdir().expect("temporary root");
|
||||
std::fs::write(temporary.path().join("large"), vec![b'x'; 2_000]).expect("write file");
|
||||
let mut config = ServerConfig::for_root(temporary.path());
|
||||
config.limits.wire = Limits::new(4_096, 1_024, 64, 64).expect("wire limits");
|
||||
let limits = config.limits.wire;
|
||||
let server = WipFsServer::new(config).expect("build server");
|
||||
let endpoint = Endpoint::parse("https://localhost/wip").expect("endpoint");
|
||||
|
||||
let fetch_request = FetchRequest {
|
||||
path: "/large".into(),
|
||||
};
|
||||
let response = server.handle_http(
|
||||
encode_fetch_request(&endpoint, &fetch_request, limits).expect("encode fetch"),
|
||||
);
|
||||
let file =
|
||||
success(decode_fetch_response(&fetch_request, &response, limits).expect("decode fetch"));
|
||||
let interface_request = FetchInterfaceRequest {
|
||||
interface: file.interfaces[0].clone(),
|
||||
};
|
||||
let response = server.handle_http(
|
||||
encode_fetch_interface_request(&endpoint, &interface_request, limits)
|
||||
.expect("encode interface fetch"),
|
||||
);
|
||||
let interface = success(
|
||||
decode_fetch_interface_response(&interface_request, &response, limits)
|
||||
.expect("decode interface"),
|
||||
);
|
||||
let call = CallOperationRequest {
|
||||
target: Target {
|
||||
path: "/large".into(),
|
||||
validator: file.validator,
|
||||
},
|
||||
interface: InterfaceTarget {
|
||||
reference: file.interfaces[0].clone(),
|
||||
validator: None,
|
||||
},
|
||||
operation: "read".into(),
|
||||
arguments: BTreeMap::from([("length".into(), Value::Integer(1_500))]),
|
||||
};
|
||||
let response = server.handle_http(
|
||||
encode_call_operation_request(&endpoint, &call, &interface.descriptor, limits)
|
||||
.expect("encode read call"),
|
||||
);
|
||||
assert_eq!(response.status(), http::StatusCode::PAYLOAD_TOO_LARGE);
|
||||
match decode_call_operation_response(&call, &interface.descriptor, &response, limits)
|
||||
.expect("decode resource failure")
|
||||
{
|
||||
DecodedResponse::ProtocolFailure { error, .. } => {
|
||||
assert_eq!(error.code, ProtocolErrorCode::ResourceLimitExceeded);
|
||||
}
|
||||
DecodedResponse::Success(_) => panic!("oversized read response must fail"),
|
||||
}
|
||||
}
|
||||
|
||||
fn assert_batch_successes(batch: &BatchResponse) {
|
||||
assert_eq!(batch.responses.len(), 3);
|
||||
assert!(matches!(
|
||||
|
||||
Reference in New Issue
Block a user