From f71236400f3009ebbd2af5dfb8faf71cf710c0f6 Mon Sep 17 00:00:00 2001 From: RRFZ <247457381+Plastic-time@users.noreply.github.com> Date: Thu, 29 Oct 2026 21:25:43 -0400 Subject: [PATCH] fix(javascript): confirm Electron renderer evidence with the static pass The focused evidence check from 4a86bca9b also counted renderer calls in bundler factory defaults that the bundler convention never runs, which the static pass skips. Such a default switched a type-module file to script context, so its unrelated unbound requires became imports again. The focused check now only screens. It follows the static pass's own traversal, so a negative result still rules evidence out. A positive result is confirmed by the static pass's direct findings on the unscoped parse, and those findings become the file's facts when they hold. --- .../javascriptAnalysisWorkerProcess.ts | 21 ++++-- .../electronScriptLoaderEvidence.test.ts | 38 +++++----- .../electronScriptLoaderEvidence.ts | 70 ++++++++++++------- .../applications/unboundRequires.test.ts | 20 +++++- 4 files changed, 99 insertions(+), 50 deletions(-) diff --git a/src/composition/javascriptAnalysisWorkerProcess.ts b/src/composition/javascriptAnalysisWorkerProcess.ts index c704d460c0..96bda15eb1 100644 --- a/src/composition/javascriptAnalysisWorkerProcess.ts +++ b/src/composition/javascriptAnalysisWorkerProcess.ts @@ -1,10 +1,12 @@ import { - electronRendererEvidenceSteps, + hasElectronRendererEvidence, + mayHaveElectronRendererEvidenceSteps, mayRunUnderElectronScriptLoader, } from "../domain/javascript/electronScriptLoaderEvidence.js"; import { moduleResolutionModeForJavaScriptReference } from "../domain/javascript/javascriptModuleResolutionMode.js"; import { parseJavaScriptSource } from "../domain/javascript/javascriptSourceParser.js"; import { analyzeParsedJavaScriptStaticSourceSteps } from "../domain/javascript/javascriptStaticAnalysis.js"; +import type { JavaScriptStaticAnalysis } from "../domain/javascript/javascriptStaticAnalysisTypes.js"; import { analyzeParsedJavaScriptSemanticsSteps } from "../domain/javascript/javascriptSemanticAnalysis.js"; import { failedJavaScriptStaticAnalysis } from "../domain/javascript/javascriptStaticAnalysisHelpers.js"; import { projectJavaScriptModuleSemantics } from "../domain/javascript/javascriptModuleSemanticIr.js"; @@ -35,20 +37,29 @@ const analyze: JavaScriptSourceWorkerAnalyzer = async ( return null; } await callbacks.phase("static"); + let facts: JavaScriptStaticAnalysis | undefined; if (mayRunUnderElectronScriptLoader(parsed.program)) { // Renderer-side evidence is read outside the package scope, where an // unbound require keeps its Electron identity; with that evidence the // whole source keeps that context, so its static and semantic facts agree. + // The screen rules evidence out cheaply; only the static pass confirms it. const script = parseJavaScriptSource(file.text.value, file.path); if ( script !== null && (await completeJavaScriptAnalysisSteps( - electronRendererEvidenceSteps(script.program), + mayHaveElectronRendererEvidenceSteps(script.program), )) - ) - parsed = script; + ) { + const scriptFacts = await completeJavaScriptAnalysisSteps( + analyzeParsedJavaScriptStaticSourceSteps(file.text.value, script), + ); + if (hasElectronRendererEvidence(scriptFacts.electron)) { + parsed = script; + facts = scriptFacts; + } + } } - const facts = await completeJavaScriptAnalysisSteps( + facts ??= await completeJavaScriptAnalysisSteps( analyzeParsedJavaScriptStaticSourceSteps(file.text.value, parsed), ); const resultBudgetBytes = await callbacks.checkpoint(facts); diff --git a/src/domain/javascript/electronScriptLoaderEvidence.test.ts b/src/domain/javascript/electronScriptLoaderEvidence.test.ts index fe290cdeeb..dd7e102f13 100644 --- a/src/domain/javascript/electronScriptLoaderEvidence.test.ts +++ b/src/domain/javascript/electronScriptLoaderEvidence.test.ts @@ -1,7 +1,8 @@ import { expect, it } from "vitest"; import { - electronRendererEvidenceSteps, + hasElectronRendererEvidence, + mayHaveElectronRendererEvidenceSteps, mayRunUnderElectronScriptLoader, } from "./electronScriptLoaderEvidence.js"; import { completeSemanticSteps } from "./javascriptSemanticTraversal.js"; @@ -66,50 +67,55 @@ it.each([ "a destructured contextBridge exposure", 'const { contextBridge } = require("electron");\ncontextBridge.exposeInMainWorld("api", {});', true, + true, ], [ "a direct ipcRenderer send", 'require("electron").ipcRenderer.send("ready");', true, + true, ], [ "a template-literal specifier", 'require(`electron`).ipcRenderer.send("ready");', true, + true, ], [ "a renderer IPC listener", 'const { ipcRenderer } = require("electron");\nipcRenderer.on("update", () => {});', true, + true, + ], + [ + "a renderer call in a never-run bundler factory default", + 'globalThis.webpackChunkApp.push([[1], {\n 7(module = require("electron").ipcRenderer.send("unused"), exports, load) {}\n}]);', + true, + false, ], [ "main-process IPC only", 'const { app, ipcMain } = require("electron");\nipcMain.handle("ping", () => app.getName());', false, + false, ], - ["no Electron operation", 'require("electron");', false], + ["no Electron operation", 'require("electron");', false, false], [ "a locally bound require", 'function load(require) {\n require("electron").contextBridge.exposeInMainWorld("api", {});\n}', false, + false, ], ] as const)( - "reads renderer-side Electron evidence from %s", - (_name, source, expected) => { + "screens and confirms renderer-side Electron evidence from %s", + (_name, source, screened, confirmed) => { const parsed = parse(source, "preload.js"); expect( - completeSemanticSteps(electronRendererEvidenceSteps(parsed.program)), - ).toBe(expected); - // The focused check agrees with the full static pass's direct findings. + completeSemanticSteps( + mayHaveElectronRendererEvidenceSteps(parsed.program), + ), + ).toBe(screened); const { electron } = analyzeParsedJavaScriptStaticSource(source, parsed); - expect( - electron.context_bridge_apis.some( - ({ resolution_basis }) => resolution_basis === undefined, - ) || - electron.ipc.some( - ({ side, resolution_basis }) => - side === "renderer" && resolution_basis === undefined, - ), - ).toBe(expected); + expect(hasElectronRendererEvidence(electron)).toBe(confirmed); }, ); diff --git a/src/domain/javascript/electronScriptLoaderEvidence.ts b/src/domain/javascript/electronScriptLoaderEvidence.ts index 4f0a508fa9..9b09ab642b 100644 --- a/src/domain/javascript/electronScriptLoaderEvidence.ts +++ b/src/domain/javascript/electronScriptLoaderEvidence.ts @@ -1,6 +1,7 @@ import * as t from "@babel/types"; import { ipcDescriptor } from "./electronStaticAnalysisIpc.js"; +import type { ElectronStaticFindings } from "./electronStaticAnalysisTypes.js"; import { electronCalleePath } from "./electronStaticAnalysisValues.js"; import { contextBridgeExposureArguments } from "./javascriptContextBridgeExposure.js"; import { ELECTRON_MODULE } from "./javascriptElectronMemberWrites.js"; @@ -11,7 +12,7 @@ import { import { hasPossibleCommonJsDelegateWrite } from "./javascriptSemanticRequireOrigin.js"; import { traverseJavaScriptAst, - traverseJavaScriptValueAstSteps, + traverseJavaScriptAstSteps, } from "./javascriptSemanticTraversal.js"; import { javaScriptRuntimeContext } from "./javascriptSourceParser.js"; import { stringValue } from "./javascriptStaticAnalysisHelpers.js"; @@ -26,7 +27,7 @@ import { stringValue } from "./javascriptStaticAnalysisHelpers.js"; * the source a module under every loader, so only scripts qualify. The * source must also name `contextBridge` or `ipcRenderer`: Electron identity * reaches a member only through an identifier, string, or template key, so a - * source naming neither cannot yield {@link electronRendererEvidenceSteps}. + * source naming neither cannot yield {@link hasElectronRendererEvidence}. */ export const mayRunUnderElectronScriptLoader = ( program: t.Program, @@ -61,14 +62,16 @@ const rendererModules: ReadonlySet = new Set([ ]); /** - * Whether a source directly calls a `contextBridge` exposure or a renderer IPC - * operation. Both are renderer-process Electron modules that the main process - * cannot obtain, so such a call places the source under a renderer-side - * loader. The check runs only the static pass's binding and Electron identity - * preamble and its exposure and IPC descriptors, without the rest of that - * pass; wrapper substitutions are inferred, so they are not expanded. + * Whether the static pass could directly observe a `contextBridge` exposure + * or a renderer IPC operation in this source. Both are renderer-process + * Electron modules that the main process cannot obtain. This screen runs the + * pass's binding and Electron identity preamble, source traversal and + * exposure and IPC descriptors, but not its bundler recognition. It therefore + * also counts factory defaults that the bundler convention never runs, which + * only that pass excludes: a false result rules evidence out, and a true one + * still needs {@link hasElectronRendererEvidence} on the pass's findings. */ -export function* electronRendererEvidenceSteps( +export function* mayHaveElectronRendererEvidenceSteps( program: t.Program, ): Generator { const index = yield* indexJavaScriptBindingsWithReceiversSteps(program); @@ -81,23 +84,38 @@ export function* electronRendererEvidenceSteps( ); const context = { electronBindings: identity.bindings }; let found = false; - yield* traverseJavaScriptValueAstSteps( - program, - index.bindings.erasedSyntaxNodes, - { - skip: () => found, - enter: (node) => { - if (!t.isCallExpression(node) && !t.isOptionalCallExpression(node)) - return; - const path = electronCalleePath(node.callee, context); - found ||= - (path?.length === 2 && - path[0] === "contextBridge" && - contextBridgeExposureArguments(path[1] ?? "", node.arguments) !== - undefined) || - ipcDescriptor(path)?.side === "renderer"; - }, + yield* traverseJavaScriptAstSteps(program, { + skip: () => found, + enter: (node) => { + if ( + index.bindings.erasedSyntaxNodes.has(node) || + (!t.isCallExpression(node) && !t.isOptionalCallExpression(node)) + ) + return; + const path = electronCalleePath(node.callee, context); + found ||= + (path?.length === 2 && + path[0] === "contextBridge" && + contextBridgeExposureArguments(path[1] ?? "", node.arguments) !== + undefined) || + ipcDescriptor(path)?.side === "renderer"; }, - ); + }); return found; } + +/** + * Whether static findings directly observe a `contextBridge` exposure or a + * renderer IPC operation, which places their source under a renderer-side + * loader. Wrapper substitutions are inferred, so they do not count. + */ +export const hasElectronRendererEvidence = ( + electron: ElectronStaticFindings, +): boolean => + electron.context_bridge_apis.some( + ({ resolution_basis }) => resolution_basis === undefined, + ) || + electron.ipc.some( + ({ side, resolution_basis }) => + side === "renderer" && resolution_basis === undefined, + ); diff --git a/tests/acceptance/applications/unboundRequires.test.ts b/tests/acceptance/applications/unboundRequires.test.ts index 1cfcd676be..09bc1fa3bc 100644 --- a/tests/acceptance/applications/unboundRequires.test.ts +++ b/tests/acceptance/applications/unboundRequires.test.ts @@ -365,14 +365,22 @@ cliTest( const source = (dependency: string) => `try { console.log(require(${JSON.stringify(dependency)})); } catch (error) { console.log(error.name); }\n`; // The nearest package.json owns each .js file's scope (Node packages.md, - // "Determining module system"). Requiring Electron, directly or through a - // local loader, does not give a Node-loaded module a require. + // "Determining module system"). Requiring Electron, directly, through a + // local loader, or in a bundler factory default that never runs, does not + // give a Node-loaded module a require. await writeFixtureFiles(root, { "package.json": JSON.stringify({ type: "module" }), "main.js": source("./dep.cjs"), "electron-main.js": 'try { require("electron"); console.log(require("./dep.cjs")); } catch (error) { console.log(error.name); }\n', "shadowed.js": `function load(require) { return require("electron").contextBridge; }\n${source("./dep.cjs")}`, + "dead-default.js": [ + source("./dep.cjs"), + "globalThis.webpackChunkApp = [];", + "globalThis.webpackChunkApp.push([[1], {", + ' 7(module = require("electron").ipcRenderer.send("unused"), exports, load) {}', + "}]);", + ].join("\n"), "dep.cjs": "module.exports = 1;\n", "legacy/package.json": JSON.stringify({ type: "commonjs" }), "legacy/main.js": source("../dep.cjs"), @@ -383,6 +391,7 @@ cliTest( ["main.js", "ReferenceError"], ["electron-main.js", "ReferenceError"], ["shadowed.js", "ReferenceError"], + ["dead-default.js", "ReferenceError"], ["legacy/main.js", "1"], ["untyped/main.js", "1"], ] as const) { @@ -402,7 +411,12 @@ cliTest( const { graph } = javascriptApplicationAnalysisResultSchema.parse( parseEvidence(response.json).normalized_result, ); - for (const path of ["main.js", "electron-main.js", "shadowed.js"]) + for (const path of [ + "main.js", + "electron-main.js", + "shadowed.js", + "dead-default.js", + ]) expect(sourceRequireFacts(graph, path), path).toEqual({ module_format: "unknown", import_relationships: 0,