mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-04 14:00:24 +00:00
fix(approvals): centralize native request binding
This commit is contained in:
@@ -143,4 +143,29 @@ describe("createDiscordNativeApprovalAdapter", () => {
|
||||
|
||||
expect(target).toEqual({ to: "987654321" });
|
||||
});
|
||||
|
||||
it("rejects origin delivery for requests bound to another Discord account", async () => {
|
||||
const adapter = createDiscordNativeApprovalAdapter();
|
||||
|
||||
const target = await adapter.native?.resolveOriginTarget?.({
|
||||
cfg: {} as never,
|
||||
accountId: "main",
|
||||
approvalKind: "plugin",
|
||||
request: {
|
||||
id: "abc",
|
||||
request: {
|
||||
title: "Plugin approval",
|
||||
description: "Let plugin proceed",
|
||||
turnSourceChannel: "discord",
|
||||
turnSourceTo: "channel:123456789",
|
||||
turnSourceAccountId: "other",
|
||||
sessionKey: "agent:main:missing",
|
||||
},
|
||||
createdAtMs: 1,
|
||||
expiresAtMs: 2,
|
||||
},
|
||||
});
|
||||
|
||||
expect(target).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,11 +1,14 @@
|
||||
import { createApproverRestrictedNativeApprovalAdapter, resolveExecApprovalSessionTarget } from "openclaw/plugin-sdk/approval-runtime";
|
||||
import {
|
||||
createApproverRestrictedNativeApprovalAdapter,
|
||||
doesApprovalRequestMatchChannelAccount,
|
||||
resolveApprovalRequestSessionTarget,
|
||||
} from "openclaw/plugin-sdk/approval-runtime";
|
||||
import type { DiscordExecApprovalConfig, OpenClawConfig } from "openclaw/plugin-sdk/config-runtime";
|
||||
import type {
|
||||
ExecApprovalRequest,
|
||||
ExecApprovalSessionTarget,
|
||||
PluginApprovalRequest,
|
||||
} from "openclaw/plugin-sdk/infra-runtime";
|
||||
import { normalizeAccountId } from "openclaw/plugin-sdk/routing";
|
||||
import { listDiscordAccountIds, resolveDiscordAccount } from "./accounts.js";
|
||||
import {
|
||||
getDiscordExecApprovalApprovers,
|
||||
@@ -34,29 +37,6 @@ function extractDiscordSessionKind(sessionKey?: string | null): "channel" | "gro
|
||||
return match[1] as "channel" | "group" | "dm";
|
||||
}
|
||||
|
||||
function isExecApprovalRequest(request: ApprovalRequest): request is ExecApprovalRequest {
|
||||
return "command" in request.request;
|
||||
}
|
||||
|
||||
function toExecLikeRequest(request: ApprovalRequest): ExecApprovalRequest {
|
||||
if (isExecApprovalRequest(request)) {
|
||||
return request;
|
||||
}
|
||||
return {
|
||||
id: request.id,
|
||||
request: {
|
||||
command: request.request.title,
|
||||
sessionKey: request.request.sessionKey ?? undefined,
|
||||
turnSourceChannel: request.request.turnSourceChannel ?? undefined,
|
||||
turnSourceTo: request.request.turnSourceTo ?? undefined,
|
||||
turnSourceAccountId: request.request.turnSourceAccountId ?? undefined,
|
||||
turnSourceThreadId: request.request.turnSourceThreadId ?? undefined,
|
||||
},
|
||||
createdAtMs: request.createdAtMs,
|
||||
expiresAtMs: request.expiresAtMs,
|
||||
};
|
||||
}
|
||||
|
||||
function normalizeDiscordOriginChannelId(value?: string | null): string | null {
|
||||
if (!value) {
|
||||
return null;
|
||||
@@ -76,15 +56,7 @@ function resolveRequestSessionTarget(params: {
|
||||
cfg: OpenClawConfig;
|
||||
request: ApprovalRequest;
|
||||
}): ExecApprovalSessionTarget | null {
|
||||
const execLikeRequest = toExecLikeRequest(params.request);
|
||||
return resolveExecApprovalSessionTarget({
|
||||
cfg: params.cfg,
|
||||
request: execLikeRequest,
|
||||
turnSourceChannel: execLikeRequest.request.turnSourceChannel ?? undefined,
|
||||
turnSourceTo: execLikeRequest.request.turnSourceTo ?? undefined,
|
||||
turnSourceAccountId: execLikeRequest.request.turnSourceAccountId ?? undefined,
|
||||
turnSourceThreadId: execLikeRequest.request.turnSourceThreadId ?? undefined,
|
||||
});
|
||||
return resolveApprovalRequestSessionTarget(params);
|
||||
}
|
||||
|
||||
function resolveDiscordOriginTarget(params: {
|
||||
@@ -92,11 +64,21 @@ function resolveDiscordOriginTarget(params: {
|
||||
accountId?: string | null;
|
||||
request: ApprovalRequest;
|
||||
}) {
|
||||
if (
|
||||
!doesApprovalRequestMatchChannelAccount({
|
||||
cfg: params.cfg,
|
||||
request: params.request,
|
||||
channel: "discord",
|
||||
accountId: params.accountId,
|
||||
})
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const sessionKind = extractDiscordSessionKind(params.request.request.sessionKey?.trim() || null);
|
||||
const turnSourceChannel = params.request.request.turnSourceChannel?.trim().toLowerCase() || "";
|
||||
const rawTurnSourceTo = params.request.request.turnSourceTo?.trim() || "";
|
||||
const turnSourceTo = normalizeDiscordOriginChannelId(rawTurnSourceTo);
|
||||
const turnSourceAccountId = params.request.request.turnSourceAccountId?.trim() || "";
|
||||
const hasExplicitOriginTarget = /^(?:channel|group):/i.test(rawTurnSourceTo);
|
||||
const turnSourceTarget =
|
||||
turnSourceChannel === "discord" &&
|
||||
@@ -105,26 +87,10 @@ function resolveDiscordOriginTarget(params: {
|
||||
(hasExplicitOriginTarget || sessionKind === "channel" || sessionKind === "group")
|
||||
? {
|
||||
to: turnSourceTo,
|
||||
accountId: turnSourceAccountId || undefined,
|
||||
}
|
||||
: null;
|
||||
if (
|
||||
turnSourceTarget?.accountId &&
|
||||
params.accountId &&
|
||||
normalizeAccountId(turnSourceTarget.accountId) !== normalizeAccountId(params.accountId)
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const sessionTarget = resolveRequestSessionTarget(params);
|
||||
if (
|
||||
sessionTarget?.channel === "discord" &&
|
||||
sessionTarget.accountId &&
|
||||
params.accountId &&
|
||||
normalizeAccountId(sessionTarget.accountId) !== normalizeAccountId(params.accountId)
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
if (
|
||||
turnSourceTarget &&
|
||||
sessionTarget?.channel === "discord" &&
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
import fs from "node:fs";
|
||||
import type { ButtonInteraction, ComponentData } from "@buape/carbon";
|
||||
import { Routes } from "discord-api-types/v10";
|
||||
import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
@@ -13,6 +14,8 @@ const { STORE_PATH, mockSessionStoreEntries } = vi.hoisted(() => ({
|
||||
|
||||
const writeStore = (store: Record<string, unknown>) => {
|
||||
mockSessionStoreEntries.value = JSON.parse(JSON.stringify(store)) as Record<string, unknown>;
|
||||
fs.writeFileSync(STORE_PATH, `${JSON.stringify(store, null, 2)}\n`, "utf8");
|
||||
clearSessionStoreCacheForTest();
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
@@ -524,6 +527,42 @@ describe("DiscordExecApprovalHandler.shouldHandle", () => {
|
||||
expect(matching.shouldHandle(createRequest())).toBe(true);
|
||||
});
|
||||
|
||||
it("filters by discord account from explicit turn-source bindings when the session store misses", () => {
|
||||
const handler = createHandler({ enabled: true, approvers: ["123"] }, "default");
|
||||
const matching = createHandler({ enabled: true, approvers: ["123"] }, "secondary");
|
||||
|
||||
expect(
|
||||
handler.shouldHandle(
|
||||
createRequest({
|
||||
sessionKey: "agent:test-agent:missing",
|
||||
turnSourceChannel: "discord",
|
||||
turnSourceAccountId: "secondary",
|
||||
}),
|
||||
),
|
||||
).toBe(false);
|
||||
expect(
|
||||
matching.shouldHandle(
|
||||
createRequest({
|
||||
sessionKey: "agent:test-agent:missing",
|
||||
turnSourceChannel: "discord",
|
||||
turnSourceAccountId: "secondary",
|
||||
}),
|
||||
),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects requests bound to another channel before account-specific handling", () => {
|
||||
const handler = createHandler({ enabled: true, approvers: ["123"] }, "default");
|
||||
expect(
|
||||
handler.shouldHandle(
|
||||
createRequest({
|
||||
turnSourceChannel: "slack",
|
||||
turnSourceAccountId: "default",
|
||||
}),
|
||||
),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("combines agent and session filters", () => {
|
||||
const handler = createHandler({
|
||||
enabled: true,
|
||||
|
||||
@@ -11,10 +11,10 @@ import {
|
||||
} from "@buape/carbon";
|
||||
import { ButtonStyle, Routes } from "discord-api-types/v10";
|
||||
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime";
|
||||
import { loadSessionStore, resolveStorePath } from "openclaw/plugin-sdk/config-runtime";
|
||||
import type { DiscordExecApprovalConfig } from "openclaw/plugin-sdk/config-runtime";
|
||||
import {
|
||||
createExecApprovalChannelRuntime,
|
||||
doesApprovalRequestMatchChannelAccount,
|
||||
type ExecApprovalChannelRuntime,
|
||||
resolveChannelNativeApprovalDeliveryPlan,
|
||||
} from "openclaw/plugin-sdk/infra-runtime";
|
||||
@@ -29,11 +29,6 @@ import type {
|
||||
PluginApprovalRequest,
|
||||
PluginApprovalResolved,
|
||||
} from "openclaw/plugin-sdk/infra-runtime";
|
||||
import {
|
||||
normalizeAccountId,
|
||||
normalizeMessageChannel,
|
||||
resolveAgentIdFromSessionKey,
|
||||
} from "openclaw/plugin-sdk/routing";
|
||||
import type { RuntimeEnv } from "openclaw/plugin-sdk/runtime-env";
|
||||
import { compileSafeRegex, testRegexWithBoundedInput } from "openclaw/plugin-sdk/security-runtime";
|
||||
import { logDebug, logError } from "openclaw/plugin-sdk/text-runtime";
|
||||
@@ -195,61 +190,6 @@ class ExecApprovalActionRow extends Row<Button> {
|
||||
}
|
||||
}
|
||||
|
||||
function resolveExecApprovalAccountId(params: {
|
||||
cfg: OpenClawConfig;
|
||||
request: ExecApprovalRequest;
|
||||
}): string | null {
|
||||
const sessionKey = params.request.request.sessionKey?.trim();
|
||||
if (!sessionKey) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
const agentId = resolveAgentIdFromSessionKey(sessionKey);
|
||||
const storePath = resolveStorePath(params.cfg.session?.store, { agentId });
|
||||
const store = loadSessionStore(storePath);
|
||||
const entry = store[sessionKey];
|
||||
const channel = normalizeMessageChannel(entry?.origin?.provider ?? entry?.lastChannel);
|
||||
if (channel && channel !== "discord") {
|
||||
return null;
|
||||
}
|
||||
const accountId = entry?.origin?.accountId ?? entry?.lastAccountId;
|
||||
return accountId?.trim() || null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function resolvePluginApprovalAccountId(params: {
|
||||
cfg: OpenClawConfig;
|
||||
request: PluginApprovalRequest;
|
||||
}): string | null {
|
||||
const fromSession = resolveExecApprovalAccountId({
|
||||
cfg: params.cfg,
|
||||
request: {
|
||||
id: params.request.id,
|
||||
request: {
|
||||
command: params.request.request.title,
|
||||
sessionKey: params.request.request.sessionKey ?? undefined,
|
||||
},
|
||||
createdAtMs: params.request.createdAtMs,
|
||||
expiresAtMs: params.request.expiresAtMs,
|
||||
},
|
||||
});
|
||||
if (fromSession) {
|
||||
return fromSession;
|
||||
}
|
||||
return params.request.request.turnSourceAccountId?.trim() || null;
|
||||
}
|
||||
|
||||
function resolveApprovalAccountId(params: {
|
||||
cfg: OpenClawConfig;
|
||||
request: ApprovalRequest;
|
||||
}): string | null {
|
||||
return isPluginApprovalRequest(params.request)
|
||||
? resolvePluginApprovalAccountId({ cfg: params.cfg, request: params.request })
|
||||
: resolveExecApprovalAccountId({ cfg: params.cfg, request: params.request });
|
||||
}
|
||||
|
||||
function resolveApprovalAgentId(request: ApprovalRequest): string | null {
|
||||
return request.request.agentId?.trim() || null;
|
||||
}
|
||||
@@ -540,15 +480,15 @@ export class DiscordExecApprovalHandler {
|
||||
return false;
|
||||
}
|
||||
|
||||
const requestAccountId = resolveApprovalAccountId({
|
||||
cfg: this.opts.cfg,
|
||||
request,
|
||||
});
|
||||
if (requestAccountId) {
|
||||
const handlerAccountId = normalizeAccountId(this.opts.accountId);
|
||||
if (normalizeAccountId(requestAccountId) !== handlerAccountId) {
|
||||
return false;
|
||||
}
|
||||
if (
|
||||
!doesApprovalRequestMatchChannelAccount({
|
||||
cfg: this.opts.cfg,
|
||||
request,
|
||||
channel: "discord",
|
||||
accountId: this.opts.accountId,
|
||||
})
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check agent filter
|
||||
|
||||
@@ -168,6 +168,45 @@ describe("slack native approval adapter", () => {
|
||||
expect(dmTargets).toEqual([]);
|
||||
});
|
||||
|
||||
it("skips native delivery when the request is bound to another Slack account", async () => {
|
||||
const originTarget = await slackNativeApprovalAdapter.native?.resolveOriginTarget?.({
|
||||
cfg: buildConfig(),
|
||||
accountId: "default",
|
||||
approvalKind: "exec",
|
||||
request: {
|
||||
id: "req-1",
|
||||
request: {
|
||||
command: "echo hi",
|
||||
turnSourceChannel: "slack",
|
||||
turnSourceTo: "channel:C123",
|
||||
turnSourceAccountId: "other",
|
||||
sessionKey: "agent:main:missing",
|
||||
},
|
||||
createdAtMs: 0,
|
||||
expiresAtMs: 1000,
|
||||
},
|
||||
});
|
||||
const dmTargets = await slackNativeApprovalAdapter.native?.resolveApproverDmTargets?.({
|
||||
cfg: buildConfig(),
|
||||
accountId: "default",
|
||||
approvalKind: "exec",
|
||||
request: {
|
||||
id: "req-1",
|
||||
request: {
|
||||
command: "echo hi",
|
||||
turnSourceChannel: "slack",
|
||||
turnSourceAccountId: "other",
|
||||
sessionKey: "agent:main:missing",
|
||||
},
|
||||
createdAtMs: 0,
|
||||
expiresAtMs: 1000,
|
||||
},
|
||||
});
|
||||
|
||||
expect(originTarget).toBeNull();
|
||||
expect(dmTargets).toEqual([]);
|
||||
});
|
||||
|
||||
it("suppresses generic slack fallback only for slack-originated approvals", () => {
|
||||
const shouldSuppress = slackNativeApprovalAdapter.delivery.shouldSuppressForwardingFallback;
|
||||
if (!shouldSuppress) {
|
||||
|
||||
@@ -81,7 +81,9 @@ describe("slack exec approvals", () => {
|
||||
});
|
||||
|
||||
it("defaults target to dm", () => {
|
||||
expect(resolveSlackExecApprovalTarget({ cfg: buildConfig({ enabled: true, approvers: ["U1"] }) })).toBe("dm");
|
||||
expect(
|
||||
resolveSlackExecApprovalTarget({ cfg: buildConfig({ enabled: true, approvers: ["U1"] }) }),
|
||||
).toBe("dm");
|
||||
});
|
||||
|
||||
it("matches slack target recipients from generic approval forwarding targets", () => {
|
||||
@@ -195,4 +197,64 @@ describe("slack exec approvals", () => {
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects requests bound to another channel or Slack account", () => {
|
||||
const cfg = buildConfig({
|
||||
enabled: true,
|
||||
approvers: ["U123"],
|
||||
});
|
||||
|
||||
expect(
|
||||
shouldHandleSlackExecApprovalRequest({
|
||||
cfg,
|
||||
accountId: "work",
|
||||
request: {
|
||||
id: "req-1",
|
||||
request: {
|
||||
command: "echo hi",
|
||||
turnSourceChannel: "discord",
|
||||
turnSourceAccountId: "work",
|
||||
},
|
||||
createdAtMs: 0,
|
||||
expiresAtMs: 1000,
|
||||
},
|
||||
}),
|
||||
).toBe(false);
|
||||
|
||||
expect(
|
||||
shouldHandleSlackExecApprovalRequest({
|
||||
cfg,
|
||||
accountId: "work",
|
||||
request: {
|
||||
id: "req-2",
|
||||
request: {
|
||||
command: "echo hi",
|
||||
turnSourceChannel: "slack",
|
||||
turnSourceAccountId: "other",
|
||||
sessionKey: "agent:ops-agent:missing",
|
||||
},
|
||||
createdAtMs: 0,
|
||||
expiresAtMs: 1000,
|
||||
},
|
||||
}),
|
||||
).toBe(false);
|
||||
|
||||
expect(
|
||||
shouldHandleSlackExecApprovalRequest({
|
||||
cfg,
|
||||
accountId: "work",
|
||||
request: {
|
||||
id: "req-3",
|
||||
request: {
|
||||
command: "echo hi",
|
||||
turnSourceChannel: "slack",
|
||||
turnSourceAccountId: "work",
|
||||
sessionKey: "agent:ops-agent:missing",
|
||||
},
|
||||
createdAtMs: 0,
|
||||
expiresAtMs: 1000,
|
||||
},
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,11 +1,9 @@
|
||||
import {
|
||||
doesApprovalRequestMatchChannelAccount,
|
||||
resolveApprovalApprovers,
|
||||
} from "openclaw/plugin-sdk/approval-runtime";
|
||||
import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime";
|
||||
import type {
|
||||
ExecApprovalRequest,
|
||||
PluginApprovalRequest,
|
||||
} from "openclaw/plugin-sdk/infra-runtime";
|
||||
import type { ExecApprovalRequest, PluginApprovalRequest } from "openclaw/plugin-sdk/infra-runtime";
|
||||
import type { ReplyPayload } from "openclaw/plugin-sdk/reply-runtime";
|
||||
import { normalizeAccountId } from "openclaw/plugin-sdk/routing";
|
||||
import { resolveSlackAccount } from "./accounts.js";
|
||||
@@ -47,6 +45,16 @@ export function shouldHandleSlackExecApprovalRequest(params: {
|
||||
accountId?: string | null;
|
||||
request: ApprovalRequest;
|
||||
}): boolean {
|
||||
if (
|
||||
!doesApprovalRequestMatchChannelAccount({
|
||||
cfg: params.cfg,
|
||||
request: params.request,
|
||||
channel: "slack",
|
||||
accountId: params.accountId,
|
||||
})
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
const config = resolveSlackAccount(params).config.execApprovals;
|
||||
if (!config?.enabled) {
|
||||
return false;
|
||||
|
||||
Reference in New Issue
Block a user