From 6bbc9dd91121d6931256859e242170d30781a2d5 Mon Sep 17 00:00:00 2001 From: Ruben Quesada Lopez Date: Tue, 1 Sep 2026 12:06:29 +0100 Subject: [PATCH 1/2] [CALCITE-7760] Harden Spark engine activation: require opt-in via system property --- .../calcite/config/CalciteSystemProperty.java | 4 ++ .../apache/calcite/jdbc/CalcitePrepare.java | 12 +++- .../calcite/jdbc/SparkHandlerGateTest.java | 68 +++++++++++++++++++ site/_docs/history.md | 5 ++ site/_docs/security_threat_model.md | 8 +++ spark/build.gradle.kts | 4 ++ .../adapter/spark/SparkHandlerImpl.java | 16 +++-- 7 files changed, 112 insertions(+), 5 deletions(-) create mode 100644 core/src/test/java/org/apache/calcite/jdbc/SparkHandlerGateTest.java diff --git a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java index 7bae4774b0a0..9c0394374744 100644 --- a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java +++ b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java @@ -525,6 +525,10 @@ public final class CalciteSystemProperty { public static final CalciteSystemProperty MAX_DECIMAL_LITERAL_PLAIN_DIGITS = intProperty("calcite.parser.maxDecimalLiteralPlainDigits", 10_000, v -> v > 0); + /** Whether the Spark engine is enabled, making the {@code spark} connection property honored. */ + public static final CalciteSystemProperty ENABLE_SPARK_ENGINE = + booleanProperty("calcite.enable.spark", false); + private static CalciteSystemProperty booleanProperty(String key, boolean defaultValue) { // Note that "" -> true (convenient for command-lines flags like '-Dflag') diff --git a/core/src/main/java/org/apache/calcite/jdbc/CalcitePrepare.java b/core/src/main/java/org/apache/calcite/jdbc/CalcitePrepare.java index ec37fd4c7a39..26e267ab47c9 100644 --- a/core/src/main/java/org/apache/calcite/jdbc/CalcitePrepare.java +++ b/core/src/main/java/org/apache/calcite/jdbc/CalcitePrepare.java @@ -22,6 +22,7 @@ import org.apache.calcite.avatica.ColumnMetaData; import org.apache.calcite.avatica.Meta; import org.apache.calcite.config.CalciteConnectionConfig; +import org.apache.calcite.config.CalciteSystemProperty; import org.apache.calcite.linq4j.Enumerable; import org.apache.calcite.linq4j.EnumerableDefaults; import org.apache.calcite.linq4j.Queryable; @@ -167,8 +168,17 @@ private Dummy() {} /** Returns a spark handler. Returns a trivial handler, for which * {@link SparkHandler#enabled()} returns {@code false}, if {@code enable} * is {@code false} or if Spark is not on the class path. Never returns - * null. */ + * null. + * + *

If {@code enable=true} this method requires the + * {@link CalciteSystemProperty#ENABLE_SPARK_ENGINE} to be active, + * otherwise an exception will be thrown. */ public static synchronized SparkHandler getSparkHandler(boolean enable) { + if (enable && !CalciteSystemProperty.ENABLE_SPARK_ENGINE.value()) { + throw new SecurityException("The 'spark' property was set on a" + + " connection, but the Spark engine has not been enabled; set the JVM" + + " system property 'calcite.enable.spark' to 'true' to enable it"); + } if (sparkHandler == null) { sparkHandler = enable ? createHandler() : new TrivialSparkHandler(); } diff --git a/core/src/test/java/org/apache/calcite/jdbc/SparkHandlerGateTest.java b/core/src/test/java/org/apache/calcite/jdbc/SparkHandlerGateTest.java new file mode 100644 index 000000000000..fb3adbf61442 --- /dev/null +++ b/core/src/test/java/org/apache/calcite/jdbc/SparkHandlerGateTest.java @@ -0,0 +1,68 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.calcite.jdbc; + +import org.junit.jupiter.api.Test; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.Statement; + +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * Tests that the Spark engine requires the operator-level opt-in + * {@code calcite.enable.spark}. + * + *

These tests run without {@code -Dcalcite.enable.spark=true} (the + * default), so the gate is closed. The Spark module's own tests run with + * the opt-in set (see {@code spark/build.gradle.kts}). + */ +public class SparkHandlerGateTest { + @Test void testSparkHandlerRequiresOperatorOptIn() { + SecurityException e = + assertThrows(SecurityException.class, () -> + CalcitePrepare.Dummy.getSparkHandler(true)); + assertThat(e.getMessage(), containsString("calcite.enable.spark")); + } + + @Test void testSparkConnectionPropertyGatedBeforePrepare() + throws Exception { + try (Connection connection = + DriverManager.getConnection("jdbc:calcite:spark=true"); + Statement statement = connection.createStatement()) { + Throwable e = + assertThrows(Throwable.class, () -> + statement.executeQuery("values (1, 2, 3, 4, 5, 6)")); + while (e != null && !(e instanceof SecurityException)) { + e = e.getCause(); + } + assertThat("expected a SecurityException in the chain", e, + notNullValue()); + assertThat(e.getMessage(), containsString("calcite.enable.spark")); + } + } + + @Test void testTrivialHandlerStillAvailableWithoutOptIn() { + assertThat(CalcitePrepare.Dummy.getSparkHandler(false).enabled(), + is(false)); + } +} diff --git a/site/_docs/history.md b/site/_docs/history.md index d1e0caae3a71..c0130eb0a7e1 100644 --- a/site/_docs/history.md +++ b/site/_docs/history.md @@ -82,6 +82,11 @@ check the width of each group, and turned `1-2-3-4-5` into requires exactly 16 bytes; a longer value used to be truncated. Blanks are not trimmed. +* [CALCITE-7760] +The Spark engine now requires the operator-level opt-in system property +`-Dcalcite.enable.spark=true`. The `spark` connection property alone no longer +activates it; a connection using `spark=true` without the opt-in will fail. + #### New features {: #new-features-1-43-0} diff --git a/site/_docs/security_threat_model.md b/site/_docs/security_threat_model.md index 3ddcd89cfa45..acc22f367bba 100644 --- a/site/_docs/security_threat_model.md +++ b/site/_docs/security_threat_model.md @@ -120,6 +120,14 @@ carve-out below. A report that reaches a sink not covered here is a model gap must add it on purpose. * A file, CSV, or JSON adapter reading the local path it was configured with. Opt-in, by the same reasoning as the os-adapter. +* The Spark engine and its side effects: a local `JavaSparkContext`, a + local HTTP class server that serves compiled query classes, and setting + the `spark.repl.class.uri` JVM system property. The `spark` connection + property alone does not activate the engine; it additionally requires + the operator-set JVM system property `calcite.enable.spark=true`, a + capability a query author does not have (see + [Attacker and trust boundary](#attacker-and-trust-boundary)). Opt-in, + by the same reasoning as the os-adapter. * Reading the file or URL named by `model=`. The property names a resource the model handler reads on connection; letting an untrusted principal set connection properties authorises that read. diff --git a/spark/build.gradle.kts b/spark/build.gradle.kts index 4975686f455a..cda178b36bd0 100644 --- a/spark/build.gradle.kts +++ b/spark/build.gradle.kts @@ -45,6 +45,10 @@ dependencies { testRuntimeOnly("org.apache.logging.log4j:log4j-slf4j-impl") tasks.withType().configureEach { + // The Spark engine requires an operator-level opt-in (see + // CalciteSystemProperty.ENABLE_SPARK_ENGINE); these tests exercise + // the engine deliberately + systemProperty("calcite.enable.spark", "true") if (JavaVersion.current() >= JavaVersion.VERSION_17) { jvmArgs("--add-exports=java.base/sun.nio.ch=ALL-UNNAMED") } diff --git a/spark/src/main/java/org/apache/calcite/adapter/spark/SparkHandlerImpl.java b/spark/src/main/java/org/apache/calcite/adapter/spark/SparkHandlerImpl.java index 4cdb715ccc24..57c36db25288 100644 --- a/spark/src/main/java/org/apache/calcite/adapter/spark/SparkHandlerImpl.java +++ b/spark/src/main/java/org/apache/calcite/adapter/spark/SparkHandlerImpl.java @@ -31,9 +31,11 @@ import org.apache.spark.api.java.JavaSparkContext; import java.io.File; +import java.io.IOException; import java.io.Serializable; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; +import java.nio.file.Files; import java.util.Calendar; import java.util.concurrent.atomic.AtomicInteger; @@ -55,13 +57,19 @@ private static class Holder { private static final SparkHandlerImpl INSTANCE = new SparkHandlerImpl(); } - private static final File CLASS_DIR = new File("build/sparkServer/classes"); + private static final File CLASS_DIR = createClassDir(); + + private static File createClassDir() { + try { + return Files.createTempDirectory("calcite-spark-classes").toFile(); + } catch (IOException e) { + throw new IllegalStateException( + "Unable to create temporary folder for the Spark class server", e); + } + } /** Creates a SparkHandlerImpl. */ private SparkHandlerImpl() { - if (!CLASS_DIR.isDirectory() && !CLASS_DIR.mkdirs()) { - System.err.println("Unable to create temporary folder " + CLASS_DIR); - } classServer = new HttpServer(CLASS_DIR); // Start the classServer and store its URI in a spark system property From 7643e6e024f907487fa08501abc3db0cca429b5c Mon Sep 17 00:00:00 2001 From: Ruben Quesada Lopez Date: Wed, 2 Sep 2026 11:11:22 +0100 Subject: [PATCH 2/2] Adjust temporary directory permissions --- .../calcite/adapter/spark/SparkHandlerImpl.java | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/spark/src/main/java/org/apache/calcite/adapter/spark/SparkHandlerImpl.java b/spark/src/main/java/org/apache/calcite/adapter/spark/SparkHandlerImpl.java index 57c36db25288..9d6b3a6a1f04 100644 --- a/spark/src/main/java/org/apache/calcite/adapter/spark/SparkHandlerImpl.java +++ b/spark/src/main/java/org/apache/calcite/adapter/spark/SparkHandlerImpl.java @@ -35,7 +35,10 @@ import java.io.Serializable; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; +import java.nio.file.FileSystems; import java.nio.file.Files; +import java.nio.file.attribute.FileAttribute; +import java.nio.file.attribute.PosixFilePermissions; import java.util.Calendar; import java.util.concurrent.atomic.AtomicInteger; @@ -61,7 +64,16 @@ private static class Holder { private static File createClassDir() { try { - return Files.createTempDirectory("calcite-spark-classes").toFile(); + // Explicitly create the directory owner-readable/writable only + // (on POSIX filesystems java.io.tmpdir, typically /tmp, is world-writable) + final FileAttribute[] attrs = + FileSystems.getDefault().supportedFileAttributeViews().contains("posix") + ? new FileAttribute[] { + PosixFilePermissions.asFileAttribute( + PosixFilePermissions.fromString("rwx------")) + } + : new FileAttribute[0]; + return Files.createTempDirectory("calcite-spark-classes", attrs).toFile(); } catch (IOException e) { throw new IllegalStateException( "Unable to create temporary folder for the Spark class server", e);