From 96d36b1d594cd68c0d44ac58159233508a5e6d3c Mon Sep 17 00:00:00 2001 From: yuriizee Date: Thu, 29 Oct 2026 06:37:42 +0000 Subject: [PATCH] Keep input out of data class constructor stack frames The library hands raw input to a constructor it does not own, so that frame cannot carry #[\SensitiveParameter]. A TypeError for a wrong input type, or an exception thrown by the constructor's own validation, kept the arguments (e.g. a card number) in its trace, where error trackers that collect frame arguments would pick them up. from(), fromLazy() and with() now wrap that frame's arguments in SensitiveParameterValue, in place, the way PHP does for a sensitive parameter. The exception type, message, line and previous chain are unchanged. --- CHANGELOG.md | 9 ++++++++ docs/attributes/sensitive.md | 2 +- src/BaseData.php | 14 ++++++++++-- src/Support/HydratorCompiler.php | 9 ++++++-- src/Support/Redactor.php | 33 +++++++++++++++++++++++++++++ tests/Fixtures/ChargeData.php | 22 +++++++++++++++++++ tests/Fixtures/HybridChargeData.php | 10 +++++++++ tests/StackTraceRedactionTest.php | 32 ++++++++++++++++++++++++++++ 8 files changed, 126 insertions(+), 5 deletions(-) create mode 100644 tests/Fixtures/ChargeData.php create mode 100644 tests/Fixtures/HybridChargeData.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 2c81e91..c52bc90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0. ## [Unreleased] +### Security +- An exception thrown by a data class constructor during `from()`, `fromLazy()` + or `with()` (a `TypeError` for a wrong input type, or the constructor's own + validation) no longer carries the raw arguments in the constructor's stack + frame: they are wrapped in `SensitiveParameterValue`, as PHP does for + `#[\SensitiveParameter]`. The exception's type, message, line and previous + chain are unchanged. Run `sdo:clear` (or `sdo:warm`) after upgrading so + file-cached hydrators are regenerated. + ## [2.8.0] — 2026-10-16 ### Added diff --git a/docs/attributes/sensitive.md b/docs/attributes/sensitive.md index df460cb..92ae5fc 100644 --- a/docs/attributes/sensitive.md +++ b/docs/attributes/sensitive.md @@ -62,7 +62,7 @@ To queue a job that needs the card, pass an identifier (or a token from your pay It requires `symfony/var-dumper` (installed with Laravel) and changes nothing until you call it. - `var_export()` has no hook and always shows the value. - A field without a cast, enum or nested type goes to the constructor as-is; a wrong type raises PHP's `TypeError`, which names the type, never the value. -- Every entry point (`from*()`, `tryFrom()`, `validate()`, `with()`, `collection()`), the compiled hydrators, pipes, casts and the Eloquent casts mark their input `#[\SensitiveParameter]`, so with `zend.exception_ignore_args=Off` stack traces show `Object(SensitiveParameterValue)` instead of the input. Your own casts and pipes need the same attribute on their `$value` / `$data` parameter. Keep `zend.exception_ignore_args=On` in production anyway: it also covers application frames. +- Every entry point (`from*()`, `tryFrom()`, `validate()`, `with()`, `collection()`), the compiled hydrators, pipes, casts and the Eloquent casts mark their input `#[\SensitiveParameter]`, so with `zend.exception_ignore_args=Off` stack traces show `Object(SensitiveParameterValue)` instead of the input. The data class constructor is your code, so the library cannot mark it: when the constructor rejects its arguments — a `TypeError` for a wrong type, or an exception your own checks throw — the library wraps that frame's arguments in `SensitiveParameterValue` before the exception leaves `from()`, `fromLazy()` or `with()`. The exception's type, message, line and `getPrevious()` chain are unchanged. Helpers the constructor calls are still your frames: mark their parameters `#[\SensitiveParameter]`. Your own casts and pipes need the same attribute on their `$value` / `$data` parameter. Keep `zend.exception_ignore_args=On` in production anyway: it also covers application frames. ## PAN masking diff --git a/src/BaseData.php b/src/BaseData.php index 2b2af0c..d6d27ad 100644 --- a/src/BaseData.php +++ b/src/BaseData.php @@ -19,6 +19,7 @@ use StdOut\SimpleDataObjects\Support\HydratorCompiler; use StdOut\SimpleDataObjects\Support\InputNormalizer; use StdOut\SimpleDataObjects\Support\MetadataRegistry; +use StdOut\SimpleDataObjects\Support\Redactor; use StdOut\SimpleDataObjects\Support\SchemaGenerator; use StdOut\SimpleDataObjects\Support\SerializerCompiler; use StdOut\SimpleDataObjects\Support\ValueCaster; @@ -94,7 +95,12 @@ public static function fromLazy(#[\SensitiveParameter] mixed $data): static } $args = (HydratorCompiler::$argResolvers[$class] ?? HydratorCompiler::compileArgs($class))($normalized); - $ghost->__construct(...$args); + + try { + $ghost->__construct(...$args); + } catch (\Throwable $e) { + throw Redactor::scrubConstructorArguments($e, $class); + } // Hybrid: the constructor only covers its own parameters — the // extra properties still need populating, same mechanism as the @@ -242,7 +248,11 @@ public function with(#[\SensitiveParameter] mixed ...$overrides): static ); } - $instance = $meta->hasConstructor ? new static(...$ctorArgs) : new static; + try { + $instance = $meta->hasConstructor ? new static(...$ctorArgs) : new static; + } catch (\Throwable $e) { // @phpstan-ignore catch.neverThrown (PHPStan sees only BaseData, which has no constructor; subclass constructors can throw) + throw Redactor::scrubConstructorArguments($e, static::class); + } // Extra (non-constructor) properties — constructor-less classes and // hybrid classes' extra fields. Legal even for readonly properties: diff --git a/src/Support/HydratorCompiler.php b/src/Support/HydratorCompiler.php index d651e5b..a4c2520 100644 --- a/src/Support/HydratorCompiler.php +++ b/src/Support/HydratorCompiler.php @@ -190,13 +190,18 @@ static function (#[\\SensitiveParameter] array \$d) use (\$p, \$pipes): \\{$clas $unknownCheck = self::buildUnknownKeysCheck($class, $meta); [$body, $argList] = self::buildParts($class, $meta); + $construct = "try {\n" + ." \$o = new \\{$class}({$argList});\n" + ." } catch (\\Throwable \$e) {\n" + ." throw \\StdOut\\SimpleDataObjects\\Support\\Redactor::scrubConstructorArguments(\$e, \\{$class}::class);\n" + ." }\n"; if ($meta->hasExtraProperties) { $extraBody = self::buildPropertyAssignments($class, $meta, includePipesPrelude: false); return <<getPrevious()) { + $property = new \ReflectionProperty($current instanceof \Exception ? \Exception::class : \Error::class, 'trace'); + /** @var list> $trace */ + $trace = $property->getValue($current); + + foreach ($trace as $i => $frame) { + if (($frame['function'] ?? null) === '__construct' + && isset($frame['class'], $frame['args']) + && is_a($class, (string) $frame['class'], true)) { + $trace[$i]['args'] = array_map( + static fn (mixed $arg): \SensitiveParameterValue => $arg instanceof \SensitiveParameterValue ? $arg : new \SensitiveParameterValue($arg), + (array) $frame['args'], + ); + } + } + + $property->setValue($current, $trace); + } + + return $e; + } + /** @param array{0: string} $match */ private static function maskDigits(array $match): string { diff --git a/tests/Fixtures/ChargeData.php b/tests/Fixtures/ChargeData.php new file mode 100644 index 0000000..8289eb4 --- /dev/null +++ b/tests/Fixtures/ChargeData.php @@ -0,0 +1,22 @@ + [static fn () => (new EncryptedCast('k'))->get($pan.'!')]; yield 'data pipe' => [static fn () => ThrowingPipeData::from(['name' => $pan])]; yield 'fromLivewire' => [static fn () => WireableEventData::fromLivewire(['name' => $pan])]; + yield 'constructor type error' => [static fn () => ChargeData::from(['pan' => $pan, 'amount' => 'ten'])]; + yield 'constructor throws' => [static fn () => ChargeData::from(['pan' => $pan, 'amount' => -1, 'cvv' => '123'])]; + yield 'inherited constructor, extra properties' => [static fn () => HybridChargeData::from(['pan' => $pan, 'amount' => -1])]; + yield 'constructor throws in fromLazy' => [static fn () => ChargeData::fromLazy(['pan' => $pan, 'amount' => -1])->amount]; + yield 'constructor throws in with' => [static fn () => ChargeData::from(['pan' => $pan, 'amount' => 1])->with(amount: -1)]; } #[DataProvider('failingCalls')] @@ -80,6 +87,31 @@ public function test_eloquent_casts_keep_input_out_of_stack_trace_arguments(): v $this->assertNoPanInTrace(fn () => (new DataCollectionCast(CastableItemData::class))->get($model, 'items', '{"pan":"'.self::PAN.'"', $attributes)); } + public function test_constructor_exception_keeps_type_message_line_and_previous(): void + { + try { + ChargeData::from(['pan' => self::PAN, 'amount' => -1, 'cvv' => '123']); + $this->fail('Expected the constructor to throw.'); + } catch (\DomainException $e) { + $this->assertSame('Charge rejected.', $e->getMessage()); + $this->assertStringEndsWith('ChargeData.php', $e->getFile()); + $this->assertInstanceOf(\InvalidArgumentException::class, $e->getPrevious()); + + $frame = $e->getTrace()[0]; + $this->assertSame([ChargeData::class, '__construct'], [$frame['class'] ?? null, $frame['function']]); + $this->assertContainsOnlyInstancesOf(\SensitiveParameterValue::class, $frame['args'] ?? []); + $this->assertSame([self::PAN, -1, '123'], array_map(static fn (\SensitiveParameterValue $arg): mixed => $arg->getValue(), $frame['args'] ?? [])); + } + } + + public function test_constructor_type_error_stays_a_type_error(): void + { + $this->expectException(\TypeError::class); + $this->expectExceptionMessage('Argument #2 ($amount) must be of type int'); + + ChargeData::from(['pan' => self::PAN, 'amount' => 'ten']); + } + private function assertNoPanInTrace(Closure $call): void { try {