bermudalamb/redefined-designs
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects 3 Releases Wiki Activity

docs(ops): the compose header contradicts itself, and step 2 drops four interpolated variables #204

New Issue
Closed
opened 2026-08-28 11:51:43 -05:00 by bermudalamb · 0 comments
bermudalamb commented 2026-08-28 11:51:43 -05:00
Owner
Copy Link
Copy Source

A review of #196 verified every quoted error line in the cutover runbook by compiling envValidation.ts and running validateEnv against production's real entry set — the 1 problem(s) and 3 problem(s) counts and the PayPal messages all match what the code emits. It also found three things that do not.

1. docker-compose.prod.yml:60-61 asserts something the same commit contradicts three lines later

# Required stack environment variables. All must be set in Portainer for this
# stack. All are secrets except DEMO_MODE:

Neither half is true. Four of the entries beneath it are not required — PAYPAL_CLIENT_ID/_SECRET/_WEBHOOK_ID are, per #196's own edit at :79-83, "Unused while DEMO_MODE is true", and BACKUP_PASSPHRASE, USPS_CLIENT_ID and USPS_CLIENT_SECRET are each labelled "Optional." And SMTP_FROM ("The From address customers see") is not a secret, so the carve-out for DEMO_MODE is wrong too.

This is not cosmetic, because production-stack-cutover.md:96 sends the operator to this exact block: "The authoritative list is in the header comment of that file, under 'Required stack environment variables'." An operator cutting over during the demo interim reads "all must be set", does not have live PayPal credentials — which is the entire reason the interim exists — and either blocks the cutover or invents a BACKUP_PASSPHRASE, which backup-and-restore.md:121 warns is how you get archives nobody can decrypt.

The pre-#196 wording was equally wrong, so this is not a regression; but #196 rewrote the line and made the assertion more emphatic while adding the sentence that falsifies it. Drop the blanket claim rather than carving out one exception: each entry already says whether it is required and when.

2. production-stack-cutover.md:63 lists nine of thirteen interpolated variables

Missing: USPS_CLIENT_ID, USPS_CLIENT_SECRET (docker-compose.prod.yml:185-186), BACKUP_PASSPHRASE (:360), UPLOADS_BASE_URL (:202).

USPS_* is the one that bites. Those almost certainly exist in today's stack, they are discarded with it at step 5, and losing them fails silently — the compose file's own comment at :182-183 says address validation "is skipped when these are empty, rather than failing, so an unset pair is a working configuration". Checkout quietly starts accepting unvalidated addresses and step 7 has nothing that would catch it.

Mitigated by the bolded lead-in ("Every stack environment variable") and step 3's redirect, but the enumeration reads as the checklist — which is exactly why adding DEMO_MODE to it was the point of #196.

3. The DB_PASSWORD failure mechanism is wrong, and its error appears above DEMO_MODE

:63 says "A missing DB_PASSWORD cannot authenticate against its own data directory." That is not what happens — the app never reaches a connection attempt. checkAlwaysRequired sees PGPASSWORD='' and refuses at boot. Verified output for a wholesale missed step 2:

[config] refusing to start — 2 problem(s) with the environment:
[config]   - PGPASSWORD is required and is not set.
[config]   - DEMO_MODE is required and must be exactly 'true' or 'false'. It decides…

So :222 headlines the crash-loop section as "the likeliest outcome of a missed step 2", but neither quoted block matches that case: the operator sees 2 problem(s) led by PGPASSWORD, not the 1 problem(s) DEMO_MODE block. With an already-initialised PGDATA the Postgres container also ignores an empty POSTGRES_PASSWORD and starts normally, so there is no authentication failure to observe either.

Minor

  • The DEMO_MODE error is quoted as a truncated prefix; the real message continues "It decides whether real payments are taken, so it has to be stated rather than inherited." The PayPal block beside it quotes in full. Grep still succeeds, so this is cosmetic.
  • Only one of checkDemoMode's two messages is shown. A value typed wrong gives DEMO_MODE must be exactly 'true' or 'false', but is 'True'. — at least as likely as unset for someone who read step 2, and the trailing-whitespace case is only distinguishable by the quotes.
  • "DEMO_MODE is the only one that is not a secret" (:65) is false — SMTP_FROM and UPLOADS_BASE_URL are not secrets either. "The only one that is a setting rather than a credential" would be true and a better hook.

Found by

