Skip to content

Commit 808b923

Browse files
bewestCopilot
bewest
and
Copilot
committed
feat(api): add _id validation to activity and food APIs
Add validation for _id field in activity and food APIs: - activity: POST, PUT, DELETE now validate _id format - food: POST, PUT, DELETE now validate _id format Accepts: undefined, null, or 24-character hex string Rejects: UUIDs, short strings, numbers, objects with 400 Bad Request Previously: - activity: 500 crash on invalid _id in save/remove - food: silently replaced invalid _id with new ObjectId (data loss) Tests added covering all validation cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 8d44a04 commit 808b923

3 files changed

Lines changed: 188 additions & 0 deletions

File tree

‎lib/api/activity/index.js‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,30 @@ var _isArray = require('lodash/isArray');
77
var consts = require('../../constants');
88
var moment = require('moment');
99

10+
/**
11+
* Validate MongoDB ObjectId format.
12+
* Accepts: undefined, null, or 24-character hex string.
13+
* Rejects: anything else (UUIDs, short strings, numbers, objects).
14+
*/
15+
function isValidObjectId(id) {
16+
if (id === undefined || id === null) return true;
17+
if (typeof id !== 'string') return false;
18+
return /^[a-fA-F0-9]{24}$/.test(id);
19+
}
20+
21+
/**
22+
* Validate _id field for each document in an array.
23+
* @returns {Object|null} - null if all valid, or {index, id} of first invalid
24+
*/
25+
function findInvalidId(docs) {
26+
for (var i = 0; i < docs.length; i++) {
27+
if (!isValidObjectId(docs[i]._id)) {
28+
return { index: i, id: docs[i]._id };
29+
}
30+
}
31+
return null;
32+
}
33+
1034
function configure(app, wares, ctx) {
1135
var express = require('express')
1236
, api = express.Router();
@@ -73,6 +97,13 @@ function configure(app, wares, ctx) {
7397
activity = [activity];
7498
}
7599

100+
// Validate _id fields before storage (return 400 on invalid)
101+
var invalid = findInvalidId(activity);
102+
if (invalid) {
103+
return res.sendJSONStatus(res, consts.HTTP_BAD_REQUEST,
104+
'Invalid _id format', 'Must be 24-character hex string or omit for auto-generation. Got: ' + String(invalid.id));
105+
}
106+
76107
ctx.activity.create(activity, function(err, created) {
77108
if (err) {
78109
console.log('Error adding activity data', err);
@@ -87,6 +118,11 @@ function configure(app, wares, ctx) {
87118
api.post('/activity/', ctx.authorization.isPermitted('api:activity:create'), post_response);
88119

89120
api.delete('/activity/:_id', ctx.authorization.isPermitted('api:activity:delete'), function(req, res) {
121+
// Validate _id parameter
122+
if (!isValidObjectId(req.params._id)) {
123+
return res.sendJSONStatus(res, consts.HTTP_BAD_REQUEST,
124+
'Invalid _id format', 'Must be 24-character hex string. Got: ' + String(req.params._id));
125+
}
90126
ctx.activity.remove(req.params._id, function() {
91127
res.json({});
92128
});
@@ -95,6 +131,13 @@ function configure(app, wares, ctx) {
95131
// update record
96132
api.put('/activity/', ctx.authorization.isPermitted('api:activity:update'), function(req, res) {
97133
var data = req.body;
134+
135+
// Validate _id if provided
136+
if (!isValidObjectId(data._id)) {
137+
return res.sendJSONStatus(res, consts.HTTP_BAD_REQUEST,
138+
'Invalid _id format', 'Must be 24-character hex string. Got: ' + String(data._id));
139+
}
140+
98141
ctx.activity.save(data, function(err, created) {
99142
if (err) {
100143
res.sendJSONStatus(res, consts.HTTP_INTERNAL_ERROR, 'Mongo Error', err);

‎lib/api/food/index.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,17 @@
22

33
var consts = require('../../constants');
44

5+
/**
6+
* Validate MongoDB ObjectId format.
7+
* Accepts: undefined, null, or 24-character hex string.
8+
* Rejects: anything else (UUIDs, short strings, numbers, objects).
9+
*/
10+
function isValidObjectId(id) {
11+
if (id === undefined || id === null) return true;
12+
if (typeof id !== 'string') return false;
13+
return /^[a-fA-F0-9]{24}$/.test(id);
14+
}
15+
516
function configure (app, wares, ctx) {
617
var express = require('express'),
718
api = express.Router( );
@@ -43,6 +54,13 @@ function configure (app, wares, ctx) {
4354
// create new record
4455
api.post('/food/', ctx.authorization.isPermitted('api:food:create'), function(req, res) {
4556
var data = req.body;
57+
58+
// Validate _id if provided
59+
if (!isValidObjectId(data._id)) {
60+
return res.sendJSONStatus(res, consts.HTTP_BAD_REQUEST,
61+
'Invalid _id format', 'Must be 24-character hex string or omit for auto-generation. Got: ' + String(data._id));
62+
}
63+
4664
ctx.food.create(data, function (err, created) {
4765
if (err) {
4866
res.sendJSONStatus(res, consts.HTTP_INTERNAL_ERROR, 'Mongo Error', err);
@@ -58,6 +76,13 @@ function configure (app, wares, ctx) {
5876
// update record
5977
api.put('/food/', ctx.authorization.isPermitted('api:food:update'), function(req, res) {
6078
var data = req.body;
79+
80+
// Validate _id if provided
81+
if (!isValidObjectId(data._id)) {
82+
return res.sendJSONStatus(res, consts.HTTP_BAD_REQUEST,
83+
'Invalid _id format', 'Must be 24-character hex string. Got: ' + String(data._id));
84+
}
85+
6186
ctx.food.save(data, function (err, created) {
6287
if (err) {
6388
res.sendJSONStatus(res, consts.HTTP_INTERNAL_ERROR, 'Mongo Error', err);
@@ -71,6 +96,11 @@ function configure (app, wares, ctx) {
7196
});
7297
// delete record
7398
api.delete('/food/:_id', ctx.authorization.isPermitted('api:food:delete'), function(req, res) {
99+
// Validate _id parameter
100+
if (!isValidObjectId(req.params._id)) {
101+
return res.sendJSONStatus(res, consts.HTTP_BAD_REQUEST,
102+
'Invalid _id format', 'Must be 24-character hex string. Got: ' + String(req.params._id));
103+
}
74104
ctx.food.remove(req.params._id, function ( ) {
75105
res.json({ });
76106
});

‎tests/api.id-validation.test.js‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
'use strict';
2+
3+
var request = require('supertest');
4+
var should = require('should');
5+
var language = require('../lib/language')();
6+
7+
describe('_id Validation API Tests', function() {
8+
this.timeout(10000);
9+
var self = this;
10+
var known = 'b723e97aa97846eb92d5264f084b2823f57c4aa1';
11+
12+
var api = require('../lib/api/');
13+
14+
before(function(done) {
15+
process.env.API_SECRET = 'this is my long pass phrase';
16+
self.env = require('../lib/server/env')();
17+
self.env.settings.authDefaultRoles = 'readable';
18+
self.env.settings.enable = ['careportal', 'api'];
19+
this.wares = require('../lib/middleware/')(self.env);
20+
self.app = require('express')();
21+
self.app.enable('api');
22+
self.app.enable('careportal');
23+
require('../lib/server/bootevent')(self.env, language).boot(function booted(ctx) {
24+
self.ctx = ctx;
25+
self.ctx.ddata = require('../lib/data/ddata')();
26+
self.app.use('/api', api(self.env, ctx));
27+
done();
28+
});
29+
});
30+
31+
describe('Activity API _id validation', function() {
32+
it('should return 400 for POST with invalid UUID _id', function(done) {
33+
request(self.app)
34+
.post('/api/activity/')
35+
.set('api-secret', known)
36+
.send({ "_id": "my-uuid-12345", "created_at": "2024-01-01T00:00:00Z", "steps": 1000 })
37+
.expect(400)
38+
.expect(function(response) {
39+
response.body.should.have.property('status', 400);
40+
response.body.message.should.match(/Invalid _id format/i);
41+
})
42+
.end(done);
43+
});
44+
45+
it('should return 400 for PUT with invalid _id', function(done) {
46+
request(self.app)
47+
.put('/api/activity/')
48+
.set('api-secret', known)
49+
.send({ "_id": "not-valid", "created_at": "2024-01-01T00:00:00Z", "steps": 1000 })
50+
.expect(400)
51+
.end(done);
52+
});
53+
54+
it('should return 400 for DELETE with invalid _id', function(done) {
55+
request(self.app)
56+
.delete('/api/activity/invalid-id')
57+
.set('api-secret', known)
58+
.expect(400)
59+
.end(done);
60+
});
61+
62+
it('should accept POST without _id (auto-generate)', function(done) {
63+
request(self.app)
64+
.post('/api/activity/')
65+
.set('api-secret', known)
66+
.send({ "created_at": "2024-01-01T00:00:00Z", "steps": 1000 })
67+
.expect(200)
68+
.end(done);
69+
});
70+
});
71+
72+
describe('Food API _id validation', function() {
73+
it('should return 400 for POST with invalid UUID _id', function(done) {
74+
request(self.app)
75+
.post('/api/food/')
76+
.set('api-secret', known)
77+
.send({ "_id": "my-uuid-12345", "name": "Apple", "type": "food", "carbs": 15 })
78+
.expect(400)
79+
.expect(function(response) {
80+
response.body.should.have.property('status', 400);
81+
response.body.message.should.match(/Invalid _id format/i);
82+
})
83+
.end(done);
84+
});
85+
86+
it('should return 400 for PUT with invalid _id', function(done) {
87+
request(self.app)
88+
.put('/api/food/')
89+
.set('api-secret', known)
90+
.send({ "_id": "not-valid", "name": "Apple", "type": "food", "carbs": 15 })
91+
.expect(400)
92+
.end(done);
93+
});
94+
95+
it('should return 400 for DELETE with invalid _id', function(done) {
96+
request(self.app)
97+
.delete('/api/food/invalid-id')
98+
.set('api-secret', known)
99+
.expect(400)
100+
.end(done);
101+
});
102+
103+
it('should accept POST without _id (auto-generate)', function(done) {
104+
request(self.app)
105+
.post('/api/food/')
106+
.set('api-secret', known)
107+
.send({ "name": "Banana", "type": "food", "carbs": 27 })
108+
.expect(200)
109+
.expect(function(response) {
110+
response.body.should.have.property('_id');
111+
})
112+
.end(done);
113+
});
114+
});
115+
});

0 commit comments

Comments
 (0)