mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 20:40:43 +00:00
Honor owner enforcement for native commands [AI] (#78864)
* fix: honor owner enforcement for native commands * addressing codex review * addressing codex review * docs: add changelog entry for PR merge
This commit is contained in:
committed by
GitHub
parent
55bff24973
commit
758051322d
@@ -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.
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -44,6 +44,20 @@ describe("resolveCommandAuthorization", () => {
|
||||
});
|
||||
}
|
||||
|
||||
function createOwnerEnforcingAllowFromPlugin(
|
||||
id: string,
|
||||
resolveAllowFrom: () => Array<string | number> | undefined,
|
||||
) {
|
||||
const entry = createAllowFromPlugin(id, resolveAllowFrom);
|
||||
return {
|
||||
...entry,
|
||||
plugin: {
|
||||
...entry.plugin,
|
||||
commands: { enforceOwnerForCommands: true },
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function registerAllowFromPlugins(...plugins: ReturnType<typeof createAllowFromPlugin>[]) {
|
||||
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: {
|
||||
|
||||
@@ -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<string | number> }) =>
|
||||
allowFrom.map((entry) => String(entry).trim()).filter(Boolean);
|
||||
|
||||
let previousPluginRegistry: ReturnType<typeof getActivePluginRegistry>;
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string | number> }) =>
|
||||
allowFrom.map((entry) => String(entry).trim()).filter(Boolean);
|
||||
|
||||
let previousPluginRegistry: ReturnType<typeof getActivePluginRegistry>;
|
||||
|
||||
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<string, unknown>,
|
||||
@@ -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<string, unknown>,
|
||||
);
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user