mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-30 07:52:41 +00:00
fix: harden dm command authorization in open mode
This commit is contained in:
@@ -688,7 +688,6 @@ export async function processMessage(
|
||||
chatIdentifier: message.chatIdentifier ?? undefined,
|
||||
})
|
||||
: false;
|
||||
const dmAuthorized = dmPolicy === "open" || ownerAllowedForCommands;
|
||||
const commandGate = resolveControlCommandGate({
|
||||
useAccessGroups,
|
||||
authorizers: [
|
||||
@@ -698,7 +697,7 @@ export async function processMessage(
|
||||
allowTextCommands: true,
|
||||
hasControlCommand: hasControlCmd,
|
||||
});
|
||||
const commandAuthorized = isGroup ? commandGate.commandAuthorized : dmAuthorized;
|
||||
const commandAuthorized = commandGate.commandAuthorized;
|
||||
|
||||
// Block control commands from unauthorized senders in groups
|
||||
if (isGroup && commandGate.shouldBlock) {
|
||||
|
||||
@@ -2305,6 +2305,51 @@ describe("BlueBubbles webhook monitor", () => {
|
||||
|
||||
expect(mockDispatchReplyWithBufferedBlockDispatcher).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not auto-authorize DM control commands in open mode without allowlists", async () => {
|
||||
mockHasControlCommand.mockReturnValue(true);
|
||||
|
||||
const account = createMockAccount({
|
||||
dmPolicy: "open",
|
||||
allowFrom: [],
|
||||
});
|
||||
const config: OpenClawConfig = {};
|
||||
const core = createMockRuntime();
|
||||
setBlueBubblesRuntime(core);
|
||||
|
||||
unregister = registerBlueBubblesWebhookTarget({
|
||||
account,
|
||||
config,
|
||||
runtime: { log: vi.fn(), error: vi.fn() },
|
||||
core,
|
||||
path: "/bluebubbles-webhook",
|
||||
});
|
||||
|
||||
const payload = {
|
||||
type: "new-message",
|
||||
data: {
|
||||
text: "/status",
|
||||
handle: { address: "+15559999999" },
|
||||
isGroup: false,
|
||||
isFromMe: false,
|
||||
guid: "msg-dm-open-unauthorized",
|
||||
date: Date.now(),
|
||||
},
|
||||
};
|
||||
|
||||
const req = createMockRequest("POST", "/bluebubbles-webhook", payload);
|
||||
const res = createMockResponse();
|
||||
|
||||
await handleBlueBubblesWebhookRequest(req, res);
|
||||
await flushAsync();
|
||||
|
||||
expect(mockDispatchReplyWithBufferedBlockDispatcher).toHaveBeenCalled();
|
||||
const latestDispatch =
|
||||
mockDispatchReplyWithBufferedBlockDispatcher.mock.calls[
|
||||
mockDispatchReplyWithBufferedBlockDispatcher.mock.calls.length - 1
|
||||
]?.[0];
|
||||
expect(latestDispatch?.ctx?.CommandAuthorized).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("typing/read receipt toggles", () => {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import { resolveControlCommandGate } from "openclaw/plugin-sdk";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { resolveMattermostEffectiveAllowFromLists } from "./monitor-auth.js";
|
||||
|
||||
@@ -34,4 +35,25 @@ describe("mattermost monitor authz", () => {
|
||||
expect(resolved.effectiveAllowFrom).toEqual(["trusted-user", "attacker"]);
|
||||
expect(resolved.effectiveGroupAllowFrom).toEqual(["trusted-user"]);
|
||||
});
|
||||
|
||||
it("does not auto-authorize DM commands in open mode without allowlists", () => {
|
||||
const resolved = resolveMattermostEffectiveAllowFromLists({
|
||||
dmPolicy: "open",
|
||||
allowFrom: [],
|
||||
groupAllowFrom: [],
|
||||
storeAllowFrom: [],
|
||||
});
|
||||
|
||||
const commandGate = resolveControlCommandGate({
|
||||
useAccessGroups: true,
|
||||
authorizers: [
|
||||
{ configured: resolved.effectiveAllowFrom.length > 0, allowed: false },
|
||||
{ configured: resolved.effectiveGroupAllowFrom.length > 0, allowed: false },
|
||||
],
|
||||
allowTextCommands: true,
|
||||
hasControlCommand: true,
|
||||
});
|
||||
|
||||
expect(commandGate.commandAuthorized).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -415,8 +415,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
|
||||
allowTextCommands,
|
||||
hasControlCommand,
|
||||
});
|
||||
const commandAuthorized =
|
||||
kind === "direct" ? accessDecision.decision === "allow" : commandGate.commandAuthorized;
|
||||
const commandAuthorized = commandGate.commandAuthorized;
|
||||
|
||||
if (accessDecision.decision !== "allow") {
|
||||
if (kind === "direct") {
|
||||
|
||||
@@ -58,3 +58,71 @@ describe("describeIMessageEchoDropLog", () => {
|
||||
).toContain("id=abc-123");
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveIMessageInboundDecision command auth", () => {
|
||||
const cfg = {} as OpenClawConfig;
|
||||
|
||||
it("does not auto-authorize DM commands in open mode without allowlists", () => {
|
||||
const decision = resolveIMessageInboundDecision({
|
||||
cfg,
|
||||
accountId: "default",
|
||||
message: {
|
||||
id: 100,
|
||||
sender: "+15555550123",
|
||||
text: "/status",
|
||||
is_from_me: false,
|
||||
is_group: false,
|
||||
},
|
||||
opts: undefined,
|
||||
messageText: "/status",
|
||||
bodyText: "/status",
|
||||
allowFrom: [],
|
||||
groupAllowFrom: [],
|
||||
groupPolicy: "open",
|
||||
dmPolicy: "open",
|
||||
storeAllowFrom: [],
|
||||
historyLimit: 0,
|
||||
groupHistories: new Map(),
|
||||
echoCache: undefined,
|
||||
logVerbose: undefined,
|
||||
});
|
||||
|
||||
expect(decision.kind).toBe("dispatch");
|
||||
if (decision.kind !== "dispatch") {
|
||||
return;
|
||||
}
|
||||
expect(decision.commandAuthorized).toBe(false);
|
||||
});
|
||||
|
||||
it("authorizes DM commands for senders in pairing-store allowlist", () => {
|
||||
const decision = resolveIMessageInboundDecision({
|
||||
cfg,
|
||||
accountId: "default",
|
||||
message: {
|
||||
id: 101,
|
||||
sender: "+15555550123",
|
||||
text: "/status",
|
||||
is_from_me: false,
|
||||
is_group: false,
|
||||
},
|
||||
opts: undefined,
|
||||
messageText: "/status",
|
||||
bodyText: "/status",
|
||||
allowFrom: [],
|
||||
groupAllowFrom: [],
|
||||
groupPolicy: "open",
|
||||
dmPolicy: "open",
|
||||
storeAllowFrom: ["+15555550123"],
|
||||
historyLimit: 0,
|
||||
groupHistories: new Map(),
|
||||
echoCache: undefined,
|
||||
logVerbose: undefined,
|
||||
});
|
||||
|
||||
expect(decision.kind).toBe("dispatch");
|
||||
if (decision.kind !== "dispatch") {
|
||||
return;
|
||||
}
|
||||
expect(decision.commandAuthorized).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -161,7 +161,6 @@ export function resolveIMessageInboundDecision(params: {
|
||||
});
|
||||
const effectiveDmAllowFrom = accessDecision.effectiveAllowFrom;
|
||||
const effectiveGroupAllowFrom = accessDecision.effectiveGroupAllowFrom;
|
||||
const dmAuthorized = !isGroup && accessDecision.decision === "allow";
|
||||
|
||||
if (accessDecision.decision !== "allow") {
|
||||
if (isGroup) {
|
||||
@@ -287,7 +286,7 @@ export function resolveIMessageInboundDecision(params: {
|
||||
allowTextCommands: true,
|
||||
hasControlCommand: hasControlCommandInMessage,
|
||||
});
|
||||
const commandAuthorized = isGroup ? commandGate.commandAuthorized : dmAuthorized;
|
||||
const commandAuthorized = commandGate.commandAuthorized;
|
||||
if (isGroup && commandGate.shouldBlock) {
|
||||
if (params.logVerbose) {
|
||||
logInboundDrop({
|
||||
|
||||
@@ -143,4 +143,33 @@ describe("signal createSignalEventHandler inbound contract", () => {
|
||||
expect.any(Object),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not auto-authorize DM commands in open mode without allowlists", async () => {
|
||||
const handler = createSignalEventHandler(
|
||||
createBaseSignalEventHandlerDeps({
|
||||
cfg: {
|
||||
messages: { inbound: { debounceMs: 0 } },
|
||||
channels: { signal: { dmPolicy: "open", allowFrom: [] } },
|
||||
},
|
||||
allowFrom: [],
|
||||
groupAllowFrom: [],
|
||||
account: "+15550009999",
|
||||
blockStreaming: false,
|
||||
historyLimit: 0,
|
||||
groupHistories: new Map(),
|
||||
}),
|
||||
);
|
||||
|
||||
await handler(
|
||||
createSignalReceiveEvent({
|
||||
dataMessage: {
|
||||
message: "/status",
|
||||
attachments: [],
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
expect(capture.ctx).toBeTruthy();
|
||||
expect(capture.ctx?.CommandAuthorized).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -475,7 +475,6 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
|
||||
const dmAccess = resolveAccessDecision(false);
|
||||
const effectiveDmAllow = dmAccess.effectiveAllowFrom;
|
||||
const effectiveGroupAllow = dmAccess.effectiveGroupAllowFrom;
|
||||
const dmAllowed = dmAccess.decision === "allow";
|
||||
|
||||
if (
|
||||
reaction &&
|
||||
@@ -573,7 +572,7 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
|
||||
allowTextCommands: true,
|
||||
hasControlCommand: hasControlCommandInMessage,
|
||||
});
|
||||
const commandAuthorized = isGroup ? commandGate.commandAuthorized : dmAllowed;
|
||||
const commandAuthorized = commandGate.commandAuthorized;
|
||||
if (isGroup && commandGate.shouldBlock) {
|
||||
logInboundDrop({
|
||||
log: logVerbose,
|
||||
|
||||
Reference in New Issue
Block a user