From 26e155987f4a1eb96c1f767fac560f427b0f7c37 Mon Sep 17 00:00:00 2001 From: Wenyu Chiou <162016108+WenyuChiou@users.noreply.github.com> Date: Sat, 24 Oct 2026 23:29:45 -0400 Subject: [PATCH] fix(javascript): invalidate Electron reads after direct eval (#7053) --- .../electronOpaqueBindingIdentity.test.ts | 131 ++++++++++++++++++ ...criptSemanticAnalysis.eval_effects.test.ts | 22 ++- .../javascript/javascriptSemanticAnalysis.ts | 20 ++- .../javascriptSemanticMutationInitializers.ts | 46 ++++++ .../javascriptSemanticOpaqueEffects.ts | 21 ++- .../electronOpaqueBindingCli.test.ts | 73 ++++++++++ 6 files changed, 308 insertions(+), 5 deletions(-) create mode 100644 src/domain/javascript/electronOpaqueBindingIdentity.test.ts create mode 100644 tests/acceptance/applications/electronOpaqueBindingCli.test.ts diff --git a/src/domain/javascript/electronOpaqueBindingIdentity.test.ts b/src/domain/javascript/electronOpaqueBindingIdentity.test.ts new file mode 100644 index 0000000000..e3464933fd --- /dev/null +++ b/src/domain/javascript/electronOpaqueBindingIdentity.test.ts @@ -0,0 +1,131 @@ +import { expect, it } from "vitest"; + +import { analyzeParsedJavaScriptStaticSource } from "./javascriptStaticAnalysis.js"; +import { parseJavaScriptSource } from "./javascriptSourceParser.js"; + +const window = "new s.BrowserWindow({webPreferences:{nodeIntegration:true}})"; +const replace = 'eval("s={BrowserWindow:class{}}");'; +const bootstrap = (consumer: string, strict = true) => + `(()=>{var m={45840:e=>{e.exports=require("electron")},100:(e,t,n)=>{${strict ? '"use strict";' : ""}${consumer}}},c={};` + + "function n(t){var r=c[t];if(void 0!==r)return r.exports;var o=c[t]={exports:{}};" + + "return m[t](o,o.exports,n),o.exports}n(100)})();"; + +const windowsIn = (source: string) => { + const parsed = parseJavaScriptSource(source, "main.cjs"); + expect(parsed).not.toBeNull(); + if (parsed === null) throw new Error("Expected valid CommonJS fixture"); + expect(parsed.errors).toEqual([]); + const result = analyzeParsedJavaScriptStaticSource(source, parsed); + expect(result.parse_status).not.toBe("failed"); + return result.electron.browser_windows; +}; + +it.each([ + ["Webpack external", (body: string) => bootstrap(`let s=n(45840);${body}`)], + [ + "literal require", + (body: string) => `(()=>{let s=require("electron");${body}})();`, + ], +] as const)( + "checks each %s read without caching validity across direct eval", + (_name, wrap) => { + const source = wrap(`${window};${replace}${window};`); + const windows = windowsIn(source); + expect(windows).toHaveLength(1); + expect(windows[0]?.location.start.column).toBe(source.indexOf(window)); + }, +); + +it("does not treat a sloppy-eval shadow as the outer Electron binding", () => { + const source = bootstrap( + `const s=n(45840);function consume(){${window};eval("var s={BrowserWindow:class{}}");${window};}consume();`, + false, + ); + const windows = windowsIn(source); + expect(windows).toHaveLength(1); + expect(windows[0]?.location.start.column).toBe(source.indexOf(window)); +}); + +it.each([ + ["indirect eval", '(0,eval)("s={BrowserWindow:class{}}");'], + ["optional eval", 'eval?.("s={BrowserWindow:class{}}");'], + [ + "a different lexical binding", + 'function other(){let s;eval("s={BrowserWindow:class{}}");}other();', + ], + ["a block-shadowed binding", '{let s;eval("s={BrowserWindow:class{}}");}'], +] as const)( + "preserves the local Electron binding across %s", + (_name, effect) => { + expect( + windowsIn(bootstrap(`let s=n(45840);${effect}${window};`)), + ).toHaveLength(1); + }, +); + +it("preserves a shadowed non-strict eval call", () => { + expect( + windowsIn( + bootstrap( + `let s=n(45840);function eval(value){}${replace}${window};`, + false, + ), + ), + ).toHaveLength(1); +}); + +it("preserves the constructor captured before eval in its arguments", () => { + expect( + windowsIn( + bootstrap( + 'let s=n(45840);new s.BrowserWindow(eval("s={BrowserWindow:class{}}"));', + ), + ), + ).toHaveLength(1); +}); + +it.each([ + [ + "a persistent binding across calls", + `let s=n(45840);function consume(){${window};${replace}}consume();consume();`, + ], + [ + "a deferred effect in a nested closure", + `let s=n(45840);function change(){${replace}}change();${window};`, + ], + [ + "a binding retained across loop iterations", + `let s=n(45840);for(let i=0;i<2;i++){${window};${replace}}`, + ], +] as const)("does not order away %s", (_name, body) => { + expect(windowsIn(bootstrap(body))).toHaveLength(0); +}); + +it("does not order away a sloppy-eval shadow from a prior loop iteration", () => { + expect( + windowsIn( + bootstrap( + `const s=n(45840);function consume(){for(let i=0;i<2;i++){${window};eval("var s={BrowserWindow:class{}}");}}consume();`, + false, + ), + ), + ).toHaveLength(0); +}); + +it("does not introduce a caller-local shadow from strict eval", () => { + expect( + windowsIn( + bootstrap( + `const s=n(45840);function consume(){eval("var s={BrowserWindow:class{}}");${window};}consume();`, + ), + ), + ).toHaveLength(1); +}); + +it("retains possible Electron writes through a binding made uncertain by eval", () => { + expect( + windowsIn( + '(()=>{let s=require("electron");eval("0");s.BrowserWindow=class{};const exact=require("electron");new exact.BrowserWindow({webPreferences:{nodeIntegration:true}})})();', + ), + ).toHaveLength(0); +}); diff --git a/src/domain/javascript/javascriptSemanticAnalysis.eval_effects.test.ts b/src/domain/javascript/javascriptSemanticAnalysis.eval_effects.test.ts index d5d5a14627..5882a2e6fe 100644 --- a/src/domain/javascript/javascriptSemanticAnalysis.eval_effects.test.ts +++ b/src/domain/javascript/javascriptSemanticAnalysis.eval_effects.test.ts @@ -1,6 +1,9 @@ import { expect, it } from "vitest"; import { analyzeJavaScriptSemantics } from "./javascriptSemanticAnalysis.js"; -import { onlyCallable } from "./javascriptSemanticAnalysis.fixture.js"; +import { + onlyBinding, + onlyCallable, +} from "./javascriptSemanticAnalysis.fixture.js"; const returned = (body: string) => onlyCallable(analyzeJavaScriptSemantics(body), "run").returnSites[0]?.value; @@ -20,6 +23,23 @@ it.each([ expect(returned(source)).toMatchObject({ status: "unknown" }); }); +it("retains uncertainty for outer function declarations across eval-bearing calls", () => { + const ir = analyzeJavaScriptSemantics( + 'function value(){} function run(){const saved=value;eval("value=2");return saved;} run();run();', + ); + expect(onlyBinding(ir, "saved").provenance.status).toBe("ambiguous"); +}); + +it.each(["||=", "??="])( + "does not treat %s as a fresh reset before eval", + (operator) => { + const ir = analyzeJavaScriptSemantics( + `let value; function run(){value ${operator} 1;const saved=(0,value);eval('value=require("node:fs")');return saved;}run();run();`, + ); + expect(onlyBinding(ir, "saved").provenance.status).toBe("ambiguous"); + }, +); + it.each([ 'eval("var value=2"); function run(){const value=1;return value;}', 'export function run(){const value={n:1};const saved=value.n;eval("value.n=2");return saved;}', diff --git a/src/domain/javascript/javascriptSemanticAnalysis.ts b/src/domain/javascript/javascriptSemanticAnalysis.ts index a2c0384af1..d5a3f3e0be 100644 --- a/src/domain/javascript/javascriptSemanticAnalysis.ts +++ b/src/domain/javascript/javascriptSemanticAnalysis.ts @@ -3,6 +3,7 @@ import { indexSemanticOpaqueEffects, semanticOpaqueBindingEffects, semanticBindingCanBeRebound, + semanticOpaqueBindingReadMayHaveChanged, } from "./javascriptSemanticOpaqueEffects.js"; import * as t from "@babel/types"; @@ -874,7 +875,7 @@ export function* indexJavaScriptElectronIdentitySteps( const writes = new SemanticPropertyPathCoverage(); let collectedWrites = false; const exportsByBinding = new Map(); - const rootExport = (root: t.Node): readonly string[] | undefined => { + const rootOrigin = (root: t.Node): readonly string[] | undefined => { if (t.isIdentifier(root)) { if (semanticResolutionBlocked(state, root, root.name)) return undefined; const binding = resolveSemanticBindingState(state, root, root.name); @@ -899,13 +900,26 @@ export function* indexJavaScriptElectronIdentitySteps( ? electronExportPath(origin.importedPath) : undefined; }; + const rootExport = (root: t.Node): readonly string[] | undefined => { + // Possible origins must still cover writes through uncertain aliases. + // Admit positive reads separately, without caching their temporal validity. + if (state.opaqueEffects.byScope.size > 0 && t.isIdentifier(root)) { + const binding = resolveSemanticBindingState(state, root, root.name); + if ( + binding !== undefined && + semanticOpaqueBindingReadMayHaveChanged(binding, root, state) + ) + return undefined; + } + return rootOrigin(root); + }; yield* collectJavaScriptIdentityWritesSteps( program, state.erasedSyntaxNodes, (node, kind) => { if (kind === "opaque-call") return [interop.opaqueMutationPath(node)]; const { root, members } = electronMemberPath(node); - const exported = kind === "binding" ? undefined : rootExport(root); + const exported = kind === "binding" ? undefined : rootOrigin(root); const path = exported === undefined ? (interop.intrinsicPath(root) ?? @@ -969,7 +983,7 @@ export function* indexJavaScriptElectronIdentitySteps( invocationPath, mayChangeAppPath: createSemanticElectronAppPathChangeQuery( state, - rootExport, + rootOrigin, interop, ), }; diff --git a/src/domain/javascript/javascriptSemanticMutationInitializers.ts b/src/domain/javascript/javascriptSemanticMutationInitializers.ts index 0286b6f677..8953f9b27b 100644 --- a/src/domain/javascript/javascriptSemanticMutationInitializers.ts +++ b/src/domain/javascript/javascriptSemanticMutationInitializers.ts @@ -556,6 +556,25 @@ export const semanticDefiniteEffectPrecedesReference = ( ); }; +/** Order a selected value before a later effect in this non-repeating body. */ +export const semanticReadPrecedesEffectWithinActivation = ( + read: t.Node, + effect: t.Node, + parents: WeakMap, +): boolean => { + const before = bindingReadPosition(read, parents); + const after = directMutationPosition(effect, parents); + if (before.position === null || before.repeatingLoop || after === null) + return false; + if (positionPrecedes(before.position, after)) return true; + return ( + before.position.body === after.body && + before.position.index === after.index && + before.position.declaratorIndex === after.declaratorIndex && + expressionEvaluationOrder(read, effect, parents) === "before" + ); +}; + /** Order straight-line source effects in one activation, preserving deferred/repeating unknowns. */ export const semanticEffectTimingWithinActivation = ( site: t.Node, @@ -1109,6 +1128,33 @@ export const semanticBindingEffectFollowsCapture = ( ); }; +/** A local declaration resets its lexical value before an ordinary member read. */ +export const semanticBindingEffectFollowsRead = ( + binding: JavaScriptSemanticBindingState, + read: t.Node, + effect: t.Node, + parents: WeakMap, +): boolean => { + const initializer = binding.initializers[0]; + if (binding.initializers.length !== 1 || initializer === undefined) + return false; + const declaration = parents.get(initializer.node); + // A logical/compound assignment may preserve a value from a prior call. + if ( + !t.isVariableDeclarator(declaration) || + declaration.init !== initializer.node + ) + return false; + const initialized = initializerPosition(initializer, parents); + const selected = bindingReadPosition(read, parents).position; + return ( + initialized !== null && + selected !== null && + positionPrecedes(initialized, selected) && + semanticReadPrecedesEffectWithinActivation(read, effect, parents) + ); +}; + /** Ignore later escapes only while reading earlier references in one activation. */ export const semanticEscapeFollowsCapture = ( binding: JavaScriptSemanticBindingState, diff --git a/src/domain/javascript/javascriptSemanticOpaqueEffects.ts b/src/domain/javascript/javascriptSemanticOpaqueEffects.ts index 9d91402419..efdd17bbb7 100644 --- a/src/domain/javascript/javascriptSemanticOpaqueEffects.ts +++ b/src/domain/javascript/javascriptSemanticOpaqueEffects.ts @@ -6,6 +6,8 @@ import { createSemanticModuleInterop } from "./javascriptModuleInterop.js"; import { semanticEffectTiming, semanticBindingEffectFollowsCapture, + semanticBindingEffectFollowsRead, + semanticReadPrecedesEffectWithinActivation, } from "./javascriptSemanticMutationInitializers.js"; import { isUnshadowedGlobal, @@ -134,6 +136,17 @@ export const semanticBindingMayHaveOpaqueEffect = ( read, effect, state.parentsByNode, + ) && + // Rebinding a freshly initialized local is activation-specific; + // mutations to the referenced object may persist across calls. + !( + kind === "binding" && + semanticBindingEffectFollowsRead( + binding, + read, + effect, + state.parentsByNode, + ) )) ) return true; @@ -186,7 +199,13 @@ export const semanticOpaqueNameMayHaveChanged = ( (binding === undefined || resolveSemanticBindingState(state, effect, binding.name) ?.bindingId === binding.bindingId) && - semanticEffectTiming(read, effect, state) !== "after", + semanticEffectTiming(read, effect, state) !== "after" && + // A sloppy-eval name belongs to this activation, not a prior call. + !semanticReadPrecedesEffectWithinActivation( + read, + effect, + state.parentsByNode, + ), ) ) return true; diff --git a/tests/acceptance/applications/electronOpaqueBindingCli.test.ts b/tests/acceptance/applications/electronOpaqueBindingCli.test.ts new file mode 100644 index 0000000000..8b573ad27f --- /dev/null +++ b/tests/acceptance/applications/electronOpaqueBindingCli.test.ts @@ -0,0 +1,73 @@ +import { writeFile } from "node:fs/promises"; +import { join } from "node:path"; + +import { expect } from "vitest"; +import { z } from "zod"; + +import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascript/javascriptApplicationAnalysis.js"; +import { connectLocalToolsMcp } from "../../fixtures/localToolsMcp.js"; +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; + +cliTest.for(["CLI", "stdio MCP"] as const)( + "retains only the pre-eval Electron window through %s", + async (surface, { cli }) => { + const root = await createTestTempDirectory("rea-electron-opaque-binding-"); + await writeFile( + join(root, "package.json"), + JSON.stringify({ main: "main.cjs" }), + ); + const source = [ + '(()=>{var m={45840:e=>{e.exports=require("electron")},100:(e,t,n)=>{', + '"use strict";let s=n(45840);', + "new s.BrowserWindow({webPreferences:{nodeIntegration:true}});", + 'eval("s={BrowserWindow:class{}}");', + "new s.BrowserWindow({webPreferences:{nodeIntegration:false}});", + "}},c={};function n(t){var r=c[t];if(void 0!==r)return r.exports;var o=c[t]={exports:{}};", + "return m[t](o,o.exports,n),o.exports}n(100)})();", + 'throw new Error("must not execute fixture");', + ].join("\n"); + await writeFile(join(root, "main.cjs"), source); + let document: unknown; + if (surface === "CLI") { + const response = await cli.run({ + arguments: [ + "analyze-javascript-application", + root, + "--json", + "--max-heap-mb", + "128", + "--analysis-timeout-ms", + "10000", + ], + }); + expect(response.exitCode, response.stderr).toBe(0); + document = response.json; + } else { + const { call } = await connectLocalToolsMcp(); + const response = await call("analyze_javascript_application", { + input_path: root, + max_heap_mb: 128, + analysis_timeout_ms: 10000, + }); + expect(response.isError, JSON.stringify(response)).not.toBe(true); + document = response.structuredContent; + } + const result = z + .object({ + normalized_result: javascriptApplicationAnalysisResultSchema, + }) + .parse(document).normalized_result; + expect(result.summary.browser_windows).toBe(1); + expect(result.statistics.parse_failures).toBe(0); + expect(result.statistics.parsed_javascript_files).toBe(1); + const windows = result.graph.nodes.filter( + ({ kind }) => kind === "browser-window", + ); + expect(windows).toHaveLength(1); + expect(windows[0]?.observations[0]?.evidence.location).toMatchObject({ + available: true, + value: { kind: "source-range", source: "main.cjs", start: { line: 3 } }, + }); + }, +);