From c4e82ab4860994003f7ff87bff3691337a5fafb5 Mon Sep 17 00:00:00 2001 From: Akihito Koriyama Date: Sat, 19 Sep 2026 02:34:58 +0900 Subject: [PATCH 1/2] Mask #[SensitiveParameter] properties in semantic log prop payloads ObjectPropertyExtractor recorded every public property of an Input, Being or Final verbatim into the becoming_open / being_close / being_final_close `prop` payload. An application marking a constructor parameter #[\SensitiveParameter] got stack-trace redaction from PHP and nothing else: the same password or reset token landed in the semantic log in plaintext, one level below a resource layer that had already redacted it. The extractor now resolves the constructor parameters carrying #[\SensitiveParameter] once per object and records ObjectPropertyExtractor::FILTERED ('[FILTERED]', the literal bear/event-sourcing uses for the same purpose) in place of any public property with a matching name. Matching by name rather than by promotion also covers a Final that assigns a marked argument to a same-named declared property. Closes #79 --- src/SemanticLog/ObjectPropertyExtractor.php | 76 +++++++++++++++++---- tests/SemanticLog/LoggerTest.php | 60 ++++++++++++++++ 2 files changed, 124 insertions(+), 12 deletions(-) diff --git a/src/SemanticLog/ObjectPropertyExtractor.php b/src/SemanticLog/ObjectPropertyExtractor.php index 92f887f..5d04d91 100644 --- a/src/SemanticLog/ObjectPropertyExtractor.php +++ b/src/SemanticLog/ObjectPropertyExtractor.php @@ -5,6 +5,7 @@ namespace Be\Framework\SemanticLog; use ReflectionClass; +use SensitiveParameter; use function array_walk; use function get_object_vars; @@ -22,9 +23,25 @@ * - Uninitialized properties (returns null) * - Been instances (excluded from output) * - Non-storable values like resources (excluded) + * - Properties named after a constructor parameter marked #[\SensitiveParameter] + * (value replaced with {@see self::FILTERED}) + * + * The last rule reuses the attribute an application already applies for PHP stack-trace + * redaction: a credential-shaped public property (a login's password, a reset token) would + * otherwise reach the semantic log verbatim, since #[\SensitiveParameter] itself only + * affects traces. Matching by name, not by promotion, also covers a Final that assigns a + * marked constructor argument to a same-named declared property. */ final class ObjectPropertyExtractor { + /** + * The value recorded in place of a sensitive property. + * + * Same literal bear/event-sourcing's SensitiveParamsFilter records for a redacted request + * param, so both layers of one observation tree read the same way. + */ + public const string FILTERED = '[FILTERED]'; + /** * Extract all public properties from an object for logging * @@ -37,33 +54,41 @@ final class ObjectPropertyExtractor */ public function extract(object $result): array { - $properties = $this->collectVisibleProperties($result); - $this->mergeDeclaredProperties($properties, $result); + $sensitive = self::sensitivePropertyNames($result); + $properties = $this->collectVisibleProperties($result, $sensitive); + $this->mergeDeclaredProperties($properties, $result, $sensitive); return $properties; } - /** @return array */ - private function collectVisibleProperties(object $result): array + /** + * @param array $sensitive + * + * @return array + */ + private function collectVisibleProperties(object $result, array $sensitive): array { $properties = []; $dynamicProperties = get_object_vars($result); array_walk( $dynamicProperties, - static function (mixed $value, string $name) use (&$properties): void { + static function (mixed $value, string $name) use (&$properties, $sensitive): void { if ($value instanceof Been || ! self::isStorableValue($value)) { return; } - self::storeProperty($properties, $name, $value); + self::storeProperty($properties, $name, $value, $sensitive); }, ); return $properties; } - /** @param array $properties */ - private function mergeDeclaredProperties(array &$properties, object $result): void + /** + * @param array $properties + * @param array $sensitive + */ + private function mergeDeclaredProperties(array &$properties, object $result, array $sensitive): void { foreach ((new ReflectionClass($result))->getProperties() as $property) { if (! $property->isPublic() || $property->isStatic()) { @@ -89,8 +114,30 @@ private function mergeDeclaredProperties(array &$properties, object $result): vo continue; } - self::storeProperty($properties, $name, $value); + self::storeProperty($properties, $name, $value, $sensitive); + } + } + + /** + * Names of constructor parameters marked #[\SensitiveParameter], keyed for lookup. + * + * @return array + */ + private static function sensitivePropertyNames(object $result): array + { + $constructor = (new ReflectionClass($result))->getConstructor(); + if ($constructor === null) { + return []; } + + $names = []; + foreach ($constructor->getParameters() as $parameter) { + if ($parameter->getAttributes(SensitiveParameter::class) !== []) { + $names[$parameter->getName()] = true; + } + } + + return $names; } /** @psalm-assert-if-true array|bool|float|int|object|string|null $value */ @@ -108,9 +155,14 @@ private static function isStorableValue(mixed $value): bool /** * @param array $properties * @param array|bool|float|int|object|string|null $value + * @param array $sensitive */ - private static function storeProperty(array &$properties, string $name, array|bool|float|int|object|string|null $value): void - { - $properties[$name] = $value; + private static function storeProperty( + array &$properties, + string $name, + array|bool|float|int|object|string|null $value, + array $sensitive, + ): void { + $properties[$name] = isset($sensitive[$name]) ? self::FILTERED : $value; } } diff --git a/tests/SemanticLog/LoggerTest.php b/tests/SemanticLog/LoggerTest.php index 21ea95f..cf92d91 100644 --- a/tests/SemanticLog/LoggerTest.php +++ b/tests/SemanticLog/LoggerTest.php @@ -19,6 +19,7 @@ use Ray\Di\Injector; use ReflectionClass; use RuntimeException; +use SensitiveParameter; use stdClass; use function assert; @@ -33,6 +34,34 @@ public function __construct( } } +#[Be(FakeProcessedData::class)] +final class SensitiveInput +{ + public function __construct( + public readonly string $loginId, + #[SensitiveParameter] + public readonly string $password, + ) { + } +} + +/** + * A Final that keeps a marked constructor argument on a same-named declared property. + * Deliberately not promoted: that is the shape under test, so the promotion sniff is off here. + */ +#[Be(FakeProcessedData::class)] +final class SensitiveAssignedInput +{ + public readonly string $authKey; // phpcs:ignore SlevomatCodingStandard.Classes.RequireConstructorPropertyPromotion.RequiredConstructorPropertyPromotion + + public function __construct( + #[SensitiveParameter] + string $authKey, + ) { + $this->authKey = $authKey; + } +} + final class LoggerTest extends TestCase { private Logger $logger; @@ -169,6 +198,37 @@ public function testOpenChainLogsInputProps(): void $this->assertSame(['data' => 'data'], $openData['context']['prop']); } + public function testOpenChainMasksSensitiveParameterProps(): void + { + $chainId = $this->logger->openChain(new SensitiveInput('admin', 'plaintext-password')); + $this->logger->closeChain(new FakeProcessedData('done'), $chainId); + + $logData = $this->semanticLogger->toArray(); + assert(is_array($logData['open']) && is_array($logData['open'][0])); + $openData = $logData['open'][0]; + assert(is_array($openData) && is_array($openData['context'])); + + // (array): semantic-logger 0.9 freezes the context and delivers a map as an object. + $this->assertSame( + ['loginId' => 'admin', 'password' => ObjectPropertyExtractor::FILTERED], + (array) $openData['context']['prop'], + ); + } + + public function testOpenChainMasksSensitiveParameterAssignedToDeclaredProp(): void + { + // The parameter is not promoted; the class assigns it to a same-named public property. + $chainId = $this->logger->openChain(new SensitiveAssignedInput('totp-shared-secret')); + $this->logger->closeChain(new FakeProcessedData('done'), $chainId); + + $logData = $this->semanticLogger->toArray(); + assert(is_array($logData['open']) && is_array($logData['open'][0])); + $openData = $logData['open'][0]; + assert(is_array($openData) && is_array($openData['context'])); + + $this->assertSame(['authKey' => ObjectPropertyExtractor::FILTERED], (array) $openData['context']['prop']); + } + public function testCloseChainLogsSuccessExit(): void { $chainId = $this->logger->openChain(new TestInput('data')); From 4c1c30df786c190a6d9fea5b82a08517bcce04e0 Mon Sep 17 00:00:00 2001 From: Akihito Koriyama Date: Sat, 19 Sep 2026 11:29:52 +0900 Subject: [PATCH 2/2] Cover Logger::close() masking on both being_close and being_final_close The extractor is shared with close(), but the sensitive-property tests only exercised openChain(). A Final assigning a marked argument to a declared property is exactly the close-path shape, so the assigned fixture is now a real Final (no #[Be]) and both close branches assert [FILTERED]. --- tests/SemanticLog/LoggerTest.php | 43 +++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/tests/SemanticLog/LoggerTest.php b/tests/SemanticLog/LoggerTest.php index cf92d91..6001c57 100644 --- a/tests/SemanticLog/LoggerTest.php +++ b/tests/SemanticLog/LoggerTest.php @@ -24,6 +24,7 @@ use function assert; use function is_array; +use function is_string; #[Be(FakeProcessedData::class)] final class TestInput @@ -46,11 +47,10 @@ public function __construct( } /** - * A Final that keeps a marked constructor argument on a same-named declared property. + * A Final (no #[Be]) that keeps a marked constructor argument on a same-named declared property. * Deliberately not promoted: that is the shape under test, so the promotion sniff is off here. */ -#[Be(FakeProcessedData::class)] -final class SensitiveAssignedInput +final class SensitiveAssignedFinal { public readonly string $authKey; // phpcs:ignore SlevomatCodingStandard.Classes.RequireConstructorPropertyPromotion.RequiredConstructorPropertyPromotion @@ -218,7 +218,7 @@ public function testOpenChainMasksSensitiveParameterProps(): void public function testOpenChainMasksSensitiveParameterAssignedToDeclaredProp(): void { // The parameter is not promoted; the class assigns it to a same-named public property. - $chainId = $this->logger->openChain(new SensitiveAssignedInput('totp-shared-secret')); + $chainId = $this->logger->openChain(new SensitiveAssignedFinal('totp-shared-secret')); $this->logger->closeChain(new FakeProcessedData('done'), $chainId); $logData = $this->semanticLogger->toArray(); @@ -229,6 +229,41 @@ public function testOpenChainMasksSensitiveParameterAssignedToDeclaredProp(): vo $this->assertSame(['authKey' => ObjectPropertyExtractor::FILTERED], (array) $openData['context']['prop']); } + public function testCloseMasksSensitiveParameterOnIntermediateBeing(): void + { + $openId = $this->logger->open(new TestInput('test data'), SensitiveInput::class, []); + // SensitiveInput carries #[Be], so this close is being_close, not being_final_close. + $this->logger->close(new SensitiveInput('admin', 'plaintext-password'), $openId); + + $closeData = $this->firstCloseData(); + $this->assertSame('being_close', $closeData['type']); + $this->assertSame( + ['loginId' => 'admin', 'password' => ObjectPropertyExtractor::FILTERED], + (array) $closeData['context']['prop'], + ); + } + + public function testCloseMasksSensitiveParameterOnFinal(): void + { + $openId = $this->logger->open(new TestInput('test data'), SensitiveAssignedFinal::class, []); + $this->logger->close(new SensitiveAssignedFinal('totp-shared-secret'), $openId); + + $closeData = $this->firstCloseData(); + $this->assertSame('being_final_close', $closeData['type']); + $this->assertSame(['authKey' => ObjectPropertyExtractor::FILTERED], (array) $closeData['context']['prop']); + } + + /** @return array{type: string, context: array} */ + private function firstCloseData(): array + { + $logData = $this->semanticLogger->toArray(); + assert(is_array($logData['open']) && is_array($logData['open'][0])); + $closeData = $logData['open'][0]['close']; + assert(is_array($closeData) && is_string($closeData['type']) && is_array($closeData['context'])); + + return ['type' => $closeData['type'], 'context' => $closeData['context']]; + } + public function testCloseChainLogsSuccessExit(): void { $chainId = $this->logger->openChain(new TestInput('data'));