diff --git a/.changeset/graceful-pumas-roar.md b/.changeset/graceful-pumas-roar.md new file mode 100644 index 000000000..00da4ed57 --- /dev/null +++ b/.changeset/graceful-pumas-roar.md @@ -0,0 +1,5 @@ +--- +type: Added +pr: 3976 +--- +**New ESLint rule `local/no-exact-case-env-access`** — flags an exact-case read of a Windows case-varying environment variable (`PATH`, `PATHEXT`, `ComSpec`, `USERPROFILE`, `TEMP`, `TMP`, `APPDATA`) off any object other than `process.env` itself, closing the gap ADR-1703's portability catalog left on production Windows semantics. (#3624) diff --git a/CONTEXT.md b/CONTEXT.md index ad6d2da05..bcd85086d 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -881,7 +881,7 @@ The prompt-level data/instruction isolation seam for untrusted web/document ingr ## Shell Command Projection Module (expanded glossary entry, 2026-05-13) -Module owning all OS-facing I/O for the tool: runtime-aware command-text rendering (hook commands, PATH action lines, shim scripts), subprocess dispatch (execGit, execNpm, execTool, probeTty, isSpawnTimeout), Windows binary resolution (resolveExecutableBinary, projectSpawnInvocation), and platform file I/O (platformWriteSync, platformReadSync, platformEnsureDir). Single seam for platform-conditional logic — one place to fix any shell or file write regression across Windows, macOS, and Linux. WINDOWS BINARY RESOLUTION — which file a declared command name actually names, and what must be handed to `spawnSync` to start it — is owned here as of #3411 (epic #3411 Phase 1), which found the seam declaration untrue for this axis: four divergent implementations had grown outside it (`execNpm`'s `shell:true`, `execTool`'s absence of any handling, a private PATH+PATHEXT scan in `gsd-core/bin/gsd-tools.cjs`, and a fourth candidate-extension array in `fallow-runner.cts`), and #3275's fix to one provably never reached the others. `resolveExecutableBinary(name, {platform, env})` returns the RESOLVED PATH (the prior `hasBinary` computed it and threw it away): on win32 it tries PATHEXT entries ONLY and never the bare name, because npm global installs drop an extensionless POSIX `sh` shim beside `foo.CMD` and a bare-name-first scan resolves to it, leaving the ENOENT unchanged (#3275); a name already carrying a PATHEXT-listed extension is tried as-is BEFORE the append loop, and a suffix outside PATHEXT is not an extension. On POSIX it answers existence only, and `execTool` does not consult it there at all — the bare name goes to `spawnSync` unchanged and Node's own PATH search does the work, keeping macOS/Linux byte-identical for a symbol whose blast radius is CRITICAL (167 affected symbols, 53 files). `projectSpawnInvocation(command, args, {platform, env})` is the inseparable second half: `CreateProcess` cannot execute a `.cmd`/`.bat` at all, so resolution alone does not fix Windows, and exporting only the resolver would leave every caller to re-derive the mediation — which is how the four copies accumulated. It mediates through `ComSpec` with an EXPLICIT argv array (`/d /s /c`), never `shell: true`: that is CVE-2024-27980's argument-injection vector and Node 26's DEP0190. Mediation keys on the target — the resolved path, or the declared name when resolution found nothing — so a `.cmd` resident in the current directory (which PATH-only resolution misses but `cmd.exe /c` still finds) keeps working, while a BARE unresolved name is passed through untouched so the spawn fails with ENOENT rather than cmd.exe's exit 9009, preserving `_spawnResult`'s `': not found'` contract. `gsd-core/bin/gsd-tools.cjs`'s `resolveSpawnBinary` is the `bin/` entry point onto this and holds no copy of the logic. CALLERS CHOOSE HOW MUCH OF THE PROJECTION TO ADOPT, and the asymmetry is deliberate: `windowsVerbatimArguments` marks the cases where mediation was REQUIRED (the caller must take `command` and `args` together, or the batch file cannot start at all), whereas a merely-RESOLVED path is an offer a caller may decline. `execTool` declines it and keeps spawning the DECLARED name — libuv's `CreateProcess` path already performs PATH+PATHEXT search, so resolving a `.exe` there buys nothing while changing what 167 dependents observe being spawned; `tests/graphify.test.cjs` pins that contract by spying on `spawnSync`'s first argument, and the Windows CI lane caught the violation when `execTool` briefly adopted the resolved path (`python3` arriving as `C:\…\python3.EXE`). `deps.spawn` accepts it, because that lane's `hasBinary` probe answers from the same resolver and probe and spawn must agree on the exact file (#3445). Env lookups go through a case-insensitive read: Windows names the variable `Path`, `process.env` is a case-insensitive proxy that hides this, and `execTool`'s `{...process.env, ...opts.env}` spread produces a PLAIN object that keeps the OS casing and loses the proxy — an exact-case `env['PATH']` there returns undefined and the scan silently sees nothing. `resolveExecutableBinary` carries two OPT-IN options, both defaulting off so Phase 1's callers are byte-identical: `prependPaths` (directories searched before `env.PATH`, in order, with the identical per-directory candidate logic — this is how `node_modules/.bin`-first precedence is expressed without env surgery) and `requireExecutable` (POSIX-only additional `accessSync(X_OK)`; a no-op on win32, where mode bits do not mean execute). `requireExecutable` is opt-in rather than default because making it unconditional would break #3445's own suite, which stages candidates with plain `writeFileSync` and never sets an exec bit — the repo bans `chmod` in tests — so every one of those would resolve to null on POSIX. As of #3618 (epic #3411 Phase 2) `src/fallow-runner.cts` holds no resolver of its own: `resolveFallowBinary` is one seam call passing `prependPaths: [/node_modules/.bin]` and `requireExecutable: true`, preserving `.bin`-before-PATH precedence and the POSIX executability check. Its prior win32 candidate list ended in a BARE `fallow`; the seam does not, and dropping it is the fix — an extensionless file beside `fallow.cmd` is npm's POSIX `sh` shim that `CreateProcess` cannot run (#3275). Note the precedence was documented BACKWARDS (`PATH` then `.bin`) in `structural-pre-pass.md` and four INVENTORY translations until #3618 corrected them; the code was always `.bin` first. As of #3619 (epic #3411 Phase 3) the seam's ownership is RATCHETED by `local/no-private-binary-resolution` (`eslint-rules/no-private-binary-resolution.cjs`, ADR-1703 catalog): re-implementing Windows binary resolution outside `src/shell-command-projection.cts` is an eslint error, keyed on the two unambiguous signals — reading `PATHEXT` in any casing from any object, and a hardcoded list carrying two or more of `.exe`/`.cmd`/`.bat`/`.com` (the shapes all four deleted resolvers actually had). `DEFECT.WINDOWS-PRIVATE-BINARY-RESOLUTION`. It deliberately does NOT flag a bare-name spawn — ~30 such sites exist and none is a defect, since `git`/`gh`/`npm` ship native `.exe` that `CreateProcess` resolves unaided — nor a `PATH` scan, which is indistinguishable from a legitimate membership check (`bin/install.js`). The extension threshold is TWO because a single `.endsWith('.cmd')` is a classification, not a candidate set (`runtime-hooks-surface.cts` derives `.cmd` shim paths that way), and matching is boundary-aware because a naive substring test flags `.execute` and `.compacting`. The seam exemption is path-SUFFIX anchored, not substring; the rule's own surface is `src/**/*.cts`, `gsd-core/bin/**/*.cjs`, `scripts/**/*.cjs`, and `hooks/**/*.js`, and `tests/**` is deliberately outside that surface — test setup legitimately assigns `process.env.PATHEXT` (`tests/fallow-runner.test.cjs`'s P3 case), so `tests/shell-command-projection-dispatch.test.cjs` is NOT linted by this rule at all; the suffix-vs-substring distinction is instead proven by RuleTester case I9's synthetic filename, not by real-world coverage of that test file. `eslint-rules/**` is outside the rule's globs entirely rather than exempted, because `lib/portability-vocab.cjs` owns the extension set. To make the ratchet strict with no carve-out, `resolveExecutableBinary` also gained `pathOverride` — "search THIS PATH, read everything else including PATHEXT from the ambient environment" — so `resolveFallowBinary` supplies its own search path without hand-threading PATHEXT, which would itself have been a private PATHEXT read. `pathOverride: ''` means an EMPTY search path, never a fallback to `env.PATH` (`!== undefined`, not truthiness). `isSpawnTimeout` is the single shared "did this subprocess time out" predicate (error.code==='ETIMEDOUT' only — cross-platform-safe; does not require signal==='SIGTERM'), consumed by worktree-safety.cts, worktree-base-ref.cts, commands.cts, and this module's own dispatchGsdCommand (#3050 — "Generative Fix Divergence"). `projectPathExportLine(targetDir)` is the single source of the `export PATH=":$PATH"` line for all three PATH-persistence lanes (repair, persist, win32 Git Bash) — it double-quote-escapes for the line's final rc-file context before any lane single-quotes it for its own `echo` transport, closing the #3118 command-substitution injection where a lane re-escaped the line itself and let a `$(…)` in the target dir execute on every new shell; the win32 cmd.exe lane fails closed (empty actions) whenever the target dir contains `"`, since that character is reserved on Windows and would otherwise close cmd's quoted region, and now tags that empty result with a typed `PATH_ACTION_REASON` (`win32_reserved_quote`) so it stays distinguishable from the unrelated "no target directory given" empty result (`no_target_dir`, #3118); the fish lane emits `fish_add_path -- ''` (`--` end-of-options separator, verified empirically against fish 4.8.1), since a leading-dash target dir is otherwise misparsed by fish's argparse-based option scanning regardless of quoting; `escapeTomlDoubleQuotedString` now escapes TOML's required control characters (U+0000-U+0008, U+000A-U+001F, U+007F), not just backslash and quote, closing a raw-newline/CR/NUL config.toml parse failure (#3118). Lives in `gsd-core/bin/lib/shell-command-projection.cjs`. See ADR-0009 (superseded "does not execute" constraint) and ADR-0010 (superseded File Operation Engine). +Module owning all OS-facing I/O for the tool: runtime-aware command-text rendering (hook commands, PATH action lines, shim scripts), subprocess dispatch (execGit, execNpm, execTool, probeTty, isSpawnTimeout), Windows binary resolution (resolveExecutableBinary, projectSpawnInvocation), and platform file I/O (platformWriteSync, platformReadSync, platformEnsureDir). Single seam for platform-conditional logic — one place to fix any shell or file write regression across Windows, macOS, and Linux. WINDOWS BINARY RESOLUTION — which file a declared command name actually names, and what must be handed to `spawnSync` to start it — is owned here as of #3411 (epic #3411 Phase 1), which found the seam declaration untrue for this axis: four divergent implementations had grown outside it (`execNpm`'s `shell:true`, `execTool`'s absence of any handling, a private PATH+PATHEXT scan in `gsd-core/bin/gsd-tools.cjs`, and a fourth candidate-extension array in `fallow-runner.cts`), and #3275's fix to one provably never reached the others. `resolveExecutableBinary(name, {platform, env})` returns the RESOLVED PATH (the prior `hasBinary` computed it and threw it away): on win32 it tries PATHEXT entries ONLY and never the bare name, because npm global installs drop an extensionless POSIX `sh` shim beside `foo.CMD` and a bare-name-first scan resolves to it, leaving the ENOENT unchanged (#3275); a name already carrying a PATHEXT-listed extension is tried as-is BEFORE the append loop, and a suffix outside PATHEXT is not an extension. On POSIX it answers existence only, and `execTool` does not consult it there at all — the bare name goes to `spawnSync` unchanged and Node's own PATH search does the work, keeping macOS/Linux byte-identical for a symbol whose blast radius is CRITICAL (167 affected symbols, 53 files). `projectSpawnInvocation(command, args, {platform, env})` is the inseparable second half: `CreateProcess` cannot execute a `.cmd`/`.bat` at all, so resolution alone does not fix Windows, and exporting only the resolver would leave every caller to re-derive the mediation — which is how the four copies accumulated. It mediates through `ComSpec` with an EXPLICIT argv array (`/d /s /c`), never `shell: true`: that is CVE-2024-27980's argument-injection vector and Node 26's DEP0190. Mediation keys on the target — the resolved path, or the declared name when resolution found nothing — so a `.cmd` resident in the current directory (which PATH-only resolution misses but `cmd.exe /c` still finds) keeps working, while a BARE unresolved name is passed through untouched so the spawn fails with ENOENT rather than cmd.exe's exit 9009, preserving `_spawnResult`'s `': not found'` contract. `gsd-core/bin/gsd-tools.cjs`'s `resolveSpawnBinary` is the `bin/` entry point onto this and holds no copy of the logic. CALLERS CHOOSE HOW MUCH OF THE PROJECTION TO ADOPT, and the asymmetry is deliberate: `windowsVerbatimArguments` marks the cases where mediation was REQUIRED (the caller must take `command` and `args` together, or the batch file cannot start at all), whereas a merely-RESOLVED path is an offer a caller may decline. `execTool` declines it and keeps spawning the DECLARED name — libuv's `CreateProcess` path already performs PATH+PATHEXT search, so resolving a `.exe` there buys nothing while changing what 167 dependents observe being spawned; `tests/graphify.test.cjs` pins that contract by spying on `spawnSync`'s first argument, and the Windows CI lane caught the violation when `execTool` briefly adopted the resolved path (`python3` arriving as `C:\…\python3.EXE`). `deps.spawn` accepts it, because that lane's `hasBinary` probe answers from the same resolver and probe and spawn must agree on the exact file (#3445). Env lookups go through a case-insensitive read: Windows names the variable `Path`, `process.env` is a case-insensitive proxy that hides this, and `execTool`'s `{...process.env, ...opts.env}` spread produces a PLAIN object that keeps the OS casing and loses the proxy — an exact-case `env['PATH']` there returns undefined and the scan silently sees nothing. This is fixed inside the seam by `envGet(env, name)` (formerly private `_envGet`, exported as of #3624), and ratcheted outside the seam by `local/no-exact-case-env-access` (ADR-1703 catalog, epic #3411 Phase 4): it flags a read of any casing of `PATH`/`PATHEXT`/`ComSpec`/`USERPROFILE`/`TEMP`/`TMP`/`APPDATA` off any receiver that is not literally `process.env`, matched via an "env-shaped receiver" check (`.env`/`['env']` or a bare `env`-named identifier) rather than a blanket case-insensitive property-name match — the blanket form produced 113 false positives against ordinary `.path`-named properties elsewhere in the tree. `DEFECT.WINDOWS-EXACT-CASE-ENV-ACCESS`. `resolveExecutableBinary` carries two OPT-IN options, both defaulting off so Phase 1's callers are byte-identical: `prependPaths` (directories searched before `env.PATH`, in order, with the identical per-directory candidate logic — this is how `node_modules/.bin`-first precedence is expressed without env surgery) and `requireExecutable` (POSIX-only additional `accessSync(X_OK)`; a no-op on win32, where mode bits do not mean execute). `requireExecutable` is opt-in rather than default because making it unconditional would break #3445's own suite, which stages candidates with plain `writeFileSync` and never sets an exec bit — the repo bans `chmod` in tests — so every one of those would resolve to null on POSIX. As of #3618 (epic #3411 Phase 2) `src/fallow-runner.cts` holds no resolver of its own: `resolveFallowBinary` is one seam call passing `prependPaths: [/node_modules/.bin]` and `requireExecutable: true`, preserving `.bin`-before-PATH precedence and the POSIX executability check. Its prior win32 candidate list ended in a BARE `fallow`; the seam does not, and dropping it is the fix — an extensionless file beside `fallow.cmd` is npm's POSIX `sh` shim that `CreateProcess` cannot run (#3275). Note the precedence was documented BACKWARDS (`PATH` then `.bin`) in `structural-pre-pass.md` and four INVENTORY translations until #3618 corrected them; the code was always `.bin` first. As of #3619 (epic #3411 Phase 3) the seam's ownership is RATCHETED by `local/no-private-binary-resolution` (`eslint-rules/no-private-binary-resolution.cjs`, ADR-1703 catalog): re-implementing Windows binary resolution outside `src/shell-command-projection.cts` is an eslint error, keyed on the two unambiguous signals — reading `PATHEXT` in any casing from any object, and a hardcoded list carrying two or more of `.exe`/`.cmd`/`.bat`/`.com` (the shapes all four deleted resolvers actually had). `DEFECT.WINDOWS-PRIVATE-BINARY-RESOLUTION`. It deliberately does NOT flag a bare-name spawn — ~30 such sites exist and none is a defect, since `git`/`gh`/`npm` ship native `.exe` that `CreateProcess` resolves unaided — nor a `PATH` scan, which is indistinguishable from a legitimate membership check (`bin/install.js`). The extension threshold is TWO because a single `.endsWith('.cmd')` is a classification, not a candidate set (`runtime-hooks-surface.cts` derives `.cmd` shim paths that way), and matching is boundary-aware because a naive substring test flags `.execute` and `.compacting`. The seam exemption is path-SUFFIX anchored, not substring; the rule's own surface is `src/**/*.cts`, `gsd-core/bin/**/*.cjs`, `scripts/**/*.cjs`, and `hooks/**/*.js`, and `tests/**` is deliberately outside that surface — test setup legitimately assigns `process.env.PATHEXT` (`tests/fallow-runner.test.cjs`'s P3 case), so `tests/shell-command-projection-dispatch.test.cjs` is NOT linted by this rule at all; the suffix-vs-substring distinction is instead proven by RuleTester case I9's synthetic filename, not by real-world coverage of that test file. `eslint-rules/**` is outside the rule's globs entirely rather than exempted, because `lib/portability-vocab.cjs` owns the extension set. To make the ratchet strict with no carve-out, `resolveExecutableBinary` also gained `pathOverride` — "search THIS PATH, read everything else including PATHEXT from the ambient environment" — so `resolveFallowBinary` supplies its own search path without hand-threading PATHEXT, which would itself have been a private PATHEXT read. `pathOverride: ''` means an EMPTY search path, never a fallback to `env.PATH` (`!== undefined`, not truthiness). `isSpawnTimeout` is the single shared "did this subprocess time out" predicate (error.code==='ETIMEDOUT' only — cross-platform-safe; does not require signal==='SIGTERM'), consumed by worktree-safety.cts, worktree-base-ref.cts, commands.cts, and this module's own dispatchGsdCommand (#3050 — "Generative Fix Divergence"). `projectPathExportLine(targetDir)` is the single source of the `export PATH=":$PATH"` line for all three PATH-persistence lanes (repair, persist, win32 Git Bash) — it double-quote-escapes for the line's final rc-file context before any lane single-quotes it for its own `echo` transport, closing the #3118 command-substitution injection where a lane re-escaped the line itself and let a `$(…)` in the target dir execute on every new shell; the win32 cmd.exe lane fails closed (empty actions) whenever the target dir contains `"`, since that character is reserved on Windows and would otherwise close cmd's quoted region, and now tags that empty result with a typed `PATH_ACTION_REASON` (`win32_reserved_quote`) so it stays distinguishable from the unrelated "no target directory given" empty result (`no_target_dir`, #3118); the fish lane emits `fish_add_path -- ''` (`--` end-of-options separator, verified empirically against fish 4.8.1), since a leading-dash target dir is otherwise misparsed by fish's argparse-based option scanning regardless of quoting; `escapeTomlDoubleQuotedString` now escapes TOML's required control characters (U+0000-U+0008, U+000A-U+001F, U+007F), not just backslash and quote, closing a raw-newline/CR/NUL config.toml parse failure (#3118). Lives in `gsd-core/bin/lib/shell-command-projection.cjs`. See ADR-0009 (superseded "does not execute" constraint) and ADR-0010 (superseded File Operation Engine). `SEAM.shellcmdproj-win-binary-resolution.owns=Windows binary resolution (resolveExecutableBinary, projectSpawnInvocation) — which file a declared command name actually names, and cmd.exe mediation` `SEAM.shellcmdproj-win-binary-resolution.enforced-by=lint-rule:no-private-binary-resolution` diff --git a/docs/adr/1703-portability-enforcement-architecture.md b/docs/adr/1703-portability-enforcement-architecture.md index fee05e77b..5c80eca5d 100644 --- a/docs/adr/1703-portability-enforcement-architecture.md +++ b/docs/adr/1703-portability-enforcement-architecture.md @@ -77,6 +77,7 @@ Replace all three with a single coherent mechanism: **AST-based ESLint rules in | `normalize-path-in-content` | `DEFECT.WINDOWS-PATH-LEAK-IN-MARKDOWN-CONTENT` (`RULESET.CONTENT-PATH-NORMALIZATION`) | `src/**/*.cts` | | `require-fs-op-fallback` | `DEFECT.WINDOWS-FS-OPS` | `src/**/*.cts`, build/install | | `no-private-binary-resolution` | `DEFECT.WINDOWS-PRIVATE-BINARY-RESOLUTION` | `src/**/*.cts`, `gsd-core/bin/**`, `scripts/**`, `hooks/**` | +| `no-exact-case-env-access` | `DEFECT.WINDOWS-EXACT-CASE-ENV-ACCESS` | `src/**/*.cts`, `gsd-core/bin/**`, `scripts/**`, `hooks/**` | **Amendment (2026-08-18, epic #3411 Phase 3 / #3619).** `no-private-binary-resolution` is the first catalog entry added after the original seven, and it extends this architecture to a @@ -110,6 +111,31 @@ Two scoping consequences worth recording, because both were arrived at rather th The rule started **green** with nothing suppressed and nothing grandfathered — Phases 1 and 2 had already removed every private resolver, which is what made a strict ratchet possible at all. +**Amendment (2026-08-27, epic #3411 Phase 4 / #3624).** `no-exact-case-env-access` extends the +architecture to a second production-runtime class: PR #3621 (epic #3411 Phase 1) shipped +`resolveExecutableBinary` reading `env['PATH']` where `env` could be a plain object (`{ +...process.env, ...opts.env }`, which loses `process.env`'s case-insensitive Proxy) — a Windows +CI-only failure caught and fixed by adding `envGet(env, name)` inside the seam. This rule +generalizes that fix into a ratchet: it flags a read of any casing of `PATH`, `PATHEXT`, +`ComSpec`, `USERPROFILE`, `TEMP`, `TMP`, or `APPDATA` (the vocabulary's +`WINDOWS_CASE_VARYING_ENV_VARS`) off any receiver that is not literally `process.env` — dot or +bracket notation, or destructuring — using the same seam exemption anchoring as +`no-private-binary-resolution`. + +Matching had to be narrower than "any property access whose name matches the vocabulary, +case-insensitively": a first pass produced 113 false positives, because ordinary lowercase +property access (`config.path`, `artifact['path']`) collides with the vocab entry `PATH` under +case-insensitive comparison. The shipped rule additionally requires the receiver to be +"env-shaped" — literally `.env` / `['env']` or a bare identifier named `env` (any +casing) — for both notations and for destructuring alike, which is what distinguishes +`opts.env['PATH']` (flagged) from `artifact['path']` (not flagged) without def-use/scope tracing. +One real pre-existing violation of the tightened rule was found and fixed in the same PR: +`src/runtime-hooks-surface.cts`'s `normalizeNodePath` read `env.APPDATA` off a runtime union +(`(opts && opts.env) || process.env`) that may be a plain object — migrated to `envGet(env, +'APPDATA')`. `envGet` (formerly the seam-private `_envGet`) is now exported from +`src/shell-command-projection.cts` specifically so this rule's remediation message ("route +through `envGet`") names a real, callable helper. + **Taxonomy coverage.** This catalog addresses every `DEFECT.WINDOWS-*` class plus `DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE` in `CONTEXT.md`, to the extent each is *statically* detectable. `DEFECT.TEST-SHELL-PIPELINE-NONPORTABLE` has two parts: (a) the CRLF / literal-`\n` diff --git a/eslint-rules/lib/portability-vocab.cjs b/eslint-rules/lib/portability-vocab.cjs index b4eaaf586..de2ca643b 100644 --- a/eslint-rules/lib/portability-vocab.cjs +++ b/eslint-rules/lib/portability-vocab.cjs @@ -100,6 +100,32 @@ const WINDOWS_EXECUTABLE_EXTENSIONS = ['.exe', '.cmd', '.bat', '.com']; */ const PATHEXT_VAR_NAME = 'PATHEXT'; +/** + * Windows environment variable names whose CONVENTIONAL casing varies from the + * all-uppercase POSIX convention (`Path` not `PATH`, `ComSpec` not `COMSPEC`, + * `TEMP`/`TMP`/`APPDATA`/`USERPROFILE` as shipped by Windows). `process.env` is + * a case-insensitive Proxy that hides this on every platform, so + * `process.env.PATH` is always safe — but a plain object (a spread of + * `process.env`, a function parameter, any object that is not literally + * `process.env` at the access site) keeps the OS's actual casing and an + * exact-case lookup against it silently returns `undefined` on Windows. + * local/no-exact-case-env-access matches a read of ANY casing of ANY of these + * names off ANY receiver that is not `process.env` itself (#3624, epic #3411 + * Phase 4). + */ +const WINDOWS_CASE_VARYING_ENV_VARS = ['PATH', 'PATHEXT', 'ComSpec', 'USERPROFILE', 'TEMP', 'TMP', 'APPDATA']; + +const WINDOWS_CASE_VARYING_ENV_VARS_LOWER = new Set(WINDOWS_CASE_VARYING_ENV_VARS.map((v) => v.toLowerCase())); + +/** + * True when `name` case-insensitively matches one of WINDOWS_CASE_VARYING_ENV_VARS. + * @param {string} name + * @returns {boolean} + */ +function isCaseVaryingEnvVarName(name) { + return typeof name === 'string' && WINDOWS_CASE_VARYING_ENV_VARS_LOWER.has(name.toLowerCase()); +} + /** * Boundary-aware match for each WINDOWS_EXECUTABLE_EXTENSIONS entry, in the * same order. A plain substring test misfires on ordinary prose/event-name @@ -403,6 +429,8 @@ module.exports = { PATH_RETURNING_FNS, WINDOWS_EXECUTABLE_EXTENSIONS, PATHEXT_VAR_NAME, + WINDOWS_CASE_VARYING_ENV_VARS, + isCaseVaryingEnvVarName, countWindowsExecutableExtensions, isPathReturningCall, isPosixSlashStringLiteral, diff --git a/eslint-rules/no-exact-case-env-access.cjs b/eslint-rules/no-exact-case-env-access.cjs new file mode 100644 index 000000000..95b59cbb4 --- /dev/null +++ b/eslint-rules/no-exact-case-env-access.cjs @@ -0,0 +1,216 @@ +'use strict'; + +const path = require('node:path'); +const { isCaseVaryingEnvVarName } = require('./lib/portability-vocab.cjs'); + +/** + * no-exact-case-env-access + * + * `process.env` is a case-insensitive Proxy on every platform, so + * `process.env.PATH` (or `process.env['PATH']`) is always safe even though + * Windows itself ships several of these variables under a different + * conventional casing (`Path`, `ComSpec`, ...). The moment that value is + * copied into, or read through, ANY other object — a spread of + * `process.env`, a function parameter, a destructure with no traceable + * source — the exact-case Windows spelling is what survives, and an + * uppercase POSIX-style lookup against it silently resolves to `undefined` + * on Windows. + * + * This rule flags two shapes reading any casing of any name in + * WINDOWS_CASE_VARYING_ENV_VARS off a receiver that is not `process.env` + * itself: + * + * 1. MemberExpression reads — `env.PATH`, `opts.env['ComSpec']` — but NOT + * `process.env.PATH` / `process.env['PATH']`. + * 2. ObjectPattern destructuring — `const { PATH } = env` — but NOT + * `const { PATH } = process.env`. + * + * The `process.env` check (isProcessEnvExpression) is deliberately + * syntactic-only, not flow-sensitive: it recognizes exactly `process.env` + * and `process['env']` at the access site, one level deep, with no alias + * tracking (`const env = process.env; env.PATH` is NOT recognized as safe + * and WILL be flagged — route through envGet or destructure directly from + * process.env instead). + * + * MemberExpression matching requires an "env-shaped" receiver (see + * isEnvShapedExpression) for BOTH notations, dot and bracket alike, because + * a first pass that flagged any `.` where `` is + * vocab-matching produced 113 false positives — ordinary lowercase property + * access like `config.path` or `entry.path` collides with the + * (case-insensitive) vocab list — and a later pass that special-cased + * bracket form to report unconditionally reintroduced the same class of + * false positive (`artifact['path']` on an unrelated `Record` is not an env read just because the key string matches): + * + * - Dot form (`X.PATH`, non-computed) and bracket form (`X['PATH']`, + * computed + string Literal) are both reported ONLY when the receiver + * `X` is "env-shaped" — i.e. an identifier literally named `env` (any + * casing) or a MemberExpression whose property resolves to `env` (any + * casing), such as `opts.env.PATH` / `opts.env['ComSpec']`. Neither + * notation is precise enough on its own risk-wise; the receiver check + * is what keeps `config.path`, `entry.path`, `artifact['path']`, etc. + * unflagged while still catching the real risk shapes. + * + * ObjectPattern destructuring gets the same dot-notation-style restriction + * for symmetry: `const { PATH } = env` / `const { PATH } = opts.env` are + * flagged, but `const { path } = someConfigObject` is not, because the + * traced source there is not env-shaped. + * + * The seam exemption is PATH-SUFFIX ANCHORED, not substring-matched — see + * isSeamFile in no-private-binary-resolution.cjs for the identical logic and + * rationale (case I9 pins the distinction there). + */ + +const SEAM_RELATIVE_PATH = 'src/shell-command-projection.cts'; + +/** + * True when `filename` IS the seam file, matched by path SUFFIX after + * normalizing separators to `/` — never by substring containment anywhere + * else in the path. + * + * @param {string} filename + * @returns {boolean} + */ +function isSeamFile(filename) { + if (typeof filename !== 'string' || filename.length === 0) return false; + const normalized = filename.split(path.sep).join('/'); + return normalized === SEAM_RELATIVE_PATH || normalized.endsWith(`/${SEAM_RELATIVE_PATH}`); +} + +/** + * True when `node` is the expression `process.env` (non-computed, property + * is Identifier `env`) or `process['env']` (computed, property is a string + * Literal `'env'`). Nothing else counts: no alias tracking, no deeper + * unwrapping — this is a deliberate, documented limit of the rule. + * + * @param {import('eslint').Rule.Node} node + * @returns {boolean} + */ +function isProcessEnvExpression(node) { + if (!node || node.type !== 'MemberExpression') return false; + if (node.object.type !== 'Identifier' || node.object.name !== 'process') return false; + if (!node.computed) { + return node.property.type === 'Identifier' && node.property.name === 'env'; + } + return node.property.type === 'Literal' && node.property.value === 'env'; +} + +/** + * True when `node` is "env-shaped": a receiver whose own name/property is an + * exact case-insensitive match to `env`, one level deep, no fuzzy/substring + * matching. Matches: + * + * - Identifier `env` / `Env` / `ENV` (a bare env parameter or variable). + * - MemberExpression `X.env` / `X.Env` / `X['env']` / `X['Env']` (property + * resolves, case-insensitively, to the literal name `env`). + * + * Anything else — including a MemberExpression whose OBJECT is further + * env-shaped (`a.b.env.PATH`) — is not unwrapped further; this is a + * deliberate one-level-deep limit, matching isProcessEnvExpression's own + * documented limit. + * + * @param {import('eslint').Rule.Node} node + * @returns {boolean} + */ +function isEnvShapedExpression(node) { + if (!node) return false; + if (node.type === 'Identifier') { + return typeof node.name === 'string' && node.name.toLowerCase() === 'env'; + } + if (node.type === 'MemberExpression') { + if (!node.computed) { + return node.property.type === 'Identifier' && node.property.name.toLowerCase() === 'env'; + } + return ( + node.property.type === 'Literal' && + typeof node.property.value === 'string' && + node.property.value.toLowerCase() === 'env' + ); + } + return false; +} + +/** + * Extracts the statically-known accessed/destructured name from a + * MemberExpression's property or an ObjectPattern Property's key. + * + * - non-computed Identifier (`env.PATH`, `{ PATH: v }`): use `.name`. + * - Literal string key/property, computed OR non-computed + * (`env['PATH']`, `{ ['PATH']: v }`, and — non-computed only for + * ObjectPattern keys — `{ 'PATH': v }`): use `.value`. + * - a computed variable expression (`env[key]`) is NOT statically + * decidable and returns `null`. + * + * A MemberExpression's non-computed property is ALWAYS an Identifier by JS + * grammar (`obj.'PATH'` is not valid syntax), so accepting a non-computed + * Literal only changes behavior for ObjectPattern keys, where both + * `{ PATH: v }` (Identifier) and `{ 'PATH': v }` (Literal) are valid + * non-computed forms. + * + * @param {boolean} computed + * @param {import('eslint').Rule.Node} node - the `property` node (MemberExpression) or `key` node (Property) + * @returns {string | null} + */ +function extractStaticName(computed, node) { + if (node.type === 'Identifier' && !computed) return node.name; + if (node.type === 'Literal' && typeof node.value === 'string') return node.value; + return null; +} + +/** @type {import('eslint').Rule.RuleModule} */ +const rule = { + meta: { + type: 'problem', + docs: { + description: 'Disallow exact-case environment variable access off a non-process.env receiver', + category: 'Portability', + }, + schema: [], + messages: { + exactCaseEnvRead: + 'Reading "{{name}}" is an exact-case environment lookup off a non-process.env object — ' + + 'Windows renames env vars (Path, ComSpec, ...) and only the process.env proxy is ' + + 'case-insensitive. Route through envGet(env, name) in src/shell-command-projection.cts, ' + + 'or destructure directly from process.env.', + }, + }, + + create(context) { + const filename = typeof context.filename === 'string' ? context.filename : context.getFilename(); + if (isSeamFile(filename)) return {}; + + return { + MemberExpression(node) { + if (isProcessEnvExpression(node.object)) return; + const name = extractStaticName(node.computed, node.property); + if (name === null || !isCaseVaryingEnvVarName(name)) return; + + if (isEnvShapedExpression(node.object)) { + context.report({ node, messageId: 'exactCaseEnvRead', data: { name } }); + } + }, + + ObjectPattern(node) { + for (const property of node.properties) { + if (property.type !== 'Property') continue; + const name = extractStaticName(property.computed, property.key); + if (name === null || !isCaseVaryingEnvVarName(name)) continue; + + let source = null; + const parent = node.parent; + if (parent && parent.type === 'VariableDeclarator' && parent.id === node) { + source = parent.init; + } else if (parent && parent.type === 'AssignmentExpression' && parent.left === node) { + source = parent.right; + } + + if (source && isProcessEnvExpression(source)) continue; + if (!source || !isEnvShapedExpression(source)) continue; + context.report({ node: property, messageId: 'exactCaseEnvRead', data: { name } }); + } + }, + }; + }, +}; + +module.exports = rule; diff --git a/eslint.config.mjs b/eslint.config.mjs index fda7b6c3c..43666016e 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -32,6 +32,7 @@ import requireSubprocessTimeout from './eslint-rules/require-subprocess-timeout. import noExternalRequireInBin from './eslint-rules/no-external-require-in-bin.cjs'; import noPrivateBinaryResolution from './eslint-rules/no-private-binary-resolution.cjs'; import requireRegisteredExit from './eslint-rules/require-registered-exit.cjs'; +import noExactCaseEnvAccess from './eslint-rules/no-exact-case-env-access.cjs'; const localPlugin = { rules: { @@ -58,6 +59,7 @@ const localPlugin = { 'no-external-require-in-bin': noExternalRequireInBin, 'no-private-binary-resolution': noPrivateBinaryResolution, 'require-registered-exit': requireRegisteredExit, + 'no-exact-case-env-access': noExactCaseEnvAccess, }, }; @@ -424,6 +426,9 @@ export default tseslint.config( // eslint-ignored (ADR-457), so a rule registered only on the emitted // surface never sees the real .cts sources (#3496). 'local/require-registered-exit': 'error', + // #3624 (epic #3411 Phase 4): flag an exact-case env-var read off a + // non-process.env receiver. See CONTEXT.md DEFECT.WINDOWS-EXACT-CASE-ENV-ACCESS. + 'local/no-exact-case-env-access': 'error', }, }, @@ -544,6 +549,8 @@ export default tseslint.config( 'local/no-private-binary-resolution': 'error', // #3910 (epic #3889 Phase 6): see the src/**/*.cts block above for detail. 'local/require-registered-exit': 'error', + // #3624: see the src/**/*.cts block above for detail. + 'local/no-exact-case-env-access': 'error', }, }, @@ -573,6 +580,8 @@ export default tseslint.config( 'local/no-private-binary-resolution': 'error', // #3910 (epic #3889 Phase 6): see the src/**/*.cts block above for detail. 'local/require-registered-exit': 'error', + // #3624: see the src/**/*.cts block above for detail. + 'local/no-exact-case-env-access': 'error', }, }, @@ -599,6 +608,8 @@ export default tseslint.config( // (n/no-process-exit: 'off') is now dead — see the src/**/*.cts block // above for detail on the rule itself. 'local/require-registered-exit': 'error', + // #3624: see the src/**/*.cts block above for detail. + 'local/no-exact-case-env-access': 'error', }, }, diff --git a/src/runtime-hooks-surface.cts b/src/runtime-hooks-surface.cts index dd48a86f1..8f8208658 100644 --- a/src/runtime-hooks-surface.cts +++ b/src/runtime-hooks-surface.cts @@ -459,8 +459,9 @@ function normalizeNodePath(execPath: string, opts?: NodeNormOpts): string { candidates.push(`${fnmRoot}/aliases/default/node.exe`); candidates.push(`${fnmRoot}/aliases/default/bin/node`); } - if (env.APPDATA) { - candidates.push(`${normalizeRootDir(env.APPDATA)}/fnm/aliases/default/node.exe`); + const appdata = shellCmdProjection.envGet(env, 'APPDATA'); + if (appdata) { + candidates.push(`${normalizeRootDir(appdata)}/fnm/aliases/default/node.exe`); } for (const candidate of candidates) { if (candidate && existsSync(candidate)) return candidate; diff --git a/src/shell-command-projection.cts b/src/shell-command-projection.cts index 535672525..277630d34 100644 --- a/src/shell-command-projection.cts +++ b/src/shell-command-projection.cts @@ -715,8 +715,12 @@ function _isFile(candidate: string, requireExecutable = false, platform: string * * Exact match wins when present, so a caller who sets the canonical name pays * no scan. + * + * Exported so callers outside this seam can route an exact-case-sensitive env + * read through it instead of accessing a non-`process.env` object directly + * (see `local/no-exact-case-env-access`). */ -function _envGet(env: NodeJS.ProcessEnv, name: string): string | undefined { +export function envGet(env: NodeJS.ProcessEnv, name: string): string | undefined { const exact = env[name]; if (exact !== undefined) return exact; const lower = name.toLowerCase(); @@ -818,7 +822,7 @@ export function resolveExecutableBinary( return _isFile(name, requireExecutable, platform) ? name : null; } const env = opts.env ?? process.env; - const rawPath = opts.pathOverride !== undefined ? opts.pathOverride : (_envGet(env, 'PATH') || ''); + const rawPath = opts.pathOverride !== undefined ? opts.pathOverride : (envGet(env, 'PATH') || ''); const pathSegments = String(rawPath).split(path.delimiter).filter(Boolean); const segments = [...(opts.prependPaths ?? []), ...pathSegments]; @@ -830,7 +834,7 @@ export function resolveExecutableBinary( return null; } - const exts = String(_envGet(env, 'PATHEXT') || DEFAULT_PATHEXT).split(';').filter(Boolean); + const exts = String(envGet(env, 'PATHEXT') || DEFAULT_PATHEXT).split(';').filter(Boolean); // A name already ending in a PATHEXT-listed extension is an address, not a stem: // probing `foo.exe` as `foo.exe.EXE` would miss the file sitting right there. // Compared case-insensitively because PATHEXT casing is not guaranteed. @@ -935,7 +939,7 @@ export function projectSpawnInvocation( return resolved ? { command: resolved, args, resolved } : { command, args, resolved: null }; } return { - command: String(_envGet(env, 'ComSpec') || 'cmd.exe'), + command: String(envGet(env, 'ComSpec') || 'cmd.exe'), args: ['/d', '/s', '/c', _buildVerbatimCmdLine(target, args)], resolved, windowsVerbatimArguments: true, diff --git a/tests/no-exact-case-env-access.rule.test.cjs b/tests/no-exact-case-env-access.rule.test.cjs new file mode 100644 index 000000000..1944924f5 --- /dev/null +++ b/tests/no-exact-case-env-access.rule.test.cjs @@ -0,0 +1,333 @@ +'use strict'; + +/** + * no-exact-case-env-access.rule.test.cjs + * + * RuleTester unit tests for the local/no-exact-case-env-access ESLint rule. + * Ids (V1-V14, I1-I8) map to the case list in the #3624 dispatch brief. + * + * RuleTester feeds fixtures to the rule directly and does not scan this test + * file's own source, so the self-flagging problem that forced eslint.config.mjs + * to carve the eslint-rules directory out of the scripts .cjs block does not + * arise here (ADR-1703 rule 5). + */ + +const { test, describe } = require('node:test'); +const assert = require('node:assert/strict'); +const { RuleTester } = require('eslint'); + +const rule = require('../eslint-rules/no-exact-case-env-access.cjs'); + +const ruleTester = new RuleTester({ + languageOptions: { + ecmaVersion: 2022, + sourceType: 'commonjs', + }, +}); + +const OUTSIDE_SEAM_FILE = 'src/some-other-module.cts'; +const SEAM_FILE = 'src/shell-command-projection.cts'; + +// ─── module shape ───────────────────────────────────────────────────────────── + +describe('no-exact-case-env-access rule module', () => { + test('exports meta and create', () => { + assert.strictEqual(typeof rule.meta, 'object'); + assert.strictEqual(typeof rule.create, 'function'); + assert.strictEqual(rule.meta.type, 'problem'); + assert.ok(rule.meta.messages.exactCaseEnvRead, 'exactCaseEnvRead message must exist'); + }); +}); + +// ─── VALID cases (V1-V14) ─────────────────────────────────────────────────── + +describe('no-exact-case-env-access: valid cases', () => { + test('V1: process.env.PATH', () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const p = process.env.PATH;`, + filename: OUTSIDE_SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test("V2: process.env['PATH']", () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const p = process.env['PATH'];`, + filename: OUTSIDE_SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test("V3: process['env'].PATH", () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const p = process['env'].PATH;`, + filename: OUTSIDE_SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test("V4: process['env']['PATH']", () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const p = process['env']['PATH'];`, + filename: OUTSIDE_SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test('V5: const { PATH } = process.env;', () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const { PATH } = process.env;`, + filename: OUTSIDE_SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test('V6: config.path — unrelated object, dot, lowercase path (real false-positive class)', () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const p = config.path;`, + filename: OUTSIDE_SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test("V7: artifact['path'] — unrelated object, bracket, lowercase path (real false-positive class)", () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const p = artifact['path'];`, + filename: OUTSIDE_SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test('V8: const { path } = someConfig; — destructuring an unrelated field from a non-env-shaped source', () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const { path } = someConfig;`, + filename: OUTSIDE_SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test('V9: env[someVar] — computed non-literal key, not statically decidable', () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const v = env[someVar];`, + filename: OUTSIDE_SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test('V10: opts.env.NODE_ENV — env-shaped receiver but name outside the vocab', () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const v = opts.env.NODE_ENV;`, + filename: OUTSIDE_SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test('V11: process.env.PATHEXT in src/shell-command-projection.cts — seam exemption', () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const ext = process.env.PATHEXT;`, + filename: SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test("V12: opts.env['PATH'] in src/shell-command-projection.cts — seam exemption applies even to a real violation shape", () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const p = opts.env['PATH'];`, + filename: SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test("V13: merged['PATH'] where merged is a bare identifier not named env — documented accepted false-negative", () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `merged['PATH'];`, + filename: OUTSIDE_SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test('V14: const { [key]: v } = opts.env; — computed non-literal destructuring key', () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const { [key]: v } = opts.env;`, + filename: OUTSIDE_SEAM_FILE, + }, + ], + invalid: [], + }); + }); + + test('V15: envGet(env, "PATH") — the case-insensitive accessor call itself is valid', () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [ + { + code: `const p = envGet(env, 'PATH');`, + filename: OUTSIDE_SEAM_FILE, + }, + ], + invalid: [], + }); + }); +}); + +// ─── INVALID cases (I1-I8) ────────────────────────────────────────────────── + +describe('no-exact-case-env-access: invalid cases', () => { + test("I1: opts.env['PATH'] — 1 error", () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [], + invalid: [ + { + code: `const p = opts.env['PATH'];`, + filename: OUTSIDE_SEAM_FILE, + errors: [{ messageId: 'exactCaseEnvRead' }], + }, + ], + }); + }); + + test('I2: opts.env.pathext — lowercase dot form — 1 error', () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [], + invalid: [ + { + code: `const ext = opts.env.pathext;`, + filename: OUTSIDE_SEAM_FILE, + errors: [{ messageId: 'exactCaseEnvRead' }], + }, + ], + }); + }); + + test("I3: env['Pathext'] — bare env-named identifier, mixed case — 1 error", () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [], + invalid: [ + { + code: `const ext = env['Pathext'];`, + filename: OUTSIDE_SEAM_FILE, + errors: [{ messageId: 'exactCaseEnvRead' }], + }, + ], + }); + }); + + test("I4: const env = { ...process.env, ...opts.env }; env['PATH']; — spread-derived, read via env-shaped identifier — 1 error total", () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [], + invalid: [ + { + code: `const env = { ...process.env, ...opts.env }; env['PATH'];`, + filename: OUTSIDE_SEAM_FILE, + errors: 1, + }, + ], + }); + }); + + test('I5: const { PATHEXT } = opts.env; — 1 error', () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [], + invalid: [ + { + code: `const { PATHEXT } = opts.env;`, + filename: OUTSIDE_SEAM_FILE, + errors: [{ messageId: 'exactCaseEnvRead' }], + }, + ], + }); + }); + + test('I6: const { PATHEXT: exts } = env; — renamed destructuring from a bare env identifier — 1 error', () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [], + invalid: [ + { + code: `const { PATHEXT: exts } = env;`, + filename: OUTSIDE_SEAM_FILE, + errors: [{ messageId: 'exactCaseEnvRead' }], + }, + ], + }); + }); + + test('I7: dot-form AND bracket-form violation in one fixture — 2 errors', () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [], + invalid: [ + { + code: `const a = opts.env.PATH; const b = opts.env['PATHEXT'];`, + filename: OUTSIDE_SEAM_FILE, + errors: 2, + }, + ], + }); + }); + + test("I8: const { 'PATH': v } = opts.env; — non-computed string-literal destructuring key — 1 error", () => { + ruleTester.run('no-exact-case-env-access', rule, { + valid: [], + invalid: [ + { + code: `const { 'PATH': v } = opts.env;`, + filename: OUTSIDE_SEAM_FILE, + errors: [{ messageId: 'exactCaseEnvRead' }], + }, + ], + }); + }); +}); + diff --git a/tests/portability-vocab-drift.test.cjs b/tests/portability-vocab-drift.test.cjs index 983d58dfc..f5287e6b2 100644 --- a/tests/portability-vocab-drift.test.cjs +++ b/tests/portability-vocab-drift.test.cjs @@ -23,7 +23,7 @@ const path = require('path'); const tsParser = require('@typescript-eslint/parser'); const espree = require('espree'); -const { PATH_RETURNING_FNS } = require('../eslint-rules/lib/portability-vocab.cjs'); +const { PATH_RETURNING_FNS, WINDOWS_CASE_VARYING_ENV_VARS } = require('../eslint-rules/lib/portability-vocab.cjs'); // Functions exported from runtime-homes.cts that do NOT return a filesystem // path and therefore are intentionally excluded from PATH_RETURNING_FNS. @@ -477,6 +477,73 @@ describe('portability-vocab drift guard', () => { }); }); +describe('exact-case-env-access vocab drift guard', () => { + test('WINDOWS_CASE_VARYING_ENV_VARS is a non-empty array', () => { + assert.ok(Array.isArray(WINDOWS_CASE_VARYING_ENV_VARS)); + assert.ok(WINDOWS_CASE_VARYING_ENV_VARS.length > 0); + }); + + test('every literal env-var name passed to envGet(...) in the seam is registered in WINDOWS_CASE_VARYING_ENV_VARS', () => { + const srcPath = path.join(__dirname, '..', 'src', 'shell-command-projection.cts'); + const src = fs.readFileSync(srcPath, 'utf-8'); + + const ast = tsParser.parse(src, { + jsx: false, + loc: true, + range: true, + comment: true, + tokens: false, + }); + + // Collect the second-argument string literal of every `envGet(...)` call + // site anywhere in the file (no special-casing of nesting — walk everything). + const collected = []; + function walk(n) { + if (!n || typeof n !== 'object') return; + if ( + n.type === 'CallExpression' && + n.callee && + n.callee.type === 'Identifier' && + n.callee.name === 'envGet' && + Array.isArray(n.arguments) && + n.arguments[1] && + n.arguments[1].type === 'Literal' && + typeof n.arguments[1].value === 'string' + ) { + collected.push(n.arguments[1].value); + } + for (const key of Object.keys(n)) { + if (key === 'parent' || key === 'tokens' || key === 'comments' || key === 'loc' || key === 'range') continue; + const child = n[key]; + if (Array.isArray(child)) { + for (const item of child) { + if (item && typeof item === 'object' && item.type) walk(item); + } + } else if (child && typeof child === 'object' && child.type) { + walk(child); + } + } + } + walk(ast); + + // Guard against the walker itself being broken (a silent 0-hit pass would + // make this test vacuously true). + assert.ok( + collected.length > 0, + `Expected to find at least one envGet(...) call site with a literal name in ${srcPath}, found none — the AST walker may be broken.`, + ); + + const vocabLower = new Set(WINDOWS_CASE_VARYING_ENV_VARS.map((v) => v.toLowerCase())); + const missing = collected.filter((name) => !vocabLower.has(name.toLowerCase())); + + assert.deepStrictEqual( + missing, + [], + `These envGet(...) literal env-var names in src/shell-command-projection.cts are missing from WINDOWS_CASE_VARYING_ENV_VARS:\n ${missing.join('\n ')}\n\nAdd them to WINDOWS_CASE_VARYING_ENV_VARS in eslint-rules/lib/portability-vocab.cjs.`, + ); + }); +}); + // ─── Known boundary (documented, not enforced) ───────────────────────────────── // // A NEW path-returning resolver added to bin/install.js that builds its path via