Keep SteamID64 HTML pattern braces out of Smarty

[maxijabase]

Description

A valid 17-digit SteamID64 (for example 76561198179807307) was rejected by native HTML validation on Add a comm block, Add a ban, Submit a ban, and the matching edit forms. The browser popover told the operator to enter a 17-digit SteamID64 even though that is exactly what they typed.

Steam2 (STEAM_0:1:N) and Steam3 ([U:1:N]) still worked.

Root cause is Smarty, not the PHP gate. Template source used:

pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"

Smarty's { / } delimiters treat {17} as a tag. The rendered HTML became \d17 (quantifier dropped). The browser then rejected every 17-digit SteamID64. The Steam2 / Steam3 arms use \d+, so they never hit this.

SteamID::HANDLER_STRICT_REGEX on the PHP side was already correct. Submitting via curl / a third-party theme that skips native validation would have succeeded. The existing E2E happy path used STEAM_0:1:14202020, so CI never caught it.

Fix: wrap the Steam64 arm as {literal}\d{17}{/literal} so Smarty leaves the braces alone. Rendered HTML is again \d{17}.

Touched templates (every occurrence of this pattern):

• web/themes/default/page_admin_comms_add.tpl (reported surface)
• web/themes/default/page_admin_bans_add.tpl
• web/themes/default/page_admin_edit_ban.tpl
• web/themes/default/page_admin_edit_comms.tpl
• web/themes/default/page_admin_edit_admins_details.tpl
• web/themes/default/page_submitban.tpl

Not affected: install wizard (Steam2-only pattern, no brace quantifier), PHP regexes, JS already inside {literal} blocks.

Motivation and Context

Operators who paste a Community ID into Add Block / Add Ban / Submit Ban hit a native validation popover and cannot submit, even though the value is valid and the server would accept it.

How Has This Been Tested?

• PHPUnit SteamIDValidationOrderTest (12 tests, 81 assertions), including:
  • source pin that every Steam ID form carries {literal}\d{17}{/literal}
  • testRenderedSteamPatternKeepsSeventeenDigitQuantifier: Smarty-compiles each extracted pattern and asserts the HTML is pattern="STEAM_[01]:[01]:\d+|\[U:1:\d+\]|\d{17}"
  • testTemplatesHaveNoBareDigitBraceQuantifiers: scans all web/themes/**/*.tpl, strips {literal} and {* *}, fails on leftover {<digits>}
• E2E: comms-add-steamid-validation.spec.ts now fills 76561198179807307 on add-block, add-ban, and submit, and asserts the rendered pattern plus validity.valid
• AGENTS.md convention + anti-pattern updated so a future \d{17} in a .tpl pattern does not regress

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.

[Rushaway]

Review

