[fix] Render apps in opaque sandboxed iframes - #17409

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Summary

This PR fixes #17402. An iframe sandboxed without allow-same-origin has an opaque origin, so document.cookie throws SecurityError. That exception escaped from getCookie during WebsocketConnection.getSessionTokens and rejected the handshake before new WebSocket, leaving the embed blank. setCookie could throw the same way during usage-stats setup.

getCookie and setCookie now treat a thrown cookie access as unavailable storage. Callers already handle a missing cookie: the handshake sends the host auth token or PLACEHOLDER_AUTH_TOKEN, and upload requests omit X-Xsrftoken. Same-origin reads and writes follow the previous path.

All three expected reviews completed (gpt-5.6-terra-xhigh, claude-opus-5-5-medium, grok-4.7-xhigh). They agree the helper-level guard is the right fix, that server-side XSRF checks stay in place, and that an external browser test is warranted. They differ on whether the missing real-browser test blocks merge, and on how strongly the PR body should say XSRF protection must be disabled. The coverage gap is follow-up external testing. The XSRF sentence should be tightened in the description.

Product Alignment

This is a user-facing bug fix: an embedded app that stayed blank now renders once the document and websocket are allowed to load. It adds no public API, config option, default, or new interaction. specs/AGENTS.md does not require a product spec for a bug fix, and specs/ has no spec for opaque-iframe cookie access. The PR body does not claim approval of a new product decision.

The implementation follows the existing missing-cookie contract (undefined) and the behavior already documented on server.enableXsrfProtection: Streamlit accepts a websocket without a valid XSRF token. File upload and auth-cookie parsing still require _streamlit_xsrf. Returning undefined when the value cannot be read matches "Default Null Over Default Error." Letting an embedded frame degrade to an anonymous session matches "Design for All Platforms." There is no new permanent user-facing surface.

The PR bullet that says XSRF protection must stay off for this embed is addressed under Readability. With protection left on, the app still renders. Uploads receive 403 and st.user stays empty. Turning server.enableXsrfProtection off is what those features need in this embed, which is the same constraint cross-origin embeds already have.

Code Quality

The fix sits in the two cookie helpers, so the websocket handshake, upload and delete requests, and usage-stats setup all pick it up. That matches localStorageAvailable() beside getCookie, which already swallows storage failures. There is no new abstraction or state.

getSessionTokens still reads the cookie after claimHostAuthToken() and still calls resetHostAuthToken() afterward. Because the read no longer throws, a one-time host token is cleared on this path.

Both helpers catch every exception, consistent with localStorageAvailable(). Current callers pass fixed cookie names (_streamlit_xsrf, ajs_anonymous_id).

Performance

No meaningful performance impact. The new try/catch runs on cookie read or write: once per websocket connect, once per upload or delete when XSRF protection is on, and during usage-stats setup. It sits off the per-message, delta, and render hot paths. WebsocketConnection.tsx is unchanged. No extra renders, copies, or repeated scans, and no benchmark is needed.

Test Coverage

Unit tests cover the failure mode at each layer:

• getCookie returns undefined when the read throws.
• setCookie does not throw when the write throws.
• The handshake uses PLACEHOLDER_AUTH_TOKEN, and prefers the host auth token when one is present.
• csrfRequest omits the XSRF header when the read throws, including when csrfEnabled is true.

vi.restoreAllMocks() in the affected afterEach hooks keeps the document.cookie spies from leaking.

There is no Playwright e2e. The PR explains why: an opaque frame cannot load the module scripts unless Access-Control-Allow-Origin is injected, and the static handler does not send that header. The author reports a manual check with that header injected. That is a reasonable substitute for this change.

gpt-5.6-terra-xhigh treated a real-browser regression test as merge-blocking. The other two reviews treat it as follow-up coverage. The unit tests spy on document.cookie and prove the fallback; a real opaque origin is what the external-test recommendation covers. That scenario cannot be added in frontend/utils/src/browser/index.test.ts, and the in-repo harness cannot load the frame's module scripts without a static CORS header this PR does not add. The gap can land in a follow-up.

