This is an automated email from the ASF dual-hosted git repository.
dongjoon-hyun pushed a commit to branch branch-4.0
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.0 by this push:
new 5c2ab30d84a7 [SPARK-57510][CORE] Enforce the modify ACL on the
`/workers/kill` endpoint in MasterWebUI
5c2ab30d84a7 is described below
commit 5c2ab30d84a7c1e2794a064823b0851e9bf54461
Author: Dongjoon Hyun <[email protected]>
AuthorDate: Wed Jun 17 15:19:02 2026 -0700
[SPARK-57510][CORE] Enforce the modify ACL on the `/workers/kill` endpoint
in MasterWebUI
### What changes were proposed in this pull request?
This PR makes the Master Web UI's `/workers/kill` endpoint check the modify
ACL, like the other kill actions (`MasterPage`, `JobsTab`, `StagesTab`). For
example, Apache Spark Master UI ignores the request when ACL is denied.
https://github.com/apache/spark/blob/28c232f295edf06836667121ad859a730f8163d4/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala#L65-L67
This is a subtask of SPARK-55556 `Improve Web Security`.
### Why are the changes needed?
`/workers/kill` was the only destructive Master UI action that did not
honor the modify ACL; it was guarded only by
`spark.master.ui.decommission.allow.mode` (IP-based). So with ACLs enabled, a
user without the modify permission could still decommission workers.
### Does this PR introduce _any_ user-facing change?
Yes. With `spark.acls.enable=true`, a `/workers/kill` request from a user
not in `spark.modify.acls` is now ignored. No change with the default config.
### How was this patch tested?
Pass the CIs with newly added `MasterWebUIAclSuite`.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8)
Closes #56574 from dongjoon-hyun/SPARK-57510.
Authored-by: Dongjoon Hyun <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit 6b3a5695ca601ce7ed05b6f37c5db545e2f9a917)
Signed-off-by: Dongjoon Hyun <[email protected]>
---
.../spark/deploy/master/ui/MasterWebUI.scala | 1 +
.../deploy/master/ui/MasterWebUIAclSuite.scala | 103 +++++++++++++++++++++
.../spark/deploy/master/ui/MasterWebUISuite.scala | 4 +-
3 files changed, 107 insertions(+), 1 deletion(-)
diff --git
a/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterWebUI.scala
b/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterWebUI.scala
index 6d2fb0245547..86e342aad066 100644
--- a/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterWebUI.scala
+++ b/core/src/main/scala/org/apache/spark/deploy/master/ui/MasterWebUI.scala
@@ -72,6 +72,7 @@ class MasterWebUI(
if (decommissionEnabled) {
attachHandler(createServletHandler("/workers/kill", new HttpServlet {
override def doPost(req: HttpServletRequest, resp:
HttpServletResponse): Unit = {
+ if (!master.securityMgr.checkModifyPermissions(req.getRemoteUser))
return
val hostnames: Seq[String] = Option(req.getParameterValues("host"))
.getOrElse(Array[String]()).toImmutableArraySeq
if (!isDecommissioningRequestAllowed(req)) {
diff --git
a/core/src/test/scala/org/apache/spark/deploy/master/ui/MasterWebUIAclSuite.scala
b/core/src/test/scala/org/apache/spark/deploy/master/ui/MasterWebUIAclSuite.scala
new file mode 100644
index 000000000000..b3e773f075c7
--- /dev/null
+++
b/core/src/test/scala/org/apache/spark/deploy/master/ui/MasterWebUIAclSuite.scala
@@ -0,0 +1,103 @@
+/*
+ * 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.spark.deploy.master.ui
+
+import jakarta.servlet.{Filter, FilterChain, ServletRequest, ServletResponse}
+import jakarta.servlet.http.{HttpServletRequest, HttpServletRequestWrapper}
+import org.mockito.Mockito.{mock, never, verify, when}
+
+import org.apache.spark.{SecurityManager, SparkConf, SparkFunSuite}
+import org.apache.spark.deploy.DeployMessages.DecommissionWorkersOnHosts
+import org.apache.spark.deploy.master.Master
+import org.apache.spark.internal.config.DECOMMISSION_ENABLED
+import org.apache.spark.internal.config.UI._
+import org.apache.spark.rpc.{RpcEndpointRef, RpcEnv}
+import org.apache.spark.util.Utils
+
+/**
+ * Tests the modify ACL enforcement of the Master Web UI's `/workers/kill`
endpoint.
+ * A [[FakeAuthFilter]] injects the remote user so the modify permission can
be exercised.
+ */
+class MasterWebUIAclSuite extends SparkFunSuite {
+ import MasterWebUISuite._
+
+ val conf = new SparkConf()
+ .set(DECOMMISSION_ENABLED, true)
+ .set(UI_FILTERS, Seq(classOf[FakeAuthFilter].getName))
+ .set(ACLS_ENABLE, true)
+ .set(UI_VIEW_ACLS, Seq("*"))
+ .set(MODIFY_ACLS, Seq("alice"))
+ val securityMgr = new SecurityManager(conf)
+ val rpcEnv = mock(classOf[RpcEnv])
+ val master = mock(classOf[Master])
+ val masterEndpointRef = mock(classOf[RpcEndpointRef])
+ when(master.securityMgr).thenReturn(securityMgr)
+ when(master.conf).thenReturn(conf)
+ when(master.rpcEnv).thenReturn(rpcEnv)
+ when(master.self).thenReturn(masterEndpointRef)
+ val masterWebUI = new MasterWebUI(master, 0)
+
+ override def beforeAll(): Unit = {
+ super.beforeAll()
+ masterWebUI.bind()
+ }
+
+ override def afterAll(): Unit = {
+ try {
+ masterWebUI.stop()
+ } finally {
+ super.afterAll()
+ }
+ }
+
+ private def killWorkers(hostnames: Seq[String], user: String): Unit = {
+ val url =
s"http://${Utils.localHostNameForURI()}:${masterWebUI.boundPort}/workers/kill/"
+ val body = convPostDataToString(hostnames.map(("host", _)))
+ val headers = Seq(FakeAuthFilter.FAKE_HTTP_USER -> user)
+ val conn = sendHttpRequest(url, "POST", body, headers)
+ // The master is mocked here, so cannot assert on the response code.
+ conn.getResponseCode
+ }
+
+ test("Allow the worker kill request with the modify permission") {
+ val hostnames = Seq("allowed")
+ killWorkers(hostnames, "alice")
+
verify(masterEndpointRef).askSync[Integer](DecommissionWorkersOnHosts(hostnames))
+ }
+
+ test("Reject the worker kill request without the modify permission") {
+ val hostnames = Seq("denied")
+ killWorkers(hostnames, "nobody")
+ verify(masterEndpointRef,
never()).askSync[Integer](DecommissionWorkersOnHosts(hostnames))
+ }
+}
+
+/** Test filter that sets the remote user from the request's HTTP_USER header.
*/
+class FakeAuthFilter extends Filter {
+ override def doFilter(req: ServletRequest, res: ServletResponse, chain:
FilterChain): Unit = {
+ val hreq = req.asInstanceOf[HttpServletRequest]
+ val wrapped = new HttpServletRequestWrapper(hreq) {
+ override def getRemoteUser(): String =
hreq.getHeader(FakeAuthFilter.FAKE_HTTP_USER)
+ }
+ chain.doFilter(wrapped, res)
+ }
+}
+
+object FakeAuthFilter {
+ val FAKE_HTTP_USER = "HTTP_USER"
+}
diff --git
a/core/src/test/scala/org/apache/spark/deploy/master/ui/MasterWebUISuite.scala
b/core/src/test/scala/org/apache/spark/deploy/master/ui/MasterWebUISuite.scala
index 37d0c5ec692d..332e7bf9b5e0 100644
---
a/core/src/test/scala/org/apache/spark/deploy/master/ui/MasterWebUISuite.scala
+++
b/core/src/test/scala/org/apache/spark/deploy/master/ui/MasterWebUISuite.scala
@@ -146,9 +146,11 @@ object MasterWebUISuite {
private[ui] def sendHttpRequest(
url: String,
method: String,
- body: String = ""): HttpURLConnection = {
+ body: String = "",
+ headers: Seq[(String, String)] = Nil): HttpURLConnection = {
val conn = new
URI(url).toURL.openConnection().asInstanceOf[HttpURLConnection]
conn.setRequestMethod(method)
+ headers.foreach { case (k, v) => conn.setRequestProperty(k, v) }
if (body.nonEmpty) {
conn.setDoOutput(true)
conn.setRequestProperty("Content-Type",
"application/x-www-form-urlencoded")
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
