diff --git a/packages/flow/deploy/on-prem/bin/audit-log.sh b/packages/flow/deploy/on-prem/bin/audit-log.sh index 091add53c3..a5757becc0 100755 --- a/packages/flow/deploy/on-prem/bin/audit-log.sh +++ b/packages/flow/deploy/on-prem/bin/audit-log.sh @@ -68,21 +68,69 @@ chmod 0700 "$(dirname "$AUDIT_LOG")" 2>/dev/null || true # Serialize the read-last-hash + append transaction. O_APPEND prevents byte # interleaving, but without this lock two writers can both chain to the same # parent and produce a structurally broken audit log. +# +# Ownership is the exclusive creation of the pid file, done by the shell itself +# with noclobber (open(2) with O_CREAT|O_EXCL). The exit status of mkdir(1) is +# not trusted for exclusion: the uutils mkdir that Ubuntu 26.04 ships reports +# success to both callers when two processes race to create one directory. lock_dir="${AUDIT_LOG}.lock" +lock_file="$lock_dir/pid" +break_file="${AUDIT_LOG}.lock-breaker" lock_acquired=0 + +# Create $1 holding this process's pid; fails if $1 already exists as any +# kind of file. noclobber alone is not enough: bash opens an existing FIFO or +# device without O_EXCL, so two writers could both "create" the same FIFO. The +# pid goes into a fresh staged name and is hard-linked into place; link(2) +# refuses an existing path of every type. Ownership is then read back from the +# file itself, not from the exit status of ln(1). +create_exclusive() { + local target="$1" staged="" owner="" linked=1 + # Refuse any existing path up front, whatever its type. + [[ -e "$target" || -L "$target" ]] && return 1 + staged="$target.$$.$RANDOM$RANDOM" + ( set -C; printf '%s\n' "$$" > "$staged" ) 2>/dev/null || return 1 + [[ -f "$staged" && ! -L "$staged" ]] || return 1 + ln "$staged" "$target" 2>/dev/null && linked=0 + rm -f "$staged" 2>/dev/null || true + [[ "$linked" -eq 0 && -f "$target" && ! -L "$target" ]] || return 1 + owner="$(< "$target")" 2>/dev/null || return 1 + [[ "$owner" == "$$" ]] +} + +# Remove a lock whose owner has exited. Between reading the owner and removing +# the lock, that owner can release it and another writer can take it, so the +# removal happens only under a second exclusive lock that serializes breakers, +# and only if the lock still names the same dead owner. Without this, a breaker +# could delete a live writer's lock and let two writers chain to the same parent. +break_stale_lock() { + local stale="$1" current="" + create_exclusive "$break_file" || return 0 + current="$(cat "$lock_file" 2>/dev/null || true)" + if [[ "$current" == "$stale" ]] && ! kill -0 "$stale" 2>/dev/null; then + rm -f "$lock_file" 2>/dev/null || true + fi + rm -f "$break_file" 2>/dev/null || true +} + for ((attempt = 0; attempt < 200; attempt++)); do - if mkdir "$lock_dir" 2>/dev/null; then - printf '%s\n' "$$" > "$lock_dir/pid" - lock_acquired=1 - break + if [[ ! -L "$lock_dir" ]]; then + mkdir -p "$lock_dir" 2>/dev/null || true + if [[ -d "$lock_dir" && ! -L "$lock_dir" ]] && create_exclusive "$lock_file"; then + lock_acquired=1 + break + fi fi owner="" - if [[ -d "$lock_dir" && ! -L "$lock_dir" && -r "$lock_dir/pid" ]]; then - owner="$(cat "$lock_dir/pid" 2>/dev/null || true)" + if [[ -L "$lock_file" ]] || [[ -e "$lock_file" && ! -f "$lock_file" ]]; then + echo "audit-log.sh: audit lock $lock_file is not a regular file; refusing" >&2 + exit 1 + fi + if [[ -d "$lock_dir" && ! -L "$lock_dir" && -f "$lock_file" && ! -L "$lock_file" ]]; then + owner="$(cat "$lock_file" 2>/dev/null || true)" fi if [[ -n "$owner" && "$owner" =~ ^[0-9]+$ ]] && ! kill -0 "$owner" 2>/dev/null; then - rm -f "$lock_dir/pid" 2>/dev/null || true - rmdir "$lock_dir" 2>/dev/null || true + break_stale_lock "$owner" continue fi sleep 0.05 @@ -92,7 +140,7 @@ if [[ "$lock_acquired" -ne 1 ]]; then exit 1 fi release_audit_lock() { - rm -f "$lock_dir/pid" 2>/dev/null || true + rm -f "$lock_file" 2>/dev/null || true rmdir "$lock_dir" 2>/dev/null || true } trap release_audit_lock EXIT diff --git a/packages/flow/deploy/on-prem/bin/lib-release.sh b/packages/flow/deploy/on-prem/bin/lib-release.sh index 7676dc4900..671a2c6f4b 100755 --- a/packages/flow/deploy/on-prem/bin/lib-release.sh +++ b/packages/flow/deploy/on-prem/bin/lib-release.sh @@ -344,32 +344,62 @@ rel_validate_release_dir() { && -x "$release_dir/venv/bin/openadapt-flow" ]] } +# Create $1 holding this process's pid; fails if $1 already exists as any +# kind of file. Exclusion does not depend on mkdir(1): the uutils mkdir that +# Ubuntu 26.04 ships reports success to both callers when two processes race to +# create one directory. Nor does it rest on noclobber alone, which opens an +# existing FIFO or device without O_EXCL. The pid goes into a fresh staged name +# and is hard-linked into place; link(2) refuses an existing path of every +# type. Ownership is then read back from the file, not from ln(1)'s status. +_rel_create_exclusive() { + local target="$1" staged="" owner="" linked=1 + # Refuse any existing path up front, whatever its type. + [[ -e "$target" || -L "$target" ]] && return 1 + staged="$target.$$.$RANDOM$RANDOM" + ( set -C; printf '%s\n' "$$" > "$staged" ) 2>/dev/null || return 1 + [[ -f "$staged" && ! -L "$staged" ]] || return 1 + ln "$staged" "$target" 2>/dev/null && linked=0 + rm -f "$staged" 2>/dev/null || true + [[ "$linked" -eq 0 && -f "$target" && ! -L "$target" ]] || return 1 + owner="$(< "$target")" 2>/dev/null || return 1 + [[ "$owner" == "$$" ]] +} + _rel_lock_acquire() { - local root="$1" owner="" + local root="$1" owner="" current="" local lock="$root/releases/.update.lock" + local breaker="$root/releases/.update.lock-breaker" mkdir -p "$root/releases" || return 1 - if mkdir "$lock" 2>/dev/null; then - printf '%s\n' "$$" > "$lock/pid" || { - rmdir "$lock" 2>/dev/null || true - return 1 - } + if [[ -L "$lock" ]]; then + _rel_err "release lock is a symlink; refusing: $lock" + return 1 + fi + mkdir -p "$lock" 2>/dev/null || true + if [[ -d "$lock" && ! -L "$lock" ]] && _rel_create_exclusive "$lock/pid"; then REL_LOCK_DIR="$lock" return 0 fi - if [[ -d "$lock" && ! -L "$lock" && -r "$lock/pid" ]]; then + if [[ -L "$lock/pid" ]] || [[ -e "$lock/pid" && ! -f "$lock/pid" ]]; then + _rel_err "release lock pid is not a regular file; refusing: $lock/pid" + return 1 + fi + if [[ -d "$lock" && ! -L "$lock" && -f "$lock/pid" && ! -L "$lock/pid" ]]; then owner="$(cat "$lock/pid" 2>/dev/null || true)" fi if [[ "$owner" =~ ^[0-9]+$ ]] && kill -0 "$owner" 2>/dev/null; then _rel_err "another update or rollback is active (pid $owner)" return 1 fi - if [[ -d "$lock" && ! -L "$lock" ]]; then - rm -f "$lock/pid" 2>/dev/null || true - if rmdir "$lock" 2>/dev/null && mkdir "$lock" 2>/dev/null; then - printf '%s\n' "$$" > "$lock/pid" || { - rmdir "$lock" 2>/dev/null || true - return 1 - } + # The owner has exited (or left an unreadable pid). Remove its lock only + # under the breaker lock and only if it still names that owner, so a + # concurrent breaker cannot remove a lock another process has just taken. + if [[ -n "$owner" ]] && _rel_create_exclusive "$breaker"; then + current="$(cat "$lock/pid" 2>/dev/null || true)" + if [[ "$current" == "$owner" ]]; then + rm -f "$lock/pid" 2>/dev/null || true + fi + rm -f "$breaker" 2>/dev/null || true + if [[ -d "$lock" && ! -L "$lock" ]] && _rel_create_exclusive "$lock/pid"; then REL_LOCK_DIR="$lock" return 0 fi diff --git a/packages/flow/deploy/on-prem/bin/test-update.sh b/packages/flow/deploy/on-prem/bin/test-update.sh index c2778b4602..e181e446d5 100755 --- a/packages/flow/deploy/on-prem/bin/test-update.sh +++ b/packages/flow/deploy/on-prem/bin/test-update.sh @@ -244,6 +244,43 @@ assert "concurrent update lock is refused" test "$?" -ne 0 rm -f "$ROOT/releases/.update.lock/pid" rmdir "$ROOT/releases/.update.lock" +# Ubuntu 26.04 ships uutils mkdir, which reports success to both callers when +# two processes race to create one directory. Model that with a mkdir shim that +# never reports EEXIST: the locks must still exclude, so they cannot rest on +# the exit status of mkdir(1). +EEXIST_SHIM="$WORK/eexist-shim" +mkdir -p "$EEXIST_SHIM" +REAL_MKDIR="$(command -v mkdir)" +cat > "$EEXIST_SHIM/mkdir" </dev/null || true +exit 0 +SHIM_EOF +chmod +x "$EEXIST_SHIM/mkdir" +mkdir "$ROOT/releases/.update.lock" +printf '%s\n' "$$" > "$ROOT/releases/.update.lock/pid" +PATH="$EEXIST_SHIM:$PATH" apply_release "$ROOT" "$ARC6" >/dev/null 2>&1 +assert "update lock holds when mkdir(1) never reports EEXIST" test "$?" -ne 0 +assert_eq "a refused update leaves the live owner's lock" "$$" \ + "$(cat "$ROOT/releases/.update.lock/pid" 2>/dev/null || echo none)" +rm -f "$ROOT/releases/.update.lock/pid" +rmdir "$ROOT/releases/.update.lock" + +# A pid path that already exists as a FIFO with a reader: bash's noclobber +# opens an existing FIFO without O_EXCL, so the write "succeeds". The lock must +# refuse any existing path, whatever its type, and leave it alone. +mkdir "$ROOT/releases/.update.lock" +mkfifo "$ROOT/releases/.update.lock/pid" +sleep 30 <> "$ROOT/releases/.update.lock/pid" & +fifo_reader=$! +apply_release "$ROOT" "$ARC6" >/dev/null 2>&1 +assert "update lock refuses an existing FIFO pid path" test "$?" -ne 0 +assert "a refused update leaves the FIFO in place" test -p "$ROOT/releases/.update.lock/pid" +kill "$fifo_reader" 2>/dev/null || true +wait "$fifo_reader" 2>/dev/null || true +rm -f "$ROOT/releases/.update.lock/pid" +rmdir "$ROOT/releases/.update.lock" + chmod -x "$ROOT/releases/2.0.0/venv/bin/openadapt-flow" rel_do_rollback "$ROOT" "" >/dev/null 2>&1 assert "incomplete rollback target is refused" test "$?" -ne 0 @@ -290,7 +327,77 @@ if grep -q 'audit chain intact' <<< "$audit_output"; then pass "concurrent audit writers preserve one hash chain" else fail "concurrent audit writers preserve one hash chain" + printf '%s\n' "$audit_output" | sed 's/^/ /' +fi + +SHIM_LOG="$WORK/eexist-audit/audit.log" +for index in 1 2 3 4 5 6 7 8 9 10 11 12; do + OPENADAPT_ONPREM_AUDIT_LOG="$SHIM_LOG" PATH="$EEXIST_SHIM:$PATH" \ + bash "$OPENADAPT_ONPREM_AUDIT_BIN" started "shim-$index" --note test >/dev/null & +done +wait +audit_output="$(OPENADAPT_ONPREM_AUDIT_LOG="$SHIM_LOG" \ + bash "$HERE/verify-airgap.sh" --config "$WORK/missing.yaml" --audit 2>&1 || true)" +if grep -q 'audit chain intact across 12 record' <<< "$audit_output"; then + pass "audit writers stay serialized when mkdir(1) never reports EEXIST" +else + fail "audit writers stay serialized when mkdir(1) never reports EEXIST" + printf '%s\n' "$audit_output" | sed 's/^/ /' +fi + +FIFO_LOG="$WORK/fifo-audit/audit.log" +mkdir -p "$FIFO_LOG.lock" +mkfifo "$FIFO_LOG.lock/pid" +sleep 30 <> "$FIFO_LOG.lock/pid" & +fifo_reader=$! +OPENADAPT_ONPREM_AUDIT_LOG="$FIFO_LOG" \ + bash "$OPENADAPT_ONPREM_AUDIT_BIN" started fifo-lock --note test >/dev/null 2>&1 +assert "audit writer refuses an existing FIFO lock file" test "$?" -ne 0 +assert "a refused audit writer appends nothing" test ! -s "$FIFO_LOG" +assert "a refused audit writer leaves the FIFO in place" test -p "$FIFO_LOG.lock/pid" +kill "$fifo_reader" 2>/dev/null || true +wait "$fifo_reader" 2>/dev/null || true + +# A writer that finds a lock owned by an exited process may break it. Force the +# race where, between that writer reading the dead owner and breaking the +# lock, the owner releases and a live writer takes the lock: a shimmed cat +# returns the dead owner once, then hands the lock to a live process. The +# breaker must leave the live writer's lock alone. +RACE="$WORK/stale-race" +mkdir -p "$RACE/shim" +RACE_LOG="$RACE/audit.log" +bash -c 'exit 0' & +dead_pid=$! +wait "$dead_pid" +sleep 30 & +live_pid=$! +mkdir "$RACE_LOG.lock" +printf '%s\n' "$dead_pid" > "$RACE_LOG.lock/pid" +REAL_CAT="$(command -v cat)" +cat > "$RACE/shim/cat" < "$RACE/handed-over" + printf '%s\n' "$live_pid" > "$RACE_LOG.lock/pid" + exit 0 fi +exec "$REAL_CAT" "\$@" +SHIM_EOF +chmod +x "$RACE/shim/cat" +OPENADAPT_ONPREM_AUDIT_LOG="$RACE_LOG" PATH="$RACE/shim:$PATH" \ + bash "$OPENADAPT_ONPREM_AUDIT_BIN" started stale-race --note test >/dev/null 2>&1 & +race_writer=$! +sleep 2 +race_owner="$(cat "$RACE_LOG.lock/pid" 2>/dev/null || echo none)" +assert_eq "stale-lock breaker leaves a live writer's lock alone" "$live_pid" "$race_owner" +rm -f "$RACE_LOG.lock/pid" +rmdir "$RACE_LOG.lock" 2>/dev/null || true +kill "$live_pid" 2>/dev/null || true +wait "$live_pid" 2>/dev/null || true +wait "$race_writer" +assert_eq "writer appends once the live lock is released" 0 "$?" +assert_eq "writer appends exactly one record" 1 "$(wc -l < "$RACE_LOG" | tr -d ' ')" if ! grep -q 'identity evidence' "$OPENADAPT_ONPREM_AUDIT_LOG"; then pass "release audit remains PHI-free" else