Backwards Compatibility

Same-origin cookie behavior is unchanged, because document.cookie does not throw there and the helpers follow the previous path. getCookie and setCookie live in private workspace packages and are not public st.* API. Opaque frames that previously failed before new WebSocket now attempt the handshake. No config defaults, CLI flags, or message formats change.

Rendering in that frame still depends on the deployment prerequisites in #17402: server.enableCORS=false (an opaque frame sends Origin: null, which the default CORS check rejects), static assets that send Access-Control-Allow-Origin, and server.enableXsrfProtection=false when uploads or st.user are required.

Security & Risk

The catch leaves server-side checks in place.

• Upload and delete routes still return 403 when X-Xsrftoken is missing or does not match _streamlit_xsrf. Omitting the header when the document cannot read the cookie keeps that check fail-closed.
• starlette_websocket.py accepts the websocket before XSRF validation. A missing token or PLACEHOLDER_AUTH_TOKEN skips parsing _streamlit_user. validate_xsrf_token compares the decoded token with hmac.compare_digest and returns false when either side is missing. server.enableXsrfProtection already documents that a valid XSRF token is not required to open the websocket.
• With server.enableCORS left on, _is_origin_allowed rejects Origin: null. The reproduction in #17402 turns CORS off, which already accepts any origin.

No new dependencies, eval, postMessage handlers, or security-header changes.

External test recommendation

• Recommend external_test: Yes
• Triggered categories: 2. Auth, cookies, CSRF, and identity binding; 3. Websocket handshake and session transport; 10. Client storage behavior
• Evidence:
  • frontend/utils/src/browser/index.ts: getCookie returns undefined when document.cookie throws, including the _streamlit_xsrf read used for the handshake and for upload headers.
  • frontend/lib/src/util/utils.ts: setCookie ignores a failed write, so usage-stats setup keeps running in an opaque frame.
  • frontend/connection/src/WebsocketConnection.test.tsx: a throwing cookie read continues the handshake with PLACEHOLDER_AUTH_TOKEN or the host auth token.
  • frontend/connection/src/DefaultStreamlitEndpoints.test.ts: a throwing cookie read omits X-Xsrftoken even when csrfEnabled is true.
• Suggested external_test focus areas:
  • Chromium iframe with sandbox="allow-scripts" and no allow-same-origin, server.enableCORS=false, server.enableXsrfProtection=false, and Access-Control-Allow-Origin on static assets: the app renders and /_stcore/stream opens.
  • The same embed with XSRF protection left on: the websocket still opens, file upload receives 403, and st.user is not filled from _streamlit_user.
  • A normal same-origin session with XSRF protection on: the handshake still sends the XSRF cookie and uploads still send X-Xsrftoken.
• Confidence: Medium
• Assumptions and gaps: Unit tests spy on document.cookie rather than creating an opaque origin. Category 4 (embedding and iframe boundary) stays clear: the diff leaves postMessage, iframe sandbox/allow, and permissions policy unchanged. Two reviews listed that category because the user scenario is an iframe; the checklist hits are the cookie read, the websocket subprotocol token, and client cookie storage. The author reports a manual opaque-frame check with the static CORS header injected. Chromium local-network access on loopback, called out in #17402, is a separate harness requirement.

Accessibility

No rendered UI, focus, labeling, or keyboard behavior changes. Accessibility impact is not applicable.

Readability

JSDoc on getCookie and setCookie leads with the sandboxed-iframe behavior. Test names state the cookie failure and the expected handshake or header outcome. Smaller comment and assertion notes are left inline.

Title ([fix] Render apps in opaque sandboxed iframes)

The title matches [type] Description, stays within 63 characters, and states the user-visible outcome. One review suggested replacing "opaque" so a changelog line needs less context. The word is the precise origin type, and the description defines it in the first paragraph. The current title can stay.

Description

The opening states the change and why the embed stayed blank. The testing plan explains the missing e2e and the separate static Access-Control-Allow-Origin gate.

