fix(config): do not echo secrets when setting config values [full-ci]

changelog/unreleased/41779

@@ -0,0 +1,19 @@
+Bugfix: Do not echo secrets when setting config values via occ
+
+config:system:set and config:app:set printed the value that had just been
+written back to stdout, so every secret configured through occ ended up in the
+terminal scrollback, the container log or the CI log of whoever ran the command.
+WOPI signing keys and the JWT secrets of apps leaked out of Docker deployments
+that configure them from a startup hook this way.
+
+Both commands now print a placeholder instead of the value when the config key
+holds a secret. Recognition reuses the existing list of sensitive keys in
+OC\SystemConfig, which gained an accessor telling whether a key path is
+sensitive, and falls back to matching the key name against the patterns
+credential, key, passwd, password, pwd, salt, secret and token. That fallback
+covers the keys of apps, which core does not know, such as wopi.token.key or
+jwt_secret. Boolean values keep being shown, as they cannot hold a secret. Only
+the confirmation output changed, the stored value is written as before.
+
+https://github.com/owncloud/core/issues/41779
+https://github.com/owncloud/core/pull/41780

core/Command/Config/App/SetConfig.php

@@ -22,13 +22,16 @@
 namespace OC\Core\Command\Config\App;
 
 use OC\Core\Command\Base;
+use OC\Core\Command\Config\SensitiveValueTrait;
 use OCP\IConfig;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class SetConfig extends Base {
+	use SensitiveValueTrait;
+
 	/** * @var IConfig */
 	protected $config;
 
@@ -93,7 +96,11 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 		$configValue = $input->getOption('value');
 		$this->config->setAppValue($appName, $configName, $configValue);
 
-		$output->writeln('<info>Config value ' . $configName . ' for app ' . $appName . ' set to ' . $configValue . '</info>');
+		// secrets are never echoed, so that they do not end up in the terminal
+		// scrollback or in any log tailing it
+		$readableValue = $this->isSensitiveKeyName($configName) ? IConfig::SENSITIVE_VALUE : $configValue;
+
+		$output->writeln('<info>Config value ' . $configName . ' for app ' . $appName . ' set to ' . $readableValue . '</info>');
 		return 0;
 	}
 }

core/Command/Config/SensitiveValueTrait.php

@@ -0,0 +1,70 @@
+<?php
+/**
+ * @author Thomas Müller <thomas.mueller@tmit.eu>
+ *
+ * @copyright Copyright (c) 2026, ownCloud GmbH
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OC\Core\Command\Config;
+
+/**
+ * Recognizes config keys which hold secrets, so that commands never echo their
+ * value back to the terminal, the container log or the CI log.
+ *
+ * The authoritative list of known core config keys lives in
+ * \OC\SystemConfig::$sensitiveValues and is exact match only. It cannot cover
+ * the keys of apps - core does not know them - so these name patterns act as a
+ * fallback for anything not on that list, e.g. "wopi.token.key" or the
+ * "jwt_secret" of an app.
+ */
+trait SensitiveValueTrait {
+	/**
+	 * Substrings which mark a config key as holding a secret. Matched case
+	 * insensitively against the key name.
+	 *
+	 * @var string[]
+	 */
+	private static $sensitiveNamePatterns = [
+		'credential',
+		'key',
+		'passwd',
+		'password',
+		'pwd',
+		'salt',
+		'secret',
+		'token',
+	];
+
+	/**
+	 * Checks whether any of the given config key names indicates a secret.
+	 *
+	 * @param string ...$names
+	 * @return bool
+	 */
+	protected function isSensitiveKeyName(...$names) {
+		foreach ($names as $name) {
+			$name = \strtolower((string) $name);
+			foreach (self::$sensitiveNamePatterns as $pattern) {
+				if (\strpos($name, $pattern) !== false) {
+					return true;
+				}
+			}
+		}
+
+		return false;
+	}
+}

core/Command/Config/System/SetConfig.php

@@ -23,13 +23,17 @@
 namespace OC\Core\Command\Config\System;
 
 use OC\Core\Command\Base;
