From 73115b548052f151bf6146779db187f8a0ed8043 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Sat, 4 Apr 2026 14:50:15 +0900 Subject: [PATCH] fix(zalouser): migrate legacy group allow aliases (#60702) * fix(channels): prefer source contract surfaces in source checkouts * fix(zalouser): migrate legacy group allow aliases --- docs/.generated/config-baseline.channel.json | 20 --- docs/.generated/config-baseline.json | 20 --- extensions/zalouser/src/config-schema.ts | 1 - extensions/zalouser/src/doctor.test.ts | 48 ++++++ extensions/zalouser/src/doctor.ts | 145 +++++++++++++++++- extensions/zalouser/src/group-policy.test.ts | 8 +- extensions/zalouser/src/group-policy.ts | 3 +- .../zalouser/src/monitor.group-gating.test.ts | 6 +- extensions/zalouser/src/monitor.ts | 2 +- extensions/zalouser/src/setup-surface.test.ts | 17 ++ extensions/zalouser/src/setup-surface.ts | 2 +- extensions/zalouser/src/types.ts | 1 - .../plugins/contract-surfaces.test.ts | 16 ++ src/channels/plugins/contract-surfaces.ts | 18 ++- ...ndled-channel-config-metadata.generated.ts | 6 - src/config/legacy-migrate.test.ts | 30 +++- 16 files changed, 281 insertions(+), 62 deletions(-) create mode 100644 extensions/zalouser/src/doctor.test.ts create mode 100644 src/channels/plugins/contract-surfaces.test.ts diff --git a/docs/.generated/config-baseline.channel.json b/docs/.generated/config-baseline.channel.json index ffbb21c8e8e..5f69d0cdbff 100644 --- a/docs/.generated/config-baseline.channel.json +++ b/docs/.generated/config-baseline.channel.json @@ -31145,16 +31145,6 @@ "tags": [], "hasChildren": true }, - { - "path": "channels.zalouser.accounts.*.groups.*.allow", - "kind": "channel", - "type": "boolean", - "required": false, - "deprecated": false, - "sensitive": false, - "tags": [], - "hasChildren": false - }, { "path": "channels.zalouser.accounts.*.groups.*.enabled", "kind": "channel", @@ -31449,16 +31439,6 @@ "tags": [], "hasChildren": true }, - { - "path": "channels.zalouser.groups.*.allow", - "kind": "channel", - "type": "boolean", - "required": false, - "deprecated": false, - "sensitive": false, - "tags": [], - "hasChildren": false - }, { "path": "channels.zalouser.groups.*.enabled", "kind": "channel", diff --git a/docs/.generated/config-baseline.json b/docs/.generated/config-baseline.json index 63c2a496be1..68056f2895d 100644 --- a/docs/.generated/config-baseline.json +++ b/docs/.generated/config-baseline.json @@ -59185,16 +59185,6 @@ "tags": [], "hasChildren": true }, - { - "path": "channels.zalouser.accounts.*.groups.*.allow", - "kind": "channel", - "type": "boolean", - "required": false, - "deprecated": false, - "sensitive": false, - "tags": [], - "hasChildren": false - }, { "path": "channels.zalouser.accounts.*.groups.*.enabled", "kind": "channel", @@ -59489,16 +59479,6 @@ "tags": [], "hasChildren": true }, - { - "path": "channels.zalouser.groups.*.allow", - "kind": "channel", - "type": "boolean", - "required": false, - "deprecated": false, - "sensitive": false, - "tags": [], - "hasChildren": false - }, { "path": "channels.zalouser.groups.*.enabled", "kind": "channel", diff --git a/extensions/zalouser/src/config-schema.ts b/extensions/zalouser/src/config-schema.ts index 3c7f5efc32c..da65741bbdf 100644 --- a/extensions/zalouser/src/config-schema.ts +++ b/extensions/zalouser/src/config-schema.ts @@ -9,7 +9,6 @@ import { import { z } from "openclaw/plugin-sdk/zod"; const groupConfigSchema = z.object({ - allow: z.boolean().optional(), enabled: z.boolean().optional(), requireMention: z.boolean().optional(), tools: ToolPolicySchema, diff --git a/extensions/zalouser/src/doctor.test.ts b/extensions/zalouser/src/doctor.test.ts new file mode 100644 index 00000000000..b89e0ada1ee --- /dev/null +++ b/extensions/zalouser/src/doctor.test.ts @@ -0,0 +1,48 @@ +import { describe, expect, it } from "vitest"; +import { zalouserDoctor } from "./doctor.js"; + +describe("zalouser doctor", () => { + it("normalizes legacy group allow aliases to enabled", () => { + const normalize = zalouserDoctor.normalizeCompatibilityConfig; + expect(normalize).toBeDefined(); + if (!normalize) { + return; + } + + const result = normalize({ + cfg: { + channels: { + zalouser: { + groups: { + "group:trusted": { + allow: true, + }, + }, + accounts: { + work: { + groups: { + "group:legacy": { + allow: false, + }, + }, + }, + }, + }, + }, + } as never, + }); + + expect(result.config.channels?.zalouser?.groups?.["group:trusted"]).toEqual({ + enabled: true, + }); + expect(result.config.channels?.zalouser?.accounts?.work?.groups?.["group:legacy"]).toEqual({ + enabled: false, + }); + expect(result.changes).toEqual( + expect.arrayContaining([ + "Moved channels.zalouser.groups.group:trusted.allow → channels.zalouser.groups.group:trusted.enabled (true).", + "Moved channels.zalouser.accounts.work.groups.group:legacy.allow → channels.zalouser.accounts.work.groups.group:legacy.enabled (false).", + ]), + ); + }); +}); diff --git a/extensions/zalouser/src/doctor.ts b/extensions/zalouser/src/doctor.ts index 1af708dab5a..f8c5c85df99 100644 --- a/extensions/zalouser/src/doctor.ts +++ b/extensions/zalouser/src/doctor.ts @@ -1,4 +1,8 @@ -import type { ChannelDoctorAdapter } from "openclaw/plugin-sdk/channel-contract"; +import type { + ChannelDoctorAdapter, + ChannelDoctorConfigMutation, + ChannelDoctorLegacyConfigRule, +} from "openclaw/plugin-sdk/channel-contract"; import type { OpenClawConfig } from "openclaw/plugin-sdk/config-runtime"; import { collectProviderDangerousNameMatchingScopes } from "openclaw/plugin-sdk/runtime"; import { isZalouserMutableGroupEntry } from "./security-audit.js"; @@ -13,6 +17,143 @@ function sanitizeForLog(value: string): string { return value.replace(/[\u0000-\u001f\u007f]+/g, " ").trim(); } +function hasLegacyZalouserGroupAllowAlias(value: unknown): boolean { + const group = asObjectRecord(value); + return Boolean(group && typeof group.allow === "boolean"); +} + +function hasLegacyZalouserGroupAllowAliases(value: unknown): boolean { + const groups = asObjectRecord(value); + return Boolean( + groups && Object.values(groups).some((group) => hasLegacyZalouserGroupAllowAlias(group)), + ); +} + +function hasLegacyZalouserAccountGroupAllowAliases(value: unknown): boolean { + const accounts = asObjectRecord(value); + if (!accounts) { + return false; + } + return Object.values(accounts).some((account) => { + const accountRecord = asObjectRecord(account); + return Boolean(accountRecord && hasLegacyZalouserGroupAllowAliases(accountRecord.groups)); + }); +} + +function normalizeZalouserGroupAllowAliases(params: { + groups: Record; + pathPrefix: string; + changes: string[]; +}): { groups: Record; changed: boolean } { + let changed = false; + const nextGroups: Record = { ...params.groups }; + for (const [groupId, groupValue] of Object.entries(params.groups)) { + const group = asObjectRecord(groupValue); + if (!group || typeof group.allow !== "boolean") { + continue; + } + const nextGroup = { ...group }; + if (typeof nextGroup.enabled !== "boolean") { + nextGroup.enabled = group.allow; + } + delete nextGroup.allow; + nextGroups[groupId] = nextGroup; + changed = true; + params.changes.push( + `Moved ${params.pathPrefix}.${groupId}.allow → ${params.pathPrefix}.${groupId}.enabled (${String(nextGroup.enabled)}).`, + ); + } + return { groups: nextGroups, changed }; +} + +function normalizeZalouserCompatibilityConfig(cfg: OpenClawConfig): ChannelDoctorConfigMutation { + const channels = asObjectRecord(cfg.channels); + const zalouser = asObjectRecord(channels?.zalouser); + if (!zalouser) { + return { config: cfg, changes: [] }; + } + + const changes: string[] = []; + let updatedZalouser: Record = zalouser; + let changed = false; + + const groups = asObjectRecord(updatedZalouser.groups); + if (groups) { + const normalized = normalizeZalouserGroupAllowAliases({ + groups, + pathPrefix: "channels.zalouser.groups", + changes, + }); + if (normalized.changed) { + updatedZalouser = { ...updatedZalouser, groups: normalized.groups }; + changed = true; + } + } + + const accounts = asObjectRecord(updatedZalouser.accounts); + if (accounts) { + let accountsChanged = false; + const nextAccounts: Record = { ...accounts }; + for (const [accountId, accountValue] of Object.entries(accounts)) { + const account = asObjectRecord(accountValue); + if (!account) { + continue; + } + const accountGroups = asObjectRecord(account.groups); + if (!accountGroups) { + continue; + } + const normalized = normalizeZalouserGroupAllowAliases({ + groups: accountGroups, + pathPrefix: `channels.zalouser.accounts.${accountId}.groups`, + changes, + }); + if (!normalized.changed) { + continue; + } + nextAccounts[accountId] = { + ...account, + groups: normalized.groups, + }; + accountsChanged = true; + } + if (accountsChanged) { + updatedZalouser = { ...updatedZalouser, accounts: nextAccounts }; + changed = true; + } + } + + if (!changed) { + return { config: cfg, changes: [] }; + } + + return { + config: { + ...cfg, + channels: { + ...cfg.channels, + zalouser: updatedZalouser as OpenClawConfig["channels"]["zalouser"], + }, + }, + changes, + }; +} + +const ZALOUSER_LEGACY_CONFIG_RULES: ChannelDoctorLegacyConfigRule[] = [ + { + path: ["channels", "zalouser", "groups"], + message: + "channels.zalouser.groups..allow is legacy; use channels.zalouser.groups..enabled instead (auto-migrated on load).", + match: hasLegacyZalouserGroupAllowAliases, + }, + { + path: ["channels", "zalouser", "accounts"], + message: + "channels.zalouser.accounts..groups..allow is legacy; use channels.zalouser.accounts..groups..enabled instead (auto-migrated on load).", + match: hasLegacyZalouserAccountGroupAllowAliases, + }, +]; + export function collectZalouserMutableAllowlistWarnings(cfg: OpenClawConfig): string[] { const hits: Array<{ path: string; entry: string }> = []; @@ -53,5 +194,7 @@ export const zalouserDoctor: ChannelDoctorAdapter = { groupModel: "hybrid", groupAllowFromFallbackToAllowFrom: false, warnOnEmptyGroupSenderAllowlist: false, + legacyConfigRules: ZALOUSER_LEGACY_CONFIG_RULES, + normalizeCompatibilityConfig: ({ cfg }) => normalizeZalouserCompatibilityConfig(cfg), collectMutableAllowlistWarnings: ({ cfg }) => collectZalouserMutableAllowlistWarnings(cfg), }; diff --git a/extensions/zalouser/src/group-policy.test.ts b/extensions/zalouser/src/group-policy.test.ts index adbeffbe86f..d9b539dc861 100644 --- a/extensions/zalouser/src/group-policy.test.ts +++ b/extensions/zalouser/src/group-policy.test.ts @@ -37,7 +37,7 @@ describe("zalouser group policy helpers", () => { it("finds the first matching group entry", () => { const groups = { - "group:123": { allow: true }, + "group:123": { enabled: true }, "team-alpha": { requireMention: false }, "*": { requireMention: true }, }; @@ -49,12 +49,12 @@ describe("zalouser group policy helpers", () => { includeGroupIdAlias: true, }), ); - expect(entry).toEqual({ allow: true }); + expect(entry).toEqual({ enabled: true }); }); it("evaluates allow/enable flags", () => { - expect(isZalouserGroupEntryAllowed({ allow: true, enabled: true })).toBe(true); - expect(isZalouserGroupEntryAllowed({ allow: false })).toBe(false); + expect(isZalouserGroupEntryAllowed({ enabled: true })).toBe(true); + expect(isZalouserGroupEntryAllowed({ allow: false } as never)).toBe(false); expect(isZalouserGroupEntryAllowed({ enabled: false })).toBe(false); expect(isZalouserGroupEntryAllowed(undefined)).toBe(false); }); diff --git a/extensions/zalouser/src/group-policy.ts b/extensions/zalouser/src/group-policy.ts index 4d116f15bf2..c712319faa0 100644 --- a/extensions/zalouser/src/group-policy.ts +++ b/extensions/zalouser/src/group-policy.ts @@ -77,5 +77,6 @@ export function isZalouserGroupEntryAllowed(entry: ZalouserGroupConfig | undefin if (!entry) { return false; } - return entry.allow !== false && entry.enabled !== false; + const legacyAllow = (entry as ZalouserGroupConfig & { allow?: unknown }).allow; + return legacyAllow !== false && entry.enabled !== false; } diff --git a/extensions/zalouser/src/monitor.group-gating.test.ts b/extensions/zalouser/src/monitor.group-gating.test.ts index 1984efae626..f984e120cbf 100644 --- a/extensions/zalouser/src/monitor.group-gating.test.ts +++ b/extensions/zalouser/src/monitor.group-gating.test.ts @@ -348,8 +348,8 @@ describe("zalouser monitor group mention gating", () => { groupPolicy: "allowlist", groupAllowFrom: ["*"], groups: { - "group:g-trusted-001": { allow: true }, - "Trusted Team": { allow: true }, + "group:g-trusted-001": { enabled: true }, + "Trusted Team": { enabled: true }, }, }, }, @@ -525,7 +525,7 @@ describe("zalouser monitor group mention gating", () => { groupPolicy: "allowlist", allowFrom: ["123"], groups: { - "group:g-1": { allow: true, requireMention: true }, + "group:g-1": { enabled: true, requireMention: true }, }, }, }, diff --git a/extensions/zalouser/src/monitor.ts b/extensions/zalouser/src/monitor.ts index f364bcb7b59..994c6852d32 100644 --- a/extensions/zalouser/src/monitor.ts +++ b/extensions/zalouser/src/monitor.ts @@ -204,7 +204,7 @@ function isSenderAllowed(senderId: string | undefined, allowFrom: string[]): boo function resolveGroupRequireMention(params: { groupId: string; groupName?: string | null; - groups: Record; + groups: Record; allowNameMatching?: boolean; }): boolean { const entry = findZalouserGroupEntry( diff --git a/extensions/zalouser/src/setup-surface.test.ts b/extensions/zalouser/src/setup-surface.test.ts index 240303b5f2d..5975441f4b7 100644 --- a/extensions/zalouser/src/setup-surface.test.ts +++ b/extensions/zalouser/src/setup-surface.test.ts @@ -160,6 +160,23 @@ describe("zalouser setup wizard", () => { ).toBe(true); }); + it("writes canonical enabled entries for configured groups", async () => { + const prompter = createQuickstartPrompter({ + groupAccess: true, + groupPolicy: "allowlist", + textByMessage: { + "Zalo groups allowlist (comma-separated)": "Family, Work", + }, + }); + + const result = await runSetup({ prompter }); + + expect(result.cfg.channels?.zalouser?.groups).toEqual({ + Family: { enabled: true, requireMention: true }, + Work: { enabled: true, requireMention: true }, + }); + }); + it("preserves non-quickstart forceAllowFrom behavior", async () => { const note = vi.fn(async (_message: string, _title?: string) => {}); const seen: string[] = []; diff --git a/extensions/zalouser/src/setup-surface.ts b/extensions/zalouser/src/setup-surface.ts index 6ccbb4ae44b..0746481552d 100644 --- a/extensions/zalouser/src/setup-surface.ts +++ b/extensions/zalouser/src/setup-surface.ts @@ -94,7 +94,7 @@ function setZalouserGroupAllowlist( groupKeys: string[], ): OpenClawConfig { const groups = Object.fromEntries( - groupKeys.map((key) => [key, { allow: true, requireMention: true }]), + groupKeys.map((key) => [key, { enabled: true, requireMention: true }]), ); return setZalouserAccountScopedConfig(cfg, accountId, { groups, diff --git a/extensions/zalouser/src/types.ts b/extensions/zalouser/src/types.ts index f29f503f7d9..5c4785c47fc 100644 --- a/extensions/zalouser/src/types.ts +++ b/extensions/zalouser/src/types.ts @@ -88,7 +88,6 @@ export type ZaloAuthStatus = { export type ZalouserToolConfig = { allow?: string[]; deny?: string[] }; export type ZalouserGroupConfig = { - allow?: boolean; enabled?: boolean; requireMention?: boolean; tools?: ZalouserToolConfig; diff --git a/src/channels/plugins/contract-surfaces.test.ts b/src/channels/plugins/contract-surfaces.test.ts new file mode 100644 index 00000000000..bbe93d26590 --- /dev/null +++ b/src/channels/plugins/contract-surfaces.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from "vitest"; +import { getBundledChannelContractSurfaceModule } from "./contract-surfaces.js"; + +describe("bundled channel contract surfaces", () => { + it("resolves Telegram contract surfaces from a source checkout", () => { + const surface = getBundledChannelContractSurfaceModule<{ + normalizeTelegramCommandName?: (value: string) => string; + }>({ + pluginId: "telegram", + preferredBasename: "contract-surfaces.ts", + }); + + expect(surface).not.toBeNull(); + expect(surface?.normalizeTelegramCommandName?.("/Hello-World")).toBe("hello_world"); + }); +}); diff --git a/src/channels/plugins/contract-surfaces.ts b/src/channels/plugins/contract-surfaces.ts index 9e899af837a..10ecf1d326d 100644 --- a/src/channels/plugins/contract-surfaces.ts +++ b/src/channels/plugins/contract-surfaces.ts @@ -61,6 +61,16 @@ function createModuleLoader() { const loadModule = createModuleLoader(); +function getContractSurfaceDiscoveryEnv(): NodeJS.ProcessEnv { + if (RUNNING_FROM_BUILT_ARTIFACT) { + return process.env; + } + return { + ...process.env, + VITEST: process.env.VITEST || "1", + }; +} + function matchesPreferredBasename( basename: ContractSurfaceBasename, preferredBasename: ContractSurfaceBasename | undefined, @@ -153,9 +163,11 @@ function loadBundledChannelContractSurfaceEntries(): Array<{ pluginId: string; surface: unknown; }> { - const discovery = discoverOpenClawPlugins({ cache: false }); + const env = getContractSurfaceDiscoveryEnv(); + const discovery = discoverOpenClawPlugins({ cache: false, env }); const manifestRegistry = loadPluginManifestRegistry({ cache: false, + env, config: {}, candidates: discovery.candidates, diagnostics: discovery.diagnostics, @@ -204,9 +216,11 @@ export function getBundledChannelContractSurfaceModule(params: { if (cachedPreferredSurfaceModules.has(cacheKey)) { return (cachedPreferredSurfaceModules.get(cacheKey) ?? null) as T | null; } - const discovery = discoverOpenClawPlugins({ cache: false }); + const env = getContractSurfaceDiscoveryEnv(); + const discovery = discoverOpenClawPlugins({ cache: false, env }); const manifestRegistry = loadPluginManifestRegistry({ cache: false, + env, config: {}, candidates: discovery.candidates, diagnostics: discovery.diagnostics, diff --git a/src/config/bundled-channel-config-metadata.generated.ts b/src/config/bundled-channel-config-metadata.generated.ts index e88192d8bb7..44592809d9b 100644 --- a/src/config/bundled-channel-config-metadata.generated.ts +++ b/src/config/bundled-channel-config-metadata.generated.ts @@ -15318,9 +15318,6 @@ export const GENERATED_BUNDLED_CHANNEL_CONFIG_METADATA = [ additionalProperties: { type: "object", properties: { - allow: { - type: "boolean", - }, enabled: { type: "boolean", }, @@ -15435,9 +15432,6 @@ export const GENERATED_BUNDLED_CHANNEL_CONFIG_METADATA = [ additionalProperties: { type: "object", properties: { - allow: { - type: "boolean", - }, enabled: { type: "boolean", }, diff --git a/src/config/legacy-migrate.test.ts b/src/config/legacy-migrate.test.ts index 01bebd1fbb2..3ce2a8d4cc9 100644 --- a/src/config/legacy-migrate.test.ts +++ b/src/config/legacy-migrate.test.ts @@ -558,7 +558,7 @@ describe("legacy migrate nested channel enabled aliases", () => { }); }); - it("moves legacy allow toggles into enabled for slack, googlechat, discord, and matrix", () => { + it("moves legacy allow toggles into enabled for slack, googlechat, discord, matrix, and zalouser", () => { const res = migrateLegacyConfig({ channels: { slack: { @@ -633,6 +633,22 @@ describe("legacy migrate nested channel enabled aliases", () => { }, }, }, + zalouser: { + groups: { + "group:trusted": { + allow: false, + }, + }, + accounts: { + work: { + groups: { + "group:legacy": { + allow: true, + }, + }, + }, + }, + }, }, }); @@ -660,6 +676,12 @@ describe("legacy migrate nested channel enabled aliases", () => { expect(res.changes).toContain( "Moved channels.matrix.accounts.work.rooms.!legacy:example.org.allow → channels.matrix.accounts.work.rooms.!legacy:example.org.enabled (true).", ); + expect(res.changes).toContain( + "Moved channels.zalouser.groups.group:trusted.allow → channels.zalouser.groups.group:trusted.enabled (false).", + ); + expect(res.changes).toContain( + "Moved channels.zalouser.accounts.work.groups.group:legacy.allow → channels.zalouser.accounts.work.groups.group:legacy.enabled (true).", + ); expect(res.config?.channels?.slack?.channels?.ops).toEqual({ enabled: false, }); @@ -675,6 +697,12 @@ describe("legacy migrate nested channel enabled aliases", () => { expect(res.config?.channels?.matrix?.accounts?.work?.rooms?.["!legacy:example.org"]).toEqual({ enabled: true, }); + expect(res.config?.channels?.zalouser?.groups?.["group:trusted"]).toEqual({ + enabled: false, + }); + expect(res.config?.channels?.zalouser?.accounts?.work?.groups?.["group:legacy"]).toEqual({ + enabled: true, + }); }); it("drops legacy allow when enabled is already set", () => {