Python: Reject encoded dot-segment paths in OpenAPI plugin (.NET and Python)#14086
Python: Reject encoded dot-segment paths in OpenAPI plugin (.NET and Python)#14086rogerbarreto wants to merge 2 commits into
Conversation
The OpenAPI plugin selects operations using the raw path but builds the request URL from a canonicalized path. Encoded dot-segments such as /resources/%2e%2e/admin pass an OperationSelectionPredicate path check, then System.Uri (.NET) and urljoin (Python) collapse them to a different route at request time. .NET: ValidatePathSegments now decodes each path segment (until stable, handling double encoding and encoded slashes) before rejecting canonical . or .. segments. Python: build_path now validates path segments with the same decode-then-reject logic before the URL is built. Adds regression tests on both stacks covering literal and encoded dot-segments, mixed case, encoded slashes, and double encoding, plus negative tests for legitimate encoded characters.
moonbox3
added
.NET
github-actions
Bot
changed the title
github-actions
Bot
changed the title
There was a problem hiding this comment.
Pull request overview
This PR hardens the OpenAPI plugins (.NET and Python) against operation-selection bypasses caused by percent-encoded dot-segments that get canonicalized differently when building runtime request URLs.
Changes:
- (.NET) Enhances
RestApiOperation.ValidatePathSegmentsto decode percent-encoding (including double-encoding) and reject./..after re-splitting on decoded separators. - (Python) Adds
_validate_path_segmentsand calls it fromRestApiOperation.build_pathto reject encoded dot-segments before URL construction. - Adds regression tests in both stacks for dot-segment rejection and allows legitimate encoded characters.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/tests/unit/connectors/openapi_plugin/test_sk_openapi.py | Adds Python unit tests asserting dot-segment rejection and preserving legitimate encoded characters. |
| python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation.py | Adds decode-then-reject dot-segment validation during path building (before urljoin). |
| dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationTests.cs | Adds .NET unit tests covering encoded dot-segments, encoded separators, and non-dot encoded characters. |
| dotnet/src/Functions/Functions.OpenApi/Model/RestApiOperation.cs | Strengthens dot-segment validation by decoding (including double-encoding) and handling decoded separators. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 92%
✗ Security Reliability
The Python _validate_path_segments implementation has a security bypass that the .NET equivalent correctly handles. After iteratively decoding a segment, the .NET code re-splits on '/' and '' to catch encoded path separators (e.g., %2f, %5c) embedded within a segment before checking for dot-segments. The Python code only checks if the fully-decoded segment is exactly '.' or '..', missing the case where an encoded slash is part of the segment (e.g., '%2e%2fadmin' decodes to '../admin', which is not in ('.', '..'). The .NET test suite covers this vector ('/resources/%2e%2fadmin') but the Python tests and implementation do not.
✓ Test Coverage
The test coverage is solid for the core encoded dot-segment cases on both platforms. However, there is an asymmetry between the .NET and Python implementations: the .NET version re-splits decoded segments on '/' and '' after decoding (catching vectors like '%2e%2fadmin' which decodes to '../admin'), with a corresponding test case. The Python implementation lacks this re-split logic and the corresponding test. While Python's urljoin doesn't currently canonicalize %2f, some downstream servers do, so this is a defense-in-depth gap. The mixed-literal/encoded cases ('%2e.' and '.%2e') tested in .NET are also absent from the Python parametrized tests, though the Python implementation would catch them.
✗ Failure Modes
The Python _validate_path_segments implementation is missing a critical re-split on decoded path separators ('/' and '') that the .NET implementation includes. A path like '/resources/%2e%2fadmin' decodes to '../admin' as a single segment, which passes the Python check (since '../admin' is neither '.' nor '..') but is correctly rejected by the .NET version. This creates an asymetric security bypass in the Python implementation for the exact attack vector this PR is designed to prevent.
✓ Design Approach
The .NET side closes the encoded-separator variant, but the Python port does not mirror that logic, so the cross-language fix is incomplete for paths where decoding reveals a hidden slash before a dot-segment.
Flagged Issues
- Python
_validate_path_segments(rest_api_operation.py:309) does not re-split decoded segments on '/' or '' after iterative decoding, allowing encoded-slash traversal payloads like '/resources/%2e%2e%2fadmin' (decodes to '../admin') to bypass theif decoded in (".", "..")check. The .NET implementation correctly re-splits decoded segments on both separators (RestApiOperation.cs:442) and tests this vector (RestApiOperationTests.cs:1475).
Automated review by rogerbarreto's agents
|
Flagged issue Python Source: automated DevFlow PR review |
Motivation and Context
Closes #14085.
The .NET and Python OpenAPI plugins select operations using the raw OpenAPI path, but build the runtime request URL from a canonicalized path. Encoded dot-segments such as
/resources/%2e%2e/adminpass anOperationSelectionPredicatepath check, yetSystem.Uri(.NET) andurljoin(Python) collapse them to a different route at request time. The selected path and the actual request target can therefore diverge.Description
RestApiOperation.ValidatePathSegments): decodes each path segment until stable (handling double encoding like%252eand encoded separators%2f/%5c) before rejecting canonical.or..segments.RestApiOperation.build_path): adds equivalent decode-then-reject validation before the URL is built.Regression tests on both stacks cover literal and encoded dot-segments, mixed case, encoded slashes, and double encoding, plus negative tests confirming legitimate encoded characters still work.
Test results:
Functions.UnitTestsOpenApi tests pass.openapi_pluginunit tests pass.Contribution Checklist