Skip to content

chore(ulmo): backport security patch - #852

Merged
Agrendalath merged 4 commits into
opencraft/ulmofrom
kaustav/bb-11021-security-patches
Jul 20, 2026
Merged

chore(ulmo): backport security patch#852
Agrendalath merged 4 commits into
opencraft/ulmofrom
kaustav/bb-11021-security-patches

Conversation

@kaustavb12

@kaustavb12 kaustavb12 commented Jul 20, 2026

Copy link
Copy Markdown
Member

This backports fixes for GHSA-rv5w-f4r5-h77g and GHSA-6cmm-8875-5pcw to the common Ulmo branch.

Private-ref: BB-11021

holaontiveros and others added 3 commits July 14, 2026 09:44
…openedx#38880)

Replace course_version in the sidebar cache key with a
block_structure_version derived from BlockStructureModel.data_version.
course_version changes eagerly on publish, causing a cache miss before
the block structure is rebuilt — poisoning the cache with stale data for
1 hour. block_structure_version only changes after the async rebuild
completes, so cache misses only occur when fresh data is available.

This is a backport of openedx#38785.

(cherry picked from commit 929815f)

Co-authored-by: Taylor Payne <wgu.taylor.payne@gmail.com>
Discussion thread titles (and other user-controlled context fields —
replier_name, author_name, username) were interpolated raw into
notification.content via `str.format(**context)`. That output is
rendered with Django's `|safe` filter in digest_content.html, which
is included by both the email_digest and batched_email body
templates, so a `<style>` block in a thread title survived into
recipient inboxes as executable CSS on email open — enabling
open-tracking, content spoofing, and phishing.

Escape at the source: in `get_notification_content`, wrap every
context value with `django.utils.html.escape` before
`template.format(**context)`, exempting the two structural keys
(`p`, `strong`) that content_templates use as HTML tag names. This
defends every renderer of `notification.content` in one place.

This is the incomplete-patch companion of GHSA-4xv3-5j4x-q8g4
(CVE-2026-42857), which sanitized the post body via
`clean_thread_html_body()` but did not cover the title path.
Fixes GHSA-rv5w-f4r5-h77g.

(cherry picked from commit 08b719ce41bb369fa0cabbe8d0547124e64c8566)
`startswith` is the wrong primitive for "is target inside directory base":
once a trailing separator drops anywhere along the way, sibling directories
whose names extend base match. We could spot-fix by re-appending the
separator before the check, but `commonpath` makes the directory-boundary
intent explicit and removes the failure mode entirely.

Fixes GHSA-6cmm-8875-5pcw.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@Agrendalath Agrendalath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaustavb12, instead of cherry-picking these commits, could you please change the base of your branch to upstream/release/ulmo? This way, when we use the "Merge pull request" feature, we will preserve the upstream commit hashes. This will allow us to easily synchronize our branch with the upstream one in the near future.

It does not matter much with the Teak branch, since we probably will not support it for too long.

@kaustavb12
kaustavb12 force-pushed the kaustav/bb-11021-security-patches branch from 7017593 to 97de058 Compare July 20, 2026 14:03
@kaustavb12

Copy link
Copy Markdown
Member Author

@Agrendalath I have changed the base of my branch to upstream/release/ulmo as requested. However, this does mean that an unrelated fix that was pushed in upstream recently is also included in this PR, but I am assuming that is what we want to do anyways. Right ?

This way, when we use the "Merge pull request" feature

Did you mean - use merge commit to merge the PR ?

@Agrendalath Agrendalath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaustavb12,

However, this does mean that an unrelated fix that was pushed in upstream recently is also included in this PR, but I am assuming that is what we want to do anyways. Right ?

Definitely - we want to keep our branch in sync with the upstream one.

This way, when we use the "Merge pull request" feature

Did you mean - use merge commit to merge the PR ?

Yep - I was simply referring to the button's text in GitHub.

@Agrendalath
Agrendalath merged commit 7ce3600 into opencraft/ulmo Jul 20, 2026
48 checks passed
@Agrendalath
Agrendalath deleted the kaustav/bb-11021-security-patches branch July 20, 2026 16:12
@kaustavb12

Copy link
Copy Markdown
Member Author

@Agrendalath

Thanks for the excellent suggestions and for merging the PR.

Yep - I was simply referring to the button's text in GitHub.

I just wanted to make sure I wasn't missing some advanced Git feature that I did not know about :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants