refactor(doctor): extract provider and shared config helpers (#51753)

* refactor(doctor): add shared doctor types

* refactor(doctor): add shared allowlist helpers

* refactor(doctor): extract empty allowlist warnings

* refactor(doctor): extract telegram allowfrom scanning

* refactor(doctor): extract telegram allowfrom repair

* refactor(doctor): extract discord id repair

* refactor(doctor): add shared object helpers

* refactor(doctor): extract mutable allowlist scanning

* refactor(doctor): extract open-policy allowfrom repair

* refactor(doctor): extract allowlist policy repair

* fix(doctor): unblock discord provider refactor checks

* refactor(doctor): fix provider layering in shared warnings
This commit is contained in:
Vincent Koc
2026-03-21 10:09:38 -07:00
committed by GitHub
parent e24bf22f98
commit 4e979ea6ca
17 changed files with 1493 additions and 1138 deletions

File diff suppressed because it is too large Load Diff

View File

@@ -0,0 +1,69 @@
import { describe, expect, it } from "vitest";
import type { OpenClawConfig } from "../../../config/config.js";
import { maybeRepairDiscordNumericIds, scanDiscordNumericIdEntries } from "./discord.js";
describe("doctor discord provider repairs", () => {
it("finds numeric id entries across discord scopes", () => {
const cfg = {
channels: {
discord: {
allowFrom: [123],
dm: { allowFrom: ["ok"], groupChannels: [456] },
execApprovals: { approvers: [789] },
guilds: {
main: {
users: [111],
roles: [222],
channels: {
general: {
users: [333],
roles: [444],
},
},
},
},
},
},
} as unknown as OpenClawConfig;
const hits = scanDiscordNumericIdEntries(cfg);
expect(hits.map((hit) => hit.path)).toEqual([
"channels.discord.allowFrom[0]",
"channels.discord.dm.groupChannels[0]",
"channels.discord.execApprovals.approvers[0]",
"channels.discord.guilds.main.users[0]",
"channels.discord.guilds.main.roles[0]",
"channels.discord.guilds.main.channels.general.users[0]",
"channels.discord.guilds.main.channels.general.roles[0]",
]);
});
it("repairs numeric discord ids into strings", () => {
const cfg = {
channels: {
discord: {
allowFrom: [123],
accounts: {
work: {
execApprovals: { approvers: [456] },
},
},
},
},
} as unknown as OpenClawConfig;
const result = maybeRepairDiscordNumericIds(cfg);
expect(result.changes).toEqual([
expect.stringContaining("channels.discord.allowFrom: converted 1 numeric entry to strings"),
expect.stringContaining(
"channels.discord.accounts.work.execApprovals.approvers: converted 1 numeric entry to strings",
),
]);
expect(result.config.channels?.discord?.allowFrom).toEqual(["123"]);
expect(result.config.channels?.discord?.accounts?.work?.execApprovals?.approvers).toEqual([
"456",
]);
});
});

View File

@@ -0,0 +1,161 @@
import type { OpenClawConfig } from "../../../config/config.js";
import { asObjectRecord } from "../shared/object.js";
import type { DoctorAccountRecord } from "../types.js";
type DiscordNumericIdHit = { path: string; entry: number };
type DiscordIdListRef = {
pathLabel: string;
holder: Record<string, unknown>;
key: string;
};
export function collectDiscordAccountScopes(
cfg: OpenClawConfig,
): Array<{ prefix: string; account: DoctorAccountRecord }> {
const scopes: Array<{ prefix: string; account: DoctorAccountRecord }> = [];
const discord = asObjectRecord(cfg.channels?.discord);
if (!discord) {
return scopes;
}
scopes.push({ prefix: "channels.discord", account: discord });
const accounts = asObjectRecord(discord.accounts);
if (!accounts) {
return scopes;
}
for (const key of Object.keys(accounts)) {
const account = asObjectRecord(accounts[key]);
if (!account) {
continue;
}
scopes.push({ prefix: `channels.discord.accounts.${key}`, account });
}
return scopes;
}
export function collectDiscordIdLists(
prefix: string,
account: DoctorAccountRecord,
): DiscordIdListRef[] {
const refs: DiscordIdListRef[] = [
{ pathLabel: `${prefix}.allowFrom`, holder: account, key: "allowFrom" },
];
const dm = asObjectRecord(account.dm);
if (dm) {
refs.push({ pathLabel: `${prefix}.dm.allowFrom`, holder: dm, key: "allowFrom" });
refs.push({ pathLabel: `${prefix}.dm.groupChannels`, holder: dm, key: "groupChannels" });
}
const execApprovals = asObjectRecord(account.execApprovals);
if (execApprovals) {
refs.push({
pathLabel: `${prefix}.execApprovals.approvers`,
holder: execApprovals,
key: "approvers",
});
}
const guilds = asObjectRecord(account.guilds);
if (!guilds) {
return refs;
}
for (const guildId of Object.keys(guilds)) {
const guild = asObjectRecord(guilds[guildId]);
if (!guild) {
continue;
}
refs.push({ pathLabel: `${prefix}.guilds.${guildId}.users`, holder: guild, key: "users" });
refs.push({ pathLabel: `${prefix}.guilds.${guildId}.roles`, holder: guild, key: "roles" });
const channels = asObjectRecord(guild.channels);
if (!channels) {
continue;
}
for (const channelId of Object.keys(channels)) {
const channel = asObjectRecord(channels[channelId]);
if (!channel) {
continue;
}
refs.push({
pathLabel: `${prefix}.guilds.${guildId}.channels.${channelId}.users`,
holder: channel,
key: "users",
});
refs.push({
pathLabel: `${prefix}.guilds.${guildId}.channels.${channelId}.roles`,
holder: channel,
key: "roles",
});
}
}
return refs;
}
export function scanDiscordNumericIdEntries(cfg: OpenClawConfig): DiscordNumericIdHit[] {
const hits: DiscordNumericIdHit[] = [];
const scanList = (pathLabel: string, list: unknown) => {
if (!Array.isArray(list)) {
return;
}
for (const [index, entry] of list.entries()) {
if (typeof entry !== "number") {
continue;
}
hits.push({ path: `${pathLabel}[${index}]`, entry });
}
};
for (const scope of collectDiscordAccountScopes(cfg)) {
for (const ref of collectDiscordIdLists(scope.prefix, scope.account)) {
scanList(ref.pathLabel, ref.holder[ref.key]);
}
}
return hits;
}
export function maybeRepairDiscordNumericIds(cfg: OpenClawConfig): {
config: OpenClawConfig;
changes: string[];
} {
const hits = scanDiscordNumericIdEntries(cfg);
if (hits.length === 0) {
return { config: cfg, changes: [] };
}
const next = structuredClone(cfg);
const changes: string[] = [];
const repairList = (pathLabel: string, holder: Record<string, unknown>, key: string) => {
const raw = holder[key];
if (!Array.isArray(raw)) {
return;
}
let converted = 0;
const updated = raw.map((entry) => {
if (typeof entry === "number") {
converted += 1;
return String(entry);
}
return entry;
});
if (converted === 0) {
return;
}
holder[key] = updated;
changes.push(
`- ${pathLabel}: converted ${converted} numeric ${converted === 1 ? "entry" : "entries"} to strings`,
);
};
for (const scope of collectDiscordAccountScopes(next)) {
for (const ref of collectDiscordIdLists(scope.prefix, scope.account)) {
repairList(ref.pathLabel, ref.holder, ref.key);
}
}
if (changes.length === 0) {
return { config: cfg, changes: [] };
}
return { config: next, changes };
}

View File

@@ -1,5 +1,8 @@
import { describe, expect, it } from "vitest";
import { collectTelegramGroupPolicyWarnings } from "./telegram.js";
import {
collectTelegramGroupPolicyWarnings,
scanTelegramAllowFromUsernameEntries,
} from "./telegram.js";
describe("doctor telegram provider warnings", () => {
it("shows first-run guidance when groups are not configured yet", () => {
@@ -53,4 +56,40 @@ describe("doctor telegram provider warnings", () => {
expect(warnings).toEqual([]);
});
it("finds non-numeric telegram allowFrom username entries across account scopes", () => {
const hits = scanTelegramAllowFromUsernameEntries({
channels: {
telegram: {
allowFrom: ["@top"],
groupAllowFrom: ["12345"],
accounts: {
work: {
allowFrom: ["tg:@work"],
groups: {
"-100123": {
allowFrom: ["topic-user"],
topics: {
"99": {
allowFrom: ["777", "@topic-user"],
},
},
},
},
},
},
},
},
});
expect(hits).toEqual([
{ path: "channels.telegram.allowFrom", entry: "@top" },
{ path: "channels.telegram.accounts.work.allowFrom", entry: "tg:@work" },
{ path: "channels.telegram.accounts.work.groups.-100123.allowFrom", entry: "topic-user" },
{
path: "channels.telegram.accounts.work.groups.-100123.topics.99.allowFrom",
entry: "@topic-user",
},
]);
});
});

View File

@@ -1,7 +1,293 @@
type DoctorAccountRecord = Record<string, unknown>;
import {
inspectTelegramAccount,
isNumericTelegramUserId,
listTelegramAccountIds,
lookupTelegramChatId,
normalizeTelegramAllowFromEntry,
} from "../../../../extensions/telegram/api.js";
import { resolveCommandSecretRefsViaGateway } from "../../../cli/command-secret-gateway.js";
import { getChannelsCommandSecretTargetIds } from "../../../cli/command-secret-targets.js";
import type { OpenClawConfig } from "../../../config/config.js";
import type { TelegramNetworkConfig } from "../../../config/types.telegram.js";
import { resolveTelegramAccount } from "../../../plugin-sdk/account-resolution.js";
import { describeUnknownError } from "../../../secrets/shared.js";
import { hasAllowFromEntries } from "../shared/allowlist.js";
import { asObjectRecord } from "../shared/object.js";
import type { DoctorAccountRecord, DoctorAllowFromList } from "../types.js";
function hasAllowFromEntries(list?: Array<string | number>) {
return Array.isArray(list) && list.map((v) => String(v).trim()).filter(Boolean).length > 0;
type TelegramAllowFromUsernameHit = { path: string; entry: string };
type TelegramAllowFromListRef = {
pathLabel: string;
holder: Record<string, unknown>;
key: "allowFrom" | "groupAllowFrom";
};
type ResolvedTelegramLookupAccount = {
token: string;
apiRoot?: string;
proxyUrl?: string;
network?: TelegramNetworkConfig;
};
export function collectTelegramAccountScopes(
cfg: OpenClawConfig,
): Array<{ prefix: string; account: Record<string, unknown> }> {
const scopes: Array<{ prefix: string; account: Record<string, unknown> }> = [];
const telegram = asObjectRecord(cfg.channels?.telegram);
if (!telegram) {
return scopes;
}
scopes.push({ prefix: "channels.telegram", account: telegram });
const accounts = asObjectRecord(telegram.accounts);
if (!accounts) {
return scopes;
}
for (const key of Object.keys(accounts)) {
const account = asObjectRecord(accounts[key]);
if (!account) {
continue;
}
scopes.push({ prefix: `channels.telegram.accounts.${key}`, account });
}
return scopes;
}
export function collectTelegramAllowFromLists(
prefix: string,
account: Record<string, unknown>,
): TelegramAllowFromListRef[] {
const refs: TelegramAllowFromListRef[] = [
{ pathLabel: `${prefix}.allowFrom`, holder: account, key: "allowFrom" },
{ pathLabel: `${prefix}.groupAllowFrom`, holder: account, key: "groupAllowFrom" },
];
const groups = asObjectRecord(account.groups);
if (!groups) {
return refs;
}
for (const groupId of Object.keys(groups)) {
const group = asObjectRecord(groups[groupId]);
if (!group) {
continue;
}
refs.push({
pathLabel: `${prefix}.groups.${groupId}.allowFrom`,
holder: group,
key: "allowFrom",
});
const topics = asObjectRecord(group.topics);
if (!topics) {
continue;
}
for (const topicId of Object.keys(topics)) {
const topic = asObjectRecord(topics[topicId]);
if (!topic) {
continue;
}
refs.push({
pathLabel: `${prefix}.groups.${groupId}.topics.${topicId}.allowFrom`,
holder: topic,
key: "allowFrom",
});
}
}
return refs;
}
export function scanTelegramAllowFromUsernameEntries(
cfg: OpenClawConfig,
): TelegramAllowFromUsernameHit[] {
const hits: TelegramAllowFromUsernameHit[] = [];
const scanList = (pathLabel: string, list: unknown) => {
if (!Array.isArray(list)) {
return;
}
for (const entry of list) {
const normalized = normalizeTelegramAllowFromEntry(entry);
if (!normalized || normalized === "*") {
continue;
}
if (isNumericTelegramUserId(normalized)) {
continue;
}
hits.push({ path: pathLabel, entry: String(entry).trim() });
}
};
for (const scope of collectTelegramAccountScopes(cfg)) {
for (const ref of collectTelegramAllowFromLists(scope.prefix, scope.account)) {
scanList(ref.pathLabel, ref.holder[ref.key]);
}
}
return hits;
}
export async function maybeRepairTelegramAllowFromUsernames(cfg: OpenClawConfig): Promise<{
config: OpenClawConfig;
changes: string[];
}> {
const hits = scanTelegramAllowFromUsernameEntries(cfg);
if (hits.length === 0) {
return { config: cfg, changes: [] };
}
const { resolvedConfig } = await resolveCommandSecretRefsViaGateway({
config: cfg,
commandName: "doctor --fix",
targetIds: getChannelsCommandSecretTargetIds(),
mode: "read_only_status",
});
const hasConfiguredUnavailableToken = listTelegramAccountIds(cfg).some((accountId) => {
const inspected = inspectTelegramAccount({ cfg, accountId });
return inspected.enabled && inspected.tokenStatus === "configured_unavailable";
});
const tokenResolutionWarnings: string[] = [];
const lookupAccounts: ResolvedTelegramLookupAccount[] = [];
const seenLookupAccounts = new Set<string>();
for (const accountId of listTelegramAccountIds(resolvedConfig)) {
let account: NonNullable<ReturnType<typeof resolveTelegramAccount>>;
try {
account = resolveTelegramAccount({ cfg: resolvedConfig, accountId });
} catch (error) {
tokenResolutionWarnings.push(
`- Telegram account ${accountId}: failed to inspect bot token (${describeUnknownError(error)}).`,
);
continue;
}
const token = account.tokenSource === "none" ? "" : account.token.trim();
if (!token) {
continue;
}
const apiRoot = account.config.apiRoot?.trim() || undefined;
const proxyUrl = account.config.proxy?.trim() || undefined;
const network = account.config.network;
const cacheKey = `${token}::${apiRoot ?? ""}::${proxyUrl ?? ""}::${JSON.stringify(network ?? {})}`;
if (seenLookupAccounts.has(cacheKey)) {
continue;
}
seenLookupAccounts.add(cacheKey);
lookupAccounts.push({ token, apiRoot, proxyUrl, network });
}
if (lookupAccounts.length === 0) {
return {
config: cfg,
changes: [
...tokenResolutionWarnings,
hasConfiguredUnavailableToken
? `- Telegram allowFrom contains @username entries, but configured Telegram bot credentials are unavailable in this command path; cannot auto-resolve (start the gateway or make the secret source available, then rerun doctor --fix).`
: `- Telegram allowFrom contains @username entries, but no Telegram bot token is configured; cannot auto-resolve (run setup or replace with numeric sender IDs).`,
],
};
}
const resolveUserId = async (raw: string): Promise<string | null> => {
const trimmed = raw.trim();
if (!trimmed) {
return null;
}
const stripped = normalizeTelegramAllowFromEntry(trimmed);
if (!stripped || stripped === "*") {
return null;
}
if (isNumericTelegramUserId(stripped)) {
return stripped;
}
if (/\s/.test(stripped)) {
return null;
}
const username = stripped.startsWith("@") ? stripped : `@${stripped}`;
for (const account of lookupAccounts) {
const controller = new AbortController();
const timeout = setTimeout(() => controller.abort(), 4000);
try {
const id = await lookupTelegramChatId({
token: account.token,
chatId: username,
signal: controller.signal,
apiRoot: account.apiRoot,
proxyUrl: account.proxyUrl,
network: account.network,
});
if (id) {
return id;
}
} catch {
// ignore and try next token
} finally {
clearTimeout(timeout);
}
}
return null;
};
const changes: string[] = [];
const next = structuredClone(cfg);
const repairList = async (pathLabel: string, holder: Record<string, unknown>, key: string) => {
const raw = holder[key];
if (!Array.isArray(raw)) {
return;
}
const out: DoctorAllowFromList = [];
const replaced: Array<{ from: string; to: string }> = [];
for (const entry of raw) {
const normalized = normalizeTelegramAllowFromEntry(entry);
if (!normalized) {
continue;
}
if (normalized === "*") {
out.push("*");
continue;
}
if (isNumericTelegramUserId(normalized)) {
out.push(normalized);
continue;
}
const resolved = await resolveUserId(String(entry));
if (resolved) {
out.push(resolved);
replaced.push({ from: String(entry).trim(), to: resolved });
} else {
out.push(String(entry).trim());
}
}
const deduped: DoctorAllowFromList = [];
const seen = new Set<string>();
for (const entry of out) {
const keyValue = String(entry).trim();
if (!keyValue || seen.has(keyValue)) {
continue;
}
seen.add(keyValue);
deduped.push(entry);
}
holder[key] = deduped;
if (replaced.length > 0) {
for (const rep of replaced.slice(0, 5)) {
changes.push(`- ${pathLabel}: resolved ${rep.from} -> ${rep.to}`);
}
if (replaced.length > 5) {
changes.push(`- ${pathLabel}: resolved ${replaced.length - 5} more @username entries`);
}
}
};
for (const scope of collectTelegramAccountScopes(next)) {
for (const ref of collectTelegramAllowFromLists(scope.prefix, scope.account)) {
await repairList(ref.pathLabel, ref.holder, ref.key);
}
}
if (changes.length === 0) {
return { config: cfg, changes: [] };
}
return { config: next, changes };
}
function hasConfiguredGroups(account: DoctorAccountRecord, parent?: DoctorAccountRecord): boolean {
@@ -14,7 +300,7 @@ function hasConfiguredGroups(account: DoctorAccountRecord, parent?: DoctorAccoun
type CollectTelegramGroupPolicyWarningsParams = {
account: DoctorAccountRecord;
prefix: string;
effectiveAllowFrom?: Array<string | number>;
effectiveAllowFrom?: DoctorAllowFromList;
dmPolicy?: string;
parent?: DoctorAccountRecord;
};
@@ -38,8 +324,8 @@ export function collectTelegramGroupPolicyWarnings(
}
const rawGroupAllowFrom =
(params.account.groupAllowFrom as Array<string | number> | undefined) ??
(params.parent?.groupAllowFrom as Array<string | number> | undefined);
(params.account.groupAllowFrom as DoctorAllowFromList | undefined) ??
(params.parent?.groupAllowFrom as DoctorAllowFromList | undefined);
// Match runtime semantics: resolveGroupAllowFromSources treats empty arrays as
// unset and falls back to allowFrom.
const groupAllowFrom = hasAllowFromEntries(rawGroupAllowFrom) ? rawGroupAllowFrom : undefined;

View File

@@ -0,0 +1,11 @@
export type AllowFromMode = "topOnly" | "topOrNested" | "nestedOnly";
export function resolveAllowFromMode(channelName: string): AllowFromMode {
if (channelName === "googlechat") {
return "nestedOnly";
}
if (channelName === "discord" || channelName === "slack") {
return "topOrNested";
}
return "topOnly";
}

View File

@@ -0,0 +1,148 @@
import { normalizeChatChannelId } from "../../../channels/registry.js";
import type { OpenClawConfig } from "../../../config/config.js";
import { readChannelAllowFromStore } from "../../../pairing/pairing-store.js";
import { DEFAULT_ACCOUNT_ID, normalizeAccountId } from "../../../routing/session-key.js";
import { resolveAllowFromMode, type AllowFromMode } from "./allow-from-mode.js";
import { hasAllowFromEntries } from "./allowlist.js";
import { asObjectRecord } from "./object.js";
export async function maybeRepairAllowlistPolicyAllowFrom(cfg: OpenClawConfig): Promise<{
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 applyRecoveredAllowFrom = (params: {
account: Record<string, unknown>;
allowFrom: string[];
mode: AllowFromMode;
prefix: string;
}) => {
const count = params.allowFrom.length;
const noun = count === 1 ? "entry" : "entries";
if (params.mode === "nestedOnly") {
const dmEntry = params.account.dm;
const dm =
dmEntry && typeof dmEntry === "object" && !Array.isArray(dmEntry)
? (dmEntry as Record<string, unknown>)
: {};
dm.allowFrom = params.allowFrom;
params.account.dm = dm;
changes.push(
`- ${params.prefix}.dm.allowFrom: restored ${count} sender ${noun} from pairing store (dmPolicy="allowlist").`,
);
return;
}
if (params.mode === "topOrNested") {
const dmEntry = params.account.dm;
const dm =
dmEntry && typeof dmEntry === "object" && !Array.isArray(dmEntry)
? (dmEntry as Record<string, unknown>)
: undefined;
const nestedAllowFrom = dm?.allowFrom as Array<string | number> | undefined;
if (dm && !Array.isArray(params.account.allowFrom) && Array.isArray(nestedAllowFrom)) {
dm.allowFrom = params.allowFrom;
changes.push(
`- ${params.prefix}.dm.allowFrom: restored ${count} sender ${noun} from pairing store (dmPolicy="allowlist").`,
);
return;
}
}
params.account.allowFrom = params.allowFrom;
changes.push(
`- ${params.prefix}.allowFrom: restored ${count} sender ${noun} from pairing store (dmPolicy="allowlist").`,
);
};
const recoverAllowFromForAccount = async (params: {
channelName: string;
account: Record<string, unknown>;
accountId?: string;
prefix: string;
}) => {
const dmEntry = params.account.dm;
const dm =
dmEntry && typeof dmEntry === "object" && !Array.isArray(dmEntry)
? (dmEntry as Record<string, unknown>)
: undefined;
const dmPolicy =
(params.account.dmPolicy as string | undefined) ?? (dm?.policy as string | undefined);
if (dmPolicy !== "allowlist") {
return;
}
const topAllowFrom = params.account.allowFrom as Array<string | number> | undefined;
const nestedAllowFrom = dm?.allowFrom as Array<string | number> | undefined;
if (hasAllowFromEntries(topAllowFrom) || hasAllowFromEntries(nestedAllowFrom)) {
return;
}
const normalizedChannelId = (normalizeChatChannelId(params.channelName) ?? params.channelName)
.trim()
.toLowerCase();
if (!normalizedChannelId) {
return;
}
const normalizedAccountId = normalizeAccountId(params.accountId) || DEFAULT_ACCOUNT_ID;
const fromStore = await readChannelAllowFromStore(
normalizedChannelId,
process.env,
normalizedAccountId,
).catch(() => []);
const recovered = Array.from(new Set(fromStore.map((entry) => String(entry).trim()))).filter(
Boolean,
);
if (recovered.length === 0) {
return;
}
applyRecoveredAllowFrom({
account: params.account,
allowFrom: recovered,
mode: resolveAllowFromMode(params.channelName),
prefix: params.prefix,
});
};
const nextChannels = next.channels as Record<string, Record<string, unknown>>;
for (const [channelName, channelConfig] of Object.entries(nextChannels)) {
if (!channelConfig || typeof channelConfig !== "object") {
continue;
}
await recoverAllowFromForAccount({
channelName,
account: channelConfig,
prefix: `channels.${channelName}`,
});
const accounts = asObjectRecord(channelConfig.accounts);
if (!accounts) {
continue;
}
for (const [accountId, accountConfig] of Object.entries(accounts)) {
if (!accountConfig || typeof accountConfig !== "object") {
continue;
}
await recoverAllowFromForAccount({
channelName,
account: accountConfig as Record<string, unknown>,
accountId,
prefix: `channels.${channelName}.accounts.${accountId}`,
});
}
}
if (changes.length === 0) {
return { config: cfg, changes: [] };
}
return { config: next, changes };
}

View File

@@ -0,0 +1,15 @@
import { describe, expect, it } from "vitest";
import { hasAllowFromEntries } from "./allowlist.js";
describe("doctor allowlist helpers", () => {
it("returns false for missing and blank entries", () => {
expect(hasAllowFromEntries()).toBe(false);
expect(hasAllowFromEntries([])).toBe(false);
expect(hasAllowFromEntries(["", " "])).toBe(false);
});
it("returns true when at least one trimmed entry is present", () => {
expect(hasAllowFromEntries([" ", "12345"])).toBe(true);
expect(hasAllowFromEntries([0, " "])).toBe(true);
});
});

View File

@@ -0,0 +1,5 @@
import type { DoctorAllowFromList } from "../types.js";
export function hasAllowFromEntries(list?: DoctorAllowFromList) {
return Array.isArray(list) && list.map((v) => String(v).trim()).filter(Boolean).length > 0;
}

View File

@@ -0,0 +1,41 @@
import { describe, expect, it } from "vitest";
import { collectEmptyAllowlistPolicyWarningsForAccount } from "./empty-allowlist-policy.js";
describe("doctor empty allowlist policy warnings", () => {
it("warns when dm allowlist mode has no allowFrom entries", () => {
const warnings = collectEmptyAllowlistPolicyWarningsForAccount({
account: { dmPolicy: "allowlist" },
channelName: "signal",
doctorFixCommand: "openclaw doctor --fix",
prefix: "channels.signal",
});
expect(warnings).toEqual([
expect.stringContaining('channels.signal.dmPolicy is "allowlist" but allowFrom is empty'),
]);
});
it("warns when non-telegram group allowlist mode does not fall back to allowFrom", () => {
const warnings = collectEmptyAllowlistPolicyWarningsForAccount({
account: { groupPolicy: "allowlist" },
channelName: "imessage",
doctorFixCommand: "openclaw doctor --fix",
prefix: "channels.imessage",
});
expect(warnings).toEqual([
expect.stringContaining("this channel does not fall back to allowFrom"),
]);
});
it("stays quiet for channels that do not use sender-based group allowlists", () => {
const warnings = collectEmptyAllowlistPolicyWarningsForAccount({
account: { groupPolicy: "allowlist" },
channelName: "discord",
doctorFixCommand: "openclaw doctor --fix",
prefix: "channels.discord",
});
expect(warnings).toEqual([]);
});
});

View File

@@ -0,0 +1,107 @@
import type { DoctorAccountRecord, DoctorAllowFromList } from "../types.js";
import { hasAllowFromEntries } from "./allowlist.js";
type CollectEmptyAllowlistPolicyWarningsParams = {
account: DoctorAccountRecord;
channelName?: string;
doctorFixCommand: string;
parent?: DoctorAccountRecord;
prefix: string;
};
function usesSenderBasedGroupAllowlist(channelName?: string): boolean {
if (!channelName) {
return true;
}
// These channels enforce group access via channel/space config, not sender-based
// groupAllowFrom lists.
return !(channelName === "discord" || channelName === "slack" || channelName === "googlechat");
}
function allowsGroupAllowFromFallback(channelName?: string): boolean {
if (!channelName) {
return true;
}
// Keep doctor warnings aligned with runtime access semantics.
return !(
channelName === "googlechat" ||
channelName === "imessage" ||
channelName === "matrix" ||
channelName === "msteams" ||
channelName === "irc"
);
}
export function collectEmptyAllowlistPolicyWarningsForAccount(
params: CollectEmptyAllowlistPolicyWarningsParams,
): string[] {
const warnings: string[] = [];
const dmEntry = params.account.dm;
const dm =
dmEntry && typeof dmEntry === "object" && !Array.isArray(dmEntry)
? (dmEntry as DoctorAccountRecord)
: undefined;
const parentDmEntry = params.parent?.dm;
const parentDm =
parentDmEntry && typeof parentDmEntry === "object" && !Array.isArray(parentDmEntry)
? (parentDmEntry as DoctorAccountRecord)
: undefined;
const dmPolicy =
(params.account.dmPolicy as string | undefined) ??
(dm?.policy as string | undefined) ??
(params.parent?.dmPolicy as string | undefined) ??
(parentDm?.policy as string | undefined) ??
undefined;
const topAllowFrom =
(params.account.allowFrom as DoctorAllowFromList | undefined) ??
(params.parent?.allowFrom as DoctorAllowFromList | undefined);
const nestedAllowFrom = dm?.allowFrom as DoctorAllowFromList | undefined;
const parentNestedAllowFrom = parentDm?.allowFrom as DoctorAllowFromList | undefined;
const effectiveAllowFrom = topAllowFrom ?? nestedAllowFrom ?? parentNestedAllowFrom;
if (dmPolicy === "allowlist" && !hasAllowFromEntries(effectiveAllowFrom)) {
warnings.push(
`- ${params.prefix}.dmPolicy is "allowlist" but allowFrom is empty — all DMs will be blocked. Add sender IDs to ${params.prefix}.allowFrom, or run "${params.doctorFixCommand}" to auto-migrate from pairing store when entries exist.`,
);
}
const groupPolicy =
(params.account.groupPolicy as string | undefined) ??
(params.parent?.groupPolicy as string | undefined) ??
undefined;
if (groupPolicy !== "allowlist" || !usesSenderBasedGroupAllowlist(params.channelName)) {
return warnings;
}
if (params.channelName === "telegram") {
return warnings;
}
const rawGroupAllowFrom =
(params.account.groupAllowFrom as DoctorAllowFromList | undefined) ??
(params.parent?.groupAllowFrom as DoctorAllowFromList | undefined);
// Match runtime semantics: resolveGroupAllowFromSources treats empty arrays as
// unset and falls back to allowFrom.
const groupAllowFrom = hasAllowFromEntries(rawGroupAllowFrom) ? rawGroupAllowFrom : undefined;
const fallbackToAllowFrom = allowsGroupAllowFromFallback(params.channelName);
const effectiveGroupAllowFrom =
groupAllowFrom ?? (fallbackToAllowFrom ? effectiveAllowFrom : undefined);
if (hasAllowFromEntries(effectiveGroupAllowFrom)) {
return warnings;
}
if (fallbackToAllowFrom) {
warnings.push(
`- ${params.prefix}.groupPolicy is "allowlist" but groupAllowFrom (and allowFrom) is empty — all group messages will be silently dropped. Add sender IDs to ${params.prefix}.groupAllowFrom or ${params.prefix}.allowFrom, or set groupPolicy to "open".`,
);
} else {
warnings.push(
`- ${params.prefix}.groupPolicy is "allowlist" but groupAllowFrom is empty — this channel does not fall back to allowFrom, so all group messages will be silently dropped. Add sender IDs to ${params.prefix}.groupAllowFrom, or set groupPolicy to "open".`,
);
}
return warnings;
}

View File

@@ -0,0 +1,77 @@
import { describe, expect, it } from "vitest";
import { scanMutableAllowlistEntries } from "./mutable-allowlist.js";
describe("doctor mutable allowlist scanner", () => {
it("finds mutable discord, irc, and zalouser entries when dangerous matching is disabled", () => {
const hits = scanMutableAllowlistEntries({
channels: {
discord: {
allowFrom: ["alice"],
guilds: {
ops: {
users: ["bob"],
roles: [],
channels: {},
},
},
},
irc: {
allowFrom: ["charlie"],
groups: {
"#ops": {
allowFrom: ["dana"],
},
},
},
zalouser: {
groups: {
"Ops Room": { allow: true },
},
},
},
});
expect(hits).toEqual(
expect.arrayContaining([
expect.objectContaining({
channel: "discord",
path: "channels.discord.allowFrom",
entry: "alice",
}),
expect.objectContaining({
channel: "discord",
path: "channels.discord.guilds.ops.users",
entry: "bob",
}),
expect.objectContaining({
channel: "irc",
path: "channels.irc.allowFrom",
entry: "charlie",
}),
expect.objectContaining({
channel: "irc",
path: "channels.irc.groups.#ops.allowFrom",
entry: "dana",
}),
expect.objectContaining({
channel: "zalouser",
path: "channels.zalouser.groups",
entry: "Ops Room",
}),
]),
);
});
it("skips scopes that explicitly allow dangerous name matching", () => {
const hits = scanMutableAllowlistEntries({
channels: {
slack: {
dangerouslyAllowNameMatching: true,
allowFrom: ["alice"],
},
},
});
expect(hits).toEqual([]);
});
});

View File

@@ -0,0 +1,305 @@
import type { OpenClawConfig } from "../../../config/config.js";
import { collectProviderDangerousNameMatchingScopes } from "../../../config/dangerous-name-matching.js";
import {
isDiscordMutableAllowEntry,
isGoogleChatMutableAllowEntry,
isIrcMutableAllowEntry,
isMSTeamsMutableAllowEntry,
isMattermostMutableAllowEntry,
isSlackMutableAllowEntry,
isZalouserMutableGroupEntry,
} from "../../../security/mutable-allowlist-detectors.js";
import { asObjectRecord } from "./object.js";
export type MutableAllowlistHit = {
channel: string;
path: string;
entry: string;
dangerousFlagPath: string;
};
function addMutableAllowlistHits(params: {
hits: MutableAllowlistHit[];
pathLabel: string;
list: unknown;
detector: (entry: string) => boolean;
channel: string;
dangerousFlagPath: string;
}) {
if (!Array.isArray(params.list)) {
return;
}
for (const entry of params.list) {
const text = String(entry).trim();
if (!text || text === "*") {
continue;
}
if (!params.detector(text)) {
continue;
}
params.hits.push({
channel: params.channel,
path: params.pathLabel,
entry: text,
dangerousFlagPath: params.dangerousFlagPath,
});
}
}
export function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[] {
const hits: MutableAllowlistHit[] = [];
for (const scope of collectProviderDangerousNameMatchingScopes(cfg, "discord")) {
if (scope.dangerousNameMatchingEnabled) {
continue;
}
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.allowFrom`,
list: scope.account.allowFrom,
detector: isDiscordMutableAllowEntry,
channel: "discord",
dangerousFlagPath: scope.dangerousFlagPath,
});
const dm = asObjectRecord(scope.account.dm);
if (dm) {
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.dm.allowFrom`,
list: dm.allowFrom,
detector: isDiscordMutableAllowEntry,
channel: "discord",
dangerousFlagPath: scope.dangerousFlagPath,
});
}
const guilds = asObjectRecord(scope.account.guilds);
if (!guilds) {
continue;
}
for (const [guildId, guildRaw] of Object.entries(guilds)) {
const guild = asObjectRecord(guildRaw);
if (!guild) {
continue;
}
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.guilds.${guildId}.users`,
list: guild.users,
detector: isDiscordMutableAllowEntry,
channel: "discord",
dangerousFlagPath: scope.dangerousFlagPath,
});
const channels = asObjectRecord(guild.channels);
if (!channels) {
continue;
}
for (const [channelId, channelRaw] of Object.entries(channels)) {
const channel = asObjectRecord(channelRaw);
if (!channel) {
continue;
}
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.guilds.${guildId}.channels.${channelId}.users`,
list: channel.users,
detector: isDiscordMutableAllowEntry,
channel: "discord",
dangerousFlagPath: scope.dangerousFlagPath,
});
}
}
}
for (const scope of collectProviderDangerousNameMatchingScopes(cfg, "slack")) {
if (scope.dangerousNameMatchingEnabled) {
continue;
}
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.allowFrom`,
list: scope.account.allowFrom,
detector: isSlackMutableAllowEntry,
channel: "slack",
dangerousFlagPath: scope.dangerousFlagPath,
});
const dm = asObjectRecord(scope.account.dm);
if (dm) {
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.dm.allowFrom`,
list: dm.allowFrom,
detector: isSlackMutableAllowEntry,
channel: "slack",
dangerousFlagPath: scope.dangerousFlagPath,
});
}
const channels = asObjectRecord(scope.account.channels);
if (!channels) {
continue;
}
for (const [channelKey, channelRaw] of Object.entries(channels)) {
const channel = asObjectRecord(channelRaw);
if (!channel) {
continue;
}
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.channels.${channelKey}.users`,
list: channel.users,
detector: isSlackMutableAllowEntry,
channel: "slack",
dangerousFlagPath: scope.dangerousFlagPath,
});
}
}
for (const scope of collectProviderDangerousNameMatchingScopes(cfg, "googlechat")) {
if (scope.dangerousNameMatchingEnabled) {
continue;
}
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.groupAllowFrom`,
list: scope.account.groupAllowFrom,
detector: isGoogleChatMutableAllowEntry,
channel: "googlechat",
dangerousFlagPath: scope.dangerousFlagPath,
});
const dm = asObjectRecord(scope.account.dm);
if (dm) {
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.dm.allowFrom`,
list: dm.allowFrom,
detector: isGoogleChatMutableAllowEntry,
channel: "googlechat",
dangerousFlagPath: scope.dangerousFlagPath,
});
}
const groups = asObjectRecord(scope.account.groups);
if (!groups) {
continue;
}
for (const [groupKey, groupRaw] of Object.entries(groups)) {
const group = asObjectRecord(groupRaw);
if (!group) {
continue;
}
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.groups.${groupKey}.users`,
list: group.users,
detector: isGoogleChatMutableAllowEntry,
channel: "googlechat",
dangerousFlagPath: scope.dangerousFlagPath,
});
}
}
for (const scope of collectProviderDangerousNameMatchingScopes(cfg, "msteams")) {
if (scope.dangerousNameMatchingEnabled) {
continue;
}
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.allowFrom`,
list: scope.account.allowFrom,
detector: isMSTeamsMutableAllowEntry,
channel: "msteams",
dangerousFlagPath: scope.dangerousFlagPath,
});
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.groupAllowFrom`,
list: scope.account.groupAllowFrom,
detector: isMSTeamsMutableAllowEntry,
channel: "msteams",
dangerousFlagPath: scope.dangerousFlagPath,
});
}
for (const scope of collectProviderDangerousNameMatchingScopes(cfg, "mattermost")) {
if (scope.dangerousNameMatchingEnabled) {
continue;
}
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.allowFrom`,
list: scope.account.allowFrom,
detector: isMattermostMutableAllowEntry,
channel: "mattermost",
dangerousFlagPath: scope.dangerousFlagPath,
});
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.groupAllowFrom`,
list: scope.account.groupAllowFrom,
detector: isMattermostMutableAllowEntry,
channel: "mattermost",
dangerousFlagPath: scope.dangerousFlagPath,
});
}
for (const scope of collectProviderDangerousNameMatchingScopes(cfg, "irc")) {
if (scope.dangerousNameMatchingEnabled) {
continue;
}
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.allowFrom`,
list: scope.account.allowFrom,
detector: isIrcMutableAllowEntry,
channel: "irc",
dangerousFlagPath: scope.dangerousFlagPath,
});
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.groupAllowFrom`,
list: scope.account.groupAllowFrom,
detector: isIrcMutableAllowEntry,
channel: "irc",
dangerousFlagPath: scope.dangerousFlagPath,
});
const groups = asObjectRecord(scope.account.groups);
if (!groups) {
continue;
}
for (const [groupKey, groupRaw] of Object.entries(groups)) {
const group = asObjectRecord(groupRaw);
if (!group) {
continue;
}
addMutableAllowlistHits({
hits,
pathLabel: `${scope.prefix}.groups.${groupKey}.allowFrom`,
list: group.allowFrom,
detector: isIrcMutableAllowEntry,
channel: "irc",
dangerousFlagPath: scope.dangerousFlagPath,
});
}
}
for (const scope of collectProviderDangerousNameMatchingScopes(cfg, "zalouser")) {
if (scope.dangerousNameMatchingEnabled) {
continue;
}
const groups = asObjectRecord(scope.account.groups);
if (!groups) {
continue;
}
for (const entry of Object.keys(groups)) {
if (!isZalouserMutableGroupEntry(entry)) {
continue;
}
hits.push({
channel: "zalouser",
path: `${scope.prefix}.groups`,
entry,
dangerousFlagPath: scope.dangerousFlagPath,
});
}
}
return hits;
}

