fix(acl): a missing allow_ip_literals means allowed (#261)
CI / Build & Quality Checks (pull_request) Successful in 1m54s
CI / Trigger Desktop Build (pull_request) Skipped
CI / Docker image build & smoke test (pull_request) Skipped
CI / Secret scan (gitleaks) (pull_request) Successful in 10s
CI / Playwright smoke (e2e) (pull_request) Successful in 11m45s
CI / Build & Quality Checks (pull_request) Successful in 1m54s
CI / Trigger Desktop Build (pull_request) Skipped
CI / Docker image build & smoke test (pull_request) Skipped
CI / Secret scan (gitleaks) (pull_request) Successful in 10s
CI / Playwright smoke (e2e) (pull_request) Successful in 11m45s
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017PCb59uY3UKqDqt7B6hzd5
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
0eccf9bea9
commit
adf94d7bc1
@@ -32,24 +32,12 @@ import { AsyncStatus, useAsyncCallback } from '../../hooks/useAsyncCallback';
|
|||||||
import { SequenceCard } from '../../components/sequence-card';
|
import { SequenceCard } from '../../components/sequence-card';
|
||||||
import { SequenceCardStyle } from '../common-settings/styles.css';
|
import { SequenceCardStyle } from '../common-settings/styles.css';
|
||||||
import { stopPropagation } from '../../utils/keyboard';
|
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 { MobileTouchTarget } from '../../styles/mobile.css';
|
||||||
import { useModalStyle } from '../../hooks/useModalStyle';
|
import { useModalStyle } from '../../hooks/useModalStyle';
|
||||||
|
|
||||||
// ── Types ─────────────────────────────────────────────────────────────────────
|
// ── 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 ─────────────────────────────────────────────────
|
// ── Server list sub-component ─────────────────────────────────────────────────
|
||||||
|
|
||||||
type ServerListProps = {
|
type ServerListProps = {
|
||||||
@@ -185,20 +173,20 @@ export function RoomServerACL({ requestClose }: RoomServerACLProps) {
|
|||||||
|
|
||||||
// Read current ACL from room state
|
// Read current ACL from room state
|
||||||
const aclEvent = useStateEvent(room, StateEvent.RoomServerAcl);
|
const aclEvent = useStateEvent(room, StateEvent.RoomServerAcl);
|
||||||
const currentAcl = aclEvent?.getContent<ServerAclContent>() ?? DEFAULT_ACL;
|
const currentAcl = normalizeServerAcl(aclEvent?.getContent());
|
||||||
|
|
||||||
// Local draft state — initialised from current ACL
|
// Local draft state — initialised from current ACL
|
||||||
const [allowList, setAllowList] = useState<string[]>(() => currentAcl.allow ?? ['*']);
|
const [allowList, setAllowList] = useState<string[]>(() => currentAcl.allow);
|
||||||
const [denyList, setDenyList] = useState<string[]>(() => currentAcl.deny ?? []);
|
const [denyList, setDenyList] = useState<string[]>(() => currentAcl.deny);
|
||||||
const [allowIpLiterals, setAllowIpLiterals] = useState<boolean>(
|
const [allowIpLiterals, setAllowIpLiterals] = useState<boolean>(
|
||||||
() => currentAcl.allow_ip_literals ?? false,
|
() => currentAcl.allow_ip_literals,
|
||||||
);
|
);
|
||||||
|
|
||||||
// Track whether there are unsaved changes
|
// Track whether there are unsaved changes
|
||||||
const isDirty =
|
const isDirty =
|
||||||
JSON.stringify(allowList) !== JSON.stringify(currentAcl.allow ?? ['*']) ||
|
JSON.stringify(allowList) !== JSON.stringify(currentAcl.allow) ||
|
||||||
JSON.stringify(denyList) !== JSON.stringify(currentAcl.deny ?? []) ||
|
JSON.stringify(denyList) !== JSON.stringify(currentAcl.deny) ||
|
||||||
allowIpLiterals !== (currentAcl.allow_ip_literals ?? false);
|
allowIpLiterals !== currentAcl.allow_ip_literals;
|
||||||
|
|
||||||
// Save handler
|
// Save handler
|
||||||
const [saveState, save] = useAsyncCallback(
|
const [saveState, save] = useAsyncCallback(
|
||||||
@@ -336,7 +324,10 @@ export function RoomServerACL({ requestClose }: RoomServerACLProps) {
|
|||||||
id="allow-ip-literals"
|
id="allow-ip-literals"
|
||||||
checked={allowIpLiterals}
|
checked={allowIpLiterals}
|
||||||
disabled={!canEdit}
|
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"
|
size="300"
|
||||||
variant="Primary"
|
variant="Primary"
|
||||||
/>
|
/>
|
||||||
|
|||||||
@@ -1,6 +1,11 @@
|
|||||||
import { test } from 'node:test';
|
import { test } from 'node:test';
|
||||||
import assert from 'node:assert/strict';
|
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', () => {
|
test('isValidServerPattern: accepts hostnames, IPs, and globs', () => {
|
||||||
for (const v of [
|
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', ['*.evil.com']), false);
|
||||||
assert.equal(matchesAnyGlob('lotusguild.org', []), 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, []);
|
||||||
|
});
|
||||||
|
|||||||
@@ -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<string, unknown> | 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,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user