From adf94d7bc14a604c0983dcd6b66f3d48188f8beb Mon Sep 17 00:00:00 2001 From: Lotus CI Date: Mon, 19 Oct 2026 14:07:03 -0400 Subject: [PATCH] fix(acl): a missing allow_ip_literals means allowed (#261) The Server ACL editor treated an absent allow_ip_literals as false, so the first save in a room with no ACL (or one omitting the field) silently banned IP-literal servers. The spec default is true. normalizeServerAcl() fills the spec defaults once and feeds the initial state, the dirty check and the save. It also stops showing an existing ACL with no allow list as allow-all: per spec that allows no servers, and the /acl command already keeps it as-is. The checkbox is driven by onClick (folds has no onChange), now readOnly so React stops warning about a checked input without onChange. Fixes #261 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017PCb59uY3UKqDqt7B6hzd5 --- .../features/room-settings/RoomServerACL.tsx | 33 ++++++---------- src/app/utils/serverAcl.test.ts | 39 ++++++++++++++++++- src/app/utils/serverAcl.ts | 26 +++++++++++++ 3 files changed, 76 insertions(+), 22 deletions(-) diff --git a/src/app/features/room-settings/RoomServerACL.tsx b/src/app/features/room-settings/RoomServerACL.tsx index 352b6396e..148a7417c 100644 --- a/src/app/features/room-settings/RoomServerACL.tsx +++ b/src/app/features/room-settings/RoomServerACL.tsx @@ -32,24 +32,12 @@ import { AsyncStatus, useAsyncCallback } from '../../hooks/useAsyncCallback'; import { SequenceCard } from '../../components/sequence-card'; import { SequenceCardStyle } from '../common-settings/styles.css'; import { stopPropagation } from '../../utils/keyboard'; -import { isValidServerPattern, matchesAnyGlob } from '../../utils/serverAcl'; +import { isValidServerPattern, matchesAnyGlob, normalizeServerAcl } from '../../utils/serverAcl'; import { MobileTouchTarget } from '../../styles/mobile.css'; import { useModalStyle } from '../../hooks/useModalStyle'; // ── Types ───────────────────────────────────────────────────────────────────── -type ServerAclContent = { - allow: string[]; - deny: string[]; - allow_ip_literals: boolean; -}; - -const DEFAULT_ACL: ServerAclContent = { - allow: ['*'], - deny: [], - allow_ip_literals: false, -}; - // ── Server list sub-component ───────────────────────────────────────────────── type ServerListProps = { @@ -185,20 +173,20 @@ export function RoomServerACL({ requestClose }: RoomServerACLProps) { // Read current ACL from room state const aclEvent = useStateEvent(room, StateEvent.RoomServerAcl); - const currentAcl = aclEvent?.getContent() ?? DEFAULT_ACL; + const currentAcl = normalizeServerAcl(aclEvent?.getContent()); // Local draft state — initialised from current ACL - const [allowList, setAllowList] = useState(() => currentAcl.allow ?? ['*']); - const [denyList, setDenyList] = useState(() => currentAcl.deny ?? []); + const [allowList, setAllowList] = useState(() => currentAcl.allow); + const [denyList, setDenyList] = useState(() => currentAcl.deny); const [allowIpLiterals, setAllowIpLiterals] = useState( - () => currentAcl.allow_ip_literals ?? false, + () => currentAcl.allow_ip_literals, ); // Track whether there are unsaved changes const isDirty = - JSON.stringify(allowList) !== JSON.stringify(currentAcl.allow ?? ['*']) || - JSON.stringify(denyList) !== JSON.stringify(currentAcl.deny ?? []) || - allowIpLiterals !== (currentAcl.allow_ip_literals ?? false); + JSON.stringify(allowList) !== JSON.stringify(currentAcl.allow) || + JSON.stringify(denyList) !== JSON.stringify(currentAcl.deny) || + allowIpLiterals !== currentAcl.allow_ip_literals; // Save handler const [saveState, save] = useAsyncCallback( @@ -336,7 +324,10 @@ export function RoomServerACL({ requestClose }: RoomServerACLProps) { id="allow-ip-literals" checked={allowIpLiterals} disabled={!canEdit} - onClick={() => setAllowIpLiterals(!allowIpLiterals)} + // folds' Checkbox has no onChange; readOnly tells React the + // click handler controls it (no "checked without onChange"). + readOnly + onClick={(e) => setAllowIpLiterals(e.currentTarget.checked)} size="300" variant="Primary" /> diff --git a/src/app/utils/serverAcl.test.ts b/src/app/utils/serverAcl.test.ts index 20bd81999..e793526af 100644 --- a/src/app/utils/serverAcl.test.ts +++ b/src/app/utils/serverAcl.test.ts @@ -1,6 +1,11 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; -import { isValidServerPattern, globToRegExp, matchesAnyGlob } from './serverAcl'; +import { + isValidServerPattern, + globToRegExp, + matchesAnyGlob, + normalizeServerAcl, +} from './serverAcl'; test('isValidServerPattern: accepts hostnames, IPs, and globs', () => { for (const v of [ @@ -42,3 +47,35 @@ test('matchesAnyGlob: true if any glob matches (self-ban detection)', () => { assert.equal(matchesAnyGlob('lotusguild.org', ['*.evil.com']), false); assert.equal(matchesAnyGlob('lotusguild.org', []), false); }); + +test('no ACL event: every server allowed, IP literals allowed (#261)', () => { + assert.deepEqual(normalizeServerAcl(undefined), { + allow: ['*'], + deny: [], + allow_ip_literals: true, + }); +}); + +test('a missing allow_ip_literals defaults to true, per the spec (#261)', () => { + assert.equal(normalizeServerAcl({ allow: ['*'], deny: ['bad.example'] }).allow_ip_literals, true); +}); + +test('an explicit allow_ip_literals is kept', () => { + assert.equal( + normalizeServerAcl({ allow: ['*'], allow_ip_literals: false }).allow_ip_literals, + false, + ); + assert.equal( + normalizeServerAcl({ allow: ['*'], allow_ip_literals: true }).allow_ip_literals, + true, + ); +}); + +test('missing or malformed lists become empty, non-strings are dropped', () => { + assert.deepEqual(normalizeServerAcl({ allow: ['a.example', 3, null] }), { + allow: ['a.example'], + deny: [], + allow_ip_literals: true, + }); + assert.deepEqual(normalizeServerAcl({ allow: 'a.example', deny: {} }).allow, []); +}); diff --git a/src/app/utils/serverAcl.ts b/src/app/utils/serverAcl.ts index c26e61b00..e3db3fee7 100644 --- a/src/app/utils/serverAcl.ts +++ b/src/app/utils/serverAcl.ts @@ -50,3 +50,29 @@ export function matchesAnyGlob(domain: string, globs: string[]): boolean { } }); } + +/** + * `m.room.server_acl` content with the spec's defaults filled in (#261). + * `allow_ip_literals` defaults to **true** when absent; `allow` and `deny` + * default to empty lists, but with no ACL event at all every server is allowed, + * so the editor starts from `allow: ['*']` in that case. + * https://spec.matrix.org/v1.12/client-server-api/#mroomserver_acl + */ +export type ServerAclContent = { + allow: string[]; + deny: string[]; + allow_ip_literals: boolean; +}; + +const stringList = (value: unknown): string[] | undefined => + Array.isArray(value) ? value.filter((v): v is string => typeof v === 'string') : undefined; + +export function normalizeServerAcl(content: Record | undefined): ServerAclContent { + if (!content) return { allow: ['*'], deny: [], allow_ip_literals: true }; + return { + allow: stringList(content.allow) ?? [], + deny: stringList(content.deny) ?? [], + allow_ip_literals: + typeof content.allow_ip_literals === 'boolean' ? content.allow_ip_literals : true, + }; +} -- 2.47.3