Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions core/src/test/java/io/kestra/core/storages/NamespaceFileTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,16 @@ void shouldThrowOnPathTraversalWithDoubleDots() {
Assertions.assertThrows(IllegalArgumentException.class, () -> NamespaceFile.normalize(Path.of("foo/../../../bar")));
}

@Test
void shouldThrowOnForwardSlashPathTraversal() {
// GHSA-h7c7-3mfc-m7pj: on a Windows JVM Path.of("/x/../../../foo.txt").toString()
// produces "\x\..\..\..\foo.txt"; toLogicalPath() converts backslashes back to "/",
// so the guard must catch the forward-slash form regardless of host OS.
Assertions.assertThrows(IllegalArgumentException.class, () -> NamespaceFile.normalize(Path.of("/x/../../../escaped.txt")));
Assertions.assertThrows(IllegalArgumentException.class, () -> NamespaceFile.normalize(Path.of("x/../../../escaped.txt")));
Assertions.assertThrows(IllegalArgumentException.class, () -> NamespaceFile.normalize(Path.of("/x/y/../../..")));
}

@Test
void shouldAllowLegitimatePathsWithSingleDotAndMultiLevelDirs() {
assertThat(toLogicalPath(NamespaceFile.normalize(Path.of("/foo/./bar")))).isEqualTo("/foo/bar");
Expand Down
12 changes: 12 additions & 0 deletions core/src/test/java/io/kestra/core/utils/FileUtilsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ void isParentTraversal_false(String path) {
"kestra:///ns/flow/exec/abc%5C%2E%2E%5C%2E%2E%5Cetc%5Cpasswd",
// mixed separators
"kestra:///ns/flow/exec/abc/..%5C..%5Cetc%5Cpasswd",
// GHSA-h7c7-3mfc-m7pj: on a Windows JVM File.separator is '\', so the old guard
// checked for "..\" and "\.." and never matched forward-slash HTTP URI payloads.
// These must be rejected regardless of the host OS.
"kestra:///ns/flow/exec/x/../../../escaped.txt",
"kestra:///ns/flow/exec/x%2F..%2F..%2F..%2Fescaped.txt",
})
void isParentTraversal_true(String path) {
assertThat(FileUtils.isParentTraversal(URI.create(path))).isTrue();
Expand All @@ -59,6 +64,13 @@ void isParentTraversal_true(String path) {
"..\\" + "etc\\passwd",
"ns\\exec\\..\\.." + "\\etc\\passwd",
"ns/exec/abc\\..\\..\\.." + "\\etc\\passwd",
// GHSA-h7c7-3mfc-m7pj: forward-slash payloads that bypass the old Windows guard
// (which checked "..\\" and "\\.." from File.separator and never matched "/" paths).
// These represent the decoded form of HTTP query-param paths, as they arrive after
// URI.getPath() decodes percent-encoding.
"/x/../../../escaped.txt",
"x/../../../escaped.txt",
"/x/y/../../..",
})
void isParentTraversalString_true(String path) {
assertThat(FileUtils.isParentTraversal(path)).isTrue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,37 @@ void pathTraversalShouldBeRejected() throws IOException, URISyntaxException {
client.toBlocking().exchange(HttpRequest.PUT("/api/v1/main/namespaces/" + namespace + "/files?from=/foo/../../test.txt&to=/bar", null)));
}

@Test
void shouldRejectForwardSlashPathTraversalOnWriteAndDelete() throws IOException, URISyntaxException {
// GHSA-h7c7-3mfc-m7pj: the old guard used File.separator ('\' on Windows) to build
// its check strings, so forward-slash HTTP URI payloads like /x/../../../foo.txt were
// never matched on a Windows JVM, allowing arbitrary file write and delete.
// Verify that POST (write) and DELETE are both rejected with the fixed guard.
String namespace = TestsUtils.randomNamespace();
Namespace namespaceStorage = namespaceFactory.of(TENANT_ID, namespace, storageInterface);
namespaceStorage.putFile(Path.of("/safe.txt"), new ByteArrayInputStream("safe".getBytes()));

MultipartBody body = MultipartBody.builder()
.addPart("fileContent", "x.txt", "OWNED via path traversal".getBytes())
.build();

// Write primitive: POST with a forward-slash traversal path must be rejected
Assertions.assertThrows(HttpClientResponseException.class, () ->
client.toBlocking().exchange(
HttpRequest.POST("/api/v1/main/namespaces/" + namespace + "/files?path=/x/../../../escaped.txt", body)
.contentType(MediaType.MULTIPART_FORM_DATA_TYPE)
));

// Delete primitive: DELETE with a forward-slash traversal path must be rejected
Assertions.assertThrows(HttpClientResponseException.class, () ->
client.toBlocking().exchange(
HttpRequest.DELETE("/api/v1/main/namespaces/" + namespace + "/files?path=/x/../../../safe.txt", null)
));

// The legitimate file must be unaffected
assertThat(namespaceStorage.exists(Path.of("/safe.txt"))).isTrue();
}

private void assertForbiddenErrorThrown(Executable executable) {
HttpClientResponseException httpClientResponseException = Assertions.assertThrows(HttpClientResponseException.class, executable);
assertThat(httpClientResponseException.getMessage()).startsWith("Illegal argument: Forbidden path: ");
Expand Down
Loading