From 3fac2e5766333dc04aa612331d43924d7ea61eda Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Thu, 29 Oct 2026 01:11:37 -0700 Subject: [PATCH 1/5] Alias a value held from process.env to the live environment `const env = process.env` compiled the hold to a snapshot record copy of the environment, so `env.X = "1"` wrote the copy and was lost while Node's write reached the real environment table (undefined natively, "1" in Node; `repros/d3-env-write.ts`). Node's held value IS the environment object. A hold whose type is the env record now aliases the live table for every member access: dotted and computed reads lower to process.envGet, writes to process.envSet, `delete` to process.envUnset, and the absence probe (`env[k] ?? fallback`, `||`, `if`) reads getenv instead of reconstructing presence over the snapshot. A BARE use of the hold (spread, Object.keys, argument, return) takes a fresh %env.snapshot at that site, so it observes the environment as it is there rather than as it was when the binding was initialized. Two holds keep copy semantics because they cannot alias safely, and a write through either is refused with SC2014 rather than lost: a hold that escapes into another function, and a hold whose declared type is not the env record. Compound and update writes through a hold (`env.X += v`, `env.X++`) are refused the same way. tests/corpus/held-env-value-writes.ts covers writes, computed writes, deletes, child inheritance, direct writes seen through the hold, a copy staying a copy, and bare-use re-snapshotting. Co-Authored-By: Claude Opus 5.5 --- packages/compiler/src/diagnostics/codes.ts | 1 + .../compiler/src/diagnostics/diagnostic.ts | 20 ++++ .../frontend/lowering/builtins/environment.ts | 108 +++++++++++++++++- .../lowering/expressions/element-access.ts | 30 ++++- .../lowering/expressions/identifiers.ts | 45 ++++++++ .../lowering/expressions/property-access.ts | 16 +++ .../src/frontend/lowering/lower-exprs.ts | 7 ++ .../src/frontend/lowering/lower-stmts.ts | 23 +++- .../compiler/src/frontend/lowering/lowerer.ts | 8 +- .../corpus/held-env-value-writes.ts.json | 6 + tests/corpus/held-env-value-writes.ts | 58 ++++++++++ .../held-env-value-writes.ts.json | 1 + 12 files changed, 314 insertions(+), 9 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/held-env-value-writes.ts.json create mode 100644 tests/corpus/held-env-value-writes.ts create mode 100644 tests/harness/corpus-costs/held-env-value-writes.ts.json diff --git a/packages/compiler/src/diagnostics/codes.ts b/packages/compiler/src/diagnostics/codes.ts index f3d7681d5..26839425b 100644 --- a/packages/compiler/src/diagnostics/codes.ts +++ b/packages/compiler/src/diagnostics/codes.ts @@ -57,6 +57,7 @@ export const DIAGNOSTIC_CODES = { SC2011: "'any'-typed values and their operations", SC2012: "standard-library form without native lowering", SC2013: "npm package import or value without native lowering", + SC2014: "write through a value held from process.env", SC2020: "standard-library or @types/node surface with no lowering", SC3001: "LLVM IR tier refusal", SC3002: "target or capability refusal", diff --git a/packages/compiler/src/diagnostics/diagnostic.ts b/packages/compiler/src/diagnostics/diagnostic.ts index 0dfd8bd02..a13d237d5 100644 --- a/packages/compiler/src/diagnostics/diagnostic.ts +++ b/packages/compiler/src/diagnostics/diagnostic.ts @@ -768,6 +768,10 @@ export const FENCE_CODES: Record= 4) return false; + const symbol = lowerer.checker.getSymbolAtLocation(inner); + const decl = symbol?.valueDeclaration ?? symbol?.declarations[0]; + if (decl === undefined) return false; + // `const env = process.env` (or an identifier alias of one). The symbol's + // value declaration is the `const` itself; the written identifier here is + // a different node than the declared name, so no node identity check. + if (ts.isVariableDeclaration(decl)) { + return holdsEnvSnapshot(lowerer, decl.initializer, depth + 1); + } + // `const { env } = process` (also `{ env: held }`). + if (ts.isBindingElement(decl) && ts.isObjectBindingPattern(decl.parent)) { + const declaration = decl.parent.parent; + const named = decl.propertyName ?? decl.name; + if ( + ts.isVariableDeclaration(declaration) && + ts.isIdentifier(named) && + named.text === "env" && + declaration.initializer !== undefined + ) { + return lowerer.isStdlibGlobal(declaration.initializer, "process"); + } + } + return false; +} + +/** The receiver expression of a write target — the assignment's or delete's + * access (`env.X`, `env[k]`), through any compound/update wrapping. Null for + * targets that write no entry of a receiver. */ +function writeTargetReceiver(node: ts.Node): ts.Expression | null { + if (ts.isBinaryExpression(node)) return writeTargetReceiver(node.left); + if (ts.isPrefixUnaryExpression(node) || ts.isPostfixUnaryExpression(node)) { + return writeTargetReceiver(node.operand); + } + if (ts.isPropertyAccessExpression(node) || ts.isElementAccessExpression(node)) { + return node.expression; + } + return null; +} + +/** Whether a MEMBER access (`env.X`, `env[k]`, and their writes) through a + * value HELD from `process.env` can go to the live environment table: the + * held binding's type must be exactly the env snapshot record (a pure + * index-signature shape whose value slot is `string | undefined`, what + * `process.env` maps to). Returns that value type, or null — a hold whose + * type differs keeps the snapshot record for reads and refuses writes (see + * refuseHeldEnvWrite). Member accesses alias the live table the way Node's + * do. A BARE use of the held value (a spread source, Object.keys, an + * argument, a return) is not a member access and takes a fresh snapshot of + * the environment at that use site (lowerIdentifierExpr). */ +export function heldEnvValueType(lowerer: Lowerer, receiver: ts.Expression): IrType | null { + if (!holdsEnvSnapshot(lowerer, receiver)) return null; + const mapped = lowerer.mapTypeOf(lowerer.typeOf(receiver)); + if (mapped?.kind !== "record") return null; + const shape = lowerer.shapes.get(mapped.shapeId); + if (!shape || shape.tuple || shape.fields.length > 0 || shape.indexValue === undefined) + return null; + const value = lowerer.envValueType(); + return typeEquals(shape.indexValue, value) ? value : null; +} + +/** Refuse a write through a value HELD from `process.env` that cannot take + * the live path: the compound and update forms (`env.X += v`, `env.X++`) + * reach this wrapper, and so does any write through a hold whose type is + * not the env record. The held value is a snapshot record, so such a write + * would land on the copy while the real environment kept its old value; + * Node writes setenv(3) through the very same object, so a program relying + * on it would silently diverge. Plain `=`, computed-key and `delete` writes + * through the env record alias the live table instead (lower-stmts.ts, + * element-access.ts); `target` is the write's or delete's access node. */ +export function refuseHeldEnvWrite(lowerer: Lowerer, target: ts.Node): void { + const receiver = writeTargetReceiver(target); + if (receiver === null || !holdsEnvSnapshot(lowerer, receiver)) return; + lowerer.pushDiag(heldEnvWriteDiag(locOf(target))); + throw new PoisonError(); +} + /** The interned `string | undefined` union — the type every env read * produces (withUndefinedArm interns [string, undefined] in canonical * order, so the string arm's tag is 0 and the undefined arm's is 1, diff --git a/packages/compiler/src/frontend/lowering/expressions/element-access.ts b/packages/compiler/src/frontend/lowering/expressions/element-access.ts index 5cb9776f6..488bde997 100644 --- a/packages/compiler/src/frontend/lowering/expressions/element-access.ts +++ b/packages/compiler/src/frontend/lowering/expressions/element-access.ts @@ -32,6 +32,7 @@ import { unionContainerArmsOk, } from "../../../ir/ir.js"; import { importMetaField, isImportMeta } from "../import-meta.js"; +import { heldEnvValueType } from "../builtins/environment.js"; import { hasOptionalChainGuard, isOptionalChainTail } from "./optional-chains.js"; import { lowerEnumAccess } from "../lower-enums.js"; import { fenceNodeModuleMutation, lowerRequireCacheElement } from "../lower-node-module.js"; @@ -234,6 +235,25 @@ export function lowerElementAccess(lowerer: Lowerer, expr: ts.ElementAccessExpre }; return lowerer.maybeNarrow(get, expr); } + // `held[expr]` for a value HELD from process.env (`const env = + // process.env`): the hold aliases the live table, so the read is the same + // getenv the direct spelling uses. + { + const live = heldEnvValueType(lowerer, expr.expression); + if (live !== null) { + const key = lowerEnvironmentKey(lowerer, expr.argumentExpression); + if (key.type.kind === "string") { + const get: IrExpr = { + kind: "libCall", + fn: "process.envGet", + args: [key], + type: live, + loc: locOf(expr), + }; + return lowerer.maybeNarrow(get, expr); + } + } + } // Expando function members in element clothing (`foo[strMem]`, // `foo[_private]` where foo is a module-level function/callable const // and the key folds or is a unique-symbol const): the member's module @@ -2217,8 +2237,14 @@ export function lowerElementWrite(lowerer: Lowerer, expr: ts.BinaryExpression): }; } // `process.env[key] = v` — the computed twin of the dotted env write: - // setenv(3), string keys and string values only. - if (lowerer.isProcessEnv(target.expression) && !target.questionDotToken) { + // setenv(3), string keys and string values only. A key write through a + // value HELD from process.env takes the same path: the hold aliases the + // live table. + if ( + !target.questionDotToken && + (lowerer.isProcessEnv(target.expression) || + heldEnvValueType(lowerer, target.expression) !== null) + ) { const loc = locOf(expr); const key = lowerEnvironmentKey(lowerer, target.argumentExpression); if (key.type.kind !== "string") { diff --git a/packages/compiler/src/frontend/lowering/expressions/identifiers.ts b/packages/compiler/src/frontend/lowering/expressions/identifiers.ts index ebe7e2556..35f04de98 100644 --- a/packages/compiler/src/frontend/lowering/expressions/identifiers.ts +++ b/packages/compiler/src/frontend/lowering/expressions/identifiers.ts @@ -95,6 +95,33 @@ import { fenceEnumObjectValue } from "../lower-enums.js"; import { mixinFnOfCallee } from "../lower-mixins.js"; import { isNavigatorGlobal, navigatorValue } from "../builtins/navigator.js"; import { moduleFileName } from "../import-meta.js"; +import { envSnapshotHelper, heldEnvValueType } from "../builtins/environment.js"; + +/** Whether this identifier is the RECEIVER of a member access — the dotted + * or bracketed `held.X` / `held[k]`, read or written, through any erasure + * wrapper. Those uses answer through the live environment table themselves + * (heldEnvValueType); every other use of a held value is a bare one and + * reads the environment as it is at that site. */ +function memberReceiverUse(expr: ts.Identifier): boolean { + let node: ts.Node = expr; + for (;;) { + const parent: ts.Node | undefined = node.parent; + if (parent === undefined) return false; + if ( + ts.isParenthesizedExpression(parent) || + ts.isAsExpression(parent) || + ts.isNonNullExpression(parent) || + ts.isSatisfiesExpression(parent) + ) { + node = parent; + continue; + } + return ( + (ts.isPropertyAccessExpression(parent) || ts.isElementAccessExpression(parent)) && + parent.expression === node + ); + } +} export function lowerIdentifierExpr(lowerer: Lowerer, expr: ts.Identifier, loc: SrcLoc): IrExpr { if (lowerer.isStdlibGlobal(expr, "console")) { @@ -183,6 +210,24 @@ export function lowerIdentifierExpr(lowerer: Lowerer, expr: ts.Identifier, loc: // when the checker types this USE as a single arm, maybeNarrow // bridges the tagged representation with a unionNarrow. const local = lowerer.resolveLocal(expr); + // A BARE use of a value HELD from process.env (`{ ...env }`, Object.keys, + // an argument, a return) reads the environment as it is HERE: Node's hold + // IS the environment object, so its contents at use time are the live + // table's. Member accesses (`env.X`, `env[k]`, their writes) reroute to + // getenv/setenv directly (heldEnvValueType) and never come through a bare + // read, so they materialize no snapshot; every other use takes a fresh + // one at this site instead of the copy the hold was given. + { + const mapped = lowerer.mapTypeOf(lowerer.typeOf(expr)); + if ( + mapped?.kind === "record" && + !memberReceiverUse(expr) && + heldEnvValueType(lowerer, expr) !== null + ) { + const helper = envSnapshotHelper(lowerer, mapped.shapeId, loc); + if (helper !== null) return { kind: "call", callee: helper, args: [], type: mapped, loc }; + } + } // A Date local holding its time value reads as that Date: only uses // that read the time value reach here (date-values.ts). if (local && lowerer.timeValueDateLocals.has(local)) { diff --git a/packages/compiler/src/frontend/lowering/expressions/property-access.ts b/packages/compiler/src/frontend/lowering/expressions/property-access.ts index 3bcaf1013..6921c9a9e 100644 --- a/packages/compiler/src/frontend/lowering/expressions/property-access.ts +++ b/packages/compiler/src/frontend/lowering/expressions/property-access.ts @@ -209,6 +209,7 @@ import { isZlibStreamMember, lowerZlibStreamBytesWritten } from "../builtins/com import { isWebCryptoReceiver } from "../builtins/crypto.js"; import { isFsStreamMember, lowerFsStreamProperty } from "../builtins/fs-streams.js"; import { keyedFieldRead } from "./keyed-field-targets.js"; +import { heldEnvValueType } from "../builtins/environment.js"; import { promisePrototypeValue } from "../thenables.js"; import { lowerChildPropertyRead } from "../handle-properties.js"; @@ -4441,6 +4442,21 @@ export function fieldGetExpr( // construction), typed as the index value armed with undefined // (mirroring lowerRecordKeyRead). if (target.container === "recordOvf") { + // A dot read through a value HELD from process.env (`const env = + // process.env; env.X`): the hold aliases the live table, so the read is + // the same getenv the direct spelling lowers to. + if (ts.isPropertyAccessExpression(blame)) { + const live = heldEnvValueType(lowerer, blame.expression); + if (live !== null) { + return { + kind: "libCall", + fn: "process.envGet", + args: [{ kind: "strLit", value: target.field, type: STRING, loc }], + type: live, + loc, + }; + } + } let obj = target.obj; if (obj.type.kind === "union" && ts.isPropertyAccessExpression(blame)) { const present = lowerer.stripUndefinedArm(obj.type); diff --git a/packages/compiler/src/frontend/lowering/lower-exprs.ts b/packages/compiler/src/frontend/lowering/lower-exprs.ts index 306fbcb2c..30187a976 100644 --- a/packages/compiler/src/frontend/lowering/lower-exprs.ts +++ b/packages/compiler/src/frontend/lowering/lower-exprs.ts @@ -110,6 +110,7 @@ import { } from "./expressions/property-access.js"; import { lowerNullishPair } from "./expressions/binary.js"; import { lowerElementCompoundParts, recordKeyLiteralText } from "./expressions/element-access.js"; +import { heldEnvValueType } from "./builtins/environment.js"; import { OBJECT_PROTOTYPE_MEMBERS } from "../shape-inference/domain.js"; export { DEFERRED_READ_MESSAGE, @@ -2517,6 +2518,12 @@ export function lowerAbsenceProbe(lowerer: Lowerer, node: ts.Expression): IrExpr const header = lowerer.lowerHttpHeadersElement(expr); if (header) return header; + // A keyed read through a value HELD from process.env (`const env = + // process.env; env[k] ?? fallback`) aliases the live table: the ordinary + // lowering is getenv, whose `string | undefined` result already carries + // the missing arm this probe reconstructs for record keys. + if (heldEnvValueType(lowerer, expr.expression) !== null) return null; + const receiverT = lowerer.mapTypeOf(lowerer.typeOf(expr.expression)); if (receiverT?.kind === "record") { const shape = lowerer.shapes.get(receiverT.shapeId); diff --git a/packages/compiler/src/frontend/lowering/lower-stmts.ts b/packages/compiler/src/frontend/lowering/lower-stmts.ts index 377b08917..923a8e328 100644 --- a/packages/compiler/src/frontend/lowering/lower-stmts.ts +++ b/packages/compiler/src/frontend/lowering/lower-stmts.ts @@ -45,6 +45,7 @@ import { isWebStreamValueInJavaScript, typeInvolvesWebStream } from "./builtins/ import * as ts from "../ts7/adapter.js"; import { lowerOptionalIndex } from "./undefined-index.js"; import { processDataMemberValue, programReadsExitCode } from "./builtins/process.js"; +import { heldEnvValueType } from "./builtins/environment.js"; import { requireInitFlagStmt } from "./require-initialization.js"; import { isJsonRequireDecl } from "./json-require.js"; import { isTimeValueDateDeclaration } from "./date-values.js"; @@ -7764,9 +7765,11 @@ function lowerDeleteStatement(lowerer: Lowerer, expr: ts.DeleteExpression): IrSt return { kind: "strLit", value: target.name.text, type: STRING, loc: locOf(target.name) }; } const keyNode = (target as ts.ElementAccessExpression).argumentExpression; - let key = lowerer.isProcessEnv(target.expression) - ? lowerEnvironmentKey(lowerer, keyNode) - : lowerer.lowerExpr(keyNode); + let key = + lowerer.isProcessEnv(target.expression) || + heldEnvValueType(lowerer, target.expression) !== null + ? lowerEnvironmentKey(lowerer, keyNode) + : lowerer.lowerExpr(keyNode); if (key.type.kind === "dyn") { key = { kind: "libCall", fn: "dyn.toStringCoerce", args: [key], type: STRING, loc: key.loc }; } @@ -7782,7 +7785,10 @@ function lowerDeleteStatement(lowerer: Lowerer, expr: ts.DeleteExpression): IrSt } return key; }; - if (lowerer.isProcessEnv(target.expression)) { + if ( + lowerer.isProcessEnv(target.expression) || + heldEnvValueType(lowerer, target.expression) !== null + ) { return { kind: "exprStmt", expr: { kind: "libCall", fn: "process.envUnset", args: [lowerKey()], type: VOID, loc }, @@ -8710,7 +8716,14 @@ export function lowerExprStatement(lowerer: Lowerer, expr: ts.Expression): IrStm // (Node stringifies everything; a non-string RHS fences instead // of diverging from Node). The computed-key form lands in // lowerElementWrite. - if (lowerer.isProcessEnv(expr.left.expression) && !expr.left.questionDotToken) { + // The same setenv for a value HELD from process.env (`const env = + // process.env; env.X = v`): the hold aliases the live table, as + // Node's does. + if ( + !expr.left.questionDotToken && + (lowerer.isProcessEnv(expr.left.expression) || + heldEnvValueType(lowerer, expr.left.expression) !== null) + ) { const loc = locOf(expr); const key: IrExpr = { kind: "strLit", diff --git a/packages/compiler/src/frontend/lowering/lowerer.ts b/packages/compiler/src/frontend/lowering/lowerer.ts index b928055eb..77a16b5cc 100644 --- a/packages/compiler/src/frontend/lowering/lowerer.ts +++ b/packages/compiler/src/frontend/lowering/lowerer.ts @@ -583,7 +583,12 @@ import { } from "./builtins/filesystem.js"; import { lowerSpawnSyncCall, lowerSpawnCall, lowerExecFileCall } from "./builtins/child-process.js"; import { lowerExecSyncCall } from "./builtins/child-process-sync.js"; -import { isProcessEnv, envValueType, lowerProcessEnvGet } from "./builtins/environment.js"; +import { + isProcessEnv, + envValueType, + lowerProcessEnvGet, + refuseHeldEnvWrite, +} from "./builtins/environment.js"; import { lowerJsonMethodCall, lowerJsonProperty } from "./builtins/json.js"; import { lowerCryptoComposedCall, lowerCryptoModuleCall } from "./builtins/crypto.js"; import { lowerUrlMethodCall, lowerSearchParamsMethodCall } from "./builtins/url.js"; @@ -16460,6 +16465,7 @@ export class Lowerer { } fieldSetStmt(target: FieldTarget, value: IrExpr, loc: SrcLoc, blame: ts.Node): IrStmt { + refuseHeldEnvWrite(this, blame); return markSloppyRecordWrites(this, fieldSetStmt(this, target, value, loc, blame), blame); } diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/held-env-value-writes.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/held-env-value-writes.ts.json new file mode 100644 index 000000000..61d10efe6 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/held-env-value-writes.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/held-env-value-writes.ts" + ], + "diags": [] +} diff --git a/tests/corpus/held-env-value-writes.ts b/tests/corpus/held-env-value-writes.ts new file mode 100644 index 000000000..f4bbce6e4 --- /dev/null +++ b/tests/corpus/held-env-value-writes.ts @@ -0,0 +1,58 @@ +// A value HELD from process.env aliases the live environment, exactly as +// Node's one process.env object does: writes, deletes and reads through the +// hold reach the real environment table (setenv/unsetenv/getenv), so a +// later direct read, `in` test, and a spawned child's inherited environment +// all observe them. A COPY made from the hold stays a copy. +import { spawnSync } from "node:child_process"; + +const env: Record = process.env; +env.SCRIPTC_HELD_WRITE = "one"; +console.log("direct:", process.env.SCRIPTC_HELD_WRITE ?? "(unset)"); +console.log("held:", env.SCRIPTC_HELD_WRITE ?? "(unset)"); + +// A computed key writes the same variable. +const key = "SCRIPTC_HELD_" + "computed"; +env[key] = "two"; +console.log("computed:", process.env[key] ?? "(unset)", env[key] ?? "(unset)"); + +// The write reaches children through the inherited environment. +const child = spawnSync("sh", ["-c", 'printf %s "$SCRIPTC_HELD_WRITE"'], { encoding: "utf8" }); +console.log("child sees:", child.stdout); + +// A write made directly is visible through the hold (one object, not a copy). +process.env.SCRIPTC_HELD_LATE = "late"; +console.log("late via hold:", env.SCRIPTC_HELD_LATE ?? "(unset)"); + +// Deletes through the hold unset the real variable. +delete env.SCRIPTC_HELD_WRITE; +console.log( + "deleted:", + process.env.SCRIPTC_HELD_WRITE === undefined, + env.SCRIPTC_HELD_WRITE === undefined, + "SCRIPTC_HELD_WRITE" in process.env, +); +delete env[key]; +console.log("computed deleted:", process.env[key] === undefined, key in process.env); + +// An absent key deletes as a no-op, like JS. +delete env.SCRIPTC_HELD_NEVER; +console.log("absent delete:", process.env.SCRIPTC_HELD_NEVER === undefined); + +// A COPY of the hold keeps copy semantics: its writes never touch the +// environment, and its own entries stay. The copy sees the environment as +// it is at the spread, including writes made after the hold. +const copy = { ...env }; +copy.SCRIPTC_HELD_COPY = "copy-only"; +console.log( + "copy:", + copy.SCRIPTC_HELD_COPY, + process.env.SCRIPTC_HELD_COPY === undefined, + copy.SCRIPTC_HELD_LATE ?? "(unset)", +); +// Every other bare use of the hold reads the environment at that point too. +console.log( + "bare:", + Object.keys(env).includes("SCRIPTC_HELD_LATE"), + JSON.stringify({ ...env }).includes('"SCRIPTC_HELD_LATE":"late"'), +); +console.log("done"); diff --git a/tests/harness/corpus-costs/held-env-value-writes.ts.json b/tests/harness/corpus-costs/held-env-value-writes.ts.json new file mode 100644 index 000000000..cbe10f3ea --- /dev/null +++ b/tests/harness/corpus-costs/held-env-value-writes.ts.json @@ -0,0 +1 @@ +{ "plain": 1700, "san": 3300 } From 97f4d092f12bd928c6dd15a1f1c85ce013ff844a Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Thu, 29 Oct 2026 01:11:41 -0700 Subject: [PATCH 2/5] Keep the ambient process.env out of the held-value alias predicate holdsEnvSnapshot() answered "yes" for the ambient `process.env` object itself, so the ambient env also took the held-alias reroutes: its absence-probe reads (`process.env.X ?? fallback`) rebuilt a recordKeyGet over an `%env.snapshot` copy instead of the direct envGet intrinsic. Node reads the live table there, and the snapshot lowering also cost the two env corpus programs an IR/LLVM difference and a smaller executable. Split the two cases the way the model already does: the ambient object is not a value HELD from itself (depth 0), while a binding's initializer (depth >= 1) still is the hold's source. The ambient keeps its existing lowering everywhere (envGet/envSet, and its snapshot only where it escapes as a value); aliases keep the live-table member reroutes and the bare-use re-snapshot from the previous commit. Co-Authored-By: Claude Opus 5.5 --- .../frontend/lowering/builtins/environment.ts | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/packages/compiler/src/frontend/lowering/builtins/environment.ts b/packages/compiler/src/frontend/lowering/builtins/environment.ts index bb32c6d53..ddb9acb0d 100644 --- a/packages/compiler/src/frontend/lowering/builtins/environment.ts +++ b/packages/compiler/src/frontend/lowering/builtins/environment.ts @@ -50,14 +50,18 @@ export function isProcessEnv(lowerer: Lowerer, node: ts.Expression): boolean { ); } -/** Whether `node` is the ambient `process.env` object, or a value HELD from - * it through a chain of bindings: `const env = process.env`, `const alias = - * env`, or the destructured `const { env } = process`. Node's held value IS - * the environment object. Here the binding holds a snapshot record of it: +/** Whether `node` is a value HELD from `process.env` through a chain of + * bindings: `const env = process.env`, `const alias = env`, or the + * destructured `const { env } = process`. Node's held value IS the + * environment object. Here the binding holds a snapshot record of it: * MEMBER accesses (`env.X`, `env[k]`, their writes, `delete env.X`) reroute * to the live table (see heldEnvValueType), and every BARE use re-snapshots * the environment at that use (see lowerIdentifierExpr), so both observe - * the environment as Node's single object would. */ + * the environment as Node's single object would. + * + * The ambient `process.env` itself is NOT a hold (depth === 0): it keeps + * its own direct lowering (envGet/envSet intrinsics, its existing snapshot + * when it escapes as a value), which is what Node's reads and writes see. */ export function holdsEnvSnapshot( lowerer: Lowerer, node: ts.Expression | undefined, @@ -77,7 +81,9 @@ export function holdsEnvSnapshot( } break; } - if (isProcessEnv(lowerer, inner)) return true; + // At depth 0 this IS the ambient object, not a value held from it; as a + // binding's initializer (depth >= 1) it is the hold's source. + if (isProcessEnv(lowerer, inner)) return depth > 0; if (!ts.isIdentifier(inner) || depth >= 4) return false; const symbol = lowerer.checker.getSymbolAtLocation(inner); const decl = symbol?.valueDeclaration ?? symbol?.declarations[0]; From 6014bb4ea501197bd8e0d723eb93cedc20b9a705 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Thu, 29 Oct 2026 01:11:42 -0700 Subject: [PATCH 3/5] Keep a returned arrow's own return type in a void slot A value-returning arrow returned through a `() => void` interface member was adopted as void, so the enclosing function promised `() => number` while the lambda built a void closure: "a void result where the '() => number' slot promises ..." natively, though Node runs it (`repros/d2-void-slot-generic.ts`, pi `event-bus.ts:27`). A lambda that is the returned value of the function being lowered reaches that function's caller, so its own return type must survive the checker's void contextual type; the void adoption in lambdaSignature now skips such a lambda. Functions with a void result keep the existing discard semantics. tests/corpus/void-slot-returned-arrow.ts pins the shape both ways. Co-Authored-By: Claude Opus 5.5 --- .../src/frontend/lowering/call-signatures.ts | 25 ++++++++++++++++++ .../corpus/void-slot-returned-arrow.ts.json | 6 +++++ tests/corpus/void-slot-returned-arrow.ts | 26 +++++++++++++++++++ .../void-slot-returned-arrow.ts.json | 1 + 4 files changed, 58 insertions(+) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/void-slot-returned-arrow.ts.json create mode 100644 tests/corpus/void-slot-returned-arrow.ts create mode 100644 tests/harness/corpus-costs/void-slot-returned-arrow.ts.json diff --git a/packages/compiler/src/frontend/lowering/call-signatures.ts b/packages/compiler/src/frontend/lowering/call-signatures.ts index 45329d077..98e78953f 100644 --- a/packages/compiler/src/frontend/lowering/call-signatures.ts +++ b/packages/compiler/src/frontend/lowering/call-signatures.ts @@ -1148,6 +1148,15 @@ export function lambdaSignature( // is the one the adoption block below takes for mappable inferred // returns, so this only changes programs that do not compile today. // + // A lambda RETURNED from the function being lowered flows into that + // function's result type, which OBSERVES its value — the checker's void + // contextual type at such a site comes from an outer assignment target + // that would discard the enclosing function's own result. So the void + // adoption below must not fire there: `{ on: (h) => () => seen.push("y") }` + // against a `(h) => void` field types the inner arrow `() => number` + // (its own type) while its contextual type is the discarded `() => void`, + // and the outer arrow's result is the `() => number` that must return it. + const returnedValue = returnedIntoValueSlot(lowerer, node); // The contextual signature is looked up at most once per lambda: both // this decision and the adoption block below need it, and the checker // call is not free in a compile this hot. @@ -1165,6 +1174,7 @@ export function lambdaSignature( ts.isArrowFunction(node) && !ts.isBlock(node.body) && isStreamUndefCallExpr(lowerer, node.body) ? VOID : (ts.isArrowFunction(node) || ts.isFunctionExpression(node)) && + !returnedValue && voidContextualSlot(lowerer, node, contextualSigs) ? VOID : lowerer.declaredReturnType(node, node); @@ -1292,6 +1302,7 @@ export function lambdaSignature( if ( !literalMethod && !isAsyncLike && + !returnedValue && !node.type && !inferredLiteral && ret.kind !== "void" && @@ -1352,6 +1363,20 @@ function voidContextualSlot( ); } +/** Whether `node` is the expression of a `return` in the function being + * lowered, and that function's result is a function type promising a + * value (`() => number`, not `() => void`). Such a lambda's value reaches + * the caller of the enclosing function, so its own return type is kept + * even when the checker's contextual type for it is `void` (see the ABI + * decision in lambdaSignature). */ +function returnedIntoValueSlot(lowerer: Lowerer, node: ts.Node): boolean { + let parent: ts.Node | undefined = node.parent; + while (parent !== undefined && ts.isParenthesizedExpression(parent)) parent = parent.parent; + if (parent === undefined || !ts.isReturnStatement(parent) || !parent.expression) return false; + const result = lowerer.ctx.returnType; + return result.kind === "func" && result.ret.kind !== "void"; +} + /** The type of the property an object-literal method fills on the * literal's contextual type; undefined when the literal has no contextual * type or the method has a computed name. */ diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/void-slot-returned-arrow.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/void-slot-returned-arrow.ts.json new file mode 100644 index 000000000..cc7e1cea7 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/void-slot-returned-arrow.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/corpus/void-slot-returned-arrow.ts" + ], + "diags": [] +} diff --git a/tests/corpus/void-slot-returned-arrow.ts b/tests/corpus/void-slot-returned-arrow.ts new file mode 100644 index 000000000..0abe87eac --- /dev/null +++ b/tests/corpus/void-slot-returned-arrow.ts @@ -0,0 +1,26 @@ +// A value-returning arrow RETURNED through a `() => void` interface member. +// TypeScript types the inner arrow `() => number` (its own type) while the +// checker contextually types it `() => void`; the enclosing arrow's result +// is `() => number`, so the value is observed and must be produced. Node +// runs this; the closure is called for its effect only. +interface Subscriber { + on(handler: (data: string) => void): () => void; +} + +function makeSubscriber(): Subscriber { + const seen: string[] = []; + return { + on: (handler) => { + seen.push("registered"); + handler("first"); + // Concise arrow whose body returns a number: the result is discarded + // at the `() => void` slot but the arrow's own type keeps it. + return () => seen.push("unsubscribed"); + }, + }; +} + +const off = makeSubscriber().on((data) => console.log("got", data)); +console.log("off is", typeof off); +off(); +console.log("done"); diff --git a/tests/harness/corpus-costs/void-slot-returned-arrow.ts.json b/tests/harness/corpus-costs/void-slot-returned-arrow.ts.json new file mode 100644 index 000000000..c321beddb --- /dev/null +++ b/tests/harness/corpus-costs/void-slot-returned-arrow.ts.json @@ -0,0 +1 @@ +{ "plain": 1600, "san": 2400 } From 08a89580858c0041d9e132b43d0ad4397d5a0737 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Thu, 29 Oct 2026 01:49:10 -0700 Subject: [PATCH 4/5] Refuse a hold of process.env whose type is not the environment record `const env: { HOME?: string } = process.env` (and every other annotation or cast that moves the hold off the env record) type-checks, but the hold in scriptc is a snapshot record taken at the initializer, so its member reads would observe the environment as it was then while Node's hold IS the environment object and reads the live table. Writes through such a hold were already refused with SC2014; reads still drifted silently. Refuse the hold where it is created: refuseHeldEnvCopyHold runs on every variable declaration whose initializer holds process.env and reports SC2014 at the binding when its type is not the env record (a pure index-signature record of string | undefined, which is what process.env maps to). tests/diagnostics/held-env-copy-hold.ts pins the refusal for a fixed-field annotation and an `any` hold. Co-Authored-By: Claude Opus 5.5 --- .../compiler/src/diagnostics/diagnostic.ts | 18 +++++++++++++ .../frontend/lowering/builtins/environment.ts | 27 +++++++++++++++++-- .../src/frontend/lowering/lower-stmts.ts | 6 ++++- .../diagnostics/held-env-copy-hold.ts.json | 6 +++++ tests/diagnostics/held-env-copy-hold.ts | 13 +++++++++ .../__snapshots__/held-env-copy-hold.ts.txt | 17 ++++++++++++ 6 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/diagnostics/held-env-copy-hold.ts.json create mode 100644 tests/diagnostics/held-env-copy-hold.ts create mode 100644 tests/harness/__snapshots__/held-env-copy-hold.ts.txt diff --git a/packages/compiler/src/diagnostics/diagnostic.ts b/packages/compiler/src/diagnostics/diagnostic.ts index a13d237d5..12f27c0b0 100644 --- a/packages/compiler/src/diagnostics/diagnostic.ts +++ b/packages/compiler/src/diagnostics/diagnostic.ts @@ -1134,6 +1134,24 @@ export function heldEnvWriteDiag(loc: SrcLoc): ScrDiagnostic { }; } +/** A BINDING that holds `process.env` under a type which is not the env + * record (`const env: Record = process.env as any`, a + * fixed-field annotation, `const { env } = process` with one): the hold is + * a snapshot record taken at the initializer, so its member reads would + * observe the environment as it was then, while Node's hold IS the + * environment object and reads the live table. Refuse where the hold is + * created rather than letting its reads drift silently. */ +export function heldEnvHoldDiag(loc: SrcLoc): ScrDiagnostic { + return { + code: "SC2014", + message: + "a value held from process.env under a type other than the environment record (a string-keyed record of string | undefined) is not supported: the hold is a snapshot copy taken where it is created, so its reads would see the environment as it was then while Node reads the live environment", + loc, + milestone: "later", + hint: "hold process.env without changing its type (const env = process.env), read the live table directly (process.env.NAME), or copy the entries you need: const env = { ...process.env, NAME: value }", + }; +} + /** Records are monomorphic structs, so a value's shape must match the * expected shape exactly OR narrow through the width-copy family * (recordWidthPlan / the overflow capture: target fields copied off the diff --git a/packages/compiler/src/frontend/lowering/builtins/environment.ts b/packages/compiler/src/frontend/lowering/builtins/environment.ts index ddb9acb0d..8aa4f5a9d 100644 --- a/packages/compiler/src/frontend/lowering/builtins/environment.ts +++ b/packages/compiler/src/frontend/lowering/builtins/environment.ts @@ -2,7 +2,7 @@ import { dynUndefinedExpr } from "../../../ir/build.js"; import * as ts from "../../ts7/adapter.js"; import { type Lowerer, PoisonError } from "../lowerer.js"; import { locOf } from "../../program.js"; -import { heldEnvWriteDiag } from "../../../diagnostics/diagnostic.js"; +import { heldEnvHoldDiag, heldEnvWriteDiag } from "../../../diagnostics/diagnostic.js"; import { pairsSnapshotHelper } from "../pairs-snapshot.js"; import { DYN, @@ -136,7 +136,14 @@ function writeTargetReceiver(node: ts.Node): ts.Expression | null { * the environment at that use site (lowerIdentifierExpr). */ export function heldEnvValueType(lowerer: Lowerer, receiver: ts.Expression): IrType | null { if (!holdsEnvSnapshot(lowerer, receiver)) return null; - const mapped = lowerer.mapTypeOf(lowerer.typeOf(receiver)); + return envRecordValueType(lowerer, lowerer.mapTypeOf(lowerer.typeOf(receiver))); +} + +/** The env-record value type a hold must have to alias the live table — + * a pure index-signature record (no fields, not a tuple) whose value slot + * is the `string | undefined` union, which is what `process.env` maps to. + * Null for every other type. */ +function envRecordValueType(lowerer: Lowerer, mapped: IrType | null): IrType | null { if (mapped?.kind !== "record") return null; const shape = lowerer.shapes.get(mapped.shapeId); if (!shape || shape.tuple || shape.fields.length > 0 || shape.indexValue === undefined) @@ -145,6 +152,22 @@ export function heldEnvValueType(lowerer: Lowerer, receiver: ts.Expression): IrT return typeEquals(shape.indexValue, value) ? value : null; } +/** Refuse a BINDING that holds `process.env` under a type which is not the + * env record — `const env: Record = process.env as any`, a + * fixed-field annotation, `const e: any = process.env`. Node's hold IS the + * environment object, so member reads through it see the live table; + * here the hold is a snapshot record taken at the initializer, so reads + * would silently observe the environment as it was then (writes are + * already refused — refuseHeldEnvWrite). Refuse the hold where it is + * created instead of letting its later reads drift. `name` is the bound + * identifier, `blame` the declaration the diagnostic points at. */ +export function refuseHeldEnvCopyHold(lowerer: Lowerer, name: ts.Identifier, blame: ts.Node): void { + if (!holdsEnvSnapshot(lowerer, name)) return; + if (envRecordValueType(lowerer, lowerer.mapTypeOf(lowerer.typeOf(name))) !== null) return; + lowerer.pushDiag(heldEnvHoldDiag(locOf(blame))); + throw new PoisonError(); +} + /** Refuse a write through a value HELD from `process.env` that cannot take * the live path: the compound and update forms (`env.X += v`, `env.X++`) * reach this wrapper, and so does any write through a hold whose type is diff --git a/packages/compiler/src/frontend/lowering/lower-stmts.ts b/packages/compiler/src/frontend/lowering/lower-stmts.ts index 923a8e328..92bc0416b 100644 --- a/packages/compiler/src/frontend/lowering/lower-stmts.ts +++ b/packages/compiler/src/frontend/lowering/lower-stmts.ts @@ -45,7 +45,7 @@ import { isWebStreamValueInJavaScript, typeInvolvesWebStream } from "./builtins/ import * as ts from "../ts7/adapter.js"; import { lowerOptionalIndex } from "./undefined-index.js"; import { processDataMemberValue, programReadsExitCode } from "./builtins/process.js"; -import { heldEnvValueType } from "./builtins/environment.js"; +import { heldEnvValueType, refuseHeldEnvCopyHold } from "./builtins/environment.js"; import { requireInitFlagStmt } from "./require-initialization.js"; import { isJsonRequireDecl } from "./json-require.js"; import { isTimeValueDateDeclaration } from "./date-values.js"; @@ -5996,6 +5996,10 @@ export function lowerVarDecl( } } if (!ts.isIdentifier(decl.name)) lowerer.unsupported("SC1031", decl.name); + // A binding that holds `process.env` under any type but the env record + // is a snapshot copy whose reads would drift from Node's live table + // (refuseHeldEnvCopyHold); refuse it here, at the hold itself. + refuseHeldEnvCopyHold(lowerer, decl.name, decl); { const alias = lowerTupleAliasDecl(lowerer, decl); if (alias) return alias; diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/diagnostics/held-env-copy-hold.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/diagnostics/held-env-copy-hold.ts.json new file mode 100644 index 000000000..9e642c45c --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/diagnostics/held-env-copy-hold.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/diagnostics/held-env-copy-hold.ts" + ], + "diags": [] +} diff --git a/tests/diagnostics/held-env-copy-hold.ts b/tests/diagnostics/held-env-copy-hold.ts new file mode 100644 index 000000000..b5c3fc627 --- /dev/null +++ b/tests/diagnostics/held-env-copy-hold.ts @@ -0,0 +1,13 @@ +// Values HELD from process.env under a type that is not the environment +// record (tsc-clean, outside the live-alias subset). Node's hold IS the +// environment object, so its reads see the live table; the hold here is a +// snapshot copy taken where it is created, so its reads would silently +// drift once the environment changes. + +// a fixed-field annotation: tsc admits it through the index signature +const home: { HOME?: string } = process.env; +console.log(home.HOME); + +// an `any` hold: no record shape to alias through +const anyEnv: any = process.env; +console.log(anyEnv.PATH); diff --git a/tests/harness/__snapshots__/held-env-copy-hold.ts.txt b/tests/harness/__snapshots__/held-env-copy-hold.ts.txt new file mode 100644 index 000000000..3004e4b35 --- /dev/null +++ b/tests/harness/__snapshots__/held-env-copy-hold.ts.txt @@ -0,0 +1,17 @@ +held-env-copy-hold.ts:8:7 - error SC2014: a value held from process.env under a type other than the environment record (a string-keyed record of string | undefined) is not supported: the hold is a snapshot copy taken where it is created, so its reads would see the environment as it was then while Node reads the live environment + + 7 | // a fixed-field annotation: tsc admits it through the index signature + 8 | const home: { HOME?: string } = process.env; + | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + 9 | console.log(home.HOME); + + hint: hold process.env without changing its type (const env = process.env), read the live table directly (process.env.NAME), or copy the entries you need: const env = { ...process.env, NAME: value } + +held-env-copy-hold.ts:12:7 - error SC2014: a value held from process.env under a type other than the environment record (a string-keyed record of string | undefined) is not supported: the hold is a snapshot copy taken where it is created, so its reads would see the environment as it was then while Node reads the live environment + + 11 | // an `any` hold: no record shape to alias through + 12 | const anyEnv: any = process.env; + | ^~~~~~~~~~~~~~~~~~~~~~~~~ + 13 | console.log(anyEnv.PATH); + + hint: hold process.env without changing its type (const env = process.env), read the live table directly (process.env.NAME), or copy the entries you need: const env = { ...process.env, NAME: value } \ No newline at end of file From 2a4892c1f239aa259bbe391777a978d151c574e6 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Thu, 29 Oct 2026 10:32:48 -0500 Subject: [PATCH 5/5] Compare held environment values by identity, refuse reassigned holds, and regenerate the surface manifest - An identity comparison (===, ==, !==, !=) between process.env and values held from it folds to equal: every bare use of a hold reads a fresh snapshot, while Node compares the one environment object. A comparison with a primitive, null or undefined keeps the ordinary lowering; one with another object is refused with SC2014. - A binding initialized from process.env that the program reassigns is no longer a hold, so its member writes no longer reach the live table after the reassignment. Its declaration is refused with SC2014, since its reads and writes before the reassignment would miss the live environment. - surface-manifest.json lists the SC2014 fence. --- .../compiler/src/diagnostics/diagnostic.ts | 31 +++++ .../frontend/lowering/builtins/environment.ts | 114 +++++++++++++++++- .../frontend/lowering/expressions/binary.ts | 7 ++ packages/compiler/surface-manifest.json | 7 ++ tests/corpus/held-env-value-writes.ts | 12 ++ tests/diagnostics/held-env-copy-hold.ts | 13 ++ .../__snapshots__/held-env-copy-hold.ts.txt | 20 ++- 7 files changed, 200 insertions(+), 4 deletions(-) diff --git a/packages/compiler/src/diagnostics/diagnostic.ts b/packages/compiler/src/diagnostics/diagnostic.ts index 12f27c0b0..d733c4104 100644 --- a/packages/compiler/src/diagnostics/diagnostic.ts +++ b/packages/compiler/src/diagnostics/diagnostic.ts @@ -1152,6 +1152,37 @@ export function heldEnvHoldDiag(loc: SrcLoc): ScrDiagnostic { }; } +/** A binding initialized from `process.env` that the program reassigns. + * Node's binding holds the environment object until the reassignment; the + * hold here is a snapshot copy, so reads and writes before the reassignment + * would miss the live environment, and a live alias would keep writing it + * after the binding holds something else. */ +export function heldEnvReassignedDiag(loc: SrcLoc): ScrDiagnostic { + return { + code: "SC2014", + message: + "a reassigned binding that holds process.env is not supported: Node's binding holds the environment object until it is reassigned, while the hold here is a snapshot copy of the environment", + loc, + milestone: "later", + hint: "hold process.env in a binding that is never reassigned (const env = process.env), or copy the entries you need: let env = { ...process.env }", + }; +} + +/** An identity comparison (`===`, `==` and their negations) between a value + * held from `process.env` and another object that is not the environment. + * Node compares the environment object itself; the hold here is a snapshot + * record, so the answer could differ from Node's. */ +export function heldEnvIdentityDiag(loc: SrcLoc): ScrDiagnostic { + return { + code: "SC2014", + message: + "comparing process.env or a value held from it by identity with another object is not supported: the value here is a snapshot copy of the environment, not the environment object", + loc, + milestone: "later", + hint: "compare the entries you need (env.NAME === other.NAME), or compare two holds of process.env, which are the same object", + }; +} + /** Records are monomorphic structs, so a value's shape must match the * expected shape exactly OR narrow through the width-copy family * (recordWidthPlan / the overflow capture: target fields copied off the diff --git a/packages/compiler/src/frontend/lowering/builtins/environment.ts b/packages/compiler/src/frontend/lowering/builtins/environment.ts index 8aa4f5a9d..3db4ff992 100644 --- a/packages/compiler/src/frontend/lowering/builtins/environment.ts +++ b/packages/compiler/src/frontend/lowering/builtins/environment.ts @@ -2,9 +2,17 @@ import { dynUndefinedExpr } from "../../../ir/build.js"; import * as ts from "../../ts7/adapter.js"; import { type Lowerer, PoisonError } from "../lowerer.js"; import { locOf } from "../../program.js"; -import { heldEnvHoldDiag, heldEnvWriteDiag } from "../../../diagnostics/diagnostic.js"; +import { + heldEnvHoldDiag, + heldEnvIdentityDiag, + heldEnvReassignedDiag, + heldEnvWriteDiag, +} from "../../../diagnostics/diagnostic.js"; +import { bindingNeverReassigned } from "../binding-analysis.js"; +import { constituentTypes } from "../../ts7/checker.js"; import { pairsSnapshotHelper } from "../pairs-snapshot.js"; import { + BOOL, DYN, type IrExpr, type IrType, @@ -91,8 +99,13 @@ export function holdsEnvSnapshot( // `const env = process.env` (or an identifier alias of one). The symbol's // value declaration is the `const` itself; the written identifier here is // a different node than the declared name, so no node identity check. + // A binding the program reassigns holds the environment only until the + // reassignment, so it is not a hold (refuseHeldEnvCopyHold refuses it). if (ts.isVariableDeclaration(decl)) { - return holdsEnvSnapshot(lowerer, decl.initializer, depth + 1); + return ( + holdsEnvSnapshot(lowerer, decl.initializer, depth + 1) && + !reassignedBinding(lowerer, symbol!, decl) + ); } // `const { env } = process` (also `{ env: held }`). if (ts.isBindingElement(decl) && ts.isObjectBindingPattern(decl.parent)) { @@ -104,12 +117,98 @@ export function holdsEnvSnapshot( named.text === "env" && declaration.initializer !== undefined ) { - return lowerer.isStdlibGlobal(declaration.initializer, "process"); + return ( + lowerer.isStdlibGlobal(declaration.initializer, "process") && + !reassignedBinding(lowerer, symbol!, declaration) + ); } } return false; } +/** Whether something writes the binding after its declaration: a `let` or + * `var` the file assigns (bindingNeverReassigned). A `const` never is. */ +function reassignedBinding( + lowerer: Lowerer, + symbol: ts.Symbol, + declaration: ts.VariableDeclaration, +): boolean { + const list = declaration.parent; + if (ts.isVariableDeclarationList(list) && (list.flags & ts.NodeFlags.Const) !== 0) return false; + return !bindingNeverReassigned(lowerer, symbol, declaration); +} + +/** Whether an expression is the environment object to Node: the ambient + * `process.env` or a value held from it (holdsEnvSnapshot). */ +function denotesEnvironment(lowerer: Lowerer, node: ts.Expression): boolean { + return isProcessEnv(lowerer, unwrapped(node)) || holdsEnvSnapshot(lowerer, node); +} + +function unwrapped(node: ts.Expression): ts.Expression { + let inner = node; + while ( + ts.isParenthesizedExpression(inner) || + ts.isAsExpression(inner) || + ts.isNonNullExpression(inner) || + ts.isSatisfiesExpression(inner) + ) + inner = inner.expression; + return inner; +} + +/** `a === b`, `a == b` and their negations where an operand is the + * environment object (denotesEnvironment). Every use of a held value reads + * a snapshot, so the snapshots would never be identical, while Node + * compares the one environment object: + * - two operands that both denote it compare equal, folded (neither + * operand has an effect: each is a binding read or `process.env`); + * - one compared with a primitive, `null` or `undefined` keeps the + * ordinary lowering, whose answer (never equal) is Node's; + * - one compared with another object is refused (heldEnvIdentityDiag). + * Null when no operand denotes the environment. */ +export function lowerEnvironmentIdentity( + lowerer: Lowerer, + expr: ts.BinaryExpression, + loc: SrcLoc, +): IrExpr | null { + const op = expr.operatorToken.kind; + const negated = + op === ts.SyntaxKind.ExclamationEqualsEqualsToken || + op === ts.SyntaxKind.ExclamationEqualsToken; + if ( + !negated && + op !== ts.SyntaxKind.EqualsEqualsEqualsToken && + op !== ts.SyntaxKind.EqualsEqualsToken + ) + return null; + const left = denotesEnvironment(lowerer, expr.left); + const right = denotesEnvironment(lowerer, expr.right); + if (!left && !right) return null; + if (left && right) return { kind: "boolLit", value: !negated, type: BOOL, loc }; + if (isPrimitiveOperand(lowerer.typeOf(left ? expr.right : expr.left))) return null; + lowerer.pushDiag(heldEnvIdentityDiag(loc)); + throw new PoisonError(); +} + +/** Whether a checker type has only primitive values (`null` and `undefined` + * included), which are never the environment object. */ +function isPrimitiveOperand(type: ts.Type): boolean { + if ((type.flags & ts.TypeFlags.Union) !== 0) + return constituentTypes(type).every(isPrimitiveOperand); + return ( + (type.flags & + (ts.TypeFlags.Null | + ts.TypeFlags.Undefined | + ts.TypeFlags.Void | + ts.TypeFlags.StringLike | + ts.TypeFlags.NumberLike | + ts.TypeFlags.BooleanLike | + ts.TypeFlags.BigIntLike | + ts.TypeFlags.ESSymbolLike)) !== + 0 + ); +} + /** The receiver expression of a write target — the assignment's or delete's * access (`env.X`, `env[k]`), through any compound/update wrapping. Null for * targets that write no entry of a receiver. */ @@ -162,6 +261,15 @@ function envRecordValueType(lowerer: Lowerer, mapped: IrType | null): IrType | n * created instead of letting its later reads drift. `name` is the bound * identifier, `blame` the declaration the diagnostic points at. */ export function refuseHeldEnvCopyHold(lowerer: Lowerer, name: ts.Identifier, blame: ts.Node): void { + if ( + ts.isVariableDeclaration(blame) && + holdsEnvSnapshot(lowerer, blame.initializer, 1) && + !holdsEnvSnapshot(lowerer, name) + ) { + // Initialized from process.env but reassigned later (holdsEnvSnapshot). + lowerer.pushDiag(heldEnvReassignedDiag(locOf(blame))); + throw new PoisonError(); + } if (!holdsEnvSnapshot(lowerer, name)) return; if (envRecordValueType(lowerer, lowerer.mapTypeOf(lowerer.typeOf(name))) !== null) return; lowerer.pushDiag(heldEnvHoldDiag(locOf(blame))); diff --git a/packages/compiler/src/frontend/lowering/expressions/binary.ts b/packages/compiler/src/frontend/lowering/expressions/binary.ts index e20604f92..a289bf70c 100644 --- a/packages/compiler/src/frontend/lowering/expressions/binary.ts +++ b/packages/compiler/src/frontend/lowering/expressions/binary.ts @@ -85,6 +85,7 @@ import { lowerUnionEquality, tagEqualityMayMissAlias } from "../strict-equality. import { withUnitArm } from "../../type-mapper.js"; import { npmStaticPackageOfPath } from "../../npm-static.js"; import { lowerWebAssemblyTypeof } from "../builtins/webassembly.js"; +import { lowerEnvironmentIdentity } from "../builtins/environment.js"; /** `v === undefined` / `v !== null` — the narrowing tests for unit-armed * unions. A union operand compared with a unit literal lowers to a @@ -1466,6 +1467,12 @@ export function lowerBinary(lowerer: Lowerer, expr: ts.BinaryExpression): IrExpr : "compound assignment as an expression (write `x op= e` as a statement, or spell out `x = x op e`)", ); } + // An identity comparison with process.env or a value held from it + // compares the one environment object, which no snapshot can stand for. + { + const identity = lowerEnvironmentIdentity(lowerer, expr, loc); + if (identity) return identity; + } if (op === ts.SyntaxKind.EqualsEqualsToken || op === ts.SyntaxKind.ExclamationEqualsToken) { const nullTest = lowerLooseNullCompare(lowerer, expr, loc); if (nullTest) return nullTest; diff --git a/packages/compiler/surface-manifest.json b/packages/compiler/surface-manifest.json index ea8295406..3bc73600c 100644 --- a/packages/compiler/surface-manifest.json +++ b/packages/compiler/surface-manifest.json @@ -285,6 +285,13 @@ "status": "unsupported", "code": "SC2013" }, + { + "id": "diagnostic.sc2014", + "kind": "diagnostic-fence", + "name": "writes through a value held from process.env (a snapshot, not the live table)", + "status": "unsupported", + "code": "SC2014" + }, { "id": "diagnostic.sc2020", "kind": "diagnostic-fence", diff --git a/tests/corpus/held-env-value-writes.ts b/tests/corpus/held-env-value-writes.ts index f4bbce6e4..0f26cc22b 100644 --- a/tests/corpus/held-env-value-writes.ts +++ b/tests/corpus/held-env-value-writes.ts @@ -55,4 +55,16 @@ console.log( Object.keys(env).includes("SCRIPTC_HELD_LATE"), JSON.stringify({ ...env }).includes('"SCRIPTC_HELD_LATE":"late"'), ); +// Holds and process.env are the one environment object: identity +// comparisons between them are equal, and with a primitive never. +const alias = env; +console.log( + "identity:", + env === env, + env === alias, + alias == process.env, + env !== process.env, + env === undefined, + env != null, +); console.log("done"); diff --git a/tests/diagnostics/held-env-copy-hold.ts b/tests/diagnostics/held-env-copy-hold.ts index b5c3fc627..e0b6ceee2 100644 --- a/tests/diagnostics/held-env-copy-hold.ts +++ b/tests/diagnostics/held-env-copy-hold.ts @@ -11,3 +11,16 @@ console.log(home.HOME); // an `any` hold: no record shape to alias through const anyEnv: any = process.env; console.log(anyEnv.PATH); + +// a reassigned hold: Node's binding is the environment until the +// reassignment, the hold here is a copy +let held = process.env; +held = { NAME: "replacement" }; +held.NAME = "written"; +console.log(held.NAME); + +// identity with another object: the hold is a snapshot record, not the +// environment object +const live = process.env; +const other: Record = {}; +console.log(live === other); diff --git a/tests/harness/__snapshots__/held-env-copy-hold.ts.txt b/tests/harness/__snapshots__/held-env-copy-hold.ts.txt index 3004e4b35..1adbccd3e 100644 --- a/tests/harness/__snapshots__/held-env-copy-hold.ts.txt +++ b/tests/harness/__snapshots__/held-env-copy-hold.ts.txt @@ -14,4 +14,22 @@ held-env-copy-hold.ts:12:7 - error SC2014: a value held from process.env under a | ^~~~~~~~~~~~~~~~~~~~~~~~~ 13 | console.log(anyEnv.PATH); - hint: hold process.env without changing its type (const env = process.env), read the live table directly (process.env.NAME), or copy the entries you need: const env = { ...process.env, NAME: value } \ No newline at end of file + hint: hold process.env without changing its type (const env = process.env), read the live table directly (process.env.NAME), or copy the entries you need: const env = { ...process.env, NAME: value } + +held-env-copy-hold.ts:17:5 - error SC2014: a reassigned binding that holds process.env is not supported: Node's binding holds the environment object until it is reassigned, while the hold here is a snapshot copy of the environment + + 16 | // reassignment, the hold here is a copy + 17 | let held = process.env; + | ^~~~~~~~~~~~~~~~~~ + 18 | held = { NAME: "replacement" }; + + hint: hold process.env in a binding that is never reassigned (const env = process.env), or copy the entries you need: let env = { ...process.env } + +held-env-copy-hold.ts:26:13 - error SC2014: comparing process.env or a value held from it by identity with another object is not supported: the value here is a snapshot copy of the environment, not the environment object + + 25 | const other: Record = {}; + 26 | console.log(live === other); + | ^~~~~~~~~~~~~~ + 27 | + + hint: compare the entries you need (env.NAME === other.NAME), or compare two holds of process.env, which are the same object \ No newline at end of file