CASSANDRA-21046 Schema annotations escape validation on CREATE and ALTER DDL statements

Patch by Jyothsna Konisa; Reviewed by Sam Tunnicliffe, Yifan Cai, Stefan Miklosovic for CASSANDRA-21046

Author: jyothsnakonisa

CHANGES.txt

@@ -1,4 +1,5 @@
 5.1
+ * Schema annotations escape validation on CREATE and ALTER DDL statements (CASSANDRA-21046)
  * Calculate once and cache the result of ModificationStatement#requiresRead as a perf optimization (CASSANDRA-21040)
  * Update system schema tables with new distributed keyspace on upgrade (CASSANDRA-20872)
  * Fix issue when running cms reconfiguration with paxos repair disabled (CASSANDRA-20869)

src/java/org/apache/cassandra/cql3/statements/schema/CopyTableStatement.java

@@ -234,6 +234,9 @@ public Keyspaces apply(ClusterMetadata metadata)
     public void validate(ClientState state)
     {
         super.validate(state);
+        // validate attributes to avoid silently accepting following statements
+        // create table ... like ... with security_label='xxx';
+        attrs.validate();
 
         // If a memtable configuration is specified, validate it against config
         if (attrs.hasOption(TableParams.Option.MEMTABLE))

src/java/org/apache/cassandra/schema/KeyspaceParams.java

@@ -59,9 +59,7 @@ public enum Option
     {
         DURABLE_WRITES,
         REPLICATION,
-        FAST_PATH,
-        COMMENT,
-        SECURITY_LABEL;
+        FAST_PATH;
 
         @Override
         public String toString()
@@ -181,8 +179,8 @@ public String toString()
                           .add(Option.DURABLE_WRITES.toString(), durableWrites)
                           .add(Option.REPLICATION.toString(), replication)
                           .add(Option.FAST_PATH.toString(), fastPath.toString())
-                          .add(Option.COMMENT.toString(), comment)
-                          .add(Option.SECURITY_LABEL.toString(), securityLabel)
+                          .add("COMMENT", comment)
+                          .add("SECURITY_LABEL", securityLabel)
                           .toString();
     }
 

src/java/org/apache/cassandra/schema/TableParams.java

@@ -70,7 +70,6 @@
 import static org.apache.cassandra.schema.TableParams.Option.MIN_INDEX_INTERVAL;
 import static org.apache.cassandra.schema.TableParams.Option.PENDING_DROP;
 import static org.apache.cassandra.schema.TableParams.Option.READ_REPAIR;
-import static org.apache.cassandra.schema.TableParams.Option.SECURITY_LABEL;
 import static org.apache.cassandra.schema.TableParams.Option.SPECULATIVE_RETRY;
 import static org.apache.cassandra.utils.LocalizeString.toLowerCaseLocalized;
 
@@ -102,8 +101,7 @@ public enum Option
         TRANSACTIONAL_MODE,
         TRANSACTIONAL_MIGRATION_FROM,
         PENDING_DROP,
-        AUTO_REPAIR,
-        SECURITY_LABEL;
+        AUTO_REPAIR;
 
         @Override
         public String toString()
@@ -342,7 +340,7 @@ public String toString()
     {
         return MoreObjects.toStringHelper(this)
                           .add(COMMENT.toString(), comment)
-                          .add(SECURITY_LABEL.toString(), securityLabel)
+                          .add("SECURITY_LABEL", securityLabel)
                           .add(ADDITIONAL_WRITE_POLICY.toString(), additionalWritePolicy)
                           .add(ALLOW_AUTO_SNAPSHOT.toString(), allowAutoSnapshot)
                           .add(BLOOM_FILTER_FP_CHANCE.toString(), bloomFilterFpChance)

test/unit/org/apache/cassandra/cql3/validation/miscellaneous/CommentAndSecurityLabelTest.java

@@ -373,6 +373,79 @@ public void testEmptyStringRejection()
         assertInvalidMessage("Cannot set security label to empty string", buildSecurityLabelStatement(ObjectType.FIELD, fieldRef, ""));
     }
 
+    @Test
+    public void testCommentAndSecurityLabelNotAllowedInCreateKeyspace()
+    {
+        // Test that comment property is rejected in CREATE KEYSPACE WITH clause
+        String createKsWithComment = "CREATE KEYSPACE ks_test WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'} AND comment = 'test comment'";
+        assertInvalidMessage("Unknown property 'comment'", createKsWithComment);
+
+        // Test that security_label property is rejected in CREATE KEYSPACE WITH clause
+        String createKsWithLabel = "CREATE KEYSPACE ks_test2 WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '1'} AND security_label = 'TEST_LABEL'";
+        assertInvalidMessage("Unknown property 'security_label'", createKsWithLabel);
+    }
+
+    @Test
+    public void testCommentAndSecurityLabelNotAllowedInAlterKeyspace()
+    {
+        createKeyspaceWithName("ks_alter_test");
+
+        // Test that comment property is rejected in ALTER KEYSPACE WITH clause
+        String alterKsWithComment = "ALTER KEYSPACE ks_alter_test WITH comment = 'test comment'";
+        assertInvalidMessage("Unknown property 'comment'", alterKsWithComment);
+
+        // Test that security_label property is rejected in ALTER KEYSPACE WITH clause
+        String alterKsWithLabel = "ALTER KEYSPACE ks_alter_test WITH security_label = 'TEST_LABEL'";
+        assertInvalidMessage("Unknown property 'security_label'", alterKsWithLabel);
+    }
+
+    @Test
+    public void testSecurityLabelNotAllowedInCreateTable()
+    {
+        createKeyspaceWithName("ks_table_test");
+
+        // Test that security_label property is rejected in CREATE TABLE WITH clause
+        String createTableWithLabel = "CREATE TABLE ks_table_test.t1 (id int PRIMARY KEY, name text) WITH security_label = 'TEST_LABEL'";
+        assertInvalidMessage("Unknown property 'security_label'", createTableWithLabel);
+
+        // Verify that comment IS allowed in CREATE TABLE for backward compatibility
+        String createTableWithComment = "CREATE TABLE ks_table_test.t2 (id int PRIMARY KEY, name text) WITH comment = 'test comment'";
+        execute(createTableWithComment);
+        assertComment(ObjectType.TABLE, "ks_table_test", "ks_table_test.t2", "test comment");
+    }
+
+    @Test
+    public void testSecurityLabelNotAllowedInAlterTable()
+    {
+        createKeyspaceWithName("ks_alter_table_test");
+        createTableWithName("ks_alter_table_test", "t1");
+
+        // Test that security_label property is rejected in ALTER TABLE WITH clause
+        String alterTableWithLabel = "ALTER TABLE ks_alter_table_test.t1 WITH security_label = 'TEST_LABEL'";
+        assertInvalidMessage("Unknown property 'security_label'", alterTableWithLabel);
+
+        // Verify that comment IS allowed in ALTER TABLE for backward compatibility
+        String alterTableWithComment = "ALTER TABLE ks_alter_table_test.t1 WITH comment = 'test comment'";
+        execute(alterTableWithComment);
+        assertComment(ObjectType.TABLE, "ks_alter_table_test", "ks_alter_table_test.t1", "test comment");
+    }
+
+    @Test
+    public void testSecurityLabelNotAllowedInCreateTableLike()
+    {
+        createKeyspaceWithName("ks_like_test");
+        createTableWithName("ks_like_test", "source_table");
+
+        // Test that security_label property is rejected in CREATE TABLE ... LIKE ... WITH clause
+        String createTableLikeWithLabel = "CREATE TABLE ks_like_test.target_table LIKE ks_like_test.source_table WITH security_label = 'TEST_LABEL'";
+        assertInvalidMessage("Unknown property 'security_label'", createTableLikeWithLabel);
+
+        // Verify that comment IS allowed in CREATE TABLE ... LIKE for backward compatibility
+        String createTableLikeWithComment = "CREATE TABLE ks_like_test.target_table2 LIKE ks_like_test.source_table WITH comment = 'test comment'";
+        execute(createTableLikeWithComment);
+        assertComment(ObjectType.TABLE, "ks_like_test", "ks_like_test.target_table2", "test comment");
+    }
+
     // Helper methods for setting comments and security labels
     private void setComment(ObjectType type, String objectName, String comment)
     {