diff --git a/src/CrestApps.Core.Docs/docs/changelog/v1.0.0.md b/src/CrestApps.Core.Docs/docs/changelog/v1.0.0.md index 96c4ec14..e03a1132 100644 --- a/src/CrestApps.Core.Docs/docs/changelog/v1.0.0.md +++ b/src/CrestApps.Core.Docs/docs/changelog/v1.0.0.md @@ -119,3 +119,4 @@ description: Initial standalone release notes for the CrestApps.Core repository. - requires the key, title, and content field mappings when creating or editing an AI data source in the MVC and Blazor sample hosts, and makes every built-in source reader fall back to the document key instead of the serialized source document when no title is mapped, so chat citations never render a full JSON document as a reference title - makes the Copilot CLI acquisition work behind corporate proxies and artifact mirrors, and downloads it only once per machine: `CrestApps.Core.AI.Copilot` now resolves the effective npm registry from `NPM_CONFIG_REGISTRY` or `npm config get registry` before the `GitHub.Copilot.SDK` targets download the CLI tarball (the SDK hardcodes `https://registry.npmjs.org`, and MSBuild's `DownloadFile` task cannot read npm configuration), and redirects the SDK's per-project, per-configuration cache to a shared cache under the NuGet global packages folder so a multi-project solution, a fresh worktree, or a CI agent no longer re-downloads the same large tarball for every project; both behaviors are opt-out through `CopilotResolveNpmRegistry` and `CopilotUseSharedCliCache`, the cache location is configurable through `CopilotCliCacheDir` (point it at a pre-seeded directory to build offline), and an explicitly set `CopilotNpmRegistryUrl`, `CopilotCliBinaryPath`, or `CopilotSkipCliDownload` always takes precedence - lets post-session processing invoke parameterized AI tool instances through the new `AIProfilePostSessionSettings.ToolInstanceNames` and `PostSessionTask.ToolInstanceNames`, merged and forwarded to the tool registry alongside the equivalent `ToolNames` so configuring only tool instances is enough to enable the tool-driven post-session path, and surfaces the per-task selection on the **Capabilities** tab of each post-session task in the AI profile create and edit screens of both the MVC and Blazor sample hosts +- reports AI tools that were excluded from a completion because the current user is not authorized for them with a single `Warning` log entry per request instead of a `Debug`-only entry, so an answer that silently lost its tools is now traceable in the default logs, and corrects the documented `IAIToolAccessEvaluator` contract to match the implemented `IsAuthorizedAsync(ClaimsPrincipal user, string toolName)` signature diff --git a/src/CrestApps.Core.Docs/docs/core/tools.md b/src/CrestApps.Core.Docs/docs/core/tools.md index 70b7d81a..be55f102 100644 --- a/src/CrestApps.Core.Docs/docs/core/tools.md +++ b/src/CrestApps.Core.Docs/docs/core/tools.md @@ -133,18 +133,19 @@ builder.Services ### `IAIToolAccessEvaluator` -Override this to control which tools are available in a given context: +Override this to control which tools the current user is allowed to invoke: ```csharp public interface IAIToolAccessEvaluator { - ValueTask IsAccessibleAsync( - AIToolMetadataEntry tool, - AICompletionContext context, - CancellationToken cancellationToken = default); + Task IsAuthorizedAsync(ClaimsPrincipal user, string toolName); } ``` +The default implementation permits every tool. Hosts that enforce permissions, such as the Orchard Core integration, replace it with an authorization-aware implementation. + +Tools the user is not authorized for are excluded from the request instead of failing it, so the model simply answers without that capability. Because a missing tool permission usually looks like an incomplete answer, every excluded tool is reported once per request with a `Warning` log entry that lists the denied tool names. + ## Custom Tool Registry Provider Supply tools from an external source (database, API, etc.): diff --git a/src/Primitives/CrestApps.Core.AI/Handlers/FunctionInvocationAICompletionServiceHandler.cs b/src/Primitives/CrestApps.Core.AI/Handlers/FunctionInvocationAICompletionServiceHandler.cs index 146f0242..45c7e395 100644 --- a/src/Primitives/CrestApps.Core.AI/Handlers/FunctionInvocationAICompletionServiceHandler.cs +++ b/src/Primitives/CrestApps.Core.AI/Handlers/FunctionInvocationAICompletionServiceHandler.cs @@ -65,6 +65,7 @@ entriesObj is not IReadOnlyList scopedEntries || var user = _httpContextAccessor.HttpContext?.User; var addedNames = new HashSet(StringComparer.OrdinalIgnoreCase); + List deniedToolNames = null; // Snapshot a stable partition before authorization or factory callbacks can mutate the source. var orderedEntries = new ToolRegistryEntry[scopedEntries.Count]; @@ -94,6 +95,8 @@ entriesObj is not IReadOnlyList scopedEntries || // unauthorized invocation via prompt injection. if (!await _toolAccessEvaluator.IsAuthorizedAsync(user, entry.Name)) { + (deniedToolNames ??= []).Add(entry.Name); + if (_logger.IsEnabled(LogLevel.Debug)) { _logger.LogDebug( @@ -116,7 +119,7 @@ entriesObj is not IReadOnlyList scopedEntries || if (_logger.IsEnabled(LogLevel.Debug)) { _logger.LogDebug( - "Skipping tool '{ToolName}' from {Source} ({Id}) ? name already registered.", + "Skipping tool '{ToolName}' from {Source} ({Id}) - name already registered.", entry.Name, entry.Source, entry.Id); } @@ -141,5 +144,14 @@ entriesObj is not IReadOnlyList scopedEntries || _logger.LogError(ex, "Failed to create tool '{ToolName}' ({Id}). Skipping.", entry.Name, entry.Id); } } + + if (deniedToolNames is not null) + { + // Surface the denial above Debug level. Otherwise the caller only sees a degraded + // answer with no indication that the configured tools were removed. + _logger.LogWarning( + "The current user is not authorized to use the following AI tools, which were excluded from the request: {ToolNames}.", + string.Join(", ", deniedToolNames)); + } } } diff --git a/tests/CrestApps.Core.Tests/Core/Orchestration/FunctionInvocationAICompletionServiceHandlerTests.cs b/tests/CrestApps.Core.Tests/Core/Orchestration/FunctionInvocationAICompletionServiceHandlerTests.cs index c3724d58..c93c49d8 100644 --- a/tests/CrestApps.Core.Tests/Core/Orchestration/FunctionInvocationAICompletionServiceHandlerTests.cs +++ b/tests/CrestApps.Core.Tests/Core/Orchestration/FunctionInvocationAICompletionServiceHandlerTests.cs @@ -4,6 +4,7 @@ using CrestApps.Core.AI.Tooling; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.AI; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; namespace CrestApps.Core.Tests.Core.Orchestration; @@ -87,6 +88,39 @@ public async Task ConfigureAsync_SnapshotsEntriesBeforeInvokingFactories() Assert.Equal(3, entries.Count); } + [Fact] + public async Task ConfigureAsync_WhenToolsAreDenied_ExcludesThemAndLogsASingleWarning() + { + var evaluator = new RecordingToolAccessEvaluator + { + DeniedToolNames = { "denied-first", "denied-second" }, + }; + var allowedTool = new TestAIFunction("allowed-tool"); + IReadOnlyList entries = + [ + CreateEntry("denied-first", "denied-first", ToolRegistryEntrySource.Local, new TestAIFunction("denied-first-tool")), + CreateEntry("allowed", "allowed", ToolRegistryEntrySource.Local, allowedTool), + CreateEntry("denied-second", "denied-second", ToolRegistryEntrySource.McpServer, new TestAIFunction("denied-second-tool")), + ]; + var completionContext = new AICompletionContext(); + completionContext.AdditionalProperties[FunctionInvocationAICompletionServiceHandler.ScopedEntriesKey] = entries; + var context = new CompletionServiceConfigureContext(new ChatOptions(), completionContext, true); + var logger = new CapturingLogger(); + var handler = new FunctionInvocationAICompletionServiceHandler( + evaluator, + new HttpContextAccessor(), + new EmptyServiceProvider(), + logger); + + await handler.ConfigureAsync(context, TestContext.Current.CancellationToken); + + Assert.Equal([allowedTool], context.ChatOptions.Tools); + + var warning = Assert.Single(logger.Messages, message => message.Level == LogLevel.Warning); + Assert.Contains("denied-first", warning.Message); + Assert.Contains("denied-second", warning.Message); + } + private static ToolRegistryEntry CreateEntry( string id, string name, @@ -106,11 +140,34 @@ private sealed class RecordingToolAccessEvaluator : IAIToolAccessEvaluator { public List ToolNames { get; } = []; + public HashSet DeniedToolNames { get; } = new(StringComparer.OrdinalIgnoreCase); + public Task IsAuthorizedAsync(ClaimsPrincipal user, string toolName) { ToolNames.Add(toolName); - return Task.FromResult(true); + return Task.FromResult(!DeniedToolNames.Contains(toolName)); + } + } + + private sealed class CapturingLogger : ILogger + { + public List<(LogLevel Level, string Message)> Messages { get; } = []; + + public IDisposable BeginScope(TState state) + where TState : notnull + => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception exception, + Func formatter) + { + Messages.Add((logLevel, formatter(state, exception))); } }