View File

@@ -0,0 +1,6 @@
export function asObjectRecord(value: unknown): Record<string, unknown> | null {
if (!value || typeof value !== "object" || Array.isArray(value)) {
return null;
}
return value as Record<string, unknown>;
}

View File

@@ -0,0 +1,54 @@
import { describe, expect, it } from "vitest";
import { maybeRepairOpenPolicyAllowFrom } from "./open-policy-allowfrom.js";
describe("doctor open-policy allowFrom repair", () => {
it('adds top-level wildcard when dmPolicy="open" has no allowFrom', () => {
const result = maybeRepairOpenPolicyAllowFrom({
channels: {
signal: {
dmPolicy: "open",
},
},
});
expect(result.changes).toEqual([
'- channels.signal.allowFrom: set to ["*"] (required by dmPolicy="open")',
]);
expect(result.config.channels?.signal?.allowFrom).toEqual(["*"]);
});
it("repairs nested-only googlechat dm allowFrom", () => {
const result = maybeRepairOpenPolicyAllowFrom({
channels: {
googlechat: {
dm: {
policy: "open",
},
},
},
});
expect(result.changes).toEqual([
'- channels.googlechat.dm.allowFrom: set to ["*"] (required by dmPolicy="open")',
]);
expect(result.config.channels?.googlechat?.dm?.allowFrom).toEqual(["*"]);
});
it("appends wildcard to discord nested dm allowFrom when top-level is absent", () => {
const result = maybeRepairOpenPolicyAllowFrom({
channels: {
discord: {
dm: {
policy: "open",
allowFrom: ["123"],
},
},
},
});
expect(result.changes).toEqual([
'- channels.discord.dm.allowFrom: added "*" (required by dmPolicy="open")',
]);
expect(result.config.channels?.discord?.dm?.allowFrom).toEqual(["123", "*"]);
});
});

