From 98793db54ae1b7d6d2c6512ffffec163e79073dd Mon Sep 17 00:00:00 2001 From: Cuihtlauac ALVARADO Date: Fri, 16 Oct 2026 18:31:27 +0200 Subject: [PATCH 01/14] feat(plan): structured composite-execution plans (retire the bash -lc wrapper) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generalize a request's flat `pipeline` into a `Plan` tree the daemon executes directly — fork/exec per stage with pipes, redirections, env, per-stage run-as, and sequencing — and no shell on the privileged path. The structure the human approves at the prompt is literally the structure that runs. Schema (src/protocol.rs): a JSON-native, `op`-tagged `Plan` enum — run, pipe, seq, redirect (incl. append and 2>&1), with-stdin-from, setenv, chdir, run-as ([ROOT]/[as user]), with-accepted-exit-codes, and an explicit `system` opaque escape hatch. A recursive `validate_tree` reuses the existing has_dangerous_chars gate on every string leaf and adds depth/node bounds, all inside the single ValidatedRequest::validate constructor so the typestate guarantee is intact. A flat `pipeline` desugars into a Plan, so every existing client keeps working; effective_privileged = privileged || plan-contains-run-as closes the "unprivileged request smuggling root" gap. Executor (src/executor.rs): two passes — a pure compile (threads env/cwd/run-as/ stdio down the tree into a flat leaf list + sequencing tree) and a run pass (pipe groups, seq short-circuit, one plan-wide deadline). Escalated leaves spawn as `sudo [-u user] -- ` / `pkexec [--user u] argv`, never `sudo sh -c`; pipeline_to_shell and the pkexec sh -c branch are deleted. setenv is held to the same env allowlist so it cannot become a bypass. Prompt (src/tui.rs): every stage renders with its run-as tag and its own resolved binary (fixing the vacuous single /usr/bin/bash resolves line); system stages are flagged opaque; a stage-count cap keeps the [y/N] question on-screen. A single plain unprivileged command keeps the classic one-line form. Wiring (src/server.rs, src/mcp.rs): dispatch keys off effective privilege; structurally-invalid plans are rejected before prompting; audit logs are plan-aware; the MCP execute tool gained a `plan` parameter documented with the seq-short-circuit and sudo-in-pipe caveats. Tests: Rung-2 properties (render-vs-execute structural identity over random plans, compile totality, a structural no-shell-on-privileged-path check), a Kani leaf harness, tests/plan_exec.rs end-to-end, and wire-level plan validation tests. backlog.md: captures the partial-seq retry hazard and the sudo-credential-in-pipe caveat as a follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) --- backlog.md | 42 ++ src/executor.rs | 1450 ++++++++++++++++++++++++++++++++----------- src/mcp.rs | 123 +++- src/proofs.rs | 33 + src/protocol.rs | 498 ++++++++++++++- src/server.rs | 36 +- src/tui.rs | 269 +++++++- tests/common/mod.rs | 1 + tests/exec.rs | 1 + tests/plan_exec.rs | 203 ++++++ tests/validation.rs | 96 +++ 11 files changed, 2323 insertions(+), 429 deletions(-) create mode 100644 tests/plan_exec.rs diff --git a/backlog.md b/backlog.md index c8e9831..de871ba 100644 --- a/backlog.md +++ b/backlog.md @@ -215,6 +215,48 @@ daemon failing open must never be the difference between safe and unsafe; the hook failing open (misconfig, different client) likewise must not — the daemon is the backstop. Captured follow-up, not active work. +## Partial-`seq` retry hazard and the sudo-credential-in-pipe caveat + +Surfaced while reviewing the structured-plan executor (the plan model above, +first increment landed). Two interacting behaviours deserve a safer story than +"the caller must know." + +**The retry hazard.** A `seq` runs its steps in order and stops at the first +non-accepted failure (`executor.rs`, `run_seq` → `SeqNode::Seq`). When a later +step fails, the earlier steps' side effects are **already committed**. The +response marks which stages ran (per-stage exit codes; a skipped stage carries +the `-1` not-run sentinel), but nothing stops a caller from resubmitting the +*same* plan — which replays the already-committed early steps. For +non-idempotent effects (`mkdir`, append-redirects, counters, `git commit`) that +double-applies or errors on the second pass. This is the classic +partial-failure/retry footgun of any `set -e` script; the plan model makes the +partial state *visible* but does not prevent the replay. + +**The sudo-credential compounding.** The most likely "later step fails" cause is +a privileged stage with no cached sudo credential. A **single** privileged stage +runs with `setpgid` + the foreground-tty guard and *can* prompt; a privileged +stage **inside a pipe** runs with `setsid` (detached, no controlling tty) and +**cannot** prompt — it relies on a cached credential. Whether a leading +`sudo true`-style prime carries into a `setsid` pipe child depends on the host's +sudoers `timestamp_type` (`global` → yes; the common `tty` default → likely +not). So a pipe whose privileged stage is neither NOPASSWD nor already-cached +fails fast with "no tty present," and credential-priming is not a reliable fix +*inside a pipe*. + +**Mitigations available today** (caller-side): order privileged stages first; +make early stages idempotent (`mkdir -p`, truncate not append); rely on +NOPASSWD / cached creds; prefer a single privileged stage over `sudo`-inside-a- +pipe; and use the per-stage exit-code array to resubmit only the not-run stages +rather than replaying the whole plan. + +**Potential improvements to weigh** (none a full fix — idempotency stays the +caller's responsibility): (1) surface the partial-execution boundary +prominently in the response/prompt so a caller resumes rather than replays; +(2) an optional executor-inserted auth-probe for privileged plans that fails +fast *before* any stateful stage, documenting the `timestamp_type` caveat; +(3) advisory flagging of obviously-non-idempotent leading stages. Captured +follow-up, not active work. + --- ## Rung 6 — seL4-style end-to-end refinement (aspirational) diff --git a/src/executor.rs b/src/executor.rs index c6e9128..dacead6 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::fs::{File, OpenOptions}; use std::io::Read; use std::os::unix::fs::PermissionsExt; use std::os::unix::process::CommandExt; @@ -10,7 +11,9 @@ use std::time::{Duration, Instant}; use base64::engine::general_purpose::STANDARD as B64; use base64::Engine; -use crate::protocol::{Request, Response, StageResult, ValidatedRequest}; +use crate::protocol::{ + Plan, RedirTarget, Response, RunAs, Shell, StageResult, Stream, ValidatedRequest, WriteMode, +}; /// Hard cap on captured stdout/stderr per stream. A privileged /// `cat /dev/zero` would otherwise OOM the daemon. When the cap is @@ -133,172 +136,667 @@ pub fn which(name: &str) -> Option { None } -/// Execute a pipeline via pkexec (local/graphical mode). -/// For multi-stage pipelines, wraps in `pkexec sh -c 'cmd1 | cmd2 | ...'`. +/// Top-level escalation mechanism chosen by the dispatcher. It governs how an +/// *escalated* leaf (one running as root or a named user) is spawned; a leaf +/// running as the daemon's own user is always spawned directly, regardless of +/// mode. `Direct` means the request is unprivileged — every leaf runs as the +/// daemon user. +#[derive(Clone, Copy, PartialEq, Eq)] +pub enum EscalationMode { + Direct, + Sudo, + Pkexec, +} + +/// Execute a request's plan under pkexec (local/graphical mode). pub fn exec_pkexec( req: &ValidatedRequest, env: &HashMap, tty_lock: &Mutex<()>, ) -> Response { - if req.pipeline.len() == 1 { - return exec_pkexec_stage(&req.pipeline[0], env, &req.id, tty_lock); + run_plan(req, env, EscalationMode::Pkexec, Some(tty_lock)) +} + +/// Execute a request's plan under sudo (remote/TUI mode). +pub fn exec_sudo( + req: &ValidatedRequest, + env: &HashMap, + tty_lock: &Mutex<()>, +) -> Response { + run_plan(req, env, EscalationMode::Sudo, Some(tty_lock)) +} + +/// Execute a request's plan directly as the current user (no escalation). +pub fn exec_direct(req: &ValidatedRequest, env: &HashMap) -> Response { + run_plan(req, env, EscalationMode::Direct, None) +} + +// --------------------------------------------------------------------------- +// Structured plan execution +// +// A request's command is canonically a `Plan` tree (`Request::exec_plan`; a +// flat `pipeline` desugars into one). Execution is two passes: +// +// Pass A (`compile_plan`): a pure walk that threads an immutable context +// (env / cwd / run-as / accepted-exit-codes / stdio) down the tree. Context +// modifiers (setenv, chdir, run-as, with-accepted-exit-codes) and stdio +// modifiers (redirect, with-stdin-from) are transparent — they adjust the +// context and recurse. Only `run`/`system` leaves produce a process. The +// output is a flat `Vec` (one per leaf, in pre-order = prompt +// order) plus a `SeqNode` sequencing tree that references leaves by index. +// +// Pass B (`run_seq`): interprets the sequencing tree — a `Pipe` group spawns +// its leaves connected by pipes, a `Seq` (the `seq` op) runs children in order and +// stops at the first non-accepted exit, a `Leaf` runs one process. Every +// process shares one plan-wide deadline. +// +// No shell is ever assembled on the privileged path: each leaf fork/execs its +// literal argv, and an escalated leaf is `sudo [-u user] -- argv` / `pkexec +// [--user user] argv`, never `sudo sh -c`. The one exception is an explicit +// `System` leaf — the named, prompt-flagged opaque escape hatch. +// --------------------------------------------------------------------------- + +/// Per-leaf run-as context, resolved from the ambient `privileged` flag and any +/// enclosing `RunAs` node. +#[derive(Clone, Debug, PartialEq, Eq)] +enum LeafRunAs { + /// Run as the daemon's own user (no escalation). + Direct, + Root, + User(String), +} + +impl LeafRunAs { + fn is_escalated(&self) -> bool { + !matches!(self, LeafRunAs::Direct) } +} + +#[derive(Clone, Debug)] +enum InSpec { + Null, + File(String), +} + +#[derive(Clone, Debug, PartialEq, Eq)] +enum OutSpec { + /// Captured into the response (piped + drained). + Capture, + File { path: String, append: bool }, + DevNull, +} + +#[derive(Clone, Debug)] +enum ErrSpec { + Capture, + File { path: String, append: bool }, + DevNull, + /// `2>&1` — stderr follows wherever stdout points. + ToStdout, +} + +/// One fully-resolved process: the leaf of a compiled plan. +struct LeafPlan { + argv: Vec, + env: HashMap, + cwd: String, + runas: LeafRunAs, + stdin: InSpec, + stdout: OutSpec, + stderr: ErrSpec, + /// True for a `System` (shell escape-hatch) leaf. The prompt flags it as + /// opaque; `resolved_bin` resolves only the shell. + opaque: bool, + /// `argv[0]` resolved through PATH + canonicalize, for the prompt. Computed + /// in Pass A so the prompt and the executor can never disagree. + resolved_bin: Option, + /// Exit codes treated as success (besides 0) for seq short-circuiting. + accepted: Vec, + forward_agent: bool, + /// Pre-order index == position in the response's `stages` and in the + /// prompt render. + stage_id: usize, +} + +/// The sequencing skeleton over the leaf list. +enum SeqNode { + Leaf(usize), + Pipe(Vec), + Seq(Vec), +} + +/// Immutable compile context threaded down the tree. +#[derive(Clone)] +struct Ctx { + env: HashMap, + cwd: String, + runas: LeafRunAs, + accepted: Vec, + forward_agent: bool, + stdin: InSpec, + stdout: OutSpec, + stderr: ErrSpec, +} - // Multi-stage: wrap entire pipeline in sh -c via pkexec - let shell_cmd = pipeline_to_shell(&req.pipeline, env); - let mut cmd = Command::new("pkexec"); - cmd.args(["sh", "-c", &shell_cmd]); - cmd.current_dir("/"); - cmd.stdin(Stdio::null()); - cmd.stdout(Stdio::piped()); - cmd.stderr(Stdio::piped()); - cmd.env_clear(); - // pkexec needs DISPLAY/WAYLAND_DISPLAY to show its dialog - if let Ok(v) = std::env::var("DISPLAY") { - cmd.env("DISPLAY", v); +impl Ctx { + fn stdio_is_default(&self) -> bool { + matches!(self.stdin, InSpec::Null) + && self.stdout == OutSpec::Capture + && matches!(self.stderr, ErrSpec::Capture) } - if let Ok(v) = std::env::var("WAYLAND_DISPLAY") { - cmd.env("WAYLAND_DISPLAY", v); +} + +struct Compiled { + leaves: Vec, + seq: SeqNode, +} + +/// Compile `req.exec_plan()` into leaves + a sequencing tree. Ambient run-as is +/// seeded from `req.privileged` (root when set, else the daemon user); `RunAs` +/// nodes override it per-subtree. +fn compile_plan(req: &ValidatedRequest, env: &HashMap) -> Result { + let ambient = if req.privileged { + LeafRunAs::Root + } else { + LeafRunAs::Direct + }; + let ctx = Ctx { + env: env.clone(), + cwd: "/".to_string(), + runas: ambient, + accepted: Vec::new(), + forward_agent: req.forward_agent, + stdin: InSpec::Null, + stdout: OutSpec::Capture, + stderr: ErrSpec::Capture, + }; + let plan = req.exec_plan(); + let mut leaves = Vec::new(); + let seq = compile_seq(&plan, ctx, &mut leaves)?; + Ok(Compiled { leaves, seq }) +} + +/// Apply a `Setenv` node to a context, holding it to the same env allowlist as +/// the request-level env so it cannot become a bypass. +fn apply_setenv(ctx: &mut Ctx, key: &str, value: &str) -> Result<(), String> { + if key == "SSH_AUTH_SOCK" { + return Err("SSH_AUTH_SOCK cannot be set by setenv; use forward_agent instead".to_string()); } - if let Ok(v) = std::env::var("XAUTHORITY") { - cmd.env("XAUTHORITY", v); + if !is_env_allowed(key) { + return Err(format!("environment variable not allowed: {key}")); } + ctx.env.insert(key.to_string(), value.to_string()); + Ok(()) +} - run_single_command(&mut cmd, &req.id, exec_timeout(), Some(tty_lock)) +/// Apply a `RunAs` node to a context. +fn apply_runas(ctx: &mut Ctx, who: &RunAs) { + ctx.runas = match who { + RunAs::Root => LeafRunAs::Root, + RunAs::User { name } => LeafRunAs::User(name.clone()), + }; + // An escalated subtree never carries the daemon's agent socket. + ctx.forward_agent = false; } -/// Execute a pipeline via sudo (remote/TUI mode). -pub fn exec_sudo( - req: &ValidatedRequest, - env: &HashMap, - tty_lock: &Mutex<()>, -) -> Response { - if req.pipeline.len() == 1 { - return exec_sudo_stage(&req.pipeline[0], env, &req.id, tty_lock); +/// Compile a plan that may expand to a sequence / pipe / single process. The +/// *context* modifiers (setenv / chdir / run-as / with-accepted-exit-codes) are +/// transparent and may wrap a composite — they adjust the context and recurse. +/// The *stdio* modifiers (redirect / with-stdin-from) target a single process, +/// so they (and bare leaves) are delegated to `compile_leaf`. +fn compile_seq(plan: &Plan, mut ctx: Ctx, leaves: &mut Vec) -> Result { + match plan { + Plan::Setenv { key, value, body } => { + apply_setenv(&mut ctx, key, value)?; + compile_seq(body, ctx, leaves) + } + Plan::Chdir { dir, body } => { + ctx.cwd = dir.clone(); + compile_seq(body, ctx, leaves) + } + Plan::RunAs { who, body } => { + apply_runas(&mut ctx, who); + compile_seq(body, ctx, leaves) + } + Plan::WithAcceptedExitCodes { codes, body } => { + ctx.accepted = codes.clone(); + compile_seq(body, ctx, leaves) + } + Plan::Seq { steps } => { + if !ctx.stdio_is_default() { + return Err("redirect / stdin-from of a seq is not supported".to_string()); + } + let mut nodes = Vec::with_capacity(steps.len()); + for s in steps { + nodes.push(compile_seq(s, ctx.clone(), leaves)?); + } + Ok(SeqNode::Seq(nodes)) + } + Plan::Pipe { stages } => { + if !ctx.stdio_is_default() { + return Err("redirect / stdin-from of a pipe is not supported".to_string()); + } + compile_pipe(stages, ctx, leaves) + } + // Redirect / with-stdin-from / bare leaves — a single process. + _ => Ok(SeqNode::Leaf(compile_leaf(plan, ctx, leaves)?)), } +} - exec_pipeline(req.inner(), env, EscalationMode::Sudo) +/// Compile a plan that must resolve to exactly one process (a pipe stage, or a +/// redirect / stdin-from body). Peels the transparent context and stdio +/// modifiers and errors on a composite. +fn compile_leaf(plan: &Plan, mut ctx: Ctx, leaves: &mut Vec) -> Result { + match plan { + Plan::Run { argv } => Ok(push_leaf(argv.clone(), false, ctx, leaves)), + Plan::System { shell, script } => { + let bin = match shell { + Shell::Sh => "sh", + Shell::Bash => "bash", + }; + let argv = vec![bin.to_string(), "-c".to_string(), script.clone()]; + Ok(push_leaf(argv, true, ctx, leaves)) + } + Plan::Setenv { key, value, body } => { + apply_setenv(&mut ctx, key, value)?; + compile_leaf(body, ctx, leaves) + } + Plan::Chdir { dir, body } => { + ctx.cwd = dir.clone(); + compile_leaf(body, ctx, leaves) + } + Plan::RunAs { who, body } => { + apply_runas(&mut ctx, who); + compile_leaf(body, ctx, leaves) + } + Plan::WithAcceptedExitCodes { codes, body } => { + ctx.accepted = codes.clone(); + compile_leaf(body, ctx, leaves) + } + Plan::Redirect { stream, target, body } => { + apply_redirect(&mut ctx, *stream, target)?; + compile_leaf(body, ctx, leaves) + } + Plan::WithStdinFrom { path, body } => { + ctx.stdin = InSpec::File(path.clone()); + compile_leaf(body, ctx, leaves) + } + Plan::Seq { .. } | Plan::Pipe { .. } => { + Err("expected a single command here, found a composite plan".to_string()) + } + } } -/// Execute a pipeline directly as the current user (no privilege escalation). -pub fn exec_direct(req: &ValidatedRequest, env: &HashMap) -> Response { - if req.pipeline.len() == 1 { - return exec_direct_stage(&req.pipeline[0], env, &req.id, req.forward_agent); +fn apply_redirect(ctx: &mut Ctx, stream: Stream, target: &RedirTarget) -> Result<(), String> { + let out = |t: &RedirTarget| -> OutSpec { + match t { + RedirTarget::File { path, mode } => OutSpec::File { + path: path.clone(), + append: *mode == WriteMode::Append, + }, + RedirTarget::DevNull => OutSpec::DevNull, + } + }; + let err = |t: &RedirTarget| -> ErrSpec { + match t { + RedirTarget::File { path, mode } => ErrSpec::File { + path: path.clone(), + append: *mode == WriteMode::Append, + }, + RedirTarget::DevNull => ErrSpec::DevNull, + } + }; + match stream { + Stream::Stdout => ctx.stdout = out(target), + Stream::Stderr => ctx.stderr = err(target), + Stream::Both => { + // `&>file` / `with-outputs-to`: stdout to the target, stderr dup'd + // onto it (2>&1). This keeps a single file handle, not two racing + // truncating opens. + ctx.stdout = out(target); + ctx.stderr = ErrSpec::ToStdout; + } } + Ok(()) +} + +/// Append a resolved leaf to the list and return its index (= stage id). +fn push_leaf(argv: Vec, opaque: bool, ctx: Ctx, leaves: &mut Vec) -> usize { + let resolved_bin = argv.first().and_then(|name| which(name)); + let stage_id = leaves.len(); + leaves.push(LeafPlan { + argv, + env: ctx.env, + cwd: ctx.cwd, + runas: ctx.runas, + stdin: ctx.stdin, + stdout: ctx.stdout, + stderr: ctx.stderr, + opaque, + resolved_bin, + accepted: ctx.accepted, + forward_agent: ctx.forward_agent, + stage_id, + }); + stage_id +} - exec_pipeline(req.inner(), env, EscalationMode::Direct) +fn compile_pipe(stages: &[Plan], ctx: Ctx, leaves: &mut Vec) -> Result { + let n = stages.len(); + let mut indices = Vec::with_capacity(n); + for (pos, stage) in stages.iter().enumerate() { + let idx = compile_leaf(stage, ctx.clone(), leaves)?; + // The pipe owns the stdout of every non-final stage, so it must not be + // independently redirected. (Put the redirect on the final stage.) + let is_last = pos == n - 1; + if !is_last && leaves[idx].stdout != OutSpec::Capture { + return Err( + "cannot redirect stdout of a non-final pipe stage; redirect the last stage" + .to_string(), + ); + } + indices.push(idx); + } + Ok(SeqNode::Pipe(indices)) } // --------------------------------------------------------------------------- -// Single-stage helpers +// Display projection — what the approval prompt renders // --------------------------------------------------------------------------- -fn exec_pkexec_stage( - argv: &[String], - env: &HashMap, - id: &str, - tty_lock: &Mutex<()>, -) -> Response { - let mut cmd = Command::new("pkexec"); - for arg in argv { - cmd.arg(arg); - } - cmd.current_dir("/"); - cmd.stdin(Stdio::null()); - cmd.stdout(Stdio::piped()); - cmd.stderr(Stdio::piped()); - cmd.env_clear(); - for (k, v) in env { - cmd.env(k, v); +/// A leaf's run-as context for display at the prompt. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum LeafPrivilege { + /// Runs as the daemon's own user — no tag. + Direct, + /// `[ROOT]`. + Root, + /// `[as ]`. + User(String), +} + +/// A displayable projection of one compiled plan leaf. The approval prompt +/// renders these, so the human sees exactly the stages — resolved binary, +/// run-as, redirections — that the executor will run. Built by the same +/// `compile_plan` walk the executor uses, so the two cannot diverge. +pub struct LeafView { + pub stage_id: usize, + pub argv: Vec, + pub privilege: LeafPrivilege, + /// `argv[0]` resolved through PATH (for `System`, the shell). + pub resolved_bin: Option, + /// True for a `System` escape-hatch leaf — rendered "opaque". + pub opaque: bool, + /// Human-readable redirection summary, e.g. `> /tmp/out`, `< in`, `2>&1`. + pub redirects: String, + /// True if this leaf is a stage of a pipe. + pub in_pipe: bool, +} + +/// Compile a request's plan purely for display at the approval prompt. Uses an +/// empty env (the environment affects execution, not the rendered structure), +/// so the only errors are structural — the same ones the executor would hit. +pub fn plan_display(req: &ValidatedRequest) -> Result, String> { + let empty = HashMap::new(); + let compiled = compile_plan(req, &empty)?; + let mut in_pipe = vec![false; compiled.leaves.len()]; + mark_pipe_members(&compiled.seq, &mut in_pipe); + Ok(compiled + .leaves + .iter() + .map(|l| LeafView { + stage_id: l.stage_id, + argv: l.argv.clone(), + privilege: match &l.runas { + LeafRunAs::Direct => LeafPrivilege::Direct, + LeafRunAs::Root => LeafPrivilege::Root, + LeafRunAs::User(n) => LeafPrivilege::User(n.clone()), + }, + resolved_bin: l.resolved_bin.clone(), + opaque: l.opaque, + redirects: summarize_redirects(l), + in_pipe: in_pipe[l.stage_id], + }) + .collect()) +} + +fn mark_pipe_members(node: &SeqNode, in_pipe: &mut [bool]) { + match node { + SeqNode::Leaf(_) => {} + SeqNode::Pipe(indices) => { + for &i in indices { + in_pipe[i] = true; + } + } + SeqNode::Seq(nodes) => { + for n in nodes { + mark_pipe_members(n, in_pipe); + } + } } - if let Ok(v) = std::env::var("DISPLAY") { - cmd.env("DISPLAY", v); +} + +fn summarize_redirects(l: &LeafPlan) -> String { + let mut parts: Vec = Vec::new(); + if let InSpec::File(p) = &l.stdin { + parts.push(format!("< {p}")); + } + match &l.stdout { + OutSpec::Capture => {} + OutSpec::DevNull => parts.push("> /dev/null".to_string()), + OutSpec::File { path, append } => { + parts.push(format!("{} {path}", if *append { ">>" } else { ">" })) + } } - if let Ok(v) = std::env::var("WAYLAND_DISPLAY") { - cmd.env("WAYLAND_DISPLAY", v); + match &l.stderr { + ErrSpec::Capture => {} + ErrSpec::DevNull => parts.push("2> /dev/null".to_string()), + ErrSpec::ToStdout => parts.push("2>&1".to_string()), + ErrSpec::File { path, append } => { + parts.push(format!("2{} {path}", if *append { ">>" } else { ">" })) + } } - if let Ok(v) = std::env::var("XAUTHORITY") { - cmd.env("XAUTHORITY", v); + parts.join(" ") +} + +// --------------------------------------------------------------------------- +// Pass B — run the compiled plan +// --------------------------------------------------------------------------- + +/// Raw result of one leaf process. `ran == false` is a leaf that a short- +/// circuited seq never reached. +#[derive(Clone)] +struct Outcome { + ran: bool, + exit: i32, + stdout: Vec, + stdout_truncated: bool, + stderr: Vec, + stderr_truncated: bool, + /// Whether this leaf's stdout contributes to the aggregate response stdout + /// (true iff its stdout was `Capture`). + captured_stdout: bool, +} + +impl Default for Outcome { + fn default() -> Self { + Outcome { + ran: false, + exit: -1, + stdout: Vec::new(), + stdout_truncated: false, + stderr: Vec::new(), + stderr_truncated: false, + captured_stdout: false, + } } +} - run_single_command(&mut cmd, id, exec_timeout(), Some(tty_lock)) +/// Status a sequencing node reports up to its parent `seq`: enough to decide +/// whether to short-circuit the sequence. +struct StepStatus { + timed_out: bool, + spawn_error: Option, + success: bool, } -fn exec_sudo_stage( - argv: &[String], +fn run_plan( + req: &ValidatedRequest, env: &HashMap, - id: &str, - tty_lock: &Mutex<()>, + mode: EscalationMode, + tty_lock: Option<&Mutex<()>>, ) -> Response { - let mut cmd = Command::new("sudo"); - for (k, v) in env { - cmd.arg(format!("{k}={v}")); + let compiled = match compile_plan(req, env) { + Ok(c) => c, + Err(e) => return Response::error(&req.id, &format!("invalid plan: {e}")), + }; + let total = exec_timeout(); + let deadline = Instant::now() + total; + let mut outcomes = vec![Outcome::default(); compiled.leaves.len()]; + + let status = run_seq( + &compiled.seq, + &compiled.leaves, + mode, + deadline, + tty_lock, + &mut outcomes, + ); + + if status.timed_out { + return Response::error( + &req.id, + &format!("command timed out after {}s", total.as_secs()), + ); } - for arg in argv { - cmd.arg(arg); + if let Some(msg) = status.spawn_error { + return Response::error(&req.id, &msg); } - cmd.current_dir("/"); - // Close stdin so a privileged child like `sudo cat` doesn't inherit - // the daemon's controlling tty and block reading from it. sudo opens - // /dev/tty itself for password entry, so this doesn't break auth. - cmd.stdin(Stdio::null()); - cmd.stdout(Stdio::piped()); - cmd.stderr(Stdio::piped()); - run_single_command(&mut cmd, id, exec_timeout(), Some(tty_lock)) + // Build the positional stage list (one per leaf, pre-order) and the + // aggregate stdout (every captured leaf, in order). + let mut stages = Vec::with_capacity(outcomes.len()); + let mut stdout = Vec::new(); + let mut stdout_truncated = false; + for o in &outcomes { + stages.push(StageResult { + exit_code: o.exit, + stderr: B64.encode(&o.stderr), + stderr_truncated: o.stderr_truncated, + }); + if o.captured_stdout && o.ran { + stdout.extend_from_slice(&o.stdout); + stdout_truncated |= o.stdout_truncated; + } + } + Response::ok_with_truncation(&req.id, stages, &stdout, stdout_truncated) } -fn exec_direct_stage( - argv: &[String], - env: &HashMap, - id: &str, - forward_agent: bool, -) -> Response { - let mut cmd = Command::new(&argv[0]); - cmd.args(&argv[1..]); - cmd.current_dir("/"); - cmd.stdin(Stdio::null()); - cmd.stdout(Stdio::piped()); - cmd.stderr(Stdio::piped()); - cmd.env_clear(); - for (k, v) in env { - cmd.env(k, v); - } - if forward_agent { - if let Some(sock) = crate::server::forwarded_agent_socket() { - cmd.env("SSH_AUTH_SOCK", &sock); +fn run_seq( + node: &SeqNode, + leaves: &[LeafPlan], + mode: EscalationMode, + deadline: Instant, + tty_lock: Option<&Mutex<()>>, + outcomes: &mut [Outcome], +) -> StepStatus { + match node { + SeqNode::Leaf(idx) => { + let leaf = &leaves[*idx]; + // A single escalated leaf gets the controlling-tty foreground so a + // sudo/pkexec password prompt doesn't EIO. Unprivileged leaves run + // with stdin=null and never touch /dev/tty. + let fg = if leaf.runas.is_escalated() { + tty_lock + } else { + None + }; + let o = run_one(leaf, mode, deadline, fg); + let st = StepStatus { + timed_out: o.timed_out, + spawn_error: o.spawn_error.clone(), + success: !o.timed_out + && o.spawn_error.is_none() + && exit_accepted(o.exit, &leaf.accepted), + }; + outcomes[*idx] = o.into_outcome(leaf.stdout == OutSpec::Capture); + st + } + SeqNode::Pipe(indices) => run_pipe(indices, leaves, mode, deadline, outcomes), + SeqNode::Seq(nodes) => { + let mut last = StepStatus { + timed_out: false, + spawn_error: None, + success: true, + }; + for n in nodes { + let st = run_seq(n, leaves, mode, deadline, tty_lock, outcomes); + let stop = st.timed_out || st.spawn_error.is_some() || !st.success; + last = st; + if stop { + break; + } + } + last } } +} - run_single_command(&mut cmd, id, exec_timeout(), None) +fn exit_accepted(exit: i32, accepted: &[i32]) -> bool { + exit == 0 || accepted.contains(&exit) } -/// Spawn `cmd`, drain stdout/stderr concurrently with caps, and wait for -/// the child up to `timeout`. Kill the child on cap-hit or timeout so it -/// doesn't outlive the daemon. -/// `fg_lock` is `Some` for privileged children (sudo, pkexec) that need -/// the controlling tty's foreground pgrp so sudo's `/dev/tty` password -/// prompt doesn't EIO; the mutex serializes the swap against prompts -/// (held by `server::handle_connection`). `None` for unprivileged -/// children: they run with `stdin=null` and piped stdio, never read -/// /dev/tty, and would needlessly serialize with daemon prompts. -fn run_single_command( - cmd: &mut Command, - id: &str, - timeout: Duration, +/// One process: its raw run result plus the control-flow flags the sequencer +/// needs. Distinct from `Outcome` so the per-leaf accepted-code and stdout- +/// capture decisions stay with the caller. +struct RunResult { + exit: i32, + stdout: Vec, + stdout_truncated: bool, + stderr: Vec, + stderr_truncated: bool, + timed_out: bool, + spawn_error: Option, +} + +impl RunResult { + fn into_outcome(self, captured_stdout: bool) -> Outcome { + Outcome { + ran: self.spawn_error.is_none(), + exit: self.exit, + stdout: self.stdout, + stdout_truncated: self.stdout_truncated, + stderr: self.stderr, + stderr_truncated: self.stderr_truncated, + captured_stdout, + } + } +} + +/// Spawn and wait for a single leaf process against the shared deadline. +fn run_one( + leaf: &LeafPlan, + mode: EscalationMode, + deadline: Instant, fg_lock: Option<&Mutex<()>>, -) -> Response { +) -> RunResult { + let mut cmd = build_command(leaf, mode); + let stdout_file = match set_stdout(&mut cmd, &leaf.stdout) { + Ok(f) => f, + Err(e) => return spawn_err(format!("failed to open stdout redirect: {e}")), + }; + if let Err(e) = set_stdin(&mut cmd, &leaf.stdin) { + return spawn_err(format!("failed to open stdin redirect: {e}")); + } + if let Err(e) = set_stderr(&mut cmd, &leaf.stderr, stdout_file.as_ref()) { + return spawn_err(format!("failed to open stderr redirect: {e}")); + } + unsafe { cmd.pre_exec(|| { - // New process group so kill_group can SIGKILL the entire - // group on timeout — otherwise a wrapper like `sh -c '... &'` - // would leave grandchild processes orphaned to PID 1, holding - // stdio pipes open and blocking our drain threads forever. - // - // setpgid (vs the historical setsid) keeps the child in the - // *same session* as the daemon, so the controlling terminal - // stays reachable. Under setsid the child had no controlling - // tty, and `sudo` could not open /dev/tty for its password + // New process group so kill_group SIGKILLs the whole group on + // timeout. setpgid (not setsid) keeps the child in the daemon's + // session so an escalated child can reach /dev/tty for a password // prompt — see issue #19. libc::setpgid(0, 0); libc::umask(0o077); @@ -308,130 +806,269 @@ fn run_single_command( let mut child = match cmd.spawn() { Ok(c) => c, - Err(e) => return Response::error(id, &format!("failed to execute command: {e}")), + Err(e) => return spawn_err(format!("failed to execute command: {e}")), }; let cpid = child.id() as libc::pid_t; - // Race-safe duplicate of the child's setpgid: closes the window - // between fork() and the child reaching its pre_exec hook. A second - // setpgid with the same args is harmless; if the child has already - // exec'd, EACCES is returned and we fall back to whatever the child - // managed to set itself. unsafe { let _ = libc::setpgid(cpid, cpid); } - - // Hand the controlling terminal's foreground to the child so reads - // from /dev/tty (e.g. sudo's password prompt) don't raise SIGTTIN - // and stop the child. Best-effort — no-op when the daemon has no - // controlling tty (headless / detached run). Skipped for - // unprivileged children: they don't read /dev/tty and the swap's - // shared `tty_lock` would needlessly queue them behind prompts. let _fg = fg_lock.and_then(|m| ForegroundGuard::take(cpid, m)); - // Drain stdout/stderr in threads so a child filling its stderr pipe - // doesn't block our read of stdout (or vice versa). let stdout_handle = child.stdout.take().map(|s| spawn_drain(s, MAX_OUTPUT_BYTES)); let stderr_handle = child.stderr.take().map(|s| spawn_drain(s, MAX_OUTPUT_BYTES)); - let deadline = Instant::now() + timeout; let exit = match wait_with_deadline(&mut child, deadline) { - Ok(s) => s, + Ok(s) => s.code().unwrap_or(-1), Err(WaitError::Timeout) => { kill_group(&mut child); - return Response::error( - id, - &format!("command timed out after {}s", timeout.as_secs()), - ); + drain_join(stdout_handle); + drain_join(stderr_handle); + return RunResult { + exit: -1, + stdout: Vec::new(), + stdout_truncated: false, + stderr: Vec::new(), + stderr_truncated: false, + timed_out: true, + spawn_error: None, + }; } Err(WaitError::Io(e)) => { kill_group(&mut child); - return Response::error(id, &format!("wait failed: {e}")); + drain_join(stdout_handle); + drain_join(stderr_handle); + return spawn_err(format!("wait failed: {e}")); } }; - let (stdout_bytes, stdout_truncated) = stdout_handle - .map(|h| h.join().unwrap_or_default()) - .unwrap_or_default(); - let (stderr_bytes, stderr_truncated) = stderr_handle - .map(|h| h.join().unwrap_or_default()) - .unwrap_or_default(); + let (stdout_bytes, stdout_truncated) = drain_join(stdout_handle); + let (stderr_bytes, stderr_truncated) = drain_join(stderr_handle); - let stage = StageResult { - exit_code: exit.code().unwrap_or(-1), - stderr: B64.encode(&stderr_bytes), + RunResult { + exit, + stdout: stdout_bytes, + stdout_truncated, + stderr: stderr_bytes, stderr_truncated, - }; - Response::ok_with_truncation(id, vec![stage], &stdout_bytes, stdout_truncated) + timed_out: false, + spawn_error: None, + } } -// --------------------------------------------------------------------------- -// Pipeline execution -// --------------------------------------------------------------------------- - -#[derive(Clone, Copy)] -enum EscalationMode { - Sudo, - Direct, +fn drain_join(h: Option, bool)>>) -> (Vec, bool) { + h.map(|h| h.join().unwrap_or_default()).unwrap_or_default() } -/// Execute a multi-stage pipeline by spawning N children connected by pipes. -fn exec_pipeline( - req: &Request, - env: &HashMap, - mode: EscalationMode, -) -> Response { - let n = req.pipeline.len(); - assert!(n >= 2); - - let mut children: Vec = Vec::with_capacity(n); - let mut prev_stdout: Option = None; - - for (i, argv) in req.pipeline.iter().enumerate() { - let is_first = i == 0; - let is_last = i == n - 1; +fn spawn_err(msg: String) -> RunResult { + RunResult { + exit: -1, + stdout: Vec::new(), + stdout_truncated: false, + stderr: Vec::new(), + stderr_truncated: false, + timed_out: false, + spawn_error: Some(msg), + } +} - let mut cmd = match mode { - EscalationMode::Sudo => { - let mut c = Command::new("sudo"); - for (k, v) in env { - c.arg(format!("{k}={v}")); - } - for arg in argv { - c.arg(arg); +/// Build the `Command` for a leaf: program + args + env + cwd, with escalation +/// applied. Stdio is configured by the caller (so pipe wiring can override it). +/// No shell is ever introduced here — an escalated leaf is `sudo … -- argv` or +/// `pkexec … argv`. +fn build_command(leaf: &LeafPlan, mode: EscalationMode) -> Command { + match &leaf.runas { + LeafRunAs::Direct => { + let mut c = Command::new(&leaf.argv[0]); + c.args(&leaf.argv[1..]); + c.env_clear(); + for (k, v) in &leaf.env { + c.env(k, v); + } + if leaf.forward_agent { + if let Some(sock) = crate::server::forwarded_agent_socket() { + c.env("SSH_AUTH_SOCK", &sock); } - c } - EscalationMode::Direct => { - let mut c = Command::new(&argv[0]); - c.args(&argv[1..]); + c.current_dir(&leaf.cwd); + c + } + escalated => match mode { + EscalationMode::Pkexec => { + let mut c = Command::new("pkexec"); + if let LeafRunAs::User(name) = escalated { + c.arg("--user"); + c.arg(name); + } + for a in &leaf.argv { + c.arg(a); + } c.env_clear(); - for (k, v) in env { + for (k, v) in &leaf.env { c.env(k, v); } - if req.forward_agent { - if let Some(sock) = crate::server::forwarded_agent_socket() { - c.env("SSH_AUTH_SOCK", &sock); + // pkexec needs these to show its dialog. + for key in ["DISPLAY", "WAYLAND_DISPLAY", "XAUTHORITY"] { + if let Ok(v) = std::env::var(key) { + c.env(key, v); } } + c.current_dir(&leaf.cwd); c } - }; + _ => { + // sudo: `sudo [-u user] K=V … -- argv`. The `--` stops option + // parsing so an argv[0] beginning with `-` is treated as the + // command, not a sudo flag. + let mut c = Command::new("sudo"); + if let LeafRunAs::User(name) = escalated { + c.arg("-u"); + c.arg(name); + } + for (k, v) in &leaf.env { + c.arg(format!("{k}={v}")); + } + c.arg("--"); + for a in &leaf.argv { + c.arg(a); + } + c.current_dir(&leaf.cwd); + c + } + }, + } +} + +fn open_out(path: &str, append: bool) -> std::io::Result { + let mut o = OpenOptions::new(); + o.write(true).create(true); + if append { + o.append(true); + } else { + o.truncate(true); + } + o.open(path) +} + +/// Configure a command's stdout. Returns the opened file handle when the target +/// is a file, so a sibling `2>&1` stderr can be dup'd onto the same handle. +fn set_stdout(cmd: &mut Command, spec: &OutSpec) -> std::io::Result> { + match spec { + OutSpec::Capture => { + cmd.stdout(Stdio::piped()); + Ok(None) + } + OutSpec::DevNull => { + cmd.stdout(Stdio::null()); + Ok(None) + } + OutSpec::File { path, append } => { + let f = open_out(path, *append)?; + cmd.stdout(f.try_clone()?); + Ok(Some(f)) + } + } +} + +fn set_stdin(cmd: &mut Command, spec: &InSpec) -> std::io::Result<()> { + match spec { + InSpec::Null => { + cmd.stdin(Stdio::null()); + } + InSpec::File(path) => { + cmd.stdin(File::open(path)?); + } + } + Ok(()) +} + +fn set_stderr(cmd: &mut Command, spec: &ErrSpec, stdout_file: Option<&File>) -> std::io::Result<()> { + match spec { + ErrSpec::Capture => { + cmd.stderr(Stdio::piped()); + } + ErrSpec::DevNull => { + cmd.stderr(Stdio::null()); + } + ErrSpec::File { path, append } => { + cmd.stderr(open_out(path, *append)?); + } + ErrSpec::ToStdout => match stdout_file { + // `2>&1` only ever pairs with a `Both` redirect, whose target is a + // file or /dev/null — so stdout is a real fd we can dup, never a + // capture pipe. + Some(f) => { + cmd.stderr(f.try_clone()?); + } + None => { + cmd.stderr(Stdio::null()); + } + }, + } + Ok(()) +} - cmd.current_dir("/"); +/// Run a pipe group: spawn every stage connected by pipes, then collect all +/// their results against the shared deadline. Pipe stages use `setsid` (not +/// `setpgid`): a pipelined child cannot interactively prompt for a password, so +/// it must be detached from the controlling tty and rely on cached credentials +/// — matching the pre-plan pipeline behaviour. +/// +/// Consequence: a privileged stage *inside a pipe* cannot prompt, and because it +/// has no controlling tty, a credential cached on the daemon's tty may not be +/// visible to it (depends on the host's sudoers `timestamp_type`: `global` yes, +/// the common `tty` default likely not). Such a stage needs NOPASSWD or an +/// already-`global`-cached credential, or it fails fast with "no tty present". +/// Prefer a single privileged stage, which runs with `setpgid` + the foreground +/// guard and *can* prompt. See the "Partial-`seq` retry hazard and the +/// sudo-credential-in-pipe caveat" item in backlog.md. +fn run_pipe( + indices: &[usize], + leaves: &[LeafPlan], + mode: EscalationMode, + deadline: Instant, + outcomes: &mut [Outcome], +) -> StepStatus { + let n = indices.len(); + let mut children: Vec = Vec::with_capacity(n); + let mut prev_stdout: Option = None; + + for (pos, &li) in indices.iter().enumerate() { + let leaf = &leaves[li]; + let is_first = pos == 0; + let is_last = pos == n - 1; - // stdin: first stage gets null, later stages get the previous - // stage's stdout. sudo's password prompt reads from /dev/tty, not - // stdin, so closing stdin doesn't break authentication and prevents - // the first stage from blocking on the daemon's tty. + let mut cmd = build_command(leaf, mode); + + // stdin: first stage honours its own spec; later stages read the + // previous stage's stdout. if let Some(prev) = prev_stdout.take() { cmd.stdin(prev); } else if is_first { - cmd.stdin(Stdio::null()); + if let Err(e) = set_stdin(&mut cmd, &leaf.stdin) { + let _guard = KillOnDrop::new(children); + return spawn_error_status(format!("failed to open stdin redirect: {e}")); + } } - cmd.stdout(Stdio::piped()); - cmd.stderr(Stdio::piped()); + // stdout: non-final stages feed the next stage; the final stage honours + // its own spec. + let stdout_file = if is_last { + match set_stdout(&mut cmd, &leaf.stdout) { + Ok(f) => f, + Err(e) => { + let _guard = KillOnDrop::new(children); + return spawn_error_status(format!("failed to open stdout redirect: {e}")); + } + } + } else { + cmd.stdout(Stdio::piped()); + None + }; + if let Err(e) = set_stderr(&mut cmd, &leaf.stderr, stdout_file.as_ref()) { + let _guard = KillOnDrop::new(children); + return spawn_error_status(format!("failed to open stderr redirect: {e}")); + } unsafe { cmd.pre_exec(|| { @@ -451,37 +1088,41 @@ fn exec_pipeline( children.push(child); } Err(e) => { - // Already-spawned children get killed via KillOnDrop below. let _guard = KillOnDrop::new(children); - return Response::error( - &req.id, - &format!("failed to spawn pipeline stage {i}: {e}"), - ); + return spawn_error_status(format!("failed to spawn pipeline stage {pos}: {e}")); } } } - collect_pipeline_results(&req.id, children, n, exec_timeout()) + collect_group(indices, leaves, children, deadline, outcomes) } -/// Collect results from all pipeline children with caps and a timeout. -/// On panic in any of the spawned drain/wait threads, the KillOnDrop guard -/// terminates every still-running child so we never orphan a process to PID 1. -fn collect_pipeline_results( - id: &str, +fn spawn_error_status(msg: String) -> StepStatus { + StepStatus { + timed_out: false, + spawn_error: Some(msg), + success: false, + } +} + +/// Collect results from a spawned pipe group, writing each stage's outcome into +/// `outcomes[stage_id]`. The group's controlling exit is the final stage's. +fn collect_group( + indices: &[usize], + leaves: &[LeafPlan], children: Vec, - n: usize, - timeout: Duration, -) -> Response { + deadline: Instant, + outcomes: &mut [Outcome], +) -> StepStatus { + let n = children.len(); let mut guard = KillOnDrop::new(children); - // Spawn one drainer per stderr (capped). The last stage's stdout is - // also drained in a thread for symmetry — we don't want to block on - // it while a sibling stage's stderr fills up. + // One stderr drainer per child (only where stderr was piped), plus the + // final stage's stdout drainer (only where it was captured). let mut stderr_handles = Vec::with_capacity(n); for child in guard.children.iter_mut() { - let stderr_pipe = child.stderr.take(); - stderr_handles.push(stderr_pipe.map(|s| spawn_drain(s, MAX_OUTPUT_BYTES))); + let pipe = child.stderr.take(); + stderr_handles.push(pipe.map(|s| spawn_drain(s, MAX_OUTPUT_BYTES))); } let stdout_handle = guard .children @@ -489,11 +1130,7 @@ fn collect_pipeline_results( .and_then(|c| c.stdout.take()) .map(|s| spawn_drain(s, MAX_OUTPUT_BYTES)); - let deadline = Instant::now() + timeout; let mut exits: Vec> = vec![None; n]; - - // Wait for each child up to the deadline. We poll in parallel by - // round-robining try_wait across all not-yet-exited children. loop { let mut all_done = true; for (i, child) in guard.children.iter_mut().enumerate() { @@ -506,17 +1143,12 @@ fn collect_pipeline_results( Err(_) => exits[i] = Some(synthetic_failure_status()), } } - if all_done { - break; - } - if Instant::now() >= deadline { + if all_done || Instant::now() >= deadline { break; } std::thread::sleep(WAIT_POLL_INTERVAL); } - // Anything not exited gets killed; record a synthetic failure status - // for those stages. let mut timed_out = false; for (i, child) in guard.children.iter_mut().enumerate() { if exits[i].is_none() { @@ -526,31 +1158,37 @@ fn collect_pipeline_results( } } - let (stdout_bytes, stdout_truncated) = stdout_handle - .map(|h| h.join().unwrap_or_default()) - .unwrap_or_default(); - - // All children have exited (or been killed); join their stderr drainers. - let mut stages = Vec::with_capacity(n); + let (stdout_bytes, stdout_truncated) = drain_join(stdout_handle); + let last_pos = n - 1; for (i, h) in stderr_handles.into_iter().enumerate() { - let (bytes, truncated) = h.map(|h| h.join().unwrap_or_default()).unwrap_or_default(); - stages.push(StageResult { - exit_code: exits[i].as_ref().and_then(|s| s.code()).unwrap_or(-1), - stderr: B64.encode(&bytes), + let (bytes, truncated) = drain_join(h); + let li = indices[i]; + let exit = exits[i].as_ref().and_then(|s| s.code()).unwrap_or(-1); + let captured = leaves[li].stdout == OutSpec::Capture; + outcomes[li] = Outcome { + ran: true, + exit, + stdout: if i == last_pos { + stdout_bytes.clone() + } else { + Vec::new() + }, + stdout_truncated: i == last_pos && stdout_truncated, + stderr: bytes, stderr_truncated: truncated, - }); + captured_stdout: captured && i == last_pos, + }; } - // All children have been waited on already; defuse the guard. let _ = guard.into_inner(); - if timed_out { - return Response::error( - id, - &format!("pipeline timed out after {}s", timeout.as_secs()), - ); + let controlling_exit = outcomes[indices[last_pos]].exit; + let last_accepted = &leaves[indices[last_pos]].accepted; + StepStatus { + timed_out, + spawn_error: None, + success: !timed_out && exit_accepted(controlling_exit, last_accepted), } - Response::ok_with_truncation(id, stages, &stdout_bytes, stdout_truncated) } // --------------------------------------------------------------------------- @@ -740,47 +1378,9 @@ impl Drop for KillOnDrop { } } -/// Build a shell command string for a pipeline with env vars (used by pkexec multi-stage). -fn pipeline_to_shell(pipeline: &[Vec], env: &HashMap) -> String { - let env_prefix: String = env - .iter() - .map(|(k, v)| format!("{}={}", shell_escape(k), shell_escape(v))) - .collect::>() - .join(" "); - - let stages: Vec = pipeline - .iter() - .map(|argv| { - argv.iter() - .map(|a| shell_escape(a)) - .collect::>() - .join(" ") - }) - .collect(); - - if env_prefix.is_empty() { - stages.join(" | ") - } else { - format!("{} {}", env_prefix, stages.join(" | ")) - } -} - -fn shell_escape(s: &str) -> String { - if s.is_empty() { - return "''".to_string(); - } - if s.chars() - .all(|c| c.is_alphanumeric() || matches!(c, '-' | '_' | '/' | '.' | '=' | ':' | ',')) - { - return s.to_string(); - } - format!("'{}'", s.replace('\'', "'\\''")) -} - #[cfg(test)] mod tests { use super::*; - use crate::prop::Rng; use std::time::Duration; fn pid_alive(pid: u32) -> bool { @@ -858,82 +1458,226 @@ mod tests { } } - // === Property: shell_escape round-trips through /bin/sh ============== - // - // Spec clause (Rung 2; proof obligation for Rung 5 Creusot on the output - // grammar of shell_escape): for every input `s`, the word produced by - // `shell_escape(s)`, evaluated by /bin/sh as a single argument, reproduces - // `s` byte-for-byte. A break here is a pipeline command-injection — - // multi-stage requests are assembled into `sh -c ' | '`. - - /// The predicate. Returns `Ok(())` on a faithful round-trip, or - /// `Err((escaped, sh_output))` describing the divergence. This is the - /// clause a Rung 5 contract will discharge; here it is checked by feeding - /// it generated inputs (`prop_*`) and known vectors. - fn shell_escape_roundtrips_through_sh(s: &str) -> Result<(), (String, Vec)> { - use std::process::Command; - let esc = shell_escape(s); - // printf %s makes the escaped word the sole argument. Empty PATH: - // even if escaping failed, an injected word resolves to nothing - // executable rather than a real command. - let out = Command::new("/bin/sh") - .arg("-c") - .arg(format!("printf %s {esc}")) - .env("PATH", "") - .current_dir("/") - .output() - .expect("spawn /bin/sh"); - if out.stdout == s.as_bytes() { - Ok(()) + // The shell_escape round-trip property that lived here is retired with the + // code it guarded: this increment removes all shell-string assembly from + // the execution path (`pipeline_to_shell` / `pkexec sh -c` are gone). The + // only surviving shell is an explicit `System` leaf, whose script is passed + // verbatim as a single `-c` argument — nothing is quoted or re-assembled, + // so there is no escaping grammar left to verify. The structural + // "no shell on the privileged path" invariant is verified directly below. + + // === Structured-plan properties (Rung 2) ============================ + use crate::prop::Rng; + use crate::protocol::{Plan, Request, RunAs, Shell, ValidatedRequest}; + + fn mk_plan_req(plan: Plan, privileged: bool) -> ValidatedRequest { + let req = Request::new_plan( + String::new(), + "test".into(), + plan, + HashMap::new(), + String::new(), + privileged, + false, + ); + ValidatedRequest::validate(req).expect("valid plan request") + } + + /// Count `Run`/`System` leaves in a plan tree (the pre-order leaf set the + /// executor and the prompt both enumerate). + fn count_leaves(p: &Plan) -> usize { + match p { + Plan::Run { .. } | Plan::System { .. } => 1, + Plan::Seq { steps } => steps.iter().map(count_leaves).sum(), + Plan::Pipe { stages } => stages.iter().map(count_leaves).sum(), + Plan::Redirect { body, .. } + | Plan::WithStdinFrom { body, .. } + | Plan::Setenv { body, .. } + | Plan::Chdir { body, .. } + | Plan::RunAs { body, .. } + | Plan::WithAcceptedExitCodes { body, .. } => count_leaves(body), + } + } + + fn word(rng: &mut Rng) -> String { + // Benign, allowlist-safe characters only; never empty. + let alpha = b"abcdef0123456789"; + let s = rng.string(alpha, 5); + if s.is_empty() { + "x".into() } else { - Err((esc, out.stdout)) + s + } + } + + /// A plan that always compiles to exactly one leaf: bare leaves plus the + /// single-process modifiers. Used for pipe stages and redirect bodies. + fn random_single(rng: &mut Rng, depth: usize) -> Plan { + if depth == 0 || rng.below(2) == 0 { + if rng.below(6) == 0 { + return Plan::System { + shell: if rng.below(2) == 0 { Shell::Sh } else { Shell::Bash }, + script: word(rng), + }; + } + let n = 1 + rng.below(3); + return Plan::Run { + argv: (0..n).map(|_| word(rng)).collect(), + }; + } + match rng.below(4) { + 0 => Plan::Setenv { + key: "LANG".into(), + value: word(rng), + body: Box::new(random_single(rng, depth - 1)), + }, + 1 => Plan::Chdir { + dir: "/".into(), + body: Box::new(random_single(rng, depth - 1)), + }, + 2 => Plan::RunAs { + who: if rng.below(2) == 0 { + RunAs::Root + } else { + RunAs::User { name: word(rng) } + }, + body: Box::new(random_single(rng, depth - 1)), + }, + _ => Plan::WithAcceptedExitCodes { + codes: vec![rng.below(5) as i32], + body: Box::new(random_single(rng, depth - 1)), + }, } } - // Driver: sweep the predicate over a metacharacter-heavy alphabet (quotes, - // $, backticks, backslash, |, ;, &, spaces, parens, braces) — exactly the - // bytes that could break out of single-quoting. Newline/NUL/control chars - // are excluded because validate_request rejects them before shell_escape - // is ever reached, so they are not part of this function's input domain. + /// A random, always-compilable plan tree. Composite modifiers wrap + /// composites; stdio modifiers and pipe stages wrap single processes. + fn random_plan(rng: &mut Rng, depth: usize) -> Plan { + if depth == 0 || rng.below(3) == 0 { + return random_single(rng, depth); + } + match rng.below(6) { + 0 => Plan::Seq { + steps: (0..1 + rng.below(3)) + .map(|_| random_plan(rng, depth - 1)) + .collect(), + }, + 1 => Plan::Pipe { + stages: (0..2 + rng.below(2)) + .map(|_| random_single(rng, depth - 1)) + .collect(), + }, + 2 => Plan::Setenv { + key: "TZ".into(), + value: word(rng), + body: Box::new(random_plan(rng, depth - 1)), + }, + 3 => Plan::RunAs { + who: RunAs::Root, + body: Box::new(random_plan(rng, depth - 1)), + }, + 4 => Plan::Redirect { + stream: Stream::Stdout, + target: RedirTarget::DevNull, + body: Box::new(random_single(rng, depth - 1)), + }, + _ => Plan::WithStdinFrom { + path: "/dev/null".into(), + body: Box::new(random_single(rng, depth - 1)), + }, + } + } + + // Spec clause (Rung 2): `plan_display` is total and panic-free over random + // plan trees, and its leaf set is **structurally identical** to the tree's + // pre-order `Run`/`System` leaves — same count, contiguous stage ids + // `0..n`, same `argv[0]` in order. This is the regression that would have + // caught the `bash -lc` hollowing: the rendered stages are exactly the + // stages the executor runs. #[test] - fn prop_shell_escape_roundtrips_through_sh() { - let alphabet: &[u8] = b"abc01 '\"\\$`|;&()<>*?{}[]=:,.-_/!#%^~@ \t"; - let mut rng = Rng::new(0x9E3779B97F4A7C15); - let mut failures = Vec::new(); - - for _ in 0..5000 { - let s = rng.string(alphabet, 23); - if let Err((esc, output)) = shell_escape_roundtrips_through_sh(&s) { - failures.push((s, esc, output)); + fn prop_plan_display_is_structurally_identical_to_the_tree() { + let mut rng = Rng::new(0xD1CE_5EED_u64); + for _ in 0..4000 { + let plan = random_plan(&mut rng, 5); + let privileged = rng.below(2) == 0; + let req = mk_plan_req(plan.clone(), privileged); + + let views = plan_display(&req).expect("random plan must compile"); + // Same number of stages as Run/System leaves. + assert_eq!(views.len(), count_leaves(&plan), "leaf count mismatch"); + // Stage ids are exactly 0..n, in order. + for (i, v) in views.iter().enumerate() { + assert_eq!(v.stage_id, i, "stage ids must be contiguous 0..n"); + assert!(!v.argv.is_empty(), "every stage has a command"); } } - assert!( - failures.is_empty(), - "shell_escape round-trip failures (input, escaped, sh-output): {:?}", - &failures[..failures.len().min(5)] - ); } - // Targeted regression vectors alongside the random sweep, checked through - // the same predicate. + // Spec clause (Rung 2): compiling a random plan for execution is total and + // panic-free, and produces exactly `count_leaves` planned processes. #[test] - fn shell_escape_known_injection_vectors_are_inert() { - let vectors = [ - "'; touch /tmp/sudo_proxy_pwned; '", - "$(touch /tmp/sudo_proxy_pwned)", - "`touch /tmp/sudo_proxy_pwned`", - "a'b\"c\\d", - "'\''", - "", - "$IFS", - "x|y", - ]; - for v in vectors { - assert!( - shell_escape_roundtrips_through_sh(v).is_ok(), - "shell_escape failed to neutralize {v:?} -> {:?}", - shell_escape(v) - ); + fn prop_compile_plan_is_total() { + let mut rng = Rng::new(0x5EED_D1CE_u64); + for _ in 0..4000 { + let plan = random_plan(&mut rng, 5); + let privileged = rng.below(2) == 0; + let req = mk_plan_req(plan.clone(), privileged); + let compiled = compile_plan(&req, &HashMap::new()).expect("compiles"); + assert_eq!(compiled.leaves.len(), count_leaves(&plan)); } } + + // Spec clause (Rung 2): the no-shell-on-privileged-path invariant, checked + // structurally on the built `Command`. An escalated, non-opaque leaf is + // spawned as `sudo [-u user] -- ` / `pkexec [--user u] argv` + // — the real binary is argv[0], and no `sh -c` / `bash -c` wrapper is ever + // introduced. The only shell is an explicit `System` leaf. + #[test] + fn escalated_leaf_is_literal_argv_never_a_shell_wrapper() { + let leaf = |runas: LeafRunAs| LeafPlan { + argv: vec!["sysctl".into(), "-w".into(), "vm.mmap_rnd_bits=28".into()], + env: HashMap::new(), + cwd: "/".into(), + runas, + stdin: InSpec::Null, + stdout: OutSpec::Capture, + stderr: ErrSpec::Capture, + opaque: false, + resolved_bin: None, + accepted: Vec::new(), + forward_agent: false, + stage_id: 0, + }; + + // Root under sudo: `sudo -- sysctl -w …`, no shell. + let c = build_command(&leaf(LeafRunAs::Root), EscalationMode::Sudo); + assert_eq!(c.get_program().to_str().unwrap(), "sudo"); + let args: Vec = c + .get_args() + .map(|a| a.to_string_lossy().into_owned()) + .collect(); + assert!(args.contains(&"--".to_string()), "sudo must use -- : {args:?}"); + assert!(args.ends_with(&["sysctl".into(), "-w".into(), "vm.mmap_rnd_bits=28".into()])); + assert!( + !args.iter().any(|a| a == "-c"), + "no shell -c on the privileged path: {args:?}" + ); + + // Named user under sudo: `sudo -u alice -- …`. + let c = build_command(&leaf(LeafRunAs::User("alice".into())), EscalationMode::Sudo); + let args: Vec = c + .get_args() + .map(|a| a.to_string_lossy().into_owned()) + .collect(); + assert_eq!(&args[0], "-u"); + assert_eq!(&args[1], "alice"); + + // Pkexec root: `pkexec sysctl …` (no shell). + let c = build_command(&leaf(LeafRunAs::Root), EscalationMode::Pkexec); + assert_eq!(c.get_program().to_str().unwrap(), "pkexec"); + + // Direct leaf: program IS the literal binary. + let c = build_command(&leaf(LeafRunAs::Direct), EscalationMode::Direct); + assert_eq!(c.get_program().to_str().unwrap(), "sysctl"); + } } diff --git a/src/mcp.rs b/src/mcp.rs index fa7f21e..50f43b5 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -46,6 +46,51 @@ fn read_timeout_for(privileged: bool, timeout_ms: Option) -> Option None, } } + +/// Build the outgoing `Request` from the tool parameters. Exactly one command +/// form is accepted: a structured `plan`, or the legacy `pipeline` / `argv`. +/// The daemon re-validates; this is only the client-side assembly. +fn build_request(params: &ExecuteParams, host: String) -> Result { + let has_plan = params.plan.is_some(); + let has_pipeline = params.pipeline.as_ref().is_some_and(|p| !p.is_empty()); + let has_argv = params.argv.as_ref().is_some_and(|a| !a.is_empty()); + if has_plan && (has_pipeline || has_argv) { + return Err("Provide either `plan` or `argv`/`pipeline`, not both.".to_string()); + } + + let session = "sudo-proxy-mcp".to_string(); + let env = params.env.clone().unwrap_or_default(); + let reason = params.description.clone().unwrap_or_default(); + + if let Some(plan_json) = ¶ms.plan { + let plan: protocol::Plan = serde_json::from_value(plan_json.clone()) + .map_err(|e| format!("invalid plan: {e}"))?; + Ok(Request::new_plan( + host, + session, + plan, + env, + reason, + params.privileged, + params.forward_agent, + )) + } else { + let pipeline = match (¶ms.pipeline, ¶ms.argv) { + (Some(p), _) if !p.is_empty() => p.clone(), + (_, Some(argv)) if !argv.is_empty() => vec![argv.clone()], + _ => return Err("Either `argv`, `pipeline`, or `plan` must be provided.".to_string()), + }; + Ok(Request::new( + host, + session, + pipeline, + env, + reason, + params.privileged, + params.forward_agent, + )) + } +} /// Total timeout for control requests (stop/ping). They never wait on a /// human, so anything beyond connect + one round-trip means trouble. const CONTROL_TIMEOUT: Duration = Duration::from_secs(10); @@ -86,6 +131,30 @@ pub struct ExecuteParams { #[serde(default)] pub pipeline: Option>>, + /// Structured execution plan (advanced; mutually exclusive with `argv`/`pipeline`). + /// A JSON object the daemon executes *directly* — no shell on the privileged + /// path — and renders stage by stage at the approval prompt, so every real + /// binary and per-stage privilege is visible. Prefer this over wrapping + /// commands in `bash -lc`. Nodes are `op`-tagged, e.g. + /// {"op":"run","argv":["sysctl","-w","vm.mmap_rnd_bits=28"]}; + /// {"op":"pipe","stages":[{"op":"run",...},{"op":"run",...}]}; + /// {"op":"seq","steps":[...]} (sequence, stops on first failure); + /// {"op":"run-as","as":"root","do":{...}} or {"op":"run-as","as":"user","name":"alice","do":{...}}; + /// {"op":"redirect","stream":"stdout","target":{"to":"file","path":"/tmp/o","mode":"truncate"},"do":{...}}; + /// {"op":"setenv","key":"LANG","value":"C","do":{...}}; {"op":"chdir","dir":"/srv","do":{...}}; + /// {"op":"system","shell":"bash","script":"..."} (explicit opaque escape hatch). + /// Semantics to plan around: a `seq` runs its steps in order and STOPS at + /// the first failing step — the earlier steps have already run and their + /// effects are committed, so make early steps idempotent (or put the + /// riskiest / privileged step first) and do not blindly resubmit a failed + /// plan; the response's per-stage exit codes show which steps ran (a skipped + /// step reports exit -1) so you can resume from the failure instead. A + /// privileged stage placed INSIDE a `pipe` cannot prompt for a sudo + /// password; if it may need one, make it a standalone `run-as` stage rather + /// than a pipe stage (or ensure passwordless sudo on the host). + #[serde(default)] + pub plan: Option, + /// Target host (omit for localhost) #[serde(default)] pub host: Option, @@ -197,11 +266,23 @@ impl McpProxy { // self-target cannot route SSH-to-self around local policy. let target = normalize_host(params.host.as_deref()); + // Build the request from exactly one command form: a structured `plan`, + // or the legacy `pipeline` / `argv`. The request is built before the + // policy checks below so they can key off the plan's *effective* + // privilege (a plan may escalate via a `run-as` node even with + // `privileged: false`). + let host_field = target.clone().unwrap_or_default(); + let req = match build_request(¶ms, host_field) { + Ok(r) => r, + Err(e) => return Ok(error_result(e)), + }; + let effective_privileged = req.effective_privileged(); + // Delegate-to-Bash (G7): an unprivileged command on the local machine // is refused — the agent's own Bash tool already applies the same // controls, so sudo-proxy running it would only be a second, weaker // gate. sudo-proxy is for privilege escalation and for remote hosts. - if target.is_none() && !params.privileged { + if target.is_none() && !effective_privileged { return Ok(error_result( "Refusing to run an unprivileged command on the local machine. \ Use your Bash tool instead — it applies the same controls without a \ @@ -211,12 +292,17 @@ impl McpProxy { )); } + if params.forward_agent && effective_privileged { + return Ok(error_result( + "forward_agent is only allowed with privileged: false".to_string(), + )); + } + let socket_path = socket_for_host(target.as_deref()); // Issue #54: the wall-clock `timeout` is opt-in for unprivileged - // commands (see `read_timeout_for`). Computed here so the decision is - // visible next to routing; `params.privileged` is Copy so it stays - // readable after the request is built below. - let read_timeout = read_timeout_for(params.privileged, params.timeout); + // commands (see `read_timeout_for`). A plan that escalates is treated as + // privileged for the timeout default, too. + let read_timeout = read_timeout_for(effective_privileged, params.timeout); if !socket_path.exists() { return Ok(error_result(format!( @@ -225,35 +311,8 @@ impl McpProxy { ))); } - // Build pipeline from either `pipeline` or `argv` parameter - let pipeline = match (params.pipeline, params.argv) { - (Some(p), _) if !p.is_empty() => p, - (_, Some(argv)) if !argv.is_empty() => vec![argv], - _ => { - return Ok(error_result( - "Either `argv` or `pipeline` must be provided.".to_string(), - )); - } - }; - let host_name = target.clone().unwrap_or_else(|| "localhost".into()); - if params.forward_agent && params.privileged { - return Ok(error_result( - "forward_agent is only allowed with privileged: false".to_string(), - )); - } - - let req = Request::new( - target.unwrap_or_default(), - "sudo-proxy-mcp".to_string(), - pipeline, - params.env.unwrap_or_default(), - params.description.unwrap_or_default(), - params.privileged, - params.forward_agent, - ); - let result = send_request(&socket_path, &req, read_timeout).await; match result { diff --git a/src/proofs.rs b/src/proofs.rs index f85f4ea..1dde1f3 100644 --- a/src/proofs.rs +++ b/src/proofs.rs @@ -139,3 +139,36 @@ fn has_dangerous_chars_matches_spec_per_char() { || matches!(u, 0x200B..=0x200F | 0x202A..=0x202E | 0x2066..=0x2069); assert_eq!(got, expected); } + +/// Proof: the structured-plan leaf check is panic-free and agrees with the +/// per-char danger spec. A `Plan::Run` whose single argv element is one symbolic +/// character validates iff that character is not dangerous — i.e. the recursive +/// `Plan::validate_tree` applies exactly the same `has_dangerous_chars` gate to a +/// plan leaf that the flat-pipeline path applies to an argv element. This is the +/// Rung 3 discharge of the structured-plan leaf clause of the Rung 2 +/// `prop_plan_display_is_structurally_identical_to_the_tree` / validation +/// predicates. +/// +/// Scope mirrors `has_dangerous_chars_matches_spec_per_char`: one leaf, one +/// character bounded below `0x2100` (every forbidden range lies ≤ `0x2069`), so +/// the single-leaf result generalises — `validate_tree` is a fold applying that +/// same per-char predicate to every string leaf. `unwind(6)` bounds the single +/// ≤3-byte char decode plus the one-element argv loop. +#[kani::proof] +#[kani::unwind(6)] +fn plan_run_leaf_matches_dangerous_char_spec() { + let c: char = kani::any(); + kani::assume((c as u32) < 0x2100); + + let mut buf = [0u8; 4]; + let s: &str = c.encode_utf8(&mut buf); + + let plan = crate::protocol::Plan::Run { + argv: vec![s.to_string()], + }; + let mut nodes = 0usize; + let accepted = plan.validate_tree(0, &mut nodes).is_ok(); + + // A single non-dangerous char is accepted; a dangerous one is rejected. + assert_eq!(accepted, !crate::protocol::has_dangerous_chars(s)); +} diff --git a/src/protocol.rs b/src/protocol.rs index e8c6bdc..6aa896d 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -32,6 +32,282 @@ impl Action { } } +/// Maximum nesting depth of a structured plan tree. Bounds stack use in the +/// recursive validator / compiler and caps pathologically-nested plans a +/// client could submit. The flat `pipeline` model never needed this; a tree +/// does. +pub const MAX_PLAN_DEPTH: usize = 32; + +/// Maximum number of nodes in a structured plan tree. A second, independent +/// bound so a wide-but-shallow plan (thousands of siblings) is rejected even +/// though it stays under `MAX_PLAN_DEPTH`. +pub const MAX_PLAN_NODES: usize = 256; + +/// Which stream a `Redirect` node retargets. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum Stream { + Stdout, + Stderr, + /// Both stdout and stderr (the `2>&1` / `with-outputs-to` case). + Both, +} + +/// Truncate (`>`) vs append (`>>`) for a file redirect. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum WriteMode { + #[default] + Truncate, + Append, +} + +/// Where a `Redirect` node sends its stream. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "to", rename_all = "kebab-case")] +pub enum RedirTarget { + File { + path: String, + #[serde(default)] + mode: WriteMode, + }, + /// `>/dev/null` — discard (the Dune `ignore-*` case). + DevNull, +} + +/// The privilege / run-as context a `RunAs` node imposes on its subtree. +/// +/// This is the security axis Dune's action DSL lacks. Any `RunAs` node makes +/// the whole request "effectively privileged" (see `Request::effective_privileged`), +/// because running as root *or* as another user requires escalation the daemon +/// gates behind the human prompt. It is the structured replacement for the +/// nested `sudo -H -u bash -lc '…'` the approval log showed. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "as", rename_all = "lowercase")] +pub enum RunAs { + Root, + User { name: String }, +} + +/// Which shell an explicit `System` escape-hatch node runs. Named so the +/// prompt can render the stage as "opaque — resolves only the shell": this is +/// the *only* place a shell re-enters the execution path, and it is never +/// hidden. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum Shell { + Sh, + Bash, +} + +/// A structured execution plan: a recursive tree the daemon executes +/// **directly** (fork/exec per stage: pipes, redirections, env, run-as, +/// sequencing) with **no shell on the privileged path**. The load-bearing +/// invariant: *the structure the human approves is literally the structure +/// that runs.* +/// +/// JSON-native (serde-tagged on `op`), not an S-expression sub-language: the +/// wire is already JSON, and a single representation that both the validator +/// and the executor walk means there is no second grammar to diverge from the +/// one that runs. A flat `pipeline` desugars into this via +/// `Request::exec_plan`, so every current client keeps working unchanged. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "op", rename_all = "kebab-case")] +pub enum Plan { + /// A single literal command: prog + args. No word-splitting, globbing, or + /// substitution — the leaf of every tree. + Run { argv: Vec }, + /// Sequence; stop at the first non-accepted exit (Dune `progn`, ≈ `&&`/`;`). + Seq { steps: Vec }, + /// Connect stdout→stdin left-to-right (Dune `pipe-stdout`, = `|`). + Pipe { stages: Vec }, + /// Redirect a stream of the inner plan to a file or /dev/null. + Redirect { + stream: Stream, + target: RedirTarget, + #[serde(rename = "do")] + body: Box, + }, + /// Feed a file as stdin to the inner plan (Dune `with-stdin-from`, = `<`). + WithStdinFrom { + path: String, + #[serde(rename = "do")] + body: Box, + }, + /// Add an env var to the inner plan's scope (Dune `setenv`). Still subject + /// to the executor's env allowlist at run time. + Setenv { + key: String, + value: String, + #[serde(rename = "do")] + body: Box, + }, + /// Change cwd for the inner plan (Dune `chdir`). + Chdir { + dir: String, + #[serde(rename = "do")] + body: Box, + }, + /// Run the inner plan under a specific privilege context — `[ROOT]` or + /// `[as ]`. The axis Dune lacks; see `RunAs`. + RunAs { + who: RunAs, + #[serde(rename = "do")] + body: Box, + }, + /// Treat these exit codes as success (Dune `with-accepted-exit-codes`). + WithAcceptedExitCodes { + codes: Vec, + #[serde(rename = "do")] + body: Box, + }, + /// EXPLICIT opaque escape hatch (Dune `system`/`bash`). Rendered at the + /// prompt as "opaque — resolves only the shell"; a strict policy mode + /// rejects it. The sole surviving shell path, and always named. + System { shell: Shell, script: String }, +} + +impl Plan { + /// Recursively reject dangerous characters on **every** string leaf and + /// enforce the depth / node-count bounds. Mirrors the flat-pipeline checks + /// in `ValidatedRequest::validate` (same `has_dangerous_chars` predicate, + /// same non-empty rule) so the Rung 2 property test and Rung 3 Kani harness + /// extend to the tree rather than restarting. + /// + /// `nodes` accumulates the total node count across the whole tree so a + /// wide plan is bounded independently of depth. + pub(crate) fn validate_tree(&self, depth: usize, nodes: &mut usize) -> Result<(), String> { + if depth > MAX_PLAN_DEPTH { + return Err(format!("plan nesting exceeds {MAX_PLAN_DEPTH} levels")); + } + *nodes += 1; + if *nodes > MAX_PLAN_NODES { + return Err(format!("plan exceeds {MAX_PLAN_NODES} nodes")); + } + let check = |s: &str, what: &str| -> Result<(), String> { + if has_dangerous_chars(s) { + Err(format!("{what} contains forbidden control/bidi characters")) + } else { + Ok(()) + } + }; + match self { + Plan::Run { argv } => { + if argv.is_empty() { + return Err("run stage must have a command".to_string()); + } + for a in argv { + check(a, "argv element")?; + } + } + Plan::Seq { steps } => { + if steps.is_empty() { + return Err("seq must have at least one step".to_string()); + } + for s in steps { + s.validate_tree(depth + 1, nodes)?; + } + } + Plan::Pipe { stages } => { + if stages.len() < 2 { + return Err("pipe must have at least two stages".to_string()); + } + for s in stages { + s.validate_tree(depth + 1, nodes)?; + } + } + Plan::Redirect { target, body, .. } => { + if let RedirTarget::File { path, .. } = target { + check(path, "redirect path")?; + } + body.validate_tree(depth + 1, nodes)?; + } + Plan::WithStdinFrom { path, body } => { + check(path, "stdin path")?; + body.validate_tree(depth + 1, nodes)?; + } + Plan::Setenv { key, value, body } => { + check(key, "env key")?; + check(value, "env value")?; + body.validate_tree(depth + 1, nodes)?; + } + Plan::Chdir { dir, body } => { + check(dir, "chdir dir")?; + body.validate_tree(depth + 1, nodes)?; + } + Plan::RunAs { who, body } => { + if let RunAs::User { name } = who { + if name.is_empty() { + return Err("run-as user must be named".to_string()); + } + check(name, "run-as user")?; + } + body.validate_tree(depth + 1, nodes)?; + } + Plan::WithAcceptedExitCodes { body, .. } => { + body.validate_tree(depth + 1, nodes)?; + } + Plan::System { script, .. } => { + check(script, "system script")?; + } + } + Ok(()) + } + + /// True if any subtree imposes a `RunAs` context. Any run-as — root or a + /// named user — requires escalation the daemon must gate as privileged, so + /// this drives `Request::effective_privileged`. + pub fn contains_runas(&self) -> bool { + match self { + Plan::Run { .. } | Plan::System { .. } => false, + Plan::RunAs { .. } => true, + Plan::Seq { steps } => steps.iter().any(Plan::contains_runas), + Plan::Pipe { stages } => stages.iter().any(Plan::contains_runas), + Plan::Redirect { body, .. } + | Plan::WithStdinFrom { body, .. } + | Plan::Setenv { body, .. } + | Plan::Chdir { body, .. } + | Plan::WithAcceptedExitCodes { body, .. } => body.contains_runas(), + } + } + + /// True if any subtree is a root `RunAs`. Drives the prompt's "Execute as + /// root?" wording (vs. a milder "with elevated privileges?" when the only + /// escalation is to a named non-root user). + pub fn contains_root(&self) -> bool { + match self { + Plan::Run { .. } | Plan::System { .. } => false, + Plan::RunAs { who: RunAs::Root, .. } => true, + Plan::RunAs { who: RunAs::User { .. }, body } => body.contains_root(), + Plan::Seq { steps } => steps.iter().any(Plan::contains_root), + Plan::Pipe { stages } => stages.iter().any(Plan::contains_root), + Plan::Redirect { body, .. } + | Plan::WithStdinFrom { body, .. } + | Plan::Setenv { body, .. } + | Plan::Chdir { body, .. } + | Plan::WithAcceptedExitCodes { body, .. } => body.contains_root(), + } + } +} + +/// Desugar a flat `pipeline` into an equivalent `Plan` tree. A single stage +/// becomes a bare `Run`; multiple stages become a `Pipe`. Ambient privilege is +/// **not** encoded here — the executor seeds its run-as context from +/// `Request::privileged`, so a desugared tree and an explicit plan are handled +/// uniformly. An empty pipeline (control actions only) yields an empty `Seq`, +/// which never reaches execution. +fn desugar_pipeline(pipeline: &[Vec]) -> Plan { + let mut stages: Vec = pipeline + .iter() + .map(|argv| Plan::Run { argv: argv.clone() }) + .collect(); + match stages.len() { + 0 => Plan::Seq { steps: vec![] }, + 1 => stages.pop().unwrap(), + _ => Plan::Pipe { stages }, + } +} + #[derive(Clone, Debug, Deserialize, Serialize)] pub struct Request { #[serde(default = "default_id")] @@ -44,6 +320,15 @@ pub struct Request { pub time: String, #[serde(default)] pub pipeline: Vec>, + /// Optional structured execution plan. When present it is the canonical + /// form the daemon validates, executes, and renders; `pipeline` is then a + /// legacy sugar left empty. Exactly one of `plan` / `pipeline` carries the + /// command. `skip_serializing_if` keeps exec requests that use `pipeline` + /// byte-identical to the pre-plan wire format, and an old daemon that drops + /// this unknown key sees an empty `pipeline` and rejects cleanly at + /// validation. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub plan: Option, #[serde(default, skip_serializing_if = "Action::is_exec")] pub action: Action, #[serde(default)] @@ -91,6 +376,36 @@ impl Request { session, time: crate::datetime::now_iso8601(), pipeline, + plan: None, + action: Action::Exec, + env, + reason, + privileged, + forward_agent, + version: VERSION.to_string(), + } + } + + /// Build an outgoing exec request carrying a structured `plan` instead of a + /// flat `pipeline`. The `pipeline` field is left empty; the daemon reads the + /// command from `plan`. + #[allow(clippy::too_many_arguments)] + pub fn new_plan( + host: String, + session: String, + plan: Plan, + env: HashMap, + reason: String, + privileged: bool, + forward_agent: bool, + ) -> Self { + Self { + id: default_id(), + host, + session, + time: crate::datetime::now_iso8601(), + pipeline: vec![], + plan: Some(plan), action: Action::Exec, env, reason, @@ -111,6 +426,7 @@ impl Request { session, time: crate::datetime::now_iso8601(), pipeline: vec![], + plan: None, action, env: HashMap::new(), reason: String::new(), @@ -119,6 +435,31 @@ impl Request { version: VERSION.to_string(), } } + + /// The canonical execution plan: the explicit `plan` if present, otherwise + /// the flat `pipeline` desugared into an equivalent tree. Single source of + /// truth for validation, execution, and the approval-prompt render, so the + /// structure the human approves is exactly the structure the executor runs. + pub fn exec_plan(&self) -> Plan { + match &self.plan { + Some(p) => p.clone(), + None => desugar_pipeline(&self.pipeline), + } + } + + /// Whether this request must be gated as privileged. True when the top-level + /// `privileged` flag is set, *or* the plan contains any `RunAs` node — a + /// root or named-user escalation an unprivileged request must not smuggle + /// past the quieter unprivileged gate. + pub fn effective_privileged(&self) -> bool { + self.privileged || self.plan.as_ref().is_some_and(Plan::contains_runas) + } + + /// Whether any part of the request runs as root — the top-level privileged + /// flag, or an explicit root `RunAs`. Drives the prompt's "as root" wording. + pub fn runs_as_root(&self) -> bool { + self.privileged || self.plan.as_ref().is_some_and(Plan::contains_root) + } } fn default_session() -> String { @@ -171,11 +512,20 @@ impl ValidatedRequest { /// argv and env. See `tui::prompt_tty` for the render this protects. pub fn validate(req: Request) -> Result { // Control actions (Stop/Ping) carry no pipeline by design; only an - // Exec request must have one. Every string-field check below stays - // unconditional, so control requests are sanitized like any other. - if req.action == Action::Exec && req.pipeline.is_empty() { + // Exec request must have one. An explicit `plan` is the modern form, so + // an Exec request is "empty" only when it has neither. Every string- + // field check below stays unconditional, so control requests are + // sanitized like any other. + if req.action == Action::Exec && req.pipeline.is_empty() && req.plan.is_none() { return Err("pipeline must not be empty".to_string()); } + // A request carries at most one command form. Rejecting both-present + // removes the ambiguity of which one the executor would run. + if !req.pipeline.is_empty() && req.plan.is_some() { + return Err("request must not set both pipeline and plan".to_string()); + } + // Flat-pipeline path: keep the exact per-stage checks (and their error + // messages) the pre-plan wire format relied on. for (stage_idx, argv) in req.pipeline.iter().enumerate() { if argv.is_empty() { return Err(format!("pipeline stage {stage_idx} must not be empty")); @@ -188,6 +538,13 @@ impl ValidatedRequest { } } } + // Structured-plan path: recursively sanitize every string leaf and + // enforce the depth / node bounds, using the same `has_dangerous_chars` + // predicate as the flat path above. + if let Some(plan) = &req.plan { + let mut nodes = 0usize; + plan.validate_tree(0, &mut nodes)?; + } for key in req.env.keys() { if has_dangerous_chars(key) { return Err(format!("env key '{key}' contains forbidden characters")); @@ -402,4 +759,139 @@ mod tests { req.session.push('\u{1b}'); assert!(ValidatedRequest::validate(req).is_err()); } + + // === Structured plan schema =========================================== + + fn run(argv: &[&str]) -> Plan { + Plan::Run { argv: argv.iter().map(|s| s.to_string()).collect() } + } + + /// A single-stage pipeline desugars to a bare `Run`; a multi-stage pipeline + /// to a `Pipe`. This is the compat shim the whole backward-compatibility + /// story rests on. + #[test] + fn desugar_pipeline_shapes() { + let single = desugar_pipeline(&[vec!["ls".into(), "-l".into()]]); + assert_eq!(single, run(&["ls", "-l"])); + + let multi = desugar_pipeline(&[vec!["ls".into()], vec!["wc".into(), "-l".into()]]); + assert_eq!(multi, Plan::Pipe { stages: vec![run(&["ls"]), run(&["wc", "-l"])] }); + + assert_eq!(desugar_pipeline(&[]), Plan::Seq { steps: vec![] }); + } + + /// An explicit `plan` round-trips through JSON with the `op`-tagged form, + /// and a flat-pipeline exec request still omits the `plan` key entirely + /// (wire compat). + #[test] + fn plan_wire_roundtrip_and_omission() { + let req = Request::new_plan( + String::new(), + "s".into(), + Plan::Seq { steps: vec![run(&["true"]), run(&["false"])] }, + HashMap::new(), + String::new(), + true, + false, + ); + let json = serde_json::to_string(&req).unwrap(); + assert!(json.contains(r#""op":"seq""#), "tagged form missing: {json}"); + let back: Request = serde_json::from_str(&json).unwrap(); + assert_eq!(back.exec_plan(), req.exec_plan()); + + // A pipeline request must not serialize a `plan` key. + let flat = exec_request(); + let flat_json = serde_json::to_string(&flat).unwrap(); + assert!(!flat_json.contains("plan"), "flat request leaked plan key: {flat_json}"); + } + + /// `effective_privileged` is true for a top-level privileged flag, and also + /// for an unprivileged request whose plan smuggles any `RunAs` node. + #[test] + fn effective_privileged_accounts_for_runas() { + let mut req = Request::new_plan( + String::new(), + "s".into(), + Plan::RunAs { + who: RunAs::Root, + body: Box::new(run(&["sysctl", "-w", "x=1"])), + }, + HashMap::new(), + String::new(), + false, // unprivileged flag... + false, + ); + assert!(req.effective_privileged(), "root RunAs must force privileged"); + assert!(req.runs_as_root()); + + // A named-user RunAs still forces the privileged gate, but is not root. + req.plan = Some(Plan::RunAs { + who: RunAs::User { name: "alice".into() }, + body: Box::new(run(&["dune", "build"])), + }); + assert!(req.effective_privileged()); + assert!(!req.runs_as_root()); + + // A plain plan with no RunAs and no flag is unprivileged. + req.plan = Some(run(&["ls"])); + req.privileged = false; + assert!(!req.effective_privileged()); + } + + #[test] + fn validate_rejects_dangerous_leaf_in_plan() { + let req = Request::new_plan( + String::new(), + "s".into(), + Plan::Setenv { + key: "LANG".into(), + value: "x\u{1b}y".into(), // ESC in a setenv value + body: Box::new(run(&["true"])), + }, + HashMap::new(), + String::new(), + false, + false, + ); + let err = ValidatedRequest::validate(req).unwrap_err(); + assert!(err.contains("forbidden"), "got: {err}"); + } + + #[test] + fn validate_rejects_both_pipeline_and_plan() { + let mut req = exec_request(); + req.plan = Some(run(&["ls"])); + let err = ValidatedRequest::validate(req).unwrap_err(); + assert!(err.contains("both pipeline and plan"), "got: {err}"); + } + + #[test] + fn validate_rejects_overdeep_plan() { + // Nest RunAs past MAX_PLAN_DEPTH. + let mut p = run(&["true"]); + for _ in 0..(MAX_PLAN_DEPTH + 2) { + p = Plan::RunAs { who: RunAs::Root, body: Box::new(p) }; + } + let req = Request::new_plan( + String::new(), "s".into(), p, HashMap::new(), String::new(), true, false, + ); + let err = ValidatedRequest::validate(req).unwrap_err(); + assert!(err.contains("nesting exceeds"), "got: {err}"); + } + + #[test] + fn validate_rejects_empty_run_and_short_pipe() { + let empty_run = Request::new_plan( + String::new(), "s".into(), Plan::Run { argv: vec![] }, + HashMap::new(), String::new(), false, false, + ); + assert!(ValidatedRequest::validate(empty_run).is_err()); + + let short_pipe = Request::new_plan( + String::new(), "s".into(), + Plan::Pipe { stages: vec![run(&["ls"])] }, + HashMap::new(), String::new(), false, false, + ); + assert!(ValidatedRequest::validate(short_pipe).is_err()); + } } diff --git a/src/server.rs b/src/server.rs index f8820a2..fee4c12 100644 --- a/src/server.rs +++ b/src/server.rs @@ -557,8 +557,9 @@ fn handle_connection( // Agent forwarding is unprivileged-only. A privileged child runs under // sudo (which env_resets) or pkexec (which we env_clear), so the socket // would be silently dropped — but we reject explicitly so a misconfigured - // caller fails loudly instead of being puzzled by a missing key. - if req.forward_agent && req.privileged { + // caller fails loudly instead of being puzzled by a missing key. A plan + // that escalates via a `run-as` node counts as privileged here too. + if req.forward_agent && req.effective_privileged() { let resp = Response::error( &req.id, "forward_agent is only allowed for unprivileged commands", @@ -638,14 +639,30 @@ fn handle_connection( crate::protocol::Action::Exec => {} } + // Reject a structurally-invalid plan (e.g. redirect of a composite, a + // disallowed setenv) before prompting — the human should never be asked to + // approve something the executor cannot run. Cheap compile; the executor + // compiles again at run time. + if let Err(msg) = crate::executor::plan_display(&req) { + let resp = Response::error(&req.id, &format!("invalid plan: {msg}")); + let _ = write_response(&mut stream, &resp); + return; + } + + let effective_privileged = req.effective_privileged(); + if verbose { - let priv_label = if req.privileged { "privileged" } else { "unprivileged" }; - let pipeline_display = crate::tui::pipeline_join(&req.pipeline); + let priv_label = if effective_privileged { + "privileged" + } else { + "unprivileged" + }; + let cmd_display = crate::tui::command_summary(&req); let client_ver = if req.version.is_empty() { "unknown" } else { req.version.as_str() }; - eprintln!("[{}] [{}] [client {}] {}", req.id, priv_label, client_ver, pipeline_display); + eprintln!("[{}] [{}] [client {}] {}", req.id, priv_label, client_ver, cmd_display); } - let resp = if req.privileged { + let resp = if effective_privileged { if pkexec_only && mode == Mode::Local { // --pkexec: pkexec itself handles auth and approval; no TTY lock. exec_pkexec(&req, &env, &tty_lock) @@ -687,7 +704,7 @@ fn handle_connection( eprintln!( "[{}] [unattended] {}", req.id, - crate::tui::pipeline_join(&req.pipeline) + crate::tui::command_summary(&req) ); if let Ok(_g) = tty_lock.try_lock() { let _ = tui::display_banner(&req); @@ -726,9 +743,9 @@ fn handle_connection( // unprivileged path uses try_lock so a still-pending privileged prompt // never delays an unprivileged command's completion. Skip pkexec // because pkexec has its own dialog. - let used_pkexec = req.privileged && pkexec_only && mode == Mode::Local; + let used_pkexec = effective_privileged && pkexec_only && mode == Mode::Local; if !used_pkexec { - if req.privileged { + if effective_privileged { let _g = lock_recover(&tty_lock); let _ = result_sink.display(&resp); } else if let Ok(_g) = tty_lock.try_lock() { @@ -1189,6 +1206,7 @@ mod tests { session: rand_chars(rng, 6), time: crate::datetime::now_iso8601(), pipeline: std::mem::take(&mut pipeline), + plan: None, action: crate::protocol::Action::Exec, env, reason: rand_chars(rng, 8), diff --git a/src/tui.rs b/src/tui.rs index 0d4ae57..de75ad6 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -5,8 +5,8 @@ use std::time::Duration; use base64::engine::general_purpose::STANDARD as B64; use base64::Engine; -use crate::executor::which; -use crate::protocol::{Response, Status, ValidatedRequest}; +use crate::executor::{plan_display, which, LeafPrivilege, LeafView}; +use crate::protocol::{Plan, RedirTarget, Response, RunAs, Status, Stream, ValidatedRequest}; /// Join argv into a shell-like display string, quoting args that contain spaces. pub fn shell_join(argv: &[String]) -> String { @@ -31,6 +31,73 @@ pub fn pipeline_join(pipeline: &[Vec]) -> String { .join(" | ") } +/// A one-line summary of a request's command, for logs and the compact +/// unattended banner. Renders the canonical `Plan` (a flat `pipeline` desugars +/// into one), so it covers both the legacy pipeline form and structured plans. +pub fn command_summary(req: &ValidatedRequest) -> String { + plan_to_string(&req.exec_plan()) +} + +/// Recursive one-line rendering of a plan tree. Structural, not resolved — +/// the detailed per-leaf resolution happens in the multi-line prompt render. +fn plan_to_string(plan: &Plan) -> String { + match plan { + Plan::Run { argv } => shell_join(argv), + Plan::System { shell, script } => { + let bin = match shell { + crate::protocol::Shell::Sh => "sh", + crate::protocol::Shell::Bash => "bash", + }; + shell_join(&[bin.to_string(), "-c".to_string(), script.clone()]) + } + Plan::Pipe { stages } => stages + .iter() + .map(plan_to_string) + .collect::>() + .join(" | "), + Plan::Seq { steps } => steps + .iter() + .map(plan_to_string) + .collect::>() + .join(" ; "), + Plan::Redirect { stream, target, body } => { + let op = match (stream, target) { + (Stream::Stdout, RedirTarget::File { path, mode }) => { + format!("{} {path}", redir_arrow(*mode)) + } + (Stream::Stderr, RedirTarget::File { path, mode }) => { + format!("2{} {path}", redir_arrow(*mode)) + } + (Stream::Both, RedirTarget::File { path, mode }) => { + format!("{} {path} 2>&1", redir_arrow(*mode)) + } + (Stream::Stdout, RedirTarget::DevNull) => "> /dev/null".to_string(), + (Stream::Stderr, RedirTarget::DevNull) => "2> /dev/null".to_string(), + (Stream::Both, RedirTarget::DevNull) => "> /dev/null 2>&1".to_string(), + }; + format!("{} {op}", plan_to_string(body)) + } + Plan::WithStdinFrom { path, body } => format!("{} < {path}", plan_to_string(body)), + Plan::Setenv { key, value, body } => format!("{key}={value} {}", plan_to_string(body)), + Plan::Chdir { dir, body } => format!("(cd {dir}; {})", plan_to_string(body)), + Plan::RunAs { who, body } => { + let tag = match who { + RunAs::Root => "[ROOT]".to_string(), + RunAs::User { name } => format!("[as {name}]"), + }; + format!("{tag} {}", plan_to_string(body)) + } + Plan::WithAcceptedExitCodes { body, .. } => plan_to_string(body), + } +} + +fn redir_arrow(mode: crate::protocol::WriteMode) -> &'static str { + match mode { + crate::protocol::WriteMode::Append => ">>", + crate::protocol::WriteMode::Truncate => ">", + } +} + /// Bold/dim/reset styling for the approval prompt — no color. All escapes are /// empty when NO_COLOR is set (https://no-color.org), so the prompt degrades to /// plain text. Output always targets /dev/tty, so this is the only gate needed. @@ -95,6 +162,12 @@ fn alert_for(no_bell: bool, no_blink: bool) -> Alert { /// rather than benign. const MAX_DISPLAY_CMD_CHARS: usize = 1024; +/// Upper bound on the number of plan stages rendered line-by-line at the prompt. +/// A plan with more stages than this has its tail hidden behind an explicit +/// marker, so a wide plan cannot push the `[y/N]` question off-screen — the same +/// anti-flooding rationale as `MAX_DISPLAY_CMD_CHARS`, applied to stage count. +const MAX_DISPLAY_STAGES: usize = 32; + /// Truncate an approval-prompt command string to a bounded number of /// characters, appending an explicit marker when it was shortened. pub fn truncate_for_display(s: &str) -> String { @@ -222,29 +295,18 @@ pub fn prompt_tty( writeln!( tty_w, "Command: {bold}{}{reset}{agent_tag}", - truncate_for_display(&pipeline_join(&req.pipeline)) + truncate_for_display(&command_summary(req)) )?; - // Show resolved path for the first stage's command. Symlinks are - // followed via canonicalize so a same-UID adversary who substitutes - // /usr/local/bin/foo -> /tmp/evil cannot hide the redirection from - // the prompt. - if let Some(first_argv) = req.pipeline.first() { - if let Some(cmd_name) = first_argv.first() { - write_resolves_line(&mut tty_w, cmd_name, &st)?; - } - - // Warn if later stages have commands not in PATH - for argv in req.pipeline.iter().skip(1) { - if let Some(cmd_name) = argv.first() { - if which(cmd_name).is_none() { - writeln!( - tty_w, - "Warning: {bold}{cmd_name}{reset} not found in PATH" - )?; - } - } - } + // Render every stage the executor will run, each with its run-as context + // and its resolved real binary. This is the structural guarantee the plan + // model buys: the vacuous single `/usr/bin/bash` resolves line is replaced + // by one resolved binary per literal stage, so a bundle of root effects can + // no longer hide inside one opaque command. Built from the same + // `compile_plan` walk the executor uses, so prompt and execution agree. + match plan_display(req) { + Ok(views) => write_plan_stages(&mut tty_w, &views, &st)?, + Err(e) => writeln!(tty_w, "{bold}⚠ invalid plan: {e}{reset}")?, } if !req.env.is_empty() { @@ -252,13 +314,18 @@ pub fn prompt_tty( writeln!(tty_w, "Env: {}", env_display.join(" "))?; } - // `a` is offered only when this daemon is `unattended_eligible` (an - // out-of-band operator opt-in). Without it, unprivileged commands get the - // same plain `[y/N]` gate as privileged ones — no way to grant unattended - // execution from the prompt. - let offer_always = eligible && !req.privileged; - let (question, choices) = if req.privileged { + // Effective privilege, not the raw `privileged` flag: a plan that smuggles + // a `RunAs` node is gated exactly like a top-level privileged request, so + // `a` is never offered for it and root wording is shown when any stage is + // root. `a` is offered only when this daemon is `unattended_eligible` (an + // out-of-band operator opt-in). + let effective_priv = req.effective_privileged(); + let offer_always = eligible && !effective_priv; + let (question, choices) = if req.runs_as_root() { ("Execute as root?", "[y/N]") + } else if effective_priv { + // Escalation only to a named non-root user (no root stage). + ("Execute with elevated privileges?", "[y/N]") } else if offer_always { writeln!( tty_w, @@ -274,8 +341,9 @@ pub fn prompt_tty( // Read single keypress with timeout (no Enter needed), then classify it // with the pure decision function so the TTY echo and the security- - // relevant mapping cannot drift apart. - let result = classify_key(read_key_timeout(&tty_r, timeout)?, req.privileged); + // relevant mapping cannot drift apart. Pass effective privilege so `a` + // (ApprovedAlways) can never be produced for a plan that escalates. + let result = classify_key(read_key_timeout(&tty_r, timeout)?, effective_priv); let label = match result { PromptResult::Timeout => "Timeout", PromptResult::Approved => "Approved", @@ -311,6 +379,69 @@ fn write_question_line( ) } +/// Render every stage of a compiled plan, one block per leaf: a header line +/// with the run-as tag, the stage's argv (and any redirections), then its +/// resolved real binary. A single plain unprivileged stage degrades to the +/// classic one-line `Resolves:` form so simple prompts stay simple. +fn write_plan_stages(w: &mut W, views: &[LeafView], style: &Style) -> io::Result<()> { + let Style { bold, dim, reset } = *style; + + // Simple case: one stage, no escalation, not opaque, no redirections — keep + // the familiar single `Resolves:` line. + if views.len() == 1 + && views[0].privilege == LeafPrivilege::Direct + && !views[0].opaque + && views[0].redirects.is_empty() + { + if let Some(name) = views[0].argv.first() { + return write_resolves_line(w, name, style); + } + return Ok(()); + } + + let shown = views.len().min(MAX_DISPLAY_STAGES); + for view in &views[..shown] { + let tag = match &view.privilege { + LeafPrivilege::Direct => String::new(), + LeafPrivilege::Root => format!("{bold}[ROOT]{reset} "), + LeafPrivilege::User(name) => format!("{bold}[as {name}]{reset} "), + }; + let redir = if view.redirects.is_empty() { + String::new() + } else { + format!(" {dim}{}{reset}", view.redirects) + }; + writeln!( + w, + " {n}. {tag}{bold}{cmd}{reset}{redir}", + n = view.stage_id + 1, + cmd = truncate_for_display(&shell_join(&view.argv)), + )?; + // Resolve line for the stage, indented under it. + write!(w, " ")?; + if view.opaque { + match &view.resolved_bin { + Some(p) => writeln!( + w, + "{bold}opaque — resolves only the shell: {}{reset}", + p.display() + )?, + None => writeln!(w, "{bold}opaque — shell not found in PATH{reset}")?, + } + } else if let Some(name) = view.argv.first() { + write_resolves_line(w, name, style)?; + } + } + if views.len() > shown { + let hidden = views.len() - shown; + writeln!( + w, + " {bold}… {hidden} more stage(s) hidden — full plan not shown{reset}" + )?; + } + Ok(()) +} + /// Write the `Resolves:` line for a command name. PATH-searches the /// name (or accepts an absolute path), then `canonicalize`s the result /// so symlink redirection — including `/usr/local/bin/foo -> /tmp/evil` @@ -371,7 +502,7 @@ pub fn display_banner(req: &ValidatedRequest) -> io::Result<()> { Err(_) => return Ok(()), }; let Style { dim, reset, .. } = style(); - let cmd = pipeline_join(&req.pipeline); + let cmd = command_summary(req); if req.forward_agent { writeln!(tty, "{dim}\u{25b6}{reset} {cmd} {dim}(agent forwarded){reset}")?; } else { @@ -601,6 +732,80 @@ mod tests { String::from_utf8(buf).expect("utf8") } + /// Every stage of a plan is rendered with its run-as tag and its own + /// resolved binary — the structural fix for the vacuous single + /// `/usr/bin/bash` resolves line. A `System` stage is flagged opaque. + #[test] + fn plan_stages_render_per_leaf_privilege_and_resolves() { + use crate::protocol::{Plan, Request, RunAs, Shell, ValidatedRequest}; + + let plan = Plan::Seq { + steps: vec![ + Plan::RunAs { + who: RunAs::Root, + body: Box::new(Plan::Run { argv: vec!["true".into()] }), + }, + Plan::RunAs { + who: RunAs::User { name: "alice".into() }, + body: Box::new(Plan::Run { argv: vec!["true".into()] }), + }, + Plan::System { + shell: Shell::Sh, + script: "echo hi".into(), + }, + ], + }; + let req = ValidatedRequest::validate(Request::new_plan( + String::new(), + "t".into(), + plan, + std::collections::HashMap::new(), + String::new(), + false, + false, + )) + .expect("valid"); + + let views = plan_display(&req).expect("compiles"); + let mut buf: Vec = Vec::new(); + let st = Style { bold: "", dim: "", reset: "" }; + write_plan_stages(&mut buf, &views, &st).expect("write"); + let out = String::from_utf8(buf).expect("utf8"); + + assert!(out.contains("[ROOT]"), "missing root tag: {out}"); + assert!(out.contains("[as alice]"), "missing user tag: {out}"); + assert!( + out.contains("opaque — resolves only the shell"), + "System stage must be flagged opaque: {out}" + ); + // The two `true` stages each get their own resolved line. + assert_eq!(out.matches("Resolves:").count(), 2, "one resolves per non-opaque stage: {out}"); + } + + /// A simple single unprivileged command keeps the classic one-line + /// `Resolves:` form (no numbered stage block). + #[test] + fn single_plain_command_uses_compact_resolves() { + use crate::protocol::{Plan, Request, ValidatedRequest}; + let req = ValidatedRequest::validate(Request::new_plan( + String::new(), + "t".into(), + Plan::Run { argv: vec!["true".into()] }, + std::collections::HashMap::new(), + String::new(), + false, + false, + )) + .expect("valid"); + let views = plan_display(&req).expect("compiles"); + let mut buf: Vec = Vec::new(); + let st = Style { bold: "", dim: "", reset: "" }; + write_plan_stages(&mut buf, &views, &st).expect("write"); + let out = String::from_utf8(buf).expect("utf8"); + assert!(out.starts_with("Resolves:"), "compact form expected: {out}"); + assert!(!out.contains("1. "), "no numbered block for a single plain stage: {out}"); + } + #[test] fn style_for_honors_no_color() { let on = style_for(false); diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 4113e26..707aa06 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -274,6 +274,7 @@ pub fn make_req(id: &str, pipeline: Vec>) -> Request { .into_iter() .map(|v| v.into_iter().map(String::from).collect()) .collect(), + plan: None, action: sudo_proxy::protocol::Action::Exec, env: Default::default(), reason: String::new(), diff --git a/tests/exec.rs b/tests/exec.rs index 2bcd3a5..a7585ff 100644 --- a/tests/exec.rs +++ b/tests/exec.rs @@ -23,6 +23,7 @@ fn make_req(pipeline: Vec>) -> ValidatedRequest { .into_iter() .map(|v| v.into_iter().map(String::from).collect()) .collect(), + plan: None, action: sudo_proxy::protocol::Action::Exec, env: HashMap::new(), reason: String::new(), diff --git a/tests/plan_exec.rs b/tests/plan_exec.rs new file mode 100644 index 0000000..cdfbb5f --- /dev/null +++ b/tests/plan_exec.rs @@ -0,0 +1,203 @@ +#![cfg(unix)] + +//! Structured-plan execution: the daemon runs a `Plan` tree directly (pipes, +//! redirections, env, sequencing) with no shell on the privileged path. These +//! drive `exec_direct` (unprivileged, so every stage runs as the test user) and +//! one end-to-end case through the real daemon. + +use std::collections::HashMap; + +use sudo_proxy::executor::exec_direct; +use sudo_proxy::protocol::{ + Plan, RedirTarget, Request, Status, Stream, ValidatedRequest, WriteMode, +}; + +mod common; + +fn run(argv: &[&str]) -> Plan { + Plan::Run { + argv: argv.iter().map(|s| s.to_string()).collect(), + } +} + +fn make_plan_req(plan: Plan) -> ValidatedRequest { + let req = Request::new_plan( + String::new(), + "test".into(), + plan, + HashMap::new(), + String::new(), + false, // unprivileged: all stages run directly as the test user + false, + ); + ValidatedRequest::validate(req).expect("valid plan request") +} + +fn decode_stdout(resp: &sudo_proxy::protocol::Response) -> String { + String::from_utf8_lossy(&common::b64_decode(resp.stdout.as_deref().unwrap_or(""))).into_owned() +} + +/// A pipe nested inside a seq: three leaves, and the response's stage list and +/// aggregate stdout match the pre-order leaf layout. +#[test] +fn pipe_inside_seq_runs_and_aggregates() { + let plan = Plan::Seq { + steps: vec![ + run(&["echo", "alpha"]), + Plan::Pipe { + stages: vec![run(&["echo", "hi"]), run(&["wc", "-c"])], + }, + ], + }; + let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); + assert_eq!(resp.status, Status::Ok, "got {resp:?}"); + // Three Run leaves → three stages, in pre-order. + assert_eq!(resp.stages.len(), 3, "stages: {:?}", resp.stages); + let out = decode_stdout(&resp); + // echo alpha (captured, standalone) + wc -c of "hi\n" == 3 (pipe tail). + assert!(out.contains("alpha"), "stdout missing echo output: {out:?}"); + assert!(out.contains('3'), "stdout missing wc result: {out:?}"); +} + +/// `>` redirects a stage's stdout to a file; the aggregate stdout is then empty. +#[test] +fn redirect_stdout_to_file_truncate() { + let tmp = tempfile::tempdir().expect("tempdir"); + let path = tmp.path().join("out.txt"); + let plan = Plan::Redirect { + stream: Stream::Stdout, + target: RedirTarget::File { + path: path.to_string_lossy().into_owned(), + mode: WriteMode::Truncate, + }, + body: Box::new(run(&["printf", "hello-file"])), + }; + let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); + assert_eq!(resp.status, Status::Ok, "got {resp:?}"); + let written = std::fs::read_to_string(&path).expect("read redirect file"); + assert_eq!(written, "hello-file"); + assert!(decode_stdout(&resp).is_empty(), "redirected stdout must not be captured"); +} + +/// `>>` appends rather than truncating. +#[test] +fn redirect_stdout_append() { + let tmp = tempfile::tempdir().expect("tempdir"); + let path = tmp.path().join("acc.txt"); + std::fs::write(&path, "seed-").unwrap(); + let plan = Plan::Redirect { + stream: Stream::Stdout, + target: RedirTarget::File { + path: path.to_string_lossy().into_owned(), + mode: WriteMode::Append, + }, + body: Box::new(run(&["printf", "more"])), + }; + let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); + assert_eq!(resp.status, Status::Ok, "got {resp:?}"); + assert_eq!(std::fs::read_to_string(&path).unwrap(), "seed-more"); +} + +/// `setenv` scopes a variable to its subtree, and the allowlist still applies. +#[test] +fn setenv_is_visible_to_the_stage() { + let plan = Plan::Setenv { + key: "LANG".into(), + value: "xyz-sentinel".into(), + body: Box::new(run(&["sh", "-c", "printf %s \"$LANG\""])), + }; + let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); + assert_eq!(resp.status, Status::Ok, "got {resp:?}"); + assert_eq!(decode_stdout(&resp), "xyz-sentinel"); +} + +/// A seq stops at the first failing step; later stages do not run. +#[test] +fn seq_short_circuits_on_failure() { + let plan = Plan::Seq { + steps: vec![ + run(&["false"]), + run(&["sh", "-c", "printf SHOULD_NOT_RUN"]), + ], + }; + let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); + assert_eq!(resp.status, Status::Ok, "got {resp:?}"); + assert_eq!(resp.stages.len(), 2); + assert_eq!(resp.stages[0].exit_code, 1, "false exits 1"); + assert!( + !decode_stdout(&resp).contains("SHOULD_NOT_RUN"), + "second stage must not run after a failure" + ); +} + +/// `with-accepted-exit-codes` lets a non-zero exit count as success, so the +/// seq continues past it. +#[test] +fn accepted_exit_codes_allow_seq_to_continue() { + let plan = Plan::Seq { + steps: vec![ + Plan::WithAcceptedExitCodes { + codes: vec![1], + body: Box::new(run(&["false"])), + }, + run(&["sh", "-c", "printf continued"]), + ], + }; + let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); + assert_eq!(resp.status, Status::Ok, "got {resp:?}"); + assert!( + decode_stdout(&resp).contains("continued"), + "accepted exit code should not stop the seq: {resp:?}" + ); +} + +/// The explicit `System` escape hatch runs a shell script verbatim. +#[test] +fn system_escape_hatch_runs() { + let plan = Plan::System { + shell: sudo_proxy::protocol::Shell::Sh, + script: "printf hatch-ran".into(), + }; + let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); + assert_eq!(resp.status, Status::Ok, "got {resp:?}"); + assert_eq!(resp.stages.len(), 1); + assert_eq!(decode_stdout(&resp), "hatch-ran"); +} + +/// End-to-end through the real daemon: a `plan`-bearing request is validated, +/// prompted (scripted-approved), executed, and its structured result returned. +#[test] +fn plan_request_end_to_end_through_daemon() { + let server = common::start_test_server(common::TestServerOpts::default()); + + let plan = Plan::Pipe { + stages: vec![run(&["printf", "a\\nb\\nc\\n"]), run(&["wc", "-l"])], + }; + let mut req = Request::new_plan( + String::new(), + "test".into(), + plan, + HashMap::new(), + String::new(), + false, + false, + ); + // The test harness' freshness window wants a current timestamp; new_plan + // already stamps one, but normalize via the shared helper to be safe. + req.time = common::iso_now(); + + let resp = common::send_request(&server.socket_path, &req); + assert_eq!(resp.status, Status::Ok, "got {resp:?}"); + assert_eq!(resp.stages.len(), 2); + assert_eq!( + decode_stdout(&resp).trim(), + "3", + "wc -l over three lines; full resp {resp:?}" + ); + + // The prompter was consulted for the plan request. + assert!( + !server.prompter.calls().is_empty(), + "daemon should have prompted" + ); +} diff --git a/tests/validation.rs b/tests/validation.rs index e21f752..7918af3 100644 --- a/tests/validation.rs +++ b/tests/validation.rs @@ -232,3 +232,99 @@ fn missing_privileged_field_defaults_to_privileged() { assert_eq!(s.prompter.call_count(), 1); assert_eq!(parsed["id"], "defprivd"); } + +// === Structured plan, at the wire ====================================== + +use sudo_proxy::protocol::{Plan, Request, RunAs}; + +fn plan_req(id: &str, plan: Plan, privileged: bool) -> Request { + let mut req = Request::new_plan( + String::new(), + "test".into(), + plan, + Default::default(), + String::new(), + privileged, + false, + ); + req.id = id.to_string(); + req.time = iso_now(); + req +} + +/// A dangerous char anywhere in a plan leaf is rejected at the daemon, same as +/// in a flat pipeline. +#[test] +fn plan_dangerous_leaf_rejected() { + let s = server(); + let plan = Plan::Run { + argv: vec!["echo".into(), "x\u{202E}y".into()], + }; + let resp = s.send(&plan_req("v-plan-bidi", plan, false)); + assert_eq!(resp.status, Status::Error); + assert!(resp.message.as_deref().unwrap().contains("forbidden")); +} + +/// A request may not carry both a `pipeline` and a `plan`. +#[test] +fn plan_and_pipeline_both_set_rejected() { + let s = server(); + let mut req = plan_req( + "v-both", + Plan::Run { argv: vec!["true".into()] }, + false, + ); + req.pipeline = vec![vec!["true".into()]]; + let resp = s.send(&req); + assert_eq!(resp.status, Status::Error); + assert!(resp.message.as_deref().unwrap().contains("both pipeline and plan")); +} + +/// An over-nested plan is rejected by the depth bound before any prompt. +#[test] +fn plan_overdeep_rejected() { + let s = server(); + let mut p = Plan::Run { argv: vec!["true".into()] }; + for _ in 0..40 { + p = Plan::RunAs { + who: RunAs::Root, + body: Box::new(p), + }; + } + let resp = s.send(&plan_req("v-deep", p, true)); + assert_eq!(resp.status, Status::Error); + assert!(resp.message.as_deref().unwrap().contains("nesting exceeds")); +} + +/// A structurally-invalid-but-char-clean plan (a redirect wrapping a seq) is +/// rejected before prompting — the daemon never asks the human to approve +/// something it cannot run. +#[test] +fn plan_structural_error_rejected_without_prompt() { + let s = server(); + let plan = Plan::Redirect { + stream: sudo_proxy::protocol::Stream::Stdout, + target: sudo_proxy::protocol::RedirTarget::DevNull, + body: Box::new(Plan::Seq { + steps: vec![ + Plan::Run { argv: vec!["true".into()] }, + Plan::Run { argv: vec!["true".into()] }, + ], + }), + }; + let resp = s.send(&plan_req("v-struct", plan, false)); + assert_eq!(resp.status, Status::Error); + assert!(resp.message.as_deref().unwrap().contains("invalid plan")); + assert_eq!(s.prompter.call_count(), 0, "must not prompt for an invalid plan"); +} + +/// A valid unprivileged plan runs end-to-end and returns Ok. +#[test] +fn plan_valid_runs() { + let s = server(); + let plan = Plan::Run { + argv: vec!["true".into()], + }; + let resp = s.send(&plan_req("v-plan-ok", plan, false)); + assert_eq!(resp.status, Status::Ok, "got: {:?}", resp); +} From 43c8ecd2b5452883318804764b0beb9b48128ec4 Mon Sep 17 00:00:00 2001 From: Cuihtlauac ALVARADO Date: Sat, 17 Oct 2026 10:06:19 +0200 Subject: [PATCH 02/14] =?UTF-8?q?feat(plan):=20add=20`alt`=20op=20(shell?= =?UTF-8?q?=20||)=20=E2=80=94=20run=20steps=20until=20one=20succeeds?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plan model had `seq` (run in order, stop at the first non-accepted failure, ≈ &&/;) but no run-on-failure branch, so `pgrep … || echo none` was inexpressible (backlog "|| fallback" gap). Add `alt` as the exact dual of `seq`: run steps left-to-right, continue past a non-accepted failure, stop on the first success. A first-class op, not a fold into `with-accepted-exit-codes` ("exit N counts as success" is a different concept from "run the fallback because the prior step failed"). - protocol: `Plan::Alt { steps }` (serde `{"op":"alt",...}`); validate_tree rejects empty; contains_runas/contains_root treat it like seq so the privilege gate still holds. - executor: `SeqNode::Alt`; compile_seq arm (same stdio-default guard as seq); compile_leaf rejects it as a pipe-stage/redirect body; run_seq inverts seq's stop condition (stop on success). Reuses exit_accepted and the plan-wide deadline unchanged. No new shell on the privileged path. - prompt/docs: one-line render joins with " || "; MCP `plan` doc documents alt as the dual of seq. - tests: 4 plan_exec cases (short-circuit-on-success, fallback-on-failure, all-fail-runs-every-step, accepted-exit-stops-alt), redirect-of-alt rejected without prompt, empty-alt rejection, wire round-trip; the Rung-2 property tests now exercise alt via random_plan. Closes the "|| fallback" backlog item. Co-Authored-By: Claude Opus 4.8 (1M context) --- backlog.md | 4 +-- src/executor.rs | 43 ++++++++++++++++++++++--- src/mcp.rs | 5 ++- src/protocol.rs | 41 ++++++++++++++++++++++-- src/tui.rs | 5 +++ tests/plan_exec.rs | 77 +++++++++++++++++++++++++++++++++++++++++++++ tests/validation.rs | 21 +++++++++++++ 7 files changed, 187 insertions(+), 9 deletions(-) diff --git a/backlog.md b/backlog.md index de871ba..99321e2 100644 --- a/backlog.md +++ b/backlog.md @@ -164,8 +164,8 @@ on top:** requests `3a7cb1f9`, `e6aeb649`, `942cd83f`, `b1f6294a`). Dune actions are synchronous; `concurrent` still joins. We need launch-and-detach returning the pid as a structured field (not a shell `$!`). -- **`||` fallback.** `progn` ≈ `;`/`&&` (stops on failure); there is no - run-on-failure branch for the log's `pgrep … || echo none`. Add it, or fold it +- ~~**`||` fallback.**~~ *Landed:* the `alt` op (dual of `seq`) runs steps until + one succeeds, covering `pgrep … || echo none`. A first-class op, not a fold into `with-accepted-exit-codes`. - **Append redirection (`>>`).** `with-stdout-to` truncates; no append mode. - **Runtime substitution** (`$(cat /proc/…)`, `$!`, `$?`). Dune resolves diff --git a/src/executor.rs b/src/executor.rs index dacead6..2bc3f36 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -263,6 +263,7 @@ enum SeqNode { Leaf(usize), Pipe(Vec), Seq(Vec), + Alt(Vec), } /// Immutable compile context threaded down the tree. @@ -372,6 +373,16 @@ fn compile_seq(plan: &Plan, mut ctx: Ctx, leaves: &mut Vec) -> Result< } Ok(SeqNode::Seq(nodes)) } + Plan::Alt { steps } => { + if !ctx.stdio_is_default() { + return Err("redirect / stdin-from of an alt is not supported".to_string()); + } + let mut nodes = Vec::with_capacity(steps.len()); + for s in steps { + nodes.push(compile_seq(s, ctx.clone(), leaves)?); + } + Ok(SeqNode::Alt(nodes)) + } Plan::Pipe { stages } => { if !ctx.stdio_is_default() { return Err("redirect / stdin-from of a pipe is not supported".to_string()); @@ -421,7 +432,7 @@ fn compile_leaf(plan: &Plan, mut ctx: Ctx, leaves: &mut Vec) -> Result ctx.stdin = InSpec::File(path.clone()); compile_leaf(body, ctx, leaves) } - Plan::Seq { .. } | Plan::Pipe { .. } => { + Plan::Seq { .. } | Plan::Alt { .. } | Plan::Pipe { .. } => { Err("expected a single command here, found a composite plan".to_string()) } } @@ -568,7 +579,7 @@ fn mark_pipe_members(node: &SeqNode, in_pipe: &mut [bool]) { in_pipe[i] = true; } } - SeqNode::Seq(nodes) => { + SeqNode::Seq(nodes) | SeqNode::Alt(nodes) => { for n in nodes { mark_pipe_members(n, in_pipe); } @@ -739,6 +750,25 @@ fn run_seq( } last } + SeqNode::Alt(nodes) => { + // Dual of `Seq`: run steps until one succeeds. Keep going past a + // non-accepted failure; stop on the first success (and on a timeout + // or spawn error, which abort the whole alt just as they do a seq). + let mut last = StepStatus { + timed_out: false, + spawn_error: None, + success: false, + }; + for n in nodes { + let st = run_seq(n, leaves, mode, deadline, tty_lock, outcomes); + let stop = st.timed_out || st.spawn_error.is_some() || st.success; + last = st; + if stop { + break; + } + } + last + } } } @@ -1488,7 +1518,7 @@ mod tests { fn count_leaves(p: &Plan) -> usize { match p { Plan::Run { .. } | Plan::System { .. } => 1, - Plan::Seq { steps } => steps.iter().map(count_leaves).sum(), + Plan::Seq { steps } | Plan::Alt { steps } => steps.iter().map(count_leaves).sum(), Plan::Pipe { stages } => stages.iter().map(count_leaves).sum(), Plan::Redirect { body, .. } | Plan::WithStdinFrom { body, .. } @@ -1556,12 +1586,17 @@ mod tests { if depth == 0 || rng.below(3) == 0 { return random_single(rng, depth); } - match rng.below(6) { + match rng.below(7) { 0 => Plan::Seq { steps: (0..1 + rng.below(3)) .map(|_| random_plan(rng, depth - 1)) .collect(), }, + 6 => Plan::Alt { + steps: (0..1 + rng.below(3)) + .map(|_| random_plan(rng, depth - 1)) + .collect(), + }, 1 => Plan::Pipe { stages: (0..2 + rng.below(2)) .map(|_| random_single(rng, depth - 1)) diff --git a/src/mcp.rs b/src/mcp.rs index 50f43b5..60ad264 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -139,6 +139,7 @@ pub struct ExecuteParams { /// {"op":"run","argv":["sysctl","-w","vm.mmap_rnd_bits=28"]}; /// {"op":"pipe","stages":[{"op":"run",...},{"op":"run",...}]}; /// {"op":"seq","steps":[...]} (sequence, stops on first failure); + /// {"op":"alt","steps":[...]} (alternatives, runs until one succeeds, ≈ `||`); /// {"op":"run-as","as":"root","do":{...}} or {"op":"run-as","as":"user","name":"alice","do":{...}}; /// {"op":"redirect","stream":"stdout","target":{"to":"file","path":"/tmp/o","mode":"truncate"},"do":{...}}; /// {"op":"setenv","key":"LANG","value":"C","do":{...}}; {"op":"chdir","dir":"/srv","do":{...}}; @@ -148,7 +149,9 @@ pub struct ExecuteParams { /// effects are committed, so make early steps idempotent (or put the /// riskiest / privileged step first) and do not blindly resubmit a failed /// plan; the response's per-stage exit codes show which steps ran (a skipped - /// step reports exit -1) so you can resume from the failure instead. A + /// step reports exit -1) so you can resume from the failure instead. An + /// `alt` is the dual: it runs its steps in order but STOPS at the first + /// SUCCESS, continuing past failures — use it for `probe || fallback`. A /// privileged stage placed INSIDE a `pipe` cannot prompt for a sudo /// password; if it may need one, make it a standalone `run-as` stage rather /// than a pipe stage (or ensure passwordless sudo on the host). diff --git a/src/protocol.rs b/src/protocol.rs index 6aa896d..529402f 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -119,6 +119,9 @@ pub enum Plan { Run { argv: Vec }, /// Sequence; stop at the first non-accepted exit (Dune `progn`, ≈ `&&`/`;`). Seq { steps: Vec }, + /// Alternatives; run steps until one succeeds, continuing past a non-accepted + /// exit and stopping on the first success (≈ `||`). Dual of `seq`. + Alt { steps: Vec }, /// Connect stdout→stdin left-to-right (Dune `pipe-stdout`, = `|`). Pipe { stages: Vec }, /// Redirect a stream of the inner plan to a file or /dev/null. @@ -208,6 +211,14 @@ impl Plan { s.validate_tree(depth + 1, nodes)?; } } + Plan::Alt { steps } => { + if steps.is_empty() { + return Err("alt must have at least one step".to_string()); + } + for s in steps { + s.validate_tree(depth + 1, nodes)?; + } + } Plan::Pipe { stages } => { if stages.len() < 2 { return Err("pipe must have at least two stages".to_string()); @@ -261,7 +272,9 @@ impl Plan { match self { Plan::Run { .. } | Plan::System { .. } => false, Plan::RunAs { .. } => true, - Plan::Seq { steps } => steps.iter().any(Plan::contains_runas), + Plan::Seq { steps } | Plan::Alt { steps } => { + steps.iter().any(Plan::contains_runas) + } Plan::Pipe { stages } => stages.iter().any(Plan::contains_runas), Plan::Redirect { body, .. } | Plan::WithStdinFrom { body, .. } @@ -279,7 +292,9 @@ impl Plan { Plan::Run { .. } | Plan::System { .. } => false, Plan::RunAs { who: RunAs::Root, .. } => true, Plan::RunAs { who: RunAs::User { .. }, body } => body.contains_root(), - Plan::Seq { steps } => steps.iter().any(Plan::contains_root), + Plan::Seq { steps } | Plan::Alt { steps } => { + steps.iter().any(Plan::contains_root) + } Plan::Pipe { stages } => stages.iter().any(Plan::contains_root), Plan::Redirect { body, .. } | Plan::WithStdinFrom { body, .. } @@ -799,6 +814,21 @@ mod tests { let back: Request = serde_json::from_str(&json).unwrap(); assert_eq!(back.exec_plan(), req.exec_plan()); + // An `alt` plan round-trips with its own `op` tag. + let alt = Request::new_plan( + String::new(), + "s".into(), + Plan::Alt { steps: vec![run(&["false"]), run(&["true"])] }, + HashMap::new(), + String::new(), + true, + false, + ); + let alt_json = serde_json::to_string(&alt).unwrap(); + assert!(alt_json.contains(r#""op":"alt""#), "tagged form missing: {alt_json}"); + let alt_back: Request = serde_json::from_str(&alt_json).unwrap(); + assert_eq!(alt_back.exec_plan(), alt.exec_plan()); + // A pipeline request must not serialize a `plan` key. let flat = exec_request(); let flat_json = serde_json::to_string(&flat).unwrap(); @@ -893,5 +923,12 @@ mod tests { HashMap::new(), String::new(), false, false, ); assert!(ValidatedRequest::validate(short_pipe).is_err()); + + let empty_alt = Request::new_plan( + String::new(), "s".into(), + Plan::Alt { steps: vec![] }, + HashMap::new(), String::new(), false, false, + ); + assert!(ValidatedRequest::validate(empty_alt).is_err()); } } diff --git a/src/tui.rs b/src/tui.rs index de75ad6..7dae632 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -60,6 +60,11 @@ fn plan_to_string(plan: &Plan) -> String { .map(plan_to_string) .collect::>() .join(" ; "), + Plan::Alt { steps } => steps + .iter() + .map(plan_to_string) + .collect::>() + .join(" || "), Plan::Redirect { stream, target, body } => { let op = match (stream, target) { (Stream::Stdout, RedirTarget::File { path, mode }) => { diff --git a/tests/plan_exec.rs b/tests/plan_exec.rs index cdfbb5f..be58318 100644 --- a/tests/plan_exec.rs +++ b/tests/plan_exec.rs @@ -151,6 +151,83 @@ fn accepted_exit_codes_allow_seq_to_continue() { ); } +/// An alt stops at the first succeeding step; later alternatives do not run. +#[test] +fn alt_short_circuits_on_success() { + let plan = Plan::Alt { + steps: vec![ + run(&["true"]), + run(&["sh", "-c", "printf SHOULD_NOT_RUN"]), + ], + }; + let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); + assert_eq!(resp.status, Status::Ok, "got {resp:?}"); + assert_eq!(resp.stages.len(), 2); + assert_eq!(resp.stages[0].exit_code, 0, "true exits 0"); + assert_eq!( + resp.stages[1].exit_code, -1, + "second alternative must not run after a success" + ); + assert!( + !decode_stdout(&resp).contains("SHOULD_NOT_RUN"), + "second alternative must not run after a success" + ); +} + +/// An alt runs the next alternative when a step fails. +#[test] +fn alt_runs_fallback_on_failure() { + let plan = Plan::Alt { + steps: vec![run(&["false"]), run(&["sh", "-c", "printf fallback"])], + }; + let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); + assert_eq!(resp.status, Status::Ok, "got {resp:?}"); + assert_eq!(resp.stages.len(), 2); + assert_eq!(resp.stages[0].exit_code, 1, "false exits 1"); + assert!( + decode_stdout(&resp).contains("fallback"), + "the fallback alternative should run after a failure: {resp:?}" + ); +} + +/// When every alternative fails, all of them run and the last failure stands. +#[test] +fn alt_all_fail_runs_every_step() { + let plan = Plan::Alt { + steps: vec![run(&["false"]), run(&["false"])], + }; + let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); + assert_eq!(resp.status, Status::Ok, "got {resp:?}"); + assert_eq!(resp.stages.len(), 2); + assert_eq!(resp.stages[0].exit_code, 1); + assert_eq!(resp.stages[1].exit_code, 1, "both alternatives run"); +} + +/// `with-accepted-exit-codes` makes a non-zero exit count as success, so the +/// alt stops there and does not run the fallback. +#[test] +fn accepted_exit_code_stops_alt() { + let plan = Plan::Alt { + steps: vec![ + Plan::WithAcceptedExitCodes { + codes: vec![1], + body: Box::new(run(&["false"])), + }, + run(&["sh", "-c", "printf SHOULD_NOT_RUN"]), + ], + }; + let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); + assert_eq!(resp.status, Status::Ok, "got {resp:?}"); + assert_eq!( + resp.stages[1].exit_code, -1, + "an accepted exit counts as success and stops the alt" + ); + assert!( + !decode_stdout(&resp).contains("SHOULD_NOT_RUN"), + "an accepted exit should stop the alt: {resp:?}" + ); +} + /// The explicit `System` escape hatch runs a shell script verbatim. #[test] fn system_escape_hatch_runs() { diff --git a/tests/validation.rs b/tests/validation.rs index 7918af3..4ca84e8 100644 --- a/tests/validation.rs +++ b/tests/validation.rs @@ -318,6 +318,27 @@ fn plan_structural_error_rejected_without_prompt() { assert_eq!(s.prompter.call_count(), 0, "must not prompt for an invalid plan"); } +/// A redirect wrapping an `alt` is structurally invalid (stdio modifiers target +/// a single process, not a composite) and is rejected before prompting. +#[test] +fn plan_redirect_of_alt_rejected_without_prompt() { + let s = server(); + let plan = Plan::Redirect { + stream: sudo_proxy::protocol::Stream::Stdout, + target: sudo_proxy::protocol::RedirTarget::DevNull, + body: Box::new(Plan::Alt { + steps: vec![ + Plan::Run { argv: vec!["false".into()] }, + Plan::Run { argv: vec!["true".into()] }, + ], + }), + }; + let resp = s.send(&plan_req("v-alt-struct", plan, false)); + assert_eq!(resp.status, Status::Error); + assert!(resp.message.as_deref().unwrap().contains("invalid plan")); + assert_eq!(s.prompter.call_count(), 0, "must not prompt for an invalid plan"); +} + /// A valid unprivileged plan runs end-to-end and returns Ok. #[test] fn plan_valid_runs() { From 0cb0139b4da0c2f66a19ccc756a12377c56a3842 Mon Sep 17 00:00:00 2001 From: Cuihtlauac ALVARADO Date: Sat, 17 Oct 2026 10:07:00 +0200 Subject: [PATCH 03/14] chore: git-ignore CLAUDE.local.md The personal, machine-specific project instructions are not meant to be checked in. Co-Authored-By: Claude Opus 4.8 (1M context) --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 5358193..994b362 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ /target /scratch +/CLAUDE.local.md From 711e9fb2dbebb3d53c2b70c1763c3519a11cd1de Mon Sep 17 00:00:00 2001 From: Cuihtlauac ALVARADO Date: Mon, 19 Oct 2026 08:14:51 +0200 Subject: [PATCH 04/14] fix(plan): flatten run-as `who` so the documented `as` wire form parses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MCP `execute` plan docs advertise `{"op":"run-as","as":"root","do":{…}}` (and `…,"as":"user","name":"…",…`), but `Plan::RunAs` held `who: RunAs` without `#[serde(flatten)]`, so the deserializer required a nested `"who":{…}` object. Any run-as plan written exactly as documented failed with `missing field 'who'` — which breaks the headline feature that replaces the `sudo -H -u` blobs. Flatten the `who` field so the ergonomic, documented `as` form both parses and serializes (no nested `who`). The plan model is unreleased, so there is no deployed client depending on the old nested shape. Docs were already correct; only the code needed to match them. Adds `run_as_wire_form_is_flattened_and_matches_docs` as a regression guard (serializes to `as`/no `who`, round-trips, and the exact documented JSON for both root and named-user forms deserializes). Found via live end-to-end testing of the plan surface (finding F1). Co-Authored-By: Claude Opus 4.8 (1M context) --- src/protocol.rs | 45 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/protocol.rs b/src/protocol.rs index 529402f..10ceaac 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -152,8 +152,12 @@ pub enum Plan { body: Box, }, /// Run the inner plan under a specific privilege context — `[ROOT]` or - /// `[as ]`. The axis Dune lacks; see `RunAs`. + /// `[as ]`. The axis Dune lacks; see `RunAs`. `who` is flattened, so + /// the wire form is `{"op":"run-as","as":"root","do":{…}}` / + /// `{"op":"run-as","as":"user","name":"…","do":{…}}` (matching the MCP docs), + /// not a nested `"who":{…}` object. RunAs { + #[serde(flatten)] who: RunAs, #[serde(rename = "do")] body: Box, @@ -835,6 +839,45 @@ mod tests { assert!(!flat_json.contains("plan"), "flat request leaked plan key: {flat_json}"); } + /// `run-as` uses the flattened wire form documented by the MCP tool — + /// `{"op":"run-as","as":"root","do":{…}}` — not a nested `"who":{…}` object. + /// Both root and named-user forms round-trip, and the exact JSON the MCP doc + /// advertises deserializes. (Regression guard for finding F1.) + #[test] + fn run_as_wire_form_is_flattened_and_matches_docs() { + let root = Request::new_plan( + String::new(), + "s".into(), + Plan::RunAs { who: RunAs::Root, body: Box::new(run(&["id", "-un"])) }, + HashMap::new(), + String::new(), + true, + false, + ); + let json = serde_json::to_string(&root).unwrap(); + assert!(json.contains(r#""op":"run-as""#), "op tag missing: {json}"); + assert!(json.contains(r#""as":"root""#), "flattened `as` missing: {json}"); + assert!(!json.contains(r#""who""#), "`who` should be flattened away: {json}"); + assert_eq!( + serde_json::from_str::(&json).unwrap().exec_plan(), + root.exec_plan() + ); + + // The exact JSON shapes the MCP `plan` doc advertises must parse. + let doc_root = r#"{"op":"run-as","as":"root","do":{"op":"run","argv":["id"]}}"#; + let doc_user = r#"{"op":"run-as","as":"user","name":"alice","do":{"op":"run","argv":["id"]}}"#; + let p_root: Plan = serde_json::from_str(doc_root).unwrap(); + let p_user: Plan = serde_json::from_str(doc_user).unwrap(); + assert_eq!(p_root, Plan::RunAs { who: RunAs::Root, body: Box::new(run(&["id"])) }); + assert_eq!( + p_user, + Plan::RunAs { + who: RunAs::User { name: "alice".into() }, + body: Box::new(run(&["id"])), + } + ); + } + /// `effective_privileged` is true for a top-level privileged flag, and also /// for an unprivileged request whose plan smuggles any `RunAs` node. #[test] From 0ad3afdc16078f55d4355298e262af60c5a78513 Mon Sep 17 00:00:00 2001 From: Cuihtlauac ALVARADO Date: Mon, 19 Oct 2026 08:15:19 +0200 Subject: [PATCH 05/14] feat(sudo-request): --plan/--plan-file to drive structured plans like the MCP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The debug client could only build flat pipelines, so the full plan surface (seq/alt/pipe/redirect/run-as/setenv/chdir/system) was unreachable from the CLI and could only be exercised via the MCP tool or hand-written socket JSON. Add `--plan JSON` and `--plan-file PATH` ('-' = stdin). When given, the client assembles the request exactly as the MCP `execute` tool does (parse JSON into a `Plan`, call `Request::new_plan`); `Request::new_plan` stamps a fresh timestamp, so no socat or hand-rolled RFC3339 is needed. Plan and command/pipeline are mutually exclusive; invalid JSON is caught client-side before anything is sent. Note this reproduces the MCP's request *assembly*, not its policy layer — it does not replicate the "refuse unprivileged local commands" rule, which is what lets you drive unprivileged local plans for testing. Docs: new docs/testing.md documents the two-terminal live-testing workflow (branch daemon + sudo-request --plan, approve in your own terminal); docs/usage.md gains --plan/--plan-file examples and a link to it. Adds unit tests for plan assembly (valid plan builds + validates, bad JSON rejected, no-plan still sends a pipeline). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/testing.md | 99 +++++++++++++++++++++++++ docs/usage.md | 11 +++ src/bin/sudo-request.rs | 155 ++++++++++++++++++++++++++++++++++++---- 3 files changed, 253 insertions(+), 12 deletions(-) create mode 100644 docs/testing.md diff --git a/docs/testing.md b/docs/testing.md new file mode 100644 index 0000000..fc221df --- /dev/null +++ b/docs/testing.md @@ -0,0 +1,99 @@ +# Manual testing: drive the daemon live + +[← README](../README.md) · see also [usage.md](usage.md), [protocol.md](protocol.md) + +The unit and integration suites (`cargo test`) cover logic and drive the daemon +through a *scripted* approval. This doc is the complement: how to exercise a new +feature **end-to-end through the real daemon and the real human gate** — the +approval prompt appears in your terminal and you approve it yourself — before +merging. It is the fastest way to see a change behave exactly as an operator (or +the MCP `execute` tool) will experience it. + +The key enabler is `sudo-request --plan`, which assembles a structured `Plan` +request the same way the MCP tool does (`mcp::build_request`), so you can drive +the full plan surface (`seq`, `alt`, `pipe`, `redirect`, `run-as`, `setenv`, +`chdir`, `system`) from the command line — no MCP client and no hand-written +socket JSON required. + +## Setup + +Build the branch under test, then run the daemon in one terminal and send +requests from another. + +**Terminal A — the daemon + approval TUI.** This is where the prompt appears and +where you press the keys. Use a dedicated test socket so you never collide with +an installed daemon's default socket: + +```bash +cargo build --release +./target/release/sudo-proxy -v --socket /tmp/sp-test.sock +``` + +**Terminal B — send requests** with the freshly built client: + +```bash +# A single command / pipeline (flat form) +./target/release/sudo-request --socket /tmp/sp-test.sock --no-privilege --print \ + echo hi '|' wc -c + +# A structured plan — the same assembly the MCP execute tool performs +./target/release/sudo-request --socket /tmp/sp-test.sock --no-privilege --print \ + --reason "try alt" \ + --plan '{"op":"alt","steps":[{"op":"run","argv":["false"]},{"op":"run","argv":["echo","fallback-ran"]}]}' +``` + +When the request arrives, Terminal A shows the per-stage prompt — each stage with +its own resolved real binary and run-as tag — and waits for a key. Press **`y`** +to approve; the client in Terminal B then prints the result. + +For a large plan, keep it in a file and use `--plan-file` (or `--plan-file -` to +read from stdin): + +```bash +./target/release/sudo-request --socket /tmp/sp-test.sock --no-privilege --print \ + --plan-file ./scratch/plan.json +``` + +## Reading the result + +With `--print`, the client shows the structured response: + +``` +exit [1, 0] +== stdout == (1 lines) +fallback-ran +``` + +- The `exit [...]` array is one entry **per stage, in prompt order**. +- A stage that was **not run** (short-circuited) reports the `-1` sentinel. For + the `alt` above with a succeeding first step you would instead see + `exit [0, -1]` — the first alternative succeeded, so the second never ran. +- `stdout` is the aggregate of every captured stage. + +## Tips + +- **Decline** with `n` or Enter (default is deny) → the client prints + `Request denied.` and exits non-zero. Good for testing the deny path. +- **Mimic the MCP session label** with `--session sudo-proxy-mcp` if a feature + keys off the session name. +- **Exercise privileged stages** by dropping `--no-privilege` (you will satisfy a + real `sudo`/`pkexec` prompt) or by using a `run-as` node inside the plan. +- **Fidelity caveat.** `sudo-request` reproduces the MCP's *request assembly*, not + its policy layer: the MCP additionally refuses unprivileged *local* commands + (invariant G7 — "use the Bash tool"). That refusal lives in the MCP execute + handler, not in the daemon, which is exactly what lets you drive unprivileged + local plans here for testing. + +## Without a human (CI / headless) + +The human gate reads `/dev/tty`, so full execution needs either a person or a +pseudo-terminal harness. Two headless-friendly paths: + +- **Pre-prompt rejection needs no keypress.** A structurally invalid plan (e.g. a + `redirect` wrapping a `seq`/`alt`) is rejected *before* prompting, so a client + call returns an `invalid plan: …` error with no interaction — handy for + smoke-testing the client→daemon plumbing against a backgrounded daemon. +- **Scripted approval** for real execution: the integration tests drive the + daemon through a scripted prompter — see `tests/plan_exec.rs` + (`plan_request_end_to_end_through_daemon`) and `tests/validation.rs`. Prefer + adding a case there over automating the live TUI. diff --git a/docs/usage.md b/docs/usage.md index 2353ebd..5aae99d 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -63,8 +63,19 @@ sudo-request --no-privilege ls /etc # Tag the request with a session name (default: sudo-request-cli) sudo-request --session my-project apt update + +# Send a structured plan (assembled like the MCP execute tool). Drives the full +# plan surface — seq / alt / pipe / redirect / run-as / setenv / chdir / system. +sudo-request --no-privilege \ + --plan '{"op":"alt","steps":[{"op":"run","argv":["false"]},{"op":"run","argv":["echo","none"]}]}' + +# ...or read the plan JSON from a file ('-' for stdin) +sudo-request --no-privilege --plan-file ./scratch/plan.json ``` +To drive a new feature end-to-end through the real daemon and human gate before +merging, see [testing.md](testing.md). + ## Remote hosts over SSH `sudo-proxy --host HOST` resolves the remote UID (cached in diff --git a/src/bin/sudo-request.rs b/src/bin/sudo-request.rs index c060ea5..45465e1 100644 --- a/src/bin/sudo-request.rs +++ b/src/bin/sudo-request.rs @@ -4,9 +4,11 @@ use std::path::PathBuf; use std::process; use std::time::Duration; +use std::collections::HashMap; + use base64::engine::general_purpose::STANDARD as B64; use base64::Engine; -use sudo_proxy::protocol::{Request, Response, Status}; +use sudo_proxy::protocol::{Plan, Request, Response, Status}; use sudo_proxy::server::default_socket_path; /// Default read timeout for waiting on the daemon's response. Must exceed @@ -32,23 +34,23 @@ fn main() { } }; - if opts.pipeline.is_empty() || opts.pipeline.iter().all(|s| s.is_empty()) { + if opts.plan.is_none() + && (opts.pipeline.is_empty() || opts.pipeline.iter().all(|s| s.is_empty())) + { eprintln!("error: no command specified"); eprintln!("Usage: sudo-request [OPTIONS] COMMAND [ARGS...] ['|' COMMAND [ARGS...] ...]"); process::exit(1); } - let socket_path = opts.socket.unwrap_or_else(default_socket_path); + let socket_path = opts.socket.clone().unwrap_or_else(default_socket_path); - let req = Request::new( - hostname(), - opts.session, - opts.pipeline, - std::collections::HashMap::new(), - opts.reason.unwrap_or_default(), - opts.privileged, - opts.forward_agent, - ); + let req = match build_request(&opts) { + Ok(r) => r, + Err(e) => { + eprintln!("error: {e}"); + process::exit(1); + } + }; // Connect and send let mut stream = match UnixStream::connect(&socket_path) { @@ -148,6 +150,38 @@ fn main() { } } +/// Assemble the outgoing `Request` from parsed options. A `--plan` / `--plan-file` +/// JSON builds a structured plan request exactly as the MCP `execute` tool does +/// (`mcp::build_request`): parse the JSON into a `Plan` and call `new_plan`. +/// Otherwise the legacy flat `pipeline` is sent. The daemon re-validates either +/// way; this is only client-side assembly. +fn build_request(opts: &Opts) -> Result { + let reason = opts.reason.clone().unwrap_or_default(); + if let Some(plan_json) = &opts.plan { + let plan: Plan = + serde_json::from_str(plan_json).map_err(|e| format!("invalid plan JSON: {e}"))?; + Ok(Request::new_plan( + hostname(), + opts.session.clone(), + plan, + HashMap::new(), + reason, + opts.privileged, + opts.forward_agent, + )) + } else { + Ok(Request::new( + hostname(), + opts.session.clone(), + opts.pipeline.clone(), + HashMap::new(), + reason, + opts.privileged, + opts.forward_agent, + )) + } +} + struct Opts { socket: Option, reason: Option, @@ -156,6 +190,9 @@ struct Opts { privileged: bool, forward_agent: bool, pipeline: Vec>, + /// Raw plan JSON from `--plan` or `--plan-file`; mutually exclusive with a + /// trailing command / pipeline. + plan: Option, } fn parse_args() -> Result { @@ -166,6 +203,8 @@ fn parse_args() -> Result { let mut print = false; let mut privileged = true; let mut forward_agent = false; + let mut plan: Option = None; + let mut plan_file: Option = None; let mut argv = Vec::new(); let mut i = 0; @@ -184,9 +223,16 @@ fn parse_args() -> Result { eprintln!(" --print Print all output to stdout (exit code, stdout, stderr)"); eprintln!(" --no-privilege Run command without privilege escalation"); eprintln!(" --forward-agent Forward the local SSH agent to the command (unprivileged only)"); + eprintln!(" --plan JSON Send a structured plan request (as the MCP does). The"); + eprintln!(" JSON is an op-tagged Plan, e.g."); + eprintln!(r#" '{{"op":"alt","steps":[{{"op":"run","argv":["false"]}},{{"op":"run","argv":["echo","fallback"]}}]}}'"#); + eprintln!(" --plan-file PATH Read the plan JSON from a file ('-' for stdin)"); eprintln!(); eprintln!("Pipeline example:"); eprintln!(" sudo-request --no-privilege ls /tmp '|' wc -l"); + eprintln!(); + eprintln!("Plan example (drives the full structured surface like the MCP):"); + eprintln!(r#" sudo-request --no-privilege --plan '{{"op":"run","argv":["id"]}}'"#); std::process::exit(0); } "--socket" => { @@ -219,6 +265,16 @@ fn parse_args() -> Result { "--forward-agent" => { forward_agent = true; } + "--plan" => { + i += 1; + plan = Some(args.get(i).ok_or("--plan requires a JSON value")?.clone()); + } + "--plan-file" => { + i += 1; + plan_file = Some(PathBuf::from( + args.get(i).ok_or("--plan-file requires a path")?, + )); + } other if other.starts_with("--") => { return Err(format!("unknown option: {other}")); } @@ -241,6 +297,20 @@ fn parse_args() -> Result { ); } + // Resolve the plan source: --plan (inline) or --plan-file (path, '-' = stdin). + let plan = match (plan, plan_file) { + (Some(_), Some(_)) => return Err("--plan and --plan-file are mutually exclusive".to_string()), + (Some(json), None) => Some(json), + (None, Some(path)) => Some(read_plan_file(&path)?), + (None, None) => None, + }; + + if plan.is_some() && !pipeline.is_empty() { + return Err( + "provide either a plan (--plan/--plan-file) or a command/pipeline, not both".to_string(), + ); + } + Ok(Opts { socket, reason, @@ -249,9 +319,24 @@ fn parse_args() -> Result { privileged, forward_agent, pipeline, + plan, }) } +/// Read plan JSON from a file, or from stdin when the path is `-`. +fn read_plan_file(path: &std::path::Path) -> Result { + if path.as_os_str() == "-" { + let mut buf = String::new(); + std::io::stdin() + .read_to_string(&mut buf) + .map_err(|e| format!("could not read plan from stdin: {e}"))?; + Ok(buf) + } else { + std::fs::read_to_string(path) + .map_err(|e| format!("could not read plan file {}: {e}", path.display())) + } +} + /// Split a flat argv on literal `|` tokens into pipeline stages. fn split_pipeline(argv: Vec) -> Vec> { if argv.is_empty() { @@ -281,3 +366,49 @@ fn hostname() -> String { .unwrap_or_else(|_| "unknown".to_string()) } +#[cfg(test)] +mod tests { + use super::*; + use sudo_proxy::protocol::ValidatedRequest; + + fn opts_with_plan(json: Option<&str>) -> Opts { + Opts { + socket: None, + reason: None, + session: "sudo-request-cli".to_string(), + print: false, + privileged: false, + forward_agent: false, + pipeline: vec![], + plan: json.map(|s| s.to_string()), + } + } + + /// A `--plan` JSON is assembled into a plan-bearing request that the daemon's + /// validator accepts — the same path the MCP `execute` tool takes. + #[test] + fn plan_json_builds_a_valid_plan_request() { + let json = r#"{"op":"alt","steps":[{"op":"run","argv":["false"]},{"op":"run","argv":["true"]}]}"#; + let req = build_request(&opts_with_plan(Some(json))).expect("build"); + assert!(req.plan.is_some(), "request must carry a plan"); + assert!(req.pipeline.is_empty(), "plan request must not carry a pipeline"); + ValidatedRequest::validate(req).expect("daemon-side validation should accept it"); + } + + /// Malformed plan JSON is rejected at assembly, before anything is sent. + #[test] + fn invalid_plan_json_is_rejected() { + assert!(build_request(&opts_with_plan(Some("{not json"))).is_err()); + } + + /// With no plan, the legacy flat pipeline is sent unchanged. + #[test] + fn no_plan_sends_flat_pipeline() { + let mut opts = opts_with_plan(None); + opts.pipeline = vec![vec!["id".to_string()]]; + let req = build_request(&opts).expect("build"); + assert!(req.plan.is_none()); + assert_eq!(req.pipeline, vec![vec!["id".to_string()]]); + } +} + From 813ca7bc99f6875b95e077fc67f373c03627da59 Mon Sep 17 00:00:00 2001 From: Cuihtlauac ALVARADO Date: Mon, 19 Oct 2026 08:58:50 +0200 Subject: [PATCH 06/14] refactor(plan)!: rename variants + add a `;` (Seq) op MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clarify the plan vocabulary and fill the sequencing gap surfaced during live testing. The old `Seq` had `&&` semantics (stop on first failure) yet its name implied `;` (run-all), which did not exist at all. Renames (wire `op` tags change; all unreleased, no deployed client affected): - `Run { argv }` -> `Exec { argv }` (op `exec`) — one command - `Seq { steps }` (&&) -> `Run { steps }` (op `run`) — stop on first failure - `System {..}` -> `Shell {..}` (op `shell`) — opaque escape hatch - enum `Shell` -> `ShellProc` — the sh/bash interpreter New op: - `Seq { steps }` (op `seq`) — `;`: run every step regardless of exit; only a timeout or spawn failure aborts; status is the last step's. The dual of `Run`. So the three chainers now map cleanly to shell: `run` = `&&`, `seq` = `;`, `alt` = `||`. The approval prompt's one-line render uses those operators (` && ` / ` ; ` / ` || ` / ` | `). Updates every match site (protocol, executor incl. SeqNode + run_seq + property-test generators, tui, mcp doc, sudo-request help/tests, Kani proof), renames the `run()` test helper to `exec()` and the misleading `*_seq_*` test names, adds `seq_runs_every_step_despite_failure`, and fixes op names in docs/usage.md and docs/testing.md. clippy -D warnings clean; lib 64 / plan_exec 13 / validation 22 green. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/testing.md | 6 +-- docs/usage.md | 5 ++- src/bin/sudo-request.rs | 6 +-- src/executor.rs | 79 ++++++++++++++++++++++++++-------- src/mcp.rs | 20 +++++---- src/proofs.rs | 4 +- src/protocol.rs | 95 ++++++++++++++++++++--------------------- src/tui.rs | 27 +++++++----- tests/plan_exec.rs | 72 +++++++++++++++++++------------ tests/validation.rs | 18 ++++---- 10 files changed, 199 insertions(+), 133 deletions(-) diff --git a/docs/testing.md b/docs/testing.md index fc221df..36ae290 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -11,8 +11,8 @@ the MCP `execute` tool) will experience it. The key enabler is `sudo-request --plan`, which assembles a structured `Plan` request the same way the MCP tool does (`mcp::build_request`), so you can drive -the full plan surface (`seq`, `alt`, `pipe`, `redirect`, `run-as`, `setenv`, -`chdir`, `system`) from the command line — no MCP client and no hand-written +the full plan surface (`exec`, `run` (`&&`), `seq` (`;`), `alt` (`||`), `pipe`, +`redirect`, `run-as`, `setenv`, `chdir`, `shell`) from the command line — no MCP client and no hand-written socket JSON required. ## Setup @@ -39,7 +39,7 @@ cargo build --release # A structured plan — the same assembly the MCP execute tool performs ./target/release/sudo-request --socket /tmp/sp-test.sock --no-privilege --print \ --reason "try alt" \ - --plan '{"op":"alt","steps":[{"op":"run","argv":["false"]},{"op":"run","argv":["echo","fallback-ran"]}]}' + --plan '{"op":"alt","steps":[{"op":"exec","argv":["false"]},{"op":"exec","argv":["echo","fallback-ran"]}]}' ``` When the request arrives, Terminal A shows the per-stage prompt — each stage with diff --git a/docs/usage.md b/docs/usage.md index 5aae99d..ec45e04 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -65,9 +65,10 @@ sudo-request --no-privilege ls /etc sudo-request --session my-project apt update # Send a structured plan (assembled like the MCP execute tool). Drives the full -# plan surface — seq / alt / pipe / redirect / run-as / setenv / chdir / system. +# plan surface — exec / run(&&) / seq(;) / alt(||) / pipe / redirect / run-as / +# setenv / chdir / shell. sudo-request --no-privilege \ - --plan '{"op":"alt","steps":[{"op":"run","argv":["false"]},{"op":"run","argv":["echo","none"]}]}' + --plan '{"op":"alt","steps":[{"op":"exec","argv":["false"]},{"op":"exec","argv":["echo","none"]}]}' # ...or read the plan JSON from a file ('-' for stdin) sudo-request --no-privilege --plan-file ./scratch/plan.json diff --git a/src/bin/sudo-request.rs b/src/bin/sudo-request.rs index 45465e1..10a2db8 100644 --- a/src/bin/sudo-request.rs +++ b/src/bin/sudo-request.rs @@ -225,14 +225,14 @@ fn parse_args() -> Result { eprintln!(" --forward-agent Forward the local SSH agent to the command (unprivileged only)"); eprintln!(" --plan JSON Send a structured plan request (as the MCP does). The"); eprintln!(" JSON is an op-tagged Plan, e.g."); - eprintln!(r#" '{{"op":"alt","steps":[{{"op":"run","argv":["false"]}},{{"op":"run","argv":["echo","fallback"]}}]}}'"#); + eprintln!(r#" '{{"op":"alt","steps":[{{"op":"exec","argv":["false"]}},{{"op":"exec","argv":["echo","fallback"]}}]}}'"#); eprintln!(" --plan-file PATH Read the plan JSON from a file ('-' for stdin)"); eprintln!(); eprintln!("Pipeline example:"); eprintln!(" sudo-request --no-privilege ls /tmp '|' wc -l"); eprintln!(); eprintln!("Plan example (drives the full structured surface like the MCP):"); - eprintln!(r#" sudo-request --no-privilege --plan '{{"op":"run","argv":["id"]}}'"#); + eprintln!(r#" sudo-request --no-privilege --plan '{{"op":"exec","argv":["id"]}}'"#); std::process::exit(0); } "--socket" => { @@ -388,7 +388,7 @@ mod tests { /// validator accepts — the same path the MCP `execute` tool takes. #[test] fn plan_json_builds_a_valid_plan_request() { - let json = r#"{"op":"alt","steps":[{"op":"run","argv":["false"]},{"op":"run","argv":["true"]}]}"#; + let json = r#"{"op":"alt","steps":[{"op":"exec","argv":["false"]},{"op":"exec","argv":["true"]}]}"#; let req = build_request(&opts_with_plan(Some(json))).expect("build"); assert!(req.plan.is_some(), "request must carry a plan"); assert!(req.pipeline.is_empty(), "plan request must not carry a pipeline"); diff --git a/src/executor.rs b/src/executor.rs index 2bc3f36..6952a25 100644 --- a/src/executor.rs +++ b/src/executor.rs @@ -12,7 +12,7 @@ use base64::engine::general_purpose::STANDARD as B64; use base64::Engine; use crate::protocol::{ - Plan, RedirTarget, Response, RunAs, Shell, StageResult, Stream, ValidatedRequest, WriteMode, + Plan, RedirTarget, Response, RunAs, ShellProc, StageResult, Stream, ValidatedRequest, WriteMode, }; /// Hard cap on captured stdout/stderr per stream. A privileged @@ -262,7 +262,11 @@ struct LeafPlan { enum SeqNode { Leaf(usize), Pipe(Vec), + /// `&&` — run in order, stop at first failure. + Run(Vec), + /// `;` — run all in order regardless of exit status. Seq(Vec), + /// `||` — run in order, stop at first success. Alt(Vec), } @@ -363,6 +367,16 @@ fn compile_seq(plan: &Plan, mut ctx: Ctx, leaves: &mut Vec) -> Result< ctx.accepted = codes.clone(); compile_seq(body, ctx, leaves) } + Plan::Run { steps } => { + if !ctx.stdio_is_default() { + return Err("redirect / stdin-from of a run is not supported".to_string()); + } + let mut nodes = Vec::with_capacity(steps.len()); + for s in steps { + nodes.push(compile_seq(s, ctx.clone(), leaves)?); + } + Ok(SeqNode::Run(nodes)) + } Plan::Seq { steps } => { if !ctx.stdio_is_default() { return Err("redirect / stdin-from of a seq is not supported".to_string()); @@ -399,11 +413,11 @@ fn compile_seq(plan: &Plan, mut ctx: Ctx, leaves: &mut Vec) -> Result< /// modifiers and errors on a composite. fn compile_leaf(plan: &Plan, mut ctx: Ctx, leaves: &mut Vec) -> Result { match plan { - Plan::Run { argv } => Ok(push_leaf(argv.clone(), false, ctx, leaves)), - Plan::System { shell, script } => { + Plan::Exec { argv } => Ok(push_leaf(argv.clone(), false, ctx, leaves)), + Plan::Shell { shell, script } => { let bin = match shell { - Shell::Sh => "sh", - Shell::Bash => "bash", + ShellProc::Sh => "sh", + ShellProc::Bash => "bash", }; let argv = vec![bin.to_string(), "-c".to_string(), script.clone()]; Ok(push_leaf(argv, true, ctx, leaves)) @@ -432,7 +446,7 @@ fn compile_leaf(plan: &Plan, mut ctx: Ctx, leaves: &mut Vec) -> Result ctx.stdin = InSpec::File(path.clone()); compile_leaf(body, ctx, leaves) } - Plan::Seq { .. } | Plan::Alt { .. } | Plan::Pipe { .. } => { + Plan::Run { .. } | Plan::Seq { .. } | Plan::Alt { .. } | Plan::Pipe { .. } => { Err("expected a single command here, found a composite plan".to_string()) } } @@ -579,7 +593,7 @@ fn mark_pipe_members(node: &SeqNode, in_pipe: &mut [bool]) { in_pipe[i] = true; } } - SeqNode::Seq(nodes) | SeqNode::Alt(nodes) => { + SeqNode::Run(nodes) | SeqNode::Seq(nodes) | SeqNode::Alt(nodes) => { for n in nodes { mark_pipe_members(n, in_pipe); } @@ -734,7 +748,9 @@ fn run_seq( st } SeqNode::Pipe(indices) => run_pipe(indices, leaves, mode, deadline, outcomes), - SeqNode::Seq(nodes) => { + SeqNode::Run(nodes) => { + // `&&` — run in order, stop at the first non-accepted failure (or a + // timeout / spawn error). Status is the last step that ran. let mut last = StepStatus { timed_out: false, spawn_error: None, @@ -750,10 +766,30 @@ fn run_seq( } last } + SeqNode::Seq(nodes) => { + // `;` — run every step regardless of exit status. Only a timeout or + // spawn failure (infrastructure errors the daemon treats as fatal + // everywhere) aborts; a non-accepted exit does not. Status is the + // last step that ran. + let mut last = StepStatus { + timed_out: false, + spawn_error: None, + success: true, + }; + for n in nodes { + let st = run_seq(n, leaves, mode, deadline, tty_lock, outcomes); + let stop = st.timed_out || st.spawn_error.is_some(); + last = st; + if stop { + break; + } + } + last + } SeqNode::Alt(nodes) => { - // Dual of `Seq`: run steps until one succeeds. Keep going past a + // Dual of `Run`: run steps until one succeeds. Keep going past a // non-accepted failure; stop on the first success (and on a timeout - // or spawn error, which abort the whole alt just as they do a seq). + // or spawn error, which abort the whole alt just as they do a run). let mut last = StepStatus { timed_out: false, spawn_error: None, @@ -1498,7 +1534,7 @@ mod tests { // === Structured-plan properties (Rung 2) ============================ use crate::prop::Rng; - use crate::protocol::{Plan, Request, RunAs, Shell, ValidatedRequest}; + use crate::protocol::{Plan, Request, RunAs, ShellProc, ValidatedRequest}; fn mk_plan_req(plan: Plan, privileged: bool) -> ValidatedRequest { let req = Request::new_plan( @@ -1517,8 +1553,10 @@ mod tests { /// executor and the prompt both enumerate). fn count_leaves(p: &Plan) -> usize { match p { - Plan::Run { .. } | Plan::System { .. } => 1, - Plan::Seq { steps } | Plan::Alt { steps } => steps.iter().map(count_leaves).sum(), + Plan::Exec { .. } | Plan::Shell { .. } => 1, + Plan::Run { steps } | Plan::Seq { steps } | Plan::Alt { steps } => { + steps.iter().map(count_leaves).sum() + } Plan::Pipe { stages } => stages.iter().map(count_leaves).sum(), Plan::Redirect { body, .. } | Plan::WithStdinFrom { body, .. } @@ -1545,13 +1583,13 @@ mod tests { fn random_single(rng: &mut Rng, depth: usize) -> Plan { if depth == 0 || rng.below(2) == 0 { if rng.below(6) == 0 { - return Plan::System { - shell: if rng.below(2) == 0 { Shell::Sh } else { Shell::Bash }, + return Plan::Shell { + shell: if rng.below(2) == 0 { ShellProc::Sh } else { ShellProc::Bash }, script: word(rng), }; } let n = 1 + rng.below(3); - return Plan::Run { + return Plan::Exec { argv: (0..n).map(|_| word(rng)).collect(), }; } @@ -1586,8 +1624,8 @@ mod tests { if depth == 0 || rng.below(3) == 0 { return random_single(rng, depth); } - match rng.below(7) { - 0 => Plan::Seq { + match rng.below(8) { + 0 => Plan::Run { steps: (0..1 + rng.below(3)) .map(|_| random_plan(rng, depth - 1)) .collect(), @@ -1597,6 +1635,11 @@ mod tests { .map(|_| random_plan(rng, depth - 1)) .collect(), }, + 7 => Plan::Seq { + steps: (0..1 + rng.below(3)) + .map(|_| random_plan(rng, depth - 1)) + .collect(), + }, 1 => Plan::Pipe { stages: (0..2 + rng.below(2)) .map(|_| random_single(rng, depth - 1)) diff --git a/src/mcp.rs b/src/mcp.rs index 60ad264..b677610 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -136,21 +136,23 @@ pub struct ExecuteParams { /// path — and renders stage by stage at the approval prompt, so every real /// binary and per-stage privilege is visible. Prefer this over wrapping /// commands in `bash -lc`. Nodes are `op`-tagged, e.g. - /// {"op":"run","argv":["sysctl","-w","vm.mmap_rnd_bits=28"]}; - /// {"op":"pipe","stages":[{"op":"run",...},{"op":"run",...}]}; - /// {"op":"seq","steps":[...]} (sequence, stops on first failure); - /// {"op":"alt","steps":[...]} (alternatives, runs until one succeeds, ≈ `||`); + /// {"op":"exec","argv":["sysctl","-w","vm.mmap_rnd_bits=28"]} (one command); + /// {"op":"pipe","stages":[{"op":"exec",...},{"op":"exec",...}]} (= `|`); + /// {"op":"run","steps":[...]} (≈ `&&`: run in order, stop on first failure); + /// {"op":"seq","steps":[...]} (≈ `;`: run every step regardless of exit); + /// {"op":"alt","steps":[...]} (≈ `||`: run until one succeeds); /// {"op":"run-as","as":"root","do":{...}} or {"op":"run-as","as":"user","name":"alice","do":{...}}; /// {"op":"redirect","stream":"stdout","target":{"to":"file","path":"/tmp/o","mode":"truncate"},"do":{...}}; /// {"op":"setenv","key":"LANG","value":"C","do":{...}}; {"op":"chdir","dir":"/srv","do":{...}}; - /// {"op":"system","shell":"bash","script":"..."} (explicit opaque escape hatch). - /// Semantics to plan around: a `seq` runs its steps in order and STOPS at - /// the first failing step — the earlier steps have already run and their + /// {"op":"shell","shell":"bash","script":"..."} (explicit opaque escape hatch). + /// Semantics to plan around: a `run` (`&&`) runs its steps in order and STOPS + /// at the first failing step — the earlier steps have already run and their /// effects are committed, so make early steps idempotent (or put the /// riskiest / privileged step first) and do not blindly resubmit a failed /// plan; the response's per-stage exit codes show which steps ran (a skipped - /// step reports exit -1) so you can resume from the failure instead. An - /// `alt` is the dual: it runs its steps in order but STOPS at the first + /// step reports exit -1) so you can resume from the failure instead. A `seq` + /// (`;`) instead runs every step regardless of exit (a timeout / spawn + /// failure still aborts). An `alt` (`||`) runs its steps until the first /// SUCCESS, continuing past failures — use it for `probe || fallback`. A /// privileged stage placed INSIDE a `pipe` cannot prompt for a sudo /// password; if it may need one, make it a standalone `run-as` stage rather diff --git a/src/proofs.rs b/src/proofs.rs index 1dde1f3..ea0ffab 100644 --- a/src/proofs.rs +++ b/src/proofs.rs @@ -141,7 +141,7 @@ fn has_dangerous_chars_matches_spec_per_char() { } /// Proof: the structured-plan leaf check is panic-free and agrees with the -/// per-char danger spec. A `Plan::Run` whose single argv element is one symbolic +/// per-char danger spec. A `Plan::Exec` whose single argv element is one symbolic /// character validates iff that character is not dangerous — i.e. the recursive /// `Plan::validate_tree` applies exactly the same `has_dangerous_chars` gate to a /// plan leaf that the flat-pipeline path applies to an argv element. This is the @@ -163,7 +163,7 @@ fn plan_run_leaf_matches_dangerous_char_spec() { let mut buf = [0u8; 4]; let s: &str = c.encode_utf8(&mut buf); - let plan = crate::protocol::Plan::Run { + let plan = crate::protocol::Plan::Exec { argv: vec![s.to_string()], }; let mut nodes = 0usize; diff --git a/src/protocol.rs b/src/protocol.rs index 10ceaac..9457ef7 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -89,13 +89,13 @@ pub enum RunAs { User { name: String }, } -/// Which shell an explicit `System` escape-hatch node runs. Named so the +/// Which interpreter an explicit `Shell` escape-hatch node runs. Named so the /// prompt can render the stage as "opaque — resolves only the shell": this is /// the *only* place a shell re-enters the execution path, and it is never /// hidden. #[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] -pub enum Shell { +pub enum ShellProc { Sh, Bash, } @@ -114,13 +114,18 @@ pub enum Shell { #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "op", rename_all = "kebab-case")] pub enum Plan { - /// A single literal command: prog + args. No word-splitting, globbing, or - /// substitution — the leaf of every tree. - Run { argv: Vec }, - /// Sequence; stop at the first non-accepted exit (Dune `progn`, ≈ `&&`/`;`). + /// A single literal command: prog + args (one `execve`). No word-splitting, + /// globbing, or substitution — the leaf of every tree. + Exec { argv: Vec }, + /// Run steps in order, stopping at the first non-accepted exit (≈ `&&`). + /// Status is the last step that ran. Dune `progn` with short-circuit. + Run { steps: Vec }, + /// Run every step in order regardless of exit (≈ `;`). A non-accepted exit + /// does not stop the rest; a timeout or spawn failure still aborts. Status + /// is the last step's. Seq { steps: Vec }, /// Alternatives; run steps until one succeeds, continuing past a non-accepted - /// exit and stopping on the first success (≈ `||`). Dual of `seq`. + /// exit and stopping on the first success (≈ `||`). Dual of `run`. Alt { steps: Vec }, /// Connect stdout→stdin left-to-right (Dune `pipe-stdout`, = `|`). Pipe { stages: Vec }, @@ -171,7 +176,7 @@ pub enum Plan { /// EXPLICIT opaque escape hatch (Dune `system`/`bash`). Rendered at the /// prompt as "opaque — resolves only the shell"; a strict policy mode /// rejects it. The sole surviving shell path, and always named. - System { shell: Shell, script: String }, + Shell { shell: ShellProc, script: String }, } impl Plan { @@ -199,25 +204,17 @@ impl Plan { } }; match self { - Plan::Run { argv } => { + Plan::Exec { argv } => { if argv.is_empty() { - return Err("run stage must have a command".to_string()); + return Err("exec stage must have a command".to_string()); } for a in argv { check(a, "argv element")?; } } - Plan::Seq { steps } => { + Plan::Run { steps } | Plan::Seq { steps } | Plan::Alt { steps } => { if steps.is_empty() { - return Err("seq must have at least one step".to_string()); - } - for s in steps { - s.validate_tree(depth + 1, nodes)?; - } - } - Plan::Alt { steps } => { - if steps.is_empty() { - return Err("alt must have at least one step".to_string()); + return Err("run/seq/alt must have at least one step".to_string()); } for s in steps { s.validate_tree(depth + 1, nodes)?; @@ -262,7 +259,7 @@ impl Plan { Plan::WithAcceptedExitCodes { body, .. } => { body.validate_tree(depth + 1, nodes)?; } - Plan::System { script, .. } => { + Plan::Shell { script, .. } => { check(script, "system script")?; } } @@ -274,9 +271,9 @@ impl Plan { /// this drives `Request::effective_privileged`. pub fn contains_runas(&self) -> bool { match self { - Plan::Run { .. } | Plan::System { .. } => false, + Plan::Exec { .. } | Plan::Shell { .. } => false, Plan::RunAs { .. } => true, - Plan::Seq { steps } | Plan::Alt { steps } => { + Plan::Run { steps } | Plan::Seq { steps } | Plan::Alt { steps } => { steps.iter().any(Plan::contains_runas) } Plan::Pipe { stages } => stages.iter().any(Plan::contains_runas), @@ -293,10 +290,10 @@ impl Plan { /// escalation is to a named non-root user). pub fn contains_root(&self) -> bool { match self { - Plan::Run { .. } | Plan::System { .. } => false, + Plan::Exec { .. } | Plan::Shell { .. } => false, Plan::RunAs { who: RunAs::Root, .. } => true, Plan::RunAs { who: RunAs::User { .. }, body } => body.contains_root(), - Plan::Seq { steps } | Plan::Alt { steps } => { + Plan::Run { steps } | Plan::Seq { steps } | Plan::Alt { steps } => { steps.iter().any(Plan::contains_root) } Plan::Pipe { stages } => stages.iter().any(Plan::contains_root), @@ -313,15 +310,15 @@ impl Plan { /// becomes a bare `Run`; multiple stages become a `Pipe`. Ambient privilege is /// **not** encoded here — the executor seeds its run-as context from /// `Request::privileged`, so a desugared tree and an explicit plan are handled -/// uniformly. An empty pipeline (control actions only) yields an empty `Seq`, +/// uniformly. An empty pipeline (control actions only) yields an empty `Run`, /// which never reaches execution. fn desugar_pipeline(pipeline: &[Vec]) -> Plan { let mut stages: Vec = pipeline .iter() - .map(|argv| Plan::Run { argv: argv.clone() }) + .map(|argv| Plan::Exec { argv: argv.clone() }) .collect(); match stages.len() { - 0 => Plan::Seq { steps: vec![] }, + 0 => Plan::Run { steps: vec![] }, 1 => stages.pop().unwrap(), _ => Plan::Pipe { stages }, } @@ -781,8 +778,8 @@ mod tests { // === Structured plan schema =========================================== - fn run(argv: &[&str]) -> Plan { - Plan::Run { argv: argv.iter().map(|s| s.to_string()).collect() } + fn exec(argv: &[&str]) -> Plan { + Plan::Exec { argv: argv.iter().map(|s| s.to_string()).collect() } } /// A single-stage pipeline desugars to a bare `Run`; a multi-stage pipeline @@ -791,12 +788,12 @@ mod tests { #[test] fn desugar_pipeline_shapes() { let single = desugar_pipeline(&[vec!["ls".into(), "-l".into()]]); - assert_eq!(single, run(&["ls", "-l"])); + assert_eq!(single, exec(&["ls", "-l"])); let multi = desugar_pipeline(&[vec!["ls".into()], vec!["wc".into(), "-l".into()]]); - assert_eq!(multi, Plan::Pipe { stages: vec![run(&["ls"]), run(&["wc", "-l"])] }); + assert_eq!(multi, Plan::Pipe { stages: vec![exec(&["ls"]), exec(&["wc", "-l"])] }); - assert_eq!(desugar_pipeline(&[]), Plan::Seq { steps: vec![] }); + assert_eq!(desugar_pipeline(&[]), Plan::Run { steps: vec![] }); } /// An explicit `plan` round-trips through JSON with the `op`-tagged form, @@ -807,14 +804,14 @@ mod tests { let req = Request::new_plan( String::new(), "s".into(), - Plan::Seq { steps: vec![run(&["true"]), run(&["false"])] }, + Plan::Run { steps: vec![exec(&["true"]), exec(&["false"])] }, HashMap::new(), String::new(), true, false, ); let json = serde_json::to_string(&req).unwrap(); - assert!(json.contains(r#""op":"seq""#), "tagged form missing: {json}"); + assert!(json.contains(r#""op":"run""#), "tagged form missing: {json}"); let back: Request = serde_json::from_str(&json).unwrap(); assert_eq!(back.exec_plan(), req.exec_plan()); @@ -822,7 +819,7 @@ mod tests { let alt = Request::new_plan( String::new(), "s".into(), - Plan::Alt { steps: vec![run(&["false"]), run(&["true"])] }, + Plan::Alt { steps: vec![exec(&["false"]), exec(&["true"])] }, HashMap::new(), String::new(), true, @@ -848,7 +845,7 @@ mod tests { let root = Request::new_plan( String::new(), "s".into(), - Plan::RunAs { who: RunAs::Root, body: Box::new(run(&["id", "-un"])) }, + Plan::RunAs { who: RunAs::Root, body: Box::new(exec(&["id", "-un"])) }, HashMap::new(), String::new(), true, @@ -864,16 +861,16 @@ mod tests { ); // The exact JSON shapes the MCP `plan` doc advertises must parse. - let doc_root = r#"{"op":"run-as","as":"root","do":{"op":"run","argv":["id"]}}"#; - let doc_user = r#"{"op":"run-as","as":"user","name":"alice","do":{"op":"run","argv":["id"]}}"#; + let doc_root = r#"{"op":"run-as","as":"root","do":{"op":"exec","argv":["id"]}}"#; + let doc_user = r#"{"op":"run-as","as":"user","name":"alice","do":{"op":"exec","argv":["id"]}}"#; let p_root: Plan = serde_json::from_str(doc_root).unwrap(); let p_user: Plan = serde_json::from_str(doc_user).unwrap(); - assert_eq!(p_root, Plan::RunAs { who: RunAs::Root, body: Box::new(run(&["id"])) }); + assert_eq!(p_root, Plan::RunAs { who: RunAs::Root, body: Box::new(exec(&["id"])) }); assert_eq!( p_user, Plan::RunAs { who: RunAs::User { name: "alice".into() }, - body: Box::new(run(&["id"])), + body: Box::new(exec(&["id"])), } ); } @@ -887,7 +884,7 @@ mod tests { "s".into(), Plan::RunAs { who: RunAs::Root, - body: Box::new(run(&["sysctl", "-w", "x=1"])), + body: Box::new(exec(&["sysctl", "-w", "x=1"])), }, HashMap::new(), String::new(), @@ -900,13 +897,13 @@ mod tests { // A named-user RunAs still forces the privileged gate, but is not root. req.plan = Some(Plan::RunAs { who: RunAs::User { name: "alice".into() }, - body: Box::new(run(&["dune", "build"])), + body: Box::new(exec(&["dune", "build"])), }); assert!(req.effective_privileged()); assert!(!req.runs_as_root()); // A plain plan with no RunAs and no flag is unprivileged. - req.plan = Some(run(&["ls"])); + req.plan = Some(exec(&["ls"])); req.privileged = false; assert!(!req.effective_privileged()); } @@ -919,7 +916,7 @@ mod tests { Plan::Setenv { key: "LANG".into(), value: "x\u{1b}y".into(), // ESC in a setenv value - body: Box::new(run(&["true"])), + body: Box::new(exec(&["true"])), }, HashMap::new(), String::new(), @@ -933,7 +930,7 @@ mod tests { #[test] fn validate_rejects_both_pipeline_and_plan() { let mut req = exec_request(); - req.plan = Some(run(&["ls"])); + req.plan = Some(exec(&["ls"])); let err = ValidatedRequest::validate(req).unwrap_err(); assert!(err.contains("both pipeline and plan"), "got: {err}"); } @@ -941,7 +938,7 @@ mod tests { #[test] fn validate_rejects_overdeep_plan() { // Nest RunAs past MAX_PLAN_DEPTH. - let mut p = run(&["true"]); + let mut p = exec(&["true"]); for _ in 0..(MAX_PLAN_DEPTH + 2) { p = Plan::RunAs { who: RunAs::Root, body: Box::new(p) }; } @@ -955,14 +952,14 @@ mod tests { #[test] fn validate_rejects_empty_run_and_short_pipe() { let empty_run = Request::new_plan( - String::new(), "s".into(), Plan::Run { argv: vec![] }, + String::new(), "s".into(), Plan::Exec { argv: vec![] }, HashMap::new(), String::new(), false, false, ); assert!(ValidatedRequest::validate(empty_run).is_err()); let short_pipe = Request::new_plan( String::new(), "s".into(), - Plan::Pipe { stages: vec![run(&["ls"])] }, + Plan::Pipe { stages: vec![exec(&["ls"])] }, HashMap::new(), String::new(), false, false, ); assert!(ValidatedRequest::validate(short_pipe).is_err()); diff --git a/src/tui.rs b/src/tui.rs index 7dae632..a78e93f 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -42,11 +42,11 @@ pub fn command_summary(req: &ValidatedRequest) -> String { /// the detailed per-leaf resolution happens in the multi-line prompt render. fn plan_to_string(plan: &Plan) -> String { match plan { - Plan::Run { argv } => shell_join(argv), - Plan::System { shell, script } => { + Plan::Exec { argv } => shell_join(argv), + Plan::Shell { shell, script } => { let bin = match shell { - crate::protocol::Shell::Sh => "sh", - crate::protocol::Shell::Bash => "bash", + crate::protocol::ShellProc::Sh => "sh", + crate::protocol::ShellProc::Bash => "bash", }; shell_join(&[bin.to_string(), "-c".to_string(), script.clone()]) } @@ -55,6 +55,11 @@ fn plan_to_string(plan: &Plan) -> String { .map(plan_to_string) .collect::>() .join(" | "), + Plan::Run { steps } => steps + .iter() + .map(plan_to_string) + .collect::>() + .join(" && "), Plan::Seq { steps } => steps .iter() .map(plan_to_string) @@ -742,20 +747,20 @@ mod tests { /// `/usr/bin/bash` resolves line. A `System` stage is flagged opaque. #[test] fn plan_stages_render_per_leaf_privilege_and_resolves() { - use crate::protocol::{Plan, Request, RunAs, Shell, ValidatedRequest}; + use crate::protocol::{Plan, Request, RunAs, ShellProc, ValidatedRequest}; - let plan = Plan::Seq { + let plan = Plan::Run { steps: vec![ Plan::RunAs { who: RunAs::Root, - body: Box::new(Plan::Run { argv: vec!["true".into()] }), + body: Box::new(Plan::Exec { argv: vec!["true".into()] }), }, Plan::RunAs { who: RunAs::User { name: "alice".into() }, - body: Box::new(Plan::Run { argv: vec!["true".into()] }), + body: Box::new(Plan::Exec { argv: vec!["true".into()] }), }, - Plan::System { - shell: Shell::Sh, + Plan::Shell { + shell: ShellProc::Sh, script: "echo hi".into(), }, ], @@ -795,7 +800,7 @@ mod tests { let req = ValidatedRequest::validate(Request::new_plan( String::new(), "t".into(), - Plan::Run { argv: vec!["true".into()] }, + Plan::Exec { argv: vec!["true".into()] }, std::collections::HashMap::new(), String::new(), false, diff --git a/tests/plan_exec.rs b/tests/plan_exec.rs index be58318..518ad22 100644 --- a/tests/plan_exec.rs +++ b/tests/plan_exec.rs @@ -14,8 +14,8 @@ use sudo_proxy::protocol::{ mod common; -fn run(argv: &[&str]) -> Plan { - Plan::Run { +fn exec(argv: &[&str]) -> Plan { + Plan::Exec { argv: argv.iter().map(|s| s.to_string()).collect(), } } @@ -40,12 +40,12 @@ fn decode_stdout(resp: &sudo_proxy::protocol::Response) -> String { /// A pipe nested inside a seq: three leaves, and the response's stage list and /// aggregate stdout match the pre-order leaf layout. #[test] -fn pipe_inside_seq_runs_and_aggregates() { - let plan = Plan::Seq { +fn pipe_inside_run_runs_and_aggregates() { + let plan = Plan::Run { steps: vec![ - run(&["echo", "alpha"]), + exec(&["echo", "alpha"]), Plan::Pipe { - stages: vec![run(&["echo", "hi"]), run(&["wc", "-c"])], + stages: vec![exec(&["echo", "hi"]), exec(&["wc", "-c"])], }, ], }; @@ -70,7 +70,7 @@ fn redirect_stdout_to_file_truncate() { path: path.to_string_lossy().into_owned(), mode: WriteMode::Truncate, }, - body: Box::new(run(&["printf", "hello-file"])), + body: Box::new(exec(&["printf", "hello-file"])), }; let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); assert_eq!(resp.status, Status::Ok, "got {resp:?}"); @@ -91,7 +91,7 @@ fn redirect_stdout_append() { path: path.to_string_lossy().into_owned(), mode: WriteMode::Append, }, - body: Box::new(run(&["printf", "more"])), + body: Box::new(exec(&["printf", "more"])), }; let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); assert_eq!(resp.status, Status::Ok, "got {resp:?}"); @@ -104,20 +104,20 @@ fn setenv_is_visible_to_the_stage() { let plan = Plan::Setenv { key: "LANG".into(), value: "xyz-sentinel".into(), - body: Box::new(run(&["sh", "-c", "printf %s \"$LANG\""])), + body: Box::new(exec(&["sh", "-c", "printf %s \"$LANG\""])), }; let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); assert_eq!(resp.status, Status::Ok, "got {resp:?}"); assert_eq!(decode_stdout(&resp), "xyz-sentinel"); } -/// A seq stops at the first failing step; later stages do not run. +/// A `run` (&&) stops at the first failing step; later stages do not run. #[test] -fn seq_short_circuits_on_failure() { - let plan = Plan::Seq { +fn run_short_circuits_on_failure() { + let plan = Plan::Run { steps: vec![ - run(&["false"]), - run(&["sh", "-c", "printf SHOULD_NOT_RUN"]), + exec(&["false"]), + exec(&["sh", "-c", "printf SHOULD_NOT_RUN"]), ], }; let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); @@ -130,17 +130,35 @@ fn seq_short_circuits_on_failure() { ); } +/// A `seq` (;) runs every step regardless of a non-accepted failure — the dual +/// of `run`'s short-circuit. Both stages run; status is the last step's. +#[test] +fn seq_runs_every_step_despite_failure() { + let plan = Plan::Seq { + steps: vec![exec(&["false"]), exec(&["sh", "-c", "printf ran-anyway"])], + }; + let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); + assert_eq!(resp.status, Status::Ok, "got {resp:?}"); + assert_eq!(resp.stages.len(), 2); + assert_eq!(resp.stages[0].exit_code, 1, "false exits 1"); + assert_eq!(resp.stages[1].exit_code, 0, "second stage still runs"); + assert!( + decode_stdout(&resp).contains("ran-anyway"), + "a `;` seq must not short-circuit on failure: {resp:?}" + ); +} + /// `with-accepted-exit-codes` lets a non-zero exit count as success, so the /// seq continues past it. #[test] -fn accepted_exit_codes_allow_seq_to_continue() { - let plan = Plan::Seq { +fn accepted_exit_codes_allow_run_to_continue() { + let plan = Plan::Run { steps: vec![ Plan::WithAcceptedExitCodes { codes: vec![1], - body: Box::new(run(&["false"])), + body: Box::new(exec(&["false"])), }, - run(&["sh", "-c", "printf continued"]), + exec(&["sh", "-c", "printf continued"]), ], }; let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); @@ -156,8 +174,8 @@ fn accepted_exit_codes_allow_seq_to_continue() { fn alt_short_circuits_on_success() { let plan = Plan::Alt { steps: vec![ - run(&["true"]), - run(&["sh", "-c", "printf SHOULD_NOT_RUN"]), + exec(&["true"]), + exec(&["sh", "-c", "printf SHOULD_NOT_RUN"]), ], }; let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); @@ -178,7 +196,7 @@ fn alt_short_circuits_on_success() { #[test] fn alt_runs_fallback_on_failure() { let plan = Plan::Alt { - steps: vec![run(&["false"]), run(&["sh", "-c", "printf fallback"])], + steps: vec![exec(&["false"]), exec(&["sh", "-c", "printf fallback"])], }; let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); assert_eq!(resp.status, Status::Ok, "got {resp:?}"); @@ -194,7 +212,7 @@ fn alt_runs_fallback_on_failure() { #[test] fn alt_all_fail_runs_every_step() { let plan = Plan::Alt { - steps: vec![run(&["false"]), run(&["false"])], + steps: vec![exec(&["false"]), exec(&["false"])], }; let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); assert_eq!(resp.status, Status::Ok, "got {resp:?}"); @@ -211,9 +229,9 @@ fn accepted_exit_code_stops_alt() { steps: vec![ Plan::WithAcceptedExitCodes { codes: vec![1], - body: Box::new(run(&["false"])), + body: Box::new(exec(&["false"])), }, - run(&["sh", "-c", "printf SHOULD_NOT_RUN"]), + exec(&["sh", "-c", "printf SHOULD_NOT_RUN"]), ], }; let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); @@ -231,8 +249,8 @@ fn accepted_exit_code_stops_alt() { /// The explicit `System` escape hatch runs a shell script verbatim. #[test] fn system_escape_hatch_runs() { - let plan = Plan::System { - shell: sudo_proxy::protocol::Shell::Sh, + let plan = Plan::Shell { + shell: sudo_proxy::protocol::ShellProc::Sh, script: "printf hatch-ran".into(), }; let resp = exec_direct(&make_plan_req(plan), &HashMap::new()); @@ -248,7 +266,7 @@ fn plan_request_end_to_end_through_daemon() { let server = common::start_test_server(common::TestServerOpts::default()); let plan = Plan::Pipe { - stages: vec![run(&["printf", "a\\nb\\nc\\n"]), run(&["wc", "-l"])], + stages: vec![exec(&["printf", "a\\nb\\nc\\n"]), exec(&["wc", "-l"])], }; let mut req = Request::new_plan( String::new(), diff --git a/tests/validation.rs b/tests/validation.rs index 4ca84e8..3578414 100644 --- a/tests/validation.rs +++ b/tests/validation.rs @@ -257,7 +257,7 @@ fn plan_req(id: &str, plan: Plan, privileged: bool) -> Request { #[test] fn plan_dangerous_leaf_rejected() { let s = server(); - let plan = Plan::Run { + let plan = Plan::Exec { argv: vec!["echo".into(), "x\u{202E}y".into()], }; let resp = s.send(&plan_req("v-plan-bidi", plan, false)); @@ -271,7 +271,7 @@ fn plan_and_pipeline_both_set_rejected() { let s = server(); let mut req = plan_req( "v-both", - Plan::Run { argv: vec!["true".into()] }, + Plan::Exec { argv: vec!["true".into()] }, false, ); req.pipeline = vec![vec!["true".into()]]; @@ -284,7 +284,7 @@ fn plan_and_pipeline_both_set_rejected() { #[test] fn plan_overdeep_rejected() { let s = server(); - let mut p = Plan::Run { argv: vec!["true".into()] }; + let mut p = Plan::Exec { argv: vec!["true".into()] }; for _ in 0..40 { p = Plan::RunAs { who: RunAs::Root, @@ -305,10 +305,10 @@ fn plan_structural_error_rejected_without_prompt() { let plan = Plan::Redirect { stream: sudo_proxy::protocol::Stream::Stdout, target: sudo_proxy::protocol::RedirTarget::DevNull, - body: Box::new(Plan::Seq { + body: Box::new(Plan::Run { steps: vec![ - Plan::Run { argv: vec!["true".into()] }, - Plan::Run { argv: vec!["true".into()] }, + Plan::Exec { argv: vec!["true".into()] }, + Plan::Exec { argv: vec!["true".into()] }, ], }), }; @@ -328,8 +328,8 @@ fn plan_redirect_of_alt_rejected_without_prompt() { target: sudo_proxy::protocol::RedirTarget::DevNull, body: Box::new(Plan::Alt { steps: vec![ - Plan::Run { argv: vec!["false".into()] }, - Plan::Run { argv: vec!["true".into()] }, + Plan::Exec { argv: vec!["false".into()] }, + Plan::Exec { argv: vec!["true".into()] }, ], }), }; @@ -343,7 +343,7 @@ fn plan_redirect_of_alt_rejected_without_prompt() { #[test] fn plan_valid_runs() { let s = server(); - let plan = Plan::Run { + let plan = Plan::Exec { argv: vec!["true".into()], }; let resp = s.send(&plan_req("v-plan-ok", plan, false)); From 94229c824424cd0e9dc21fb91ec062814a6be895 Mon Sep 17 00:00:00 2001 From: Cuihtlauac ALVARADO Date: Mon, 19 Oct 2026 09:16:43 +0200 Subject: [PATCH 07/14] docs: contrast sudo-proxy's security model with agent harnesses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a "Security model vs. agent harnesses" section to docs/architecture.md and a short cross-linked note in the README. Two axes: the gatekeeper (a mandatory human keypress, no classifier/auto-approve — vs. a harness's software policy + optional classifier + optional human), and privilege direction (sudo-proxy escalates beyond what the agent has; a harness confines an agent that already runs as you). The two are complementary, not competing. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 10 +++++++++ docs/architecture.md | 53 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/README.md b/README.md index 93efa9a..d6b0b0a 100644 --- a/README.md +++ b/README.md @@ -63,6 +63,16 @@ relaxed if you deliberately mark a host eligible in `hosts.json` *and* confirm once per session (a grant that is never persisted). See [REVIEWING.md](REVIEWING.md) invariant **G7**. +**Escalate vs. confine.** sudo-proxy's security model is the inverse of an agent +harness's. A harness (Claude Code, Codex CLI, …) gates with *software* — +permission rules plus an optional classifier — and runs the agent at *your own* +privilege, narrowing it to a safe subset. sudo-proxy inverts both: a *human* +approves every command (no classifier, no auto-approve) and execution is +*escalated* beyond what the agent has. The two are complementary — the harness +bounds the agent to your privileges; sudo-proxy is how it reaches past them, with +a human as the escalation authority. See +[docs/architecture.md](docs/architecture.md#security-model-vs-agent-harnesses). + For how this relates to mcp-firewall, sandboxing, polkit, doas, and other neighboring tools, see [docs/comparison.md](docs/comparison.md). diff --git a/docs/architecture.md b/docs/architecture.md index 8b29519..7c7ab5e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -2,6 +2,59 @@ [← README](../README.md) +## Security model vs. agent harnesses + +sudo-proxy and the permission model of an agent CLI harness (Claude Code, Codex +CLI, and similar) differ on two axes. Getting these straight explains why +sudo-proxy leans so hard on the displayed-equals-executed guarantee. + +**1. The gatekeeper (human vs. software).** Both are *mediated*: the model never +executes anything itself — it only emits a *request* that something else decides +on and runs. The difference is what sits in the gate: + +- A **harness** gates with **software**: configured permission rules + (allow / ask / deny), optionally a safety classifier, and a human prompt only + on the "ask" path. The designed-for case is **automated** — an allowlisted + command runs with no human in the loop. +- **sudo-proxy** gates with a **mandatory human keypress** on `/dev/tty` for + *every* command, fail-closed (default deny, 60 s timeout). There is **no + classifier and no auto-approve** for privileged commands; software only + *rejects* (dangerous characters, structural invalidity, replay/freshness) — it + never *approves*. The human is the sole approving authority. + +**2. Privilege direction (escalate vs. confine).** This is the deeper difference +in assumption: + +- A **harness** performs capability *restriction*. The agent already runs **as + you**, with your ambient privileges; the harness *narrows* that to a safe + subset. Its executor (the Bash tool) runs **unprivileged** — no escalation. +- **sudo-proxy** performs capability *escalation*. The client/agent has **no + privilege of its own**; sudo-proxy *grants* one it otherwise lacks — + `sudo` / `pkexec` to **root or another user** — and binds each grant to a + human who read the exact command. + +| | Agent harness (Claude Code, Codex CLI) | sudo-proxy | +|---|---|---| +| Model's role | emits a tool request | emits a request (MCP / socket client) | +| Gate | permission rules + optional classifier + optional human | **mandatory human** per command; no classifier | +| Default decision | can be fully automated | never automated (always a keypress) | +| Executor privilege | your own (ambient) | **escalated** (root / other user) | +| Purpose | **confine** an agent that runs as you | **escalate** beyond what the agent has | +| Failure mode | depends on policy | fail-closed to a human (default deny) | + +Both start from the same premise — the model's output is an *untrusted request*, +not an authorization — and answer "who authorizes it?" differently: **software +policy** (harness) vs. **a human reading the literal command** (sudo-proxy). +Because the authorizer is a human rather than a classifier, sudo-proxy's entire +security reduces to *what that human can see and comprehend at the prompt* — +hence invariants **G1** (nothing privileged runs without a human approving the +exact command shown) and the per-stage resolved-binary rendering +([REVIEWING.md](../REVIEWING.md)). + +The two are **complementary, not competing**: the harness confines the agent to +your privileges; sudo-proxy is how the agent reaches *past* them for a rare +privileged action — with a human, not a classifier, as the escalation authority. + ## Project structure ``` From 1118a435d56316523ed10215c49cecad4d22075b Mon Sep 17 00:00:00 2001 From: Cuihtlauac ALVARADO Date: Mon, 19 Oct 2026 10:14:56 +0200 Subject: [PATCH 08/14] docs(backlog): capture the shell-escape-hatch design goal + remaining levers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Record the plan model's raison d'être — make the `Shell` escape hatch unnecessary (shrink-and-flag, not delete) — and three remaining levers: close the `exec sh -c` loophole, implement the documented strict policy mode, and catalogue the shell tail (detach, read-file) as first-class op proposals. Also mark the append-redirection gap as landed. Co-Authored-By: Claude Opus 4.8 (1M context) --- backlog.md | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/backlog.md b/backlog.md index 99321e2..2366f61 100644 --- a/backlog.md +++ b/backlog.md @@ -167,7 +167,8 @@ on top:** - ~~**`||` fallback.**~~ *Landed:* the `alt` op (dual of `seq`) runs steps until one succeeds, covering `pgrep … || echo none`. A first-class op, not a fold into `with-accepted-exit-codes`. -- **Append redirection (`>>`).** `with-stdout-to` truncates; no append mode. +- ~~**Append redirection (`>>`).**~~ *Landed:* `redirect` takes `mode:"append"` + alongside `"truncate"` (and `stream:"both"` for `2>&1`). - **Runtime substitution** (`$(cat /proc/…)`, `$!`, `$?`). Dune resolves `%{…}` at rule-eval, not runtime — so it has no runtime `$(…)` either, which *confirms* our plan: promote the common cases to first-class ops (a read-file op @@ -187,6 +188,49 @@ shell`) and a policy mode rejects it — making `no-opaque-blob` a special case daemon-side *reject-unresolvable-stages*, enforced structurally and for every client rather than by regex in one user's hook. +## Make the shell escape hatch avoidable (plan-model design goal + remaining levers) + +**The design goal.** The `Plan` type's *raison d'être* is to make its `Shell` +constructor (the opaque escape hatch, formerly `System`) unnecessary: every +structured op exists so a given shell feature need not be expressed by shelling +out. The real invariant is **no shell on the privileged path**; `Shell` is the +one node that breaks displayed-equals-executed — it resolves only the shell +binary, so its inner commands are invisible at the prompt. The goal is therefore +not to *delete* `Shell` (the shell tail — runtime `$(…)`, `$!`, globbing, +word-splitting, profile functions — is open-ended) but to **shrink-and-flag**: +drive real usage into structured ops, and keep every remaining shell use loud, +countable, and policy-rejectable. Read each `Shell` node as measurable debt — a +prompt to ask "what op would have avoided this?" That loop already produced `alt` +(for `||`) and `seq` (for `;`). See +[docs/architecture.md](docs/architecture.md) ("Security model vs. agent +harnesses") for the framing. + +**Remaining levers** (rough payoff order): + +1. **Close the `exec sh -c` loophole** (highest payoff). The goal is quietly + defeated today not by `Shell` but by its dishonest sibling: a plain `Exec` + leaf (or pipe stage) `["sh","-c","