diff --git a/lib/server/agents/channels.js b/lib/server/agents/channels.js index 6ca8f198..5cca0169 100644 --- a/lib/server/agents/channels.js +++ b/lib/server/agents/channels.js @@ -1026,6 +1026,11 @@ const createChannelsDomain = ({ throw new Error("Channel login is currently only supported for WhatsApp"); } const accountId = String(rawAccountId || "").trim() || "default"; + if (!isValidChannelAccountId(accountId)) { + throw new Error( + "Channel account id must be lowercase letters, numbers, and hyphens only", + ); + } const loginArgs = [ "channels login", `--channel ${shellEscapeArg(provider)}`, @@ -1060,6 +1065,15 @@ const createChannelsDomain = ({ throw new Error("Channel login status is currently only supported for WhatsApp"); } const accountId = String(rawAccountId || "").trim() || "default"; + // accountId reaches hasSavedWhatsAppCredentials() below, which joins it + // straight into a credentials file path -- reject anything shaped like a + // path (e.g. `../../../../etc/shadow`) before it gets there, same as + // createChannelAccount already does for account creation. + if (!isValidChannelAccountId(accountId)) { + throw new Error( + "Channel account id must be lowercase letters, numbers, and hyphens only", + ); + } return { provider, accountId, diff --git a/lib/server/agents/shared.js b/lib/server/agents/shared.js index 86b4a6a9..b61a4993 100644 --- a/lib/server/agents/shared.js +++ b/lib/server/agents/shared.js @@ -49,18 +49,32 @@ const shellEscapeArg = (value) => const resolveCredentialsDirPath = ({ OPENCLAW_DIR }) => path.join(OPENCLAW_DIR, "credentials"); +const isWithinDir = (rootDir, candidatePath) => { + const resolvedRoot = path.resolve(rootDir); + const resolvedCandidate = path.resolve(candidatePath); + return ( + resolvedCandidate === resolvedRoot || + resolvedCandidate.startsWith(`${resolvedRoot}${path.sep}`) + ); +}; + const resolveWhatsAppCredentialCandidatePaths = ({ OPENCLAW_DIR, accountId, }) => { const credentialsDir = resolveCredentialsDirPath({ OPENCLAW_DIR }); + // normalizeChannelAccountId only trims/defaults -- it doesn't reject `../`, + // and accountId can come straight from a query param (login-status route) + // or a config-supplied account key (e.g. an imported openclaw.json), so + // guard the actual filesystem paths here regardless of what already + // validated (or didn't) upstream. const normalizedAccountId = normalizeChannelAccountId(accountId); return [ path.join(credentialsDir, "whatsapp", normalizedAccountId, "creds.json"), ...(normalizedAccountId === "default" ? [path.join(credentialsDir, "creds.json")] : []), - ]; + ].filter((candidatePath) => isWithinDir(credentialsDir, candidatePath)); }; const hasSavedWhatsAppCredentials = ({ diff --git a/tests/server/agents-service.test.js b/tests/server/agents-service.test.js index 5d699862..eecb0caf 100644 --- a/tests/server/agents-service.test.js +++ b/tests/server/agents-service.test.js @@ -1954,6 +1954,43 @@ describe("server/agents/service", () => { }); }); + it("rejects a path-traversal-shaped accountId for whatsapp login status", () => { + const fsMock = buildFsMock({ initialConfig: {} }); + const service = createAgentsService({ + fs: fsMock, + OPENCLAW_DIR: "/test/.openclaw", + }); + + expect(() => + service.getChannelAccountLoginStatus({ + provider: "whatsapp", + accountId: "../../../../etc/shadow", + }), + ).toThrow(/lowercase letters, numbers, and hyphens/); + // Must reject before ever touching the filesystem. + expect(fsMock.readFileSync).not.toHaveBeenCalled(); + }); + + it("rejects a path-traversal-shaped accountId for whatsapp login", async () => { + const fsMock = buildFsMock({ initialConfig: {} }); + const service = createAgentsService({ + fs: fsMock, + OPENCLAW_DIR: "/test/.openclaw", + readEnvFile: vi.fn(() => []), + writeEnvFile: vi.fn(), + reloadEnv: vi.fn(), + restartGateway: vi.fn(async () => {}), + clawCmd: vi.fn(async () => ({ ok: true })), + }); + + await expect( + service.runChannelAccountLogin({ + provider: "whatsapp", + accountId: "../../../../etc/shadow", + }), + ).rejects.toThrow(/lowercase letters, numbers, and hyphens/); + }); + it("rejects channel login for non-whatsapp providers", async () => { const fsMock = buildFsMock({ initialConfig: {} }); const service = createAgentsService({ diff --git a/tests/server/agents-shared.test.js b/tests/server/agents-shared.test.js new file mode 100644 index 00000000..2232f0ab --- /dev/null +++ b/tests/server/agents-shared.test.js @@ -0,0 +1,79 @@ +const fs = require("fs"); +const os = require("os"); +const path = require("path"); + +const { + resolveWhatsAppCredentialCandidatePaths, + hasSavedWhatsAppCredentials, +} = require("../../lib/server/agents/shared"); + +const kTempDirs = []; +const createTempDir = () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "alphaclaw-agents-shared-")); + kTempDirs.push(tempDir); + return tempDir; +}; +afterEach(() => { + while (kTempDirs.length > 0) { + fs.rmSync(kTempDirs.pop(), { recursive: true, force: true }); + } +}); + +describe("agents/shared WhatsApp credential path containment", () => { + it("never returns a candidate path outside the credentials directory for a traversal-shaped accountId", () => { + const OPENCLAW_DIR = createTempDir(); + + const candidates = resolveWhatsAppCredentialCandidatePaths({ + OPENCLAW_DIR, + accountId: "../../../../etc/shadow", + }); + + const credentialsDir = path.join(OPENCLAW_DIR, "credentials"); + for (const candidate of candidates) { + expect( + candidate === credentialsDir || + candidate.startsWith(`${credentialsDir}${path.sep}`), + ).toBe(true); + } + }); + + it("does not report a real file outside OPENCLAW_DIR as linked WhatsApp credentials", () => { + const OPENCLAW_DIR = createTempDir(); + const outsideDir = createTempDir(); + + // A real, non-empty file named exactly like what the function looks for + // (creds.json), sitting outside OPENCLAW_DIR entirely -- stands in for + // some unrelated real file the accountId traversal happens to reach. + fs.writeFileSync(path.join(outsideDir, "creds.json"), "root:x:0:0\n", "utf8"); + + // accountId such that OPENCLAW_DIR/credentials/whatsapp//creds.json + // resolves to outsideDir/creds.json. + const accountId = path + .relative(path.join(OPENCLAW_DIR, "credentials", "whatsapp"), outsideDir) + .split(path.sep) + .join("/"); + + const linked = hasSavedWhatsAppCredentials({ + fsImpl: fs, + OPENCLAW_DIR, + accountId, + }); + + expect(linked).toBe(false); + }); + + it("still reports linked when real, in-directory WhatsApp credentials exist", () => { + const OPENCLAW_DIR = createTempDir(); + const credsDir = path.join(OPENCLAW_DIR, "credentials", "whatsapp", "default"); + fs.mkdirSync(credsDir, { recursive: true }); + fs.writeFileSync(path.join(credsDir, "creds.json"), "{}", "utf8"); + + const linked = hasSavedWhatsAppCredentials({ + fsImpl: fs, + OPENCLAW_DIR, + accountId: "default", + }); + + expect(linked).toBe(true); + }); +});