mirror of
https://github.com/OpenRouterTeam/spawn.git
synced 2026-05-22 11:24:18 +00:00
fix(security): propagate path normalization to all cloud modules (#2693)
* fix(security): propagate path normalization to all cloud upload/download functions PR #2690 added normalize() before path traversal checks in AWS but not the other clouds. Apply the same defense-in-depth to GCP, DigitalOcean, Hetzner, Sprite, and shared validateRemotePath. Agent: code-health Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix(security): use normalized path in all file transfer operations Addresses code review: replace original remotePath with normalizedRemote in scp commands and bash operations to prevent validation bypass. - digitalocean: use normalizedRemote in uploadFile scp and derive expandedPath from normalizedRemote in downloadFile - hetzner: same pattern for uploadFile/downloadFile - gcp: derive expandedPath from normalizedRemote.replace(...) in both uploadFile and downloadFile - sprite: use normalizedRemote in bash mkdir/mv command and derive expandedPath from normalizedRemote in downloadFile Agent: pr-maintainer Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(security): close validation bypass in agent-setup and AWS file ops validateRemotePath() validated the normalized path but returned void, so the caller still used the original unsanitized remotePath in shell commands — bypassing the normalization check entirely. Fix: return the normalized path and use it in all file operations. Also fix AWS uploadFile/downloadFile which validated normalizedRemote but used the original remotePath in scp commands. Agent: pr-maintainer Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- 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:
A
GitHub
parent
bae921a295
commit
644593eaea
7 changed files with 57 additions and 45 deletions
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "@openrouter/spawn",
|
||||
"version": "0.20.4",
|
||||
"version": "0.20.5",
|
||||
"type": "module",
|
||||
"bin": {
|
||||
"spawn": "cli.js"
|
||||
|
|
|
|||
|
|
@ -1145,7 +1145,7 @@ export async function uploadFile(localPath: string, remotePath: string): Promise
|
|||
...SSH_BASE_OPTS,
|
||||
...keyOpts,
|
||||
localPath,
|
||||
`${SSH_USER}@${_state.instanceIp}:${remotePath}`,
|
||||
`${SSH_USER}@${_state.instanceIp}:${normalizedRemote}`,
|
||||
],
|
||||
{
|
||||
stdio: [
|
||||
|
|
@ -1176,7 +1176,7 @@ export async function downloadFile(remotePath: string, localPath: string): Promi
|
|||
throw new Error(`Invalid remote path: ${remotePath}`);
|
||||
}
|
||||
const keyOpts = getSshKeyOpts(await ensureSshKeys());
|
||||
const expandedPath = remotePath.replace(/^\$HOME/, "~");
|
||||
const expandedPath = normalizedRemote.replace(/^\$HOME/, "~");
|
||||
const proc = Bun.spawn(
|
||||
[
|
||||
"scp",
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import type { CloudInstance, VMConnection } from "../history.js";
|
|||
import type { CloudInitTier } from "../shared/agents";
|
||||
|
||||
import { mkdirSync, readFileSync } from "node:fs";
|
||||
import { normalize } from "node:path";
|
||||
import * as p from "@clack/prompts";
|
||||
import { getErrorMessage, isNumber, isString, toObjectArray, toRecord } from "@openrouter/spawn-shared";
|
||||
import { handleBillingError, isBillingError, showNonBillingError } from "../shared/billing-guidance";
|
||||
|
|
@ -1307,10 +1308,11 @@ export async function runServer(cmd: string, timeoutSecs?: number, ip?: string):
|
|||
|
||||
export async function uploadFile(localPath: string, remotePath: string, ip?: string): Promise<void> {
|
||||
const serverIp = ip || _state.serverIp;
|
||||
const normalizedRemote = normalize(remotePath);
|
||||
if (
|
||||
!/^[a-zA-Z0-9/_.~-]+$/.test(remotePath) ||
|
||||
remotePath.includes("..") ||
|
||||
remotePath.split("/").some((s) => s.startsWith("-"))
|
||||
!/^[a-zA-Z0-9/_.~-]+$/.test(normalizedRemote) ||
|
||||
normalizedRemote.includes("..") ||
|
||||
normalizedRemote.split("/").some((s) => s.startsWith("-"))
|
||||
) {
|
||||
logError(`Invalid remote path: ${remotePath}`);
|
||||
throw new Error("Invalid remote path");
|
||||
|
|
@ -1323,7 +1325,7 @@ export async function uploadFile(localPath: string, remotePath: string, ip?: str
|
|||
...SSH_BASE_OPTS,
|
||||
...keyOpts,
|
||||
localPath,
|
||||
`root@${serverIp}:${remotePath}`,
|
||||
`root@${serverIp}:${normalizedRemote}`,
|
||||
],
|
||||
{
|
||||
stdio: [
|
||||
|
|
@ -1346,17 +1348,18 @@ export async function uploadFile(localPath: string, remotePath: string, ip?: str
|
|||
|
||||
export async function downloadFile(remotePath: string, localPath: string, ip?: string): Promise<void> {
|
||||
const serverIp = ip || _state.serverIp;
|
||||
const normalizedRemote = normalize(remotePath);
|
||||
if (
|
||||
!/^[a-zA-Z0-9/_.~$-]+$/.test(remotePath) ||
|
||||
remotePath.includes("..") ||
|
||||
remotePath.split("/").some((s) => s.startsWith("-"))
|
||||
!/^[a-zA-Z0-9/_.~$-]+$/.test(normalizedRemote) ||
|
||||
normalizedRemote.includes("..") ||
|
||||
normalizedRemote.split("/").some((s) => s.startsWith("-"))
|
||||
) {
|
||||
logError(`Invalid remote path: ${remotePath}`);
|
||||
throw new Error("Invalid remote path");
|
||||
}
|
||||
|
||||
const keyOpts = getSshKeyOpts(await ensureSshKeys());
|
||||
const expandedPath = remotePath.replace(/^\$HOME/, "~");
|
||||
const expandedPath = normalizedRemote.replace(/^\$HOME/, "~");
|
||||
|
||||
const proc = Bun.spawn(
|
||||
[
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ import type { CloudInstance, VMConnection } from "../history.js";
|
|||
import type { CloudInitTier } from "../shared/agents";
|
||||
|
||||
import { existsSync, readFileSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { join, normalize } from "node:path";
|
||||
import { isString, toObjectArray } from "@openrouter/spawn-shared";
|
||||
import { handleBillingError, isBillingError, showNonBillingError } from "../shared/billing-guidance";
|
||||
import { getPackagesForTier, NODE_INSTALL_CMD, needsBun, needsNode } from "../shared/cloud-init";
|
||||
|
|
@ -997,17 +997,18 @@ export async function uploadFile(localPath: string, remotePath: string): Promise
|
|||
logError(`Invalid local path: ${localPath}`);
|
||||
throw new Error("Invalid local path");
|
||||
}
|
||||
const normalizedRemote = normalize(remotePath);
|
||||
if (
|
||||
!/^[a-zA-Z0-9/_.~$-]+$/.test(remotePath) ||
|
||||
remotePath.includes("..") ||
|
||||
remotePath.split("/").some((s) => s.startsWith("-"))
|
||||
!/^[a-zA-Z0-9/_.~$-]+$/.test(normalizedRemote) ||
|
||||
normalizedRemote.includes("..") ||
|
||||
normalizedRemote.split("/").some((s) => s.startsWith("-"))
|
||||
) {
|
||||
logError(`Invalid remote path: ${remotePath}`);
|
||||
throw new Error("Invalid remote path");
|
||||
}
|
||||
const username = resolveUsername();
|
||||
// Expand $HOME on remote side
|
||||
const expandedPath = remotePath.replace(/^\$HOME/, "~");
|
||||
const expandedPath = normalizedRemote.replace(/^\$HOME/, "~");
|
||||
const keyOpts = getSshKeyOpts(await ensureSshKeys());
|
||||
|
||||
const proc = Bun.spawn(
|
||||
|
|
@ -1043,16 +1044,17 @@ export async function downloadFile(remotePath: string, localPath: string): Promi
|
|||
logError(`Invalid local path: ${localPath}`);
|
||||
throw new Error("Invalid local path");
|
||||
}
|
||||
const normalizedRemote = normalize(remotePath);
|
||||
if (
|
||||
!/^[a-zA-Z0-9/_.~$-]+$/.test(remotePath) ||
|
||||
remotePath.includes("..") ||
|
||||
remotePath.split("/").some((s) => s.startsWith("-"))
|
||||
!/^[a-zA-Z0-9/_.~$-]+$/.test(normalizedRemote) ||
|
||||
normalizedRemote.includes("..") ||
|
||||
normalizedRemote.split("/").some((s) => s.startsWith("-"))
|
||||
) {
|
||||
logError(`Invalid remote path: ${remotePath}`);
|
||||
throw new Error("Invalid remote path");
|
||||
}
|
||||
const username = resolveUsername();
|
||||
const expandedPath = remotePath.replace(/^\$HOME/, "~");
|
||||
const expandedPath = normalizedRemote.replace(/^\$HOME/, "~");
|
||||
const keyOpts = getSshKeyOpts(await ensureSshKeys());
|
||||
|
||||
const proc = Bun.spawn(
|
||||
|
|
|
|||
|
|
@ -4,6 +4,7 @@ import type { CloudInstance, VMConnection } from "../history.js";
|
|||
import type { CloudInitTier } from "../shared/agents";
|
||||
|
||||
import { mkdirSync, readFileSync } from "node:fs";
|
||||
import { normalize } from "node:path";
|
||||
import { getErrorMessage, isNumber, isString, toObjectArray, toRecord } from "@openrouter/spawn-shared";
|
||||
import { handleBillingError, isBillingError, showNonBillingError } from "../shared/billing-guidance";
|
||||
import { getPackagesForTier, NODE_INSTALL_CMD, needsBun, needsNode } from "../shared/cloud-init";
|
||||
|
|
@ -615,10 +616,11 @@ export async function runServer(cmd: string, timeoutSecs?: number, ip?: string):
|
|||
|
||||
export async function uploadFile(localPath: string, remotePath: string, ip?: string): Promise<void> {
|
||||
const serverIp = ip || _state.serverIp;
|
||||
const normalizedRemote = normalize(remotePath);
|
||||
if (
|
||||
!/^[a-zA-Z0-9/_.~-]+$/.test(remotePath) ||
|
||||
remotePath.includes("..") ||
|
||||
remotePath.split("/").some((s) => s.startsWith("-"))
|
||||
!/^[a-zA-Z0-9/_.~-]+$/.test(normalizedRemote) ||
|
||||
normalizedRemote.includes("..") ||
|
||||
normalizedRemote.split("/").some((s) => s.startsWith("-"))
|
||||
) {
|
||||
logError(`Invalid remote path: ${remotePath}`);
|
||||
throw new Error("Invalid remote path");
|
||||
|
|
@ -632,7 +634,7 @@ export async function uploadFile(localPath: string, remotePath: string, ip?: str
|
|||
...SSH_BASE_OPTS,
|
||||
...keyOpts,
|
||||
localPath,
|
||||
`root@${serverIp}:${remotePath}`,
|
||||
`root@${serverIp}:${normalizedRemote}`,
|
||||
],
|
||||
{
|
||||
stdio: [
|
||||
|
|
@ -655,17 +657,18 @@ export async function uploadFile(localPath: string, remotePath: string, ip?: str
|
|||
|
||||
export async function downloadFile(remotePath: string, localPath: string, ip?: string): Promise<void> {
|
||||
const serverIp = ip || _state.serverIp;
|
||||
const normalizedRemote = normalize(remotePath);
|
||||
if (
|
||||
!/^[a-zA-Z0-9/_.~$-]+$/.test(remotePath) ||
|
||||
remotePath.includes("..") ||
|
||||
remotePath.split("/").some((s) => s.startsWith("-"))
|
||||
!/^[a-zA-Z0-9/_.~$-]+$/.test(normalizedRemote) ||
|
||||
normalizedRemote.includes("..") ||
|
||||
normalizedRemote.split("/").some((s) => s.startsWith("-"))
|
||||
) {
|
||||
logError(`Invalid remote path: ${remotePath}`);
|
||||
throw new Error("Invalid remote path");
|
||||
}
|
||||
|
||||
const keyOpts = getSshKeyOpts(await ensureSshKeys());
|
||||
const expandedPath = remotePath.replace(/^\$HOME/, "~");
|
||||
const expandedPath = normalizedRemote.replace(/^\$HOME/, "~");
|
||||
|
||||
const proc = Bun.spawn(
|
||||
[
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ import type { AgentConfig } from "./agents";
|
|||
import type { Result } from "./ui";
|
||||
|
||||
import { unlinkSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { join, normalize } from "node:path";
|
||||
import { getErrorMessage } from "@openrouter/spawn-shared";
|
||||
import { getTmpDir } from "./paths";
|
||||
import { asyncTryCatch, asyncTryCatchIf, isOperationalError, tryCatchIf } from "./result.js";
|
||||
|
|
@ -64,24 +64,26 @@ async function installAgent(
|
|||
* Allows shell variable references ($HOME, ${HOME}) but rejects anything
|
||||
* that could break out of double-quoted shell interpolation.
|
||||
*/
|
||||
function validateRemotePath(remotePath: string): void {
|
||||
function validateRemotePath(remotePath: string): string {
|
||||
// Allow alphanumerics, forward slashes, dots, underscores, tildes, hyphens,
|
||||
// and shell variable syntax ($, {, }). Reject everything else — especially
|
||||
// backticks, semicolons, pipes, quotes, newlines, and null bytes.
|
||||
if (!/^[\w/.~${}:-]+$/.test(remotePath)) {
|
||||
const normalizedRemote = normalize(remotePath);
|
||||
if (!/^[\w/.~${}:-]+$/.test(normalizedRemote)) {
|
||||
throw new Error(`uploadConfigFile: remotePath contains unsafe characters: ${remotePath}`);
|
||||
}
|
||||
// Block path traversal
|
||||
if (remotePath.includes("..")) {
|
||||
// Block path traversal (normalize resolves . segments first)
|
||||
if (normalizedRemote.includes("..")) {
|
||||
throw new Error(`uploadConfigFile: remotePath must not contain "..": ${remotePath}`);
|
||||
}
|
||||
return normalizedRemote;
|
||||
}
|
||||
|
||||
/**
|
||||
* Upload a config file to the remote machine via a temp file and mv.
|
||||
*/
|
||||
async function uploadConfigFile(runner: CloudRunner, content: string, remotePath: string): Promise<void> {
|
||||
validateRemotePath(remotePath);
|
||||
const safePath = validateRemotePath(remotePath);
|
||||
|
||||
const tmpFile = join(getTmpDir(), `spawn_config_${Date.now()}_${Math.random().toString(36).slice(2)}`);
|
||||
writeFileSync(tmpFile, content, {
|
||||
|
|
@ -97,7 +99,7 @@ async function uploadConfigFile(runner: CloudRunner, content: string, remotePath
|
|||
const tempRemote = `/tmp/spawn_config_${Date.now()}`;
|
||||
await runner.uploadFile(tmpFile, tempRemote);
|
||||
await runner.runServer(
|
||||
`mkdir -p $(dirname "${remotePath}") && chmod 600 ${shellQuote(tempRemote)} && mv ${shellQuote(tempRemote)} "${remotePath}"`,
|
||||
`mkdir -p $(dirname "${safePath}") && chmod 600 ${shellQuote(tempRemote)} && mv ${shellQuote(tempRemote)} "${safePath}"`,
|
||||
);
|
||||
})(),
|
||||
),
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@
|
|||
import type { VMConnection } from "../history.js";
|
||||
|
||||
import { existsSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { join, normalize } from "node:path";
|
||||
import { getErrorMessage } from "@openrouter/spawn-shared";
|
||||
import { getUserHome } from "../shared/paths";
|
||||
import { asyncTryCatch } from "../shared/result.js";
|
||||
|
|
@ -506,10 +506,11 @@ async function runSpriteSilent(cmd: string): Promise<void> {
|
|||
* The -file flag format is "localpath:remotepath".
|
||||
*/
|
||||
export async function uploadFileSprite(localPath: string, remotePath: string): Promise<void> {
|
||||
const normalizedRemote = normalize(remotePath);
|
||||
if (
|
||||
!/^[a-zA-Z0-9/_.~-]+$/.test(remotePath) ||
|
||||
remotePath.includes("..") ||
|
||||
remotePath.split("/").some((s) => s.startsWith("-"))
|
||||
!/^[a-zA-Z0-9/_.~-]+$/.test(normalizedRemote) ||
|
||||
normalizedRemote.includes("..") ||
|
||||
normalizedRemote.split("/").some((s) => s.startsWith("-"))
|
||||
) {
|
||||
logError(`Invalid remote path: ${remotePath}`);
|
||||
throw new Error("Invalid remote path");
|
||||
|
|
@ -518,7 +519,7 @@ export async function uploadFileSprite(localPath: string, remotePath: string): P
|
|||
const spriteCmd = getSpriteCmd()!;
|
||||
// Generate a random temp path on remote to prevent symlink attacks
|
||||
const tempRandom = crypto.randomUUID().replace(/-/g, "").slice(0, 16);
|
||||
const basename = remotePath.split("/").pop() || "file";
|
||||
const basename = normalizedRemote.split("/").pop() || "file";
|
||||
const tempRemote = `/tmp/sprite_upload_${basename}_${tempRandom}`;
|
||||
|
||||
await spriteRetry("sprite upload", async () => {
|
||||
|
|
@ -534,7 +535,7 @@ export async function uploadFileSprite(localPath: string, remotePath: string): P
|
|||
"--",
|
||||
"bash",
|
||||
"-c",
|
||||
`mkdir -p $(dirname '${remotePath}') && mv '${tempRemote}' '${remotePath}'`,
|
||||
`mkdir -p $(dirname '${normalizedRemote}') && mv '${tempRemote}' '${normalizedRemote}'`,
|
||||
],
|
||||
{
|
||||
stdio: [
|
||||
|
|
@ -555,17 +556,18 @@ export async function uploadFileSprite(localPath: string, remotePath: string): P
|
|||
|
||||
/** Download a file from the remote sprite by catting it to stdout. */
|
||||
export async function downloadFileSprite(remotePath: string, localPath: string): Promise<void> {
|
||||
const normalizedRemote = normalize(remotePath);
|
||||
if (
|
||||
!/^[a-zA-Z0-9/_.~$-]+$/.test(remotePath) ||
|
||||
remotePath.includes("..") ||
|
||||
remotePath.split("/").some((s) => s.startsWith("-"))
|
||||
!/^[a-zA-Z0-9/_.~$-]+$/.test(normalizedRemote) ||
|
||||
normalizedRemote.includes("..") ||
|
||||
normalizedRemote.split("/").some((s) => s.startsWith("-"))
|
||||
) {
|
||||
logError(`Invalid remote path: ${remotePath}`);
|
||||
throw new Error("Invalid remote path");
|
||||
}
|
||||
|
||||
const spriteCmd = getSpriteCmd()!;
|
||||
const expandedPath = remotePath.replace(/^\$HOME/, "~");
|
||||
const expandedPath = normalizedRemote.replace(/^\$HOME/, "~");
|
||||
|
||||
await spriteRetry("sprite download", async () => {
|
||||
const proc = Bun.spawn(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue