Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions packages/cli/src/commands/promote.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import fs from 'fs';
import { quoteShellArg } from '../utils/shell.js';
import path from 'path';
import chalk from 'chalk';
import { TypeScriptParser, WorkflowBuilder } from '@n8n-as-code/transformer';
Expand Down Expand Up @@ -1235,6 +1236,3 @@ function quoteString(value: string): string {
return `'${value.replace(/\\/g, '\\\\').replace(/'/g, "\\'")}'`;
}

function quoteShellArg(value: string): string {
return `'${value.replace(/'/g, `'\\''`)}'`;
}
4 changes: 1 addition & 3 deletions packages/cli/src/commands/update-ai.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import chalk from 'chalk';
import { quoteShellArg } from '../utils/shell.js';
import fs from 'fs';
import { readFileSync, existsSync } from 'fs';
import { join, dirname, resolve, delimiter, basename } from 'path';
Expand Down Expand Up @@ -61,9 +62,6 @@ function readAgentsMdLevel(projectRoot: string): number | undefined {
return Number.isSafeInteger(parsed) ? parsed : undefined;
}

function quoteShellArg(value: string): string {
return `'${value.replace(/'/g, `'\\''`)}'`;
}

function hasWorkspaceDevCommand(projectRoot: string): boolean {
return N8NAC_DEV_CONFIG_FILENAMES.some((filename) => existsSync(join(projectRoot, filename)));
Expand Down
8 changes: 3 additions & 5 deletions packages/cli/src/core/services/sync-manager.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import fs from 'fs';
import { quoteShellArg } from '../../utils/shell.js';
import path from 'path';
import EventEmitter from 'events';
import { N8nApiClient } from './n8n-api-client.js';
Expand Down Expand Up @@ -395,7 +396,7 @@ export class SyncManager extends EventEmitter {
const suggestedPath = scopeRelativeToCwd === '' ? `./${trimmed}` : path.join(scopeRelativeToCwd, trimmed);
throw new Error(
`Cannot push "${trimmed}": use the full relative path to the workflow file, not a bare filename.\n` +
`Example: n8nac push ${this.quoteShellArg(suggestedPath)}`
`Example: n8nac push ${quoteShellArg(suggestedPath)}`
);
}

Expand Down Expand Up @@ -424,7 +425,7 @@ export class SyncManager extends EventEmitter {
`Cannot push "${trimmed}": path is not within the active sync scope.\n` +
`Active sync scope : ${scopeLabel}\n` +
`Expected path form: ${suggestedPath}\n` +
`Run : n8nac push ${this.quoteShellArg(suggestedPath)}\n\n` +
`Run : n8nac push ${quoteShellArg(suggestedPath)}\n\n` +
`Tip: run \`n8nac workspace status --json\` and read \`workflowsPath\` to ` +
`get the exact relative path where workflow files must be created and pushed from.`
);
Expand All @@ -447,9 +448,6 @@ export class SyncManager extends EventEmitter {
}
}

