From 3f64d4ad7b23c95de4cc8ae4de9861d958593436 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 21:21:15 +0100 Subject: [PATCH] refactor(config): compile toolsBySender policy and migrate legacy keys --- src/commands/doctor-config-flow.test.ts | 43 +++++++ src/commands/doctor-config-flow.ts | 138 +++++++++++++++++++++++ src/config/group-policy.ts | 144 ++++++++++++++---------- src/config/types.tools.ts | 24 ++++ 4 files changed, 292 insertions(+), 57 deletions(-) diff --git a/src/commands/doctor-config-flow.test.ts b/src/commands/doctor-config-flow.test.ts index 001bf5f70..2f6015503 100644 --- a/src/commands/doctor-config-flow.test.ts +++ b/src/commands/doctor-config-flow.test.ts @@ -378,6 +378,49 @@ describe("doctor config flow", () => { expect(cfg.channels.discord.accounts.work.allowFrom).toEqual(["*"]); }); + it("migrates legacy toolsBySender keys to typed id entries on repair", async () => { + const result = await runDoctorConfigWithInput({ + repair: true, + config: { + channels: { + whatsapp: { + groups: { + "123@g.us": { + toolsBySender: { + owner: { allow: ["exec"] }, + alice: { deny: ["exec"] }, + "id:owner": { deny: ["exec"] }, + "username:@ops-bot": { allow: ["fs.read"] }, + "*": { deny: ["exec"] }, + }, + }, + }, + }, + }, + }, + run: loadAndMaybeMigrateDoctorConfig, + }); + + const cfg = result.cfg as unknown as { + channels: { + whatsapp: { + groups: { + "123@g.us": { + toolsBySender: Record; + }; + }; + }; + }; + }; + const toolsBySender = cfg.channels.whatsapp.groups["123@g.us"].toolsBySender; + expect(toolsBySender.owner).toBeUndefined(); + expect(toolsBySender.alice).toBeUndefined(); + expect(toolsBySender["id:owner"]).toEqual({ deny: ["exec"] }); + expect(toolsBySender["id:alice"]).toEqual({ deny: ["exec"] }); + expect(toolsBySender["username:@ops-bot"]).toEqual({ allow: ["fs.read"] }); + expect(toolsBySender["*"]).toEqual({ deny: ["exec"] }); + }); + it("repairs googlechat dm.policy open by setting dm.allowFrom on repair", async () => { const result = await runDoctorConfigWithInput({ repair: true, diff --git a/src/commands/doctor-config-flow.ts b/src/commands/doctor-config-flow.ts index 6b4a5e13f..cabae3922 100644 --- a/src/commands/doctor-config-flow.ts +++ b/src/commands/doctor-config-flow.ts @@ -15,6 +15,7 @@ import { readConfigFileSnapshot, } from "../config/config.js"; import { applyPluginAutoEnable } from "../config/plugin-auto-enable.js"; +import { parseToolsBySenderTypedKey } from "../config/types.tools.js"; import { listInterpreterLikeSafeBins, resolveMergedSafeBinProfileFixtures, @@ -836,6 +837,120 @@ function maybeRepairExecSafeBinProfiles(cfg: OpenClawConfig): { return { config: next, changes, warnings }; } +type LegacyToolsBySenderKeyHit = { + toolsBySenderPath: Array; + pathLabel: string; + key: string; + targetKey: string; +}; + +function collectLegacyToolsBySenderKeyHits( + value: unknown, + pathParts: Array, + hits: LegacyToolsBySenderKeyHit[], +) { + if (Array.isArray(value)) { + for (const [index, entry] of value.entries()) { + collectLegacyToolsBySenderKeyHits(entry, [...pathParts, index], hits); + } + return; + } + const record = asObjectRecord(value); + if (!record) { + return; + } + + const toolsBySender = asObjectRecord(record.toolsBySender); + if (toolsBySender) { + const path = [...pathParts, "toolsBySender"]; + const pathLabel = formatPath(path); + for (const rawKey of Object.keys(toolsBySender)) { + const trimmed = rawKey.trim(); + if (!trimmed || trimmed === "*" || parseToolsBySenderTypedKey(trimmed)) { + continue; + } + hits.push({ + toolsBySenderPath: path, + pathLabel, + key: rawKey, + targetKey: `id:${trimmed}`, + }); + } + } + + for (const [key, nested] of Object.entries(record)) { + if (key === "toolsBySender") { + continue; + } + collectLegacyToolsBySenderKeyHits(nested, [...pathParts, key], hits); + } +} + +function scanLegacyToolsBySenderKeys(cfg: OpenClawConfig): LegacyToolsBySenderKeyHit[] { + const hits: LegacyToolsBySenderKeyHit[] = []; + collectLegacyToolsBySenderKeyHits(cfg, [], hits); + return hits; +} + +function maybeRepairLegacyToolsBySenderKeys(cfg: OpenClawConfig): { + config: OpenClawConfig; + changes: string[]; +} { + const next = structuredClone(cfg); + const hits = scanLegacyToolsBySenderKeys(next); + if (hits.length === 0) { + return { config: cfg, changes: [] }; + } + + const summary = new Map(); + let changed = false; + + for (const hit of hits) { + const toolsBySender = asObjectRecord(resolvePathTarget(next, hit.toolsBySenderPath)); + if (!toolsBySender || !(hit.key in toolsBySender)) { + continue; + } + const row = summary.get(hit.pathLabel) ?? { migrated: 0, dropped: 0, examples: [] }; + + if (toolsBySender[hit.targetKey] === undefined) { + toolsBySender[hit.targetKey] = toolsBySender[hit.key]; + row.migrated++; + if (row.examples.length < 3) { + row.examples.push(`${hit.key} -> ${hit.targetKey}`); + } + } else { + row.dropped++; + if (row.examples.length < 3) { + row.examples.push(`${hit.key} (kept existing ${hit.targetKey})`); + } + } + delete toolsBySender[hit.key]; + summary.set(hit.pathLabel, row); + changed = true; + } + + if (!changed) { + return { config: cfg, changes: [] }; + } + + const changes: string[] = []; + for (const [pathLabel, row] of summary) { + if (row.migrated > 0) { + const suffix = row.examples.length > 0 ? ` (${row.examples.join(", ")})` : ""; + changes.push( + `- ${pathLabel}: migrated ${row.migrated} legacy key${row.migrated === 1 ? "" : "s"} to typed id: entries${suffix}.`, + ); + } + if (row.dropped > 0) { + changes.push( + `- ${pathLabel}: removed ${row.dropped} legacy key${row.dropped === 1 ? "" : "s"} where typed id: entries already existed.`, + ); + } + } + + return { config: next, changes }; +} + async function maybeMigrateLegacyConfig(): Promise { const changes: string[] = []; const home = resolveHomeDir(); @@ -991,6 +1106,15 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { pendingChanges = true; cfg = allowFromRepair.config; } + + const toolsBySenderRepair = maybeRepairLegacyToolsBySenderKeys(candidate); + if (toolsBySenderRepair.changes.length > 0) { + note(toolsBySenderRepair.changes.join("\n"), "Doctor changes"); + candidate = toolsBySenderRepair.config; + pendingChanges = true; + cfg = toolsBySenderRepair.config; + } + const safeBinProfileRepair = maybeRepairExecSafeBinProfiles(candidate); if (safeBinProfileRepair.changes.length > 0) { note(safeBinProfileRepair.changes.join("\n"), "Doctor changes"); @@ -1035,6 +1159,20 @@ export async function loadAndMaybeMigrateDoctorConfig(params: { ); } + const toolsBySenderHits = scanLegacyToolsBySenderKeys(candidate); + if (toolsBySenderHits.length > 0) { + const sample = toolsBySenderHits[0]; + const sampleLabel = sample ? `${sample.pathLabel}.${sample.key}` : "toolsBySender"; + note( + [ + `- Found ${toolsBySenderHits.length} legacy untyped toolsBySender key${toolsBySenderHits.length === 1 ? "" : "s"} (for example ${sampleLabel}).`, + "- Untyped sender keys are deprecated; use explicit prefixes (id:, e164:, username:, name:).", + `- Run "${formatCliCommand("openclaw doctor --fix")}" to migrate legacy keys to typed id: entries.`, + ].join("\n"), + "Doctor warnings", + ); + } + const safeBinCoverage = scanExecSafeBinCoverage(candidate); if (safeBinCoverage.length > 0) { const interpreterHits = safeBinCoverage.filter((hit) => hit.isInterpreter); diff --git a/src/config/group-policy.ts b/src/config/group-policy.ts index d1f124570..2c5c4b7aa 100644 --- a/src/config/group-policy.ts +++ b/src/config/group-policy.ts @@ -1,7 +1,12 @@ import type { ChannelId } from "../channels/plugins/types.js"; import { normalizeAccountId } from "../routing/session-key.js"; import type { OpenClawConfig } from "./config.js"; -import type { GroupToolPolicyBySenderConfig, GroupToolPolicyConfig } from "./types.tools.js"; +import { + parseToolsBySenderTypedKey, + type GroupToolPolicyBySenderConfig, + type GroupToolPolicyConfig, + type ToolsBySenderKeyType, +} from "./types.tools.js"; export type GroupPolicyChannel = ChannelId; @@ -51,15 +56,22 @@ export type GroupToolPolicySender = { }; type SenderKeyType = "id" | "e164" | "username" | "name"; +type CompiledSenderPolicy = { + buckets: SenderPolicyBuckets; + wildcard?: GroupToolPolicyConfig; +}; -const SENDER_KEY_TYPES: SenderKeyType[] = ["id", "e164", "username", "name"]; const warnedLegacyToolsBySenderKeys = new Set(); +const compiledToolsBySenderCache = new WeakMap< + GroupToolPolicyBySenderConfig, + CompiledSenderPolicy +>(); type ParsedSenderPolicyKey = | { kind: "wildcard" } | { kind: "typed"; type: SenderKeyType; key: string }; -type SenderPolicyBuckets = Record>; +type SenderPolicyBuckets = Record>; function normalizeSenderKey( value: string, @@ -87,21 +99,6 @@ function normalizeLegacySenderKey(value: string): string { }); } -function parseTypedSenderKey(rawKey: string): { type: SenderKeyType; value: string } | undefined { - const lowered = rawKey.toLowerCase(); - for (const type of SENDER_KEY_TYPES) { - const prefix = `${type}:`; - if (!lowered.startsWith(prefix)) { - continue; - } - return { - type, - value: rawKey.slice(prefix.length), - }; - } - return undefined; -} - function warnLegacyToolsBySenderKey(rawKey: string) { const trimmed = rawKey.trim(); if (!trimmed || warnedLegacyToolsBySenderKeys.has(trimmed)) { @@ -125,7 +122,7 @@ function parseSenderPolicyKey(rawKey: string): ParsedSenderPolicyKey | undefined if (trimmed === "*") { return { kind: "wildcard" }; } - const typed = parseTypedSenderKey(trimmed); + const typed = parseToolsBySenderTypedKey(trimmed); if (typed) { const key = normalizeTypedSenderKey(typed.value, typed.type); if (!key) { @@ -160,39 +157,9 @@ function createSenderPolicyBuckets(): SenderPolicyBuckets { }; } -function normalizeCandidate(value: string | null | undefined, type: SenderKeyType): string { - const trimmed = value?.trim(); - if (!trimmed) { - return ""; - } - return normalizeTypedSenderKey(trimmed, type); -} - -function normalizeSenderIdCandidates(value: string | null | undefined): string[] { - const trimmed = value?.trim(); - if (!trimmed) { - return []; - } - const typed = normalizeTypedSenderKey(trimmed, "id"); - const legacy = normalizeLegacySenderKey(trimmed); - if (!typed) { - return legacy ? [legacy] : []; - } - if (!legacy || legacy === typed) { - return [typed]; - } - return [typed, legacy]; -} - -export function resolveToolsBySender( - params: { - toolsBySender?: GroupToolPolicyBySenderConfig; - } & GroupToolPolicySender, -): GroupToolPolicyConfig | undefined { - const toolsBySender = params.toolsBySender; - if (!toolsBySender) { - return undefined; - } +function compileToolsBySenderPolicy( + toolsBySender: GroupToolPolicyBySenderConfig, +): CompiledSenderPolicy | undefined { const entries = Object.entries(toolsBySender); if (entries.length === 0) { return undefined; @@ -218,34 +185,97 @@ export function resolveToolsBySender( } } + return { buckets, wildcard }; +} + +function resolveCompiledToolsBySenderPolicy( + toolsBySender: GroupToolPolicyBySenderConfig, +): CompiledSenderPolicy | undefined { + const cached = compiledToolsBySenderCache.get(toolsBySender); + if (cached) { + return cached; + } + const compiled = compileToolsBySenderPolicy(toolsBySender); + if (!compiled) { + return undefined; + } + // Config is loaded once and treated as immutable; cache compiled sender policy by object identity. + compiledToolsBySenderCache.set(toolsBySender, compiled); + return compiled; +} + +function normalizeCandidate(value: string | null | undefined, type: SenderKeyType): string { + const trimmed = value?.trim(); + if (!trimmed) { + return ""; + } + return normalizeTypedSenderKey(trimmed, type); +} + +function normalizeSenderIdCandidates(value: string | null | undefined): string[] { + const trimmed = value?.trim(); + if (!trimmed) { + return []; + } + const typed = normalizeTypedSenderKey(trimmed, "id"); + const legacy = normalizeLegacySenderKey(trimmed); + if (!typed) { + return legacy ? [legacy] : []; + } + if (!legacy || legacy === typed) { + return [typed]; + } + return [typed, legacy]; +} + +function matchToolsBySenderPolicy( + compiled: CompiledSenderPolicy, + params: GroupToolPolicySender, +): GroupToolPolicyConfig | undefined { for (const senderIdCandidate of normalizeSenderIdCandidates(params.senderId)) { - const match = buckets.id.get(senderIdCandidate); + const match = compiled.buckets.id.get(senderIdCandidate); if (match) { return match; } } const senderE164 = normalizeCandidate(params.senderE164, "e164"); if (senderE164) { - const match = buckets.e164.get(senderE164); + const match = compiled.buckets.e164.get(senderE164); if (match) { return match; } } const senderUsername = normalizeCandidate(params.senderUsername, "username"); if (senderUsername) { - const match = buckets.username.get(senderUsername); + const match = compiled.buckets.username.get(senderUsername); if (match) { return match; } } const senderName = normalizeCandidate(params.senderName, "name"); if (senderName) { - const match = buckets.name.get(senderName); + const match = compiled.buckets.name.get(senderName); if (match) { return match; } } - return wildcard; + return compiled.wildcard; +} + +export function resolveToolsBySender( + params: { + toolsBySender?: GroupToolPolicyBySenderConfig; + } & GroupToolPolicySender, +): GroupToolPolicyConfig | undefined { + const toolsBySender = params.toolsBySender; + if (!toolsBySender) { + return undefined; + } + const compiled = resolveCompiledToolsBySenderPolicy(toolsBySender); + if (!compiled) { + return undefined; + } + return matchToolsBySenderPolicy(compiled, params); } function resolveChannelGroups( diff --git a/src/config/types.tools.ts b/src/config/types.tools.ts index c6aeba129..53014d530 100644 --- a/src/config/types.tools.ts +++ b/src/config/types.tools.ts @@ -176,6 +176,30 @@ export type GroupToolPolicyConfig = { deny?: string[]; }; +export const TOOLS_BY_SENDER_KEY_TYPES = ["id", "e164", "username", "name"] as const; +export type ToolsBySenderKeyType = (typeof TOOLS_BY_SENDER_KEY_TYPES)[number]; + +export function parseToolsBySenderTypedKey( + rawKey: string, +): { type: ToolsBySenderKeyType; value: string } | undefined { + const trimmed = rawKey.trim(); + if (!trimmed) { + return undefined; + } + const lowered = trimmed.toLowerCase(); + for (const type of TOOLS_BY_SENDER_KEY_TYPES) { + const prefix = `${type}:`; + if (!lowered.startsWith(prefix)) { + continue; + } + return { + type, + value: trimmed.slice(prefix.length), + }; + } + return undefined; +} + /** * Per-sender overrides. *