setup: keep guest hostname resolvable

CHANGELOG.md

@@ -83,6 +83,18 @@
 
 ### Fixes
 
+- **The guest hostname resolves, so `sudo` stops warning** — Instance creation
+  renamed the Firecracker guest to `claude-<name>` in `/etc/hostname` but left
+  the image's `127.0.1.1 claude-vm` entry in `/etc/hosts`, so every `sudo` in
+  the guest printed `sudo: unable to resolve host claude-<name>` before running.
+  Both files are now written together at create and restore, and the guest
+  hostname is clamped to fit the kernel's 64-byte hostname limit so long
+  instance names still get a resolvable name. No image rebuild is needed — the patch is
+  per-instance, and the image's own entry is what gets overwritten — but
+  `patch_guest_network` runs only on create and restore, so an existing VM
+  keeps the stale entry until `coop restore <vm> --image <image>` or a destroy
+  and recreate.
+
 - **Install Codex's complete runtime package** (#442) — Recent Codex releases
   use a companion `codex-code-mode-host` executable, but coop installed only
   the raw `codex` binary, causing Code Mode to fail closed at startup. Image

docs/backends.md

@@ -77,7 +77,7 @@ All three steps are idempotent. If the artifact already exists and is up to date
 Creating an instance (`coop up`) follows this sequence:
 
 1. Copies the template rootfs to the instance directory using `cp --reflink=auto` for copy-on-write on supported filesystems.
-2. Mounts the copy and patches the guest network config with the instance's unique IP address and hostname.
+2. Mounts the copy and patches the guest network config with the instance's unique IP address, plus `/etc/hostname` and the matching `/etc/hosts` alias so the guest can resolve its own name.
 3. Optionally resizes the rootfs if a larger disk was requested (truncate + e2fsck + resize2fs).
 4. Writes a Firecracker JSON config specifying the kernel, rootfs drive, vCPU/memory allocation, network interface, and vsock device.
 5. Creates and attaches a TAP device to the bridge (see TAP networking below).

docs/trust-model.md

@@ -49,6 +49,16 @@ user launched it.
   `tar_pipe_pull` / `rsync_pull` bring guest-authored file contents, filenames,
   and symlinks onto the host filesystem. This is the **widest guest→host
   channel** and the primary place a path-traversal or symlink escape could land.
