Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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: 2 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ deployment. Treat them as untrusted:
- Validate names with `src/common/validation.ts` before they reach SSH hosts,
paths, or the CLI.
- In SSH `ProxyCommand` (the one shell context), build flags with
`getGlobalShellFlags`, which validates the URL and quotes it with
`escapeShellArg`.
`getGlobalShellFlags`, which validates the URL and quotes values with
`escapeProxyCommandArg` (open-remote-ssh splits it without a shell).

## Code style

Expand Down
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@
generally available. Existing values remain effective until the new setting is
explicitly set in user settings. OAuth remains disabled by default.

### Fixed

- Fix "Premature close" since v1.17.0 in Positron and other editors that use
the open source Remote - SSH extension (`open-remote-ssh`): the SSH
`ProxyCommand` single-quoted `--url` and `--global-config`, which these
editors pass to the CLI, and `--token=` merged with the next argument.
- `coder.headerCommand` now works with `open-remote-ssh` unless it contains `$`,
a backtick, `\`, `%` or a line break.
- On Windows, a `coder.headerCommand` containing `"` was split into several
arguments before it reached the CLI.

## [v1.17.0](https://github.com/coder/vscode-coder/releases/tag/v1.17.0) 2026-10-13

### Added
Expand Down
28 changes: 23 additions & 5 deletions src/common/strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,14 @@ export function lowercase<T extends string>(value: T): Lowercase<T> {
* Not a universal shell-escape: `$VAR` / `$(...)` / `%VAR%` still expand
* inside `"..."`. For untrusted values use {@link escapeShellArg}.
*
* A leading, trailing or doubled `=` is quoted too: open-remote-ssh's
* ssh-config@5 treats an unquoted `=` as a separator.
*
* @see https://docs.python.org/3/library/shlex.html#shlex.quote
* @see https://learn.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way
*/
export function escapeCommandArg(arg: string): string {
if (arg !== "" && /^[\w@%+,=:./-]+$/.test(arg)) {
if (/^[\w@%+,:./-]+(?:=[\w@%+,:./-]+)*$/.test(arg)) {
return arg;
}
return `"${arg.replaceAll('"', String.raw`\"`)}"`;
Expand All @@ -28,14 +31,29 @@ export function escapeCommandArg(arg: string): string {
/**
* Cross-platform shell quoting that blocks variable expansion. Use for
* values from outside the user's local settings (e.g. server-controlled).
* The win32 branch targets Windows OpenSSH, which runs `ProxyCommand`
* without a shell and turns `%%` back into `%`. A shell such as cmd.exe or
* Git Bash's `sh` still expands inside those quotes.
* The win32 branch targets Windows OpenSSH, which starts `ProxyCommand`
* without a shell: `%%` becomes `%` and the CLI splits the line with the
* Windows argv rules (`\"`). Git Bash's `sh` still expands `$` inside.
*/
export function escapeShellArg(arg: string): string {
if (os.platform() === "win32") {
const escaped = arg.replace(/"/g, '""').replace(/%/g, "%%");
// `\\+` consumes backslash runs not before a quote, keeping this linear.
const escaped = arg
.replace(/(\\*)("|$)|\\+/g, (run, slashes?: string, quote?: string) =>
slashes === undefined
? run
: `${slashes}${slashes}${quote ? '\\"' : ""}`,
)
.replace(/%/g, "%%");
return `"${escaped}"`;
}
return `'${arg.replace(/'/g, "'\\''")}'`;
}

/**
* Double quotes read the same in a shell and in open-remote-ssh, which splits
* `ProxyCommand` itself, unless the value needs escaping there.
*/
export function escapeProxyCommandArg(arg: string): string {
return /[$`\\%\r\n]/.test(arg) ? escapeShellArg(arg) : escapeCommandArg(arg);
}
6 changes: 3 additions & 3 deletions src/settings/cli.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import os from "node:os";

import { expandPath } from "../common/fs";
import { escapeCommandArg, escapeShellArg } from "../common/strings";
import { escapeCommandArg, escapeProxyCommandArg } from "../common/strings";
import { validateDeploymentUrl } from "../common/validation";

import { getHeaderArgs } from "./headers";
Expand Down Expand Up @@ -51,8 +51,8 @@ export function getGlobalShellFlags(
// covers URLs that reach storage some other way.
validateDeploymentUrl(auth.url);
return [
"--token=",
...buildGlobalFlags(configs, auth, escapeCommandArg, escapeShellArg),
escapeCommandArg("--token="),
...buildGlobalFlags(configs, auth, escapeCommandArg, escapeProxyCommandArg),
];
}

Expand Down
2 changes: 1 addition & 1 deletion src/settings/headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export function getHeaderCommand(
return cmd || undefined;
}

/** Returns `--header-command` args; shell callers pass `escapeShellArg` as `esc`. */
/** Returns `--header-command` args, quoted with `esc`. */
export function getHeaderArgs(
config: Pick<WorkspaceConfiguration, "get">,
esc: (s: string) => string = (s) => s,
Expand Down
41 changes: 31 additions & 10 deletions test/unit/common/strings.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import os from "node:os";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";

import { escapeCommandArg, escapeShellArg } from "@/common/strings";
import {
escapeCommandArg,
escapeProxyCommandArg,
escapeShellArg,
} from "@/common/strings";

describe("escapeCommandArg", () => {
it("returns simple strings unquoted", () => {
Expand All @@ -17,6 +21,13 @@ describe("escapeCommandArg", () => {
expect(escapeCommandArg("")).toBe('""');
});

it.each([["--token="], ["=x"], ["x==y"]])(
"quotes a leading, trailing or doubled `=`: %j",
(input) => {
expect(escapeCommandArg(input)).toBe(`"${input}"`);
},
);

it("escapes double quotes and wraps in quotes", () => {
expect(escapeCommandArg('say "hello"')).toBe(String.raw`"say \"hello\""`);
});
Expand Down Expand Up @@ -85,20 +96,30 @@ describe("escapeShellArg", () => {
expect(escapeShellArg("env=dev")).toBe('"env=dev"');
});

it('doubles embedded `"`', () => {
expect(escapeShellArg('regions=["us","eu"]')).toBe(
'"regions=[""us"",""eu""]"',
);
it.each([
['regions=["us","eu"]', String.raw`"regions=[\"us\",\"eu\"]"`],
['C:\\a b\\ \\"x\\', '"C:\\a b\\ \\\\\\"x\\\\"'],
])('escapes `"` and the backslashes before it in %j', (value, expected) => {
expect(escapeShellArg(value)).toBe(expected);
});

it("doubles `%` so OpenSSH token expansion keeps it literal", () => {
expect(escapeShellArg("%PATH%")).toBe('"%%PATH%%"');
});
});
});

it("keeps cmd metachars inside the quoted region", () => {
expect(escapeShellArg('foo" & calc.exe & "x')).toBe(
'"foo"" & calc.exe & ""x"',
);
});
describe("escapeProxyCommandArg", () => {
it.each([
["--token=", escapeCommandArg],
['say "hi"', escapeCommandArg],
["get-headers --profile 'prod' | jq", escapeCommandArg],
["$TOKEN", escapeShellArg],
["`whoami`", escapeShellArg],
[String.raw`a\b`, escapeShellArg],
["%h", escapeShellArg],
["line\nbreak", escapeShellArg],
])("quotes %j", (value, escape) => {
expect(escapeProxyCommandArg(value)).toBe(escape(value));
});
});
57 changes: 55 additions & 2 deletions test/unit/remote/remote.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Api } from "coder/site/src/api/api";
import { vol } from "memfs";
import { spawnSync } from "node:child_process";
import * as semver from "semver";
import { beforeEach, describe, expect, it, vi } from "vitest";
import * as vscode from "vscode";
Expand Down Expand Up @@ -28,7 +29,7 @@ import {
MockUserInteraction,
useEditor,
} from "../../mocks/testHelpers";
import { quoteCommand } from "../../utils/platform";
import { isWindows } from "../../utils/platform";

import type { CliManager } from "@/cli/cliManager";
import type { Commands } from "@/commands";
Expand Down Expand Up @@ -395,9 +396,61 @@ describe("Remote", () => {
});

expect(elideNetworkInfoDir(proxyCommand)).toBe(
`/mock/coder --token= --global-config ${quoteCommand("/mock/global")} --url ${quoteCommand("https://coder.example.com")} ${expected}`,
`/mock/coder "--token=" --global-config /mock/global --url https://coder.example.com ${expected}`,
);
});

