mirror of https://github.com/openclaw/openclaw.git
Commands: require owner for /config and /debug (#44305)
* Commands: add non-owner gate helper * Commands: enforce owner-only config and debug * Commands/test: cover owner-only config and debug * Changelog: add owner-only config debug entry * Commands/test: split config owner gating section * Commands: redact sender ids in verbose command logs * Commands: preserve internal read-only config access * Commands/test: keep operator.write config show coverage non-owner
This commit is contained in:
parent
fda4965818
commit
08aa57a3de
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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 || "<unknown>"}`,
|
||||
`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 };
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -181,6 +181,9 @@ describe("handleCommands gating", () => {
|
|||
commands: { config: false, debug: false, text: true },
|
||||
channels: { whatsapp: { allowFrom: ["*"] } },
|
||||
}) as OpenClawConfig,
|
||||
applyParams: (params: ReturnType<typeof buildParams>) => {
|
||||
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<typeof buildParams>) => {
|
||||
params.command.senderIsOwner = true;
|
||||
},
|
||||
expectedText: "/debug is disabled",
|
||||
},
|
||||
{
|
||||
|
|
@ -223,6 +229,9 @@ describe("handleCommands gating", () => {
|
|||
channels: { whatsapp: { allowFrom: ["*"] } },
|
||||
} as OpenClawConfig;
|
||||
},
|
||||
applyParams: (params: ReturnType<typeof buildParams>) => {
|
||||
params.command.senderIsOwner = true;
|
||||
},
|
||||
expectedText: "/config is disabled",
|
||||
},
|
||||
{
|
||||
|
|
@ -239,6 +248,9 @@ describe("handleCommands gating", () => {
|
|||
channels: { whatsapp: { allowFrom: ["*"] } },
|
||||
} as OpenClawConfig;
|
||||
},
|
||||
applyParams: (params: ReturnType<typeof buildParams>) => {
|
||||
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 = {
|
||||
|
|
|
|||
Loading…
Reference in New Issue