This is an automated email from the ASF dual-hosted git repository. liyang-kylin pushed a commit to branch kylin5 in repository https://gitbox.apache.org/repos/asf/kylin.git
commit f848745a831de8d67846e35404935714fbde8e95 Author: Li Yang <[email protected]> AuthorDate: Fri Oct 2 10:15:41 2026 +0800 KYLIN-6105 improve OpsService backup file path check --- .../org/apache/kylin/rest/service/OpsService.java | 38 +++++++++++++++------- .../apache/kylin/rest/service/OpsServiceTest.java | 30 +++++++++++++++++ 2 files changed, 57 insertions(+), 11 deletions(-) diff --git a/src/ops-service/src/main/java/org/apache/kylin/rest/service/OpsService.java b/src/ops-service/src/main/java/org/apache/kylin/rest/service/OpsService.java index 662aebc5db..b1685b68b3 100644 --- a/src/ops-service/src/main/java/org/apache/kylin/rest/service/OpsService.java +++ b/src/ops-service/src/main/java/org/apache/kylin/rest/service/OpsService.java @@ -32,6 +32,7 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.stream.Collectors; +import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.fs.FSDataInputStream; import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileStatus; @@ -170,25 +171,40 @@ public class OpsService { Path path = new Path(pathStr); FileSystem fs = HadoopUtil.getWorkingFileSystem(); if (fs.exists(path)) { - if (!pathStr.startsWith(META_BACKUP_PATH)) { - return "can not delete path not in metadata backup dir " + META_BACKUP_PATH; - } - String rootPath = getMetaBackupStoreDir(project); - if (!pathStr.startsWith(rootPath) || pathStr.equals(rootPath)) { + Path rootPath = normalizePath(fs, new Path(getMetaBackupStoreDir(project))); + Path resolvedPath = normalizePath(fs, path); + if (!isStrictDescendant(resolvedPath, rootPath)) { return "can not delete path not in metadata backup dir " + rootPath; } - fs.delete(path, true); - log.info("Delete metadata backup {} succeed.", path.toUri()); - if (fs.listStatus(path.getParent()).length == 0) { - fs.delete(path.getParent(), true); - log.info("Delete project path {} for no metadata backup exist.", path.getParent().toUri()); + fs.delete(resolvedPath, true); + log.info("Delete metadata backup {} succeed.", resolvedPath.toUri()); + Path parentPath = resolvedPath.getParent(); + if (parentPath != null && isDescendantOrSelf(parentPath, rootPath) && fs.exists(parentPath) + && fs.listStatus(parentPath).length == 0) { + fs.delete(parentPath, true); + log.info("Delete project path {} for no metadata backup exist.", parentPath.toUri()); } - return path + " delete succeed."; + return resolvedPath + " delete succeed."; } } return pathStr + " path not exist"; } + private static boolean isStrictDescendant(Path candidate, Path root) { + return isDescendantOrSelf(candidate, root) && !candidate.equals(root); + } + + private static Path normalizePath(FileSystem fs, Path path) throws IOException { + Path normalized = fs.exists(path) ? fs.resolvePath(path) : path; + return new Path(normalized.toUri().normalize()).makeQualified(fs.getUri(), fs.getWorkingDirectory()); + } + + private static boolean isDescendantOrSelf(Path candidate, Path root) { + return StringUtils.equals(candidate.toUri().getScheme(), root.toUri().getScheme()) + && StringUtils.equals(candidate.toUri().getAuthority(), root.toUri().getAuthority()) + && (candidate.equals(root) || candidate.toUri().getPath().startsWith(root.toUri().getPath() + PATH_SEP)); + } + public void cancelAndDeleteMetadataBackup(String rootPath, String project) throws IOException { MetadataBackup.cancelAndAsyncDeleteBackup(rootPath, project); } diff --git a/src/ops-service/src/test/java/org/apache/kylin/rest/service/OpsServiceTest.java b/src/ops-service/src/test/java/org/apache/kylin/rest/service/OpsServiceTest.java index ab0d6533ae..c325bf0f41 100644 --- a/src/ops-service/src/test/java/org/apache/kylin/rest/service/OpsServiceTest.java +++ b/src/ops-service/src/test/java/org/apache/kylin/rest/service/OpsServiceTest.java @@ -187,6 +187,36 @@ public class OpsServiceTest extends NLocalFileMetadataTestCase { .contains("path not exist" + " : " + "can not delete path not in metadata backup dir")); } + @Test + public void testDeleteBackupPathConfinement() throws IOException { + FileSystem fs = HadoopUtil.getWorkingFileSystem(); + String project = "default"; + String siblingProject = "default_extra"; + Path projectRoot = new Path(OpsService.getMetaBackupStoreDir(project)); + Path backup = new Path(projectRoot, "backup-id"); + Path sibling = new Path(OpsService.getMetaBackupStoreDir(siblingProject), "keep"); + fs.mkdirs(backup); + fs.mkdirs(sibling); + + try { + Assert.assertTrue(OpsService.deleteMetadataBackup(projectRoot.toString(), project) + .contains("can not delete path not in metadata backup dir")); + Assert.assertTrue(fs.exists(projectRoot)); + + String traversal = new Path(projectRoot, "backup-id/../../default_extra/keep").toString(); + Assert.assertTrue(OpsService.deleteMetadataBackup(traversal, project) + .contains("can not delete path not in metadata backup dir")); + Assert.assertTrue(fs.exists(sibling)); + + Assert.assertTrue( + OpsService.deleteMetadataBackup(backup.toString(), project).contains("delete succeed")); + Assert.assertFalse(fs.exists(backup)); + Assert.assertTrue(fs.exists(sibling)); + } finally { + fs.delete(new Path(OpsService.PROJECT_METADATA_BACKUP_PATH), true); + } + } + @Test public void testMetadataRestoreStatus() throws IOException { String backupPath = opsService.backupMetadata(UnitOfWork.GLOBAL_UNIT);
