fix(config): resolve DM allowFrom via canonical-or-legacy before warning

The generic dmPolicy/allowFrom warning read only the canonical top-level
allowFrom, so channels that keep their wildcard under the legacy dm.allowFrom
alias (e.g. Discord/Slack, mode=topOnly/topOrNested) got a false 'all DMs
dropped' warning even though runtime honors dm.allowFrom. Resolve policy and
allowFrom through the shared resolveChannelDm* helpers with the channel's
dmAllowFromMode (matching runtime and doctor), and skip nestedOnly channels
whose canonical fields live under dm.* and do not match this warning's
top-level paths. Adds a Discord legacy-alias regression test.

Addresses ClawSweeper review finding P1 (false positives on legacy dm.allowFrom).
This commit is contained in:
Alex Knight
2026-06-16 07:30:02 +10:00
parent 6810c67f0c
commit 90e72a67a3
2 changed files with 47 additions and 20 deletions

View File

@@ -393,6 +393,27 @@ describe("validateConfigObjectWithPlugins channel metadata (applyDefaults: true)
false,
);
});
it('does not warn when dmPolicy="open" is satisfied by the legacy dm.allowFrom alias', () => {
// Runtime resolves allowFrom as canonical `allowFrom` ?? legacy `dm.allowFrom`, so a
// top-level-canonical channel (Discord) that keeps its wildcard under `dm.allowFrom`
// is valid and must not produce a false "all DMs dropped" warning.
const result = validateConfigObjectWithPlugins({
channels: {
discord: {
enabled: true,
token: "test-token",
dmPolicy: "open",
dm: { allowFrom: ["*"] },
},
},
});
expect(result.ok).toBe(true);
expect(result.warnings.some((warning) => warning.path === "channels.discord.allowFrom")).toBe(
false,
);
});
});
describe("validateConfigObjectRawWithPlugins channel metadata", () => {

View File

@@ -5,6 +5,11 @@ import { isCanonicalDottedDecimalIPv4, isLoopbackIpAddress } from "@openclaw/net
import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce";
import { sanitizeForLog } from "../../packages/terminal-core/src/ansi.js";
import { resolveAgentWorkspaceDir, resolveDefaultAgentId } from "../agents/agent-scope.js";
import {
resolveChannelDmAllowFrom,
resolveChannelDmPolicy,
} from "../channels/plugins/dm-access.js";
import { getDoctorChannelCapabilities } from "../commands/doctor/channel-capabilities.js";
import { isPathInside } from "../infra/path-guards.js";
import { planManifestModelCatalogSuppressions } from "../model-catalog/index.js";
import {
@@ -316,15 +321,6 @@ function formatRawChannelConfigIssueMessage(message: string): string {
return `invalid config: ${message}`;
}
function asAllowFromList(value: unknown): Array<string | number> | undefined {
if (!Array.isArray(value)) {
return undefined;
}
return value.filter(
(entry): entry is string | number => typeof entry === "string" || typeof entry === "number",
);
}
function buildDmPolicyDependencyWarning(params: {
channelId: string;
accountId?: string;
@@ -344,11 +340,20 @@ function buildDmPolicyDependencyWarning(params: {
return { path: allowFromPath, message };
}
// Channel map keys that are not channels and must be skipped while scanning DM policy.
const DM_POLICY_PSEUDO_CHANNEL_KEYS = new Set(["defaults", "modelByChannel", "tools"]);
/**
* Surface dmPolicy/allowFrom dependency problems generically for every channel that
* uses the shared top-level dmPolicy/allowFrom contract. These configs parse fine but
* drop every DM at runtime, so we warn (rather than reject) to stay consistent with
* `security audit`/`doctor` and avoid breaking existing-but-usable configs on upgrade.
* exposes DM policy via the canonical top-level `dmPolicy`/`allowFrom` fields. These
* configs parse fine but drop every DM at runtime, so we warn (rather than reject) to
* stay consistent with `security audit`/`doctor` and avoid breaking existing-but-usable
* configs on upgrade.
*
* Resolution goes through the shared DM-access helpers so the warning matches the
* effective policy/allowFrom the runtime sees, including the legacy `dm.*` aliases and
* account->channel inheritance. `nestedOnly` channels (canonical fields under `dm.*`)
* are skipped because their config shape does not match this warning's top-level paths.
*/
function collectChannelDmPolicyDependencyWarnings(config: OpenClawConfig): ConfigValidationIssue[] {
if (!config.channels || !isRecord(config.channels)) {
@@ -356,15 +361,16 @@ function collectChannelDmPolicyDependencyWarnings(config: OpenClawConfig): Confi
}
const warnings: ConfigValidationIssue[] = [];
for (const [channelId, channelValue] of Object.entries(config.channels)) {
if (channelId === "defaults" || channelId === "modelByChannel" || !isRecord(channelValue)) {
if (DM_POLICY_PSEUDO_CHANNEL_KEYS.has(channelId) || !isRecord(channelValue)) {
continue;
}
const mode = getDoctorChannelCapabilities(channelId).dmAllowFromMode;
if (mode === "nestedOnly") {
continue;
}
const channelPolicy =
typeof channelValue.dmPolicy === "string" ? channelValue.dmPolicy : undefined;
const channelAllowFrom = asAllowFromList(channelValue.allowFrom);
const channelViolation = evaluateDmPolicyAllowFromDependency({
policy: channelPolicy,
allowFrom: channelAllowFrom,
policy: resolveChannelDmPolicy({ account: channelValue, mode }),
allowFrom: resolveChannelDmAllowFrom({ account: channelValue, mode }),
});
if (channelViolation) {
warnings.push(buildDmPolicyDependencyWarning({ channelId, violation: channelViolation }));
@@ -377,8 +383,8 @@ function collectChannelDmPolicyDependencyWarnings(config: OpenClawConfig): Confi
continue;
}
const accountViolation = evaluateDmPolicyAllowFromDependency({
policy: typeof accountValue.dmPolicy === "string" ? accountValue.dmPolicy : channelPolicy,
allowFrom: asAllowFromList(accountValue.allowFrom) ?? channelAllowFrom,
policy: resolveChannelDmPolicy({ account: accountValue, parent: channelValue, mode }),
allowFrom: resolveChannelDmAllowFrom({ account: accountValue, parent: channelValue, mode }),
});
if (accountViolation) {
warnings.push(