Skip to content
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

Removing TrustService from Kernel.Core (to become extension) #1895

Merged
merged 5 commits into from
Jul 7, 2023

Conversation

shawncal
Copy link
Member

@shawncal shawncal commented Jul 7, 2023

Motivation and Context

The TrustService, along with related types like TrustAwareString showcase a really interesting pattern, but we'd like to repackage them into a SemanticKernal.Extensions.Trust package with a lot more configurability, rather than baking these into the SK Core. With the upcoming support for pre- and post- completion hooks in a Plan, we believe we can build a better solution that leverages those hooks, but is optional, configurable, and fully replaceable if our solution doesn't meet your needs.

Other reasons:

  • The TrustAwareString type is at odds with the transition we're making to allow any objects (not just strings) in SK's context variables, function args, and return types.
  • Security issues have been raised by architects that indicate that this pattern may give a false sense of security, as the strings in the TrustAwareString objects can be manipulated by static casts, and are therefore unreliable.

Description

  • Stripping out all TrustService, TrustAwareString, IsSensitive and related references from the SK.Core and Abstractions packages.

**These will be added back in the form of a new SemanticKernel.Extensions.Trust namespace with optional packages and more features, including an auditable trust chain.

Contribution Checklist

@shawncal shawncal requested a review from dmytrostruk July 7, 2023 16:26
@shawncal shawncal requested a review from a team as a code owner July 7, 2023 16:26
@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel kernel.core labels Jul 7, 2023
dmytrostruk
dmytrostruk previously approved these changes Jul 7, 2023
Copy link
Member

@dmytrostruk dmytrostruk left a comment

Choose a reason for hiding this comment

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

LGTM!

@shawncal shawncal enabled auto-merge July 7, 2023 16:52
@dmytrostruk dmytrostruk added the PR: ready to merge PR has been approved by all reviewers, and is ready to merge. label Jul 7, 2023
@shawncal shawncal added this pull request to the merge queue Jul 7, 2023
Merged via the queue into microsoft:main with commit a6c3cd1 Jul 7, 2023
10 checks passed
@shawncal shawncal deleted the trustservice-removal2 branch July 7, 2023 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code PR: ready to merge PR has been approved by all reviewers, and is ready to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants