Merge pull request #43968 from exi/custom-binary-search-paths

feat: make search path for BinaryFinder customizable.

config/config.sample.php

@@ -2533,4 +2533,20 @@ $CONFIG = [
  * Defaults to ``true``
  */
 'enable_non-accessible_features' => true,
+
+/**
+ * Directories where nextcloud looks for binaries.
+ * This is used to find external binaries like libreoffice, sendmail, ffmpeg and more.
+ *
+ * Defaults to ``['/usr/local/sbin','/usr/local/bin','/usr/sbin','/usr/bin','/sbin','/bin','/opt/bin']``
+ */
+'binary_search_paths' => [
+	'/usr/local/sbin',
+	'/usr/local/bin',
+	'/usr/sbin',
+	'/usr/bin',
+	'/sbin',
+	'/bin',
+	'/opt/bin',
+],
 ];

lib/private/BinaryFinder.php

@@ -11,15 +11,28 @@ namespace OC;
 use OCP\IBinaryFinder;
 use OCP\ICache;
 use OCP\ICacheFactory;
+use OCP\IConfig;
 use Symfony\Component\Process\ExecutableFinder;
 
 /**
  * Service that find the binary path for a program
  */
 class BinaryFinder implements IBinaryFinder {
+	public const DEFAULT_BINARY_SEARCH_PATHS = [
+		'/usr/local/sbin',
+		'/usr/local/bin',
+		'/usr/sbin',
+		'/usr/bin',
+		'/sbin',
+		'/bin',
+		'/opt/bin',
+	];
 	private ICache $cache;
 
-	public function __construct(ICacheFactory $cacheFactory) {
+	public function __construct(
+		ICacheFactory   $cacheFactory,
+		private IConfig $config,
+	) {
 		$this->cache = $cacheFactory->createLocal('findBinaryPath');
 	}
 
@@ -37,15 +50,10 @@ class BinaryFinder implements IBinaryFinder {
 		if (\OCP\Util::isFunctionEnabled('exec')) {
 			$exeSniffer = new ExecutableFinder();
 			// Returns null if nothing is found
-			$result = $exeSniffer->find($program, null, [
-				'/usr/local/sbin',
-				'/usr/local/bin',
-				'/usr/sbin',
-				'/usr/bin',
-				'/sbin',
-				'/bin',
-				'/opt/bin',
-			]);
+			$result = $exeSniffer->find(
+				$program,
+				null,
+				$this->config->getSystemValue('binary_search_paths', self::DEFAULT_BINARY_SEARCH_PATHS));
 			if ($result === null) {
 				$result = false;
 			}

tests/lib/BinaryFinderTest.php

@@ -0,0 +1,84 @@
+<?php
+
+declare(strict_types = 1);
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace Test;
+
+use OC\BinaryFinder;
+use OC\Memcache\ArrayCache;
+use OCP\ICache;
+use OCP\ICacheFactory;
+use OCP\IConfig;
+
+class BinaryFinderTest extends TestCase {
+	private ICache $cache;
+	private ICacheFactory $cacheFactory;
+	private $oldEnv;
+
+	protected function setUp(): void {
+		$this->oldEnv = getenv('PATH');
+		// BinaryFinder always includes the "PATH" environment variable into the search path,
+		// which we want to avoid in this test because they are not usually found in webserver
+		// deployments.
+		putenv('PATH=""');
+		$this->cacheFactory = $this->createMock(ICacheFactory::class);
+		$this->cache = new ArrayCache();
+		$this->cacheFactory->method('createLocal')->with('findBinaryPath')->willReturn($this->cache);
+	}
+
+	protected function tearDown(): void {
+		putenv('PATH=' . $this->oldEnv);
+	}
+
+	public function testDefaultFindsCat() {
+		$config = $this->createMock(IConfig::class);
+		$config
+			->method('getSystemValue')
+			->with('binary_search_paths', $this->anything())
+			->will($this->returnCallback(function ($key, $default) {
+				return $default;
+			}));
+		$finder = new BinaryFinder($this->cacheFactory, $config);
+		$this->assertEquals($finder->findBinaryPath('cat'), '/usr/bin/cat');
+		$this->assertEquals($this->cache->get('cat'), '/usr/bin/cat');
+	}
+
+	public function testDefaultDoesNotFindCata() {
+		$config = $this->createMock(IConfig::class);
+		$config
+			->method('getSystemValue')
+			->with('binary_search_paths', $this->anything())
+			->will($this->returnCallback(function ($key, $default) {
+				return $default;
+			}));
+		$finder = new BinaryFinder($this->cacheFactory, $config);
+		$this->assertFalse($finder->findBinaryPath('cata'));
+		$this->assertFalse($this->cache->get('cata'));
+	}
+
+	public function testCustomPathFindsCat() {
+		$config = $this->createMock(IConfig::class);
+		$config
+			->method('getSystemValue')
+			->with('binary_search_paths', $this->anything())
+			->willReturn(['/usr/bin']);
+		$finder = new BinaryFinder($this->cacheFactory, $config);
+		$this->assertEquals($finder->findBinaryPath('cat'), '/usr/bin/cat');
+		$this->assertEquals($this->cache->get('cat'), '/usr/bin/cat');
+	}
+
+	public function testWrongCustomPathDoesNotFindCat() {
+		$config = $this->createMock(IConfig::class);
+		$config
+			->method('getSystemValue')
+			->with('binary_search_paths')
+			->willReturn(['/wrong']);
+		$finder = new BinaryFinder($this->cacheFactory, $config);
+		$this->assertFalse($finder->findBinaryPath('cat'));
+		$this->assertFalse($this->cache->get('cat'));
+	}
+}