Skip to content

Commit 0d2cb75

Browse files
BenjaminMichaelisCopilot
BenjaminMichaelis
and
Copilot
authored
fix: OWASP AI Agent Security hardening for chat and MCP endpoints (#1073)
## Summary Implements all actionable findings from an OWASP AI Agent Security Cheat Sheet review of the chat and MCP subsystems. The implementation was reviewed in two rounds by GPT-5.5 and Claude Opus 4.6, which found and resolved additional issues before this PR was created. Fixes: #1064 Fixes: #1066 Fixes: #1067 Fixes: #1068 Fixes: #1069 Fixes: #1070 Fixes: #1071 --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: BenjaminMichaelis <22186029+BenjaminMichaelis@users.noreply.github.com>
1 parent f52ca45 commit 0d2cb75

13 files changed

Lines changed: 662 additions & 89 deletions

File tree

‎EssentialCSharp.Chat.Shared/Models/AIOptions.cs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,20 @@ public class AIOptions
2222
/// </summary>
2323
public string Endpoint { get; set; } = string.Empty;
2424

25+
/// <summary>
26+
/// Static allowlist of MCP tool names the AI agent is permitted to invoke.
27+
/// Tools not on this list are neither advertised to the model nor executed.
28+
/// </summary>
29+
/// <remarks>
30+
/// When empty and <see cref="AllowAllMcpTools"/> is <c>false</c> (the default), all MCP tool
31+
/// calls are denied — fail-secure. Set <see cref="AllowAllMcpTools"/> to <c>true</c> to allow
32+
/// all tools without an explicit list (useful in development environments only).
33+
/// </remarks>
34+
public List<string> AllowedMcpTools { get; set; } = [];
35+
36+
/// <summary>
37+
/// When <c>true</c>, bypasses the <see cref="AllowedMcpTools"/> allowlist and permits all
38+
/// MCP tools. Should only be set in non-production environments.
39+
/// </summary>
40+
public bool AllowAllMcpTools { get; set; }
2541
}

‎EssentialCSharp.Chat.Shared/Services/AIChatService.cs‎

Lines changed: 146 additions & 24 deletions
Large diffs are not rendered by default.

‎EssentialCSharp.Chat/Program.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,7 @@ static int Main(string[] args)
218218
var fullResponse = new System.Text.StringBuilder();
219219

