✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

• [security] Prevent SSRF attacks via path traversal in component file handling #13733 👈 (View in Graphite)
• develop

This stack of pull requests is managed by Graphite. Learn more about stacking.

✅ PR preview is ready!

📈 Frontend coverage change detected

The frontend unit test (vitest) coverage has increased by 0.0500%

• Current PR: 86.4300% (13706 lines, 1859 missed)
• Latest develop: 86.3800% (13706 lines, 1866 missed)

🎉 Great job on improving test coverage!

📊 View detailed coverage comparison

Summary

This PR adds security enhancements to prevent Server-Side Request Forgery (SSRF) vulnerabilities in component file handling. The key changes are:

1. New _is_unsafe_path() function - Detects potentially dangerous path patterns (UNC paths, absolute paths, path traversal, null bytes) before any filesystem operations occur.
2. Early validation in build_safe_abspath() - Validates paths BEFORE calling os.path.realpath() to prevent Windows from triggering SMB connections to attacker-controlled servers.
3. Stricter path handling - Rejects paths containing .. segments even if they would normalize safely (defense-in-depth).

Code Quality

The implementation is well-structured and follows Streamlit's coding standards:

Strengths:

• The _is_unsafe_path() function (lines 33-74 in component_file_utils.py) is properly prefixed with _ indicating it's private/internal use only
• Comprehensive NumPy-style docstrings explaining the security rationale and parameters
• Clear inline comments explaining each security check
• The security comment in build_safe_abspath() (lines 97-99) clearly documents why early validation is necessary

Code structure in _is_unsafe_path():

# Check order is logical and well-commented:
# 1. Null bytes (path truncation attacks)
# 2. UNC paths (Windows network shares)
# 3. Windows drive letters (case-insensitive)
# 4. Rooted absolute paths
# 5. Path traversal segments

The path traversal detection correctly handles:

• Mixed separators (\ and /)
• Legitimate filenames containing dots (e.g., file..name.js, ..config.js)

Test Coverage

Excellent test coverage with 211 new lines of tests. The tests follow best practices from the Python test guide:

1. Parameterized tests - Uses @pytest.mark.parametrize extensively with descriptive IDs
2. Docstrings - All test functions have clear docstrings explaining purpose
3. Anti-regression tests - Includes tests ensuring safe paths still work (test_safe_path_still_resolves_correctly, test_safe_nested_path_resolves)
4. Negative assertions - Tests both that unsafe paths are rejected AND that safe paths are allowed

Test categories covered:

• UNC paths (backslash and forward slash variants)
• Windows drive letters (uppercase and lowercase)
• Rooted absolute paths
• Path traversal (simple, complex, mixed separators)
• Null byte injection
• URL-decoded malicious paths
• Windows special path prefixes (\\?\, \\.\)
• Edge cases (empty string, current directory, filenames with dots)

Minor observation: The import from urllib.parse import unquote inside test_url_decoded_paths_are_rejected could be moved to the top of the file for consistency, but this is a non-blocking style preference.

Backwards Compatibility

Breaking change: Paths containing .. segments that would have previously normalized safely (e.g., sub/../inside.txt) are now rejected. This change is:

• Intentional and documented - The removed test case normalized_parent_segments is replaced with test_normalized_parent_segments_rejected() explaining this security-motivated behavior change
• Low impact - Normal component file requests don't use .. in paths; this primarily affects attack payloads
• Justified - Defense-in-depth against SSRF attacks where path traversal could be combined with other techniques

The change is unlikely to affect legitimate users since component URLs are typically clean paths like component_name/file.js.

Security & Risk

Security assessment: GOOD

The fix correctly addresses the SSRF vulnerability:

1. Root cause addressed - The check runs BEFORE os.path.realpath(), preventing Windows from initiating SMB connections to attacker-controlled servers
2. Comprehensive pattern detection - Covers UNC paths, drive letters, absolute paths, traversal, and null bytes
3. Platform-agnostic - Rejects Windows-specific attacks even on Linux servers, which is the correct approach since:
   • Some deployments may use Windows
   • Defense-in-depth is appropriate for security code
4. No bypass vectors identified - The implementation handles:
   • URL encoding (handled by framework before reaching this code)
   • Mixed path separators
   • Case variations in drive letters

Minor edge case not covered but acceptable: C:foo (Windows relative path on a specific drive) is not explicitly rejected, but this format is rarely used and would likely be caught by other security mechanisms downstream.

Recommendations

