Skip to content

Python: OpenApi: Harden operation path handling for consistent selection and request targeting (.Net & Python)#14140

Merged
rogerbarreto merged 5 commits into
microsoft:mainfrom
rogerbarreto:rogerbarreto/openapi-operation-selection-hardening-119552
Jul 6, 2026
Merged

Python: OpenApi: Harden operation path handling for consistent selection and request targeting (.Net & Python)#14140
rogerbarreto merged 5 commits into
microsoft:mainfrom
rogerbarreto:rogerbarreto/openapi-operation-selection-hardening-119552

Conversation

@rogerbarreto

Copy link
Copy Markdown
Member

Motivation and Context

The OpenAPI plugin imports operations from a spec and lets the host restrict which operations are exposed via an OperationSelectionPredicate (and via OperationsToExclude). Operation selection and request-URL construction should share one canonical interpretation of an operation path. This change closes two consistency gaps where a spec-supplied path could be interpreted differently at selection time than at request time.

  1. Selection vs. request path normalization. The selection predicate received the raw operation path, while the request URL is built from a canonicalized path. An encoded dot-segment (for example %2e%2e) is textually different from the endpoint it canonicalizes to, so it could pass a path-based include/exclude filter yet resolve to a different route. Operations whose path contains an encoded or literal dot-segment are now excluded before the predicate is consulted, symmetric with the existing request-time dot-segment rejection.

  2. Request target reconciliation (defense in depth). After the request URL is built, verify it did not move off the configured server. An operation path that is an absolute or authority-changing URI (for example https://another-host/admin) carries no dot-segment, so it passes path-segment validation, yet System.Uri / urljoin resolve it to a different scheme, host, or port. Such a request is now rejected before it is sent, so a credential-bearing request cannot be redirected to an unintended target.

Both changes are applied symmetrically to the .NET and Python OpenAPI ports.

Description

  • .NET
    • OpenApiDocumentParser skips operations whose path contains a dot-segment before invoking the selection predicate.
    • RestApiOperation extracts a shared ContainsDotSegment helper (reused by the existing ValidatePathSegments) and, after building the request URL, reconciles its scheme/host/port and base path against the configured server.
  • Python
    • openapi_parser skips dot-segment operations before the predicate runs.
    • rest_api_operation adds a shared _contains_dot_segment helper and a post-construction _ensure_request_target_matches_server reconciliation.

Tests

  • .NET: added selection-exclusion and request-target reconciliation tests; full Functions.UnitTests OpenApi suite green (488).
  • Python: added parity tests for both behaviors; tests/unit/connectors/openapi_plugin green (173), ruff + ruff-format + mypy + pre-commit clean.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • The PR follows the SK Contribution Guidelines
  • All unit tests pass, and I have added new tests where possible

The operation-selection predicate was invoked with the raw operation path, so an
encoded dot-segment (for example "%2e%2e") could differ textually from the endpoint
it canonicalizes to at request time and slip past an include/exclude selection filter.

Exclude operations whose path contains an encoded or literal dot-segment before the
selection predicate is consulted, in both the .NET and Python OpenAPI parsers. The
dot-segment detection is shared with the existing request-time path validation.
… construction

As defense in depth, verify after the request URL is built that URI construction did not
move the request off the configured server. An operation path that is an absolute or
authority-changing URI (for example "https://another-host/admin") carries no dot-segment,
so it passes path-segment validation, yet System.Uri / urljoin resolve it to a different
scheme, host, or port. Such a request could carry credentials to an unintended target.

Reject any built request whose scheme, host, or port differs from the configured server, or
whose canonical path falls outside the server base path, in both the .NET and Python OpenAPI
request builders.
Copilot AI review requested due to automatic review settings July 6, 2026 13:52
@rogerbarreto rogerbarreto requested review from a team as code owners July 6, 2026 13:52
@giles17 giles17 added .NET Issue or Pull requests regarding .NET code python Pull requests for the Python Semantic Kernel labels Jul 6, 2026
@github-actions github-actions Bot changed the title OpenApi: Harden operation path handling for consistent selection and request targeting (.Net & Python) .Net: OpenApi: Harden operation path handling for consistent selection and request targeting (.Net & Python) Jul 6, 2026
@github-actions github-actions Bot changed the title .Net: OpenApi: Harden operation path handling for consistent selection and request targeting (.Net & Python) Python: OpenApi: Harden operation path handling for consistent selection and request targeting (.Net & Python) Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
connectors/openapi_plugin
   openapi_parser.py136894%82, 139, 203, 260, 286–287, 290–291
connectors/openapi_plugin/models
   rest_api_operation.py2742790%101–102, 111–112, 121–122, 132, 141–142, 161–162, 171–172, 191–192, 228–229, 299, 302–305, 307, 351, 376, 378, 481
TOTAL28930565680% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
4062 23 💤 0 ❌ 0 🔥 1m 50s ⏱️

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hardens OpenAPI plugin operation path handling so operation selection and request URL construction share a consistent, defensive interpretation of paths in both the .NET and Python implementations.

Changes:

  • Exclude operations containing literal or percent-encoded dot-segments before invoking the operation-selection predicate (prevents selection-time bypass).
  • After constructing the request URL, reject requests whose resolved target moves off the configured server (scheme/host/port) or outside the server base path.
  • Add unit tests in both .NET and Python covering authority-changing paths and selection-time exclusion.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
python/tests/unit/connectors/openapi_plugin/test_sk_openapi.py Adds Python tests for authority-changing URL rejection and dot-segment exclusion before predicate selection.
python/semantic_kernel/connectors/openapi_plugin/openapi_parser.py Skips dot-segment operations before consulting the selection predicate (Python).
python/semantic_kernel/connectors/openapi_plugin/models/rest_api_operation.py Adds request-target reconciliation after URL construction and extracts shared dot-segment detection (Python).
dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationTests.cs Adds .NET tests for rejecting authority-changing operation paths and allowing same-server paths.
dotnet/src/Functions/Functions.UnitTests/OpenApi/OpenApiDocumentParserV20Tests.cs Adds .NET test ensuring encoded dot-segment paths are excluded before the selection predicate runs.
dotnet/src/Functions/Functions.OpenApi/OpenApi/OpenApiDocumentParser.cs Skips dot-segment operations before predicate selection (C#).
dotnet/src/Functions/Functions.OpenApi/Model/RestApiOperation.cs Adds request-target reconciliation and extracts dot-segment detection helper (C#).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated Code Review

Reviewers: 5 | Confidence: 89%

✓ Correctness

This PR correctly implements two complementary security hardening measures for OpenAPI operation path handling: (1) excluding dot-segment paths before the selection predicate is consulted, and (2) verifying the constructed request URL still targets the configured server. The Python implementation correctly uses urlparse for authority comparison (scheme/hostname/port tuple), and urljoin preserves the server authority for relative paths so only malicious absolute URIs trigger rejection. The refactoring of _validate_path_segments into a bool-returning _contains_dot_segment helper is a clean extraction with identical logic. No correctness bugs found.

✓ Security Reliability

This PR adds two complementary security hardening measures for OpenAPI operation path handling. The implementation is sound: (1) dot-segment paths are excluded before the selection predicate runs, preventing encoded traversal bypasses, and (2) a post-construction URL check rejects requests that moved off the configured server. Both .NET and Python implementations follow the same logical structure with platform-appropriate APIs. The defense-in-depth layers interact correctly — dot-segment detection handles path traversal while authority reconciliation handles absolute-URI redirection. No security or reliability issues found.

✓ Test Coverage

The PR adds good test coverage for the two new security behaviors (dot-segment selection exclusion and authority-changing path rejection). However, the base path reconciliation check in _ensure_request_target_matches_server — which rejects requests that escape the configured server's base path while keeping the same authority — has zero test coverage in both Python and .NET. This is a security-relevant defense-in-depth code path that can be triggered by an operation path like https://example.com/other/admin against a server configured as https://example.com/api/.

✓ Failure Modes

This PR implements a well-designed defense-in-depth approach to harden OpenAPI operation path handling. The dot-segment exclusion before predicate evaluation and the post-construction authority/base-path reconciliation are correctly implemented in both .NET and Python. Key behaviors verified: (1) urlparse port comparison works correctly because urljoin preserves the base URL's authority for relative paths - port mismatches only occur for absolute-URI attack paths which should be rejected, (2) the base_path startswith check is safe against prefix confusion due to guaranteed trailing slash from get_server_url, (3) exceptions are properly raised rather than swallowed at all trust boundaries. No silent failures, lost errors, or security bypasses identified.

✓ Design Approach

The hardening is directionally good, but the parser-side reconciliation is still incomplete: dot-segment paths are filtered before the selection predicate, while absolute or authority-changing paths are still offered to the predicate and only fail later during request construction. I did not find a concrete correctness or security regression beyond that inconsistency, so this is a design-level suggestion rather than a blocking defect.

Suggestions

  • Consider extending the pre-predicate exclusion to absolute/authority-changing paths as well. Currently create_rest_api_operations only skips dot-segments before invoking the predicate, but a path like https://evil.com/admin can still be imported by a path-based predicate even though it will be rejected later by _ensure_request_target_matches_server. Excluding such paths earlier would make selection and request construction share one canonical interpretation of an operation path.

Automated review by rogerbarreto's agents

Fold URL userinfo into the request-target reconciliation so an operation path such as
"https://user:pass@host/..." cannot inject credentials into the request even when the host,
scheme, and port still match the configured server. The Python check now compares userinfo
alongside scheme/host/port; the .NET check already covers it because GetLeftPart(Authority)
includes userinfo, and tests are added to lock in the behavior on both ports.
… reconciliation

Extend the Python parser to exclude absolute or authority-changing operation paths (for example
"https://another-host/admin") before the selection predicate is consulted, so selection and request
construction share one canonical target. The .NET OpenAPI reader already rejects such non-relative
path keys during parsing, so a regression test locks that in rather than adding an unreachable check.

Add the missing test coverage for the base-path reconciliation branch, which rejects a same-authority
request whose canonical path escapes the configured server base path, in both the .NET and Python
request builders.
@rogerbarreto

Copy link
Copy Markdown
Member Author

Thanks for the review. Both points addressed in 78203e9:

Base-path reconciliation test coverage. Added tests for the same-authority base-path-escape branch on both sides: ItShouldRejectSameAuthorityBasePathEscape (.NET) and test_build_operation_url_rejects_same_authority_base_path_escape (Python).

Pre-predicate exclusion of absolute / authority-changing paths. Extended the Python parser to exclude such paths before the selection predicate runs (_is_non_relative_path), matching the dot-segment handling. The .NET OpenAPI reader already rejects a non-relative path key at parse time (... is not a valid property at #/paths), so it can never reach selection; added ItShouldRejectDocumentWithNonRelativeOperationPathAsync to lock that in rather than adding an unreachable check.

@rogerbarreto rogerbarreto enabled auto-merge July 6, 2026 16:13
@rogerbarreto rogerbarreto added this pull request to the merge queue Jul 6, 2026
Merged via the queue into microsoft:main with commit f6391ad Jul 6, 2026
35 checks passed
@rogerbarreto rogerbarreto deleted the rogerbarreto/openapi-operation-selection-hardening-119552 branch July 6, 2026 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

.NET Issue or Pull requests regarding .NET code python Pull requests for the Python Semantic Kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants