Merge pull request 'Feature/107 compose required env' (#108) from feature/107-compose-required-env into main

Reviewed-on: #108

README.md

@@ -97,6 +97,10 @@ mkdir -p /tmp/redefined-uploads
 
 `PUBLIC_URL` is required only alongside SMTP because its only job is building links in email; an environment that cannot send mail does not need it. `UPLOADS_DIR` has no such reprieve — its fallback of `/app/uploads` is correct inside the container and wrong everywhere else.
 
+**Adding to the required list has reach beyond this repository.** A variable added to `ALWAYS_REQUIRED` must also be set in every environment that deploys, and there are two of those. `docker-compose.qa.yml` is checked automatically — `backend/tests/unit/composeEnvironment.test.ts` reads the validator's own list and fails if the compose file does not set something on it, which is what #107 existed to prevent from recurring. Production runs from a Portainer stack **outside this repository**, so nothing can check it: that one has to be updated by hand, before the deploy rather than during it.
+
+Note also that setting a variable in Portainer's stack environment is not the same as giving it to the container. Stack variables are interpolated into the compose file as `${VAR}`; a service receives exactly what its own `environment:` block lists. A variable with no line there never arrives, however carefully it was set in the UI.
+
 **Note (PowerShell):** environment variables set with `$env:` only last for the current terminal session/tab. If you close and reopen VS Code's terminal, you'll need to re-run step 3 before starting the backend again.
 
 ### 4. Run the backend

backend/src/envValidation.ts

@@ -27,7 +27,13 @@ export interface EnvValidation {
 }
 
 // Without these the process cannot do its job at all.
-const ALWAYS_REQUIRED = [
+//
+// Exported so tests/unit/composeEnvironment.test.ts can assert the deploying
+// environment actually sets them. #107 happened because this list grew and
+// docker-compose.qa.yml did not: the check has to read this list rather than a
+// copy of it, or the next variable added here goes unguarded in exactly the
+// same way.
+export const ALWAYS_REQUIRED = [
   'PGHOST',
   'PGPORT',
   'PGUSER',

backend/tests/unit/composeEnvironment.test.ts

@@ -0,0 +1,76 @@
+import { readFileSync } from 'fs';
+import path from 'path';
+import { ALWAYS_REQUIRED } from '../../src/envValidation';
+
+/**
+ * The guard for #107.
+ *
+ * That failure was not the missing variable — it was that nothing connected two
+ * files. envValidation.ts gained a required variable and docker-compose.qa.yml
+ * did not set it, and nothing noticed until the container refused to boot on a
+ * deploy. CI passed throughout, because CI sets its own environment and never
+ * reads the compose file.
+ *
+ * So this reads the real list from the validator rather than a copy. A copy
+ * would pass forever while the next added variable went unguarded in precisely
+ * the same way.
+ *
+ * What this cannot cover: production runs from a Portainer stack outside this
+ * repository, so nothing here can check it. Adding a required variable still
+ * means updating that stack by hand, and this test is not evidence that it was
+ * done.
+ */
+
+const COMPOSE_PATH = path.resolve(__dirname, '..', '..', '..', 'docker-compose.qa.yml');
+const compose = readFileSync(COMPOSE_PATH, 'utf8');
+
+// Only real environment entries — `- NAME=value` at an indented list position.
+// A mention inside a comment cannot match, because a comment line starts with #.
+function environmentEntries(source: string): Map<string, string> {
+  const entries = new Map<string, string>();
+  for (const line of source.split('\n')) {
+    const match = /^\s+- ([A-Z_0-9]+)=(.*)$/.exec(line);
+    if (match) {
+      entries.set(match[1], match[2].trim());
+    }
+  }
+  return entries;
+}
+
+const entries = environmentEntries(compose);
+
+describe('the QA compose file provides everything the app requires to boot', () => {
+  it('parsed some environment entries at all', () => {
+    // Guards the guard: a regex that matched nothing would make every
+    // assertion below vacuously true.
+    expect(entries.size).toBeGreaterThan(5);
+  });
+
+  it.each([...ALWAYS_REQUIRED])('sets %s', (name) => {
+    expect(entries.has(name)).toBe(true);
+  });
+
+  // DEMO_MODE is required too, but validated separately from ALWAYS_REQUIRED
+  // because its rule is stricter than presence — it must be exactly 'true' or
+  // 'false'. Named explicitly here so it is not missed by reading only the list.
+  it('sets DEMO_MODE, to one of the two values that are allowed', () => {
+    expect(entries.has('DEMO_MODE')).toBe(true);
+    expect(['true', 'false']).toContain(entries.get('DEMO_MODE'));
+  });
+
+  // The reason UPLOADS_DIR is hardcoded rather than taken from a stack
+  // variable is that it has to agree with the volume mapping. That claim is
+  // only worth making if something checks it.
+  it('points UPLOADS_DIR at the directory the uploads volume is mounted on', () => {
+    const uploadsDir = entries.get('UPLOADS_DIR');
+    expect(uploadsDir).toBeTruthy();
+    expect(compose).toContain(`:${uploadsDir}`);
+  });
+
+  // Interpolated rather than hardcoded, so the secret itself never enters the
+  // repository. Present as a reference is what matters; an unset stack variable
+  // resolves to empty, which the validator and the gate both read as "off".
+  it('references ADMIN_GATE_SECRET from the stack rather than holding a value', () => {
+    expect(entries.get('ADMIN_GATE_SECRET')).toBe('${ADMIN_GATE_SECRET}');
+  });
+});

docker-compose.qa.yml

@@ -37,6 +37,9 @@
 #   QA_SMTP_USER    — Brevo SMTP login. Named QA_ for the same reason as the
 #   QA_SMTP_PASSWORD  database password: pasting production's variables in here
 #   QA_SMTP_FROM      must not silently work.
+#   ADMIN_GATE_SECRET — the shared secret Nginx Proxy Manager injects as the
+#                     X-Admin-Gate header on the gated location. Both sides must
+#                     hold the same value or the admin API returns 403. See #63.
 
 services:
   redefined-designs-qa:
@@ -98,6 +101,19 @@ services:
       - SITE_CURRENCY=USD
       - RESERVATION_MINUTES=15
       - PUBLIC_URL=${PUBLIC_URL}
+
+      # Required since #64, and previously inherited from the code's fallback —
+      # which is exactly what that change set out to stop. Hardcoded rather than
+      # taken from a stack variable because it is not a secret and because it
+      # has to match the right-hand side of the volume mapping below; splitting
+      # it across two files is how they drift apart.
+      - UPLOADS_DIR=/app/uploads
+
+      # Interpolated from the stack environment so the secret itself never
+      # enters the repository. Note that setting it in Portainer alone is not
+      # enough: stack variables are substituted into this file, not handed to
+      # the container, so a variable with no line here never reaches the app.
+      - ADMIN_GATE_SECRET=${ADMIN_GATE_SECRET}
     volumes:
       # Separate uploads directory. Sharing production's would let a QA run
       # write into, and a QA teardown delete, real product images.