[fix] Reject network paths and null bytes in st.Page

[lukasmasuch]

Describe your changes

Hardens st.Page() path handling to run validation before any filesystem
access (Path.resolve() / Path.is_file()):

• Rejects Windows UNC/network paths (e.g. \\server\share, //server/share,
  and the mixed-separator spellings /\... and \/... that Windows also
  resolves as a UNC root). On Windows, resolving such a path would initiate an
  SMB connection to an attacker-controlled host, enabling SSRF and NTLM hash
  disclosure. Absolute/drive-local paths remain allowed (part of the public
  st.Page contract).
• Rejects page paths containing null bytes on all platforms, surfacing a clear
  StreamlitAPIException instead of a raw ValueError.

The UNC check is factored into a shared is_windows_unc_path() helper in
path_security.py (reused by is_unsafe_path_pattern).

GitHub Issue Link (if applicable)

Internal security ticket: SNOW-3715977

Testing Plan

• Unit Tests (Python)
  • lib/tests/streamlit/path_security_test.py — is_windows_unc_path(), incl. mixed-separator spellings.
  • lib/tests/streamlit/navigation/page_test.py — st.Page rejects network paths (asserting resolve() is not called) and null bytes, while still allowing absolute local paths and non-Windows network-style paths.
• No E2E tests: the behavior is Windows-SMB-specific and cannot be meaningfully exercised by the Linux-based e2e harness; unit tests with env_util.IS_WINDOWS patched are the correct layer.

Made with Cursor

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[snyk-io]

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

Status	Scan Engine	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.

[github-actions]

✅ PR preview is ready!

Name	Link
📦 Wheel file	https://core-previews.s3-us-west-2.amazonaws.com/pr-16146/streamlit-1.60.0-py3-none-any.whl
📦 @streamlit/component-v2-lib	Download from artifacts
🕹️ Preview app	pr-16146.streamlit.app (☁️ Deploy here if not accessible)

[lukasmasuch]

Added Agent Docs

• security-findings.md: Security investigation for SNOW-3715977 — analysis of the st.Page UNC/network-path exposure (SSRF / NTLM hash disclosure), the mixed-separator bypass, and the residual compatibility boundary for absolute/drive-local paths.

[greptile-apps]

Greptile Summary

Hardens st.Page path validation before filesystem access.

• Rejects null bytes in page paths on all platforms.
• Rejects UNC and device-namespace paths on Windows.
• Adds a shared lexical UNC-path helper and corresponding unit coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failures remain.

Important Files Changed

Filename	Overview
lib/streamlit/navigation/page.py	Validates null bytes and Windows network paths before resolving or inspecting page files.
lib/streamlit/path_security.py	Adds a shared lexical helper for detecting Windows UNC and device-namespace path prefixes.
lib/tests/streamlit/navigation/page_test.py	Covers early rejection of unsafe paths and continued acceptance of supported local paths.
lib/tests/streamlit/path_security_test.py	Covers UNC detection across slash variants and local-path exclusions.

_{Reviews (4): Last reviewed commit: "Address AI review nits in st.Page path t..." | Re-trigger Greptile}

[github-actions]

Summary

