Skip to content

fix(auth): use CHAINLIT_ROOT_PATH value as cookie path; constant-time OAuth state check#2934

Open
devteamaegis wants to merge 1 commit into
Chainlit:mainfrom
devteamaegis:fix/cookie-path-and-state-validation
Open

fix(auth): use CHAINLIT_ROOT_PATH value as cookie path; constant-time OAuth state check#2934
devteamaegis wants to merge 1 commit into
Chainlit:mainfrom
devteamaegis:fix/cookie-path-and-state-validation

Conversation

@devteamaegis

@devteamaegis devteamaegis commented May 24, 2026

Copy link
Copy Markdown

Summary

Two bugs in backend/chainlit/auth/cookie.py.


Bug 1 — Cookie path always "/" when CHAINLIT_ROOT_PATH is set

Root cause: Module-level initialisation used the value of CHAINLIT_ROOT_PATH as an environment variable key:

# Before (buggy)
if _cookie_root_path := os.environ.get("CHAINLIT_ROOT_PATH", None):
    _cookie_path = os.environ.get(_cookie_root_path, "/")
#                                  ^^^^^^^^^^^^^^^^
#  This treats the value (e.g. "/myapp") as a key into os.environ,
#  which always returns the default "/" since "/myapp" is not a valid
#  env var name.

Impact: set_auth_cookie and clear_auth_cookie delete stale cookie chunks using path=_cookie_path. When an app is deployed under a sub-path and CHAINLIT_ROOT_PATH is configured, the path used for deletion is "/" instead of the intended path, so old chunks are never cleared after logout or token rotation.

Fix:

if _cookie_root_path := os.environ.get("CHAINLIT_ROOT_PATH", None):
    _cookie_path = _cookie_root_path  # use the value directly

Bug 2 — Non-constant-time OAuth state comparison

validate_oauth_state_cookie used a regular != comparison:

# Before
if oauth_state != state:
    raise Exception("oauth state does not correspond")

String equality (!=) is not constant-time and can leak information about the expected state value through timing side-channels. While network jitter limits practical exploitability, the fix is trivial: use hmac.compare_digest, which is the standard Python idiom for comparing secrets. An explicit None guard was also added so a missing cookie is rejected before the digest call.

# After
if oauth_state is None or not hmac.compare_digest(oauth_state, state):
    raise Exception("oauth state does not correspond")

Tests

Three new tests in tests/auth/test_cookie.py:

Test Covers
test_cookie_path_uses_chainlit_root_path_value Bug 1: _cookie_path equals CHAINLIT_ROOT_PATH value after reload
test_validate_oauth_state_cookie_rejects_missing_state Bug 2: absent state cookie raises exception
test_validate_oauth_state_cookie_accepts_correct_state Bug 2: matching state does not raise

Summary by cubic

Fixes auth cookie path to use CHAINLIT_ROOT_PATH directly, so cookie cleanup works when the app runs under a sub-path. Switches OAuth state validation to constant-time comparison and rejects missing state cookies.

  • Bug Fixes
    • Use CHAINLIT_ROOT_PATH as the cookie path, ensuring set_auth_cookie/clear_auth_cookie remove stale chunks under sub-paths.
    • Validate OAuth state with hmac.compare_digest and raise when the state cookie is missing.

Written for commit 05e77bf. Summary will update on new commits.

Review in cubic

Two bugs in `backend/chainlit/auth/cookie.py`:

1. **Cookie path computed incorrectly when CHAINLIT_ROOT_PATH is set**

   The module-level initialisation read:

       if _cookie_root_path := os.environ.get("CHAINLIT_ROOT_PATH", None):
           _cookie_path = os.environ.get(_cookie_root_path, "/")

   `os.environ.get(_cookie_root_path, "/")` used the *value* of
   `CHAINLIT_ROOT_PATH` (e.g. `"/myapp"`) as a *key* to look up yet
   another environment variable.  Since `"/myapp"` is never a valid env
   var name the lookup always returned the default `"/"`.  Cookie deletion
   in `set_auth_cookie` and `clear_auth_cookie` therefore used path `"/"`
   instead of the intended root path, leaving stale cookies on clients
   after logout or token rotation when the app is deployed under a
   sub-path.

   Fix: assign `_cookie_path = _cookie_root_path` directly.

2. **Non-constant-time OAuth state comparison**

   `validate_oauth_state_cookie` used `oauth_state != state` (regular
   string equality), which is not constant-time and can leak the expected
   state value via timing side-channels.  Replace with
   `hmac.compare_digest`, which is the standard Python idiom for
   constant-time secret comparison.  Also added an explicit `None` guard
   so a missing state cookie is rejected before the digest comparison.

Tests added:
- test_cookie_path_uses_chainlit_root_path_value
- test_validate_oauth_state_cookie_rejects_missing_state
- test_validate_oauth_state_cookie_accepts_correct_state
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. auth Pertaining to authentication. backend Pertains to the Python backend. bug Something isn't working unit-tests Has unit tests. labels May 24, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 2 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread backend/chainlit/auth/cookie.py
@dokterbob

Copy link
Copy Markdown
Collaborator

@devteamaegis Code looks good, just some linting nitpicks. Nice to finally have the cookie path fixed!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes two issues in Chainlit’s backend auth cookie handling: (1) correctly deriving the cookie deletion path from CHAINLIT_ROOT_PATH, and (2) hardening OAuth state validation by using a constant-time comparison to reduce timing side-channel risk.

Changes:

  • Fix module initialization so _cookie_path uses the value of CHAINLIT_ROOT_PATH (instead of treating it as an env var key).
  • Update validate_oauth_state_cookie to use hmac.compare_digest (and reject missing state cookies) for constant-time comparison.
  • Add unit tests covering the root-path cookie behavior and OAuth state validation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
backend/chainlit/auth/cookie.py Fixes cookie path derivation and makes OAuth state comparison constant-time.
backend/tests/auth/test_cookie.py Adds tests for CHAINLIT_ROOT_PATH cookie path derivation and OAuth state validation behaviors.

Comment on lines +149 to +155
monkeypatch.setenv("CHAINLIT_ROOT_PATH", "/myapp")
monkeypatch.delenv("CHAINLIT_AUTH_COOKIE_PATH", raising=False)
importlib.reload(cookie_module)
assert cookie_module._cookie_path == "/myapp", (
f"Expected _cookie_path to be '/myapp' but got '{cookie_module._cookie_path}'. "
"CHAINLIT_ROOT_PATH value should be used directly as the cookie path."
)
Comment on lines 26 to 30
_cookie_secure = _cookie_samesite == "none"
if _cookie_root_path := os.environ.get("CHAINLIT_ROOT_PATH", None):
_cookie_path = os.environ.get(_cookie_root_path, "/")
_cookie_path = _cookie_root_path
else:
_cookie_path = os.environ.get("CHAINLIT_AUTH_COOKIE_PATH", "/")

@dokterbob dokterbob left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good find but some quite significant fixups suggested by copilot!

@devteamaegis devteamaegis force-pushed the fix/cookie-path-and-state-validation branch from fa04ea9 to 05e77bf Compare June 11, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auth Pertaining to authentication. backend Pertains to the Python backend. bug Something isn't working security size:S This PR changes 10-29 lines, ignoring generated files. unit-tests Has unit tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants