From 81378efb71de135b3d896de141610f66ea66338d Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Fri, 30 Oct 2026 02:21:23 -0700 Subject: [PATCH] fix(electron): keep literal IPC sends ambiguous beside unresolved handlers (#8120) * fix(electron): keep literal IPC sends ambiguous beside unresolved handlers A main handler on an unresolved channel can receive any literal renderer send of a compatible mode, but pairing ignored it unless it had a scoped receiver. The send was then reported unpaired, or paired with another handler. Treat an unresolved-channel handler as a competitor that never pairs. The wrapper pass seeds only calls with a literal first argument, so it now records which invocations of a dynamic site it traced. The syntax finding gains `invocation_channels: "literal"` only when every invocation is a traced direct call of an unexported local function supplying a literal channel. Such a site is already represented by its per-invocation findings and does not compete. Fixes #8118 Co-Authored-By: Claude Opus 5.5 * fix(electron): keep guarded wrapper registrations competing in IPC pairing A guarded wrapper registration keeps its predicate on a call path and has no IPC finding. It still counted toward `invocation_channels: "literal"`, so pairing lost it: a send on that channel was reported unpaired, or paired uniquely with a direct handler. The enumerated site now carries these channels as `guarded_invocation_channels`, and each one competes for its own channel without pairing. Index each binding's direct call sites by appending in place. Spreading the earlier calls on every reference was quadratic in a file's call count. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- docs/javascript-artifact-reconstruction.md | 8 +- .../javascript/ElectronBoundaryAnalysis.ts | 33 +++- .../javascript/electronStaticAnalysisTypes.ts | 17 +- .../javascript/electronWrappedCallPaths.ts | 175 ++++++++++++++++-- .../JavaScriptAnalysisWorkerSchemas.ts | 2 + .../electronDynamicIpcHandlersCli.test.ts | 101 ++++++++++ 6 files changed, 317 insertions(+), 19 deletions(-) create mode 100644 tests/acceptance/applications/electronDynamicIpcHandlersCli.test.ts diff --git a/docs/javascript-artifact-reconstruction.md b/docs/javascript-artifact-reconstruction.md index 396352c5b2..88d97864a9 100644 --- a/docs/javascript-artifact-reconstruction.md +++ b/docs/javascript-artifact-reconstruction.md @@ -644,7 +644,13 @@ identity. This static check does not evaluate arbitrary call side effects or runtime registration. Missing findings do not establish that an application has no Electron boundary usage. -IPC pairing is an inference, not an observation. A renderer `invoke` pairs only +IPC pairing is an inference, not an observation. A main handler registered on an +unresolved channel may receive any literal channel, so a compatible renderer send +or `invoke` beside it is ambiguous, not unpaired. A wrapper's registration is +exempt only when every invocation is a traced direct call supplying a literal +channel, each recorded as its own finding. A guarded invocation keeps its +predicate on a call path instead, so its channel still competes without pairing. +A renderer `invoke` pairs only with one unique `ipcMain.handle`/`handleOnce` candidate on the same exact literal channel; a renderer send pairs only with one unique `ipcMain.on`/`once`/`addListener` candidate. Dynamic channels are never paired, and multiple compatible handlers diff --git a/src/application/javascript/ElectronBoundaryAnalysis.ts b/src/application/javascript/ElectronBoundaryAnalysis.ts index a38cc30b81..828c67f473 100644 --- a/src/application/javascript/ElectronBoundaryAnalysis.ts +++ b/src/application/javascript/ElectronBoundaryAnalysis.ts @@ -68,14 +68,14 @@ export const createElectronIpcContext = ( .filter(isPairableRendererTransmission) .map((transmission) => { const owner = owners.get(transmission.key); + const channel = transmission.finding.channel; const matching = - transmission.finding.channel === null + channel === null ? [] : handlers.filter( (handler) => - (handler.finding.channel === transmission.finding.channel || - (handler.finding.receiver !== undefined && - handler.finding.channel === null)) && + (handler.finding.channel === channel || + mayReceiveChannel(handler, channel)) && compatibleIpcModes(transmission.finding, handler.finding), ); const candidates = matching.filter( @@ -94,14 +94,14 @@ export const createElectronIpcContext = ( !coverageComplete && owner != null && candidates.length === 1 && - candidates[0]?.finding.receiver === undefined && + isExactIpcHandler(candidates[0]) && !matching.some((handler) => owners.get(handler.key) == null) ? (candidates[0] ?? null) : null, status: !uncertain && candidates.length === 1 && - candidates[0]?.finding.receiver === undefined + isExactIpcHandler(candidates[0]) ? "paired" : uncertain || matching.length > 0 ? "ambiguous" @@ -240,6 +240,27 @@ const isRendererTransmission = ({ finding }: ElectronIpcRecord): boolean => const isPairableRendererTransmission = (record: ElectronIpcRecord): boolean => isRendererTransmission(record) && record.finding.operation !== "send-to-host"; +/** + * An unresolved channel may receive any literal channel, so it competes without + * ever pairing. A wrapper site whose invocations all supplied literal channels + * is represented by their findings, except guarded invocations, which have none + * and still compete for their own channels. + */ +const mayReceiveChannel = ( + { finding }: ElectronIpcRecord, + channel: string, +): boolean => + finding.channel === null && + (finding.receiver !== undefined || + finding.invocation_channels !== "literal" || + finding.guarded_invocation_channels?.includes(channel) === true); + +/** Only an ipcMain registration on the same literal channel can be paired. */ +const isExactIpcHandler = (handler: ElectronIpcRecord | undefined): boolean => + handler !== undefined && + handler.finding.receiver === undefined && + handler.finding.channel !== null; + const isMainHandler = ({ finding }: ElectronIpcRecord): boolean => finding.side === "main" && (finding.mode === "listen" || finding.mode === "handle"); diff --git a/src/domain/javascript/electronStaticAnalysisTypes.ts b/src/domain/javascript/electronStaticAnalysisTypes.ts index 7cba97065b..241a499c26 100644 --- a/src/domain/javascript/electronStaticAnalysisTypes.ts +++ b/src/domain/javascript/electronStaticAnalysisTypes.ts @@ -138,7 +138,22 @@ export type ElectronIpcFinding = ElectronIpcDescriptor & ElectronIpcFindingContext & ( | { readonly channel: string; readonly channel_expression: null } - | { readonly channel: null; readonly channel_expression: string } + | { + readonly channel: null; + readonly channel_expression: string; + /** + * Every invocation of this site is a traced direct call that supplied a + * literal channel, recorded as its own wrapper-substitution finding or + * guarded call path. Absent when any invocation may supply another value. + */ + readonly invocation_channels?: "literal"; + /** + * Channels from guarded invocations, which keep their predicate on a + * call path instead of becoming findings. Each still competes for its + * channel in pairing. + */ + readonly guarded_invocation_channels?: readonly string[]; + } ); /** Static check involving an IPC sender, frame, process, URL, or origin. */ diff --git a/src/domain/javascript/electronWrappedCallPaths.ts b/src/domain/javascript/electronWrappedCallPaths.ts index 93967c5434..4cc15a028d 100644 --- a/src/domain/javascript/electronWrappedCallPaths.ts +++ b/src/domain/javascript/electronWrappedCallPaths.ts @@ -13,6 +13,7 @@ import { resolveSemanticBindingState, semanticResolutionBlocked, type JavaScriptSemanticAnalysisState, + type JavaScriptSemanticBindingState, } from "./javascriptSemanticState.js"; import { semanticIdentifierRole } from "./javascriptSemanticProjection.js"; import type { JavaScriptElectronIdentity } from "./javascriptSemanticAnalysis.js"; @@ -138,7 +139,9 @@ const priorPath = ( }; /** One physical IPC operation, independent of how its channel was resolved. */ -const ipcSiteKey = (value: ElectronIpcFinding): string => +const ipcSiteKey = ( + value: Pick, +): string => JSON.stringify([ value.side, value.operation, @@ -432,6 +435,110 @@ export function* collectElectronWrappedCallPathsSteps( let ipcByEndpoint: Map | undefined; // Direct operations whose channel the syntax pass left unresolved, by site. let unresolvedIpcSites: Map | undefined; + // Traced invocation outcomes at IPC sites: the call sites of invocations + // that supplied a literal channel, or any other outcome. + const literalInvocationSites = new Map< + string, + { + readonly node: t.Node; + readonly calls: Set; + // Literals whose guarded registration has no IPC finding of its own. + readonly guardedChannels: Set; + } + >(); + const openInvocationSites = new Set(); + // Runtime references to each binding: direct call sites, or null when any + // other reference can invoke it from sites the trace never seeds. + let directCalls: + | Map + | undefined; + const directCallsTo = ( + binding: JavaScriptSemanticBindingState, + ): readonly t.CallExpression[] | null => { + if (directCalls === undefined) { + const references = new Map< + JavaScriptSemanticBindingState, + t.CallExpression[] | null + >(); + traverseJavaScriptValueAst(program, state.erasedSyntaxNodes, { + enter: (node, parent, readAncestors) => { + if ( + !t.isIdentifier(node) || + parent === null || + semanticIdentifierRole(node, parent, readAncestors) === null + ) + return; + const referenced = resolveSemanticBindingState( + state, + node, + node.name, + ); + if (referenced === undefined) return; + const calls = references.get(referenced); + if (calls === null) return; + if (!t.isCallExpression(parent) || parent.callee !== node) + references.set(referenced, null); + else if (calls === undefined) references.set(referenced, [parent]); + else calls.push(parent); + }, + }); + directCalls = references; + } + return directCalls.get(binding) ?? []; + }; + /** + * Whether every invocation of the function enclosing a node is a direct call + * whose own invocation chain the trace followed. Seeds start only at literal + * arguments, so an untraced call site may supply any channel. + */ + const tracedInvocationsOnly = ( + node: t.Node, + traced: ReadonlySet, + active: ReadonlySet = new Set(), + ): boolean => { + let fn = state.parentsByNode.get(node); + while (fn !== undefined && !t.isFunction(fn)) + fn = state.parentsByNode.get(fn); + // Top-level code runs once with the values the trace evaluated. + if (fn === undefined) return true; + if (active.has(fn)) return false; + const declarator = state.parentsByNode.get(fn); + const declaration = t.isFunctionDeclaration(fn) + ? fn + : t.isVariableDeclarator(declarator) && declarator.init === fn + ? state.parentsByNode.get(declarator) + : undefined; + const owner = + declaration === undefined + ? undefined + : state.parentsByNode.get(declaration); + // An exported declaration is invoked by importers this file never sees. + if ( + declaration === undefined || + t.isExportNamedDeclaration(owner) || + t.isExportDefaultDeclaration(owner) + ) + return false; + const id = t.isFunctionDeclaration(fn) + ? fn.id + : t.isVariableDeclarator(declarator) && t.isIdentifier(declarator.id) + ? declarator.id + : undefined; + const binding = + id === undefined || id === null + ? undefined + : resolveSemanticBindingState(state, id, id.name); + const calls = binding === undefined ? null : directCallsTo(binding); + return ( + calls !== null && + calls.length > 0 && + calls.every( + (call) => + traced.has(ipcInvocationKey(range(call))) && + tracedInvocationsOnly(call, traced, new Set([...active, fn])), + ) + ); + }; const ipcAtEndpoint = (): Map => { if (ipcByEndpoint !== undefined) return ipcByEndpoint; ipcByEndpoint = new Map(); @@ -825,18 +932,26 @@ export function* collectElectronWrappedCallPathsSteps( endpoint: string, guardedDispatch: boolean, ): void => { + const location = range(node); + // The site evaluated in its own scope is not an invocation of it. + const invocationSite = + context.calls.length > 0 + ? ipcSiteKey({ ...descriptor, location }) + : undefined; + const open = (): void => { + if (invocationSite !== undefined) openInvocationSites.add(invocationSite); + }; const first = args[0]; if (first?.kind !== "literal" || typeof first.value !== "string") { accumulator.unknownFindings += 1; - return; + return open(); } const channelContext = metadataContext(context, first); - if (!channelContext) return; + if (!channelContext) return open(); const handlerValue = args[1] ?? symbolicUnknown("missing handler"); const handlerContext = metadataContext(channelContext, handlerValue); - if (!handlerContext) return; + if (!handlerContext) return open(); const channel = first.value; - const location = range(node); const invocationLocation = javascriptSourceRangeKey(range(context.entryNode)) === javascriptSourceRangeKey(location) @@ -936,6 +1051,21 @@ export function* collectElectronWrappedCallPathsSteps( : undefined; if (!findingGuarded && registrationKey !== undefined) unconditionalRegistrations.add(registrationKey); + // A guarded registration keeps its predicate on the call path instead of + // becoming an IPC finding, so the site carries its channel to pairing. + const guardedOnly = + findingGuarded && + (registrationKey === undefined || + !unconditionalRegistrations.has(registrationKey)); + if (invocationSite !== undefined) { + let traced = literalInvocationSites.get(invocationSite); + if (traced === undefined) { + traced = { node, calls: new Set(), guardedChannels: new Set() }; + literalInvocationSites.set(invocationSite, traced); + } + for (const call of context.calls) traced.calls.add(call); + if (guardedOnly) traced.guardedChannels.add(channel); + } if (!duplicate && !findingGuarded) { const value: ElectronIpcFinding = { ...finding, @@ -986,11 +1116,7 @@ export function* collectElectronWrappedCallPathsSteps( if (descriptor.mode !== "send" && descriptor.mode !== "invoke" && !fn) accumulator.unknownFindings += 1; } - } else if ( - findingGuarded && - (registrationKey === undefined || - !unconditionalRegistrations.has(registrationKey)) - ) { + } else if (guardedOnly) { accumulator.unknownFindings += 1; // A guarded registration is a path candidate even when its callback has // no observable body. Keep its predicate instead of promoting it to an @@ -1435,7 +1561,34 @@ export function* collectElectronWrappedCallPathsSteps( if (remainingVisits < 0 || resolver.truncated) { accumulator.unknownFindings += 1; accumulator.electronCallPathLimitReached = true; - } + } else + // A dynamic site's channels are enumerated only when the complete trace + // reached it with literal channels, no other invocation outcome, and no + // way to invoke its wrapper from sites the trace does not seed. + for (const [site, index] of unresolvedIpcSites ?? []) { + const entry = accumulator.ipc[index]; + const traced = literalInvocationSites.get(site); + if ( + entry?.value.channel === null && + traced !== undefined && + !openInvocationSites.has(site) && + tracedInvocationsOnly(traced.node, traced.calls) + ) + accumulator.ipc[index] = { + ...entry, + value: { + ...entry.value, + invocation_channels: "literal", + ...(traced.guardedChannels.size === 0 + ? {} + : { + guarded_invocation_channels: [...traced.guardedChannels].sort( + compareUnicodeCodePoints, + ), + }), + }, + }; + } if (resolver.unresolved) accumulator.unknownFindings += 1; return paths; } diff --git a/src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.ts b/src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.ts index 2f276d85dd..443032cbf9 100644 --- a/src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.ts +++ b/src/javascript/analysis/JavaScriptAnalysisWorkerSchemas.ts @@ -91,6 +91,8 @@ const ipcChannel = (shape: Shape) => receiver: ipcReceiver, invocation_location: range.optional(), resolution_basis: z.literal("wrapper-substitution").optional(), + invocation_channels: z.literal("literal").exactOptional(), + guarded_invocation_channels: strings.exactOptional(), ...shape, channel: z.null(), channel_expression: z.string(), diff --git a/tests/acceptance/applications/electronDynamicIpcHandlersCli.test.ts b/tests/acceptance/applications/electronDynamicIpcHandlersCli.test.ts new file mode 100644 index 0000000000..d9b9005998 --- /dev/null +++ b/tests/acceptance/applications/electronDynamicIpcHandlersCli.test.ts @@ -0,0 +1,101 @@ +import { expect } from "vitest"; + +import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; +import { cliTest } from "../../support/cli/cliFixture.js"; +import { analyzeJavaScriptDirectory } from "../../support/cli/javascriptApplication.js"; +import { writeFixtureFiles } from "../../support/javascriptApplicationFixture.js"; + +const window = `const { app, BrowserWindow, ipcMain } = require("electron"); +const path = require("node:path"); +const win = new BrowserWindow({ webPreferences: { preload: path.join(__dirname, "preload.js") } });`; +const sends = `const { ipcRenderer } = require("electron"); +ipcRenderer.send("app:log", 1); +ipcRenderer.invoke("app:get");`; + +const cases = [ + { + // An unresolved channel may receive app:log; it never handles invoke. + shape: "a directly registered computed channel", + main: `ipcMain.on(process.env.CHANNEL, () => {}); +ipcMain.handle("app:get", () => 1);`, + preload: sends, + pairing: { paired: 1, ambiguous: 1, unpaired: 0 }, + }, + { + shape: "a wrapper with one computed call site", + main: `function register(channel, handler) { ipcMain.on(channel, handler); } +register("app:log", () => {}); +register(process.env.CHANNEL, () => {});`, + preload: sends, + pairing: { paired: 0, ambiguous: 1, unpaired: 1 }, + }, + { + // Importers of the wrapper can register channels this file never names. + shape: "an exported wrapper", + main: `function register(channel, handler) { ipcMain.on(channel, handler); } +register("other", () => {}); +module.exports = register;`, + preload: sends, + pairing: { paired: 0, ambiguous: 1, unpaired: 1 }, + }, + { + // Every call site supplies a literal; the guarded one competes only for + // its own channel, so app:log still pairs with its finding. + shape: "a wrapper whose call sites are all literal", + main: `function register(channel, handler) { ipcMain.on(channel, handler); } +register("app:log", () => {}); +if (process.env.EXTRA) register("other", () => {});`, + preload: sends, + pairing: { paired: 1, ambiguous: 0, unpaired: 1 }, + }, + { + // A guarded registration has no IPC finding, yet it may receive app:log. + shape: "a guarded wrapper registration", + main: `function register(channel, handler) { ipcMain.on(channel, handler); } +if (process.env.EXTRA) register("app:log", () => {});`, + preload: sends, + pairing: { paired: 0, ambiguous: 1, unpaired: 1 }, + }, + { + // The guarded registration competes with the direct handler on app:log. + shape: "a guarded wrapper registration and a direct handler", + main: `function register(channel, handler) { ipcMain.on(channel, handler); } +if (process.env.EXTRA) register("app:log", () => {}); +ipcMain.on("app:log", () => {});`, + preload: sends, + pairing: { paired: 0, ambiguous: 1, unpaired: 1 }, + }, + { + // A renderer listener and a main-to-renderer send receive no renderer sends. + shape: "computed channels in other directions", + main: `ipcMain.on("app:log", () => {}); +win.webContents.send(process.env.CHANNEL, 1);`, + preload: `const { ipcRenderer } = require("electron"); +ipcRenderer.on(process.env.CHANNEL, () => {}); +ipcRenderer.send("app:log", 1);`, + pairing: { paired: 1, ambiguous: 0, unpaired: 0 }, + }, +] as const; + +for (const surface of ["CLI", "stdio MCP"] as const) + cliTest.for(cases)( + `keeps literal sends unknown beside $shape through ${surface}`, + async ({ main, preload, pairing }, { cli }) => { + const root = await createTestTempDirectory("rea-electron-dynamic-ipc-"); + await writeFixtureFiles(root, { + "package.json": JSON.stringify({ + name: "dynamic-ipc", + version: "1.0.0", + main: "main.js", + }), + "main.js": `${window}\n${main}`, + "preload.js": preload, + }); + const { summary } = await analyzeJavaScriptDirectory(surface, cli, root); + expect(summary.ipc).toMatchObject({ + paired_renderer_transmissions: pairing.paired, + ambiguous_renderer_transmissions: pairing.ambiguous, + unpaired_literal_renderer_transmissions: pairing.unpaired, + }); + }, + );