Matrix: tighten native approval gating

This commit is contained in:
Gustavo Madeira Santana
2026-04-02 14:29:24 -04:00
parent 477d81734f
commit ae420a355c
13 changed files with 301 additions and 10 deletions

View File

@@ -95,6 +95,62 @@ describe("matrix native approval adapter", () => {
expect(targets).toEqual([{ to: "user:@owner:example.org" }]);
});
it("keeps plugin forwarding fallback active when native delivery is exec-only", () => {
const shouldSuppress = matrixNativeApprovalAdapter.delivery?.shouldSuppressForwardingFallback;
if (!shouldSuppress) {
throw new Error("delivery suppression helper unavailable");
}
expect(
shouldSuppress({
cfg: buildConfig(),
approvalKind: "plugin",
target: {
channel: "matrix",
to: "room:!ops:example.org",
accountId: "default",
},
request: {
id: "req-1",
request: {
command: "echo hi",
turnSourceChannel: "matrix",
turnSourceTo: "room:!ops:example.org",
turnSourceAccountId: "default",
},
createdAtMs: 0,
expiresAtMs: 1000,
},
}),
).toBe(false);
});
it("preserves room-id case when matching Matrix origin targets", async () => {
const target = await matrixNativeApprovalAdapter.native?.resolveOriginTarget?.({
cfg: buildConfig(),
accountId: "default",
approvalKind: "exec",
request: {
id: "req-1",
request: {
command: "echo hi",
turnSourceChannel: "matrix",
turnSourceTo: "room:!Ops:Example.org",
turnSourceThreadId: "$thread",
turnSourceAccountId: "default",
sessionKey: "agent:main:matrix:channel:!Ops:Example.org",
},
createdAtMs: 0,
expiresAtMs: 1000,
},
});
expect(target).toEqual({
to: "room:!Ops:Example.org",
threadId: "$thread",
});
});
it("keeps plugin approval auth independent from exec approvers", () => {
const cfg = buildConfig({
dm: { allowFrom: ["@owner:example.org"] },

View File

@@ -27,7 +27,10 @@ function normalizeComparableTarget(value: string): string {
if (!target) {
return value.trim().toLowerCase();
}
return `${target.kind}:${target.id}`.toLowerCase();
if (target.kind === "user") {
return `user:${normalizeMatrixUserId(target.id)}`;
}
return `${target.kind.toLowerCase()}:${target.id}`;
}
function resolveMatrixNativeTarget(raw: string): string | null {
@@ -127,6 +130,17 @@ const splitMatrixApprovalCapability = splitChannelApprovalCapability(
matrixNativeApprovalCapability,
);
const matrixBaseNativeApprovalAdapter = splitMatrixApprovalCapability.native;
const matrixBaseDeliveryAdapter = splitMatrixApprovalCapability.delivery;
type MatrixForwardingSuppressionParams = Parameters<
NonNullable<NonNullable<typeof matrixBaseDeliveryAdapter>["shouldSuppressForwardingFallback"]>
>[0];
const matrixDeliveryAdapter = matrixBaseDeliveryAdapter && {
...matrixBaseDeliveryAdapter,
shouldSuppressForwardingFallback: (params: MatrixForwardingSuppressionParams) =>
params.approvalKind === "plugin"
? false
: (matrixBaseDeliveryAdapter.shouldSuppressForwardingFallback?.(params) ?? false),
};
const matrixExecOnlyNativeApprovalAdapter = matrixBaseNativeApprovalAdapter && {
describeDeliveryCapabilities: (
params: Parameters<typeof matrixBaseNativeApprovalAdapter.describeDeliveryCapabilities>[0],
@@ -176,7 +190,7 @@ export const matrixApprovalCapability = createChannelApprovalCapability({
);
},
approvals: {
delivery: matrixNativeApprovalCapability.delivery,
delivery: matrixDeliveryAdapter,
native: matrixExecOnlyNativeApprovalAdapter,
render: matrixNativeApprovalCapability.render,
},
@@ -187,7 +201,7 @@ export const matrixNativeApprovalAdapter = {
authorizeActorAction: matrixApprovalCapability.authorizeActorAction,
getActionAvailabilityState: matrixApprovalCapability.getActionAvailabilityState,
},
delivery: matrixApprovalCapability.delivery,
delivery: matrixDeliveryAdapter,
render: matrixApprovalCapability.render,
native: matrixExecOnlyNativeApprovalAdapter,
};

View File

@@ -223,4 +223,37 @@ describe("MatrixExecApprovalHandler", () => {
}),
);
});
it("honors request decision constraints in pending approval text", async () => {
const cfg = {
channels: {
matrix: {
homeserver: "https://matrix.example.org",
userId: "@bot:example.org",
accessToken: "tok",
execApprovals: {
enabled: true,
approvers: ["@owner:example.org"],
target: "channel",
},
},
},
} as OpenClawConfig;
const { handler, sendMessage } = createHandler(cfg);
await handler.handleRequested({
...baseRequest,
request: {
...baseRequest.request,
ask: "always",
allowedDecisions: ["allow-once", "deny"],
},
});
expect(sendMessage).toHaveBeenCalledWith(
"room:!ops:example.org",
expect.not.stringContaining("allow-always"),
expect.anything(),
);
});
});

View File

@@ -65,11 +65,15 @@ function buildPendingApprovalText(params: { request: ApprovalRequest; nowMs: num
approvalId: params.request.id,
approvalSlug: params.request.id.slice(0, 8),
approvalCommandId: params.request.id,
ask: params.request.request.ask ?? undefined,
agentId: params.request.request.agentId ?? undefined,
allowedDecisions: params.request.request.allowedDecisions,
command: resolveExecApprovalCommandDisplay((params.request as ExecApprovalRequest).request)
.commandText,
cwd: (params.request as ExecApprovalRequest).request.cwd ?? undefined,
host: (params.request as ExecApprovalRequest).request.host === "node" ? "node" : "gateway",
nodeId: (params.request as ExecApprovalRequest).request.nodeId ?? undefined,
sessionKey: params.request.request.sessionKey ?? undefined,
expiresAtMs: params.request.expiresAtMs,
nowMs: params.nowMs,
}).text!;

View File

@@ -32,6 +32,11 @@ function buildConfig(
describe("matrix exec approvals", () => {
it("requires enablement and an explicit or inferred approver", () => {
expect(isMatrixExecApprovalClientEnabled({ cfg: buildConfig() })).toBe(false);
expect(
isMatrixExecApprovalClientEnabled({
cfg: buildConfig(undefined, { dm: { allowFrom: ["@owner:example.org"] } }),
}),
).toBe(false);
expect(isMatrixExecApprovalClientEnabled({ cfg: buildConfig({ enabled: true }) })).toBe(false);
expect(
isMatrixExecApprovalClientEnabled({
@@ -56,6 +61,13 @@ describe("matrix exec approvals", () => {
expect(isMatrixExecApprovalApprover({ cfg, senderId: "@owner:example.org" })).toBe(false);
});
it("ignores wildcard allowlist entries when inferring exec approvers", () => {
const cfg = buildConfig({ enabled: true }, { dm: { allowFrom: ["*"] } });
expect(getMatrixExecApprovalApprovers({ cfg })).toEqual([]);
expect(isMatrixExecApprovalClientEnabled({ cfg })).toBe(false);
});
it("defaults target to dm", () => {
expect(
resolveMatrixExecApprovalTarget({
@@ -102,6 +114,8 @@ describe("matrix exec approvals", () => {
execApproval: {
approvalId: "req-1",
approvalSlug: "req-1",
agentId: "ops-agent",
sessionKey: "agent:ops-agent:matrix:channel:!ops:example.org",
},
},
};
@@ -121,6 +135,30 @@ describe("matrix exec approvals", () => {
).toBe(false);
});
it("keeps local prompts when filters exclude the request", () => {
const payload = {
channelData: {
execApproval: {
approvalId: "req-1",
approvalSlug: "req-1",
agentId: "other-agent",
sessionKey: "agent:other-agent:matrix:channel:!ops:example.org",
},
},
};
expect(
shouldSuppressLocalMatrixExecApprovalPrompt({
cfg: buildConfig({
enabled: true,
approvers: ["@owner:example.org"],
agentFilter: ["ops-agent"],
}),
payload,
}),
).toBe(false);
});
it("normalizes prefixed approver ids", () => {
expect(normalizeMatrixApproverId("matrix:@owner:example.org")).toBe("@owner:example.org");
expect(normalizeMatrixApproverId("user:@owner:example.org")).toBe("@owner:example.org");

View File

@@ -1,11 +1,13 @@
import {
createChannelExecApprovalProfile,
getExecApprovalReplyMetadata,
isChannelExecApprovalTargetRecipient,
resolveApprovalRequestAccountId,
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 { ReplyPayload } from "openclaw/plugin-sdk/reply-runtime";
import { normalizeAccountId } from "openclaw/plugin-sdk/routing";
import { resolveMatrixAccount } from "./matrix/accounts.js";
import { normalizeMatrixUserId } from "./matrix/monitor/allowlist.js";
@@ -17,6 +19,25 @@ export function normalizeMatrixApproverId(value: string | number): string | unde
return normalized || undefined;
}
function normalizeMatrixExecApproverId(value: string | number): string | undefined {
const normalized = normalizeMatrixApproverId(value);
return normalized === "*" ? undefined : normalized;
}
function resolveMatrixExecApprovalConfig(params: {
cfg: OpenClawConfig;
accountId?: string | null;
}) {
const config = resolveMatrixAccount(params).config.execApprovals;
if (!config) {
return { enabled: false } as const;
}
return {
...config,
enabled: config.enabled === true,
};
}
export function getMatrixExecApprovalApprovers(params: {
cfg: OpenClawConfig;
accountId?: string | null;
@@ -25,7 +46,7 @@ export function getMatrixExecApprovalApprovers(params: {
return resolveApprovalApprovers({
explicit: account.execApprovals?.approvers,
allowFrom: account.dm?.allowFrom,
normalizeApprover: normalizeMatrixApproverId,
normalizeApprover: normalizeMatrixExecApproverId,
});
}
@@ -44,7 +65,7 @@ export function isMatrixExecApprovalTargetRecipient(params: {
}
const matrixExecApprovalProfile = createChannelExecApprovalProfile({
resolveConfig: (params) => resolveMatrixAccount(params).config.execApprovals,
resolveConfig: resolveMatrixExecApprovalConfig,
resolveApprovers: getMatrixExecApprovalApprovers,
normalizeSenderId: normalizeMatrixApproverId,
isTargetRecipient: isMatrixExecApprovalTargetRecipient,
@@ -68,5 +89,49 @@ export const isMatrixExecApprovalApprover = matrixExecApprovalProfile.isApprover
export const isMatrixExecApprovalAuthorizedSender = matrixExecApprovalProfile.isAuthorizedSender;
export const resolveMatrixExecApprovalTarget = matrixExecApprovalProfile.resolveTarget;
export const shouldHandleMatrixExecApprovalRequest = matrixExecApprovalProfile.shouldHandleRequest;
export const shouldSuppressLocalMatrixExecApprovalPrompt =
matrixExecApprovalProfile.shouldSuppressLocalPrompt;
function buildFilterCheckRequest(params: {
approvalId: string;
payload: ReplyPayload;
}): ExecApprovalRequest | null {
const metadata = getExecApprovalReplyMetadata(params.payload);
if (!metadata) {
return null;
}
return {
id: params.approvalId,
request: {
command: "",
agentId: metadata.agentId ?? null,
sessionKey: metadata.sessionKey ?? null,
},
createdAtMs: 0,
expiresAtMs: 0,
};
}
export function shouldSuppressLocalMatrixExecApprovalPrompt(params: {
cfg: OpenClawConfig;
accountId?: string | null;
payload: ReplyPayload;
}): boolean {
if (!matrixExecApprovalProfile.shouldSuppressLocalPrompt(params)) {
return false;
}
const metadata = getExecApprovalReplyMetadata(params.payload);
if (!metadata) {
return false;
}
const request = buildFilterCheckRequest({
approvalId: metadata.approvalId,
payload: params.payload,
});
if (!request) {
return false;
}
return shouldHandleMatrixExecApprovalRequest({
cfg: params.cfg,
accountId: params.accountId,
request,
});
}

View File

@@ -309,6 +309,7 @@ describe("slack native approval adapter", () => {
expect(
shouldSuppress({
cfg: buildConfig(),
approvalKind: "exec",
target: { channel: "slack", to: "channel:C123ROOM", accountId: "default" },
request: {
id: "approval-1",
@@ -326,6 +327,7 @@ describe("slack native approval adapter", () => {
expect(
shouldSuppress({
cfg: buildConfig(),
approvalKind: "exec",
target: { channel: "slack", to: "channel:C123ROOM", accountId: "default" },
request: {
id: "approval-1",

View File

@@ -512,6 +512,7 @@ export type ChannelApprovalDeliveryAdapter = {
hasConfiguredDmRoute?: (params: { cfg: OpenClawConfig }) => boolean;
shouldSuppressForwardingFallback?: (params: {
cfg: OpenClawConfig;
approvalKind: ChannelApprovalKind;
target: ChannelApprovalForwardTarget;
request: ExecApprovalRequest;
}) => boolean;

View File

@@ -169,6 +169,7 @@ function buildSyntheticApprovalRequest(routeRequest: ApprovalRouteRequest): Exec
}
function shouldSkipForwardingFallback(params: {
approvalKind: "exec" | "plugin";
target: ExecApprovalForwardTarget;
cfg: OpenClawConfig;
routeRequest: ApprovalRouteRequest;
@@ -181,6 +182,7 @@ function shouldSkipForwardingFallback(params: {
return (
adapter?.delivery?.shouldSuppressForwardingFallback?.({
cfg: params.cfg,
approvalKind: params.approvalKind,
target: params.target,
request: buildSyntheticApprovalRequest(params.routeRequest),
}) ?? false
@@ -502,8 +504,15 @@ function createApprovalHandlers<
resolveSessionTarget: params.resolveSessionTarget,
})
: []),
].filter((target) => !shouldSkipForwardingFallback({ target, cfg, routeRequest }));
].filter(
(target) =>
!shouldSkipForwardingFallback({
approvalKind: params.strategy.kind,
target,
cfg,
routeRequest,
}),
);
if (filteredTargets.length === 0) {
return false;
}
@@ -598,7 +607,15 @@ function createApprovalHandlers<
resolveSessionTarget: params.resolveSessionTarget,
})
: []),
].filter((target) => !shouldSkipForwardingFallback({ target, cfg, routeRequest }));
].filter(
(target) =>
!shouldSkipForwardingFallback({
approvalKind: params.strategy.kind,
target,
cfg,
routeRequest,
}),
);
}
}
if (!targets?.length) {

View File

@@ -76,14 +76,18 @@ describe("exec approval reply helpers", () => {
execApproval: {
approvalId: " req-1 ",
approvalSlug: " slug-1 ",
agentId: " agent-1 ",
allowedDecisions: ["allow-once", "bad", "deny", "allow-always", 3],
sessionKey: " session-1 ",
},
},
}),
).toEqual({
approvalId: "req-1",
approvalSlug: "slug-1",
agentId: "agent-1",
allowedDecisions: ["allow-once", "deny", "allow-always"],
sessionKey: "session-1",
});
});
@@ -104,7 +108,9 @@ describe("exec approval reply helpers", () => {
execApproval: {
approvalId: "req-1",
approvalSlug: "slug-1",
agentId: undefined,
allowedDecisions: ["allow-once", "allow-always", "deny"],
sessionKey: undefined,
},
});
expect(payload.interactive).toEqual({
@@ -180,6 +186,27 @@ describe("exec approval reply helpers", () => {
});
});
it("stores agent and session metadata for downstream suppression checks", () => {
const payload = buildExecApprovalPendingReplyPayload({
approvalId: "req-meta",
approvalSlug: "slug-meta",
agentId: "ops-agent",
sessionKey: "agent:ops-agent:matrix:channel:!room:example.org",
command: "echo ok",
host: "gateway",
});
expect(payload.channelData).toEqual({
execApproval: {
approvalId: "req-meta",
approvalSlug: "slug-meta",
agentId: "ops-agent",
allowedDecisions: ["allow-once", "allow-always", "deny"],
sessionKey: "agent:ops-agent:matrix:channel:!room:example.org",
},
});
});
it("uses a longer fence for commands containing triple backticks", () => {
const payload = buildExecApprovalPendingReplyPayload({
approvalId: "req-2",

View File

@@ -15,7 +15,9 @@ export type ExecApprovalUnavailableReason =
export type ExecApprovalReplyMetadata = {
approvalId: string;
approvalSlug: string;
agentId?: string;
allowedDecisions?: readonly ExecApprovalReplyDecision[];
sessionKey?: string;
};
export type ExecApprovalActionDescriptor = {
@@ -31,11 +33,13 @@ export type ExecApprovalPendingReplyParams = {
approvalSlug: string;
approvalCommandId?: string;
ask?: string | null;
agentId?: string | null;
allowedDecisions?: readonly ExecApprovalReplyDecision[];
command: string;
cwd?: string;
host: ExecHost;
nodeId?: string;
sessionKey?: string | null;
expiresAtMs?: number;
nowMs?: number;
};
@@ -231,10 +235,16 @@ export function getExecApprovalReplyMetadata(
value === "allow-once" || value === "allow-always" || value === "deny",
)
: undefined;
const agentId =
typeof record.agentId === "string" ? record.agentId.trim() || undefined : undefined;
const sessionKey =
typeof record.sessionKey === "string" ? record.sessionKey.trim() || undefined : undefined;
return {
approvalId,
approvalSlug,
agentId,
allowedDecisions,
sessionKey,
};
}
@@ -297,7 +307,9 @@ export function buildExecApprovalPendingReplyPayload(
execApproval: {
approvalId: params.approvalId,
approvalSlug: params.approvalSlug,
agentId: params.agentId?.trim() || undefined,
allowedDecisions,
sessionKey: params.sessionKey?.trim() || undefined,
},
},
};

View File

@@ -165,6 +165,7 @@ describe("createApproverRestrictedNativeApprovalAdapter", () => {
expect(
shouldSuppressForwardingFallback({
cfg: {} as never,
approvalKind: "exec",
target: { channel: "telegram", to: "target-1" },
request: {
request: {
@@ -179,6 +180,7 @@ describe("createApproverRestrictedNativeApprovalAdapter", () => {
expect(
shouldSuppressForwardingFallback({
cfg: {} as never,
approvalKind: "exec",
target: { channel: "telegram", to: "target-1" },
request: {
request: {
@@ -193,6 +195,7 @@ describe("createApproverRestrictedNativeApprovalAdapter", () => {
expect(
shouldSuppressForwardingFallback({
cfg: {} as never,
approvalKind: "exec",
target: { channel: "slack", to: "target-1" },
request: {
request: {
@@ -208,6 +211,21 @@ describe("createApproverRestrictedNativeApprovalAdapter", () => {
cfg: {} as never,
accountId: "topic-1",
});
expect(
shouldSuppressForwardingFallback({
cfg: {} as never,
approvalKind: "plugin",
target: { channel: "telegram", to: "target-1" },
request: {
request: {
command: "pwd",
turnSourceChannel: "telegram",
turnSourceAccountId: "topic-1",
},
} as never,
}),
).toBe(false);
});
});

View File

@@ -18,6 +18,7 @@ type ApprovalAdapterParams = {
type DeliverySuppressionParams = {
cfg: OpenClawConfig;
approvalKind: ApprovalKind;
target: { channel: string; accountId?: string | null };
request: { request: { turnSourceChannel?: string | null; turnSourceAccountId?: string | null } };
};
@@ -126,6 +127,9 @@ function buildApproverRestrictedNativeApprovalCapability(
(resolvedAccountId === undefined
? input.target.accountId?.trim()
: resolvedAccountId.trim()) || undefined;
if (input.approvalKind === "plugin") {
return false;
}
return params.isNativeDeliveryEnabled({ cfg: input.cfg, accountId });
},
},