diff --git a/CHANGELOG.md b/CHANGELOG.md index 919a038579c..f0db7729ed2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -144,6 +144,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Honor owner enforcement for native commands [AI]. (#78864) Thanks @pgondhi987. - Tavily: resolve dedicated `tavily_search` and `tavily_extract` tool credentials from the active runtime config snapshot, so `exec` SecretRef-backed API keys do not reach the tools unresolved. (#78610) Thanks @VACInc. - Gateway/sessions: clear cached skills snapshots during `/new` and `sessions.reset` so long-lived channel sessions rebuild the visible skill list after skills change. (#78873) Thanks @Evizero. - fix(auto-reply): gate inline skill tool dispatch [AI]. (#78517) Thanks @pgondhi987. diff --git a/src/auto-reply/command-auth.ts b/src/auto-reply/command-auth.ts index d922df87003..7f91f81502f 100644 --- a/src/auto-reply/command-auth.ts +++ b/src/auto-reply/command-auth.ts @@ -430,6 +430,7 @@ function resolveOwnerAuthorizationState(params: { function resolveCommandSenderAuthorization(params: { commandAuthorized: boolean; + enforceOwnerForCommands: boolean; nativeCommandAuthorized: boolean; isOwnerForCommands: boolean; senderCandidates: string[]; @@ -437,6 +438,9 @@ function resolveCommandSenderAuthorization(params: { providerResolutionError: boolean; commandsAllowFromConfigured: boolean; }): boolean { + if (params.enforceOwnerForCommands && !params.isOwnerForCommands) { + return false; + } if ( params.commandsAllowFromList !== null || (params.providerResolutionError && params.commandsAllowFromConfigured) @@ -707,9 +711,10 @@ export function resolveCommandAuthorization(params: { ? senderIsOwner : senderIsOwnerByScope || Boolean(matchedCommandOwner); const nativeCommandAuthorized = - commandAuthorized && ctx.CommandSource === "native" && !ownerAllowlistConfigured; + commandAuthorized && ctx.CommandSource === "native" && !requireOwner; const isAuthorizedSender = resolveCommandSenderAuthorization({ commandAuthorized, + enforceOwnerForCommands: enforceOwner, nativeCommandAuthorized, isOwnerForCommands, senderCandidates, diff --git a/src/auto-reply/command-control.test.ts b/src/auto-reply/command-control.test.ts index 1125ce90ee4..afe10f4e63f 100644 --- a/src/auto-reply/command-control.test.ts +++ b/src/auto-reply/command-control.test.ts @@ -44,6 +44,20 @@ describe("resolveCommandAuthorization", () => { }); } + function createOwnerEnforcingAllowFromPlugin( + id: string, + resolveAllowFrom: () => Array | undefined, + ) { + const entry = createAllowFromPlugin(id, resolveAllowFrom); + return { + ...entry, + plugin: { + ...entry.plugin, + commands: { enforceOwnerForCommands: true }, + }, + }; + } + function registerAllowFromPlugins(...plugins: ReturnType[]) { setActivePluginRegistry(createTestRegistry(plugins)); } @@ -202,7 +216,7 @@ describe("resolveCommandAuthorization", () => { expect(auth.isAuthorizedSender).toBe(false); }); - it("allows channel-validated native commands when plugin owner enforcement has no owner allowlist", () => { + it("rejects channel-validated native commands when plugin owner enforcement has no owner allowlist", () => { setActivePluginRegistry( createTestRegistry([ { @@ -242,7 +256,7 @@ describe("resolveCommandAuthorization", () => { }); expect(auth.senderIsOwner).toBe(false); - expect(auth.isAuthorizedSender).toBe(true); + expect(auth.isAuthorizedSender).toBe(false); }); it("uses explicit owner allowlist when allowFrom is empty", () => { @@ -632,6 +646,62 @@ describe("resolveCommandAuthorization", () => { expect(auth.isAuthorizedSender).toBe(true); }); + it("requires owner identity before commands.allowFrom when the plugin enforces owner-only commands", () => { + registerAllowFromPlugins(createOwnerEnforcingAllowFromPlugin("telegram", () => ["*"])); + const cfg = { + commands: { + allowFrom: { + "*": ["*"], + }, + }, + channels: { telegram: { allowFrom: ["*"] } }, + } as OpenClawConfig; + + const auth = resolveCommandAuthorization({ + ctx: { + Provider: "telegram", + Surface: "telegram", + ChatType: "group", + From: "telegram:999", + SenderId: "999", + CommandSource: "native", + } as MsgContext, + cfg, + commandAuthorized: true, + }); + + expect(auth.senderIsOwner).toBe(false); + expect(auth.isAuthorizedSender).toBe(false); + }); + + it("keeps commands.allowFrom available to non-owner command users when an owner allowlist is configured", () => { + const cfg = { + commands: { + ownerAllowFrom: ["discord:owner"], + allowFrom: { + discord: ["helper"], + }, + }, + channels: { discord: { allowFrom: ["*"] } }, + } as OpenClawConfig; + + const auth = resolveCommandAuthorization({ + ctx: { + Provider: "discord", + Surface: "discord", + ChatType: "group", + From: "discord:helper", + SenderId: "helper", + CommandSource: "native", + } as MsgContext, + cfg, + commandAuthorized: true, + }); + + expect(auth.senderIsOwner).toBe(false); + expect(auth.isAuthorizedSender).toBe(true); + }); + it("does not treat conversation ids in From as sender identities", () => { const cfg = { commands: { diff --git a/src/auto-reply/reply/commands-stop-target.test.ts b/src/auto-reply/reply/commands-stop-target.test.ts index f3fca86b351..0cf415de302 100644 --- a/src/auto-reply/reply/commands-stop-target.test.ts +++ b/src/auto-reply/reply/commands-stop-target.test.ts @@ -1,5 +1,13 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { OpenClawConfig } from "../../config/config.js"; +import { + getActivePluginRegistry, + resetPluginRuntimeStateForTest, + setActivePluginRegistry, +} from "../../plugins/runtime.js"; +import { createOutboundTestPlugin, createTestRegistry } from "../../test-utils/channel-plugins.js"; +import { resolveCommandAuthorization } from "../command-auth.js"; +import type { MsgContext } from "../templating.js"; import { handleStopCommand } from "./commands-session-abort.js"; import "./commands-session-abort.test-support.js"; import type { HandleCommandsParams } from "./commands-types.js"; @@ -48,6 +56,35 @@ vi.mock("./reply-run-registry.js", () => ({ }, })); +const formatAllowFrom = ({ allowFrom }: { allowFrom: Array }) => + allowFrom.map((entry) => String(entry).trim()).filter(Boolean); + +let previousPluginRegistry: ReturnType; + +function registerOwnerEnforcingTelegramPlugin() { + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "telegram", + plugin: { + ...createOutboundTestPlugin({ + id: "telegram", + outbound: { deliveryMode: "direct" }, + }), + commands: { enforceOwnerForCommands: true }, + config: { + listAccountIds: () => ["default"], + resolveAccount: () => ({}), + resolveAllowFrom: () => ["*"], + formatAllowFrom, + }, + }, + source: "test", + }, + ]), + ); +} + function buildStopParams(): HandleCommandsParams { return { cfg: { @@ -85,10 +122,19 @@ function buildStopParams(): HandleCommandsParams { describe("handleStopCommand target fallback", () => { beforeEach(() => { + previousPluginRegistry = getActivePluginRegistry(); vi.clearAllMocks(); persistAbortTargetEntryMock.mockResolvedValue(true); }); + afterEach(() => { + if (previousPluginRegistry) { + setActivePluginRegistry(previousPluginRegistry); + } else { + resetPluginRuntimeStateForTest(); + } + }); + it("does not fall back to the wrapper session when a distinct target session is missing from store", async () => { const params = buildStopParams(); @@ -120,4 +166,47 @@ describe("handleStopCommand target fallback", () => { }), ); }); + + it("rejects native stop commands from non-owner senders when the plugin enforces owner-only commands", async () => { + registerOwnerEnforcingTelegramPlugin(); + const params = buildStopParams(); + const cfg = { + commands: { text: true, allowFrom: { "*": ["*"] } }, + channels: { telegram: { allowFrom: ["*"] } }, + } as OpenClawConfig; + const ctx = { + Provider: "telegram", + Surface: "telegram", + ChatType: "group", + From: "telegram:999", + SenderId: "999", + CommandSource: "native", + CommandTargetSessionKey: "agent:target:telegram:direct:123", + } as MsgContext; + const auth = resolveCommandAuthorization({ + ctx, + cfg, + commandAuthorized: true, + }); + params.cfg = cfg; + params.ctx = ctx; + params.command.senderId = auth.senderId; + params.command.senderIsOwner = auth.senderIsOwner; + params.command.isAuthorizedSender = auth.isAuthorizedSender; + params.command.from = auth.from; + params.command.to = auth.to; + + const result = await handleStopCommand(params, true); + + expect(auth.senderIsOwner).toBe(false); + expect(auth.isAuthorizedSender).toBe(false); + expect(result).toEqual({ + shouldContinue: false, + reply: { text: "You are not authorized to use this command." }, + }); + expect(replyRunAbortMock).not.toHaveBeenCalled(); + expect(persistAbortTargetEntryMock).not.toHaveBeenCalled(); + expect(createInternalHookEventMock).not.toHaveBeenCalled(); + expect(stopSubagentsForRequesterMock).not.toHaveBeenCalled(); + }); }); diff --git a/src/auto-reply/reply/commands-subagents-routing.test.ts b/src/auto-reply/reply/commands-subagents-routing.test.ts index 2d46ff74cdb..770e2bb02d4 100644 --- a/src/auto-reply/reply/commands-subagents-routing.test.ts +++ b/src/auto-reply/reply/commands-subagents-routing.test.ts @@ -1,4 +1,13 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../../config/config.js"; +import { + getActivePluginRegistry, + resetPluginRuntimeStateForTest, + setActivePluginRegistry, +} from "../../plugins/runtime.js"; +import { createOutboundTestPlugin, createTestRegistry } from "../../test-utils/channel-plugins.js"; +import { resolveCommandAuthorization } from "../command-auth.js"; +import type { MsgContext } from "../templating.js"; import { COMMAND, COMMAND_KILL, @@ -7,8 +16,51 @@ import { resolveSubagentsAction, stopWithText, } from "./commands-subagents-dispatch.js"; +import { handleSubagentsCommand } from "./commands-subagents.js"; import type { HandleCommandsParams } from "./commands-types.js"; +const handleSubagentsSpawnActionMock = vi.hoisted(() => + vi.fn(async () => ({ shouldContinue: false, reply: { text: "spawned" } })), +); +const listControlledSubagentRunsMock = vi.hoisted(() => vi.fn(() => [])); + +vi.mock("./commands-subagents/action-spawn.js", () => ({ + handleSubagentsSpawnAction: handleSubagentsSpawnActionMock, +})); + +vi.mock("./commands-subagents-control.runtime.js", () => ({ + listControlledSubagentRuns: listControlledSubagentRunsMock, +})); + +const formatAllowFrom = ({ allowFrom }: { allowFrom: Array }) => + allowFrom.map((entry) => String(entry).trim()).filter(Boolean); + +let previousPluginRegistry: ReturnType; + +function registerOwnerEnforcingTelegramPlugin() { + setActivePluginRegistry( + createTestRegistry([ + { + pluginId: "telegram", + plugin: { + ...createOutboundTestPlugin({ + id: "telegram", + outbound: { deliveryMode: "direct" }, + }), + commands: { enforceOwnerForCommands: true }, + config: { + listAccountIds: () => ["default"], + resolveAccount: () => ({}), + resolveAllowFrom: () => ["*"], + formatAllowFrom, + }, + }, + source: "test", + }, + ]), + ); +} + function buildParams( commandBody: string, ctxOverrides?: Record, @@ -57,6 +109,19 @@ function buildParams( } describe("subagents command dispatch", () => { + beforeEach(() => { + previousPluginRegistry = getActivePluginRegistry(); + vi.clearAllMocks(); + }); + + afterEach(() => { + if (previousPluginRegistry) { + setActivePluginRegistry(previousPluginRegistry); + } else { + resetPluginRuntimeStateForTest(); + } + }); + it("prefers native command target session keys", () => { const params = buildParams("/subagents list", { CommandSource: "native", @@ -112,4 +177,46 @@ describe("subagents command dispatch", () => { reply: { text: "hello" }, }); }); + + it("rejects native spawn commands from non-owner senders when the plugin enforces owner-only commands", async () => { + registerOwnerEnforcingTelegramPlugin(); + const cfg = { + commands: { allowFrom: { "*": ["*"] } }, + channels: { telegram: { allowFrom: ["*"] } }, + } as OpenClawConfig; + const ctx = { + Provider: "telegram", + Surface: "telegram", + ChatType: "group", + From: "telegram:999", + SenderId: "999", + CommandSource: "native", + SessionKey: "agent:main:telegram:slash-session", + CommandTargetSessionKey: "agent:main:telegram:target", + } as MsgContext; + const auth = resolveCommandAuthorization({ + ctx, + cfg, + commandAuthorized: true, + }); + const params = buildParams( + "/subagents spawn beta do the thing", + ctx as unknown as Record, + ); + params.cfg = cfg; + params.command.senderId = auth.senderId; + params.command.senderIsOwner = auth.senderIsOwner; + params.command.isAuthorizedSender = auth.isAuthorizedSender; + params.command.ownerList = auth.ownerList; + params.command.from = auth.from; + params.command.to = auth.to; + + const result = await handleSubagentsCommand(params, true); + + expect(auth.senderIsOwner).toBe(false); + expect(auth.isAuthorizedSender).toBe(false); + expect(result).toEqual({ shouldContinue: false }); + expect(listControlledSubagentRunsMock).not.toHaveBeenCalled(); + expect(handleSubagentsSpawnActionMock).not.toHaveBeenCalled(); + }); });