From b05273de61b69019fba7293583db9d10a0c40555 Mon Sep 17 00:00:00 2001 From: gitwithuli Date: Mon, 16 Feb 2026 15:29:19 -0500 Subject: [PATCH] fix: doctor --fix auto-repairs dmPolicy="open" missing allowFrom wildcard When a channel is configured with dmPolicy="open" but without allowFrom: ["*"], the gateway rejects the config and exits. The error message suggests running "openclaw doctor --fix", but the doctor had no repair logic for this case. This adds a repair step that automatically adds "*" to allowFrom (or creates it) when dmPolicy="open" is set without the required wildcard. Handles both top-level and nested dm.allowFrom, as well as per-account configs. Co-Authored-By: Claude Opus 4.6 --- src/commands/doctor-config-flow.e2e.test.ts | 117 ++++++++++++++++++++ src/commands/doctor-config-flow.ts | 105 ++++++++++++++++++ 2 files changed, 222 insertions(+) diff --git a/src/commands/doctor-config-flow.e2e.test.ts b/src/commands/doctor-config-flow.e2e.test.ts index 25638d5b94f..9903e5ed242 100644 --- a/src/commands/doctor-config-flow.e2e.test.ts +++ b/src/commands/doctor-config-flow.e2e.test.ts @@ -229,4 +229,121 @@ describe("doctor config flow", () => { ]); }); }); + + it('adds allowFrom ["*"] when dmPolicy="open" and allowFrom is missing on repair', async () => { + const result = await runDoctorConfigWithInput({ + repair: true, + config: { + channels: { + discord: { + token: "test-token", + dmPolicy: "open", + groupPolicy: "open", + }, + }, + }, + }); + + const cfg = result.cfg as unknown as { + channels: { discord: { allowFrom: string[]; dmPolicy: string } }; + }; + expect(cfg.channels.discord.allowFrom).toEqual(["*"]); + expect(cfg.channels.discord.dmPolicy).toBe("open"); + }); + + it("adds * to existing allowFrom array when dmPolicy is open on repair", async () => { + const result = await runDoctorConfigWithInput({ + repair: true, + config: { + channels: { + slack: { + botToken: "xoxb-test", + appToken: "xapp-test", + dmPolicy: "open", + allowFrom: ["U123"], + }, + }, + }, + }); + + const cfg = result.cfg as unknown as { + channels: { slack: { allowFrom: string[] } }; + }; + expect(cfg.channels.slack.allowFrom).toContain("*"); + expect(cfg.channels.slack.allowFrom).toContain("U123"); + }); + + it("repairs nested dm.allowFrom when top-level allowFrom is absent on repair", async () => { + const result = await runDoctorConfigWithInput({ + repair: true, + config: { + channels: { + discord: { + token: "test-token", + dmPolicy: "open", + dm: { allowFrom: ["123"] }, + }, + }, + }, + }); + + const cfg = result.cfg as unknown as { + channels: { discord: { dm: { allowFrom: string[] }; allowFrom?: string[] } }; + }; + // When dmPolicy is set at top level but allowFrom only exists nested in dm, + // the repair adds "*" to dm.allowFrom + if (cfg.channels.discord.dm) { + expect(cfg.channels.discord.dm.allowFrom).toContain("*"); + expect(cfg.channels.discord.dm.allowFrom).toContain("123"); + } else { + // If doctor flattened the config, allowFrom should be at top level + expect(cfg.channels.discord.allowFrom).toContain("*"); + } + }); + + it("skips repair when allowFrom already includes *", async () => { + const result = await runDoctorConfigWithInput({ + repair: true, + config: { + channels: { + discord: { + token: "test-token", + dmPolicy: "open", + allowFrom: ["*"], + }, + }, + }, + }); + + const cfg = result.cfg as unknown as { + channels: { discord: { allowFrom: string[] } }; + }; + expect(cfg.channels.discord.allowFrom).toEqual(["*"]); + }); + + it("repairs per-account dmPolicy open without allowFrom on repair", async () => { + const result = await runDoctorConfigWithInput({ + repair: true, + config: { + channels: { + discord: { + token: "test-token", + accounts: { + work: { + token: "test-token-2", + dmPolicy: "open", + }, + }, + }, + }, + }, + }); + + const cfg = result.cfg as unknown as { + channels: { + discord: { accounts: { work: { allowFrom: string[]; dmPolicy: string } } }; + }; + }; + expect(cfg.channels.discord.accounts.work.allowFrom).toEqual(["*"]); + }); }); diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index d36a40222ba..dc18ea948cc 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -553,6 +553,92 @@ function maybeRepairDiscordNumericIds(cfg: OpenClawConfig): { return { config: next, changes }; } +/** + * Scan all channel configs for dmPolicy="open" without allowFrom including "*". + * This configuration is rejected by the schema validator but can easily occur when + * users (or integrations) set dmPolicy to "open" without realising that an explicit + * allowFrom wildcard is also required. + */ +function maybeRepairOpenPolicyAllowFrom(cfg: OpenClawConfig): { + config: OpenClawConfig; + changes: string[]; +} { + const channels = cfg.channels; + if (!channels || typeof channels !== "object") { + return { config: cfg, changes: [] }; + } + + const next = structuredClone(cfg); + const changes: string[] = []; + + const ensureWildcard = ( + channelName: string, + account: Record, + prefix: string, + ) => { + const dmPolicy = + (account.dmPolicy as string | undefined) ?? + ((account.dm as Record | undefined)?.policy as string | undefined); + + if (dmPolicy !== "open") { + return; + } + + // Check top-level allowFrom first, then nested dm.allowFrom + const topAllowFrom = account.allowFrom as Array | undefined; + const dm = account.dm as Record | undefined; + const nestedAllowFrom = dm?.allowFrom as Array | undefined; + + const hasWildcard = (list?: Array) => + list?.some((v) => String(v).trim() === "*") ?? false; + + if (hasWildcard(topAllowFrom) || hasWildcard(nestedAllowFrom)) { + return; + } + + // Prefer setting top-level allowFrom (it takes precedence) + if (Array.isArray(topAllowFrom)) { + (account.allowFrom as Array).push("*"); + changes.push(`- ${prefix}.allowFrom: added "*" (required by dmPolicy="open")`); + } else if (Array.isArray(nestedAllowFrom)) { + (dm!.allowFrom as Array).push("*"); + changes.push(`- ${prefix}.dm.allowFrom: added "*" (required by dmPolicy="open")`); + } else { + account.allowFrom = ["*"]; + changes.push(`- ${prefix}.allowFrom: set to ["*"] (required by dmPolicy="open")`); + } + }; + + const nextChannels = next.channels as Record>; + for (const [channelName, channelConfig] of Object.entries(nextChannels)) { + if (!channelConfig || typeof channelConfig !== "object") { + continue; + } + + // Check the top-level channel config + ensureWildcard(channelName, channelConfig, `channels.${channelName}`); + + // Check per-account configs (e.g. channels.discord.accounts.mybot) + const accounts = channelConfig.accounts as Record> | undefined; + if (accounts && typeof accounts === "object") { + for (const [accountName, accountConfig] of Object.entries(accounts)) { + if (accountConfig && typeof accountConfig === "object") { + ensureWildcard( + channelName, + accountConfig, + `channels.${channelName}.accounts.${accountName}`, + ); + } + } + } + } + + if (changes.length === 0) { + return { config: cfg, changes: [] }; + } + return { config: next, changes }; +} + async function maybeMigrateLegacyConfig(): Promise { const changes: string[] = []; const home = resolveHomeDir(); @@ -699,6 +785,14 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { pendingChanges = true; cfg = discordRepair.config; } + + const allowFromRepair = maybeRepairOpenPolicyAllowFrom(candidate); + if (allowFromRepair.changes.length > 0) { + note(allowFromRepair.changes.join("\n"), "Doctor changes"); + candidate = allowFromRepair.config; + pendingChanges = true; + cfg = allowFromRepair.config; + } } else { const hits = scanTelegramAllowFromUsernameEntries(candidate); if (hits.length > 0) { @@ -721,6 +815,17 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { "Doctor warnings", ); } + + const allowFromScan = maybeRepairOpenPolicyAllowFrom(candidate); + if (allowFromScan.changes.length > 0) { + note( + [ + ...allowFromScan.changes, + `- Run "${formatCliCommand("openclaw doctor --fix")}" to add missing allowFrom wildcards.`, + ].join("\n"), + "Doctor warnings", + ); + } } const unknown = stripUnknownConfigKeys(candidate);