From d9f553bccf65eb6f27d465a64fc95eb741d589cc Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Wed, 8 Apr 2026 15:39:17 -0400 Subject: [PATCH] fix(matrix/doctor): preserve allowlist semantics for trusted dm policy --- CHANGELOG.md | 1 + extensions/matrix/src/doctor-contract.ts | 34 ++++++++---------------- extensions/matrix/src/doctor.test.ts | 15 ++++------- 3 files changed, 17 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76afe4975b5..224721a6fe5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ Docs: https://docs.openclaw.ai - Browser/security: re-run blocked-destination safety checks after interaction-driven main-frame navigations from click, evaluate, hook-triggered click, and batched action flows, so browser interactions cannot bypass the SSRF quarantine when they land on forbidden URLs. (#63226) Thanks @eleqtrizit. - Providers/Ollama: allow Ollama models using the native `api: "ollama"` path to optionally display thinking output when `/think` is set to a non-off level. (#62712) Thanks @hoyyeva. - QA/live auth: fail fast when live QA scenarios hit classified auth or runtime failure replies, including raw scenario wait paths, and sanitize missing-key guidance so gateway auth problems surface as actionable errors instead of timeouts. (#63333) Thanks @shakkernerd. +- Matrix/doctor: migrate legacy `channels.matrix.dm.policy: "trusted"` configs back to compatible DM policies during `openclaw doctor --fix`, preserving explicit `allowFrom` boundaries as `allowlist` and defaulting empty legacy configs to `pairing`. (#62942) Thanks @lukeboyett. ## 2026.4.8 diff --git a/extensions/matrix/src/doctor-contract.ts b/extensions/matrix/src/doctor-contract.ts index a3935b70a45..ad4884629f0 100644 --- a/extensions/matrix/src/doctor-contract.ts +++ b/extensions/matrix/src/doctor-contract.ts @@ -71,35 +71,23 @@ function migrateLegacyTrustedDmPolicy(params: { if (!dm || dm.policy !== "trusted") { return { entry: params.entry, changed: false }; } - // Always migrate "trusted" → "pairing" and leave dm.allowFrom intact. - // - // Both "pairing" and "allowlist" merge dm.allowFrom with the pairing store - // entries from core.channel.pairing.readAllowFromStore() in - // resolveMatrixMonitorAccessState (extensions/matrix/src/matrix/monitor/access-state.ts), - // so an explicit allowFrom list and previously paired senders are accepted - // under either policy. The only semantic difference is what happens to an - // unknown sender: - // - // - "pairing": send a pairing request reply (operator can approve) - // - "allowlist": drop silently (no path for new senders to gain access) - // - // We don't know exactly what behavior legacy "trusted" had for unknown - // senders, but "pairing" is a strict superset of "allowlist" for accepting - // existing senders, and it preserves the ability to onboard new ones, which - // is the most likely intent of an operator who chose "trusted" in the first - // place. Mapping unconditionally to "pairing" also avoids the edge case - // where allowFrom is whitespace-only or otherwise effectively empty after - // downstream normalization. - const nextDm = { ...dm, policy: "pairing" }; const allowFromRaw = dm.allowFrom; - const preservedCount = Array.isArray(allowFromRaw) + // Trim before counting: downstream allowlist normalization drops whitespace-only + // entries, so a config like [" "] must still fall back to "pairing" + // instead of becoming an effectively empty allowlist. + const allowFromEntries = Array.isArray(allowFromRaw) ? allowFromRaw.filter( (entry): entry is string => typeof entry === "string" && entry.trim().length > 0, ).length : 0; + // Preserve the operator's existing trust boundary when an explicit allowFrom + // list is present; only fall back to pairing when the effective allowlist is + // empty. + const nextPolicy: "allowlist" | "pairing" = allowFromEntries > 0 ? "allowlist" : "pairing"; + const nextDm = { ...dm, policy: nextPolicy }; params.changes.push( - `Migrated ${params.pathPrefix}.dm.policy "trusted" → "pairing" (legacy alias removed; ` + - `${preservedCount > 0 ? `preserved ${preservedCount} ${params.pathPrefix}.dm.allowFrom ${preservedCount === 1 ? "entry" : "entries"}` : "no allowFrom entries present"}).`, + `Migrated ${params.pathPrefix}.dm.policy "trusted" → "${nextPolicy}" (legacy alias removed; ` + + `${allowFromEntries > 0 ? `preserved ${allowFromEntries} ${params.pathPrefix}.dm.allowFrom ${allowFromEntries === 1 ? "entry" : "entries"}` : "no allowFrom entries present, defaulting to pairing for safety"}).`, ); return { entry: { ...params.entry, dm: nextDm }, changed: true }; } diff --git a/extensions/matrix/src/doctor.test.ts b/extensions/matrix/src/doctor.test.ts index 99526967563..ac4a00f825a 100644 --- a/extensions/matrix/src/doctor.test.ts +++ b/extensions/matrix/src/doctor.test.ts @@ -233,12 +233,7 @@ describe("matrix doctor", () => { ); }); - it("migrates legacy channels.matrix.dm.policy 'trusted' with allowFrom to 'pairing' (allowFrom preserved)", () => { - // Always map "trusted" to "pairing": both policies accept the explicit - // allowFrom list AND any pairing-store entries via the merged - // effectiveAllowFrom in resolveMatrixMonitorAccessState. "pairing" also - // preserves the ability for new senders to request access, which most - // closely matches the intent of an operator who chose "trusted". + it("migrates legacy channels.matrix.dm.policy 'trusted' with allowFrom to 'allowlist'", () => { const normalize = matrixDoctor.normalizeCompatibilityConfig; expect(normalize).toBeDefined(); if (!normalize) { @@ -263,11 +258,11 @@ describe("matrix doctor", () => { result.config.channels?.matrix as { dm?: { policy?: string; allowFrom?: string[] } } )?.dm; - expect(matrixDm?.policy).toBe("pairing"); + expect(matrixDm?.policy).toBe("allowlist"); expect(matrixDm?.allowFrom).toEqual(["@alice:example.org", "@bob:example.org"]); expect(result.changes).toEqual( expect.arrayContaining([ - expect.stringContaining('Migrated channels.matrix.dm.policy "trusted" → "pairing"'), + expect.stringContaining('Migrated channels.matrix.dm.policy "trusted" → "allowlist"'), expect.stringContaining("preserved 2 channels.matrix.dm.allowFrom entries"), ]), ); @@ -373,13 +368,13 @@ describe("matrix doctor", () => { } )?.accounts; - expect(accounts?.work?.dm?.policy).toBe("pairing"); + expect(accounts?.work?.dm?.policy).toBe("allowlist"); expect(accounts?.work?.dm?.allowFrom).toEqual(["@boss:example.org"]); expect(accounts?.personal?.dm?.policy).toBe("pairing"); expect(result.changes).toEqual( expect.arrayContaining([ expect.stringContaining( - 'Migrated channels.matrix.accounts.work.dm.policy "trusted" → "pairing"', + 'Migrated channels.matrix.accounts.work.dm.policy "trusted" → "allowlist"', ), expect.stringContaining( 'Migrated channels.matrix.accounts.personal.dm.policy "trusted" → "pairing"',