220220
await foreach (var (text, responseId) in aiChatService.GetChatCompletionStream(
221-
prompt: userInput/*, mcpClient: mcpClient*/, previousResponseId: previousResponseId, systemPrompt: customSystemPrompt, cancellationToken: cancellationToken))
221+
prompt: userInput/*, mcpClient: mcpClient*/, previousResponseId: previousResponseId, systemPrompt: customSystemPrompt, endUserId: "console-local", cancellationToken: cancellationToken))
222222
{
223223
if (!string.IsNullOrEmpty(text))
224224
{
@@ -238,7 +238,7 @@ static int Main(string[] args)
238238
{
239239
// Non-streaming response with optional tools and conversation context
240240
var (response, responseId) = await aiChatService.GetChatCompletion(
241-
prompt: userInput, previousResponseId: previousResponseId, systemPrompt: customSystemPrompt, cancellationToken: cancellationToken);
241+
prompt: userInput, previousResponseId: previousResponseId, systemPrompt: customSystemPrompt, endUserId: "console-local", cancellationToken: cancellationToken);
242242

243243
Console.WriteLine(response);
244244
conversationHistory.Add(("Assistant", response));

‎EssentialCSharp.Web.Tests/McpApiTokenServiceTests.cs‎

Lines changed: 119 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using EssentialCSharp.Web.Models;
12
using EssentialCSharp.Web.Services;
23
using Microsoft.AspNetCore.Mvc.Testing;
34
using Microsoft.Extensions.DependencyInjection;
@@ -8,13 +9,41 @@ namespace EssentialCSharp.Web.Tests;
89
[ClassDataSource<WebApplicationFactory>(Shared = SharedType.PerClass)]
910
public class McpApiTokenServiceTests(WebApplicationFactory factory)
1011
{
11-
[Test]
12-
public async Task CreateTokenAsync_WithoutExpiry_UsesSixMonthDefault()
12+
private readonly List<IServiceScope> _scopes = [];
13+
14+
[After(Test)]
15+
public void DisposeScopes()
1316
{
14-
string userId = await McpTestHelper.CreateUserAsync(factory, "mcp-default-expiry");
17+
foreach (var scope in _scopes)
18+
scope.Dispose();
19+
_scopes.Clear();
20+
}
21+
22+
private async Task<(string UserId, McpApiTokenService TokenService)> ArrangeAsync(string prefix)
23+
{
24+
string userId = await McpTestHelper.CreateUserAsync(factory, prefix);
25+
var scope = factory.Services.CreateScope();
26+
_scopes.Add(scope);
27+
var tokenService = scope.ServiceProvider.GetRequiredService<McpApiTokenService>();
28+
return (userId, tokenService);
29+
}
1530

16-
using var scope = factory.Services.CreateScope();
31+
private async Task<McpApiTokenService> FillToLimitAsync(string userId)
32+
{
33+
var scope = factory.Services.CreateScope();
34+
_scopes.Add(scope);
1735
var tokenService = scope.ServiceProvider.GetRequiredService<McpApiTokenService>();
36+
for (int i = 0; i < McpApiTokenService.MaxTokensPerUser; i++)
37+
{
38+
await tokenService.CreateTokenAsync(userId, $"token-{i}");
39+
}
40+
return tokenService;
41+
}
42+
43+
[Test]
44+
public async Task CreateTokenAsync_WithoutExpiry_UsesSixMonthDefault()
45+
{
46+
var (userId, tokenService) = await ArrangeAsync("mcp-default-expiry");
1847

1948
(_, var entity) = await tokenService.CreateTokenAsync(userId, "default-expiry");
2049

@@ -26,10 +55,7 @@ await Assert.That(entity.ExpiresAt!.Value)
2655
[Test]
2756
public async Task CreateTokenAsync_WithExpiryWithinSixMonths_UsesRequestedExpiry()
2857
{
29-
string userId = await McpTestHelper.CreateUserAsync(factory, "mcp-custom-expiry");
30-
31-
using var scope = factory.Services.CreateScope();
32-
var tokenService = scope.ServiceProvider.GetRequiredService<McpApiTokenService>();
58+
var (userId, tokenService) = await ArrangeAsync("mcp-custom-expiry");
3359
DateTime requestedExpiry = DateTime.UtcNow.AddMonths(3);
3460

3561
(_, var entity) = await tokenService.CreateTokenAsync(userId, "custom-expiry", requestedExpiry);
@@ -41,10 +67,7 @@ public async Task CreateTokenAsync_WithExpiryWithinSixMonths_UsesRequestedExpiry
4167
[Test]
4268
public async Task CreateTokenAsync_WithExpiryBeyondSixMonths_Throws()
4369
{
44-
string userId = await McpTestHelper.CreateUserAsync(factory, "mcp-max-expiry");
45-
46-
using var scope = factory.Services.CreateScope();
47-
var tokenService = scope.ServiceProvider.GetRequiredService<McpApiTokenService>();
70+
var (userId, tokenService) = await ArrangeAsync("mcp-max-expiry");
4871
DateTime requestedExpiry = McpApiTokenService.GetDefaultExpirationUtc(DateTime.UtcNow).AddDays(2);
4972

5073
await Assert.That(() => tokenService.CreateTokenAsync(userId, "too-long", requestedExpiry))
@@ -55,20 +78,97 @@ await Assert.That(() => tokenService.CreateTokenAsync(userId, "too-long", reques
5578
[Test]
5679
public async Task CreateTokenAsync_WithExplicitCreatedAt_UsesReferenceTimeForDefaultExpiry()
5780
{
58-
string userId = await McpTestHelper.CreateUserAsync(factory, "mcp-explicit-created-at");
59-
60-
using var scope = factory.Services.CreateScope();
61-
var tokenService = scope.ServiceProvider.GetRequiredService<McpApiTokenService>();
81+
var (userId, tokenService) = await ArrangeAsync("mcp-explicit-created-at");
6282
DateTime createdAtUtc = new(2026, 4, 30, 23, 59, 59, DateTimeKind.Utc);
6383

6484
(_, var entity) = await tokenService.CreateTokenAsync(
65-
userId,
66-
"explicit-created-at",
67-
createdAtUtc: createdAtUtc);
85+
userId, "explicit-created-at", createdAtUtc: createdAtUtc);
6886

6987
await Assert.That(entity.CreatedAt).IsEqualTo(createdAtUtc);
7088
await Assert.That(entity.ExpiresAt).IsNotNull();
7189
await Assert.That(entity.ExpiresAt!.Value)
7290
.IsEqualTo(McpApiTokenService.GetDefaultExpirationUtc(createdAtUtc));
7391
}
92+
93+
[Test]
94+
public async Task GetActiveTokenCountAsync_NoTokens_ReturnsZero()
95+
{
96+
var (userId, tokenService) = await ArrangeAsync("mcp-count-zero");
97+
98+
int count = await tokenService.GetActiveTokenCountAsync(userId);
99+
100+
await Assert.That(count).IsEqualTo(0);
101+
}
102+
103+
[Test]
104+
public async Task GetActiveTokenCountAsync_ActiveTokens_CountsAll()
105+
{
106+
var (userId, tokenService) = await ArrangeAsync("mcp-count-active");
107+
108+
await tokenService.CreateTokenAsync(userId, "token-1");
109+
await tokenService.CreateTokenAsync(userId, "token-2");
110+
await tokenService.CreateTokenAsync(userId, "token-3");
111+
112+
int count = await tokenService.GetActiveTokenCountAsync(userId);
113+
114+
await Assert.That(count).IsEqualTo(3);
115+
}
116+
117+
[Test]
118+
public async Task GetActiveTokenCountAsync_RevokedToken_ExcludedFromCount()
119+
{
120+
var (userId, tokenService) = await ArrangeAsync("mcp-count-revoked");
121+
122+
await tokenService.CreateTokenAsync(userId, "active-token");
123+
(_, var revokedEntity) = await tokenService.CreateTokenAsync(userId, "revoked-token");
124+
await tokenService.RevokeTokenAsync(revokedEntity.Id, userId);
125+
126+
int count = await tokenService.GetActiveTokenCountAsync(userId);
127+
128+
await Assert.That(count).IsEqualTo(1);
129+
}
130+
131+
[Test]
132+
public async Task GetActiveTokenCountAsync_ExpiredToken_ExcludedFromCount()
133+
{
134+
var (userId, tokenService) = await ArrangeAsync("mcp-count-expired");
135+
136+
// createdAt 7 months ago → max expiry = 1 month ago; use 2 months ago as expiresAt
137+
DateTime createdAt = DateTime.UtcNow.AddMonths(-7);
138+
DateTime pastExpiry = DateTime.UtcNow.AddMonths(-2);
139+
await tokenService.CreateTokenAsync(userId, "expired-token",
140+
expiresAt: pastExpiry, createdAtUtc: createdAt);
141+
await tokenService.CreateTokenAsync(userId, "valid-token");
142+
143+
int count = await tokenService.GetActiveTokenCountAsync(userId);
144+
145+
await Assert.That(count).IsEqualTo(1);
146+
}
147+
148+
[Test]
149+
public async Task CreateTokenAsync_AtMaxLimit_ThrowsTokenLimitExceededException()
150+
{
151+
var (userId, _) = await ArrangeAsync("mcp-at-limit");
152+
var tokenService = await FillToLimitAsync(userId);
153+
154+
await Assert.That(() => tokenService.CreateTokenAsync(userId, "one-too-many"))
155+
.Throws<TokenLimitExceededException>();
156+
}
157+
158+
[Test]
159+
public async Task CreateTokenAsync_AfterRevokingAtLimit_AllowsNewToken()
160+
{
161+
var (userId, _) = await ArrangeAsync("mcp-revoke-then-create");
162+
var tokenService = await FillToLimitAsync(userId);
163+
164+
// Revoke the last token to free a slot
165+
var tokens = await tokenService.GetUserTokensAsync(userId);
166+
await tokenService.RevokeTokenAsync(tokens[0].Id, userId);
167+
168+
// Should now succeed — active count dropped below max
169+
(_, var newEntity) = await tokenService.CreateTokenAsync(userId, "replacement");
170+
await Assert.That(newEntity).IsNotNull();
171+
int activeCount = await tokenService.GetActiveTokenCountAsync(userId);
172+
await Assert.That(activeCount).IsEqualTo(McpApiTokenService.MaxTokensPerUser);
173+
}
74174
}

‎EssentialCSharp.Web.Tests/ResponseIdValidationServiceTests.cs‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
using EssentialCSharp.Web.Services;
2+
using Microsoft.Extensions.Caching.Memory;
3+
4+
namespace EssentialCSharp.Web.Tests;
5+
6+
public class ResponseIdValidationServiceTests
7+
{
8+
// Match production SizeLimit so SetSize(1) is exercised in tests, not silently ignored.
9+
private static MemoryCache CreateCache() => new(new MemoryCacheOptions { SizeLimit = 10_000 });
10+
11+
private static ResponseIdValidationService CreateService(MemoryCache cache) => new(cache);
12+
13+
[Test]
14+
[Arguments(null)]
15+
[Arguments("")]
16+
public async Task ValidateResponseId_BlankResponseId_AllowsNewConversation(string? responseId)
17+
{
18+
using var cache = CreateCache();
19+
var service = CreateService(cache);
20+
21+
bool result = service.ValidateResponseId("user1", responseId);
22+
23+
await Assert.That(result).IsTrue();
24+
}
25+
26+
[Test]
27+
[Arguments(null)]
28+
[Arguments("")]
29+
public async Task ValidateResponseId_BlankUserId_Rejects(string? userId)
30+
{
31+
using var cache = CreateCache();
32+
var service = CreateService(cache);
33+
34+
bool result = service.ValidateResponseId(userId, "resp_123");
35+
36+
await Assert.That(result).IsFalse();
37+
}
38+
39+
[Test]
40+
public async Task ValidateResponseId_CacheMiss_AllowsGracefulDegradation()
41+
{
42+
using var cache = CreateCache();
43+
var service = CreateService(cache);
44+
// No RecordResponseId call — simulate server restart / different instance
45+
46+
bool result = service.ValidateResponseId("user1", "resp_unknown");
47+
48+
await Assert.That(result).IsTrue();
49+
}
50+
51+
[Test]
52+
public async Task ValidateResponseId_RecordedByOwner_Validates()
53+
{
54+
using var cache = CreateCache();
55+
var service = CreateService(cache);
56+
service.RecordResponseId("user1", "resp_abc");
57+
58+
bool result = service.ValidateResponseId("user1", "resp_abc");
59+
60+
await Assert.That(result).IsTrue();
61+
}
62+
63+
[Test]
64+
public async Task ValidateResponseId_RecordedByDifferentUser_Rejects()
65+
{
66+
using var cache = CreateCache();
67+
var service = CreateService(cache);
68+
service.RecordResponseId("user1", "resp_abc");
69+
70+
bool result = service.ValidateResponseId("user2", "resp_abc");
71+
72+
await Assert.That(result).IsFalse();
73+
}
74+
75+
[Test]
76+
public async Task RecordResponseId_NullInputs_DoesNotThrow()
77+
{
78+
using var cache = CreateCache();
79+
var service = CreateService(cache);
80+
81+
service.RecordResponseId(null, "resp_abc");
82+
service.RecordResponseId("user1", null);
83+
service.RecordResponseId(null, null);
84+
85+
// Verify the service is still functional after no-op calls
86+
service.RecordResponseId("user1", "resp_abc");
87+
await Assert.That(service.ValidateResponseId("user1", "resp_abc")).IsTrue();
88+
}
89+
90+
[Test]
91+
public async Task ValidateResponseId_MultipleResponseIds_EachValidatedIndependently()
92+
{
93+
using var cache = CreateCache();
94+
var service = CreateService(cache);
95+
service.RecordResponseId("user1", "resp_001");
96+
service.RecordResponseId("user1", "resp_002");
97+
98+
await Assert.That(service.ValidateResponseId("user1", "resp_001")).IsTrue();
99+
await Assert.That(service.ValidateResponseId("user1", "resp_002")).IsTrue();
100+
// Unrecorded ID for same user → cache miss → allow
101+
await Assert.That(service.ValidateResponseId("user1", "resp_003")).IsTrue();
102+
}
103+
104+
[Test]
105+
public async Task ValidateResponseId_TwoUsers_IsolatedFromEachOther()
106+
{
107+
using var cache = CreateCache();
108+
var service = CreateService(cache);
109+
service.RecordResponseId("user1", "resp_A");
110+
service.RecordResponseId("user2", "resp_B");
111+
112+
await Assert.That(service.ValidateResponseId("user1", "resp_A")).IsTrue();
113+
await Assert.That(service.ValidateResponseId("user2", "resp_B")).IsTrue();
114+
await Assert.That(service.ValidateResponseId("user2", "resp_A")).IsFalse();
115+
await Assert.That(service.ValidateResponseId("user1", "resp_B")).IsFalse();
116+
}
117+
118+
[Test]
119+
public async Task RecordResponseId_SizeLimitEnforced_EntryCountedInCache()
120+
{
121+
using var cache = CreateCache();
122+
var service = CreateService(cache);
123+
124+
// Record an entry — with SizeLimit set, SetSize(1) should count toward the cache size.
125+
service.RecordResponseId("user1", "resp_size_test");
126+
127+
// Verify it was recorded (i.e., not silently evicted due to misconfiguration).
128+
await Assert.That(service.ValidateResponseId("user1", "resp_size_test")).IsTrue();
129+
}
130+
}

0 commit comments

Comments
 (0)