Skip to content

test(user): cover HTML metacharacters in username validation - #41738

Merged
DeepDiver1975 merged 2 commits into
masterfrom
test/username-metachar-regression
Jul 27, 2026
Merged

test(user): cover HTML metacharacters in username validation#41738
DeepDiver1975 merged 2 commits into
masterfrom
test/username-metachar-regression

Conversation

@DeepDiver1975

Copy link
Copy Markdown
Contributor

Summary

Adds regression coverage for HTML/script metacharacters in the username
validation allow-list. Test-only — no production behaviour changes.

OC\User\Manager::createUser() already rejects every character outside
a-z, A-Z, 0-9 and +_.@-', so markup can never reach a stored
username. That guard was only exercised by three generic invalid-character
cases (John#Smith, John^Smith, JohnSmith(CEO)), none of which looked
like an injection payload. This extends the data provider so a future
relaxation of the allow-list fails the suite instead of passing unnoticed.

Added cases:

  • "><script>alert(document.cookie)</script>
  • <img src=x onerror=alert(1)>
  • John<b>Smith
  • John"Smith

Background

This came out of triaging an external report claiming stored XSS via the
username field in /settings/users. The report did not reproduce: the
allow-list rejects the payload server-side at
lib/private/User/Manager.php, and the render sink in
settings/js/users/users.js uses jQuery .text() (textContent), so no
markup is interpretable. Both admin-panel and provisioning-API creation
paths funnel through the same createUser() method, so there is no
bypassing path.

No fix was required. These tests exist so that conclusion stays true.

Testing

$ phpunit -c tests/phpunit-autotest.xml --filter testUsernameHasInvalidChars
OK (7 tests, 14 assertions)

$ phpunit -c tests/phpunit-autotest.xml tests/lib/User/ManagerTest.php
OK (42 tests, 95 assertions)

Both new payloads confirmed rejected by the existing regex; full
ManagerTest green with no regressions.

🤖 Generated with Claude Code

The username allow-list in OC\User\Manager::createUser() already rejects any
character outside "a-z", "A-Z", "0-9" and "+_.@-'", so HTML and script
metacharacters can never reach a stored username. That guard was only covered
by three generic invalid-character cases, none resembling an injection payload.

Extend the invalid-character data provider with quote, angle-bracket and full
script-tag payloads so a future relaxation of the allow-list fails the suite
rather than passing unnoticed. Test-only change; no production behaviour is
affected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
@DeepDiver1975
DeepDiver1975 requested a review from a team as a code owner July 27, 2026 15:21
The entry used "Tests:" as its type, which calens rejects - it validates
against a closed set of Bugfix, Change, Enhancement and Security, so the
changelog lint failed. Use "Change:", matching how existing test and
tooling entries are typed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
@DeepDiver1975
DeepDiver1975 merged commit fca7f8d into master Jul 27, 2026
29 checks passed
@DeepDiver1975
DeepDiver1975 deleted the test/username-metachar-regression branch July 27, 2026 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants