From a42827b00633b0f59b8dbad9931596a96e67d10a Mon Sep 17 00:00:00 2001 From: Greg Priday Date: Tue, 13 Oct 2026 13:25:29 +0200 Subject: [PATCH] fix(clipboard): copy macOS file references without Finder automation - Write the file URL to NSPasteboard from osascript's own process (JXA AppKit bridge) instead of `tell app "Finder"`, which macOS refuses with -1743 and no prompt in terminals that lack Apple Events access, such as Daintree's - Pass the path as an argument after `--`, so it needs no escaping and a leading dash is not read as an option - copyFileReference now reports when it could only copy the path as text, with the reason, on every platform - Report that as "Path copied as text, not as a file reference" rather than "File reference copied", and fail it under --strict (exit 1); ignore context warns the same way - Cover the fallback with a cross-platform deliverOutput test and a macOS e2e, and document it --- docs/reference/migrating-to-1.0.md | 4 + docs/technical/behavior-ledger.md | 4 + src/commands/copy.js | 14 ++- src/commands/ignore.js | 11 +- src/ui/feedback/model.js | 18 ++- src/utils/clipboard.js | 107 ++++++++++++----- tests/e2e/file-reference.os.test.js | 52 ++++++++- .../unit/commands/copy.deliverOutput.test.js | 97 +++++++++++++++ tests/unit/ui/feedback/model.test.js | 20 ++++ tests/unit/utils/clipboard.test.js | 110 +++++++++++++----- 10 files changed, 364 insertions(+), 73 deletions(-) create mode 100644 tests/unit/commands/copy.deliverOutput.test.js diff --git a/docs/reference/migrating-to-1.0.md b/docs/reference/migrating-to-1.0.md index 85efea6..0a56a54 100644 --- a/docs/reference/migrating-to-1.0.md +++ b/docs/reference/migrating-to-1.0.md @@ -236,6 +236,10 @@ that had to fall back to the built-in patterns. `--strict` is documented as "enable every applicable policy-failure check" and did not read that list. It does now, and exits 3. +A clipboard that could not take what was asked for is a different case: the +machine failed rather than a check, so `--strict` exits 1. That includes a file +reference the platform could only copy as its path in text. + ### `toJSON()` no longer includes the stack `error.toJSON()` — which is also what `JSON.stringify(error)` calls — returns diff --git a/docs/technical/behavior-ledger.md b/docs/technical/behavior-ledger.md index 89642cc..0f3af58 100644 --- a/docs/technical/behavior-ledger.md +++ b/docs/technical/behavior-ledger.md @@ -467,6 +467,10 @@ kilobytes of inline context. - `-r` / `--as-reference` is accepted and does nothing, since it now selects the default. - `-o`, `--display`, and `--stream` are unaffected and still take precedence. +- Where the platform cannot put a file on the clipboard, the path is copied as + text and the run says "Path copied as text, not as a file reference", with the + reason; `--strict` makes that exit 1. On macOS the reference is written to the + pasteboard directly, so it needs no Automation permission for Finder. **Folder-profile discovery is no longer tied to `-r`.** Auto-discovery of a `.copytree.yml` used to happen only when `-r` was passed. diff --git a/src/commands/copy.js b/src/commands/copy.js index cf2087f..4bad193 100644 --- a/src/commands/copy.js +++ b/src/commands/copy.js @@ -649,7 +649,19 @@ async function deliverOutput(result, request, root, reporter) { if (destination.type === 'reference') { const tempFile = await writeReferenceFile(output, root, request.content.format); try { - await (await clipboard()).copyFileReference(tempFile); + const copied = await (await clipboard()).copyFileReference(tempFile); + if (copied?.asText) { + // The path is on the clipboard, which an agent can still open, but a + // browser will not attach it, so this is not the reference asked for. + return { + requested: 'reference', + actual: 'path', + status: 'warning', + path: tempFile, + fallbackUsed: true, + cause: copied.cause, + }; + } return { requested: 'reference', actual: 'reference', diff --git a/src/commands/ignore.js b/src/commands/ignore.js index cafabc9..a1c9229 100644 --- a/src/commands/ignore.js +++ b/src/commands/ignore.js @@ -239,8 +239,15 @@ async function ignoreContext(request, context) { const file = await writeReferenceFile(text, root, format === 'json' ? 'json' : 'markdown'); const { default: clipboard } = await import('../utils/clipboard.js'); try { - await clipboard.copyFileReference(file); - feedback.write(`Ignore context written to ${file} and copied as a file reference`); + const copied = await clipboard.copyFileReference(file); + if (copied?.asText) { + feedback.write( + `Ignore context written to ${file}; its path was copied as text (${copied.cause})`, + { level: 'warn' }, + ); + } else { + feedback.write(`Ignore context written to ${file} and copied as a file reference`); + } } catch (error) { feedback.write( `Ignore context written to ${file} (clipboard unavailable: ${error.message})`, diff --git a/src/ui/feedback/model.js b/src/ui/feedback/model.js index 4ac500a..1b1e0b0 100644 --- a/src/ui/feedback/model.js +++ b/src/ui/feedback/model.js @@ -222,7 +222,7 @@ export function classifyWarnings(result = {}, extra = {}) { if (extra.delivery?.fallbackUsed) { warnings.push({ code: WARNING_CODES.CLIPBOARD_FALLBACK, - message: `Clipboard unavailable — output saved to ${extra.delivery.path}`, + message: fallbackHeadline(extra.delivery), data: { path: extra.delivery.path, cause: extra.delivery.cause }, }); } @@ -230,6 +230,17 @@ export function classifyWarnings(result = {}, extra = {}) { return warnings; } +/** + * What a clipboard fallback left the reader holding. + * + * @param {Object} [delivery] - Delivery result with `fallbackUsed` + * @returns {string} Headline without a glyph + */ +function fallbackHeadline(delivery) { + if (delivery?.actual === 'path') return 'Path copied as text, not as a file reference'; + return `Clipboard unavailable — output saved to ${delivery?.path ?? 'a temporary file'}`; +} + /** * Things worth mentioning that are not problems. * @@ -314,10 +325,11 @@ export function buildCompletionModel({ return { event: FEEDBACK_EVENTS.RUN_COMPLETE, status: 'warning', - headline: `Clipboard unavailable — output saved to ${delivery?.path ?? 'a temporary file'}`, + headline: fallbackHeadline(delivery), metrics: buildMetrics(stats, verbose), warnings: warnings.filter((w) => w !== fallback), - notes: notices, + // The headline says what happened; the cause is what the reader can fix. + notes: delivery?.actual === 'path' && delivery.cause ? [delivery.cause, ...notices] : notices, details: { path: delivery?.path, destination: delivery?.actual }, stats, }; diff --git a/src/utils/clipboard.js b/src/utils/clipboard.js index 8f69dad..dae4ccb 100644 --- a/src/utils/clipboard.js +++ b/src/utils/clipboard.js @@ -7,11 +7,11 @@ import { logger } from './logger.js'; /** * How long to wait for the platform's clipboard helper. * - * Short on purpose: a wedged Finder or a headless session must not hang a copy + * Short on purpose: a wedged helper or a headless session must not hang a copy * that has otherwise finished. But two seconds is a guess about someone else's * machine, and it is wrong on a loaded CI runner, a remote desktop session, or * a VM with cold page cache — where the helper is killed mid-write and the run - * silently falls back to copying text. + * falls back to copying the path as text. * * `COPYTREE_CLIPBOARD_TIMEOUT_MS` raises it for those environments. * @@ -22,6 +22,27 @@ function clipboardTimeout() { return Number.isInteger(configured) && configured > 0 ? configured : 2000; } +/** + * Put a file URL on the general pasteboard from osascript's own process. + * + * `tell app "Finder" to set the clipboard` is an Apple Event, and macOS lets it + * through only when the terminal hosting this process declares Automation + * access and the user has approved it. A terminal that does not declare it is + * refused with -1743 and no prompt, so the copy degraded to text there. Writing + * NSPasteboard through the AppKit bridge sends no Apple Event and needs no + * consent; `writeObjects` with an NSURL publishes `public.file-url` and the + * legacy `NSFilenamesPboardType`, which is what browsers read for a file paste. + * The path arrives as an argument, so it needs no escaping. + */ +const MACOS_PASTEBOARD_SCRIPT = `ObjC.import('AppKit'); +function run(argv) { + const pasteboard = $.NSPasteboard.generalPasteboard; + pasteboard.clearContents; + if (!pasteboard.writeObjects($([$.NSURL.fileURLWithPath(argv[0])]))) { + throw new Error('the pasteboard did not accept the file URL'); + } +}`; + /** * Enhanced clipboard utilities with file reference support */ @@ -37,10 +58,32 @@ class Clipboard { return await clipboardy.write(text); } + /** + * Copy the path as text in place of a file reference. + * + * An agent can still open a pasted path, so this is a degraded success rather + * than a failure. It is reported as one, because a browser will not attach a + * path and "File reference copied" would send the reader to paste it there. + * + * @param {string} text - Path or file URI to copy + * @param {string} cause - Why no file reference could be copied + * @returns {Promise<{asText: true, cause: string}>} Outcome + * @private + */ + static async _copyAsText(text, cause) { + await this.copyText(text); + return { asText: true, cause }; + } + /** * Copy file reference to clipboard (cross-platform) + * + * Falls back to the path as text when the platform cannot take a file, and + * says so in the result; rejects only when the text copy fails too. + * * @param {string} filePath - The file path to copy as reference - * @returns {Promise} + * @returns {Promise<{asText: boolean, cause?: string}>} Whether a file reference + * was copied, or only its path as text and why */ static async copyFileReference(filePath) { if (process.platform === 'win32') { @@ -65,7 +108,7 @@ class Clipboard { 'Failed to copy file reference using PowerShell, falling back to text:', error.message, ); - return this.copyText(filePath); + return this._copyAsText(filePath, error.message); } } else if (process.platform === 'linux') { try { @@ -73,8 +116,7 @@ class Clipboard { const method = Clipboard._detectLinuxClipboardMethod(fileUri); if (!method) { - // Neither xclip nor wl-copy available; fall back to plain-text URI - return this.copyText(fileUri); + return this._copyAsText(fileUri, 'no display server detected'); } const result = spawnSync(method.tool, method.args, { @@ -88,7 +130,10 @@ class Clipboard { `Failed to copy file reference using ${method.tool}, falling back to text:`, result.error?.message || `exit code ${result.status}`, ); - return this.copyText(fileUri); + return this._copyAsText( + fileUri, + result.error?.message || `${method.tool} exited with code ${result.status}`, + ); } } catch (error) { logger.debug( @@ -96,44 +141,44 @@ class Clipboard { error.message, ); const fileUri = url.pathToFileURL(filePath).toString(); - return this.copyText(fileUri); + return this._copyAsText(fileUri, error.message); } } else if (process.platform === 'darwin') { try { - // Use AppleScript to copy file reference. spawnSync passes the script as a - // direct argument (no shell), so single quotes in filePath are safe. - const escapedFilePath = filePath.replace(/\\/g, '\\\\').replace(/"/g, '\\"'); - const script = `set aFile to POSIX file "${escapedFilePath}" -tell app "Finder" to set the clipboard to aFile`; - - const result = spawnSync('osascript', ['-e', script], { - encoding: 'utf8', - stdio: 'pipe', - timeout: clipboardTimeout(), - }); + const result = spawnSync( + 'osascript', + // `--` so a relative path starting with `-` is not read as an option. + ['-l', 'JavaScript', '-e', MACOS_PASTEBOARD_SCRIPT, '--', filePath], + { + encoding: 'utf8', + stdio: 'pipe', + timeout: clipboardTimeout(), + }, + ); // `spawnSync` does not throw when the child exits non-zero, so the - // result had to be inspected and was not. A headless session, a denied - // automation permission or a Finder that will not answer all left the - // clipboard untouched while the run reported "File reference copied" — - // the user pastes, gets whatever was there before, and has no reason to - // suspect this command. The Linux branch above already checks both. + // result had to be inspected and was not. A headless session or a + // pasteboard that would not take the URL left no file on the clipboard + // while the run reported "File reference copied" — the user pastes, + // gets something else, and has no reason to suspect this command. if (result?.error || (result && result.status !== 0)) { + // The first line only: the reason becomes a note under the headline. const reason = result.error?.message || - (result.stderr || '').trim() || - `osascript exited with code ${result.status}`; + (result.stderr || '').trim().split('\n')[0] || + (result.signal + ? `osascript was stopped by ${result.signal}` + : `osascript exited with code ${result.status}`); throw new Error(reason); } } catch (error) { logger.debug('Failed to copy file reference, falling back to text:', error.message); - // Fallback to copying path as text - return this.copyText(filePath); + return this._copyAsText(filePath, error.message); } } else { - // Fallback for other OSes - return this.copyText(filePath); + return this._copyAsText(filePath, `${process.platform} has no file-reference mechanism`); } + return { asText: false }; } /** @@ -151,7 +196,7 @@ tell app "Finder" to set the clipboard to aFile`; return { supported: true, mechanism: 'osascript', - detail: 'Finder clipboard via AppleScript', + detail: 'NSPasteboard via osascript', }; } if (process.platform === 'win32') { diff --git a/tests/e2e/file-reference.os.test.js b/tests/e2e/file-reference.os.test.js index a9fb33d..98edbc1 100644 --- a/tests/e2e/file-reference.os.test.js +++ b/tests/e2e/file-reference.os.test.js @@ -83,13 +83,12 @@ describe('OS-specific file reference copy', () => { expect(existsSync(logFile)).toBe(true); const logged = readFileSync(logFile, 'utf8'); - expect(logged).toContain('-e'); - expect(logged).toContain('set aFile to POSIX file'); - expect(logged).toContain('tell app "Finder" to set the clipboard to aFile'); + expect(logged).toContain('JavaScript'); + expect(logged).toContain('NSPasteboard'); + expect(logged).not.toContain('Finder'); - const match = logged.match(/POSIX file "([^"]+\.xml)"/); - expect(match).not.toBeNull(); - const referencedPath = match[1].replace(/\\"/g, '"').replace(/\\\\/g, '\\'); + // The path is the script's last argument, one per logged line. + const referencedPath = logged.trimEnd().split('\n').at(-1); // `/copytree//-.` expect(referencedPath).toMatch(/\/copytree\/simple-project\//); expect(path.basename(referencedPath)).toMatch(/^\d{4}-\d{2}-\d{2}T[\d-]+Z-[0-9a-f]{8}\.xml$/); @@ -166,4 +165,45 @@ describe('OS-specific file reference copy', () => { // freshly-written script is genuinely slow under some sandboxes and // security tooling, so the headroom is not theoretical. }, 60000); + + // A file reference that degrades to the path as text used to report "File + // reference copied", so `--strict` passed and the reader pasted a path into a + // browser expecting an attachment. + (process.platform === 'darwin' ? test : test.skip)( + 'reports a reference that fell back to text, and fails it under --strict', + async () => { + tempDir = mkdtempSync(path.join(os.tmpdir(), 'copytree-file-ref-text-')); + const binDir = path.join(tempDir, 'bin'); + const logFile = path.join(tempDir, 'pbcopy.log'); + mkdirSync(binDir, { recursive: true }); + writeFileSync( + path.join(binDir, 'osascript'), + '#!/bin/sh\necho "execution error: the pasteboard is unavailable" >&2\nexit 1\n', + { mode: 0o755 }, + ); + writeFileSync(path.join(binDir, 'pbcopy'), '#!/bin/sh\ncat > "$COPYTREE_TEST_LOG"\n', { + mode: 0o755, + }); + + const { code, stderr } = await runCli( + [PROJECT, '--as-reference', '--format', 'xml', '--strict'], + { + env: { + COPYTREE_TEST_LOG: logFile, + PATH: prependPath(binDir, process.env.PATH), + TMPDIR: tempDir, + TMP: tempDir, + TEMP: tempDir, + COPYTREE_CLIPBOARD_TIMEOUT_MS: '30000', + }, + }, + ); + + expect(code).toBe(1); + expect(stderr).toContain('Delivery fell back to path'); + expect(stderr).toContain('the pasteboard is unavailable'); + expectReferencePath(readFileSync(logFile, 'utf8').trim()); + }, + 60000, + ); }); diff --git a/tests/unit/commands/copy.deliverOutput.test.js b/tests/unit/commands/copy.deliverOutput.test.js new file mode 100644 index 0000000..6b76026 --- /dev/null +++ b/tests/unit/commands/copy.deliverOutput.test.js @@ -0,0 +1,97 @@ +/** + * Reference delivery, as the command reports it. + * + * The clipboard layer can put a file on the clipboard, or only its path as + * text. These run on every platform, so the classification is checked on the + * Linux CI runners too, not only by the macOS end-to-end test. + */ + +import { jest } from '@jest/globals'; +import path from 'path'; +import os from 'os'; +import { mkdtempSync, rmSync, existsSync } from 'fs'; + +jest.unmock('../../../src/utils/fsx.js'); + +let deliverOutput; +let Clipboard; +let classifyWarnings; +let buildCompletionModel; +let WARNING_CODES; + +beforeAll(async () => { + ({ deliverOutput } = await import('../../../src/commands/copy.js')); + ({ default: Clipboard } = await import('../../../src/utils/clipboard.js')); + ({ classifyWarnings, buildCompletionModel, WARNING_CODES } = + await import('../../../src/ui/feedback/model.js')); +}); + +describe('deliverOutput — reference destination', () => { + let tempDir; + let written; + const reporter = { phase: jest.fn() }; + const request = { destination: { type: 'reference' }, content: { format: 'xml' } }; + + beforeEach(() => { + tempDir = mkdtempSync(path.join(os.tmpdir(), 'copytree-deliver-')); + written = null; + }); + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }); + // References land under the shared temp root, in a folder named for the + // project, which here is this test's own temp directory. + if (written) rmSync(path.dirname(written), { recursive: true, force: true }); + }); + + async function deliver() { + const delivery = await deliverOutput({ output: '' }, request, tempDir, reporter); + written = delivery.path; + return delivery; + } + + it('reports a file reference when one was copied', async () => { + jest.spyOn(Clipboard, 'copyFileReference').mockResolvedValue({ asText: false }); + + const delivery = await deliver(); + + expect(delivery).toMatchObject({ actual: 'reference', fallbackUsed: false }); + expect(existsSync(delivery.path)).toBe(true); + expect(classifyWarnings({ stats: {} }, { delivery })).toEqual([]); + }); + + it('reports the path as text, with its cause, when no file could be copied', async () => { + jest + .spyOn(Clipboard, 'copyFileReference') + .mockResolvedValue({ asText: true, cause: 'no display server detected' }); + + const delivery = await deliver(); + + expect(delivery).toMatchObject({ + requested: 'reference', + actual: 'path', + status: 'warning', + fallbackUsed: true, + cause: 'no display server detected', + }); + const warnings = classifyWarnings({ stats: {} }, { delivery }); + expect(warnings.map((w) => w.code)).toEqual([WARNING_CODES.CLIPBOARD_FALLBACK]); + const model = buildCompletionModel({ delivery, stats: { files: 1 }, warnings }); + expect(model.headline).toBe('Path copied as text, not as a file reference'); + expect(model.notes).toEqual(['no display server detected']); + }); + + it('reports the saved file when even the text copy failed', async () => { + jest.spyOn(Clipboard, 'copyFileReference').mockRejectedValue(new Error('no clipboard')); + + const delivery = await deliver(); + + expect(delivery).toMatchObject({ actual: 'file', fallbackUsed: true, cause: 'no clipboard' }); + const model = buildCompletionModel({ + delivery, + stats: { files: 1 }, + warnings: classifyWarnings({ stats: {} }, { delivery }), + }); + expect(model.headline).toBe(`Clipboard unavailable — output saved to ${delivery.path}`); + }); +}); diff --git a/tests/unit/ui/feedback/model.test.js b/tests/unit/ui/feedback/model.test.js index 2bde454..4008b3a 100644 --- a/tests/unit/ui/feedback/model.test.js +++ b/tests/unit/ui/feedback/model.test.js @@ -143,6 +143,26 @@ describe('completion models', () => { expect(model.warnings).toHaveLength(0); }); + it('says the path went as text when a reference could not be copied', () => { + const delivery = { + requested: 'reference', + actual: 'path', + path: '/tmp/proj.xml', + fallbackUsed: true, + cause: 'execution error: the pasteboard is unavailable', + }; + const model = buildCompletionModel({ + delivery, + stats, + warnings: classifyWarnings({ stats: {} }, { delivery }), + }); + + expect(model.status).toBe('warning'); + expect(model.headline).toBe('Path copied as text, not as a file reference'); + expect(model.notes).toEqual(['execution error: the pasteboard is unavailable']); + expect(model.warnings).toHaveLength(0); + }); + it('reports a preview as neutral, not success', () => { const model = buildDryRunModel({ stats }); diff --git a/tests/unit/utils/clipboard.test.js b/tests/unit/utils/clipboard.test.js index 8d43170..2ec2f0d 100644 --- a/tests/unit/utils/clipboard.test.js +++ b/tests/unit/utils/clipboard.test.js @@ -93,9 +93,10 @@ describe('Clipboard', () => { }); const spy = jest.spyOn(Clipboard, 'copyText').mockResolvedValueOnce(); - await Clipboard.copyFileReference('C:\\test\\file.xml'); + const copied = await Clipboard.copyFileReference('C:\\test\\file.xml'); expect(spy).toHaveBeenCalledWith('C:\\test\\file.xml'); + expect(copied).toEqual({ asText: true, cause: 'PowerShell not found' }); spy.mockRestore(); }); }); @@ -103,7 +104,7 @@ describe('Clipboard', () => { describe('copyFileReference — macOS', () => { beforeEach(() => setPlatform('darwin')); - it('uses spawnSync with osascript (no shell layer)', async () => { + it('writes the pasteboard through osascript (no shell layer)', async () => { spawnSync.mockReturnValue({ status: 0, stdout: '', stderr: '' }); await Clipboard.copyFileReference('/Users/test/output.xml'); @@ -111,11 +112,30 @@ describe('Clipboard', () => { expect(execSync).not.toHaveBeenCalled(); expect(spawnSync).toHaveBeenCalledWith( 'osascript', - ['-e', expect.stringContaining('POSIX file "/Users/test/output.xml"')], + [ + '-l', + 'JavaScript', + '-e', + expect.stringContaining('NSPasteboard'), + '--', + '/Users/test/output.xml', + ], expect.objectContaining({ stdio: 'pipe' }), ); }); + // An Apple Event to Finder is refused with -1743, and no prompt, in any + // terminal that does not declare Automation access, so the copy degraded to + // text there. Writing the pasteboard in-process needs no consent. + it('sends no Apple Event to Finder', async () => { + spawnSync.mockReturnValue({ status: 0, stdout: '', stderr: '' }); + await Clipboard.copyFileReference('/Users/test/output.xml'); + + const script = spawnSync.mock.calls[0][1][3]; + expect(script).not.toMatch(/Finder|tell app/i); + expect(script).toContain('writeObjects'); + }); + // `spawnSync` does not throw when the child exits non-zero, so the result // had to be inspected and was not: a denied automation permission or a // headless session left the clipboard untouched while the run reported @@ -124,12 +144,46 @@ describe('Clipboard', () => { spawnSync.mockReturnValue({ status: 1, stderr: 'execution error: not allowed' }); const spy = jest.spyOn(Clipboard, 'copyText').mockResolvedValue(undefined); - await Clipboard.copyFileReference('/Users/test/output.xml'); + const copied = await Clipboard.copyFileReference('/Users/test/output.xml'); expect(spy).toHaveBeenCalledWith('/Users/test/output.xml'); + expect(copied).toEqual({ asText: true, cause: 'execution error: not allowed' }); spy.mockRestore(); }); + it('keeps a path starting with a dash from being read as an option', async () => { + spawnSync.mockReturnValue({ status: 0, stdout: '', stderr: '' }); + await Clipboard.copyFileReference('-exports/context.xml'); + + expect(spawnSync.mock.calls[0][1].slice(-2)).toEqual(['--', '-exports/context.xml']); + }); + + it('gives the first line of stderr as the cause', async () => { + spawnSync.mockReturnValue({ status: 1, stderr: 'execution error: refused\n at run\n' }); + jest.spyOn(Clipboard, 'copyText').mockResolvedValue(undefined); + + const copied = await Clipboard.copyFileReference('/Users/test/output.xml'); + + expect(copied.cause).toBe('execution error: refused'); + }); + + it('names the signal when osascript is stopped without a word', async () => { + spawnSync.mockReturnValue({ status: null, signal: 'SIGKILL', stderr: '' }); + jest.spyOn(Clipboard, 'copyText').mockResolvedValue(undefined); + + const copied = await Clipboard.copyFileReference('/Users/test/output.xml'); + + expect(copied.cause).toBe('osascript was stopped by SIGKILL'); + }); + + it('reports a file reference when the pasteboard took it', async () => { + spawnSync.mockReturnValue({ status: 0, stdout: '', stderr: '' }); + + await expect(Clipboard.copyFileReference('/Users/test/output.xml')).resolves.toEqual({ + asText: false, + }); + }); + it('falls back to text when osascript cannot be spawned', async () => { spawnSync.mockReturnValue({ error: new Error('ENOENT'), status: null }); const spy = jest.spyOn(Clipboard, 'copyText').mockResolvedValue(undefined); @@ -153,29 +207,19 @@ describe('Clipboard', () => { spy.mockRestore(); }); - it('escapes double quotes for AppleScript', async () => { - await Clipboard.copyFileReference('/Users/test/"quoted"/file.xml'); - - const script = spawnSync.mock.calls[0][1][1]; - expect(script).toContain('\\"quoted\\"'); - }); - - it('escapes backslashes for AppleScript', async () => { - await Clipboard.copyFileReference('/Users/test\\path/file.xml'); - - const script = spawnSync.mock.calls[0][1][1]; - expect(script).toContain('test\\\\path'); - }); - - it('safely handles paths with single quotes (no shell injection)', async () => { - // Previously used execSync(`osascript -e '...'`) which was vulnerable to single-quote injection. - // Now uses spawnSync — single quotes in the path are passed verbatim to osascript. - await Clipboard.copyFileReference("/Users/it's/file.xml"); + it.each([ + ['double quotes', '/Users/test/"quoted"/file.xml'], + ['backslashes', '/Users/test\\path/file.xml'], + ['single quotes', "/Users/it's/file.xml"], + ['spaces', '/Users/test/My Project/file.xml'], + ])('passes a path with %s as an argument, unescaped', async (_label, filePath) => { + spawnSync.mockReturnValue({ status: 0, stdout: '', stderr: '' }); + await Clipboard.copyFileReference(filePath); - expect(spawnSync).toHaveBeenCalledTimes(1); expect(execSync).not.toHaveBeenCalled(); - const script = spawnSync.mock.calls[0][1][1]; - expect(script).toContain("it's"); + const args = spawnSync.mock.calls[0][1]; + expect(args.at(-1)).toBe(filePath); + expect(args[3]).not.toContain(filePath); }); }); @@ -277,9 +321,10 @@ describe('Clipboard', () => { // Neither DISPLAY nor WAYLAND_DISPLAY set const spy = jest.spyOn(Clipboard, 'copyText').mockResolvedValueOnce(); - await Clipboard.copyFileReference('/home/user/file.txt'); + const copied = await Clipboard.copyFileReference('/home/user/file.txt'); expect(spy).toHaveBeenCalledWith('file:///home/user/file.txt'); + expect(copied).toEqual({ asText: true, cause: 'no display server detected' }); expect(spawnSync).not.toHaveBeenCalled(); spy.mockRestore(); }); @@ -290,7 +335,7 @@ describe('Clipboard', () => { spawnSync.mockReturnValueOnce({ status: 1, error: null }); const spy = jest.spyOn(Clipboard, 'copyText').mockResolvedValueOnce(); - await Clipboard.copyFileReference('/home/user/file.txt'); + const copied = await Clipboard.copyFileReference('/home/user/file.txt'); expect(spawnSync).toHaveBeenCalledTimes(1); expect(spawnSync).toHaveBeenCalledWith( @@ -299,6 +344,7 @@ describe('Clipboard', () => { expect.objectContaining({ input: 'copy\nfile:///home/user/file.txt' }), ); expect(spy).toHaveBeenCalledWith('file:///home/user/file.txt'); + expect(copied).toEqual({ asText: true, cause: 'xclip exited with code 1' }); spy.mockRestore(); }); @@ -307,10 +353,11 @@ describe('Clipboard', () => { spawnSync.mockReturnValueOnce({ status: null, error: new Error('ENOENT') }); const spy = jest.spyOn(Clipboard, 'copyText').mockResolvedValueOnce(); - await Clipboard.copyFileReference('/home/user/file.txt'); + const copied = await Clipboard.copyFileReference('/home/user/file.txt'); expect(spawnSync).toHaveBeenCalledTimes(1); expect(spy).toHaveBeenCalledWith('file:///home/user/file.txt'); + expect(copied).toEqual({ asText: true, cause: 'ENOENT' }); spy.mockRestore(); }); @@ -321,9 +368,10 @@ describe('Clipboard', () => { }); const spy = jest.spyOn(Clipboard, 'copyText').mockResolvedValueOnce(); - await Clipboard.copyFileReference('/home/user/file.txt'); + const copied = await Clipboard.copyFileReference('/home/user/file.txt'); expect(spy).toHaveBeenCalledWith('file:///home/user/file.txt'); + expect(copied).toEqual({ asText: true, cause: 'spawn exploded' }); spy.mockRestore(); }); @@ -332,7 +380,9 @@ describe('Clipboard', () => { process.env.XDG_CURRENT_DESKTOP = 'GNOME'; spawnSync.mockReturnValueOnce({ status: 0, error: null }); - await Clipboard.copyFileReference('/home/user/my documents/file.txt'); + await expect( + Clipboard.copyFileReference('/home/user/my documents/file.txt'), + ).resolves.toEqual({ asText: false }); expect(spawnSync).toHaveBeenCalledWith( 'xclip',