Conversation
📝 WalkthroughWalkthroughA datacite toast notifications script tag is added to app/index.html with specific attributes (id, data-site, src, async). The script is positioned after the plausible analytics script and before the MathJax configuration block. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/index.html`:
- Line 23: The external script tag with id "datacite-toasts"
(src="https://assets.datacite.org/javascripts/toast_notifications.js") lacks
Subresource Integrity and CORS attributes; compute the SHA-384 SRI hash for that
exact file (e.g., via the provided curl | openssl command), then update the
script element to include integrity="sha384-<computed_hash>" and
crossorigin="anonymous" so the browser can verify the file and fetch it
anonymously.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
app/index.html
| <link integrity="" rel="stylesheet" type="text/css" href="{{rootURL}}assets/bracco.css"> | ||
|
|
||
| <script defer data-domain="doi.datacite.org" src="https://plausible.io/js/script.js"></script> | ||
| <script id="datacite-toasts" data-site="fabrica" src="https://assets.datacite.org/javascripts/toast_notifications.js" async></script> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine app/index.html
find . -name "index.html" -path "*/app/*" | head -5Repository: datacite/bracco
Length of output: 75
🏁 Script executed:
# Check the content around line 23
if [ -f "app/index.html" ]; then
sed -n '15,30p' app/index.html | cat -n
fiRepository: datacite/bracco
Length of output: 1068
🏁 Script executed:
# Test accessibility and headers of the external script
curl -sSI "https://assets.datacite.org/javascripts/toast_notifications.js" 2>&1 | head -20Repository: datacite/bracco
Length of output: 519
🏁 Script executed:
# Compute SRI hash if accessible
curl -sSL "https://assets.datacite.org/javascripts/toast_notifications.js" 2>&1 | head -c 500Repository: datacite/bracco
Length of output: 559
🏁 Script executed:
# Compute the actual SRI hash for the script
curl -sSL "https://assets.datacite.org/javascripts/toast_notifications.js" | openssl dgst -sha384 -binary | openssl base64 -ARepository: datacite/bracco
Length of output: 201
🏁 Script executed:
# Compute SRI hash using Python
python3 << 'EOF'
import hashlib
import base64
import urllib.request
url = "https://assets.datacite.org/javascripts/toast_notifications.js"
try:
with urllib.request.urlopen(url) as response:
content = response.read()
# Compute SHA384
sha384_hash = hashlib.sha384(content).digest()
sri_hash = base64.b64encode(sha384_hash).decode('utf-8')
print(f"sha384-{sri_hash}")
print(f"Script size: {len(content)} bytes")
except Exception as e:
print(f"Error: {e}")
EOFRepository: datacite/bracco
Length of output: 193
Add integrity protection and crossorigin attribute for the remote script include.
Line 23 loads executable code from a mutable URL without integrity and crossorigin attributes. The asset is served from S3/CloudFront with version tracking but lacks cache-control directives for immutable versioning, allowing unintended upstream script changes to reach clients.
Add integrity with a Subresource Integrity (SRI) hash and crossorigin="anonymous":
Suggested hardening change
- <script id="datacite-toasts" data-site="fabrica" src="https://assets.datacite.org/javascripts/toast_notifications.js" async></script>
+ <script id="datacite-toasts" data-site="fabrica" src="https://assets.datacite.org/javascripts/toast_notifications.js" integrity="sha384-<hash>" crossorigin="anonymous" async></script>Compute the hash with:
curl -sSL "https://assets.datacite.org/javascripts/toast_notifications.js" | openssl dgst -sha384 -binary | openssl base64 -A🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/index.html` at line 23, The external script tag with id "datacite-toasts"
(src="https://assets.datacite.org/javascripts/toast_notifications.js") lacks
Subresource Integrity and CORS attributes; compute the SHA-384 SRI hash for that
exact file (e.g., via the provided curl | openssl command), then update the
script element to include integrity="sha384-<computed_hash>" and
crossorigin="anonymous" so the browser can verify the file and fetch it
anonymously.
There was a problem hiding this comment.
I see that Wendel has already approved this, but I think coderabbit has some good suggestions and I wanted to make a comment on that. We don't use these security measures now, but I am thinking that perhaps we need to think about how we should change what we do to embrace some of these practices. (Not necessarily in this PR).
3 participants
Purpose
Loads the following script to display "Toast Notice" posts published in Wordpress: https://github.com/datacite/assets/blob/master/source/javascripts/toast_notifications.js
closes: https://github.com/datacite/product-backlog/issues/626
Approach
Open Questions and Pre-Merge TODOs
Learning
Types of changes
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Reviewer, please remember our guidelines:
Summary by CodeRabbit