diff --git a/CHANGELOG.md b/CHANGELOG.md index bb897b043b4..38a860abfeb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ Docs: https://docs.openclaw.ai - Security/exec approvals: escape invisible Unicode format characters in approval prompts so zero-width command text renders as visible `\u{...}` escapes instead of spoofing the reviewed command. (`GHSA-pcqg-f7rg-xfvv`)(#43687) Thanks @EkiXu and @vincentkoc. - Security/exec detection: normalize compatibility Unicode and strip invisible formatting code points before obfuscation checks so zero-width and fullwidth command tricks no longer suppress heuristic detection. (`GHSA-9r3v-37xh-2cf6`)(#44091) Thanks @wooluo and @vincentkoc. - Security/exec allowlist: preserve POSIX case sensitivity and keep `?` within a single path segment so exact-looking allowlist patterns no longer overmatch executables across case or directory boundaries. (`GHSA-f8r2-vg7x-gh8m`)(#43798) Thanks @zpbrent and @vincentkoc. +- Security/commands: require sender ownership for `/config` and `/debug` so authorized non-owner senders can no longer reach owner-only config and runtime debug surfaces. (`GHSA-r7vr-gr74-94p8`)(#44305) Thanks @tdjackey and @vincentkoc. - Security/gateway auth: clear unbound client-declared scopes on shared-token WebSocket connects so device-less shared-token operators cannot self-declare elevated scopes. (`GHSA-rqpp-rjj8-7wv8`)(#44306) Thanks @LUOYEcode and @vincentkoc. - Security/browser.request: block persistent browser profile create/delete routes from write-scoped `browser.request` so callers can no longer persist admin-only browser profile changes through the browser control surface. (`GHSA-vmhq-cqm9-6p7q`)(#43800) Thanks @tdjackey and @vincentkoc. - Security/agent: reject public spawned-run lineage fields and keep workspace inheritance on the internal spawned-session path so external `agent` callers can no longer override the gateway workspace boundary. (`GHSA-2rqg-gjgv-84jm`)(#43801) Thanks @tdjackey and @vincentkoc. diff --git a/src/auto-reply/reply/command-gates.ts b/src/auto-reply/reply/command-gates.ts index 49cf21c6861..1f0b441f51a 100644 --- a/src/auto-reply/reply/command-gates.ts +++ b/src/auto-reply/reply/command-gates.ts @@ -1,6 +1,7 @@ import type { CommandFlagKey } from "../../config/commands.js"; import { isCommandFlagEnabled } from "../../config/commands.js"; import { logVerbose } from "../../globals.js"; +import { redactIdentifier } from "../../logging/redact-identifier.js"; import { isInternalMessageChannel } from "../../utils/message-channel.js"; import type { ReplyPayload } from "../types.js"; import type { CommandHandlerResult, HandleCommandsParams } from "./commands-types.js"; @@ -13,7 +14,20 @@ export function rejectUnauthorizedCommand( return null; } logVerbose( - `Ignoring ${commandLabel} from unauthorized sender: ${params.command.senderId || ""}`, + `Ignoring ${commandLabel} from unauthorized sender: ${redactIdentifier(params.command.senderId)}`, + ); + return { shouldContinue: false }; +} + +export function rejectNonOwnerCommand( + params: HandleCommandsParams, + commandLabel: string, +): CommandHandlerResult | null { + if (params.command.senderIsOwner) { + return null; + } + logVerbose( + `Ignoring ${commandLabel} from non-owner sender: ${redactIdentifier(params.command.senderId)}`, ); return { shouldContinue: false }; } diff --git a/src/auto-reply/reply/commands-config.ts b/src/auto-reply/reply/commands-config.ts index 0d00358e582..96b5a5d9be5 100644 --- a/src/auto-reply/reply/commands-config.ts +++ b/src/auto-reply/reply/commands-config.ts @@ -22,7 +22,9 @@ import { setConfigOverride, unsetConfigOverride, } from "../../config/runtime-overrides.js"; +import { isInternalMessageChannel } from "../../utils/message-channel.js"; import { + rejectNonOwnerCommand, rejectUnauthorizedCommand, requireCommandFlagEnabled, requireGatewayClientScopeForInternalChannel, @@ -43,6 +45,12 @@ export const handleConfigCommand: CommandHandler = async (params, allowTextComma if (unauthorized) { return unauthorized; } + const allowInternalReadOnlyShow = + configCommand.action === "show" && isInternalMessageChannel(params.command.channel); + const nonOwner = allowInternalReadOnlyShow ? null : rejectNonOwnerCommand(params, "/config"); + if (nonOwner) { + return nonOwner; + } const disabled = requireCommandFlagEnabled(params.cfg, { label: "/config", configKey: "config", @@ -197,6 +205,10 @@ export const handleDebugCommand: CommandHandler = async (params, allowTextComman if (unauthorized) { return unauthorized; } + const nonOwner = rejectNonOwnerCommand(params, "/debug"); + if (nonOwner) { + return nonOwner; + } const disabled = requireCommandFlagEnabled(params.cfg, { label: "/debug", configKey: "debug", diff --git a/src/auto-reply/reply/commands.test.ts b/src/auto-reply/reply/commands.test.ts index 073cc36488c..8f48029fd18 100644 --- a/src/auto-reply/reply/commands.test.ts +++ b/src/auto-reply/reply/commands.test.ts @@ -181,6 +181,9 @@ describe("handleCommands gating", () => { commands: { config: false, debug: false, text: true }, channels: { whatsapp: { allowFrom: ["*"] } }, }) as OpenClawConfig, + applyParams: (params: ReturnType) => { + params.command.senderIsOwner = true; + }, expectedText: "/config is disabled", }, { @@ -191,6 +194,9 @@ describe("handleCommands gating", () => { commands: { config: false, debug: false, text: true }, channels: { whatsapp: { allowFrom: ["*"] } }, }) as OpenClawConfig, + applyParams: (params: ReturnType) => { + params.command.senderIsOwner = true; + }, expectedText: "/debug is disabled", }, { @@ -223,6 +229,9 @@ describe("handleCommands gating", () => { channels: { whatsapp: { allowFrom: ["*"] } }, } as OpenClawConfig; }, + applyParams: (params: ReturnType) => { + params.command.senderIsOwner = true; + }, expectedText: "/config is disabled", }, { @@ -239,6 +248,9 @@ describe("handleCommands gating", () => { channels: { whatsapp: { allowFrom: ["*"] } }, } as OpenClawConfig; }, + applyParams: (params: ReturnType) => { + params.command.senderIsOwner = true; + }, expectedText: "/debug is disabled", }, ]); @@ -670,6 +682,36 @@ describe("extractMessageText", () => { }); }); +describe("handleCommands /config owner gating", () => { + it("blocks /config show from authorized non-owner senders", async () => { + const cfg = { + commands: { config: true, text: true }, + channels: { whatsapp: { allowFrom: ["*"] } }, + } as OpenClawConfig; + const params = buildParams("/config show", cfg); + params.command.senderIsOwner = false; + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply).toBeUndefined(); + }); + + it("keeps /config show working for owners", async () => { + const cfg = { + commands: { config: true, text: true }, + channels: { whatsapp: { allowFrom: ["*"] } }, + } as OpenClawConfig; + readConfigFileSnapshotMock.mockResolvedValueOnce({ + valid: true, + parsed: { messages: { ackreaction: ":)" } }, + }); + const params = buildParams("/config show messages.ackReaction", cfg); + params.command.senderIsOwner = true; + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain("Config messages.ackreaction"); + }); +}); + describe("handleCommands /config configWrites gating", () => { it("blocks /config set when channel config writes are disabled", async () => { const cfg = { @@ -677,6 +719,7 @@ describe("handleCommands /config configWrites gating", () => { channels: { whatsapp: { allowFrom: ["*"], configWrites: false } }, } as OpenClawConfig; const params = buildParams('/config set messages.ackReaction=":)"', cfg); + params.command.senderIsOwner = true; const result = await handleCommands(params); expect(result.shouldContinue).toBe(false); expect(result.reply?.text).toContain("Config writes are disabled"); @@ -704,6 +747,7 @@ describe("handleCommands /config configWrites gating", () => { Surface: "telegram", }, ); + params.command.senderIsOwner = true; const result = await handleCommands(params); expect(result.shouldContinue).toBe(false); expect(result.reply?.text).toContain("channels.telegram.accounts.work.configWrites=true"); @@ -720,6 +764,7 @@ describe("handleCommands /config configWrites gating", () => { Provider: "telegram", Surface: "telegram", }); + params.command.senderIsOwner = true; const result = await handleCommands(params); expect(result.shouldContinue).toBe(false); expect(result.reply?.text).toContain( @@ -738,6 +783,7 @@ describe("handleCommands /config configWrites gating", () => { GatewayClientScopes: ["operator.write"], }); params.command.channel = INTERNAL_MESSAGE_CHANNEL; + params.command.senderIsOwner = true; const result = await handleCommands(params); expect(result.shouldContinue).toBe(false); expect(result.reply?.text).toContain("requires operator.admin"); @@ -757,6 +803,7 @@ describe("handleCommands /config configWrites gating", () => { GatewayClientScopes: ["operator.write"], }); params.command.channel = INTERNAL_MESSAGE_CHANNEL; + params.command.senderIsOwner = false; const result = await handleCommands(params); expect(result.shouldContinue).toBe(false); expect(result.reply?.text).toContain("Config messages.ackreaction"); @@ -780,6 +827,7 @@ describe("handleCommands /config configWrites gating", () => { GatewayClientScopes: ["operator.write", "operator.admin"], }); params.command.channel = INTERNAL_MESSAGE_CHANNEL; + params.command.senderIsOwner = true; const result = await handleCommands(params); expect(result.shouldContinue).toBe(false); expect(writeConfigFileMock).toHaveBeenCalledOnce(); @@ -822,6 +870,7 @@ describe("handleCommands /config configWrites gating", () => { }, ); params.command.channel = INTERNAL_MESSAGE_CHANNEL; + params.command.senderIsOwner = true; const result = await handleCommands(params); expect(result.shouldContinue).toBe(false); expect(result.reply?.text).toContain("Config updated"); @@ -830,6 +879,32 @@ describe("handleCommands /config configWrites gating", () => { }); }); +describe("handleCommands /debug owner gating", () => { + it("blocks /debug show from authorized non-owner senders", async () => { + const cfg = { + commands: { debug: true, text: true }, + channels: { whatsapp: { allowFrom: ["*"] } }, + } as OpenClawConfig; + const params = buildParams("/debug show", cfg); + params.command.senderIsOwner = false; + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply).toBeUndefined(); + }); + + it("keeps /debug show working for owners", async () => { + const cfg = { + commands: { debug: true, text: true }, + channels: { whatsapp: { allowFrom: ["*"] } }, + } as OpenClawConfig; + const params = buildParams("/debug show", cfg); + params.command.senderIsOwner = true; + const result = await handleCommands(params); + expect(result.shouldContinue).toBe(false); + expect(result.reply?.text).toContain("Debug overrides"); + }); +}); + describe("handleCommands bash alias", () => { it("routes !poll and !stop through the /bash handler", async () => { const cfg = {