+- **Rootfs files touched while loop-mounted during setup.** `setup.rs`
+  `patch_guest_network` reads and rewrites the guest's `/etc/hosts`, and `coop
+  commit` turns a guest-mutated rootfs into an image template — so the guest
+  authors both the contents and the directory entry at that path on every later
+  create/restore. Contents are read bounded and best-effort
+  (`bound_guest_hosts` degrades to a default rather than aborting the
+  lifecycle). The paths are **host** paths: `MountGuard::simple` is a loop
+  mount, not a chroot, so the traversal rule below applies to every
+  `{mount}/…` string there — compare `verify_chroot_binaries`, which runs
+  `test -x` *inside* a chroot for that reason. Not currently validated.
 - **Guest command output read by the host.** e.g. `check_guest_dirty` reads
   `git status --porcelain` from the guest. Today this only gates control flow /
   is printed to the user — it is never fed into `sh -c` on the host. Keep it

src/setup.rs

@@ -9,7 +9,7 @@ use anyhow::{Context, Result, bail};
 use serde::{Deserialize, Serialize};
 
 use crate::cmd::{Cmd, command_exists};
-use crate::config::{CoopConfig, ImageName, Instance};
+use crate::config::{CoopConfig, ImageName, Instance, InstanceName};
 use crate::devcontainer_oci::{InstalledFeature, ResolvedFeature, installed_features};
 use crate::guest::{
     BASE_PACKAGES, DOCKER_PACKAGES, GH_PACKAGES, GuestUser, ProfileDef, SCRIPT_CLAUDE_CODE,
@@ -163,7 +163,6 @@ pub fn create_instance(
         }
     }
 
-    // Patch the guest's network config with the correct IP for this instance
     patch_guest_network(inst)?;
 
     Ok(())
@@ -254,8 +253,8 @@ pub fn commit_instance_rootfs(cfg: &CoopConfig, inst: &Instance, image: &ImageNa
 ///
 /// Mirrors [`create_instance`]'s copy + network-patch, but sources the
 /// rootfs from an arbitrary image rather than the instance's origin
-/// image. The network config is re-patched for this instance's IP,
-/// overwriting whatever address the template baked in at commit time.
+/// image. The network config and guest identity are re-patched for this
+/// instance, overwriting whatever the template baked in at commit time.
 pub fn restore_instance_rootfs(cfg: &CoopConfig, inst: &Instance, image: &ImageName) -> Result<()> {
     let template = cfg.template_path_for(image);
     if !template.exists() {
@@ -334,14 +333,19 @@ impl Drop for MountGuard {
     }
 }
 
-/// Mount the instance rootfs and rewrite the systemd-networkd config
-/// with the instance's unique guest IP.
+/// Mount the instance rootfs and rewrite its network identity: the
+/// systemd-networkd config with the instance's unique guest IP, plus
+/// `/etc/hostname` and the matching `/etc/hosts` alias.
 fn patch_guest_network(inst: &Instance) -> Result<()> {
     let rootfs_str = inst.rootfs_path().display().to_string();
     let mount_dir = inst.dir.join("rootfs-mount");
     let mount_str = mount_dir.display().to_string();
 
-    tracing::info!("Patching guest network: IP={}", inst.guest_ip());
+    let hostname = guest_hostname(&inst.name);
+    tracing::info!(
+        "Patching guest network: IP={}, hostname={hostname}",
+        inst.guest_ip()
+    );
 
     let _guard = MountGuard::simple(&rootfs_str, &mount_str)?;
 
@@ -358,17 +362,143 @@ fn patch_guest_network(inst: &Instance) -> Result<()> {
         .stdin_write(network_config.as_bytes())
         .context("Failed to write guest network config")?;
 
-    // Also patch hostname to include instance name
-    let hostname = format!("claude-{}\n", inst.name);
+    patch_guest_identity(&mount_str, &hostname)?;
+
+    // _guard dropped here → unmount + rmdir
+    Ok(())
+}
+
+/// Debian/Ubuntu convention: the machine's own name lives on a `127.0.1.1`
+/// line, separate from `127.0.0.1 localhost`.
+const GUEST_HOSTS_ALIAS_IP: &str = "127.0.1.1";
+
+/// Cap on the guest `/etc/hosts` read. The file is guest-authored — `coop
+/// commit` snapshots a mutated rootfs into an image template — so the guest
+/// must not get to choose how much the host allocates. A real hosts file is a
+/// few hundred bytes.
+const MAX_GUEST_HOSTS_BYTES: usize = 64 * 1024;
+
+/// One byte under Linux's `HOST_NAME_MAX` of 64, which POSIX counts without
+/// the terminator. A 57-character instance name is already pathological, so
+/// the spare byte is free — and the clamp holds even where the limit is read
+/// as including the terminator.
+const MAX_GUEST_HOSTNAME_LEN: usize = 63;
+
+/// The guest hostname for an instance: `claude-<name>`, clamped to fit
+/// `HOST_NAME_MAX`.
+///
+/// Instance names allow 64 characters, so the prefixed form reaches 71 — past
+/// the kernel's limit. An over-long name cannot reach
+/// the running hostname intact, so `/etc/hostname` and the `/etc/hosts` alias
+/// would then name different things. Deriving both from this one value is what
+/// keeps them equal.
+fn guest_hostname(name: &InstanceName) -> String {
+    // `truncate` is a byte index, and `InstanceName` is validated ASCII, so
+    // every index is a char boundary.
+    let mut hostname = format!("claude-{name}");
+    hostname.truncate(MAX_GUEST_HOSTNAME_LEN);
+    hostname
+}
+
+/// Write `/etc/hostname` and the matching `/etc/hosts` alias inside a mounted
+/// guest rootfs.
+///
+/// `sudo` resolves the machine's own hostname on every invocation, so a
+/// hostname with no hosts entry makes it print `unable to resolve host` ahead
+/// of each `sudo` command in the guest. The template ships `claude-vm`
+/// (`scripts/guest/guest-config.sh`) while each instance gets its own name, so
+/// the two files have to move together.
+fn patch_guest_identity(mount_str: &str, hostname: &str) -> Result<()> {
     let hostfile = format!("{mount_str}/etc/hostname");
     Cmd::new("tee")
         .arg(&hostfile)
         .sudo()
-        .stdin_write(hostname.as_bytes())
+        .stdin_write(format!("{hostname}\n").as_bytes())
         .context("Failed to write hostname")?;
 
-    // _guard dropped here → unmount + rmdir
-    Ok(())
+    let hostsfile = format!("{mount_str}/etc/hosts");
+    let hosts = hosts_with_hostname(&read_guest_hosts(&hostsfile), hostname);
+    Cmd::new("tee")
+        .arg(&hostsfile)
+        .sudo()
+        .stdin_write(hosts.as_bytes())
+        .context("Failed to write guest hosts file")?;
+
+    // `tee` preserves an existing file's mode but creates a new one under
+    // root's umask, which comes from the host's PAM/`login.defs`, not coop — a
+    // hardened umask hides /etc/hosts from the guest's resolver. Reachable only
+    // if the guest deleted the file before `coop commit`. Cf. `PID_TRAMPOLINE`.
+    Cmd::new("chmod")
+        .arg("644")
+        .arg(&hostsfile)
+        .sudo()
+        .run()
+        .context("Failed to set guest hosts file mode")
+}
+
+/// Read a guest `/etc/hosts`, bounded one byte past the cap so that an
+/// oversized file is detected rather than written back truncated.
+///
+/// The file is guest-authored (see `docs/trust-model.md`), so the read is
+/// best-effort: a cosmetic fix must not fail `coop up` on a rootfs that used
+/// to boot. [`bound_guest_hosts`] holds that decision.
+fn read_guest_hosts(path: &str) -> String {
+    let read = Cmd::new("head")
+        .arg("-c")
+        .arg((MAX_GUEST_HOSTS_BYTES + 1).to_string())
+        .arg(path)
+        .sudo()
+        .capture();
+    bound_guest_hosts(read)
+}
+
+/// Decide what a bounded read of the guest hosts file yields: the contents, or
+/// empty for "no usable file" — which [`hosts_with_hostname`] turns into a
+/// default.
+fn bound_guest_hosts(read: Result<String>) -> String {
+    match read {
+        Ok(contents) if contents.len() > MAX_GUEST_HOSTS_BYTES => {
+            tracing::warn!("Guest hosts file exceeds {MAX_GUEST_HOSTS_BYTES} bytes — replacing it");
+            String::new()
+        }
+        Ok(contents) => contents,
+        Err(e) => {
+            // `capture`'s error text is the command line, not the file's bytes.
+            tracing::warn!("Guest hosts file is unreadable ({e}) — writing a fresh one");
+            String::new()
+        }
+    }
+}
+
+/// Return `contents` with the guest's own-hostname entry set to `hostname`.
+///
+/// The matched line is replaced whole rather than edited: its aliases name the
+/// previous hostname, and carrying them forward keeps a stale name resolvable.
+/// Blank input yields a minimal default file. Idempotent.
+fn hosts_with_hostname(contents: &str, hostname: &str) -> String {
+    let entry = format!("{GUEST_HOSTS_ALIAS_IP} {hostname}\n");
+    if contents.trim().is_empty() {
+        return format!("127.0.0.1 localhost\n{entry}");
+    }
+
+    let mut found = false;
+    let mut output = String::new();
+    for line in contents.lines() {
+        if line.split_whitespace().next() == Some(GUEST_HOSTS_ALIAS_IP) {
+            if found {
+                continue;
+            }
+            output.push_str(&entry);
+            found = true;
+        } else {
+            output.push_str(line);
+            output.push('\n');
+        }
+    }
+    if !found {
+        output.push_str(&entry);
+    }
+    output
 }
 
 // ── Template management ───────────────────────────────────────
