Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions approval-gate/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion approval-gate/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

[package]
name = "approval-gate"
version = "1.0.8"
version = "1.0.9"
edition = "2021"
publish = false

Expand Down
2 changes: 1 addition & 1 deletion approval-gate/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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=<json>` tail,
`S220`, `C215`, `C220`) whose message carries a `grant_hint=<json>` 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
Expand Down
2 changes: 1 addition & 1 deletion approval-gate/src/filesystem_access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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=";

Expand Down
2 changes: 1 addition & 1 deletion approval-gate/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
40 changes: 40 additions & 0 deletions shell/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion shell/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion shell/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

[package]
name = "shell"
version = "0.9.2"
version = "0.10.0"
edition = "2021"
publish = false

Expand Down
90 changes: 78 additions & 12 deletions shell/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` |

Comment on lines +189 to 198

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

S220 is referenced as C220's "fs twin" but never documented in the ## Errors table.

Line 197 names S220 as C220's fs twin, and approval-gate's JAIL_SCOPE_CODES (["S215", "S220", "C215", "C220"]) confirms it's a live code — but the S2xx table below (lines 256-269) jumps from S218 straight to S300 with no S220 entry, leaving the "fs twin" cross-reference dangling for readers.

📝 Suggested addition to the S2xx table
 | `S218` | `fs.max_read_bytes` / `fs.max_write_bytes` cap exceeded. |
+| `S220` | Path resolves inside the `fs.host_roots` jail but outside the per-call `cwd`/session scope. |
 | `S300` | Sandbox VM boot failed (needs a virtualization host: Apple Silicon or `/dev/kvm`). |

Also applies to: 256-269

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

In `@shell/README.md` around lines 189 - 198, Add the missing S220 entry to the
README’s S2xx Errors table, documenting it as the fs twin for C220 with the same
scope-root violation meaning. Place it between S218 and S300 so the existing
C220/S220 cross-reference and JAIL_SCOPE_CODES documentation are complete.

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:
Expand All @@ -214,18 +257,41 @@ 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. |
| `S300` | Sandbox VM boot failed (needs a virtualization host: Apple Silicon or `/dev/kvm`). |

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
Expand Down
39 changes: 35 additions & 4 deletions shell/src/code/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,25 @@ pub struct CoderConfig {
#[schemars(skip)]
pub base_paths: Vec<PathBuf>,

/// 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<PathBuf>,

/// Glob patterns matched against the path *relative to its containing
/// root*. Matching files can be listed but not
/// read/written/deleted/created.
Expand All @@ -48,13 +67,13 @@ pub struct CoderConfig {
pub default_exclude_globs: Vec<String>,

/// 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,
Expand Down Expand Up @@ -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")]
Expand All @@ -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.
Expand Down Expand Up @@ -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<PathBuf>,
/// 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<PathBuf>,
/// 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
Expand All @@ -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(),
Expand Down Expand Up @@ -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(),
}
Expand Down
Loading
Loading