From 8aa7d6fafb5b9496ca372550eaaad4ac11936327 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 21 May 2026 19:20:15 +0200 Subject: [PATCH] fix(env-check): mark APNs allOrNone and render the satisfying anyOf alternative MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two W10 QA findings on the v1.4.42 env-check wave, both tied to the v1.4.40 AP-2 silent-disable scenario this script was conceived to catch: 1. SD-H1 — the APNs group in `scripts/env-manifest.json` shipped without `allOrNone: true`, so the synthetic `` required row never fired. An operator setting three of four `APNS_*` vars and leaving `APNS_KEY` / `APNS_KEY_FILE` empty would see exit 0 and green-light the deploy — exactly the v1.4.40 paper-cut the wave was meant to prevent. Adds the flag. 2. CR-H2 — the renderer printed `[OK] APNS_KEY` even when only `APNS_KEY_FILE` was set. An operator grepping the env block for `APNS_KEY` would find nothing and file a ghost issue. The `CheckResult` now carries `satisfiedBy`, populated to the matched alternative when it differs from the primary; the renderer prints `[OK] APNS_KEY (satisfied by APNS_KEY_FILE)`. Tests pin both fixes — the AP-2 scenario synthetic-row trip (16 → 19 passing), and the `satisfiedBy` branch coverage on the anyOf match path. --- scripts/__tests__/check-env.test.ts | 48 +++++++++++++++++++++++++++++ scripts/check-env.ts | 18 +++++++++-- scripts/env-manifest.json | 3 +- 3 files changed, 66 insertions(+), 3 deletions(-) diff --git a/scripts/__tests__/check-env.test.ts b/scripts/__tests__/check-env.test.ts index 5d880cb71..c27d6fd51 100644 --- a/scripts/__tests__/check-env.test.ts +++ b/scripts/__tests__/check-env.test.ts @@ -66,6 +66,7 @@ const SAMPLE_MANIFEST = { name: "APNs", description: "iOS push", required: false, + allOrNone: true, variables: [ { name: "APNS_KEY_ID", purpose: "key id" }, { name: "APNS_TEAM_ID", purpose: "team id" }, @@ -134,6 +135,34 @@ describe("checkEnv — anyOf alternatives", () => { expect(apnsKey.present).toBe(true); }); + it("records satisfiedBy when an anyOf alternative satisfies the row", () => { + // The label `[OK] APNS_KEY` was misleading pre-v1.4.42 — an operator + // who saw it would grep for `APNS_KEY` in the env block and find + // nothing, file a ghost issue. `satisfiedBy` lets the renderer + // surface the alternative that actually matched. + const out = checkEnv(SAMPLE_MANIFEST, { + DATABASE_URL: "x", + ENCRYPTION_KEY: "y", + APNS_KEY_ID: "abc", + APNS_TEAM_ID: "def", + APNS_KEY_FILE: "/secrets/apns.p8", + }); + const apnsKey = out.find((r) => r.variable === "APNS_KEY")!; + expect(apnsKey.satisfiedBy).toBe("APNS_KEY_FILE"); + }); + + it("leaves satisfiedBy undefined when the primary name is the one set", () => { + const out = checkEnv(SAMPLE_MANIFEST, { + DATABASE_URL: "x", + ENCRYPTION_KEY: "y", + APNS_KEY_ID: "abc", + APNS_TEAM_ID: "def", + APNS_KEY: "-----BEGIN PRIVATE KEY-----...", + }); + const apnsKey = out.find((r) => r.variable === "APNS_KEY")!; + expect(apnsKey.satisfiedBy).toBeUndefined(); + }); + it("treats APNS_KEY as satisfied when APNS_KEY is set (the 12-factor variant)", () => { const out = checkEnv(SAMPLE_MANIFEST, { DATABASE_URL: "x", @@ -191,6 +220,25 @@ describe("checkEnv — all-or-none groups", () => { }); expect(out.find((r) => r.variable === "")).toBeUndefined(); }); + + it("catches the v1.4.40 AP-2 scenario — 3 of 4 APNS_* set, key missing", () => { + // Direct regression pin for the gap the env-check wave was conceived + // to close. Pre-v1.4.42 the APNs group lacked `allOrNone: true` so + // this exact env-block shape exited 0 and silently disabled APNs. + const out = checkEnv(SAMPLE_MANIFEST, { + DATABASE_URL: "x", + ENCRYPTION_KEY: "y", + APNS_KEY_ID: "abc", + APNS_TEAM_ID: "def", + // APNS_KEY / APNS_KEY_FILE deliberately absent + }); + const synth = out.find( + (r) => r.group === "APNs" && r.variable === "", + )!; + expect(synth).toBeDefined(); + expect(synth.required).toBe(true); + expect(synth.note).toContain("2/3"); + }); }); describe("checkEnv — optional groups (informational)", () => { diff --git a/scripts/check-env.ts b/scripts/check-env.ts index 061d22e13..29e563ec7 100644 --- a/scripts/check-env.ts +++ b/scripts/check-env.ts @@ -67,6 +67,14 @@ interface CheckResult { present: boolean; required: boolean; note?: string; + /** + * For `anyOf` rows: the alternative that actually satisfied the row. + * Set only when the matched name differs from the primary `variable` + * — e.g. APNS_KEY satisfied by APNS_KEY_FILE. Lets the renderer print + * `[OK] APNS_KEY (satisfied by APNS_KEY_FILE)` instead of misleading + * the operator into greping for the primary name. + */ + satisfiedBy?: string; } /** @@ -129,7 +137,8 @@ export function checkEnv( for (const v of group.variables) { const names = v.anyOf ?? [v.name]; - const present = names.some((n) => isPresent(env[n])); + const matched = names.find((n) => isPresent(env[n])); + const present = matched !== undefined; const note = v.anyOf ? `Satisfied by any of: ${v.anyOf.join(", ")}` : undefined; @@ -139,6 +148,8 @@ export function checkEnv( present, required: group.required, note, + satisfiedBy: + present && matched && matched !== v.name ? matched : undefined, }); if (present) presentInGroup++; else absentInGroup++; @@ -188,7 +199,10 @@ function renderResults(results: CheckResult[]): { currentGroup = r.group; } if (r.present) { - console.log(` [OK] ${r.variable}`); + console.log( + ` [OK] ${r.variable}` + + (r.satisfiedBy ? ` (satisfied by ${r.satisfiedBy})` : ""), + ); } else if (r.required) { console.log( ` [MISSING-REQUIRED] ${r.variable}` + diff --git a/scripts/env-manifest.json b/scripts/env-manifest.json index def38748d..1bc7b0c8e 100644 --- a/scripts/env-manifest.json +++ b/scripts/env-manifest.json @@ -56,8 +56,9 @@ }, { "name": "APNs (iOS push)", - "description": "All four core APNS_* vars together. Missing any disables APNs silently — the v1.4.40 AP-2 gap that motivated this manifest.", + "description": "All four core APNS_* vars together. Missing any disables APNs silently — the v1.4.40 AP-2 gap that motivated this manifest. allOrNone trips the synthetic required row whenever 1-3 of the 4 are set.", "required": false, + "allOrNone": true, "variables": [ { "name": "APNS_KEY_ID",