diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 577a8aa..c764e38 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -73,6 +73,32 @@ jobs: echo "crate ${VERSION} did not appear on crates.io in time" >&2 exit 1 + # The MCPB package entry in server.json points at a GitHub release asset + # (built + uploaded by release.yml on the same tag) and must carry that + # file's sha256. release.yml runs in parallel, so wait for the asset, then + # compute and inject the hash. The registry validates the URL with a HEAD + # request at publish time, so the asset must be live before we publish. + - name: Wait for MCPB asset and inject sha256 into server.json + run: | + URL=$(jq -r '.packages[] | select(.registryType=="mcpb") | .identifier' server.json) + echo "MCPB asset: $URL" + TMP=$(mktemp) + for i in $(seq 1 60); do + if curl -fsSL -A "$CRATES_UA" "$URL" -o "$TMP"; then + echo "MCPB asset is live" + break + fi + echo "waiting for MCPB asset (attempt ${i})..." + sleep 10 + [ "$i" -eq 60 ] && { echo "MCPB asset did not appear in time" >&2; exit 1; } + done + SHA=$(sha256sum "$TMP" | awk '{print $1}') + echo "sha256=$SHA" + jq --arg h "$SHA" \ + '(.packages[] | select(.registryType=="mcpb") | .fileSha256) = $h' \ + server.json > server.json.tmp + mv server.json.tmp server.json + - name: Install mcp-publisher run: | curl -L "https://github.com/modelcontextprotocol/registry/releases/latest/download/mcp-publisher_$(uname -s | tr '[:upper:]' '[:lower:]')_$(uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/').tar.gz" | tar xz mcp-publisher diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3ea2cf5..95bdeca 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -41,19 +41,30 @@ jobs: tar czf "${DIR}.tar.gz" "$DIR" echo "TARBALL=${DIR}.tar.gz" >> "$GITHUB_ENV" + - name: Build MCPB bundle + run: | + VERSION="${GITHUB_REF#refs/tags/v}" + MCPB="sudo-proxy-mcp-v${VERSION}-x86_64-linux.mcpb" + packaging/mcpb/build-mcpb.sh \ + "$VERSION" \ + target/x86_64-unknown-linux-musl/release \ + . + echo "MCPB=${MCPB}" >> "$GITHUB_ENV" + - name: Install rsign2 (minisign signer) run: cargo install rsign2 --locked - - name: Sign tarball (minisign) + - name: Sign release assets (minisign) run: | umask 077 printf '%s\n' "$MINISIGN_SECRET_KEY" > minisign.key - rsign sign -W -s minisign.key -x "${TARBALL}.sig" \ - -t "sudo-proxy ${GITHUB_REF#refs/tags/} release" \ - "$TARBALL" + for f in "$TARBALL" "$MCPB"; do + rsign sign -W -s minisign.key -x "${f}.sig" \ + -t "sudo-proxy ${GITHUB_REF#refs/tags/} release" \ + "$f" + rsign verify -P "$MINISIGN_PUBKEY" -x "${f}.sig" "$f" + done rm -f minisign.key - rsign verify -P "$MINISIGN_PUBKEY" -x "${TARBALL}.sig" "$TARBALL" - echo "SIGNATURE=${TARBALL}.sig" >> "$GITHUB_ENV" env: MINISIGN_SECRET_KEY: ${{ secrets.MINISIGN_SECRET_KEY }} MINISIGN_PUBKEY: RWT7gwtBU0v4puI76u0oYwMAT9nmYwGimSOnqJJ+kHExsjTDQj1eZkMW @@ -61,7 +72,9 @@ jobs: - name: Create GitHub Release run: | TAG="${GITHUB_REF#refs/tags/}" - gh release create "$TAG" "$TARBALL" "$SIGNATURE" \ + gh release create "$TAG" \ + "$TARBALL" "${TARBALL}.sig" \ + "$MCPB" "${MCPB}.sig" \ --title "$TAG" \ --generate-notes env: diff --git a/Cargo.lock b/Cargo.lock index b4cca7b..079b44f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -573,7 +573,7 @@ checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" [[package]] name = "sudo-proxy" -version = "1.0.0" +version = "1.1.0" dependencies = [ "base64", "libc", diff --git a/Cargo.toml b/Cargo.toml index d4c50e5..cddcd6e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "sudo-proxy" -version = "1.0.0" +version = "1.1.0" edition = "2021" license = "MIT" description = "Privileged command execution proxy with human approval via pkexec or sudo" diff --git a/README.md b/README.md index e3eb019..cbf8299 100644 --- a/README.md +++ b/README.md @@ -141,7 +141,7 @@ as `mcp-name: io.github.tarides/sudo-proxy`. ## Documentation - [docs/install.md](docs/install.md) — install variants, remote deploy, building from source -- [docs/mcp.md](docs/mcp.md) — MCP tools (`start_server`, `execute`, `update_host`), config, known hosts +- [docs/mcp.md](docs/mcp.md) — MCP tools (`start_server`, `execute`, `status`, `stop_server`, `update_host`), config, known hosts - [docs/usage.md](docs/usage.md) — CLI flags, non-privileged mode, SSH tunnels, agent forwarding - [docs/protocol.md](docs/protocol.md) — JSON-line wire protocol over the Unix socket - [docs/security.md](docs/security.md) — security model; [docs/security-audit.md](docs/security-audit.md) — point-in-time audit; [docs/threat-model.md](docs/threat-model.md) — STRIDE + attack tree; [docs/formalisation-roadmap.md](docs/formalisation-roadmap.md) — graduated-assurance plan; [docs/assurance-case.md](docs/assurance-case.md) — GSN argument diff --git a/docs/mcp.md b/docs/mcp.md index df48db5..d0f25e8 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -30,6 +30,24 @@ sudo-proxy as tools over stdio JSON-RPC. Any MCP-capable AI client - `host` (required): hostname to update. - `description`: human-readable description (e.g. "CI server"). - `os`: operating system info (e.g. "Ubuntu 24.04"). +- Partial update: only the fields provided are changed; a host not yet in the + registry is added automatically. + +**`stop_server`** — stop a running sudo-proxy daemon. +- `host`: which daemon to stop (omit for the local one). +- Sends a `stop` control request over the socket. The daemon prints a + shutdown notice on its terminal (no approval prompt), exits, and its + terminal window — and SSH tunnel, for remote hosts — closes. +- "Not running" is reported as a normal (non-error) result. +- Daemons older than 1.1 don't understand `stop`; the tool reports their + version and asks for a manual `q`/Ctrl+C in the daemon's terminal. + +**`status`** — report daemon status without executing anything. +- `host`: check one daemon; omit to check the local daemon plus every host in + the registry. +- Per host: socket presence, readiness, live daemon version (via a `ping` + control request that needs no human approval), and registry metadata. +- Read-only except for refreshing the registry's last-connected/version cache. ## Claude Code configuration @@ -106,7 +124,8 @@ correctly (no Docker needed): cargo test --test mcp_introspection ``` -It should enumerate `execute`, `start_server`, and `update_host`. +It should enumerate `execute`, `start_server`, `status`, `stop_server`, and +`update_host`. ## Glama terminology @@ -116,7 +135,7 @@ sudo-proxy: | Glama term | sudo-proxy | | ------------- | ---------- | | **Server** | the `sudo-proxy-mcp` binary — the stdio MCP server, listed as `tarides/sudo-proxy`. | -| **Tools** | `execute`, `start_server`, `update_host`. | +| **Tools** | `execute`, `start_server`, `status`, `stop_server`, `update_host`. | | **Connector** | *none* — a connector is a **remote/hosted** MCP server (a managed HTTP endpoint). sudo-proxy is local-only, so it is a server but never a connector. | Two caveats: @@ -130,5 +149,5 @@ Two caveats: process. sudo-proxy's own *server* — what the `start_server` tool spawns — is the `sudo-proxy` host daemon (Unix socket + TUI) that the MCP server proxies to. That daemon, `sudo-request`, `pkexec-cache`, and target *hosts* all sit - below Glama's vocabulary; in MCP terms sudo-proxy is one server exposing three + below Glama's vocabulary; in MCP terms sudo-proxy is one server exposing five tools. diff --git a/docs/protocol.md b/docs/protocol.md index 91a0089..29b400a 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -27,7 +27,8 @@ The wire shape is always a list of stages: a single command is to `a | b | c`). The MCP `execute` tool accepts a convenience `argv` field and wraps it for you. -Field defaults (every field except `pipeline` is optional on the wire): +Field defaults (every field is optional on the wire, but an `exec` +request with an empty or missing `pipeline` is rejected at validation): - `id` — defaults to a fresh UUIDv4. - `host`, `session`, `time`, `reason`, `env` — empty if omitted, but @@ -37,6 +38,21 @@ Field defaults (every field except `pipeline` is optional on the wire): client that forgets the field still goes through approval + sudo). - `forward_agent` — defaults to `false`. Setting `true` is only valid when `privileged: false`. +- `action` — defaults to `"exec"` (and is omitted from the wire for exec + requests, keeping them byte-identical to the pre-1.1 format). See + *Control actions* below. + +**Control actions** (since 1.1): `"action": "stop"` asks the daemon to +shut down cleanly — it prints a notice on its TTY (no approval prompt), +replies `ok`, and exits; `"action": "ping"` replies `ok` with +`"message": "pong"` without prompting. Both are sent with +`"pipeline": []` and pass every gate an exec request does (same-UID +check, field sanitization, freshness, replay protection), so a captured +stop request cannot be replayed later. Compatibility: a pre-1.1 daemon +ignores the unknown `action` key and rejects the empty pipeline at +validation — before any prompt — with `"pipeline must not be empty"`; +since every response carries the daemon's `version`, new clients use +that reply to detect peers that predate control actions. **Response:** ```jsonc diff --git a/packaging/mcpb/build-mcpb.sh b/packaging/mcpb/build-mcpb.sh new file mode 100755 index 0000000..1c7f355 --- /dev/null +++ b/packaging/mcpb/build-mcpb.sh @@ -0,0 +1,46 @@ +#!/usr/bin/env sh +# Build an MCPB bundle (.mcpb) for the sudo-proxy-mcp MCP server. +# +# An .mcpb file is a zip of a manifest.json (which pins `sudo-proxy-mcp` as the +# entry point) plus the binary itself. Unlike the crates.io package, this lets +# MCP clients and directory probes launch the *MCP server* binary directly — +# `cargo install sudo-proxy` installs four binaries and the registry has no way +# to say "run sudo-proxy-mcp, not sudo-proxy". See docs/mcp.md. +# +# Usage: build-mcpb.sh +# version release version, e.g. 1.1.0 (injected into the manifest) +# bin_dir directory containing the built `sudo-proxy-mcp` binary +# out_dir where to write the .mcpb (and its .sha256) +# +# Prints the sha256 to stdout and writes /.mcpb.sha256. +set -eu + +VERSION="${1:?version required}" +BIN_DIR="${2:?bin_dir required}" +OUT_DIR="${3:?out_dir required}" + +HERE=$(CDPATH= cd -- "$(dirname -- "$0")" && pwd) +MANIFEST_SRC="$HERE/manifest.json" +BIN="$BIN_DIR/sudo-proxy-mcp" +NAME="sudo-proxy-mcp-v${VERSION}-x86_64-linux.mcpb" +OUT="$OUT_DIR/$NAME" + +[ -f "$BIN" ] || { echo "error: $BIN not found" >&2; exit 1; } + +mkdir -p "$OUT_DIR" +STAGE=$(mktemp -d) +trap 'rm -rf "$STAGE"' EXIT + +# Canonical manifest content lives in manifest.json; only the version is injected +# so it always tracks the release tag (single source of truth = Cargo.toml/tag). +jq --arg v "$VERSION" '.version = $v' "$MANIFEST_SRC" > "$STAGE/manifest.json" +cp "$BIN" "$STAGE/sudo-proxy-mcp" +chmod +x "$STAGE/sudo-proxy-mcp" + +rm -f "$OUT" +# -X: no extra file attributes/timestamps -> reproducible-ish archive. +( cd "$STAGE" && zip -qX "$OUT" manifest.json sudo-proxy-mcp ) + +SHA=$(sha256sum "$OUT" | awk '{print $1}') +printf '%s' "$SHA" > "$OUT.sha256" +echo "$SHA" diff --git a/packaging/mcpb/manifest.json b/packaging/mcpb/manifest.json new file mode 100644 index 0000000..c593b03 --- /dev/null +++ b/packaging/mcpb/manifest.json @@ -0,0 +1,22 @@ +{ + "manifest_version": "0.3", + "name": "sudo-proxy", + "version": "1.1.0", + "description": "Privileged command execution proxy with human approval via pkexec or sudo", + "author": { + "name": "Tarides" + }, + "homepage": "https://github.com/tarides/sudo-proxy", + "documentation": "https://github.com/tarides/sudo-proxy/blob/main/docs/mcp.md", + "server": { + "type": "binary", + "entry_point": "sudo-proxy-mcp", + "mcp_config": { + "command": "${__dirname}/sudo-proxy-mcp", + "args": [] + } + }, + "compatibility": { + "platforms": ["linux"] + } +} diff --git a/server.json b/server.json index 0756339..bf1e415 100644 --- a/server.json +++ b/server.json @@ -3,7 +3,7 @@ "name": "io.github.tarides/sudo-proxy", "title": "sudo-proxy", "description": "Privileged command execution proxy with human approval via pkexec or sudo", - "version": "1.0.0", + "version": "1.1.0", "repository": { "url": "https://github.com/tarides/sudo-proxy", "source": "github" @@ -13,7 +13,16 @@ "registryType": "cargo", "registryBaseUrl": "https://crates.io", "identifier": "sudo-proxy", - "version": "1.0.0", + "version": "1.1.0", + "transport": { + "type": "stdio" + } + }, + { + "registryType": "mcpb", + "identifier": "https://github.com/tarides/sudo-proxy/releases/download/v1.1.0/sudo-proxy-mcp-v1.1.0-x86_64-linux.mcpb", + "version": "1.1.0", + "fileSha256": "SHA256-INJECTED-BY-PUBLISH-WORKFLOW", "transport": { "type": "stdio" } diff --git a/src/bin/sudo-proxy.rs b/src/bin/sudo-proxy.rs index bcc06be..dfac2b4 100644 --- a/src/bin/sudo-proxy.rs +++ b/src/bin/sudo-proxy.rs @@ -73,7 +73,7 @@ fn main() { let prompter: Arc = Arc::new(TtyPrompter); let sink: Arc = Arc::new(TtyResultSink); - let shutdown = AtomicBool::new(false); + let shutdown = Arc::new(AtomicBool::new(false)); let in_flight = Arc::new(AtomicUsize::new(0)); let tty_lock = Arc::new(Mutex::new(())); @@ -94,25 +94,34 @@ fn main() { ..Default::default() }; - if let Err(e) = server::run( + match server::run( &socket_path, config, prompter, sink, - &shutdown, + Arc::clone(&shutdown), in_flight, tty_lock, ) { - eprintln!("error: {e}"); - // Only remove the socket file if it is ours. AddrInUse means - // another sudo-proxy is already bound there — deleting that - // file would silently break the live daemon's reachability for - // every subsequent client without taking it down, leaving a - // running-but-unreachable process behind. - if e.kind() != std::io::ErrorKind::AddrInUse { + // Clean return: only reachable via a stop request. Remove our + // socket so the next start doesn't find a stale file, then exit 0 + // — the terminal window closes with us, and for a remote daemon + // the command-mode SSH session (and its tunnel) ends too. + Ok(()) => { let _ = std::fs::remove_file(&socket_path); } - process::exit(1); + Err(e) => { + eprintln!("error: {e}"); + // Only remove the socket file if it is ours. AddrInUse means + // another sudo-proxy is already bound there — deleting that + // file would silently break the live daemon's reachability for + // every subsequent client without taking it down, leaving a + // running-but-unreachable process behind. + if e.kind() != std::io::ErrorKind::AddrInUse { + let _ = std::fs::remove_file(&socket_path); + } + process::exit(1); + } } } diff --git a/src/mcp.rs b/src/mcp.rs index a43621d..b828552 100644 --- a/src/mcp.rs +++ b/src/mcp.rs @@ -20,11 +20,14 @@ use tokio::io::{AsyncBufReadExt, AsyncReadExt, AsyncWriteExt, BufReader}; use tokio::net::UnixStream; use crate::hosts::HostsConfig; -use crate::protocol::{self, Request, Response, Status}; +use crate::protocol::{self, Action, Request, Response, Status}; use crate::server::default_socket_path; const DEFAULT_TIMEOUT_MS: u64 = 120_000; const MAX_TIMEOUT_MS: u64 = 600_000; +/// 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); /// Whether SUDO_PROXY_MCP_VERBOSE is set. MCP stdio is JSON-RPC over /// stdout, so the only safe trace sink is stderr. Cached once at first @@ -116,6 +119,20 @@ pub struct UpdateHostParams { pub os: Option, } +#[derive(Debug, serde::Deserialize, schemars::JsonSchema)] +pub struct StopServerParams { + /// Host whose daemon to stop (omit for the local daemon) + #[serde(default)] + pub host: Option, +} + +#[derive(Debug, serde::Deserialize, schemars::JsonSchema)] +pub struct StatusParams { + /// Host to check (omit to report the local daemon plus every known host) + #[serde(default)] + pub host: Option, +} + // --------------------------------------------------------------------------- // McpProxy — the MCP server // --------------------------------------------------------------------------- @@ -140,7 +157,7 @@ impl McpProxy { } #[tool( - description = "Execute a command through sudo-proxy with human approval. The command runs on the target host (local by default). A human must approve privileged commands before they execute. Call start_server first if sudo-proxy is not already running. Supports pipelines: use `pipeline` for multi-stage commands (e.g. [[\"ls\", \"/tmp\"], [\"wc\", \"-l\"]]) or `argv` for a single command." + description = "Execute a command (or multi-stage pipeline) on a sudo-proxy host after a human approves it at that host's terminal. Provide `argv` for a single command or `pipeline` for piped stages (e.g. [[\"ls\", \"/tmp\"], [\"wc\", \"-l\"]]); `host` targets a remote daemon started via start_server (omit for localhost); `timeout` is in milliseconds (default 120000, clamped to 600000). Blocks until the human answers, then returns the final stage's stdout plus per-stage stderr and exit codes. Errors: 'Request denied by user.' if the human declines, a timeout error if unanswered within 60s, and 'sudo-proxy is not running' if the daemon is down — call start_server first." )] async fn execute( &self, @@ -203,7 +220,7 @@ impl McpProxy { } #[tool( - description = "Start a sudo-proxy server. Local (no host): opens a terminal window with sudo-proxy's TUI for command approval. Remote (host given): opens a terminal window with SSH running sudo-proxy, with a socket tunnel so execute calls reach the remote host." + description = "Start a sudo-proxy approval daemon: with no `host`, opens a local terminal window running the approval TUI; with `host`, opens a terminal running SSH to that host with a Unix-socket tunnel so subsequent execute calls reach it. Idempotent: if the daemon (or tunnel) is already live it returns 'already running' without spawning anything. Blocks while polling for end-to-end socket readiness — up to 5s locally, up to 30s for remote tunnels — and returns as soon as the daemon answers. Set `forward_agent: true` to enable SSH agent forwarding for unprivileged remote commands (ignored locally); errors if no terminal emulator is found or the socket is not ready within the polling window." )] async fn start_server( &self, @@ -230,7 +247,7 @@ impl McpProxy { } #[tool( - description = "Update metadata for a known host. Use this to record a host's description or OS after learning it during a session." + description = "Record or update metadata for a host in the sudo-proxy registry (~/.config/sudo-proxy/hosts.json), which is surfaced in this server's instructions and in status output. Partial update: only the `description` and/or `os` fields you provide are changed; omitted fields keep their current values. A `host` not yet in the registry is added automatically. Returns 'Updated host '; errors only if the host name contains characters outside [A-Za-z0-9._@:-]." )] async fn update_host( &self, @@ -253,6 +270,165 @@ impl McpProxy { params.host ))])) } + + #[tool( + description = "Stop a running sudo-proxy daemon: sends a stop request over its socket; the daemon prints a shutdown notice on its terminal (no approval prompt is required), exits, and its terminal window — and SSH tunnel, for remote hosts — closes. `host` selects a remote daemon started via start_server; omit it for the local one. Returns a non-error 'not running' message if no socket exists; on success, confirms after briefly polling for the socket to disappear and removes any stale tunnel socket. If the target runs a sudo-proxy version that predates remote stop, returns an error asking you to press q or Ctrl+C in that daemon's terminal instead." + )] + async fn stop_server( + &self, + Parameters(params): Parameters, + ) -> Result { + if let Some(ref h) = params.host { + if let Err(e) = crate::server::validate_host(h) { + return Ok(error_result(format!("invalid host: {e}"))); + } + } + let socket_path = socket_for_host(params.host.as_deref()); + let host_name = params.host.clone().unwrap_or_else(|| "localhost".into()); + + if !socket_path.exists() { + return Ok(CallToolResult::success(vec![Content::text(format!( + "sudo-proxy is not running on {host_name} (no socket at {})", + socket_path.display() + ))])); + } + + let req = Request::control( + params.host.unwrap_or_default(), + "sudo-proxy-mcp".to_string(), + Action::Stop, + ); + match send_request(&socket_path, &req, CONTROL_TIMEOUT).await { + Ok(resp) if resp.status == Status::Ok => { + // Wait for the daemon (and, remotely, the ssh tunnel) to go + // away so a follow-up start_server doesn't race the old + // socket. Best-effort: report success either way. + for _ in 0..50 { + if !socket_path.exists() || !remote_socket_ready(&socket_path).await { + break; + } + tokio::time::sleep(Duration::from_millis(100)).await; + } + // ssh may leave the local end of the -L forward behind. + if socket_path.exists() && !remote_socket_ready(&socket_path).await { + let _ = std::fs::remove_file(&socket_path); + } + Ok(CallToolResult::success(vec![Content::text(format!( + "Stopped sudo-proxy on {host_name}." + ))])) + } + Ok(resp) if is_pre_action_daemon(&resp) => { + touch_host(&host_name, &resp.version); + let ver = if resp.version.is_empty() { "unknown" } else { &resp.version }; + Ok(error_result(format!( + "{host_name} runs sudo-proxy {ver}, which predates remote stop. \ + Press q or Ctrl+C in that daemon's terminal window to stop it." + ))) + } + Ok(resp) => Ok(format_response(resp)), + Err(e) => Ok(error_result(e)), + } + } + + #[tool( + description = "Report the status of sudo-proxy daemons without executing any command: with `host`, checks that one daemon; with no arguments, checks the local daemon plus every host in the registry. For each host it reports whether the socket exists and answers, the live daemon version (learned via a ping that needs no human approval), and registry metadata (description, OS, last connected). Read-only except for refreshing the registry's last-connected/version cache after a successful ping. Hosts that are down are reported as 'not running' — that is a normal result, not an error." + )] + async fn status( + &self, + Parameters(params): Parameters, + ) -> Result { + let config = HostsConfig::load(); + // The registry key for the local daemon is "localhost" (written by + // touch_host); it must be probed at the default socket, never at a + // tunnel path. + let targets: Vec> = match params.host { + Some(h) => { + if let Err(e) = crate::server::validate_host(&h) { + return Ok(error_result(format!("invalid host: {e}"))); + } + vec![if h == "localhost" { None } else { Some(h) }] + } + None => std::iter::once(None) + .chain( + config + .hosts + .keys() + .filter(|h| h.as_str() != "localhost") + .cloned() + .map(Some), + ) + .collect(), + }; + + let mut lines = Vec::new(); + for target in targets { + let name = target.clone().unwrap_or_else(|| "localhost".into()); + let mut line = format!("{name}: {}", probe_one(target.as_deref()).await); + if let Some(info) = config.hosts.get(&name) { + if !info.description.is_empty() { + line.push_str(&format!(" — {}", info.description)); + } + if !info.os.is_empty() { + line.push_str(&format!(" ({})", info.os)); + } + if !info.last_connected.is_empty() { + line.push_str(&format!(" [last: {}]", info.last_connected)); + } + } + lines.push(line); + } + Ok(CallToolResult::success(vec![Content::text( + lines.join("\n"), + )])) + } +} + +/// An old daemon (predating control actions) rejects a stop/ping request at +/// validation with this message — before any prompt — and its error response +/// still carries its version. +fn is_pre_action_daemon(resp: &Response) -> bool { + resp.status == Status::Error + && resp + .message + .as_deref() + .is_some_and(|m| m.contains("pipeline must not be empty")) +} + +/// One-host status probe: socket presence, readiness, then a live ping for +/// the daemon's version. The only mutation is the hosts.json cache refresh +/// after a successful exchange. +async fn probe_one(host: Option<&str>) -> String { + let sock = socket_for_host(host); + let name = host.unwrap_or("localhost"); + if !sock.exists() { + return "not running (no socket)".into(); + } + if !remote_socket_ready(&sock).await { + return "socket present but not answering (stale?)".into(); + } + let req = Request::control( + host.unwrap_or_default().to_string(), + "sudo-proxy-mcp".to_string(), + Action::Ping, + ); + match send_request(&sock, &req, CONTROL_TIMEOUT).await { + Ok(resp) if resp.status == Status::Ok => { + touch_host(name, &resp.version); + let ver = if resp.version.is_empty() { "unknown" } else { &resp.version }; + format!("running (sudo-proxy {ver})") + } + Ok(resp) if is_pre_action_daemon(&resp) => { + // The version is still learned from the error reply. + touch_host(name, &resp.version); + let ver = if resp.version.is_empty() { "unknown" } else { &resp.version }; + format!("running (sudo-proxy {ver}, predates ping)") + } + Ok(resp) => format!( + "error: {}", + resp.message.unwrap_or_else(|| "unknown error".into()) + ), + Err(e) => format!("unreachable: {e}"), + } } /// Render the MCP server's instructions block. Pure function over a diff --git a/src/protocol.rs b/src/protocol.rs index c62b7a4..e8c6bdc 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -8,6 +8,30 @@ use std::ops::Deref; /// the same crate, so this resolves to the same value everywhere. pub const VERSION: &str = env!("CARGO_PKG_VERSION"); +/// What the sender wants the daemon to do. Absent on the wire for old +/// clients, so it defaults to `Exec`; exec requests from new clients skip +/// the field entirely, keeping them byte-identical to the pre-1.1 format. +/// +/// Control actions (`Stop`, `Ping`) are sent with an empty `pipeline`. An +/// old daemon (< 1.1) ignores the unknown `action` key, rejects the empty +/// pipeline at `ValidatedRequest::validate` without prompting the human, +/// and its error `Response` still carries its `version` — which is how new +/// clients detect a peer that predates control actions. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum Action { + #[default] + Exec, + Stop, + Ping, +} + +impl Action { + fn is_exec(&self) -> bool { + matches!(self, Action::Exec) + } +} + #[derive(Clone, Debug, Deserialize, Serialize)] pub struct Request { #[serde(default = "default_id")] @@ -18,7 +42,10 @@ pub struct Request { pub session: String, #[serde(default)] pub time: String, + #[serde(default)] pub pipeline: Vec>, + #[serde(default, skip_serializing_if = "Action::is_exec")] + pub action: Action, #[serde(default)] pub env: HashMap, #[serde(default)] @@ -64,6 +91,7 @@ impl Request { session, time: crate::datetime::now_iso8601(), pipeline, + action: Action::Exec, env, reason, privileged, @@ -71,6 +99,26 @@ impl Request { version: VERSION.to_string(), } } + + /// Build a control request (`Stop` / `Ping`). The empty pipeline is + /// deliberate: it is what makes an old daemon reject the request at + /// validation — before any prompt — while still stamping its version + /// on the error response. + pub fn control(host: String, session: String, action: Action) -> Self { + Self { + id: default_id(), + host, + session, + time: crate::datetime::now_iso8601(), + pipeline: vec![], + action, + env: HashMap::new(), + reason: String::new(), + privileged: false, + forward_agent: false, + version: VERSION.to_string(), + } + } } fn default_session() -> String { @@ -122,7 +170,10 @@ impl ValidatedRequest { /// client-side `validate_host`; all are held to the same sanitization as /// argv and env. See `tui::prompt_tty` for the render this protects. pub fn validate(req: Request) -> Result { - if req.pipeline.is_empty() { + // 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() { return Err("pipeline must not be empty".to_string()); } for (stage_idx, argv) in req.pipeline.iter().enumerate() { @@ -282,3 +333,73 @@ impl Response { self.stages.last().map(|s| s.exit_code).unwrap_or(0) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn exec_request() -> Request { + Request::new( + String::new(), + "test".into(), + vec![vec!["ls".into()]], + HashMap::new(), + String::new(), + true, + false, + ) + } + + /// Wire compat: a request from an old client (no `action` key) must + /// deserialize to `Action::Exec`. + #[test] + fn action_absent_defaults_to_exec() { + let json = r#"{"id":"a","session":"s","time":"2026-01-01T00:00:00Z","pipeline":[["ls"]]}"#; + let req: Request = serde_json::from_str(json).unwrap(); + assert_eq!(req.action, Action::Exec); + } + + /// Wire compat the other way: an exec request from a new client must be + /// byte-identical to the pre-1.1 format — no `action` key on the wire. + #[test] + fn exec_request_omits_action_on_wire() { + let json = serde_json::to_string(&exec_request()).unwrap(); + assert!(!json.contains("action"), "exec must not serialize `action`: {json}"); + } + + /// A control request must serialize an explicit empty pipeline (so old + /// daemons parse it and reject at validate, version-stamping the error) + /// and must round-trip its action. + #[test] + fn control_request_serializes_empty_pipeline_and_action() { + let req = Request::control(String::new(), "test".into(), Action::Stop); + let json = serde_json::to_string(&req).unwrap(); + assert!(json.contains(r#""pipeline":[]"#), "missing empty pipeline: {json}"); + assert!(json.contains(r#""action":"stop""#), "missing action: {json}"); + let back: Request = serde_json::from_str(&json).unwrap(); + assert_eq!(back.action, Action::Stop); + } + + #[test] + fn validate_allows_empty_pipeline_for_control_only() { + for action in [Action::Stop, Action::Ping] { + let req = Request::control(String::new(), "test".into(), action); + assert!(ValidatedRequest::validate(req).is_ok(), "{action:?} must validate"); + } + let mut req = Request::control(String::new(), "test".into(), Action::Exec); + req.pipeline = vec![]; + assert_eq!( + ValidatedRequest::validate(req).unwrap_err(), + "pipeline must not be empty" + ); + } + + /// Control requests go through the same dangerous-character sanitization + /// as exec requests — every displayed field is still checked. + #[test] + fn validate_still_sanitizes_control_fields() { + let mut req = Request::control(String::new(), "test".into(), Action::Ping); + req.session.push('\u{1b}'); + assert!(ValidatedRequest::validate(req).is_err()); + } +} diff --git a/src/server.rs b/src/server.rs index 5ec7516..44a855d 100644 --- a/src/server.rs +++ b/src/server.rs @@ -326,7 +326,7 @@ pub fn run( config: ServerConfig, prompter: Arc, result_sink: Arc, - shutdown: &AtomicBool, + shutdown: Arc, in_flight: Arc, tty_lock: Arc>, ) -> std::io::Result<()> { @@ -398,6 +398,7 @@ pub fn run( let pkexec_only = config.pkexec_only; let verbose = config.verbose; let confirm_unprivileged = Arc::clone(&confirm_unprivileged); + let shutdown = Arc::clone(&shutdown); thread::spawn(move || { let _guard = guard; handle_connection( @@ -411,6 +412,7 @@ pub fn run( sink, seen, tty, + shutdown, ); }); } @@ -428,6 +430,7 @@ fn handle_connection( result_sink: Arc, seen_ids: Arc>, tty_lock: Arc>, + shutdown: Arc, ) { // Defense-in-depth: refuse connections from other UIDs even though the // socket file is 0600 inside a 0700 runtime dir. @@ -532,6 +535,41 @@ fn handle_connection( } } + // Control actions, dispatched only after every gate above (peer uid, + // validation, freshness, replay) — a captured stop request cannot be + // replayed later to take the daemon down. + match req.action { + crate::protocol::Action::Ping => { + if verbose { + let client_ver = if req.version.is_empty() { "unknown" } else { req.version.as_str() }; + eprintln!("[{}] ping from {} (client {client_ver})", req.id, req.session); + } + let mut resp = Response::ok(&req.id, vec![], b""); + resp.message = Some("pong".to_string()); + let _ = write_response(&mut stream, &resp); + return; + } + crate::protocol::Action::Stop => { + // No approval prompt (by design): a same-UID peer can already + // SIGTERM the daemon, so the gate would add friction without + // adding a security boundary. The TTY notice keeps the human + // informed; try_lock so a pending prompt can't delay shutdown. + if let Ok(_g) = tty_lock.try_lock() { + let _ = tui::display_notice(&format!( + "sudo-proxy stopped by {}", + req.session + )); + } + // Reply before flipping the flag so the client reliably reads + // the Ok response before the process exits. + let resp = Response::ok(&req.id, vec![], b""); + let _ = write_response(&mut stream, &resp); + shutdown.store(true, Ordering::Relaxed); + return; + } + crate::protocol::Action::Exec => {} + } + if verbose { let priv_label = if req.privileged { "privileged" } else { "unprivileged" }; let pipeline_display = crate::tui::pipeline_join(&req.pipeline); @@ -1034,6 +1072,7 @@ mod tests { session: rand_chars(rng, 6), time: crate::datetime::now_iso8601(), pipeline: std::mem::take(&mut pipeline), + action: crate::protocol::Action::Exec, env, reason: rand_chars(rng, 8), privileged: rng.next_u64() & 1 == 0, diff --git a/src/tui.rs b/src/tui.rs index 14ea109..129806e 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -299,6 +299,17 @@ pub fn display_banner(req: &ValidatedRequest) -> io::Result<()> { Ok(()) } +/// Print a one-line dim notice on /dev/tty (e.g. "stopped by "). +/// Best-effort: silently returns Ok if /dev/tty cannot be opened. +pub fn display_notice(msg: &str) -> io::Result<()> { + let mut tty = match OpenOptions::new().write(true).open("/dev/tty") { + Ok(f) => f, + Err(_) => return Ok(()), + }; + let Style { dim, reset, .. } = style(); + writeln!(tty, "{dim}\u{25a0}{reset} {msg}") +} + /// Display the command result on /dev/tty. Truncate stdout/stderr to 3 lines. pub fn display_result(resp: &Response) -> io::Result<()> { let mut tty = OpenOptions::new().write(true).open("/dev/tty")?; diff --git a/tests/common/mod.rs b/tests/common/mod.rs index c5ec5ee..33e1e5f 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -157,7 +157,7 @@ pub fn start_test_server(opts: TestServerOpts) -> TestServer { config, prompter_arc, sink_arc, - &sh_arc, + sh_arc, inflight_arc, tty_lock_arc, ); @@ -251,6 +251,7 @@ pub fn make_req(id: &str, pipeline: Vec>) -> Request { .into_iter() .map(|v| v.into_iter().map(String::from).collect()) .collect(), + action: sudo_proxy::protocol::Action::Exec, env: Default::default(), reason: String::new(), privileged: false, @@ -259,6 +260,13 @@ pub fn make_req(id: &str, pipeline: Vec>) -> Request { } } +#[allow(dead_code)] +pub fn make_control_req(id: &str, action: sudo_proxy::protocol::Action) -> Request { + let mut req = make_req(id, vec![]); + req.action = action; + req +} + pub fn iso_now() -> String { let secs = SystemTime::now() .duration_since(UNIX_EPOCH) diff --git a/tests/exec.rs b/tests/exec.rs index 154fdfc..2bcd3a5 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(), + action: sudo_proxy::protocol::Action::Exec, env: HashMap::new(), reason: String::new(), privileged: false, diff --git a/tests/mcp_introspection.rs b/tests/mcp_introspection.rs index ad7ab51..14fddbd 100644 --- a/tests/mcp_introspection.rs +++ b/tests/mcp_introspection.rs @@ -89,7 +89,7 @@ fn mcp_server_enumerates_its_tools() { names.sort_unstable(); assert_eq!( names, - ["execute", "start_server", "update_host"], + ["execute", "start_server", "status", "stop_server", "update_host"], "unexpected MCP tool set — this is exactly what Glama enumerates to score the server", ); diff --git a/tests/stop.rs b/tests/stop.rs new file mode 100644 index 0000000..2721b24 --- /dev/null +++ b/tests/stop.rs @@ -0,0 +1,117 @@ +#![cfg(unix)] + +//! Control actions (`stop` / `ping`): a stop request shuts the daemon down +//! cleanly without a prompt; a ping answers without a prompt and without +//! disturbing normal service; both stay behind the freshness gate. + +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +use std::sync::{Arc, Mutex}; +use std::thread; +use std::time::{Duration, Instant}; + +use sudo_proxy::protocol::{Action, Status}; +use sudo_proxy::server; + +mod common; +use common::{ + iso_offset, make_control_req, make_req, send_request, start_test_server, ScriptedPrompter, + RecordingSink, TestServerOpts, +}; + +/// A stop request gets an Ok response, never reaches the prompter, and makes +/// `server::run` return cleanly. +#[test] +fn stop_request_shuts_down_run_without_prompt() { + let tempdir = tempfile::tempdir_in("/tmp").expect("tempdir"); + let path = tempdir.path().join("stop.sock"); + + let prompter = Arc::new(ScriptedPrompter::new()); + let sink = Arc::new(RecordingSink::new()); + let shutdown = Arc::new(AtomicBool::new(false)); + let in_flight = Arc::new(AtomicUsize::new(0)); + let tty_lock = Arc::new(Mutex::new(())); + + let p = Arc::clone(&prompter); + let s = Arc::clone(&sink); + let sh = Arc::clone(&shutdown); + let inf = Arc::clone(&in_flight); + let tty = Arc::clone(&tty_lock); + let path_thread = path.clone(); + let handle = thread::spawn(move || { + server::run( + &path_thread, + server::ServerConfig::default(), + p, + s, + sh, + inf, + tty, + ) + }); + + // Wait for the listener. + let deadline = Instant::now() + Duration::from_secs(2); + while Instant::now() < deadline { + if std::os::unix::net::UnixStream::connect(&path).is_ok() { + break; + } + thread::sleep(Duration::from_millis(10)); + } + + let resp = send_request(&path, &make_control_req("stop-1", Action::Stop)); + assert_eq!(resp.status, Status::Ok, "stop must be acknowledged: {resp:?}"); + + // The accept loop polls the flag every 50ms; run must return Ok soon. + let start = Instant::now(); + let result = handle.join().expect("run thread must not panic"); + assert!(result.is_ok(), "run must return cleanly after stop: {result:?}"); + assert!( + start.elapsed() < Duration::from_secs(2), + "run took too long to notice the stop flag" + ); + assert_eq!( + prompter.call_count(), + 0, + "stop must not go through the approval prompt" + ); + assert!(shutdown.load(Ordering::Relaxed), "stop must set the flag"); +} + +/// A ping answers Ok("pong") without a prompt, and the daemon keeps serving +/// exec requests afterwards. +#[test] +fn ping_answers_without_prompt_and_daemon_survives() { + let s = start_test_server(TestServerOpts::default()); + + let resp = s.send(&make_control_req("ping-1", Action::Ping)); + assert_eq!(resp.status, Status::Ok, "ping must succeed: {resp:?}"); + assert_eq!(resp.message.as_deref(), Some("pong")); + assert_eq!( + resp.version, + sudo_proxy::protocol::VERSION, + "ping response must carry the daemon version" + ); + assert_eq!(s.prompter.call_count(), 0, "ping must not prompt"); + + let resp = s.send(&make_req("ping-then-exec", vec![vec!["true"]])); + assert_eq!(resp.status, Status::Ok, "daemon must survive a ping"); +} + +/// Control actions sit behind the freshness gate: a stale stop request is +/// rejected and the daemon keeps running. +#[test] +fn stale_stop_is_rejected_and_daemon_survives() { + let s = start_test_server(TestServerOpts::default()); + + let mut req = make_control_req("stale-stop", Action::Stop); + req.time = iso_offset(-300); + let resp = s.send(&req); + assert_eq!(resp.status, Status::Error); + assert!( + resp.message.as_deref().unwrap_or("").contains("too old"), + "expected freshness rejection, got: {resp:?}" + ); + + let resp = s.send(&make_req("still-alive", vec![vec!["true"]])); + assert_eq!(resp.status, Status::Ok, "daemon must survive a stale stop"); +} diff --git a/tests/transport.rs b/tests/transport.rs index 20c28ed..9e49f51 100644 --- a/tests/transport.rs +++ b/tests/transport.rs @@ -45,7 +45,7 @@ fn refuses_to_clobber_active_server() { let path = s.socket_path.clone(); let prompter = Arc::new(common::ScriptedPrompter::new()); let sink = Arc::new(NoopResultSink); - let shutdown = AtomicBool::new(false); + let shutdown = Arc::new(AtomicBool::new(false)); let in_flight = Arc::new(AtomicUsize::new(0)); let tty_lock = Arc::new(Mutex::new(())); @@ -54,7 +54,7 @@ fn refuses_to_clobber_active_server() { server::ServerConfig::default(), prompter, sink, - &shutdown, + shutdown, in_flight, tty_lock, ); @@ -104,7 +104,7 @@ fn replaces_stale_socket_file() { config, p_for_thread, Arc::new(NoopResultSink), - &s_for_thread, + s_for_thread, inflight_thread, tty_lock_thread, )