Post-#149 review pass 4: 1 medium + 2 low findings (asymmetric migration, script hygiene)

fourth-pass review on the broader stack at HEAD e12ddab, dedup'd against everything filed today (#148 (closed) closed, #149 (closed) in flight with finding 2 added today as note, #150 (closed) MR !4 (closed) pending merge). three real findings, all hand-verified.

a fourth finding (second instance of #149 (closed) finding 4 LOWER(...) index defeat on timecurve_warbow_steals_by_victim_day) was flagged inline on #149 (closed) since the fix is identical to that ticket's existing patch.

ordered by severity descending. finding 1 is a deploy / rollback hygiene gap worth pre-mainnet attention; findings 2 and 3 are post-deploy hardening.

Finding 1 — Indexer: 20260504180000_timecurve_unredeemed_launched_token migration ships .up.sql with no .down.sql (Medium)

indexer/migrations/ listing:

ls indexer/migrations/ | grep 20260504180000
20260504180000_timecurve_unredeemed_launched_token.up.sql

every other migration in the directory ships a paired .down.sql (verified spot-checks). this one shipped .up only.

concrete consequence: rollback of this schema step (the unredeemed-launched-token table introduced for the #128 (closed) sweep flow) cannot run via the migration runner. an operator forced to revert post-deploy must drop the table by hand, and the migrator's _sqlx_migrations version table will not be reconciled — blocking automated re-application of the same version. on a TGE-bound deploy with an active rollback plan this is a failure mode worth closing pre-mainnet.

mitigation: add 20260504180000_timecurve_unredeemed_launched_token.down.sql containing the inverse DROP TABLE IF EXISTS … (and any related index drops) so the pair is symmetric with the rest of the migration set.

Finding 2 — Script: start-qa-local-full-stack.sh background Vite has no trap; orphans on orchestrator death (Low)

scripts/start-qa-local-full-stack.sh:115-128:

echo "=== Vite dev (127.0.0.1:${port}) ===" >&2
(
  cd "${FRONTEND}"
  npm run dev -- --host 127.0.0.1 --port "${port}"
) >>"${log_file}" 2>&1 &
echo $! >"${pid_file}"
echo "  Log: ${log_file}   PID: $(cat "${pid_file}")" >&2
for _ in $(seq 1 60); do
  if curl -sf "http://127.0.0.1:${port}/" >/dev/null 2>&1; then
    break
  fi
  sleep 0.5
done

backgrounds npm run dev, writes PID — but no trap … EXIT INT TERM upstream to kill it on script abort.

concrete failure mode: if the orchestrator dies or the user Ctrl-Cs after the readiness loop has begun, the Vite child keeps holding ${FRONTEND_DEV_PORT:-5173}. next start-qa-local-full-stack.sh invocation either:

  • fails to bind 5173 (clean failure, easy to diagnose)
  • silently passes the readiness curl probe at L122-127 against the orphan from the previous run (silent failure — Vite serves stale code from the previous run while the operator thinks they're testing fresh)

I've actually hit the silent-orphan version of this today during the #150 (closed) stack bring-up — anvil orphan held 8545 from a previous failed run and the second attempt's readiness probe falsely passed against it. same shape applies to Vite here.

mitigation: add a trap inside the launching function before the background invocation:

trap 'kill "$(cat "${pid_file}" 2>/dev/null)" 2>/dev/null || true' EXIT INT TERM

or have the readiness loop fail-closed if curl ever returned 0 BEFORE the loop's first sleep (catches stale-orphan masquerading as fresh-ready).

Finding 3 — Library: _yieldomega_env_set_line sed escaping is fragile (Low — preventive)

scripts/lib/kumbaya_local_anvil_env.sh:37-52:

# Idempotent: set or replace KEY=value (hex addresses — avoid special chars in values).
_yieldomega_env_set_line() {
  local file="$1" key="$2" val="$3"
  ...
  if grep -q "^${key}=" "${file}" 2>/dev/null; then
    sed -i "s#^${key}=.*#${key}=${val}#" "${file}"

${key} is interpolated unescaped into the regex side and ${val} is interpolated unescaped into the replacement side of the sed command. header at L37 acknowledges "hex addresses — avoid special chars in values".

today's callers (VITE_KUMBAYA_* keys, 0x-prefixed addresses) are safe. but the helper is reused via:

  • yieldomega_frontend_merge_kumbaya_vite_full
  • yieldomega_frontend_merge_vite_kumbaya_buy_router_only

the moment any future caller passes a value containing &, \, or # (path separators, signed payloads, base64 with / in it, etc.), the replacement becomes silently wrong (& re-injects the matched LHS, # ends the s-expression early, \ injects backreferences). silent corruption rather than loud failure.

mitigation: pre-escape both sides:

local k_esc v_esc
k_esc="$(printf '%s' "${key}" | sed -e 's/[][\.*^$\/]/\\&/g')"
v_esc="$(printf '%s' "${val}" | sed -e 's/[\&\/]/\\&/g')"
sed -i "s#^${k_esc}=.*#${k_esc}=${v_esc}#" "${file}"

or switch to a different in-place editor (awk, python, dedicated env-file lib) that doesn't treat values as regex. preventive — no exploit today, but the moment someone adds a key with a non-trivial value the breakage will be silent.

Pre-deploy relevance

  • deploy-tonight scope: finding 1 (rollback safety pre-mainnet).
  • post-deploy hardening: findings 2, 3.

cc @PlasticDigits