From dce4a38bbce02ed4fe34afffc6d7b51b2dbace9d Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 16 Jun 2026 17:43:32 +0800 Subject: [PATCH] fix(config): couple channel diagnostics to schema owner --- src/config/channel-config-metadata.ts | 27 +++++- .../validation.channel-metadata.test.ts | 92 +++++++++++++++++++ src/config/validation.ts | 34 ++----- 3 files changed, 122 insertions(+), 31 deletions(-) diff --git a/src/config/channel-config-metadata.ts b/src/config/channel-config-metadata.ts index 12ae9f0af04d..d9dbde0a2d73 100644 --- a/src/config/channel-config-metadata.ts +++ b/src/config/channel-config-metadata.ts @@ -6,7 +6,12 @@ import type { PluginManifestRegistry } from "../plugins/manifest-registry.js"; import type { PluginOrigin } from "../plugins/plugin-origin.types.js"; import type { ChannelUiMetadata, PluginUiMetadata } from "./schema.js"; -type ChannelMetadataRecord = ChannelUiMetadata & { +export type ChannelSchemaMetadataWithOwnership = ChannelUiMetadata & { + schemaPluginId?: string; + schemaPluginOrigin?: PluginOrigin; +}; + +type ChannelMetadataRecord = ChannelSchemaMetadataWithOwnership & { originRank: number; }; @@ -49,10 +54,10 @@ export function collectPluginSchemaMetadata(registry: PluginManifestRegistry): P .map(({ originRank: _originRank, ...record }) => record); } -/** Collects per-channel config UI metadata from plugin manifests and channel config blocks. */ -export function collectChannelSchemaMetadata( +/** Collects per-channel config metadata with the plugin that supplied the selected schema. */ +export function collectChannelSchemaMetadataWithOwnership( registry: PluginManifestRegistry, -): ChannelUiMetadata[] { +): ChannelSchemaMetadataWithOwnership[] { const byChannelId = new Map(); for (const record of registry.plugins) { @@ -71,6 +76,8 @@ export function collectChannelSchemaMetadata( description: rootDescription ?? current?.description, configSchema: current?.configSchema, configUiHints: current?.configUiHints, + schemaPluginId: current?.schemaPluginId, + schemaPluginOrigin: current?.schemaPluginOrigin, originRank, }); } @@ -93,6 +100,8 @@ export function collectChannelSchemaMetadata( description: channelConfig.description ?? rootDescription ?? current?.description, configSchema: channelConfig.schema, configUiHints: channelConfig.uiHints as ChannelUiMetadata["configUiHints"], + schemaPluginId: channelConfig.schema === undefined ? undefined : record.id, + schemaPluginOrigin: channelConfig.schema === undefined ? undefined : record.origin, originRank, }); } @@ -102,3 +111,13 @@ export function collectChannelSchemaMetadata( .toSorted((left, right) => left.id.localeCompare(right.id)) .map(({ originRank: _originRank, ...entry }) => entry); } + +/** Collects public per-channel config UI metadata without internal schema ownership. */ +export function collectChannelSchemaMetadata( + registry: PluginManifestRegistry, +): ChannelUiMetadata[] { + return collectChannelSchemaMetadataWithOwnership(registry).map( + ({ schemaPluginId: _schemaPluginId, schemaPluginOrigin: _schemaPluginOrigin, ...entry }) => + entry, + ); +} diff --git a/src/config/validation.channel-metadata.test.ts b/src/config/validation.channel-metadata.test.ts index c8e0b6b39acf..9b87b78026d5 100644 --- a/src/config/validation.channel-metadata.test.ts +++ b/src/config/validation.channel-metadata.test.ts @@ -124,6 +124,37 @@ function createExternalFeishuSchemaWithCloserMetadataRegistry(): PluginManifestR }; } +function createExternalFeishuSchemaWithRootOnlyShadowRegistry(): PluginManifestRegistry { + const firstSchema = createExternalFeishuSchemaRegistry().plugins[0]; + return { + diagnostics: [], + plugins: [ + firstSchema, + createPluginManifestRecord({ + id: "workspace-channel-labels", + origin: "workspace", + channels: ["feishu"], + }), + createPluginManifestRecord({ + id: "other-global-feishu", + origin: "global", + channels: ["feishu"], + channelConfigs: { + feishu: { + schema: { + type: "object", + properties: { + otherField: { type: "string" }, + }, + additionalProperties: false, + }, + }, + }, + }), + ], + }; +} + function createCompatPluginConfigSchemaRegistry(): PluginManifestRegistry { return { diagnostics: [], @@ -352,6 +383,67 @@ describe("validateConfigObjectRawWithPlugins channel metadata", () => { } }); + it("keeps schema ownership coupled when closer root metadata preserves a schema", () => { + mockLoadPluginManifestRegistry.mockReturnValue( + createExternalFeishuSchemaWithRootOnlyShadowRegistry(), + ); + + const result = validateConfigObjectRawWithPlugins({ + channels: { + feishu: { + appId: "app-id", + appSecret: "secret", + unsupportedField: true, + }, + }, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.issues).toContainEqual( + expect.objectContaining({ + path: "channels.feishu", + message: + 'invalid config for plugin openclaw-lark: must not have additional properties: "unsupportedField"', + }), + ); + expect(result.issues.map((issue) => issue.message)).not.toContain( + 'invalid config for plugin other-global-feishu: must not have additional properties: "unsupportedField"', + ); + } + }); + + it("sanitizes the schema owner in validation diagnostics", () => { + const unsafeId = `openclaw${String.fromCharCode(10)}${String.fromCharCode(27)}[31m-lark`; + const registry = createExternalFeishuSchemaRegistry(); + registry.plugins[0] = { + ...registry.plugins[0], + id: unsafeId, + }; + mockLoadPluginManifestRegistry.mockReturnValue(registry); + + const result = validateConfigObjectRawWithPlugins({ + channels: { + feishu: { + appId: "app-id", + appSecret: "secret", + unsupportedField: true, + }, + }, + }); + + expect(result.ok).toBe(false); + if (!result.ok) { + expect(result.issues).toContainEqual( + expect.objectContaining({ + path: "channels.feishu", + message: + 'invalid config for plugin openclaw-lark: must not have additional properties: "unsupportedField"', + }), + ); + } + }); + it("keeps raw channel validation diagnostics plugin-agnostic", () => { const result = validateConfigObjectRawWithPlugins({ channels: { diff --git a/src/config/validation.ts b/src/config/validation.ts index 89a8c9473392..2fdcc6f7df39 100644 --- a/src/config/validation.ts +++ b/src/config/validation.ts @@ -3,6 +3,7 @@ import path from "node:path"; import { collectConfiguredModelRefs } from "@openclaw/model-catalog-core/configured-model-refs"; import { isCanonicalDottedDecimalIPv4, isLoopbackIpAddress } from "@openclaw/net-policy/ip"; 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 { isPathInside } from "../infra/path-guards.js"; import { planManifestModelCatalogSuppressions } from "../model-catalog/index.js"; @@ -42,7 +43,7 @@ import { isRecord, resolveUserPath } from "../utils.js"; import { findDuplicateAgentDirs, formatDuplicateAgentDirError } from "./agent-dirs.js"; import { appendAllowedValuesHint, summarizeAllowedValues } from "./allowed-values.js"; import { GENERATED_BUNDLED_CHANNEL_CONFIG_METADATA } from "./bundled-channel-config-metadata.generated.js"; -import { collectChannelSchemaMetadata } from "./channel-config-metadata.js"; +import { collectChannelSchemaMetadataWithOwnership } from "./channel-config-metadata.js"; import { shouldSuppressMissingCodexPluginDiagnostics } from "./codex-plugin-diagnostics.js"; import { materializeRuntimeConfig } from "./materialize.js"; import type { OpenClawConfig, ConfigValidationIssue } from "./types.js"; @@ -1073,8 +1074,9 @@ function validateConfigObjectWithPluginsBase( }; const formatChannelConfigIssueMessage = (message: string, pluginId?: string): string => { - if (pluginId) { - return `invalid config for plugin ${pluginId}: ${message}`; + const safePluginId = pluginId ? sanitizeForLog(pluginId).trim() : ""; + if (safePluginId) { + return `invalid config for plugin ${safePluginId}: ${message}`; } return formatRawChannelConfigIssueMessage(message); }; @@ -1229,34 +1231,12 @@ function validateConfigObjectWithPluginsBase( (entry) => [entry.channelId, { schema: entry.schema }] as const, ), ); - const ownerByChannelId = new Map< - string, - { pluginId: string; origin: string; originRank: number } - >(); - const originRank: Record = { config: 0, workspace: 1, global: 2, bundled: 3 }; - for (const record of info.registry.plugins) { - const rank = originRank[record.origin] ?? Number.MAX_SAFE_INTEGER; - for (const [channelId, channelConfig] of Object.entries(record.channelConfigs ?? {})) { - if (channelConfig.schema === undefined) { - continue; - } - const current = ownerByChannelId.get(channelId); - if (!current || rank <= current.originRank) { - ownerByChannelId.set(channelId, { - pluginId: record.id, - origin: record.origin, - originRank: rank, - }); - } - } - } - for (const entry of collectChannelSchemaMetadata(info.registry)) { + for (const entry of collectChannelSchemaMetadataWithOwnership(info.registry)) { const current = info.channelSchemas.get(entry.id); if (entry.configSchema) { - const owner = ownerByChannelId.get(entry.id); info.channelSchemas.set(entry.id, { schema: entry.configSchema, - pluginId: owner?.origin === "bundled" ? undefined : owner?.pluginId, + pluginId: entry.schemaPluginOrigin === "bundled" ? undefined : entry.schemaPluginId, }); continue; }