fix: TOCTOU race in openbao secret-manager script

sources/openbao-config/0.1.0/templates/openbao-secret-manager-cm.yaml

@@ -6,38 +6,79 @@ metadata:
 data:
   manage-secrets.sh: |
     #!/bin/bash
-    set -e
-    
+    set -euo pipefail
+
     # Validate environment
-    if [ -z "$BAO_TOKEN" ]; then
+    if [ -z "${BAO_TOKEN:-}" ]; then
       echo "ERROR: BAO_TOKEN not set"
       exit 1
     fi
-
+    if [ -z "${BAO_ADDR:-}" ]; then
+      echo "ERROR: BAO_ADDR not set"
+      exit 1
+    fi
+
     echo "Managing OpenBao secrets from /tmp/secrets/secrets.env..."
 
     created=0
     updated=0
     skipped=0
+    failed=0
+
+    # Returns the HTTP status for a KV v2 path's metadata. A plain "bao kv get"
+    # exit code can't distinguish a genuine 404 (path never existed) from a
+    # sealed vault, 403, or standby 5xx -- all of those look like "missing",
+    # which is what let this job silently clobber a live random secret when
+    # it raced the openbao-unseal-job. Ask the API directly instead.
+    # Metadata (not data) because -cas=0 checks metadata.current_version.
+    kv_status() {
+      local rel="${1#secrets/}"
+      curl -s -o /dev/null -w '%{http_code}' \
+        --connect-timeout 5 --max-time 10 \
+        -H "X-Vault-Token: $BAO_TOKEN" \
+        "$BAO_ADDR/v1/secrets/metadata/$rel"
+    }
 
     while IFS='|' read -r path type value bytes; do
       [[ "$path" =~ ^#.*$ ]] || [[ -z "$path" ]] && continue
 
-      if bao kv get "$path" > /dev/null 2>&1; then
+      # curl prints 000 and exits non-zero when it can't connect at all; without
+      # the || the failed substitution would trip set -e and kill the run with
+      # no diagnostic whatsoever.
+      code=$(kv_status "$path") || code=000
+      case "$code" in
+        200) exists=true ;;
+        404) exists=false ;;
+        *)
+          echo "ERROR: cannot determine state of $path (HTTP $code)"
+          failed=$((failed + 1))
+          continue
+          ;;
+      esac
+
+      if [ "$exists" = "true" ]; then
         # Path already exists. random secrets are write-once so they are never
         # regenerated. static secrets are reconciled: if the desired value has
         # changed (e.g. a previously stored unrendered template literal), update
         # it so downstream ExternalSecrets converge on the correct value.
         if [ "$type" = "static" ]; then
-          final_value=$(echo "$value" | envsubst)
+          if ! final_value=$(echo "$value" | envsubst); then
+            echo "ERROR: envsubst failed for $path"
+            failed=$((failed + 1))
+            continue
+          fi
           current_value=$(bao kv get -field=value "$path" 2>/dev/null || echo "")
           if [ "$current_value" = "$final_value" ]; then
             echo "SKIP: $path (static, unchanged)"
             skipped=$((skipped + 1))
           else
             echo "UPDATE: $path (static, value changed)"
-            bao kv put "$path" value="$final_value" || { echo "ERROR: Failed to update $path"; exit 1; }
-            updated=$((updated + 1))
+            if bao kv put "$path" value="$final_value"; then
+              updated=$((updated + 1))
+            else
+              echo "ERROR: Failed to update $path"
+              failed=$((failed + 1))
+            fi
           fi
         else
           echo "SKIP: $path (already exists)"
@@ -47,29 +88,50 @@ data:
         echo "CREATE: $path ($type)"
 
         if [ "$type" = "static" ]; then
-          final_value=$(echo "$value" | envsubst)
-          bao kv put "$path" value="$final_value" || { echo "ERROR: Failed to create $path"; exit 1; }
+          if ! final_value=$(echo "$value" | envsubst); then
+            echo "ERROR: envsubst failed for $path"
+            failed=$((failed + 1))
+            continue
+          fi
+          if bao kv put "$path" value="$final_value"; then
+            created=$((created + 1))
+          else
+            echo "ERROR: Failed to create $path"
+            failed=$((failed + 1))
+          fi
         elif [ "$type" = "random" ]; then
-          random_value=$(bao write -field=random_bytes sys/tools/random bytes="$bytes" format=hex) || { echo "ERROR: Failed to generate random for $path"; exit 1; }
-          bao kv put "$path" value="$random_value" || { echo "ERROR: Failed to store $path"; exit 1; }
+          random_value=$(bao write -field=random_bytes sys/tools/random bytes="$bytes" format=hex) || {
+            echo "ERROR: Failed to generate random for $path"
+            failed=$((failed + 1))
+            continue
+          }
+          # -cas=0: write only if this path has no current version. OpenBao
+          # evaluates that atomically against its own state, so even if the
+          # kv_status read above was wrong (e.g. it raced a concurrent
+          # unseal), a secret that actually exists rejects the write instead
+          # of being silently clobbered.
+          if bao kv put -cas=0 "$path" value="$random_value"; then
+            created=$((created + 1))
+          else
+            echo "ERROR: Failed to store $path (check-and-set rejected: path already has a version. If it was soft-deleted, run 'bao kv undelete')"
+            failed=$((failed + 1))
+          fi
         else
           echo "ERROR: Unknown secret type: $type"
-          exit 1
+          failed=$((failed + 1))
         fi
-
-        created=$((created + 1))
       fi
     done < /tmp/secrets/secrets.env
 
