From 03c2173391c5866d54f3328c97e7405c33b33e8f Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 5 Sep 2026 13:37:26 +0200 Subject: [PATCH 1/2] Preserve argv through systemd scope wrappers agent-identity: mbp2025.direct.omp.5tek2r48 agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.11 agent-runtime: OMP 18.0.11 tooling-profile: dotfiles@4cc7b25 --- ...-wrappers-disable-environment-expansion.md | 69 +++++++++++ ...6-09-05-systemd-scope-argv-transparency.md | 54 +++++++++ docs/vrs/ontology.md | 13 ++ docs/vrs/requirements.md | 6 + docs/vrs/spec.md | 22 ++++ src/isolate.rs | 111 ++++++++++++------ 6 files changed, 238 insertions(+), 37 deletions(-) create mode 100644 docs/vrs/.decisions/0016-systemd-scope-wrappers-disable-environment-expansion.md create mode 100644 docs/vrs/.experiments/2026-09-05-systemd-scope-argv-transparency.md diff --git a/docs/vrs/.decisions/0016-systemd-scope-wrappers-disable-environment-expansion.md b/docs/vrs/.decisions/0016-systemd-scope-wrappers-disable-environment-expansion.md new file mode 100644 index 00000000..41740c7c --- /dev/null +++ b/docs/vrs/.decisions/0016-systemd-scope-wrappers-disable-environment-expansion.md @@ -0,0 +1,69 @@ +# Systemd scope wrappers disable environment expansion + +Status: accepted + +Accepted on 2026-09-05 for R40 launch argv transparency. + +## Context + +st2 adds `systemd-run` around every PTY and exec launch when Linux systemd +scope isolation is available. Rust passes the task program and arguments to +that outer process as opaque OS strings, but `systemd-run` performs its own +command-line environment expansion by default. A Linux systemd 260 experiment +passed the literal `$HOME:${UNSET}:$$` through the wrapper and observed `$$` +changed to `$`. The wrapper therefore did not preserve launch argv even though +st2 itself never invoked a shell or edited the argument. + +The scope wrapper is also the mechanism behind control-plane replacement +safety. Any correction must retain scope creation, collection, inherited I/O, +PTY behavior, exit status, and outliving-child membership. + +## Options + +| Option | Tradeoffs | +| --- | --- | +| Disable systemd expansion before the separator — selected | Preserves the original OS strings without changing scope mechanics. | +| Keep systemd's default expansion | Rejected because the live probe changed caller-owned argument data. | +| Rewrite dollar signs into systemd escape sequences | Rejected because st2 would have to interpret every OS string and maintain a wrapper-specific transform. | +| Shell-quote the inner command | Rejected because there is no shell boundary; quote bytes would become task argument bytes, while adding a shell would introduce evaluation. | +| Drop systemd scope isolation for affected commands | Rejected because it would trade argument corruption for violation of R11 control-plane replacement safety. | + +## Evidence and Argument + +The Linux systemd 260 experiment distinguished the wrapper's default from the +selected option with one literal input. Default expansion changed the terminal +`$$` in `$HOME:${UNSET}:$$` to `$`; `--expand-environment=no` preserved the +complete input. With expansion disabled, non-TTY argv and standard descriptors, +real-PTY terminal status, exit propagation, outliving-child scope membership, +collection, exact-name reuse, and cleanup all retained their prior behavior. + +The option is the narrowest boundary fix: systemd owns the unwanted +interpretation, and systemd exposes a switch that removes it. Rewriting task +arguments inside st2 would replace downstream interpretation with an +st2-maintained encoding and would no longer be opaque launch argv. + +## Decision + +Linux scope launches use this exact outer argument order: + +```text +systemd-run --user --scope --collect --quiet --unit= --expand-environment=no -- ... +``` + +`--expand-environment=no` is passed as a `systemd-run` option before the `--` +separator. st2 appends the program and each argument after the separator as the +original OS strings. It does not quote, escape, expand, or render those values +through a shell. Detached and degraded-detached modes remain direct +pass-throughs. + +## Consequences + +- Dollar-bearing literals, including `$HOME`, `${UNSET}`, and `$$`, reach PTY + and exec tasks byte-for-byte in scope mode. +- The systemd option sequence is part of the tested wrapper contract; the only + addition to the prior shape is the expansion-disable option before the + separator. +- macOS and Linux hosts without usable user scopes keep the existing direct + program-and-argv path. +- Scope lifetime and I/O behavior are unchanged. The supporting live evidence + is recorded in the [systemd scope argv experiment](../.experiments/2026-09-05-systemd-scope-argv-transparency.md). diff --git a/docs/vrs/.experiments/2026-09-05-systemd-scope-argv-transparency.md b/docs/vrs/.experiments/2026-09-05-systemd-scope-argv-transparency.md new file mode 100644 index 00000000..8000eac9 --- /dev/null +++ b/docs/vrs/.experiments/2026-09-05-systemd-scope-argv-transparency.md @@ -0,0 +1,54 @@ +# Systemd scope argv transparency + +Date: 2026-09-05 +Fixture: Linux with a functioning systemd 260 user manager + +## Question + +Does `systemd-run --user --scope` preserve dollar-bearing task arguments by +default, and does `--expand-environment=no` correct that boundary without +changing the scope, I/O, PTY, exit-status, collection, or name-reuse behavior +on which st2 already depends? + +## Method + +A disposable helper was launched through the same ordered scope options used by +st2. It received the single literal argument `$HOME:${UNSET}:$$` and reported +the value it actually received. The probe ran once with systemd's default +command-line expansion and once with `--expand-environment=no` before the `--` +separator. + +The expansion-disabled form was then exercised through two transport fixtures: +a non-TTY process with distinct argv, stdin, stdout, stderr, and exit-status +probes, and a real PTY process that reported terminal status for all three +standard descriptors. A final lifetime probe left a child alive after the +wrapper exited, observed scope membership and collection, reused the same unit +name after collection, and checked fixture cleanup. + +## Result + +| Probe | Observation | +| --- | --- | +| Default `systemd-run` expansion | The received value differed from the supplied literal: the terminal `$$` became `$`. | +| `--expand-environment=no` before `--` | The helper received the complete literal `$HOME:${UNSET}:$$` unchanged. | +| Non-TTY transport | argv, stdin, stdout, and stderr probes were preserved; the wrapper returned the task's exit status 37. | +| Real PTY transport | stdin, stdout, and stderr remained TTYs; the wrapper returned the task's exit status 23. | +| Outliving child | The child remained in the same active scope after wrapper exit. `--collect` unloaded the scope only after the child exited. | +| Unit reuse and cleanup | Exact-name reuse succeeded after collection. No matching process, unit, or temporary fixture path remained. | + +## Conclusion + +Systemd's default command-line expansion is not transparent to caller-owned +argv. Passing `--expand-environment=no` before the command separator is the +narrow correction: it preserves the complete dollar-bearing literal while all +measured scope lifecycle, descriptor, PTY, exit-status, collection, and reuse +semantics remain unchanged. This supports [R40 launch argv transparency](../requirements.md) +and [decision 0016](../.decisions/0016-systemd-scope-wrappers-disable-environment-expansion.md). + +## VRS Impact + +- `requirements.md` adds R40 launch argv transparency. +- `ontology.md` defines **launch argv** as the canonical task-wide term. +- `spec.md` fixes the exact systemd scope wrapper order and its deterministic + scope/pass-through tests. +- Decision 0016 records the selected systemd option and rejected alternatives. \ No newline at end of file diff --git a/docs/vrs/ontology.md b/docs/vrs/ontology.md index a7f48d0f..6080f0b7 100644 --- a/docs/vrs/ontology.md +++ b/docs/vrs/ontology.md @@ -38,6 +38,19 @@ A terminal-backed or terminal-free unit declared for an agent. Authority: [Agent Spec task contract](https://github.com/compoundingtech/evals/blob/main/AGENT-SPEC.md#compact-and-explicit-tasks); [`Task`](../../crates/agent-spec/src/spec.rs#L121-L148) +### launch argv + +The ordered, opaque OS-string sequence comprising a task program and each of +its arguments at the launcher boundary. A platform wrapper may prepend its own +outer arguments, but it does not parse, expand, escape, or otherwise rewrite +the launch argv. This is not a shell command line. Use *provider argv* only +when referring specifically to the canonical agent provider; *launch argv* +applies to every PTY and exec task. + +Authority: [R06 restartable launch definitions and R40 launch argv +transparency](requirements.md); +[host-local scheduling and supervision](spec.md#host-local-scheduling-and-supervision) + ### agent ID The explicit catalog-global immutable identifier of one logical agent subject. diff --git a/docs/vrs/requirements.md b/docs/vrs/requirements.md index bb587ede..402384e1 100644 --- a/docs/vrs/requirements.md +++ b/docs/vrs/requirements.md @@ -126,6 +126,12 @@ accepted. - **R06 Restartable launch definitions:** A restarted PTY or exec receives the complete effective launch definition, including environment and supported launch fields. +- **R40 Launch argv transparency:** Every st2-added launch wrapper preserves the + effective task program and each argument as ordered, opaque OS strings. A + Linux systemd scope launch disables systemd command-line environment + expansion before the wrapper separator, so dollar-bearing literals including + `$HOME`, `${UNSET}`, and `$$` reach the task byte-for-byte. Detached and + degraded-detached launches remain exact pass-throughs. - **R07 Verified hooks:** Required hook content is installed explicitly and verified before a rendered agent depends on it. The selected receipt carries the binary's real source identity regardless of build system. Ordered diff --git a/docs/vrs/spec.md b/docs/vrs/spec.md index 22bd86cb..b08c6c90 100644 --- a/docs/vrs/spec.md +++ b/docs/vrs/spec.md @@ -771,6 +771,28 @@ validate ──► materialize ──► host-local st2 scheduler/reconciler manual PTY restart under a different ambient environment reconstructs the same effective color policy. Adoption of an already-live task remains non-mutating: this policy is applied only when st2 creates a generation. +- **R40:** [Launch argv](ontology.md#launch-argv) is opaque at every + st2-added wrapper boundary. In Linux systemd scope mode the exact outer + command order is: + + ```text + systemd-run --user --scope --collect --quiet --unit= --expand-environment=no -- ... + ``` + + `--expand-environment=no` is an outer `systemd-run` option immediately before + the `--` separator. `` and every `` are appended as their + original OS strings, without shell rendering, dollar escaping, or + environment substitution. Detached and degraded-detached modes remain + ` ...` pass-throughs with no outer command. + + `src/isolate.rs::tests::wrap_scope_disables_expansion_and_preserves_dollar_bearing_argv` + fixes the complete wrapper order and proves literal `$HOME`, `${UNSET}`, and + `$$` after the separator. + `src/isolate.rs::tests::wrap_detached_modes_preserve_exact_program_and_argv` + proves both pass-through modes. The live-system distinction and unaffected + scope semantics are recorded in the + [systemd scope argv experiment](.experiments/2026-09-05-systemd-scope-argv-transparency.md) + and [decision 0016](.decisions/0016-systemd-scope-wrappers-disable-environment-expansion.md). - **R07:** Hook bundles are explicit, content-addressed, installed separately, and verified before materialization references them. Their receipts use the same resolved build identity as the binary's version surfaces for both diff --git a/src/isolate.rs b/src/isolate.rs index f8c907ec..a7d24b78 100644 --- a/src/isolate.rs +++ b/src/isolate.rs @@ -8,12 +8,13 @@ //! The fix: spawn each task into its own OS supervision domain, independent of BOTH the spawner and //! the transport daemon — one goal, per-OS mechanism. //! -//! - **Linux**: `systemd-run --user --scope --unit= --collect --quiet -- `. The task runs -//! in its own transient scope = its own cgroup, registered with the user manager as a **sibling** of -//! the transport unit (a scope created inside a service lands at `app.slice/`, not nested -//! under the service). A cascade kill of the transport unit's cgroup cannot -//! reach a sibling. `--scope` (not `--service`) keeps st2 the logical supervisor — systemd provides -//! only the cgroup; adoption/teardown/restart stay st2's. `--collect` GCs the scope once it empties. +//! - **Linux**: `systemd-run --user --scope --collect --quiet --unit=` +//! `--expand-environment=no -- `. The task runs in its own transient scope = its own cgroup, +//! registered with the user manager as a **sibling** of the transport unit (a scope created inside +//! a service lands at `app.slice/`, not nested under the service). A cascade kill of the +//! transport unit's cgroup cannot reach a sibling. `--scope` (not `--service`) keeps st2 the logical +//! supervisor — systemd provides only the cgroup; adoption/teardown/restart stay st2's. `--collect` +//! GCs the scope once it empties. //! - **macOS / non-systemd Linux**: `setsid` + reparent to init/launchd is the whole defense — there //! are no cgroups, and launchd does not cascade-kill detached children. Here [`wrap`] is a no-op //! pass-through; the caller's existing `setsid` (exec) or the `pty` daemon (pty) provides detachment. @@ -25,8 +26,8 @@ use std::ffi::OsStr; use std::process::{Command, Stdio}; -use std::sync::OnceLock; use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::OnceLock; /// How a task is isolated from its spawner and the transport daemon. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -110,17 +111,23 @@ pub fn scope_unit(task_id: &str) -> String { /// Build the OUTER launch [`Command`] for the inner `program` + `args`, isolated under `unit`. /// -/// In [`Isolation::Scope`] this is `systemd-run --user --scope --unit= --collect --quiet -- -/// `; otherwise it is ` ` verbatim. Either way the caller applies -/// env / cwd / stdio / `pre_exec` to the returned Command and they reach the task — for `--scope`, -/// scope mode runs the command in the caller's context, so cwd, environment, and stdio fds all -/// inherit (verified). +/// In [`Isolation::Scope`] this is `systemd-run --user --scope --collect --quiet --unit= +/// --expand-environment=no -- `; otherwise it is ` ` verbatim. +/// Disabling systemd's environment expansion keeps every inner argv element opaque, including +/// dollar-bearing literals. Either way the caller applies env / cwd / stdio / `pre_exec` to the +/// returned Command and they reach the task — for `--scope`, scope mode runs the command in the +/// caller's context, so cwd, environment, and stdio fds all inherit (verified). pub fn wrap(unit: &str, program: &OsStr, args: &[&OsStr]) -> Command { - match mode() { + wrap_for_mode(mode(), unit, program, args) +} + +fn wrap_for_mode(isolation: Isolation, unit: &str, program: &OsStr, args: &[&OsStr]) -> Command { + match isolation { Isolation::Scope => { let mut c = Command::new("systemd-run"); c.args(["--user", "--scope", "--collect", "--quiet"]) .arg(format!("--unit={unit}")) + .arg("--expand-environment=no") .arg("--") .arg(program) .args(args); @@ -154,32 +161,62 @@ mod tests { } #[test] - fn wrap_scope_prefixes_systemd_run_but_passthrough_does_not() { - // We can't force `mode()` per-test (it's process-cached), so assert the shape that matches the - // detected mode: on a systemd Linux CI box it's Scope; otherwise pass-through. - let cmd = wrap( + fn wrap_scope_disables_expansion_and_preserves_dollar_bearing_argv() { + let cmd = wrap_for_mode( + Isolation::Scope, "st2-x.scope", - OsStr::new("sh"), - &[OsStr::new("-c"), OsStr::new("true")], + OsStr::new("provider"), + &[ + OsStr::new("$HOME"), + OsStr::new("${UNSET}"), + OsStr::new("$$"), + ], ); - let args: Vec = cmd - .get_args() - .map(|a| a.to_string_lossy().into_owned()) - .collect(); - match mode() { - Isolation::Scope => { - assert_eq!(cmd.get_program(), OsStr::new("systemd-run")); - assert!(args.contains(&"--scope".to_string())); - assert!(args.contains(&"--unit=st2-x.scope".to_string())); - assert!(args.contains(&"--collect".to_string())); - // The inner command follows the `--` separator, verbatim. - let sep = args.iter().position(|a| a == "--").unwrap(); - assert_eq!(&args[sep + 1..], &["sh", "-c", "true"]); - } - Isolation::Detached | Isolation::DegradedDetached => { - assert_eq!(cmd.get_program(), OsStr::new("sh")); - assert_eq!(args, vec!["-c", "true"]); - } + let args: Vec<&OsStr> = cmd.get_args().collect(); + + assert_eq!(cmd.get_program(), OsStr::new("systemd-run")); + assert_eq!( + args, + vec![ + OsStr::new("--user"), + OsStr::new("--scope"), + OsStr::new("--collect"), + OsStr::new("--quiet"), + OsStr::new("--unit=st2-x.scope"), + OsStr::new("--expand-environment=no"), + OsStr::new("--"), + OsStr::new("provider"), + OsStr::new("$HOME"), + OsStr::new("${UNSET}"), + OsStr::new("$$"), + ] + ); + } + + #[test] + fn wrap_detached_modes_preserve_exact_program_and_argv() { + for isolation in [Isolation::Detached, Isolation::DegradedDetached] { + let cmd = wrap_for_mode( + isolation, + "unused.scope", + OsStr::new("provider"), + &[ + OsStr::new("$HOME"), + OsStr::new("${UNSET}"), + OsStr::new("$$"), + ], + ); + let args: Vec<&OsStr> = cmd.get_args().collect(); + + assert_eq!(cmd.get_program(), OsStr::new("provider")); + assert_eq!( + args, + vec![ + OsStr::new("$HOME"), + OsStr::new("${UNSET}"), + OsStr::new("$$"), + ] + ); } } } From 7f060de2fe101f1a343f230599494c691f56dfba Mon Sep 17 00:00:00 2001 From: schickling-assistant <261620128+schickling-assistant@users.noreply.github.com> Date: Sat, 5 Sep 2026 19:07:22 +0200 Subject: [PATCH 2/2] fix: require systemd 254 for opaque scope argv agent-identity: mbp2025.direct.omp.5tek2r48 agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.11 agent-runtime: OMP 18.0.11 tooling-profile: dotfiles@4cc7b25 --- src/isolate.rs | 64 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/src/isolate.rs b/src/isolate.rs index a7d24b78..43df4273 100644 --- a/src/isolate.rs +++ b/src/isolate.rs @@ -8,38 +8,38 @@ //! The fix: spawn each task into its own OS supervision domain, independent of BOTH the spawner and //! the transport daemon — one goal, per-OS mechanism. //! -//! - **Linux**: `systemd-run --user --scope --collect --quiet --unit=` +//! - **Linux with systemd 254+**: `systemd-run --user --scope --collect --quiet --unit=` //! `--expand-environment=no -- `. The task runs in its own transient scope = its own cgroup, //! registered with the user manager as a **sibling** of the transport unit (a scope created inside //! a service lands at `app.slice/`, not nested under the service). A cascade kill of the //! transport unit's cgroup cannot reach a sibling. `--scope` (not `--service`) keeps st2 the logical //! supervisor — systemd provides only the cgroup; adoption/teardown/restart stay st2's. `--collect` //! GCs the scope once it empties. -//! - **macOS / non-systemd Linux**: `setsid` + reparent to init/launchd is the whole defense — there -//! are no cgroups, and launchd does not cascade-kill detached children. Here [`wrap`] is a no-op -//! pass-through; the caller's existing `setsid` (exec) or the `pty` daemon (pty) provides detachment. -//! If isolation was *wanted* (a Linux box) but systemd is unreachable, we degrade to that same -//! pass-through and log a loud WARN — never a silent "isolated" claim. +//! - **macOS / unsupported Linux**: `setsid` + reparent to init/launchd is the fallback. Here [`wrap`] +//! is a no-op pass-through; the caller's existing `setsid` (exec) or the `pty` daemon (pty) provides +//! detachment. If isolation was wanted but the systemd user manager or the exact-argv capability +//! is unavailable, we degrade to that pass-through and log a loud WARN — never a silent +//! "isolated" claim. //! //! Teardown is unchanged: the scope is for **survival only**. `pty kill` / the exec process-group kill //! still tear tasks down; the scope just prevents the transport from taking them as collateral. use std::ffi::OsStr; use std::process::{Command, Stdio}; -use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::OnceLock; +use std::sync::atomic::{AtomicU64, Ordering}; /// How a task is isolated from its spawner and the transport daemon. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Isolation { - /// Linux: own transient systemd `--user` scope (own cgroup, sibling of the transport unit). + /// Linux with systemd 254+: own transient `--user` scope with opaque inner argv. Scope, /// macOS / non-systemd: `setsid` + reparent to init/launchd (no cgroup needed — the transport /// cannot cascade-kill a detached process on these platforms). Detached, - /// Isolation was wanted (a Linux host) but systemd is unreachable — degraded to `Detached` with a - /// logged WARN. Distinct from `Detached` so callers/tests can tell an intended pass-through from a - /// degraded one. + /// Isolation was wanted (a Linux host) but the required systemd user scope capability is + /// unavailable — degraded to `Detached` with a logged WARN. Distinct from `Detached` so + /// callers/tests can tell an intended pass-through from a degraded one. DegradedDetached, } @@ -57,10 +57,10 @@ fn detect() -> Isolation { } else { // The old line embedded a manual "WARN" prefix; the facade carries severity now. tracing::warn!( - "st2: systemd user scopes unavailable (no `systemd-run --user` / no \ - $XDG_RUNTIME_DIR) — spawning tasks WITHOUT cgroup isolation. A transport/supervisor \ - restart may cascade-kill them. Enable a user systemd manager (loginctl enable-linger) \ - to restore isolation." + "st2: systemd user scopes with opaque argv unavailable (`systemd-run` 254+ and \ + $XDG_RUNTIME_DIR are required) — spawning tasks WITHOUT cgroup isolation. A \ + transport/supervisor restart may cascade-kill them. Upgrade systemd and enable a \ + user manager (`loginctl enable-linger`) to restore isolation." ); Isolation::DegradedDetached } @@ -70,20 +70,29 @@ fn detect() -> Isolation { } } -/// Whether a `--user` systemd scope can be created here. +/// Whether a `--user` systemd scope can preserve the inner argv exactly. fn systemd_user_available() -> bool { if std::env::var_os("XDG_RUNTIME_DIR").is_none() { return false; } Command::new("systemd-run") .args(["--user", "--version"]) - .stdout(Stdio::null()) .stderr(Stdio::null()) - .status() - .map(|s| s.success()) + .output() + .map(|output| { + output.status.success() && systemd_version_supports_exact_argv(&output.stdout) + }) .unwrap_or(false) } +fn systemd_version_supports_exact_argv(output: &[u8]) -> bool { + std::str::from_utf8(output) + .ok() + .and_then(|output| output.split_ascii_whitespace().nth(1)) + .and_then(|version| version.parse::().ok()) + .is_some_and(|version| version >= 254) +} + static SCOPE_SEQ: AtomicU64 = AtomicU64::new(0); /// A fresh, systemd-safe scope unit name for a task id. The name is **write-only** — st2 references it @@ -160,6 +169,23 @@ mod tests { assert_ne!(scope_unit("x"), scope_unit("x")); } + #[test] + fn exact_argv_requires_systemd_254_or_newer() { + assert!(!systemd_version_supports_exact_argv( + b"systemd 249 (249.11)\n" + )); + assert!(!systemd_version_supports_exact_argv( + b"systemd 252 (252.38)\n" + )); + assert!(systemd_version_supports_exact_argv( + b"systemd 254 (254.5)\n" + )); + assert!(systemd_version_supports_exact_argv( + b"systemd 257 (257.7)\n" + )); + assert!(!systemd_version_supports_exact_argv(b"unexpected output\n")); + } + #[test] fn wrap_scope_disables_expansion_and_preserves_dollar_bearing_argv() { let cmd = wrap_for_mode(