Skip to content

fix: neutralize untrusted text and gate external URLs in UI sinks - #9565

Open
mikhail-dcl wants to merge 8 commits into
devfrom
fix/sec-008-external-url-hardening
Open

fix: neutralize untrusted text and gate external URLs in UI sinks#9565
mikhail-dcl wants to merge 8 commits into
devfrom
fix/sec-008-external-url-hardening

Conversation

@mikhail-dcl

@mikhail-dcl mikhail-dcl commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Pull Request Description

What does this PR change?

Text written by other users — profile and community names, event/place descriptions, announcements, friend-request messages — reached rich-text TMP labels unsanitized, and an attacker-authored link could open with no consent prompt. Fixes SEC-008, SEC-023, SEC-034, SEC-050 and the client half of SEC-084.

Two mechanisms, chosen per label:

  • richText off in the prefab where a label renders nothing but untrusted text.
  • Escaping in code via the new RichTextSanitizer where the label's own copy is markup (a <b> run, or the <link> the description linkifier emits) and turning it plain would break it.

RichTextSanitizer also escapes the backslash, which is not cosmetic: TMP rewrites a \uXXXX sequence into the character it denotes inside the array its tag parser reads (TMP_Text.PopulateTextProcessingArray, case 117). That branch is gated by neither parseCtrlCharacters nor the input-source check above it, which ships commented out — so a brackets-only filter let markup through unparsed. This also closes the same hole in the already-shipped transaction-confirmation copy, where a scene name could otherwise hide the recipient and amount.

A third mechanism applies to chat: gate on provenance, never on content. An incoming message used to skip the formatting pipeline when its text merely began with 🟢/🔴/🟡, so any peer could prefix theirs and choose whether it was processed. The skip now reads IsSystemMessage alone. Note the injection there was never gated on the prefix at all — FormatText only wraps URLs in <link=url> and never escaped anything — so peer text is now escaped before formatting.

Changes

Area Change Mechanism
RichTextSanitizer (new) Single escaper for author-written text: Escape (content), EscapeAttribute (tag attribute), Truncate (cap only), shared name/body caps. One allocation per escaping call; none when there is nothing to escape
External-URL prompt Destination and domain rendered as plain text Prefab
Passport link title Another user's link title can no longer dress a URL up as a familiar one Prefab
Event/place descriptions Escaped before linkification; clicks routed through the consent prompt instead of opening directly Code
Navmap names/titles Event and place names, coordinates, world names, event list entries Prefab + code
Navmap host/creator <b> templates keep rich text, values escaped Code
Profile name element Renders filtered ValidatedName; falls back to the raw name when the filter leaves nothing (emoji-only names) Prefab + code
Friend panel, push notification, friend requests Raw Name replaced with filtered name; message bodies escaped inside their <b> template Prefab + code
Notifications, community cards/members, NFT prompt, chat username, photo-detail person Names, descriptions and announcement bodies neutralized and bounded Prefab + code
Chat messages Skip gated on IsSystemMessage instead of a forgeable status-emoji prefix; peer text escaped before linkification. ChatEntryMessageBubbleElement overrides the content label back to rich text, so this text does render markup Code
TransactionRecipientUtils Private duplicate escaper folded into the shared one Code
Cleanup Dead SimpleUserNameElement.Setup overload and unused webBrowser ctor parameters removed Code

Already landed in #9466 and only verified here, not changed: the http(s)-only scheme allowlist and the (scheme, host) trust key that closed the empty-host prompt bypass.

Note for reviewers: PlaceToast.prefab binds four view fields — LiveEventNameLabel, DescriptionLabel, CoordinatesLabel, ParcelCountLabel — to a single TMP component, so turning rich text off for the coordinates would silence the description's links. Those writers escape in code instead. UntrustedTextLabelsShould pins both directions (nine labels plain, five deliberately rich) and asserts that shared binding, so the guard cannot be "fixed" by flipping the flag it protects.

Server-side halves of SEC-084 (decentraland/events approval-reset bug, decentraland/places output sanitization) are out of scope for this repo.

Test Instructions

Steps (standard run):

metaforge explorer run 9565

Steps (fresh account):

metaforge account create --clear
metaforge explorer run 9565

Prerequisites

Set these up first — every check below reuses them.

  • Profile A — display name with markup: <size=400%><color=#00FF00>Verified Admin
  • Profile B — display name using the escape form instead of brackets: \u003Csize=400%\u003EAdmin
  • Profile C — display name of emoji only, e.g. 👽👽
  • Profile D — display name of 200+ characters
  • A place or event you can edit, description containing all three of: <link="smb://attacker/share">click here</link>, <size=400%>huge, and a plain https://decentraland.org
  • A community you own, with markup in its name, description and an announcement body
  • A second account you can send nearby chat from, to post a message reading 🟢 <link="https://evil.example.com">Verified ✔</link>
  • Profiles A–D added as friends, and one pending friend request whose message body contains <b><size=400%>urgent

What "pass" looks like

