diff --git a/extensions/openshell/src/backend.ts b/extensions/openshell/src/backend.ts index 4c4805e30a4f..c4713eaa8c96 100644 --- a/extensions/openshell/src/backend.ts +++ b/extensions/openshell/src/backend.ts @@ -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; 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", diff --git a/extensions/openshell/src/cli.ts b/extensions/openshell/src/cli.ts index 653d70b14314..41fe3097af5b 100644 --- a/extensions/openshell/src/cli.ts +++ b/extensions/openshell/src/cli.ts @@ -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; diff --git a/extensions/openshell/src/openshell-core.test.ts b/extensions/openshell/src/openshell-core.test.ts index 55bc599884cc..09bc43877b53 100644 --- a/extensions/openshell/src/openshell-core.test.ts +++ b/extensions/openshell/src/openshell-core.test.ts @@ -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 ""', + env: {}, + }), + ).toThrow(/unresolved placeholder token /); + }); + 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 ", + env: {}, + usePty: false, + }), + ).rejects.toThrow(/unresolved placeholder token /); + 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); diff --git a/src/agents/sandbox.ts b/src/agents/sandbox.ts index 6085cdcf3285..7b0c01ff9ced 100644 --- a/src/agents/sandbox.ts +++ b/src/agents/sandbox.ts @@ -38,6 +38,7 @@ export { buildExecRemoteCommand, buildRemoteCommand, buildSshSandboxArgv, + buildValidatedExecRemoteCommand, createSshSandboxSessionFromConfigText, createSshSandboxSessionFromSettings, disposeSshSandboxSession, diff --git a/src/agents/sandbox/ssh-backend.ts b/src/agents/sandbox/ssh-backend.ts index f2fa34c9a16e..d3908f6e59ee 100644 --- a/src/agents/sandbox/ssh-backend.ts +++ b/src/agents/sandbox/ssh-backend.ts @@ -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, diff --git a/src/agents/sandbox/ssh.test.ts b/src/agents/sandbox/ssh.test.ts index 1c8b644a792d..c2c8178952c0 100644 --- a/src/agents/sandbox/ssh.test.ts +++ b/src/agents/sandbox/ssh.test.ts @@ -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 --ref main", + env: {}, + }); + + expect(command).toContain(`'/bin/sh'`); + expect(command).toContain(`'workflow run --ref main'`); + }); + + it.each([ + ["workflow install ", /unresolved placeholder token /], + ["workflow run --ref main", /unresolved placeholder token /], + ['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 < { + 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 out", + "cat output", + 'cat "output file"', + "cat <<'EOF' > literal.txt", + "", + '"unterminated quote text is data here', + "`unterminated backtick text is data here", + "EOF", + ": <"', + "# workflow run ", + ].join("\n"), + env: {}, + }), + ).not.toThrow(); + }); + it.runIf(process.platform !== "win32")( "rejects upload trees with symlinks that escape the local workspace", async () => { diff --git a/src/agents/sandbox/ssh.ts b/src/agents/sandbox/ssh.ts index b4e33868b382..df8f97b3c661 100644 --- a/src/agents/sandbox/ssh.ts +++ b/src/agents/sandbox/ssh.ts @@ -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 { + 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; diff --git a/src/plugin-sdk/sandbox.ts b/src/plugin-sdk/sandbox.ts index 3ca2694a502c..23204e8658e3 100644 --- a/src/plugin-sdk/sandbox.ts +++ b/src/plugin-sdk/sandbox.ts @@ -25,6 +25,7 @@ export { buildExecRemoteCommand, buildRemoteCommand, buildSshSandboxArgv, + buildValidatedExecRemoteCommand, createRemoteShellSandboxFsBridge, createWritableRenameTargetResolver, createSshSandboxSessionFromConfigText,