[security][core] escape values at dashboard html sinks the api escaper does not cover - #7954
Closed
ar2rsawseen wants to merge 2 commits into
Closed
[security][core] escape values at dashboard html sinks the api escaper does not cover#7954ar2rsawseen wants to merge 2 commits into
ar2rsawseen wants to merge 2 commits into
Conversation
…istory actions column The export/purge history datatable builds an html string in onReady and the template renders it, so every value interpolated into it has to be html-safe before it gets there. Values taken from the row arrive through common.returnOutput, which escape_html_entities has already escaped, so they are deliberately left alone: escaping them a second time would surface the entities literally in the ui. One value in that string comes from countlyGlobal instead. That object is serialized into the dashboard by express-expose, whose escaping is for the javascript string context and is value-preserving by design, so the api's html escaping never applied to it. It is now escaped where it is interpolated. Adds test/unit-tests/plugins.compliance-hub.actions-escaping.js, which loads the real module in a sandbox and exercises the actual onReady builder. It pins both directions: a value carrying markup is neutralized, and an already-escaped api value is not double-escaped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ad of html
The populator template and environment delete confirmations rendered their
body with v-html, but the value bound there is a localized sentence with a name
substituted into a {0} placeholder. Both names are html-decoded on the way in,
so the escaping the api applied no longer held by the time they were rendered.
Text interpolation removes the sink instead of filtering what reaches it.
Checked every assignment to this dialog object and every populator locale file
that supplies these strings, in all 26 languages present: none contain tags, so
nothing renders differently.
Left the plugins plugin's dependency confirmation on v-html deliberately. Its
strings do carry markup: plugins.confirm has a <br/><br/> in all 24 translated
locale files, even though the default locale no longer has it. The values
interpolated into that one are plugin titles from package metadata rather than
anything a dashboard account can write.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ar2rsawseen
force-pushed
the
security/compliance-hub-actions-app-name-escape
branch
from
August 19, 2026 06:53
24688c4 to
9d71e4e
Compare
This was referenced Aug 19, 2026
Closed
Member
Author
|
Superseded by #7970, which consolidates the frontend HTML-sink hardening into a single per-repo PR. The same change (same authorship) is included there. Closing this one. |
ar2rsawseen
deleted the
security/compliance-hub-actions-app-name-escape
branch
August 19, 2026 14:17
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
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.
Found while looking at how the dashboard's HTML sinks are fed, alongside #7949.
Countly's model is that API output is HTML-escaped server side by
common.escape_html_entities, so a value that came from an API response is already safe by the time a template renders it. This PR covers the two situations where that guarantee does not hold, and were the only ones the sweep found still open.1. A value the API escaper does not cover
The compliance-hub export/purge history datatable builds an HTML string in
onReadyand the template renders it. Most of what goes into that string comes from the row, i.e. throughcommon.returnOutput, and is already escaped: those are deliberately left alone, because escaping them a second time would surface the entities literally in the UI.One value in that string comes from
countlyGlobalinstead. That object is serialized into the dashboard byexpress-expose, and that serializer escapes for the JavaScript string context, not the HTML one. It is value-preserving by design (see #7949), so the API's HTML escaping was never applied to this value at any point. It is now escaped where it is interpolated.2. Dialog bodies that never needed HTML
The populator template and environment delete confirmations rendered their body with
v-html, where the bound value is a localized sentence with a name substituted into a{0}placeholder. Both names are HTML-decoded on the way in, so the escaping the API applied no longer held at render time.Rather than filter what reaches those, they now use text interpolation, which removes the sink. I checked every assignment to this dialog object and every populator locale file that supplies these strings, in all 26 languages present: none contain tags, so nothing renders differently.
The plugins plugin's dependency confirmation is deliberately left on
v-html. Its strings genuinely carry markup —plugins.confirmhas a<br/><br/>in all 24 translated locale files, even though the default locale no longer does — so converting it would render those literally for every non-English user. The values interpolated into that one are plugin titles from package metadata rather than anything a dashboard account can write, so there is nothing to neutralize. Worth knowing for anyone auditing the same pattern: identical-looking sinks here have opposite correct answers, and the default.propertiesfile is not a reliable guide to what the translations contain.Scope
The sweep covered
v-htmlbindings, jQuery.html(<var>),innerHTML,insertAdjacentHTML, and everyunescapeHtmlcall site, in this repo and in the enterprise plugins, cross-referencing each sink against whether its source is API-escaped or not.countly.models.jsactions columnvue/components/vis.jschart tooltips<textarea v-html>in hooks effectsformatTimeAgo/ i18n-only bindingsv-htmlbindingscountly.template.jsandappIdsToNamesconsumersCounts, so the sweep's completeness is checkable: 62
v-htmlbindings here and 60 in the enterprise plugins, 57.html()hits of which 12 are application code, 45innerHTML-family hits of which about 20 are application code, and 137unescapeHtmlcall sites classified by destination.Worth recording for future readers: a detached-element
.html()is still a live sink, since image loading is not gated on document insertion. The ones above are inert because of their inputs, not because they are detached.Verification
New file
test/unit-tests/plugins.compliance-hub.actions-escaping.jsloads the real module in a sandbox and exercises the actualonReadybuilder, so it tests the shipped code path rather than a copy. It pins both directions, because the failure mode here is symmetric: a value carrying markup must be neutralized, and an already-escaped API value must not be double-escaped.Countly Request, network dependent). Baseline established by re-running with the new file excluded, not assumed.Propagation
Handled in their own PRs rather than by widening this one:
release.24.05: [security][core] escape values at dashboard html sinks the api escaper does not cover (24.05) #7965main: Countly/countly-platform#1121Platform is a straight port, since those frontend files are unchanged from this repo.
release.24.05is not: there the environment delete string isAre you sure you want to delete <b>{0}</b> environment?, so that dialog keepsv-htmland escapes the name at the assignment instead of converting the template. Same class of fix, different mechanism, chosen by what each branch's locale files actually contain.🤖 Generated with Claude Code