Summary

This PR updates static file serving to use native MIME type detection instead of an extension allowlist, and aligns Tornado and Starlette behavior via guess_content_type(). It also updates Tornado unit tests for additional extensions (.html, .css) and JavaScript MIME handling.

Code Quality

The implementation is generally clean and improves consistency between server stacks. I found two issues that should be addressed before merge:

• lib/streamlit/web/server/starlette/starlette_routes.py (around lines 852-855): behavior changed to always use guess_content_type(safe_path) for /app/static responses, but there is no corresponding Starlette endpoint test coverage in lib/tests/streamlit/web/server/starlette/starlette_routes_test.py (current tests cover helper functions and cookie/CORS helpers, not /app/static content-type behavior).
• e2e_playwright/__snapshots__/linux/st_time_input_test/st_time_input-dropdown-custom-theme[chromium].png (binary file, no line numbers): this appears unrelated to static file MIME changes and introduces unrelated visual baseline churn in a backend-focused PR.

Test Coverage

Coverage is improved for Tornado static file serving in lib/tests/streamlit/web/server/app_static_file_handler_test.py (notably around lines 141-184), including checks for JS/HTML/CSS MIME types and nosniff.
However, test coverage is incomplete for this change set:

• No Starlette route test verifies /app/static content-type behavior after the logic change in starlette_routes.py.
• No test currently validates parity between Tornado and Starlette for representative extensions (e.g., .js, .html, .css, unknown extension/no extension).

Backwards Compatibility

This is an intentional behavior change: files that previously defaulted to text/plain (e.g., .js, .html, .css) now return native MIME types. That should help legitimate use cases, but it can change runtime behavior for apps that depended on the old forced-plain-text semantics.

Security & Risk

Risk is moderate and mostly around behavioral surface area:

• Serving native MIME types for HTML/JS increases executable content exposure compared to forced text/plain (even though this is intentional and static serving is config-gated).
• X-Content-Type-Options: nosniff is still set (good), but server-stack parity should be explicitly tested to reduce regression risk between Tornado and Starlette implementations.
• The unrelated snapshot delta adds avoidable review and regression noise.

Accessibility

No frontend code paths were modified, so there are no direct accessibility changes to assess in this PR.

Recommendations

1. Add Starlette tests for /app/static content types and security headers (at least .js, .html, .css, and unknown/no extension) in lib/tests/streamlit/web/server/starlette/starlette_routes_test.py.
2. Remove the unrelated snapshot file change from this PR, or include the corresponding e2e test-context changes and rationale if it is truly required.

Verdict

CHANGES REQUESTED: The core implementation is close, but missing Starlette-side behavioral tests and an unrelated snapshot binary change make this not ready to merge yet.

This is an automated AI review by gpt-5.3-codex-high. Please verify the feedback and use your judgment.

Summary

This PR removes the SAFE_APP_STATIC_FILE_EXTENSIONS allowlist that previously forced non-allowlisted file types (e.g., .js, .html, .css) to be served as text/plain from the /app/static/ endpoint. Static files are now served with their natural content types using the existing guess_content_type() utility. Both the Tornado (AppStaticFileHandler) and Starlette (_app_static_endpoint) handlers are updated for consistent behavior.

Key changes:

• Removed the SAFE_APP_STATIC_FILE_EXTENSIONS tuple and Path import from app_static_file_handler.py
• The Tornado handler now overrides get_content_type() to use the shared guess_content_type() instead of relying on set_extra_headers() to force text/plain
• The Starlette handler passes guess_content_type() directly as media_type to FileResponse
• MAX_APP_STATIC_FILE_SIZE is now annotated with Final
• Unit tests updated to verify correct content-type headers for .js, .html, .css, and other extensions
• Unrelated snapshot update (st_time_input-dropdown-custom-theme) likely from merge with develop

Code Quality

The code is clean, well-structured, and follows existing patterns in the codebase.

Positives:

• Good reuse of the existing guess_content_type() from component_file_utils.py, avoiding code duplication between Tornado and Starlette handlers.
• The noqa: ARG002 annotation on set_extra_headers is appropriate since path is now unused but required by the Tornado interface, and the accompanying comment explains why.
• Adding Final to MAX_APP_STATIC_FILE_SIZE is a nice cleanup.
• Clean removal of unused from pathlib import Path imports in both files.

