From d5a074cdc6041e98d2d0c34b17c3759d014e7660 Mon Sep 17 00:00:00 2001
From: Reno Reckling <e-github@wthack.de>
Date: Wed, 6 Mar 2024 14:24:00 +0100
Subject: [PATCH] feat: make search path for BinaryFinder customizable.

This feature is important for nextcloud running on
distributions like NixOS, where all the standard
search paths do not exist.

Also added tests.

This fixes issue #43922

Signed-off-by: Reno Reckling <e-github@wthack.de>
---
 config/config.sample.php       | 16 +++++++
 lib/private/BinaryFinder.php   | 30 +++++++-----
 tests/lib/BinaryFinderTest.php | 84 ++++++++++++++++++++++++++++++++++
 3 files changed, 119 insertions(+), 11 deletions(-)
 create mode 100644 tests/lib/BinaryFinderTest.php

diff --git a/config/config.sample.php b/config/config.sample.php
index c1961f7d250b5..21a77a8cbf646 100644
--- a/config/config.sample.php
+++ b/config/config.sample.php
@@ -2533,4 +2533,20 @@
  * 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',
+],
 ];
diff --git a/lib/private/BinaryFinder.php b/lib/private/BinaryFinder.php
index f7ac7a5195cac..1ca760a677491 100644
--- a/lib/private/BinaryFinder.php
+++ b/lib/private/BinaryFinder.php
@@ -11,15 +11,28 @@
 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,17 +50,12 @@ public function findBinaryPath(string $program) {
 		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;
+				$result = false
 			}
 		}
 		// store the value for 5 minutes
diff --git a/tests/lib/BinaryFinderTest.php b/tests/lib/BinaryFinderTest.php
new file mode 100644
index 0000000000000..3ad7fb48430bb
--- /dev/null
+++ b/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'));
+	}
+}