security(import): add isValidImportFilename guard and regression tests
Codifies the path-traversal constraint that was previously safe by accident (findFileRecursive's getFileName() strip) but had no explicit guard or test coverage. Fixes issue #530. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -320,6 +320,17 @@ public class MassImportService {
|
||||
return new ProcessResult(processed, skippedFiles);
|
||||
}
|
||||
|
||||
private boolean isValidImportFilename(String filename) {
|
||||
if (filename == null || filename.isBlank()) return false;
|
||||
if (filename.contains("/")) return false;
|
||||
if (filename.contains("\\")) return false;
|
||||
if (filename.contains("..")) return false;
|
||||
if (filename.equals(".")) return false;
|
||||
if (filename.contains("\0")) return false;
|
||||
if (Paths.get(filename).isAbsolute()) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
// package-private: Mockito spy in tests can override to inject IOException
|
||||
InputStream openFileStream(File file) throws IOException {
|
||||
return new FileInputStream(file);
|
||||
|
||||
@@ -438,6 +438,62 @@ class MassImportServiceTest {
|
||||
verify(documentService).findByOriginalFilename("doc002.pdf");
|
||||
}
|
||||
|
||||
// ─── isValidImportFilename — security regression — do not remove ─────────
|
||||
|
||||
@Test
|
||||
void isValidImportFilename_returnsFalse_whenFilenameIsNull() {
|
||||
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", (String) null);
|
||||
assertThat(result).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void isValidImportFilename_returnsFalse_whenFilenameIsBlank() {
|
||||
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", " ");
|
||||
assertThat(result).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void isValidImportFilename_returnsFalse_whenFilenameContainsForwardSlash() {
|
||||
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "etc/passwd");
|
||||
assertThat(result).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void isValidImportFilename_returnsFalse_whenFilenameContainsBackslash() {
|
||||
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "..\\etc\\passwd");
|
||||
assertThat(result).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void isValidImportFilename_returnsFalse_whenFilenameContainsDotDot() {
|
||||
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "doc..evil.pdf");
|
||||
assertThat(result).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void isValidImportFilename_returnsFalse_whenFilenameIsDotDot() {
|
||||
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "..");
|
||||
assertThat(result).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void isValidImportFilename_returnsFalse_whenFilenameIsAbsolutePath() {
|
||||
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "/etc/passwd");
|
||||
assertThat(result).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void isValidImportFilename_returnsFalse_whenFilenameContainsNullByte() {
|
||||
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "file\0.pdf");
|
||||
assertThat(result).isFalse();
|
||||
}
|
||||
|
||||
@Test
|
||||
void isValidImportFilename_returnsTrue_whenFilenameIsPlainBasename() {
|
||||
boolean result = ReflectionTestUtils.invokeMethod(service, "isValidImportFilename", "document.pdf");
|
||||
assertThat(result).isTrue();
|
||||
}
|
||||
|
||||
// ─── importSingleDocument — non-blank optional fields ────────────────────
|
||||
|
||||
@Test
|
||||
|
||||
Reference in New Issue
Block a user