Skip to content

refactor(server): scope skew-protection exemption to login form#10164

Merged
mscolnick merged 1 commit into
mainfrom
ms/skew-protection-login-exemption
Jul 16, 2026
Merged

refactor(server): scope skew-protection exemption to login form#10164
mscolnick merged 1 commit into
mainfrom
ms/skew-protection-login-exemption

Conversation

@mscolnick

Copy link
Copy Markdown
Contributor

Only the server-rendered login form legitimately posts without the
server token, so exempt it by path instead of by Content-Type. Adds
tests covering the login exemption and token enforcement for
form-encoded requests.

Copilot AI review requested due to automatic review settings July 13, 2026 17:59
@vercel

vercel Bot commented Jul 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
marimo-docs Ready Ready Preview, Comment Jul 15, 2026 3:39pm

Request Review

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 narrows the SkewProtectionMiddleware exemption so that only the server-rendered login form POST (/auth/login) is allowed to submit without the Marimo-Server-Token header, rather than exempting all application/x-www-form-urlencoded requests. It also adds tests to ensure skew protection is enforced for form-encoded requests while still allowing the login form to function.

Changes:

  • Refactor skew-protection exemption logic to key off the /auth/login path instead of Content-Type.
  • Add test coverage confirming form-urlencoded requests still require a valid skew token.
  • Add a test confirming /auth/login POST is not gated by skew protection.

Reviewed changes

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

File Description
marimo/_server/api/middleware.py Replaces the Content-Type-based exemption with a path-based exemption for /auth/login.
tests/_server/api/test_middleware.py Adds tests covering skew token enforcement for form content types and exemption for the login form POST.

Comment on lines +171 to +178
response = client.post(
"/api/home/running_notebooks",
headers={
**token_header("fake-token", "old-skew-id"),
"Content-Type": "application/x-www-form-urlencoded",
},
content=json.dumps({}),
)
Comment on lines +182 to +189
response = client.post(
"/api/home/running_notebooks",
headers={
**token_header("fake-token"),
"Content-Type": "application/x-www-form-urlencoded",
},
content=json.dumps({}),
)
Comment thread tests/_server/api/test_middleware.py Outdated
Comment on lines +205 to +207
assert response.status_code != 401, (
"skew protection should not gate /auth/login"
)
Comment thread marimo/_server/api/middleware.py Outdated
Comment on lines +133 to +135
# If the login form submission, then skip.
# The login page is a server-rendered plain HTML form and does not
# attach the server token header.

@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.

No issues found across 2 files

Architecture diagram
sequenceDiagram
    participant Client as HTTP Client
    participant Middleware as SkewProtectionMiddleware
    participant App as Application

    Note over Client,App: POST requests to server

    Client->>Middleware: POST /auth/login (no skew token)
    Middleware->>Middleware: Check request method (POST)
    Middleware->>Middleware: Path ends with "/auth/login"? → Yes
    Middleware->>App: Pass through (exempted)
    App-->>Client: Login page response

    Client->>Middleware: POST /api/other (form‑encoded, no valid token)
    Middleware->>Middleware: Check request method (POST)
    Middleware->>Middleware: Path is exempt? → No
    Middleware->>Middleware: Path is /api/kernel/execute? → No
    Middleware->>Middleware: Validate skew token header
    alt Valid token present
        Middleware->>App: Pass through
        App-->>Client: Normal response
    else Token invalid or missing
        Middleware-->>Client: 401 Unauthorized
    end
Loading

Re-trigger cubic

@mscolnick
mscolnick requested a review from dmadisetti July 15, 2026 15:35
Only the server-rendered login form legitimately posts without the
server token, so exempt it by path instead of by Content-Type. Adds
tests covering the login exemption and token enforcement for
form-encoded requests.
@mscolnick
mscolnick merged commit 7758fc1 into main Jul 16, 2026
45 of 60 checks passed
@mscolnick
mscolnick deleted the ms/skew-protection-login-exemption branch July 16, 2026 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants