From c3e78e7653461203bd860262b94450f13dcea75e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 09:56:43 +0300 Subject: [PATCH 1/2] [security][compliance-hub] escape interpolated values in the export history actions column Backport of the master change. 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 --- .../public/javascripts/countly.models.js | 8 +- ...plugins.compliance-hub.actions-escaping.js | 132 ++++++++++++++++++ 2 files changed, 139 insertions(+), 1 deletion(-) create mode 100644 test/unit-tests/plugins.compliance-hub.actions-escaping.js diff --git a/plugins/compliance-hub/frontend/public/javascripts/countly.models.js b/plugins/compliance-hub/frontend/public/javascripts/countly.models.js index bd758822308..c4e6f858295 100644 --- a/plugins/compliance-hub/frontend/public/javascripts/countly.models.js +++ b/plugins/compliance-hub/frontend/public/javascripts/countly.models.js @@ -194,7 +194,13 @@ var ret = "

" + ((jQuery.i18n.map["systemlogs.action." + row.a]) ? jQuery.i18n.map["systemlogs.action." + row.a] : row.a) + "

"; if (typeof row.i === "object") { if (typeof row.i.app_id !== "undefined" && countlyGlobal.apps[row.i.app_id]) { - ret += "

" + jQuery.i18n.map["systemlogs.for-app"] + ": " + countlyGlobal.apps[row.i.app_id].name + "

"; + //this string is rendered with v-html, so every interpolated value must already be + //HTML-safe. Everything taken from "row" arrives through common.returnOutput, which + //escape_html_entities has already escaped, so it must NOT be escaped again here or + //the entities would show up literally. The app name is the exception: it comes from + //countlyGlobal, which is serialized into the dashboard's script island by + //express-expose and is never HTML-escaped, so it reaches us raw and is escaped here. + ret += "

" + jQuery.i18n.map["systemlogs.for-app"] + ": " + countlyCommon.encodeHtml(countlyGlobal.apps[row.i.app_id].name) + "

"; } if (typeof row.i.appuser_id !== "undefined") { ret += "

" + jQuery.i18n.map["systemlogs.for-appuser"] + ": " + row.i.appuser_id + "

"; diff --git a/test/unit-tests/plugins.compliance-hub.actions-escaping.js b/test/unit-tests/plugins.compliance-hub.actions-escaping.js new file mode 100644 index 00000000000..2c512055487 --- /dev/null +++ b/test/unit-tests/plugins.compliance-hub.actions-escaping.js @@ -0,0 +1,132 @@ +var should = require("should"); +var fs = require("fs"); +var path = require("path"); +var vm = require("vm"); + +// The export/purge history datatable builds an HTML string in onReady and the template renders it +// with v-html (plugins/compliance-hub/frontend/public/templates/exportHistory.html). Everything +// interpolated into that string therefore has to be HTML-safe already. +// +// Two opposite mistakes are possible and this file guards both directions: +// +// 1. NOT escaping the app name. It is read from countlyGlobal, which express-expose serializes +// into the dashboard's inline script island. That serializer escapes for the JavaScript string +// context only and is deliberately value-preserving, so the value arrives raw. An app admin can +// set their own app's name, and a global admin's dashboard lists every app, so an unescaped name +// is a stored cross-user XSS. +// 2. Escaping the values that came from the API. Those already went through +// common.escape_html_entities in common.returnOutput, so escaping them again would render the +// entities literally in the UI. +var SRC = path.resolve(__dirname, "../../plugins/compliance-hub/frontend/public/javascripts/countly.models.js"); + +/** + * Load countly.models.js in a sandbox and hand back the onReady callbacks it registers, + * keyed by data-table name. + * @param {object} apps - the countlyGlobal.apps map the module should see + * @returns {object} map of resource name to its onReady function + */ +function loadResources(apps) { + var resources = {}; + var noop = function() {}; + // matches countlyCommon.encodeHtml, which is `div.innerText = x; return div.innerHTML`: + // the text-node serializer escapes &, < and > and leaves quotes alone. + var encodeHtml = function(html) { + return (html + "").replace(/&/g, "&").replace(//g, ">"); + }; + var sandbox = { + window: {}, + countlyCommon: { + encodeHtml: encodeHtml, + formatTimeAgoText: function() { + return { text: "just now" }; + }, + getDescendantProp: noop, + API_PARTS: { data: { r: "/o" } }, + ACTIVE_APP_ID: "5f1a2b3c4d5e6f0011223344", + periodObj: {}, + getPeriodForAjax: function() { + return "30days"; + } + }, + CountlyHelpers: { createMetricModel: noop }, + jQuery: { i18n: { map: { "systemlogs.for-app": "For app", "systemlogs.for-appuser": "For app user", "systemlogs.action.export": "Data exported" } } }, + CV: { + i18n: function(k) { + return k; + } + }, + countlyGlobal: { apps: apps }, + countlyTaskManager: {}, + countlyVue: { + vuex: { + ServerDataTable: function(name, cfg) { + resources[name] = cfg.onReady; + return { name: name }; + }, + Module: function() { + return {}; + }, + MutationsFor: noop, + ActionsFor: noop + } + } + }; + sandbox.global = sandbox; + vm.createContext(sandbox); + vm.runInContext(fs.readFileSync(SRC, "utf8"), sandbox, { filename: SRC }); + return resources; +} + +describe("compliance-hub export history actions escaping", function() { + var APP_ID = "5f1a2b3c4d5e6f0011223344"; + + /** + * Run the export-history onReady over a single row. + * @param {string} appName - the app name countlyGlobal should carry + * @param {object} i - the row's "i" payload + * @returns {string} the built actions HTML + */ + function actionsFor(appName, i) { + var apps = {}; + apps[APP_ID] = { name: appName }; + var onReady = loadResources(apps).exportHistoryDataResource; + should.exist(onReady); + var rows = onReady({}, [{ a: "export", ts: 0, i: i || { app_id: APP_ID } }]); + return rows[0].actions; + } + + it("escapes an app name that carries a tag", function(done) { + var actions = actionsFor(''); + actions.indexOf(""); + actions.indexOf("").should.equal(-1); + done(); + }); + + it("leaves no raw angle bracket from the app name", function(done) { + var actions = actionsFor(""); + // the only markup left must be the

