From c70d59d0452b91ca765bcb04b2c340f2811a55ba Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 18:28:07 +0000 Subject: [PATCH 1/4] fix(service-settings): select crypto posture from the deployment signal only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `detectMode` read `env.VITEST` — a test-RUNNER variable — as a vote for `'test'` posture. `'test'` is the branch that takes an ephemeral key, never touches disk, and never refuses to boot, so a runner variable decided whether the fail-loud production gate ran at all. Runner variables are inherited by every process the runner spawns, so a real `os serve` spawned from a vitest worker with `{ ...process.env }` booted with production auth and test crypto. The read is deleted rather than narrowed. Its documented purpose — in-process unit tests get test posture — is preserved, because vitest sets BOTH variables on the same worker (vitest 4.1.10, `prepareVitest()`: `process.env.VITEST = "true"; process.env.NODE_ENV ??= "test";`, repeated as `NODE_ENV: process.env.NODE_ENV || "test"` in each worker's env). In-process the two spellings are indistinguishable; they differ only for an INHERITING child, which is the defect. `crypto-posture-deployment-signal.test.ts` pins both halves against a COPY of the real worker environment rather than a hand-written fixture, so it pins whatever the runner actually exports. Part of #11352 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- .../crypto-posture-deployment-signal.test.ts | 175 ++++++++++++++++++ .../src/local-crypto-provider.ts | 59 +++++- 2 files changed, 231 insertions(+), 3 deletions(-) create mode 100644 packages/services/service-settings/src/crypto-posture-deployment-signal.test.ts diff --git a/packages/services/service-settings/src/crypto-posture-deployment-signal.test.ts b/packages/services/service-settings/src/crypto-posture-deployment-signal.test.ts new file mode 100644 index 0000000000..1624dee4db --- /dev/null +++ b/packages/services/service-settings/src/crypto-posture-deployment-signal.test.ts @@ -0,0 +1,175 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #11352 — crypto posture is selected from the DEPLOYMENT signal, never from a + * test-RUNNER variable. + * + * ## What this pins, and why it is not the same as "which key was picked" + * + * `detectMode()` decides whether `LocalCryptoProvider`'s fail-loud guarantee is + * ARMED. `'test'` is not a softer flavour of `'production'` — it is the branch + * that takes an ephemeral key, never touches disk, and **never refuses to + * boot**. The refusal IS the gate, so this file is graded on whether the + * refusal is present, not on which key material a boot ended up with. + * + * Before this card, `detectMode` read: + * + * if (env.VITEST || env.NODE_ENV === 'test') return 'test'; + * + * `VITEST` describes the RUNNER, and a runner variable is inherited by every + * process the runner spawns. A real `os serve` spawned from a vitest worker + * with `{ ...process.env }` therefore booted with its crypto layer in `test` + * mode — no stable key, no disk, no refusal — however production-shaped the + * rest of that boot was. + * + * ## Why the environment here is a COPY of the real worker env + * + * A spawned child does not receive a hand-written fixture; it receives a copy + * of its parent's `process.env`. So the map every case below starts from is + * exactly that — `{ ...process.env }`, this vitest worker's own environment, + * runner variables and all — with a single deliberate mutation per case. A + * hand-written `{ VITEST: 'true' }` would pin the variable this file happens to + * know about today; a copy pins whatever the runner actually exports. + * + * `carriesRunnerVariables` below is the anti-vacuity control: if the worker + * stopped exporting runner variables altogether, every "a leaked runner + * variable does not move posture" case would pass while measuring nothing, so + * the file says so out loud instead. + */ + +import { describe, expect, it, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, rmSync, existsSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { randomBytes } from 'node:crypto'; + +import { LocalCryptoProvider } from './local-crypto-provider.js'; + +/** + * The runner-variable class, spelled the same way + * `packages/cli/test/helpers/serve-process.ts` spells it: `TEST` exactly, plus + * `VITEST` and anything `VITEST_`-prefixed. `JEST_WORKER_ID` rides along + * because the class is "a variable that says a RUNNER is present", not "the + * runner this repo uses today". + */ +const RUNNER_ENV_KEYS = [ + 'TEST', + 'VITEST', + 'VITEST_WORKER_ID', + 'VITEST_POOL_ID', + 'VITEST_MODE', + 'JEST_WORKER_ID', +] as const; + +type EnvMap = Record; + +/** This worker's REAL environment — what any child it spawned would inherit. */ +const workerEnv = (): EnvMap => ({ ...process.env } as EnvMap); + +/** Strip every key-bearing variable, so each case exercises the no-stable-key path. */ +const withoutKeySources = (env: EnvMap): EnvMap => ({ + ...env, + OS_SECRET_KEY: undefined, + OS_DEV_CRYPTO_KEY: undefined, + OBJECTSTACK_DEV_CRYPTO_KEY: undefined, + OS_CRYPTO_AUTOKEY: undefined, +}); + +describe('#11352 — crypto posture reads the deployment signal, not the runner', () => { + let home: string; + let base: EnvMap; + + beforeEach(() => { + home = mkdtempSync(join(tmpdir(), 'os-crypto-posture-')); + // `OS_HOME` and `HOME` both pinned at an empty dir: no operator-provisioned + // key file exists for any case, which is the state the refusal is about. + base = withoutKeySources({ ...workerEnv(), HOME: home, OS_HOME: home }); + }); + afterEach(() => { + rmSync(home, { recursive: true, force: true }); + }); + + const keyFile = () => join(home, 'dev-crypto-key'); + + it('is measuring something: this worker really does export runner variables', () => { + const carriesRunnerVariables = RUNNER_ENV_KEYS.filter((k) => process.env[k] !== undefined); + expect( + carriesRunnerVariables.length, + 'no runner variable is present in this worker, so every leak case below would pass vacuously', + ).toBeGreaterThan(0); + // The specific one this card is about, and the one a spawned child inherits. + expect(process.env.VITEST).toBeDefined(); + }); + + it('in-process unit tests still get test posture — carried by NODE_ENV, not by VITEST', () => { + // The half that must NOT move. vitest sets `NODE_ENV ??= 'test'` on the + // same worker it sets `VITEST=true` on, so the documented in-process + // intent survives the runner variable losing its vote. + expect(process.env.NODE_ENV).toBe('test'); + + const p = new LocalCryptoProvider({ env: base }); + expect(p.keySource).toBe('ephemeral'); + expect(existsSync(keyFile()), 'test posture must never touch disk').toBe(false); + }); + + it('a deployment in production posture REFUSES to boot without a stable key', () => { + // THE CARD. Identical map to the case above — this worker's own env, + // runner variables included — with the deployment signal set to what a real + // `os serve` deployment carries. Before #11352 the inherited `VITEST=true` + // won this decision and the boot SUCCEEDED on an ephemeral key. + const env = { ...base, NODE_ENV: 'production' }; + expect(env.VITEST, 'the leak is still in the map — that is the point').toBeDefined(); + + expect(() => new LocalCryptoProvider({ env })).toThrow(/Refusing to start in production/); + expect(existsSync(keyFile()), 'a refused boot must not have minted a key').toBe(false); + }); + + it('still resolves a real production key when one IS provisioned', () => { + // The refusal is a gate, not a wall: production posture with a stable key + // boots, runner variables present or not. + const hex = randomBytes(32).toString('hex'); + const p = new LocalCryptoProvider({ env: { ...base, NODE_ENV: 'production', OS_SECRET_KEY: hex } }); + expect(p.keySource).toBe('env:OS_SECRET_KEY'); + }); + + describe('no runner variable moves the answer, in any deployment posture', () => { + // A table rather than one case per variable: the defect class is "a runner + // variable votes", so the pin has to be that NONE of them does, in EVERY + // posture — including the postures where the wrong answer would look benign. + const postures = [ + ['production', 'production'], + ['development', 'development'], + ['test', 'test'], + ] as const; + + for (const [nodeEnv] of postures) { + for (const runnerKey of RUNNER_ENV_KEYS) { + it(`NODE_ENV=${nodeEnv} is unchanged by ${runnerKey}`, () => { + const clean: EnvMap = { ...base, NODE_ENV: nodeEnv }; + for (const k of RUNNER_ENV_KEYS) clean[k] = undefined; + const leaked: EnvMap = { ...clean, [runnerKey]: 'true' }; + + expect(outcomeOf(leaked)).toBe(outcomeOf(clean)); + }); + } + } + + /** + * Collapse a construction to the only two things this gate is about: + * did it REFUSE, and if it booted, from where did the key come. + * A fresh temp home per call so one case's minted dev key is never the + * next case's `source: 'file'`. + */ + function outcomeOf(env: EnvMap): string { + const scratch = mkdtempSync(join(tmpdir(), 'os-crypto-outcome-')); + try { + const p = new LocalCryptoProvider({ env: { ...env, HOME: scratch, OS_HOME: scratch } }); + return `booted:${p.keySource}`; + } catch (err) { + return `refused:${String((err as Error).message).split('\n')[0]}`; + } finally { + rmSync(scratch, { recursive: true, force: true }); + } + } + }); +}); diff --git a/packages/services/service-settings/src/local-crypto-provider.ts b/packages/services/service-settings/src/local-crypto-provider.ts index 243f8152b4..7525e844eb 100644 --- a/packages/services/service-settings/src/local-crypto-provider.ts +++ b/packages/services/service-settings/src/local-crypto-provider.ts @@ -56,9 +56,11 @@ import { dirname, join } from 'node:path'; * running under a key that won't survive a restart. Development and test keep * the ergonomic fallback so local loops and unit tests stay frictionless. * - * `mode` is auto-detected from `NODE_ENV` (`production` → strict; - * `test`/`VITEST` → ephemeral, no disk; otherwise `development`) and can be + * `mode` is auto-detected from `NODE_ENV` alone (`production` → strict; + * `test` → ephemeral, no disk; otherwise `development`) and can be * overridden via `opts.mode` for embedders that manage their own lifecycle. + * Which variables may and may not decide that is spelled out at `detectMode` + * below — it is a security question, not a formatting one. * * ## Handle format * id — `sec_` + 32 hex chars (122 bits of entropy) @@ -129,8 +131,59 @@ export interface LocalCryptoProviderOptions { const processEnv = (): EnvMap => ((globalThis as { process?: { env?: EnvMap } }).process?.env ?? {}) as EnvMap; +/** + * Deployment posture — read from `NODE_ENV` and from NOTHING ELSE. + * + * ## ⛔ Never widen this to a test-RUNNER variable + * + * This function decides whether the fail-loud guarantee documented above is + * ARMED. `'test'` is not a softer flavour of `'production'`: it is the branch + * that takes an ephemeral key, never touches disk, and — the part that matters + * — never refuses to boot. **The refusal is the gate.** So a variable that can + * reach this function decides whether a security gate runs, and the only + * variables allowed to do that are the ones that describe the DEPLOYMENT. + * + * This line used to read: + * + * if (env.VITEST || env.NODE_ENV === 'test') return 'test'; + * + * `VITEST` describes the RUNNER, not the deployment, and a runner variable is + * INHERITED by every process the runner spawns. Measured on this repo: a real + * `os serve` spawned from a vitest worker with `{ ...process.env }` carried + * `VITEST=true` into the child, so that boot's crypto layer sat in `test` mode + * — ephemeral key, no disk, no refusal — while the rest of the boot was in + * production posture. `packages/cli/test/serve-node-env-production-default` + * `.e2e.test.ts`, a pin whose entire subject is *"unset `NODE_ENV` means + * production"*, ran that way for its whole life: production for auth, test for + * crypto. Nothing said a word, because a gate that does not run prints nothing. + * + * ## Why deleting it does not move in-process unit tests + * + * Reading `VITEST` was intentional: in-process unit tests must get `test` + * posture so they neither mint a key file in `$HOME` nor fail on a machine + * without one. That intent is preserved here rather than dropped, because + * vitest sets BOTH variables on the same worker — measured in vitest 4.1.10's + * own source, `prepareVitest()`: + * + * process.env.TEST = "true"; + * process.env.VITEST = "true"; + * process.env.NODE_ENV ??= "test"; + * + * and it repeats `NODE_ENV: process.env.NODE_ENV || "test"` in the env it hands + * each worker. So an in-process test already satisfies `NODE_ENV === 'test'` + * and lands on this function's first line without `VITEST` participating at + * all. The two spellings are indistinguishable IN-PROCESS and differ only for + * an INHERITING child — which is precisely the defect. `local-crypto-provider` + * `.test.ts` pins both halves: the in-process posture (a real worker, real + * `process.env`, no injected map) and the refusal under a leaked runner + * variable. + * + * `pnpm check:runner-env-posture` keeps the whole class shut, so the next + * author who reaches for a runner variable in product code is told here rather + * than by an operator whose secrets stopped decrypting. + */ const detectMode = (env: EnvMap): CryptoMode => { - if (env.VITEST || env.NODE_ENV === 'test') return 'test'; + if (env.NODE_ENV === 'test') return 'test'; if (env.NODE_ENV === 'production') return 'production'; return 'development'; }; From f02a30525989192bc0bcab83181a4472ea09c468 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 18:53:27 +0000 Subject: [PATCH 2/4] feat(scripts): gate product source against test-runner environment reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the class the crypto-posture defect belonged to rather than only its one member. `check:runner-env-posture` scans every `src` tree under packages/, apps/ and examples/ and refuses `TEST`, `VITEST`/`VITEST_*` and `JEST_WORKER_ID`. `NODE_ENV` is deliberately not banned: it describes the DEPLOYMENT, and a deployment may declare itself a test deployment — a runner may not declare it on the deployment's behalf. Two variables, two subsystems, one week, the same shape: `TEST` leaking into better-auth's origin check, `VITEST` leaking into crypto posture. Nothing mechanical stops the third. Comments and string/template/regex literals are masked through the shared `js-comment-mask.mjs`, so the fixed file's header can keep QUOTING the banned line — deleting the explanation is how a defect like this comes back. The bracket pass exists because the self-test caught its absence: with literals masked, `env['VITEST']` vanished and the gate reported a confident zero about the first spelling an author would reach for if the dotted one were rejected. An array literal naming the family (what the code that STRIPS these variables must do) is still not a read. Verified both directions on real source, not only on self-test fixtures: restoring the pre-fix line to `local-crypto-provider.ts` makes the gate exit 1 naming `local-crypto-provider.ts:186 VITEST` — one finding, and none of the six prose mentions of `VITEST` in that file's own header. Part of #11352 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- .github/workflows/lint.yml | 14 ++ package.json | 1 + scripts/check-runner-env-posture.mjs | 316 +++++++++++++++++++++++++++ 3 files changed, 331 insertions(+) create mode 100644 scripts/check-runner-env-posture.mjs diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index a993b48c15..4e7b0fa2c7 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1198,6 +1198,20 @@ jobs: - name: Single authz resolver guard run: pnpm check:authz-resolver + # #11352. Product code may not read a test-RUNNER variable (`TEST`, + # `VITEST*`, `JEST_WORKER_ID`). Runner variables are INHERITED by every + # process the runner spawns, so a product decision keyed off one is made + # for spawned SERVERS too: `local-crypto-provider.ts` selected its crypto + # posture with `if (env.VITEST || ...)`, and `'test'` posture is the branch + # that never refuses to boot without a stable key. A real `os serve` + # spawned from a vitest worker therefore ran with production auth and test + # crypto — invisibly, because a gate that does not run prints nothing. + # `NODE_ENV` is deliberately NOT banned: it describes the deployment. + # Dependency-free source scan over `**/src/**`, comments and literals + # masked; runs its own --self-test first. + - name: Runner-env posture guard + run: pnpm check:runner-env-posture + # #4093 follow-up. Discovery tells a consumer an absent capability is # absent AND what to install. The first half has been carefully honest # since #2462/#4000; the second was invented from the slot name, so ten diff --git a/package.json b/package.json index dccceb164f..4b3b26810c 100644 --- a/package.json +++ b/package.json @@ -67,6 +67,7 @@ "check:adr-links": "node scripts/check-adr-links.mjs --self-test && node scripts/check-adr-links.mjs", "check:platform-checklist": "node scripts/checklist-select.mjs --self-test && node scripts/check-platform-checklist.mjs", "check:org-identifier": "node scripts/check-org-identifier.mjs --self-test && node scripts/check-org-identifier.mjs", + "check:runner-env-posture": "node scripts/check-runner-env-posture.mjs --self-test && node scripts/check-runner-env-posture.mjs", "check:authz-resolver": "node scripts/check-single-authz-resolver.mjs --self-test && node scripts/check-single-authz-resolver.mjs", "check:slot-lookup": "node scripts/check-slot-lookup-ratchet.mjs", "check:query-options-erasure": "node scripts/check-query-options-erasure-ratchet.mjs --self-test && node scripts/check-query-options-erasure-ratchet.mjs", diff --git a/scripts/check-runner-env-posture.mjs b/scripts/check-runner-env-posture.mjs new file mode 100644 index 0000000000..7deeb18aa0 --- /dev/null +++ b/scripts/check-runner-env-posture.mjs @@ -0,0 +1,316 @@ +#!/usr/bin/env node +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * check-runner-env-posture -- product source may not read a test-RUNNER + * environment variable. + * + * node scripts/check-runner-env-posture.mjs # scan the tree + * node scripts/check-runner-env-posture.mjs --self-test # verify the checker + * + * ## The defect this exists for, measured + * + * `packages/services/service-settings/src/local-crypto-provider.ts` selected + * its crypto posture like this: + * + * if (env.VITEST || env.NODE_ENV === 'test') return 'test'; + * + * `'test'` there is not a softer flavour of `'production'`. It is the branch + * that takes an ephemeral key, never touches disk, and **never refuses to + * boot** -- and the refusal is the whole point of that class. So one runner + * variable decided whether a security gate ran. + * + * Runner variables are INHERITED. Vitest sets `TEST`, `VITEST`, `VITEST_MODE`, + * `VITEST_WORKER_ID` and `VITEST_POOL_ID` on its worker, and every process that + * worker spawns with `{ ...process.env }` receives them. A real `os serve` + * spawned that way therefore booted with production auth and TEST crypto: + * `packages/cli/test/serve-node-env-production-default.e2e.test.ts`, a pin + * whose entire subject is *"unset `NODE_ENV` means production"*, ran that way + * for its whole life. Nothing said a word -- a gate that does not run prints + * nothing -- and it was found only incidentally, while closing the sibling + * `TEST` leak into better-auth's origin check one layer down. + * + * Two variables, two subsystems, one week, same shape. That is a CLASS, and a + * class is what a gate is for. + * + * ## What is banned, and what deliberately is not + * + * Banned: the identifiers that say *a test runner is present* -- `TEST` + * exactly, `VITEST` and anything `VITEST_`-prefixed, and `JEST_WORKER_ID`. They + * describe the RUNNER. + * + * Not banned: `NODE_ENV`, including `NODE_ENV === 'test'`. That describes the + * DEPLOYMENT, it is this repo's one established environment source (Prime + * Directive #9 lists it as a third-party exception, and `seed-loader.ts` and + * `discovery.zod.ts` both fold it), and it is what a deployment sets on + * purpose. The distinction is the entire rule: a deployment may declare itself + * a test deployment; a runner may not declare it on the deployment's behalf. + * + * This is also why the fix for the defect above was to DELETE the `VITEST` + * read rather than to narrow it. In-process unit tests still get test posture, + * because vitest sets both variables on the same worker + * (`prepareVitest()`: `process.env.VITEST = "true"; process.env.NODE_ENV ??= + * "test";`). In-process the two spellings are indistinguishable; they differ + * only for an inheriting child, which is the defect. + * + * ## Population: every `src` tree, and nothing outside one + * + * Product source lives under `src/`. A test that reads `VITEST` is doing its + * job -- `packages/cli/test/helpers/serve-process.ts` names the whole family in + * order to STRIP it, and `examples/app-showcase/test/` does the same. Scanning + * them would force an allowlist, and an allowlist is a hole the next real + * defect falls through quietly. So the population is the tree where the rule + * has no exceptions, and files that are tests by name are dropped even there. + * + * ## Comments and literals are masked + * + * Through `scripts/js-comment-mask.mjs`, for the reason `check-parse-guard.mjs` + * gives for the same choice: the file this gate was written for now carries a + * long comment QUOTING the banned line, and a gate that cannot tell prose from + * code would either flag that documentation or force it to be deleted -- and + * deleting the explanation is how the next author re-introduces the defect. + */ + +import { readFileSync, readdirSync, statSync, existsSync } from 'node:fs'; +import { join, relative, resolve, sep } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { scanSource, blank } from './js-comment-mask.mjs'; +import { isEntrypoint } from './invoked-as.mjs'; + +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const ROOT = resolve(HERE, '..'); + +/** Trees that can hold product source. */ +export const SCANNED_ROOTS = ['packages', 'apps', 'examples']; + +/** Directory names never descended into. */ +const SKIP_DIRS = new Set(['node_modules', 'dist', 'build', '.turbo', '.next', 'coverage', '.git']); + +const SOURCE_EXT = /\.(m?[jt]sx?|cts)$/; + +/** A file that is a test by name or by the directory it sits in. */ +export function isTestFile(relPath) { + const p = relPath.split(sep).join('/'); + if (/\.(test|spec|e2e|pin|bench)\.[^/]+$/.test(p)) return true; + if (/\.(e2e|pin)\.(test|spec)\.[^/]+$/.test(p)) return true; + return /(^|\/)(__tests__|__mocks__|__fixtures__|fixtures|test|tests)\//.test(p); +} + +/** Product source: under a `src/` segment, not a test, not generated. */ +export function isProductSource(relPath) { + const p = relPath.split(sep).join('/'); + if (!SOURCE_EXT.test(p)) return false; + if (!/(^|\/)src\//.test(p)) return false; + if (p.endsWith('.d.ts')) return false; + return !isTestFile(p); +} + +/** + * The runner-variable class. `TEST` is spelled exactly; `VITEST` covers the + * whole prefixed namespace, so a variable vitest adds tomorrow is already in. + */ +export const RUNNER_ENV_PATTERN = /\b(TEST|VITEST(?:_[A-Z0-9_]+)?|JEST_WORKER_ID)\b/g; + +/** + * A BRACKET access whose key is a quoted runner token — `env['VITEST']`. + * + * This needs its own pass, and the self-test is why it exists: the first pass + * runs over source with string literals blanked, so `env['VITEST']` vanished + * from it entirely and the gate reported a confident zero about the one + * spelling an author would reach for FIRST if the dotted one were rejected. + * + * The character before `[` is required to be an identifier tail or a closing + * paren/bracket, which is what separates an INDEX from an array literal: + * `env['VITEST']` matches, `const keys = ['VITEST']` does not. That matters — + * naming the family in an array is exactly what the code that STRIPS these + * variables has to do. + */ +export const RUNNER_ENV_BRACKET_PATTERN = + /[A-Za-z0-9_$\])]\s*(?:\?\.)?\[\s*(['"`])(TEST|VITEST(?:_[A-Z0-9_]+)?|JEST_WORKER_ID)\1\s*\]/g; + +/** + * Findings in one file's source text. + * + * Two passes, because the maskings a text scan needs pull in opposite + * directions. Pass 1 blanks comments AND literals — prose and payloads that + * merely NAME a variable are not reads — and catches every unquoted spelling + * (`env.VITEST`, a destructure, an optional chain). Pass 2 blanks comments + * only and looks for indexing syntax, because the quoted key of a bracket + * access IS the read and pass 1 has just erased it. + * + * Offsets are preserved by both maskings, so a reported line number still + * points at the real line. + */ +export function findRunnerEnvReads(source) { + const flags = scanSource(source); + + const commentMasked = blank(source, flags.comment); + const bothMasked = blank(commentMasked, flags.literal); + + const seen = new Set(); + const out = []; + const lineAt = (index) => source.slice(0, index).split('\n').length; + const push = (index, token) => { + const line = lineAt(index); + const key = `${line}:${token}`; + if (seen.has(key)) return; + seen.add(key); + out.push({ line, token }); + }; + + RUNNER_ENV_PATTERN.lastIndex = 0; + let m; + while ((m = RUNNER_ENV_PATTERN.exec(bothMasked)) !== null) push(m.index, m[1]); + + RUNNER_ENV_BRACKET_PATTERN.lastIndex = 0; + let b; + while ((b = RUNNER_ENV_BRACKET_PATTERN.exec(commentMasked)) !== null) push(b.index, b[2]); + + out.sort((x, y) => x.line - y.line); + return out; +} + +function walk(dir, acc) { + let entries; + try { + entries = readdirSync(dir, { withFileTypes: true }); + } catch { + return acc; + } + for (const e of entries) { + if (e.name.startsWith('.') && e.name !== '.') continue; + const full = join(dir, e.name); + if (e.isDirectory()) { + if (SKIP_DIRS.has(e.name)) continue; + walk(full, acc); + } else if (e.isFile()) { + const rel = relative(ROOT, full); + if (isProductSource(rel)) acc.push(rel); + } + } + return acc; +} + +export function collectFiles(root = ROOT) { + const acc = []; + for (const r of SCANNED_ROOTS) { + const dir = join(root, r); + if (existsSync(dir) && statSync(dir).isDirectory()) walk(dir, acc); + } + return acc.sort(); +} + +export function scanTree(root = ROOT) { + const findings = []; + for (const rel of collectFiles(root)) { + const source = readFileSync(join(root, rel), 'utf8'); + for (const hit of findRunnerEnvReads(source)) findings.push({ file: rel, ...hit }); + } + return findings; +} + +function report(findings, fileCount) { + if (findings.length === 0) { + console.log(`✓ check-runner-env-posture: ${fileCount} product source file(s), no test-runner variable read.`); + return 0; + } + console.error('✗ check-runner-env-posture: product source reads a test-RUNNER environment variable.\n'); + for (const f of findings) console.error(` ${f.file}:${f.line} ${f.token}`); + console.error( + '\n A runner variable is INHERITED by every process the runner spawns, so a product\n' + + ' decision keyed off one is made for spawned servers too — and the measured result\n' + + ' was a security gate that silently stopped running (see this script\'s header).\n\n' + + ' Key the decision off the DEPLOYMENT instead: `NODE_ENV` is this repo\'s one\n' + + ' environment source, and vitest sets `NODE_ENV=test` on the same worker it sets\n' + + ' `VITEST` on, so in-process tests keep their posture without the runner voting.\n\n' + + ' A genuinely new case is an edit to this gate plus a --self-test case, never an\n' + + ' allowlist entry.', + ); + return 1; +} + +// --------------------------------------------------------------------------- +// Self-test — the shapes, not today's corpus +// --------------------------------------------------------------------------- + +export function selfTest() { + const cases = []; + const t = (name, actual, expected) => cases.push([name, actual, expected]); + const tokens = (src) => findRunnerEnvReads(src).map((h) => h.token); + + // --- Detection: the spellings an author actually reaches for. + t('member access', tokens('if (env.VITEST) return 1;'), ['VITEST']); + t('process.env member', tokens('const x = process.env.VITEST;'), ['VITEST']); + t('optional chain', tokens('const x = process?.env?.VITEST;'), ['VITEST']); + t('bracket access', tokens("const x = env['VITEST_WORKER_ID'];"), ['VITEST_WORKER_ID']); + t('bracket access, double-quoted', tokens('const x = env["TEST"];'), ['TEST']); + t('bracket access through an optional chain', tokens("const x = process.env?.['VITEST'];"), ['VITEST']); + t('an ARRAY literal naming the family is not a read', tokens("const keys = ['VITEST', 'TEST'];"), []); + t('a bracket read is reported once, not twice', findRunnerEnvReads("env['VITEST'];").length, 1); + t('destructure', tokens('const { VITEST, HOME } = process.env;'), ['VITEST']); + t('bare TEST', tokens('if (env.TEST) return 1;'), ['TEST']); + t('jest worker', tokens('if (env.JEST_WORKER_ID) return 1;'), ['JEST_WORKER_ID']); + t('the whole VITEST namespace, not a fixed list', tokens('env.VITEST_SOMETHING_NEW;'), ['VITEST_SOMETHING_NEW']); + t('two reads on one line are both reported', tokens('env.VITEST || env.TEST;'), ['VITEST', 'TEST']); + + // --- The line number points at the real line, offsets preserved by the mask. + t('line number survives masking', findRunnerEnvReads('// x\n/* y */\nif (env.VITEST) {}').map((h) => h.line), [3]); + + // --- NOT flagged: the deployment signal, which is the whole point. + t('NODE_ENV is not a runner variable', tokens("if (env.NODE_ENV === 'test') return 1;"), []); + t("the string 'test' is not the token TEST", tokens("if (mode === 'test') return 1;"), []); + + // --- NOT flagged: prose and payloads. A gate that cannot tell them apart + // forces the explanation to be deleted, which is how this comes back. + t('a line comment quoting the banned line', tokens('// if (env.VITEST || x) return 1;'), []); + t('a block comment quoting it', tokens('/**\n * if (env.VITEST) return 1;\n */\nconst a = 1;'), []); + t('a string payload naming it', tokens("const s = 'VITEST';"), []); + t('a template payload naming it', tokens('const s = `TEST=${x}`;'), []); + + // --- Longer identifiers must not be split by the word boundaries. + t('MANIFEST is not TEST', tokens('const MANIFEST = 1;'), []); + t('TEST_TIMEOUT is not TEST', tokens('const TEST_TIMEOUT = 1;'), []); + t('LATEST is not TEST', tokens('const LATEST = 1;'), []); + + // --- Population. + t('product source counts', isProductSource('packages/services/service-settings/src/local-crypto-provider.ts'), true); + t('a unit test beside it does not', isProductSource('packages/services/service-settings/src/local-crypto-provider.test.ts'), false); + t('an e2e in a test dir does not', isProductSource('packages/cli/test/helpers/serve-process.ts'), false); + t('an example test dir does not', isProductSource('examples/app-showcase/test/vitest-console-teardown-race.test.ts'), false); + t('a d.ts does not', isProductSource('examples/app-showcase/src/types.d.ts'), false); + t('a repo script outside src/ does not', isProductSource('scripts/check-runner-env-posture.mjs'), false); + t('a package script outside src/ does not', isProductSource('packages/spec/scripts/build-schemas.mjs'), false); + + // --- Wiring. Unwiring the gate must redden HERE rather than go quiet. + const pkg = JSON.parse(readFileSync(join(ROOT, 'package.json'), 'utf8')); + t('a package.json alias invokes this script', /check-runner-env-posture\.mjs/.test(pkg.scripts?.['check:runner-env-posture'] ?? ''), true); + t('...and runs the self-test with it', /--self-test/.test(pkg.scripts?.['check:runner-env-posture'] ?? ''), true); + const lintYml = readFileSync(join(ROOT, '.github/workflows/lint.yml'), 'utf8'); + t('a lint job runs the alias', lintYml.includes('pnpm check:runner-env-posture'), true); + + // --- The corpus itself, as a case rather than as the run's only evidence. + t('today\'s tree is clean', scanTree().length, 0); + + let failed = 0; + for (const [name, actual, expected] of cases) { + const ok = JSON.stringify(actual) === JSON.stringify(expected); + if (!ok) failed++; + console.log(` ${ok ? '✓' : '✗'} ${name}${ok ? '' : ` (got ${JSON.stringify(actual)}, want ${JSON.stringify(expected)})`}`); + } + if (failed) { + console.error(`✗ check-runner-env-posture self-test: ${failed} of ${cases.length} case(s) failed.`); + return 1; + } + console.log(`✓ check-runner-env-posture self-test: ${cases.length} cases pass.`); + return 0; +} + +if (isEntrypoint(import.meta.url)) { + if (process.argv.includes('--self-test')) { + process.exit(selfTest()); + } else { + const files = collectFiles(); + process.exit(report(scanTree(), files.length)); + } +} From c3b2e7a5d38cd1f6cfda7cbc9d269a5a5e1b5bcc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 19:16:51 +0000 Subject: [PATCH 3/4] test(service-settings): annotate the production-posture env map as EnvMap `pnpm --filter @objectstack/service-settings typecheck` reported TS2339 "Property 'VITEST' does not exist on type '{ NODE_ENV: string; }'": spreading the `EnvMap`-typed base into an unannotated object literal dropped the index signature, so the anti-vacuity assertion that the leak is still in the map did not compile. Annotating the literal keeps it. Also adds the changeset for the behaviour change. Part of #11352 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- .../crypto-posture-deployment-signal.md | 25 +++++++++++++++++++ .../crypto-posture-deployment-signal.test.ts | 2 +- 2 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 .changeset/crypto-posture-deployment-signal.md diff --git a/.changeset/crypto-posture-deployment-signal.md b/.changeset/crypto-posture-deployment-signal.md new file mode 100644 index 0000000000..b6ea4dcd68 --- /dev/null +++ b/.changeset/crypto-posture-deployment-signal.md @@ -0,0 +1,25 @@ +--- +"@objectstack/service-settings": minor +--- + +**Security:** `LocalCryptoProvider` selects its crypto posture from the deployment signal only. A test-runner variable inherited by a spawned server can no longer disarm the production key refusal (#11352). + +`detectMode` read `env.VITEST` as a vote for `'test'` posture: + +```ts +if (env.VITEST || env.NODE_ENV === 'test') return 'test'; +``` + +`'test'` is not a softer flavour of `'production'`. It is the branch that takes an ephemeral key, never touches disk, and **never refuses to boot** — and that refusal is the reason the class exists: minting a key at boot makes every previously-written `sys_secret` value (encrypted settings, `secret` fields, datasource credentials) undecryptable after the next restart or on another node, invisibly at encrypt time. So one runner variable decided whether a security gate ran at all. + +Runner variables are **inherited**. Vitest sets `TEST`, `VITEST`, `VITEST_MODE`, `VITEST_WORKER_ID` and `VITEST_POOL_ID` on its worker, and every process that worker spawns with `{ ...process.env }` receives them. Measured on this repo: a real `os serve` spawned that way booted with production auth and **test** crypto. `packages/cli/test/serve-node-env-production-default.e2e.test.ts` — a pin whose entire subject is *"unset `NODE_ENV` means production"* — ran that way for its whole life, and nothing said a word, because a gate that does not run prints nothing. It surfaced only incidentally, while closing the sibling `TEST` leak into better-auth's origin check one layer down. + +**What changes for you.** A process that boots with `NODE_ENV=production`, no `OS_SECRET_KEY`/`OS_DEV_CRYPTO_KEY`, no persisted key file, no `OS_CRYPTO_AUTOKEY` — and a runner variable in its environment — now **refuses to start** instead of running on an ephemeral key. That is the documented fail-loud guarantee arriving where it was previously skipped, not a new restriction: supply the key the refusal names. + +``` +OS_SECRET_KEY=$(openssl rand -hex 32) +``` + +**What does not change.** In-process unit tests still get `test` posture — ephemeral key, disk never touched. The `VITEST` read is deleted rather than narrowed because vitest sets both variables on the same worker (`prepareVitest()`: `process.env.VITEST = "true"; process.env.NODE_ENV ??= "test";`, repeated as `NODE_ENV: process.env.NODE_ENV || "test"` in each worker's env). In-process the two spellings are indistinguishable; they differ only for an **inheriting child**, which is precisely the defect. `NODE_ENV` remains the one signal, and a deployment that declares itself a test deployment still gets test posture. + +The whole class is now gated: `pnpm check:runner-env-posture` refuses `TEST`, `VITEST`/`VITEST_*` and `JEST_WORKER_ID` anywhere in product source, so the next author is told at authoring time rather than by an operator whose secrets stopped decrypting. `NODE_ENV` is deliberately not banned — it describes the deployment, and a deployment may declare itself a test deployment; a runner may not declare it on the deployment's behalf. diff --git a/packages/services/service-settings/src/crypto-posture-deployment-signal.test.ts b/packages/services/service-settings/src/crypto-posture-deployment-signal.test.ts index 1624dee4db..b3dd8266c8 100644 --- a/packages/services/service-settings/src/crypto-posture-deployment-signal.test.ts +++ b/packages/services/service-settings/src/crypto-posture-deployment-signal.test.ts @@ -117,7 +117,7 @@ describe('#11352 — crypto posture reads the deployment signal, not the runner' // runner variables included — with the deployment signal set to what a real // `os serve` deployment carries. Before #11352 the inherited `VITEST=true` // won this decision and the boot SUCCEEDED on an ephemeral key. - const env = { ...base, NODE_ENV: 'production' }; + const env: EnvMap = { ...base, NODE_ENV: 'production' }; expect(env.VITEST, 'the leak is still in the map — that is the point').toBeDefined(); expect(() => new LocalCryptoProvider({ env })).toThrow(/Refusing to start in production/); From 1b86bcb713a6d1c8546a981cc8cfeaefefa0f7b8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 20:11:09 +0000 Subject: [PATCH 4/4] chore(pm): record the bare-root verdict check:runner-env-posture owes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bare-root-worklist --self-test` reddened on this branch: the new gate's SCANNED_ROOTS holds three bare single-segment words, so it joined the invisible bare-root species unjudged. That meta-gate is asking exactly the question this card is about — a guard that quietly stops guarding — so the verdict is recorded rather than the question silenced. REFUSE-UNSPELLABLE on all three roots, measured, not estimated: packages 1757 of 5049 (35%) examples 150 of 240 (63%) apps 0 of 35 (0%) Unspellable rather than merely wide, and the distinction is the `src` SEGMENT: the true population is `packages/**/src/**`, and `collapseHint` reduces that to `packages`. So the only declaration the idiom can express also claims every manifest, changelog, fixture and the 2658 test files this gate deliberately skips. Its nearest neighbour `check:authz-resolver` is REFUSE-WIDE at a similar 39% because ITS population really is every non-test source under the root; this one is not. The apps leg is the load-bearing one: at 0 of 35 a subtree declaration would not be imprecise but FALSE — pasting this gate into every apps card to reach nothing. `apps` stays in SCANNED_ROOTS deliberately so an apps package that grows a src tree is covered the day it lands. Data only: pure insertion, no logic in the meta-gate touched, and `check:runner-env-posture` itself is unchanged — it is the deliverable, not the thing to narrow. node scripts/pm/bare-root-worklist.mjs --self-test OK self-test: 37 live row(s), 34 unreachable as spelled, 34 recorded verdict(s) — none stale, none missing. Part of #11352 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4 --- scripts/pm/bare-root-worklist.mjs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/scripts/pm/bare-root-worklist.mjs b/scripts/pm/bare-root-worklist.mjs index 2b3f493570..0257665799 100644 --- a/scripts/pm/bare-root-worklist.mjs +++ b/scripts/pm/bare-root-worklist.mjs @@ -224,6 +224,29 @@ const TRIAGE = new Map([ why: 'one named file per child directory, 11 of 50 (22%). It already reaches its own cards ' + 'through the artifact roster it names file by file, so the miss is smaller than the row', }], + ['check:runner-env-posture SCANNED_ROOTS packages', { + verdict: 'REFUSE-UNSPELLABLE', + why: 'non-test source beneath a `src` SEGMENT — 1757 of 5049 (35%). The segment is what makes ' + + 'this unspellable rather than merely wide: `packages/**/src/**` is the true population and ' + + 'collapseHint reduces it to `packages`, so the only spellable claim also names every ' + + 'package manifest, changelog, fixture and the 2658 test files this gate deliberately skips. ' + + 'Its nearest neighbour check:authz-resolver is REFUSE-WIDE at a similar 39% because ITS ' + + 'population really is every non-test source under the root; this one is not', + }], + ['check:runner-env-posture SCANNED_ROOTS examples', { + verdict: 'REFUSE-UNSPELLABLE', + why: '150 of 240 (63%), the same `src`-segment filter, refused with its packages half rather ' + + 'than split: declaring the smaller root would name the gate on example cards and stay ' + + 'silent on the package cards where product source actually lives', + }], + ['check:runner-env-posture SCANNED_ROOTS apps', { + verdict: 'REFUSE-UNSPELLABLE', + why: 'MEASURED AT ZERO — 0 of 35. No `src` tree exists under this root today, so a subtree ' + + 'declaration here would not be imprecise but false: it would paste this gate into every ' + + 'apps card to reach nothing. The root stays in SCANNED_ROOTS deliberately, so an apps ' + + 'package that grows a src tree is covered the day it lands rather than the day someone ' + + 'remembers — which is the same silent-coverage-loss this gate exists to prevent', + }], ['check:changeset-gate-self-tests PACKAGE_ROOTS packages', { verdict: 'REFUSE-UNSPELLABLE', why: 'workspace manifests only — 73 of 4903 (1.5%)',