Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 26 additions & 13 deletions lib/server/gateway.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const path = require("path");
const { spawn, execSync } = require("child_process");
const { spawn, execSync, execFileSync } = require("child_process");
const fs = require("fs");
const net = require("net");
const {
Expand Down Expand Up @@ -709,8 +709,21 @@ const syncChannelConfig = (savedVars, mode = "all") => {
if (ch === "slack") {
const appToken = savedMap[def.extraEnvKeys?.[0]];
if (!appToken) continue;
execSync(
`openclaw channels add --channel slack --bot-token "${token}" --app-token "${appToken}"`,
// execFileSync passes token/appToken as argv entries, never through a
// shell, so a value containing `"`, `$`, or backticks can't break out
// of the command (unlike the old execSync + string-interpolation form).
execFileSync(
"openclaw",
[
"channels",
"add",
"--channel",
"slack",
"--bot-token",
token,
"--app-token",
appToken,
],
{ env, timeout: 15000, encoding: "utf8" },
);
let raw = fs.readFileSync(configPath, "utf8");
Expand All @@ -722,11 +735,11 @@ const syncChannelConfig = (savedVars, mode = "all") => {
}
fs.writeFileSync(configPath, raw);
} else {
execSync(`openclaw channels add --channel ${ch} --token "${token}"`, {
env,
timeout: 15000,
encoding: "utf8",
});
execFileSync(
"openclaw",
["channels", "add", "--channel", ch, "--token", token],
{ env, timeout: 15000, encoding: "utf8" },
);
const raw = fs.readFileSync(configPath, "utf8");
if (raw.includes(token)) {
fs.writeFileSync(
Expand All @@ -748,11 +761,11 @@ const syncChannelConfig = (savedVars, mode = "all") => {
) {
console.log(`[alphaclaw] Removing channel: ${ch}`);
try {
execSync(`openclaw channels remove --channel ${ch} --delete`, {
env,
timeout: 15000,
encoding: "utf8",
});
execFileSync(
"openclaw",
["channels", "remove", "--channel", ch, "--delete"],
{ env, timeout: 15000, encoding: "utf8" },
);
console.log(`[alphaclaw] Channel ${ch} removed`);
} catch (e) {
console.error(
Expand Down
72 changes: 72 additions & 0 deletions tests/server/gateway.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const kAlphaclawConfigPath = path.join(OPENCLAW_DIR, "alphaclaw.json");
const modulePath = require.resolve("../../lib/server/gateway");
const originalSpawn = childProcess.spawn;
const originalExecSync = childProcess.execSync;
const originalExecFileSync = childProcess.execFileSync;
const originalExistsSync = fs.existsSync;
const originalMkdirSync = fs.mkdirSync;
const originalReaddirSync = fs.readdirSync;
Expand Down Expand Up @@ -57,6 +58,7 @@ describe("server/gateway restart behavior", () => {
afterEach(() => {
childProcess.spawn = originalSpawn;
childProcess.execSync = originalExecSync;
childProcess.execFileSync = originalExecFileSync;
fs.existsSync = originalExistsSync;
fs.mkdirSync = originalMkdirSync;
fs.readdirSync = originalReaddirSync;
Expand Down Expand Up @@ -1275,4 +1277,74 @@ describe("server/gateway restart behavior", () => {
}
}
});

it("passes a channel bot token containing shell metacharacters as a single argv entry, never through a shell", () => {
const maliciousToken = '123:ABC" ; touch /tmp/pwned ; echo "';
const execFileSyncMock = vi.fn(() => "");
childProcess.execFileSync = execFileSyncMock;
fs.readFileSync = vi.fn((targetPath, ...args) => {
if (String(targetPath) === `${OPENCLAW_DIR}/openclaw.json`) {
return JSON.stringify({ channels: {} });
}
return originalReadFileSync(targetPath, ...args);
});
fs.writeFileSync = vi.fn();
delete require.cache[modulePath];
const gateway = require(modulePath);

gateway.syncChannelConfig(
[{ key: "TELEGRAM_BOT_TOKEN", value: maliciousToken }],
"add",
);

expect(execFileSyncMock).toHaveBeenCalledTimes(1);
const [command, args] = execFileSyncMock.mock.calls[0];
expect(command).toBe("openclaw");
expect(args).toEqual([
"channels",
"add",
"--channel",
"telegram",
"--token",
maliciousToken,
]);
});

it("passes slack bot/app tokens as separate argv entries, never concatenated into a shell string", () => {
const maliciousBotToken = 'xoxb-1" ; touch /tmp/pwned ; echo "';
const maliciousAppToken = 'xapp-1" ; touch /tmp/pwned2 ; echo "';
const execFileSyncMock = vi.fn(() => "");
childProcess.execFileSync = execFileSyncMock;
fs.readFileSync = vi.fn((targetPath, ...args) => {
if (String(targetPath) === `${OPENCLAW_DIR}/openclaw.json`) {
return JSON.stringify({ channels: {} });
}
return originalReadFileSync(targetPath, ...args);
});
fs.writeFileSync = vi.fn();
delete require.cache[modulePath];
const gateway = require(modulePath);

gateway.syncChannelConfig(
[
{ key: "SLACK_BOT_TOKEN", value: maliciousBotToken },
{ key: "SLACK_APP_TOKEN", value: maliciousAppToken },
],
"add",
);

expect(execFileSyncMock).toHaveBeenCalledTimes(1);
const [command, args] = execFileSyncMock.mock.calls[0];
expect(command).toBe("openclaw");
expect(args).toEqual([
"channels",
"add",
"--channel",
"slack",
"--bot-token",
maliciousBotToken,
"--app-token",
maliciousAppToken,
]);
});
});