@@ -1959,4 +2089,124 @@ mod tests {
         .to_string();
         assert!(err.contains("network is on fire"), "{err}");
     }
+
+    #[test]
+    fn hosts_with_hostname_replaces_stale_guest_alias() {
+        let existing = "127.0.0.1 localhost\n127.0.1.1 claude-vm\n::1 localhost\n";
+        assert_eq!(
+            hosts_with_hostname(existing, "claude-auditor-1"),
+            "127.0.0.1 localhost\n127.0.1.1 claude-auditor-1\n::1 localhost\n"
+        );
+    }
+
+    #[test]
+    fn hosts_with_hostname_adds_missing_guest_alias() {
+        assert_eq!(
+            hosts_with_hostname("127.0.0.1 localhost\n", "claude-auditor-1"),
+            "127.0.0.1 localhost\n127.0.1.1 claude-auditor-1\n"
+        );
+    }
+
+    #[test]
+    fn hosts_with_hostname_replaces_the_whole_matched_line() {
+        assert_eq!(
+            hosts_with_hostname("127.0.1.1 claude-vm claude-vm.local # old\n", "claude-a"),
+            "127.0.1.1 claude-a\n"
+        );
+    }
+
+    #[test]
+    fn hosts_with_hostname_collapses_duplicate_aliases() {
+        let existing = "127.0.1.1 claude-vm\n127.0.0.1 localhost\n127.0.1.1 claude-vm\n";
+        assert_eq!(
+            hosts_with_hostname(existing, "claude-a"),
+            "127.0.1.1 claude-a\n127.0.0.1 localhost\n"
+        );
+    }
+
+    #[test]
+    fn hosts_with_hostname_normalizes_a_missing_trailing_newline() {
+        assert_eq!(
+            hosts_with_hostname("127.0.0.1 localhost", "claude-a"),
+            "127.0.0.1 localhost\n127.0.1.1 claude-a\n"
+        );
+    }
+
+    /// Blank input is what [`bound_guest_hosts`] degrades to, so the result has
+    /// to be a usable hosts file — not one holding only the guest alias.
+    #[test]
+    fn hosts_with_hostname_synthesizes_a_default_from_blank_input() {
+        let expected = "127.0.0.1 localhost\n127.0.1.1 claude-a\n";
+        assert_eq!(hosts_with_hostname("", "claude-a"), expected);
+        assert_eq!(hosts_with_hostname(" \n\n", "claude-a"), expected);
+    }
+
+    #[test]
+    fn hosts_with_hostname_preserves_comments_and_blank_lines() {
+        let existing = "# managed by post-install\n\n10.0.0.5 registry.internal\n127.0.1.1 old\n";
+        assert_eq!(
+            hosts_with_hostname(existing, "claude-a"),
+            "# managed by post-install\n\n10.0.0.5 registry.internal\n127.0.1.1 claude-a\n"
+        );
+    }
+
+    #[test]
+    fn hosts_with_hostname_is_idempotent() {
+        let once = hosts_with_hostname("127.0.0.1 localhost\n127.0.1.1 claude-vm\n", "claude-a");
+        assert_eq!(hosts_with_hostname(&once, "claude-a"), once);
+    }
+
+    fn instance_name(name: &str) -> InstanceName {
+        InstanceName::new(name).unwrap()
+    }
+
+    #[test]
+    fn guest_hostname_prefixes_the_instance_name() {
+        assert_eq!(
+            guest_hostname(&instance_name("auditor-1")),
+            "claude-auditor-1"
+        );
+    }
+
+    /// Asserted against literals, not `MAX_GUEST_HOSTNAME_LEN` — comparing the
+    /// output length to the constant that produced it passes for any bound.
+    #[test]
+    fn guest_hostname_clamps_the_longest_instance_name() {
+        let longest = instance_name(&"n".repeat(64));
+        assert_eq!(
+            guest_hostname(&longest),
+            format!("claude-{}", "n".repeat(56))
+        );
+    }
+
+    /// The exact-fit boundary: 56 characters is the longest name that survives
+    /// the clamp untouched, so this pins which names the fix still reaches.
+    #[test]
+    fn guest_hostname_leaves_an_exactly_fitting_name_intact() {
+        let name = "n".repeat(56);
+        assert_eq!(
+            guest_hostname(&instance_name(&name)),
+            format!("claude-{name}")
+        );
+        assert_eq!(guest_hostname(&instance_name(&name)).len(), 63);
+    }
+
+    #[test]
+    fn bound_guest_hosts_passes_through_a_file_at_the_cap() {
+        let at_cap = "a".repeat(MAX_GUEST_HOSTS_BYTES);
+        assert_eq!(bound_guest_hosts(Ok(at_cap.clone())), at_cap);
+    }
+
+    /// `head -c` reads one byte past the cap, so an oversized file arrives as
+    /// `MAX + 1` bytes and must be discarded rather than written back truncated.
+    #[test]
+    fn bound_guest_hosts_discards_a_file_over_the_cap() {
+        let over_cap = "a".repeat(MAX_GUEST_HOSTS_BYTES + 1);
+        assert_eq!(bound_guest_hosts(Ok(over_cap)), "");
+    }
+
+    #[test]
+    fn bound_guest_hosts_degrades_on_a_read_error() {
+        assert_eq!(bound_guest_hosts(Err(anyhow::anyhow!("no such file"))), "");
+    }
 }