diff --git a/shell/src/code/functions/mod.rs b/shell/src/code/functions/mod.rs index 91e16edee..6cdc59c5f 100644 --- a/shell/src/code/functions/mod.rs +++ b/shell/src/code/functions/mod.rs @@ -234,30 +234,78 @@ pub fn catalog() -> Vec { ] } +/// Register a coder function whose request carries a `base_dir`, folding the +/// operator-selected session directory into the jail +/// ([`PathResolver::session_scoped`]) ONCE — here, not copy-pasted per verb — +/// before delegating to the typed handler. The standard arm threads the live +/// `CoderConfig` snapshot; [`register_scoped_no_cfg`] is the variant for +/// handlers that don't read config. +/// +/// [`PathResolver::session_scoped`]: crate::code::path::PathResolver::session_scoped +macro_rules! register_scoped { + ($iii:expr, $id:expr, $desc:expr, $resolver:expr, $cfg:expr, $req:ty, $handle:path $(,)?) => {{ + let resolver = $resolver; + let cfg = $cfg; + $iii.register_function( + $id, + RegisterFunction::new_async(move |req: $req| { + let resolver = resolver.clone(); + let cfg = cfg.clone(); + async move { + let cfg = cfg.read().await.clone(); + let resolver = resolver.session_scoped(req.base_dir.as_deref()); + $handle(resolver, cfg, req).await.map_err(Error::from) + } + }) + .description($desc), + ); + }}; +} + +/// `base_dir`-scoping registrar for handlers that take no `CoderConfig`. +/// Mirrors [`register_scoped`] without the config read. +macro_rules! register_scoped_no_cfg { + ($iii:expr, $id:expr, $desc:expr, $resolver:expr, $req:ty, $handle:path $(,)?) => {{ + let resolver = $resolver; + $iii.register_function( + $id, + RegisterFunction::new_async(move |req: $req| { + let resolver = resolver.clone(); + async move { + let resolver = resolver.session_scoped(req.base_dir.as_deref()); + $handle(resolver, req).await.map_err(Error::from) + } + }) + .description($desc), + ); + }}; +} + pub fn register_all(iii: &IIIClient, resolver: Arc, cfg: ConfigCell) { - // DRIFT GUARD: the register_* calls below and the entries in - // `catalog()` must stay 1:1 — catalog() feeds the wire-schema goldens + // DRIFT GUARD: the registrations below and the entries in `catalog()` + // must stay 1:1 — catalog() feeds the wire-schema goldens // (tests/code_golden_schemas.rs). Adding a function to one list but not // the other trips the debug_assert below (exercised engine-free by // `tests::register_all_count_matches_catalog`). let mut registered: usize = 0; + // `info` carries no `base_dir`, so it is registered unscoped. register_info(iii, resolver.clone(), cfg.clone()); registered += 1; - register_read_file(iii, resolver.clone(), cfg.clone()); + register_scoped!(iii, READ_FILE_ID, READ_FILE_DESC, resolver.clone(), cfg.clone(), read_file::ReadFileInput, read_file::handle); registered += 1; - register_search(iii, resolver.clone(), cfg.clone()); + register_scoped!(iii, SEARCH_ID, SEARCH_DESC, resolver.clone(), cfg.clone(), search::SearchInput, search::handle); registered += 1; - register_update_file(iii, resolver.clone(), cfg.clone()); + register_scoped!(iii, UPDATE_FILE_ID, UPDATE_FILE_DESC, resolver.clone(), cfg.clone(), update_file::UpdateFileInput, update_file::handle); registered += 1; - register_create_file(iii, resolver.clone(), cfg.clone()); + register_scoped!(iii, CREATE_FILE_ID, CREATE_FILE_DESC, resolver.clone(), cfg.clone(), create_file::CreateFileInput, create_file::handle); registered += 1; - register_delete_file(iii, resolver.clone()); + register_scoped!(iii, LIST_FOLDER_ID, LIST_FOLDER_DESC, resolver.clone(), cfg.clone(), list_folder::ListFolderInput, list_folder::handle); registered += 1; - register_list_folder(iii, resolver.clone(), cfg.clone()); + register_scoped!(iii, TREE_ID, TREE_DESC, resolver.clone(), cfg.clone(), tree::TreeInput, tree::handle); registered += 1; - register_tree(iii, resolver.clone(), cfg.clone()); + register_scoped_no_cfg!(iii, DELETE_FILE_ID, DELETE_FILE_DESC, resolver.clone(), delete_file::DeleteFileInput, delete_file::handle); registered += 1; - register_move_file(iii, resolver); + register_scoped_no_cfg!(iii, MOVE_FILE_ID, MOVE_FILE_DESC, resolver, move_file::MoveFileInput, move_file::handle); registered += 1; debug_assert_eq!( registered, @@ -284,132 +332,6 @@ fn register_info(iii: &IIIClient, resolver: Arc, cfg: ConfigCell) ); } -fn register_read_file(iii: &IIIClient, resolver: Arc, cfg: ConfigCell) { - iii.register_function( - READ_FILE_ID, - RegisterFunction::new_async(move |req: read_file::ReadFileInput| { - let resolver = resolver.clone(); - let cfg = cfg.clone(); - async move { - let cfg = cfg.read().await.clone(); - read_file::handle(resolver, cfg, req) - .await - .map_err(Error::from) - } - }) - .description(READ_FILE_DESC), - ); -} - -fn register_search(iii: &IIIClient, resolver: Arc, cfg: ConfigCell) { - iii.register_function( - SEARCH_ID, - RegisterFunction::new_async(move |req: search::SearchInput| { - let resolver = resolver.clone(); - let cfg = cfg.clone(); - async move { - let cfg = cfg.read().await.clone(); - search::handle(resolver, cfg, req) - .await - .map_err(Error::from) - } - }) - .description(SEARCH_DESC), - ); -} - -fn register_update_file(iii: &IIIClient, resolver: Arc, cfg: ConfigCell) { - iii.register_function( - UPDATE_FILE_ID, - RegisterFunction::new_async(move |req: update_file::UpdateFileInput| { - let resolver = resolver.clone(); - let cfg = cfg.clone(); - async move { - let cfg = cfg.read().await.clone(); - update_file::handle(resolver, cfg, req) - .await - .map_err(Error::from) - } - }) - .description(UPDATE_FILE_DESC), - ); -} - -fn register_create_file(iii: &IIIClient, resolver: Arc, cfg: ConfigCell) { - iii.register_function( - CREATE_FILE_ID, - RegisterFunction::new_async(move |req: create_file::CreateFileInput| { - let resolver = resolver.clone(); - let cfg = cfg.clone(); - async move { - let cfg = cfg.read().await.clone(); - create_file::handle(resolver, cfg, req) - .await - .map_err(Error::from) - } - }) - .description(CREATE_FILE_DESC), - ); -} - -fn register_delete_file(iii: &IIIClient, resolver: Arc) { - iii.register_function( - DELETE_FILE_ID, - RegisterFunction::new_async(move |req: delete_file::DeleteFileInput| { - let resolver = resolver.clone(); - async move { - delete_file::handle(resolver, req) - .await - .map_err(Error::from) - } - }) - .description(DELETE_FILE_DESC), - ); -} - -fn register_list_folder(iii: &IIIClient, resolver: Arc, cfg: ConfigCell) { - iii.register_function( - LIST_FOLDER_ID, - RegisterFunction::new_async(move |req: list_folder::ListFolderInput| { - let resolver = resolver.clone(); - let cfg = cfg.clone(); - async move { - let cfg = cfg.read().await.clone(); - list_folder::handle(resolver, cfg, req) - .await - .map_err(Error::from) - } - }) - .description(LIST_FOLDER_DESC), - ); -} - -fn register_tree(iii: &IIIClient, resolver: Arc, cfg: ConfigCell) { - iii.register_function( - TREE_ID, - RegisterFunction::new_async(move |req: tree::TreeInput| { - let resolver = resolver.clone(); - let cfg = cfg.clone(); - async move { - let cfg = cfg.read().await.clone(); - tree::handle(resolver, cfg, req).await.map_err(Error::from) - } - }) - .description(TREE_DESC), - ); -} - -fn register_move_file(iii: &IIIClient, resolver: Arc) { - iii.register_function( - MOVE_FILE_ID, - RegisterFunction::new_async(move |req: move_file::MoveFileInput| { - let resolver = resolver.clone(); - async move { move_file::handle(resolver, req).await.map_err(Error::from) } - }) - .description(MOVE_FILE_DESC), - ); -} - #[cfg(test)] mod tests { use super::*; diff --git a/shell/src/code/path.rs b/shell/src/code/path.rs index 3bbfd38fd..0d7072df4 100644 --- a/shell/src/code/path.rs +++ b/shell/src/code/path.rs @@ -27,6 +27,7 @@ //! old MIRROR-INVARIANT between two copies is gone). use std::path::{Path, PathBuf}; +use std::sync::Arc; use globset::{Glob, GlobSet, GlobSetBuilder}; @@ -432,6 +433,67 @@ impl PathResolver { None => self.require_writable(rel), } } + + /// Clone this resolver with `base_canon` ADDED to the allowed roots — + /// the per-session working directory the harness scoped the call to. + /// A *selected* directory is thereby folded into the jail, so every + /// downstream check treats it as a first-class root: containment in + /// [`resolve`]/[`resolve_in`], the non-accessible denylist (which + /// relativises via [`containing_root`]), [`is_root`], and + /// [`session_root`]. No-op (the root is not duplicated) when + /// `base_canon` already sits inside a configured root — the existing + /// in-jail behaviour is byte-for-byte unchanged. + /// + /// [`resolve`]: Self::resolve + /// [`resolve_in`]: Self::resolve_in + /// [`containing_root`]: Self::containing_root + /// [`is_root`]: Self::is_root + /// [`session_root`]: Self::session_root + fn with_session_root(&self, base_canon: PathBuf) -> Self { + let mut roots_canon = self.roots_canon.clone(); + if !roots_canon.iter().any(|r| base_canon.starts_with(r)) { + roots_canon.push(base_canon); + } + Self { + roots_canon, + non_accessible: self.non_accessible.clone(), + default_exclude: self.default_exclude.clone(), + default_exclude_dirs: self.default_exclude_dirs.clone(), + } + } + + /// Per-call jail for a session scoped to `base_dir`: returns a resolver + /// whose roots include the SELECTED working directory, so a directory + /// the operator picked in the console (delivered as `base_dir`) is + /// reachable instead of rejected with C215. + /// + /// Returns the shared resolver UNCHANGED when there is no `base_dir`, + /// when `base_dir` already canonicalises inside a configured root (the + /// common case — only an `Arc` bump), or when it cannot be + /// canonicalised (the handler's own `resolve_in` then produces the + /// precise error). Only when the selected directory sits OUTSIDE the + /// configured jail is it added via [`with_session_root`]. + /// + /// This is safe to widen on: `base_dir` is stamped by the harness + /// control plane (workspace injection), never by the model, so only + /// operator-chosen directories grow the jail — and `resolve_in` still + /// scopes access to the session directory, while the non-accessible + /// denylist still applies because the selected directory is now a real + /// root. + /// + /// [`with_session_root`]: Self::with_session_root + pub fn session_scoped(self: &Arc, base_dir: Option<&str>) -> Arc { + let Some(bd) = base_dir else { + return self.clone(); + }; + let Ok(base_canon) = self.canonicalize_wire(bd, Path::new(bd)) else { + return self.clone(); + }; + if self.containing_root(&base_canon).is_some() { + return self.clone(); + } + Arc::new(self.with_session_root(base_canon)) + } } fn compile_globset(patterns: &[String], key: &str) -> Result { @@ -721,6 +783,50 @@ mod tests { assert!(r.is_non_accessible(&abs_b), ".env in root[1] must match"); } + #[test] + fn session_scoped_adds_selected_dir_outside_jail() { + // A directory the operator selects (delivered as `base_dir`) that + // lives OUTSIDE the configured jail must become reachable — added + // into the effective roots — while the denylist still applies and + // access stays scoped to that directory. + let jail = tempdir().unwrap(); + let selected = tempdir().unwrap(); + std::fs::write(selected.path().join("a.txt"), b"x").unwrap(); + std::fs::write(selected.path().join(".env"), b"secret").unwrap(); + let r = std::sync::Arc::new( + PathResolver::new(&cfg_roots(vec![jail.path().to_path_buf()], vec!["**/.env"])) + .unwrap(), + ); + let sel = selected.path().display().to_string(); + + // Before scoping: the selected dir is outside every allowed root. + assert!( + r.resolve_in(&sel, "a.txt").is_err(), + "selected dir outside the jail must reject until added" + ); + + // session_scoped folds the selected dir into the jail roots. + let scoped = r.session_scoped(Some(&sel)); + let abs = scoped + .resolve_in(&sel, "a.txt") + .expect("selected dir reachable once added to host_roots"); + assert!(abs.starts_with(canon(selected.path()))); + + // The non-accessible denylist still guards the added root. + let env_abs = scoped.resolve_in(&sel, ".env").unwrap(); + assert!( + scoped.is_non_accessible(&env_abs), + "**/.env must still be blocked inside the selected dir" + ); + + // A base_dir already inside a configured root is a no-op. + let inside = r.session_scoped(Some(&jail.path().display().to_string())); + assert_eq!(inside.roots().len(), r.roots().len()); + + // No base_dir is a no-op too. + assert_eq!(r.session_scoped(None).roots().len(), r.roots().len()); + } + #[test] fn default_config_constructs_resolver() { // CI interface collection boots the worker with zero config from a