From af8dcfb9c0b4c655725f6743f2796834b1aa4bbd Mon Sep 17 00:00:00 2001 From: kkdev92 <112151103+kkdev92@users.noreply.github.com> Date: Tue, 10 Nov 2026 17:01:40 +0900 Subject: [PATCH] perf: start Windows PowerShell once, directly, to read the clipboard On Windows a paste waited for PowerShell longer than it needed to, in three ways. PowerShell 7 was preferred when it was installed, although it takes longer to start than Windows PowerShell and the script needs nothing it adds. The command went through cmd.exe. And the first paste in a window started a PowerShell an extra time, only to see that one runs. The provider now starts powershell.exe itself, with an argument array, and lets the read be the check: the first executable that gets through the script is remembered for later reads. PowerShell 7 is tried when Windows PowerShell cannot be started or fails, so a machine that relied on it keeps working. isAvailable() answers from the platform alone. A read that outlives the timeout is reported as a timeout and does not go on to the other executable; the old check looked for an error code that a timeout does not set. macOS and Linux are untouched. Measured with the script as shipped, eight runs each, median. On an ARM64 laptop with PowerShell 7 installed the read went from 682 ms to 337 ms. On an x64 hosted CI runner it went from 465 ms to 305 ms with text on the clipboard and from 527 ms to 362 ms with an image, and the extra start that is gone took 422 ms. The new test file replaces child_process and fails on the code before this change, 11 tests of 11. One more test, on Windows only, starts the real powershell.exe with the provider's arguments. Co-Authored-By: Claude Fable 5.2 --- CHANGELOG.md | 11 + SECURITY.md | 4 +- src/clipboard/clipboard-manager.ts | 12 +- src/clipboard/providers/windows-provider.ts | 180 ++++++++----- .../windows-provider-powershell.test.ts | 254 ++++++++++++++++++ .../providers/windows-provider.test.ts | 30 ++- 6 files changed, 410 insertions(+), 81 deletions(-) create mode 100644 test/clipboard/providers/windows-provider-powershell.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1117712..88f426b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- On Windows, a paste starts PowerShell once, and starts it sooner. The + clipboard is read by Windows PowerShell, started directly. PowerShell 7 used + to be preferred when it was installed, although it takes longer to start; + the command went through `cmd.exe`; and the first paste in each window + started a PowerShell an extra time only to check that one runs. With + PowerShell 7 installed, the read took a third to a half less time on the two + machines it was measured on. PowerShell 7 is still used when Windows + PowerShell cannot be started or fails. macOS and Linux are unchanged. + ## [0.10.0] - 2026-10-17 **A paste never replaces an image that is already saved, and an image pasted diff --git a/SECURITY.md b/SECURITY.md index a9dd0fd..f96d127 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -33,9 +33,9 @@ All shell commands are executed using: - **Environment Variables** for parameter passing ```typescript -// Safe PowerShell execution +// Safe PowerShell execution: a constant script, started without a shell const encoded = encodePowerShellCommand(script); -const cmd = `powershell.exe -EncodedCommand ${encoded}`; +execFile('powershell.exe', ['-NoProfile', '-EncodedCommand', encoded]); ``` ### Path Traversal Prevention diff --git a/src/clipboard/clipboard-manager.ts b/src/clipboard/clipboard-manager.ts index cc26775..bc5d67e 100644 --- a/src/clipboard/clipboard-manager.ts +++ b/src/clipboard/clipboard-manager.ts @@ -47,13 +47,15 @@ export class ClipboardManager { /** * Check if the clipboard provider is available on the current system * - * Verifies that the platform-specific clipboard tool (PowerShell, osascript, - * xclip, or wl-paste) is installed and accessible. + * Verifies that the platform-specific clipboard tool (osascript, xclip, or + * wl-paste) is installed and accessible. Windows has nothing to look for — + * Windows PowerShell is part of it — so its provider answers without + * starting anything. * * A confirmed-available result is cached: the tools don't disappear - * mid-session, and re-probing costs a process spawn on every paste - * (a full PowerShell startup on Windows). Negative results are NOT - * cached so a missing tool can be installed without reloading. + * mid-session, and re-probing costs a process spawn on every paste. + * Negative results are NOT cached so a missing tool can be installed + * without reloading. * * @returns True if clipboard access is available, false otherwise */ diff --git a/src/clipboard/providers/windows-provider.ts b/src/clipboard/providers/windows-provider.ts index dc9525e..9d80cc1 100644 --- a/src/clipboard/providers/windows-provider.ts +++ b/src/clipboard/providers/windows-provider.ts @@ -3,7 +3,7 @@ * Uses PowerShell with Base64 encoding for secure execution */ -import { exec } from 'child_process'; +import { execFile } from 'child_process'; import { promisify } from 'util'; import type { ClipboardData } from '../../core/types'; import { ClipboardError } from '../../core/errors'; @@ -12,10 +12,17 @@ import { BaseClipboardProvider } from '../clipboard-provider'; import { encodePowerShellCommand } from '../../security/sanitizer'; import { getTempFileManager } from '../../security/temp-file-manager'; -const execAsync = promisify(exec); +const execFileAsync = promisify(execFile); -// Cache PowerShell path to avoid repeated detection -let cachedPowerShellPath: string | null = null; +/** + * PowerShell executables, in the order they are tried. + * + * Windows PowerShell goes first: it is part of Windows, the script needs + * nothing newer than the .NET clipboard API that both have, and it starts + * sooner than PowerShell 7 does. PowerShell 7 is there for a machine where + * Windows PowerShell cannot be started, or does not get through the script. + */ +const POWERSHELL_EXECUTABLES = ['powershell.exe', 'pwsh'] as const; /** * PowerShell script to check if clipboard contains an image and save it to a temp file. @@ -87,32 +94,50 @@ $image.Dispose() */ const EXECUTION_POLICY = 'RemoteSigned'; +/** + * The arguments PowerShell is started with. All of them are constant; the one + * value that changes per read, the temporary file, goes in the environment. + */ +const POWERSHELL_ARGUMENTS: readonly string[] = [ + '-NoLogo', + '-NoProfile', + '-NonInteractive', + '-ExecutionPolicy', + EXECUTION_POLICY, + '-EncodedCommand', + encodePowerShellCommand(CLIPBOARD_SCRIPT), +]; + +/** Node ends a child that outlives `timeout`, and reports it as killed. */ +function isTimeout(error: unknown): boolean { + return (error as { killed?: boolean }).killed === true; +} + +/** The executable is not on the PATH. */ +function isNotFound(error: unknown): boolean { + return (error as NodeJS.ErrnoException).code === 'ENOENT'; +} + /** * Windows clipboard provider using PowerShell */ export class WindowsClipboardProvider extends BaseClipboardProvider { private readonly tempFileManager = getTempFileManager(); private currentTempFile: string | null = null; + /** The executable that last read the clipboard, so later reads start with it. */ + private powerShell: string | null = null; constructor() { super('windows'); } - async isAvailable(): Promise { - // pwsh also exists on Linux/macOS, but the WinForms clipboard does not - if (process.platform !== 'win32') { - return false; - } - - try { - // Detecting the PowerShell executable doubles as the availability - // check, so a paste pays for at most one probe (cached afterwards) - // instead of an extra PowerShell startup per call. - await this.getPowerShellPath(); - return true; - } catch { - return false; - } + isAvailable(): Promise { + // Windows PowerShell is part of Windows, so there is nothing to look for + // first. Starting a PowerShell only to see that it starts took about as + // long as the read itself; a machine where none starts finds out from the + // read, which says so. (pwsh also exists on Linux and macOS, but the + // WinForms clipboard does not.) + return Promise.resolve(process.platform === 'win32'); } async hasImage(): Promise { @@ -141,42 +166,70 @@ export class WindowsClipboardProvider extends BaseClipboardProvider { throw new ClipboardError('Temp file not initialized'); } - // Encode the script for safe execution - const encodedCommand = encodePowerShellCommand(CLIPBOARD_SCRIPT); - - // Get PowerShell path (prefer pwsh for ARM64) - const psPath = await this.getPowerShellPath(); - - try { - const { stdout, stderr } = await execAsync( - `"${psPath}" -NoLogo -NoProfile -NonInteractive -ExecutionPolicy ${EXECUTION_POLICY} -EncodedCommand ${encodedCommand}`, - { - timeout: TIMEOUTS.CLIPBOARD_READ, - env: { - ...process.env, - CLIP_TEMP_PATH: this.currentTempFile, - }, - windowsHide: true, + // The executable that worked last time, or each candidate in turn. Nothing + // checks beforehand that one starts: the read is the check, so a paste + // costs one PowerShell startup rather than two. + const executables = this.powerShell !== null ? [this.powerShell] : POWERSHELL_EXECUTABLES; + + let failure: ClipboardError | null = null; + for (const executable of executables) { + try { + const data = await this.readWith(executable, this.currentTempFile); + this.powerShell = executable; + return data; + } catch (error) { + if (isTimeout(error)) { + // Waiting out a second timeout would only make the wait longer + throw new ClipboardError('Clipboard read timed out'); + } + if (!isNotFound(error)) { + // Of several failures, the first from a PowerShell that ran is the + // one worth reporting + failure ??= new ClipboardError( + `Failed to read clipboard: ${error instanceof Error ? error.message : String(error)}` + ); } - ); - - // Ignore non-error stderr output: - // - WARNING: PowerShell warnings - // - CLIXML progress messages (contain ' { + const { stdout, stderr } = await execFileAsync(executable, [...POWERSHELL_ARGUMENTS], { + timeout: TIMEOUTS.CLIPBOARD_READ, + env: { + ...process.env, + CLIP_TEMP_PATH: tempFile, + }, + windowsHide: true, + }); + + // Ignore non-error stderr output: + // - WARNING: PowerShell warnings + // - CLIXML progress messages (contain ' { - // Return cached path if available - if (cachedPowerShellPath !== null) { - return cachedPowerShellPath; - } - - // Check for PowerShell 7 (pwsh) first - better for ARM64 - try { - await execAsync('pwsh -NoProfile -Command "echo ok"', { timeout: 5000 }); - cachedPowerShellPath = 'pwsh'; - } catch { - // Fall back to Windows PowerShell, but verify it actually runs so - // isAvailable() reports honestly (e.g. on non-Windows systems) - await execAsync('powershell.exe -NoProfile -Command "echo ok"', { timeout: 5000 }); - cachedPowerShellPath = 'powershell.exe'; - } - - return cachedPowerShellPath; - } - async cleanup(): Promise { if (this.currentTempFile !== null && this.currentTempFile !== '') { await this.tempFileManager.cleanup(this.currentTempFile); @@ -344,7 +377,8 @@ export function parseResultOutput(stdout: string): ParsedResultOutput { } /** - * The PowerShell script used for clipboard operations. + * The PowerShell script used for clipboard operations, and the arguments + * PowerShell is started with to run it. * Exported for testing purposes. */ -export { CLIPBOARD_SCRIPT }; +export { CLIPBOARD_SCRIPT, POWERSHELL_ARGUMENTS }; diff --git a/test/clipboard/providers/windows-provider-powershell.test.ts b/test/clipboard/providers/windows-provider-powershell.test.ts new file mode 100644 index 0000000..e2b29d2 --- /dev/null +++ b/test/clipboard/providers/windows-provider-powershell.test.ts @@ -0,0 +1,254 @@ +/** + * How the Windows provider starts PowerShell + * + * child_process is mocked, so this runs on every platform and never touches the + * real clipboard: the stand-in records how PowerShell was started and answers + * the way the clipboard script would. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { exec, execFile } from 'child_process'; +import { writeFileSync } from 'fs'; + +vi.mock('child_process', async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, exec: vi.fn(), execFile: vi.fn() }; +}); + +import { + WindowsClipboardProvider, + CLIPBOARD_SCRIPT, +} from '../../../src/clipboard/providers/windows-provider'; +import { ClipboardManager } from '../../../src/clipboard/clipboard-manager'; +import { ClipboardError } from '../../../src/core/errors'; +import { PNG_SIGNATURE, TIMEOUTS } from '../../../src/core/constants'; +import { encodePowerShellCommand } from '../../../src/security/sanitizer'; + +interface Start { + file: string; + args: readonly string[]; + options: { env?: NodeJS.ProcessEnv; timeout?: number; windowsHide?: boolean }; +} + +type Outcome = { stdout: string; stderr?: string } | Error; +type Done = (error: Error | null, result?: { stdout: string; stderr: string }) => void; + +/** Every start of PowerShell is recorded and answered with what `respond` returns. */ +function fakePowerShell(respond: (start: Start) => Outcome): Start[] { + const starts: Start[] = []; + vi.mocked(execFile).mockImplementation((( + file: string, + args: readonly string[], + options: Start['options'], + done: Done + ) => { + const start = { file, args, options }; + starts.push(start); + const outcome = respond(start); + if (outcome instanceof Error) { + done(outcome); + } else { + done(null, { stdout: outcome.stdout, stderr: outcome.stderr ?? '' }); + } + }) as never); + return starts; +} + +/** What Node reports when the executable is not on the PATH. */ +function notFound(file: string): Error { + return Object.assign(new Error(`spawn ${file} ENOENT`), { code: 'ENOENT' }); +} + +/** What Node reports when PowerShell ran and exited with an error. */ +function failed(file: string): Error { + return Object.assign(new Error(`Command failed: ${file} -NoLogo`), { code: 1, killed: false }); +} + +/** What Node reports when the child outlived `timeout` and was ended. */ +function timedOut(file: string): Error { + return Object.assign(new Error(`Command failed: ${file} -NoLogo`), { + code: null, + killed: true, + signal: 'SIGTERM', + }); +} + +const TEXT_ONLY = { stdout: '::RESULT::False,True,\r\n' }; + +describe('WindowsClipboardProvider starting PowerShell', () => { + const platform = Object.getOwnPropertyDescriptor(process, 'platform'); + let provider: WindowsClipboardProvider; + + beforeEach(() => { + vi.mocked(execFile).mockReset(); + // Going through a shell is what this provider must not do; if it does, + // fail at once instead of waiting for a callback that never comes. + vi.mocked(exec).mockReset(); + vi.mocked(exec).mockImplementation(((command: string, _options: unknown, done: Done) => { + done(new Error(`started through a shell: ${command}`)); + }) as never); + provider = new WindowsClipboardProvider(); + }); + + afterEach(async () => { + await provider.cleanup(); + vi.unstubAllEnvs(); + if (platform !== undefined) { + Object.defineProperty(process, 'platform', platform); + } + }); + + function onWindows(): void { + Object.defineProperty(process, 'platform', { ...platform, value: 'win32' }); + } + + it('starts Windows PowerShell itself, once, with the script as a single argument', async () => { + const starts = fakePowerShell(() => TEXT_ONLY); + + const data = await provider.getImageData(); + + expect(data).toEqual({ hasImage: false, hasText: true, imageBuffer: null, format: null }); + expect(starts).toHaveLength(1); + expect(starts[0].file).toBe('powershell.exe'); + expect(starts[0].args).toEqual([ + '-NoLogo', + '-NoProfile', + '-NonInteractive', + '-ExecutionPolicy', + 'RemoteSigned', + '-EncodedCommand', + encodePowerShellCommand(CLIPBOARD_SCRIPT), + ]); + expect(starts[0].options.timeout).toBe(TIMEOUTS.CLIPBOARD_READ); + expect(starts[0].options.windowsHide).toBe(true); + expect(exec).not.toHaveBeenCalled(); + }); + + it('passes the temporary file in the environment and returns the image written there', async () => { + vi.stubEnv('CLIPSHOT_TEST_INHERITED', 'kept'); + const png = Buffer.concat([PNG_SIGNATURE, Buffer.from('pixels')]); + const starts = fakePowerShell(({ options }) => { + const target = options.env?.CLIP_TEMP_PATH ?? ''; + writeFileSync(target, png); + return { stdout: `::RESULT::True,False,${target}\r\n` }; + }); + + const data = await provider.getImageData(); + + expect(data.hasImage).toBe(true); + expect(data.format).toBe('png'); + expect(data.imageBuffer?.equals(png)).toBe(true); + expect(starts[0].options.env?.CLIP_TEMP_PATH).toMatch(/\.png$/); + // The rest of the environment goes through untouched + expect(starts[0].options.env?.CLIPSHOT_TEST_INHERITED).toBe('kept'); + }); + + it('answers isAvailable() on Windows without starting anything', async () => { + onWindows(); + const starts = fakePowerShell(() => TEXT_ONLY); + + expect(await provider.isAvailable()).toBe(true); + expect(starts).toHaveLength(0); + expect(exec).not.toHaveBeenCalled(); + }); + + it('starts one PowerShell for the first paste, the availability check included', async () => { + onWindows(); + const starts = fakePowerShell(() => TEXT_ONLY); + const manager = new ClipboardManager(provider); + + await manager.getImageData(); + + expect(starts.map((start) => start.file)).toEqual(['powershell.exe']); + expect(exec).not.toHaveBeenCalled(); + }); + + it('keeps using Windows PowerShell for later reads', async () => { + const starts = fakePowerShell(() => TEXT_ONLY); + + await provider.getImageData(); + await provider.getImageData(); + + expect(starts.map((start) => start.file)).toEqual(['powershell.exe', 'powershell.exe']); + }); + + it('falls back to PowerShell 7 when Windows PowerShell is not found, and stays with it', async () => { + const starts = fakePowerShell(({ file }) => + file === 'powershell.exe' ? notFound(file) : TEXT_ONLY + ); + + const first = await provider.getImageData(); + expect(first.hasText).toBe(true); + expect(starts.map((start) => start.file)).toEqual(['powershell.exe', 'pwsh']); + expect(starts[1].args).toEqual(starts[0].args); + + await provider.getImageData(); + expect(starts.map((start) => start.file)).toEqual(['powershell.exe', 'pwsh', 'pwsh']); + }); + + it('falls back to PowerShell 7 when Windows PowerShell starts but fails', async () => { + const starts = fakePowerShell(({ file }) => + file === 'powershell.exe' ? failed(file) : TEXT_ONLY + ); + + const data = await provider.getImageData(); + + expect(data.hasText).toBe(true); + expect(starts.map((start) => start.file)).toEqual(['powershell.exe', 'pwsh']); + }); + + it('reports what the PowerShell that ran said when the other one is not installed', async () => { + fakePowerShell(({ file }) => + file === 'powershell.exe' + ? { stdout: '', stderr: 'Exception calling "GetImage": the clipboard is busy' } + : notFound(file) + ); + + const failure = await provider.getImageData().then( + () => null, + (error: unknown) => error + ); + + expect(failure).toBeInstanceOf(ClipboardError); + expect((failure as ClipboardError).message).toContain('the clipboard is busy'); + expect((failure as ClipboardError).message).not.toContain('ENOENT'); + }); + + it('says PowerShell could not be started when neither is found, and looks again next time', async () => { + let installed = false; + const starts = fakePowerShell(({ file }) => (installed ? TEXT_ONLY : notFound(file))); + + const failure = await provider.getImageData().then( + () => null, + (error: unknown) => error + ); + + expect(failure).toBeInstanceOf(ClipboardError); + expect((failure as ClipboardError).message).toMatch(/PowerShell could not be started/); + expect((failure as ClipboardError).userMessage).toBe('PowerShell could not be started'); + expect(starts.map((start) => start.file)).toEqual(['powershell.exe', 'pwsh']); + + installed = true; + await provider.getImageData(); + expect(starts.map((start) => start.file)).toEqual(['powershell.exe', 'pwsh', 'powershell.exe']); + }); + + it('does not start another PowerShell after a timeout', async () => { + const starts = fakePowerShell(({ file }) => timedOut(file)); + + await expect(provider.getImageData()).rejects.toThrow('Clipboard read timed out'); + expect(starts.map((start) => start.file)).toEqual(['powershell.exe']); + }); + + it('accepts the progress record PowerShell writes to stderr on a first run', async () => { + const progress = + '#< CLIXML\n' + + 'Completed'; + const starts = fakePowerShell(() => ({ ...TEXT_ONLY, stderr: progress })); + + const data = await provider.getImageData(); + + expect(data.hasText).toBe(true); + expect(starts).toHaveLength(1); + }); +}); diff --git a/test/clipboard/providers/windows-provider.test.ts b/test/clipboard/providers/windows-provider.test.ts index 32408b5..20cccae 100644 --- a/test/clipboard/providers/windows-provider.test.ts +++ b/test/clipboard/providers/windows-provider.test.ts @@ -7,16 +7,21 @@ */ import { describe, it, expect } from 'vitest'; -import { exec } from 'child_process'; +import { exec, execFile } from 'child_process'; +import { mkdtempSync, rmSync } from 'fs'; +import { tmpdir } from 'os'; +import { join } from 'path'; import { promisify } from 'util'; import { parseResultOutput, CLIPBOARD_SCRIPT, + POWERSHELL_ARGUMENTS, isARM64Windows, WindowsClipboardProvider, } from '../../../src/clipboard/providers/windows-provider'; const execAsync = promisify(exec); +const execFileAsync = promisify(execFile); describe('WindowsClipboardProvider', () => { describe('parseResultOutput', () => { @@ -289,5 +294,28 @@ describe('WindowsClipboardProvider', () => { expect(stderr).toBe(''); }); + + it('should run the clipboard script when started the way the provider starts it', async () => { + if (process.platform !== 'win32') { + return; + } + + // The same executable and argument array as the provider, without a + // shell: this is where an argument mangled on its way to PowerShell + // would show. Only reads the clipboard; an image on it lands in the + // temporary folder removed below. + const folder = mkdtempSync(join(tmpdir(), 'clipshot-test-')); + try { + const { stdout } = await execFileAsync('powershell.exe', [...POWERSHELL_ARGUMENTS], { + timeout: POWERSHELL_TIMEOUT_MS, + env: { ...process.env, CLIP_TEMP_PATH: join(folder, 'clipboard.png') }, + windowsHide: true, + }); + + expect(stdout).toMatch(/^::RESULT::(True|False),(True|False),/m); + } finally { + rmSync(folder, { recursive: true, force: true }); + } + }); }); });