Skip to content

Commit ba136cc

Browse files
Bartok9steipete
Bartok9
authored and
steipete
committed
fix(qqbot): render safe exec approval previews
Render pending approvals from shared typed views, add marked grapheme-safe display wraps for QQ Desktop, preserve the 300 UTF-16-unit cap, fence command and metadata safely, and report plugin expiry from the runtime clock. Fixes #101979
1 parent d18e48c commit ba136cc

4 files changed

Lines changed: 323 additions & 45 deletions

File tree

‎extensions/qqbot/src/bridge/approval/handler-runtime.test.ts‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
// Qqbot tests cover native approval presentation behavior.
2+
import type {
3+
ExecApprovalPendingView,
4+
PluginApprovalPendingView,
5+
} from "openclaw/plugin-sdk/approval-handler-runtime";
6+
import { resolveExecApprovalCommandDisplay } from "openclaw/plugin-sdk/approval-runtime";
7+
import { describe, expect, it } from "vitest";
8+
import type { InlineKeyboard } from "../../engine/types.js";
9+
import { qqbotApprovalNativeRuntime } from "./handler-runtime.js";
10+
11+
type QQBotPendingPayload = {
12+
text: string;
13+
keyboard: InlineKeyboard;
14+
};
15+
16+
function createExecView(commandText: string): ExecApprovalPendingView {
17+
return {
18+
approvalId: "approval-1",
19+
approvalKind: "exec",
20+
phase: "pending",
21+
title: "Exec Approval Required",
22+
metadata: [],
23+
commandText,
24+
commandPreview: "short preview",
25+
actions: [
26+
{
27+
decision: "allow-once",
28+
label: "Allow Once",
29+
command: "/approve approval-1 allow-once",
30+
style: "success",
31+
},
32+
{
33+
decision: "deny",
34+
label: "Deny",
35+
command: "/approve approval-1 deny",
36+
style: "danger",
37+
},
38+
],
39+
expiresAtMs: Date.now() + 60_000,
40+
};
41+
}
42+
43+
function createPluginView(expiresAtMs: number): PluginApprovalPendingView {
44+
return {
45+
approvalId: "plugin:approval-1",
46+
approvalKind: "plugin",
47+
phase: "pending",
48+
title: "Install plugin",
49+
description: "Approve the requested plugin",
50+
metadata: [],
51+
pluginId: "example-plugin",
52+
toolName: "plugin.install",
53+
agentId: "main",
54+
severity: "critical",
55+
actions: [
56+
{
57+
decision: "allow-once",
58+
label: "Allow Once",
59+
command: "/approve plugin:approval-1 allow-once",
60+
style: "success",
61+
},
62+
{
63+
decision: "deny",
64+
label: "Deny",
65+
command: "/approve plugin:approval-1 deny",
66+
style: "danger",
67+
},
68+
],
69+
expiresAtMs,
70+
};
71+
}
72+
73+
describe("qqbotApprovalNativeRuntime", () => {
74+
it("renders the sanitized primary command with callback buttons", async () => {
75+
const secret = `ghp_${"a".repeat(36)}`;
76+
const rawCommand = `printf '${secret}\u200b'\n你好😀`;
77+
const commandText = resolveExecApprovalCommandDisplay({ command: rawCommand }).commandText;
78+
const view = createExecView(commandText);
79+
view.cwd = "/tmp\n![fake](u)";
80+
view.agentId = "agent```fake";
81+
const payload = (await qqbotApprovalNativeRuntime.presentation.buildPendingPayload({
82+
cfg: {} as never,
83+
accountId: "default",
84+
context: {},
85+
request: {
86+
id: "approval-1",
87+
request: { command: rawCommand, commandPreview: "short preview" },
88+
createdAtMs: Date.now(),
89+
expiresAtMs: view.expiresAtMs,
90+
},
91+
approvalKind: "exec",
92+
nowMs: Date.now(),
93+
view,
94+
})) as QQBotPendingPayload;
95+
96+
expect(commandText).not.toContain(secret);
97+
expect(commandText).toContain("\\u{200B}");
98+
expect(commandText).toContain("\\u{A}");
99+
expect(commandText).toContain("你好😀");
100+
expect(payload.text.replace(/[↩\n]/g, "")).toContain(commandText);
101+
expect(payload.text).not.toContain(secret);
102+
expect(payload.text).not.toContain("short preview");
103+
expect(payload.text).not.toContain("/tmp\n![fake]");
104+
expect(payload.text).toContain("📁 目录:\n```\n/tmp\\u{A}![fake](u)\n```");
105+
expect(payload.text).toContain("🤖 Agent:\n````\nagent```fake\n````");
106+
expect(payload.keyboard.content.rows[0]?.buttons.map((button) => button.action.data)).toEqual([
107+
"approve:approval-1:allow-once",
108+
"approve:approval-1:deny",
109+
]);
110+
});
111+
112+
it("renders a plugin approval's actual remaining lifetime", async () => {
113+
const nowMs = 1_000_000;
114+
const view = createPluginView(nowMs + 600_000);
115+
const payload = (await qqbotApprovalNativeRuntime.presentation.buildPendingPayload({
116+
cfg: {} as never,
117+
accountId: "default",
118+
context: {},
119+
request: {
120+
id: view.approvalId,
121+
request: {
122+
title: "stale raw title",
123+
description: "stale raw description",
124+
severity: "info",
125+
},
126+
createdAtMs: nowMs,
127+
expiresAtMs: view.expiresAtMs,
128+
},
129+
approvalKind: "plugin",
130+
nowMs,
131+
view,
132+
})) as QQBotPendingPayload;
133+
134+
expect(payload.text).toContain("🔴 审批请求");
135+
expect(payload.text).toContain("📋 Install plugin");
136+
expect(payload.text).toContain("📝 Approve the requested plugin");
137+
expect(payload.text).not.toContain("stale raw");
138+
expect(payload.text).toContain("⏱️ 超时: 600 秒");
139+
expect(payload.keyboard.content.rows[0]?.buttons.map((button) => button.action.data)).toEqual([
140+
"approve:plugin:approval-1:allow-once",
141+
"approve:plugin:approval-1:deny",
142+
]);
143+
});
144+
});

