Conversation
34919b7 to
b946ef5
Compare
|
🤖 Tested locally — LGTM. ✅ Checked out the branch in an isolated worktree and ran the non-destructive checks (no containers started, DB/Desktop untouched):
Nice touch pinning the test-relay launcher's host + credentials to the local dev stack and only borrowing the port from Compose, so a developer's remote A few non-blocking notes:
(I only ran the static/rendered tier; I didn't start containers on a custom port since that shares the local Docker stack.) |
|
@ravarora2 Thanks for the thorough review and the LGTM. I addressed two small follow-ups from your non-blocking notes:
I intentionally kept a portless Verified with |
5d3c564 to
dd2a8c8
Compare
Signed-off-by: Mikhail Vlasenko <ya@smkthat.ru>
Signed-off-by: Mikhail Vlasenko <ya@smkthat.ru>
Canonicalize loopback aliases before comparing configured service ports, reject unspecified bind addresses, and cover the setup boundary before Compose or migrations can run. Co-authored-by: Ravneet Arora <rarora@squareup.com> Signed-off-by: Ravneet Arora <rarora@squareup.com>
dd2a8c8 to
ac7abeb
Compare
|
🤖 Follow-up change summary after the review blocker was fixed: What changed
Verification
The configurable-port design remains unchanged; this commit hardens only the local-address safety boundary and its regression coverage. |
ravarora2
left a comment
There was a problem hiding this comment.
lgtm.Let's wait for owners to have a final review
Summary
Local setup currently publishes fixed Docker host ports while application URLs
come from
.env. ChangingDATABASE_URLorREDIS_URLtherefore leavesCompose on the old port and can make migrations time out against the wrong
endpoint.
This change:
.env, whilepreserving the current defaults and
127.0.0.1bindings;when a loopback URL disagrees with its mapped port;
Compose while pinning database, Redis, and MinIO endpoints and credentials to
the development stack, so values from
.envcannot redirect schema changesto an external service;
.envtake effecteven when the existing Postgres and Redis containers are healthy;
mappings, credential isolation, URL validation, Compose reconciliation, and
external-endpoint protection in the relay-test launcher.
Related issue
Fixes #2479.
Related work:
DATABASE_URL.security boundary for every configurable mapping.
Compared with the open alternatives, this PR keeps the scope limited to local
service configuration while covering all published Compose ports and the
setup/test call sites that consume them.
Testing
./scripts/test-dev-service-ports.shbash -n scripts/dev-setup.sh scripts/start-relay-for-tests.sh scripts/lib/dev-service-env.sh scripts/test-dev-service-ports.shshellcheck scripts/dev-setup.sh scripts/start-relay-for-tests.sh scripts/lib/dev-service-env.sh scripts/test-dev-service-ports.shdocker compose --env-file .env.example config --quietdocker compose config --quietwith custom local Postgres, Redis, and MinIO portsjust setupwith those custom ports (Compose reconciliation, migrations,seed, dependency setup, and hooks all succeeded)
just test-unitjust ci(formatting, linting, builds, and all Rust, desktop, web, and mobiletest suites, including 4,715 desktop, 2,402 Tauri, and 1,261 mobile tests)
No UI changes; screenshots are not applicable.