diff --git a/CHANGELOG.md b/CHANGELOG.md index d03caca8..a33e9a4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,10 @@ breaking entries are marked **BREAKING**. so `git worktree list` is now declarable. Flags, positionals, tail, and stdin stay leaf-only, refused at `build()` on a node. See `docs/wrapped_command.md` for the full grammar. +- **`Filesystem::canonicalize` and `KernelBackend::canonicalize`** — a + defaulted, containment-checked path canonicalizer. `LocalFs` and + `VfsRouter` override it with `resolve_beneath`; `readlink -f` and + `realpath` are now thin callers instead of walking symlinks themselves. ### Fixed - **A mount point's ancestors are navigable again** — with a backend at `/` and @@ -43,6 +47,10 @@ breaking entries are marked **BREAKING**. aliases, declared effects, and command-level aliases** — each rendered on one side and silently dropped on the other. Both now render every field from one shared implementation. +- **`readlink -f`/`realpath` no longer leak a path outside a rooted mount** — + they walked symlinks by re-routing every hop through the mount table, so a + symlink escaping its own mount resolved against whatever else was mounted, + including `/`, instead of being refused. ## [0.17.0] - 2026-08-31 diff --git a/crates/kaish-kernel/src/backend/local.rs b/crates/kaish-kernel/src/backend/local.rs index 9b769940..564211eb 100644 --- a/crates/kaish-kernel/src/backend/local.rs +++ b/crates/kaish-kernel/src/backend/local.rs @@ -336,6 +336,17 @@ impl KernelBackend for LocalBackend { Ok(()) } + /// Delegates to the router, which delegates to the mount that owns the + /// path. The trait default would walk component by component through + /// this backend's own `lstat`/`read_link` — each of those a router + /// lookup plus, on a rooted `LocalFs` mount, a full `resolve_beneath` + /// from its root — turning one canonicalize into an O(n²) walk for an + /// n-component path. Routing straight to `VfsRouter::canonicalize` + /// keeps it to one resolve per mount crossed. + async fn canonicalize(&self, path: &Path, allow_missing_final: bool) -> BackendResult { + Ok(self.vfs.canonicalize(path, allow_missing_final).await?) + } + // ═══════════════════════════════════════════════════════════════════════════ // Tool Dispatch // ═══════════════════════════════════════════════════════════════════════════ diff --git a/crates/kaish-kernel/src/backend/overlay.rs b/crates/kaish-kernel/src/backend/overlay.rs index 8de3aba6..42be606c 100644 --- a/crates/kaish-kernel/src/backend/overlay.rs +++ b/crates/kaish-kernel/src/backend/overlay.rs @@ -388,6 +388,30 @@ impl KernelBackend for VirtualOverlayBackend { } } + /// The same three-way split every other operation here uses: a + /// kaish-owned virtual path canonicalizes through `self.vfs`; a shared + /// ancestor (`/v`, `/`, …) is a directory this backend synthesizes, + /// never a symlink, so it canonicalizes to itself; everything else + /// canonicalizes through the embedder's own backend. + /// + /// Delegating rather than inheriting the trait default matters here + /// specifically: the default's per-hop walk calls `lstat`/`read_link` on + /// this type for every component, re-running `is_virtual_path` / + /// `is_shared_ancestor` at each hop instead of asking the owning side + /// once for the whole path. A symlink that lives entirely under the + /// embedder's backend must resolve — and be contained — through that one + /// backend's own resolver, not be re-routed through this split hop by + /// hop. + async fn canonicalize(&self, path: &Path, allow_missing_final: bool) -> BackendResult { + if self.is_virtual_path(path) { + Ok(self.vfs.canonicalize(path, allow_missing_final).await?) + } else if self.is_shared_ancestor(path) { + Ok(path.to_path_buf()) + } else { + self.inner.canonicalize(path, allow_missing_final).await + } + } + // ═══════════════════════════════════════════════════════════════════════════ // Tool Dispatch // ═══════════════════════════════════════════════════════════════════════════ diff --git a/crates/kaish-kernel/src/tools/builtin/readlink.rs b/crates/kaish-kernel/src/tools/builtin/readlink.rs index 762b71f8..dcf813ec 100644 --- a/crates/kaish-kernel/src/tools/builtin/readlink.rs +++ b/crates/kaish-kernel/src/tools/builtin/readlink.rs @@ -5,14 +5,11 @@ use async_trait::async_trait; use clap::{CommandFactory, Parser}; -use std::path::{Path, PathBuf}; +use std::path::Path; use crate::interpreter::{ExecResult, OutputData}; use crate::tools::{schema_from_clap, ExecContext, ToolCtx, GlobalFlags, Tool, ToolArgs, ToolSchema}; -/// Maximum symlink hops to prevent infinite loops (matches Linux MAXSYMLINKS). -const MAX_SYMLINK_HOPS: usize = 40; - /// Readlink tool: read symlink target or canonicalize a path. pub struct Readlink; @@ -82,15 +79,16 @@ impl Tool for Readlink { let resolved = ctx.resolve_path(&path_str); if canonicalize { - // GNU readlink -f: canonicalize through symlinks. - // Missing final component is allowed (resolves parents only). - match canonicalize_path_allow_missing_final(ctx, &resolved).await { + // GNU readlink -f: canonicalize through symlinks. A missing + // final component is allowed (the backend resolves parents + // only); a missing intermediate component still errors. + match ctx.backend.canonicalize(Path::new(&resolved), true).await { Ok(canonical) => { output.push_str(&canonical.to_string_lossy()); output.push('\n'); } - Err(msg) => { - last_err = Some(format!("readlink: {}: {}", path_str, msg)); + Err(e) => { + last_err = Some(format!("readlink: {}: {}", path_str, e)); exit_code = 1; } } @@ -145,157 +143,6 @@ impl Tool for Readlink { } } -/// Canonicalize a path through the VFS backend, following symlinks at every -/// component. Missing final component is allowed (GNU `readlink -f` semantics): -/// if the last component doesn't exist but its parent does, return the -/// normalized parent + final component. -/// -/// Returns an error if any intermediate (non-final) component is missing or -/// if symlink resolution loops. -pub async fn canonicalize_path_allow_missing_final( - ctx: &ExecContext, - path: &Path, -) -> Result { - let components: Vec<_> = path.components().collect(); - let total = components.len(); - - if total == 0 { - return Err("empty path".to_string()); - } - - let mut current = PathBuf::new(); - - for (idx, component) in components.iter().enumerate() { - let is_last = idx + 1 == total; - - match component { - std::path::Component::RootDir => { - current.push("/"); - } - std::path::Component::CurDir => {} - std::path::Component::ParentDir => { - current.pop(); - } - std::path::Component::Normal(_) => { - current.push(component); - // Resolve symlinks at this component. - current = resolve_symlink_component(ctx, current, is_last).await?; - } - std::path::Component::Prefix(_) => { - current.push(component); - } - } - } - - Ok(current) -} - -/// Resolve potential symlinks at `path`, following the chain up to -/// `MAX_SYMLINK_HOPS`. If `allow_missing` is true and the path doesn't exist, -/// return `path` unchanged (the caller already knows it's the final component). -async fn resolve_symlink_component( - ctx: &ExecContext, - path: PathBuf, - allow_missing: bool, -) -> Result { - let mut current = path; - - for _ in 0..MAX_SYMLINK_HOPS { - match ctx.backend.lstat(Path::new(¤t)).await { - Ok(entry) if entry.is_symlink() => { - let target = ctx - .backend - .read_link(Path::new(¤t)) - .await - .map_err(|e| e.to_string())?; - - if target.is_absolute() { - current = target; - } else { - // Relative target: resolve from the link's parent directory. - let parent = current.parent().unwrap_or(Path::new("/")); - current = parent.join(target); - } - // Normalize out any . and .. introduced by the target. - current = normalize_path_buf(current); - } - Ok(_) => { - // Not a symlink — resolved. - return Ok(current); - } - Err(e) => { - use crate::backend::BackendError; - match &e { - BackendError::NotFound(_) if allow_missing => { - // Final component missing — allowed per GNU readlink -f. - return Ok(current); - } - BackendError::NotFound(_) => { - return Err(format!( - "No such file or directory: {}", - current.display() - )); - } - _ => return Err(e.to_string()), - } - } - } - } - - Err(format!( - "too many levels of symbolic links: {}", - current.display() - )) -} - -/// Normalize a PathBuf by collapsing `.` and `..` components without -/// filesystem access. This handles targets injected by symlink resolution. -fn normalize_path_buf(path: PathBuf) -> PathBuf { - let mut components: Vec = Vec::new(); - let is_absolute = path.is_absolute(); - - for component in path.components() { - match component { - std::path::Component::RootDir => {} - std::path::Component::CurDir => {} - std::path::Component::ParentDir => { - // A `..` cancels a preceding *normal* component, but on a - // relative path it must ACCUMULATE past the start (and past - // other leading `..`s): `../../a` normalizes to `../../a`, not - // `a`. Only pop when the last component is a real name. - let last_is_parent = components - .last() - .map(|s| s.as_os_str() == std::ffi::OsStr::new("..")) - .unwrap_or(false); - if components.is_empty() || last_is_parent { - if !is_absolute { - components.push("..".into()); - } - // Absolute: a leading `..` at root is a no-op (stays at /). - } else { - components.pop(); - } - } - std::path::Component::Normal(s) => { - components.push(s.to_os_string()); - } - std::path::Component::Prefix(_) => {} - } - } - - if is_absolute { - let mut result = PathBuf::from("/"); - for c in components { - result.push(c); - } - result - } else if components.is_empty() { - PathBuf::from(".") - } else { - components.iter().collect() - } -} - #[cfg(test)] mod tests { use super::*; @@ -408,44 +255,4 @@ mod tests { assert!(!result.ok()); assert!(result.err.contains("No such file"), "got: {}", result.err); } - - #[test] - fn test_normalize_path_buf_absolute() { - assert_eq!( - normalize_path_buf(PathBuf::from("/usr/bin/../lib")), - PathBuf::from("/usr/lib") - ); - assert_eq!( - normalize_path_buf(PathBuf::from("/usr/./bin")), - PathBuf::from("/usr/bin") - ); - } - - #[test] - fn test_normalize_path_buf_relative() { - assert_eq!( - normalize_path_buf(PathBuf::from("a/b/../c")), - PathBuf::from("a/c") - ); - } - - #[test] - fn test_normalize_path_buf_relative_leading_parents_accumulate() { - // Regression (Gemini review): leading/consecutive `..` on a relative - // path must accumulate, not cancel each other — `../../a` is `../../a`, - // not `a`. A trailing `..` past the start likewise accumulates. - assert_eq!( - normalize_path_buf(PathBuf::from("../../a")), - PathBuf::from("../../a") - ); - assert_eq!( - normalize_path_buf(PathBuf::from("../a/../..")), - PathBuf::from("../..") - ); - // A `..` still cancels a preceding real component. - assert_eq!( - normalize_path_buf(PathBuf::from("../a/b/..")), - PathBuf::from("../a") - ); - } } diff --git a/crates/kaish-kernel/src/tools/builtin/realpath.rs b/crates/kaish-kernel/src/tools/builtin/realpath.rs index 87f7c906..ee03dad7 100644 --- a/crates/kaish-kernel/src/tools/builtin/realpath.rs +++ b/crates/kaish-kernel/src/tools/builtin/realpath.rs @@ -75,13 +75,15 @@ impl Tool for Realpath { }; let resolved = ctx.resolve_path(&path_str); - match canonicalize_path_full(ctx, &resolved).await { + // GNU realpath (no -m): every component, including the final + // one, must exist — `allow_missing_final: false`. + match ctx.backend.canonicalize(std::path::Path::new(&resolved), false).await { Ok(canonical) => { output.push_str(&canonical.to_string_lossy()); output.push('\n'); } - Err(msg) => { - last_err = Some(format!("realpath: {}: {}", path_str, msg)); + Err(e) => { + last_err = Some(format!("realpath: {}: {}", path_str, e)); exit_code = 1; } } @@ -96,33 +98,6 @@ impl Tool for Realpath { } } -/// Canonicalize a path through the VFS, requiring all components including the -/// final one to exist. Uses the shared symlink-following logic from `readlink`. -async fn canonicalize_path_full( - ctx: &ExecContext, - path: &std::path::Path, -) -> Result { - use super::readlink::canonicalize_path_allow_missing_final; - use std::path::Path; - - let canonical = canonicalize_path_allow_missing_final(ctx, path).await?; - - // For realpath (no -m), the final resolved path must exist. - match ctx.backend.stat(Path::new(&canonical)).await { - Ok(_) => Ok(canonical), - Err(e) => { - use crate::backend::BackendError; - Err(match &e { - BackendError::NotFound(_) => format!( - "No such file or directory: {}", - canonical.display() - ), - _ => e.to_string(), - }) - } - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/kaish-kernel/src/vfs/router.rs b/crates/kaish-kernel/src/vfs/router.rs index b792c2da..82780538 100644 --- a/crates/kaish-kernel/src/vfs/router.rs +++ b/crates/kaish-kernel/src/vfs/router.rs @@ -406,6 +406,39 @@ impl Filesystem for VfsRouter { fs.read_link(&relative).await } + /// Delegates to the mount that owns `path`, translating VFS-absolute to + /// mount-relative going in and back going out — the mount answers in + /// its own namespace, same as every other `Filesystem` method here. + /// + /// `.` and `..` are folded lexically before routing, so a `..` that + /// walks from one mount into another (or into a synthesized ancestor) + /// resolves against the right one, the way `symlink`'s absolute-target + /// rewrite already folds before it picks a mount. + /// + /// Falls back the way `stat` does: a synthesized ancestor of a mount + /// (`/v` above `/v/jobs`) is a directory the router creates, never a + /// symlink, so it canonicalizes to itself. + async fn canonicalize(&self, path: &Path, allow_missing_final: bool) -> io::Result { + let normalized = lexical_absolute(path); + if normalized == Path::new("/") { + return Ok(PathBuf::from("/")); + } + + let answer = match self.mount_of(&normalized) { + Ok((mount_path, fs, relative)) => { + let mount_path = mount_path.to_path_buf(); + fs.canonicalize(&relative, allow_missing_final) + .await + .map(|resolved| mount_path.join(resolved)) + } + Err(e) => Err(e), + }; + match answer { + Ok(resolved) => Ok(resolved), + Err(e) => self.or_synthesized_ancestor(&normalized, e, || normalized.clone()), + } + } + async fn symlink(&self, target: &Path, link: &Path) -> io::Result<()> { let (link_mount, fs, relative_link) = self.mount_of(link)?; // A backend refuses an absolute target: it has no namespace to read diff --git a/crates/kaish-kernel/tests/readlink_rooted_mount_tests.rs b/crates/kaish-kernel/tests/readlink_rooted_mount_tests.rs new file mode 100644 index 00000000..f59f79a7 --- /dev/null +++ b/crates/kaish-kernel/tests/readlink_rooted_mount_tests.rs @@ -0,0 +1,218 @@ +//! `readlink -f` / `realpath` on a LocalFs mount rooted below `/`. +//! +//! Reproduces a bug reported against v0.17.0 by the kaibo project: a kernel +//! shape with `LocalFs` mounted read-only at a deep VFS path (mirroring its +//! own host directory, several path components below `/`) and `MemoryFs` at +//! `/` — the common embedder pattern (`kaijutsu`, `kaibo`). `readlink -f` +//! failed on every operand, reporting the FIRST PATH COMPONENT of the mount +//! root as "No such file or directory", because `canonicalize_path_allow_missing_final` +//! walks every component of the VFS path through `lstat`, including the +//! components ABOVE the mount point that no single backend owns. +//! +//! Bare `readlink` (no `-f`) was unaffected — it does one `lstat` on the +//! full, already-mount-relative path, never walking ancestors. + +// Test-fixture code: unwrap/expect on known-good setup is the idiom here. +#![allow(clippy::unwrap_used, clippy::expect_used)] +// Symlinks are unix-only; real FS via localfs feature. +#![cfg(all(feature = "localfs", unix))] + +use std::os::unix::fs::symlink; +use std::path::Path; +use std::sync::Arc; + +use kaish_kernel::vfs::{LocalFs, MemoryFs, VfsRouter}; +use kaish_kernel::{Kernel, KernelBackend, KernelConfig, LocalBackend}; + +fn tempdir() -> tempfile::TempDir { + // Several path components deep under CARGO_TARGET_TMPDIR (itself deep), + // so the mount root is not adjacent to `/` — the shape that reproduces + // the bug. `fixture_root` joins on two more components below this. + tempfile::Builder::new() + .prefix("readlink-rooted-") + .tempdir_in(env!("CARGO_TARGET_TMPDIR")) + .expect("tempdir under CARGO_TARGET_TMPDIR") +} + +/// The mount's VFS path AND host root: several components below `/`, and, +/// per the report, the mount path mirrors the host path (the same string +/// used as both the VFS prefix and the real directory) — the common +/// embedder pattern of projecting a project's own absolute host path +/// straight into the VFS namespace. +fn fixture_root(base: &tempfile::TempDir) -> std::path::PathBuf { + let root = base.path().join("project").join("fixture"); + std::fs::create_dir_all(&root).expect("mkdir project/fixture"); + root +} + +/// LocalFs read-only at `fixture_root`, mounted at that SAME path in VFS +/// space; MemoryFs at `/` — the kaibo-reported shape. +fn rooted_kernel(root: &Path) -> Kernel { + let mut vfs = VfsRouter::new(); + vfs.mount(root.to_path_buf(), LocalFs::read_only(root.to_path_buf())); + vfs.mount("/", MemoryFs::new()); + let backend: Arc = Arc::new(LocalBackend::new(Arc::new(vfs))); + let config = KernelConfig::isolated().with_cwd(root.to_path_buf()); + Kernel::with_backend(backend, config, |_| {}, |_| {}).expect("with_backend kernel") +} + +/// Control: the same fixture layout with LocalFs mounted at VFS root `/` +/// (the ordinary, unrooted shape existing tests already cover). Must keep +/// passing — proves the fix does not regress the common case. +fn unrooted_kernel(root: &Path) -> Kernel { + let config = KernelConfig::repl() + .with_cwd(root.to_path_buf()) + .with_trash(false); + Kernel::new(config).expect("kernel") +} + +async fn run(kernel: &Kernel, script: &str) -> (String, String, i64) { + let r = kernel.execute(script).await.expect("kernel execute"); + (r.text_out().trim().to_string(), r.err.clone(), r.code) +} + +fn seed(root: &Path) { + std::fs::create_dir_all(root.join("d")).unwrap(); + std::fs::write(root.join("d/a.txt"), "content").unwrap(); + std::fs::write(root.join("top.txt"), "top-level").unwrap(); + symlink("d/a.txt", root.join("link.txt")).unwrap(); + symlink("nosuchtarget", root.join("dangling")).unwrap(); +} + +// --------------------------------------------------------------------------- +// Rooted mount: all five reported cases +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn rooted_bare_readlink_on_symlink_works() { + let base = tempdir(); + let root = fixture_root(&base); + seed(&root); + let k = rooted_kernel(&root); + + let (out, err, code) = run(&k, "readlink link.txt").await; + assert_eq!(code, 0, "bare readlink should succeed: err={err}"); + assert_eq!(out, "d/a.txt"); +} + +#[tokio::test] +async fn rooted_readlink_f_on_symlink_resolves() { + let base = tempdir(); + let root = fixture_root(&base); + seed(&root); + let k = rooted_kernel(&root); + + let (out, err, code) = run(&k, "readlink -f link.txt").await; + assert_eq!(code, 0, "readlink -f on a symlink should succeed: err={err}"); + let expected = root.join("d/a.txt").to_string_lossy().into_owned(); + assert_eq!(out, expected); +} + +#[tokio::test] +async fn rooted_readlink_f_on_regular_file_resolves() { + let base = tempdir(); + let root = fixture_root(&base); + seed(&root); + let k = rooted_kernel(&root); + + let (out, err, code) = run(&k, "readlink -f top.txt").await; + assert_eq!(code, 0, "readlink -f on a plain regular file should succeed: err={err}"); + let expected = root.join("top.txt").to_string_lossy().into_owned(); + assert_eq!(out, expected); +} + +#[tokio::test] +async fn rooted_readlink_f_on_dangling_link_resolves() { + let base = tempdir(); + let root = fixture_root(&base); + seed(&root); + let k = rooted_kernel(&root); + + let (out, err, code) = run(&k, "readlink -f dangling").await; + assert_eq!(code, 0, "readlink -f on a dangling link should succeed (GNU allows a missing final target): err={err}"); + let expected = root.join("nosuchtarget").to_string_lossy().into_owned(); + assert_eq!(out, expected); +} + +#[tokio::test] +async fn rooted_readlink_f_on_missing_file_resolves() { + let base = tempdir(); + let root = fixture_root(&base); + seed(&root); + let k = rooted_kernel(&root); + + let (out, err, code) = run(&k, "readlink -f nosuchfile").await; + assert_eq!(code, 0, "readlink -f on a missing final component should succeed: err={err}"); + let expected = root.join("nosuchfile").to_string_lossy().into_owned(); + assert_eq!(out, expected); +} + +#[tokio::test] +async fn rooted_readlink_f_link_escaping_root_is_refused() { + // A symlink inside the root whose target is an absolute host path + // outside the mount's own root. Containment must still be refused — + // fixing the ancestor-walk bug must not open this hole. + let base = tempdir(); + let root = fixture_root(&base); + seed(&root); + let outside = tempfile::tempdir().expect("outside tempdir"); + std::fs::write(outside.path().join("secret.txt"), "s").unwrap(); + // The link is NOT named for what it does. The builtin formats a failure as + // `readlink: : `, so an operand containing "escape" makes + // every failure — including the ancestor-walk bug this fix removes — satisfy + // an assertion looking for that word. The name must not be able to pass the + // test on the operand's behalf. + symlink(outside.path().join("secret.txt"), root.join("outward.txt")).unwrap(); + + let k = rooted_kernel(&root); + let (out, err, code) = run(&k, "readlink -f outward.txt").await; + assert_ne!( + code, 0, + "readlink -f through a link escaping the mount root must be refused, got out={out:?}" + ); + assert!( + err.contains("path escapes root"), + "containment must be what refused it, got: {err}" + ); + // The ancestor-walk bug refused everything with this message. If it is back, + // the refusal above is the old bug wearing the new test's clothes. + assert!( + !err.contains("No such file or directory"), + "refused for the wrong reason — this is the ancestor-walk bug, not containment: {err}" + ); + // The target must not leak, whatever the reason for refusal. + assert!( + !out.contains("secret.txt"), + "an escaping target must never be printed, got out={out:?}" + ); +} + +#[tokio::test] +async fn rooted_realpath_on_symlink_resolves() { + let base = tempdir(); + let root = fixture_root(&base); + seed(&root); + let k = rooted_kernel(&root); + + let (out, err, code) = run(&k, "realpath link.txt").await; + assert_eq!(code, 0, "realpath on a symlink should succeed: err={err}"); + let expected = root.join("d/a.txt").to_string_lossy().into_owned(); + assert_eq!(out, expected); +} + +// --------------------------------------------------------------------------- +// Control: unrooted (LocalFs at VFS `/`) must keep working +// --------------------------------------------------------------------------- + +#[tokio::test] +async fn unrooted_readlink_f_on_symlink_still_resolves() { + let base = tempdir(); + let root = fixture_root(&base); + seed(&root); + let k = unrooted_kernel(&root); + + let (out, err, code) = run(&k, "readlink -f link.txt").await; + assert_eq!(code, 0, "control (unrooted) readlink -f must still pass: err={err}"); + let expected = root.join("d/a.txt").to_string_lossy().into_owned(); + assert_eq!(out, expected); +} diff --git a/crates/kaish-kernel/tests/router_canonicalize_containment_tests.rs b/crates/kaish-kernel/tests/router_canonicalize_containment_tests.rs new file mode 100644 index 00000000..6d7334fb --- /dev/null +++ b/crates/kaish-kernel/tests/router_canonicalize_containment_tests.rs @@ -0,0 +1,103 @@ +//! A symlink inside a rooted mount must not canonicalize past that mount's +//! root, even when another mount (or the mount covering `/`) happens to +//! have something at the escaped path. +//! +//! `readlink -f`/`realpath` used to walk a path hop by hop through +//! `ctx.backend` (the router), recomputing the owning mount via +//! `find_mount` on every hop. A symlink target that walked far enough above +//! its own mount's root via `..` got re-routed through the mount table from +//! scratch instead of being refused — the escape resolved against whatever +//! mount happened to cover the folded VFS-absolute path, not the mount the +//! symlink actually lives on. `VfsRouter::canonicalize` picks the owning +//! mount once and hands the whole walk to that mount's own +//! containment-checked resolver, so the escape is refused before it ever +//! reaches the mount table again. + +// Test-fixture code: unwrap/expect on known-good setup is the idiom here. +#![allow(clippy::unwrap_used, clippy::expect_used)] +#![cfg(all(feature = "localfs", unix))] + +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +use kaish_kernel::vfs::{LocalFs, MemoryFs, VfsRouter}; +use kaish_kernel::{Kernel, KernelBackend, KernelConfig, LocalBackend}; + +fn tempdir() -> tempfile::TempDir { + tempfile::Builder::new() + .prefix("router-canonicalize-containment-") + .tempdir_in(env!("CARGO_TARGET_TMPDIR")) + .expect("tempdir under CARGO_TARGET_TMPDIR") +} + +/// A rooted `LocalFs` mount several components below `/`, mirroring its own +/// host path (matching `router_mount_ancestor_tests.rs`'s fixture shape) — +/// plus a `/` mount with a "secret" file at the path the escape would land +/// on if the router ever re-routed a symlink hop through the mount table. +async fn kernel_with_escape_target(base: &tempfile::TempDir) -> (Kernel, PathBuf) { + use kaish_kernel::vfs::Filesystem; + + let root = base.path().join("project").join("fixture"); + std::fs::create_dir_all(&root).expect("mkdir project/fixture"); + + // 20 levels of `..` clamps to `/` lexically however deep `root` is — + // no need to count `root`'s own depth exactly. + let escape_target: PathBuf = std::iter::repeat_n("..", 20).collect::().join("secret"); + std::os::unix::fs::symlink(&escape_target, root.join("escape")).expect("symlink escape"); + + let mut vfs = VfsRouter::new(); + vfs.mount(root.to_path_buf(), LocalFs::new(root.to_path_buf())); + let outside = MemoryFs::new(); + // The "secret" a mount-table re-route would leak: readlink -f must + // never print this path for a symlink that lives inside the rooted + // mount above. + outside + .write(Path::new("secret"), b"leaked") + .await + .expect("write /secret in the root mount"); + vfs.mount("/", outside); + + let backend: Arc = Arc::new(LocalBackend::new(Arc::new(vfs))); + let config = KernelConfig::isolated().with_cwd(root.to_path_buf()); + let kernel = Kernel::with_backend(backend, config, |_| {}, |_| {}).expect("with_backend kernel"); + (kernel, root) +} + +async fn run(kernel: &Kernel, line: &str) -> (String, String, i64) { + let result = kernel.execute(line).await.expect("execute"); + ( + result.text_out().trim_end().to_string(), + result.err.trim_end().to_string(), + result.code, + ) +} + +#[tokio::test] +async fn readlink_f_refuses_a_symlink_that_escapes_its_mount_root() { + let base = tempdir(); + let (kernel, root) = kernel_with_escape_target(&base).await; + + let (out, err, code) = run(&kernel, &format!("readlink -f {}/escape", root.display())).await; + assert_ne!(code, 0, "readlink -f must refuse an escaping symlink, got: {out:?}"); + assert!( + !out.contains("secret"), + "readlink -f must never print the path a mount-table re-route would leak: {out:?}" + ); + assert!( + err.contains("escapes root") || err.contains("No such file"), + "expected a containment or not-found refusal, got: {err}" + ); +} + +#[tokio::test] +async fn realpath_refuses_a_symlink_that_escapes_its_mount_root() { + let base = tempdir(); + let (kernel, root) = kernel_with_escape_target(&base).await; + + let (out, _err, code) = run(&kernel, &format!("realpath {}/escape", root.display())).await; + assert_ne!(code, 0, "realpath must refuse an escaping symlink, got: {out:?}"); + assert!( + !out.contains("secret"), + "realpath must never print the path a mount-table re-route would leak: {out:?}" + ); +} diff --git a/crates/kaish-tool-api/src/backend.rs b/crates/kaish-tool-api/src/backend.rs index fc0b6988..0c4be886 100644 --- a/crates/kaish-tool-api/src/backend.rs +++ b/crates/kaish-tool-api/src/backend.rs @@ -10,7 +10,7 @@ use std::path::{Path, PathBuf}; use async_trait::async_trait; use kaish_types::backend::{ - BackendResult, MountInfo, PatchOp, ReadRange, ToolInfo, ToolResult, WriteMode, + BackendError, BackendResult, MountInfo, PatchOp, ReadRange, ToolInfo, ToolResult, WriteMode, }; use kaish_types::{DirEntry, PathAccess, ToolArgs}; @@ -99,6 +99,44 @@ pub trait KernelBackend: Send + Sync { /// mount, and refused when they are not. async fn symlink(&self, target: &Path, link: &Path) -> BackendResult<()>; + /// Resolve `path` to its canonical form: follow every symlink hop, fold + /// `.` and `..` lexically. The final component may be missing when + /// `allow_missing_final` is true (GNU `readlink -f` semantics); a + /// missing INTERMEDIATE component is always an error. Symlink hops are + /// capped at 40, matching Linux `MAXSYMLINKS`; exceeding the cap is an + /// error, never a silent stop. + /// + /// The default walks component by component through + /// [`KernelBackend::lstat`] and [`KernelBackend::read_link`], so it + /// inherits whatever containment those already give. `LocalBackend` + /// overrides this to delegate straight to the VFS layer's single-shot + /// resolver instead of one round trip per hop. + async fn canonicalize(&self, path: &Path, allow_missing_final: bool) -> BackendResult { + let components: Vec<_> = path.components().collect(); + let total = components.len(); + let mut current = PathBuf::new(); + + for (idx, component) in components.iter().enumerate() { + let is_last = idx + 1 == total; + match component { + std::path::Component::RootDir => {} + std::path::Component::CurDir => {} + std::path::Component::ParentDir => { + current.pop(); + } + std::path::Component::Normal(_) => { + current.push(component); + current = + resolve_symlink_hop(self, current, is_last && allow_missing_final).await?; + } + std::path::Component::Prefix(_) => { + current.push(component); + } + } + } + Ok(current) + } + // ═══════════════════════════════════════════════════════════════════════ // Tool Dispatch // ═══════════════════════════════════════════════════════════════════════ @@ -165,3 +203,56 @@ pub trait KernelBackend: Send + Sync { /// `git` that hand paths to external C libraries need the real path. fn resolve_real_path(&self, path: &Path) -> Option; } + +/// Symlink hops [`KernelBackend::canonicalize`]'s default walk follows +/// before refusing, matching Linux's `MAXSYMLINKS`. +const MAX_SYMLINK_HOPS: usize = 40; + +/// Follow the symlink chain starting at `path`, if any, to the entry it +/// names. `allow_missing` permits `path` itself to be absent; every hop +/// short of it must exist. +async fn resolve_symlink_hop( + backend: &B, + path: PathBuf, + allow_missing: bool, +) -> BackendResult { + let mut current = path; + for _ in 0..MAX_SYMLINK_HOPS { + match backend.lstat(¤t).await { + Ok(entry) if entry.is_symlink() => { + let target = backend.read_link(¤t).await?; + current = if target.is_absolute() { + target + } else { + let parent = current.parent().unwrap_or(Path::new("")); + parent.join(target) + }; + current = fold_dots(current); + } + Ok(_) => return Ok(current), + Err(BackendError::NotFound(_)) if allow_missing => return Ok(current), + Err(e) => return Err(e), + } + } + Err(BackendError::InvalidOperation(format!( + "too many levels of symbolic links: {}", + current.display() + ))) +} + +/// Collapse `.` and `..` lexically in a path: `..` past the start is +/// dropped, not accumulated, matching the VFS layer's own clamp-at-root +/// rule for a root-relative path. +fn fold_dots(path: PathBuf) -> PathBuf { + let mut out = PathBuf::new(); + for component in path.components() { + match component { + std::path::Component::ParentDir => { + out.pop(); + } + std::path::Component::CurDir => {} + other => out.push(other), + } + } + out +} diff --git a/crates/kaish-vfs/src/conformance.rs b/crates/kaish-vfs/src/conformance.rs index fc46cba2..aa43943b 100644 --- a/crates/kaish-vfs/src/conformance.rs +++ b/crates/kaish-vfs/src/conformance.rs @@ -689,6 +689,87 @@ pub async fn remove_refuses_the_root(fs: &dyn Filesystem) -> Result<(), String> Ok(()) } +/// `canonicalize` on a symlink whose target walks far enough above the root +/// via `..` must never leak an out-of-namespace answer: a rooted backend +/// (`LocalFs`) refuses it, and a backend with no boundary to enforce +/// (`MemoryFs`, `OverlayFs`) has nowhere to escape to, so it must answer +/// with an ordinary in-namespace path — never one still carrying a `..` +/// above this filesystem's own root. +pub async fn canonicalize_of_an_escaping_symlink_stays_in_bounds( + fs: &dyn Filesystem, +) -> Result<(), String> { + fs.symlink( + Path::new("../../../../../../../../../../outside"), + Path::new("escape"), + ) + .await + .map_err(|e| format!("symlink: {e}"))?; + + match fs.canonicalize(Path::new("escape"), true).await { + // Refused: the correct answer for a rooted backend, and no answer + // can leak. `Unsupported` is not that — it is a backend that never + // implemented the method, which would pass this case without ever + // deciding anything. + Err(error) if error.kind() != std::io::ErrorKind::Unsupported => Ok(()), + Err(error) => Err(format!( + "canonicalize(escape) is unimplemented, so this case proved nothing: {error}" + )), + Ok(resolved) => { + if resolved + .components() + .any(|c| matches!(c, std::path::Component::ParentDir)) + { + Err(format!( + "canonicalize(escape) returned a path that still walks \ + above the root: {}", + resolved.display() + )) + } else { + Ok(()) + } + } + } +} + +/// A missing FINAL path component is allowed only when the caller asks for +/// it; a missing INTERMEDIATE component is always an error, whichever way +/// that flag is set. +pub async fn canonicalize_allows_a_missing_final_component_only( + fs: &dyn Filesystem, +) -> Result<(), String> { + fs.mkdir(Path::new("d")).await.map_err(|e| format!("mkdir d: {e}"))?; + + let resolved = fs + .canonicalize(Path::new("d/missing"), true) + .await + .map_err(|e| format!("canonicalize(d/missing, allow_missing_final=true): {e}"))?; + if resolved != Path::new("d/missing") { + return Err(format!( + "expected canonicalize(d/missing) == d/missing, got {}", + resolved.display() + )); + } + + if fs.canonicalize(Path::new("d/missing"), false).await.is_ok() { + return Err( + "expected canonicalize(d/missing, allow_missing_final=false) to error".to_string(), + ); + } + + if fs + .canonicalize(Path::new("d/missing/deeper"), true) + .await + .is_ok() + { + return Err( + "expected a missing intermediate component to error even with \ + allow_missing_final=true" + .to_string(), + ); + } + Ok(()) +} + pub async fn rename_refuses_the_root(fs: &dyn Filesystem) -> Result<(), String> { fs.write(Path::new("keep"), b"K") .await @@ -742,6 +823,8 @@ pub const CASES: &[(&str, Case)] = &[ case!(rename_to_itself_spelled_with_dotdot_keeps_the_file), case!(remove_refuses_the_root), case!(rename_refuses_the_root), + case!(canonicalize_of_an_escaping_symlink_stays_in_bounds), + case!(canonicalize_allows_a_missing_final_component_only), ]; /// Runs every case, each against its own fresh root from `make_root`. diff --git a/crates/kaish-vfs/src/local.rs b/crates/kaish-vfs/src/local.rs index 75d8959e..970e82eb 100644 --- a/crates/kaish-vfs/src/local.rs +++ b/crates/kaish-vfs/src/local.rs @@ -392,6 +392,49 @@ impl Filesystem for LocalFs { fs::read_link(&full_path).await } + /// A single [`resolve_beneath`] call, containment-checked there, instead + /// of the trait default's one round trip per path component. + /// + /// `resolve_beneath`'s `Follow::Final` alone does not distinguish a + /// missing FINAL component from a missing INTERMEDIATE one — it resolves + /// to the deepest existing ancestor and appends whatever is missing, + /// however many components that is. This checks the resolved answer's + /// parent separately: when the answer itself is missing, an existing + /// parent means only the final component was absent (honor + /// `allow_missing_final`); a missing parent means something deeper was + /// missing too, which is always an error. + async fn canonicalize(&self, path: &Path, allow_missing_final: bool) -> io::Result { + let canonical_root = self.root.canonicalize().map_err(|error| { + io::Error::new( + error.kind(), + format!("mount root {}: {error}", self.root.display()), + ) + })?; + let full = self.resolve(path, Follow::Final)?; + + if fs::metadata(&full).await.is_err() { + let parent_exists = match full.parent() { + Some(parent) => fs::metadata(parent).await.is_ok(), + None => false, + }; + if !parent_exists || !allow_missing_final { + return Err(io::Error::new( + io::ErrorKind::NotFound, + format!("No such file or directory: {}", path.display()), + )); + } + } + + let relative = full.strip_prefix(&canonical_root).map_err(|_| { + io::Error::other(format!( + "canonicalize: {} is not under {} (internal error)", + full.display(), + canonical_root.display() + )) + })?; + Ok(relative.to_path_buf()) + } + async fn symlink(&self, target: &Path, link: &Path) -> io::Result<()> { self.check_writable()?; diff --git a/crates/kaish-vfs/src/traits.rs b/crates/kaish-vfs/src/traits.rs index 8b2e24e3..444642de 100644 --- a/crates/kaish-vfs/src/traits.rs +++ b/crates/kaish-vfs/src/traits.rs @@ -241,6 +241,103 @@ pub trait Filesystem: Send + Sync { // Default: same as stat (for backends that don't support symlinks) self.stat(path).await } + + /// Resolve `path` to its canonical form: follow every symlink hop, + /// fold `.` and `..` lexically, root-relative in and root-relative out + /// — same as every other path this trait takes and returns. + /// + /// The final component may be missing when `allow_missing_final` is + /// true (GNU `readlink -f` semantics). A missing INTERMEDIATE + /// component is always an error, whichever way `allow_missing_final` + /// is set. Symlink hops are capped at 40, matching Linux + /// `MAXSYMLINKS`; exceeding the cap is an error, never a silent stop. + /// + /// The default walks component by component through [`Filesystem::lstat`] + /// and [`Filesystem::read_link`], so it inherits whatever containment + /// those already give — correct for a backend with no root to enforce + /// (`MemoryFs`, an unrooted `LocalFs`). `LocalFs` overrides this with + /// one containment-checked resolve instead of a round trip per hop; + /// `VfsRouter` overrides it to delegate to the mount that owns the + /// path. + async fn canonicalize(&self, path: &Path, allow_missing_final: bool) -> io::Result { + let components: Vec<_> = path.components().collect(); + let total = components.len(); + let mut current = PathBuf::new(); + + for (idx, component) in components.iter().enumerate() { + let is_last = idx + 1 == total; + match component { + std::path::Component::RootDir => {} + std::path::Component::CurDir => {} + std::path::Component::ParentDir => { + current.pop(); + } + std::path::Component::Normal(_) => { + current.push(component); + current = + resolve_symlink_hop(self, current, is_last && allow_missing_final).await?; + } + std::path::Component::Prefix(_) => { + current.push(component); + } + } + } + Ok(current) + } +} + +/// Symlink hops [`Filesystem::canonicalize`]'s default walk follows before +/// refusing, matching Linux's `MAXSYMLINKS`. +const MAX_SYMLINK_HOPS: usize = 40; + +/// Follow the symlink chain starting at `path`, if any, to the entry it +/// names. `allow_missing` permits `path` itself to be absent; every hop +/// short of it must exist. +async fn resolve_symlink_hop( + fs: &F, + path: PathBuf, + allow_missing: bool, +) -> io::Result { + let mut current = path; + for _ in 0..MAX_SYMLINK_HOPS { + match fs.lstat(¤t).await { + Ok(entry) if entry.is_symlink() => { + let target = fs.read_link(¤t).await?; + current = if target.is_absolute() { + target + } else { + let parent = current.parent().unwrap_or(Path::new("")); + parent.join(target) + }; + current = fold_dots(current); + } + Ok(_) => return Ok(current), + Err(e) if e.kind() == io::ErrorKind::NotFound && allow_missing => return Ok(current), + Err(e) => return Err(e), + } + } + Err(io::Error::other(format!( + "too many levels of symbolic links: {}", + current.display() + ))) +} + +/// Collapse `.` and `..` lexically in a root-relative path: `..` past the +/// start is dropped, not accumulated — the same clamp-at-root rule +/// `MemoryFs`'s own path normalization uses, since a root-relative path has +/// no "above root" to walk into. +fn fold_dots(path: PathBuf) -> PathBuf { + let mut out = PathBuf::new(); + for component in path.components() { + match component { + std::path::Component::ParentDir => { + out.pop(); + } + std::path::Component::CurDir => {} + other => out.push(other), + } + } + out } /// Whether two paths spell the same name once `.` and `..` are resolved diff --git a/docs/EMBEDDING.md b/docs/EMBEDDING.md index 9ea4e34e..4b3b9bd4 100644 --- a/docs/EMBEDDING.md +++ b/docs/EMBEDDING.md @@ -402,6 +402,39 @@ against any `Filesystem`; `run_all(make_root)` takes a closure that returns a fresh empty root per case. A backend that inherits the trait's `lstat` default fails the first case rather than passing silently. +### Canonicalizing a path (`canonicalize`) + +`readlink -f` and `realpath` both resolve through `Filesystem::canonicalize` +(and its mirror, `KernelBackend::canonicalize`): follow every symlink hop, +fold `.` and `..` lexically. The final component may be missing when the +caller passes `allow_missing_final: true` (GNU `readlink -f` semantics, +which `realpath` also uses but then requires the answer to exist); a missing +INTERMEDIATE component is always an error, whichever way that flag is set. +Symlink hops are capped at 40, matching Linux `MAXSYMLINKS`. + +The default walks component by component through `lstat`/`read_link`, so it +inherits whatever containment those already give — correct for a backend +with no root to enforce (`MemoryFs`, an unrooted `LocalFs`). `LocalFs` +overrides it with one `resolve_beneath` call instead of a round trip per +hop, containment-checked the same way `read`/`write`/`stat` already are: a +symlink target that walks above the mount's root is refused with +`path escapes root: {path} is not under {root}`. `VfsRouter` overrides it to +delegate to the single mount that owns the path, translating VFS-absolute to +mount-relative and back — a synthesized ancestor of a mount (`/v` above +`/v/jobs`) is a directory the router creates, never a symlink, so it +canonicalizes to itself. + +**A custom `KernelBackend` composed with `Kernel::with_backend`'s +`VirtualOverlayBackend` needs the same override**, and for the same reason +`VfsRouter` does: the trait default's per-hop walk calls `lstat`/`read_link` +on the overlay itself, re-deciding "virtual, shared-ancestor, or inner" on +every hop instead of asking the owning side once for the whole path. A +symlink that lives entirely under the embedder's own backend must resolve — +and be contained — through that backend's own resolver in one call, not be +re-routed through the split hop by hop. `VirtualOverlayBackend` already +overrides it this way; a hand-rolled `KernelBackend` wrapping another one +should follow the same shape. + ### Reporting file permissions (`path_access`) `test -r`, `test -w`, and `test -x` ask the mount that owns the path, through