Skip to content

refactor(dbviewer): validate aggregation operators against an allow-list #7868

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ar2rsawseen wants to merge 5 commits into
base: master
Choose a base branch
from
Open

refactor(dbviewer): validate aggregation operators against an allow-list #7868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e0fa42d
refactor(dbviewer): validate aggregation operators against an allow-list
ar2rsawseen Jul 30, 2026
74e10d5
docs(dbviewer): note why the empty changes object is still passed
ar2rsawseen Jul 30, 2026
b28bf81
test(dbviewer): update query guard tests for rejection instead of str…
ar2rsawseen Jul 30, 2026
3143a85
fix(dbviewer): withhold the password-reset token from reads and joins
ar2rsawseen Jul 30, 2026
7396ceb
chore(dbviewer): drop an unused eslint-disable from the operator probe
ar2rsawseen Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 49 additions & 35 deletions plugins/dbviewer/api/api.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,15 @@ const FEATURE_NAME = 'dbviewer';
// upper bound on rows returned per find()/aggregation page, to keep a crafted
// limit/iDisplayLength from requesting an unbounded result set
const MAX_DBVIEWER_LIMIT = 10000;
// Aggregation-stage allow-list and the recursive sanitizer that strips blocked
// stages at every depth (including inside $facet sub-pipelines). Kept in a
// dedicated module so it can be unit-tested in isolation.
const { sanitizeAggregation, ALLOWED_STAGES_USER, ALLOWED_STAGES_GLOBAL_ADMIN } = require('./parts/aggregation_guard.js');
const { sanitizeProjection, escapeRegExp } = require('./parts/query_guard.js');
// Allow-list of MongoDB operators and the validator that rejects a pipeline using
// anything outside it, at any depth. Kept in a dedicated module so it can be
// unit-tested in isolation.
const { sanitizeAggregation, ALLOWED_OPERATORS_USER, ALLOWED_OPERATORS_GLOBAL_ADMIN } = require('./parts/aggregation_guard.js');
const { findDisallowedProjectionValue, escapeRegExp } = require('./parts/query_guard.js');
// Fields the viewer must never return, and the helpers that remove them. Held in one
// module because the same redaction applies in three places (document read,
// collection read, aggregation) and a per-site list drifts.
const { hasRedactedFields, redactFields, redactionStage } = require('./parts/redaction.js');
var spawn = require('child_process').spawn,
child;

