From eae8eb873998f5b746875c110fc6d2339cb77dfc Mon Sep 17 00:00:00 2001 From: Haoyan Geng Date: Mon, 3 Aug 2026 22:47:29 +0000 Subject: [PATCH] [SPARK-58531][SQL][CONNECT] Make spark.sql.artifact.copyFromLocalToFs.allowDestLocal a static conf and remove the deprecated spark.connect.copyFromLocalToFs.allowDestLocal Completes the Spark 4.0 migration of the `copyFromLocalToFs` local-destination gate: `spark.sql.artifact.copyFromLocalToFs.allowDestLocal` becomes a static SQL conf and the deprecated `spark.connect.copyFromLocalToFs.allowDestLocal` is removed, leaving one conf for the gate. The gate controls whether a caller can write to an arbitrary path on the driver's local filesystem, so it is meant to be set by whoever starts the driver -- as its own doc says, "when starting spark driver". The deprecated Connect conf was a static conf and enforced that, but its replacement was declared as a runtime conf, so a session could set it and override the value the driver was started with. Declaring the replacement in StaticSQLConf restores that property. Co-authored-by: Isaac --- .../apache/spark/sql/internal/SQLConf.scala | 19 +------------------ .../spark/sql/internal/StaticSQLConf.scala | 12 ++++++++++++ .../spark/sql/connect/config/Connect.scala | 12 ------------ .../spark/sql/artifact/ArtifactManager.scala | 10 ++++------ .../sql/artifact/ArtifactManagerSuite.scala | 16 ++++++++++++++-- .../configs-without-binding-policy-exceptions | 1 - 6 files changed, 31 insertions(+), 39 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala index 8a5b54e5cbcdb..15addaf8dadef 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala @@ -7668,23 +7668,6 @@ object SQLConf { .booleanConf .createWithDefault(true) - // Deprecate "spark.connect.copyFromLocalToFs.allowDestLocal" in favor of this config. This is - // currently optional because we don't want to break existing users who are using the old config. - // If this config is set, then we override the deprecated config. - val ARTIFACT_COPY_FROM_LOCAL_TO_FS_ALLOW_DEST_LOCAL = - buildConf("spark.sql.artifact.copyFromLocalToFs.allowDestLocal") - .internal() - .doc(""" - |Allow `spark.copyFromLocalToFs` destination to be local file system - | path on spark driver node when - |`spark.sql.artifact.copyFromLocalToFs.allowDestLocal` is true. - |This will allow user to overwrite arbitrary file on spark - |driver node we should only enable it for testing purpose. - |""".stripMargin) - .version("4.0.0") - .booleanConf - .createOptional - val LEGACY_RETAIN_FRACTION_DIGITS_FIRST = buildConf("spark.sql.legacy.decimal.retainFractionDigitsOnTruncate") .internal() @@ -8239,7 +8222,7 @@ object SQLConf { DeprecatedConfig(ESCAPED_STRING_LITERALS.key, "4.0", "Use raw string literals with the `r` prefix instead. "), DeprecatedConfig("spark.connect.copyFromLocalToFs.allowDestLocal", "4.0", - s"Use '${ARTIFACT_COPY_FROM_LOCAL_TO_FS_ALLOW_DEST_LOCAL.key}' instead."), + s"Use '${StaticSQLConf.ARTIFACT_COPY_FROM_LOCAL_TO_FS_ALLOW_DEST_LOCAL.key}' instead."), DeprecatedConfig(ALLOW_ZERO_INDEX_IN_FORMAT_STRING.key, "4.0", "Increase indexes by 1 " + "in `strfmt` of the `format_string` function. Refer to the first argument by \"1$\"."), DeprecatedConfig(SHUFFLE_DEPENDENCY_FILE_CLEANUP_ENABLED.key, "4.1", diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/StaticSQLConf.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/StaticSQLConf.scala index 72e36250b068a..8b2cfc8a8d029 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/StaticSQLConf.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/internal/StaticSQLConf.scala @@ -338,6 +338,18 @@ object StaticSQLConf { .booleanConf .createWithDefault(true) + val ARTIFACT_COPY_FROM_LOCAL_TO_FS_ALLOW_DEST_LOCAL = + buildStaticConf("spark.sql.artifact.copyFromLocalToFs.allowDestLocal") + .internal() + .doc("Allow the `copyFromLocalToFs` destination to be a local file system path on the " + + "driver node. This lets the caller overwrite arbitrary files on the driver node, so it " + + "should only be enabled for testing purposes. This is a static conf: it can only be set " + + "when starting the driver, and not from a session.") + .version("4.3.0") + .withBindingPolicy(ConfigBindingPolicy.NOT_APPLICABLE) + .booleanConf + .createWithDefault(false) + val REFLECT_ALLOW_LIST = buildStaticConf("spark.sql.reflect.allowList") .doc("A comma-separated allow list of regular expressions matched against the canonical " + "static method name (in the form `class.method`, e.g. `java.util.UUID.randomUUID`) " + diff --git a/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/config/Connect.scala b/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/config/Connect.scala index f5a71a6674657..10531e063224d 100644 --- a/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/config/Connect.scala +++ b/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/config/Connect.scala @@ -22,7 +22,6 @@ import java.util.concurrent.TimeUnit import org.apache.spark.SparkEnv import org.apache.spark.network.util.ByteUnit import org.apache.spark.sql.connect.common.config.ConnectCommon -import org.apache.spark.sql.internal.SQLConf import org.apache.spark.sql.internal.SQLConf.buildConf object Connect { @@ -302,17 +301,6 @@ object Connect { .intConf .createWithDefault(200) - val CONNECT_COPY_FROM_LOCAL_TO_FS_ALLOW_DEST_LOCAL = - buildStaticConf("spark.connect.copyFromLocalToFs.allowDestLocal") - .internal() - .doc(s""" - |(Deprecated since Spark 4.0, please set - |'${SQLConf.ARTIFACT_COPY_FROM_LOCAL_TO_FS_ALLOW_DEST_LOCAL.key}' instead. - |""".stripMargin) - .version("3.5.0") - .booleanConf - .createWithDefault(false) - val CONNECT_UI_SESSION_LIMIT = buildStaticConf("spark.sql.connect.ui.retainedSessions") .doc("The number of client sessions kept in the Spark Connect UI history.") .version("3.5.0") diff --git a/sql/core/src/main/scala/org/apache/spark/sql/artifact/ArtifactManager.scala b/sql/core/src/main/scala/org/apache/spark/sql/artifact/ArtifactManager.scala index 804b5269c929c..9b22cf4d9dd6a 100644 --- a/sql/core/src/main/scala/org/apache/spark/sql/artifact/ArtifactManager.scala +++ b/sql/core/src/main/scala/org/apache/spark/sql/artifact/ArtifactManager.scala @@ -36,7 +36,7 @@ import org.apache.spark.internal.{Logging, LogKeys} import org.apache.spark.internal.config.{CONNECT_SCALA_UDF_STUB_PREFIXES, EXECUTOR_USER_CLASS_PATH_FIRST} import org.apache.spark.sql.Artifact import org.apache.spark.sql.classic.SparkSession -import org.apache.spark.sql.internal.SQLConf +import org.apache.spark.sql.internal.{SQLConf, StaticSQLConf} import org.apache.spark.sql.util.ArtifactUtils import org.apache.spark.storage.{BlockManager, CacheId, StorageLevel} import org.apache.spark.util.{ChildFirstURLClassLoader, StubClassLoader, Utils} @@ -504,10 +504,8 @@ class ArtifactManager(session: SparkSession) extends AutoCloseable with Logging val localPath = serverLocalStagingPath val fs = destFSPath.getFileSystem(hadoopConf) if (fs.isInstanceOf[LocalFileSystem]) { - val allowDestLocalConf = - session.sessionState.conf.getConf(SQLConf.ARTIFACT_COPY_FROM_LOCAL_TO_FS_ALLOW_DEST_LOCAL) - .getOrElse( - session.conf.get("spark.connect.copyFromLocalToFs.allowDestLocal").contains("true")) + val allowDestLocalConf = session.sessionState.conf.getConf( + StaticSQLConf.ARTIFACT_COPY_FROM_LOCAL_TO_FS_ALLOW_DEST_LOCAL) if (!allowDestLocalConf) { // To avoid security issue, by default, @@ -517,7 +515,7 @@ class ArtifactManager(session: SparkSession) extends AutoCloseable with Logging // We can temporarily allow the behavior by setting spark config // `spark.sql.artifact.copyFromLocalToFs.allowDestLocal` // to `true` when starting spark driver, we should only enable it for testing - // purpose. + // purpose. It is a static conf, so it cannot be set from a session. throw new SparkUnsupportedOperationException("_LEGACY_ERROR_TEMP_3161") } } diff --git a/sql/core/src/test/scala/org/apache/spark/sql/artifact/ArtifactManagerSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/artifact/ArtifactManagerSuite.scala index a1678335b7755..598229d9e58e8 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/artifact/ArtifactManagerSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/artifact/ArtifactManagerSuite.scala @@ -22,10 +22,10 @@ import java.nio.file.{Files, Path, Paths} import org.apache.spark.{SparkConf, SparkException, SparkRuntimeException} import org.apache.spark.metrics.source.CodegenMetrics -import org.apache.spark.sql.Artifact +import org.apache.spark.sql.{AnalysisException, Artifact} import org.apache.spark.sql.classic.SparkSession import org.apache.spark.sql.functions.col -import org.apache.spark.sql.internal.SQLConf +import org.apache.spark.sql.internal.{SQLConf, StaticSQLConf} import org.apache.spark.sql.test.SharedSparkSession import org.apache.spark.sql.types.DataTypes import org.apache.spark.storage.CacheId @@ -253,6 +253,18 @@ class ArtifactManagerSuite extends SharedSparkSession { assert(copiedClassFile.exists()) } + test("SPARK-58531: allowDestLocal cannot be set from a session") { + // The conf gates writes to a local filesystem destination on the driver, so it must stay a + // static conf: a session that could turn it on would be able to write to arbitrary paths on + // the driver. Guard against it being made session-settable again. + val key = StaticSQLConf.ARTIFACT_COPY_FROM_LOCAL_TO_FS_ALLOW_DEST_LOCAL.key + assert(SQLConf.isStaticConfigKey(key)) + checkError( + exception = intercept[AnalysisException](spark.conf.set(key, "true")), + condition = "CANNOT_MODIFY_STATIC_CONFIG", + parameters = Map("key" -> s""""$key"""")) + } + test("Removal of resources") { withTempPath { path => diff --git a/sql/hive/src/test/resources/conf/binding-policy-exceptions/configs-without-binding-policy-exceptions b/sql/hive/src/test/resources/conf/binding-policy-exceptions/configs-without-binding-policy-exceptions index 37598f184c021..69f1adf5f62a6 100644 --- a/sql/hive/src/test/resources/conf/binding-policy-exceptions/configs-without-binding-policy-exceptions +++ b/sql/hive/src/test/resources/conf/binding-policy-exceptions/configs-without-binding-policy-exceptions @@ -453,7 +453,6 @@ spark.sql.ansi.enabled spark.sql.ansi.enforceReservedKeywords spark.sql.ansi.relationPrecedence spark.sql.artifact.cacheStorageLevel -spark.sql.artifact.copyFromLocalToFs.allowDestLocal spark.sql.artifact.isolation.alwaysApplyClassloader spark.sql.artifact.isolation.enabled spark.sql.assumeAnsiFalseIfNotPersisted.enabled