mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-21 10:16:44 +00:00
fix(gateway): send approval route notices with write scope (#93656)
* fix(gateway): send approval route notices with write scope * fix(gateway): avoid approval runtime import cycle * fix(approvals): preserve scoped runtime requests --------- Co-authored-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com>
This commit is contained in:
co-authored by
Vincent Koc
parent
42dcf7075f
commit
1469441ff4
@@ -7,6 +7,35 @@ import {
|
||||
deliverApprovalRequestViaChannelNativePlan,
|
||||
} from "./approval-native-runtime.js";
|
||||
|
||||
const hoisted = vi.hoisted(() => ({
|
||||
callGatewayLeastPrivilege: vi.fn(async () => ({ ok: true })),
|
||||
createOperatorApprovalsGatewayClient: vi.fn(
|
||||
async (params: { onHelloOk?: (hello: unknown) => void }) => {
|
||||
queueMicrotask(() => params.onHelloOk?.({ type: "hello-ok" }));
|
||||
return {
|
||||
request: vi.fn(async () => ({ ok: true })),
|
||||
stop: vi.fn(),
|
||||
};
|
||||
},
|
||||
),
|
||||
startGatewayClientWhenEventLoopReady: vi.fn(async () => ({
|
||||
ready: true,
|
||||
aborted: false,
|
||||
})),
|
||||
}));
|
||||
|
||||
vi.mock("../gateway/call.js", () => ({
|
||||
callGatewayLeastPrivilege: hoisted.callGatewayLeastPrivilege,
|
||||
}));
|
||||
|
||||
vi.mock("../gateway/operator-approvals-client.js", () => ({
|
||||
createOperatorApprovalsGatewayClient: hoisted.createOperatorApprovalsGatewayClient,
|
||||
}));
|
||||
|
||||
vi.mock("../gateway/client-start-readiness.js", () => ({
|
||||
startGatewayClientWhenEventLoopReady: hoisted.startGatewayClientWhenEventLoopReady,
|
||||
}));
|
||||
|
||||
const execRequest = {
|
||||
id: "approval-1",
|
||||
request: {
|
||||
@@ -17,6 +46,9 @@ const execRequest = {
|
||||
};
|
||||
|
||||
afterEach(() => {
|
||||
hoisted.callGatewayLeastPrivilege.mockClear();
|
||||
hoisted.createOperatorApprovalsGatewayClient.mockClear();
|
||||
hoisted.startGatewayClientWhenEventLoopReady.mockClear();
|
||||
clearApprovalNativeRouteStateForTest();
|
||||
vi.useRealTimers();
|
||||
});
|
||||
@@ -226,6 +258,75 @@ describe("createChannelNativeApprovalRuntime", () => {
|
||||
expect(resolvedCall.entries).toEqual([{ chatId: "plugin:secondary", messageId: "m1" }]);
|
||||
});
|
||||
|
||||
it("sends route notices over least-privilege gateway calls", async () => {
|
||||
const runtime = createChannelNativeApprovalRuntime({
|
||||
label: "test/native-runtime-route-notice",
|
||||
clientDisplayName: "Test",
|
||||
channel: "slack",
|
||||
channelLabel: "Slack",
|
||||
cfg: { gateway: { auth: { token: "configured-token" } } } as never,
|
||||
accountId: "default",
|
||||
nativeAdapter: {
|
||||
describeDeliveryCapabilities: () => ({
|
||||
enabled: true,
|
||||
preferredSurface: "approver-dm",
|
||||
supportsOriginSurface: true,
|
||||
supportsApproverDmSurface: true,
|
||||
notifyOriginWhenDmOnly: true,
|
||||
}),
|
||||
resolveOriginTarget: async () => ({
|
||||
to: "channel:C123",
|
||||
threadId: "1712345678.123456",
|
||||
}),
|
||||
resolveApproverDmTargets: async () => [{ to: "user:owner" }],
|
||||
},
|
||||
isConfigured: () => true,
|
||||
shouldHandle: () => true,
|
||||
buildPendingContent: async () => "pending exec",
|
||||
prepareTarget: async ({ plannedTarget }) => ({
|
||||
dedupeKey: plannedTarget.target.to,
|
||||
target: { chatId: plannedTarget.target.to },
|
||||
}),
|
||||
deliverTarget: async () => ({ chatId: "user:owner", messageId: "m1" }),
|
||||
finalizeResolved: async () => {},
|
||||
});
|
||||
|
||||
await runtime.start();
|
||||
try {
|
||||
await runtime.handleRequested({
|
||||
id: "approval-route-notice",
|
||||
request: {
|
||||
command: "echo hi",
|
||||
turnSourceChannel: "slack",
|
||||
turnSourceTo: "channel:C123",
|
||||
turnSourceAccountId: "default",
|
||||
turnSourceThreadId: "1712345678.123456",
|
||||
},
|
||||
createdAtMs: 0,
|
||||
expiresAtMs: Date.now() + 60_000,
|
||||
});
|
||||
} finally {
|
||||
await runtime.stop();
|
||||
}
|
||||
|
||||
expect(hoisted.callGatewayLeastPrivilege).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
config: { gateway: { auth: { token: "configured-token" } } },
|
||||
method: "send",
|
||||
clientName: "gateway-client",
|
||||
mode: "backend",
|
||||
params: {
|
||||
channel: "slack",
|
||||
to: "channel:C123",
|
||||
accountId: "default",
|
||||
threadId: "1712345678.123456",
|
||||
message: "Approval required. I sent the approval request to Slack DMs, not this chat.",
|
||||
idempotencyKey: "approval-route-notice:approval-route-notice",
|
||||
},
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("runs expiration through the shared runtime factory", async () => {
|
||||
vi.useFakeTimers();
|
||||
const finalizeExpired = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// Creates channel-native approval runtimes and delivery flows.
|
||||
import type { ChannelApprovalNativeAdapter } from "../channels/plugins/approval-native.types.js";
|
||||
import type { OpenClawConfig } from "../config/types.openclaw.js";
|
||||
import { GATEWAY_CLIENT_MODES, GATEWAY_CLIENT_NAMES } from "../utils/message-channel.js";
|
||||
import {
|
||||
resolveChannelNativeApprovalDeliveryPlan,
|
||||
type ChannelApprovalNativePlannedTarget,
|
||||
@@ -192,9 +193,6 @@ export function createChannelNativeApprovalRuntime<
|
||||
const nowMs = adapter.nowMs ?? Date.now;
|
||||
const resolveApprovalKind =
|
||||
adapter.resolveApprovalKind ?? ((request: TRequest) => defaultResolveApprovalKind(request));
|
||||
let runtimeRequest:
|
||||
| ((method: string, params: Record<string, unknown>) => Promise<unknown>)
|
||||
| null = null;
|
||||
const handledEventKinds = new Set<ExecApprovalChannelRuntimeEventKind>(
|
||||
adapter.eventKinds ?? ["exec"],
|
||||
);
|
||||
@@ -204,10 +202,15 @@ export function createChannelNativeApprovalRuntime<
|
||||
channelLabel: adapter.channelLabel,
|
||||
accountId: adapter.accountId,
|
||||
requestGateway: async <T>(method: string, params: Record<string, unknown>): Promise<T> => {
|
||||
if (!runtimeRequest) {
|
||||
throw new Error(`${adapter.label}: gateway client not connected`);
|
||||
}
|
||||
return (await runtimeRequest(method, params)) as T;
|
||||
const { callGatewayLeastPrivilege } = await import("../gateway/call.js");
|
||||
return await callGatewayLeastPrivilege<T>({
|
||||
config: adapter.cfg,
|
||||
...(adapter.gatewayUrl ? { url: adapter.gatewayUrl } : {}),
|
||||
method,
|
||||
params,
|
||||
clientName: GATEWAY_CLIENT_NAMES.GATEWAY_CLIENT,
|
||||
mode: GATEWAY_CLIENT_MODES.BACKEND,
|
||||
});
|
||||
},
|
||||
});
|
||||
|
||||
@@ -334,8 +337,6 @@ export function createChannelNativeApprovalRuntime<
|
||||
},
|
||||
});
|
||||
|
||||
runtimeRequest = (method, params) => runtime.request(method, params);
|
||||
|
||||
return {
|
||||
...runtime,
|
||||
async start() {
|
||||
|
||||
@@ -319,6 +319,7 @@ describe("createExecApprovalChannelRuntime", () => {
|
||||
|
||||
await runtime.start();
|
||||
await runtime.request("exec.approval.resolve", { id: "abc", decision: "deny" });
|
||||
await runtime.request("exec.approval.list", {});
|
||||
|
||||
expect(mockGatewayClientStarts).toHaveBeenCalledTimes(1);
|
||||
expectStartGatewayClientCall();
|
||||
@@ -326,6 +327,33 @@ describe("createExecApprovalChannelRuntime", () => {
|
||||
id: "abc",
|
||||
decision: "deny",
|
||||
});
|
||||
expect(mockGatewayClientRequests).toHaveBeenCalledWith("exec.approval.list", {});
|
||||
});
|
||||
|
||||
it("rejects write RPCs before they reach the approvals-only gateway client", async () => {
|
||||
const runtime = createExecApprovalChannelRuntime({
|
||||
label: "test/exec-approvals",
|
||||
clientDisplayName: "Test Exec Approvals",
|
||||
cfg: {} as never,
|
||||
isConfigured: () => true,
|
||||
shouldHandle: () => true,
|
||||
deliverRequested: async () => [],
|
||||
finalizeResolved: async () => undefined,
|
||||
});
|
||||
|
||||
await runtime.start();
|
||||
mockGatewayClientRequests.mockClear();
|
||||
|
||||
await expect(
|
||||
runtime.request("send", {
|
||||
channel: "slack",
|
||||
to: "channel:C123",
|
||||
message: "hello",
|
||||
}),
|
||||
).rejects.toThrow(
|
||||
"test/exec-approvals: operator approvals runtime cannot dispatch send; use a write-capable gateway client",
|
||||
);
|
||||
expect(mockGatewayClientRequests).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("fails startup when gateway client readiness times out before start", async () => {
|
||||
|
||||
@@ -3,6 +3,7 @@ import { readConnectErrorDetailCode } from "../../packages/gateway-protocol/src/
|
||||
import type { EventFrame } from "../../packages/gateway-protocol/src/index.js";
|
||||
import { startGatewayClientWhenEventLoopReady } from "../gateway/client-start-readiness.js";
|
||||
import type { GatewayClient, GatewayReconnectPausedInfo } from "../gateway/client.js";
|
||||
import { isApprovalMethod } from "../gateway/method-scopes.js";
|
||||
import { createOperatorApprovalsGatewayClient } from "../gateway/operator-approvals-client.js";
|
||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
import { formatErrorMessage } from "./errors.js";
|
||||
@@ -425,6 +426,11 @@ export function createExecApprovalChannelRuntime<
|
||||
handleExpired,
|
||||
|
||||
async request<T = unknown>(method: string, params: Record<string, unknown>): Promise<T> {
|
||||
if (!isApprovalMethod(method)) {
|
||||
throw new Error(
|
||||
`${adapter.label}: operator approvals runtime cannot dispatch ${method}; use a write-capable gateway client`,
|
||||
);
|
||||
}
|
||||
if (!gatewayClient) {
|
||||
throw new Error(`${adapter.label}: gateway client not connected`);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user