From c2976cb3bb0e18bded978dff40e30980acc0346f Mon Sep 17 00:00:00 2001 From: xuxiong1 Date: Thu, 12 Feb 2026 16:04:42 -0800 Subject: [PATCH 1/3] Allow slash in snapshot file name validation Signed-off-by: xuxiong1 --- .../org/opensearch/core/common/Strings.java | 10 +++++++ .../opensearch/core/common/StringsTests.java | 26 +++++++++++++++++++ .../BlobStoreIndexShardSnapshot.java | 2 +- .../snapshots/blobstore/FileInfoTests.java | 25 ++++++++++++++++++ 4 files changed, 62 insertions(+), 1 deletion(-) diff --git a/libs/core/src/main/java/org/opensearch/core/common/Strings.java b/libs/core/src/main/java/org/opensearch/core/common/Strings.java index e8379e11ea26a..8277229b0b749 100644 --- a/libs/core/src/main/java/org/opensearch/core/common/Strings.java +++ b/libs/core/src/main/java/org/opensearch/core/common/Strings.java @@ -331,6 +331,16 @@ public static boolean validFileNameExcludingAstrix(String fileName) { return true; } + public static boolean validFileNameExcludingSlash(String fileName) { + for (int i = 0; i < fileName.length(); i++) { + char c = fileName.charAt(i); + if (c != '/' && c != '\\' && INVALID_FILENAME_CHARS.contains(c)) { + return false; + } + } + return true; + } + /** * Copy the given Collection into a String array. * The Collection must contain String elements only. diff --git a/libs/core/src/test/java/org/opensearch/core/common/StringsTests.java b/libs/core/src/test/java/org/opensearch/core/common/StringsTests.java index be7af18b106a8..079f2ecb52fc2 100644 --- a/libs/core/src/test/java/org/opensearch/core/common/StringsTests.java +++ b/libs/core/src/test/java/org/opensearch/core/common/StringsTests.java @@ -125,4 +125,30 @@ public void testIsDigits() { assertFalse(Strings.isDigits("123.4")); assertFalse(Strings.isDigits("123f")); } + + public void testValidFileNameExcludingSlash() { + // Valid cases - no invalid chars + assertTrue(Strings.validFileNameExcludingSlash("validfile")); + assertTrue(Strings.validFileNameExcludingSlash("file123")); + assertTrue(Strings.validFileNameExcludingSlash("file-name.txt")); + + // Valid cases - slashes are allowed + assertTrue(Strings.validFileNameExcludingSlash("path/to/file")); + assertTrue(Strings.validFileNameExcludingSlash("path\\to\\file")); + assertTrue(Strings.validFileNameExcludingSlash("/absolute/path")); + assertTrue(Strings.validFileNameExcludingSlash("mixed/path\\file")); + + // Invalid cases - other invalid chars should still fail + assertFalse(Strings.validFileNameExcludingSlash("file*name")); + assertFalse(Strings.validFileNameExcludingSlash("file?name")); + assertFalse(Strings.validFileNameExcludingSlash("file\"name")); + assertFalse(Strings.validFileNameExcludingSlash("filename")); + assertFalse(Strings.validFileNameExcludingSlash("file|name")); + assertFalse(Strings.validFileNameExcludingSlash("file name")); // space + assertFalse(Strings.validFileNameExcludingSlash("file,name")); // comma + + // Edge case - path with invalid char + assertFalse(Strings.validFileNameExcludingSlash("path/to/file*name")); + } } diff --git a/server/src/main/java/org/opensearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java b/server/src/main/java/org/opensearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java index 2b718fb055e14..6c8ad5500a2d3 100644 --- a/server/src/main/java/org/opensearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java +++ b/server/src/main/java/org/opensearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java @@ -340,7 +340,7 @@ public static FileInfo fromXContent(XContentParser parser) throws IOException { // Verify that file information is complete if (name == null || Strings.validFileName(name) == false) { throw new OpenSearchParseException("missing or invalid file name [" + name + "]"); - } else if (physicalName == null || Strings.validFileName(physicalName) == false) { + } else if (physicalName == null || Strings.validFileNameExcludingSlash(physicalName) == false) { throw new OpenSearchParseException("missing or invalid physical file name [" + physicalName + "]"); } else if (length < 0) { throw new OpenSearchParseException("missing or invalid file length"); diff --git a/server/src/test/java/org/opensearch/index/snapshots/blobstore/FileInfoTests.java b/server/src/test/java/org/opensearch/index/snapshots/blobstore/FileInfoTests.java index 3db9aeb9264a7..f431af0db2fb8 100644 --- a/server/src/test/java/org/opensearch/index/snapshots/blobstore/FileInfoTests.java +++ b/server/src/test/java/org/opensearch/index/snapshots/blobstore/FileInfoTests.java @@ -193,4 +193,29 @@ public void testGetPartSize() { assertEquals(numBytes, metadata.length()); } } + + public void testPhysicalNameWithSlash() throws IOException { + // Physical names with slashes should now be valid + String name = "foobar"; + String physicalName = "path/to/_foobar"; // contains slash + long length = 100; + + XContentBuilder builder = MediaTypeRegistry.contentBuilder(MediaTypeRegistry.JSON); + builder.startObject(); + builder.field(FileInfo.NAME, name); + builder.field(FileInfo.PHYSICAL_NAME, physicalName); + builder.field(FileInfo.LENGTH, length); + builder.field(FileInfo.WRITTEN_BY, Version.LATEST.toString()); + builder.field(FileInfo.CHECKSUM, "666"); + builder.endObject(); + byte[] xContent = BytesReference.toBytes(BytesReference.bytes(builder)); + + // Should parse successfully without throwing + final BlobStoreIndexShardSnapshot.FileInfo parsedInfo; + try (XContentParser parser = createParser(JsonXContent.jsonXContent, xContent)) { + parser.nextToken(); + parsedInfo = BlobStoreIndexShardSnapshot.FileInfo.fromXContent(parser); + } + assertThat(parsedInfo.physicalName(), equalTo(physicalName)); + } } From 25781d4e92fe3c3aa3e2860d6b799c39b95cc6ca Mon Sep 17 00:00:00 2001 From: xuxiong1 Date: Tue, 17 Feb 2026 13:03:04 -0800 Subject: [PATCH 2/3] correct method name Signed-off-by: xuxiong1 --- libs/core/src/main/java/org/opensearch/core/common/Strings.java | 2 +- .../org/opensearch/ingest/common/RemoveByPatternProcessor.java | 2 +- .../cluster/metadata/MetadataIndexTemplateService.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/libs/core/src/main/java/org/opensearch/core/common/Strings.java b/libs/core/src/main/java/org/opensearch/core/common/Strings.java index 8277229b0b749..4c760113b2d21 100644 --- a/libs/core/src/main/java/org/opensearch/core/common/Strings.java +++ b/libs/core/src/main/java/org/opensearch/core/common/Strings.java @@ -321,7 +321,7 @@ public static boolean validFileName(String fileName) { return true; } - public static boolean validFileNameExcludingAstrix(String fileName) { + public static boolean validFileNameExcludingAsterisk(String fileName) { for (int i = 0; i < fileName.length(); i++) { char c = fileName.charAt(i); if (c != '*' && INVALID_FILENAME_CHARS.contains(c)) { diff --git a/modules/ingest-common/src/main/java/org/opensearch/ingest/common/RemoveByPatternProcessor.java b/modules/ingest-common/src/main/java/org/opensearch/ingest/common/RemoveByPatternProcessor.java index da87f5201db72..7db2067ca9a7a 100644 --- a/modules/ingest-common/src/main/java/org/opensearch/ingest/common/RemoveByPatternProcessor.java +++ b/modules/ingest-common/src/main/java/org/opensearch/ingest/common/RemoveByPatternProcessor.java @@ -163,7 +163,7 @@ private void validateFieldPatterns(String processorTag, List patterns, S if (fieldPattern.startsWith("_")) { validationErrors.add(patternKey + " [" + fieldPattern + "] must not start with '_'"); } - if (Strings.validFileNameExcludingAstrix(fieldPattern) == false) { + if (Strings.validFileNameExcludingAsterisk(fieldPattern) == false) { validationErrors.add( patternKey + " [" + fieldPattern + "] must not contain the following characters " + Strings.INVALID_FILENAME_CHARS ); diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java index ae2a94ae8286c..cc15fd8b6f99b 100644 --- a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java +++ b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java @@ -1617,7 +1617,7 @@ private void validate(String name, @Nullable Settings settings, List ind if (indexPattern.startsWith("_")) { validationErrors.add("index_pattern [" + indexPattern + "] must not start with '_'"); } - if (Strings.validFileNameExcludingAstrix(indexPattern) == false) { + if (Strings.validFileNameExcludingAsterisk(indexPattern) == false) { validationErrors.add( "index_pattern [" + indexPattern + "] must not contain the following characters " + Strings.INVALID_FILENAME_CHARS ); From 8289c7c3fdfc871b021777e2bdd440ecadc7901f Mon Sep 17 00:00:00 2001 From: xuxiong1 Date: Fri, 20 Feb 2026 11:36:25 -0800 Subject: [PATCH 3/3] reject path traversal Signed-off-by: xuxiong1 --- .../org/opensearch/core/common/Strings.java | 11 ++++++++++ .../opensearch/core/common/StringsTests.java | 12 +++++++++-- .../snapshots/blobstore/FileInfoTests.java | 21 +++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/libs/core/src/main/java/org/opensearch/core/common/Strings.java b/libs/core/src/main/java/org/opensearch/core/common/Strings.java index 4c760113b2d21..06df6984a06e8 100644 --- a/libs/core/src/main/java/org/opensearch/core/common/Strings.java +++ b/libs/core/src/main/java/org/opensearch/core/common/Strings.java @@ -332,12 +332,23 @@ public static boolean validFileNameExcludingAsterisk(String fileName) { } public static boolean validFileNameExcludingSlash(String fileName) { + // Reject absolute paths + if (fileName.startsWith("/") || fileName.startsWith("\\")) { + return false; + } + // Reject invalid characters (excluding slashes) for (int i = 0; i < fileName.length(); i++) { char c = fileName.charAt(i); if (c != '/' && c != '\\' && INVALID_FILENAME_CHARS.contains(c)) { return false; } } + // Reject path traversal: ".." as a path segment + for (String segment : fileName.split("[/\\\\]")) { + if (segment.equals("..")) { + return false; + } + } return true; } diff --git a/libs/core/src/test/java/org/opensearch/core/common/StringsTests.java b/libs/core/src/test/java/org/opensearch/core/common/StringsTests.java index 079f2ecb52fc2..bbc6026baade5 100644 --- a/libs/core/src/test/java/org/opensearch/core/common/StringsTests.java +++ b/libs/core/src/test/java/org/opensearch/core/common/StringsTests.java @@ -132,12 +132,20 @@ public void testValidFileNameExcludingSlash() { assertTrue(Strings.validFileNameExcludingSlash("file123")); assertTrue(Strings.validFileNameExcludingSlash("file-name.txt")); - // Valid cases - slashes are allowed + // Valid cases - relative slashes are allowed assertTrue(Strings.validFileNameExcludingSlash("path/to/file")); assertTrue(Strings.validFileNameExcludingSlash("path\\to\\file")); - assertTrue(Strings.validFileNameExcludingSlash("/absolute/path")); assertTrue(Strings.validFileNameExcludingSlash("mixed/path\\file")); + // Invalid cases - absolute paths must be rejected + assertFalse(Strings.validFileNameExcludingSlash("/absolute/path")); + assertFalse(Strings.validFileNameExcludingSlash("\\absolute\\path")); + + // Invalid cases - path traversal must be rejected + assertFalse(Strings.validFileNameExcludingSlash("../etc/passwd")); + assertFalse(Strings.validFileNameExcludingSlash("path/../../etc/passwd")); + assertFalse(Strings.validFileNameExcludingSlash("path\\..\\..\\secret")); + // Invalid cases - other invalid chars should still fail assertFalse(Strings.validFileNameExcludingSlash("file*name")); assertFalse(Strings.validFileNameExcludingSlash("file?name")); diff --git a/server/src/test/java/org/opensearch/index/snapshots/blobstore/FileInfoTests.java b/server/src/test/java/org/opensearch/index/snapshots/blobstore/FileInfoTests.java index f431af0db2fb8..b576f4e03ca38 100644 --- a/server/src/test/java/org/opensearch/index/snapshots/blobstore/FileInfoTests.java +++ b/server/src/test/java/org/opensearch/index/snapshots/blobstore/FileInfoTests.java @@ -218,4 +218,25 @@ public void testPhysicalNameWithSlash() throws IOException { } assertThat(parsedInfo.physicalName(), equalTo(physicalName)); } + + public void testPhysicalNameWithPathTraversal() throws IOException { + // Path traversal and absolute paths in physical name should be rejected + String[] maliciousNames = { "../etc/passwd", "path/../../secret", "/absolute/path" }; + for (String malicious : maliciousNames) { + XContentBuilder builder = MediaTypeRegistry.contentBuilder(MediaTypeRegistry.JSON); + builder.startObject(); + builder.field(FileInfo.NAME, "segments_1"); + builder.field(FileInfo.PHYSICAL_NAME, malicious); + builder.field(FileInfo.LENGTH, 100); + builder.field(FileInfo.WRITTEN_BY, Version.LATEST.toString()); + builder.field(FileInfo.CHECKSUM, "666"); + builder.endObject(); + byte[] xContent = BytesReference.toBytes(BytesReference.bytes(builder)); + + try (XContentParser parser = createParser(JsonXContent.jsonXContent, xContent)) { + parser.nextToken(); + expectThrows(OpenSearchParseException.class, () -> FileInfo.fromXContent(parser)); + } + } + } }