mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-03 03:03:24 -04:00
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 <noreply@anthropic.com>
This commit is contained in:
committed by
Peter Steinberger
parent
71dad89193
commit
b05273de61
@@ -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(["*"]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string, unknown>,
|
||||
prefix: string,
|
||||
) => {
|
||||
const dmPolicy =
|
||||
(account.dmPolicy as string | undefined) ??
|
||||
((account.dm as Record<string, unknown> | undefined)?.policy as string | undefined);
|
||||
|
||||
if (dmPolicy !== "open") {
|
||||
return;
|
||||
}
|
||||
|
||||
// Check top-level allowFrom first, then nested dm.allowFrom
|
||||
const topAllowFrom = account.allowFrom as Array<string | number> | undefined;
|
||||
const dm = account.dm as Record<string, unknown> | undefined;
|
||||
const nestedAllowFrom = dm?.allowFrom as Array<string | number> | undefined;
|
||||
|
||||
const hasWildcard = (list?: Array<string | number>) =>
|
||||
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<string | number>).push("*");
|
||||
changes.push(`- ${prefix}.allowFrom: added "*" (required by dmPolicy="open")`);
|
||||
} else if (Array.isArray(nestedAllowFrom)) {
|
||||
(dm!.allowFrom as Array<string | number>).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<string, Record<string, unknown>>;
|
||||
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<string, Record<string, unknown>> | 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<string[]> {
|
||||
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);
|
||||
|
||||
Reference in New Issue
Block a user