[45] Prevent mismatches between engine and details (#152)

src/metabase/driver.clj

@@ -6,12 +6,14 @@
   SQL-based drivers can use the `:sql` driver as a parent, and JDBC-based SQL drivers can use `:sql-jdbc`. Both of
   these drivers define additional multimethods that child drivers should implement; see [[metabase.driver.sql]] and
   [[metabase.driver.sql-jdbc]] for more details."
-  (:require [clojure.string :as str]
+  (:require [clojure.set :as set]
+            [clojure.string :as str]
             [clojure.tools.logging :as log]
             [java-time :as t]
             [metabase.driver.impl :as driver.impl]
             [metabase.models.setting :as setting :refer [defsetting]]
             [metabase.plugins.classloader :as classloader]
+            [metabase.util :as u]
             [metabase.util.i18n :refer [deferred-tru trs tru]]
             [metabase.util.schema :as su]
             [potemkin :as p]
@@ -244,14 +246,28 @@
   [_]
   nil)
 
+(defn dispatch-on-initialized-driver-safe-keys
+  "Dispatch on initialized driver, except checks for `classname`,
+  `subprotocol`, `connection-uri` in the details map in order to
+  prevent a mismatch in spec type vs driver."
+  [driver details-map]
+  (let [invalid-keys #{"classname" "subprotocol" "connection-uri"}
+        ks           (->> details-map keys
+                          (map name)
+                          (map u/lower-case-en) set)]
+    (when (seq (set/intersection ks invalid-keys))
+      (throw (ex-info "Cannot specify subname, protocol, or connection-uri in details map"
+                      {:invalid-keys (set/intersection ks invalid-keys)})))
+    (dispatch-on-initialized-driver driver)))
+
 (defmulti can-connect?
   "Check whether we can connect to a `Database` with `details-map` and perform a simple query. For example, a SQL
   database might try running a query like `SELECT 1;`. This function should return truthy if a connection to the DB
   can be made successfully, otherwise it should return falsey or throw an appropriate Exception. Exceptions if a
   connection cannot be made. Throw an `ex-info` containing a truthy `::can-connect-message?` in `ex-data`
   in order to suppress logging expected driver validation messages during setup."
   {:arglists '([driver details])}
-  dispatch-on-initialized-driver
+  dispatch-on-initialized-driver-safe-keys
   :hierarchy #'hierarchy)
 
 (defmulti describe-database

src/metabase/driver/sql_jdbc/connection.clj

@@ -31,7 +31,7 @@
   WANT A CONNECTION SPEC FOR RUNNING QUERIES USE [[db->pooled-connection-spec]] INSTEAD WHICH WILL RETURN A *POOLED*
   CONNECTION SPEC."
   {:arglists '([driver details-map])}
-  driver/dispatch-on-initialized-driver
+  driver/dispatch-on-initialized-driver-safe-keys
   :hierarchy #'driver/hierarchy)
 
 

test/metabase/driver/h2_test.clj

@@ -70,7 +70,28 @@
       (is (instance? clojure.lang.ExceptionInfo result))
       (is (partial= {:cause "Malicious keys detected"
                      :data {:keys ["TRACE_LEVEL_SYSTEM_OUT"]}}
-                    (Throwable->map result))))))
+                    (Throwable->map result)))))
+  (testing "Reject connection details which lie about their driver"
+    (let [conn "mem:fake-h2-db"
+          f (fn f [details]
+              (try (driver/can-connect? :postgres details)
+                   ::did-not-throw
+                   (catch Exception e e)))]
+      (testing "connection-uri"
+        (let [result (f {:connection-uri conn})]
+          (is (= "Cannot specify subname, protocol, or connection-uri in details map"
+                 (ex-message result)))
+          (is (= {:invalid-keys #{"connection-uri"}} (ex-data result)))))
+      (testing "subprotocol"
+        (let [result (f {:db conn, :subprotocol "h2"})]
+          (is (= "Cannot specify subname, protocol, or connection-uri in details map"
+                 (ex-message result)))
+          (is (= {:invalid-keys #{"subprotocol"}} (ex-data result)))))
+      (testing "subprotocol"
+        (let [result (f {:db conn, :classname "org.h2.Driver"})]
+          (is (= "Cannot specify subname, protocol, or connection-uri in details map"
+                 (ex-message result)))
+          (is (= {:invalid-keys #{"classname"}} (ex-data result))))))))
 
 (deftest db-default-timezone-test
   (mt/test-driver :h2