From fc2f735a361f13c4627c2c011fbdcaf09d285291 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 20:37:42 +0300 Subject: [PATCH] [fix][star-rating] stop the by-id widget lookups returning internal fields /o/feedback/multiple-widgets-by-id and /o/feedback/widget look a widget up by _id and return the stored document as it is. The app-scoped /feedback/widgets, serving the same rendering purpose, projects to the fields a widget needs and then deletes cohortID with the comment "no need to return more data than needed". The two by-id lookups did neither, so an anonymous caller holding a widget id also received targeting, the audience segmentation query, and cohortID. Both now exclude those two fields. Excluded rather than allow-listed on purpose. These endpoints render every widget type, so an allow-list drawn from the rating-only projection would drop what surveys and nps need, and the caller is an sdk already deployed in the field. Naming the two internal fields cannot break rendering. Left alone deliberately: the endpoints stay anonymous. They exist to be called by the web sdk with no session and no app_id to scope by, so requiring authentication or an app_key would stop widgets rendering. The nfd counter ping is also unchanged, for the same reason as the earlier decision about timesShown. Related: the write routes of this plugin were bound to their app in #7622. This is the read half of the same shape. Co-Authored-By: Claude Opus 5 --- plugins/star-rating/api/api.js | 17 +++++++- ...plugins.star-rating.widget-by-id-fields.js | 39 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) create mode 100644 test/unit-tests/plugins.star-rating.widget-by-id-fields.js diff --git a/plugins/star-rating/api/api.js b/plugins/star-rating/api/api.js index c3626c52fd4..a75c3e6884e 100644 --- a/plugins/star-rating/api/api.js +++ b/plugins/star-rating/api/api.js @@ -1330,6 +1330,19 @@ function uploadFile(myfile, id, callback) { * @apiDescription: Get feedback widgets with or without filters * @apiParam: 'app_key', app_key of related application provided by sdk request */ + //These two lookups serve the sdk, so they answer without a session and without an + //app_id to scope by, and they have to keep doing that or widgets stop rendering. + //What they must not do is hand out the fields the app-scoped /feedback/widgets + //deliberately withholds: targeting, which is the audience segmentation query, and + //cohortID, which that handler fetches only to test membership and then deletes with + //the comment "no need to return more data than needed". + // + //Excluded rather than allow-listed on purpose. These endpoints render every widget + //type, so an allow-list drawn from the rating-only projection above would drop the + //fields surveys and nps need, and the caller is an sdk in the field that cannot be + //redeployed. Naming the internal fields cannot break rendering. + const WIDGET_INTERNAL_FIELDS = {targeting: 0, cohortID: 0}; + plugins.register('/o/feedback/multiple-widgets-by-id', function(ob) { var params = ob.params; var collectionName = 'feedback_widgets'; @@ -1348,7 +1361,7 @@ function uploadFile(myfile, id, callback) { _id: { $in: widgetIdsArray } - }).toArray(function(err, docs) { + }, {projection: WIDGET_INTERNAL_FIELDS}).toArray(function(err, docs) { if (!err) { if (docs.length) { common.returnOutput(params, docs); @@ -1510,7 +1523,7 @@ function uploadFile(myfile, id, callback) { common.db.collection(collectionName).findOne({ "_id": widgetId - }, function(err, doc) { + }, {projection: WIDGET_INTERNAL_FIELDS}, function(err, doc) { if (err) { common.returnMessage(params, 500, err.message); } diff --git a/test/unit-tests/plugins.star-rating.widget-by-id-fields.js b/test/unit-tests/plugins.star-rating.widget-by-id-fields.js new file mode 100644 index 00000000000..fc9be367c0e --- /dev/null +++ b/test/unit-tests/plugins.star-rating.widget-by-id-fields.js @@ -0,0 +1,39 @@ +require("should"); +var fs = require("fs"); + +// /o/feedback/multiple-widgets-by-id and /o/feedback/widget answer the sdk without a +// session and without an app_id to scope by, so they cannot be gated without breaking +// widget rendering. What they can do is stop returning the fields the app-scoped +// /feedback/widgets deliberately withholds: targeting, the audience segmentation +// query, and cohortID, which that handler deletes with "no need to return more data +// than needed". +// +// Asserted against the source because both handlers are registered on a plugin bus at +// load time and their bodies are not reachable as functions from a unit test. + +describe("feedback widget by-id lookups", function() { + var src = fs.readFileSync(__dirname + "/../../plugins/star-rating/api/api.js", "utf8"); + + it("declares the internal fields as excluded", function() { + src.should.match(/const WIDGET_INTERNAL_FIELDS = \{targeting: 0, cohortID: 0\}/); + }); + + it("applies the exclusion to the batch lookup", function() { + var batch = src.slice(src.indexOf("'/o/feedback/multiple-widgets-by-id'")); + batch = batch.slice(0, batch.indexOf("plugins.register", 10)); + batch.should.match(/\$in: widgetIdsArray[\s\S]{0,80}projection: WIDGET_INTERNAL_FIELDS/); + }); + + it("applies the exclusion to the single lookup", function() { + var single = src.slice(src.indexOf("'/o/feedback/widget'")); + single = single.slice(0, single.indexOf("plugins.register", 10)); + single.should.match(/"_id": widgetId[\s\S]{0,60}projection: WIDGET_INTERNAL_FIELDS/); + }); + + it("excludes rather than allow-lists, so widget types beyond rating still render", function() { + // an inclusion projection here would drop the fields surveys and nps need, and + // the caller is an sdk in the field that cannot be redeployed + var decl = src.match(/const WIDGET_INTERNAL_FIELDS = \{[^}]*\}/)[0]; + decl.should.not.match(/: 1/); + }); +});