9/10 Security round 2: retire CKEditor 4 and jQuery 1.11.3 - #643
Open
mcfrank wants to merge 3 commits into
Open
Conversation
…XSS fix)
The study waiver ("Opening Dialog Box") and custom end message are
authored by researchers in a rich-text widget and were rendered to
participants with |safe — so any HTML a researcher stored, including
<script>, onclick handlers, or javascript: links, executed raw in every
parent's browser. A new sanitize_richtext filter runs the stored HTML
through bleach with a safe subset (formatting, links restricted to
http/https/mailto, images) and strips scripts, event handlers, and
dangerous protocols. Applied at the three participant render sites
(waiver modal, standard and CAT completion pages).
This is the parent-facing half of #624; the editor-package replacement
(retiring the EOL CKEditor 4 itself) is tracked separately.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
django-ckeditor bundled CKEditor 4.22, which is end-of-life with known unfixed vulnerabilities, and ckeditor_uploader exposed an arbitrary-file-upload endpoint. Both are removed. The two rich-text study fields (waiver / "Opening Dialog Box" and the custom end message) become plain Textareas that researchers fill with Markdown; on render sanitize_richtext converts the Markdown to HTML and bleaches it (the sanitization added in the previous commit). Content stored earlier as raw HTML passes through Markdown unchanged and is sanitized the same way, so existing waivers still render. The model fields change from RichTextUploadingField to TextField (same underlying text column, so no schema change and no new migration). Historical migrations that imported the ckeditor field classes are rewritten to models.TextField — identical DB representation — so they still import with the package gone. Removes ckeditor/ckeditor_uploader from INSTALLED_APPS, the CKEDITOR_* settings, the ckeditor URL include, and django-ckeditor from requirements. Verified: study form renders textareas with no CKEditor assets; markdown and legacy-HTML both render safely; script/onclick/javascript: stripped; migrations consistent; study-settings and instrument view tests green. Per the chosen option (Markdown over a CKEditor-5 migration). Closes #624. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The site loaded a tangle of jQuery versions: participant pages ran the end-of-life 1.11.3 (2014, known XSS vectors), the researcher and registration bases loaded 1.11.3 AND 3.6.1 (so their JS already ran on 3.x — the 1.11.3 line was dead weight), some pages pulled 3.2.1-slim, and several used scheme-relative //code.jquery.com CDN URLs (an http MITM surface). Everything now loads a single self-hosted jQuery 3.7.1 (latest 3.x; the only removed-in-3 APIs the app used were none — .bind() is merely deprecated). jquery-ui is bumped 1.11.3 -> 1.13.2, which officially supports jQuery 3. The 1.11.3 files are deleted. Verified in the browser on every JS-heavy flow, zero console errors: participant checklist (checkbox toggle + page navigation), background form (bootstrap-datepicker opens, inputmask, selects), researcher console (study selector, table, row-edit modal), and register/login (jquery-ui 1.13.2). URL smoke baseline unchanged. Closes #625. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Continues the stack (targets the correctness branch; merge after #642). Closes #624, #625.
Two end-of-life client dependencies with known vulnerabilities are removed.
CKEditor 4 → Markdown (#624)
django-ckeditor bundled CKEditor 4.22 (EOL, unfixed CVEs) and
ckeditor_uploaderexposed an arbitrary-file-upload endpoint. Both removed.|safe, so researcher-stored HTML (including<script>) executed raw in every parent's browser. A newsanitize_richtextfilter now runs the content through Markdown → bleach at all three render sites. (Worth merging even independent of the editor swap.)RichTextUploadingField→TextField(same text column → no schema change, no new migration). Five historical migrations that imported the CKEditor field classes are rewritten tomodels.TextFieldso they still import with the package gone.jQuery 1.11.3 → 3.7.1 (#625)
The site loaded a tangle: participant pages on EOL 1.11.3, researcher/registration bases double-loading 1.11.3 and 3.6.1, some pages on 3.2.1-slim, several via scheme-relative
//code.jquery.comCDN. All collapsed to a single self-hosted jQuery 3.7.1; jquery-ui bumped 1.11.3 → 1.13.2 (jQuery-3 compatible); 1.11.3 files deleted.Verified live on every JS-heavy flow, zero console errors: participant checklist (checkbox toggle + page navigation), background form (datepicker/inputmask/selects), researcher console (study selector, table, row-edit modal), auth pages.
QA
URL smoke baseline unchanged. Broad non-Selenium suite: 173 tests, the only two errors are pre-existing/flaky and unrelated —
test_study_apiis the pandas-3 bug filed as #640, and a WeasyPrint PDFtest_getthat passes in isolation (parallel-runner flake).🤖 Generated with Claude Code