diff --git a/docs/electron-observation.md b/docs/electron-observation.md index 272c4e6511..2b8309f0db 100644 --- a/docs/electron-observation.md +++ b/docs/electron-observation.md @@ -29,15 +29,20 @@ files, including pages a packaged application serves from inside `app.asar`. For those, the first `.asar` path component that is a regular file is the archive, the remainder must name a regular file in its header, and the reported path is the canonical archive path joined with that member. Remote hosts, -encoded path separators, ASAR directories or links, and nonexistent paths are +encoded separators in the pathname, ASAR directories or links, and nonexistent paths are rejected. The endpoint exposes every eligible target and its local path -metadata. +metadata. Query strings and fragments do not participate in filesystem path +admission, including their encoded separator text. Listing and inspection +Evidence both retain the observed browser product/revision; host platform and +architecture remain explicitly unknown. Cancellation stops further file authorization after an already-started filesystem operation settles. ASAR descriptor-close failures remain typed incomplete cleanup with an exact retryable owner; they are not treated as a missing or unauthorized page. A later authorization and provider shutdown retry retained descriptors -before reporting their release. +before reporting their release. Failed listing retains its already-authorized +target prefix and explicit remaining-target uncertainty through cleanup errors +and exact-owner retries. ## Workflow diff --git a/src/application/javascript/ElectronEvidence.ts b/src/application/javascript/ElectronEvidence.ts index 911ef16a99..22237bde36 100644 --- a/src/application/javascript/ElectronEvidence.ts +++ b/src/application/javascript/ElectronEvidence.ts @@ -30,20 +30,15 @@ export const createElectronEvidence = ( result, confidence: "observed", authority: "external-service", - environment: - "target" in result - ? { - id: `${result.browser.product}@${result.browser.revision}`, - platform: "unknown", - architecture: "unknown", - isolation: "none", - } - : null, + environment: { + id: `${result.browser.product}@${result.browser.revision}`, + platform: "unknown", + architecture: "unknown", + isolation: "none", + }, limitations: [ ...result.limitations, - ...("target" in result - ? ["Browser host platform and architecture were not observed."] - : []), + "Browser host platform and architecture were not observed.", ], }); diff --git a/src/browser/ElectronFileScope.cleanup.test.ts b/src/browser/ElectronFileScope.cleanup.test.ts index bb2a7b8016..f4cd21c36d 100644 --- a/src/browser/ElectronFileScope.cleanup.test.ts +++ b/src/browser/ElectronFileScope.cleanup.test.ts @@ -1,4 +1,6 @@ -import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { createServer } from "node:http"; +import { projectAnalysisError } from "../domain/analysisErrorProjection.js"; +import { mkdtemp, realpath, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; @@ -97,7 +99,9 @@ afterEach(async () => { }); const asarPage = async (): Promise => { - const root = await mkdtemp(join(tmpdir(), "rea-electron-file-scope-")); + const root = await realpath( + await mkdtemp(join(tmpdir(), "rea-electron-file-scope-")), + ); roots.push(root); const archive = join(root, "app.asar"); const json = Buffer.from( @@ -295,3 +299,89 @@ it("retries a failed ASAR descriptor lease during provider shutdown", async () = expect(handleState.events).toEqual(["open", "close", "close"]); }); + +it.each(["completed", "read-failed", "cancelled"] as const)( + "retains the completed listing prefix after ASAR finalization: %s", + async (mode) => { + const archiveUrl = await asarPage(); + const page = join(roots.at(-1)!, "first.html"); + await writeFile(page, "first"); + const controller = new AbortController(); + handleState.failCloseCount = 1; + if (mode === "read-failed") + handleState.readFailure = new Error("controlled ASAR read denial"); + if (mode === "cancelled") { + handleState.closeController = controller; + handleState.closeReason = { request: "ASAR descriptor close" }; + } + let endpoint = ""; + const server = createServer((request, response) => { + response.setHeader("Content-Type", "application/json"); + response.end( + JSON.stringify( + request.url === "/json/version" + ? { + Browser: "Electron/fixture", + "Protocol-Version": "1.3", + "WebKit-Version": "revision", + "User-Agent": "fixture", + "V8-Version": "V8", + webSocketDebuggerUrl: + endpoint.replace("http:", "ws:") + + "/devtools/browser/fixture", + } + : [pathToFileURL(page).href, archiveUrl].map((url, index) => ({ + id: `page-${index}`, + type: "page", + title: `page ${index}`, + url, + attached: false, + webSocketDebuggerUrl: + endpoint.replace("http:", "ws:") + + `/devtools/page/page-${index}`, + })), + ), + ); + }); + await new Promise((resolve) => + server.listen(0, "127.0.0.1", resolve), + ); + const address = server.address(); + if (address === null || typeof address === "string") + throw new Error("Expected discovery port"); + endpoint = `http://127.0.0.1:${address.port}`; + const provider = new CdpElectronProvider(); + try { + const result = await provider.listTargets( + { cdp_endpoint: endpoint }, + { signal: controller.signal }, + ); + if (result.ok) throw new Error("Expected ASAR finalization failure"); + const origin = result.error.cleanupPrimary ?? result.error; + expect(origin.diagnostics).toMatchObject({ + completed_prefix: { + examined_targets: 2, + allowed_targets: [{ target_id: "page-0", file_path: page }], + coverage: expect.stringContaining("remaining targets are unknown"), + }, + }); + expect(JSON.stringify(projectAnalysisError(result.error))).toContain( + "completed_prefix", + ); + expect(result.error.cleanupIncomplete).toBe(true); + expect(handleState.events).toEqual(["open", "close"]); + await provider.close(); + expect(result.error.cleanupIncomplete).toBe(false); + expect(handleState.events).toEqual(["open", "close", "close"]); + expect(origin.diagnostics).toHaveProperty("completed_prefix"); + } finally { + handleState.failCloseCount = 0; + await provider.close(); + await new Promise((resolve, reject) => + server.close((cause) => + cause === undefined ? resolve() : reject(cause), + ), + ); + } + }, +); diff --git a/src/browser/ElectronFileScope.ts b/src/browser/ElectronFileScope.ts index fb8a6be882..c9bffaaf37 100644 --- a/src/browser/ElectronFileScope.ts +++ b/src/browser/ElectronFileScope.ts @@ -92,7 +92,6 @@ export const mapElectronFileAuthorizationFailure = ( const primary = hasCancellation ? new AnalysisCancelledError(operation, { cause: cancellationReason, - ...(diagnostics === undefined ? {} : { diagnostics }), }) : physicalPrimary instanceof AnalysisError ? physicalPrimary @@ -108,8 +107,14 @@ export const mapElectronFileAuthorizationFailure = ( { operation, cause }, ); bindArtifactFileCleanup(cause, cleanup); + if (diagnostics !== undefined) + (primary ?? cleanup).retainDiagnostics(diagnostics); if (primary === undefined) return cleanup; - const composed = analysisErrorWithCleanupFailure(primary, cleanup, operation); + const composed = analysisErrorWithCleanupFailure( + primary, + cleanup, + operation, + ).bindCleanupDependencies([primary, cleanup]); bindArtifactFileCleanup(cause, composed); return composed; }; @@ -120,7 +125,6 @@ const resolveAuthorizedElectronFile = async ( asarHeaderAdmission: AsarHeaderAdmission, ): Promise => { throwIfAborted(signal); - if (/%(?:2f|5c)/iu.test(value)) return undefined; let url: URL; try { url = new URL(value); @@ -133,7 +137,8 @@ const resolveAuthorizedElectronFile = async ( url.protocol !== "file:" || url.hostname !== "" || url.username !== "" || - url.password !== "" + url.password !== "" || + /%(?:2f|5c)/iu.test(url.pathname) ) return undefined; let path: string; diff --git a/tests/boundary/cli/electronCli.test.ts b/tests/boundary/cli/electronCli.test.ts index 2733ee6a06..7020b22ab0 100644 --- a/tests/boundary/cli/electronCli.test.ts +++ b/tests/boundary/cli/electronCli.test.ts @@ -43,7 +43,7 @@ it( await writeFile(join(root, "index.html"), ""); await writeFile(join(root, "app.js"), "export const app = true;"); const browser = await startFakeCdpBrowser({ - electronFileUrl: pathToFileURL(join(root, "index.html")).href, + electronFileUrl: `${pathToFileURL(join(root, "index.html")).href}?next=%2Fsettings#%5Csettings`, commandEvents: ({ method }) => method === "Debugger.enable" ? [ @@ -67,6 +67,15 @@ it( ); expect(listed).toMatchObject({ operation: "list_electron_targets", + environment: { + id: expect.any(String), + platform: "unknown", + architecture: "unknown", + isolation: "none", + }, + limitations: expect.arrayContaining([ + "Browser host platform and architecture were not observed.", + ]), normalized_result: { targets: [{ target_id: "electron-page" }], }, @@ -84,6 +93,7 @@ it( ); expect(inspected).toMatchObject({ operation: "inspect_electron_page", + environment: listed.environment, provider: { id: "rea-cdp-electron" }, normalized_result: { target: { file_path: join(root, "index.html") }, diff --git a/tests/boundary/filesystem/electronFileScope.test.ts b/tests/boundary/filesystem/electronFileScope.test.ts index b2ad1f1003..24f4ecc87e 100644 --- a/tests/boundary/filesystem/electronFileScope.test.ts +++ b/tests/boundary/filesystem/electronFileScope.test.ts @@ -156,3 +156,26 @@ describe("Electron file scope", () => { ).toBeUndefined(); }); }); + +it("keeps encoded query and fragment separators outside filesystem admission", async () => { + const root = await createTestTempDirectory("rea-electron-url-components-"); + const path = join(root, "index.html"); + await writeFile(path, "allowed"); + const base = pathToFileURL(path).href; + for (const suffix of [ + "", + "?next=/settings", + "#/settings", + "?next=%2Fsettings", + "?next=%5Csettings", + "#%2Fsettings", + "#%5Csettings", + ]) + await expect(authorizedElectronFile(`${base}${suffix}`)).resolves.toBe( + path, + ); + for (const encoded of ["%2F", "%5C"]) + await expect( + authorizedElectronFile(`${pathToFileURL(root).href}${encoded}index.html`), + ).resolves.toBeUndefined(); +}); diff --git a/tests/boundary/mcp/electronMcp.test.ts b/tests/boundary/mcp/electronMcp.test.ts index 1d6a8fb318..2814c0cf06 100644 --- a/tests/boundary/mcp/electronMcp.test.ts +++ b/tests/boundary/mcp/electronMcp.test.ts @@ -286,7 +286,7 @@ it("exposes endpoint-scoped Electron discovery and inspection as Evidence", asyn ); await writeFile(join(root, "worker.js"), "self.onmessage = () => {};\n"); const browser = await startFakeCdpBrowser({ - electronFileUrl: pathToFileURL(join(root, "index.html")).href, + electronFileUrl: `${pathToFileURL(join(root, "index.html")).href}?next=%2Fsettings#%5Csettings`, commandEvents: ({ method }) => method === "Debugger.enable" ? [ @@ -332,6 +332,15 @@ it("exposes endpoint-scoped Electron discovery and inspection as Evidence", asyn }); expect(listed.isError).not.toBe(true); expect(listed.structuredContent).toMatchObject({ + environment: { + id: expect.any(String), + platform: "unknown", + architecture: "unknown", + isolation: "none", + }, + limitations: expect.arrayContaining([ + "Browser host platform and architecture were not observed.", + ]), normalized_result: { targets: [{ target_id: "electron-page" }], }, @@ -345,6 +354,10 @@ it("exposes endpoint-scoped Electron discovery and inspection as Evidence", asyn include_script_sources: true, }, }); + expect(inspected.isError).not.toBe(true); + expect(parseEvidence(inspected.structuredContent).environment).toEqual( + parseEvidence(listed.structuredContent).environment, + ); const analyzed = await client.callTool({ name: "analyze_javascript_application", arguments: { input_path: root },