fix(runtime): VendSecret over mTLS — the boot contract passes end to end

[dshakes]

EXPECT_VENDSECRET=1 passes on real KVM. All four assertions green, twice, with no leaked TAPs:

PASS: Firecracker backend available (Linux + firecracker + /dev/kvm)
PASS: microVM booted AND guest is live
PASS: harness live + secret vended over mTLS (CN=vm_id; value never logged)
PASS: ALL ASSERTIONS PASSED — live Firecracker boot + mTLS VendSecret validated end to end

The security-critical part of the boot contract — a per-VM client cert presented over mTLS and checked against the manager's allowlist — now runs on every boot instead of being skipped.

Three faults, each visible only once the previous cleared

1. tonic silently replaced the TLS config

The generated client's connect() routes through tonic::transport::Endpoint::new, which for an https:// URI does:

return me.tls_config(ClientTlsConfig::new().with_enabled_roots());

It overwrites the configured TLS — and with_enabled_roots discards its own receiver. With neither roots feature enabled that leaves an empty root store, no client identity, no domain.

That is exactly what the guest showed, and it unified three symptoms into one cause:

Symptom	Because
UnknownIssuer	no roots
"client auth requested but no cert available"	no identity
verification against the IP, not the name	no domain

Fix: connect via endpoint.connect() and build the client from the channel. That path doesn't go through Endpoint::new, so the configured TLS survives.

Worth knowing repo-wide: any code doing SomeClient::connect(endpoint) over https with a custom ClientTlsConfig is silently getting default TLS.

2. Declared secrets never reached the guest

The Firecracker backend never put spec.secrets in the boot args, so the harness saw none declared and its cache refused every request — a spec declaring a secret behaved identically to one declaring none. The manager now emits one lantern.secret.<ENV>=<uri> per secret and the harness reassembles the JSON. One key each rather than a JSON blob, because kernel args are whitespace-separated and quotes are stripped from values.

3. Nothing ever populated the secret cache

refresh_loop only renews entries already cached, and get_or_vend is driven by the workload asking over the socket — so a VM with no workload never exercised the vend path at all, which is precisely the path worth verifying. The harness now prefetches declared secrets at boot: best-effort, and spawned so a slow manager can't delay boot.

Tradeoff, stated plainly: this holds a declared-but-unused secret in guest memory for the VM's life. That's already true of any secret the workload touches once, and declared secrets are by definition ones it may use.

Verification

2 consecutive runs, 4/4 assertions each, 0 leaked TAPs. 153 harness + 346 manager tests pass; clippy and rustfmt clean on both crates.

This closes the gap documented on #198.

[github-actions]

🔷 Gemini cross-audit (agent:audit-gemini)

An independent audit of the unified diff in pr-diff.patch against the base codebase has identified one Critical / Blocking security vulnerability, two correctness/safety concerns, and a gap in test coverage.

---

1. Blocking / Critical Security Issue

🚨 Command-Line / Boot-Argument Injection via Unsanitized Whitespace

• Location: services/runtime-manager/src/backends/firecracker.rs (in build_boot_args)

• Impact: Remote Code Execution (RCE) / Privilege Escalation within the guest VM.

• Analysis:
  The new logic dynamically injects declared secrets into the kernel command line:

  let declared_secrets: String = req
      .secrets
      .iter()
      .filter(|s| !s.env_var.is_empty() && !s.vault_ref.is_empty())
      .map(|s| {
          let env = s.env_var.replace([' ', '"', '\'', '='], "_");
          let uri = s.vault_ref.replace([' ', '"', '\''], "_");
          format!(" lantern.secret.{env}={uri}")
      })
      .collect();

  While the code sanitizes space (' '), double quotes ('"'), and single quotes ('\''), it fails to sanitize other whitespace characters such as tabs ('\t'), newlines ('\n'), and carriage returns ('\r').

  Since the Firecracker guest kernel parses command line arguments by splitting on any Unicode whitespace (including tabs), a malicious tenant-supplied vault_ref or env_var can inject arbitrary boot arguments.

  For example, a secret with vault_ref = "uri\tlantern.env.LANTERN_WORKLOAD_CMD=reboot" would serialize into the boot args containing a tab character. Inside the guest, hydrate_env_from_cmdline() parses /proc/cmdline via .split_whitespace(), splitting on the tab and executing the injection.

  Exploitation vectors:

  1. Arbitrary Guest Code Execution: Injecting lantern.env.LANTERN_WORKLOAD_CMD=sh -c "..." to run arbitrary commands as root (PID 1) during VM boot.
  2. Bypassing Socket Peer-Authentication: Injecting lantern.env.LANTERN_WORKLOAD_UID=0 (or another target UID) to subvert the peer-credentials socket check in secrets.rs, allowing unauthorized processes to steal secrets.
  3. MitM / DNS Poisoning: Injecting lantern.env.LANTERN_MANAGER_ADDR to point to an attacker-controlled endpoint.

