From 4e663beeb8fc07496236a5aba8649910527a140b Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Thu, 29 Oct 2026 05:09:45 +0800 Subject: [PATCH] fix(javascript): retain guards after labelled breaks (#7841) * fix(javascript): retain guards after labelled breaks * fix(javascript): bound labelled exit inspection * fix(javascript): preserve local break completion * fix(javascript): avoid exits through unknown control flow * fix(javascript): retain guards after bounded jump scan --- .../electronCallPathBlockPrefixes.test.ts | 28 ++- .../electronCallPathBlockPrefixes.ts | 199 ++++++++++++++++-- .../electronWrappedCallPaths.test.ts | 41 ++++ .../javascript/electronWrappedCallPaths.ts | 3 +- .../javascriptSymbolicControlFlow.ts | 16 -- .../cli/electronWrappedCallPaths.test.ts | 143 ++++++++++--- 6 files changed, 363 insertions(+), 67 deletions(-) diff --git a/src/domain/javascript/electronCallPathBlockPrefixes.test.ts b/src/domain/javascript/electronCallPathBlockPrefixes.test.ts index d2ef3db168..7834d552a4 100644 --- a/src/domain/javascript/electronCallPathBlockPrefixes.test.ts +++ b/src/domain/javascript/electronCallPathBlockPrefixes.test.ts @@ -24,9 +24,10 @@ it("indexes a real flat block once, retaining contextual guards in source order" }); const steps = prefixes.before(block, tail); expect(steps.next()).toEqual({ done: false, value: undefined }); - expect(examined).toBe(1024); + expect(examined).toBe(2048); const prefix = completeSemanticSteps(steps); - expect(examined).toBe(block.body.length); + const completeVisits = examined; + expect(completeVisits).toBeGreaterThan(block.body.length); expect(prefix).toMatchObject({ before: 2050, entries: [ @@ -54,7 +55,7 @@ it("indexes a real flat block once, retaining contextual guards in source order" const earlier = completeSemanticSteps(prefixes.before(block, statement)); expect(earlier?.entries).toBe(prefix?.entries); } - expect(examined).toBe(block.body.length); + expect(examined).toBe(completeVisits); expect(completeSemanticSteps(prefixes.before(block, first))?.before).toBe(0); }); @@ -69,10 +70,27 @@ it("does not publish an incomplete block index when its work budget ends", () => let remaining = 2; const prefixes = new ElectronCallPathBlockPrefixes(() => --remaining >= 0); expect(completeSemanticSteps(prefixes.before(block, tail))).toBeUndefined(); - remaining = block.body.length; + remaining = 100; expect(completeSemanticSteps(prefixes.before(block, tail))).toMatchObject({ before: 3, entries: [{ position: 2, fact: { kind: "branch" } }], }); - expect(remaining).toBe(0); + expect(remaining).toBeGreaterThan(0); +}); + +it("charges nested exit checks to the same budget and does not cache a partial index", () => { + const parsed = parseJavaScriptSource( + `{ if (x) { { { throw 0; } } } send(); }`, + ); + const block = parsed?.program.body[0]; + if (!t.isBlockStatement(block)) throw new Error("Expected parsed block"); + const tail = block.body.at(-1); + if (tail === undefined) throw new Error("Expected terminal statement"); + let remaining = 4; + const prefixes = new ElectronCallPathBlockPrefixes(() => --remaining >= 0); + expect(completeSemanticSteps(prefixes.before(block, tail))).toBeUndefined(); + remaining = 100; + expect(completeSemanticSteps(prefixes.before(block, tail))).toMatchObject({ + entries: [{ fact: { kind: "branch", consequentExits: true } }], + }); }); diff --git a/src/domain/javascript/electronCallPathBlockPrefixes.ts b/src/domain/javascript/electronCallPathBlockPrefixes.ts index de0f655d0e..0af1a9df08 100644 --- a/src/domain/javascript/electronCallPathBlockPrefixes.ts +++ b/src/domain/javascript/electronCallPathBlockPrefixes.ts @@ -1,5 +1,4 @@ import * as t from "@babel/types"; -import { symbolicDefiniteExit } from "./javascriptSymbolicControlFlow.js"; /** Syntax alone can establish an exit, but never a branch's contextual truth. */ export type ElectronStatementFallthrough = @@ -11,6 +10,35 @@ export type ElectronStatementFallthrough = readonly alternateExits: boolean; }; +interface Completion { + readonly fallthrough: boolean; + readonly exits: boolean; + readonly breaks: ReadonlySet; + /** Unsupported control flow may leave the current labelled statement. */ + readonly uncertain: boolean; +} + +const FALLTHROUGH: Completion = { + fallthrough: true, + exits: false, + breaks: new Set(), + uncertain: false, +}; +const EXIT: Completion = { + fallthrough: false, + exits: true, + breaks: new Set(), + uncertain: false, +}; +const UNKNOWN: Completion = { ...FALLTHROUGH, uncertain: true }; + +const mergeCompletions = (left: Completion, right: Completion): Completion => ({ + fallthrough: left.fallthrough || right.fallthrough, + exits: left.exits || right.exits, + breaks: new Set([...left.breaks, ...right.breaks]), + uncertain: left.uncertain || right.uncertain, +}); + interface PrefixEntry { readonly position: number; readonly fact: ElectronStatementFallthrough; @@ -34,23 +62,161 @@ export class ElectronCallPathBlockPrefixes { ElectronStatementFallthrough | null >(); - constructor(private readonly admitVisit: () => boolean) {} + constructor( + private readonly admitVisit: () => boolean, + private readonly parents?: WeakMap, + ) {} - statement(statement: t.Statement): ElectronStatementFallthrough | null { + /** Retain jumps that may leave an unsupported construct, without entering functions. */ + private possibleJumps(root: t.Node): Completion | undefined { + const breaks = new Set(); + let uncertain = false; + const pending: Array<{ node: t.Node; depth: number }> = [ + { node: root, depth: 0 }, + ]; + while (pending.length > 0) { + const item = pending.pop(); + if (item === undefined) break; + const { node, depth } = item; + if (!this.admitVisit()) return undefined; + if (depth >= 128) { + uncertain = true; + continue; + } + if (t.isFunction(node) || t.isClass(node)) continue; + if ( + (t.isBreakStatement(node) || t.isContinueStatement(node)) && + node.label + ) { + // Labels inside this unsupported construct consume their own jumps. + let parent = this.parents?.get(node); + let local = false; + while (parent && parent !== root) { + if (!this.admitVisit()) return undefined; + if ( + t.isLabeledStatement(parent) && + parent.label.name === node.label.name + ) { + local = true; + break; + } + parent = this.parents?.get(parent); + } + if (!local) breaks.add(node.label.name); + } + for (const key of t.VISITOR_KEYS[node.type] ?? []) { + const value = (node as unknown as Record)[key]; + const children = Array.isArray(value) ? value : [value]; + for (const child of children) { + if (t.isNode(child)) pending.push({ node: child, depth: depth + 1 }); + } + } + } + return { ...FALLTHROUGH, breaks, uncertain }; + } + + /** Track local breaks separately so unreachable statements cannot invent an exit. */ + private completion( + node: t.Node | null | undefined, + depth = 0, + ): Completion | undefined { + if (!node) return FALLTHROUGH; + if (depth >= 128) return UNKNOWN; + if (!this.admitVisit()) return undefined; + if (t.isReturnStatement(node) || t.isThrowStatement(node)) return EXIT; + if (t.isBreakStatement(node)) + return { + fallthrough: false, + exits: false, + breaks: new Set([node.label?.name ?? ""]), + uncertain: false, + }; + if (t.isLabeledStatement(node)) { + const body = this.completion(node.body, depth + 1); + if (body === undefined || !body.breaks.has(node.label.name)) return body; + const breaks = new Set(body.breaks); + breaks.delete(node.label.name); + return { ...body, fallthrough: true, breaks }; + } + if (t.isBlockStatement(node)) { + let result = FALLTHROUGH; + for (const child of node.body) { + if (!result.fallthrough) break; + const next = this.completion(child, depth + 1); + if (next === undefined) return undefined; + result = mergeCompletions({ ...result, fallthrough: false }, next); + } + return result; + } + if ( + t.isLoop(node) || + t.isSwitchStatement(node) || + t.isTryStatement(node) || + t.isWithStatement(node) + ) + return this.possibleJumps(node); + if (!t.isIfStatement(node)) return FALLTHROUGH; + const consequent = this.completion(node.consequent, depth + 1); + if (consequent === undefined) return undefined; + const alternate = this.completion(node.alternate, depth + 1); + return alternate === undefined + ? undefined + : mergeCompletions(consequent, alternate); + } + + private exits( + node: t.Node | null | undefined, + block: t.BlockStatement, + ): boolean | undefined { + const result = this.completion(node); + if (result === undefined || result.fallthrough || result.uncertain) + return result === undefined ? undefined : false; + for (const label of result.breaks) { + if (!label || !this.parents) return false; + let matched = false; + for ( + let parent = this.parents.get(block), n = 0; + parent && n < 128; + parent = this.parents.get(parent), n++ + ) { + if (!this.admitVisit()) return undefined; + if (t.isFunction(parent)) break; + if (t.isLabeledStatement(parent) && parent.label.name === label) { + matched = true; + break; + } + } + if (!matched) return false; + } + return result.exits || result.breaks.size > 0; + } + + statement( + statement: t.Statement, + block: t.BlockStatement, + ): ElectronStatementFallthrough | null | undefined { const cached = this.#facts.get(statement); if (cached !== undefined) return cached; - const fact: ElectronStatementFallthrough | null = symbolicDefiniteExit( - statement, - ) - ? { kind: "exit" } - : t.isIfStatement(statement) - ? { - kind: "branch", - test: statement.test, - consequentExits: symbolicDefiniteExit(statement.consequent), - alternateExits: symbolicDefiniteExit(statement.alternate), - } - : null; + let fact: ElectronStatementFallthrough | null; + if (t.isIfStatement(statement)) { + if (!this.admitVisit()) return undefined; + const consequent = this.exits(statement.consequent, block); + const alternate = this.exits(statement.alternate, block); + if (consequent === undefined || alternate === undefined) return undefined; + fact = + consequent && alternate + ? { kind: "exit" } + : { + kind: "branch", + test: statement.test, + consequentExits: consequent, + alternateExits: alternate, + }; + } else { + const result = this.exits(statement, block); + if (result === undefined) return undefined; + fact = result ? { kind: "exit" } : null; + } this.#facts.set(statement, fact); return fact; } @@ -70,7 +236,8 @@ export class ElectronCallPathBlockPrefixes { for (const [position, statement] of block.body.entries()) { if (!this.admitVisit()) return undefined; positions.set(statement, position); - const fact = this.statement(statement); + const fact = this.statement(statement, block); + if (fact === undefined) return undefined; if (fact !== null) entries.push({ position, fact }); if ((position + 1) % 1024 === 0) yield; } diff --git a/src/domain/javascript/electronWrappedCallPaths.test.ts b/src/domain/javascript/electronWrappedCallPaths.test.ts index 6d479c0da0..f166f14f63 100644 --- a/src/domain/javascript/electronWrappedCallPaths.test.ts +++ b/src/domain/javascript/electronWrappedCallPaths.test.ts @@ -1,6 +1,47 @@ import { describe, expect, it } from "vitest"; import { analyzeJavaScriptStaticSource } from "./javascriptStaticAnalysis.js"; +it("retains the continuing predicate after a break to an enclosing label", () => { + const result = analyzeJavaScriptStaticSource(`const e=require("electron"); + guard: { + if (!ready) break guard; + e.ipcMain.on("guarded", event => event.sender.copy()); + } + `); + expect( + result.electron.call_paths?.map(({ conditions }) => + conditions.map(({ expression }) => expression), + ), + ).toEqual([["!(!ready)"], ["!(!ready)"]]); +}); + +it("retains a labelled-break guard on preload bridge paths", () => { + const result = analyzeJavaScriptStaticSource(` + const { contextBridge, ipcRenderer } = require("electron"); + const ready = process.env.APP_READY; + guard: { + if (!ready) break guard; + contextBridge.exposeInMainWorld("api", { ping: () => ipcRenderer.invoke("ping") }); + } + `); + expect( + result.electron.call_paths?.map(({ conditions }) => + conditions.map(({ expression }) => expression), + ), + ).toEqual([["!(!ready)"]]); +}); + +it("does not treat a break to an inner label as an exit from its outer block", () => { + const result = analyzeJavaScriptStaticSource(`const e=require("electron"); + outer: { + inner: { if (stop) break inner; } + e.ipcMain.on("after", event => event.sender.copy()); + } + `); + expect(result.electron.call_paths).toHaveLength(1); + expect(result.electron.call_paths?.[0]?.conditions).toEqual([]); +}); + it("preserves every direct observation in a long flat sibling block", () => { const channels = Array.from({ length: 256 }, (_, index) => `flat:${index}`); const result = analyzeJavaScriptStaticSource(`const e=require("electron"); { diff --git a/src/domain/javascript/electronWrappedCallPaths.ts b/src/domain/javascript/electronWrappedCallPaths.ts index be503ebd73..faba9aa8f5 100644 --- a/src/domain/javascript/electronWrappedCallPaths.ts +++ b/src/domain/javascript/electronWrappedCallPaths.ts @@ -256,6 +256,7 @@ export function* collectElectronWrappedCallPathsSteps( let remainingVisits = 1_000_000; const prefixes = new ElectronCallPathBlockPrefixes( () => --remainingVisits >= 0, + state.parentsByNode, ); const resolver = new JavaScriptSymbolicBindings(state, program, identity); const paths: ElectronCallPath[] = []; @@ -1172,7 +1173,7 @@ export function* collectElectronWrappedCallPathsSteps( for (const statement of node.body) { walk(statement, env, next); const fallthrough = sequentialFallthrough( - prefixes.statement(statement), + prefixes.statement(statement, node) ?? null, env, next, ); diff --git a/src/domain/javascript/javascriptSymbolicControlFlow.ts b/src/domain/javascript/javascriptSymbolicControlFlow.ts index 234d6f4d14..eacd9082f5 100644 --- a/src/domain/javascript/javascriptSymbolicControlFlow.ts +++ b/src/domain/javascript/javascriptSymbolicControlFlow.ts @@ -52,19 +52,3 @@ export const symbolicLogicalRight = ( const truth = symbolicTruth(left); return logicalRightOperand(operator, truth, undefined); }; - -/** Only definite synchronous exits; unsupported control flow stays unknown. */ -export const symbolicDefiniteExit = ( - node: t.Node | null | undefined, - depth = 0, -): boolean => { - if (!node || depth >= 128) return false; - if (t.isReturnStatement(node) || t.isThrowStatement(node)) return true; - if (t.isBlockStatement(node)) - return node.body.some((child) => symbolicDefiniteExit(child, depth + 1)); - return ( - t.isIfStatement(node) && - symbolicDefiniteExit(node.consequent, depth + 1) && - symbolicDefiniteExit(node.alternate, depth + 1) - ); -}; diff --git a/tests/boundary/cli/electronWrappedCallPaths.test.ts b/tests/boundary/cli/electronWrappedCallPaths.test.ts index 58e41d0f11..7e87395ec1 100644 --- a/tests/boundary/cli/electronWrappedCallPaths.test.ts +++ b/tests/boundary/cli/electronWrappedCallPaths.test.ts @@ -242,41 +242,107 @@ cliTest( }, ); +const fallthroughCases = [ + { + name: "conditional early return", + body: 'if(x)return;e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});', + conditions: ["!(x)"], + coverage: "partial", + }, + { + name: "equivalent nested branch", + body: 'if(!x){e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});}', + conditions: ["!x"], + coverage: "partial", + }, + { + name: "conditional break to enclosing label", + body: 'guard:{if(!x)break guard;e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});}', + conditions: ["!(!x)"], + coverage: "partial", + }, + { + name: "nested label breaking enclosing label", + body: 'guard:{if(!x)inner:{break guard;}e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});}', + conditions: ["!(!x)"], + coverage: "partial", + }, + { + name: "nested label breaking itself", + body: 'guard:{if(!x)inner:{break inner;}e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});}', + conditions: [], + coverage: "complete", + }, + { + name: "inner break skips unreachable throw", + body: 'guard:{if(x)inner:{break inner;throw 0;}e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});}', + conditions: [], + coverage: "complete", + }, + { + name: "loop may break inner label before throw", + body: 'guard:{if(x)inner:{while(true){break inner;}throw 0;}e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});}', + conditions: [], + coverage: "complete", + }, + { + name: "try may break inner label before throw", + body: 'guard:{if(x)inner:{try{break inner;}finally{}throw 0;}e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});}', + conditions: [], + coverage: "complete", + }, + { + name: "ordinary loop before return retains guard", + body: 'if(x){while(false){}return;}e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});', + conditions: ["!(x)"], + coverage: "partial", + }, + { + name: "true guard and ordinary loop before return", + body: 'if(true){while(false){}return;}e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});', + conditions: [], + coverage: "complete", + }, + { + name: "ordinary loop before throw retains guard", + body: 'if(x){for(const item of [])cleanup(item);throw 0;}e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});', + conditions: ["!(x)"], + coverage: "partial", + }, + { + name: "loop local label cannot skip return", + body: 'if(x){while(false){local:{break local;}}return;}e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});', + conditions: ["!(x)"], + coverage: "partial", + }, + { + name: "break to inner label does not guard outer continuation", + body: 'outer:{inner:{if(x)break inner;}e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});}', + conditions: [], + coverage: "complete", + }, + { + name: "definite early return", + body: 'return;e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});', + conditions: [], + coverage: "complete", + }, + { + name: "unguarded exposure", + body: 'e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});', + conditions: [], + coverage: "complete", + }, +] as const; + cliTest( "preserves early-return predicates on recovered bridge paths through CLI and stdio MCP", async ({ cli }) => { - const cases = [ - { - name: "conditional early return", - body: 'if(x)return;e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});', - conditions: ["!(x)"], - coverage: "partial", - }, - { - name: "equivalent nested branch", - body: 'if(!x){e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});}', - conditions: ["!x"], - coverage: "partial", - }, - { - name: "definite early return", - body: 'return;e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});', - conditions: [], - coverage: "complete", - }, - { - name: "unguarded exposure", - body: 'e.contextBridge.exposeInMainWorld("api",{copy:()=>e.ipcRenderer.send("x")});', - conditions: [], - coverage: "complete", - }, - ] as const; - for (const surface of ["CLI", "stdio MCP"] as const) { const connection = surface === "stdio MCP" ? await connectLocalToolsMcp() : undefined; const artifacts = new Set(); - for (const fixture of cases) { + for (const fixture of fallthroughCases) { const root = await createTestTempDirectory( "rea-electron-early-return-", ); @@ -300,7 +366,10 @@ cliTest( observation.properties.entry === "api.copy" && observation.properties.step_kind === "terminal", ); - if (fixture.name === "definite early return") { + if ( + fixture.name === "definite early return" || + fixture.name === "true guard and ordinary loop before return" + ) { expect(terminal).toBeUndefined(); continue; } @@ -311,6 +380,22 @@ cliTest( ({ expression }) => expression, ), ).toEqual(fixture.conditions); + if (fixture.name === "conditional break to enclosing label") { + const edges = application.graph.edges.filter( + (edge) => + edge.evidence.extractor.operation === + "trace-electron-wrapper-path", + ); + expect(edges.length).toBeGreaterThan(0); + for (const edge of edges) { + const conditions = z + .array(callPathConditionSchema) + .parse(edge.properties.conditions); + expect(conditions.map(({ expression }) => expression)).toEqual( + fixture.conditions, + ); + } + } expect(terminalEvidence.evidence.coverage.status).toBe( fixture.coverage, );