Skip to content

Commit 4fdf12a

Browse files
tariqksolimantariqksoliman
authored
Merge pull request #4 from JPL-Devin/devin/1775595279-blocker-fixes-e2e-tests
fix: BLOCKER fixes + E2E & unit tests for security, correctness, and code quality
2 parents 683db2b + 7335fe0 commit 4fdf12a

File tree

19 files changed

+663
-194
lines changed

19 files changed

+663
-194
lines changed

‎API/Backend/Geodatasets/routes/geodatasets.js‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -205,14 +205,14 @@ function get(reqtype, req, res, next) {
205205
t += [
206206
`((`,
207207
`${startProp} IS NOT NULL AND ${endProp} IS NOT NULL AND`,
208-
` ${startProp} >= ${start_time}`,
209-
` AND ${endProp} <= ${end_time}`,
208+
` ${startProp} >= :start_time`,
209+
` AND ${endProp} <= :end_time`,
210210
`)`,
211211
` OR `,
212212
`(`,
213213
`${startProp} IS NULL AND ${endProp} IS NOT NULL AND`,
214-
` ${endProp} >= ${start_time}`,
215-
` AND ${endProp} <= ${end_time}`,
214+
` ${endProp} >= :start_time`,
215+
` AND ${endProp} <= :end_time`,
216216
`))`
217217
].join('')
218218
q += t;
@@ -653,14 +653,14 @@ router.post("/intersect", function (req, res, next) {
653653
t += [
654654
`((`,
655655
`${startProp} IS NOT NULL AND ${endProp} IS NOT NULL AND`,
656-
` ${startProp} >= ${start_time}`,
657-
` AND ${endProp} <= ${end_time}`,
656+
` ${startProp} >= :start_time`,
657+
` AND ${endProp} <= :end_time`,
658658
`)`,
659659
` OR `,
660660
`(`,
661661
`${startProp} IS NULL AND ${endProp} IS NOT NULL AND`,
662-
` ${endProp} >= ${start_time}`,
663-
` AND ${endProp} <= ${end_time}`,
662+
` ${endProp} >= :start_time`,
663+
` AND ${endProp} <= :end_time`,
664664
`))`
665665
].join('')
666666
q += t;
@@ -800,17 +800,17 @@ router.get("/aggregations", function (req, res, next) {
800800
endProp = Utils.forceAlphaNumUnder(req.query.endProp || endProp);
801801
// prettier-ignore
802802
t += [
803-
`(`,
803+
`((`,
804804
`${startProp} IS NOT NULL AND ${endProp} IS NOT NULL AND`,
805-
` ${startProp} >= ${start_time}`,
806-
` AND ${endProp} <= ${end_time}`,
805+
` ${startProp} >= :start_time`,
806+
` AND ${endProp} <= :end_time`,
807807
`)`,
808808
` OR `,
809809
`(`,
810810
`${startProp} IS NULL AND ${endProp} IS NOT NULL AND`,
811-
` ${endProp} >= ${start_time}`,
812-
` AND ${endProp} <= ${end_time}`,
813-
`)`
811+
` ${endProp} >= :start_time`,
812+
` AND ${endProp} <= :end_time`,
813+
`))`
814814
].join('')
815815
q += t;
816816
}

‎API/Backend/Users/routes/users.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -298,19 +298,19 @@ router.post("/login", function (req, res) {
298298
: "",
299299
});
300300
});
301-
return null;
301+
return;
302302
})
303303
.catch((err) => {
304304
res.send({ status: "failure", message: "Login failed." });
305-
return null;
305+
return;
306306
});
307-
return null;
307+
return;
308308
} else {
309309
res.send({
310310
status: "failure",
311311
message: "Invalid username or password.",
312312
});
313-
return null;
313+
return;
314314
}
315315
}
316316

