This is an automated email from the ASF dual-hosted git repository.
dongjoon-hyun pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new f5302b6b81b3 [SPARK-57480][SQL] Stash allowSettingSystemProperties on
a static field to prevent `set:hiveconf` bypass
f5302b6b81b3 is described below
commit f5302b6b81b383eb098f404b118d7b206e56d809
Author: Anupam Yadav <[email protected]>
AuthorDate: Mon Jun 15 21:17:42 2026 -0700
[SPARK-57480][SQL] Stash allowSettingSystemProperties on a static field to
prevent `set:hiveconf` bypass
### What changes were proposed in this pull request?
This PR moves the
`spark.sql.legacy.hive.thriftServer.allowSettingSystemProperties` toggle out of
per-session `HiveConf` and onto a static field on `HiveSessionImpl`, populated
once at server init by `SparkSQLSessionManager`.
### Why are the changes needed?
SPARK-57441 added a default-deny gate on `set:system:*` in
`HiveSessionImpl.setVariable`, but read its toggle from
`ss.getConf().getBoolean(...)` -- the per-session `HiveConf`. The same
`setVariable` method writes to that same `HiveConf` from its `HIVECONF_PREFIX`
branch via `setConf` -> `verifyAndSet`, with no validation that rejects
`spark.*` keys (Hive's `getConfVars` returns null for non-Hive keys, the
`key.startsWith("hive.")` fallback does not fire, and `verifyAndSet` has no def
[...]
A JDBC URL of the form
```
jdbc:hive2://host:port/db?set:hiveconf:spark.sql.legacy.hive.thriftServer.allowSettingSystemProperties=true;set:system:foo=bar
```
flips the flag in the per-session `HiveConf` via the first directive, then
the second directive's gate read sees `true` and proceeds with
`System.setProperty`, bypassing the default-deny. See
[SPARK-57480](https://issues.apache.org/jira/browse/SPARK-57480) for the repro
and impact analysis.
The fix detaches the gate read from per-session state. The static field is
set once at `SparkSQLSessionManager.init` from the server-wide `StaticSQLConf`,
and is no longer reachable from any per-session `set:` overlay. The
`set:hiveconf:` write to the flag key still succeeds in writing into the
per-session `HiveConf` (preserved for backward compatibility), but it is now a
no-op for the gate.
### Does this PR introduce _any_ user-facing change?
No behavior change for users. The default-deny posture from SPARK-57441 is
preserved, and the legacy escape-hatch
(`spark.sql.legacy.hive.thriftServer.allowSettingSystemProperties=true` in the
server's static SQLConf) still works.
### How was this patch tested?
Added `HiveSessionImplSuite` regression test that exercises the exact
bypass:
1. Open a session with the legacy flag explicitly disabled.
2. Call `setVariable("hiveconf:" + flagKey, "true")` -- this is what
`configureSession` does for `set:hiveconf:` in a JDBC URL.
3. Call `setVariable("system:" + propKey, propValue)`.
4. Assert that step 3 returns `1` (rejected) and `System.getProperty` is
null.
I verified the regression test fails when the production fix is reverted
(gate read goes back to `ss.getConf().getBoolean(...)`): test reports `0 did
not equal 1 system:* should still be rejected, got rc=0`.
All four `HiveSessionImplSuite` tests pass with the fix in place.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.7
Closes #56528 from yadavay-amzn/SPARK-57480-thriftserver-system-prop-bypass.
Authored-by: Anupam Yadav <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 5eae1a39909b634200bfac2aa26ed92cd4fad77b)
Signed-off-by: Dongjoon Hyun <[email protected]>
---
.../hive/service/cli/session/HiveSessionImpl.java | 18 +++++++++++--
.../hive/thriftserver/SparkSQLSessionManager.scala | 10 +++----
.../hive/thriftserver/HiveSessionImplSuite.scala | 31 +++++++++++++++++++---
3 files changed, 48 insertions(+), 11 deletions(-)
diff --git
a/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
b/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
index 512c2716f8b7..fd14da589f1b 100644
---
a/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
+++
b/sql/hive-thriftserver/src/main/java/org/apache/hive/service/cli/session/HiveSessionImpl.java
@@ -234,6 +234,19 @@ public class HiveSessionImpl implements HiveSession {
}
}
+ // Resolved once at server init by SparkSQLSessionManager. See
setAllowSettingSystemProperties.
+ private static volatile boolean allowSettingSystemProperties = false;
+
+ /**
+ * Configure whether `set:system:*` is permitted. Intended to be called once
at server init
+ * from SparkSQLSessionManager. Reading and writing this value is
intentionally not routed
+ * through any per-session HiveConf, since that would let a low-privilege
client mutate it
+ * via `set:hiveconf:` from the same JDBC overlay this gate is meant to
guard.
+ */
+ public static void setAllowSettingSystemProperties(boolean allow) {
+ allowSettingSystemProperties = allow;
+ }
+
// Copy from org.apache.hadoop.hive.ql.processors.SetProcessor, only changes:
// 1. setConf(varname, propName, varvalue, true) when
varname.startsWith(HIVECONF_PREFIX)
// 2. system:* variables can not be set unless the legacy configuration
@@ -251,8 +264,9 @@ public class HiveSessionImpl implements HiveSession {
} else if (varname.startsWith(SYSTEM_PREFIX)){
// Setting JVM system properties is allowed only when the legacy
configuration
// spark.sql.legacy.hive.thriftServer.allowSettingSystemProperties is
enabled.
- if (ss.getConf().getBoolean(
- "spark.sql.legacy.hive.thriftServer.allowSettingSystemProperties",
false)) {
+ // The toggle is read from a static field set once at server init time,
NOT from the
+ // per-session HiveConf, to prevent bypass via `set:hiveconf:`.
+ if (allowSettingSystemProperties) {
String propName = varname.substring(SYSTEM_PREFIX.length());
System.getProperties().setProperty(propName,
substitution.substitute(ss.getConf(),varvalue));
} else {
diff --git
a/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLSessionManager.scala
b/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLSessionManager.scala
index 6a4b41b93547..f3a8db37e3df 100644
---
a/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLSessionManager.scala
+++
b/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLSessionManager.scala
@@ -21,7 +21,7 @@ import scala.util.control.NonFatal
import org.apache.hadoop.hive.conf.HiveConf
import org.apache.hive.service.cli.SessionHandle
-import org.apache.hive.service.cli.session.SessionManager
+import org.apache.hive.service.cli.session.{HiveSessionImpl, SessionManager}
import org.apache.hive.service.rpc.thrift.TProtocolVersion
import org.apache.hive.service.server.HiveServer2
@@ -40,10 +40,10 @@ private[hive] class SparkSQLSessionManager(hiveServer:
HiveServer2, sparkSession
override def init(hiveConf: HiveConf): Unit = {
setSuperField(this, "operationManager", sparkSqlOperationManager)
- // Propagate the legacy toggle into HiveConf so that
HiveSessionImpl.setVariable,
- // which runs during session open before the SparkSession is attached, can
read it.
- hiveConf.setBoolean(
-
StaticSQLConf.LEGACY_HIVE_THRIFT_SERVER_ALLOW_SETTING_SYSTEM_PROPERTIES.key,
+ // Stash the toggle on a static field instead of writing it into HiveConf.
Storing it on
+ // per-session HiveConf would let a JDBC client flip it from the same
`set:` overlay the
+ // system: gate is meant to guard (via `set:hiveconf:<key>=true`).
+ HiveSessionImpl.setAllowSettingSystemProperties(
sparkSession.sessionState.conf.getConf(
StaticSQLConf.LEGACY_HIVE_THRIFT_SERVER_ALLOW_SETTING_SYSTEM_PROPERTIES))
super.init(hiveConf)
diff --git
a/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveSessionImplSuite.scala
b/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveSessionImplSuite.scala
index 89dfe2a6c492..523ef898bb9c 100644
---
a/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveSessionImplSuite.scala
+++
b/sql/hive-thriftserver/src/test/scala/org/apache/spark/sql/hive/thriftserver/HiveSessionImplSuite.scala
@@ -66,10 +66,8 @@ class HiveSessionImplSuite extends SparkFunSuite {
}
private def withSystemPropSession[T](allowSettingSystemProperties:
Boolean)(f: => T): T = {
+
HiveSessionImpl.setAllowSettingSystemProperties(allowSettingSystemProperties)
val conf = new HiveConf()
- conf.setBoolean(
-
StaticSQLConf.LEGACY_HIVE_THRIFT_SERVER_ALLOW_SETTING_SYSTEM_PROPERTIES.key,
- allowSettingSystemProperties)
val sessionImpl = new HiveSessionImpl(
TProtocolVersion.HIVE_CLI_SERVICE_PROTOCOL_V1, "", "", conf, "")
sessionImpl.setSessionManager(new SessionManager(null))
@@ -77,7 +75,11 @@ class HiveSessionImplSuite extends SparkFunSuite {
sessionImpl.open(Map.empty[String, String].asJava)
// open() does not call SessionState.start(), so err is not initialized in
this unit test.
SessionState.get().err = new PrintStream(new ByteArrayOutputStream())
- try f finally sessionImpl.close()
+ try f finally {
+ sessionImpl.close()
+ // Restore default-deny so other tests don't inherit the relaxed state.
+ HiveSessionImpl.setAllowSettingSystemProperties(false)
+ }
}
test("SPARK-57441: system:* variables can not be set by default") {
@@ -105,6 +107,27 @@ class HiveSessionImplSuite extends SparkFunSuite {
System.clearProperty(key)
}
}
+
+ test("SPARK-57480: set:hiveconf cannot flip the legacy flag mid-session to
bypass " +
+ "the system:* gate") {
+ val key = "spark.test.HiveSessionImplSuite.bypassRegression"
+ val flagKey =
StaticSQLConf.LEGACY_HIVE_THRIFT_SERVER_ALLOW_SETTING_SYSTEM_PROPERTIES.key
+ try {
+ withSystemPropSession(allowSettingSystemProperties = false) {
+ // The system: gate must not read its toggle from the per-session
HiveConf, since
+ // `set:hiveconf:` directives write to that same HiveConf. Verify a
client cannot
+ // flip the flag mid-session via `set:hiveconf:<flag-key>=true` and
then bypass
+ // the gate. The hiveconf write itself is permitted (no-op for the
gate).
+ HiveSessionImpl.setVariable("hiveconf:" + flagKey, "true")
+ val rcSystem = HiveSessionImpl.setVariable("system:" + key, "value")
+ assert(rcSystem == 1, s"system:* should still be rejected, got
rc=$rcSystem")
+ assert(System.getProperty(key) == null,
+ s"system:* should not have been set, got ${System.getProperty(key)}")
+ }
+ } finally {
+ System.clearProperty(key)
+ }
+ }
}
class OperationManagerMock extends OperationManager {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
