mirror of https://github.com/openclaw/openclaw.git
fix: stabilize config default-leak landing tests (#56834) (thanks @openperf)
* fix(config): prevent AJV schema defaults from leaking into persisted config Fixes #56772. Ensures that channel and plugin AJV validations respect the applyDefaults option, preventing runtime defaults from being written to openclaw.json during doctor/update flows. * test: address review feedback on #56772 fix - Split validation.channel-metadata.test.ts into applyDefaults true/false cases (fixes CI) - Update io.write-config.test.ts regression test to use a mock plugin registry, ensuring it actually exercises the AJV default injection path * fix(config): revert applyDefaults passthrough to prevent required-field regression Codex-connector correctly identified that BlueBubbles channel schema marks enrichGroupParticipantsFromContacts as both default:true and required. Passing applyDefaults:false during write validation would cause required checks to fail, breaking writeConfigFile entirely. Reverted validation.ts to always use applyDefaults:true for channel/plugin AJV validation. The protection against default leakage into persisted config is fully handled by the persistCandidate change in io.ts (cfgToWrite uses the pre-validation merge-patched value, not validated.config). Updated validation.channel-metadata.test.ts to reflect this architecture. * fix(config): apply legacy web-search normalization to persistCandidate * fix: stabilize config default-leak landing tests (#56834) (thanks @openperf) --------- Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
parent
22f56433e0
commit
16df3de098
|
|
@ -86,6 +86,7 @@ Docs: https://docs.openclaw.ai
|
|||
- Cron/announce: preserve all deliverable text payloads for announce mode instead of collapsing to the last chunk, so multi-line cron reports deliver in full to Telegram forum topics.
|
||||
- Harden async approval followup delivery in webchat-only sessions (#57359) Thanks @joshavant.
|
||||
- Status: fix cache hit rate exceeding 100% by deriving denominator from prompt-side token fields instead of potentially undersized totalTokens. Fixes #26643.
|
||||
- Config/update: stop `openclaw doctor` write-backs from persisting plugin-injected channel defaults, so `openclaw update` no longer seeds config keys that later break service refresh validation. (#56834) Thanks @openperf.
|
||||
|
||||
## 2026.3.28
|
||||
|
||||
|
|
|
|||
|
|
@ -638,12 +638,14 @@ async function maybeRestartService(params: {
|
|||
invocationCwd: params.invocationCwd,
|
||||
});
|
||||
} catch (err) {
|
||||
if (!params.opts.json) {
|
||||
defaultRuntime.log(
|
||||
theme.warn(
|
||||
`Failed to refresh gateway service environment from updated install: ${String(err)}`,
|
||||
),
|
||||
);
|
||||
// Always log the refresh failure so callers can detect it (issue #56772).
|
||||
// Previously this was silently suppressed in --json mode, hiding the root
|
||||
// cause and preventing auto-update callers from detecting the failure.
|
||||
const message = `Failed to refresh gateway service environment from updated install: ${String(err)}`;
|
||||
if (params.opts.json) {
|
||||
defaultRuntime.error(message);
|
||||
} else {
|
||||
defaultRuntime.log(theme.warn(message));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -31,6 +31,7 @@ import {
|
|||
resolveConfigIncludes,
|
||||
} from "./includes.js";
|
||||
import { migrateLegacyConfig } from "./legacy-migrate.js";
|
||||
import { normalizeLegacyWebSearchConfig } from "./legacy-web-search.js";
|
||||
import { findLegacyConfigIssues } from "./legacy.js";
|
||||
import {
|
||||
asResolvedSourceConfig,
|
||||
|
|
@ -2127,9 +2128,13 @@ export function createConfigIO(overrides: ConfigIoDeps = {}) {
|
|||
//
|
||||
// We use only the root file's parsed content (no $include resolution) to avoid
|
||||
// pulling values from included files into the root config on write-back.
|
||||
// Apply env restoration to validated.config (which has runtime defaults stripped
|
||||
// per issue #6070) rather than the raw caller input.
|
||||
let cfgToWrite = validated.config;
|
||||
// Use persistCandidate (the merge-patched value before validation) rather than
|
||||
// validated.config, because plugin/channel AJV validation may inject schema
|
||||
// defaults (e.g., enrichGroupParticipantsFromContacts) that should not be
|
||||
// persisted to disk (issue #56772).
|
||||
// Apply legacy web-search normalization so that migration results are still
|
||||
// persisted even though we bypass validated.config.
|
||||
let cfgToWrite = normalizeLegacyWebSearchConfig(persistCandidate) as OpenClawConfig;
|
||||
try {
|
||||
if (deps.fs.existsSync(configPath)) {
|
||||
const currentRaw = await deps.fs.promises.readFile(configPath, "utf-8");
|
||||
|
|
|
|||
|
|
@ -5,6 +5,16 @@ import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
|||
import { createConfigIO } from "./io.js";
|
||||
import type { OpenClawConfig } from "./types.js";
|
||||
|
||||
// Mock the plugin manifest registry so we can register a fake channel whose
|
||||
// AJV JSON Schema carries a `default` value. This lets the #56772 regression
|
||||
// test exercise the exact code path that caused the bug: AJV injecting
|
||||
// defaults during the write-back validation pass.
|
||||
const mockLoadPluginManifestRegistry = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("../plugins/manifest-registry.js", () => ({
|
||||
loadPluginManifestRegistry: (...args: unknown[]) => mockLoadPluginManifestRegistry(...args),
|
||||
}));
|
||||
|
||||
describe("config io write", () => {
|
||||
let fixtureRoot = "";
|
||||
let homeCaseId = 0;
|
||||
|
|
@ -21,6 +31,13 @@ describe("config io write", () => {
|
|||
|
||||
beforeAll(async () => {
|
||||
fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-config-io-"));
|
||||
|
||||
// Default: return an empty plugin list so existing tests that don't need
|
||||
// plugin-owned channel schemas keep working unchanged.
|
||||
mockLoadPluginManifestRegistry.mockReturnValue({
|
||||
diagnostics: [],
|
||||
plugins: [],
|
||||
});
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
|
|
@ -353,6 +370,95 @@ describe("config io write", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("does not leak channel plugin AJV defaults into persisted config (issue #56772)", async () => {
|
||||
// Regression test for #56772. Mock the BlueBubbles channel metadata so
|
||||
// read-time AJV validation injects the same default that triggered the
|
||||
// write-back leak.
|
||||
mockLoadPluginManifestRegistry.mockReturnValue({
|
||||
diagnostics: [],
|
||||
plugins: [
|
||||
{
|
||||
id: "bluebubbles",
|
||||
origin: "bundled",
|
||||
channels: ["bluebubbles"],
|
||||
channelCatalogMeta: {
|
||||
id: "bluebubbles",
|
||||
label: "BlueBubbles",
|
||||
blurb: "BlueBubbles channel",
|
||||
},
|
||||
channelConfigs: {
|
||||
bluebubbles: {
|
||||
schema: {
|
||||
type: "object",
|
||||
properties: {
|
||||
enrichGroupParticipantsFromContacts: {
|
||||
type: "boolean",
|
||||
default: true,
|
||||
},
|
||||
serverUrl: {
|
||||
type: "string",
|
||||
},
|
||||
},
|
||||
additionalProperties: true,
|
||||
},
|
||||
uiHints: {},
|
||||
},
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await withSuiteHome(async (home) => {
|
||||
const { configPath, io, snapshot } = await writeConfigAndCreateIo({
|
||||
home,
|
||||
initialConfig: {
|
||||
gateway: { port: 18789 },
|
||||
channels: {
|
||||
bluebubbles: {
|
||||
serverUrl: "http://localhost:1234",
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// Simulate doctor: clone snapshot.config, make a small change, write back.
|
||||
const next = structuredClone(snapshot.config);
|
||||
const gateway =
|
||||
next.gateway && typeof next.gateway === "object"
|
||||
? (next.gateway as Record<string, unknown>)
|
||||
: {};
|
||||
next.gateway = {
|
||||
...gateway,
|
||||
auth: { mode: "token" },
|
||||
};
|
||||
await io.writeConfigFile(next);
|
||||
|
||||
const persisted = JSON.parse(await fs.readFile(configPath, "utf-8")) as Record<
|
||||
string,
|
||||
unknown
|
||||
>;
|
||||
|
||||
// The persisted config should contain only explicitly set values.
|
||||
expect(persisted.gateway).toEqual({
|
||||
port: 18789,
|
||||
auth: { mode: "token" },
|
||||
});
|
||||
|
||||
// The critical assertion: the AJV-injected BlueBubbles default must not
|
||||
// appear in the persisted config.
|
||||
const channels = persisted.channels as Record<string, Record<string, unknown>> | undefined;
|
||||
expect(channels?.bluebubbles).toBeDefined();
|
||||
expect(channels?.bluebubbles).not.toHaveProperty("enrichGroupParticipantsFromContacts");
|
||||
expect(channels?.bluebubbles?.serverUrl).toBe("http://localhost:1234");
|
||||
});
|
||||
|
||||
// Restore the default empty-plugins mock for subsequent tests.
|
||||
mockLoadPluginManifestRegistry.mockReturnValue({
|
||||
diagnostics: [],
|
||||
plugins: [],
|
||||
});
|
||||
});
|
||||
|
||||
it("does not reintroduce Slack/Discord legacy dm.policy defaults when writing", async () => {
|
||||
await withSuiteHome(async (home) => {
|
||||
const { configPath, io, snapshot } = await writeConfigAndCreateIo({
|
||||
|
|
|
|||
|
|
@ -6,42 +6,46 @@ vi.mock("../plugins/manifest-registry.js", () => ({
|
|||
loadPluginManifestRegistry: (...args: unknown[]) => mockLoadPluginManifestRegistry(...args),
|
||||
}));
|
||||
|
||||
describe("validateConfigObjectRawWithPlugins channel metadata", () => {
|
||||
it("applies bundled channel defaults from plugin-owned schema metadata", async () => {
|
||||
mockLoadPluginManifestRegistry.mockReturnValue({
|
||||
diagnostics: [],
|
||||
plugins: [
|
||||
{
|
||||
function setupTelegramSchemaWithDefault() {
|
||||
mockLoadPluginManifestRegistry.mockReturnValue({
|
||||
diagnostics: [],
|
||||
plugins: [
|
||||
{
|
||||
id: "telegram",
|
||||
origin: "bundled",
|
||||
channels: ["telegram"],
|
||||
channelCatalogMeta: {
|
||||
id: "telegram",
|
||||
origin: "bundled",
|
||||
channels: ["telegram"],
|
||||
channelCatalogMeta: {
|
||||
id: "telegram",
|
||||
label: "Telegram",
|
||||
blurb: "Telegram channel",
|
||||
},
|
||||
channelConfigs: {
|
||||
telegram: {
|
||||
schema: {
|
||||
type: "object",
|
||||
properties: {
|
||||
dmPolicy: {
|
||||
type: "string",
|
||||
enum: ["pairing", "allowlist"],
|
||||
default: "pairing",
|
||||
},
|
||||
label: "Telegram",
|
||||
blurb: "Telegram channel",
|
||||
},
|
||||
channelConfigs: {
|
||||
telegram: {
|
||||
schema: {
|
||||
type: "object",
|
||||
properties: {
|
||||
dmPolicy: {
|
||||
type: "string",
|
||||
enum: ["pairing", "allowlist"],
|
||||
default: "pairing",
|
||||
},
|
||||
additionalProperties: false,
|
||||
},
|
||||
uiHints: {},
|
||||
additionalProperties: false,
|
||||
},
|
||||
uiHints: {},
|
||||
},
|
||||
},
|
||||
],
|
||||
});
|
||||
},
|
||||
],
|
||||
});
|
||||
}
|
||||
|
||||
const { validateConfigObjectRawWithPlugins } = await import("./validation.js");
|
||||
const result = validateConfigObjectRawWithPlugins({
|
||||
describe("validateConfigObjectWithPlugins channel metadata (applyDefaults: true)", () => {
|
||||
it("applies bundled channel defaults from plugin-owned schema metadata", async () => {
|
||||
setupTelegramSchemaWithDefault();
|
||||
|
||||
const { validateConfigObjectWithPlugins } = await import("./validation.js");
|
||||
const result = validateConfigObjectWithPlugins({
|
||||
channels: {
|
||||
telegram: {},
|
||||
},
|
||||
|
|
@ -55,3 +59,32 @@ describe("validateConfigObjectRawWithPlugins channel metadata", () => {
|
|||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("validateConfigObjectRawWithPlugins channel metadata", () => {
|
||||
it("still injects channel AJV defaults even in raw mode — persistence safety is handled by io.ts", async () => {
|
||||
// Channel and plugin AJV validation always runs with applyDefaults: true
|
||||
// (hardcoded) to avoid breaking schemas that mark defaulted fields as
|
||||
// required (e.g., BlueBubbles enrichGroupParticipantsFromContacts).
|
||||
//
|
||||
// The actual protection against leaking these defaults to disk lives in
|
||||
// writeConfigFile (io.ts), which uses persistCandidate (the pre-validation
|
||||
// merge-patched value) instead of validated.config.
|
||||
setupTelegramSchemaWithDefault();
|
||||
|
||||
const { validateConfigObjectRawWithPlugins } = await import("./validation.js");
|
||||
const result = validateConfigObjectRawWithPlugins({
|
||||
channels: {
|
||||
telegram: {},
|
||||
},
|
||||
});
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
if (result.ok) {
|
||||
// AJV defaults ARE injected into validated.config even in raw mode.
|
||||
// This is intentional — see comment above.
|
||||
expect(result.config.channels?.telegram).toEqual(
|
||||
expect.objectContaining({ dmPolicy: "pairing" }),
|
||||
);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue