feat(agentd): an image-set default user for execs that omit user (#595)

.config/mise/tasks/ci.toml

@@ -161,6 +161,10 @@ description = "CI's `mutants` job's command: the wrapper over cargo-mutants. The
 # `--in-place` mutates this checkout, which the runner throws away, so rust-cache can restore
 # dependencies into its target. Run from a worktree you keep, pass it without `--in-place`, or
 # run `mise run mutants`.
+# The CLI's parity test spawns microvms-core's `parity-tunnel-peer` example from the test
+# binary's target directory, and a mutant's `cargo test -p microvms-cli` never builds another
+# package's example, so the fixture is built first into the target `--in-place` tests use.
+depends = ["parity:fixtures"]
 tools = { "cargo:cargo-mutants" = "{{vars.cargo_mutants}}" }
 run = "./tools/check-mutants.py"
 

.config/trivyignore.yaml

@@ -43,7 +43,8 @@ misconfigurations:
       (docs/TRUST.md), and the MicroVM, one tenant per Firecracker VM with its
       own kernel, is the isolation boundary for the host. DS-0002's goal, an
       unprivileged workload process, applies to agentd's execs: an exec runs
-      as the user its caller names, and as root when it names none. Revisit
+      as the user its caller names, as the image's AGENTD_DEFAULT_USER when
+      it names none and the image sets one, and as root otherwise. Revisit
       this exception if agentd stops needing to demote execs. The
       strands-box-codex-sdk example also needs root at start to give its
       sandbox a private mount namespace with a fresh /proc before

changelog.d/595.added.md

@@ -0,0 +1,16 @@
+- **An image can set the user an exec without `user` runs as (#595).** The daemon reads
+  `AGENTD_DEFAULT_USER` from the image's environment at startup: a user name, or a uid as
+  digits. A start request that omits `user`, from `exec` or `run --exec` alike, resolves it at
+  each start to a row of the guest's `/etc/passwd`, by name or by uid, so a user an earlier exec
+  created resolves on the next. The child gets that row's uid, primary gid, `HOME`, `USER` and
+  `LOGNAME`, a uid included. A request's own `user` still runs as given, `root` and `0`
+  included. If no row matches, digits included, each start without a `user` answers
+  `400 unknown_user`, and the detail mentions the variable. Uploads don't follow the variable:
+  what `PUT /v1/fs/tar` and `PUT /v1/fs/file` create stays root's, so the default user can read
+  the tree `run . --exec` and `cp` upload but can't write into it. An exec that must write there
+  runs with `user: 0`, or the image or an exec with `user: 0` hands the tree to the default user
+  with `chown` first. With the variable unset, nothing changes. Workload hook handlers still run
+  as root. The agent recipe's `chown` of `/workspace` and a sync's in-guest deletions now ask
+  for uid 0 by number, so an image's default user can't make them fail. An image that sets the
+  variable needs a client at this release or later: an older client sends that `chown` and those
+  deletions with no `user`, so they run as the default user and fail.

crates/agentd/src/config.rs

@@ -93,8 +93,25 @@ pub struct Config {
     /// The group database a start request's group name is resolved against. `/etc/group`
     /// outside tests, for `passwd_path`'s reason.
     pub group_path: std::path::PathBuf,
+    /// The user a start request runs as when it names no `user`, from
+    /// [`DEFAULT_USER_VAR`] (AGENTD-22). `None` keeps the daemon's own user, which is root.
+    ///
+    /// Kept as the image wrote it and resolved per request exactly as a request's `user`
+    /// string is: a passwd name, or digits that name no row as a uid. Per request because a
+    /// user an earlier exec created resolves on the next. A request's own `user` always wins,
+    /// `root` and `0` included (AGENTD-23), so an authorized caller can still ask for root.
+    ///
+    /// Read from the image's environment like every other `AGENTD_*` setting, and never from
+    /// a request or the launch environment: who a workload runs as when its caller doesn't
+    /// say is the image author's decision. Workload hook handlers don't take it; they are
+    /// image-owned code and run as the daemon's user.
+    pub default_user: Option<String>,
 }
 
+/// The variable naming [`Config::default_user`]. A refusal of the default names it, so an
+/// image author who misspelled the user can find where it was set.
+pub const DEFAULT_USER_VAR: &str = "AGENTD_DEFAULT_USER";
+
 /// The handler budget's ceiling: five seconds under the platform's 60 s hook limit.
 pub const MAX_HOOK_HANDLER_SECS: u64 = 55;
 
@@ -128,6 +145,7 @@ impl Default for Config {
             hook_handler_timeout: Duration::from_secs(20),
             passwd_path: std::path::PathBuf::from("/etc/passwd"),
             group_path: std::path::PathBuf::from("/etc/group"),
+            default_user: None,
         }
     }
 }
