From ca6f3a99c9e03c21ebe84d43df9945e4134b7800 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 1 Aug 2026 20:06:17 +0300 Subject: [PATCH 1/5] security(rights): require app admin access to be granted, not merely unsaid hasAdminAccess started from `true` and only cleared when an entry for the app existed with `all` falsy. An app the member had no entry for never cleared it, so every unlisted app came back as one they administer. Master already starts from false and requires every operation to carry `all`, so this branch had been diverging on the meaning of app admin access. Everything reached through this helper inherited the fail-open. hasCreateRight, hasReadRight, hasUpdateRight and hasDeleteRight all fall through to it, so they passed for apps nobody had granted. Proven on this branch before the change: for a member holding only feature-scoped alerts rights on appA, hasUpdateRight("alerts", "appB") and hasReadRight("alerts", "appB") both returned true. After it they are false, while appA stays true. The practical effect was that the cross-app authorization work on this branch was inert. The hooks fix merged in #7863 calls these helpers at four sites, and the alerts create guard calls hasCreateRight; all of them were being answered "yes" for apps the caller held nothing on. This repairs those in place, and is a prerequisite for the alerts stored-app backport. Note this does not widen anything. Access that was explicitly granted is unaffected: an app listed in permission._.a returns true from the early return, untouched here, and an app carrying all:true across c/r/u/d still returns true. What stops passing is admin access the permission document never expressed, either no entry at all or a partial grant that does not cover all four operations. permission._.a is what the UI writes when somebody is made an app admin, so the grant path users actually go through is unchanged. Verified against the full unit suite: 101 passing / 5 failing before, 112 passing / the same 5 failing after. Those 5 (validateArgs extra types, countly request) are pre-existing and unrelated. Co-Authored-By: Claude Opus 5 --- api/utils/rights.js | 21 +++- .../api.utils.rights.hasAdminAccess.js | 95 +++++++++++++++++++ 2 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 test/unit-tests/api.utils.rights.hasAdminAccess.js diff --git a/api/utils/rights.js b/api/utils/rights.js index 6cb3024ff6e..e782c47e549 100644 --- a/api/utils/rights.js +++ b/api/utils/rights.js @@ -1130,15 +1130,30 @@ exports.hasAdminAccess = function(member, app_id, type) { return true; } - var isAdmin = true; + //Admin access has to be granted, not merely left unsaid. This used to start true + //and only clear when an entry for the app existed with `all` falsy, so an app the + //member had no entry for never cleared it and every such app came back as one they + //administer. Any check reached through here (hasCreateRight, hasReadRight, + //hasUpdateRight, hasDeleteRight) then passed for apps nobody had granted. + var isAdmin = false; // check users who has permission property if (hasPermissionObject) { var types = type ? [type] : ["c", "r", "u", "d"]; + var passesAllRules = true; for (var i = 0; i < types.length; i++) { - if (member.permission[types[i]] && member.permission[types[i]][app_id] && !member.permission[types[i]][app_id].all) { - isAdmin = false; + if (member.permission[types[i]] && member.permission[types[i]][app_id]) { + if (!member.permission[types[i]][app_id].all) { + passesAllRules = false; + } + } + else { + passesAllRules = false; } } + if (passesAllRules) { + isAdmin = true; + } + } // check legacy users who has admin_of property // users should have at least one app in admin_of array diff --git a/test/unit-tests/api.utils.rights.hasAdminAccess.js b/test/unit-tests/api.utils.rights.hasAdminAccess.js new file mode 100644 index 00000000000..d897b84b366 --- /dev/null +++ b/test/unit-tests/api.utils.rights.hasAdminAccess.js @@ -0,0 +1,95 @@ +require("should"); +var rights = require("../../api/utils/rights.js"); + +// hasAdminAccess used to start from "true" and only clear when an entry for the app +// existed with `all` falsy. An app the member had no entry for never cleared it, so +// every unlisted app came back as one they administer, and every right that falls +// through to it (create/read/update/delete) passed for apps nobody had granted. +// +// The two halves matter equally. Access that was actually granted has to survive, or +// members lose apps they legitimately hold; access that was never expressed has to be +// refused, which is the bug itself. +describe("rights.hasAdminAccess", function() { + describe("access that was explicitly granted, which must survive", function() { + it("allows an app listed in the app-admin list", function() { + // this is what the UI writes when somebody is made an app admin + var member = {permission: {_: {a: ["appA"]}, c: {}, r: {}, u: {}, d: {}}}; + rights.hasAdminAccess(member, "appA").should.equal(true); + }); + it("allows an app carrying all:true across all four operations", function() { + var member = { + permission: { + _: {a: []}, + c: {appA: {all: true}}, + r: {appA: {all: true}}, + u: {appA: {all: true}}, + d: {appA: {all: true}} + } + }; + rights.hasAdminAccess(member, "appA").should.equal(true); + }); + it("allows a legacy member their admin_of app", function() { + rights.hasAdminAccess({admin_of: ["appA"], user_of: ["appA"]}, "appA").should.equal(true); + }); + it("allows a global admin any app", function() { + rights.hasAdminAccess({global_admin: true}, "whatever").should.equal(true); + }); + }); + + describe("access that was never granted, which must be refused", function() { + it("refuses an app the member has no entry for", function() { + // the bug: this returned true, making every unlisted app an admin app + var member = {permission: {_: {a: []}, c: {}, r: {}, u: {}, d: {}}}; + Boolean(rights.hasAdminAccess(member, "appB")).should.equal(false); + }); + it("refuses an app granted for only some of the four operations", function() { + // read-all on an app is not the same as administering it + var member = {permission: {_: {a: []}, c: {}, r: {appA: {all: true}}, u: {}, d: {}}}; + Boolean(rights.hasAdminAccess(member, "appA")).should.equal(false); + }); + it("refuses an app whose grants are feature-scoped rather than all", function() { + var member = { + permission: { + _: {a: []}, + c: {appA: {all: false, allowed: {alerts: true}}}, + r: {appA: {all: false, allowed: {alerts: true}}}, + u: {appA: {all: false, allowed: {alerts: true}}}, + d: {appA: {all: false, allowed: {alerts: true}}} + } + }; + Boolean(rights.hasAdminAccess(member, "appA")).should.equal(false); + }); + it("refuses a legacy member an app they are only user_of", function() { + Boolean(rights.hasAdminAccess({admin_of: [], user_of: ["appA"]}, "appA")).should.equal(false); + }); + }); + + // The reason this matters beyond hasAdminAccess itself: the per-feature right + // helpers fall through to it, so the fail-open silently granted every feature on + // every unlisted app. Those are what the cross-app authorization fixes are built + // on, and they were inert on this branch until this was corrected. + describe("the per-feature rights that fall through to it", function() { + var member = { + permission: { + _: {a: [], u: [["appA"]]}, + c: {appA: {all: false, allowed: {alerts: true}}}, + r: {appA: {all: false, allowed: {alerts: true}}}, + u: {appA: {all: false, allowed: {alerts: true}}}, + d: {} + } + }; + it("keeps the right on the app the member holds it for", function() { + Boolean(rights.hasUpdateRight("alerts", "appA", member)).should.equal(true); + Boolean(rights.hasReadRight("alerts", "appA", member)).should.equal(true); + Boolean(rights.hasCreateRight("alerts", "appA", member)).should.equal(true); + }); + it("refuses the right on an app the member holds nothing for", function() { + Boolean(rights.hasUpdateRight("alerts", "appB", member)).should.equal(false); + Boolean(rights.hasReadRight("alerts", "appB", member)).should.equal(false); + Boolean(rights.hasCreateRight("alerts", "appB", member)).should.equal(false); + }); + it("refuses a feature the member was not granted on an app they do hold", function() { + Boolean(rights.hasUpdateRight("dbviewer", "appA", member)).should.equal(false); + }); + }); +}); From 2d12e8f22f830cd19fd786cf6893811c5eaf5ced Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 14 Aug 2026 17:36:19 +0300 Subject: [PATCH 2/5] [fix][views] apply the token owner's read right on the heatmap endpoint The countly-token branch of /o/actions resolves the target app from the caller supplied app_key and then served the data once the token itself verified. It never resolved the token to the member who created it, so that member's own rights were never consulted. Everywhere else a token acts as its owner: verify_return hands back the owner, and the usual validation loads that member and applies their rights, while the token's app and endpoint fields only narrow things further. This branch skipped that step, which left the optional app restriction as the only thing bounding which app could be read. A token saved without an app restriction is not narrowed at all, which is correct in itself, so nothing remained to bound the read. Load the owner and require a views read right on the app resolved from app_key, the way validateRead does for the api_key branch below it. The heatmap feature has been out of the product for over two years and this endpoint stays only so that a long-standing integration does not break, so this keeps the existing flow working: the dashboard mints its token scoped to the active app, and its owner holds the read right for that app. --- plugins/views/api/api.js | 63 ++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 25 deletions(-) diff --git a/plugins/views/api/api.js b/plugins/views/api/api.js index 0e3b7863343..cc6099553ae 100644 --- a/plugins/views/api/api.js +++ b/plugins/views/api/api.js @@ -8,7 +8,7 @@ var pluginOb = {}, plugins = require('../../pluginManager.js'), fetch = require('../../../api/parts/data/fetch.js'), log = common.log('views:api'), - { validateRead, validateUpdate, validateDelete } = require('../../../api/utils/rights.js'); + { validateRead, validateUpdate, validateDelete, hasReadRight } = require('../../../api/utils/rights.js'); const viewsUtils = require("./parts/viewsUtils.js"); const FEATURE_NAME = 'views'; @@ -1546,31 +1546,44 @@ const escapedViewSegments = { "name": true, "segment": true, "height": true, "wi callback: function(owner, expires_after) { if (owner) { var token = params.req.headers["countly-token"]; - if (expires_after < 600 && expires_after > -1) { - authorize.extend_token({ - extendTill: Date.now() + 600000, //10 minutes - token: params.req.headers["countly-token"], - callback: function(/*err,res*/) { - params.token_headers = {"countly-token": token, "content-language": token, "Access-Control-Expose-Headers": "countly-token"}; - params.app_id = app._id; - params.app_cc = app.country; - params.appTimezone = app.timezone; - params.app = app; - params.time = common.initTimeObj(params.appTimezone, params.qstring.timestamp); - getHeatmap(params); - } - }); + //A token acts as its owner: everywhere else it is resolved to the + //member who created it and that member's own rights decide what the + //request may read, while the token's app and endpoint fields only narrow + //it further. This branch used to skip that step, so the app resolved from + //the caller's app_key was served without consulting the owner at all. + //Apply the owner's read right here, the way validateRead does for the + //api_key branch below. + common.db.collection('members').findOne({_id: common.db.ObjectID(owner + "")}, function(memberErr, member) { + if (memberErr || !member || !hasReadRight(FEATURE_NAME, app._id + "", member)) { + common.returnMessage(params, 401, 'User does not have view right for this application'); + return false; + } + if (expires_after < 600 && expires_after > -1) { + authorize.extend_token({ + extendTill: Date.now() + 600000, //10 minutes + token: params.req.headers["countly-token"], + callback: function(/*err,res*/) { + params.token_headers = {"countly-token": token, "content-language": token, "Access-Control-Expose-Headers": "countly-token"}; + params.app_id = app._id; + params.app_cc = app.country; + params.appTimezone = app.timezone; + params.app = app; + params.time = common.initTimeObj(params.appTimezone, params.qstring.timestamp); + getHeatmap(params); + } + }); - } - else { - params.token_headers = {"countly-token": token, "content-language": token, "Access-Control-Expose-Headers": "countly-token"}; - params.app_id = app._id; - params.app_cc = app.country; - params.appTimezone = app.timezone; - params.app = app; - params.time = common.initTimeObj(params.appTimezone, params.qstring.timestamp); - getHeatmap(params); - } + } + else { + params.token_headers = {"countly-token": token, "content-language": token, "Access-Control-Expose-Headers": "countly-token"}; + params.app_id = app._id; + params.app_cc = app.country; + params.appTimezone = app.timezone; + params.app = app; + params.time = common.initTimeObj(params.appTimezone, params.qstring.timestamp); + getHeatmap(params); + } + }); } else { common.returnMessage(params, 401, 'User does not have view right for this application'); From 07547cea53554f44171bff65fe26bdcb2a5abcc8 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 22:35:04 +0300 Subject: [PATCH 3/5] security(rights): return a real boolean from hasAdminAccess, and cover the delete right Answers both review notes. hasAdminAccess ended in `isAdmin || member.global_admin`, and global_admin is usually absent rather than false, so the function returned undefined for the common "not an admin" case. The JSDoc promises a boolean and a caller comparing strictly or serialising the result should not have to know the difference; the return is coerced now. The per-feature regression cases covered create, read and update falling through to it and left out delete, which uses the same `hasAdminAccess(..., "d")` path and is the most expensive of the four to grant by accident. The test member now carries a delete grant and the delete right is asserted alongside the others, on the app it holds, on one it does not, and for a feature it was not granted. There is also a case asserting the return is `false` rather than undefined, so the coercion cannot be dropped silently. 12 passing. --- api/utils/rights.js | 5 ++++- .../unit-tests/api.utils.rights.hasAdminAccess.js | 15 ++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/api/utils/rights.js b/api/utils/rights.js index e782c47e549..a655af6705a 100644 --- a/api/utils/rights.js +++ b/api/utils/rights.js @@ -1160,7 +1160,10 @@ exports.hasAdminAccess = function(member, app_id, type) { else { isAdmin = typeof member.admin_of !== "undefined" && member.admin_of.indexOf(app_id) > -1; } - return isAdmin || member.global_admin; + //coerced: global_admin is often simply absent, and `isAdmin || undefined` is + //undefined rather than false. The JSDoc promises a boolean, and a caller comparing + //strictly or serialising the result should not have to know the difference. + return !!(isAdmin || member.global_admin); }; exports.hasCreateRight = function(feature, app_id, member) { diff --git a/test/unit-tests/api.utils.rights.hasAdminAccess.js b/test/unit-tests/api.utils.rights.hasAdminAccess.js index d897b84b366..dc95a80828d 100644 --- a/test/unit-tests/api.utils.rights.hasAdminAccess.js +++ b/test/unit-tests/api.utils.rights.hasAdminAccess.js @@ -75,21 +75,34 @@ describe("rights.hasAdminAccess", function() { c: {appA: {all: false, allowed: {alerts: true}}}, r: {appA: {all: false, allowed: {alerts: true}}}, u: {appA: {all: false, allowed: {alerts: true}}}, - d: {} + d: {appA: {all: false, allowed: {alerts: true}}} } }; it("keeps the right on the app the member holds it for", function() { Boolean(rights.hasUpdateRight("alerts", "appA", member)).should.equal(true); Boolean(rights.hasReadRight("alerts", "appA", member)).should.equal(true); Boolean(rights.hasCreateRight("alerts", "appA", member)).should.equal(true); + Boolean(rights.hasDeleteRight("alerts", "appA", member)).should.equal(true); }); it("refuses the right on an app the member holds nothing for", function() { Boolean(rights.hasUpdateRight("alerts", "appB", member)).should.equal(false); Boolean(rights.hasReadRight("alerts", "appB", member)).should.equal(false); Boolean(rights.hasCreateRight("alerts", "appB", member)).should.equal(false); + //delete goes through the same hasAdminAccess(..., "d") fall-through, and a + //delete granted by a fail-open is the most expensive of the four + Boolean(rights.hasDeleteRight("alerts", "appB", member)).should.equal(false); }); it("refuses a feature the member was not granted on an app they do hold", function() { Boolean(rights.hasUpdateRight("dbviewer", "appA", member)).should.equal(false); + Boolean(rights.hasDeleteRight("dbviewer", "appA", member)).should.equal(false); + }); + + it("answers with a real boolean rather than undefined", function() { + // global_admin is usually absent rather than false, and `isAdmin || + // member.global_admin` then evaluated to undefined: the JSDoc promises a + // boolean and a caller comparing strictly would have been surprised + rights.hasAdminAccess(member, "appB", "d").should.equal(false); + rights.hasAdminAccess(member, "appB").should.equal(false); }); }); }); From 6708ad059837687ab723bdc1a7e821949fc276a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 14:07:07 +0300 Subject: [PATCH 4/5] [fix][security][views] keep the legacy read path open for a token owner hasReadRight covers feature permissions, an app admin, a legacy admin_of admin and a global admin. It does not cover the plain legacy membership validateRead still honours: a member stored before permission objects existed has no `permission` at all and is granted read when user_of carries the app (api/utils/rights.js). Refusing those here would take away access the same member's api_key still has, which is not what this fix is for. The check now goes through viewsUtils.ownerCanRead, which applies hasReadRight for a member with a permission object and the user_of fallback for one without. Once a permission object exists, user_of is not consulted - that mirrors validateRead, which takes the legacy branch only when permission is undefined. While mirroring validateRead: it also refuses a locked account, and this branch did not. A locked member's token kept working. ownerCanRead refuses it. Put in viewsUtils rather than left inline in api.js so it can be tested without standing up a request. 11 cases covering both member shapes, both directions. Stacked on security/rights-admin-access-fail-open-2405 (#7873), which has to merge first. On release.24.05 hasAdminAccess still starts from isAdmin = true and only clears when an entry for the app exists with `all` falsy, so an app the member has no entry for comes back as one they administer and hasReadRight returns true for it. Applying the owner's read right on top of that would have been fail-open, and two of the tests here fail without #7873 for exactly that reason. Same stacking as the alerts backport. --- plugins/views/api/api.js | 4 +- plugins/views/api/parts/viewsUtils.js | 28 ++++++++ .../plugins.views.token-owner-rights.js | 71 +++++++++++++++++++ 3 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 test/unit-tests/plugins.views.token-owner-rights.js diff --git a/plugins/views/api/api.js b/plugins/views/api/api.js index cc6099553ae..e5dcb7f6bab 100644 --- a/plugins/views/api/api.js +++ b/plugins/views/api/api.js @@ -8,7 +8,7 @@ var pluginOb = {}, plugins = require('../../pluginManager.js'), fetch = require('../../../api/parts/data/fetch.js'), log = common.log('views:api'), - { validateRead, validateUpdate, validateDelete, hasReadRight } = require('../../../api/utils/rights.js'); + { validateRead, validateUpdate, validateDelete } = require('../../../api/utils/rights.js'); const viewsUtils = require("./parts/viewsUtils.js"); const FEATURE_NAME = 'views'; @@ -1554,7 +1554,7 @@ const escapedViewSegments = { "name": true, "segment": true, "height": true, "wi //Apply the owner's read right here, the way validateRead does for the //api_key branch below. common.db.collection('members').findOne({_id: common.db.ObjectID(owner + "")}, function(memberErr, member) { - if (memberErr || !member || !hasReadRight(FEATURE_NAME, app._id + "", member)) { + if (memberErr || !member || !viewsUtils.ownerCanRead(member, app._id + "", FEATURE_NAME)) { common.returnMessage(params, 401, 'User does not have view right for this application'); return false; } diff --git a/plugins/views/api/parts/viewsUtils.js b/plugins/views/api/parts/viewsUtils.js index 013bcdc5071..dd3c6bc0e3e 100644 --- a/plugins/views/api/parts/viewsUtils.js +++ b/plugins/views/api/parts/viewsUtils.js @@ -2,8 +2,36 @@ var common = require('../../../../api/utils/common.js'); var plugins = require('../../../pluginManager.js'); var log = common.log('views:api'); var crypto = require('crypto'); +var { hasReadRight } = require('../../../../api/utils/rights.js'); module.exports = { + /** + * Whether a token's owner may read a feature for an app. + * + * hasReadRight covers feature permissions, an app admin and a global admin, but not + * the legacy membership validateRead still honours: a member stored before permission + * objects existed has no `permission` at all and is granted read through `user_of`. + * Refusing those would take away access the same member's api_key still has, so the + * fallback is applied here too, and a locked account is refused as validateRead + * refuses it. + * @param {object} member - the member a token resolved to + * @param {string} appId - id of the app the request resolved to + * @param {string} feature - feature being read + * @returns {boolean} true when this owner may read + **/ + ownerCanRead: function(member, appId, feature) { + if (!member || member.locked) { + return false; + } + if (member.global_admin) { + return true; + } + if (typeof member.permission === "undefined") { + return Array.isArray(member.user_of) && member.user_of.indexOf(appId) !== -1; + } + return !!hasReadRight(feature, appId, member); + }, + ommit_segments: function(options, callback) { var db = options.db || common.db; var omit = options.omit || []; diff --git a/test/unit-tests/plugins.views.token-owner-rights.js b/test/unit-tests/plugins.views.token-owner-rights.js new file mode 100644 index 00000000000..a20bbd9e9eb --- /dev/null +++ b/test/unit-tests/plugins.views.token-owner-rights.js @@ -0,0 +1,71 @@ +var should = require("should"); +var viewsUtils = require("../../plugins/views/api/parts/viewsUtils.js"); + +// The heatmap token branch used to serve whatever app the caller's app_key resolved to, +// without consulting the member the token belongs to. It now applies the owner's read +// right - but the api_key path it has to agree with is validateRead, which grants read to +// a legacy member through user_of, and hasReadRight alone does not. A token owner must +// not be refused where the same member's api_key would be accepted. + +describe("views: read right of a token owner", function() { + var APP = "6a41837e902bfd5369ddc610"; + var OTHER = "6a41837e902bfd5369ddc611"; + var FEATURE = "views"; + + describe("members carrying a permission object", function() { + it("allows the feature being explicitly allowed for this app", function() { + var member = {permission: {r: {}}}; + member.permission.r[APP] = {allowed: {views: true}}; + viewsUtils.ownerCanRead(member, APP, FEATURE).should.equal(true); + }); + it("allows an app admin, whose read permission is marked all", function() { + var member = {permission: {r: {}}}; + member.permission.r[APP] = {all: true}; + viewsUtils.ownerCanRead(member, APP, FEATURE).should.equal(true); + }); + it("refuses the feature being allowed on a different app", function() { + var member = {permission: {r: {}}}; + member.permission.r[OTHER] = {allowed: {views: true}}; + viewsUtils.ownerCanRead(member, APP, FEATURE).should.equal(false); + }); + it("refuses a different feature on this app", function() { + var member = {permission: {r: {}}}; + member.permission.r[APP] = {allowed: {crashes: true}}; + viewsUtils.ownerCanRead(member, APP, FEATURE).should.equal(false); + }); + }); + + describe("legacy members, stored before permission objects existed", function() { + it("allows one whose user_of carries this app", function() { + // validateRead grants exactly this, so refusing it here would take away access + // the same member's api_key still has + viewsUtils.ownerCanRead({user_of: [OTHER, APP]}, APP, FEATURE).should.equal(true); + }); + it("refuses one whose user_of does not", function() { + viewsUtils.ownerCanRead({user_of: [OTHER]}, APP, FEATURE).should.equal(false); + }); + it("refuses one with no user_of at all", function() { + viewsUtils.ownerCanRead({}, APP, FEATURE).should.equal(false); + viewsUtils.ownerCanRead({user_of: "not an array"}, APP, FEATURE).should.equal(false); + }); + it("does not consult user_of once a permission object is present", function() { + // a member who has been migrated is governed by the permission object alone + viewsUtils.ownerCanRead( + {permission: {r: {}}, user_of: [APP]}, APP, FEATURE).should.equal(false); + }); + }); + + describe("the cases that override everything else", function() { + it("allows a global admin", function() { + viewsUtils.ownerCanRead({global_admin: true}, APP, FEATURE).should.equal(true); + }); + it("refuses a locked account, whatever else it carries", function() { + viewsUtils.ownerCanRead({global_admin: true, locked: true}, APP, FEATURE).should.equal(false); + viewsUtils.ownerCanRead({user_of: [APP], locked: true}, APP, FEATURE).should.equal(false); + }); + it("refuses a missing member rather than throwing", function() { + viewsUtils.ownerCanRead(null, APP, FEATURE).should.equal(false); + viewsUtils.ownerCanRead(undefined, APP, FEATURE).should.equal(false); + }); + }); +}); From 913fea3e9d74c39cc1ae47c22b2f7367de960e82 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 14:58:04 +0300 Subject: [PATCH 5/5] [chore][test] require should for its side effect rather than binding it The suite uses the value.should.equal(...) style, so the module is needed for what it does to Object.prototype and never through the local name. CodeQL is right that the binding is dead. --- test/unit-tests/plugins.views.token-owner-rights.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit-tests/plugins.views.token-owner-rights.js b/test/unit-tests/plugins.views.token-owner-rights.js index a20bbd9e9eb..57d6c07297c 100644 --- a/test/unit-tests/plugins.views.token-owner-rights.js +++ b/test/unit-tests/plugins.views.token-owner-rights.js @@ -1,4 +1,4 @@ -var should = require("should"); +require("should"); var viewsUtils = require("../../plugins/views/api/parts/viewsUtils.js"); // The heatmap token branch used to serve whatever app the caller's app_key resolved to,