Root cause and fix are correct, and the coverage is unusually thorough for a one-character-class bug. {17} is a valid Smarty expression tag (auto-literal only spares { followed by whitespace), so it rendered as 17 and the browser received \d17 — which not only rejects every real SteamID64 but would have accepted junk like 917. {ldelim}17{rdelim} is the right escape here, and I agree with the call to avoid opening a new {literal} pair inside an attribute given how SmartyTemplateRule pairs it against a {literal} mention in a preceding {* *} comment.

What I verified locally on fix/steamid64-html-pattern:

• All six pattern="STEAM_…" occurrences under web/themes/ are converted; grep -rn 'pattern="STEAM_' --include=*.tpl shows no stragglers.
• themes/default/install/page_admin.tpl is correctly untouched — pattern="STEAM_[01]:[01]:[0-9]+", no brace quantifier.
• The one remaining \d{17} in a template (page_admin_bans_add.tpl:333, the IIFE's JS regex) sits inside the {literal} block spanning 277–431, so it is genuinely safe and correctly excluded by the new scanner.
• None of the six views override View::DELIMITERS, so {ldelim}/{rdelim} really are live tags on all of these surfaces. (Only LoginView, BlockitView, KickitView, AdminServersRconView use -{ }-.)
• CI is green on all five required checks.

No blocking objection. Notes below, roughly in descending order of value.

1. The PR description no longer matches the code

The body says the fix is {literal}\d{17}{/literal}; 21d636c6 replaced that with {ldelim}17{rdelim}, and the second approach is the one being merged (and the one AGENTS.md now mandates). Since this body is what lands in the squash commit and is what the next person greps for, it's worth rewriting the "Fix:" paragraph — including the sentence about {literal} being the answer, which the new AGENTS.md anti-pattern explicitly warns against.

2. The new E2E test escapes the file's serial guard

test.describe('flow: SteamID64 native HTML pattern') is a plain describe, sitting next to a describe.serial whose own comment spells out why that's unsafe:

.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).

With fullyParallel: true and local workers: <cores>, the new test's page.goto('/index.php?p=admin&c=comms') can land inside exactly that window, get bounced to login, and fail on expect(steam).toBeVisible(). CI pins workers: 1 so the gate won't see it, which makes it the worst kind of flake — local only. Either move the test inside the existing describe.serial, or add test.describe.configure({ mode: 'serial' }) at file scope.

3. testRenderedSteamPatternKeepsSeventeenDigitQuantifier hardcodes the default delimiters

new Smarty() compiles every snippet with { }. That is right for all six templates today, but the assertion's premise is "this is how the app renders it", and the app doesn't universally render with { }. If a Steam ID input ever lands in a -{ }- template, this test would happily assert against the wrong grammar (and {ldelim} would ship to the browser verbatim). Deriving the pair from the bound View's DELIMITERS — the same constant SmartyTemplateRule already reads — would make the test say what it means.

4. Smaller things in the same test

• preg_match grabs only the first pattern="STEAM_…" per file. testFormTemplatesCarryStrictSteamPattern is likewise assertStringContainsString, so a template that grows a second Steam input with a bare {17} would be caught only by the directory scanner, not by the two tests that are nominally about these six files. preg_match_all + a loop closes that.
• The six iterations all write to the same $compileDir . '/snippet.tpl' and set templateDir === compileDir. It works, but only because setForceCompile(true) is on and Smarty re-reads the source each pass — that's a lot of weight on an implementation detail for a test whose whole point is not trusting Smarty. $theme->fetch('eval:' . $m[0]) drops the file juggling entirely; failing that, a per-template filename.
• Nothing removes sys_get_temp_dir() . '/sbpp-test-smarty-steam-pattern-<pid>' afterwards. A tearDownAfterClass unlink would keep repeated local runs from accumulating snippets plus their compiled .php.

5. The directory scanner is over-broad on -{ }- templates

testTemplatesHaveNoBareDigitBraceQuantifiers walks all of themes/ and flags any {<digits>} outside {literal} / {* *}. In page_login.tpl, page_blockit.tpl, page_kickit.tpl, page_admin_servers_rcon.tpl a bare {17} is inert text, and those files also wouldn't use {literal} to opt out. No offenders today, so it passes — but the next person who writes a legitimate [0-9]{1,3} in the RCON template gets a failure with a message telling them to apply a fix that would actually break it. Skipping files whose View overrides DELIMITERS (or at least saying so in the failure message) would help.

Separately: this is a general template-hygiene rule living in a class named SteamIDValidationOrderTest. Given web/includes/PHPStan/SmartyTemplateRule.php already exists and already reads DELIMITERS, PHPStan looks like the more natural home — it'd also make the delimiter awareness fall out for free. Non-blocking, and I understand co-locating it with the regression it came from.

6. Follow-up thought

This pattern now exists six times in templates, once more as a JS regex in page_admin_bans_add.tpl, and again as SteamID::HANDLER_STRICT_REGEX — and the PR's response is to pin the duplication byte-for-byte in three tests. That's consistent with the convention AGENTS.md documents, so I'm not asking to change it here. But this bug is a direct consequence of the copies: a single PHP-side constant surfaced to the templates as a variable would have made "Smarty ate the braces" structurally impossible, and would collapse three source-pinning tests into one. Might be worth an issue.