From 37c0520a0b9c23105340838147096702a8b02c03 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Gondhi Date: Mon, 4 May 2026 22:12:06 +0530 Subject: [PATCH] fix(device-pair): require pairing scope for pair command [AI] (#76377) * fix: restrict device pairing command access * addressing review-skill * addressing review-skill * addressing codex review * address codex review feedback * addressing codex review * addressing codex review * addressing codex review * addressing codex review * docs: add changelog entry for PR merge --- CHANGELOG.md | 1 + extensions/device-pair/index.test.ts | 167 +++++++++++--- extensions/device-pair/index.ts | 30 +-- .../device-pair/pair-command-auth.test.ts | 38 +++- extensions/device-pair/pair-command-auth.ts | 37 ++-- .../native-command.plugin-dispatch.test.ts | 209 ++++++++++++++++++ .../discord/src/monitor/native-command.ts | 55 ++++- src/plugins/commands.test.ts | 80 ++++++- src/plugins/commands.ts | 25 ++- .../contracts/host-hooks.contract.test.ts | 3 +- src/plugins/types.ts | 2 +- 11 files changed, 556 insertions(+), 91 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7177cabbb8f..80f9d7bbf3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- fix(device-pair): require pairing scope for pair command [AI]. (#76377) Thanks @pgondhi987. - fix(qqbot): keep private commands off framework surface [AI]. (#77212) Thanks @pgondhi987. - Memory/wiki: preserve representation from both corpora in `corpus=all` searches while backfilling unused result capacity, so memory hits are not starved by numerically higher wiki integer scores. Fixes #77337. Thanks @hclsys. - Telegram: clean up tool-only draft previews after assistant message boundaries so transient `Surfacing...` tool-status bubbles do not linger when no matching final preview arrives. Thanks @BunsDev. diff --git a/extensions/device-pair/index.test.ts b/extensions/device-pair/index.test.ts index 0afd9e7e09e..008b47a103b 100644 --- a/extensions/device-pair/index.test.ts +++ b/extensions/device-pair/index.test.ts @@ -258,6 +258,7 @@ describe("device-pair /pair qr", () => { it("returns an inline QR image for webchat surfaces", async () => { const command = registerPairCommand(); + expect(command.requiredScopes).toEqual(["operator.pairing"]); const result = await command.handler( createCommandContext({ channel: "webchat", @@ -296,7 +297,24 @@ describe("device-pair /pair qr", () => { expect(pluginApiMocks.issueDeviceBootstrapToken).not.toHaveBeenCalled(); expect(result).toEqual({ - text: "⚠️ This command requires operator.pairing for internal gateway callers.", + text: "⚠️ This command requires operator.pairing.", + }); + }); + + it("rejects qr setup for non-gateway command surfaces without pairing scopes", async () => { + const command = registerPairCommand(); + const result = await command.handler( + createCommandContext({ + channel: "telegram", + args: "qr", + commandBody: "/pair qr", + gatewayClientScopes: undefined, + }), + ); + + expect(pluginApiMocks.issueDeviceBootstrapToken).not.toHaveBeenCalled(); + expect(result).toEqual({ + text: "⚠️ This command requires operator.pairing.", }); }); @@ -433,7 +451,12 @@ describe("device-pair /pair qr", () => { runtime: createChannelRuntime(testCase.runtimeKey, testCase.sendKey, sendMessage), }); - const result = await command.handler(createCommandContext(testCase.ctx)); + const result = await command.handler( + createCommandContext({ + ...testCase.ctx, + gatewayClientScopes: INTERNAL_PAIRING_SCOPES, + }), + ); const text = requireText(result); expect(sendMessage).toHaveBeenCalledTimes(1); @@ -479,6 +502,7 @@ describe("device-pair /pair qr", () => { createCommandContext({ channel: "discord", senderId: "123", + gatewayClientScopes: INTERNAL_PAIRING_SCOPES, }), ); const text = requireText(result); @@ -497,6 +521,7 @@ describe("device-pair /pair qr", () => { createCommandContext({ channel: "msteams", senderId: "8:orgid:123", + gatewayClientScopes: INTERNAL_PAIRING_SCOPES, }), ); const text = requireText(result); @@ -514,6 +539,7 @@ describe("device-pair /pair qr", () => { channel: "telegram", args: "cleanup", commandBody: "/pair cleanup", + gatewayClientScopes: INTERNAL_PAIRING_SCOPES, }), ); @@ -534,7 +560,7 @@ describe("device-pair /pair qr", () => { expect(pluginApiMocks.clearDeviceBootstrapTokens).not.toHaveBeenCalled(); expect(result).toEqual({ - text: "⚠️ This command requires operator.pairing for internal gateway callers.", + text: "⚠️ This command requires operator.pairing.", }); }); @@ -551,7 +577,24 @@ describe("device-pair /pair qr", () => { expect(pluginApiMocks.clearDeviceBootstrapTokens).not.toHaveBeenCalled(); expect(result).toEqual({ - text: "⚠️ This command requires operator.pairing for internal gateway callers.", + text: "⚠️ This command requires operator.pairing.", + }); + }); + + it("rejects status for non-gateway command surfaces without pairing scopes", async () => { + const command = registerPairCommand(); + const result = await command.handler( + createCommandContext({ + channel: "telegram", + args: "status", + commandBody: "/pair status", + gatewayClientScopes: undefined, + }), + ); + + expect(vi.mocked(listDevicePairing)).not.toHaveBeenCalled(); + expect(result).toEqual({ + text: "⚠️ This command requires operator.pairing.", }); }); }); @@ -578,7 +621,7 @@ describe("device-pair /pair default setup code", () => { expect(pluginApiMocks.issueDeviceBootstrapToken).not.toHaveBeenCalled(); expect(result).toEqual({ - text: "⚠️ This command requires operator.pairing for internal gateway callers.", + text: "⚠️ This command requires operator.pairing.", }); }); @@ -595,7 +638,7 @@ describe("device-pair /pair default setup code", () => { expect(pluginApiMocks.issueDeviceBootstrapToken).not.toHaveBeenCalled(); expect(result).toEqual({ - text: "⚠️ This command requires operator.pairing for internal gateway callers.", + text: "⚠️ This command requires operator.pairing.", }); }); @@ -612,10 +655,44 @@ describe("device-pair /pair default setup code", () => { expect(pluginApiMocks.issueDeviceBootstrapToken).not.toHaveBeenCalled(); expect(result).toEqual({ - text: "⚠️ This command requires operator.pairing for internal gateway callers.", + text: "⚠️ This command requires operator.pairing.", }); }); + it("fails closed for non-gateway setup code issuance when scopes are absent", async () => { + const command = registerPairCommand(); + const result = await command.handler( + createCommandContext({ + channel: "telegram", + args: "", + commandBody: "/pair", + gatewayClientScopes: undefined, + }), + ); + + expect(pluginApiMocks.issueDeviceBootstrapToken).not.toHaveBeenCalled(); + expect(result).toEqual({ + text: "⚠️ This command requires operator.pairing.", + }); + }); + + it("allows command owners to issue setup codes from non-gateway command surfaces", async () => { + const command = registerPairCommand(); + const result = await command.handler( + createCommandContext({ + channel: "telegram", + args: "", + commandBody: "/pair", + gatewayClientScopes: undefined, + senderIsOwner: true, + }), + ); + const text = requireText(result); + + expect(pluginApiMocks.issueDeviceBootstrapToken).toHaveBeenCalledTimes(1); + expect(text).toContain("Pairing setup code generated."); + }); + it("normalizes secure bare publicUrl host ports before issuing setup codes", async () => { const command = registerPairCommand({ config: { @@ -909,7 +986,7 @@ describe("device-pair /pair approve", () => { expect(vi.mocked(approveDevicePairing)).not.toHaveBeenCalled(); expect(result).toEqual({ - text: "⚠️ This command requires operator.pairing for internal gateway callers.", + text: "⚠️ This command requires operator.pairing.", }); }); @@ -924,10 +1001,7 @@ describe("device-pair /pair approve", () => { expect(result).toEqual({ text: "✅ Paired Victim Phone (ios)." }); }); - it("does not force an empty caller scope context for external approvals", async () => { - mockPendingPairingList(); - vi.mocked(approveDevicePairing).mockResolvedValueOnce(makeApprovedPairingResult()); - + it("rejects non-gateway approvals without pairing scopes", async () => { const command = registerPairCommand(); const result = await command.handler( createCommandContext({ @@ -938,7 +1012,49 @@ describe("device-pair /pair approve", () => { }), ); - expect(vi.mocked(approveDevicePairing)).toHaveBeenCalledWith("req-1"); + expect(vi.mocked(approveDevicePairing)).not.toHaveBeenCalled(); + expect(result).toEqual({ + text: "⚠️ This command requires operator.pairing.", + }); + }); + + it("allows command owners to approve from non-gateway command surfaces", async () => { + mockPendingPairingList(); + vi.mocked(approveDevicePairing).mockResolvedValueOnce(makeApprovedPairingResult()); + + const command = registerPairCommand(); + const result = await command.handler( + createCommandContext({ + channel: "telegram", + args: "approve latest", + commandBody: "/pair approve latest", + gatewayClientScopes: undefined, + senderIsOwner: true, + }), + ); + + expect(vi.mocked(approveDevicePairing)).toHaveBeenCalledWith("req-1", { + callerScopes: ["operator.pairing"], + }); + expect(result).toEqual({ text: "✅ Paired Victim Phone (ios)." }); + }); + + it("preserves gateway caller scopes for command-owner approvals", async () => { + mockPendingPairingList(); + vi.mocked(approveDevicePairing).mockResolvedValueOnce(makeApprovedPairingResult()); + + const command = registerPairCommand(); + const result = await command.handler( + createCommandContext({ + channel: "telegram", + args: "approve latest", + commandBody: "/pair approve latest", + gatewayClientScopes: INTERNAL_PAIRING_SCOPES, + senderIsOwner: true, + }), + ); + + expectApproveCalledWithInternalPairingScopes(); expect(result).toEqual({ text: "✅ Paired Victim Phone (ios)." }); }); @@ -957,7 +1073,7 @@ describe("device-pair /pair approve", () => { expect(vi.mocked(approveDevicePairing)).not.toHaveBeenCalled(); expect(result).toEqual({ - text: "⚠️ This command requires operator.pairing for internal gateway callers.", + text: "⚠️ This command requires operator.pairing.", }); }); @@ -978,24 +1094,9 @@ describe("device-pair /pair approve", () => { }); }); - it("preserves approvals for non-gateway command surfaces", async () => { + it("approves from command surfaces that carry pairing scopes", async () => { mockPendingPairingList(); - vi.mocked(approveDevicePairing).mockResolvedValueOnce( - makeApprovedPairingResult({ - device: { - scopes: ["operator.admin"], - approvedScopes: ["operator.admin"], - tokens: { - operator: { - token: "token-1", - role: "operator", - scopes: ["operator.admin"], - createdAtMs: Date.now(), - }, - }, - }, - }), - ); + vi.mocked(approveDevicePairing).mockResolvedValueOnce(makeApprovedPairingResult()); const command = registerPairCommand(); const result = await command.handler( @@ -1003,11 +1104,11 @@ describe("device-pair /pair approve", () => { channel: "telegram", args: "approve latest", commandBody: "/pair approve latest", - gatewayClientScopes: undefined, + gatewayClientScopes: INTERNAL_PAIRING_SCOPES, }), ); - expect(vi.mocked(approveDevicePairing)).toHaveBeenCalledWith("req-1"); + expectApproveCalledWithInternalPairingScopes(); expect(result).toEqual({ text: "✅ Paired Victim Phone (ios)." }); }); }); diff --git a/extensions/device-pair/index.ts b/extensions/device-pair/index.ts index b7683919873..967d8d27b5e 100644 --- a/extensions/device-pair/index.ts +++ b/extensions/device-pair/index.ts @@ -579,20 +579,6 @@ function resolveQrReplyTarget(ctx: QrCommandContext): string { ); } -const PAIR_SETUP_NON_ISSUING_ACTIONS = new Set([ - "approve", - "cleanup", - "clear", - "notify", - "pending", - "revoke", - "status", -]); - -function issuesPairSetupCode(action: string): boolean { - return !action || action === "qr" || !PAIR_SETUP_NON_ISSUING_ACTIONS.has(action); -} - async function issueSetupPayload(url: string): Promise { const { issueDeviceBootstrapToken, PAIRING_SETUP_BOOTSTRAP_PROFILE } = await loadDevicePairApiModule(); @@ -661,6 +647,7 @@ export default definePluginEntry({ name: "pair", description: "Generate setup codes and approve device pairing requests.", acceptsArgs: true, + requiredScopes: ["operator.pairing"], handler: async (ctx) => { const args = normalizeOptionalString(ctx.args) ?? ""; const tokens = args.split(/\s+/).filter(Boolean); @@ -673,6 +660,7 @@ export default definePluginEntry({ const authState = resolvePairingCommandAuthState({ channel: ctx.channel, gatewayClientScopes, + senderIsOwner: ctx.senderIsOwner, }); api.logger.info?.( `device-pair: /pair invoked channel=${ctx.channel} sender=${ctx.senderId ?? "unknown"} action=${ @@ -680,6 +668,10 @@ export default definePluginEntry({ }`, ); + if (authState.isMissingPairingPrivilege) { + return buildMissingPairingScopeReply(); + } + if (action === "status" || action === "pending") { const [{ listDevicePairing }, { formatPendingRequests }] = await Promise.all([ loadDevicePairApiModule(), @@ -700,9 +692,6 @@ export default definePluginEntry({ } if (action === "approve") { - if (authState.isMissingInternalPairingPrivilege) { - return buildMissingPairingScopeReply(); - } const [ { listDevicePairing }, { approvePendingPairingRequest, selectPendingApprovalRequest }, @@ -726,9 +715,6 @@ export default definePluginEntry({ } if (action === "cleanup" || action === "clear" || action === "revoke") { - if (authState.isMissingInternalPairingPrivilege) { - return buildMissingPairingScopeReply(); - } const { clearDeviceBootstrapTokens } = await loadDevicePairApiModule(); const cleared = await clearDeviceBootstrapTokens(); return { @@ -743,10 +729,6 @@ export default definePluginEntry({ if (authLabelResult.error) { return { text: `Error: ${authLabelResult.error}` }; } - if (issuesPairSetupCode(action) && authState.isMissingInternalPairingPrivilege) { - return buildMissingPairingScopeReply(); - } - const urlResult = await resolveMobilePairingGatewayUrl(api); if (!urlResult.url) { return { text: `Error: ${urlResult.error ?? "Gateway URL unavailable."}` }; diff --git a/extensions/device-pair/pair-command-auth.test.ts b/extensions/device-pair/pair-command-auth.test.ts index 36f209c51da..a021b359229 100644 --- a/extensions/device-pair/pair-command-auth.test.ts +++ b/extensions/device-pair/pair-command-auth.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest"; import { resolvePairingCommandAuthState } from "./pair-command-auth.js"; describe("device-pair pairing command auth", () => { - it("treats non-gateway channels as external approvals", () => { + it("fails closed for non-gateway channels without pairing scopes", () => { expect( resolvePairingCommandAuthState({ channel: "telegram", @@ -10,11 +10,25 @@ describe("device-pair pairing command auth", () => { }), ).toEqual({ isInternalGatewayCaller: false, - isMissingInternalPairingPrivilege: false, + isMissingPairingPrivilege: true, approvalCallerScopes: undefined, }); }); + it("accepts command owners on non-gateway channels", () => { + expect( + resolvePairingCommandAuthState({ + channel: "telegram", + gatewayClientScopes: undefined, + senderIsOwner: true, + }), + ).toEqual({ + isInternalGatewayCaller: false, + isMissingPairingPrivilege: false, + approvalCallerScopes: ["operator.pairing"], + }); + }); + it("fails closed for webchat when scopes are absent", () => { expect( resolvePairingCommandAuthState({ @@ -23,7 +37,7 @@ describe("device-pair pairing command auth", () => { }), ).toEqual({ isInternalGatewayCaller: true, - isMissingInternalPairingPrivilege: true, + isMissingPairingPrivilege: true, approvalCallerScopes: [], }); }); @@ -36,7 +50,7 @@ describe("device-pair pairing command auth", () => { }), ).toEqual({ isInternalGatewayCaller: true, - isMissingInternalPairingPrivilege: false, + isMissingPairingPrivilege: false, approvalCallerScopes: ["operator.write", "operator.pairing"], }); expect( @@ -46,8 +60,22 @@ describe("device-pair pairing command auth", () => { }), ).toEqual({ isInternalGatewayCaller: true, - isMissingInternalPairingPrivilege: false, + isMissingPairingPrivilege: false, approvalCallerScopes: ["operator.admin"], }); }); + + it("preserves gateway scopes for command owners with gateway scope context", () => { + expect( + resolvePairingCommandAuthState({ + channel: "telegram", + gatewayClientScopes: ["operator.write", "operator.pairing"], + senderIsOwner: true, + }), + ).toEqual({ + isInternalGatewayCaller: true, + isMissingPairingPrivilege: false, + approvalCallerScopes: ["operator.write", "operator.pairing"], + }); + }); }); diff --git a/extensions/device-pair/pair-command-auth.ts b/extensions/device-pair/pair-command-auth.ts index caac521f2ca..c70e4ef626a 100644 --- a/extensions/device-pair/pair-command-auth.ts +++ b/extensions/device-pair/pair-command-auth.ts @@ -1,14 +1,17 @@ type PairingCommandAuthParams = { channel: string; gatewayClientScopes?: readonly string[] | null; + senderIsOwner?: boolean; }; type PairingCommandAuthState = { isInternalGatewayCaller: boolean; - isMissingInternalPairingPrivilege: boolean; + isMissingPairingPrivilege: boolean; approvalCallerScopes?: readonly string[]; }; +const COMMAND_OWNER_PAIRING_SCOPES = ["operator.pairing"] as const; + function isInternalGatewayPairingCaller(params: PairingCommandAuthParams): boolean { return params.channel === "webchat" || Array.isArray(params.gatewayClientScopes); } @@ -17,30 +20,38 @@ export function resolvePairingCommandAuthState( params: PairingCommandAuthParams, ): PairingCommandAuthState { const isInternalGatewayCaller = isInternalGatewayPairingCaller(params); - if (!isInternalGatewayCaller) { + if (isInternalGatewayCaller) { + const approvalCallerScopes = Array.isArray(params.gatewayClientScopes) + ? params.gatewayClientScopes + : []; + const isMissingPairingPrivilege = + !approvalCallerScopes.includes("operator.pairing") && + !approvalCallerScopes.includes("operator.admin"); + return { isInternalGatewayCaller, - isMissingInternalPairingPrivilege: false, - approvalCallerScopes: undefined, + isMissingPairingPrivilege, + approvalCallerScopes, }; } - const approvalCallerScopes = Array.isArray(params.gatewayClientScopes) - ? params.gatewayClientScopes - : []; - const isMissingInternalPairingPrivilege = - !approvalCallerScopes.includes("operator.pairing") && - !approvalCallerScopes.includes("operator.admin"); + if (params.senderIsOwner === true) { + return { + isInternalGatewayCaller, + isMissingPairingPrivilege: false, + approvalCallerScopes: COMMAND_OWNER_PAIRING_SCOPES, + }; + } return { isInternalGatewayCaller, - isMissingInternalPairingPrivilege, - approvalCallerScopes, + isMissingPairingPrivilege: true, + approvalCallerScopes: undefined, }; } export function buildMissingPairingScopeReply(): { text: string } { return { - text: "⚠️ This command requires operator.pairing for internal gateway callers.", + text: "⚠️ This command requires operator.pairing.", }; } diff --git a/extensions/discord/src/monitor/native-command.plugin-dispatch.test.ts b/extensions/discord/src/monitor/native-command.plugin-dispatch.test.ts index dbd7bd65b56..184592537c6 100644 --- a/extensions/discord/src/monitor/native-command.plugin-dispatch.test.ts +++ b/extensions/discord/src/monitor/native-command.plugin-dispatch.test.ts @@ -228,6 +228,22 @@ function registerPairPlugin(params?: { discordNativeName?: string }) { ).toEqual({ ok: true }); } +function registerScopedPairPlugin( + handler = vi.fn(async ({ args }: { args?: string }) => ({ text: `paired:${args ?? ""}` })), +) { + expect( + registerPluginCommand("demo-plugin", { + name: "pair", + description: "Pair device", + acceptsArgs: true, + requireAuth: false, + requiredScopes: ["operator.pairing"], + handler, + }), + ).toEqual({ ok: true }); + return handler; +} + async function expectPairCommandReply(params: { cfg: OpenClawConfig; commandName: string; @@ -389,6 +405,73 @@ describe("Discord native plugin command dispatch", () => { }); }); + it("does not treat Discord DM allowlist users as scoped plugin command owners", async () => { + const cfg = { + channels: { + discord: { + dm: { enabled: true, policy: "open", allowFrom: ["user:owner"] }, + }, + }, + } as OpenClawConfig; + const interaction = createInteraction(); + interaction.options.getString.mockReturnValue("now"); + const handler = registerScopedPairPlugin(); + const command = await createPluginCommand({ cfg, name: "pair" }); + + await (command as { run: (interaction: unknown) => Promise }).run(interaction as unknown); + + expect(handler).not.toHaveBeenCalled(); + expect(interaction.followUp).toHaveBeenCalledWith( + expect.objectContaining({ + content: "⚠️ This command requires gateway scope: operator.pairing.", + }), + ); + expect(interaction.reply).not.toHaveBeenCalled(); + }); + + it("allows generic command owners to run scoped Discord plugin commands without gateway scopes", async () => { + const cfg = { + commands: { + ownerAllowFrom: ["discord:123456789012345678"], + }, + channels: { + discord: { + dm: { enabled: true, policy: "open", allowFrom: ["*"] }, + }, + }, + } as OpenClawConfig; + const interaction = createInteraction({ userId: "123456789012345678" }); + interaction.options.getString.mockReturnValue("now"); + const handler = registerScopedPairPlugin(); + const command = await createPluginCommand({ cfg, name: "pair" }); + + await (command as { run: (interaction: unknown) => Promise }).run(interaction as unknown); + + expect(handler).toHaveBeenCalledTimes(1); + expect(interaction.followUp).toHaveBeenCalledWith( + expect.objectContaining({ content: "paired:now" }), + ); + expect(interaction.reply).not.toHaveBeenCalled(); + }); + + it("rejects authorized Discord non-owners for scoped plugin commands without gateway scopes", async () => { + const cfg = createConfig(); + const interaction = createInteraction({ userId: "authorized-non-owner" }); + interaction.options.getString.mockReturnValue("now"); + const handler = registerScopedPairPlugin(); + const command = await createPluginCommand({ cfg, name: "pair" }); + + await (command as { run: (interaction: unknown) => Promise }).run(interaction as unknown); + + expect(handler).not.toHaveBeenCalled(); + expect(interaction.followUp).toHaveBeenCalledWith( + expect.objectContaining({ + content: "⚠️ This command requires gateway scope: operator.pairing.", + }), + ); + expect(interaction.reply).not.toHaveBeenCalled(); + }); + it("blocks unauthorized Discord senders before requireAuth:false plugin commands execute", async () => { const cfg = { commands: { @@ -455,6 +538,132 @@ describe("Discord native plugin command dispatch", () => { expect(interaction.reply).not.toHaveBeenCalled(); }); + it("ignores non-Discord generic command owners when authorizing guild plugin commands", async () => { + const cfg = { + commands: { + ownerAllowFrom: ["telegram:123456789"], + }, + channels: { + discord: { + groupPolicy: "allowlist", + guilds: { + "345678901234567890": { + channels: { + "234567890123456789": { + enabled: true, + requireMention: false, + }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + const commandSpec: NativeCommandSpec = { + name: "pair", + description: "Pair", + acceptsArgs: true, + }; + const interaction = createInteraction({ + channelType: ChannelType.GuildText, + channelId: "234567890123456789", + guildId: "345678901234567890", + guildName: "Test Guild", + }); + interaction.user.id = "999999999999999999"; + interaction.options.getString.mockReturnValue("now"); + + expect( + registerPluginCommand("demo-plugin", { + name: "pair", + description: "Pair device", + acceptsArgs: true, + requireAuth: false, + handler: async ({ args }) => ({ text: `open:${args ?? ""}` }), + }), + ).toEqual({ ok: true }); + const executeSpy = runtimeModuleMocks.executePluginCommand.mockResolvedValue({ + text: "open:now", + }); + const command = await createNativeCommand(cfg, commandSpec); + + await (command as { run: (interaction: unknown) => Promise }).run(interaction as unknown); + + expect(executeSpy).toHaveBeenCalledWith( + expect.objectContaining({ + command: expect.objectContaining({ name: "pair" }), + args: "now", + }), + ); + expect(interaction.followUp).toHaveBeenCalledWith( + expect.objectContaining({ content: "open:now" }), + ); + expect(interaction.reply).not.toHaveBeenCalled(); + }); + + it("keeps non-matching Discord command owners from restricting guild plugin commands", async () => { + const cfg = { + commands: { + ownerAllowFrom: ["discord:123456789012345678"], + }, + channels: { + discord: { + groupPolicy: "allowlist", + guilds: { + "345678901234567890": { + channels: { + "234567890123456789": { + enabled: true, + requireMention: false, + }, + }, + }, + }, + }, + }, + } as OpenClawConfig; + const commandSpec: NativeCommandSpec = { + name: "pair", + description: "Pair", + acceptsArgs: true, + }; + const interaction = createInteraction({ + channelType: ChannelType.GuildText, + channelId: "234567890123456789", + guildId: "345678901234567890", + guildName: "Test Guild", + }); + interaction.user.id = "999999999999999999"; + interaction.options.getString.mockReturnValue("now"); + + expect( + registerPluginCommand("demo-plugin", { + name: "pair", + description: "Pair device", + acceptsArgs: true, + requireAuth: false, + handler: async ({ args }) => ({ text: `open:${args ?? ""}` }), + }), + ).toEqual({ ok: true }); + const executeSpy = runtimeModuleMocks.executePluginCommand.mockResolvedValue({ + text: "open:now", + }); + const command = await createNativeCommand(cfg, commandSpec); + + await (command as { run: (interaction: unknown) => Promise }).run(interaction as unknown); + + expect(executeSpy).toHaveBeenCalledWith( + expect.objectContaining({ + command: expect.objectContaining({ name: "pair" }), + args: "now", + }), + ); + expect(interaction.followUp).toHaveBeenCalledWith( + expect.objectContaining({ content: "open:now" }), + ); + expect(interaction.reply).not.toHaveBeenCalled(); + }); + it("rejects group DM slash commands outside dm.groupChannels before dispatch", async () => { const cfg = { commands: { diff --git a/extensions/discord/src/monitor/native-command.ts b/extensions/discord/src/monitor/native-command.ts index ad4527577f6..c0c72097a77 100644 --- a/extensions/discord/src/monitor/native-command.ts +++ b/extensions/discord/src/monitor/native-command.ts @@ -16,6 +16,7 @@ import { import { resolveChunkMode, resolveTextChunkLimit } from "openclaw/plugin-sdk/reply-chunking"; import { createSubsystemLogger, logVerbose } from "openclaw/plugin-sdk/runtime-env"; import { resolveOpenProviderRuntimeGroupPolicy } from "openclaw/plugin-sdk/runtime-group-policy"; +import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime"; import { resolveDiscordAccountAllowFrom, resolveDiscordAccountDmPolicy, @@ -85,6 +86,36 @@ import type { ThreadBindingManager } from "./thread-bindings.js"; const log = createSubsystemLogger("discord/native-command"); export { __testing } from "./native-command.runtime.js"; +function resolveDiscordCommandOwnerAllowFrom(cfg: OpenClawConfig): string[] | undefined { + const raw = cfg.commands?.ownerAllowFrom; + if (!Array.isArray(raw) || raw.length === 0) { + return undefined; + } + const entries: string[] = []; + for (const entry of raw) { + const trimmed = normalizeOptionalString(String(entry ?? "")) ?? ""; + if (!trimmed) { + continue; + } + const separatorIndex = trimmed.indexOf(":"); + if (separatorIndex > 0) { + const prefix = trimmed.slice(0, separatorIndex).toLowerCase(); + if (prefix === "discord") { + const remainder = normalizeOptionalString(trimmed.slice(separatorIndex + 1)) ?? ""; + if (remainder) { + entries.push(remainder); + } + continue; + } + if (prefix !== "user" && prefix !== "pk") { + continue; + } + } + entries.push(trimmed); + } + return entries.length > 0 ? entries : undefined; +} + export function createDiscordNativeCommand(params: { command: NativeCommandSpec; cfg: OpenClawConfig; @@ -270,8 +301,19 @@ async function dispatchDiscordCommandInteraction(params: { cfg, accountId, }) ?? []; - const { ownerAllowList, ownerAllowed: ownerOk } = resolveDiscordOwnerAccess({ - allowFrom: configuredDmAllowFrom, + const commandOwnerAllowFrom = resolveDiscordCommandOwnerAllowFrom(cfg); + const { ownerAllowList: discordOwnerAllowList, ownerAllowed: discordOwnerOk } = + resolveDiscordOwnerAccess({ + allowFrom: configuredDmAllowFrom, + sender: { + id: sender.id, + name: sender.name, + tag: sender.tag, + }, + allowNameMatching, + }); + const { ownerAllowed: commandOwnerOk } = resolveDiscordOwnerAccess({ + allowFrom: commandOwnerAllowFrom, sender: { id: sender.id, name: sender.name, @@ -279,6 +321,10 @@ async function dispatchDiscordCommandInteraction(params: { }, allowNameMatching, }); + const commandOwnerAllowAll = commandOwnerAllowFrom?.includes("*") === true; + const senderIsCommandOwner = commandOwnerOk || commandOwnerAllowAll; + const ownerAllowListConfigured = discordOwnerAllowList != null; + const ownerOk = discordOwnerOk; const commandsAllowFromAccess = resolveDiscordNativeCommandAllowlistAccess({ cfg, accountId, @@ -447,7 +493,7 @@ async function dispatchDiscordCommandInteraction(params: { memberRoleIds, sender, allowNameMatching, - ownerAllowListConfigured: ownerAllowList != null, + ownerAllowListConfigured, ownerAllowed: ownerOk, }); if (!commandAuthorized && !(await canBypassConfiguredAcpGuildGuards())) { @@ -527,6 +573,7 @@ async function dispatchDiscordCommandInteraction(params: { channel: "discord", channelId, isAuthorizedSender: commandAuthorized, + senderIsOwner: senderIsCommandOwner, sessionKey: effectiveRoute.sessionKey, commandBody: prompt, config: cfg, @@ -611,7 +658,7 @@ async function dispatchDiscordCommandInteraction(params: { commandTargetSessionKey, channel: "discord", senderId: sender.id, - senderIsOwner: ownerOk, + senderIsOwner: senderIsCommandOwner, isAuthorizedSender: commandAuthorized, isGroup: isGuild || isGroupDm, defaultGroupActivation: () => diff --git a/src/plugins/commands.test.ts b/src/plugins/commands.test.ts index b56d4879d32..8dcb113ee0e 100644 --- a/src/plugins/commands.test.ts +++ b/src/plugins/commands.test.ts @@ -1,5 +1,5 @@ import path from "node:path"; -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createChannelTestPluginBase, createTestRegistry } from "../test-utils/channel-plugins.js"; import { listRegisteredPluginAgentPromptGuidance } from "./command-registry-state.js"; import { @@ -608,6 +608,84 @@ describe("registerPluginCommand", () => { expect(observedOwnerStatus).toBeUndefined(); }); + it("allows command owners to run scoped plugin commands without gateway scopes", async () => { + let observedOwnerStatus: boolean | undefined; + const handler = vi.fn(async (ctx: { senderIsOwner?: boolean }) => { + observedOwnerStatus = ctx.senderIsOwner; + return { text: "ok" }; + }); + registerPluginCommand("demo-plugin", { + name: "pairlike", + description: "Scoped command", + requiredScopes: ["operator.pairing"], + handler, + }); + const match = matchPluginCommand("/pairlike"); + expect(match).toBeTruthy(); + + const result = await executePluginCommand({ + command: match!.command, + channel: "telegram", + isAuthorizedSender: true, + senderIsOwner: true, + commandBody: "/pairlike", + config: {}, + }); + + expect(result).toEqual({ text: "ok" }); + expect(handler).toHaveBeenCalledTimes(1); + expect(observedOwnerStatus).toBe(true); + }); + + it("rejects command owners when explicit gateway scopes miss the required scope", async () => { + const handler = vi.fn(async () => ({ text: "ok" })); + registerPluginCommand("demo-plugin", { + name: "pairlike", + description: "Scoped command", + requiredScopes: ["operator.pairing"], + handler, + }); + const match = matchPluginCommand("/pairlike"); + expect(match).toBeTruthy(); + + const result = await executePluginCommand({ + command: match!.command, + channel: "webchat", + isAuthorizedSender: true, + senderIsOwner: true, + commandBody: "/pairlike", + gatewayClientScopes: ["operator.write"], + config: {}, + }); + + expect(result).toEqual({ text: "⚠️ This command requires gateway scope: operator.pairing." }); + expect(handler).not.toHaveBeenCalled(); + }); + + it("rejects non-owner scoped plugin commands without gateway scopes", async () => { + const handler = vi.fn(async () => ({ text: "ok" })); + registerPluginCommand("demo-plugin", { + name: "pairlike", + description: "Scoped command", + requiredScopes: ["operator.pairing"], + handler, + }); + const match = matchPluginCommand("/pairlike"); + expect(match).toBeTruthy(); + + const result = await executePluginCommand({ + command: match!.command, + channel: "telegram", + isAuthorizedSender: true, + senderIsOwner: false, + commandBody: "/pairlike", + config: {}, + }); + + expect(result).toEqual({ text: "⚠️ This command requires gateway scope: operator.pairing." }); + expect(handler).not.toHaveBeenCalled(); + }); + it("skips direct plugin command execution on unsupported channels", async () => { let handlerCalled = false; const handler = async () => { diff --git a/src/plugins/commands.ts b/src/plugins/commands.ts index bc438c3829e..db7f042d07c 100644 --- a/src/plugins/commands.ts +++ b/src/plugins/commands.ts @@ -223,11 +223,17 @@ export async function executePluginCommand(params: { logVerbose(`Plugin command /${command.name} blocked: unknown gateway scope`); return { text: "⚠️ This command has invalid gateway scope configuration." }; } - if (requiredScopes.length > 0 && params.gatewayClientScopes) { - const scopes = new Set(params.gatewayClientScopes ?? []); - const hasAdmin = scopes.has(ADMIN_SCOPE); - const missingScope = requiredScopes.find((scope) => !hasAdmin && !scopes.has(scope)); - if (missingScope) { + if (requiredScopes.length > 0) { + const senderIsOwner = params.senderIsOwner === true; + const scopes = Array.isArray(params.gatewayClientScopes) + ? new Set(params.gatewayClientScopes) + : undefined; + const hasGatewayScopeContext = scopes !== undefined; + const hasAdmin = scopes?.has(ADMIN_SCOPE) === true; + const missingScope = scopes + ? requiredScopes.find((scope) => !hasAdmin && !scopes.has(scope)) + : requiredScopes[0]; + if (missingScope && (hasGatewayScopeContext || !senderIsOwner)) { logVerbose(`Plugin command /${command.name} blocked: missing gateway scope ${missingScope}`); return { text: `⚠️ This command requires gateway scope: ${missingScope}.` }; } @@ -247,10 +253,11 @@ export async function executePluginCommand(params: { }); const effectiveAccountId = bindingConversation?.accountId ?? params.accountId; const senderIsOwnerForCommand = - isTrustedReservedCommandOwner(command) && - command.ownership === "reserved" && - isReservedCommandName(command.name) && - command.pluginId === normalizeLowercaseStringOrEmpty(command.name) + requiredScopes.length > 0 || + (isTrustedReservedCommandOwner(command) && + command.ownership === "reserved" && + isReservedCommandName(command.name) && + command.pluginId === normalizeLowercaseStringOrEmpty(command.name)) ? params.senderIsOwner : undefined; const diagnosticsPrivateRoutedForCommand = diff --git a/src/plugins/contracts/host-hooks.contract.test.ts b/src/plugins/contracts/host-hooks.contract.test.ts index acdfac4be56..108ba0e0624 100644 --- a/src/plugins/contracts/host-hooks.contract.test.ts +++ b/src/plugins/contracts/host-hooks.contract.test.ts @@ -1305,7 +1305,7 @@ describe("host-hook fixture plugin contract", () => { expect(validatePluginsUiDescriptorsParams({ pluginId: "host-hook-fixture" })).toBe(false); }); - it("enforces command requiredScopes for gateway clients while preserving text command continuations", async () => { + it("enforces command requiredScopes for gateway clients and command owners", async () => { const handlerCalls: string[] = []; const { config, registry } = createPluginRegistryFixture(); registerTestPlugin({ @@ -1360,6 +1360,7 @@ describe("host-hook fixture plugin contract", () => { senderId: "owner", channel: "whatsapp", isAuthorizedSender: true, + senderIsOwner: true, sessionKey: "agent:main:main", commandBody: "/approval-fixture resume-text", config, diff --git a/src/plugins/types.ts b/src/plugins/types.ts index 2280ecbca75..b7df4b8b781 100644 --- a/src/plugins/types.ts +++ b/src/plugins/types.ts @@ -1980,7 +1980,7 @@ export type OpenClawPluginCommandDefinition = { acceptsArgs?: boolean; /** Whether only authorized senders can use this command (default: true) */ requireAuth?: boolean; - /** Gateway operator scopes required when invoked through an internal gateway client. */ + /** Operator scopes required by gateway clients; command owners may satisfy this on chat surfaces. */ requiredScopes?: OperatorScope[]; /** * Allows a bundled plugin to claim a command name that is otherwise reserved