From 8686789655d14314a15a30de1d16faf6295b0c95 Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Mon, 27 Apr 2026 14:15:03 +0530 Subject: [PATCH 1/2] fix: coerce string CLI inputs for Boolean params CLI args arrive as strings via getopt. When a param's validator is `Boolean` (loose mode), strings like "true"/"false"/"1"/"0" pass validation but remain strings. If the action callback declares a `bool` parameter, PHP's implicit string-to-bool cast turns any non-empty string except "0" into `true` -- so `--commit=false` silently becomes `true` and the action runs as if `--commit=true` had been passed. Validators in utopia-php are pure (validate only, never mutate), so the coercion has to happen here at the dispatch boundary in `CLI::getParams()`. After validation, if the param's validator (unwrapping `Nullable`) is `Boolean` and the value is still a string, we run it through `filter_var(..., FILTER_VALIDATE_BOOLEAN)` before handing it to the callback. Bool defaults and non-string inputs pass through untouched, so this is a safe, additive change for existing callers. --- src/CLI/CLI.php | 43 +++++++++++++++++++- tests/CLI/CLITest.php | 93 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 1 deletion(-) diff --git a/src/CLI/CLI.php b/src/CLI/CLI.php index 0611d29..621d60b 100644 --- a/src/CLI/CLI.php +++ b/src/CLI/CLI.php @@ -7,6 +7,8 @@ use Utopia\DI\Container; use Utopia\Servers\Hook; use Utopia\Validator; +use Utopia\Validator\Boolean; +use Utopia\Validator\Nullable; class CLI { @@ -302,7 +304,7 @@ protected function getParams(Hook $hook): array $this->validate($key, $param, $value); - $params[$this->camelCaseIt($key)] = $value; + $params[$this->camelCaseIt($key)] = $this->coerce($param['validator'], $value); } foreach ($hook->getDependencies() as $dependency) { @@ -410,6 +412,45 @@ protected function validate(string $key, array $param, $value): void } } + /** + * Coerce string CLI inputs to native PHP types based on the param's validator. + * + * CLI args arrive as strings via getopt. When the validator is `Boolean` + * (loose mode), strings like "true"/"false"/"1"/"0" pass validation but + * remain strings. If the action callback declares a `bool` parameter, PHP's + * implicit string-to-bool cast turns any non-empty string except "0" into + * `true` -- so `--commit=false` silently becomes `true`. Validators in + * utopia-php are pure (validate only, never mutate), so the coercion has + * to happen here at the dispatch boundary. + * + * Only string inputs are coerced; bool defaults are passed through + * untouched. + * + * @param Validator|callable $validator + * @param mixed $value + * @return mixed + */ + protected function coerce(Validator|callable $validator, mixed $value): mixed + { + if (! is_string($value)) { + return $value; + } + + if (\is_callable($validator)) { + $validator = $validator(); + } + + while ($validator instanceof Nullable) { + $validator = $validator->getValidator(); + } + + if ($validator instanceof Boolean) { + return \filter_var($value, FILTER_VALIDATE_BOOLEAN); + } + + return $value; + } + public function setContainer(Container $container): self { $this->parentContainer = $container; diff --git a/tests/CLI/CLITest.php b/tests/CLI/CLITest.php index 078536f..dec4c13 100755 --- a/tests/CLI/CLITest.php +++ b/tests/CLI/CLITest.php @@ -7,6 +7,8 @@ use Utopia\CLI\CLI; use Utopia\DI\Container; use Utopia\Validator\ArrayList; +use Utopia\Validator\Boolean; +use Utopia\Validator\Nullable; use Utopia\Validator\Text; class CLITest extends TestCase @@ -289,6 +291,97 @@ public function testMatch() $this->assertEquals(null, $cli->match()); } + /** + * @return iterable + */ + public static function looseBooleanValuesProvider(): iterable + { + yield '"false" string' => ['false', false]; + yield '"true" string' => ['true', true]; + yield '"0" string' => ['0', false]; + yield '"1" string' => ['1', true]; + } + + /** + * Regression: --flag=false used to arrive as the literal string "false", + * which PHP's implicit string-to-bool cast turned into `true` at the + * `bool $flag` parameter boundary. The CLI dispatcher now coerces string + * inputs whose validator is `Boolean` to a real PHP bool. + * + * @dataProvider looseBooleanValuesProvider + */ + public function testBooleanParamCoercesStringInput(string $input, bool $expected): void + { + $captured = null; + + $cli = new CLI(new Generic(), ['test.php', 'build', '--commit='.$input]); + + $cli + ->task('build') + ->param('commit', false, new Boolean(true), 'Commit changes', true) + ->action(function (bool $commit) use (&$captured) { + $captured = $commit; + }); + + $cli->run(); + + $this->assertSame($expected, $captured); + } + + public function testBooleanParamUsesDefaultWhenOmitted(): void + { + $captured = null; + + $cli = new CLI(new Generic(), ['test.php', 'build']); + + $cli + ->task('build') + ->param('commit', false, new Boolean(true), 'Commit changes', true) + ->action(function (bool $commit) use (&$captured) { + $captured = $commit; + }); + + $cli->run(); + + $this->assertFalse($captured); + } + + public function testBooleanParamCoercionUnwrapsNullable(): void + { + $captured = 'untouched'; + + $cli = new CLI(new Generic(), ['test.php', 'build', '--commit=false']); + + $cli + ->task('build') + ->param('commit', null, new Nullable(new Boolean(true)), 'Commit changes', true) + ->action(function (bool $commit) use (&$captured) { + $captured = $commit; + }); + + $cli->run(); + + $this->assertFalse($captured); + } + + public function testNonBooleanValidatorPassesValueThroughUnchanged(): void + { + $captured = null; + + $cli = new CLI(new Generic(), ['test.php', 'build', '--name=false']); + + $cli + ->task('build') + ->param('name', '', new Text(64), 'A name') + ->action(function (string $name) use (&$captured) { + $captured = $name; + }); + + $cli->run(); + + $this->assertSame('false', $captured); + } + public function testEscaping() { ob_start(); From a6ae1eb443fc5e82dfe190f11c1ccb2f1d8db68a Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Mon, 27 Apr 2026 14:22:05 +0530 Subject: [PATCH 2/2] fix: preserve empty-string sentinel in coerce() Optional params with `--flag=` (or default `''`) bypass `validate()` entirely, so they reach `coerce()` un-validated. The previous version ran them through `filter_var($value, FILTER_VALIDATE_BOOLEAN)`, which silently returns `false` for empty strings -- a behaviour change for callers (e.g. Cloud's `Patch.php`) that use `''` as a three-state "not set" sentinel before resolving to `null`. Now coerce returns early for empty strings, and uses `FILTER_NULL_ON_FAILURE` for everything else so any unrecognised input falls back to the original value rather than getting silently downgraded to `false`. Spotted by Greptile review on PR #52. --- src/CLI/CLI.php | 15 ++++++++++----- tests/CLI/CLITest.php | 24 ++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/src/CLI/CLI.php b/src/CLI/CLI.php index 621d60b..e4c96e2 100644 --- a/src/CLI/CLI.php +++ b/src/CLI/CLI.php @@ -424,7 +424,10 @@ protected function validate(string $key, array $param, $value): void * to happen here at the dispatch boundary. * * Only string inputs are coerced; bool defaults are passed through - * untouched. + * untouched. Strings that `filter_var` doesn't recognise (including the + * empty string, which bypasses `validate()` for optional params) are + * passed through unchanged so callers that use `''` as a sentinel for + * "not set" keep working. * * @param Validator|callable $validator * @param mixed $value @@ -432,7 +435,7 @@ protected function validate(string $key, array $param, $value): void */ protected function coerce(Validator|callable $validator, mixed $value): mixed { - if (! is_string($value)) { + if (! is_string($value) || $value === '') { return $value; } @@ -444,11 +447,13 @@ protected function coerce(Validator|callable $validator, mixed $value): mixed $validator = $validator->getValidator(); } - if ($validator instanceof Boolean) { - return \filter_var($value, FILTER_VALIDATE_BOOLEAN); + if (! ($validator instanceof Boolean)) { + return $value; } - return $value; + $coerced = \filter_var($value, FILTER_VALIDATE_BOOLEAN, FILTER_NULL_ON_FAILURE); + + return $coerced === null ? $value : $coerced; } public function setContainer(Container $container): self diff --git a/tests/CLI/CLITest.php b/tests/CLI/CLITest.php index dec4c13..3abb6d9 100755 --- a/tests/CLI/CLITest.php +++ b/tests/CLI/CLITest.php @@ -364,6 +364,30 @@ public function testBooleanParamCoercionUnwrapsNullable(): void $this->assertFalse($captured); } + /** + * Empty-string params bypass `validate()` when optional, so they reach + * `coerce()` un-validated. We must NOT silently turn them into `false` + * (callers like Cloud's `Patch.php` use `''` as a "not set" sentinel and + * later resolve it to `null`/three-state). + */ + public function testBooleanParamPreservesEmptyStringSentinel(): void + { + $captured = 'untouched'; + + $cli = new CLI(new Generic(), ['test.php', 'build']); + + $cli + ->task('build') + ->param('commit', '', new Boolean(true), 'Commit changes', true) + ->action(function ($commit) use (&$captured) { + $captured = $commit; + }); + + $cli->run(); + + $this->assertSame('', $captured); + } + public function testNonBooleanValidatorPassesValueThroughUnchanged(): void { $captured = null;