fix: preflight malformed openshell exec commands

This commit is contained in:
brokemac79
2026-05-24 01:05:37 +01:00
committed by Peter Steinberger
parent f87aa0ff1b
commit aafed830a5
8 changed files with 521 additions and 13 deletions
+6 -6
View File
@@ -20,7 +20,7 @@ import {
import { normalizeLowercaseStringOrEmpty } from "openclaw/plugin-sdk/string-coerce-runtime";
import type { OpenShellSandboxBackend } from "./backend.types.js";
import {
buildExecRemoteCommand,
buildValidatedExecRemoteCommand,
buildRemoteCommand,
createOpenShellSshSession,
runOpenShellCli,
@@ -211,6 +211,11 @@ class OpenShellSandboxBackendImpl {
env: Record<string, string>;
usePty: boolean;
}): Promise<{ argv: string[]; token: PendingExec }> {
const remoteCommand = buildValidatedExecRemoteCommand({
command: params.command,
workdir: params.workdir ?? this.params.remoteWorkspaceDir,
env: params.env,
});
await this.ensureSandboxExists();
if (this.params.execContext.config.mode === "mirror") {
await this.syncWorkspaceToRemote();
@@ -220,11 +225,6 @@ class OpenShellSandboxBackendImpl {
const sshSession = await createOpenShellSshSession({
context: this.params.execContext,
});
const remoteCommand = buildExecRemoteCommand({
command: params.command,
workdir: params.workdir ?? this.params.remoteWorkspaceDir,
env: params.env,
});
return {
argv: [
"ssh",
+5 -1
View File
@@ -6,7 +6,11 @@ import {
} from "openclaw/plugin-sdk/sandbox";
import type { ResolvedOpenShellPluginConfig } from "./config.js";
export { buildExecRemoteCommand, shellEscape } from "openclaw/plugin-sdk/sandbox";
export {
buildExecRemoteCommand,
buildValidatedExecRemoteCommand,
shellEscape,
} from "openclaw/plugin-sdk/sandbox";
export type OpenShellExecContext = {
config: ResolvedOpenShellPluginConfig;
@@ -1,12 +1,19 @@
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { createSandboxTestContext } from "openclaw/plugin-sdk/test-fixtures";
import type { CreateSandboxBackendParams } from "openclaw/plugin-sdk/sandbox";
import {
createSandboxBrowserConfig,
createSandboxPruneConfig,
createSandboxSshConfig,
createSandboxTestContext,
} from "openclaw/plugin-sdk/test-fixtures";
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
import type { OpenShellSandboxBackend } from "./backend.js";
import {
applyGatewayEndpointToSshConfig,
buildExecRemoteCommand,
buildValidatedExecRemoteCommand,
buildOpenShellBaseArgv,
resolveOpenShellCommand,
runOpenShellCli,
@@ -19,6 +26,7 @@ const cliMocks = vi.hoisted(() => ({
}));
let createOpenShellSandboxBackendManager: typeof import("./backend.js").createOpenShellSandboxBackendManager;
let createOpenShellSandboxBackendFactory: typeof import("./backend.js").createOpenShellSandboxBackendFactory;
describe("openshell cli helpers", () => {
const originalEnv = { ...process.env };
@@ -77,6 +85,15 @@ describe("openshell cli helpers", () => {
expect(command).toContain(`'cd '"'"'/sandbox/project'"'"' && pwd && printenv TOKEN'`);
});
it("uses the shared SSH exec command preflight", () => {
expect(() =>
buildValidatedExecRemoteCommand({
command: 'workflow run <workflow-id> "<task>"',
env: {},
}),
).toThrow(/unresolved placeholder token <workflow-id>/);
});
it("passes direct gateway endpoints to openshell commands without registration", async () => {
const calls: string[][] = [];
const openshellCommand = await makeExecutable({
@@ -151,7 +168,8 @@ describe("openshell backend manager", () => {
runOpenShellCli: cliMocks.runOpenShellCli,
};
});
({ createOpenShellSandboxBackendManager } = await import("./backend.js"));
({ createOpenShellSandboxBackendFactory, createOpenShellSandboxBackendManager } =
await import("./backend.js"));
});
afterAll(() => {
@@ -261,10 +279,59 @@ describe("openshell backend manager", () => {
args: ["sandbox", "delete", "openclaw-session-5678"],
});
});
it("rejects malformed exec commands before opening an OpenShell SSH session", async () => {
const factory = createOpenShellSandboxBackendFactory({
pluginConfig: resolveOpenShellPluginConfig({
command: "openshell",
}),
});
const backend = await factory({
sessionKey: "agent:main:turn",
scopeKey: "agent:main",
workspaceDir: "/tmp/workspace",
agentWorkspaceDir: "/tmp/workspace",
cfg: createOpenShellBackendSandboxConfig(),
});
await expect(
backend.buildExecSpec({
command: "workflow install <name>",
env: {},
usePty: false,
}),
).rejects.toThrow(/unresolved placeholder token <name>/);
expect(cliMocks.runOpenShellCli).not.toHaveBeenCalled();
});
});
const tempDirs: string[] = [];
function createOpenShellBackendSandboxConfig(): CreateSandboxBackendParams["cfg"] {
return {
mode: "all",
backend: "openshell",
scope: "session",
workspaceAccess: "rw",
workspaceRoot: "/tmp/openclaw-sandboxes",
docker: {
image: "openclaw-sandbox:bookworm-slim",
containerPrefix: "openclaw-sbx-",
workdir: "/workspace",
readOnlyRoot: false,
tmpfs: [],
network: "none",
capDrop: [],
binds: [],
env: {},
},
ssh: createSandboxSshConfig("/tmp/openclaw-sandboxes"),
browser: createSandboxBrowserConfig(),
tools: { allow: ["*"], deny: [] },
prune: createSandboxPruneConfig(),
};
}
async function makeTempDir(prefix: string) {
const dir = await fs.mkdtemp(path.join(os.tmpdir(), prefix));
tempDirs.push(dir);
+1
View File
@@ -38,6 +38,7 @@ export {
buildExecRemoteCommand,
buildRemoteCommand,
buildSshSandboxArgv,
buildValidatedExecRemoteCommand,
createSshSandboxSessionFromConfigText,
createSshSandboxSessionFromSettings,
disposeSshSandboxSession,
+4 -4
View File
@@ -16,9 +16,9 @@ import {
} from "./remote-fs-bridge.js";
import { sanitizeEnvVars } from "./sanitize-env-vars.js";
import {
buildExecRemoteCommand,
buildRemoteCommand,
buildSshSandboxArgv,
buildValidatedExecRemoteCommand,
createSshSandboxSessionFromSettings,
disposeSshSandboxSession,
runSshSandboxCommand,
@@ -143,13 +143,13 @@ class SshSandboxBackendImpl {
remoteWorkspaceDir: this.params.runtimePaths.remoteWorkspaceDir,
remoteAgentWorkspaceDir: this.params.runtimePaths.remoteAgentWorkspaceDir,
buildExecSpec: async ({ command, workdir, env, usePty }) => {
await this.ensureRuntime();
const sshSession = await this.createSession();
const remoteCommand = buildExecRemoteCommand({
const remoteCommand = buildValidatedExecRemoteCommand({
command,
workdir: workdir ?? this.params.runtimePaths.remoteWorkspaceDir,
env,
});
await this.ensureRuntime();
const sshSession = await this.createSession();
return {
argv: buildSshSandboxArgv({
session: sshSession,
+59
View File
@@ -4,6 +4,7 @@ import path from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import {
buildExecRemoteCommand,
buildValidatedExecRemoteCommand,
createSshSandboxSessionFromSettings,
disposeSshSandboxSession,
type SshSandboxSession,
@@ -110,6 +111,64 @@ describe("sandbox ssh helpers", () => {
expect(command).toContain(`'cd '"'"'/sandbox/project'"'"' && pwd && printenv TOKEN'`);
});
it("keeps the public exec command builder quote-only for compatibility", () => {
const command = buildExecRemoteCommand({
command: "workflow run <workflow-id> --ref main",
env: {},
});
expect(command).toContain(`'/bin/sh'`);
expect(command).toContain(`'workflow run <workflow-id> --ref main'`);
});
it.each([
["workflow install <name>", /unresolved placeholder token <name>/],
["workflow run <workflow-id> --ref main", /unresolved placeholder token <workflow-id>/],
['echo "unterminated', /unclosed double quote/],
["printf '%s", /unclosed single quote/],
["echo foo\\", /trailing backslash escape/],
["echo `date", /unterminated backtick command substitution/],
["echo $(date", /unterminated command substitution/],
["echo $((1 << 2)", /unterminated arithmetic expansion/],
["cat <<EOF", /unterminated here-doc EOF/],
["cat <<EOF\nstill open", /unterminated here-doc EOF/],
])("rejects malformed generated exec commands: %s", (rawCommand, message) => {
expect(() =>
buildValidatedExecRemoteCommand({
command: rawCommand,
env: {},
}),
).toThrow(message);
});
it("allows shell features and quoted placeholder-looking text", () => {
expect(() =>
buildValidatedExecRemoteCommand({
command: [
"cat < input.txt > output.txt",
"cat <in>out",
"cat <input> output",
'cat <input-file> "output file"',
"cat <<'EOF' > literal.txt",
"<workflow-id>",
'"unterminated quote text is data here',
"`unterminated backtick text is data here",
"EOF",
": <<EOF $(printf '%s' hi\n)\nbody\nEOF",
"echo $(cat <<EOF\ninside\nEOF\n)",
"cat <<EOF\r\nwindows line endings\r\nEOF\r\n",
"echo $(printf '%s' ok)",
"echo `date`",
"diff <(sort left.txt) <(sort right.txt)",
"echo $((1 << 2))",
'printf "%s\\n" "<name>"',
"# workflow run <workflow-id>",
].join("\n"),
env: {},
}),
).not.toThrow();
});
it.runIf(process.platform !== "win32")(
"rejects upload trees with symlinks that escape the local workspace",
async () => {
+376
View File
@@ -74,6 +74,194 @@ export function buildRemoteCommand(argv: string[]): string {
return argv.map((entry) => shellEscape(entry)).join(" ");
}
type ExecCommandQuoteState = "plain" | "single" | "double";
type ExecCommandFrame = {
kind: "root" | "command-substitution" | "arithmetic" | "backtick";
quote: ExecCommandQuoteState;
escaping: boolean;
parenDepth: number;
};
type HeredocMarker = {
delimiter: string;
stripLeadingTabs: boolean;
};
type PendingHeredoc = HeredocMarker & {
frameDepth: number;
};
function assertValidExecRemoteCommand(command: string): void {
const frames: ExecCommandFrame[] = [
{ kind: "root", quote: "plain", escaping: false, parenDepth: 0 },
];
const pendingHeredocs: PendingHeredoc[] = [];
for (let index = 0; index < command.length; index += 1) {
const frame = frames.at(-1);
if (!frame) {
throw new Error("Malformed SSH/OpenShell exec command: parser state underflow.");
}
const char = command[index];
if (frame.escaping) {
frame.escaping = false;
continue;
}
if (frame.quote === "single") {
if (char === "'") {
frame.quote = "plain";
}
continue;
}
if (char === "\\") {
frame.escaping = true;
continue;
}
if (frame.quote === "double") {
if (char === '"') {
frame.quote = "plain";
continue;
}
if (char === "`") {
frames.push(createExecCommandFrame("backtick"));
continue;
}
if (char === "$" && command[index + 1] === "(" && command[index + 2] === "(") {
frames.push(createExecCommandFrame("arithmetic", 2));
index += 2;
continue;
}
if (char === "$" && command[index + 1] === "(") {
frames.push(createExecCommandFrame("command-substitution", 1));
index += 1;
}
continue;
}
if (frame.kind === "arithmetic") {
if (char === "(") {
frame.parenDepth += 1;
continue;
}
if (char === ")") {
frame.parenDepth -= 1;
if (frame.parenDepth === 0) {
frames.pop();
}
}
continue;
}
if (char === "\n") {
const frameHeredocs = pendingHeredocs.filter(
(pending) => pending.frameDepth === frames.length,
);
if (frameHeredocs.length > 0) {
index = skipHeredocBodies(command, index + 1, frameHeredocs) - 1;
for (const pending of frameHeredocs) {
pendingHeredocs.splice(pendingHeredocs.indexOf(pending), 1);
}
continue;
}
}
if (frame.kind === "backtick" && char === "`") {
frames.pop();
continue;
}
if (char === "'") {
frame.quote = "single";
continue;
}
if (char === '"') {
frame.quote = "double";
continue;
}
if (char === "`") {
frames.push(createExecCommandFrame("backtick"));
continue;
}
if (char === "$" && command[index + 1] === "(" && command[index + 2] === "(") {
frames.push(createExecCommandFrame("arithmetic", 2));
index += 2;
continue;
}
if (char === "$" && command[index + 1] === "(") {
frames.push(createExecCommandFrame("command-substitution", 1));
index += 1;
continue;
}
if (char === "#" && isShellCommentStart(command, index)) {
index = skipShellComment(command, index) - 1;
continue;
}
if (char === "<") {
const heredoc = readHeredoc(command, index);
if (heredoc) {
pendingHeredocs.push({
...heredoc.pending,
frameDepth: frames.length,
});
index = heredoc.endIndex - 1;
continue;
}
const placeholder = readPlaceholderToken(command, index);
if (placeholder) {
throw new Error(
`Malformed SSH/OpenShell exec command: unresolved placeholder token ${placeholder}.`,
);
}
}
if (frame.kind === "command-substitution") {
if (char === "(") {
frame.parenDepth += 1;
continue;
}
if (char === ")") {
frame.parenDepth -= 1;
if (frame.parenDepth === 0) {
frames.pop();
}
}
}
}
const openFrame = frames.at(-1);
if (openFrame?.escaping) {
throw new Error("Malformed SSH/OpenShell exec command: trailing backslash escape.");
}
if (pendingHeredocs.length > 0) {
throw new Error(
`Malformed SSH/OpenShell exec command: unterminated here-doc ${pendingHeredocs[0].delimiter}.`,
);
}
for (let index = frames.length - 1; index >= 0; index -= 1) {
const frame = frames[index];
if (frame.quote === "single") {
throw new Error("Malformed SSH/OpenShell exec command: unclosed single quote.");
}
if (frame.quote === "double") {
throw new Error("Malformed SSH/OpenShell exec command: unclosed double quote.");
}
if (frame.kind === "backtick") {
throw new Error(
"Malformed SSH/OpenShell exec command: unterminated backtick command substitution.",
);
}
if (frame.kind === "command-substitution") {
throw new Error("Malformed SSH/OpenShell exec command: unterminated command substitution.");
}
if (frame.kind === "arithmetic") {
throw new Error("Malformed SSH/OpenShell exec command: unterminated arithmetic expansion.");
}
}
}
export function buildExecRemoteCommand(params: {
command: string;
workdir?: string;
@@ -95,6 +283,194 @@ export function buildExecRemoteCommand(params: {
return buildRemoteCommand(argv);
}
export function buildValidatedExecRemoteCommand(params: {
command: string;
workdir?: string;
env: Record<string, string>;
}): string {
assertValidExecRemoteCommand(params.command);
return buildExecRemoteCommand(params);
}
function createExecCommandFrame(kind: ExecCommandFrame["kind"], parenDepth = 0): ExecCommandFrame {
return { kind, quote: "plain", escaping: false, parenDepth };
}
function readPlaceholderToken(command: string, index: number): string | null {
const match = /^<[A-Za-z][A-Za-z0-9_-]*>/.exec(command.slice(index));
if (!match) {
return null;
}
if (isLikelyGeneratedWorkflowPlaceholder(command, index)) {
return match[0];
}
const next = command[index + match[0].length];
if (next === undefined || /[\r\n;&|)]/.test(next)) {
return match[0];
}
if (next === " " || next === "\t") {
return hasRedirectionTargetAfter(command, index + match[0].length) ? null : match[0];
}
return null;
}
function hasRedirectionTargetAfter(command: string, index: number): boolean {
let cursor = index;
while (command[cursor] === " " || command[cursor] === "\t") {
cursor += 1;
}
return command[cursor] !== undefined && !/[;&|()<>\r\n]/.test(command[cursor]);
}
function isLikelyGeneratedWorkflowPlaceholder(command: string, index: number): boolean {
const prefix = command.slice(0, index);
const segmentStart =
Math.max(
prefix.lastIndexOf("\n"),
prefix.lastIndexOf(";"),
prefix.lastIndexOf("&"),
prefix.lastIndexOf("|"),
) + 1;
const currentCommand = prefix.slice(segmentStart).trim();
return /^workflow(?:\s+[A-Za-z0-9._/-]+)*$/.test(currentCommand);
}
function readHeredoc(
command: string,
index: number,
): { pending: HeredocMarker; endIndex: number } | null {
if (command[index + 1] !== "<" || command[index + 2] === "<") {
return null;
}
let cursor = index + 2;
const stripLeadingTabs = command[cursor] === "-";
if (stripLeadingTabs) {
cursor += 1;
}
while (command[cursor] === " " || command[cursor] === "\t") {
cursor += 1;
}
const delimiter = readHeredocDelimiter(command, cursor);
if (!delimiter) {
throw new Error("Malformed SSH/OpenShell exec command: missing here-doc delimiter.");
}
return {
pending: { delimiter: delimiter.value, stripLeadingTabs },
endIndex: delimiter.endIndex,
};
}
function readHeredocDelimiter(
command: string,
index: number,
): { value: string; endIndex: number } | null {
let cursor = index;
let delimiter = "";
let quote: ExecCommandQuoteState = "plain";
let escaping = false;
while (cursor < command.length) {
const char = command[cursor];
if (escaping) {
delimiter += char;
escaping = false;
cursor += 1;
continue;
}
if (quote === "single") {
if (char === "'") {
quote = "plain";
} else {
delimiter += char;
}
cursor += 1;
continue;
}
if (quote === "double") {
if (char === '"') {
quote = "plain";
} else if (char === "\\") {
escaping = true;
} else {
delimiter += char;
}
cursor += 1;
continue;
}
if (char === "\\") {
escaping = true;
cursor += 1;
continue;
}
if (char === "'") {
quote = "single";
cursor += 1;
continue;
}
if (char === '"') {
quote = "double";
cursor += 1;
continue;
}
if (isHeredocDelimiterTerminator(char)) {
break;
}
delimiter += char;
cursor += 1;
}
if (quote !== "plain" || escaping) {
throw new Error("Malformed SSH/OpenShell exec command: unterminated here-doc delimiter.");
}
return delimiter ? { value: delimiter, endIndex: cursor } : null;
}
function isHeredocDelimiterTerminator(char: string | undefined): boolean {
return (
char === undefined || /\s/.test(char) || [";", "&", "|", "(", ")", "<", ">"].includes(char)
);
}
function skipHeredocBodies(
command: string,
index: number,
pendingHeredocs: PendingHeredoc[],
): number {
let cursor = index;
for (const pending of pendingHeredocs) {
let found = false;
while (cursor <= command.length) {
const lineEnd = command.indexOf("\n", cursor);
const endIndex = lineEnd === -1 ? command.length : lineEnd;
const rawLine = command.slice(cursor, endIndex);
const normalizedLine = rawLine.endsWith("\r") ? rawLine.slice(0, -1) : rawLine;
const line = pending.stripLeadingTabs ? normalizedLine.replace(/^\t+/, "") : normalizedLine;
cursor = lineEnd === -1 ? command.length : lineEnd + 1;
if (line === pending.delimiter) {
found = true;
break;
}
if (lineEnd === -1) {
break;
}
}
if (!found) {
throw new Error(
`Malformed SSH/OpenShell exec command: unterminated here-doc ${pending.delimiter}.`,
);
}
}
return cursor;
}
function isShellCommentStart(command: string, index: number): boolean {
const previous = command[index - 1];
return previous === undefined || /[\s;&|()]/.test(previous);
}
function skipShellComment(command: string, index: number): number {
const newlineIndex = command.indexOf("\n", index);
return newlineIndex === -1 ? command.length : newlineIndex;
}
export function buildSshSandboxArgv(params: {
session: SshSandboxSession;
remoteCommand: string;
+1
View File
@@ -25,6 +25,7 @@ export {
buildExecRemoteCommand,
buildRemoteCommand,
buildSshSandboxArgv,
buildValidatedExecRemoteCommand,
createRemoteShellSandboxFsBridge,
createWritableRenameTargetResolver,
createSshSandboxSessionFromConfigText,