[fix][core] keep server side rendering on the dashboard origin - #7926
Merged
Conversation
api/utils/render.js builds the url it opens by concatenating the configured dashboard host with the requested view, and /o/render lets the caller choose that view. With the default countlyConfig.path of "" the prefix is exactly "http://localhost", so a view that does not begin with "/" changes the host rather than the path, and the headless browser opens that instead of the dashboard. Parse the concatenated url and require its origin to match the dashboard's before navigating. What gets navigated to is the concatenation itself, unchanged, so a configured countlyConfig.path keeps behaving exactly as it did. Parsing settles the whole family of shapes at once, including the ones the url parser normalises away, and it agrees with what Chromium does with the same string since both use the WHATWG parser. Also restrict the renderer to the dashboard origin with request interception, the same control api/utils/pdf.js already uses. The dashboard serves all of its own assets, so nothing in a normal render is refused: measured against real Chromium, own origin images and stylesheets still load and the screenshot is still produced. A private range denylist would be the wrong control here. The intended render target is loopback, which api/utils/ssrf-protection.js blocks by design.
ar2rsawseen
commented
Aug 24, 2026
…y uses, not only its own
The subresource control was right in shape and too tight in practice. Both cases in
the review note check out against the tree:
frontend/express/views/dashboard.html prefixes every core stylesheet and script
with countlyConfig.cdn, so a deployment that sets it loses its bootstrap assets
and the render times out waiting for the dashboard to initialize.
frontend/express/public/javascripts/countly/vue/components/vis.js requests
https://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png and passes no subdomains
option, so leaflet expands {s} to a, b and c. Map widgets render blank.
So the check is now against a small allowlist instead of a single origin: the
dashboard, the configured cdn, the tile provider, and whatever the operator adds in
countlyConfig.render.allowedOrigins. Entries may be a bare origin or any url on one.
Everything else is still aborted, and the navigation check on the view is untouched -
that is the control that stops the renderer reaching what the server can reach, and
allowing an asset origin does not weaken it.
The cdn lives in the dashboard's config rather than the api's. In the standard layout
both are in the same tree and it is picked up with no operator action; where the api
runs without the dashboard the file is absent, the require fails closed, and the
origin is named in the config array instead.
The tile origins are listed as four exact strings rather than matched by host suffix,
so the allowlist stays a set of strings to compare with no pattern to get wrong. A
look-alike host such as tile.openstreetmap.org.evil.example is a different origin and
stays out; there is a test for it.
An unparseable dashboard host now yields an empty allowlist, which refuses every
request rather than degrading into "allow anything".
Also corrected a comment that claimed this mirrors api/utils/pdf.js. It does not -
pdf.js renders supplied html and intercepts nothing.
Ten new tests, and the platform copy of the suite that was missing there entirely.
Cookiezaurs
approved these changes
Aug 25, 2026
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.
What
api/utils/render.jsbuilds the url it opens by concatenating the configured dashboard host with the requested view:/o/rendertakes that view from the request (params.qstring.view). With the defaultcountlyConfig.pathof""the prefix is exactlyhttp://localhost, so a view that does not begin with/changes the host rather than the path:/#/dashboardhttp://localhost/#/dashboard, as intended@169.254.169.254/latest/...169.254.169.254:8500/v1/kv/?recurselocalhost:8500.internal.example/xlocalhost.internal.exampleThe headless browser then loads that from the server's own network position.
Change
countlyConfig.pathkeeps behaving exactly as it did. Parsing settles the whole family of shapes at once, including the ones the url parser normalises away (tab, newline, carriage return), and it agrees with what Chromium does with the same string since both use the WHATWG parser.api/utils/pdf.jsuses, so the page's subresources are bounded too and not just the navigation.A private range denylist would be the wrong control here: the intended render target is loopback, which
api/utils/ssrf-protection.jsblocks by design.Scope
Everything that drives a headless browser, with a verdict for each:
api/utils/render.js,page.goto(host + view)api/utils/render.js, login token navigationapi/utils/pdf.jsapi/utils/requestProcessor.js, the/o/rendercallerparams.qstring.viewthrough, now checked at the sinkplugins/dashboards/api/api.jscaller"/dashboard?ssr=true#/custom/" + id, unaffected and covered by the tests belowrender.jsand norenderViewcallerNeither caller sets
options.host, so the base always stays the configured dashboard.Verification
test/unit-tests/api.utils.render.js, 38 cases: every host rewriting shape is refused for each host form, and every view the product itself renders is allowed and returned byte identical. Swapping the check back for plain concatenation fails 13 of them, so the tests do pin the behaviour.renderViewandrenderPDFintegration suite (test/2.api/99.api.utils.render.js) passes against the patched file, 7 of 7, including both real screenshot cases.