Skip to content

feat(shell): fold the selected session directory into the jail roots #375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ytallo wants to merge 2 commits into from
Closed

feat(shell): fold the selected session directory into the jail roots #375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16779d8
feat(shell): fold the selected session directory into the jail roots
ytallo Jun 30, 2026
d5fdb28
refactor(shell): collapse coder fn registration into a session-scopin…
ytallo Jun 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
194 changes: 58 additions & 136 deletions shell/src/code/functions/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,30 +234,78 @@ pub fn catalog() -> Vec<FunctionSpec> {
]
}

/// 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<PathResolver>, 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,
Expand All @@ -284,132 +332,6 @@ fn register_info(iii: &IIIClient, resolver: Arc<PathResolver>, cfg: ConfigCell)
);
}

fn register_read_file(iii: &IIIClient, resolver: Arc<PathResolver>, 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<PathResolver>, 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<PathResolver>, 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<PathResolver>, 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<PathResolver>) {
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<PathResolver>, 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<PathResolver>, 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<PathResolver>) {
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::*;
Expand Down
106 changes: 106 additions & 0 deletions shell/src/code/path.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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<Self>, base_dir: Option<&str>) -> Arc<Self> {
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))
}
Comment on lines +485 to +496

@coderabbitai coderabbitai Bot Jun 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 1. Where is base_dir defined on the input structs and how is it deserialized?
rg -nP -C3 '\bbase_dir\b' --type=rust shell/src/code/functions | rg -nP -C3 '(struct|serde|deserialize|Deserialize|Option<String>)'

# 2. Find the input struct definitions carrying base_dir.
ast-grep run --pattern 'struct $S { $$$ base_dir: $T, $$$ }' --lang rust shell/src

# 3. Is base_dir ever overwritten/injected server-side (control plane / workspace injection)
#    rather than taken verbatim from the deserialized model request?
rg -nP -C3 '\bbase_dir\s*[:=]' --type=rust shell/src | rg -nvP '//'

# 4. How requests reach register_function / new_async — is the arg payload model-authored?
rg -nP -C4 'new_async|register_function' --type=rust shell/src/code/functions/mod.rs

Repository: iii-hq/workers

Length of output: 50371

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== functions/mod.rs outline =="
ast-grep outline shell/src/code/functions/mod.rs --view expanded | sed -n '1,220p'

echo
echo "== functions/types.rs outline =="
ast-grep outline shell/src/functions/types.rs --view expanded | sed -n '1,260p'

echo
echo "== request structs with base_dir in code/functions =="
rg -n -C4 'pub base_dir: Option<String>|base_dir: req\.base_dir\.as_deref\(\)|base_dir:' shell/src/code/functions/*.rs shell/src/functions/types.rs

echo
echo "== registration / schema plumbing in functions/mod.rs =="
sed -n '1,260p' shell/src/code/functions/mod.rs

echo
echo "== top of functions/types.rs =="
sed -n '1,240p' shell/src/functions/types.rs

Repository: iii-hq/workers

Length of output: 50370

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== shell/src/code/functions/mod.rs around register_all =="
sed -n '220,460p' shell/src/code/functions/mod.rs

echo
echo "== shell/src/code/functions/mod.rs around catalog/spec =="
sed -n '170,240p' shell/src/code/functions/mod.rs

echo
echo "== all use sites of req.base_dir.as_deref() in code/functions =="
rg -n -C2 'req\.base_dir\.as_deref\(\)' shell/src/code/functions

echo
echo "== all JsonSchema/Deserialize request structs with base_dir =="
rg -n -C3 '#\[derive\([^]]*Deserialize[^]]*\)' shell/src/code/functions/*.rs shell/src/functions/types.rs | sed -n '1,220p'

Repository: iii-hq/workers

Length of output: 29640

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== shell/src/code/path.rs around session root logic =="
sed -n '430,540p' shell/src/code/path.rs

echo
echo "== shell/src/code/path.rs around resolve_in / require_writable_in =="
sed -n '340,430p' shell/src/code/path.rs

echo
echo "== shell/src/code/path.rs around canonicalize_wire / containing_root =="
sed -n '140,220p' shell/src/code/path.rs

Repository: iii-hq/workers

Length of output: 12784

Treat base_dir as untrusted input before widening the jail. base_dir is part of the deserialized tool request schema and is passed straight into session_scoped(), so the allowed roots expand from caller-supplied data. Move it to server-side metadata or reject non-control-plane values before cloning the resolver.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@shell/src/code/path.rs` around lines 485 - 496, Treat the
session_scoped(base_dir) input as untrusted and do not use caller-provided
request data to widen the jail. Update session_scoped in Path so it only derives
the session root from server-side/control-plane metadata, or explicitly reject
non-control-plane base_dir values before calling canonicalize_wire,
containing_root, or with_session_root; keep the clone-only behavior for
untrusted/absent inputs.

}

fn compile_globset(patterns: &[String], key: &str) -> Result<GlobSet, CoderError> {
Expand Down Expand Up @@ -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
Expand Down
Loading