Do not deploy ExApp environment variables with an empty default as the literal string Array

lib/Service/ExAppEnvVarsHelper.php

@@ -0,0 +1,72 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\AppAPI\Service;
+
+use InvalidArgumentException;
+
+/**
+ * Normalize declared ExApp environment variables from info.xml before they are deployed and persisted.
+ *
+ * The manifest is parsed with a simplexml/json roundtrip, where an empty element
+ * (`<default></default>` or `<default/>`) becomes an empty array instead of an empty string.
+ * Left as-is, such a value passes the "drop variables with an empty value" filter and is later
+ * stringified to the literal `Array` in the container environment.
+ *
+ * The helper produces a canonical NAME => {name, displayName, description, default, value} map with
+ * every field a string, applies caller overrides (occ `--env`, UI deploy options, stored deploy
+ * options on update), and drops variables whose final value is empty.
+ */
+class ExAppEnvVarsHelper {
+	/**
+	 * @param array $variables raw `environment-variables.variable` entries: a list, or a single entry as produced by SimpleXML for one `<variable>` element
+	 * @param array $overrides deploy-option overrides, NAME => value or NAME => ['value' => value]; overrides for undeclared names are ignored
+	 * @return array normalized NAME-keyed map, entries with an empty final value removed
+	 * @throws InvalidArgumentException on the first malformed variable; message identifies the entry and field
+	 */
+	public static function normalizeAndValidate(array $variables, array $overrides): array {
+		if (!array_is_list($variables)) {
+			$variables = [$variables];
+		}
+		$envVars = [];
+		foreach ($variables as $index => $variable) {
+			if (!is_array($variable)) {
+				throw new InvalidArgumentException(sprintf('variable #%d: entry must be an object, got %s', $index, get_debug_type($variable)));
+			}
+			$name = $variable['name'] ?? null;
+			if (!is_string($name) || trim($name) === '') {
+				throw new InvalidArgumentException(sprintf("variable #%d: 'name' must be a non-empty string, got %s", $index, get_debug_type($name)));
+			}
+			$default = self::toString($variable['default'] ?? '');
+			$envVars[$name] = [
+				'name' => $name,
+				'displayName' => self::toString($variable['display-name'] ?? ''),
+				'description' => self::toString($variable['description'] ?? ''),
+				'default' => $default,
+				'value' => $default,
+			];
+		}
+		foreach ($overrides as $name => $value) {
+			if (array_key_exists($name, $envVars)) {
+				$envVars[$name]['value'] = self::toString($value['value'] ?? $value ?? '');
+			}
+		}
+		return array_filter($envVars, static function (array $envVar) {
+			return $envVar['value'] !== '';
+		});
+	}
+
+	/**
+	 * An empty XML element arrives as [] after the simplexml/json roundtrip: treat any
+	 * non-scalar as an empty string so the empty-value filter applies to every input shape.
+	 */
+	private static function toString(mixed $value): string {
+		return is_scalar($value) ? (string)$value : '';
+	}
+}

lib/Service/ExAppService.php

