diff --git a/crates/temper-mcp/src/runtime.rs b/crates/temper-mcp/src/runtime.rs index 72d2986e..1e5dd3da 100644 --- a/crates/temper-mcp/src/runtime.rs +++ b/crates/temper-mcp/src/runtime.rs @@ -433,6 +433,11 @@ impl RuntimeContext { entity_set_resolver: None, binary_path: None, api_key: api_key.as_deref(), + // Local stdio MCP server: the host process is the + // developer's own machine, so host ops (local file + // read, cargo build) act on the developer's checkout + // and are legitimately allowed. + allow_host_ops: true, }; temper_sandbox::dispatch::dispatch_temper_method( &ctx, diff --git a/crates/temper-sandbox/src/dispatch.rs b/crates/temper-sandbox/src/dispatch.rs index 56b52431..6b808594 100644 --- a/crates/temper-sandbox/src/dispatch.rs +++ b/crates/temper-sandbox/src/dispatch.rs @@ -38,6 +38,14 @@ pub struct DispatchContext<'a> { pub binary_path: Option<&'a std::path::Path>, /// Optional API key for authentication. pub api_key: Option<&'a str>, + /// Whether this dispatch context may perform host-process operations — + /// local filesystem reads and spawning `cargo` (`upload_wasm`, + /// `compile_wasm`). True only for a runner whose host process is the + /// developer's own machine (the local stdio MCP server). The + /// server-hosted REPL sets this false: its host process is the Temper + /// server, so host ops there read the server's filesystem and run code + /// as the server user — a host-compromise vector, not a developer op. + pub allow_host_ops: bool, } impl<'a> DispatchContext<'a> { @@ -87,6 +95,16 @@ pub async fn dispatch_temper_method( dispatch_governance(ctx, method, args).await } // --- WASM --- + // Host ops (local file read, `cargo build`) are rejected before + // touching the filesystem unless this context is host-trusted. The + // server-hosted REPL is not: running these in the server process is a + // host-compromise vector (arbitrary file read + RCE as the server user). + "upload_wasm" | "compile_wasm" if !ctx.allow_host_ops => Err(format!( + "temper.{method}() is not available in this context. Host operations \ + (local file read, cargo build) run only on the developer's own \ + machine via the local MCP server, never inside the Temper server \ + process." + )), "upload_wasm" | "compile_wasm" => dispatch_wasm(ctx, method, args).await, // --- Evolution / Observe --- "get_trajectories" | "get_insights" | "get_evolution_records" | "check_sentinel" => { @@ -736,3 +754,94 @@ fn resolve_sdk_path(binary_path: Option<&std::path::Path>) -> Result DispatchContext<'_> { + DispatchContext { + http: client, + base_url: "http://127.0.0.1:1", + tenant: "default", + agent_id: None, + agent_type: None, + session_id: None, + principal_id: None, + principal_kind: None, + agent_role: None, + entity_set_resolver: None, + binary_path: None, + api_key: None, + allow_host_ops, + } + } + + fn str_args(values: &[&str]) -> Vec { + values + .iter() + .map(|v| MontyObject::String((*v).to_string())) + .collect() + } + + /// When the context is not host-trusted, `upload_wasm` is rejected before + /// the filesystem is touched. We pass a path that exists and is readable + /// (`/etc/hosts`): if the gate failed to fire, dispatch would read it and + /// then fail on the loopback POST — a different error. Getting the + /// "not available in this context" message proves the read never happened. + #[tokio::test] + async fn upload_wasm_rejected_without_host_ops() { + let client = reqwest::Client::new(); + let args = str_args(&["mod", "/etc/hosts"]); + let err = dispatch_temper_method(&ctx(&client, false), "upload_wasm", &args, &[]) + .await + .expect_err("upload_wasm must be rejected without host ops"); + assert!( + err.contains("not available in this context"), + "expected host-op rejection, got: {err}" + ); + assert!( + !err.contains("failed to read"), + "gate must fire before any filesystem read, got: {err}" + ); + } + + /// `compile_wasm` is gated the same way and must be rejected before it + /// spawns `rustup`/`cargo`. + #[tokio::test] + async fn compile_wasm_rejected_without_host_ops() { + let client = reqwest::Client::new(); + let args = str_args(&["mod", "pub fn main() {}"]); + let err = dispatch_temper_method(&ctx(&client, false), "compile_wasm", &args, &[]) + .await + .expect_err("compile_wasm must be rejected without host ops"); + assert!( + err.contains("not available in this context"), + "expected host-op rejection, got: {err}" + ); + } + + /// With host ops allowed (the local stdio MCP context), the gate does not + /// fire: dispatch proceeds into `upload_wasm` and fails at the filesystem + /// read of a nonexistent path — proving the capability flag, not a hardcoded + /// block, is what governs the two host methods. + #[tokio::test] + async fn upload_wasm_allowed_with_host_ops_reaches_filesystem() { + let client = reqwest::Client::new(); + let args = str_args(&["mod", "/nonexistent/temper-arn166-does-not-exist"]); + let err = dispatch_temper_method(&ctx(&client, true), "upload_wasm", &args, &[]) + .await + .expect_err("read of a nonexistent path must fail"); + assert!( + err.contains("failed to read"), + "gate must allow the read attempt when host ops are permitted, got: {err}" + ); + assert!( + !err.contains("not available in this context"), + "host-trusted context must not reject host ops, got: {err}" + ); + } +} diff --git a/crates/temper-sandbox/src/repl.rs b/crates/temper-sandbox/src/repl.rs index 3936bae0..2fdd00bf 100644 --- a/crates/temper-sandbox/src/repl.rs +++ b/crates/temper-sandbox/src/repl.rs @@ -80,6 +80,10 @@ pub async fn run_repl(config: &ReplConfig, code: &str) -> Result { entity_set_resolver: None, binary_path: None, api_key: None, + // Server-hosted REPL: the host process is the Temper + // server, so host ops (local file read, cargo build) are a + // host-compromise vector and are disallowed here. + allow_host_ops: false, }; dispatch_temper_method(&ctx, &function_name, args, &kwargs).await } diff --git a/crates/temper-server/src/api/mod.rs b/crates/temper-server/src/api/mod.rs index 6f9499e6..69a6353d 100644 --- a/crates/temper-server/src/api/mod.rs +++ b/crates/temper-server/src/api/mod.rs @@ -227,6 +227,70 @@ pub(crate) async fn require_policy_auth( None } +/// Authorize a REPL sandbox execution request against Cedar policies. +/// +/// `POST /api/repl` runs agent-supplied code that can invoke every non-host +/// `temper.*` method, so executing the REPL at all is a privileged capability +/// and must be authorized — not open to any caller who can reach the port. +/// +/// Returns `Some(response)` (403 + `AuthorizationDenied`, with the recorded +/// decision id) if denied, `None` if allowed. Admin principals bypass, matching +/// [`require_policy_auth`]. +/// +/// Enforcement strength is bounded by the identity source: until the principal +/// is a resolved credential rather than a self-asserted `x-temper-*` header +/// (ARN-167), this gate trusts the claimed principal kind. The host-op +/// isolation in `temper-sandbox` — not this gate — is the unconditional stop +/// for the arbitrary file-read / RCE the endpoint otherwise exposed. +pub(crate) async fn require_repl_auth( + state: &ServerState, + headers: &HeaderMap, + tenant: &str, +) -> Option { + let security_ctx = security_context_from_headers(headers, None, None, None); + if matches!(security_ctx.principal.kind, PrincipalKind::Admin) { + return None; + } + if let Err(denial) = state.authorize_with_context( + &security_ctx, + "execute_repl", + "Sandbox", + &std::collections::BTreeMap::new(), + tenant, + ) { + let reason = denial.to_string(); + let pd = record_authz_denial( + state, + DenialInput { + tenant, + security_ctx: &security_ctx, + agent_id_override: None, + action: "execute_repl", + resource_type: "Sandbox", + resource_id: tenant, + resource_attrs: serde_json::json!({"tenant": tenant}), + reason: &reason, + module_name: None, + from_status: None, + }, + ) + .await; + return Some( + ( + StatusCode::FORBIDDEN, + axum::Json(serde_json::json!({ + "error": { + "code": "AuthorizationDenied", + "message": format!("{reason} Decision {}", pd.id), + } + })), + ) + .into_response(), + ); + } + None +} + /// Cedar policy-management gate as an axum extractor. /// /// Runs [`require_policy_auth`] against the `{tenant}` path parameter before diff --git a/crates/temper-server/src/api/repl.rs b/crates/temper-server/src/api/repl.rs index 0518647a..98c6a7bb 100644 --- a/crates/temper-server/src/api/repl.rs +++ b/crates/temper-server/src/api/repl.rs @@ -21,12 +21,17 @@ pub(crate) struct ReplRequest { /// POST /api/repl — execute Python code in the Temper Monty sandbox. /// /// The sandbox provides `temper.*` methods (create, action, submit_specs, etc.) -/// that loop back to this server via HTTP. Agent identity is extracted from +/// that loop back to this server over HTTP; each loopback call is subject to the +/// server's normal Cedar authorization. Agent identity is taken from /// `X-Temper-Principal-Id` / `X-Temper-Principal-Kind` / `X-Temper-Agent-Role` -/// headers and forwarded on internal requests. +/// headers and forwarded on those internal requests. /// -/// Security: 180s timeout, 64MB memory, method allowlisting, no filesystem or -/// network access. External APIs go through `[[integration]]` in IOA specs. +/// Executing the REPL requires Cedar authorization (`execute_repl` on +/// `Sandbox`). Host operations — local filesystem reads and `cargo build` +/// (`upload_wasm`/`compile_wasm`) — are NOT available here: this dispatch +/// context is not host-trusted, so those methods are rejected before touching +/// the host. They run only on the developer's own machine via the local MCP +/// server. Resource bounds: 180s timeout, 64MB memory. #[instrument(skip_all, fields(otel.name = "POST /api/repl"))] pub(crate) async fn handle_repl( State(state): State, @@ -59,6 +64,13 @@ pub(crate) async fn handle_repl( Err(e) => return e.into_response(), }; + // Executing the REPL is a privileged capability (arbitrary agent code that + // can drive every non-host temper.* method). Gate it behind Cedar before + // running anything. + if let Some(resp) = super::require_repl_auth(&state, &headers, &tenant).await { + return resp; + } + let agent_id = principal_id.clone(); let port = state.listen_port.get().copied().unwrap_or(4200); let code = body.code; diff --git a/crates/temper-server/tests/repl_auth_gate.rs b/crates/temper-server/tests/repl_auth_gate.rs new file mode 100644 index 00000000..bfca7d48 --- /dev/null +++ b/crates/temper-server/tests/repl_auth_gate.rs @@ -0,0 +1,128 @@ +//! `/api/repl` authorization gate (ARN-166). +//! +//! `POST /api/repl` executes agent-supplied code that can drive every non-host +//! `temper.*` method, so executing it is a privileged capability gated behind +//! Cedar (`execute_repl` on `Sandbox`). These tests pin the gate against a real, +//! deny-by-default tenant policy set: +//! +//! - an agent with no `execute_repl` grant is denied (403) before any code runs; +//! - an admin bypasses the gate (matching `require_policy_auth`) and runs; +//! - an agent that IS granted `execute_repl` runs — proving the gate is a real +//! Cedar evaluation keyed on the action, not a blanket agent-deny. +//! +//! The route is mounted only under the `observe` feature — the same feature +//! production (`temper-cli`) builds with — so this file compiles to nothing +//! without it. Run with: `cargo test -p temper-server --test repl_auth_gate +//! --features observe`. +#![cfg(feature = "observe")] + +use axum::body::Body; +use axum::http::{Request, StatusCode}; +use temper_runtime::ActorSystem; +use temper_server::{ServerState, SpecRegistry, build_router}; +use temper_spec::csdl::parse_csdl; +use tower::ServiceExt; + +const CSDL_XML: &str = include_str!("../../../test-fixtures/specs/model.csdl.xml"); +const ORDER_IOA: &str = include_str!("../../../test-fixtures/specs/order.ioa.toml"); + +/// Grants `execute_repl` to admins only — an agent is not covered, so Cedar +/// default-denies it. (Admins bypass the gate before Cedar anyway; the grant +/// keeps the policy realistic rather than empty.) +const ADMIN_ONLY_REPL: &str = + r#"permit(principal is Admin, action == Action::"execute_repl", resource);"#; + +/// Grants `execute_repl` to agents — the authorized-agent surface. +const AGENT_REPL: &str = + r#"permit(principal is Agent, action == Action::"execute_repl", resource);"#; + +/// Build server state whose `default` tenant has `policy` loaded, so the gate is +/// evaluated against real Cedar policies rather than the permissive fallback +/// that `from_registry` installs (which would allow everything and make the +/// deny assertion vacuous). +fn state_with_policy(policy: &str) -> ServerState { + let csdl = parse_csdl(CSDL_XML).expect("CSDL should parse"); + let mut registry = SpecRegistry::new(); + registry.register_tenant( + "default", + csdl, + CSDL_XML.to_string(), + &[("Order", ORDER_IOA)], + ); + let state = ServerState::from_registry(ActorSystem::new("repl-auth-gate-test"), registry); + state + .authz + .reload_tenant_policies("default", policy) + .expect("tenant policy should load"); + state +} + +fn repl_request(principal_kind: &str, code: &str) -> Request { + Request::post("/api/repl") + .header("Content-Type", "application/json") + .header("X-Temper-Principal-Kind", principal_kind) + .header("X-Temper-Principal-Id", "arn166-test-principal") + .body(Body::from(serde_json::json!({ "code": code }).to_string())) + .unwrap() +} + +/// An agent with no `execute_repl` grant is denied with 403 before any code +/// runs — the tenant policy grants the action to admins only, so Cedar +/// default-denies the agent. +#[tokio::test] +async fn repl_denied_for_unauthorized_agent() { + let app = build_router(state_with_policy(ADMIN_ONLY_REPL)); + let response = app + .oneshot(repl_request("agent", "result = 1 + 1")) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::FORBIDDEN); + let body = axum::body::to_bytes(response.into_body(), 1024 * 1024) + .await + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(json["error"]["code"], "AuthorizationDenied"); +} + +/// Admin bypasses the gate (matching `require_policy_auth`) even against the +/// deny-by-default policy set, so the REPL still runs for an authorized surface +/// — the gate closes the hole without removing the capability. A clean run +/// returns 200 with `error: null`; a sandbox execution error would surface as a +/// non-null `error` at 200 (see `handle_repl`), so asserting both status and a +/// null `error` pins that the code actually executed rather than silently +/// erroring. +#[tokio::test] +async fn repl_allowed_for_admin() { + let app = build_router(state_with_policy(ADMIN_ONLY_REPL)); + let response = app + .oneshot(repl_request("admin", "result = 1 + 1")) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + let body = axum::body::to_bytes(response.into_body(), 1024 * 1024) + .await + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(json["error"], serde_json::Value::Null); +} + +/// An agent that IS granted `execute_repl` passes the gate and runs — proving +/// the gate is a real Cedar evaluation keyed on the action, not a hardcoded +/// agent-deny. Same success shape as the admin case: 200 with `error: null`. +#[tokio::test] +async fn repl_allowed_for_authorized_agent() { + let app = build_router(state_with_policy(AGENT_REPL)); + let response = app + .oneshot(repl_request("agent", "result = 1 + 1")) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK); + let body = axum::body::to_bytes(response.into_body(), 1024 * 1024) + .await + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).unwrap(); + assert_eq!(json["error"], serde_json::Value::Null); +} diff --git a/docs/adrs/0155-repl-host-op-isolation-and-authz.md b/docs/adrs/0155-repl-host-op-isolation-and-authz.md new file mode 100644 index 00000000..8b35d16a --- /dev/null +++ b/docs/adrs/0155-repl-host-op-isolation-and-authz.md @@ -0,0 +1,170 @@ +# ADR-0155: REPL host-op isolation and authorization gate + +- Status: Accepted +- Date: 2026-07-06 +- Deciders: Temper core maintainers +- Related: + - ARN-166: `[CRITICAL] Kernel /api/repl is unauthenticated → arbitrary host file-read + RCE` + - ARN-167: `[CRITICAL] TemperPaw auth bypass via self-asserted x-temper-* principal` (global header-trust; separate PR) + - `crates/temper-sandbox/src/dispatch.rs` (shared `temper.*` dispatch) + - `crates/temper-sandbox/src/repl.rs` (server REPL runner) + - `crates/temper-mcp/src/runtime.rs` (local stdio MCP runner) + - `crates/temper-server/src/api/repl.rs`, `crates/temper-server/src/api/mod.rs` (HTTP surface) + +## Context + +`POST /api/repl` executes agent-supplied Python in the Monty sandbox. The Python +can call `temper.*` methods, dispatched by the shared +`temper_sandbox::dispatch::dispatch_temper_method`. Two of those methods perform +**host-process** operations rather than looping back over HTTP: + +- `temper.upload_wasm(name, path)` → `tokio::fs::read(path)` on an attacker-chosen + path, then POSTs the bytes to `/api/wasm/modules/{name}`. Arbitrary host file + read; no dependency on cwd or a resolved binary path, so it is live in every + deployment including production. +- `temper.compile_wasm(name, src)` → writes attacker Rust to a temp crate and runs + `cargo build`. Code execution as the server user via `build.rs`/proc-macros, + gated today only by the server's cwd happening to be a temper workspace. + +Two facts make this exploitable: + +1. **`dispatch_temper_method` is context-blind.** It is called by exactly two + runners — the server REPL (`temper-sandbox/src/repl.rs`, whose host process is + the **Temper server**) and `temper-mcp` (`temper-mcp/src/runtime.rs`, a + **stdio** server the developer runs on their **own machine**). Host filesystem + and `cargo build` operations are legitimate only for the second: they act on + the developer's local checkout. For the first, they act on the server host — + which is never the intent. + +2. **`/api/repl` has no authorization.** The route is registered with no auth + extractor, and `handle_repl` reads the principal straight from the + self-asserted `x-temper-principal-id` header. Anyone who can reach the port can + execute REPL code — fully open on a standalone kernel with no `TEMPER_API_KEY`, + and reachable under TemperPaw via the ARN-167 header bypass. + +The endpoint's own doc comment claims "no filesystem or network access" — false. + +## Decision + +Two independent layers. Layer B removes the host-compromise primitive and is +sufficient on its own to close the RCE/file-read even in fully-open mode; layer A +restores authorization to the endpoint. + +### Sub-Decision B: Host ops are a capability of the dispatch context, not a method + +Add an explicit capability to `DispatchContext`: + +```rust +pub struct DispatchContext<'a> { + // ... + /// Whether this dispatch context may perform host-process operations + /// (local filesystem reads, spawning `cargo`). True only for a runner + /// whose host process is the developer's own machine (the local stdio + /// MCP server). The server-hosted REPL sets this false: its host process + /// is the Temper server, so host ops there are a host-compromise vector. + pub allow_host_ops: bool, +} +``` + +`upload_wasm` and `compile_wasm` are the host ops. When `allow_host_ops` is +false, they are rejected before touching the filesystem or spawning a process, +in the same style as the already-blocked governance writes +(`approve_decision`/`deny_decision`/`set_policy`): + +```rust +"upload_wasm" | "compile_wasm" if !ctx.allow_host_ops => Err(format!( + "temper.{method}() is not available in this context. Host operations \ + (local file read, cargo build) run only on the developer's own machine \ + via the local MCP server, never inside the Temper server process." +)), +"upload_wasm" | "compile_wasm" => dispatch_wasm(ctx, method, args).await, +``` + +Runner settings: +- `temper-sandbox/src/repl.rs` (server REPL): `allow_host_ops: false`. +- `temper-mcp/src/runtime.rs` (local stdio MCP): `allow_host_ops: true`. + +**Why this approach**: it fixes the class of problem — "the server-reachable +dispatch context must not perform host-process operations" — generically, rather +than deleting two method names that could be reintroduced. A future host op is +covered by the same gate. The trust boundary is expressed where it actually lives +(the runner that owns the host process), not smuggled into an unrelated field. + +### Sub-Decision A: `/api/repl` requires Cedar authorization + +`/api/repl` gains an authorization gate before it runs any code. The REPL can +call every non-host `temper.*` method (entity writes, spec submission, app +install), so it is a privileged capability and must be authorized, not open to +any caller who reaches the port. The gate reuses the server's existing Cedar +enforcement (`authorize_with_context`) against a dedicated `execute_repl` action +on a `Sandbox` resource, recording a governance decision on denial exactly like +the other gated endpoints. + +**Why this approach**: it makes the REPL a governed capability using the same +Cedar path as spec submission and policy management, so denials are visible and +approvable through the existing Observe flow. It does not attempt to fix the +global self-asserted-header trust — that is ARN-167 and lands separately; once it +does, this same gate runs against a resolved principal with no code change here. + +## Consequences + +### Positive +- Arbitrary host file read and RCE via `/api/repl` are removed outright — the + server-reachable dispatch context can no longer touch the host filesystem or + spawn processes, in any deployment mode. +- The REPL becomes a Cedar-governed capability; unauthorized use is denied and + recorded rather than silently executed. +- The host-op trust boundary is now explicit and reusable for any future method. + +### Negative +- `upload_wasm`/`compile_wasm` no longer work through `/api/repl`. This is + intended: those ops only ever made sense against the developer's local + filesystem, which the server path never had. They remain available through the + local stdio MCP server, which is their correct home. + +### Risks +- If a legitimate flow somewhere relied on `/api/repl` performing host ops, it + breaks. Investigation found no such caller: both wasm host ops are documented + as developer/CLI operations and the only production consumer that sets + `allow_host_ops: true` is the local stdio MCP. Mitigation: the rejection + message names the correct path (local MCP). + +### DST Compliance +- `temper-server` is simulation-visible. The new `allow_host_ops` field is a + plain `bool` carried on an existing struct; no new time, randomness, threads, + or I/O are introduced on the sim path. The Cedar gate uses the existing + `authorize_with_context` + `sim_now`/`sim_uuid` denial-recording path. No new + `// determinism-ok` annotations required. + +## Non-Goals + +- Fixing the global self-asserted `x-temper-*` header trust (ARN-167). +- Sandboxing the Monty interpreter further, or restricting the non-host + `temper.*` methods the REPL may call beyond the authorization gate. +- Changing how `temper-mcp` resolves the wasm SDK path. + +## Alternatives Considered + +1. **Delete `upload_wasm`/`compile_wasm` from `dispatch_temper_method` entirely.** + Rejected: it removes a capability the local MCP developer flow legitimately + uses, and CLAUDE.md forbids dropping working capabilities. The capability is + not the problem; running it in the server host process is. +2. **Gate host ops on `binary_path.is_some()`.** Rejected: `binary_path` is a + convenience for SDK resolution, not a trust signal — both current runners pass + `None`, so this would either block the legitimate MCP path or fail open. Trust + must be an explicit, named capability. +3. **Auth gate alone (layer A only).** Rejected: it leaves the host-compromise + primitive intact wherever the REPL is reachable (open standalone kernel, or any + future auth regression). Removing the primitive is the durable fix. +4. **Make `/api/repl` a dev-only, feature-gated endpoint.** Considered. It would + remove the endpoint from production entirely, but the REPL is a legitimate + agent capability (Code Mode) that TemperPaw uses in production; gating it off + would remove a working capability. Authorization + host-op isolation preserves + the capability while closing the hole. + +## Rollback Policy + +Both layers are additive and independently reversible. Reverting Sub-Decision B +restores host ops to the server dispatch (re-opening the primitive); reverting +Sub-Decision A removes the authorization gate. Neither changes on-disk state or +spec formats, so rollback is a code revert with no migration.