Conversation
`CustomSessionMiddleware` has always accepted `same_site`, but the only construction site never passes it, so the session cookie is always `SameSite=Lax`. That makes marimo unusable in an iframe on another origin: the cookie is third-party there, browsers that restrict third-party cookies drop it, the request that follows is unauthenticated, and marimo redirects to `/auth/login` — a page it serves with `X-Frame-Options: DENY`, so the frame cannot render and there is no recoverable state. `MARIMO_SESSION_COOKIE_SAMESITE` wires the argument through, alongside the existing `MARIMO_SESSION_COOKIE_SECURE`. The default is unchanged (`lax`). `SameSite=None` without `Secure` is rejected outright by browsers, so honouring that pair as given would hand back a cookie that is silently dropped — the same failure this is meant to fix. `CustomSessionMiddleware` therefore implies `Secure` when `same_site="none"`, and says so in a warning. It costs nothing: a cross-site cookie is unusable over plain HTTP regardless. `env_choice` is the enum-valued counterpart to `is_env_true`. An unrecognised value warns and falls back to the default rather than raising, so a typo in a deployment's environment degrades to documented behaviour instead of stopping the server from starting. Found from the marimohub side, where kernels are embedded cross-site under its default `subdomain` exposure: marimo-team/marimohub#328.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Pull Request opener is not an author or co-author of any commit in this PR.
This check is blocked to guard against commits being submitted under a trusted identity the submitter does not control. If this PR is a legitimate cherry-pick, release-engineering submission, or mailing-list-style patch delivery, the repository maintainer can opt out of this check by setting
I have read the CLA Document and I hereby sign the CLA 0 out of 2 committers have signed the CLA. |
| def env_choice(key: str, choices: tuple[T, ...], default: T) -> T: | ||
| """Return the env var `key` constrained to one of `choices`. | ||
|
|
||
| Comparison is case-insensitive and ignores surrounding whitespace. An unset | ||
| variable returns `default`; an unrecognised one warns and returns `default` | ||
| rather than raising, so a typo in a deployment's environment degrades to the | ||
| documented behaviour instead of preventing the server from starting. | ||
| """ | ||
| value = os.environ.get(key) | ||
| if value is None: | ||
| return default | ||
| normalized = value.strip().lower() | ||
| if normalized in choices: | ||
| return cast("T", normalized) | ||
| LOGGER.warning( | ||
| "%s=%r is not one of %s; falling back to %r.", | ||
| key, | ||
| value, | ||
| ", ".join(choices), | ||
| default, | ||
| ) | ||
| return default |
There was a problem hiding this comment.
Could be stringy typed if this check is excessive. Also a fallback when the config could def become a failure ?
| | `MARIMO_SKIP_UPDATE_CHECK` | If set to "1", marimo will skip checking for updates when starting. | Not set | | ||
| | `MARIMO_SQL_DEFAULT_LIMIT` | Default limit for SQL query results. If not set, no limit is applied. | Not set | | ||
| | `MARIMO_SESSION_COOKIE_SECURE` | If set to `true`/`1`, marks the session cookie as `Secure` so browsers only send it over HTTPS. Enable when serving marimo behind TLS. | `false` | | ||
| | `MARIMO_SESSION_COOKIE_SAMESITE` | `SameSite` attribute of the session cookie: `lax`, `strict`, or `none`. Set `none` when marimo is embedded in an iframe on another origin — the cookie is third-party there, and browsers that restrict third-party cookies drop it under `lax`, so token auth can never complete. `none` implies `Secure` and therefore requires HTTPS. | `lax` | |
There was a problem hiding this comment.
i think this is fine - we may want to go with Cookies Having Independent Partitioned State (CHIPS)
which might be more secure than forcing just SameSite=None. Im looking into this more as an option
|
Going to close this. May re-open with a flag for Partition cookies (CHIPS) instead of overriding the same site |
This pull request was authored by a coding agent.
CustomSessionMiddlewarealready acceptssame_site, but the only construction site never passes it — so the session cookie is alwaysSameSite=Lax.That makes marimo unusable in a cross-origin iframe: the cookie is third-party, browsers that restrict third-party cookies drop it, the next request is unauthenticated, and marimo redirects to
/auth/login— which it serves withX-Frame-Options: DENY. Blank frame, no recovery.What this adds:
MARIMO_SESSION_COOKIE_SAMESITE, beside the existingMARIMO_SESSION_COOKIE_SECURE. Default unchanged (lax).One judgement call:
same_site="none"impliesSecure. Browsers rejectNonewithout it, so honouring the pair as given would hand back a silentropped cookie — the exact bug being fixed.Found from marimohub, where kernels are framed cross-site under its default
subdomainexposure: marimo-team/marimohub#328Happy to move this to an issue first if you would rather discuss the shape.