diff --git a/bootstrap/helpers/parsers.php b/bootstrap/helpers/parsers.php index 765ec995e8..991023b621 100644 --- a/bootstrap/helpers/parsers.php +++ b/bootstrap/helpers/parsers.php @@ -196,18 +196,38 @@ function validateComposeArrayVolumeSource(string $source): void } /** - * Splits a top-level network `name:` that is one whole Compose variable (`${VAR}`, `${VAR:-default}` - * or `${VAR-default}`), such as an external network that differs per server. + * Matches one braced Compose variable: `${VAR}`, `${VAR:-default}`, `${VAR-default}`, + * `${VAR:?error}`, `${VAR?error}`, `${VAR:+value}` or `${VAR+value}`. + */ +const COMPOSE_NETWORK_VARIABLE_PATTERN = '\$\{([A-Za-z_][A-Za-z0-9_]*)(?::?([-?+])([^}]*))?\}'; + +/** + * Splits a top-level network `name:` that is one whole Compose variable (see + * COMPOSE_NETWORK_VARIABLE_PATTERN), such as an external network that differs per server. Only the + * `-` operators give a default; the text after `?` is an error message and after `+` a value that + * Compose uses only when the variable is set. * * @return array{variable: string, default: ?string}|null */ function composeNetworkNameVariable(string $name): ?array { - if (preg_match('/\A\$\{([A-Za-z_][A-Za-z0-9_]*)(?::?-([^}]*))?\}\z/', $name, $matches) !== 1) { + if (preg_match('/\A'.COMPOSE_NETWORK_VARIABLE_PATTERN.'\z/', $name, $matches) !== 1) { return null; } - return ['variable' => $matches[1], 'default' => $matches[2] ?? null]; + return ['variable' => $matches[1], 'default' => ($matches[2] ?? '') === '-' ? $matches[3] : null]; +} + +/** + * Tells if the text after the operator of a Compose variable is safe. A default or alternative value + * becomes part of the network name; an error message only shows in the Compose error, so it can + * also contain spaces and simple punctuation. + */ +function isSafeComposeNetworkVariableOperand(string $operator, string $operand): bool +{ + $pattern = $operator === '?' ? '/\A[A-Za-z0-9 _.,:!-]*\z/' : '/\A[A-Za-z0-9_.-]*\z/'; + + return preg_match($pattern, $operand) === 1; } /** @@ -254,20 +274,19 @@ function validateComposeNetworkNameField(string $name): void } /** - * Allows $VAR, ${VAR}, ${VAR:-default} and ${VAR-default} with safe defaults in a network value that Compose interpolates. + * Allows $VAR and the braced variables of COMPOSE_NETWORK_VARIABLE_PATTERN with safe operands in a network value that Compose interpolates. */ function validateComposeNetworkNameWithVariables(string $name, string $context, ?string $serviceName = null): void { $variable = composeNetworkNameVariable($name); - if ($variable !== null) { - $isValid = $variable['default'] === null || ValidationPatterns::isValidDockerNetwork($variable['default']); + if ($variable !== null && $variable['default'] !== null) { + $isValid = ValidationPatterns::isValidDockerNetwork($variable['default']); } else { $hasSafeDefaults = true; $withoutVariables = preg_replace_callback( - '/\$\{([A-Za-z_][A-Za-z0-9_]*)(?::?-([^}]*))?\}|\$[A-Za-z_][A-Za-z0-9_]*/', + '/'.COMPOSE_NETWORK_VARIABLE_PATTERN.'|\$[A-Za-z_][A-Za-z0-9_]*/', function (array $matches) use (&$hasSafeDefaults): string { - $default = $matches[2] ?? ''; - if ($default !== '' && preg_match('/\A[A-Za-z0-9_.-]+\z/', $default) !== 1) { + if (! isSafeComposeNetworkVariableOperand($matches[2] ?? '', $matches[3] ?? '')) { $hasSafeDefaults = false; } diff --git a/tests/Feature/ApplicationNetworkNameVariableTest.php b/tests/Feature/ApplicationNetworkNameVariableTest.php index 0a5bf190ec..4e1cebe8a2 100644 --- a/tests/Feature/ApplicationNetworkNameVariableTest.php +++ b/tests/Feature/ApplicationNetworkNameVariableTest.php @@ -170,6 +170,28 @@ YAML); expect($application->environment_variables()->where('key', 'SHARED_NETWORK')->exists())->toBeFalse(); }); +test('required network name variables are created without their error message as value', function () { + $application = makeComposeApplicationWithNetworks(<<<'YAML' + shared: + external: true + name: ${SHARED_NETWORK?error} + other: + name: ${OTHER_NETWORK:+other-network} + plain: + name: plain-network +YAML); + + $application->parse(); + + $variables = $application->environment_variables()->get()->keyBy('key'); + + expect($variables->has('SHARED_NETWORK'))->toBeTrue() + ->and((string) $variables->get('SHARED_NETWORK')->value)->toBe('') + ->and($variables->has('OTHER_NETWORK'))->toBeTrue() + ->and((string) $variables->get('OTHER_NETWORK')->value)->toBe('') + ->and($application->fresh()->docker_compose)->toContain('${SHARED_NETWORK?error}'); +}); + test('the deployment .env of a compose application contains the network name variable', function () { $application = makeComposeApplicationWithNetworks(<<<'YAML' shared: diff --git a/tests/Unit/DockerComposeInjectionCompatibilityTest.php b/tests/Unit/DockerComposeInjectionCompatibilityTest.php index 8192a41ddd..609fdf2fcf 100644 --- a/tests/Unit/DockerComposeInjectionCompatibilityTest.php +++ b/tests/Unit/DockerComposeInjectionCompatibilityTest.php @@ -241,8 +241,18 @@ test('service network list entries accept the same variables as network names', 'plain variable' => ['$NET'], 'variable mixed with text' => ['${COMPOSE_PROJECT_NAME}_default'], 'text with a variable default' => ['app-${ENV:-prod}'], + 'required variable' => ['${NET?error}'], + 'required non-empty variable with a message' => ['${NET:?NET must be set}'], + 'variable with an alternative value' => ['${NET:+proxy}'], + 'text with a required variable' => ['app-${ENV?error}'], ]); +test('a network name field can be a required variable', function () { + $compose = "services:\n web:\n image: nginx\n networks:\n - example\nnetworks:\n example:\n name: '\${CONTAINER_NAME?error}'\n"; + + expect(fn () => validateDockerComposeForInjection($compose))->not->toThrow(Exception::class); +}); + test('unsafe service network list entries with variables are rejected', function (string $network) { $compose = "services:\n web:\n image: nginx\n networks:\n - '{$network}'\n"; @@ -254,6 +264,9 @@ test('unsafe service network list entries with variables are rejected', function 'default with command substitution' => ['${NET:-$(id)}'], 'default with a space' => ['${NET:-x y}'], 'default with backticks' => ['${NET:-`id`}'], + 'error message with command substitution' => ['${NET?$(id)}'], + 'error message with a separator' => ['${NET:?x;id}'], + 'alternative value with a space' => ['${NET:+x y}'], ]); test('a service network key with a variable is rejected because Compose does not interpolate keys', function () { diff --git a/tests/Unit/PreSaveValidationTest.php b/tests/Unit/PreSaveValidationTest.php index ac9d8d2adc..9e0e9e7609 100644 --- a/tests/Unit/PreSaveValidationTest.php +++ b/tests/Unit/PreSaveValidationTest.php @@ -288,6 +288,8 @@ YAML; 'variable' => ['${SHARED_NETWORK}'], 'variable with a default' => ['${SHARED_NETWORK:-traefik_public}'], 'variable with an unset-only default' => ['${SHARED_NETWORK-traefik-public.1}'], + 'required variable' => ['${SHARED_NETWORK?error}'], + 'required non-empty variable' => ['${SHARED_NETWORK:?missing}'], ]); test('validateDockerComposeForInjection still blocks unsafe compose network names with variables', function (string $name, string $where) { @@ -304,7 +306,7 @@ test('validateDockerComposeForInjection still blocks unsafe compose network name 'text around a variable with command substitution' => ['prefix_${NET}$(id)', 'name'], 'nested variable' => ['${NET:-${OTHER}}', 'name'], 'invalid variable name' => ['${1NET}', 'name'], - 'required-variable form' => ['${NET:?missing}', 'name'], + 'required variable with command substitution in the message' => ['${NET:?$(id)}', 'name'], 'newline' => ["\${NET}\nid", 'name'], 'variable as network key' => ['${NET}', 'key'], ]);