‎configure/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "configure",
3-
"version": "4.2.36-20260402",
3+
"version": "4.2.38-20260408",
44
"homepage": "./configure/build",
55
"private": true,
66
"dependencies": {

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "mmgis",
3-
"version": "4.2.36-20260402",
3+
"version": "4.2.38-20260408",
44
"description": "A web-based mapping and localization solution for science operation on planetary missions.",
55
"homepage": "build",
66
"repository": {

‎playwright.config.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ export default defineConfig({
1111
// Test file patterns
1212
testMatch: "**/*.spec.js",
1313

14-
// Timeout per test
15-
timeout: 30 * 1000,
14+
// Timeout per test (2 minutes — E2E tests may need extra time on slower machines)
15+
timeout: 120 * 1000,
1616

1717
// Test execution settings
1818
fullyParallel: true,

‎scripts/middleware.js‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,17 @@ async function compositeImageUrls(urls) {
3838

3939
async function onlyExistingFilepaths(paths) {
4040
const filePromises = [];
41-
paths.forEach((path) => {
41+
paths.forEach((filePath) => {
4242
filePromises.push(
4343
new Promise(async (resolve, reject) => {
4444
try {
45-
await fs.promises.access(`${rootDirMissions}${path}`);
46-
resolve(path);
45+
const fullPath = path.resolve(`${rootDirMissions}${filePath}`);
46+
if (!fullPath.replace(/\\/g, '/').startsWith(path.resolve(rootDirMissions).replace(/\\/g, '/') + '/')) {
47+
resolve(false);
48+
return;
49+
}
50+
await fs.promises.access(fullPath);
51+
resolve(filePath);
4752
} catch (err) {
4853
resolve(false);
4954
}

‎src/essence/Basics/Map_/Map_.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1329,6 +1329,7 @@ async function makeTileLayer(layerObj, mapContext = null) {
13291329
layerObj.tileMatrixSet || 'WebMercatorQuad'
13301330
}/{z}/{x}/{y}.webp?url=${layerUrl}${bandsParam}${resamplingParam}`
13311331

1332+
break
13321333
default:
13331334
break
13341335
}

‎src/essence/Tools/Chemistry/chemistryplot.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ import L_ from '../../Basics/Layers_/Layers_'
55
import CursorInfo from '../../Ancillary/CursorInfo'
66

77
var clearFunction
8+
var chemsArray = []
9+
var chemsNames = []
10+
var apxsArray = []
11+
var apxsNames = []
812

913
function makeChemistryPlot(chems, names, id) {
1014
var svg

‎src/normalize.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ define(function() {
8888

8989
baseParts.pop();
9090

91+
var curPart;
9192
while (curPart = uriParts.shift())
9293
if (curPart == '..')
9394
baseParts.pop();
@@ -105,7 +106,7 @@ define(function() {
105106
var baseParts = base.split('/');
106107
baseParts.pop();
107108
base = baseParts.join('/') + '/';
108-
i = 0;
109+
var i = 0;
109110
while (base.substr(i, 1) == uri.substr(i, 1))
110111
i++;
111112
while (base.substr(i, 1) != '/')
@@ -116,11 +117,12 @@ define(function() {
116117
// each base folder difference is thus a backtrack
117118
baseParts = base.split('/');
118119
var uriParts = uri.split('/');
119-
out = '';
120+
var out = '';
120121
while (baseParts.shift())
121122
out += '../';
122123

123124
// finally add uri parts
125+
var curPart;
124126
while (curPart = uriParts.shift())
125127
out += curPart + '/';
126128

‎tests/e2e/api/draw.spec.js‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { test, expect } from '@playwright/test';
2+
3+
/**
4+
* E2E tests for Draw API endpoints.
5+
* Backend routes: API/Backend/Draw/routes/files.js
6+
*
7+
* Currently covers:
8+
* - POST /api/files/getfile (impacted by SQL injection fix in filesutils.js)
9+
*
10+
* Future tests can cover:
11+
* - POST /api/files/getfiles
12+
* - POST /api/files/gethistory
13+
* - POST /api/files/add, /edit, /remove, /merge, /split, /undo
14+
* - POST /api/files/publish, /clip
15+
*/
16+
17+
test.describe('Draw API', () => {
18+
19+
test.describe('POST /api/files/getfile', () => {
20+
21+
test('returns a valid response for a basic getfile request', async ({ request }) => {
22+
const response = await request.post('/api/files/getfile', {
23+
data: {
24+
id: 1,
25+
test: 'false',
26+
},
27+
});
28+
// Should not crash (no 500)
29+
expect(response.status()).toBeLessThan(500);
30+
const body = await response.json();
31+
expect(body).toHaveProperty('status');
32+
});
33+
34+
test('returns a valid response with temporal filter parameters', async ({ request }) => {
35+
// Exercises the timeProp SQL path that was fixed
36+
const response = await request.post('/api/files/getfile', {
37+
data: {
38+
id: 1,
39+
test: 'false',
40+
timeProp: 'sol',
41+
startTime: '2024-01-01T00:00:00Z',
42+
endTime: '2024-12-31T23:59:59Z',
43+
},
44+
});
45+
expect(response.status()).toBeLessThan(500);
46+
const body = await response.json();
47+
expect(body).toHaveProperty('status');
48+
});
49+
50+
test('handles malicious timeProp without server error', async ({ request }) => {
51+
// SQL injection attempt via timeProp — should be sanitized, not crash
52+
const response = await request.post('/api/files/getfile', {
53+
data: {
54+
id: 1,
55+
test: 'false',
56+
timeProp: "'; DROP TABLE user_features; --",
57+
startTime: '2024-01-01T00:00:00Z',
58+
endTime: '2024-12-31T23:59:59Z',
59+
},
60+
});
61+
expect(response.status()).toBeLessThan(500);
62+
});
63+
64+
});
65+
66+
});

0 commit comments

Comments
 (0)
⚡