Allow common DDL commands in actions for H2 (#29826)

* Add safe DDL commands to the whitelist for actions

* Run 'GRANT ALTER ANY SCHEMA TO GUEST;' on sample database

* Grant permissions for test databases and add test for sample database

* Fix test

* Fix test

* Add test for adding/removing columns

src/metabase/driver/h2.clj

@@ -169,11 +169,23 @@
     (boolean
      ;; Command types are organized with all DDL commands listed first, so all ddl commands are before ALTER_SEQUENCE.
      ;; see https://github.com/h2database/h2database/blob/master/h2/src/main/org/h2/command/CommandInterface.java#L297
+     ;; This doesn't list all the possible commands, but it lists the most common and useful ones.
      (and (every? #{CommandInterface/INSERT
                     CommandInterface/MERGE
                     CommandInterface/TRUNCATE_TABLE
                     CommandInterface/UPDATE
                     CommandInterface/DELETE
+                    CommandInterface/CREATE_TABLE
+                    CommandInterface/DROP_TABLE
+                    CommandInterface/CREATE_SCHEMA
+                    CommandInterface/DROP_SCHEMA
+                    CommandInterface/ALTER_TABLE_RENAME
+                    CommandInterface/ALTER_TABLE_ADD_COLUMN
+                    CommandInterface/ALTER_TABLE_DROP_COLUMN
+                    CommandInterface/ALTER_TABLE_ALTER_COLUMN_CHANGE_TYPE
+                    CommandInterface/ALTER_TABLE_ALTER_COLUMN_NOT_NULL
+                    CommandInterface/ALTER_TABLE_ALTER_COLUMN_DROP_NOT_NULL
+                    CommandInterface/ALTER_TABLE_ALTER_COLUMN_RENAME
                     ;; Read-only commands might not make sense for actions, but they are allowed
                     CommandInterface/SELECT ; includes SHOW, TABLE, VALUES
                     CommandInterface/EXPLAIN

test/metabase/driver/h2_test.clj

@@ -167,7 +167,6 @@
                       "ORDER BY ATTEMPTS.DATE ASC")
                  (some-> (qp/compile query) :query pretty-sql))))))))
 
-
 (deftest check-action-commands-test
   (mt/test-driver :h2
     (are [query] (= true (#'h2/every-command-allowed-for-actions? (#'h2/classify-query (u/the-id (mt/db)) query)))
@@ -183,12 +182,15 @@
       "truncate table venues"
       "insert into venues values (1, 'Chicken Chow')"
       "merge into venues key(1) values (1, 'Chicken Chow')"
-      "merge into venues using (select 1 as id) as source on (venues.id = source.id) when matched then update set name = 'Chicken Chow';")
-
-    (are [query] (= false (#'h2/every-command-allowed-for-actions? (#'h2/classify-query (u/the-id (mt/db)) query)))
+      "merge into venues using (select 1 as id) as source on (venues.id = source.id) when matched then update set name = 'Chicken Chow';"
       "create table venues (id int, name varchar(255))"
-      "alter table venues add column address varchar(255)"
       "drop table venues"
+      "update venues set name = 'bill'"
+      "insert into venues (name) values ('bill')"
+      "create table venues"
+      "alter table venues add column address varchar(255)")
+
+    (are [query] (= false (#'h2/every-command-allowed-for-actions? (#'h2/classify-query (u/the-id (mt/db)) query)))
       "select * from venues; update venues set name = 'stomp';
        CREATE ALIAS EXEC AS 'String shellexec(String cmd) throws java.io.IOException {Runtime.getRuntime().exec(cmd);return \"y4tacker\";}';
        EXEC ('open -a Calculator.app')"

test/metabase/sample_data_test.clj

@@ -147,3 +147,34 @@
               (testing "after"
                 (is (= []
                        (rating)))))))))))
+
+(deftest ddl-sample-database-test
+  (testing "should be able to execute DDL statements on the Sample Database"
+    (mt/with-temp Database [db (sample-database-db true)]
+      (sync/sync-database! db)
+      (mt/with-db db
+        (let [conn-spec (sql-jdbc.conn/db->pooled-connection-spec (mt/db))
+              get-tables (fn [] (set (mapv :table_name (jdbc/query conn-spec "SHOW TABLES;"))))
+              show-columns-from (fn [table-name] (set (mapv :field (jdbc/query conn-spec (str "SHOW COLUMNS FROM " table-name ";")))))
+              get-schemas (fn [] (set (mapv :schema_name (jdbc/query conn-spec "SHOW SCHEMAS;"))))]
+          (testing "create schema"
+            (is (not (contains? (get-schemas) "NEW_SCHEMA")))
+            (jdbc/execute! conn-spec "CREATE SCHEMA NEW_SCHEMA;")
+            (is (contains? (set (get-schemas)) "NEW_SCHEMA")))
+          (testing "drop schema"
+            (jdbc/execute! conn-spec "DROP SCHEMA NEW_SCHEMA;")
+            (is (not (contains? (get-schemas) "NEW_SCHEMA"))))
+          (testing "create table"
+            (is (not (contains? (get-tables) "NEW_TABLE")))
+            (jdbc/execute! conn-spec "CREATE TABLE NEW_TABLE (id INTEGER);")
+            (is (contains? (get-tables) "NEW_TABLE"))
+          (testing "add column"
+            (is (not (contains? (show-columns-from "NEW_TABLE") "NEW_COLUMN")))
+            (jdbc/execute! conn-spec "ALTER TABLE NEW_TABLE ADD COLUMN NEW_COLUMN VARCHAR(255);")
+            (is (contains? (show-columns-from "NEW_TABLE") "NEW_COLUMN"))
+          (testing "remove column"
+            (jdbc/execute! conn-spec "ALTER TABLE NEW_TABLE DROP COLUMN NEW_COLUMN;")
+            (is (not (contains? (show-columns-from "NEW_TABLE") "NEW_COLUMN")))
+          (testing "drop table"
+              (jdbc/execute! conn-spec "DROP TABLE NEW_TABLE;")
+              (is (not (contains? (get-tables) "NEW_TABLE"))))))))))))

test/metabase/test/data/h2.clj

@@ -69,7 +69,8 @@
   (str
    ;; Create a non-admin account 'GUEST' which will be used from here on out
    "CREATE USER IF NOT EXISTS GUEST PASSWORD 'guest';\n"
-
+   ;; Grant permissions for DDL statements
+   "GRANT ALTER ANY SCHEMA TO GUEST;"
    ;; Set DB_CLOSE_DELAY here because only admins are allowed to do it, so we can't set it via the connection string.
    ;; Set it to to -1 (no automatic closing)
    "SET DB_CLOSE_DELAY -1;"))