Skip to content

.Net Suggestion - Support "include list" for OpenApiFunctionExecutionParameters #10514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
crickman opened this issue Feb 13, 2025 · 2 comments · Fixed by #11710
Closed

.Net Suggestion - Support "include list" for OpenApiFunctionExecutionParameters #10514

crickman opened this issue Feb 13, 2025 · 2 comments · Fixed by #11710
Assignees
Labels
api_proposal Issues tagged with this label propose a change to the public API surface experimental Associated with an experimental feature function_calling .NET Issue or Pull requests regarding .NET code openapi Issues related to the OpenAPI function importer sk team issue A tag to denote issues that where created by the Semantic Kernel team (i.e., not the community)

Comments

@crickman
Copy link
Contributor

OpenApiFunctionExecutionParameters supports the ability to exclude specified API from the overall specification. This makes sense since the surface for an Open API may be quite large (huge even!).

The thing is, as an API might grow anything not excluded is included by default. This is a classic design bug: logic based on disallowed state instead of allowed state. This design bug is often associated with a security context and in this case, it could very well result in a security exploit.

Since OpenApiFunctionExecutionParameters.OperationsToExclude is marked experimental (SKEXP0040), considering this prior to feature graduation may be prudent.

Instead of OperationsToExclude, supporting OperationsToInclude could provide developers with the precision that aligns with the most common intent. OperationsToExclude could then either be deprecated or removed. If OperationsToExclude & OperationsToInclude were to exist side-by-side, then the inclusion list would have logical priority. Throwing an exception of both were defined may be the most clear contract. If neither are defined, then the entire API would be included in the spec.

As a work-around, a developer can define the JSON API spec as a resource and edit it as they desire; however, this result in a brittle pattern where parameter changes to included api wouldn't be reflected dynamically.

@crickman crickman added .NET Issue or Pull requests regarding .NET code api_proposal Issues tagged with this label propose a change to the public API surface experimental Associated with an experimental feature function_calling openapi Issues related to the OpenAPI function importer triage labels Feb 13, 2025
@lynn-orrell
Copy link

I would highly recommend that we remove OperationsToExclude and replace it with OperationsToInclude. Not doing so can (and most likely will) lead to a security breach at some point.

@markwallace-microsoft markwallace-microsoft moved this to Sprint: Planned in Semantic Kernel Feb 13, 2025
@evchaki evchaki added the sk team issue A tag to denote issues that where created by the Semantic Kernel team (i.e., not the community) label Feb 18, 2025
@SergeyMenshykh
Copy link
Member

Hi @crickman, we've already started moving in this direction and introduced the OperationSelectionPredicate, which can be used to either include or exclude operations based on operation id, path, method, and description. This sample ImportOperationsBasedOnInclusionListAsync demonstrates this. The predicate is not propagated to the OpenApiFunctionExecutionParameters yet, but it eventually will be.

@SergeyMenshykh SergeyMenshykh moved this from Sprint: Planned to Sprint: In Progress in Semantic Kernel Feb 27, 2025
@SergeyMenshykh SergeyMenshykh moved this from Sprint: In Progress to Bug in Semantic Kernel Feb 28, 2025
@SergeyMenshykh SergeyMenshykh moved this from Bug to Sprint: In Progress in Semantic Kernel Apr 23, 2025
@SergeyMenshykh SergeyMenshykh moved this from Sprint: In Progress to Sprint: In Review in Semantic Kernel Apr 24, 2025
github-merge-queue bot pushed a commit that referenced this issue Apr 25, 2025
### Motivation, Context and Description

This PR adds an operation selector predicate that can include or exclude
operations based on id, method, path, and description. It also obsoletes
the `OperationsToExclude` exclusion list, which is limited to filtering
out operations by operation id only.

Closes: #10514
@SergeyMenshykh SergeyMenshykh moved this from Sprint: In Review to Sprint: Done in Semantic Kernel Apr 25, 2025
glorious-beard pushed a commit to glorious-beard/semantic-kernel that referenced this issue May 6, 2025
### Motivation, Context and Description

This PR adds an operation selector predicate that can include or exclude
operations based on id, method, path, and description. It also obsoletes
the `OperationsToExclude` exclusion list, which is limited to filtering
out operations by operation id only.

Closes: microsoft#10514
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api_proposal Issues tagged with this label propose a change to the public API surface experimental Associated with an experimental feature function_calling .NET Issue or Pull requests regarding .NET code openapi Issues related to the OpenAPI function importer sk team issue A tag to denote issues that where created by the Semantic Kernel team (i.e., not the community)
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants