[CELEBORN-2317] Validate applicationId to prevent worker path traversal

common/src/main/scala/org/apache/celeborn/common/util/Utils.scala

@@ -700,6 +700,17 @@ object Utils extends Logging {
     (appId, shuffleId)
   }
 
+  private val appIdPattern = "^[A-Za-z0-9_-]+$".r
+
+  def validateAppId(applicationId: String): Unit = {
+    if (applicationId == null || applicationId.isEmpty ||
+      appIdPattern.findFirstIn(applicationId).isEmpty) {
+      throw new IllegalArgumentException(
+        s"Invalid application id: '$applicationId'. " +
+          "Application id must be non-empty and match [A-Za-z0-9_-]+.")
+    }
+  }
+
   def splitPartitionLocationUniqueId(uniqueId: String): (Int, Int) = {
     val splits = uniqueId.split("-")
     val partitionId = splits.dropRight(1).mkString("-").toInt

common/src/test/scala/org/apache/celeborn/common/util/UtilsSuite.scala

@@ -289,4 +289,28 @@ class UtilsSuite extends CelebornFunSuite {
       celebornConf)
     assert(testInstance.isInstanceOf[DefaultIdentityProvider])
   }
+
+  test("validateAppId rejects path traversal and accepts valid ids") {
+    Seq(
+      "application_1234567890123_0001",
+      "local-1234567890123",
+      "app1",
+      "my-app-id",
+      "app_with_underscores").foreach { id =>
+      Utils.validateAppId(id)
+    }
+
+    Seq(
+      "../etc/passwd",
+      "app/../secret",
+      "app/id",
+      "app\\id",
+      "app id",
+      "",
+      null).foreach { id =>
+      intercept[IllegalArgumentException] {
+        Utils.validateAppId(id)
+      }
+    }
+  }
 }

worker/src/main/scala/org/apache/celeborn/service/deploy/worker/Controller.scala

@@ -115,6 +115,7 @@ private[deploy] class Controller(
           pushDataTimeout,
           partitionSplitEnabled,
           isSegmentGranularityVisible) =>
+      Utils.validateAppId(applicationId)
       checkAuth(context, applicationId)
       val shuffleKey = Utils.makeShuffleKey(applicationId, shuffleId)
       workerSource.sample(WorkerSource.RESERVE_SLOTS_TIME, shuffleKey) {
@@ -146,6 +147,7 @@ private[deploy] class Controller(
           mapAttempts,
           epoch,
           mockFailure) =>
+      Utils.validateAppId(applicationId)
       checkAuth(context, applicationId)
       val shuffleKey = Utils.makeShuffleKey(applicationId, shuffleId)
       logDebug(s"Received CommitFiles request, $shuffleKey, primary files" +
@@ -164,7 +166,9 @@ private[deploy] class Controller(
         s"$commitFilesTimeMs ms.")
 
     case DestroyWorkerSlots(shuffleKey, primaryLocations, replicaLocations, mockFailure) =>
-      checkAuth(context, Utils.splitShuffleKey(shuffleKey)._1)
+      val applicationId = Utils.splitShuffleKey(shuffleKey)._1
+      Utils.validateAppId(applicationId)
+      checkAuth(context, applicationId)
       handleDestroy(context, shuffleKey, primaryLocations, replicaLocations, mockFailure)
   }
 

worker/src/main/scala/org/apache/celeborn/service/deploy/worker/storage/StorageManager.scala

@@ -1209,8 +1209,15 @@ final private[worker] class StorageManager(conf: CelebornConf, workerSource: Abs
         val dir = dirs(getNextIndex % dirs.size)
         val mountPoint = DeviceInfo.getMountPoint(dir.getAbsolutePath, mountPoints)
         val shuffleDir = new File(dir, s"$appId/$shuffleId")
-        shuffleDir.mkdirs()
         val file = new File(shuffleDir, fileName)
+        // Defense in depth: ensure the resolved path stays under the working dir
+        // even if appId / shuffleId / fileName contained traversal characters.
+        val dirCanonical = dir.getCanonicalPath + File.separator
+        if (!file.getCanonicalPath.startsWith(dirCanonical)) {
+          throw new IOException(
+            s"Refusing to create shuffle file outside working dir: ${file.getCanonicalPath}")
+        }
+        shuffleDir.mkdirs()
         try {
           if (file.exists()) {
             throw new FileAlreadyExistsException(