Media pipeline fixes + Docker security hardening#118
Merged
Conversation
…ials - Image/album deletion resolved originals against <root>/app (dirname depth off by one) so the @Unlink never fired and originals piled up on disk. Deletions now go through UploadService::deleteOriginalIfUnreferenced(), which also refuses to unlink an original still referenced by another images row — originals are shared by design (sha1 dedup + attach). - generateVariantsForImage() could only produce AVIF through Imagick; on web SAPIs without Imagick/vips every AVIF request failed and returned 404 forever. Added a vips attempt and a GD imageavif() fallback (mirroring the existing WebP fallback). - Access-denied media responses (blur/placeholder served under the sharp variant URL) were cacheable `public, immutable`, so browsers kept the blur long after the visitor unlocked the album. They are now `private, no-store`. - VariantMaintenanceService now counts only formats the runtime can actually generate (jxl is CLI-only) — previously every image looked perpetually incomplete and the daily cron re-scanned the whole library on each run. It also reconciles protected-album variants back out of public/media on every daily pass. - CsrfMiddleware no longer adds X-CSRF-Token to image responses: a shared cache could store one visitor's session-bound token and replay it.
- MigrationsTest seeded no plugin_status table, so the 1.4.18 migration (which deactivates the retired bundled plugins there) crashed both apply-in-order and idempotency tests. Seed the pre-1.4.18 shape, assert the retired plugins end up deactivated, and fix the marker-settings assertion (1.4.18 deletes two of the markers 1.2.0 inserted). - PHPStan could not resolve the optional Jcupitt\Vips\Image class used behind class_exists() guards in ImageEngine (4 class.notFound errors). Added a scan-only stub instead of composer-requiring jcupitt/vips, which would drag in ext-ffi.
Fixes every production npm advisory reported against the app bundle: - tinymce 6.8.6 -> 8.7.0 (five XSS advisories); the init now declares license_key 'gpl', required for self-hosted TinyMCE 7+. - swiper 11 -> 14.0.2 (prototype-pollution, critical). The galleries use the stable bundle API (new Swiper + navigation/pagination/thumbs), no call-site changes needed. - lodash / preact patched via npm audit. Assets rebuilt (vite + copy-vendor + FA subset); non-hashed app.css and admin-app.css synced from the manifest. `npm audit --omit=dev` is clean; remaining advisories affect dev-only tooling that never ships.
…aller - Base image php:8.3-apache-bookworm -> php:8.5-apache-trixie plus a dist-upgrade layer: clears the CVE backlog scanners reported against the old imaging libraries (libheif, libde265, openexr, perl). Assets stage moves to Node 22. Imagick builds and serves AVIF/HEIC on 8.5. - The base language packs (storage/translations/*.json) were excluded by .dockerignore AND shadowed by the storage volume, so containers ran with raw translation keys. They are now baked into /usr/share/cimaise/translations and re-seeded by the entrypoint on every boot, which also heals existing volumes on upgrade. - Global ServerName silences Apache's AH00558 startup warning. - The compose file passes the bundled MySQL coordinates to the app as CIMAISE_DB_* variables; installer.php reads them, preselects MySQL and prefills the whole form — no credentials to look up or type. Defaults are overridable via CIMAISE_MYSQL_* in a .env next to the compose file, and the MySQL service still publishes no host port. - First-boot entrypoint banner documents the port mapping (-p 8080:80) and the effective database coordinates until the installer has run.
Contributor
|
Important Review skippedToo many files! This PR contains 238 files, which is 88 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (38)
📒 Files selected for processing (238)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Correctness: - MediaController 304 responses now carry Content-Type: PHP-level header middleware (CsrfMiddleware's image guard) must see the revalidation as an image, or the session-bound X-CSRF-Token gets folded into publicly-cached entries by revalidating shared caches. Denial responses (blur/placeholder under the sharp URL) unified behind withDenialHeaders(), which also adds the X-Robots-Tag the sibling denial paths already sent. - sw.js cache-first now honours Cache-Control: no-store — it used to cache the denial blur under the sharp variant URL and serve it forever, undoing the server-side fix for every PWA client. - The five admin page templates that call tinymce.init() directly now pass license_key 'gpl' (TinyMCE 7+ requirement), matching resources/admin.js. - The installer no longer renders the CIMAISE_DB_PASSWORD into the pre-auth HTML (any visitor could read it during the install window). The password is applied server-side on submit, and only when the submitted host, user and database exactly match the env-provided coordinates — so the fallback cannot be replayed against an attacker-chosen host. The prefill notice also survives a failed first attempt (e.g. MySQL still initializing). - UploadService::canGenerateFormat() now mirrors the real encoder chain, including the CIMAISE_DISABLE_IMAGICK opt-out that raw ImageEngine capabilities ignore; VariantMaintenanceService uses it, so maintenance can never count a format the generator cannot emit. - deleteOriginalIfUnreferenced(): blank legacy paths no longer count as failed deletions, and originals are resolved against the same candidate locations generateVariantsForImage() uses (legacy /media/originals rows). - Docker entrypoint translation seed is checksum-guarded: admin-modified packs (Texts import with save-to-file) are preserved across restarts, seeding failures are reported instead of swallowed, and the awk probe no longer aborts the set -e entrypoint on first boot. Cleanup / performance: - resizeWithGdWebp/resizeWithGdAvif collapsed into one resizeWithGd() (plus AVIF speed 7 — libaom's default is drastically slower for a few percent of size on GD-only hosts). - Protected-album relocation loop extracted to ProtectedMediaStorage::relocateAllProtectedAlbums(), shared by the one-time quarantine and the daily reconciliation. - Migration 1.4.19: indexes on images.original_path and image_variants.path — both were probed per-file in delete loops and the daily reconciliation as full table scans. Schemas + template.sqlite regenerated, version bumped to 1.4.19.
This was referenced Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
Deep audit of the upload → thumbnail → serving pipeline, plus security hardening of the Docker image after a scanner reported 307 CVEs against
fabiodalez/cimaise:latest.Media pipeline fixes (
fix(media))<root>/app/storage/...(wrongdirnamedepth) so the@unlinksilently no-oped. I also discovered originals are shared between images (sha1 dedup on upload + attach copyingoriginal_path, up to 23 rows per file in my data), so a naive path fix would have caused data loss. Deletions now go throughUploadService::deleteOriginalIfUnreferenced(), which unlinks only when no survivingimagesrow references the file.imageavif()fallback, mirroring the WebP fallback. Verified live: a missingmd.avifnow generates on demand under GD.public, immutable: the blur/placeholder served under the sharp variant URL stuck in the browser for up to 7 days after unlocking a protected album. Denial responses are nowprivate, no-store.jxl(CLI-only) as an expected format, so every image looked incomplete and the whole library was re-scanned daily. It now counts only formats the runtime can generate, and reconciles protected-album variants back out ofpublic/mediaon every pass.X-CSRF-Tokenremoved from image responses — a shared cache could store one visitor's session-bound token and replay it to others.Dependency security (
build(deps))license_key: 'gpl'added to the init as required from v7).npm audit --omit=devandcomposer auditare both clean; the remaining npm advisories are dev-only tooling (vite/esbuild dev server) that never ships.Docker hardening (
feat(docker))php:8.3-apache-bookworm→php:8.5-apache-trixie+ adist-upgradelayer: the flagged system packages are now libheif 1.19.8, libde265 1.0.15, openexr 3.1.13, perl 5.40. Node stage 18 → 22. Imagick builds fine on PHP 8.5 (AVIF + HEIC verified in the container)..dockerignoreand shadowed by the storage volume. They are now baked into the image and re-seeded by the entrypoint on every boot, which also heals existing volumes on upgrade.ServerNamekills the AH00558 startup warning.CIMAISE_DB_*env vars and the installer preselects MySQL with every field prefilled. Defaults are overridable viaCIMAISE_MYSQL_*in a.envnext to the compose file; the MySQL service publishes no host port. A first-boot banner documents the port mapping and the effective credentials until the installer runs.Testing
MigrationsTesterrors introduced with the 1.4.18 migration).Note: the published image only picks these fixes up with the next release tag (
v*→ docker-publish workflow).