Skip to content

Added a private 'package' media type for zip and 7z files that only Site Administrator and Content API service can download. - #350

Merged
AlexSkrypnyk merged 11 commits into
developfrom
feature/private-packages
Oct 15, 2026
Merged

AlexSkrypnyk merged 11 commits into
developfrom
feature/private-packages

Conversation

@AlexSkrypnyk

@AlexSkrypnyk AlexSkrypnyk commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

A new package media type stores zip and 7z files in field_do_m_package under private://packages/[date:custom:Y]-[date:custom:m], and the new do_package module makes every operation on it - view, download, create, edit, delete, the media listings and the /admin/content/files overview - depend on the access package media permission, which only the civictheme_site_administrator and do_content_api roles hold. Deleting a package deletes its files on the spot (the file_managed row, the usage rows and the file on disk), and so does replacing its file without a new revision or deleting a revision that held the only reference.

A private-scheme file field doesn't restrict anything by itself on this site: core's FileAccessControlHandler grants a private file to whoever can view the media that references it, and media view is the single site-wide view media permission, which the anonymous and authenticated roles both hold, so a logged-out visitor would have been able to download any package. The editorial roles also hold the cross-bundle update any media, delete any media and create media. 2 more gaps sat on the download routes: key_auth isn't a global authentication provider and system.files and system.private_file_download declare no _auth option, so the content API key never authenticated on a download URL, and an anonymous 403 there went out as Cache-Control: max-age=900, public, which Cloudflare caches for .zip and .7z by extension - 1 logged-out request (a chat app unfurling a pasted link, say) could leave a 403 at the edge for authorised users to receive. Deleting the media never removed the file either: file.settings:make_unused_managed_files_temporary is false here, so core leaves an unused file permanent and cron's garbage collection never touches it, and core releases usage only for the default revision's file, so a zip replaced in a new revision kept a usage row pointing at a media item that no longer exists. Meanwhile /admin/content/files listed every package file name to content authors and approvers, who hold access files overview.

After merge, /system/files/packages/... answers 200 to Site Administrators and to the Content API service (with its api-key header) and 403 to content authors, approvers, other authenticated users and anonymous visitors; the anonymous 403 carries Cache-Control: must-revalidate, no-cache, private, the on-disk path under /sites/default/files/private/ is a 404, and /jsonapi/media/package returns an empty data for anyone without the permission. Deleting a package through its delete form, the bulk Delete media action, JSON:API or code removes every file its revisions held unless another entity still uses it, after which the download URL is a 404, and /admin/content/files shows package files only to roles with the permission. Nothing else moves: no theme or front-end change, media of every other type keeps core's delete behaviour (its file stays on disk), no documentation changes, standalone_url stays off, and the built-in administrator role and user 1 keep access by definition.

Before / After

BEFORE  (private file field, nothing gating it)        AFTER  (package media + do_package)
────────────────────────────────────────────────────   ────────────────────────────────────────────────────
GET /system/files/packages/2026-10/a.zip               GET /system/files/packages/2026-10/a.zip
 │                                                      │
 ├─ authenticate                                        ├─ authenticate
 │   └─ _auth unset: global providers only              │   └─ _auth = global providers + key_auth
 │      key_auth not allowed here: api-key gets 403     │      cookie and api-key both resolve an account
 │                                                      │
 ├─ access                                              ├─ access
 │   └─ file inherits its media's view access           │   ├─ PackageAccessHook gate
 │      view media is site-wide: anonymous passes       │   │   no access package media ──▶ forbidden
 │      every role holding view media is allowed        │   └─ otherwise core media and file access decide
 │                                                      │
 └─ respond                                             └─ respond
     └─ any 403: max-age=900, public                        └─ must-revalidate, no-cache, private
        Cloudflare caches .zip and .7z by extension            route no_cache: page cache stores nothing
GET /system/files/packages/2026-10/a.zip

audience                         BEFORE (private file field only)  AFTER (package media + do_package)
───────────────────────────────  ────────────────────────────────  ──────────────────────────────────
anonymous                        200                               403, no-cache, private
authenticated                    200                               403
content author                   200                               403
content approver                 200                               403
Site Administrator               200                               200
Content API service (api-key)    403, api-key refused              200
Delete a package whose zip was replaced once (rev 1 holds a.zip, rev 2 holds b.zip)

