Merge: B4.4 microVM credential injection over vsock

crates/bins/clawmates-node/src/microvm.rs

@@ -328,17 +328,24 @@ pub async fn inject(vms: &Vms, vm_id: &str, dest: &str, tar_b64: &str) -> Result
 }
 
 /// Run a command in the guest and return its exit code and output.
+/// `env` is passed straight to the guest, which validates it and refuses the
+/// exec if any entry is unusable. It is NOT logged here or anywhere on the way:
+/// this is the channel credentials travel on.
 pub async fn exec(
     vms: &Vms,
     vm_id: &str,
     cmd: &str,
     cwd: Option<&str>,
     timeout_secs: u64,
+    env: Option<&Value>,
 ) -> Result<Value, String> {
     let uds = uds_of(vms, vm_id).await?;
     rpc(
         &uds,
-        &json!({ "op": "exec", "cmd": cmd, "cwd": cwd, "timeout": timeout_secs }),
+        &json!({
+            "op": "exec", "cmd": cmd, "cwd": cwd,
+            "timeout": timeout_secs, "env": env,
+        }),
     )
     .await
 }
@@ -431,7 +438,15 @@ pub async fn handle_op(op: &str, v: &Value, vms: &Vms) -> (bool, String) {
         "vm_inject" => inject(vms, &vm_id, &s("dest"), &s("tar_b64")).await,
         "vm_exec" => {
             let cwd = v.get("cwd").and_then(Value::as_str);
-            exec(vms, &vm_id, &s("cmd"), cwd, u("timeout", 3600)).await
+            exec(
+                vms,
+                &vm_id,
+                &s("cmd"),
+                cwd,
+                u("timeout", 3600),
+                v.get("env"),
+            )
+            .await
         }
         "vm_collect" => collect(vms, &vm_id, &s("path")).await,
         "vm_destroy" => destroy(vms, &vm_id).await,
@@ -535,7 +550,7 @@ pub async fn selftest() -> bool {
 
     // The guest must SEE what we injected — an inject that reports ok while
     // landing nothing is the failure shape this codebase keeps paying for.
-    let r = exec(&vms, id, "cat /work/marker.txt", None, 30).await;
+    let r = exec(&vms, id, "cat /work/marker.txt", None, 30, None).await;
     let saw = r
         .as_ref()
         .map(|v| v["stdout"].as_str().unwrap_or_default().contains("INJECTED-OK"))
@@ -544,15 +559,57 @@ pub async fn selftest() -> bool {
 
     // A failing command must come back as rc != 0, not as a transport error:
     // the caller needs to tell "the command failed" from "we could not run it".
-    let r = exec(&vms, id, "exit 3", None, 30).await;
+    let r = exec(&vms, id, "exit 3", None, 30, None).await;
     check(
         r.as_ref().map(|v| v["rc"] == json!(3)).unwrap_or(false),
         "a failing command reports rc=3 rather than an error",
         format!("{r:?}"),
     );
 
+    // Credentials reach the agent CLI as exec env, and this is the only place
+    // that is proven over the real vsock wire rather than in a unit test: the
+    // failure it guards against is a `claude -p` with no token, which does not
+    // error — it hangs.
+    let r = exec(
+        &vms,
+        id,
+        "printf %s \"$CLAWMATES_ENV_PROBE\"",
+        None,
+        30,
+        Some(&json!({ "CLAWMATES_ENV_PROBE": "env-injection-ok" })),
+    )
+    .await;
+    check(
+        r.as_ref()
+            .map(|v| v["stdout"] == json!("env-injection-ok"))
+            .unwrap_or(false),
+        "injected env reaches the guest command",
+        format!("{r:?}"),
+    );
+
+    // And an entry the guest cannot honour must fail the exec rather than run
+    // the command without it.
+    let r = exec(
+        &vms,
+        id,
+        "true",
+        None,
+        30,
+        Some(&json!({ "BAD=NAME": "x" })),
+    )
+    .await;
+    let refused = r
+        .as_ref()
+        .map(|v| v["ok"] == json!(false) && v["rc"].is_null())
+        .unwrap_or(false);
+    check(
+        refused,
+        "an unusable env entry refuses the exec instead of dropping it",
+        format!("{r:?}"),
+    );
+
     // Work produced in the guest must come back out.
-    let _ = exec(&vms, id, "echo PRODUCED-OK > /work/out.txt", None, 30).await;
+    let _ = exec(&vms, id, "echo PRODUCED-OK > /work/out.txt", None, 30, None).await;
     let r = collect(&vms, id, "/work").await;
     let round_tripped = r
         .as_ref()
@@ -580,7 +637,7 @@ pub async fn selftest() -> bool {
     // and it is exec'd inside the guest rather than inferred from the image name.
     match required_cli(backend.as_deref()) {
         Some((cli, probe)) => {
-            let r = exec(&vms, id, probe, None, 60).await;
+            let r = exec(&vms, id, probe, None, 60, None).await;
             let (rc, out) = match r.as_ref() {
                 Ok(v) => (
                     v["rc"].as_i64(),
@@ -599,7 +656,7 @@ pub async fn selftest() -> bool {
             // git is what delivery is built on: the host captures a phase by
             // diffing the collected tree, so an image without git delivers
             // nothing no matter which CLI it has.
-            let r = exec(&vms, id, "git --version", None, 30).await;
+            let r = exec(&vms, id, "git --version", None, 30, None).await;
             check(
                 r.as_ref().map(|v| v["rc"] == json!(0)).unwrap_or(false),
                 "the guest provides git",
@@ -632,7 +689,7 @@ pub async fn selftest() -> bool {
 
     // And the VM must not be usable afterwards — a destroy that leaves a live
     // guest answering is worse than one that errors.
-    let r = exec(&vms, id, "echo still-here", None, 5).await;
+    let r = exec(&vms, id, "echo still-here", None, 5, None).await;
     check(
         r.is_err(),
         "a destroyed VM can no longer be exec'd",

crates/bins/fcagent/src/main.rs

@@ -135,6 +135,56 @@ fn handle(req: &Value) -> Value {
     }
 }
 
+/// Extra environment for the command, on top of the image's own.
+///
+/// This is how credentials reach the agent CLI. An env var rather than a file
+/// because the per-VM rootfs is destroyed with the VM but an env var never
+/// touches the guest disk at all — it exists only in the process's environment
+/// for the length of one exec.
+///
+/// **Every problem here fails the exec.** The tempting alternative — skip the
+/// entry we could not use and run anyway — produces a `claude -p` with no
+/// credential, and that does not error: it hangs. A phase stuck at `running`
+/// for ten minutes with nothing in the logs is exactly what a missing token
+/// looked like on the container path, so a request we cannot honour in full is
+/// refused with a reason instead.
+///
+/// Errors name the key and never the value: the value is the secret, and an
+/// error string travels back over the wire and into logs.
+fn env_pairs(req: &Value) -> Result<Vec<(String, String)>, String> {
+    let Some(env) = req.get("env") else {
+        return Ok(Vec::new());
+    };
+    // `null` means "nothing to add" — that is what a caller with no credentials
+    // serialises. Anything else that is not an object is a caller bug.
+    if env.is_null() {
+        return Ok(Vec::new());
+    }
+    let Some(map) = env.as_object() else {
+        return Err("exec env must be an object of name → string".into());
+    };
+    let mut out = Vec::with_capacity(map.len());
+    for (k, v) in map {
+        let Some(val) = v.as_str() else {
+            return Err(format!("exec env {k}: value must be a string"));
+        };
+        // `putenv` semantics: a name containing '=' would be parsed as part of
+        // the value, silently defining a different variable than the one asked
+        // for. A NUL truncates at the C boundary, for the same class of reason.
+        if k.is_empty() {
+            return Err("exec env has an empty variable name".into());
+        }
+        if k.contains('=') || k.contains('\0') {
+            return Err(format!("exec env {k:?}: name may not contain '=' or NUL"));
+        }
+        if val.contains('\0') {
+            return Err(format!("exec env {k}: value may not contain NUL"));
+        }
+        out.push((k.clone(), val.to_string()));
+    }
+    Ok(out)
+}
+
 fn op_exec(req: &Value) -> Value {
     let cmd = req.get("cmd").and_then(Value::as_str).unwrap_or_default();
     if cmd.is_empty() {
@@ -142,6 +192,10 @@ fn op_exec(req: &Value) -> Value {
     }
     let cwd = req.get("cwd").and_then(Value::as_str).unwrap_or("/");
     let secs = req.get("timeout").and_then(Value::as_u64).unwrap_or(3600);
+    let env = match env_pairs(req) {
+        Ok(v) => v,
+        Err(e) => return json!({ "ok": false, "error": e }),
+    };
 
     // The image's ENV was written to /etc/profile.d by the rootfs builder;
     // `sh -c` does not read it, so source it here — otherwise a CLI that relies
@@ -158,6 +212,7 @@ fn op_exec(req: &Value) -> Value {
     let mut c = Command::new("/bin/sh");
     c.arg("-c")
         .arg(&sourced)
+        .envs(env)
         .current_dir(if Path::new(cwd).is_dir() { cwd } else { "/" })
         .stdin(Stdio::null())
         .stdout(Stdio::piped())
@@ -292,6 +347,84 @@ fn op_get(req: &Value) -> Value {
 mod tests {
     use super::*;
 
+    /// The credential has to actually reach the command. This is the whole
+    /// point of the op, and the failure it prevents is silent: a `claude -p`
+    /// with no token hangs rather than erroring.
+    #[test]
+    fn injected_env_reaches_the_command() {
+        let r = op_exec(&json!({
+            "op": "exec",
+            "cmd": "printf %s \"$CLAUDE_CODE_OAUTH_TOKEN\"",
+            "env": { "CLAUDE_CODE_OAUTH_TOKEN": "sk-test-value" },
+            "timeout": 30,
+        }));
+        assert_eq!(r["rc"], json!(0));
+        assert_eq!(r["stdout"], json!("sk-test-value"));
+    }
+
+    /// And it must survive the profile.d sourcing that runs first — a
+    /// credential set on the process and then clobbered by the shell would
+    /// look identical to one that never arrived.
+    #[test]
+    fn injected_env_survives_the_image_env_file() {
+        let r = op_exec(&json!({
+            "op": "exec",
+            "cmd": "printf %s \"$INJECTED_PROBE\"",
+            "env": { "INJECTED_PROBE": "still-here" },
+            "timeout": 30,
+        }));
+        assert_eq!(r["stdout"], json!("still-here"));
+    }
+
+    /// No env is the ordinary case and must not be an error.
+    #[test]
+    fn absent_or_null_env_is_not_an_error() {
+        for req in [
+            json!({ "op": "exec", "cmd": "true", "timeout": 30 }),
+            json!({ "op": "exec", "cmd": "true", "env": null, "timeout": 30 }),
+            json!({ "op": "exec", "cmd": "true", "env": {}, "timeout": 30 }),
+        ] {
+            assert_eq!(op_exec(&req)["rc"], json!(0), "{req}");
+        }
+    }
+
+    /// An env entry we cannot honour fails the whole exec rather than being
+    /// dropped. Running without the credential is the outcome this refuses:
+    /// it does not error, it hangs, which is far harder to diagnose than a
+    /// rejected request.
+    #[test]
+    fn an_unusable_env_entry_fails_the_exec_instead_of_being_skipped() {
+        let cases = [
+            json!({ "A=B": "x" }),
+            json!({ "": "x" }),
+            json!({ "TOKEN": 42 }),
+            json!({ "TOKEN": null }),
+        ];
+        for env in cases {
+            let r = op_exec(&json!({
+                "op": "exec", "cmd": "true", "env": env.clone(), "timeout": 30,
+            }));
+            assert_eq!(r["ok"], json!(false), "env {env} should be refused");
+            assert!(r["rc"].is_null(), "nothing ran, so there is no rc: {r}");
+        }
+        // A non-object env is a caller bug, not an empty map.
+        let r = op_exec(&json!({ "op": "exec", "cmd": "true", "env": "TOKEN=x" }));
+        assert_eq!(r["ok"], json!(false));
+    }
+
+    /// An error about a credential must not quote the credential: it travels
+    /// back over the wire and into the server's logs.
+    #[test]
+    fn an_env_error_never_echoes_the_value() {
+        let r = op_exec(&json!({
+            "op": "exec", "cmd": "true", "timeout": 30,
+            "env": { "A=B": "super-secret-token" },
+        }));
+        let err = r["error"].as_str().unwrap_or_default();
+        assert!(!err.contains("super-secret-token"), "leaked the value: {err}");
+        assert!(err.contains("A=B"), "should name the key: {err}");
+    }
+
     #[test]
     fn an_unknown_op_is_reported_not_ignored() {
         let r = handle(&json!({ "op": "teleport" }));

crates/cm-api/src/microvm_client.rs

@@ -127,19 +127,41 @@ impl<'a> MicroVm<'a> {
     /// exit is not an error here — the caller has to be able to tell "the build
     /// failed" from "we could not reach the VM", and collapsing them is the
     /// defect this codebase keeps paying for.
+    /// `env` carries the provider credentials (see
+    /// [`crate::mission_runtime::forwarded_provider_env`]). It is sent, never
+    /// logged: this is the only channel by which a secret reaches the guest, and
+    /// the guest refuses the exec rather than running a command without an entry
+    /// it could not honour.
     pub async fn exec(
         &self,
         cmd: &str,
         cwd: Option<&str>,
         timeout_secs: u64,
+        env: &[(String, String)],
     ) -> Result<ExecOut, String> {
+        let env: Option<Value> = (!env.is_empty()).then(|| {
+            env.iter()
+                .map(|(k, v)| (k.clone(), Value::String(v.clone())))
+                .collect::<serde_json::Map<_, _>>()
+                .into()
+        });
         let v = self
             .call(
                 "vm_exec",
-                json!({ "cmd": cmd, "cwd": cwd, "timeout": timeout_secs }),
+                json!({ "cmd": cmd, "cwd": cwd, "timeout": timeout_secs, "env": env }),
                 hub_deadline(timeout_secs),
             )
             .await?;
+        // A guest that refused to run the command reports `ok: false` and no rc
+        // — a rejected env entry, for instance. Surface its reason: falling
+        // through to the missing-rc error below would hide the cause behind a
+        // symptom.
+        if v.get("ok").and_then(Value::as_bool) == Some(false) {
+            return Err(format!(
+                "vm_exec did not run: {}",
+                v.get("error").and_then(Value::as_str).unwrap_or("unknown")
+            ));
+        }
         // A missing rc is not "success" — it means the guest did not report one,
         // which we must not read as zero.
         let rc = v

crates/cm-api/src/mission_runtime.rs

@@ -108,6 +108,38 @@ pub fn forwarded_provider_keys(auth: RuntimeAuth) -> Vec<&'static str> {
     keys
 }
 
+/// The forwarded credentials that are actually SET on this server, as pairs.
+///
+/// The container path and the microVM path must agree on which credentials
+/// travel, or a mission behaves differently depending on where it landed —
+/// including the expensive way, where one path forwards `ANTHROPIC_API_KEY` and
+/// bills the API while the other uses the subscription. So both read this, and
+/// `forwarded_provider_keys` above stays the single list.
+///
+/// A key that is unset is simply absent: on the microVM path an empty-string
+/// value would make `claude` believe it has a credential and fail
+/// authentication instead of reporting that it has none.
+pub fn forwarded_provider_env(auth: RuntimeAuth) -> Vec<(String, String)> {
+    provider_env_from(auth, |k| std::env::var(k).ok())
+}
+
+/// The testable half of [`forwarded_provider_env`]. The lookup is a parameter
+/// because a test cannot set process environment variables here — the workspace
+/// denies `unsafe`, and `set_var` is racy across test threads regardless.
+fn provider_env_from(
+    auth: RuntimeAuth,
+    lookup: impl Fn(&str) -> Option<String>,
+) -> Vec<(String, String)> {
+    forwarded_provider_keys(auth)
+        .into_iter()
+        .filter_map(|k| {
+            lookup(k)
+                .filter(|v| !v.trim().is_empty())
+                .map(|v| (k.to_string(), v))
+        })
+        .collect()
+}
+
 /// Read `CLAWMATES_RUNTIME_AUTH`, defaulting to `api_key`.
 ///
 /// Defaulting to the existing behaviour is deliberate: an unset or misspelled
@@ -486,10 +518,8 @@ impl MissionRuntimeProvisioner {
         // The other three are unrelated providers (Gemini/Groq/OpenAI) with no
         // subscription equivalent, so they forward in both modes.
         let auth_mode = runtime_auth_mode();
-        for key in forwarded_provider_keys(auth_mode) {
+        for (key, v) in forwarded_provider_env(auth_mode) {
-            if let Ok(v) = std::env::var(key) {
+            env.push(format!("{key}={v}"));
-                env.push(format!("{key}={v}"));
-            }
         }
         eprintln!(
             "mission_runtime: mission {mission_id} container auth mode = {} \
@@ -959,6 +989,38 @@ mod tests {
     /// means every mission silently bills the API while looking correct. There
     /// is no error to observe — only the invoice. If this test ever goes red,
     /// the subscription path is off even though nothing appears broken.
+    /// The container path and the microVM path must forward the SAME set. If
+    /// they diverge, a mission behaves differently depending on where it landed
+    /// — including the expensive way, where one path forwards the API key and
+    /// bills it while the other uses the subscription.
+    #[test]
+    fn both_execution_paths_forward_the_same_credentials() {
+        for auth in [RuntimeAuth::ApiKey, RuntimeAuth::Subscription] {
+            let keys = forwarded_provider_keys(auth);
+            // Every key set in the environment, and nothing else.
+            let pairs = provider_env_from(auth, |k| Some(format!("value-of-{k}")));
+            let got: Vec<&str> = pairs.iter().map(|(k, _)| k.as_str()).collect();
+            assert_eq!(got, keys, "{}", auth.as_str());
+            for (k, v) in &pairs {
+                assert_eq!(v, &format!("value-of-{k}"), "value must pass through");
+            }
+        }
+    }
+
+    /// An unset or blank credential is ABSENT, not empty. On the microVM path an
+    /// empty string would make `claude` believe it has a credential and fail
+    /// authentication, instead of reporting that it has none.
+    #[test]
+    fn a_blank_credential_is_omitted_rather_than_forwarded_empty() {
+        let pairs = provider_env_from(RuntimeAuth::Subscription, |k| match k {
+            "CLAUDE_CODE_OAUTH_TOKEN" => Some("   ".into()),
+            "OPENAI_API_KEY" => Some(String::new()),
+            "GEMINI_API_KEY" => Some("real".into()),
+            _ => None,
+        });
+        assert_eq!(pairs, vec![("GEMINI_API_KEY".to_string(), "real".to_string())]);
+    }
+
     #[test]
     fn subscription_mode_withholds_the_anthropic_api_key() {
         let keys = forwarded_provider_keys(RuntimeAuth::Subscription);