From 441d218ad667cdc0504e5472ca66a4e8cc26bcb7 Mon Sep 17 00:00:00 2001 From: exceptionfactory Date: Wed, 9 Sep 2026 12:28:11 -0500 Subject: [PATCH] NIFI-16329 Added Coordinate Validation for Registry NAR Bundles --- .../extract/nar/NarBundleExtractor.java | 34 +++------- .../bundle/model/BundleIdentifier.java | 8 +-- .../registry/bundle/util/BundleUtils.java | 12 ++++ .../extract/nar/TestNarBundleExtractor.java | 28 +++++++- .../bundle/model/TestBundleIdentifier.java | 41 ++++++++++++ .../registry/bundle/util/TestBundleUtils.java | 43 ++++++++++++ .../FileSystemBundlePersistenceProvider.java | 25 +++---- .../extension/StandardBundleCoordinate.java | 4 ++ .../StandardBundleVersionCoordinate.java | 5 ++ .../FileSystemFlowPersistenceProvider.java | 24 ++----- .../flow/git/GitFlowPersistenceProvider.java | 21 ++++-- .../registry/service/RegistryService.java | 24 +++++-- .../extension/StandardExtensionService.java | 35 +++++++--- ...stFileSystemBundlePersistenceProvider.java | 57 +++++++++++++++- .../TestStandardBundleCoordinate.java | 57 ++++++++++++++++ .../TestStandardBundleVersionCoordinate.java | 67 +++++++++++++++++++ ...TestFileSystemFlowPersistenceProvider.java | 12 ++++ .../git/TestGitFlowPersistenceProvider.java | 29 ++++++++ .../registry/service/TestRegistryService.java | 27 ++++++++ .../apache/nifi/registry/util/FileUtils.java | 27 ++++++++ .../nifi/registry/util/TestFileUtils.java | 30 +++++++++ .../aws/S3BundlePersistenceProvider.java | 7 +- 22 files changed, 533 insertions(+), 84 deletions(-) create mode 100644 nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/test/java/org/apache/nifi/registry/bundle/model/TestBundleIdentifier.java create mode 100644 nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/test/java/org/apache/nifi/registry/bundle/util/TestBundleUtils.java create mode 100644 nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/extension/TestStandardBundleCoordinate.java create mode 100644 nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/extension/TestStandardBundleVersionCoordinate.java diff --git a/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/main/java/org/apache/nifi/registry/bundle/extract/nar/NarBundleExtractor.java b/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/main/java/org/apache/nifi/registry/bundle/extract/nar/NarBundleExtractor.java index c06030d56af5..72fd3219bbeb 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/main/java/org/apache/nifi/registry/bundle/extract/nar/NarBundleExtractor.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/main/java/org/apache/nifi/registry/bundle/extract/nar/NarBundleExtractor.java @@ -134,33 +134,21 @@ public BundleDetails extract(final InputStream inputStream) throws IOException { } private BundleIdentifier getBundleCoordinate(final Attributes attributes) { - try { - final String groupId = attributes.getValue(NarManifestEntry.NAR_GROUP.getManifestName()); - final String artifactId = attributes.getValue(NarManifestEntry.NAR_ID.getManifestName()); - final String version = attributes.getValue(NarManifestEntry.NAR_VERSION.getManifestName()); - - return new BundleIdentifier(groupId, artifactId, version); - } catch (Exception e) { - throw new BundleException("Unable to obtain bundle coordinate due to: " + e.getMessage(), e); - } + final String groupId = attributes.getValue(NarManifestEntry.NAR_GROUP.getManifestName()); + final String artifactId = attributes.getValue(NarManifestEntry.NAR_ID.getManifestName()); + final String version = attributes.getValue(NarManifestEntry.NAR_VERSION.getManifestName()); + return new BundleIdentifier(groupId, artifactId, version); } private BundleIdentifier getDependencyBundleCoordinate(final Attributes attributes) { - try { - final String dependencyGroupId = attributes.getValue(NarManifestEntry.NAR_DEPENDENCY_GROUP.getManifestName()); - final String dependencyArtifactId = attributes.getValue(NarManifestEntry.NAR_DEPENDENCY_ID.getManifestName()); - final String dependencyVersion = attributes.getValue(NarManifestEntry.NAR_DEPENDENCY_VERSION.getManifestName()); - - final BundleIdentifier dependencyCoordinate; - if (dependencyArtifactId != null) { - dependencyCoordinate = new BundleIdentifier(dependencyGroupId, dependencyArtifactId, dependencyVersion); - } else { - dependencyCoordinate = null; - } - return dependencyCoordinate; - } catch (Exception e) { - throw new BundleException("Unable to obtain bundle coordinate for dependency due to: " + e.getMessage(), e); + final String dependencyGroupId = attributes.getValue(NarManifestEntry.NAR_DEPENDENCY_GROUP.getManifestName()); + final String dependencyArtifactId = attributes.getValue(NarManifestEntry.NAR_DEPENDENCY_ID.getManifestName()); + final String dependencyVersion = attributes.getValue(NarManifestEntry.NAR_DEPENDENCY_VERSION.getManifestName()); + if (dependencyArtifactId == null) { + return null; } + + return new BundleIdentifier(dependencyGroupId, dependencyArtifactId, dependencyVersion); } private BuildInfo getBuildInfo(final Attributes attributes) { diff --git a/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/main/java/org/apache/nifi/registry/bundle/model/BundleIdentifier.java b/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/main/java/org/apache/nifi/registry/bundle/model/BundleIdentifier.java index 8b9e1361cc91..d6cffc702550 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/main/java/org/apache/nifi/registry/bundle/model/BundleIdentifier.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/main/java/org/apache/nifi/registry/bundle/model/BundleIdentifier.java @@ -16,7 +16,7 @@ */ package org.apache.nifi.registry.bundle.model; -import static org.apache.nifi.registry.bundle.util.BundleUtils.validateNotBlank; +import org.apache.nifi.registry.bundle.util.BundleUtils; /** * The identifier of an extension bundle (i.e group + artifact + version). @@ -33,9 +33,9 @@ public BundleIdentifier(final String groupId, final String artifactId, final Str this.groupId = groupId; this.artifactId = artifactId; this.version = version; - validateNotBlank("Group Id", this.groupId); - validateNotBlank("Artifact Id", this.artifactId); - validateNotBlank("Version", this.version); + BundleUtils.validateCoordinateField("Group Id", this.groupId); + BundleUtils.validateCoordinateField("Artifact Id", this.artifactId); + BundleUtils.validateCoordinateField("Version", this.version); this.identifier = this.groupId + ":" + this.artifactId + ":" + this.version; } diff --git a/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/main/java/org/apache/nifi/registry/bundle/util/BundleUtils.java b/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/main/java/org/apache/nifi/registry/bundle/util/BundleUtils.java index 4c684ab95817..2a5bf95aa9fe 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/main/java/org/apache/nifi/registry/bundle/util/BundleUtils.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/main/java/org/apache/nifi/registry/bundle/util/BundleUtils.java @@ -34,4 +34,16 @@ public static void validateNotBlank(String fieldName, String value) { } } + public static void validateCoordinateField(final String fieldName, final String value) { + validateNotBlank(fieldName, value); + + if (".".equals(value) || "..".equals(value)) { + throw new IllegalArgumentException(fieldName + " is not a valid coordinate field"); + } + + if (value.indexOf('/') >= 0 || value.indexOf('\\') >= 0 || value.indexOf('\0') >= 0) { + throw new IllegalArgumentException(fieldName + " contains invalid characters"); + } + } + } diff --git a/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/test/java/org/apache/nifi/registry/bundle/extract/nar/TestNarBundleExtractor.java b/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/test/java/org/apache/nifi/registry/bundle/extract/nar/TestNarBundleExtractor.java index a7f18f22b6c8..804e3f4488ac 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/test/java/org/apache/nifi/registry/bundle/extract/nar/TestNarBundleExtractor.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/test/java/org/apache/nifi/registry/bundle/extract/nar/TestNarBundleExtractor.java @@ -97,7 +97,15 @@ public void testExtractFromGoodNarWithDependencies() throws IOException { @Test public void testExtractFromNarMissingRequiredManifestEntries() throws IOException { try (final InputStream in = new FileInputStream("src/test/resources/nars/nifi-missing-manifest-entries.nar")) { - assertThrows(BundleException.class, () -> extractor.extract(in)); + assertThrows(IllegalArgumentException.class, () -> extractor.extract(in)); + } + } + + @Test + public void testExtractFromNarWithParentDirectoryCoordinates(@TempDir final Path tempDir) throws IOException { + final Path narPath = writeNar(tempDir, "..", "..", "1.0.0"); + try (final InputStream in = Files.newInputStream(narPath)) { + assertThrows(IllegalArgumentException.class, () -> extractor.extract(in)); } } @@ -200,4 +208,22 @@ public void testExtractFromNarWithMetaInfDirectoryBeforeManifest(@TempDir final } } + private Path writeNar(final Path tempDir, final String groupId, final String artifactId, final String version) throws IOException { + final Path narPath = tempDir.resolve("testing.nar"); + try (final JarOutputStream jarOutputStream = new JarOutputStream(Files.newOutputStream(narPath))) { + final JarEntry manifestEntry = new JarEntry("META-INF/MANIFEST.MF"); + jarOutputStream.putNextEntry(manifestEntry); + jarOutputStream.write(( + "Manifest-Version: 1.0\n" + + "Nar-Group: " + groupId + "\n" + + "Nar-Id: " + artifactId + "\n" + + "Nar-Version: " + version + "\n" + + "Build-Timestamp: 2024-01-01T00:00:00Z\n\n" + ).getBytes(StandardCharsets.UTF_8)); + jarOutputStream.closeEntry(); + } + + return narPath; + } + } diff --git a/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/test/java/org/apache/nifi/registry/bundle/model/TestBundleIdentifier.java b/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/test/java/org/apache/nifi/registry/bundle/model/TestBundleIdentifier.java new file mode 100644 index 000000000000..aebddad057bc --- /dev/null +++ b/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/test/java/org/apache/nifi/registry/bundle/model/TestBundleIdentifier.java @@ -0,0 +1,41 @@ +/* + * 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.nifi.registry.bundle.model; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class TestBundleIdentifier { + + @Test + void testValidIdentifier() { + final BundleIdentifier identifier = new BundleIdentifier("org.apache.nifi", "nifi-standard-nar", "2.0.0-SNAPSHOT"); + assertEquals("org.apache.nifi", identifier.getGroupId()); + assertEquals("nifi-standard-nar", identifier.getArtifactId()); + assertEquals("2.0.0-SNAPSHOT", identifier.getVersion()); + } + + @Test + void testRejectsInvalidComponents() { + assertThrows(IllegalArgumentException.class, () -> new BundleIdentifier("..", "nifi-standard-nar", "1.0.0")); + assertThrows(IllegalArgumentException.class, () -> new BundleIdentifier("org.apache.nifi", "..", "1.0.0")); + assertThrows(IllegalArgumentException.class, () -> new BundleIdentifier("org.apache.nifi", "nifi-standard-nar", "..")); + assertThrows(IllegalArgumentException.class, () -> new BundleIdentifier("org/apache", "nifi-standard-nar", "1.0.0")); + } +} diff --git a/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/test/java/org/apache/nifi/registry/bundle/util/TestBundleUtils.java b/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/test/java/org/apache/nifi/registry/bundle/util/TestBundleUtils.java new file mode 100644 index 000000000000..87d960bca2dc --- /dev/null +++ b/nifi-registry/nifi-registry-core/nifi-registry-bundle-utils/src/test/java/org/apache/nifi/registry/bundle/util/TestBundleUtils.java @@ -0,0 +1,43 @@ +/* + * 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.nifi.registry.bundle.util; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +class TestBundleUtils { + + @Test + void testValidateCoordinateFieldAcceptsTypicalValues() { + BundleUtils.validateCoordinateField("Group Id", "org.apache.nifi"); + BundleUtils.validateCoordinateField("Artifact Id", "nifi-standard-nar"); + BundleUtils.validateCoordinateField("Version", "2.0.0-SNAPSHOT"); + BundleUtils.validateCoordinateField("Version", "1.0.0+build.5"); + } + + @Test + void testValidateCoordinateFieldRejectsInvalidValues() { + assertThrows(IllegalArgumentException.class, () -> BundleUtils.validateCoordinateField("Group Id", null)); + assertThrows(IllegalArgumentException.class, () -> BundleUtils.validateCoordinateField("Group Id", " ")); + assertThrows(IllegalArgumentException.class, () -> BundleUtils.validateCoordinateField("Group Id", ".")); + assertThrows(IllegalArgumentException.class, () -> BundleUtils.validateCoordinateField("Group Id", "..")); + assertThrows(IllegalArgumentException.class, () -> BundleUtils.validateCoordinateField("Group Id", "org/apache")); + assertThrows(IllegalArgumentException.class, () -> BundleUtils.validateCoordinateField("Artifact Id", "art\\ifact")); + assertThrows(IllegalArgumentException.class, () -> BundleUtils.validateCoordinateField("Version", "1.0.0\0")); + } +} diff --git a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/extension/FileSystemBundlePersistenceProvider.java b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/extension/FileSystemBundlePersistenceProvider.java index 0fe7e0d38918..f0883920b29a 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/extension/FileSystemBundlePersistenceProvider.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/extension/FileSystemBundlePersistenceProvider.java @@ -186,14 +186,14 @@ public synchronized void deleteAllBundleVersions(final BundleCoordinate bundleCo // delete the directory for the group and bucket if there is nothing left final File groupDir = bundleDir.getParentFile(); final File[] groupFiles = groupDir.listFiles(); - if (groupFiles.length == 0) { + if (groupFiles != null && groupFiles.length == 0) { final boolean deletedGroup = groupDir.delete(); if (!deletedGroup) { LOGGER.error("Unable to delete group directory: {}", groupDir.getAbsolutePath()); } else { final File bucketDir = groupDir.getParentFile(); final File[] bucketFiles = bucketDir.listFiles(); - if (bucketFiles.length == 0) { + if (bucketFiles != null && bucketFiles.length == 0) { final boolean deletedBucket = bucketDir.delete(); if (!deletedBucket) { LOGGER.error("Unable to delete bucket directory: {}", bucketDir.getAbsolutePath()); @@ -214,7 +214,7 @@ static File getBundleDirectory(final File bundleStorageDir, final BundleCoordina final String artifactId = bundleCoordinate.getArtifactId(); final Path artifactPath = getArtifactPath(bucketId, groupId, artifactId); - return getChildLocation(bundleStorageDir, artifactPath); + return FileUtils.getChildLocation(bundleStorageDir, artifactPath); } static File getBundleVersionDirectory(final File bundleStorageDir, final BundleVersionCoordinate versionCoordinate) { @@ -225,7 +225,7 @@ static File getBundleVersionDirectory(final File bundleStorageDir, final BundleV final Path artifactPath = getArtifactPath(bucketId, groupId, artifactId); final Path versionPath = Paths.get(sanitize(version)).normalize(); - return getChildLocation(bundleStorageDir, artifactPath.resolve(versionPath)); + return FileUtils.getChildLocation(bundleStorageDir, artifactPath.resolve(versionPath)); } static File getBundleFile(final File parentDir, final BundleVersionCoordinate versionCoordinate) { @@ -235,7 +235,7 @@ static File getBundleFile(final File parentDir, final BundleVersionCoordinate ve final String bundleFileExtension = getBundleFileExtension(bundleType); final String bundleFilename = sanitize(artifactId) + "-" + sanitize(version) + bundleFileExtension; - return getChildLocation(parentDir, Paths.get(bundleFilename)); + return FileUtils.getChildLocation(parentDir, Paths.get(bundleFilename)); } static Path getArtifactPath(final String bucketId, final String groupId, final String artifactId) { @@ -243,7 +243,12 @@ static Path getArtifactPath(final String bucketId, final String groupId, final S } static String sanitize(final String input) { - return FileUtils.sanitizeFilename(input).trim().toLowerCase(); + final String sanitized = FileUtils.sanitizeFilename(input).trim().toLowerCase(); + if (".".equals(sanitized) || "..".equals(sanitized)) { + throw new IllegalArgumentException("Coordinate component is not a valid path name"); + } + + return sanitized; } static String getBundleFileExtension(final BundleVersionType bundleType) { @@ -265,12 +270,4 @@ private static String getNormalizedBucketId(final String id) { } } - private static File getChildLocation(final File parentDir, final Path childLocation) { - final Path parentPath = parentDir.toPath().normalize(); - final Path childPath = parentPath.resolve(childLocation.normalize()); - if (childPath.startsWith(parentPath)) { - return childPath.toFile(); - } - throw new IllegalArgumentException(String.format("Child location not valid [%s]", childLocation)); - } } diff --git a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/extension/StandardBundleCoordinate.java b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/extension/StandardBundleCoordinate.java index a109d9ecefa6..337e2c9aff58 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/extension/StandardBundleCoordinate.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/extension/StandardBundleCoordinate.java @@ -17,6 +17,7 @@ package org.apache.nifi.registry.provider.extension; import org.apache.commons.lang3.Validate; +import org.apache.nifi.registry.bundle.util.BundleUtils; import org.apache.nifi.registry.extension.BundleCoordinate; import java.util.Objects; @@ -34,6 +35,9 @@ private StandardBundleCoordinate(final Builder builder) { Validate.notBlank(this.bucketId, "Bucket Id is required"); Validate.notBlank(this.groupId, "Group Id is required"); Validate.notBlank(this.artifactId, "Artifact Id is required"); + BundleUtils.validateCoordinateField("Bucket Id", this.bucketId); + BundleUtils.validateCoordinateField("Group Id", this.groupId); + BundleUtils.validateCoordinateField("Artifact Id", this.artifactId); } @Override diff --git a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/extension/StandardBundleVersionCoordinate.java b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/extension/StandardBundleVersionCoordinate.java index a3e076362d1d..30913257aa5c 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/extension/StandardBundleVersionCoordinate.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/extension/StandardBundleVersionCoordinate.java @@ -17,6 +17,7 @@ package org.apache.nifi.registry.provider.extension; import org.apache.commons.lang3.Validate; +import org.apache.nifi.registry.bundle.util.BundleUtils; import org.apache.nifi.registry.extension.BundleVersionCoordinate; import org.apache.nifi.registry.extension.BundleVersionType; @@ -41,6 +42,10 @@ private StandardBundleVersionCoordinate(final Builder builder) { Validate.notBlank(this.artifactId, "Artifact Id is required"); Validate.notBlank(this.version, "Version is required"); Validate.notNull(this.type, "BundleVersionType is required"); + BundleUtils.validateCoordinateField("Bucket Id", this.bucketId); + BundleUtils.validateCoordinateField("Group Id", this.groupId); + BundleUtils.validateCoordinateField("Artifact Id", this.artifactId); + BundleUtils.validateCoordinateField("Version", this.version); } @Override diff --git a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/flow/FileSystemFlowPersistenceProvider.java b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/flow/FileSystemFlowPersistenceProvider.java index c8adfe8fb786..25228acb0a3e 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/flow/FileSystemFlowPersistenceProvider.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/flow/FileSystemFlowPersistenceProvider.java @@ -78,14 +78,14 @@ public void onConfigured(final ProviderConfigurationContext configurationContext @Override public synchronized void saveFlowContent(final FlowSnapshotContext context, final byte[] content) throws FlowPersistenceException { - final File bucketDir = getChildLocation(flowStorageDir, getNormalizedIdPath(context.getBucketId())); + final File bucketDir = FileUtils.getChildLocation(flowStorageDir, getNormalizedIdPath(context.getBucketId())); try { FileUtils.ensureDirectoryExistAndCanReadAndWrite(bucketDir); } catch (IOException e) { throw new FlowPersistenceException("Error accessing bucket directory at " + bucketDir.getAbsolutePath(), e); } - final File flowDir = getChildLocation(bucketDir, getNormalizedIdPath(context.getFlowId())); + final File flowDir = FileUtils.getChildLocation(bucketDir, getNormalizedIdPath(context.getFlowId())); try { FileUtils.ensureDirectoryExistAndCanReadAndWrite(flowDir); } catch (IOException e) { @@ -93,7 +93,7 @@ public synchronized void saveFlowContent(final FlowSnapshotContext context, fina } final String versionString = String.valueOf(context.getVersion()); - final File versionDir = getChildLocation(flowDir, Paths.get(versionString)); + final File versionDir = FileUtils.getChildLocation(flowDir, Paths.get(versionString)); try { FileUtils.ensureDirectoryExistAndCanReadAndWrite(versionDir); } catch (IOException e) { @@ -101,7 +101,7 @@ public synchronized void saveFlowContent(final FlowSnapshotContext context, fina } final String versionExtension = versionString + SNAPSHOT_EXTENSION; - final File versionFile = getChildLocation(versionDir, Paths.get(versionExtension)); + final File versionFile = FileUtils.getChildLocation(versionDir, Paths.get(versionExtension)); if (versionFile.exists()) { throw new FlowPersistenceException("Unable to save, a snapshot already exists with version " + versionString); } @@ -141,7 +141,7 @@ public synchronized void deleteAllFlowContent(final String bucketId, final Strin final Path bucketIdPath = getNormalizedIdPath(bucketId); final Path flowIdPath = getNormalizedIdPath(flowId); final Path bucketFlowPath = bucketIdPath.resolve(flowIdPath); - final File flowDir = getChildLocation(flowStorageDir, bucketFlowPath); + final File flowDir = FileUtils.getChildLocation(flowStorageDir, bucketFlowPath); if (!flowDir.exists()) { LOGGER.debug("Snapshot directory does not exist at {}", flowDir.getAbsolutePath()); return; @@ -161,7 +161,7 @@ public synchronized void deleteAllFlowContent(final String bucketId, final Strin } // delete the directory for the bucket if there is nothing left - final File bucketDir = getChildLocation(flowStorageDir, getNormalizedIdPath(bucketId)); + final File bucketDir = FileUtils.getChildLocation(flowStorageDir, getNormalizedIdPath(bucketId)); final File[] bucketFiles = bucketDir.listFiles(); if (bucketFiles == null || bucketFiles.length == 0) { final boolean deletedBucket = bucketDir.delete(); @@ -192,17 +192,7 @@ public synchronized void deleteFlowContent(final String bucketId, final String f protected File getSnapshotFile(final String bucketId, final String flowId, final int version) { final String versionExtension = version + SNAPSHOT_EXTENSION; final Path snapshotLocation = Paths.get(getNormalizedId(bucketId), getNormalizedId(flowId), Integer.toString(version), versionExtension); - return getChildLocation(flowStorageDir, snapshotLocation); - } - - private File getChildLocation(final File parentDir, final Path childLocation) { - final Path parentPath = parentDir.toPath().normalize(); - final Path childPathNormalized = childLocation.normalize(); - final Path childPath = parentPath.resolve(childPathNormalized); - if (childPath.startsWith(parentPath)) { - return childPath.toFile(); - } - throw new IllegalArgumentException(String.format("Child location not valid [%s]", childLocation)); + return FileUtils.getChildLocation(flowStorageDir, snapshotLocation); } private Path getNormalizedIdPath(final String id) { diff --git a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/flow/git/GitFlowPersistenceProvider.java b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/flow/git/GitFlowPersistenceProvider.java index d7d94814cf12..075102c0960e 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/flow/git/GitFlowPersistenceProvider.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/provider/flow/git/GitFlowPersistenceProvider.java @@ -33,6 +33,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -146,10 +147,10 @@ public void saveFlowContent(FlowSnapshotContext context, byte[] content) throws flow.putVersion(context.getVersion(), flowPointer); - final File bucketDir = new File(flowStorageDir, bucketDirName); - final File flowSnippetFile = new File(bucketDir, flowSnapshotFilename); + final File bucketDir = getChildFile(flowStorageDir, bucketDirName); + final File flowSnippetFile = getChildFile(bucketDir, flowSnapshotFilename); - final File currentBucketDir = isEmpty(currentBucketDirName) ? null : new File(flowStorageDir, currentBucketDirName); + final File currentBucketDir = isEmpty(currentBucketDirName) ? null : getChildFile(flowStorageDir, currentBucketDirName); if (currentBucketDir != null && currentBucketDir.isDirectory()) { if (isBucketNameChanged) { logger.debug("Detected bucket name change from {} to {}, moving it.", currentBucketDirName, bucketDirName); @@ -166,7 +167,7 @@ public void saveFlowContent(FlowSnapshotContext context, byte[] content) throws try { if (currentFlowSnapshotFilename.isPresent() && !flowSnapshotFilename.equals(currentFlowSnapshotFilename.get())) { // Delete old file if flow name has been changed. - final File latestFlowSnapshotFile = new File(bucketDir, currentFlowSnapshotFilename.get()); + final File latestFlowSnapshotFile = getChildFile(bucketDir, currentFlowSnapshotFilename.get()); logger.debug("Detected flow name change from {} to {}, deleting the old snapshot file.", currentFlowSnapshotFilename.get(), flowSnapshotFilename); latestFlowSnapshotFile.delete(); @@ -231,8 +232,8 @@ public void deleteAllFlowContent(String bucketId, String flowId) throws FlowPers final Flow.FlowPointer flowPointer = flow.getFlowVersion(latestVersion); // Delete the flow snapshot. - final File bucketDir = new File(flowStorageDir, bucket.getBucketDirName()); - final File flowSnapshotFile = new File(bucketDir, flowPointer.getFileName()); + final File bucketDir = getChildFile(flowStorageDir, bucket.getBucketDirName()); + final File flowSnapshotFile = getChildFile(bucketDir, flowPointer.getFileName()); if (flowSnapshotFile.exists()) { if (!flowSnapshotFile.delete()) { throw new FlowPersistenceException(format("Failed to delete flow content for %s:%s in bucket %s:%s", @@ -264,6 +265,14 @@ public void deleteAllFlowContent(String bucketId, String flowId) throws FlowPers } + private File getChildFile(final File parentDir, final String childName) { + try { + return FileUtils.getChildLocation(parentDir, Paths.get(childName)); + } catch (final IllegalArgumentException e) { + throw new FlowPersistenceException(e.getMessage(), e); + } + } + private Bucket getBucketOrFail(String bucketId) throws FlowPersistenceException { final Optional bucketOpt = flowMetaData.getBucket(bucketId); if (!bucketOpt.isPresent()) { diff --git a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/RegistryService.java b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/RegistryService.java index 864462821585..7aa9405ed0ab 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/RegistryService.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/RegistryService.java @@ -261,19 +261,31 @@ public Bucket deleteBucket(final String bucketIdentifier) { } // for each bundle in the bucket, delete all versions from the bundle persistence provider - for (final BundleEntity bundleEntity : metadataService.getBundlesByBucket(existingBucket.getId())) { + final List bundleEntities = metadataService.getBundlesByBucket(existingBucket.getId()); + if (bundleEntities != null) { + for (final BundleEntity bundleEntity : bundleEntities) { + deletePersistedBundleVersions(bundleEntity); + } + } + + // now delete the bucket from the metadata provider, which deletes all flows referencing it + metadataService.deleteBucket(existingBucket); + + return BucketMappings.map(existingBucket); + } + + private void deletePersistedBundleVersions(final BundleEntity bundleEntity) { + try { final BundleCoordinate bundleCoordinate = new StandardBundleCoordinate.Builder() .bucketId(bundleEntity.getBucketId()) .groupId(bundleEntity.getGroupId()) .artifactId(bundleEntity.getArtifactId()) .build(); bundlePersistenceProvider.deleteAllBundleVersions(bundleCoordinate); + } catch (final IllegalArgumentException e) { + LOGGER.error("Unable to delete persisted content for bundle [{}] because the stored coordinates are not a valid path", + bundleEntity.getId(), e); } - - // now delete the bucket from the metadata provider, which deletes all flows referencing it - metadataService.deleteBucket(existingBucket); - - return BucketMappings.map(existingBucket); } // ---------------------- BucketItem methods --------------------------------------------- diff --git a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/extension/StandardExtensionService.java b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/extension/StandardExtensionService.java index 81d6d7a7178c..07431bc2df46 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/extension/StandardExtensionService.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/extension/StandardExtensionService.java @@ -457,13 +457,7 @@ public Bundle deleteBundle(final Bundle bundle) { metadataService.deleteBundle(bundle.getIdentifier()); // delete all content associated with the bundle in the persistence provider - final BundleCoordinate bundleCoordinate = new StandardBundleCoordinate.Builder() - .bucketId(bundle.getBucketIdentifier()) - .groupId(bundle.getGroupId()) - .artifactId(bundle.getArtifactId()) - .build(); - - bundlePersistenceProvider.deleteAllBundleVersions(bundleCoordinate); + deletePersistedBundleVersions(bundle); return bundle; } @@ -622,8 +616,7 @@ public BundleVersion deleteBundleVersion(final BundleVersion bundleVersion) { metadataService.deleteBundleVersion(extensionBundleVersionId); // delete content associated with the bundle version in the persistence provider - final BundleVersionCoordinate versionCoordinate = getVersionCoordinate(bundleVersion); - bundlePersistenceProvider.deleteBundleVersion(versionCoordinate); + deletePersistedBundleVersion(bundleVersion); return bundleVersion; } @@ -907,6 +900,30 @@ public SortedSet getExtensionRepoVersions(final Buc // ------ Helper Methods ------- + private void deletePersistedBundleVersions(final Bundle bundle) { + try { + final BundleCoordinate bundleCoordinate = new StandardBundleCoordinate.Builder() + .bucketId(bundle.getBucketIdentifier()) + .groupId(bundle.getGroupId()) + .artifactId(bundle.getArtifactId()) + .build(); + bundlePersistenceProvider.deleteAllBundleVersions(bundleCoordinate); + } catch (final IllegalArgumentException e) { + LOGGER.error("Unable to delete persisted content for bundle [{}] because the stored coordinates are not a valid path", + bundle.getIdentifier(), e); + } + } + + private void deletePersistedBundleVersion(final BundleVersion bundleVersion) { + try { + final BundleVersionCoordinate versionCoordinate = getVersionCoordinate(bundleVersion); + bundlePersistenceProvider.deleteBundleVersion(versionCoordinate); + } catch (final IllegalArgumentException e) { + LOGGER.error("Unable to delete persisted content for bundle version [{}] because the stored coordinates are not a valid path", + bundleVersion.getVersionMetadata().getId(), e); + } + } + private BundleVersionCoordinate getVersionCoordinate(final BundleVersion bundleVersion) { return getVersionCoordinate(bundleVersion.getBundle(), bundleVersion.getVersionMetadata()); } diff --git a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/extension/TestFileSystemBundlePersistenceProvider.java b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/extension/TestFileSystemBundlePersistenceProvider.java index ce5acad2f381..df154a81b035 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/extension/TestFileSystemBundlePersistenceProvider.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/extension/TestFileSystemBundlePersistenceProvider.java @@ -37,6 +37,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.util.HashMap; import java.util.Map; @@ -250,6 +251,52 @@ public void testDeleteAllBundleVersionsWhenDoesNotExist() { assertEquals(0, bundleStorageDir.listFiles().length); } + @Test + public void testCreateRejectsParentDirectoryCoordinates() throws IOException { + final File markerFile = createParentMarker(); + try { + final BundleVersionCoordinate versionCoordinate = getVersionCoordinate(BUCKET_ID, "..", "..", FIRST_VERSION, BundleVersionType.NIFI_NAR); + assertThrows(IllegalArgumentException.class, () -> createBundleVersion(fileSystemBundleProvider, versionCoordinate, "evil")); + assertTrue(markerFile.exists()); + assertTrue(bundleStorageDir.exists()); + assertFalse(new File(bundleStorageDir.getParentFile(), FIRST_VERSION).exists()); + } finally { + markerFile.delete(); + } + } + + @Test + public void testCreateAllowsSnapshotAndBuildMetadataVersions() throws IOException { + final String snapshotContent = "snapshot-content"; + final BundleVersionCoordinate snapshotCoordinate = getVersionCoordinate(BUCKET_ID, GROUP_ID, ARTIFACT_ID, "2.0.0-SNAPSHOT", BundleVersionType.NIFI_NAR); + createBundleVersion(fileSystemBundleProvider, snapshotCoordinate, snapshotContent); + verifyBundleVersion(bundleStorageDir, snapshotCoordinate, snapshotContent); + + final String buildMetadataContent = "build-metadata-content"; + final BundleVersionCoordinate buildMetadataCoordinate = getVersionCoordinate(BUCKET_ID, GROUP_ID, ARTIFACT_ID, "1.0.0+build.5", BundleVersionType.NIFI_NAR); + createBundleVersion(fileSystemBundleProvider, buildMetadataCoordinate, buildMetadataContent); + verifyBundleVersion(bundleStorageDir, buildMetadataCoordinate, buildMetadataContent); + } + + @Test + public void testDeleteAllBundleVersionsRejectsParentDirectoryCoordinates() throws IOException { + final File markerFile = createParentMarker(); + try { + final BundleCoordinate bundleCoordinate = getBundleCoordinate(BUCKET_ID, "..", ".."); + assertThrows(IllegalArgumentException.class, () -> fileSystemBundleProvider.deleteAllBundleVersions(bundleCoordinate)); + assertTrue(markerFile.exists()); + assertTrue(bundleStorageDir.exists()); + } finally { + markerFile.delete(); + } + } + + private File createParentMarker() throws IOException { + final File markerFile = new File(bundleStorageDir.getParentFile(), "registry-parent-marker.txt"); + Files.writeString(markerFile.toPath(), "keep"); + return markerFile; + } + private void createBundleVersion(final BundlePersistenceProvider persistenceProvider, final BundleVersionCoordinate versionCoordinate, final String content) throws IOException { @@ -287,10 +334,14 @@ private static BundleVersionCoordinate getVersionCoordinate(final String bucketI } private static BundleCoordinate getBundleCoordinate() { + return getBundleCoordinate(BUCKET_ID, GROUP_ID, ARTIFACT_ID); + } + + private static BundleCoordinate getBundleCoordinate(final String bucketId, final String groupId, final String artifactId) { final BundleCoordinate coordinate = Mockito.mock(BundleCoordinate.class); - when(coordinate.getBucketId()).thenReturn(BUCKET_ID); - when(coordinate.getGroupId()).thenReturn(GROUP_ID); - when(coordinate.getArtifactId()).thenReturn(ARTIFACT_ID); + when(coordinate.getBucketId()).thenReturn(bucketId); + when(coordinate.getGroupId()).thenReturn(groupId); + when(coordinate.getArtifactId()).thenReturn(artifactId); return coordinate; } diff --git a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/extension/TestStandardBundleCoordinate.java b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/extension/TestStandardBundleCoordinate.java new file mode 100644 index 000000000000..d51d8b18f080 --- /dev/null +++ b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/extension/TestStandardBundleCoordinate.java @@ -0,0 +1,57 @@ +/* + * 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.nifi.registry.provider.extension; + +import org.apache.nifi.registry.extension.BundleCoordinate; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class TestStandardBundleCoordinate { + + private static final String BUCKET_ID = "b0000000-0000-0000-0000-000000000000"; + + @Test + void testBuildAcceptsTypicalCoordinates() { + final BundleCoordinate coordinate = new StandardBundleCoordinate.Builder() + .bucketId(BUCKET_ID) + .groupId("org.apache.nifi") + .artifactId("nifi-standard-nar") + .build(); + assertEquals(BUCKET_ID, coordinate.getBucketId()); + assertEquals("org.apache.nifi", coordinate.getGroupId()); + assertEquals("nifi-standard-nar", coordinate.getArtifactId()); + } + + @Test + void testBuildRejectsInvalidComponents() { + assertInvalid("..", "nifi-standard-nar"); + assertInvalid("org.apache.nifi", ".."); + assertInvalid(".", "nifi-standard-nar"); + assertInvalid("org/apache", "nifi-standard-nar"); + assertInvalid("org.apache.nifi", "art\\ifact"); + } + + private void assertInvalid(final String groupId, final String artifactId) { + assertThrows(IllegalArgumentException.class, () -> new StandardBundleCoordinate.Builder() + .bucketId(BUCKET_ID) + .groupId(groupId) + .artifactId(artifactId) + .build()); + } +} diff --git a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/extension/TestStandardBundleVersionCoordinate.java b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/extension/TestStandardBundleVersionCoordinate.java new file mode 100644 index 000000000000..a995eed2cc21 --- /dev/null +++ b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/extension/TestStandardBundleVersionCoordinate.java @@ -0,0 +1,67 @@ +/* + * 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.nifi.registry.provider.extension; + +import org.apache.nifi.registry.extension.BundleVersionCoordinate; +import org.apache.nifi.registry.extension.BundleVersionType; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class TestStandardBundleVersionCoordinate { + + private static final String BUCKET_ID = "b0000000-0000-0000-0000-000000000000"; + + @Test + void testBuildAcceptsTypicalCoordinates() { + assertAccepted("org.apache.nifi", "nifi-standard-nar", "2.0.0-SNAPSHOT"); + assertAccepted("org.apache.nifi", "nifi-standard-nar", "1.0.0+build.5"); + } + + @Test + void testBuildRejectsInvalidComponents() { + assertInvalid("..", "nifi-standard-nar", "1.0.0"); + assertInvalid("org.apache.nifi", "..", "1.0.0"); + assertInvalid("org.apache.nifi", "nifi-standard-nar", ".."); + assertInvalid(".", "nifi-standard-nar", "1.0.0"); + assertInvalid("org/apache", "nifi-standard-nar", "1.0.0"); + } + + private void assertAccepted(final String groupId, final String artifactId, final String version) { + final BundleVersionCoordinate coordinate = new StandardBundleVersionCoordinate.Builder() + .bucketId(BUCKET_ID) + .groupId(groupId) + .artifactId(artifactId) + .version(version) + .type(BundleVersionType.NIFI_NAR) + .build(); + assertEquals(groupId, coordinate.getGroupId()); + assertEquals(artifactId, coordinate.getArtifactId()); + assertEquals(version, coordinate.getVersion()); + } + + private void assertInvalid(final String groupId, final String artifactId, final String version) { + assertThrows(IllegalArgumentException.class, () -> new StandardBundleVersionCoordinate.Builder() + .bucketId(BUCKET_ID) + .groupId(groupId) + .artifactId(artifactId) + .version(version) + .type(BundleVersionType.NIFI_NAR) + .build()); + } +} diff --git a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/flow/TestFileSystemFlowPersistenceProvider.java b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/flow/TestFileSystemFlowPersistenceProvider.java index 29aa8fa6afeb..f85ae596295f 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/flow/TestFileSystemFlowPersistenceProvider.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/flow/TestFileSystemFlowPersistenceProvider.java @@ -178,6 +178,18 @@ public void testDeleteSnapshot() { fileSystemFlowProvider.deleteFlowContent(SECOND_BUCKET_ID, FLOW_ID, 1); } + @Test + public void testSaveRejectsParentDirectoryIdentifiers() { + final FlowSnapshotContext context = Mockito.mock(FlowSnapshotContext.class); + when(context.getBucketId()).thenReturn(".."); + when(context.getFlowId()).thenReturn(FLOW_ID); + when(context.getVersion()).thenReturn(1); + + assertThrows(IllegalArgumentException.class, () -> fileSystemFlowProvider.saveFlowContent(context, FIRST_VERSION.getBytes(StandardCharsets.UTF_8))); + assertTrue(flowStorageDir.exists()); + assertFalse(new File(flowStorageDir.getParentFile(), FLOW_ID).exists()); + } + private void createAndSaveSnapshot(final FlowPersistenceProvider flowPersistenceProvider, final int version, final String contentString) { final FlowSnapshotContext context = Mockito.mock(FlowSnapshotContext.class); when(context.getBucketId()).thenReturn(BUCKET_ID); diff --git a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/flow/git/TestGitFlowPersistenceProvider.java b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/flow/git/TestGitFlowPersistenceProvider.java index c88eee6aa05f..ab725af18a7d 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/flow/git/TestGitFlowPersistenceProvider.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/provider/flow/git/TestGitFlowPersistenceProvider.java @@ -42,6 +42,9 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; public class TestGitFlowPersistenceProvider { @@ -289,6 +292,32 @@ public void testLoadCommitHistories() throws GitAPIException, IOException { }, true); } + @Test + public void testSaveRejectsParentDirectoryBucketName() throws GitAPIException, IOException { + final Map properties = new HashMap<>(); + properties.put(GitFlowPersistenceProvider.FLOW_STORAGE_DIR_PROP, "target/git-parent-dir-bucket"); + + assertProvider(properties, g -> { }, p -> { + final StandardFlowSnapshotContext context = new StandardFlowSnapshotContext.Builder() + .bucketId("bucket-id-A") + .bucketName("..") + .flowId("flow-id-1") + .flowName("flow") + .author("unit-test-user") + .comments("Initial commit.") + .snapshotTimestamp(new Date().getTime()) + .version(1) + .build(); + + assertThrows(FlowPersistenceException.class, () -> p.saveFlowContent(context, "content".getBytes(StandardCharsets.UTF_8))); + + final File gitDir = new File("target/git-parent-dir-bucket"); + assertTrue(gitDir.exists()); + final File escapedSnapshot = new File(gitDir.getParentFile(), "flow.snapshot"); + assertFalse(escapedSnapshot.exists()); + }, true); + } + @Test public void testLoadLargeFlow() throws GitAPIException, IOException { final Map properties = new HashMap<>(); diff --git a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/service/TestRegistryService.java b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/service/TestRegistryService.java index dd9df4bb739d..2851a3825976 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/service/TestRegistryService.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-framework/src/test/java/org/apache/nifi/registry/service/TestRegistryService.java @@ -24,6 +24,7 @@ import org.apache.nifi.flow.VersionedProcessor; import org.apache.nifi.registry.bucket.Bucket; import org.apache.nifi.registry.db.entity.BucketEntity; +import org.apache.nifi.registry.db.entity.BundleEntity; import org.apache.nifi.registry.db.entity.FlowEntity; import org.apache.nifi.registry.db.entity.FlowSnapshotEntity; import org.apache.nifi.registry.diff.ComponentDifference; @@ -65,6 +66,7 @@ import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -295,6 +297,31 @@ public void testDeleteBucketWithFlows() { .deleteAllFlowContent(eq(bucketToDelete.getId()), eq(flowToDelete.getId())); } + @Test + public void testDeleteBucketWithInvalidBundleCoordinates() { + final BucketEntity bucketToDelete = new BucketEntity(); + bucketToDelete.setId("b1"); + bucketToDelete.setName("My Bucket"); + bucketToDelete.setCreated(new Date()); + + final BundleEntity unsafeBundle = new BundleEntity(); + unsafeBundle.setId("bundle1"); + unsafeBundle.setBucketId(bucketToDelete.getId()); + unsafeBundle.setGroupId(".."); + unsafeBundle.setArtifactId(".."); + + when(metadataService.getBucketById(bucketToDelete.getId())).thenReturn(bucketToDelete); + when(metadataService.getFlowsByBucket(bucketToDelete.getId())).thenReturn(Collections.emptyList()); + when(metadataService.getBundlesByBucket(bucketToDelete.getId())).thenReturn(Collections.singletonList(unsafeBundle)); + + final Bucket deletedBucket = registryService.deleteBucket(bucketToDelete.getId()); + assertNotNull(deletedBucket); + assertEquals(bucketToDelete.getId(), deletedBucket.getIdentifier()); + + verify(metadataService).deleteBucket(bucketToDelete); + verify(bundlePersistenceProvider, never()).deleteAllBundleVersions(any()); + } + // ---------------------- Test VersionedFlow methods --------------------------------------------- @Test diff --git a/nifi-registry/nifi-registry-core/nifi-registry-utils/src/main/java/org/apache/nifi/registry/util/FileUtils.java b/nifi-registry/nifi-registry-core/nifi-registry-utils/src/main/java/org/apache/nifi/registry/util/FileUtils.java index c2f5c8eb1b47..f848e0503072 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-utils/src/main/java/org/apache/nifi/registry/util/FileUtils.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-utils/src/main/java/org/apache/nifi/registry/util/FileUtils.java @@ -414,4 +414,31 @@ public static String sanitizeFilename(String filename) { } return cleanName.toString(); } + + /** + * Resolves {@code childLocation} against {@code parentDir} and returns the resulting file only when + * the normalized absolute path remains a strict child of the parent. Relative parent segments, + * absolute child locations, and paths that resolve to the parent itself are rejected. + * + * @param parentDir the directory that must contain the result + * @param childLocation a relative path to resolve under the parent + * @return the resolved child file + */ + public static File getChildLocation(final File parentDir, final Path childLocation) { + if (parentDir == null) { + throw new IllegalArgumentException("Parent directory is required"); + } + + if (childLocation == null || childLocation.isAbsolute()) { + throw new IllegalArgumentException(String.format("Child location not valid [%s]", childLocation)); + } + + final Path parentPath = parentDir.toPath().toAbsolutePath().normalize(); + final Path childPath = parentPath.resolve(childLocation).normalize(); + if (!childPath.startsWith(parentPath) || childPath.equals(parentPath)) { + throw new IllegalArgumentException(String.format("Child location not valid [%s]", childLocation)); + } + + return childPath.toFile(); + } } diff --git a/nifi-registry/nifi-registry-core/nifi-registry-utils/src/test/java/org/apache/nifi/registry/util/TestFileUtils.java b/nifi-registry/nifi-registry-core/nifi-registry-utils/src/test/java/org/apache/nifi/registry/util/TestFileUtils.java index 5679f5cefb60..268e20e23a10 100644 --- a/nifi-registry/nifi-registry-core/nifi-registry-utils/src/test/java/org/apache/nifi/registry/util/TestFileUtils.java +++ b/nifi-registry/nifi-registry-core/nifi-registry-utils/src/test/java/org/apache/nifi/registry/util/TestFileUtils.java @@ -18,8 +18,15 @@ package org.apache.nifi.registry.util; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.File; +import java.nio.file.Path; +import java.nio.file.Paths; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; public class TestFileUtils { @Test @@ -28,4 +35,27 @@ public void testSanitizeFilename() { final String sanitizedFilename = FileUtils.sanitizeFilename(filename); assertEquals("This___is___a_test", sanitizedFilename); } + + @Test + public void testGetChildLocationAcceptsContainedPath(@TempDir final Path tempDir) { + final File parentDir = tempDir.toFile(); + final File child = FileUtils.getChildLocation(parentDir, Paths.get("bucket", "group", "artifact")); + final Path parentPath = parentDir.toPath().toAbsolutePath().normalize(); + final Path childPath = child.toPath().toAbsolutePath().normalize(); + assertTrue(childPath.startsWith(parentPath)); + assertEquals(parentPath.resolve(Paths.get("bucket", "group", "artifact")), childPath); + } + + @Test + public void testGetChildLocationRejectsEscapeAndIdentity(@TempDir final Path tempDir) { + final File parentDir = tempDir.toFile(); + assertThrows(IllegalArgumentException.class, () -> FileUtils.getChildLocation(parentDir, Paths.get(".."))); + assertThrows(IllegalArgumentException.class, () -> FileUtils.getChildLocation(parentDir, Paths.get("..", "1.0.0"))); + assertThrows(IllegalArgumentException.class, () -> FileUtils.getChildLocation(parentDir, Paths.get("..", ".."))); + assertThrows(IllegalArgumentException.class, () -> FileUtils.getChildLocation(parentDir, Paths.get("."))); + assertThrows(IllegalArgumentException.class, () -> FileUtils.getChildLocation(parentDir, Paths.get(""))); + assertThrows(IllegalArgumentException.class, () -> FileUtils.getChildLocation(parentDir, tempDir.resolve("other"))); + assertThrows(IllegalArgumentException.class, () -> FileUtils.getChildLocation(null, Paths.get("child"))); + assertThrows(IllegalArgumentException.class, () -> FileUtils.getChildLocation(parentDir, null)); + } } diff --git a/nifi-registry/nifi-registry-extensions/nifi-registry-aws/nifi-registry-aws-extensions/src/main/java/org/apache/nifi/registry/aws/S3BundlePersistenceProvider.java b/nifi-registry/nifi-registry-extensions/nifi-registry-aws/nifi-registry-aws-extensions/src/main/java/org/apache/nifi/registry/aws/S3BundlePersistenceProvider.java index 776bfa6efbcf..0a0717afde95 100644 --- a/nifi-registry/nifi-registry-extensions/nifi-registry-aws/nifi-registry-aws-extensions/src/main/java/org/apache/nifi/registry/aws/S3BundlePersistenceProvider.java +++ b/nifi-registry/nifi-registry-extensions/nifi-registry-aws/nifi-registry-aws-extensions/src/main/java/org/apache/nifi/registry/aws/S3BundlePersistenceProvider.java @@ -331,7 +331,12 @@ private String getBundlePrefix(final String bucketId, final String groupId, fina } private static String sanitize(final String input) { - return FileUtils.sanitizeFilename(input).trim().toLowerCase(); + final String sanitized = FileUtils.sanitizeFilename(input).trim().toLowerCase(); + if (".".equals(sanitized) || "..".equals(sanitized)) { + throw new IllegalArgumentException("Coordinate component is not a valid path name"); + } + + return sanitized; } static String getBundleFileExtension(final BundleVersionType bundleType) {