cli: serialize sandbox exec admission across CLI processes (SL3.9)

doc/run.md

@@ -1528,8 +1528,14 @@ of one sandbox take effect one at a time: each waits up to `--timeout` seconds
 for any other in progress, and of two overlapping starts, exactly one launches
 OpenVMM while the other then fails because the sandbox is already running.
 `exec` holds the lock only while it finds the running OpenVMM, so a long
-workload does not delay `stop`. A provisioned state directory keeps
-`sandbox.lock`, and `deprovision` removes it with NVX's other files. A
+workload does not delay `stop`. One `exec` at a time runs in a sandbox, across
+processes: an `exec` takes the sandbox's exec lock, `exec.lock` in the state
+directory, before it finds the running OpenVMM, and holds it until the workload
+ends. An `exec` that finds it held tries again until `--wait` seconds, 60 by
+default, have passed, and then fails with `sandbox-busy`. `stop` takes no exec
+lock, so it proceeds while a workload runs. A provisioned state directory keeps
+`sandbox.lock`, and `exec.lock` from its first `exec` on, and `deprovision`
+removes them with NVX's other files. A
 transition that leaves a state directory without `config.json`, such as one of
 a sandbox that was never provisioned, removes `sandbox.lock` as it releases the
 lock. These are the rules of the sandbox store, which other orchestrators of
@@ -1633,7 +1639,8 @@ template, so it has the same artifact requirements. It checks that:
   capability, control socket, report staging file, or OpenVMM process;
 - every operation fails on a state directory that was never provisioned, a
   repeated `provision` leaves the configuration unchanged, and `exec` and
-  `stop` fail before `start`;
+  `stop` fail before `start`, though the `exec` leaves the `exec.lock` that the
+  state directory keeps from then on;
 - a repeated `start` and a `deprovision` of a running sandbox fail without
   changing its runtime record, capability, or OpenVMM process, and the
   sandbox keeps serving requests;
@@ -1649,7 +1656,7 @@ template, so it has the same artifact requirements. It checks that:
   `--outcome-report` path exists fails before its workload runs and leaves the
   file intact;
 - `stop` ends OpenVMM and its control socket or pipe, leaves only
-  `config.json`, `sandbox.lock`, `openvmm.log`, and a successful
+  `config.json`, `sandbox.lock`, `exec.lock`, `openvmm.log`, and a successful
   `outcome.json`, and leaves a cleanly unmounted scratch file system, after
   which `exec` and `stop` fail;
 - two overlapping `start` calls then launch a single OpenVMM process: one
@@ -1662,12 +1669,13 @@ template, so it has the same artifact requirements. It checks that:
   changing the runtime record or OpenVMM process, while the checkout's CLI
   still serves requests. Its `stop` then terminates OpenVMM without a guest
   shutdown and warns that it did, and leaves only `config.json`,
-  `sandbox.lock`, and `openvmm.log`, without a control socket or pipe, after
-  which its `stop` finds the sandbox not running, and the sandbox starts again;
+  `sandbox.lock`, `exec.lock`, and `openvmm.log`, without a control socket or
+  pipe, after which its `stop` finds the sandbox not running, and the sandbox
+  starts again;
 - after OpenVMM exits without a `stop`, `exec` reports the stale runtime state
   once and clears it, after which `stop` finds the sandbox not running, and the
-  state directory holds only `config.json`, `sandbox.lock`, and `openvmm.log`,
-  without a control socket or pipe; and
+  state directory holds only `config.json`, `sandbox.lock`, `exec.lock`, and
+  `openvmm.log`, without a control socket or pipe; and
 - `deprovision` removes only NVX's files: it fails and keeps a foreign file in
   the state directory, and once that file is gone, it removes the directory,
   which a later `exec` does not recreate.

doc/usage.md

@@ -99,7 +99,7 @@ each code for callers of the `nvx` library.
 | `sandbox-not-running` | 1 | `exec` or `stop` found the VM of a provisioned sandbox not running. |
 | `sandbox-running` | 1 | `start` or `deprovision` found the sandbox's VM, or a launch of it, running. |
 | `sandbox-exited` | 1 | The sandbox's OpenVMM exited without a `stop`: the transition that finds it reports it and clears its runtime record, and `start` launches the sandbox again. |
