fix(security): tab-nabbing hardening + /acl self-lockout guard (SEC-3/4)

SEC-3: add `noopener,noreferrer` to the 5 `window.open(_blank)` sites that
don't use the returned handle (UserChips, OidcManageAccount, OtherDevices x2,
Verification), closing reverse tab-nabbing. SSOStage is intentionally excluded —
it needs the window handle + intact opener for its origin-checked SSO
postMessage handshake.

SEC-4: guard the `/acl` slash command against bricking the room.
- Extract the ACL glob helpers (isValidServerPattern/globToRegExp/matchesAnyGlob)
  from RoomServerACL into a shared utils/serverAcl.ts (+ unit test) so the
  command and the settings editor validate identically.
- Default a MISSING allow list to `*` only when the room has NO existing ACL
  (a first `/acl -d x` otherwise sent `allow: []`, which bricks the room); an
  existing ACL's absent/empty allow is preserved, not silently widened.
- Reject invalid globs; fail CLOSED on the universally-catastrophic cases
  (empty allow, or a `*` deny) even when the local domain is unknown; and reject
  any change that would ban this homeserver (self-lockout).

Guard hardened per two review passes (fail-closed on unknown domain; no silent
federation widening).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

src/app/components/user-profile/UserChips.tsx

@@ -114,7 +114,7 @@ export function ServerChip({ server }: { server: string }) {
                 size="300"
                 radii="300"
                 onClick={() => {
-                  window.open(`https://${server}`, '_blank');
+                  window.open(`https://${server}`, '_blank', 'noopener,noreferrer');
                   close();
                 }}
               >

src/app/features/room-settings/RoomServerACL.tsx

@@ -32,6 +32,7 @@ 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 { useModalStyle } from '../../hooks/useModalStyle';
 
 // ── Types ─────────────────────────────────────────────────────────────────────
@@ -48,57 +49,6 @@ const DEFAULT_ACL: ServerAclContent = {
   allow_ip_literals: false,
 };
 
-// ── Validation ────────────────────────────────────────────────────────────────
-
-/**
- * Validate a server-name glob for an ACL entry.
- *
- * Matrix ACL `allow`/`deny` entries are globs where `*` (any run of chars) and
- * `?` (single char) may appear ANYWHERE — e.g. `*`, `*.example.com`,
- * `1.2.3.*`, `10.0.0.?`, `*.evil.*`, `*bad*`. We therefore validate the *glob*
- * rather than a concrete hostname:
- *   - reject empty / whitespace-only
- *   - allow only hostname/IP chars plus the wildcards `*` and `?`
- *     (letters, digits, dots, hyphens, colons for ports/IPv6 — NO underscore)
- *   - reject consecutive/leading/trailing dots (`...`, `.foo`, `foo.`)
- *   - reject entries with no alphanumeric or wildcard char (bare `-`, lone `:`)
- */
-function isValidServerPattern(value: string): boolean {
-  const v = value.trim();
-  if (!v) return false;
-  // Only hostname/IP glob chars — wildcards may appear at any position.
-  if (!/^[A-Za-z0-9.:*?-]+$/.test(v)) return false;
-  // Structural rules for the dotted parts.
-  if (v.startsWith('.') || v.endsWith('.') || v.includes('..')) return false;
-  // Must carry actual signal — reject pure punctuation like `-`, `:` or `-.-`.
-  if (!/[A-Za-z0-9*?]/.test(v)) return false;
-  return true;
-}
-
-/**
- * Convert an ACL glob (`*` = any run, `?` = single char) to an anchored RegExp,
- * escaping every other regex metacharacter. Used only for local self-ban
- * detection — never sent to the server.
- */
-function globToRegExp(glob: string): RegExp {
-  const escaped = glob.replace(/[.+^${}()|[\]\\]/g, '\\$&');
-  const pattern = escaped.replace(/\*/g, '.*').replace(/\?/g, '.');
-  // Case-INsensitive: Synapse's glob_to_regex uses IGNORECASE and hostnames are
-  // case-insensitive, so a deny like `MATRIX.foo.org` must still be detected as
-  // self-banning `matrix.foo.org` (otherwise the warning is a false negative).
-  return new RegExp(`^${pattern}$`, 'i');
-}
-
-function matchesAnyGlob(domain: string, globs: string[]): boolean {
-  return globs.some((glob) => {
-    try {
-      return globToRegExp(glob).test(domain);
-    } catch {
-      return false;
-    }
-  });
-}
-
 // ── Server list sub-component ─────────────────────────────────────────────────
 
 type ServerListProps = {

src/app/features/settings/account/OidcManageAccount.tsx

@@ -20,7 +20,11 @@ export function OidcManageAccount() {
   const open = useCallback(() => {
     const authUrl = authMetadata?.account_management_uri ?? authMetadata?.issuer;
     if (!authUrl) return;
-    window.open(withSearchParam(authUrl, { action: accountManagementActions.profile }), '_blank');
+    window.open(
+      withSearchParam(authUrl, { action: accountManagementActions.profile }),
+      '_blank',
+      'noopener,noreferrer',
+    );
   }, [authMetadata, accountManagementActions]);
 
   if (!authMetadata) return null;

src/app/features/settings/devices/OtherDevices.tsx

@@ -38,6 +38,7 @@ export function OtherDevices({ devices, refreshDeviceList, showVerification }: O
         action: accountManagementActions.sessionsList,
       }),
       '_blank',
+      'noopener,noreferrer',
     );
   }, [authMetadata, accountManagementActions]);
 
