From f350959f1746dc40fe6e73fc04b22544247799bc Mon Sep 17 00:00:00 2001 From: Theo Ephraim Date: Sun, 13 Sep 2026 22:24:38 -0700 Subject: [PATCH 1/8] fix(varlock): defer process.env typing when another .d.ts declares ProcessEnv wrangler types writes a NodeJS.ProcessEnv augmentation into worker-configuration.d.ts. TypeScript merges all declarations of that interface and requires every shared key to be identical, so ours could never coexist with it: an optional item alone (FOO?: string vs FOO: string) fails with TS2320, as do enums and booleans. Default processEnv to none when another .d.ts next to the schema (or the generated file) already declares ProcessEnv, and note which file caused it at the top of the generated output. Detecting the declaration rather than the platform covers infra-as-code setups with no wrangler config, and misses nothing that was actually conflicting. Explicit processEnv= still wins. --- .bumpy/process-env-augmentation-conflict.md | 5 + .../content/docs/integrations/cloudflare.mdx | 12 +++ .../docs/reference/root-decorators.mdx | 2 + packages/varlock/src/env-graph/index.ts | 1 + .../env-graph/lib/type-generation/banners.ts | 5 +- .../lib/type-generation/code-generators.ts | 23 +++- .../detect-process-env-augmentation.ts | 90 ++++++++++++++++ .../lib/type-generation/emitters/ts.ts | 12 +++ .../env-graph/lib/type-generation/index.ts | 1 + .../env-graph/test/type-generation.test.ts | 102 +++++++++++++++++- 10 files changed, 250 insertions(+), 3 deletions(-) create mode 100644 .bumpy/process-env-augmentation-conflict.md create mode 100644 packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts diff --git a/.bumpy/process-env-augmentation-conflict.md b/.bumpy/process-env-augmentation-conflict.md new file mode 100644 index 000000000..c33bd873f --- /dev/null +++ b/.bumpy/process-env-augmentation-conflict.md @@ -0,0 +1,5 @@ +--- +varlock: patch +--- + +Skip the process.env type augmentation when another .d.ts (e.g. wrangler's worker-configuration.d.ts) already declares NodeJS.ProcessEnv, which previously caused a TS2320 conflict. Set processEnv=strict on @generateTsTypes to override. diff --git a/packages/varlock-website/src/content/docs/integrations/cloudflare.mdx b/packages/varlock-website/src/content/docs/integrations/cloudflare.mdx index 2adf03cc6..7ea1be896 100644 --- a/packages/varlock-website/src/content/docs/integrations/cloudflare.mdx +++ b/packages/varlock-website/src/content/docs/integrations/cloudflare.mdx @@ -254,6 +254,18 @@ Some framework templates split their TypeScript setup into multiple referenced c This types the `ENV` object. For the native `env` parameter in your fetch handler, use [`varlock-wrangler types`](#varlock-wrangler-types) to generate an `Env` interface that includes your varlock-managed vars alongside your other Cloudflare bindings. +### `process.env` is typed by wrangler, not varlock + +`wrangler types` writes its own `NodeJS.ProcessEnv` augmentation into `worker-configuration.d.ts`, covering every var and secret it knows about. TypeScript merges all declarations of that interface and requires every shared key to be identical across them, so varlock's augmentation and wrangler's cannot both apply: a single optional item (`FOO?: string` against wrangler's `FOO: string`) fails with `TS2320`, as do enums and booleans, which varlock types as literal unions. + +When varlock sees an existing `NodeJS.ProcessEnv` declaration next to your schema, it defers and skips its own, noting why at the top of the generated file. Nothing is lost if you generate types with [`varlock-wrangler types`](#varlock-wrangler-types): it feeds your schema's keys to wrangler, so all of them are typed on `process.env` as strings. Use `ENV` where you want the coerced types (enums, booleans, numbers) and the docs from your schema. + +Setting `processEnv=strict` on [`@generateTsTypes`](/reference/root-decorators/#generatetstypes) opts back in, but only do that if nothing else is declaring `NodeJS.ProcessEnv`: with wrangler's block present, the two conflict and TypeScript reports `TS2320`. + +:::caution[With `skipLibCheck: true` the conflict is silent] +Most Worker templates enable `skipLibCheck`, which suppresses `TS2320` in `.d.ts` files entirely. The two declarations still clash, and whichever file TypeScript reaches first wins, so a key can silently take wrangler's literal type from `wrangler.jsonc` instead of your schema's. Deferring avoids that as well. +::: + ---- ## Log redaction and leak prevention diff --git a/packages/varlock-website/src/content/docs/reference/root-decorators.mdx b/packages/varlock-website/src/content/docs/reference/root-decorators.mdx index 227d9f7f2..57aeea6ff 100644 --- a/packages/varlock-website/src/content/docs/reference/root-decorators.mdx +++ b/packages/varlock-website/src/content/docs/reference/root-decorators.mdx @@ -654,6 +654,8 @@ TypeScript type declarations. Makes `import { ENV } from 'varlock/env'` typed an - `local`: export a package-local typed `ENV` from the generated file (with **no** global augmentation, since `processEnv`/`importMetaEnv` default to `none` too, so nothing merges across packages). Use this in monorepos where multiple packages have different schemas. Requires a `.ts` output path (not `.d.ts`, since it contains a runtime re-export), and you import from the generated file (e.g. `import { ENV } from './env'`). - `none`: emit only the type definitions, no `ENV` binding. - `processEnv`: how `process.env` is typed: `strict`, `loose`, or `none` (defaults to `strict`; to `none` when `exposeEnv=local`, or when [`@disableProcessEnvInjection`](#disableprocessenvinjection) is set, since values aren't put on `process.env` then). Note that `@types/node` declares a base string index signature that can't be removed, so extra keys remain allowed on `process.env` regardless. + + It also defaults to `none` when another `.d.ts` sitting next to your schema (or next to the generated file) already declares `NodeJS.ProcessEnv`. The common case is `worker-configuration.d.ts`, written by [`wrangler types`](/integrations/cloudflare/#type-safety-and-intellisense). TypeScript merges every declaration of that one interface and requires each shared key to be identical, so two of them can't coexist: an optional item alone (`FOO?: string` vs `FOO: string`) is enough to fail with `TS2320`. The generated file names the file that triggered the skip. Set `processEnv=strict` to emit ours anyway. - `importMetaEnv`: how `import.meta.env` is typed: `strict`, `loose`, or `none` (defaults to `strict`; to `none` when `exposeEnv=local`). ```env-spec diff --git a/packages/varlock/src/env-graph/index.ts b/packages/varlock/src/env-graph/index.ts index c3eb6e878..637f1f3c7 100644 --- a/packages/varlock/src/env-graph/index.ts +++ b/packages/varlock/src/env-graph/index.ts @@ -17,6 +17,7 @@ export { export { builtInCodeGenerators, collectTypeGenItems, + findConflictingProcessEnvAugmentation, generateCsharpEnvSrc, generateGoEnvSrc, generateJavaEnvSrc, diff --git a/packages/varlock/src/env-graph/lib/type-generation/banners.ts b/packages/varlock/src/env-graph/lib/type-generation/banners.ts index c2f9b385b..c2c2f5a7c 100644 --- a/packages/varlock/src/env-graph/lib/type-generation/banners.ts +++ b/packages/varlock/src/env-graph/lib/type-generation/banners.ts @@ -1,8 +1,11 @@ export type TypeGenCommentStyle = '//' | '#'; +/** stable substring of the banner, used to recognize our own generated files */ +export const AUTOGENERATED_FILE_MARKER = 'THIS IS AN AUTOGENERATED FILE'; + const AUTOGENERATED_BANNER_LINES = [ '🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑', - '🛑 THIS IS AN AUTOGENERATED FILE - DO NOT EDIT DIRECTLY 🛑', + `🛑 ${AUTOGENERATED_FILE_MARKER} - DO NOT EDIT DIRECTLY 🛑`, '🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑 🛑', ]; diff --git a/packages/varlock/src/env-graph/lib/type-generation/code-generators.ts b/packages/varlock/src/env-graph/lib/type-generation/code-generators.ts index 5cd4c48d2..d79654791 100644 --- a/packages/varlock/src/env-graph/lib/type-generation/code-generators.ts +++ b/packages/varlock/src/env-graph/lib/type-generation/code-generators.ts @@ -1,3 +1,4 @@ +import path from 'node:path'; import type { EnvGraph } from '../env-graph'; import type { TypeGenItemInfo } from '../config-item'; import { isVarlockReservedKey } from '../reserved-vars'; @@ -8,6 +9,7 @@ import { generatePhpEnvSrc } from './emitters/php'; import { generatePythonEnvSrc } from './emitters/python'; import { generateRustEnvSrc } from './emitters/rust'; import { generateTsTypesSrc } from './emitters/ts'; +import { findConflictingProcessEnvAugmentation } from './detect-process-env-augmentation'; import { type ResolvedFieldType } from './shared'; /** Everything a code generator needs to produce a single output file. */ @@ -67,7 +69,7 @@ const LANG_TO_DECORATOR: Record = { csharp: 'generateCsharpEnv', }; -function generateTsFile(ctx: CodeGenContext): Promise { +async function generateTsFile(ctx: CodeGenContext): Promise { // local mode emits a runtime re-export (TypeScript syntax) — a `.d.ts` has no runtime binding, // and anything else (`.js`, ...) would not parse, so require a real `.ts` module if (ctx.options.exposeEnv === 'local' @@ -83,6 +85,25 @@ function generateTsFile(ctx: CodeGenContext): Promise { if (options.processEnv === undefined && ctx.graph.isProcessEnvInjectionDisabled) { options.processEnv = 'none'; } + // `NodeJS.ProcessEnv` is a single global interface, and merged declarations of it must have + // identical members for every shared key, so ours cannot coexist with another tool's (see + // detect-process-env-augmentation). When something else already declares it, defer rather than + // emit a guaranteed TS2320 - an explicit `processEnv=` still wins. `exposeEnv=local` already + // defaults to `none`, so there is nothing to defer there. + if (options.processEnv === undefined && options.exposeEnv !== 'local') { + const conflictPath = await findConflictingProcessEnvAugmentation({ + dirs: [ctx.sourceDir, path.dirname(ctx.outputPath)], + outputPath: ctx.outputPath, + }); + if (conflictPath) { + options.processEnv = 'none'; + options.processEnvSkipNote = [ + `NOTE: the \`process.env\` augmentation was skipped because ${path.basename(conflictPath)}`, + 'already declares NodeJS.ProcessEnv, and two declarations of it cannot both apply.', + 'Use `ENV` for the full types, or set `processEnv=strict` on @generateTsTypes to override.', + ].join('\n'); + } + } // `@injectUndefinedAsEmpty` means unset items land on process.env as empty strings, so the // process.env augmentation drops its optionality (graph-level flag, not a decorator arg) options.injectUndefinedAsEmpty = ctx.graph.injectUndefinedAsEmpty; diff --git a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts new file mode 100644 index 000000000..247f2e750 --- /dev/null +++ b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts @@ -0,0 +1,90 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { AUTOGENERATED_FILE_MARKER } from './banners'; + +/** + * `NodeJS.ProcessEnv` is a single global interface, so two declarations of it must have + * *identical* members for every shared key or TypeScript errors (TS2320 - "Interface + * 'ProcessEnv' cannot simultaneously extend types ... are not identical"). Our augmentation can + * never match another tool's: an optional item alone (`FOO?: string` vs `FOO: string`) is enough + * to conflict, as are enums and booleans, which we type as literal unions. + * + * The best known example is `wrangler types`, which writes a `NodeJS.ProcessEnv` augmentation + * into `worker-configuration.d.ts` covering every var and secret. Rather than detect Cloudflare + * (or any other platform - infra-as-code setups may not have a wrangler config at all), we look + * for the thing that actually collides: an existing ProcessEnv declaration. + * + * Note the failure bias. Missing a declaration is safe: we keep emitting what we emit today, and + * if nothing else declares ProcessEnv there was no conflict to avoid. Matching too eagerly would + * silently drop typing, so the generated file names the file we found. + */ + +/** + * Only the head of each file is scanned. These declarations sit at the top of generated files + * (wrangler writes env types first, then ~1MB of runtime types after a `// Begin runtime types` + * marker), and codegen runs on every load, so slurping whole files would be wasteful. + */ +const MAX_SCAN_BYTES = 64 * 1024; + +async function readFileHead(filePath: string, maxBytes = MAX_SCAN_BYTES): Promise { + let fileHandle; + try { + fileHandle = await fs.promises.open(filePath, 'r'); + const buffer = Buffer.alloc(maxBytes); + const { bytesRead } = await fileHandle.read(buffer, 0, maxBytes, 0); + return buffer.subarray(0, bytesRead).toString('utf-8'); + } catch { + // unreadable (deleted mid-scan, permissions, ...) - treat as "no declaration found" + return undefined; + } finally { + await fileHandle?.close().catch(() => undefined); + } +} + +/** + * Both signals must be present. Checking for the two names rather than matching a specific shape + * keeps this from coupling to any one tool's output, which we don't control and which changes. + */ +function declaresProcessEnv(src: string): boolean { + return src.includes('namespace NodeJS') && src.includes('interface ProcessEnv'); +} + +/** + * Look for a `.d.ts` alongside the schema (and alongside the generated file, when that lives + * elsewhere) that already augments `NodeJS.ProcessEnv`. + * + * Returns the absolute path of the first match, or undefined. Our own generated files are + * skipped - they carry the autogenerated banner, so a second `@generateTsTypes` writing into the + * same directory doesn't look like a foreign declaration. + */ +export async function findConflictingProcessEnvAugmentation(opts: { + /** directories to scan, non-recursively */ + dirs: Array; + /** absolute path of the file being generated, never treated as a conflict */ + outputPath: string; +}): Promise { + const scanned = new Set(); + for (const dir of opts.dirs) { + const resolvedDir = path.resolve(dir); + if (scanned.has(resolvedDir)) continue; + scanned.add(resolvedDir); + + let entries: Array; + try { + entries = await fs.promises.readdir(resolvedDir); + } catch { + continue; + } + + for (const entry of entries.sort()) { + if (!entry.endsWith('.d.ts')) continue; + const filePath = path.join(resolvedDir, entry); + if (filePath === path.resolve(opts.outputPath)) continue; + const src = await readFileHead(filePath); + if (!src) continue; + if (src.includes(AUTOGENERATED_FILE_MARKER)) continue; + if (declaresProcessEnv(src)) return filePath; + } + } + return undefined; +} diff --git a/packages/varlock/src/env-graph/lib/type-generation/emitters/ts.ts b/packages/varlock/src/env-graph/lib/type-generation/emitters/ts.ts index bad61cc51..9250a2bb4 100644 --- a/packages/varlock/src/env-graph/lib/type-generation/emitters/ts.ts +++ b/packages/varlock/src/env-graph/lib/type-generation/emitters/ts.ts @@ -169,6 +169,12 @@ export type TsGenOptions = { * through it, so a schema key may be absent there regardless of injection mode. */ injectUndefinedAsEmpty?: boolean; + /** + * Set when `processEnv` was defaulted to `none` because another `.d.ts` already declares + * `NodeJS.ProcessEnv` (not a decorator arg). Emitted as a comment in the generated file, so the + * skip is visible where someone would notice it, naming the file found and how to override. + */ + processEnvSkipNote?: string; }; // defaults preserve the historical output: globally augment `varlock/env`, and augment both globals @@ -181,6 +187,7 @@ const DEFAULT_TS_GEN_OPTIONS = { processEnv: 'strict', importMetaEnv: 'strict', injectUndefinedAsEmpty: false, + processEnvSkipNote: '', } satisfies Required; const TS_ENV_EXPOSURE_VALUES: ReadonlyArray = ['global', 'local', 'none']; @@ -207,6 +214,7 @@ function resolveTsGenOptions(options: Record = {}): Required, optio '/* eslint-disable */', ]; + if (opts.processEnvSkipNote) { + tsSrc.push('', ..._.map(opts.processEnvSkipNote.split('\n'), (line) => `// ${line}`)); + } + // `local` exposure re-exports the runtime ENV proxy, so we need to import it if (opts.exposeEnv === 'local') { tsSrc.push("import { ENV as _ENV } from 'varlock/env';", ''); diff --git a/packages/varlock/src/env-graph/lib/type-generation/index.ts b/packages/varlock/src/env-graph/lib/type-generation/index.ts index d09db6607..b0b034830 100644 --- a/packages/varlock/src/env-graph/lib/type-generation/index.ts +++ b/packages/varlock/src/env-graph/lib/type-generation/index.ts @@ -11,6 +11,7 @@ export { type CoercedType, type ResolvedFieldType, } from './shared'; +export { findConflictingProcessEnvAugmentation } from './detect-process-env-augmentation'; export { builtInCodeGenerators, collectTypeGenItems, diff --git a/packages/varlock/src/env-graph/test/type-generation.test.ts b/packages/varlock/src/env-graph/test/type-generation.test.ts index 26da35a71..592f29573 100644 --- a/packages/varlock/src/env-graph/test/type-generation.test.ts +++ b/packages/varlock/src/env-graph/test/type-generation.test.ts @@ -1,5 +1,5 @@ import { - afterEach, describe, expect, test, vi, + afterEach, beforeEach, describe, expect, test, vi, } from 'vitest'; import outdent from 'outdent'; import path from 'node:path'; @@ -7,6 +7,7 @@ import path from 'node:path'; import { EnvGraph, DotEnvFileDataSource, collectTypeGenItems, + findConflictingProcessEnvAugmentation, generateTsTypesSrc, resolveFieldType, resolveFieldTypes, @@ -1428,4 +1429,103 @@ describe('type generation', () => { } }); }); + describe('existing NodeJS.ProcessEnv declarations', () => { + // `NodeJS.ProcessEnv` is one global interface, so a second declaration of it (wrangler writes + // one into worker-configuration.d.ts) conflicts with ours on any shared key that isn't + // *identical*, and an optional item alone is enough. We defer instead of emitting a guaranteed + // TS2320. See detect-process-env-augmentation.ts. + const FOREIGN_DTS = outdent` + declare namespace NodeJS { + interface ProcessEnv extends StringifyValues> {} + } + `; + + let tempDir: string; + let cwdSpy: ReturnType; + + beforeEach(async () => { + const fs = await import('node:fs'); + const os = await import('node:os'); + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'varlock-process-env-augment-')); + cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(tempDir); + }); + afterEach(async () => { + cwdSpy.mockRestore(); + await import('node:fs').then((fs) => fs.promises.rm(tempDir, { recursive: true, force: true })); + }); + + async function writeFile(name: string, contents: string) { + const fs = await import('node:fs'); + await fs.promises.writeFile(path.join(tempDir, name), contents, 'utf-8'); + } + async function generate(decoratorArgs: string) { + const g = new EnvGraph(); + await g.setRootDataSource(new DotEnvFileDataSource('.env.schema', { + overrideContents: outdent` + # @defaultSensitive=false + # @generateTsTypes(${decoratorArgs}) + # --- + SOME_ITEM=val # @optional + `, + })); + await g.finishLoad(); + await g.runCodeGeneratorsIfNeeded(); + const fs = await import('node:fs'); + return fs.promises.readFile(path.join(tempDir, 'env.d.ts'), 'utf-8'); + } + + test('finds a foreign .d.ts that declares ProcessEnv', async () => { + await writeFile('worker-configuration.d.ts', FOREIGN_DTS); + const found = await findConflictingProcessEnvAugmentation({ + dirs: [tempDir], + outputPath: path.join(tempDir, 'env.d.ts'), + }); + expect(found).toBe(path.join(tempDir, 'worker-configuration.d.ts')); + }); + + test('ignores our own generated files, the output path, and non-.d.ts files', async () => { + // a second @generateTsTypes output in the same dir carries our banner, not a foreign decl + await writeFile('other-env.d.ts', await generate('path=env.d.ts')); + await writeFile('env.d.ts', FOREIGN_DTS); + await writeFile('globals.ts', FOREIGN_DTS); + const found = await findConflictingProcessEnvAugmentation({ + dirs: [tempDir], + outputPath: path.join(tempDir, 'env.d.ts'), + }); + expect(found).toBeUndefined(); + }); + + test('a file naming only one of the two markers is not a match', async () => { + await writeFile('partial.d.ts', 'declare namespace NodeJS { interface Process { foo: string } }'); + const found = await findConflictingProcessEnvAugmentation({ + dirs: [tempDir], + outputPath: path.join(tempDir, 'env.d.ts'), + }); + expect(found).toBeUndefined(); + }); + + test('defaults processEnv to none, and says why in the generated file', async () => { + await writeFile('worker-configuration.d.ts', FOREIGN_DTS); + const src = await generate('path=env.d.ts'); + expect(src).not.toContain('namespace NodeJS'); + expect(src).toContain('worker-configuration.d.ts'); + expect(src).toContain('set `processEnv=strict`'); + // only process.env defers; import.meta.env and the ENV types are untouched + expect(src).toContain('interface ImportMetaEnv'); + expect(src).toContain("declare module 'varlock/env'"); + }); + + test('explicit processEnv= wins over the deferral', async () => { + await writeFile('worker-configuration.d.ts', FOREIGN_DTS); + const src = await generate('path=env.d.ts, processEnv=strict'); + expect(src).toContain('namespace NodeJS'); + expect(src).not.toContain('was skipped because'); + }); + + test('emits the augmentation normally when nothing else declares ProcessEnv', async () => { + const src = await generate('path=env.d.ts'); + expect(src).toContain('namespace NodeJS'); + expect(src).not.toContain('was skipped because'); + }); + }); }); From d1da98a59f61494ef100f5c2691891653649146b Mon Sep 17 00:00:00 2001 From: Theo Ephraim Date: Sun, 13 Sep 2026 23:02:10 -0700 Subject: [PATCH 2/8] fix(varlock): scope ProcessEnv detection to the NodeJS namespace The two substring checks did not establish that ProcessEnv was declared inside NodeJS, so `interface ProcessEnvExtra`, a ProcessEnv declared after the namespace block, or a mention in a comment all read as a conflict and would have silently dropped valid process.env typing. Strip comments, then brace-match each `namespace NodeJS {` block and require the interface inside it. --- .../detect-process-env-augmentation.ts | 39 +++++++++++++++++-- .../env-graph/test/type-generation.test.ts | 35 ++++++++++++++++- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts index 247f2e750..21ddb71bc 100644 --- a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts +++ b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts @@ -41,12 +41,43 @@ async function readFileHead(filePath: string, maxBytes = MAX_SCAN_BYTES): Promis } } +/** strip comments so a mention of these declarations in prose never counts as one */ +function stripComments(src: string): string { + return src.replace(/\/\*[\s\S]*?\*\//g, ' ').replace(/\/\/[^\n]*/g, ' '); +} + +const NODEJS_NAMESPACE_OPEN = /\bnamespace\s+NodeJS\s*\{/g; +const PROCESS_ENV_INTERFACE = /\binterface\s+ProcessEnv\b/; + /** - * Both signals must be present. Checking for the two names rather than matching a specific shape - * keeps this from coupling to any one tool's output, which we don't control and which changes. + * True when the source declares `interface ProcessEnv` *within* a `namespace NodeJS` block. Both + * names have to be checked together: `interface ProcessEnvExtra` next to an unrelated + * `namespace NodeJS { interface Process {} }` isn't a conflict, and wrongly treating it as one + * would silently drop typing that was fine. + * + * Deliberately matched by structure rather than by any one tool's output shape, which we don't + * control and which changes: this covers both `declare namespace NodeJS { ... }` (what wrangler + * writes) and the `declare global { namespace NodeJS { ... } }` form we emit ourselves. */ -function declaresProcessEnv(src: string): boolean { - return src.includes('namespace NodeJS') && src.includes('interface ProcessEnv'); +function declaresProcessEnv(rawSrc: string): boolean { + const src = stripComments(rawSrc); + NODEJS_NAMESPACE_OPEN.lastIndex = 0; + let match = NODEJS_NAMESPACE_OPEN.exec(src); + while (match) { + // walk from the namespace's opening brace to its matching close, so a `ProcessEnv` declared + // after the block (or in a sibling one) doesn't count as being inside it + let depth = 1; + let i = match.index + match[0].length; + const bodyStart = i; + while (i < src.length && depth > 0) { + if (src[i] === '{') depth++; + else if (src[i] === '}') depth--; + i++; + } + if (PROCESS_ENV_INTERFACE.test(src.slice(bodyStart, depth === 0 ? i - 1 : undefined))) return true; + match = NODEJS_NAMESPACE_OPEN.exec(src); + } + return false; } /** diff --git a/packages/varlock/src/env-graph/test/type-generation.test.ts b/packages/varlock/src/env-graph/test/type-generation.test.ts index 592f29573..401e2436f 100644 --- a/packages/varlock/src/env-graph/test/type-generation.test.ts +++ b/packages/varlock/src/env-graph/test/type-generation.test.ts @@ -1495,8 +1495,24 @@ describe('type generation', () => { expect(found).toBeUndefined(); }); - test('a file naming only one of the two markers is not a match', async () => { - await writeFile('partial.d.ts', 'declare namespace NodeJS { interface Process { foo: string } }'); + test.each([ + ['only one of the two names is present', 'declare namespace NodeJS { interface Process { foo: string } }'], + // both names appear, but ProcessEnv is not declared inside the NodeJS namespace + [ + 'ProcessEnv is declared outside the namespace', outdent` + declare namespace NodeJS { interface Process { foo: string } } + interface ProcessEnv { UNRELATED: string } + `, + ], + ['the interface name only starts with ProcessEnv', 'declare namespace NodeJS { interface ProcessEnvExtra { foo: string } }'], + [ + 'the declaration is inside a comment', outdent` + // declare namespace NodeJS { interface ProcessEnv {} } + export {}; + `, + ], + ])('not a conflict when %s', async (_label, contents) => { + await writeFile('other.d.ts', contents); const found = await findConflictingProcessEnvAugmentation({ dirs: [tempDir], outputPath: path.join(tempDir, 'env.d.ts'), @@ -1504,6 +1520,21 @@ describe('type generation', () => { expect(found).toBeUndefined(); }); + test('matches the nested `declare global` form we emit ourselves', async () => { + await writeFile('other.d.ts', outdent` + declare global { + namespace NodeJS { + interface ProcessEnv extends Something {} + } + } + `); + const found = await findConflictingProcessEnvAugmentation({ + dirs: [tempDir], + outputPath: path.join(tempDir, 'env.d.ts'), + }); + expect(found).toBe(path.join(tempDir, 'other.d.ts')); + }); + test('defaults processEnv to none, and says why in the generated file', async () => { await writeFile('worker-configuration.d.ts', FOREIGN_DTS); const src = await generate('path=env.d.ts'); From 3ee2740a295d8db04ba7f7134f51ed6363df0a42 Mon Sep 17 00:00:00 2001 From: Theo Ephraim Date: Sun, 13 Sep 2026 23:23:56 -0700 Subject: [PATCH 3/8] fix(varlock): ignore string literal contents when scanning for ProcessEnv A `'}'` literal type inside a `namespace NodeJS` block ended the brace walk early, and a literal spelled `'interface ProcessEnv'` faked a declaration. Strip string and template literals alongside comments before scanning. --- .../detect-process-env-augmentation.ts | 26 ++++++++++++++++--- .../env-graph/test/type-generation.test.ts | 16 ++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts index 21ddb71bc..080a2d21d 100644 --- a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts +++ b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts @@ -41,9 +41,21 @@ async function readFileHead(filePath: string, maxBytes = MAX_SCAN_BYTES): Promis } } -/** strip comments so a mention of these declarations in prose never counts as one */ -function stripComments(src: string): string { - return src.replace(/\/\*[\s\S]*?\*\//g, ' ').replace(/\/\/[^\n]*/g, ' '); +// string and template literal types are legal in a `.d.ts`, and their contents are not syntax: a +// `'}'` would otherwise close a namespace early (missing a real conflict) and a literal spelled +// `'interface ProcessEnv'` would fake one. Matched in a single left-to-right pass so whichever +// quote opens first wins, and replaced with a space so neighbouring tokens don't fuse. +const LITERALS = /'(?:[^'\\\n]|\\.)*'|"(?:[^"\\\n]|\\.)*"|`(?:[^`\\]|\\.)*`/g; + +/** + * Reduce a source file to just the parts that can be declaration syntax. Comments go first, so an + * apostrophe in prose can't open a "literal" that swallows real code after it. + */ +function stripNonSyntax(src: string): string { + return src + .replace(/\/\*[\s\S]*?\*\//g, ' ') + .replace(/\/\/[^\n]*/g, ' ') + .replace(LITERALS, ' '); } const NODEJS_NAMESPACE_OPEN = /\bnamespace\s+NodeJS\s*\{/g; @@ -58,9 +70,15 @@ const PROCESS_ENV_INTERFACE = /\binterface\s+ProcessEnv\b/; * Deliberately matched by structure rather than by any one tool's output shape, which we don't * control and which changes: this covers both `declare namespace NodeJS { ... }` (what wrangler * writes) and the `declare global { namespace NodeJS { ... } }` form we emit ourselves. + * + * This is a scan, not a parser, so pathological input can still fool it. That is an acceptable + * trade here: a missed declaration just leaves today's behaviour in place (and the conflict it + * would have caused is a loud `TS2320` with a documented one-arg fix), while the false-positive + * direction, which silently drops typing, needs the literal text of a declaration to appear + * outside comments and strings. */ function declaresProcessEnv(rawSrc: string): boolean { - const src = stripComments(rawSrc); + const src = stripNonSyntax(rawSrc); NODEJS_NAMESPACE_OPEN.lastIndex = 0; let match = NODEJS_NAMESPACE_OPEN.exec(src); while (match) { diff --git a/packages/varlock/src/env-graph/test/type-generation.test.ts b/packages/varlock/src/env-graph/test/type-generation.test.ts index 401e2436f..1c1b57a61 100644 --- a/packages/varlock/src/env-graph/test/type-generation.test.ts +++ b/packages/varlock/src/env-graph/test/type-generation.test.ts @@ -1511,6 +1511,8 @@ describe('type generation', () => { export {}; `, ], + // literal contents are not syntax either + ['the declaration text is inside a string literal', "declare namespace NodeJS { const marker: 'interface ProcessEnv'; }"], ])('not a conflict when %s', async (_label, contents) => { await writeFile('other.d.ts', contents); const found = await findConflictingProcessEnvAugmentation({ @@ -1520,6 +1522,20 @@ describe('type generation', () => { expect(found).toBeUndefined(); }); + test('a `}` inside a string literal does not end the namespace early', async () => { + await writeFile('other.d.ts', outdent` + declare namespace NodeJS { + type Brace = '}'; + interface ProcessEnv extends Something {} + } + `); + const found = await findConflictingProcessEnvAugmentation({ + dirs: [tempDir], + outputPath: path.join(tempDir, 'env.d.ts'), + }); + expect(found).toBe(path.join(tempDir, 'other.d.ts')); + }); + test('matches the nested `declare global` form we emit ourselves', async () => { await writeFile('other.d.ts', outdent` declare global { From 0f7101b8dfb3900e1677f34a698bb18b4541519c Mon Sep 17 00:00:00 2001 From: Theo Ephraim Date: Sun, 13 Sep 2026 23:55:23 -0700 Subject: [PATCH 4/8] fix(varlock): consume comments and literals in source order Stripping comments before literals let a literal spelled '//' or '/*' hide a real declaration after it; stripping literals first would let a comment's apostrophe do the same. Fold both token classes into one regex so a single left-to-right pass takes whichever opens first. --- .../detect-process-env-augmentation.ts | 28 ++++++++------- .../env-graph/test/type-generation.test.ts | 36 +++++++++++++++++++ 2 files changed, 51 insertions(+), 13 deletions(-) diff --git a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts index 080a2d21d..76377c684 100644 --- a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts +++ b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts @@ -41,21 +41,23 @@ async function readFileHead(filePath: string, maxBytes = MAX_SCAN_BYTES): Promis } } -// string and template literal types are legal in a `.d.ts`, and their contents are not syntax: a -// `'}'` would otherwise close a namespace early (missing a real conflict) and a literal spelled -// `'interface ProcessEnv'` would fake one. Matched in a single left-to-right pass so whichever -// quote opens first wins, and replaced with a space so neighbouring tokens don't fuse. -const LITERALS = /'(?:[^'\\\n]|\\.)*'|"(?:[^"\\\n]|\\.)*"|`(?:[^`\\]|\\.)*`/g; +// Neither comments nor string/template literals are declaration syntax: a `'}'` literal would +// close a namespace early (missing a real conflict), while `'interface ProcessEnv'` in a comment +// or a literal would fake one. Both token classes are alternatives of ONE regex so a single +// left-to-right pass consumes whichever opens first: stripping either class ahead of the other +// lets it be spoofed (`// don't` eating real code, or `type M = '//'` hiding the rest of a line). +const COMMENTS_AND_LITERALS = new RegExp([ + /\/\*[\s\S]*?\*\//, // block comment + /\/\/[^\n]*/, // line comment + /'(?:[^'\\\n]|\\.)*'/, // single-quoted + /"(?:[^"\\\n]|\\.)*"/, // double-quoted + /`(?:[^`\\]|\\.)*`/, // template literal +].map((r) => r.source).join('|'), 'g'); -/** - * Reduce a source file to just the parts that can be declaration syntax. Comments go first, so an - * apostrophe in prose can't open a "literal" that swallows real code after it. - */ +/** Reduce a source file to just the parts that can be declaration syntax. */ function stripNonSyntax(src: string): string { - return src - .replace(/\/\*[\s\S]*?\*\//g, ' ') - .replace(/\/\/[^\n]*/g, ' ') - .replace(LITERALS, ' '); + // replaced with a space rather than removed, so neighbouring tokens can't fuse + return src.replace(COMMENTS_AND_LITERALS, ' '); } const NODEJS_NAMESPACE_OPEN = /\bnamespace\s+NodeJS\s*\{/g; diff --git a/packages/varlock/src/env-graph/test/type-generation.test.ts b/packages/varlock/src/env-graph/test/type-generation.test.ts index 1c1b57a61..4f642d7fc 100644 --- a/packages/varlock/src/env-graph/test/type-generation.test.ts +++ b/packages/varlock/src/env-graph/test/type-generation.test.ts @@ -1522,6 +1522,42 @@ describe('type generation', () => { expect(found).toBeUndefined(); }); + // comments and literals have to be consumed in source order: stripping either class first + // lets the other be spoofed, hiding a real declaration that follows + test.each([ + // a line comment delimiter inside a literal, with the declaration on the same line + ['a line comment delimiter', "type Marker = '//'; interface ProcessEnv extends Something {}"], + // a block comment delimiter inside a literal, closed by a `*/` in a later literal + [ + 'a block comment delimiter', outdent` + type Open = '/*'; + interface ProcessEnv extends Something {} + type Close = '*/'; + `, + ], + ])('a declaration is still found past a string literal containing %s', async (_label, body) => { + await writeFile('other.d.ts', `declare namespace NodeJS {\n${body}\n}`); + const found = await findConflictingProcessEnvAugmentation({ + dirs: [tempDir], + outputPath: path.join(tempDir, 'env.d.ts'), + }); + expect(found).toBe(path.join(tempDir, 'other.d.ts')); + }); + + test('an apostrophe in a comment does not hide a declaration below it', async () => { + await writeFile('other.d.ts', outdent` + declare namespace NodeJS { + // don't let this comment's apostrophe open a literal + interface ProcessEnv extends Something {} + } + `); + const found = await findConflictingProcessEnvAugmentation({ + dirs: [tempDir], + outputPath: path.join(tempDir, 'env.d.ts'), + }); + expect(found).toBe(path.join(tempDir, 'other.d.ts')); + }); + test('a `}` inside a string literal does not end the namespace early', async () => { await writeFile('other.d.ts', outdent` declare namespace NodeJS { From 3d189b0d2da5dece22b810c7065e64bd952abb57 Mon Sep 17 00:00:00 2001 From: Theo Ephraim Date: Mon, 14 Sep 2026 00:04:27 -0700 Subject: [PATCH 5/8] refactor(varlock): match ProcessEnv declarations with one adjacency regex Replace the brace-matching namespace walk with a single regex requiring `interface ProcessEnv` to open the `namespace NodeJS` block, which is what every generator that writes one actually emits. `[^}]` cannot run past the first member, so a ProcessEnv declared after the namespace closes still cannot match. This gives up detecting a declaration that is not the namespace's first member. That is the fail-safe direction: a miss leaves today's behaviour and surfaces as TS2320 with the documented processEnv=none fix, while a false positive would silently drop valid typing. --- .../detect-process-env-augmentation.ts | 73 ++++++++----------- .../env-graph/test/type-generation.test.ts | 9 +++ 2 files changed, 38 insertions(+), 44 deletions(-) diff --git a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts index 76377c684..13cd93f8e 100644 --- a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts +++ b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts @@ -41,11 +41,12 @@ async function readFileHead(filePath: string, maxBytes = MAX_SCAN_BYTES): Promis } } -// Neither comments nor string/template literals are declaration syntax: a `'}'` literal would -// close a namespace early (missing a real conflict), while `'interface ProcessEnv'` in a comment -// or a literal would fake one. Both token classes are alternatives of ONE regex so a single -// left-to-right pass consumes whichever opens first: stripping either class ahead of the other -// lets it be spoofed (`// don't` eating real code, or `type M = '//'` hiding the rest of a line). +// Comments and string/template literals are not declaration syntax, and either can spoof the +// match in both directions (prose that reads like a declaration, or a stray `}` that hides one). +// Both classes are alternatives of ONE regex, so a single left-to-right pass takes whichever +// opens first: stripping either class ahead of the other lets it be spoofed (`// don't` eating +// real code, or `type M = '//'` hiding the rest of a line). Replaced with a space, not removed, +// so neighbouring tokens can't fuse. const COMMENTS_AND_LITERALS = new RegExp([ /\/\*[\s\S]*?\*\//, // block comment /\/\/[^\n]*/, // line comment @@ -54,50 +55,34 @@ const COMMENTS_AND_LITERALS = new RegExp([ /`(?:[^`\\]|\\.)*`/, // template literal ].map((r) => r.source).join('|'), 'g'); -/** Reduce a source file to just the parts that can be declaration syntax. */ -function stripNonSyntax(src: string): string { - // replaced with a space rather than removed, so neighbouring tokens can't fuse - return src.replace(COMMENTS_AND_LITERALS, ' '); -} - -const NODEJS_NAMESPACE_OPEN = /\bnamespace\s+NodeJS\s*\{/g; -const PROCESS_ENV_INTERFACE = /\binterface\s+ProcessEnv\b/; - /** - * True when the source declares `interface ProcessEnv` *within* a `namespace NodeJS` block. Both - * names have to be checked together: `interface ProcessEnvExtra` next to an unrelated - * `namespace NodeJS { interface Process {} }` isn't a conflict, and wrongly treating it as one - * would silently drop typing that was fine. + * `interface ProcessEnv` opening a `namespace NodeJS` block. The two tokens must be adjacent + * (nothing but whitespace, modifiers, or `}`-free text between them), which is what every + * generator that writes one of these actually emits, ours included: * - * Deliberately matched by structure rather than by any one tool's output shape, which we don't - * control and which changes: this covers both `declare namespace NodeJS { ... }` (what wrangler - * writes) and the `declare global { namespace NodeJS { ... } }` form we emit ourselves. + * declare namespace NodeJS { interface ProcessEnv extends ... {} } // wrangler + * declare global { namespace NodeJS { interface ProcessEnv ... } } // varlock * - * This is a scan, not a parser, so pathological input can still fool it. That is an acceptable - * trade here: a missed declaration just leaves today's behaviour in place (and the conflict it - * would have caused is a loud `TS2320` with a documented one-arg fix), while the false-positive - * direction, which silently drops typing, needs the literal text of a declaration to appear - * outside comments and strings. + * Requiring adjacency is what keeps this honest without a parser. `[^}]` can't run past the end + * of the first member, so a `ProcessEnv` declared *after* the namespace closes can't match, and + * `\b` keeps `interface ProcessEnvExtra` out. + */ +const NODEJS_PROCESS_ENV = /\bnamespace\s+NodeJS\s*\{[^}]{0,400}?\binterface\s+ProcessEnv\b/; + +/** + * True when the source looks like it declares `NodeJS.ProcessEnv`. + * + * A heuristic on purpose. It is deliberately biased to miss rather than over-match, because the + * two failure directions are not symmetric: a miss just leaves today's behaviour in place, and + * the conflict it would have avoided shows up as a loud `TS2320` with a documented one-arg fix + * (`processEnv=none`), whereas a false positive silently drops typing that was fine. So anything + * it can't read confidently (a declaration that isn't the first member of its namespace, or one + * buried in an exotic literal the strip above doesn't recognize) simply doesn't match, and lands + * the user on the error and the documented fix. Matching by structure rather than by any one tool's output shape also keeps + * it from rotting when that output changes. */ function declaresProcessEnv(rawSrc: string): boolean { - const src = stripNonSyntax(rawSrc); - NODEJS_NAMESPACE_OPEN.lastIndex = 0; - let match = NODEJS_NAMESPACE_OPEN.exec(src); - while (match) { - // walk from the namespace's opening brace to its matching close, so a `ProcessEnv` declared - // after the block (or in a sibling one) doesn't count as being inside it - let depth = 1; - let i = match.index + match[0].length; - const bodyStart = i; - while (i < src.length && depth > 0) { - if (src[i] === '{') depth++; - else if (src[i] === '}') depth--; - i++; - } - if (PROCESS_ENV_INTERFACE.test(src.slice(bodyStart, depth === 0 ? i - 1 : undefined))) return true; - match = NODEJS_NAMESPACE_OPEN.exec(src); - } - return false; + return NODEJS_PROCESS_ENV.test(rawSrc.replace(COMMENTS_AND_LITERALS, ' ')); } /** diff --git a/packages/varlock/src/env-graph/test/type-generation.test.ts b/packages/varlock/src/env-graph/test/type-generation.test.ts index 4f642d7fc..887cc53d6 100644 --- a/packages/varlock/src/env-graph/test/type-generation.test.ts +++ b/packages/varlock/src/env-graph/test/type-generation.test.ts @@ -1505,6 +1505,15 @@ describe('type generation', () => { `, ], ['the interface name only starts with ProcessEnv', 'declare namespace NodeJS { interface ProcessEnvExtra { foo: string } }'], + // the match requires ProcessEnv to open the namespace, so a later member is a (fail-safe) miss + [ + 'ProcessEnv is not the first member of the namespace', outdent` + declare namespace NodeJS { + interface Process { foo: string } + interface ProcessEnv { BAR: string } + } + `, + ], [ 'the declaration is inside a comment', outdent` // declare namespace NodeJS { interface ProcessEnv {} } From 4b8d4a540734c12153987932dc6f01d801d496d5 Mon Sep 17 00:00:00 2001 From: Theo Ephraim Date: Mon, 14 Sep 2026 16:14:04 -0700 Subject: [PATCH 6/8] fix(varlock): keep line continuations inside string literals when scanning A backslash-newline pair ended the literal match, exposing the rest of the literal as if it were syntax, so declaration text inside a continued string literal read as a real ProcessEnv declaration. Let the escape alternative take any character, newline included. --- .../type-generation/detect-process-env-augmentation.ts | 8 +++++--- .../varlock/src/env-graph/test/type-generation.test.ts | 2 ++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts index 13cd93f8e..7484c52a3 100644 --- a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts +++ b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts @@ -50,9 +50,11 @@ async function readFileHead(filePath: string, maxBytes = MAX_SCAN_BYTES): Promis const COMMENTS_AND_LITERALS = new RegExp([ /\/\*[\s\S]*?\*\//, // block comment /\/\/[^\n]*/, // line comment - /'(?:[^'\\\n]|\\.)*'/, // single-quoted - /"(?:[^"\\\n]|\\.)*"/, // double-quoted - /`(?:[^`\\]|\\.)*`/, // template literal + // the escape alternative takes any character, newline included, so a backslash-newline line + // continuation stays inside the literal instead of ending the match and exposing its contents + /'(?:[^'\\\n]|\\[\s\S])*'/, // single-quoted + /"(?:[^"\\\n]|\\[\s\S])*"/, // double-quoted + /`(?:[^`\\]|\\[\s\S])*`/, // template literal ].map((r) => r.source).join('|'), 'g'); /** diff --git a/packages/varlock/src/env-graph/test/type-generation.test.ts b/packages/varlock/src/env-graph/test/type-generation.test.ts index 887cc53d6..ef3423a60 100644 --- a/packages/varlock/src/env-graph/test/type-generation.test.ts +++ b/packages/varlock/src/env-graph/test/type-generation.test.ts @@ -1522,6 +1522,8 @@ describe('type generation', () => { ], // literal contents are not syntax either ['the declaration text is inside a string literal', "declare namespace NodeJS { const marker: 'interface ProcessEnv'; }"], + // a backslash-newline continuation keeps the literal open across lines + ['the declaration text is inside a continued string literal', "declare namespace NodeJS {\n type Marker = 'a\\\ninterface ProcessEnv b';\n}"], ])('not a conflict when %s', async (_label, contents) => { await writeFile('other.d.ts', contents); const found = await findConflictingProcessEnvAugmentation({ From 2c4c25a76773175f973a97873cedd19778683617 Mon Sep 17 00:00:00 2001 From: Theo Ephraim Date: Tue, 15 Sep 2026 01:16:21 -0700 Subject: [PATCH 7/8] fix(varlock): treat a CRLF line continuation as one escape when scanning A backslash followed by CRLF consumed only the carriage return, leaving the newline to end the quoted-literal match and expose its contents as syntax. Match CRLF as a single unit in the escape alternative. --- .../detect-process-env-augmentation.ts | 11 ++++++----- .../src/env-graph/test/type-generation.test.ts | 3 ++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts index 7484c52a3..7be1a1070 100644 --- a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts +++ b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts @@ -50,11 +50,12 @@ async function readFileHead(filePath: string, maxBytes = MAX_SCAN_BYTES): Promis const COMMENTS_AND_LITERALS = new RegExp([ /\/\*[\s\S]*?\*\//, // block comment /\/\/[^\n]*/, // line comment - // the escape alternative takes any character, newline included, so a backslash-newline line - // continuation stays inside the literal instead of ending the match and exposing its contents - /'(?:[^'\\\n]|\\[\s\S])*'/, // single-quoted - /"(?:[^"\\\n]|\\[\s\S])*"/, // double-quoted - /`(?:[^`\\]|\\[\s\S])*`/, // template literal + // the escape alternative takes any character (CRLF as one unit, since a lone trailing `\n` + // would end the match), so a backslash-newline line continuation stays inside the literal + // instead of ending it and exposing its contents as if they were syntax + /'(?:[^'\\\n]|\\(?:\r\n|[\s\S]))*'/, // single-quoted + /"(?:[^"\\\n]|\\(?:\r\n|[\s\S]))*"/, // double-quoted + /`(?:[^`\\]|\\(?:\r\n|[\s\S]))*`/, // template literal ].map((r) => r.source).join('|'), 'g'); /** diff --git a/packages/varlock/src/env-graph/test/type-generation.test.ts b/packages/varlock/src/env-graph/test/type-generation.test.ts index ef3423a60..83fa2edb0 100644 --- a/packages/varlock/src/env-graph/test/type-generation.test.ts +++ b/packages/varlock/src/env-graph/test/type-generation.test.ts @@ -1522,8 +1522,9 @@ describe('type generation', () => { ], // literal contents are not syntax either ['the declaration text is inside a string literal', "declare namespace NodeJS { const marker: 'interface ProcessEnv'; }"], - // a backslash-newline continuation keeps the literal open across lines + // a backslash-newline continuation keeps the literal open across lines, either line ending ['the declaration text is inside a continued string literal', "declare namespace NodeJS {\n type Marker = 'a\\\ninterface ProcessEnv b';\n}"], + ['the continued string literal uses CRLF', "declare namespace NodeJS {\r\n type Marker = 'a\\\r\ninterface ProcessEnv b';\r\n}"], ])('not a conflict when %s', async (_label, contents) => { await writeFile('other.d.ts', contents); const found = await findConflictingProcessEnvAugmentation({ From 923e125ce28e0f371e388dbf59b7a2ee15ff1850 Mon Sep 17 00:00:00 2001 From: Theo Ephraim Date: Tue, 15 Sep 2026 01:45:23 -0700 Subject: [PATCH 8/8] fix(varlock): remove exponential backtracking from the literal scanner Spelling CRLF out in the escape alternative gave a backslash followed by CRLF two ways to match, which backtracks exponentially on a literal that never closes (CodeQL js/redos, high). Normalize line endings up front instead, so every escape is exactly two characters and each alternative is unambiguous. Measured on the reported witness: 58ms at 22 repetitions before, flat after. --- .../detect-process-env-augmentation.ts | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts index 7be1a1070..370fd44a4 100644 --- a/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts +++ b/packages/varlock/src/env-graph/lib/type-generation/detect-process-env-augmentation.ts @@ -50,12 +50,14 @@ async function readFileHead(filePath: string, maxBytes = MAX_SCAN_BYTES): Promis const COMMENTS_AND_LITERALS = new RegExp([ /\/\*[\s\S]*?\*\//, // block comment /\/\/[^\n]*/, // line comment - // the escape alternative takes any character (CRLF as one unit, since a lone trailing `\n` - // would end the match), so a backslash-newline line continuation stays inside the literal - // instead of ending it and exposing its contents as if they were syntax - /'(?:[^'\\\n]|\\(?:\r\n|[\s\S]))*'/, // single-quoted - /"(?:[^"\\\n]|\\(?:\r\n|[\s\S]))*"/, // double-quoted - /`(?:[^`\\]|\\(?:\r\n|[\s\S]))*`/, // template literal + // the escape alternative takes any character, newline included, so a backslash-newline line + // continuation stays inside the literal instead of ending it and exposing its contents. Line + // endings are normalized before this runs, so `\\` + newline is always exactly two characters: + // spelling CRLF out here instead would give `\\` + CRLF two ways to match, which backtracks + // exponentially on an unterminated literal (js/redos). + /'(?:[^'\\\n]|\\[\s\S])*'/, // single-quoted + /"(?:[^"\\\n]|\\[\s\S])*"/, // double-quoted + /`(?:[^`\\]|\\[\s\S])*`/, // template literal ].map((r) => r.source).join('|'), 'g'); /** @@ -85,7 +87,9 @@ const NODEJS_PROCESS_ENV = /\bnamespace\s+NodeJS\s*\{[^}]{0,400}?\binterface\s+P * it from rotting when that output changes. */ function declaresProcessEnv(rawSrc: string): boolean { - return NODEJS_PROCESS_ENV.test(rawSrc.replace(COMMENTS_AND_LITERALS, ' ')); + // normalize line endings first so the patterns above only ever deal with `\n` + const src = rawSrc.replace(/\r\n?/g, '\n'); + return NODEJS_PROCESS_ENV.test(src.replace(COMMENTS_AND_LITERALS, ' ')); } /**