security(dashboards): scope copied widgets to the copier's app access (24.05) - #7857
Open
ar2rsawseen wants to merge 1 commit into
Open
security(dashboards): scope copied widgets to the copier's app access (24.05)#7857ar2rsawseen wants to merge 1 commit into
ar2rsawseen wants to merge 1 commit into
Conversation
Backport of the master fix. This branch never received the January 2026 pentest fix (c27fd19), so it had no widget app filtering at all - dashboards/api.js imported only validateUser. On this branch the issue is therefore reachable without any sharing or copying: any authenticated member could create their own dashboard, add a widget with apps set to an arbitrary app id, and read that app's analytics through /o/dashboard/data. Dashboard permissions are intentionally separate from app permissions, so a dashboard may be shared with a user who cannot read the apps its widgets point at. Copying such a dashboard, however, made the copier the OWNER of the copied widgets, and owners have edit rights, letting a view-only recipient rewrite a borrowed widget's query and read more of that app than the dashboard owner chose to show them. Changes, matching master: - Copying a dashboard skips any widget referencing an app the copier cannot read. Widgets with no apps, such as notes, still copy. - add-widget validates the widget's app ids against the creator's access and rejects unauthorized ones instead of storing them. - Widget app lists are normalized to a real array of 24 character hex ids, so an array-like value such as {length: 1, "0": "<app id>"} cannot be stored and then read back through .length/[i] by fetchWidgetApps and the /dashboard/data handlers. - update-widget keeps the apps a widget already references, since the dashboard owner delegated that by granting edit access, but checks app ids being newly added against the editor's own access. Regression tests cover the reported copy chain, the array-like injection, adding a widget for an unreadable app, and that copying still works for a user who can read the apps. Co-Authored-By: Claude <noreply@anthropic.com>
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.
Backport of the master fix in #7858 to
release.24.05.Not a clean cherry-pick: this branch never received the January 2026 pentest fix (
c27fd19e2d0), soplugins/dashboards/api/api.jsimported onlyvalidateUserand had no widget app filtering at all. The guard is therefore introduced here rather than replaced, which makes this branch the more exposed of the two targets.What changes
Dashboard permissions are intentionally separate from app permissions, so a dashboard may be shared with a user who cannot read the apps its widgets reference. Copying such a dashboard, however, made the copier the owner of the copied widgets, and owners have edit rights — so a view-only recipient could rewrite a borrowed widget's query and read more of that app than the dashboard owner chose to show them.
add-widgetvalidates the widget's app ids against the creator's access and rejects unauthorized ones rather than silently stripping them.Arraycannot be stored and then read back through.length/[i]byfetchWidgetAppsand the/dashboard/datahandlers.update-widgetkeeps the apps a widget already references — the dashboard owner delegated that by granting edit access — but checks app ids being newly added against the editor's own access.Tests
plugins/dashboards/tests.jsgains four cases, identical to the master branch: the copy chain, the array-like injection,add-widgetfor an unreadable app, and a positive case confirming copy still works for a user who can read the apps.Verified locally:
npx eslint plugins/dashboards/andnode --checkpass. The plugin suite itself runs in CI (test-api-plugins).Note for reviewers
The CHANGELOG entry went under the current
24.05.51heading rather than being appended to the24.05.50"backport of #7535" Security Fixes block, since this is a separate fix. Happy to consolidate if you prefer.🤖 Generated with Claude Code