From 5466cec066b83c31c987327d5a31fd65ff7d67e8 Mon Sep 17 00:00:00 2001
From: atlas <atlas@hyperhive.local>
Date: Sat, 26 Sep 2026 15:42:30 +0200
Subject: [PATCH] host-modules: fix atomic_write_secret's leftover-tmp bug;
 convert swarm-bao's forwarder-oidc writer too
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

atomic_write_secret's cleanup trap used RETURN, which never fires when
set -e aborts the function mid-body (a failing cat/chmod/chown), so a
secret-bearing temp file was left behind instead of being removed.
The write now runs in a subshell with its own EXIT trap, invoked via a
named handler (so `local rc=$?` is a normal, shellcheck-visible
assignment) that only removes the temp file when the subshell's exit
status is nonzero — the subshell's trap table is private, so a calling
unit's own EXIT trap is untouched. Reproduced the leftover-tmp bug
against the prior commit, confirmed it's gone, and confirmed both the
success path and a caller's own EXIT trap still work as before.

swarm-bao.nix's swarm-bao-forwarder-oidc unit had the identical
write-then-chmod-on-live-path defect as the four sites already fixed
here (fetches an OIDC client secret from swarm-bao, printfs it to the
live host path, chowns/chmods after) and was missed by the original
sweep. Converted it to atomic_write_secret; content and final
owner/mode (root:root, 0400) are unchanged.

Refs #4723
---
 nix/host-modules/lib/atomic-write-secret.nix | 28 +++++++++++++++-----
 nix/host-modules/swarm-bao.nix               |  9 ++++---
 2 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/nix/host-modules/lib/atomic-write-secret.nix b/nix/host-modules/lib/atomic-write-secret.nix
index dccf7e9d..cf50e12a 100644
--- a/nix/host-modules/lib/atomic-write-secret.nix
+++ b/nix/host-modules/lib/atomic-write-secret.nix
@@ -26,12 +26,28 @@
   atomic_write_secret() {
     local mode="$1" owner="$2" target="$3" tmp
     tmp="$(mktemp "$(dirname -- "$target")/.$(basename -- "$target").XXXXXX")"
-    trap 'rm -f "$tmp"' RETURN
-    cat > "$tmp"
-    chmod "$mode" "$tmp"
-    if [ -n "$owner" ]; then
-      chown "$owner" "$tmp"
-    fi
+    # The write happens in a subshell so its own EXIT trap cleans up `$tmp`
+    # on failure (a `RETURN` trap does not fire when `set -e` aborts the
+    # function mid-body) without touching an EXIT trap the calling script's
+    # own `script` may already have — subshell traps are local to the
+    # subshell. A named handler, not an inline trap string, so `local rc=$?`
+    # is a normal function-local assignment shellcheck can see: it only
+    # removes `$tmp` when `$?` is nonzero — on the ordinary path the
+    # subshell also exits successfully, and `$tmp` has to survive that to
+    # reach the `mv` below.
+    (
+      _atomic_write_secret_cleanup() {
+        local rc=$?
+        [ "$rc" -eq 0 ] || rm -f "$tmp"
+        exit "$rc"
+      }
+      trap _atomic_write_secret_cleanup EXIT
+      cat > "$tmp"
+      chmod "$mode" "$tmp"
+      if [ -n "$owner" ]; then
+        chown "$owner" "$tmp"
+      fi
+    )
     mv -f "$tmp" "$target"
   }
 ''
diff --git a/nix/host-modules/swarm-bao.nix b/nix/host-modules/swarm-bao.nix
index c517e763..459934f3 100644
--- a/nix/host-modules/swarm-bao.nix
+++ b/nix/host-modules/swarm-bao.nix
@@ -764,6 +764,8 @@ let
   forwarderSecretInContainer = "/var/lib/swarm-bao-otel-oidc/${cfg.otel.clientId}.secret";
   forwarderHostSecretPath = "/var/lib/nixos-containers/${cfg.machine}${forwarderSecretInContainer}";
   forwarderHostSecretDir = builtins.dirOf forwarderHostSecretPath;
+
+  atomicWriteSecret = import ./lib/atomic-write-secret.nix { };
   # The collector runs under `DynamicUser` and opens `client_secret_file`
   # itself, so there is no uid to hand a 0400 file to. `LoadCredential` reads
   # it as root before the sandbox exists and re-exposes it under a path that
@@ -1797,6 +1799,8 @@ in
         script = ''
           set -euo pipefail
 
+          ${atomicWriteSecret}
+
           # `bao`'s own message is the only thing separating a missing value
           # from a refused identity from an unreachable store, and this unit
           # retries on all three — so it reports which one rather than
@@ -1837,10 +1841,7 @@ in
           # an argument in /proc the way `install <<<"$secret"` or an `echo`
           # from `path` would.
           install -d -m 0755 ${lib.escapeShellArg forwarderHostSecretDir}
-          umask 077
-          printf '%s\n' "$secret" > ${lib.escapeShellArg forwarderHostSecretPath}
-          chown root:root ${lib.escapeShellArg forwarderHostSecretPath}
-          chmod 0400 ${lib.escapeShellArg forwarderHostSecretPath}
+          printf '%s\n' "$secret" | atomic_write_secret 0400 root:root ${lib.escapeShellArg forwarderHostSecretPath}
         '';
       };
     })