(streampark) 02/02: [Improve] validate module name before file resolution

[be...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

wolfboys pushed a commit to branch fix/validate-module-name
in repository https://gitbox.apache.org/repos/asf/streampark.git

commit 6b8d60f904c88ebc042bc1cb1051c84959d9dd34
Author: benjobs <be...@apache.org>
AuthorDate: Sat Aug 1 07:28:54 2026 +0800

    [Improve] validate module name before file resolution
    
    Validate the project module name in jars/listConf/getAppConfPath before
    resolving it against the project distribution home, so the resolved path
    is always a direct child of the expected directory. Also remove the
    unused checkjar endpoint that had no callers.
    
    Adds ProjectServiceImplTest covering module-name validation.
---
 .../core/controller/ApplicationController.java     |  13 --
 .../core/service/impl/ProjectServiceImpl.java      |  35 +++++-
 .../core/service/impl/ProjectServiceImplTest.java  | 136 +++++++++++++++++++++
 3 files changed, 167 insertions(+), 17 deletions(-)

diff --git a/streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/controller/ApplicationController.java b/streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/controller/ApplicationController.java
index 5a84d7300..9620ed329 100644
--- a/streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/controller/ApplicationController.java
+++ b/streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/controller/ApplicationController.java
@@ -17,7 +17,6 @@
 
 package org.apache.streampark.console.core.controller;
 
-import org.apache.streampark.common.util.Utils;
 import org.apache.streampark.common.util.YarnUtils;
 import org.apache.streampark.console.base.domain.RestRequest;
 import org.apache.streampark.console.base.domain.RestResponse;
@@ -43,7 +42,6 @@ import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RestController;
 import org.springframework.web.multipart.MultipartFile;
 
-import java.io.File;
 import java.io.IOException;
 import java.io.Serializable;
 import java.net.URI;
@@ -240,17 +238,6 @@ public class ApplicationController {
     return RestResponse.success(deleted);
   }
 
-  @PostMapping("checkjar")
-  public RestResponse checkjar(String jar) {
-    File file = new File(jar);
-    try {
-      Utils.checkJarFile(file.toURI().toURL());
-      return RestResponse.success(true);
-    } catch (IOException e) {
-      return RestResponse.success(file).message(e.getLocalizedMessage());
-    }
-  }
-
   @PostMapping("upload")
   @RequiresPermissions("app:create")
   public RestResponse upload(MultipartFile file) throws Exception {
diff --git a/streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/service/impl/ProjectServiceImpl.java b/streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/service/impl/ProjectServiceImpl.java
index ba72d4331..303966958 100644
--- a/streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/service/impl/ProjectServiceImpl.java
+++ b/streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/service/impl/ProjectServiceImpl.java
@@ -34,6 +34,7 @@ import org.apache.streampark.console.base.util.FileUtils;
 import org.apache.streampark.console.base.util.GZipUtils;
 import org.apache.streampark.console.base.util.GitUtils;
 import org.apache.streampark.console.base.util.ObjectUtils;
+import org.apache.streampark.console.base.util.PathUtils;
 import org.apache.streampark.console.core.entity.Application;
 import org.apache.streampark.console.core.entity.Project;
 import org.apache.streampark.console.core.enums.BuildState;
@@ -45,6 +46,7 @@ import org.apache.streampark.console.core.service.ProjectService;
 import org.apache.streampark.console.core.task.FlinkAppHttpWatcher;
 import org.apache.streampark.console.core.task.ProjectBuildTask;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.flink.configuration.MemorySize;
 
 import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
@@ -270,9 +272,7 @@ public class ProjectServiceImpl extends ServiceImpl<ProjectMapper, Project>
   @Override
   public List<String> jars(Project project) {
     List<String> list = new ArrayList<>(0);
-    ApiAlertException.throwIfNull(
-        project.getModule(), "Project module can't be null, please check.");
-    File apps = new File(project.getDistHome(), project.getModule());
+    File apps = resolveModuleDir(project);
     for (File file : Objects.requireNonNull(apps.listFiles())) {
       if (file.getName().endsWith(".jar")) {
         list.add(file.getName());
@@ -283,6 +283,8 @@ public class ProjectServiceImpl extends ServiceImpl<ProjectMapper, Project>
 
   @Override
   public String getAppConfPath(Long id, String module) {
+    ApiAlertException.throwIfTrue(StringUtils.isBlank(module), "Invalid module.");
+    ApiAlertException.throwIfTrue(StringUtils.containsAny(module, '/', '\\'), "Invalid module.");
     Project project = getById(id);
     File appHome = project.getDistHome();
     File[] files = appHome.listFiles();
@@ -365,8 +367,10 @@ public class ProjectServiceImpl extends ServiceImpl<ProjectMapper, Project>
 
   @Override
   public List<Map<String, Object>> listConf(Project project) {
+    // Validate the module name BEFORE entering the try-block, so an invalid name is rejected
+    // with ApiAlertException instead of being swallowed by the file-operation catch below.
+    File file = resolveModuleDir(project);
     try {
-      File file = new File(project.getDistHome(), project.getModule());
       File unzipFile = new File(file.getAbsolutePath().replaceAll(".tar.gz", ""));
       if (!unzipFile.exists()) {
         GZipUtils.decompress(file.getAbsolutePath(), file.getParentFile().getAbsolutePath());
@@ -385,6 +389,29 @@ public class ProjectServiceImpl extends ServiceImpl<ProjectMapper, Project>
     return null;
   }
 
+  /**
+   * Resolves the module directory/file after validating that the module name is a single-level
+   * entry located directly under the project distribution home.
+   *
+   * @param project the project whose module directory to resolve
+   * @return the canonical module file
+   */
+  private File resolveModuleDir(Project project) {
+    ApiAlertException.throwIfTrue(
+        StringUtils.isBlank(project.getModule()), "Project module can't be null, please check.");
+    ApiAlertException.throwIfTrue(
+        StringUtils.containsAny(project.getModule(), '/', '\\'), "Invalid module.");
+    try {
+      File projectDistHome = project.getDistHome().getCanonicalFile();
+      File moduleDir = new File(projectDistHome, project.getModule()).getCanonicalFile();
+      ApiAlertException.throwIfFalse(
+          PathUtils.isDirectChildPath(projectDistHome, moduleDir), "Invalid module.");
+      return moduleDir;
+    } catch (IOException e) {
+      throw new ApiAlertException("Invalid module.", e);
+    }
+  }
+
   private void eachFile(File file, List<Map<String, Object>> list, Boolean isRoot) {
     if (file != null && file.exists() && file.listFiles() != null) {
       if (isRoot) {
diff --git a/streampark-console/streampark-console-service/src/test/java/org/apache/streampark/console/core/service/impl/ProjectServiceImplTest.java b/streampark-console/streampark-console-service/src/test/java/org/apache/streampark/console/core/service/impl/ProjectServiceImplTest.java
new file mode 100644
index 000000000..c7f9dfb24
--- /dev/null
+++ b/streampark-console/streampark-console-service/src/test/java/org/apache/streampark/console/core/service/impl/ProjectServiceImplTest.java
@@ -0,0 +1,136 @@
+/*
+ * 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.streampark.console.core.service.impl;
+
+import org.apache.streampark.console.base.exception.ApiAlertException;
+import org.apache.streampark.console.core.entity.Project;
+import org.apache.streampark.console.core.mapper.ProjectMapper;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.junit.jupiter.api.io.TempDir;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.test.util.ReflectionTestUtils;
+
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+import static org.mockito.Mockito.when;
+
+/**
+ * Unit tests for {@link ProjectServiceImpl} module-name validation on the {@code jars}, {@code
+ * listConf} and {@code getAppConfPath} methods.
+ */
+@ExtendWith(MockitoExtension.class)
+class ProjectServiceImplTest {
+
+  @Mock private ProjectMapper projectMapper;
+
+  private final ProjectServiceImpl projectService = new ProjectServiceImpl();
+
+  @TempDir private Path tempDir;
+
+  @BeforeEach
+  void setUp() {
+    // ServiceImpl#getById delegates to baseMapper.selectById(...).
+    ReflectionTestUtils.setField(projectService, "baseMapper", projectMapper);
+  }
+
+  @Test
+  void jarsShouldListJarFilesUnderProjectModule() throws Exception {
+    Path distHome = Files.createDirectory(tempDir.resolve("dist"));
+    Path moduleDir = Files.createDirectory(distHome.resolve("app"));
+    Files.createFile(moduleDir.resolve("foo.jar"));
+    Files.createFile(moduleDir.resolve("README.txt"));
+
+    List<String> jars = projectService.jars(project(1L, distHome, "app"));
+
+    // Only direct children ending with .jar are returned.
+    assertThat(jars).containsExactly("foo.jar");
+  }
+
+  @Test
+  void jarsShouldRejectRelativeModule() throws Exception {
+    Files.createDirectories(tempDir.resolve("outside"));
+
+    assertThatThrownBy(() -> projectService.jars(project(1L, tempDir, "../outside")))
+        .isInstanceOf(ApiAlertException.class)
+        .hasMessage("Invalid module.");
+  }
+
+  @Test
+  void jarsShouldRejectModuleWithPathSeparator() throws Exception {
+    assertThatThrownBy(() -> projectService.jars(project(1L, tempDir, "a/b")))
+        .isInstanceOf(ApiAlertException.class)
+        .hasMessage("Invalid module.");
+  }
+
+  @Test
+  void listConfShouldRejectRelativeModule() {
+    assertThatThrownBy(() -> projectService.listConf(project(1L, tempDir, "../../etc")))
+        .isInstanceOf(ApiAlertException.class)
+        .hasMessage("Invalid module.");
+  }
+
+  @Test
+  void getAppConfPathShouldRejectModuleWithPathSeparator() {
+    assertThatThrownBy(() -> projectService.getAppConfPath(1L, "../etc"))
+        .isInstanceOf(ApiAlertException.class)
+        .hasMessage("Invalid module.");
+  }
+
+  @Test
+  void getAppConfPathShouldRejectBlankModule() {
+    assertThatThrownBy(() -> projectService.getAppConfPath(1L, " "))
+        .isInstanceOf(ApiAlertException.class)
+        .hasMessage("Invalid module.");
+  }
+
+  @Test
+  void getAppConfPathShouldResolveValidModule() throws Exception {
+    Path distHome = Files.createDirectory(tempDir.resolve("dist"));
+    Files.createDirectories(distHome.resolve("app"));
+    when(projectMapper.selectById(1L)).thenReturn(project(1L, distHome, null));
+
+    String confPath = projectService.getAppConfPath(1L, "app");
+
+    assertThat(confPath).isEqualTo(distHome.resolve("app").toFile().getAbsolutePath());
+  }
+
+  private Project project(Long id, Path distHome, String module) {
+    return new Project() {
+      {
+        setId(id);
+        if (module != null) {
+          setModule(module);
+        }
+      }
+
+      @Override
+      public File getDistHome() {
+        return distHome.toFile();
+      }
+    };
+  }
+}