wrappers this builder emits itself + actions.replace(/<\/?p[^>]*>/g, "").indexOf("<").should.equal(-1); + done(); + }); + + it("keeps an ordinary app name readable", function(done) { + var actions = actionsFor("My Application"); + actions.should.containEql("For app: My Application"); + done(); + }); + + it("does not double-escape values the API already escaped", function(done) { + // returnOutput turns ' into '; escaping again would surface "&#39;" in the UI + var actions = actionsFor("My Application", { app_id: APP_ID, appuser_id: "user's-id" }); + actions.should.containEql("user's-id"); + actions.indexOf("&#39;").should.equal(-1); + done(); + }); +}); From 926502a15911e39fb3e8ae3231c1ba72e0cef7dc Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 19 Aug 2026 09:56:44 +0300 Subject: [PATCH 2/2] [security][populator] stop rendering decoded names as html in the delete confirmations Backport of the master change, adjusted for this branch: the two populator delete confirmations need different fixes here, because their localized strings differ from master's. Both dialogs substitute a name that was html-decoded on the way in, so the escaping the api applied no longer held by the time it was rendered. - the template delete confirmation carries no markup in any of the 26 locale files, so its body is now text interpolation, which removes the sink - the environment delete confirmation cannot do that on this branch. Its string is "Are you sure you want to delete {0} environment?", so the body has to stay v-html and the name is escaped where it is substituted instead. On master the same string has no markup, which is why that branch converts the template and this one does not. Left the plugins plugin's dependency confirmation on v-html, as on master: plugins.confirm carries a

in all translated locale files, and the values interpolated into it are plugin titles from package metadata rather than anything a dashboard account can write. Co-Authored-By: Claude Opus 4.8 --- .../populator/frontend/public/javascripts/countly.views.js | 6 +++++- plugins/populator/frontend/public/templates/populator.html | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/plugins/populator/frontend/public/javascripts/countly.views.js b/plugins/populator/frontend/public/javascripts/countly.views.js index 8cea6458ca0..ec2b1655c56 100644 --- a/plugins/populator/frontend/public/javascripts/countly.views.js +++ b/plugins/populator/frontend/public/javascripts/countly.views.js @@ -552,7 +552,11 @@ saveButtonLabel: CV.i18n('common.yes'), cancelButtonLabel: CV.i18n('common.cancel'), title: CV.i18n('populator.environment-delete-warning-title'), - text: CV.i18n('populator.environment-delete-warning-description', this.filterByEnvironmentOptions.filter(x => x.value === this.environmentId)[0].label) + //this sentence is rendered with v-html because the localized string itself carries + //markup, so the environment name substituted into it has to be escaped here. The name + //was html-decoded when the dropdown options were built, which is what the dropdown + //needs, so the escaping the api applied no longer holds by this point. + text: CV.i18n('populator.environment-delete-warning-description', countlyCommon.encodeHtml(this.filterByEnvironmentOptions.filter(x => x.value === this.environmentId)[0].label)) }; }, calculateWidth: function(percentage) { diff --git a/plugins/populator/frontend/public/templates/populator.html b/plugins/populator/frontend/public/templates/populator.html index fcea9db058e..88aab7e6e71 100644 --- a/plugins/populator/frontend/public/templates/populator.html +++ b/plugins/populator/frontend/public/templates/populator.html @@ -170,7 +170,8 @@