mirror of
https://github.com/microsoft/semantic-kernel.git
synced 2026-10-12 02:19:06 +00:00
Python: Reject encoded dot-segment paths in OpenAPI plugin (.NET and Python) (#14086)
## 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/admin` pass an `OperationSelectionPredicate` path check, yet `System.Uri` (.NET) and `urljoin` (Python) collapse them to a different route at request time. The selected path and the actual request target can therefore diverge. ## Description - **.NET** (`RestApiOperation.ValidatePathSegments`): decodes each path segment until stable (handling double encoding like `%252e` and encoded separators `%2f`/`%5c`) before rejecting canonical `.` or `..` segments. - **Python** (`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: - .NET: 482/482 `Functions.UnitTests` OpenApi tests pass. - Python: 109/109 `openapi_plugin` unit tests pass. ## Contribution Checklist - [x] The code builds clean without any errors or warnings - [x] The PR follows the SK Contribution Guidelines - [x] All unit tests pass, and I have added new tests where possible - [x] I didn't break anyone 😄
This commit is contained in:
@@ -412,19 +412,39 @@ public sealed class RestApiOperation
|
||||
};
|
||||
|
||||
/// <summary>
|
||||
/// Validates that the path does not contain dot-segments (. or ..) that could enable path traversal.
|
||||
/// Validates that the path does not contain dot-segments (. or ..) that could enable path traversal,
|
||||
/// including percent-encoded forms (e.g. "%2e%2e") that <see cref="Uri"/> canonicalizes at request time.
|
||||
/// ".." navigates up one path segment, enabling traversal to unintended endpoints.
|
||||
/// "." refers to the current directory — harmless but unexpected, so rejected to prevent misuse.
|
||||
/// </summary>
|
||||
/// <param name="path">The path to validate.</param>
|
||||
private static void ValidatePathSegments(string path)
|
||||
{
|
||||
var segments = path.Split('/');
|
||||
for (int i = 0; i < segments.Length; i++)
|
||||
// Split on the structural path separator first.
|
||||
foreach (var rawSegment in path.Split('/'))
|
||||
{
|
||||
if (segments[i] == "." || segments[i] == "..")
|
||||
// Decode percent-encoding until stable to catch encoded ("%2e") and
|
||||
// double-encoded ("%252e") dot-segments before URI canonicalization.
|
||||
var decoded = rawSegment;
|
||||
for (int i = 0; i < 5; i++)
|
||||
{
|
||||
throw new KernelException($"Path '{path}' contains a dot-segment, which could lead to path traversal.");
|
||||
var unescaped = Uri.UnescapeDataString(decoded);
|
||||
if (string.Equals(unescaped, decoded, StringComparison.Ordinal))
|
||||
{
|
||||
break;
|
||||
}
|
||||
|
||||
decoded = unescaped;
|
||||
}
|
||||
|
||||
// A decoded segment may itself contain encoded separators ("%2f"/"%5c"),
|
||||
// so re-split on both '/' and '\' and reject any resulting dot-segment.
|
||||
foreach (var segment in decoded.Split('/', '\\'))
|
||||
{
|
||||
if (segment == "." || segment == "..")
|
||||
{
|
||||
throw new KernelException($"Path '{path}' contains a dot-segment, which could lead to path traversal.");
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1467,6 +1467,92 @@ public class RestApiOperationTests
|
||||
Assert.Equal("https://example.com/api/files/report.v2.txt", url.OriginalString);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("/resources/%2e%2e/admin")]
|
||||
[InlineData("/resources/%2E%2E/admin")]
|
||||
[InlineData("/resources/%2e./admin")]
|
||||
[InlineData("/resources/.%2e/admin")]
|
||||
[InlineData("/resources/%2e%2e%2fadmin")]
|
||||
[InlineData("/resources/%252e%252e/admin")]
|
||||
[InlineData("/resources/%2e/admin")]
|
||||
public void ItShouldRejectEncodedDotSegmentInPathTemplate(string path)
|
||||
{
|
||||
// Arrange — operation path template contains an encoded dot-segment that
|
||||
// System.Uri would canonicalize into a path-traversal at request time.
|
||||
var sut = new RestApiOperation(
|
||||
id: "fake_id",
|
||||
servers: [new RestApiServer("https://example.com/api")],
|
||||
path: path,
|
||||
method: HttpMethod.Get,
|
||||
description: "fake_description",
|
||||
parameters: [],
|
||||
responses: new Dictionary<string, RestApiExpectedResponse>(),
|
||||
securityRequirements: []
|
||||
);
|
||||
|
||||
var arguments = new Dictionary<string, object?>();
|
||||
|
||||
// Act & Assert — encoded dot-segments must be rejected before URL is built
|
||||
var ex = Assert.Throws<KernelException>(() => sut.BuildOperationUrl(arguments));
|
||||
Assert.Contains("dot-segment", ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ItShouldRejectEncodedDotSegmentInPathParameter()
|
||||
{
|
||||
// Arrange — path parameter value is an encoded ".." (%2e%2e)
|
||||
var parameters = new List<RestApiParameter> {
|
||||
new(
|
||||
name: "id",
|
||||
type: "string",
|
||||
isRequired: true,
|
||||
expand: false,
|
||||
location: RestApiParameterLocation.Path,
|
||||
style: RestApiParameterStyle.Simple)
|
||||
};
|
||||
|
||||
var sut = new RestApiOperation(
|
||||
id: "fake_id",
|
||||
servers: [new RestApiServer("https://example.com/api")],
|
||||
path: "/resources/{id}/details",
|
||||
method: HttpMethod.Get,
|
||||
description: "fake_description",
|
||||
parameters: parameters,
|
||||
responses: new Dictionary<string, RestApiExpectedResponse>(),
|
||||
securityRequirements: []
|
||||
);
|
||||
|
||||
var arguments = new Dictionary<string, object?> { { "id", "%2e%2e" } };
|
||||
|
||||
// Act & Assert — encoded dot-segments in parameter values must be rejected
|
||||
var ex = Assert.Throws<KernelException>(() => sut.BuildOperationUrl(arguments));
|
||||
Assert.Contains("dot-segment", ex.Message);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ItShouldAllowEncodedNonDotSegmentCharactersInPathTemplate()
|
||||
{
|
||||
// Arrange — path contains encoded characters that are NOT dot-segments
|
||||
var sut = new RestApiOperation(
|
||||
id: "fake_id",
|
||||
servers: [new RestApiServer("https://example.com/api")],
|
||||
path: "/resources/a%20b/details",
|
||||
method: HttpMethod.Get,
|
||||
description: "fake_description",
|
||||
parameters: [],
|
||||
responses: new Dictionary<string, RestApiExpectedResponse>(),
|
||||
securityRequirements: []
|
||||
);
|
||||
|
||||
var arguments = new Dictionary<string, object?>();
|
||||
|
||||
// Act
|
||||
var url = sut.BuildOperationUrl(arguments);
|
||||
|
||||
// Assert — legitimate encoded characters must not be rejected
|
||||
Assert.Equal("https://example.com/api/resources/a%20b/details", url.OriginalString);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ItShouldEncodeServerVariableValuesLookedUpByArgumentName()
|
||||
{
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
import re
|
||||
from typing import Any, Final
|
||||
from urllib.parse import ParseResult, ParseResultBytes, quote, urlencode, urljoin, urlparse, urlunparse
|
||||
from urllib.parse import ParseResult, ParseResultBytes, quote, unquote, urlencode, urljoin, urlparse, urlunparse
|
||||
|
||||
from semantic_kernel.connectors.openapi_plugin.models.rest_api_expected_response import (
|
||||
RestApiExpectedResponse,
|
||||
@@ -289,8 +289,31 @@ class RestApiOperation:
|
||||
)
|
||||
continue
|
||||
path_template = path_template.replace(f"{{{parameter.name}}}", quote(str(argument), safe=""))
|
||||
self._validate_path_segments(path_template)
|
||||
return path_template
|
||||
|
||||
@staticmethod
|
||||
def _validate_path_segments(path: str) -> None:
|
||||
"""Reject dot-segments (. or ..), including percent-encoded forms, that enable path traversal.
|
||||
|
||||
The operation is selected using the raw path but the request URL is built from a canonicalized
|
||||
path, so encoded dot-segments such as "%2e%2e" must be rejected before the URL is constructed.
|
||||
"""
|
||||
for segment in path.split("/"):
|
||||
decoded = segment
|
||||
for _ in range(5):
|
||||
unescaped = unquote(decoded)
|
||||
if unescaped == decoded:
|
||||
break
|
||||
decoded = unescaped
|
||||
# A decoded segment may contain encoded separators ("%2f"/"%5c"), so re-split on
|
||||
# both "/" and "\" and reject any resulting dot-segment.
|
||||
for part in decoded.replace("\\", "/").split("/"):
|
||||
if part in (".", ".."):
|
||||
raise FunctionExecutionException(
|
||||
f"Path '{path}' contains a dot-segment, which could lead to path traversal."
|
||||
)
|
||||
|
||||
def build_query_string(self, arguments: dict[str, Any]) -> str:
|
||||
"""Build the query string for the operation."""
|
||||
segments = []
|
||||
|
||||
@@ -435,9 +435,9 @@ def test_build_path_prevents_path_traversal():
|
||||
id="test", method="GET", servers=["https://example.com/"], path="/resource/{id}", params=parameters
|
||||
)
|
||||
arguments = {"id": "../../admin"}
|
||||
result = operation.build_path(operation.path, arguments)
|
||||
# The slashes must be encoded so ../../admin becomes a single path segment, not a traversal
|
||||
assert result == "/resource/..%2F..%2Fadmin"
|
||||
# Encoded separators that decode into dot-segments must be rejected, not silently encoded
|
||||
with pytest.raises(FunctionExecutionException, match="dot-segment"):
|
||||
operation.build_path(operation.path, arguments)
|
||||
|
||||
|
||||
def test_build_path_double_encodes_pre_encoded_values():
|
||||
@@ -462,6 +462,40 @@ def test_build_path_encodes_unicode_characters():
|
||||
assert result == "/resource/caf%C3%A9%20r%C3%A9sum%C3%A9"
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"path",
|
||||
[
|
||||
"/resources/../admin",
|
||||
"/resources/./admin",
|
||||
"/resources/%2e%2e/admin",
|
||||
"/resources/%2E%2E/admin",
|
||||
"/resources/%2e/admin",
|
||||
"/resources/%2e%2e%2fadmin",
|
||||
"/resources/%252e%252e/admin",
|
||||
],
|
||||
)
|
||||
def test_build_path_rejects_dot_segment_in_template(path):
|
||||
operation = RestApiOperation(id="test", method="GET", servers=["https://example.com/"], path=path, params=[])
|
||||
with pytest.raises(FunctionExecutionException, match="dot-segment"):
|
||||
operation.build_path(operation.path, {})
|
||||
|
||||
|
||||
def test_build_path_rejects_dot_segment_via_parameter():
|
||||
parameters = [RestApiParameter(name="id", type="string", location=RestApiParameterLocation.PATH, is_required=True)]
|
||||
operation = RestApiOperation(
|
||||
id="test", method="GET", servers=["https://example.com/"], path="/resource/{id}/details", params=parameters
|
||||
)
|
||||
with pytest.raises(FunctionExecutionException, match="dot-segment"):
|
||||
operation.build_path(operation.path, {"id": ".."})
|
||||
|
||||
|
||||
def test_build_path_allows_encoded_non_dot_segment_characters():
|
||||
operation = RestApiOperation(
|
||||
id="test", method="GET", servers=["https://example.com/"], path="/resources/a%20b/details", params=[]
|
||||
)
|
||||
assert operation.build_path(operation.path, {}) == "/resources/a%20b/details"
|
||||
|
||||
|
||||
def test_build_query_string_with_required_parameter():
|
||||
parameters = [
|
||||
RestApiParameter(name="query", type="string", location=RestApiParameterLocation.QUERY, is_required=True)
|
||||
|
||||
Reference in New Issue
Block a user