Two different outcomes depending on the row below:

  • Inert — the markup is visible as ordinary characters (you literally read <size=400%>, possibly with slightly odd-looking angle brackets ‹ ›). Nothing is huge, coloured, hidden, or clickable.
  • Still works ⚠️ — the UI's own styling and links must be unchanged from dev. These are the regression rows; a bug here is caused by this PR.
  • Highlight if the safety introduced is too strict — in some UIs it might be desirable to support rich texts and links (e.g. if it's already used by the existing players / communities / events, etc.) and it doesn't lead to the security breaches. Bring these cases to me, I will evaluate security implications.

UI surfaces to check

# Where in the UI How to reach it What to verify
1 External-URL prompt Click a link in a place/event description, or a passport link Shows the full destination URL, and markup in it is inert
2 Passport → Links Open Profile A's passport Link titles inert; clicking still opens the prompt
3 Navmap → place panel Search a place, open its panel Place name, created by …, coordinates/world name all inert
4 Navmap → place panel description Same panel ⚠️ Embedded <link> inert, but the plain https:// is still blue, clickable, and prompts
5 Navmap → place toast Click a pin on the map (small popup, not the panel) ⚠️ Same as #4 — this uses one shared label, so links here are the most likely thing to break
6 Navmap → event panel Open an event Event name inert; hosted by X - at Y inert
7 Navmap → event panel styling Same panel ⚠️ hosted by / at must still be bold
8 Navmap → event list rows Place panel with upcoming events Event names inert
9 Mention suggestions Type @ in chat Profile A/B names inert, list rows not stretched
10 Voice-chat titlebar Join a voice call with Profile A Name inert, titlebar not stretched
11 Profile card / donation flow Open a profile card; start a donation Name inert on confirm, loading and confirmed screens
12 Friends list Open the friends panel Names inert, rows not stretched
13 Friend push notification Have Profile A friend you Name inert
14 Friend request panel Open the pending request Message body inert
15 Friend request styling Same panel ⚠️ Sender name prefix must still be bold (Name:)
16 Friend request toast Receive a request ⚠️ Title must keep its two colours — name in one, "wants to be your friend" in another
17 Communities → card Open your community Name and description inert
18 Communities → browser results Search communities Title, owner and description inert
19 Communities → members Members tab Member names inert
20 Communities → announcements Announcements tab Author name and body inert
21 NFT prompt Click an NFT link in a scene Description inert
22 Chat — username Have Profile A send a message Username inert, bubble not stretched
23 Chat — message body From the second account, send <size=400%>huge and <link="https://evil.example.com">Verified ✔</link> Both render as literal text; no oversized text and no clickable link
24 Chat — forged system line From the second account, send 🟢 Your wallet needs verification Renders as an ordinary message under that account's name and avatar — not as a system line with the system icon and colour
25 Chat — real system line Run /goto 10,10 ⚠️ The 🟢/🔴 result still renders as a system line, styled as before
26 Chat — links still work Send a plain https://decentraland.org ⚠️ Still becomes a blue clickable link and still opens the consent prompt
27 Photo detail Take a photo with Profile A visible, open it Name in the people list inert
28 Transaction confirmation Trigger a scene payment ⚠️ Amount, MANA icon and recipient still render; a crafted scene name cannot hide them

Additional Testing Notes

  • Profile B is the important one. It carries no angle brackets at all, so it is the case a naive filter misses. If B renders huge or coloured anywhere, the fix is incomplete.
  • Profile C must not vanish. An emoji-only name previously risked rendering as an empty label — it must still show.
  • Profile D is truncated on purpose: names cut at 64 characters with a trailing , bodies at 1000. That is expected, not a bug.
  • Angle brackets in legitimate text may appear as ‹ › lookalikes. Expected. Straight quotes and apostrophes in ordinary prose must be untouched.
  • The chat rows matter most for regressions. Chat is the one surface where the gate changed, not just a flag: real /goto system lines must still look like system lines, and ordinary URLs must still linkify.
  • Rows 4, 5, 7, 15, 16, 25, 26 and 28 are the regression-sensitive ones — worth checking against a dev build side by side.

Quality Checklist

  • Changes have been tested locally
  • Documentation has been updated (if required)
  • Performance impact has been considered
  • For SDK features: Test scene is included

🤖 Generated with Claude Code

mikhail-dcl and others added 2 commits August 3, 2026 13:18
…C-008/034/050/084)

Attacker-authored strings reached rich-text TMP labels, and an attacker-authored
link could open without consent.

SEC-008 — the prompt rendered its destination, and a passport link its title, as
rich text, so markup could dress an arbitrary URL up as a familiar one and make
the single approval easy to obtain. Rich text is off in the prefabs for labels
that carry nothing but untrusted text. The http(s) allowlist and (scheme, host)
trust key from #9466 were verified, not changed.

SEC-084 — event and place descriptions were linkified without escaping the
author's own markup, and a resulting click went straight to the browser sink with
no prompt. Descriptions now escape before linkification and route clicks through
the external-URL prompt. The escaping is opt-in via a new entry point because the
four other linkifier callers build links from pre-authored <link=ID> markup that
must stay live.

SEC-034 / SEC-050 — profile, member, announcement and notification sinks bound
raw names. They now use the filtered ValidatedName where available, escape where
the label's own copy is markup that must keep working, and cap length everywhere.

RichTextSanitizer is the single escaper, replacing a private duplicate in
TransactionRecipientUtils. It also escapes the backslash: TMP decodes a
backslash-u sequence into the character it denotes inside the array its tag
parser reads, so a brackets-only filter let markup through unparsed. That branch
is gated by neither parseCtrlCharacters nor the input-source check above it,
which ships commented out, so escaping is the only available defence. This also
closes the hole in the transaction-confirmation copy, where a scene name could
otherwise hide the recipient and amount.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Follow-up to 7243c73, covering the sinks the first pass left and three more
found while checking them.

Prefab flags, for labels that render nothing but text another user wrote: place
and event names, coordinates and event list entries in the navmap; the community
result card and card header descriptions; the NFT prompt description; the chat
username; and NameContainer, which covers the announcement author, friend panel,
push notification and photo-detail person card at once.

Code escaping, for labels whose own copy is markup and would break if turned
plain: the navmap <b> host and creator templates, and FriendRequestController's
three message sinks.

Not previously listed, same class, now fixed: FriendPushNotificationView and
FriendPanelUserView bound the raw Name rather than the filtered ValidatedName,
FriendRequestController assigned a raw message body to a rich-text label, and
the community name, title and owner were unescaped alongside the descriptions.

PlaceToast.prefab binds four view fields — LiveEventNameLabel, DescriptionLabel,
CoordinatesLabel and ParcelCountLabel — to one TMP component, so turning rich
text off for the coordinates would also silence the description's links. Those
writers escape in code instead. UntrustedTextLabelsShould now pins both halves
of the contract, nine labels plain and five deliberately rich, and asserts that
shared binding so the guard cannot be "fixed" by flipping the flag it protects.

Also drops the SimpleUserNameElement overload that had no callers, and folds the
announcement body cap into RichTextSanitizer.DEFAULT_BODY_LENGTH.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@mikhail-dcl
mikhail-dcl requested review from a team as code owners August 3, 2026 10:19
@github-actions
github-actions Bot requested a review from anicalbano August 3, 2026 10:19
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

@decentraland-bot
decentraland-bot self-requested a review August 3, 2026 10:19

@decentraland-bot decentraland-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

STEP 2 — Root-cause check: PASS

The PR correctly identifies and fixes the root causes:

  1. TMP markup injection — Untrusted text (user-authored names, descriptions, announcements) reached rich-text TMP labels without escaping, allowing an attacker to inject <size>, <color>, <link> tags.
  2. Missing consent prompt — Event/place description links opened directly via the OS browser without asking the user to approve.
  3. \uXXXX bypass — TMP decodes backslash-u escape sequences inside PopulateTextProcessingArray (case 117), bypassing a brackets-only filter. The fix neutralizes the backslash alongside the angle brackets.

The fix is at the right level: a shared escaper (RichTextSanitizer) applied at every sink, complemented by prefab-level richText=false where the label renders nothing but untrusted text.

STEP 3 — Design & integration: PASS

RichTextSanitizer is a stateless static utility class — it holds no lifecycle state, manages no entities or resources, and belongs in DCL.UI alongside the TMP labels it protects. No lifecycle owner search needed.

