Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -412,19 +412,39 @@ value is string { } strValue &&
};

/// <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.");
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1467,6 +1467,92 @@ public void ItShouldAllowDotsInNonSegmentPathParameterValues()
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()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -289,8 +289,31 @@ def build_path(self, path_template: str, arguments: dict[str, Any]) -> str:
)
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 = []
Expand Down
40 changes: 37 additions & 3 deletions python/tests/unit/connectors/openapi_plugin/test_sk_openapi.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand All @@ -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)
Expand Down
Loading