-    echo "DONE: Created $created, Updated $updated, Skipped $skipped"
+    echo "DONE: Created $created, Updated $updated, Skipped $skipped, Failed $failed"
 
     # AIWB scoped API-key write credential (EAI-7277). Unlike cluster-auth (which reuses the
     # root token), AIWB gets a token scoped to secrets/apikeys/* so it cannot write elsewhere.
     # No renewal machinery: re-mint only when the stored token is absent or no longer valid.
     # Capabilities match the AIWB OpenBao client — data create/read/update, metadata
     # create/update (delete_version_after / native expiry) and delete (revocation).
     echo "Ensuring apikeys-write-policy..."
-    echo '
+    if ! echo '
     path "secrets/data/apikeys/*" {
       capabilities = ["create", "read", "update"]
     }
@@ -78,24 +140,72 @@ data:
     }
     path "auth/token/lookup-self" {
       capabilities = ["read"]
-    }' | bao policy write apikeys-write-policy -
-
-    need_mint=true
-    if stored=$(bao kv get -field=value secrets/aiwb-openbao-token 2>/dev/null); then
-      if BAO_TOKEN="$stored" bao token lookup >/dev/null 2>&1; then
-        need_mint=false
-        echo "SKIP: secrets/aiwb-openbao-token (scoped token still valid)"
-      fi
+    }' | bao policy write apikeys-write-policy -; then
+      echo "ERROR: Failed to write apikeys-write-policy"
+      failed=$((failed + 1))
     fi
+
+    # Same clobber shape as the main loop -- classify by HTTP status, not CLI exit code.
+    need_mint=false
+    old_token=""
+    token_status=$(kv_status secrets/aiwb-openbao-token) || token_status=000
+    case "$token_status" in
+      404)
+        need_mint=true
+        ;;
+      200)
+        if stored=$(bao kv get -field=value secrets/aiwb-openbao-token 2>/dev/null); then
+          if BAO_TOKEN="$stored" bao token lookup >/dev/null 2>&1; then
+            echo "SKIP: secrets/aiwb-openbao-token (scoped token still valid)"
+          else
+            need_mint=true
+            old_token="$stored"
+          fi
+        else
+          echo "ERROR: cannot read secrets/aiwb-openbao-token despite HTTP 200"
+          failed=$((failed + 1))
+        fi
+        ;;
+      *)
+        echo "ERROR: cannot determine state of secrets/aiwb-openbao-token (HTTP $token_status)"
+        failed=$((failed + 1))
+        ;;
+    esac
+
     if [ "$need_mint" = "true" ]; then
       echo "MINT: secrets/aiwb-openbao-token (scoped apikeys-write token)"
-      new_token=$(bao token create -policy=apikeys-write-policy -ttl=768h -orphan -field=token) || { echo "ERROR: Failed to mint aiwb-openbao-token"; exit 1; }
-      bao kv put secrets/aiwb-openbao-token value="$new_token" || { echo "ERROR: Failed to store aiwb-openbao-token"; exit 1; }
+      if new_token=$(bao token create -policy=apikeys-write-policy -ttl=768h -orphan -field=token); then
+        if [ -n "$old_token" ]; then
+          cas_version=$(bao kv get -format=json secrets/aiwb-openbao-token 2>/dev/null | jq -r '.data.metadata.version') || cas_version=""
+        else
+          cas_version=0
+        fi
+        if [ -n "$cas_version" ] && bao kv put -cas="$cas_version" secrets/aiwb-openbao-token value="$new_token"; then
+          [ -n "$old_token" ] && bao token revoke "$old_token" >/dev/null 2>&1
+        else
+          echo "ERROR: Failed to store aiwb-openbao-token (check-and-set rejected: path changed concurrently)"
+          failed=$((failed + 1))
+          # the freshly minted token was never recorded anywhere; don't orphan it
+          bao token revoke "$new_token" >/dev/null 2>&1
+        fi
+      else
+        echo "ERROR: Failed to mint aiwb-openbao-token"
+        failed=$((failed + 1))
+      fi
     fi
 
     # Special case: cluster-auth-openbao-token uses actual BAO_TOKEN (for init job only)
-    if [ "$INIT_MODE" = "true" ]; then
+    if [ "${INIT_MODE:-}" = "true" ]; then
       echo "CREATE: secrets/cluster-auth-openbao-token (special - init mode)"
-      bao kv put secrets/cluster-auth-openbao-token value="${BAO_TOKEN}" || { echo "ERROR: Failed to create cluster-auth-openbao-token"; exit 1; }
-      echo "Initial secret generation completed"
+      if bao kv put secrets/cluster-auth-openbao-token value="${BAO_TOKEN}"; then
+        echo "Initial secret generation completed"
+      else
+        echo "ERROR: Failed to create cluster-auth-openbao-token"
+        failed=$((failed + 1))
+      fi
+    fi
+
+    if [ "$failed" -gt 0 ]; then
+      echo "ERROR: $failed secret(s) failed to reconcile, see log above"
+      exit 1
     fi

sources/openbao-config/0.1.0/templates/openbao-secret-manager-cronjob.yaml

@@ -4,7 +4,7 @@ metadata:
   name: openbao-secret-manager
   namespace: cf-openbao
 spec:
-  schedule: "*/5 * * * *"  # Every 5 minutes
+  schedule: "2-59/5 * * * *"  # Every 5 minutes, offset from openbao-unseal-job so they don't co-run
   concurrencyPolicy: Forbid  # Don't run multiple jobs concurrently
   failedJobsHistoryLimit: 3
   successfulJobsHistoryLimit: 3

sources/openbao-init-job/0.1.0/templates/cf-init-openbao-job.yaml

@@ -4,6 +4,7 @@ metadata:
   name: openbao-init-job
   namespace: cf-openbao
 spec:
+  activeDeadlineSeconds: 1800
   template:
     spec:
       restartPolicy: Never