From 9af0f86cd29393c7aef8a18c058bdb7eb2740b64 Mon Sep 17 00:00:00 2001 From: "pithead-developer[bot]" Date: Wed, 30 Sep 2026 17:08:56 +0000 Subject: [PATCH 1/3] fix: claim refresh override before writing its contents --- tests/README.md | 4 +++- tests/e2e-refresh-window.sh | 25 ++++++++++++++++------ tests/test-refresh-profile.sh | 39 +++++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 7 deletions(-) diff --git a/tests/README.md b/tests/README.md index ebc0dea..7398e94 100644 --- a/tests/README.md +++ b/tests/README.md @@ -34,8 +34,10 @@ for five minutes around the apply; the changed ceiling must still appear within Control-phase requests pass the temporary bearer token through curl's stdin configuration, keeping it out of process arguments. Each POST uses its own securely allocated response file, removed after reading the change identifier. +The drop-in is opened exclusively and claimed before writing: a partial write fails setup but remains +owned for cleanup; an existing or competing override is refused without being claimed or removed. Cleanup quiesces the timer and instrumented pass, emits only measurement records from the journal, removes -the drop-in, reloads systemd and restores the timer’s prior active state before config/runtime restoration. +the owned drop-in, reloads systemd and restores the timer’s prior active state before config/runtime restoration. The job fails if journal collection or measurement export fails, no profiled refresh completes successfully, or cleanup fails. Begin events from unfinished helpers are retained to locate stalled operations, including a pass stopped by cleanup. diff --git a/tests/e2e-refresh-window.sh b/tests/e2e-refresh-window.sh index f05a4e2..fe78627 100644 --- a/tests/e2e-refresh-window.sh +++ b/tests/e2e-refresh-window.sh @@ -13,12 +13,25 @@ refresh_profile_start() { systemctl is-active --quiet rigforge-api-refresh.timer && REFRESH_PROFILE_TIMER_ACTIVE=1 REFRESH_PROFILE_SINCE=$(date -u '+%Y-%m-%d %H:%M:%S') local dropin="$dir/rigforge-profile.conf" - ( - umask 077 - set -o noclobber - printf '[Service]\nExecStart=\nExecStart=/bin/bash %s %s\n' "$script" "$RIGFORGE" >"$dropin" - ) || return 1 - REFRESH_PROFILE_DROPIN="$dropin" + local original_umask original_noclobber=0 + original_umask=$(umask) + case $- in *C*) original_noclobber=1 ;; esac + umask 077 + set -o noclobber + # Claim only after the exclusive open succeeds, before any write can fail. + # The scoped descriptor preserves a caller's fd 9 and avoids reopening the pathname. + if { + umask "$original_umask" + [ "$original_noclobber" = 1 ] || set +o noclobber + REFRESH_PROFILE_DROPIN="$dropin" + printf '[Service]\nExecStart=\nExecStart=/bin/bash %s %s\n' "$script" "$RIGFORGE" >&9 + } 9>"$dropin"; then + : + else + umask "$original_umask" + [ "$original_noclobber" = 1 ] || set +o noclobber + return 1 + fi systemctl daemon-reload } refresh_profile_state() { diff --git a/tests/test-refresh-profile.sh b/tests/test-refresh-profile.sh index 2be9763..ef38562 100644 --- a/tests/test-refresh-profile.sh +++ b/tests/test-refresh-profile.sh @@ -98,6 +98,45 @@ if refresh_profile_start; then exit 1; fi refresh_profile_finish check test "$(cat "$owned")" = 'existing override' rm "$owned" +# A failed partial write must remain owned until cleanup removes it. +saved_umask=$(umask) +umask 027 +exec 9>"$CASE_DIR/caller-fd" +printf() { + if [[ "$1" == '[Service]'* ]]; then + builtin printf '[Service]\nExecStart=\n' + return 7 + fi + builtin printf "$@" +} +if refresh_profile_start; then exit 1; fi +unset -f printf +check test "$(umask)" = 0027 +case $- in *C*) exit 1 ;; esac +builtin printf 'caller descriptor preserved\n' >&9 +exec 9>&- +check test "$(cat "$CASE_DIR/caller-fd")" = 'caller descriptor preserved' +umask "$saved_umask" +check test "$REFRESH_PROFILE_DROPIN" = "$owned" +check test "$(cat "$owned")" = "$(builtin printf '[Service]\nExecStart=')" +refresh_profile_finish >"$CASE_DIR/partial-write-collected" +check test ! -e "$owned" +check test -z "$REFRESH_PROFILE_DROPIN" +# A competitor arriving after the initial existence check must not be claimed or removed. +mkdir() { + command mkdir "$@" || return + builtin printf 'competing override\n' >"$owned" +} +set -o noclobber +if refresh_profile_start 2>"$CASE_DIR/collision-error"; then exit 1; fi +unset -f mkdir +case $- in *C*) : ;; *) exit 1 ;; esac +set +o noclobber +check test "$(umask)" = "$saved_umask" +check test -z "$REFRESH_PROFILE_DROPIN" +refresh_profile_finish +check test "$(cat "$owned")" = 'competing override' +rm "$owned" # A window without a completed profiled refresh cannot report successful diagnostics. refresh_profile_start journalctl() { :; } From 71948f3322791d7442d1da2d36ddb0a1fb09a7f9 Mon Sep 17 00:00:00 2001 From: "pithead-developer[bot]" Date: Wed, 30 Sep 2026 17:13:33 +0000 Subject: [PATCH 2/3] fix: stage refresh policy before atomic publication --- tests/README.md | 5 +++-- tests/e2e-refresh-window.sh | 34 +++++++++++++--------------------- tests/test-refresh-profile.sh | 35 +++++++++++++++++++++++++++++++++-- 3 files changed, 49 insertions(+), 25 deletions(-) diff --git a/tests/README.md b/tests/README.md index 7398e94..086cb7a 100644 --- a/tests/README.md +++ b/tests/README.md @@ -34,8 +34,9 @@ for five minutes around the apply; the changed ceiling must still appear within Control-phase requests pass the temporary bearer token through curl's stdin configuration, keeping it out of process arguments. Each POST uses its own securely allocated response file, removed after reading the change identifier. -The drop-in is opened exclusively and claimed before writing: a partial write fails setup but remains -owned for cleanup; an existing or competing override is refused without being claimed or removed. +The drop-in is written in a private staging file and published atomically with `link` only after the +write succeeds. A partial write fails setup and cleanup removes the staging file. Publication refuses +an existing or competing destination, including symlinks, without claiming or removing it. Cleanup quiesces the timer and instrumented pass, emits only measurement records from the journal, removes the owned drop-in, reloads systemd and restores the timer’s prior active state before config/runtime restoration. The job fails if journal collection or measurement export fails, no profiled refresh completes diff --git a/tests/e2e-refresh-window.sh b/tests/e2e-refresh-window.sh index fe78627..aad6204 100644 --- a/tests/e2e-refresh-window.sh +++ b/tests/e2e-refresh-window.sh @@ -1,6 +1,6 @@ # shellcheck shell=bash # Reserved control-phase lifecycle; overrides only ExecStart, retaining the real service policy. -REFRESH_PROFILE_DROPIN="" REFRESH_PROFILE_SINCE="" REFRESH_PROFILE_TIMER_ACTIVE="" +REFRESH_PROFILE_STAGING="" REFRESH_PROFILE_DROPIN="" REFRESH_PROFILE_SINCE="" REFRESH_PROFILE_TIMER_ACTIVE="" refresh_profile_start() { local dir="${REFRESH_PROFILE_UNIT_ROOT:-/run/systemd/system}/rigforge-api-refresh.service.d" local script @@ -13,25 +13,14 @@ refresh_profile_start() { systemctl is-active --quiet rigforge-api-refresh.timer && REFRESH_PROFILE_TIMER_ACTIVE=1 REFRESH_PROFILE_SINCE=$(date -u '+%Y-%m-%d %H:%M:%S') local dropin="$dir/rigforge-profile.conf" - local original_umask original_noclobber=0 - original_umask=$(umask) - case $- in *C*) original_noclobber=1 ;; esac - umask 077 - set -o noclobber - # Claim only after the exclusive open succeeds, before any write can fail. - # The scoped descriptor preserves a caller's fd 9 and avoids reopening the pathname. - if { - umask "$original_umask" - [ "$original_noclobber" = 1 ] || set +o noclobber - REFRESH_PROFILE_DROPIN="$dropin" - printf '[Service]\nExecStart=\nExecStart=/bin/bash %s %s\n' "$script" "$RIGFORGE" >&9 - } 9>"$dropin"; then - : - else - umask "$original_umask" - [ "$original_noclobber" = 1 ] || set +o noclobber - return 1 - fi + # Stage privately: failed writes must never expose an incomplete service policy. + REFRESH_PROFILE_STAGING=$(mktemp "$dir/.rigforge-profile.XXXXXX") || return 1 + printf '[Service]\nExecStart=\nExecStart=/bin/bash %s %s\n' "$script" "$RIGFORGE" >|"$REFRESH_PROFILE_STAGING" || return 1 + # POSIX link uses the exact destination (unlike ln's directory handling) and never replaces it. + link "$REFRESH_PROFILE_STAGING" "$dropin" || return 1 + REFRESH_PROFILE_DROPIN="$dropin" + rm -f "$REFRESH_PROFILE_STAGING" || return 1 + REFRESH_PROFILE_STAGING="" systemctl daemon-reload } refresh_profile_state() { @@ -42,8 +31,11 @@ refresh_profile_state() { -p ExecMainStartTimestamp -p ExecMainExitTimestamp -p ExecMainStatus } refresh_profile_finish() { - [ -n "$REFRESH_PROFILE_DROPIN" ] || return 0 local log rc=0 + if [ -n "$REFRESH_PROFILE_STAGING" ]; then + if rm -f "$REFRESH_PROFILE_STAGING"; then REFRESH_PROFILE_STAGING=""; else rc=1; fi + fi + [ -n "$REFRESH_PROFILE_DROPIN" ] || return "$rc" refresh_profile_state || rc=1 # Quiesce both units before collecting: the timer could otherwise dispatch during cleanup. systemctl stop rigforge-api-refresh.timer || rc=1 diff --git a/tests/test-refresh-profile.sh b/tests/test-refresh-profile.sh index ef38562..1aadcef 100644 --- a/tests/test-refresh-profile.sh +++ b/tests/test-refresh-profile.sh @@ -117,10 +117,14 @@ builtin printf 'caller descriptor preserved\n' >&9 exec 9>&- check test "$(cat "$CASE_DIR/caller-fd")" = 'caller descriptor preserved' umask "$saved_umask" -check test "$REFRESH_PROFILE_DROPIN" = "$owned" -check test "$(cat "$owned")" = "$(builtin printf '[Service]\nExecStart=')" +check test -z "$REFRESH_PROFILE_DROPIN" +check test ! -e "$owned" +staged="$REFRESH_PROFILE_STAGING" +check test "$(cat "$staged")" = "$(builtin printf '[Service]\nExecStart=')" refresh_profile_finish >"$CASE_DIR/partial-write-collected" check test ! -e "$owned" +check test ! -e "$staged" +check test -z "$REFRESH_PROFILE_STAGING" check test -z "$REFRESH_PROFILE_DROPIN" # A competitor arriving after the initial existence check must not be claimed or removed. mkdir() { @@ -137,6 +141,33 @@ check test -z "$REFRESH_PROFILE_DROPIN" refresh_profile_finish check test "$(cat "$owned")" = 'competing override' rm "$owned" +# A raced symlink to a nonregular target must also remain untouched. +mkdir() { + command mkdir "$@" || return + ln -s /dev/null "$owned" +} +if refresh_profile_start 2>"$CASE_DIR/symlink-collision-error"; then exit 1; fi +unset -f mkdir +check test -z "$REFRESH_PROFILE_DROPIN" +refresh_profile_finish +check test -L "$owned" +check test "$(readlink "$owned")" = /dev/null +rm "$owned" +# Failure to unlink staging after publication must not prevent live-override cleanup. +rm() { + if [ "${2:-}" = "$REFRESH_PROFILE_STAGING" ]; then return 7; fi + command rm "$@" +} +if refresh_profile_start; then exit 1; fi +staged="$REFRESH_PROFILE_STAGING" +check test -n "$REFRESH_PROFILE_DROPIN" +if refresh_profile_finish >"$CASE_DIR/staging-unlink-failed"; then exit 1; fi +check test ! -e "$owned" +check test -e "$staged" +unset -f rm +refresh_profile_finish +check test ! -e "$staged" +check test -z "$REFRESH_PROFILE_STAGING" # A window without a completed profiled refresh cannot report successful diagnostics. refresh_profile_start journalctl() { :; } From 9dd42f5b5a08b8a5039df39c5b2983f02e204c0f Mon Sep 17 00:00:00 2001 From: "pithead-developer[bot]" Date: Wed, 30 Sep 2026 17:15:15 +0000 Subject: [PATCH 3/3] test: cover staging cleanup retry and allocation refusal --- tests/test-refresh-profile.sh | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/test-refresh-profile.sh b/tests/test-refresh-profile.sh index 1aadcef..a59cdae 100644 --- a/tests/test-refresh-profile.sh +++ b/tests/test-refresh-profile.sh @@ -97,7 +97,7 @@ printf 'existing override\n' >"$owned" if refresh_profile_start; then exit 1; fi refresh_profile_finish check test "$(cat "$owned")" = 'existing override' -rm "$owned" +command rm "$owned" # A failed partial write must remain owned until cleanup removes it. saved_umask=$(umask) umask 027 @@ -121,6 +121,11 @@ check test -z "$REFRESH_PROFILE_DROPIN" check test ! -e "$owned" staged="$REFRESH_PROFILE_STAGING" check test "$(cat "$staged")" = "$(builtin printf '[Service]\nExecStart=')" +rm() { return 7; } +if refresh_profile_finish; then exit 1; fi +check test "$REFRESH_PROFILE_STAGING" = "$staged" +check test -e "$staged" +unset -f rm refresh_profile_finish >"$CASE_DIR/partial-write-collected" check test ! -e "$owned" check test ! -e "$staged" @@ -140,7 +145,7 @@ check test "$(umask)" = "$saved_umask" check test -z "$REFRESH_PROFILE_DROPIN" refresh_profile_finish check test "$(cat "$owned")" = 'competing override' -rm "$owned" +command rm "$owned" # A raced symlink to a nonregular target must also remain untouched. mkdir() { command mkdir "$@" || return @@ -152,7 +157,15 @@ check test -z "$REFRESH_PROFILE_DROPIN" refresh_profile_finish check test -L "$owned" check test "$(readlink "$owned")" = /dev/null -rm "$owned" +command rm "$owned" +# Staging allocation failure must fail setup without claiming any override. +mktemp() { return 7; } +if refresh_profile_start; then exit 1; fi +unset -f mktemp +check test -z "$REFRESH_PROFILE_STAGING" +check test -z "$REFRESH_PROFILE_DROPIN" +refresh_profile_finish +check test ! -e "$owned" # Failure to unlink staging after publication must not prevent live-override cleanup. rm() { if [ "${2:-}" = "$REFRESH_PROFILE_STAGING" ]; then return 7; fi