Skip to content

Commit

Permalink
Removing TrustService from Kernel.Core (to become extension) (#1895)
Browse files Browse the repository at this point in the history
### 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
<!-- Before submitting this PR, please make sure: -->
- [X] The code builds clean without any errors or warnings
- [X] The PR follows SK Contribution Guidelines
(https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
- [X] The code follows the .NET coding conventions
(https://learn.microsoft.com/dotnet/csharp/fundamentals/coding-style/coding-conventions)
verified with `dotnet format`
- [X] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄

---------

Co-authored-by: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com>
  • Loading branch information
shawncal and dmytrostruk committed Jul 7, 2023
1 parent 6b3e46d commit a6c3cd1
Show file tree
Hide file tree
Showing 33 changed files with 90 additions and 3,500 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using Microsoft.SemanticKernel.Memory;
using Microsoft.SemanticKernel.Orchestration;
using Microsoft.SemanticKernel.Planning;
using Microsoft.SemanticKernel.Security;
using Microsoft.SemanticKernel.SemanticFunctions;
using Microsoft.SemanticKernel.SkillDefinition;
using Moq;
Expand Down Expand Up @@ -144,8 +143,7 @@ private Mock<IKernel> CreateMockKernelAndFunctionFlowWithTestString(string testP
kernel.Setup(x => x.RegisterSemanticFunction(
It.IsAny<string>(),
It.IsAny<string>(),
It.IsAny<SemanticFunctionConfig>(),
It.IsAny<ITrustService?>()
It.IsAny<SemanticFunctionConfig>()
)).Returns(mockFunctionFlowFunction.Object);

return kernel;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using Microsoft.SemanticKernel.Orchestration;
using Microsoft.SemanticKernel.Planning;
using Microsoft.SemanticKernel.Planning.Sequential;
using Microsoft.SemanticKernel.Security;
using Microsoft.SemanticKernel.SemanticFunctions;
using Microsoft.SemanticKernel.SkillDefinition;
using Moq;
Expand Down Expand Up @@ -86,8 +85,7 @@ private void CreateKernelAndFunctionCreateMocks(List<(string name, string skillN
kernelMock.Setup(x => x.RegisterSemanticFunction(
It.IsAny<string>(),
It.IsAny<string>(),
It.IsAny<SemanticFunctionConfig>(),
It.IsAny<ITrustService?>()
It.IsAny<SemanticFunctionConfig>()
)).Returns(mockFunction.Object);
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using Microsoft.SemanticKernel.Memory;
using Microsoft.SemanticKernel.Orchestration;
using Microsoft.SemanticKernel.Planning;
using Microsoft.SemanticKernel.Security;
using Microsoft.SemanticKernel.SemanticFunctions;
using Microsoft.SemanticKernel.SkillDefinition;
using Moq;
Expand Down Expand Up @@ -104,8 +103,7 @@ public async Task ItCanCreatePlanAsync(string goal)
kernel.Setup(x => x.RegisterSemanticFunction(
It.IsAny<string>(),
It.IsAny<string>(),
It.IsAny<SemanticFunctionConfig>(),
It.IsAny<ITrustService?>()
It.IsAny<SemanticFunctionConfig>()
)).Returns(mockFunctionFlowFunction.Object);

var planner = new Microsoft.SemanticKernel.Planning.SequentialPlanner(kernel.Object);
Expand Down Expand Up @@ -192,8 +190,7 @@ public async Task InvalidXMLThrowsAsync()
kernel.Setup(x => x.RegisterSemanticFunction(
It.IsAny<string>(),
It.IsAny<string>(),
It.IsAny<SemanticFunctionConfig>(),
It.IsAny<ITrustService?>()
It.IsAny<SemanticFunctionConfig>()
)).Returns(mockFunctionFlowFunction.Object);

var planner = new Microsoft.SemanticKernel.Planning.SequentialPlanner(kernel.Object);
Expand Down
4 changes: 0 additions & 4 deletions dotnet/src/IntegrationTests/Planning/PlanTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ public async Task CanExecutePanWithTreeStepsAsync()
}

[Theory]
[InlineData(null, "Write a poem or joke and send it in an e-mail to Kai.", null)]
[InlineData("", "Write a poem or joke and send it in an e-mail to Kai.", "")]
[InlineData("Hello World!", "Write a poem or joke and send it in an e-mail to Kai.", "some_email@email.com")]
public async Task CanExecuteRunPlanSimpleManualStateAsync(string input, string goal, string email)
Expand Down Expand Up @@ -180,7 +179,6 @@ public async Task CanExecuteRunPlanSimpleManualStateAsync(string input, string g
}

[Theory]
[InlineData(null, "Write a poem or joke and send it in an e-mail to Kai.", null)]
[InlineData("", "Write a poem or joke and send it in an e-mail to Kai.", "")]
[InlineData("Hello World!", "Write a poem or joke and send it in an e-mail to Kai.", "some_email@email.com")]
public async Task CanExecuteRunPlanSimpleManualStateNoVariableAsync(string input, string goal, string email)
Expand Down Expand Up @@ -214,14 +212,12 @@ public async Task CanExecuteRunPlanSimpleManualStateNoVariableAsync(string input
}

[Theory]
[InlineData(null, "Write a poem or joke and send it in an e-mail to Kai.", null)]
[InlineData("", "Write a poem or joke and send it in an e-mail to Kai.", "")]
[InlineData("Hello World!", "Write a poem or joke and send it in an e-mail to Kai.", "some_email@email.com")]
public async Task CanExecuteRunPlanManualStateAsync(string input, string goal, string email)
{
// Arrange
IKernel target = this.InitializeKernel();

var emailSkill = target.ImportSkill(new EmailSkillFake());

// Create the input mapping from parent (plan) plan state to child plan (sendEmailPlan) state.
Expand Down
Loading

0 comments on commit a6c3cd1

Please sign in to comment.