mirror of https://github.com/openclaw/openclaw.git
Sandbox: sanitize SSH subprocess env (#57848)
* Sandbox: sanitize SSH subprocess env * Sandbox: add sanitize env undefined test
This commit is contained in:
parent
f0af186726
commit
cfe1445953
File diff suppressed because one or more lines are too long
File diff suppressed because one or more lines are too long
|
|
@ -0,0 +1,29 @@
|
|||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { buildOpenShellSshExecEnv } from "./backend.js";
|
||||
|
||||
describe("openshell backend env", () => {
|
||||
const originalEnv = { ...process.env };
|
||||
|
||||
afterEach(() => {
|
||||
for (const key of Object.keys(process.env)) {
|
||||
if (!(key in originalEnv)) {
|
||||
delete process.env[key];
|
||||
}
|
||||
}
|
||||
Object.assign(process.env, originalEnv);
|
||||
});
|
||||
|
||||
it("filters blocked secrets from ssh exec env", () => {
|
||||
process.env.OPENAI_API_KEY = "sk-test-secret";
|
||||
process.env.ANTHROPIC_API_KEY = "sk-ant-test-secret";
|
||||
process.env.LANG = "en_US.UTF-8";
|
||||
process.env.NODE_ENV = "test";
|
||||
|
||||
const env = buildOpenShellSshExecEnv();
|
||||
|
||||
expect(env.OPENAI_API_KEY).toBeUndefined();
|
||||
expect(env.ANTHROPIC_API_KEY).toBeUndefined();
|
||||
expect(env.LANG).toBe("en_US.UTF-8");
|
||||
expect(env.NODE_ENV).toBe("test");
|
||||
});
|
||||
});
|
||||
|
|
@ -17,6 +17,7 @@ import {
|
|||
disposeSshSandboxSession,
|
||||
resolvePreferredOpenClawTmpDir,
|
||||
runSshSandboxCommand,
|
||||
sanitizeEnvVars,
|
||||
} from "openclaw/plugin-sdk/sandbox";
|
||||
import {
|
||||
buildExecRemoteCommand,
|
||||
|
|
@ -41,6 +42,10 @@ type PendingExec = {
|
|||
sshSession: SshSandboxSession;
|
||||
};
|
||||
|
||||
export function buildOpenShellSshExecEnv(): NodeJS.ProcessEnv {
|
||||
return sanitizeEnvVars(process.env).allowed;
|
||||
}
|
||||
|
||||
export type OpenShellSandboxBackend = SandboxBackendHandle &
|
||||
RemoteShellSandboxHandle & {
|
||||
mode: "mirror" | "remote";
|
||||
|
|
@ -123,7 +128,7 @@ async function createOpenShellSandboxBackend(params: {
|
|||
const pending = await impl.prepareExec({ command, workdir, env, usePty });
|
||||
return {
|
||||
argv: pending.argv,
|
||||
env: process.env,
|
||||
env: buildOpenShellSshExecEnv(),
|
||||
stdinMode: "pipe-open",
|
||||
finalizeToken: pending.token,
|
||||
};
|
||||
|
|
@ -180,7 +185,7 @@ class OpenShellSandboxBackendImpl {
|
|||
const pending = await self.prepareExec({ command, workdir, env, usePty });
|
||||
return {
|
||||
argv: pending.argv,
|
||||
env: process.env,
|
||||
env: buildOpenShellSshExecEnv(),
|
||||
stdinMode: "pipe-open",
|
||||
finalizeToken: pending.token,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -45,6 +45,7 @@ export {
|
|||
shellEscape,
|
||||
uploadDirectoryToSshTarget,
|
||||
} from "./sandbox/ssh.js";
|
||||
export { sanitizeEnvVars } from "./sandbox/sanitize-env-vars.js";
|
||||
export { createRemoteShellSandboxFsBridge } from "./sandbox/remote-fs-bridge.js";
|
||||
export { createWritableRenameTargetResolver } from "./sandbox/fs-bridge-rename-targets.js";
|
||||
export { resolveWritableRenameTargets } from "./sandbox/fs-bridge-rename-targets.js";
|
||||
|
|
|
|||
|
|
@ -54,4 +54,15 @@ describe("sanitizeEnvVars", () => {
|
|||
expect(result.allowed).toEqual({ NODE_ENV: "test" });
|
||||
expect(result.blocked).toEqual(["FOO"]);
|
||||
});
|
||||
|
||||
it("skips undefined values when sanitizing process-style env maps", () => {
|
||||
const result = sanitizeEnvVars({
|
||||
NODE_ENV: "test",
|
||||
OPTIONAL_SECRET: undefined,
|
||||
OPENAI_API_KEY: undefined,
|
||||
});
|
||||
|
||||
expect(result.allowed).toEqual({ NODE_ENV: "test" });
|
||||
expect(result.blocked).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -60,7 +60,7 @@ function matchesAnyPattern(value: string, patterns: readonly RegExp[]): boolean
|
|||
}
|
||||
|
||||
export function sanitizeEnvVars(
|
||||
envVars: Record<string, string>,
|
||||
envVars: Record<string, string | undefined>,
|
||||
options: EnvSanitizationOptions = {},
|
||||
): EnvVarSanitizationResult {
|
||||
const allowed: Record<string, string> = {};
|
||||
|
|
@ -72,7 +72,7 @@ export function sanitizeEnvVars(
|
|||
|
||||
for (const [rawKey, value] of Object.entries(envVars)) {
|
||||
const key = rawKey.trim();
|
||||
if (!key) {
|
||||
if (!key || value === undefined) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -135,6 +135,8 @@ async function expectBackendCreationToReject(params: {
|
|||
}
|
||||
|
||||
describe("ssh sandbox backend", () => {
|
||||
const originalEnv = { ...process.env };
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.clearAllMocks();
|
||||
sshMocks.createSshSandboxSessionFromSettings.mockResolvedValue(createSession());
|
||||
|
|
@ -157,6 +159,12 @@ describe("ssh sandbox backend", () => {
|
|||
});
|
||||
|
||||
afterEach(() => {
|
||||
for (const key of Object.keys(process.env)) {
|
||||
if (!(key in originalEnv)) {
|
||||
delete process.env[key];
|
||||
}
|
||||
}
|
||||
Object.assign(process.env, originalEnv);
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
|
|
@ -316,6 +324,29 @@ describe("ssh sandbox backend", () => {
|
|||
expect(sshMocks.disposeSshSandboxSession).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("filters blocked secrets from exec subprocess env", async () => {
|
||||
process.env.OPENAI_API_KEY = "sk-test-secret";
|
||||
process.env.LANG = "en_US.UTF-8";
|
||||
const backend = await createSshSandboxBackend({
|
||||
sessionKey: "agent:worker:task",
|
||||
scopeKey: "agent:worker",
|
||||
workspaceDir: "/tmp/workspace",
|
||||
agentWorkspaceDir: "/tmp/agent",
|
||||
cfg: createBackendSandboxConfig({
|
||||
target: "peter@example.com:2222",
|
||||
}),
|
||||
});
|
||||
|
||||
const execSpec = await backend.buildExecSpec({
|
||||
command: "pwd",
|
||||
env: {},
|
||||
usePty: false,
|
||||
});
|
||||
|
||||
expect(execSpec.env?.OPENAI_API_KEY).toBeUndefined();
|
||||
expect(execSpec.env?.LANG).toBe("en_US.UTF-8");
|
||||
});
|
||||
|
||||
it("rejects docker binds and missing ssh target", async () => {
|
||||
await expectBackendCreationToReject({
|
||||
binds: ["/tmp:/tmp:rw"],
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ import {
|
|||
createRemoteShellSandboxFsBridge,
|
||||
type RemoteShellSandboxHandle,
|
||||
} from "./remote-fs-bridge.js";
|
||||
import { sanitizeEnvVars } from "./sanitize-env-vars.js";
|
||||
import {
|
||||
buildExecRemoteCommand,
|
||||
buildRemoteCommand,
|
||||
|
|
@ -152,7 +153,7 @@ class SshSandboxBackendImpl {
|
|||
remoteCommand,
|
||||
tty: usePty,
|
||||
}),
|
||||
env: process.env,
|
||||
env: sanitizeEnvVars(process.env).allowed,
|
||||
stdinMode: "pipe-open",
|
||||
finalizeToken: { sshSession } satisfies PendingExec,
|
||||
};
|
||||
|
|
|
|||
|
|
@ -0,0 +1,123 @@
|
|||
import type { ChildProcess, SpawnOptions } from "node:child_process";
|
||||
import { EventEmitter } from "node:events";
|
||||
import { PassThrough } from "node:stream";
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const spawnMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
type MockChildProcess = EventEmitter & {
|
||||
stdin: PassThrough;
|
||||
stdout: PassThrough;
|
||||
stderr: PassThrough;
|
||||
kill: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
|
||||
function createMockChildProcess(): MockChildProcess {
|
||||
const child = new EventEmitter() as MockChildProcess;
|
||||
child.stdin = new PassThrough();
|
||||
child.stdout = new PassThrough();
|
||||
child.stderr = new PassThrough();
|
||||
child.kill = vi.fn();
|
||||
return child;
|
||||
}
|
||||
|
||||
vi.mock("node:child_process", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("node:child_process")>();
|
||||
return {
|
||||
...actual,
|
||||
spawn: spawnMock,
|
||||
};
|
||||
});
|
||||
|
||||
let runSshSandboxCommand: typeof import("./ssh.js").runSshSandboxCommand;
|
||||
let uploadDirectoryToSshTarget: typeof import("./ssh.js").uploadDirectoryToSshTarget;
|
||||
|
||||
beforeAll(async () => {
|
||||
({ runSshSandboxCommand, uploadDirectoryToSshTarget } = await import("./ssh.js"));
|
||||
});
|
||||
|
||||
describe("ssh subprocess env sanitization", () => {
|
||||
const originalEnv = { ...process.env };
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
for (const key of Object.keys(process.env)) {
|
||||
if (!(key in originalEnv)) {
|
||||
delete process.env[key];
|
||||
}
|
||||
}
|
||||
Object.assign(process.env, originalEnv);
|
||||
});
|
||||
|
||||
it("filters blocked secrets before spawning ssh commands", async () => {
|
||||
spawnMock.mockImplementationOnce(
|
||||
(_command: string, _args: readonly string[], _options: SpawnOptions): ChildProcess => {
|
||||
const child = createMockChildProcess();
|
||||
process.nextTick(() => {
|
||||
child.emit("close", 0);
|
||||
});
|
||||
return child as unknown as ChildProcess;
|
||||
},
|
||||
);
|
||||
|
||||
process.env.OPENAI_API_KEY = "sk-test-secret";
|
||||
process.env.LANG = "en_US.UTF-8";
|
||||
|
||||
await runSshSandboxCommand({
|
||||
session: {
|
||||
command: "ssh",
|
||||
configPath: "/tmp/openclaw-test-ssh-config",
|
||||
host: "openclaw-sandbox",
|
||||
},
|
||||
remoteCommand: "true",
|
||||
});
|
||||
|
||||
const spawnOptions = spawnMock.mock.calls[0]?.[2] as SpawnOptions | undefined;
|
||||
const env = spawnOptions?.env;
|
||||
expect(env?.OPENAI_API_KEY).toBeUndefined();
|
||||
expect(env?.LANG).toBe("en_US.UTF-8");
|
||||
});
|
||||
|
||||
it("filters blocked secrets before spawning ssh uploads", async () => {
|
||||
spawnMock
|
||||
.mockImplementationOnce(
|
||||
(_command: string, _args: readonly string[], _options: SpawnOptions): ChildProcess => {
|
||||
const child = createMockChildProcess();
|
||||
process.nextTick(() => {
|
||||
child.emit("close", 0);
|
||||
});
|
||||
return child as unknown as ChildProcess;
|
||||
},
|
||||
)
|
||||
.mockImplementationOnce(
|
||||
(_command: string, _args: readonly string[], _options: SpawnOptions): ChildProcess => {
|
||||
const child = createMockChildProcess();
|
||||
process.nextTick(() => {
|
||||
child.emit("close", 0);
|
||||
});
|
||||
return child as unknown as ChildProcess;
|
||||
},
|
||||
);
|
||||
|
||||
process.env.ANTHROPIC_API_KEY = "sk-test-secret";
|
||||
process.env.NODE_ENV = "test";
|
||||
|
||||
await uploadDirectoryToSshTarget({
|
||||
session: {
|
||||
command: "ssh",
|
||||
configPath: "/tmp/openclaw-test-ssh-config",
|
||||
host: "openclaw-sandbox",
|
||||
},
|
||||
localDir: "/tmp/workspace",
|
||||
remoteDir: "/remote/workspace",
|
||||
});
|
||||
|
||||
const sshSpawnOptions = spawnMock.mock.calls[1]?.[2] as SpawnOptions | undefined;
|
||||
const env = sshSpawnOptions?.env;
|
||||
expect(env?.ANTHROPIC_API_KEY).toBeUndefined();
|
||||
expect(env?.NODE_ENV).toBe("test");
|
||||
});
|
||||
});
|
||||
|
|
@ -6,6 +6,7 @@ import { parseSshTarget } from "../../infra/ssh-tunnel.js";
|
|||
import { resolvePreferredOpenClawTmpDir } from "../../infra/tmp-openclaw-dir.js";
|
||||
import { resolveUserPath } from "../../utils.js";
|
||||
import type { SandboxBackendCommandResult } from "./backend.js";
|
||||
import { sanitizeEnvVars } from "./sanitize-env-vars.js";
|
||||
|
||||
export type SshSandboxSettings = {
|
||||
command: string;
|
||||
|
|
@ -212,10 +213,11 @@ export async function runSshSandboxCommand(
|
|||
remoteCommand: params.remoteCommand,
|
||||
tty: params.tty,
|
||||
});
|
||||
const sshEnv = sanitizeEnvVars(process.env).allowed;
|
||||
return await new Promise<SandboxBackendCommandResult>((resolve, reject) => {
|
||||
const child = spawn(argv[0], argv.slice(1), {
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
env: process.env,
|
||||
env: sshEnv,
|
||||
signal: params.signal,
|
||||
});
|
||||
const stdoutChunks: Buffer[] = [];
|
||||
|
|
@ -266,6 +268,7 @@ export async function uploadDirectoryToSshTarget(params: {
|
|||
session: params.session,
|
||||
remoteCommand,
|
||||
});
|
||||
const sshEnv = sanitizeEnvVars(process.env).allowed;
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
const tar = spawn("tar", ["-C", params.localDir, "-cf", "-", "."], {
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
|
|
@ -273,7 +276,7 @@ export async function uploadDirectoryToSshTarget(params: {
|
|||
});
|
||||
const ssh = spawn(sshArgv[0], sshArgv.slice(1), {
|
||||
stdio: ["pipe", "pipe", "pipe"],
|
||||
env: process.env,
|
||||
env: sshEnv,
|
||||
signal: params.signal,
|
||||
});
|
||||
const tarStderr: Buffer[] = [];
|
||||
|
|
|
|||
|
|
@ -37,6 +37,7 @@ export {
|
|||
resolveWritableRenameTargets,
|
||||
resolveWritableRenameTargetsForBridge,
|
||||
runSshSandboxCommand,
|
||||
sanitizeEnvVars,
|
||||
shellEscape,
|
||||
uploadDirectoryToSshTarget,
|
||||
} from "../agents/sandbox.js";
|
||||
|
|
|
|||
Loading…
Reference in New Issue