Code review of 2fe3aa0 (#196), Important 1-3 plus Minor 1-3. Docs and compose comments only.

A review of #196 verified every quoted error line in the cutover runbook by compiling `envValidation.ts` and running `validateEnv` against production's real entry set — the `1 problem(s)` and `3 problem(s)` counts and the PayPal messages all match what the code emits. It also found three things that do not. ## 1. `docker-compose.prod.yml:60-61` asserts something the same commit contradicts three lines later ``` # Required stack environment variables. All must be set in Portainer for this # stack. All are secrets except DEMO_MODE: ``` Neither half is true. Four of the entries beneath it are not required — `PAYPAL_CLIENT_ID/_SECRET/_WEBHOOK_ID` are, per #196's own edit at `:79-83`, "Unused while `DEMO_MODE` is `true`", and `BACKUP_PASSPHRASE`, `USPS_CLIENT_ID` and `USPS_CLIENT_SECRET` are each labelled "Optional." And `SMTP_FROM` ("The From address customers see") is not a secret, so the carve-out for `DEMO_MODE` is wrong too. This is not cosmetic, because `production-stack-cutover.md:96` sends the operator to this exact block: *"The authoritative list is in the header comment of that file, under 'Required stack environment variables'."* An operator cutting over during the demo interim reads "all must be set", does not have live PayPal credentials — which is the entire reason the interim exists — and either blocks the cutover or invents a `BACKUP_PASSPHRASE`, which `backup-and-restore.md:121` warns is how you get archives nobody can decrypt. The pre-#196 wording was equally wrong, so this is not a regression; but #196 rewrote the line and made the assertion more emphatic while adding the sentence that falsifies it. Drop the blanket claim rather than carving out one exception: each entry already says whether it is required and when. ## 2. `production-stack-cutover.md:63` lists nine of thirteen interpolated variables Missing: `USPS_CLIENT_ID`, `USPS_CLIENT_SECRET` (`docker-compose.prod.yml:185-186`), `BACKUP_PASSPHRASE` (`:360`), `UPLOADS_BASE_URL` (`:202`). `USPS_*` is the one that bites. Those almost certainly exist in today's stack, they are discarded with it at step 5, and losing them **fails silently** — the compose file's own comment at `:182-183` says address validation "is skipped when these are empty, rather than failing, so an unset pair is a working configuration". Checkout quietly starts accepting unvalidated addresses and step 7 has nothing that would catch it. Mitigated by the bolded lead-in ("**Every** stack environment variable") and step 3's redirect, but the enumeration reads as the checklist — which is exactly why adding `DEMO_MODE` to it was the point of #196. ## 3. The `DB_PASSWORD` failure mechanism is wrong, and its error appears above `DEMO_MODE` `:63` says *"A missing `DB_PASSWORD` cannot authenticate against its own data directory."* That is not what happens — the app never reaches a connection attempt. `checkAlwaysRequired` sees `PGPASSWORD=''` and refuses at boot. Verified output for a wholesale missed step 2: ``` [config] refusing to start — 2 problem(s) with the environment: [config] - PGPASSWORD is required and is not set. [config] - DEMO_MODE is required and must be exactly 'true' or 'false'. It decides… ``` So `:222` headlines the crash-loop section as "the likeliest outcome of a missed step 2", but neither quoted block matches that case: the operator sees `2 problem(s)` led by `PGPASSWORD`, not the `1 problem(s)` `DEMO_MODE` block. With an already-initialised `PGDATA` the Postgres container also ignores an empty `POSTGRES_PASSWORD` and starts normally, so there is no authentication failure to observe either. ## Minor - The `DEMO_MODE` error is quoted as a truncated prefix; the real message continues "It decides whether real payments are taken, so it has to be stated rather than inherited." The PayPal block beside it quotes in full. Grep still succeeds, so this is cosmetic. - Only one of `checkDemoMode`'s two messages is shown. A value typed wrong gives `DEMO_MODE must be exactly 'true' or 'false', but is 'True'.` — at least as likely as unset for someone who read step 2, and the trailing-whitespace case is only distinguishable by the quotes. - "`DEMO_MODE` is the only one that is not a secret" (`:65`) is false — `SMTP_FROM` and `UPLOADS_BASE_URL` are not secrets either. "The only one that is a setting rather than a credential" would be true and a better hook. ## Found by Code review of `2fe3aa0` (#196), Important 1-3 plus Minor 1-3. Docs and compose comments only.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
calamb (Chelsea Lamb) bermudalamb (Thom Lamb)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: bermudalamb/redefined-designs#204