Skip to content

Commit 45d8686

Browse files
steipetesteipete
committed
fix: enforce dm allowFrom inheritance across account channels (#27936) (thanks @widingmarcus-cyber)
1 parent 0fdac31 commit 45d8686

File tree

5 files changed

+355
-107
lines changed

5 files changed

+355
-107
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ Docs: https://docs.openclaw.ai
1717

1818
### Fixes
1919

20+
- Telegram/DM allowlist runtime inheritance: enforce `dmPolicy: "allowlist"` `allowFrom` requirements using effective account-plus-parent config across account-capable channels (Telegram, Discord, Slack, Signal, iMessage, IRC, BlueBubbles, WhatsApp), and align `openclaw doctor` checks to the same inheritance logic so DM traffic is not silently dropped after upgrades. (#27936) Thanks @widingmarcus-cyber.
2021
- Delivery queue/recovery backoff: prevent retry starvation by persisting `lastAttemptAt` on failed sends and deferring recovery retries until each entry's `lastAttemptAt + backoff` window is eligible, while continuing to recover ready entries behind deferred ones. Landed from contributor PR #27710 by @Jimmy-xuzimo. Thanks @Jimmy-xuzimo.
2122
- Google Chat/Lifecycle: keep Google Chat `startAccount` pending until abort in webhook mode so startup is no longer interpreted as immediate exit, preventing auto-restart loops and webhook-target churn. (#27384) thanks @junsuwhy.
2223
- Temp dirs/Linux umask: force `0700` permissions after temp-dir creation and self-heal existing writable temp dirs before trust checks so `umask 0002` installs no longer crash-loop on startup. Landed from contributor PR #27860 by @stakeswky. (#27853) Thanks @stakeswky.

‎src/commands/doctor-config-flow.ts‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1112,23 +1112,40 @@ function detectEmptyAllowlistPolicy(cfg: OpenClawConfig): string[] {
11121112
const hasEntries = (list?: Array<string | number>) =>
11131113
Array.isArray(list) && list.map((v) => String(v).trim()).filter(Boolean).length > 0;
11141114

1115-
const checkAccount = (account: Record<string, unknown>, prefix: string) => {
1115+
const checkAccount = (
1116+
account: Record<string, unknown>,
1117+
prefix: string,
1118+
parent?: Record<string, unknown>,
1119+
) => {
11161120
const dmEntry = account.dm;
11171121
const dm =
11181122
dmEntry && typeof dmEntry === "object" && !Array.isArray(dmEntry)
11191123
? (dmEntry as Record<string, unknown>)
11201124
: undefined;
1125+
const parentDmEntry = parent?.dm;
1126+
const parentDm =
1127+
parentDmEntry && typeof parentDmEntry === "object" && !Array.isArray(parentDmEntry)
1128+
? (parentDmEntry as Record<string, unknown>)
1129+
: undefined;
11211130
const dmPolicy =
1122-
(account.dmPolicy as string | undefined) ?? (dm?.policy as string | undefined) ?? undefined;
1131+
(account.dmPolicy as string | undefined) ??
1132+
(dm?.policy as string | undefined) ??
1133+
(parent?.dmPolicy as string | undefined) ??
1134+
(parentDm?.policy as string | undefined) ??
1135+
undefined;
11231136

11241137
if (dmPolicy !== "allowlist") {
11251138
return;
11261139
}
11271140

1128-
const topAllowFrom = account.allowFrom as Array<string | number> | undefined;
1141+
const topAllowFrom =
1142+
(account.allowFrom as Array<string | number> | undefined) ??
1143+
(parent?.allowFrom as Array<string | number> | undefined);
11291144
const nestedAllowFrom = dm?.allowFrom as Array<string | number> | undefined;
1145+
const parentNestedAllowFrom = parentDm?.allowFrom as Array<string | number> | undefined;
1146+
const effectiveAllowFrom = topAllowFrom ?? nestedAllowFrom ?? parentNestedAllowFrom;
11301147

1131-
if (hasEntries(topAllowFrom) || hasEntries(nestedAllowFrom)) {
1148+
if (hasEntries(effectiveAllowFrom)) {
11321149
return;
11331150
}
11341151

@@ -1153,7 +1170,7 @@ function detectEmptyAllowlistPolicy(cfg: OpenClawConfig): string[] {
11531170
if (!account || typeof account !== "object") {
11541171
continue;
11551172
}
1156-
checkAccount(account, `channels.${channelName}.accounts.${accountId}`);
1173+
checkAccount(account, `channels.${channelName}.accounts.${accountId}`, channelConfig);
11571174
}
11581175
}
11591176
}

‎src/config/config.allowlist-requires-allowfrom.test.ts‎

Lines changed: 76 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,115 +1,145 @@
11
import { describe, expect, it } from "vitest";
22
import { validateConfigObject } from "./config.js";
33

4-
describe('dmPolicy="allowlist" requires non-empty allowFrom', () => {
4+
describe('dmPolicy="allowlist" requires non-empty effective allowFrom', () => {
55
it('rejects telegram dmPolicy="allowlist" without allowFrom', () => {
66
const res = validateConfigObject({
77
channels: { telegram: { dmPolicy: "allowlist", botToken: "fake" } },
88
});
99
expect(res.ok).toBe(false);
1010
if (!res.ok) {
11-
expect(res.issues.some((i) => i.path.includes("allowFrom"))).toBe(true);
11+
expect(res.issues.some((i) => i.path.includes("channels.telegram.allowFrom"))).toBe(true);
1212
}
1313
});
1414

15-
it('rejects telegram dmPolicy="allowlist" with empty allowFrom', () => {
15+
it('rejects signal dmPolicy="allowlist" without allowFrom', () => {
1616
const res = validateConfigObject({
17-
channels: { telegram: { dmPolicy: "allowlist", allowFrom: [], botToken: "fake" } },
17+
channels: { signal: { dmPolicy: "allowlist" } },
1818
});
1919
expect(res.ok).toBe(false);
2020
if (!res.ok) {
21-
expect(res.issues.some((i) => i.path.includes("allowFrom"))).toBe(true);
21+
expect(res.issues.some((i) => i.path.includes("channels.signal.allowFrom"))).toBe(true);
2222
}
2323
});
2424

25-
it('accepts telegram dmPolicy="allowlist" with allowFrom entries', () => {
25+
it('rejects discord dmPolicy="allowlist" without allowFrom', () => {
2626
const res = validateConfigObject({
27-
channels: { telegram: { dmPolicy: "allowlist", allowFrom: ["12345"], botToken: "fake" } },
27+
channels: { discord: { dmPolicy: "allowlist" } },
2828
});
29-
expect(res.ok).toBe(true);
29+
expect(res.ok).toBe(false);
30+
if (!res.ok) {
31+
expect(
32+
res.issues.some((i) => i.path.includes("channels.discord") && i.path.includes("allowFrom")),
33+
).toBe(true);
34+
}
3035
});
3136

32-
it('accepts telegram dmPolicy="pairing" without allowFrom', () => {
37+
it('rejects whatsapp dmPolicy="allowlist" without allowFrom', () => {
3338
const res = validateConfigObject({
34-
channels: { telegram: { dmPolicy: "pairing", botToken: "fake" } },
39+
channels: { whatsapp: { dmPolicy: "allowlist" } },
3540
});
36-
expect(res.ok).toBe(true);
41+
expect(res.ok).toBe(false);
42+
if (!res.ok) {
43+
expect(res.issues.some((i) => i.path.includes("channels.whatsapp.allowFrom"))).toBe(true);
44+
}
3745
});
3846

39-
it('rejects signal dmPolicy="allowlist" without allowFrom', () => {
47+
it('accepts dmPolicy="pairing" without allowFrom', () => {
4048
const res = validateConfigObject({
41-
channels: { signal: { dmPolicy: "allowlist" } },
49+
channels: { telegram: { dmPolicy: "pairing", botToken: "fake" } },
4250
});
43-
expect(res.ok).toBe(false);
44-
if (!res.ok) {
45-
expect(res.issues.some((i) => i.path.includes("allowFrom"))).toBe(true);
46-
}
51+
expect(res.ok).toBe(true);
4752
});
53+
});
4854

49-
it('accepts signal dmPolicy="allowlist" with allowFrom entries', () => {
55+
describe('account dmPolicy="allowlist" uses inherited allowFrom', () => {
56+
it("accepts telegram account allowlist when parent allowFrom exists", () => {
5057
const res = validateConfigObject({
51-
channels: { signal: { dmPolicy: "allowlist", allowFrom: ["+1234567890"] } },
58+
channels: {
59+
telegram: {
60+
allowFrom: ["12345"],
61+
accounts: { bot1: { dmPolicy: "allowlist", botToken: "fake" } },
62+
},
63+
},
5264
});
5365
expect(res.ok).toBe(true);
5466
});
5567

56-
it('rejects discord dmPolicy="allowlist" without allowFrom', () => {
68+
it("rejects telegram account allowlist when neither account nor parent has allowFrom", () => {
5769
const res = validateConfigObject({
58-
channels: { discord: { dmPolicy: "allowlist" } },
70+
channels: { telegram: { accounts: { bot1: { dmPolicy: "allowlist", botToken: "fake" } } } },
5971
});
6072
expect(res.ok).toBe(false);
6173
if (!res.ok) {
62-
expect(res.issues.some((i) => i.path.includes("allowFrom"))).toBe(true);
74+
expect(
75+
res.issues.some((i) => i.path.includes("channels.telegram.accounts.bot1.allowFrom")),
76+
).toBe(true);
6377
}
6478
});
6579

66-
it('accepts discord dmPolicy="allowlist" with allowFrom entries', () => {
80+
it("accepts signal account allowlist when parent allowFrom exists", () => {
6781
const res = validateConfigObject({
68-
channels: { discord: { dmPolicy: "allowlist", allowFrom: ["123456789"] } },
82+
channels: {
83+
signal: { allowFrom: ["+15550001111"], accounts: { work: { dmPolicy: "allowlist" } } },
84+
},
6985
});
7086
expect(res.ok).toBe(true);
7187
});
7288

73-
it('rejects whatsapp dmPolicy="allowlist" without allowFrom', () => {
89+
it("accepts discord account allowlist when parent allowFrom exists", () => {
7490
const res = validateConfigObject({
75-
channels: { whatsapp: { dmPolicy: "allowlist" } },
91+
channels: {
92+
discord: { allowFrom: ["123456789"], accounts: { work: { dmPolicy: "allowlist" } } },
93+
},
7694
});
77-
expect(res.ok).toBe(false);
78-
if (!res.ok) {
79-
expect(res.issues.some((i) => i.path.includes("allowFrom"))).toBe(true);
80-
}
95+
expect(res.ok).toBe(true);
8196
});
8297

83-
it('accepts whatsapp dmPolicy="allowlist" with allowFrom entries', () => {
98+
it("accepts slack account allowlist when parent allowFrom exists", () => {
8499
const res = validateConfigObject({
85-
channels: { whatsapp: { dmPolicy: "allowlist", allowFrom: ["+1234567890"] } },
100+
channels: {
101+
slack: {
102+
allowFrom: ["U123"],
103+
botToken: "xoxb-top",
104+
appToken: "xapp-top",
105+
accounts: {
106+
work: { dmPolicy: "allowlist", botToken: "xoxb-work", appToken: "xapp-work" },
107+
},
108+
},
109+
},
86110
});
87111
expect(res.ok).toBe(true);
88112
});
89113

90-
it('accepts telegram account dmPolicy="allowlist" without own allowFrom (inherits from parent)', () => {
91-
// Account-level schemas skip allowFrom validation because accounts inherit
92-
// allowFrom from the parent channel config at runtime.
114+
it("accepts whatsapp account allowlist when parent allowFrom exists", () => {
93115
const res = validateConfigObject({
94116
channels: {
95-
telegram: {
96-
accounts: {
97-
bot1: { dmPolicy: "allowlist", botToken: "fake" },
98-
},
99-
},
117+
whatsapp: { allowFrom: ["+15550001111"], accounts: { work: { dmPolicy: "allowlist" } } },
100118
},
101119
});
102120
expect(res.ok).toBe(true);
103121
});
104122

105-
it('accepts telegram account dmPolicy="allowlist" with allowFrom entries', () => {
123+
it("accepts imessage account allowlist when parent allowFrom exists", () => {
106124
const res = validateConfigObject({
107125
channels: {
108-
telegram: {
109-
accounts: {
110-
bot1: { dmPolicy: "allowlist", allowFrom: ["12345"], botToken: "fake" },
111-
},
112-
},
126+
imessage: { allowFrom: ["alice"], accounts: { work: { dmPolicy: "allowlist" } } },
127+
},
128+
});
129+
expect(res.ok).toBe(true);
130+
});
131+
132+
it("accepts irc account allowlist when parent allowFrom exists", () => {
133+
const res = validateConfigObject({
134+
channels: { irc: { allowFrom: ["nick"], accounts: { work: { dmPolicy: "allowlist" } } } },
135+
});
136+
expect(res.ok).toBe(true);
137+
});
138+
139+
it("accepts bluebubbles account allowlist when parent allowFrom exists", () => {
140+
const res = validateConfigObject({
141+
channels: {
142+
bluebubbles: { allowFrom: ["sender"], accounts: { work: { dmPolicy: "allowlist" } } },
113143
},
114144
});
115145
expect(res.ok).toBe(true);

0 commit comments

Comments
 (0)