+use OC\Core\Command\Config\SensitiveValueTrait;
 use OC\SystemConfig;
+use OCP\IConfig;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 
 class SetConfig extends Base {
+	use SensitiveValueTrait;
+
 	/** * @var SystemConfig */
 	protected $systemConfig;
 
@@ -77,7 +81,9 @@ protected function configure() {
 	protected function execute(InputInterface $input, OutputInterface $output): int {
 		$configNames = $input->getArgument('name');
 		$configName = $configNames[0];
-		$configValue = $this->castValue($input->getOption('value'), $input->getOption('type'));
+		$sensitive = $this->systemConfig->isSensitiveKey($configNames)
+			|| $this->isSensitiveKeyName(...$configNames);
+		$configValue = $this->castValue($input->getOption('value'), $input->getOption('type'), $sensitive);
 		$updateOnly = $input->getOption('update-only');
 
 		if ($configName === '') {
@@ -111,10 +117,12 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 	/**
 	 * @param string $value
 	 * @param string $type
+	 * @param bool $sensitive when true the readable value holds a placeholder
+	 *                        instead of the value itself
 	 * @return mixed
 	 * @throws \InvalidArgumentException
 	 */
-	protected function castValue($value, $type) {
+	protected function castValue($value, $type, $sensitive = false) {
 		switch ($type) {
 			case 'integer':
 			case 'int':
@@ -123,7 +131,7 @@ protected function castValue($value, $type) {
 				}
 				return [
 					'value' => (int) $value,
-					'readable-value' => 'integer ' . (int) $value,
+					'readable-value' => 'integer ' . $this->readableValue((int) $value, $sensitive),
 				];
 
 			case 'double':
@@ -133,11 +141,13 @@ protected function castValue($value, $type) {
 				}
 				return [
 					'value' => (double) $value,
-					'readable-value' => 'double ' . (double) $value,
+					'readable-value' => 'double ' . $this->readableValue((double) $value, $sensitive),
 				];
 
 			case 'boolean':
 			case 'bool':
+				// a boolean cannot hold a secret - both of its values are
+				// public knowledge - so it is never masked
 				$value = \strtolower($value);
 				switch ($value) {
 					case 'true':
@@ -167,7 +177,7 @@ protected function castValue($value, $type) {
 				$value = (string) $value;
 				return [
 					'value' => $value,
-					'readable-value' => ($value === '') ? 'empty string' : 'string ' . $value,
+					'readable-value' => ($value === '') ? 'empty string' : 'string ' . $this->readableValue($value, $sensitive),
 				];
 
 			case 'json':
@@ -177,14 +187,26 @@ protected function castValue($value, $type) {
 				}
 				return [
 					'value' => $decodedJson,
-					'readable-value' => 'json ' . $value,
+					'readable-value' => 'json ' . $this->readableValue($value, $sensitive),
 				];
 
 			default:
 				throw new \InvalidArgumentException('Invalid type');
 		}
 	}
 
+	/**
+	 * The value as it is shown to the user - secrets are never echoed, so that
+	 * they do not end up in the terminal scrollback or in any log tailing it.
+	 *
+	 * @param mixed $value
+	 * @param bool $sensitive
+	 * @return string
+	 */
+	private function readableValue($value, $sensitive) {
+		return $sensitive ? IConfig::SENSITIVE_VALUE : (string) $value;
+	}
+
 	/**
 	 * @param array $configNames
 	 * @param mixed $existingValues

lib/private/SystemConfig.php

@@ -141,6 +141,48 @@ public function getFilteredValue($key, $default = '') {
 		return $value;
 	}
 
+	/**
+	 * Checks whether the value stored under the given key path holds sensitive
+	 * data, based on the very same list that getFilteredValue() uses.
+	 *
+	 * A key is reported as sensitive when it is marked as such itself, or when
+	 * it is the parent of a sensitive key - setting or printing the parent
+	 * passes the nested secret along as well.
+	 *
+	 * @param array $keys key path, e.g. ['redis', 'password']
+	 * @return bool
+	 */
+	public function isSensitiveKey(array $keys) {
+		$definition = $this->sensitiveValues;
+
+		foreach ($keys as $key) {
+			if (!\is_array($definition)) {
+				return false;
+			}
+
+			if (!isset($definition[$key])) {
+				/*
+				 * The 0 key indicates a possibly repeating array of actual
+				 * entries 0,1,2,3... - they all share the same definition.
+				 */
+				if (\is_numeric($key) && isset($definition[0])) {
+					$definition = $definition[0];
+					continue;
+				}
+				return false;
+			}
+
+			$definition = $definition[$key];
+
+			if ($definition === true) {
+				return true;
+			}
+		}
+
+		// the remaining definition still holds sensitive keys below this path
+		return $keys !== [] && \is_array($definition);
+	}
+
 	/**
 	 * Delete a system wide defined value
 	 *

tests/Core/Command/Config/App/SetConfigTest.php

@@ -22,6 +22,7 @@
 namespace Tests\Core\Command\Config\App;
 
 use OC\Core\Command\Config\App\SetConfig;
+use OCP\IConfig;
 use Test\TestCase;
 
 class SetConfigTest extends TestCase {
@@ -113,4 +114,65 @@ public function testSet($configName, $newValue, $configExists, $updateOnly, $upd
 
 		self::invokePrivate($this->command, 'execute', [$this->consoleInput, $this->consoleOutput]);
 	}
+
+	public function sensitiveValueProvider() {
+		return [
+			['jwt_secret', 'VmNz3qhMWjsxnw4rMPpFxnM7C9hg9Xw', true],
+			['token', 'VmNz3qhMWjsxnw4rMPpFxnM7C9hg9Xw', true],
+			['wopi_key', 'VmNz3qhMWjsxnw4rMPpFxnM7C9hg9Xw', true],
+			['smtp_password', 'VmNz3qhMWjsxnw4rMPpFxnM7C9hg9Xw', true],
+			['bind_pwd', 'VmNz3qhMWjsxnw4rMPpFxnM7C9hg9Xw', true],
+			['user_salt', 'VmNz3qhMWjsxnw4rMPpFxnM7C9hg9Xw', true],
+			['api_credentials', 'VmNz3qhMWjsxnw4rMPpFxnM7C9hg9Xw', true],
+			// name matching is case insensitive
+			['JWT_SECRET', 'VmNz3qhMWjsxnw4rMPpFxnM7C9hg9Xw', true],
+			// harmless keys keep showing the value
+			['DocumentServerUrl', 'https://example.com/onlyoffice', false],
+			['enabled', 'yes', false],
+		];
+	}
+
+	/**
+	 * @dataProvider sensitiveValueProvider
+	 *
+	 * @param string $configName
+	 * @param string $newValue
+	 * @param bool $expectMasked
+	 */
+	public function testSensitiveValueIsNotEchoed($configName, $newValue, $expectMasked) {
+		$this->config->method('getAppKeys')
+			->with('app-name')
+			->willReturn([$configName]);
+
+		$this->consoleInput->method('getArgument')
+			->willReturnMap([
+				['app', 'app-name'],
+				['name', $configName],
+			]);
+		$this->consoleInput->method('getOption')
+			->with('value')
+			->willReturn($newValue);
+		$this->consoleInput->method('hasParameterOption')
+			->with('--update-only')
+			->willReturn(false);
+
+		$message = null;
+		$this->consoleOutput->expects($this->once())
+			->method('writeln')
+			->willReturnCallback(function ($line) use (&$message) {
+				$message = $line;
+			});
+
+		self::invokePrivate($this->command, 'execute', [$this->consoleInput, $this->consoleOutput]);
+
+		$this->assertStringContainsString($configName, $message);
+		$this->assertStringContainsString('app-name', $message);
+		if ($expectMasked) {
+			$this->assertStringNotContainsString($newValue, $message);
+			$this->assertStringContainsString(IConfig::SENSITIVE_VALUE, $message);
+		} else {
+			$this->assertStringContainsString($newValue, $message);
+			$this->assertStringNotContainsString(IConfig::SENSITIVE_VALUE, $message);
+		}
+	}
 }