-| `sandbox-busy` | 1 | Another lifecycle transition held the sandbox's lock until `--timeout` passed. |
+| `sandbox-busy` | 1 | Another lifecycle transition held the sandbox's lock until `--timeout` passed, or another `exec` held its exec lock until `--wait` passed. |
 | `runtime-mismatch` | 1 | The sandbox's VM runs under another runtime identity than the artifacts', whose differing parts the message names, so only `stop`, which terminates it, applies. |
 | `vmm-indeterminate` | 1 | Whether the sandbox's OpenVMM runs cannot be determined, so its records remain for a later attempt. |
 | `format-unsupported` | 1 | A record or report has a format that this build does not read, such as the state of a sandbox that another build provisioned or started. |
@@ -259,6 +259,7 @@ nvx-rs/target/release/nvx-rs sandbox
     [--workload-user UID:GID]
     [--memory-mib MIB]
     [--timeout SECONDS]
+    [--wait SECONDS]
     [--exec-timeout-ms MILLISECONDS]
     [--cwd GUEST_PATH]
     [--environment KEY=VALUE]...
@@ -313,6 +314,7 @@ do, and their names cannot be abbreviated either.
 | `--workload-user UID:GID` | `65534:65534` | Select the fixed non-root workload identity for `run` or `provision`. Both IDs must be ASCII decimal integers from 1 to 4294967294, without signs, whitespace, or underscores. |
 | `--memory-mib MIB` | `256` | Set guest memory in MiB, an ASCII decimal integer from 1 to 17592186044415, without signs, whitespace, or underscores. |
 | `--timeout SECONDS` | `60` | Set the timeout for each managed control request and its response in `start`, `exec`, and `stop`, and the longest time that `provision`, `start`, `stop`, and `deprovision` wait for another lifecycle transition of the sandbox. It is a decimal number, with an optional fraction and exponent, that must be finite and greater than 0. |
+| `--wait SECONDS` | `60` | Set the longest time that a managed `exec` waits for the `exec` that another `nvx-rs` process runs in the sandbox to end, since a sandbox runs one `exec` at a time; it then fails with `sandbox-busy`. `stop` never waits for an `exec`. Like `--timeout`, which still bounds each wait for another lifecycle transition, it is a decimal number that must be finite and greater than 0. |
 | `--exec-timeout-ms MILLISECONDS` | `0` | Set the managed `exec` guest workload timeout to an ASCII decimal integer from 0 to 4294967295, without signs, whitespace, or underscores; zero disables the workload deadline. This is separate from the finite host `--timeout` control deadline. |
 | `--cwd GUEST_PATH` | `/` | Set an absolute working directory inside the workload root for managed `exec`. It is resolved after the workload identity and root are applied; missing, inaccessible, or non-directory paths fail the workload launch. Default and layered environments point `PWD` at this directory; an exact replacement environment controls `PWD` itself. |
 | `--environment KEY=VALUE` | omitted | Set the exact managed `exec` environment. Repeat for multiple entries. Empty values, spaces, additional equals signs, and UTF-8 are preserved. Inline values are visible in the invoking host process arguments; use `--environment-file` for sensitive values. |

nvx-rs/README.md

@@ -2869,6 +2869,7 @@ them:
 | --- | --- |
 | `config.json` | The provisioned sandbox, in format 9 only: `image` holds its format, path and identity; `upper` is tagged as scratch with its path or RAM with `max_memory_percent`. `distro_binding` binds the image, and `scratch_binding` binds scratch or is `null` for RAM. The export and network are typed, `null` when absent. An atomic pretty-JSON write uses mode 0600, and start reads the same strict typed fields without a legacy reader. |
 | `sandbox.lock` | The store's lock, which serializes the lifecycle transitions |
