fix(cubejs): fourth review of the confinement (MotherDuck and Databricks options)

services/cubejs/scripts/patchCubeYamlCompiler.mjs

@@ -115,6 +115,11 @@ const DUCKDB_CREATE_PATCHED = `        const safeExtensions = ${DUCKDB_SAFE_EXTE
             allow_community_extensions: 'false',
             allow_persistent_secrets: 'false',
             temp_directory: require('fs').mkdtempSync(require('path').join(require('os').tmpdir(), 'duckdb-spill-')),
+            // Every tenant's DuckDB shares the cube process and its disk:
+            // bounded unless the operator raises them.
+            memory_limit: process.env.CUBEJS_DB_DUCKDB_MEMORY_LIMIT || '1GB',
+            threads: process.env.CUBEJS_DB_DUCKDB_THREADS || '2',
+            max_temp_directory_size: process.env.CUBEJS_DB_DUCKDB_MAX_TEMP_DIRECTORY_SIZE || '4GB',
         };
         const instance = await node_api_1.DuckDBInstance.create(dbUrl, dbOptions);`;
 

services/cubejs/src/__tests__/cube17Regression.test.js

@@ -376,6 +376,9 @@ y`);
     }
     const driver = ServerCore.createDriver("duckdb", {});
     try {
+      const [limits] = await driver.query("SELECT current_setting('threads') AS t, current_setting('max_temp_directory_size') AS m", []);
+      assert.equal(Number(limits.t), 2, "bounded threads by default");
+      assert.match(String(limits.m), /GiB|GB/);
       await assert.rejects(driver.query("CALL enable_logging(storage='stdout')", []), /not allowed/);
     } finally {
       await driver.release();

services/cubejs/src/utils/__tests__/confineDbParams.test.js

@@ -20,6 +20,18 @@ describe("tenant datasource params are confined, not refused", () => {
     assert.deepEqual(await confineDbParams("duckdb", { duckdbS3UseCredentialChain: true, initSql: "x" }, ds), { initSql: "x" });
     assert.equal((await confineDbParams("duckdb", { databasePath: "md:my_db?motherduck_token=t" }, ds)).databasePath, "md:my_db?motherduck_token=t");
     await refused("duckdb", { databasePath: "md:my_db" }, /MotherDuck needs a token/);
+    // MotherDuck options would point the pod's requests anywhere, before the sandbox
+    for (const databasePath of [
+      "md:x?motherduck_token=t&motherduck_host=169.254.169.254/latest/meta-data?",
+      "md:x?motherduck_token=t&motherduck_port=9999",
+      "md:?motherduck_extension_download_url=http://10.0.0.1/",
+    ]) {
+      await refused("duckdb", { databasePath }, /options other than the token/);
+    }
+    await refused("duckdb", { databasePath: "md:x/../y?motherduck_token=t" }, /database names/);
+    await refused("duckdb", { motherDuckToken: "t&motherduck_host=10.0.0.1" }, /malformed MotherDuck token/);
+    await refused("duckdb", { databasePath: "md:x", motherDuckToken: "t&x=y" }, /malformed MotherDuck token/);
+    assert.equal((await confineDbParams("duckdb", { databasePath: "md:x", motherDuckToken: "eyJ.abc_-1" }, ds)).motherDuckToken, "eyJ.abc_-1");
     assert.equal((await confineDbParams("duckdb", { databasePath: "a.duckdb" }, ds)).databasePath, "/data/local-db/ds-1/a.duckdb");
     await refused("duckdb", { databasePath: "/etc/passwd" }, /under \/data\/local-db\/ds-1/);
     await refused("duckdb", { databasePath: "../ds-2/b.duckdb" }, /under \/data\/local-db\/ds-1/);
@@ -69,6 +81,13 @@ describe("tenant datasource params are confined, not refused", () => {
     // the Databricks driver class through the generic type, by URL or property
     await refused("jdbc", { url: `${url};LogLevel=6`, drivername: "com.databricks.client.jdbc.Driver" }, /logging/);
     await refused("jdbc", { url, properties: { LogPath: "/tmp/x" } }, /logging/);
+    // options that read or write pod files, or reach other hosts, whichever type loads the driver
+    for (const option of ["VolumeOperationAllowedLocalPaths=/", "StagingAllowedLocalPaths=/", "SSLKeyStore=/app/k.jks", "UseProxy=1;ProxyHost=10.0.0.1"]) {
+      await refused("databricks-jdbc", { url: `${url};${option}`, token: "t" }, /options not allowed/);
+      await refused("jdbc", { url: `${url};${option}` }, /options not allowed/);
+    }
+    await refused("jdbc", { url, properties: { VolumeOperationAllowedLocalPaths: "/" } }, /options not allowed/);
+    assert.equal((await confineDbParams("databricks-jdbc", { url: `${url};AuthMech=3;ConnCatalog=main`, token: "t" }, ds)).url, `${url};AuthMech=3;ConnCatalog=main`);
   });
 
   it("cubestore: never ours, never private, websocket only, every plain connection pinned", async () => {

services/cubejs/src/utils/confineDbParams.js

@@ -72,15 +72,40 @@ const addresses = async (host) => {
   }
 };
 
+// MotherDuck connection strings take options (motherduck_host, download URLs,
+// ports) that point the pod's requests anywhere -- at instance creation,
+// before the sandbox applies -- and a token is pasted into one. Only a
+// database name and a token-shaped token.
+const MOTHERDUCK_TOKEN = /^[A-Za-z0-9._-]+$/;
+const confineMotherDuck = (file, token) => {
+  const [name, query = ""] = String(file).split(/\?(.*)/s);
+  if (!/^(md|motherduck):[A-Za-z0-9_.-]*$/i.test(name)) {
+    refuse("duckdb", "MotherDuck database names are letters, digits, '_', '.' and '-'");
+  }
+  let urlToken = null;
+  for (const pair of query ? query.split("&") : []) {
+    const [key, value = ""] = pair.split("=");
+    if (key.trim().toLowerCase() !== "motherduck_token") {
+      refuse("duckdb", "MotherDuck connection options other than the token are not allowed");
+    }
+    urlToken = decodeURIComponent(value);
+  }
+  for (const t of [urlToken, token]) {
+    if (t != null && !MOTHERDUCK_TOKEN.test(String(t))) refuse("duckdb", "malformed MotherDuck token");
+  }
+  // Without a token MotherDuck falls back to an interactive device login that
+  // blocks the request for a minute.
+  if (!urlToken && !token) refuse("duckdb", "MotherDuck needs a token");
+};
+
 const confineDuckDb = (params, { dataSourceId }) => {
   // The credential chain hands tenant SQL this pod's own AWS credentials.
   const { duckdbS3UseCredentialChain, ...rest } = params;
   const file = rest.databasePath;
   const motherDuck = /^(md|motherduck):/i.test(String(file ?? ""));
-  // Without a token MotherDuck falls back to an interactive device login that
-  // blocks the request for a minute.
-  if (motherDuck && !rest.motherDuckToken && !/motherduck_token=/i.test(file)) {
-    refuse("duckdb", "MotherDuck needs a token");
+  if (motherDuck) confineMotherDuck(file, rest.motherDuckToken);
+  else if (rest.motherDuckToken != null && !MOTHERDUCK_TOKEN.test(String(rest.motherDuckToken))) {
+    refuse("duckdb", "malformed MotherDuck token");
   }
   if (file && file !== ":memory:" && !motherDuck) {
     rest.databasePath = localFile("duckdb", file, dataSourceId);
@@ -122,6 +147,30 @@ const sharedJdbcClassPath = () => {
   return [...jarsIn(jdbcDriversDir()), ...databricks];
 };
 
+// The Databricks JDBC driver runs in our JVM, and its connection options
+// include ones that read or write pod files (Volume/Staging allowed local
+// paths, SSL key stores, log files) or reach other hosts (proxies). Only the
+// options a connection needs, in the URL and in the properties, whichever
+// JDBC-family type loads the driver.
+const DATABRICKS_OPTIONS = new Set([
+  "httppath", "authmech", "uid", "pwd", "ssl", "transportmode", "conncatalog",
+  "connschema", "usenativequery", "enablearrow", "auth_flow", "oauth2clientid",
+  "oauth2secret", "useragententry", "enablegeospatialsupport", "sockettimeout",
+  "rowsfetchedperblock",
+]);
+const refuseDatabricksOptions = (dbType, params) => {
+  const url = String(params.url ?? "");
+  if (!/^\s*jdbc:(databricks|spark):\/\//i.test(url) && dbType !== "databricks-jdbc") return;
+  const keys = [
+    ...url.split(";").slice(1).map((kv) => kv.split("=")[0].trim()).filter(Boolean),
+    ...Object.keys(params.properties ?? {}),
+  ];
+  const disallowed = keys.filter((key) => !DATABRICKS_OPTIONS.has(key.toLowerCase()));
+  if (disallowed.length) {
+    refuse(dbType, `Databricks connection options not allowed: ${disallowed.join(", ")}`);
+  }
+};
+
 // JDBC driver logging (LogPath, LogLevel, ... in the URL or properties) makes
 // the JVM-wide driver write files on this pod and trace every tenant's
 // connections. Any JDBC-family type can load the Databricks driver.
@@ -139,6 +188,7 @@ const confineJdbc = (params) => {
     refuse("jdbc", "in-process database URLs are not allowed");
   }
   refuseDriverLogging("jdbc", params);
+  refuseDatabricksOptions("jdbc", params);
   const dir = jdbcDriversDir();
   for (const entry of [params.customClassPath ?? []].flat()) {
     // One entry, one jar: a class-path separator would add others.
@@ -156,6 +206,7 @@ const confineDatabricks = (params) => {
     refuse("databricks-jdbc", "url must be a jdbc:databricks:// URL");
   }
   refuseDriverLogging("databricks-jdbc", params);
+  refuseDatabricksOptions("databricks-jdbc", params);
   return params;
 };