private quoteShellArg(value: string): string {
return `'${value.replace(/'/g, `'\\''`)}'`;
}

public async resolveConflict(workflowId: string, filename: string, resolution: 'local' | 'remote'): Promise<void> {
await this.ensureInitialized();
Expand Down
24 changes: 24 additions & 0 deletions packages/cli/src/utils/shell.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* Quote a path or argument for a shell we do not control.
*
* Nothing here is executed by us. The result is written into generated agent context
* and into CLI hints, for a person or an agent to paste into whatever shell they have.
* On Windows that is often `cmd.exe`, which does not strip POSIX single quotes: it hands
* them to the program verbatim, so a single-quoted path fails the moment it contains a
* space. Double quotes are understood by `cmd.exe`, PowerShell and bash alike, which
* makes them the only portable choice there. Windows forbids `"` in a path, so nothing
* needs escaping.
*
* POSIX keeps single quotes, which suppress every expansion; double quotes do not.
*
* Known ceiling: on Windows a value containing `$` or a backtick still expands under
* PowerShell and bash. Both are legal in Windows filenames, and no escaping satisfies
* cmd.exe, PowerShell and bash at once. Emit a relative path where you can.
*
* `platform` is injectable so the Windows branch is testable from any host.
*/
export function quoteShellArg(value: string, platform: NodeJS.Platform = process.platform): string {
return platform === 'win32'
? `"${value}"`
Comment thread
coderabbitai[bot] marked this conversation as resolved.
: `'${value.replace(/'/g, `'\\''`)}'`;
}
32 changes: 32 additions & 0 deletions packages/cli/tests/unit/shell.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { describe, expect, it } from 'vitest';
import { quoteShellArg } from '../../src/utils/shell.js';

// Written with an explicit BACKSLASH constant rather than source escapes. The escaping is
// what these tests are about, so a literal that a tool or an editor can quietly reshape
// would let the test agree with a broken implementation.
const BACKSLASH = String.fromCharCode(92);

describe('quoteShellArg', () => {
it('double-quotes on Windows, because cmd.exe does not strip single quotes', () => {
// Measured: `node '<path with a space>'` fails under cmd.exe with MODULE_NOT_FOUND,
// the quotes reaching node as part of the path. Double quotes work in cmd.exe,
// PowerShell and bash alike.
const p = 'C:' + BACKSLASH + 'dir with space' + BACKSLASH + 'e.js';

expect(quoteShellArg(p, 'win32')).toBe('"' + p + '"');
});

it('single-quotes on POSIX, where they suppress every expansion', () => {
expect(quoteShellArg('/home/u/my dir/e.js', 'linux')).toBe("'/home/u/my dir/e.js'");
});

it('closes, escapes and reopens around an embedded single quote on POSIX', () => {
expect(quoteShellArg("/home/u/it's/e.js", 'darwin'))
.toBe("'/home/u/it'" + BACKSLASH + "''s/e.js'");
});

it('quotes a plain value on both, so callers never concatenate bare', () => {
expect(quoteShellArg('n8nac', 'win32')).toBe('"n8nac"');
expect(quoteShellArg('n8nac', 'linux')).toBe("'n8nac'");
});
});
5 changes: 3 additions & 2 deletions packages/cli/tests/unit/sync-manager.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import fs from 'fs';
import { quoteShellArg } from '../../src/utils/shell.js';
import os from 'os';
import path from 'path';
import { describe, it, expect, vi } from 'vitest';
Expand Down Expand Up @@ -74,7 +75,7 @@ describe('SyncManager push filename contract', () => {
const outsidePath = path.join(TMP, 'outside-workflow.workflow.ts');
expect(() => manager.resolvePushTarget(outsidePath)).toThrowError(
expect.objectContaining({
message: expect.stringContaining("Run : n8nac push '")
message: expect.stringContaining('Run : n8nac push ')
})
);
});
Expand Down Expand Up @@ -109,7 +110,7 @@ describe('SyncManager push filename contract', () => {
);
expect(() => manager.resolvePushTarget(path.join(TMP, 'outside workflow.workflow.ts'))).toThrowError(
expect.objectContaining({
message: expect.stringContaining("Run : n8nac push './outside workflow.workflow.ts'")
message: expect.stringContaining(`Run : n8nac push ${quoteShellArg('./outside workflow.workflow.ts')}`)
})
);

Expand Down
9 changes: 8 additions & 1 deletion packages/vscode-extension/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2444,8 +2444,15 @@ function resolveAiContextManagerCommandOverride(context: vscode.ExtensionContext
return `node ${quoteShellArg(siblingManagerCliPath)}`;
}

// Mirrors packages/cli/src/utils/shell.ts. Duplicated on purpose: a three-line pure
// function is not worth widening the n8nac public type surface across a package boundary.
// cmd.exe does not strip POSIX single quotes, so a single-quoted path reaches the program
// with the quotes still attached and fails on the first space. Double quotes are understood
// by cmd.exe, PowerShell and bash alike, and Windows forbids `"` in a path.
function quoteShellArg(value: string): string {
return `'${value.replace(/'/g, `'\\''`)}'`;
return process.platform === 'win32'
? `"${value}"`
Comment thread
coderabbitai[bot] marked this conversation as resolved.
: `'${value.replace(/'/g, `'\\''`)}'`;
}

async function updateAiContextAfterSyncInitialization(
Expand Down
Loading