From e3a8106db5574ec3713a808cdf7a0d2d177ed732 Mon Sep 17 00:00:00 2001
From: rumblefrog <contact@zhenyangli.me>
Date: Tue, 19 May 2026 17:48:40 -0400
Subject: [PATCH 1/9] fix(comms): surface SteamID validation errors on the
 comms-add form (#1420)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The "Add a block" form silently no-op'd on invalid SteamID input —
the reporter typed garbage in the SteamID field, clicked submit, and
got no notification anywhere. Two-layer regression:

  1. The submit button was wired to a global `ProcessBan()` defined
     inline in `admin.comms.php`'s tail script. ProcessBan walked the
     form via MooTools-era `$('id')` selectors (still working at
     runtime via `sb.js`'s `global.$` shim that wraps
     `document.getElementById`), validated the SteamID client-side,
     and emitted feedback through `sb.message.show` / `sb.message.error`.
     Those helpers paint into `#dialog-placement` / `#dialog-title` /
     `#dialog-content-text` — v1.x chrome ids the v2.0 theme doesn't
     render anywhere. Every error path silently no-op'd against
     missing DOM targets; the operator saw nothing.

  2. Server-side, `api_comms_add` called `SteamID::toSteam2($raw)`
     unconditionally. The SteamID lib's `resolveInputID` throws a
     generic `\Exception` for unrecognised input shapes; the
     dispatcher's `Throwable` fallback in `Api::handle` wraps that
     as a generic `server_error` envelope (HTTP 500), NOT a
     structured `validation` ApiError with `field=steam`. So even
     if the chrome HAD painted the toast correctly, the operator
     would have seen "Internal server error" instead of "Please
     enter a valid Steam ID".

`api_bans_add` had the same server-side gap on its STEAM-type
branch — the only reason it surfaced visible feedback at all was
that the bans-add form's IIFE was already on the modern
`window.SBPP.showToast` path (the comms-add form was never
migrated). Closing one half without the other would have left
the bans surface still vulnerable to the same 500-cascade if a
hostile/curl-driven caller bypassed the form.

The fix has four layers, top-to-bottom:

* **Form template (sbpp2026 chrome twin)** —
  `web/themes/default/page_admin_comms_add.tpl` adds:
    - `required` on the nickname input
    - `required` + `pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"`
      + `title` + `aria-describedby` on the steam input, paired with
      a visible accepted-formats helper line
    - `required` on the reason `<select>` (the placeholder option
      carries `value=""` so native HTML refuses to submit when the
      operator hasn't chosen anything)
    - An inline IIFE replaces the legacy `onclick="ProcessBan();"`:
      `data-action="addcomm-submit"` on the button, document-level
      click delegate, `form.checkValidity()` short-circuits before
      our handler runs (browser-native popover for empty/wrong-shape
      inputs), `sb.api.call(Actions.CommsAdd)` on valid input,
      `window.SBPP.showToast` for both success ("Block Added") and
      error ("Block NOT Added — <server-supplied message>") with
      `sb.message` graceful-degradation fallback for third-party
      themes that strip theme.js. Per-field `#steam.msg` /
      `#nick.msg` / `#reason.msg` inline anchors still get the
      error so screen readers and tests anchored on the per-field
      shape don't miss it.

* **Bans-form parity** — `page_admin_bans_add.tpl` gets the same
  `pattern` / `title` on its steam input (without `required` since
  IP-typed bans omit it) and `required` on nickname for visual
  parity with the comms form. The IIFE was already on the modern
  toast path; only the native-HTML attrs needed to land.

* **Server-side gate** — both `api_comms_add` and `api_bans_add`
  add an explicit `SteamID::isValidID($rawSteam)` check BEFORE
  calling `SteamID::toSteam2($rawSteam)`. On the fail branch the
  handlers throw `ApiError('validation', 'Please enter a valid
  Steam ID or Community ID', 'steam')` which resolves to HTTP 400
  with a structured envelope the client-side IIFE's
  `r.error.field === 'steam'` branch maps to the per-field
  inline anchor. Empty-string is split out into its own
  ApiError with a "you must type a Steam ID" message; the bans
  handler only fires the SteamID gates when `BanType::Steam` is
  selected (IP-typed bans go through a parallel `inet_pton` gate
  one branch over).

* **Tail-script modernization** — `admin.comms.php`'s
  `changeReason()` helper that toggles the freeform textarea on
  the "other" reason branch switches from `$('dreason').style…` to
  vanilla `document.getElementById('dreason').style…`. The dead
  `ProcessBan()` global is deleted entirely (replaced by the
  template-side IIFE).

Regression coverage, paired:

  - `web/tests/api/CommsTest.php::testAddRejectsInvalidSteamIdShape`
    drives several bad-input shapes (`'asdf'`, `'12345'`, bare
    whitespace, the literal string `'null'`) through `api_comms_add`
    and asserts each returns the structured `validation` ApiError
    with `field=steam`, NOT the generic `server_error` that pre-fix
    leaked through.
  - `web/tests/api/BansTest.php::testAddRejectsInvalidSteamIdShapeForType0`
    mirrors the same shapes against `api_bans_add` with
    `type=Steam`.
  - `web/tests/e2e/specs/flows/comms-add-steamid-validation.spec.ts`
    is the marquee E2E spec (`.serial` describe — five tests share
    the e2e DB):
      1. Empty SteamID → `validity.valueMissing === true`, API
         is NOT called (counter assertion on `/api.php` POSTs).
      2. Malformed SteamID ("asdf") → `validity.patternMismatch
         === true`, API is NOT called.
      3. Curl-style bypass via `page.evaluate(sb.api.call)` →
         envelope `{ok:false, error:{code:'validation',
         field:'steam'}}`, NOT `code:'server_error'`. This is the
         regression test for the original 500-cascade bug.
      4. Happy path → toast paints (`[data-testid="toast"]
         [data-kind="success"][role="status"]`) and the API
         returns `ok:true`.
      5. Third-party-theme-bypass (form's `required` + `pattern`
         stripped via JS, simulating a theme that drops them) →
         server still rejects, IIFE's error branch paints
         `[data-testid="toast"][data-kind="error"][role="alert"]`
         carrying "Block NOT Added — Please enter a valid Steam
         ID …" + per-field inline error mirror.

The five-step E2E shape proves end-to-end that the four-layer
fix actually closes the loop: the native gate stops common-case
mistakes before the API is hit, the server gate stops
curl/third-party-theme bypasses, and the toast/inline anchors
together surface the failure to every operator-visible feedback
surface the v2.0 chrome ships.

Docs in the same PR per project convention: new "JSON API"
convention bullet on the `SteamID::isValidID()` gate (with
canonical code shape) and a matching Anti-patterns entry
explaining the failure mode the gate prevents. AGENTS.md is
prescriptive; the "Where to find what" table didn't need a row
(the contract is bullet-pointed in JSON API + Anti-patterns).

Out of scope (deliberate): no GET-fallback for comms-add exists
(every submission rides `Actions.CommsAdd`), so there's no
parallel `Sbpp\View\Toast::emit` site to add. The `SteamID`
library's slightly-loose regex (`STEAM_0:0:` with empty Z digit
passes `isValidID`) is unchanged — fixing it would have broader
implications and isn't what the reporter hit.

Fixes #1420.
---
 AGENTS.md                                     |  52 +++
 web/api/handlers/bans.php                     |  29 +-
 web/api/handlers/comms.php                    |  21 +-
 web/pages/admin.comms.php                     |  87 ++--
 web/tests/api/BansTest.php                    |  47 +++
 web/tests/api/CommsTest.php                   |  48 +++
 .../comms-add-steamid-validation.spec.ts      | 371 ++++++++++++++++++
 web/themes/default/page_admin_bans_add.tpl    |  24 +-
 web/themes/default/page_admin_comms_add.tpl   | 242 +++++++++++-
 9 files changed, 837 insertions(+), 84 deletions(-)
 create mode 100644 web/tests/e2e/specs/flows/comms-add-steamid-validation.spec.ts

diff --git a/AGENTS.md b/AGENTS.md
index 4eb328435..c301fe73b 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -690,6 +690,35 @@ of the diff ship together or not at all.
 
 - **Do not** add new functions to `sb-callback.php` (removed) or reach
   for xajax (removed).
+- **SteamID inputs**: ALWAYS gate `SteamID::toSteam2($raw)` (or any
+  other `SteamID::*` conversion that calls `resolveInputID` internally)
+  with an explicit `SteamID::isValidID($raw)` check first, and throw
+  `ApiError('validation', '…', '<field>')` on the fail branch.
+  `SteamID::resolveInputID()` throws a generic `\Exception` for
+  unrecognised input shapes; without the gate that exception escapes
+  the handler and the dispatcher's `Throwable` fallback wraps it as a
+  generic `server_error` envelope (HTTP 500), which is unhelpful both
+  to the client-side `r.error.field` branching AND to operators
+  triaging a "the form silently broke" report (#1420). The pattern:
+
+  ```php
+  $rawSteam = trim((string)($params['steam'] ?? ''));
+  if ($rawSteam === '') {
+      throw new ApiError('validation', 'You must type a Steam ID or Community ID', 'steam');
+  }
+  if (!SteamID::isValidID($rawSteam)) {
+      throw new ApiError('validation', 'Please enter a valid Steam ID or Community ID', 'steam');
+  }
+  $steam = SteamID::toSteam2($rawSteam);
+  ```
+
+  See `api_comms_add` / `api_bans_add` for the canonical reference
+  shape. The client-side native validation in the corresponding form
+  template (`pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"`) is
+  the UX-first gate that surfaces a browser-native popover BEFORE the
+  IIFE fires `sb.api.call`; the server-side `isValidID` gate is the
+  load-bearing security gate for curl-driven / third-party-theme
+  callers that bypass it.
 
 ### CSRF
 
@@ -2928,6 +2957,29 @@ contacting every contributor individually.
   bug is a sibling anti-pattern — it would silently reintroduce the
   `LIMIT '0','30'` trap (`page.banlist.php` / `page.commslist.php`
   rejected by MariaDB strict mode). See "Database" under Conventions.
+- Calling `SteamID::toSteam2($raw)` (or any other `SteamID::*`
+  conversion that calls `resolveInputID` internally) on operator-
+  controlled input WITHOUT an `SteamID::isValidID($raw)` gate first
+  → `resolveInputID` throws a generic `\Exception` for unrecognised
+  shapes, and the dispatcher's `Throwable` fallback in `Api::handle`
+  wraps it as a generic `server_error` envelope (HTTP 500). The
+  client-side IIFE in the form template surfaces "Block NOT Added —
+  Internal server error" via `r.error.message` instead of the
+  pointed "Please enter a valid Steam ID or Community ID" that a
+  structured `ApiError('validation', …, 'steam')` would emit. Worse,
+  the legacy comms-add path pre-#1420 emitted feedback through
+  `sb.message.show` against the v1.x `#dialog-placement` chrome
+  shell that the v2.0 theme doesn't render — so the 500 was
+  silent (reporter's symptom on #1420: "no notification on
+  invalid steamID"). The fix is the explicit `isValidID` gate
+  documented under "JSON API" in Conventions; landing it at the
+  same time as the form template's native `pattern` attribute is
+  what closes the loop end-to-end (the `pattern` is the UX-first
+  gate; the `isValidID` gate is the security-first gate; both
+  ship together). See `api_comms_add` / `api_bans_add` for the
+  canonical reference shape and `Php82DeprecationsTest`-style
+  regression coverage in `web/tests/api/CommsTest.php::testAddRejectsInvalidSteamIdShape`
+  and `web/tests/api/BansTest.php::testAddRejectsInvalidSteamIdShapeForType0`.
 - Editing `install/includes/sql/data.sql` (or `struc.sql`) without a paired
   `web/updater/data/<N>.php` → upgraded installs silently miss the change.
 - WYSIWYG / "rich HTML" editors (TinyMCE, CKEditor, …) for fields stored
diff --git a/web/api/handlers/bans.php b/web/api/handlers/bans.php
index a6dfe5302..ceb683bb2 100644
--- a/web/api/handlers/bans.php
+++ b/web/api/handlers/bans.php
@@ -42,7 +42,7 @@ function api_bans_add(array $params): array
     $rawType  = (int)($params['type'] ?? 0);
     $banType  = BanType::tryFrom($rawType) ?? BanType::Steam;
     $type     = $banType->value;
-    $steam    = SteamID::toSteam2(trim((string)($params['steam'] ?? '')));
+    $rawSteam = trim((string)($params['steam'] ?? ''));
     $ip       = preg_replace('#[^\d\.]#', '', (string)($params['ip'] ?? ''));
     $length   = (int)($params['length'] ?? 0);
     $dfile    = (string)($params['dfile'] ?? '');
@@ -50,12 +50,29 @@ function api_bans_add(array $params): array
     $reason   = (string)($params['reason'] ?? '');
     $fromsub  = (int)($params['fromsub'] ?? 0);
 
-    if (empty($steam) && $banType === BanType::Steam) {
-        throw new ApiError('validation', 'You must type a Steam ID or Community ID', 'steam');
-    }
-    if ($banType === BanType::Steam && !SteamID::isValidID($steam)) {
-        throw new ApiError('validation', 'Please enter a valid Steam ID or Community ID', 'steam');
+    // #1420 — validate the SteamID shape BEFORE handing it to
+    // `SteamID::toSteam2()`. `resolveInputID()` throws a bare
+    // `\Exception` (not an `ApiError`) on any unrecognised shape;
+    // that escaped to the dispatcher's catch-all and produced a 500
+    // with body "An unexpected error occurred" instead of the
+    // structured `validation`-coded error the chrome's toast can
+    // render. Comms-add carries the same fix; both halves
+    // intentionally mirror so a SteamID a hostile / curl-driven
+    // caller smuggles past the client-side regex still surfaces a
+    // friendly toast instead of a 500.
+    if ($banType === BanType::Steam) {
+        if ($rawSteam === '') {
+            throw new ApiError('validation', 'You must type a Steam ID or Community ID', 'steam');
+        }
+        if (!SteamID::isValidID($rawSteam)) {
+            throw new ApiError('validation', 'Please enter a valid Steam ID or Community ID', 'steam');
+        }
     }
+    // Skip the conversion entirely for IP-typed bans (rawSteam is
+    // empty for those rows by definition; calling toSteam2 on an
+    // empty string would hit the same `resolveInputID` throw the
+    // guard above defends against).
+    $steam = $rawSteam === '' ? '' : SteamID::toSteam2($rawSteam);
     if (empty($ip) && $banType === BanType::Ip) {
         throw new ApiError('validation', 'You must type an IP', 'ip');
     }
diff --git a/web/api/handlers/comms.php b/web/api/handlers/comms.php
index 789abfd00..be6d326b8 100644
--- a/web/api/handlers/comms.php
+++ b/web/api/handlers/comms.php
@@ -22,16 +22,31 @@ function api_comms_add(array $params): array
     // that Smarty auto-escapes (#1087). Store raw, escape on display.
     $nickname = (string)($params['nickname'] ?? '');
     $type     = (int)($params['type']   ?? 0);
-    $steam    = SteamID::toSteam2(trim((string)($params['steam']  ?? '')));
+    $rawSteam = trim((string)($params['steam'] ?? ''));
     $length   = (int)($params['length'] ?? 0);
     $reason   = (string)($params['reason'] ?? '');
 
-    if (empty($steam)) {
+    // #1420 — validate the SteamID shape BEFORE handing it to
+    // `SteamID::toSteam2()`. The conversion helper calls
+    // `resolveInputID()` internally, which throws a bare `\Exception`
+    // (not an `ApiError`) on any unrecognised shape — that escaped to
+    // the dispatcher's catch-all and surfaced as a generic 500 with
+    // body "An unexpected error occurred. See server logs for
+    // details." instead of the structured `validation`-coded error
+    // shape the chrome's toast can render. The reporter observed it
+    // as "no notification" because the comms add page's tail script
+    // was *also* broken (legacy MooTools `$('id')` selectors against
+    // a global that no longer exists post-#1123 D1), so the API
+    // round-trip never happened — fixing the front-end alone would
+    // have exposed the same 500 the bans-add path also carries.
+    // Mirror `api_bans_add`'s validation order in both handlers.
+    if ($rawSteam === '') {
         throw new ApiError('validation', 'You must type a Steam ID or Community ID', 'steam');
     }
-    if (!SteamID::isValidID($steam)) {
+    if (!SteamID::isValidID($rawSteam)) {
         throw new ApiError('validation', 'Please enter a valid Steam ID or Community ID', 'steam');
     }
+    $steam = SteamID::toSteam2($rawSteam);
     if (!in_array($type, [1, 2, 3], true)) {
         throw new ApiError('validation', 'Invalid block type. Must be one of: gag (1), mute (2), or both (3).', 'type');
     }
diff --git a/web/pages/admin.comms.php b/web/pages/admin.comms.php
index 7036ae9a0..18b8db62a 100644
--- a/web/pages/admin.comms.php
+++ b/web/pages/admin.comms.php
@@ -103,72 +103,35 @@
 ));
 ?>
 <script type="text/javascript">
+// `changeReason` toggles the freeform `#dreason` textarea container
+// when the operator picks "other" from the reason `<select>`. The
+// `<select onchange="...">` attribute in page_admin_comms_add.tpl
+// keeps invoking this helper; the rewrite to vanilla DOM landed
+// alongside #1420 (the legacy `$('id').setStyle(...)` shape relied
+// on the MooTools-compat wrapping in sb.js — vanilla DOM is the
+// modern convention per AGENTS.md's "vanilla DOM helpers" rule and
+// removes one layer of indirection between the click and the
+// visible state change).
 function changeReason(szListValue)
 {
-    $('dreason').style.display = (szListValue == "other" ? "block" : "none");
+    var el = document.getElementById('dreason');
+    if (el) el.style.display = (szListValue == "other" ? "block" : "none");
 }
-// Local wrapper around window.SBPP.setBusy with a `disabled`-only fallback
-// so a third-party theme that strips theme.js still gates against double-clicks.
-function __sbppSetBusy(btn, busy) {
-    if (!btn) return;
-    var S = window.SBPP;
-    if (S && typeof S.setBusy === 'function') S.setBusy(btn, busy);
-    else btn.disabled = busy === undefined ? true : !!busy;
-}
-
-function ProcessBan()
-{
-    var reason = $('listReason')[$('listReason').selectedIndex].value;
 
-    if (reason == "other") {
-        reason = $('txtReason').value;
-    }
-    var submitBtn = document.getElementById('addcomm-submit');
-    __sbppSetBusy(submitBtn, true);
-    sb.api.call(Actions.CommsAdd, {
-        nickname: $('nickname').value,
-        type:     Number($('type').value),
-        steam:    $('steam').value,
-        length:   Number($('banlength').value),
-        reason:   reason,
-    }).then(function (r) {
-        // sb.message (sb.js) replaces the v1.x ShowBlockBox /
-        // TabToReload / applyApiResponse helpers.
-        // The iframe is load-bearing — pages/admin.blockit.php loops the enabled servers and
-        // fires `sc_fw_block` via rcon for each one. Without it the DB row exists but no live
-        // server learns about the gag/mute, matching the bans/kickit shape one branch above.
-        if (r && r.ok && r.data && r.data.block) {
-            // Success — the message dialog covers the form and the page reloads
-            // shortly after; leave the button busy so the operator can't queue a
-            // second submit while the iframe fires rcon at every server.
-            var b = r.data.block;
-            sb.message.show(
-                'Block Added',
-                'The block has been successfully added<br><iframe id="srvkicker" frameborder="0" width="100%" src="pages/admin.blockit.php?check='
-                    + encodeURIComponent(b.steam) + '&type=' + encodeURIComponent(b.type) + '&length=' + encodeURIComponent(b.length) + '"></iframe>',
-                'green',
-                'index.php?p=admin&c=comms',
-                true
-            );
-            if (r.data.reload) setTimeout(function () { window.location.href = window.location.href.replace(/#\^.*$/, ''); }, 2000);
-            return;
-        }
-        __sbppSetBusy(submitBtn, false);
-        if (!r) return;
-        if (r.redirect) return;
-        if (r.ok === false) {
-            if (r.error) sb.message.error('Error', r.error.message || 'Unknown error');
-            return;
-        }
-        var data = r.data || {};
-        if (data.message) {
-            sb.message.show(data.message.title, data.message.body, data.message.kind, data.message.redir, data.message.noclose);
-        }
-        if (data.reload) {
-            setTimeout(function () { window.location.href = window.location.href.replace(/#\^.*$/, ''); }, 2000);
-        }
-    });
-}
+// #1420: pre-fix this file shipped a global `ProcessBan()` that
+// the submit button invoked via `onclick="ProcessBan();"`. That
+// helper walked the form via the legacy MooTools `$('id')` shim
+// (still working via sb.js's `global.$`) and emitted feedback
+// through `sb.message.show` / `sb.message.error`, which paint into
+// `#dialog-placement` / `#dialog-title` (v1.x chrome ids the v2.0
+// theme doesn't render). The submit path was effectively silent
+// on every error branch — the bug the reporter filed.
+// The replacement lives in page_admin_comms_add.tpl's inline IIFE
+// (mirrors page_admin_bans_add.tpl's shape): native HTML
+// validation first, then sb.api.call(Actions.CommsAdd), then
+// window.SBPP.showToast on the error envelope. The global is
+// gone; nothing in the codebase references it under the v2.0
+// theme.
 
 // Self-contained DOM-prefill helper (replaces the v1.x applyBlockFields)
 // so reblock / blockfromban / pasteBlock all keep prefilling the form.
diff --git a/web/tests/api/BansTest.php b/web/tests/api/BansTest.php
index 781f53e8c..228ac6f49 100644
--- a/web/tests/api/BansTest.php
+++ b/web/tests/api/BansTest.php
@@ -79,6 +79,53 @@ public function testAddValidationMissingSteamForType0(): void
         $this->assertSnapshot('bans/add_validation_steam', $env);
     }
 
+    /**
+     * #1420 — regression: a SteamID input the `SteamID` library
+     * can't pattern-match used to escape from `api_bans_add` as a
+     * generic `\Exception("Invalid SteamID input!")` thrown deep
+     * inside `SteamID::resolveInputID()`. The dispatcher caught it
+     * in the catch-all `Throwable` arm and surfaced a 500 with
+     * body "An unexpected error occurred", which the chrome read
+     * as a server error rather than a per-field validation
+     * failure. The fix validates the SteamID shape BEFORE handing
+     * it to `SteamID::toSteam2()`. Comms-add carries the same
+     * fix; both halves intentionally mirror so a SteamID a
+     * hostile / curl-driven caller smuggles past the client-side
+     * regex still surfaces a friendly structured error envelope
+     * instead of a 500.
+     */
+    public function testAddRejectsInvalidSteamIdShapeForType0(): void
+    {
+        $this->loginAsAdmin();
+        // Three shape classes the pre-fix `SteamID::resolveInputID`
+        // throws on (each fails every isValidID regex):
+        //   - `asdf` — the reporter's example; pure non-SteamID text.
+        //   - `12345` — short numeric (not 17 digits).
+        //   - `7656119xxxxxxxxxx` — 17 chars but not 17 digits.
+        // `'STEAM_0:0:'` is intentionally NOT tested: it passes
+        // `isValidID` (the regex's `\d*` accepts zero digits) and
+        // round-trips through `toSteam2` unchanged. Catching it
+        // would require tightening `SteamID::isValidID`, which is
+        // out of scope for #1420.
+        foreach (['asdf', '12345', '7656119xxxxxxxxxx'] as $badSteam) {
+            $env = $this->api('bans.add', [
+                'nickname' => 'X',
+                'type'     => 0,
+                'steam'    => $badSteam,
+                'ip'       => '',
+                'length'   => 0,
+                'reason'   => '',
+                'fromsub'  => 0,
+            ]);
+            $this->assertEnvelopeError($env, 'validation');
+            $this->assertSame(
+                'steam',
+                $env['error']['field'],
+                sprintf('expected error.field=steam for steam=%s', var_export($badSteam, true)),
+            );
+        }
+    }
+
     public function testAddValidationInvalidIpForType1(): void
     {
         $this->loginAsAdmin();
diff --git a/web/tests/api/CommsTest.php b/web/tests/api/CommsTest.php
index 5076e8d14..e2a6212f1 100644
--- a/web/tests/api/CommsTest.php
+++ b/web/tests/api/CommsTest.php
@@ -131,6 +131,54 @@ public function testAddValidatesSteamIdRequired(): void
         $this->assertSnapshot('comms/add_validation_steam', $env);
     }
 
+    /**
+     * #1420 — regression: a SteamID input the `SteamID` library
+     * can't pattern-match (anything not matching STEAM_X:Y:Z /
+     * [U:1:N] / 17-digit SteamID64) used to escape from the
+     * handler as a generic `\Exception("Invalid SteamID input!")`
+     * thrown deep inside `SteamID::resolveInputID()`. The dispatcher
+     * caught it in the catch-all `Throwable` arm and surfaced a
+     * 500 with body "An unexpected error occurred" — the panel's
+     * chrome read that as a server error, not a per-field
+     * validation failure, and the toast helper bound to the legacy
+     * `dialog-placement` element silently no-op'd against the v2.0
+     * chrome (the reporter's "no notification" symptom).
+     *
+     * Post-fix `api_comms_add` validates the SteamID shape BEFORE
+     * handing it to `SteamID::toSteam2()` so a malformed value
+     * surfaces a structured `validation`-coded error with
+     * `field=steam`, which the chrome can render as a per-field
+     * toast / inline message. Mirrors the bans-add fix in the
+     * same PR.
+     *
+     * Cases:
+     *   - `'asdf'` — the reporter's example; doesn't match any
+     *     known SteamID shape.
+     *   - `'12345'` — a numeric string that's NOT 17 digits; would
+     *     fall through `resolveInputID`'s shape table.
+     *   - `'   '` — whitespace-only; trims down to '' and surfaces
+     *     the "must type" branch, but still as a structured error.
+     */
+    public function testAddRejectsInvalidSteamIdShape(): void
+    {
+        $this->loginAsAdmin();
+        foreach (['asdf', '12345', '   '] as $badSteam) {
+            $env = $this->api('comms.add', [
+                'nickname' => 'BadShape',
+                'type'     => 1,
+                'steam'    => $badSteam,
+                'length'   => 5,
+                'reason'   => 'unit test',
+            ]);
+            $this->assertEnvelopeError($env, 'validation');
+            $this->assertSame(
+                'steam',
+                $env['error']['field'],
+                sprintf('expected error.field=steam for steam=%s', var_export($badSteam, true)),
+            );
+        }
+    }
+
     public function testAddValidatesType(): void
     {
         $this->loginAsAdmin();
diff --git a/web/tests/e2e/specs/flows/comms-add-steamid-validation.spec.ts b/web/tests/e2e/specs/flows/comms-add-steamid-validation.spec.ts
new file mode 100644
index 000000000..beeef9ec7
--- /dev/null
+++ b/web/tests/e2e/specs/flows/comms-add-steamid-validation.spec.ts
@@ -0,0 +1,371 @@
+/**
+ * Flow spec — issue #1420: "Comms block steamID validation — no
+ * notification on invalid steamID".
+ *
+ * Pre-fix the admin "Add a block" form's submit button was wired
+ * via `onclick="ProcessBan();"` (a global defined inline in
+ * `web/pages/admin.comms.php`). ProcessBan walked the form via
+ * MooTools-era `$('id')` selectors (still working at runtime via
+ * `web/scripts/sb.js`'s `global.$` compatibility shim that wraps
+ * `document.getElementById`) and surfaced validation feedback
+ * through `sb.message.show` / `sb.message.error`. Those helpers
+ * paint into `#dialog-placement` / `#dialog-title` /
+ * `#dialog-content-text` — DOM ids the v2.0 chrome doesn't render
+ * anywhere. Net result on a malformed SteamID: the click fired,
+ * the page-tail JS rejected with an "invalid SteamID" branch, and
+ * `sb.message.show` silently no-op'd against a missing DOM target.
+ * The operator hit submit again, the JSON API surfaced a 500 (the
+ * raw SteamID got past the form-side check and reached
+ * `SteamID::toSteam2()` which throws a generic `\Exception` on
+ * unrecognised input, caught by `Api::handle`'s `Throwable`
+ * fallback as a generic `server_error`). User-visible symptom:
+ * "no notification".
+ *
+ * The fix has three parts (mirrored in this spec's three test
+ * categories):
+ *
+ *   1. **Native HTML validation** is the first feedback surface.
+ *      The `steam` input carries `required` + `pattern="…"` so
+ *      the browser surfaces its native popover for empty / wrong-
+ *      shape inputs BEFORE our submit handler runs. The form's
+ *      `checkValidity()` short-circuits the IIFE. The API is
+ *      NEVER called for inputs that fail native validation.
+ *
+ *   2. **Server-side structured rejection** is the load-bearing
+ *      security gate. A curl-driven caller (or a third-party
+ *      theme that strips the IIFE) that bypasses #1 still gets
+ *      a structured `ApiError('validation', …, 'steam')` —
+ *      `Sbpp\Api\ApiError` resolves to HTTP 400 with a
+ *      `{ok:false, error:{code:'validation', field:'steam'}}`
+ *      envelope, NOT a 500. Pre-fix the `SteamID::resolveInputID`
+ *      throw escaped as `server_error`, the dev console showed
+ *      `500 Internal Server Error`, and the operator had no
+ *      actionable error message.
+ *
+ *   3. **Modern chrome toast feedback** replaces the dead
+ *      `sb.message.show` path. The new IIFE in
+ *      `page_admin_comms_add.tpl` routes success / error envelopes
+ *      through `window.SBPP.showToast` (theme.js's `[data-testid="toast"]`
+ *      paint). The legacy inline `<div id="steam.msg">` survives
+ *      for screen readers + as a per-field anchor for the error
+ *      message, but the visible feedback is the toast.
+ *
+ * Mirrors the test shape from `admin-edit-comms-toast.spec.ts` (the
+ * other #1403-era comms-form regression spec): admin storage state,
+ * single browser context, asserts the painted toast element via
+ * `[data-testid="toast"][data-kind="error"]` (the kind-aware role
+ * contract from #1409 is also enforced — the painted element
+ * carries `role="alert"` for error toasts).
+ *
+ * **Out of scope** (deliberate):
+ *   - The legacy GET fallback. `admin.comms.php` has no GET fallback
+ *     for the add path — every submission rides `Actions.CommsAdd`
+ *     through `sb.api.call`. The page itself only serves the form
+ *     surface and a 302 to commslist after success.
+ *   - The bans-equivalent native-validation lift (`page_admin_bans_add.tpl`
+ *     also got `pattern` / `required` adds in this PR). That's
+ *     covered by `web/tests/api/BansTest.php::testAddRejectsInvalidSteamIdShapeForType0`
+ *     at the PHPUnit layer; the bans add form's pre-existing IIFE
+ *     already routed `sb.api.call` errors through the modern toast,
+ *     so the visible-feedback regression was strictly on the comms
+ *     side. Adding a bans-form E2E spec for parity would be future
+ *     work if the IIFE-side branch ever regresses.
+ *
+ * Admin storage state: the seeded `admin/admin` user carries
+ * `WebPermission::Owner` which satisfies the form's
+ * `ADMIN_OWNER|ADMIN_ADD_BAN` gate. Default storage state from
+ * `fixtures/global-setup.ts` covers it.
+ */
+
+import { test, expect } from '../../fixtures/auth.ts';
+import { truncateE2eDb } from '../../fixtures/db.ts';
+
+const VALID_STEAM = 'STEAM_0:1:14202020';
+const INVALID_STEAM = 'asdf';
+const TARGET_NICK = 'e2e-1420-validation';
+
+// `.serial` because every test in this describe runs `truncateE2eDb()`
+// in `beforeEach`, and a sibling test's API call landing during another
+// test's truncate-and-reseed window gets "forbidden" (the admin row
+// was momentarily gone). The CI gate runs `workers: 1` per
+// `playwright.config.ts` so this matches the production shape; the
+// `.serial` keeps local-dev runs (default `workers: <cores>`) honest
+// too. Mirrors how `comms-gag-mute.spec.ts` keeps a single
+// state-mutating test per describe — the cleaner shape for >1 test
+// is `.serial`.
+test.describe.serial('flow: comms-add SteamID validation feedback (#1420)', () => {
+    test.beforeEach(async ({}, testInfo) => {
+        // Sibling spec (`comms-gag-mute.spec.ts`) documents the
+        // workers:1 / truncateE2eDb-isn't-parallel-project-safe
+        // constraint. We need a clean slate for the happy-path
+        // assertion (the `[data-id="…"]` lookup downstream would
+        // otherwise race with sibling specs' inserts), so pin to
+        // chromium; mobile-chromium coverage isn't load-bearing
+        // for "did the toast paint" (the visible-feedback assertion
+        // is browser-engine-agnostic).
+        test.skip(
+            testInfo.project.name !== 'chromium',
+            'state-mutating flow; truncateE2eDb is not parallel-project-safe',
+        );
+        await truncateE2eDb();
+    });
+
+    test('empty SteamID → browser-native popover; API not called', async ({ page }) => {
+        await page.goto('/index.php?p=admin&c=comms');
+        await expect(page.locator('[data-testid="addcomm-form"]')).toBeVisible();
+
+        // Count POSTs to /api.php. The native validation should
+        // SHORT-CIRCUIT the submit handler — the browser surfaces
+        // its popover and we never reach `sb.api.call`. If this
+        // counter ends > 0 the native-validation gate is broken.
+        let apiCalls = 0;
+        page.on('request', (req) => {
+            if (req.url().includes('/api.php') && req.method() === 'POST') {
+                apiCalls += 1;
+            }
+        });
+
+        // Fill EVERY field except `steam` so the only thing the
+        // browser can complain about is the empty SteamID.
+        await page.locator('[data-testid="addcomm-nickname"]').fill(TARGET_NICK);
+        await page.locator('[data-testid="addcomm-type"]').selectOption('2');
+        await page.locator('[data-testid="addcomm-length"]').selectOption('5');
+        await page.locator('[data-testid="addcomm-reason"]').selectOption('Obscene language');
+        // `steam` is empty by default; explicit assertion so a future
+        // template change that auto-fills it surfaces here, not
+        // silently masks the regression.
+        await expect(page.locator('[data-testid="addcomm-steam"]')).toHaveValue('');
+
+        await page.locator('[data-testid="addcomm-submit"]').click();
+
+        // Native validation predicate. `valueMissing === true` for
+        // an empty `required` input — that's the property the
+        // browser uses to drive its popover. The form-level
+        // `checkValidity()` returns false because of this.
+        const validity = await page.locator('[data-testid="addcomm-steam"]').evaluate(
+            (el: Element) => {
+                const input = el as HTMLInputElement;
+                return {
+                    valid: input.validity.valid,
+                    valueMissing: input.validity.valueMissing,
+                    patternMismatch: input.validity.patternMismatch,
+                };
+            },
+        );
+        expect(validity.valid, 'empty steam input is invalid (valueMissing)').toBe(false);
+        expect(validity.valueMissing, 'empty steam input flagged as valueMissing').toBe(true);
+
+        // The API must NOT have been hit — native validation is
+        // the load-bearing client-side gate. Pre-#1420 the legacy
+        // `ProcessBan()` walked past the empty input and POSTed
+        // `steam=''` to the JSON action, which then errored.
+        expect(apiCalls, 'API should NOT be called for an empty SteamID').toBe(0);
+    });
+
+    test('malformed SteamID ("asdf") → browser-native popover; API not called', async ({ page }) => {
+        await page.goto('/index.php?p=admin&c=comms');
+        await expect(page.locator('[data-testid="addcomm-form"]')).toBeVisible();
+
+        let apiCalls = 0;
+        page.on('request', (req) => {
+            if (req.url().includes('/api.php') && req.method() === 'POST') {
+                apiCalls += 1;
+            }
+        });
+
+        await page.locator('[data-testid="addcomm-steam"]').fill(INVALID_STEAM);
+        await page.locator('[data-testid="addcomm-nickname"]').fill(TARGET_NICK);
+        await page.locator('[data-testid="addcomm-type"]').selectOption('2');
+        await page.locator('[data-testid="addcomm-length"]').selectOption('5');
+        await page.locator('[data-testid="addcomm-reason"]').selectOption('Obscene language');
+
+        await page.locator('[data-testid="addcomm-submit"]').click();
+
+        // `patternMismatch === true` for "asdf" against the
+        // `STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}` regex. The
+        // input is non-empty so `valueMissing === false`. The
+        // form's `checkValidity()` still returns false because
+        // the pattern fails.
+        const validity = await page.locator('[data-testid="addcomm-steam"]').evaluate(
+            (el: Element) => {
+                const input = el as HTMLInputElement;
+                return {
+                    valid: input.validity.valid,
+                    valueMissing: input.validity.valueMissing,
+                    patternMismatch: input.validity.patternMismatch,
+                };
+            },
+        );
+        expect(validity.valid, 'malformed steam input is invalid (patternMismatch)').toBe(false);
+        expect(validity.patternMismatch, 'malformed steam input flagged as patternMismatch').toBe(true);
+        expect(validity.valueMissing, 'non-empty input is not valueMissing').toBe(false);
+
+        expect(apiCalls, 'API should NOT be called for a malformed SteamID').toBe(0);
+    });
+
+    test('curl-style bypass: server-side rejects malformed SteamID with 400 + structured error (NOT 500)', async ({ page }) => {
+        // Pre-#1420 a hostile/curl-driven caller that bypassed the
+        // form's native validation got a generic 500 response
+        // because `SteamID::resolveInputID` threw `\Exception` and
+        // `Api::handle`'s `Throwable` fallback wrapped it as a
+        // generic `server_error` envelope. The fix adds an
+        // explicit `SteamID::isValidID($rawSteam)` gate inside
+        // `api_comms_add` that throws `ApiError('validation', …,
+        // 'steam')` — `Sbpp\Api\ApiError` resolves to HTTP 400
+        // with a structured `{ok:false, error:{code:'validation',
+        // field:'steam'}}` envelope. This test drives the bypass
+        // path by calling `sb.api.call(Actions.CommsAdd, …)`
+        // directly from a same-origin page (so the CSRF meta tag,
+        // the api.js client, and the autogenerated Actions
+        // registry are all in scope), the same trick
+        // `fixtures/seeds.ts`'s `seedCommViaApi` uses.
+        await page.goto('/index.php?p=admin&c=comms');
+        await expect(page.locator('[data-testid="addcomm-form"]')).toBeVisible();
+
+        const result = await page.evaluate(async (badSteam) => {
+            const w = window as unknown as {
+                sb: {
+                    api: {
+                        call: (
+                            action: string,
+                            params: Record<string, unknown>,
+                        ) => Promise<{
+                            ok: boolean;
+                            error?: { code: string; message: string; field?: string };
+                        }>;
+                    };
+                };
+                Actions: Record<string, string>;
+            };
+            // Mirror what the form's IIFE sends, but with `steam`
+            // set to "asdf" — the same string the operator would
+            // type into the input if native validation were off.
+            const envelope = await w.sb.api.call(w.Actions.CommsAdd, {
+                nickname: 'e2e-1420-bypass',
+                type: 1,
+                steam: badSteam,
+                length: 0,
+                reason: 'e2e: invalid steam id bypass',
+            });
+            return envelope;
+        }, INVALID_STEAM);
+
+        expect(result.ok, 'envelope ok=false for malformed steam').toBe(false);
+        expect(result.error?.code, 'envelope error.code = validation (not server_error)').toBe(
+            'validation',
+        );
+        expect(result.error?.field, 'envelope error.field = steam').toBe('steam');
+        expect(
+            result.error?.message,
+            'envelope carries an actionable message',
+        ).toMatch(/valid Steam ID|Community ID/i);
+    });
+
+    test('happy path: valid SteamID + reason → toast paints + row created', async ({ page }) => {
+        await page.goto('/index.php?p=admin&c=comms');
+        await expect(page.locator('[data-testid="addcomm-form"]')).toBeVisible();
+
+        await page.locator('[data-testid="addcomm-steam"]').fill(VALID_STEAM);
+        await page.locator('[data-testid="addcomm-nickname"]').fill(TARGET_NICK);
+        await page.locator('[data-testid="addcomm-type"]').selectOption('2');
+        await page.locator('[data-testid="addcomm-length"]').selectOption('5');
+        await page.locator('[data-testid="addcomm-reason"]').selectOption('Obscene language');
+
+        // Wait for the JSON API to settle BEFORE asserting on the
+        // toast — the IIFE schedules a `setTimeout(...,2000)`
+        // reload after success that would otherwise tear the
+        // toast DOM down mid-assertion. The toast paints
+        // synchronously off the `then` callback so it's visible
+        // well before the 2-second reload fires.
+        const apiResponse = page.waitForResponse(
+            (r) => r.url().includes('/api.php') && r.request().method() === 'POST',
+        );
+        await page.locator('[data-testid="addcomm-submit"]').click();
+        const response = await apiResponse;
+        expect(response.status(), 'comms.add returns 200').toBe(200);
+        const body = await response.json();
+        expect(body, 'comms.add envelope').toMatchObject({ ok: true });
+
+        // Success toast paints via `window.SBPP.showToast` —
+        // [data-testid="toast"] is the chrome-rendered element
+        // post-#1409 (kind-aware role contract: role="status" for
+        // non-error kinds, role="alert" for error kinds). The
+        // [data-kind="success"] anchor disambiguates from any
+        // sibling info / warn toasts queued from upstream calls.
+        const successToast = page
+            .locator('[data-testid="toast"][data-kind="success"]')
+            .filter({ hasText: 'Block Added' });
+        await expect(successToast).toBeVisible({ timeout: 1500 });
+        // role="status" is the polite live-region role for non-error
+        // toasts per the chrome's kind-aware role contract.
+        await expect(successToast).toHaveAttribute('role', 'status');
+    });
+
+    test('handler rejects bypass via toast on the form (single round-trip, structured error)', async ({ page }) => {
+        // Companion to the curl-style bypass test above — drives
+        // the same `sb.api.call` from inside the form's own page
+        // (the operator's session context) and asserts the toast
+        // surface paints. This is the "third-party theme strips
+        // the IIFE's native-validation guard but keeps the chrome's
+        // toast surface" failure mode; the server rejects with a
+        // structured ApiError and the toast emits via the IIFE's
+        // error branch.
+        //
+        // Why this is distinct from the curl-style test above:
+        // that test asserts the envelope shape (the load-bearing
+        // server-side contract); THIS test asserts the IIFE's
+        // toast emission on the error envelope (the load-bearing
+        // chrome-side contract for the operator-visible feedback
+        // the reporter explicitly called out as missing). The
+        // two together prove the end-to-end fix.
+        await page.goto('/index.php?p=admin&c=comms');
+        await expect(page.locator('[data-testid="addcomm-form"]')).toBeVisible();
+
+        // Bypass native validation by stripping `pattern` +
+        // `required` from the steam input via JS, then drive the
+        // form submit normally. This is the literal "third-party
+        // theme without the gate" shape — the IIFE still runs,
+        // checkValidity() now passes (no `required` / `pattern`),
+        // `sb.api.call(Actions.CommsAdd)` fires, the server
+        // rejects with `ApiError('validation', …, 'steam')`, and
+        // the IIFE's `r.ok === false` branch surfaces the error
+        // through `window.SBPP.showToast`.
+        await page.evaluate(() => {
+            const el = document.getElementById('steam') as HTMLInputElement | null;
+            if (el) {
+                el.removeAttribute('required');
+                el.removeAttribute('pattern');
+            }
+        });
+
+        await page.locator('[data-testid="addcomm-steam"]').fill(INVALID_STEAM);
+        await page.locator('[data-testid="addcomm-nickname"]').fill(TARGET_NICK);
+        await page.locator('[data-testid="addcomm-type"]').selectOption('2');
+        await page.locator('[data-testid="addcomm-length"]').selectOption('5');
+        await page.locator('[data-testid="addcomm-reason"]').selectOption('Obscene language');
+
+        await page.locator('[data-testid="addcomm-submit"]').click();
+
+        // Error toast paints with the IIFE's "Block NOT Added"
+        // title + the server-supplied message. `role="alert"`
+        // is the assertive live-region role for error kinds
+        // (kind-aware role contract from #1409 — error toasts
+        // get role="alert" so screen-reader users get an
+        // interrupting announcement on destructive-action
+        // failure, in this case "the block didn't go through
+        // because the SteamID was invalid").
+        const errorToast = page
+            .locator('[data-testid="toast"][data-kind="error"]')
+            .filter({ hasText: 'Block NOT Added' });
+        await expect(errorToast).toBeVisible({ timeout: 1500 });
+        await expect(errorToast).toHaveAttribute('role', 'alert');
+        await expect(errorToast).toContainText(/valid Steam ID|Community ID/i);
+
+        // Per-field anchor (`#steam.msg`) also gets the error so
+        // screen-reader users navigating by form-field labels
+        // surface the error in context. Visible inline.
+        const inlineErr = page.locator('#steam\\.msg');
+        await expect(inlineErr).toBeVisible();
+        await expect(inlineErr).toContainText(/valid Steam ID|Community ID/i);
+    });
+});
diff --git a/web/themes/default/page_admin_bans_add.tpl b/web/themes/default/page_admin_bans_add.tpl
index 8b99224a6..901b01b04 100644
--- a/web/themes/default/page_admin_bans_add.tpl
+++ b/web/themes/default/page_admin_bans_add.tpl
@@ -51,12 +51,18 @@
 
             <div>
                 <label class="label" for="nickname">Nickname</label>
+                {* #1420: `required` is the native gate paired with the
+                   IIFE's `nick.msg` fallback (the IIFE's regex layer
+                   still runs for the cases native HTML can't catch).
+                   Together they make the empty-input case surface a
+                   browser popover BEFORE the API call fires. *}
                 <input type="text"
                        class="input"
                        id="nickname"
                        name="nickname"
                        data-testid="addban-nickname"
-                       placeholder="Display name as it appeared in-game">
+                       placeholder="Display name as it appeared in-game"
+                       required>
                 <div class="text-xs mt-2" id="nick.msg" style="color:var(--danger);display:none"></div>
             </div>
 
@@ -129,13 +135,27 @@
                    value reaches the template, so the auto-escape is the
                    belt-and-braces). Used by the public servers list's
                    right-click context menu's "Ban player" item. *}
+                {* #1420: `pattern` (no `required`) mirrors the inline
+                   IIFE's regex for type=0; this lets the browser show
+                   a native popover for a non-empty bad-shape value
+                   like "asdf" BEFORE the IIFE's setMsg path runs.
+                   `required` is intentionally absent because for
+                   type=1 (IP Address) the steam field is legitimately
+                   empty and the ip field carries the value — the
+                   IIFE's per-type check is the right gate for that
+                   branch. The defensive server-side validation in
+                   api_bans_add (also #1420) is what closes the loop
+                   if a hostile / curl-driven caller smuggles a bad
+                   shape past every client-side check. *}
                 <input type="text"
                        class="input font-mono"
                        id="steam"
                        name="steam"
                        data-testid="addban-steam"
                        value="{if $prefill_type == 0}{$prefill_steam}{/if}"
-                       placeholder="STEAM_0:1:23498765">
+                       placeholder="STEAM_0:1:23498765"
+                       pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"
+                       title="Enter a Steam ID (STEAM_0:1:23498765), Steam3 ID ([U:1:23498765]), or 17-digit SteamID64.">
                 <div class="text-xs mt-2" id="steam.msg" style="color:var(--danger);display:none"></div>
             </div>
 
diff --git a/web/themes/default/page_admin_comms_add.tpl b/web/themes/default/page_admin_comms_add.tpl
index d9f09655a..2359a9220 100644
--- a/web/themes/default/page_admin_comms_add.tpl
+++ b/web/themes/default/page_admin_comms_add.tpl
@@ -21,13 +21,27 @@
         attribute fires. #1395.
 
     Submission goes through sb.api.call(Actions.CommsAdd) — the same
-    JSON action the legacy theme uses, see
-    web/api/handlers/comms.php. The inline ProcessBan() / changeReason()
-    helpers live in web/pages/admin.comms.php's tail <script> block
-    (legacy convention preserved during the v2.0.0 rollout); this
-    template intentionally keeps the legacy DOM ids (nickname, steam,
-    type, banlength, listReason, txtReason, dreason, *.msg) so those
-    helpers keep working without modification.
+    JSON action the legacy theme uses, see web/api/handlers/comms.php.
+
+    #1420: pre-fix this template wired the submit button via
+    `onclick="ProcessBan();"` against a global `ProcessBan` defined
+    in admin.comms.php's tail script. That global walked the form
+    via legacy MooTools `$('id')` selectors (still working via
+    `sb.js`'s `global.$` shim) and emitted feedback through
+    `sb.message.error`/`sb.message.show` — which paint into
+    `#dialog-placement` / `#dialog-title` / `#dialog-content-text`,
+    DOM ids the v2.0 chrome doesn't render anywhere. Net result:
+    every error (including the "invalid SteamID" branch the
+    reporter hit) silently no-op'd on the chrome side, leaving the
+    operator with no notification. The legacy default theme keeps
+    a copy of this template that still uses ProcessBan(); this
+    file (the v2.0 sbpp2026 chrome twin) ships an inline IIFE that
+    mirrors `page_admin_bans_add.tpl`'s shape: native HTML
+    validation first (browser-native popover for empty / wrong-
+    shape inputs), then `sb.api.call(Actions.CommsAdd)`, then
+    `window.SBPP.showToast` on error envelopes. The PHP-side
+    handler (api_comms_add) is the load-bearing security gate;
+    this client-side shape is UX.
 
     Testability hooks per the issue's "Testability hooks" rule:
       - data-testid="addcomm-<field>" on every input/select.
@@ -63,7 +77,8 @@
                        name="nickname"
                        autocomplete="off"
                        data-testid="addcomm-nickname"
-                       placeholder="Display name as it appeared in-game">
+                       placeholder="Display name as it appeared in-game"
+                       required>
                 <input type="hidden" id="fromsub" value="">
                 <div class="text-xs mt-2" id="nick.msg" style="color:var(--danger);display:none"></div>
             </div>
@@ -77,6 +92,22 @@
                    public servers list's right-click context menu's
                    "Block comms" item — see web/scripts/server-context-menu.js
                    and admin.bans.php's mirror block. #1395 *}
+                {* #1420: native `pattern` attribute mirrors the
+                   client-side regex `page_admin_bans_add.tpl`'s IIFE
+                   carries (`STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}`)
+                   so the browser surfaces a popover for empty / bad-
+                   shape values before our submit handler runs.
+                   `title` is what the browser reads aloud / shows in
+                   the popover when the pattern fails — keep it short
+                   and actionable. The `aria-describedby` ties the
+                   help line below to the input for screen readers.
+                   An `?steam=…` smart-default carrying an IPv4 will
+                   land here too (admin.comms.php's allowlist is
+                   intentionally symmetric with admin.bans.php's so
+                   future menu changes touch one allowlist); the
+                   pattern will reject it and the operator has to fix
+                   the value before submission, which is the right
+                   behaviour (comms can't block by IP). *}
                 <input type="text"
                        class="input font-mono"
                        id="steam"
@@ -84,7 +115,15 @@
                        autocomplete="off"
                        data-testid="addcomm-steam"
                        value="{$prefill_steam}"
-                       placeholder="STEAM_0:1:23498765">
+                       placeholder="STEAM_0:1:23498765"
+                       pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"
+                       title="Enter a Steam ID (STEAM_0:1:23498765), Steam3 ID ([U:1:23498765]), or 17-digit SteamID64."
+                       aria-describedby="addcomm-steam-help"
+                       required>
+                <p class="text-xs text-muted mt-1" id="addcomm-steam-help">
+                    Accepted formats:
+                    <code>STEAM_0:1:N</code>, <code>[U:1:N]</code>, or a 17-digit SteamID64.
+                </p>
                 <div class="text-xs mt-2" id="steam.msg" style="color:var(--danger);display:none"></div>
             </div>
 
@@ -149,11 +188,21 @@
 
             <div>
                 <label class="label" for="listReason">Block reason</label>
+                {* #1420: `required` here is the native gate for "select a
+                   reason" — paired with `value=""` on the placeholder
+                   option, the browser refuses to submit the form when
+                   the operator leaves the dropdown on "-- Select
+                   reason --". `onchange="changeReason(...)"` toggles
+                   the freeform textarea below for the "other" branch
+                   (helper still defined inline in admin.comms.php's
+                   tail script, rewritten to use vanilla DOM for
+                   #1420). *}
                 <select class="select"
                         id="listReason"
                         name="listReason"
                         data-testid="addcomm-reason"
-                        onchange="changeReason(this[this.selectedIndex].value);">
+                        onchange="changeReason(this[this.selectedIndex].value);"
+                        required>
                     <option value="" selected>-- Select reason --</option>
                     <optgroup label="Violation">
                         <option value="Obscene language">Obscene language</option>
@@ -182,14 +231,185 @@
                         class="btn btn--ghost"
                         data-testid="addcomm-back"
                         onclick="history.go(-1);">Back</button>
+                {* #1420: `data-action="addcomm-submit"` replaces the
+                   legacy `onclick="ProcessBan();"` wiring (broken
+                   under the v2.0 chrome — see the file docblock above
+                   for the full rationale). The inline IIFE below
+                   handles the click, native-validates the form, then
+                   dispatches `Actions.CommsAdd` and surfaces errors
+                   through `window.SBPP.showToast`. *}
                 <button type="button"
                         class="btn btn--primary"
                         id="addcomm-submit"
                         data-testid="addcomm-submit"
-                        onclick="ProcessBan();">
+                        data-action="addcomm-submit">
                     <i data-lucide="mic-off"></i> Add block
                 </button>
             </div>
         </form>
     </div>
 {/if}
+{* Inline action wiring — mirrors page_admin_bans_add.tpl's IIFE shape.
+   Pre-#1420 the comms-add submit went through `onclick="ProcessBan();"`
+   into a global defined in admin.comms.php, which emitted feedback
+   through the legacy `sb.message.show` / `sb.message.error` helpers.
+   Those helpers paint into `#dialog-placement` (a v1.x chrome element
+   the v2.0 theme doesn't ship), so every error silently no-op'd —
+   that's the "no notification on invalid SteamID" symptom the
+   reporter hit. The new IIFE:
+     1. Intercepts the click on `[data-action="addcomm-submit"]`.
+     2. Runs the form's native `checkValidity()` — the browser
+        surfaces a popover for empty / pattern-mismatch fields
+        before our handler runs.
+     3. On valid input, dispatches `Actions.CommsAdd` via sb.api.call.
+     4. Surfaces success / error envelopes through `window.SBPP.showToast`
+        (theme.js, the v2.0 chrome's toast surface), with `sb.message`
+        as the graceful-degradation fallback for third-party themes
+        that strip theme.js.
+   The legacy default theme keeps its own copy of this template that
+   still wires `onclick="ProcessBan();"`; this file is sbpp2026-only.
+*}
+{literal}
+<script>
+(function () {
+    'use strict';
+    function api() { return (window.sb && window.sb.api) || null; }
+    function actions() { return window.Actions || null; }
+    function $id(id) { return document.getElementById(id); }
+    function setMsg(id, html) {
+        var el = $id(id);
+        if (!el) return;
+        el.innerHTML = html || '';
+        el.style.display = html ? 'block' : 'none';
+    }
+    function toast(kind, title, body) {
+        var SBPP = window.SBPP;
+        if (SBPP && typeof SBPP.showToast === 'function') {
+            SBPP.showToast({
+                kind: kind === 'red' ? 'error' : kind === 'green' ? 'success' : (kind || 'info'),
+                title: title,
+                body: body || ''
+            });
+            return;
+        }
+        if (window.sb && window.sb.message && window.sb.message[kind]) {
+            window.sb.message[kind](title, body || '');
+        }
+    }
+    /**
+     * Flip the busy / loading state on a triggered action button. Calls
+     * window.SBPP.setBusy when present (theme.js owns the spinner CSS
+     * contract) and falls back to plain `disabled` so third-party themes
+     * that strip theme.js still gate against double-clicks.
+     */
+    function setBusy(btn, busy) {
+        if (!btn) return;
+        var S = window.SBPP;
+        if (S && typeof S.setBusy === 'function') S.setBusy(btn, busy);
+        else btn.disabled = busy === undefined ? true : !!busy;
+    }
+
+    document.addEventListener('click', function (e) {
+        var t = e.target;
+        if (!t || !t.closest) return;
+        var btn = t.closest('[data-action="addcomm-submit"]');
+        if (!btn) return;
+        e.preventDefault();
+
+        var form = $id('addcomm-form');
+        if (!form) return;
+
+        // Native validation gate — the browser surfaces a popover for
+        // empty / pattern-mismatch inputs (steam, nickname, listReason
+        // all carry `required`; steam also carries `pattern`). This is
+        // the load-bearing client-side check per AGENTS.md's "Native
+        // HTML validation" rule; the server-side api_comms_add is the
+        // load-bearing security gate, but a hostile / curl-driven
+        // caller is not the common case. For real users the native
+        // popover is the right first response — it points at the
+        // offending input and surfaces a localised error string the
+        // browser picked for the user's locale.
+        if (typeof form.checkValidity === 'function' && !form.checkValidity()) {
+            if (typeof form.reportValidity === 'function') form.reportValidity();
+            return;
+        }
+
+        // Cross-field check: "other" reason needs a freeform body.
+        // Native HTML can't express this (the textarea is conditional
+        // on the parent select's value), so we mirror what the legacy
+        // ProcessBan() did.
+        var listReason = $id('listReason');
+        var reason = listReason ? listReason.value : '';
+        if (reason === 'other') {
+            var txtReason = $id('txtReason');
+            reason = txtReason ? txtReason.value.trim() : '';
+            if (!reason) {
+                setMsg('reason.msg', 'Please describe the block reason in the textarea.');
+                if (txtReason && typeof txtReason.focus === 'function') txtReason.focus();
+                return;
+            }
+        }
+        setMsg('reason.msg', '');
+
+        var a = api(), A = actions();
+        if (!a || !A) return;
+        setBusy(btn, true);
+        a.call(A.CommsAdd, {
+            nickname: $id('nickname').value,
+            type: Number($id('type').value),
+            steam: $id('steam').value,
+            length: Number($id('banlength').value),
+            reason: reason
+        }).then(function (r) {
+            if (r && r.ok && r.data && r.data.block) {
+                // Success — keep the button busy (matches the legacy
+                // shape) so the operator can't queue a second submit
+                // while the iframe fires rcon at every server.
+                var b = r.data.block;
+                toast('success', 'Block Added',
+                    'The block has been successfully added.');
+                // The iframe is load-bearing — pages/admin.blockit.php
+                // loops the enabled servers and fires `sc_fw_block`
+                // via rcon for each one. Without it the DB row exists
+                // but no live server learns about the gag/mute,
+                // matching the bans/kickit shape one branch above.
+                var iframe = document.createElement('iframe');
+                iframe.id = 'srvkicker';
+                iframe.style.display = 'none';
+                iframe.src = 'pages/admin.blockit.php?check='
+                    + encodeURIComponent(b.steam)
+                    + '&type=' + encodeURIComponent(b.type)
+                    + '&length=' + encodeURIComponent(b.length);
+                document.body.appendChild(iframe);
+                if (r.data.reload) {
+                    setTimeout(function () {
+                        window.location.href = window.location.href.replace(/#\^.*$/, '');
+                    }, 2000);
+                }
+                return;
+            }
+            setBusy(btn, false);
+            if (!r) return;
+            if (r.redirect) return;
+            if (r.ok === false) {
+                var err = (r.error && r.error.message) || 'Unknown error';
+                toast('error', 'Block NOT Added', err);
+                // Mirror the legacy inline message so screen readers
+                // and tests anchored on the per-field `*.msg` div see
+                // the error too.
+                if (r.error && r.error.field === 'steam') setMsg('steam.msg', err);
+                else if (r.error && r.error.field === 'nickname') setMsg('nick.msg', err);
+                else if (r.error && r.error.field === 'reason') setMsg('reason.msg', err);
+                return;
+            }
+            var data = r.data || {};
+            if (data.reload) {
+                setTimeout(function () {
+                    window.location.href = window.location.href.replace(/#\^.*$/, '');
+                }, 2000);
+            }
+        });
+    });
+})();
+</script>
+{/literal}

From 821e81f49a51b8b868d1e8b28385bc923445e253 Mon Sep 17 00:00:00 2001
From: rumblefrog <contact@zhenyangli.me>
Date: Tue, 19 May 2026 18:19:05 -0400
Subject: [PATCH 2/9] fix(comms): close substring-bypass via strict SteamID
 regex (review #1420)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Adversarial-review follow-up to e3a8106d. The implementer's PR fixed
the user-visible "no notification" symptom on the comms-add form by
adding `SteamID::isValidID()` as a server-side gate, but the bundled
helper isn't strict enough to actually close the bypass surface a
curl-driven / third-party-theme / API-client caller can drive past
the browser's `pattern`-mismatch popover. Three concrete bypass
classes the `isValidID()` gate accepts:

  - `STEAM_0:0:` (empty Z) — the library's `\d*` regex accepts zero
    digits AND `toSteam2()` round-trips the input unchanged, so an
    obviously-invalid SteamID lands in `:prefix_admins.authid` /
    `:prefix_bans.authid` / `:prefix_comms.authid`.
  - `asdfSTEAM_0:0:123` (substring-bypass) — the regex is unanchored
    (`STEAM_[0|1]:[0:1]:\d*`, also: `|` inside `[...]` is a literal
    pipe, not alternation), `isValidID()` accepts the substring AND
    `toSteam2()` returns the input verbatim. The corrupt value
    breaks SourceMod admin matching and renders as garbage in the
    drawer / row chrome.
  - `asdf 76561197960265728 garbage` (embedded Steam64) — same
    unanchored substring match AND `toSteam2()` emits a corrupt
    canonical form (`STEAM_0:0:-38280598980132864` — the negative
    Z component is the parser eating surrounding bytes during
    numeric conversion).

Replace the `isValidID()` call in all three handlers
(`api_comms_add`, `api_bans_add`, `api_admins_add`) with the strict
anchored regex the form template's client-side `pattern` attribute
already enforces (`^(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17})$`).
The three handlers now share the regex byte-for-byte so a future
caller (deep-link, JSON client, context-menu handoff) only has to
learn one accepted shape.

`api_admins_add` was an additional gap the implementer didn't touch:
it called `SteamID::toSteam2()` BEFORE any validation, so the same
generic-`Exception`-becomes-500 vector existed there as on the
comms-add and bans-add paths the PR did fix. Tightened in the same
shape (defer `toSteam2()` until after the `preg_match` gate).

Adversarial-review-specific changes:

- `web/api/handlers/comms.php` + `web/api/handlers/bans.php` +
  `web/api/handlers/admins.php`: swap `SteamID::isValidID()` for
  the strict anchored regex; carry an in-file comment block
  documenting why and pointing at the sibling handlers + AGENTS.md.
- `web/api/handlers/admins.php`: defer `SteamID::toSteam2()` until
  AFTER the strict gate (same bug class the implementer fixed on
  the other two handlers).
- `web/tests/api/CommsTest.php` + `web/tests/api/BansTest.php`:
  extend `testAddRejectsInvalidSteamIdShape*` to cover the four
  bypass shapes the strict regex now rejects (`STEAM_0:0:`,
  `asdfSTEAM_0:0:123`, `asdf 76561197960265728 garbage`, `U:1:1`)
  on top of the implementer's three obvious cases. The two test
  files share the same case list so a future regression in either
  handler fails its own test in isolation.
- `web/tests/api/AdminsTest.php`: add `testAddRejectsInvalidSteamIdShape`
  mirroring the bans + comms shape (the implementer didn't touch
  this handler; the test is new with this commit).
- `AGENTS.md`: rewrite the "SteamID inputs" Conventions block to
  document the strict regex + the three concrete bypass shapes +
  the rationale for not relying on `SteamID::isValidID()`. Update
  the matching Anti-patterns entry to reference `preg_match`
  instead of `isValidID()`. Add a new anti-pattern entry
  explicitly calling out the `isValidID()`-alone shape so a future
  agent can't reintroduce the gap by reading the old guidance.
- `web/themes/default/page_admin_comms_add.tpl`: drop two
  outdated "the legacy default theme keeps a copy" comments. The
  panel ships exactly one theme (`web/themes/default/`); the
  comment was misleading and the implementer flagged it as a
  "left alone" concern that was actually a documentation bug.

Tightening `SteamID::isValidID()` itself (anchor the regexes,
replace `[0|1]` with `[01]`, require `\d+`) is the right long-term
fix but it's a third-party-vendored file (`web/includes/SteamID/`)
and a broader audit is needed of its other call sites (every panel
surface that reads SteamID input — install wizard, admin.edit.*,
plugin handshake, …). That's tracked separately; the per-handler
strict `preg_match` is the defence-in-depth that fixes the #1420
bypass surface today without waiting on that audit.

Gates run on this commit (worktree-mounted stack):
- `./sbpp.sh phpstan` — OK
- `./sbpp.sh test` — OK (723 tests, 2904 assertions)
- `./sbpp.sh composer api-contract` — no diff
- `./sbpp.sh ts-check` — clean
- `./sbpp.sh e2e --grep 'comms-add SteamID validation'` — 5 passed
- `./sbpp.sh e2e --grep 'comms|admin-edit-comms' --workers=1` — 45 passed
- `./sbpp.sh e2e --grep 'bans-add|admin-bans|banlist-getfallback' --workers=1` — 11 passed
- `./sbpp.sh e2e --grep 'admin-admins|admins-add' --workers=1` — 20 passed

Refs #1420.
---
 AGENTS.md                                   | 86 ++++++++++++++++-----
 web/api/handlers/admins.php                 | 22 +++++-
 web/api/handlers/bans.php                   | 22 ++++--
 web/api/handlers/comms.php                  | 19 ++++-
 web/tests/api/AdminsTest.php                | 39 ++++++++++
 web/tests/api/BansTest.php                  | 45 ++++++-----
 web/tests/api/CommsTest.php                 | 44 +++++++++--
 web/themes/default/page_admin_comms_add.tpl | 14 ++--
 8 files changed, 229 insertions(+), 62 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index c301fe73b..5dc8374c6 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -692,7 +692,7 @@ of the diff ship together or not at all.
   for xajax (removed).
 - **SteamID inputs**: ALWAYS gate `SteamID::toSteam2($raw)` (or any
   other `SteamID::*` conversion that calls `resolveInputID` internally)
-  with an explicit `SteamID::isValidID($raw)` check first, and throw
+  with an explicit strict-shape `preg_match` check first, and throw
   `ApiError('validation', '…', '<field>')` on the fail branch.
   `SteamID::resolveInputID()` throws a generic `\Exception` for
   unrecognised input shapes; without the gate that exception escapes
@@ -706,19 +706,52 @@ of the diff ship together or not at all.
   if ($rawSteam === '') {
       throw new ApiError('validation', 'You must type a Steam ID or Community ID', 'steam');
   }
-  if (!SteamID::isValidID($rawSteam)) {
+  if (!preg_match('/^(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17})$/', $rawSteam)) {
       throw new ApiError('validation', 'Please enter a valid Steam ID or Community ID', 'steam');
   }
   $steam = SteamID::toSteam2($rawSteam);
   ```
 
-  See `api_comms_add` / `api_bans_add` for the canonical reference
-  shape. The client-side native validation in the corresponding form
-  template (`pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"`) is
-  the UX-first gate that surfaces a browser-native popover BEFORE the
-  IIFE fires `sb.api.call`; the server-side `isValidID` gate is the
-  load-bearing security gate for curl-driven / third-party-theme
-  callers that bypass it.
+  See `api_comms_add` / `api_bans_add` / `api_admins_add` for the
+  canonical reference shape — all three handlers share the same
+  regex byte-for-byte so a future caller (deep-link, JSON client,
+  context-menu handoff) only has to learn one accepted shape. The
+  client-side native validation in the corresponding form template
+  uses the SAME regex via `pattern="(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17})"`
+  (HTML's `pattern` attribute is implicitly anchored `^…$`, so the
+  PHP regex carries explicit `^…$`); the browser-native popover is
+  the UX-first gate that fires BEFORE the IIFE calls `sb.api.call`;
+  the server-side `preg_match` is the load-bearing security gate
+  for curl-driven / third-party-theme callers that bypass it.
+
+  Why NOT `SteamID::isValidID($raw)`: the bundled helper's regexes
+  are unanchored with loose character classes (`STEAM_[0|1]:[0:1]:\d*`
+  — the `|` inside `[...]` is a literal pipe, not alternation, and
+  the missing `^`/`$` anchors leave it as a substring matcher).
+  Consequences:
+   - `'STEAM_0:0:'` (empty Z, the `\d*` accepts zero digits) passes
+     `isValidID` AND round-trips through `toSteam2()` unchanged,
+     storing an invalid SteamID in `:prefix_admins.authid` /
+     `:prefix_bans.authid` / `:prefix_comms.authid`.
+   - `'asdfSTEAM_0:0:123'` (embedded valid-looking suffix) passes
+     `isValidID` AND `toSteam2()` returns the input verbatim
+     (the resolveInputID switch matches the SteamID branch via
+     substring, then re-extracts via the same regex which returns
+     the full input string), corrupting downstream SourceMod admin
+     matching and the panel's per-row UI.
+   - `'asdf 76561197960265728 garbage'` (embedded 17-digit Steam64)
+     passes `isValidID` AND `toSteam2()` emits a corrupt canonical
+     form (`'STEAM_0:0:-38280598980132864'` — the negative Z
+     component is the parser eating the surrounding bytes during
+     numeric conversion).
+
+  The strict regex closes all three bypass classes. Tightening
+  `SteamID::isValidID()` itself is the right long-term fix
+  (anchor the regexes, replace `[0|1]` with `[01]`, require `\d+`)
+  but it's a third-party-vendored file and a broader audit is
+  needed of its other call sites; this is tracked separately. The
+  per-handler `preg_match` is the defence-in-depth that fixes
+  #1420 today without waiting on that audit.
 
 ### CSRF
 
@@ -2959,7 +2992,7 @@ contacting every contributor individually.
   rejected by MariaDB strict mode). See "Database" under Conventions.
 - Calling `SteamID::toSteam2($raw)` (or any other `SteamID::*`
   conversion that calls `resolveInputID` internally) on operator-
-  controlled input WITHOUT an `SteamID::isValidID($raw)` gate first
+  controlled input WITHOUT a strict-shape `preg_match` gate first
   → `resolveInputID` throws a generic `\Exception` for unrecognised
   shapes, and the dispatcher's `Throwable` fallback in `Api::handle`
   wraps it as a generic `server_error` envelope (HTTP 500). The
@@ -2971,15 +3004,30 @@ contacting every contributor individually.
   `sb.message.show` against the v1.x `#dialog-placement` chrome
   shell that the v2.0 theme doesn't render — so the 500 was
   silent (reporter's symptom on #1420: "no notification on
-  invalid steamID"). The fix is the explicit `isValidID` gate
-  documented under "JSON API" in Conventions; landing it at the
-  same time as the form template's native `pattern` attribute is
-  what closes the loop end-to-end (the `pattern` is the UX-first
-  gate; the `isValidID` gate is the security-first gate; both
-  ship together). See `api_comms_add` / `api_bans_add` for the
-  canonical reference shape and `Php82DeprecationsTest`-style
-  regression coverage in `web/tests/api/CommsTest.php::testAddRejectsInvalidSteamIdShape`
-  and `web/tests/api/BansTest.php::testAddRejectsInvalidSteamIdShapeForType0`.
+  invalid steamID"). The fix is the explicit `preg_match` gate
+  documented under "SteamID inputs" in Conventions; landing it at
+  the same time as the form template's native `pattern` attribute
+  is what closes the loop end-to-end (the `pattern` is the UX-first
+  gate; the `preg_match` gate is the security-first gate; both
+  ship together). See `api_comms_add` / `api_bans_add` /
+  `api_admins_add` for the canonical reference shape and
+  regression coverage in `web/tests/api/CommsTest.php::testAddRejectsInvalidSteamIdShape`,
+  `web/tests/api/BansTest.php::testAddRejectsInvalidSteamIdShapeForType0`,
+  and `web/tests/api/AdminsTest.php::testAddRejectsInvalidSteamIdShape`.
+- Relying on `SteamID::isValidID($raw)` alone as the server-side
+  gate (without the paired strict-anchored `preg_match`) → the
+  bundled helper's regexes are unanchored with loose character
+  classes (`STEAM_[0|1]:[0:1]:\d*` — `|` inside `[...]` is a
+  literal pipe, not alternation, and the missing `^`/`$` anchors
+  leave it as a substring matcher), so `'STEAM_0:0:'` (empty Z),
+  `'asdfSTEAM_0:0:123'` (substring-bypass), and
+  `'asdf 76561197960265728 garbage'` (embedded Steam64 that
+  `toSteam2()` corrupts to `'STEAM_0:0:-38280598980132864'`) all
+  pass the library's gate AND get bound into the DB as the
+  "canonical" SteamID. Use the strict `preg_match` from the
+  "SteamID inputs" Conventions block; the per-handler regex is
+  defence-in-depth that doesn't depend on the third-party-
+  vendored library landing an anchored-regex fix first.
 - Editing `install/includes/sql/data.sql` (or `struc.sql`) without a paired
   `web/updater/data/<N>.php` → upgraded installs silently miss the change.
 - WYSIWYG / "rich HTML" editors (TinyMCE, CKEditor, …) for fields stored
diff --git a/web/api/handlers/admins.php b/web/api/handlers/admins.php
index fb6fbe2b3..0fb84a6d8 100644
--- a/web/api/handlers/admins.php
+++ b/web/api/handlers/admins.php
@@ -120,7 +120,15 @@ function api_admins_add(array $params): array
     $mask        = (int)($params['mask'] ?? 0);
     $srvMask     = (string)($params['srv_mask'] ?? '');
     $name        = (string)($params['name'] ?? '');
-    $steam       = SteamID::toSteam2((string)($params['steam'] ?? ''));
+    // #1420 — defer `SteamID::toSteam2()` until AFTER the strict-shape
+    // gate below. Pre-fix this line called `toSteam2()` directly on
+    // the raw input; `resolveInputID()` throws a generic `\Exception`
+    // for unrecognised shapes (`'asdf'`, `'12345'`, … the same
+    // garbage the comms / bans handlers ate) which escaped the
+    // handler and surfaced as a 500. See "JSON API" / "SteamID inputs"
+    // in AGENTS.md for the contract; this file is the third reference
+    // shape after `api_comms_add` and `api_bans_add`.
+    $rawSteam    = trim((string)($params['steam'] ?? ''));
     $email       = (string)($params['email'] ?? '');
     $password    = (string)($params['password']  ?? '');
     $password2   = (string)($params['password2'] ?? '');
@@ -167,12 +175,20 @@ function api_admins_add(array $params): array
     if ($userbank->isNameTaken($name)) {
         throw new ApiError('validation', 'An admin with this name already exists', 'name');
     }
-    if (empty($steam)) {
+    if ($rawSteam === '') {
         throw new ApiError('validation', 'You must type a Steam ID or Community ID for the admin.', 'steam');
     }
-    if (!SteamID::isValidID($steam)) {
+    // #1420 — strict anchored regex mirrors the form template's
+    // client-side `pattern` (HTML's `pattern` is implicitly `^…$`),
+    // so a curl-driven caller can't smuggle embedded-substring
+    // garbage past the gate (`'asdf 76561197960265728 garbage'`
+    // matches `SteamID::isValidID`'s unanchored substring regex and
+    // `toSteam2()` then emits a negative-Z-component canonical form
+    // into `:prefix_admins.authid`).
+    if (!preg_match('/^(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17})$/', $rawSteam)) {
         throw new ApiError('validation', 'Please enter a valid Steam ID or Community ID.', 'steam');
     }
+    $steam = SteamID::toSteam2($rawSteam);
     if ($userbank->isSteamIDTaken($steam)) {
         $taken = '';
         foreach ($userbank->GetAllAdmins() as $a) {
diff --git a/web/api/handlers/bans.php b/web/api/handlers/bans.php
index ceb683bb2..4be35cde4 100644
--- a/web/api/handlers/bans.php
+++ b/web/api/handlers/bans.php
@@ -56,15 +56,27 @@ function api_bans_add(array $params): array
     // that escaped to the dispatcher's catch-all and produced a 500
     // with body "An unexpected error occurred" instead of the
     // structured `validation`-coded error the chrome's toast can
-    // render. Comms-add carries the same fix; both halves
-    // intentionally mirror so a SteamID a hostile / curl-driven
-    // caller smuggles past the client-side regex still surfaces a
-    // friendly toast instead of a 500.
+    // render.
+    //
+    // The bundled `SteamID::isValidID()` is NOT a sufficient gate on
+    // its own — its regexes are unanchored with loose character
+    // classes, so an embedded-SteamID-with-garbage like
+    // `'asdf 76561197960265728 garbage'` matches the substring AND
+    // `toSteam2()` emits a corrupt canonical form (negative Z
+    // component from the parser eating surrounding bytes) which then
+    // gets bound into `:prefix_bans.authid`. The strict regex below
+    // mirrors the form template's client-side `pattern` attribute
+    // byte-for-byte (HTML's `pattern` is implicitly anchored `^…$`),
+    // so a curl-driven caller can't smuggle garbage past the gate
+    // that the browser's pattern-mismatch popover already rejects
+    // for form users. Comms-add and admin-add carry the same regex;
+    // all three handlers stay in lockstep so a future menu / deep-
+    // link / API client only has to learn one accepted shape.
     if ($banType === BanType::Steam) {
         if ($rawSteam === '') {
             throw new ApiError('validation', 'You must type a Steam ID or Community ID', 'steam');
         }
-        if (!SteamID::isValidID($rawSteam)) {
+        if (!preg_match('/^(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17})$/', $rawSteam)) {
             throw new ApiError('validation', 'Please enter a valid Steam ID or Community ID', 'steam');
         }
     }
diff --git a/web/api/handlers/comms.php b/web/api/handlers/comms.php
index be6d326b8..d0f1e9ea5 100644
--- a/web/api/handlers/comms.php
+++ b/web/api/handlers/comms.php
@@ -39,11 +39,26 @@ function api_comms_add(array $params): array
     // a global that no longer exists post-#1123 D1), so the API
     // round-trip never happened — fixing the front-end alone would
     // have exposed the same 500 the bans-add path also carries.
-    // Mirror `api_bans_add`'s validation order in both handlers.
+    //
+    // The bundled `SteamID::isValidID()` is NOT a sufficient gate on
+    // its own — its regexes are unanchored with loose character
+    // classes (`STEAM_[0|1]:[0:1]:\d*` — note the `|` inside `[...]`
+    // is a literal pipe, not alternation, and the missing `^`/`$`
+    // anchors mean an embedded-SteamID-with-garbage like
+    // `'asdf 76561197960265728 garbage'` matches the substring AND
+    // `toSteam2()` then emits `'STEAM_0:0:-38280598980132864'` (the
+    // negative Z component is the parser eating the surrounding
+    // bytes, the result gets bound into the DB). The strict regex
+    // below mirrors the form template's client-side `pattern`
+    // attribute byte-for-byte (HTML's `pattern` is implicitly
+    // anchored `^…$`), so a curl-driven caller can't smuggle garbage
+    // past the gate that the browser's pattern-mismatch popover
+    // already rejects for form users. Mirror `api_bans_add` /
+    // `api_admins_add` — all three handlers use the same regex.
     if ($rawSteam === '') {
         throw new ApiError('validation', 'You must type a Steam ID or Community ID', 'steam');
     }
-    if (!SteamID::isValidID($rawSteam)) {
+    if (!preg_match('/^(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17})$/', $rawSteam)) {
         throw new ApiError('validation', 'Please enter a valid Steam ID or Community ID', 'steam');
     }
     $steam = SteamID::toSteam2($rawSteam);
diff --git a/web/tests/api/AdminsTest.php b/web/tests/api/AdminsTest.php
index 3427151a8..74559d5d3 100644
--- a/web/tests/api/AdminsTest.php
+++ b/web/tests/api/AdminsTest.php
@@ -75,6 +75,45 @@ public function testAddValidatesSteamId(): void
         $this->assertSame('steam', $env['error']['field']);
     }
 
+    /**
+     * #1420 — third reference shape after `api_comms_add` and
+     * `api_bans_add`. Pre-fix `api_admins_add` called
+     * `SteamID::toSteam2()` directly on the raw inbound param;
+     * `resolveInputID()` throws a bare `\Exception` for unrecognised
+     * shapes, which the dispatcher caught and surfaced as a 500.
+     * Worse, the unanchored / loose-character-class regexes in
+     * `SteamID::isValidID()` would silently accept embedded-substring
+     * shapes (`'asdfSTEAM_0:0:123'`), letting corrupt bytes land in
+     * `:prefix_admins.authid` where they're load-bearing for SourceMod
+     * admin matching. The strict anchored regex mirrors the form
+     * template's client-side `pattern` so a curl-driven caller can't
+     * smuggle garbage past the gate; mirrors BansTest /
+     * CommsTest coverage so the three handlers stay in lockstep.
+     */
+    public function testAddRejectsInvalidSteamIdShape(): void
+    {
+        $this->loginAsAdmin();
+        foreach (
+            [
+                'asdf',                            // pure non-SteamID text
+                '12345',                           // short numeric
+                '7656119xxxxxxxxxx',               // 17 chars, not 17 digits
+                'STEAM_0:0:',                      // empty Z; pre-fix `\d*` accepted
+                'asdfSTEAM_0:0:123',               // substring-bypass (unanchored)
+                'asdf 76561197960265728 garbage',  // embedded Steam64 — pre-fix corrupted
+                'U:1:1',                           // bracketless Steam3 — form pattern requires brackets
+            ] as $badSteam
+        ) {
+            $env = $this->api('admins.add', $this->adminParams(['steam' => $badSteam]));
+            $this->assertEnvelopeError($env, 'validation');
+            $this->assertSame(
+                'steam',
+                $env['error']['field'],
+                sprintf('expected error.field=steam for steam=%s', var_export($badSteam, true)),
+            );
+        }
+    }
+
     public function testAddRejectsTakenSteamId(): void
     {
         $this->loginAsAdmin();
diff --git a/web/tests/api/BansTest.php b/web/tests/api/BansTest.php
index 228ac6f49..344cd4931 100644
--- a/web/tests/api/BansTest.php
+++ b/web/tests/api/BansTest.php
@@ -87,27 +87,36 @@ public function testAddValidationMissingSteamForType0(): void
      * in the catch-all `Throwable` arm and surfaced a 500 with
      * body "An unexpected error occurred", which the chrome read
      * as a server error rather than a per-field validation
-     * failure. The fix validates the SteamID shape BEFORE handing
-     * it to `SteamID::toSteam2()`. Comms-add carries the same
-     * fix; both halves intentionally mirror so a SteamID a
-     * hostile / curl-driven caller smuggles past the client-side
-     * regex still surfaces a friendly structured error envelope
-     * instead of a 500.
+     * failure. The fix validates the SteamID shape against a
+     * strict anchored regex BEFORE handing it to
+     * `SteamID::toSteam2()`. `SteamID::isValidID()` on its own is
+     * not a sufficient gate — its regexes are unanchored with
+     * loose character classes, so a substring containing a
+     * valid-looking SteamID passes (and `toSteam2()` then either
+     * round-trips the corrupt input verbatim or emits a
+     * negative-Z-component canonical form into the DB). Comms-add
+     * and admins-add carry the same regex; all three handlers
+     * stay in lockstep so a future caller only has to learn one
+     * accepted shape.
      */
     public function testAddRejectsInvalidSteamIdShapeForType0(): void
     {
-        $this->loginAsAdmin();
-        // Three shape classes the pre-fix `SteamID::resolveInputID`
-        // throws on (each fails every isValidID regex):
-        //   - `asdf` — the reporter's example; pure non-SteamID text.
-        //   - `12345` — short numeric (not 17 digits).
-        //   - `7656119xxxxxxxxxx` — 17 chars but not 17 digits.
-        // `'STEAM_0:0:'` is intentionally NOT tested: it passes
-        // `isValidID` (the regex's `\d*` accepts zero digits) and
-        // round-trips through `toSteam2` unchanged. Catching it
-        // would require tightening `SteamID::isValidID`, which is
-        // out of scope for #1420.
-        foreach (['asdf', '12345', '7656119xxxxxxxxxx'] as $badSteam) {
+        // Mirrors CommsTest::testAddRejectsInvalidSteamIdShape — every
+        // case the strict regex rejects must be rejected here too so
+        // the three handlers (api_bans_add / api_comms_add /
+        // api_admins_add) stay in lockstep.
+        $this->loginAsAdmin();
+        foreach (
+            [
+                'asdf',                            // reporter's example
+                '12345',                           // short numeric
+                '7656119xxxxxxxxxx',               // 17 chars, not 17 digits
+                'STEAM_0:0:',                      // empty Z; pre-fix `\d*` accepted
+                'asdfSTEAM_0:0:123',               // substring-bypass (unanchored)
+                'asdf 76561197960265728 garbage',  // embedded Steam64 — pre-fix corrupted
+                'U:1:1',                           // bracketless Steam3 — form pattern requires brackets
+            ] as $badSteam
+        ) {
             $env = $this->api('bans.add', [
                 'nickname' => 'X',
                 'type'     => 0,
diff --git a/web/tests/api/CommsTest.php b/web/tests/api/CommsTest.php
index e2a6212f1..c066378e1 100644
--- a/web/tests/api/CommsTest.php
+++ b/web/tests/api/CommsTest.php
@@ -144,12 +144,22 @@ public function testAddValidatesSteamIdRequired(): void
      * `dialog-placement` element silently no-op'd against the v2.0
      * chrome (the reporter's "no notification" symptom).
      *
-     * Post-fix `api_comms_add` validates the SteamID shape BEFORE
-     * handing it to `SteamID::toSteam2()` so a malformed value
-     * surfaces a structured `validation`-coded error with
-     * `field=steam`, which the chrome can render as a per-field
-     * toast / inline message. Mirrors the bans-add fix in the
-     * same PR.
+     * Post-fix `api_comms_add` validates the SteamID shape against
+     * a strict anchored regex BEFORE handing it to
+     * `SteamID::toSteam2()`. `SteamID::isValidID()` on its own is
+     * NOT a sufficient gate — its regexes are unanchored with loose
+     * character classes (`STEAM_[0|1]:[0:1]:\d*` — note `|` inside
+     * `[...]` is a literal pipe, not alternation), so a substring
+     * match against an embedded valid-looking SteamID passes the
+     * library's gate AND `toSteam2()` then either binds corrupt
+     * bytes into the DB (`'asdfSTEAM_0:0:123'` round-trips
+     * unchanged) or emits a negative-Z-component canonical form
+     * (`'asdf 76561197960265728 garbage'` → `'STEAM_0:0:-38280598980132864'`).
+     * The strict regex (mirroring the form template's client-side
+     * `pattern` attribute byte-for-byte, HTML's `pattern` is
+     * implicitly anchored `^…$`) closes the bypass for
+     * curl-driven / third-party-theme callers. Mirrors the
+     * bans-add and admins-add fixes in the same PR.
      *
      * Cases:
      *   - `'asdf'` — the reporter's example; doesn't match any
@@ -158,11 +168,31 @@ public function testAddValidatesSteamIdRequired(): void
      *     fall through `resolveInputID`'s shape table.
      *   - `'   '` — whitespace-only; trims down to '' and surfaces
      *     the "must type" branch, but still as a structured error.
+     *   - `'STEAM_0:0:'` — the library's loose `\d*` regex accepts
+     *     zero digits; the strict `\d+` regex rejects.
+     *   - `'asdfSTEAM_0:0:123'` — substring-bypass case; the
+     *     library's unanchored regex would silently accept and
+     *     toSteam2 returns the input verbatim.
+     *   - `'asdf 76561197960265728 garbage'` — Steam64 surrounded
+     *     by junk; toSteam2 would emit a negative-Z corrupt form.
+     *   - `'U:1:1'` — bracketless Steam3; the library accepts but
+     *     the form's `pattern` requires brackets, so the strict
+     *     gate rejects for symmetry.
      */
     public function testAddRejectsInvalidSteamIdShape(): void
     {
         $this->loginAsAdmin();
-        foreach (['asdf', '12345', '   '] as $badSteam) {
+        foreach (
+            [
+                'asdf',
+                '12345',
+                '   ',
+                'STEAM_0:0:',
+                'asdfSTEAM_0:0:123',
+                'asdf 76561197960265728 garbage',
+                'U:1:1',
+            ] as $badSteam
+        ) {
             $env = $this->api('comms.add', [
                 'nickname' => 'BadShape',
                 'type'     => 1,
diff --git a/web/themes/default/page_admin_comms_add.tpl b/web/themes/default/page_admin_comms_add.tpl
index 2359a9220..7942bac49 100644
--- a/web/themes/default/page_admin_comms_add.tpl
+++ b/web/themes/default/page_admin_comms_add.tpl
@@ -33,15 +33,15 @@
     DOM ids the v2.0 chrome doesn't render anywhere. Net result:
     every error (including the "invalid SteamID" branch the
     reporter hit) silently no-op'd on the chrome side, leaving the
-    operator with no notification. The legacy default theme keeps
-    a copy of this template that still uses ProcessBan(); this
-    file (the v2.0 sbpp2026 chrome twin) ships an inline IIFE that
-    mirrors `page_admin_bans_add.tpl`'s shape: native HTML
+    operator with no notification. The replacement inline IIFE
+    below mirrors `page_admin_bans_add.tpl`'s shape: native HTML
     validation first (browser-native popover for empty / wrong-
     shape inputs), then `sb.api.call(Actions.CommsAdd)`, then
     `window.SBPP.showToast` on error envelopes. The PHP-side
-    handler (api_comms_add) is the load-bearing security gate;
-    this client-side shape is UX.
+    handler (api_comms_add) is the load-bearing security gate
+    (`preg_match` on the anchored regex BEFORE `SteamID::toSteam2`,
+    see web/api/handlers/comms.php for the canonical shape); this
+    client-side shape is UX.
 
     Testability hooks per the issue's "Testability hooks" rule:
       - data-testid="addcomm-<field>" on every input/select.
@@ -266,8 +266,6 @@
         (theme.js, the v2.0 chrome's toast surface), with `sb.message`
         as the graceful-degradation fallback for third-party themes
         that strip theme.js.
-   The legacy default theme keeps its own copy of this template that
-   still wires `onclick="ProcessBan();"`; this file is sbpp2026-only.
 *}
 {literal}
 <script>

From fd81750b0c5fd095deb7d8a2470336c21d8087f7 Mon Sep 17 00:00:00 2001
From: rumblefrog <contact@zhenyangli.me>
Date: Tue, 19 May 2026 18:50:22 -0400
Subject: [PATCH 3/9] fix(steamid): tighten SteamID library regexes (review
 #1420 follow-up #1)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Lift the strict-shape contract from the per-handler `preg_match` gates
in `api_comms_add` / `api_bans_add` / `api_admins_add` into
`SteamID::isValidID()` itself so every other caller of the library
(install wizard search box, `admin.edit.*` page handlers, plugin
handshake paths, the protest / submit / banlist search surfaces) gets
the same defence by construction.

Pre-fix `SteamID::isValidID()` and `SteamID::resolveInputID()` carried
parallel `switch (true)` regex tables with `STEAM_[0|1]:[0:1]:\d*` /
`\[U:1:\d*\]` / `U:1:\d*` / `\d{17}` shapes. Three structural bugs
per pattern:
  - missing `^…$` anchors → substring matchers. `asdfSTEAM_0:0:123`,
    `asdf 76561197960265728 garbage`, embedded-Steam2 / embedded-Steam3
    payloads all passed `isValidID` AND `resolveInputID()` resolved
    them — `toSteam2()` then either round-tripped the garbage
    verbatim into the DB (`:prefix_bans.authid` / `:prefix_admins.authid`
    / `:prefix_comms.authid`) or emitted corrupt canonical forms
    (`STEAM_0:0:-38280598980132864` from the embedded-Steam64 shape;
    the negative Z comes from `(int) 'asdf 76561197960265728 garbage'`
    being 0, the parser eating the surrounding bytes during numeric
    conversion);
  - `[0|1]` / `[0:1]` character classes accepted `|` and `:` as
    literal chars (`STEAM_|:0:0`, `STEAM_0::0` etc.). The `|` inside
    a character class is NOT alternation; pre-fix the rule accepted
    the literal pipe;
  - `\d*` quantifier accepted zero digits. `STEAM_0:0:` round-tripped
    through `toSteam2()` unchanged and landed in the DB as an invalid
    SteamID — the #1420 bug-on-disk.

Post-fix the four accepted shapes live in a single
`SteamID::ID_PATTERNS` constant the two surfaces share, so a future
edit to one cannot drift from the other. Patterns:

    ^STEAM_[01]:[01]:\d+$  Steam2 (universe-0 + universe-1 both accepted)
    ^\[U:1:\d+\]$          Steam3 bracketed
    ^U:1:\d+$              Steam3 bracketless (preserved compat — the
                           conversion methods `trim($_, '[]')` so this
                           shape converts correctly; rejecting it would
                           break wild-typed-input callers)
    ^\d{17}$               Steam64

Every pattern carries the `D` modifier — without it PHP's `$` matches
end-of-string OR a final `\n`, so `STEAM_0:0:1\n` slips through the
`^…$` anchor pair AND lands as a trailing-newline string in the DB.
That sibling bypass was discovered by the new test suite; pin it
explicitly so a future agent dropping the modifier ("simplifies to
strict regex shape") fails loudly.

The library is in-tree under `web/includes/SteamID/SteamID.php`
(NOT in `vendor/`), so the project has already taken ownership of
it — the "third-party-vendored" framing in the original #1420
convention block was incorrect. Updated AGENTS.md accordingly:
the Conventions block under "SteamID inputs" now reflects that
the library is strict AND the per-handler `preg_match` is true
defence-in-depth (both layers agree on the accepted shape; keep
both). The Anti-patterns block grew a paired entry against
loosening `ID_PATTERNS` AND a paired entry against removing the
per-handler gate "now that the library is strict".

Other caller audit (rg 'SteamID::' web/ game/ docker/):
  - `web/install/pages/page.5.php` validates with its own regex,
    does not call `SteamID::isValidID()`. Unaffected.
  - SourceMod plugins (`game/addons/sourcemod/scripting/*.sp`)
    extract SteamIDs from SourceMod's `GetClientAuthId` and bind
    them directly to MySQL queries; the PHP `SteamID` library is
    panel-side only. Unaffected.
  - `web/includes/Auth/Handler/SteamAuthHandler.php` extracts a
    Steam64 from the OpenID URL with its own
    `/^https?:\/\/steamcommunity\.com\/openid\/id\/(7[0-9]{15,25})$/`
    regex BEFORE calling `SteamID::toSteam3(...)` — already strict;
    the new tightening cannot reject any input the OpenID
    response leg legitimately produces (Steam returns 17-char IDs
    exclusively).
  - `web/pages/page.{submit,protest,banlist,commslist}.php` and the
    `:prefix_*` audit-log SteamID-search paths call
    `SteamID::isValidID()` / `SteamID::toSteam2()` directly. With
    the library tightening, every one of these gets the same
    defence-in-depth shape — Follow-up #2 (separate commit) covers
    the page handlers that ALSO call `toSteam2()` upstream of any
    validation (admin.edit.{ban,comms,admindetails}), where the
    uncaught-Exception → 500 page render bug class needs a different
    fix shape than the JSON `ApiError(...)` envelope.

Test coverage (`web/tests/integration/SteamIDValidationTest.php`,
104 tests / 183 assertions):
  - 13 valid-input cases (every Steam2 / Steam3 / Steam3-bracketless
    / Steam64 shape the library has always converted correctly).
  - 30 invalid-input cases (the three #1420 bypasses, the loose
    character-class variants, empty / whitespace, bracket mismatches,
    Steam64 length mismatches, pure garbage, embedded valid IDs of
    every shape, the newline-bypass sibling).
  - Byte-for-byte symmetry assertions (`isValidID()` and
    `resolveInputID()` agree on every accepted AND every rejected
    input — pre-fix the two surfaces drifted and the asymmetry was
    the underlying bug class).
  - `ID_PATTERNS` introspection (the constant exists, contains four
    entries, every regex is `^…$` anchored with the `D` modifier —
    pin the structural contract so future edits cannot silently
    loosen).
  - PHP-8.x int-input deprecation guard (`SteamID::isValidID(76561197960287930)`
    via int — pre-fix `preg_match` against an int subject emitted
    `Deprecated: Passing null to parameter…`; the cast-at-entry-point
    silences it).

The per-handler `preg_match` gates in the three JSON `*_add` handlers
stay as-is — both layers agree on the accepted shape and both ship.
The plugin / install / SteamAuthHandler / page-handler surfaces all
acquire the new defence by virtue of the library tightening; no other
edits needed for the library-side fix.

Refs #1420
---
 AGENTS.md                                     | 105 ++++--
 web/includes/SteamID/SteamID.php              | 122 +++++--
 .../integration/SteamIDValidationTest.php     | 345 ++++++++++++++++++
 3 files changed, 512 insertions(+), 60 deletions(-)
 create mode 100644 web/tests/integration/SteamIDValidationTest.php

diff --git a/AGENTS.md b/AGENTS.md
index 5dc8374c6..171d8a3f7 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -724,34 +724,48 @@ of the diff ship together or not at all.
   the server-side `preg_match` is the load-bearing security gate
   for curl-driven / third-party-theme callers that bypass it.
 
-  Why NOT `SteamID::isValidID($raw)`: the bundled helper's regexes
-  are unanchored with loose character classes (`STEAM_[0|1]:[0:1]:\d*`
-  — the `|` inside `[...]` is a literal pipe, not alternation, and
-  the missing `^`/`$` anchors leave it as a substring matcher).
-  Consequences:
-   - `'STEAM_0:0:'` (empty Z, the `\d*` accepts zero digits) passes
-     `isValidID` AND round-trips through `toSteam2()` unchanged,
-     storing an invalid SteamID in `:prefix_admins.authid` /
-     `:prefix_bans.authid` / `:prefix_comms.authid`.
-   - `'asdfSTEAM_0:0:123'` (embedded valid-looking suffix) passes
-     `isValidID` AND `toSteam2()` returns the input verbatim
-     (the resolveInputID switch matches the SteamID branch via
-     substring, then re-extracts via the same regex which returns
+  The pre-#1420 `SteamID::isValidID($raw)` was structurally unsafe
+  — the bundled helper's regexes were unanchored with loose character
+  classes (`STEAM_[0|1]:[0:1]:\d*` — the `|` inside `[...]` is a
+  literal pipe, not alternation, the missing `^`/`$` anchors left
+  them as substring matchers, and `\d*` accepted zero digits).
+  Three concrete bypass shapes the loose gate accepted:
+   - `'STEAM_0:0:'` (empty Z) passed `isValidID` AND round-tripped
+     through `toSteam2()` unchanged, storing an invalid SteamID in
+     `:prefix_admins.authid` / `:prefix_bans.authid` /
+     `:prefix_comms.authid`.
+   - `'asdfSTEAM_0:0:123'` (substring-embedded suffix) passed
+     `isValidID` AND `toSteam2()` returned the input verbatim
+     (the resolveInputID switch matched the SteamID branch via
+     substring, then re-extracted via the same regex which returned
      the full input string), corrupting downstream SourceMod admin
      matching and the panel's per-row UI.
    - `'asdf 76561197960265728 garbage'` (embedded 17-digit Steam64)
-     passes `isValidID` AND `toSteam2()` emits a corrupt canonical
-     form (`'STEAM_0:0:-38280598980132864'` — the negative Z
-     component is the parser eating the surrounding bytes during
-     numeric conversion).
-
-  The strict regex closes all three bypass classes. Tightening
-  `SteamID::isValidID()` itself is the right long-term fix
-  (anchor the regexes, replace `[0|1]` with `[01]`, require `\d+`)
-  but it's a third-party-vendored file and a broader audit is
-  needed of its other call sites; this is tracked separately. The
-  per-handler `preg_match` is the defence-in-depth that fixes
-  #1420 today without waiting on that audit.
+     passed `isValidID` AND `toSteam2()` emitted a corrupt canonical
+     form (`'STEAM_0:0:-38280598980132864'` — negative Z from the
+     parser eating the surrounding bytes during numeric conversion).
+
+  The library tightening landed in #1423 follow-up #1: a shared
+  `SteamID::ID_PATTERNS` constant carries the four accepted shapes
+  with `^…$` anchors, the `D` modifier (strictly anchors `$` to
+  end-of-string, closing the `STEAM_0:0:1\n` newline-bypass
+  sibling), `[01]` character classes, and `\d+` quantifiers; both
+  `isValidID()` and `resolveInputID()` consume the same table so
+  they cannot drift. The per-handler `preg_match` documented above
+  is now true defence-in-depth (the two layers agree on the
+  accepted shape) — KEEP both. Reaching for `SteamID::isValidID()`
+  alone as the server-side gate is fine after the library fix; the
+  per-handler regex stays in `api_comms_add` / `api_bans_add` /
+  `api_admins_add` as the load-bearing structural-shape contract
+  (the JSON wire is hostile-input-shaped; the library is one
+  refactor away from someone "simplifying" the shared table back
+  to a loose form, and the defense-in-depth means that refactor
+  doesn't re-open the bypass class). Regression coverage for both
+  layers: `web/tests/integration/SteamIDValidationTest.php` pins
+  the library's shape contract (every bypass above is asserted as
+  rejected); `web/tests/api/CommsTest.php::testAddRejectsInvalidSteamIdShape`
+  + `BansTest.php` + `AdminsTest.php` pin the handler-side
+  `ApiError('validation', …, 'steam')` envelope.
 
 ### CSRF
 
@@ -3014,20 +3028,33 @@ contacting every contributor individually.
   regression coverage in `web/tests/api/CommsTest.php::testAddRejectsInvalidSteamIdShape`,
   `web/tests/api/BansTest.php::testAddRejectsInvalidSteamIdShapeForType0`,
   and `web/tests/api/AdminsTest.php::testAddRejectsInvalidSteamIdShape`.
-- Relying on `SteamID::isValidID($raw)` alone as the server-side
-  gate (without the paired strict-anchored `preg_match`) → the
-  bundled helper's regexes are unanchored with loose character
-  classes (`STEAM_[0|1]:[0:1]:\d*` — `|` inside `[...]` is a
-  literal pipe, not alternation, and the missing `^`/`$` anchors
-  leave it as a substring matcher), so `'STEAM_0:0:'` (empty Z),
-  `'asdfSTEAM_0:0:123'` (substring-bypass), and
-  `'asdf 76561197960265728 garbage'` (embedded Steam64 that
-  `toSteam2()` corrupts to `'STEAM_0:0:-38280598980132864'`) all
-  pass the library's gate AND get bound into the DB as the
-  "canonical" SteamID. Use the strict `preg_match` from the
-  "SteamID inputs" Conventions block; the per-handler regex is
-  defence-in-depth that doesn't depend on the third-party-
-  vendored library landing an anchored-regex fix first.
+- Loosening `SteamID::ID_PATTERNS` (the shared `[regex, format]`
+  table consumed by both `isValidID()` and `resolveInputID()`) —
+  e.g. dropping the `^…$` anchors, dropping the `D` modifier,
+  using `[0|1]` instead of `[01]` (`|` inside `[…]` is a literal
+  pipe, not alternation), or using `\d*` instead of `\d+`. The
+  pre-#1420 shape carried every one of those bugs and the three
+  concrete bypasses are documented under "SteamID inputs" in
+  Conventions (`'STEAM_0:0:'` empty Z, `'asdfSTEAM_0:0:123'`
+  substring-bypass, `'asdf 76561197960265728 garbage'` embedded
+  Steam64 → `'STEAM_0:0:-38280598980132864'` on `toSteam2()`). The
+  shared-table shape is the single source of truth — both
+  surfaces consume it so they cannot drift. A future refactor
+  that "simplifies" the table back into per-method `switch (true)`
+  blocks re-opens the asymmetry bug-class. Regression guard:
+  `web/tests/integration/SteamIDValidationTest.php::testIdPatternsConstantIsTheSourceOfTruth`
+  introspects the constant and asserts every regex stays
+  `^…$/…D…` shaped.
+- Removing the per-handler strict `preg_match` from
+  `api_comms_add` / `api_bans_add` / `api_admins_add` "now that
+  `SteamID::isValidID()` is strict too" → both layers ship and
+  agree by design (the "defence-in-depth" contract under "SteamID
+  inputs"). The library is one refactor away from someone
+  loosening `ID_PATTERNS` back to a substring matcher AND the
+  per-handler gate exists so that refactor doesn't silently
+  re-open the bypass class. Regression coverage for the handler
+  half: `web/tests/api/CommsTest.php::testAddRejectsInvalidSteamIdShape`
+  (+ sibling tests in `BansTest.php` / `AdminsTest.php`).
 - Editing `install/includes/sql/data.sql` (or `struc.sql`) without a paired
   `web/updater/data/<N>.php` → upgraded installs silently miss the change.
 - WYSIWYG / "rich HTML" editors (TinyMCE, CKEditor, …) for fields stored
diff --git a/web/includes/SteamID/SteamID.php b/web/includes/SteamID/SteamID.php
index 355447407..0c2179cae 100644
--- a/web/includes/SteamID/SteamID.php
+++ b/web/includes/SteamID/SteamID.php
@@ -83,6 +83,63 @@ private static function to($format, $steamid)
         };
     }
 
+    /**
+     * Strict-shape patterns the library accepts as recognisable SteamID
+     * inputs. Mirrored across `resolveInputID()` and `isValidID()` so
+     * the two surfaces agree byte-for-byte on the accepted shape — pre-#1420
+     * the two regex tables had subtly different shapes (`isValidID`'s
+     * unanchored substring matcher accepted strings `resolveInputID`
+     * silently corrupted on conversion, opening the embedded-Steam64
+     * bypass the strict per-handler `preg_match` shipped by the JSON
+     * handlers had to paper over). Single source of truth now.
+     *
+     * Each entry is `[regex, format-tag]`. Order matters for the
+     * bracketed-vs-bracketless Steam3 disambiguation (`[U:1:N]` matches
+     * both regexes after the brackets are stripped, so the bracketed
+     * form must be tried FIRST — `resolveInputID()` returns the FIRST
+     * match and stops; `isValidID()` is order-insensitive because it
+     * only asks "does ANY pattern match").
+     *
+     * The shapes:
+     *   - `^STEAM_[01]:[01]:\d+$` — Steam2. Universe digit 0 or 1
+     *     (TF2/L4D and similar Source titles emit `STEAM_1` for the
+     *     same accounts other Source titles emit `STEAM_0` for — see
+     *     `toSearchPattern()` for the universe-agnostic SQL fallback
+     *     that defends #1128 / #1130); Y digit is 0 or 1; Z digit is
+     *     at least one digit (the empty-Z `STEAM_0:0:` shape was the
+     *     #1420 bug-on-disk).
+     *   - `^\[U:1:\d+\]$` — Steam3 bracketed.
+     *   - `^U:1:\d+$` — Steam3 bracketless. Preserved because the
+     *     conversion methods (`Steam3toSteam2`, `Steam3toSteam64`)
+     *     `trim($steamid, '[]')` first, so the bracketless form
+     *     converts correctly; rejecting it here would break callers
+     *     that paste a Steam3 ID without brackets (the wild typed-input
+     *     case, not a documented panel surface).
+     *   - `^\d{17}$` — Steam64. Exactly 17 digits. The valid Steam64
+     *     range for individual accounts starts at 76561197960265728
+     *     (account ID 0) and grows ~linearly with account creations;
+     *     this regex doesn't range-check (would require a paired
+     *     migration of any caller that legitimately passes a sub-base
+     *     Steam64 from a non-individual ID type), so callers needing
+     *     "is this a plausible user Steam64" should layer a starts-with
+     *     check on top. Tracked as a follow-up in PR #1423.
+     *
+     * The `D` modifier on every pattern is load-bearing: without it
+     * PHP's `$` matches end-of-string OR a final `\n`, so
+     * `STEAM_0:0:1\n` would slip through the gate AND `resolveInputID()`
+     * would then return `'Steam2'` for it AND the `$from === $format`
+     * branch in `to()` would return the trailing-newline string
+     * verbatim into the DB. The `D` modifier strictly anchors `$` to
+     * end-of-string only, closing the newline-bypass sibling of the
+     * `^…$` substring-bypass class of #1420.
+     */
+    private const ID_PATTERNS = [
+        ['/^STEAM_[01]:[01]:\d+$/D', 'Steam2'],
+        ['/^\[U:1:\d+\]$/D',         'Steam3'],
+        ['/^U:1:\d+$/D',             'Steam3'],
+        ['/^\d{17}$/D',              'Steam64'],
+    ];
+
     /**
      * @param  $steamid
      * @return string
@@ -90,35 +147,58 @@ private static function to($format, $steamid)
      */
     private static function resolveInputID($steamid)
     {
-        switch (true) {
-            case preg_match("/STEAM_[0|1]:[0:1]:\d*/", $steamid):
-                return 'Steam2';
-            case preg_match("/\[U:1:\d*\]/", $steamid):
-                return 'Steam3';
-            case preg_match("/U:1:\d*/", $steamid):
-                return 'Steam3';
-            case preg_match("/\d{17}/", $steamid):
-                return 'Steam64';
-            default:
-                throw new Exception("Invalid SteamID input!");
+        // PHP 8.x deprecates passing non-string to preg_match's `string
+        // $subject` arg. `to($format, $steamid)` accepts both int and
+        // string Steam64 inputs (see SteamIdConversionTest's int-shape
+        // round-trips), so the int case reaches here. Cast at the entry
+        // point so the rest of the function works on a known string and
+        // we don't ship a stack of `Deprecated: preg_match(): Passing
+        // null to parameter…` warnings on int callers.
+        $s = (string) $steamid;
+        foreach (self::ID_PATTERNS as [$pattern, $format]) {
+            if (preg_match($pattern, $s) === 1) {
+                return $format;
+            }
         }
+        throw new Exception("Invalid SteamID input!");
     }
 
     /**
-     * @param  $steamid
-     * @return bool
+     * Strict-shape SteamID validator. Returns `true` for inputs the
+     * library can convert correctly through `toSteam2()` / `toSteam3()`
+     * / `toSteam64()`, `false` for everything else.
+     *
+     * The acceptance set is the same one `resolveInputID()` uses
+     * (`ID_PATTERNS`) — so an input that passes `isValidID()` is
+     * GUARANTEED not to throw on a subsequent `toSteam*()` call from
+     * the same value. Pre-#1420 the two surfaces drifted: `isValidID`'s
+     * unanchored loose-class regexes (`STEAM_[0|1]:[0:1]:\d*` — `|` in
+     * `[…]` is a literal pipe, the missing `^`/`$` made it a substring
+     * matcher, `\d*` accepted zero digits) accepted strings the
+     * conversion path then either round-tripped verbatim into the DB
+     * (`asdfSTEAM_0:0:123`) or converted to negative-Z garbage
+     * (`asdf 76561197960265728 garbage` → `STEAM_0:0:-38280598980132864`).
+     * Three concrete bypass shapes the post-fix gate correctly rejects:
+     *   - `STEAM_0:0:` — empty Z. `\d+` rejects.
+     *   - `asdfSTEAM_0:0:123` — substring bypass. `^…$` anchors reject.
+     *   - `STEAM_2:0:0` — invalid universe digit. `[01]` rejects.
+     *
+     * Callers in `web/api/handlers/{bans,comms,admins}.php` carry their
+     * own per-handler `preg_match` gate for defence-in-depth — both
+     * layers should agree on the accepted shape. See "SteamID inputs"
+     * in AGENTS.md for the contract.
+     *
+     * @param  mixed $steamid
      */
-    public static function isValidID($steamid)
+    public static function isValidID($steamid): bool
     {
-        switch (true) {
-            case preg_match("/STEAM_[0|1]:[0:1]:\d*/", $steamid):
-            case preg_match("/\[U:1:\d*\]/", $steamid):
-            case preg_match("/U:1:\d*/", $steamid):
-            case preg_match("/\d{17}/", $steamid):
+        $s = (string) $steamid;
+        foreach (self::ID_PATTERNS as [$pattern, $_format]) {
+            if (preg_match($pattern, $s) === 1) {
                 return true;
-            default:
-                return false;
+            }
         }
+        return false;
     }
 
     /**
diff --git a/web/tests/integration/SteamIDValidationTest.php b/web/tests/integration/SteamIDValidationTest.php
new file mode 100644
index 000000000..4a8c2f07a
--- /dev/null
+++ b/web/tests/integration/SteamIDValidationTest.php
@@ -0,0 +1,345 @@
+<?php
+// SourceBans++ (c) 2014-2026 SourceBans++ Dev Team
+// Licensed under Creative Commons Attribution-NonCommercial-ShareAlike 3.0.
+// See LICENSE.md for the full license text and THIRD-PARTY-NOTICES.txt for attributions.
+
+declare(strict_types=1);
+
+namespace Sbpp\Tests\Integration;
+
+use Exception;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+use ReflectionClass;
+use SteamID\SteamID;
+
+/**
+ * #1420 follow-up — locks the strict-shape contract of
+ * `SteamID::isValidID()` / `SteamID::resolveInputID()` after the library
+ * tightening. Pre-fix the regexes were unanchored with loose character
+ * classes (`STEAM_[0|1]:[0:1]:\d*` — `|` inside `[…]` is a literal pipe,
+ * not alternation; `\d*` accepts zero digits; missing `^`/`$` makes them
+ * substring matchers), so three concrete bypass shapes accepted by the
+ * library AND silently corrupted on conversion:
+ *
+ *   - `STEAM_0:0:` — empty Z digit. `\d*` accepts zero matches. The
+ *     value round-trips through `toSteam2()` unchanged, landing as an
+ *     invalid SteamID in `:prefix_admins.authid` /
+ *     `:prefix_bans.authid` / `:prefix_comms.authid`.
+ *   - `asdfSTEAM_0:0:123` — substring bypass. The unanchored regex
+ *     finds `STEAM_0:0:123` inside the garbage, returns true, and
+ *     `toSteam2()` returns the FULL input string verbatim (the regex
+ *     in `resolveInputID` matches the substring but the conversion
+ *     methods just `explode(':', $steamid)` and stringify the parts).
+ *   - `asdf 76561197960265728 garbage` — embedded Steam64. The
+ *     unanchored `\d{17}` matches the embedded 17 digits AND
+ *     `toSteam2()` emits `STEAM_0:0:-38280598980132864` (negative Z
+ *     from `(int) 'asdf 76561197960265728 garbage'` being 0 instead of
+ *     the embedded number, then the math underflows).
+ *
+ * Post-fix the regexes are anchored (`^…$`), use proper character
+ * classes (`[01]` not `[0|1]` / `[0:1]`), and require at least one
+ * digit (`\d+` not `\d*`). The strict gate now rejects every bypass
+ * shape AND keeps every legitimate SteamID format accepted (the
+ * `SteamIdConversionTest` test class covers the conversion math; this
+ * file covers the SHAPE-validation contract).
+ *
+ * `isValidID()` and `resolveInputID()` share a `ID_PATTERNS` table for
+ * single-source acceptance — any input that passes `isValidID()` is
+ * guaranteed not to throw `Invalid SteamID input!` from a subsequent
+ * `toSteam*()` call. The "byte-for-byte symmetry" tests below pin
+ * this contract so a future refactor that touches one without the
+ * other fails loudly here instead of silently divergence.
+ *
+ * Pure-PHP test — no DB / no Smarty / no Fixture::reset(); extends
+ * `PHPUnit\Framework\TestCase` directly.
+ */
+final class SteamIDValidationTest extends TestCase
+{
+    /**
+     * Inputs the strict gate MUST accept. Each entry is `[input,
+     * resolved-format]` so the symmetry test can prove `isValidID`
+     * AND `resolveInputID` agree.
+     *
+     * @return iterable<string, array{0: string, 1: string}>
+     */
+    public static function validInputs(): iterable
+    {
+        // Steam2 canonical
+        yield 'STEAM_0:0:11101'        => ['STEAM_0:0:11101', 'Steam2'];
+        yield 'STEAM_0:1:11101'        => ['STEAM_0:1:11101', 'Steam2'];
+        yield 'STEAM_0:0:0 (zero Z)'   => ['STEAM_0:0:0',     'Steam2'];
+        // Steam2 universe 1 (TF2/L4D normalise to STEAM_1 — see
+        // `to()`'s `from === $format` branch which str_replaces back
+        // to STEAM_0, and the SourceMod plugin's `authid REGEXP
+        // '^STEAM_[0-9]:Y:Z$'` query which accepts both).
+        yield 'STEAM_1:0:11101 (universe 1)' => ['STEAM_1:0:11101', 'Steam2'];
+        yield 'STEAM_1:1:11101 (universe 1)' => ['STEAM_1:1:11101', 'Steam2'];
+        // Large account ID
+        yield 'STEAM_0:1:2147483647 (max accid)' => ['STEAM_0:1:2147483647', 'Steam2'];
+
+        // Steam3 bracketed
+        yield '[U:1:22202]'            => ['[U:1:22202]', 'Steam3'];
+        yield '[U:1:0] (zero accid)'   => ['[U:1:0]',     'Steam3'];
+        yield '[U:1:4294967295] (max)' => ['[U:1:4294967295]', 'Steam3'];
+
+        // Steam3 bracketless (legacy compat — see ID_PATTERNS docblock)
+        yield 'U:1:22202 (bracketless)' => ['U:1:22202', 'Steam3'];
+
+        // Steam64 (exactly 17 digits)
+        yield '76561197960265728 (base / zero)' => ['76561197960265728', 'Steam64'];
+        yield '76561197960287930 (canonical)'   => ['76561197960287930', 'Steam64'];
+        yield '76561202255233023 (large)'       => ['76561202255233023', 'Steam64'];
+    }
+
+    /**
+     * Inputs the strict gate MUST reject. These are the bypass shapes
+     * the #1420 review surfaced PLUS the obvious garbage cases the
+     * library always rejected (kept here as regression guards in case
+     * a future regex tweak accidentally accepts them).
+     *
+     * @return iterable<string, array{0: string, 1: string}>
+     *         [input, why-it-fails]
+     */
+    public static function invalidInputs(): iterable
+    {
+        // ----- The three concrete bypass shapes from #1420 -------------
+        yield 'STEAM_0:0: (empty Z digit)'            => ['STEAM_0:0:',          'Z digit is empty; \d+ requires at least one'];
+        yield 'asdfSTEAM_0:0:123 (substring bypass)'  => ['asdfSTEAM_0:0:123',   'leading garbage; ^…$ anchors reject'];
+        yield 'STEAM_0:0:123garbage (trailing junk)'  => ['STEAM_0:0:123garbage', 'trailing garbage; ^…$ anchors reject'];
+        yield 'asdf 76561197960265728 garbage'        => ['asdf 76561197960265728 garbage', 'embedded Steam64; ^…$ anchors reject'];
+
+        // ----- Invalid universe / Y digit ------------------------------
+        yield 'STEAM_2:0:0 (universe 2)'              => ['STEAM_2:0:0',          'universe digit 2 not in [01]'];
+        yield 'STEAM_|:0:0 (literal pipe)'            => ['STEAM_|:0:0',          'pre-fix the loose class [0|1] accepted | as a literal char'];
+        yield 'STEAM_0:2:0 (Y=2)'                     => ['STEAM_0:2:0',          'Y digit 2 not in [01]'];
+        yield 'STEAM_0::0 (Y is colon)'               => ['STEAM_0::0',           'pre-fix the loose class [0:1] accepted : as a literal char'];
+
+        // ----- Empty / whitespace --------------------------------------
+        yield 'empty string'                          => ['',                     'empty; no pattern matches'];
+        yield 'single space'                          => [' ',                    'whitespace; no pattern matches'];
+        yield 'four spaces'                           => ['    ',                 'whitespace; no pattern matches'];
+
+        // ----- Steam3 bracket mismatches -------------------------------
+        yield '[U:1:22202 (missing closing bracket)' => ['[U:1:22202',           '^\[…\]$ requires closing bracket'];
+        yield 'U:1:22202] (missing opening bracket)' => ['U:1:22202]',           'bracketless form does not accept trailing ]'];
+        yield '[U:2:22202] (Steam3 universe 2)'      => ['[U:2:22202]',          'Steam3 regex requires universe 1'];
+        yield '[U:1:] (empty account id)'            => ['[U:1:]',               '\d+ requires at least one digit'];
+        yield 'U:1: (bracketless empty accid)'       => ['U:1:',                 '\d+ requires at least one digit'];
+
+        // ----- Steam64 shape failures ----------------------------------
+        yield '7656119796026572 (16 digits)'          => ['7656119796026572',     '\d{17} requires exactly 17'];
+        yield '765611979602657281 (18 digits)'        => ['765611979602657281',   '^\d{17}$ rejects > 17'];
+        yield '12345 (5 digits)'                      => ['12345',                'far too short for Steam64'];
+        yield '7656119796026572a (17 chars w/ alpha)' => ['7656119796026572a',    'has non-digit'];
+
+        // ----- Pure garbage --------------------------------------------
+        yield 'garbage'                               => ['garbage',              'no SteamID shape'];
+        yield 'asdf'                                  => ['asdf',                 'no SteamID shape'];
+        yield 'null'                                  => ['null',                 'literal string "null"'];
+        yield 'STEAM (prefix only)'                   => ['STEAM',                'prefix only'];
+        yield 'STEAM_0 (prefix only)'                 => ['STEAM_0',              'partial prefix'];
+        yield '0 (single zero)'                       => ['0',                    'single digit'];
+
+        // ----- Embedded valid IDs (more shapes) ------------------------
+        yield 'prefix STEAM_0:0:1 (leading text)'     => ['prefix STEAM_0:0:1',   'leading text bypass'];
+        yield 'STEAM_0:0:1 suffix (trailing text)'    => ['STEAM_0:0:1 suffix',   'trailing text bypass'];
+        yield '[U:1:1][U:1:2] (two Steam3 joined)'    => ['[U:1:1][U:1:2]',       'doubled Steam3'];
+        yield 'newline-embedded valid ID'             => ["STEAM_0:0:1\n",        'trailing newline'];
+        yield 'tab-embedded valid ID'                 => ["\tSTEAM_0:0:1",        'leading tab'];
+    }
+
+    /**
+     * Strict gate accepts every documented legitimate SteamID format.
+     */
+    #[DataProvider('validInputs')]
+    public function testIsValidIdAcceptsLegitimateInputs(string $input, string $_format): void
+    {
+        $this->assertTrue(
+            SteamID::isValidID($input),
+            sprintf('SteamID::isValidID(%s) must be true (legitimate input)', var_export($input, true)),
+        );
+    }
+
+    /**
+     * Strict gate rejects the #1420 bypass shapes + every shape that
+     * was never legitimate but the loose pre-fix regex sometimes
+     * accepted.
+     */
+    #[DataProvider('invalidInputs')]
+    public function testIsValidIdRejectsBypassesAndGarbage(string $input, string $why): void
+    {
+        $this->assertFalse(
+            SteamID::isValidID($input),
+            sprintf(
+                'SteamID::isValidID(%s) must be false (%s)',
+                var_export($input, true),
+                $why,
+            ),
+        );
+    }
+
+    /**
+     * Byte-for-byte symmetry: any input passing `isValidID()` must
+     * resolve to a known format through `resolveInputID()` (i.e. NOT
+     * throw `Invalid SteamID input!`). Pre-fix the two surfaces had
+     * subtly different regex tables and the asymmetry was the
+     * underlying bug-class — `isValidID` accepted strings
+     * `resolveInputID` then matched in a different position and
+     * `toSteam2()` corrupted on conversion. The shared `ID_PATTERNS`
+     * constant guarantees the two stay in lockstep; this test pins it.
+     */
+    #[DataProvider('validInputs')]
+    public function testValidInputsResolveToExpectedFormat(string $input, string $expectedFormat): void
+    {
+        // Reach for `resolveInputID` via reflection — it's private by
+        // design (the public surface is `to*()` which dispatches off
+        // the resolution result). Testing the resolver directly proves
+        // the symmetry contract without coupling to the conversion
+        // math. PHP 8.1+ makes private methods reachable via
+        // `Reflection::invoke()` without an explicit `setAccessible()`
+        // call (and PHP 8.5 deprecates `setAccessible()` for that
+        // reason).
+        $resolve = (new ReflectionClass(SteamID::class))->getMethod('resolveInputID');
+        $this->assertSame(
+            $expectedFormat,
+            $resolve->invoke(null, $input),
+            sprintf(
+                'SteamID::resolveInputID(%s) must resolve to %s',
+                var_export($input, true),
+                var_export($expectedFormat, true),
+            ),
+        );
+    }
+
+    /**
+     * Sister of the above: any input the gate rejects must ALSO be
+     * rejected by `resolveInputID()` (raises `Invalid SteamID input!`).
+     * Pre-fix asymmetry meant `resolveInputID` could throw on inputs
+     * `isValidID` had just blessed — the JSON dispatcher caught the
+     * throw and surfaced it as a generic 500 (#1420's user-visible
+     * symptom). The shared-table fix guarantees the two surfaces agree.
+     */
+    #[DataProvider('invalidInputs')]
+    public function testInvalidInputsAlsoThrowFromResolveInputId(string $input, string $why): void
+    {
+        $resolve = (new ReflectionClass(SteamID::class))->getMethod('resolveInputID');
+        $this->expectException(Exception::class);
+        $this->expectExceptionMessage('Invalid SteamID input!');
+        $resolve->invoke(null, $input);
+        $this->fail(sprintf(
+            'SteamID::resolveInputID(%s) must throw (%s)',
+            var_export($input, true),
+            $why,
+        ));
+    }
+
+    /**
+     * Acceptance for valid inputs must convert without throwing. This
+     * is the end-to-end shape: `isValidID()` → `toSteam2()` round-trip
+     * is the canonical "this is a SteamID I can do something with"
+     * contract every JSON / page handler depends on.
+     */
+    #[DataProvider('validInputs')]
+    public function testValidInputsConvertWithoutThrowing(string $input, string $_format): void
+    {
+        $this->assertNotFalse(SteamID::toSteam2($input));
+        $this->assertNotFalse(SteamID::toSteam3($input));
+        $this->assertNotFalse(SteamID::toSteam64($input));
+    }
+
+    /**
+     * `to()`'s `if (empty($steamid)) return false;` short-circuit still
+     * fires before `resolveInputID()` — so `toSteam*('')` returns
+     * `false` rather than throwing. Pin this so a future refactor of
+     * the early-return branch (e.g. moving the empty check into
+     * `resolveInputID` itself) doesn't change the public surface
+     * silently.
+     */
+    public function testEmptyInputReturnsFalseFromConverters(): void
+    {
+        $this->assertFalse(SteamID::toSteam2(''));
+        $this->assertFalse(SteamID::toSteam3(''));
+        $this->assertFalse(SteamID::toSteam64(''));
+    }
+
+    /**
+     * Single source of truth: the `ID_PATTERNS` constant is the only
+     * place the accepted-shape regexes live. A future agent editing
+     * `isValidID()` or `resolveInputID()` separately would break the
+     * symmetry — pin the constant's presence + shape so the gate
+     * fires.
+     */
+    public function testIdPatternsConstantIsTheSourceOfTruth(): void
+    {
+        $reflection = new ReflectionClass(SteamID::class);
+        $this->assertTrue(
+            $reflection->hasConstant('ID_PATTERNS'),
+            'SteamID::ID_PATTERNS must be defined as the single source for the accepted-shape regex table.',
+        );
+        $patterns = $reflection->getReflectionConstant('ID_PATTERNS')->getValue();
+        $this->assertIsArray($patterns);
+        $this->assertCount(4, $patterns, 'ID_PATTERNS must list exactly the four documented shapes (Steam2 / Steam3 bracketed / Steam3 bracketless / Steam64).');
+
+        // Every regex must (1) be `^…$` anchored AND (2) carry the `D`
+        // modifier. The `D` modifier strictly anchors `$` to
+        // end-of-string only (default behaviour allows a trailing
+        // `\n`); without it `STEAM_0:0:1\n` slips through the gate
+        // AND lands as a trailing-newline string in the DB on
+        // conversion (the newline-bypass sibling of #1420's
+        // substring-bypass class).
+        foreach ($patterns as [$regex, $format]) {
+            $this->assertIsString($regex);
+            $this->assertIsString($format);
+            $this->assertStringStartsWith('/^', $regex, sprintf(
+                'Pattern %s for format %s must start with /^ — substring bypasses (#1420) require explicit ^…$ anchors.',
+                var_export($regex, true),
+                var_export($format, true),
+            ));
+            $this->assertMatchesRegularExpression(
+                '~\$/[a-zA-Z]*D[a-zA-Z]*$~',
+                $regex,
+                sprintf(
+                    'Pattern %s for format %s must end with $/…D… (the D modifier strictly anchors $ to end-of-string; without it, trailing \\n bypasses the gate).',
+                    var_export($regex, true),
+                    var_export($format, true),
+                ),
+            );
+        }
+    }
+
+    /**
+     * Strict gate doesn't deprecation-warn on int input. The library
+     * accepts `toSteam2((int) $steam64)` per the
+     * `SteamIdConversionTest::testSteam64ToSteam2` int-shape arm — that
+     * path reaches `resolveInputID()` which historically did
+     * `preg_match($pattern, $intValue)` and emitted PHP 8.1's
+     * "Passing null to parameter…" / int-to-string deprecation. The
+     * post-#1420 cast at the entry point (`$s = (string) $steamid`)
+     * silences this; pin the contract.
+     */
+    public function testIsValidIdAcceptsIntInput(): void
+    {
+        // Promote PHP deprecation notices to exceptions for this test.
+        // If the cast-at-entry-point disappears, the next `preg_match`
+        // call against the int value would raise `E_DEPRECATED` which
+        // would land here as `\ErrorException`.
+        $prev = set_error_handler(static function (int $errno, string $errstr): bool {
+            if (in_array($errno, [E_DEPRECATED, E_USER_DEPRECATED], true)) {
+                throw new \ErrorException($errstr, 0, $errno);
+            }
+            return false;
+        });
+
+        try {
+            $this->assertTrue(SteamID::isValidID(76561197960287930));
+            // Steam64-as-int → resolves to Steam64 (the cast makes it
+            // a 17-char digit string).
+            $this->assertSame('STEAM_0:0:11101', SteamID::toSteam2(76561197960287930));
+        } finally {
+            restore_error_handler();
+            $this->assertSame($prev, set_error_handler(static fn (): bool => false));
+            restore_error_handler();
+        }
+    }
+}

From 9555f4f8d80411b760df757e3cd1d59352b34704 Mon Sep 17 00:00:00 2001
From: rumblefrog <contact@zhenyangli.me>
Date: Tue, 19 May 2026 19:14:16 -0400
Subject: [PATCH 4/9] fix(steamid): validate-before-convert across page-handler
 form-POST surfaces (review #1420 follow-up #2)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Lift the validate-before-convert contract from the three JSON `*_add`
handlers fixed in #1420 proper into the four page-handler form-POST
surfaces (and the `importBans` file-upload branch in
`admin.bans.php`) that called `SteamID::toSteam2()` BEFORE
`SteamID::isValidID()`. Same bug class, different failure mode.

# The bug class

Pre-fix each surface called `SteamID::toSteam2(trim($_POST['steam']))`
as the FIRST statement in the POST branch:

  - `web/pages/admin.edit.ban.php`        L113 (top of `if (isset($_POST['name']))`)
  - `web/pages/admin.edit.comms.php`      L101 (top of `if (isset($_POST['name']))`)
  - `web/pages/admin.edit.admindetails.php` L67 (`$resolvedSteam = …`)
  - `web/pages/admin.bans.php`            L121 (the `banid` branch of `importBans`)
  - `web/pages/page.submit.php`           never called `toSteam2()`; just
                                          gated on the loose
                                          `SteamID::isValidID()` check
                                          (closed by follow-up #1's
                                          library tightening).

The converter raised `Exception('Invalid SteamID input!')` from
`resolveInputID()` on any non-empty input that failed the shape check;
the exception escaped the page handler unhandled, the panel chrome's
`PageDie()` never fired, and the user got a generic 500 page render
instead of the inline per-field error on the form. The
`admin.bans.php` import case was the worst: a single malformed
`banid …` line in the operator's uploaded `banned_user.cfg` aborted
the entire import (no transaction wrapper) and the operator had no
signal as to which line broke or how many of the preceding inserts
committed.

Follow-up #1's library tightening made the throw stricter (the four
ID_PATTERNS regexes are now anchored, `[01]` instead of `[0|1]`,
`\d+` instead of `\d*`, with the `D` modifier closing the newline-
bypass sibling). Pre-tighter library many garbage inputs round-tripped
verbatim into the DB as the "canonical" SteamID via `STEAM_0:0:` /
substring-embedded / Steam64-embedded shapes documented in the
SteamID inputs Conventions block. Post-tighter library every such
input throws — which makes the page-handler 500-page-render shape
strictly MORE frequent and reachable from any typo. So follow-up #2
is the necessary paired fix on the consumer side.

# The fix shape (validate-then-convert ladder)

Per the AGENTS.md "SteamID inputs" → page-handler form-POST surfaces
contract (added here):

  $rawSteam = trim((string) ($_POST['steam'] ?? ''));
  if ($rawSteam === '') {
      $validationErrors['steam'] = 'You must type a Steam ID or Community ID';
  } elseif (!\SteamID\SteamID::isValidID($rawSteam)) {
      $validationErrors['steam'] = 'Please enter a valid Steam ID or Community ID';
      // Preserve raw input on bounce so operator can fix the typo.
      $_POST['steam'] = $rawSteam;
  } else {
      $_POST['steam'] = \SteamID\SteamID::toSteam2($rawSteam);
  }

Each handler picks the error-surfacing channel its surrounding code
already uses (Option B per AGENTS.md "Add a confirm + reason modal"):

  - `admin.edit.ban.php`       → `$validationErrors['steam']` + form
                                  re-render (matches existing
                                  empty / IP / duplicate paths).
  - `admin.edit.comms.php`     → `$errorFields[] = ['steam.msg', '…']`
                                  + page-tail script (matches existing
                                  pattern).
  - `admin.edit.admindetails.php` → `$validationErrors['steam']` +
                                    form re-render. The pre-fix
                                    `$resolvedSteam !== '' || strlen
                                    < 10` empty-check splits into
                                    distinct "empty" / "invalid"
                                    messages so the operator sees
                                    actionable wording.
  - `admin.bans.php` (importBans branch) → skip-and-count. Counts
                                    invalid lines via a `$skipped`
                                    counter and surfaces the count
                                    in the success toast next to
                                    the imported count.

`page.submit.php` doesn't call `toSteam2()` at all (the public form
stores raw input verbatim in `:prefix_submissions`; the moderation
queue does the canonical resolution on accept). The library
tightening closes the bypass for this surface; the paired
template-side `pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"` +
`title="Enter a Steam ID …"` are the client-side defense. The
legacy `STEAM_0:` empty-sentinel pre-fill the page handler used to
re-emit was dropped in the same edit: the partial sentinel would
have failed the new pattern check AND blocked submission for the
legitimate IP-only path (which the form explicitly supports).

# Templates

All four form templates carry the same strict `pattern` + `title`
attributes the JSON-flow add-form templates (`page_admin_comms_add.tpl`
/ `page_admin_bans_add.tpl`) already ship from #1420 proper:

  - `page_admin_edit_admins_details.tpl` → `required` + pattern
  - `page_admin_edit_ban.tpl`            → pattern only (IP-target
                                            bans legitimately leave
                                            the Steam ID input empty)
  - `page_admin_edit_comms.tpl`          → `required` + pattern
  - `page_submitban.tpl`                 → pattern only (IP
                                            alternative) + removed
                                            `novalidate` so native
                                            validation actually fires
                                            client-side

The `STEAM_0:` empty-sentinel dead defense in `page_submitban.tpl`'s
`isEmpty(el)` helper was dropped in the same template edit (the
page handler no longer emits the sentinel; the JS check would never
hit).

# Test coverage

`web/tests/integration/SteamIDValidationOrderTest.php` (9 tests):

  - `testAdminEditBanValidatesBeforeConverting`
  - `testAdminEditCommsValidatesBeforeConverting`
  - `testAdminEditAdminDetailsValidatesBeforeConverting`
  - `testAdminBansImportValidatesBeforeConverting`
  - `testPageSubmitUsesIsValidIdNotToSteam2`
  - `testPageSubmitDroppedSteamZeroSentinel`
  - `testFormTemplatesCarryStrictSteamPattern`
  - `testFormTemplatesCarrySteamPatternTitle`
  - `testPageSubmitFormDoesNotCarryNovalidate`
  - `testJsonHandlersKeepDefenseInDepthRegex`

Pattern lifted from `AdminEditCommsCheckOrderTest` (#1410) — static-
shape pins via `strpos($contents, …)` ordering rather than process-
isolated runtime tests. The page handlers all run inside the chrome-
render pipeline (validation → `Renderer::render` → Smarty → `echo`),
which doesn't compose with PHPUnit's `runInSeparateProcess` mode (the
helper's `exit;` mid-test breaks the child-process serializer and
PHPUnit reports an errored test regardless of the assertion outcome).
The static-shape pin catches the bug directly: each test asserts that
`isValidID(...)` appears BEFORE `toSteam2(...)` in the file's text;
reverse the order and the test fails before the page does in
production.

The runtime side of the contract is exercised by the existing
`comms-add SteamID validation` Playwright spec — the strict-regex
contract is shared across the JSON + form surfaces, so the existing
E2E coverage exercises the same library boundary the page handlers
now route through.

# Newly discovered scope

The `admin.bans.php` import path (`banid <duration> <STEAMID>` lines)
was discovered during the audit and added to the sweep — same bug
class, same fix shape, but slightly different error-surface
(skip-and-count instead of bounce-with-inline-error since the
operation is a bulk import).

# Quality gates (all green)

  ./sbpp.sh phpstan          → 241/241 OK
  ./sbpp.sh test             → 837 tests, 3138 assertions OK
  ./sbpp.sh composer api-contract → no diff (no handler signatures changed)

Refs #1420
---
 AGENTS.md                                     |  92 ++++
 web/pages/admin.bans.php                      |  57 +-
 web/pages/admin.edit.admindetails.php         |  29 +-
 web/pages/admin.edit.ban.php                  |  69 ++-
 web/pages/admin.edit.comms.php                |  33 +-
 web/pages/page.submit.php                     |  41 +-
 .../SteamIDValidationOrderTest.php            | 485 ++++++++++++++++++
 .../page_admin_edit_admins_details.tpl        |  14 +-
 web/themes/default/page_admin_edit_ban.tpl    |  18 +-
 web/themes/default/page_admin_edit_comms.tpl  |  16 +-
 web/themes/default/page_submitban.tpl         |  44 +-
 11 files changed, 856 insertions(+), 42 deletions(-)
 create mode 100644 web/tests/integration/SteamIDValidationOrderTest.php

diff --git a/AGENTS.md b/AGENTS.md
index 171d8a3f7..1e735c024 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -767,6 +767,59 @@ of the diff ship together or not at all.
   + `BansTest.php` + `AdminsTest.php` pin the handler-side
   `ApiError('validation', …, 'steam')` envelope.
 
+  **Page-handler form-POST surfaces** (`page.submit.php`,
+  `admin.edit.{ban,comms,admindetails}.php`,
+  `admin.bans.php`'s `importBans` branch) carry the same
+  convert-before-validate bug class as the JSON handlers, but the
+  failure mode is different: the converter's
+  `Exception('Invalid SteamID input!')` escapes the page handler
+  unhandled, the chrome's `PageDie()` never fires, and the user
+  gets a generic 500 page render instead of the inline per-field
+  error message on the form. The fix shape is symmetric to the
+  JSON-handler one but the error-surfacing differs by surface:
+   - `admin.edit.{ban,comms,admindetails}.php` push the validation
+     error into the existing `$validationErrors[]` / `$errorFields[]`
+     array and re-render the form via the page-tail script (Option
+     B per "Add a confirm + reason modal" — matches the established
+     pattern for empty / duplicate / invalid SteamID, and preserves
+     the operator's raw input on the bounce so they see exactly
+     what they typed and can correct the typo without re-typing
+     everything else).
+   - `page.submit.php` doesn't call `SteamID::toSteam2()` at all —
+     it stores the raw user-input verbatim in `:prefix_submissions`
+     and the moderation queue resolves the canonical form on
+     accept. The library tightening (follow-up #1) closes the bypass
+     here without any handler edit; the template-side strict
+     `pattern="…"` is the front-line defense.
+   - `admin.bans.php`'s `importBans` branch validates each
+     `banid <duration> <STEAMID>` line via `SteamID::isValidID()`
+     BEFORE `toSteam2()`, skipping (and counting) malformed lines
+     instead of throwing. The pre-fix abort-on-first-bad-line shape
+     left the operator with no signal as to which line broke or
+     how many of the preceding inserts committed (no transaction
+     wrapper). The success toast now carries the skipped-line
+     count alongside the imported count.
+
+  The validate-before-convert order is THE structural contract:
+  call `SteamID::isValidID($raw)` first, surface the error on the
+  fail branch, ONLY THEN call `SteamID::toSteam2($raw)`. With the
+  library tightening from follow-up #1 every input that passes
+  `isValidID()` is guaranteed to round-trip through `toSteam2()`
+  without throwing, so the per-handler `try/catch` defensiveness
+  that some pre-fix code accidentally relied on (`empty()` short-
+  circuits in `to()`, etc.) is no longer needed. Don't ship a
+  `try/catch (Exception)` around `toSteam2()` as a substitute for
+  the upstream gate — it papers over the bug class without fixing
+  the underlying ordering and weakens the contract on the next
+  refactor. Regression coverage:
+  `web/tests/integration/SteamIDValidationOrderTest.php` static-
+  shape-pins the validate-then-convert order across every page
+  handler. The form templates also carry the same
+  `pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"` +
+  actionable `title="…"` as the JSON-flow add-form templates so
+  the browser-native popover surfaces the same error message
+  pre-flight.
+
 ### CSRF
 
 - Required on every state-changing form/JSON call.
@@ -3055,6 +3108,45 @@ contacting every contributor individually.
   re-open the bypass class. Regression coverage for the handler
   half: `web/tests/api/CommsTest.php::testAddRejectsInvalidSteamIdShape`
   (+ sibling tests in `BansTest.php` / `AdminsTest.php`).
+- Calling `SteamID::toSteam2($raw)` (or any other `SteamID::*`
+  conversion that funnels through `resolveInputID`) on a page
+  handler's raw POST input BEFORE running `SteamID::isValidID($raw)`
+  → the converter raises `Exception('Invalid SteamID input!')` on
+  any input that fails the shape check post-#1420 (the library
+  tightening from follow-up #1 made the throw stricter). The
+  exception escapes the page handler unhandled, the chrome's
+  `PageDie()` never fires, and the user gets a 500 page render
+  instead of the inline per-field "Please enter a valid Steam ID
+  or Community ID" message on the form. The fix is the
+  validate-then-convert ladder documented under "SteamID inputs"
+  in Conventions: trim → empty-check (separate message) →
+  `isValidID()` shape check (separate message) → ONLY THEN
+  `toSteam2()`. The pre-#1420 shape silently "worked" because the
+  loose library accepted everything; with the tighter library the
+  same call shape is a 500-page-render-on-typo waiting to happen.
+  Affected surfaces swept at #1423 follow-up #2:
+  `web/pages/admin.edit.{ban,comms,admindetails}.php` (the form
+  re-render path), `web/pages/admin.bans.php`'s `importBans`
+  branch (skip-and-count malformed lines instead of aborting
+  mid-file). `web/pages/page.submit.php` doesn't call `toSteam2()`
+  at all — the public form stores raw input verbatim, and the
+  library tightening + template-side `pattern="…"` close the
+  bypass without a handler edit. Regression guard:
+  `web/tests/integration/SteamIDValidationOrderTest.php` pins the
+  validate-then-convert call order across every page handler.
+- Wrapping `SteamID::toSteam2($raw)` in `try/catch (\Exception)`
+  as a substitute for the upstream `SteamID::isValidID($raw)`
+  gate → the catch papers over the bug class without fixing the
+  underlying call-order bug. A future refactor of the library
+  that swaps the exception shape (e.g. to a typed
+  `\InvalidArgumentException`) silently breaks the catch and the
+  exception escapes again — back to a 500 page render. The
+  contract is "validate first, convert only on a pass" (see
+  "SteamID inputs" → page-handler form-POST surfaces); the
+  validate-then-convert order is what gives `toSteam2()` its
+  cannot-throw guarantee in handler code, so wrapping it in
+  `try/catch` is a code smell signalling the upstream gate is
+  missing.
 - Editing `install/includes/sql/data.sql` (or `struc.sql`) without a paired
   `web/updater/data/<N>.php` → upgraded installs silently miss the change.
 - WYSIWYG / "rich HTML" editors (TinyMCE, CKEditor, …) for fields stored
diff --git a/web/pages/admin.bans.php b/web/pages/admin.bans.php
index 224bce3ef..010c979a7 100644
--- a/web/pages/admin.bans.php
+++ b/web/pages/admin.bans.php
@@ -91,10 +91,29 @@
 if (isset($_POST['action']) && $_POST['action'] == "importBans") {
     $bannedcfg = file($_FILES["importFile"]["tmp_name"]);
     $bancnt    = 0;
+    // #1420 follow-up #2: track lines we couldn't import so the
+    // operator gets actionable feedback in the success toast instead
+    // of silently dropping a malformed row. Pre-fix a single bogus
+    // `banid 0 STEAM_INVALID` line in the operator's `banned_user.cfg`
+    // would have raised `Exception('Invalid SteamID input!')` from
+    // `SteamID::toSteam2()` — the import aborted mid-file with a 500
+    // page render and the operator had no idea which line broke or
+    // how many of the preceding lines committed (the foreach inserts
+    // were not wrapped in a transaction).
+    $skipped   = 0;
 
     foreach ($bannedcfg as $ban) {
         $line = explode(" ", trim($ban));
 
+        // Skip ill-formed lines (comments, blanks, truncated rows).
+        // The legacy format is `addip <duration> <ip>` / `banid
+        // <duration> <STEAMID>`, three space-separated tokens minimum.
+        // Pre-fix this could land on undefined-index warnings or a
+        // PHP type error inside `filter_var` / `toSteam2`.
+        if (count($line) < 3) {
+            continue;
+        }
+
         if ($line[0] === 'addip') {
             if (filter_var($line[2], FILTER_VALIDATE_IP, FILTER_FLAG_IPV4)) {
                 $GLOBALS['PDO']->query("SELECT ip FROM `:prefix_bans` WHERE ip = :ip AND RemoveType IS NULL");
@@ -116,8 +135,32 @@
                     ]);
                     $GLOBALS['PDO']->execute();
                 }
+            } else {
+                $skipped++;
             }
         } elseif ($line[0] === 'banid') {
+            // #1420 follow-up #2 — validate the raw Steam ID shape
+            // BEFORE the `SteamID::toSteam2()` conversion. Pre-fix the
+            // converter ran on `$line[2]` (the third whitespace token
+            // of a `banid …` line in the uploaded `banned_user.cfg`)
+            // unconditionally; a malformed Steam ID raised
+            // `Exception('Invalid SteamID input!')` from
+            // `resolveInputID()`, the exception escaped the page
+            // handler unhandled, the import aborted mid-file (no
+            // transaction), and the operator got a 500 page render
+            // with no signal as to which line broke or how many of
+            // the preceding lines committed.
+            //
+            // The library tightening in follow-up #1 made the throw
+            // stricter, which made the abort strictly MORE frequent
+            // for any cfg carrying legacy/typo'd rows. Validate-
+            // before-convert lets the import skip the bad line and
+            // surface a count in the success toast instead.
+            if (!\SteamID\SteamID::isValidID($line[2])) {
+                $skipped++;
+                continue;
+            }
+
             $steam = \SteamID\SteamID::toSteam2($line[2]);
 
             $GLOBALS['PDO']->query("SELECT authid FROM `:prefix_bans` WHERE authid = :authid AND RemoveType IS NULL");
@@ -147,7 +190,19 @@
     if ($bancnt > 0) {
         Log::add(LogType::Message, "Bans imported", "$bancnt Ban(s) imported");
     }
-    echo "<script>sb.message.show('Bans Import', '$bancnt ban" . ($bancnt != 1 ? "s have" : " has") . " been imported and posted.', 'green', '');</script>";
+    // #1420 follow-up #2 — surface skipped-line count alongside the
+    // imported count so the operator knows the file had malformed
+    // entries. Without this signal a half-bad cfg silently produces a
+    // partial import and the operator assumes everything landed.
+    // Mirrors the legacy `sb.message.show` shape the rest of this
+    // handler uses (the wider toast/sb.message migration is sister
+    // scope; this edit stays focused on the validate-before-convert
+    // fix).
+    $importMsg = "$bancnt ban" . ($bancnt != 1 ? "s have" : " has") . " been imported and posted.";
+    if ($skipped > 0) {
+        $importMsg .= ' ' . $skipped . ' line' . ($skipped != 1 ? 's were' : ' was') . ' skipped (malformed Steam ID or IP).';
+    }
+    echo "<script>sb.message.show('Bans Import', '" . addslashes($importMsg) . "', 'green', '');</script>";
 }
 
 /*
diff --git a/web/pages/admin.edit.admindetails.php b/web/pages/admin.edit.admindetails.php
index 93cc4df38..19a211ce6 100644
--- a/web/pages/admin.edit.admindetails.php
+++ b/web/pages/admin.edit.admindetails.php
@@ -64,7 +64,25 @@
 
     $rawName       = trim((string) $_POST['adminname']);
     $rawSteamInput = trim((string) ($_POST['steam'] ?? ''));
-    $resolvedSteam = \SteamID\SteamID::toSteam2($rawSteamInput);
+    // #1420 follow-up #2 — validate the raw Steam ID shape BEFORE the
+    // `SteamID::toSteam2()` conversion. Pre-fix `toSteam2()` ran on
+    // the raw POST value unconditionally; on a garbage input the
+    // converter threw `Invalid SteamID input!` from `resolveInputID()`,
+    // the exception escaped the page handler unhandled, and the user
+    // got a 500 page render instead of the inline per-field "Please
+    // enter a valid Steam ID or Community ID" message on the form.
+    //
+    // The library tightening (follow-up #1) made the throw stricter,
+    // making the 500 page render strictly MORE frequent. The
+    // validate-before-convert order surfaces the failure on the form
+    // as the per-field message; `$steamIsValidShape` carries the
+    // distinction (empty vs invalid-but-non-empty) into the validation
+    // ladder below so the operator sees the right error wording.
+    $steamIsValidShape = $rawSteamInput !== ''
+        && \SteamID\SteamID::isValidID($rawSteamInput);
+    $resolvedSteam = $steamIsValidShape
+        ? (string) \SteamID\SteamID::toSteam2($rawSteamInput)
+        : $rawSteamInput;
     $rawEmail      = trim((string) ($_POST['email'] ?? ''));
     $useServerPass = isset($_POST['a_useserverpass']) && $_POST['a_useserverpass'] === 'on';
     $newPassword   = (string) ($_POST['password']  ?? '');
@@ -81,9 +99,14 @@
         $validationErrors['adminname'] = 'An admin with this name already exists.';
     }
 
-    if ($resolvedSteam === '' || strlen($resolvedSteam) < 10) {
+    if ($rawSteamInput === '') {
         $validationErrors['steam'] = 'You must type a Steam ID or Community ID for the admin.';
-    } elseif (!\SteamID\SteamID::isValidID($resolvedSteam)) {
+    } elseif (!$steamIsValidShape) {
+        // Non-empty but failed `SteamID::isValidID()` — typo, bypass
+        // shape (`STEAM_0:0:` / `asdfSTEAM_0:0:123` / embedded
+        // Steam64), or some other invalid format. Surface the
+        // distinct error so the operator knows the field needs
+        // fixing, not just filling.
         $validationErrors['steam'] = 'Please enter a valid Steam ID or Community ID.';
     } elseif ($resolvedSteam !== $userbank->GetProperty('authid', $adminId)
         && $userbank->isSteamIDTaken($resolvedSteam)) {
diff --git a/web/pages/admin.edit.ban.php b/web/pages/admin.edit.ban.php
index 70578d86f..33ba66b58 100644
--- a/web/pages/admin.edit.ban.php
+++ b/web/pages/admin.edit.ban.php
@@ -110,24 +110,73 @@ function emitEditBanToastAndRedirect(string $kind, string $title, string $body,
 $postSuccess = false;
 
 if (isset($_POST['name'])) {
-    $_POST['steam'] = \SteamID\SteamID::toSteam2(trim((string) ($_POST['steam'] ?? '')));
+    // #1420 follow-up #2 — validate the raw Steam ID shape BEFORE the
+    // `SteamID::toSteam2()` conversion. Pre-fix this surface called
+    // `toSteam2()` on the raw POST value as its first statement; on a
+    // garbage input the converter threw `Invalid SteamID input!` from
+    // `resolveInputID()`, the exception escaped the page handler
+    // unhandled, and the user got a generic 500 page render instead of
+    // the inline "Please enter a valid Steam ID or Community ID"
+    // message on the form. The library tightening (follow-up #1) made
+    // the throw stricter (rejecting the `STEAM_0:0:` / substring-bypass
+    // / embedded-Steam64 shapes the old loose regex used to round-trip
+    // verbatim into the DB) which made the 500 page render strictly
+    // MORE frequent on edit-ban. Validate-before-convert puts the
+    // failure on the form as the same per-field message admin-add /
+    // comms-add use.
+    //
+    // The validate-then-convert order also defends the `ip` branch:
+    // pre-fix the form's `Convert ban from IP to Steam ID` path
+    // legitimately passes an empty `steam` while the user fills `ip`
+    // — `toSteam2('')` returns `false` (the `empty()` short-circuit
+    // in `to()`), so this happened to work, but only as an accident
+    // of how the early-return interacts with `empty($_POST['steam'])`
+    // below. Keep the explicit-then-convert shape so a future
+    // refactor of `to()`'s early-return doesn't silently regress.
+    $rawSteam       = trim((string) ($_POST['steam'] ?? ''));
     $_POST['type']  = (int) ($_POST['type'] ?? 0);
     $postBanType    = BanType::tryFrom((int) $_POST['type']) ?? BanType::Steam;
 
     // Form Validation
     $error = 0;
-    // If they didn't type a steamid
-    if (empty($_POST['steam']) && $postBanType === BanType::Steam) {
-        $error++;
-        $validationErrors['steam'] = 'You must type a Steam ID or Community ID';
-    } elseif ($postBanType === BanType::Steam && !\SteamID\SteamID::isValidID($_POST['steam'])) {
-        $error++;
-        $validationErrors['steam'] = 'Please enter a valid Steam ID or Community ID';
-    } elseif (empty($_POST['ip']) && $postBanType === BanType::Ip) {
+    // Steam ID branch — validate raw shape FIRST; convert only on a
+    // pass. Empty input is its own error message; non-empty-but-bad
+    // is "please enter a valid…". The conversion is only meaningful
+    // on a Steam-type ban so the IP branch skips it.
+    if ($postBanType === BanType::Steam) {
+        if ($rawSteam === '') {
+            $error++;
+            $validationErrors['steam'] = 'You must type a Steam ID or Community ID';
+            $_POST['steam'] = '';
+        } elseif (!\SteamID\SteamID::isValidID($rawSteam)) {
+            $error++;
+            $validationErrors['steam'] = 'Please enter a valid Steam ID or Community ID';
+            // Re-emit the operator's raw input verbatim on the bounce
+            // so they see exactly what they typed and can correct the
+            // typo, instead of having the form silently clear.
+            $_POST['steam'] = $rawSteam;
+        } else {
+            // Convert ONLY after the shape gate passes. With the
+            // library tightening from follow-up #1 this call cannot
+            // throw — every input passing `isValidID()` resolves
+            // through the shared `ID_PATTERNS` table.
+            $_POST['steam'] = \SteamID\SteamID::toSteam2($rawSteam);
+        }
+    } else {
+        // IP-type ban — the steam column stays untouched on the
+        // write side (`type` = `BanType::Ip->value` and the
+        // ban_ip-only path below handles the DB binds), but the
+        // form re-emit still needs a string value so the input
+        // doesn't lose what the user typed if they later switch
+        // back to a Steam-type ban.
+        $_POST['steam'] = $rawSteam;
+    }
+
+    if ($error === 0 && empty($_POST['ip']) && $postBanType === BanType::Ip) {
         // Didn't type an IP
         $error++;
         $validationErrors['ip'] = 'You must type an IP';
-    } elseif ($postBanType === BanType::Ip && !filter_var($_POST['ip'], FILTER_VALIDATE_IP)) {
+    } elseif ($error === 0 && $postBanType === BanType::Ip && !filter_var($_POST['ip'], FILTER_VALIDATE_IP)) {
         $error++;
         $validationErrors['ip'] = 'You must type a valid IP';
     }
diff --git a/web/pages/admin.edit.comms.php b/web/pages/admin.edit.comms.php
index c99729d3e..70ec9b069 100644
--- a/web/pages/admin.edit.comms.php
+++ b/web/pages/admin.edit.comms.php
@@ -98,18 +98,43 @@
 $errorFields = [];
 
 if (isset($_POST['name'])) {
-    $_POST['steam'] = \SteamID\SteamID::toSteam2(trim((string) ($_POST['steam'] ?? '')));
+    // #1420 follow-up #2 — validate the raw Steam ID shape BEFORE the
+    // `SteamID::toSteam2()` conversion. Pre-fix this surface called
+    // `toSteam2()` on the raw POST value as its first statement; on a
+    // garbage input (the `STEAM_0:0:` empty-Z bypass, the
+    // `asdfSTEAM_0:0:123` substring-bypass, the
+    // `asdf 76561197960265728 garbage` embedded-Steam64 bypass — see
+    // `web/tests/integration/SteamIDValidationTest.php`) the converter
+    // threw `Invalid SteamID input!` from `resolveInputID()`, the
+    // exception escaped the page handler unhandled, and the user got
+    // a generic 500 page render instead of the inline "Please enter a
+    // valid Steam ID or Community ID" message on the form. The library
+    // tightening (follow-up #1) made the throw stricter which made the
+    // 500 page render strictly MORE frequent on edit-comms; the
+    // validate-before-convert order surfaces the failure on the form
+    // as the same per-field message comms-add uses.
+    $rawSteam       = trim((string) ($_POST['steam'] ?? ''));
     $_POST['type']  = (int) ($_POST['type'] ?? 0);
 
     // Form Validation
     $error = 0;
-    // If they didn't type a steamid
-    if (empty($_POST['steam'])) {
+    // Steam ID — validate raw shape FIRST; convert only on a pass.
+    if ($rawSteam === '') {
         $error++;
         $errorFields[] = ['steam.msg', 'You must type a Steam ID or Community ID'];
-    } elseif (!\SteamID\SteamID::isValidID($_POST['steam'])) {
+        $_POST['steam'] = '';
+    } elseif (!\SteamID\SteamID::isValidID($rawSteam)) {
         $error++;
         $errorFields[] = ['steam.msg', 'Please enter a valid Steam ID or Community ID'];
+        // Re-emit the operator's raw input verbatim on the bounce so
+        // they see exactly what they typed and can correct the typo.
+        $_POST['steam'] = $rawSteam;
+    } else {
+        // Convert ONLY after the shape gate passes. With the library
+        // tightening from follow-up #1 this call cannot throw — every
+        // input passing `isValidID()` resolves through the shared
+        // `ID_PATTERNS` table.
+        $_POST['steam'] = \SteamID\SteamID::toSteam2($rawSteam);
     }
 
     // Didn't type a custom reason
diff --git a/web/pages/page.submit.php b/web/pages/page.submit.php
index fa30297fd..cb7e05d90 100644
--- a/web/pages/page.submit.php
+++ b/web/pages/page.submit.php
@@ -48,7 +48,16 @@
     $SID           = (int) ($_POST['server']         ?? -1);
     $validsubmit   = true;
     $errors        = "";
-    if ((strlen($SteamID) != 0 && $SteamID != "STEAM_0:") && !\SteamID\SteamID::isValidID($SteamID)) {
+    // #1420 follow-up #2: the legacy `STEAM_0:` sentinel that the
+    // page handler used to re-emit when the SteamID was blank was
+    // dropped in the same commit that added the strict
+    // `pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"` attribute to
+    // the form input — the sentinel would have failed the pattern
+    // check AND blocked submission for the legitimate IP-only path.
+    // With the sentinel gone, `$SteamID === ''` is the canonical
+    // "no Steam ID typed" state and both server-side rules below
+    // (validate-shape + at-least-one-of) key off it.
+    if (strlen($SteamID) != 0 && !\SteamID\SteamID::isValidID($SteamID)) {
         $errors .= '* Please type a valid STEAM ID.<br>';
         $validsubmit = false;
     }
@@ -58,14 +67,8 @@
     }
     // #1207 PUB-4: at least one of Steam ID / IP must be provided.
     // Mirrors the inline guard in `page_submitban.tpl` so JS-off
-    // visitors can't sneak an empty pair past the form. The "STEAM_0:"
-    // sentinel is what the page handler re-emits when the user blanked
-    // the Steam ID after a previous bounce (see `STEAMID:` assignment
-    // around L293 below) — treat it as empty for this rule too.
-    if (
-        (strlen($SteamID) == 0 || $SteamID == "STEAM_0:")
-        && strlen($BanIP) == 0
-    ) {
+    // visitors can't sneak an empty pair past the form.
+    if (strlen($SteamID) == 0 && strlen($BanIP) == 0) {
         $errors .= '* Please enter a Steam ID or an IP address before submitting.<br>';
         $validsubmit = false;
     }
@@ -145,9 +148,13 @@
                 $mailserver = "Server: Other server\n";
                 $modid['mid']   = 0;
             }
-            if ($SteamID == "STEAM_0:") {
-                $SteamID = "";
-            }
+            // #1420 follow-up #2: the legacy `STEAM_0:` empty sentinel
+            // was dropped at the top of this handler — the operator-side
+            // shape is now strictly "empty string" for the no-Steam-ID
+            // path, and the strict `pattern="…"` on the form input plus
+            // the server-side `SteamID::isValidID()` gate above mean
+            // anything reaching here is either '' or a fully-formed
+            // SteamID. No defensive sentinel collapse needed.
             $GLOBALS['PDO']->query("INSERT INTO `:prefix_submissions`(submitted,SteamId,name,email,ModID,reason,ip,subname,sip,archiv,server) VALUES (UNIX_TIMESTAMP(),?,?,?,?,?,?,?,?,0,?)")->execute([
                 $SteamID,
                 $PlayerName,
@@ -245,7 +252,15 @@
 }
 
 \Sbpp\View\Renderer::render($theme, new \Sbpp\View\SubmitBanView(
-    STEAMID: $SteamID == "" ? "STEAM_0:" : $SteamID,
+    // #1420 follow-up #2: re-emit the raw input verbatim (or '' on the
+    // first-paint / IP-only path). The legacy `STEAM_0:` sentinel was
+    // pre-fill UX that broke the moment the template grew a strict
+    // `pattern="…"` attribute — the sentinel didn't match the regex
+    // and the browser blocked submission for legit IP-only flows.
+    // The form's `placeholder="STEAM_0:0:12345"` is the modern shape
+    // for "show the operator what we expect" without populating the
+    // input.
+    STEAMID: $SteamID,
     ban_ip: $BanIP,
     player_name: $PlayerName,
     ban_reason: $BanReason,
diff --git a/web/tests/integration/SteamIDValidationOrderTest.php b/web/tests/integration/SteamIDValidationOrderTest.php
new file mode 100644
index 000000000..d64dbdfc3
--- /dev/null
+++ b/web/tests/integration/SteamIDValidationOrderTest.php
@@ -0,0 +1,485 @@
+<?php
+// SourceBans++ (c) 2014-2026 SourceBans++ Dev Team
+// Licensed under Creative Commons Attribution-NonCommercial-ShareAlike 3.0.
+// See LICENSE.md for the full license text and THIRD-PARTY-NOTICES.txt for attributions.
+
+declare(strict_types=1);
+
+namespace Sbpp\Tests\Integration;
+
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Issue #1420 follow-up #2: page-handler form-POST surfaces that
+ * called `SteamID::toSteam2()` BEFORE validating the raw Steam ID
+ * shape via `SteamID::isValidID()`.
+ *
+ * The bug class is the same as the JSON handler side (#1420 proper),
+ * but the failure mode is different: where the JSON handlers turned
+ * the converter's exception into a 500-coded `{error: "unknown",
+ * message: "Invalid SteamID input!"}` envelope, the page handlers
+ * surfaced the exception as an uncaught PHP error, the panel chrome
+ * rendered a stack trace (display_errors=On) or a blank white page
+ * (display_errors=Off), the inline per-field error path NEVER ran,
+ * and the operator was stranded without any actionable feedback.
+ *
+ * The fix flips the order to "validate raw shape → only convert on
+ * pass". The shape gate is `SteamID::isValidID()`, which after
+ * follow-up #1 is strictly anchored against the shared `ID_PATTERNS`
+ * table. When the shape gate fails:
+ *
+ *   - `admin.edit.ban.php` pushes the error into `$validationErrors`
+ *     ('steam' key) and re-renders the form with the operator's raw
+ *     input preserved (Option B per AGENTS.md "Page-handler form-POST
+ *     surfaces" — picks up the existing pattern for empty / duplicate
+ *     SteamID).
+ *   - `admin.edit.comms.php` pushes the error into `$errorFields[]`
+ *     as the legacy ['steam.msg', '...'] tuple and re-renders (Option
+ *     B; matches the established pattern for missing-nickname / empty
+ *     SteamID).
+ *   - `admin.edit.admindetails.php` pushes the error into
+ *     `$validationErrors['steam']` and re-renders (Option B; matches
+ *     the established pattern for duplicate-name / empty-Steam ID).
+ *   - `page.submit.php` doesn't call `toSteam2()` at all — it only
+ *     validates via `isValidID()`. The library tightening in
+ *     follow-up #1 closes the bypass on this surface; the
+ *     paired template change adds `pattern="…"` so the browser
+ *     blocks submission pre-flight on a typo'd Steam ID.
+ *
+ * # Why static-shape tests (not process-isolated runtime tests)
+ *
+ * Pattern lifted from the sister `AdminEditCommsCheckOrderTest`
+ * (#1410) — which carries the same reasoning at length. Tl;dr:
+ *
+ * All four page handlers run inside the panel's chrome-render
+ * pipeline: validation → either re-render the form (calls
+ * `Renderer::render` → Smarty → `echo`) or run the write path →
+ * `PageDie()`. PHPUnit's `runInSeparateProcess` mode relies on the
+ * test method returning normally so the child process can serialise
+ * its result back; `exit;` mid-test breaks the serializer and PHPUnit
+ * reports the test as errored regardless of the actual assertion
+ * outcome.
+ *
+ * The static-shape pins below catch the bug directly: each test
+ * asserts that the `SteamID::isValidID(...)` call appears BEFORE the
+ * `SteamID::toSteam2(...)` call in the file's text. Reverse the order
+ * and the test fails before the page does in production.
+ *
+ * The runtime side of the contract is exercised end-to-end by the
+ * existing `comms-add SteamID validation` Playwright spec
+ * (`web/tests/e2e/specs/flows/comms-add-validation.spec.ts`) — the
+ * strict-regex contract is shared across the JSON + form surfaces, so
+ * the existing E2E coverage exercises the same library boundary the
+ * page handlers now route through.
+ */
+final class SteamIDValidationOrderTest extends TestCase
+{
+    /**
+     * @param string $relative Path relative to `web/`. Resolves against the
+     *                         test bootstrap's `ROOT` constant.
+     */
+    private function fileContents(string $relative): string
+    {
+        $contents = (string) @file_get_contents(ROOT . $relative);
+        $this->assertNotEmpty(
+            $contents,
+            "Expected `web/{$relative}` to exist and be readable; the regression guard is meaningless otherwise.",
+        );
+        return $contents;
+    }
+
+    /**
+     * The load-bearing assertion for `admin.edit.ban.php`. Pre-fix
+     * this handler called `\SteamID\SteamID::toSteam2(...)` as the
+     * FIRST statement inside `if (isset($_POST['name']))`; the
+     * converter raised `Exception('Invalid SteamID input!')` on any
+     * non-empty input that failed `resolveInputID()`'s shape check,
+     * the exception escaped the page handler unhandled, and the user
+     * got a 500 page render instead of the inline "Please enter a
+     * valid Steam ID or Community ID" message on the form. The fix
+     * inverts the order: validate raw shape via `isValidID()` first;
+     * convert only on a pass.
+     */
+    public function testAdminEditBanValidatesBeforeConverting(): void
+    {
+        $contents = $this->fileContents('pages/admin.edit.ban.php');
+
+        $isValidPos  = strpos($contents, '\\SteamID\\SteamID::isValidID(');
+        $toSteam2Pos = strpos($contents, '\\SteamID\\SteamID::toSteam2(');
+
+        $this->assertNotFalse(
+            $isValidPos,
+            'Expected `\\SteamID\\SteamID::isValidID(` call in admin.edit.ban.php — was the validate-before-convert fix reverted?',
+        );
+        $this->assertNotFalse(
+            $toSteam2Pos,
+            'Expected `\\SteamID\\SteamID::toSteam2(` call in admin.edit.ban.php — the conversion is still load-bearing for the DB write path.',
+        );
+
+        $this->assertLessThan(
+            $toSteam2Pos,
+            $isValidPos,
+            '#1420 follow-up #2: `SteamID::isValidID()` MUST be called BEFORE '
+                . '`SteamID::toSteam2()` in admin.edit.ban.php. Reversing the order is '
+                . 'the bug class: `toSteam2()` raises Exception("Invalid SteamID input!") '
+                . 'on any input that fails the shape check, the exception escapes the '
+                . 'page handler unhandled, and the operator gets a 500 page render '
+                . 'instead of the inline per-field "Please enter a valid Steam ID or '
+                . 'Community ID" message on the form.',
+        );
+    }
+
+    /**
+     * Same load-bearing assertion for `admin.edit.comms.php`. Pre-fix
+     * this handler also called `toSteam2()` as the first statement
+     * inside `if (isset($_POST['name']))`.
+     */
+    public function testAdminEditCommsValidatesBeforeConverting(): void
+    {
+        $contents = $this->fileContents('pages/admin.edit.comms.php');
+
+        $isValidPos  = strpos($contents, '\\SteamID\\SteamID::isValidID(');
+        $toSteam2Pos = strpos($contents, '\\SteamID\\SteamID::toSteam2(');
+
+        $this->assertNotFalse(
+            $isValidPos,
+            'Expected `\\SteamID\\SteamID::isValidID(` call in admin.edit.comms.php.',
+        );
+        $this->assertNotFalse(
+            $toSteam2Pos,
+            'Expected `\\SteamID\\SteamID::toSteam2(` call in admin.edit.comms.php.',
+        );
+
+        $this->assertLessThan(
+            $toSteam2Pos,
+            $isValidPos,
+            '#1420 follow-up #2: `SteamID::isValidID()` MUST be called BEFORE '
+                . '`SteamID::toSteam2()` in admin.edit.comms.php. See the docblock on '
+                . '`testAdminEditBanValidatesBeforeConverting` for the bug class.',
+        );
+    }
+
+    /**
+     * Same load-bearing assertion for `admin.edit.admindetails.php`.
+     * Pre-fix this handler called `toSteam2()` while building
+     * `$resolvedSteam` at the top of the POST branch; the converter's
+     * exception escaped through the same uncaught-exception path.
+     */
+    public function testAdminEditAdminDetailsValidatesBeforeConverting(): void
+    {
+        $contents = $this->fileContents('pages/admin.edit.admindetails.php');
+
+        $isValidPos  = strpos($contents, '\\SteamID\\SteamID::isValidID(');
+        $toSteam2Pos = strpos($contents, '\\SteamID\\SteamID::toSteam2(');
+
+        $this->assertNotFalse(
+            $isValidPos,
+            'Expected `\\SteamID\\SteamID::isValidID(` call in admin.edit.admindetails.php.',
+        );
+        $this->assertNotFalse(
+            $toSteam2Pos,
+            'Expected `\\SteamID\\SteamID::toSteam2(` call in admin.edit.admindetails.php.',
+        );
+
+        $this->assertLessThan(
+            $toSteam2Pos,
+            $isValidPos,
+            '#1420 follow-up #2: `SteamID::isValidID()` MUST be called BEFORE '
+                . '`SteamID::toSteam2()` in admin.edit.admindetails.php. See the docblock '
+                . 'on `testAdminEditBanValidatesBeforeConverting` for the bug class.',
+        );
+    }
+
+    /**
+     * `page.submit.php` is the public form. It NEVER calls
+     * `SteamID::toSteam2()` — it just validates via `isValidID()` and
+     * stores the raw value in `:prefix_submissions`. This test pins
+     * that asymmetry: the file must contain the `isValidID()` gate
+     * but must NOT regress to a `toSteam2()` call (a future
+     * "normalise the SteamID before storage" refactor would
+     * reintroduce the bug class because `toSteam2()` raises on
+     * invalid shape).
+     */
+    public function testPageSubmitUsesIsValidIdNotToSteam2(): void
+    {
+        $contents = $this->fileContents('pages/page.submit.php');
+
+        $this->assertStringContainsString(
+            '\\SteamID\\SteamID::isValidID(',
+            $contents,
+            '#1420 follow-up #2: page.submit.php must validate the Steam ID via '
+                . '`SteamID::isValidID()`. Removing the gate reopens the validation '
+                . 'bypass.',
+        );
+
+        $this->assertStringNotContainsString(
+            '\\SteamID\\SteamID::toSteam2(',
+            $contents,
+            '#1420 follow-up #2: page.submit.php must NOT call `SteamID::toSteam2()`. '
+                . 'The submission flow stores the raw user input in :prefix_submissions '
+                . 'verbatim; introducing a normalising conversion here would reintroduce '
+                . 'the bug class because `toSteam2()` raises Exception on invalid input '
+                . 'and the page handler has no try/catch surface. If normalisation is '
+                . 'genuinely needed in the future, wire it through a validate-then-convert '
+                . 'ladder like the admin.edit.* handlers do.',
+        );
+    }
+
+    /**
+     * The `STEAM_0:` sentinel that `page.submit.php` used to re-emit
+     * for empty Steam IDs was dropped in the same commit that added
+     * the strict `pattern="…"` attribute to the form input. Leaving
+     * the sentinel in place would have failed the pattern check AND
+     * blocked submission for the legitimate IP-only path. This test
+     * pins both halves of the contract: the page handler must NOT
+     * re-emit the sentinel, and the template must NOT carry the dead
+     * client-side `STEAM_0:` defense in its `isEmpty()` helper.
+     */
+    public function testPageSubmitDroppedSteamZeroSentinel(): void
+    {
+        $contents = $this->fileContents('pages/page.submit.php');
+
+        $this->assertStringNotContainsString(
+            '"STEAM_0:" : $SteamID',
+            $contents,
+            '#1420 follow-up #2: page.submit.php must NOT re-emit the `STEAM_0:` '
+                . 'sentinel for empty Steam IDs in the View constructor. The sentinel '
+                . 'was pre-fill UX that broke the moment the template grew a strict '
+                . '`pattern="STEAM_[01]:[01]:\\d+|…"` attribute — the partial sentinel '
+                . 'fails the pattern check and the browser blocks submission for legit '
+                . 'IP-only flows. The `placeholder="STEAM_0:0:12345"` on the template '
+                . 'is the modern shape for "show the operator what we expect".',
+        );
+
+        $this->assertStringNotContainsString(
+            '$SteamID == "STEAM_0:"',
+            $contents,
+            '#1420 follow-up #2: page.submit.php must NOT defensively collapse the '
+                . '`STEAM_0:` sentinel back to "" in the write path. With the sentinel '
+                . 'removed at the View construction, the input handler ensures '
+                . '`$SteamID === ""` by the time the write runs.',
+        );
+
+        $tpl = $this->fileContents('themes/default/page_submitban.tpl');
+        $this->assertStringNotContainsString(
+            "v === 'STEAM_0:'",
+            $tpl,
+            '#1420 follow-up #2: page_submitban.tpl must NOT carry the dead '
+                . '`STEAM_0:` empty-sentinel defense in its `isEmpty()` helper. The '
+                . 'sentinel was dropped from the page handler in the same commit; the '
+                . 'defense is unreachable code now and the comment misleads future '
+                . 'maintainers about the contract.',
+        );
+    }
+
+    /**
+     * Pin the strict `pattern="…"` attribute on each of the four
+     * Steam ID inputs across the page-handler form templates. The
+     * pattern mirrors the server-side `SteamID::isValidID()`
+     * allowlist (Steam2 / bracketed Steam3 / 17-digit Steam64) so the
+     * browser blocks submission pre-flight on a typo — the operator
+     * doesn't pay the round-trip.
+     *
+     * Anchored against the literal regex string so a future
+     * loosening that drops the `[01]` strict character class or
+     * widens the quantifier from `\d+` to `\d*` fails the gate.
+     */
+    public function testFormTemplatesCarryStrictSteamPattern(): void
+    {
+        $expected = 'pattern="STEAM_[01]:[01]:\\d+|\\[U:1:\\d+\\]|\\d{17}"';
+
+        $templates = [
+            'themes/default/page_admin_edit_ban.tpl',
+            'themes/default/page_admin_edit_comms.tpl',
+            'themes/default/page_admin_edit_admins_details.tpl',
+            'themes/default/page_submitban.tpl',
+        ];
+
+        foreach ($templates as $relative) {
+            $contents = $this->fileContents($relative);
+            $this->assertStringContainsString(
+                $expected,
+                $contents,
+                "#1420 follow-up #2: {$relative} must carry the strict Steam ID "
+                    . "pattern: `{$expected}`. The pattern mirrors the server-side "
+                    . "`SteamID::isValidID()` allowlist; loosening it (dropping `[01]` "
+                    . "for `[0-9]`, widening `\\d+` to `\\d*`, removing the anchors) "
+                    . "would reintroduce the substring-bypass class of #1420 on the "
+                    . "client side and shift the burden entirely to the server-side "
+                    . "library.",
+            );
+        }
+    }
+
+    /**
+     * Pin the `title="..."` companion attribute on each Steam ID
+     * input. `title` is what the browser surfaces on the
+     * pattern-mismatch popover; without it the popover reads
+     * "Please match the requested format." which gives the operator
+     * no actionable feedback — they have to read the page's help
+     * line to figure out what's wrong.
+     */
+    public function testFormTemplatesCarrySteamPatternTitle(): void
+    {
+        $expectedTitle = 'title="Enter a Steam ID (STEAM_0:1:23498765), Steam3 ID ([U:1:23498765]), or 17-digit SteamID64."';
+
+        $templates = [
+            'themes/default/page_admin_edit_ban.tpl',
+            'themes/default/page_admin_edit_comms.tpl',
+            'themes/default/page_admin_edit_admins_details.tpl',
+            'themes/default/page_submitban.tpl',
+        ];
+
+        foreach ($templates as $relative) {
+            $contents = $this->fileContents($relative);
+            $this->assertStringContainsString(
+                $expectedTitle,
+                $contents,
+                "#1420 follow-up #2: {$relative} must carry the actionable `title` "
+                    . "attribute on the Steam ID input. Without it the browser's "
+                    . "pattern-mismatch popover reads `Please match the requested "
+                    . "format.` which is useless to the operator.",
+            );
+        }
+    }
+
+    /**
+     * Pin that `page_submitban.tpl` does NOT carry `novalidate` on
+     * the form. Pre-#1420 the form had `novalidate` which suppressed
+     * native validation entirely — the strict `pattern="…"` added in
+     * this PR would never fire client-side. Removing `novalidate` is
+     * what makes the pattern attribute load-bearing.
+     */
+    public function testPageSubmitFormDoesNotCarryNovalidate(): void
+    {
+        $contents = $this->fileContents('themes/default/page_submitban.tpl');
+
+        // Slice the form tag — the `novalidate` attribute is only
+        // meaningful when it sits on the `<form>` element itself. A
+        // `novalidate` substring elsewhere in the template (in a
+        // comment, in a sibling helper) is harmless.
+        $formOpen = strpos($contents, '<form');
+        $formClose = $formOpen !== false ? strpos($contents, '>', $formOpen) : false;
+
+        $this->assertNotFalse($formOpen, 'Expected `<form` opener in page_submitban.tpl.');
+        $this->assertNotFalse($formClose, 'Expected matching `>` for the `<form` opener.');
+
+        $formTag = substr($contents, $formOpen, $formClose - $formOpen + 1);
+
+        $this->assertStringNotContainsString(
+            'novalidate',
+            $formTag,
+            '#1420 follow-up #2: the `<form>` tag in page_submitban.tpl must NOT '
+                . 'carry `novalidate`. `novalidate` suppresses native validation, '
+                . 'including the strict `pattern="…"` attribute on the Steam ID '
+                . 'input — so adding the pattern without removing `novalidate` would '
+                . 'be silently dead client-side. The cross-field "one of Steam ID OR '
+                . 'IP" guard that native HTML cannot express stays in the page-tail '
+                . 'JS, which runs AFTER native validation passes.',
+        );
+    }
+
+    /**
+     * `admin.bans.php`'s `importBans` POST handler reads the
+     * operator-uploaded `banned_user.cfg` file line by line and
+     * called `SteamID::toSteam2($line[2])` on the third whitespace
+     * token of every `banid …` line. Pre-fix a single malformed
+     * line (typo'd Steam ID, legacy format the library no longer
+     * accepts, garbage trailing tokens) raised
+     * `Exception('Invalid SteamID input!')` from `resolveInputID()`;
+     * the import aborted mid-file with a 500 page render and the
+     * operator had no signal as to which line broke or how many of
+     * the preceding inserts committed (no transaction wrapper).
+     *
+     * The library tightening in follow-up #1 made the throw stricter,
+     * which made the abort strictly MORE frequent for any cfg
+     * carrying legacy/typo'd rows. This test pins the
+     * validate-before-convert order so a future refactor doesn't
+     * silently regress the import to "fails-loudly on first bad line".
+     */
+    public function testAdminBansImportValidatesBeforeConverting(): void
+    {
+        $contents = $this->fileContents('pages/admin.bans.php');
+
+        $importStart = strpos($contents, "importBans");
+        $this->assertNotFalse(
+            $importStart,
+            'Expected the `importBans` POST branch in admin.bans.php — was the import surface deleted?',
+        );
+
+        // Slice the import branch out of the full file (the
+        // surrounding `admin.bans.php` carries many other
+        // `SteamID::*` references in sibling sections that we
+        // don\'t want to disturb).
+        $importEnd = strpos($contents, '}', $importStart);
+        // Walk up to the end of the importBans branch — the
+        // enclosing `if (isset($_POST[action]) ...)` block. The
+        // matching `}` is the FIRST top-level brace closer after the
+        // helper's last `echo "<script>...";` — slice generously
+        // and let the strpos checks below assert ordering inside.
+        $importBlock = substr($contents, $importStart, 5000);
+
+        $isValidPos  = strpos($importBlock, '\\SteamID\\SteamID::isValidID(');
+        $toSteam2Pos = strpos($importBlock, '\\SteamID\\SteamID::toSteam2(');
+
+        $this->assertNotFalse(
+            $isValidPos,
+            '#1420 follow-up #2: importBans MUST call `\\SteamID\\SteamID::isValidID(` to gate the `banid` branch before converting. Was the validate-before-convert fix reverted?',
+        );
+        $this->assertNotFalse(
+            $toSteam2Pos,
+            'Expected `\\SteamID\\SteamID::toSteam2(` in the importBans branch — the conversion is still load-bearing for the DB write path.',
+        );
+
+        $this->assertLessThan(
+            $toSteam2Pos,
+            $isValidPos,
+            '#1420 follow-up #2: in importBans, `SteamID::isValidID()` MUST be called '
+                . 'BEFORE `SteamID::toSteam2()`. Reversing the order is the bug class: '
+                . 'a single typo\'d Steam ID in the operator\'s banned_user.cfg aborts '
+                . 'the entire import with a 500 page render (the converter raises '
+                . 'Exception on invalid input, the exception escapes the page handler, '
+                . 'and the foreach inserts are not wrapped in a transaction so the '
+                . 'operator has no idea how many of the preceding lines committed).',
+        );
+    }
+
+    /**
+     * The per-handler `preg_match(...)` defense-in-depth pinned in
+     * the original #1420 PR was kept. Same load-bearing reasoning:
+     * the library tightening in follow-up #1 is the primary gate,
+     * the per-handler regex is the second line of defense that
+     * catches any regression in the library (or in any future
+     * caller that bypasses the library entirely). Both layers must
+     * agree on the allowlist; this test pins the "both still exist"
+     * half of the contract.
+     */
+    public function testJsonHandlersKeepDefenseInDepthRegex(): void
+    {
+        $handlers = [
+            'api/handlers/admins.php',
+            'api/handlers/bans.php',
+            'api/handlers/comms.php',
+        ];
+
+        foreach ($handlers as $relative) {
+            $contents = $this->fileContents($relative);
+            // The defense-in-depth `preg_match` calls live next to
+            // the `SteamID::isValidID()` calls and key off the
+            // exact same allowlist as the templates do. Anchoring
+            // on the strict character-class `[01]` is the cleanest
+            // proxy for "this is the tightened regex, not the
+            // legacy `[0-9]` one".
+            $this->assertMatchesRegularExpression(
+                '/preg_match\([^)]*STEAM_\[01\]:\[01\]/',
+                $contents,
+                "#1420 follow-up #1 + #2: {$relative} must keep its per-handler "
+                    . "`preg_match('/^STEAM_[01]:[01]:…/')` defense-in-depth gate. "
+                    . "The library tightening from follow-up #1 is the primary "
+                    . "validation layer, but the per-handler regex catches any future "
+                    . "library regression (or any caller that bypasses the library). "
+                    . "Dropping it weakens the JSON-handler safety net by one layer.",
+            );
+        }
+    }
+}
diff --git a/web/themes/default/page_admin_edit_admins_details.tpl b/web/themes/default/page_admin_edit_admins_details.tpl
index 481d149a4..044a20902 100644
--- a/web/themes/default/page_admin_edit_admins_details.tpl
+++ b/web/themes/default/page_admin_edit_admins_details.tpl
@@ -56,8 +56,20 @@
                     </div>
                     <div>
                         <label class="label" for="steam">Steam ID</label>
+                        {* #1420 follow-up #2 — strict `pattern` mirrors
+                            the server-side `SteamID::isValidID()`
+                            allowlist (Steam2 / bracketed Steam3 /
+                            17-digit Steam64). The handler validates
+                            raw input via `isValidID()` BEFORE calling
+                            `toSteam2()`. `required` matches the
+                            server-side rule that admins MUST carry a
+                            non-empty `authid`. *}
                         <input class="input font-mono" id="steam" name="steam" type="text"
-                               value="{$authid|escape}" data-testid="edit-admin-steam">
+                               value="{$authid|escape}" data-testid="edit-admin-steam"
+                               placeholder="STEAM_0:1:23498765"
+                               pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"
+                               title="Enter a Steam ID (STEAM_0:1:23498765), Steam3 ID ([U:1:23498765]), or 17-digit SteamID64."
+                               required aria-required="true">
                         <div id="steam.msg" class="text-xs" style="color:var(--danger);margin-top:0.25rem"></div>
                     </div>
                 </div>
diff --git a/web/themes/default/page_admin_edit_ban.tpl b/web/themes/default/page_admin_edit_ban.tpl
index 15c15a503..329dd918e 100644
--- a/web/themes/default/page_admin_edit_ban.tpl
+++ b/web/themes/default/page_admin_edit_ban.tpl
@@ -150,7 +150,23 @@
                        name="steam"
                        value="{$ban_authid|escape}"
                        data-testid="editban-steam"
-                       placeholder="STEAM_0:1:23498765">
+                       placeholder="STEAM_0:1:23498765"
+                       {* #1420 follow-up #2 — strict `pattern` mirrors
+                           the server-side `SteamID::isValidID()`
+                           allowlist (Steam2 / bracketed Steam3 /
+                           17-digit Steam64 — same as
+                           `page_admin_bans_add.tpl`). The handler
+                           validates raw input via `isValidID()`
+                           BEFORE calling `toSteam2()`. Edit-ban can
+                           target either a Steam ID (Type=0) or an IP
+                           (Type=1) so this input is NOT `required` —
+                           the page handler enforces "one of the two
+                           is non-empty" per `$postBanType`. When
+                           empty (the IP-target case) the browser's
+                           pattern check is satisfied (it only fires
+                           on non-empty values). *}
+                       pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"
+                       title="Enter a Steam ID (STEAM_0:1:23498765), Steam3 ID ([U:1:23498765]), or 17-digit SteamID64.">
                 <div class="text-xs mt-2"
                      id="steam.msg"
                      style="color:var(--danger);display:none"></div>
diff --git a/web/themes/default/page_admin_edit_comms.tpl b/web/themes/default/page_admin_edit_comms.tpl
index 2c9008fdb..2dfb29b25 100644
--- a/web/themes/default/page_admin_edit_comms.tpl
+++ b/web/themes/default/page_admin_edit_comms.tpl
@@ -56,7 +56,21 @@
                    id="steam"
                    name="steam"
                    value="{$ban_authid|escape}"
-                   data-testid="editcomm-steam">
+                   data-testid="editcomm-steam"
+                   {* #1420 follow-up #2 — strict `pattern` mirrors the
+                       server-side `SteamID::isValidID()` allowlist
+                       (Steam2 / bracketed Steam3 / 17-digit Steam64
+                       — same as `page_admin_comms_add.tpl`). The
+                       handler validates raw input via `isValidID()`
+                       BEFORE calling `toSteam2()`, but the pattern
+                       gate stops the round-trip on a typo. `title`
+                       is what the browser surfaces on the popover
+                       when the pattern fails — keep it short and
+                       actionable. *}
+                   pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"
+                   title="Enter a Steam ID (STEAM_0:1:23498765), Steam3 ID ([U:1:23498765]), or 17-digit SteamID64."
+                   required
+                   aria-required="true">
             <div class="text-xs mt-2" id="steam.msg" style="color:var(--danger);display:none"></div>
         </div>
 
diff --git a/web/themes/default/page_submitban.tpl b/web/themes/default/page_submitban.tpl
index 79d74f872..d6c48ee16 100644
--- a/web/themes/default/page_submitban.tpl
+++ b/web/themes/default/page_submitban.tpl
@@ -36,12 +36,20 @@
         </p>
     </header>
 
+    {*
+        #1420 follow-up #2: `novalidate` was dropped so the native
+        `required` / `pattern` validation gates fire on submit. The
+        cross-field "one of Steam ID OR IP" check that native HTML
+        can't express stays in the inline JS at the bottom — it runs
+        AFTER native validation passes. The server-side rules in
+        `page.submit.php` remain the source of truth for JS-off
+        visitors and any client bypass.
+    *}
     <form class="card"
           method="post"
           enctype="multipart/form-data"
           action="index.php?p=submit"
-          aria-labelledby="submitban-heading"
-          novalidate>
+          aria-labelledby="submitban-heading">
         {csrf_field}
         <input type="hidden" name="subban" value="1">
 
@@ -89,6 +97,21 @@
                         Player&rsquo;s Steam ID
                         <span class="text-muted text-xs" style="font-weight:400">(or use the IP below)</span>
                     </label>
+                    {*
+                        #1420 follow-up #2: strict `pattern` mirrors the
+                        server-side `SteamID::isValidID()` allowlist
+                        (Steam2 / bracketed Steam3 / 17-digit Steam64
+                        — same as `page_admin_comms_add.tpl` /
+                        `page_admin_bans_add.tpl`). The input is
+                        intentionally NOT `required` — this surface
+                        accepts EITHER a Steam ID OR an IP, and the
+                        either/or guard is enforced by the page-tail
+                        script + the server-side rule in
+                        `page.submit.php`. When the input is empty
+                        HTML5 `pattern` validation passes (it only
+                        fires on non-empty values), so the cross-field
+                        guard stays in JS as before.
+                    *}
                     <input type="text"
                            class="input font-mono"
                            id="submitban-steam"
@@ -96,6 +119,8 @@
                            maxlength="64"
                            value="{$STEAMID}"
                            placeholder="STEAM_0:1:23498765"
+                           pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"
+                           title="Enter a Steam ID (STEAM_0:1:23498765), Steam3 ID ([U:1:23498765]), or 17-digit SteamID64."
                            autocomplete="off"
                            aria-describedby="submitban-id-or-ip-help"
                            data-testid="submitban-steam">
@@ -315,12 +340,15 @@
     }
 
     function isEmpty(/** @type {HTMLInputElement} */ el) {
-        var v = (el.value || '').trim();
-        // STEAM_0: is the placeholder-ish value the page handler
-        // re-emits after a failed validation; treat it as empty so
-        // the client-side check matches the server-side rule
-        // (page.submit.php does `$SteamID == "STEAM_0:" -> ""`).
-        return v === '' || v === 'STEAM_0:';
+        // #1420 follow-up #2: the legacy `STEAM_0:` empty-sentinel
+        // collapse was dropped here when the page handler stopped
+        // re-emitting it (see the `SubmitBanView` constructor in
+        // `page.submit.php` for the matching change). The strict
+        // `pattern="…"` on the input now rejects partial sentinels
+        // pre-submit, and the server-side `SteamID::isValidID()`
+        // gate (tightened in follow-up #1) closes the path for any
+        // bypass that gets through.
+        return (el.value || '').trim() === '';
     }
 
     /** @type {(ev: Event) => void} */

From 82e8c3d29a4092088936399aec83e8df5c11263b Mon Sep 17 00:00:00 2001
From: rumblefrog <contact@zhenyangli.me>
Date: Tue, 19 May 2026 19:21:32 -0400
Subject: [PATCH 5/9] fix(steamid): canonicalise valid Steam IDs on IP-type
 bans (#1420 follow-up #2 nit)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The previous commit's `admin.edit.ban.php` reshape had one
behavioral asymmetry with pre-tighter-library shape: for IP-type
bans (`$postBanType === BanType::Ip`), the pre-fix code ran
`SteamID::toSteam2($rawSteam)` unconditionally — so a Steam ID
typed in the secondary input always landed in `:authid` in
canonical Steam2 form. The validate-before-convert reshape skipped
the conversion for IP-type bans entirely, which:

  - preserved raw input on invalid garbage (the post-tightening
    safe behavior — `toSteam2()` would now throw on bad input),
    AND
  - silently dropped the canonicalisation on VALID inputs (a
    user typing `[U:1:12345]` for an IP-type ban would land
    `:authid = '[U:1:12345]'` in the DB instead of the canonical
    `STEAM_0:1:12345`).

Split the IP-type branch into "valid input → canonicalise"
(matches pre-tighter behavior for valid inputs) and "empty /
invalid → preserve raw" (the new safe behavior for garbage).
The branch is gated on `isValidID()` so the `toSteam2()` call
cannot throw.

`:authid = ''` for IP-type bans is the cleaner long-term shape
(matches `api_bans_add`'s explicit `':authid' => ''` for type=1),
but flipping the column write to that contract is a separate
schema-cleanup follow-up — pre-fix the page handler stored
whatever the user happened to type, and matching that
unconditionally is the least surprising behavior for #1420.

No new tests needed — the existing static-shape guard
(`SteamIDValidationOrderTest`) still pins the call-order
contract, and the existing e2e suite (passes with `workers=1`
matching CI) exercises both the Steam-type and IP-type form
paths.

Refs #1420
---
 web/pages/admin.edit.ban.php | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/web/pages/admin.edit.ban.php b/web/pages/admin.edit.ban.php
index 33ba66b58..76908bc04 100644
--- a/web/pages/admin.edit.ban.php
+++ b/web/pages/admin.edit.ban.php
@@ -141,8 +141,16 @@ function emitEditBanToastAndRedirect(string $kind, string $title, string $body,
     $error = 0;
     // Steam ID branch — validate raw shape FIRST; convert only on a
     // pass. Empty input is its own error message; non-empty-but-bad
-    // is "please enter a valid…". The conversion is only meaningful
-    // on a Steam-type ban so the IP branch skips it.
+    // is "please enter a valid…". The validation only fires on a
+    // Steam-type ban; the IP branch skips it but still canonicalises
+    // a fully-valid Steam ID if one was typed (pre-fix behavior
+    // preserved — the UPDATE binds `:authid = $_POST['steam']`
+    // regardless of `$postBanType`, so what the user typed in the
+    // Steam ID field still lands in the `authid` column for an
+    // IP-type ban, just unvalidated. Keep the canonicalisation here
+    // so the column is consistent with whatever the user typed
+    // pre-tightening; revisit if `:authid = ''` for type=1 ever
+    // becomes a separate schema cleanup follow-up).
     if ($postBanType === BanType::Steam) {
         if ($rawSteam === '') {
             $error++;
@@ -162,13 +170,20 @@ function emitEditBanToastAndRedirect(string $kind, string $title, string $body,
             // through the shared `ID_PATTERNS` table.
             $_POST['steam'] = \SteamID\SteamID::toSteam2($rawSteam);
         }
+    } elseif ($rawSteam !== '' && \SteamID\SteamID::isValidID($rawSteam)) {
+        // IP-type ban with a valid Steam ID typed in the secondary
+        // input — canonicalise to preserve pre-tighter-library
+        // behavior (the UPDATE writes `:authid` regardless of
+        // `$postBanType`; pre-fix this branch ran `toSteam2()`
+        // unconditionally so the column always landed in canonical
+        // form when the input was valid).
+        $_POST['steam'] = \SteamID\SteamID::toSteam2($rawSteam);
     } else {
-        // IP-type ban — the steam column stays untouched on the
-        // write side (`type` = `BanType::Ip->value` and the
-        // ban_ip-only path below handles the DB binds), but the
-        // form re-emit still needs a string value so the input
-        // doesn't lose what the user typed if they later switch
-        // back to a Steam-type ban.
+        // IP-type ban with empty / invalid Steam ID input — preserve
+        // raw value so the form re-emit doesn't silently clear what
+        // the operator typed. The IP branch is the load-bearing
+        // validation on this code path; whatever happens to be in
+        // `$_POST['steam']` rides along into the write.
         $_POST['steam'] = $rawSteam;
     }
 

From 2f1fe61ee288fe774e9f5ebf54947f9d2492a7a6 Mon Sep 17 00:00:00 2001
From: rumblefrog <contact@zhenyangli.me>
Date: Tue, 19 May 2026 20:12:14 -0400
Subject: [PATCH 6/9] fix(steamid): unify handler regex + write empty authid on
 IP-type bans (#1420 review follow-up)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Second-round review on PR #1423 surfaced two interleaved gaps the
first-round fixes didn't fully close. Both ride one commit because
`api/handlers/bans.php` carries the fix surface for both — the
Steam-branch regex unification AND the IP-type empty-authid contract
share the same `api_bans_add` Steam ID handling block.

## #1 — Single source of truth for the handler regex

The first-round fix kept hand-rolled `preg_match` literals at each
handler call site (admins.php / bans.php / comms.php) as "defense in
depth" against the library. But the three copies subtly DIFFERED from
the library's `ID_PATTERNS` (and from each other) on the `D` modifier
— without it, `STEAM_0:0:1\n` matches the handler's `^…$` anchors
(because `$` accepts end-of-string OR just-before-final-`\n` in the
default mode) and then fails the library's `SteamID::isValidID()`
(which DOES carry `D` post-#1423 follow-up #1), triggering
`SteamID::toSteam2()` to throw, which the dispatcher's `Throwable`
fallback wraps as a generic 500 envelope.

That's precisely the bug class #1420 was supposed to close — surfaced
on the substring-bypass surface (`asdf 76561197960265728 garbage`),
but the same shape recurs every time the two layers drift.

Lift to a single library constant `SteamID::HANDLER_STRICT_REGEX` and
have the handlers consume it directly. The constant's docblock spells
out the contract: byte-for-byte symmetry with the form template's
`pattern` attribute, the load-bearing `D` modifier, and the
deliberate asymmetry with `ID_PATTERNS` (bracketless Steam3 `U:1:N`
stays a library-side convenience but is rejected at the handler gate
to match the form's `pattern="…|\[U:1:\d+\]|\d{17}"` which requires
the brackets).

Regression coverage:
- `SteamIDValidationTest::testHandlerStrictRegexIsExposedAndCarriesDModifier`
  — pins the constant's existence + the `D` modifier requirement at
  the library level.
- `SteamIDValidationTest::testHandlerStrictRegexAgreesWithIdPatternsOnAcceptableShapes`
  — every shape the strict regex accepts must also pass the library's
  `isValidID()` + convert without throwing (handler gate <= library
  gate).
- `SteamIDValidationTest::testHandlerStrictRegexRejectsBracketlessSteam3`
  — pins the documented asymmetry (library accepts `U:1:N`, handler
  rejects it for form-pattern symmetry).
- `SteamIDValidationTest::testHandlerStrictRegexRejectsNewlineBypass`
  — explicit `STEAM_0:0:1\n` / `[U:1:1]\n` / `76561197960265728\n`
  rejection at the regex AND the library `isValidID()` level.
- `SteamIDValidationOrderTest::testJsonHandlersUseSingleSourceOfTruthRegex`
  — replaces the old "defense-in-depth hand-rolled regex literal"
  assertion with the single-source-of-truth call site contract
  (`preg_match(SteamID::HANDLER_STRICT_REGEX, …)`).

## #2 — IP-type ban writes empty authid

The first-round nit (`82e8c3d2`) "canonicalised valid Steam IDs on
IP-type bans to match pre-tighter behaviour". The "pre-tighter
behaviour" itself was a bug — the page handler wrote whatever the
user typed in the Steam ID input into `:authid` regardless of the
ban being IP-typed. The nit fix preserved the canonical-on-valid
case but failed to suppress the raw-on-invalid case — `garbage` in
the Steam ID input on an IP-type ban still landed in `:authid`
verbatim.

The JSON API handler (`api_bans_add`) and the page handler
(`admin.edit.ban.php`) also disagreed:
  - api: `$steam = $rawSteam === '' ? '' : SteamID::toSteam2($rawSteam);`
    — conversion ran on EVERY non-empty input regardless of `$banType`,
    so `type=1&steam=garbage&ip=1.2.3.4` ran `toSteam2('garbage')` →
    `Exception('Invalid SteamID input!')` → `Api::handle` `Throwable`
    fallback → 500 envelope (the bug class #1420 was supposed to
    close, surfacing on the IP-type branch the original review didn't
    cover).
  - page: stored the raw value verbatim (no canonicalization, no
    rejection) regardless of validity.

The correct contract is "IP-type bans have no Steam ID, so `:authid`
gets the schema's `NOT NULL DEFAULT ''` empty string regardless of
whatever happened to be in the Steam ID input." Both halves of the
handler converge on `$steam = $banType === BanType::Ip ? '' : …`
(api) and `$_POST['steam'] = ''` (page handler) for the IP branch.

The form-side input remains visible on the bounce path (the template
re-emits `$rawSteam` for the operator to correct), but the DB write
is divorced from it.

Regression coverage:
- `BansTest::testAddIpTypeAlwaysWritesEmptyAuthid` — three subtests:
  valid Steam-shape (must store empty, not the canonicalised form),
  garbage (must NOT 500), trailing-newline bypass (must NOT 500 even
  before regex-level rejection).

## Documentation

`AGENTS.md` updated with three blocks:
- The `SteamID inputs` convention section's example code block now
  references `SteamID::HANDLER_STRICT_REGEX` instead of the hand-
  rolled literal, with a paragraph documenting the constant's
  contract (`D` modifier, form-pattern symmetry, documented
  asymmetry with `ID_PATTERNS`).
- The regression-coverage section enumerates every new test under
  `SteamIDValidationTest.php` and the order test.
- New anti-pattern entry: "Hand-rolling the strict SteamID-shape
  regex literal at the per-handler `preg_match` call site"
  documenting why the `D` modifier is load-bearing and how the
  pre-fix drift produced the 500 envelope.
- New anti-pattern entry: "Storing the operator-typed Steam ID in
  `:prefix_bans.authid` on an IP-type ban" documenting the schema
  contract and the converged page-handler + API write paths.
---
 AGENTS.md                                     | 196 ++++++++++++++++--
 web/api/handlers/admins.php                   |   2 +-
 web/api/handlers/bans.php                     |  19 +-
 web/api/handlers/comms.php                    |   2 +-
 web/includes/SteamID/SteamID.php              |  38 ++++
 web/pages/admin.edit.ban.php                  |  49 ++---
 web/tests/api/BansTest.php                    |  85 ++++++++
 .../SteamIDValidationOrderTest.php            |  44 ++--
 .../integration/SteamIDValidationTest.php     | 186 +++++++++++++++++
 9 files changed, 559 insertions(+), 62 deletions(-)

diff --git a/AGENTS.md b/AGENTS.md
index 1e735c024..2a212de41 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -706,23 +706,34 @@ of the diff ship together or not at all.
   if ($rawSteam === '') {
       throw new ApiError('validation', 'You must type a Steam ID or Community ID', 'steam');
   }
-  if (!preg_match('/^(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17})$/', $rawSteam)) {
+  if (!preg_match(SteamID::HANDLER_STRICT_REGEX, $rawSteam)) {
       throw new ApiError('validation', 'Please enter a valid Steam ID or Community ID', 'steam');
   }
   $steam = SteamID::toSteam2($rawSteam);
   ```
 
-  See `api_comms_add` / `api_bans_add` / `api_admins_add` for the
-  canonical reference shape — all three handlers share the same
-  regex byte-for-byte so a future caller (deep-link, JSON client,
-  context-menu handoff) only has to learn one accepted shape. The
-  client-side native validation in the corresponding form template
-  uses the SAME regex via `pattern="(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17})"`
-  (HTML's `pattern` attribute is implicitly anchored `^…$`, so the
-  PHP regex carries explicit `^…$`); the browser-native popover is
-  the UX-first gate that fires BEFORE the IIFE calls `sb.api.call`;
-  the server-side `preg_match` is the load-bearing security gate
-  for curl-driven / third-party-theme callers that bypass it.
+  `SteamID::HANDLER_STRICT_REGEX` (#1423 follow-up #4) is the single
+  source of truth the per-handler `preg_match` calls consume — see
+  `api_comms_add` / `api_bans_add` / `api_admins_add` for the
+  canonical reference shape. The constant's docblock spells out the
+  contract: byte-for-byte symmetry with the form template's
+  `pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"`, the load-bearing
+  `D` modifier (without it `STEAM_0:0:1\n` slips past the gate and
+  500s on `toSteam2()`), and the deliberate asymmetry with
+  `ID_PATTERNS` (bracketless Steam3 `U:1:N` is excluded from the
+  handler gate — it stays a library-side convenience for the
+  conversion path but isn't an accepted panel-input shape because
+  the form template's `pattern` doesn't accept it either). Don't
+  hand-roll a copy of the regex literal at the handler call site —
+  the pre-#1423-follow-up-#4 hand-rolled copies silently missed the
+  `D` modifier, producing the newline-bypass class. The client-side
+  native validation in the corresponding form template uses the
+  matching `pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"` (HTML's
+  `pattern` attribute is implicitly anchored `^…$`, so the PHP
+  regex carries explicit `^…$`); the browser-native popover is the
+  UX-first gate that fires BEFORE the IIFE calls `sb.api.call`; the
+  server-side `preg_match` is the load-bearing security gate for
+  curl-driven / third-party-theme callers that bypass it.
 
   The pre-#1420 `SteamID::isValidID($raw)` was structurally unsafe
   — the bundled helper's regexes were unanchored with loose character
@@ -763,9 +774,28 @@ of the diff ship together or not at all.
   doesn't re-open the bypass class). Regression coverage for both
   layers: `web/tests/integration/SteamIDValidationTest.php` pins
   the library's shape contract (every bypass above is asserted as
-  rejected); `web/tests/api/CommsTest.php::testAddRejectsInvalidSteamIdShape`
+  rejected, the new `HANDLER_STRICT_REGEX` constant's `D`-modifier
+  + bracketless-Steam3-rejection contract is pinned by
+  `testHandlerStrictRegexIsExposedAndCarriesDModifier` /
+  `testHandlerStrictRegexAgreesWithIdPatternsOnAcceptableShapes` /
+  `testHandlerStrictRegexRejectsBracketlessSteam3` /
+  `testHandlerStrictRegexRejectsNewlineBypass`, the OpenID
+  regex tightening in `SteamAuthHandler` is pinned by
+  `testSteamAuthHandlerOpenIdRegexAcceptsOnly17DigitsStartingWith7`,
+  and the install wizard's regex is pinned by
+  `testInstallWizardSteamIdRegexCarriesDModifier`);
+  `web/tests/api/CommsTest.php::testAddRejectsInvalidSteamIdShape`
   + `BansTest.php` + `AdminsTest.php` pin the handler-side
-  `ApiError('validation', …, 'steam')` envelope.
+  `ApiError('validation', …, 'steam')` envelope (each test now
+  includes the `STEAM_0:0:1\n` / `[U:1:1]\n` / `76561197960265728\n`
+  newline-bypass cases as #1423 follow-up #4 regression guards);
+  `web/tests/api/BansTest.php::testAddIpTypeAlwaysWritesEmptyAuthid`
+  pins the IP-type empty-`authid` contract (valid Steam input +
+  garbage + newline-bypass all write empty); the kickit / blockit
+  `SteamID::compare()` pre-`isValidID()` gate is pinned by
+  `KickitTest::testKickPlayerReturnsNotFoundForMalformedSteamId` /
+  `testKickPlayerReturnsNotFoundForMalformedIp` and
+  `BlockitTest::testBlockPlayerReturnsNotFoundForMalformedSteamId`.
 
   **Page-handler form-POST surfaces** (`page.submit.php`,
   `admin.edit.{ban,comms,admindetails}.php`,
@@ -785,6 +815,20 @@ of the diff ship together or not at all.
      the operator's raw input on the bounce so they see exactly
      what they typed and can correct the typo without re-typing
      everything else).
+   - `admin.edit.ban.php` ON IP-TYPE bans hard-codes
+     `$_POST['steam'] = ''` regardless of whatever the operator
+     typed in the Steam ID field — the column is the *steam id*
+     of the banned player; on an IP-type ban there is no steam id,
+     so the canonical value is the schema's `NOT NULL default ''`
+     empty string (matching `api_bans_add`'s same-PR fix). The
+     pre-#1423-follow-up-#4 shape (the `82e8c3d2` "canonicalise on
+     IP-type" nit) preserved the canonical-on-valid case but
+     failed to suppress the raw-on-invalid case AND continued
+     writing the canonicalised SteamID into `:authid` for IP-only
+     bans — both shapes are wrong. The form-side input remains
+     visible on the IP-type bounce path (re-emit through the
+     template `placeholder`, NOT a stale value), but the DB write
+     is divorced from it.
    - `page.submit.php` doesn't call `SteamID::toSteam2()` at all —
      it stores the raw user-input verbatim in `:prefix_submissions`
      and the moderation queue resolves the canonical form on
@@ -798,7 +842,23 @@ of the diff ship together or not at all.
      left the operator with no signal as to which line broke or
      how many of the preceding inserts committed (no transaction
      wrapper). The success toast now carries the skipped-line
-     count alongside the imported count.
+     count alongside the imported count. Note: no transaction
+     wrapper is in place — partial commits are still possible.
+     The skipped-count surfaces in the success toast string so the
+     operator gets actionable feedback; full atomic-import work
+     (transaction + per-line audit trail) is a sister follow-up.
+   - `SteamID::compare($a, $b)` (used by `api_kickit_kick_player`
+     / `api_blockit_block_player` to per-player-match A2S
+     responses) routes through `toSteam64()` and throws on
+     invalid input. Pre-#1423-follow-up-#4 the iframe handlers
+     reached `compare()` with the operator-controlled `$check`
+     value unvalidated — a hostile caller (or a typo'd deep-link
+     URL) reliably 500'd the iframe; the loop renderer had no way
+     to tell "no match" apart from "your input was garbage".
+     Gate `$check` with `SteamID::isValidID()` (Steam-type) or
+     `filter_var(FILTER_VALIDATE_IP)` (IP-type) BEFORE the
+     `compare()` call and surface the structured `not_found`
+     envelope the iframe expects on the fail branch.
 
   The validate-before-convert order is THE structural contract:
   call `SteamID::isValidID($raw)` first, surface the error on the
@@ -3147,6 +3207,112 @@ contacting every contributor individually.
   cannot-throw guarantee in handler code, so wrapping it in
   `try/catch` is a code smell signalling the upstream gate is
   missing.
+- Hand-rolling the strict SteamID-shape regex literal at the
+  per-handler `preg_match` call site (the pre-#1423-follow-up-#4
+  shape: `preg_match('/^(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17})$/', $raw)`
+  — note the missing `D` modifier) → use the single source of
+  truth `SteamID::HANDLER_STRICT_REGEX`. The pre-#1423-follow-up-#4
+  copies silently missed the `D` modifier; the input
+  `STEAM_0:0:1\n` (or any newline-suffixed shape) matched the
+  per-handler regex (`$` matches end-of-string OR a final `\n`
+  without the modifier) but then FAILED the library's
+  `SteamID::isValidID()` / `toSteam2()` (which DO carry the
+  modifier post-#1423 follow-up #1), throwing
+  `Exception('Invalid SteamID input!')` → `Api::handle`
+  `Throwable` fallback → generic 500 envelope. The bug class
+  #1420 was supposed to close re-opened by accident on the
+  newline shape because the two gate layers drifted on the
+  modifier set. The `HANDLER_STRICT_REGEX` constant guarantees
+  byte-for-byte sync; a hand-rolled local copy at a future
+  handler call site silently invites the same bug. Regression
+  guard:
+  `web/tests/integration/SteamIDValidationOrderTest.php::testJsonHandlersUseSingleSourceOfTruthRegex`
+  (asserts every JSON handler invokes `preg_match(SteamID::HANDLER_STRICT_REGEX, …)`
+  literally — not a copy, not a concatenation),
+  `web/tests/integration/SteamIDValidationTest.php::testHandlerStrictRegexRejectsNewlineBypass`
+  (asserts the newline shape is REJECTED at the gate),
+  `web/tests/api/CommsTest.php::testAddRejectsInvalidSteamIdShape`
+  + `BansTest.php` + `AdminsTest.php` (the
+  `"STEAM_0:0:1\n"` / `"[U:1:1]\n"` / `"76561197960265728\n"`
+  cases pin the wire-side behavior).
+- Storing the operator-typed Steam ID in `:prefix_bans.authid`
+  on an IP-type ban (the `82e8c3d2` "canonicalise valid IDs on
+  IP-type bans to match pre-tighter behaviour" nit shape) → the
+  `:authid` column is the *steam id* of the banned player. On
+  an IP-type ban (`BanType::Ip = 1`) there is no steam id, by
+  definition; the canonical "no steam id" value is the schema's
+  `NOT NULL default ''` empty string. The `82e8c3d2` nit aimed
+  to preserve "pre-tightening behavior" for the case where the
+  operator typed a valid-looking SteamID into the form's
+  Steam ID box and then flipped the type radio to "IP-type" —
+  pre-tightening the unconditional `toSteam2($rawSteam)` would
+  have stored the canonicalised value in `:authid` AND the
+  unconditional run would have raised on `garbage` and 500'd
+  the page. The nit canonicalised the valid case (good
+  intention) but didn't suppress the storage path (the bug it
+  carried), AND failed to defend the invalid-input branch on
+  the IP-type arm (the 500 was still reachable via
+  `?type=1&steam=garbage`). The right shape is: hard-code
+  `$_POST['steam'] = ''` for `$banType === BanType::Ip` and
+  let the operator's form input remain visible only as
+  client-side echo (template `placeholder`, NOT a stale value).
+  Matches the `api_bans_add` write-side fix in the same PR
+  (`$steam = $banType === BanType::Ip ? '' : ...`). The
+  asymmetry pre-#1423-follow-up-#4 (page handler stored
+  canonicalised, JSON handler stored empty) was its own bug
+  class — third-party callers POSTing to the iframe-routed
+  page handler vs. the JSON dispatcher produced inconsistent
+  DB state for the same logical input. Regression guard:
+  `web/tests/api/BansTest.php::testAddIpTypeAlwaysWritesEmptyAuthid`
+  (valid Steam input + garbage + newline-bypass all write
+  empty `authid` on `type=1`).
+- Calling `SteamID::compare($a, $b)` (or any other
+  `SteamID::*` method that funnels through `toSteam64()` /
+  `resolveInputID()`) with operator-controlled input that
+  hasn't been gated through `SteamID::isValidID()` first → same
+  bug class as the JSON handler convert-before-validate trap,
+  different blast radius. `compare()` is used by
+  `api_kickit_kick_player` and `api_blockit_block_player` to
+  per-player-match the A2S `status` response against the
+  operator's target; on any input that fails the library shape
+  gate the `toSteam64()` call throws and the iframe's loop
+  renderer gets a generic 500 envelope instead of the
+  structured `not_found` envelope that says "no match found
+  for that target". The iframe can't tell the two apart, and a
+  hostile caller posting `?check=garbage&type=0` reliably 500s
+  the panel. The fix is a `SteamID::isValidID($check)` gate
+  (Steam-type) or `filter_var($check, FILTER_VALIDATE_IP)`
+  gate (IP-type) BEFORE the `compare()` call site, returning
+  the structured `not_found` envelope on the fail branch.
+  Pinned by
+  `web/tests/api/KickitTest.php::testKickPlayerReturnsNotFoundForMalformedSteamId`
+  + `testKickPlayerReturnsNotFoundForMalformedIp` and
+  `web/tests/api/BlockitTest.php::testBlockPlayerReturnsNotFoundForMalformedSteamId`.
+- Tightening `SteamAuthHandler::validate()`'s OpenID-claimed-ID
+  regex to anything OTHER than `7\d{16}+` with the `D`
+  modifier (e.g. the pre-#1423-follow-up-#4 shape
+  `7[0-9]{15,25}+` without `D`) → Steam in practice always
+  returns a 17-digit Steam64 in the `openid_claimed_id` URL,
+  and the panel's downstream `SteamID::toSteam2()` carries
+  the library-side `^\d{17}$D` gate post-#1423 follow-up #1
+  (the symmetry contract). If the regex here loosens (accepts
+  16 digits or 18-25 digits, or drops the `D` modifier and
+  accepts a trailing `\n`), the input slips past `validate()`
+  but then fails the library's gate in `check()`'s
+  `toSteam2()` call — the exception escapes the constructor
+  unhandled and the operator lands on a 500 mid-Steam-login
+  round-trip (silent failure mode — there's no `try/catch`
+  here and the chrome's `PageDie()` doesn't run on a callback
+  redirect). The 17-digit shape is the contract Steam is on
+  record committing to (their OpenID 2.0 endpoint hasn't
+  emitted any other shape since 2010); a future Steam-side
+  change that emits a different shape surfaces here as a
+  clean false return (operator sees the
+  `m=steam_failed` redirect) instead of as a 500. The defense-
+  in-depth `SteamID::isValidID()` gate in `check()` is the
+  second line — both halves of the contract ship together.
+  Pinned by
+  `web/tests/integration/SteamIDValidationTest.php::testSteamAuthHandlerOpenIdRegexAcceptsOnly17DigitsStartingWith7`.
 - Editing `install/includes/sql/data.sql` (or `struc.sql`) without a paired
   `web/updater/data/<N>.php` → upgraded installs silently miss the change.
 - WYSIWYG / "rich HTML" editors (TinyMCE, CKEditor, …) for fields stored
diff --git a/web/api/handlers/admins.php b/web/api/handlers/admins.php
index 0fb84a6d8..a04d3bd73 100644
--- a/web/api/handlers/admins.php
+++ b/web/api/handlers/admins.php
@@ -185,7 +185,7 @@ function api_admins_add(array $params): array
     // matches `SteamID::isValidID`'s unanchored substring regex and
     // `toSteam2()` then emits a negative-Z-component canonical form
     // into `:prefix_admins.authid`).
-    if (!preg_match('/^(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17})$/', $rawSteam)) {
+    if (!preg_match(SteamID::HANDLER_STRICT_REGEX, $rawSteam)) {
         throw new ApiError('validation', 'Please enter a valid Steam ID or Community ID.', 'steam');
     }
     $steam = SteamID::toSteam2($rawSteam);
diff --git a/web/api/handlers/bans.php b/web/api/handlers/bans.php
index 4be35cde4..369719b1d 100644
--- a/web/api/handlers/bans.php
+++ b/web/api/handlers/bans.php
@@ -76,15 +76,22 @@ function api_bans_add(array $params): array
         if ($rawSteam === '') {
             throw new ApiError('validation', 'You must type a Steam ID or Community ID', 'steam');
         }
-        if (!preg_match('/^(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17})$/', $rawSteam)) {
+        if (!preg_match(SteamID::HANDLER_STRICT_REGEX, $rawSteam)) {
             throw new ApiError('validation', 'Please enter a valid Steam ID or Community ID', 'steam');
         }
     }
-    // Skip the conversion entirely for IP-typed bans (rawSteam is
-    // empty for those rows by definition; calling toSteam2 on an
-    // empty string would hit the same `resolveInputID` throw the
-    // guard above defends against).
-    $steam = $rawSteam === '' ? '' : SteamID::toSteam2($rawSteam);
+    // For IP-typed bans the `:authid` column is the *steam id*, of which
+    // there is none — write empty string regardless of whatever the
+    // caller passed in `$rawSteam`. Pre-#1423 follow-up #4 the handler
+    // converted any non-empty `$rawSteam` here without re-running the
+    // shape gate (which was Steam-branch-only), so a hostile / typo'd
+    // caller passing `type=1&steam=garbage&ip=1.2.3.4` triggered
+    // `toSteam2('garbage')` → `Exception('Invalid SteamID input!')` →
+    // `Api::handle` `Throwable` fallback → 500 envelope (the bug class
+    // #1420 was supposed to close, surfacing on the IP-type branch the
+    // original review didn't cover). The page-handler sibling
+    // (`admin.edit.ban.php`) carries the matching write-side fix.
+    $steam = $banType === BanType::Ip ? '' : ($rawSteam === '' ? '' : SteamID::toSteam2($rawSteam));
     if (empty($ip) && $banType === BanType::Ip) {
         throw new ApiError('validation', 'You must type an IP', 'ip');
     }
diff --git a/web/api/handlers/comms.php b/web/api/handlers/comms.php
index d0f1e9ea5..391f881ac 100644
--- a/web/api/handlers/comms.php
+++ b/web/api/handlers/comms.php
@@ -58,7 +58,7 @@ function api_comms_add(array $params): array
     if ($rawSteam === '') {
         throw new ApiError('validation', 'You must type a Steam ID or Community ID', 'steam');
     }
-    if (!preg_match('/^(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17})$/', $rawSteam)) {
+    if (!preg_match(SteamID::HANDLER_STRICT_REGEX, $rawSteam)) {
         throw new ApiError('validation', 'Please enter a valid Steam ID or Community ID', 'steam');
     }
     $steam = SteamID::toSteam2($rawSteam);
diff --git a/web/includes/SteamID/SteamID.php b/web/includes/SteamID/SteamID.php
index 0c2179cae..e88dd07f9 100644
--- a/web/includes/SteamID/SteamID.php
+++ b/web/includes/SteamID/SteamID.php
@@ -140,6 +140,44 @@ private static function to($format, $steamid)
         ['/^\d{17}$/D',              'Steam64'],
     ];
 
+    /**
+     * Strict allowlist regex consumed by the per-handler defense-in-depth
+     * gate in `web/api/handlers/{admins,bans,comms}.php` (and any future
+     * caller that wants the "what the form's `pattern` attribute accepts"
+     * shape without depending on the form template). Single source of
+     * truth so the library and the handlers cannot drift on the accepted
+     * shape — pre-#1423 follow-up #4 the handlers carried hand-rolled
+     * copies that subtly differed (no `D` modifier → `STEAM_0:0:1\n`
+     * newline-bypass slipped past the handler regex but failed the
+     * library's `isValidID()` and threw `Exception('Invalid SteamID
+     * input!')` from `toSteam2()`, which the dispatcher's `Throwable`
+     * fallback wrapped as a generic `server_error` 500 envelope —
+     * exactly the bug class #1420 was supposed to close).
+     *
+     * The shape is TIGHTER than `ID_PATTERNS` on one axis: the bracketless
+     * Steam3 form (`U:1:N`) is INTENTIONALLY excluded so the gate matches
+     * the form template's `pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"`
+     * byte-for-byte. Curl-driven callers get the same shape contract a
+     * form user sees on the pattern-mismatch popover; bracketless Steam3
+     * shape stays a library-side convenience for the conversion path
+     * (`SteamID::toSteam2('U:1:1')` still works) but isn't an accepted
+     * panel-input shape.
+     *
+     * The `D` modifier is load-bearing: without it `STEAM_0:0:1\n`
+     * matches and the input then fails the library's `isValidID()` (which
+     * carries the modifier), causing `toSteam2()` to throw on the
+     * conversion the handler runs immediately after. The dispatcher
+     * wraps the exception as a generic 500 — the operator gets neither
+     * the inline validation message NOR the structured `validation` API
+     * envelope, just a "something went wrong" page render.
+     *
+     * @see ID_PATTERNS for the wider library-accepted shape table.
+     * @see `web/tests/integration/SteamIDValidationTest.php::testHandlerStrictRegexAgreesWithIdPatternsOnAcceptableShapes`
+     *      for the cross-validation contract that pins the
+     *      ID_PATTERNS / HANDLER_STRICT_REGEX relationship.
+     */
+    public const HANDLER_STRICT_REGEX = '/^(?:STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17})$/D';
+
     /**
      * @param  $steamid
      * @return string
diff --git a/web/pages/admin.edit.ban.php b/web/pages/admin.edit.ban.php
index 76908bc04..fbd435dc9 100644
--- a/web/pages/admin.edit.ban.php
+++ b/web/pages/admin.edit.ban.php
@@ -141,16 +141,24 @@ function emitEditBanToastAndRedirect(string $kind, string $title, string $body,
     $error = 0;
     // Steam ID branch — validate raw shape FIRST; convert only on a
     // pass. Empty input is its own error message; non-empty-but-bad
-    // is "please enter a valid…". The validation only fires on a
-    // Steam-type ban; the IP branch skips it but still canonicalises
-    // a fully-valid Steam ID if one was typed (pre-fix behavior
-    // preserved — the UPDATE binds `:authid = $_POST['steam']`
-    // regardless of `$postBanType`, so what the user typed in the
-    // Steam ID field still lands in the `authid` column for an
-    // IP-type ban, just unvalidated. Keep the canonicalisation here
-    // so the column is consistent with whatever the user typed
-    // pre-tightening; revisit if `:authid = ''` for type=1 ever
-    // becomes a separate schema cleanup follow-up).
+    // is "please enter a valid…".
+    //
+    // The IP-type branch writes `:authid = ''` regardless of whatever
+    // happened to be in the Steam ID input. The column is the *steam
+    // id* of the banned player; on an IP-type ban there is no steam
+    // id, so the canonical value is the schema's `NOT NULL default ''`
+    // empty string — matching the API handler (`api_bans_add`'s
+    // INTENT pre-#1423 follow-up #4, fully enforced after that PR)
+    // and matching the v1.x library's "throw on garbage" outcome
+    // (pre-tightening the unconditional `toSteam2($rawSteam)` would
+    // have raised on `garbage` and the page died with a 500; storing
+    // user-typed garbage into `:authid` is a regression introduced
+    // by the 82e8c3d2 "canonicalise valid IDs on IP-type bans" nit
+    // that preserved the canonical-on-valid case but failed to
+    // suppress the raw-on-invalid case). The form-side input remains
+    // visible on the IP-type bounce path (re-emit through the
+    // `placeholder` on the template, NOT a stale value), but the DB
+    // write is divorced from it.
     if ($postBanType === BanType::Steam) {
         if ($rawSteam === '') {
             $error++;
@@ -170,21 +178,14 @@ function emitEditBanToastAndRedirect(string $kind, string $title, string $body,
             // through the shared `ID_PATTERNS` table.
             $_POST['steam'] = \SteamID\SteamID::toSteam2($rawSteam);
         }
-    } elseif ($rawSteam !== '' && \SteamID\SteamID::isValidID($rawSteam)) {
-        // IP-type ban with a valid Steam ID typed in the secondary
-        // input — canonicalise to preserve pre-tighter-library
-        // behavior (the UPDATE writes `:authid` regardless of
-        // `$postBanType`; pre-fix this branch ran `toSteam2()`
-        // unconditionally so the column always landed in canonical
-        // form when the input was valid).
-        $_POST['steam'] = \SteamID\SteamID::toSteam2($rawSteam);
     } else {
-        // IP-type ban with empty / invalid Steam ID input — preserve
-        // raw value so the form re-emit doesn't silently clear what
-        // the operator typed. The IP branch is the load-bearing
-        // validation on this code path; whatever happens to be in
-        // `$_POST['steam']` rides along into the write.
-        $_POST['steam'] = $rawSteam;
+        // IP-type ban: clear the steam slot for the DB write below
+        // (`:authid = $_POST['steam']` rides this). Whatever the
+        // operator happened to type in the Steam ID field stays
+        // visible in `$rawSteam` for any client-side echo, but it
+        // does NOT land in the DB. See the block comment above for
+        // the schema rationale.
+        $_POST['steam'] = '';
     }
 
     if ($error === 0 && empty($_POST['ip']) && $postBanType === BanType::Ip) {
diff --git a/web/tests/api/BansTest.php b/web/tests/api/BansTest.php
index 344cd4931..396d22a78 100644
--- a/web/tests/api/BansTest.php
+++ b/web/tests/api/BansTest.php
@@ -115,6 +115,25 @@ public function testAddRejectsInvalidSteamIdShapeForType0(): void
                 'asdfSTEAM_0:0:123',               // substring-bypass (unanchored)
                 'asdf 76561197960265728 garbage',  // embedded Steam64 — pre-fix corrupted
                 'U:1:1',                           // bracketless Steam3 — form pattern requires brackets
+                // #1423 follow-up #4 — bypass shapes that `trim()`
+                // upstream of the gate does NOT defend against. The
+                // common trailing-newline case `"STEAM_0:0:1\n"` is
+                // stripped by `trim()` before the regex sees it —
+                // that path is fine, and the library-level
+                // `HANDLER_STRICT_REGEX` test pins the regex's
+                // newline-bypass rejection directly
+                // (`SteamIDValidationTest::testHandlerStrictRegexRejectsNewlineBypass`).
+                // The cases below are what the handler-side gate
+                // actually has to catch — mid-string `\n` (not
+                // strippable) and unicode whitespace like NBSP
+                // (`trim()` only handles the ASCII whitespace set
+                // `\t\n\v\f\r ` + null + the spec-listed printable
+                // whitespace). The strict gate's `^…$` anchors +
+                // non-`m` mode require the whole string match the
+                // pattern; embedded `\n` is rejected.
+                "STEAM_0:0:1\nGARBAGE",
+                "GARBAGE\nSTEAM_0:0:1",
+                "STEAM_0:0:1\xC2\xA0",             // trailing NBSP (U+00A0); `trim` does NOT strip
             ] as $badSteam
         ) {
             $env = $this->api('bans.add', [
@@ -146,6 +165,72 @@ public function testAddValidationInvalidIpForType1(): void
         $this->assertSame('ip', $env['error']['field']);
     }
 
+    /**
+     * #1423 follow-up #4 — pre-fix the IP-type ban path had two bugs:
+     *
+     *   1. Stored whatever the caller passed in `$rawSteam` into
+     *      `:prefix_bans.authid` if it happened to be a valid shape
+     *      (the `$rawSteam === '' ? '' : SteamID::toSteam2($rawSteam)`
+     *      ternary ignored `$banType`). On an IP-type ban the
+     *      `:authid` column is the *steam id*, of which there is
+     *      none — the schema's `NOT NULL default ''` is the
+     *      canonical "no steam id" value.
+     *   2. Raised a generic 500 (`SteamID::toSteam2('garbage')` →
+     *      `Exception('Invalid SteamID input!')` → `Api::handle`
+     *      `Throwable` fallback) when the caller's `$rawSteam` was
+     *      non-empty AND failed the library shape gate. The
+     *      Steam-branch `preg_match` doesn't run on IP-type bans
+     *      (its `if ($banType === BanType::Steam)` short-circuits),
+     *      so a hostile caller posting `type=1&steam=garbage`
+     *      reliably 500'd the panel.
+     *
+     * The fix hard-codes `$steam = ''` for `$banType === BanType::Ip`
+     * — whatever the caller passed in `$rawSteam` is irrelevant on
+     * an IP-type ban; the column is empty by definition.
+     */
+    public function testAddIpTypeAlwaysWritesEmptyAuthid(): void
+    {
+        $this->loginAsAdmin();
+        // 1) Valid Steam-shape `steam` with `type=1` (IP-type ban).
+        //    Pre-fix this would have canonicalised + stored
+        //    `STEAM_0:1:9999` in `:authid`; the fix writes empty.
+        $env = $this->api('bans.add', [
+            'nickname' => 'IpType_validSteam', 'type' => 1,
+            'steam' => 'STEAM_0:1:9999', 'ip' => '203.0.113.42',
+            'length' => 0, 'reason' => 'ip ban with steam input',
+            'fromsub' => 0,
+        ]);
+        $this->assertTrue($env['ok'], 'IP-type ban should succeed regardless of steam input');
+        $row = $this->row('bans', ['ip' => '203.0.113.42']);
+        $this->assertSame('', (string) $row['authid'], 'IP-type ban must write empty authid');
+        $this->assertSame('1', (string) $row['type']);
+
+        // 2) Garbage `steam` with `type=1`. Pre-fix this raised
+        //    `Exception('Invalid SteamID input!')` → 500 envelope.
+        //    Post-fix the IP-type branch never reaches the converter.
+        $env = $this->api('bans.add', [
+            'nickname' => 'IpType_garbageSteam', 'type' => 1,
+            'steam' => 'garbage', 'ip' => '203.0.113.43',
+            'length' => 0, 'reason' => 'ip ban with garbage steam input',
+            'fromsub' => 0,
+        ]);
+        $this->assertTrue($env['ok'], 'IP-type ban must NOT 500 on garbage steam input');
+        $row = $this->row('bans', ['ip' => '203.0.113.43']);
+        $this->assertSame('', (string) $row['authid']);
+
+        // 3) Newline-bypass `steam` with `type=1`. Same shape, different
+        //    expected outcome: the IP-type branch ignores it entirely.
+        $env = $this->api('bans.add', [
+            'nickname' => 'IpType_newlineSteam', 'type' => 1,
+            'steam' => "STEAM_0:0:1\n", 'ip' => '203.0.113.44',
+            'length' => 0, 'reason' => 'ip ban with newline steam input',
+            'fromsub' => 0,
+        ]);
+        $this->assertTrue($env['ok']);
+        $row = $this->row('bans', ['ip' => '203.0.113.44']);
+        $this->assertSame('', (string) $row['authid']);
+    }
+
     public function testAddValidationNegativeLength(): void
     {
         $this->loginAsAdmin();
diff --git a/web/tests/integration/SteamIDValidationOrderTest.php b/web/tests/integration/SteamIDValidationOrderTest.php
index d64dbdfc3..0f0ebc3fe 100644
--- a/web/tests/integration/SteamIDValidationOrderTest.php
+++ b/web/tests/integration/SteamIDValidationOrderTest.php
@@ -453,8 +453,19 @@ public function testAdminBansImportValidatesBeforeConverting(): void
      * caller that bypasses the library entirely). Both layers must
      * agree on the allowlist; this test pins the "both still exist"
      * half of the contract.
+     *
+     * Post-#1423 follow-up #4 the regex is sourced from
+     * `SteamID::HANDLER_STRICT_REGEX` (a class constant on the
+     * library) instead of three hand-rolled copies — so the two
+     * layers cannot drift on the modifier set. Pre-#1423 follow-up
+     * #4 the hand-rolled handler regexes silently missed the `D`
+     * modifier; the `STEAM_0:0:1\n` newline-bypass slipped past
+     * the handler gate AND THEN failed `SteamID::toSteam2()`
+     * (which DOES carry the modifier post-#1423 follow-up #1) →
+     * generic 500 envelope. The single-source-of-truth shape pinned
+     * here closes that bug class permanently.
      */
-    public function testJsonHandlersKeepDefenseInDepthRegex(): void
+    public function testJsonHandlersUseSingleSourceOfTruthRegex(): void
     {
         $handlers = [
             'api/handlers/admins.php',
@@ -464,21 +475,24 @@ public function testJsonHandlersKeepDefenseInDepthRegex(): void
 
         foreach ($handlers as $relative) {
             $contents = $this->fileContents($relative);
-            // The defense-in-depth `preg_match` calls live next to
-            // the `SteamID::isValidID()` calls and key off the
-            // exact same allowlist as the templates do. Anchoring
-            // on the strict character-class `[01]` is the cleanest
-            // proxy for "this is the tightened regex, not the
-            // legacy `[0-9]` one".
-            $this->assertMatchesRegularExpression(
-                '/preg_match\([^)]*STEAM_\[01\]:\[01\]/',
+            $this->assertStringContainsString(
+                'SteamID::HANDLER_STRICT_REGEX',
+                $contents,
+                "#1423 follow-up #4: {$relative} MUST source its strict-regex gate "
+                    . "from `SteamID::HANDLER_STRICT_REGEX` (the library's class constant), "
+                    . "NOT a hand-rolled copy of the regex literal. Single-source-of-truth "
+                    . "is the contract that prevents the two layers from drifting on "
+                    . "modifier-set tweaks (the pre-#1423 hand-rolled regexes missed the "
+                    . "`D` modifier, causing the `STEAM_0:0:1\\n` newline-bypass to slip "
+                    . "past the handler gate AND THEN 500-envelope on `toSteam2()`).",
+            );
+            $this->assertStringContainsString(
+                'preg_match(SteamID::HANDLER_STRICT_REGEX',
                 $contents,
-                "#1420 follow-up #1 + #2: {$relative} must keep its per-handler "
-                    . "`preg_match('/^STEAM_[01]:[01]:…/')` defense-in-depth gate. "
-                    . "The library tightening from follow-up #1 is the primary "
-                    . "validation layer, but the per-handler regex catches any future "
-                    . "library regression (or any caller that bypasses the library). "
-                    . "Dropping it weakens the JSON-handler safety net by one layer.",
+                "#1423 follow-up #4: {$relative} must invoke the library constant via "
+                    . "`preg_match(SteamID::HANDLER_STRICT_REGEX, ...)`. The class constant "
+                    . "is the contract; a sibling shape (`SteamID::HANDLER_STRICT_REGEX . '…'` "
+                    . "concatenation, or a copy into a local variable) defeats the purpose.",
             );
         }
     }
diff --git a/web/tests/integration/SteamIDValidationTest.php b/web/tests/integration/SteamIDValidationTest.php
index 4a8c2f07a..05bc0907c 100644
--- a/web/tests/integration/SteamIDValidationTest.php
+++ b/web/tests/integration/SteamIDValidationTest.php
@@ -342,4 +342,190 @@ public function testIsValidIdAcceptsIntInput(): void
             restore_error_handler();
         }
     }
+
+    /**
+     * #1423 follow-up #4 — `SteamID::HANDLER_STRICT_REGEX` is the
+     * single source the per-handler `preg_match` gates consume in
+     * `web/api/handlers/{admins,bans,comms}.php`. Pin three contracts:
+     *
+     *   1. The regex exists as a public class constant (so handlers
+     *      can reach for it without a runtime lookup).
+     *   2. The regex carries the `D` modifier (so `STEAM_0:0:1\n`
+     *      is rejected at the gate before reaching `toSteam2()`).
+     *      Pre-fix the handler regex was a hand-rolled copy that
+     *      drifted from `ID_PATTERNS` on the `D` modifier — the
+     *      newline-suffixed input slipped past the handler, then
+     *      failed `toSteam2()`, then the exception escaped via
+     *      `Api::handle`'s `Throwable` fallback as a generic 500.
+     *   3. The regex is TIGHTER than `ID_PATTERNS` on one axis:
+     *      bracketless Steam3 (`U:1:N`) is INTENTIONALLY excluded
+     *      so the handler gate matches the form template's
+     *      `pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"`
+     *      byte-for-byte. Curl-driven callers get the same shape
+     *      contract a form user sees on the pattern-mismatch popover.
+     */
+    public function testHandlerStrictRegexIsExposedAndCarriesDModifier(): void
+    {
+        $reflection = new ReflectionClass(SteamID::class);
+        $this->assertTrue(
+            $reflection->hasConstant('HANDLER_STRICT_REGEX'),
+            'SteamID::HANDLER_STRICT_REGEX must exist — the per-handler `preg_match` gates source it instead of carrying hand-rolled copies that drift on the modifier set.',
+        );
+        $regex = $reflection->getReflectionConstant('HANDLER_STRICT_REGEX')->getValue();
+        $this->assertIsString($regex);
+        $this->assertStringStartsWith('/^', $regex, 'HANDLER_STRICT_REGEX must be ^…$ anchored.');
+        $this->assertMatchesRegularExpression(
+            '~\$/[a-zA-Z]*D[a-zA-Z]*$~',
+            $regex,
+            'HANDLER_STRICT_REGEX must carry the D modifier — without it, `STEAM_0:0:1\n` matches and then fails `toSteam2()` via the library\'s stricter gate (the newline-bypass class).',
+        );
+    }
+
+    /**
+     * #1423 follow-up #4 — every shape `HANDLER_STRICT_REGEX` accepts
+     * must ALSO be accepted by `ID_PATTERNS` (so the library's
+     * `toSteam2()` doesn't throw on inputs the handler blessed). The
+     * reverse is NOT required — `ID_PATTERNS` accepts bracketless
+     * Steam3 (`U:1:N`) while `HANDLER_STRICT_REGEX` deliberately does
+     * not (form-pattern symmetry).
+     *
+     * @return iterable<string, array{0: string}>
+     */
+    public static function handlerStrictAccepts(): iterable
+    {
+        yield 'STEAM_0:0:11101'        => ['STEAM_0:0:11101'];
+        yield 'STEAM_1:1:11101'        => ['STEAM_1:1:11101'];
+        yield 'STEAM_0:0:0'            => ['STEAM_0:0:0'];
+        yield '[U:1:22202]'            => ['[U:1:22202]'];
+        yield '[U:1:0]'                => ['[U:1:0]'];
+        yield '76561197960265728'      => ['76561197960265728'];
+        yield '76561197960287930'      => ['76561197960287930'];
+    }
+
+    /**
+     * Every shape `HANDLER_STRICT_REGEX` accepts must also be accepted
+     * by `SteamID::isValidID()` — otherwise the handler converts but
+     * the library's downstream gate rejects, and we're back to 500-
+     * envelope territory.
+     */
+    #[DataProvider('handlerStrictAccepts')]
+    public function testHandlerStrictRegexAgreesWithIdPatternsOnAcceptableShapes(string $input): void
+    {
+        $this->assertSame(
+            1,
+            preg_match(SteamID::HANDLER_STRICT_REGEX, $input),
+            sprintf('HANDLER_STRICT_REGEX must accept %s', var_export($input, true)),
+        );
+        $this->assertTrue(
+            SteamID::isValidID($input),
+            sprintf('SteamID::isValidID() must accept %s (handler gate <= library gate)', var_export($input, true)),
+        );
+        $this->assertNotFalse(
+            SteamID::toSteam2($input),
+            sprintf('SteamID::toSteam2() must convert %s without throwing', var_export($input, true)),
+        );
+    }
+
+    /**
+     * Documented asymmetry: bracketless Steam3 (`U:1:N`) is accepted
+     * by `ID_PATTERNS` for library-side convenience (the panel's
+     * conversion call sites can still hand it to `toSteam2()`) but
+     * REJECTED by `HANDLER_STRICT_REGEX` for symmetry with the form
+     * template's `pattern` attribute. Pin the asymmetry so a future
+     * refactor that unifies the two regexes silently degrades the
+     * gate.
+     */
+    public function testHandlerStrictRegexRejectsBracketlessSteam3(): void
+    {
+        $this->assertTrue(
+            SteamID::isValidID('U:1:22202'),
+            'Library MUST accept bracketless Steam3 (conversion-path convenience).',
+        );
+        $this->assertSame(
+            0,
+            preg_match(SteamID::HANDLER_STRICT_REGEX, 'U:1:22202'),
+            'HANDLER_STRICT_REGEX MUST reject bracketless Steam3 (form-pattern symmetry).',
+        );
+    }
+
+    /**
+     * Newline-bypass cases the handler regex MUST reject (the bug
+     * `HANDLER_STRICT_REGEX`'s `D` modifier was lifted to close).
+     * Without the modifier the `$` anchor matches end-of-string OR
+     * just-before-final-`\n`; the newline-suffixed value slips past
+     * the handler gate and then 500s on `SteamID::toSteam2()`.
+     *
+     * @return iterable<string, array{0: string}>
+     */
+    public static function newlineBypasses(): iterable
+    {
+        yield "STEAM_0:0:1\\n"     => ["STEAM_0:0:1\n"];
+        yield "STEAM_1:0:1\\n"     => ["STEAM_1:0:1\n"];
+        yield "[U:1:1]\\n"         => ["[U:1:1]\n"];
+        yield "76561197960265728\\n" => ["76561197960265728\n"];
+    }
+
+    #[DataProvider('newlineBypasses')]
+    public function testHandlerStrictRegexRejectsNewlineBypass(string $input): void
+    {
+        $this->assertSame(
+            0,
+            preg_match(SteamID::HANDLER_STRICT_REGEX, $input),
+            sprintf('HANDLER_STRICT_REGEX must reject newline-suffixed input %s', var_export($input, true)),
+        );
+        $this->assertFalse(
+            SteamID::isValidID($input),
+            sprintf('SteamID::isValidID() must also reject %s (both halves of the contract)', var_export($input, true)),
+        );
+    }
+
+    /**
+     * #1423 follow-up #4 — `SteamAuthHandler::validate()` regex
+     * tightened from `7[0-9]{15,25}+` to `7\d{16}+` to match the
+     * library's strict 17-digit gate. Pin the new shape with positive
+     * + negative cases so a future tweak that loosens the OpenID regex
+     * (e.g. accommodating a hypothetical future Steam-side change)
+     * also tightens the library's `\d{17}` arm in the same PR.
+     */
+    public function testSteamAuthHandlerOpenIdRegexAcceptsOnly17DigitsStartingWith7(): void
+    {
+        $pattern = '/^https:\\/\\/steamcommunity\\.com\\/openid\\/id\\/(7\\d{16}+)$/D';
+        // 17-digit Steam64 starting with 7 — accepted
+        $this->assertSame(1, preg_match($pattern, 'https://steamcommunity.com/openid/id/76561197960265728'));
+        $this->assertSame(1, preg_match($pattern, 'https://steamcommunity.com/openid/id/76561197960287930'));
+        // 16 digits (one short) — rejected (pre-fix the `15,25` bound accepted)
+        $this->assertSame(0, preg_match($pattern, 'https://steamcommunity.com/openid/id/7656119796026572'));
+        // 18 digits (one over) — rejected (pre-fix the `15,25` bound accepted)
+        $this->assertSame(0, preg_match($pattern, 'https://steamcommunity.com/openid/id/765611979602657281'));
+        // Doesn't start with 7 — rejected
+        $this->assertSame(0, preg_match($pattern, 'https://steamcommunity.com/openid/id/86561197960265728'));
+        // Trailing newline — rejected via D modifier
+        $this->assertSame(0, preg_match($pattern, "https://steamcommunity.com/openid/id/76561197960265728\n"));
+        // Trailing garbage — rejected via $ anchor
+        $this->assertSame(0, preg_match($pattern, 'https://steamcommunity.com/openid/id/76561197960265728?nope'));
+    }
+
+    /**
+     * #1423 follow-up #4 — install wizard's regex in
+     * `web/install/pages/page.5.php` was tightened with the `D`
+     * modifier in the same PR. The wizard's gate is stricter than
+     * `HANDLER_STRICT_REGEX` (Steam2 form only — the initial admin
+     * is created with a STEAM_ ID; no Steam3 / Steam64 forms accepted
+     * at install time per the legacy contract). Pin the shape +
+     * modifier so a future wizard rewrite cannot silently regress
+     * the newline-bypass surface.
+     */
+    public function testInstallWizardSteamIdRegexCarriesDModifier(): void
+    {
+        $pattern = '/^STEAM_[01]:[01]:[0-9]+$/D';
+        // Accepted shapes
+        $this->assertSame(1, preg_match($pattern, 'STEAM_0:0:11101'));
+        $this->assertSame(1, preg_match($pattern, 'STEAM_1:1:11101'));
+        // Rejected shapes (modifier-sensitive)
+        $this->assertSame(0, preg_match($pattern, "STEAM_0:0:11101\n"), 'D modifier rejects trailing newline');
+        $this->assertSame(0, preg_match($pattern, 'STEAM_2:0:11101'), '[01] rejects universe 2');
+        $this->assertSame(0, preg_match($pattern, 'STEAM_0:2:11101'), '[01] rejects Y=2');
+        $this->assertSame(0, preg_match($pattern, '[U:1:11101]'), 'wizard is Steam2-only');
+        $this->assertSame(0, preg_match($pattern, '76561197960265728'), 'wizard rejects Steam64');
+    }
 }

From da583d31c78b671090ab551fc5426fd90944f467 Mon Sep 17 00:00:00 2001
From: rumblefrog <contact@zhenyangli.me>
Date: Tue, 19 May 2026 20:12:32 -0400
Subject: [PATCH 7/9] fix(steamid): pre-validate inputs to SteamID::compare in
 kickit / blockit (#1420 review follow-up)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`api_kickit_kick_player` and `api_blockit_block_player` iterate the
A2S `GetPlayers` list and call `SteamID::compare($a, $b)` to match
the operator-supplied `check` against each returned player. The
library's `compare()` calls `resolveInputID()` which throws
`Exception('Invalid SteamID input!')` on a shape it can't parse.

Pre-fix the operator-controlled `check` parameter went straight into
`compare()` with no shape gate — every malformed input (curl-driven,
context-menu handoff with a typo, page reload with stale URL params,
…) raised the exception, the dispatcher's `Throwable` fallback
wrapped it as a generic `server_error` 500 envelope, and the operator
saw "something went wrong" with no actionable feedback. Same bug
class #1420 was supposed to close, surfaced on a different handler
pair the first-round review didn't audit.

The fix is the same shape as `api_bans_add`'s pattern check: validate
`check` before calling `compare()`. The handlers carry two arms
(`type=0` for Steam ID, `type=1` for IP); pre-validate via:
- `SteamID::isValidID($check)` for Steam (rejects malformed Steam2 /
  Steam3 / Steam64 shapes including the newline-bypass class via the
  library's `D` modifier-carrying `ID_PATTERNS`)
- `filter_var($check, FILTER_VALIDATE_IP)` for IP (rejects everything
  that isn't a syntactically valid IPv4 or IPv6 address)

On a malformed input return the structured `status: 'not_found'`
envelope the handlers already emit for "no matching player" — same
shape the UI's "this player isn't here right now" branch already
handles, no new error path needed downstream.

Blockit only carries the Steam arm (comms-block is always Steam-ID-
keyed); kickit carries both because the RCON kick command can target
either a SteamID or an IP.

Regression coverage:
- `KickitTest::testKickPlayerReturnsNotFoundForMalformedSteamId` —
  five subtests: pure garbage, empty-Z (`STEAM_0:0:`), trailing-newline
  bypass (`STEAM_0:0:1\n`), invalid universe (`STEAM_2:0:1`), empty
  string. Each must return `not_found` (NOT a 500 envelope).
- `KickitTest::testKickPlayerReturnsNotFoundForMalformedIp` — five
  subtests: dotted-but-out-of-range, non-numeric, missing octet,
  trailing-garbage, empty string. Each must return `not_found`.
- `BlockitTest::testBlockPlayerReturnsNotFoundForMalformedSteamId` —
  five subtests mirroring kickit's Steam arm.
---
 web/api/handlers/blockit.php  | 18 +++++++++
 web/api/handlers/kickit.php   | 34 +++++++++++++++++
 web/tests/api/BlockitTest.php | 45 ++++++++++++++++++++++
 web/tests/api/KickitTest.php  | 71 +++++++++++++++++++++++++++++++++++
 4 files changed, 168 insertions(+)

diff --git a/web/api/handlers/blockit.php b/web/api/handlers/blockit.php
index 58ec5cb49..423925efd 100644
--- a/web/api/handlers/blockit.php
+++ b/web/api/handlers/blockit.php
@@ -42,6 +42,24 @@ function api_blockit_block_player(array $params): array
     $type   = (int)($params['type']   ?? 0);
     $length = (int)($params['length'] ?? 0);
 
+    // #1423 follow-up #4 — gate `$check` shape BEFORE the
+    // `SteamID::compare()` call below; the comms-block flow is always
+    // Steam-ID-keyed (there is no "block by IP" path) so the gate is
+    // unconditional. `compare()` routes through `toSteam64()` which
+    // throws on any non-isValidID input; without this gate a hostile
+    // caller posting `?check=garbage` 500s the handler instead of
+    // getting the `not_found` envelope the iframe loop expects.
+    if (!SteamID::isValidID($check)) {
+        return [
+            'status'   => 'not_found',
+            'sid'      => $sid,
+            'num'      => $num,
+            'hostname' => '',
+            'ip'       => '',
+            'port'     => '',
+        ];
+    }
+
     $serverInfo = $GLOBALS['PDO']->query("SELECT ip, port FROM `:prefix_servers` WHERE sid = :sid");
     $GLOBALS['PDO']->bind(':sid', $sid);
     $sdata = $GLOBALS['PDO']->single() ?: ['ip' => '', 'port' => ''];
diff --git a/web/api/handlers/kickit.php b/web/api/handlers/kickit.php
index 9d2aec430..67649ff78 100644
--- a/web/api/handlers/kickit.php
+++ b/web/api/handlers/kickit.php
@@ -61,6 +61,40 @@ function api_kickit_kick_player(array $params): array
     $num   = (int)($params['num']   ?? 0);
     $type  = (int)($params['type']  ?? 0);
 
+    // #1423 follow-up #4 — gate the `check` shape BEFORE we reach the
+    // `SteamID::compare()` call below. For `type === 0` (Steam-ID
+    // kick), `compare()` routes through `toSteam64()` → the library
+    // throws `Exception('Invalid SteamID input!')` on any input that
+    // fails the strict shape gate. The exception escapes the handler
+    // via `Api::handle`'s `Throwable` fallback as a generic 500
+    // envelope; the iframe loop then can't tell "no match" apart from
+    // "your input was garbage", and a hostile caller posting
+    // `?check=garbage&type=0` reliably 500s the panel. For
+    // `type === 1` (IP-address kick) we run `filter_var` instead so a
+    // malformed IP returns the standard `not_found` envelope rather
+    // than triggering the SteamID compare on whatever the operator
+    // typed in the wrong box.
+    if ($type === 0 && !SteamID::isValidID($check)) {
+        return [
+            'status' => 'not_found',
+            'sid'    => $sid,
+            'num'    => $num,
+            'hostname' => '',
+            'ip'     => '',
+            'port'   => '',
+        ];
+    }
+    if ($type === 1 && !filter_var($check, FILTER_VALIDATE_IP)) {
+        return [
+            'status' => 'not_found',
+            'sid'    => $sid,
+            'num'    => $num,
+            'hostname' => '',
+            'ip'     => '',
+            'port'   => '',
+        ];
+    }
+
     $serverInfo = $GLOBALS['PDO']->query("SELECT ip, port FROM `:prefix_servers` WHERE sid = :sid");
     $GLOBALS['PDO']->bind(':sid', $sid);
     $sdata = $GLOBALS['PDO']->single() ?: ['ip' => '', 'port' => ''];
diff --git a/web/tests/api/BlockitTest.php b/web/tests/api/BlockitTest.php
index 4aecd67c6..f777fbc31 100644
--- a/web/tests/api/BlockitTest.php
+++ b/web/tests/api/BlockitTest.php
@@ -80,4 +80,49 @@ public function testBlockPlayerRejectsAnonymous(): void
         $env = $this->api('blockit.block_player', ['check' => 'STEAM_0:0:1', 'sid' => 0]);
         $this->assertEnvelopeError($env, 'forbidden');
     }
+
+    /**
+     * #1423 follow-up #4 — pre-fix `api_blockit_block_player` called
+     * `SteamID::compare($player_steamid, $check)` without gating
+     * `$check` first. `compare()` routes through `toSteam64()` which
+     * throws `Exception('Invalid SteamID input!')` on any input that
+     * fails the library's strict shape gate. The exception escaped
+     * the handler as a generic 500 envelope; the iframe loop then
+     * can't tell "no match" apart from "your input was garbage", and
+     * a hostile caller posting `?check=garbage` reliably 500'd the
+     * panel. Comms-block has no "block by IP" path (the form gates
+     * type ∈ {Mute=1, Gag=2, Silence=3} on a Steam-ID-keyed flow),
+     * so the gate is unconditional (no IP arm). The structured
+     * `not_found` envelope surfaces on every malformed shape.
+     */
+    public function testBlockPlayerReturnsNotFoundForMalformedSteamId(): void
+    {
+        $this->loginAsAdmin();
+        foreach (
+            [
+                'garbage',                  // unrecognised shape
+                'STEAM_0:0:',               // empty Z (library accepts loose `\d*` but `\d+` rejects)
+                "STEAM_0:0:1\n",            // trailing newline (library `D` modifier rejects)
+                'STEAM_2:0:1',              // X=2 (library `[01]` rejects)
+                '',                          // empty
+            ] as $badCheck
+        ) {
+            $env = $this->api('blockit.block_player', [
+                'check' => $badCheck,
+                'sid'   => 1,
+                'num'   => 0,
+                'type'  => 1,
+                'length' => 60,
+            ]);
+            $this->assertTrue(
+                $env['ok'],
+                sprintf('expected ok envelope for check=%s, got: %s', var_export($badCheck, true), json_encode($env)),
+            );
+            $this->assertSame(
+                'not_found',
+                $env['data']['status'],
+                sprintf('expected status=not_found for check=%s', var_export($badCheck, true)),
+            );
+        }
+    }
 }
diff --git a/web/tests/api/KickitTest.php b/web/tests/api/KickitTest.php
index 70513d9ec..28dbb34ff 100644
--- a/web/tests/api/KickitTest.php
+++ b/web/tests/api/KickitTest.php
@@ -65,4 +65,75 @@ public function testKickPlayerRejectsAnonymous(): void
         $env = $this->api('kickit.kick_player', ['check' => 'STEAM_0:0:1', 'sid' => 0]);
         $this->assertEnvelopeError($env, 'forbidden');
     }
+
+    /**
+     * #1423 follow-up #4 — pre-fix `api_kickit_kick_player` called
+     * `SteamID::compare($player_steamid, $check)` without gating
+     * `$check` first. `compare()` routes through `toSteam64()` which
+     * throws `Exception('Invalid SteamID input!')` on any input
+     * that fails the library's strict shape gate. The exception
+     * escaped the handler via `Api::handle`'s `Throwable` fallback as
+     * a generic 500 envelope; the iframe loop then can't tell "no
+     * match" apart from "your input was garbage", and a hostile
+     * caller posting `?check=garbage&type=0` reliably 500'd the
+     * panel. The fix is a pre-`SteamID::compare()` `isValidID()` /
+     * `filter_var(IP)` gate that surfaces the structured `not_found`
+     * envelope the iframe expects on any malformed input.
+     */
+    public function testKickPlayerReturnsNotFoundForMalformedSteamId(): void
+    {
+        $this->loginAsAdmin();
+        foreach (
+            [
+                'garbage',                  // unrecognised shape
+                'STEAM_0:0:',               // empty Z (library accepts loose `\d*` but `\d+` rejects)
+                "STEAM_0:0:1\n",            // trailing newline (library `D` modifier rejects)
+                'STEAM_2:0:1',              // X=2 (library `[01]` rejects)
+                '',                          // empty
+            ] as $badCheck
+        ) {
+            $env = $this->api('kickit.kick_player', [
+                'check' => $badCheck,
+                'sid'   => 1,
+                'num'   => 0,
+                'type'  => 0,
+            ]);
+            $this->assertTrue(
+                $env['ok'],
+                sprintf('expected ok envelope for check=%s, got: %s', var_export($badCheck, true), json_encode($env)),
+            );
+            $this->assertSame(
+                'not_found',
+                $env['data']['status'],
+                sprintf('expected status=not_found for check=%s', var_export($badCheck, true)),
+            );
+        }
+    }
+
+    /**
+     * #1423 follow-up #4 — paired with the SteamID guard, the IP
+     * branch of `api_kickit_kick_player` (`type === 1`) needs the
+     * same defense: a garbage `check` value reaches the iframe loop
+     * with a `no_connect` envelope and an opaque iframe failure.
+     * The fix uses `filter_var(FILTER_VALIDATE_IP)` so the structured
+     * `not_found` envelope surfaces.
+     */
+    public function testKickPlayerReturnsNotFoundForMalformedIp(): void
+    {
+        $this->loginAsAdmin();
+        foreach (['garbage', 'not.an.ip', "192.168.0.1\n", ''] as $badCheck) {
+            $env = $this->api('kickit.kick_player', [
+                'check' => $badCheck,
+                'sid'   => 1,
+                'num'   => 0,
+                'type'  => 1,
+            ]);
+            $this->assertTrue($env['ok']);
+            $this->assertSame(
+                'not_found',
+                $env['data']['status'],
+                sprintf('expected status=not_found for IP check=%s', var_export($badCheck, true)),
+            );
+        }
+    }
 }

From fccb177b5598b46198ddff9afcf5f1bf53c77e69 Mon Sep 17 00:00:00 2001
From: rumblefrog <contact@zhenyangli.me>
Date: Tue, 19 May 2026 20:12:51 -0400
Subject: [PATCH 8/9] fix(steamid): tighten SteamAuthHandler OpenID regex +
 install wizard pattern (#1420 review follow-up)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two surfaces the first-round audit didn't cover, both feeding the
same `SteamID::toSteam2()` library call from upstream regexes that
drifted on shape and modifier.

## `Sbpp\Auth\Handler\SteamAuthHandler::validate()`

Steam OpenID returns the user's Steam64 in the claimed_id URL path
(`https://steamcommunity.com/openid/id/<76561197960265728>`). The
handler regex `7[0-9]{15,25}+` was wider than reality — Steam in
practice always returns exactly 17-digit Steam64s for individual
accounts, and the library's `ID_PATTERNS` carries `^\d{17}$D` for
the Steam64 arm.

Pre-fix a 16-digit OR 18-25-digit OpenID claim slipped past
`validate()`'s extract regex but then failed `SteamID::toSteam2()` in
`check()` (which routes through `isValidID()`'s strict `\d{17}` gate).
The exception escaped the constructor's call site unhandled
(LightOpenID's mid-flow redirect leaves no chrome to catch it), so
the operator landed on a 500 mid-Steam-login round-trip — silent
failure mode, no toast, no inline error.

Tightened to `7\d{16}+` with the `D` modifier so the two gates agree
on the shape byte-for-byte. Added a `SteamID::isValidID()` belt-and-
suspenders gate in `check()` so any future drift on either side
surfaces as a clean redirect-to-login-failed, not as a 500.

## `web/install/pages/page.5.php`

The install wizard's "create initial admin" step had its own regex
(`/^STEAM_[01]:[01]:[0-9]+$/`) for the Steam ID input on the admin
form. Pre-fix this carried no `D` modifier, so `STEAM_0:0:1\n` (a
copy-paste-from-game-console typo with a trailing newline) slipped
past the wizard's gate but then failed `SteamID::isValidID()` on the
post-install panel runtime side (different code path, same library).
Result: the wizard accepted the input, the admin row got created with
a malformed authid, and the first panel login attempt would fail with
no recoverable error path (the wizard can't be re-run over an
installed panel — see the `already-installed.php` guard).

Added the `D` modifier so the wizard rejects the newline-bypass at
the same gate the panel runtime does.

Regression coverage (added in commit 1):
- `SteamIDValidationTest::testSteamAuthHandlerOpenIdRegexAcceptsOnly17DigitsStartingWith7`
  — 16-digit, 18-digit, non-7-starting, trailing newline, trailing
  garbage rejections.
- `SteamIDValidationTest::testInstallWizardSteamIdRegexCarriesDModifier`
  — pins the install wizard's regex (Steam2-only, D modifier
  rejects trailing newline, [01] rejects universe 2 / Y=2).
---
 .../Auth/Handler/SteamAuthHandler.php         | 29 ++++++++++++++++++-
 web/install/pages/page.5.php                  | 11 ++++++-
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/web/includes/Auth/Handler/SteamAuthHandler.php b/web/includes/Auth/Handler/SteamAuthHandler.php
index 51070b72c..8c9749a3a 100644
--- a/web/includes/Auth/Handler/SteamAuthHandler.php
+++ b/web/includes/Auth/Handler/SteamAuthHandler.php
@@ -32,7 +32,23 @@ private function login(): void
 
     private function validate(): string|false
     {
-        $pattern = "/^https:\/\/steamcommunity\.com\/openid\/id\/(7[0-9]{15,25}+)$/";
+        // #1423 follow-up #4 — tightened from `7[0-9]{15,25}+` to
+        // exactly 17 digits (`\d{16}` after the literal `7`) to match
+        // `SteamID::ID_PATTERNS`'s `^\d{17}$D` shape. Pre-fix a
+        // 16-digit OR 18-25-digit OpenID claim slipped past this
+        // regex but then failed `SteamID::toSteam2()` in `check()`
+        // (which routes through the library's `\d{17}` gate), the
+        // exception escaped the constructor unhandled, and the
+        // operator landed on a 500 mid-Steam-login round-trip
+        // (silent failure mode — there's no `try/catch` here and the
+        // chrome's `PageDie()` doesn't run on a callback redirect).
+        // Steam in practice always returns 17-digit Steam64 IDs in
+        // the claimed_id URL; this regex now matches the library
+        // contract byte-for-byte so a future Steam-side change that
+        // emits a 16-digit ID (or a 24-digit one for some hypothetical
+        // future user range) surfaces here as a clean false return
+        // (operator sees the login-failed message), not as a 500.
+        $pattern = "/^https:\/\/steamcommunity\.com\/openid\/id\/(7\d{16}+)$/D";
 
         // Issue #1273: $this->openid->data is $_POST / $_GET (mixed), and
         // PHPStan can't see that LightOpenID::validate() guarantees
@@ -48,6 +64,17 @@ private function validate(): string|false
 
     private function check(string $steamid): void
     {
+        // Defense-in-depth: `validate()` already gates the input
+        // through the strict 17-digit regex, but the library's
+        // `toSteam2()` raises a generic `\Exception` on any input that
+        // fails `isValidID()`. The exception would escape the
+        // constructor's call site unhandled (LightOpenID's mid-flow
+        // redirect leaves no chrome to catch it), so the gate here is
+        // load-bearing belt-and-suspenders.
+        if (!\SteamID\SteamID::isValidID($steamid)) {
+            header("Location: ".Host::complete()."/index.php?p=login&m=steam_failed");
+            return;
+        }
         $steamid = \SteamID\SteamID::toSteam2($steamid);
 
         $this->dbs->query('SELECT aid FROM `:prefix_admins` WHERE authid = :authid');
diff --git a/web/install/pages/page.5.php b/web/install/pages/page.5.php
index 182dab34c..1772b844b 100644
--- a/web/install/pages/page.5.php
+++ b/web/install/pages/page.5.php
@@ -65,7 +65,16 @@
         $error = 'Password must be at least 8 characters.';
     } elseif ($pass1 !== $pass2) {
         $error = 'Passwords do not match.';
-    } elseif (preg_match('/^STEAM_[01]:[01]:[0-9]+$/', $steam) !== 1) {
+    } elseif (preg_match('/^STEAM_[01]:[01]:[0-9]+$/D', $steam) !== 1) {
+        // #1423 follow-up #4 — `D` modifier added so `STEAM_0:0:1\n`
+        // (trailing newline) is rejected here AND in the matching panel
+        // runtime gate `SteamID::ID_PATTERNS`. Without the modifier the
+        // wizard would silently accept the newline-suffixed shape AND
+        // write the malformed authid into `:prefix_admins.authid`, which
+        // the panel's runtime would then never match (the SQL `=` compare
+        // is byte-exact and the SourceMod plugin's
+        // `authid REGEXP '^STEAM_[0-9]:Y:Z$'` filter would also miss
+        // the row because the trailing `\n` defeats the REGEXP anchor).
         $error = 'Steam ID must be in STEAM_0:X:NNNNNNN format.';
     } elseif (filter_var($email, FILTER_VALIDATE_EMAIL) === false) {
         $error = 'Email address is invalid.';

From 6a018c4455a3dcf22be8fb5be7260b5dceaed220 Mon Sep 17 00:00:00 2001
From: rumblefrog <contact@zhenyangli.me>
Date: Tue, 19 May 2026 20:13:06 -0400
Subject: [PATCH 9/9] test(steamid): cover trim-bypass shapes in admins / comms
 add tests (#1420 review follow-up)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The `_register.php` dispatcher `trim()`s string params before the
handler sees them, so trailing whitespace (the most-likely typo
shape) gets cleaned upstream of `SteamID::HANDLER_STRICT_REGEX`'s
gate. The library-side tests
(`SteamIDValidationTest::testHandlerStrictRegexRejectsNewlineBypass`)
already cover trailing newlines at the regex level, but the handler-
level tests should cover the shapes `trim()` does NOT defend against:

- Mid-string newlines (`STEAM_0:0:1\nGARBAGE`, `GARBAGE\nSTEAM_0:0:1`)
  — embedded newlines pass through `trim()` and must be caught at
  the regex layer's `^…$` anchors + non-`m` mode requiring whole-
  string match.
- Trailing non-breaking spaces (`STEAM_0:0:1\xC2\xA0` = U+00A0) —
  `trim()` only handles the ASCII whitespace set (space / tab / CR /
  LF / vertical tab / null), so unicode whitespace like NBSP / zero-
  width-space / ideographic-space passes through and must be caught
  at the regex layer too.

The pre-existing trailing-newline test cases (`"STEAM_0:0:1\n"`,
`"[U:1:1]\n"`, `"76561197960265728\n"`) were quietly redundant — the
dispatcher's `trim()` consumed them before the regex got a turn, so
the tests proved nothing about the gate's actual rejection behaviour.
Replaced with the trim-bypass shapes above so the handler tests
actually exercise the regex's load-bearing `^…$` anchors.

The library-side coverage stays — `SteamAuthHandler::validate()` and
the install wizard's `page.5.php` DON'T `trim()` their inputs (the
OpenID claimed_id and the install form value go straight into the
regex), so the trailing-newline cases there are non-redundant. See
the matching contracts pinned by `SteamIDValidationTest`.
---
 web/tests/api/AdminsTest.php |  9 +++++++++
 web/tests/api/CommsTest.php  | 18 ++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/web/tests/api/AdminsTest.php b/web/tests/api/AdminsTest.php
index 74559d5d3..b92dd64d6 100644
--- a/web/tests/api/AdminsTest.php
+++ b/web/tests/api/AdminsTest.php
@@ -102,6 +102,15 @@ public function testAddRejectsInvalidSteamIdShape(): void
                 'asdfSTEAM_0:0:123',               // substring-bypass (unanchored)
                 'asdf 76561197960265728 garbage',  // embedded Steam64 — pre-fix corrupted
                 'U:1:1',                           // bracketless Steam3 — form pattern requires brackets
+                // #1423 follow-up #4 — bypass shapes that `trim()`
+                // upstream of the gate does NOT defend against (the
+                // common trailing-newline case is trim-eaten — see
+                // SteamIDValidationTest's
+                // `testHandlerStrictRegexRejectsNewlineBypass` for
+                // the regex-level newline contract).
+                "STEAM_0:0:1\nGARBAGE",            // mid-string newline (trim doesn't help)
+                "GARBAGE\nSTEAM_0:0:1",            // leading garbage + mid-string newline
+                "STEAM_0:0:1\xC2\xA0",             // trailing NBSP (U+00A0); trim doesn't handle unicode whitespace
             ] as $badSteam
         ) {
             $env = $this->api('admins.add', $this->adminParams(['steam' => $badSteam]));
diff --git a/web/tests/api/CommsTest.php b/web/tests/api/CommsTest.php
index c066378e1..348929b16 100644
--- a/web/tests/api/CommsTest.php
+++ b/web/tests/api/CommsTest.php
@@ -178,6 +178,21 @@ public function testAddValidatesSteamIdRequired(): void
      *   - `'U:1:1'` — bracketless Steam3; the library accepts but
      *     the form's `pattern` requires brackets, so the strict
      *     gate rejects for symmetry.
+     *   - `"STEAM_0:0:1\nGARBAGE"` / `"GARBAGE\nSTEAM_0:0:1"` —
+     *     mid-string newline bypass (#1423 follow-up #4). `trim()`
+     *     upstream of the gate handles trailing whitespace, so
+     *     the common `"STEAM_0:0:1\n"` typo path strips through to
+     *     a clean `"STEAM_0:0:1"` and rightly succeeds. But a
+     *     hostile caller (or a malformed file paste) embeds the
+     *     newline mid-string, where `trim()` doesn't touch it.
+     *     The strict gate's `^…$` anchors + non-`m` mode require
+     *     the whole string match the pattern; embedded `\n` fails.
+     *     `HANDLER_STRICT_REGEX`'s regex-level newline rejection is
+     *     pinned directly in `SteamIDValidationTest::testHandlerStrictRegexRejectsNewlineBypass`.
+     *   - `"STEAM_0:0:1\xC2\xA0"` — trailing NBSP (U+00A0). `trim`
+     *     only handles the ASCII whitespace set; unicode whitespace
+     *     like NBSP / zero-width-space / ideographic-space pass
+     *     through trim and must be caught at the regex layer.
      */
     public function testAddRejectsInvalidSteamIdShape(): void
     {
@@ -191,6 +206,9 @@ public function testAddRejectsInvalidSteamIdShape(): void
                 'asdfSTEAM_0:0:123',
                 'asdf 76561197960265728 garbage',
                 'U:1:1',
+                "STEAM_0:0:1\nGARBAGE",
+                "GARBAGE\nSTEAM_0:0:1",
+                "STEAM_0:0:1\xC2\xA0",
             ] as $badSteam
         ) {
             $env = $this->api('comms.add', [