Security: Potential Information Disclosure in Error Handling

[tomaioo]

Summary

Security: Potential Information Disclosure in Error Handling

Problem

Severity: Medium | File: shiny/_error.py:L28

In shiny/_error.py, the ErrorMiddleware catches generic exceptions and prints them to stdout with print("Unhandled error: " + str(e)), then returns a generic 500 message. However, the HTTPException handler returns e.detail directly in the response body without sanitization. If e.detail contains sensitive information, it could be leaked to the client. Additionally, the comment in the code suggests uncertainty about proper logging.

Solution

Sanitize or limit the detail returned in HTTPException responses. Use proper structured logging instead of print statements. Consider logging full exception details server-side while returning generic messages to clients.

Changes

• shiny/_error.py (modified)

[cpsievert]

Thanks for the PR. I want to flag that the threat model described here doesn't hold for this specific code path.

ErrorMiddleware is only ever installed in one place, guarded by if is_pyodide: (shiny/_app.py):

if is_pyodide:
    middleware.append(starlette.middleware.Middleware(ErrorMiddleware))

is_pyodide is "pyodide" in sys.modules (shiny/_shinyenv.py) — i.e. this middleware runs only when the app is executing inside Pyodide/WebAssembly (shinylive), never in a normal server deployment. I checked the full git history of shiny/_app.py and this middleware has been conditional on is_pyodide since it was first introduced; there's no other call site in the codebase.

In Pyodide mode, "server" and "client" are the same browser tab/process for the same user — there's no network boundary for e.detail to cross, so returning it to the "response" isn't disclosing anything to a second party. Also, no code in this repo raises HTTPException with custom detail; the exceptions this middleware catches come from Starlette's own routing (404 "Not Found", 405 "Method Not Allowed"), which are already generic, non-sensitive strings.

Given that, replacing e.detail with a hardcoded "An error occurred" doesn't close a real information-disclosure gap here, but it does regress usability: developers debugging a shinylive app that 404s will now see an opaque message instead of the specific Starlette error. Could you clarify what concrete disclosure scenario this addresses given the Pyodide-only code path? If there isn't one, I'd suggest keeping e.detail as-is for that part of the change.

The print → logger.exception swap in the except Exception branch seems reasonable on its own, though it'd be good to confirm it doesn't change what's visible in the browser console for pyodide/shinylive users (I haven't verified how stdout vs. the logging module surfaces there).