feat: implement Subresource Integrity (SRI) for CDN resources#2963
feat: implement Subresource Integrity (SRI) for CDN resources#2963crivetimihai merged 6 commits intomainfrom
Conversation
|
The implementation correctly adds Subresource Integrity (SRI) across all external CDN assets and integrates verification into CI. The approach is clean and production-ready: Single source of truth via scripts/cdn_resources.py Verification summary: The PR implementation is working correctly. |
crivetimihai
left a comment
There was a problem hiding this comment.
Solid security enhancement. SRI for all 15 CDN resources is exactly the right approach.
Strengths:
- Clean architecture:
cdn_resources.pyas single source of truth,generate-sri-hashes.pyandverify-sri-hashes.pyas separate scripts load_sri_hashes()withlru_cache(maxsize=1)— file read once per process- CI pipeline includes
sri-verifystep — catches CDN drift on every build - Tailwind JIT compiler excluded with clear comment — correct, SRI doesn't work with dynamic content
- Version pinning: Alpine.js from
@3.x.x→@3.14.1, Chart.js from generic URL to@4.4.1— necessary for SRI - SHA-384 per W3C recommendation
- Airgapped mode unaffected — SRI only added to CDN paths
- Good test coverage for
load_sri_hashes(caching, file not found, invalid JSON, permission error)
Minor observations:
-
load_sri_hashescatches all exceptions (admin.py:274) — while this prevents startup failures, it also silently masks file system issues. Consider logging aterrorlevel instead ofwarningforPermissionErrorcases specifically. -
HTMX URL changed from
https://unpkg.com/htmx.org@1.9.10tohttps://unpkg.com/htmx.org@1.9.10/dist/htmx.min.js— this is the canonical path (the old URL redirects). Good fix. -
MANIFEST.in update (
recursive-include mcpgateway *.json) — needed forsri_hashes.jsonto be included in package distributions. -
Documentation updates in ADR and securing.md are comprehensive and include the update workflow.
LGTM.
|
Thanks @suciu-daniel — SRI for CDN resources is an important security hardening measure. Prevents supply chain attacks via tampered CDN assets. LGTM. |
Implements comprehensive SRI protection for all external CDN resources to prevent tampering and ensure content integrity. Changes: - Pin 15 CDN resources to specific versions (HTMX 1.9.10, Alpine.js 3.14.1, etc.) - Generate SHA-384 integrity hashes for all CDN resources - Add integrity and crossorigin attributes to all CDN script/link tags - Create shared scripts/cdn_resources.py module for DRY configuration - Implement hash generation (scripts/generate-sri-hashes.py) - Implement hash verification (scripts/verify-sri-hashes.py) - Add CI pipeline verification (make sri-verify) - Document SRI in security guide and ADR-0014 - Add Tailwind JIT exclusion comments (dynamic content incompatible with SRI) - Use @lru_cache for efficient hash loading (no global variables) Templates updated: - admin.html: HTMX with canonical /dist/htmx.min.js URL - login.html: Font Awesome with corrected closing tag - change-password-required.html: Font Awesome All 15 CDN resources now protected with cryptographic integrity verification. Closes #2558 Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com>
Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com>
- Add 8 test cases covering all code paths in load_sri_hashes() - Test success case, file not found, invalid JSON, permission errors - Test lru_cache behavior and integration with admin endpoints - Test edge cases: empty file, unicode content - Remove shebang from scripts/cdn_resources.py (library module) Closes #2558 Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com>
- Fix docs referencing wrong file (generate-sri-hashes.py -> cdn_resources.py) - Add *.json to pyproject.toml package-data for wheel builds - Add sri_hashes context to forgot-password and reset-password endpoints - Fix HTMX script tag indentation in admin.html - Use %s logger format instead of f-string in load_sri_hashes - Remove unused monkeypatch parameter from SRI tests - Add Tailwind JIT exclusion comments to forgot-password and reset-password Closes #2558 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
86c4d3f to
d7b69ad
Compare
Review Fixes AppliedRebased onto Bugs Fixed
Style/Consistency Fixes
Verification
|
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
The SRI PR added crossorigin="anonymous" to CDN resources (required by the W3C SRI spec for cross-origin integrity checks). This changed CDN fetch mode from no-cors to cors. Playwright's set_extra_http_headers sends the Authorization header on ALL requests including cross-origin CDN fetches, triggering CORS preflight failures on CDNs that don't whitelist Authorization — blocking Alpine.js, CodeMirror, and other scripts from loading. Fix: use cookie-only auth in _inject_jwt_cookie and _set_admin_jwt_cookie. The JWT cookie alone is sufficient for same-origin page navigation and HTMX requests. Closes #2558 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Review Round 2: RBAC test fix + Makefile sri-verifyRoot cause found for RBAC test failuresThe SRI PR added Playwright's A/B test confirmed: cookie+header = 28 CORS errors, Alpine NOT LOADED; cookie-only = 0 CORS errors, Alpine LOADED. Fixes in this push
Test results after fix
|
* feat: implement Subresource Integrity (SRI) for CDN resources Implements comprehensive SRI protection for all external CDN resources to prevent tampering and ensure content integrity. Changes: - Pin 15 CDN resources to specific versions (HTMX 1.9.10, Alpine.js 3.14.1, etc.) - Generate SHA-384 integrity hashes for all CDN resources - Add integrity and crossorigin attributes to all CDN script/link tags - Create shared scripts/cdn_resources.py module for DRY configuration - Implement hash generation (scripts/generate-sri-hashes.py) - Implement hash verification (scripts/verify-sri-hashes.py) - Add CI pipeline verification (make sri-verify) - Document SRI in security guide and ADR-0014 - Add Tailwind JIT exclusion comments (dynamic content incompatible with SRI) - Use @lru_cache for efficient hash loading (no global variables) Templates updated: - admin.html: HTMX with canonical /dist/htmx.min.js URL - login.html: Font Awesome with corrected closing tag - change-password-required.html: Font Awesome All 15 CDN resources now protected with cryptographic integrity verification. Closes #2558 Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com> * style: add blank line before lru_cache decorator for PEP 8 compliance Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com> * test: add comprehensive test coverage for load_sri_hashes() - Add 8 test cases covering all code paths in load_sri_hashes() - Test success case, file not found, invalid JSON, permission errors - Test lru_cache behavior and integration with admin endpoints - Test edge cases: empty file, unicode content - Remove shebang from scripts/cdn_resources.py (library module) Closes #2558 Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com> * fix: review fixes for SRI implementation - Fix docs referencing wrong file (generate-sri-hashes.py -> cdn_resources.py) - Add *.json to pyproject.toml package-data for wheel builds - Add sri_hashes context to forgot-password and reset-password endpoints - Fix HTMX script tag indentation in admin.html - Use %s logger format instead of f-string in load_sri_hashes - Remove unused monkeypatch parameter from SRI tests - Add Tailwind JIT exclusion comments to forgot-password and reset-password Closes #2558 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * Makefile update Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> * fix(test): remove Authorization header from cookie injection to fix CORS The SRI PR added crossorigin="anonymous" to CDN resources (required by the W3C SRI spec for cross-origin integrity checks). This changed CDN fetch mode from no-cors to cors. Playwright's set_extra_http_headers sends the Authorization header on ALL requests including cross-origin CDN fetches, triggering CORS preflight failures on CDNs that don't whitelist Authorization — blocking Alpine.js, CodeMirror, and other scripts from loading. Fix: use cookie-only auth in _inject_jwt_cookie and _set_admin_jwt_cookie. The JWT cookie alone is sufficient for same-origin page navigation and HTMX requests. Closes #2558 Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> --------- Signed-off-by: SuciuDaniel <Daniel.Vasile.Suciu@ibm.com> Signed-off-by: Mihai Criveti <crivetimihai@gmail.com> Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
Summary
Implements comprehensive Subresource Integrity (SRI) protection for all external CDN resources to prevent tampering and ensure content integrity.
Changes
Infrastructure
scripts/cdn_resources.pymodule as single source of truthscripts/generate-sri-hashes.py)scripts/verify-sri-hashes.py)mcpgateway/sri_hashes.jsonTemplates
Code Quality
@lru_cache(maxsize=1)for efficient hash loading (no global variables)crossorigin="anonymous"for CORS complianceCI/CD
make sri-generateandmake sri-verifytargetsDocumentation
docs/docs/manage/securing.mdwith SRI sectionTesting
All 15 CDN resources verified:
make sri-verify # ✅ All 15 SRI hashes verified successfully!Security Impact
Closes #2558