Mask #[SensitiveParameter] properties in semantic log prop payloads

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<string, mixed> */
-    private function collectVisibleProperties(object $result): array
+    /**
+     * @param array<string, true> $sensitive
+     *
+     * @return array<string, mixed>
+     */
+    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<string, mixed> $properties */
-    private function mergeDeclaredProperties(array &$properties, object $result): void
+    /**
+     * @param array<string, mixed> $properties
+     * @param array<string, true>  $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<string, true>
+     */
+    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<array-key, mixed>|bool|float|int|object|string|null $value */
@@ -108,9 +155,14 @@ private static function isStorableValue(mixed $value): bool
     /**
      * @param array<string, mixed>                                      $properties
      * @param array<array-key, mixed>|bool|float|int|object|string|null $value
+     * @param array<string, true>                                       $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;
     }
 }

tests/SemanticLog/LoggerTest.php

@@ -19,10 +19,12 @@
 use Ray\Di\Injector;
 use ReflectionClass;
 use RuntimeException;
+use SensitiveParameter;
 use stdClass;
 
 use function assert;
 use function is_array;
+use function is_string;
 
 #[Be(FakeProcessedData::class)]
 final class TestInput
@@ -33,6 +35,33 @@ public function __construct(
     }
 }
 
+#[Be(FakeProcessedData::class)]
+final class SensitiveInput
+{
+    public function __construct(
+        public readonly string $loginId,
+        #[SensitiveParameter]
+        public readonly string $password,
+    ) {
+    }
+}
+
+/**
+ * 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.
+ */
+final class SensitiveAssignedFinal
+{
+    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,72 @@ 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 SensitiveAssignedFinal('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 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<string, mixed>} */
+    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'));