mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-18 19:04:45 +00:00
fix(qqbot): authorize approval button callbacks [AI] (#80892)
* fix: authorize qqbot approval buttons * fix: authorize qqbot approval buttons * addressing claude review * docs: add changelog entry for PR merge
This commit is contained in:
committed by
GitHub
parent
2ec1a27c9f
commit
6e498a1f62
@@ -6,6 +6,7 @@ Docs: https://docs.openclaw.ai
|
||||
|
||||
### Fixes
|
||||
|
||||
- fix(qqbot): authorize approval button callbacks [AI]. (#80892) Thanks @pgondhi987.
|
||||
- Scrub streamable MCP redirect headers [AI]. (#80906) Thanks @pgondhi987.
|
||||
- fix(memory-wiki): require admin scope for ingest [AI]. (#80897) Thanks @pgondhi987.
|
||||
- memory-wiki: require write scope for Obsidian search [AI]. (#80904) Thanks @pgondhi987.
|
||||
|
||||
@@ -1,10 +1,9 @@
|
||||
/**
|
||||
* QQ Bot Approval Capability — entry point.
|
||||
*
|
||||
* QQBot uses a simpler approval model than Telegram/Slack: any user who
|
||||
* can see the inline-keyboard buttons can approve. No explicit approver
|
||||
* list is required — the bot simply sends the approval message to the
|
||||
* originating conversation and whoever clicks the button resolves it.
|
||||
* QQBot uses a simpler approval model than Telegram/Slack: when no
|
||||
* approver list is configured, the bot sends the approval message to the
|
||||
* originating conversation and any participant can approve from there.
|
||||
*
|
||||
* When `execApprovals` IS configured, it gates which requests are
|
||||
* handled natively and who is authorized. When it is NOT configured,
|
||||
@@ -23,9 +22,8 @@ import {
|
||||
isQQBotExecApprovalClientEnabled,
|
||||
matchesQQBotApprovalAccount,
|
||||
shouldHandleQQBotExecApprovalRequest,
|
||||
isQQBotExecApprovalAuthorizedSender,
|
||||
isQQBotExecApprovalApprover,
|
||||
resolveQQBotExecApprovalConfig,
|
||||
authorizeQQBotApprovalAction,
|
||||
} from "../../exec-approvals.js";
|
||||
import { ensurePlatformAdapter } from "../bootstrap.js";
|
||||
import { resolveQQBotAccount } from "../config.js";
|
||||
@@ -103,18 +101,8 @@ function canResolveTarget(request: {
|
||||
|
||||
function createQQBotApprovalCapability(): ChannelApprovalCapability {
|
||||
return createChannelApprovalCapability({
|
||||
authorizeActorAction: ({ cfg, accountId, senderId, approvalKind }) => {
|
||||
if (hasExecApprovalConfig({ cfg, accountId })) {
|
||||
const authorized =
|
||||
approvalKind === "plugin"
|
||||
? isQQBotExecApprovalApprover({ cfg, accountId, senderId })
|
||||
: isQQBotExecApprovalAuthorizedSender({ cfg, accountId, senderId });
|
||||
return authorized
|
||||
? { authorized: true }
|
||||
: { authorized: false, reason: "You are not authorized to approve this request." };
|
||||
}
|
||||
return { authorized: true };
|
||||
},
|
||||
authorizeActorAction: ({ cfg, accountId, senderId, approvalKind }) =>
|
||||
authorizeQQBotApprovalAction({ cfg, accountId, senderId, approvalKind }),
|
||||
|
||||
getActionAvailabilityState: ({
|
||||
cfg,
|
||||
|
||||
@@ -181,7 +181,9 @@ export async function startGateway(ctx: CoreGatewayContext): Promise<void> {
|
||||
}
|
||||
};
|
||||
|
||||
const handleInteraction = createInteractionHandler(account, ctx.runtime, log);
|
||||
const handleInteraction = createInteractionHandler(account, ctx.runtime, log, {
|
||||
getActiveCfg: () => activeCfgProvider.getActiveCfg(),
|
||||
});
|
||||
|
||||
const connection = new GatewayConnection({
|
||||
account,
|
||||
|
||||
206
extensions/qqbot/src/engine/gateway/interaction-handler.test.ts
Normal file
206
extensions/qqbot/src/engine/gateway/interaction-handler.test.ts
Normal file
@@ -0,0 +1,206 @@
|
||||
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { registerPlatformAdapter, type PlatformAdapter } from "../adapter/index.js";
|
||||
import type { InteractionEvent } from "../types.js";
|
||||
import { createInteractionHandler } from "./interaction-handler.js";
|
||||
import type { GatewayAccount, GatewayPluginRuntime } from "./types.js";
|
||||
|
||||
const acknowledgeInteractionMock = vi.hoisted(() => vi.fn(async () => undefined));
|
||||
|
||||
vi.mock("../messaging/sender.js", () => ({
|
||||
accountToCreds: (account: GatewayAccount) => ({
|
||||
appId: account.appId,
|
||||
clientSecret: account.clientSecret,
|
||||
}),
|
||||
acknowledgeInteraction: acknowledgeInteractionMock,
|
||||
}));
|
||||
|
||||
const resolveApprovalMock = vi.fn(async () => true);
|
||||
|
||||
const account: GatewayAccount = {
|
||||
accountId: "default",
|
||||
appId: "app",
|
||||
clientSecret: "secret",
|
||||
markdownSupport: false,
|
||||
config: {},
|
||||
};
|
||||
|
||||
const runtime = {} as GatewayPluginRuntime;
|
||||
|
||||
function makeRestrictedCfg(approvers: string[]): OpenClawConfig {
|
||||
return {
|
||||
channels: {
|
||||
qqbot: {
|
||||
appId: "app",
|
||||
clientSecret: "secret",
|
||||
execApprovals: {
|
||||
enabled: true,
|
||||
approvers,
|
||||
},
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
}
|
||||
|
||||
function makeUnrestrictedCfg(): OpenClawConfig {
|
||||
return {
|
||||
channels: {
|
||||
qqbot: {
|
||||
appId: "app",
|
||||
clientSecret: "secret",
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
}
|
||||
|
||||
function makeApprovalEvent(overrides: Partial<InteractionEvent> = {}): InteractionEvent {
|
||||
return {
|
||||
id: "interaction-1",
|
||||
type: 11,
|
||||
chat_type: 1,
|
||||
group_openid: "group-1",
|
||||
group_member_openid: "ATTACKER_OPENID",
|
||||
version: 1,
|
||||
data: {
|
||||
type: 11,
|
||||
resolved: {
|
||||
button_data: "approve:exec:abc12345:allow-once",
|
||||
user_id: "ATTACKER_USER_ID",
|
||||
},
|
||||
},
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function installPlatformAdapter(): void {
|
||||
registerPlatformAdapter({
|
||||
validateRemoteUrl: vi.fn(async () => undefined),
|
||||
resolveSecret: vi.fn(async (value: unknown) => (typeof value === "string" ? value : undefined)),
|
||||
downloadFile: vi.fn(async () => "/tmp/file"),
|
||||
fetchMedia: vi.fn(async () => {
|
||||
throw new Error("unused");
|
||||
}),
|
||||
getTempDir: () => "/tmp",
|
||||
hasConfiguredSecret: (value: unknown) => typeof value === "string" && value.length > 0,
|
||||
normalizeSecretInputString: (value: unknown) => (typeof value === "string" ? value : undefined),
|
||||
resolveSecretInputString: ({ value }: { value: unknown }) =>
|
||||
typeof value === "string" ? value : undefined,
|
||||
resolveApproval: resolveApprovalMock,
|
||||
} as PlatformAdapter);
|
||||
}
|
||||
|
||||
describe("createInteractionHandler approval buttons", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
installPlatformAdapter();
|
||||
});
|
||||
|
||||
it("rejects approval button clicks from users outside the configured approvers", async () => {
|
||||
const handler = createInteractionHandler(account, runtime, undefined, {
|
||||
getActiveCfg: () => makeRestrictedCfg(["OWNER_OPENID"]),
|
||||
});
|
||||
|
||||
handler(makeApprovalEvent());
|
||||
|
||||
await vi.waitFor(() => expect(acknowledgeInteractionMock).toHaveBeenCalled());
|
||||
|
||||
expect(acknowledgeInteractionMock).toHaveBeenCalledWith(
|
||||
{ appId: "app", clientSecret: "secret" },
|
||||
"interaction-1",
|
||||
0,
|
||||
{ content: "You are not authorized to approve this request." },
|
||||
);
|
||||
expect(resolveApprovalMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not authorize from resolved user id when the actor openid is not approved", async () => {
|
||||
const handler = createInteractionHandler(account, runtime, undefined, {
|
||||
getActiveCfg: () => makeRestrictedCfg(["OWNER_OPENID"]),
|
||||
});
|
||||
|
||||
handler(
|
||||
makeApprovalEvent({
|
||||
data: {
|
||||
type: 11,
|
||||
resolved: {
|
||||
button_data: "approve:exec:abc12345:allow-once",
|
||||
user_id: "OWNER_OPENID",
|
||||
},
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
await vi.waitFor(() => expect(acknowledgeInteractionMock).toHaveBeenCalled());
|
||||
|
||||
expect(acknowledgeInteractionMock).toHaveBeenCalledWith(
|
||||
{ appId: "app", clientSecret: "secret" },
|
||||
"interaction-1",
|
||||
0,
|
||||
{ content: "You are not authorized to approve this request." },
|
||||
);
|
||||
expect(resolveApprovalMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("resolves approval button clicks from configured approvers", async () => {
|
||||
const handler = createInteractionHandler(account, runtime, undefined, {
|
||||
getActiveCfg: () => makeRestrictedCfg(["OWNER_OPENID"]),
|
||||
});
|
||||
|
||||
handler(makeApprovalEvent({ group_member_openid: "OWNER_OPENID" }));
|
||||
|
||||
await vi.waitFor(() =>
|
||||
expect(resolveApprovalMock).toHaveBeenCalledWith("exec:abc12345", "allow-once"),
|
||||
);
|
||||
});
|
||||
|
||||
it("uses the direct user openid when a group member openid is unavailable", async () => {
|
||||
const handler = createInteractionHandler(account, runtime, undefined, {
|
||||
getActiveCfg: () => makeRestrictedCfg(["OWNER_OPENID"]),
|
||||
});
|
||||
|
||||
handler(
|
||||
makeApprovalEvent({
|
||||
chat_type: 2,
|
||||
group_openid: undefined,
|
||||
group_member_openid: undefined,
|
||||
user_openid: "OWNER_OPENID",
|
||||
}),
|
||||
);
|
||||
|
||||
await vi.waitFor(() =>
|
||||
expect(resolveApprovalMock).toHaveBeenCalledWith("exec:abc12345", "allow-once"),
|
||||
);
|
||||
});
|
||||
|
||||
it("allows approval button clicks when exec approvals are not configured", async () => {
|
||||
const handler = createInteractionHandler(account, runtime, undefined, {
|
||||
getActiveCfg: () => makeUnrestrictedCfg(),
|
||||
});
|
||||
|
||||
handler(makeApprovalEvent());
|
||||
|
||||
await vi.waitFor(() =>
|
||||
expect(resolveApprovalMock).toHaveBeenCalledWith("exec:abc12345", "allow-once"),
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects approval button clicks when active config cannot be loaded", async () => {
|
||||
const handler = createInteractionHandler(account, runtime, undefined, {
|
||||
getActiveCfg: () => {
|
||||
throw new Error("config unavailable");
|
||||
},
|
||||
});
|
||||
|
||||
handler(makeApprovalEvent());
|
||||
|
||||
await vi.waitFor(() => expect(acknowledgeInteractionMock).toHaveBeenCalled());
|
||||
|
||||
expect(acknowledgeInteractionMock).toHaveBeenCalledWith(
|
||||
{ appId: "app", clientSecret: "secret" },
|
||||
"interaction-1",
|
||||
0,
|
||||
{ content: "Approval is unavailable." },
|
||||
);
|
||||
expect(resolveApprovalMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -5,12 +5,14 @@
|
||||
*
|
||||
* 1. **Config query** (type=2001) — reads config, ACKs with `claw_cfg`.
|
||||
* 2. **Config update** (type=2002) — writes config, ACKs with updated snapshot.
|
||||
* 3. **Approval button** (other) — ACKs, resolves approval via PlatformAdapter.
|
||||
* 3. **Approval button** (other) — ACKs, resolves authorized approval actions.
|
||||
*
|
||||
* Config query/update require `runtime.config`. When unavailable, those
|
||||
* branches fall through to a bare ACK (backward-compatible).
|
||||
*/
|
||||
|
||||
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts";
|
||||
import { authorizeQQBotApprovalAction } from "../../exec-approvals.js";
|
||||
import { resolveQQBotEffectivePolicies } from "../access/resolve-policy.js";
|
||||
import { getPlatformAdapter } from "../adapter/index.js";
|
||||
import { parseApprovalButtonData } from "../approval/index.js";
|
||||
@@ -151,6 +153,7 @@ export function createInteractionHandler(
|
||||
account: GatewayAccount,
|
||||
runtime: GatewayPluginRuntime,
|
||||
log?: EngineLogger,
|
||||
options?: { getActiveCfg?: () => OpenClawConfig },
|
||||
): (event: InteractionEvent) => void {
|
||||
return (event) => {
|
||||
const creds = accountToCreds(account);
|
||||
@@ -174,33 +177,155 @@ export function createInteractionHandler(
|
||||
}
|
||||
|
||||
// ---- Approval button / other ----
|
||||
void acknowledgeInteraction(creds, event.id).catch((err) => {
|
||||
log?.error(`Interaction ACK failed: ${err instanceof Error ? err.message : String(err)}`);
|
||||
});
|
||||
|
||||
const parsed = parseApprovalButtonData(event.data?.resolved?.button_data ?? "");
|
||||
if (!parsed) {
|
||||
void acknowledgeInteraction(creds, event.id).catch((err) => {
|
||||
log?.error(`Interaction ACK failed: ${err instanceof Error ? err.message : String(err)}`);
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
const adapter = getPlatformAdapter();
|
||||
if (!adapter.resolveApproval) {
|
||||
log?.error("resolveApproval not available on PlatformAdapter");
|
||||
return;
|
||||
}
|
||||
|
||||
void adapter.resolveApproval(parsed.approvalId, parsed.decision).then((ok) => {
|
||||
if (ok) {
|
||||
log?.info(`Approval resolved: id=${parsed.approvalId}, decision=${parsed.decision}`);
|
||||
} else {
|
||||
log?.error(`Approval resolve failed: id=${parsed.approvalId}`);
|
||||
}
|
||||
void handleApprovalButtonInteraction({
|
||||
accountId: account.accountId,
|
||||
creds,
|
||||
event,
|
||||
getActiveCfg: options?.getActiveCfg ?? runtime.config?.current,
|
||||
log,
|
||||
parsed,
|
||||
});
|
||||
};
|
||||
}
|
||||
|
||||
// ============ Helpers ============
|
||||
|
||||
async function handleApprovalButtonInteraction(params: {
|
||||
accountId: string;
|
||||
creds: { appId: string; clientSecret: string };
|
||||
event: InteractionEvent;
|
||||
getActiveCfg?: () => OpenClawConfig | Record<string, unknown>;
|
||||
log?: EngineLogger;
|
||||
parsed: { approvalId: string; decision: "allow-once" | "allow-always" | "deny" };
|
||||
}): Promise<void> {
|
||||
if (!params.getActiveCfg) {
|
||||
await acknowledgeApprovalInteraction(params.creds, params.event, params.log, {
|
||||
content: "Approval is unavailable.",
|
||||
});
|
||||
params.log?.error("Approval button rejected: active config is unavailable");
|
||||
return;
|
||||
}
|
||||
|
||||
let cfg: OpenClawConfig;
|
||||
try {
|
||||
cfg = params.getActiveCfg() as OpenClawConfig;
|
||||
} catch (err) {
|
||||
await acknowledgeApprovalInteraction(params.creds, params.event, params.log, {
|
||||
content: "Approval is unavailable.",
|
||||
});
|
||||
params.log?.error(
|
||||
`Approval button rejected: active config failed to load: ${
|
||||
err instanceof Error ? err.message : String(err)
|
||||
}`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const authorization = authorizeApprovalButtonActor({
|
||||
cfg,
|
||||
accountId: params.accountId,
|
||||
event: params.event,
|
||||
approvalKind: resolveApprovalKind(params.parsed.approvalId),
|
||||
});
|
||||
if (!authorization.authorized) {
|
||||
await acknowledgeApprovalInteraction(params.creds, params.event, params.log, {
|
||||
content: authorization.reason ?? "You are not authorized to approve this request.",
|
||||
});
|
||||
params.log?.info(`Approval button rejected: id=${params.parsed.approvalId}`);
|
||||
return;
|
||||
}
|
||||
|
||||
await acknowledgeApprovalInteraction(params.creds, params.event, params.log);
|
||||
|
||||
const adapter = getPlatformAdapter();
|
||||
if (!adapter.resolveApproval) {
|
||||
params.log?.error("resolveApproval not available on PlatformAdapter");
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const ok = await adapter.resolveApproval(params.parsed.approvalId, params.parsed.decision);
|
||||
if (ok) {
|
||||
params.log?.info(
|
||||
`Approval resolved: id=${params.parsed.approvalId}, decision=${params.parsed.decision}`,
|
||||
);
|
||||
} else {
|
||||
params.log?.error(`Approval resolve failed: id=${params.parsed.approvalId}`);
|
||||
}
|
||||
} catch (err) {
|
||||
params.log?.error(
|
||||
`Approval resolve failed: id=${params.parsed.approvalId}: ${
|
||||
err instanceof Error ? err.message : String(err)
|
||||
}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async function acknowledgeApprovalInteraction(
|
||||
creds: { appId: string; clientSecret: string },
|
||||
event: InteractionEvent,
|
||||
log: EngineLogger | undefined,
|
||||
data?: Record<string, unknown>,
|
||||
): Promise<void> {
|
||||
try {
|
||||
await acknowledgeInteraction(creds, event.id, 0, data);
|
||||
} catch (err) {
|
||||
log?.error(`Interaction ACK failed: ${err instanceof Error ? err.message : String(err)}`);
|
||||
}
|
||||
}
|
||||
|
||||
function authorizeApprovalButtonActor(params: {
|
||||
cfg: OpenClawConfig;
|
||||
accountId: string;
|
||||
event: InteractionEvent;
|
||||
approvalKind: "exec" | "plugin";
|
||||
}): { authorized: boolean; reason?: string } {
|
||||
const senderIds = resolveApprovalActorSenderIds(params.event);
|
||||
if (senderIds.length === 0) {
|
||||
return authorizeQQBotApprovalAction({
|
||||
cfg: params.cfg,
|
||||
accountId: params.accountId,
|
||||
senderId: null,
|
||||
approvalKind: params.approvalKind,
|
||||
});
|
||||
}
|
||||
|
||||
let denial: { authorized: boolean; reason?: string } | undefined;
|
||||
for (const senderId of senderIds) {
|
||||
const result = authorizeQQBotApprovalAction({
|
||||
cfg: params.cfg,
|
||||
accountId: params.accountId,
|
||||
senderId,
|
||||
approvalKind: params.approvalKind,
|
||||
});
|
||||
if (result.authorized) {
|
||||
return result;
|
||||
}
|
||||
denial ??= result;
|
||||
}
|
||||
return denial ?? { authorized: false, reason: "You are not authorized to approve this request." };
|
||||
}
|
||||
|
||||
function resolveApprovalActorSenderIds(event: InteractionEvent): string[] {
|
||||
const ids = [event.group_member_openid, event.user_openid].flatMap((value) => {
|
||||
const normalized = typeof value === "string" ? value.trim() : "";
|
||||
return normalized ? [normalized] : [];
|
||||
});
|
||||
return Array.from(new Set(ids));
|
||||
}
|
||||
|
||||
function resolveApprovalKind(approvalId: string): "exec" | "plugin" {
|
||||
return approvalId.toLowerCase().startsWith("plugin:") ? "plugin" : "exec";
|
||||
}
|
||||
|
||||
/** Execute an async handler, ACK with the result, and handle errors. */
|
||||
async function handleWithAck(
|
||||
creds: { appId: string; clientSecret: string },
|
||||
|
||||
@@ -216,3 +216,22 @@ export const isQQBotExecApprovalClientEnabled = qqbotExecApprovalProfile.isClien
|
||||
export const isQQBotExecApprovalApprover = qqbotExecApprovalProfile.isApprover;
|
||||
export const isQQBotExecApprovalAuthorizedSender = qqbotExecApprovalProfile.isAuthorizedSender;
|
||||
export const shouldHandleQQBotExecApprovalRequest = qqbotExecApprovalProfile.shouldHandleRequest;
|
||||
|
||||
export function authorizeQQBotApprovalAction(params: {
|
||||
cfg: OpenClawConfig;
|
||||
accountId?: string | null;
|
||||
senderId?: string | null;
|
||||
approvalKind: "exec" | "plugin";
|
||||
}): { authorized: boolean; reason?: string } {
|
||||
if (resolveQQBotExecApprovalConfig(params) === undefined) {
|
||||
return { authorized: true };
|
||||
}
|
||||
|
||||
const authorized =
|
||||
params.approvalKind === "plugin"
|
||||
? isQQBotExecApprovalApprover(params)
|
||||
: isQQBotExecApprovalAuthorizedSender(params);
|
||||
return authorized
|
||||
? { authorized: true }
|
||||
: { authorized: false, reason: "You are not authorized to approve this request." };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user