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 }); + } + }); }); });