diff --git a/native/windows/README.md b/native/windows/README.md index 73884ae057..b1ba7e7b24 100644 --- a/native/windows/README.md +++ b/native/windows/README.md @@ -118,6 +118,9 @@ automatically. Alternatively, build it using the Linux/MinGW lane above. a 256 MiB Job Object commit limit and a 64 MiB JavaScript heap. It retains any failed cleanup owner and retries it before admitting another capture. It does not retain a detached native thread or target-owned surface in the server. + Requests use bounded UTF-8 JSON on owned protocol stdin, avoiding Windows + command-line expansion of long paths. The helper receives the provider's + selected environment with ambient Node preload options removed. A thread-local DPI context gives physical client dimensions without changing the parent DPI policy. The implementation never enumerates windows, captures a desktop/region, searches titles, sends input, changes permissions, launches diff --git a/native/windows/src/window_capture.cc b/native/windows/src/window_capture.cc index 60c3ac794a..d9d7ce0fa2 100644 --- a/native/windows/src/window_capture.cc +++ b/native/windows/src/window_capture.cc @@ -604,6 +604,12 @@ WindowImage captureWindow(DWORD pid, ULONG_PTR handle, int maximumDimension, surface->makeOpaque(); validateWindowOwner(window, pid); auto observedProcess = validateProcessIdentity(pid, startToken, imagePath); + RECT observedBounds{}; + require(GetClientRect(window, &observedBounds) != FALSE, + "Read selected window client bounds after rendering failed"); + require(EqualRect(&bounds, &observedBounds) != FALSE, + "Selected window client area changed during capture; retry after resizing stops", L"", + ERROR_INVALID_DATA, FailureKind::WindowState); const int longest = std::max(width, height); const double scale = longest <= maximumDimension ? 1.0 : static_cast(maximumDimension) / longest; const int renderedWidth = std::max(1, static_cast(std::floor(width * scale))); diff --git a/src/composition/auxiliaryAnalysisProviders.ts b/src/composition/auxiliaryAnalysisProviders.ts index 6a57c310ea..a6072b81c2 100644 --- a/src/composition/auxiliaryAnalysisProviders.ts +++ b/src/composition/auxiliaryAnalysisProviders.ts @@ -50,7 +50,7 @@ export const auxiliaryAnalysisProviderDeclarations = ( load: async () => { const { NativeWindowsUiProvider } = await import("../windows/NativeWindowsUiProvider.js"); - return new NativeWindowsUiProvider(undefined, platform); + return new NativeWindowsUiProvider(environment, undefined, platform); }, }, { diff --git a/src/contracts/toolEffects.test.ts b/src/contracts/toolEffects.test.ts index fbb92e2ab4..d03e31a55b 100644 --- a/src/contracts/toolEffects.test.ts +++ b/src/contracts/toolEffects.test.ts @@ -137,6 +137,7 @@ describe("tool effects", () => { "capture_electron_scenario", "capture_native_ui_scenario", "capture_process_scenario", + "capture_windows_window_screenshot", "close_gdb_session", "close_rizin_debug_session", "cutter_command", diff --git a/src/process/OwnedCommand.ts b/src/process/OwnedCommand.ts index 3a77c73425..0548a52f39 100644 --- a/src/process/OwnedCommand.ts +++ b/src/process/OwnedCommand.ts @@ -63,6 +63,8 @@ export const runOwnedCommand = async ( }, options: { readonly signal?: AbortSignal | undefined; + /** Complete protocol input, written and closed under the same deadline. */ + readonly input?: Uint8Array; /** Synchronously accept unresolved cleanup ownership; execution still returns its original outcome. Must not throw. */ readonly retainCleanup?: ( owner: ProviderProcessSupervisor, @@ -88,6 +90,7 @@ export const runOwnedCommand = async ( deadline, callerSignal, options.retainCleanup, + options.input, ); } finally { deadline.dispose(); @@ -110,12 +113,14 @@ const collectOwnedCommand = async ( owner: ProviderProcessSupervisor, result: Extract, ) => void, + input?: Uint8Array, ): Promise => { const signal = deadline.signal; let launched: SpawnedOwnedProviderProcess; try { launched = await launcher({ ...spawn, + ...(input === undefined ? {} : { stdin: "pipe" as const }), signal, }); } catch (cause: unknown) { @@ -152,8 +157,25 @@ const collectOwnedCommand = async ( }, ); let failure: OwnedCommandFailure | undefined; + let inputPending = input !== undefined; try { - while (!(await supervisor.waitForOutputClose(10))) { + if (input !== undefined) { + const stream = launched.process.stdin; + if (stream == null) + throw new Error("Owned command protocol input is unavailable."); + const inputError = (cause: Error) => { + inputPending = false; + processFailure ??= `Command protocol input failed: ${cause.message}`; + }; + // Retain this listener through cleanup: stopping a blocked writer can + // emit a late error, including when its supervisor must be retained. + stream.on("error", inputError); + stream.end(input, (cause?: Error | null) => { + inputPending = false; + if (cause != null) inputError(cause); + }); + } + while (!(await supervisor.waitForOutputClose(10)) || inputPending) { const interruption = deadline.interruption; if (interruption !== undefined) throw new OwnedCommandFailure( diff --git a/src/windows/NativeWindowsUiProvider.test.ts b/src/windows/NativeWindowsUiProvider.test.ts index 2e6b090336..70f95552a2 100644 --- a/src/windows/NativeWindowsUiProvider.test.ts +++ b/src/windows/NativeWindowsUiProvider.test.ts @@ -236,9 +236,11 @@ it("retains and retries an executable descriptor whose close first fails", async "Selected executable descriptor cleanup failed", ); close.mockRestore(); - const client = new NativeWindowsUiProvider(host(), "linux").createClient( - TARGET, - ); + const client = new NativeWindowsUiProvider( + {}, + host(), + "linux", + ).createClient(TARGET); await expect(client.close()).resolves.toMatchObject({ ok: true }); await expect( hashWindowsCaptureExecutable(executable), @@ -309,7 +311,7 @@ it("retains both final identity owners before returning their primary failure", cleanup: { resources: [TARGET.path, processPath] }, }, }); - const client = new NativeWindowsUiProvider(host(), "linux").createClient( + const client = new NativeWindowsUiProvider({}, host(), "linux").createClient( TARGET, ); await expect(client.close()).resolves.toMatchObject({ ok: true }); diff --git a/src/windows/NativeWindowsUiProvider.ts b/src/windows/NativeWindowsUiProvider.ts index 3d02bfed29..630bd69ba0 100644 --- a/src/windows/NativeWindowsUiProvider.ts +++ b/src/windows/NativeWindowsUiProvider.ts @@ -139,7 +139,9 @@ export interface WindowsWindowCaptureHost { ): Promise; } -const systemCaptureHost = (): WindowsWindowCaptureHost => ({ +const systemCaptureHost = ( + environment: Readonly, +): WindowsWindowCaptureHost => ({ capture: async ( pid, windowHandle, @@ -166,6 +168,7 @@ const systemCaptureHost = (): WindowsWindowCaptureHost => ({ target_sha256: targetIdentity.sha256, target_file_identity: targetIdentity.fileIdentity, }, + environment, signal, ); }, @@ -177,7 +180,10 @@ export class NativeWindowsUiProvider implements AnalysisProvider { readonly #capabilities: readonly CapabilityDescriptor[]; constructor( - private readonly captureHost: WindowsWindowCaptureHost = systemCaptureHost(), + environment: Readonly = process.env, + private readonly captureHost: WindowsWindowCaptureHost = systemCaptureHost( + environment, + ), platform: NodeJS.Platform = process.platform, ) { this.#capabilities = nativeWindowsUiCapabilities(platform); @@ -246,7 +252,7 @@ export class NativeWindowsUiProvider implements AnalysisProvider { export const captureWindowsWindowScreenshot = async ( target: BinaryTarget, parameters: Readonly>, - host: WindowsWindowCaptureHost = systemCaptureHost(), + host: WindowsWindowCaptureHost = systemCaptureHost(process.env), options: ExecutionOptions = {}, ): Promise> => { if (options.signal?.aborted) diff --git a/src/windows/WindowsNativeLoader.test.ts b/src/windows/WindowsNativeLoader.test.ts index 1158d2e150..3753d9a8c0 100644 --- a/src/windows/WindowsNativeLoader.test.ts +++ b/src/windows/WindowsNativeLoader.test.ts @@ -100,7 +100,7 @@ describe("selected-window native admission", () => { }); it("refuses an addon that does not expose selected-window inspection", () => { - const value = fixture({ jobMemoryLimit: true }); + const value = fixture({ jobMemoryLimit: true, protocolStdin: true }); expect(loadWindowsWindowCaptureAuthority(value.host)).toMatchObject({ available: false, failureStage: "inspection", @@ -109,12 +109,24 @@ describe("selected-window native admission", () => { it("requires capture support and the helper's enforced memory control", () => { expect( loadWindowsWindowCaptureAuthority( - fixture({ windowCapture: true, jobMemoryLimit: true }).host, + fixture({ + windowCapture: true, + jobMemoryLimit: true, + protocolStdin: true, + }).host, ), ).toMatchObject({ available: true }); expect( loadWindowsWindowCaptureAuthority(fixture({ windowCapture: true }).host), ).toMatchObject({ available: false, failureStage: "inspection" }); + expect( + loadWindowsWindowCaptureAuthority( + fixture({ windowCapture: true, jobMemoryLimit: true }).host, + ), + ).toMatchObject({ + available: false, + reason: expect.stringContaining("protocol stdin"), + }); }); }); diff --git a/src/windows/WindowsNativeLoader.ts b/src/windows/WindowsNativeLoader.ts index 9f4830d51e..6aeb3d319f 100644 --- a/src/windows/WindowsNativeLoader.ts +++ b/src/windows/WindowsNativeLoader.ts @@ -247,6 +247,13 @@ export const loadWindowsWindowCaptureAuthority = ( reason: "Selected-window capture requires owned helper Job Object memory limits; install the matching native artifact.", }; + if (binding.inspection.protocolStdin !== true) + return { + available: false, + failureStage: "inspection", + reason: + "Selected-window capture requires owned helper protocol stdin; install the matching native artifact.", + }; try { let observed: unknown; try { diff --git a/src/windows/WindowsWindowCaptureExecution.test.ts b/src/windows/WindowsWindowCaptureExecution.test.ts new file mode 100644 index 0000000000..5d2b275e37 --- /dev/null +++ b/src/windows/WindowsWindowCaptureExecution.test.ts @@ -0,0 +1,98 @@ +import { createHash } from "node:crypto"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, expect, it, vi } from "vitest"; + +import * as command from "../process/OwnedCommand.js"; +import { executeWindowsCapture } from "./WindowsWindowCaptureExecution.js"; +import { WINDOWS_CAPTURE_REQUEST_BYTES } from "./WindowsWindowCaptureProtocol.js"; +import * as native from "./WindowsNativeLoader.js"; +import { NativeWindowsUiProvider } from "./NativeWindowsUiProvider.js"; +import { pe } from "../domain/binaryTarget.fixture.js"; + +afterEach(() => vi.restoreAllMocks()); + +it("sends long selected paths through stdin and preserves only the selected environment", async () => { + const run = vi + .spyOn(command, "runOwnedCommand") + .mockRejectedValue( + new command.OwnedCommandFailure( + "cancelled", + "Fixture stopped after launch inspection", + ), + ); + const request = { + pid: 42, + window_handle: "0x1234", + max_dimension: 320, + target_path: "C:" + "\\a".repeat(16000), + target_sha256: "a".repeat(64), + target_file_identity: "1:42", + }; + await expect( + executeWindowsCapture(request, { + TEMP: "selected-temp", + REA_CAPTURE_SELECTED: "selected", + NODE_OPTIONS: "--require=never-run", + }), + ).rejects.toMatchObject({ _tag: "AnalysisCancelledError" }); + expect(run).toHaveBeenCalledTimes(1); + const [spawn, , options] = run.mock.calls[0]!; + expect(spawn.arguments).toHaveLength(2); + expect(spawn.hostEnvironment).toEqual({ + TEMP: "selected-temp", + REA_CAPTURE_SELECTED: "selected", + }); + const input = Buffer.from(options?.input ?? []); + expect(input.byteLength).toBeGreaterThan(32767); + expect(input.byteLength).toBeLessThanOrEqual(WINDOWS_CAPTURE_REQUEST_BYTES); + expect(JSON.parse(input.toString("utf8"))).toEqual(request); +}); + +it("carries the provider's selected environment through executable admission to helper launch", async () => { + const directory = await mkdtemp(join(tmpdir(), "rea-capture-environment-")); + const path = join(directory, "fixture.exe"); + const bytes = pe(0x8664); + await writeFile(path, bytes); + vi.spyOn(native, "requireWindowsWindowCaptureAuthority").mockReturnValue({ + call: () => { + throw new Error("Admission must not invoke native paint directly"); + }, + }); + const run = vi + .spyOn(command, "runOwnedCommand") + .mockRejectedValue( + new command.OwnedCommandFailure( + "cancelled", + "Fixture stopped before native launch", + ), + ); + const client = new NativeWindowsUiProvider({ + TEMP: "provider-selected", + }).createClient({ + kind: "executable", + path, + sha256: createHash("sha256").update(bytes).digest("hex"), + format: "pe", + architecture: "x86_64", + availableArchitectures: ["x86_64"], + executableRole: "application", + managed: false, + }); + try { + expect( + await client.execute("capture_windows_window_screenshot", { + pid: 42, + window_handle: "0x1234", + }), + ).toMatchObject({ ok: false, error: { _tag: "AnalysisCancelledError" } }); + expect(run).toHaveBeenCalledTimes(1); + expect(run.mock.calls[0]![0].hostEnvironment).toEqual({ + TEMP: "provider-selected", + }); + } finally { + await client.close(); + await rm(directory, { recursive: true, force: true }); + } +}); diff --git a/src/windows/WindowsWindowCaptureExecution.ts b/src/windows/WindowsWindowCaptureExecution.ts index a3301a5c6c..e2002d5990 100644 --- a/src/windows/WindowsWindowCaptureExecution.ts +++ b/src/windows/WindowsWindowCaptureExecution.ts @@ -18,7 +18,7 @@ import { WINDOWS_CAPTURE_REPLY_BYTES, WINDOWS_CAPTURE_TIMEOUT_MS, windowsCaptureReplySchema, - type windowsCaptureRequestSchema, + windowsCaptureRequestSchema, } from "./WindowsWindowCaptureProtocol.js"; import type { z } from "zod"; @@ -50,6 +50,7 @@ export const retryWindowsCaptureCleanup = async (): Promise< /** Isolate the blocking OS API in one bounded, supervised helper at a time. */ export const executeWindowsCapture = async ( request: z.output, + selectedEnvironment: Readonly, signal?: AbortSignal, ): Promise => { if (active) @@ -67,7 +68,7 @@ export const executeWindowsCapture = async ( // Preserve inherited values without allowing ambient Node options to run // preload scripts or replace this helper's explicit heap limit. const environment = Object.fromEntries( - Object.entries(process.env).filter( + Object.entries(selectedEnvironment).filter( ([name]) => name.toUpperCase() !== "NODE_OPTIONS", ), ); @@ -79,7 +80,6 @@ export const executeWindowsCapture = async ( fileURLToPath( new URL("../windows-capture-runtime/worker.js", import.meta.url), ), - JSON.stringify(request), ], runId: `rea-window-capture-${randomUUID()}`, expectedCommand: null, @@ -90,7 +90,13 @@ export const executeWindowsCapture = async ( timeoutMs: WINDOWS_CAPTURE_TIMEOUT_MS, diagnosticBytes: WINDOWS_CAPTURE_REPLY_BYTES, }, - { signal }, + { + signal, + input: Buffer.from( + JSON.stringify(windowsCaptureRequestSchema.parse(request)), + "utf8", + ), + }, ); const value: unknown = JSON.parse(snapshot.stdout.text); const reply = windowsCaptureReplySchema.parse(value); diff --git a/src/windows/WindowsWindowCaptureProtocol.ts b/src/windows/WindowsWindowCaptureProtocol.ts index b2666d77fb..3fc004788e 100644 --- a/src/windows/WindowsWindowCaptureProtocol.ts +++ b/src/windows/WindowsWindowCaptureProtocol.ts @@ -7,6 +7,7 @@ import { export const WINDOWS_CAPTURE_TIMEOUT_MS = 5000; const MAX_WINDOWS_TEXT_UNITS = 32767; +export const WINDOWS_CAPTURE_REQUEST_BYTES = MAX_WINDOWS_TEXT_UNITS * 6 + 4096; // Title and image path can each expand to six ASCII bytes per UTF-16 unit // under JSON escaping. Reserve the remaining fixed fields independently. export const WINDOWS_CAPTURE_REPLY_BYTES = @@ -17,9 +18,9 @@ export const WINDOWS_CAPTURE_REPLY_BYTES = /** Private process protocol; identity is validated again by the native owner. */ export const windowsCaptureRequestSchema = z.strictObject({ ...windowsWindowScreenshotInputSchema.shape, - target_path: z.string().min(1), + target_path: z.string().min(1).max(MAX_WINDOWS_TEXT_UNITS), target_sha256: z.string().regex(/^[a-f0-9]{64}$/u), - target_file_identity: z.string().min(1), + target_file_identity: z.string().min(1).max(32), }); const identityFields = { diff --git a/src/windows/WindowsWindowCaptureWorker.ts b/src/windows/WindowsWindowCaptureWorker.ts index e9a7520f52..2f3c8fef71 100644 --- a/src/windows/WindowsWindowCaptureWorker.ts +++ b/src/windows/WindowsWindowCaptureWorker.ts @@ -1,6 +1,7 @@ import { requireWindowsWindowCaptureAuthority } from "./WindowsNativeLoader.js"; import { nativeWindowsCaptureSchema, + WINDOWS_CAPTURE_REQUEST_BYTES, windowsCaptureFailureSchema, windowsCaptureRequestSchema, } from "./WindowsWindowCaptureProtocol.js"; @@ -9,8 +10,19 @@ import { // calls it; the parent can terminate its Job Object without blocking the server // or leaving a detached native thread/surface in the long-lived REA process. try { - const encoded = process.argv[2]; - if (encoded === undefined) throw new Error("Missing capture request"); + const chunks: Buffer[] = []; + let bytes = 0; + for await (const chunk of process.stdin) { + if (!Buffer.isBuffer(chunk)) + throw new Error("Invalid capture request bytes"); + bytes += chunk.byteLength; + if (bytes > WINDOWS_CAPTURE_REQUEST_BYTES) + throw new Error("Capture request exceeds its protocol byte limit"); + chunks.push(chunk); + } + const encoded = new TextDecoder("utf-8", { fatal: true }).decode( + Buffer.concat(chunks, bytes), + ); const value: unknown = JSON.parse(encoded); const input = windowsCaptureRequestSchema.parse(value); const authority = requireWindowsWindowCaptureAuthority(); diff --git a/tests/boundary/process/ownedCommand.test.ts b/tests/boundary/process/ownedCommand.test.ts index 33c1d09552..249668728b 100644 --- a/tests/boundary/process/ownedCommand.test.ts +++ b/tests/boundary/process/ownedCommand.test.ts @@ -101,6 +101,41 @@ it("collects a real owned command and verifies its exit before returning diagnos }); }); +it("delivers complete protocol input larger than the Windows command-line limit", async () => { + const input = Buffer.from( + JSON.stringify({ path: "C:" + "\\a".repeat(16000) }), + ); + expect(input.byteLength).toBeGreaterThan(32767); + const result = await runOwnedCommand( + command( + "const chunks=[]; for await (const chunk of process.stdin) chunks.push(chunk); process.stdout.write(Buffer.concat(chunks));", + ), + { timeoutMs: 2000, diagnosticBytes: input.byteLength + 1024 }, + { input }, + ); + expect(result.stdout.text).toBe(input.toString("utf8")); +}); + +it("times out and cleans up a command whose protocol reader never consumes input", async () => { + await expect( + runOwnedCommand( + command("setInterval(() => {}, 1000)"), + { timeoutMs: 500, diagnosticBytes: 1024 }, + { input: Buffer.alloc(2 * 1024 * 1024) }, + ), + ).rejects.toMatchObject({ reason: "timeout", cleanupFailure: null }); +}); + +it("reports a protocol write failure when the child closes its input early", async () => { + await expect( + runOwnedCommand( + command("process.stdin.destroy()"), + { timeoutMs: 2000, diagnosticBytes: 1024 }, + { input: Buffer.alloc(2 * 1024 * 1024) }, + ), + ).rejects.toMatchObject({ reason: "process", collectionFailure: true }); +}); + it("fails an accepted nonzero command when an output stream errors and cleans it up", async () => { const stdout = new PassThrough(); const stderr = new PassThrough();