chore: sync opencraft/ulmo with upstream/release/ulmo (security patches, BB-10886)#842
Merged
Merged
Conversation
The set_course_mode_price view had no authorization check beyond @login_required, meaning any authenticated user could POST to it and rewrite the honor-mode price for any course — a privilege escalation vulnerability. Ideally this endpoint would be removed: it has no known callers in the UI (no templates or JS reference it), no tests, and targets the legacy 'honor' mode. However, it is publicly routed and external consumers may depend on it, so removal requires going through the DEPR process before we can act. In the meantime, this commit closes the security hole regardless of how active the endpoint is. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
validate_timestamp_and_nonce previously returned True unconditionally, allowing any captured LTI launch request to be replayed indefinitely. Now rejects requests whose oauth_timestamp falls outside a ±5-minute window, then atomically records the nonce in the Django cache via cache.add() (OEP-0022 key generation via get_cache_key). A replay returns False immediately because cache.add() only writes when the key is absent. TieredCache is intentionally not used here: it has no atomic add primitive, so a separate get-then-set would leave a race window that defeats the protection. See the updated docstring for details. Documents the requirement for a shared cache backend (Redis or Memcached) in multi-node deployments in both the app and repo READMEs. Fixes GHSA-6gm5-c49g-p3h9 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add missing class docstring to TimestampAndNonceValidatorTest (C0115) - Move time.time patch from class decorator into setUp/addCleanup to eliminate unused mock parameters in every test method (PT019) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The setting annotation used setting_name/setting_default which triggers E7670 (setting-boolean-default-value) for boolean defaults. This was fixed on master using toggle_name/toggle_default annotations (which allow boolean values) but was not backported to ulmo. Fixing here alongside the other style issues. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Bumps the `openedx-forum` pin from 0.3.8 to 0.4.1 on `release/ulmo` to pull in the fix for the "pinned"-NULL sort bug (openedx/forum#270, commit 78b36e4) where discussion threads were not ordering correctly when users selected "recent first" — old threads with NULL `pinned` values floated above newer threads. Reported in: https://discuss.openedx.org/t/discuss-forum-messages-order-not-organized-as-expected-in-teak/18665 The 0.3.8 -> 0.4.1 delta brings in: * 0.3.9 — five small bug fixes (Mongo content_type, timestamps, MySQL read_states lookup, Forum V2 author field, build). * 0.4.0 — MySQL backend query optimizations (N+1 fixes, prefetch/select_related); backwards compatible. * 0.4.1 — the `pinned` NULL sort fix (commit 78b36e4) plus the optional Typesense search backend (additive, off by default). Caps openedx-forum at <=0.4.1 in constraints.txt to avoid: * 0.4.2 — drops Python 3.11 support. * 0.4.3 — removes the MongoDB backend (Ulmo deployments may rely on it). The constraint should be removed on master / post-Ulmo release lines. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The PUT /api/contentstore/v1/videos/{course_id}/download endpoint fetched
every client-supplied files[].url server-side with
requests.get(url, allow_redirects=True) and returned the bytes inside the
ZIP response. Because the URLs were never validated, an authenticated user
with studio read access could point them at internal services or cloud
metadata endpoints and exfiltrate the responses (GHSA-fpf9-9rpr-jvrx).
By design these URLs are always a subset of the course's own VAL
encoded_videos[].url values (the same data the video listing hands the
frontend). Restrict fetches to that allowlist: build the set of legitimate
URLs for the course and reject any request containing a URL outside it
before any HTTP request is made. This eliminates the SSRF rather than
merely narrowing it.
Adds VideoDownloadViewTest (the endpoint previously had no test coverage)
covering the allowed-URL success path, rejection of disallowed URLs without
any outbound request, mixed allowed/disallowed batches, and the non-staff
permission gate.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…B-10886-security-patches
tecoholic
force-pushed
the
tecoholic/BB-10886-security-patches
branch
from
79db610 to
ffae33f
Compare
tecoholic
changed the title
xitij2000
approved these changes
Jun 16, 2026
xitij2000
left a comment
xitij2000
left a comment
Member
There was a problem hiding this comment.
Approved as cherry-picks of existing commits.
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.
Syncs
opencraft/ulmowithupstream/release/ulmovia a merge (same approach as #840), which brings in the three Open edX security-advisory fixes with their original upstream commit SHAs preserved, plus the small follow-up commits that accompany them upstream.Security fixes brought in
241b914a190a92cf25defd93ef5f99set_course_mode_priceendpointOther commits included in the sync
723be600584f15b6d4e0SAML_METADATA_URL_ALLOW_PRIVATE_IPS67ef02d526Important
Do not use anything other than "Create a merge commit" for this PR. And do not use the > "Update with*" button. We want to preserve original commit IDs.
Partially Generated with Claude Code