Skip to main content

max / makenotwork

bento: close agent /pull RCE-read + harden command surface (Run 2 CR1/CR2/S3) CR1: ops-agent /pull was unauthenticated and unconfined (tokio::fs::read of any path) — exfiltrated signing/SSH keys from any tailnet peer. Route /pull and /health through the same whois->authorize gate as /run, gate /pull on the build-log observe grant, confine reads to a configured pull_root (canonicalized, no traversal), and stream the file instead of buffering it whole. /health no longer leaks grant detail to unknown callers. CR2: document the agent trust boundary (the capability token gates the action label, not the command bytes; the tailnet allow-list is the perimeter) and make ops-agent refuse to bind a non-tailnet/non-loopback interface so the allow-list can't be bypassed off-tailnet. S3: validate env names as shell identifiers in the daemon env() host fn and in the transport env rendering (the name is interpolated unquoted) — closes the ${...} command-injection seam. Also collapse pre-existing clippy collapsible_if drift in ops-core/live_log.rs so the shared workspace gates clean. ops-exec 35 tests, bento-daemon 39, ops-core + sando-daemon green; clippy -D warnings clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-23 01:11 UTC
Commit: cf7bcfe0b9c4f5b0c7fff935eb9ef88657e97bf9
Parent: 83486aa
6 files changed, +354 insertions, -70 deletions
M bento/daemon/src/engine.rs +13
@@ -511,6 +511,19 @@ pub fn build_engine(ctx: Arc<RecipeCtx>) -> Engine {
511 511 {
512 512 let ctx = ctx.clone();
513 513 engine.register_fn("env", move |host: &str, key: &str| -> Result<String, Box<EvalAltResult>> {
514 + // The key is interpolated into a `${...}` shell expansion, so it must
515 + // be a bare shell identifier — anything else (quotes, `}`, `$`, `;`)
516 + // could break out and run arbitrary commands on the host. Validate
517 + // before building the command; this is the one env read that can't
518 + // sh-quote its argument (a quoted var name doesn't expand).
519 + if key.is_empty()
520 + || !key.chars().next().is_some_and(|c| c == '_' || c.is_ascii_alphabetic())
521 + || !key.chars().all(|c| c == '_' || c.is_ascii_alphanumeric())
522 + {
523 + return Err(rhai_err(format!(
524 + "env name `{key}` must be a shell identifier ([A-Za-z_][A-Za-z0-9_]*)"
525 + )));
526 + }
514 527 // Read via the shell so it works on remote hosts too.
515 528 let (code, tail) = ctx
516 529 .run(host, &format!("printf '%s' \"${{{key}}}\""))
M shared/ops-core/src/live_log.rs +14 -14
@@ -46,10 +46,10 @@ impl LiveLog {
46 46
47 47 /// Flush and close the file. Best-effort; errors are logged.
48 48 pub async fn close(mut self) {
49 - if let Some(mut f) = self.file.take() {
50 - if let Err(e) = f.flush().await {
51 - tracing::warn!(error = %e, path = %self.path.display(), "live log flush failed");
52 - }
49 + if let Some(mut f) = self.file.take()
50 + && let Err(e) = f.flush().await
51 + {
52 + tracing::warn!(error = %e, path = %self.path.display(), "live log flush failed");
53 53 }
54 54 }
55 55
@@ -67,11 +67,11 @@ impl LogSink for LiveLog {
67 67 if bytes.is_empty() {
68 68 return;
69 69 }
70 - if let Some(f) = self.file.as_mut() {
71 - if let Err(e) = f.write_all(bytes).await {
72 - tracing::warn!(error = %e, path = %self.path.display(), "live log write failed");
73 - self.file = None;
74 - }
70 + if let Some(f) = self.file.as_mut()
71 + && let Err(e) = f.write_all(bytes).await
72 + {
73 + tracing::warn!(error = %e, path = %self.path.display(), "live log write failed");
74 + self.file = None;
75 75 }
76 76 let text = String::from_utf8_lossy(bytes);
77 77 (self.on_chunk)(self.seq, &text);
@@ -80,11 +80,11 @@ impl LogSink for LiveLog {
80 80 }
81 81
82 82 async fn open_for_append(path: &Path) -> Option<File> {
83 - if let Some(parent) = path.parent() {
84 - if let Err(e) = tokio::fs::create_dir_all(parent).await {
85 - tracing::warn!(error = %e, dir = %parent.display(), "could not create log dir");
86 - return None;
87 - }
83 + if let Some(parent) = path.parent()
84 + && let Err(e) = tokio::fs::create_dir_all(parent).await
85 + {
86 + tracing::warn!(error = %e, dir = %parent.display(), "could not create log dir");
87 + return None;
88 88 }
89 89 match tokio::fs::OpenOptions::new().create(true).append(true).open(path).await {
90 90 Ok(f) => Some(f),
M shared/ops-exec/src/agent.rs +181 -49
@@ -10,18 +10,30 @@
10 10 //!
11 11 //! macOS deployment is an Aqua LaunchAgent (`LimitLoadToSessionType = Aqua`) so
12 12 //! build+sign run in the GUI security session where codesign can use the key.
13 + //!
14 + //! ## Trust boundary (read before touching `/run`)
15 + //!
16 + //! The capability model gates the *action label* (`build`/`sign`/…), not the
17 + //! command bytes: a `Step` carries arbitrary `argv`/`shell_script` that the
18 + //! agent runs verbatim once the label is permitted. An allow-listed caller is
19 + //! therefore trusted to run **arbitrary code** on this host under any action it
20 + //! is granted — the perimeter is the Tailscale `whois` allow-list plus the
21 + //! tailnet-only bind (`ops-agent` refuses a non-tailnet `listen`), not the
22 + //! capability token. Do not treat a narrow grant as a command sandbox. Every
23 + //! request-facing route here (`/run`, `/pull`, `/health`) resolves identity and
24 + //! authorizes before doing work — there is no unauthenticated surface.
13 25
14 26 use crate::capability::CapabilitySet;
15 27 use crate::executor::Executor;
16 28 use crate::remote::LogSink;
17 - use crate::step::Action;
29 + use crate::step::{Action, ObserveKind};
18 30 use crate::transport::LocalExec;
19 31 use crate::wire::{Frame, HealthResponse, RunRequest};
20 32 use anyhow::{Context, Result};
21 33 use async_trait::async_trait;
22 34 use serde::Deserialize;
23 35 use std::net::{IpAddr, SocketAddr};
24 - use std::path::PathBuf;
36 + use std::path::{Component, Path, PathBuf};
25 37 use std::sync::Arc;
26 38
27 39 /// The agent's local configuration (TOML).
@@ -37,6 +49,11 @@ pub struct AgentConfig {
37 49 /// implies. The effective grant is `caller.caps ∩ self.grant`.
38 50 #[serde(default)]
39 51 pub allow: Vec<CallerGrant>,
52 + /// Root directory `/pull` may read from. Requests are confined under this
53 + /// canonicalized path; `..` and symlinks that escape it are rejected.
54 + /// `None` disables `/pull` entirely (it returns 403).
55 + #[serde(default)]
56 + pub pull_root: Option<PathBuf>,
40 57 }
41 58
42 59 /// Grant tokens as they appear in config: `actuate = [...]`, `observe = [...]`.
@@ -97,17 +114,24 @@ pub enum AuthDecision {
97 114 Denied,
98 115 }
99 116
117 + /// The effective grant for a caller: `caller_grant ∩ agent_grant`, or `None` if
118 + /// the caller is not in this agent's allow-list. The single source of truth for
119 + /// what an authenticated caller may do — every route authorizes against it.
120 + fn effective_grant(config: &AgentConfig, caller: &CallerIdentity) -> Option<CapabilitySet> {
121 + config
122 + .allow
123 + .iter()
124 + .find(|c| caller.matches(&c.identity))
125 + .map(|entry| entry.to_caps().intersect(&config.grant.to_caps()))
126 + }
127 +
100 128 /// Pure authorization core — no IO, fully unit-testable. The effective grant is
101 129 /// `caller_grant ∩ agent_grant`; the action must be permitted by it.
102 130 pub fn authorize(config: &AgentConfig, caller: &CallerIdentity, action: &Action) -> AuthDecision {
103 - let Some(entry) = config.allow.iter().find(|c| caller.matches(&c.identity)) else {
104 - return AuthDecision::UnknownCaller;
105 - };
106 - let effective = entry.to_caps().intersect(&config.grant.to_caps());
107 - if effective.permits(action) {
108 - AuthDecision::Allow
109 - } else {
110 - AuthDecision::Denied
131 + match effective_grant(config, caller) {
132 + None => AuthDecision::UnknownCaller,
133 + Some(effective) if effective.permits(action) => AuthDecision::Allow,
134 + Some(_) => AuthDecision::Denied,
111 135 }
112 136 }
113 137
@@ -188,7 +212,8 @@ impl LogSink for ChannelSink {
188 212
189 213 /// Build the axum router. Serve it with
190 214 /// `.into_make_service_with_connect_info::<SocketAddr>()` so handlers see the
191 - /// peer address.
215 + /// peer address. Every route resolves the caller's tailnet identity and
216 + /// authorizes before doing work — there is no unauthenticated surface.
192 217 pub fn router(state: AgentState) -> axum::Router {
193 218 use axum::routing::{get, post};
194 219 axum::Router::new()
@@ -198,15 +223,40 @@ pub fn router(state: AgentState) -> axum::Router {
198 223 .with_state(state)
199 224 }
200 225
226 + /// Resolve the caller's tailnet identity and effective grant, or an early
227 + /// 403 response. The single gate every route passes through: a `whois` failure
228 + /// or an unknown caller never reaches handler logic.
229 + async fn resolve_caller(
230 + state: &AgentState,
231 + peer: IpAddr,
232 + ) -> Result<(CallerIdentity, CapabilitySet), axum::response::Response> {
233 + use axum::http::StatusCode;
234 + use axum::response::IntoResponse;
235 + let identity = (state.whois)(peer)
236 + .await
237 + .map_err(|e| (StatusCode::FORBIDDEN, format!("whois failed: {e}")).into_response())?;
238 + let effective = effective_grant(&state.config, &identity).ok_or_else(|| {
239 + (StatusCode::FORBIDDEN, format!("unknown caller: {}", identity.node)).into_response()
240 + })?;
241 + Ok((identity, effective))
242 + }
243 +
201 244 async fn health(
202 245 axum::extract::State(state): axum::extract::State<AgentState>,
203 - ) -> axum::Json<HealthResponse> {
204 - let caps = state.config.grant.to_caps();
205 - axum::Json(HealthResponse {
206 - ok: true,
207 - actuate: caps.actuate_tokens().map(String::from).collect(),
208 - observe: caps.observe_kinds().map(|k| k.token()).collect(),
209 - })
246 + axum::extract::ConnectInfo(peer): axum::extract::ConnectInfo<SocketAddr>,
247 + ) -> axum::response::Response {
248 + use axum::response::IntoResponse;
249 + // Liveness is observable to any caller, but the grant detail (what this host
250 + // can do) is only disclosed to an authenticated, allow-listed caller — an
251 + // unknown peer learns the agent is up, nothing more.
252 + let (actuate, observe) = match resolve_caller(&state, peer.ip()).await {
253 + Ok((_, effective)) => (
254 + effective.actuate_tokens().map(String::from).collect(),
255 + effective.observe_kinds().map(|k| k.token()).collect(),
256 + ),
257 + Err(_) => (Vec::new(), Vec::new()),
258 + };
259 + axum::Json(HealthResponse { ok: true, actuate, observe }).into_response()
210 260 }
211 261
212 262 async fn run(
@@ -217,36 +267,22 @@ async fn run(
217 267 use axum::http::StatusCode;
218 268 use axum::response::IntoResponse;
219 269
220 - // Identity → authorization (agent-side enforcement).
221 - let identity = match (state.whois)(peer.ip()).await {
222 - Ok(id) => id,
223 - Err(e) => {
224 - return (StatusCode::FORBIDDEN, format!("whois failed: {e}")).into_response();
225 - }
270 + // Identity → authorization (agent-side enforcement). Note: this authorizes
271 + // the action *label*; the step's argv runs verbatim once permitted (see the
272 + // trust-boundary note at the top of this module).
273 + let (identity, effective) = match resolve_caller(&state, peer.ip()).await {
274 + Ok(pair) => pair,
275 + Err(resp) => return resp,
226 276 };
227 - match authorize(&state.config, &identity, &req.step.action) {
228 - AuthDecision::Allow => {}
229 - AuthDecision::UnknownCaller => {
230 - return (StatusCode::FORBIDDEN, format!("unknown caller: {}", identity.node)).into_response();
231 - }
232 - AuthDecision::Denied => {
233 - return (
234 - StatusCode::FORBIDDEN,
235 - format!("action `{:?}` denied for {}", req.step.action, identity.node),
236 - )
237 - .into_response();
238 - }
277 + if !effective.permits(&req.step.action) {
278 + return (
279 + StatusCode::FORBIDDEN,
280 + format!("action `{:?}` denied for {}", req.step.action, identity.node),
281 + )
282 + .into_response();
239 283 }
240 284
241 285 // Run locally under the effective grant, streaming NDJSON frames back.
242 - let effective = state
243 - .config
244 - .allow
245 - .iter()
246 - .find(|c| identity.matches(&c.identity))
247 - .map(|c| c.to_caps().intersect(&state.config.grant.to_caps()))
248 - .unwrap_or_default();
249 -
250 286 let (tx, rx) = tokio::sync::mpsc::channel::<Result<axum::body::Bytes, std::io::Error>>(64);
251 287 tokio::spawn(async move {
252 288 let exec = LocalExec::new(effective);
@@ -267,16 +303,74 @@ struct PullQuery {
267 303 path: PathBuf,
268 304 }
269 305
306 + /// Confine `requested` to `root`: both are canonicalized (resolving `..` and
307 + /// symlinks), and the result must lie under `root`. Returns `None` on any
308 + /// escape or if the path does not resolve (caller maps that to 403/404).
309 + fn confine_to_root(root: &Path, requested: &Path) -> Option<PathBuf> {
310 + // Reject obviously-hostile shapes before hitting the filesystem.
311 + if requested.components().any(|c| matches!(c, Component::ParentDir)) {
312 + return None;
313 + }
314 + let canon_root = root.canonicalize().ok()?;
315 + let canon = requested.canonicalize().ok()?;
316 + canon.starts_with(&canon_root).then_some(canon)
317 + }
318 +
270 319 async fn pull(
271 - axum::extract::State(_state): axum::extract::State<AgentState>,
320 + axum::extract::State(state): axum::extract::State<AgentState>,
321 + axum::extract::ConnectInfo(peer): axum::extract::ConnectInfo<SocketAddr>,
272 322 axum::extract::Query(q): axum::extract::Query<PullQuery>,
273 323 ) -> axum::response::Response {
274 324 use axum::http::StatusCode;
275 325 use axum::response::IntoResponse;
276 - match tokio::fs::read(&q.path).await {
277 - Ok(bytes) => bytes.into_response(),
278 - Err(e) => (StatusCode::NOT_FOUND, format!("pull {}: {e}", q.path.display())).into_response(),
326 +
327 + // Identity → authorization. Reading host files is an observe-plane action,
328 + // gated by the `build-log` observe grant.
329 + let (_identity, effective) = match resolve_caller(&state, peer.ip()).await {
330 + Ok(pair) => pair,
331 + Err(resp) => return resp,
332 + };
333 + if !effective.permits_observe(&ObserveKind::BuildLog) {
334 + return (StatusCode::FORBIDDEN, "pull denied: requires `build-log` observe grant").into_response();
279 335 }
336 +
337 + // Confine to the configured artifacts root. No root configured ⇒ disabled.
338 + let Some(root) = &state.config.pull_root else {
339 + return (StatusCode::FORBIDDEN, "pull disabled: no pull_root configured").into_response();
340 + };
341 + let Some(path) = confine_to_root(root, &q.path) else {
342 + return (StatusCode::NOT_FOUND, format!("pull {}: not found under pull_root", q.path.display()))
343 + .into_response();
344 + };
345 +
346 + // Stream the file in chunks rather than buffering it whole (artifacts can be
347 + // hundreds of MB) — same channel/Body pattern as `/run`.
348 + let file = match tokio::fs::File::open(&path).await {
349 + Ok(f) => f,
350 + Err(e) => return (StatusCode::NOT_FOUND, format!("pull {}: {e}", path.display())).into_response(),
351 + };
352 + let (tx, rx) = tokio::sync::mpsc::channel::<Result<axum::body::Bytes, std::io::Error>>(8);
353 + tokio::spawn(async move {
354 + use tokio::io::AsyncReadExt;
355 + let mut file = file;
356 + let mut buf = vec![0u8; 64 * 1024];
357 + loop {
358 + match file.read(&mut buf).await {
359 + Ok(0) => break,
360 + Ok(n) => {
361 + if tx.send(Ok(axum::body::Bytes::copy_from_slice(&buf[..n]))).await.is_err() {
362 + break;
363 + }
364 + }
365 + Err(e) => {
366 + let _ = tx.send(Err(e)).await;
367 + break;
368 + }
369 + }
370 + }
371 + });
372 + let stream = tokio_stream::wrappers::ReceiverStream::new(rx);
373 + axum::body::Body::from_stream(stream).into_response()
280 374 }
281 375
282 376 #[cfg(test)]
@@ -294,8 +388,9 @@ mod tests {
294 388 allow: vec![CallerGrant {
295 389 identity: "fw13".into(),
296 390 actuate: vec!["build".into(), "sign".into(), "notarize".into(), "staple".into()],
297 - observe: vec![],
391 + observe: vec!["build-log".into()],
298 392 }],
393 + pull_root: None,
299 394 }
300 395 }
301 396
@@ -337,4 +432,41 @@ mod tests {
337 432 let tagged = CallerIdentity { node: "whatever".into(), tags: vec!["tag:builder".into()] };
338 433 assert_eq!(authorize(&c, &tagged, &Action::Sign), AuthDecision::Allow);
339 434 }
435 +
436 + #[test]
437 + fn pull_requires_a_known_caller_with_build_log_observe() {
438 + // An unknown caller has no effective grant at all.
439 + let stranger = CallerIdentity { node: "stranger".into(), tags: vec![] };
440 + assert!(effective_grant(&cfg(), &stranger).is_none());
441 +
442 + // A known caller granted only actuate (no build-log observe) cannot pull.
443 + let mut c = cfg();
444 + c.allow[0].observe = vec![];
445 + c.grant.observe = vec![];
446 + let eff = effective_grant(&c, &fw13()).unwrap();
447 + assert!(!eff.permits_observe(&ObserveKind::BuildLog), "no observe grant ⇒ pull denied");
448 +
449 + // The default fixture grants build-log on both sides ⇒ pull permitted.
450 + let eff = effective_grant(&cfg(), &fw13()).unwrap();
451 + assert!(eff.permits_observe(&ObserveKind::BuildLog));
452 + }
453 +
454 + #[test]
455 + fn confine_rejects_traversal_and_escape() {
456 + let dir = tempfile::tempdir().unwrap();
457 + let root = dir.path().join("artifacts");
458 + std::fs::create_dir_all(&root).unwrap();
459 + std::fs::write(root.join("ok.bin"), b"x").unwrap();
460 + // A secret one level above the root.
461 + std::fs::write(dir.path().join("secret"), b"s").unwrap();
462 +
463 + // In-root file resolves.
464 + assert!(confine_to_root(&root, &root.join("ok.bin")).is_some());
465 + // `..` escape is rejected.
466 + assert!(confine_to_root(&root, &root.join("../secret")).is_none());
467 + // Absolute path outside the root is rejected.
468 + assert!(confine_to_root(&root, &dir.path().join("secret")).is_none());
469 + // A non-existent in-root path does not resolve (caller maps to 404).
470 + assert!(confine_to_root(&root, &root.join("missing")).is_none());
471 + }
340 472 }
M shared/ops-exec/src/bin/ops-agent.rs +44 -1
@@ -10,7 +10,7 @@
10 10
11 11 use anyhow::{Context, Result};
12 12 use ops_exec::agent::{AgentConfig, AgentState, router};
13 - use std::net::SocketAddr;
13 + use std::net::{IpAddr, SocketAddr};
14 14
15 15 #[tokio::main]
16 16 async fn main() -> Result<()> {
@@ -24,6 +24,13 @@ async fn main() -> Result<()> {
24 24 let config: AgentConfig = toml::from_str(&raw).context("parsing config toml")?;
25 25 let listen = config.listen;
26 26
27 + // The agent runs arbitrary commands under a caller's grant and can read
28 + // files via /pull; its only perimeter is the tailnet allow-list. Refuse to
29 + // bind a public interface so the allow-list can never be bypassed by a route
30 + // off the tailnet (a misconfigured `listen = "0.0.0.0:..."` is a hard error).
31 + ensure_tailnet_or_loopback(listen.ip())
32 + .with_context(|| format!("invalid listen address {listen}"))?;
33 +
27 34 tracing::info!(%listen, allow = config.allow.len(), "ops-agent starting");
28 35 let state = AgentState::new(config);
29 36 let app = router(state);
@@ -40,6 +47,25 @@ async fn main() -> Result<()> {
40 47 Ok(())
41 48 }
42 49
50 + /// Permit only loopback or tailnet addresses: Tailscale's CGNAT range
51 + /// `100.64.0.0/10` (IPv4) and ULA `fc00::/7` (covers Tailscale's `fd7a::/16`).
52 + /// A public or unspecified (`0.0.0.0`) bind is rejected.
53 + fn ensure_tailnet_or_loopback(ip: IpAddr) -> Result<()> {
54 + let ok = match ip {
55 + IpAddr::V4(v4) => {
56 + let o = v4.octets();
57 + v4.is_loopback() || (o[0] == 100 && (64..=127).contains(&o[1]))
58 + }
59 + IpAddr::V6(v6) => v6.is_loopback() || (v6.segments()[0] & 0xfe00) == 0xfc00,
60 + };
61 + anyhow::ensure!(
62 + ok,
63 + "ops-agent must listen on a tailnet (100.64.0.0/10 or fc00::/7) or loopback address, \
64 + never a public or unspecified interface; got {ip}"
65 + );
66 + Ok(())
67 + }
68 +
43 69 fn env_filter() -> tracing_subscriber::EnvFilter {
44 70 tracing_subscriber::EnvFilter::try_from_default_env()
45 71 .unwrap_or_else(|_| tracing_subscriber::EnvFilter::new("info"))
@@ -59,3 +85,20 @@ fn parse_config_arg() -> Option<String> {
59 85 }
60 86 None
61 87 }
88 +
89 + #[cfg(test)]
90 + mod tests {
91 + use super::*;
92 +
93 + #[test]
94 + fn accepts_loopback_and_tailnet_rejects_public() {
95 + let ok = |s: &str| ensure_tailnet_or_loopback(s.parse().unwrap()).is_ok();
96 + assert!(ok("127.0.0.1"));
97 + assert!(ok("100.103.89.95")); // Tailscale CGNAT
98 + assert!(ok("::1"));
99 + assert!(ok("fd7a:115c:a1e0::1")); // Tailscale ULA
100 + assert!(!ok("0.0.0.0")); // unspecified — would expose off-tailnet
101 + assert!(!ok("192.168.1.10")); // LAN
102 + assert!(!ok("1.2.3.4")); // public
103 + }
104 + }
M shared/ops-exec/src/transport.rs +29
@@ -50,6 +50,19 @@ fn gate(caps: &CapabilitySet, host: &str, step: &Step) -> Result<()> {
50 50 }
51 51 }
52 52
53 + /// An env *value* is sh-quoted at render time, but the *name* is interpolated
54 + /// verbatim (`NAME=value`), so a name with shell metacharacters could inject a
55 + /// command. Require each name to be a bare shell identifier before dispatch.
56 + fn validate_env_names(step: &Step) -> Result<()> {
57 + for (k, _) in &step.env {
58 + let valid = !k.is_empty()
59 + && k.chars().next().is_some_and(|c| c == '_' || c.is_ascii_alphabetic())
60 + && k.chars().all(|c| c == '_' || c.is_ascii_alphanumeric());
61 + anyhow::ensure!(valid, "env name `{k}` must be a shell identifier ([A-Za-z_][A-Za-z0-9_]*)");
62 + }
63 + Ok(())
64 + }
65 +
53 66 /// Build the rsync `Command` shared by local and ssh push/pull. `remote_spec`
54 67 /// is either a plain path (local) or `target:path` (ssh).
55 68 fn rsync_command(src: &str, dst: &str, over_ssh: bool, opts: &SyncOpts) -> Command {
@@ -94,6 +107,7 @@ impl LocalExec {
94 107 impl Executor for LocalExec {
95 108 async fn run_streaming(&self, step: &Step, sink: &mut dyn LogSink) -> Result<RunOutput> {
96 109 gate(&self.caps, "local", step)?;
110 + validate_env_names(step)?;
97 111 let cmd = self.host.command(&render_shell_line(step));
98 112 run_command_into_sink(cmd, sink).await
99 113 }
@@ -134,6 +148,7 @@ impl SshExec {
134 148 impl Executor for SshExec {
135 149 async fn run_streaming(&self, step: &Step, sink: &mut dyn LogSink) -> Result<RunOutput> {
136 150 gate(&self.caps, self.host.ssh_target(), step)?;
151 + validate_env_names(step)?;
137 152 let cmd = self.host.command(&render_shell_line(step));
138 153 run_command_into_sink(cmd, sink).await
139 154 }
@@ -188,6 +203,20 @@ mod tests {
188 203 }
189 204
190 205 #[tokio::test]
206 + async fn rejects_malicious_env_name_before_dispatch() {
207 + let dir = tempfile::tempdir().unwrap();
208 + let marker = dir.path().join("pwned");
209 + let exec = LocalExec::new(CapabilitySet::actuate_only([Action::Build]));
210 + let mut sink = vec_sink();
211 + // A shell-metacharacter env name would otherwise inject a command.
212 + let step = Step::new(Action::Build, ["true"])
213 + .with_env(format!("X; touch {}", marker.display()), "v");
214 + let err = exec.run_streaming(&step, &mut sink).await.unwrap_err();
215 + assert!(err.to_string().contains("shell identifier"));
216 + assert!(!marker.exists(), "rejected env name must not execute");
217 + }
218 +
219 + #[tokio::test]
191 220 async fn local_exec_runs_granted_step() {
192 221 let exec = LocalExec::new(CapabilitySet::actuate_only([Action::Deploy]));
193 222 let mut sink = vec_sink();
M shared/ops-exec/tests/agent_e2e.rs +73 -6
@@ -7,6 +7,7 @@
7 7 use ops_exec::agent::{AgentConfig, AgentState, CallerGrant, CallerIdentity, GrantConfig, router};
8 8 use ops_exec::{Action, AgentRpc, CapabilityDenied, CapabilitySet, Executor, LogSink, Step};
9 9 use std::net::SocketAddr;
10 + use std::path::PathBuf;
10 11 use std::sync::Arc;
11 12
12 13 #[derive(Default)]
@@ -21,7 +22,16 @@ impl LogSink for VecSink {
21 22 /// Spin the agent on an ephemeral loopback port; whois always says the caller
22 23 /// is `fw13`. Returns the base URL.
23 24 async fn spawn_agent(allow: Vec<CallerGrant>, grant: GrantConfig) -> String {
24 - let config = AgentConfig { listen: "127.0.0.1:0".parse().unwrap(), grant, allow };
25 + spawn_agent_with_pull(allow, grant, None).await
26 + }
27 +
28 + /// As [`spawn_agent`], but with a configurable `pull_root` for the file-read path.
29 + async fn spawn_agent_with_pull(
30 + allow: Vec<CallerGrant>,
31 + grant: GrantConfig,
32 + pull_root: Option<PathBuf>,
33 + ) -> String {
34 + let config = AgentConfig { listen: "127.0.0.1:0".parse().unwrap(), grant, allow, pull_root };
25 35 let state = AgentState {
26 36 config: Arc::new(config),
27 37 whois: Arc::new(|_ip| {
@@ -110,15 +120,24 @@ async fn caller_side_gate_rejects_before_round_trip() {
110 120 assert!(err.downcast_ref::<CapabilityDenied>().is_some(), "expected caller-side CapabilityDenied");
111 121 }
112 122
123 + /// An allow-listed caller with a `build-log` observe grant can pull a file that
124 + /// lives under the configured `pull_root`.
113 125 #[tokio::test]
114 - async fn agent_pull_serves_a_file() {
126 + async fn agent_pull_serves_an_in_root_file() {
115 127 let dir = tempfile::tempdir().unwrap();
116 - let artifact = dir.path().join("GoingsOn.dmg");
128 + let root = dir.path().join("dist");
129 + tokio::fs::create_dir_all(&root).await.unwrap();
130 + let artifact = root.join("GoingsOn.dmg");
117 131 tokio::fs::write(&artifact, b"DMGBYTES").await.unwrap();
118 132
119 - let base = spawn_agent(
120 - vec![CallerGrant { identity: "fw13".into(), actuate: vec![], observe: vec![] }],
121 - builder_grant(),
133 + let grant = GrantConfig {
134 + actuate: vec!["build".into()],
135 + observe: vec!["build-log".into()],
136 + };
137 + let base = spawn_agent_with_pull(
138 + vec![CallerGrant { identity: "fw13".into(), actuate: vec![], observe: vec!["build-log".into()] }],
139 + grant,
140 + Some(root.clone()),
122 141 )
123 142 .await;
124 143 let rpc = AgentRpc::new(base, "mbp", CapabilitySet::default());
@@ -126,3 +145,51 @@ async fn agent_pull_serves_a_file() {
126 145 rpc.pull(&artifact, &local, &Default::default()).await.unwrap();
127 146 assert_eq!(tokio::fs::read(&local).await.unwrap(), b"DMGBYTES");
128 147 }
148 +
149 + /// A caller without a `build-log` observe grant is refused, even for an in-root
150 + /// file — pull is gated, not open.
151 + #[tokio::test]
152 + async fn agent_pull_denied_without_observe_grant() {
153 + let dir = tempfile::tempdir().unwrap();
154 + let root = dir.path().join("dist");
155 + tokio::fs::create_dir_all(&root).await.unwrap();
156 + let artifact = root.join("a.bin");
157 + tokio::fs::write(&artifact, b"x").await.unwrap();
158 +
159 + let base = spawn_agent_with_pull(
160 + vec![CallerGrant { identity: "fw13".into(), actuate: vec!["build".into()], observe: vec![] }],
161 + builder_grant(),
162 + Some(root.clone()),
163 + )
164 + .await;
165 + let rpc = AgentRpc::new(base, "mbp", CapabilitySet::default());
166 + let local = dir.path().join("out.bin");
167 + // The agent returns 403; AgentRpc surfaces it as a non-2xx status error and
168 + // the file never transfers.
169 + assert!(rpc.pull(&artifact, &local, &Default::default()).await.is_err());
170 + assert!(!local.exists(), "denied pull must not write a file");
171 + }
172 +
173 + /// A path outside `pull_root` is refused even for an authorized caller — the
174 + /// confinement boundary holds against traversal.
175 + #[tokio::test]
176 + async fn agent_pull_denied_outside_root() {
177 + let dir = tempfile::tempdir().unwrap();
178 + let root = dir.path().join("dist");
179 + tokio::fs::create_dir_all(&root).await.unwrap();
180 + let secret = dir.path().join("secret.key");
181 + tokio::fs::write(&secret, b"topsecret").await.unwrap();
182 +
183 + let grant = GrantConfig { actuate: vec![], observe: vec!["build-log".into()] };
184 + let base = spawn_agent_with_pull(
185 + vec![CallerGrant { identity: "fw13".into(), actuate: vec![], observe: vec!["build-log".into()] }],
186 + grant,
187 + Some(root.clone()),
188 + )
189 + .await;
190 + let rpc = AgentRpc::new(base, "mbp", CapabilitySet::default());
191 + let local = dir.path().join("out.key");
192 + let err = rpc.pull(&secret, &local, &Default::default()).await.unwrap_err();
193 + assert!(!local.exists(), "out-of-root pull must not write a file");
194 + let _ = err; // status is non-2xx; the point is the secret never transfers
195 + }