refactor(config): compile toolsBySender policy and migrate legacy keys

2026-02-22 21:21:15 +01:00
parent c73837d269
commit 3f64d4ad7b
4 changed files with 292 additions and 57 deletions
--- 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<string, { allow?: string[]; deny?: string[] }>;
+            };
+          };
+        };
+      };
+    };
+    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,
--- 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<string | number>;
+  pathLabel: string;
+  key: string;
+  targetKey: string;
+};
+
+function collectLegacyToolsBySenderKeyHits(
+  value: unknown,
+  pathParts: Array<string | number>,
+  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<string, { migrated: number; dropped: number; examples: string[] }>();
+  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<string[]> {
  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);
--- 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<string>();
+const compiledToolsBySenderCache = new WeakMap<
+  GroupToolPolicyBySenderConfig,
+  CompiledSenderPolicy
+>();

 type ParsedSenderPolicyKey =
  | { kind: "wildcard" }
  | { kind: "typed"; type: SenderKeyType; key: string };

-type SenderPolicyBuckets = Record<SenderKeyType, Map<string, GroupToolPolicyConfig>>;
+type SenderPolicyBuckets = Record<ToolsBySenderKeyType, Map<string, GroupToolPolicyConfig>>;

 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(
--- 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.
 *