fix(node-sync): write the git identity through the merged view, not the overlay upper

.github/workflows/centaur-ci.yml

@@ -304,6 +304,19 @@ jobs:
           cargo clippy --all-targets -- -D warnings
           cargo test
 
+      # `cargo test` above runs as the unprivileged runner, so every test that needs
+      # a real overlay mount SELF-SKIPS there — silently. That is how the git identity
+      # shipped writing to an overlay's upperdir behind a live mount: invisible to the
+      # agent, and green on every lane. Re-run those under sudo so they actually gate
+      # (ubuntu-latest supports overlayfs + sudo — same assumption ci/overlay-validation.sh
+      # makes). Separate CARGO_TARGET_DIR so root-owned artifacts cannot poison the
+      # rust-cache the runner saves at job end.
+      - name: Privileged overlay tests (real mounts, root)
+        working-directory: runtime/node-sync
+        run: |
+          sudo -E env "PATH=$PATH" CARGO_TARGET_DIR=/tmp/privileged-target \
+            cargo test --test git_identity_overlay_visibility -- --nocapture
+
       # Real overlay + scan-demo: locks in the syscall proofs (whiteout→Delete,
       # symlink→metadata-only, openat2 NO_SYMLINKS blocks the /etc/shadow escape).
       - name: Overlay scanner validation

runtime/node-sync/src/bin/centaur-node-syncd/linux_daemon/mod.rs

@@ -427,19 +427,43 @@ fn run_multi_session(global: GlobalConfig, overlays_root: PathBuf, once: bool, i
                     };
                     let wip_remounted = !has_active_mount
                         || mounted_overlays.get(&discovered.session) != Some(&plan);
+                    let mounted = match mount_overlay(plan, Some(discovered.manifest.agent_uid)) {
+                        Ok(plan) => plan,
+                        Err(e) => {
+                            eprintln!("session {}: overlay mount: {e}", discovered.session);
+                            continue;
+                        }
+                    };
                     // Git author identity is server-derived per CLAIM, and a claim is a
                     // mount — so refresh it only when the mount is being (re)established,
                     // never on every tick. An unconditional per-session GET per tick is
                     // exactly the steady-state cost the atrium delta protocol exists to
-                    // avoid. Writing into the upper before mount_overlay is deliberate:
-                    // the agent must never observe a home without its identity.
+                    // avoid.
+                    //
+                    // This MUST write through `mounted.merged`, never `discovered.upper`.
+                    // A warm pod's overlay is already mounted long before the claim that
+                    // gives it an identity, and modifying an overlay's upperdir behind a
+                    // live mount is undefined: the file lands on disk but the merged view
+                    // keeps serving its cached (negative) dentry, so the agent never sees
+                    // it — and the path is left so incoherent that a later mkdir through
+                    // the mount fails with ESTALE. Shipped exactly that way once: the
+                    // identity file was present in the upper and the agent still committed
+                    // as "Centaur AI". Same hazard the lowerdir comment above describes.
                     //
                     // Failures here must NOT skip the mount. The image bakes a
                     // "Centaur AI" identity and /opt/centaur/gitconfig includes this file
                     // only if it exists, so an absent identity degrades to exactly the
                     // pre-existing behavior. Gating the mount on identity would turn a
                     // surface blip into "the agent's home never mounts" — trading a
                     // recoverable misattributed commit for an unrecoverable dead session.
