[test] Guard server cookie names against Community Cloud cookie filtering#15474
[test] Guard server cookie names against Community Cloud cookie filtering#15474sfc-gh-kmcgrady wants to merge 3 commits into
Conversation
…ring The Streamlit Community Cloud proxy only forwards an allowlist of known cookies to the app; any cookie not on it is stripped before reaching the app. A newly added cookie therefore works locally but silently breaks on Community Cloud, which is what broke st.login via the _streamlit_session cookie. Add a guard test that fails when the set of server cookie names changes, plus a comment at the cookie definitions, so the allowlist requirement is caught at PR time instead of in production.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
✅ PR preview is ready!
|
kmcgrady
added
impact:internal
The guard previously listed the cookie constants by hand, so it only caught renames -- a newly added *_COOKIE_NAME constant would not appear in the actual set and the test would still pass. Discover every *_COOKIE_NAME constant from the module instead, and document the convention at the definitions.
|
| Filename | Overview |
|---|---|
| lib/streamlit/web/server/starlette/starlette_server_config.py | Adds an explanatory comment block documenting the Community Cloud cookie allowlist requirement; no functional changes. |
| lib/tests/streamlit/web/server/starlette/starlette_server_config_test.py | New test file: discovers all *_COOKIE_NAME constants in the module via vars() and asserts they match an explicit expected set, guarding against unallowlisted cookies slipping through on Community Cloud. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Developer adds a cookie in starlette_server_config.py] --> B{Uses COOKIE_NAME suffix?}
B -->|Yes| C[Guard test discovers it via vars]
B -->|No| G[Guard misses it - covered by docstring convention]
C --> D{Name in expected_cookie_names?}
D -->|Yes| E[Test PASSES]
D -->|No| F[Test FAILS - prompts dev to update Cloud allowlist first]
F --> H[Dev confirms allowlist updated in Cloud infra repo]
H --> I[Update expected_cookie_names and merge]
Reviews (2): Last reviewed commit: "Reword cookie-allowlist comment to remov..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a regression guard to prevent new/renamed Starlette server cookies from silently breaking on Streamlit Community Cloud due to proxy cookie allowlisting, by pinning the expected set of server cookie names in a unit test and documenting the convention in the server config module.
Changes:
- Added a unit test that discovers all
*_COOKIE_NAMEconstants instarlette_server_configand asserts they match an explicit expected set. - Documented the “define cookie names here + ensure Cloud proxy allowlist” requirement next to the cookie-name constants.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| lib/tests/streamlit/web/server/starlette/starlette_server_config_test.py | Adds a guard test that fails when server cookie names change, prompting Cloud allowlist verification. |
| lib/streamlit/web/server/starlette/starlette_server_config.py | Adds an inline comment documenting the cookie-name convention and the Community Cloud proxy constraint. |
| assert actual_cookie_names == expected_cookie_names, ( | ||
| "Server cookie names changed. Ensure the new cookie is allowlisted by " | ||
| "the Streamlit Community Cloud proxy before updating this test, or it " | ||
| "will be stripped on Cloud." | ||
| ) |
There was a problem hiding this comment.
Verified this isn't the case with pytest 9.x: a custom message and the set-diff introspection both appear on failure (e.g. Extra items in the left set: '_streamlit_foo'). The message isn't suppressing the diff — it adds the actionable allowlist guidance on top of it, so keeping it. Thanks!
Describe your changes
The Streamlit Community Cloud proxy only forwards an allowlist of known cookies to the app; any cookie not on that allowlist is stripped from the request before it reaches the app pod. A newly added server cookie therefore works locally but silently breaks on Community Cloud — which is exactly what happened to
st.loginonce the OAuth handshake started relying on the_streamlit_sessioncookie (issue #15455).This PR adds a lightweight regression guard so the coupling is caught at PR time instead of in production:
starlette_server_config_test.py): asserts the set of server cookie names matches an explicit expected set. Adding or renaming a cookie fails the test with a message telling the developer to ensure the new cookie is allowlisted by the Community Cloud proxy first.starlette_server_config.pydocumenting the requirement.No behavior change — this is test/documentation only. The actual Community Cloud allowlist update for
_streamlit_sessionis handled separately in the Cloud infra repo.Cookie inventory (current Starlette server)
_streamlit_user_streamlit_user_tokens_streamlit_xsrf_streamlit_sessionGitHub Issue Link (if applicable)
Related to #15455
Testing Plan
test_server_cookie_names_match_cloud_allowlistpasses; lint/format clean.