No blocking issues found. The following are optional improvements:

1. Consider logging rejected paths (non-blocking): For security monitoring, it might be useful to log when unsafe paths are rejected. However, this should be done carefully to avoid log injection.

2. Move import to top of file (style, non-blocking): In test_url_decoded_paths_are_rejected, the from urllib.parse import unquote import could be moved to the file's import section for consistency.

Verdict

APPROVED: This is a well-implemented security fix with comprehensive test coverage. The code follows Streamlit's conventions, the breaking change is justified and documented, and no security bypasses or regressions were identified. The PR is ready for merge.

This is an automated AI review using opus-4.5-thinking. Please verify the feedback and use your judgment.

Summary

This PR centralizes path safety validation into a shared helper and applies it before any filesystem resolution in component and app static file handling, preventing UNC/drive-based SSRF vectors and tightening traversal checks. It also adds comprehensive Python unit tests for unsafe patterns and regression coverage around early rejection.

Code Quality

The refactor is clean and keeps path security checks in one place (lib/streamlit/path_security.py) while reusing it in both component routing and registration flows. Docstrings explain the security rationale and the call sites are updated consistently; I did not spot structural issues or regressions in the logic.

Test Coverage

Unit tests were added and are thorough, including regression checks that os.path.realpath is not called on unsafe inputs and coverage for UNC/drive/null-byte/traversal patterns (lib/tests/streamlit/web/server/component_file_utils_test.py, lib/tests/streamlit/path_security_test.py). No e2e or frontend tests were touched, which is appropriate for this backend-only change. Tests were not run locally (per instructions).

Backwards Compatibility

Behavior is intentionally stricter: paths containing .. segments and drive-qualified prefixes are now rejected even if they would normalize inside the root. This could affect components that rely on such paths, but the change is security-motivated and consistent across handlers.

Security & Risk

Security posture is improved: unsafe path patterns are rejected before any realpath calls, reducing SSRF/NTLM exposure on Windows. I did not find new security risks or regression hazards in the updated logic.

Recommendations

1. Consider a brief release note or changelog entry describing the stricter path validation (rejection of .. segments and drive-qualified paths) to help users understand potential edge-case behavior changes.

Verdict

APPROVED: Security fix is well-scoped, tests are comprehensive, and no blocking issues were found.

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

Summary

This PR hardens Streamlit’s static/component file-path handling to prevent Windows UNC-path SSRF/NTLM leakage by introducing a shared is_unsafe_path_pattern validator and ensuring validation happens before any os.path.realpath() calls. It also extends similar protection to the Tornado AppStaticFileHandler and adds comprehensive Python unit tests for the new behavior.

Code Quality

• Good consolidation and consistency: Centralizing the logic in lib/streamlit/path_security.py reduces the risk of divergence between code paths and makes the security invariant (“validate before filesystem resolution”) explicit.
• Good “early check” placement:
  • build_safe_abspath rejects unsafe patterns before any realpath() on untrusted input (lib/streamlit/web/server/component_file_utils.py:57-73).
  • AppStaticFileHandler.get_absolute_path rejects unsafe patterns before delegating to Tornado (lib/streamlit/web/server/app_static_file_handler.py:57-63).
• Minor: error messaging accuracy: ComponentPathUtils._assert_relative_no_traversal distinguishes traversal vs “absolute” by scanning segments (lib/streamlit/components/v2/component_path_utils.py:149-160), but non-traversal unsafe inputs (e.g. null bytes) will still raise “Absolute paths…” which can be misleading.
• Minor: redundancy/clarity in validator: is_unsafe_path_pattern checks both path.startswith(("\\", "/")) and os.path.isabs(path) (lib/streamlit/path_security.py:86-92). This is fine, but the combination is slightly redundant and may benefit from a brief comment explaining why both are retained (or remove one if not needed).

Test Coverage

• Unit tests are strong and mostly follow repo guidance:
  • New dedicated tests for the shared validator in lib/tests/streamlit/path_security_test.py (typed, docstrings, paramized coverage).
  • Good regression test asserting realpath() is not called for unsafe input (lib/tests/streamlit/web/server/component_file_utils_test.py:253-272), which directly encodes the SSRF security invariant.
  • Good integration coverage for Tornado handler via HTTP-level tests (lib/tests/streamlit/web/server/app_static_file_handler_test.py:245-286).
