From 705f22a5bd8c5817c982b97ba3cdd0d74f2d6938 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 10:28:10 +0200 Subject: [PATCH 01/23] Heal every public/.htaccess Pinakes ever shipped, not only the 0.7.71 one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit public/.htaccess is preserved on upgrade, so an install keeps the shape it was installed with. The 0.8.6 heal only recognised the 0.7.71 – 0.8.5 private-uploads rule; older installs kept rules that serve private uploads straight from disk: - up to 0.7.17 there is no private rule at all: every existing file under /uploads/, Digital Library files and archive documents included, is served as it is; - 0.7.18 – 0.7.70 anchor the private rules on %{REQUEST_URI}, so under a subfolder they never match and existing private files are served as they are; - 0.7.71 – 0.8.5 lack [NC], so /uploads/Digital/ skips the private rule on a case-insensitive disk. PrivateUploadsRewriteHeal now brings each shape to the 0.8.6 pair (the per-directory private rule with [NC], then the public passthrough), replacing only lines that are still exactly what was shipped, and refuses any result where the private rule does not come first. Tests: tests/fixtures/htaccess-history holds the 13 distinct shipped files. htaccess-heal-history-480.unit.php checks each healed file (private rule with [NC] first, no anchored REQUEST_URI rule left, every other directive untouched, idempotent) and that edited rules are left alone. htaccess-heal-apache-480.test.sh serves each one on a real Apache, before and after, at the root and in a subfolder: before the heal every format older than 0.8.6 leaks private files somewhere; after it none does and covers stay static (104 checks). --- .gitignore | 1 + app/Support/PrivateUploadsRewriteHeal.php | 104 +++++++-- .../fixtures/htaccess-history/0.1.1.htaccess | 35 ++++ .../fixtures/htaccess-history/0.1.7.htaccess | 81 +++++++ .../fixtures/htaccess-history/0.4.5.htaccess | 82 ++++++++ .../fixtures/htaccess-history/0.4.8.htaccess | 82 ++++++++ .../htaccess-history/0.4.9.1.htaccess | 95 +++++++++ .../fixtures/htaccess-history/0.7.13.htaccess | 149 +++++++++++++ .../fixtures/htaccess-history/0.7.18.htaccess | 159 ++++++++++++++ .../fixtures/htaccess-history/0.7.21.htaccess | 168 +++++++++++++++ .../fixtures/htaccess-history/0.7.31.htaccess | 168 +++++++++++++++ .../fixtures/htaccess-history/0.7.59.htaccess | 168 +++++++++++++++ .../htaccess-history/0.7.71-rc.1.htaccess | 191 +++++++++++++++++ .../fixtures/htaccess-history/0.7.71.htaccess | 196 +++++++++++++++++ .../htaccess-history/0.8.6-rc.1.htaccess | 198 ++++++++++++++++++ tests/htaccess-heal-apache-480.test.sh | 106 ++++++++++ tests/htaccess-heal-history-480.unit.php | 61 ++++++ 17 files changed, 2028 insertions(+), 16 deletions(-) create mode 100644 tests/fixtures/htaccess-history/0.1.1.htaccess create mode 100644 tests/fixtures/htaccess-history/0.1.7.htaccess create mode 100644 tests/fixtures/htaccess-history/0.4.5.htaccess create mode 100644 tests/fixtures/htaccess-history/0.4.8.htaccess create mode 100644 tests/fixtures/htaccess-history/0.4.9.1.htaccess create mode 100644 tests/fixtures/htaccess-history/0.7.13.htaccess create mode 100644 tests/fixtures/htaccess-history/0.7.18.htaccess create mode 100644 tests/fixtures/htaccess-history/0.7.21.htaccess create mode 100644 tests/fixtures/htaccess-history/0.7.31.htaccess create mode 100644 tests/fixtures/htaccess-history/0.7.59.htaccess create mode 100644 tests/fixtures/htaccess-history/0.7.71-rc.1.htaccess create mode 100644 tests/fixtures/htaccess-history/0.7.71.htaccess create mode 100644 tests/fixtures/htaccess-history/0.8.6-rc.1.htaccess create mode 100644 tests/htaccess-heal-apache-480.test.sh create mode 100644 tests/htaccess-heal-history-480.unit.php diff --git a/.gitignore b/.gitignore index fb9b6c12d..13ce1ae29 100644 --- a/.gitignore +++ b/.gitignore @@ -472,6 +472,7 @@ tests/fixtures/* !tests/fixtures/*.csv !tests/fixtures/*.tsv !tests/fixtures/sru/ +!tests/fixtures/htaccess-history/ !tests/helpers/ tests/helpers/* !tests/helpers/*.php diff --git a/app/Support/PrivateUploadsRewriteHeal.php b/app/Support/PrivateUploadsRewriteHeal.php index d9580895a..884ca6d85 100644 --- a/app/Support/PrivateUploadsRewriteHeal.php +++ b/app/Support/PrivateUploadsRewriteHeal.php @@ -4,34 +4,69 @@ namespace App\Support; /** - * Brings the two 0.8.6 lines of public/.htaccess to installs that predate them (#480). + * Brings public/.htaccess of an upgraded installation to the 0.8.6 private-uploads rules (#480). * - * public/.htaccess is on the updater's preserved list, because it holds the operator's - * own rules, so a change to the shipped file never reaches an upgraded install: + * public/.htaccess is on the updater's preserved list, because it holds the operator's own + * rules, so a change to the shipped file never reaches an upgraded install. Four shapes of + * the /uploads/ rules were shipped over the years, and an install keeps whichever it was + * installed with: * - * - the private-uploads rewrite gained [NC]. On a case-insensitive disk (macOS, some - * shared hosts) /uploads/Digital/x is the same file as /uploads/digital/x; without the - * flag that spelling skipped the rewrite to Pinakes and the public rule below served - * a Digital Library file still waiting in the old folder without any check; - * - PDF and WAV joined the no-gzip list, so a Range request gets the bytes on disk. + * A. (≤ 0.7.17) no private rule at all: every existing file under /uploads/ is served as + * it is, Digital Library files and archive documents included; + * B. (0.7.18 – 0.7.70) private rules on an anchored %{REQUEST_URI}: correct at the domain + * root, but under a subfolder the anchor never matches and existing private files are + * served as they are; + * C. (0.7.71 – 0.8.5) the per-directory private rule without [NC]: on a case-insensitive + * disk /uploads/Digital/x skips it and reaches the public rule; + * D. (0.8.6) the private rule with [NC], before the public one. * - * Each line is replaced only when it is still exactly the line an older version - * shipped: a line the operator edited is theirs, and is left alone. + * Each shape is replaced only where its lines are still exactly what was shipped: a rule + * the operator wrote is theirs, and is left alone. The result must have the private rule + * with [NC] before any public /uploads/ rule; otherwise the file is not touched at all. + * PDF and WAV also join the no-gzip list, so a Range request gets the bytes on disk. */ final class PrivateUploadsRewriteHeal { - private const REWRITE_OLD = '/^([ \t]*)RewriteRule \^uploads\/\(digital\|archives\/documents\|storage\)\/ index\.php \[QSA,L\][ \t]*$/m'; - private const REWRITE_NEW = '$1RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [NC,QSA,L]'; + private const PRIVATE_RULE = 'RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [NC,QSA,L]'; + private const PUBLIC_RULE = 'RewriteRule ^uploads/ - [L]'; + + /** C: the 0.7.71 – 0.8.5 private rule, without [NC]. */ + private const REWRITE_C = '/^([ \t]*)RewriteRule \^uploads\/\(digital\|archives\/documents\|storage\)\/ index\.php \[QSA,L\][ \t]*$/m'; + + /** A: "RewriteCond ^/uploads/ + RewriteRule ^ - [L]" with nothing in between. */ + private const PASSTHROUGH_A = '/^([ \t]*)RewriteCond %\{REQUEST_URI\} \^\/uploads\/ \[NC\][ \t]*\r?\n[ \t]*RewriteRule \^ - \[L\][ \t]*(?=\r?\n|$)/m'; + + /** B, first half: the public passthrough excluding the private folders. */ + private const PASSTHROUGH_B = '/^([ \t]*)RewriteCond %\{REQUEST_URI\} \^\/uploads\/ \[NC\][ \t]*\r?\n[ \t]*RewriteCond %\{REQUEST_URI\} !\^\/uploads\/\(digital\|archives\/documents\|storage\)\/ \[NC\][ \t]*\r?\n[ \t]*RewriteRule \^ - \[L\][ \t]*(?=\r?\n|$)/m'; + + /** B, second half: the private rule on REQUEST_URI, with the comment that introduced it. */ + private const PRIVATE_B = '/^(?:[ \t]*# PRIVATE uploaded content[^\r\n]*\r?\n(?:[ \t]*#[^\r\n]*\r?\n)*)?[ \t]*RewriteCond %\{REQUEST_URI\} \^\/uploads\/\(digital\|archives\/documents\|storage\)\/ \[NC\][ \t]*\r?\n[ \t]*RewriteRule \^ index\.php \[QSA,L\][ \t]*\r?\n?/m'; private const NOGZIP_OLD = '/^([ \t]*)SetEnvIfNoCase Request_URI \\\\\.\(\?:gif\|jpe\?g\|png\|webp\|ico\|woff2\?\|gz\|zip\|epub\|mp4\|webm\|mp3\|m4a\|ogg\)\$ no-gzip dont-vary[ \t]*$/m'; private const NOGZIP_NEW = '$1SetEnvIfNoCase Request_URI \.(?:gif|jpe?g|png|webp|ico|woff2?|gz|zip|epub|pdf|mp4|webm|mp3|m4a|ogg|wav)$ no-gzip dont-vary'; - /** The content with the old shipped lines replaced; unchanged when there is nothing to do. */ + /** The content with the shipped rules brought to 0.8.6; unchanged when nothing applies or the result would not be safe. */ public static function apply(string $content): string { - $out = preg_replace(self::REWRITE_OLD, self::REWRITE_NEW, $content); - $out = is_string($out) ? preg_replace(self::NOGZIP_OLD, self::NOGZIP_NEW, $out) : null; - return is_string($out) ? $out : $content; + $out = $content; + if (preg_match(self::REWRITE_C, $out) === 1) { + $out = (string) preg_replace(self::REWRITE_C, '$1' . self::PRIVATE_RULE, $out); + } elseif (preg_match(self::PASSTHROUGH_B, $out) === 1 && preg_match(self::PRIVATE_B, $out) === 1) { + // Both halves of B, exactly as shipped: drop the REQUEST_URI private rule and put + // the per-directory pair where the public passthrough was, private rule first. + $out = (string) preg_replace(self::PRIVATE_B, '', $out, 1); + $out = (string) preg_replace_callback(self::PASSTHROUGH_B, static fn (array $m): string => self::block($m[1]), $out, 1); + } elseif (preg_match(self::PASSTHROUGH_A, $out) === 1 && !self::hasPrivateRule($out)) { + $out = (string) preg_replace_callback(self::PASSTHROUGH_A, static fn (array $m): string => self::block($m[1]), $out, 1); + } + $out = (string) preg_replace(self::NOGZIP_OLD, self::NOGZIP_NEW, $out); + + if ($out === $content) { + return $content; + } + // Whatever changed in the rewrite rules must leave the private rule, with [NC], ahead + // of every public /uploads/ rule; anything else is a file we do not understand. + return self::rulesChanged($content, $out) && !self::safe($out) ? $content : $out; } /** @@ -71,4 +106,41 @@ public static function heal(?string $path = null, ?string $backupDir = null): bo } } } + + /** Whether $content is in the safe 0.8.6 shape: the [NC] private rule before every public /uploads/ rule. */ + public static function safe(string $content): bool + { + $private = strpos($content, self::PRIVATE_RULE); + if ($private === false) { + return false; + } + foreach (['/^[ \t]*RewriteRule \^uploads\/ - \[L\]/m', '/^[ \t]*RewriteCond %\{REQUEST_URI\} \^\/uploads\/ \[NC\]/m'] as $public) { + if (preg_match($public, $content, $m, PREG_OFFSET_CAPTURE) === 1 && $m[0][1] < $private) { + return false; + } + } + return true; + } + + private static function hasPrivateRule(string $content): bool + { + return preg_match('/^[ \t]*Rewrite(?:Rule|Cond)[^\r\n]*uploads\/?\(digital\|archives\/documents\|storage\)/m', $content) === 1; + } + + private static function rulesChanged(string $before, string $after): bool + { + $rules = static fn (string $s): string => implode("\n", preg_grep('/^\s*Rewrite/', preg_split('/\r?\n/', $s) ?: []) ?: []); + return $rules($before) !== $rules($after); + } + + /** The 0.8.6 pair, at the indentation of the lines it replaces. */ + private static function block(string $indent): string + { + return $indent . "# Private uploads (Digital Library files, archive documents, generic storage) go\n" + . $indent . "# through index.php, which checks who may open them; [NC] keeps /uploads/Digital/\n" + . $indent . "# from slipping past on a case-insensitive disk. Added by the 0.8.6 upgrade (#480).\n" + . $indent . self::PRIVATE_RULE . "\n\n" + . $indent . "# Allow direct access to the remaining PUBLIC static files in /uploads/.\n" + . $indent . self::PUBLIC_RULE; + } } diff --git a/tests/fixtures/htaccess-history/0.1.1.htaccess b/tests/fixtures/htaccess-history/0.1.1.htaccess new file mode 100644 index 000000000..8156b551d --- /dev/null +++ b/tests/fixtures/htaccess-history/0.1.1.htaccess @@ -0,0 +1,35 @@ + + RewriteEngine On + + # Handle Authorization Header + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + + # Allow direct access to static files in /uploads/ directory + RewriteCond %{REQUEST_URI} ^/uploads/ [NC] + RewriteRule ^ - [L] + + # Redirect all requests to index.php + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteRule ^ index.php [QSA,L] + + +# Security Headers + + # Prevent MIME-type sniffing + Header always set X-Content-Type-Options "nosniff" + + # Enable XSS protection + Header always set X-XSS-Protection "1; mode=block" + + # Prevent clickjacking + Header always set X-Frame-Options "SAMEORIGIN" + + # Content Security Policy - restrictive but allows inline styles/scripts for compatibility + # All assets are self-hosted (fonts, styles, scripts) for privacy and performance + Header always set Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline'; font-src 'self' data:; img-src 'self' data: https: http: blob:; connect-src 'self'; frame-ancestors 'self';" + + # Referrer policy + Header always set Referrer-Policy "strict-origin-when-cross-origin" + diff --git a/tests/fixtures/htaccess-history/0.1.7.htaccess b/tests/fixtures/htaccess-history/0.1.7.htaccess new file mode 100644 index 000000000..3309653c3 --- /dev/null +++ b/tests/fixtures/htaccess-history/0.1.7.htaccess @@ -0,0 +1,81 @@ + + RewriteEngine On + + # Handle Authorization Header + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + + # Allow direct access to static files in /uploads/ directory + RewriteCond %{REQUEST_URI} ^/uploads/ [NC] + RewriteRule ^ - [L] + + # Redirect all requests to index.php + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteRule ^ index.php [QSA,L] + + +# Security Headers + + # Prevent MIME-type sniffing + Header always set X-Content-Type-Options "nosniff" + + # Enable XSS protection + Header always set X-XSS-Protection "1; mode=block" + + # Prevent clickjacking + Header always set X-Frame-Options "SAMEORIGIN" + + # Content Security Policy - restrictive but allows inline styles/scripts for compatibility + # Allows Google Fonts for Inter font family + Header always set Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' data: https://fonts.gstatic.com; img-src 'self' data: https: http: blob:; connect-src 'self'; frame-src 'self' https://www.openstreetmap.org; frame-ancestors 'self';" + + # Referrer policy + Header always set Referrer-Policy "strict-origin-when-cross-origin" + + +# Block access to sensitive files (defense in depth) +# Even though these files are outside webroot, this prevents misconfiguration attacks + + + Require all denied + + + Order deny,allow + Deny from all + + + +# Audio/Video Streaming Support (for Digital Library plugin) + + # Enable Range Requests for audio/video streaming + # Required for seek functionality in audio players + + Header set Accept-Ranges bytes + # Allow CORS for audio/video files (required for cross-origin playback) + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + Header set Access-Control-Allow-Headers "Range, Content-Type" + Header set Access-Control-Expose-Headers "Content-Length, Content-Range, Accept-Ranges" + + + # CORS for digital content in uploads/digital/ directory + + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + + + +# Correct MIME types for audio files + + AddType audio/mpeg .mp3 + AddType audio/mp4 .m4a + AddType audio/ogg .ogg + AddType audio/wav .wav + AddType audio/webm .weba + AddType video/mp4 .mp4 + AddType video/webm .webm + AddType video/ogg .ogv + AddType application/pdf .pdf + AddType application/epub+zip .epub + diff --git a/tests/fixtures/htaccess-history/0.4.5.htaccess b/tests/fixtures/htaccess-history/0.4.5.htaccess new file mode 100644 index 000000000..387b612d0 --- /dev/null +++ b/tests/fixtures/htaccess-history/0.4.5.htaccess @@ -0,0 +1,82 @@ + + Options +FollowSymLinks + RewriteEngine On + + # Handle Authorization Header + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + + # Allow direct access to static files in /uploads/ directory + RewriteCond %{REQUEST_URI} ^/uploads/ [NC] + RewriteRule ^ - [L] + + # Redirect all requests to index.php + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteRule ^ index.php [QSA,L] + + +# Security Headers + + # Prevent MIME-type sniffing + Header always set X-Content-Type-Options "nosniff" + + # Enable XSS protection + Header always set X-XSS-Protection "1; mode=block" + + # Prevent clickjacking + Header always set X-Frame-Options "SAMEORIGIN" + + # Content Security Policy - restrictive but allows inline styles/scripts for compatibility + # Allows Google Fonts for Inter font family + Header always set Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' 'unsafe-eval'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' data: https://fonts.gstatic.com; img-src 'self' data: https: http: blob:; connect-src 'self'; frame-src 'self' https://www.openstreetmap.org; frame-ancestors 'self';" + + # Referrer policy + Header always set Referrer-Policy "strict-origin-when-cross-origin" + + +# Block access to sensitive files (defense in depth) +# Even though these files are outside webroot, this prevents misconfiguration attacks + + + Require all denied + + + Order deny,allow + Deny from all + + + +# Audio/Video Streaming Support (for Digital Library plugin) + + # Enable Range Requests for audio/video streaming + # Required for seek functionality in audio players + + Header set Accept-Ranges bytes + # Allow CORS for audio/video files (required for cross-origin playback) + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + Header set Access-Control-Allow-Headers "Range, Content-Type" + Header set Access-Control-Expose-Headers "Content-Length, Content-Range, Accept-Ranges" + + + # CORS for digital content in uploads/digital/ directory + + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + + + +# Correct MIME types for audio files + + AddType audio/mpeg .mp3 + AddType audio/mp4 .m4a + AddType audio/ogg .ogg + AddType audio/wav .wav + AddType audio/webm .weba + AddType video/mp4 .mp4 + AddType video/webm .webm + AddType video/ogg .ogv + AddType application/pdf .pdf + AddType application/epub+zip .epub + diff --git a/tests/fixtures/htaccess-history/0.4.8.htaccess b/tests/fixtures/htaccess-history/0.4.8.htaccess new file mode 100644 index 000000000..c87c8b79f --- /dev/null +++ b/tests/fixtures/htaccess-history/0.4.8.htaccess @@ -0,0 +1,82 @@ + + Options +FollowSymLinks + RewriteEngine On + + # Handle Authorization Header + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + + # Allow direct access to static files in /uploads/ directory + RewriteCond %{REQUEST_URI} ^/uploads/ [NC] + RewriteRule ^ - [L] + + # Redirect all requests to index.php + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteRule ^ index.php [QSA,L] + + +# Security Headers + + # Prevent MIME-type sniffing + Header always set X-Content-Type-Options "nosniff" + + # Enable XSS protection + Header always set X-XSS-Protection "1; mode=block" + + # Prevent clickjacking + Header always set X-Frame-Options "SAMEORIGIN" + + # Content Security Policy - restrictive but allows inline styles/scripts for compatibility + # Allows Google Fonts for Inter font family + Header always set Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' https://cdnjs.cloudflare.com; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com https://cdnjs.cloudflare.com; font-src 'self' data: https://fonts.gstatic.com https://cdnjs.cloudflare.com; img-src 'self' data: https: http: blob:; connect-src 'self'; frame-src 'self' data: blob: about: https://www.openstreetmap.org; child-src 'self' data: blob: about:; frame-ancestors 'self';" + + # Referrer policy + Header always set Referrer-Policy "strict-origin-when-cross-origin" + + +# Block access to sensitive files (defense in depth) +# Even though these files are outside webroot, this prevents misconfiguration attacks + + + Require all denied + + + Order deny,allow + Deny from all + + + +# Audio/Video Streaming Support (for Digital Library plugin) + + # Enable Range Requests for audio/video streaming + # Required for seek functionality in audio players + + Header set Accept-Ranges bytes + # Allow CORS for audio/video files (required for cross-origin playback) + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + Header set Access-Control-Allow-Headers "Range, Content-Type" + Header set Access-Control-Expose-Headers "Content-Length, Content-Range, Accept-Ranges" + + + # CORS for digital content in uploads/digital/ directory + + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + + + +# Correct MIME types for audio files + + AddType audio/mpeg .mp3 + AddType audio/mp4 .m4a + AddType audio/ogg .ogg + AddType audio/wav .wav + AddType audio/webm .weba + AddType video/mp4 .mp4 + AddType video/webm .webm + AddType video/ogg .ogv + AddType application/pdf .pdf + AddType application/epub+zip .epub + diff --git a/tests/fixtures/htaccess-history/0.4.9.1.htaccess b/tests/fixtures/htaccess-history/0.4.9.1.htaccess new file mode 100644 index 000000000..89c1ec5ec --- /dev/null +++ b/tests/fixtures/htaccess-history/0.4.9.1.htaccess @@ -0,0 +1,95 @@ +# PHP upload limits (required for Digital Library plugin: eBook up to 100MB, audiobook up to 500MB) + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + Options -Indexes +FollowSymLinks + RewriteEngine On + + # Handle Authorization Header + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + + # Allow direct access to static files in /uploads/ directory + RewriteCond %{REQUEST_URI} ^/uploads/ [NC] + RewriteRule ^ - [L] + + # Redirect all requests to index.php + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteRule ^ index.php [QSA,L] + + +# Security Headers + + # Prevent MIME-type sniffing + Header always set X-Content-Type-Options "nosniff" + + # Enable XSS protection + Header always set X-XSS-Protection "1; mode=block" + + # Prevent clickjacking + Header always set X-Frame-Options "SAMEORIGIN" + + # Content Security Policy - restrictive but allows inline styles/scripts for compatibility + # Allows Google Fonts for Inter font family + Header always set Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' https://cdnjs.cloudflare.com; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com https://cdnjs.cloudflare.com; font-src 'self' data: https://fonts.gstatic.com https://cdnjs.cloudflare.com; img-src 'self' data: https: http: blob:; connect-src 'self'; frame-src 'self' data: blob: about: https://www.openstreetmap.org; child-src 'self' data: blob: about:; frame-ancestors 'self';" + + # Referrer policy + Header always set Referrer-Policy "strict-origin-when-cross-origin" + + +# Block access to sensitive files (defense in depth) +# Even though these files are outside webroot, this prevents misconfiguration attacks + + + Require all denied + + + Order deny,allow + Deny from all + + + +# Audio/Video Streaming Support (for Digital Library plugin) + + # Enable Range Requests for audio/video streaming + # Required for seek functionality in audio players + + Header set Accept-Ranges bytes + # Allow CORS for audio/video files (required for cross-origin playback) + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + Header set Access-Control-Allow-Headers "Range, Content-Type" + Header set Access-Control-Expose-Headers "Content-Length, Content-Range, Accept-Ranges" + + + # CORS for digital content in uploads/digital/ directory + + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + + + +# Correct MIME types for audio files + + AddType audio/mpeg .mp3 + AddType audio/mp4 .m4a + AddType audio/ogg .ogg + AddType audio/wav .wav + AddType audio/webm .weba + AddType video/mp4 .mp4 + AddType video/webm .webm + AddType video/ogg .ogv + AddType application/pdf .pdf + AddType application/epub+zip .epub + diff --git a/tests/fixtures/htaccess-history/0.7.13.htaccess b/tests/fixtures/htaccess-history/0.7.13.htaccess new file mode 100644 index 000000000..92fae583c --- /dev/null +++ b/tests/fixtures/htaccess-history/0.7.13.htaccess @@ -0,0 +1,149 @@ +# PHP upload limits (required for Digital Library plugin: eBook up to 100MB, audiobook up to 500MB) + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + Options -Indexes +FollowSymLinks + RewriteEngine On + + # Handle Authorization Header + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + + # Allow direct access to static files in /uploads/ directory + RewriteCond %{REQUEST_URI} ^/uploads/ [NC] + RewriteRule ^ - [L] + + # Redirect all requests to index.php + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteRule ^ index.php [QSA,L] + + +# === Pinakes performance block (auto-managed by Pinakes 0.7.13+) === +# HTTP compression + long-term caching for static assets. +# Asset URLs are version-busted via ?v=X.Y.Z (see HtmlHelper::asset), so +# cache invalidation happens automatically at every release — immutable+1y +# is safe for fingerprinted assets. +# Every directive is gated by so the file stays valid on +# hosts that don't ship mod_deflate / mod_expires / mod_headers / mod_brotli. + + + AddOutputFilterByType DEFLATE text/html text/plain text/css text/xml \ + text/javascript application/javascript application/x-javascript \ + application/json application/ld+json application/xml application/rss+xml \ + application/atom+xml application/xhtml+xml \ + image/svg+xml font/ttf font/otf font/woff + # Don't re-compress already-compressed payloads + SetEnvIfNoCase Request_URI \.(?:gif|jpe?g|png|webp|ico|woff2?|gz|zip|epub|mp4|webm|mp3|m4a|ogg)$ no-gzip dont-vary + + + + AddOutputFilterByType BROTLI_COMPRESS text/html text/plain text/css \ + text/javascript application/javascript application/json \ + application/ld+json application/xml image/svg+xml font/ttf font/otf + + + + ExpiresActive On + # 1 year for versioned static assets + ExpiresByType text/css "access plus 1 year" + ExpiresByType application/javascript "access plus 1 year" + ExpiresByType text/javascript "access plus 1 year" + ExpiresByType font/woff2 "access plus 1 year" + ExpiresByType font/woff "access plus 1 year" + ExpiresByType font/ttf "access plus 1 year" + ExpiresByType image/x-icon "access plus 1 year" + # 1 month for content images (book covers, uploaded media) + ExpiresByType image/jpeg "access plus 1 month" + ExpiresByType image/png "access plus 1 month" + ExpiresByType image/webp "access plus 1 month" + ExpiresByType image/gif "access plus 1 month" + ExpiresByType image/svg+xml "access plus 1 month" + + + + # Mark versioned assets as immutable — browsers skip revalidation until TTL. + # ?v= querystring busts the cache on every release, so this is safe. + + Header set Cache-Control "public, max-age=31536000, immutable" + + + Header set Cache-Control "public, max-age=2592000" + + +# === end Pinakes performance block === + +# Security Headers + + # Prevent MIME-type sniffing + Header always set X-Content-Type-Options "nosniff" + + # Enable XSS protection + Header always set X-XSS-Protection "1; mode=block" + + # Prevent clickjacking + Header always set X-Frame-Options "SAMEORIGIN" + + # Content Security Policy - restrictive but allows inline styles/scripts for compatibility + # Allows Google Fonts for Inter font family + Header always set Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' https://cdnjs.cloudflare.com; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com https://cdnjs.cloudflare.com; font-src 'self' data: https://fonts.gstatic.com https://cdnjs.cloudflare.com; img-src 'self' data: https: http: blob:; connect-src 'self'; frame-src 'self' data: blob: about: https://www.openstreetmap.org; child-src 'self' data: blob: about:; frame-ancestors 'self';" + + # Referrer policy + Header always set Referrer-Policy "strict-origin-when-cross-origin" + + +# Block access to sensitive files (defense in depth) +# Even though these files are outside webroot, this prevents misconfiguration attacks + + + Require all denied + + + Order deny,allow + Deny from all + + + +# Audio/Video Streaming Support (for Digital Library plugin) + + # Enable Range Requests for audio/video streaming + # Required for seek functionality in audio players + + Header set Accept-Ranges bytes + # Allow CORS for audio/video files (required for cross-origin playback) + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + Header set Access-Control-Allow-Headers "Range, Content-Type" + Header set Access-Control-Expose-Headers "Content-Length, Content-Range, Accept-Ranges" + + + # CORS for digital content in uploads/digital/ directory + + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + + + +# Correct MIME types for audio files + + AddType audio/mpeg .mp3 + AddType audio/mp4 .m4a + AddType audio/ogg .ogg + AddType audio/wav .wav + AddType audio/webm .weba + AddType video/mp4 .mp4 + AddType video/webm .webm + AddType video/ogg .ogv + AddType application/pdf .pdf + AddType application/epub+zip .epub + diff --git a/tests/fixtures/htaccess-history/0.7.18.htaccess b/tests/fixtures/htaccess-history/0.7.18.htaccess new file mode 100644 index 000000000..c917169a5 --- /dev/null +++ b/tests/fixtures/htaccess-history/0.7.18.htaccess @@ -0,0 +1,159 @@ +# PHP upload limits (required for Digital Library plugin: eBook up to 100MB, audiobook up to 500MB) + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + Options -Indexes +FollowSymLinks + RewriteEngine On + + # Handle Authorization Header + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + + # Allow direct access to PUBLIC static files in /uploads/ (covers, events, + # CMS images, branding). + RewriteCond %{REQUEST_URI} ^/uploads/ [NC] + RewriteCond %{REQUEST_URI} !^/uploads/(digital|archives/documents|storage)/ [NC] + RewriteRule ^ - [L] + + # PRIVATE uploaded content — digital-library files, archive documents and + # generic storage — must NOT be served directly. Force them through + # index.php (even when the file exists on disk, so the !-f rule below would + # otherwise skip them) so PrivateModeMiddleware (and any future per-file + # auth) governs access (issue #160). See ProtectedUploadController. + RewriteCond %{REQUEST_URI} ^/uploads/(digital|archives/documents|storage)/ [NC] + RewriteRule ^ index.php [QSA,L] + + # Redirect all requests to index.php + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteRule ^ index.php [QSA,L] + + +# === Pinakes performance block (auto-managed by Pinakes 0.7.13+) === +# HTTP compression + long-term caching for static assets. +# Asset URLs are version-busted via ?v=X.Y.Z (see HtmlHelper::asset), so +# cache invalidation happens automatically at every release — immutable+1y +# is safe for fingerprinted assets. +# Every directive is gated by so the file stays valid on +# hosts that don't ship mod_deflate / mod_expires / mod_headers / mod_brotli. + + + AddOutputFilterByType DEFLATE text/html text/plain text/css text/xml \ + text/javascript application/javascript application/x-javascript \ + application/json application/ld+json application/xml application/rss+xml \ + application/atom+xml application/xhtml+xml \ + image/svg+xml font/ttf font/otf font/woff + # Don't re-compress already-compressed payloads + SetEnvIfNoCase Request_URI \.(?:gif|jpe?g|png|webp|ico|woff2?|gz|zip|epub|mp4|webm|mp3|m4a|ogg)$ no-gzip dont-vary + + + + AddOutputFilterByType BROTLI_COMPRESS text/html text/plain text/css \ + text/javascript application/javascript application/json \ + application/ld+json application/xml image/svg+xml font/ttf font/otf + + + + ExpiresActive On + # 1 year for versioned static assets + ExpiresByType text/css "access plus 1 year" + ExpiresByType application/javascript "access plus 1 year" + ExpiresByType text/javascript "access plus 1 year" + ExpiresByType font/woff2 "access plus 1 year" + ExpiresByType font/woff "access plus 1 year" + ExpiresByType font/ttf "access plus 1 year" + ExpiresByType image/x-icon "access plus 1 year" + # 1 month for content images (book covers, uploaded media) + ExpiresByType image/jpeg "access plus 1 month" + ExpiresByType image/png "access plus 1 month" + ExpiresByType image/webp "access plus 1 month" + ExpiresByType image/gif "access plus 1 month" + ExpiresByType image/svg+xml "access plus 1 month" + + + + # Mark versioned assets as immutable — browsers skip revalidation until TTL. + # ?v= querystring busts the cache on every release, so this is safe. + + Header set Cache-Control "public, max-age=31536000, immutable" + + + Header set Cache-Control "public, max-age=2592000" + + +# === end Pinakes performance block === + +# Security Headers + + # Prevent MIME-type sniffing + Header always set X-Content-Type-Options "nosniff" + + # Enable XSS protection + Header always set X-XSS-Protection "1; mode=block" + + # Prevent clickjacking + Header always set X-Frame-Options "SAMEORIGIN" + + # Content Security Policy - restrictive but allows inline styles/scripts for compatibility + # Allows Google Fonts for Inter font family + Header always set Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' https://cdnjs.cloudflare.com; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com https://cdnjs.cloudflare.com; font-src 'self' data: https://fonts.gstatic.com https://cdnjs.cloudflare.com; img-src 'self' data: https: http: blob:; connect-src 'self'; frame-src 'self' data: blob: about: https://www.openstreetmap.org; child-src 'self' data: blob: about:; frame-ancestors 'self';" + + # Referrer policy + Header always set Referrer-Policy "strict-origin-when-cross-origin" + + +# Block access to sensitive files (defense in depth) +# Even though these files are outside webroot, this prevents misconfiguration attacks + + + Require all denied + + + Order deny,allow + Deny from all + + + +# Audio/Video Streaming Support (for Digital Library plugin) + + # Enable Range Requests for audio/video streaming + # Required for seek functionality in audio players + + Header set Accept-Ranges bytes + # Allow CORS for audio/video files (required for cross-origin playback) + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + Header set Access-Control-Allow-Headers "Range, Content-Type" + Header set Access-Control-Expose-Headers "Content-Length, Content-Range, Accept-Ranges" + + + # CORS for digital content in uploads/digital/ directory + + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + + + +# Correct MIME types for audio files + + AddType audio/mpeg .mp3 + AddType audio/mp4 .m4a + AddType audio/ogg .ogg + AddType audio/wav .wav + AddType audio/webm .weba + AddType video/mp4 .mp4 + AddType video/webm .webm + AddType video/ogg .ogv + AddType application/pdf .pdf + AddType application/epub+zip .epub + diff --git a/tests/fixtures/htaccess-history/0.7.21.htaccess b/tests/fixtures/htaccess-history/0.7.21.htaccess new file mode 100644 index 000000000..7d5c4b422 --- /dev/null +++ b/tests/fixtures/htaccess-history/0.7.21.htaccess @@ -0,0 +1,168 @@ +# PHP upload limits (required for Digital Library plugin: eBook up to 100MB, audiobook up to 500MB) +# +# NOTE: these php_value directives ONLY take effect when PHP runs as an Apache +# module (mod_php). On php-fpm / FastCGI / CGI / suExec hosting — the common +# case on cPanel and most shared hosts — the blocks are +# skipped and these limits are IGNORED. There the server's own PHP config wins, +# so raise post_max_size and upload_max_filesize via php.ini / MultiPHP INI +# Editor / the php-fpm pool (php_admin_value[...]). The ~30MB admin-UI upgrade +# ZIP needs post_max_size >= 32M; if it's lower the upload is rejected and the +# app surfaces a clear 413 "file too large" message (see CsrfMiddleware). + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + Options -Indexes +FollowSymLinks + RewriteEngine On + + # Handle Authorization Header + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + + # Allow direct access to PUBLIC static files in /uploads/ (covers, events, + # CMS images, branding). + RewriteCond %{REQUEST_URI} ^/uploads/ [NC] + RewriteCond %{REQUEST_URI} !^/uploads/(digital|archives/documents|storage)/ [NC] + RewriteRule ^ - [L] + + # PRIVATE uploaded content — digital-library files, archive documents and + # generic storage — must NOT be served directly. Force them through + # index.php (even when the file exists on disk, so the !-f rule below would + # otherwise skip them) so PrivateModeMiddleware (and any future per-file + # auth) governs access (issue #160). See ProtectedUploadController. + RewriteCond %{REQUEST_URI} ^/uploads/(digital|archives/documents|storage)/ [NC] + RewriteRule ^ index.php [QSA,L] + + # Redirect all requests to index.php + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteRule ^ index.php [QSA,L] + + +# === Pinakes performance block (auto-managed by Pinakes 0.7.13+) === +# HTTP compression + long-term caching for static assets. +# Asset URLs are version-busted via ?v=X.Y.Z (see HtmlHelper::asset), so +# cache invalidation happens automatically at every release — immutable+1y +# is safe for fingerprinted assets. +# Every directive is gated by so the file stays valid on +# hosts that don't ship mod_deflate / mod_expires / mod_headers / mod_brotli. + + + AddOutputFilterByType DEFLATE text/html text/plain text/css text/xml \ + text/javascript application/javascript application/x-javascript \ + application/json application/ld+json application/xml application/rss+xml \ + application/atom+xml application/xhtml+xml \ + image/svg+xml font/ttf font/otf font/woff + # Don't re-compress already-compressed payloads + SetEnvIfNoCase Request_URI \.(?:gif|jpe?g|png|webp|ico|woff2?|gz|zip|epub|mp4|webm|mp3|m4a|ogg)$ no-gzip dont-vary + + + + AddOutputFilterByType BROTLI_COMPRESS text/html text/plain text/css \ + text/javascript application/javascript application/json \ + application/ld+json application/xml image/svg+xml font/ttf font/otf + + + + ExpiresActive On + # 1 year for versioned static assets + ExpiresByType text/css "access plus 1 year" + ExpiresByType application/javascript "access plus 1 year" + ExpiresByType text/javascript "access plus 1 year" + ExpiresByType font/woff2 "access plus 1 year" + ExpiresByType font/woff "access plus 1 year" + ExpiresByType font/ttf "access plus 1 year" + ExpiresByType image/x-icon "access plus 1 year" + # 1 month for content images (book covers, uploaded media) + ExpiresByType image/jpeg "access plus 1 month" + ExpiresByType image/png "access plus 1 month" + ExpiresByType image/webp "access plus 1 month" + ExpiresByType image/gif "access plus 1 month" + ExpiresByType image/svg+xml "access plus 1 month" + + + + # Mark versioned assets as immutable — browsers skip revalidation until TTL. + # ?v= querystring busts the cache on every release, so this is safe. + + Header set Cache-Control "public, max-age=31536000, immutable" + + + Header set Cache-Control "public, max-age=2592000" + + +# === end Pinakes performance block === + +# Security Headers + + # Prevent MIME-type sniffing + Header always set X-Content-Type-Options "nosniff" + + # Enable XSS protection + Header always set X-XSS-Protection "1; mode=block" + + # Prevent clickjacking + Header always set X-Frame-Options "SAMEORIGIN" + + # Content Security Policy - restrictive but allows inline styles/scripts for compatibility + # Allows Google Fonts for Inter font family + Header always set Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' https://cdnjs.cloudflare.com; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com https://cdnjs.cloudflare.com; font-src 'self' data: https://fonts.gstatic.com https://cdnjs.cloudflare.com; img-src 'self' data: https: http: blob:; connect-src 'self'; frame-src 'self' data: blob: about: https://www.openstreetmap.org; child-src 'self' data: blob: about:; frame-ancestors 'self';" + + # Referrer policy + Header always set Referrer-Policy "strict-origin-when-cross-origin" + + +# Block access to sensitive files (defense in depth) +# Even though these files are outside webroot, this prevents misconfiguration attacks + + + Require all denied + + + Order deny,allow + Deny from all + + + +# Audio/Video Streaming Support (for Digital Library plugin) + + # Enable Range Requests for audio/video streaming + # Required for seek functionality in audio players + + Header set Accept-Ranges bytes + # Allow CORS for audio/video files (required for cross-origin playback) + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + Header set Access-Control-Allow-Headers "Range, Content-Type" + Header set Access-Control-Expose-Headers "Content-Length, Content-Range, Accept-Ranges" + + + # CORS for digital content in uploads/digital/ directory + + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + + + +# Correct MIME types for audio files + + AddType audio/mpeg .mp3 + AddType audio/mp4 .m4a + AddType audio/ogg .ogg + AddType audio/wav .wav + AddType audio/webm .weba + AddType video/mp4 .mp4 + AddType video/webm .webm + AddType video/ogg .ogv + AddType application/pdf .pdf + AddType application/epub+zip .epub + diff --git a/tests/fixtures/htaccess-history/0.7.31.htaccess b/tests/fixtures/htaccess-history/0.7.31.htaccess new file mode 100644 index 000000000..28851c243 --- /dev/null +++ b/tests/fixtures/htaccess-history/0.7.31.htaccess @@ -0,0 +1,168 @@ +# PHP upload limits (required for Digital Library plugin: eBook up to 100MB, audiobook up to 500MB) +# +# NOTE: these php_value directives ONLY take effect when PHP runs as an Apache +# module (mod_php). On php-fpm / FastCGI / CGI / suExec hosting — the common +# case on cPanel and most shared hosts — the blocks are +# skipped and these limits are IGNORED. There the server's own PHP config wins, +# so raise post_max_size and upload_max_filesize via php.ini / MultiPHP INI +# Editor / the php-fpm pool (php_admin_value[...]). The ~30MB admin-UI upgrade +# ZIP needs post_max_size >= 32M; if it's lower the upload is rejected and the +# app surfaces a clear 413 "file too large" message (see CsrfMiddleware). + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + Options -Indexes +FollowSymLinks + RewriteEngine On + + # Handle Authorization Header + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + + # Allow direct access to PUBLIC static files in /uploads/ (covers, events, + # CMS images, branding). + RewriteCond %{REQUEST_URI} ^/uploads/ [NC] + RewriteCond %{REQUEST_URI} !^/uploads/(digital|archives/documents|storage)/ [NC] + RewriteRule ^ - [L] + + # PRIVATE uploaded content — digital-library files, archive documents and + # generic storage — must NOT be served directly. Force them through + # index.php (even when the file exists on disk, so the !-f rule below would + # otherwise skip them) so PrivateModeMiddleware (and any future per-file + # auth) governs access (issue #160). See ProtectedUploadController. + RewriteCond %{REQUEST_URI} ^/uploads/(digital|archives/documents|storage)/ [NC] + RewriteRule ^ index.php [QSA,L] + + # Redirect all requests to index.php + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteRule ^ index.php [QSA,L] + + +# === Pinakes performance block (auto-managed by Pinakes 0.7.13+) === +# HTTP compression + long-term caching for static assets. +# Asset URLs are version-busted via ?v=X.Y.Z (see HtmlHelper::asset), so +# cache invalidation happens automatically at every release — immutable+1y +# is safe for fingerprinted assets. +# Every directive is gated by so the file stays valid on +# hosts that don't ship mod_deflate / mod_expires / mod_headers / mod_brotli. + + + AddOutputFilterByType DEFLATE text/html text/plain text/css text/xml \ + text/javascript application/javascript application/x-javascript \ + application/json application/ld+json application/xml application/rss+xml \ + application/atom+xml application/xhtml+xml \ + image/svg+xml font/ttf font/otf font/woff + # Don't re-compress already-compressed payloads + SetEnvIfNoCase Request_URI \.(?:gif|jpe?g|png|webp|ico|woff2?|gz|zip|epub|mp4|webm|mp3|m4a|ogg)$ no-gzip dont-vary + + + + AddOutputFilterByType BROTLI_COMPRESS text/html text/plain text/css \ + text/javascript application/javascript application/json \ + application/ld+json application/xml image/svg+xml font/ttf font/otf + + + + ExpiresActive On + # 1 year for versioned static assets + ExpiresByType text/css "access plus 1 year" + ExpiresByType application/javascript "access plus 1 year" + ExpiresByType text/javascript "access plus 1 year" + ExpiresByType font/woff2 "access plus 1 year" + ExpiresByType font/woff "access plus 1 year" + ExpiresByType font/ttf "access plus 1 year" + ExpiresByType image/x-icon "access plus 1 year" + # 1 month for content images (book covers, uploaded media) + ExpiresByType image/jpeg "access plus 1 month" + ExpiresByType image/png "access plus 1 month" + ExpiresByType image/webp "access plus 1 month" + ExpiresByType image/gif "access plus 1 month" + ExpiresByType image/svg+xml "access plus 1 month" + + + + # Mark versioned assets as immutable — browsers skip revalidation until TTL. + # ?v= querystring busts the cache on every release, so this is safe. + + Header set Cache-Control "public, max-age=31536000, immutable" + + + Header set Cache-Control "public, max-age=2592000" + + +# === end Pinakes performance block === + +# Security Headers + + # Prevent MIME-type sniffing + Header always set X-Content-Type-Options "nosniff" + + # Enable XSS protection + Header always set X-XSS-Protection "1; mode=block" + + # Prevent clickjacking + Header always set X-Frame-Options "SAMEORIGIN" + + # Content Security Policy - restrictive but allows inline styles/scripts for compatibility + # Allows Google Fonts for Inter font family + Header always set Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline' 'wasm-unsafe-eval' https://cdnjs.cloudflare.com; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com https://cdnjs.cloudflare.com; font-src 'self' data: https://fonts.gstatic.com https://cdnjs.cloudflare.com; img-src 'self' data: https: http: blob:; connect-src 'self'; frame-src 'self' data: blob: about: https://www.openstreetmap.org; child-src 'self' data: blob: about:; frame-ancestors 'self';" + + # Referrer policy + Header always set Referrer-Policy "strict-origin-when-cross-origin" + + +# Block access to sensitive files (defense in depth) +# Even though these files are outside webroot, this prevents misconfiguration attacks + + + Require all denied + + + Order deny,allow + Deny from all + + + +# Audio/Video Streaming Support (for Digital Library plugin) + + # Enable Range Requests for audio/video streaming + # Required for seek functionality in audio players + + Header set Accept-Ranges bytes + # Allow CORS for audio/video files (required for cross-origin playback) + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + Header set Access-Control-Allow-Headers "Range, Content-Type" + Header set Access-Control-Expose-Headers "Content-Length, Content-Range, Accept-Ranges" + + + # CORS for digital content in uploads/digital/ directory + + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + + + +# Correct MIME types for audio files + + AddType audio/mpeg .mp3 + AddType audio/mp4 .m4a + AddType audio/ogg .ogg + AddType audio/wav .wav + AddType audio/webm .weba + AddType video/mp4 .mp4 + AddType video/webm .webm + AddType video/ogg .ogv + AddType application/pdf .pdf + AddType application/epub+zip .epub + diff --git a/tests/fixtures/htaccess-history/0.7.59.htaccess b/tests/fixtures/htaccess-history/0.7.59.htaccess new file mode 100644 index 000000000..7f0089580 --- /dev/null +++ b/tests/fixtures/htaccess-history/0.7.59.htaccess @@ -0,0 +1,168 @@ +# PHP upload limits (required for Digital Library plugin: eBook up to 100MB, audiobook up to 500MB) +# +# NOTE: these php_value directives ONLY take effect when PHP runs as an Apache +# module (mod_php). On php-fpm / FastCGI / CGI / suExec hosting — the common +# case on cPanel and most shared hosts — the blocks are +# skipped and these limits are IGNORED. There the server's own PHP config wins, +# so raise post_max_size and upload_max_filesize via php.ini / MultiPHP INI +# Editor / the php-fpm pool (php_admin_value[...]). The ~30MB admin-UI upgrade +# ZIP needs post_max_size >= 32M; if it's lower the upload is rejected and the +# app surfaces a clear 413 "file too large" message (see CsrfMiddleware). + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + Options -Indexes +FollowSymLinks + RewriteEngine On + + # Handle Authorization Header + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + + # Allow direct access to PUBLIC static files in /uploads/ (covers, events, + # CMS images, branding). + RewriteCond %{REQUEST_URI} ^/uploads/ [NC] + RewriteCond %{REQUEST_URI} !^/uploads/(digital|archives/documents|storage)/ [NC] + RewriteRule ^ - [L] + + # PRIVATE uploaded content — digital-library files, archive documents and + # generic storage — must NOT be served directly. Force them through + # index.php (even when the file exists on disk, so the !-f rule below would + # otherwise skip them) so PrivateModeMiddleware (and any future per-file + # auth) governs access (issue #160). See ProtectedUploadController. + RewriteCond %{REQUEST_URI} ^/uploads/(digital|archives/documents|storage)/ [NC] + RewriteRule ^ index.php [QSA,L] + + # Redirect all requests to index.php + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteRule ^ index.php [QSA,L] + + +# === Pinakes performance block (auto-managed by Pinakes 0.7.13+) === +# HTTP compression + long-term caching for static assets. +# Asset URLs are version-busted via ?v=X.Y.Z (see HtmlHelper::asset), so +# cache invalidation happens automatically at every release — immutable+1y +# is safe for fingerprinted assets. +# Every directive is gated by so the file stays valid on +# hosts that don't ship mod_deflate / mod_expires / mod_headers / mod_brotli. + + + AddOutputFilterByType DEFLATE text/html text/plain text/css text/xml \ + text/javascript application/javascript application/x-javascript \ + application/json application/ld+json application/xml application/rss+xml \ + application/atom+xml application/xhtml+xml \ + image/svg+xml font/ttf font/otf font/woff + # Don't re-compress already-compressed payloads + SetEnvIfNoCase Request_URI \.(?:gif|jpe?g|png|webp|ico|woff2?|gz|zip|epub|mp4|webm|mp3|m4a|ogg)$ no-gzip dont-vary + + + + AddOutputFilterByType BROTLI_COMPRESS text/html text/plain text/css \ + text/javascript application/javascript application/json \ + application/ld+json application/xml image/svg+xml font/ttf font/otf + + + + ExpiresActive On + # 1 year for versioned static assets + ExpiresByType text/css "access plus 1 year" + ExpiresByType application/javascript "access plus 1 year" + ExpiresByType text/javascript "access plus 1 year" + ExpiresByType font/woff2 "access plus 1 year" + ExpiresByType font/woff "access plus 1 year" + ExpiresByType font/ttf "access plus 1 year" + ExpiresByType image/x-icon "access plus 1 year" + # 1 month for content images (book covers, uploaded media) + ExpiresByType image/jpeg "access plus 1 month" + ExpiresByType image/png "access plus 1 month" + ExpiresByType image/webp "access plus 1 month" + ExpiresByType image/gif "access plus 1 month" + ExpiresByType image/svg+xml "access plus 1 month" + + + + # Mark versioned assets as immutable — browsers skip revalidation until TTL. + # ?v= querystring busts the cache on every release, so this is safe. + + Header set Cache-Control "public, max-age=31536000, immutable" + + + Header set Cache-Control "public, max-age=2592000" + + +# === end Pinakes performance block === + +# Security Headers + + # Prevent MIME-type sniffing + Header always set X-Content-Type-Options "nosniff" + + # Enable XSS protection + Header always set X-XSS-Protection "1; mode=block" + + # Prevent clickjacking + Header always set X-Frame-Options "SAMEORIGIN" + + # The application emits a per-response nonce-based Content-Security-Policy. + # Do not override it here: a static header would either break inline elements + # or require unsafe-inline for every response. + + # Referrer policy + Header always set Referrer-Policy "strict-origin-when-cross-origin" + + +# Block access to sensitive files (defense in depth) +# Even though these files are outside webroot, this prevents misconfiguration attacks + + + Require all denied + + + Order deny,allow + Deny from all + + + +# Audio/Video Streaming Support (for Digital Library plugin) + + # Enable Range Requests for audio/video streaming + # Required for seek functionality in audio players + + Header set Accept-Ranges bytes + # Allow CORS for audio/video files (required for cross-origin playback) + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + Header set Access-Control-Allow-Headers "Range, Content-Type" + Header set Access-Control-Expose-Headers "Content-Length, Content-Range, Accept-Ranges" + + + # CORS for digital content in uploads/digital/ directory + + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + + + +# Correct MIME types for audio files + + AddType audio/mpeg .mp3 + AddType audio/mp4 .m4a + AddType audio/ogg .ogg + AddType audio/wav .wav + AddType audio/webm .weba + AddType video/mp4 .mp4 + AddType video/webm .webm + AddType video/ogg .ogv + AddType application/pdf .pdf + AddType application/epub+zip .epub + diff --git a/tests/fixtures/htaccess-history/0.7.71-rc.1.htaccess b/tests/fixtures/htaccess-history/0.7.71-rc.1.htaccess new file mode 100644 index 000000000..11a5e9cf7 --- /dev/null +++ b/tests/fixtures/htaccess-history/0.7.71-rc.1.htaccess @@ -0,0 +1,191 @@ +# PHP upload limits (required for Digital Library plugin: eBook up to 100MB, audiobook up to 500MB) +# +# NOTE: these php_value directives ONLY take effect when PHP runs as an Apache +# module (mod_php). On php-fpm / FastCGI / CGI / suExec hosting — the common +# case on cPanel and most shared hosts — the blocks are +# skipped and these limits are IGNORED. There the server's own PHP config wins, +# so raise post_max_size and upload_max_filesize via php.ini / MultiPHP INI +# Editor / the php-fpm pool (php_admin_value[...]). The ~30MB admin-UI upgrade +# ZIP needs post_max_size >= 32M; if it's lower the upload is rejected and the +# app surfaces a clear 413 "file too large" message (see CsrfMiddleware). + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + Options -Indexes +FollowSymLinks + RewriteEngine On + + # Handle Authorization Header + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + + # === Pinakes LiteSpeed privacy bypass === + # Application response headers are too late + # when LSCache can answer before PHP, so any mutation, Authorization header + # or visitor-specific cookie prevents a cache hit at the web-server layer. + # A sole validated locale cookie is safe and is varied independently at + # lookup time and by the application's X-LiteSpeed-Vary response header. + + RewriteRule .* - [E=Cache-Vary:pinakes_locale] + + RewriteCond %{REQUEST_METHOD} !^(GET|HEAD)$ [NC] + RewriteRule .* - [E=Cache-Control:no-cache] + + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=Cache-Control:no-cache] + + RewriteCond %{HTTP:Cookie} !^$ + RewriteCond %{HTTP:Cookie} !^\s*pinakes_locale=[A-Za-z]{2}_[A-Za-z]{2}\s*;?\s*$ [NC] + RewriteRule .* - [E=Cache-Control:no-cache] + + # === end Pinakes LiteSpeed privacy bypass === + + # PRIVATE uploaded content — digital-library files, archive documents and + # generic storage — must NOT be served directly. Force them through + # index.php (even when the file exists on disk, so the !-f rule below would + # otherwise skip them) so PrivateModeMiddleware (and any future per-file + # auth) governs access (issue #160). See ProtectedUploadController. + # A relative RewriteRule pattern is matched against the per-directory + # stripped path, so it stays correct under a subdirectory install where + # %{REQUEST_URI} carries the extra prefix (an anchored ^/uploads/ would not + # match there and would leak the private files). Private rule first so its + # [L] wins before the public catch-all below. + RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [QSA,L] + + # Allow direct access to the remaining PUBLIC static files in /uploads/ + # (covers, events, CMS images, branding). + RewriteRule ^uploads/ - [L] + + # Redirect all requests to index.php + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteRule ^ index.php [QSA,L] + + +# === Pinakes performance block (auto-managed by Pinakes 0.7.13+) === +# HTTP compression + long-term caching for static assets. +# Asset URLs are version-busted via ?v=X.Y.Z (see HtmlHelper::asset), so +# cache invalidation happens automatically at every release — immutable+1y +# is safe for fingerprinted assets. +# Every directive is gated by so the file stays valid on +# hosts that don't ship mod_deflate / mod_expires / mod_headers / mod_brotli. + + + AddOutputFilterByType DEFLATE text/html text/plain text/css text/xml \ + text/javascript application/javascript application/x-javascript \ + application/json application/ld+json application/xml application/rss+xml \ + application/atom+xml application/xhtml+xml \ + image/svg+xml font/ttf font/otf font/woff + # Don't re-compress already-compressed payloads + SetEnvIfNoCase Request_URI \.(?:gif|jpe?g|png|webp|ico|woff2?|gz|zip|epub|mp4|webm|mp3|m4a|ogg)$ no-gzip dont-vary + + + + AddOutputFilterByType BROTLI_COMPRESS text/html text/plain text/css \ + text/javascript application/javascript application/json \ + application/ld+json application/xml image/svg+xml font/ttf font/otf + + + + ExpiresActive On + # 1 year for versioned static assets + ExpiresByType text/css "access plus 1 year" + ExpiresByType application/javascript "access plus 1 year" + ExpiresByType text/javascript "access plus 1 year" + ExpiresByType font/woff2 "access plus 1 year" + ExpiresByType font/woff "access plus 1 year" + ExpiresByType font/ttf "access plus 1 year" + ExpiresByType image/x-icon "access plus 1 year" + # 1 month for content images (book covers, uploaded media) + ExpiresByType image/jpeg "access plus 1 month" + ExpiresByType image/png "access plus 1 month" + ExpiresByType image/webp "access plus 1 month" + ExpiresByType image/gif "access plus 1 month" + ExpiresByType image/svg+xml "access plus 1 month" + + + + # Mark versioned assets as immutable — browsers skip revalidation until TTL. + # ?v= querystring busts the cache on every release, so this is safe. + + Header set Cache-Control "public, max-age=31536000, immutable" + + + Header set Cache-Control "public, max-age=2592000" + + +# === end Pinakes performance block === + +# Security Headers + + # Prevent MIME-type sniffing + Header always set X-Content-Type-Options "nosniff" + + # Enable XSS protection + Header always set X-XSS-Protection "1; mode=block" + + # Prevent clickjacking + Header always set X-Frame-Options "SAMEORIGIN" + + # The application emits a per-response nonce-based Content-Security-Policy. + # Do not override it here: a static header would either break inline elements + # or require unsafe-inline for every response. + + # Referrer policy + Header always set Referrer-Policy "strict-origin-when-cross-origin" + + +# Block access to sensitive files (defense in depth) +# Even though these files are outside webroot, this prevents misconfiguration attacks + + + Require all denied + + + Order deny,allow + Deny from all + + + +# Audio/Video Streaming Support (for Digital Library plugin) + + # Enable Range Requests for audio/video streaming + # Required for seek functionality in audio players + + Header set Accept-Ranges bytes + # Allow CORS for audio/video files (required for cross-origin playback) + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + Header set Access-Control-Allow-Headers "Range, Content-Type" + Header set Access-Control-Expose-Headers "Content-Length, Content-Range, Accept-Ranges" + + + # CORS for digital content in uploads/digital/ directory + + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + + + +# Correct MIME types for audio files + + AddType audio/mpeg .mp3 + AddType audio/mp4 .m4a + AddType audio/ogg .ogg + AddType audio/wav .wav + AddType audio/webm .weba + AddType video/mp4 .mp4 + AddType video/webm .webm + AddType video/ogg .ogv + AddType application/pdf .pdf + AddType application/epub+zip .epub + diff --git a/tests/fixtures/htaccess-history/0.7.71.htaccess b/tests/fixtures/htaccess-history/0.7.71.htaccess new file mode 100644 index 000000000..39fde00bf --- /dev/null +++ b/tests/fixtures/htaccess-history/0.7.71.htaccess @@ -0,0 +1,196 @@ +# PHP upload limits (required for Digital Library plugin: eBook up to 100MB, audiobook up to 500MB) +# +# NOTE: these php_value directives ONLY take effect when PHP runs as an Apache +# module (mod_php). On php-fpm / FastCGI / CGI / suExec hosting — the common +# case on cPanel and most shared hosts — the blocks are +# skipped and these limits are IGNORED. There the server's own PHP config wins, +# so raise post_max_size and upload_max_filesize via php.ini / MultiPHP INI +# Editor / the php-fpm pool (php_admin_value[...]). The ~30MB admin-UI upgrade +# ZIP needs post_max_size >= 32M; if it's lower the upload is rejected and the +# app surfaces a clear 413 "file too large" message (see CsrfMiddleware). + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + Options -Indexes +FollowSymLinks + RewriteEngine On + + # Handle Authorization Header + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + + # === Pinakes LiteSpeed privacy bypass === + # Application response headers are too late + # when LSCache can answer before PHP, so any mutation, Authorization header + # or visitor-specific cookie prevents a cache hit at the web-server layer. + # A sole validated locale cookie is safe and is varied independently at + # lookup time and by the application's X-LiteSpeed-Vary response header. + + # Enable LSCache request lookup/store for this vhost. Without it LSWS + # ignores the X-LiteSpeed-Cache-Control response headers and no page is + # ever served from edge cache. + CacheLookup on + + RewriteRule .* - [E=Cache-Vary:pinakes_locale] + + RewriteCond %{REQUEST_METHOD} !^(GET|HEAD)$ [NC] + RewriteRule .* - [E=Cache-Control:no-cache] + + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=Cache-Control:no-cache] + + RewriteCond %{HTTP:Cookie} !^$ + RewriteCond %{HTTP:Cookie} !^\s*pinakes_locale=[A-Za-z]{2}_[A-Za-z]{2}\s*;?\s*$ [NC] + RewriteRule .* - [E=Cache-Control:no-cache] + + # === end Pinakes LiteSpeed privacy bypass === + + # PRIVATE uploaded content — digital-library files, archive documents and + # generic storage — must NOT be served directly. Force them through + # index.php (even when the file exists on disk, so the !-f rule below would + # otherwise skip them) so PrivateModeMiddleware (and any future per-file + # auth) governs access (issue #160). See ProtectedUploadController. + # A relative RewriteRule pattern is matched against the per-directory + # stripped path, so it stays correct under a subdirectory install where + # %{REQUEST_URI} carries the extra prefix (an anchored ^/uploads/ would not + # match there and would leak the private files). Private rule first so its + # [L] wins before the public catch-all below. + RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [QSA,L] + + # Allow direct access to the remaining PUBLIC static files in /uploads/ + # (covers, events, CMS images, branding). + RewriteRule ^uploads/ - [L] + + # Redirect all requests to index.php + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteRule ^ index.php [QSA,L] + + +# === Pinakes performance block (auto-managed by Pinakes 0.7.13+) === +# HTTP compression + long-term caching for static assets. +# Asset URLs are version-busted via ?v=X.Y.Z (see HtmlHelper::asset), so +# cache invalidation happens automatically at every release — immutable+1y +# is safe for fingerprinted assets. +# Every directive is gated by so the file stays valid on +# hosts that don't ship mod_deflate / mod_expires / mod_headers / mod_brotli. + + + AddOutputFilterByType DEFLATE text/html text/plain text/css text/xml \ + text/javascript application/javascript application/x-javascript \ + application/json application/ld+json application/xml application/rss+xml \ + application/atom+xml application/xhtml+xml \ + image/svg+xml font/ttf font/otf font/woff + # Don't re-compress already-compressed payloads + SetEnvIfNoCase Request_URI \.(?:gif|jpe?g|png|webp|ico|woff2?|gz|zip|epub|mp4|webm|mp3|m4a|ogg)$ no-gzip dont-vary + + + + AddOutputFilterByType BROTLI_COMPRESS text/html text/plain text/css \ + text/javascript application/javascript application/json \ + application/ld+json application/xml image/svg+xml font/ttf font/otf + + + + ExpiresActive On + # 1 year for versioned static assets + ExpiresByType text/css "access plus 1 year" + ExpiresByType application/javascript "access plus 1 year" + ExpiresByType text/javascript "access plus 1 year" + ExpiresByType font/woff2 "access plus 1 year" + ExpiresByType font/woff "access plus 1 year" + ExpiresByType font/ttf "access plus 1 year" + ExpiresByType image/x-icon "access plus 1 year" + # 1 month for content images (book covers, uploaded media) + ExpiresByType image/jpeg "access plus 1 month" + ExpiresByType image/png "access plus 1 month" + ExpiresByType image/webp "access plus 1 month" + ExpiresByType image/gif "access plus 1 month" + ExpiresByType image/svg+xml "access plus 1 month" + + + + # Mark versioned assets as immutable — browsers skip revalidation until TTL. + # ?v= querystring busts the cache on every release, so this is safe. + + Header set Cache-Control "public, max-age=31536000, immutable" + + + Header set Cache-Control "public, max-age=2592000" + + +# === end Pinakes performance block === + +# Security Headers + + # Prevent MIME-type sniffing + Header always set X-Content-Type-Options "nosniff" + + # Enable XSS protection + Header always set X-XSS-Protection "1; mode=block" + + # Prevent clickjacking + Header always set X-Frame-Options "SAMEORIGIN" + + # The application emits a per-response nonce-based Content-Security-Policy. + # Do not override it here: a static header would either break inline elements + # or require unsafe-inline for every response. + + # Referrer policy + Header always set Referrer-Policy "strict-origin-when-cross-origin" + + +# Block access to sensitive files (defense in depth) +# Even though these files are outside webroot, this prevents misconfiguration attacks + + + Require all denied + + + Order deny,allow + Deny from all + + + +# Audio/Video Streaming Support (for Digital Library plugin) + + # Enable Range Requests for audio/video streaming + # Required for seek functionality in audio players + + Header set Accept-Ranges bytes + # Allow CORS for audio/video files (required for cross-origin playback) + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + Header set Access-Control-Allow-Headers "Range, Content-Type" + Header set Access-Control-Expose-Headers "Content-Length, Content-Range, Accept-Ranges" + + + # CORS for digital content in uploads/digital/ directory + + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + + + +# Correct MIME types for audio files + + AddType audio/mpeg .mp3 + AddType audio/mp4 .m4a + AddType audio/ogg .ogg + AddType audio/wav .wav + AddType audio/webm .weba + AddType video/mp4 .mp4 + AddType video/webm .webm + AddType video/ogg .ogv + AddType application/pdf .pdf + AddType application/epub+zip .epub + diff --git a/tests/fixtures/htaccess-history/0.8.6-rc.1.htaccess b/tests/fixtures/htaccess-history/0.8.6-rc.1.htaccess new file mode 100644 index 000000000..e23ad86c1 --- /dev/null +++ b/tests/fixtures/htaccess-history/0.8.6-rc.1.htaccess @@ -0,0 +1,198 @@ +# PHP upload limits (required for Digital Library plugin: eBook up to 100MB, audiobook up to 500MB) +# +# NOTE: these php_value directives ONLY take effect when PHP runs as an Apache +# module (mod_php). On php-fpm / FastCGI / CGI / suExec hosting — the common +# case on cPanel and most shared hosts — the blocks are +# skipped and these limits are IGNORED. There the server's own PHP config wins, +# so raise post_max_size and upload_max_filesize via php.ini / MultiPHP INI +# Editor / the php-fpm pool (php_admin_value[...]). The ~30MB admin-UI upgrade +# ZIP needs post_max_size >= 32M; if it's lower the upload is rejected and the +# app surfaces a clear 413 "file too large" message (see CsrfMiddleware). + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + php_value upload_max_filesize 512M + php_value post_max_size 520M + php_value max_execution_time 300 + php_value max_input_time 300 + + + Options -Indexes +FollowSymLinks + RewriteEngine On + + # Handle Authorization Header + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + + # === Pinakes LiteSpeed privacy bypass === + # Application response headers are too late + # when LSCache can answer before PHP, so any mutation, Authorization header + # or visitor-specific cookie prevents a cache hit at the web-server layer. + # A sole validated locale cookie is safe and is varied independently at + # lookup time and by the application's X-LiteSpeed-Vary response header. + + # Enable LSCache request lookup/store for this vhost. Without it LSWS + # ignores the X-LiteSpeed-Cache-Control response headers and no page is + # ever served from edge cache. + CacheLookup on + + RewriteRule .* - [E=Cache-Vary:pinakes_locale] + + RewriteCond %{REQUEST_METHOD} !^(GET|HEAD)$ [NC] + RewriteRule .* - [E=Cache-Control:no-cache] + + RewriteCond %{HTTP:Authorization} . + RewriteRule .* - [E=Cache-Control:no-cache] + + RewriteCond %{HTTP:Cookie} !^$ + RewriteCond %{HTTP:Cookie} !^\s*pinakes_locale=[A-Za-z]{2}_[A-Za-z]{2}\s*;?\s*$ [NC] + RewriteRule .* - [E=Cache-Control:no-cache] + + # === end Pinakes LiteSpeed privacy bypass === + + # PRIVATE uploaded content — digital-library files, archive documents and + # generic storage — must NOT be served directly. Force them through + # index.php (even when the file exists on disk, so the !-f rule below would + # otherwise skip them) so PrivateModeMiddleware (and any future per-file + # auth) governs access (issue #160). See ProtectedUploadController. + # A relative RewriteRule pattern is matched against the per-directory + # stripped path, so it stays correct under a subdirectory install where + # %{REQUEST_URI} carries the extra prefix (an anchored ^/uploads/ would not + # match there and would leak the private files). Private rule first so its + # [L] wins before the public catch-all below. + # [NC]: on a case-insensitive disk /uploads/Digital/x is the same file as + # /uploads/digital/x, and must not slip past to the public rule below (#480). + RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [NC,QSA,L] + + # Allow direct access to the remaining PUBLIC static files in /uploads/ + # (covers, events, CMS images, branding). + RewriteRule ^uploads/ - [L] + + # Redirect all requests to index.php + RewriteCond %{REQUEST_FILENAME} !-f + RewriteCond %{REQUEST_FILENAME} !-d + RewriteRule ^ index.php [QSA,L] + + +# === Pinakes performance block (auto-managed by Pinakes 0.7.13+) === +# HTTP compression + long-term caching for static assets. +# Asset URLs are version-busted via ?v=X.Y.Z (see HtmlHelper::asset), so +# cache invalidation happens automatically at every release — immutable+1y +# is safe for fingerprinted assets. +# Every directive is gated by so the file stays valid on +# hosts that don't ship mod_deflate / mod_expires / mod_headers / mod_brotli. + + + AddOutputFilterByType DEFLATE text/html text/plain text/css text/xml \ + text/javascript application/javascript application/x-javascript \ + application/json application/ld+json application/xml application/rss+xml \ + application/atom+xml application/xhtml+xml \ + image/svg+xml font/ttf font/otf font/woff + # Don't re-compress already-compressed payloads + SetEnvIfNoCase Request_URI \.(?:gif|jpe?g|png|webp|ico|woff2?|gz|zip|epub|pdf|mp4|webm|mp3|m4a|ogg|wav)$ no-gzip dont-vary + + + + AddOutputFilterByType BROTLI_COMPRESS text/html text/plain text/css \ + text/javascript application/javascript application/json \ + application/ld+json application/xml image/svg+xml font/ttf font/otf + + + + ExpiresActive On + # 1 year for versioned static assets + ExpiresByType text/css "access plus 1 year" + ExpiresByType application/javascript "access plus 1 year" + ExpiresByType text/javascript "access plus 1 year" + ExpiresByType font/woff2 "access plus 1 year" + ExpiresByType font/woff "access plus 1 year" + ExpiresByType font/ttf "access plus 1 year" + ExpiresByType image/x-icon "access plus 1 year" + # 1 month for content images (book covers, uploaded media) + ExpiresByType image/jpeg "access plus 1 month" + ExpiresByType image/png "access plus 1 month" + ExpiresByType image/webp "access plus 1 month" + ExpiresByType image/gif "access plus 1 month" + ExpiresByType image/svg+xml "access plus 1 month" + + + + # Mark versioned assets as immutable — browsers skip revalidation until TTL. + # ?v= querystring busts the cache on every release, so this is safe. + + Header set Cache-Control "public, max-age=31536000, immutable" + + + Header set Cache-Control "public, max-age=2592000" + + +# === end Pinakes performance block === + +# Security Headers + + # Prevent MIME-type sniffing + Header always set X-Content-Type-Options "nosniff" + + # Enable XSS protection + Header always set X-XSS-Protection "1; mode=block" + + # Prevent clickjacking + Header always set X-Frame-Options "SAMEORIGIN" + + # The application emits a per-response nonce-based Content-Security-Policy. + # Do not override it here: a static header would either break inline elements + # or require unsafe-inline for every response. + + # Referrer policy + Header always set Referrer-Policy "strict-origin-when-cross-origin" + + +# Block access to sensitive files (defense in depth) +# Even though these files are outside webroot, this prevents misconfiguration attacks + + + Require all denied + + + Order deny,allow + Deny from all + + + +# Audio/Video Streaming Support (for Digital Library plugin) + + # Enable Range Requests for audio/video streaming + # Required for seek functionality in audio players + + Header set Accept-Ranges bytes + # Allow CORS for audio/video files (required for cross-origin playback) + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + Header set Access-Control-Allow-Headers "Range, Content-Type" + Header set Access-Control-Expose-Headers "Content-Length, Content-Range, Accept-Ranges" + + + # CORS for digital content in uploads/digital/ directory + + Header set Access-Control-Allow-Origin "*" + Header set Access-Control-Allow-Methods "GET, OPTIONS" + + + +# Correct MIME types for audio files + + AddType audio/mpeg .mp3 + AddType audio/mp4 .m4a + AddType audio/ogg .ogg + AddType audio/wav .wav + AddType audio/webm .weba + AddType video/mp4 .mp4 + AddType video/webm .webm + AddType video/ogg .ogv + AddType application/pdf .pdf + AddType application/epub+zip .epub + diff --git a/tests/htaccess-heal-apache-480.test.sh b/tests/htaccess-heal-apache-480.test.sh new file mode 100644 index 000000000..5c0eb825e --- /dev/null +++ b/tests/htaccess-heal-apache-480.test.sh @@ -0,0 +1,106 @@ +#!/bin/bash +# Issue #480 — every public/.htaccess Pinakes ever shipped, on a real Apache, before and after +# the 0.8.6 heal (App\Support\PrivateUploadsRewriteHeal). +# +# For each file in tests/fixtures/htaccess-history/ a throwaway Apache serves a site whose +# uploads/digital, uploads/archives/documents and uploads/copertine hold real files and whose +# index.php is a plain marker, at the domain root and in a subfolder. After the heal: +# - every private upload (digital, archive documents, the /uploads/Digital/ spelling) must +# reach index.php, never the file on disk; +# - a public upload (a cover) must still be served as a static file. +# "before" is printed for information: every format older than 0.8.6 leaks somewhere. +# +# Needs httpd with mod_rewrite (Homebrew: /opt/homebrew/bin/httpd). Uses port 18480. +# Run: bash tests/htaccess-heal-apache-480.test.sh +set -u +REPO="$(cd "$(dirname "$0")/.." && pwd)" +HTTPD="${HTTPD:-$(command -v httpd || true)}" +MAIN_CONF="${HTTPD_MAIN_CONF:-/opt/homebrew/etc/httpd/httpd.conf}" +PORT="${PORT:-18480}" +if [ -z "$HTTPD" ] || [ ! -f "$MAIN_CONF" ]; then + echo "SKIP: no httpd / $MAIN_CONF (set HTTPD and HTTPD_MAIN_CONF)" + exit 0 +fi +W="$(mktemp -d)" +trap '"$HTTPD" -f "$W/httpd.conf" -k stop >/dev/null 2>&1; rm -rf "$W"' EXIT +grep -E "^LoadModule" "$MAIN_CONF" > "$W/modules.conf" +SERVER_ROOT="$("$HTTPD" -V 2>/dev/null | sed -n 's/.*HTTPD_ROOT="\(.*\)"/\1/p')" +MIME="$(dirname "$MAIN_CONF")/mime.types" + +site() { # $1 docroot, $2 .htaccess + mkdir -p "$1/uploads/digital" "$1/uploads/archives/documents" "$1/uploads/copertine" + printf 'SECRET-DIGITAL' > "$1/uploads/digital/secret.pdf" + printf 'SECRET-ARCHIVE' > "$1/uploads/archives/documents/doc.pdf" + printf 'PUBLIC-COVER' > "$1/uploads/copertine/a.jpg" + printf 'ROUTED-TO-INDEX' > "$1/index.php" + cp "$2" "$1/.htaccess" +} +probe() { # $1 path -> LEAK | php | static | httpNNN + local body code + body="$(curl -s "http://127.0.0.1:$PORT$1")" + code="$(curl -s -o /dev/null -w '%{http_code}' "http://127.0.0.1:$PORT$1")" + case "$body" in + SECRET-*) echo LEAK ;; + ROUTED-TO-INDEX) echo php ;; + PUBLIC-COVER) echo static ;; + *) echo "http$code" ;; + esac +} + +PRIVATE=(/uploads/digital/secret.pdf /uploads/Digital/secret.pdf /uploads/archives/documents/doc.pdf + /sub/uploads/digital/secret.pdf /sub/uploads/Digital/secret.pdf /sub/uploads/archives/documents/doc.pdf) +PUBLIC=(/uploads/copertine/a.jpg /sub/uploads/copertine/a.jpg) +fail=0 +checks=0 +for f in "$REPO"/tests/fixtures/htaccess-history/*.htaccess; do + v="$(basename "$f" .htaccess)" + php -r 'require $argv[1] . "/vendor/autoload.php"; echo App\Support\PrivateUploadsRewriteHeal::apply(file_get_contents($argv[2]));' "$REPO" "$f" > "$W/healed-$v" + for state in before after; do + src="$f"; [ "$state" = after ] && src="$W/healed-$v" + D="$W/site-$v-$state"; site "$D" "$src"; site "$D/sub" "$src" + cat > "$W/httpd.conf" < + AllowOverride All + Require all granted + +EOF + "$HTTPD" -f "$W/httpd.conf" -k start || { echo "FAIL $v: httpd did not start"; exit 1; } + sleep 0.6 + line="" + for p in "${PRIVATE[@]}"; do + r="$(probe "$p")"; line="$line $r" + if [ "$state" = after ]; then + checks=$((checks + 1)) + # On a case-sensitive disk /uploads/Digital/ is simply another, missing path. + if [ "$r" != php ] && ! { [[ "$p" == *Digital* ]] && [ "$r" = http404 ]; }; then + echo " FAIL $v after heal: $p -> $r"; fail=$((fail + 1)) + fi + fi + done + for p in "${PUBLIC[@]}"; do + r="$(probe "$p")"; line="$line $r" + if [ "$state" = after ]; then + checks=$((checks + 1)) + [ "$r" = static ] || { echo " FAIL $v after heal: $p -> $r (public file not served)"; fail=$((fail + 1)); } + fi + done + printf ' %-12s %-6s%s\n' "$v" "$state" "$line" + "$HTTPD" -f "$W/httpd.conf" -k stop >/dev/null 2>&1 + sleep 0.6 + done +done +echo +if [ "$fail" -eq 0 ]; then + echo "ALL $checks PASS (every shipped .htaccess, after the heal, on a real Apache)" +else + echo "$fail FAILED of $checks" +fi +exit $((fail > 0)) diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php new file mode 100644 index 000000000..b6e2b9de0 --- /dev/null +++ b/tests/htaccess-heal-history-480.unit.php @@ -0,0 +1,61 @@ + array_values(array_filter(preg_split('/\r?\n/', $s) ?: [], + static fn (string $l): bool => trim($l) !== '' && !str_starts_with(ltrim($l), '#') + && !preg_match('/^\s*Rewrite(Rule|Cond)\b/', $l) && !str_contains($l, 'no-gzip'))); + +$files = glob(__DIR__ . '/fixtures/htaccess-history/*.htaccess') ?: []; +natsort($files); +$check(count($files) >= 13, count($files) . ' shipped versions of public/.htaccess'); +foreach ($files as $file) { + $version = basename($file, '.htaccess'); + $old = (string) file_get_contents($file); + $new = Heal::apply($old); + $hasUploads = str_contains($old, 'uploads'); + if (!$hasUploads) { + $check($new === $old, "{$version}: no /uploads/ rule at all, left as it is"); + continue; + } + $check(Heal::safe($new), "{$version}: private uploads go through index.php with [NC], before the public /uploads/ rule"); + $check(!preg_match('/^\s*RewriteCond %\{REQUEST_URI\} \^\/uploads\//m', $new), "{$version}: no anchored REQUEST_URI rule left (it never matches under a subfolder)"); + $check($rest($new) === $rest($old), "{$version}: every other directive is untouched"); + $check(Heal::apply($new) === $new, "{$version}: a second upgrade changes nothing"); + $check(str_contains($new, 'epub|pdf|mp4|webm|mp3|m4a|ogg|wav)$ no-gzip') || !str_contains($old, 'no-gzip'), "{$version}: PDF and WAV are never compressed"); +} + +// A rule the operator wrote is theirs. +$a = (string) file_get_contents(__DIR__ . '/fixtures/htaccess-history/0.7.13.htaccess'); +$edited = str_replace("RewriteCond %{REQUEST_URI} ^/uploads/ [NC]\n RewriteRule ^ - [L]", "RewriteCond %{REQUEST_URI} ^/uploads/ [NC]\n RewriteCond %{HTTP_HOST} ^cdn\\. [NC]\n RewriteRule ^ - [L]", $a); +$check($edited !== $a && !Heal::safe(Heal::apply($edited)) && preg_replace('/^.*no-gzip.*$/m', '', Heal::apply($edited)) === preg_replace('/^.*no-gzip.*$/m', '', $edited), + 'an /uploads/ rule the operator edited is left alone'); +$b = (string) file_get_contents(__DIR__ . '/fixtures/htaccess-history/0.7.21.htaccess'); +$half = preg_replace('/^\s*RewriteCond %\{REQUEST_URI\} \^\/uploads\/\(digital\|archives\/documents\|storage\)\/ \[NC\]\r?\n\s*RewriteRule \^ index\.php \[QSA,L\]\r?\n/m', '', $b); +$check($half !== $b && preg_replace('/^.*no-gzip.*$/m', '', Heal::apply((string) $half)) === preg_replace('/^.*no-gzip.*$/m', '', (string) $half), + 'a 0.7.18-style file missing its private half is not rewritten into an unsafe order'); + +echo "\n" . ($fail === 0 ? "ALL {$pass} PASS" : "{$fail} FAILED, {$pass} passed") . "\n"; +exit($fail > 0 ? 1 : 0); From e4dc83ec06c83786587b018809f98956171b1992 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 10:45:18 +0200 Subject: [PATCH 02/23] Heal .htaccess files with CRLF line ends, and say so when the heal declines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - REWRITE_C and the no-gzip pattern now end with a lookahead that accepts \r, so a 0.7.71 – 0.8.5 file saved on Windows gets [NC] too; the lines the heal adds keep the file's CRLF line ends. - heal() returns false whenever the file still lets private uploads be served from disk, also when it declined a rule the operator edited. The once-per-version window and the Updater log it with the exact rule to add, so a declined heal is never silent. - Tests: every shipped .htaccess converted to CRLF heals, stays CRLF and is idempotent; heal() returns true on a shipped file and false on an edited one. --- app/Support/PluginManager.php | 2 +- app/Support/PrivateUploadsRewriteHeal.php | 39 +++++++++++++++-------- app/Support/Updater.php | 2 +- tests/digital-access-matrix-480.unit.php | 3 +- tests/htaccess-heal-history-480.unit.php | 28 ++++++++++++++++ 5 files changed, 57 insertions(+), 17 deletions(-) diff --git a/app/Support/PluginManager.php b/app/Support/PluginManager.php index bc57314fd..6f3530339 100644 --- a/app/Support/PluginManager.php +++ b/app/Support/PluginManager.php @@ -2317,7 +2317,7 @@ public function loadActivePlugins(): void // adds [NC] to the private-uploads rewrite. Idempotent; a file the operator // edited by hand is left alone. if (!PrivateUploadsRewriteHeal::heal()) { - SecureLogger::error('[PluginManager] public/.htaccess not updated for #480: add [NC] to the RewriteRule ^uploads/(digital|archives/documents|storage)/ line by hand'); + SecureLogger::error('[PluginManager] public/.htaccess still lets private uploads be served from disk (#480): put "RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [NC,QSA,L]" before the public /uploads/ rule, or restore the shipped file'); } QueryCache::set($maintenanceKey, 1, 900); diff --git a/app/Support/PrivateUploadsRewriteHeal.php b/app/Support/PrivateUploadsRewriteHeal.php index 884ca6d85..a727ce697 100644 --- a/app/Support/PrivateUploadsRewriteHeal.php +++ b/app/Support/PrivateUploadsRewriteHeal.php @@ -31,7 +31,7 @@ final class PrivateUploadsRewriteHeal private const PUBLIC_RULE = 'RewriteRule ^uploads/ - [L]'; /** C: the 0.7.71 – 0.8.5 private rule, without [NC]. */ - private const REWRITE_C = '/^([ \t]*)RewriteRule \^uploads\/\(digital\|archives\/documents\|storage\)\/ index\.php \[QSA,L\][ \t]*$/m'; + private const REWRITE_C = '/^([ \t]*)RewriteRule \^uploads\/\(digital\|archives\/documents\|storage\)\/ index\.php \[QSA,L\][ \t]*(?=\r?$)/m'; /** A: "RewriteCond ^/uploads/ + RewriteRule ^ - [L]" with nothing in between. */ private const PASSTHROUGH_A = '/^([ \t]*)RewriteCond %\{REQUEST_URI\} \^\/uploads\/ \[NC\][ \t]*\r?\n[ \t]*RewriteRule \^ - \[L\][ \t]*(?=\r?\n|$)/m'; @@ -42,22 +42,24 @@ final class PrivateUploadsRewriteHeal /** B, second half: the private rule on REQUEST_URI, with the comment that introduced it. */ private const PRIVATE_B = '/^(?:[ \t]*# PRIVATE uploaded content[^\r\n]*\r?\n(?:[ \t]*#[^\r\n]*\r?\n)*)?[ \t]*RewriteCond %\{REQUEST_URI\} \^\/uploads\/\(digital\|archives\/documents\|storage\)\/ \[NC\][ \t]*\r?\n[ \t]*RewriteRule \^ index\.php \[QSA,L\][ \t]*\r?\n?/m'; - private const NOGZIP_OLD = '/^([ \t]*)SetEnvIfNoCase Request_URI \\\\\.\(\?:gif\|jpe\?g\|png\|webp\|ico\|woff2\?\|gz\|zip\|epub\|mp4\|webm\|mp3\|m4a\|ogg\)\$ no-gzip dont-vary[ \t]*$/m'; + private const NOGZIP_OLD = '/^([ \t]*)SetEnvIfNoCase Request_URI \\\\\.\(\?:gif\|jpe\?g\|png\|webp\|ico\|woff2\?\|gz\|zip\|epub\|mp4\|webm\|mp3\|m4a\|ogg\)\$ no-gzip dont-vary[ \t]*(?=\r?$)/m'; private const NOGZIP_NEW = '$1SetEnvIfNoCase Request_URI \.(?:gif|jpe?g|png|webp|ico|woff2?|gz|zip|epub|pdf|mp4|webm|mp3|m4a|ogg|wav)$ no-gzip dont-vary'; /** The content with the shipped rules brought to 0.8.6; unchanged when nothing applies or the result would not be safe. */ public static function apply(string $content): string { $out = $content; + // A file saved on Windows keeps its CRLF line ends, also in the lines added here. + $eol = str_contains($content, "\r\n") ? "\r\n" : "\n"; if (preg_match(self::REWRITE_C, $out) === 1) { $out = (string) preg_replace(self::REWRITE_C, '$1' . self::PRIVATE_RULE, $out); } elseif (preg_match(self::PASSTHROUGH_B, $out) === 1 && preg_match(self::PRIVATE_B, $out) === 1) { // Both halves of B, exactly as shipped: drop the REQUEST_URI private rule and put // the per-directory pair where the public passthrough was, private rule first. $out = (string) preg_replace(self::PRIVATE_B, '', $out, 1); - $out = (string) preg_replace_callback(self::PASSTHROUGH_B, static fn (array $m): string => self::block($m[1]), $out, 1); + $out = (string) preg_replace_callback(self::PASSTHROUGH_B, static fn (array $m): string => self::block($m[1], $eol), $out, 1); } elseif (preg_match(self::PASSTHROUGH_A, $out) === 1 && !self::hasPrivateRule($out)) { - $out = (string) preg_replace_callback(self::PASSTHROUGH_A, static fn (array $m): string => self::block($m[1]), $out, 1); + $out = (string) preg_replace_callback(self::PASSTHROUGH_A, static fn (array $m): string => self::block($m[1], $eol), $out, 1); } $out = (string) preg_replace(self::NOGZIP_OLD, self::NOGZIP_NEW, $out); @@ -70,8 +72,10 @@ public static function apply(string $content): string } /** - * Heal the file in place, keeping a backup (in storage/backups) and the file's mode. True - * when nothing needed doing or the rewrite succeeded; false when it was needed and failed. + * Heal the file in place, keeping a backup (in storage/backups) and the file's mode. + * True when the file ends up safe (or has no rewrite rules at all); false when it still + * lets private uploads be served from disk: the rewrite failed, or the heal declined a + * rule the operator edited. Callers log false, so a declined heal is never silent. */ public static function heal(?string $path = null, ?string $backupDir = null): bool { @@ -85,7 +89,7 @@ public static function heal(?string $path = null, ?string $backupDir = null): bo } $healed = self::apply($content); if ($healed === $content) { - return true; + return self::acceptable($content); } $mode = @fileperms($path); if (!is_int($mode) || (!is_dir($backupDir) && !@mkdir($backupDir, 0775, true)) @@ -99,7 +103,8 @@ public static function heal(?string $path = null, ?string $backupDir = null): bo try { return @file_put_contents($temporary, $healed, LOCK_EX) !== false && @chmod($temporary, $mode & 07777) - && @rename($temporary, $path); + && @rename($temporary, $path) + && self::acceptable($healed); } finally { if (file_exists($temporary)) { @unlink($temporary); @@ -122,6 +127,12 @@ public static function safe(string $content): bool return true; } + /** Safe, or not an Apache rewrite file at all (nothing this heal could act on). */ + private static function acceptable(string $content): bool + { + return self::safe($content) || preg_match('/^\s*RewriteRule\b/m', $content) !== 1; + } + private static function hasPrivateRule(string $content): bool { return preg_match('/^[ \t]*Rewrite(?:Rule|Cond)[^\r\n]*uploads\/?\(digital\|archives\/documents\|storage\)/m', $content) === 1; @@ -134,13 +145,13 @@ private static function rulesChanged(string $before, string $after): bool } /** The 0.8.6 pair, at the indentation of the lines it replaces. */ - private static function block(string $indent): string + private static function block(string $indent, string $eol = "\n"): string { - return $indent . "# Private uploads (Digital Library files, archive documents, generic storage) go\n" - . $indent . "# through index.php, which checks who may open them; [NC] keeps /uploads/Digital/\n" - . $indent . "# from slipping past on a case-insensitive disk. Added by the 0.8.6 upgrade (#480).\n" - . $indent . self::PRIVATE_RULE . "\n\n" - . $indent . "# Allow direct access to the remaining PUBLIC static files in /uploads/.\n" + return $indent . '# Private uploads (Digital Library files, archive documents, generic storage) go' . $eol + . $indent . '# through index.php, which checks who may open them; [NC] keeps /uploads/Digital/' . $eol + . $indent . '# from slipping past on a case-insensitive disk. Added by the 0.8.6 upgrade (#480).' . $eol + . $indent . self::PRIVATE_RULE . $eol . $eol + . $indent . '# Allow direct access to the remaining PUBLIC static files in /uploads/.' . $eol . $indent . self::PUBLIC_RULE; } } diff --git a/app/Support/Updater.php b/app/Support/Updater.php index 49bd23bf8..32a5d2908 100644 --- a/app/Support/Updater.php +++ b/app/Support/Updater.php @@ -4049,7 +4049,7 @@ public function runMigrations(string $fromVersion, string $toVersion): array // shipped line never arrives: replace it in place while it is still the // exact line an older version shipped. Idempotent. if (!PrivateUploadsRewriteHeal::heal()) { - $this->debugLog('WARNING', 'public/.htaccess not updated for #480: add [NC] to the RewriteRule ^uploads/(digital|archives/documents|storage)/ line by hand'); + $this->debugLog('WARNING', 'public/.htaccess still lets private uploads be served from disk (#480): put "RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [NC,QSA,L]" before the public /uploads/ rule, or restore the shipped file'); } return [ diff --git a/tests/digital-access-matrix-480.unit.php b/tests/digital-access-matrix-480.unit.php index 81d55361d..16b57a184 100644 --- a/tests/digital-access-matrix-480.unit.php +++ b/tests/digital-access-matrix-480.unit.php @@ -628,7 +628,8 @@ function url(string $path = '/'): string $check(\App\Support\PrivateUploadsRewriteHeal::heal($ht, $tmp . '/backups') && count(glob($tmp . '/backups/htaccess.bak-480-*') ?: []) === 1, 'a second upgrade changes nothing'); $own = str_replace('index.php [QSA,L]', 'index.php [QSA,L,E=MINE:1]', $old); file_put_contents($ht, $own); -$check(\App\Support\PrivateUploadsRewriteHeal::heal($ht, $tmp . '/backups') && str_contains((string) file_get_contents($ht), '[QSA,L,E=MINE:1]'), 'a rule the administrator edited is left alone'); +$check(!\App\Support\PrivateUploadsRewriteHeal::heal($ht, $tmp . '/backups') && str_contains((string) file_get_contents($ht), '[QSA,L,E=MINE:1]'), + 'a rule the administrator edited is left alone, and the heal reports it (false: the caller logs it)'); unset($_SESSION); echo "\n" . ($fail === 0 ? "ALL {$pass} PASS" : "{$fail} FAILED, {$pass} passed") . "\n"; diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index b6e2b9de0..14ef34004 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -47,9 +47,37 @@ $check(str_contains($new, 'epub|pdf|mp4|webm|mp3|m4a|ogg|wav)$ no-gzip') || !str_contains($old, 'no-gzip'), "{$version}: PDF and WAV are never compressed"); } +// Saved on Windows: CRLF line ends are recognised, and kept in the lines the heal adds. +foreach ($files as $file) { + $version = basename($file, '.htaccess'); + $crlf = str_replace("\n", "\r\n", str_replace("\r\n", "\n", (string) file_get_contents($file))); + if (!str_contains($crlf, 'uploads')) { + continue; + } + $healed = Heal::apply($crlf); + $check(Heal::safe($healed) && preg_match('/(? Date: Tue, 20 Oct 2026 10:58:57 +0200 Subject: [PATCH 03/23] Count any spelling of the private uploads rule as safe safe() accepted only the exact shipped text, so a custom file that is safe but written differently (flags in another order, extra flags or spaces, the long names nocase and last) was logged as leaking on every maintenance window. It now accepts any RewriteRule for the private folders whose flags include NC (or nocase) and L (or last), still required to come before the public /uploads/ rule. --- app/Support/PrivateUploadsRewriteHeal.php | 17 +++++++++++++++-- tests/htaccess-heal-history-480.unit.php | 9 +++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/app/Support/PrivateUploadsRewriteHeal.php b/app/Support/PrivateUploadsRewriteHeal.php index a727ce697..760a88064 100644 --- a/app/Support/PrivateUploadsRewriteHeal.php +++ b/app/Support/PrivateUploadsRewriteHeal.php @@ -115,8 +115,21 @@ public static function heal(?string $path = null, ?string $backupDir = null): bo /** Whether $content is in the safe 0.8.6 shape: the [NC] private rule before every public /uploads/ rule. */ public static function safe(string $content): bool { - $private = strpos($content, self::PRIVATE_RULE); - if ($private === false) { + // Any spelling of the private rule counts (flags in another order, extra flags, more + // spaces) as long as it ignores case and stops there: a custom but safe file must + // not be reported as leaking on every maintenance window. + $private = null; + if (preg_match_all('/^[ \t]*RewriteRule[ \t]+\^uploads\/\(digital\|archives\/documents\|storage\)\/[ \t]+index\.php[ \t]+\[([^\]\r\n]*)\]/mi', $content, $all, PREG_SET_ORDER | PREG_OFFSET_CAPTURE) > 0) { + foreach ($all as $match) { + $flags = array_map(static fn (string $f): string => strtoupper(trim($f)), explode(',', $match[1][0])); + // NC / nocase and L / last are the same flags under their long names. + if (array_intersect(['NC', 'NOCASE'], $flags) !== [] && array_intersect(['L', 'LAST'], $flags) !== []) { + $private = $match[0][1]; + break; + } + } + } + if ($private === null) { return false; } foreach (['/^[ \t]*RewriteRule \^uploads\/ - \[L\]/m', '/^[ \t]*RewriteCond %\{REQUEST_URI\} \^\/uploads\/ \[NC\]/m'] as $public) { diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index 14ef34004..d3be4da64 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -68,6 +68,15 @@ $check(Heal::heal($path, $dir . '/backups') && Heal::safe((string) file_get_contents($path)), 'heal() on a shipped 0.7.71 file: true, and the file is safe'); $check(Heal::heal($path, $dir . '/backups') && count(glob($dir . '/backups/htaccess.bak-480-*') ?: []) === 1, 'a second heal() is true and makes no new backup'); +// A safe file written by hand in another spelling is safe: no false warning on every upgrade. +$healed = Heal::apply((string) file_get_contents(__DIR__ . '/fixtures/htaccess-history/0.7.71.htaccess')); +$respelled = str_replace('index.php [NC,QSA,L]', 'index.php [L,QSA,nocase]', $healed); +$check($respelled !== $healed && Heal::safe($respelled), 'the long flag names (nocase, last) count as NC and L'); +$reordered = str_replace('index.php [NC,QSA,L]', 'index.php [QSA, L, NC, E=MINE:1]', $healed); +$check($reordered !== $healed && Heal::safe($reordered) && Heal::apply($reordered) === $reordered, 'the private rule with flags in another order and extra spaces is safe, and left alone'); +$withoutNc = str_replace('index.php [NC,QSA,L]', 'index.php [QSA,L]', $healed); +$check(!Heal::safe($withoutNc), 'without NC it is not safe'); + // A rule the operator wrote is theirs. $a = (string) file_get_contents(__DIR__ . '/fixtures/htaccess-history/0.7.13.htaccess'); $edited = str_replace("RewriteCond %{REQUEST_URI} ^/uploads/ [NC]\n RewriteRule ^ - [L]", "RewriteCond %{REQUEST_URI} ^/uploads/ [NC]\n RewriteCond %{HTTP_HOST} ^cdn\\. [NC]\n RewriteRule ^ - [L]", $a); From eb7de34e50e0aeeab1c0bec78f8f94cf4b50d8e5 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 11:11:42 +0200 Subject: [PATCH 04/23] Test the long flag names of the private uploads rule, and a rule without L --- tests/htaccess-heal-history-480.unit.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index d3be4da64..c9e55cfb6 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -70,12 +70,14 @@ // A safe file written by hand in another spelling is safe: no false warning on every upgrade. $healed = Heal::apply((string) file_get_contents(__DIR__ . '/fixtures/htaccess-history/0.7.71.htaccess')); -$respelled = str_replace('index.php [NC,QSA,L]', 'index.php [L,QSA,nocase]', $healed); +$respelled = str_replace('index.php [NC,QSA,L]', 'index.php [last,QSA,nocase]', $healed); $check($respelled !== $healed && Heal::safe($respelled), 'the long flag names (nocase, last) count as NC and L'); $reordered = str_replace('index.php [NC,QSA,L]', 'index.php [QSA, L, NC, E=MINE:1]', $healed); $check($reordered !== $healed && Heal::safe($reordered) && Heal::apply($reordered) === $reordered, 'the private rule with flags in another order and extra spaces is safe, and left alone'); $withoutNc = str_replace('index.php [NC,QSA,L]', 'index.php [QSA,L]', $healed); $check(!Heal::safe($withoutNc), 'without NC it is not safe'); +$withoutL = str_replace('index.php [NC,QSA,L]', 'index.php [NC,QSA]', $healed); +$check(!Heal::safe($withoutL), 'without L (or last) it is not safe: the public rule below would still apply'); // A rule the operator wrote is theirs. $a = (string) file_get_contents(__DIR__ . '/fixtures/htaccess-history/0.7.13.htaccess'); From 80164ac8360e904409df9753c2652b8d737d712f Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 13:32:50 +0200 Subject: [PATCH 05/23] Leave a heal alone when an operator RewriteCond still waits above the block mod_rewrite hands every queued RewriteCond to the next RewriteRule, whatever comments, blank lines or other directives sit in between. A condition the operator wrote above the shipped /uploads/ block therefore moved onto the private rule the heal inserted (shapes A and B), or stayed on the one shape C rewrote, so the private rule only applied when that condition matched and every other request reached the public passthrough. safe() still called the file safe, so neither the updater nor the maintenance window logged it. apply() now leaves a block untouched when a RewriteCond is pending above it, and safe() no longer counts a conditional private rule as protective, so heal() returns false and the callers log the file. The history tests cover each shipped shape with a condition directly above the block and with a comment, a blank line or another directive in between. --- app/Support/PrivateUploadsRewriteHeal.php | 54 ++++++++++++++++-- tests/htaccess-heal-history-480.unit.php | 67 +++++++++++++++++++++++ 2 files changed, 115 insertions(+), 6 deletions(-) diff --git a/app/Support/PrivateUploadsRewriteHeal.php b/app/Support/PrivateUploadsRewriteHeal.php index 760a88064..78a0f3c73 100644 --- a/app/Support/PrivateUploadsRewriteHeal.php +++ b/app/Support/PrivateUploadsRewriteHeal.php @@ -51,14 +51,31 @@ public static function apply(string $content): string $out = $content; // A file saved on Windows keeps its CRLF line ends, also in the lines added here. $eol = str_contains($content, "\r\n") ? "\r\n" : "\n"; + // A RewriteCond the operator put above a matched block binds to the first RewriteRule + // below it, whatever else lies in between: rewriting or splicing the block would move + // that condition onto the inserted private rule (or, for B's removal, onto the next + // rule), so such a block is left as it is and safe() reports the file. if (preg_match(self::REWRITE_C, $out) === 1) { - $out = (string) preg_replace(self::REWRITE_C, '$1' . self::PRIVATE_RULE, $out); - } elseif (preg_match(self::PASSTHROUGH_B, $out) === 1 && preg_match(self::PRIVATE_B, $out) === 1) { + $out = (string) preg_replace_callback( + self::REWRITE_C, + static fn (array $m): string => self::conditionPending($out, $m[0][1]) ? $m[0][0] : $m[1][0] . self::PRIVATE_RULE, + $out, + -1, + $count, + PREG_OFFSET_CAPTURE + ); + } elseif (preg_match(self::PASSTHROUGH_B, $out, $passthrough, PREG_OFFSET_CAPTURE) === 1 + && preg_match(self::PRIVATE_B, $out, $private, PREG_OFFSET_CAPTURE) === 1) { // Both halves of B, exactly as shipped: drop the REQUEST_URI private rule and put // the per-directory pair where the public passthrough was, private rule first. - $out = (string) preg_replace(self::PRIVATE_B, '', $out, 1); - $out = (string) preg_replace_callback(self::PASSTHROUGH_B, static fn (array $m): string => self::block($m[1], $eol), $out, 1); - } elseif (preg_match(self::PASSTHROUGH_A, $out) === 1 && !self::hasPrivateRule($out)) { + // The private half's offset is above its optional comment block, which the + // backward scan skips anyway. + if (!self::conditionPending($out, $passthrough[0][1]) && !self::conditionPending($out, $private[0][1])) { + $out = (string) preg_replace(self::PRIVATE_B, '', $out, 1); + $out = (string) preg_replace_callback(self::PASSTHROUGH_B, static fn (array $m): string => self::block($m[1], $eol), $out, 1); + } + } elseif (preg_match(self::PASSTHROUGH_A, $out, $passthrough, PREG_OFFSET_CAPTURE) === 1 && !self::hasPrivateRule($out) + && !self::conditionPending($out, $passthrough[0][1])) { $out = (string) preg_replace_callback(self::PASSTHROUGH_A, static fn (array $m): string => self::block($m[1], $eol), $out, 1); } $out = (string) preg_replace(self::NOGZIP_OLD, self::NOGZIP_NEW, $out); @@ -123,7 +140,10 @@ public static function safe(string $content): bool foreach ($all as $match) { $flags = array_map(static fn (string $f): string => strtoupper(trim($f)), explode(',', $match[1][0])); // NC / nocase and L / last are the same flags under their long names. - if (array_intersect(['NC', 'NOCASE'], $flags) !== [] && array_intersect(['L', 'LAST'], $flags) !== []) { + // A RewriteCond above it makes it conditional: whatever the condition rejects + // goes on to the public rule, so such a rule protects nothing on its own. + if (array_intersect(['NC', 'NOCASE'], $flags) !== [] && array_intersect(['L', 'LAST'], $flags) !== [] + && !self::conditionPending($content, $match[0][1])) { $private = $match[0][1]; break; } @@ -151,6 +171,28 @@ private static function hasPrivateRule(string $content): bool return preg_match('/^[ \t]*Rewrite(?:Rule|Cond)[^\r\n]*uploads\/?\(digital\|archives\/documents\|storage\)/m', $content) === 1; } + /** + * Whether a RewriteCond is waiting for the rule that starts at $offset (a line start). + * mod_rewrite queues every RewriteCond it reads and hands the queue to the next + * RewriteRule, which is the only directive that takes it: comments, blank lines and any + * other directive in between (SetEnvIf, Header, Options, RewriteBase, RewriteEngine…) + * leave it pending. So the scan walks back past everything that is not a RewriteRule or + * a RewriteCond: the first RewriteCond means a condition is pending, the first + * RewriteRule (such as the shipped E=HTTP_AUTHORIZATION one) means the queue was taken. + * Lines are read in file order, without following / sections. + */ + private static function conditionPending(string $content, int $offset): bool + { + $lines = preg_split('/\r?\n/', substr($content, 0, $offset)) ?: []; + for ($i = count($lines) - 1; $i >= 0; $i--) { + // Directive names are case-insensitive in Apache. + if (preg_match('/^[ \t]*Rewrite(Rule|Cond)\b/i', $lines[$i], $m) === 1) { + return strtolower($m[1]) === 'cond'; + } + } + return false; + } + private static function rulesChanged(string $before, string $after): bool { $rules = static fn (string $s): string => implode("\n", preg_grep('/^\s*Rewrite/', preg_split('/\r?\n/', $s) ?: []) ?: []); diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index c9e55cfb6..a8625d6f1 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -96,5 +96,72 @@ $check($half !== $b && preg_replace('/^.*no-gzip.*$/m', '', Heal::apply((string) $half)) === preg_replace('/^.*no-gzip.*$/m', '', (string) $half), 'a 0.7.18-style file missing its private half is not rewritten into an unsafe order'); +// An operator RewriteCond ABOVE a shipped block binds to the first RewriteRule below it, across +// comments, blank lines and any other directive (only a RewriteRule takes the pending +// conditions). Splicing the private rule there would make it conditional (and on +// every other host the public rule serves private files from disk), so the block stays as it +// is and the file is reported as not safe. +$rules = static fn (string $s): array => array_values(preg_grep('/^\s*Rewrite(Rule|Cond)\b/', preg_split('/\r?\n/', $s) ?: []) ?: []); +$condAbove = static fn (string $s, string $anchor, string $between): string + => (string) preg_replace('/' . preg_quote($anchor, '/') . '/', " RewriteCond %{HTTP_HOST} ^cdn\\\\. [NC]\n" . $between . $anchor, $s, 1); +$healReports = static function (string $content): bool { + $d = sys_get_temp_dir() . '/htaccess-heal-' . bin2hex(random_bytes(4)); + mkdir($d); + file_put_contents($d . '/.htaccess', $content); + $ok = Heal::heal($d . '/.htaccess', $d . '/backups'); + array_map('unlink', glob($d . '/backups/*') ?: []); + @rmdir($d . '/backups'); + @unlink($d . '/.htaccess'); + @rmdir($d); + return $ok; +}; +$fixture = static fn (string $v): string => (string) file_get_contents(__DIR__ . "/fixtures/htaccess-history/{$v}.htaccess"); +$cases = [ + '0.7.13 (A)' => [$fixture('0.7.13'), " RewriteCond %{REQUEST_URI} ^/uploads/ [NC]\n RewriteRule ^ - [L]"], + '0.7.13 (A), above the comment' => [$fixture('0.7.13'), ' # Allow direct access to static files in /uploads/ directory'], + '0.7.21 (B passthrough)' => [$fixture('0.7.21'), " RewriteCond %{REQUEST_URI} ^/uploads/ [NC]\n RewriteCond %{REQUEST_URI} !^"], + '0.7.21 (B passthrough), above the comment' => [$fixture('0.7.21'), ' # Allow direct access to PUBLIC static files in /uploads/'], + '0.7.21 (B private pair)' => [$fixture('0.7.21'), ' RewriteCond %{REQUEST_URI} ^/uploads/(digital'], + '0.7.21 (B private pair), above the comment' => [$fixture('0.7.21'), ' # PRIVATE uploaded content'], + '0.7.71 (C)' => [$fixture('0.7.71'), ' RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [QSA,L]'], +]; +$betweens = [ + 'directly above' => '', + 'with a comment between' => " # operator note\n", + 'with a blank line between' => "\n", + 'with a SetEnvIf between' => " SetEnvIf Request_URI x foo=1\n", + 'with Options and RewriteBase between' => " Options -Indexes\n RewriteBase /\n", +]; +foreach ($cases as $label => [$base, $anchor]) { + foreach ($betweens as $how => $between) { + $edited = $condAbove($base, $anchor, $between); + $healed = Heal::apply($edited); + $check($edited !== $base && $rules($healed) === $rules($edited) && !Heal::safe($healed) && !$healReports($edited), + "{$label}: a RewriteCond {$how} is not moved onto the private rule, and heal() reports the file"); + } +} +$healed86 = Heal::apply($fixture('0.7.71')); +foreach ($betweens as $how => $between) { + $edited = $condAbove($healed86, ' RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [NC,QSA,L]', $between); + $check($edited !== $healed86 && !Heal::safe($edited) && !$healReports($edited) && $rules(Heal::apply($edited)) === $rules($edited), + "a healed 0.8.6 file with a RewriteCond {$how} the [NC] private rule is not safe"); +} +$check(Heal::safe($healed86) && Heal::safe($fixture('0.8.6-rc.1')), 'the E=HTTP_AUTHORIZATION RewriteRule or a comment above the private rule is no pending condition'); +// The other side of the same rule: a directive with no RewriteCond waiting behind it, or an +// operator condition already taken by its own RewriteRule, leaves nothing pending, so the +// shipped block still heals. +$directive = " SetEnvIf Request_URI x foo=1\n"; +$bound = " RewriteCond %{HTTP_HOST} ^cdn\\. [NC]\n RewriteRule ^ - [E=CDN:1]\n SetEnvIf Request_URI x foo=1\n"; +foreach (['0.7.13 (A)' => ['0.7.13', ' # Allow direct access to static files in /uploads/ directory'], + '0.7.21 (B)' => ['0.7.21', ' # Allow direct access to PUBLIC static files in /uploads/'], + '0.7.71 (C)' => ['0.7.71', ' RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [QSA,L]']] as $label => [$v, $anchor]) { + foreach (['a directive alone' => $directive, 'a condition bound to its own rule' => $bound] as $how => $insert) { + $edited = (string) preg_replace('/' . preg_quote($anchor, '/') . '/', $insert . $anchor, $fixture($v), 1); + $healed = Heal::apply($edited); + $check($edited !== $fixture($v) && $healed !== $edited && Heal::safe($healed) && $healReports($edited), + "{$label}: {$how} above the block leaves nothing pending, and the file heals"); + } +} + echo "\n" . ($fail === 0 ? "ALL {$pass} PASS" : "{$fail} FAILED, {$pass} passed") . "\n"; exit($fail > 0 ? 1 : 0); From e0ef520ca57c59b77a2d6c56319915dfcdb2272c Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 13:56:31 +0200 Subject: [PATCH 06/23] Run the patterns above the private rule instead of matching two spellings safe() only looked for the public /uploads/ rule as "RewriteRule ^uploads/ - [L]" or as the old "RewriteCond %{REQUEST_URI} ^/uploads/ [NC]" head. The same passthrough written as "^uploads/ - [L,NC]", "^uploads - [L]", "^/?uploads/.*$ - [END]", in lower case or with more spaces, above the private rule served private files from disk while safe() called the file safe, so neither the updater nor the maintenance window logged it. safe() now takes every passthrough ("-") above the private rule that stops rewriting (L, last, END) or skips rules (S=n), and runs its pattern against the private paths, also in another case: without NC a pattern still reaches the file on a case-insensitive disk. Every spelling counts the same, a "^ - [L]" behind any RewriteCond included, while a passthrough without L, one limited to a public subfolder or a negated one stays safe. The shipped files only have E= rules there, which never stop, so none of them changes verdict. --- app/Support/PrivateUploadsRewriteHeal.php | 39 ++++++++++++++++++++--- tests/htaccess-heal-history-480.unit.php | 20 ++++++++++++ 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/app/Support/PrivateUploadsRewriteHeal.php b/app/Support/PrivateUploadsRewriteHeal.php index 78a0f3c73..ffacff7cb 100644 --- a/app/Support/PrivateUploadsRewriteHeal.php +++ b/app/Support/PrivateUploadsRewriteHeal.php @@ -149,15 +149,44 @@ public static function safe(string $content): bool } } } - if ($private === null) { + return $private !== null && !self::publicRuleBefore($content, $private); + } + + /** + * Whether a rule before $offset can hand a private file to Apache as it is on disk: + * a passthrough ("-") that stops rewriting (L, last, END) or skips rules (S, skip), and + * whose pattern matches a private path. The pattern is run, not read, so every spelling + * counts the same: "^uploads/ - [L,NC]", "^uploads - [L]", "^/?uploads/.*$", other case or + * spacing, a rule on "^" behind a RewriteCond (its condition may well hold). A path in + * another case is tried too: without NC it still reaches the file on a case-insensitive + * disk. A passthrough without L goes on to the private rule below it, so it leaks nothing. + */ + private static function publicRuleBefore(string $content, int $offset): bool + { + $rule = '/^[ \t]*RewriteRule[ \t]+("[^"\r\n]*"|[^ \t\r\n]+)[ \t]+-(?:[ \t]+\[([^\]\r\n]*)\])?[ \t]*(?=\r?$)/mi'; + if (preg_match_all($rule, substr($content, 0, $offset), $rules, PREG_SET_ORDER) === 0) { return false; } - foreach (['/^[ \t]*RewriteRule \^uploads\/ - \[L\]/m', '/^[ \t]*RewriteCond %\{REQUEST_URI\} \^\/uploads\/ \[NC\]/m'] as $public) { - if (preg_match($public, $content, $m, PREG_OFFSET_CAPTURE) === 1 && $m[0][1] < $private) { - return false; + foreach ($rules as $match) { + $flags = array_map(static fn (string $f): string => strtoupper(trim($f)), explode(',', $match[2] ?? '')); + $stops = array_intersect(['L', 'LAST', 'END'], $flags) !== [] + || preg_grep('/^(?:S|SKIP)=[1-9]/', $flags) !== []; + if (!$stops) { + continue; + } + $pattern = trim($match[1], '"'); + $negated = str_starts_with($pattern, '!'); + $regex = '#' . str_replace('#', '\#', $negated ? substr($pattern, 1) : $pattern) . '#' + . (array_intersect(['NC', 'NOCASE'], $flags) !== [] ? 'i' : ''); + foreach (['uploads/digital/a.pdf', 'uploads/Digital/a.pdf', 'uploads/archives/documents/a.pdf', 'uploads/storage/a'] as $path) { + $hit = @preg_match($regex, $path); + // A pattern PCRE cannot read is one we cannot vouch for. + if ($hit === false || ($hit === 1) !== $negated) { + return true; + } } } - return true; + return false; } /** Safe, or not an Apache rewrite file at all (nothing this heal could act on). */ diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index a8625d6f1..7485a055b 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -79,6 +79,26 @@ $withoutL = str_replace('index.php [NC,QSA,L]', 'index.php [NC,QSA]', $healed); $check(!Heal::safe($withoutL), 'without L (or last) it is not safe: the public rule below would still apply'); +// A public /uploads/ rule ABOVE the private one serves private files from disk in any spelling. +$privateLine = ' RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [NC,QSA,L]'; +$publicAbove = static fn (string $public): string => str_replace($privateLine, " {$public}\n{$privateLine}", $healed); +$check(str_contains($healed, $privateLine) && Heal::safe($healed), 'the healed 0.7.71 file has the private line the next checks put rules above'); +foreach (['RewriteRule ^uploads/ - [L,NC]', 'RewriteRule ^uploads - [L]', 'RewriteRule ^uploads/ - [L]', 'rewriterule ^uploads/ - [l]', + 'RewriteRule ^/?uploads/ - [last]', 'RewriteRule ^uploads/.*$ - [END]', 'RewriteRule uploads/ - [QSA,L]', 'RewriteRule ^Uploads/ - [NC,L]', + 'RewriteRule "^uploads/" - [L]', 'RewriteRule ^uploads/(?!digital/) - [L]', 'RewriteRule ^uploads/ - [S=1]', + "RewriteCond %{REQUEST_URI} ^/uploads [NC]\n RewriteRule ^ - [L]", "RewriteCond %{HTTP_HOST} ^cdn\\. [NC]\n RewriteRule .* - [L]"] as $public) { + $check(!Heal::safe($publicAbove($public)), 'a public rule above the private one is not safe: ' . str_replace("\n ", ' + ', $public)); +} +// The second-to-last above: without NC, /uploads/Digital/ skips the lookahead on a case-insensitive disk. +foreach (['RewriteRule ^uploads/ - [NC]' => 'a passthrough without L goes on to the private rule below', + 'RewriteRule ^uploads/thumbs/ - [L]' => 'a passthrough on a public subfolder only', + 'RewriteRule ^uploadsarchive/ - [L]' => 'a folder whose name only starts with "uploads"', + 'RewriteRule !^uploads/ - [L]' => 'a negated pattern that skips /uploads/', + 'RewriteRule ^uploads/(?!digital/|archives/documents/|storage/) - [NC,L]' => 'a pattern that excludes every private folder, with NC', + 'RewriteRule .* - [E=MINE:1]' => 'an E= rule like the shipped ones'] as $public => $why) { + $check(Heal::safe($publicAbove($public)), "{$why} is safe: {$public}"); +} + // A rule the operator wrote is theirs. $a = (string) file_get_contents(__DIR__ . '/fixtures/htaccess-history/0.7.13.htaccess'); $edited = str_replace("RewriteCond %{REQUEST_URI} ^/uploads/ [NC]\n RewriteRule ^ - [L]", "RewriteCond %{REQUEST_URI} ^/uploads/ [NC]\n RewriteCond %{HTTP_HOST} ^cdn\\. [NC]\n RewriteRule ^ - [L]", $a); From ad47c7dc7c5c1c3f50acf6ac0b9f8ae7562e0bfc Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 14:23:39 +0200 Subject: [PATCH 07/23] Report a stopping passthrough unless its pattern provably misses the private folders Running each pattern against four sample paths only proved those paths: "RewriteRule \.epub$ - [L]" above the private rule matches none of them, so safe() called the file safe while Apache served every private EPUB from disk. The same went for a start in another case without NC ("^UPLOADS/Digital/"), a negated "!^uploads/" (which lets /Uploads/Digital/ through on a case-insensitive disk), and a literal start deeper inside a private folder. A passthrough that stops rewriting or skips rules is now reported unless the pattern is anchored with ^ and its literal start, compared without case, leads away from every private folder ("^uploads/thumbs/", "^assets/"), with no "|" anywhere (in "^assets/|uploads/" the second branch has no anchor), and none of the sample private paths matches it either. Anything else, a lookahead included, cannot be proved without reasoning about the regex, so it is logged rather than trusted. The shipped files only have E= rules above the private rule, which never stop, so none of them changes verdict. --- app/Support/PrivateUploadsRewriteHeal.php | 61 +++++++++++++++-------- tests/htaccess-heal-history-480.unit.php | 19 +++++-- 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/app/Support/PrivateUploadsRewriteHeal.php b/app/Support/PrivateUploadsRewriteHeal.php index ffacff7cb..38530e18f 100644 --- a/app/Support/PrivateUploadsRewriteHeal.php +++ b/app/Support/PrivateUploadsRewriteHeal.php @@ -153,13 +153,16 @@ public static function safe(string $content): bool } /** - * Whether a rule before $offset can hand a private file to Apache as it is on disk: - * a passthrough ("-") that stops rewriting (L, last, END) or skips rules (S, skip), and - * whose pattern matches a private path. The pattern is run, not read, so every spelling - * counts the same: "^uploads/ - [L,NC]", "^uploads - [L]", "^/?uploads/.*$", other case or - * spacing, a rule on "^" behind a RewriteCond (its condition may well hold). A path in - * another case is tried too: without NC it still reaches the file on a case-insensitive - * disk. A passthrough without L goes on to the private rule below it, so it leaks nothing. + * Whether a rule before $offset may hand a private file to Apache as it is on disk: a + * passthrough ("-") that stops rewriting (L, last, END) or skips rules (S, skip), unless + * its pattern provably never matches under a private folder. Trying sample paths proves + * nothing ("\.epub$ - [L]" misses a .pdf sample and serves every private EPUB), so the + * only proof taken is a pattern anchored with ^ whose literal start leads away from every + * private folder, compared without case (a disk that ignores case serves + * /uploads/Digital/ too): "^uploads/thumbs/" or "^assets/" pass, while "^uploads/", + * "^/?uploads", "\.epub$", "^ - [L]" behind any RewriteCond, a lookahead and a negated + * pattern are reported. A passthrough without L goes on to the private rule below it, + * so it leaks nothing. */ private static function publicRuleBefore(string $content, int $offset): bool { @@ -171,24 +174,42 @@ private static function publicRuleBefore(string $content, int $offset): bool $flags = array_map(static fn (string $f): string => strtoupper(trim($f)), explode(',', $match[2] ?? '')); $stops = array_intersect(['L', 'LAST', 'END'], $flags) !== [] || preg_grep('/^(?:S|SKIP)=[1-9]/', $flags) !== []; - if (!$stops) { - continue; - } - $pattern = trim($match[1], '"'); - $negated = str_starts_with($pattern, '!'); - $regex = '#' . str_replace('#', '\#', $negated ? substr($pattern, 1) : $pattern) . '#' - . (array_intersect(['NC', 'NOCASE'], $flags) !== [] ? 'i' : ''); - foreach (['uploads/digital/a.pdf', 'uploads/Digital/a.pdf', 'uploads/archives/documents/a.pdf', 'uploads/storage/a'] as $path) { - $hit = @preg_match($regex, $path); - // A pattern PCRE cannot read is one we cannot vouch for. - if ($hit === false || ($hit === 1) !== $negated) { - return true; - } + if ($stops && !self::missesPrivateFolders(trim($match[1], '"'))) { + return true; } } return false; } + /** Whether $pattern can only match paths that start away from every private folder. */ + private static function missesPrivateFolders(string $pattern): bool + { + // "^" and then plain characters, up to the first one with a regex meaning; a + // character followed by a quantifier may be absent, so it is not part of the start. + // Any "|" may open an unanchored branch ("^assets/|uploads/"), so it is no proof. + if (str_contains($pattern, '|') || preg_match('/^\^([A-Za-z0-9_\/-]*)(.?)/', $pattern, $m) !== 1) { + return false; + } + // Belt and braces: whatever the start says, no private path may match. + foreach (['uploads/digital/a.pdf', 'uploads/Digital/a.pdf', 'uploads/digital/a.epub', 'uploads/digital/a.mp3', 'uploads/digital/a.m4a', + 'uploads/archives/documents/a.pdf', 'uploads/archives/documents/a.jpg', 'uploads/storage/a', 'uploads/storage/a.png'] as $path) { + if (@preg_match('#' . str_replace('#', '\#', $pattern) . '#i', $path) !== 0) { + return false; + } + } + $start = strtolower(in_array($m[2], ['?', '*', '{'], true) ? substr($m[1], 0, -1) : $m[1]); + if ($start === '') { + return false; + } + foreach (['uploads/digital/', 'uploads/archives/documents/', 'uploads/storage/'] as $folder) { + // Some path starts with both exactly when one start is a prefix of the other. + if (str_starts_with($folder, $start) || str_starts_with($start, $folder)) { + return false; + } + } + return true; + } + /** Safe, or not an Apache rewrite file at all (nothing this heal could act on). */ private static function acceptable(string $content): bool { diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index 7485a055b..b037acd17 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -86,15 +86,26 @@ foreach (['RewriteRule ^uploads/ - [L,NC]', 'RewriteRule ^uploads - [L]', 'RewriteRule ^uploads/ - [L]', 'rewriterule ^uploads/ - [l]', 'RewriteRule ^/?uploads/ - [last]', 'RewriteRule ^uploads/.*$ - [END]', 'RewriteRule uploads/ - [QSA,L]', 'RewriteRule ^Uploads/ - [NC,L]', 'RewriteRule "^uploads/" - [L]', 'RewriteRule ^uploads/(?!digital/) - [L]', 'RewriteRule ^uploads/ - [S=1]', - "RewriteCond %{REQUEST_URI} ^/uploads [NC]\n RewriteRule ^ - [L]", "RewriteCond %{HTTP_HOST} ^cdn\\. [NC]\n RewriteRule .* - [L]"] as $public) { + "RewriteCond %{REQUEST_URI} ^/uploads [NC]\n RewriteRule ^ - [L]", "RewriteCond %{HTTP_HOST} ^cdn\\. [NC]\n RewriteRule .* - [L]", + // Patterns that never name /uploads/ still reach every private file with that ending or shape. + 'RewriteRule \.epub$ - [L]', 'RewriteRule \.(pdf|mp3)$ - [L,NC]', 'RewriteRule ^(.*)$ - [L]', 'RewriteRule ^.*/a\.pdf$ - [END]', + 'RewriteRule \.(epub|mp3|m4a|ogg|wav)$ - [L]', 'RewriteRule \.(jpe?g|png|webp)$ - [NC,L]', 'RewriteRule ^uploads/digital/.+\.epub$ - [L]', + // A literal start that leads into a private folder, or one cut short by a quantifier. + 'RewriteRule ^uploads/digital - [L]', 'RewriteRule ^uploads/storage/public/ - [L]', 'RewriteRule ^uploadss?/ - [L]', 'RewriteRule ^UPLOADS/Digital/ - [L]', + // A safe-looking start with an unanchored branch after "|", at any depth. + 'RewriteRule ^assets/|uploads/ - [L]', 'RewriteRule ^thumbs/|\.pdf$ - [NC,L]', 'RewriteRule ^(assets|uploads)/ - [L]', + // Without NC, /uploads/Digital/ skips a lowercase exclusion on a case-insensitive disk. + 'RewriteRule !^uploads/ - [L]', + // Unprovable without reasoning about the regex, so reported: a lookahead is no proof. + 'RewriteRule ^uploads/(?!digital/|archives/documents/|storage/) - [NC,L]'] as $public) { $check(!Heal::safe($publicAbove($public)), 'a public rule above the private one is not safe: ' . str_replace("\n ", ' + ', $public)); } -// The second-to-last above: without NC, /uploads/Digital/ skips the lookahead on a case-insensitive disk. foreach (['RewriteRule ^uploads/ - [NC]' => 'a passthrough without L goes on to the private rule below', + 'RewriteRule \.epub$ - [NC]' => 'a passthrough on an ending without L', 'RewriteRule ^uploads/thumbs/ - [L]' => 'a passthrough on a public subfolder only', + 'RewriteRule ^uploads/digitalx/ - [L]' => 'a folder that only starts like a private one', 'RewriteRule ^uploadsarchive/ - [L]' => 'a folder whose name only starts with "uploads"', - 'RewriteRule !^uploads/ - [L]' => 'a negated pattern that skips /uploads/', - 'RewriteRule ^uploads/(?!digital/|archives/documents/|storage/) - [NC,L]' => 'a pattern that excludes every private folder, with NC', + 'RewriteRule ^assets/.*\.css$ - [L]' => 'a passthrough outside /uploads/', 'RewriteRule .* - [E=MINE:1]' => 'an E= rule like the shipped ones'] as $public => $why) { $check(Heal::safe($publicAbove($public)), "{$why} is safe: {$public}"); } From 1bfc4f8d5e9170de9cef77fb14139c2cbd660889 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 15:11:00 +0200 Subject: [PATCH 08/23] Keep the no-gzip update when the rewrite heal declines apply() put PDF and WAV into the no-gzip list in the same pass as the rewrite heal, and when the rewritten rules turned out not to be safe (an operator passthrough such as "^uploads/ - [L,NC]" above the private rule) it returned the original file, dropping the no-gzip line with them. Such an install kept compressing PDF and WAV, so a Range request got the whole compressed body instead of the bytes on disk. Before this branch the no-gzip update was applied on every heal. apply() now decides on the rewrite rules first and runs the no-gzip update on whichever content it keeps. A declined heal still leaves every rewrite rule as it was and heal() still returns false, so the caller logs the file; it only writes the no-gzip line, with a backup, and the next run writes nothing. --- app/Support/PrivateUploadsRewriteHeal.php | 22 ++++++++------- tests/htaccess-heal-history-480.unit.php | 34 +++++++++++++++++++++++ 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/app/Support/PrivateUploadsRewriteHeal.php b/app/Support/PrivateUploadsRewriteHeal.php index 38530e18f..0b1b0e03f 100644 --- a/app/Support/PrivateUploadsRewriteHeal.php +++ b/app/Support/PrivateUploadsRewriteHeal.php @@ -22,8 +22,9 @@ * * Each shape is replaced only where its lines are still exactly what was shipped: a rule * the operator wrote is theirs, and is left alone. The result must have the private rule - * with [NC] before any public /uploads/ rule; otherwise the file is not touched at all. - * PDF and WAV also join the no-gzip list, so a Range request gets the bytes on disk. + * with [NC] before any public /uploads/ rule; otherwise the rewrite rules are not touched + * at all. PDF and WAV join the no-gzip list either way, so a Range request gets the bytes + * on disk. */ final class PrivateUploadsRewriteHeal { @@ -45,7 +46,7 @@ final class PrivateUploadsRewriteHeal private const NOGZIP_OLD = '/^([ \t]*)SetEnvIfNoCase Request_URI \\\\\.\(\?:gif\|jpe\?g\|png\|webp\|ico\|woff2\?\|gz\|zip\|epub\|mp4\|webm\|mp3\|m4a\|ogg\)\$ no-gzip dont-vary[ \t]*(?=\r?$)/m'; private const NOGZIP_NEW = '$1SetEnvIfNoCase Request_URI \.(?:gif|jpe?g|png|webp|ico|woff2?|gz|zip|epub|pdf|mp4|webm|mp3|m4a|ogg|wav)$ no-gzip dont-vary'; - /** The content with the shipped rules brought to 0.8.6; unchanged when nothing applies or the result would not be safe. */ + /** The content with the shipped rules brought to 0.8.6; the rewrite rules stay as they are when nothing applies or the result would not be safe. */ public static function apply(string $content): string { $out = $content; @@ -78,14 +79,15 @@ public static function apply(string $content): string && !self::conditionPending($out, $passthrough[0][1])) { $out = (string) preg_replace_callback(self::PASSTHROUGH_A, static fn (array $m): string => self::block($m[1], $eol), $out, 1); } - $out = (string) preg_replace(self::NOGZIP_OLD, self::NOGZIP_NEW, $out); - - if ($out === $content) { - return $content; - } // Whatever changed in the rewrite rules must leave the private rule, with [NC], ahead - // of every public /uploads/ rule; anything else is a file we do not understand. - return self::rulesChanged($content, $out) && !self::safe($out) ? $content : $out; + // of every public /uploads/ rule; anything else is a file we do not understand, and + // its rewrite rules stay as they are. + if (self::rulesChanged($content, $out) && !self::safe($out)) { + $out = $content; + } + // The no-gzip list does not depend on the rewrite rules, so a declined heal still + // gets PDF and WAV in it. + return (string) preg_replace(self::NOGZIP_OLD, self::NOGZIP_NEW, $out); } /** diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index b037acd17..bf8a367a0 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -171,6 +171,40 @@ "{$label}: a RewriteCond {$how} is not moved onto the private rule, and heal() reports the file"); } } +// A heal whose rewritten rules would still not be safe (an operator passthrough above the +// private rule) keeps every rewrite rule as it was, but the no-gzip list does not depend on +// them: PDF and WAV still join it, and the second pass changes nothing. +$publicAbove = [ + '0.7.71 (C) with "^uploads/ - [L,NC]" above the private rule' => ['0.7.71', ' RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [QSA,L]', " RewriteRule ^uploads/ - [L,NC]\n"], + '0.7.71 (C) with "\.pdf$ - [L]" above the private rule' => ['0.7.71', ' RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [QSA,L]', " RewriteRule \\.pdf$ - [L]\n"], + '0.7.13 (A) with "^uploads/ - [L]" above the block' => ['0.7.13', ' # Allow direct access to static files in /uploads/ directory', " RewriteRule ^uploads/ - [L]\n"], +]; +foreach ($publicAbove as $label => [$v, $anchor, $insert]) { + $edited = (string) preg_replace('/' . preg_quote($anchor, '/') . '/', $insert . $anchor, $fixture($v), 1); + $healed = Heal::apply($edited); + $check($edited !== $fixture($v) && $rules($healed) === $rules($edited) && !Heal::safe($healed) + && str_contains($healed, 'epub|pdf|mp4|webm|mp3|m4a|ogg|wav)$ no-gzip') && Heal::apply($healed) === $healed, + "{$label}: the rewrite rules stay, PDF and WAV still join the no-gzip list"); +} +// On disk: that heal writes only the no-gzip line, keeps a backup, and still returns false so +// the caller logs the file; the next run finds nothing to do and writes nothing. +$d = sys_get_temp_dir() . '/htaccess-heal-' . bin2hex(random_bytes(4)); +mkdir($d); +[$v, $anchor, $insert] = reset($publicAbove); +$declined = (string) preg_replace('/' . preg_quote($anchor, '/') . '/', $insert . $anchor, $fixture($v), 1); +file_put_contents($d . '/.htaccess', $declined); +$first = Heal::heal($d . '/.htaccess', $d . '/backups'); +$written = (string) file_get_contents($d . '/.htaccess'); +$backups = count(glob($d . '/backups/*') ?: []); +$second = Heal::heal($d . '/.htaccess', $d . '/backups'); +$check(!$first && !$second && $rules($written) === $rules($declined) + && str_contains($written, 'epub|pdf|mp4|webm|mp3|m4a|ogg|wav)$ no-gzip') && !str_contains($declined, '|pdf|') + && $backups === 1 && count(glob($d . '/backups/*') ?: []) === 1 && file_get_contents($d . '/.htaccess') === $written, + 'a declined heal writes only the no-gzip line, still returns false, and the second run writes nothing'); +array_map('unlink', glob($d . '/backups/*') ?: []); +@rmdir($d . '/backups'); +@unlink($d . '/.htaccess'); +@rmdir($d); $healed86 = Heal::apply($fixture('0.7.71')); foreach ($betweens as $how => $between) { $edited = $condAbove($healed86, ' RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [NC,QSA,L]', $between); From 572ad9710e8f965cca689fb9bfd70f33601ffd73 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 15:24:21 +0200 Subject: [PATCH 09/23] Count a rule chained into the private one as a condition A RewriteRule with C (chain) runs the next rule only when it matched itself, so "RewriteRule ^uploads/thumbs/ - [C]" right above the private rule skips it for every private path, whatever its substitution. conditionPending() stopped at the first RewriteRule and called the queue taken, so safe() counted the chained private rule as protective and the heal could rewrite a shape C rule into the chain. The backward scan now treats a RewriteRule carrying C or chain like a pending RewriteCond: the private rule below it is not counted as safe, and apply() leaves that block as it is so heal() reports the file. A cookie flag such as CO= is no chain and stays safe. --- app/Support/PrivateUploadsRewriteHeal.php | 14 +++++++++++--- tests/htaccess-heal-history-480.unit.php | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/app/Support/PrivateUploadsRewriteHeal.php b/app/Support/PrivateUploadsRewriteHeal.php index 0b1b0e03f..80606fbc9 100644 --- a/app/Support/PrivateUploadsRewriteHeal.php +++ b/app/Support/PrivateUploadsRewriteHeal.php @@ -142,8 +142,9 @@ public static function safe(string $content): bool foreach ($all as $match) { $flags = array_map(static fn (string $f): string => strtoupper(trim($f)), explode(',', $match[1][0])); // NC / nocase and L / last are the same flags under their long names. - // A RewriteCond above it makes it conditional: whatever the condition rejects - // goes on to the public rule, so such a rule protects nothing on its own. + // A RewriteCond above it, or a rule chained into it with C, makes it conditional: + // whatever the condition rejects goes on to the public rule, so such a rule + // protects nothing on its own. if (array_intersect(['NC', 'NOCASE'], $flags) !== [] && array_intersect(['L', 'LAST'], $flags) !== [] && !self::conditionPending($content, $match[0][1])) { $private = $match[0][1]; @@ -231,6 +232,9 @@ private static function hasPrivateRule(string $content): bool * leave it pending. So the scan walks back past everything that is not a RewriteRule or * a RewriteCond: the first RewriteCond means a condition is pending, the first * RewriteRule (such as the shipped E=HTTP_AUTHORIZATION one) means the queue was taken. + * That RewriteRule still makes the next one conditional when it carries C (chain): a + * chained rule runs only when the one before it matched, so "^uploads/thumbs/ - [C]" + * above the private rule skips it for every private path, whatever the substitution. * Lines are read in file order, without following / sections. */ private static function conditionPending(string $content, int $offset): bool @@ -239,7 +243,11 @@ private static function conditionPending(string $content, int $offset): bool for ($i = count($lines) - 1; $i >= 0; $i--) { // Directive names are case-insensitive in Apache. if (preg_match('/^[ \t]*Rewrite(Rule|Cond)\b/i', $lines[$i], $m) === 1) { - return strtolower($m[1]) === 'cond'; + if (strtolower($m[1]) === 'cond') { + return true; + } + $flags = preg_match('/\[([^\]\r\n]*)\][ \t]*$/', $lines[$i], $f) === 1 ? explode(',', $f[1]) : []; + return array_intersect(['C', 'CHAIN'], array_map(static fn (string $x): string => strtoupper(trim($x)), $flags)) !== []; } } return false; diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index bf8a367a0..ae46ee653 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -228,5 +228,20 @@ } } + +// A rule chained into the private one ([C]) makes it run only when that rule matched, so +// whatever the chained pattern leaves out skips it: any substitution, any pattern. +$base71 = Heal::apply($fixture('0.7.71')); +$chainAbove = static fn (string $rule): string => str_replace($privateLine, " {$rule}\n{$privateLine}", $base71); +$check(substr_count($base71, $privateLine) === 1 && Heal::safe($base71), 'the healed 0.7.71 file has one private line to chain into, and is safe'); +foreach (['RewriteRule ^uploads/thumbs/ - [C]', 'RewriteRule ^x$ index.php [chain]', 'RewriteRule ^assets/ - [QSA, c]'] as $chained) { + $check(!Heal::safe($chainAbove($chained)), "a rule chained into the private one is not safe: {$chained}"); +} +$c71 = $fixture('0.7.71'); +$chainedC = str_replace(' RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [QSA,L]', " RewriteRule ^assets/ - [C]\n RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [QSA,L]", $c71); +$check($chainedC !== $c71 && $rules(Heal::apply($chainedC)) === $rules($chainedC) && !Heal::safe(Heal::apply($chainedC)), + '0.7.71 (C) with a chained rule right above: the private rule is not rewritten into the chain, and the file is reported'); +$check(Heal::safe($chainAbove('RewriteRule ^x$ - [CO=seen:1:example.org]')), 'a cookie flag (CO=) is no chain: safe'); + echo "\n" . ($fail === 0 ? "ALL {$pass} PASS" : "{$fail} FAILED, {$pass} passed") . "\n"; exit($fail > 0 ? 1 : 0); From f96e474e28c0b598e0cfe53e8ed553ad8681241c Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 15:27:15 +0200 Subject: [PATCH 10/23] Test Digital Library files uploaded on an older release across the upgrade tests/digital-upgrade-480.spec.js runs twice against the same sandbox install. Before the upgrade the admin uploads two PDFs and an audiobook through the old book form and an anonymous visitor downloads them, Range requests included. After it, the same files are in storage/digital byte for byte at the same addresses, still open to everyone while the default is left as it was, and once the admin limits one to signed-in readers and one to staff the visitor is sent to sign in or refused, another case of the folder name or a dot segment does not get around the check, the reader opens only theirs and the admin both. digital-access-480.spec.js takes the install root from E2E_INSTALL_ROOT, so the same suite can run against the upgraded sandbox. --- tests/digital-access-480.spec.js | 8 +- tests/digital-upgrade-480.spec.js | 252 ++++++++++++++++++++++++++++++ 2 files changed, 258 insertions(+), 2 deletions(-) create mode 100644 tests/digital-upgrade-480.spec.js diff --git a/tests/digital-access-480.spec.js b/tests/digital-access-480.spec.js index fcb6566d4..b89502e57 100644 --- a/tests/digital-access-480.spec.js +++ b/tests/digital-access-480.spec.js @@ -10,7 +10,8 @@ const fs = require('fs'); const path = require('path'); const BASE = process.env.E2E_BASE_URL || 'http://localhost:8081'; -const ROOT = path.join(__dirname, '..'); +// The install under test: this checkout by default, the upgraded sandbox under scripts/reinstall-test.sh. +const ROOT = process.env.E2E_INSTALL_ROOT || path.join(__dirname, '..'); const STORAGE = path.join(ROOT, 'storage/digital'); const LEGACY = path.join(ROOT, 'public/uploads/digital'); test.skip(!process.env.E2E_ADMIN_EMAIL || !process.env.E2E_DB_NAME, 'E2E credentials required'); @@ -293,7 +294,10 @@ test.describe.serial('Digital Library access control (#480)', () => { expect(mode(path.join(STORAGE, legacyName))).toBe('644'); // No deny rule in the old folder: Apache would apply it before the rewrite to Pinakes. const legacyRule = path.join(LEGACY, '.htaccess'); - expect(fs.existsSync(legacyRule) ? fs.readFileSync(legacyRule, 'utf8') : '').not.toContain('Require all denied'); + // The pre-0.8.6 plugin leaves one there that denies only scripts, inside : + // that one stays, a deny outside it would refuse the PDFs too. + const legacyText = fs.existsSync(legacyRule) ? fs.readFileSync(legacyRule, 'utf8') : ''; + expect(legacyText.replace(//gi, '')).not.toContain('Require all denied'); // The settings page counts nothing left to move. const pluginId = db("SELECT id FROM plugins WHERE name='digital-library'"); await page.goto(`${BASE}/admin/plugins/${pluginId}/settings`); diff --git a/tests/digital-upgrade-480.spec.js b/tests/digital-upgrade-480.spec.js new file mode 100644 index 000000000..fc9879a84 --- /dev/null +++ b/tests/digital-upgrade-480.spec.js @@ -0,0 +1,252 @@ +// @ts-check +// Issue #480 — Digital Library files uploaded on an older release, then the upgrade. +// +// Run twice by scripts/reinstall-test.sh (Test B), against the same sandbox install: +// E2E_UPGRADE_PHASE=before on the old release (files go to public/uploads/digital and +// anyone can download them), the admin uploads through the +// book form like a person would; +// E2E_UPGRADE_PHASE=after on the upgraded install: the same files, byte for byte, now +// in storage/digital, still served at the same addresses, and +// the per-file visibility of 0.8.6 enforced on them. +// The state between the two runs (book id, addresses, checksums) is a JSON file. +// The database is only used to prepare the book and a reader, never to upload or check. +const { test, expect } = require('@playwright/test'); +const { execFileSync } = require('child_process'); +const crypto = require('crypto'); +const fs = require('fs'); +const path = require('path'); + +const BASE = process.env.E2E_BASE_URL || 'http://localhost:8081'; +const ROOT = process.env.E2E_INSTALL_ROOT || path.join(__dirname, '..'); +const PHASE = process.env.E2E_UPGRADE_PHASE || ''; +const STATE = process.env.E2E_UPGRADE_STATE || path.join(ROOT, 'storage/tmp/digital-upgrade-480.json'); +const STORAGE = path.join(ROOT, 'storage/digital'); +const LEGACY = path.join(ROOT, 'public/uploads/digital'); +test.skip(!process.env.E2E_ADMIN_EMAIL || !process.env.E2E_DB_NAME || !PHASE, 'E2E credentials and E2E_UPGRADE_PHASE required'); + +function db(sql) { + const args = ['-u', process.env.E2E_DB_USER, process.env.E2E_DB_NAME, '-N', '-B', '-e', sql]; + if (process.env.E2E_DB_SOCKET) args.unshift('-S', process.env.E2E_DB_SOCKET); + return execFileSync('mysql', args, { encoding: 'utf8', env: { ...process.env, MYSQL_PWD: process.env.E2E_DB_PASS } }).trim(); +} +const sha = buffer => crypto.createHash('sha256').update(buffer).digest('hex'); + +async function loginWith(page, email, password) { + await page.locator('input[name=email]').fill(email); + await page.locator('input[name=password]').fill(password); + await page.locator('button[type=submit]').click(); + await page.waitForURL(u => !/accedi|login/.test(u.pathname)); +} +async function loginAdmin(page) { + await page.goto(`${BASE}/admin/dashboard`); + if (await page.locator('input[name=email]').isVisible()) { + await loginWith(page, process.env.E2E_ADMIN_EMAIL, process.env.E2E_ADMIN_PASS); + } +} +async function saveBook(page) { + await page.locator('#bookForm button[type=submit]').click(); + const confirmation = page.locator('.swal2-confirm'); + if (await confirmation.isVisible({ timeout: 3000 }).catch(() => false)) await confirmation.click(); + await page.waitForURL(u => !/\/admin\/books\/(edit|create)/.test(u.pathname)); +} +/** The Uppy result alert after each upload: dismissed like a person would. */ +async function dismissAlert(page) { + const ok = page.locator('.swal2-confirm'); + if (await ok.isVisible({ timeout: 3000 }).catch(() => false)) await ok.click(); +} + +const pdf = fs.readFileSync(path.join(__dirname, 'fixtures/archive-test.pdf')); +// The audio uploader of the old release takes MP3, M4A or OGG: an ID3 header and eight +// silent MPEG-1 Layer III frames, which the server's content check reads as audio/mpeg. +const mp3 = Buffer.concat([Buffer.from('ID3\x03\x00\x00\x00\x00\x00\x00', 'latin1'), + ...Array.from({ length: 8 }, () => Buffer.concat([Buffer.from([0xff, 0xfb, 0x90, 0x64]), Buffer.alloc(413)]))]); + +test.describe.serial(`Digital Library across the upgrade (#480) — ${PHASE}`, () => { + test.skip(PHASE !== 'before', 'before-upgrade phase only'); + + /** @type {{bookId:number, files:Record}} */ + const state = { bookId: 0, files: {} }; + + test('the digital-library plugin is active on the old release', async ({ page }) => { + await loginAdmin(page); + await page.goto(`${BASE}/admin/plugins`); + const id = db("SELECT id FROM plugins WHERE name='digital-library'"); + expect(id, 'digital-library registered').not.toBe(''); + if (db("SELECT is_active FROM plugins WHERE name='digital-library'") !== '1') { + // The card of this plugin, found by its title like a person would. + const card = page.locator('[data-plugin-id]').filter({ has: page.getByRole('heading', { name: /Digital Library/ }) }).first(); + await expect(card).toBeVisible(); + await card.getByRole('button', { name: 'Attiva plugin' }).click(); + const activated = page.waitForResponse(r => r.url().includes(`/admin/plugins/${id}/activate`)); + await page.locator('.swal2-confirm:visible').click(); + const response = await activated; + const body = await response.json().catch(() => ({})); + expect(response.status(), JSON.stringify(body)).toBe(200); + expect(body.success, JSON.stringify(body)).toBe(true); + await page.locator('.swal2-confirm:visible').click(); + } + await expect.poll(() => db("SELECT is_active FROM plugins WHERE name='digital-library'")).toBe('1'); + }); + + test('the admin uploads two PDFs and an audiobook through the book form', async ({ page }) => { + const title = `Upgrade480-${Date.now()}`; + db(`INSERT INTO libri (titolo, created_at, updated_at) VALUES ('${title}', NOW(), NOW())`); + state.bookId = Number(db(`SELECT id FROM libri WHERE titolo='${title}' AND deleted_at IS NULL`)); + await loginAdmin(page); + await page.goto(`${BASE}/admin/books/edit/${state.bookId}`); + + await page.locator('#upload-ebook-btn').click(); + await page.locator('#ebook-uploader input[type=file]').setInputFiles([ + { name: 'upgrade-480-public.pdf', mimeType: 'application/pdf', buffer: pdf }, + { name: 'upgrade-480-readers.pdf', mimeType: 'application/pdf', buffer: pdf }, + ]); + const rows = page.locator('[data-attachment-row]'); + await expect(rows).toHaveCount(2, { timeout: 20000 }); + await dismissAlert(page); + + await page.locator('#upload-audio-btn').click(); + await page.locator('#audio-uploader input[type=file]').setInputFiles( + { name: 'upgrade-480-audio.mp3', mimeType: 'audio/mpeg', buffer: mp3 }); + await expect(rows).toHaveCount(3, { timeout: 20000 }); + await dismissAlert(page); + + const labels = ['Public edition', 'Readers edition', 'Audiobook']; + for (const [i, label] of labels.entries()) { + await rows.nth(i).locator('input[name$="[label]"]').fill(label); + const url = await rows.nth(i).locator('input[name$="[url]"]').inputValue(); + const buffer = i === 2 ? mp3 : pdf; + state.files[label] = { url, sha: sha(buffer), size: buffer.length }; + } + await saveBook(page); + + // The old release keeps them in the public uploads folder, as uploaded. + for (const { url, sha: digest } of Object.values(state.files)) { + expect(url).toMatch(/^\/uploads\/digital\/[A-Za-z0-9._-]+\.(pdf|mp3)$/); + const file = path.join(LEGACY, path.basename(url)); + expect(fs.existsSync(file), `${url} in public/uploads/digital`).toBe(true); + expect(sha(fs.readFileSync(file))).toBe(digest); + } + const saved = JSON.parse(db(`SELECT digital_attachments FROM libri WHERE id=${state.bookId}`) || '[]'); + expect(saved.map(a => a.label)).toEqual(labels); + fs.mkdirSync(path.dirname(STATE), { recursive: true }); + fs.writeFileSync(STATE, JSON.stringify(state, null, 2)); + }); + + test('on the old release an anonymous visitor sees and downloads every file', async ({ page, request }) => { + await page.goto(`${BASE}/libro/${state.bookId}`); + const list = page.locator('.digital-attachments-list'); + for (const label of Object.keys(state.files)) await expect(list).toContainText(label); + for (const { url, sha: digest, size } of Object.values(state.files)) { + const response = await request.get(`${BASE}${url}`, { maxRedirects: 0 }); + expect(response.status(), url).toBe(200); + expect(sha(await response.body())).toBe(digest); + const range = await request.get(`${BASE}${url}`, { maxRedirects: 0, headers: { Range: 'bytes=0-99' } }); + expect(range.status(), `${url} with Range`).toBe(206); + expect((await range.body()).length).toBe(Math.min(100, size)); + } + }); +}); + +test.describe.serial('Digital Library across the upgrade (#480) — after', () => { + test.skip(PHASE !== 'after', 'after-upgrade phase only'); + + const RUN = Date.now(); + const password = `Pk480u-${RUN}-x`; + const reader = { email: `upgrade-reader-${RUN}@example.test`, id: 0 }; + /** @type {{bookId:number, files:Record}} */ + let state; + + test.beforeAll(() => { + state = JSON.parse(fs.readFileSync(STATE, 'utf8')); + const hash = execFileSync('php', ['-r', 'echo password_hash($argv[1], PASSWORD_DEFAULT);', password], { encoding: 'utf8' }); + db(`INSERT INTO utenti (codice_tessera, nome, cognome, email, password, stato, tipo_utente, email_verificata, privacy_accettata) VALUES ('U480${RUN % 1000000}', 'Upgrade', 'Reader', '${reader.email}', '${hash}', 'attivo', 'standard', 1, 1)`); + reader.id = Number(db(`SELECT id FROM utenti WHERE email='${reader.email}'`)); + }); + + test('the uploaded files moved to storage/digital, byte for byte, at the same addresses', async ({ page }) => { + // The first page under the new code finishes whatever the upgrade left to move. + await page.goto(`${BASE}/libro/${state.bookId}`); + for (const [label, { url, sha: digest }] of Object.entries(state.files)) { + const name = path.basename(url); + expect(fs.existsSync(path.join(STORAGE, name)), `${label} in storage/digital`).toBe(true); + expect(sha(fs.readFileSync(path.join(STORAGE, name)))).toBe(digest); + expect(fs.existsSync(path.join(LEGACY, name)), `${label} no longer in public/uploads/digital`).toBe(false); + } + const saved = JSON.parse(db(`SELECT digital_attachments FROM libri WHERE id=${state.bookId}`) || '[]'); + expect(saved.map(a => a.url)).toEqual(Object.values(state.files).map(f => f.url)); + }); + + test('with the default left as it was, anonymous visitors still open every file', async ({ page, request }) => { + await page.goto(`${BASE}/libro/${state.bookId}`); + const list = page.locator('.digital-attachments-list'); + for (const label of Object.keys(state.files)) await expect(list).toContainText(label); + await expect(list.locator('.digital-attachment--locked')).toHaveCount(0); + await expect(list.locator('audio source')).toHaveCount(1); + for (const { url, sha: digest, size } of Object.values(state.files)) { + const response = await request.get(`${BASE}${url}`, { maxRedirects: 0 }); + expect(response.status(), url).toBe(200); + expect(sha(await response.body())).toBe(digest); + const range = await request.get(`${BASE}${url}`, { maxRedirects: 0, headers: { Range: 'bytes=0-99' } }); + expect(range.status(), `${url} with Range`).toBe(206); + expect(range.headers()['content-encoding'] || '').not.toContain('gzip'); + expect((await range.body()).length).toBe(Math.min(100, size)); + } + }); + + test('the admin limits one file to signed-in readers and one to staff on the upgraded form', async ({ page }) => { + await loginAdmin(page); + await page.goto(`${BASE}/admin/books/edit/${state.bookId}`); + const rows = page.locator('[data-attachment-row]'); + await expect(rows).toHaveCount(3); + await rows.nth(1).locator('select[name$="[visibility]"]').selectOption('users'); + await rows.nth(2).locator('select[name$="[visibility]"]').selectOption('staff'); + await saveBook(page); + const saved = JSON.parse(db(`SELECT digital_attachments FROM libri WHERE id=${state.bookId}`)); + expect(saved.map(a => a.visibility)).toEqual(['inherit', 'users', 'staff']); + }); + + test('an anonymous visitor is sent to sign in for the readers file and refused the staff one', async ({ page, request }) => { + const get = (url, headers = {}) => request.get(`${BASE}${url}`, { maxRedirects: 0, headers }); + const { 'Public edition': open, 'Readers edition': readers, Audiobook: staff } = state.files; + expect((await get(open.url)).status()).toBe(200); + const toLogin = await get(readers.url); + expect(toLogin.status()).toBe(302); + expect(toLogin.headers()['location']).toContain('return_url='); + expect((await get(staff.url)).status()).toBe(403); + // Another case of the folder name is no way around the check, nor is a dot segment. + for (const variant of [readers.url.replace('/digital/', '/Digital/'), readers.url.replace('/uploads/', '/uploads/./')]) { + const response = await request.get(`${BASE}${variant}`, { maxRedirects: 0 }); + const served = response.status() === 200 && sha(await response.body()) === readers.sha; + expect(served, `${variant} must not serve the file`).toBe(false); + } + await page.goto(`${BASE}/libro/${state.bookId}`); + const list = page.locator('.digital-attachments-list'); + await expect(list).not.toContainText('Audiobook'); + await expect(list.locator('.digital-attachment--locked', { hasText: 'Readers edition' })).toBeVisible(); + await expect(page.locator(`a[href*="${path.basename(readers.url)}"]`)).toHaveCount(0); + }); + + test('a signed-in reader opens the readers file but not the staff one; the admin opens both', async ({ page, browser }) => { + const { 'Readers edition': readers, Audiobook: staff } = state.files; + await page.goto(`${BASE}/libro/${state.bookId}`); + const bookPath = new URL(page.url()).pathname; + await page.locator('.digital-attachment--locked', { hasText: 'Readers edition' }).locator('.digital-attachment__login').click(); + await loginWith(page, reader.email, password); + await page.waitForURL(u => u.pathname === bookPath); + const asReader = await page.request.get(`${BASE}${readers.url}`, { maxRedirects: 0 }); + expect(asReader.status()).toBe(200); + expect(sha(await asReader.body())).toBe(readers.sha); + expect((await page.request.get(`${BASE}${staff.url}`, { maxRedirects: 0 })).status()).toBe(403); + + const admin = await browser.newPage(); + await loginAdmin(admin); + const asStaff = await admin.request.get(`${BASE}${staff.url}`, { maxRedirects: 0 }); + expect(asStaff.status()).toBe(200); + expect(sha(await asStaff.body())).toBe(staff.sha); + await admin.close(); + }); + + test.afterAll(() => { + if (reader.id) db(`DELETE FROM utenti WHERE id=${reader.id}`); + }); +}); From 62a5144a794b252b25d39d16bfdc76659ae2e41b Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 15:57:03 +0200 Subject: [PATCH 11/23] Cover the one-file eBook and audiobook fields of releases before 0.7.89 in the upgrade test Before the attachment list of 0.7.89 the book form had one eBook and one audiobook field, and the edit page lived at /admin/libri/modifica/N up to 0.7.17. The upgrade test now uploads through whichever form the old release has, gives each file a role (open, readers, staff) instead of a label, finds the rows of the upgraded form by address, and also tries /UPLOADS/ on the protected files. --- tests/digital-upgrade-480.spec.js | 194 +++++++++++++++++------------- 1 file changed, 111 insertions(+), 83 deletions(-) diff --git a/tests/digital-upgrade-480.spec.js b/tests/digital-upgrade-480.spec.js index fc9879a84..6e18d26f9 100644 --- a/tests/digital-upgrade-480.spec.js +++ b/tests/digital-upgrade-480.spec.js @@ -8,6 +8,9 @@ // E2E_UPGRADE_PHASE=after on the upgraded install: the same files, byte for byte, now // in storage/digital, still served at the same addresses, and // the per-file visibility of 0.8.6 enforced on them. +// The old book form comes in two shapes: since 0.7.89 a list of attachments (two PDFs and +// an audiobook here), before it one eBook and one audiobook field. Each file has a role: +// "open" stays public, "readers" is limited to signed-in readers, "staff" to staff. // The state between the two runs (book id, addresses, checksums) is a JSON file. // The database is only used to prepare the book and a reader, never to upload or check. const { test, expect } = require('@playwright/test'); @@ -43,29 +46,54 @@ async function loginAdmin(page) { await loginWith(page, process.env.E2E_ADMIN_EMAIL, process.env.E2E_ADMIN_PASS); } } +/** The book edit page: /admin/books/edit/N since 0.7.18-ish, /admin/libri/modifica/N before. */ +async function openEditForm(page, bookId) { + for (const route of [`/admin/books/edit/${bookId}`, `/admin/libri/modifica/${bookId}`]) { + const response = await page.goto(`${BASE}${route}`); + if (response && response.status() === 200 && await page.locator('#bookForm').count()) return; + } + throw new Error(`no edit form for book ${bookId}`); +} async function saveBook(page) { await page.locator('#bookForm button[type=submit]').click(); const confirmation = page.locator('.swal2-confirm'); if (await confirmation.isVisible({ timeout: 3000 }).catch(() => false)) await confirmation.click(); - await page.waitForURL(u => !/\/admin\/books\/(edit|create)/.test(u.pathname)); + await page.waitForURL(u => !/\/admin\/(books\/(edit|create)|libri\/(modifica|crea))/.test(u.pathname)); } /** The Uppy result alert after each upload: dismissed like a person would. */ async function dismissAlert(page) { const ok = page.locator('.swal2-confirm'); if (await ok.isVisible({ timeout: 3000 }).catch(() => false)) await ok.click(); } +/** Upload through one of the plugin's uploaders (ebook or audio), as the admin would. */ +async function upload(page, kind, files) { + await page.locator(`#upload-${kind}-btn`).click(); + await page.locator(`#${kind}-uploader input[type=file]`).setInputFiles(files); +} const pdf = fs.readFileSync(path.join(__dirname, 'fixtures/archive-test.pdf')); -// The audio uploader of the old release takes MP3, M4A or OGG: an ID3 header and eight +// The audio uploader of the old releases takes MP3, M4A or OGG: an ID3 header and eight // silent MPEG-1 Layer III frames, which the server's content check reads as audio/mpeg. const mp3 = Buffer.concat([Buffer.from('ID3\x03\x00\x00\x00\x00\x00\x00', 'latin1'), ...Array.from({ length: 8 }, () => Buffer.concat([Buffer.from([0xff, 0xfb, 0x90, 0x64]), Buffer.alloc(413)]))]); +const entry = (url, buffer) => ({ url, sha: sha(buffer), size: buffer.length }); + +/** The file at url answers 200 with these bytes, and a Range request 206 with its start. */ +async function expectServed(request, file) { + const response = await request.get(`${BASE}${file.url}`, { maxRedirects: 0 }); + expect(response.status(), file.url).toBe(200); + expect(sha(await response.body()), `${file.url} intact`).toBe(file.sha); + const range = await request.get(`${BASE}${file.url}`, { maxRedirects: 0, headers: { Range: 'bytes=0-99' } }); + expect(range.status(), `${file.url} with Range`).toBe(206); + expect(range.headers()['content-encoding'] || '', `${file.url} not compressed`).not.toContain('gzip'); + expect((await range.body()).length).toBe(Math.min(100, file.size)); +} test.describe.serial(`Digital Library across the upgrade (#480) — ${PHASE}`, () => { test.skip(PHASE !== 'before', 'before-upgrade phase only'); - /** @type {{bookId:number, files:Record}} */ - const state = { bookId: 0, files: {} }; + /** @type {{bookId:number, mode:string, files:Record}} */ + const state = { bookId: 0, mode: '', files: {} }; test('the digital-library plugin is active on the old release', async ({ page }) => { await loginAdmin(page); @@ -73,11 +101,9 @@ test.describe.serial(`Digital Library across the upgrade (#480) — ${PHASE}`, ( const id = db("SELECT id FROM plugins WHERE name='digital-library'"); expect(id, 'digital-library registered').not.toBe(''); if (db("SELECT is_active FROM plugins WHERE name='digital-library'") !== '1') { - // The card of this plugin, found by its title like a person would. - const card = page.locator('[data-plugin-id]').filter({ has: page.getByRole('heading', { name: /Digital Library/ }) }).first(); - await expect(card).toBeVisible(); - await card.getByRole('button', { name: 'Attiva plugin' }).click(); + // The plugin's own "Attiva" / "Attiva plugin" button, in every release of the page. const activated = page.waitForResponse(r => r.url().includes(`/admin/plugins/${id}/activate`)); + await page.locator(`button[onclick="activatePlugin(${id})"]`).click(); await page.locator('.swal2-confirm:visible').click(); const response = await activated; const body = await response.json().catch(() => ({})); @@ -88,62 +114,63 @@ test.describe.serial(`Digital Library across the upgrade (#480) — ${PHASE}`, ( await expect.poll(() => db("SELECT is_active FROM plugins WHERE name='digital-library'")).toBe('1'); }); - test('the admin uploads two PDFs and an audiobook through the book form', async ({ page }) => { + test('the admin uploads the digital files through the old book form', async ({ page }) => { const title = `Upgrade480-${Date.now()}`; db(`INSERT INTO libri (titolo, created_at, updated_at) VALUES ('${title}', NOW(), NOW())`); state.bookId = Number(db(`SELECT id FROM libri WHERE titolo='${title}' AND deleted_at IS NULL`)); await loginAdmin(page); - await page.goto(`${BASE}/admin/books/edit/${state.bookId}`); + await openEditForm(page, state.bookId); + // The plugin's uploader is drawn by its book.form.digital_fields hook: if it is missing, + // say what the database holds for the plugin instead of waiting on a button. + await expect(page.locator('#upload-ebook-btn'), db( + "SELECT CONCAT_WS(' ', p.id, p.is_active, p.version, p.path, ph.hook_name, ph.callback_method, ph.is_active) FROM plugins p LEFT JOIN plugin_hooks ph ON ph.plugin_id = p.id WHERE p.name = 'digital-library'" + )).toBeVisible({ timeout: 10000 }); + state.mode = await page.locator('#add-digital-attachment').count() ? 'attachments' : 'fields'; - await page.locator('#upload-ebook-btn').click(); - await page.locator('#ebook-uploader input[type=file]').setInputFiles([ - { name: 'upgrade-480-public.pdf', mimeType: 'application/pdf', buffer: pdf }, - { name: 'upgrade-480-readers.pdf', mimeType: 'application/pdf', buffer: pdf }, - ]); - const rows = page.locator('[data-attachment-row]'); - await expect(rows).toHaveCount(2, { timeout: 20000 }); - await dismissAlert(page); - - await page.locator('#upload-audio-btn').click(); - await page.locator('#audio-uploader input[type=file]').setInputFiles( - { name: 'upgrade-480-audio.mp3', mimeType: 'audio/mpeg', buffer: mp3 }); - await expect(rows).toHaveCount(3, { timeout: 20000 }); - await dismissAlert(page); - - const labels = ['Public edition', 'Readers edition', 'Audiobook']; - for (const [i, label] of labels.entries()) { - await rows.nth(i).locator('input[name$="[label]"]').fill(label); - const url = await rows.nth(i).locator('input[name$="[url]"]').inputValue(); - const buffer = i === 2 ? mp3 : pdf; - state.files[label] = { url, sha: sha(buffer), size: buffer.length }; + if (state.mode === 'attachments') { + await upload(page, 'ebook', [ + { name: 'upgrade-480-open.pdf', mimeType: 'application/pdf', buffer: pdf }, + { name: 'upgrade-480-readers.pdf', mimeType: 'application/pdf', buffer: pdf }, + ]); + const rows = page.locator('[data-attachment-row]'); + await expect(rows).toHaveCount(2, { timeout: 20000 }); + await dismissAlert(page); + await upload(page, 'audio', { name: 'upgrade-480-staff.mp3', mimeType: 'audio/mpeg', buffer: mp3 }); + await expect(rows).toHaveCount(3, { timeout: 20000 }); + await dismissAlert(page); + for (const [i, [role, label, buffer]] of [['open', 'Public edition', pdf], ['readers', 'Readers edition', pdf], ['staff', 'Audiobook', mp3]].entries()) { + await rows.nth(i).locator('input[name$="[label]"]').fill(label); + state.files[role] = entry(await rows.nth(i).locator('input[name$="[url]"]').inputValue(), buffer); + } + } else { + // One eBook and one audiobook field: the eBook goes to readers, the audiobook to staff. + await upload(page, 'ebook', { name: 'upgrade-480-readers.pdf', mimeType: 'application/pdf', buffer: pdf }); + await expect(page.locator('#file_url').first()).toHaveValue(/\/uploads\/digital\/.+\.pdf$/, { timeout: 20000 }); + await dismissAlert(page); + await upload(page, 'audio', { name: 'upgrade-480-staff.mp3', mimeType: 'audio/mpeg', buffer: mp3 }); + await expect(page.locator('#audio_url').first()).toHaveValue(/\/uploads\/digital\/.+\.mp3$/, { timeout: 20000 }); + await dismissAlert(page); + state.files.readers = entry(await page.locator('#file_url').first().inputValue(), pdf); + state.files.staff = entry(await page.locator('#audio_url').first().inputValue(), mp3); } await saveBook(page); - // The old release keeps them in the public uploads folder, as uploaded. + // The old release keeps them in the public uploads folder, as uploaded, and lists them on the book. for (const { url, sha: digest } of Object.values(state.files)) { expect(url).toMatch(/^\/uploads\/digital\/[A-Za-z0-9._-]+\.(pdf|mp3)$/); const file = path.join(LEGACY, path.basename(url)); expect(fs.existsSync(file), `${url} in public/uploads/digital`).toBe(true); expect(sha(fs.readFileSync(file))).toBe(digest); } - const saved = JSON.parse(db(`SELECT digital_attachments FROM libri WHERE id=${state.bookId}`) || '[]'); - expect(saved.map(a => a.label)).toEqual(labels); + const listed = db(`SELECT CONCAT_WS('|', file_url, audio_url) FROM libri WHERE id=${state.bookId}`) + + (state.mode === 'attachments' ? db(`SELECT digital_attachments FROM libri WHERE id=${state.bookId}`) : ''); + for (const { url } of Object.values(state.files)) expect(listed, `${url} saved on the book`).toContain(url); fs.mkdirSync(path.dirname(STATE), { recursive: true }); fs.writeFileSync(STATE, JSON.stringify(state, null, 2)); }); - test('on the old release an anonymous visitor sees and downloads every file', async ({ page, request }) => { - await page.goto(`${BASE}/libro/${state.bookId}`); - const list = page.locator('.digital-attachments-list'); - for (const label of Object.keys(state.files)) await expect(list).toContainText(label); - for (const { url, sha: digest, size } of Object.values(state.files)) { - const response = await request.get(`${BASE}${url}`, { maxRedirects: 0 }); - expect(response.status(), url).toBe(200); - expect(sha(await response.body())).toBe(digest); - const range = await request.get(`${BASE}${url}`, { maxRedirects: 0, headers: { Range: 'bytes=0-99' } }); - expect(range.status(), `${url} with Range`).toBe(206); - expect((await range.body()).length).toBe(Math.min(100, size)); - } + test('on the old release an anonymous visitor downloads every file, Range requests included', async ({ request }) => { + for (const file of Object.values(state.files)) await expectServed(request, file); }); }); @@ -153,8 +180,9 @@ test.describe.serial('Digital Library across the upgrade (#480) — after', () = const RUN = Date.now(); const password = `Pk480u-${RUN}-x`; const reader = { email: `upgrade-reader-${RUN}@example.test`, id: 0 }; - /** @type {{bookId:number, files:Record}} */ + /** @type {{bookId:number, mode:string, files:Record}} */ let state; + const name = role => path.basename(state.files[role].url); test.beforeAll(() => { state = JSON.parse(fs.readFileSync(STATE, 'utf8')); @@ -166,71 +194,71 @@ test.describe.serial('Digital Library across the upgrade (#480) — after', () = test('the uploaded files moved to storage/digital, byte for byte, at the same addresses', async ({ page }) => { // The first page under the new code finishes whatever the upgrade left to move. await page.goto(`${BASE}/libro/${state.bookId}`); - for (const [label, { url, sha: digest }] of Object.entries(state.files)) { - const name = path.basename(url); - expect(fs.existsSync(path.join(STORAGE, name)), `${label} in storage/digital`).toBe(true); - expect(sha(fs.readFileSync(path.join(STORAGE, name)))).toBe(digest); - expect(fs.existsSync(path.join(LEGACY, name)), `${label} no longer in public/uploads/digital`).toBe(false); + for (const [role, { url, sha: digest }] of Object.entries(state.files)) { + expect(fs.existsSync(path.join(STORAGE, name(role))), `${role} file in storage/digital`).toBe(true); + expect(sha(fs.readFileSync(path.join(STORAGE, name(role))))).toBe(digest); + expect(fs.existsSync(path.join(LEGACY, name(role))), `${role} file no longer in public/uploads/digital`).toBe(false); + const listed = db(`SELECT CONCAT_WS('|', file_url, audio_url, digital_attachments) FROM libri WHERE id=${state.bookId}`); + expect(listed, `${url} still the address saved on the book`).toContain(url); } - const saved = JSON.parse(db(`SELECT digital_attachments FROM libri WHERE id=${state.bookId}`) || '[]'); - expect(saved.map(a => a.url)).toEqual(Object.values(state.files).map(f => f.url)); }); test('with the default left as it was, anonymous visitors still open every file', async ({ page, request }) => { await page.goto(`${BASE}/libro/${state.bookId}`); const list = page.locator('.digital-attachments-list'); - for (const label of Object.keys(state.files)) await expect(list).toContainText(label); await expect(list.locator('.digital-attachment--locked')).toHaveCount(0); - await expect(list.locator('audio source')).toHaveCount(1); - for (const { url, sha: digest, size } of Object.values(state.files)) { - const response = await request.get(`${BASE}${url}`, { maxRedirects: 0 }); - expect(response.status(), url).toBe(200); - expect(sha(await response.body())).toBe(digest); - const range = await request.get(`${BASE}${url}`, { maxRedirects: 0, headers: { Range: 'bytes=0-99' } }); - expect(range.status(), `${url} with Range`).toBe(206); - expect(range.headers()['content-encoding'] || '').not.toContain('gzip'); - expect((await range.body()).length).toBe(Math.min(100, size)); + for (const role of Object.keys(state.files)) { + await expect(page.locator(`.digital-attachments-list [href*="${name(role)}"], .digital-attachments-list [src*="${name(role)}"]`).first()).toBeAttached(); } + for (const file of Object.values(state.files)) await expectServed(request, file); }); test('the admin limits one file to signed-in readers and one to staff on the upgraded form', async ({ page }) => { await loginAdmin(page); - await page.goto(`${BASE}/admin/books/edit/${state.bookId}`); + await openEditForm(page, state.bookId); const rows = page.locator('[data-attachment-row]'); - await expect(rows).toHaveCount(3); - await rows.nth(1).locator('select[name$="[visibility]"]').selectOption('users'); - await rows.nth(2).locator('select[name$="[visibility]"]').selectOption('staff'); + await expect(rows).toHaveCount(Object.keys(state.files).length); + for (const [role, visibility] of [['readers', 'users'], ['staff', 'staff']]) { + const row = rows.filter({ has: page.locator(`input[name$="[url]"][value$="${name(role)}"]`) }); + await expect(row).toHaveCount(1); + await row.locator('select[name$="[visibility]"]').selectOption(visibility); + } await saveBook(page); const saved = JSON.parse(db(`SELECT digital_attachments FROM libri WHERE id=${state.bookId}`)); - expect(saved.map(a => a.visibility)).toEqual(['inherit', 'users', 'staff']); + const visibilityOf = role => saved.find(a => a.url === state.files[role].url)?.visibility; + expect(visibilityOf('readers')).toBe('users'); + expect(visibilityOf('staff')).toBe('staff'); + if (state.files.open) expect(visibilityOf('open')).toBe('inherit'); }); test('an anonymous visitor is sent to sign in for the readers file and refused the staff one', async ({ page, request }) => { const get = (url, headers = {}) => request.get(`${BASE}${url}`, { maxRedirects: 0, headers }); - const { 'Public edition': open, 'Readers edition': readers, Audiobook: staff } = state.files; - expect((await get(open.url)).status()).toBe(200); - const toLogin = await get(readers.url); + if (state.files.open) await expectServed(request, state.files.open); + const toLogin = await get(state.files.readers.url); expect(toLogin.status()).toBe(302); expect(toLogin.headers()['location']).toContain('return_url='); - expect((await get(staff.url)).status()).toBe(403); + expect((await get(state.files.staff.url)).status()).toBe(403); // Another case of the folder name is no way around the check, nor is a dot segment. - for (const variant of [readers.url.replace('/digital/', '/Digital/'), readers.url.replace('/uploads/', '/uploads/./')]) { - const response = await request.get(`${BASE}${variant}`, { maxRedirects: 0 }); - const served = response.status() === 200 && sha(await response.body()) === readers.sha; - expect(served, `${variant} must not serve the file`).toBe(false); + for (const role of ['readers', 'staff']) { + const { url, sha: digest } = state.files[role]; + for (const variant of [url.replace('/digital/', '/Digital/'), url.replace('/uploads/', '/uploads/./'), url.replace('/uploads/', '/UPLOADS/')]) { + const response = await request.get(`${BASE}${variant}`, { maxRedirects: 0 }); + const served = response.status() === 200 && sha(await response.body()) === digest; + expect(served, `${variant} must not serve the file`).toBe(false); + } } await page.goto(`${BASE}/libro/${state.bookId}`); - const list = page.locator('.digital-attachments-list'); - await expect(list).not.toContainText('Audiobook'); - await expect(list.locator('.digital-attachment--locked', { hasText: 'Readers edition' })).toBeVisible(); - await expect(page.locator(`a[href*="${path.basename(readers.url)}"]`)).toHaveCount(0); + await expect(page.locator('.digital-attachments-list .digital-attachment--locked')).toHaveCount(1); + for (const role of ['readers', 'staff']) { + await expect(page.locator(`[href*="${name(role)}"], [src*="${name(role)}"]`)).toHaveCount(0); + } }); test('a signed-in reader opens the readers file but not the staff one; the admin opens both', async ({ page, browser }) => { - const { 'Readers edition': readers, Audiobook: staff } = state.files; + const { readers, staff } = state.files; await page.goto(`${BASE}/libro/${state.bookId}`); const bookPath = new URL(page.url()).pathname; - await page.locator('.digital-attachment--locked', { hasText: 'Readers edition' }).locator('.digital-attachment__login').click(); + await page.locator('.digital-attachment--locked .digital-attachment__login').first().click(); await loginWith(page, reader.email, password); await page.waitForURL(u => u.pathname === bookPath); const asReader = await page.request.get(`${BASE}${readers.url}`, { maxRedirects: 0 }); From efc14a292612fbd8343e5ebdd76e5cc106833d8c Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 17:10:24 +0200 Subject: [PATCH 12/23] Sync the existing tables of an inactive bundled plugin when its version changes When a bundled plugin's version on disk moved past the one in the database, only an active plugin had its schema brought up to date (onActivate runs ensureSchema). A plugin that is registered but switched off kept its tables as they were. An install upgraded from 0.7.x keeps ncip-server registered and off, with the ncip_transactions of that time, so it lacked prenotazione_id and its foreign key until someone switched NCIP on, and its schema differed from a fresh install's. The version bump now runs ensureSchema for an inactive plugin whose declared tables already exist and miss a column or foreign key. It registers no hook and leaves the plugin off. A plugin whose tables were never created, like an Archives nobody activated, is left alone, since activating it creates them. The check uses is_callable because most plugin wrappers forward these methods through __call. plugin-schema-selfheal-fkcol.unit.php covers both cases: the inactive ncip-server gets the column and foreign key at the bump and stays off without hooks, and an inactive Archives without tables gets none. --- app/Support/PluginManager.php | 32 +++++++++++++++ tests/plugin-schema-selfheal-fkcol.unit.php | 45 +++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/app/Support/PluginManager.php b/app/Support/PluginManager.php index 6f3530339..34e0661ea 100644 --- a/app/Support/PluginManager.php +++ b/app/Support/PluginManager.php @@ -483,6 +483,38 @@ public function autoRegisterBundledPlugins(): int // that is behind. Each plugin now syncs independently. SecureLogger::error("[PluginManager] onActivate failed during upgrade for $pluginName; version rolled back to $dbVersion, other plugins keep syncing, retry next boot. Error: " . $e->getMessage()); } + } else { + // An inactive plugin whose tables already exist only gets them + // synced by onActivate: a column or foreign key a later version + // adds stays missing until someone switches the plugin on + // (ncip_transactions.prenotazione_id on an install upgraded + // from 0.7.x). Bring those tables to this version now, once, + // without registering any hook. A plugin whose tables are not + // there (never installed, like an untouched Archives) is left + // alone: activating it creates them. + try { + $schemaInstance = $this->instantiatePlugin([ + 'id' => (int) $row['id'], + 'name' => $pluginName, + 'path' => $pluginName, + 'main_file' => $pluginMeta['main_file'] ?? 'wrapper.php', + ]); + // is_callable, not method_exists: most wrappers forward to the + // plugin class through __call. + $declared = is_callable([$schemaInstance, 'expectedTables']) ? $schemaInstance->expectedTables() : []; + $tablesInstalled = is_array($declared) && $declared !== [] + && !$this->expectedTablesMissing($schemaInstance); + if ($tablesInstalled && is_callable([$schemaInstance, 'ensureSchema']) + && ($this->expectedColumnsMissing($schemaInstance) || $this->expectedForeignKeysMissing($schemaInstance))) { + $schemaResult = $schemaInstance->ensureSchema(); + if (is_array($schemaResult) && !empty($schemaResult['failed'])) { + SecureLogger::warning("[PluginManager] Schema sync of inactive $pluginName left: " . implode(', ', (array) $schemaResult['failed'])); + } + } + } catch (\Throwable $e) { + // Not fatal: activating the plugin runs ensureSchema again. + SecureLogger::warning("[PluginManager] Schema sync of inactive $pluginName failed: " . $e->getMessage()); + } } } elseif ($diskVersion === $dbVersion && (int) ($row['is_active'] ?? 0) === 1) { // Same disk/DB version, active plugin. Re-run onActivate when diff --git a/tests/plugin-schema-selfheal-fkcol.unit.php b/tests/plugin-schema-selfheal-fkcol.unit.php index 42e9faab5..29b870130 100644 --- a/tests/plugin-schema-selfheal-fkcol.unit.php +++ b/tests/plugin-schema-selfheal-fkcol.unit.php @@ -251,6 +251,51 @@ function check(bool $cond, string $desc): void check($columnExists('libri', 'audio_url'), 'B06 idempotent: column intact after a second sync'); } +/* ===== Scenario C — an INACTIVE plugin's existing tables follow its version ===== */ +// An install upgraded from 0.7.x keeps ncip-server registered but switched off, with +// the ncip_transactions of that time: no prenotazione_id. The version bump used to sync +// the schema of active plugins only, so the column stayed missing until someone +// switched NCIP on. The bump now brings the tables an inactive plugin already has to +// its version, without registering any hook or switching it on. +$ncipC = $db->query("SELECT id, is_active, version FROM plugins WHERE name='ncip-server'")->fetch_assoc(); +if ($ncipC) { + $cId = (int) $ncipC['id']; + $cActive0 = (int) $ncipC['is_active']; + $cHooks0 = $hookCount($cId); + $db->query("UPDATE plugins SET is_active=0 WHERE id={$cId}"); + $db->query("DELETE FROM plugin_hooks WHERE plugin_id={$cId}"); + @$db->query("ALTER TABLE ncip_transactions DROP FOREIGN KEY ncip_transactions_ibfk_3"); + @$db->query("ALTER TABLE ncip_transactions DROP COLUMN prenotazione_id"); + $db->query("UPDATE plugins SET version='0.0.1' WHERE id={$cId}"); + check(!$columnExists('ncip_transactions', 'prenotazione_id'), 'C01 setup: inactive ncip-server at an older version, ncip_transactions without prenotazione_id'); + + $runSync(); + check($columnExists('ncip_transactions', 'prenotazione_id'), 'C02 the version bump adds prenotazione_id to the inactive plugin\'s table'); + check($fkExists('ncip_transactions', 'prenotazione_id', 'prenotazioni'), 'C03 ...with its foreign key'); + $after = $db->query("SELECT is_active, version FROM plugins WHERE id={$cId}")->fetch_assoc(); + check((int) $after['is_active'] === 0 && $hookCount($cId) === 0, 'C04 the plugin stays switched off, with no hook registered'); + check((string) $after['version'] === $diskVersion('ncip-server'), 'C05 its version is the one on disk'); + + // Put the activation state back; an active plugin gets its hooks again from the sync. + $db->query("UPDATE plugins SET is_active={$cActive0} WHERE id={$cId}"); + if ($cActive0 === 1) { + $runSync(); + check($hookCount($cId) >= $cHooks0, 'C06 restored: ncip-server active again with its hooks'); + } +} + +/* ===== Scenario D — an inactive plugin that never created its tables is left alone ===== */ +$arch = $db->query("SELECT id, is_active, version FROM plugins WHERE name='archives'")->fetch_assoc(); +$archTable = static fn (): bool => (bool) $db->query("SELECT 1 FROM information_schema.TABLES WHERE TABLE_SCHEMA=DATABASE() AND TABLE_NAME='archival_units'")->num_rows; +if ($arch && (int) $arch['is_active'] === 0 && !$archTable()) { + $dId = (int) $arch['id']; + $dVersion0 = $db->real_escape_string((string) $arch['version']); + $db->query("UPDATE plugins SET version='0.0.1' WHERE id={$dId}"); + $runSync(); + check(!$archTable(), 'D01 the version bump of an inactive, never-installed Archives creates no table'); + $db->query("UPDATE plugins SET version='{$dVersion0}' WHERE id={$dId}"); +} + // Belt and suspenders: if neither scenario ran a single assertion, the gate // would report success without verifying anything. Refuse to pass empty. if ($TESTNO === 0) { From af2267396771d8bb8ddd799bd5b42297215cf939 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 17:10:29 +0200 Subject: [PATCH 13/23] Re-register the Digital Library hooks on old releases that installed it without them Some 0.7.x installers left digital-library active with only its routes hook, so the book form had no uploader and the upgrade test could not upload anything on 0.7.68. The test now switches the plugin off and on from the plugins page when book.form.digital_fields is missing, as an admin would, and checks the hook is there before uploading. --- tests/digital-upgrade-480.spec.js | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tests/digital-upgrade-480.spec.js b/tests/digital-upgrade-480.spec.js index 6e18d26f9..93d179a74 100644 --- a/tests/digital-upgrade-480.spec.js +++ b/tests/digital-upgrade-480.spec.js @@ -95,23 +95,32 @@ test.describe.serial(`Digital Library across the upgrade (#480) — ${PHASE}`, ( /** @type {{bookId:number, mode:string, files:Record}} */ const state = { bookId: 0, mode: '', files: {} }; - test('the digital-library plugin is active on the old release', async ({ page }) => { + test('the digital-library plugin is active on the old release, with its hooks', async ({ page }) => { await loginAdmin(page); - await page.goto(`${BASE}/admin/plugins`); const id = db("SELECT id FROM plugins WHERE name='digital-library'"); expect(id, 'digital-library registered').not.toBe(''); - if (db("SELECT is_active FROM plugins WHERE name='digital-library'") !== '1') { - // The plugin's own "Attiva" / "Attiva plugin" button, in every release of the page. - const activated = page.waitForResponse(r => r.url().includes(`/admin/plugins/${id}/activate`)); - await page.locator(`button[onclick="activatePlugin(${id})"]`).click(); + /** Click the plugin's own button, confirm, and check the JSON answer of the action. */ + const toggle = async action => { + await page.goto(`${BASE}/admin/plugins`); + const answered = page.waitForResponse(r => r.url().includes(`/admin/plugins/${id}/${action}`)); + await page.locator(`button[onclick="${action}Plugin(${id})"]`).click(); await page.locator('.swal2-confirm:visible').click(); - const response = await activated; + const response = await answered; const body = await response.json().catch(() => ({})); expect(response.status(), JSON.stringify(body)).toBe(200); expect(body.success, JSON.stringify(body)).toBe(true); await page.locator('.swal2-confirm:visible').click(); + }; + const hooks = () => Number(db(`SELECT COUNT(*) FROM plugin_hooks WHERE plugin_id=${id} AND hook_name='book.form.digital_fields'`)); + if (db(`SELECT is_active FROM plugins WHERE id=${id}`) !== '1') await toggle('activate'); + // Some 0.7.x installers left the plugin active with only its routes hook: the book form + // then has no uploader. Switching it off and on again, as an admin would, registers them. + if (hooks() === 0) { + await toggle('deactivate'); + await toggle('activate'); } - await expect.poll(() => db("SELECT is_active FROM plugins WHERE name='digital-library'")).toBe('1'); + expect(db(`SELECT is_active FROM plugins WHERE id=${id}`)).toBe('1'); + expect(hooks(), 'book.form.digital_fields registered').toBeGreaterThan(0); }); test('the admin uploads the digital files through the old book form', async ({ page }) => { From a07aba1cf8a2d78a1d4a2e7703cd433e8d8ace44 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 17:39:34 +0200 Subject: [PATCH 14/23] Sync an inactive plugin's tables even when its version bump is already taken The previous release's Updater runs the bundled plugin sync itself, with its own PluginManager still in memory. On an install upgraded from 0.7.16 that old code takes NCIP's 1.0.0 to 1.0.5 bump, so the new code finds the plugin already at the disk version and never adds ncip_transactions.prenotazione_id. The inactive-plugin schema sync now also runs on the same-version path. It runs once per release, and only reads until a column or foreign key is actually missing. --- app/Support/PluginManager.php | 81 ++++++++++++--------- tests/plugin-schema-selfheal-fkcol.unit.php | 12 +++ 2 files changed, 60 insertions(+), 33 deletions(-) diff --git a/app/Support/PluginManager.php b/app/Support/PluginManager.php index 34e0661ea..78dd731f3 100644 --- a/app/Support/PluginManager.php +++ b/app/Support/PluginManager.php @@ -484,40 +484,18 @@ public function autoRegisterBundledPlugins(): int SecureLogger::error("[PluginManager] onActivate failed during upgrade for $pluginName; version rolled back to $dbVersion, other plugins keep syncing, retry next boot. Error: " . $e->getMessage()); } } else { - // An inactive plugin whose tables already exist only gets them - // synced by onActivate: a column or foreign key a later version - // adds stays missing until someone switches the plugin on - // (ncip_transactions.prenotazione_id on an install upgraded - // from 0.7.x). Bring those tables to this version now, once, - // without registering any hook. A plugin whose tables are not - // there (never installed, like an untouched Archives) is left - // alone: activating it creates them. - try { - $schemaInstance = $this->instantiatePlugin([ - 'id' => (int) $row['id'], - 'name' => $pluginName, - 'path' => $pluginName, - 'main_file' => $pluginMeta['main_file'] ?? 'wrapper.php', - ]); - // is_callable, not method_exists: most wrappers forward to the - // plugin class through __call. - $declared = is_callable([$schemaInstance, 'expectedTables']) ? $schemaInstance->expectedTables() : []; - $tablesInstalled = is_array($declared) && $declared !== [] - && !$this->expectedTablesMissing($schemaInstance); - if ($tablesInstalled && is_callable([$schemaInstance, 'ensureSchema']) - && ($this->expectedColumnsMissing($schemaInstance) || $this->expectedForeignKeysMissing($schemaInstance))) { - $schemaResult = $schemaInstance->ensureSchema(); - if (is_array($schemaResult) && !empty($schemaResult['failed'])) { - SecureLogger::warning("[PluginManager] Schema sync of inactive $pluginName left: " . implode(', ', (array) $schemaResult['failed'])); - } - } - } catch (\Throwable $e) { - // Not fatal: activating the plugin runs ensureSchema again. - SecureLogger::warning("[PluginManager] Schema sync of inactive $pluginName failed: " . $e->getMessage()); - } + $this->syncInactivePluginSchema((int) $row['id'], $pluginName, $pluginMeta); } - } elseif ($diskVersion === $dbVersion && (int) ($row['is_active'] ?? 0) === 1) { - // Same disk/DB version, active plugin. Re-run onActivate when + } elseif ((int) ($row['is_active'] ?? 0) !== 1) { + // Same version, but the bump may already have been taken by the + // previous release's Updater, which runs this sync with its own + // PluginManager still in memory: an install upgraded from 0.7.16 + // arrives here with NCIP at 1.0.5 and prenotazione_id missing. + // This pass runs once per release, and the check is read-only + // until something is actually missing. + $this->syncInactivePluginSchema((int) $row['id'], $pluginName, $pluginMeta); + } elseif ($diskVersion === $dbVersion) { + // Same disk/DB version, active plugin (the inactive ones took the branch above). Re-run onActivate when // EITHER: // (a) hooks are missing — code added hooks not yet in DB // (e.g. merging branches that extend the same version), or @@ -2780,6 +2758,43 @@ public static function clearPluginCache(): void SitemapCache::invalidate('plugin lifecycle change'); } + /** + * An inactive plugin whose tables already exist only gets them synced by + * onActivate: a column or foreign key a later version adds stays missing until + * someone switches the plugin on (ncip_transactions.prenotazione_id on an + * install upgraded from 0.7.x). Bring those tables to this version without + * registering any hook. A plugin whose tables are not there (never installed, + * like an untouched Archives) is left alone: activating it creates them. + * + * @param array $pluginMeta + */ + private function syncInactivePluginSchema(int $pluginId, string $pluginName, array $pluginMeta): void + { + try { + $schemaInstance = $this->instantiatePlugin([ + 'id' => $pluginId, + 'name' => $pluginName, + 'path' => $pluginName, + 'main_file' => $pluginMeta['main_file'] ?? 'wrapper.php', + ]); + // is_callable, not method_exists: most wrappers forward to the + // plugin class through __call. + $declared = is_callable([$schemaInstance, 'expectedTables']) ? $schemaInstance->expectedTables() : []; + $tablesInstalled = is_array($declared) && $declared !== [] + && !$this->expectedTablesMissing($schemaInstance); + if ($tablesInstalled && is_callable([$schemaInstance, 'ensureSchema']) + && ($this->expectedColumnsMissing($schemaInstance) || $this->expectedForeignKeysMissing($schemaInstance))) { + $schemaResult = $schemaInstance->ensureSchema(); + if (is_array($schemaResult) && !empty($schemaResult['failed'])) { + SecureLogger::warning("[PluginManager] Schema sync of inactive $pluginName left: " . implode(', ', (array) $schemaResult['failed'])); + } + } + } catch (\Throwable $e) { + // Not fatal: activating the plugin runs ensureSchema again. + SecureLogger::warning("[PluginManager] Schema sync of inactive $pluginName failed: " . $e->getMessage()); + } + } + /** * Version stamp used to scope the maintenance marker: a new release (new * version.json) invalidates it so bundled plugins re-sync right away. diff --git a/tests/plugin-schema-selfheal-fkcol.unit.php b/tests/plugin-schema-selfheal-fkcol.unit.php index 29b870130..a7d3715dc 100644 --- a/tests/plugin-schema-selfheal-fkcol.unit.php +++ b/tests/plugin-schema-selfheal-fkcol.unit.php @@ -276,6 +276,18 @@ function check(bool $cond, string $desc): void check((int) $after['is_active'] === 0 && $hookCount($cId) === 0, 'C04 the plugin stays switched off, with no hook registered'); check((string) $after['version'] === $diskVersion('ncip-server'), 'C05 its version is the one on disk'); + // The bump can be gone before this code ever runs: the previous release's Updater + // syncs the plugins with its own PluginManager still in memory, so an install + // upgraded from 0.7.16 arrives with NCIP already at the disk version and the + // column still missing. The sync has to fix that too. + @$db->query("ALTER TABLE ncip_transactions DROP FOREIGN KEY ncip_transactions_ibfk_3"); + @$db->query("ALTER TABLE ncip_transactions DROP COLUMN prenotazione_id"); + check(!$columnExists('ncip_transactions', 'prenotazione_id'), 'C07 setup: inactive ncip-server already at the disk version, prenotazione_id missing'); + $runSync(); + check($columnExists('ncip_transactions', 'prenotazione_id'), 'C08 the sync adds prenotazione_id with no version bump'); + check($fkExists('ncip_transactions', 'prenotazione_id', 'prenotazioni'), 'C09 ...with its foreign key'); + check((int) $db->query("SELECT is_active FROM plugins WHERE id={$cId}")->fetch_row()[0] === 0 && $hookCount($cId) === 0, 'C10 the plugin is still off, with no hook'); + // Put the activation state back; an active plugin gets its hooks again from the sync. $db->query("UPDATE plugins SET is_active={$cActive0} WHERE id={$cId}"); if ($cActive0 === 1) { From 5789146fa6da94d490c020c1eb288fd7cb4cf353 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 18:04:47 +0200 Subject: [PATCH 15/23] Give the real cadence of the inactive plugin schema pass The comment said the same-version sync of an inactive bundled plugin runs once per release. autoRegisterBundledPlugins() runs on every maintenance window (the 900-second plugins_maintenance_* marker per version stamp) and on every getAllPlugins() call, so on each load of the admin plugins page. The check stays read-only until something is missing; the comment now says when it runs, so nobody relies on a once-per-release guarantee. --- app/Support/PluginManager.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/Support/PluginManager.php b/app/Support/PluginManager.php index 78dd731f3..3b95647b2 100644 --- a/app/Support/PluginManager.php +++ b/app/Support/PluginManager.php @@ -491,8 +491,10 @@ public function autoRegisterBundledPlugins(): int // previous release's Updater, which runs this sync with its own // PluginManager still in memory: an install upgraded from 0.7.16 // arrives here with NCIP at 1.0.5 and prenotazione_id missing. - // This pass runs once per release, and the check is read-only - // until something is actually missing. + // This pass runs on every maintenance window (900 s per version + // stamp) and on every getAllPlugins() call (each load of the admin + // plugins page), and the check is read-only until something is + // actually missing. $this->syncInactivePluginSchema((int) $row['id'], $pluginName, $pluginMeta); } elseif ($diskVersion === $dbVersion) { // Same disk/DB version, active plugin (the inactive ones took the branch above). Re-run onActivate when From 0cbbed3a62e32730476412fb3accb4b3fce40303 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 18:06:04 +0200 Subject: [PATCH 16/23] Skip the upgrade spec without a phase under a reason the skip audit accepts The regular browser shards run every spec, and digital-upgrade-480.spec.js has nothing to do there without E2E_UPGRADE_PHASE. Its single skip reason, "E2E credentials and E2E_UPGRADE_PHASE required", matched the audit's "required" pattern, so all 8 tests counted as hidden infrastructure failures and failed shard 2. The missing phase now skips with its own reason, like the 412 upgrade spec, while missing credentials keep a reason the audit still flags. --- tests/digital-upgrade-480.spec.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/digital-upgrade-480.spec.js b/tests/digital-upgrade-480.spec.js index 93d179a74..4911a7935 100644 --- a/tests/digital-upgrade-480.spec.js +++ b/tests/digital-upgrade-480.spec.js @@ -25,7 +25,10 @@ const PHASE = process.env.E2E_UPGRADE_PHASE || ''; const STATE = process.env.E2E_UPGRADE_STATE || path.join(ROOT, 'storage/tmp/digital-upgrade-480.json'); const STORAGE = path.join(ROOT, 'storage/digital'); const LEGACY = path.join(ROOT, 'public/uploads/digital'); -test.skip(!process.env.E2E_ADMIN_EMAIL || !process.env.E2E_DB_NAME || !PHASE, 'E2E credentials and E2E_UPGRADE_PHASE required'); +// Without a phase this is the regular browser run, where the spec has nothing to do; a +// reason with "required" there would read as broken infrastructure to the skip audit. +test.skip(!PHASE, 'Runs only around a real upgrade (phase given by E2E_UPGRADE_PHASE)'); +test.skip(!process.env.E2E_ADMIN_EMAIL || !process.env.E2E_DB_NAME, 'E2E credentials required'); function db(sql) { const args = ['-u', process.env.E2E_DB_USER, process.env.E2E_DB_NAME, '-N', '-B', '-e', sql]; From 233565bf9f23c9c09d7f2107a1ce57b3728ccd99 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 18:35:54 +0200 Subject: [PATCH 17/23] Keep the schema-only instance of a switched-off plugin from registering hooks syncInactivePluginSchema() built its instance through instantiatePlugin(), which always calls setPluginId(). digital-library, open-library, goodlib and scraping-pro register their hooks there whenever they have none, and digital-library also moves its files, so syncing the schema of a plugin the admin switched off gave it hook rows. instantiatePlugin() now takes $setPluginId (true by default) and the schema-only path passes false; no ensureSchema() reads the plugin id. Scenario E covers it with an inactive digital-library, and fails on the previous commit. Scenario C now drops prenotazione_id together with whatever foreign key holds it under its actual name, and registers a restore, so a failure halfway puts NCIP's activation state, version and hooks back. The upgrade spec sends the /Digital/, /./ and /UPLOADS/ variants through curl --path-as-is: Playwright's request.get parses the URL and drops the "/./" segment, so that variant only repeated the plain request. --- app/Support/PluginManager.php | 14 ++++-- tests/digital-upgrade-480.spec.js | 19 ++++++- tests/plugin-schema-selfheal-fkcol.unit.php | 56 +++++++++++++++++++-- 3 files changed, 80 insertions(+), 9 deletions(-) diff --git a/app/Support/PluginManager.php b/app/Support/PluginManager.php index 3b95647b2..49ba08342 100644 --- a/app/Support/PluginManager.php +++ b/app/Support/PluginManager.php @@ -2773,12 +2773,14 @@ public static function clearPluginCache(): void private function syncInactivePluginSchema(int $pluginId, string $pluginName, array $pluginMeta): void { try { + // Schema only: no setPluginId(), which would register the hooks of a + // plugin that is switched off (or, for digital-library, move its files). $schemaInstance = $this->instantiatePlugin([ 'id' => $pluginId, 'name' => $pluginName, 'path' => $pluginName, 'main_file' => $pluginMeta['main_file'] ?? 'wrapper.php', - ]); + ], false); // is_callable, not method_exists: most wrappers forward to the // plugin class through __call. $declared = is_callable([$schemaInstance, 'expectedTables']) ? $schemaInstance->expectedTables() : []; @@ -2920,7 +2922,13 @@ private function disableOrphanHook(int $pluginId, string $hookName, string $call return $affected > 0; } - private function instantiatePlugin(array $plugin): object + /** + * Build a plugin instance. $setPluginId false gives a schema-only instance: some + * plugins (digital-library, open-library, goodlib, scraping-pro) register their + * hooks in setPluginId(), and digital-library also moves its files, which a + * plugin the admin switched off must not do. No ensureSchema() reads the id. + */ + private function instantiatePlugin(array $plugin, bool $setPluginId = true): object { // Save plugin data to prefixed variables before require_once // This prevents plugin files from overwriting $plugin variable (which some do) @@ -2942,7 +2950,7 @@ private function instantiatePlugin(array $plugin): object $instance = new $className($this->db, $this->hookManager); - if (is_callable([$instance, 'setPluginId'])) { + if ($setPluginId && is_callable([$instance, 'setPluginId'])) { try { $instance->setPluginId($_pluginId); } catch (\Throwable $e) { diff --git a/tests/digital-upgrade-480.spec.js b/tests/digital-upgrade-480.spec.js index 4911a7935..3f34588c8 100644 --- a/tests/digital-upgrade-480.spec.js +++ b/tests/digital-upgrade-480.spec.js @@ -17,6 +17,7 @@ const { test, expect } = require('@playwright/test'); const { execFileSync } = require('child_process'); const crypto = require('crypto'); const fs = require('fs'); +const os = require('os'); const path = require('path'); const BASE = process.env.E2E_BASE_URL || 'http://localhost:8081'; @@ -30,6 +31,20 @@ const LEGACY = path.join(ROOT, 'public/uploads/digital'); test.skip(!PHASE, 'Runs only around a real upgrade (phase given by E2E_UPGRADE_PHASE)'); test.skip(!process.env.E2E_ADMIN_EMAIL || !process.env.E2E_DB_NAME, 'E2E credentials required'); +// GET exactly the path given: Playwright's request.get parses the URL (WHATWG), which +// drops a "/./" segment before sending, so that variant would repeat the plain request. +function getPathAsIs(url) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'digital-upgrade-480-')); + const bodyPath = path.join(dir, 'response'); + try { + const status = execFileSync('curl', ['--silent', '--show-error', '--path-as-is', '--max-redirs', '0', + '--output', bodyPath, '--write-out', '%{http_code}', url], { encoding: 'utf8' }).trim(); + return { status: Number(status), body: fs.existsSync(bodyPath) ? fs.readFileSync(bodyPath) : Buffer.alloc(0) }; + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +} + function db(sql) { const args = ['-u', process.env.E2E_DB_USER, process.env.E2E_DB_NAME, '-N', '-B', '-e', sql]; if (process.env.E2E_DB_SOCKET) args.unshift('-S', process.env.E2E_DB_SOCKET); @@ -254,8 +269,8 @@ test.describe.serial('Digital Library across the upgrade (#480) — after', () = for (const role of ['readers', 'staff']) { const { url, sha: digest } = state.files[role]; for (const variant of [url.replace('/digital/', '/Digital/'), url.replace('/uploads/', '/uploads/./'), url.replace('/uploads/', '/UPLOADS/')]) { - const response = await request.get(`${BASE}${variant}`, { maxRedirects: 0 }); - const served = response.status() === 200 && sha(await response.body()) === digest; + const response = getPathAsIs(`${BASE}${variant}`); + const served = response.status === 200 && sha(response.body) === digest; expect(served, `${variant} must not serve the file`).toBe(false); } } diff --git a/tests/plugin-schema-selfheal-fkcol.unit.php b/tests/plugin-schema-selfheal-fkcol.unit.php index a7d3715dc..b3ebe080a 100644 --- a/tests/plugin-schema-selfheal-fkcol.unit.php +++ b/tests/plugin-schema-selfheal-fkcol.unit.php @@ -106,6 +106,19 @@ function check(bool $cond, string $desc): void $hookCount = function (int $pluginId) use ($db): int { return (int) $db->query("SELECT COUNT(*) FROM plugin_hooks WHERE plugin_id={$pluginId}")->fetch_row()[0]; }; +// Drop prenotazione_id with whatever foreign key holds it, under its actual name: an +// install whose FK is not called ncip_transactions_ibfk_3 would keep both otherwise. +$dropPrenotazione = static function (mysqli $db): void { + $res = $db->query("SELECT DISTINCT CONSTRAINT_NAME FROM information_schema.KEY_COLUMN_USAGE + WHERE TABLE_SCHEMA=DATABASE() AND TABLE_NAME='ncip_transactions' + AND COLUMN_NAME='prenotazione_id' AND REFERENCED_TABLE_NAME IS NOT NULL"); + foreach ($res ? $res->fetch_all() : [] as [$name]) { + $db->query('ALTER TABLE ncip_transactions DROP FOREIGN KEY `' . str_replace('`', '``', (string) $name) . '`'); + } + if ($db->query("SHOW COLUMNS FROM ncip_transactions LIKE 'prenotazione_id'")->num_rows > 0) { + $db->query('ALTER TABLE ncip_transactions DROP COLUMN prenotazione_id'); + } +}; $diskVersion = function (string $plugin): string { $meta = json_decode((string) @file_get_contents(__DIR__ . "/../storage/plugins/{$plugin}/plugin.json"), true); return (string) ($meta['version'] ?? '1.0.0'); @@ -262,10 +275,19 @@ function check(bool $cond, string $desc): void $cId = (int) $ncipC['id']; $cActive0 = (int) $ncipC['is_active']; $cHooks0 = $hookCount($cId); + $cVersion0 = (string) $ncipC['version']; + // A failure halfway must not leave NCIP switched off without its hooks: put the + // activation state, the version and (for an active plugin) the hooks back. + $restores[] = function (mysqli $db) use ($cId, $cActive0, $cVersion0, $runSync): void { + $v = $db->real_escape_string($cVersion0); + $db->query("UPDATE plugins SET is_active={$cActive0}, version='{$v}' WHERE id={$cId}"); + if ($cActive0 === 1) { + $runSync(); + } + }; $db->query("UPDATE plugins SET is_active=0 WHERE id={$cId}"); $db->query("DELETE FROM plugin_hooks WHERE plugin_id={$cId}"); - @$db->query("ALTER TABLE ncip_transactions DROP FOREIGN KEY ncip_transactions_ibfk_3"); - @$db->query("ALTER TABLE ncip_transactions DROP COLUMN prenotazione_id"); + $dropPrenotazione($db); $db->query("UPDATE plugins SET version='0.0.1' WHERE id={$cId}"); check(!$columnExists('ncip_transactions', 'prenotazione_id'), 'C01 setup: inactive ncip-server at an older version, ncip_transactions without prenotazione_id'); @@ -280,8 +302,7 @@ function check(bool $cond, string $desc): void // syncs the plugins with its own PluginManager still in memory, so an install // upgraded from 0.7.16 arrives with NCIP already at the disk version and the // column still missing. The sync has to fix that too. - @$db->query("ALTER TABLE ncip_transactions DROP FOREIGN KEY ncip_transactions_ibfk_3"); - @$db->query("ALTER TABLE ncip_transactions DROP COLUMN prenotazione_id"); + $dropPrenotazione($db); check(!$columnExists('ncip_transactions', 'prenotazione_id'), 'C07 setup: inactive ncip-server already at the disk version, prenotazione_id missing'); $runSync(); check($columnExists('ncip_transactions', 'prenotazione_id'), 'C08 the sync adds prenotazione_id with no version bump'); @@ -308,6 +329,33 @@ function check(bool $cond, string $desc): void $db->query("UPDATE plugins SET version='{$dVersion0}' WHERE id={$dId}"); } +/* ===== Scenario E — the inactive sync never registers hooks or moves files ===== */ +// digital-library registers its hooks in setPluginId() whenever it has none (and moves +// its files there too). The schema-only instance of a switched-off plugin must skip +// that call, or a plugin the admin turned off gets hook rows behind their back. +if (isset($dlId)) { + $eHooks0 = $hookCount($dlId); + $restores[] = function (mysqli $db) use ($dlId, $dlActive0, $runSync): void { + $db->query("UPDATE plugins SET is_active={$dlActive0} WHERE id={$dlId}"); + if ($dlActive0 === 1) { + $runSync(); + } + }; + $db->query("UPDATE plugins SET is_active=0, version='0.0.1' WHERE id={$dlId}"); + $db->query("DELETE FROM plugin_hooks WHERE plugin_id={$dlId}"); + @$db->query("ALTER TABLE libri DROP COLUMN audio_url"); + check(!$columnExists('libri', 'audio_url') && $hookCount($dlId) === 0, 'E01 setup: inactive digital-library at an older version, no hooks, libri.audio_url missing'); + $runSync(); + check($columnExists('libri', 'audio_url'), 'E02 the version bump still syncs the inactive plugin\'s column'); + check($hookCount($dlId) === 0, 'E03 ...without registering any hook for the switched-off plugin'); + check((int) $db->query("SELECT is_active FROM plugins WHERE id={$dlId}")->fetch_row()[0] === 0, 'E04 ...and it stays switched off'); + $db->query("UPDATE plugins SET is_active={$dlActive0} WHERE id={$dlId}"); + if ($dlActive0 === 1) { + $runSync(); + check($hookCount($dlId) >= $eHooks0, 'E05 restored: digital-library active again with its hooks'); + } +} + // Belt and suspenders: if neither scenario ran a single assertion, the gate // would report success without verifying anything. Refuse to pass empty. if ($TESTNO === 0) { From a099335db3407af0aab185535b56e9dee926e13d Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 19:38:37 +0000 Subject: [PATCH 18/23] Keep public/.htaccess when PCRE gives up, accept a private rule ending in END apply() cast every preg_* result to a string. When PCRE gives up (backtrack or JIT limits) those calls return null, so apply() returned '' and heal() wrote an empty public/.htaccess, then called it acceptable because it holds no RewriteRule: the site lost its front controller and every protection with it (review F007). A null now keeps the file as it is, and the next maintenance window tries again. safe() took a private rule as protective only with L or last. END stops rewriting too, so a custom file with [NC,QSA,END] was reported as leaking on every maintenance window (part of F010). tests/htaccess-heal-history-480.unit.php runs apply() and heal() with pcre.backtrack_limit=1 and checks the file on disk is unchanged, and checks the END spelling; the three checks fail on the previous commit (the on-disk file was emptied). ALL 198 PASS, and digital-access-matrix-480.unit.php still passes. --- app/Support/PrivateUploadsRewriteHeal.php | 21 +++++++++++------ tests/htaccess-heal-history-480.unit.php | 28 +++++++++++++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/app/Support/PrivateUploadsRewriteHeal.php b/app/Support/PrivateUploadsRewriteHeal.php index 80606fbc9..a4555d640 100644 --- a/app/Support/PrivateUploadsRewriteHeal.php +++ b/app/Support/PrivateUploadsRewriteHeal.php @@ -57,7 +57,7 @@ public static function apply(string $content): string // that condition onto the inserted private rule (or, for B's removal, onto the next // rule), so such a block is left as it is and safe() reports the file. if (preg_match(self::REWRITE_C, $out) === 1) { - $out = (string) preg_replace_callback( + $out = preg_replace_callback( self::REWRITE_C, static fn (array $m): string => self::conditionPending($out, $m[0][1]) ? $m[0][0] : $m[1][0] . self::PRIVATE_RULE, $out, @@ -72,12 +72,18 @@ public static function apply(string $content): string // The private half's offset is above its optional comment block, which the // backward scan skips anyway. if (!self::conditionPending($out, $passthrough[0][1]) && !self::conditionPending($out, $private[0][1])) { - $out = (string) preg_replace(self::PRIVATE_B, '', $out, 1); - $out = (string) preg_replace_callback(self::PASSTHROUGH_B, static fn (array $m): string => self::block($m[1], $eol), $out, 1); + $out = preg_replace(self::PRIVATE_B, '', $out, 1); + $out = $out === null ? null : preg_replace_callback(self::PASSTHROUGH_B, static fn (array $m): string => self::block($m[1], $eol), $out, 1); } } elseif (preg_match(self::PASSTHROUGH_A, $out, $passthrough, PREG_OFFSET_CAPTURE) === 1 && !self::hasPrivateRule($out) && !self::conditionPending($out, $passthrough[0][1])) { - $out = (string) preg_replace_callback(self::PASSTHROUGH_A, static fn (array $m): string => self::block($m[1], $eol), $out, 1); + $out = preg_replace_callback(self::PASSTHROUGH_A, static fn (array $m): string => self::block($m[1], $eol), $out, 1); + } + // preg_* return null when PCRE gives up (backtrack or JIT limits): cast to a string, + // that null became an empty file, which heal() wrote over public/.htaccess and then + // called acceptable because it has no RewriteRule. A failure keeps the file instead. + if ($out === null) { + return $content; } // Whatever changed in the rewrite rules must leave the private rule, with [NC], ahead // of every public /uploads/ rule; anything else is a file we do not understand, and @@ -87,7 +93,7 @@ public static function apply(string $content): string } // The no-gzip list does not depend on the rewrite rules, so a declined heal still // gets PDF and WAV in it. - return (string) preg_replace(self::NOGZIP_OLD, self::NOGZIP_NEW, $out); + return preg_replace(self::NOGZIP_OLD, self::NOGZIP_NEW, $out) ?? $content; } /** @@ -141,11 +147,12 @@ public static function safe(string $content): bool if (preg_match_all('/^[ \t]*RewriteRule[ \t]+\^uploads\/\(digital\|archives\/documents\|storage\)\/[ \t]+index\.php[ \t]+\[([^\]\r\n]*)\]/mi', $content, $all, PREG_SET_ORDER | PREG_OFFSET_CAPTURE) > 0) { foreach ($all as $match) { $flags = array_map(static fn (string $f): string => strtoupper(trim($f)), explode(',', $match[1][0])); - // NC / nocase and L / last are the same flags under their long names. + // NC / nocase and L / last are the same flags under their long names; END + // stops rewriting too, so a private rule ending in END protects as well. // A RewriteCond above it, or a rule chained into it with C, makes it conditional: // whatever the condition rejects goes on to the public rule, so such a rule // protects nothing on its own. - if (array_intersect(['NC', 'NOCASE'], $flags) !== [] && array_intersect(['L', 'LAST'], $flags) !== [] + if (array_intersect(['NC', 'NOCASE'], $flags) !== [] && array_intersect(['L', 'LAST', 'END'], $flags) !== [] && !self::conditionPending($content, $match[0][1])) { $private = $match[0][1]; break; diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index ae46ee653..e60e47254 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -243,5 +243,33 @@ '0.7.71 (C) with a chained rule right above: the private rule is not rewritten into the chain, and the file is reported'); $check(Heal::safe($chainAbove('RewriteRule ^x$ - [CO=seen:1:example.org]')), 'a cookie flag (CO=) is no chain: safe'); +// END stops rewriting like L, so a private rule ending in END protects as well: a custom +// but safe file must not be reported on every maintenance window. +$endPrivate = str_replace($privateLine, str_replace('[NC,QSA,L]', '[NC,QSA,END]', $privateLine), $base71); +$check($endPrivate !== $base71 && Heal::safe($endPrivate), 'a private rule ending in END instead of L is safe'); + +// When PCRE gives up (backtrack or JIT limits), preg_* return null. Cast to a string, that +// null became an empty public/.htaccess, which heal() wrote and called acceptable. +$pcreLimits = [ini_get('pcre.backtrack_limit'), ini_get('pcre.jit')]; +$tmpDir = sys_get_temp_dir() . '/htaccess-heal-pcre-' . getmypid(); +@mkdir($tmpDir . '/backups', 0775, true); +$tmpFile = $tmpDir . '/.htaccess'; +file_put_contents($tmpFile, $fixture('0.7.71')); +try { + ini_set('pcre.jit', '0'); + ini_set('pcre.backtrack_limit', '1'); + $check(Heal::apply($fixture('0.7.71')) === $fixture('0.7.71'), 'when PCRE gives up, apply() keeps the file as it is, never an empty one'); + Heal::heal($tmpFile, $tmpDir . '/backups'); + clearstatcache(); + $check((string) file_get_contents($tmpFile) === $fixture('0.7.71'), 'and heal() leaves public/.htaccess untouched on disk'); +} finally { + ini_set('pcre.backtrack_limit', (string) $pcreLimits[0]); + ini_set('pcre.jit', (string) $pcreLimits[1]); + array_map('unlink', glob($tmpDir . '/backups/*') ?: []); + @rmdir($tmpDir . '/backups'); + @unlink($tmpFile); + @rmdir($tmpDir); +} + echo "\n" . ($fail === 0 ? "ALL {$pass} PASS" : "{$fail} FAILED, {$pass} passed") . "\n"; exit($fail > 0 ? 1 : 0); From 13dbdd45126aacab5090ca2780994f1e2d39bb93 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 19:47:20 +0000 Subject: [PATCH 19/23] Report an Apache file PCRE could not read, cover a replace that gives up after the match acceptable() now treats a preg_match error as "has rewrite rules", so heal() returns false instead of calling the file safe. The PCRE test asserts that, and adds a 0.7.71 file whose rule is recognised but whose replace gives up on a later line: apply() keeps the original and heal() leaves the file on disk. The --path-as-is curl in the upgrade spec gets --max-time and an exec timeout. --- app/Support/PrivateUploadsRewriteHeal.php | 2 +- tests/digital-upgrade-480.spec.js | 4 ++-- tests/htaccess-heal-history-480.unit.php | 13 ++++++++++++- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/app/Support/PrivateUploadsRewriteHeal.php b/app/Support/PrivateUploadsRewriteHeal.php index a4555d640..6d2a8df6e 100644 --- a/app/Support/PrivateUploadsRewriteHeal.php +++ b/app/Support/PrivateUploadsRewriteHeal.php @@ -223,7 +223,7 @@ private static function missesPrivateFolders(string $pattern): bool /** Safe, or not an Apache rewrite file at all (nothing this heal could act on). */ private static function acceptable(string $content): bool { - return self::safe($content) || preg_match('/^\s*RewriteRule\b/m', $content) !== 1; + return self::safe($content) || preg_match('/^\s*RewriteRule\b/m', $content) === 0; } private static function hasPrivateRule(string $content): bool diff --git a/tests/digital-upgrade-480.spec.js b/tests/digital-upgrade-480.spec.js index 3f34588c8..235426d07 100644 --- a/tests/digital-upgrade-480.spec.js +++ b/tests/digital-upgrade-480.spec.js @@ -37,8 +37,8 @@ function getPathAsIs(url) { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'digital-upgrade-480-')); const bodyPath = path.join(dir, 'response'); try { - const status = execFileSync('curl', ['--silent', '--show-error', '--path-as-is', '--max-redirs', '0', - '--output', bodyPath, '--write-out', '%{http_code}', url], { encoding: 'utf8' }).trim(); + const status = execFileSync('curl', ['--silent', '--show-error', '--path-as-is', '--max-redirs', '0', '--max-time', '15', + '--output', bodyPath, '--write-out', '%{http_code}', url], { encoding: 'utf8', timeout: 20000 }).trim(); return { status: Number(status), body: fs.existsSync(bodyPath) ? fs.readFileSync(bodyPath) : Buffer.alloc(0) }; } finally { fs.rmSync(dir, { recursive: true, force: true }); diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index e60e47254..876c01d85 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -259,9 +259,20 @@ ini_set('pcre.jit', '0'); ini_set('pcre.backtrack_limit', '1'); $check(Heal::apply($fixture('0.7.71')) === $fixture('0.7.71'), 'when PCRE gives up, apply() keeps the file as it is, never an empty one'); - Heal::heal($tmpFile, $tmpDir . '/backups'); + $check(!Heal::heal($tmpFile, $tmpDir . '/backups'), 'and heal() reports the file instead of calling it safe'); clearstatcache(); $check((string) file_get_contents($tmpFile) === $fixture('0.7.71'), 'and heal() leaves public/.htaccess untouched on disk'); + + // The 0.7.71 rule is found, but the replace that follows gives up on a later line whose + // trailing blanks make PCRE backtrack: apply() must keep the original, not write null. + $giveUp = $fixture('0.7.71') . 'RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [QSA,L]' . str_repeat(' ', 2000) . "x\n"; + file_put_contents($tmpFile, $giveUp); + ini_set('pcre.backtrack_limit', '1000'); + $check(preg_match('/^([ \t]*)RewriteRule \^uploads\/\(digital\|archives\/documents\|storage\)\/ index\.php \[QSA,L\][ \t]*(?=\r?$)/m', $giveUp) === 1, 'the fixture is recognised under the lowered limit'); + $check(Heal::apply($giveUp) === $giveUp, 'when the replace gives up after the match, apply() keeps the file as it is'); + Heal::heal($tmpFile, $tmpDir . '/backups'); + clearstatcache(); + $check((string) file_get_contents($tmpFile) === $giveUp, 'and heal() neither empties nor rewrites public/.htaccess'); } finally { ini_set('pcre.backtrack_limit', (string) $pcreLimits[0]); ini_set('pcre.jit', (string) $pcreLimits[1]); From 716221c18325ef4667945447a128e426bc0e836d Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 19:48:36 +0000 Subject: [PATCH 20/23] Keep the checked rewrite heal when only the no-gzip replace gives up At the last replace $out is the original or a heal that passed safe(), so a PCRE failure there falls back to it rather than discarding the security fix. --- app/Support/PrivateUploadsRewriteHeal.php | 2 +- tests/htaccess-heal-history-480.unit.php | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/Support/PrivateUploadsRewriteHeal.php b/app/Support/PrivateUploadsRewriteHeal.php index 6d2a8df6e..88ade9222 100644 --- a/app/Support/PrivateUploadsRewriteHeal.php +++ b/app/Support/PrivateUploadsRewriteHeal.php @@ -93,7 +93,7 @@ public static function apply(string $content): string } // The no-gzip list does not depend on the rewrite rules, so a declined heal still // gets PDF and WAV in it. - return preg_replace(self::NOGZIP_OLD, self::NOGZIP_NEW, $out) ?? $content; + return preg_replace(self::NOGZIP_OLD, self::NOGZIP_NEW, $out) ?? $out; } /** diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index 876c01d85..7ff54b1fc 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -273,6 +273,11 @@ Heal::heal($tmpFile, $tmpDir . '/backups'); clearstatcache(); $check((string) file_get_contents($tmpFile) === $giveUp, 'and heal() neither empties nor rewrites public/.htaccess'); + + // Only the no-gzip replace gives up: the rewrite heal already passed safe() and stays. + $noGzipGivesUp = $fixture('0.7.71') . 'SetEnvIfNoCase Request_URI \.(?:gif|jpe?g|png|webp|ico|woff2?|gz|zip|epub|mp4|webm|mp3|m4a|ogg)$ no-gzip dont-vary' . str_repeat(' ', 2000) . "x\n"; + $kept = Heal::apply($noGzipGivesUp); + $check($kept !== $noGzipGivesUp && Heal::safe($kept), 'when only the no-gzip replace gives up, apply() still returns the safe rewrite heal'); } finally { ini_set('pcre.backtrack_limit', (string) $pcreLimits[0]); ini_set('pcre.jit', (string) $pcreLimits[1]); From fc28895fe0988aeb22b5c30a939ad6611b37ddd0 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 23:53:27 +0200 Subject: [PATCH 21/23] test: cover upload rewrite backup and filesystem failures --- tests/htaccess-heal-history-480.unit.php | 36 ++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index 7ff54b1fc..02d06f171 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -287,5 +287,41 @@ @rmdir($tmpDir); } +// Filesystem failures must never replace the operator's only copy of the rules. +$disk = sys_get_temp_dir() . '/htaccess-review-' . bin2hex(random_bytes(5)); +mkdir($disk); +try { + $missing = $disk . '/missing.htaccess'; + $check(Heal::heal($missing, $disk . '/backups') && !file_exists($missing) && !file_exists($disk . '/backups'), + 'absent .htaccess: no empty replacement and no needless backup directory'); + + $plain = "# nginx-only installation\nOptions -Indexes\n"; + file_put_contents($disk . '/plain', $plain); + $check(Heal::heal($disk . '/plain', $disk . '/backups') && file_get_contents($disk . '/plain') === $plain && !file_exists($disk . '/backups'), + 'a file without rewrite rules is accepted byte-for-byte without a backup'); + + $original = $fixture('0.7.71'); + file_put_contents($disk . '/blocked', $original); + file_put_contents($disk . '/not-a-directory', 'occupied'); + $check(!Heal::heal($disk . '/blocked', $disk . '/not-a-directory') && file_get_contents($disk . '/blocked') === $original && glob($disk . '/.pinakes-480-*') === [], + 'backup path occupied by a file: report failure, preserve the rules and leave no temporary file'); + + file_put_contents($disk . '/mode', $original); + chmod($disk . '/mode', 0640); + $healed = Heal::heal($disk . '/mode', $disk . '/backups'); + clearstatcache(true, $disk . '/mode'); + $check($healed && (fileperms($disk . '/mode') & 0777) === 0640 && Heal::safe((string) file_get_contents($disk . '/mode')), + 'atomic replacement keeps the operator\'s 0640 permissions while protecting private files'); + + $backupFiles = glob($disk . '/backups/*') ?: []; + $check(count($backupFiles) === 1 && file_get_contents($backupFiles[0]) === $original, + 'the upgrade backup contains the exact old file, so the operator can restore it'); +} finally { + foreach (glob($disk . '/backups/*') ?: [] as $backup) { unlink($backup); } + if (is_dir($disk . '/backups')) { rmdir($disk . '/backups'); } + foreach (glob($disk . '/*') ?: [] as $file) { unlink($file); } + rmdir($disk); +} + echo "\n" . ($fail === 0 ? "ALL {$pass} PASS" : "{$fail} FAILED, {$pass} passed") . "\n"; exit($fail > 0 ? 1 : 0); From 22d06f7577db23cb9c04939ee2a53ef5cc334ac6 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 21 Oct 2026 00:38:02 +0200 Subject: [PATCH 22/23] Do not read a PCRE give-up in the public rule search as "no public rule" publicRuleBefore() compared preg_match_all() with 0 only. When PCRE gave up on the head of the file (backtrack or JIT limit), the call returned false, the loop ran over nothing and the function answered "no public rule", so safe() called a file it had not read safe: the same fail-open acceptable() had before 23c9df7. A false result now counts as a public rule. The history test puts a line above the private rule whose trailing blanks make the search give up under the lowered limit, checks the search really returns false there, and fails on the previous commit. --- app/Support/PrivateUploadsRewriteHeal.php | 7 ++++++- tests/htaccess-heal-history-480.unit.php | 9 +++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/Support/PrivateUploadsRewriteHeal.php b/app/Support/PrivateUploadsRewriteHeal.php index 88ade9222..bf231f7be 100644 --- a/app/Support/PrivateUploadsRewriteHeal.php +++ b/app/Support/PrivateUploadsRewriteHeal.php @@ -177,7 +177,12 @@ public static function safe(string $content): bool private static function publicRuleBefore(string $content, int $offset): bool { $rule = '/^[ \t]*RewriteRule[ \t]+("[^"\r\n]*"|[^ \t\r\n]+)[ \t]+-(?:[ \t]+\[([^\]\r\n]*)\])?[ \t]*(?=\r?$)/mi'; - if (preg_match_all($rule, substr($content, 0, $offset), $rules, PREG_SET_ORDER) === 0) { + $found = preg_match_all($rule, substr($content, 0, $offset), $rules, PREG_SET_ORDER); + // false is PCRE giving up, not "no rule": a file it could not read is not safe. + if ($found === false) { + return true; + } + if ($found === 0) { return false; } foreach ($rules as $match) { diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index 02d06f171..a82d70dae 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -278,6 +278,15 @@ $noGzipGivesUp = $fixture('0.7.71') . 'SetEnvIfNoCase Request_URI \.(?:gif|jpe?g|png|webp|ico|woff2?|gz|zip|epub|mp4|webm|mp3|m4a|ogg)$ no-gzip dont-vary' . str_repeat(' ', 2000) . "x\n"; $kept = Heal::apply($noGzipGivesUp); $check($kept !== $noGzipGivesUp && Heal::safe($kept), 'when only the no-gzip replace gives up, apply() still returns the safe rewrite heal'); + + // The search for public rules above the private one gives up on a line whose trailing + // blanks make PCRE backtrack: that is no proof of "no public rule", so not safe. + $privateRule = ' RewriteRule ^uploads/(digital|archives/documents|storage)/ index.php [NC,QSA,L]'; + $healed71 = Heal::apply($fixture('0.7.71')); + $publicGivesUp = str_replace($privateRule, ' RewriteRule x -' . str_repeat(' ', 2000) . "y\n" . $privateRule, $healed71); + $check($publicGivesUp !== $healed71 && preg_match_all('/^[ \t]*RewriteRule[ \t]+("[^"\r\n]*"|[^ \t\r\n]+)[ \t]+-(?:[ \t]+\[([^\]\r\n]*)\])?[ \t]*(?=\r?$)/mi', $publicGivesUp) === false, + 'the search for public rules gives up on that file under the lowered limit'); + $check(!Heal::safe($publicGivesUp), 'when the search for public rules gives up, safe() does not call the file safe'); } finally { ini_set('pcre.backtrack_limit', (string) $pcreLimits[0]); ini_set('pcre.jit', (string) $pcreLimits[1]); From 185dd4faa754d6eb6157add0f06cf64f6f5fbf56 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 21 Oct 2026 00:34:12 +0000 Subject: [PATCH 23/23] Read a lower-case RewriteRule as a rewrite rule in acceptable() Apache matches directive names without case, and safe() and conditionPending() already do. acceptable() did not, so a file whose only rule was "rewriterule ^uploads/ - [l]" counted as having no rewrite rules and heal() called it safe without a log line. The history test covers it and fails on the previous commit. --- app/Support/PrivateUploadsRewriteHeal.php | 2 +- tests/htaccess-heal-history-480.unit.php | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/app/Support/PrivateUploadsRewriteHeal.php b/app/Support/PrivateUploadsRewriteHeal.php index bf231f7be..3218032da 100644 --- a/app/Support/PrivateUploadsRewriteHeal.php +++ b/app/Support/PrivateUploadsRewriteHeal.php @@ -228,7 +228,7 @@ private static function missesPrivateFolders(string $pattern): bool /** Safe, or not an Apache rewrite file at all (nothing this heal could act on). */ private static function acceptable(string $content): bool { - return self::safe($content) || preg_match('/^\s*RewriteRule\b/m', $content) === 0; + return self::safe($content) || preg_match('/^\s*RewriteRule\b/mi', $content) === 0; } private static function hasPrivateRule(string $content): bool diff --git a/tests/htaccess-heal-history-480.unit.php b/tests/htaccess-heal-history-480.unit.php index a82d70dae..d796b6e8e 100644 --- a/tests/htaccess-heal-history-480.unit.php +++ b/tests/htaccess-heal-history-480.unit.php @@ -309,6 +309,12 @@ $check(Heal::heal($disk . '/plain', $disk . '/backups') && file_get_contents($disk . '/plain') === $plain && !file_exists($disk . '/backups'), 'a file without rewrite rules is accepted byte-for-byte without a backup'); + // Apache reads directive names without case: a lower-case public rule is still a rule. + $lower = "rewriteengine on\nrewriterule ^uploads/ - [l]\n"; + file_put_contents($disk . '/lower', $lower); + $check(!Heal::heal($disk . '/lower', $disk . '/backups') && file_get_contents($disk . '/lower') === $lower, + 'a lower-case public rule with no private rule is reported, not taken for a file without rewrite rules'); + $original = $fixture('0.7.71'); file_put_contents($disk . '/blocked', $original); file_put_contents($disk . '/not-a-directory', 'occupied');