The third bullet says XSRF protection must stay off for this embed. That matches the issue's advice for uploads and auth, and it overstates what rendering requires. With protection left on, the app renders and the websocket opens; file uploads are rejected and st.user stays empty. gpt-5.6-terra-xhigh read the current sentence as retaining an existing requirement. The server already accepts the websocket without a valid XSRF token, so the bullet should name the features that still need the cookie.

Proposed rewrite:

Apps in an iframe sandboxed without `allow-same-origin` now open their websocket. That frame has an opaque origin, so `document.cookie` throws `SecurityError` and used to reject the handshake before the app could render.

- `getCookie` treats a failed read as a missing cookie. The handshake sends the host auth token, or the built-in placeholder token when the host did not provide one.
- `setCookie` ignores a failed write, so usage-stats setup keeps running in that frame.
- The frame still cannot store `_streamlit_xsrf`. With XSRF protection enabled, the app renders and the websocket opens, but file uploads are rejected and `st.user` stays empty. Turn `server.enableXsrfProtection` off when those features are needed in this embed.

Recommendations

1. Tighten the PR body's XSRF bullet using the rewrite in Readability, so readers can see that the websocket already connects without the token and that uploads and st.user are what still need the cookie.
2. Follow-up: add a Chromium opaque-iframe check under @pytest.mark.external_test, using the focus areas above. The in-repo Playwright harness cannot load the module scripts in that frame without a static CORS header the server does not send.

Verdict

APPROVED: The cookie helpers stop an opaque sandboxed iframe from rejecting the websocket handshake, server-side XSRF checks stay in place, and the unit tests cover the read, write, handshake, and upload paths. The real-browser gap is follow-up external coverage, and the XSRF wording fix belongs in the PR description.

This is an automated consolidated AI review by grok-4.7-xhigh. Please verify the feedback and use your judgment.

This review also includes 4 inline comment(s) on specific code lines.

Summary

All four expected reviews completed (gpt-5.6-sol-high, claude-opus-5-5-high, gemini-3.8-flash-high, grok-4.7-xhigh). Each approved PR #17409.

The PR fixes #17402. An app in an iframe sandboxed without allow-same-origin has an opaque origin, so reading or writing document.cookie throws SecurityError. That exception rejected connectToWebSocket() inside getSessionTokens before new WebSocket, and the frame stayed blank. getCookie now treats a thrown read as a missing cookie, and setCookie ignores a thrown write. The handshake then sends the host auth token, or the existing PLACEHOLDER_AUTH_TOKEN when the host did not provide one.

Reviewers agreed the change is small, fail-closed, and unit-tested at the helpers, the websocket token, and the upload XSRF header. They differed on PR-description wording, which external-test categories apply, and optional comment edits. Those differences are resolved below. None are merge-blocking.

Product Alignment

This is a user-facing bug fix: the sandboxed app renders and connects where it used to stay blank. It adds no public st.* API, config option, CLI flag, default, or new interaction. specs/AGENTS.md does not require a product spec for bug fixes. No spec already on develop defines this cookie path. The merged st.iframe spec covers embedding content inside a Streamlit app, which is a different surface. The PR body does not claim a separate product approval. The behavior matches the expected result in issue #17402.

One review marked this section not applicable because there is no new API. The observable change is that the embedded app renders. That is user-facing, and it is still a bug fix, so the spec requirement does not apply.

The leftover limits match the server that is already shipped. server.enableXsrfProtection does not require a valid XSRF token to accept the websocket. When the frame cannot store _streamlit_xsrf, the server skips the user cookie, so st.user stays empty, and upload or delete requests that omit X-Xsrftoken are rejected. Turning XSRF protection off for this embed is the same constraint already used for cross-origin frames. The fix adds no permanent API surface. It follows principle 15 (a cookie that cannot be read comes back as missing) and principle 36 (embedded iframes keep working, with cookie-backed features unavailable).

Code Quality

