From c0af89fd71a90a584918f7f65a0000560c863f99 Mon Sep 17 00:00:00 2001
From: Benjamin Frueh <benjamin.frueh@gmail.com>
Date: Thu, 4 Jun 2026 07:23:23 +0200
Subject: [PATCH] chore: improve placeholder substitution

Signed-off-by: Benjamin Frueh <benjamin.frueh@gmail.com>
---
 README.md                       |  3 +++
 lib/BackgroundJobs/Launcher.php | 25 ++++++++++++++++++++++---
 lib/Operation.php               | 33 +++++++++++++++------------------
 3 files changed, 40 insertions(+), 21 deletions(-)

diff --git a/README.md b/README.md
index 790f129..d98ff99 100644
--- a/README.md
+++ b/README.md
@@ -24,6 +24,9 @@ When defining the script, you can specify one of the following placeholders that
 
 When no placeholder was specified, then the exact command as given is being executed.
 
+### Security
+Do not wrap placeholders in quotes, as they are already quoted by the app. Wrapping them in quotes can neutralize this and may allow command execution via filenames. Files containing `$(` or `` ` `` in their filename will be skipped.
+
 ### Hints
 
 Events for files and folders are triggered by file system operations. An operation like
diff --git a/lib/BackgroundJobs/Launcher.php b/lib/BackgroundJobs/Launcher.php
index d3d3b1e..acae34a 100644
--- a/lib/BackgroundJobs/Launcher.php
+++ b/lib/BackgroundJobs/Launcher.php
@@ -25,6 +25,7 @@
 namespace OCA\WorkflowScript\BackgroundJobs;
 
 use Exception;
+use InvalidArgumentException;
 use OC\Files\View;
 use OCA\WorkflowScript\AppInfo\Application;
 use OCP\AppFramework\Utility\ITimeFactory;
@@ -48,8 +49,7 @@ protected function run($argument): void {
 		if (strpos($command, '%f')) {
 			$path = isset($argument['path']) ? (string)$argument['path'] : '';
 			try {
-				$view = new View(dirname($path));
-				$tmpFile = $view->toTmpFile(basename($path));
+				$command = str_replace('%f', escapeshellarg($this->resolveLocalPath($path)), $command);
 			} catch (Exception $e) {
 				$this->logger->warning($e->getMessage(), [
 					'app' => Application::APPID,
@@ -57,7 +57,6 @@ protected function run($argument): void {
 				]);
 				return;
 			}
-			$command = str_replace('%f', escapeshellarg($tmpFile), $command);
 		}
 
 		// with wrapping sh around the command, we leave any redirects intact,
@@ -69,4 +68,24 @@ protected function run($argument): void {
 		);
 		shell_exec($wrapper);
 	}
+
+	/**
+	 * @throws InvalidArgumentException
+	 */
+	private function resolveLocalPath(string $path): string {
+		try {
+			$view = new View();
+			$localFile = $view->getLocalFile($path);
+			if ($localFile !== false && file_exists($localFile)) {
+				return $localFile;
+			}
+			$tmpFile = $view->toTmpFile($path);
+			if ($tmpFile === false) {
+				throw new InvalidArgumentException();
+			}
+			return $tmpFile;
+		} catch (Exception) {
+			throw new InvalidArgumentException('Could not resolve local path for: ' . $path);
+		}
+	}
 }
diff --git a/lib/Operation.php b/lib/Operation.php
index 988820f..719a784 100644
--- a/lib/Operation.php
+++ b/lib/Operation.php
@@ -24,9 +24,6 @@
 
 namespace OCA\WorkflowScript;
 
-use Exception;
-use InvalidArgumentException;
-use OC\Files\View;
 use OC\User\NoUserException;
 use OCA\Files_Sharing\SharedStorage;
 use OCA\GroupFolders\Mount\GroupFolderStorage;
@@ -151,6 +148,21 @@ public function onEvent(string $eventName, Event $event, IRuleMatcher $ruleMatch
 			}
 
 			$matches = $ruleMatcher->getFlows(false);
+			if (empty($matches)) {
+				return;
+			}
+
+			if (preg_match('/\$\(|`/', $node->getName())) {
+				$this->logger->warning(
+					'Potentially dangerous characters in filename, skipping workflow',
+					[
+						'app' => Application::APPID,
+						'file' => $node->getPath(),
+					]
+				);
+				return;
+			}
+
 			foreach ($matches as $match) {
 				try {
 					$command = $this->buildCommand($match['operation'], $node, $eventName, $extra);
@@ -194,21 +206,6 @@ protected function buildCommand(string $template, Node $node, string $event, arr
 			unset($ncRelPath);
 		}
 
-		if (strpos($command, '%f')) {
-			try {
-				$view = new View();
-				if ($node instanceof FileNode) {
-					$fullPath = $view->getLocalFile($node->getPath());
-				}
-				if (!isset($fullPath) || $fullPath === false) {
-					throw new InvalidArgumentException();
-				}
-				$command = str_replace('%f', escapeshellarg($fullPath), $command);
-			} catch (Exception) {
-				throw new InvalidArgumentException('Could not determine full path');
-			}
-		}
-
 		if (strpos($command, '%i')) {
 			$nodeID = -1;
 			try {