@@ -52,6 +53,7 @@ export function OtherDevices({ devices, refreshDeviceList, showVerification }: O
           device_id: deviceId,
         }),
         '_blank',
+        'noopener,noreferrer',
       );
     },
     [authMetadata, accountManagementActions],

src/app/features/settings/devices/Verification.tsx

@@ -278,6 +278,7 @@ export function DeviceVerificationOptions() {
           action: accountManagementActions.crossSigningReset,
         }),
         '_blank',
+        'noopener,noreferrer',
       );
       return;
     }

src/app/hooks/useCommands.ts

@@ -25,6 +25,7 @@ import {
 import { useRoomNavigate } from './useRoomNavigate';
 import { Membership, StateEvent } from '../../types/matrix/room';
 import { getStateEvent, sendStateEvent } from '../utils/room';
+import { isValidServerPattern, matchesAnyGlob } from '../utils/serverAcl';
 import { splitWithSpace } from '../utils/common';
 import { createRoomEncryptionState } from '../components/create-room';
 
@@ -509,8 +510,15 @@ export const useCommands = (mx: MatrixClient, room: Room): CommandRecord => {
             StateEvent.RoomServerAcl,
           )?.getContent<RoomServerAclEventContent>();
 
+          // When there is NO existing ACL, default allow to `*` (allow all) so a
+          // first `/acl -d evil.com` doesn't send `allow: []` — which per spec
+          // allows NO server and bricks the room. But do NOT synthesize `*` for a
+          // room that already HAS an ACL whose allow key is absent/empty: that
+          // would silently widen a deny-all room to allow-all. Such a state is
+          // preserved as-is and then rejected by the guard below (fail closed,
+          // rather than silently opening federation).
           const aclContent: RoomServerAclEventContent = {
-            allow: serverAcl?.allow ? [...serverAcl.allow] : [],
+            allow: serverAcl ? (serverAcl.allow ? [...serverAcl.allow] : []) : ['*'],
             allow_ip_literals: serverAcl?.allow_ip_literals,
             deny: serverAcl?.deny ? [...serverAcl.deny] : [],
           };
@@ -534,6 +542,40 @@ export const useCommands = (mx: MatrixClient, room: Room): CommandRecord => {
           aclContent.allow?.sort();
           aclContent.deny?.sort();
 
+          // Validate every added glob (empty/whitespace/malformed patterns).
+          const invalidPattern = [...allowList, ...denyList].find(
+            (servername) => !isValidServerPattern(servername),
+          );
+          if (invalidPattern) {
+            throw new Error(`/acl: invalid server pattern "${invalidPattern}"`);
+          }
+
+          const resultAllow = aclContent.allow ?? [];
+          const resultDeny = aclContent.deny ?? [];
+
+          // Fail CLOSED on the universally-catastrophic cases regardless of
+          // whether we can resolve our own server: an empty allow list bans every
+          // server, and a `*` deny bans everyone.
+          if (resultAllow.length === 0 || resultDeny.includes('*')) {
+            throw new Error(
+              '/acl: refused — this would ban all servers from the room. Use Room Settings → Server ACL to edit safely.',
+            );
+          }
+
+          // Refuse a change that would lock THIS homeserver out of the room — a
+          // room-bricking footgun (denying the local server, or an allow list that
+          // no longer matches it). Mirrors the Room Settings ACL editor's self-ban
+          // detection.
+          const localDomain = mx.getDomain() ?? '';
+          if (
+            localDomain &&
+            (!matchesAnyGlob(localDomain, resultAllow) || matchesAnyGlob(localDomain, resultDeny))
+          ) {
+            throw new Error(
+              `/acl: refused — this would ban this homeserver (${localDomain}) from the room`,
+            );
+          }
+
           await sendStateEvent(mx, room.roomId, StateEvent.RoomServerAcl, aclContent);
         },
       },

src/app/utils/serverAcl.test.ts

