diff --git a/approval-gate/Cargo.lock b/approval-gate/Cargo.lock index 78637bbe6..e32a015a3 100644 --- a/approval-gate/Cargo.lock +++ b/approval-gate/Cargo.lock @@ -83,7 +83,7 @@ checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" [[package]] name = "approval-gate" -version = "1.0.8" +version = "1.0.9" dependencies = [ "anyhow", "approval-gate", @@ -545,7 +545,7 @@ dependencies = [ [[package]] name = "harness" -version = "1.3.3" +version = "1.4.0" dependencies = [ "anyhow", "async-trait", diff --git a/approval-gate/Cargo.toml b/approval-gate/Cargo.toml index f2d46bef9..85b102a5b 100644 --- a/approval-gate/Cargo.toml +++ b/approval-gate/Cargo.toml @@ -2,7 +2,7 @@ [package] name = "approval-gate" -version = "1.0.8" +version = "1.0.9" edition = "2021" publish = false diff --git a/approval-gate/README.md b/approval-gate/README.md index 478e8a7ba..a86b4d2b7 100644 --- a/approval-gate/README.md +++ b/approval-gate/README.md @@ -106,7 +106,7 @@ restart, reconcile with one `approval::list-pending` call. ## Folder-access grant watch When a `shell::*` / `coder::*` call fails with a jail-scope error (`S215`, -`S220`, `C215`, `C218`) whose message carries a `grant_hint=` tail, +`S220`, `C215`, `C220`) whose message carries a `grant_hint=` tail, `approval::grant-watch` (a `post_trigger` hook, bound `on_error: "fail_open"` so a crashed/absent watch never turns an already-decided result into a stuck call) converts the failure into a `kind: "folder_access"` pending record diff --git a/approval-gate/src/filesystem_access.rs b/approval-gate/src/filesystem_access.rs index 9e7dac06f..dcdfa780d 100644 --- a/approval-gate/src/filesystem_access.rs +++ b/approval-gate/src/filesystem_access.rs @@ -10,7 +10,7 @@ use crate::types::{AccessRequest, HookInput, HookResult}; /// The jail-scope rejection codes a `filesystem_access_request` can ride on. -pub const JAIL_SCOPE_CODES: [&str; 4] = ["S215", "S220", "C215", "C218"]; +pub const JAIL_SCOPE_CODES: [&str; 4] = ["S215", "S220", "C215", "C220"]; const ACCESS_REQUEST_MARKER: &str = "filesystem_access_request="; diff --git a/approval-gate/src/types.rs b/approval-gate/src/types.rs index f169ff709..88f8ba2c7 100644 --- a/approval-gate/src/types.rs +++ b/approval-gate/src/types.rs @@ -157,7 +157,7 @@ pub struct AccessRequest { /// The raw offending path from the request, as sent by the caller. pub attempted_path: String, /// The jail-scope rejection code that produced this hint (`S215`, - /// `S220`, `C215`, `C218`). + /// `S220`, `C215`, `C220`). pub error_code: String, } diff --git a/approval-gate/tests/golden/schemas/approval.get-pending.json b/approval-gate/tests/golden/schemas/approval.get-pending.json index 55f1a0523..ced07effc 100644 --- a/approval-gate/tests/golden/schemas/approval.get-pending.json +++ b/approval-gate/tests/golden/schemas/approval.get-pending.json @@ -39,7 +39,7 @@ "type": "string" }, "error_code": { - "description": "The jail-scope rejection code that produced this hint (`S215`, `S220`, `C215`, `C218`).", + "description": "The jail-scope rejection code that produced this hint (`S215`, `S220`, `C215`, `C220`).", "type": "string" }, "requested_root": { diff --git a/approval-gate/tests/golden/schemas/approval.list-pending.json b/approval-gate/tests/golden/schemas/approval.list-pending.json index 1901db4af..2aed04739 100644 --- a/approval-gate/tests/golden/schemas/approval.list-pending.json +++ b/approval-gate/tests/golden/schemas/approval.list-pending.json @@ -53,7 +53,7 @@ "type": "string" }, "error_code": { - "description": "The jail-scope rejection code that produced this hint (`S215`, `S220`, `C215`, `C218`).", + "description": "The jail-scope rejection code that produced this hint (`S215`, `S220`, `C215`, `C220`).", "type": "string" }, "requested_root": { diff --git a/approval-gate/tests/golden/schemas/approval.pending-created.json b/approval-gate/tests/golden/schemas/approval.pending-created.json index 1d6e4058c..8ab3f6d1a 100644 --- a/approval-gate/tests/golden/schemas/approval.pending-created.json +++ b/approval-gate/tests/golden/schemas/approval.pending-created.json @@ -11,7 +11,7 @@ "type": "string" }, "error_code": { - "description": "The jail-scope rejection code that produced this hint (`S215`, `S220`, `C215`, `C218`).", + "description": "The jail-scope rejection code that produced this hint (`S215`, `S220`, `C215`, `C220`).", "type": "string" }, "requested_root": { diff --git a/shell/CHANGELOG.md b/shell/CHANGELOG.md index f100d6982..700409d79 100644 --- a/shell/CHANGELOG.md +++ b/shell/CHANGELOG.md @@ -1,5 +1,45 @@ # Changelog +## 0.10.0 + +### Breaking + +- **`coder::*` error codes renumbered to align with the `S2xx` scheme** — + equal digits now mean the same failure class on both surfaces: + already-exists `C217` → `C213`; too-large `C213` → `C218`; + outside-session `C218` → `C220`. The approval-gate's jail-scope + allowlist (`C218` → `C220`) ships in the same release wave; a stale + approval-gate will not prompt on coder session-escapes until upgraded. +- **`shell::fs::*` existence redaction unified with coder's** — + permission-denied, protected-glob, and `fs.denylist_paths` rejections + fold into `S211` with the single "not found or not accessible" wording + (previously `S215`). `S215` is now exclusively a jail-confinement + escape. This closes an existence-probing side channel. + +### Changed + +- **`coder::*` follows the deny-only policy when unjailed (MOT-4099)** — + with `fs.allow_unjailed: true` and empty `fs.host_roots`, coder accepts + absolute paths anywhere on the host; the cwd + `/tmp` fallback roots + only anchor relative paths, and the harness-stamped working directory + is trusted as the anchor under both filesystem boundaries (matching + `shell::exec`'s cwd contract). `fs.denylist_paths` now applies to + `coder::*` in every mode (redacted `C211`). Jailed deployments are + unchanged; the `workspace` boundary keeps session scoping so the + folder-approval flow still triggers. +- **Unjailed `shell::fs::*` now enforces `code.non_accessible_globs`** — + previously the glob check silently skipped paths outside every + configured root, leaving secrets unprotected in unjailed mode. +- `coder::info` reports the effective access `mode` (`jailed` | + `unjailed`). + +### Migration + +- Update any consumer branching on `C213`/`C217`/`C218` to the new + numbers (`C218`/`C213`/`C220` respectively). +- Update any consumer that distinguished `S215` permission/denylist + rejections from `S211` not-found — both are now `S211` by design. + ## 0.8.0 Deny-only, permissive-first policy across the board: the shell no longer diff --git a/shell/Cargo.lock b/shell/Cargo.lock index 9d969de48..b40c6da0a 100644 --- a/shell/Cargo.lock +++ b/shell/Cargo.lock @@ -1775,7 +1775,7 @@ dependencies = [ [[package]] name = "shell" -version = "0.9.2" +version = "0.10.0" dependencies = [ "anyhow", "async-trait", diff --git a/shell/Cargo.toml b/shell/Cargo.toml index aa5ad6585..8acdee41e 100644 --- a/shell/Cargo.toml +++ b/shell/Cargo.toml @@ -2,7 +2,7 @@ [package] name = "shell" -version = "0.9.2" +version = "0.10.0" edition = "2021" publish = false diff --git a/shell/README.md b/shell/README.md index a4512288f..21e7c7188 100644 --- a/shell/README.md +++ b/shell/README.md @@ -182,20 +182,63 @@ fully unjailed, regardless of `fs.allow_unjailed`. Roots come from `fs.host_roots` (with the cwd+`/tmp` fallback noted above); protection globs come from `code.non_accessible_globs` in the shipped `config.yaml`'s `code:` block — the **same** list `shell::fs::*` enforces. -`coder::*` returns its own `C2xx` codes, distinct from `shell::*`'s `S2xx`: +`coder::*` returns its own `C2xx` codes. Since 0.10.0 the NUMBERS mean the +same thing on both surfaces — `C2xx` and `S2xx` with equal digits are the +same failure class, so a caller can learn the taxonomy once: -| Code | Meaning | -|---|---| -| `C210` | Malformed input: bad payload, illegal line numbers, overlapping ops. | -| `C211` | Path not found, or matched `non_accessible_globs` — deliberately the same code for both, so a caller can't probe for a denied file's existence by toggling the glob. | -| `C213` | File exceeds `max_read_bytes`/`max_write_bytes`. | -| `C215` | Path escapes every allowed root, lexically or through a symlink. | -| `C216` | Underlying I/O error. | -| `C217` | `create-file` saw an existing file and `overwrite=false`. | -| `C218` | Path resolves inside a configured root but outside the per-call `scope_root` the session is scoped to. | +| Code | Meaning | fs twin | +|---|---|---| +| `C210` | Malformed input: bad payload, illegal line numbers, overlapping ops. | `S210` | +| `C211` | Path not found, permission denied, matched `non_accessible_globs`, or under `fs.denylist_paths` — deliberately ONE code and wording for all four, so a caller can't probe for a denied path's existence. | `S211` | +| `C213` | `create-file`/`move` saw an existing target and `overwrite=false`. | `S213` | +| `C215` | Path escapes every allowed root, lexically or through a symlink (jailed mode only). | `S215` | +| `C216` | Underlying I/O error. | `S216` | +| `C218` | File exceeds `max_read_bytes`/`max_write_bytes`. | `S218` | +| `C220` | Path resolves inside a configured root but outside the per-call `scope_root` the session is scoped to. | `S220` | No separate install: `iii worker add shell` brings the whole surface. +## Two surfaces, one contract + +`shell::fs::*` and `coder::*` are two views of the same filesystem, served by +this one worker under ONE policy: the same jail (`fs.host_roots`), the same +unjailed opt-in (`fs.allow_unjailed`), the same protected globs +(`code.non_accessible_globs`), the same operator denylist +(`fs.denylist_paths`), and — since 0.10.0 — the same error-code semantics +(equal digits, equal meaning) and the same existence redaction (a denied path +reads exactly like a missing one on both surfaces). + +They differ in ergonomics, and each operation has a twin: + +| Task | `coder::*` (agent-ergonomic) | `shell::fs::*` (byte-level) | +|---|---|---| +| read | `coder::read-file` — inline text, windowed, batched | `shell::fs::read` — streams bytes via channel | +| create | `coder::create-file` — batched inline text | `shell::fs::write` — inline or streamed, modes | +| edit | `coder::update-file` — line ops + regex, post-apply echoes | `shell::fs::sed` — regex replace across files | +| delete | `coder::delete-file` — batched, per-entry errors | `shell::fs::rm` — single path | +| list | `coder::list-folder` / `coder::tree` — paginated, noise-filtered | `shell::fs::ls` — single directory | +| move | `coder::move` — batched, cross-root copy+delete | `shell::fs::mv` — single path | +| search | `coder::search` — budgeted, context lines | `shell::fs::grep` — raw matches | +| introspect | `coder::info` — mode, roots, caps, globs | `shell::fs::stat` — one path's metadata | + +Conventions (hold these when adding functions to either surface): + +- **Batching**: `coder::*` operations are batched with per-entry error + isolation. `shell::fs::*` point operations are single-path (`write` and + `sed` are the historical exceptions). +- **Naming**: `coder::*` uses verb-noun kebab (`read-file`); `shell::fs::*` + uses the unix tool name (`read`, `ls`, `mv`). Follow the surface you are + extending. +- **Errors**: one `{ code, message }` envelope; C/S codes with equal digits + mean the same failure class (see [Errors](#errors)). Redaction: never let + a denied path read differently from a missing one. +- **Discovery**: `coder::info` is the agent-facing contract report. + `shell::config-status` (reload health) and `shell::workspace::*` (console + working-directory picker) are operator/console control plane, not agent + tools. +- **Sandbox**: `shell::fs::*`/`shell::exec` accept `target: sandbox`; + `coder::*` is host-only. + ## Hot-reload When the `configuration` worker pushes an updated config, the shell worker swaps in the new security policy and fs backend atomically. A few things to know: @@ -214,11 +257,11 @@ Returned error bodies carry a stable `code` field. Denylist rejections come back |---|---| | `S200` | In-VM execution failure on a sandbox target. | | `S210` | Invalid request: non-absolute path, empty command or pattern, bad octal mode, malformed payload, `sandbox.enabled: false` on a sandbox-targeted call, a `cwd` that is not a directory, an `env` key in the dangerous-key denylist, `cwd`/`env`/`stdin` supplied on a sandbox target (host-only), an inline string `content` on a sandbox-targeted `shell::fs::write`, or both single `path`/`content` and `files` on `shell::fs::write`. | -| `S211` | Path not found (including a `cwd` that does not exist). | +| `S211` | Path not found, permission denied, matched the protected globs, or under `fs.denylist_paths` — ONE code and wording for all four (redaction: a denied path reads exactly like a missing one). Includes a `cwd` that does not exist. | | `S212` | Wrong file type for the operation (for example, a file where a directory was expected). | | `S213` | Path already exists. | | `S214` | Directory not empty (non-recursive `rm`). | -| `S215` | Path (or a per-call `cwd`) escapes the `fs.host_roots` jail, hits `fs.denylist_paths`, or permission denied. | +| `S215` | Path (or a per-call `cwd`) escapes the `fs.host_roots` jail — exclusively a confinement escape; denylist and permission-denied fold into `S211`. | | `S216` | Generic shell-internal failure: host spawn error, channel error, or a bad engine response. | | `S217` | Invalid regex passed to `grep`/`sed`. | | `S218` | `fs.max_read_bytes` / `fs.max_write_bytes` cap exceeded. | @@ -226,6 +269,29 @@ Returned error bodies carry a stable `code` field. Denylist rejections come back Sandbox-forwarded `fs::*`/`exec` errors can also surface engine codes verbatim instead of collapsing to `S216`: `S001`–`S004` (sandbox lifecycle), `S100`–`S102` (image/VM/resource), `S300`, and `S400`. Branch on the specific code where relevant; only an unrecognized engine code falls back to `S216`. +## Upgrading to 0.10.0 + +- **BREAKING: three `coder::*` error codes renumbered** to align with the + `S2xx` scheme (equal digits now mean the same failure class on both + surfaces): already-exists `C217` → `C213`; too-large `C213` → `C218`; + outside-session `C218` → `C220`. Consumers branching on the old numbers + must update; the approval-gate's jail-scope allowlist ships the matching + change in the same release wave. +- **BREAKING: `shell::fs::*` existence redaction.** Permission-denied, + protected-glob (`code.non_accessible_globs`), and `fs.denylist_paths` + rejections now return `S211` with the same "not found or not accessible" + wording as a missing path, instead of `S215`. `S215` is now exclusively a + jail-confinement escape. Callers must not distinguish "missing" from + "denied" — that distinction was an existence-probing side channel. +- **`coder::*` is permissive when unjailed.** With the explicit opt-in + (`fs.allow_unjailed: true`, empty `fs.host_roots`) the coder surface now + follows the same deny-only policy as `shell::fs::*`: absolute paths + anywhere on the host, the cwd + `/tmp` fallback roots demoted to + relative-path anchors, and the harness-selected working directory trusted + as the anchor. `fs.denylist_paths` now applies to `coder::*` in every + mode. Jailed deployments (explicit `fs.host_roots`) are unchanged. + `coder::info` reports the effective `mode`. + ## Upgrading to 0.8.0 - **BREAKING: `allowlist` removed.** The command allowlist is gone — the diff --git a/shell/src/code/config.rs b/shell/src/code/config.rs index 2019e45c1..965b49690 100644 --- a/shell/src/code/config.rs +++ b/shell/src/code/config.rs @@ -32,6 +32,25 @@ pub struct CoderConfig { #[schemars(skip)] pub base_paths: Vec, + /// Runtime plumbing, NEVER read from config: true when the operator + /// explicitly opted into the unjailed mode (`fs.allow_unjailed: true` + /// with empty `fs.host_roots`). Copied here by + /// `ShellConfig::code_resolver_config` so the resolver applies the same + /// deny-only policy as `shell::fs::*`: absolute paths anywhere on the + /// host, confined only by `denylist_paths` and `non_accessible_globs`. + /// The `base_paths` fallback roots still anchor relative wire paths. + #[serde(skip)] + #[schemars(skip)] + pub unjailed: bool, + + /// Runtime plumbing, NEVER read from config: `fs.denylist_paths` copied + /// here by `ShellConfig::code_resolver_config` so both surfaces honor + /// the same operator path denylist. Matching paths are rejected with + /// the redacted C211 (indistinguishable from missing). + #[serde(skip)] + #[schemars(skip)] + pub denylist_paths: Vec, + /// Glob patterns matched against the path *relative to its containing /// root*. Matching files can be listed but not /// read/written/deleted/created. @@ -48,13 +67,13 @@ pub struct CoderConfig { pub default_exclude_globs: Vec, /// Per-file IO ceiling, in bytes, for `coder::read-file` in every mode - /// (full, windowed, batch). A larger file fails with C213 naming the + /// (full, windowed, batch). A larger file fails with C218 naming the /// size. Default 10485760 (10 MiB). #[serde(default = "default_max_read_bytes")] pub max_read_bytes: u64, /// Cap, in bytes, on the content of a single `coder::create-file` / - /// `coder::update-file` call (C213 when exceeded). Default 10485760 + /// `coder::update-file` call (C218 when exceeded). Default 10485760 /// (10 MiB). #[serde(default = "default_max_write_bytes")] pub max_write_bytes: u64, @@ -95,7 +114,7 @@ pub struct CoderConfig { /// replacements before being counted, so the cap bounds what the /// caller actually receives). Entries are collected in request order /// until this budget is exhausted; an entry reached with zero budget - /// remaining gets a per-entry C213. Single-path FULL reads are + /// remaining gets a per-entry C218. Single-path FULL reads are /// budgeted by `max_output_bytes` instead; `max_read_bytes` remains /// the per-file IO ceiling in every mode. #[serde(default = "default_batch_read_budget_bytes")] @@ -106,7 +125,7 @@ pub struct CoderConfig { /// RETURNED CONTENT after UTF-8 sanitization (numbered prefixes /// included) — the same accounting unit as `batch_read_budget_bytes`. /// A full read whose converted content would exceed this budget - /// fails with a C213 that reports the file's size and line count and + /// fails with a C218 that reports the file's size and line count and /// names the recovery paths (window, stat probe, or per-call /// `max_output_bytes` raise, clamped to `max_read_bytes`). Windowed /// reads and batch mode are NOT governed by this key. @@ -185,6 +204,14 @@ pub struct JailSignature { /// boundary, so it is restart-required: the `PathResolver` canonicalizes /// these once at boot and refuses to swap them live. pub base_paths: Vec, + /// The unjailed opt-in (`fs.allow_unjailed` + empty `fs.host_roots`). + /// Flipping it moves the security boundary wholesale, so it is + /// restart-required like the root set it derives from. + pub unjailed: bool, + /// The operator path denylist (`fs.denylist_paths`), canonicalized into + /// the resolver at boot. Part of the deny-only protection layer, so + /// restart-required. + pub denylist_paths: Vec, /// The access-deny globs. These are the read/write/delete protection layer /// (e.g. `.env`, `*.pem`), compiled into the `PathResolver` at boot. A /// change alters the security posture, so it is restart-required — never @@ -201,6 +228,8 @@ impl Default for CoderConfig { fn default() -> Self { Self { base_paths: Vec::new(), + unjailed: false, + denylist_paths: Vec::new(), non_accessible_globs: Vec::new(), default_exclude_globs: default_default_exclude_globs(), max_read_bytes: default_max_read_bytes(), @@ -261,6 +290,8 @@ impl CoderConfig { pub fn jail_signature(&self) -> JailSignature { JailSignature { base_paths: self.base_paths.clone(), + unjailed: self.unjailed, + denylist_paths: self.denylist_paths.clone(), non_accessible_globs: self.non_accessible_globs.clone(), default_exclude_globs: self.default_exclude_globs.clone(), } diff --git a/shell/src/code/error.rs b/shell/src/code/error.rs index 563e90769..89d40bc55 100644 --- a/shell/src/code/error.rs +++ b/shell/src/code/error.rs @@ -56,8 +56,8 @@ pub enum CoderError { NotFoundOrDenied(String), /// File exceeds `max_read_bytes` or `max_write_bytes`. - #[error("C213: {0}")] - #[serde(rename = "C213")] + #[error("C218: {0}")] + #[serde(rename = "C218")] TooLarge(String), /// Path escapes every allowed root, lexically or through a symlink. @@ -71,16 +71,16 @@ pub enum CoderError { Io(String), /// `create-file` saw an existing file and `overwrite=false`. - #[error("C217: {0}")] - #[serde(rename = "C217")] + #[error("C213: {0}")] + #[serde(rename = "C213")] AlreadyExists(String), /// Path canonicalises inside a configured root but OUTSIDE the /// per-call `scope_root` the session is scoped to. Distinct from `C215` /// (outside EVERY root) so the rejection can name the session /// directory rather than contradict `coder::info`'s allowed-roots list. - #[error("C218: {0}")] - #[serde(rename = "C218")] + #[error("C220: {0}")] + #[serde(rename = "C220")] OutsideSession(String), } @@ -97,11 +97,11 @@ impl CoderError { match self { CoderError::BadInput(_) => "C210", CoderError::NotFoundOrDenied(_) => "C211", - CoderError::TooLarge(_) => "C213", + CoderError::TooLarge(_) => "C218", CoderError::OutsideBase(_) => "C215", CoderError::Io(_) => "C216", - CoderError::AlreadyExists(_) => "C217", - CoderError::OutsideSession(_) => "C218", + CoderError::AlreadyExists(_) => "C213", + CoderError::OutsideSession(_) => "C220", } } @@ -263,10 +263,10 @@ mod tests { std::io::Error::new(std::io::ErrorKind::AlreadyExists, "exists"), "some/file.txt", ); - assert_eq!(e.code(), "C217"); + assert_eq!(e.code(), "C213"); assert!( e.message().starts_with("some/file.txt: "), - "C217 via io_for_path must prefix the caller path: {}", + "C213 via io_for_path must prefix the caller path: {}", e.message() ); } @@ -274,7 +274,7 @@ mod tests { #[test] fn io_already_exists_maps_to_c217() { let e: CoderError = std::io::Error::new(std::io::ErrorKind::AlreadyExists, "x").into(); - assert_eq!(e.code(), "C217"); + assert_eq!(e.code(), "C213"); } #[test] diff --git a/shell/src/code/functions/create_file.rs b/shell/src/code/functions/create_file.rs index df37f260c..7b8c1928c 100644 --- a/shell/src/code/functions/create_file.rs +++ b/shell/src/code/functions/create_file.rs @@ -81,7 +81,7 @@ pub struct CreateFileResult { pub success: bool, pub bytes_written: u64, /// Structured error for this entry. `code` is stable for programmatic - /// branching (e.g. `"C217"` means already-exists; pass `overwrite=true` + /// branching (e.g. `"C213"` means already-exists; pass `overwrite=true` /// to replace). `message` carries the corrective action an LLM agent /// needs to make a successful second call. #[serde(skip_serializing_if = "Option::is_none")] @@ -301,7 +301,7 @@ mod tests { .unwrap(); assert!(!out.results[0].success); let err = out.results[0].error.as_ref().unwrap(); - assert_eq!(err.code, "C217"); + assert_eq!(err.code, "C213"); assert_eq!( std::fs::read_to_string(tmp.path().join("a.txt")).unwrap(), "old" @@ -361,7 +361,7 @@ mod tests { #[tokio::test] async fn jail_escape_aborts_batch_before_any_write() { // Jail-scope failures must be top-level call errors so the harness - // post-trigger approval hook can see the C215/C218 and hold the call. + // post-trigger approval hook can see the C215/C220 and hold the call. // Preflight all paths before I/O: a later escape must not leave earlier // entries partially written before the call is re-invoked after a grant. let (tmp, r, c) = setup(); @@ -425,7 +425,7 @@ mod tests { .await .unwrap(); assert!(!out.results[0].success); - assert_eq!(out.results[0].error.as_ref().unwrap().code, "C213"); + assert_eq!(out.results[0].error.as_ref().unwrap().code, "C218"); } #[tokio::test] diff --git a/shell/src/code/functions/info.rs b/shell/src/code/functions/info.rs index 2aeb433b3..a4f5ea3a4 100644 --- a/shell/src/code/functions/info.rs +++ b/shell/src/code/functions/info.rs @@ -23,12 +23,32 @@ fn example_info_input() -> serde_json::Value { serde_json::json!({}) } +/// Effective access mode of the coder surface — the same deny-only policy +/// switch `shell::fs::*` runs under. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum AccessMode { + /// Paths are confined to `base_paths`; anything outside is rejected. + Jailed, + /// Operator opt-in (`fs.allow_unjailed: true`, empty `fs.host_roots`): + /// absolute paths anywhere on the host are accepted, confined only by + /// `fs.denylist_paths` and `non_accessible_globs`. `base_paths` only + /// anchor relative wire paths. + Unjailed, +} + #[derive(Debug, Serialize, JsonSchema)] pub struct InfoOutput { + /// `jailed`: paths must stay inside `base_paths`. `unjailed`: absolute + /// paths anywhere on the host are accepted (deny-only, like + /// `shell::fs::*`) and `base_paths` only anchor relative paths. + pub mode: AccessMode, + /// Canonical absolute paths of the allowed roots, in configuration order. /// The primary root (index 0) is where relative wire paths resolve; an - /// absolute path is accepted when it canonicalises inside ANY of these. - /// Paths outside every root are rejected — use `shell::fs::*` instead. + /// absolute path is accepted when it canonicalises inside ANY of these + /// (in `unjailed` mode: anywhere on the host). In `jailed` mode paths + /// outside every root are rejected — use `shell::fs::*` instead. pub base_paths: Vec, /// Convenience duplicate of `base_paths[0]` — the primary allowed root. @@ -49,14 +69,14 @@ pub struct InfoOutput { pub default_exclude_globs: Vec, /// Per-file IO ceiling for `coder::read-file`. Full reads of files - /// larger than this are rejected with C213; windowed reads cap the + /// larger than this are rejected with C218; windowed reads cap the /// returned window bytes instead, so larger files stay readable /// window by window. Also the ceiling for `coder::search` content /// scanning — larger files are silently skipped during search. pub max_read_bytes: u64, /// Maximum bytes that `coder::create-file` or `coder::update-file` will - /// accept for a single file write. Larger writes are rejected with C213. + /// accept for a single file write. Larger writes are rejected with C218. pub max_write_bytes: u64, /// Default `max_depth` used by `coder::tree` when the caller omits it. @@ -94,7 +114,7 @@ pub struct InfoOutput { /// counted, so the cap bounds what the caller actually receives). /// Entries are collected in request order; each entry may consume up /// to `min(remaining_budget, max_read_bytes)`. An entry reached with - /// zero remaining budget receives a per-entry C213 naming this key, + /// zero remaining budget receives a per-entry C218 naming this key, /// its value, and the bytes already consumed, with recovery guidance. /// Budget topology: batch reads are governed by this key; single-path /// full reads by `max_output_bytes`; windowed reads by `max_read_bytes` @@ -104,7 +124,7 @@ pub struct InfoOutput { /// Context budget for single-path FULL reads in `coder::read-file`, /// in bytes of returned content. Full reads larger than this return - /// C213 with the file's size/line count and window/stat recovery + /// C218 with the file's size/line count and window/stat recovery /// guidance; a per-call `max_output_bytes` override is available on /// `coder::read-file` (clamped to `max_read_bytes`). pub max_output_bytes: u64, @@ -133,6 +153,11 @@ fn inner(resolver: &PathResolver, cfg: &CoderConfig) -> InfoOutput { let primary_root = base_paths[0].clone(); InfoOutput { + mode: if resolver.unjailed() { + AccessMode::Unjailed + } else { + AccessMode::Jailed + }, base_paths, primary_root, non_accessible_globs: cfg.non_accessible_globs.clone(), @@ -219,6 +244,7 @@ mod tests { search_response_budget_bytes: 29, batch_read_budget_bytes: 23, max_output_bytes: 31, + ..CoderConfig::default() }); let resolver = Arc::new(PathResolver::new(&cfg).unwrap()); @@ -227,6 +253,7 @@ mod tests { // primary_root == base_paths[0] assert!(!out.base_paths.is_empty()); assert_eq!(out.primary_root, out.base_paths[0]); + assert_eq!(out.mode, AccessMode::Jailed); // globs assert_eq!( diff --git a/shell/src/code/functions/mod.rs b/shell/src/code/functions/mod.rs index 4e4971213..5aa459ed3 100644 --- a/shell/src/code/functions/mod.rs +++ b/shell/src/code/functions/mod.rs @@ -38,14 +38,17 @@ use crate::code::state::CodeCells; // --------------------------------------------------------------------------- const INFO_ID: &str = "coder::info"; -const INFO_DESC: &str = "Report the coder jail: canonical allowed roots (primary first), \ - per-file size caps, response budgets (max_output_bytes, \ - batch_read_budget_bytes, search_response_budget_bytes), \ - listing/search limits, the non-accessible glob patterns, and the \ - default_exclude_globs noise filter applied by tree/search. Call \ - this FIRST when unsure where coder may read or write, or when a \ - path was rejected — paths outside every allowed root need the \ - shell worker's shell::fs::* instead."; +const INFO_DESC: &str = "Report the coder access contract: the effective mode (jailed = \ + paths confined to the allowed roots; unjailed = deny-only, absolute \ + paths anywhere on the host, roots anchor relative paths only), \ + canonical allowed roots (primary first), per-file size caps, \ + response budgets (max_output_bytes, batch_read_budget_bytes, \ + search_response_budget_bytes), listing/search limits, the \ + non-accessible glob patterns, and the default_exclude_globs noise \ + filter applied by tree/search. Call this FIRST when unsure where \ + coder may read or write, or when a path was rejected — in jailed \ + mode, paths outside every allowed root need the shell worker's \ + shell::fs::* instead."; const READ_FILE_ID: &str = "coder::read-file"; const READ_FILE_DESC: &str = "Read a file window-first: probe with stat: true (size/mtime/mode \ @@ -57,12 +60,12 @@ const READ_FILE_DESC: &str = "Read a file window-first: probe with stat: true (s matching coder::update-file's line ops exactly. Full reads are \ budgeted by max_output_bytes (default 128 KiB; per-call override \ clamped to max_read_bytes) — an over-budget full read fails with \ - a C213 carrying the file's size, line count, and the window/stat \ + a C218 carrying the file's size, line count, and the window/stat \ recovery calls. Batch mode: pass paths[] (XOR path) \ to read multiple files in one call — entries are processed in \ request order against batch_read_budget_bytes, measured in \ bytes of returned content (after UTF-8 sanitization); per-entry \ - errors (C211/C213) leave other entries unaffected. Paths are relative \ + errors (C211/C218) leave other entries unaffected. Paths are relative \ to the primary allowed root or absolute inside any allowed root \ (coder::info lists them); for host paths outside the jail use \ shell::fs::*. Non-accessible paths return C211."; diff --git a/shell/src/code/functions/move_file.rs b/shell/src/code/functions/move_file.rs index 1c5f06773..e80056cb8 100644 --- a/shell/src/code/functions/move_file.rs +++ b/shell/src/code/functions/move_file.rs @@ -94,7 +94,7 @@ pub struct MoveFileResult { pub moved: bool, /// Structured error for this entry. `code` is stable for programmatic - /// branching (e.g. `"C217"` means destination exists; pass `overwrite=true` + /// branching (e.g. `"C213"` means destination exists; pass `overwrite=true` /// to replace; `"C210"` for disallowed operations such as cross-root /// directory moves, moving a root itself, or a destination that is a /// directory — the message then names the corrected target path). @@ -271,7 +271,7 @@ fn try_move_one( if dst_meta.is_dir() && !src_meta.is_dir() { // overwrite=true can't fix this (a file cannot replace a // directory via rename), so don't send the caller down the - // C217 dead end — tell them the actual corrective call. + // C213 dead end — tell them the actual corrective call. let fname = abs_from .file_name() .map(|f| f.to_string_lossy().into_owned()) @@ -673,7 +673,7 @@ mod tests { } // ------------------------------------------------------------------ - // overwrite=false → C217 + // overwrite=false → C213 // ------------------------------------------------------------------ #[tokio::test] async fn overwrite_false_dst_exists_c217() { @@ -696,8 +696,8 @@ mod tests { .unwrap(); assert!(!out.results[0].success); let err = out.results[0].error.as_ref().unwrap(); - assert_eq!(err.code, "C217"); - // House C217 shape — identical format to create-file's (no colon). + assert_eq!(err.code, "C213"); + // House C213 shape — identical format to create-file's (no colon). assert_eq!( err.message, "dst.txt already exists; pass overwrite=true to replace" @@ -959,7 +959,7 @@ mod tests { // ------------------------------------------------------------------ // dst is a directory + src is a file → prescriptive C210 (not the - // C217 "pass overwrite=true" dead end — overwrite can't fix it). + // C213 "pass overwrite=true" dead end — overwrite can't fix it). // ------------------------------------------------------------------ #[tokio::test] async fn dst_is_directory_src_is_file_prescriptive_c210() { diff --git a/shell/src/code/functions/read_file.rs b/shell/src/code/functions/read_file.rs index 45f6ef860..b47d92d47 100644 --- a/shell/src/code/functions/read_file.rs +++ b/shell/src/code/functions/read_file.rs @@ -12,7 +12,7 @@ //! invalid bytes expand to 3-byte U+FFFD replacements BEFORE they are //! counted, so binary files can never deliver more than the budget). An //! entry cut short by the remaining budget succeeds with `more_lines: -//! true`; an entry reached with zero budget gets a per-entry C213 (names +//! true`; an entry reached with zero budget gets a per-entry C218 (names //! the config key + value, bytes consumed, and recovery guidance). //! Per-entry resolution/glob/stat failures return per-entry C211; budget //! is not consumed by failed entries. @@ -21,7 +21,7 @@ //! budget state — resolve + stat run BEFORE the zero-budget check, so a //! missing path and a glob-denied path both return C211 (identical //! wording, verbatim path echo) even after exhaustion. Only an existing, -//! accessible regular file may receive the budget C213. +//! accessible regular file may receive the budget C218. //! //! **XOR rule**: `path` XOR `paths` must be set; both or neither → C210. //! @@ -33,9 +33,9 @@ //! number; prefix bytes are charged against every byte cap/budget. //! Single-path FULL reads are additionally bounded by the //! `max_output_bytes` config (per-call override clamped to -//! `max_read_bytes`); the C213 carries size + total_lines + the +//! `max_read_bytes`); the C218 carries size + total_lines + the //! corrective calls. REDACTION ORDERING everywhere: resolve → deny -//! (C211) → metadata syscalls → budget (C213) — classification must +//! (C211) → metadata syscalls → budget (C218) — classification must //! never depend on budget state, and deny must precede any metadata //! syscall so stat/budget can't become an existence or size oracle. //! @@ -199,7 +199,7 @@ pub struct ReadFileInput { /// content bytes after UTF-8 conversion (numbered prefixes included). /// Values above `max_read_bytes` are silently clamped to it. When the /// full content would exceed the effective budget the call fails with - /// a C213 naming the file's size and `total_lines` — recover by + /// a C218 naming the file's size and `total_lines` — recover by /// windowing with `line_from`/`line_to`, probing with `stat: true`, /// or raising this field. Full reads only: combining it with /// `line_from`/`line_to` is C210 (windows are bounded by @@ -568,12 +568,12 @@ fn parse_window( /// Full (non-windowed) read: the whole file, pre-checked against /// `max_read_bytes`, then against the `max_output_bytes` context budget -/// (converted wire bytes, numbered prefixes included). Both C213s are +/// (converted wire bytes, numbered prefixes included). Both C218s are /// recovery tools: they name the actual sizes and the corrective calls. /// /// ORDERING (REDACTION RULE): callers run resolve → deny → metadata /// before reaching here, so by construction only an existing, accessible -/// regular file can ever receive either C213. +/// regular file can ever receive either C218. fn full_read( abs: &Path, wire_path: &str, @@ -694,7 +694,7 @@ fn stat_counts( /// Windowed read (single-path mode): stream lines `from..=to` via /// `BufReader`. The `max_read_bytes` cap bounds the COLLECTED window's /// RAW bytes — the T7 contract — never the file size (windowed mode -/// never returns C213 for an oversize file). +/// never returns C218 for an oversize file). fn windowed_read( abs: &Path, wire_path: &str, @@ -784,7 +784,7 @@ fn entry_failure(path: String, error: WireError) -> ReadEntryResult { /// regular-file check all run BEFORE the zero-budget check, so error /// classification never depends on budget state. Otherwise an agent /// could exhaust the budget and then distinguish a missing path (which -/// would hit the budget C213) from a glob-denied one (C211 at resolve). +/// would hit the budget C218) from a glob-denied one (C211 at resolve). fn batch_read( resolver: &PathResolver, cfg: &CoderConfig, @@ -861,7 +861,7 @@ fn batch_read( } // Stat probe: metadata only, no content — consumes no budget and - // is deliberately exempt from the zero-budget C213 below (stat is + // is deliberately exempt from the zero-budget C218 below (stat is // the cheap probe; an exhausted batch can still size files). // Resolve + deny + metadata already ran, so classification stays // budget-independent (REDACTION INVARIANT). @@ -892,7 +892,7 @@ fn batch_read( let consumed = cfg.batch_read_budget_bytes - remaining_budget; // Zero-budget check — only an existing, accessible regular file - // can reach this point, so C213 leaks nothing about protected or + // can reach this point, so C218 leaks nothing about protected or // missing paths. The message reports the ACTUAL accounted // consumption. if remaining_budget == 0 { @@ -1165,7 +1165,7 @@ mod tests { let (tmp, r, c) = setup(); std::fs::write(tmp.path().join("big.bin"), vec![0u8; 2048]).unwrap(); let err = handle(r, c, full("big.bin")).await.unwrap_err(); - assert!(err.contains("C213"), "got: {err}"); + assert!(err.contains("C218"), "got: {err}"); assert!(err.contains("line_from"), "got: {err}"); assert!(err.contains("line_to"), "got: {err}"); } @@ -1434,7 +1434,7 @@ mod tests { #[tokio::test] async fn batch_budget_partial_entry_has_more_lines_true() { // Budget: 10 bytes. Each line is 5 bytes. First entry consumes 10 - // bytes (2 lines). Second entry has zero budget → C213. + // bytes (2 lines). Second entry has zero budget → C218. let (tmp, r, c) = setup_with_batch_budget(10); std::fs::write(tmp.path().join("a.txt"), "aaaa\nbbbb\ncccc\n").unwrap(); std::fs::write(tmp.path().join("b.txt"), "data\n").unwrap(); @@ -1453,24 +1453,24 @@ mod tests { assert!(results[0].success); assert_eq!(results[0].more_lines, Some(true)); assert_eq!(results[0].lines_returned, Some(2)); - // Second entry: zero budget → C213 + // Second entry: zero budget → C218 assert!(!results[1].success); let wire = results[1].error.as_ref().unwrap(); - assert_eq!(wire.code, "C213"); + assert_eq!(wire.code, "C218"); assert!( wire.message.contains("batch_read_budget_bytes"), - "C213 must name the config key: {}", + "C218 must name the config key: {}", wire.message ); assert!( wire.message.contains("10"), - "C213 must name the budget value: {}", + "C218 must name the budget value: {}", wire.message ); } // ----------------------------------------------------------------------- - // Batch mode — zero-budget C213 details + // Batch mode — zero-budget C218 details // ----------------------------------------------------------------------- #[tokio::test] @@ -1493,7 +1493,7 @@ mod tests { assert!(results[0].success); assert!(!results[1].success); let wire = results[1].error.as_ref().unwrap(); - assert_eq!(wire.code, "C213"); + assert_eq!(wire.code, "C218"); assert!(wire.message.contains("batch_read_budget_bytes")); assert!(wire.message.contains('5'), "must name the value"); // Recovery guidance @@ -1511,7 +1511,7 @@ mod tests { #[tokio::test] async fn batch_budget_smaller_than_first_line_succeeds_with_empty_more_lines() { // Budget of 2 bytes; first line is "aaaaaaaa\n" (9 bytes). - // No-torn-lines: empty content, more_lines=true — NOT a C213 error. + // No-torn-lines: empty content, more_lines=true — NOT a C218 error. let (tmp, r, c) = setup_with_batch_budget(2); std::fs::write(tmp.path().join("f.txt"), "aaaaaaaa\nb\n").unwrap(); let out = handle(r, c, batch(vec![ReadTarget::Path("f.txt".into())])) @@ -1562,19 +1562,19 @@ mod tests { assert_eq!(results[0].more_lines, Some(true)); assert_eq!(results[0].is_utf8, Some(false)); - // Entry 1: zero wire budget remains → C213 reporting the ACTUAL + // Entry 1: zero wire budget remains → C218 reporting the ACTUAL // accounted consumption, not a hardcoded value. assert!(!results[1].success); let wire = results[1].error.as_ref().unwrap(); - assert_eq!(wire.code, "C213"); + assert_eq!(wire.code, "C218"); assert!( wire.message.contains("batch_read_budget_bytes is 10"), - "C213 must name the key + value: {}", + "C218 must name the key + value: {}", wire.message ); assert!( wire.message.contains("returned 10 bytes"), - "C213 must report actual accounted consumption: {}", + "C218 must report actual accounted consumption: {}", wire.message ); @@ -1628,7 +1628,7 @@ mod tests { async fn batch_post_exhaustion_missing_and_denied_indistinguishable() { // After the budget hits zero, a missing path and a glob-denied // path must BOTH return C211 with byte-identical message suffixes - // and verbatim path echoes — C213 may only reach an existing, + // and verbatim path echoes — C218 may only reach an existing, // accessible entry, or an agent could probe for protected files // by exhausting the budget first. let (tmp, r, c) = setup_with_batch_budget(5); @@ -1640,9 +1640,9 @@ mod tests { c, batch(vec![ ReadTarget::Path("eat.txt".into()), // consumes the whole budget - ReadTarget::Path("missing.txt".into()), // must be C211, NOT C213 + ReadTarget::Path("missing.txt".into()), // must be C211, NOT C218 ReadTarget::Path(".env".into()), // C211 (glob-denied) - ReadTarget::Path("exists.txt".into()), // C213 — exists + accessible + ReadTarget::Path("exists.txt".into()), // C218 — exists + accessible ]), ) .await @@ -1677,8 +1677,8 @@ mod tests { assert_eq!(results[1].path, "missing.txt"); assert_eq!(results[2].path, ".env"); - // Only the existing, accessible entry receives the budget C213. - assert_eq!(results[3].error.as_ref().unwrap().code, "C213"); + // Only the existing, accessible entry receives the budget C218. + assert_eq!(results[3].error.as_ref().unwrap().code, "C218"); } // ----------------------------------------------------------------------- @@ -1956,7 +1956,7 @@ mod tests { batch(vec![ ReadTarget::Path("eat.txt".into()), // consumes the whole budget target_stat("probe.txt"), // still succeeds - ReadTarget::Path("probe.txt".into()), // C213 — budget gone + ReadTarget::Path("probe.txt".into()), // C218 — budget gone ]), ) .await @@ -1967,7 +1967,7 @@ mod tests { assert_eq!(results[1].total_lines, Some(2)); assert_eq!(results[1].size, Some(6)); assert!(results[1].content.is_none()); - assert_eq!(results[2].error.as_ref().unwrap().code, "C213"); + assert_eq!(results[2].error.as_ref().unwrap().code, "C218"); } // ----------------------------------------------------------------------- @@ -2018,7 +2018,7 @@ mod tests { async fn numbered_prefix_charged_to_batch_budget() { // a.txt prefixed = "1→aaaa\n2→bbbb\n" = 18 wire bytes, exactly the // budget; unprefixed it is only 10 and b.txt would still fit. The - // prefix bytes must consume the budget → b.txt gets C213. + // prefix bytes must consume the budget → b.txt gets C218. let (tmp, r, c) = setup_with_batch_budget(18); std::fs::write(tmp.path().join("a.txt"), "aaaa\nbbbb\n").unwrap(); std::fs::write(tmp.path().join("b.txt"), "x\n").unwrap(); @@ -2039,7 +2039,7 @@ mod tests { Some("1\u{2192}aaaa\n2\u{2192}bbbb\n") ); assert!(!results[1].success, "prefix bytes must consume budget"); - assert_eq!(results[1].error.as_ref().unwrap().code, "C213"); + assert_eq!(results[1].error.as_ref().unwrap().code, "C218"); // Control: the same batch unprefixed fits both entries. let out = handle( @@ -2084,7 +2084,7 @@ mod tests { std::fs::write(tmp.path().join("big.txt"), "aaaa\nbbbb\ncccc\ndddd\n").unwrap(); let err = handle(r, c, full("big.txt")).await.unwrap_err(); let (code, msg) = parse_wire(&err); - assert_eq!(code, "C213"); + assert_eq!(code, "C218"); // The message is itself the recovery tool: size, total_lines, the // config key + per-call override, and every corrective call. assert!(msg.contains("20 bytes"), "must carry file size: {msg}"); @@ -2109,7 +2109,7 @@ mod tests { .unwrap(); assert_eq!(out.content.unwrap().len(), 131_072); let err = handle(r, c, full("over.txt")).await.unwrap_err(); - assert!(err.contains("C213"), "got: {err}"); + assert!(err.contains("C218"), "got: {err}"); assert!(err.contains("max_output_bytes"), "got: {err}"); } @@ -2131,7 +2131,7 @@ mod tests { async fn per_call_max_output_bytes_clamps_to_max_read_bytes() { // File: 8 invalid bytes + '\n' = 9 raw bytes (under max_read_bytes // 15) but 25 CONVERTED wire bytes. Per-call budget 1000 silently - // clamps to max_read_bytes (15) → 25 > 15 → C213. The config + // clamps to max_read_bytes (15) → 25 > 15 → C218. The config // budget (1000) alone would have admitted it — the clamp applies // to the per-call override. let (tmp, r, c) = setup_with_output_budget(1000, 15); @@ -2150,7 +2150,7 @@ mod tests { ..ReadFileInput::default() }; let err = handle(r, c, req).await.unwrap_err(); - assert!(err.contains("C213"), "clamped per-call must refuse: {err}"); + assert!(err.contains("C218"), "clamped per-call must refuse: {err}"); } #[tokio::test] @@ -2162,7 +2162,7 @@ mod tests { let err = handle(r.clone(), c.clone(), full("big.txt")) .await .unwrap_err(); - assert!(err.contains("C213")); + assert!(err.contains("C218")); let out = handle(r, c, window_req("big.txt", Some(1), Some(4))) .await .unwrap(); @@ -2186,7 +2186,7 @@ mod tests { } /// REDACTION ORDERING regression: resolve → deny (C211) → size → budget - /// (C213). A denied or missing path must classify C211 no matter how + /// (C218). A denied or missing path must classify C211 no matter how /// the budget relates to the file. #[tokio::test] async fn denied_huge_file_is_c211_not_c213() { diff --git a/shell/src/code/path.rs b/shell/src/code/path.rs index ac448863e..8cb16ee03 100644 --- a/shell/src/code/path.rs +++ b/shell/src/code/path.rs @@ -4,6 +4,11 @@ //! runtime from `fs.host_roots`). Relative wire //! paths resolve against the FIRST root (the "primary"); absolute wire //! paths are accepted when they canonicalise inside ANY allowed root. +//! When the operator opts into the unjailed mode (`fs.allow_unjailed: true` +//! with empty `fs.host_roots`) the containment gate is skipped — matching +//! `shell::fs::*`'s deny-only policy — and the roots become relative-path +//! ANCHORS only; `fs.denylist_paths` and `non_accessible_globs` still +//! apply everywhere. //! `PathResolver` canonicalises inputs (symlink-aware) and verifies //! containment so `..` and crafted symlinks cannot escape. A `GlobSet` //! built from `non_accessible_globs` further blocks read/write/delete on @@ -53,6 +58,14 @@ pub struct PathResolver { /// or symlink merely NAMED like an excluded directory. default_exclude_dirs: GlobSet, grant_roots_canon: Vec, + /// Deny-only permissive mode (`fs.allow_unjailed: true`, empty + /// `fs.host_roots`): the containment gate is skipped and `roots_canon` + /// only anchors relative wire paths, mirroring `shell::fs::*`. + unjailed: bool, + /// Canonical `fs.denylist_paths`. A resolved path under any entry is + /// rejected with the redacted C211 in every mode — the same operator + /// denylist `shell::fs::*` enforces. + denylist_canon: Vec, } /// Effective roots when `base_paths` is empty: the engine workspace cwd @@ -123,16 +136,40 @@ impl PathResolver { "default_exclude_glob", )?; - tracing::info!(roots = ?roots_canon, "path resolver roots"); + let mut denylist_canon = Vec::with_capacity(cfg.denylist_paths.len()); + for deny in &cfg.denylist_paths { + match canonicalize_with_fallback(deny) { + Ok(canon) => { + if !denylist_canon.contains(&canon) { + denylist_canon.push(canon); + } + } + Err(e) => tracing::warn!( + path = %deny.display(), + error = %e, + "skipping denylist path: cannot canonicalize" + ), + } + } + + tracing::info!(roots = ?roots_canon, unjailed = cfg.unjailed, "path resolver roots"); Ok(Self { roots_canon, non_accessible, default_exclude, default_exclude_dirs, grant_roots_canon: Vec::new(), + unjailed: cfg.unjailed, + denylist_canon, }) } + /// True when the operator opted into the deny-only permissive mode + /// (`fs.allow_unjailed: true` with empty `fs.host_roots`). + pub fn unjailed(&self) -> bool { + self.unjailed + } + /// Primary root — the first configured (and reachable) root. Relative /// wire paths resolve against it. pub fn base_root(&self) -> &Path { @@ -190,6 +227,8 @@ impl PathResolver { default_exclude: self.default_exclude.clone(), default_exclude_dirs: self.default_exclude_dirs.clone(), grant_roots_canon, + unjailed: self.unjailed, + denylist_canon: self.denylist_canon.clone(), }) } @@ -225,9 +264,7 @@ impl PathResolver { return None; } let base_canon = self.canonicalize_wire(scope_root, base_path).ok()?; - self.containing_root(&base_canon) - .is_some() - .then_some(base_canon) + (self.unjailed || self.containing_root(&base_canon).is_some()).then_some(base_canon) } /// Comma-separated display of all allowed roots, for C215 messages. @@ -253,11 +290,14 @@ impl PathResolver { self.base_root().join(wire) }; let canon = self.canonicalize_wire(path, &joined)?; - let inside = if is_absolute { - self.containing_root(&canon).is_some() - } else { - canon.starts_with(self.base_root()) - }; + // Unjailed (deny-only) mode: no containment — the roots only anchored + // the relative join above. The denylist gate below still applies. + let inside = self.unjailed + || if is_absolute { + self.containing_root(&canon).is_some() + } else { + canon.starts_with(self.base_root()) + }; if !inside { if is_absolute { // Absolute path outside every allowed root. The marker @@ -279,9 +319,21 @@ impl PathResolver { ))); } } + self.deny_check(path, &canon)?; Ok(canon) } + /// Reject a canonical path under any `fs.denylist_paths` entry with the + /// redacted C211 — the REDACTION INVARIANT applies to the operator + /// denylist exactly as it does to `non_accessible_globs`: a denylisted + /// path must be indistinguishable from a missing one. + fn deny_check(&self, path: &str, canon: &Path) -> Result<(), CoderError> { + if self.denylist_canon.iter().any(|d| canon.starts_with(d)) { + return Err(CoderError::not_found_or_denied(path)); + } + Ok(()) + } + /// Path's location relative to its CONTAINING root as a forward-slash /// string, suitable for glob matching. Returns `None` if `abs` isn't /// under any allowed root (should never happen for paths that came @@ -322,13 +374,19 @@ impl PathResolver { } fn matches_rel(&self, set: &GlobSet, abs: &Path) -> bool { - let Some(rel) = self.relative(abs) else { - return false; - }; - if rel.is_empty() { - return false; + if let Some(rel) = self.relative(abs) { + return !rel.is_empty() && set.is_match(&rel); + } + // Unjailed mode resolves paths outside every anchor root, where no + // root-relative form exists. Match against the root-stripped absolute + // form instead so `**/`-style patterns keep protecting secrets + // anywhere on the host. Jailed resolutions never reach this branch. + if self.unjailed { + let abs_str = abs.to_string_lossy().replace('\\', "/"); + let stripped = abs_str.trim_start_matches('/'); + return !stripped.is_empty() && set.is_match(stripped); } - set.is_match(&rel) + false } /// Shared symlink-safe canonicalisation of a joined wire path, with the @@ -374,7 +432,7 @@ impl PathResolver { /// absolute `path` is taken as-is. Either form runs through the SAME /// `canonicalize_wire` as `resolve`. /// 3. the canonical result must stay inside the canonical `scope_root` — - /// else `C218` (DX-1), which NAMES the session directory rather than + /// else `C220` (DX-1), which NAMES the session directory rather than /// reusing the generic "outside every allowed root" wording. /// /// This is strictly NARROWER than `resolve`: it can only reject paths @@ -425,7 +483,7 @@ impl PathResolver { "this session is scoped to {base}; {path} is inside an \ allowed root but outside the session directory — use a \ path under {base}.{hint}", - hint = crate::filesystem_access::request_suffix("C218", path, &canon), + hint = crate::filesystem_access::request_suffix("C220", path, &canon), ))); } // Outside the session AND outside every allowed root: the @@ -437,6 +495,7 @@ impl PathResolver { hint = crate::filesystem_access::request_suffix("C215", path, &canon), ))); } + self.deny_check(path, &canon)?; Ok(canon) } @@ -448,7 +507,9 @@ impl PathResolver { ))); } let anchor_canon = self.canonicalize_wire(anchor, anchor_path)?; - if self.containing_root(&anchor_canon).is_none() { + // Unjailed mode trusts the harness-stamped working directory anywhere + // on the host — the same contract shell::exec's cwd already honors. + if !self.unjailed && self.containing_root(&anchor_canon).is_none() { return Err(CoderError::OutsideBase(format!( "scope_root is outside every allowed root: {anchor}. {C215_ROOTS_PREFIX}{roots}", roots = self.roots_list() @@ -1090,7 +1151,7 @@ mod tests { // ----------------------------------------------------------------------- // Per-call scope_root (resolve_in / require_writable_scope): a containment // check + relative-anchor LAYERED on the existing jail core. These - // exercise the new C218 DX-1 error and prove scope_root=None is unchanged. + // exercise the new C220 DX-1 error and prove scope_root=None is unchanged. // ----------------------------------------------------------------------- /// Relative wire paths anchor at scope_root, NOT the primary root — even @@ -1137,7 +1198,7 @@ mod tests { } /// DX-1: an absolute path that IS inside a configured root but OUTSIDE - /// scope_root is rejected with the new C218 code, and the message NAMES + /// scope_root is rejected with the new C220 code, and the message NAMES /// the session directory (not the generic "outside every allowed root" /// wording, which would contradict coder::info's allowed-roots list). #[test] @@ -1152,30 +1213,30 @@ mod tests { // sibling.txt is inside the allowed root but OUTSIDE session/. let sibling = tmp.path().join("sibling.txt").display().to_string(); let err = r.resolve_in(&base, &sibling).unwrap_err(); - assert_eq!(err.code(), "C218"); + assert_eq!(err.code(), "C220"); let msg = err.to_string(); // DX-1: the session directory must be named in the rejection. assert!( msg.contains(&base), - "C218 must name the session dir ({base}); got: {msg}" + "C220 must name the session dir ({base}); got: {msg}" ); // DX-1: it must NOT reuse the generic "outside every allowed root" // wording — that path genuinely lives in an allowed root. assert!( !msg.contains("outside every allowed root"), - "C218 must not contradict the allowed-roots list; got: {msg}" + "C220 must not contradict the allowed-roots list; got: {msg}" ); // It should affirmatively explain the path is inside an allowed root. assert!( msg.contains("inside an allowed root"), - "C218 should clarify the path is inside a root but outside the \ + "C220 should clarify the path is inside a root but outside the \ session; got: {msg}" ); } /// In a multi-root config, an absolute path inside the SECOND root but /// outside the scope_root (which lives in the FIRST root) is still the - /// DX-1 C218 case — it is inside *an* allowed root, just not the session. + /// DX-1 C220 case — it is inside *an* allowed root, just not the session. #[test] fn resolve_in_absolute_in_other_root_outside_scope_root_is_c218() { let a = tempdir().unwrap(); @@ -1190,7 +1251,7 @@ mod tests { let base = canon(a.path()).display().to_string(); let in_b = b.path().join("y.txt").display().to_string(); let err = r.resolve_in(&base, &in_b).unwrap_err(); - assert_eq!(err.code(), "C218"); + assert_eq!(err.code(), "C220"); assert!(err.to_string().contains(&base)); } @@ -1295,14 +1356,14 @@ mod tests { // ../../ climbs above the allowed root entirely. let err = r.resolve_in(&base, "../../escape.txt").unwrap_err(); assert!( - err.code() == "C215" || err.code() == "C218", - "escape must fail closed (C215/C218); got {}", + err.code() == "C215" || err.code() == "C220", + "escape must fail closed (C215/C220); got {}", err.code() ); } /// A single `..` that stays inside the allowed root but climbs OUT of - /// scope_root is the DX-1 C218 case (inside a root, outside the session). + /// scope_root is the DX-1 C220 case (inside a root, outside the session). #[test] fn resolve_in_dotdot_within_root_but_outside_base_is_c218() { let tmp = tempdir().unwrap(); @@ -1313,7 +1374,7 @@ mod tests { // `../secret.txt` collapses to /secret.txt — inside the root, // outside the session. let err = r.resolve_in(&base, "../secret.txt").unwrap_err(); - assert_eq!(err.code(), "C218"); + assert_eq!(err.code(), "C220"); assert!(err.to_string().contains(&base)); } @@ -1330,8 +1391,8 @@ mod tests { let base = tmp.path().join("session").display().to_string(); let err = r.resolve_in(&base, "escape/child.txt").unwrap_err(); assert!( - err.code() == "C215" || err.code() == "C218", - "symlink escape must fail closed (C215/C218); got {}", + err.code() == "C215" || err.code() == "C220", + "symlink escape must fail closed (C215/C220); got {}", err.code() ); } @@ -1382,4 +1443,158 @@ mod tests { canon(&tmp.path().join("sibling/shared.txt")) ); } + + // ------------------------------------------------------------------ + // Unjailed (deny-only) mode: fs.allow_unjailed + empty fs.host_roots. + // The roots become relative-path anchors; containment is skipped; + // denylist_paths and non_accessible_globs still protect everywhere. + // ------------------------------------------------------------------ + + fn cfg_unjailed(anchor: PathBuf, globs: Vec<&str>) -> CoderConfig { + CoderConfig { + unjailed: true, + ..cfg_with(anchor, globs) + } + } + + #[test] + fn unjailed_accepts_absolute_path_outside_all_roots() { + let anchor = tempdir().unwrap(); + let outside = tempdir().unwrap(); + std::fs::write(outside.path().join("free.txt"), b"free").unwrap(); + let r = PathResolver::new(&cfg_unjailed(anchor.path().to_path_buf(), vec![])).unwrap(); + let got = r + .resolve(&outside.path().join("free.txt").display().to_string()) + .unwrap(); + assert_eq!(got, canon(&outside.path().join("free.txt"))); + } + + #[test] + fn unjailed_relative_still_anchors_at_primary_root() { + let anchor = tempdir().unwrap(); + std::fs::write(anchor.path().join("here.txt"), b"here").unwrap(); + let r = PathResolver::new(&cfg_unjailed(anchor.path().to_path_buf(), vec![])).unwrap(); + let got = r.resolve("here.txt").unwrap(); + assert_eq!(got, canon(&anchor.path().join("here.txt"))); + } + + #[test] + fn unjailed_configured_roots_scope_anchors_outside_all_roots() { + // Anthony's exact scenario (MOT-4099): the harness stamps a working + // directory OUTSIDE every anchor root under the degraded + // configured_roots boundary. Unjailed must trust it as the anchor. + let anchor = tempdir().unwrap(); + let workspace = tempdir().unwrap(); + std::fs::write(workspace.path().join("doc.md"), b"doc").unwrap(); + let r = PathResolver::new(&cfg_unjailed(anchor.path().to_path_buf(), vec![])).unwrap(); + let scope = crate::fs::FsScope { + root: workspace.path().display().to_string(), + grants: Vec::new(), + boundary: crate::fs::FsBoundary::ConfiguredRoots, + }; + assert_eq!( + r.resolve_scope(Some(&scope), "doc.md").unwrap(), + canon(&workspace.path().join("doc.md")) + ); + } + + #[test] + fn jailed_configured_roots_scope_outside_roots_still_rejected() { + // Without the opt-in the degraded boundary keeps failing closed — + // the pre-MOT-4099 behavior for explicitly jailed deployments. + let root = tempdir().unwrap(); + let workspace = tempdir().unwrap(); + let r = PathResolver::new(&cfg_with(root.path().to_path_buf(), vec![])).unwrap(); + let scope = crate::fs::FsScope { + root: workspace.path().display().to_string(), + grants: Vec::new(), + boundary: crate::fs::FsBoundary::ConfiguredRoots, + }; + let err = r.resolve_scope(Some(&scope), "doc.md").unwrap_err(); + assert_eq!(err.code(), "C215"); + assert!(err.to_string().contains("scope_root is outside"), "{err}"); + } + + #[test] + fn unjailed_non_accessible_globs_protect_outside_roots() { + // The REDACTION INVARIANT must hold anywhere on the host: a + // protected file outside every anchor root still returns C211. + let anchor = tempdir().unwrap(); + let outside = tempdir().unwrap(); + std::fs::write(outside.path().join(".env"), b"SECRET=1").unwrap(); + let r = + PathResolver::new(&cfg_unjailed(anchor.path().to_path_buf(), vec!["**/.env"])).unwrap(); + let abs = r + .resolve(&outside.path().join(".env").display().to_string()) + .unwrap(); + assert!(r.is_non_accessible(&abs), "glob must match outside roots"); + let err = r + .require_writable_scope(None, &outside.path().join(".env").display().to_string()) + .unwrap_err(); + assert_eq!(err.code(), "C211"); + } + + #[test] + fn denylist_paths_reject_with_redacted_c211_in_both_modes() { + let anchor = tempdir().unwrap(); + let denied = tempdir().unwrap(); + std::fs::write(denied.path().join("passwd"), b"x").unwrap(); + // Unjailed: the denylist is the only confinement — must hold. + let cfg = CoderConfig { + denylist_paths: vec![denied.path().to_path_buf()], + ..cfg_unjailed(anchor.path().to_path_buf(), vec![]) + }; + let r = PathResolver::new(&cfg).unwrap(); + let err = r + .resolve(&denied.path().join("passwd").display().to_string()) + .unwrap_err(); + assert_eq!(err.code(), "C211", "denylisted must be redacted, not C215"); + + // Jailed with the denied dir INSIDE a root: the denylist still wins. + let jailed_cfg = CoderConfig { + base_paths: vec![denied.path().to_path_buf()], + denylist_paths: vec![denied.path().to_path_buf()], + ..CoderConfig::default() + }; + let r = PathResolver::new(&jailed_cfg).unwrap(); + let err = r.resolve("passwd").unwrap_err(); + assert_eq!(err.code(), "C211"); + } + + #[test] + fn unjailed_session_root_resolves_outside_all_roots() { + // move/delete use session_root to refuse operating on the session + // dir itself; that protection must survive the unjailed mode where + // the stamped root sits outside every anchor root. + let anchor = tempdir().unwrap(); + let workspace = tempdir().unwrap(); + let r = PathResolver::new(&cfg_unjailed(anchor.path().to_path_buf(), vec![])).unwrap(); + assert_eq!( + r.session_root(&workspace.path().display().to_string()), + Some(canon(workspace.path())) + ); + } + + #[test] + fn unjailed_workspace_boundary_still_scopes_to_session() { + // With the approval hook live (workspace boundary), unjailed does + // NOT bypass the session scope: escapes keep raising C220 so the + // grant flow still triggers. + let anchor = tempdir().unwrap(); + std::fs::create_dir_all(anchor.path().join("session")).unwrap(); + std::fs::create_dir_all(anchor.path().join("elsewhere")).unwrap(); + std::fs::write(anchor.path().join("elsewhere/f.txt"), b"f").unwrap(); + let r = Arc::new( + PathResolver::new(&cfg_unjailed(anchor.path().to_path_buf(), vec![])).unwrap(), + ); + let session = anchor.path().join("session").display().to_string(); + let scoped = r.session_scoped(Some(&session), None); + let err = scoped + .resolve_in( + &session, + &anchor.path().join("elsewhere/f.txt").display().to_string(), + ) + .unwrap_err(); + assert_eq!(err.code(), "C220"); + } } diff --git a/shell/src/config.rs b/shell/src/config.rs index 28db468e5..f67d6d1f9 100644 --- a/shell/src/config.rs +++ b/shell/src/config.rs @@ -515,13 +515,12 @@ impl ShellConfig { /// registration and used as the runtime fallback when the stored value is /// null, so the worker boots with no config file at all (database-style /// zero-config). Unjailed by design (`allow_unjailed: true`, empty - /// `host_roots`): `shell::fs::*` and the per-call `cwd` on `shell::exec` - /// operate against the real filesystem, confined only by - /// `denylist_paths` — matching `shell::exec` itself, which is deny-only - /// rather than confinement-based. `coder::*` is unaffected: it falls back - /// to its own default roots (engine workspace cwd + `/tmp`, see - /// `code::path::default_roots`) whenever `host_roots` is empty, regardless - /// of `allow_unjailed`. `Default::default()` is ALSO unjailed (empty + /// `host_roots`): `shell::fs::*`, `coder::*`, and the per-call `cwd` on + /// `shell::exec` all operate against the real filesystem, confined only + /// by `denylist_paths` + `code.non_accessible_globs` — one deny-only + /// policy across both surfaces. `coder::*` keeps its fallback roots + /// (engine workspace cwd + `/tmp`, see `code::path::default_roots`) as + /// relative-path anchors only. `Default::default()` is ALSO unjailed (empty /// `host_roots`) but leaves `allow_unjailed: false`, so an operator config /// that merely omits the `fs` section — never explicitly opting in — /// still fails closed; only this seed opts in explicitly. This seed is @@ -584,10 +583,15 @@ impl ShellConfig { /// glob/budget settings from the `code` block, with ROOTS taken from /// `fs.host_roots` (the unified jail) — `code.base_paths` is runtime /// plumbing filled here, never read from config. This is what keeps the - /// merge's promise that the operator sets the root once. + /// merge's promise that the operator sets the root once. The unjailed + /// opt-in and `fs.denylist_paths` ride along the same way, so the coder + /// resolver applies the SAME deny-only policy as `shell::fs::*` when the + /// operator runs unjailed. pub fn code_resolver_config(&self) -> crate::code::config::CoderConfig { let mut c = self.code.clone(); c.base_paths = self.fs.roots(); + c.unjailed = self.fs.allow_unjailed && self.fs.host_roots.is_empty(); + c.denylist_paths = self.fs.denylist_paths.clone(); c } } @@ -1219,6 +1223,34 @@ sandbox: vec!["**/.env".to_string()], "the rest of the code block is preserved" ); + assert!( + !resolved.unjailed, + "explicit host_roots must never mark the resolver unjailed" + ); + } + + /// The unjailed opt-in and `fs.denylist_paths` ride the same wiring hop + /// as the roots: unjailed only when `allow_unjailed` AND empty + /// `host_roots`; the denylist is copied verbatim in every mode. + #[test] + fn code_resolver_config_threads_unjailed_and_denylist() { + let seed = ShellConfig::seed_default(); + let resolved = seed.code_resolver_config(); + assert!(resolved.unjailed, "seed (allow_unjailed + empty roots)"); + assert_eq!(resolved.denylist_paths, seed.fs.denylist_paths); + + let mut pinned = ShellConfig::seed_default(); + pinned.fs.host_roots = vec!["/tmp".into()]; + assert!( + !pinned.code_resolver_config().unjailed, + "pinned roots override the allow_unjailed opt-in" + ); + + let no_opt_in = ShellConfig::default(); + assert!( + !no_opt_in.code_resolver_config().unjailed, + "empty roots WITHOUT the opt-in stay jailed" + ); } #[test] diff --git a/shell/src/configuration.rs b/shell/src/configuration.rs index 248321a4b..6ee86569e 100644 --- a/shell/src/configuration.rs +++ b/shell/src/configuration.rs @@ -530,7 +530,10 @@ mod tests { } #[test] - fn zero_config_seed_builds_coder_cells_with_narrow_fallback_roots() { + fn zero_config_seed_builds_unjailed_coder_cells_with_anchor_roots() { + // The seed opts into unjailed (allow_unjailed + empty host_roots), so + // the coder resolver runs deny-only like shell::fs::* — with the + // narrow cwd + /tmp fallback kept as relative-path ANCHORS, not a jail. let cells = build_code_cells(&ShellConfig::seed_default()) .expect("zero-config shell must expose coder functions"); let resolver = cells.resolver.blocking_read(); @@ -538,12 +541,29 @@ mod tests { let expected_tmp = std::fs::canonicalize("/tmp").expect("/tmp exists"); assert_eq!(resolver.roots(), &[expected_cwd, expected_tmp]); + assert!( + resolver.unjailed(), + "the explicit allow_unjailed opt-in must reach the coder resolver" + ); assert!( cells.config.blocking_read().base_paths.is_empty(), "empty configured roots must keep using the coder-only fallback" ); } + #[test] + fn explicit_host_roots_build_a_jailed_coder_resolver() { + // allow_unjailed only takes effect when host_roots is EMPTY: an + // operator who pins roots keeps the jail even with the opt-in set. + let mut c = ShellConfig::seed_default(); + c.fs.host_roots = vec![std::path::PathBuf::from("/tmp")]; + let cells = build_code_cells(&c).expect("pinned roots build"); + assert!( + !cells.resolver.blocking_read().unjailed(), + "explicit host_roots must keep the coder resolver jailed" + ); + } + #[test] fn prepare_config_accepts_pinned_host_roots() { let mut c = ShellConfig::default(); @@ -837,8 +857,10 @@ mod tests { ); assert_eq!(cells.config.read().await.base_paths, vec![dir_b]); - // Clearing host_roots is an explicit shell::fs::* widening, but coder - // remains jailed to its own cwd + /tmp fallback and stays available. + // Reloading to the seed (allow_unjailed + empty host_roots) widens + // coder to the same deny-only policy as shell::fs::*: the cwd + /tmp + // fallback stays as relative-path anchors and the resolver reports + // unjailed. let unjailed = ShellConfig::seed_default(); let res = reload_serialized(&state, { let unjailed = unjailed.clone(); @@ -855,6 +877,10 @@ mod tests { std::fs::canonicalize("/tmp").expect("/tmp exists"), ] ); + assert!( + after.unjailed(), + "seed reload must widen coder to deny-only" + ); assert!(cells.config.read().await.base_paths.is_empty()); } diff --git a/shell/src/exec/policy.rs b/shell/src/exec/policy.rs index 2e7423ec1..19c1cc950 100644 --- a/shell/src/exec/policy.rs +++ b/shell/src/exec/policy.rs @@ -198,9 +198,16 @@ fn confine_scope_root( .map_err(|e| ExecError::new("S210", format!("{scope_root}: {e}")))?; for deny in denylist_canon { if canon.starts_with(deny) { + // REDACTION INVARIANT: a denylisted scope_root is reported with + // the same S211 wording as a missing one (see + // `fs::error::S211_REDACTED_SUFFIX`) so callers cannot probe + // for the existence of operator-denied directories. return Err(ExecError::new( - "S215", - format!("scope_root is denylisted: {scope_root}"), + "S211", + format!( + "scope_root {scope_root}: {}", + crate::fs::error::S211_REDACTED_SUFFIX + ), )); } } diff --git a/shell/src/fs/error.rs b/shell/src/fs/error.rs index c851da98b..9168a09d8 100644 --- a/shell/src/fs/error.rs +++ b/shell/src/fs/error.rs @@ -11,6 +11,15 @@ pub struct FsError { pub message: String, } +/// The one allowed S211 redaction suffix, mirroring the coder surface's +/// C211 REDACTION INVARIANT: "not found", "permission denied", +/// glob-protected, and operator-denylisted all fold into this single +/// wording so callers cannot probe for the existence of a protected path. +/// Shared with `exec`'s scope_root denylist rejection — keep the wordings +/// in lockstep. +pub const S211_REDACTED_SUFFIX: &str = + "not found or not accessible. Verify the path with shell::fs::ls."; + impl FsError { pub fn new(code: &'static str, message: impl Into) -> Self { Self { @@ -19,15 +28,27 @@ impl FsError { } } + /// The single S211 constructor (REDACTION INVARIANT — see + /// [`S211_REDACTED_SUFFIX`]): missing, permission-denied, + /// glob-protected, and denylisted paths all produce this exact shape. + pub fn not_found_or_denied(path: &str) -> Self { + Self::new("S211", format!("{path}: {S211_REDACTED_SUFFIX}")) + } + /// Mirrors the engine daemon's `SandboxError::from_io` so wire codes /// match across host and sandbox backends. pub fn from_io(path: &str, err: io::Error) -> Self { match err.kind() { - io::ErrorKind::NotFound => Self::new("S211", format!("path not found: {path}")), + // NotFound and PermissionDenied fold into the redacted S211 — + // raw OS text ("Permission denied (os error 13)") would let a + // caller distinguish "missing" from "denied" and probe for + // protected files. + io::ErrorKind::NotFound | io::ErrorKind::PermissionDenied => { + Self::not_found_or_denied(path) + } io::ErrorKind::AlreadyExists => { Self::new("S213", format!("path already exists: {path}")) } - io::ErrorKind::PermissionDenied => Self::new("S215", format!("{path}: {err}")), // S212 (wrong file type) and S214 (dir not empty) are checked // at the op level — they don't map to stable `ErrorKind` variants. _ => Self::new("S216", format!("{path}: {err}")), @@ -84,8 +105,15 @@ mod tests { assert_eq!(e.code, "S211"); let e = FsError::from_io("/x", io::Error::new(ErrorKind::AlreadyExists, "dupe")); assert_eq!(e.code, "S213"); + // REDACTION INVARIANT: PermissionDenied folds into S211 and must + // not leak raw OS text distinguishing it from "missing". let e = FsError::from_io("/x", io::Error::new(ErrorKind::PermissionDenied, "no")); - assert_eq!(e.code, "S215"); + assert_eq!(e.code, "S211"); + assert!( + e.message.contains("not found or not accessible"), + "{}", + e.message + ); let e = FsError::from_io("/x", io::Error::other("io")); assert_eq!(e.code, "S216"); } diff --git a/shell/src/fs/host.rs b/shell/src/fs/host.rs index 9ad4870d7..e1e0cad5c 100644 --- a/shell/src/fs/host.rs +++ b/shell/src/fs/host.rs @@ -259,10 +259,9 @@ impl HostFsBackend { pub(crate) fn validate_path(&self, path: &str) -> Result { let canon = confine_path(path, &self.host_roots_canon, &self.denylist_canon)?; if self.is_non_accessible(&canon) { - return Err(FsError::new( - "S215", - format!("path is protected (non_accessible): {path}"), - )); + // REDACTION INVARIANT: protected paths fold into the same S211 as + // missing ones so callers cannot probe for their existence. + return Err(FsError::not_found_or_denied(path)); } Ok(canon) } @@ -314,10 +313,9 @@ impl HostFsBackend { }; let access_scope = restrict_to_workspace.then_some(scope_root_canon).flatten(); if self.is_non_accessible_scoped(&canon, access_scope, scope_grants_canon) { - return Err(FsError::new( - "S215", - format!("path is protected (non_accessible): {path}"), - )); + // REDACTION INVARIANT: protected paths fold into the same S211 as + // missing ones so callers cannot probe for their existence. + return Err(FsError::not_found_or_denied(path)); } Ok(canon) } @@ -433,7 +431,9 @@ pub(crate) fn confine_path( } for deny_canon in denylist_canon { if canon.starts_with(deny_canon) { - return Err(FsError::new("S215", format!("path is denylisted: {path}"))); + // Operator denylist folds into the redacted S211 too — approval + // cannot override an explicit deny, so no jail-escape hint. + return Err(FsError::not_found_or_denied(path)); } } Ok(canon) @@ -458,14 +458,26 @@ fn path_is_non_accessible( if non_accessible.is_empty() { return false; } + let mut contained = false; for root in host_roots_canon { if let Ok(rel) = canon.strip_prefix(root) { + contained = true; let rel = rel.to_string_lossy().replace('\\', "/"); if !rel.is_empty() && non_accessible.is_match(&rel) { return true; } } } + // No containing root — the unjailed mode (empty roots, no scope). The + // globs must still protect secrets there: match against the + // root-stripped absolute form, the same fallback the coder resolver + // uses. Contained paths never reach this, so jailed matching is + // byte-for-byte unchanged. + if !contained { + let abs = canon.to_string_lossy().replace('\\', "/"); + let stripped = abs.trim_start_matches('/'); + return !stripped.is_empty() && non_accessible.is_match(stripped); + } false } @@ -563,10 +575,9 @@ fn confine_scope_root( } for deny_canon in denylist_canon { if canon.starts_with(deny_canon) { - return Err(FsError::new( - "S215", - format!("scope_root is denylisted: {scope_root}"), - )); + // REDACTION INVARIANT: denylisted folds into the same S211 as + // missing, so callers cannot probe operator-denied directories. + return Err(FsError::not_found_or_denied(scope_root)); } } Ok(Some(canon)) @@ -1675,10 +1686,7 @@ impl FsBackend for HostFsBackend { // D4: a protected file is locked for modification, exactly like // shell::fs::write/rm (which route through validate_path_scoped). if path_is_non_accessible(&canon, &access_roots, &non_accessible) { - return Err(FsError::new( - "S215", - format!("path is protected (non_accessible): {f}"), - )); + return Err(FsError::not_found_or_denied(f)); } } @@ -2181,8 +2189,14 @@ mod tests { }); let backend = HostFsBackend::try_new(cfg, Arc::new(StubChan)).unwrap(); let err = backend.validate_path(".env").unwrap_err(); - assert_eq!(err.code, "S215"); - assert!(err.message.contains("protected"), "got: {}", err.message); + assert_eq!(err.code, "S211"); + // REDACTION INVARIANT: the message must NOT say "protected" — a + // denied path reads exactly like a missing one. + assert!( + err.message.contains("not found or not accessible"), + "got: {}", + err.message + ); assert!(backend.validate_path("ok.txt").is_ok()); } @@ -2493,8 +2507,34 @@ mod tests { assert!(!path_is_non_accessible(&ok, &roots, &gs)); } + #[test] + fn path_is_non_accessible_protects_unjailed_paths_without_roots() { + // The unjailed gap (MOT-4099 audit): with empty host_roots the glob + // loop never ran and secrets globs were silently skipped. The + // absolute-form fallback must protect them; a non-matching neighbor + // stays accessible. + let dir = tmp(); + let secret = std::fs::canonicalize(&dir).unwrap().join(".env"); + let plain = std::fs::canonicalize(&dir).unwrap().join("notes.txt"); + let mut bld = GlobSetBuilder::new(); + bld.add(Glob::new("**/.env").unwrap()); + let gs = bld.build().unwrap(); + + assert!( + path_is_non_accessible(&secret, &[], &gs), + "empty roots must fall back to absolute-form matching" + ); + assert!(!path_is_non_accessible(&plain, &[], &gs)); + + // A contained path keeps root-relative-only matching: a glob that + // would only match the absolute form must NOT fire inside a root. + let roots = vec![std::fs::canonicalize(&dir).unwrap()]; + assert!(path_is_non_accessible(&secret, &roots, &gs)); + assert!(!path_is_non_accessible(&plain, &roots, &gs)); + } + #[tokio::test] - async fn sed_rejects_non_accessible_file_s215() { + async fn sed_rejects_non_accessible_file_with_redacted_s211() { // D4: sed confines in a spawn_blocking closure (not validate_path_scoped) // — it must still hard-reject a protected file, exactly like write/rm. let root = tmp(); @@ -2521,7 +2561,7 @@ mod tests { }) .await .unwrap_err(); - assert_eq!(err.code, "S215"); + assert_eq!(err.code, "S211"); // The protected file is untouched. assert_eq!( fs::read_to_string(root.join(".env")).unwrap(), @@ -2636,7 +2676,7 @@ mod tests { let err = h .validate_path(root.join("etc/shadow").to_str().unwrap()) .unwrap_err(); - assert_eq!(err.code, "S215"); + assert_eq!(err.code, "S211"); } #[test] @@ -3121,7 +3161,7 @@ mod tests { } #[tokio::test] - async fn chmod_with_uid_unprivileged_returns_s215() { + async fn chmod_with_uid_unprivileged_returns_redacted_s211() { let root = tmp(); let f = root.join("c.txt"); std::fs::write(&f, b"x").unwrap(); @@ -3139,7 +3179,7 @@ mod tests { match resp { Ok(_) => {} - Err(e) => assert_eq!(e.code, "S215"), + Err(e) => assert_eq!(e.code, "S211"), } } @@ -4237,7 +4277,7 @@ mod tests { }) .await .expect_err("protected file under selected scope_root must stay locked"); - assert_eq!(err.code, "S215"); + assert_eq!(err.code, "S211"); } #[tokio::test] diff --git a/shell/src/functions/workspace.rs b/shell/src/functions/workspace.rs index af9ea698d..38678ff93 100644 --- a/shell/src/functions/workspace.rs +++ b/shell/src/functions/workspace.rs @@ -135,7 +135,9 @@ fn canonical_workspace_dir(path: &str, cfg: &ShellConfig) -> Result Result<()> { ); } - // coder::* remains jailed even when shell::fs::* is explicitly unjailed: - // an empty fs.host_roots uses the code resolver's narrow cwd + /tmp - // fallback. Building the cells unconditionally keeps the zero-config - // worker useful while preserving the coder path boundary. + // coder::* follows the same policy switch as shell::fs::*: with the + // explicit unjailed opt-in (fs.allow_unjailed + empty fs.host_roots) the + // resolver runs deny-only against the real filesystem, keeping the narrow + // cwd + /tmp fallback roots as relative-path anchors; without the opt-in + // an empty fs.host_roots stays jailed to that fallback. let code_cells = configuration::build_code_cells(&cfg) .map_err(anyhow::Error::msg) .context("building initial code surface state (coder::*)")?; @@ -237,8 +238,9 @@ async fn main() -> Result<()> { // Code surface (folded coder::*): explicit fs.host_roots are shared with // shell::fs::*; when they are empty, PathResolver supplies its own narrow - // cwd + /tmp defaults. In both cases the resolver is a jail. A bad glob or - // unreachable root aborts startup so the worker never half-boots. + // cwd + /tmp defaults — as the jail without the unjailed opt-in, as + // relative-path anchors with it. A bad glob or unreachable root aborts + // startup so the worker never half-boots. code::register_all(&iii, code_cells); tracing::info!("code surface (coder::*) registered"); @@ -554,36 +556,41 @@ fn register_fs(iii: &iii_sdk::IIIClient, state: &AppState) { "List directory contents. `path` is relative to the primary fs jail root (the first \ fs.host_roots entry) when set, otherwise absolute. `target` defaults to host; pass \ { kind: \"sandbox\", sandbox_id } \ - to run in a microVM. Errors return { code, message }; common: S210 bad path, S211 not found, \ - S212 not a directory, S215 jail/denylist."); + to run in a microVM. Errors return { code, message }; common: S210 bad path, S211 not found \ + or not accessible, S212 not a directory, S215 jail escape. For paginated or recursive \ + listings with noise filtering, prefer coder::list-folder / coder::tree."); fs_fn!("shell::fs::stat", fs_stat, fs::StatRequest, fs::StatResponse, "Stat a single path (jail-relative when fs.host_roots are set). Returns the entry's type, size, \ - mode, and mtime. Errors return { code, message }; common: S211 not found, S215 jail/denylist."); + mode, and mtime. Errors return { code, message }; common: S211 not found or not accessible, \ + S215 jail escape. coder::read-file with stat: true additionally reports total_lines."); fs_fn!("shell::fs::mkdir", fs_mkdir, fs::MkdirRequest, fs::MkdirResponse, "Create a directory. `mode` is an octal string like \"0755\". `parents: true` creates missing \ parents and is idempotent. Returns { created, path, already_existed }. Errors return \ - { code, message }; common: S210 bad mode, S213 exists, S215 jail/denylist."); + { code, message }; common: S210 bad mode, S213 exists, S215 jail escape."); fs_fn!( "shell::fs::rm", fs_rm, fs::RmRequest, fs::RmResponse, "Remove a path. `recursive: true` is required to delete a non-empty directory. Returns \ - { removed, path, was_present }. Errors return { code, message }; common: S211 not found, \ - S214 dir not empty (pass recursive), S215 jail/denylist." + { removed, path, was_present }. Errors return { code, message }; common: S211 not found or \ + not accessible, S214 dir not empty (pass recursive), S215 jail escape. To remove several \ + paths in one call, use coder::delete-file (batched, per-entry errors)." ); fs_fn!("shell::fs::chmod", fs_chmod, fs::ChmodRequest, fs::ChmodResponse, "Change permissions. `mode` is an octal string like \"0644\". `uid`/`gid` optionally chown. \ `recursive: true` walks the tree (symlinks skipped). Returns { entries_changed, path, recursive }. \ - Errors return { code, message }; common: S210 bad mode, S211 not found, S215 jail/denylist."); + Errors return { code, message }; common: S210 bad mode, S211 not found or not accessible, \ + S215 jail escape."); fs_fn!( "shell::fs::mv", fs_mv, fs::MvRequest, fs::MvResponse, "Move/rename a path. `overwrite: true` allows replacing an existing dst. Returns \ - { moved, src, dst, overwrote }. Errors return { code, message }; common: S211 src not found, \ - S213 dst exists, S215 jail/denylist." + { moved, src, dst, overwrote }. Errors return { code, message }; common: S211 src not found \ + or not accessible, S213 dst exists, S215 jail escape. To move several paths in one call, \ + use coder::move (batched, per-entry errors)." ); fs_fn!( "shell::fs::grep", @@ -592,13 +599,15 @@ fn register_fs(iii: &iii_sdk::IIIClient, state: &AppState) { fs::GrepResponse, "Search file contents. `pattern` is a Rust regex (RE2-like). `recursive` defaults true. \ `include_glob`/`exclude_glob` filter paths. Returns { matches, truncated }. Errors return \ - { code, message }; common: S217 bad regex, S215 jail/denylist." + { code, message }; common: S217 bad regex, S215 jail escape. For token-budgeted search with \ + context lines and noise filtering, prefer coder::search." ); fs_fn!("shell::fs::sed", fs_sed, fs::SedRequest, fs::SedResponse, "Find-and-replace across files. `pattern` is a Rust regex by default (set regex:false for a \ literal). Provide either `files` (explicit list) or `path` (+ recursive). Returns \ { results, total_replacements }. Errors return { code, message }; common: S217 bad regex, \ - S211 not found, S215 jail/denylist."); + S211 not found or not accessible, S215 jail escape. For line-oriented edits with post-apply \ + echoes (no re-read needed), prefer coder::update-file."); fs_fn!( "shell::fs::write", fs_write, @@ -612,13 +621,15 @@ fn register_fs(iii: &iii_sdk::IIIClient, state: &AppState) { content, mode?, parents? }, ...]` instead of the single-file fields (host target, inline \ content) — the response then carries per-file results in `files`. `mode` is octal \ (default \"0644\"); `parents: true` creates missing parents. Errors return { code, message }; \ - common: S210 bad mode/payload or inline-on-sandbox, S215 jail/denylist, S218 payload exceeds \ - max_write_bytes, S216 channel/IO error." + common: S210 bad mode/payload or inline-on-sandbox, S211 not accessible, S215 jail escape, \ + S218 payload exceeds max_write_bytes, S216 channel/IO error. For plain text files, \ + coder::create-file (batched) avoids the streaming channel." ); fs_fn!("shell::fs::read", fs_read, fs::ReadRequest, fs::ReadResponseWire, "Stream a file from a path. Returns a ContentRef the caller reads from, plus size/mode/mtime. \ - Errors return { code, message }; common: S211 not found, S212 path is a directory, S215 \ - jail/denylist, S218 file exceeds max_read_bytes, S216 channel/IO error."); + Errors return { code, message }; common: S211 not found or not accessible, S212 path is a \ + directory, S215 jail escape, S218 file exceeds max_read_bytes, S216 channel/IO error. For \ + text files, coder::read-file returns content inline (windowed, batched) with no channel."); } /// Wait for SIGINT or, on Unix, SIGTERM so `docker stop` / `kubectl delete` diff --git a/shell/tests/code_golden_errors.rs b/shell/tests/code_golden_errors.rs index e0cc21312..475dab2ba 100644 --- a/shell/tests/code_golden_errors.rs +++ b/shell/tests/code_golden_errors.rs @@ -16,15 +16,15 @@ //! - C211: missing path; glob-denied path (byte-identical suffix — //! REDACTION INVARIANT); recursive-delete subtree-blocked; move missing //! source -//! - C213: read cap; write cap; batch budget exhausted; full-read output +//! - C218: read cap; write cap; batch budget exhausted; full-read output //! budget (the recovery-tool message: size + total_lines + corrective //! calls) //! - C215: relative `..` escape; absolute outside all roots; dangling //! symlink //! - C216: io passthrough (via the public `From` conversion — //! see the case comment for why no handler drives this one) -//! - C217: create-file exists without overwrite; move destination exists -//! without overwrite (one house C217 shape) +//! - C213: create-file exists without overwrite; move destination exists +//! without overwrite (one house C213 shape) //! //! Regenerate with `UPDATE_GOLDENS=1 cargo test`. @@ -280,7 +280,7 @@ async fn error_message_formats_match_golden() { ); } - // --- C213: read cap and write cap ----------------------------------- + // --- C218: read cap and write cap ----------------------------------- { let tiny = Arc::new(CoderConfig { base_paths: vec![jail.root0.clone(), jail.root1.clone()], @@ -300,7 +300,7 @@ async fn error_message_formats_match_golden() { ) .await .expect_err("over-cap read must fail"); - put("C213_read_cap_exceeded", parse_wire_string(&err), &jail); + put("C218_read_cap_exceeded", parse_wire_string(&err), &jail); let got = create_err( &jail, @@ -314,10 +314,10 @@ async fn error_message_formats_match_golden() { }, ) .await; - put("C213_write_cap_exceeded", got, &jail); + put("C218_write_cap_exceeded", got, &jail); } - // --- C213: batch budget exhausted ------------------------------------- + // --- C218: batch budget exhausted ------------------------------------- { let tiny_budget = Arc::new(CoderConfig { base_paths: vec![jail.root0.clone(), jail.root1.clone()], @@ -349,13 +349,13 @@ async fn error_message_formats_match_golden() { ); let wire = results[1].error.as_ref().expect("second entry must fail"); put( - "C213_batch_budget_exhausted", + "C218_batch_budget_exhausted", (wire.code.clone(), wire.message.clone()), &jail, ); } - // --- C213: full-read output budget (the recovery-tool message) ------- + // --- C218: full-read output budget (the recovery-tool message) ------- { let tiny_output = Arc::new(CoderConfig { base_paths: vec![jail.root0.clone(), jail.root1.clone()], @@ -375,7 +375,7 @@ async fn error_message_formats_match_golden() { .await .expect_err("over-output full read must fail"); put( - "C213_full_read_output_budget_exceeded", + "C218_full_read_output_budget_exceeded", parse_wire_string(&err), &jail, ); @@ -414,7 +414,7 @@ async fn error_message_formats_match_golden() { put("C216_io_passthrough", from_coder_error(&e), &jail); } - // --- C217: create-file exists without overwrite --------------------- + // --- C213: create-file exists without overwrite --------------------- { std::fs::write(jail.root0.join("exists.txt"), "old").unwrap(); let got = create_err( @@ -429,10 +429,10 @@ async fn error_message_formats_match_golden() { }, ) .await; - put("C217_create_exists_without_overwrite", got, &jail); + put("C213_create_exists_without_overwrite", got, &jail); } - // --- C217: move destination exists without overwrite ---------------- + // --- C213: move destination exists without overwrite ---------------- { std::fs::write(jail.root0.join("move-src.txt"), "src").unwrap(); std::fs::write(jail.root0.join("move-dst.txt"), "dst").unwrap(); @@ -455,7 +455,7 @@ async fn error_message_formats_match_golden() { .as_ref() .expect("entry must carry an error"); put( - "C217_move_dst_exists_without_overwrite", + "C213_move_dst_exists_without_overwrite", (wire.code.clone(), wire.message.clone()), &jail, ); diff --git a/shell/tests/code_path_jail.rs b/shell/tests/code_path_jail.rs index 4d56711f4..6514130b6 100644 --- a/shell/tests/code_path_jail.rs +++ b/shell/tests/code_path_jail.rs @@ -160,7 +160,7 @@ async fn scoped_scope_root_blocks_sibling_escape_across_handlers() { let scope_root = project.to_string_lossy().into_owned(); // `../sibling.txt` is still inside the worker's allowed root, so a plain - // jail check is not enough. Session-scoped calls must reject it with C218. + // jail check is not enough. Session-scoped calls must reject it with C220. let read_err = read_handle( r.clone(), c.clone(), @@ -176,7 +176,7 @@ async fn scoped_scope_root_blocks_sibling_escape_across_handlers() { ) .await .expect_err("scoped read must not escape the session dir"); - assert_eq!(wire_err_code(&read_err), "C218"); + assert_eq!(wire_err_code(&read_err), "C220"); let create_err = create_handle( r.clone(), @@ -198,7 +198,7 @@ async fn scoped_scope_root_blocks_sibling_escape_across_handlers() { ) .await .expect_err("scoped create must not escape the session dir"); - assert_eq!(wire_err_code(&create_err), "C218"); + assert_eq!(wire_err_code(&create_err), "C220"); assert!( !tmp.path().join("created-outside.txt").exists(), "rejected create must not write into a sibling of the session dir" @@ -218,7 +218,7 @@ async fn scoped_scope_root_blocks_sibling_escape_across_handlers() { ) .await .expect_err("scoped delete must not escape the session dir"); - assert_eq!(wire_err_code(&delete_err), "C218"); + assert_eq!(wire_err_code(&delete_err), "C220"); assert!( tmp.path().join("sibling.txt").exists(), "rejected delete must leave sibling files untouched" diff --git a/shell/tests/code_unified_protection.rs b/shell/tests/code_unified_protection.rs index 5ca590fb5..b9d70f1f7 100644 --- a/shell/tests/code_unified_protection.rs +++ b/shell/tests/code_unified_protection.rs @@ -3,7 +3,7 @@ //! After the coder→shell fold, secrets are declared ONCE under //! `code.non_accessible_globs`, and the merge's promise is that the SAME //! globs guard BOTH surfaces: the `coder::*` code surface (C211 show-but-lock) -//! AND the `shell::fs::*` host surface (S215 hard-reject). +//! AND the `shell::fs::*` host surface (S211 hard-reject, redacted like C211). //! //! `configuration::build_runtime` is the single place that copies //! `code.non_accessible_globs` onto the host fs backend @@ -13,7 +13,7 @@ //! would silently disable fs secret protection while every other test stayed //! green. These tests close that hole end-to-end: a glob declared ONLY under //! `code` must cause the host fs backend that `build_runtime` produces to -//! protect a matching in-jail path with S215. +//! protect a matching in-jail path with the redacted S211. use shell::code::config::CoderConfig; use shell::config::{FsConfig, ShellConfig}; @@ -43,7 +43,8 @@ fn runtime_for( } /// A glob declared ONLY under `code.non_accessible_globs` must reject a -/// matching `shell::fs::read` with S215. The protected read returns S215 in +/// matching `shell::fs::read` with the redacted S211 (REDACTION INVARIANT: +/// indistinguishable from missing). The protected read rejects in /// `validate_path` (before the streaming channel), so this stays fast and /// deterministic even with a stub engine. #[tokio::test] @@ -62,15 +63,19 @@ async fn code_non_accessible_globs_block_the_fs_read() { .await .expect_err("a glob declared under code.non_accessible_globs must block the fs read too"); assert_eq!( - err.code, "S215", - "the fs surface must reject the code-declared protected path with S215; got {err:?}" + err.code, "S211", + "the fs surface must reject the code-declared protected path with the redacted S211; got {err:?}" + ); + assert!( + err.message.contains("not found or not accessible"), + "redacted wording required; got {err:?}" ); } /// Causality control: with NO globs under `code`, the SAME in-jail path is no /// longer protected on the fs surface. `stat` exercises the identical /// `validate_path` gate as `read` but creates no channel, so the positive -/// case is fast and deterministic — proving the S215 above is caused +/// case is fast and deterministic — proving the rejection above is caused /// specifically by the propagated glob, not by some unrelated rejection. #[tokio::test] async fn fs_surface_not_protected_without_code_glob() { @@ -88,7 +93,7 @@ async fn fs_surface_not_protected_without_code_glob() { }) .await .expect_err("stat must hit the same protection gate as read"); - assert_eq!(err.code, "S215", "got {err:?}"); + assert_eq!(err.code, "S211", "got {err:?}"); // …and without it, the very same stat succeeds. let open = runtime_for(tmp.path(), vec![]); diff --git a/shell/tests/e2e/workers/harness/src/cases-fs-host-jail.ts b/shell/tests/e2e/workers/harness/src/cases-fs-host-jail.ts index 9dae7a7ed..e85585930 100644 --- a/shell/tests/e2e/workers/harness/src/cases-fs-host-jail.ts +++ b/shell/tests/e2e/workers/harness/src/cases-fs-host-jail.ts @@ -11,20 +11,22 @@ export const FS_HOST_JAIL_CASES: TestCase[] = [ }, }, { - name: 'fs_host_denylist_etc_passwd_rejected_s215', + // 0.10.0 redaction: denylisted folds into S211, indistinguishable from + // a missing path (S215 is exclusively a jail-confinement escape now). + name: 'fs_host_denylist_etc_passwd_rejected_s211_redacted', async run(ctx: CaseContext) { await ctx.expectError( () => ctx.call('shell::fs::stat', { path: '/etc/passwd' }), - 'S215', + 'S211', ); }, }, { - name: 'fs_host_denylist_etc_shadow_rejected_s215', + name: 'fs_host_denylist_etc_shadow_rejected_s211_redacted', async run(ctx: CaseContext) { await ctx.expectError( () => ctx.call('shell::fs::read', { path: '/etc/shadow' }), - 'S215', + 'S211', ); }, }, diff --git a/shell/tests/e2e/workers/harness/src/cases-fs-protocol-break.ts b/shell/tests/e2e/workers/harness/src/cases-fs-protocol-break.ts index 2991bffac..a27325ff3 100644 Binary files a/shell/tests/e2e/workers/harness/src/cases-fs-protocol-break.ts and b/shell/tests/e2e/workers/harness/src/cases-fs-protocol-break.ts differ diff --git a/shell/tests/features/coder/create_file.feature b/shell/tests/features/coder/create_file.feature index 976df55eb..0b2065f71 100644 --- a/shell/tests/features/coder/create_file.feature +++ b/shell/tests/features/coder/create_file.feature @@ -22,7 +22,7 @@ Feature: coder create-file adversarial behavior """ {"files":[{"path":"same.txt","content":"new","overwrite":false}]} """ - Then the result for "same.txt" failed with code "C217" + Then the result for "same.txt" failed with code "C213" And the file "same.txt" equals: """ old diff --git a/shell/tests/features/coder/move.feature b/shell/tests/features/coder/move.feature index 1d8494d46..5a093b118 100644 --- a/shell/tests/features/coder/move.feature +++ b/shell/tests/features/coder/move.feature @@ -29,7 +29,7 @@ Feature: coder move adversarial behavior """ {"files":[{"from":"from.txt","to":"to.txt","overwrite":false}]} """ - Then the move from "from.txt" to "to.txt" failed with code "C217" + Then the move from "from.txt" to "to.txt" failed with code "C213" And the file "from.txt" exists And the file "to.txt" equals: """ diff --git a/shell/tests/features/coder/path_security.feature b/shell/tests/features/coder/path_security.feature index f08c261ca..b21d5112b 100644 --- a/shell/tests/features/coder/path_security.feature +++ b/shell/tests/features/coder/path_security.feature @@ -52,4 +52,4 @@ Feature: coder path security """ {"path":"../outside-session.txt","fs_scope":{"root":"{{session}}","boundary":"workspace"}} """ - Then the call failed with code "C218" + Then the call failed with code "C220" diff --git a/shell/tests/features/coder/read_file.feature b/shell/tests/features/coder/read_file.feature index d10c92135..7badc5091 100644 --- a/shell/tests/features/coder/read_file.feature +++ b/shell/tests/features/coder/read_file.feature @@ -55,7 +55,7 @@ Feature: coder read-file adversarial behavior {"paths":["batch/a.txt","batch/b.txt"]} """ Then the batch read result for "batch/a.txt" succeeded - And the batch read result for "batch/b.txt" failed with code "C213" + And the batch read result for "batch/b.txt" failed with code "C218" Scenario: path and paths are mutually exclusive Given a jailed code surface diff --git a/shell/tests/golden/errors.json b/shell/tests/golden/errors.json index 4e658bc11..d531b9959 100644 --- a/shell/tests/golden/errors.json +++ b/shell/tests/golden/errors.json @@ -35,21 +35,13 @@ "code": "C211", "message": "missing.txt: not found or not accessible. Verify the path with coder::list-folder or coder::tree." }, - "C213_batch_budget_exhausted": { + "C213_create_exists_without_overwrite": { "code": "C213", - "message": "batch budget exhausted before reaching budget-b.txt: batch_read_budget_bytes is 5 and earlier entries already returned 5 bytes of content (after UTF-8 sanitization). To recover: request fewer or smaller entries, use per-entry line_from/line_to windows, or raise batch_read_budget_bytes in coder config." - }, - "C213_full_read_output_budget_exceeded": { - "code": "C213", - "message": "over-output.txt: a full read would return 9 bytes of content (file is 9 bytes, 3 lines), which exceeds max_output_bytes (8). To recover: read a slice with line_from/line_to, probe metadata cheaply with stat: true, or re-call with a higher per-call max_output_bytes (values above max_read_bytes are clamped)." - }, - "C213_read_cap_exceeded": { - "code": "C213", - "message": "big.txt is 16 bytes, which exceeds max_read_bytes (8). Read a smaller file, raise max_read_bytes in coder config, or read a slice with line_from/line_to." + "message": "exists.txt already exists; pass overwrite=true to replace" }, - "C213_write_cap_exceeded": { + "C213_move_dst_exists_without_overwrite": { "code": "C213", - "message": "big-create.txt is 16 bytes, which exceeds max_write_bytes (8). Split the content into smaller files or raise max_write_bytes in coder config." + "message": "move-dst.txt already exists; pass overwrite=true to replace" }, "C215_absolute_outside_all_roots": { "code": "C215", @@ -67,12 +59,20 @@ "code": "C216", "message": "synthetic io failure" }, - "C217_create_exists_without_overwrite": { - "code": "C217", - "message": "exists.txt already exists; pass overwrite=true to replace" + "C218_batch_budget_exhausted": { + "code": "C218", + "message": "batch budget exhausted before reaching budget-b.txt: batch_read_budget_bytes is 5 and earlier entries already returned 5 bytes of content (after UTF-8 sanitization). To recover: request fewer or smaller entries, use per-entry line_from/line_to windows, or raise batch_read_budget_bytes in coder config." }, - "C217_move_dst_exists_without_overwrite": { - "code": "C217", - "message": "move-dst.txt already exists; pass overwrite=true to replace" + "C218_full_read_output_budget_exceeded": { + "code": "C218", + "message": "over-output.txt: a full read would return 9 bytes of content (file is 9 bytes, 3 lines), which exceeds max_output_bytes (8). To recover: read a slice with line_from/line_to, probe metadata cheaply with stat: true, or re-call with a higher per-call max_output_bytes (values above max_read_bytes are clamped)." + }, + "C218_read_cap_exceeded": { + "code": "C218", + "message": "big.txt is 16 bytes, which exceeds max_read_bytes (8). Read a smaller file, raise max_read_bytes in coder config, or read a slice with line_from/line_to." + }, + "C218_write_cap_exceeded": { + "code": "C218", + "message": "big-create.txt is 16 bytes, which exceeds max_write_bytes (8). Split the content into smaller files or raise max_write_bytes in coder config." } } diff --git a/shell/tests/golden/schemas/coder.create-file.json b/shell/tests/golden/schemas/coder.create-file.json index f99a4ea78..b513450c2 100644 --- a/shell/tests/golden/schemas/coder.create-file.json +++ b/shell/tests/golden/schemas/coder.create-file.json @@ -85,7 +85,7 @@ "type": "null" } ], - "description": "Structured error for this entry. `code` is stable for programmatic branching (e.g. `\"C217\"` means already-exists; pass `overwrite=true` to replace). `message` carries the corrective action an LLM agent needs to make a successful second call." + "description": "Structured error for this entry. `code` is stable for programmatic branching (e.g. `\"C213\"` means already-exists; pass `overwrite=true` to replace). `message` carries the corrective action an LLM agent needs to make a successful second call." }, "path": { "description": "Canonical absolute path (resolved through the jail); the caller's input verbatim when resolution failed.", diff --git a/shell/tests/golden/schemas/coder.info.json b/shell/tests/golden/schemas/coder.info.json index 9c0ebc610..a541fc2da 100644 --- a/shell/tests/golden/schemas/coder.info.json +++ b/shell/tests/golden/schemas/coder.info.json @@ -1,5 +1,5 @@ { - "description": "Report the coder jail: canonical allowed roots (primary first), per-file size caps, response budgets (max_output_bytes, batch_read_budget_bytes, search_response_budget_bytes), listing/search limits, the non-accessible glob patterns, and the default_exclude_globs noise filter applied by tree/search. Call this FIRST when unsure where coder may read or write, or when a path was rejected — paths outside every allowed root need the shell worker's shell::fs::* instead.", + "description": "Report the coder access contract: the effective mode (jailed = paths confined to the allowed roots; unjailed = deny-only, absolute paths anywhere on the host, roots anchor relative paths only), canonical allowed roots (primary first), per-file size caps, response budgets (max_output_bytes, batch_read_budget_bytes, search_response_budget_bytes), listing/search limits, the non-accessible glob patterns, and the default_exclude_globs noise filter applied by tree/search. Call this FIRST when unsure where coder may read or write, or when a path was rejected — in jailed mode, paths outside every allowed root need the shell worker's shell::fs::* instead.", "function_id": "coder::info", "request_schema": { "$schema": "http://json-schema.org/draft-07/schema#", @@ -12,16 +12,37 @@ }, "response_schema": { "$schema": "http://json-schema.org/draft-07/schema#", + "definitions": { + "AccessMode": { + "description": "Effective access mode of the coder surface — the same deny-only policy switch `shell::fs::*` runs under.", + "oneOf": [ + { + "description": "Paths are confined to `base_paths`; anything outside is rejected.", + "enum": [ + "jailed" + ], + "type": "string" + }, + { + "description": "Operator opt-in (`fs.allow_unjailed: true`, empty `fs.host_roots`): absolute paths anywhere on the host are accepted, confined only by `fs.denylist_paths` and `non_accessible_globs`. `base_paths` only anchor relative wire paths.", + "enum": [ + "unjailed" + ], + "type": "string" + } + ] + } + }, "properties": { "base_paths": { - "description": "Canonical absolute paths of the allowed roots, in configuration order. The primary root (index 0) is where relative wire paths resolve; an absolute path is accepted when it canonicalises inside ANY of these. Paths outside every root are rejected — use `shell::fs::*` instead.", + "description": "Canonical absolute paths of the allowed roots, in configuration order. The primary root (index 0) is where relative wire paths resolve; an absolute path is accepted when it canonicalises inside ANY of these (in `unjailed` mode: anywhere on the host). In `jailed` mode paths outside every root are rejected — use `shell::fs::*` instead.", "items": { "type": "string" }, "type": "array" }, "batch_read_budget_bytes": { - "description": "Aggregate budget across a single `paths[]` batch call to `coder::read-file`, measured in bytes of returned content (after UTF-8 sanitization — invalid bytes expand to U+FFFD before being counted, so the cap bounds what the caller actually receives). Entries are collected in request order; each entry may consume up to `min(remaining_budget, max_read_bytes)`. An entry reached with zero remaining budget receives a per-entry C213 naming this key, its value, and the bytes already consumed, with recovery guidance. Budget topology: batch reads are governed by this key; single-path full reads by `max_output_bytes`; windowed reads by `max_read_bytes` applied per returned window — `max_read_bytes` is also the per-file IO ceiling for all of them.", + "description": "Aggregate budget across a single `paths[]` batch call to `coder::read-file`, measured in bytes of returned content (after UTF-8 sanitization — invalid bytes expand to U+FFFD before being counted, so the cap bounds what the caller actually receives). Entries are collected in request order; each entry may consume up to `min(remaining_budget, max_read_bytes)`. An entry reached with zero remaining budget receives a per-entry C218 naming this key, its value, and the bytes already consumed, with recovery guidance. Budget topology: batch reads are governed by this key; single-path full reads by `max_output_bytes`; windowed reads by `max_read_bytes` applied per returned window — `max_read_bytes` is also the per-file IO ceiling for all of them.", "format": "uint64", "minimum": 0.0, "type": "integer" @@ -46,23 +67,31 @@ "type": "integer" }, "max_output_bytes": { - "description": "Context budget for single-path FULL reads in `coder::read-file`, in bytes of returned content. Full reads larger than this return C213 with the file's size/line count and window/stat recovery guidance; a per-call `max_output_bytes` override is available on `coder::read-file` (clamped to `max_read_bytes`).", + "description": "Context budget for single-path FULL reads in `coder::read-file`, in bytes of returned content. Full reads larger than this return C218 with the file's size/line count and window/stat recovery guidance; a per-call `max_output_bytes` override is available on `coder::read-file` (clamped to `max_read_bytes`).", "format": "uint64", "minimum": 0.0, "type": "integer" }, "max_read_bytes": { - "description": "Per-file IO ceiling for `coder::read-file`. Full reads of files larger than this are rejected with C213; windowed reads cap the returned window bytes instead, so larger files stay readable window by window. Also the ceiling for `coder::search` content scanning — larger files are silently skipped during search.", + "description": "Per-file IO ceiling for `coder::read-file`. Full reads of files larger than this are rejected with C218; windowed reads cap the returned window bytes instead, so larger files stay readable window by window. Also the ceiling for `coder::search` content scanning — larger files are silently skipped during search.", "format": "uint64", "minimum": 0.0, "type": "integer" }, "max_write_bytes": { - "description": "Maximum bytes that `coder::create-file` or `coder::update-file` will accept for a single file write. Larger writes are rejected with C213.", + "description": "Maximum bytes that `coder::create-file` or `coder::update-file` will accept for a single file write. Larger writes are rejected with C218.", "format": "uint64", "minimum": 0.0, "type": "integer" }, + "mode": { + "allOf": [ + { + "$ref": "#/definitions/AccessMode" + } + ], + "description": "`jailed`: paths must stay inside `base_paths`. `unjailed`: absolute paths anywhere on the host are accepted (deny-only, like `shell::fs::*`) and `base_paths` only anchor relative paths." + }, "non_accessible_globs": { "description": "Glob patterns matched per root (root-relative). Files whose root-relative path matches are listable but not readable/writable/deletable/creatable; they return C211.", "items": { @@ -118,6 +147,7 @@ "max_output_bytes", "max_read_bytes", "max_write_bytes", + "mode", "non_accessible_globs", "primary_root", "search_default_max_line_bytes", diff --git a/shell/tests/golden/schemas/coder.move.json b/shell/tests/golden/schemas/coder.move.json index 48f352e87..85319b637 100644 --- a/shell/tests/golden/schemas/coder.move.json +++ b/shell/tests/golden/schemas/coder.move.json @@ -76,7 +76,7 @@ "type": "null" } ], - "description": "Structured error for this entry. `code` is stable for programmatic branching (e.g. `\"C217\"` means destination exists; pass `overwrite=true` to replace; `\"C210\"` for disallowed operations such as cross-root directory moves, moving a root itself, or a destination that is a directory — the message then names the corrected target path). `message` carries the corrective action an LLM agent needs to make a successful second call." + "description": "Structured error for this entry. `code` is stable for programmatic branching (e.g. `\"C213\"` means destination exists; pass `overwrite=true` to replace; `\"C210\"` for disallowed operations such as cross-root directory moves, moving a root itself, or a destination that is a directory — the message then names the corrected target path). `message` carries the corrective action an LLM agent needs to make a successful second call." }, "from": { "description": "Canonical absolute path of the source (resolved through the jail); the caller's input verbatim when resolution failed.", diff --git a/shell/tests/golden/schemas/coder.read-file.json b/shell/tests/golden/schemas/coder.read-file.json index a72ea4f7d..4c70df16e 100644 --- a/shell/tests/golden/schemas/coder.read-file.json +++ b/shell/tests/golden/schemas/coder.read-file.json @@ -1,5 +1,5 @@ { - "description": "Read a file window-first: probe with stat: true (size/mtime/mode plus total_lines, no content), then fetch just the lines you need with line_from/line_to (1-based, inclusive) — windows keep files larger than max_read_bytes readable window by window, with more_lines/total_lines reporting what remains. numbered: true prefixes each line with its absolute 1-based file line number, matching coder::update-file's line ops exactly. Full reads are budgeted by max_output_bytes (default 128 KiB; per-call override clamped to max_read_bytes) — an over-budget full read fails with a C213 carrying the file's size, line count, and the window/stat recovery calls. Batch mode: pass paths[] (XOR path) to read multiple files in one call — entries are processed in request order against batch_read_budget_bytes, measured in bytes of returned content (after UTF-8 sanitization); per-entry errors (C211/C213) leave other entries unaffected. Paths are relative to the primary allowed root or absolute inside any allowed root (coder::info lists them); for host paths outside the jail use shell::fs::*. Non-accessible paths return C211.", + "description": "Read a file window-first: probe with stat: true (size/mtime/mode plus total_lines, no content), then fetch just the lines you need with line_from/line_to (1-based, inclusive) — windows keep files larger than max_read_bytes readable window by window, with more_lines/total_lines reporting what remains. numbered: true prefixes each line with its absolute 1-based file line number, matching coder::update-file's line ops exactly. Full reads are budgeted by max_output_bytes (default 128 KiB; per-call override clamped to max_read_bytes) — an over-budget full read fails with a C218 carrying the file's size, line count, and the window/stat recovery calls. Batch mode: pass paths[] (XOR path) to read multiple files in one call — entries are processed in request order against batch_read_budget_bytes, measured in bytes of returned content (after UTF-8 sanitization); per-entry errors (C211/C218) leave other entries unaffected. Paths are relative to the primary allowed root or absolute inside any allowed root (coder::info lists them); for host paths outside the jail use shell::fs::*. Non-accessible paths return C211.", "function_id": "coder::read-file", "request_schema": { "$schema": "http://json-schema.org/draft-07/schema#", @@ -97,7 +97,7 @@ }, "max_output_bytes": { "default": null, - "description": "Per-call override of the `max_output_bytes` config (default 131072) that budgets single-path FULL reads, measured in returned content bytes after UTF-8 conversion (numbered prefixes included). Values above `max_read_bytes` are silently clamped to it. When the full content would exceed the effective budget the call fails with a C213 naming the file's size and `total_lines` — recover by windowing with `line_from`/`line_to`, probing with `stat: true`, or raising this field. Full reads only: combining it with `line_from`/`line_to` is C210 (windows are bounded by `max_read_bytes` instead); ignored when `paths` is set (batch mode is governed by `batch_read_budget_bytes`).", + "description": "Per-call override of the `max_output_bytes` config (default 131072) that budgets single-path FULL reads, measured in returned content bytes after UTF-8 conversion (numbered prefixes included). Values above `max_read_bytes` are silently clamped to it. When the full content would exceed the effective budget the call fails with a C218 naming the file's size and `total_lines` — recover by windowing with `line_from`/`line_to`, probing with `stat: true`, or raising this field. Full reads only: combining it with `line_from`/`line_to` is C210 (windows are bounded by `max_read_bytes` instead); ignored when `paths` is set (batch mode is governed by `batch_read_budget_bytes`).", "format": "uint64", "minimum": 0.0, "type": [