fix(container): data-uid chown is drift-gated, not unconditional every tick

apply_data_uid ran a recursive sudo chown on every prepare_for_start, and the
reconciler re-prepares — archi-dev-box's journal showed postgres-btcpay rechowned
every ~45s despite already-correct ownership, and on framework-pt the same loop
surfaced as operator-visible 'chown failed' noise. chown_for_rootless_container
now stats the target first and returns early when the top-level owner already
matches the host-mapped uid:gid. Deep drift in a running container is still
caught by ensure_running_container_ownership's in-container write-probe, which
is the authority that actually matters (it probes writability, not stat bits).

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
archipelago
2026-08-07 18:54:39 -04:00
co-authored by Claude
parent 494400df29
commit ca106c5a43
@@ -277,6 +277,21 @@ async fn chown_for_rootless_container(uid_gid: &str, path: &str) -> Result<()> {
.map(|(u, g)| (u.parse::<u32>().unwrap_or(0), g.parse::<u32>().unwrap_or(0)))
.unwrap_or((0, 0));
// Idempotence: a directory already owned by the host-mapped target must
// not get another recursive chown on every prepare/reconcile tick — that
// unconditional chown is what looped on postgres-btcpay every ~45s
// (archi-dev-box, 2026-08-07). Deep-file drift in a RUNNING container is
// still caught by ensure_running_container_ownership's write-probe.
let host_uid_gid = if uid > 0 && uid < 100_000 {
let map = |id: u32| if id == 0 { 1000 } else { 100_000 + id - 1 };
format!("{}:{}", map(uid), map(gid))
} else {
uid_gid.to_string()
};
if ownership_already_correct(path, &host_uid_gid) {
return Ok(());
}
if uid > 0 && uid < 100_000 {
let output = tokio::process::Command::new("podman")
.args(["unshare", "chown", "-R", uid_gid, path])
@@ -295,12 +310,6 @@ async fn chown_for_rootless_container(uid_gid: &str, path: &str) -> Result<()> {
// crash-loop, framework-pt 2026-08-06). Container uid N (N>=1) lives at
// subuid_base + N - 1; the fleet provisions base 100000. uid 0 and
// already-mapped ids (>=100000) pass through untouched.
let host_uid_gid = if uid > 0 && uid < 100_000 {
let map = |id: u32| if id == 0 { 1000 } else { 100_000 + id - 1 };
format!("{}:{}", map(uid), map(gid))
} else {
uid_gid.to_string()
};
let status = host_sudo(&["chown", "-R", &host_uid_gid, path])
.await
.with_context(|| format!("sudo chown -R {host_uid_gid} {path}"))?;
@@ -327,6 +336,23 @@ async fn chown_for_rootless_container(uid_gid: &str, path: &str) -> Result<()> {
))
}
/// Cheap drift gate for `chown_for_rootless_container`: does the path's
/// top-level owner already match the target "uid:gid" string? A miss on a
/// missing path answers false (the caller's mkdir runs first anyway).
fn ownership_already_correct(path: &str, host_uid_gid: &str) -> bool {
use std::os::unix::fs::MetadataExt;
let Some((uid_s, gid_s)) = host_uid_gid.split_once(':') else {
return false;
};
let (Ok(uid), Ok(gid)) = (uid_s.parse::<u32>(), gid_s.parse::<u32>()) else {
return false;
};
let Ok(md) = std::fs::metadata(path) else {
return false;
};
md.uid() == uid && md.gid() == gid
}
/// `(container-id, mount-dest)` pairs whose in-container chown returned a hard,
/// permanent failure (e.g. "Operation not permitted" on a mount that can't be
/// re-owned from inside the userns). Remembered for the life of the process so
@@ -5811,8 +5837,7 @@ app:
}
#[tokio::test]
async fn manifest_generated_files_can_overwrite_when_declared() {
let rt = Arc::new(MockRuntime::default());
async fn manifest_generated_files_can_overwrite_when_declared() {let rt = Arc::new(MockRuntime::default());
let orch = orch_with(rt.clone()).await;
let data_dir = tempfile::tempdir_in("/var/lib/archipelago").unwrap();
@@ -6796,4 +6821,19 @@ app:
// zombie guard reports it dead → the reconciler recreates.
assert!(!pid_is_alive(2_000_000_000));
}
#[test]
fn ownership_drift_gate_matches_on_exact_owner_only() {
// A tempdir owned by this process's uid stands in for a correctly
// chowned data dir; any other uid reads as drift.
let tmp = tempfile::tempdir().unwrap();
let path = tmp.path().to_string_lossy().to_string();
let my_uid = unsafe { libc::geteuid() };
let mine = format!("{0}:{0}", my_uid);
assert!(ownership_already_correct(&path, &mine));
assert!(!ownership_already_correct(&path, "100998:100998"));
assert!(!ownership_already_correct("/nonexistent/path", &mine));
assert!(!ownership_already_correct(&path, "garbage"));
assert!(!ownership_already_correct(&path, "1:2:3"));
}
}