Skip to content

Commit 9ee6e9b

Browse files
authored
[Improve] validate module name before file resolution (#4466)
* [Refactor] extract path containment helpers into PathUtils Move the isDescendantPath/isDirectChildPath checks from ApplicationServiceImpl (private) into a shared base/util PathUtils class so the same canonical-path containment logic can be reused elsewhere. Pure refactor, no behavior change. * [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.
1 parent 79b382b commit 9ee6e9b

7 files changed

Lines changed: 238 additions & 36 deletions

File tree

README.md

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,6 @@
3131
[![Total Downloads](https://img.shields.io/github/downloads/apache/streampark/total.svg?style=for-the-badge&label=downloads)](https://streampark.apache.org/download)
3232
[![Twitter Follow](https://img.shields.io/twitter/follow/ASFStreamPark?label=follow&logo=x&style=for-the-badge)](https://twitter.com/ASFStreamPark)
3333

34-
**[Website](https://streampark.apache.org)**  |  
35-
**[Official Documentation](https://streampark.apache.org/docs/get-started/intro)**  |  
36-
**[FAQ](https://github.com/apache/streampark/issues/507)**
3734

3835
![](https://streampark.apache.org/image/dashboard-preview.png)
3936

docker/README.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,3 @@ docker-compose up -d
2020

2121
http://localhost:10000
2222

23-
#### [more detail](https://streampark.apache.org/docs/get-started/docker-deployment)
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.streampark.console.base.util;
19+
20+
import java.io.File;
21+
import java.io.IOException;
22+
23+
/**
24+
* Filesystem path containment utilities.
25+
*
26+
* <p>All checks resolve the canonical (absolute, normalized, symlink-resolved) paths before
27+
* comparison, so the supplied name is always confined to the expected directory.
28+
*/
29+
public final class PathUtils {
30+
31+
private PathUtils() {}
32+
33+
/**
34+
* Returns {@code true} only when the canonical path of {@code child} is strictly located
35+
* somewhere <em>inside</em> {@code parent} (at any depth). A {@code child} that equals {@code
36+
* parent} is NOT considered a descendant.
37+
*
38+
* @param parent the allowed root directory
39+
* @param child the path to check
40+
* @return {@code true} if {@code child} is strictly under {@code parent}
41+
* @throws IOException if canonical path resolution fails
42+
*/
43+
public static boolean isDescendantPath(File parent, File child) throws IOException {
44+
String parentPath = parent.getCanonicalPath();
45+
String childPath = child.getCanonicalPath();
46+
return childPath.startsWith(parentPath.concat(File.separator));
47+
}
48+
49+
/**
50+
* Returns {@code true} only when the canonical parent directory of {@code child} is exactly
51+
* {@code parent}, i.e. {@code child} is a <em>direct</em> (first-level) entry of {@code parent}.
52+
* This is stricter than {@link #isDescendantPath(File, File)} and is used to confine a supplied
53+
* name (e.g. a module archive) to a single level under the expected directory.
54+
*
55+
* @param parent the allowed root directory
56+
* @param child the path to check
57+
* @return {@code true} if {@code child} sits directly under {@code parent}
58+
* @throws IOException if canonical path resolution fails
59+
*/
60+
public static boolean isDirectChildPath(File parent, File child) throws IOException {
61+
File childParent = child.getCanonicalFile().getParentFile();
62+
return childParent != null && parent.getCanonicalFile().equals(childParent.getCanonicalFile());
63+
}
64+
}

streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/controller/ApplicationController.java

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
package org.apache.streampark.console.core.controller;
1919

20-
import org.apache.streampark.common.util.Utils;
2120
import org.apache.streampark.common.util.YarnUtils;
2221
import org.apache.streampark.console.base.domain.RestRequest;
2322
import org.apache.streampark.console.base.domain.RestResponse;
@@ -43,7 +42,6 @@
4342
import org.springframework.web.bind.annotation.RestController;
4443
import org.springframework.web.multipart.MultipartFile;
4544

46-
import java.io.File;
4745
import java.io.IOException;
4846
import java.io.Serializable;
4947
import java.net.URI;
@@ -240,17 +238,6 @@ public RestResponse deleteBak(ApplicationBackUp backUp) throws InternalException
240238
return RestResponse.success(deleted);
241239
}
242240

243-
@PostMapping("checkjar")
244-
public RestResponse checkjar(String jar) {
245-
File file = new File(jar);
246-
try {
247-
Utils.checkJarFile(file.toURI().toURL());
248-
return RestResponse.success(true);
249-
} catch (IOException e) {
250-
return RestResponse.success(file).message(e.getLocalizedMessage());
251-
}
252-
}
253-
254241
@PostMapping("upload")
255242
@RequiresPermissions("app:create")
256243
public RestResponse upload(MultipartFile file) throws Exception {

streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/service/impl/ApplicationServiceImpl.java

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.apache.streampark.console.base.mybatis.pager.MybatisPager;
4343
import org.apache.streampark.console.base.util.CommonUtils;
4444
import org.apache.streampark.console.base.util.ObjectUtils;
45+
import org.apache.streampark.console.base.util.PathUtils;
4546
import org.apache.streampark.console.base.util.WebUtils;
4647
import org.apache.streampark.console.core.bean.AppControl;
4748
import org.apache.streampark.console.core.bean.MavenDependency;
@@ -1243,33 +1244,24 @@ File getReadableConfFile(Application application) throws IOException {
12431244
new File(StringUtils.removeEnd(moduleArchive.getAbsolutePath(), ".tar.gz"))
12441245
.getCanonicalFile();
12451246
ApiAlertException.throwIfFalse(
1246-
isDirectChildPath(projectDistHome, moduleArchive), "Invalid module.");
1247+
PathUtils.isDirectChildPath(projectDistHome, moduleArchive), "Invalid module.");
12471248
ApiAlertException.throwIfFalse(
1248-
isDirectChildPath(projectDistHome, moduleHome), "Invalid module.");
1249+
PathUtils.isDirectChildPath(projectDistHome, moduleHome), "Invalid module.");
12491250
ApiAlertException.throwIfFalse(moduleHome.isDirectory(), "Invalid module.");
12501251

12511252
File confHome = new File(moduleHome, "conf").getCanonicalFile();
1252-
ApiAlertException.throwIfFalse(isDescendantPath(moduleHome, confHome), "Invalid config.");
1253+
ApiAlertException.throwIfFalse(
1254+
PathUtils.isDescendantPath(moduleHome, confHome), "Invalid config.");
12531255
ApiAlertException.throwIfFalse(confHome.isDirectory(), "Invalid config.");
12541256

12551257
File configFile = new File(application.getConfig()).getCanonicalFile();
12561258

12571259
ApiAlertException.throwIfFalse(configFile.isFile(), "Invalid config.");
1258-
ApiAlertException.throwIfFalse(isDescendantPath(confHome, configFile), "Invalid config.");
1260+
ApiAlertException.throwIfFalse(
1261+
PathUtils.isDescendantPath(confHome, configFile), "Invalid config.");
12591262
return configFile;
12601263
}
12611264

1262-
private boolean isDescendantPath(File parent, File child) throws IOException {
1263-
String parentPath = parent.getCanonicalPath();
1264-
String childPath = child.getCanonicalPath();
1265-
return childPath.startsWith(parentPath.concat(File.separator));
1266-
}
1267-
1268-
private boolean isDirectChildPath(File parent, File child) throws IOException {
1269-
File childParent = child.getCanonicalFile().getParentFile();
1270-
return childParent != null && parent.getCanonicalFile().equals(childParent.getCanonicalFile());
1271-
}
1272-
12731265
@Override
12741266
public Application getApp(Application appParam) {
12751267
Application application = this.baseMapper.getApp(appParam);

streampark-console/streampark-console-service/src/main/java/org/apache/streampark/console/core/service/impl/ProjectServiceImpl.java

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.apache.streampark.console.base.util.GZipUtils;
3535
import org.apache.streampark.console.base.util.GitUtils;
3636
import org.apache.streampark.console.base.util.ObjectUtils;
37+
import org.apache.streampark.console.base.util.PathUtils;
3738
import org.apache.streampark.console.core.entity.Application;
3839
import org.apache.streampark.console.core.entity.Project;
3940
import org.apache.streampark.console.core.enums.BuildState;
@@ -45,6 +46,7 @@
4546
import org.apache.streampark.console.core.task.FlinkAppHttpWatcher;
4647
import org.apache.streampark.console.core.task.ProjectBuildTask;
4748

49+
import org.apache.commons.lang3.StringUtils;
4850
import org.apache.flink.configuration.MemorySize;
4951

5052
import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
@@ -270,9 +272,7 @@ public List<String> modules(Long id) {
270272
@Override
271273
public List<String> jars(Project project) {
272274
List<String> list = new ArrayList<>(0);
273-
ApiAlertException.throwIfNull(
274-
project.getModule(), "Project module can't be null, please check.");
275-
File apps = new File(project.getDistHome(), project.getModule());
275+
File apps = resolveModuleDir(project);
276276
for (File file : Objects.requireNonNull(apps.listFiles())) {
277277
if (file.getName().endsWith(".jar")) {
278278
list.add(file.getName());
@@ -283,6 +283,8 @@ public List<String> jars(Project project) {
283283

284284
@Override
285285
public String getAppConfPath(Long id, String module) {
286+
ApiAlertException.throwIfTrue(StringUtils.isBlank(module), "Invalid module.");
287+
ApiAlertException.throwIfTrue(StringUtils.containsAny(module, '/', '\\'), "Invalid module.");
286288
Project project = getById(id);
287289
File appHome = project.getDistHome();
288290
File[] files = appHome.listFiles();
@@ -365,8 +367,10 @@ public List<String> getAllTags(Project project) {
365367

366368
@Override
367369
public List<Map<String, Object>> listConf(Project project) {
370+
// Validate the module name BEFORE entering the try-block, so an invalid name is rejected
371+
// with ApiAlertException instead of being swallowed by the file-operation catch below.
372+
File file = resolveModuleDir(project);
368373
try {
369-
File file = new File(project.getDistHome(), project.getModule());
370374
File unzipFile = new File(file.getAbsolutePath().replaceAll(".tar.gz", ""));
371375
if (!unzipFile.exists()) {
372376
GZipUtils.decompress(file.getAbsolutePath(), file.getParentFile().getAbsolutePath());
@@ -385,6 +389,29 @@ public List<Map<String, Object>> listConf(Project project) {
385389
return null;
386390
}
387391

392+
/**
393+
* Resolves the module directory/file after validating that the module name is a single-level
394+
* entry located directly under the project distribution home.
395+
*
396+
* @param project the project whose module directory to resolve
397+
* @return the canonical module file
398+
*/
399+
private File resolveModuleDir(Project project) {
400+
ApiAlertException.throwIfTrue(
401+
StringUtils.isBlank(project.getModule()), "Project module can't be null, please check.");
402+
ApiAlertException.throwIfTrue(
403+
StringUtils.containsAny(project.getModule(), '/', '\\'), "Invalid module.");
404+
try {
405+
File projectDistHome = project.getDistHome().getCanonicalFile();
406+
File moduleDir = new File(projectDistHome, project.getModule()).getCanonicalFile();
407+
ApiAlertException.throwIfFalse(
408+
PathUtils.isDirectChildPath(projectDistHome, moduleDir), "Invalid module.");
409+
return moduleDir;
410+
} catch (IOException e) {
411+
throw new ApiAlertException("Invalid module.", e);
412+
}
413+
}
414+
388415
private void eachFile(File file, List<Map<String, Object>> list, Boolean isRoot) {
389416
if (file != null && file.exists() && file.listFiles() != null) {
390417
if (isRoot) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
package org.apache.streampark.console.core.service.impl;
19+
20+
import org.apache.streampark.console.base.exception.ApiAlertException;
21+
import org.apache.streampark.console.core.entity.Project;
22+
import org.apache.streampark.console.core.mapper.ProjectMapper;
23+
24+
import org.junit.jupiter.api.BeforeEach;
25+
import org.junit.jupiter.api.Test;
26+
import org.junit.jupiter.api.extension.ExtendWith;
27+
import org.junit.jupiter.api.io.TempDir;
28+
import org.mockito.Mock;
29+
import org.mockito.junit.jupiter.MockitoExtension;
30+
import org.springframework.test.util.ReflectionTestUtils;
31+
32+
import java.io.File;
33+
import java.nio.file.Files;
34+
import java.nio.file.Path;
35+
import java.util.List;
36+
37+
import static org.assertj.core.api.Assertions.assertThat;
38+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
39+
import static org.mockito.Mockito.when;
40+
41+
/**
42+
* Unit tests for {@link ProjectServiceImpl} module-name validation on the {@code jars}, {@code
43+
* listConf} and {@code getAppConfPath} methods.
44+
*/
45+
@ExtendWith(MockitoExtension.class)
46+
class ProjectServiceImplTest {
47+
48+
@Mock private ProjectMapper projectMapper;
49+
50+
private final ProjectServiceImpl projectService = new ProjectServiceImpl();
51+
52+
@TempDir private Path tempDir;
53+
54+
@BeforeEach
55+
void setUp() {
56+
// ServiceImpl#getById delegates to baseMapper.selectById(...).
57+
ReflectionTestUtils.setField(projectService, "baseMapper", projectMapper);
58+
}
59+
60+
@Test
61+
void jarsShouldListJarFilesUnderProjectModule() throws Exception {
62+
Path distHome = Files.createDirectory(tempDir.resolve("dist"));
63+
Path moduleDir = Files.createDirectory(distHome.resolve("app"));
64+
Files.createFile(moduleDir.resolve("foo.jar"));
65+
Files.createFile(moduleDir.resolve("README.txt"));
66+
67+
List<String> jars = projectService.jars(project(1L, distHome, "app"));
68+
69+
// Only direct children ending with .jar are returned.
70+
assertThat(jars).containsExactly("foo.jar");
71+
}
72+
73+
@Test
74+
void jarsShouldRejectRelativeModule() throws Exception {
75+
Files.createDirectories(tempDir.resolve("outside"));
76+
77+
assertThatThrownBy(() -> projectService.jars(project(1L, tempDir, "../outside")))
78+
.isInstanceOf(ApiAlertException.class)
79+
.hasMessage("Invalid module.");
80+
}
81+
82+
@Test
83+
void jarsShouldRejectModuleWithPathSeparator() throws Exception {
84+
assertThatThrownBy(() -> projectService.jars(project(1L, tempDir, "a/b")))
85+
.isInstanceOf(ApiAlertException.class)
86+
.hasMessage("Invalid module.");
87+
}
88+
89+
@Test
90+
void listConfShouldRejectRelativeModule() {
91+
assertThatThrownBy(() -> projectService.listConf(project(1L, tempDir, "../../etc")))
92+
.isInstanceOf(ApiAlertException.class)
93+
.hasMessage("Invalid module.");
94+
}
95+
96+
@Test
97+
void getAppConfPathShouldRejectModuleWithPathSeparator() {
98+
assertThatThrownBy(() -> projectService.getAppConfPath(1L, "../etc"))
99+
.isInstanceOf(ApiAlertException.class)
100+
.hasMessage("Invalid module.");
101+
}
102+
103+
@Test
104+
void getAppConfPathShouldRejectBlankModule() {
105+
assertThatThrownBy(() -> projectService.getAppConfPath(1L, " "))
106+
.isInstanceOf(ApiAlertException.class)
107+
.hasMessage("Invalid module.");
108+
}
109+
110+
@Test
111+
void getAppConfPathShouldResolveValidModule() throws Exception {
112+
Path distHome = Files.createDirectory(tempDir.resolve("dist"));
113+
Files.createDirectories(distHome.resolve("app"));
114+
when(projectMapper.selectById(1L)).thenReturn(project(1L, distHome, null));
115+
116+
String confPath = projectService.getAppConfPath(1L, "app");
117+
118+
assertThat(confPath).isEqualTo(distHome.resolve("app").toFile().getAbsolutePath());
119+
}
120+
121+
private Project project(Long id, Path distHome, String module) {
122+
return new Project() {
123+
{
124+
setId(id);
125+
if (module != null) {
126+
setModule(module);
127+
}
128+
}
129+
130+
@Override
131+
public File getDistHome() {
132+
return distHome.toFile();
133+
}
134+
};
135+
}
136+
}

0 commit comments

Comments
 (0)