diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d76778..366cf2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## v0.3.0 + +- The OCI runtime is now `exec`'d rather than spawned as a child process. containerd's shim invokes the runtime through go-runc, which builds commands with Go's `exec.CommandContext`; a cancelled or timed-out call delivers SIGKILL to the direct child only, so the interceptor died and the real runtime was orphaned beneath it. For a `runc delete` that left the task undeleted and its `containerd-shim-runc-v2` resident indefinitely, at roughly 5 MiB each until the host was rebooted. +- A runtime killed by a signal is now reported as signal death. Previously it was mapped to exit code -1, which `process::exit` truncates to 255 — indistinguishable from a runtime that genuinely exited 255. +- **Deployment change required for the above to take effect.** Docker generates its own non-`exec` `/bin/sh` wrapper for any runtime whose `runtimeArgs` is non-empty, which re-inserts exactly the process layer this release removes. Pass the interceptor's flags from a wrapper script of your own that `exec`s, name that script as the runtime's `path`, and leave `runtimeArgs` unset. See "Passing flags" in the README. +- Networking mounts (`/etc/hosts`, `/etc/hostname`, `/etc/resolv.conf`) are now always made read-only. This is a behaviour change for anyone who did not previously pass `--oi-readonly-networking-mounts`, and there is no opt-out. The flag is retained as an accepted no-op so an existing `daemon.json` that passes it keeps working. The mounts themselves are left in place, so Docker's embedded DNS resolver still works for sibling-container name resolution. Mount options are normalised rather than appended to: a mount that arrives with no options at all, or with a `rw` following a `ro`, now ends up read-only too, because `ro` is always placed last and that is the flag the kernel honours. +- The container spec is only written back to disk when a modification actually changed it. + ## v0.2.3 - Switched CI from `dtolnay/rust-toolchain` to `actions-rust-lang/setup-rust-toolchain`. CI builds now compile with `RUSTFLAGS=-D warnings` and surface rustc/rustfmt warnings as PR annotations; release builds are exempt so a new stable lint cannot break a tagged release. Dependabot now tracks GitHub Actions pins. diff --git a/Cargo.lock b/Cargo.lock index f3514e0..00bca4b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -236,7 +236,7 @@ checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" [[package]] name = "oci-interceptor" -version = "0.2.3" +version = "0.3.0" dependencies = [ "anyhow", "clap", diff --git a/Cargo.toml b/Cargo.toml index f95d8db..478096c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ license = "MIT OR Apache-2.0" name = "oci-interceptor" publish = false repository = "https://github.com/picoCTF/oci-interceptor" -version = "0.2.3" +version = "0.3.0" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html diff --git a/README.md b/README.md index 8cc3df7..993310c 100644 --- a/README.md +++ b/README.md @@ -38,7 +38,7 @@ Options: --oi-runtime-path Path to OCI runtime. [default: runc] --oi-readonly-networking-mounts - Mount networking files as readonly + Deprecated, and now a no-op: networking mounts are always readonly --oi-write-debug-output Write debug output --oi-debug-output-dir @@ -70,10 +70,7 @@ option, as it defaults to `runc`, the default runtime bundled with Docker. "default-runtime": "oci-interceptor", "runtimes": { "oci-interceptor": { - "path": "/usr/local/bin/oci-interceptor", - "runtimeArgs": [ - "--oi-readonly-networking-mounts" - ] + "path": "/usr/local/bin/oci-interceptor" } } } @@ -82,7 +79,40 @@ The Docker daemon must be restarted (`systemctl restart docker.service`) in orde Note that if you set `oci-interceptor` as the default runtime, you can still bypass it for a specific container by specifying `docker run --runtime=runc`. -While it is not possible to override `runtimeArgs` with a `docker run` option, you could specify multiple interceptor "runtimes" (with different flags) and switch between them using `docker run --runtime=`. +#### Passing flags: use a wrapper script, not `runtimeArgs` + +**Do not put the interceptor's flags in `runtimeArgs`.** Docker generates its own wrapper +script for any runtime whose `runtimeArgs` is non-empty, and that generated wrapper does +not `exec`: + +```sh +#!/bin/sh +/usr/local/bin/oci-interceptor --flags $@ +``` + +It therefore stays in the process tree between the containerd shim and the real runtime. +containerd invokes the runtime through go-runc, which builds commands with Go's +`exec.CommandContext`; a cancelled or timed-out call delivers SIGKILL to the direct child +only — that shell — orphaning `runc` beneath it. For a `runc delete` the task is then never +deleted and its `containerd-shim-runc-v2` never shuts down, leaking roughly 5 MiB per +occurrence until the host is rebooted. That defeats the point of the interceptor exec'ing +the runtime at all. + +If you have no flags to pass, you need no wrapper at all: name the binary itself as `path`, +as in the example above. Otherwise put the flags in a wrapper script of your own that +`exec`s, and name that script as `path`: + +```sh +#!/bin/sh +exec /usr/local/bin/oci-interceptor --oi-env HTTP_PROXY=http://proxy.example:3128 "$@" +``` + +Keep `"$@"` quoted — Docker's generated wrapper uses a bare `$@`, which word-splits and +glob-expands its arguments. An absent or empty `runtimeArgs` generates no wrapper. + +You can still run several interceptor "runtimes" with different flags and switch between +them using `docker run --runtime=`; each one needs its own wrapper script, named by +its own `path`. ## Supported Customizations @@ -100,12 +130,14 @@ users to fill the host storage volume. This can usually only be circumvented by manually creating read-only bind mounts over these paths (in which case Docker can no longer manage the container's DNS configuration) or by making the entire rootfs read-only (which severely constrains the workloads possible inside the container). -To avoid this issue, specify the `--oi-readonly-networking-mounts` flag. This modifies these mounts to be read-only, preventing writes from inside the container. +These mounts are always modified to be read-only, preventing writes from inside the container. The mounts themselves are left in place: Docker points a container on a user-defined network at its embedded DNS resolver by writing `nameserver 127.0.0.11` into the bind-mounted `/etc/resolv.conf`, so removing the mount would break resolution of sibling containers by name. Only write access is taken away. + +The `--oi-readonly-networking-mounts` flag is retained as an accepted no-op so that an existing `daemon.json` passing it keeps working; it no longer has any effect. #### Related issues - Workaround for [moby#13152](https://github.com/moby/moby/issues/41991), [moby#41991](https://github.com/moby/moby/issues/41991) (without custom bind mounts or making entire rootfs readonly) -- Optionally reverts [moby#5129](https://github.com/moby/moby/pull/5129) +- Reverts [moby#5129](https://github.com/moby/moby/pull/5129) ### Overriding environment variables @@ -127,7 +159,7 @@ Unit tests run with `cargo test`. End-to-end integration tests live in `tests/in To run the integration tests locally on a Linux host with Docker: 1. Build and install the binary: `cargo build --release && sudo install -m 0755 target/release/oci-interceptor /usr/local/bin/oci-interceptor` -2. Configure `/etc/docker/daemon.json` with the named runtimes listed in the module docs of [tests/integration.rs](tests/integration.rs). The CI workflow (`.github/workflows/CI.yml`, `integration` job) is the canonical reference for the required daemon.json. +2. Configure `/etc/docker/daemon.json` with the named runtimes listed in the module docs of [tests/integration.rs](tests/integration.rs). The CI workflow (`.github/workflows/CI.yml`, `integration` job) shows the exact set the suite expects. Note that it is a test harness, not a reference deployment: it uses `runtimeArgs` because that is the only native per-runtime flag mechanism and the flag assertions need it. For a real host, follow [Passing flags](#passing-flags-use-a-wrapper-script-not-runtimeargs) instead. 3. `sudo systemctl restart docker` 4. `OCI_INTERCEPTOR_INTEGRATION=1 cargo test --test integration -- --test-threads=1` @@ -135,7 +167,7 @@ CI runs the same suite on every push and pull request, which provides a regressi ### Debug output -Specify the `--oi-write-debug-output` flag to write original, parsed, and modified container configs to the directory specified as `--oi-debug-output-dir` (default `/var/log/oci-interceptor`). +Specify the `--oi-write-debug-output` flag to write original, parsed, and modified container configs to the directory specified as `--oi-debug-output-dir` (default `/var/log/oci-interceptor`). As with every other flag, pass these from an exec'ing wrapper script rather than `runtimeArgs` — see [Passing flags](#passing-flags-use-a-wrapper-script-not-runtimeargs). The resulting files will be named: - `_original.json` (the original config) diff --git a/src/main.rs b/src/main.rs index 819bbc8..e01a22d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,7 @@ use clap::{ use env_vars::{EnvVar, EnvVarOverride, modify_env_vars, parse_env_var}; use networking_mounts::modify_networking_mounts; use oci_spec::runtime::Spec; -use std::{fs, io::Write, path::PathBuf, process}; +use std::{fs, io::Write, os::unix::process::CommandExt, path::PathBuf, process}; fn main() -> Result<()> { let matches = clap::Command::new(crate_name!()) @@ -26,10 +26,13 @@ fn main() -> Result<()> { .help("Path to OCI runtime."), ) .arg( + // Retained as an accepted no-op so a daemon.json still passing it keeps + // working: rejecting it would fail every container launch on any host + // upgraded before its runtime config was updated. Arg::new("readonly-networking-mounts") .long("oi-readonly-networking-mounts") .action(ArgAction::SetTrue) - .help("Mount networking files as readonly"), + .help("Deprecated, and now a no-op: networking mounts are always readonly"), ) .arg( Arg::new("write-debug-output") @@ -137,11 +140,20 @@ fn main() -> Result<()> { serde_json::to_writer_pretty(&parsed_file, &spec)?; } - // Make any enabled modifications - if matches.get_flag("readonly-networking-mounts") { - modify_networking_mounts(&mut spec); - spec_modified = true; - } + // Make any enabled modifications. + // + // Networking mounts are always made read-only. Docker bind-mounts these three + // files from outside the container's writable layer, so where XFS project quotas + // restrict that layer they are an escape hatch for filling the host volume -- see + // "Read-only networking mounts" in the README. On a shared host that affects every + // other container, which is what makes it worth enforcing unconditionally. + // + // Note this is not a barrier between containers: each gets its own copy of these + // files, so tampering only ever affects the container doing it, and a process with + // code execution can bypass them anyway (HOSTALIASES, RES_OPTIONS, LD_PRELOAD, or + // simply querying a resolver directly). Treat it as quota enforcement, with + // in-container tamper resistance as a side effect. + spec_modified |= modify_networking_mounts(&mut spec); if !env_var_overrides.is_empty() { modify_env_vars(&mut spec, env_var_overrides); spec_modified = true; @@ -176,7 +188,8 @@ fn main() -> Result<()> { .write_all(format!("{} {}\n", runtime_path, runtime_options.join(" ")).as_bytes())?; runtime_calls.flush()?; } - std::process::exit(call_oci_runtime(runtime_path, runtime_options)?); + // On success this replaces the current process, so nothing below runs. + Err(call_oci_runtime(runtime_path, runtime_options)) } /// Extracts the container bundle path from the trailing runtime options, if present. @@ -194,19 +207,29 @@ fn get_bundle_path(options: &mut [String]) -> Option { None } -/// Calls the actual OCI runtime, passing along any runtime options. -fn call_oci_runtime(runtime_path: &str, options: Vec) -> Result { - let mut child = process::Command::new(runtime_path) +/// Replaces this process with the actual OCI runtime, passing along any runtime options. +/// +/// This execs rather than spawning a child and waiting on it, so that the runtime inherits +/// our PID and we leave the process tree entirely. A wrapper that spawns and waits is +/// wrong for an OCI runtime in two ways: +/// +/// - containerd's shim invokes the runtime through go-runc, which builds commands with +/// Go's `exec.CommandContext`. On a cancelled or timed-out call that delivers SIGKILL to +/// the direct child only. With a wrapper in between, the wrapper dies and the real +/// runtime is orphaned, while the shim believes the call was cancelled. For a +/// `runc delete` that leaves the task undeleted and its shim never shut down. +/// - A runtime killed by a signal was reported to the caller as exit code -1, which +/// `process::exit` truncates to 255. Callers that distinguish signal death from a 255 +/// exit status saw the wrong thing. Exec'ing reports the real wait status. +/// +/// Returns only on failure to exec. +fn call_oci_runtime(runtime_path: &str, options: Vec) -> anyhow::Error { + let err = process::Command::new(runtime_path) .args(options.as_slice()) - .spawn() - .with_context(|| "Failed to execute underlying OCI runtime")?; - let status = child - .wait() - .with_context(|| "Failed to wait on OCI runtime process")?; - match status.code() { - Some(code) => Ok(code), - None => Ok(-1), // child process was killed by a signal - } + .exec(); + anyhow::Error::new(err).context(format!( + "Failed to execute underlying OCI runtime: {runtime_path}" + )) } #[cfg(test)] diff --git a/src/networking_mounts.rs b/src/networking_mounts.rs index 78bce6a..fcc8165 100644 --- a/src/networking_mounts.rs +++ b/src/networking_mounts.rs @@ -7,24 +7,104 @@ use oci_spec::runtime::Spec; /// - /etc/resolv.conf /// /// in the container config, making them read-only. -pub(crate) fn modify_networking_mounts(spec: &mut Spec) { - if let Some(mounts) = spec.mounts() { - let mut mounts = mounts.clone(); - for mount in mounts.iter_mut() { - match mount.destination().to_str() { - Some("/etc/hosts") | Some("/etc/hostname") | Some("/etc/resolv.conf") => { - if let Some(options) = mount.options() - && !options.contains(&"ro".into()) - { - let mut options = options.clone(); - options.push("ro".into()); - mount.set_options(Some(options)); - } +/// +/// The mounts themselves are left in place. Docker points a container on a user-defined +/// network at its embedded DNS resolver by writing `nameserver 127.0.0.11` into the +/// bind-mounted /etc/resolv.conf, so removing that mount would break resolution of +/// sibling containers by name. Only write access is taken away. +/// +/// Returns whether any mount was actually changed. +pub(crate) fn modify_networking_mounts(spec: &mut Spec) -> bool { + let Some(mounts) = spec.mounts() else { + return false; + }; + let mut mounts = mounts.clone(); + let mut changed = false; + for mount in mounts.iter_mut() { + match mount.destination().to_str() { + Some("/etc/hosts") | Some("/etc/hostname") | Some("/etc/resolv.conf") => { + // Absent options are still writable, and the kernel takes the last of a + // conflicting pair, so an existing "ro" earlier in the list does not + // survive a later "rw". Drop both and re-append "ro" so it always wins. + let current = mount.options().clone().unwrap_or_default(); + let mut wanted: Vec = current + .iter() + .filter(|option| *option != "ro" && *option != "rw") + .cloned() + .collect(); + wanted.push(String::from("ro")); + if wanted != current { + mount.set_options(Some(wanted)); + changed = true; } - Some(_) => {} - None => {} } + _ => {} } + } + if changed { spec.set_mounts(Some(mounts)); } + changed +} + +#[cfg(test)] +mod tests { + use super::modify_networking_mounts; + use oci_spec::runtime::Spec; + + fn spec_with(mounts: &str) -> Spec { + serde_json::from_str(&format!("{{\"ociVersion\":\"1.0.0\",\"mounts\":{mounts}}}")) + .expect("test spec should parse") + } + + fn options_of(spec: &Spec, destination: &str) -> Vec { + spec.mounts() + .as_ref() + .expect("spec should have mounts") + .iter() + .find(|mount| mount.destination().to_str() == Some(destination)) + .expect("mount should be present") + .options() + .clone() + .unwrap_or_default() + } + + #[test] + fn a_mount_without_options_still_becomes_readonly() { + let mut spec = spec_with("[{\"destination\":\"/etc/hosts\"}]"); + assert!(modify_networking_mounts(&mut spec)); + assert_eq!(options_of(&spec, "/etc/hosts"), vec!["ro"]); + } + + #[test] + fn a_writable_mount_keeps_its_other_options() { + let mut spec = + spec_with("[{\"destination\":\"/etc/hosts\",\"options\":[\"rbind\",\"rw\"]}]"); + assert!(modify_networking_mounts(&mut spec)); + assert_eq!(options_of(&spec, "/etc/hosts"), vec!["rbind", "ro"]); + } + + #[test] + fn a_later_rw_does_not_survive_an_earlier_ro() { + // The kernel takes the last of a conflicting pair, so this mount was writable. + let mut spec = + spec_with("[{\"destination\":\"/etc/resolv.conf\",\"options\":[\"ro\",\"rw\"]}]"); + assert!(modify_networking_mounts(&mut spec)); + assert_eq!(options_of(&spec, "/etc/resolv.conf"), vec!["ro"]); + } + + #[test] + fn an_already_readonly_mount_reports_no_change() { + let mut spec = + spec_with("[{\"destination\":\"/etc/hostname\",\"options\":[\"rbind\",\"ro\"]}]"); + assert!(!modify_networking_mounts(&mut spec)); + assert_eq!(options_of(&spec, "/etc/hostname"), vec!["rbind", "ro"]); + } + + #[test] + fn unrelated_mounts_are_left_writable() { + let mut spec = spec_with("[{\"destination\":\"/tmp\",\"options\":[\"rw\"]}]"); + assert!(!modify_networking_mounts(&mut spec)); + assert_eq!(options_of(&spec, "/tmp"), vec!["rw"]); + } } diff --git a/tests/integration.rs b/tests/integration.rs index c0a9c93..143c76f 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -90,16 +90,16 @@ fn passthrough_propagates_nonzero_exit_code() { } #[test] -fn networking_mounts_writable_without_flag() { - if !check_enabled("networking_mounts_writable_without_flag") { +fn networking_mounts_readonly_without_any_flag() { + if !check_enabled("networking_mounts_readonly_without_any_flag") { return; } for target in ["/etc/hosts", "/etc/hostname", "/etc/resolv.conf"] { let opts = mount_options("oi-default", target); let opts: Vec<&str> = opts.split(',').collect(); assert!( - opts.contains(&"rw"), - "{target} expected to be rw by default, got: {opts:?}" + opts.contains(&"ro"), + "{target} expected to be ro with no flags at all, got: {opts:?}" ); } } diff --git a/tests/runtime_invocation.rs b/tests/runtime_invocation.rs new file mode 100644 index 0000000..b4c51dd --- /dev/null +++ b/tests/runtime_invocation.rs @@ -0,0 +1,246 @@ +//! Tests for how oci-interceptor hands off to the underlying OCI runtime. +//! +//! These use a stub runtime script rather than runc, so they need neither Docker nor a +//! real runtime and always run as part of `cargo test`. +//! +//! The property under test is that the interceptor *execs* the runtime rather than +//! spawning it as a child and waiting. containerd's shim invokes the runtime through +//! go-runc, which builds commands with Go's `exec.CommandContext`; a cancelled call +//! delivers SIGKILL to the direct child only. If the interceptor were still in the +//! process tree at that point, the signal would kill the interceptor and orphan the real +//! runtime underneath it. + +#![cfg(unix)] + +use std::fs; +use std::os::unix::fs::PermissionsExt; +use std::os::unix::process::ExitStatusExt; +use std::path::{Path, PathBuf}; +use std::process::{Command, Stdio}; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::thread::sleep; +use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; + +const BIN: &str = env!("CARGO_BIN_EXE_oci-interceptor"); + +fn unique_dir(prefix: &str) -> PathBuf { + static COUNTER: AtomicU64 = AtomicU64::new(0); + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0); + let n = COUNTER.fetch_add(1, Ordering::Relaxed); + let dir = std::env::temp_dir().join(format!("oi-{prefix}-{nanos}-{n}")); + fs::create_dir_all(&dir).expect("failed to create temp dir"); + dir +} + +/// Writes a stub runtime that records its own PID and then blocks, so the test can +/// inspect the process tree while a runtime call is in flight. +fn write_stub_runtime(dir: &Path, pid_file: &Path) -> PathBuf { + let path = dir.join("stub-runtime"); + fs::write( + &path, + format!( + "#!/bin/sh\necho $$ > {}\nexec sleep 30\n", + pid_file.display() + ), + ) + .expect("failed to write stub runtime"); + fs::set_permissions(&path, fs::Permissions::from_mode(0o755)) + .expect("failed to chmod stub runtime"); + path +} + +fn wait_for_pid_file(pid_file: &Path) -> u32 { + let deadline = Instant::now() + Duration::from_secs(10); + while Instant::now() < deadline { + if let Ok(contents) = fs::read_to_string(pid_file) + && let Ok(pid) = contents.trim().parse::() + { + return pid; + } + sleep(Duration::from_millis(20)); + } + panic!( + "stub runtime never recorded its PID at {}", + pid_file.display() + ); +} + +fn process_alive(pid: u32) -> bool { + // `kill -0` rather than a /proc lookup: this module is cfg(unix), and on a Unix + // without procfs the path test reports every process as dead, which would turn the + // orphan assertion below into a green test that checks nothing. Panic rather than + // defaulting to false, for the same reason. + Command::new("kill") + .args(["-0", &pid.to_string()]) + // kill(1) writes "No such process" to stderr for a dead pid, which is the normal + // result here and only clutters the test output. + .stderr(Stdio::null()) + .status() + .expect("failed to run kill -0") + .success() +} + +/// The interceptor must exec the runtime, leaving the runtime with the interceptor's own +/// PID. If it spawned and waited instead, the runtime would report a different PID. +#[test] +fn execs_runtime_in_place_rather_than_spawning_a_child() { + let dir = unique_dir("exec"); + let pid_file = dir.join("runtime.pid"); + let stub = write_stub_runtime(&dir, &pid_file); + + let mut child = Command::new(BIN) + .arg("--oi-runtime-path") + .arg(&stub) + .args(["state", "some-container-id"]) + .spawn() + .expect("failed to invoke oci-interceptor"); + + let runtime_pid = wait_for_pid_file(&pid_file); + let interceptor_pid = child.id(); + + let _ = child.kill(); + let _ = child.wait(); + // When this assertion is about to fail the runtime is a separate process, so killing + // the interceptor leaves it behind. Clean up before reporting. + if runtime_pid != interceptor_pid { + force_kill(runtime_pid); + } + let _ = fs::remove_dir_all(&dir); + + assert_eq!( + runtime_pid, interceptor_pid, + "runtime ran as PID {runtime_pid} but the interceptor was PID {interceptor_pid}; \ + the interceptor spawned the runtime as a child instead of exec'ing it, which \ + orphans the runtime when the caller signals the interceptor" + ); +} + +/// Signalling the interceptor must take the runtime down with it. A spawn-and-wait +/// wrapper leaves the runtime running after the wrapper is killed. +#[test] +fn signalling_the_interceptor_does_not_orphan_the_runtime() { + let dir = unique_dir("orphan"); + let pid_file = dir.join("runtime.pid"); + let stub = write_stub_runtime(&dir, &pid_file); + + let mut child = Command::new(BIN) + .arg("--oi-runtime-path") + .arg(&stub) + .args(["delete", "some-container-id"]) + .spawn() + .expect("failed to invoke oci-interceptor"); + + let runtime_pid = wait_for_pid_file(&pid_file); + + // SIGKILL to the process the caller started, exactly as go-runc's + // exec.CommandContext does when a runtime call is cancelled. + child.kill().expect("failed to kill oci-interceptor"); + child.wait().expect("failed to reap oci-interceptor"); + + // Give the kernel a moment to tear the process down before checking. + let deadline = Instant::now() + Duration::from_secs(5); + while process_alive(runtime_pid) && Instant::now() < deadline { + sleep(Duration::from_millis(20)); + } + let orphaned = process_alive(runtime_pid); + + if orphaned { + force_kill(runtime_pid); + } + let _ = fs::remove_dir_all(&dir); + + assert!( + !orphaned, + "runtime PID {runtime_pid} survived the interceptor being killed; a cancelled \ + runtime call would leave an orphaned runc behind, and its shim never shut down" + ); +} + +/// Best-effort SIGKILL so a failing test does not leak the stub runtime. +fn force_kill(pid: u32) { + let _ = Command::new("kill").arg("-9").arg(pid.to_string()).status(); +} + +/// Writes a stub runtime that exits immediately in a given way. +fn write_stub(dir: &Path, name: &str, body: &str) -> PathBuf { + let path = dir.join(name); + fs::write( + &path, + format!( + "#!/bin/sh +{body} +" + ), + ) + .expect("failed to write stub"); + fs::set_permissions(&path, fs::Permissions::from_mode(0o755)).expect("failed to chmod stub"); + path +} + +/// A runtime that cannot be exec'd must surface as a non-zero exit naming the path, rather +/// than being mistaken for a runtime that ran and failed. +#[test] +fn failure_to_exec_the_runtime_is_reported() { + let out = Command::new(BIN) + .arg("--oi-runtime-path") + .arg("/nonexistent/runc") + .args(["state", "some-container-id"]) + .output() + .expect("failed to invoke oci-interceptor"); + + assert!(!out.status.success(), "expected a non-zero exit"); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + stderr.contains("/nonexistent/runc"), + "stderr should name the runtime it could not execute, got: {stderr}" + ); +} + +/// A runtime killed by a signal must be reported as signal death. Spawning and waiting +/// reported it as exit code -1, which process::exit truncates to 255 — indistinguishable +/// from a runtime that genuinely exited 255. +#[test] +fn a_signal_killed_runtime_is_not_reported_as_exit_255() { + let dir = unique_dir("signal"); + let stub = write_stub(&dir, "suicidal-runtime", "kill -TERM $$"); + + let status = Command::new(BIN) + .arg("--oi-runtime-path") + .arg(&stub) + .args(["state", "some-container-id"]) + .status() + .expect("failed to invoke oci-interceptor"); + + let _ = fs::remove_dir_all(&dir); + assert_eq!( + status.signal(), + Some(15), + "expected death by SIGTERM, got {status:?}" + ); + assert_eq!( + status.code(), + None, + "a signal-killed runtime must not present as an ordinary exit code" + ); +} + +/// Exit codes must survive the exec. Covered by the integration suite too, but that is +/// gated behind OCI_INTERCEPTOR_INTEGRATION and silently passes without Docker. +#[test] +fn runtime_exit_code_propagates() { + let dir = unique_dir("exitcode"); + let stub = write_stub(&dir, "exit42-runtime", "exit 42"); + + let status = Command::new(BIN) + .arg("--oi-runtime-path") + .arg(&stub) + .args(["state", "some-container-id"]) + .status() + .expect("failed to invoke oci-interceptor"); + + let _ = fs::remove_dir_all(&dir); + assert_eq!(status.code(), Some(42)); +}