‎extensions/qqbot/src/bridge/approval/handler-runtime.ts‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,6 @@ type QQBotPendingPayload = {
4646
keyboard: InlineKeyboard;
4747
};
4848

49-
function isExecRequest(request: ApprovalRequest): request is ExecApprovalRequest {
50-
return "expiresAtMs" in request;
51-
}
52-
5349
function resolveQQTarget(request: ApprovalRequest): { type: ChatScope; id: string } | null {
5450
const sessionConversation = resolveApprovalRequestSessionConversation({
5551
request: request as never,
@@ -129,17 +125,17 @@ const qqbotApprovalRuntimeSpec: ChannelApprovalNativeRuntimeSpec<
129125
},
130126

131127
presentation: {
132-
buildPendingPayload: ({ request, view }) => {
133-
const req = request as ApprovalRequest;
134-
const text = isExecRequest(req) ? buildExecApprovalText(req) : buildPluginApprovalText(req);
128+
buildPendingPayload: ({ view, nowMs }) => {
129+
const text =
130+
view.approvalKind === "exec"
131+
? buildExecApprovalText(view, nowMs)
132+
: buildPluginApprovalText(view, nowMs);
135133
const keyboard = buildApprovalKeyboard(
136-
req.id,
134+
view.approvalId,
137135
view.actions.map((action) => action.decision),
138136
);
139137
getBridgeLogger().debug?.(
140-
`[qqbot:approval-runtime] buildPendingPayload requestId=${req.id} kind=${
141-
isExecRequest(req) ? "exec" : "plugin"
142-
}`,
138+
`[qqbot:approval-runtime] buildPendingPayload requestId=${view.approvalId} kind=${view.approvalKind}`,
143139
);
144140
return { text, keyboard };
145141
},

‎extensions/qqbot/src/engine/approval/index.test.ts‎

Lines changed: 81 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,29 @@
11
// Qqbot tests cover index plugin behavior.
2+
import type { ExecApprovalPendingView } from "openclaw/plugin-sdk/approval-handler-runtime";
23
import { describe, expect, it } from "vitest";
34
import { buildApprovalKeyboard, buildExecApprovalText } from "./index.js";
45

6+
function createExecView(commandText: string): ExecApprovalPendingView {
7+
return {
8+
approvalId: "approval-1",
9+
approvalKind: "exec",
10+
phase: "pending",
11+
title: "Exec Approval Required",
12+
metadata: [],
13+
commandText,
14+
actions: [],
15+
expiresAtMs: Date.now() + 60_000,
16+
};
17+
}
18+
19+
function readCommandBlock(text: string): { body: string; fence: string } {
20+
const match = text.match(/(?:^|\n)(`{3,})\n([\s\S]*?)\n\1(?:\n|$)/);
21+
if (!match?.[1] || match[2] === undefined) {
22+
throw new Error("Expected fenced command preview");
23+
}
24+
return { fence: match[1], body: match[2] };
25+
}
26+
527
describe("buildApprovalKeyboard", () => {
628
it("omits allow-always when the decision is unavailable", () => {
729
const keyboard = buildApprovalKeyboard("approval-123", ["allow-once", "deny"]);
@@ -23,16 +45,65 @@ describe("buildApprovalKeyboard", () => {
2345
});
2446

2547
describe("buildExecApprovalText", () => {
26-
it("truncates the command preview on a UTF-16 boundary without splitting surrogate pairs", () => {
48+
it("keeps a truncated command UTF-16 well formed", () => {
2749
const safePrefix = "x".repeat(299);
28-
const text = buildExecApprovalText({
29-
id: "approval-1",
30-
expiresAtMs: Date.now() + 60_000,
31-
request: {
32-
commandPreview: `${safePrefix}🎉 trailing text`,
33-
},
34-
});
35-
const expectedCommandBlock = ["```", safePrefix, "```"].join("\n");
36-
expect(text).toContain(expectedCommandBlock);
50+
const text = buildExecApprovalText(createExecView(`${safePrefix}🎉 trailing text`));
51+
const { body } = readCommandBlock(text);
52+
53+
expect(body.replace(/[↩\n]/g, "")).toBe(`${safePrefix}…[truncated]`);
54+
expect(body).not.toContain("🎉");
55+
});
56+
57+
it("wraps ASCII and double-width text after 24 graphemes", () => {
58+
const ascii = readCommandBlock(buildExecApprovalText(createExecView("x".repeat(25))));
59+
const wide = readCommandBlock(
60+
buildExecApprovalText(createExecView(`${"表".repeat(24)}😀`)),
61+
);
62+
63+
expect(ascii.body).toBe(`${"x".repeat(24)}↩\nx`);
64+
expect(wide.body).toBe(`${"表".repeat(24)}↩\n😀`);
65+
});
66+
67+
it("keeps an extended emoji grapheme intact at the 300-unit cap", () => {
68+
const family = "👨‍👩‍👧‍👦";
69+
const command = `${"x".repeat(289)}${family}`;
70+
const { body } = readCommandBlock(buildExecApprovalText(createExecView(command)));
71+
72+
expect(body.replace(/[↩\n]/g, "")).toBe(command);
73+
});
74+
75+
it("shows a truncation marker when the first grapheme exceeds the cap", () => {
76+
const oversizedGrapheme = `x${"\u0301".repeat(300)}`;
77+
const { body } = readCommandBlock(
78+
buildExecApprovalText(createExecView(`${oversizedGrapheme}; echo hidden`)),
79+
);
80+
81+
expect(body.replace(/[↩\n]/g, "")).toBe(
82+
`${oversizedGrapheme.slice(0, 300)}…[truncated]`,
83+
);
84+
});
85+
86+
it("marks a display wrap before a shell comment boundary", () => {
87+
const command = `${"x".repeat(22)} \\# harmless ; echo dangerous`;
88+
const text = buildExecApprovalText(createExecView(command));
89+
const { body } = readCommandBlock(text);
90+
91+
expect(text).toContain("↩ = display wrap only; not command text");
92+
expect(body).toContain(" \\↩\n# harmless ; echo danger↩\nous");
93+
expect(body.replace(/[↩\n]/g, "")).toBe(command);
94+
});
95+
96+
it("uses a longer fence when the command contains triple backticks", () => {
97+
const command = "echo ```danger```";
98+
const { body, fence } = readCommandBlock(buildExecApprovalText(createExecView(command)));
99+
100+
expect(fence).toBe("````");
101+
expect(body).toBe(command);
102+
});
103+
104+
it("reserves the display-wrap marker", () => {
105+
const { body } = readCommandBlock(buildExecApprovalText(createExecView("echo ↩")));
106+
107+
expect(body).toBe("echo \\u{21A9}");
37108
});
38109
});

0 commit comments

Comments
 (0)