From 8c7051f046343f7a97104335fb3da605d33bf6b4 Mon Sep 17 00:00:00 2001 From: Sam & Claude Date: Sat, 20 Jun 2026 22:58:30 +0200 Subject: [PATCH] fix(vault): canonicalize + allowed-root containment on provision target (#92) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #91 added a string-equality registered-vs-spawned root check, which doesn't catch `..`, symlinks, or a root pointing outside the jails tree. Add a real containment guard in colibri-vault::provision, the layer that writes the .env: - Before create_dir_all, canonicalize the target (resolving `..`/symlinks) and assert it is STRICTLY under the allowed jail-root base; refuse otherwise. Running before create_dir_all means a traversal/symlink target can't even create a directory outside the tree, let alone an .env. - Allowed base defaults to /usr/local/bastille/jails (FreeBSD/Bastille), overridable via COLIBRI_JAIL_ROOT_BASE for Linux/Docker volume roots. - Fail-closed: returns VaultError::TargetEscapesRoot; the daemon spawn hook already treats provision errors as fail-soft (no .env written). - Tests: child accepted; base-itself / nonexistent / `..`-escape / symlink-escape all refused (no tempfile dep — uses std temp_dir). Acceptance (#92): a target with `..`, a symlink, or resolving outside the jail root is refused, no .env written. fmt + clippy --all-targets clean; cargo test --workspace 230 passed / 0 failed. Co-Authored-By: Claude Opus 4.8 --- crates/colibri-vault/src/lib.rs | 97 +++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/crates/colibri-vault/src/lib.rs b/crates/colibri-vault/src/lib.rs index 5668105..d7b2e2e 100644 --- a/crates/colibri-vault/src/lib.rs +++ b/crates/colibri-vault/src/lib.rs @@ -53,6 +53,9 @@ pub enum VaultError { #[error("cannot write to target: {0}")] TargetNotWritable(PathBuf), + #[error("target '{target}' does not resolve under the allowed jail root '{base}'")] + TargetEscapesRoot { target: PathBuf, base: PathBuf }, + #[error("I/O error: {0}")] Io(#[from] std::io::Error), @@ -77,6 +80,13 @@ pub async fn provision( // Verify bw is available which("bw").ok_or(VaultError::BwNotFound)?; + // Containment: the target must resolve (symlinks + `..` collapsed) to a path + // strictly under the allowed jail-root base. This runs BEFORE create_dir_all + // so a traversal/symlink target can never have a directory — let alone an + // .env — created outside the jails tree. Fail-closed (returns Err; the + // spawn hook treats provision errors as fail-soft "no .env written"). + assert_contained(target_dir)?; + // Verify target is writable std::fs::create_dir_all(target_dir)?; let test = target_dir.join(".vault-write-test"); @@ -102,6 +112,40 @@ pub struct ProvisionResult { pub path: PathBuf, } +// ── Containment ─────────────────────────────────────── + +/// Allowed jail-root base. Tenant `.env` targets must resolve to a path strictly +/// under this. Defaults to the FreeBSD/Bastille convention; override with +/// `COLIBRI_JAIL_ROOT_BASE` (e.g. the container volume root on Linux/Docker). +fn jail_root_base() -> PathBuf { + std::env::var_os("COLIBRI_JAIL_ROOT_BASE") + .map(PathBuf::from) + .unwrap_or_else(|| PathBuf::from("/usr/local/bastille/jails")) +} + +/// Canonicalize `target` and assert it lives strictly under `base` (also +/// canonicalized), so `..` and symlinks cannot escape. Returns the resolved +/// target on success. Errors if either path cannot be canonicalized (e.g. the +/// target does not exist — provisioning targets an already-spawned jail). +fn assert_contained_under(base: &Path, target: &Path) -> Result { + let escapes = || VaultError::TargetEscapesRoot { + target: target.to_path_buf(), + base: base.to_path_buf(), + }; + let base_real = std::fs::canonicalize(base).map_err(|_| escapes())?; + let target_real = std::fs::canonicalize(target).map_err(|_| escapes())?; + if target_real != base_real && target_real.starts_with(&base_real) { + Ok(target_real) + } else { + Err(escapes()) + } +} + +/// Containment guard against the configured [`jail_root_base`]. +fn assert_contained(target: &Path) -> Result { + assert_contained_under(&jail_root_base(), target) +} + // ── Internal helpers ────────────────────────────────── struct VaultSession { @@ -476,4 +520,57 @@ mod tests { assert!(validate_key("KEY\nMALICIOUS=1").is_empty()); assert!(validate_key("").is_empty()); } + + fn unique_tmp(tag: &str) -> PathBuf { + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(); + let mut p = std::env::temp_dir(); + p.push(format!( + "colibri-vault-{tag}-{}-{nanos}", + std::process::id() + )); + p + } + + #[test] + fn assert_contained_under_enforces_jail_root() { + use std::fs; + let base = unique_tmp("base"); + let jail_root = base.join("jailA/root"); + fs::create_dir_all(&jail_root).unwrap(); + + // a real child resolves and is accepted + assert_eq!( + assert_contained_under(&base, &jail_root).unwrap(), + fs::canonicalize(&jail_root).unwrap() + ); + // the base itself is not strictly under the base + assert!(assert_contained_under(&base, &base).is_err()); + // a nonexistent target cannot be canonicalized -> refused + assert!(assert_contained_under(&base, &base.join("nope")).is_err()); + // `..` traversal climbing above the base -> refused + assert!(assert_contained_under(&base, &jail_root.join("../../..")).is_err()); + + let _ = fs::remove_dir_all(&base); + } + + #[cfg(unix)] + #[test] + fn assert_contained_under_rejects_symlink_escape() { + use std::fs; + let base = unique_tmp("symbase"); + let outside = unique_tmp("outside"); + fs::create_dir_all(&base).unwrap(); + fs::create_dir_all(&outside).unwrap(); + let link = base.join("evil"); + std::os::unix::fs::symlink(&outside, &link).unwrap(); + + // a symlink under base that resolves outside is refused + assert!(assert_contained_under(&base, &link).is_err()); + + let _ = fs::remove_dir_all(&base); + let _ = fs::remove_dir_all(&outside); + } } -- 2.45.3