• No e2e tests added: That seems appropriate here since the change is backend path validation + handler behavior; unit/integration tests cover the critical invariant well.
• One small best-practice note: lib/tests/streamlit/web/server/app_static_file_handler_test.py is unittest-based and many pre-existing test methods are untyped; the newly added methods are typed, which is good, but the file overall doesn’t fully match the “new tests should be fully annotated” guideline. I wouldn’t block on this since it’s consistent with the existing test style in that file.

Backwards Compatibility

• Intentional behavior tightening: build_safe_abspath now rejects paths containing .. segments even if they would normalize safely (e.g. sub/../inside.txt) (lib/tests/streamlit/web/server/component_file_utils_test.py:144-152). This is a behavioral change that could affect any callers that previously relied on passing normalized traversal segments.
  • Given the security context, this seems reasonable, but it’s worth calling out explicitly in the PR description/release notes if this endpoint behavior is user-observable (e.g. some apps or proxies generating such URLs).
• HTTP status semantics: The static handler now returns 403 early for “unsafe patterns” (lib/streamlit/web/server/app_static_file_handler.py:57-63), where some cases may previously have been 404. This should be acceptable (and arguably preferable) for malicious inputs, but it is technically observable.

Security & Risk

• Security improvement is high-signal and correctly targeted:
  • Prevents Windows from triggering SMB connections by ensuring UNC/drive-like paths are rejected before any realpath() resolution on attacker-controlled input (lib/streamlit/path_security.py:21-45, lib/streamlit/web/server/component_file_utils.py:57-63).
  • Extends similar hardening to Tornado’s static path resolution entrypoint (lib/streamlit/web/server/app_static_file_handler.py:57-63).
• Low regression risk overall due to extensive unit coverage and the fact that the new checks are additive/early-return.
• Potential edge-case tradeoff (acceptable): The validator treats leading backslashes as unsafe on all platforms (lib/streamlit/path_security.py:86-92), which could theoretically reject rare “valid” POSIX filenames that begin with \. In the context of URL/asset paths, this seems like a sensible defense-in-depth tradeoff.

Recommendations

1. In ComponentPathUtils._assert_relative_no_traversal, consider returning a more precise error for non-traversal unsafe patterns (e.g. distinguish null byte / UNC / drive) instead of always falling back to “Absolute paths…” (lib/streamlit/components/v2/component_path_utils.py:149-160).
2. Consider simplifying or documenting the overlapping absolute-path checks in is_unsafe_path_pattern (lib/streamlit/path_security.py:86-92) to make the intent clearer (e.g. why both prefix checks and os.path.isabs are needed).
3. (Non-blocking) If this behavior change is user-observable, explicitly document the tightened rejection of normalized .. segments in the PR description/changelog notes (lib/tests/streamlit/web/server/component_file_utils_test.py:144-152).

Verdict

APPROVED: The security fix is well-scoped, defensively implemented (early validation), and backed by strong unit/integration tests with a key regression check that prevents reintroducing the SSRF vector.

This is an automated AI review using gpt-5.2-high. Please verify the feedback and use your judgment.

Summary

This PR centralizes path validation into a shared security helper and applies it to component asset resolution and app static file handling, preventing UNC/drive-based SSRF and traversal patterns before filesystem operations. It also adds targeted unit tests to document the security invariants and regression risks.

Code Quality

The refactor is clear and well-scoped: the shared is_unsafe_path_pattern keeps behavior consistent across the codebase, and the new early checks are documented and placed before realpath/path resolution. The updated tests are organized and include negative assertions that align with our testing guidelines. No maintainability issues found.

Test Coverage

Unit tests were added for the new shared helper and for the component/static-file path handling, including regression coverage that ensures realpath is not called for unsafe inputs. No e2e coverage is needed here since behavior is server-side path validation with no UI changes. No frontend changes, so TypeScript test guidance is not applicable.

Backwards Compatibility

Paths containing .. segments are now rejected up front even if they would normalize inside the root. This is an intentional security hardening, but it is a behavior change that could affect unusual component asset references that rely on sub/../file style paths.

def build_safe_abspath(component_root: str, relative_url_path: str) -> str | None:
    r"""Build an absolute path inside ``component_root`` if safe.
    ...
    if is_unsafe_path_pattern(relative_url_path):
        return None
    ...

Security & Risk

Security posture is improved: SSRF/UNC path resolution is blocked before any filesystem operations, and explicit tests guard against regressions. No new security risks identified.

Recommendations

1. No changes requested.

Verdict

APPROVED: Security hardening is solid with good unit coverage and no functional regressions observed.

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