From 5582a5520801543d9b16a9df4b9d56161084c40c Mon Sep 17 00:00:00 2001 From: fs Date: Thu, 19 Mar 2026 19:08:28 +0100 Subject: [PATCH] Split security logging contracts --- phpunit.xml | 4 +- .../SecurityLoggingContractTest.php | 91 ------------------- .../SecurityLoggingRedactionContractTest.php | 63 +++++++++++++ .../SecurityLoggingRuntimeContractTest.php | 39 ++++++++ .../SecurityLoggingTelemetryContractTest.php | 21 +++++ tests/README.md | 2 +- 6 files changed, 127 insertions(+), 93 deletions(-) delete mode 100644 tests/Architecture/SecurityLoggingContractTest.php create mode 100644 tests/Architecture/SecurityLoggingRedactionContractTest.php create mode 100644 tests/Architecture/SecurityLoggingRuntimeContractTest.php create mode 100644 tests/Architecture/SecurityLoggingTelemetryContractTest.php diff --git a/phpunit.xml b/phpunit.xml index b0c0b77..93e8811 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -41,7 +41,9 @@ tests/Architecture/PublicPageAdminContractTest.php tests/Architecture/PublicPageRoutingContractTest.php tests/Architecture/SecurityCryptoContractTest.php - tests/Architecture/SecurityLoggingContractTest.php + tests/Architecture/SecurityLoggingRedactionContractTest.php + tests/Architecture/SecurityLoggingRuntimeContractTest.php + tests/Architecture/SecurityLoggingTelemetryContractTest.php tests/Architecture/AsideNavigationContractTest.php diff --git a/tests/Architecture/SecurityLoggingContractTest.php b/tests/Architecture/SecurityLoggingContractTest.php deleted file mode 100644 index dc4dcc5..0000000 --- a/tests/Architecture/SecurityLoggingContractTest.php +++ /dev/null @@ -1,91 +0,0 @@ -projectRootPath(); - $violations = []; - - foreach (['lib', 'pages'] as $dir) { - $basePath = $root . '/' . $dir; - $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($basePath)); - - /** @var \SplFileInfo $file */ - foreach ($iterator as $file) { - if (!$file->isFile() || $file->getExtension() !== 'php') { - continue; - } - - $content = (string) file_get_contents($file->getPathname()); - $relativePath = str_replace($root . '/', '', $file->getPathname()); - - if (preg_match_all('/^.*\berror_log\s*\(.*$/m', $content, $matches)) { - foreach ($matches[0] as $line) { - foreach (self::PII_VARIABLE_PATTERNS as $piiPattern) { - if (preg_match($piiPattern, $line)) { - $violations[] = $relativePath . ': ' . trim($line); - break; - } - } - } - } - } - } - - sort($violations); - - $this->assertSame( - [], - $violations, - "error_log() calls with potential PII variables (GR-SEC-002):\n" . implode("\n", $violations) - ); - } - - public function testErrorLogUsageIsMinimal(): void - { - $violations = array_merge( - $this->findPatternMatchesInPhpFiles('lib', '/\berror_log\s*\(/'), - $this->findPatternMatchesInPhpFiles('pages', '/\berror_log\s*\(/') - ); - $violations = array_values(array_unique($violations)); - sort($violations); - - $this->assertLessThanOrEqual( - 3, - count($violations), - "error_log() usage growing — consider structured logging (GR-SEC-002). Found in:\n" - . implode("\n", $violations) - ); - } -} diff --git a/tests/Architecture/SecurityLoggingRedactionContractTest.php b/tests/Architecture/SecurityLoggingRedactionContractTest.php new file mode 100644 index 0000000..28fae24 --- /dev/null +++ b/tests/Architecture/SecurityLoggingRedactionContractTest.php @@ -0,0 +1,63 @@ +readProjectFile('lib/Service/Audit/SystemAuditRedactionService.php'); + + foreach ([ + "'password'", + "'access_token'", + "'refresh_token'", + "'id_token'", + "'authorization'", + "'api_key'", + "'client_secret'", + "'smtp_password'", + "'microsoft_shared_client_secret'", + "'email'", + "'to_email'", + ] as $needle) { + self::assertStringContainsString($needle, $content); + } + + self::assertStringContainsString("return str_contains(\$key, 'password')", $content); + self::assertStringContainsString("|| str_contains(\$key, 'secret')", $content); + self::assertStringContainsString("|| str_contains(\$key, 'token')", $content); + self::assertStringContainsString("|| str_contains(\$key, 'authorization')", $content); + self::assertStringContainsString("|| str_ends_with(\$key, '_key');", $content); + } + + public function testApiAuditRedactionCoversCredentialsAndAuthorizationFields(): void + { + $content = $this->readProjectFile('lib/Service/Audit/ApiAuditService.php'); + + foreach ([ + "'token'", + "'access_token'", + "'refresh_token'", + "'id_token'", + "'secret'", + "'password'", + "'authorization'", + "'api_key'", + "'client_secret'", + "'key'", + ] as $needle) { + self::assertStringContainsString($needle, $content); + } + + self::assertStringContainsString("return str_contains(\$key, 'token')", $content); + self::assertStringContainsString("|| str_contains(\$key, 'secret')", $content); + self::assertStringContainsString("|| str_contains(\$key, 'password')", $content); + self::assertStringContainsString("|| str_contains(\$key, 'authorization')", $content); + self::assertStringContainsString("|| str_ends_with(\$key, '_key');", $content); + } +} diff --git a/tests/Architecture/SecurityLoggingRuntimeContractTest.php b/tests/Architecture/SecurityLoggingRuntimeContractTest.php new file mode 100644 index 0000000..dfd9a4f --- /dev/null +++ b/tests/Architecture/SecurityLoggingRuntimeContractTest.php @@ -0,0 +1,39 @@ +findPatternMatchesInPhpFiles('lib', '/\berror_log\s*\(/'), + $this->findPatternMatchesInPhpFiles('pages', '/\berror_log\s*\(/') + ); + sort($violations); + + self::assertSame( + ['pages/admin/frontend-telemetry/ingest().php'], + $violations, + "Unexpected error_log() usage outside the telemetry ingest fallback:\n" . implode("\n", $violations) + ); + + $content = $this->readProjectFile('pages/admin/frontend-telemetry/ingest().php'); + self::assertStringContainsString( + "error_log('[frontend-telemetry] ingest record_failed request_id=' . RequestContext::id());", + $content + ); + + preg_match('/^.*\berror_log\s*\(.*$/m', $content, $matches); + $errorLogLine = $matches[0] ?? ''; + + self::assertNotSame('', $errorLogLine, 'Expected a dedicated telemetry error_log() fallback line.'); + self::assertStringNotContainsString('$payload', $errorLogLine); + self::assertStringNotContainsString('$record', $errorLogLine); + self::assertStringNotContainsString('$events', $errorLogLine); + } +} diff --git a/tests/Architecture/SecurityLoggingTelemetryContractTest.php b/tests/Architecture/SecurityLoggingTelemetryContractTest.php new file mode 100644 index 0000000..656f67b --- /dev/null +++ b/tests/Architecture/SecurityLoggingTelemetryContractTest.php @@ -0,0 +1,21 @@ +readProjectFile('lib/Service/Audit/FrontendTelemetryIngestService.php'); + + self::assertStringContainsString('[REDACTED]', $content); + self::assertStringContainsString('[REDACTED_EMAIL]', $content); + self::assertStringContainsString('[UUID]', $content); + self::assertStringContainsString('[TOKEN]', $content); + self::assertStringContainsString('(token|access_token|refresh_token|id_token|authorization|api_key|secret)', $content); + } +} diff --git a/tests/README.md b/tests/README.md index 602478b..d0eaf1b 100644 --- a/tests/README.md +++ b/tests/README.md @@ -33,6 +33,6 @@ ohne dass jede Aenderung immer die komplette Suite im Kopf behalten muss. ## Naechste Refactor-Kandidaten - `tests/Architecture/ContainerFactoryContractTest.php` -- `tests/Architecture/SecurityLoggingContractTest.php` - `tests/Architecture/StatusTaxonomyContractTest.php` - `tests/Architecture/AuthLoginAccessibilityContractTest.php` +- `tests/Architecture/SecurityCryptoContractTest.php`