Add Subresource Integrity value to manifest #238
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.
Hello maintainers,
this is a first draft on how we could alter the manifest format to account for more information than just the
logical path -> digested path
mapping without dropping support for existing manifests.In #56 this was postponed with the option to revisit at a later time. With Rails 8 shipping propshaft by default I feel like it's a good time to open up this topic again.
The need / right place for this?
Is this something you're willing to have within this library?
I would love to see this in here, as it for me is part of "Digest stamping" functionality, where we provide another layer of security that the assets we loaded and processed are in fact the ones we expect for the digest.
One could argue it could live outside, but I fear it would replicate a lot from propshaft to basically take file inputs and build a manifest which only does
file path -> integrity value
mappings.If you are strongly against it, we can stop the discussion here and could at least leave a written trail for anyone else looking for this feature in the future.
Evolving the Manifest
Right now the manifest cannot be evolved at all without some sort of backwards incompatibility. This is because there was never a "namespace" reserved to work in there.
Currently: Simple Manifest
The root of the JSON is directly the
Record<LogicalPath, DigestedPath>
mapping.For the sake of this discussion I will call this the "simple manifest".
We cannot introduce a key on the root-level of JSON because it would be treated as a
LogicalPath
key.Upcoming: Extensible Manifest
What we can do, though, is to alter the value type:
This allows to account for additional values that can come up in the future, so for the sake of this discussion I will call this the "extensible manifest".
Using the new integrity check value
This PR by itself won't be of much help for anyone using propshaft via Rails. So I'd love to hear from you how you envision this to work in combination with Rails and the
javascript_include_tag
/stylesheet_link_tag
helpers.Likely the resolvers would have a way to read (static) or calculate (dynamic) the integrity value for the requested asset. For not this is kept backwards compatible "as-is" and only resolves to the digested file path.
(For my use-case it's fine to have it only on the extensible manifest as we would use this right away. So I don't have opinions here.)
Moving forward with the new manifest format
I was thinking about moving the logic into an actual
Manifest
class to encapsulate format and access for both the simple and extensible manifest, where we internally use the extensible format by mapping theDigestedPath
from the simple manifest to the key of the extensible manifestAssetEntry
, but for my proposal I opted for an "inline change" where you can clearly see where moving pieces are in this approach.For example, we only opt-in into the extensible manifest when we we use the Subresource Integrity Check feature. This will cause the least friction for a transition process, but eventually a breaking semver will be necessary if we want to move everyone to the new format.
Code Refactorings
The whole "with integrity" naming feels off to me, but is clear and on-point for the moment. If we decide on how the different manifests will be called, I'll do another pass on streamlining the naming.