Tests für Upload und Ui, strengere CI-Prüfungen
- 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 <noreply@anthropic.com>
This commit is contained in:
parent
36e06ac817
commit
0716311ff6
5 changed files with 220 additions and 10 deletions
35
.github/workflows/ci.yml
vendored
35
.github/workflows/ci.yml
vendored
|
|
@ -29,9 +29,40 @@ jobs:
|
||||||
php -l "$f"
|
php -l "$f"
|
||||||
done
|
done
|
||||||
|
|
||||||
- name: Validate JSON language/migration assets
|
- name: Validate language files
|
||||||
run: |
|
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:
|
test:
|
||||||
name: Unit Tests & Static Analysis
|
name: Unit Tests & Static Analysis
|
||||||
|
|
|
||||||
|
|
@ -16,6 +16,15 @@ class Upload
|
||||||
'favicon' => ['ico', 'png', 'svg'],
|
'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
|
private static function dir(): string
|
||||||
{
|
{
|
||||||
return dirname(__DIR__) . '/uploads';
|
return dirname(__DIR__) . '/uploads';
|
||||||
|
|
@ -68,13 +77,13 @@ class Upload
|
||||||
return '';
|
return '';
|
||||||
}
|
}
|
||||||
if ($file['error'] !== UPLOAD_ERR_OK) {
|
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'])) {
|
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) {
|
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'];
|
$allowed = self::ALLOWED[$kind] ?? self::ALLOWED['image'];
|
||||||
|
|
@ -83,7 +92,7 @@ class Upload
|
||||||
$ext = 'jpg';
|
$ext = 'jpg';
|
||||||
}
|
}
|
||||||
if (!in_array($ext, $allowed, true)) {
|
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']);
|
$data = (string)file_get_contents($file['tmp_name']);
|
||||||
|
|
@ -93,18 +102,18 @@ class Upload
|
||||||
} elseif ($ext !== 'ico') {
|
} elseif ($ext !== 'ico') {
|
||||||
// Raster: muss als Bild lesbar sein
|
// Raster: muss als Bild lesbar sein
|
||||||
if (@getimagesize($file['tmp_name']) === false) {
|
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()) {
|
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;
|
$name = bin2hex(random_bytes(8)) . '.' . $ext;
|
||||||
$dest = self::dir() . '/' . $name;
|
$dest = self::dir() . '/' . $name;
|
||||||
if (file_put_contents($dest, $data) === false) {
|
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);
|
@chmod($dest, 0644);
|
||||||
|
|
||||||
|
|
@ -118,7 +127,7 @@ class Upload
|
||||||
private static function sanitizeSvg(string $svg): string
|
private static function sanitizeSvg(string $svg): string
|
||||||
{
|
{
|
||||||
if (stripos($svg, '<svg') === false) {
|
if (stripos($svg, '<svg') === false) {
|
||||||
throw new RuntimeException(__('upload_error_type'));
|
throw new RuntimeException(self::msg('upload_error_type', 'Dieser Dateityp wird nicht unterstützt.'));
|
||||||
}
|
}
|
||||||
|
|
||||||
$svg = preg_replace('#<\s*(script|foreignObject|iframe|embed|object|animate|set)\b[^>]*>.*?<\s*/\s*\1\s*>#is', '', $svg);
|
$svg = preg_replace('#<\s*(script|foreignObject|iframe|embed|object|animate|set)\b[^>]*>.*?<\s*/\s*\1\s*>#is', '', $svg);
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,11 @@
|
||||||
parameters:
|
parameters:
|
||||||
level: 5
|
level: 5
|
||||||
|
# __() stammt aus der I18n-Klasse und wird global definiert
|
||||||
|
scanFiles:
|
||||||
|
- includes/I18n.php
|
||||||
paths:
|
paths:
|
||||||
- includes/Totp.php
|
- includes/Totp.php
|
||||||
- includes/Crypto.php
|
- includes/Crypto.php
|
||||||
- includes/ApiKey.php
|
- includes/ApiKey.php
|
||||||
|
- includes/Ui.php
|
||||||
|
- includes/Upload.php
|
||||||
|
|
|
||||||
93
tests/UiTest.php
Normal file
93
tests/UiTest.php
Normal file
|
|
@ -0,0 +1,93 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace Tests;
|
||||||
|
|
||||||
|
use PHPUnit\Framework\TestCase;
|
||||||
|
|
||||||
|
require_once __DIR__ . '/../includes/Ui.php';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Sehr einfache Datenbank-Attrappe: liefert nur Einstellungen zurueck.
|
||||||
|
*/
|
||||||
|
class FakeSettings
|
||||||
|
{
|
||||||
|
/** @var array<string, string> */
|
||||||
|
private array $values;
|
||||||
|
|
||||||
|
/** @param array<string, string> $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', '../'));
|
||||||
|
}
|
||||||
|
}
|
||||||
72
tests/UploadTest.php
Normal file
72
tests/UploadTest.php
Normal file
|
|
@ -0,0 +1,72 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
declare(strict_types=1);
|
||||||
|
|
||||||
|
namespace Tests;
|
||||||
|
|
||||||
|
use PHPUnit\Framework\TestCase;
|
||||||
|
use ReflectionMethod;
|
||||||
|
use RuntimeException;
|
||||||
|
|
||||||
|
require_once __DIR__ . '/../includes/Upload.php';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Der SVG-Filter ist die sicherheitskritische Stelle beim Bild-Upload:
|
||||||
|
* hochgeladene Grafiken werden aus der eigenen Domain ausgeliefert, aktive
|
||||||
|
* Inhalte darin waeren damit gespeichertes XSS.
|
||||||
|
*/
|
||||||
|
class UploadTest extends TestCase
|
||||||
|
{
|
||||||
|
private function sanitize(string $svg): string
|
||||||
|
{
|
||||||
|
$method = new ReflectionMethod(\Upload::class, 'sanitizeSvg');
|
||||||
|
$method->setAccessible(true);
|
||||||
|
|
||||||
|
return $method->invoke(null, $svg);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testRemovesScriptElement(): void
|
||||||
|
{
|
||||||
|
$clean = $this->sanitize('<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script><rect/></svg>');
|
||||||
|
|
||||||
|
$this->assertStringNotContainsString('<script', $clean);
|
||||||
|
$this->assertStringNotContainsString('alert(1)', $clean);
|
||||||
|
$this->assertStringContainsString('<rect/>', $clean);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testRemovesEventHandlers(): void
|
||||||
|
{
|
||||||
|
$clean = $this->sanitize('<svg xmlns="http://www.w3.org/2000/svg" onload="alert(1)"><rect onclick=\'steal()\'/></svg>');
|
||||||
|
|
||||||
|
$this->assertStringNotContainsString('onload', $clean);
|
||||||
|
$this->assertStringNotContainsString('onclick', $clean);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testRemovesJavascriptLinks(): void
|
||||||
|
{
|
||||||
|
$clean = $this->sanitize('<svg xmlns="http://www.w3.org/2000/svg"><a xlink:href="javascript:alert(1)">x</a></svg>');
|
||||||
|
|
||||||
|
$this->assertStringNotContainsString('javascript:', $clean);
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testKeepsHarmlessMarkup(): void
|
||||||
|
{
|
||||||
|
$svg = '<svg xmlns="http://www.w3.org/2000/svg" width="10" height="10"><circle cx="5" cy="5" r="4" fill="#0f766e"/></svg>';
|
||||||
|
|
||||||
|
$this->assertSame($svg, $this->sanitize($svg));
|
||||||
|
}
|
||||||
|
|
||||||
|
public function testRejectsNonSvgContent(): void
|
||||||
|
{
|
||||||
|
$this->expectException(RuntimeException::class);
|
||||||
|
$this->sanitize('GIF89a<html>');
|
||||||
|
}
|
||||||
|
|
||||||
|
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(''));
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Add table
Add a link
Reference in a new issue