introduce safe characters per AWS guidelines

Author: smiklosovic

src/java/org/apache/cassandra/service/snapshot/SnapshotOptions.java

@@ -43,6 +43,12 @@ public class SnapshotOptions
 {
     public static final String SKIP_FLUSH = "skipFlush";
     public static final String TTL = "ttl";
+
+    // AWS S3 "Safe characters" for object keys:
+    //   0-9  a-z  A-Z  !  -  _  .  *  '  (  )
+    // See https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html#object-key-guidelines
+    // Hyphen is placed last in the character class, so it stays literal and never becomes a range operator.
+    private static final Pattern SAFE_SNAPSHOT_NAME = Pattern.compile("[a-zA-Z0-9!_.*'()-]+");
     public final SnapshotType type;
     public final String tag;
     public final DurationSpec.IntSecondsBound ttl;
@@ -223,12 +229,23 @@ private void validateTag(String tag)
                                                           FILENAME_LENGTH, resolvedSnapshotname.length(), resolvedSnapshotname));
             }
 
-            // "-" is important, we are already generating system snapshots with "-" in them so this has to stay
-            // \\w are just "words" consisting of [a-zA-Z0-9_]
-            if (!Pattern.compile("[\\w-]+").matcher(resolvedSnapshotname).matches())
+            // Allowed characters follow the AWS S3 "Safe characters" set documented at
+            // https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html#object-key-guidelines :
+            //   0-9  a-z  A-Z  !  -  _  .  *  '  (  )
+            // The path separator '/' is intentionally excluded,
+            // which is what blocks traversal attempts such as "../../mysnapshot"
+            if (!SAFE_SNAPSHOT_NAME.matcher(resolvedSnapshotname).matches())
             {
                 throw new IllegalArgumentException("Snapshot name contains illegal characters: " + resolvedSnapshotname);
             }
+
+            // Because '.' is part of the safe set, the names "." and ".." would pass the charset check
+            // but resolve to the snapshots/ directory itself or its parent (the live table directory)
+            // when used as a path component. Reject them explicitly.
+            if (resolvedSnapshotname.equals(".") || resolvedSnapshotname.equals(".."))
+            {
+                throw new IllegalArgumentException("Snapshot name '" + resolvedSnapshotname + "' is reserved");
+            }
         }
 
         private void validateTTL(boolean ephemeral, DurationSpec.IntSecondsBound ttl)
@@ -243,7 +260,6 @@ private void validateTTL(boolean ephemeral, DurationSpec.IntSecondsBound ttl)
             if (ephemeral && ttl != null)
                 throw new IllegalStateException(format("can not take ephemeral snapshot (%s) while ttl is specified too", tag));
         }
-
     }
 
     @Override

test/unit/org/apache/cassandra/service/snapshot/SnapshotOptionsTest.java

@@ -38,13 +38,26 @@ public class SnapshotOptionsTest
     @Test
     public void testSnapshotNameValidation()
     {
+        // Previously-allowed alphanumerics, '-' and '_' must still be accepted.
         validate("atag", USER);
         validate("a-tag", USER);
         validate("a_tag", USER);
         validate("a_tag" + Instant.now().toEpochMilli(), USER);
         validate("a_tag_1and_something2-more", USER);
         validate("a".repeat(255), USER);
 
+        // AWS S3 "Safe characters" newly accepted by the relaxed allowlist:
+        //   !  .  *  '  (  )
+        // See https://docs.aws.amazon.com/AmazonS3/latest/userguide/object-keys.html#object-key-guidelines
+        validate("snap.2026-05-20", USER);
+        validate("important!", USER);
+        validate("backup*", USER);
+        validate("o'snap", USER);
+        validate("snap(1)", USER);
+        validate("!._-*'()", USER);
+        // Dots embedded in a name are not traversal: with '/' excluded, "a..tag" is just a literal directory.
+        validate("a..tag", USER);
+
         assertThatThrownBy(() -> validate("a".repeat(256), USER))
         .isInstanceOf(IllegalArgumentException.class)
         .hasMessage("Snapshot name must not be more than 255 characters long for " +
@@ -55,13 +68,28 @@ public void testSnapshotNameValidation()
         .isInstanceOf(IllegalArgumentException.class)
         .hasMessageContaining("Snapshot name must not be more than 255 characters long");
 
+        // '/' is not in the S3-safe set; this is what kills traversal attempts like "../../mysnapshot".
         assertThatThrownBy(() -> validate('a' + File.pathSeparator() + "tag", USER))
         .isInstanceOf(IllegalArgumentException.class)
         .hasMessage("Snapshot name contains illegal characters: a/tag");
 
-        assertThatThrownBy(() -> validate("a..tag", USER))
+        // Other characters outside the S3-safe set must still be rejected.
+        assertThatThrownBy(() -> validate("a tag", USER))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Snapshot name contains illegal characters: a tag");
+        assertThatThrownBy(() -> validate("a:tag", USER))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Snapshot name contains illegal characters: a:tag");
+
+        // "." and ".." pass the charset check but resolve to the snapshots/ dir itself
+        // and its parent (the live table dir) respectively, so they must be rejected as reserved.
+        assertThatThrownBy(() -> validate(".", USER))
+        .isInstanceOf(IllegalArgumentException.class)
+        .hasMessage("Snapshot name '.' is reserved");
+
+        assertThatThrownBy(() -> validate("..", USER))
         .isInstanceOf(IllegalArgumentException.class)
-        .hasMessage("Snapshot name contains illegal characters: a..tag");
+        .hasMessage("Snapshot name '..' is reserved");
     }
 
     private void validate(String tag, SnapshotType type)