fix(doctor): inherit dangerous name-matching flag in mutable allowlist scan

This commit is contained in:
Peter Steinberger
2026-02-24 01:16:14 +00:00
parent e5931554bf
commit 22467902ea
2 changed files with 86 additions and 33 deletions

View File

@@ -2,6 +2,7 @@ import fs from "node:fs/promises";
import path from "node:path"; import path from "node:path";
import { describe, expect, it, vi } from "vitest"; import { describe, expect, it, vi } from "vitest";
import { withTempHome } from "../../test/helpers/temp-home.js"; import { withTempHome } from "../../test/helpers/temp-home.js";
import * as noteModule from "../terminal/note.js";
import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js"; import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js";
import { runDoctorConfigWithInput } from "./doctor-config-flow.test-utils.js"; import { runDoctorConfigWithInput } from "./doctor-config-flow.test-utils.js";
@@ -54,6 +55,34 @@ describe("doctor config flow", () => {
}); });
}); });
it("does not warn on mutable account allowlists when dangerous name matching is inherited", async () => {
const noteSpy = vi.spyOn(noteModule, "note").mockImplementation(() => {});
try {
await runDoctorConfigWithInput({
config: {
channels: {
slack: {
dangerouslyAllowNameMatching: true,
accounts: {
work: {
allowFrom: ["alice"],
},
},
},
},
},
run: loadAndMaybeMigrateDoctorConfig,
});
const doctorWarnings = noteSpy.mock.calls
.filter((call) => call[1] === "Doctor warnings")
.map((call) => String(call[0]));
expect(doctorWarnings.some((line) => line.includes("mutable allowlist"))).toBe(false);
} finally {
noteSpy.mockRestore();
}
});
it("drops unknown keys on repair", async () => { it("drops unknown keys on repair", async () => {
const result = await runDoctorConfigWithInput({ const result = await runDoctorConfigWithInput({
repair: true, repair: true,

View File

@@ -192,6 +192,10 @@ function asObjectRecord(value: unknown): Record<string, unknown> | null {
return value as Record<string, unknown>; return value as Record<string, unknown>;
} }
function asOptionalBoolean(value: unknown): boolean | undefined {
return typeof value === "boolean" ? value : undefined;
}
function collectTelegramAccountScopes( function collectTelegramAccountScopes(
cfg: OpenClawConfig, cfg: OpenClawConfig,
): Array<{ prefix: string; account: Record<string, unknown> }> { ): Array<{ prefix: string; account: Record<string, unknown> }> {
@@ -585,11 +589,18 @@ type MutableAllowlistHit = {
dangerousFlagPath: string; dangerousFlagPath: string;
}; };
type ProviderAccountScope = {
prefix: string;
account: Record<string, unknown>;
dangerousNameMatchingEnabled: boolean;
dangerousFlagPath: string;
};
function collectProviderAccountScopes( function collectProviderAccountScopes(
cfg: OpenClawConfig, cfg: OpenClawConfig,
provider: string, provider: string,
): Array<{ prefix: string; account: Record<string, unknown> }> { ): ProviderAccountScope[] {
const scopes: Array<{ prefix: string; account: Record<string, unknown> }> = []; const scopes: ProviderAccountScope[] = [];
const channels = asObjectRecord(cfg.channels); const channels = asObjectRecord(cfg.channels);
if (!channels) { if (!channels) {
return scopes; return scopes;
@@ -598,7 +609,15 @@ function collectProviderAccountScopes(
if (!providerCfg) { if (!providerCfg) {
return scopes; return scopes;
} }
scopes.push({ prefix: `channels.${provider}`, account: providerCfg }); const providerPrefix = `channels.${provider}`;
const providerDangerousFlagPath = `${providerPrefix}.dangerouslyAllowNameMatching`;
const providerDangerousNameMatchingEnabled = providerCfg.dangerouslyAllowNameMatching === true;
scopes.push({
prefix: providerPrefix,
account: providerCfg,
dangerousNameMatchingEnabled: providerDangerousNameMatchingEnabled,
dangerousFlagPath: providerDangerousFlagPath,
});
const accounts = asObjectRecord(providerCfg.accounts); const accounts = asObjectRecord(providerCfg.accounts);
if (!accounts) { if (!accounts) {
return scopes; return scopes;
@@ -608,7 +627,18 @@ function collectProviderAccountScopes(
if (!account) { if (!account) {
continue; continue;
} }
scopes.push({ prefix: `channels.${provider}.accounts.${key}`, account }); const accountPrefix = `${providerPrefix}.accounts.${key}`;
const accountDangerousNameMatching = asOptionalBoolean(account.dangerouslyAllowNameMatching);
scopes.push({
prefix: accountPrefix,
account,
dangerousNameMatchingEnabled:
accountDangerousNameMatching ?? providerDangerousNameMatchingEnabled,
dangerousFlagPath:
accountDangerousNameMatching == null
? providerDangerousFlagPath
: `${accountPrefix}.dangerouslyAllowNameMatching`,
});
} }
return scopes; return scopes;
} }
@@ -733,8 +763,7 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
const hits: MutableAllowlistHit[] = []; const hits: MutableAllowlistHit[] = [];
for (const scope of collectProviderAccountScopes(cfg, "discord")) { for (const scope of collectProviderAccountScopes(cfg, "discord")) {
const dangerousFlagPath = `${scope.prefix}.dangerouslyAllowNameMatching`; if (scope.dangerousNameMatchingEnabled) {
if (scope.account.dangerouslyAllowNameMatching === true) {
continue; continue;
} }
addMutableAllowlistHits({ addMutableAllowlistHits({
@@ -743,7 +772,7 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: scope.account.allowFrom, list: scope.account.allowFrom,
detector: isDiscordMutableAllowEntry, detector: isDiscordMutableAllowEntry,
channel: "discord", channel: "discord",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
const dm = asObjectRecord(scope.account.dm); const dm = asObjectRecord(scope.account.dm);
if (dm) { if (dm) {
@@ -753,7 +782,7 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: dm.allowFrom, list: dm.allowFrom,
detector: isDiscordMutableAllowEntry, detector: isDiscordMutableAllowEntry,
channel: "discord", channel: "discord",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
} }
const guilds = asObjectRecord(scope.account.guilds); const guilds = asObjectRecord(scope.account.guilds);
@@ -771,7 +800,7 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: guild.users, list: guild.users,
detector: isDiscordMutableAllowEntry, detector: isDiscordMutableAllowEntry,
channel: "discord", channel: "discord",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
const channels = asObjectRecord(guild.channels); const channels = asObjectRecord(guild.channels);
if (!channels) { if (!channels) {
@@ -788,15 +817,14 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: channel.users, list: channel.users,
detector: isDiscordMutableAllowEntry, detector: isDiscordMutableAllowEntry,
channel: "discord", channel: "discord",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
} }
} }
} }
for (const scope of collectProviderAccountScopes(cfg, "slack")) { for (const scope of collectProviderAccountScopes(cfg, "slack")) {
const dangerousFlagPath = `${scope.prefix}.dangerouslyAllowNameMatching`; if (scope.dangerousNameMatchingEnabled) {
if (scope.account.dangerouslyAllowNameMatching === true) {
continue; continue;
} }
addMutableAllowlistHits({ addMutableAllowlistHits({
@@ -805,7 +833,7 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: scope.account.allowFrom, list: scope.account.allowFrom,
detector: isSlackMutableAllowEntry, detector: isSlackMutableAllowEntry,
channel: "slack", channel: "slack",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
const dm = asObjectRecord(scope.account.dm); const dm = asObjectRecord(scope.account.dm);
if (dm) { if (dm) {
@@ -815,7 +843,7 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: dm.allowFrom, list: dm.allowFrom,
detector: isSlackMutableAllowEntry, detector: isSlackMutableAllowEntry,
channel: "slack", channel: "slack",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
} }
const channels = asObjectRecord(scope.account.channels); const channels = asObjectRecord(scope.account.channels);
@@ -833,14 +861,13 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: channel.users, list: channel.users,
detector: isSlackMutableAllowEntry, detector: isSlackMutableAllowEntry,
channel: "slack", channel: "slack",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
} }
} }
for (const scope of collectProviderAccountScopes(cfg, "googlechat")) { for (const scope of collectProviderAccountScopes(cfg, "googlechat")) {
const dangerousFlagPath = `${scope.prefix}.dangerouslyAllowNameMatching`; if (scope.dangerousNameMatchingEnabled) {
if (scope.account.dangerouslyAllowNameMatching === true) {
continue; continue;
} }
addMutableAllowlistHits({ addMutableAllowlistHits({
@@ -849,7 +876,7 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: scope.account.groupAllowFrom, list: scope.account.groupAllowFrom,
detector: isGoogleChatMutableAllowEntry, detector: isGoogleChatMutableAllowEntry,
channel: "googlechat", channel: "googlechat",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
const dm = asObjectRecord(scope.account.dm); const dm = asObjectRecord(scope.account.dm);
if (dm) { if (dm) {
@@ -859,7 +886,7 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: dm.allowFrom, list: dm.allowFrom,
detector: isGoogleChatMutableAllowEntry, detector: isGoogleChatMutableAllowEntry,
channel: "googlechat", channel: "googlechat",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
} }
const groups = asObjectRecord(scope.account.groups); const groups = asObjectRecord(scope.account.groups);
@@ -877,14 +904,13 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: group.users, list: group.users,
detector: isGoogleChatMutableAllowEntry, detector: isGoogleChatMutableAllowEntry,
channel: "googlechat", channel: "googlechat",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
} }
} }
for (const scope of collectProviderAccountScopes(cfg, "msteams")) { for (const scope of collectProviderAccountScopes(cfg, "msteams")) {
const dangerousFlagPath = `${scope.prefix}.dangerouslyAllowNameMatching`; if (scope.dangerousNameMatchingEnabled) {
if (scope.account.dangerouslyAllowNameMatching === true) {
continue; continue;
} }
addMutableAllowlistHits({ addMutableAllowlistHits({
@@ -893,7 +919,7 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: scope.account.allowFrom, list: scope.account.allowFrom,
detector: isMSTeamsMutableAllowEntry, detector: isMSTeamsMutableAllowEntry,
channel: "msteams", channel: "msteams",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
addMutableAllowlistHits({ addMutableAllowlistHits({
hits, hits,
@@ -901,13 +927,12 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: scope.account.groupAllowFrom, list: scope.account.groupAllowFrom,
detector: isMSTeamsMutableAllowEntry, detector: isMSTeamsMutableAllowEntry,
channel: "msteams", channel: "msteams",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
} }
for (const scope of collectProviderAccountScopes(cfg, "mattermost")) { for (const scope of collectProviderAccountScopes(cfg, "mattermost")) {
const dangerousFlagPath = `${scope.prefix}.dangerouslyAllowNameMatching`; if (scope.dangerousNameMatchingEnabled) {
if (scope.account.dangerouslyAllowNameMatching === true) {
continue; continue;
} }
addMutableAllowlistHits({ addMutableAllowlistHits({
@@ -916,7 +941,7 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: scope.account.allowFrom, list: scope.account.allowFrom,
detector: isMattermostMutableAllowEntry, detector: isMattermostMutableAllowEntry,
channel: "mattermost", channel: "mattermost",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
addMutableAllowlistHits({ addMutableAllowlistHits({
hits, hits,
@@ -924,13 +949,12 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: scope.account.groupAllowFrom, list: scope.account.groupAllowFrom,
detector: isMattermostMutableAllowEntry, detector: isMattermostMutableAllowEntry,
channel: "mattermost", channel: "mattermost",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
} }
for (const scope of collectProviderAccountScopes(cfg, "irc")) { for (const scope of collectProviderAccountScopes(cfg, "irc")) {
const dangerousFlagPath = `${scope.prefix}.dangerouslyAllowNameMatching`; if (scope.dangerousNameMatchingEnabled) {
if (scope.account.dangerouslyAllowNameMatching === true) {
continue; continue;
} }
addMutableAllowlistHits({ addMutableAllowlistHits({
@@ -939,7 +963,7 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: scope.account.allowFrom, list: scope.account.allowFrom,
detector: isIrcMutableAllowEntry, detector: isIrcMutableAllowEntry,
channel: "irc", channel: "irc",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
addMutableAllowlistHits({ addMutableAllowlistHits({
hits, hits,
@@ -947,7 +971,7 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: scope.account.groupAllowFrom, list: scope.account.groupAllowFrom,
detector: isIrcMutableAllowEntry, detector: isIrcMutableAllowEntry,
channel: "irc", channel: "irc",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
const groups = asObjectRecord(scope.account.groups); const groups = asObjectRecord(scope.account.groups);
if (!groups) { if (!groups) {
@@ -964,7 +988,7 @@ function scanMutableAllowlistEntries(cfg: OpenClawConfig): MutableAllowlistHit[]
list: group.allowFrom, list: group.allowFrom,
detector: isIrcMutableAllowEntry, detector: isIrcMutableAllowEntry,
channel: "irc", channel: "irc",
dangerousFlagPath, dangerousFlagPath: scope.dangerousFlagPath,
}); });
} }
} }