The change stays in the two helpers every cookie caller already uses (WebsocketConnection.getSessionTokens, DefaultStreamlitEndpoints.csrfRequest, and MetricsManager.getAnonymousId). Each try covers only the document.cookie access, which matches localStorageAvailable() in the same module. Name matching stays outside getCookie's try, so an invalid name still throws SyntaxError from String.match. The complexity is proportionate to the bug. Optional comment and naming edits are in the inline comments.

Performance

There is no meaningful performance impact. getCookie and setCookie run on websocket connect, on file upload and delete, and once while usage stats are set up. They sit outside the per-message, delta, and render hot paths in frontend/AGENTS.md. The added work is one property access inside try/catch. WebsocketConnection.tsx is unchanged, so there is no extra decode, copy, or render. Performance coverage is not warranted.

Test Coverage

Reviewers agreed the unit coverage fits the change:

• A throwing read returns undefined.
• A throwing write is ignored.
• The handshake uses PLACEHOLDER_AUTH_TOKEN, and still prefers a host auth token and clears that one-time token.
• With CSRF enabled, the upload request omits the X-Xsrftoken header and withCredentials.

Skipping Playwright is reasonable. An opaque frame cannot load the module scripts unless /static/ sends Access-Control-Allow-Origin, and this PR leaves that header alone. Chromium's local-network-access check also blocks a loopback websocket. The PR records a manual check with the static header injected. A MetricsManager test for the sandboxed path is optional: that path combines these guards with the existing localStorageAvailable() fallback.

One review suggested calling vi.restoreAllMocks() before resetting document.cookie in DefaultStreamlitEndpoints.test.ts. The new test spies only on the getter, and that file already resets the cookie and then restores mocks. The setter still accepts the assignment. That reorder is optional.

Backwards Compatibility

Documents that can read and write cookies keep the previous behavior, including SyntaxError for an invalid cookie name. The helpers are internal. No public API, protocol format, config option, or default changes. The sandbox path changes from an uncaught SecurityError during connect to a connected app.

In a frame that can store neither cookies nor localStorage, usage-stats setup generates a new anonymous id on each load. That path used to throw during startup, so existing sessions are unaffected.

Security & Risk

Reviewers agreed there is no security regression. A thrown cookie read is handled like a missing _streamlit_xsrf cookie, and the sensitive paths fail closed:

• csrfRequest omits X-Xsrftoken and withCredentials. With XSRF protection enabled, upload and delete routes stay rejected.
• The handshake sends the host token or PLACEHOLDER_AUTH_TOKEN. Any client can already send the placeholder. validate_xsrf_token returns false when the supplied token or the cookie is missing, so the server leaves the auth cookie unloaded and st.user empty.
• An opaque origin has no cookie jar, so the browser also withholds _streamlit_xsrf and the auth cookies on the socket. The parent page still cannot read the frame DOM without allow-same-origin.

The server origin check is unchanged. An opaque-origin websocket sends the header Origin: null. _is_origin_allowed treats that value as hostname null and rejects it while server.enableCORS is true (lib/streamlit/web/server/starlette/starlette_websocket.py). Issue #17402's reproduction already passes --server.enableCORS false. That requirement is existing server policy. It should be stated in the PR description so the XSRF bullet does not read as if the socket opens under default CORS. It is not a defect in this diff.

No dependencies, external requests, runtime code execution, postMessage handling, iframe policy, or security headers change.

External test recommendation

• Recommend external_test: Yes
• Triggered categories: 2. Auth, cookies, CSRF, and identity binding; 3. Websocket handshake and session transport; 10. Client storage behavior
• Evidence:
  • frontend/utils/src/browser/index.ts: getCookie treats a thrown document.cookie read as a missing cookie. That read supplies _streamlit_xsrf for the handshake and for upload CSRF.
  • frontend/lib/src/util/utils.ts: setCookie ignores a thrown write, so usage-stats setup continues in a frame that cannot store cookies.
  • frontend/connection/src/WebsocketConnection.test.tsx: a throwing cookie read now finishes connectToWebSocket with PLACEHOLDER_AUTH_TOKEN or the host auth token in the Sec-WebSocket-Protocol list.
  • frontend/connection/src/DefaultStreamlitEndpoints.test.ts: with CSRF enabled, a throwing cookie read omits the X-Xsrftoken header on the upload request.
