Serve _upload files with content-disposition: attachment

[masenf]

• All files served from the /_upload endpoint will now trigger download
• PDF files are exempt, but they will always be served with the content-type set
• If an application needs some other behavior, they are encouraged to mount their own StaticFiles instance to handle it.

[codspeed-hq]

Merging this PR will not alter performance

✅ 9 untouched benchmarks

---

_{Comparing masenf/upload-endpoint-headers (5f45343) with main (8fb2fb4)}

[greptile-apps]

Greptile Summary

This PR hardens the /_upload file-serving endpoint by adding a thin ASGI middleware (UploadedFilesHeadersMiddleware) that attaches Content-Disposition: attachment and X-Content-Type-Options: nosniff to every response, preventing browsers from rendering potentially-malicious uploaded files (e.g. HTML/JS) inline. PDF files are exempted from the attachment disposition and instead have their content-type explicitly set to application/pdf.

Key changes:

• _upload.py: New UploadedFilesHeadersMiddleware ASGI middleware class added after the upload handler.
• reflex/app.py: The StaticFiles mount for /_upload is wrapped with the new middleware.
• tests/integration/test_upload.py: New test_uploaded_file_security_headers test validates the XSS-mitigation behaviour end-to-end.
• tests/integration/test_media.py: Extended to test an uploaded PNG served through /_upload, but contains an accidental breakpoint() debug statement that will hang CI.

Issues found:

• P0 – breakpoint() in test_media.py:199 must be removed before merging; it will freeze any automated test run.
• P1 – Duplicate content-type for PDFs – the middleware appends content-type: application/pdf without first removing the header already set by Starlette's StaticFiles, resulting in two content-type headers in the response.
• P2 – Side-effect in @rx.var – the generated_image computed var writes a file to disk on every state read, which is fragile in a test context.

Confidence Score: 3/5

Not safe to merge — a breakpoint() will hang CI and a P1 duplicate-header bug for PDFs needs fixing first.

Two concrete issues block this PR: (1) a breakpoint() debug statement at test_media.py:199 that will freeze any automated test run (P0), and (2) the PDF branch of the middleware appends a second content-type header rather than replacing the existing one, producing an invalid HTTP response for PDF files (P1). The core security logic is sound, but these must be addressed before merging.

tests/integration/test_media.py (breakpoint) and packages/reflex-components-core/src/reflex_components_core/core/_upload.py (duplicate content-type for PDFs)

Vulnerabilities

• XSS mitigation – The PR's primary goal is to prevent stored-XSS via uploaded HTML/JS files. Content-Disposition: attachment and X-Content-Type-Options: nosniff are the correct mitigations and are applied consistently to all non-PDF uploads.
• PDF inline rendering – PDFs are intentionally exempted from the attachment disposition so browsers can render them inline. Ensure this is an acceptable risk for the application's threat model (e.g., malicious PDFs with embedded JavaScript).
• No new secrets or credential handling introduced.

Important Files Changed

Filename	Overview
packages/reflex-components-core/src/reflex_components_core/core/_upload.py	Adds UploadedFilesHeadersMiddleware that injects Content-Disposition: attachment and X-Content-Type-Options: nosniff on all upload responses, with a PDF carve-out — but the PDF branch appends a duplicate content-type header instead of replacing the existing one.
reflex/app.py	Wraps the StaticFiles mount for /_upload with the new UploadedFilesHeadersMiddleware — minimal, correct one-liner change.
tests/integration/test_media.py	Adds a test for the uploaded-image flow but contains an accidental breakpoint() on line 199 that will hang CI, and a computed-var with file I/O side effects.
tests/integration/test_upload.py	Adds test_uploaded_file_security_headers that uploads a malicious HTML file and verifies Content-Disposition: attachment / X-Content-Type-Options: nosniff headers; logic is sound.

Sequence Diagram

sequenceDiagram
    participant Browser
    participant UploadedFilesHeadersMiddleware
    participant StaticFiles

    Browser->>UploadedFilesHeadersMiddleware: GET /_upload/file.html
    UploadedFilesHeadersMiddleware->>StaticFiles: forward (scope, receive, send_with_headers)
    StaticFiles-->>UploadedFilesHeadersMiddleware: http.response.start (content-type: text/html)
    Note over UploadedFilesHeadersMiddleware: Appends X-Content-Type-Options: nosniff<br/>Appends Content-Disposition: attachment
    UploadedFilesHeadersMiddleware-->>Browser: http.response.start (modified headers)
    StaticFiles-->>UploadedFilesHeadersMiddleware: http.response.body (file bytes)
    UploadedFilesHeadersMiddleware-->>Browser: http.response.body (unchanged)

    Browser->>UploadedFilesHeadersMiddleware: GET /_upload/doc.pdf
    UploadedFilesHeadersMiddleware->>StaticFiles: forward (scope, receive, send_with_headers)
    StaticFiles-->>UploadedFilesHeadersMiddleware: http.response.start (content-type: application/pdf)
    Note over UploadedFilesHeadersMiddleware: Appends X-Content-Type-Options: nosniff<br/>⚠️ Also appends content-type: application/pdf (duplicate!)
    UploadedFilesHeadersMiddleware-->>Browser: http.response.start (two content-type headers)
    StaticFiles-->>UploadedFilesHeadersMiddleware: http.response.body (file bytes)
    UploadedFilesHeadersMiddleware-->>Browser: http.response.body (unchanged)

Loading

_{Reviews (1): Last reviewed commit: "Serve _upload files with content-disposi..." | Re-trigger Greptile}

[greptile-apps]

Duplicate content-type header for PDF responses

Starlette's StaticFiles already sets a content-type header for .pdf files (it uses Python's mimetypes module, which maps .pdf → application/pdf). By unconditionally appending a second content-type: application/pdf, the response ends up with two content-type headers, which is technically malformed HTTP and can confuse strict proxies or clients.

The intent (force the browser to know this is a PDF so it renders inline) is sound, but the implementation should replace any existing content-type instead of appending to it:

async def send_with_headers(message: MutableMapping[str, Any]) -> None:
    if message["type"] == "http.response.start":
        headers = list(message.get("headers", []))
        headers.append((b"x-content-type-options", b"nosniff"))
        if is_pdf:
            # Replace any existing content-type to ensure correct MIME type.
            headers = [h for h in headers if h[0].lower() != b"content-type"]
            headers.append((b"content-type", b"application/pdf"))
        else:
            headers.append((b"content-disposition", b"attachment"))
        message = {**message, "headers": headers}
    await send(message)