@@ -175,10 +193,32 @@ impl Config {
         if let Some(secs) = env_parse::<u64>("AGENTD_HOOK_HANDLER_TIMEOUT_SECS") {
             cfg.hook_handler_timeout = Duration::from_secs(secs.clamp(1, MAX_HOOK_HANDLER_SECS));
         }
+        cfg.default_user = default_user(std::env::var_os(DEFAULT_USER_VAR));
+        // Logged to the image's log group because `/v1/health` can't carry it yet: `Health` is
+        // constructible field by field, so a new field is a breaking change to the protocol
+        // crate.
+        if let Some(user) = &cfg.default_user {
+            tracing::info!(
+                default_user = %user,
+                "a start request that omits user runs as {DEFAULT_USER_VAR}"
+            );
+        }
         cfg
     }
 }
 
+/// [`DEFAULT_USER_VAR`] as the daemon keeps it: set means set, an empty value included.
+///
+/// Unlike the numeric knobs, a value that names nobody is not ignored. It resolves the way a
+/// request's `user` string does, so an empty or misspelled one refuses every start that
+/// names no user, naming the variable, instead of quietly running the workload as root. That
+/// costs no reachability: the daemon still boots, and a request that names its user still
+/// runs. A value that isn't UTF-8 is kept lossily, which no passwd name matches, so it is
+/// refused the same way rather than dropped.
+fn default_user(raw: Option<std::ffi::OsString>) -> Option<String> {
+    raw.map(|raw| raw.to_string_lossy().into_owned())
+}
+
 /// Parses a boolean knob, accepting the spellings a Dockerfile or a Terraform
 /// template actually produces. `bool::from_str` takes only `true`/`false`, so
 /// `AGENTD_REPAIR_IDENTITY=0` would otherwise be ignored with a warning — and a
@@ -219,5 +259,44 @@ mod tests {
         // single noisy command can kill a daemon nothing can restart.
         assert!(cfg.max_output_bytes < 64 * 1024 * 1024);
         assert!(cfg.max_drain_bytes < cfg.max_body_bytes);
+        assert_eq!(cfg.default_user, None, "unset keeps the daemon's own user");
+    }
+
+    /// **AGENTD-22.** The default user is kept as the image set it: unset is none, and any
+    /// set value, empty or not UTF-8 included, is kept to be resolved, so a value that names
+    /// nobody is refused at the first start instead of falling back to root.
+    #[test]
+    fn the_default_user_is_kept_as_set() {
+        use std::os::unix::ffi::OsStringExt;
+        assert_eq!(default_user(None), None);
+        assert_eq!(default_user(Some("agent".into())), Some("agent".into()));
+        assert_eq!(default_user(Some("1000".into())), Some("1000".into()));
+        assert_eq!(default_user(Some("".into())), Some(String::new()));
+        let lossy = default_user(Some(std::ffi::OsString::from_vec(vec![b'a', 0xff])))
+            .expect("a non-UTF-8 value is kept");
+        assert_eq!(lossy, "a\u{fffd}");
+    }
+
+    /// **AGENTD-22.** `from_env` reads the default user from the daemon's environment. The
+    /// read happens in a copy of this test binary started with the variable set, because this
+    /// crate can't set its own environment: `set_var` is `unsafe`, which the crate forbids.
+    #[test]
+    fn from_env_reads_the_default_user_from_the_environment() {
+        const CHILD: &str = "AGENTD_TEST_FROM_ENV_CHILD";
+        const NAME: &str = "config::tests::from_env_reads_the_default_user_from_the_environment";
+        if std::env::var_os(CHILD).is_some() {
+            assert_eq!(Config::from_env().default_user.as_deref(), Some("probe"));
+            return;
+        }
+        let output = std::process::Command::new(std::env::current_exe().expect("test binary"))
+            .args(["--exact", NAME])
+            .env(CHILD, "1")
+            .env(DEFAULT_USER_VAR, "probe")
+            .output()
+            .expect("child test");
+        let stdout = String::from_utf8_lossy(&output.stdout);
+        assert!(output.status.success(), "{stdout}");
+        // A filter that matched nothing would pass too, having checked nothing.
+        assert!(stdout.contains("1 passed"), "{stdout}");
     }
 }

