enhance(#3624): local/no-exact-case-env-access — ratchet ADR-1703 onto production env reads (epic #3411 Phase 4) (#3976)
* enhance(#3624): local/no-exact-case-env-access — ratchet ADR-1703 onto production env reads (epic #3411 Phase 4) Extends ADR-1703's portability rule catalog with a second production-runtime rule: it flags an exact-case read of a Windows case-varying environment variable (PATH, PATHEXT, ComSpec, USERPROFILE, TEMP, TMP, APPDATA) off any receiver that is not process.env itself, matched via an env-shaped-receiver check to avoid colliding with ordinary `.path`-named properties elsewhere in the tree. Exports the seam's private `_envGet` as `envGet` so the rule's remediation message names a real helper, and fixes the one pre-existing violation the tightened rule found (`src/runtime-hooks-surface.cts`'s `env.APPDATA` read). Closes #3624 * fix(#3624): extractStaticName recognizes non-computed Literal destructuring keys; add missing accessor-call test case Review findings from the code-review + isolated-adversarial passes: - extractStaticName only matched non-computed Identifier keys, so a destructuring like `const { 'PATH': v } = opts.env;` (the issue's own I8 acceptance case) silently evaded the rule. Widened to accept a Literal key regardless of computed, which is safe for MemberExpression too (its non-computed property is always an Identifier by grammar). - Added the missing RuleTester valid case for "a case-insensitive accessor call" (envGet(env, 'PATH')) from the issue's Done-when checklist. * docs: backfill changeset PR number for #3624 (PR #3976) --------- Co-authored-by: sim <sim@local>
This commit is contained in:
@@ -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 `<expr>.env` / `<expr>['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`
|
||||
|
||||
Reference in New Issue
Block a user