@@ -261,7 +261,7 @@ public function getAppInfo(string $appId, ?string $infoXml, ?string $jsonInfo, ?
 			# fill 'id' if it is missing(this field was called `appid` in previous versions in json)
 			$appInfo['id'] = $appInfo['id'] ?? $appId;
 			# during manual install JSON can have all values at root level
-			foreach (['docker-install', 'translations_folder', 'routes', 'k8s-service-roles'] as $key) {
+			foreach (['docker-install', 'translations_folder', 'routes', 'k8s-service-roles', 'environment-variables'] as $key) {
 				if (isset($appInfo[$key])) {
 					$appInfo['external-app'][$key] = $appInfo[$key];
 					unset($appInfo[$key]);
@@ -291,34 +291,6 @@ public function getAppInfo(string $appId, ?string $infoXml, ?string $jsonInfo, ?
 					$appInfo['external-app']['routes'] = [$appInfo['external-app']['routes']['route']];
 				}
 			}
-			// Advanced deploy options
-			if (isset($appInfo['external-app']['environment-variables']['variable'])) {
-				$envVars = [];
-				if (!isset($appInfo['external-app']['environment-variables']['variable'][0])) {
-					$appInfo['external-app']['environment-variables']['variable'] = [$appInfo['external-app']['environment-variables']['variable']];
-				}
-				foreach ($appInfo['external-app']['environment-variables']['variable'] as $envVar) {
-					$envVars[$envVar['name']] = [
-						'name' => $envVar['name'],
-						'displayName' => $envVar['display-name'] ?? '',
-						'description' => $envVar['description'] ?? '',
-						'default' => $envVar['default'] ?? '',
-						'value' => $envVar['default'] ?? '',
-					];
-				}
-				if (isset($deployOptions['environment_variables']) && count(array_keys($deployOptions['environment_variables'])) > 0) {
-					// override with given deploy options values
-					foreach ($deployOptions['environment_variables'] as $key => $value) {
-						if (array_key_exists($key, $envVars)) {
-							$envVars[$key]['value'] = $value['value'] ?? $value ?? '';
-						}
-					}
-				}
-				$envVars = array_filter($envVars, function ($envVar) {
-					return $envVar['value'] !== '';
-				});
-				$appInfo['external-app']['environment-variables'] = $envVars;
-			}
 			if (isset($appInfo['external-app']['k8s-service-roles']['role'])) {
 				$roles = $appInfo['external-app']['k8s-service-roles']['role'];
 				if (!isset($roles[0])) {
@@ -344,6 +316,21 @@ public function getAppInfo(string $appId, ?string $infoXml, ?string $jsonInfo, ?
 				}
 			}
 		}
+		// Advanced deploy options; runs for both the XML and the JSON path so the
+		// environment-variables contract of the returned appInfo is input-format independent
+		if (isset($appInfo['external-app']['environment-variables']['variable'])) {
+			$variables = $appInfo['external-app']['environment-variables']['variable'];
+			if (!is_array($variables)) {
+				return ['error' => sprintf("ExApp '%s' has invalid environment variable definition. 'variable' must be an object or a list of objects, got %s", $appId, get_debug_type($variables))];
+			}
+			try {
+				$appInfo['external-app']['environment-variables'] = ExAppEnvVarsHelper::normalizeAndValidate(
+					$variables, $deployOptions['environment_variables'] ?? []
+				);
+			} catch (InvalidArgumentException $e) {
+				return ['error' => sprintf("ExApp '%s' has invalid environment variable definition. %s", $appId, $e->getMessage())];
+			}
+		}
 		if (isset($appInfo['external-app']['routes'])) {
 			if (!is_array($appInfo['external-app']['routes'])) {
 				return ['error' => sprintf("ExApp '%s' has invalid route definition. 'routes' must be a list of route objects, got %s", $appId, get_debug_type($appInfo['external-app']['routes']))];

tests/php/Service/ExAppEnvVarsHelperTest.php

@@ -0,0 +1,145 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\AppAPI\Tests\php\Service;
+
+use InvalidArgumentException;
+use OCA\AppAPI\Service\ExAppEnvVarsHelper;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+class ExAppEnvVarsHelperTest extends TestCase {
+
+	/**
+	 * Regression test for https://github.com/nextcloud/app_api/issues/969: an empty <default>
+	 * element must not survive as an empty array and end up as the literal string `Array` in
+	 * the container environment. The input is produced by the same simplexml/json roundtrip
+	 * getAppInfo uses, so the [] shape is real, not hand-crafted.
+	 */
+	#[DataProvider('emptyDefaultXmlProvider')]
+	public function testEmptyDefaultElementFromRealXmlIsDropped(string $xml): void {
+		$parsed = json_decode(json_encode((array)simplexml_load_string($xml)), true);
+		$variables = $parsed['environment-variables']['variable'];
+
+		// lock in the SimpleXML behavior the bug depends on: empty element parses to []
+		self::assertSame([], $variables['default']);
+
+		self::assertSame([], ExAppEnvVarsHelper::normalizeAndValidate($variables, []));
+	}
+
+	public static function emptyDefaultXmlProvider(): array {
+		return [
+			'<default></default>' => [
+				'<external-app><environment-variables><variable>'
+				. '<name>EMPTY_ELEM</name><display-name>Empty</display-name><description>d</description><default></default>'
+				. '</variable></environment-variables></external-app>',
+			],
+			'<default/>' => [
+				'<external-app><environment-variables><variable>'
+				. '<name>EMPTY_ELEM</name><display-name>Empty</display-name><description>d</description><default/>'
+				. '</variable></environment-variables></external-app>',
+			],
+		];
+	}
+
+	#[DataProvider('validVariablesProvider')]
+	public function testNormalizeAndValidate(array $variables, array $overrides, array $expected): void {
+		self::assertSame($expected, ExAppEnvVarsHelper::normalizeAndValidate($variables, $overrides));
+	}
+
+	public static function validVariablesProvider(): array {
+		return [
+			'single <variable> arrives as one object, not a list' => [
+				['name' => 'A', 'display-name' => 'Var A', 'description' => 'desc', 'default' => 'x'],
+				[],
+				['A' => ['name' => 'A', 'displayName' => 'Var A', 'description' => 'desc', 'default' => 'x', 'value' => 'x']],
+			],
+			'variable without <default> is dropped' => [
+				[['name' => 'A', 'display-name' => 'Var A', 'description' => 'desc']],
+				[],
+				[],
+			],
+			'empty-element default ([]) is dropped, sibling with a value survives' => [
+				[
+					['name' => 'EMPTY_ELEM', 'display-name' => 'Empty', 'description' => 'd', 'default' => []],
+					['name' => 'KEPT', 'display-name' => 'Kept', 'description' => 'd', 'default' => 'v'],
+				],
+				[],
+				['KEPT' => ['name' => 'KEPT', 'displayName' => 'Kept', 'description' => 'd', 'default' => 'v', 'value' => 'v']],
+			],
+			'empty-element display-name and description become empty strings' => [
+				[['name' => 'A', 'display-name' => [], 'description' => [], 'default' => 'x']],
+				[],
+				['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => 'x', 'value' => 'x']],
+			],
+			'override replaces the default value' => [
+				[['name' => 'A', 'default' => 'x']],
+				['A' => 'y'],
+				['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => 'x', 'value' => 'y']],
+			],
+			'override with empty value drops the variable' => [
+				[['name' => 'A', 'default' => 'x']],
+				['A' => ''],
+				[],
+			],
+			'override in stored deploy-options shape' => [
+				[['name' => 'A', 'default' => 'x']],
+				['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => 'x', 'value' => 'y']],
+				['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => 'x', 'value' => 'y']],
+			],
+			'stored pre-fix deploy option with [] value is dropped, not deployed as Array' => [
+				[['name' => 'A', 'default' => []]],
+				['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => [], 'value' => []]],
+				[],
+			],
+			'override for an undeclared variable is ignored' => [
+				[['name' => 'A', 'default' => 'x']],
+				['B' => 'y'],
+				['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => 'x', 'value' => 'x']],
+			],
+			'no variables declared' => [[], ['A' => 'y'], []],
+			'numeric override is canonicalized to string' => [
+				[['name' => 'A', 'default' => 'x']],
+				['A' => 123],
+				['A' => ['name' => 'A', 'displayName' => '', 'description' => '', 'default' => 'x', 'value' => '123']],
+			],
+		];
+	}
+
+	#[DataProvider('invalidVariablesProvider')]
+	public function testNormalizeAndValidateRejects(array $variables, string $expectedMessageFragment): void {
+		try {
+			ExAppEnvVarsHelper::normalizeAndValidate($variables, []);
+			self::fail('Expected InvalidArgumentException, none thrown');
+		} catch (InvalidArgumentException $e) {
+			self::assertStringContainsString($expectedMessageFragment, $e->getMessage());
+		}
+	}
+
+	public static function invalidVariablesProvider(): array {
+		return [
+			'entry is not an array' => [
+				['not-an-object'],
+				'variable #0: entry must be an object',
+			],
+			'missing name' => [
+				[['display-name' => 'X', 'default' => 'v']],
+				"variable #0: 'name' must be a non-empty string",
+			],
+			'empty <name/> element parses to an array' => [
+				[['name' => [], 'default' => 'v']],
+				"variable #0: 'name' must be a non-empty string",
+			],
+			'whitespace-only name' => [
+				[['name' => '  ', 'default' => 'v']],
+				"variable #0: 'name' must be a non-empty string",
+			],
+		];
+	}
+}