Overview

When the compiled JavaScript and CSS code for code components get written to a file, the filename is calculated using \Drupal\Component\Utility\Crypt::hmacBase64 with Settings::getHashSalt() as the key. Whenever we access the files, the filename is re-calculated on-the-fly. Due to relying on the value of Settings::getHashSalt(), the filenames are not stable.

  1. Rotating the hash salt is a security best practice;
  2. Cloning database and files between different environments can happen, and different environments will have different hash salt values.

These scenarios cause the Astro Islands to point at non-existing files.

Proposed resolution

Change the way the filename is calculated or store the calculated filename on the JavaScriptComponent entity instead of calculating it on-demand. — use the config entity's uuid, which is also used for config management/syncing/importing: duplicate UUIDs are refused, and will result in an error like An entity with this UUID already exists but the machine name does not match.

That achieves the same end result, without the per-environment dependency of the hash salt.


More context:

  1. The code for calculating the filenames was added in commit 9d295 in #3498889: ComponentSource plugin for code components.
  2. @longwave suggested if we could "use a more user friendly filename and rely on the Drupal cache busting query string to handle invalidation" while working on #3499933: Storage for CSS shared across in-browser code components (and other use cases in the future), and Wim agreed as a future improvement. Maybe it's time to make that happen?
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

balintbrews created an issue. See original summary.

balintbrews’s picture

Crediting people who participated in the conversation that resulted in this issue.

balintbrews’s picture

Issue tags: +beta blocker
wim leers’s picture

Issue summary: View changes
wim leers’s picture

Component: Theme builder » Component sources
Issue tags: +DX (Developer Experience)
Parent issue: » #3520484: [META] Production-ready ComponentSource plugins

isholgueras made their first commit to this issue’s fork.

isholgueras’s picture

I've pushed a proof of concept of removing the hash by the original id.

I've adapted the tests and expected more things to break but they don't.

  1. I've created a code component named nachojs and added to components. XB created default/files/astro-island/nachojs.js well.
  2. I've modified the nachojs.js file title "nachojs changed title" and it changes well.

I've run all the tests that I've modified and all pass (except for those hash/filename that I expect), but I'll let the CI to run the whole battery.

mglaman’s picture

One thought I had was keeping a hash, but making it a hardcoded salt. So v0, v1, v2, etc. Whenever XB has a breaking change, this could be bumped and an upgrade hook written. No idea _why_ a breaking change could happen, but it is atleast an option to load different files?

isholgueras’s picture

Assigned: Unassigned » wim leers
Status: Active » Needs review
wim leers’s picture

Assigned: wim leers » isholgueras
Status: Needs review » Needs work

This looks surprisingly simple! 🤯

Just one Q. 😊

wim leers’s picture

🏓

isholgueras’s picture

Assigned: isholgueras » wim leers
Status: Needs work » Needs review
wim leers’s picture

Assigned: wim leers » Unassigned
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

This looks simpler & more elegant than I dared hope! 😄

wim leers’s picture

Status: Reviewed & tested by the community » Fixed

Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.