ewilliams28/openclaw
1
0
Fork 0
mirror of https://github.com/openclaw/openclaw.git synced 2026-08-26 20:35:39 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(approvals): prevent cross-channel exec approval leak (#122517)

Browse Source 
Reject unbound foreign-channel fallback at shared approval-account selection while preserving recorded bindings and explicit forwarding targets. Cover Telegram and Matrix routing contracts.

Co-authored-by: vatsalgargg <vatsalg80@gmail.com>
Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
Vatsal Garg
2026-08-14 13:11:03 +05:30
committed by GitHub
parent db2854e681
commit d8a1ebbb49
4 changed files with 76 additions and 62 deletions
Download Patch File Download Diff File Expand all files Collapse all files
extensions/matrix/src/exec-approvals.test.ts
+17 -30
View File
@@ -111,10 +111,11 @@ function buildMultiAccountMatrixConfig(params: {
} as OpenClawConfig;
}
function makeForeignChannelApprovalRequest(params: {
function makeChannelApprovalRequest(params: {
id: string;
sessionKey?: string;
agentId?: string;
turnSourceChannel?: string;
}): MatrixExecApprovalRequest {
return {
id: params.id,
@@ -122,7 +123,7 @@ function makeForeignChannelApprovalRequest(params: {
command: "echo hi",
agentId: params.agentId ?? "ops-agent",
sessionKey: params.sessionKey ?? "agent:ops-agent:missing",
turnSourceChannel: "slack",
turnSourceChannel: params.turnSourceChannel ?? "slack",
turnSourceTo: "channel:C123",
},
createdAtMs: 0,
@@ -401,7 +402,7 @@ describe("matrix exec approvals", () => {
},
});
const cfg = buildMultiAccountMatrixConfig({ sessionStorePath: storePath });
const request = makeForeignChannelApprovalRequest({
const request = makeChannelApprovalRequest({
id: "req-3",
sessionKey: "agent:ops-agent:matrix:channel:!room:example.org",
});
@@ -422,26 +423,6 @@ describe("matrix exec approvals", () => {
).toBe(true);
});
it("reports each eligible foreign-channel account as a raw route candidate", () => {
const cfg = buildMultiAccountMatrixConfig({});
const request = makeForeignChannelApprovalRequest({ id: "req-4" });
expect(
shouldHandleMatrixExecApprovalRequest({
cfg,
accountId: "default",
request,
}),
).toBe(true);
expect(
shouldHandleMatrixExecApprovalRequest({
cfg,
accountId: "ops",
request,
}),
).toBe(true);
});
it("reports each eligible same-channel account as a raw route candidate", () => {
const cfg = buildMultiAccountMatrixConfig({});
const request: MatrixExecApprovalRequest = {
@@ -457,14 +438,14 @@ describe("matrix exec approvals", () => {
expect(shouldHandleMatrixExecApprovalRequest({ cfg, accountId: "ops", request })).toBe(true);
});
it("allows unbound foreign-channel approvals when only one matrix account can handle them", () => {
it("rejects unbound foreign-channel approvals even when only one matrix account can handle them", () => {
const cfg = buildMultiAccountMatrixConfig({
opsExecApprovals: {
enabled: false,
approvers: ["@owner:example.org"],
},
});
const request = makeForeignChannelApprovalRequest({ id: "req-5" });
const request = makeChannelApprovalRequest({ id: "req-5" });
expect(
shouldHandleMatrixExecApprovalRequest({
@@ -472,7 +453,7 @@ describe("matrix exec approvals", () => {
accountId: "default",
request,
}),
).toBe(true);
).toBe(false);
expect(
shouldHandleMatrixExecApprovalRequest({
cfg,
@@ -482,7 +463,7 @@ describe("matrix exec approvals", () => {
).toBe(false);
});
it("uses request filters when checking foreign-channel matrix ambiguity", () => {
it("uses request filters when checking unbound matrix account eligibility", () => {
const cfg = buildMultiAccountMatrixConfig({
defaultExecApprovals: {
enabled: true,
@@ -495,7 +476,10 @@ describe("matrix exec approvals", () => {
agentFilter: ["other-agent"],
},
});
const request = makeForeignChannelApprovalRequest({ id: "req-6" });
const request = makeChannelApprovalRequest({
id: "req-6",
turnSourceChannel: "matrix",
});
expect(
shouldHandleMatrixExecApprovalRequest({
@@ -513,11 +497,14 @@ describe("matrix exec approvals", () => {
).toBe(false);
});
it("ignores disabled matrix accounts when checking foreign-channel ambiguity", () => {
it("ignores disabled matrix accounts when checking unbound account eligibility", () => {
const cfg = buildMultiAccountMatrixConfig({
opsOverrides: { enabled: false },
});
const request = makeForeignChannelApprovalRequest({ id: "req-7" });
const request = makeChannelApprovalRequest({
id: "req-7",
turnSourceChannel: "matrix",
});
expect(
shouldHandleMatrixExecApprovalRequest({
extensions/telegram/src/exec-approvals.test.ts
+22 -32
View File
@@ -95,16 +95,17 @@ function buildMultiAccountTelegramConfig(params: {
} as OpenClawConfig;
}
function makeForeignChannelApprovalRequest(params: {
function makeChannelApprovalRequest(params: {
id: string;
sessionKey?: string;
turnSourceChannel?: string;
}): TelegramExecApprovalRequest {
return {
id: params.id,
request: {
command: "echo hi",
sessionKey: params.sessionKey ?? "agent:ops:missing",
turnSourceChannel: "slack",
turnSourceChannel: params.turnSourceChannel ?? "slack",
turnSourceTo: "channel:C123",
},
createdAtMs: 0,
@@ -168,7 +169,7 @@ describe("telegram exec approvals", () => {
expect(isTelegramExecApprovalApprover({ cfg, senderId: "67890" })).toBe(true);
});
it("does not require explicit Telegram exec approvers when command owner identifies the Telegram operator", () => {
it("does not require explicit Telegram exec approvers when command owner identifies the operator", () => {
const cfg = {
...buildConfig(),
commands: {
@@ -182,7 +183,10 @@ describe("telegram exec approvals", () => {
expect(
shouldHandleTelegramExecApprovalRequest({
cfg,
request: makeForeignChannelApprovalRequest({ id: "discord-diagnostics" }),
request: makeChannelApprovalRequest({
id: "telegram-diagnostics",
turnSourceChannel: "telegram",
}),
}),
).toBe(true);
});
@@ -252,7 +256,7 @@ describe("telegram exec approvals", () => {
},
});
const cfg = buildMultiAccountTelegramConfig({ sessionStorePath: storePath });
const request = makeForeignChannelApprovalRequest({
const request = makeChannelApprovalRequest({
id: "req-2",
sessionKey: "agent:ops:telegram:direct:123",
});
@@ -273,26 +277,6 @@ describe("telegram exec approvals", () => {
).toBe(true);
});
it("reports each eligible foreign-channel account as a raw route candidate", () => {
const cfg = buildMultiAccountTelegramConfig({});
const request = makeForeignChannelApprovalRequest({ id: "req-3" });
expect(
shouldHandleTelegramExecApprovalRequest({
cfg,
accountId: "default",
request,
}),
).toBe(true);
expect(
shouldHandleTelegramExecApprovalRequest({
cfg,
accountId: "ops",
request,
}),
).toBe(true);
});
it("reports each eligible same-channel account as a raw route candidate", () => {
const cfg = buildMultiAccountTelegramConfig({});
const request: TelegramExecApprovalRequest = {
@@ -312,11 +296,11 @@ describe("telegram exec approvals", () => {
expect(shouldHandleTelegramExecApprovalRequest({ cfg, accountId: "ops", request })).toBe(true);
});
it("allows unbound foreign-channel approvals when only one telegram account can handle them", () => {
it("rejects unbound foreign-channel approvals even when only one telegram account can handle them", () => {
const cfg = buildMultiAccountTelegramConfig({
opsExecApprovals: { enabled: false, approvers: ["123"] },
});
const request = makeForeignChannelApprovalRequest({ id: "req-4" });
const request = makeChannelApprovalRequest({ id: "req-4" });
expect(
shouldHandleTelegramExecApprovalRequest({
@@ -324,7 +308,7 @@ describe("telegram exec approvals", () => {
accountId: "default",
request,
}),
).toBe(true);
).toBe(false);
expect(
shouldHandleTelegramExecApprovalRequest({
cfg,
@@ -334,7 +318,7 @@ describe("telegram exec approvals", () => {
).toBe(false);
});
it("uses request filters when checking foreign-channel telegram ambiguity", () => {
it("uses request filters when checking unbound telegram account eligibility", () => {
const cfg = buildMultiAccountTelegramConfig({
defaultExecApprovals: {
enabled: true,
@@ -347,7 +331,10 @@ describe("telegram exec approvals", () => {
agentFilter: ["other"],
},
});
const request = makeForeignChannelApprovalRequest({ id: "req-5" });
const request = makeChannelApprovalRequest({
id: "req-5",
turnSourceChannel: "telegram",
});
expect(
shouldHandleTelegramExecApprovalRequest({
@@ -459,9 +446,12 @@ describe("telegram exec approvals", () => {
).toBe(false);
});
it("ignores disabled telegram accounts when checking foreign-channel ambiguity", () => {
it("ignores disabled telegram accounts when checking unbound account eligibility", () => {
const cfg = buildMultiAccountTelegramConfig({ opsOverrides: { enabled: false } });
const request = makeForeignChannelApprovalRequest({ id: "req-6" });
const request = makeChannelApprovalRequest({
id: "req-6",
turnSourceChannel: "telegram",
});
expect(
shouldHandleTelegramExecApprovalRequest({
src/infra/approval-request-account-binding.ts
+4
View File
@@ -261,6 +261,10 @@ export function doesApprovalRequestSelectChannelAccount(params: {
if (boundAccountId || forwardAccountIds.length > 0) {
return false;
}
const turnSourceChannel = normalizeOptionalChannel(params.request.request.turnSourceChannel);
if (turnSourceChannel && turnSourceChannel !== normalizeOptionalChannel(params.channel)) {
return false;
}
const eligibleAccountIds = params.eligibleAccountIds
.map(normalizeOptionalAccountId)
.filter((candidate): candidate is string => Boolean(candidate));
src/infra/exec-approval-session-target.test.ts
+33
View File
@@ -90,6 +90,39 @@ describe("native approval account selection", () => {
).toBe(false);
});
it("rejects foreign-channel fallback but preserves explicit forwarding", () => {
const request = buildRequest({ turnSourceChannel: "whatsapp" });
const explicitTargetConfig: OpenClawConfig = {
approvals: {
exec: {
enabled: true,
mode: "targets",
targets: [{ channel: "telegram", to: "owner" }],
},
},
};
expect(
doesApprovalRequestSelectChannelAccount({
cfg: {},
request,
channel: "telegram",
accountId: "default",
defaultAccountId: "default",
eligibleAccountIds: ["default"],
}),
).toBe(false);
expect(
doesApprovalRequestSelectChannelAccount({
cfg: explicitTargetConfig,
request,
channel: "telegram",
accountId: "default",
defaultAccountId: "default",
eligibleAccountIds: ["default"],
}),
).toBe(true);
});
it("selects the recorded account even when several accounts are eligible", () => {
const request = buildRequest({
turnSourceChannel: "telegram",