gabriel / musehub public
Closed #194 Bug
filed by gabriel human · 34 days ago

deploy.sh hardcodes Nonproduction ECR registry — prod deploy authenticates to the wrong account

0 Anchors
— Blast radius
— Churn 30d
0 Proposals

Summary

Prod deploy from a clean checkout is still broken: deploy.sh:36 hardcodes the Nonproduction ECR registry, and :202-204 logs into it, while :47 pulls whatever image push.sh passes in — so a "prod" deploy authenticates to the Nonproduction registry while trying to pull an image that was pushed to (and only exists in) Production's registry.

Why #159 didn't catch this

#159 closed with "deploy.sh needed no changes — it already takes ECR_IMAGE as a parameter." That's true of ECR_IMAGE, but not of ECR_REGISTRY — and ECR_REGISTRY is what docker login actually uses. The gap survived because nothing had cause to open deploy.sh itself during that work:

  • #150 fixed push.sh's per-environment auth (separate script).
  • #159 concluded deploy.sh was already parameterized (true only for the image, not the registry).
  • 07-secrets-and-config.md notes deploy.sh/secrets.sh are deliberately decoupled — reasonable in general, but it meant no one re-checked deploy.sh's own registry logic when push.sh went per-environment.

Fix

aaronrene has this branched off dev @ 8a1389ae — 2 files, 26 tests. Currently queued behind #193 (the accepted_at push-blocker), since shipping it requires a push.

  • #159 — closed without catching this; not reopening (the closure was correct for what it covered), but this ticket documents the gap it missed.

Source

Reported by aaronrene, 2026-09-10 (relayed via message to gabriel, processed 2026-09-11).

◎ Activity1
●
gabriel opened this issue 34 days ago
○
gabriel 34 days ago

Fixed and verified ✅

Root cause confirmed exactly as reported: ECR_REGISTRY was hardcoded to the Nonproduction account and used independently of ECR_IMAGE (which push.sh already parameterizes correctly per-environment). Fix: derive ECR_REGISTRY from ECR_IMAGE via ${ECR_IMAGE%%/*} instead of hardcoding a second account ID that has to be kept in sync by hand — makes the two variables structurally incapable of drifting apart again.

Note: aaronrene's original fix was branched locally (dev @ 8a1389ae, 2 files, 26 tests) but was never pushed (blocked by #193) and wasn't fetchable from any shared remote, so this is an independent reimplementation from the ticket's description rather than his original diff — the end result (derive from ECR_IMAGE) may or may not match his approach exactly.

TDD: tests/test_deployment.py::TestEcrRegistryDerivedFromImage (5 new tests) — actually executes the real variable-derivation lines from deploy.sh in a bash subprocess (not just text/regex matching, since this is a bash variable-scoping bug): staging image → staging registry, production image → production registry (the exact regression), FULL_IMAGE still correct, no-ECR_IMAGE falls back to staging, and a static guard against ECR_REGISTRY ever being re-hardcoded. 34/34 in the file, no regressions.

Deployed and verified live on staging — a full real deploy via bash deploy/push.sh staging ran end-to-end with the fixed script (login → pull → migrate → health-check → blue/green flip), succeeded cleanly.

Closing.

✓
closed this issue 34 days ago