security(star-rating): authorize the /o?method=star ratings read (24.05) - #7956
Merged
shepardly[bot] merged 6 commits intoAug 27, 2026
Merged
Conversation
Backport of #7955 to release.24.05. The star-rating dashboard read performed no authorization. Authentication on /o is per method: core methods call validateUserForDataReadAPI themselves, and the default branch hands the validators to plugins as helpers without calling them, so a plugin that claims a request must authorize it. This branch claimed the request and never did, so it answered callers with no account, token or session, for any app_id. Wrap it in validateRead(params, FEATURE_NAME, ...), the same check the sibling reads in this file already apply, running before the period parameter is validated so an unauthorized caller cannot probe the endpoint. The only caller is the dashboard Ratings page, which sends the session credential, so it is unaffected. Reported through the security bug bounty programme (received 2026-08-18). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ar2rsawseen
commented
Aug 24, 2026
result[data[0]] === undefined is not a "have I seen this platform" test on a plain object.
The platform name comes off the public star-rating event, in the platform_version_rate
segmentation, so anyone who can write to the app chooses it, and "__proto__",
"constructor", "toString", "valueOf" and "hasOwnProperty" all read back as inherited
members rather than as undefined. The array is then never created and the next line throws
TypeError: result[data[0]].indexOf is not a function
so one planted row denies the whole read for every authorized caller until it ages out.
Verified by running the loop, not by reading it.
Built with Object.create(null) instead. Nothing else changes: the keys are still ordinary
strings, and JSON.stringify serialises a null prototype object identically, which is what
returnOutput does with it. The platform has a second accumulator in the granular branch
and it gets the same treatment.
Tests lift both the accumulator's declaration and the loop out of the real source, so the
choice of object is what is under test rather than one the test made for itself. Four of
the five fail against the previous code, with the TypeError above.
The changelog is generated from PR and commit titles later, so an entry written by hand here is duplicated work at best. It is also the single worst file in this wave for conflicts: every merge to the base appends a line, which re-conflicts every open branch that also appends one. Eight of the sixteen conflicts across these security PRs today were this file and nothing else, and two of them came back within the hour. Only the lines this branch added are removed - the file is otherwise the base's, and the change here was purely additive, so nothing else moves.
Cookiezaurs
approved these changes
Aug 27, 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
2 participants
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 #7955 to
release.24.05./o?method=starreturns the platform and application-version combinations that have received star ratings, and performed no authorization — any caller supplying anapp_idreceived that application's data with noapi_key, noauth_tokenand no session, while the sibling/o/feedback/datacorrectly answers400 Missing parameter "api_key" or "auth_token"on the identical credential-less request.Authentication on
/ois per method, not global: core methods callvalidateUserForDataReadAPIthemselves, and thedefault:branch hands the validators to plugins as helpers without calling them, so a plugin that claims a request must authorize it. This branch claimed the request, returnedtrue, and never called a validator.Fix (identical to #7955): wrap the branch in
validateRead(params, FEATURE_NAME, …), the same check the sibling reads in this file already apply, running before theperiodparameter is validated so an unauthorized caller cannot probe the endpoint through its error responses. The diff is mostly re-indentation from introducing the callback.The only caller is the dashboard Ratings page (
starRatingPlugin.requestPlatformVersion), which sends the session credential and an app the member has access to, so it is unaffected.eslintclean.🤖 Generated with Claude Code