• Remediation:
  Ensure both env_var and vault_ref are strictly validated (e.g., using an alphanumeric regex) or that all possible whitespace characters (including \t, \n, \r) are stripped/replaced before they are appended to the command line. (Note: The existing extra_env builder in the same file shares this tab/newline injection vulnerability and should be patched too.)

---

2. Correctness & Concurrency Issues

⚠️ Undefined Behavior / Soundness Violation via std::env::set_var

• Location: services/harness/src/bootenv.rs (in assemble_declared_secrets)
• Analysis:
  The harness uses std::env::set_var inside bootstrap() to write LANTERN_DECLARED_SECRETS into the process environment. The code comments assert this is safe:

  // SAFETY: called from main before any thread is spawned.
  unsafe { std::env::set_var("LANTERN_DECLARED_SECRETS", &json) };

  However, bootstrap() is called from inside an async fn main decorated with #[tokio::main]. The macro spawns multi-threaded worker threads before the body of main begins executing. Mutating the environment with set_var while other threads are running (even if idle) is a data race and technically undefined behavior (UB) in Rust (hence why it is unsafe in modern Rust editions).
• Remediation:
  Change #[tokio::main] to a synchronous fn main(), perform the synchronous bootenv::bootstrap() there (when the process is guaranteed to be single-threaded), and then manually build/execute the Tokio runtime to block on the async initialization loop.

⚠️ Cache Stampede / Duplicate RPCs during Warming

• Location: services/harness/src/secrets.rs (in prefetch_declared)
• Analysis:
  prefetch_declared runs as an asynchronous background task at startup. Simultaneously, the guest workload is spawned. If the workload immediately requests a declared secret over the UNIX socket before the prefetch has finished or cached it:
  1. Both the workload request thread and the background prefetch loop will concurrently call get_or_vend() for the same env_name.
  2. Because there is no request coalescing or lock, both threads will concurrently invoke self.manager.vend_secret(...).
  3. This triggers duplicate, redundant mTLS gRPC requests to the manager/vault, creating unnecessary overhead during VM boot.
• Remediation:
  Implement a lock, a singleflight mechanism, or use a state machine to track in-flight secret-vending requests and merge duplicate concurrent calls.

---

3. Missed Edge Cases & Test Coverage

🔍 No Tests for assemble_declared_secrets()

• Location: services/harness/src/bootenv.rs
• Analysis:
  While the existing bootenv.rs has high-quality unit tests verifying the translation of command-line environment variables, no unit test was added to verify assemble_declared_secrets()'s parser logic, string splitting, or correct JSON formatting.
• Remediation:
  Add a unit test checking that assemble_declared_secrets correctly parses lantern.secret. keys from dummy command lines and successfully serializes them into the expected SecretRef JSON structure.

---

4. Positive Feedback (Missed by Prior Review)

👏 The endpoint.connect() Tonic Client fix is Outstanding

• Location: services/harness/src/manager_client.rs
• Analysis:
  The modification replacing pb::runtime_harness_client::RuntimeHarnessClient::connect(endpoint) with endpoint.connect().await is a very deep and accurate fix. As documented in the code comments, Tonic’s Client::connect internally re-creates the endpoint from its URI which silently overwrites any custom ClientTlsConfig we applied. Connecting on the endpoint directly and passing the resulting channel to ::new() is the correct and robust way to preserve mTLS configuration.