SetAuthorTextWithClickeableLinks combines escaping, assignment, linkification, and consent-prompt routing in a single call. The coupling is intentional — the docstring correctly notes that separating escape from assignment invites drift (SEC-084). It accesses ViewDependencies.GlobalUIViews statically, trading constructor-injected UnityAppWebBrowser for a hidden dependency. ViewDependencies is an established pattern in this codebase, and the trade buys a simpler constructor surface and the guarantee that all description links route through consent. Worth documenting as tech debt for testability in a follow-up.

The removal of webBrowser from EventInfoPanelController and PlaceInfoPanelController constructors is clean — all callers in ExplorePanelPlugin are updated.

STEP 4 — Member audit

Member Consumers Assessment
RichTextSanitizer.Escape ~6 direct + via SetAuthorTextWithClickeableLinks Well-scoped
RichTextSanitizer.EscapeAttribute 1 (TransactionRecipientUtils.HighlightLink) Distinct responsibility (attribute context); not a merge candidate
RichTextSanitizer.EscapeAndTruncate ~15 sinks Well-scoped
RichTextSanitizer.Truncate ~4 sinks Well-scoped
SetAuthorTextWithClickeableLinks 2 sinks (event/place description) Documents its coupling contract
IsTrusted 2 within ExternalUrlPromptController Clean extraction of duplicated condition
SetUserName (private) 1 (Setup) Thin setter with a security contract documented in its summary — acceptable per §11

No single-use derived predicates or absent-≠-false problems found.

STEP 5 — Line-level findings

See inline comments. Three P2 findings, no P0/P1.

Additional note (not inline): In ExternalUrlPromptController, the consent dialog displays uri.AbsoluteUri (line 103), but all three OpenUrlMainThreadOnly calls (lines 49, 69, 72) pass uri.OriginalString. The comment at line 100 claims AbsoluteUri is “the canonical form UnityAppWebBrowser hands to Application.OpenURL” — if that’s true, using OriginalString in the controller creates a theoretical mismatch between what the user consents to and what opens. Not blocking (the domain is always the same, and the labels have richText off), but worth aligning in a follow-up.

Security review: No issues found

The escaping is complete:

  • <, >, \ are the only independent markup entry points in TMP. {/} only matter inside <sprite> attributes (which require < first). Unicode normalization (NFKC) would map back to \, but TMP does not normalize its input buffer, so no bypass exists.
  • \uXXXX / \UXXXXXXXX sequences are handled by escaping \.
  • All description links now route through the consent prompt. Non-web schemes are blocked by the existing http(s) allowlist in ExternalUrlPromptController.Params.
  • Prefab guards (UntrustedTextLabelsShould) pin the richText flags on shipped assets, preventing accidental re-enablement.

Summary

Thorough, well-implemented security fix. Two defence layers (prefab flags + code escaping) cover all UI sinks that render untrusted text. The RichTextSanitizer is allocation-free for the common case (no markup) and handles edge cases (surrogate pairs, \uXXXX sequences). Six new test classes (~870 lines) provide comprehensive coverage of the sanitizer, name element, notification text, description links, external URL prompt, and prefab label assertions.

No P0 or P1 issues. Three P2 findings noted in inline comments.


REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Security hardening across UI rendering of untrusted text in navmap, friends, notifications, communities, chat, passport, external URL prompt, with a new shared sanitizer utility and async consent flow changes
QA_REQUIRED: YES


Reviewed by Jarvis 🤖 · Requested by decentraland-bot via GitHub

Comment thread Explorer/Assets/DCL/UI/Profiles/ProfileElements/SimpleUserNameElement.cs Outdated
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

🔍 Jarvis reviewed this PR and found no blocking issues, but assessed it as complex — human DEV review is still required before merging.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

badge

Warnings count reduced: 13940 => 13886

