Skip to content

Commit d657aa9

Browse files
committed
fix: add path traversal validation for loadRecords taskId and add corresponding tests
1 parent 48ce30a commit d657aa9

2 files changed

Lines changed: 48 additions & 1 deletion

File tree

shenyu-admin/src/main/java/org/apache/shenyu/admin/record/LocalJsonFileHttpRecordRepository.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,15 @@ public List<RecordTaskInfo> listRecordTasks() {
108108

109109
@Override
110110
public List<ShenyuHttpRequestRecordDTO.Record> loadRecords(final String taskId) {
111-
Path filePath = Paths.get(properties.getStoragePath(), taskId + ".jsonl");
111+
if (StringUtils.isBlank(taskId) || !taskId.matches("^[a-zA-Z0-9_-]+$")) {
112+
LOG.warn("Invalid taskId: {}", taskId);
113+
return new ArrayList<>();
114+
}
115+
Path filePath = Paths.get(properties.getStoragePath(), taskId + ".jsonl").normalize();
116+
if (!filePath.startsWith(Paths.get(properties.getStoragePath()).normalize())) {
117+
LOG.warn("Path traversal detected: {}", taskId);
118+
return new ArrayList<>();
119+
}
112120
List<ShenyuHttpRequestRecordDTO.Record> records = new ArrayList<>();
113121
if (!Files.exists(filePath)) {
114122
LOG.warn("Record file not found: {}", filePath);

shenyu-admin/src/test/java/org/apache/shenyu/admin/record/LocalJsonFileHttpRecordRepositoryTest.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,45 @@ void testListRecordTasksWithMultipleFiles() throws IOException {
278278
assertEquals(2, taskMap.get("beta").getRecordCount());
279279
}
280280

281+
@Test
282+
void testLoadRecordsWithInvalidTaskIdPathTraversal() {
283+
List<ShenyuHttpRequestRecordDTO.Record> loaded = repository.loadRecords("../../etc/passwd");
284+
assertTrue(loaded.isEmpty());
285+
}
286+
287+
@Test
288+
void testLoadRecordsWithInvalidTaskIdNull() {
289+
List<ShenyuHttpRequestRecordDTO.Record> loaded = repository.loadRecords(null);
290+
assertTrue(loaded.isEmpty());
291+
}
292+
293+
@Test
294+
void testLoadRecordsWithInvalidTaskIdBlank() {
295+
List<ShenyuHttpRequestRecordDTO.Record> loaded = repository.loadRecords("");
296+
assertTrue(loaded.isEmpty());
297+
loaded = repository.loadRecords(" ");
298+
assertTrue(loaded.isEmpty());
299+
}
300+
301+
@Test
302+
void testLoadRecordsWithInvalidTaskIdSpecialChars() {
303+
assertTrue(repository.loadRecords("task/1").isEmpty());
304+
assertTrue(repository.loadRecords("task\\1").isEmpty());
305+
assertTrue(repository.loadRecords("task.1").isEmpty());
306+
assertTrue(repository.loadRecords("task id").isEmpty());
307+
assertTrue(repository.loadRecords("task!@#").isEmpty());
308+
}
309+
310+
@Test
311+
void testLoadRecordsWithValidTaskIdCharacters() throws IOException {
312+
ShenyuHttpRequestRecordDTO.Record record = buildRecord("task-1_abc", "trace-1", "GET", "/a", null, null, null);
313+
repository.save(buildDTO(Collections.singletonList(record)));
314+
315+
List<ShenyuHttpRequestRecordDTO.Record> loaded = repository.loadRecords("task-1_abc");
316+
assertEquals(1, loaded.size());
317+
assertEquals("task-1_abc", loaded.get(0).getTaskId());
318+
}
319+
281320
@Test
282321
void testLoadRecordsWithMultipleRecords() {
283322
ShenyuHttpRequestRecordDTO.Record r1 = buildRecord("task-1", "trace-1", "GET", "/a", null, null, null);

0 commit comments

Comments
 (0)