BEFORE                                               AFTER
──────────────────────────────────────────────────   ──────────────────────────────────────────────────
media row and both revisions deleted                 hook_media_predelete: PackageFileHook collects
 │                                                    │   a.zip and b.zip from every revision
 ├─ b.zip: usage released                             ├─ media row and both revisions deleted
 │   make_unused_managed_files_temporary: false       │
 │   └─ stays permanent, cron never collects it       └─ hook_media_delete: per file, if no package
 │                                                        revision references it and no other entity
 └─ a.zip: usage row still names the deleted media        records usage ──▶ $file->delete()
                                                          file_managed row, usage rows, file on disk
both zips stay in private://packages/ and keep           both zips gone; download URL is a 404
their file_managed rows indefinitely

Checklist before requesting a review

  • Subject includes ticket number as [#123] Verb in past tense.
  • Ticket number #123 added to description
  • Added context in Changed section
  • Self-reviewed code and commented in commented complex areas.
  • Added tests for fix/feature.
  • Relevant tests run and passed locally.

Changed

  1. web/modules/custom/do_package/src/Hook/PackageAccessHook.php - mediaAccess() and mediaCreateAccess() forbid every operation on a package media, including its creation, for accounts without access package media, and return neutral for everything else. The hook never returns allowed, so core's own operation checks still decide for accounts holding the permission: the Content API service can view published packages and create them through its existing create media, but can't edit or delete them, while Site Administrators manage everything. Unpublishing a package withdraws it from the API.
  2. Same class, queryMediaAccessAlter() (hook_query_media_access_alter) - adds bundle <> 'package' on the base media or media_field_data table for accounts without the permission, so JSON:API collections return an empty data with no omitted UUIDs, and the media overview and media library show no packages to authors and approvers.
  3. Same class, queryFileAccessAlter() (hook_query_file_access_alter) - adds uri NOT LIKE 'private://packages/%' on the base file_managed table for the same accounts, so the Files overview, its usage page and JSON:API file collections skip package files. restrictedBaseTableAlias() holds the account and base table checks both alters share, and PackageConfigTest::testFileDirectory() pins the field's upload directory under PackageAccessHook::DIRECTORY.
  4. web/modules/custom/do_package/src/Hook/PackageFileHook.php - hook_media_predelete collects the file of every revision of a package before core drops the revision rows, then hook_media_delete, hook_media_update and hook_media_revision_delete delete each file that no package revision references and no other entity records usage of. Saving with a new revision keeps the previous file, because the older revision still references it and a revert needs it. Entity hooks rather than a form alter cover the bulk action, JSON:API and code as well as the delete form, with no file permission needed beyond the package's own delete access.
  5. web/modules/custom/do_package/src/Routing/RouteSubscriber.php - sets no_cache on system.files and system.private_file_download. Core's DenyNoCacheRoutes response policy then sends every response on those 2 routes as must-revalidate, no-cache, private, and the page cache never stores any of them. A denied download otherwise carries the public page max-age, so a cache keyed on the URL alone would serve that 403 to allowed accounts.
  6. web/modules/custom/do_content_api/src/Routing/RouteSubscriber.php and do_content_api.services.yml - the subscriber now covers subrequests.front-controller, system.files and system.private_file_download, and takes authentication_collector as an argument. A route with no _auth option is seeded with the global providers before key_auth is added, so Site Administrators keep cookie access to private files while the API key authenticates there too.
  7. config/default/ - the package media type, its field_do_m_package storage (uri_scheme: private) and field (required, directory packages/[date:custom:Y]-[date:custom:m], extensions zip 7z), the default and media_library form and view displays, and do_package in core.extension.yml. media.type.package declares an enforced dependency on do_package, so uninstalling the module removes the type instead of leaving packages readable by everyone holding view media. package isn't added to the civictheme_editorial workflow, because a moderated package would start as a draft the Content API service can't view. The config was built on a running site with a throwaway script and exported with drush cex.
  8. config/default/user.role.civictheme_site_administrator.yml and config/default/user.role.do_content_api.yml - Site Administrator receives access package media plus the per-bundle create package media, edit any package media, edit own package media, delete any package media and delete own package media it holds for every other media type; Content API service receives access package media only.
  9. web/modules/custom/do_base/tests/src/Traits/ExportedConfigTrait.php - loadRolesWithPermission() now holds the role scan that PreviewLinkConfigTest had inline, including the rule that a role flagged is_admin counts as granted, and PackageConfigTest reuses it to pin which roles hold each of the 4 package permissions.
  10. Tests - unit: PackageAccessHookTest (including the file query alter), PackageConfigTest, the do_package and do_content_api RouteSubscriberTest classes and ServiceRolePermissionsTest; kernel: PackageAccessTest runs a real private file through core's file download access and access-checked file queries for Site Administrator, Content API service, content author and anonymous, and PackageFileTest covers deleting a package with 2 revisions, a file still used by another package or module, replacing a file with and without a new revision, revision deletes, and a document bundle whose files survive an update, a revision delete and a delete; both extend the new PackageKernelTestBase. Behat: tests/behat/features/package_media.feature with the package.zip and package.7z fixtures.
  11. Verified locally: the unit suite passes (434 tests), the do_package kernel group passes (38 cases) and package_media.feature passes (20 scenarios) through nginx - the download matrix (anonymous, authenticated, author and approver get 403; Site Administrator and Content API service get 200), an API key download (200), the on-disk path /sites/default/files/private/... (404), an empty anonymous JSON:API collection, .zip and .7z uploads accepted while .jpg is rejected, /admin/content/files hiding package files from authors and approvers, and a package deleted from its delete form leaving no Files entry and a 404 download URL. A manual check returned 200 application/zip with Cache-Control: private for the API key download, and Cache-Control: must-revalidate, no-cache, private for the anonymous 403.

Contrib modules considered before writing the gate:

Module Verdict
media_private_access Per-media-type view permission, but experimental ("NOT READY to be used in production"), no stable release, 6 sites
private_files_download_permission 3.1.3 Per-directory role filter on private downloads; supports Drupal 11 but the project is marked no longer maintained
field_permissions 1.5 Healthy (67k sites) but gates the file field only, so package names, UUIDs and owners stay readable to anonymous over JSON:API and to editors in listings
file_entity_perms Drupal 7 era, needs a file_entity patch, no security coverage
media_entity_download 2.4 Adds a /media/{id}/download route with no access model of its own

Contrib modules considered for deleting the file together with the package:

Module Verdict
media_file_delete 1.3.2 The closest fit (covered, about 20k sites): a checkbox on the media delete and bulk delete forms that calls $file->delete(). Its 2 settings are site-wide, so making it automatic changes every media type; it swaps form classes only, so JSON:API and programmatic deletes leave the file; it reads only the current revision's file; and it needs core's delete any file for a file another user uploaded
file_delete 3.0.3 Replaces the file delete form; marks the file temporary by default, refuses a file in use, and has no media integration
fancy_file_delete 2.0.11 Admin batch tool for deleting files by ID or as orphans, with an open report of in-use files being deleted
auditfiles 4.2.4 Manual reports on the public scheme by default; a file with a stale usage row from an old revision never shows as unused

Screenshots

Add Package form with package.zip uploaded, as a Site Administrator

Summary by CodeRabbit

  • New Features
    • Added private package archives in ZIP and 7z formats, with access limited to accounts granted package access.
    • Site administrators can create and manage package media. The Content API role can access and download packages using API-key authentication.
    • Package files are excluded from public listings and direct public access.
  • Bug Fixes
    • Private package downloads are configured not to be cached.
    • Package files are removed when no package revision or other content uses them.
  • Tests
    • Added coverage for package access, uploads, downloads, file-type restrictions, and file cleanup.

… 'access package media' and to keep private file responses uncached.
…e file system for Site Administrator and Content API service.
…oad routes while keeping their global providers.
…ccessTest' and renamed the injected account in 'PackageAccessHook'.
@AlexSkrypnyk AlexSkrypnyk added the AUTOMERGE Pull request has been approved and set to automerge label Oct 15, 2026
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Team
  • Run ID: ebe54884-65ab-4f8d-b3aa-2df09255bf3e
📥 Commits

Reviewing files that changed from the base of the PR and between f32be4b and 8c6f0a2.

📒 Files selected for processing: 1
  • web/modules/custom/do_package/tests/src/Kernel/PackageFileTest.php

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour.

📝 Walkthrough

This change adds a private package media type for ZIP and 7z files, access controls for package media and downloads, and cleanup for package files no longer in use. It also adds API-key authentication support to selected routes and configuration, kernel, unit, and Behat tests.

Changes: Package media access and lifecycle

Layer / File(s) Summary
Package media configuration
web/modules/custom/do_package/do_package.info.yml, web/modules/custom/do_package/do_package.permissions.yml, config/default/*package*, config/default/field.*.field_do_m_package.yml, config/default/core.extension.yml, config/default/user.role.*.yml, web/modules/custom/do_package/tests/src/Unit/PackageConfigTest.php, web/modules/custom/do_base/tests/src/Traits/ExportedConfigTrait.php, web/modules/custom/do_base/tests/src/Unit/PreviewLinkConfigTest.php
Adds the package media type, private file field, form and display settings, and role permissions. Adds and uses a helper to find exported roles with a permission or administrator status.
Package access and private-file handling
web/modules/custom/do_package/src/Hook/PackageAccessHook.php, web/modules/custom/do_package/src/Routing/RouteSubscriber.php, web/modules/custom/do_package/do_package.services.yml, web/modules/custom/do_package/tests/*, tests/behat/features/package_media.feature
Adds package media and creation access checks, filters package media and files from selected queries for accounts without permission, and sets private-file routes to no-cache. Tests cover access, downloads, queries, visibility, and uploads.
Package file lifecycle
web/modules/custom/do_package/src/Hook/PackageFileHook.php, web/modules/custom/do_package/tests/src/Kernel/PackageFileTest.php, web/modules/custom/do_package/tests/src/Kernel/PackageKernelTestBase.php
Checks package revisions and other entity usage before deleting files that are no longer referenced. Tests cover deletion, replacement, and revision deletion.
API authentication on selected routes
web/modules/custom/do_content_api/do_content_api.services.yml, web/modules/custom/do_content_api/src/Routing/RouteSubscriber.php, web/modules/custom/do_content_api/tests/src/Unit/*, config/default/user.role.do_content_api.yml
Adds key_auth to selected routes without duplicating providers. Uses global authentication providers when _auth is absent. Grants the Content API role package access and updates its permission tests.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~30 minutes

Change: Feature

Merge Risk: ⚪ Minimal · up to 8c6f0

The revision-deletion test exercises the cleanup path and verifies retention and removal. No actionable merge-blocking risk is established by the reviewed evidence.

Pre-merge checks: Passed 5
Passed Passed checks: 5 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely summarizes the main change: a private package media type for ZIP and 7z files with restricted download access for Site Administrators and the Content API service.
Docstring Coverage Passed Docstring coverage is 96.30% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 81 functions across 14 files.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@github-actions

This comment has been minimized.

@codecov-commenter

codecov-commenter commented Oct 15, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.14679% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.54%. Comparing base (9e7037f) to head (8c6f0a2).

Files with missing lines Patch % Lines
...package/tests/src/Kernel/PackageKernelTestBase.php 0.00% 26 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #350      +/-   ##
===========================================
- Coverage    87.23%   85.54%   -1.69%     
===========================================
  Files           29       33       +4     
  Lines          658      761     +103     
===========================================
+ Hits           574      651      +77     
- Misses          84      110      +26     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@AlexSkrypnyk AlexSkrypnyk added the Needs review Pull request needs a review from assigned developers label Oct 15, 2026
@AlexSkrypnyk
AlexSkrypnyk temporarily deployed to PR-350 October 15, 2026 01:10 Inactive
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown

Code coverage (threshold: 80%)

  Classes: 56.25% (18/32)
  Methods: 79.83% (95/119)
  Lines:   88.06% (841/955)
Per-class coverage
Drupal\do_ai_alt_text\AltTextGenerator
  Methods:  85.71% ( 6/ 7)   Lines:  98.72% ( 77/ 78)
Drupal\do_ai_alt_text\Plugin\Action\RegenerateImageAltText
  Methods: 100.00% ( 8/ 8)   Lines: 100.00% ( 63/ 63)
Drupal\do_base\EventSubscriber\PreviewLinkCacheSubscriber
  Methods:  33.33% ( 1/ 3)   Lines:  11.11% (  1/  9)
Drupal\do_base\EventSubscriber\ThemeColorSubscriber
  Methods:  66.67% ( 2/ 3)   Lines:  40.00% (  4/ 10)
Drupal\do_base\Hook\AutomatedListPagerHook
  Methods: 100.00% ( 5/ 5)   Lines: 100.00% ( 31/ 31)
Drupal\do_base\Hook\LibraryInfoAlterHook
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% (  9/  9)
Drupal\do_base\Hook\MetatagsAlterHook
  Methods: 100.00% ( 7/ 7)   Lines: 100.00% ( 58/ 58)
Drupal\do_base\Hook\PageAttachmentsHook
  Methods:  87.50% ( 7/ 8)   Lines:  94.92% ( 56/ 59)
Drupal\do_base\Hook\TwigValidatorRuleInfoAlterHook
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  3/  3)
Drupal\do_base\NavigationScriptHash
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% ( 27/ 27)
Drupal\do_base\Twig\IconAssetsExtension
  Methods:  50.00% ( 1/ 2)   Lines:  33.33% (  1/  3)
Drupal\do_base\Twig\ImageDimensionsExtension
  Methods:  60.00% ( 3/ 5)   Lines:  87.04% ( 47/ 54)
Drupal\do_content_api\EventSubscriber\JsonApiWriteGateSubscriber
  Methods:  66.67% ( 2/ 3)   Lines:  88.89% (  8/  9)
Drupal\do_content_api\Hook\EntityCreateAccessHook
  Methods: 100.00% ( 2/ 2)   Lines: 100.00% ( 12/ 12)
Drupal\do_content_api\Hook\ModerationPolicyHook
  Methods:  50.00% ( 1/ 2)   Lines:  93.75% ( 15/ 16)
Drupal\do_content_api\Routing\RouteSubscriber
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% ( 11/ 11)
Drupal\do_feed\FeedUrlBuilder
  Methods: 100.00% ( 4/ 4)   Lines: 100.00% ( 18/ 18)
Drupal\do_feed\Form\FeedSettingsForm
  Methods: 100.00% ( 4/ 4)   Lines: 100.00% ( 15/ 15)
Drupal\do_feed\Hook\EntityDeleteHook
  Methods:  50.00% ( 1/ 2)   Lines:  92.31% ( 12/ 13)
Drupal\do_feed\Hook\EntityPresaveHook
  Methods: 100.00% ( 4/ 4)   Lines: 100.00% ( 54/ 54)
Drupal\do_feed\Hook\PreprocessParagraphHook
  Methods: 100.00% ( 2/ 2)   Lines: 100.00% ( 14/ 14)
Drupal\do_feed\Hook\PreprocessViewsViewRowRssHook
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  3/  3)
Drupal\do_feed\Hook\ViewsPreViewHook
  Methods:  50.00% ( 1/ 2)   Lines:  96.43% ( 27/ 28)
Drupal\do_generated_content\Generator\CaseMatrix
  Methods: 100.00% ( 3/ 3)   Lines: 100.00% ( 12/ 12)
Drupal\do_generated_content\Generator\RelativeDate
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  1/  1)
Drupal\do_package\Hook\PackageAccessHook
  Methods: 100.00% ( 7/ 7)   Lines: 100.00% ( 27/ 27)
Drupal\do_package\Hook\PackageFileHook
  Methods: 100.00% (11/11)   Lines: 100.00% ( 40/ 40)
Drupal\do_package\Routing\RouteSubscriber
  Methods: 100.00% ( 1/ 1)   Lines: 100.00% (  5/  5)

@AlexSkrypnyk
AlexSkrypnyk merged commit ffc2347 into develop Oct 15, 2026
11 checks passed
@AlexSkrypnyk
AlexSkrypnyk deleted the feature/private-packages branch October 15, 2026 03:33

This branch was successfully deployed

1 active deployment
PR-350 — 8c6f0a28 Deployed Oct 15, 2026 by AlexSkrypnyk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AUTOMERGE Pull request has been approved and set to automerge Needs review Pull request needs a review from assigned developers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants