fix(docker): copy .npmrc before npm ci and pin the Node major to .nvmrc

Dockerfile

@@ -1,4 +1,6 @@
-FROM node:lts-slim
+# Pinned to the major declared in .nvmrc (not `lts-slim`, which floats
+# independently of it). Kept in sync by scripts/tests/dockerfileToolchain.unit.tests.js.
+FROM node:24-slim
 
 # switch user
 USER node
@@ -9,7 +11,7 @@ WORKDIR /home/node
 # Install app dependencies & setup
 ARG NODE_ENV=production
 ENV NODE_ENV=${NODE_ENV}
-COPY --chown=node:node package*.json ./
+COPY --chown=node:node package*.json .npmrc ./
 RUN if [ "$NODE_ENV" = "test" ]; then npm ci --include=dev; else npm ci --omit=dev; fi
 COPY --chown=node:node . .
 

ERRORS.md

@@ -37,3 +37,4 @@ Use this file as a compact memory of recurring AI mistakes.
 - [2026-07-17] billing/stripe: #3964 fixed the admin (409 abort) and reconcile (skip-comparison) call sites of the shared plan resolver but left `billing.webhook.service.js resolvePlan`'s `?? 'free'` fallback in place -> a webhook for an existing paid org whose Stripe price is unresolvable (unmapped `priceId`, no valid `metadata.planId` — e.g. a manually-sold enterprise price) silently downgraded the org to `'free'` on the next `customer.subscription.updated`/`.created`/`invoice.payment_succeeded` event; a webhook cannot 409-abort mid-flight like the admin path, so the fix instead retains the org's/subscription's already-loaded `.plan` (never a hardcoded `'free'`) and emits `billing.webhook.plan_unresolved` for manual review — only a brand-new subscription with zero prior plan reference (no DB row anywhere for that org) still defaults to `'free'`; see pierreb-devkit/Node#3970
 - [2026-07-25] billing: `forceRotateForPlanChange` (week-quota snapshot rotation) was wired only into `handleSubscriptionUpdated`'s plan-change block -> `handleCheckoutCompleted` and both branches of `handleSubscriptionCreated` never rotated the current-week quota snapshot after a mid-week plan activation, so an upgraded org kept the previous plan's weekly quota (often 0 on free→paid) until the next weekly reset; fixed by calling the same non-fatal, logged `forceRotateForPlanChange(organizationId, { preserveUsage: true })` from all three activation call sites, and by making `billing.usage.service.js incrementMeter` read the live plan quota (already fetched for the snapshot write) instead of the potentially-stale `updatedDoc.meterQuota` for overflow decisions — mirrors the existing live-quota display fix in `billing.controller.js`; see pierreb-devkit/Node#3988
 - [2026-09-04] deps/engines: `engines.node` declared `">=22.0.0"` while the committed lock was already npm-11-shaped, so `npm ci` under npm 10 (bundled by every Node 22.x) failed mid-install with a confusing `Missing: conventional-commits-filter@6.0.1 from lock file` error instead of a clean engines rejection -> a public package's `engines.node`/`engines.npm` must bound the exact npm major that wrote the lock, not just "a Node version that happens to work today"; pin the toolchain with `.nvmrc` + CI's `setup-node` reading it (`node-version-file`, not a floating `lts/*`) and add `engine-strict=true` to `.npmrc` so an unsupported toolchain fails fast at the engines check instead of at dependency resolution; see pierreb-devkit/Node#4053
+- [2026-09-04] docker: #4053 pinned the toolchain for CI and local installs, but the Dockerfile still copied `package*.json`, ran `npm ci`, and only then `COPY . .` — so `.npmrc` (`engine-strict=true`) landed AFTER the install, and `FROM node:lts-slim` floated independently of `.nvmrc`, so an unsupported base image inside Docker still reproduced the confusing lockfile error instead of a clean `EBADENGINE`, and the image could silently drift onto a newer Node major once it enters LTS -> a toolchain fail-fast fix must be verified in every install path, not just the one exercised by CI; copy `.npmrc` alongside `package*.json` before `npm ci` in every installing stage, and pin `FROM` to the `.nvmrc` major with a unit test checking the two against each other instead of a floating tag; see pierreb-devkit/Node#4060

scripts/tests/dockerfileToolchain.unit.tests.js

@@ -0,0 +1,138 @@
+/**
+ * Unit tests for the Dockerfile ↔ toolchain consistency guarded by #4060.
+ *
+ * `FROM` cannot read `.nvmrc` at build time, so the Node major is restated
+ * as a literal in the Dockerfile instead of derived. These tests are the
+ * drift guard: if the Dockerfile's major and `.nvmrc` are ever edited
+ * independently, this fails instead of the drift going unnoticed until a
+ * Node LTS rollover changes what `FROM node:lts-slim` resolves to.
+ *
+ * The second test guards the sibling defect from the same issue: `.npmrc`
+ * (carrying `engine-strict=true`) must be copied into the build context
+ * before `npm ci` runs, or the engines fail-fast never reaches the
+ * Docker build.
+ *
+ * Both checks are stage-aware: the Dockerfile is split on every top-level
+ * `FROM` into per-stage blocks, and each check walks every stage instead of
+ * only the first. A single-occurrence check would miss a multi-stage build
+ * where an earlier stage (e.g. a build stage) is correct but a later one
+ * (e.g. the runtime stage that actually ships) drifts.
+ *
+ * Scope, decided deliberately for #4060's review:
+ *  - A `FROM` whose image is not `node:...` (a distroless/scratch runtime,
+ *    or `FROM <previous-stage-name>`) has no Node major to check, so it's
+ *    skipped by the major-pin test — but if that stage runs `npm ci`, it's
+ *    still held to the .npmrc-before-npm-ci rule.
+ *  - The .npmrc check is stage-local only: it does not trace whether a
+ *    stage that `FROM`s a previous *named* stage inherits an already-copied
+ *    `.npmrc` from that parent. Resolving that means walking the FROM
+ *    graph, which is a Dockerfile parser, not a drift guard for one file.
+ *    A stage that runs `npm ci` is expected to COPY `.npmrc` itself; widen
+ *    this if that inheritance pattern shows up for real.
+ *  - A digest-pinned image (`node:24-slim@sha256:...`) is accepted the
+ *    same as its tag-only form for the major check.
+ *  - Stage boundaries are detected separately from image-token parsing: any
+ *    line starting with `FROM` opens a new stage, but if its shape can't be
+ *    parsed (e.g. a `--platform=...` flag, which this guard doesn't
+ *    support) the test throws instead of silently merging it into the
+ *    previous stage — a strict-regex-as-splitter would make an
+ *    unrecognized `FROM` vanish rather than fail, which is the same class
+ *    of blind spot #4060's review flagged in the first place.
+ */
+import { describe, test, expect } from '@jest/globals';
+import { readFileSync } from 'node:fs';
+import { fileURLToPath } from 'node:url';
+import path from 'node:path';
+
+const rootDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..');
+const dockerfile = readFileSync(path.join(rootDir, 'Dockerfile'), 'utf8');
+
+/**
+ * Splits the Dockerfile into per-stage blocks. A new stage starts at every
+ * top-level `FROM` line; each block runs from its `FROM` up to (but not
+ * including) the next `FROM`, or end of file for the last stage.
+ *
+ * Boundary detection (which lines start a stage) is intentionally separate
+ * from image-token extraction (what the stage's image is): matching both
+ * in one strict regex would make a `FROM` line the regex doesn't recognize
+ * silently disappear into the previous stage's block instead of failing —
+ * exactly the kind of miss this guard exists to prevent. So any line that
+ * starts with `FROM` opens a stage; if its shape can't then be parsed, the
+ * test throws rather than guessing.
+ *
+ * @param {string} text - the full Dockerfile contents
+ * @returns {Array<{image: string, block: string}>} one entry per stage, in file
+ *   order: `image` is the raw image token from the `FROM` line (tag or
+ *   digest-pinned, or a prior stage's alias), `block` is that stage's lines up
+ *   to the next `FROM` or end of file.
+ * @throws {Error} if a `FROM` line's shape cannot be parsed, rather than
+ *   silently folding it into the previous stage.
+ */
+function getStages(text) {
+  const fromLineRe = /^FROM\b[^\n]*$/gim;
+  const imageRe = /^FROM[ \t]+(\S+)(?:[ \t]+AS[ \t]+\S+)?[ \t]*$/i;
+
+  const froms = [];
+  let match;
+  while ((match = fromLineRe.exec(text)) !== null) {
+    const line = match[0];
+    const imageMatch = line.match(imageRe);
+    if (!imageMatch) {
+      throw new Error(
+        `Dockerfile has a FROM line the stage-splitter can't parse (e.g. a --platform flag ` +
+          `isn't supported) — extend getStages() in dockerfileToolchain.unit.tests.js or ` +
+          `simplify the line: "${line}"`,
+      );
+    }
+    froms.push({ index: match.index, image: imageMatch[1] });
+  }
+
+  return froms.map((from, i) => ({
+    image: from.image,
+    block: text.slice(from.index, i + 1 < froms.length ? froms[i + 1].index : text.length),
+  }));
+}
+
+describe('Dockerfile toolchain pin (#4060)', () => {
+  test('every FROM node:<major>-slim stage matches the major pinned in .nvmrc', () => {
+    const nvmrcMajor = readFileSync(path.join(rootDir, '.nvmrc'), 'utf8')
+      .trim()
+      .replace(/^v/, '')
+      .split('.')[0];
+
+    const stages = getStages(dockerfile);
+    const nodeStages = stages.filter((stage) => /^node:/.test(stage.image));
+    expect(nodeStages.length).toBeGreaterThan(0);
+
+    for (const stage of nodeStages) {
+      const majorMatch = stage.image.match(/^node:(\d+)(?:\.\d+)*-slim(?:@sha256:[0-9a-f]{64})?$/);
+      if (!majorMatch) {
+        throw new Error(
+          `FROM ${stage.image} is a node image but not a pinned "node:<major>-slim" form ` +
+            `(.nvmrc pins major ${nvmrcMajor}) — a floating tag like "node:lts-slim" defeats the pin`,
+        );
+      }
+      expect(majorMatch[1]).toBe(nvmrcMajor);
+    }
+  });
+
+  test('.npmrc is copied before npm ci runs, in every stage that runs npm ci', () => {
+    const npmCiRe = /^RUN[^\n]*npm ci/m;
+    const npmrcCopyRe = /^COPY[^\n]*\.npmrc[^\n]*$/m;
+
+    const stages = getStages(dockerfile);
+    const npmCiStages = stages.filter((stage) => npmCiRe.test(stage.block));
+    expect(npmCiStages.length).toBeGreaterThan(0);
+
+    for (const stage of npmCiStages) {
+      const npmCiIndex = stage.block.search(npmCiRe);
+      const copyIndex = stage.block.search(npmrcCopyRe);
+      if (copyIndex === -1 || copyIndex > npmCiIndex) {
+        throw new Error(
+          `Stage "FROM ${stage.image}" runs npm ci without copying .npmrc first in that same ` +
+            'stage (engine-strict from .npmrc must be in place before npm ci runs)',
+        );
+      }
+    }
+  });
+});