Expand Down Expand Up @@ -139,10 +143,8 @@ var spawn = require('child_process').spawn,
}
dbs[dbNameOnParam].collection(params.qstring.collection).findOne(docFilter, function(err, results) {
if (!err) {
if (params.qstring.collection === 'members' && results) {
delete results.password;
delete results.api_key;
delete results.two_factor_auth;
if (hasRedactedFields(params.qstring.collection)) {
redactFields(params.qstring.collection, results);
}
else if (params.qstring.collection === 'auth_tokens' && results) {
if (results._id) {
Expand Down Expand Up @@ -238,10 +240,15 @@ var spawn = require('child_process').spawn,
common.returnMessage(params, 400, common.unsafeQueryError(badOp));
return false;
}
//restrict the projection to plain field include/exclude — drop any
//expression / field-path alias (e.g. {x:"$password"}) that could
//compute or rename fields the viewer otherwise removes
sanitizeProjection(projection);
//the projection must be plain field include/exclude. An expression or
//field-path alias (e.g. {x:"$password"}) could compute or rename fields
//the viewer otherwise removes, so reject rather than quietly dropping
//the field and returning different results than were asked for
var badProjection = findDisallowedProjectionValue(projection);
if (badProjection) {
common.returnMessage(params, 400, 'Projection for "' + badProjection.name + '" must be 0 or 1');
return;
}

var base_filter = {};
if (!params.member.global_admin) {
Expand Down Expand Up @@ -284,10 +291,8 @@ var spawn = require('child_process').spawn,
var stream = cursor.skip(skip).limit(limit).stream({
transform: function(doc) {

if (params.qstring.collection === 'members' && doc) {
delete doc.password;
delete doc.api_key;
delete doc.two_factor_auth;
if (hasRedactedFields(params.qstring.collection)) {
redactFields(params.qstring.collection, doc);
}
else if (params.qstring.collection === 'auth_tokens' && doc) {
if (doc._id) {
Expand Down Expand Up @@ -418,13 +423,14 @@ var spawn = require('child_process').spawn,
aggregation.push({ "$limit": Math.min(iDisplayLength, MAX_DBVIEWER_LIMIT) });
}
}
if (collection === 'members') {
var redaction = redactionStage(collection);
if (redaction) {
// Insert the redaction as the very first stage so no
// user-supplied stage — including a leading $match using
// $expr, or a $project/$group that aliases or references the
// field — can read the raw credential fields before they are
// removed.
aggregation.splice(0, 0, {"$project": {"password": 0, "api_key": 0, "two_factor_auth": 0}});
aggregation.splice(0, 0, redaction);
}
else if (collection === 'auth_tokens') {
aggregation.splice(0, 0, {"$addFields": {"_id": "***redacted***"}});
Expand Down Expand Up @@ -573,39 +579,47 @@ var spawn = require('child_process').spawn,
}
// handle aggregation request
else if (isContainDb && params.qstring.aggregation) {
// Validate the pipeline against the caller's allow-list and run
// it. Global admins get the broader list (may join/union, but
// still no writes and never into a redacted collection); other
// users get the restricted list. Disallowed stages are stripped;
// server-side-JS operators and joins into members/auth_tokens
// reject the request.
var runAggregation = function(allowedStages) {
// Validate the pipeline against the caller's allow-list of MongoDB
// operators and run it unchanged. Global admins get the broader
// list (they may join and union, but still not into a redacted
// collection); other users get the restricted one. Anything the
// caller may not use rejects the request rather than being removed
// from the pipeline, so the result is never quietly different from
// what was asked for.
var runAggregation = function(allowedOperators) {
try {
let aggregation = EJSON.parse(params.qstring.aggregation);
var guard = sanitizeAggregation(aggregation, allowedStages);
var guard = sanitizeAggregation(aggregation, allowedOperators);
if (guard.error) {
var msg = guard.error.type === "join"
? 'Aggregation may not join the "' + guard.error.name + '" collection'
: 'Aggregation may not use the "' + guard.error.name + '" operator';
var msg;
if (guard.error.type === "join") {
msg = 'Aggregation may not join the "' + guard.error.name + '" collection';
}
else if (guard.error.type === "stage") {
msg = 'Aggregation stage at ' + guard.error.where + ' does not name a pipeline stage';
}
else {
msg = 'Aggregation may not use the "' + guard.error.name + '" operator (at ' + guard.error.where + ')';
}
common.returnMessage(params, 400, msg);
return;
}
if (guard.changes && Object.keys(guard.changes).length > 0) {
log.d("Removed stages from pipeline: ", JSON.stringify(guard.changes));
}
//guard.changes is always empty now that nothing is stripped. It is
//still passed so the response keeps its "removed" field and the UI
//contract does not change
aggregate(params.qstring.collection, aggregation, guard.changes);
}
catch (e) {
common.returnMessage(params, 500, 'Aggregation object is not valid.');
}
};
if (params.member.global_admin) {
runAggregation(ALLOWED_STAGES_GLOBAL_ADMIN);
runAggregation(ALLOWED_OPERATORS_GLOBAL_ADMIN);
}
else {
userHasAccess(params, params.qstring.collection, function(hasAccess) {
if (hasAccess || params.qstring.collection === "events_data" || params.qstring.collection === "drill_events") {
runAggregation(ALLOWED_STAGES_USER);
runAggregation(ALLOWED_OPERATORS_USER);
}
else {
common.returnMessage(params, 401, 'User does not have right tot view this colleciton');
Expand Down
Loading
Loading