diff --git a/changelog.d/fixed-17862-atomic-runtime-config-save.md b/changelog.d/fixed-17862-atomic-runtime-config-save.md new file mode 100644 index 0000000000..0dad10650e --- /dev/null +++ b/changelog.d/fixed-17862-atomic-runtime-config-save.md @@ -0,0 +1 @@ +- Config saves now write `/etc/hive/hive.yaml` and `/data/hive.yaml.runtime` atomically (temp file + fsync + rename onto the symlink target), the entrypoint refreshes the legacy `/data/hive.yaml.bak` fallback from a validated runtime config on every clean boot, and falling back to it logs a loud WARN — a liveness kill mid-save no longer boots a months-old ACMM level and roster (#17862). diff --git a/src/deploy/entrypoint.sh b/src/deploy/entrypoint.sh index 66e68db376..87e19e6667 100644 --- a/src/deploy/entrypoint.sh +++ b/src/deploy/entrypoint.sh @@ -64,7 +64,9 @@ EXIT_NET_ADMIN_REQUIRED=77 # file is the only copy of the live config, and mutating it at boot risks # losing owner customisations with no warning. The old name is picked up # read-only until the first save writes the new one, after which both -# exist and the new one wins. +# exist and the new one wins. Once the new name is the boot source, every +# clean boot refreshes the old name from it (#17862) so the fallback is +# never months stale; see hive_refresh_legacy_fallback. HIVE_CONFIG_PATH="${HIVE_CONFIG:-/etc/hive/hive.yaml}" HIVE_CONFIG_RUNTIME="/data/hive.yaml.runtime" HIVE_CONFIG_RUNTIME_LEGACY="/data/hive.yaml.bak" @@ -771,18 +773,89 @@ for _cfg in "$HIVE_CONFIG_RUNTIME" "$HIVE_CONFIG_RUNTIME_LEGACY" /data/hive.yaml done unset _cfg +# hive_file_mtime echoes a file's mtime as epoch seconds (0 when unreadable). +# Both stat dialects are tried because the image is Debian but the helper +# must also behave on the BusyBox/macOS shells the tests run under. +hive_file_mtime() { + stat -c %Y "$1" 2>/dev/null || stat -f %m "$1" 2>/dev/null || echo 0 +} + # hive_runtime_config_read echoes the path to read the persisted runtime # config from: the new name when it is present and non-empty, else the # legacy name when that is, else empty. Read-only — it never creates, # renames or removes anything on the PVC. +# +# #17862: falling back to the legacy name is never routine any more. Every +# clean boot refreshes it from the runtime config (hive_refresh_legacy_fallback), +# so reaching it means the runtime config was lost or truncated since the +# last boot. Say so LOUDLY on stderr with both files' ages: a hive that +# silently booted a 3-month-old hive.yaml.bak (level 3 instead of 6) is the +# incident this guards against. When the legacy copy is OLDER than the +# runtime file it is replacing — i.e. the runtime file was written after +# the last refresh and then damaged — the WARN says the fallback is stale +# and names the overlay/audit paths an operator can reconstruct from. hive_runtime_config_read() { if [ -f "$HIVE_CONFIG_RUNTIME" ] && [ -s "$HIVE_CONFIG_RUNTIME" ]; then echo "$HIVE_CONFIG_RUNTIME" elif [ -f "$HIVE_CONFIG_RUNTIME_LEGACY" ] && [ -s "$HIVE_CONFIG_RUNTIME_LEGACY" ]; then + _rt_state="missing" + _rt_mtime=0 + if [ -f "$HIVE_CONFIG_RUNTIME" ]; then + _rt_state="present but EMPTY (0 bytes — likely truncated by a kill mid-save)" + _rt_mtime="$(hive_file_mtime "$HIVE_CONFIG_RUNTIME")" + fi + _lg_mtime="$(hive_file_mtime "$HIVE_CONFIG_RUNTIME_LEGACY")" + _now="$(date +%s)" + _lg_age_h=$(( (_now - _lg_mtime) / 3600 )) + echo "[entrypoint] WARN: runtime config $HIVE_CONFIG_RUNTIME is $_rt_state; FALLING BACK to legacy $HIVE_CONFIG_RUNTIME_LEGACY (last refreshed ${_lg_age_h}h ago)." >&2 + if [ "$_rt_mtime" -gt "$_lg_mtime" ] 2>/dev/null; then + echo "[entrypoint] WARN: the legacy fallback is OLDER than the damaged runtime config ($(( (_rt_mtime - _lg_mtime) / 3600 ))h behind). Settings saved since then (acmm_level, agent roster, pauses, models) are NOT in this file — check /data/hive.yaml.dashboard and /data/audit.jsonl before trusting the booted config. See hivecommons/hive#17862." >&2 + fi + unset _rt_state _rt_mtime _lg_mtime _now _lg_age_h echo "$HIVE_CONFIG_RUNTIME_LEGACY" fi } +# hive_refresh_legacy_fallback copies a GOOD runtime config over the legacy +# fallback name so the fallback is at most one boot stale (#17862). Before +# this, hive.yaml.bak was frozen at the rename (Aug 2026) on every hive, and +# a truncated runtime file booted a months-old config. +# +# Only runs when THIS boot is reading from the new name (so the legacy file +# is strictly older, never the only copy — the Docker/LXC worry behind the +# original never-write rule) and the runtime file parses as YAML with a +# project section (so a half-written file is never promoted to fallback). +# Copies to a temp name and renames so the fallback is never itself +# truncated by a kill. Best-effort: a failure here only means the fallback +# stays at its previous age. +hive_refresh_legacy_fallback() { + [ "$1" = "$HIVE_CONFIG_RUNTIME" ] || return 0 + [ -s "$HIVE_CONFIG_RUNTIME" ] || return 0 + if command -v python3 >/dev/null 2>&1; then + python3 -c ' +import sys, yaml +with open(sys.argv[1]) as f: + d = yaml.safe_load(f) +sys.exit(0 if isinstance(d, dict) and isinstance(d.get("project"), dict) else 1) +' "$HIVE_CONFIG_RUNTIME" 2>/dev/null || { + echo "[entrypoint] WARN: $HIVE_CONFIG_RUNTIME does not parse as a hive config; legacy fallback $HIVE_CONFIG_RUNTIME_LEGACY left untouched" >&2 + return 0 + } + fi + if cmp -s "$HIVE_CONFIG_RUNTIME" "$HIVE_CONFIG_RUNTIME_LEGACY" 2>/dev/null; then + return 0 + fi + _lg_tmp="${HIVE_CONFIG_RUNTIME_LEGACY}.tmp.$$" + if cp "$HIVE_CONFIG_RUNTIME" "$_lg_tmp" 2>/dev/null && mv -f "$_lg_tmp" "$HIVE_CONFIG_RUNTIME_LEGACY" 2>/dev/null; then + hive_harden_runtime_config "$HIVE_CONFIG_RUNTIME_LEGACY" + echo "[entrypoint] Legacy fallback $HIVE_CONFIG_RUNTIME_LEGACY refreshed from $HIVE_CONFIG_RUNTIME" + else + rm -f "$_lg_tmp" 2>/dev/null + echo "[entrypoint] WARN: could not refresh legacy fallback $HIVE_CONFIG_RUNTIME_LEGACY from $HIVE_CONFIG_RUNTIME" >&2 + fi + unset _lg_tmp +} + # hive_ghe_git_host echoes this hive's configured GitHub host when it is NOT # public github.com (i.e. a GitHub Enterprise instance such as github.ibm.com), # and nothing at all otherwise. Derived the same way the Go binary's @@ -1018,6 +1091,7 @@ if [ "$IS_KUBERNETES" = "true" ]; then # result equal to its own input on every boot. HIVE_CONFIG_BOOT="$(hive_runtime_config_read)" if [ -n "$HIVE_CONFIG_BOOT" ]; then + hive_refresh_legacy_fallback "$HIVE_CONFIG_BOOT" # Steady state: boot from what this hive last had. The ConfigMap seed is # deliberately NOT merged — it is frozen at provision and every field it # could contribute is either already here or re-asserted by the hub on @@ -1207,6 +1281,7 @@ else # first boot on new code seeds hive.yaml.runtime from the legacy content # without touching the legacy file itself. HIVE_CONFIG_SOURCE="$(hive_runtime_config_read)" + hive_refresh_legacy_fallback "$HIVE_CONFIG_SOURCE" if [ -f "$HIVE_CONFIG_PATH" ] && [ -s "$HIVE_CONFIG_PATH" ] && [ -z "$HIVE_CONFIG_SOURCE" ]; then # First boot: config exists but no PVC runtime config yet — seed it cp "$HIVE_CONFIG_PATH" "$HIVE_CONFIG_RUNTIME" diff --git a/src/deploy/test_entrypoint_runtime_config.sh b/src/deploy/test_entrypoint_runtime_config.sh index 4f97399d2c..6e21a20570 100755 --- a/src/deploy/test_entrypoint_runtime_config.sh +++ b/src/deploy/test_entrypoint_runtime_config.sh @@ -2,10 +2,13 @@ # Tests the hive.yaml.bak -> hive.yaml.runtime migration in entrypoint.sh. # # The rename is copy-forward: readers prefer /data/hive.yaml.runtime and fall -# back to the legacy /data/hive.yaml.bak, and NOTHING renames or removes the -# legacy file on the PVC. ~51 live hives carry only the legacy name, and on -# Docker/LXC that file is the single copy of the live config — mutating it at -# boot could lose owner customisations with no warning. +# back to the legacy /data/hive.yaml.bak, and NOTHING removes the legacy file +# on the PVC. ~51 live hives carry only the legacy name, and on Docker/LXC +# that file is the single copy of the live config. +# +# #17862 added one deliberate write: when the NEW name is the boot source and +# parses as a config, the legacy file is refreshed from it (temp + mv) so the +# fallback is never months stale, and falling back to it logs a loud WARN. # # Rather than grepping the script, this extracts the real resolver function and # executes it against real files, so it fails if the logic regresses. @@ -38,9 +41,9 @@ echo "=== entrypoint runtime-config migration tests ===" # Extract the resolver verbatim from the entrypoint so this tests the shipped # code, not a copy that can drift away from it. -RESOLVER="$(sed -n '/^hive_runtime_config_read() {/,/^}/p' "$ENTRYPOINT")" -if [ -z "$RESOLVER" ]; then - echo " FAIL: could not extract hive_runtime_config_read from $ENTRYPOINT" +RESOLVER="$(sed -n '/^hive_file_mtime() {/,/^}/p; /^hive_runtime_config_read() {/,/^}/p' "$ENTRYPOINT")" +if ! grep -q 'hive_runtime_config_read() {' <<<"$RESOLVER" || ! grep -q 'hive_file_mtime() {' <<<"$RESOLVER"; then + echo " FAIL: could not extract hive_runtime_config_read / hive_file_mtime from $ENTRYPOINT" exit 1 fi @@ -63,7 +66,7 @@ resolve() { export HIVE_CONFIG_RUNTIME HIVE_CONFIG_RUNTIME_LEGACY local out - out="$(eval "$RESOLVER"; hive_runtime_config_read)" + out="$(eval "$RESOLVER"; hive_runtime_config_read 2>/dev/null)" case "$out" in "$HIVE_CONFIG_RUNTIME") echo "new" ;; @@ -95,17 +98,96 @@ check "empty new + good legacy -> legacy" "legacy" "$(resolve "EMPTY" "old: cfg" # 6. Both empty is the same as neither. check "empty new + empty legacy -> empty" "none" "$(resolve "EMPTY" "EMPTY")" -# 7. The legacy file must never be renamed or deleted. On Docker/LXC it is the -# only copy of the live config, so a destructive migration could lose owner -# customisations silently. -if grep -Eq '(mv|rm)[^\n]*HIVE_CONFIG_RUNTIME_LEGACY' "$ENTRYPOINT"; then - echo " FAIL: entrypoint moves or removes the legacy runtime config" - echo " the migration must be copy-forward only" +# 7. The legacy file must never be deleted, and the only thing allowed to +# overwrite it is hive_refresh_legacy_fallback (#17862), which copies a +# newer, validated runtime config over it. On Docker/LXC it can be the +# only copy of the live config, so nothing else may touch it. +if grep -Eq 'rm[^\n]*HIVE_CONFIG_RUNTIME_LEGACY' "$ENTRYPOINT"; then + echo " FAIL: entrypoint removes the legacy runtime config" FAIL=$((FAIL + 1)) else - echo " PASS: legacy runtime config is never moved or removed" + echo " PASS: legacy runtime config is never removed" PASS=$((PASS + 1)) fi +REFRESH="$(sed -n '/^hive_refresh_legacy_fallback() {/,/^}/p' "$ENTRYPOINT")" +mv_total="$(grep -cE 'mv[^\n]*HIVE_CONFIG_RUNTIME_LEGACY' "$ENTRYPOINT" || true)" +mv_in_refresh="$(grep -cE 'mv[^\n]*HIVE_CONFIG_RUNTIME_LEGACY' <<<"$REFRESH" || true)" +check "legacy runtime config is only overwritten inside hive_refresh_legacy_fallback" "$mv_total" "$mv_in_refresh" + +# ── #17862: the legacy fallback is refreshed on clean boots and WARNs loudly ── +# +# The incident: a kill mid-save left a 0-byte hive.yaml.runtime, the resolver +# fell back to a hive.yaml.bak frozen three months earlier, and the hive +# booted a stale ACMM level with no indication in the log. Two properties +# close it: (a) a clean boot from the new name refreshes the legacy copy so it +# is at most one boot old, and (b) a fallback is announced on stderr. + +echo +echo "=== #17862: legacy fallback freshness and WARN ===" + +# resolve_stderr — stderr the resolver emits. +resolve_stderr() { + local new_content="$1" legacy_content="$2" + local dir + dir="$(mktemp -d)" + # shellcheck disable=SC2064 + trap "rm -rf '$dir'" RETURN + HIVE_CONFIG_RUNTIME="$dir/hive.yaml.runtime" + HIVE_CONFIG_RUNTIME_LEGACY="$dir/hive.yaml.bak" + [ -n "$new_content" ] && printf '%s' "$new_content" > "$HIVE_CONFIG_RUNTIME" + [ -n "$legacy_content" ] && printf '%s' "$legacy_content" > "$HIVE_CONFIG_RUNTIME_LEGACY" + [ "$new_content" = "EMPTY" ] && : > "$HIVE_CONFIG_RUNTIME" + export HIVE_CONFIG_RUNTIME HIVE_CONFIG_RUNTIME_LEGACY + (eval "$RESOLVER"; hive_runtime_config_read 2>&1 >/dev/null) || true +} + +# (b) Falling back is loud; the steady state is silent. +warn_out="$(resolve_stderr "EMPTY" "old: cfg")" +if grep -q 'WARN.*FALLING BACK to legacy' <<<"$warn_out" && grep -q '0 bytes' <<<"$warn_out"; then + echo " PASS: empty runtime + legacy fallback logs a loud WARN naming the truncation" + PASS=$((PASS + 1)) +else + echo " FAIL: fallback to legacy did not WARN (got: '$warn_out')" + FAIL=$((FAIL + 1)) +fi +check "steady-state boot from the new name is silent" "" "$(resolve_stderr "new: cfg" "old: cfg")" + +# (a) hive_refresh_legacy_fallback: the legacy copy is overwritten from a +# valid runtime file, never from an invalid one, and only when the new +# name is the boot source. +HARDEN_FN="$(sed -n '/^hive_harden_runtime_config() {/,/^}/p' "$ENTRYPOINT")" +refresh_case() { + # refresh_case ; prints legacy content after. + local src="$1" rt="$2" lg="$3" dir + dir="$(mktemp -d)" + # shellcheck disable=SC2064 + trap "rm -rf '$dir'" RETURN + HIVE_CONFIG_RUNTIME="$dir/hive.yaml.runtime" + HIVE_CONFIG_RUNTIME_LEGACY="$dir/hive.yaml.bak" + HIVE_RUNTIME_USER="$(id -un)"; HIVE_RUNTIME_GROUP="$(id -gn)" + export HIVE_CONFIG_RUNTIME HIVE_CONFIG_RUNTIME_LEGACY HIVE_RUNTIME_USER HIVE_RUNTIME_GROUP + printf '%s' "$rt" > "$HIVE_CONFIG_RUNTIME" + printf '%s' "$lg" > "$HIVE_CONFIG_RUNTIME_LEGACY" + local arg="$HIVE_CONFIG_RUNTIME" + [ "$src" = legacy ] && arg="$HIVE_CONFIG_RUNTIME_LEGACY" + (eval "$HARDEN_FN"; eval "$REFRESH"; hive_refresh_legacy_fallback "$arg" >/dev/null 2>&1) || true + cat "$HIVE_CONFIG_RUNTIME_LEGACY" +} +# Contents are single-line here so $(...) newline stripping cannot bite. +GOOD_RT='{project: {org: acme}, acmm_level: 6}' +STALE_LG='{acmm_level: 3}' +check "clean boot from new name refreshes the legacy fallback" \ + "$GOOD_RT" "$(refresh_case new "$GOOD_RT" "$STALE_LG")" +check "boot from the legacy name leaves it untouched" \ + "$STALE_LG" "$(refresh_case legacy "$GOOD_RT" "$STALE_LG")" +if command -v python3 >/dev/null 2>&1 && python3 -c 'import yaml' 2>/dev/null; then + check "unparsable runtime config never overwrites the fallback" \ + "$STALE_LG" "$(refresh_case new 'project: [unterminated' "$STALE_LG")" + check "runtime config without a project section never overwrites the fallback" \ + "$STALE_LG" "$(refresh_case new '{acmm_level: 0}' "$STALE_LG")" +else + hive_test_skip "python3+yaml unavailable — cannot exercise the parse guard in hive_refresh_legacy_fallback" +fi # 8. Every write of the config path to the PVC targets the NEW name. Counted, # not just grepped: there are two such writes (the K8s post-merge snapshot diff --git a/src/pkg/config/atomic_write.go b/src/pkg/config/atomic_write.go new file mode 100644 index 0000000000..bdd969964d --- /dev/null +++ b/src/pkg/config/atomic_write.go @@ -0,0 +1,116 @@ +package config + +import ( + "errors" + "fmt" + "os" + "path/filepath" +) + +// writeFileAtomic replaces the contents of path without ever exposing a +// truncated or partial file to a concurrent reader or to the next boot. +// +// Motivation (#17862): Save used O_WRONLY|O_TRUNC in place on the PVC runtime +// config. A SIGKILL (liveness probe) landing between the truncate and the +// write left a 0-byte /data/hive.yaml.runtime; the entrypoint's `-s` check +// then fell back to a months-stale /data/hive.yaml.bak and a hive booted +// with a three-month-old ACMM level and roster. +// +// Strategy: write data to a temp file in the TARGET's directory, fsync it, +// then rename it onto the target. Two constraints shape the details: +// +// - path may be a symlink (hosted spokes link /etc/hive/hive.yaml to the +// PVC runtime config, #17468). The rename must land on the link's +// TARGET, not replace the link with a regular file, so the final path is +// resolved with EvalSymlinks first. Pinned by TestSaveThroughSymlinkKeepsLink. +// - path may be a single-file Docker bind mount, where rename over the +// mount point fails (EBUSY) and would also swap the inode the host sees. +// When the rename fails, fall back to the previous in-place write so +// those deployments keep working exactly as before; they never had +// atomicity and lose nothing here. +// +// mode is applied to the temp file before the rename, so a pre-existing +// world-readable inode does not carry its bits through (WriteFile's mode +// only applies on create). The fallback path chmods explicitly for the same +// reason. +func writeFileAtomic(path string, data []byte, mode os.FileMode) error { + target := path + if resolved, err := filepath.EvalSymlinks(path); err == nil { + target = resolved + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("resolving %s: %w", path, err) + } + + dir := filepath.Dir(target) + tmp, err := os.CreateTemp(dir, "."+filepath.Base(target)+".tmp-*") + if err != nil { + return fmt.Errorf("creating temp file in %s: %w", dir, err) + } + tmpPath := tmp.Name() + cleanup := func() { _ = os.Remove(tmpPath) } + + if err := tmp.Chmod(mode); err != nil { + _ = tmp.Close() + cleanup() + return fmt.Errorf("chmod temp file %s: %w", tmpPath, err) + } + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + cleanup() + return fmt.Errorf("writing temp file %s: %w", tmpPath, err) + } + if err := tmp.Sync(); err != nil { + _ = tmp.Close() + cleanup() + return fmt.Errorf("syncing temp file %s: %w", tmpPath, err) + } + if err := tmp.Close(); err != nil { + cleanup() + return fmt.Errorf("closing temp file %s: %w", tmpPath, err) + } + + if err := os.Rename(tmpPath, target); err != nil { + cleanup() + // Bind-mounted single file (or any filesystem that refuses the + // rename): keep the inode and write in place, as Save always did. + if inPlaceErr := writeFileInPlace(target, data, mode); inPlaceErr != nil { + return fmt.Errorf("renaming %s onto %s: %w (in-place fallback also failed: %v)", tmpPath, target, err, inPlaceErr) + } + return nil + } + syncDir(dir) + return nil +} + +// writeFileInPlace is the pre-#17862 O_TRUNC write, kept only as the fallback +// for targets that cannot be renamed over. It is NOT crash-safe. +func writeFileInPlace(target string, data []byte, mode os.FileMode) error { + f, err := os.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, mode) + if err != nil { + return err + } + if _, err := f.Write(data); err != nil { + _ = f.Close() + return err + } + if err := f.Sync(); err != nil { + _ = f.Close() + return err + } + if err := f.Close(); err != nil { + return err + } + return os.Chmod(target, mode) +} + +// syncDir fsyncs a directory so the rename that just happened in it is +// durable. Best-effort: some filesystems (and all of Windows) refuse to open +// a directory for sync, and a failure here cannot un-do the rename. +func syncDir(dir string) { + d, err := os.Open(dir) + if err != nil { + return + } + _ = d.Sync() + _ = d.Close() +} diff --git a/src/pkg/config/atomic_write_test.go b/src/pkg/config/atomic_write_test.go new file mode 100644 index 0000000000..dfdb561e23 --- /dev/null +++ b/src/pkg/config/atomic_write_test.go @@ -0,0 +1,288 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestWriteFileAtomicReplacesContentAndMode covers the plain case: an +// existing world-readable file is replaced wholesale and ends up with the +// requested mode, and no temp file is left behind. +func TestWriteFileAtomicReplacesContentAndMode(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "hive.yaml.runtime") + if err := os.WriteFile(path, []byte("old: content\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := writeFileAtomic(path, []byte("new: content\n"), 0o600); err != nil { + t.Fatalf("writeFileAtomic: %v", err) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != "new: content\n" { + t.Fatalf("content = %q, want new content", got) + } + fi, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if fi.Mode().Perm() != 0o600 { + t.Fatalf("mode = %o, want 0600 (pre-existing 0644 bits must not survive)", fi.Mode().Perm()) + } + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatal(err) + } + for _, e := range entries { + if strings.Contains(e.Name(), ".tmp-") { + t.Fatalf("temp file %s left behind", e.Name()) + } + } +} + +// TestWriteFileAtomicCreatesMissingFile pins that a missing target is +// created (Save's old create fallback) rather than failing on EvalSymlinks. +func TestWriteFileAtomicCreatesMissingFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "hive.yaml") + if err := writeFileAtomic(path, []byte("a: 1\n"), 0o644); err != nil { + t.Fatalf("writeFileAtomic on missing file: %v", err) + } + if got, _ := os.ReadFile(path); string(got) != "a: 1\n" { + t.Fatalf("content = %q", got) + } +} + +// TestWriteFileAtomicThroughSymlinkKeepsLink is the #17468 constraint at the +// writer level: the rename must land on the link TARGET so the symlink the +// copy-config init container created survives, and the target carries the +// new bytes. +func TestWriteFileAtomicThroughSymlinkKeepsLink(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "data", "hive.yaml.runtime") + link := filepath.Join(dir, "etc", "hive.yaml") + for _, d := range []string{filepath.Dir(target), filepath.Dir(link)} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(target, []byte("old\n"), 0o600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + if err := writeFileAtomic(link, []byte("new\n"), 0o600); err != nil { + t.Fatalf("writeFileAtomic through link: %v", err) + } + fi, err := os.Lstat(link) + if err != nil { + t.Fatal(err) + } + if fi.Mode()&os.ModeSymlink == 0 { + t.Fatalf("%s is no longer a symlink — rename replaced the link instead of its target", link) + } + if got, _ := os.ReadFile(target); string(got) != "new\n" { + t.Fatalf("target content = %q, want new", got) + } + // Nothing may be left in the link's directory: the temp file must be + // created next to the TARGET, or the rename would cross directories. + entries, _ := os.ReadDir(filepath.Dir(link)) + if len(entries) != 1 { + t.Fatalf("link dir has %d entries, want just the link", len(entries)) + } +} + +// TestSaveSurvivesKillMidWrite is the #17862 regression. It cannot SIGKILL +// the test process, so it pins the property that makes a kill harmless: +// at no point during Save does the runtime config path refer to a file +// with fewer bytes than the previous complete config. A temp-file+rename +// writer satisfies this trivially; the old O_TRUNC in-place write fails it +// because the file is observably 0 bytes between truncate and write. +// +// The check is made by racing a reader against repeated Saves and +// asserting it never observes an empty or partial file. +func TestSaveSurvivesKillMidWrite(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, "hive.yaml") + runtimePath := filepath.Join(dir, "hive.yaml.runtime") + origOverlay, origRuntime := DashboardOverlayFile, RuntimeConfigFile + DashboardOverlayFile = filepath.Join(dir, "hive.yaml.dashboard") + RuntimeConfigFile = runtimePath + t.Cleanup(func() { DashboardOverlayFile, RuntimeConfigFile = origOverlay, origRuntime }) + + level := 6 + cfg := &Config{ + SourcePath: cfgPath, + Project: ProjectConfig{Org: "testorg", Name: "test"}, + AgentRuntimeConfig: AgentRuntimeConfig{ + Agents: map[string]AgentConfig{"scanner": {Role: "scanner"}}, + }, + ACMMLevel: &level, + } + if err := cfg.Save(); err != nil { + t.Fatalf("initial Save: %v", err) + } + full, err := os.ReadFile(runtimePath) + if err != nil || len(full) == 0 { + t.Fatalf("initial runtime config unreadable: %v", err) + } + + stop := make(chan struct{}) + bad := make(chan string, 1) + go func() { + for { + select { + case <-stop: + return + default: + } + for _, p := range []string{runtimePath, cfgPath} { + got, err := os.ReadFile(p) + if err != nil { + continue // a rename window may briefly ENOENT on some filesystems; never partial + } + if len(got) < len(full) { + select { + case bad <- p: + default: + } + return + } + } + } + }() + for i := 0; i < 200; i++ { + if err := cfg.Save(); err != nil { + t.Fatalf("Save %d: %v", i, err) + } + } + close(stop) + select { + case p := <-bad: + t.Fatalf("reader observed a truncated/partial config at %s during Save — write is not atomic", p) + default: + } +} + +func TestWriteFileAtomicMissingDirFails(t *testing.T) { + path := filepath.Join(t.TempDir(), "no-such-dir", "hive.yaml") + err := writeFileAtomic(path, []byte("x"), 0o600) + if err == nil { + t.Fatal("expected error when target directory does not exist") + } + if !strings.Contains(err.Error(), "creating temp file") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestWriteFileAtomicResolveErrorIsReported(t *testing.T) { + dir := t.TempDir() + file := filepath.Join(dir, "regular") + if err := os.WriteFile(file, []byte("f"), 0o600); err != nil { + t.Fatal(err) + } + // Using a regular file as a path component makes EvalSymlinks fail with + // ENOTDIR, which is not ErrNotExist and must surface as a resolve error. + err := writeFileAtomic(filepath.Join(file, "hive.yaml"), []byte("x"), 0o600) + if err == nil { + t.Fatal("expected resolve error") + } + if !strings.Contains(err.Error(), "resolving") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestWriteFileAtomicDanglingSymlinkIsReplaced(t *testing.T) { + dir := t.TempDir() + link := filepath.Join(dir, "hive.yaml") + if err := os.Symlink(filepath.Join(dir, "missing-target"), link); err != nil { + t.Skipf("symlink not supported: %v", err) + } + if err := writeFileAtomic(link, []byte("fresh"), 0o600); err != nil { + t.Fatalf("writeFileAtomic: %v", err) + } + got, err := os.ReadFile(link) + if err != nil { + t.Fatal(err) + } + if string(got) != "fresh" { + t.Fatalf("got %q", got) + } +} + +func TestWriteFileAtomicRenameFailureFallsBackAndReportsBoth(t *testing.T) { + dir := t.TempDir() + // A non-empty directory as the target: rename of a file onto it fails, + // and the in-place O_TRUNC fallback cannot open a directory for writing. + target := filepath.Join(dir, "hive.yaml") + if err := os.MkdirAll(filepath.Join(target, "child"), 0o700); err != nil { + t.Fatal(err) + } + err := writeFileAtomic(target, []byte("x"), 0o600) + if err == nil { + t.Fatal("expected rename + fallback failure") + } + if !strings.Contains(err.Error(), "in-place fallback also failed") { + t.Fatalf("unexpected error: %v", err) + } + entries, _ := os.ReadDir(dir) + for _, e := range entries { + if strings.HasPrefix(e.Name(), ".hive.yaml.tmp-") { + t.Fatalf("temp file %s leaked after failure", e.Name()) + } + } +} + +func TestWriteFileInPlaceWritesAndSetsMode(t *testing.T) { + path := filepath.Join(t.TempDir(), "hive.yaml") + if err := os.WriteFile(path, []byte("old-longer-content"), 0o644); err != nil { + t.Fatal(err) + } + if err := writeFileInPlace(path, []byte("new"), 0o600); err != nil { + t.Fatalf("writeFileInPlace: %v", err) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != "new" { + t.Fatalf("got %q, want truncated rewrite", got) + } + st, err := os.Stat(path) + if err != nil { + t.Fatal(err) + } + if st.Mode().Perm() != 0o600 { + t.Fatalf("mode = %o, want 600", st.Mode().Perm()) + } +} + +func TestWriteFileInPlaceOpenFailure(t *testing.T) { + path := filepath.Join(t.TempDir(), "missing", "hive.yaml") + if err := writeFileInPlace(path, []byte("x"), 0o600); err == nil { + t.Fatal("expected open failure for missing directory") + } +} + +func TestSyncDirToleratesMissingDir(t *testing.T) { + syncDir(filepath.Join(t.TempDir(), "nope")) + syncDir(t.TempDir()) +} + +func TestSourceConfigModePreservesExistingOrDefaults(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "hive.yaml") + if err := os.WriteFile(path, []byte("a: 1\n"), 0o600); err != nil { + t.Fatal(err) + } + if got := sourceConfigMode(path); got != 0o600 { + t.Fatalf("existing mode = %o, want 600", got) + } + if got := sourceConfigMode(filepath.Join(dir, "absent.yaml")); got != 0o644 { + t.Fatalf("default mode = %o, want 644", got) + } +} diff --git a/src/pkg/config/config.go b/src/pkg/config/config.go index 6c28e87550..6b6aa56633 100644 --- a/src/pkg/config/config.go +++ b/src/pkg/config/config.go @@ -7752,18 +7752,14 @@ func (c *Config) saveLocked() error { // layer this Save is about to persist. overlayErr := c.saveDashboardOverlay() - // Open the existing file (preserving its inode) rather than creating a - // temp file and renaming. Rename breaks Docker bind mounts because it - // replaces the inode — the host file is never updated, so acmm_level - // and other runtime changes are lost on container restart. + // Replace the primary config atomically (temp file + fsync + rename onto + // the resolved target) — see writeFileAtomic for why that is safe on the + // #17468 symlink and still works on single-file Docker bind mounts. // - // #17468: the same in-place write is what keeps hosted spokes durable. - // There the copy-config init container makes SourcePath - // (/etc/hive/hive.yaml, an emptyDir) a symlink to RuntimeConfigFile on - // the PVC; O_WRONLY|O_TRUNC follows the link and writes the PVC file, - // whereas a temp-file+rename would replace the link with a regular - // emptyDir file and silently revert the fix. Pinned by - // TestSaveThroughSymlinkKeepsLink — do not switch this to a rename. + // #17862: the previous O_WRONLY|O_TRUNC in-place write could be killed + // between truncate and write (liveness probe on a crash-looping hive), + // leaving a 0-byte runtime config that the entrypoint skipped in favour + // of a months-stale hive.yaml.bak. Pinned by TestSaveSurvivesKillMidWrite. // // #3961: a source-path failure must NOT abort the save. On deployments // that mount the config read-only (a ConfigMap mounted straight at @@ -7777,23 +7773,8 @@ func (c *Config) saveLocked() error { // writing the runtime layer, and report success iff the state will actually // survive a restart. var srcErr error - f, err := os.OpenFile(c.SourcePath, os.O_WRONLY|os.O_TRUNC, 0o644) - if err != nil { - // File may not exist yet — fall back to create. Continue below so - // the PVC backup and dashboard overlay are still written. - if writeErr := os.WriteFile(c.SourcePath, data, 0o644); writeErr != nil { - srcErr = fmt.Errorf("writing config (create fallback): %w", writeErr) - } - } else { - if _, err := f.Write(data); err != nil { - _ = f.Close() // best-effort cleanup; the write error is what's recorded - srcErr = fmt.Errorf("writing config: %w", err) - } else if err := f.Sync(); err != nil { - _ = f.Close() // best-effort cleanup; the sync error is what's recorded - srcErr = fmt.Errorf("syncing config: %w", err) - } else if err := f.Close(); err != nil { - srcErr = fmt.Errorf("closing config: %w", err) - } + if err := writeFileAtomic(c.SourcePath, data, sourceConfigMode(c.SourcePath)); err != nil { + srcErr = fmt.Errorf("writing config: %w", err) } // Persist the runtime config to the PVC. In K8s this is the boot input @@ -7817,11 +7798,11 @@ func (c *Config) saveLocked() error { if guardLivePVCPathUnderTest(runtimePath, defaultRuntimeConfigFile, "RuntimeConfigFile") { // Not written, so not persisted: keep the primary-path error visible. runtimeErr = errLivePVCPathGuarded - } else if err := os.WriteFile(runtimePath, data, 0o600); err != nil { + } else if err := writeFileAtomic(runtimePath, data, 0o600); err != nil { // Common cause: init container created the file as root, runtime user // can't overwrite. Remove and retry so runtime state is not silently lost. - _ = os.Remove(runtimePath) // best-effort; the retry's own WriteFile error is what's recorded below - if retryErr := os.WriteFile(runtimePath, data, 0o600); retryErr != nil { + _ = os.Remove(runtimePath) // best-effort; the retry's own error is what's recorded below + if retryErr := writeFileAtomic(runtimePath, data, 0o600); retryErr != nil { runtimeErr = retryErr log.Printf("[config] warning: failed to write PVC runtime config to %s (even after remove): %v", runtimePath, retryErr) } else { @@ -7829,12 +7810,6 @@ func (c *Config) saveLocked() error { } } else { log.Printf("[config] PVC runtime config written to %s", runtimePath) - // os.WriteFile's mode only applies when it CREATES the file; a - // pre-existing world-readable inode (every hive deployed before - // this fix) keeps its old 0644 bits, so tighten explicitly. - if chmodErr := os.Chmod(runtimePath, 0o600); chmodErr != nil { - log.Printf("[config] warning: failed to tighten permissions on %s: %v", runtimePath, chmodErr) - } } if srcErr == nil { @@ -7853,6 +7828,16 @@ func (c *Config) saveLocked() error { return srcErr } +// sourceConfigMode returns the mode Save should give the primary config +// path: the existing file's permission bits when it exists (an operator may +// have tightened them), else the historical 0644 default. +func sourceConfigMode(path string) os.FileMode { + if fi, err := os.Stat(path); err == nil { + return fi.Mode().Perm() + } + return 0o644 +} + // RuntimeConfigFile is where Save() persists the full runtime config on the // PVC. Its role differs by environment, which is exactly why the old // hive.yaml.bak name was misleading enough to cost debugging time: @@ -7877,11 +7862,12 @@ const defaultRuntimeConfigFile = "/data/hive.yaml.runtime" // RuntimeConfigFileLegacy is the pre-rename name of RuntimeConfigFile. // -// It is READ as a fallback and never written, renamed or removed: ~51 live -// hives carry only this file, and on Docker/LXC it is the single copy of -// their live configuration. Mutating it at boot could lose owner -// customisations with no warning, so the migration is copy-forward only — -// readers prefer RuntimeConfigFile and fall back to this one. +// It is READ as a fallback and never written by Save: ~51 live hives carry +// only this file, and on Docker/LXC it is the single copy of their live +// configuration. Readers prefer RuntimeConfigFile and fall back to this one. +// The entrypoint refreshes it from a good RuntimeConfigFile on every clean +// boot (#17862) — a fallback frozen at the Aug 2026 rename booted a +// months-old level/roster when a kill mid-save truncated the runtime file. // // Removable one release after every live hive has written the new name. const RuntimeConfigFileLegacy = "/data/hive.yaml.bak" diff --git a/src/pkg/hub/saas_provision.go b/src/pkg/hub/saas_provision.go index 8ef3b3db00..b5de4a99ca 100644 --- a/src/pkg/hub/saas_provision.go +++ b/src/pkg/hub/saas_provision.go @@ -3464,11 +3464,12 @@ spec: # Steady state (#17468): /etc/hive is an emptyDir, so a regular file # there is a throwaway cache and operator edits to it vanished on # restart. Symlink it to the PVC runtime config instead, so the path - # every runbook names IS the persisted boot source. Config.Save writes - # the config path in place (O_TRUNC, no rename), so the link survives - # saves. Only the NEW name is linked: the legacy .bak is never written - # (config.RuntimeConfigFileLegacy), and linking it would make every - # save write it. + # every runbook names IS the persisted boot source. Config.Save + # resolves the link and renames its temp file onto the TARGET + # (#17862), so the link survives saves. Only the NEW name is linked: + # the legacy .bak is refreshed by the entrypoint from a good runtime + # file on clean boots (never written by Save), and linking it would + # make every save write it. command: ["sh", "-c", "if [ -s /data/hive.yaml.runtime ]; then echo runtime-config-exists-for-recovery; ln -sf /data/hive.yaml.runtime /etc/hive/hive.yaml && echo config-path-linked-to-runtime || echo config-path-link-failed; elif [ -s /data/hive.yaml.bak ]; then echo legacy-runtime-config-exists-for-recovery; else cp /etc/hive-seed/hive.yaml /etc/hive/hive.yaml && echo configmap-copied-first-boot; fi"] volumeMounts: - name: config