@@ -0,0 +1,44 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { isValidServerPattern, globToRegExp, matchesAnyGlob } from './serverAcl';
+
+test('isValidServerPattern: accepts hostnames, IPs, and globs', () => {
+  for (const v of [
+    '*',
+    '*.example.com',
+    'matrix.org',
+    '1.2.3.*',
+    '10.0.0.?',
+    '*.evil.*',
+    '*bad*',
+  ]) {
+    assert.equal(isValidServerPattern(v), true, v);
+  }
+});
+
+test('isValidServerPattern: rejects empty, malformed, and non-glob junk', () => {
+  for (const v of ['', '   ', '.foo', 'foo.', 'a..b', '-', ':', 'under_score', 'a,b', 'a b']) {
+    assert.equal(isValidServerPattern(v), false, v);
+  }
+});
+
+test('globToRegExp: * = any run, ? = single char, other metachars escaped', () => {
+  assert.equal(globToRegExp('*').test('anything.org'), true);
+  assert.equal(globToRegExp('*.evil.com').test('a.evil.com'), true);
+  assert.equal(globToRegExp('*.evil.com').test('evil.com'), false); // needs the leading label
+  assert.equal(globToRegExp('1.2.3.?').test('1.2.3.4'), true);
+  assert.equal(globToRegExp('1.2.3.?').test('1.2.3.45'), false);
+  // The dot is a literal, not a regex wildcard.
+  assert.equal(globToRegExp('a.b').test('axb'), false);
+});
+
+test('globToRegExp: case-insensitive (hostnames + Synapse IGNORECASE)', () => {
+  assert.equal(globToRegExp('MATRIX.foo.org').test('matrix.foo.org'), true);
+});
+
+test('matchesAnyGlob: true if any glob matches (self-ban detection)', () => {
+  assert.equal(matchesAnyGlob('lotusguild.org', ['*']), true);
+  assert.equal(matchesAnyGlob('lotusguild.org', ['*.evil.com', 'lotus*.org']), true);
+  assert.equal(matchesAnyGlob('lotusguild.org', ['*.evil.com']), false);
+  assert.equal(matchesAnyGlob('lotusguild.org', []), false);
+});

src/app/utils/serverAcl.ts

@@ -0,0 +1,52 @@
+// Shared helpers for `m.room.server_acl` server-name globs, used by both the
+// Room Settings ACL editor and the `/acl` slash command so their validation and
+// self-lockout detection stay identical.
+
+/**
+ * Validate a server-name glob for an ACL entry.
+ *
+ * Matrix ACL `allow`/`deny` entries are globs where `*` (any run of chars) and
+ * `?` (single char) may appear ANYWHERE — e.g. `*`, `*.example.com`,
+ * `1.2.3.*`, `10.0.0.?`, `*.evil.*`, `*bad*`. We therefore validate the *glob*
+ * rather than a concrete hostname:
+ *   - reject empty / whitespace-only
+ *   - allow only hostname/IP chars plus the wildcards `*` and `?`
+ *     (letters, digits, dots, hyphens, colons for ports/IPv6 — NO underscore)
+ *   - reject consecutive/leading/trailing dots (`...`, `.foo`, `foo.`)
+ *   - reject entries with no alphanumeric or wildcard char (bare `-`, lone `:`)
+ */
+export function isValidServerPattern(value: string): boolean {
+  const v = value.trim();
+  if (!v) return false;
+  // Only hostname/IP glob chars — wildcards may appear at any position.
+  if (!/^[A-Za-z0-9.:*?-]+$/.test(v)) return false;
+  // Structural rules for the dotted parts.
+  if (v.startsWith('.') || v.endsWith('.') || v.includes('..')) return false;
+  // Must carry actual signal — reject pure punctuation like `-`, `:` or `-.-`.
+  if (!/[A-Za-z0-9*?]/.test(v)) return false;
+  return true;
+}
+
+/**
+ * Convert an ACL glob (`*` = any run, `?` = single char) to an anchored RegExp,
+ * escaping every other regex metacharacter. Used only for local self-ban
+ * detection — never sent to the server.
+ */
+export function globToRegExp(glob: string): RegExp {
+  const escaped = glob.replace(/[.+^${}()|[\]\\]/g, '\\$&');
+  const pattern = escaped.replace(/\*/g, '.*').replace(/\?/g, '.');
+  // Case-INsensitive: Synapse's glob_to_regex uses IGNORECASE and hostnames are
+  // case-insensitive, so a deny like `MATRIX.foo.org` must still be detected as
+  // self-banning `matrix.foo.org` (otherwise the warning is a false negative).
+  return new RegExp(`^${pattern}$`, 'i');
+}
+
+export function matchesAnyGlob(domain: string, globs: string[]): boolean {
+  return globs.some((glob) => {
+    try {
+      return globToRegExp(glob).test(domain);
+    } catch {
+      return false;
+    }
+  });
+}