mirror of
https://github.com/OpenRouterTeam/spawn.git
synced 2026-04-28 03:49:31 +00:00
security: consolidate shellQuote across all clouds (defense-in-depth) (#2535)
PR #2533 hardened GCP with shellQuote() and null-byte rejection, but left Hetzner, DigitalOcean, AWS, and connect.ts using inline .replace(/'/g, "'\\''") without null-byte validation. - Move shellQuote to shared/ui.ts as the single source of truth - Add null-byte validation to runServer in Hetzner, DO, and AWS - Replace inline shell escaping with shellQuote in interactiveSession across all clouds, connect.ts, and agents.ts buildEnvBlock - Re-export shellQuote from gcp.ts for backwards compatibility Agent: security-auditor Co-authored-by: B <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
parent
58a2d3bf18
commit
dfd08ad48c
9 changed files with 53 additions and 32 deletions
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@openrouter/spawn",
|
||||
"version": "0.16.17",
|
||||
"version": "0.16.18",
|
||||
"type": "module",
|
||||
"bin": {
|
||||
"spawn": "cli.js"
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { describe, expect, it } from "bun:test";
|
||||
import { shellQuote } from "../gcp/gcp.js";
|
||||
import { shellQuote } from "../shared/ui.js";
|
||||
|
||||
describe("shellQuote", () => {
|
||||
it("should wrap simple strings in single quotes", () => {
|
||||
|
|
|
|||
|
|
@ -34,6 +34,7 @@ import {
|
|||
promptSpawnNameShared,
|
||||
sanitizeTermValue,
|
||||
selectFromList,
|
||||
shellQuote,
|
||||
validateRegionName,
|
||||
} from "../shared/ui";
|
||||
|
||||
|
|
@ -1052,6 +1053,9 @@ export async function waitForCloudInit(maxAttempts = 60): Promise<void> {
|
|||
}
|
||||
|
||||
export async function runServer(cmd: string, timeoutSecs?: number): Promise<void> {
|
||||
if (!cmd || /\0/.test(cmd)) {
|
||||
throw new Error("Invalid command: must be non-empty and must not contain null bytes");
|
||||
}
|
||||
const fullCmd = `export PATH="$HOME/.npm-global/bin:$HOME/.claude/local/bin:$HOME/.local/bin:$HOME/.bun/bin:$PATH" && ${cmd}`;
|
||||
const keyOpts = getSshKeyOpts(await ensureSshKeys());
|
||||
const proc = Bun.spawn(
|
||||
|
|
@ -1060,7 +1064,7 @@ export async function runServer(cmd: string, timeoutSecs?: number): Promise<void
|
|||
...SSH_BASE_OPTS,
|
||||
...keyOpts,
|
||||
`${SSH_USER}@${_state.instanceIp}`,
|
||||
`bash -c '${fullCmd.replace(/'/g, "'\\''")}'`,
|
||||
`bash -c ${shellQuote(fullCmd)}`,
|
||||
],
|
||||
{
|
||||
stdio: [
|
||||
|
|
@ -1119,12 +1123,11 @@ export async function uploadFile(localPath: string, remotePath: string): Promise
|
|||
}
|
||||
|
||||
export async function interactiveSession(cmd: string): Promise<number> {
|
||||
if (!cmd || /\0/.test(cmd)) {
|
||||
throw new Error("Invalid command: must be non-empty and must not contain null bytes");
|
||||
}
|
||||
const term = sanitizeTermValue(process.env.TERM || "xterm-256color");
|
||||
// Single-quote escaping prevents premature shell expansion of $variables in cmd
|
||||
const shellEscapedCmd = cmd.replace(/'/g, "'\\''");
|
||||
// Pass command directly to SSH (no outer bash -c wrapper) — matches Hetzner/DO behavior.
|
||||
// The extra bash -c layer added latency and an unnecessary shell process.
|
||||
const fullCmd = `export TERM=${term} PATH="$HOME/.npm-global/bin:$HOME/.claude/local/bin:$HOME/.local/bin:$HOME/.bun/bin:$PATH" && exec bash -l -c '${shellEscapedCmd}'`;
|
||||
const fullCmd = `export TERM=${term} PATH="$HOME/.npm-global/bin:$HOME/.claude/local/bin:$HOME/.local/bin:$HOME/.bun/bin:$PATH" && exec bash -l -c ${shellQuote(cmd)}`;
|
||||
const keyOpts = getSshKeyOpts(await ensureSshKeys());
|
||||
const exitCode = spawnInteractive([
|
||||
"ssh",
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ import { getHistoryPath } from "../shared/paths.js";
|
|||
import { tryCatch } from "../shared/result.js";
|
||||
import { SSH_INTERACTIVE_OPTS, spawnInteractive } from "../shared/ssh.js";
|
||||
import { ensureSshKeys, getSshKeyOpts } from "../shared/ssh-keys.js";
|
||||
import { shellQuote } from "../shared/ui.js";
|
||||
import { getErrorMessage } from "./shared.js";
|
||||
|
||||
/** Execute a shell command and resolve/reject on process close/error */
|
||||
|
|
@ -180,7 +181,7 @@ export async function cmdEnterAgent(
|
|||
|
||||
// Standard SSH connection with agent launch
|
||||
p.log.step(`Entering ${pc.bold(agentName)} on ${pc.bold(connection.ip)}...`);
|
||||
const escapedRemoteCmd = remoteCmd.replace(/'/g, "'\\''");
|
||||
const quotedRemoteCmd = shellQuote(remoteCmd);
|
||||
const keyOpts = getSshKeyOpts(await ensureSshKeys());
|
||||
return runInteractiveCommand(
|
||||
"ssh",
|
||||
|
|
@ -189,9 +190,9 @@ export async function cmdEnterAgent(
|
|||
...keyOpts,
|
||||
`${connection.user}@${connection.ip}`,
|
||||
"--",
|
||||
`bash -lc '${escapedRemoteCmd}'`,
|
||||
`bash -lc ${quotedRemoteCmd}`,
|
||||
],
|
||||
`Failed to enter ${agentName}`,
|
||||
`ssh -t ${connection.user}@${connection.ip} -- bash -lc '${escapedRemoteCmd}'`,
|
||||
`ssh -t ${connection.user}@${connection.ip} -- bash -lc ${quotedRemoteCmd}`,
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -42,6 +42,7 @@ import {
|
|||
prompt,
|
||||
sanitizeTermValue,
|
||||
selectFromList,
|
||||
shellQuote,
|
||||
toKebabCase,
|
||||
validateRegionName,
|
||||
validateServerName,
|
||||
|
|
@ -1155,6 +1156,9 @@ export async function waitForCloudInit(ip?: string, maxAttempts = 60): Promise<v
|
|||
}
|
||||
|
||||
export async function runServer(cmd: string, timeoutSecs?: number, ip?: string): Promise<void> {
|
||||
if (!cmd || /\0/.test(cmd)) {
|
||||
throw new Error("Invalid command: must be non-empty and must not contain null bytes");
|
||||
}
|
||||
const serverIp = ip || _state.serverIp;
|
||||
const fullCmd = `export PATH="$HOME/.npm-global/bin:$HOME/.claude/local/bin:$HOME/.local/bin:$HOME/.bun/bin:$PATH" && ${cmd}`;
|
||||
const keyOpts = getSshKeyOpts(await ensureSshKeys());
|
||||
|
|
@ -1228,11 +1232,12 @@ export async function uploadFile(localPath: string, remotePath: string, ip?: str
|
|||
}
|
||||
|
||||
export async function interactiveSession(cmd: string, ip?: string): Promise<number> {
|
||||
if (!cmd || /\0/.test(cmd)) {
|
||||
throw new Error("Invalid command: must be non-empty and must not contain null bytes");
|
||||
}
|
||||
const serverIp = ip || _state.serverIp;
|
||||
const term = sanitizeTermValue(process.env.TERM || "xterm-256color");
|
||||
// Single-quote escaping prevents premature shell expansion of $variables in cmd
|
||||
const shellEscapedCmd = cmd.replace(/'/g, "'\\''");
|
||||
const fullCmd = `export TERM=${term} PATH="$HOME/.npm-global/bin:$HOME/.claude/local/bin:$HOME/.local/bin:$HOME/.bun/bin:$PATH" && exec bash -l -c '${shellEscapedCmd}'`;
|
||||
const fullCmd = `export TERM=${term} PATH="$HOME/.npm-global/bin:$HOME/.claude/local/bin:$HOME/.local/bin:$HOME/.bun/bin:$PATH" && exec bash -l -c ${shellQuote(cmd)}`;
|
||||
const keyOpts = getSshKeyOpts(await ensureSshKeys());
|
||||
|
||||
const exitCode = spawnInteractive([
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ import {
|
|||
promptSpawnNameShared,
|
||||
sanitizeTermValue,
|
||||
selectFromList,
|
||||
shellQuote,
|
||||
} from "../shared/ui";
|
||||
|
||||
const DASHBOARD_URL = "https://console.cloud.google.com/compute/instances";
|
||||
|
|
@ -1083,14 +1084,5 @@ export async function destroyInstance(name?: string): Promise<void> {
|
|||
|
||||
// ─── Shell Quoting ──────────────────────────────────────────────────────────
|
||||
|
||||
/** POSIX single-quote escaping: wraps `s` in single quotes and escapes any
|
||||
* embedded single quotes with the standard `'\''` technique.
|
||||
*
|
||||
* Defense-in-depth: rejects null bytes which could truncate the string at
|
||||
* the C/OS level even though callers already validate for them. */
|
||||
export function shellQuote(s: string): string {
|
||||
if (/\0/.test(s)) {
|
||||
throw new Error("shellQuote: input must not contain null bytes");
|
||||
}
|
||||
return "'" + s.replace(/'/g, "'\\''") + "'";
|
||||
}
|
||||
// shellQuote is now imported from shared/ui.ts and re-exported for backwards compat
|
||||
export { shellQuote } from "../shared/ui";
|
||||
|
|
|
|||
|
|
@ -33,6 +33,7 @@ import {
|
|||
promptSpawnNameShared,
|
||||
sanitizeTermValue,
|
||||
selectFromList,
|
||||
shellQuote,
|
||||
validateRegionName,
|
||||
} from "../shared/ui";
|
||||
|
||||
|
|
@ -576,6 +577,9 @@ export async function waitForCloudInit(ip?: string, maxAttempts = 60): Promise<v
|
|||
}
|
||||
|
||||
export async function runServer(cmd: string, timeoutSecs?: number, ip?: string): Promise<void> {
|
||||
if (!cmd || /\0/.test(cmd)) {
|
||||
throw new Error("Invalid command: must be non-empty and must not contain null bytes");
|
||||
}
|
||||
const serverIp = ip || _state.serverIp;
|
||||
const fullCmd = `export PATH="$HOME/.npm-global/bin:$HOME/.claude/local/bin:$HOME/.local/bin:$HOME/.bun/bin:$PATH" && ${cmd}`;
|
||||
const keyOpts = getSshKeyOpts(await ensureSshKeys());
|
||||
|
|
@ -650,11 +654,12 @@ export async function uploadFile(localPath: string, remotePath: string, ip?: str
|
|||
}
|
||||
|
||||
export async function interactiveSession(cmd: string, ip?: string): Promise<number> {
|
||||
if (!cmd || /\0/.test(cmd)) {
|
||||
throw new Error("Invalid command: must be non-empty and must not contain null bytes");
|
||||
}
|
||||
const serverIp = ip || _state.serverIp;
|
||||
const term = sanitizeTermValue(process.env.TERM || "xterm-256color");
|
||||
// Single-quote escaping prevents premature shell expansion of $variables in cmd
|
||||
const shellEscapedCmd = cmd.replace(/'/g, "'\\''");
|
||||
const fullCmd = `export TERM=${term} PATH="$HOME/.npm-global/bin:$HOME/.claude/local/bin:$HOME/.local/bin:$HOME/.bun/bin:$PATH" && exec bash -l -c '${shellEscapedCmd}'`;
|
||||
const fullCmd = `export TERM=${term} PATH="$HOME/.npm-global/bin:$HOME/.claude/local/bin:$HOME/.local/bin:$HOME/.bun/bin:$PATH" && exec bash -l -c ${shellQuote(cmd)}`;
|
||||
|
||||
const keyOpts = getSshKeyOpts(await ensureSshKeys());
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
// shared/agents.ts — AgentConfig interface + shared helpers (cloud-agnostic)
|
||||
|
||||
import { logError } from "./ui";
|
||||
import { logError, shellQuote } from "./ui";
|
||||
|
||||
// ─── Types ───────────────────────────────────────────────────────────────────
|
||||
|
||||
|
|
@ -109,9 +109,12 @@ export function generateEnvConfig(pairs: string[]): string {
|
|||
logError(`SECURITY: Invalid environment variable name rejected: ${key}`);
|
||||
continue;
|
||||
}
|
||||
// Escape single quotes in value
|
||||
const escaped = value.replace(/'/g, "'\\''");
|
||||
lines.push(`export ${key}='${escaped}'`);
|
||||
// Reject null bytes in value (defense-in-depth)
|
||||
if (/\0/.test(value)) {
|
||||
logError(`SECURITY: Null byte in environment variable value rejected: ${key}`);
|
||||
continue;
|
||||
}
|
||||
lines.push(`export ${key}=${shellQuote(value)}`);
|
||||
}
|
||||
return lines.join("\n") + "\n";
|
||||
}
|
||||
|
|
|
|||
|
|
@ -253,6 +253,18 @@ export function loadApiToken(cloud: string): string | null {
|
|||
);
|
||||
}
|
||||
|
||||
/** POSIX single-quote escaping: wraps `s` in single quotes and escapes any
|
||||
* embedded single quotes with the standard `'\''` technique.
|
||||
*
|
||||
* Defense-in-depth: rejects null bytes which could truncate the string at
|
||||
* the C/OS level even though callers already validate for them. */
|
||||
export function shellQuote(s: string): string {
|
||||
if (/\0/.test(s)) {
|
||||
throw new Error("shellQuote: input must not contain null bytes");
|
||||
}
|
||||
return "'" + s.replace(/'/g, "'\\''") + "'";
|
||||
}
|
||||
|
||||
/** JSON-escape a string (returns the quoted JSON string). */
|
||||
export function jsonEscape(s: string): string {
|
||||
return JSON.stringify(s);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue