Never log password

Ecodev · Oct 11, 2023 · 1722f6c · 1722f6c
1 parent 25e0952
commit 1722f6c
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 4 deletions.
diff --git a/src/Log/EventCompleter.php b/src/Log/EventCompleter.php
@@ -41,17 +41,16 @@ private function getEnvData(): array
 
         if (PHP_SAPI === 'cli') {
             global $argv;
-            $request = $argv;
             $ip = !empty(getenv('REMOTE_ADDR')) ? getenv('REMOTE_ADDR') : 'script';
             $url = implode(' ', $argv);
             $referer = '';
         } else {
-            $request = $_REQUEST;
             $ip = $_SERVER['REMOTE_ADDR'] ?? '';
             $url = $this->baseUrl . $_SERVER['REQUEST_URI'];
             $referer = $_SERVER['HTTP_REFERER'] ?? '';
         }
 
+        $request = $_REQUEST;
         $request = $this->removeSensitiveData($request);
 
         $envData = [
@@ -71,9 +70,10 @@ private function getEnvData(): array
      */
     protected function removeSensitiveData(array $request): array
     {
+        unset($request['password']);
         foreach ($request as &$r) {
             if (is_array($r)) {
-                unset($r['variables']['password']);
+                $r = $this->removeSensitiveData($r);
             }
         }
 

diff --git a/tests/Log/EventCompleterTest.php b/tests/Log/EventCompleterTest.php
@@ -14,6 +14,7 @@ class EventCompleterTest extends TestCase
     protected function tearDown(): void
     {
         CurrentUser::set(null);
+        $_REQUEST = [];
     }
 
     public function testProcessMinimal(): void
@@ -41,19 +42,36 @@ public function testProcess(): void
 
         CurrentUser::set($user);
         putenv('REMOTE_ADDR=127.0.0.1');
+        $_REQUEST = [
+            'password' => 'sensitive',
+            'variables' => [
+                'other' => [
+                    'password' => 'sensitive',
+                    'foo' => 123,
+                ],
+            ],
+        ];
+
         $completed = new EventCompleter('https://example.com');
         $actual = $completed->process([
             'message' => '',
             'extra' => [
                 'errno' => 1,
             ],
         ]);
+
         self::assertStringContainsString('Stacktrace:', $actual['message']);
         self::assertSame(123, $actual['creator_id']);
         self::assertSame('my login', $actual['login']);
         self::assertIsString($actual['url']);
         self::assertIsString($actual['referer']);
-        self::assertIsString($actual['request']);
+        self::assertSame([
+            'variables' => [
+                'other' => [
+                    'foo' => 123,
+                ],
+            ],
+        ], json_decode($actual['request'], true));
         self::assertSame('127.0.0.1', $actual['ip']);
     }
 }