Skip to content

Commit e5cf7c3

Browse files
ShreyasPatil3105ShreyasPatil3105
andauthored
fix(core): patch storage quota evasion loophole in update pipeline (geturbackend#224)
fix(core): patch storage quota evasion loophole in update pipeline fix(core): implement atomic transactions and resolve code review feedback Co-authored-by: ShreyasPatil3105 <ShreyasPatil3105-shreyasp310505@gmail.com>
1 parent 7114111 commit e5cf7c3

1 file changed

Lines changed: 115 additions & 60 deletions

File tree

apps/public-api/src/controllers/data.controller.js

Lines changed: 115 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ const { QueryEngine } = require("@urbackend/common");
77
const { validateData, validateUpdateData, aggregateSchema, webhookQueue } = require("@urbackend/common");
88
const { performance } = require('perf_hooks');
99
const { z } = require("zod");
10-
const {
11-
AppError,
10+
const {
11+
AppError,
1212
enqueueCollectionCleanup,
1313
syncCollectionCleanup
1414
} = require("@urbackend/common");
@@ -109,8 +109,7 @@ module.exports.insertData = async (req, res) => {
109109
};
110110

111111
// BULK INSERT DATA
112-
113-
module.exports.bulkInsertData = async (req, res, next) => {
112+
module.exports.bulkInsertData = async (req, res, next) => {
114113
try {
115114
const MAX_BULK_INSERT_LIMIT = 100;
116115

@@ -119,16 +118,16 @@ module.exports.insertData = async (req, res) => {
119118
const incomingData = req.body;
120119

121120
if (!Array.isArray(incomingData)) {
122-
return next(new AppError("Request body must be an array of objects", 400));
121+
return next(new AppError(400, "Request body must be an array of objects"));
123122
}
124123

125124
if (incomingData.length === 0) {
126-
return next(new AppError("Request body cannot be empty", 400));
125+
return next(new AppError(400, "Request body cannot be empty"));
127126
}
128127

129128
if (incomingData.length > MAX_BULK_INSERT_LIMIT) {
130129
return next(
131-
new AppError(`Maximum ${MAX_BULK_INSERT_LIMIT} records allowed`, 400)
130+
new AppError(400, `Maximum ${MAX_BULK_INSERT_LIMIT} records allowed`)
132131
);
133132
}
134133

@@ -137,7 +136,7 @@ module.exports.insertData = async (req, res) => {
137136
);
138137

139138
if (!collectionConfig) {
140-
return next(new AppError("Collection not found", 404));
139+
return next(new AppError(404, "Collection not found"));
141140
}
142141

143142
const schemaRules = collectionConfig.model;
@@ -159,7 +158,7 @@ module.exports.insertData = async (req, res) => {
159158
// Prevent manual injection of soft-delete fields
160159
delete cleanData.isDeleted;
161160
delete cleanData.deletedAt;
162-
161+
163162
validData.push(sanitize({
164163
...cleanData,
165164
isDeleted: false,
@@ -170,7 +169,7 @@ module.exports.insertData = async (req, res) => {
170169

171170
if (invalidIndices.length > 0) {
172171
return next(
173-
new AppError(`Invalid records at index: ${invalidIndices.join(", ")}`, 400)
172+
new AppError(400, `Invalid records at index: ${invalidIndices.join(", ")}`)
174173
);
175174
}
176175

@@ -196,16 +195,17 @@ module.exports.insertData = async (req, res) => {
196195
console.error(err);
197196
}
198197

199-
if (isDuplicateKeyError(err)) {
200-
return next(
201-
new AppError("Duplicate value violates unique constraint.", 409)
202-
);
203-
}
198+
if (isDuplicateKeyError(err)) {
199+
return next(
200+
new AppError(409, "Duplicate value violates unique constraint.")
201+
);
202+
}
204203

205-
return next(new AppError("Failed to insert bulk data", 500));
206-
}
204+
return next(new AppError(500, "Failed to insert bulk data"));
205+
}
207206
};
208207

208+
209209
// GET ALL DATA
210210
module.exports.getAllData = async (req, res) => {
211211
try {
@@ -261,6 +261,8 @@ module.exports.getAllData = async (req, res) => {
261261
features.sort().populate();
262262

263263
const total = await features.count();
264+
const parsedLimit = parseInt(req.query.limit, 10);
265+
const limit = Math.max(1, Math.min(Number.isNaN(parsedLimit) ? 100 : parsedLimit, 100));
264266

265267
// Use cursor-based pagination if cursor parameter is provided, otherwise use offset-based
266268
const useCursor = !!req.query.cursor;
@@ -276,7 +278,7 @@ module.exports.getAllData = async (req, res) => {
276278
let items = data;
277279
let nextCursor = null;
278280
if (useCursor) {
279-
const limit = Math.min(parseInt(req.query.limit, 10) || 100, 100);
281+
280282
features.generateNextCursor(data, limit);
281283
items = data.slice(0, limit);
282284
nextCursor = features.nextCursor;
@@ -286,16 +288,16 @@ module.exports.getAllData = async (req, res) => {
286288

287289
const responseMeta = useCursor
288290
? {
289-
total,
290-
cursor: req.query.cursor || null,
291-
nextCursor,
292-
limit: Math.max(1, Math.min(parseInt(req.query.limit, 10) || 100, 100)),
293-
}
291+
total,
292+
cursor: req.query.cursor || null,
293+
nextCursor,
294+
limit,
295+
}
294296
: {
295-
total,
296-
page: parseInt(req.query.page, 10) || 1,
297-
limit: Math.max(1, Math.min(parseInt(req.query.limit, 10) || 100, 100)),
298-
};
297+
total,
298+
page: parseInt(req.query.page, 10) || 1,
299+
limit,
300+
};
299301

300302
res.json({
301303
success: true,
@@ -350,7 +352,7 @@ module.exports.getSingleDoc = async (req, res) => {
350352
);
351353

352354
const baseFilter = req.rlsFilter && typeof req.rlsFilter === 'object' ? req.rlsFilter : {};
353-
355+
354356
// Soft delete filter
355357
const includeDeleted = req.query.include_deleted === 'true';
356358
const softDeleteFilter = includeDeleted ? {} : { isDeleted: { $ne: true } };
@@ -442,7 +444,7 @@ module.exports.aggregateData = async (req, res) => {
442444
// $geoNear and $search must be the first stage in the pipeline if present
443445
let effectivePipeline = [];
444446
const firstStage = pipeline.length > 0 ? Object.keys(pipeline[0])[0] : null;
445-
447+
446448
if (firstStage === '$geoNear' || firstStage === '$search') {
447449
effectivePipeline = [
448450
pipeline[0],
@@ -487,20 +489,20 @@ module.exports.aggregateData = async (req, res) => {
487489
};
488490

489491
// UPDATE DATA
490-
module.exports.updateSingleData = async (req, res) => {
492+
module.exports.updateSingleData = async (req, res, next) => {
491493
try {
492494
const { collectionName, id } = req.params;
493495
const project = req.project;
494496
const incomingData = req.body;
495497

496498
if (!isValidId(id))
497-
return res.status(400).json({ error: "Invalid ID format." });
499+
return next(new AppError(400, "Invalid ID format."));
498500

499501
const collectionConfig = project.collections.find(
500502
(c) => c.name === collectionName,
501503
);
502504
if (!collectionConfig)
503-
return res.status(404).json({ error: "Collection not found" });
505+
return next(new AppError(404, "Collection not found"));
504506

505507
const connection = await getConnection(project._id);
506508
const Model = getCompiledModel(
@@ -517,7 +519,7 @@ module.exports.updateSingleData = async (req, res) => {
517519
);
518520

519521
if (validationError)
520-
return res.status(400).json({ error: validationError });
522+
return next(new AppError(400, validationError));
521523

522524
// Prevent manual injection of soft-delete fields
523525
delete updateData.isDeleted;
@@ -526,14 +528,70 @@ module.exports.updateSingleData = async (req, res) => {
526528
const sanitizedData = sanitize(updateData);
527529

528530
const baseFilter = req.rlsFilter && typeof req.rlsFilter === 'object' ? req.rlsFilter : {};
531+
const queryFilter = { $and: [{ _id: id }, { isDeleted: { $ne: true } }, baseFilter] };
529532

530-
const result = await Model.findOneAndUpdate(
531-
{ $and: [{ _id: id }, { isDeleted: { $ne: true } }, baseFilter] },
532-
{ $set: sanitizedData },
533-
{ new: true, runValidators: true },
534-
).lean();
533+
let result;
534+
535+
// Only enforce quota for internal databases
536+
if (!project.resources.db.isExternal) {
537+
const session = await mongoose.startSession();
538+
session.startTransaction();
539+
540+
try {
541+
// 1. Fetch existing doc securely within transaction
542+
const existingDoc = await Model.findOne(queryFilter).session(session).lean();
543+
if (!existingDoc) {
544+
await session.abortTransaction();
545+
session.endSession();
546+
return next(new AppError(404, "Document not found."));
547+
}
548+
549+
// 2. Calculate sizes
550+
const oldSize = mongoose.mongo.BSON.calculateObjectSize(existingDoc);
551+
const simulatedNewDoc = { ...existingDoc, ...sanitizedData };
552+
const newSize = mongoose.mongo.BSON.calculateObjectSize(simulatedNewDoc);
553+
const sizeDelta = newSize - oldSize;
554+
555+
// 3. Enforce quota if size is increasing
556+
if (sizeDelta > 0) {
557+
if ((project.databaseUsed || 0) + sizeDelta > project.databaseLimit) {
558+
await session.abortTransaction();
559+
session.endSession();
560+
return next(new AppError(403, "Storage quota exceeded. Please upgrade your plan."));
561+
}
562+
}
535563

536-
if (!result) return res.status(404).json({ error: "Document not found." });
564+
// 4. Update the document
565+
result = await Model.findOneAndUpdate(
566+
queryFilter,
567+
{ $set: sanitizedData },
568+
{ new: true, runValidators: true, session },
569+
).lean();
570+
571+
// 5. Apply the delta (positive or negative) atomically
572+
await Project.findByIdAndUpdate(
573+
project._id,
574+
{ $inc: { databaseUsed: sizeDelta } },
575+
{ session }
576+
);
577+
578+
await session.commitTransaction();
579+
session.endSession();
580+
} catch (error) {
581+
await session.abortTransaction();
582+
session.endSession();
583+
throw error;
584+
}
585+
} else {
586+
// External DB Flow (No quota checks)
587+
result = await Model.findOneAndUpdate(
588+
queryFilter,
589+
{ $set: sanitizedData },
590+
{ new: true, runValidators: true },
591+
).lean();
592+
593+
if (!result) return next(new AppError(404, "Document not found."));
594+
}
537595

538596
await webhookQueue.add('trigger-webhook', {
539597
projectId: project._id,
@@ -542,20 +600,17 @@ module.exports.updateSingleData = async (req, res) => {
542600
payload: result
543601
}, { removeOnComplete: true });
544602

545-
res.json({ message: "Updated", data: result });
603+
res.json({ success: true, data: result, message: "Updated" });
546604
} catch (err) {
547605
if (process.env.NODE_ENV !== 'test') {
548606
console.error(err);
549607
}
550608

551609
if (isDuplicateKeyError(err)) {
552-
return res.status(409).json({
553-
error: "Duplicate value violates unique constraint.",
554-
details: err.message,
555-
});
610+
return next(new AppError(409, "Duplicate value violates unique constraint."));
556611
}
557612

558-
res.status(500).json({ error: err.message });
613+
return next(new AppError(500, err.message));
559614
}
560615
};
561616

@@ -588,11 +643,11 @@ module.exports.deleteSingleDoc = async (req, res) => {
588643

589644
const result = await Model.findOneAndUpdate(
590645
{ _id: id, isDeleted: { $ne: true }, ...(req.rlsFilter || {}) },
591-
{
592-
$set: {
593-
isDeleted: true,
594-
deletedAt: new Date()
595-
}
646+
{
647+
$set: {
648+
isDeleted: true,
649+
deletedAt: new Date()
650+
}
596651
},
597652
{ new: false } // return the original document for webhook
598653
).lean();
@@ -605,7 +660,7 @@ module.exports.deleteSingleDoc = async (req, res) => {
605660
try {
606661
await enqueueCollectionCleanup(project._id, collectionName);
607662
} catch (err) {
608-
console.error("Failed to enqueue trash cleanup job", { projectId: String(project._id), collectionName, err });
663+
console.error("Failed to enqueue trash cleanup job", { projectId: String(project._id), collectionName, err });
609664
}
610665

611666
await webhookQueue.add('trigger-webhook', {
@@ -657,17 +712,17 @@ module.exports.recoverSingleDoc = async (req, res, next) => {
657712
const thirtyDaysAgo = new Date(Date.now() - 30 * 24 * 60 * 60 * 1000);
658713

659714
const result = await Model.findOneAndUpdate(
660-
{
661-
_id: id,
662-
isDeleted: true,
715+
{
716+
_id: id,
717+
isDeleted: true,
663718
deletedAt: { $gte: thirtyDaysAgo },
664-
...(req.rlsFilter || {})
719+
...(req.rlsFilter || {})
665720
},
666-
{
667-
$set: {
668-
isDeleted: false,
669-
deletedAt: null
670-
}
721+
{
722+
$set: {
723+
isDeleted: false,
724+
deletedAt: null
725+
}
671726
},
672727
{ new: true }
673728
).lean();

0 commit comments

Comments
 (0)