// OpenSSH runs the command with `sh -c` on Unix; Win32-OpenSSH passes it
// to CreateProcessW, so the program's argv parser splits it.
it("reaches the CLI unchanged through this platform's OpenSSH", () => {
const configDir = "/Users/me/Library/Application Support/coder";
const headerCommand = 'echo "X: $TOKEN" "C:\\a b\\" `id`';
const pathResolver = new PathResolver("/mock/global", "/mock/log");
new MockConfigurationProvider().set("coder.headerCommand", headerCommand);
const proxyCommand = buildSshProxyCommand({
pathResolver,
binaryPath: "coder",
cliAuth: { ...CLI_AUTH, configDir },
logArgs: [],
sessionApp: "vscode",
hostPrefix: "coder-vscode.coder.example.com--",
});

// Swap the CLI for Node printing the arguments it receives.
const printArgv = proxyCommand.replace(
/^coder /,
`-p "JSON.stringify(process.argv.slice(1))" -- `,
);
const result = isWindows()
? spawnSync(process.execPath, [printArgv], {
argv0: "node",
encoding: "utf8",
windowsVerbatimArguments: true,
})
: spawnSync("sh", ["-c", `"$NODE" ${printArgv}`], {
encoding: "utf8",
env: { ...process.env, NODE: process.execPath },
});

expect(JSON.parse(result.stdout)).toStrictEqual([
"--token=",
"--global-config",
configDir,
"--url",
CLI_AUTH.url,
"--header-command",
headerCommand,
"ssh",
"--disable-autostart",
"--stdio",
"--usage-app=vscode",
"--network-info-dir",
pathResolver.getNetworkInfoPath(),
"--ssh-host-prefix",
"coder-vscode.coder.example.com--",
"%h",
]);
});
});
});

Expand Down
Loading
Loading