Minor observation:

• In app_static_file_handler.py:83-86, the get_content_type() override uses self.absolute_path or "" as a defensive guard. Tornado's own get_content_type() does assert self.absolute_path is not None instead. Since get_content_type is only called after validate_absolute_path sets self.absolute_path, the or "" guard is safe but will silently fall back to application/octet-stream rather than failing loudly if something unexpected happens. This is a minor stylistic choice and acceptable.

Test Coverage

The test updates are appropriate and cover the behavioral change well:

• test_static_files_200 was rewritten to verify that .js files now get a JavaScript content type (using a platform-agnostic "javascript" in Content-Type check, which is good since it may be text/javascript or application/javascript depending on the system).
• The parameterized test_static_files_with_common_extensions_200 now includes .html (text/html) and .css (text/css) entries, which were previously forced to text/plain.
• All tests still verify the X-Content-Type-Options: nosniff security header.
• The guess_content_type() function itself already has its own unit tests in component_file_utils_test.py.

Missing test coverage considerations:

• There are no E2E tests specifically for static file serving with these new content types. However, the PR description notes this feature is behind a config option, and the unit tests provide adequate coverage for this handler-level change. E2E tests were not present before either, so this is not a regression in coverage.
• No Starlette-side unit test is updated. The Starlette handler already imported guess_content_type for other purposes, and the change is minimal (removing the allowlist check and passing the function result directly). The Tornado-side tests adequately validate the shared logic.

Backwards Compatibility

This is an intentional behavior change, but the PR author and team have assessed it:

• Files that were previously served as text/plain (e.g., .js, .html, .css, .svg) will now be served with their native content types. This is the explicit goal of the PR.
• The SAFE_APP_STATIC_FILE_EXTENSIONS constant is removed. If any third-party code was importing this (unlikely since it's an internal module), it would break. However, this is a private/internal constant not part of the public API.
• The change improves developer experience since static files will now work as expected (e.g., linking to a .js file from a custom component will correctly serve it as JavaScript).
• The MAX_APP_STATIC_FILE_SIZE constant is still exported and available.

This change is backwards compatible from the user-facing API perspective. The behavioral change (correct content types) is a feature improvement, not a regression.

Security & Risk

This change has security implications that appear to have been properly evaluated (the PR has the security-assessment-completed label):

• XSS via HTML files: Serving .html files as text/html from the static directory could theoretically enable XSS. However:
  • The static directory is controlled by the app developer (not end users).
  • Custom components already serve arbitrary file types without restrictions.
  • The X-Content-Type-Options: nosniff header is still set, preventing MIME-type sniffing attacks.
  • The feature is behind a config option (server.enableStaticServing).
• JavaScript execution: .js files served with proper content types could be loaded as scripts. Same mitigations apply — only the app developer controls the static directory.
• SVG files: SVGs served as image/svg+xml can contain embedded scripts, but again only the developer controls the content.
• Path traversal protections remain intact: The is_unsafe_path_pattern() check, symlink validation, and directory escape prevention are unchanged.
• Size limits remain: MAX_APP_STATIC_FILE_SIZE (200 MB) is unchanged.

Risk level: Low. The security posture is appropriate given that static file serving is developer-controlled and already optional.

Accessibility

No frontend UI changes in this PR. The snapshot update for st_time_input appears to be from a merge with develop and is unrelated. No accessibility concerns.

Recommendations

1. Consider whether the get_content_type override is necessary in the Tornado handler. Tornado's default StaticFileHandler.get_content_type() implementation is functionally identical to guess_content_type() — both use mimetypes.guess_type() with the same gzip/encoding/octet-stream logic. The override exists for "consistent behavior with Starlette handler" per the comment, but since both ultimately call the same underlying function, it's arguably redundant. That said, having it explicit makes the behavior self-documenting and ensures future changes to guess_content_type are reflected consistently. This is a very minor point and can be left as-is.

2. The unrelated snapshot change (st_time_input-dropdown-custom-theme[chromium].png) should be verified separately to ensure it's an expected artifact from the develop merge and not a test regression.

Verdict

APPROVED: Clean, well-tested removal of an overcautious content-type restriction for static file serving, with proper security mitigations retained. The change brings consistency between Tornado and Starlette handlers and aligns static file serving behavior with custom components.

This is an automated AI review by opus-4.6-thinking.