fix: scope cookies to CHAINLIT_ROOT_PATH for multi-app deployments#2825
fix: scope cookies to CHAINLIT_ROOT_PATH for multi-app deployments#2825gonzalo123 wants to merge 4 commits intoChainlit:mainfrom
Conversation
Fix bug where _cookie_path used CHAINLIT_ROOT_PATH value as an env var name instead of as the path itself. Add path=_cookie_path to all set_cookie/delete_cookie calls that were missing it.
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/chainlit/auth/cookie.py">
<violation number="1" location="backend/chainlit/auth/cookie.py:135">
P2: Legacy auth/state cookies set at `path='/'` are never deleted after scoping to `_cookie_path`, so older root-path cookies can continue to be sent alongside new scoped cookies and be picked up instead, causing stale auth/state after upgrade.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…OT_PATH When upgrading from pre-scoped versions, browsers may still hold old cookies at path="/". Add _delete_legacy_cookies helper that explicitly deletes them when _cookie_path != "/" to prevent stale auth behavior.
There was a problem hiding this comment.
3 issues found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="backend/tests/auth/test_cookie.py">
<violation number="1" location="backend/tests/auth/test_cookie.py:196">
P2: Substring check for `Path=/` will also match `Path=/app1`, so the test can pass even if deletion happens at the scoped path. This weak assertion can miss regressions in legacy cookie deletion behavior.</violation>
</file>
<file name="backend/chainlit/auth/cookie.py">
<violation number="1" location="backend/chainlit/auth/cookie.py:47">
P2: Unconditional root-path legacy cookie deletion can remove same-named cookies from other apps on the same host, causing cross-app session/logout disruption in multi-app deployments.</violation>
<violation number="2" location="backend/chainlit/auth/cookie.py:155">
P2: Legacy root-path cleanup is incomplete: stale removed chunk cookies are only deleted at `_cookie_path`, allowing mixed old/new chunk token reconstruction.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
backend/tests/auth/test_cookie.py
Outdated
| cookie_module._delete_legacy_cookies(response, "access_token") | ||
| set_cookie_header = response.headers.get("set-cookie", "") | ||
| assert "access_token" in set_cookie_header | ||
| assert 'Path=/' in set_cookie_header |
There was a problem hiding this comment.
P2: Substring check for Path=/ will also match Path=/app1, so the test can pass even if deletion happens at the scoped path. This weak assertion can miss regressions in legacy cookie deletion behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/tests/auth/test_cookie.py, line 196:
<comment>Substring check for `Path=/` will also match `Path=/app1`, so the test can pass even if deletion happens at the scoped path. This weak assertion can miss regressions in legacy cookie deletion behavior.</comment>
<file context>
@@ -171,6 +171,32 @@ def test_cookie_path_explicit_overrides_root_path(monkeypatch):
+ cookie_module._delete_legacy_cookies(response, "access_token")
+ set_cookie_header = response.headers.get("set-cookie", "")
+ assert "access_token" in set_cookie_header
+ assert 'Path=/' in set_cookie_header
+ assert 'Max-Age=0' in set_cookie_header
+
</file context>
| assert 'Path=/' in set_cookie_header | |
| parts = [part.strip() for part in set_cookie_header.split(";")] | |
| assert "Path=/" in parts |
| return | ||
| for name in names: | ||
| response.delete_cookie( | ||
| key=name, path="/", secure=_cookie_secure, samesite=_cookie_samesite |
There was a problem hiding this comment.
P2: Unconditional root-path legacy cookie deletion can remove same-named cookies from other apps on the same host, causing cross-app session/logout disruption in multi-app deployments.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/chainlit/auth/cookie.py, line 47:
<comment>Unconditional root-path legacy cookie deletion can remove same-named cookies from other apps on the same host, causing cross-app session/logout disruption in multi-app deployments.</comment>
<file context>
@@ -34,6 +34,20 @@
+ return
+ for name in names:
+ response.delete_cookie(
+ key=name, path="/", secure=_cookie_secure, samesite=_cookie_samesite
+ )
+
</file context>
…assertion - Add _delete_legacy_cookies call when deleting leftover chunk cookies - Fix weak Path=/ substring assertion in test to use exact match
Summary
_cookie_pathusedCHAINLIT_ROOT_PATHvalue as an env var name instead of using it directly as the cookie pathpath=_cookie_pathto allset_cookiecalls that were missing it (access_token,oauth_state,X-Chainlit-Session-id)path=_cookie_pathtoclear_oauth_state_cookie'sdelete_cookiecallProblem
When multiple Chainlit apps are mounted on the same domain under different subpaths (e.g.,
/app1and/app2), their cookies collide because all cookies are set withpath="/". This causes 401 Unauthorized errors when switching between browser tabs running different apps.The root cause is twofold:
_cookie_pathresolution has a bug:os.environ.get(_cookie_root_path, "/")treats the value (e.g.,/app1) as an env var nameset_cookiecalls never passpath=, so cookies always default topath="/"Changes
backend/chainlit/auth/cookie.py: Fix_cookie_pathresolution and addpath=_cookie_pathto allset_cookie/delete_cookiecallsbackend/chainlit/server.py: Import_cookie_pathand use it for theX-Chainlit-Session-idcookiebackend/tests/auth/test_cookie.py: Add 3 tests for_cookie_pathresolution logicBackwards Compatibility
No breaking changes. When
CHAINLIT_ROOT_PATHis not set,_cookie_pathremains"/"— identical to current behavior.Summary by cubic
Scope all Chainlit cookies to the app’s root path to prevent cross-app collisions on the same domain. Also delete legacy root-path cookies to avoid stale auth after upgrading.
Written for commit 7247fe4. Summary will update on new commits.