fix: pin CDN scripts + theme CSS with SRI integrity hashes (closes #19)#20
Merged
fix: pin CDN scripts + theme CSS with SRI integrity hashes (closes #19)#20
Conversation
The dashboard loads three cross-origin assets without integrity
attributes:
highlight.js/11.9.0/styles/vs2015.min.css
highlight.js/11.9.0/highlight.min.js
marked/12.0.1/marked.min.js
Marked.js parses raw markdown and highlight.js runs on every code
block — a swapped CDN payload would execute in our origin with full
DOM access. Subresource Integrity (sha512 hash + crossorigin="anonymous")
makes the browser refuse anything whose hash does not match a
known-good value.
Wrinkle: static/js/app.js used to swap `hljsLink.href` between the
dark (vs2015) and light (github) stylesheets at runtime via a plain
ternary. Adding integrity to the static <link> tag would have made
the runtime swap break — the browser reads the (now-stale) integrity
at fetch time and refuses the new sheet.
Fix: introduced HLJS_THEME_SHEETS const map keyed by theme name,
each entry carrying { href, integrity }. New applyHljsTheme(theme)
helper sets integrity FIRST then href so the browser sees the right
hash when it triggers the new fetch. Both inline ternary call sites
(applyTheme and setWorkspaceMode) now call the helper instead of
duplicating the logic.
All four SHA-512 hashes verified two ways:
1. cdnjs SRI API
gh api 'https://api.cdnjs.com/libraries/<name>/<version>?fields=sri'
2. Re-derived from actual CDN content
curl -sL <url> | openssl dgst -sha512 -binary | base64
→ all four match.
pytest 75/75 OK (no behaviour change in Python code).
Live HTTP smoke: served HTML carries integrity + crossorigin on all
three tags; manual browser verification path documented in PR body.
📝 WalkthroughWalkthroughAdds Subresource Integrity (SRI) attributes to CDN-loaded Highlight.js and Marked.js assets in ChangesSRI Pinning and Theme-Aware Asset Loading
Sequence DiagramsequenceDiagram
participant User
participant App as app.js
participant DOM
participant Browser
participant CDN
rect rgba(240, 100, 100, 0.5)
Note over User,CDN: OLD: Theme swap risk (integrity mismatch)
User->>App: request theme change
App->>DOM: set hljsLink.href only
DOM->>Browser: request stylesheet (wrong integrity)
Browser->>CDN: fetch stylesheet
CDN-->>Browser: stylesheet payload
Browser->>Browser: hash mismatch → reject stylesheet
end
rect rgba(100, 200, 100, 0.5)
Note over User,CDN: NEW: Coordinated integrity + href update
User->>App: request theme change
App->>App: lookup HLJS_THEME_SHEETS[theme]
App->>DOM: applyHljsTheme(theme) — set integrity then href
DOM->>Browser: request stylesheet (correct integrity)
Browser->>CDN: fetch stylesheet
CDN-->>Browser: stylesheet payload
Browser->>Browser: hash matches → apply stylesheet
DOM->>User: theme updated
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Collaborator
|
Nice to have Maintenance — vs2015.min.css URL and hash exist in both static/index.html and HLJS_THEME_SHEETS.dark in static/js/app.js. On a highlight.js version bump, both must stay aligned (comments already point at issue #19 and the cdnjs SRI API). A build step or single generated snippet would remove that drift class; without a bundler, the current cross-referenced comments are a reasonable tradeoff. |
clean6378-max-it
approved these changes
May 6, 2026
Per review feedback on PR #20 — vs2015.min.css URL and SRI hash live in both static/index.html (initial load) and static/js/app.js (HLJS_THEME_SHEETS.dark, runtime swap). On a highlight.js version bump both must update together; the cross-referenced comments warn about it but couldn't enforce it. This test extracts the href + integrity from each file and asserts both pairs match, turning the drift hazard into a checked invariant.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_hljs_theme_consistency.py (1)
30-43: ⚡ Quick winHarden regex matching for exact keys/attributes.
Current patterns can match substrings (for example,
data-hrefordarkMode), which can create false positives as files evolve.Suggested patch
- attr_m = re.search(re.escape(attr) + r'\s*=\s*"([^"]*)"', m.group(0)) + attr_m = re.search( + rf'\b{re.escape(attr)}\b\s*=\s*["\']([^"\']*)["\']', + m.group(0), + ) @@ - block = re.search(re.escape(theme) + r"\s*:\s*\{([^}]*)\}", js, re.DOTALL) + block = re.search( + rf"\b{re.escape(theme)}\b\s*:\s*\{{([^}}]*)\}}", + js, + re.DOTALL, + ) @@ - m = re.search(key + r"\s*:\s*['\"]([^'\"]+)['\"]", body) + m = re.search( + rf"\b{re.escape(key)}\b\s*:\s*['\"]([^'\"]+)['\"]", + body, + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_hljs_theme_consistency.py` around lines 30 - 43, The regexes are too permissive and can match substrings; tighten them by forcing exact key/attribute boundaries: in the HTML attribute lookup (the attr_m search) require a word boundary or non-identifier context before and an equals sign after (e.g. use (?<![A-Za-z0-9_-])attr(?=\s*=) or \battr(?=\s*=)); in _js_theme_entry tighten the block and key matches to avoid substrings by matching the literal object key with boundaries (e.g. use (?<![A-Za-z0-9_-])" + re.escape(theme) + r"(?<![A-Za-z0-9_-])\s*:\s*\{...}" or add \b around theme) and match keys ("href" and "integrity") with lookarounds like (?<![A-Za-z0-9_-])href(?![A-Za-z0-9_-])\s*:\s*['\"]...['\"]; update the regexes in _js_theme_entry and the earlier attr_m search accordingly so only exact keys/attributes are captured.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_hljs_theme_consistency.py`:
- Around line 30-43: The regexes are too permissive and can match substrings;
tighten them by forcing exact key/attribute boundaries: in the HTML attribute
lookup (the attr_m search) require a word boundary or non-identifier context
before and an equals sign after (e.g. use (?<![A-Za-z0-9_-])attr(?=\s*=) or
\battr(?=\s*=)); in _js_theme_entry tighten the block and key matches to avoid
substrings by matching the literal object key with boundaries (e.g. use
(?<![A-Za-z0-9_-])" + re.escape(theme) + r"(?<![A-Za-z0-9_-])\s*:\s*\{...}" or
add \b around theme) and match keys ("href" and "integrity") with lookarounds
like (?<![A-Za-z0-9_-])href(?![A-Za-z0-9_-])\s*:\s*['\"]...['\"]; update the
regexes in _js_theme_entry and the earlier attr_m search accordingly so only
exact keys/attributes are captured.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1a5768ea-cc0e-4864-af73-f3ed638e228d
📒 Files selected for processing (1)
tests/test_hljs_theme_consistency.py
wpak-ai
approved these changes
May 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Problem
Three cross-origin assets in
static/index.htmlloaded without integrity attributes — the dashboard executed whatever the CDN delivered. Marked.js parses raw markdown and highlight.js runs on every code block, so a swapped payload would have full DOM access in our origin.Change
static/index.html—integrity="sha512-…"+crossorigin="anonymous"on all three CDN tags (highlight.js JS + CSS + marked.js).static/js/app.js— runtime theme swap previously rewrotehrefbetweenvs2015.min.cssandgithub.min.cssvia inline ternary. With integrity on the static tag, that would have left a stale hash at the moment of fetch and broken the UI on theme toggle. Replaced both inline sites with a const-map helper:Test plan
All four SHA-512 hashes verified two ways:
gh api 'https://api.cdnjs.com/libraries/<name>/<version>?fields=sri'curl -sL <url> | openssl dgst -sha512 -binary | base64Both methods agree for all four files (vs2015 / github CSS, highlight.min.js, marked.min.js).
End-to-end:
pytest tests/— 75/75 OK (no behaviour change in Python).integrity+crossoriginon all three CDN tags.http://127.0.0.1:5000/.Manual browser verification (DevTools → Console + Network):
github.min.cssfetched fresh with its own hash, page recolours, no SRI error.vs2015.min.cssre-fetches / cached, no SRI error.Forced negative test (optional): mutate one character of any
integrityattr → browser refuses the resource with the distinctiveFailed to find a valid digest in the 'integrity' attribute…console message. Confirms enforcement is real, not silently bypassed.Closes #19.
Summary by CodeRabbit
Bug Fixes
Security
Tests