Warnings/errors in files changed by this PR (116)
Assets/DCL/InWorldCamera/PhotoDetail/VisiblePersonController.cs:84  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetBackgroundColor(Color)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/PushNotifications/FriendPushNotificationView.cs:52  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.Setup(ProfileRepositoryWrapper, Color, string?, string)' is obsolete: 'Use Bind instead.'
Assets/DCL/Communities/CommunitiesBrowser/CommunityResultCardView.cs:298  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.Setup(ProfileRepositoryWrapper, in CompactInfo)' is obsolete: 'Use Bind instead.'
Assets/DCL/Communities/CommunitiesCard/Announcements/AnnouncementCardView.cs:102  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.Setup(ProfileRepositoryWrapper, in CompactInfo)' is obsolete: 'Use Bind instead.'
Assets/DCL/Communities/CommunitiesCard/Members/MemberListItemView.cs:118  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.Setup(ProfileRepositoryWrapper, in CompactInfo)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/FriendPanel/Sections/FriendPanelUserView.cs:81  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.Setup(ProfileRepositoryWrapper, in CompactInfo)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/Requests/FriendRequestController.cs:180  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetupAsync(ProfileRepositoryWrapper, Color, string?, string, CancellationToken, bool)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/Requests/FriendRequestController.cs:242  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetupAsync(ProfileRepositoryWrapper, Color, string?, string, CancellationToken, bool)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/Requests/FriendRequestController.cs:269  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetupAsync(ProfileRepositoryWrapper, Color, string?, string, CancellationToken, bool)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/Requests/FriendRequestController.cs:401  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetupAsync(ProfileRepositoryWrapper, Color, string?, string, CancellationToken, bool)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/Requests/FriendRequestController.cs:408  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetupAsync(ProfileRepositoryWrapper, Color, string?, string, CancellationToken, bool)' is obsolete: 'Use Bind instead.'
Assets/DCL/InWorldCamera/PhotoDetail/VisiblePersonController.cs:95  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetupAsync(ProfileRepositoryWrapper, Color, string?, string, CancellationToken, bool)' is obsolete: 'Use Bind instead.'
Assets/DCL/NftPrompt/NftPromptController.cs:51  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/NftPrompt/NftPromptController.cs:81  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/NftPrompt/NftPromptController.cs:117  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/NftPrompt/NftPromptController.cs:125  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/NftPrompt/NftPromptController.cs:133  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/NftPrompt/NftPromptController.cs:141  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/NftPrompt/NftPromptController.cs:154  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/Friends/UI/PushNotifications/FriendPushNotificationView.cs:57  CSharpWarnings::CS8604  Possible null reference argument for parameter 'audioClipConfig' in 'DCL.Audio.UIAudioEventsBus.SendPlayAudioEvent'
Assets/DCL/Communities/CommunitiesCard/CommunityCardView.cs:250  CSharpWarnings::CS8604  Possible null reference argument for parameter 'config' in 'DCL.UI.GenericContextMenuParameter.GenericContextMenuParameter'
Assets/DCL/Navmap/PlaceInfoPanelController.cs:172  CSharpWarnings::CS8604  Possible null reference argument for parameter 'creatorAddress' in 'DCL.Donations.UI.DonationsPanelParameter.Create'
Assets/DCL/Navmap/EventInfoPanelController.cs:185  CSharpWarnings::CS8604  Possible null reference argument for parameter 'eventId' in 'DCL.EventsApi.HttpEventsApiService.MarkAsInterestedAsync'
Assets/DCL/Navmap/EventInfoPanelController.cs:187  CSharpWarnings::CS8604  Possible null reference argument for parameter 'eventId' in 'DCL.EventsApi.HttpEventsApiService.MarkAsNotInterestedAsync'
Assets/DCL/Navmap/PlaceInfoPanelController.cs:531  CSharpWarnings::CS8604  Possible null reference argument for parameter 'placeId' in 'DCL.InWorldCamera.CameraReelGallery.CameraReelGalleryController.ShowPlaceGalleryAsync'
Assets/DCL/Navmap/EventInfoPanelController.cs:162  CSharpWarnings::CS8604  Possible null reference argument for parameter 'title' in 'DCL.EventsApi.GoogleUserCalendar.Add'
Assets/DCL/Notifications/NotificationEntry/FriendsNotificationView.cs:21  CSharpWarnings::CS8618  Non-nullable event 'NotificationClicked' is uninitialized. Consider adding the 'required' modifier or declaring the event as nullable.
Assets/DCL/NftPrompt/NftPromptController.cs:35  CSharpWarnings::CS8618  Non-nullable field 'placeImageController' must contain a non-null value when exiting constructor. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Navmap/PlaceInfoPanelController.cs:65  CSharpWarnings::CS8618  Non-nullable fields 'homeButton', 'cameraReelGalleryController' must contain non-null values when exiting constructor. Consider adding the 'required' modifiers or declaring the fields as nullable.
Assets/DCL/Profiles/SharedAPI/Profile.CompactInfo.cs:79  CSharpWarnings::CS8618  Non-nullable members 'userId', 'ValidatedName', 'DisplayName', 'UnclaimedName', 'MentionName' must contain non-null values when exiting constructor. Consider adding the 'required' modifiers or declaring the members as nullable.
Assets/DCL/Notifications/NotificationEntry/FriendsNotificationView.cs:23  CSharpWarnings::CS8618  Non-nullable property 'Notification' is uninitialized. Consider adding the 'required' modifier or declaring the property as nullable.
Assets/DCL/Navmap/PlaceInfoPanelController.cs:81  CSharpWarnings::CS8625  Cannot convert null literal to non-nullable reference type
Assets/DCL/NftPrompt/NftPromptController.cs:192  CSharpWarnings::CS8625  Cannot convert null literal to non-nullable reference type
Assets/DCL/Navmap/PlaceInfoPanelController.cs:531  ConditionalAccessQualifierIsNonNullableAccordingToAPIContract  Conditional access qualifier expression is never null according to nullable reference types' annotations
Assets/DCL/NftPrompt/NftPromptController.cs:73  ConditionalAccessQualifierIsNonNullableAccordingToAPIContract  Conditional access qualifier expression is never null according to nullable reference types' annotations
Assets/DCL/Notifications/NotificationEntry/FriendsNotificationView.cs:151  ConditionalAccessQualifierIsNonNullableAccordingToAPIContract  Conditional access qualifier expression is never null according to nullable reference types' annotations
Assets/DCL/Tests/Editor/ChatHistoryServiceSanitizationShould.cs:53  ConditionalAccessQualifierIsNonNullableAccordingToAPIContract  Conditional access qualifier expression is never null according to nullable reference types' annotations
Assets/DCL/Communities/CommunitiesCard/Members/MemberListItemView.cs:67  InconsistentNaming  Name 'CanUnHover' does not match rule 'non_public_members_should_be_camel_case'. Suggested name is 'canUnHover'.
Assets/DCL/Friends/UI/FriendPanel/Sections/FriendPanelUserView.cs:37  InconsistentNaming  Name 'CanUnHover' does not match rule 'non_public_members_should_be_camel_case'. Suggested name is 'canUnHover'.
Assets/DCL/PluginSystem/Global/ExplorePanelPlugin.cs:786  InconsistentNaming  Name 'EventInfoPrefab' does not match rule 'non_public_members_should_be_camel_case'. Suggested name is 'eventInfoPrefab'.
Assets/DCL/PluginSystem/Global/ExplorePanelPlugin.cs:785  InconsistentNaming  Name 'PlaceDetailPanelPrefab' does not match rule 'non_public_members_should_be_camel_case'. Suggested name is 'placeDetailPanelPrefab'.
Assets/DCL/PluginSystem/Global/ExplorePanelPlugin.cs:787  InconsistentNaming  Name 'QualityPresets' does not match rule 'non_public_members_should_be_camel_case'. Suggested name is 'qualityPresets'.
Assets/DCL/UI/TMP_Text_ClickeableLink.cs:9  InconsistentNaming  Name 'TMP_Text_ClickeableLink' does not match rule 'members_should_be_pascal_case'. Suggested name is 'TmpTextClickeableLink'.
Assets/DCL/PluginSystem/Global/ExplorePanelPlugin.cs:137  InconsistentNaming  Name 'assetBundleURL' does not match rule 'non_public_members_should_be_camel_case'. Suggested name is 'assetBundleUrl'.
Assets/DCL/Communities/CommunitiesBrowser/CommunityResultCardView.cs:44  InconsistentNaming  Name 'communityThumbnail' does not match rule 'members_should_be_pascal_case'. Suggested name is 'CommunityThumbnail'.
Assets/DCL/PluginSystem/Global/ExplorePanelPlugin.cs:161  InconsistentNaming  Name 'marketplaceCreditsAPIClient' does not match rule 'non_public_members_should_be_camel_case'. Suggested name is 'marketplaceCreditsApiClient'.
Assets/DCL/PluginSystem/Global/ExplorePanelPlugin.cs:265  InconsistentNaming  Name 'marketplaceCreditsAPIClient' does not match rule 'parameters_should_be_camel_case'. Suggested name is 'marketplaceCreditsApiClient'.
Assets/DCL/NftPrompt/NftPromptController.cs:26  InconsistentNaming  Name 'nftInfoAPIClient' does not match rule 'non_public_members_should_be_camel_case'. Suggested name is 'nftInfoApiClient'.
Assets/DCL/NftPrompt/NftPromptController.cs:39  InconsistentNaming  Name 'nftInfoAPIClient' does not match rule 'parameters_should_be_camel_case'. Suggested name is 'nftInfoApiClient'.
Assets/DCL/Communities/CommunitiesBrowser/CommunityResultCardView.cs:413  InconsistentNaming  Name 'picture' does not match rule 'members_should_be_pascal_case'. Suggested name is 'Picture'.

