This is an automated email from the ASF dual-hosted git repository.
pvillard31 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/nifi.git
The following commit(s) were added to refs/heads/main by this push:
new 78df4c14b98 NIFI-16110 Improved MiNiFi C2 Asset File Name Validation
(#11427)
78df4c14b98 is described below
commit 78df4c14b98114dfc7127061684684fb7d186182
Author: David Handermann <[email protected]>
AuthorDate: Wed Jul 15 03:28:07 2026 -0500
NIFI-16110 Improved MiNiFi C2 Asset File Name Validation (#11427)
- Aligned Asset File Name validation for update and synchronization
strategies
---
.../c2/command/UpdateAssetCommandHelper.java | 61 +++++++++++++------
.../syncresource/DefaultSyncResourceStrategy.java | 4 +-
.../c2/command/UpdateAssetCommandHelperTest.java | 69 +++++++++++++---------
3 files changed, 85 insertions(+), 49 deletions(-)
diff --git
a/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/main/java/org/apache/nifi/minifi/c2/command/UpdateAssetCommandHelper.java
b/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/main/java/org/apache/nifi/minifi/c2/command/UpdateAssetCommandHelper.java
index 166168bca86..102f55077f7 100644
---
a/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/main/java/org/apache/nifi/minifi/c2/command/UpdateAssetCommandHelper.java
+++
b/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/main/java/org/apache/nifi/minifi/c2/command/UpdateAssetCommandHelper.java
@@ -25,9 +25,15 @@ import java.io.UncheckedIOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static java.util.regex.Pattern.compile;
public class UpdateAssetCommandHelper {
+ public static final Pattern ALLOWED_RESOURCE_PATH_PATTERN =
compile("^(?:[^~<>:|\"?*./\\\\]+(?:[/\\\\][^~<>:|\"?*./\\\\]+)*)?$");
+
private static final Logger LOG =
LoggerFactory.getLogger(UpdateAssetCommandHelper.class);
private final String assetDirectory;
@@ -45,26 +51,47 @@ public class UpdateAssetCommandHelper {
}
}
- public boolean assetUpdatePrecondition(String assetFileName, Boolean
forceDownload) {
- Path assetPath = Paths.get(assetDirectory, assetFileName);
- if (Files.exists(assetPath) && !forceDownload) {
- LOG.info("Asset file already exists on path {}. Asset won't be
downloaded", assetPath);
- return false;
+ public boolean assetUpdatePrecondition(final String assetFileName, final
Boolean forceDownload) {
+ final boolean downloadEnabled;
+
+ final Matcher assetFileNameMatcher =
ALLOWED_RESOURCE_PATH_PATTERN.matcher(assetFileName);
+ if (assetFileNameMatcher.matches()) {
+ final Path assetPath = Paths.get(assetDirectory, assetFileName);
+ if (Files.exists(assetPath) && !forceDownload) {
+ LOG.info("Asset File found at [{}] Download disabled",
assetPath);
+ downloadEnabled = false;
+ } else {
+ LOG.info("Asset File not found at [{}] or Force Download
enabled", assetPath);
+ downloadEnabled = true;
+ }
+ } else {
+ LOG.warn("Asset File Name [{}] not allowed for downloading",
assetFileName);
+ downloadEnabled = false;
}
- LOG.info("Asset file does not exist or force download is on. Asset
will be downloaded to {}", assetPath);
- return true;
+
+ return downloadEnabled;
}
- public boolean assetPersistFunction(String assetFileName, byte[]
assetBinary) {
- Path assetPath = Paths.get(assetDirectory, assetFileName);
- try {
- Files.deleteIfExists(assetPath);
- Files.write(assetPath, assetBinary);
- LOG.info("Asset was persisted to {}, {} bytes were written",
assetPath, assetBinary.length);
- return true;
- } catch (IOException e) {
- LOG.error("Persisting asset failed. File creation was not
successful targeting {}", assetPath, e);
- return false;
+ public boolean assetPersistFunction(final String assetFileName, final
byte[] assetBinary) {
+ boolean persisted;
+
+ final Matcher assetFileNameMatcher =
ALLOWED_RESOURCE_PATH_PATTERN.matcher(assetFileName);
+ if (assetFileNameMatcher.matches()) {
+ final Path assetPath = Paths.get(assetDirectory, assetFileName);
+ try {
+ Files.deleteIfExists(assetPath);
+ Files.write(assetPath, assetBinary);
+ LOG.info("Asset was persisted to {}, {} bytes were written",
assetPath, assetBinary.length);
+ persisted = true;
+ } catch (final IOException e) {
+ LOG.error("Persisting asset failed. File creation was not
successful targeting {}", assetPath, e);
+ persisted = false;
+ }
+ } else {
+ LOG.warn("Asset File Name [{}] not allowed for writing",
assetFileName);
+ persisted = false;
}
+
+ return persisted;
}
}
diff --git
a/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/main/java/org/apache/nifi/minifi/c2/command/syncresource/DefaultSyncResourceStrategy.java
b/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/main/java/org/apache/nifi/minifi/c2/command/syncresource/DefaultSyncResourceStrategy.java
index 228e8656d6f..f98012d62b6 100644
---
a/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/main/java/org/apache/nifi/minifi/c2/command/syncresource/DefaultSyncResourceStrategy.java
+++
b/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/main/java/org/apache/nifi/minifi/c2/command/syncresource/DefaultSyncResourceStrategy.java
@@ -33,7 +33,6 @@ import java.util.Optional;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.Function;
-import java.util.regex.Pattern;
import static java.nio.file.Files.copy;
import static java.nio.file.Files.createTempFile;
@@ -42,17 +41,16 @@ import static java.util.Map.entry;
import static java.util.Optional.empty;
import static java.util.UUID.randomUUID;
import static java.util.function.Predicate.not;
-import static java.util.regex.Pattern.compile;
import static
org.apache.nifi.c2.protocol.api.C2OperationState.OperationState.FULLY_APPLIED;
import static
org.apache.nifi.c2.protocol.api.C2OperationState.OperationState.NOT_APPLIED;
import static
org.apache.nifi.c2.protocol.api.C2OperationState.OperationState.NO_OPERATION;
import static
org.apache.nifi.c2.protocol.api.C2OperationState.OperationState.PARTIALLY_APPLIED;
import static org.apache.nifi.c2.protocol.api.ResourceType.ASSET;
+import static
org.apache.nifi.minifi.c2.command.UpdateAssetCommandHelper.ALLOWED_RESOURCE_PATH_PATTERN;
public class DefaultSyncResourceStrategy implements SyncResourceStrategy {
private static final Logger LOG =
LoggerFactory.getLogger(DefaultSyncResourceStrategy.class);
- private static final Pattern ALLOWED_RESOURCE_PATH_PATTERN =
compile("^(?:[^~<>:\\|\\\"\\?\\*\\.\\/\\\\]+(?:[/\\\\][^~<>:\\|\\\"\\?\\*\\.\\/\\\\]+)*)?$");
private static final Set<Entry<OperationState, OperationState>>
SUCCESS_RESULT_PAIRS = Set.of(
entry(NO_OPERATION, NO_OPERATION),
diff --git
a/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/test/java/org/apache/nifi/minifi/c2/command/UpdateAssetCommandHelperTest.java
b/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/test/java/org/apache/nifi/minifi/c2/command/UpdateAssetCommandHelperTest.java
index 33f03f838e3..663b9d87374 100644
---
a/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/test/java/org/apache/nifi/minifi/c2/command/UpdateAssetCommandHelperTest.java
+++
b/minifi/minifi-nar-bundles/minifi-framework-bundle/minifi-framework/minifi-framework-core/src/test/java/org/apache/nifi/minifi/c2/command/UpdateAssetCommandHelperTest.java
@@ -20,31 +20,43 @@ package org.apache.nifi.minifi.c2.command;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.FieldSource;
import java.io.File;
import java.io.IOException;
import java.io.UncheckedIOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import static java.lang.Boolean.FALSE;
import static java.lang.Boolean.TRUE;
import static java.nio.charset.Charset.defaultCharset;
-import static java.nio.charset.StandardCharsets.UTF_8;
import static java.nio.file.Files.exists;
import static java.nio.file.Files.isDirectory;
import static java.nio.file.Files.readAllLines;
-import static java.nio.file.Files.write;
import static java.util.Collections.singletonList;
import static org.apache.commons.lang3.StringUtils.EMPTY;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertIterableEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
-public class UpdateAssetCommandHelperTest {
+class UpdateAssetCommandHelperTest {
+
+ private static final String[] INVALID_ASSET_FILE_NAMES = new String[]{
+ "~",
+ "./",
+ "../",
+ "./../",
+ "../sibling-directory",
+ "/tmp"
+ };
private static final String ASSET_DIRECTORY = "asset_directory";
- private static final String ASSET_FILE = "asset.file";
+ private static final String ASSET_FILE = "asset-file";
+ private static final byte[] CONTENT =
String.class.getSimpleName().getBytes(StandardCharsets.UTF_8);
@TempDir
private File tempDir;
@@ -53,89 +65,88 @@ public class UpdateAssetCommandHelperTest {
private UpdateAssetCommandHelper updateAssetCommandHelper;
@BeforeEach
- public void setUp() {
+ void setUp() {
assetDirectory = Paths.get(tempDir.getAbsolutePath(), ASSET_DIRECTORY);
updateAssetCommandHelper = new
UpdateAssetCommandHelper(assetDirectory.toString());
}
@Test
- public void testAssetDirectoryShouldBeCreated() {
- // given + when
+ void testAssetDirectoryShouldBeCreated() {
updateAssetCommandHelper.createAssetDirectory();
- // then
assertTrue(exists(assetDirectory));
assertTrue(isDirectory(assetDirectory));
}
@Test
- public void testAssetFileDoesNotExist() {
- // given
+ void testAssetFileDoesNotExist() {
updateAssetCommandHelper.createAssetDirectory();
- // when
boolean result =
updateAssetCommandHelper.assetUpdatePrecondition(ASSET_FILE, FALSE);
- // then
assertTrue(result);
}
@Test
- public void testAssetFileExistsAndForceDownloadOff() {
- // given
+ void testAssetFileExistsAndForceDownloadOff() {
updateAssetCommandHelper.createAssetDirectory();
touchAssetFile();
- // when
boolean result =
updateAssetCommandHelper.assetUpdatePrecondition(ASSET_FILE, FALSE);
- // then
assertFalse(result);
}
@Test
- public void testAssetFileExistsAndForceDownloadOn() {
- // given
+ void testAssetFileExistsAndForceDownloadOn() {
updateAssetCommandHelper.createAssetDirectory();
touchAssetFile();
- // when
boolean result =
updateAssetCommandHelper.assetUpdatePrecondition(ASSET_FILE, TRUE);
- // then
assertTrue(result);
}
@Test
- public void testAssetPersistedCorrectly() throws IOException {
- // given
+ void testAssetPersistedCorrectly() throws IOException {
updateAssetCommandHelper.createAssetDirectory();
String testAssetContent = "test file content";
- // when
boolean result =
updateAssetCommandHelper.assetPersistFunction(ASSET_FILE,
testAssetContent.getBytes(defaultCharset()));
- // then
assertTrue(result);
assertIterableEquals(singletonList(testAssetContent),
readAllLines(assetDirectory.resolve(ASSET_FILE), defaultCharset()));
}
@Test
- public void testAssetDirectoryDoesNotExistWhenPersistingAsset() {
- // given
+ void testAssetDirectoryDoesNotExistWhenPersistingAsset() {
String testAssetContent = "test file content";
- // when
boolean result =
updateAssetCommandHelper.assetPersistFunction(ASSET_FILE,
testAssetContent.getBytes(defaultCharset()));
- // then
assertFalse(result);
assertFalse(exists(assetDirectory.resolve(ASSET_FILE)));
}
+ @ParameterizedTest
+ @FieldSource("INVALID_ASSET_FILE_NAMES")
+ void testAssetUpdatePreconditionDirectoriesDisallowed(final String
assetName) {
+ final boolean completed =
updateAssetCommandHelper.assetUpdatePrecondition(assetName, true);
+
+ assertFalse(completed);
+ }
+
+ @ParameterizedTest
+ @FieldSource("INVALID_ASSET_FILE_NAMES")
+ void testAssetPersistFunctionDirectoriesDisallowed(final String assetName)
{
+ final boolean completed =
updateAssetCommandHelper.assetPersistFunction(assetName, CONTENT);
+
+ assertFalse(completed);
+ }
+
private void touchAssetFile() {
try {
- write(Paths.get(assetDirectory.toString(), ASSET_FILE),
EMPTY.getBytes(UTF_8));
+ Files.writeString(Paths.get(assetDirectory.toString(),
ASSET_FILE), EMPTY);
} catch (IOException e) {
throw new UncheckedIOException("Failed to touch file", e);
}