+                    //
+                    // Ordering caveat: mount_overlay writes the ready marker, and a warm
+                    // pod's marker predates this claim entirely, so there is no
+                    // write-before-ready guarantee to be had here — the agent could in
+                    // principle commit in the seconds before this lands. That degrades to
+                    // the baked identity rather than a wrong one, which is why it is
+                    // acceptable; closing it for real needs the claim path itself to carry
+                    // the identity.
                     if wip_remounted {
                         let identity_client = HttpAtriumClient::new(
                             &global.base_url,
@@ -449,7 +473,7 @@ fn run_multi_session(global: GlobalConfig, overlays_root: PathBuf, once: bool, i
                         match identity_client.get_git_identity() {
                             Ok(identity) => {
                                 if let Err(error) = materialize_git_identity(
-                                    &discovered.upper,
+                                    &mounted,
                                     identity.as_ref(),
                                     Some(discovered.manifest.agent_uid),
                                 ) {
@@ -467,13 +491,6 @@ fn run_multi_session(global: GlobalConfig, overlays_root: PathBuf, once: bool, i
                             }
                         }
                     }
-                    let mounted = match mount_overlay(plan, Some(discovered.manifest.agent_uid)) {
-                        Ok(plan) => plan,
-                        Err(e) => {
-                            eprintln!("session {}: overlay mount: {e}", discovered.session);
-                            continue;
-                        }
-                    };
                     let session = session_config_from_discovered(&discovered, &mounted);
                     mounted_overlays.insert(discovered.session.clone(), mounted);
                     let first_seen = !states.contains_key(&session.session);

runtime/node-sync/src/materializer.rs

@@ -6,6 +6,7 @@ use std::sync::{Mutex, OnceLock};
 use std::time::{Duration, Instant};
 
 use crate::http_client::GitIdentity;
+use crate::overlay_mount::OverlayMountPlan;
 use crate::runtime::{AtriumChannel, AtriumClient, ContextDeltaRequest, ContextDocResponse};
 use crate::state::{ContextDocState, DaemonState};
 
@@ -87,14 +88,24 @@ If a listed path is missing, it is still materializing — wait a few seconds an
 pub const CONTEXT_READY_MARKER: &str = ".atrium-context-ready";
 pub const GIT_IDENTITY_RELATIVE_PATH: &str = ".config/git/atrium-identity";
 
-/// Authoritatively materialize the server-derived identity into an overlay upper.
+/// Authoritatively materialize the server-derived identity into a session's home.
 /// `None` is the 204 fallback and removes a prior identity so the image default wins.
+///
+/// Takes the mounted `OverlayMountPlan` rather than a bare path ON PURPOSE: this must
+/// be written through `plan.merged`, and an earlier version took a `&Path` and got
+/// handed the overlay's `upper` instead. That compiled, passed every tempdir unit
+/// test, and was silently useless in production — a warm pod's overlay is already
+/// mounted long before the claim that gives it an identity, and writing to an
+/// upperdir behind a live mount is undefined: the file lands on disk while the merged
+/// view keeps serving its cached negative dentry, so the agent never sees it and
+/// commits as the baked "Centaur AI". Deriving the path here instead of accepting one
+/// makes that mistake unrepresentable rather than merely documented.
 pub fn materialize_git_identity(
-    home: &Path,
+    plan: &OverlayMountPlan,
     identity: Option<&GitIdentity>,
     agent_uid: Option<u32>,
 ) -> Result<(), String> {
-    let dst = home.join(GIT_IDENTITY_RELATIVE_PATH);
+    let dst = plan.merged.join(GIT_IDENTITY_RELATIVE_PATH);
     let Some(identity) = identity else {
         remove_file_if_present(&dst)?;
         return Ok(());
@@ -988,6 +999,17 @@ mod tests {
         }
     }
 
+    // Build via the real constructor so the plan cannot drift from production's shape.
+    // `merged` is a plain tempdir here: these unit tests cover the FILE contract
+    // (escaping, idempotency, 204-removal). They CANNOT cover overlay visibility, which
+    // is what actually broke in production — see tests/git_identity_overlay_visibility.rs.
+    fn plan_for(home: &Path) -> OverlayMountPlan {
+        let overlays_root = home.join("overlays-root");
+        std::fs::create_dir_all(&overlays_root).unwrap();
+        crate::overlay_mount::plan_overlay_mount(&overlays_root, "unit", home, "", &[], None)
+            .unwrap()
+    }
+
     // Pinned to `/` for the same reason the writer is: git discovers a repository from
     // the cwd even for `--file` reads, so running the suite from inside a linked
     // worktree (whose .git is a gitfile) would fail these on repo discovery, not on
@@ -1009,7 +1031,7 @@ mod tests {
         std::fs::create_dir_all(dst.parent().unwrap()).unwrap();
         std::fs::write(&dst, b"stale").unwrap();
 
-        materialize_git_identity(temp.path(), None, None).unwrap();
+        materialize_git_identity(&plan_for(temp.path()), None, None).unwrap();
 
         assert!(!dst.exists());
     }
@@ -1020,7 +1042,7 @@ mod tests {
         let identity = git_identity("Allan Niemerg");
         let dst = temp.path().join(GIT_IDENTITY_RELATIVE_PATH);
 
-        materialize_git_identity(temp.path(), Some(&identity), None).unwrap();
+        materialize_git_identity(&plan_for(temp.path()), Some(&identity), None).unwrap();
 
         assert_eq!(
             std::fs::read_to_string(&dst).unwrap(),
@@ -1043,7 +1065,7 @@ mod tests {
         let identity = git_identity(hostile);
         let dst = temp.path().join(GIT_IDENTITY_RELATIVE_PATH);
 
-        materialize_git_identity(temp.path(), Some(&identity), None).unwrap();
+        materialize_git_identity(&plan_for(temp.path()), Some(&identity), None).unwrap();
 
         let output = git_config_get(&dst, "user.name");
         assert!(output.status.success());
@@ -1064,9 +1086,9 @@ mod tests {
         let identity = git_identity("Allan Niemerg");
         let dst = temp.path().join(GIT_IDENTITY_RELATIVE_PATH);
 
-        materialize_git_identity(temp.path(), Some(&identity), None).unwrap();
+        materialize_git_identity(&plan_for(temp.path()), Some(&identity), None).unwrap();
         let first = std::fs::read(&dst).unwrap();
-        materialize_git_identity(temp.path(), Some(&identity), None).unwrap();
+        materialize_git_identity(&plan_for(temp.path()), Some(&identity), None).unwrap();
 
         assert_eq!(std::fs::read(&dst).unwrap(), first);
     }

runtime/node-sync/tests/git_identity_overlay_visibility.rs

@@ -0,0 +1,107 @@
+//! Pins the one thing a tempdir test can never see: the git author identity has to
+//! be visible to the AGENT, which reads it through the overlay's merged view.
+//!
+//! We shipped this wrong once. The daemon wrote the identity into the overlay's
+//! upperdir before mounting, which is fine for a cold create but silently useless
+//! for a warm claim: a warm pod's overlay is already mounted minutes before the
+//! claim that gives it an identity, and modifying an upperdir behind a live mount
+//! is undefined behaviour. The file landed on disk, `ls` on the upper showed it,
+//! every unit test passed — and the agent still committed as "Centaur AI", because
+//! the merged view kept serving its cached negative dentry. The upper was left so
+//! incoherent that a later mkdir through the mount failed with ESTALE.
+//!
+//! So: assert against the MERGED path, on a real overlay, with the mount already
+//! established — the warm-claim shape. A unit test on a plain tempdir passes either
+//! way and proves nothing.
+//!
+//! Needs Linux + root + a non-overlayfs TMPDIR (overlay upperdirs cannot live on
+//! overlayfs; in a container, mount a tmpfs and point TMPDIR at it). Skips silently
+//! otherwise, same as the other privileged on-node validations.
+
+#![cfg(target_os = "linux")]
+
+use std::fs;
+use std::process::Command;
+
+use centaur_node_sync::http_client::GitIdentity;
+use centaur_node_sync::materializer::{GIT_IDENTITY_RELATIVE_PATH, materialize_git_identity};
+use centaur_node_sync::overlay_mount::{mount_overlay, plan_overlay_mount, unmount_overlay};
+
+fn identity() -> GitIdentity {
+    // `source` rides the wire for observability but the daemon is a tolerant reader
+    // and never needs it, so it is deliberately absent from this struct.
+    GitIdentity {
+        author_name: "Gary".to_string(),
+        author_email: "10901359+gbasin@users.noreply.github.com".to_string(),
+        session_id: "2f30f3db-e964-4e25-9371-7d323836c1c7".to_string(),
+        harness: "codex".to_string(),
+    }
+}
+
+fn git_get(file: &std::path::Path, key: &str) -> String {
+    let out = Command::new("git")
+        .current_dir("/")
+        .args(["config", "--file"])
+        .arg(file)
+        .args(["--get", key])
+        .output()
+        .unwrap();
+    String::from_utf8(out.stdout).unwrap().trim().to_string()
+}
+
+/// The warm-claim shape: overlay ALREADY mounted, then the identity arrives.
+#[test]
+fn identity_written_after_mount_is_visible_through_the_merged_view() {
+    if unsafe { libc::geteuid() } != 0 {
+        eprintln!(
+            "SKIP: identity_written_after_mount_is_visible_through_the_merged_view requires root"
+        );
+        return;
+    }
+
+    let session = "git-identity-visibility-it";
+    let root = std::env::temp_dir().join(format!("gid-it-{}", std::process::id()));
+    let _ = fs::remove_dir_all(&root);
+    let overlays_root = root.join("overlays");
+    let merged = root.join("merged").join(session);
+    fs::create_dir_all(&overlays_root).unwrap();
+    fs::create_dir_all(&merged).unwrap();
+
+    let plan = plan_overlay_mount(&overlays_root, session, &merged, "", &[], None).unwrap();
+    let mounted = mount_overlay(plan, None).expect("overlay must mount");
+
+    // The agent is already live against this mount at this point — exactly the warm
+    // pod that has been sitting in the pool. NOW the claim's identity shows up.
+    materialize_git_identity(&mounted, Some(&identity()), None).expect("materialize");
+
+    let seen_by_agent = mounted.merged.join(GIT_IDENTITY_RELATIVE_PATH);
+    assert!(
+        seen_by_agent.exists(),
+        "identity is invisible through the merged view — the agent would commit as the baked \
+         image identity. This is the exact bug that shipped: writing via the upperdir of a live \
+         overlay leaves the file on disk but out of the agent's view."
+    );
+    assert_eq!(
+        git_get(&seen_by_agent, "user.email"),
+        "10901359+gbasin@users.noreply.github.com"
+    );
+    assert_eq!(git_get(&seen_by_agent, "user.name"), "Gary");
+    // The [atrium] block feeds the commit-msg hook's provenance trailers; it travels
+    // in the same file, so emit both or neither.
+    assert_eq!(
+        git_get(&seen_by_agent, "atrium.sessionId"),
+        "2f30f3db-e964-4e25-9371-7d323836c1c7"
+    );
+    assert_eq!(git_get(&seen_by_agent, "atrium.harness"), "codex");
+
+    // A 204 must clear it through the same view, so a revoked identity cannot linger
+    // and keep authoring as someone who no longer resolves.
+    materialize_git_identity(&mounted, None, None).expect("204 removes");
+    assert!(
+        !seen_by_agent.exists(),
+        "a 204 must remove the identity through the merged view, not just the upper"
+    );
+
+    unmount_overlay(&mounted).unwrap();
+    let _ = fs::remove_dir_all(&root);
+}