fix(apl): escape validation text before it becomes tooltip HTML - #1536
Merged
Merged
Conversation
`ui/index.ts` sets `allowHTML: true` as a global tippy default, and
`makeListItemValidations` builds its tooltip body by interpolating each
validation message into `<li>${v}</li>`. Those messages are not authored
by us: the sim formats user-supplied names straight into them, e.g.
`the APL group-reference and variable validations` — "Group reference '%s' not found" —
and `apl_values_operators.go:668,679` for variable names.
Rotations are shared by link and imported as JSON, so a rotation whose
group is named `<img src=x onerror=...>` runs script in the reader's
browser as soon as the warning tooltip renders.
Escaping after `ActionId.replaceAllInString` loses nothing: that call
substitutes spell *names* as plain text and injects no markup of its own,
so the only markup left in the string is the part we wrote.
Claude-Session: https://claude.ai/code/session_01Cup3PCdDi6MPDRPFDrZoTw
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
APL validation messages are rendered as HTML, and they are not all authored by us.
ui/index.tssetsallowHTML: trueas a global tippy default, andListPicker.makeListItemValidationsbuilds its tooltip body by interpolating eachvalidation straight into
<li>${v}</li>. The sim formats user-supplied names into thosemessages —
apl_action_group_reference.godoes it for a missing group name, andapl_values_operators.gofor variable names.Rotations are shared by link and imported as JSON, so a rotation carrying markup in a group
or variable name has that markup rendered in the reader's browser as soon as the warning
tooltip appears. The reader does not have to do anything except open the rotation and hover
a warning.
The fix
Escape the message before it is interpolated.
This is safe to do late, after
ActionId.replaceAllInString, because that call substitutesspell names as plain text — it injects no markup of its own. The only markup left in the
string is the
<p>/<ul>/<li>wrapper we write ourselves, which is built outside theescaped span.
One helper and one call site.
Notes
mop,cata,tbc-newand theWoWSimsCN-Mopfork. The sink is byte-identical in each, so the patch is the same.sod,tbc-oldandwotlkare not affected — they do not have this sink.allowHTML: trueis a global tippy default, which makesevery
tippy({ content })call site in the tree an HTML sink. This PR fixes the one that isreachable from user-controlled data; auditing the rest is worth doing separately.