This PR hardens st.Page() path handling by adding pre-filesystem validation that rejects Windows UNC/network paths (e.g. \\server\share, //server/share, and mixed-separator variants) and null-byte paths on all platforms. On Windows, resolving a UNC path can trigger an SMB connection to an attacker-controlled host, enabling SSRF and NTLM hash disclosure. A new is_windows_unc_path() helper is factored into path_security.py and shared with the existing is_unsafe_path_pattern() function for DRY reuse.

Reviewer agreement: Both reviewers (claude-4.6-opus-high-thinking and gpt-5.3-codex-high) approved this PR unanimously with no merge-blocking issues identified.

Code Quality

Both reviewers agree the implementation is clean, focused, and well-structured:

• Correct placement: Validation runs after the URL check but before any Path.resolve() / Path.is_file() call — exactly where it must be to prevent the attack vector.
• Minimal surface: The is_windows_unc_path() helper is a concise lexical check that normalizes forward slashes before checking, covering all four mixed-separator combinations (\\, //, \/, /\).
• Platform-scoped: The UNC check is correctly gated behind env_util.IS_WINDOWS. On POSIX, //server/share is a valid (if unusual) path with no SMB auto-connect behavior.
• DRY refactor: is_unsafe_path_pattern() now delegates to is_windows_unc_path() instead of duplicating the prefix check, and gains coverage of previously-missed mixed-separator variants.
• Import style: Follows project conventions for utility module imports.

No code quality issues identified by either reviewer.

Test Coverage

Both reviewers confirm strong, comprehensive test coverage:

• path_security_test.py: Parametrized positive tests for all UNC variants (backslash, forward slash, mixed, extended \\?\UNC\..., device namespace \\.\device\...) and negative tests for local paths (POSIX absolute, Windows drive, rooted, relative).
• page_test.py: End-to-end unit tests that verify UNC paths are rejected with resolve.assert_not_called() (confirming no filesystem access), null bytes are rejected before filesystem access, non-Windows platforms still accept // paths, and absolute local paths still work (backwards-compat regression guard).
• Edge cases: Path objects (not just strings), extended UNC, and device namespace paths are all covered.

Both reviewers agree the absence of E2E tests is appropriate — the behavior is Windows-specific SMB interaction that cannot be meaningfully exercised by the Linux-based e2e harness.

Backwards Compatibility

Both reviewers agree there are no breaking changes:

• UNC paths on Windows: Previously would have attempted (and likely failed or hung) an SMB connection. Now fails fast with a clear StreamlitAPIException. No legitimate use case is lost.
• Null bytes: Previously caused a raw ValueError from the OS; now raises a friendlier StreamlitAPIException. No legitimate user code is affected.
• Absolute/drive-local paths: Explicitly still allowed, tested by test_allows_absolute_local_paths.
• POSIX behavior: Completely unchanged; the UNC check is gated behind IS_WINDOWS.

Security & Risk

Both reviewers agree this is a well-scoped security hardening PR that closes a real attack vector (SNOW-3715977):

• Threat model: An attacker supplies a UNC path to force the server process to connect to an attacker-controlled SMB share, leaking NTLM credentials.
• Fix correctness: The lexical check runs before any Path method that triggers filesystem resolution — the correct approach.
• Defense in depth: The null-byte check prevents path truncation attacks on any platform.
• No new dependencies, external requests, or attack surface introduced.
• Residual risk: Low. The rejected inputs were never valid page paths.

External test recommendation

• Recommend external_test: No
• Triggered categories: None
• Evidence:
  • lib/streamlit/navigation/page.py: Pure Python input validation; no route, websocket, auth, cookie, storage, or embedding boundary changes.
  • lib/streamlit/path_security.py: Utility-level lexical path classification; no runtime networking or cross-origin behavior changes.
  • Test files: Unit-only coverage additions matching backend validation scope.
• Suggested external_test focus areas: None required. Optional follow-up: manual Windows validation of UNC rejection in a real host environment.
• Confidence: High (both reviewers agree)
• Assumptions and gaps: Assessment is diff-based and does not include runtime execution in a Windows environment.

Accessibility

No frontend/UI changes — not applicable. Both reviewers agree.

Readability

Both reviewers found no readability issues across all changed files:

• lib/streamlit/path_security.py: The is_windows_unc_path docstring is clear, explains intent (lexical check before filesystem ops), and documents the mixed-separator rationale concisely.
• lib/streamlit/navigation/page.py: Inline comments explain why the check exists (SMB credential disclosure) and why absolute paths are still allowed (public API contract). No rewrite needed.
• Test files: Test names are descriptive and self-documenting (test_rejects_windows_network_paths_before_resolving, test_rejects_null_byte_paths_on_all_platforms, etc.). Docstrings add appropriate context.
• PR title: Clear, specific, format-compliant ([fix] Reject network paths and null bytes in st.Page).
• PR description: Well-structured with security rationale, scope documentation, and testing plan.

One optional polish suggestion (from gpt-5.3-codex-high): Expand security acronyms (SMB, SSRF, NTLM) on first use in the PR description for broader reviewer readability.

Recommendations

No merge-blocking issues. Two optional, non-blocking observations:

1. (Optional) Consider adding a st.Page test case for a \\.\device\... input path at the API entrypoint level, mirroring the helper-level device namespace coverage in path_security_test.py. (gpt-5.3-codex-high)
2. (Optional) The test_allows_absolute_local_paths test uses Path.cwd(), coupling test behavior to the runner's working directory. This is fine in practice since class-level mocking ensures is_file() returns True regardless, but worth noting. (claude-4.6-opus-high-thinking)

Verdict

APPROVED: Well-crafted security fix with correct placement of validation, comprehensive test coverage, no backwards-compatibility risk, and clean code structure. Both reviewers unanimously approved with no merge-blocking issues.

Model	Status	Verdict
claude-4.6-opus-high-thinking	Completed	APPROVED
gpt-5.3-codex-high	Completed	APPROVED

---

This is a consolidated AI review by claude-4.6-opus-high-thinking, synthesizing reviews from 2 models. Please verify the feedback and use your judgment.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[lukasmasuch]

Addressed review feedback:

• CI (Python 3.10 / min-deps failure): The new tests patched streamlit.navigation.page.env_util.IS_WINDOWS, but on Python 3.10 unittest.mock's dotted-path importer resolves streamlit.navigation to the st.navigation function (not the subpackage module), raising AttributeError: 'function' object has no attribute 'page'. Switched the patch target to streamlit.env_util.IS_WINDOWS (same module object, matches the existing convention in event_based_path_watcher_test.py). Green on 3.10–3.14 locally.
• AI review (optional Implement Caching object to support running code blocks only once #1): Added a \\.\device\... device-namespace case to the st.Page entrypoint test, mirroring the helper-level coverage in path_security_test.py.
• AI review (optional Improve caching code #2 — Path.cwd() coupling): No change needed; Path.is_file is mocked at the class level, so the test is independent of the runner's working directory (as the reviewer noted).
• AI review (optional readability — acronyms): Left as-is; SMB/SSRF/NTLM are already used with explanatory context in the description.
• greptile P1 (\\?\C:\... extended-length local paths): Declined with reasoning on the inline thread — the shared helper is intentionally a simple fail-closed lexical check, \\?\ disables Windows path normalization (so allowing it could add traversal risk), and the standard C:\... absolute-path form still works.

[github-actions]

Summary

This PR hardens st.Page() path handling to prevent two classes of attack when Streamlit runs on Windows:

1. UNC/network path rejection: Paths like \\server\share\page.py (and all mixed-separator variants) are rejected before Path.resolve() or Path.is_file() can initiate an SMB connection, preventing SSRF and NTLM credential disclosure.
2. Null byte rejection: Paths containing \x00 are rejected on all platforms with a clear StreamlitAPIException instead of a raw ValueError.

The UNC detection is factored into a shared is_windows_unc_path() helper in path_security.py, which is also used by the existing is_unsafe_path_pattern() function (replacing an inline startswith check).

Reviewer consensus: Both reviewers (claude-4.6-opus-high-thinking and gpt-5.3-codex-high) approved the PR unanimously. All expected models completed their reviews successfully.

Code Quality

Both reviewers agree the implementation is clean, well-structured, and follows existing codebase patterns:

• Good factoring: The is_windows_unc_path() helper is a pure lexical check that normalizes / to \ before checking for \\ prefix, correctly catching all four mixed-separator spellings.
• Correct placement: Validation runs before any filesystem access (Path.resolve(), Path.is_file()), which is the critical security invariant.
• Defense in depth: The UNC check is gated on env_util.IS_WINDOWS in page.py (only Windows auto-initiates SMB), while is_unsafe_path_pattern() applies it unconditionally (appropriate for its stricter use in component file serving).
• Preserves public API contract: Absolute local paths (e.g., C:\Users\...) remain allowed, as documented in the st.Page API.
• Behavior-preserving refactor: The refactoring of is_unsafe_path_pattern() to use is_windows_unc_path() is behavior-preserving — the mixed-separator patterns newly caught by the helper were already caught by the subsequent path.startswith(("\\", "/")) check.

No merge-blocking code quality issues found.

Test Coverage

Both reviewers agree test coverage is thorough and well-designed:

• path_security_test.py: New TestIsWindowsUncPath class with parametrized positive cases (6 UNC variants including mixed separators and device namespaces) and negative cases (5 local path types).
• page_test.py: Tests for UNC rejection (7 variants including Path objects), null byte rejection (string and Path), an explicit regression test for allowing network-style paths on non-Windows, and a regression test for allowing absolute local paths.
• Anti-regression: Tests assert resolve.assert_not_called() to verify the security invariant that no filesystem access occurs before validation.
• No E2E tests: Both reviewers agree this is the correct decision — the behavior is Windows-SMB-specific and cannot be meaningfully exercised by the Linux-based E2E harness; unit tests with IS_WINDOWS patched are the correct layer.

Noted gap (non-blocking): Behavior is simulated via patched env_util.IS_WINDOWS in Linux CI, not exercised on a real Windows runtime. Both reviewers acknowledge this as an acceptable residual limitation.

Backwards Compatibility

Both reviewers agree there are no breaking changes:

• Non-Windows platforms: The UNC check only fires when env_util.IS_WINDOWS is True. POSIX paths like //server/share/page.py remain valid on Linux/macOS (confirmed by test_allows_network_style_paths_on_non_windows).
• Windows absolute local paths: C:\Users\...\page.py and similar drive-local paths are still allowed (confirmed by test_allows_absolute_local_paths).
• Null bytes: Were never valid in practice (the OS would reject them), but now produce a clearer StreamlitAPIException instead of a raw ValueError.

Security & Risk

Both reviewers agree the security fix is sound and addresses a real vulnerability:

• Threat model: On Windows, Path.resolve() on a UNC path triggers an SMB connection, which can leak the server process's NTLM hash to an attacker-controlled host.
• Fix correctness: The lexical check runs before any filesystem operation, preventing the SMB connection entirely.
• Mixed-separator handling: The slash normalization (path.replace("/", "\\")) correctly catches all four mixed-separator spellings that Windows resolves as UNC roots.
• No new dependencies or external requests: The fix is pure string validation.
• Residual risk: Low; main uncertainty is Windows-runtime validation depth in CI.

External test recommendation

• Recommend external_test: No
• Triggered categories: None
• Evidence:
  • lib/streamlit/navigation/page.py: Pure input validation added before filesystem access; no routing, auth, WebSocket, embedding, asset serving, cross-origin, or header changes.
  • lib/streamlit/path_security.py: Internal helper refactoring with no network or runtime impact.
  • Test files are test-only updates validating backend input handling.
• Suggested external_test focus areas: None — the change is a pre-I/O validation gate with no effect on externally observable behavior.
• Confidence: High
• Assumptions and gaps: None — the change is strictly additive input validation in the Python backend.

Both reviewers independently reached the same conclusion on external test risk.

Accessibility

No frontend changes; accessibility is not affected. Both reviewers agree.

Readability

lib/streamlit/path_security.py

is_windows_unc_path docstring (line 36): Clear and well-structured. Correctly explains the normalization strategy, the variants caught, and the security rationale. No changes needed. (Both reviewers agree.)

Inline comment at line 87: Still accurate after the refactoring. No changes needed.

lib/streamlit/navigation/page.py

Comment at lines 339–342: One reviewer (GPT) noted the wording says "Reject UNC paths" while the guard also rejects device-namespace patterns, and suggested rewording to "Reject Windows network/device-namespace paths." The other reviewer (Claude) found the comment clear and sufficient. Resolution: The current wording is acceptable — UNC paths encompass device namespaces (\\?\, \\.\) in Windows terminology, and the comment's focus on the security rationale (SMB credential disclosure) is more important than enumerating every sub-category. This is a non-blocking style preference.

lib/tests/streamlit/navigation/page_test.py

Test names and docstrings are descriptive and self-explanatory across both test files. No changes needed.

PR title and description

Both reviewers agree the PR title and description are clear, concise, and follow expected conventions. No rewrite needed.

Recommendations

1. Consider removing the @patch("streamlit.env_util.IS_WINDOWS", False) decorator on test_rejects_null_byte_paths_on_all_platforms (line 119 of page_test.py). The null byte check runs before the UNC check and doesn't depend on IS_WINDOWS, so this patch is unnecessary. Removing it would make the "all platforms" claim in the test name more accurate. (Raised by Claude; non-blocking.)

2. Consider adding an explicit test for device-namespace paths like \\?\C:\... to document whether they are expected to be allowed or rejected, locking in intent for future maintainers. (Raised by GPT; non-blocking.)

3. Consider improving the assertion in test_allows_network_style_paths_on_non_windows (line 137 of page_test.py). The current assert st.Page(...) is not None is trivially true — the real assertion is that no exception is raised. Either drop the assertion or assert a meaningful property. (Raised by Claude; non-blocking nitpick.)

Verdict

APPROVED: A well-executed security fix that correctly prevents UNC-based SSRF/NTLM credential disclosure and null-byte path attacks in st.Page(), with thorough test coverage, no breaking changes, and clean code quality. Both reviewers approved unanimously with only minor, non-blocking suggestions.

---

This is a consolidated AI review by claude-4.6-opus-high-thinking, synthesizing reviews from claude-4.6-opus-high-thinking and gpt-5.3-codex-high.

This review also includes 2 inline comment(s) on specific code lines.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Summary

This PR hardens st.Page() path handling by adding two pre-filesystem-access validation checks:

1. UNC/network path rejection (Windows only): Before Path.resolve() or Path.is_file() can trigger an SMB connection to an attacker-controlled host, paths that look like Windows UNC paths (e.g. \\server\share, //server/share, and mixed-separator variants) are rejected with a clear StreamlitAPIException. The check is gated on env_util.IS_WINDOWS.
2. Null-byte rejection (all platforms): Paths containing \x00 are rejected with a descriptive error instead of letting Python raise a raw ValueError.

The UNC detection is factored into a shared is_windows_unc_path() helper in path_security.py, also reused by the existing is_unsafe_path_pattern() function.

Reviewer consensus: Both reviewers (claude-4.6-opus-high-thinking and gpt-5.3-codex-high) approved this PR unanimously with no critical or blocking issues identified.

Code Quality

Both reviewers agreed the implementation is clean, focused, and follows existing patterns:

• The is_windows_unc_path() helper is a pure lexical check — easy to test and reason about.
• It is correctly reused in both st.Page.__init__ and is_unsafe_path_pattern, eliminating duplication.
• Validation is placed at exactly the right point — after the external-URL early return, before any filesystem access.
• Import style follows codebase conventions.
• Error messages are specific and user-facing.

No code quality issues were raised by either reviewer.

Test Coverage

Both reviewers found unit test coverage strong and comprehensive:

• path_security_test.py: 7 positive UNC cases (backslash, forward-slash, mixed, extended-length, device-namespace) and 5 negative cases. Uses pytest.mark.parametrize correctly.
• page_test.py: 7 parameterized UNC-rejection cases with IS_WINDOWS patched, 2 null-byte cases, 1 non-Windows allowance test, and 1 absolute-local-path backwards-compatibility assertion. Includes anti-regression assertions verifying Path.resolve() is never called.
• No E2E tests — both reviewers agreed this is correctly justified since the behavior is Windows-SMB-specific and the Linux-based e2e harness cannot meaningfully exercise it.

Backwards Compatibility

Both reviewers agreed there are no breaking changes for legitimate use cases. The PR only rejects inputs that were previously either:

• Dangerous (UNC paths triggering SMB on Windows — security vulnerability).
• Invalid (null-byte paths would crash with a raw ValueError).

Absolute local paths (e.g. C:\Users\... or /home/user/...) remain allowed, as tested.

Both reviewers noted the fail-closed treatment of extended-length Windows device paths (\\?\C:\...) — these are also rejected. Both agreed this is an acceptable and intentional security posture. One reviewer (gpt-5.3-codex-high) suggested documenting this explicitly; the other (claude-4.6-opus-high-thinking) noted it is already documented in the implementation.

Security & Risk

Both reviewers confirmed the security posture is improved:

• Prevents unintended SMB/network access and NTLM credential disclosure during path resolution on Windows.
• The helper normalizes / to \ before checking, catching all 4 mixed-separator combinations.
• Fail-closed design: any \\-prefixed input is treated as a network path.
• No new attack surface, no new dependencies, no endpoint/auth/session changes.
• Low regression risk — changes are narrowly scoped and purely additive.

External test recommendation

• Recommend external_test: No
• Triggered categories: None
• Evidence:
  • lib/streamlit/navigation/page.py: Adds pre-filesystem lexical validation — no routing, auth, WebSocket, embedding, CORS, or asset-serving changes.
  • lib/streamlit/path_security.py: Pure lexical helper — no network, server, or browser interaction.
• Suggested external_test focus areas: N/A
• Confidence: High
• Assumptions and gaps: Changes are purely backend input validation with no impact on externally-hosted behavior, iframe embedding, or cross-origin resource loading.

Accessibility

No frontend changes — not applicable. Both reviewers agreed.

Readability

Both reviewers found the code well-documented with clear naming and appropriate comments explaining security rationale.

One minor finding (agreed by both reviewers):

• lib/tests/streamlit/path_security_test.py line 45: The phrase "are also rejected fail-closed" uses "fail-closed" awkwardly as an adverb. Suggested rewrite: "are also rejected as a fail-closed policy: any \\-prefixed input is treated as a network path."

PR title and description: Both reviewers found the title concise and specific, and the description well-structured with clear security rationale and testing plan. One reviewer noted the internal ticket reference (SNOW-3715977) is acceptable for a security fix.

Recommendations

1. (Non-blocking) In path_security_test.py line 45, consider rewording "fail-closed" for grammatical clarity (see Readability section).
2. (Non-blocking) Consider adding a brief note in the PR description that the internal ticket reference is for security tracking purposes, to set expectations for external readers.

Verdict

APPROVED: This is a well-scoped, well-tested security hardening fix with unanimous approval from both reviewers. The implementation is clean, test coverage is comprehensive, and there are no backwards-compatibility concerns. The minor suggestions above are non-blocking and can be addressed in follow-up.

---

Model	Status	Verdict
claude-4.6-opus-high-thinking	Completed	APPROVED
gpt-5.3-codex-high	Completed	APPROVED

---

This is a consolidated AI review by claude-4.6-opus-high-thinking. Please verify the feedback and use your judgment.

This review also includes 1 inline comment(s) on specific code lines.