• Suggested external_test focus areas:
  • Load an externally hosted app in sandbox="allow-scripts" (no allow-same-origin) with server.enableCORS=false, once the static bundle is allowed to load, and confirm the app renders and /_stcore/stream opens.
  • With server.enableXsrfProtection=true, confirm the socket still opens, file upload is rejected, and st.user stays empty.
  • With XSRF protection off, confirm a widget rerun and a file upload complete in that same frame.
  • When the host supplies an auth token, confirm that token is what the handshake sends and that the one-time token is cleared.
  • Regression: a normal same-origin session still sends _streamlit_xsrf on the websocket subprotocol and on the upload X-Xsrftoken header.
• Confidence: Medium
• Assumptions and gaps: jsdom does not implement opaque-origin cookie blocking, so the unit tests mock the throw. The manual note in the PR body is the only real-browser evidence. Existing external embeds use allow-same-origin, so they never hit this path. /static/ still has no Access-Control-Allow-Origin, so a stock external embed can fail while loading scripts before the cookie guard runs. Two reviews also listed category 4 (embedding and iframe boundary). The checklist hits that category when the diff changes host-to-guest messaging, iframe sizing, sandbox or allow attributes, or permissions policy. This diff changes cookie access inside an existing sandbox, so category 4 stays off the list. One review listed category 6 (uploads). Upload routes and csrfRequest are unchanged; a missing cookie already omitted the XSRF header, and a thrown read now takes that same path. Category 2 covers that cookie. The upload checks above are still worth running. External coverage can follow this fix.

Accessibility

No rendered control, name, focus order, or keyboard behavior changes. This section does not apply.

Readability

Line-level comment and naming notes for getCookie, setCookie, and the new websocket test are in the inline comments.

PR title. [fix] Render apps in opaque sandboxed iframes matches the [type] Description form, stays within the length limit, and states the user-visible outcome. One review preferred "cookie-blocked" in place of "opaque". "Opaque" is the accurate term: this origin makes document.cookie throw. A cookie that is blocked or partitioned returns an empty string and already followed the missing-cookie path. Keep the title.

PR description. The body leads with the change, explains the XSRF tradeoff, and says why Playwright is skipped. Two reviews asked for no rewrite. The opening sentence stops at "open their websocket"; the user-visible result is that the app renders. The XSRF bullet says the websocket opens with XSRF protection enabled, and it omits server.enableCORS=false, which an Origin: null handshake needs while CORS protection is on.

Proposed opening sentence:

Apps embedded in an iframe sandboxed without allow-same-origin now render instead of staying blank.

Proposed replacement for the last behavior bullet:

The frame still cannot store _streamlit_xsrf, and its websocket sends Origin: null. To embed this way, set server.enableCORS=false. With XSRF protection on, the app renders and connects, but file uploads are rejected and st.user stays empty; set server.enableXsrfProtection=false if those features are needed.

That description update can land with the PR or in a follow-up. It does not block merge.

Recommendations

1. Add the server.enableCORS=false requirement to the PR description, using the rewrite in Readability, so the XSRF bullet describes the full setup for an opaque-origin embed.
2. Add an @pytest.mark.external_test Chromium scenario for sandbox="allow-scripts" when a hosted fixture can serve the static bundle with the required CORS headers. Scope and gaps are in the external-test section. That coverage can follow this fix.
3. Optional comment, naming, and unit-test tightenings are in the inline comments.

Verdict

APPROVED: All four reviews approve, and the cookie guards fix the blank sandbox while XSRF and origin checks stay fail-closed.

This is an automated AI review consolidated by grok-4.7-xhigh. Please verify the feedback and use your judgment.

This review also includes 7 inline comment(s) on specific code lines.

Auto-approved by AI final review (workflow run).