fix(dev-env): pass the whole of .env through, not seven names of it - #42
Conversation
The script sources `.env` and then printed seven `export` lines. Everything else
it had just read died with the subshell, because both callers do
`eval "$(scripts/dev-env.sh)"` — so a variable added to `.env` never reached the
server. `README.md` said "the script loads .env, which the server does not do
itself", and it loaded seven keys.
# How it presents, which is the worst part
Silently, and split in two. `MEDIA_URL` unset makes the call route answer 503,
while `VITE_MEDIA_URL` — read by Vite from `apps/web/.env`, a different file —
still shows the call button. The client offers a call the server refuses and
neither side says why: finding it takes reading the network panel for a 503 on
`/v1/groups/{id}/call/token`.
`VAPID_SUBJECT` behaves the same way, which is why web push had to be started
with the variable on the command line. `ACCOUNT_STORAGE_BYTES` silently reverts
to its default.
# What is emitted now
Every name the file defines, minus the five this script computes. Re-emitting
`DATABASE_URL` or `SERVER_ADDR` from the file would put every branch back on one
database and one port, which is the thing this file exists to prevent.
The names come from the file rather than from the environment: `set -a` exported
`.env` into this process, but so is `PATH` and everything else a shell carries,
and emitting the environment would hand the caller a copy of ours.
# Checked by hand, since nothing here is unit-testable
- `scripts/dev-server.sh --release` alone now starts a server whose `/proc`
environment carries `MEDIA_URL`, `MEDIA_API_KEY`, `MEDIA_API_SECRET`,
`RELAY_URLS` and `RELAY_SECRET`. It carried none before.
- `DATABASE_URL` still resolves to the branch's own database, and `SERVER_ADDR`
to its own port.
- A value with an apostrophe survives the `eval` — that is what the escaping is
for, and it is the case that would have broken it.
- An empty value stays empty rather than failing under `set -u`.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7787d70162
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # the thing this file exists to prevent. | ||
| derived=" DATABASE_URL SERVER_ADDR WHISPEE_API ALLOWED_ORIGINS WEB_PORT " | ||
|
|
||
| sed -n 's/^[[:space:]]*\([A-Za-z_][A-Za-z0-9_]*\)=.*/\1/p' "$env_file" | while read -r name; do |
There was a problem hiding this comment.
.env
When .env uses the common shell-compatible form export MEDIA_URL=... (or any other export NAME=value assignment), sourcing the file sets the variable, but this sed expression does not extract its name, so the generated output silently omits it and neither launcher passes it onward. This contradicts the new promise that every value defined by the file reaches the server; accept the optional export prefix when collecting assignment names.
Useful? React with 👍 / 👎.
1 participant
scripts/dev-env.shsources.envand then printed sevenexportlines. Everything else it had just read died with the subshell, because both callers doeval "$(scripts/dev-env.sh)". A variable added to.envtherefore never reached the server, andREADME.mdsaid "the script loads .env, which the server does not do itself" while it loaded seven keys.How it presents, which is the part worth fixing
Silently, and split across two halves that contradict each other.
MEDIA_URLunset makes/v1/groups/{id}/call/tokenanswer 503 —Media::from_environmentfinds nothing,media.sfuisNone, andcall_tokenreturnsApiError::Unavailable. MeanwhileVITE_MEDIA_URLis read by Vite fromapps/web/.env, a different file, soCALLS_CONFIGUREDis true and the call button is drawn.The client offers a call the server refuses. Neither side says why: the 503 is only visible in the network panel, and the thread simply records "Appel manqué".
VAPID_SUBJECTbehaves identically — which is why web push had to be started with the variable on the command line rather than from.env.ACCOUNT_STORAGE_BYTESsilently reverts to its default, and a quota nobody set looks exactly like a quota that was set.What is emitted now
Every name the file defines, minus the five this script computes. Re-emitting
DATABASE_URLorSERVER_ADDRfrom the file would put every branch back on one database and one port, which is what this script exists to prevent — so those keep their derived values.The names are read back out of the file rather than taken from the environment.
set -aexported.envinto this process, but so isPATHand everything else a shell carries; emitting the environment would hand the caller a copy of ours. Reading the keys from the file is what makes "what the file defines" the exact boundary.What was run
Nothing here is unit-testable — it is a shell script whose output is
evaled — so it was checked by hand:scripts/dev-server.sh --releasealone now starts a server whose/proc/<pid>/environcarriesMEDIA_URL,MEDIA_API_KEY,MEDIA_API_SECRET,RELAY_URLSandRELAY_SECRET. It carried none of them before this change.DATABASE_URLstill resolves to the branch's own database andSERVER_ADDRto its own port — the isolation the script is for.eval. That is what the escaping is for and it is the case that would have broken it; a value with spaces and an empty value were checked in the same pass.bash -n scripts/dev-env.sh.cargo test -p server --release— green.How this was found
Configuring calls locally so the call button would appear. It did appear, and every call failed. The button comes from the client's own build-time flag; the refusal came from a server that had never seen
MEDIA_URL. Reading the network panel showed the 503, and following that back showed the script.