View File

@@ -0,0 +1,114 @@
import type { OpenClawConfig } from "../../../config/config.js";
import { resolveAllowFromMode, type AllowFromMode } from "./allow-from-mode.js";
import { asObjectRecord } from "./object.js";
function hasWildcard(list?: Array<string | number>) {
return list?.some((v) => String(v).trim() === "*") ?? false;
}
export 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 = (
account: Record<string, unknown>,
prefix: string,
mode: AllowFromMode,
) => {
const dmEntry = account.dm;
const dm =
dmEntry && typeof dmEntry === "object" && !Array.isArray(dmEntry)
? (dmEntry as Record<string, unknown>)
: undefined;
const dmPolicy =
(account.dmPolicy as string | undefined) ?? (dm?.policy as string | undefined) ?? undefined;
if (dmPolicy !== "open") {
return;
}
const topAllowFrom = account.allowFrom as Array<string | number> | undefined;
const nestedAllowFrom = dm?.allowFrom as Array<string | number> | undefined;
if (mode === "nestedOnly") {
if (hasWildcard(nestedAllowFrom)) {
return;
}
if (dm && Array.isArray(nestedAllowFrom)) {
dm.allowFrom = [...nestedAllowFrom, "*"];
changes.push(`- ${prefix}.dm.allowFrom: added "*" (required by dmPolicy="open")`);
} else {
const nextDm = dm ?? {};
nextDm.allowFrom = ["*"];
account.dm = nextDm;
changes.push(`- ${prefix}.dm.allowFrom: set to ["*"] (required by dmPolicy="open")`);
}
return;
}
if (mode === "topOrNested") {
if (hasWildcard(topAllowFrom) || hasWildcard(nestedAllowFrom)) {
return;
}
if (Array.isArray(topAllowFrom)) {
account.allowFrom = [...topAllowFrom, "*"];
changes.push(`- ${prefix}.allowFrom: added "*" (required by dmPolicy="open")`);
} else if (dm && Array.isArray(nestedAllowFrom)) {
dm.allowFrom = [...nestedAllowFrom, "*"];
changes.push(`- ${prefix}.dm.allowFrom: added "*" (required by dmPolicy="open")`);
} else {
account.allowFrom = ["*"];
changes.push(`- ${prefix}.allowFrom: set to ["*"] (required by dmPolicy="open")`);
}
return;
}
if (hasWildcard(topAllowFrom)) {
return;
}
if (Array.isArray(topAllowFrom)) {
account.allowFrom = [...topAllowFrom, "*"];
changes.push(`- ${prefix}.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<string, Record<string, unknown>>;
for (const [channelName, channelConfig] of Object.entries(nextChannels)) {
if (!channelConfig || typeof channelConfig !== "object") {
continue;
}
const allowFromMode = resolveAllowFromMode(channelName);
ensureWildcard(channelConfig, `channels.${channelName}`, allowFromMode);
const accounts = asObjectRecord(channelConfig.accounts);
if (!accounts) {
continue;
}
for (const [accountName, accountConfig] of Object.entries(accounts)) {
if (accountConfig && typeof accountConfig === "object") {
ensureWildcard(
accountConfig as Record<string, unknown>,
`channels.${channelName}.accounts.${accountName}`,
allowFromMode,
);
}
}
}
if (changes.length === 0) {
return { config: cfg, changes: [] };
}
return { config: next, changes };
}

View File

@@ -0,0 +1,3 @@
export type DoctorAccountRecord = Record<string, unknown>;
export type DoctorAllowFromEntry = string | number;
export type DoctorAllowFromList = DoctorAllowFromEntry[];