fix(javascript): retain write identity and operand evaluation order

src/domain/javascript/electronIntrinsicCapture.test.ts

@@ -11,6 +11,26 @@ const WINDOW = `let electron = require("electron");
   electron = __toESM(electron, 1); new electron.BrowserWindow({});`;
 
 describe("captured intrinsic invocation reads", () => {
+  it("evaluates assignment operands before recording their writes", () => {
+    const result = analyzeJavaScriptStaticSource(`
+      const define = Object.defineProperty;
+      const metadata = {};
+      Object.defineProperty = (Object.defineProperty(metadata, "x", {value:1}), replacement);
+      ${CAPTURED_INTEROP} ${WINDOW}
+    `);
+    expect(result.electron.browser_windows).toHaveLength(1);
+  });
+
+  it("retains whole-object mutations at a captured function's later member read", () => {
+    const result = analyzeJavaScriptStaticSource(`
+      const define = Object.defineProperty;
+      Object.assign(define, {call: replacement});
+      const metadata = {}; define.call(null, metadata, "x", {value:1});
+      ${CAPTURED_INTEROP} ${WINDOW}
+    `);
+    expect(result.electron.browser_windows).toEqual([]);
+  });
+
   it.each([
     '(0, Object.defineProperty.call)(null,Object,"create",{value:replacement});',
     '(0, Object.defineProperty.apply)(null,[Object,"create",{value:replacement}]);',

src/domain/javascript/javascriptElectronMemberWrites.ts

@@ -52,6 +52,24 @@ export interface JavaScriptIntrinsicInvocation {
     | undefined;
 }
 
+/** Distinguish replacing a value from mutating members of the captured object. */
+export type JavaScriptIdentityWriteKind =
+  | "replace-value"
+  | "mutate-object"
+  | "opaque";
+
+/** The source boundary after the operands required for this effect are evaluated. */
+export const javaScriptIdentityWriteOffset = (node: t.Node): number =>
+  (t.isForOfStatement(node) || t.isForInStatement(node)
+    ? node.right.end
+    : t.isCallExpression(node) ||
+        t.isOptionalCallExpression(node) ||
+        t.isAssignmentExpression(node) ||
+        t.isUpdateExpression(node) ||
+        t.isUnaryExpression(node, { operator: "delete" })
+      ? node.end
+      : node.start) ?? Infinity;
+
 /**
  * Index writes through possible lexical object identities. Resolve the complete
  * object before appending the written key: self-reference properties can denote
@@ -68,7 +86,11 @@ export function* collectJavaScriptIdentityWritesSteps(
   ) => JavaScriptIntrinsicInvocation | undefined,
   staticString: (node: t.Node | undefined) => string | undefined,
   freshObject: (node: t.Node) => boolean,
-  retain: (node: t.Node, path: readonly (string | null)[]) => void,
+  retain: (
+    node: t.Node,
+    path: readonly (string | null)[],
+    kind: JavaScriptIdentityWriteKind,
+  ) => void,
 ): Generator<void> {
   const objectMutators = new Set([
     "defineProperty",
@@ -83,87 +105,105 @@ export function* collectJavaScriptIdentityWritesSteps(
     "setPrototypeOf",
   ]);
   const keyedMutators = new Set(["set", "deleteProperty", "defineProperty"]);
-  yield* traverseJavaScriptAstSteps(program, {
-    enter: (node) => {
-      const target = t.isAssignmentExpression(node)
-        ? node.left
-        : t.isUpdateExpression(node)
-          ? node.argument
-          : t.isUnaryExpression(node, { operator: "delete" })
-            ? node.argument
-            : t.isForOfStatement(node) || t.isForInStatement(node)
-              ? node.left
-              : undefined;
-      if (target === undefined) return;
-      for (const leaf of semanticWriteTargets(target)) {
-        if (t.isIdentifier(leaf)) {
-          for (const identity of origin(leaf, "binding"))
-            retain(node, identity);
-          continue;
+  const retainCallEffects = (
+    node: t.CallExpression | t.OptionalCallExpression,
+  ): void => {
+    const invocation = intrinsicInvocation(node);
+    if (invocation?.kind === "opaque") {
+      for (const scope of origin(node, "opaque-call"))
+        retain(node, scope, "opaque");
+      return;
+    }
+    const callee = invocation?.path;
+    const arguments_ = invocation?.arguments;
+    const method = callee?.at(-1);
+    const owner = callee?.length === 3 ? callee[1] : undefined;
+    if (
+      (owner === "Object" && objectMutators.has(method ?? "")) ||
+      (owner === "Reflect" && reflectMutators.has(method ?? ""))
+    ) {
+      // Reflect.set can write through its explicit receiver as well as
+      // dispatching through its target. Both identities need examination.
+      const targets =
+        owner === "Reflect" &&
+        method === "set" &&
+        arguments_ !== undefined &&
+        (arguments_.length >= 4 || arguments_.some(t.isSpreadElement))
+          ? [
+              arguments_[0],
+              arguments_.some(t.isSpreadElement) ? undefined : arguments_[3],
+            ]
+          : [arguments_?.[0]];
+      for (const target of targets) {
+        // A known mutator with an opaque argument list can overwrite any
+        // captured intrinsic. Do not turn an unexamined target into proof.
+        if (target === undefined || t.isSpreadElement(target)) {
+          for (const scope of origin(node, "opaque-call"))
+            retain(node, scope, "opaque");
         }
-        const member = unwrapJavaScriptExpression(leaf).node;
-        if (
-          !t.isMemberExpression(member) &&
-          !t.isOptionalMemberExpression(member)
-        )
-          continue;
-        const key = semanticStaticPropertyKey(member.property, member.computed);
-        for (const identity of origin(member.object, "member"))
-          retain(node, [...identity, key]);
-      }
-    },
-    exit: (node) => {
-      if (t.isCallExpression(node) || t.isOptionalCallExpression(node)) {
-        const invocation = intrinsicInvocation(node);
-        if (invocation?.kind === "opaque") {
-          for (const scope of origin(node, "opaque-call")) retain(node, scope);
-          return;
-        }
-        const callee = invocation?.path;
-        const arguments_ = invocation?.arguments;
-        const method = callee?.at(-1);
-        const owner = callee?.length === 3 ? callee[1] : undefined;
-        if (
-          (owner === "Object" && objectMutators.has(method ?? "")) ||
-          (owner === "Reflect" && reflectMutators.has(method ?? ""))
-        ) {
-          // Reflect.set can write through its explicit receiver as well as
-          // dispatching through its target. Both identities need examination.
-          const targets =
-            owner === "Reflect" &&
-            method === "set" &&
-            arguments_ !== undefined &&
-            (arguments_.length >= 4 || arguments_.some(t.isSpreadElement))
-              ? [
-                  arguments_[0],
-                  arguments_.some(t.isSpreadElement)
-                    ? undefined
-                    : arguments_[3],
-                ]
-              : [arguments_?.[0]];
-          for (const target of targets) {
-            // A known mutator with an opaque argument list can overwrite any
-            // captured intrinsic. Do not turn an unexamined target into proof.
-            if (target === undefined || t.isSpreadElement(target)) {
-              for (const scope of origin(node, "opaque-call"))
-                retain(node, scope);
-            }
-            if (t.isNode(target) && !t.isSpreadElement(target)) {
-              const identities = origin(target, "member");
-              if (identities.length > 0) {
-                const key = keyedMutators.has(method ?? "")
-                  ? [staticString(arguments_?.[1]) ?? null]
-                  : [];
-                for (const identity of identities)
-                  retain(node, [...identity, ...key]);
-              } else if (!freshObject(target)) {
-                for (const scope of origin(node, "opaque-call"))
-                  retain(node, scope);
-              }
-            }
+        if (t.isNode(target) && !t.isSpreadElement(target)) {
+          const identities = origin(target, "member");
+          if (identities.length > 0) {
+            const key = keyedMutators.has(method ?? "")
+              ? [staticString(arguments_?.[1]) ?? null]
+              : [];
+            for (const identity of identities)
+              retain(
+                node,
+                [...identity, ...key],
+                keyedMutators.has(method ?? "")
+                  ? "replace-value"
+                  : "mutate-object",
+              );
+          } else if (!freshObject(target)) {
+            for (const scope of origin(node, "opaque-call"))
+              retain(node, scope, "opaque");
           }
         }
       }
+    }
+  };
+  const retainTargets = (node: t.Node): void => {
+    const target = t.isAssignmentExpression(node)
+      ? node.left
+      : t.isUpdateExpression(node)
+        ? node.argument
+        : t.isUnaryExpression(node, { operator: "delete" })
+          ? node.argument
+          : t.isForOfStatement(node) || t.isForInStatement(node)
+            ? node.left
+            : undefined;
+    if (target === undefined) return;
+    for (const leaf of semanticWriteTargets(target)) {
+      if (t.isIdentifier(leaf)) {
+        for (const identity of origin(leaf, "binding"))
+          retain(node, identity, "replace-value");
+        continue;
+      }
+      const member = unwrapJavaScriptExpression(leaf).node;
+      if (
+        !t.isMemberExpression(member) &&
+        !t.isOptionalMemberExpression(member)
+      )
+        continue;
+      const key = semanticStaticPropertyKey(member.property, member.computed);
+      for (const identity of origin(member.object, "member"))
+        retain(node, [...identity, key], "replace-value");
+    }
+  };
+  const loopBodies = new WeakMap<t.Node, t.ForOfStatement | t.ForInStatement>();
+  yield* traverseJavaScriptAstSteps(program, {
+    enter: (node) => {
+      if (t.isForOfStatement(node) || t.isForInStatement(node))
+        loopBodies.set(node.body, node);
+      const loop = loopBodies.get(node);
+      if (loop !== undefined) retainTargets(loop);
+    },
+    exit: (node) => {
+      if (t.isCallExpression(node) || t.isOptionalCallExpression(node))
+        retainCallEffects(node);
+      else if (!t.isForOfStatement(node) && !t.isForInStatement(node))
+        retainTargets(node);
     },
   });
 }

src/domain/javascript/javascriptModuleInterop.ts

@@ -17,6 +17,8 @@ import {
 } from "./javascriptSemanticTraversal.js";
 import {
   electronMemberPath,
+  javaScriptIdentityWriteOffset,
+  type JavaScriptIdentityWriteKind,
   type JavaScriptIntrinsicInvocation,
 } from "./javascriptElectronMemberWrites.js";
 import {
@@ -99,7 +101,17 @@ export const createSemanticModuleInterop = (
     readonly scope: string;
     readonly offset: number;
     readonly path: readonly (string | null)[];
+    readonly kind: JavaScriptIdentityWriteKind;
   }[] = [];
+  const invocationMemberWriteCovers = (
+    write: (typeof writes)[number],
+    read: readonly string[],
+  ): boolean =>
+    (write.kind === "opaque" ||
+      (write.kind === "mutate-object" &&
+        write.path.length === read.length - 1) ||
+      (write.kind === "replace-value" && write.path.length === read.length)) &&
+    intrinsicWriteCovers(write.path, read);
   const invalidatedBindings = new Map<string, boolean>();
   const invalidatedCalls = new WeakMap<t.Node, boolean>();
   const opaqueMutationPath = (node: t.Node): readonly string[] => {
@@ -293,9 +305,8 @@ export const createSemanticModuleInterop = (
             (write) =>
               (write.scope === scope || write.scope === "intrinsic") &&
               write.offset < offset &&
-              ((write.path.length >= currentPath.length &&
-                intrinsicWriteCovers(write.path, currentPath)) ||
-                intrinsicWriteCovers(write.path, [
+              (invocationMemberWriteCovers(write, currentPath) ||
+                invocationMemberWriteCovers(write, [
                   "intrinsic",
                   "Function",
                   "prototype",
@@ -711,11 +722,16 @@ export const createSemanticModuleInterop = (
     staticString,
     isFreshObject,
     opaqueMutationPath,
-    retainWrite: (node: t.Node, path: readonly (string | null)[]) => {
+    retainWrite: (
+      node: t.Node,
+      path: readonly (string | null)[],
+      kind: JavaScriptIdentityWriteKind = "replace-value",
+    ) => {
       writes.push({
         scope: opaqueMutationPath(node).join("."),
-        offset: node.start ?? Infinity,
+        offset: javaScriptIdentityWriteOffset(node),
         path: path[0] === "opaque-intrinsic" ? ["intrinsic"] : path,
+        kind,
       });
     },
     invalidated: (binding: JavaScriptSemanticBindingState): boolean => {

src/domain/javascript/javascriptSemanticAnalysis.ts

@@ -462,9 +462,9 @@ export function* indexJavaScriptElectronIdentitySteps(
     interop.intrinsicInvocation,
     interop.staticString,
     interop.isFreshObject,
-    (node, path) => {
+    (node, path, kind) => {
       if (path[0] === "electron") writes.retain(path);
-      else interop.retainWrite(node, path);
+      else interop.retainWrite(node, path, kind);
     },
   );
   collectedWrites = true;