+| `exec.lock` | The exec lock, which an `exec` holds from before it finds the running VM until its workload ends, so that one `exec` at a time runs in the sandbox; it stays from the first `exec` until `deprovision` |
 | `launch.json` | The store's journal of a start in progress, or of one that was interrupted |
 | `runtime.json` | The store's runtime record of the running VM, in its format 4: the OpenVMM process, by its ID, its start time, and the host's boot, its control endpoint, `STATE/control.sock` or `//./pipe/openvmm-microvm-HEX` on Windows, and the [runtime identity](#runtime-identity) of its start |
 | `control.capability` | The 32-byte capability of the running OpenVMM, with mode 0600 |
@@ -2878,7 +2879,10 @@ them:
 
 `provision`, `start`, `stop`, and `deprovision` hold the store's lock for their
 whole transition, and `exec` only while it finds the running VM, so that a long
-workload cannot delay a `stop`. The CLI takes the store's
+workload cannot delay a `stop`. An `exec` that finds another holding
+`exec.lock` releases the store's lock and tries again until `--wait` seconds,
+60 by default, have passed, and then fails with `sandbox-busy`; `stop` takes no
+exec lock. The CLI takes the store's
 [lifecycle locks](src/host/store/mod.rs) with `config.json` as its
 configuration record: a transition that leaves the state directory without it,
 such as one of a sandbox that was never provisioned, removes the lock file as
@@ -3041,7 +3045,9 @@ guest agents of NVX advertise bits 0, 4, and 6 in sandbox mode.
 - The state directory holds the store's files: `sandbox.lock` replaces the
   `lifecycle.lock` of `nvx.py`, `runtime.json` has the store's format, and each
   start replaces `openvmm.log`, which `nvx.py` appended to. `exec` holds the
-  lock while it finds the running VM, which `nvx.py` did not. A runtime record
+  lock while it finds the running VM, which `nvx.py` did not, and holds
+  `exec.lock` until its workload ends, so that one `exec` at a time runs in a
+  sandbox, where `nvx.py` ran them at once and had no `--wait`. A runtime record
   of an OpenVMM that exited is cleared by the first transition that finds it,
   so that `start` launches the sandbox again, where `nvx.py` refused to.
 - `provision` refuses an existing state directory that is not private to the
@@ -3300,7 +3306,7 @@ usage errors, which no library error has.
 | `sandbox-not-running` | `Other` | The operation needs the sandbox's VM, which is not running |
 | `sandbox-running` | `Other` | The operation needs a stopped sandbox, but its VM, or a launch of it, may still run |
 | `sandbox-exited` | `Exited` | The sandbox's VM exited without a stop: the transition that finds it clears its runtime record, and a start launches it again |
-| `sandbox-busy` | `TimedOut` | Another lifecycle transition of the sandbox held its lock until the deadline passed |
+| `sandbox-busy` | `TimedOut` | Another lifecycle transition, or `exec`, of the sandbox held its lock until the deadline passed |
 | `runtime-mismatch` | `RuntimeMismatch` | The sandbox's VM runs under another runtime identity than the artifacts', whose differing parts the message names, so only a stop, which terminates it, applies |
 | `vmm-indeterminate` | `Other` | Whether the OpenVMM process of the sandbox runs cannot be determined, so its records remain for a later attempt |
 | `format-unsupported` | `Io` | A record or report has a format or schema version that this build does not read, such as the state of a sandbox that another build provisioned or started |

nvx-rs/src/error.rs

@@ -132,8 +132,8 @@ pub enum ErrorCode {
     SandboxRunning,
     /// `sandbox-exited`: the sandbox's VM exited without a stop. A start launches it again.
     SandboxExited,
-    /// `sandbox-busy`: another lifecycle transition of the sandbox held its lock until the
-    /// deadline passed.
+    /// `sandbox-busy`: another lifecycle transition, or exec, of the sandbox held its lock until
+    /// the deadline passed.
     SandboxBusy,
     /// `runtime-mismatch`: the sandbox's VM runs under another runtime identity than the
     /// artifacts', so it is not driven, and only a stop, which terminates it, applies.

nvx-rs/src/host/cli/exec_lock.rs

@@ -0,0 +1,57 @@
+//! The exec lock of a sandbox, which runs one managed exec at a time in it, across processes.
+//!
+//! An exec holds the OS lock of `exec.lock` in the sandbox's state directory from before it finds
+//! the running VM until its workload ends. A release leaves the file, which `deprovision` removes
+//! with the store's files, so that no release unlinks a file that another exec has open.
+
+use std::fs::{File, TryLockError};
+
+use super::store::{LockedState, open_lock};
+use super::{Exit, Result, check_interrupt};
+use crate::ErrorCode;
+use crate::host::platform;
+
+/// The name of the exec lock in a sandbox's state directory.
+pub(crate) const NAME: &str = "exec.lock";
+
+/// The held exec lock of a sandbox, which dropping it releases.
+#[derive(Debug)]
+pub(crate) struct ExecLock {
+    file: File,
+}
+
+impl ExecLock {
+    /// Tries the exec lock of the sandbox whose lifecycle lock `state` holds, without waiting, and
+    /// returns `None` while another exec holds it. The lock file is created and checked as the
+    /// store's own lock file is, and refused once its entry no longer names the locked file.
+    pub(crate) fn try_acquire(state: &LockedState) -> Result<Option<Self>> {
+        check_interrupt()?;
+        let directory = state.directory()?;
+        let guard = Self {
+            file: open_lock(&state.dir().join(NAME))?,
+        };
+        match guard.file.try_lock() {
+            Ok(()) => {}
+            Err(TryLockError::WouldBlock) => return Ok(None),
+            Err(TryLockError::Error(error)) => return Err(Exit::from_os_error(error)),
+        }
+        let held = platform::opened_file_id(&guard.file).map_err(Exit::from_os_error)?;
+        if directory.entry_id(NAME).map_err(Exit::from_os_error)? != Some(held) {
+            return Err(Exit::error(
+                ErrorCode::Io,
+                "sandbox exec lock was removed or replaced",
+            ));
+        }
+        Ok(Some(guard))
+    }
+}
+
+impl Drop for ExecLock {
+    fn drop(&mut self) {
+        // Unlock explicitly so a concurrently forked child's descriptor cannot extend admission.
+        let _ = self.file.unlock();
+    }
+}
+
+#[cfg(test)]
+mod tests;

nvx-rs/src/host/cli/exec_lock/tests.rs

@@ -0,0 +1,164 @@
+use super::*;
+use crate::host::cli::deadline::Deadline;
+use crate::host::cli::store::{StateDir, StateFile};
+use crate::host::platform;
+use std::fs;
+
+fn state() -> (tempfile::TempDir, StateDir) {
+    let directory = tempfile::tempdir().unwrap();
+    let state = StateDir::create(&directory.path().join("state")).unwrap();
+    (directory, state)
+}
+
+fn admit(state: &StateDir) -> Result<Option<ExecLock>> {
+    let locked = StateDir::open(state.dir())?.lock(Deadline::after(1.0))?;
+    ExecLock::try_acquire(&locked)
+}
+
+#[test]
+fn admission_is_private_nonblocking_and_keeps_its_file_on_release() {
+    let (_directory, state) = state();
+    let held = admit(&state).unwrap().unwrap();
+    let path = state.dir().join(NAME);
+    let identity = platform::file_id(&path).unwrap();
+    assert_eq!(platform::private_file_problem(&held.file).unwrap(), None);
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt;
+
+        assert_eq!(
+            fs::metadata(&path).unwrap().permissions().mode() & 0o777,
+            0o600
+        );
+    }
+    assert!(admit(&state).unwrap().is_none());
+    let other = StateDir::create(&_directory.path().join("other")).unwrap();
+    drop(admit(&other).unwrap().unwrap());
+    drop(held);
+    let _next = admit(&state).unwrap().unwrap();
+    assert_eq!(platform::file_id(&path).unwrap(), identity);
+}
+
+#[test]
+fn a_replaced_lifecycle_lock_refuses_admission_before_file_creation() {
+    let (_directory, state) = state();
+    let old = state.lock(Deadline::after(1.0)).unwrap();
+    fs::rename(state.path(StateFile::Lock), state.dir().join("old.lock")).unwrap();
+    let current = state.lock(Deadline::after(1.0)).unwrap();
+    assert_eq!(
+        ExecLock::try_acquire(&old).unwrap_err().code(),
+        Some(ErrorCode::Io)
+    );
+    assert!(!state.dir().join(NAME).exists());
+    let _held = ExecLock::try_acquire(&current).unwrap().unwrap();
+    drop(old);
+    assert!(ExecLock::try_acquire(&current).unwrap().is_none());
+}
+
+#[cfg(unix)]
+#[test]
+fn a_link_to_an_already_locked_exec_file_is_refused() {
+    let (_directory, state) = state();
+    let _held = admit(&state).unwrap().unwrap();
+    let path = state.dir().join(NAME);
+    let target = state.dir().join("old.lock");
+    fs::rename(&path, &target).unwrap();
+    std::os::unix::fs::symlink(&target, &path).unwrap();
+    assert_eq!(admit(&state).unwrap_err().code(), Some(ErrorCode::Io));
+    assert!(path.is_symlink());
+}
+
+#[test]
+fn non_regular_exec_lock_entries_are_refused() {
+    let (_directory, state) = state();
+    let path = state.dir().join(NAME);
+    fs::create_dir(&path).unwrap();
+    assert_eq!(admit(&state).unwrap_err().code(), Some(ErrorCode::Io));
+    assert!(path.is_dir());
+    fs::remove_dir(&path).unwrap();
+    #[cfg(unix)]
+    {
+        let target = state.dir().join("target");
+        fs::write(&target, "keep").unwrap();
+        std::os::unix::fs::symlink(&target, &path).unwrap();
+        assert_eq!(admit(&state).unwrap_err().code(), Some(ErrorCode::Io));
+        assert!(path.is_symlink());
+        assert_eq!(fs::read_to_string(target).unwrap(), "keep");
+    }
+    #[cfg(windows)]
+    {
+        use std::os::windows::process::CommandExt;
+        use std::process::{Command, Stdio};
+
+        let target = state.dir().join("target");
+        fs::create_dir(&target).unwrap();
+        let mut command = Command::new("cmd");
+        command
+            .raw_arg(format!(
+                "/C mklink /J \"{}\" \"{}\"",
+                path.display(),
+                target.display()
+            ))
+            .stdout(Stdio::null());
+        let child = {
+            let _spawn = platform::inherit::lock_spawns();
+            command.spawn().unwrap()
+        };
+        assert!(child.wait_with_output().unwrap().status.success());
+        assert_eq!(admit(&state).unwrap_err().code(), Some(ErrorCode::Io));
+        assert_eq!(fs::read_dir(target).unwrap().count(), 0);
+    }
+}
+
+#[cfg(windows)]
+#[test]
+fn exec_lock_files_accessible_to_other_users_are_refused_without_restriction() {
+    use std::process::{Command, Stdio};
+
+    let (_directory, state) = state();
+    drop(admit(&state).unwrap().unwrap());
+    let path = state.dir().join(NAME);
+    let mut command = Command::new("icacls");
+    command
+        .arg(&path)
+        .args(["/grant", "*S-1-1-0:(F)"])
+        .stdout(Stdio::null());
+    let child = {
+        let _spawn = platform::inherit::lock_spawns();
+        command.spawn().unwrap()
+    };
+    assert!(child.wait_with_output().unwrap().status.success());
+    let problem = platform::private_file_problem(&File::open(&path).unwrap()).unwrap();
+    assert!(problem.is_some());
+    assert_eq!(admit(&state).unwrap_err().code(), Some(ErrorCode::Io));
+    assert_eq!(
+        platform::private_file_problem(&File::open(&path).unwrap()).unwrap(),
+        problem
+    );
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn private_directories_protect_exec_locks_without_changing_the_store_file_policy() {
+    use std::os::unix::fs::PermissionsExt;
+
+    let (_directory, state) = state();
+    drop(admit(&state).unwrap().unwrap());
+    let path = state.dir().join(NAME);
+    fs::set_permissions(&path, fs::Permissions::from_mode(0o644)).unwrap();
+    drop(admit(&state).unwrap().unwrap());
+    assert_eq!(
+        fs::metadata(&path).unwrap().permissions().mode() & 0o777,
+        0o644
+    );
+    fs::set_permissions(state.dir(), fs::Permissions::from_mode(0o755)).unwrap();
+    assert_eq!(admit(&state).unwrap_err().code(), Some(ErrorCode::Io));
+    assert_eq!(
+        fs::metadata(state.dir()).unwrap().permissions().mode() & 0o777,
+        0o755
+    );
+    assert_eq!(
+        fs::metadata(&path).unwrap().permissions().mode() & 0o777,
+        0o644
+    );
+}

nvx-rs/src/host/cli/mod.rs

@@ -42,6 +42,7 @@ mod cpu_profile;
 mod deadline;
 mod download;
 mod egress;
+mod exec_lock;
 mod json;
 mod network;
 mod output;