Skip to content

Conversation

@akiva10b
Copy link
Contributor

@akiva10b akiva10b commented Jan 20, 2026

Note

High Risk
Adds a new user-scoped feature flag persisted via a new Django model/migration and uses it to gate chatbot token generation and template injection, touching auth-adjacent and security-sensitive paths. Also introduces new client-side URL bootstrapping logic and updates runtime dependencies, increasing rollout risk if misconfigured.

Overview
Introduces a per-user “experiments” flag stored in a new UserExperimentSettings model (with initial migration) and exposes it via Django admin and the account settings page; profile updates now drop experiments changes unless the user is eligible.

Adds an experimental chatbot integration gated by that flag: new settings (CHATBOT_USER_ID_SECRET, CHATBOT_API_BASE_URL, CHATBOT_USE_LOCAL_SCRIPT), a chatbot_user_token context processor that mints an encrypted, expiring token, and conditional rendering/loading of the <lc-chatbot> widget in base.html.

Enhances the React reader to handle a new sefaria:bootstrap-url event that can initialize panels directly from a URL (including connections/versions/lang params), and tweaks copy handling to ignore selections inside the chatbot element; dependencies are updated (cryptography, gunicorn, setuptools).

Written by Cursor Bugbot for commit f9b93e8. This will update automatically on new commits. Configure here.

@mergify
Copy link

mergify bot commented Jan 20, 2026

🧪 CI Insights

Here's what we observed from your CI run for f9b93e8.

🟢 All jobs passed!

But CI Insights is watching 👀

return {"chatbot_user_token": None, "chatbot_enabled": False}
profile = UserProfile(user_obj=request.user)
if not getattr(profile, "experiments", False):
return {"chatbot_user_token": None, "chatbot_enabled": False}
Copy link

Choose a reason for hiding this comment

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

Chatbot authorization bypass after permission revocation

Medium Severity

The chatbot_user_token context processor checks profile.experiments (MongoDB user preference) but not user_has_experiments() (PostgreSQL admin-controlled permission). Other parts of the codebase correctly use user_has_experiments() to verify access. This means if an admin revokes a user's experiment access via UserExperimentSettings, the chatbot will still be shown because profile.experiments remains True in MongoDB. The context processor needs to verify both the admin permission and user preference.

Fix in Cursor Fix in Web


# Make this unique, and don't share it with anybody.
SECRET_KEY = ''
CHATBOT_USER_ID_SECRET = 'secret'
Copy link

Choose a reason for hiding this comment

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

Missing CHATBOT_API_BASE_URL default causes AttributeError crash

High Severity

The context processor accesses settings.CHATBOT_API_BASE_URL at line 132, but this setting is only added to local_settings_example.py, not to the base sefaria/settings.py. While CHATBOT_USER_ID_SECRET is correctly added to the base settings file, CHATBOT_API_BASE_URL is not. If a deployment has a local_settings.py that doesn't define this setting, the context processor will crash with AttributeError for users with experiments enabled.

Additional Locations (1)

Fix in Cursor Fix in Web

}
this.setState({panels: panels});
if (opts.saveLastPlace !== false) {
this.saveLastPlace(basePanel, 1, hasConnections && this.props.multiPanel);
Copy link

Choose a reason for hiding this comment

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

Wrong panel index passed to saveLastPlace

Medium Severity

In bootstrapTextUrl, basePanel is placed at index 0 in the panels array (const panels = [basePanel]), but saveLastPlace is called with n=1. Inside saveLastPlace, doesPanelHaveSidebar(1) checks this.state.panels[2], which references stale pre-setState state at the wrong index. The openingSidebar parameter masks this in some cases, but when there's no connections panel, the sidebar detection uses incorrect old state. The index argument here needs to be 0 to match the actual position of basePanel.

Additional Locations (1)

Fix in Cursor Fix in Web

from sefaria.utils.util import short_to_long_lang_code
from sefaria.utils.chatbot import build_chatbot_user_token
from sefaria.utils.hebrew import hebrew_parasha_name
from reader.views import render_react_component, _get_user_calendar_params
Copy link

Choose a reason for hiding this comment

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

Unused imports added to context_processors module

Low Severity

Several newly added top-level imports are unused: UserHistorySet, UserWrapper, calendars, short_to_long_lang_code, hebrew_parasha_name, render_react_component, and _get_user_calendar_params. Only UserProfile and build_chatbot_user_token are actually used by the new chatbot_user_token function. Notably, the existing code pattern in this file does a local import from reader.views (line 84) to avoid heavy top-level dependencies; the new top-level from reader.views import ... breaks that convention.

Fix in Cursor Fix in Web

return {"chatbot_user_token": None, "chatbot_enabled": False}
if not CHATBOT_USER_ID_SECRET:
return {"chatbot_user_token": None, "chatbot_enabled": False}
profile = UserProfile(user_obj=request.user)
Copy link

Choose a reason for hiding this comment

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

Expensive UserProfile created on every authenticated page load

Medium Severity

The chatbot_user_token context processor constructs a UserProfile(user_obj=request.user) for every authenticated user on every user-visible page. UserProfile.__init__ executes five MongoDB queries (profile document, followers, followees, blockers, blockees). The vast majority of users won't have experiments enabled, making these queries pure overhead. A lightweight check like user_has_experiments(request.user) (a single PostgreSQL EXISTS query) before creating the UserProfile would avoid this cost for nearly all requests.

Fix in Cursor Fix in Web

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.


# Make this unique, and don't share it with anybody.
SECRET_KEY = ''
CHATBOT_USER_ID_SECRET = 'secret'
Copy link

Choose a reason for hiding this comment

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

Weak default secret passes the emptiness guard

Medium Severity

CHATBOT_USER_ID_SECRET defaults to the string 'secret' in base settings and the helm chart env var fallback. The guard if not CHATBOT_USER_ID_SECRET in the context processor won't catch this because 'secret' is truthy, so tokens would silently be generated with a well-known key if the environment variable is not configured. Compare with SECRET_KEY which defaults to '' (falsy), causing an obvious failure.

Additional Locations (2)

Fix in Cursor Fix in Web

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.

3 participants