NIFI-16329 Add Coordinate Validation for Registry NAR Bundles

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) {

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;
     }

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");
+        }
+    }
+
 }

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;
+    }
+
 }

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"));
+    }
+}

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"));
+    }
+}

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,15 +235,20 @@ 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) {
         return Paths.get(getNormalizedBucketId(bucketId), sanitize(groupId), sanitize(artifactId)).normalize();
     }
 
     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));
-    }
 }

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

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