From 1ffc319831ef0bdfa315ffde6a7d881cb45bc2db Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Thu, 26 Feb 2026 05:31:51 -0500 Subject: [PATCH] Doctor: keep allowFrom account-scoped in multi-account configs --- docs/channels/discord.md | 6 ++ docs/channels/slack.md | 6 ++ docs/channels/telegram.md | 4 + ...fault-account-bindings.integration.test.ts | 2 +- src/commands/doctor-config-flow.test.ts | 44 +++++++++- src/commands/doctor-config-flow.ts | 10 +-- .../doctor-legacy-config.migrations.test.ts | 30 +++---- src/commands/doctor-legacy-config.test.ts | 8 +- src/commands/doctor-legacy-config.ts | 2 +- src/discord/accounts.test.ts | 58 +++++++++++++ src/slack/accounts.test.ts | 85 +++++++++++++++++++ src/telegram/accounts.test.ts | 69 +++++++++++++++ 12 files changed, 294 insertions(+), 30 deletions(-) create mode 100644 src/discord/accounts.test.ts create mode 100644 src/slack/accounts.test.ts diff --git a/docs/channels/discord.md b/docs/channels/discord.md index 31913842e4d..83f2cd4de48 100644 --- a/docs/channels/discord.md +++ b/docs/channels/discord.md @@ -376,6 +376,12 @@ Example: If DM policy is not open, unknown users are blocked (or prompted for pairing in `pairing` mode). + Multi-account precedence: + + - `channels.discord.accounts.default.allowFrom` applies only to the `default` account. + - Named accounts inherit `channels.discord.allowFrom` when their own `allowFrom` is unset. + - Named accounts do not inherit `channels.discord.accounts.default.allowFrom`. + DM target format for delivery: - `user:` diff --git a/docs/channels/slack.md b/docs/channels/slack.md index 869df30ad99..22b7f816e37 100644 --- a/docs/channels/slack.md +++ b/docs/channels/slack.md @@ -152,6 +152,12 @@ For actions/directory reads, user token can be preferred when configured. For wr - `dm.groupEnabled` (group DMs default false) - `dm.groupChannels` (optional MPIM allowlist) + Multi-account precedence: + + - `channels.slack.accounts.default.allowFrom` applies only to the `default` account. + - Named accounts inherit `channels.slack.allowFrom` when their own `allowFrom` is unset. + - Named accounts do not inherit `channels.slack.accounts.default.allowFrom`. + Pairing in DMs uses `openclaw pairing approve slack `. diff --git a/docs/channels/telegram.md b/docs/channels/telegram.md index 46db95202b4..a4713d9c027 100644 --- a/docs/channels/telegram.md +++ b/docs/channels/telegram.md @@ -719,6 +719,10 @@ Primary reference: - `channels.telegram.allowFrom`: DM allowlist (numeric Telegram user IDs). `open` requires `"*"`. `openclaw doctor --fix` can resolve legacy `@username` entries to IDs. - `channels.telegram.groupPolicy`: `open | allowlist | disabled` (default: allowlist). - `channels.telegram.groupAllowFrom`: group sender allowlist (numeric Telegram user IDs). `openclaw doctor --fix` can resolve legacy `@username` entries to IDs. +- Multi-account precedence: + - `channels.telegram.accounts.default.allowFrom` and `channels.telegram.accounts.default.groupAllowFrom` apply only to the `default` account. + - Named accounts inherit `channels.telegram.allowFrom` and `channels.telegram.groupAllowFrom` when account-level values are unset. + - Named accounts do not inherit `channels.telegram.accounts.default.allowFrom` / `groupAllowFrom`. - `channels.telegram.groups`: per-group defaults + allowlist (use `"*"` for global defaults). - `channels.telegram.groups..groupPolicy`: per-group override for groupPolicy (`open | allowlist | disabled`). - `channels.telegram.groups..requireMention`: mention gating default. diff --git a/src/commands/doctor-config-flow.missing-default-account-bindings.integration.test.ts b/src/commands/doctor-config-flow.missing-default-account-bindings.integration.test.ts index 0856b3aa9b5..dae204ede43 100644 --- a/src/commands/doctor-config-flow.missing-default-account-bindings.integration.test.ts +++ b/src/commands/doctor-config-flow.missing-default-account-bindings.integration.test.ts @@ -14,7 +14,7 @@ vi.mock("./doctor-legacy-config.js", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, - normalizeLegacyConfigValues: (cfg: unknown) => ({ + normalizeCompatibilityConfigValues: (cfg: unknown) => ({ config: cfg, changes: [], }), diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts index c9c500388fd..752c96c0746 100644 --- a/src/commands/doctor-config-flow.test.ts +++ b/src/commands/doctor-config-flow.test.ts @@ -198,11 +198,11 @@ describe("doctor config flow", () => { }; expect(cfg.channels.telegram.allowFrom).toBeUndefined(); expect(cfg.channels.telegram.groupAllowFrom).toBeUndefined(); - expect(cfg.channels.telegram.accounts.default.allowFrom).toEqual(["111"]); - expect(cfg.channels.telegram.accounts.default.groupAllowFrom).toEqual(["222"]); expect(cfg.channels.telegram.groups["-100123"].allowFrom).toEqual(["333"]); expect(cfg.channels.telegram.groups["-100123"].topics["99"].allowFrom).toEqual(["444"]); expect(cfg.channels.telegram.accounts.alerts.allowFrom).toEqual(["444"]); + expect(cfg.channels.telegram.accounts.default.allowFrom).toEqual(["111"]); + expect(cfg.channels.telegram.accounts.default.groupAllowFrom).toEqual(["222"]); } finally { vi.unstubAllGlobals(); } @@ -261,11 +261,17 @@ describe("doctor config flow", () => { }); const cfg = result.cfg as unknown as { - channels: { discord: RepairedDiscordPolicy }; + channels: { + discord: Omit & { + allowFrom?: string[]; + accounts: Record & { + default: { allowFrom: string[] }; + }; + }; + }; }; expect(cfg.channels.discord.allowFrom).toBeUndefined(); - expect(cfg.channels.discord.accounts.default.allowFrom).toEqual(["123"]); expect(cfg.channels.discord.dm.allowFrom).toEqual(["456"]); expect(cfg.channels.discord.dm.groupChannels).toEqual(["789"]); expect(cfg.channels.discord.execApprovals.approvers).toEqual(["321"]); @@ -273,6 +279,7 @@ describe("doctor config flow", () => { expect(cfg.channels.discord.guilds["100"].roles).toEqual(["222"]); expect(cfg.channels.discord.guilds["100"].channels.general.users).toEqual(["333"]); expect(cfg.channels.discord.guilds["100"].channels.general.roles).toEqual(["444"]); + expect(cfg.channels.discord.accounts.default.allowFrom).toEqual(["123"]); expect(cfg.channels.discord.accounts.work.allowFrom).toEqual(["555"]); expect(cfg.channels.discord.accounts.work.dm.allowFrom).toEqual(["666"]); expect(cfg.channels.discord.accounts.work.dm.groupChannels).toEqual(["777"]); @@ -288,6 +295,35 @@ describe("doctor config flow", () => { }); }); + it("does not restore top-level allowFrom when config is intentionally default-account scoped", async () => { + const result = await runDoctorConfigWithInput({ + repair: true, + config: { + channels: { + discord: { + accounts: { + default: { token: "discord-default-token", allowFrom: ["123"] }, + work: { token: "discord-work-token" }, + }, + }, + }, + }, + run: loadAndMaybeMigrateDoctorConfig, + }); + + const cfg = result.cfg as { + channels: { + discord: { + allowFrom?: string[]; + accounts: Record; + }; + }; + }; + + expect(cfg.channels.discord.allowFrom).toBeUndefined(); + expect(cfg.channels.discord.accounts.default.allowFrom).toEqual(["123"]); + }); + it('adds allowFrom ["*"] when dmPolicy="open" and allowFrom is missing on repair', async () => { const result = await runDoctorConfigWithInput({ repair: true, diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index fffa67bff4d..f7f53d29ae6 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -40,7 +40,7 @@ import { import { listTelegramAccountIds, resolveTelegramAccount } from "../telegram/accounts.js"; import { note } from "../terminal/note.js"; import { isRecord, resolveHomeDir } from "../utils.js"; -import { normalizeLegacyConfigValues } from "./doctor-legacy-config.js"; +import { normalizeCompatibilityConfigValues } from "./doctor-legacy-config.js"; import type { DoctorOptions } from "./doctor-prompter.js"; import { autoMigrateLegacyStateDir } from "./doctor-state-migrations.js"; @@ -1474,7 +1474,7 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { if (snapshot.legacyIssues.length > 0) { note( snapshot.legacyIssues.map((issue) => `- ${issue.path}: ${issue.message}`).join("\n"), - "Legacy config keys detected", + "Compatibility config keys detected", ); const { config: migrated, changes } = migrateLegacyConfig(snapshot.parsed); if (changes.length > 0) { @@ -1485,18 +1485,18 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { pendingChanges = pendingChanges || changes.length > 0; } if (shouldRepair) { - // Legacy migration (2026-01-02, commit: 16420e5b) — normalize per-provider allowlists; move WhatsApp gating into channels.whatsapp.allowFrom. + // Compatibility migration (2026-01-02, commit: 16420e5b) — normalize per-provider allowlists; move WhatsApp gating into channels.whatsapp.allowFrom. if (migrated) { cfg = migrated; } } else { fixHints.push( - `Run "${formatCliCommand("openclaw doctor --fix")}" to apply legacy migrations.`, + `Run "${formatCliCommand("openclaw doctor --fix")}" to apply compatibility migrations.`, ); } } - const normalized = normalizeLegacyConfigValues(candidate); + const normalized = normalizeCompatibilityConfigValues(candidate); if (normalized.changes.length > 0) { note(normalized.changes.join("\n"), "Doctor changes"); candidate = normalized.config; diff --git a/src/commands/doctor-legacy-config.migrations.test.ts b/src/commands/doctor-legacy-config.migrations.test.ts index 775966bae1d..e364d1b7168 100644 --- a/src/commands/doctor-legacy-config.migrations.test.ts +++ b/src/commands/doctor-legacy-config.migrations.test.ts @@ -2,9 +2,9 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { normalizeLegacyConfigValues } from "./doctor-legacy-config.js"; +import { normalizeCompatibilityConfigValues } from "./doctor-legacy-config.js"; -describe("normalizeLegacyConfigValues", () => { +describe("normalizeCompatibilityConfigValues", () => { let previousOauthDir: string | undefined; let tempOauthDir: string | undefined; @@ -15,7 +15,7 @@ describe("normalizeLegacyConfigValues", () => { const expectNoWhatsAppConfigForLegacyAuth = (setup?: () => void) => { setup?.(); - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ messages: { ackReaction: "👀", ackReactionScope: "group-mentions" }, }); expect(res.config.channels?.whatsapp).toBeUndefined(); @@ -41,7 +41,7 @@ describe("normalizeLegacyConfigValues", () => { }); it("does not add whatsapp config when missing and no auth exists", () => { - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ messages: { ackReaction: "👀" }, }); @@ -50,7 +50,7 @@ describe("normalizeLegacyConfigValues", () => { }); it("copies legacy ack reaction when whatsapp config exists", () => { - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ messages: { ackReaction: "👀", ackReactionScope: "group-mentions" }, channels: { whatsapp: {} }, }); @@ -91,7 +91,7 @@ describe("normalizeLegacyConfigValues", () => { try { writeCreds(customDir); - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ messages: { ackReaction: "👀", ackReactionScope: "group-mentions" }, channels: { whatsapp: { accounts: { work: { authDir: customDir } } } }, }); @@ -107,7 +107,7 @@ describe("normalizeLegacyConfigValues", () => { }); it("migrates Slack dm.policy/dm.allowFrom to dmPolicy/allowFrom aliases", () => { - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ channels: { slack: { dm: { enabled: true, policy: "open", allowFrom: ["*"] }, @@ -125,7 +125,7 @@ describe("normalizeLegacyConfigValues", () => { }); it("migrates Discord account dm.policy/dm.allowFrom to dmPolicy/allowFrom aliases", () => { - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ channels: { discord: { accounts: { @@ -147,7 +147,7 @@ describe("normalizeLegacyConfigValues", () => { }); it("migrates Discord streaming boolean alias to streaming enum", () => { - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ channels: { discord: { streaming: true, @@ -173,7 +173,7 @@ describe("normalizeLegacyConfigValues", () => { }); it("migrates Discord legacy streamMode into streaming enum", () => { - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ channels: { discord: { streaming: false, @@ -191,7 +191,7 @@ describe("normalizeLegacyConfigValues", () => { }); it("migrates Telegram streamMode into streaming enum", () => { - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ channels: { telegram: { streamMode: "block", @@ -207,7 +207,7 @@ describe("normalizeLegacyConfigValues", () => { }); it("migrates Slack legacy streaming keys to unified config", () => { - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ channels: { slack: { streaming: false, @@ -226,7 +226,7 @@ describe("normalizeLegacyConfigValues", () => { }); it("moves missing default account from single-account top-level config when named accounts already exist", () => { - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ channels: { telegram: { enabled: true, @@ -264,7 +264,7 @@ describe("normalizeLegacyConfigValues", () => { }); it("migrates browser ssrfPolicy allowPrivateNetwork to dangerouslyAllowPrivateNetwork", () => { - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ browser: { ssrfPolicy: { allowPrivateNetwork: true, @@ -282,7 +282,7 @@ describe("normalizeLegacyConfigValues", () => { }); it("normalizes conflicting browser SSRF alias keys without changing effective behavior", () => { - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ browser: { ssrfPolicy: { allowPrivateNetwork: true, diff --git a/src/commands/doctor-legacy-config.test.ts b/src/commands/doctor-legacy-config.test.ts index 38e51757b21..acc256c13b5 100644 --- a/src/commands/doctor-legacy-config.test.ts +++ b/src/commands/doctor-legacy-config.test.ts @@ -1,9 +1,9 @@ import { describe, expect, it } from "vitest"; -import { normalizeLegacyConfigValues } from "./doctor-legacy-config.js"; +import { normalizeCompatibilityConfigValues } from "./doctor-legacy-config.js"; -describe("normalizeLegacyConfigValues preview streaming aliases", () => { +describe("normalizeCompatibilityConfigValues preview streaming aliases", () => { it("normalizes telegram boolean streaming aliases to enum", () => { - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ channels: { telegram: { streaming: false, @@ -17,7 +17,7 @@ describe("normalizeLegacyConfigValues preview streaming aliases", () => { }); it("normalizes discord boolean streaming aliases to enum", () => { - const res = normalizeLegacyConfigValues({ + const res = normalizeCompatibilityConfigValues({ channels: { discord: { streaming: true, diff --git a/src/commands/doctor-legacy-config.ts b/src/commands/doctor-legacy-config.ts index 35cd5fba277..4d8117bd841 100644 --- a/src/commands/doctor-legacy-config.ts +++ b/src/commands/doctor-legacy-config.ts @@ -8,7 +8,7 @@ import { } from "../config/discord-preview-streaming.js"; import { DEFAULT_ACCOUNT_ID } from "../routing/session-key.js"; -export function normalizeLegacyConfigValues(cfg: OpenClawConfig): { +export function normalizeCompatibilityConfigValues(cfg: OpenClawConfig): { config: OpenClawConfig; changes: string[]; } { diff --git a/src/discord/accounts.test.ts b/src/discord/accounts.test.ts new file mode 100644 index 00000000000..6fd11965a07 --- /dev/null +++ b/src/discord/accounts.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "vitest"; +import { resolveDiscordAccount } from "./accounts.js"; + +describe("resolveDiscordAccount allowFrom precedence", () => { + it("prefers accounts.default.allowFrom over top-level for default account", () => { + const resolved = resolveDiscordAccount({ + cfg: { + channels: { + discord: { + allowFrom: ["top"], + accounts: { + default: { allowFrom: ["default"], token: "token-default" }, + }, + }, + }, + }, + accountId: "default", + }); + + expect(resolved.config.allowFrom).toEqual(["default"]); + }); + + it("falls back to top-level allowFrom for named account without override", () => { + const resolved = resolveDiscordAccount({ + cfg: { + channels: { + discord: { + allowFrom: ["top"], + accounts: { + work: { token: "token-work" }, + }, + }, + }, + }, + accountId: "work", + }); + + expect(resolved.config.allowFrom).toEqual(["top"]); + }); + + it("does not inherit default account allowFrom for named account when top-level is absent", () => { + const resolved = resolveDiscordAccount({ + cfg: { + channels: { + discord: { + accounts: { + default: { allowFrom: ["default"], token: "token-default" }, + work: { token: "token-work" }, + }, + }, + }, + }, + accountId: "work", + }); + + expect(resolved.config.allowFrom).toBeUndefined(); + }); +}); diff --git a/src/slack/accounts.test.ts b/src/slack/accounts.test.ts new file mode 100644 index 00000000000..d89d29bbbb6 --- /dev/null +++ b/src/slack/accounts.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, it } from "vitest"; +import { resolveSlackAccount } from "./accounts.js"; + +describe("resolveSlackAccount allowFrom precedence", () => { + it("prefers accounts.default.allowFrom over top-level for default account", () => { + const resolved = resolveSlackAccount({ + cfg: { + channels: { + slack: { + allowFrom: ["top"], + accounts: { + default: { + botToken: "xoxb-default", + appToken: "xapp-default", + allowFrom: ["default"], + }, + }, + }, + }, + }, + accountId: "default", + }); + + expect(resolved.config.allowFrom).toEqual(["default"]); + }); + + it("falls back to top-level allowFrom for named account without override", () => { + const resolved = resolveSlackAccount({ + cfg: { + channels: { + slack: { + allowFrom: ["top"], + accounts: { + work: { botToken: "xoxb-work", appToken: "xapp-work" }, + }, + }, + }, + }, + accountId: "work", + }); + + expect(resolved.config.allowFrom).toEqual(["top"]); + }); + + it("does not inherit default account allowFrom for named account when top-level is absent", () => { + const resolved = resolveSlackAccount({ + cfg: { + channels: { + slack: { + accounts: { + default: { + botToken: "xoxb-default", + appToken: "xapp-default", + allowFrom: ["default"], + }, + work: { botToken: "xoxb-work", appToken: "xapp-work" }, + }, + }, + }, + }, + accountId: "work", + }); + + expect(resolved.config.allowFrom).toBeUndefined(); + }); + + it("falls back to top-level dm.allowFrom when allowFrom alias is unset", () => { + const resolved = resolveSlackAccount({ + cfg: { + channels: { + slack: { + dm: { allowFrom: ["U123"] }, + accounts: { + work: { botToken: "xoxb-work", appToken: "xapp-work" }, + }, + }, + }, + }, + accountId: "work", + }); + + expect(resolved.config.allowFrom).toBeUndefined(); + expect(resolved.config.dm?.allowFrom).toEqual(["U123"]); + }); +}); diff --git a/src/telegram/accounts.test.ts b/src/telegram/accounts.test.ts index 3eaee29819b..919ca989fe3 100644 --- a/src/telegram/accounts.test.ts +++ b/src/telegram/accounts.test.ts @@ -99,3 +99,72 @@ describe("resolveTelegramAccount", () => { expect(lines).toContain("resolve { accountId: 'work', enabled: true, tokenSource: 'config' }"); }); }); + +describe("resolveTelegramAccount allowFrom precedence", () => { + it("prefers accounts.default allowlists over top-level for default account", () => { + const resolved = resolveTelegramAccount({ + cfg: { + channels: { + telegram: { + allowFrom: ["top"], + groupAllowFrom: ["top-group"], + accounts: { + default: { + botToken: "123:default", + allowFrom: ["default"], + groupAllowFrom: ["default-group"], + }, + }, + }, + }, + }, + accountId: "default", + }); + + expect(resolved.config.allowFrom).toEqual(["default"]); + expect(resolved.config.groupAllowFrom).toEqual(["default-group"]); + }); + + it("falls back to top-level allowlists for named account without overrides", () => { + const resolved = resolveTelegramAccount({ + cfg: { + channels: { + telegram: { + allowFrom: ["top"], + groupAllowFrom: ["top-group"], + accounts: { + work: { botToken: "123:work" }, + }, + }, + }, + }, + accountId: "work", + }); + + expect(resolved.config.allowFrom).toEqual(["top"]); + expect(resolved.config.groupAllowFrom).toEqual(["top-group"]); + }); + + it("does not inherit default account allowlists for named account when top-level is absent", () => { + const resolved = resolveTelegramAccount({ + cfg: { + channels: { + telegram: { + accounts: { + default: { + botToken: "123:default", + allowFrom: ["default"], + groupAllowFrom: ["default-group"], + }, + work: { botToken: "123:work" }, + }, + }, + }, + }, + accountId: "work", + }); + + expect(resolved.config.allowFrom).toBeUndefined(); + expect(resolved.config.groupAllowFrom).toBeUndefined(); + }); +});