on-prem locks: hold on Ubuntu 26.04 (uutils mkdir) and never break a live writer's lock

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

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

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" <<SHIM_EOF
+#!/usr/bin/env bash
+"$REAL_MKDIR" "\$@" 2>/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" <<SHIM_EOF
+#!/usr/bin/env bash
+if [[ "\${1:-}" == "$RACE_LOG.lock/pid" && ! -e "$RACE/handed-over" ]]; then
+  "$REAL_CAT" "\$1"
+  : > "$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