…and 66 more (see the csharp-lint-reports artifact).

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

badge

All Unity tests passed ✅

TESTS SUITE Result Passed Failed Skipped
EditMode ✅ Passed 24544 0 13
PlayMode ✅ Passed 236 0 5

Review feedback on #9565.

The confirmation dialog's mainText renders rich text, and sixteen call sites
format community, member and place names into it — the reviewer flagged one of
them (CommunityResultCardView's raw currentCommunityName). None of the nine
dialog text arguments carries markup of its own, so the fix is the flag rather
than an escape at each site: richText off on mainText and subText. additonalUrlText
stays rich, since it carries the linkified URL the dialog can show.

The ValidatedName-with-fallback pattern that had spread to four call sites now
lives on Profile.CompactInfo as ValidatedNameOrRaw. Named for what it does rather
than "safe": the fallback is verbatim user input, so a label rendering it still
needs richText off or an escape, and a name implying otherwise is how this class
of bug comes back.

Also clears 36 nullable-reference warnings across the files this PR touches, which
unblocks the warning ratchet. Mostly `= null!` on Unity-serialized fields, the
established convention here; three private fields in NftPromptController that were
declared non-nullable while every call site already treated them as nullable; and
PlaceInfoPanelController.thumbnailImage, where the annotation was the lie — it is
readonly, assigned unconditionally from a factory that always returns a new
instance, and already dereferenced unconditionally elsewhere.

Left alone deliberately: Profile.CompactInfo's uninitialized members, because
MentionName and UnclaimedName genuinely end up null for a name with no
alphanumerics and seeding them would change behaviour for every consumer; and two
latent NREs in PlaceInfoPanelController where cameraReelGalleryController and
homeButton are assigned under a null check but dereferenced without one. Both want
their own change, not this one.

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

@decentraland-bot decentraland-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review: fix: neutralize untrusted text and gate external URLs in UI sinks

STEP 2 — Root-cause check

Problem: Text written by other users — profile and community names, event/place descriptions, announcements, friend-request messages — reached rich-text TMP labels unsanitized. An attacker-authored <link> could open a URL with no consent prompt, and <size=0> / <color=#00000000> could hide critical copy (such as a transaction's recipient and amount). Additionally, TMP's \uXXXX escape-sequence decoder could be used to bypass a brackets-only filter.

Does the diff fix the cause? Yes. The PR addresses the root cause at every UI sink rather than patching a symptom. Two correct mechanisms are used per label:

  1. richText off in the prefab for labels that render nothing but untrusted text.
  2. Code-level escaping via the new RichTextSanitizer for labels whose own copy is styled markup that must keep working.

Description links that previously opened through webBrowser.OpenUrlMainThreadOnly (no consent) are now routed through the external-URL consent prompt. The webBrowser dependency is cleanly removed from both PlaceInfoPanelController and EventInfoPanelController.

PASS


STEP 3 — Design & integration

RichTextSanitizer (new static utility, DCL.UI namespace)

This is a pure, stateless string transformation — no ECS entities, no Unity lifecycle, no persistent state. A static class is the correct shape; it belongs with the TMP UI infrastructure it protects, not in an ECS system. The codebase already uses TextMeshProExtensions in the same namespace for TMP helpers.

The string.Create with SpanAction approach is well-suited to the GC-pressure constraints:

  • Common case (no markup): IndexOfMarkup short-circuits and returns the original string reference — zero allocation. Pinned by ReturnTheSameInstanceWhenThereIsNothingToEscape.
  • Escape case: One allocation via string.Create; the static lambda avoids closure allocation.
  • Truncate+escape: Cut, swaps, and ellipsis all land in one buffer — one allocation instead of Substring + escape + concat.

ValidatedNameOrRaw on Profile.CompactInfo — correctly placed on the type that owns both ValidatedName and Name. Pure computed property, no side effects, no allocation. Used by 4+ call sites.

SetAuthorTextWithClickeableLinks — correct composition: escape → assign → linkify → route clicks through consent prompt. Centralizing this prevents the escape-then-linkify steps from drifting apart across callers.

Removed webBrowser parameters — confirmed both EventInfoPanelController.OpenUrl and PlaceInfoPanelController.OpenUrl were the sole consumers. Three call sites in ExplorePanelPlugin updated. Clean removal, no dangling references.

Deleted SimpleUserNameElement.Setup(string, string, bool, Color) overload — all 5 callers (ProfileInputSuggestionElement, DonationLoadingView, DonationConfirmedView, SimpleProfileView, DonationDefaultView) use the CompactInfo overload. No external consumers of the deleted overload exist.

IsTrusted extraction in ExternalUrlPromptController — used by two call sites (OnViewShow and WaitForCloseIntentAsync), so the extraction eliminates duplication and is justified.

PASS


STEP 4 — Member audit

Member Consumer count Verdict
RichTextSanitizer.Escape 13 (direct + via EscapeAndTruncate + via SetAuthorTextWithClickeableLinks) Core escaper, not single-use
RichTextSanitizer.EscapeAttribute 1 (TransactionRecipientUtils.HighlightLink) Justified: distinct threat model (attribute position vs content position)
RichTextSanitizer.Truncate 5 call sites Justified: labels with richText off need only a cap
RichTextSanitizer.EscapeAndTruncate 15+ call sites Primary consumer entry point
ValidatedNameOrRaw 4 call sites Justified, all callers pass through EscapeAndTruncate
IsTrusted 2 call sites Justified, eliminates duplication

PASS


STEP 5 — Line-level findings

See inline comments below. All findings are P2 (minor).


Security review

The sanitizer is sound against TMP's markup parser:

  • < and > replacement blocks all tag types (<size>, <color>, <link>, <b>, <sprite>, etc.).
  • \ replacement closes the \uXXXX / \UXXXXXXXX decode path in TMP_Text.PopulateTextProcessingArray — tested by NeutralizeMarkupSmuggledAsAUtf16EscapeSequence and the UTF-32 variant.
  • " replacement in attribute position prevents early <link="..."> closure.
  • AbsoluteUri (not OriginalString) displayed in the consent prompt — percent-encoding is a second barrier against markup smuggling in the displayed URL.
  • Null-URI path correctly clears the previous prompt's destination and callback.
  • The http(s)-only scheme allowlist in ExternalUrlPolicy is tight and tested.
  • SetAuthorTextWithClickeableLinks order of operations is correct: escape first (neutralizing injected <link> tags), then linkify bare https:// URLs, then route clicks through the consent prompt.

No P0 or P1 security issues found. ✅


STEP 8 — Non-blocking warnings

No Main.unity modification detected. ✅


REVIEW_RESULT: PASS ✅
COMPLEXITY: COMPLEX
COMPLEXITY_REASON: Touches UI sinks across navmap, friends, communities, chat, notifications, donations, passport, NFT prompt, and external URL prompt; introduces a shared security utility and rearchitects description-link routing.
QA_REQUIRED: YES


Reviewed by Jarvis 🤖 · Requested by mikhail-dcl via GitHub

Comment on lines +32 to +33
[TestCase("Assets/DCL/Communities/CommunitiesBrowser/Prefabs/CommunityResultCard.prefab", nameof(CommunityResultCardView), "communityDescription")]
[TestCase("Assets/DCL/NftPrompt/Assets/NftPrompt.prefab", nameof(NftPromptView), "<TextDescription>k__BackingField")]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Test coverage gap for CommunityCardHeader.prefab description label. This prefab's description label had richText changed from 1 to 0 in this PR, and CommunityCardView.ConfigureCommunity uses Truncate (no escaping) for that description — relying entirely on the prefab flag for safety. If someone reverts the flag in the inspector, the description becomes injectable and no test catches it.

Suggested change
[TestCase("Assets/DCL/Communities/CommunitiesBrowser/Prefabs/CommunityResultCard.prefab", nameof(CommunityResultCardView), "communityDescription")]
[TestCase("Assets/DCL/NftPrompt/Assets/NftPrompt.prefab", nameof(NftPromptView), "<TextDescription>k__BackingField")]
[TestCase("Assets/DCL/Communities/CommunitiesBrowser/Prefabs/CommunityResultCard.prefab", nameof(CommunityResultCardView), "communityDescription")]
[TestCase("Assets/DCL/Communities/CommunitiesCard/Prefabs/CommunityCardHeader.prefab", "CommunityCardView", "<communityDescription>k__BackingField")]
[TestCase("Assets/DCL/NftPrompt/Assets/NftPrompt.prefab", nameof(NftPromptView), "<TextDescription>k__BackingField")]

mikhail-dcl and others added 3 commits August 4, 2026 13:01
…(SEC-023)

An incoming chat message skipped the formatting pipeline when its text merely
began with 🟢/🔴/🟡 followed by system-looking punctuation. That is a content
check, so any co-located peer could prefix their message and choose whether their
own text was processed.

Two things the finding's mitigation note gets wrong, both verified here.

ChatEntryMessageContentElement.prefab does carry m_isRichText: 0, but
ChatEntryMessageBubbleElement.prefab overrides it back to 1 for the content
element nested inside it, and that bubble is embedded in ChatEntry_BASE.prefab —
the base of the entries the feed instantiates. Peer message text therefore renders
as rich text, with parseCtrlCharacters on, so escape sequences decode there too.

And FormatText never sanitized anything: its own tests show it wraps URLs in
<link=url> and leaves the rest untouched. Skipping it did not create the
injection, which means the injection applied to every peer message, prefix or not.
So the fix is escaping, not merely closing the prefix hole: peer text is escaped
before formatting, leaving the peer's markup inert while the <link> and <color>
runs the formatter adds still work.

The skip now reads IsSystemMessage alone, and IsCopyOfSystemMessage,
AfterMarkerLooksLikeSystem and TrimStartIgnorables are gone.

The spoofing leg needs no change: the feed picks the system prefab
(ChatMessageFeedView) and the system icon and colour (CreateMessageViewModelCommand)
off IsSystemMessage, which a peer cannot set, so a 🟢 message from a peer renders
in an ordinary bubble under its sender's name.

Not done here: the finding also asks for IsSystemMessage to be persisted, since
the history serializer stores four fields and omits it — which is why the prefix
sniff existed. Changing an encrypted-history format needs backward-compatible
reads and belongs in its own change. The consequence meanwhile is benign: a
reloaded system line is escaped and linkified like any other text, and system copy
carries no markup.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Conflicts:
#	Explorer/Assets/DCL/Navmap/PlaceInfoPanelController.cs
The earlier passes covered the community card and browser result card. A
community name reaches seven more labels, each authored by whoever owns the
community: the event card's community line, the community title header, the
"my communities" list, the requests-received group header, both voice-chat call
views, and the chat titlebar — which carries a community name for a community
channel and the other party's name for a DM.

The world chat bubble is escaped one step earlier, in ChatWorldBubbleService,
where community data enters the bubble path. Its label is NametagElement, a UI
Toolkit Label whose assembly (DCL.AvatarRendering) cannot see this helper, and
adding that assembly reference to reach one label is the worse trade. UI Toolkit
text has rich text on by default and uses the same bracket syntax, so the risk is
real and the same escaper answers it.

Also aligns the authoring field with the display path. Name length already
agreed — the input caps at 30 against a 64 display cap — but the input accepted
characters the display escapes, so typing "<3" would render back as "‹3". The
input now rejects exactly what Escape rewrites, reading the set from
RichTextSanitizer.IsRewritten so the two cannot drift, with a test pinning that
they agree in both directions. This is consistency, not protection: the
Communities API is open, so a name that never passes through this field is
precisely what the escaping exists for.

Test fix, unrelated to the above: the dev merge changed Profile.CompactInfo's
first parameter from string to UserId, which left two of these test files
uncompilable. They now build the id with UserId.New(...).Unwrap(), the pattern
the existing tests use for known-valid constants.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

🚦 CI Status

Build

Windows and Mac build successful in Unity Cloud! You can find a link to the downloadable artifact below.

Name Link
Commit fc96670
Logs https://github.com/decentraland/unity-explorer/actions/runs/32022738957
Download Windows https://github.com/decentraland/unity-explorer/suites/86812079470/artifacts/9286976969
Download Windows S3 https://explorer-artifacts.decentraland.org/@dcl/unity-explorer/branch/fix/sec-008-external-url-hardening/pr-25192-fc96670/Decentraland_windows64.zip
Download Mac https://github.com/decentraland/unity-explorer/suites/86812079470/artifacts/9286988338
Download Mac S3 https://explorer-artifacts.decentraland.org/@dcl/unity-explorer/branch/fix/sec-008-external-url-hardening/pr-25192-fc96670/Decentraland_macos.zip
Built on 2026-08-17T11:37:39Z

Lint

Warnings count reduced: 13156 => 13112

Warnings/errors in files changed by this PR (157)
Assets/DCL/InWorldCamera/PhotoDetail/VisiblePersonController.cs:84  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetBackgroundColor(Color)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/PushNotifications/FriendPushNotificationView.cs:52  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.Setup(ProfileRepositoryWrapper, Color, string?, string)' is obsolete: 'Use Bind instead.'
Assets/DCL/Communities/CommunitiesBrowser/CommunityResultCardView.cs:298  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.Setup(ProfileRepositoryWrapper, in CompactInfo)' is obsolete: 'Use Bind instead.'
Assets/DCL/Communities/CommunitiesCard/Announcements/AnnouncementCardView.cs:102  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.Setup(ProfileRepositoryWrapper, in CompactInfo)' is obsolete: 'Use Bind instead.'
Assets/DCL/Communities/CommunitiesCard/Members/MemberListItemView.cs:118  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.Setup(ProfileRepositoryWrapper, in CompactInfo)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/FriendPanel/Sections/FriendPanelUserView.cs:81  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.Setup(ProfileRepositoryWrapper, in CompactInfo)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/Requests/FriendRequestController.cs:180  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetupAsync(ProfileRepositoryWrapper, Color, string?, string, CancellationToken, bool)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/Requests/FriendRequestController.cs:242  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetupAsync(ProfileRepositoryWrapper, Color, string?, string, CancellationToken, bool)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/Requests/FriendRequestController.cs:269  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetupAsync(ProfileRepositoryWrapper, Color, string?, string, CancellationToken, bool)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/Requests/FriendRequestController.cs:401  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetupAsync(ProfileRepositoryWrapper, Color, string?, string, CancellationToken, bool)' is obsolete: 'Use Bind instead.'
Assets/DCL/Friends/UI/Requests/FriendRequestController.cs:408  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetupAsync(ProfileRepositoryWrapper, Color, string?, string, CancellationToken, bool)' is obsolete: 'Use Bind instead.'
Assets/DCL/InWorldCamera/PhotoDetail/VisiblePersonController.cs:95  CSharpWarnings::CS0618  CS0618: Method 'DCL.UI.ProfileElements.ProfilePictureView.SetupAsync(ProfileRepositoryWrapper, Color, string?, string, CancellationToken, bool)' is obsolete: 'Use Bind instead.'
Assets/DCL/NftPrompt/NftPromptController.cs:51  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/NftPrompt/NftPromptController.cs:81  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/NftPrompt/NftPromptController.cs:117  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/NftPrompt/NftPromptController.cs:125  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/NftPrompt/NftPromptController.cs:133  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/NftPrompt/NftPromptController.cs:141  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/NftPrompt/NftPromptController.cs:154  CSharpWarnings::CS8602  Dereference of a possibly null reference
Assets/DCL/Friends/UI/Requests/FriendRequestController.cs:408  CSharpWarnings::CS8604  Possible null reference argument for parameter '_' in 'DCL.UI.ProfileElements.ProfilePictureView.SetupAsync'
Assets/DCL/Friends/UI/PushNotifications/FriendPushNotificationView.cs:57  CSharpWarnings::CS8604  Possible null reference argument for parameter 'audioClipConfig' in 'DCL.Audio.UIAudioEventsBus.SendPlayAudioEvent'
Assets/DCL/Communities/CommunitiesCard/CommunityCardView.cs:250  CSharpWarnings::CS8604  Possible null reference argument for parameter 'config' in 'DCL.UI.GenericContextMenuParameter.GenericContextMenuParameter'
Assets/DCL/Navmap/EventInfoPanelController.cs:185  CSharpWarnings::CS8604  Possible null reference argument for parameter 'eventId' in 'DCL.EventsApi.HttpEventsApiService.MarkAsInterestedAsync'
Assets/DCL/Navmap/EventInfoPanelController.cs:187  CSharpWarnings::CS8604  Possible null reference argument for parameter 'eventId' in 'DCL.EventsApi.HttpEventsApiService.MarkAsNotInterestedAsync'
Assets/DCL/Communities/CommunityCreation/CommunityCreationEditionView.cs:226  CSharpWarnings::CS8604  Possible null reference argument for parameter 'sprite' in 'DCL.UI.ImageView.SetImage'
Assets/DCL/Navmap/EventInfoPanelController.cs:162  CSharpWarnings::CS8604  Possible null reference argument for parameter 'title' in 'DCL.EventsApi.GoogleUserCalendar.Add'
Assets/DCL/Communities/CommunitiesBrowser/CommunityResultCardView.cs:299  CSharpWarnings::CS8604  Possible null reference argument for parameter 'wallet' in 'DCL.FeatureFlags.OfficialWalletsHelper.IsOfficialWallet'
Assets/DCL/Friends/UI/FriendPanel/Sections/FriendPanelUserView.cs:80  CSharpWarnings::CS8604  Possible null reference argument for parameter 'wallet' in 'DCL.FeatureFlags.OfficialWalletsHelper.IsOfficialWallet'
Assets/DCL/Notifications/NotificationEntry/FriendsNotificationView.cs:21  CSharpWarnings::CS8618  Non-nullable event 'NotificationClicked' is uninitialized. Consider adding the 'required' modifier or declaring the event as nullable.
Assets/DCL/UI/Communities/CommunityTitleView.cs:23  CSharpWarnings::CS8618  Non-nullable field 'ContextMenuClosed' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/UI/Communities/CommunityTitleView.cs:22  CSharpWarnings::CS8618  Non-nullable field 'ContextMenuOpened' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/UI/Communities/CommunityTitleView.cs:24  CSharpWarnings::CS8618  Non-nullable field 'ViewCommunityRequested' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Chat/_Refactor/ChatViews/ChatDefaultTitlebarView.cs:29  CSharpWarnings::CS8618  Non-nullable field 'buttonClose' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Chat/_Refactor/ChatViews/ChatDefaultTitlebarView.cs:31  CSharpWarnings::CS8618  Non-nullable field 'buttonOpenContextMenu' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Chat/_Refactor/ChatViews/ChatDefaultTitlebarView.cs:30  CSharpWarnings::CS8618  Non-nullable field 'buttonOpenMembers' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Chat/_Refactor/ChatViews/ChatDefaultTitlebarView.cs:32  CSharpWarnings::CS8618  Non-nullable field 'buttonOpenProfileContextMenu' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Chat/_Refactor/ChatViews/ChatDefaultTitlebarView.cs:34  CSharpWarnings::CS8618  Non-nullable field 'buttonStartCall' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Chat/_Refactor/ChatViews/ChatDefaultTitlebarView.cs:38  CSharpWarnings::CS8618  Non-nullable field 'chatProfileView' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Chat/_Refactor/ChatViews/ChatDefaultTitlebarView.cs:54  CSharpWarnings::CS8618  Non-nullable field 'connectionStatusIndicator' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/UI/Communities/CommunityTitleView.cs:42  CSharpWarnings::CS8618  Non-nullable field 'contextMenuConfig' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/UI/Communities/CommunityTitleView.cs:37  CSharpWarnings::CS8618  Non-nullable field 'contextMenuSettings' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/UI/Communities/CommunityTitleView.cs:40  CSharpWarnings::CS8618  Non-nullable field 'cts' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Communities/CommunitiesBrowser/MyCommunityCardView.cs:25  CSharpWarnings::CS8618  Non-nullable field 'currentCommunityId' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Chat/_Refactor/ChatViews/ChatDefaultTitlebarView.cs:48  CSharpWarnings::CS8618  Non-nullable field 'currentTitlebarViewModel' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Communities/CommunitiesBrowser/MyCommunityCardView.cs:20  CSharpWarnings::CS8618  Non-nullable field 'listenersCountView' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Communities/CommunitiesBrowser/MyCommunityCardView.cs:21  CSharpWarnings::CS8618  Non-nullable field 'listeningTooltip' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Chat/_Refactor/ChatViews/ChatDefaultTitlebarView.cs:41  CSharpWarnings::CS8618  Non-nullable field 'loadingView' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Chat/_Refactor/ChatViews/ChatDefaultTitlebarView.cs:40  CSharpWarnings::CS8618  Non-nullable field 'nearbyAutoTranslateIndicator' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/Chat/_Refactor/ChatViews/ChatDefaultTitlebarView.cs:39  CSharpWarnings::CS8618  Non-nullable field 'nearbyElementsContainer' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.
Assets/DCL/UI/Communities/CommunityTitleView.cs:39  CSharpWarnings::CS8618  Non-nullable field 'openContextMenu' is uninitialized. Consider adding the 'required' modifier or declaring the field as nullable.

…and 107 more (see the csharp-lint-reports artifact).

Tests

All Unity tests passed ✅

TESTS SUITE Result Passed Failed Skipped
EditMode ✅ Passed 25088 0 13
PlayMode ✅ Passed 236 0 37

mikhail-dcl and others added 2 commits August 17, 2026 13:58
A profile name is a schema-valid string that no backend rejects, so the client is
the only thing between a crafted one and the labels that render it. Testing that
honestly means putting a crafted name on a real profile and letting it come back
from the catalyst, rather than trusting a unit test's idea of the payload.

/setname publishes the name raw. It deliberately filters nothing: whatever the UI
does with it on the way back is the thing under test.

Debug-only, via the existing IChatCommand hook. Not because it grants an attacker
anything — the profile API is open and a crafted profile is exactly this — but
because it is an instrument, not a feature.

Two details the dispatcher forces. It splits the message on spaces, so the
parameters are rejoined here; and it collapses ", " into "," before splitting,
which cannot be undone at this layer. Hence \uXXXX decoding, which is also the way
to set characters the chat input will not carry and to reproduce the escape
sequences TMP decodes on its own.

The reply echoes the name escaped and adds the derived ValidatedName. Command
replies are emitted as system messages, the one path deliberately left
unsanitized, so echoing raw markup there would inject it into the client's own
copy and confuse the test being run. A failed publish restores the previous name.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The command is a testing instrument, not shipped behaviour, so pinning it with
its own suite buys maintenance rather than confidence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@decentraland-bot

Copy link
Copy Markdown
Contributor

PR #9565, run #32025873407

Builds: Windows change, Windows baseline, macOS change, macOS baseline

How to read this table
  • Each build is measured 3 times. The values are the median, and (min–max) is the lowest and highest of those runs — a wide range means the metric is noisy and small differences are not trustworthy.
  • Δ is Change minus Baseline (a negative Δ means Change is faster).
  • 🟢 faster / 🔴 slower — a real difference: larger than both 3% and the run-to-run range.
  • ⚪ within noise — the difference is smaller than how much the build varies between its own runs, so it cannot be told apart from random variation. Treat it as no change.
  • Exceptions per run — the average number of exceptions in a run's log; more than the baseline is flagged 🔴 even when frame times look fine. The Exception breakdown under each table groups them by the explorer's report category and exception type (as totals across the runs).
  • A run that logged unusually many exceptions (at least 10 and 5× the median of its build's runs — e.g. a service was down during it) is excluded from all numbers and called out under the table.

Intel Core i5

Metric Baseline Change Δ Result
Samples 2701 (×3) 2397 (×3)
CPU average 33.2 ms (33.2–34.6) 37.3 ms (37.3–38.2) 4.1 ms 🔴 12% slower
CPU 1% worst 34.3 ms (33.5–184.5) 286.6 ms (284.3–303.2) 252.4 ms 🔴 737% slower
CPU 0.1% worst 41.4 ms (33.7–332.0) 305.0 ms (304.9–317.5) 263.6 ms ⚪ within noise
GPU average 9.3 ms (9.2–9.4) 9.5 ms (9.4–9.6) 0.2 ms ⚪ within noise
GPU 1% worst 20.7 ms (19.8–26.9) 31.9 ms (30.6–32.7) 11.2 ms 🔴 54% slower
GPU 0.1% worst 36.3 ms (31.6–37.7) 38.8 ms (37.5–39.1) 2.4 ms ⚪ within noise
Exceptions per run 66 66 0 ⚪ none new
Exception breakdown
Exception Baseline (3 runs) Change (3 runs)
[UI] DllNotFoundException 192 192
[ENGINE] NullReferenceException 3 3
[ENGINE] ObjectDisposedException 3 3

Apple M1

Metric Baseline Change Δ Result
Samples 4368 (×3) 4074 (×2)
CPU average 20.5 ms (20.3–21.6) 23.2 ms (22.0–23.2) 2.7 ms 🔴 13% slower
CPU 1% worst 34.7 ms (33.9–34.7) 229.8 ms (229.7–229.8) 195.1 ms 🔴 562% slower
CPU 0.1% worst 34.9 ms (34.9–35.3) 237.5 ms (232.5–237.5) 202.6 ms 🔴 580% slower
GPU average 1.0 ms (0.1–1.6) 9.4 ms (2.2–9.4) 8.4 ms 🔴 871% slower
GPU 1% worst 34.2 ms (7.7–34.8) 36.6 ms (34.0–36.6) 2.4 ms ⚪ within noise
GPU 0.1% worst 35.9 ms (35.1–37.2) 37.8 ms (35.8–37.8) 2.0 ms ⚪ within noise
Exceptions per run 0 0 0 ⚪ none new

⚠️ One Change run was excluded as untrustworthy: it logged 12 exceptions, while the other runs logged 0, 0.

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