From 0716311ff658ae783d1a66a46cbac94325d8fbb3 Mon Sep 17 00:00:00 2001 From: Friederich Loheide Date: Wed, 23 Sep 2026 06:52:22 +0000 Subject: [PATCH] =?UTF-8?q?Tests=20f=C3=BCr=20Upload=20und=20Ui,=20strenge?= =?UTF-8?q?re=20CI-Pr=C3=BCfungen?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - tests/UploadTest.php prüft den SVG-Filter (Skripte, Event-Handler, javascript:-Verweise) und Upload::isLocal gegen Pfad-Tricks - tests/UiTest.php prüft Versionsstempel, Media-Pfade und die Branding-Overrides inklusive Abweisung ungültiger Farbwerte - PHPStan analysiert jetzt auch includes/Ui.php und includes/Upload.php - CI vergleicht die Sprachdateien (gleiche Schlüsselmenge) und prüft, dass jeder im Code verwendete Schlüssel existiert Dabei aufgefallen und behoben: Upload.php rief __() direkt auf und wäre außerhalb einer Seite mit geladener I18n mit einem Fatal Error abgebrochen; jetzt gibt es einen Fallback auf die deutsche Meldung. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 35 ++++++++++++++- includes/Upload.php | 25 +++++++---- phpstan.neon | 5 +++ tests/UiTest.php | 93 ++++++++++++++++++++++++++++++++++++++++ tests/UploadTest.php | 72 +++++++++++++++++++++++++++++++ 5 files changed, 220 insertions(+), 10 deletions(-) create mode 100644 tests/UiTest.php create mode 100644 tests/UploadTest.php diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5aaaad9..e88d61a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,9 +29,40 @@ jobs: php -l "$f" done - - name: Validate JSON language/migration assets + - name: Validate language files run: | - php -r 'foreach (glob("lang/*.php") as $f) { $a = require $f; if (!is_array($a)) { fwrite(STDERR, "Bad lang file: $f\n"); exit(1);} } echo "lang OK\n";' + php -r ' + $de = require "lang/de.php"; $en = require "lang/en.php"; + if (!is_array($de) || !is_array($en)) { fwrite(STDERR, "Bad lang file\n"); exit(1); } + $missingEn = array_diff(array_keys($de), array_keys($en)); + $missingDe = array_diff(array_keys($en), array_keys($de)); + if ($missingEn || $missingDe) { + fwrite(STDERR, "Fehlend in en: " . implode(", ", $missingEn) . "\n"); + fwrite(STDERR, "Fehlend in de: " . implode(", ", $missingDe) . "\n"); + exit(1); + } + echo "lang OK (" . count($de) . " Schluessel)\n";' + + - name: Check that every used translation key exists + run: | + php -r ' + $de = require "lang/de.php"; + $missing = []; + $it = new RecursiveIteratorIterator(new RecursiveDirectoryIterator(".", FilesystemIterator::SKIP_DOTS)); + foreach ($it as $file) { + $path = $file->getPathname(); + if (substr($path, -4) !== ".php") continue; + if (strpos($path, "/vendor/") !== false || strpos($path, "/tools/") !== false) continue; + preg_match_all("/__\(\s*\x27([a-z0-9_]+)\x27/", file_get_contents($path), $m); + foreach ($m[1] as $key) { + if (!isset($de[$key]) && substr($key, -1) !== "_") { $missing[$key] = $path; } + } + } + if ($missing) { + foreach ($missing as $key => $path) { fwrite(STDERR, "Unbekannter Schluessel $key in $path\n"); } + exit(1); + } + echo "Alle verwendeten Schluessel vorhanden\n";' test: name: Unit Tests & Static Analysis diff --git a/includes/Upload.php b/includes/Upload.php index 88c2271..39713bf 100644 --- a/includes/Upload.php +++ b/includes/Upload.php @@ -16,6 +16,15 @@ class Upload 'favicon' => ['ico', 'png', 'svg'], ]; + /** + * Uebersetzte Meldung – faellt auf Deutsch zurueck, wenn die Klasse + * ausserhalb einer Seite mit geladener I18n verwendet wird. + */ + private static function msg(string $key, string $fallback): string + { + return function_exists('__') ? __($key) : $fallback; + } + private static function dir(): string { return dirname(__DIR__) . '/uploads'; @@ -68,13 +77,13 @@ class Upload return ''; } if ($file['error'] !== UPLOAD_ERR_OK) { - throw new RuntimeException(__('upload_error_generic')); + throw new RuntimeException(self::msg('upload_error_generic', 'Die Datei konnte nicht hochgeladen werden.')); } if (!is_uploaded_file($file['tmp_name'])) { - throw new RuntimeException(__('upload_error_generic')); + throw new RuntimeException(self::msg('upload_error_generic', 'Die Datei konnte nicht hochgeladen werden.')); } if ($file['size'] > self::MAX_BYTES) { - throw new RuntimeException(__('upload_error_size')); + throw new RuntimeException(self::msg('upload_error_size', 'Die Datei ist zu groß (maximal 3 MB).')); } $allowed = self::ALLOWED[$kind] ?? self::ALLOWED['image']; @@ -83,7 +92,7 @@ class Upload $ext = 'jpg'; } if (!in_array($ext, $allowed, true)) { - throw new RuntimeException(__('upload_error_type')); + throw new RuntimeException(self::msg('upload_error_type', 'Dieser Dateityp wird nicht unterstützt.')); } $data = (string)file_get_contents($file['tmp_name']); @@ -93,18 +102,18 @@ class Upload } elseif ($ext !== 'ico') { // Raster: muss als Bild lesbar sein if (@getimagesize($file['tmp_name']) === false) { - throw new RuntimeException(__('upload_error_type')); + throw new RuntimeException(self::msg('upload_error_type', 'Dieser Dateityp wird nicht unterstützt.')); } } if (!self::ensureDir()) { - throw new RuntimeException(__('upload_error_dir')); + throw new RuntimeException(self::msg('upload_error_dir', 'Der Ordner uploads/ ist nicht beschreibbar.')); } $name = bin2hex(random_bytes(8)) . '.' . $ext; $dest = self::dir() . '/' . $name; if (file_put_contents($dest, $data) === false) { - throw new RuntimeException(__('upload_error_dir')); + throw new RuntimeException(self::msg('upload_error_dir', 'Der Ordner uploads/ ist nicht beschreibbar.')); } @chmod($dest, 0644); @@ -118,7 +127,7 @@ class Upload private static function sanitizeSvg(string $svg): string { if (stripos($svg, ']*>.*?<\s*/\s*\1\s*>#is', '', $svg); diff --git a/phpstan.neon b/phpstan.neon index 818df76..95edbd1 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,6 +1,11 @@ parameters: level: 5 + # __() stammt aus der I18n-Klasse und wird global definiert + scanFiles: + - includes/I18n.php paths: - includes/Totp.php - includes/Crypto.php - includes/ApiKey.php + - includes/Ui.php + - includes/Upload.php diff --git a/tests/UiTest.php b/tests/UiTest.php new file mode 100644 index 0000000..8d55513 --- /dev/null +++ b/tests/UiTest.php @@ -0,0 +1,93 @@ + */ + private array $values; + + /** @param array $values */ + public function __construct(array $values = []) + { + $this->values = $values; + } + + public function getSetting(string $key, $default = null) + { + return $this->values[$key] ?? $default; + } +} + +class UiTest extends TestCase +{ + public function testAssetUrlCarriesVersionStamp(): void + { + $url = \Ui::asset('assets/global.css'); + + $this->assertStringStartsWith('assets/global.css?v=', $url); + $this->assertMatchesRegularExpression('/\?v=\d+$/', $url); + } + + public function testAssetUrlRespectsBasePath(): void + { + $this->assertStringStartsWith('../assets/global.css?v=', \Ui::asset('assets/global.css', '../')); + } + + public function testBrandingStyleIsEmptyForDefaults(): void + { + $db = new FakeSettings([ + 'brand_accent' => \Ui::DEFAULT_ACCENT, + 'brand_accent_dark' => \Ui::DEFAULT_ACCENT_DARK, + 'brand_gradient_from' => \Ui::DEFAULT_GRADIENT_FROM, + 'brand_gradient_to' => \Ui::DEFAULT_GRADIENT_TO, + 'brand_radius' => (string)\Ui::DEFAULT_RADIUS, + ]); + + $this->assertSame('', \Ui::brandingStyle($db)); + $this->assertSame('', \Ui::brandingStyle(null)); + } + + public function testBrandingStyleUsesCustomColour(): void + { + $style = \Ui::brandingStyle(new FakeSettings(['brand_accent' => '#0F766E'])); + + $this->assertStringContainsString('--accent:#0f766e', $style); + $this->assertStringContainsString('[data-theme="dark"]', $style); + } + + public function testBrandingStyleIgnoresInvalidColour(): void + { + $style = \Ui::brandingStyle(new FakeSettings(['brand_accent' => 'rot; background:url(x)'])); + + $this->assertSame('', $style, 'Ungueltige Farben duerfen keinen Override erzeugen'); + } + + public function testBrandingRadiusIsClamped(): void + { + $style = \Ui::brandingStyle(new FakeSettings(['brand_radius' => '999'])); + + $this->assertStringContainsString('--r-lg:28px', $style); + } + + public function testMediaUrlKeepsAbsoluteAddresses(): void + { + $this->assertSame('https://cdn.example.com/logo.svg', \Ui::mediaUrl('https://cdn.example.com/logo.svg', '../')); + $this->assertSame('/logo.svg', \Ui::mediaUrl('/logo.svg', '../')); + $this->assertSame('', \Ui::mediaUrl('', '../')); + } + + public function testMediaUrlPrefixesUploads(): void + { + $this->assertSame('../uploads/logo.png', \Ui::mediaUrl('uploads/logo.png', '../')); + } +} diff --git a/tests/UploadTest.php b/tests/UploadTest.php new file mode 100644 index 0000000..46ec8b6 --- /dev/null +++ b/tests/UploadTest.php @@ -0,0 +1,72 @@ +setAccessible(true); + + return $method->invoke(null, $svg); + } + + public function testRemovesScriptElement(): void + { + $clean = $this->sanitize(''); + + $this->assertStringNotContainsString('assertStringNotContainsString('alert(1)', $clean); + $this->assertStringContainsString('', $clean); + } + + public function testRemovesEventHandlers(): void + { + $clean = $this->sanitize(''); + + $this->assertStringNotContainsString('onload', $clean); + $this->assertStringNotContainsString('onclick', $clean); + } + + public function testRemovesJavascriptLinks(): void + { + $clean = $this->sanitize('x'); + + $this->assertStringNotContainsString('javascript:', $clean); + } + + public function testKeepsHarmlessMarkup(): void + { + $svg = ''; + + $this->assertSame($svg, $this->sanitize($svg)); + } + + public function testRejectsNonSvgContent(): void + { + $this->expectException(RuntimeException::class); + $this->sanitize('GIF89a'); + } + + public function testIsLocalOnlyAcceptsUploadPaths(): void + { + $this->assertTrue(\Upload::isLocal('uploads/abc.png')); + $this->assertFalse(\Upload::isLocal('https://example.com/logo.png')); + $this->assertFalse(\Upload::isLocal('uploads/../config.php')); + $this->assertFalse(\Upload::isLocal('')); + } +}