From 59d3932655d405fb102406a3145cf7e1a85529dd Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Sat, 18 Jul 2026 22:22:22 +0100 Subject: [PATCH 1/2] Redact sensitive parameters in stack traces --- AGENTS.md | 18 +- CHANGELOG.md | 1 + UPGRADING.md | 12 ++ composer.json | 3 +- src/Exception/CommandException.php | 8 +- src/ServiceClient.php | 54 ++++-- tests/SensitiveParameterTest.php | 253 +++++++++++++++++++++++++++++ 7 files changed, 329 insertions(+), 20 deletions(-) create mode 100644 tests/SensitiveParameterTest.php diff --git a/AGENTS.md b/AGENTS.md index 6eecaea..7a9c98d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,8 +2,22 @@ ## Code and tooling -- All code must remain compatible with PHP 7.4, and PHPStan and PHP-CS-Fixer - must be run against PHP 7.4. +- All code must remain compatible with PHP 7.4. Run PHPStan and PHP-CS-Fixer + only under a PHP 7.4.x runtime; never run either tool under any other PHP + major.minor version. +- Use fully qualified `#[\SensitiveParameter]` on concrete executable + parameters when their established role normally carries a secret, + credential-bearing aggregate, or confidential Guzzle-owned container, and + the active frame can throw or invoke throwing code. +- Repeat the attribute on every qualifying owned caller, callee, concrete trait + method, and closure parameter. Do not add it to interfaces, abstract-only + declarations, pure/no-realistic-throw helpers, assignment-only sites, + arbitrary generic payloads, or completed non-recoverable derivatives. +- For PHP 7.4 compatibility, put `#[\SensitiveParameter]` on its own line and + the parameter on the following line, expand the complete parameter list, and + never add a comma after the final parameter. Native trace redaction starts on + PHP 8.2 and does not redact logs, messages, properties, wire traffic, captured + variables, return values, or the separate backtrace `$this`/`object`. - Always pass an explicit character list to `trim()`, `ltrim()`, and `rtrim()`; never rely on the default characters. - Handle `preg_*` engine failures: when the result is used as data, test for diff --git a/CHANGELOG.md b/CHANGELOG.md index a62a29d..398238a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ * Require service client response transformers to return `ResultInterface` values * Require command collections to be iterable and reject single commands in `executeAll()` and `executeAllAsync()` * Require custom middleware, transformers, and `executeAll()` callbacks to accept exact argument types instead of relying on scalar coercion +* Redact credential-bearing command and HTTP arguments from PHP 8.2+ backtraces * Preserve requests from PSR-18 request and network exceptions and from Guzzle transfer exceptions when wrapping command failures * Allow command exception constructors to accept any previous `Throwable` * Add generic PHPDoc return types to asynchronous service client APIs diff --git a/UPGRADING.md b/UPGRADING.md index 8ac9c33..3a6a86a 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -117,6 +117,18 @@ userland callbacks continue to work at runtime when PHP accepts them. `ServiceClient` no longer supports native PHP `serialize()` or `unserialize()`. Persist command names and parameter arrays instead of runtime client objects. +#### Sensitive Parameters and Backtraces + +Guzzle Command 2.0 marks credential-bearing command, request, response, and +exception parameters as sensitive. On PHP 8.2 and later, exception traces and +backtraces replace those argument values with `SensitiveParameterValue` +objects. PHP 7.4 through 8.1 retain the original trace arguments. + +This does not redact exception messages, application logs, HTTP traffic, +properties, captured variables, return values, or the separate `$this` object +in an explicit backtrace. Custom middleware and transformers must mark their +own sensitive parameters. + 1.0 from 0.8 ------------ diff --git a/composer.json b/composer.json index 612a83a..022d893 100644 --- a/composer.json +++ b/composer.json @@ -30,7 +30,8 @@ "guzzlehttp/promises": "^3.0@dev", "guzzlehttp/psr7": "^3.0@dev", "psr/http-client": "^1.0", - "symfony/polyfill-php80": "^1.25" + "symfony/polyfill-php80": "^1.25", + "symfony/polyfill-php82": "^1.27" }, "require-dev": { "bamarni/composer-bin-plugin": "^1.8.2", diff --git a/src/Exception/CommandException.php b/src/Exception/CommandException.php index 4dfd390..25b81e2 100644 --- a/src/Exception/CommandException.php +++ b/src/Exception/CommandException.php @@ -25,8 +25,12 @@ class CommandException extends \RuntimeException implements GuzzleException private ?ResponseInterface $response; - public static function fromPrevious(CommandInterface $command, \Exception $prev): self - { + public static function fromPrevious( + #[\SensitiveParameter] + CommandInterface $command, + #[\SensitiveParameter] + \Exception $prev + ): self { // If the exception is already a command exception, return it. if ($prev instanceof self && $command === $prev->getCommand()) { return $prev; diff --git a/src/ServiceClient.php b/src/ServiceClient.php index 15bf97e..f7eec12 100644 --- a/src/ServiceClient.php +++ b/src/ServiceClient.php @@ -67,21 +67,28 @@ public function getHandlerStack(): HandlerStack return $this->handlerStack; } - public function getCommand(string $name, array $params = []): CommandInterface - { + public function getCommand( + string $name, + #[\SensitiveParameter] + array $params = [] + ): CommandInterface { return new Command($name, $params, clone $this->handlerStack); } - public function execute(CommandInterface $command): ResultInterface - { + public function execute( + #[\SensitiveParameter] + CommandInterface $command + ): ResultInterface { return $this->executeAsync($command)->wait(); } /** * @return PromiseInterface */ - public function executeAsync(CommandInterface $command): PromiseInterface - { + public function executeAsync( + #[\SensitiveParameter] + CommandInterface $command + ): PromiseInterface { $stack = $command->getHandlerStack() ?: $this->handlerStack; /** @var callable(CommandInterface): PromiseInterface $handler */ @@ -103,8 +110,11 @@ public function executeAsync(CommandInterface $command): PromiseInterface * * @return array */ - public function executeAll(iterable $commands, array $options = []): array - { + public function executeAll( + #[\SensitiveParameter] + iterable $commands, + array $options = [] + ): array { $fulfilled = $options['fulfilled'] ?? null; $rejected = $options['rejected'] ?? null; @@ -145,8 +155,11 @@ public function executeAll(iterable $commands, array $options = []): array * * @return PromiseInterface */ - public function executeAllAsync(iterable $commands, array $options = []): PromiseInterface - { + public function executeAllAsync( + #[\SensitiveParameter] + iterable $commands, + array $options = [] + ): PromiseInterface { // A single command satisfies the iterable type but would be iterated // as a collection of its own parameters. if ($commands instanceof CommandInterface) { @@ -183,8 +196,11 @@ public function executeAllAsync(iterable $commands, array $options = []): Promis * * @see ServiceClientInterface::getCommand */ - public function __call(string $name, array $args) - { + public function __call( + string $name, + #[\SensitiveParameter] + array $args + ) { $args = isset($args[0]) ? $args[0] : []; if (str_ends_with($name, 'Async')) { $command = $this->getCommand(substr($name, 0, -5), $args); @@ -202,7 +218,10 @@ public function __call(string $name, array $args) */ private function createCommandHandler(): callable { - return function (CommandInterface $command): PromiseInterface { + return function ( + #[\SensitiveParameter] + CommandInterface $command + ): PromiseInterface { return Promise\Coroutine::of(function () use ($command): \Generator { // Prepare the HTTP options. $opts = $command['@http'] ?: []; @@ -226,8 +245,10 @@ private function createCommandHandler(): callable /** * Transforms a Command object into a Request object. */ - private function transformCommandToRequest(CommandInterface $command): RequestInterface - { + private function transformCommandToRequest( + #[\SensitiveParameter] + CommandInterface $command + ): RequestInterface { $transform = $this->commandToRequestTransformer; return $transform($command); @@ -238,8 +259,11 @@ private function transformCommandToRequest(CommandInterface $command): RequestIn * into a Result object. */ private function transformResponseToResult( + #[\SensitiveParameter] ResponseInterface $response, + #[\SensitiveParameter] RequestInterface $request, + #[\SensitiveParameter] CommandInterface $command ): ResultInterface { $transform = $this->responseToResultTransformer; diff --git a/tests/SensitiveParameterTest.php b/tests/SensitiveParameterTest.php new file mode 100644 index 0000000..b8a18d2 --- /dev/null +++ b/tests/SensitiveParameterTest.php @@ -0,0 +1,253 @@ +getName(); + }, $reflection->getParameters()); + foreach ($sensitiveParameters as $sensitiveParameter) { + self::assertContains($sensitiveParameter, $parameterNames); + } + $attributeCount += count($sensitiveParameters); + + continue; + } + + foreach ($reflection->getParameters() as $parameter) { + $attributes = $parameter->getAttributes(\SensitiveParameter::class); + $expected = in_array($parameter->getName(), $sensitiveParameters, true) + ? 1 + : 0; + + self::assertCount($expected, $attributes, $class.'::'.$method.'::$'.$parameter->getName()); + foreach ($attributes as $attribute) { + self::assertInstanceOf(\SensitiveParameter::class, $attribute->newInstance()); + } + $attributeCount += count($attributes); + } + } + + self::assertSame(12, $attributeCount); + } + + public function testCommandHandlerClosureParameterIsSensitive(): void + { + $client = new ServiceClient( + $this->createMock(HttpClientInterface::class), + static function (CommandInterface $command): RequestInterface { + return new Request('GET', '/'); + }, + static function ( + ResponseInterface $response, + RequestInterface $request, + CommandInterface $command + ): ResultInterface { + return new Result(); + } + ); + $factory = new \ReflectionMethod(ServiceClient::class, 'createCommandHandler'); + $factory->setAccessible(true); + $handler = $factory->invoke($client); + self::assertInstanceOf(\Closure::class, $handler); + + $parameters = (new \ReflectionFunction($handler))->getParameters(); + self::assertCount(1, $parameters); + self::assertSame('command', $parameters[0]->getName()); + if (\PHP_VERSION_ID < 80000) { + return; + } + self::assertCount(1, $parameters[0]->getAttributes(\SensitiveParameter::class)); + } + + public function testInterfacesAndAssignmentOnlyConstructorAreNotAnnotated(): void + { + if (\PHP_VERSION_ID < 80000) { + self::markTestSkipped('Attributes are not reflected before PHP 8.0.'); + } + + foreach ((new \ReflectionClass(ServiceClientInterface::class))->getMethods() as $method) { + foreach ($method->getParameters() as $parameter) { + self::assertCount(0, $parameter->getAttributes(\SensitiveParameter::class)); + } + } + + $constructor = new \ReflectionMethod(Command::class, '__construct'); + self::assertCount(0, $constructor->getParameters()[1]->getAttributes(\SensitiveParameter::class)); + } + + public function testResponseTransformerArgumentsAreRedactedFromTrace(): void + { + $this->requireNativeTraceRedaction(); + + $client = new ServiceClient( + $this->createMock(HttpClientInterface::class), + static function (CommandInterface $command): RequestInterface { + return new Request('GET', '/'); + }, + static function ( + ResponseInterface $response, + RequestInterface $request, + CommandInterface $command + ): ResultInterface { + throw new \RuntimeException('transform failed'); + } + ); + $method = new \ReflectionMethod(ServiceClient::class, 'transformResponseToResult'); + $method->setAccessible(true); + + $previous = ini_get('zend.exception_ignore_args'); + try { + self::assertNotFalse(ini_set('zend.exception_ignore_args', '0')); + $method->invoke( + $client, + new Response(500), + new Request('GET', '/?api_key=request-secret'), + new Command('GetSecret', ['api_key' => 'command-secret']) + ); + self::fail('Expected the response transformer to throw.'); + } catch (\RuntimeException $exception) { + $frame = self::findFrame($exception, ServiceClient::class, 'transformResponseToResult'); + self::assertCount(3, $frame['args']); + foreach ($frame['args'] as $argument) { + self::assertInstanceOf(\SensitiveParameterValue::class, $argument); + } + } finally { + if ($previous !== false) { + ini_set('zend.exception_ignore_args', $previous); + } + } + } + + public function testCommandExceptionFactoryArgumentsAreRedactedFromTrace(): void + { + $this->requireNativeTraceRedaction(); + + $command = $this->createMock(CommandInterface::class); + $previousException = new class($command) extends CommandException { + public function __construct(CommandInterface $command) + { + parent::__construct('previous-secret', $command); + } + + public function getCommand(): CommandInterface + { + throw new \RuntimeException('command access failed'); + } + }; + + $previous = ini_get('zend.exception_ignore_args'); + try { + self::assertNotFalse(ini_set('zend.exception_ignore_args', '0')); + CommandException::fromPrevious($command, $previousException); + self::fail('Expected command name access to throw.'); + } catch (\RuntimeException $exception) { + $frame = self::findFrame($exception, CommandException::class, 'fromPrevious'); + self::assertCount(2, $frame['args']); + self::assertInstanceOf(\SensitiveParameterValue::class, $frame['args'][0]); + self::assertInstanceOf(\SensitiveParameterValue::class, $frame['args'][1]); + } finally { + if ($previous !== false) { + ini_set('zend.exception_ignore_args', $previous); + } + } + } + + /** + * @return array + */ + private static function inventory(): array + { + return [ + [ServiceClient::class, 'getCommand', ['params']], + [ServiceClient::class, 'execute', ['command']], + [ServiceClient::class, 'executeAsync', ['command']], + [ServiceClient::class, 'executeAll', ['commands']], + [ServiceClient::class, 'executeAllAsync', ['commands']], + [ServiceClient::class, '__call', ['args']], + [ServiceClient::class, 'transformCommandToRequest', ['command']], + [ServiceClient::class, 'transformResponseToResult', ['response', 'request', 'command']], + [CommandException::class, 'fromPrevious', ['command', 'prev']], + ]; + } + + private function requireNativeTraceRedaction(): void + { + if (\PHP_VERSION_ID < 80200) { + self::markTestSkipped('Native trace redaction requires PHP 8.2.'); + } + + $previous = ini_get('zend.exception_ignore_args'); + if (ini_set('zend.exception_ignore_args', '0') === false) { + self::markTestSkipped('Trace arguments cannot be enabled.'); + } + if ($previous !== false) { + ini_set('zend.exception_ignore_args', $previous); + } + } + + private static function countSourceAttributes(string $directory): int + { + $count = 0; + $files = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($directory)); + foreach ($files as $file) { + if (!$file->isFile() || $file->getExtension() !== 'php') { + continue; + } + + $contents = file_get_contents($file->getPathname()); + self::assertNotFalse($contents); + $count += substr_count($contents, '#[\\SensitiveParameter]'); + } + + return $count; + } + + /** + * @return array + */ + private static function findFrame(\Throwable $exception, string $class, string $function): array + { + foreach ($exception->getTrace() as $frame) { + if (($frame['class'] ?? null) === $class && ($frame['function'] ?? null) === $function) { + return $frame; + } + } + + self::fail('Unable to find '.$class.'::'.$function.' in the exception trace.'); + } +} From dbe8834a74be41c9b0d06dd1a3dfce253768d0b9 Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Sat, 18 Jul 2026 22:31:59 +0100 Subject: [PATCH 2/2] Remove sensitive parameter tests --- tests/SensitiveParameterTest.php | 253 ------------------------------- 1 file changed, 253 deletions(-) delete mode 100644 tests/SensitiveParameterTest.php diff --git a/tests/SensitiveParameterTest.php b/tests/SensitiveParameterTest.php deleted file mode 100644 index b8a18d2..0000000 --- a/tests/SensitiveParameterTest.php +++ /dev/null @@ -1,253 +0,0 @@ -getName(); - }, $reflection->getParameters()); - foreach ($sensitiveParameters as $sensitiveParameter) { - self::assertContains($sensitiveParameter, $parameterNames); - } - $attributeCount += count($sensitiveParameters); - - continue; - } - - foreach ($reflection->getParameters() as $parameter) { - $attributes = $parameter->getAttributes(\SensitiveParameter::class); - $expected = in_array($parameter->getName(), $sensitiveParameters, true) - ? 1 - : 0; - - self::assertCount($expected, $attributes, $class.'::'.$method.'::$'.$parameter->getName()); - foreach ($attributes as $attribute) { - self::assertInstanceOf(\SensitiveParameter::class, $attribute->newInstance()); - } - $attributeCount += count($attributes); - } - } - - self::assertSame(12, $attributeCount); - } - - public function testCommandHandlerClosureParameterIsSensitive(): void - { - $client = new ServiceClient( - $this->createMock(HttpClientInterface::class), - static function (CommandInterface $command): RequestInterface { - return new Request('GET', '/'); - }, - static function ( - ResponseInterface $response, - RequestInterface $request, - CommandInterface $command - ): ResultInterface { - return new Result(); - } - ); - $factory = new \ReflectionMethod(ServiceClient::class, 'createCommandHandler'); - $factory->setAccessible(true); - $handler = $factory->invoke($client); - self::assertInstanceOf(\Closure::class, $handler); - - $parameters = (new \ReflectionFunction($handler))->getParameters(); - self::assertCount(1, $parameters); - self::assertSame('command', $parameters[0]->getName()); - if (\PHP_VERSION_ID < 80000) { - return; - } - self::assertCount(1, $parameters[0]->getAttributes(\SensitiveParameter::class)); - } - - public function testInterfacesAndAssignmentOnlyConstructorAreNotAnnotated(): void - { - if (\PHP_VERSION_ID < 80000) { - self::markTestSkipped('Attributes are not reflected before PHP 8.0.'); - } - - foreach ((new \ReflectionClass(ServiceClientInterface::class))->getMethods() as $method) { - foreach ($method->getParameters() as $parameter) { - self::assertCount(0, $parameter->getAttributes(\SensitiveParameter::class)); - } - } - - $constructor = new \ReflectionMethod(Command::class, '__construct'); - self::assertCount(0, $constructor->getParameters()[1]->getAttributes(\SensitiveParameter::class)); - } - - public function testResponseTransformerArgumentsAreRedactedFromTrace(): void - { - $this->requireNativeTraceRedaction(); - - $client = new ServiceClient( - $this->createMock(HttpClientInterface::class), - static function (CommandInterface $command): RequestInterface { - return new Request('GET', '/'); - }, - static function ( - ResponseInterface $response, - RequestInterface $request, - CommandInterface $command - ): ResultInterface { - throw new \RuntimeException('transform failed'); - } - ); - $method = new \ReflectionMethod(ServiceClient::class, 'transformResponseToResult'); - $method->setAccessible(true); - - $previous = ini_get('zend.exception_ignore_args'); - try { - self::assertNotFalse(ini_set('zend.exception_ignore_args', '0')); - $method->invoke( - $client, - new Response(500), - new Request('GET', '/?api_key=request-secret'), - new Command('GetSecret', ['api_key' => 'command-secret']) - ); - self::fail('Expected the response transformer to throw.'); - } catch (\RuntimeException $exception) { - $frame = self::findFrame($exception, ServiceClient::class, 'transformResponseToResult'); - self::assertCount(3, $frame['args']); - foreach ($frame['args'] as $argument) { - self::assertInstanceOf(\SensitiveParameterValue::class, $argument); - } - } finally { - if ($previous !== false) { - ini_set('zend.exception_ignore_args', $previous); - } - } - } - - public function testCommandExceptionFactoryArgumentsAreRedactedFromTrace(): void - { - $this->requireNativeTraceRedaction(); - - $command = $this->createMock(CommandInterface::class); - $previousException = new class($command) extends CommandException { - public function __construct(CommandInterface $command) - { - parent::__construct('previous-secret', $command); - } - - public function getCommand(): CommandInterface - { - throw new \RuntimeException('command access failed'); - } - }; - - $previous = ini_get('zend.exception_ignore_args'); - try { - self::assertNotFalse(ini_set('zend.exception_ignore_args', '0')); - CommandException::fromPrevious($command, $previousException); - self::fail('Expected command name access to throw.'); - } catch (\RuntimeException $exception) { - $frame = self::findFrame($exception, CommandException::class, 'fromPrevious'); - self::assertCount(2, $frame['args']); - self::assertInstanceOf(\SensitiveParameterValue::class, $frame['args'][0]); - self::assertInstanceOf(\SensitiveParameterValue::class, $frame['args'][1]); - } finally { - if ($previous !== false) { - ini_set('zend.exception_ignore_args', $previous); - } - } - } - - /** - * @return array - */ - private static function inventory(): array - { - return [ - [ServiceClient::class, 'getCommand', ['params']], - [ServiceClient::class, 'execute', ['command']], - [ServiceClient::class, 'executeAsync', ['command']], - [ServiceClient::class, 'executeAll', ['commands']], - [ServiceClient::class, 'executeAllAsync', ['commands']], - [ServiceClient::class, '__call', ['args']], - [ServiceClient::class, 'transformCommandToRequest', ['command']], - [ServiceClient::class, 'transformResponseToResult', ['response', 'request', 'command']], - [CommandException::class, 'fromPrevious', ['command', 'prev']], - ]; - } - - private function requireNativeTraceRedaction(): void - { - if (\PHP_VERSION_ID < 80200) { - self::markTestSkipped('Native trace redaction requires PHP 8.2.'); - } - - $previous = ini_get('zend.exception_ignore_args'); - if (ini_set('zend.exception_ignore_args', '0') === false) { - self::markTestSkipped('Trace arguments cannot be enabled.'); - } - if ($previous !== false) { - ini_set('zend.exception_ignore_args', $previous); - } - } - - private static function countSourceAttributes(string $directory): int - { - $count = 0; - $files = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($directory)); - foreach ($files as $file) { - if (!$file->isFile() || $file->getExtension() !== 'php') { - continue; - } - - $contents = file_get_contents($file->getPathname()); - self::assertNotFalse($contents); - $count += substr_count($contents, '#[\\SensitiveParameter]'); - } - - return $count; - } - - /** - * @return array - */ - private static function findFrame(\Throwable $exception, string $class, string $function): array - { - foreach ($exception->getTrace() as $frame) { - if (($frame['class'] ?? null) === $class && ($frame['function'] ?? null) === $function) { - return $frame; - } - } - - self::fail('Unable to find '.$class.'::'.$function.' in the exception trace.'); - } -}