fix: harden mac release service account reads

skills/release-mac-app/scripts/lib/mac_release.sh

@@ -104,6 +104,7 @@ mac_release_load_1password_env() {
 
   require_bin tmux op node
   local account vault socket_dir socket session op_window work_dir script runner env_file log_file status_file
+  local service_account_token_file needs_service_account=0
   account=${MAC_RELEASE_OP_ACCOUNT:-my.1password.com}
   vault=${MAC_RELEASE_OP_VAULT:-}
   socket_dir=${CLAWDBOT_TMUX_SOCKET_DIR:-${TMPDIR:-/tmp}/clawdbot-tmux-sockets}
@@ -140,10 +141,30 @@ mac_release_load_1password_env() {
   trap 'cleanup_1password_env; exit 130' INT
   trap 'cleanup_1password_env; exit 143' TERM
 
+  if [[ "$primary_missing" == "1" && "${MAC_RELEASE_OP_USE_SERVICE_ACCOUNT:-0}" == "1" ]]; then
+    needs_service_account=1
+  fi
+  if [[ "$codesign_missing" == "1" && "${MAC_RELEASE_CODESIGN_OP_USE_SERVICE_ACCOUNT-${MAC_RELEASE_OP_USE_SERVICE_ACCOUNT:-0}}" == "1" ]]; then
+    needs_service_account=1
+  fi
+  if [[ "$env_refs_missing" == "1" && "${MAC_RELEASE_OP_USE_SERVICE_ACCOUNT:-0}" == "1" ]]; then
+    needs_service_account=1
+  fi
+  service_account_token_file=
+  if [[ "$needs_service_account" == "1" ]]; then
+    [[ -n "${OP_SERVICE_ACCOUNT_TOKEN:-}" ]] || {
+      cleanup_1password_env
+      restore_1password_traps
+      mac_release_die "OP_SERVICE_ACCOUNT_TOKEN is required for 1Password service-account reads"
+    }
+    service_account_token_file="$work_dir/service-account-token"
+    (umask 077; printf '%s' "$OP_SERVICE_ACCOUNT_TOKEN" >"$service_account_token_file")
+  fi
+
   cat >"$script" <<'SCRIPT'
 #!/usr/bin/env bash
 set -euo pipefail
-set +x
+set +vx
 
 item=${MAC_RELEASE_OP_ITEM:-}
 account=${MAC_RELEASE_OP_ACCOUNT:-my.1password.com}
@@ -163,18 +184,31 @@ work_dir=$(mktemp -d /tmp/mac-release-op-json.XXXXXX)
 trap 'rm -rf "$work_dir"' EXIT
 : >"$env_file"
 
+run_op() {
+  local use_service_account=$1 target_account=$2
+  shift 2
+  if [[ "$use_service_account" == "1" ]]; then
+    OP_LOAD_DESKTOP_APP_SETTINGS=false \
+      OP_BIOMETRIC_UNLOCK_ENABLED=false \
+      OP_SERVICE_ACCOUNT_TOKEN="${OP_SERVICE_ACCOUNT_TOKEN:?}" \
+      op "$@" </dev/null
+  else
+    env -u OP_SERVICE_ACCOUNT_TOKEN \
+      -u MOLTY_OP_SERVICE_ACCOUNT_TOKEN \
+      -u OP_LOAD_DESKTOP_APP_SETTINGS \
+      -u OP_BIOMETRIC_UNLOCK_ENABLED \
+      op "$@" --account "$target_account"
+  fi
+}
+
 read_item() {
   local target_item=$1 target_account=$2 target_vault=$3 use_service_account=$4 output=$5
-  local args=(item get "$target_item" --account "$target_account" --format json)
+  local args=(item get "$target_item" --format json)
   if [[ -n "$target_vault" ]]; then
     args+=(--vault "$target_vault")
   fi
 
-  if [[ -n "$target_vault" || "$use_service_account" == "1" ]]; then
-    op "${args[@]}" >"$output" 2>>"$log_file"
-  else
-    env -u OP_SERVICE_ACCOUNT_TOKEN -u MOLTY_OP_SERVICE_ACCOUNT_TOKEN op "${args[@]}" >"$output" 2>>"$log_file"
-  fi
+  run_op "$use_service_account" "$target_account" "${args[@]}" >"$output" 2>>"$log_file"
 }
 
 if [[ "$read_primary" == "1" ]]; then
@@ -236,16 +270,16 @@ NODE
 fi
 
 if [[ "${MAC_RELEASE_OP_ENV_REFS_READ:-0}" == "1" && -n "${MAC_RELEASE_OP_ENV_REFS:-}" ]]; then
-  while IFS= read -r env_ref_entry; do
+  while IFS= read -r env_ref_entry <&3; do
     [[ -n "${env_ref_entry// /}" ]] || continue
     env_ref_name=${env_ref_entry%%=*}
     env_ref_uri=${env_ref_entry#*=}
-    env_ref_value=$(op read "$env_ref_uri" 2>>"$log_file") ||
+    env_ref_value=$(run_op "${MAC_RELEASE_OP_USE_SERVICE_ACCOUNT:-0}" "$account" read "$env_ref_uri" 2>>"$log_file") ||
       { echo "op read failed for $env_ref_name" >&2; exit 1; }
     [[ -n "$env_ref_value" ]] || { echo "empty 1Password value for $env_ref_name" >&2; exit 1; }
     printf "export %s='%s'\n" "$env_ref_name" "${env_ref_value//\'/\'\\\'\'}" >>"$env_file"
     echo "$env_ref_name: len=${#env_ref_value}" >&2
-  done < <(tr ';' '\n' <<<"${MAC_RELEASE_OP_ENV_REFS}")
+  done 3< <(tr ';' '\n' <<<"${MAC_RELEASE_OP_ENV_REFS}")
 fi
 
 chmod 600 "$env_file"
@@ -255,6 +289,7 @@ SCRIPT
 
   {
     printf '#!/usr/bin/env bash\n'
+    printf 'set +vx\n'
     printf 'set -euo pipefail\n'
     printf 'export PATH=%q\n' "$PATH"
     printf 'export MAC_RELEASE_OP_ITEM=%q\n' "${MAC_RELEASE_OP_ITEM:-}"
@@ -275,6 +310,15 @@ SCRIPT
     printf 'export MAC_RELEASE_OP_ENV_REFS_READ=%q\n' "$env_refs_missing"
     printf 'export MAC_RELEASE_OP_ENV_FILE=%q\n' "$env_file"
     printf 'export MAC_RELEASE_OP_LOG_FILE=%q\n' "$log_file"
+    printf 'export MAC_RELEASE_OP_SERVICE_ACCOUNT_TOKEN_FILE=%q\n' "$service_account_token_file"
+    cat <<'RUNNER'
+if [[ -n "$MAC_RELEASE_OP_SERVICE_ACCOUNT_TOKEN_FILE" ]]; then
+  OP_SERVICE_ACCOUNT_TOKEN=$(<"$MAC_RELEASE_OP_SERVICE_ACCOUNT_TOKEN_FILE")
+  rm -f "$MAC_RELEASE_OP_SERVICE_ACCOUNT_TOKEN_FILE"
+  [[ -n "$OP_SERVICE_ACCOUNT_TOKEN" ]] || { echo "empty 1Password service-account token" >&2; exit 1; }
+  export OP_SERVICE_ACCOUNT_TOKEN
+fi
+RUNNER
     printf 'bash %q\n' "$script"
   } >"$runner"
   chmod 700 "$runner"
@@ -285,7 +329,7 @@ SCRIPT
 
   : >"$log_file"
   tmux -S "$socket" send-keys -t "$op_window" -- \
-    "bash $(mac_release_tmux_quote "$runner"); printf '%s\n' \$? > $(mac_release_tmux_quote "$status_file")" C-m
+    "env -u BASH_ENV bash $(mac_release_tmux_quote "$runner"); printf '%s\n' \$? > $(mac_release_tmux_quote "$status_file")" C-m
 
   local deadline=$((SECONDS + ${MAC_RELEASE_OP_WAIT_SECONDS:-300}))
   until [[ -f "$status_file" ]]; do

skills/release-mac-app/scripts/mac-release.test.sh

@@ -0,0 +1,198 @@
+#!/usr/bin/env bash
+# shellcheck disable=SC1091,SC2030,SC2031
+set -euo pipefail
+
+script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+test_root="$(mktemp -d /tmp/mac-release-test.XXXXXX)"
+trap 'rm -rf "$test_root"' EXIT
+mkdir -p "$test_root/bin"
+printf '%s\n' 'pane-input' 'pane-input' >"$test_root/pane-input"
+
+cat >"$test_root/bin/tmux" <<'TMUX'
+#!/usr/bin/env bash
+set -euo pipefail
+printf '%q ' "$@" >>"${MAC_RELEASE_TEST_ROOT:?}/tmux.log"
+printf '\n' >>"$MAC_RELEASE_TEST_ROOT/tmux.log"
+
+case " $* " in
+  *" has-session "*) exit 0 ;;
+  *" new-session "*)
+    echo "unexpected new tmux session" >&2
+    exit 1
+    ;;
+  *" new-window "*) printf '@7\n' ;;
+  *" send-keys "*)
+    command_text=
+    previous=
+    for arg in "$@"; do
+      if [[ "$previous" == "--" ]]; then
+        command_text=$arg
+        break
+      fi
+      previous=$arg
+    done
+    [[ -n "$command_text" ]]
+    runner_path=${command_text#* bash }
+    runner_path=${runner_path%%;*}
+    [[ -f "$runner_path" ]]
+    work_dir=${runner_path%/*}
+    if grep -Fq "${MAC_RELEASE_TEST_TOKEN:?}" "$command_text" "$runner_path" "$work_dir/read-op.sh"; then
+      echo "service-account token appeared in tmux command or generated script" >&2
+      exit 1
+    fi
+    if [[ "$MAC_RELEASE_TEST_MODE" == "service" ]]; then
+      token_file="$work_dir/service-account-token"
+      [[ -f "$token_file" ]]
+      token_mode=$(stat -f '%Lp' "$token_file" 2>/dev/null || stat -c '%a' "$token_file")
+      [[ "$token_mode" == "600" ]]
+      [[ "$(<"$token_file")" == "$MAC_RELEASE_TEST_TOKEN" ]]
+    else
+      [[ ! -e "$work_dir/service-account-token" ]]
+    fi
+    env -u OP_SERVICE_ACCOUNT_TOKEN -u MOLTY_OP_SERVICE_ACCOUNT_TOKEN \
+      bash --noprofile --norc -c "$command_text" <"$MAC_RELEASE_TEST_ROOT/pane-input"
+    [[ ! -e "$work_dir/service-account-token" ]]
+    ;;
+  *" kill-window "*) ;;
+  *)
+    echo "unexpected tmux call" >&2
+    exit 1
+    ;;
+esac
+TMUX
+chmod +x "$test_root/bin/tmux"
+
+cat >"$test_root/bin/op" <<'OP'
+#!/usr/bin/env bash
+set -euo pipefail
+
+has_account=0
+for arg in "$@"; do
+  [[ "$arg" != "--account" ]] || has_account=1
+done
+
+case "${MAC_RELEASE_TEST_MODE:?}" in
+  service)
+    [[ "${OP_LOAD_DESKTOP_APP_SETTINGS:-}" == "false" ]]
+    [[ "${OP_BIOMETRIC_UNLOCK_ENABLED:-}" == "false" ]]
+    [[ "${OP_SERVICE_ACCOUNT_TOKEN:-}" == "${MAC_RELEASE_TEST_TOKEN:?}" ]]
+    [[ "$has_account" == "0" ]]
+    if IFS= read -r pane_input; then
+      echo "service-account op command read from the pane: $pane_input" >&2
+      exit 1
+    fi
+    ;;
+  interactive)
+    [[ -z "${OP_SERVICE_ACCOUNT_TOKEN:-}" ]]
+    [[ -z "${MOLTY_OP_SERVICE_ACCOUNT_TOKEN:-}" ]]
+    [[ -z "${OP_LOAD_DESKTOP_APP_SETTINGS:-}" ]]
+    [[ -z "${OP_BIOMETRIC_UNLOCK_ENABLED:-}" ]]
+    [[ "$has_account" == "1" ]]
+    [[ " $* " == *" --account test.1password.example "* ]]
+    IFS= read -r pane_input
+    [[ "$pane_input" == "pane-input" ]]
+    ;;
+  *) exit 1 ;;
+esac
+
+printf 'mode=%s load=%s biometric=%s account=%s args=' \
+  "$MAC_RELEASE_TEST_MODE" \
+  "${OP_LOAD_DESKTOP_APP_SETTINGS:-unset}" \
+  "${OP_BIOMETRIC_UNLOCK_ENABLED:-unset}" \
+  "$has_account" >>"${MAC_RELEASE_TEST_ROOT:?}/op.log"
+printf '%q ' "$@" >>"$MAC_RELEASE_TEST_ROOT/op.log"
+printf '\n' >>"$MAC_RELEASE_TEST_ROOT/op.log"
+
+if [[ "$1 $2" == "item get" ]]; then
+  printf '%s\n' '{"fields":[{"label":"TEST_SECRET","value":"loaded-value"},{"label":"keychain_path","value":"/tmp/release.keychain-db"},{"label":"keychain_password","value":"password-value"}]}'
+elif [[ "$1" == "read" ]]; then
+  printf '%s\n' 'ref-value'
+else
+  echo "unexpected op call" >&2
+  exit 1
+fi
+OP
+chmod +x "$test_root/bin/op"
+
+# shellcheck source=lib/mac_release.sh
+source "$script_dir/lib/mac_release.sh"
+
+service_token='service-token-that-must-never-appear'
+service_output="$test_root/service.output"
+(
+  trap - EXIT
+  export PATH="$test_root/bin:$PATH"
+  export MAC_RELEASE_TEST_ROOT="$test_root"
+  export MAC_RELEASE_TEST_MODE=service
+  export MAC_RELEASE_TEST_TOKEN="$service_token"
+  export OP_SERVICE_ACCOUNT_TOKEN="$service_token"
+  export MAC_RELEASE_OP_ITEM='Release credentials'
+  export MAC_RELEASE_OP_FIELDS=TEST_SECRET
+  export MAC_RELEASE_OP_ACCOUNT=test.1password.example
+  export MAC_RELEASE_OP_USE_SERVICE_ACCOUNT=1
+  export MAC_RELEASE_OP_VAULT=Molty
+  export MAC_RELEASE_OP_ENV_REFS='EXTRA_SECRET=op://Molty/Release credentials/extra'
+  export MAC_RELEASE_CODESIGN_OP_ITEM='Signing keychain'
+  unset MAC_RELEASE_CODESIGN_OP_VAULT
+
+  mac_release_load_1password_env
+  [[ "$TEST_SECRET" == "loaded-value" ]]
+  [[ "$EXTRA_SECRET" == "ref-value" ]]
+  [[ "$MAC_RELEASE_CODESIGN_KEYCHAIN" == "/tmp/release.keychain-db" ]]
+  [[ "$MAC_RELEASE_CODESIGN_KEYCHAIN_PASSWORD" == "password-value" ]]
+) >"$service_output" 2>&1
+
+grep -Fq 'has-session' "$test_root/tmux.log"
+[[ "$(grep -c 'new-window' "$test_root/tmux.log")" == "1" ]]
+if grep -Fq 'new-session' "$test_root/tmux.log"; then
+  echo "pre-existing op-work session was replaced" >&2
+  exit 1
+fi
+[[ "$(grep -c '^mode=service ' "$test_root/op.log")" == "3" ]]
+if grep -Fq -- '--account' "$test_root/op.log"; then
+  echo "service-account op command included --account" >&2
+  exit 1
+fi
+grep -Fq 'load=false biometric=false account=0' "$test_root/op.log"
+[[ "$(grep -c -- '--vault Molty' "$test_root/op.log")" == "2" ]]
+if grep -Fq "$service_token" "$service_output" "$test_root/tmux.log" "$test_root/op.log"; then
+  echo "service-account token was disclosed" >&2
+  exit 1
+fi
+
+: >"$test_root/tmux.log"
+: >"$test_root/op.log"
+interactive_output="$test_root/interactive.output"
+(
+  trap - EXIT
+  export PATH="$test_root/bin:$PATH"
+  export MAC_RELEASE_TEST_ROOT="$test_root"
+  export MAC_RELEASE_TEST_MODE=interactive
+  export MAC_RELEASE_TEST_TOKEN="$service_token"
+  unset OP_SERVICE_ACCOUNT_TOKEN
+  export MOLTY_OP_SERVICE_ACCOUNT_TOKEN='legacy-token-that-must-not-be-used'
+  export OP_LOAD_DESKTOP_APP_SETTINGS=false
+  export OP_BIOMETRIC_UNLOCK_ENABLED=false
+  export MAC_RELEASE_OP_ITEM='Personal release credentials'
+  export MAC_RELEASE_OP_FIELDS=TEST_SECRET
+  export MAC_RELEASE_OP_ACCOUNT=test.1password.example
+  export MAC_RELEASE_OP_USE_SERVICE_ACCOUNT=0
+  export MAC_RELEASE_OP_VAULT=Private
+  export MAC_RELEASE_OP_ENV_REFS='EXTRA_SECRET=op://Private/Personal release credentials/extra'
+  unset MAC_RELEASE_CODESIGN_OP_ITEM
+
+  mac_release_load_1password_env
+  [[ "$TEST_SECRET" == "loaded-value" ]]
+  [[ "$EXTRA_SECRET" == "ref-value" ]]
+) >"$interactive_output" 2>&1
+
+[[ "$(grep -c '^mode=interactive ' "$test_root/op.log")" == "2" ]]
+grep -Fq 'mode=interactive load=unset biometric=unset account=1' "$test_root/op.log"
+grep -Fq -- '--account test.1password.example' "$test_root/op.log"
+grep -Fq -- '--vault Private' "$test_root/op.log"
+if grep -Fq "$service_token" "$interactive_output" "$test_root/tmux.log" "$test_root/op.log"; then
+  echo "ambient service-account token was disclosed by desktop flow" >&2
+  exit 1
+fi
+
+echo "mac release 1Password tests passed"