fix(sandbox,server): isolate REPL host ops and gate /api/repl behind Cedar (ARN-166)#342
fix(sandbox,server): isolate REPL host ops and gate /api/repl behind Cedar (ARN-166)#342rita-aga wants to merge 2 commits into
Conversation
…Cedar (ARN-166)
POST /api/repl was unauthenticated and dispatched temper.upload_wasm /
temper.compile_wasm, which ran host-process operations inside the Temper server
process: tokio::fs::read on an attacker-chosen path (arbitrary host file read,
retrievable via the module store) and cargo build on attacker Rust (RCE as the
server user). The handler doc comment even claimed "no filesystem or network
access" - false.
Two independent layers (see docs/adrs/0155):
- Host-op isolation (closes the RCE/file-read unconditionally): add
allow_host_ops to DispatchContext. dispatch_temper_method rejects
upload_wasm/compile_wasm before touching the host unless the context is
host-trusted. Only the local stdio MCP (the developer's own machine) sets it
true; the server REPL sets it false. The struct has no Default/builder and the
field is required, so every constructor must set it (compile-enforced).
- Authorization: /api/repl now requires Cedar (execute_repl on Sandbox) before
running any code, recording a governance decision on denial like the other
gated endpoints. Admin bypasses, matching require_policy_auth - header-spoofable
until ARN-170, which the host-op isolation does not depend on.
Tests: temper-sandbox host_op_gate_tests (3 - gate fires before the fs read;
allowed-with-host-ops still reaches the fs, proving the flag governs); temper-server
repl_auth_gate (3 - deny unauthorized agent via a real deny-by-default policy,
admin bypass, authorized-agent allow; observe-gated since the route is
observe-only). Verified live: even as admin, awaited upload_wasm('/etc/passwd')
and compile_wasm are rejected; agent/anonymous get 403; benign REPL still runs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BM1pdoLhHH7TQgGuiadQiy
Wrap register_tenant call to satisfy the pre-push rustfmt gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BM1pdoLhHH7TQgGuiadQiy
|
@greptile review |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let security_ctx = security_context_from_headers(headers, None, None, None); | ||
| if matches!(security_ctx.principal.kind, PrincipalKind::Admin) { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
security_context_from_headers reads PrincipalKind from the unverified X-Temper-Principal-Kind header. Any caller who can reach the port can self-assert X-Temper-Principal-Kind: admin and bypass the Cedar check entirely, reaching the REPL sandbox with access to all non-host temper.* methods. This is the system-wide header-trust issue tracked under ARN-170 (and ARN-167 for TemperPaw) and is consistent with how every other Cedar gate in the codebase (require_policy_auth, require_observe_auth) behaves — the PR description acknowledges this explicitly. Layer B (allow_host_ops: false) is the durable stop for the worst-case attack regardless, but Layer A as written is effectively a no-op against a motivated unauthenticated caller until the credential-resolution path lands.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/temper-server/src/api/mod.rs
Line: 250-253
Comment:
**Admin bypass is fully header-spoofable — Layer A gate provides no real barrier until ARN-170 lands**
`security_context_from_headers` reads `PrincipalKind` from the unverified `X-Temper-Principal-Kind` header. Any caller who can reach the port can self-assert `X-Temper-Principal-Kind: admin` and bypass the Cedar check entirely, reaching the REPL sandbox with access to all non-host `temper.*` methods. This is the system-wide header-trust issue tracked under ARN-170 (and ARN-167 for TemperPaw) and is consistent with how every other Cedar gate in the codebase (`require_policy_auth`, `require_observe_auth`) behaves — the PR description acknowledges this explicitly. Layer B (`allow_host_ops: false`) is the durable stop for the worst-case attack regardless, but Layer A as written is effectively a no-op against a motivated unauthenticated caller until the credential-resolution path lands.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
Correct, and this is the load-bearing point. This is the systemic Class A header-trust issue (ARN-170), not a repl-specific hole — fixing it locally in require_repl_auth would duplicate the edge fix that ARN-170 makes once, for every Cedar gate. Layer B (allow_host_ops: false) closes the RCE/file-read regardless. Merge dependency: this PR's Layer-A guarantee is only complete once ARN-170's auth-edge PR lands, so #342 should merge with or after it, not alone. Not downgrading severity — it's real until ARN-170 merges; it resolves there, not here. (Doc comment still cites ARN-167; correcting to ARN-170 in a follow-up commit.)
| if let Err(denial) = state.authorize_with_context( | ||
| &security_ctx, | ||
| "execute_repl", | ||
| "Sandbox", | ||
| &std::collections::BTreeMap::new(), | ||
| tenant, | ||
| ) { |
There was a problem hiding this comment.
resource_attrs empty in Cedar call, {tenant} in denial record
The authorize_with_context call passes &std::collections::BTreeMap::new() while record_authz_denial records {"tenant": tenant}. This means any Cedar policy that inspects resource.tenant on a Sandbox resource (e.g., restricting REPL access to a specific tenant) would never match — Cedar sees an attribute-less resource. The PR description notes this as a known P2 with no security impact, and require_policy_auth has the same inconsistency, so this is at least consistent. Populating the Cedar call with the same attrs as the denial record would let future policies use resource.tenant.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/temper-server/src/api/mod.rs
Line: 254-260
Comment:
**`resource_attrs` empty in Cedar call, `{tenant}` in denial record**
The `authorize_with_context` call passes `&std::collections::BTreeMap::new()` while `record_authz_denial` records `{"tenant": tenant}`. This means any Cedar policy that inspects `resource.tenant` on a `Sandbox` resource (e.g., restricting REPL access to a specific tenant) would never match — Cedar sees an attribute-less resource. The PR description notes this as a known P2 with no security impact, and `require_policy_auth` has the same inconsistency, so this is at least consistent. Populating the Cedar call with the same attrs as the denial record would let future policies use `resource.tenant`.
How can I resolve this? If you propose a fix, please make it concise.
| /// isolation in `temper-sandbox` — not this gate — is the unconditional stop | ||
| /// for the arbitrary file-read / RCE the endpoint otherwise exposed. |
There was a problem hiding this comment.
The doc comment cites ARN-167 as the pending fix for the Admin/principal bypass, but per the PR description, ARN-167 is the TemperPaw sibling and ARN-170 is the kernel-specific tracker ("kernel Class A: header-trusted admin + fail-open"). A developer looking up ARN-167 to understand the kernel residual will land on the TemperPaw issue, not the open kernel work.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/temper-server/src/api/mod.rs
Line: 243-244
Comment:
**Wrong ARN reference in doc comment**
The doc comment cites ARN-167 as the pending fix for the Admin/principal bypass, but per the PR description, ARN-167 is the TemperPaw sibling and ARN-170 is the kernel-specific tracker ("kernel Class A: header-trusted admin + fail-open"). A developer looking up ARN-167 to understand the kernel residual will land on the TemperPaw issue, not the open kernel work.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| 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. |
There was a problem hiding this comment.
upload_wasm_allowed_with_host_ops_reaches_filesystem asserts err.contains("failed to read") to prove the filesystem read was attempted (gate did not fire). If dispatch_wasm ever changes its IO error format (e.g., to "io error" or "cannot read"), this assertion will fail on a refactoring-only change and will need updating alongside the error message, creating a maintenance coupling that is easy to overlook.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/temper-sandbox/src/dispatch.rs
Line: 815-830
Comment:
**Test positive-path proof is coupled to a raw error-message substring**
`upload_wasm_allowed_with_host_ops_reaches_filesystem` asserts `err.contains("failed to read")` to prove the filesystem read was attempted (gate did not fire). If `dispatch_wasm` ever changes its IO error format (e.g., to `"io error"` or `"cannot read"`), this assertion will fail on a refactoring-only change and will need updating alongside the error message, creating a maintenance coupling that is easy to overlook.
How can I resolve this? If you propose a fix, please make it concise.1 participant
Closes ARN-166 (CRITICAL) under epic ARN-165. Draft for review — do not merge.
Problem
POST /api/replexecutes agent-supplied Python that can calltemper.*methods. Two of those ran in the host process, not the sandbox:temper.upload_wasm(name, path)→tokio::fs::read(path)on an attacker path → arbitrary host file read (retrievable via/api/wasm/modules/{name}).temper.compile_wasm(name, src)→cargo buildon attacker Rust → RCE as the server user.The route had no authorization (identity read from self-asserted
x-temper-*headers), so it was fully open on a standalone kernel and reachable under TemperPaw via the header bypass. The handler's own doc claimed "no filesystem or network access" — false.Fix (two independent layers — ADR-0155)
Layer B — host-op isolation (the durable, identity-independent fix). A new
allow_host_ops: boolonDispatchContext.upload_wasm/compile_wasmare rejected before touching the filesystem or spawning a process unless the context is host-trusted. Only the local stdio MCP server (temper-mcp/src/runtime.rs, the developer's own machine) setstrue; the server-hosted REPL (temper-sandbox/src/repl.rs) setsfalse. Enumeration: the only two productionDispatchContextconstructors are those two; noDefaultimpl exists; the field is required, so any future constructor must choose explicitly. The server path cannot reach host ops.Layer A — Cedar authorization on
/api/repl.require_repl_authgates execution via the sameauthorize_with_contextpath as policy management (execute_replonSandbox), recording a governance decision on denial. Deny → 403 before any code runs.Verification
temper serve: even as admin (bypasses layer A),POST /api/repl {await temper.upload_wasm('x','/etc/passwd')}→ rejected, file never read, zero loopback POSTs;compile_wasmsame. Agent/anonymous → 403 before code runs. Benignreturn 1+1→{"result":2}(capability preserved).temper-sandbox host_op_gate_tests(3, assert the fs read never happens / the flag governs) +temper-server repl_auth_gate(3, deny-by-default Cedar denial, admin bypass, authorized allow) — all pass. Fulltemper-serverlib suite green (a one-off native SIGSEGV under parallel load did not reproduce — see ARN-197).Residual
The Cedar gate's Admin/principal derivation is header-spoofable until ARN-170 (kernel Class A: header-trusted admin + fail-open) lands; ARN-167 is the TemperPaw sibling. The RCE/file-read is closed independently by layer B regardless. One follow-up P2 noted in review (empty
resource_attrsto Cedar vs{tenant}on the denial record — no security impact).🤖 Generated with Claude Code
Greptile Summary
This PR closes a critical security vulnerability (ARN-166) where
POST /api/replhad no authorization and allowed agent-supplied Python to invoketemper.upload_wasm/temper.compile_wasm, resulting in arbitrary host file-read and RCE as the server user. The fix is two independent layers: a newallow_host_ops: boolfield onDispatchContextthat gates host-process operations unconditionally at the dispatch layer, and a Cedar authorization gate (execute_replonSandbox) added to the HTTP handler before any code runs.upload_wasmandcompile_wasmare rejected before touching the filesystem or spawningcargounlessallow_host_opsistrue; only the local stdio MCP server (developer's machine) sets this true — the server-hosted REPL sets itfalse. The field is required on the struct (noDefaultimpl), so any futureDispatchContextconstructor must choose explicitly.require_repl_authfollows the sameauthorize_with_context+ denial-recording pattern as the existingrequire_policy_auth; Admin principals bypass matching that function's behavior; the enforcement boundary (header-spoofable until ARN-170) is documented in the function's doc comment.Confidence Score: 4/5
The unconditional host-op gate in dispatch.rs is the durable core of the fix and is correctly placed before any filesystem or process access; the Cedar gate in api/mod.rs is consistent with existing auth patterns.
Both security layers look correct: allow_host_ops false on the server REPL path closes the file-read and RCE primitives before any IO, and require_repl_auth follows the same Cedar enforcement path as require_policy_auth. The two findings are documentation and test fragility nits that do not affect runtime behavior.
crates/temper-server/src/api/mod.rs for the ARN-167 vs ARN-170 doc comment reference; crates/temper-sandbox/src/dispatch.rs for the failed to read string assertion in the positive-path host-op test.
Important Files Changed
allow_host_ops: booltoDispatchContextand a guard arm that rejectsupload_wasm/compile_wasmbefore any filesystem/process access when the flag is false; includes thorough unit tests pinning gate-fires-first semantics.allow_host_ops: falseon the server-hosted REPL'sDispatchContext; change is a one-liner in the right struct construction site.allow_host_ops: truefor the local stdio MCP server context; the comment correctly explains why this is the only legitimate location for host ops.require_repl_auth, a Cedar gate forPOST /api/replusing the sameauthorize_with_context+ denial-recording pattern asrequire_policy_auth; minor: doc comment cites ARN-167 (TemperPaw) where ARN-170 (kernel) is the open kernel tracker.require_repl_authcall before any code execution inhandle_repl; correctly placed after tenant extraction and beforespawn_blocking.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant C as Caller participant H as handle_repl participant A as require_repl_auth (Cedar) participant S as run_repl (sandbox) participant D as dispatch_temper_method C->>H: POST /api/repl H->>H: extract_tenant() H->>A: require_repl_auth(state, headers, tenant) alt Admin principal (header-asserted, ARN-170 pending) A-->>H: None (bypass) else Cedar deny A-->>H: Some(403 AuthorizationDenied) H-->>C: 403 else Cedar allow A-->>H: None end H->>S: spawn_blocking, run_repl(config, code) S->>D: "dispatch_temper_method(ctx allow_host_ops=false)" alt upload_wasm or compile_wasm D-->>S: Err not available in this context else other temper method D->>D: HTTP loopback (Cedar-gated) D-->>S: Ok(result) end S-->>H: result or error H-->>C: 200%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant C as Caller participant H as handle_repl participant A as require_repl_auth (Cedar) participant S as run_repl (sandbox) participant D as dispatch_temper_method C->>H: POST /api/repl H->>H: extract_tenant() H->>A: require_repl_auth(state, headers, tenant) alt Admin principal (header-asserted, ARN-170 pending) A-->>H: None (bypass) else Cedar deny A-->>H: Some(403 AuthorizationDenied) H-->>C: 403 else Cedar allow A-->>H: None end H->>S: spawn_blocking, run_repl(config, code) S->>D: "dispatch_temper_method(ctx allow_host_ops=false)" alt upload_wasm or compile_wasm D-->>S: Err not available in this context else other temper method D->>D: HTTP loopback (Cedar-gated) D-->>S: Ok(result) end S-->>H: result or error H-->>C: 200Prompt To Fix All With AI
Reviews (2): Last reviewed commit: "style: rustfmt repl_auth_gate test (ARN-..." | Re-trigger Greptile