crates/agentd/src/exec.rs

@@ -1202,10 +1202,13 @@ fn build_command(
     let launch_env = state.launch_env();
     let config = state.config();
     // Read per request rather than cached: a workload that runs `useradd` before its next
-    // exec is asking for exactly that user, and the files are a few kilobytes.
-    let passwd = match req.user {
-        Some(_) => read_database(&config.passwd_path),
-        None => String::new(),
+    // exec is asking for exactly that user, and the files are a few kilobytes. The image's
+    // default user is read the same way, for the same reason (AGENTD-22).
+    let default_user = config.default_user.as_deref();
+    let passwd = if req.user.is_some() || default_user.is_some() {
+        read_database(&config.passwd_path)
+    } else {
+        String::new()
     };
     let group = match req.group {
         Some(protocol::exec::NameOrId::Name(_)) => read_database(&config.group_path),
@@ -1218,6 +1221,7 @@ fn build_command(
             image_env: state.image_env(),
             passwd: &passwd,
             group: &group,
+            default_user,
             is_executable: &is_executable_file,
         },
     )?;
@@ -2946,6 +2950,15 @@ mod tests {
     /// `/home/tester`) and whose group file lists `crew` (this process's gid), holding
     /// `inherited` as the environment it inherited at startup, bootstrapped with `launch`.
     fn guest(inherited: &[(&str, &str)], launch: &[(&str, &str)]) -> (AppState, tempfile::TempDir) {
+        guest_with_default(inherited, launch, None)
+    }
+
+    /// [`guest`] whose image sets `AGENTD_DEFAULT_USER` to `default_user`.
+    fn guest_with_default(
+        inherited: &[(&str, &str)],
+        launch: &[(&str, &str)],
+        default_user: Option<&str>,
+    ) -> (AppState, tempfile::TempDir) {
         let dir = tempfile::tempdir().expect("a guest directory");
         let (uid, gid) = (own_id("-u"), own_id("-g"));
         std::fs::write(
@@ -2966,6 +2979,7 @@ mod tests {
         let config = Config {
             passwd_path: dir.path().join("passwd"),
             group_path: dir.path().join("group"),
+            default_user: default_user.map(str::to_string),
             ..Config::default()
         };
         let state = AppState::with_image_env(config, launch_env(inherited));
@@ -3075,6 +3089,102 @@ mod tests {
         assert_eq!(outcome.stdout.trim(), "/from-launch tester");
     }
 
+    /// **AGENTD-22, AGENTD-24.** Through the real handler, a start that names no user runs as
+    /// the image's default user, with that row's identity. The default is resolved at each
+    /// request: while the guest has no such user the start is 400 `unknown_user` naming the
+    /// variable and nothing runs, and once a row for it appears the next start runs as it.
+    #[tokio::test]
+    async fn a_start_that_names_no_user_runs_as_the_default_resolved_per_request() {
+        let probe = "echo \"$(id -u) $HOME $USER\"";
+        let (state, _dir) = guest_with_default(&[], &[], Some("tester"));
+        let mut request = req("default-tester", &[probe]);
+        request.shell = true.into();
+        let outcome = started(&state, request).await;
+        assert_eq!(
+            outcome.stdout.trim(),
+            format!("{} /home/tester tester", own_id("-u"))
+        );
+
+        let (state, dir) = guest_with_default(&[], &[], Some("latecomer"));
+        let marker = dir.path().join("ran");
+        let mut request = req("default-missing", &[&format!("touch {}", marker.display())]);
+        request.shell = true.into();
+        let detail = refused(&state, request, "unknown_user", &marker).await;
+        assert!(
+            detail.contains("\"latecomer\"") && detail.contains("AGENTD_DEFAULT_USER"),
+            "{detail}"
+        );
+
+        // What a `useradd` in an earlier exec leaves behind.
+        let mut passwd = std::fs::read_to_string(dir.path().join("passwd")).expect("passwd");
+        passwd.push_str(&format!(
+            "latecomer:x:{}:{}::/home/latecomer:/bin/sh\n",
+            own_id("-u"),
+            own_id("-g")
+        ));
+        std::fs::write(dir.path().join("passwd"), passwd).expect("passwd");
+        let mut request = req("default-late", &[probe]);
+        request.shell = true.into();
+        let outcome = started(&state, request).await;
+        assert_eq!(
+            outcome.stdout.trim(),
+            format!("{} /home/latecomer latecomer", own_id("-u"))
+        );
+    }
+
+    /// **AGENTD-22, AGENTD-24.** Through the real handler, a default spelled as digits runs as
+    /// the row of that uid, with its identity, and digits no row carries are refused naming
+    /// the variable and the missing row, where a request's own digits would run.
+    #[tokio::test]
+    async fn a_numeric_default_runs_as_its_row_and_one_with_no_row_is_refused() {
+        let probe = "echo \"$(id -u) $(id -g) $HOME $USER\"";
+        let uid = own_id("-u").to_string();
+        let (state, _dir) = guest_with_default(&[], &[], Some(&uid));
+        let mut request = req("default-digits", &[probe]);
+        request.shell = true.into();
+        let outcome = started(&state, request).await;
+        assert_eq!(
+            outcome.stdout.trim(),
+            format!("{} {} /home/tester tester", own_id("-u"), own_id("-g"))
+        );
+
+        let (state, dir) = guest_with_default(&[], &[], Some("4242424"));
+        let marker = dir.path().join("ran");
+        let mut request = req(
+            "default-digits-no-row",
+            &[&format!("touch {}", marker.display())],
+        );
+        request.shell = true.into();
+        let detail = refused(&state, request, "unknown_user", &marker).await;
+        assert!(
+            detail.contains("\"4242424\"")
+                && detail.contains("AGENTD_DEFAULT_USER")
+                && detail.contains("needs a passwd row"),
+            "{detail}"
+        );
+    }
+
+    /// **AGENTD-23.** A user the start names wins over the default. The default here is an
+    /// account this non-root test cannot become (uid 65534), so a daemon that applied it
+    /// would fail the spawn; naming `tester` runs as `tester`, by name and by uid.
+    #[tokio::test]
+    async fn a_user_the_start_names_wins_over_the_default() {
+        let (state, dir) = guest_with_default(&[], &[], Some("elsewhere"));
+        let mut passwd = std::fs::read_to_string(dir.path().join("passwd")).expect("passwd");
+        passwd.push_str("elsewhere:x:65534:65534::/nonexistent:/bin/sh\n");
+        std::fs::write(dir.path().join("passwd"), passwd).expect("passwd");
+
+        let mut request = req("named-wins", &["/bin/sh", "-c", "echo $(id -u) $USER"]);
+        request.user = Some("tester".into());
+        let outcome = started(&state, request).await;
+        assert_eq!(outcome.stdout.trim(), format!("{} tester", own_id("-u")));
+
+        let mut request = req("uid-wins", &["/bin/sh", "-c", "echo $(id -u) $USER"]);
+        request.user = Some(own_id("-u").into());
+        let outcome = started(&state, request).await;
+        assert_eq!(outcome.stdout.trim(), format!("{} tester", own_id("-u")));
+    }
+
     /// **AGENTD-14.** `shell: "bash"` runs the script under bash: a pipefail pipeline fails
     /// where a POSIX `sh` that is dash would refuse the option outright, and `$0` names bash.
     #[tokio::test]