fix(javascript): invalidate Electron reads after direct eval (#7053)

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);
+});

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;}',

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<string, readonly string[] | null>();
-  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,
     ),
   };

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<t.Node, t.Node>,
+): 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<t.Node, t.Node>,
+): 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,

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;

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 } },
+    });
+  },
+);