From b80fbd5ff3b45eee80ca38103d73a761a47e9ec8 Mon Sep 17 00:00:00 2001 From: MCKRUZ Date: Fri, 11 Sep 2026 21:55:04 -0400 Subject: [PATCH 1/4] fix: resolve envelope grant/declared-tool published names (#626) EnvelopePermissionRuleProvider built its Deny and autonomy-ceiling baseline rules straight from an envelope's declared AllowedTools grants and a bundle's declared tool names, without ever resolving a tool's self-reported published name -- the value ThreePhasePermissionResolver.Matches actually compares a rule's pattern against at invocation. An operator-authored grant naming a tool by its DI key (rather than its published name) silently never matched at invocation, so the tool fell through to the closing catch-all Deny despite being "granted" in config -- a real functional defect masked as fail-closed-by-accident. Mirrors #612's fix for PluginPermissionRuleProvider's DeniedTools: each grant and declared-tool name is expanded to also include its resolved published name (via the existing FirstPartyToolLookup) when it differs. Mutation testing caught a real bug in the first cut of this fix: the declared-tools Deny-check tested each expanded name-form independently against the granted set, so a tool declared by key but granted by its published name (or vice versa) was denied under whichever form wasn't the literal grant string -- even though the tool IS genuinely granted under the other form. Fixed by deciding grant membership once per original declared tool (checking whether ANY of its forms is granted) before emitting a Deny for any of them. #625 (the same pattern suggested for PluginPermissionRuleProvider's autonomy-baseline rules) was investigated and closed as not applicable: that method's own "own-surface constraint" already excludes any name resolvable via keyed DI before it reaches the rule loop, so the exact divergence this fix resolves cannot occur there -- confirmed by a test that produced zero rules before being reverted. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Aao7Y3hU22v6VH1RdiSxYu --- .../EnvelopePermissionRuleProvider.cs | 104 +++++++++++++++++- .../EnvelopePermissionRuleProviderTests.cs | 78 ++++++++++++- .../EnvelopeEnforcementIntegrationTests.cs | 13 ++- .../SubPlanEnvelopeConfinementTests.cs | 5 +- 4 files changed, 188 insertions(+), 12 deletions(-) diff --git a/src/Content/Application/Application.Core/Permissions/EnvelopePermissionRuleProvider.cs b/src/Content/Application/Application.Core/Permissions/EnvelopePermissionRuleProvider.cs index 622bb3920..c8f67b5e1 100644 --- a/src/Content/Application/Application.Core/Permissions/EnvelopePermissionRuleProvider.cs +++ b/src/Content/Application/Application.Core/Permissions/EnvelopePermissionRuleProvider.cs @@ -1,6 +1,7 @@ using Application.AI.Common.Interfaces.Permissions; using Application.AI.Common.Services.Bundles; using Application.AI.Common.Services.Governance; +using Application.AI.Common.Services.Tools; using Domain.AI.Bundles; using Domain.AI.Governance; using Domain.AI.Permissions; @@ -98,14 +99,21 @@ public sealed class EnvelopePermissionRuleProvider : IPermissionRuleProvider private const int BaselinePriority = 5; private readonly ILogger _logger; + private readonly FirstPartyToolLookup _firstPartyToolLookup; /// /// Initializes a new instance of the class. /// /// Logger for rejected wildcard grants in an envelope's allowlist. - public EnvelopePermissionRuleProvider(ILogger logger) + /// + /// Resolves a granted/declared name's self-reported published name for #626 — see + /// . + /// + public EnvelopePermissionRuleProvider( + ILogger logger, FirstPartyToolLookup firstPartyToolLookup) { _logger = logger; + _firstPartyToolLookup = firstPartyToolLookup; } /// @@ -121,17 +129,35 @@ public Task> GetRulesAsync( return Task.FromResult>([]); var rules = new List(); - var grantedNames = ValidGrants(envelope); + + // #626: both the envelope's own grants and a bundle's declared tools are authored the same + // DI-key-shaped way #612 found for a plugin's DeniedTools — expanding each name to also + // include its resolved, self-reported published name (when it differs) before either loop + // below consumes it closes the identical failure shape: without this, an operator-authored + // grant naming a tool by its DI key silently never matches at invocation (ThreePhasePermissionResolver.Matches + // compares against the published name), so the tool falls through to the closing Deny despite + // being "granted" in config — a real functional defect masked as fail-closed-by-accident (see + // this type's remarks on the closing Deny). + var grantedNames = ExpandWithPublishedNameCoverage(ValidGrants(envelope)); // 1. Bypass-immune Deny for each declared tool the envelope does not grant. Build the grant set once // so the membership test is O(1) per declared tool rather than a linear scan of the allowlist. + // #626: each declared tool's key-and-published forms are checked for grant membership TOGETHER + // (a single decision per tool), not independently — checking them independently found a real + // bug during mutation testing: a tool declared by key and granted by its published name (or + // vice versa) was denied under whichever one of its two forms wasn't the literal grant string, + // even though the tool IS genuinely granted under the other form. var granted = new HashSet(grantedNames, StringComparer.OrdinalIgnoreCase); - foreach (var toolName in EnumerateDeclaredTools(agentId)) + foreach (var declaredName in EnumerateDeclaredTools(agentId)) { - if (!granted.Contains(toolName)) + var declaredForms = WithPublishedNameForms(declaredName); + if (declaredForms.Any(granted.Contains)) + continue; + + foreach (var form in declaredForms) { rules.Add(new ToolPermissionRule( - toolName, + form, null, PermissionBehaviorType.Deny, PermissionRuleSource.CapabilityEnvelope, @@ -178,6 +204,74 @@ public Task> GetRulesAsync( return Task.FromResult>(rules); } + /// + /// Expands to also include each name's resolved, self-reported published + /// name (#626) via , flattened and deduplicated + /// case-insensitively. Safe only for a set consumed as a flat membership test (the granted-names + /// set, used by 's baseline loop and as the Deny-check's membership + /// target) — not for the declared-tools Deny loop itself, which must decide per ORIGINAL + /// declared tool whether ANY of its forms is granted before emitting a Deny for any of them; see + /// that loop's own comment for the bug this distinction fixes. + /// + private IReadOnlyList ExpandWithPublishedNameCoverage(IReadOnlyCollection names) + { + var expanded = new List(names.Count); + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + + foreach (var name in names) + foreach (var form in WithPublishedNameForms(name)) + if (seen.Add(form)) + expanded.Add(form); + + return expanded; + } + + /// + /// alone, or plus its resolved, self-reported + /// published name when it resolves to a real first-party tool whose name disagrees (#626) — the + /// value ThreePhasePermissionResolver.Matches actually compares a rule's pattern against at + /// invocation, which can legitimately disagree with a DI registration key an envelope grant or a + /// bundle's declared-tools list names it by. + /// + private IReadOnlyList WithPublishedNameForms(string name) + { + if (TryResolvePublishedName(name, out var publishedName) + && !string.Equals(publishedName, name, StringComparison.OrdinalIgnoreCase)) + return [name, publishedName]; + + return [name]; + } + + /// + /// Resolves 's converted, self-reported — + /// see PluginPermissionRuleProvider.TryResolvePublishedName's remarks for the full rationale + /// (#612), shared verbatim here for the envelope provider's identical need (#626). Returns + /// , with set to + /// itself, when the key names no known first-party tool (an MCP tool name, for which no + /// first-party resolution is possible or needed) OR constructing it throws. + /// + private bool TryResolvePublishedName(string toolKey, out string publishedName) + { + var tool = _firstPartyToolLookup.TryResolve(toolKey, out var constructionError); + if (tool is not null) + { + publishedName = tool.Name; + return true; + } + + if (constructionError is not null) + { + _logger.LogError(constructionError, + "Could not construct first-party tool '{ToolKey}' to learn its published name for a " + + "capability-envelope rule — the key-pattern rule for it still applies, but a caller " + + "invoking it under a self-reported name that disagrees with the key would not be covered.", + toolKey); + } + + publishedName = toolKey; + return false; + } + /// /// The tool names an envelope actually grants: its /// entries less any that contain a wildcard, which are rejected with an error log. diff --git a/src/Content/Tests/Application.Core.Tests/Permissions/EnvelopePermissionRuleProviderTests.cs b/src/Content/Tests/Application.Core.Tests/Permissions/EnvelopePermissionRuleProviderTests.cs index f9cd7a05b..cd0129dbc 100644 --- a/src/Content/Tests/Application.Core.Tests/Permissions/EnvelopePermissionRuleProviderTests.cs +++ b/src/Content/Tests/Application.Core.Tests/Permissions/EnvelopePermissionRuleProviderTests.cs @@ -1,5 +1,6 @@ using Application.AI.Common.Services.Bundles; using Application.AI.Common.Services.Governance; +using Application.AI.Common.Services.Tools; using Application.Core.Permissions; using Domain.AI.Agents; using Domain.AI.Bundles; @@ -8,6 +9,7 @@ using Domain.AI.Skills; using Domain.AI.Tools; using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Xunit; @@ -23,7 +25,11 @@ namespace Application.Core.Tests.Permissions; public sealed class EnvelopePermissionRuleProviderTests { private readonly EnvelopePermissionRuleProvider _provider = - new(NullLogger.Instance); + new( + NullLogger.Instance, + // #626: empty key set is fine — no case in this suite names a tool whose published name + // disagrees with its key, so TryResolvePublishedName always falls back to the key itself. + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet())); /// /// The rules written for a specific tool name, i.e. everything except the closing catch-all. Most @@ -207,6 +213,76 @@ public async Task WildcardGrant_IsRejected_AndGrantsNothing() } } + // --- #626: rule.ToolPattern is matched against a tool's PUBLISHED (self-reported) name at + // invocation, but an envelope grant / a bundle's declared tools can legitimately name a tool by + // its DI registration key instead — the identical failure shape #612 fixed for + // PluginPermissionRuleProvider's DeniedTools. Uses its own provider instance (not the shared + // _provider field) so each test can register its own divergent-name tool. + + private static EnvelopePermissionRuleProvider ProviderWithDivergentTool(string key, string publishedName) + { + var services = new ServiceCollection(); + var mock = new Moq.Mock(); + mock.Setup(t => t.Name).Returns(publishedName); + services.AddKeyedSingleton(key, mock.Object); + var lookup = new FirstPartyToolLookup(services.BuildServiceProvider(), new HashSet { key }); + return new EnvelopePermissionRuleProvider(NullLogger.Instance, lookup); + } + + [Fact] + public async Task GrantByKeyDisagreeingWithPublishedName_AlsoEmitsBaselineForThePublishedName() + { + var provider = ProviderWithDivergentTool("registered_key", "self_reported_name"); + var overlay = Overlay(Agent("bundle", "registered_key")); + using (EphemeralAgentOverlayAccessor.Begin(overlay)) + using (CapabilityEnvelopeAccessor.Begin(Envelope(tools: ["registered_key"], ceiling: AutonomyLevel.Autonomous))) + { + var rules = await provider.GetRulesAsync("bundle"); + + PerTool(rules).Should().NotContain(r => r.Behavior == PermissionBehaviorType.Deny, + "the declared tool IS granted (just under a name that disagrees with its published name) " + + "and must not be spuriously denied"); + PerTool(rules).Should().Contain(r => r.ToolPattern == "registered_key" && r.IsAuthoritativeBaseline); + PerTool(rules).Should().Contain(r => r.ToolPattern == "self_reported_name" && r.IsAuthoritativeBaseline, + "without resolving the published name, the grant silently never matches at invocation, " + + "since ThreePhasePermissionResolver.Matches compares against the published name"); + } + } + + [Fact] + public async Task DeclaredByKey_GrantedByPublishedName_IsNotSpuriouslyDenied() + { + // Isolates the DECLARED-tools expansion specifically (distinct from the grant expansion): + // the bundle declares the tool by its DI key, but the operator's grant already used the + // tool's published name directly (a legitimate, arguably more natural way to author a grant). + // Without resolving the DECLARED key to its published name too, "registered_key" is checked + // against a granted set that only contains "self_reported_name" and is wrongly denied, even + // though the tool is genuinely granted. + var provider = ProviderWithDivergentTool("registered_key", "self_reported_name"); + var overlay = Overlay(Agent("bundle", "registered_key")); + using (EphemeralAgentOverlayAccessor.Begin(overlay)) + using (CapabilityEnvelopeAccessor.Begin(Envelope(tools: ["self_reported_name"], ceiling: AutonomyLevel.Autonomous))) + { + var rules = await provider.GetRulesAsync("bundle"); + + PerTool(rules).Should().NotContain(r => r.Behavior == PermissionBehaviorType.Deny, + "the declared tool (by key) IS granted (by published name) and must not be spuriously denied"); + } + } + + [Fact] + public async Task GrantByKeyMatchingPublishedName_EmitsOnlyOneBaselineRule() + { + // No divergence: must not double-emit a redundant rule for the common case. + var provider = ProviderWithDivergentTool("bash", "bash"); + using (CapabilityEnvelopeAccessor.Begin(Envelope(tools: ["bash"], ceiling: AutonomyLevel.Autonomous))) + { + var rules = await provider.GetRulesAsync("bundle"); + + PerTool(rules).Should().ContainSingle(r => r.ToolPattern == "bash"); + } + } + private static AgentDefinition Agent(string id, params string[] allowedTools) => new() { Id = id, Name = id, AllowedTools = allowedTools }; diff --git a/src/Content/Tests/Infrastructure.AI.Tests/Permissions/EnvelopeEnforcementIntegrationTests.cs b/src/Content/Tests/Infrastructure.AI.Tests/Permissions/EnvelopeEnforcementIntegrationTests.cs index 16688065d..674409f6c 100644 --- a/src/Content/Tests/Infrastructure.AI.Tests/Permissions/EnvelopeEnforcementIntegrationTests.cs +++ b/src/Content/Tests/Infrastructure.AI.Tests/Permissions/EnvelopeEnforcementIntegrationTests.cs @@ -130,18 +130,21 @@ private ThreePhasePermissionResolver Resolver( var skillRegistry = new Mock(); skillRegistry.Setup(r => r.GetAll()).Returns(skills ?? []); + // #524 round-2 / #626: empty key set is fine — pluginRegistry never configures + // GetBoundaryStatus, so Moq's default (PluginBoundaryStatus.Verified) means the + // blanket-deny path this lookup feeds never fires here, and no case in this suite names a + // tool whose published name disagrees with its key. + var firstPartyToolLookup = new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()); + IPermissionRuleProvider[] providers = [ new AutonomyTierRuleProvider( tierResolver.Object, options, NullLogger.Instance), new PluginPermissionRuleProvider( pluginRegistry.Object, skillRegistry.Object, new ServiceCollection().BuildServiceProvider(), - // #524 round-2: empty key set is fine — pluginRegistry never configures - // GetBoundaryStatus, so Moq's default (PluginBoundaryStatus.Verified) means the - // blanket-deny path this lookup feeds never fires here. - new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()), + firstPartyToolLookup, NullLogger.Instance), - new EnvelopePermissionRuleProvider(NullLogger.Instance), + new EnvelopePermissionRuleProvider(NullLogger.Instance, firstPartyToolLookup), new ConfigBasedRuleProvider(options) ]; diff --git a/src/Content/Tests/Infrastructure.AI.Tests/Planner/StepExecutors/SubPlanEnvelopeConfinementTests.cs b/src/Content/Tests/Infrastructure.AI.Tests/Planner/StepExecutors/SubPlanEnvelopeConfinementTests.cs index 49985b9f9..7e0295725 100644 --- a/src/Content/Tests/Infrastructure.AI.Tests/Planner/StepExecutors/SubPlanEnvelopeConfinementTests.cs +++ b/src/Content/Tests/Infrastructure.AI.Tests/Planner/StepExecutors/SubPlanEnvelopeConfinementTests.cs @@ -6,6 +6,7 @@ using Application.AI.Common.Interfaces.Tools; using Application.AI.Common.Services.Agent; using Application.AI.Common.Services.Governance; +using Application.AI.Common.Services.Tools; using Application.Core.Permissions; using Domain.AI.Bundles; using Domain.AI.Changes; @@ -162,7 +163,9 @@ private static ServiceProvider BuildChildServices(Dictionary decis services.AddSingleton(decisions); services.AddScoped(); services.AddSingleton(new ThreePhasePermissionResolver( - [new EnvelopePermissionRuleProvider(NullLogger.Instance)], + [new EnvelopePermissionRuleProvider( + NullLogger.Instance, + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()))], safetyGates.Object, new GlobPatternMatcher(), new Mock().Object, From b1b3b45d37627d86f940a6adbc93c037a65b9564 Mon Sep 17 00:00:00 2001 From: MCKRUZ Date: Fri, 11 Sep 2026 22:11:04 -0400 Subject: [PATCH 2/4] refactor: extract shared published-name resolution, shrink GetRulesAsync (#626 code-review) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review on the #626 fix found two clean, bounded issues: - TryResolvePublishedName/WithPublishedNameForms logic was duplicated near-verbatim between EnvelopePermissionRuleProvider and PluginPermissionRuleProvider — the exact anti-pattern FirstPartyToolLookup itself exists to prevent (#387). Moved the resolve-or-fall-back-to-key logic onto FirstPartyToolLookup.TryResolvePublishedName; both callers now wrap it with only their own context-specific log message. - EnvelopePermissionRuleProvider.GetRulesAsync had grown to ~83 lines. Extracted its three rule-emission blocks into named helper methods. Two other findings (no caching on GetRulesAsync; permission-summary duplicate-entry display for a tool covered by both key and published-name forms) need their own design pass and are tracked as #651 and #652. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Aao7Y3hU22v6VH1RdiSxYu --- .../Services/Tools/FirstPartyToolLookup.cs | 30 +++++++ .../EnvelopePermissionRuleProvider.cs | 85 +++++++++++-------- .../PluginPermissionRuleProvider.cs | 18 ++-- 3 files changed, 89 insertions(+), 44 deletions(-) diff --git a/src/Content/Application/Application.AI.Common/Services/Tools/FirstPartyToolLookup.cs b/src/Content/Application/Application.AI.Common/Services/Tools/FirstPartyToolLookup.cs index f08b20f13..71d9547cb 100644 --- a/src/Content/Application/Application.AI.Common/Services/Tools/FirstPartyToolLookup.cs +++ b/src/Content/Application/Application.AI.Common/Services/Tools/FirstPartyToolLookup.cs @@ -97,4 +97,34 @@ public FirstPartyToolLookup( /// fail-closed response to an unverified plugin boundary needs every name to deny, not one). /// public IReadOnlySet RegisteredFirstPartyToolKeys => _registeredFirstPartyToolKeys; + + /// + /// Resolves 's converted, self-reported — the + /// value a permission resolver actually matches a rule's pattern against at invocation, which can + /// legitimately disagree with a DI registration key a manifest (a plugin's DeniedTools, a + /// capability envelope's grant, a bundle's declared tools) names it by. Returns + /// , with set to + /// itself, when the key names no known first-party tool (an MCP tool name, for which no + /// first-party resolution is possible or needed) OR constructing it throws. + /// + /// + /// #626 code-review: originally duplicated near-verbatim between PluginPermissionRuleProvider + /// (#612) and EnvelopePermissionRuleProvider (#626) — exactly the anti-pattern this type's + /// own class remarks say it exists to prevent (#387: "found duplicated — twice"). Deliberately pure + /// (no logging): a construction failure is a caller-specific concern (each rule provider names a + /// different kind of manifest entry in its own log message), so each caller still wraps this with + /// its own one-line log-on-failure — only the resolve-or-fall-back-to-key logic itself is shared. + /// + public bool TryResolvePublishedName(string toolKey, out string publishedName, out Exception? constructionError) + { + var tool = TryResolve(toolKey, out constructionError); + if (tool is not null) + { + publishedName = tool.Name; + return true; + } + + publishedName = toolKey; + return false; + } } diff --git a/src/Content/Application/Application.Core/Permissions/EnvelopePermissionRuleProvider.cs b/src/Content/Application/Application.Core/Permissions/EnvelopePermissionRuleProvider.cs index c8f67b5e1..d996cba10 100644 --- a/src/Content/Application/Application.Core/Permissions/EnvelopePermissionRuleProvider.cs +++ b/src/Content/Application/Application.Core/Permissions/EnvelopePermissionRuleProvider.cs @@ -128,8 +128,6 @@ public Task> GetRulesAsync( if (envelope is null) return Task.FromResult>([]); - var rules = new List(); - // #626: both the envelope's own grants and a bundle's declared tools are authored the same // DI-key-shaped way #612 found for a plugin's DeniedTools — expanding each name to also // include its resolved, self-reported published name (when it differs) before either loop @@ -140,14 +138,28 @@ public Task> GetRulesAsync( // this type's remarks on the closing Deny). var grantedNames = ExpandWithPublishedNameCoverage(ValidGrants(envelope)); - // 1. Bypass-immune Deny for each declared tool the envelope does not grant. Build the grant set once - // so the membership test is O(1) per declared tool rather than a linear scan of the allowlist. - // #626: each declared tool's key-and-published forms are checked for grant membership TOGETHER - // (a single decision per tool), not independently — checking them independently found a real - // bug during mutation testing: a tool declared by key and granted by its published name (or - // vice versa) was denied under whichever one of its two forms wasn't the literal grant string, - // even though the tool IS genuinely granted under the other form. + var rules = new List(); + AddDeclaredButUngrantedDenyRules(rules, agentId, grantedNames); + AddAutonomyCeilingBaselineRules(rules, envelope, grantedNames); + AddClosingDenyRule(rules); + + return Task.FromResult>(rules); + } + + /// + /// Bypass-immune Deny for each declared tool the envelope does not grant. Builds the grant set once + /// so the membership test is O(1) per declared tool rather than a linear scan of the allowlist. + /// #626: each declared tool's key-and-published forms are checked for grant membership TOGETHER + /// (a single decision per tool), not independently — checking them independently found a real bug + /// during mutation testing: a tool declared by key and granted by its published name (or vice + /// versa) was denied under whichever one of its two forms wasn't the literal grant string, even + /// though the tool IS genuinely granted under the other form. + /// + private void AddDeclaredButUngrantedDenyRules( + List rules, string agentId, IReadOnlyList grantedNames) + { var granted = new HashSet(grantedNames, StringComparer.OrdinalIgnoreCase); + foreach (var declaredName in EnumerateDeclaredTools(agentId)) { var declaredForms = WithPublishedNameForms(declaredName); @@ -165,14 +177,20 @@ public Task> GetRulesAsync( IsBypassImmune: true)); } } + } - // 2. Authoritative autonomy-ceiling baseline for each granted tool. Restricted and Supervised both - // map to Ask (approval required); only Autonomous maps to Allow (shared with every other rule - // provider so the tier-to-behavior policy cannot drift). NOTE: because live mid-tool-call approval - // routing is deferred, the governor currently treats Ask as a fail-closed block — so today a - // non-Autonomous ceiling effectively suspends the bundle's tool use rather than gating it for - // approval. This matches how plugin and tier baselines behave and is documented on - // CapabilityEnvelope.AutonomyCeiling; wiring the ceiling into live approval is a follow-up. + /// + /// Authoritative autonomy-ceiling baseline for each granted tool. Restricted and Supervised both + /// map to Ask (approval required); only Autonomous maps to Allow (shared with every other rule + /// provider so the tier-to-behavior policy cannot drift). NOTE: because live mid-tool-call approval + /// routing is deferred, the governor currently treats Ask as a fail-closed block — so today a + /// non-Autonomous ceiling effectively suspends the bundle's tool use rather than gating it for + /// approval. This matches how plugin and tier baselines behave and is documented on + /// CapabilityEnvelope.AutonomyCeiling; wiring the ceiling into live approval is a follow-up. + /// + private static void AddAutonomyCeilingBaselineRules( + List rules, CapabilityEnvelope envelope, IReadOnlyList grantedNames) + { var ceilingBehavior = envelope.AutonomyCeiling.ToDefaultPermissionBehavior(); foreach (var toolName in grantedNames) @@ -186,12 +204,17 @@ public Task> GetRulesAsync( IsAuthoritativeBaseline: true, BaselineTier: PermissionBaselineTier.GrantBoundary)); } + } - // 3. Closing Deny — everything the envelope did not grant. Emitted last and least specific so the - // per-name grants above outrank it, and at int.MaxValue priority so any future same-specificity - // baseline also wins. This is what turns the allowlist into a closed set: without it an ungranted - // name matches no envelope rule and resolution falls through to the host's generic autonomy tier, - // which in the shipped bundle-host configuration says Allow. + /// + /// Closing Deny — everything the envelope did not grant. Emitted last and least specific so the + /// per-name grants above outrank it, and at int.MaxValue priority so any future same-specificity + /// baseline also wins. This is what turns the allowlist into a closed set: without it an ungranted + /// name matches no envelope rule and resolution falls through to the host's generic autonomy tier, + /// which in the shipped bundle-host configuration says Allow. + /// + private static void AddClosingDenyRule(List rules) + { rules.Add(new ToolPermissionRule( "*", null, @@ -200,8 +223,6 @@ public Task> GetRulesAsync( Priority: int.MaxValue, IsAuthoritativeBaseline: true, BaselineTier: PermissionBaselineTier.GrantBoundary)); - - return Task.FromResult>(rules); } /// @@ -244,22 +265,19 @@ private IReadOnlyList WithPublishedNameForms(string name) /// /// Resolves 's converted, self-reported — - /// see PluginPermissionRuleProvider.TryResolvePublishedName's remarks for the full rationale - /// (#612), shared verbatim here for the envelope provider's identical need (#626). Returns + /// see 's remarks for the full rationale + /// (#612/#626; the resolve-or-fall-back-to-key logic itself now lives there, shared with + /// PluginPermissionRuleProvider's identical need, per #626 code-review). Returns /// , with set to /// itself, when the key names no known first-party tool (an MCP tool name, for which no /// first-party resolution is possible or needed) OR constructing it throws. /// private bool TryResolvePublishedName(string toolKey, out string publishedName) { - var tool = _firstPartyToolLookup.TryResolve(toolKey, out var constructionError); - if (tool is not null) - { - publishedName = tool.Name; - return true; - } + var resolved = _firstPartyToolLookup.TryResolvePublishedName( + toolKey, out publishedName, out var constructionError); - if (constructionError is not null) + if (!resolved && constructionError is not null) { _logger.LogError(constructionError, "Could not construct first-party tool '{ToolKey}' to learn its published name for a " + @@ -268,8 +286,7 @@ private bool TryResolvePublishedName(string toolKey, out string publishedName) toolKey); } - publishedName = toolKey; - return false; + return resolved; } /// diff --git a/src/Content/Application/Application.Core/Permissions/PluginPermissionRuleProvider.cs b/src/Content/Application/Application.Core/Permissions/PluginPermissionRuleProvider.cs index 9f72640f0..09c3093f5 100644 --- a/src/Content/Application/Application.Core/Permissions/PluginPermissionRuleProvider.cs +++ b/src/Content/Application/Application.Core/Permissions/PluginPermissionRuleProvider.cs @@ -367,18 +367,17 @@ private void AddDenyRuleWithPublishedNameCoverage(List rules /// cached: a tool whose constructor throws stays uncovered by the published-name rule for as long /// as the cached result stands (until the next mutation triggers a /// recompute), not retried on every call. A dependency-not-wired failure is deterministic, so a - /// retry would fail identically anyway. + /// retry would fail identically anyway. The resolve-or-fall-back-to-key logic itself lives on + /// (#626 code-review: was duplicated + /// near-verbatim with EnvelopePermissionRuleProvider's copy) — only the log-on-failure + /// message, specific to this caller's DeniedTools context, stays here. /// private bool TryResolvePublishedName(string toolKey, out string publishedName) { - var tool = _firstPartyToolLookup.TryResolve(toolKey, out var constructionError); - if (tool is not null) - { - publishedName = tool.Name; - return true; - } + var resolved = _firstPartyToolLookup.TryResolvePublishedName( + toolKey, out publishedName, out var constructionError); - if (constructionError is not null) + if (!resolved && constructionError is not null) { // Error, not Warning: this is a bypass-immune security control (a plugin's DeniedTools // backstop) now only partially enforced for this one tool — a level that gets filtered @@ -390,8 +389,7 @@ private bool TryResolvePublishedName(string toolKey, out string publishedName) toolKey); } - publishedName = toolKey; - return false; + return resolved; } /// From 3667e8f3e5b2084c303b99c0f7c31cc4d4210bbc Mon Sep 17 00:00:00 2001 From: MCKRUZ Date: Fri, 11 Sep 2026 23:34:50 -0400 Subject: [PATCH 3/4] fix: make ToolInvocationGovernor agree with the envelope rule layer on published names (#626) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI's correctness-review gate blocked the prior commit with a real finding: the published-name expansion #626 added to EnvelopePermissionRuleProvider only changed the RULE layer's decision. ToolInvocationGovernor independently re-confirms every resolver Allow against the envelope's raw AllowedTools list (CapabilityEnvelope.GrantsTool, a literal string match) as defence in depth. Since that check never learned about published-name expansion: - An Allow the rule layer enabled via a key/published-name match still got silently re-blocked by the governor's raw check — the #626 fix was inert for the exact case it was meant to fix. - The declared-tool Deny loop's grouping fix (which correctly stops emitting a redundant Deny when a tool IS granted under its other name-form) dropped real bypass-immune coverage, because the governor's independent check didn't actually treat that tool as granted either. Fix: extracted the shared "does this envelope grant toolName" resolution (including the key/published-name expansion) onto a new CapabilityEnvelopeGrantResolver, used by BOTH EnvelopePermissionRuleProvider and ToolInvocationGovernor.EnvelopeGrantsToolWhenArmed. The two layers now agree by construction, closing the gap the reviewer found. Added governor-level regression tests (ToolInvocationGovernorEnvelopeTests) proving the fix actually changes the real enforcement outcome, not just the rule set — mutation-tested by temporarily swapping in a resolver with no published-name coverage and confirming the new test fails without the fix. Full solution test suites re-run clean: Application.AI.Common.Tests (2716), Application.Core.Tests (1215), Infrastructure.AI.Tests (3517/3523, 6 pre- existing skips). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Aao7Y3hU22v6VH1RdiSxYu --- .../DependencyInjection.cs | 7 ++ .../CapabilityEnvelopeGrantResolver.cs | 100 ++++++++++++++++++ .../Governance/ToolInvocationGovernor.cs | 20 ++-- .../EnvelopePermissionRuleProvider.cs | 82 ++------------ .../Governance/ToolBehaviorPostureTests.cs | 4 +- .../Governance/ToolCompositionPostureTests.cs | 4 +- .../ToolInvocationGovernorEnvelopeTests.cs | 66 +++++++++++- .../Governance/ToolInvocationGovernorTests.cs | 9 +- .../ToolPathScopingEndToEndTests.cs | 3 +- .../EnvelopePermissionRuleProviderTests.cs | 6 +- .../EnvelopeEnforcementIntegrationTests.cs | 11 +- .../SubPlanEnvelopeConfinementTests.cs | 11 +- 12 files changed, 229 insertions(+), 94 deletions(-) create mode 100644 src/Content/Application/Application.AI.Common/Services/Governance/CapabilityEnvelopeGrantResolver.cs diff --git a/src/Content/Application/Application.AI.Common/DependencyInjection.cs b/src/Content/Application/Application.AI.Common/DependencyInjection.cs index fdcc6ed38..997d94bba 100644 --- a/src/Content/Application/Application.AI.Common/DependencyInjection.cs +++ b/src/Content/Application/Application.AI.Common/DependencyInjection.cs @@ -130,6 +130,13 @@ public static IServiceCollection AddApplicationAIDependencies( services.AddSingleton(sp => new Services.Tools.FirstPartyToolLookup( sp, new HashSet(KeyedToolRegistrationKeys(services), StringComparer.Ordinal))); + // Single source of truth for "does this capability envelope grant toolName" (#626) — shared by + // EnvelopePermissionRuleProvider (Application.Core) and ToolInvocationGovernor's independent + // runtime re-confirmation, so a first-party tool's key/published-name divergence resolves the + // same way in both. See CapabilityEnvelopeGrantResolver's remarks. + services.AddSingleton(sp => new Services.Governance.CapabilityEnvelopeGrantResolver( + sp.GetRequiredService())); + // Sandbox capability enforcement — profile resolution and enforcement. The resolver reads a // tool's own ITool.RequiredCapabilities/MinimumIsolation declaration via the shared // FirstPartyToolLookup (#387). diff --git a/src/Content/Application/Application.AI.Common/Services/Governance/CapabilityEnvelopeGrantResolver.cs b/src/Content/Application/Application.AI.Common/Services/Governance/CapabilityEnvelopeGrantResolver.cs new file mode 100644 index 000000000..faacef611 --- /dev/null +++ b/src/Content/Application/Application.AI.Common/Services/Governance/CapabilityEnvelopeGrantResolver.cs @@ -0,0 +1,100 @@ +using Application.AI.Common.Services.Tools; +using Domain.AI.Bundles; + +namespace Application.AI.Common.Services.Governance; + +/// +/// The single source of truth for "does this capability envelope grant toolName", accounting for a +/// first-party tool's DI-registration-key vs. self-reported published-name divergence (#626) — shared +/// by every consumer that must agree on the answer. +/// +/// +/// +/// is a literal, case-insensitive membership test against +/// . That is correct for an MCP tool grant (no +/// registration-key/published-name distinction exists for those) but incomplete for a first-party +/// tool: an operator can author a grant by the tool's DI registration key, while +/// ThreePhasePermissionResolver.Matches — and every other consumer that checks a tool's +/// identity at invocation — compares against the tool's self-reported ITool.Name, which can +/// legitimately disagree with its key. +/// +/// +/// Both halves of envelope enforcement must resolve this identically, by construction. +/// EnvelopePermissionRuleProvider builds permission rules from the envelope's grants, +/// and ToolInvocationGovernor.EnvelopeGrantsToolWhenArmed independently re-confirms a resolver +/// Allow against the same envelope as defence in depth — its own remarks state the two "must agree by +/// construction". Before this type existed, #626 gave the rule layer its own private published-name +/// expansion without updating the governor's check, so the rule layer would allow (or skip denying) a +/// key-authored grant, coverage the governor's raw, unexpanded check would still refuse — the fix was +/// runtime-inert for the case it was meant to fix, and correctness-review caught the mismatch this +/// method exists to close. Every consumer of "does the envelope grant this first-party tool" should go +/// through this type instead of calling directly. +/// +/// +public sealed class CapabilityEnvelopeGrantResolver +{ + private readonly FirstPartyToolLookup _firstPartyToolLookup; + + /// Initializes a new instance of the class. + /// Resolves a first-party tool's self-reported published name. + public CapabilityEnvelopeGrantResolver(FirstPartyToolLookup firstPartyToolLookup) + { + ArgumentNullException.ThrowIfNull(firstPartyToolLookup); + _firstPartyToolLookup = firstPartyToolLookup; + } + + /// + /// Whether grants — checking + /// 's literal membership in + /// first, then (only when that fails) whether any grant entry is a first-party tool's DI key whose + /// resolved published name matches . + /// + public bool Grants(CapabilityEnvelope envelope, string toolName) + { + ArgumentNullException.ThrowIfNull(envelope); + + if (string.IsNullOrWhiteSpace(toolName)) + return false; + + if (envelope.GrantsTool(toolName)) + return true; + + foreach (var grant in envelope.AllowedTools) + { + if (string.IsNullOrWhiteSpace(grant)) + continue; + + if (_firstPartyToolLookup.TryResolvePublishedName(grant, out var publishedName, out _) + && string.Equals(publishedName, toolName, StringComparison.OrdinalIgnoreCase)) + return true; + } + + return false; + } + + /// + /// Expands to also include each name's resolved, self-reported published + /// name when it names a first-party tool by DI key and the two disagree — flattened and + /// deduplicated case-insensitively, preserving first-seen order. + /// + public IReadOnlyList ExpandWithPublishedNameCoverage(IReadOnlyCollection names) + { + ArgumentNullException.ThrowIfNull(names); + + var expanded = new List(names.Count); + var seen = new HashSet(StringComparer.OrdinalIgnoreCase); + + foreach (var name in names) + { + if (seen.Add(name)) + expanded.Add(name); + + if (_firstPartyToolLookup.TryResolvePublishedName(name, out var publishedName, out _) + && !string.Equals(publishedName, name, StringComparison.OrdinalIgnoreCase) + && seen.Add(publishedName)) + expanded.Add(publishedName); + } + + return expanded; + } +} diff --git a/src/Content/Application/Application.AI.Common/Services/Governance/ToolInvocationGovernor.cs b/src/Content/Application/Application.AI.Common/Services/Governance/ToolInvocationGovernor.cs index 4c57f5d9c..b81188bb4 100644 --- a/src/Content/Application/Application.AI.Common/Services/Governance/ToolInvocationGovernor.cs +++ b/src/Content/Application/Application.AI.Common/Services/Governance/ToolInvocationGovernor.cs @@ -77,6 +77,7 @@ public sealed partial class ToolInvocationGovernor : IToolInvocationGovernor private readonly IOptionsMonitor _permissionsConfig; private readonly IOptionsMonitor _sandboxConfig; private readonly ILogger _logger; + private readonly CapabilityEnvelopeGrantResolver _envelopeGrantResolver; public ToolInvocationGovernor( IAgentExecutionContext executionContext, @@ -93,7 +94,8 @@ public ToolInvocationGovernor( IOptionsMonitor governanceConfig, IOptionsMonitor permissionsConfig, IOptionsMonitor sandboxConfig, - ILogger logger) + ILogger logger, + CapabilityEnvelopeGrantResolver envelopeGrantResolver) { _executionContext = executionContext; _toolPermissionService = toolPermissionService; @@ -110,6 +112,7 @@ public ToolInvocationGovernor( _permissionsConfig = permissionsConfig; _sandboxConfig = sandboxConfig; _logger = logger; + _envelopeGrantResolver = envelopeGrantResolver; } /// @@ -140,15 +143,20 @@ public ToolInvocationGovernor( /// /// /// The two must agree by construction — the envelope's own rules are built from the same - /// AllowedTools list this reads, matched with the same case-insensitive comparer. A - /// disagreement therefore means the resolver reached Allow by a path that did not consult the - /// envelope, which is precisely the condition worth failing closed on. + /// AllowedTools list this reads, matched through the same + /// that resolves a first-party tool's + /// key/published-name divergence (#626), not a raw + /// check — a mismatch there previously meant a rule-layer Allow enabled by that resolution could + /// still be silently re-blocked here, and a rule-layer Deny it correctly skipped was not actually + /// covered here either. A disagreement after routing both sides through the shared resolver means + /// the resolver reached Allow by a path that did not consult the envelope, which is precisely the + /// condition worth failing closed on. /// /// /// The tool the resolver has authorized. /// True when no envelope is armed, or when the armed envelope grants the tool. - private static bool EnvelopeGrantsToolWhenArmed(string toolName) - => CapabilityEnvelopeAccessor.Current is not { } envelope || envelope.GrantsTool(toolName); + private bool EnvelopeGrantsToolWhenArmed(string toolName) + => CapabilityEnvelopeAccessor.Current is not { } envelope || _envelopeGrantResolver.Grants(envelope, toolName); /// public async ValueTask AuthorizeAsync( diff --git a/src/Content/Application/Application.Core/Permissions/EnvelopePermissionRuleProvider.cs b/src/Content/Application/Application.Core/Permissions/EnvelopePermissionRuleProvider.cs index d996cba10..e5ee6835b 100644 --- a/src/Content/Application/Application.Core/Permissions/EnvelopePermissionRuleProvider.cs +++ b/src/Content/Application/Application.Core/Permissions/EnvelopePermissionRuleProvider.cs @@ -1,7 +1,6 @@ using Application.AI.Common.Interfaces.Permissions; using Application.AI.Common.Services.Bundles; using Application.AI.Common.Services.Governance; -using Application.AI.Common.Services.Tools; using Domain.AI.Bundles; using Domain.AI.Governance; using Domain.AI.Permissions; @@ -99,21 +98,22 @@ public sealed class EnvelopePermissionRuleProvider : IPermissionRuleProvider private const int BaselinePriority = 5; private readonly ILogger _logger; - private readonly FirstPartyToolLookup _firstPartyToolLookup; + private readonly CapabilityEnvelopeGrantResolver _envelopeGrantResolver; /// /// Initializes a new instance of the class. /// /// Logger for rejected wildcard grants in an envelope's allowlist. - /// - /// Resolves a granted/declared name's self-reported published name for #626 — see - /// . + /// + /// Resolves a granted/declared name's self-reported published name for #626, shared with + /// ToolInvocationGovernor's independent runtime re-confirmation so the two agree by + /// construction — see 's remarks. /// public EnvelopePermissionRuleProvider( - ILogger logger, FirstPartyToolLookup firstPartyToolLookup) + ILogger logger, CapabilityEnvelopeGrantResolver envelopeGrantResolver) { _logger = logger; - _firstPartyToolLookup = firstPartyToolLookup; + _envelopeGrantResolver = envelopeGrantResolver; } /// @@ -136,7 +136,7 @@ public Task> GetRulesAsync( // compares against the published name), so the tool falls through to the closing Deny despite // being "granted" in config — a real functional defect masked as fail-closed-by-accident (see // this type's remarks on the closing Deny). - var grantedNames = ExpandWithPublishedNameCoverage(ValidGrants(envelope)); + var grantedNames = _envelopeGrantResolver.ExpandWithPublishedNameCoverage(ValidGrants(envelope)); var rules = new List(); AddDeclaredButUngrantedDenyRules(rules, agentId, grantedNames); @@ -162,7 +162,7 @@ private void AddDeclaredButUngrantedDenyRules( foreach (var declaredName in EnumerateDeclaredTools(agentId)) { - var declaredForms = WithPublishedNameForms(declaredName); + var declaredForms = _envelopeGrantResolver.ExpandWithPublishedNameCoverage([declaredName]); if (declaredForms.Any(granted.Contains)) continue; @@ -225,70 +225,6 @@ private static void AddClosingDenyRule(List rules) BaselineTier: PermissionBaselineTier.GrantBoundary)); } - /// - /// Expands to also include each name's resolved, self-reported published - /// name (#626) via , flattened and deduplicated - /// case-insensitively. Safe only for a set consumed as a flat membership test (the granted-names - /// set, used by 's baseline loop and as the Deny-check's membership - /// target) — not for the declared-tools Deny loop itself, which must decide per ORIGINAL - /// declared tool whether ANY of its forms is granted before emitting a Deny for any of them; see - /// that loop's own comment for the bug this distinction fixes. - /// - private IReadOnlyList ExpandWithPublishedNameCoverage(IReadOnlyCollection names) - { - var expanded = new List(names.Count); - var seen = new HashSet(StringComparer.OrdinalIgnoreCase); - - foreach (var name in names) - foreach (var form in WithPublishedNameForms(name)) - if (seen.Add(form)) - expanded.Add(form); - - return expanded; - } - - /// - /// alone, or plus its resolved, self-reported - /// published name when it resolves to a real first-party tool whose name disagrees (#626) — the - /// value ThreePhasePermissionResolver.Matches actually compares a rule's pattern against at - /// invocation, which can legitimately disagree with a DI registration key an envelope grant or a - /// bundle's declared-tools list names it by. - /// - private IReadOnlyList WithPublishedNameForms(string name) - { - if (TryResolvePublishedName(name, out var publishedName) - && !string.Equals(publishedName, name, StringComparison.OrdinalIgnoreCase)) - return [name, publishedName]; - - return [name]; - } - - /// - /// Resolves 's converted, self-reported — - /// see 's remarks for the full rationale - /// (#612/#626; the resolve-or-fall-back-to-key logic itself now lives there, shared with - /// PluginPermissionRuleProvider's identical need, per #626 code-review). Returns - /// , with set to - /// itself, when the key names no known first-party tool (an MCP tool name, for which no - /// first-party resolution is possible or needed) OR constructing it throws. - /// - private bool TryResolvePublishedName(string toolKey, out string publishedName) - { - var resolved = _firstPartyToolLookup.TryResolvePublishedName( - toolKey, out publishedName, out var constructionError); - - if (!resolved && constructionError is not null) - { - _logger.LogError(constructionError, - "Could not construct first-party tool '{ToolKey}' to learn its published name for a " + - "capability-envelope rule — the key-pattern rule for it still applies, but a caller " + - "invoking it under a self-reported name that disagrees with the key would not be covered.", - toolKey); - } - - return resolved; - } - /// /// The tool names an envelope actually grants: its /// entries less any that contain a wildcard, which are rejected with an error log. diff --git a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolBehaviorPostureTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolBehaviorPostureTests.cs index 15b2b8747..9125c405d 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolBehaviorPostureTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolBehaviorPostureTests.cs @@ -322,7 +322,9 @@ private ToolCallAdmissionPipeline Pipeline(GovernanceConfig governance) monitor, Mock.Of>(m => m.CurrentValue == new PermissionsConfig()), Mock.Of>(m => m.CurrentValue == new SandboxConfig()), - NullLogger.Instance); + NullLogger.Instance, + new CapabilityEnvelopeGrantResolver( + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()))); return AdmissionHarness.Pipeline(governor: governor, trace: trace); } diff --git a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolCompositionPostureTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolCompositionPostureTests.cs index dfab55cd6..c190f34f1 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolCompositionPostureTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolCompositionPostureTests.cs @@ -203,7 +203,9 @@ private async Task Admit(GovernanceConfig governance, ToolCom monitor, Mock.Of>(m => m.CurrentValue == new PermissionsConfig()), Mock.Of>(m => m.CurrentValue == new SandboxConfig()), - NullLogger.Instance); + NullLogger.Instance, + new CapabilityEnvelopeGrantResolver( + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()))); var pipeline = AdmissionHarness.Pipeline(governor: governor, trace: trace); diff --git a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorEnvelopeTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorEnvelopeTests.cs index b975d5f4e..911e0e444 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorEnvelopeTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorEnvelopeTests.cs @@ -4,6 +4,7 @@ using Application.AI.Common.Interfaces.Sandbox; using Application.AI.Common.Interfaces.Tools; using Application.AI.Common.Services.Governance; +using Application.AI.Common.Services.Tools; using Domain.AI.Bundles; using Domain.AI.Changes; using Domain.AI.Governance; @@ -12,6 +13,7 @@ using Domain.Common.Config.AI; using Domain.Common.Config.AI.Permissions; using Domain.Common.Config.AI.Sandbox; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; @@ -74,7 +76,7 @@ public ToolInvocationGovernorEnvelopeTests() /// private GovernanceTraceRecorder _trace = null!; - private ToolInvocationGovernor Build() + private ToolInvocationGovernor Build(CapabilityEnvelopeGrantResolver? envelopeGrantResolver = null) { var governanceMonitor = Mock.Of>(m => m.CurrentValue == _governanceOff); _trace = new GovernanceTraceRecorder(governanceMonitor, _riskClassifier); @@ -87,11 +89,29 @@ private ToolInvocationGovernor Build() _trace, governanceMonitor, Mock.Of>(m => m.CurrentValue == _permissionsConfig), Mock.Of>(m => m.CurrentValue == _sandbox), - NullLogger.Instance); + NullLogger.Instance, + envelopeGrantResolver ?? new CapabilityEnvelopeGrantResolver( + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()))); } private static CapabilityEnvelope Envelope() => new() { AllowedTools = [Tool] }; + /// + /// A resolver that knows is a first-party tool whose self-reported + /// published name is — used by the #626 regression tests below to + /// prove the governor's independent re-check agrees with the rule layer on a key/published-name + /// divergence, not just that the rule layer alone computes the right rules. + /// + private static CapabilityEnvelopeGrantResolver ResolverWithDivergentTool(string key, string publishedName) + { + var services = new ServiceCollection(); + var mock = new Mock(); + mock.Setup(t => t.Name).Returns(publishedName); + services.AddKeyedSingleton(key, mock.Object); + return new CapabilityEnvelopeGrantResolver( + new FirstPartyToolLookup(services.BuildServiceProvider(), new HashSet { key })); + } + [Fact] public async Task GlobalOff_NoBundleRun_PassesThroughWithoutEvaluating() { @@ -227,4 +247,46 @@ public async Task ResolverAllowsToolGrantedInDifferentCase_IsAllowed() Assert.True(decision.IsAllowed); } + + // --- #626 correctness-review regression: a published-name-expansion fix at the rule-provider + // layer alone cannot change any invocation outcome, because this governor independently + // re-confirms the envelope's raw grant list. These tests exercise the governor's own check + // directly, proving CapabilityEnvelopeGrantResolver — not just EnvelopePermissionRuleProvider's + // rules — resolves a first-party tool's key/published-name divergence. + + [Fact] + public async Task EnvelopeGrantsByKey_InvocationUsesDivergentPublishedName_IsAllowed() + { + // The envelope's operator-authored grant names the tool by its DI registration key, but the + // call arrives (as it always does at runtime) under the tool's self-reported published name. + _permissions + .Setup(x => x.ResolvePermissionAsync(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny?>(), It.IsAny())) + .ReturnsAsync(PermissionDecision.Allow("granted by key")); + var governor = Build(ResolverWithDivergentTool("tool_key", "published_tool")); + + ToolInvocationDecision decision; + using (CapabilityEnvelopeAccessor.Begin(new CapabilityEnvelope { AllowedTools = ["tool_key"] })) + decision = await governor.AuthorizeAsync("published_tool", CancellationToken.None); + + Assert.True(decision.IsAllowed); + } + + [Fact] + public async Task EnvelopeGrantsByKey_InvocationUsesUnrelatedName_IsStillDenied() + { + // Sanity check for the test above: wiring a divergent-name-aware resolver must not turn into + // "allow anything" — a name with no relationship to the grant is still refused. + _permissions + .Setup(x => x.ResolvePermissionAsync(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny?>(), It.IsAny())) + .ReturnsAsync(PermissionDecision.Allow("resolver arbitration bug")); + var governor = Build(ResolverWithDivergentTool("tool_key", "published_tool")); + + ToolInvocationDecision decision; + using (CapabilityEnvelopeAccessor.Begin(new CapabilityEnvelope { AllowedTools = ["tool_key"] })) + decision = await governor.AuthorizeAsync("some_unrelated_tool", CancellationToken.None); + + Assert.False(decision.IsAllowed); + } } diff --git a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorTests.cs index 62fd02e28..e4c794c1d 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorTests.cs @@ -4,6 +4,7 @@ using Application.AI.Common.Interfaces.Sandbox; using Application.AI.Common.Interfaces.Tools; using Application.AI.Common.Services.Governance; +using Application.AI.Common.Services.Tools; using Domain.AI.Changes; using Domain.AI.Governance; using Domain.AI.Permissions; @@ -11,6 +12,7 @@ using Domain.Common.Config.AI; using Domain.Common.Config.AI.Permissions; using Domain.Common.Config.AI.Sandbox; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; @@ -110,7 +112,12 @@ private ToolInvocationGovernor Build(GovernanceConfig? governance = null) governanceMonitor, Mock.Of>(m => m.CurrentValue == _permissionsConfig), Mock.Of>(m => m.CurrentValue == _sandbox), - NullLogger.Instance); + NullLogger.Instance, + // No ambient envelope in this suite (see ToolInvocationGovernorEnvelopeTests for that), so + // EnvelopeGrantsToolWhenArmed short-circuits true without ever consulting this — an empty + // lookup is fine. + new CapabilityEnvelopeGrantResolver( + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()))); } [Fact] diff --git a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolPathScopingEndToEndTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolPathScopingEndToEndTests.cs index 7458a84dc..7154321be 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolPathScopingEndToEndTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolPathScopingEndToEndTests.cs @@ -114,7 +114,8 @@ private static (ToolInvocationGovernor Governor, GovernanceTraceRecorder Trace) governanceMonitor, Mock.Of>(m => m.CurrentValue == new PermissionsConfig()), sandboxMonitor, - NullLogger.Instance); + NullLogger.Instance, + new CapabilityEnvelopeGrantResolver(lookup)); return (governor, trace); } diff --git a/src/Content/Tests/Application.Core.Tests/Permissions/EnvelopePermissionRuleProviderTests.cs b/src/Content/Tests/Application.Core.Tests/Permissions/EnvelopePermissionRuleProviderTests.cs index cd0129dbc..79653da7b 100644 --- a/src/Content/Tests/Application.Core.Tests/Permissions/EnvelopePermissionRuleProviderTests.cs +++ b/src/Content/Tests/Application.Core.Tests/Permissions/EnvelopePermissionRuleProviderTests.cs @@ -29,7 +29,8 @@ public sealed class EnvelopePermissionRuleProviderTests NullLogger.Instance, // #626: empty key set is fine — no case in this suite names a tool whose published name // disagrees with its key, so TryResolvePublishedName always falls back to the key itself. - new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet())); + new CapabilityEnvelopeGrantResolver( + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()))); /// /// The rules written for a specific tool name, i.e. everything except the closing catch-all. Most @@ -226,7 +227,8 @@ private static EnvelopePermissionRuleProvider ProviderWithDivergentTool(string k mock.Setup(t => t.Name).Returns(publishedName); services.AddKeyedSingleton(key, mock.Object); var lookup = new FirstPartyToolLookup(services.BuildServiceProvider(), new HashSet { key }); - return new EnvelopePermissionRuleProvider(NullLogger.Instance, lookup); + return new EnvelopePermissionRuleProvider( + NullLogger.Instance, new CapabilityEnvelopeGrantResolver(lookup)); } [Fact] diff --git a/src/Content/Tests/Infrastructure.AI.Tests/Permissions/EnvelopeEnforcementIntegrationTests.cs b/src/Content/Tests/Infrastructure.AI.Tests/Permissions/EnvelopeEnforcementIntegrationTests.cs index 674409f6c..6951afa06 100644 --- a/src/Content/Tests/Infrastructure.AI.Tests/Permissions/EnvelopeEnforcementIntegrationTests.cs +++ b/src/Content/Tests/Infrastructure.AI.Tests/Permissions/EnvelopeEnforcementIntegrationTests.cs @@ -144,7 +144,9 @@ private ThreePhasePermissionResolver Resolver( pluginRegistry.Object, skillRegistry.Object, new ServiceCollection().BuildServiceProvider(), firstPartyToolLookup, NullLogger.Instance), - new EnvelopePermissionRuleProvider(NullLogger.Instance, firstPartyToolLookup), + new EnvelopePermissionRuleProvider( + NullLogger.Instance, + new CapabilityEnvelopeGrantResolver(firstPartyToolLookup)), new ConfigBasedRuleProvider(options) ]; @@ -297,9 +299,10 @@ public async Task AutonomousPluginTool_OutsideTheEnvelope_IsDenied(bool useTierP // outright and the bundle could invoke a tool the caller was never granted. // // The bundle's overlay declares only file_system, so EnumerateDeclaredTools never sees - // k8sgpt_analyze and phase 1b emits no bypass-immune Deny for it. The resolver is the sole - // enforcement point for tool names — ToolInvocationGovernor has no independent GrantsTool check — - // so if this resolves to anything but Deny, the tool runs. + // k8sgpt_analyze and phase 1b emits no bypass-immune Deny for it. ToolInvocationGovernor's + // independent re-check (via CapabilityEnvelopeGrantResolver, #626) would also refuse an + // ungranted tool here, but this test exercises the resolver alone — so if this resolves to + // anything but Deny, the resolver itself (not a second gate) has to be relied on to stop it. var plugin = AutonomousPlugin("k8s-ops"); var skill = PluginSkill("k8s-ops", "k8sgpt_analyze"); diff --git a/src/Content/Tests/Infrastructure.AI.Tests/Planner/StepExecutors/SubPlanEnvelopeConfinementTests.cs b/src/Content/Tests/Infrastructure.AI.Tests/Planner/StepExecutors/SubPlanEnvelopeConfinementTests.cs index 7e0295725..8cdac0ae0 100644 --- a/src/Content/Tests/Infrastructure.AI.Tests/Planner/StepExecutors/SubPlanEnvelopeConfinementTests.cs +++ b/src/Content/Tests/Infrastructure.AI.Tests/Planner/StepExecutors/SubPlanEnvelopeConfinementTests.cs @@ -158,14 +158,19 @@ private static ServiceProvider BuildChildServices(Dictionary decis It.IsAny?>(), It.IsAny?>(), It.IsAny())) .ReturnsAsync(Result.Success()); + // Shared with both the rule provider and the real governor below (registered into the + // container so the governor's independent re-check, CapabilityEnvelopeGrantResolver, agrees + // with the rule layer by construction — #626). + var envelopeGrantResolver = new CapabilityEnvelopeGrantResolver( + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet())); + var services = new ServiceCollection(); services.AddSingleton(typeof(Microsoft.Extensions.Logging.ILogger<>), typeof(NullLogger<>)); services.AddSingleton(decisions); + services.AddSingleton(envelopeGrantResolver); services.AddScoped(); services.AddSingleton(new ThreePhasePermissionResolver( - [new EnvelopePermissionRuleProvider( - NullLogger.Instance, - new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()))], + [new EnvelopePermissionRuleProvider(NullLogger.Instance, envelopeGrantResolver)], safetyGates.Object, new GlobPatternMatcher(), new Mock().Object, From ae1a1060699bc52c5a8606913344620b56f629ba Mon Sep 17 00:00:00 2001 From: MCKRUZ Date: Fri, 11 Sep 2026 23:48:08 -0400 Subject: [PATCH 4/4] fix: restore log-on-construction-failure in CapabilityEnvelopeGrantResolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent /code-review passes caught the same regression: extracting the shared resolver dropped the ERROR log EnvelopePermissionRuleProvider used to emit when a first-party tool's constructor throws while resolving its published name. FirstPartyToolLookup.TryResolvePublishedName is deliberately pure (no logging) specifically so every caller supplies its own log-on-failure — both call sites here were passing `out _`, discarding it entirely with no substitute anywhere in the new type. Added ILogger and a private wrapper that logs only the genuine construction-failure case (not the normal "not a first-party tool" case), used by both Grants() and ExpandWithPublishedNameCoverage(). Full suites re-run clean: Application.AI.Common.Tests (2716), Application.Core.Tests (1215), envelope-scoped Infrastructure.AI.Tests (19). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Aao7Y3hU22v6VH1RdiSxYu --- .../DependencyInjection.cs | 3 +- .../CapabilityEnvelopeGrantResolver.cs | 38 +++++++++++++++++-- .../Governance/ToolBehaviorPostureTests.cs | 3 +- .../Governance/ToolCompositionPostureTests.cs | 3 +- .../ToolInvocationGovernorEnvelopeTests.cs | 6 ++- .../Governance/ToolInvocationGovernorTests.cs | 3 +- .../ToolPathScopingEndToEndTests.cs | 2 +- .../EnvelopePermissionRuleProviderTests.cs | 6 ++- .../EnvelopeEnforcementIntegrationTests.cs | 3 +- .../SubPlanEnvelopeConfinementTests.cs | 3 +- 10 files changed, 56 insertions(+), 14 deletions(-) diff --git a/src/Content/Application/Application.AI.Common/DependencyInjection.cs b/src/Content/Application/Application.AI.Common/DependencyInjection.cs index 997d94bba..51bbe8bc6 100644 --- a/src/Content/Application/Application.AI.Common/DependencyInjection.cs +++ b/src/Content/Application/Application.AI.Common/DependencyInjection.cs @@ -135,7 +135,8 @@ public static IServiceCollection AddApplicationAIDependencies( // runtime re-confirmation, so a first-party tool's key/published-name divergence resolves the // same way in both. See CapabilityEnvelopeGrantResolver's remarks. services.AddSingleton(sp => new Services.Governance.CapabilityEnvelopeGrantResolver( - sp.GetRequiredService())); + sp.GetRequiredService(), + sp.GetRequiredService>())); // Sandbox capability enforcement — profile resolution and enforcement. The resolver reads a // tool's own ITool.RequiredCapabilities/MinimumIsolation declaration via the shared diff --git a/src/Content/Application/Application.AI.Common/Services/Governance/CapabilityEnvelopeGrantResolver.cs b/src/Content/Application/Application.AI.Common/Services/Governance/CapabilityEnvelopeGrantResolver.cs index faacef611..0e0591aa4 100644 --- a/src/Content/Application/Application.AI.Common/Services/Governance/CapabilityEnvelopeGrantResolver.cs +++ b/src/Content/Application/Application.AI.Common/Services/Governance/CapabilityEnvelopeGrantResolver.cs @@ -1,5 +1,6 @@ using Application.AI.Common.Services.Tools; using Domain.AI.Bundles; +using Microsoft.Extensions.Logging; namespace Application.AI.Common.Services.Governance; @@ -34,13 +35,44 @@ namespace Application.AI.Common.Services.Governance; public sealed class CapabilityEnvelopeGrantResolver { private readonly FirstPartyToolLookup _firstPartyToolLookup; + private readonly ILogger _logger; /// Initializes a new instance of the class. /// Resolves a first-party tool's self-reported published name. - public CapabilityEnvelopeGrantResolver(FirstPartyToolLookup firstPartyToolLookup) + /// + /// Logs a construction failure per 's + /// documented calling contract — that method is deliberately pure, so every caller must supply its + /// own one-line log-on-failure. + /// + public CapabilityEnvelopeGrantResolver( + FirstPartyToolLookup firstPartyToolLookup, ILogger logger) { ArgumentNullException.ThrowIfNull(firstPartyToolLookup); + ArgumentNullException.ThrowIfNull(logger); _firstPartyToolLookup = firstPartyToolLookup; + _logger = logger; + } + + /// + /// Wraps to honor its documented + /// calling contract: log a construction failure (not merely "no such first-party tool", which is + /// the normal, silent case for an MCP tool name). + /// + private bool TryResolvePublishedName(string toolKey, out string publishedName) + { + var resolved = _firstPartyToolLookup.TryResolvePublishedName( + toolKey, out publishedName, out var constructionError); + + if (!resolved && constructionError is not null) + { + _logger.LogError(constructionError, + "Could not construct first-party tool '{ToolKey}' to learn its published name for a " + + "capability-envelope grant check — the raw grant entry still applies, but a caller " + + "invoking it under a self-reported name that disagrees with the key would not be covered.", + toolKey); + } + + return resolved; } /// @@ -64,7 +96,7 @@ public bool Grants(CapabilityEnvelope envelope, string toolName) if (string.IsNullOrWhiteSpace(grant)) continue; - if (_firstPartyToolLookup.TryResolvePublishedName(grant, out var publishedName, out _) + if (TryResolvePublishedName(grant, out var publishedName) && string.Equals(publishedName, toolName, StringComparison.OrdinalIgnoreCase)) return true; } @@ -89,7 +121,7 @@ public IReadOnlyList ExpandWithPublishedNameCoverage(IReadOnlyCollection if (seen.Add(name)) expanded.Add(name); - if (_firstPartyToolLookup.TryResolvePublishedName(name, out var publishedName, out _) + if (TryResolvePublishedName(name, out var publishedName) && !string.Equals(publishedName, name, StringComparison.OrdinalIgnoreCase) && seen.Add(publishedName)) expanded.Add(publishedName); diff --git a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolBehaviorPostureTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolBehaviorPostureTests.cs index 9125c405d..fe509698e 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolBehaviorPostureTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolBehaviorPostureTests.cs @@ -324,7 +324,8 @@ private ToolCallAdmissionPipeline Pipeline(GovernanceConfig governance) Mock.Of>(m => m.CurrentValue == new SandboxConfig()), NullLogger.Instance, new CapabilityEnvelopeGrantResolver( - new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()))); + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()), + NullLogger.Instance)); return AdmissionHarness.Pipeline(governor: governor, trace: trace); } diff --git a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolCompositionPostureTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolCompositionPostureTests.cs index c190f34f1..cf71ff874 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolCompositionPostureTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolCompositionPostureTests.cs @@ -205,7 +205,8 @@ private async Task Admit(GovernanceConfig governance, ToolCom Mock.Of>(m => m.CurrentValue == new SandboxConfig()), NullLogger.Instance, new CapabilityEnvelopeGrantResolver( - new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()))); + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()), + NullLogger.Instance)); var pipeline = AdmissionHarness.Pipeline(governor: governor, trace: trace); diff --git a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorEnvelopeTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorEnvelopeTests.cs index 911e0e444..1bc7abefb 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorEnvelopeTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorEnvelopeTests.cs @@ -91,7 +91,8 @@ private ToolInvocationGovernor Build(CapabilityEnvelopeGrantResolver? envelopeGr Mock.Of>(m => m.CurrentValue == _sandbox), NullLogger.Instance, envelopeGrantResolver ?? new CapabilityEnvelopeGrantResolver( - new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()))); + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()), + NullLogger.Instance)); } private static CapabilityEnvelope Envelope() => new() { AllowedTools = [Tool] }; @@ -109,7 +110,8 @@ private static CapabilityEnvelopeGrantResolver ResolverWithDivergentTool(string mock.Setup(t => t.Name).Returns(publishedName); services.AddKeyedSingleton(key, mock.Object); return new CapabilityEnvelopeGrantResolver( - new FirstPartyToolLookup(services.BuildServiceProvider(), new HashSet { key })); + new FirstPartyToolLookup(services.BuildServiceProvider(), new HashSet { key }), + NullLogger.Instance); } [Fact] diff --git a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorTests.cs index e4c794c1d..e70c22c7d 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolInvocationGovernorTests.cs @@ -117,7 +117,8 @@ private ToolInvocationGovernor Build(GovernanceConfig? governance = null) // EnvelopeGrantsToolWhenArmed short-circuits true without ever consulting this — an empty // lookup is fine. new CapabilityEnvelopeGrantResolver( - new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()))); + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()), + NullLogger.Instance)); } [Fact] diff --git a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolPathScopingEndToEndTests.cs b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolPathScopingEndToEndTests.cs index 7154321be..490c305bd 100644 --- a/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolPathScopingEndToEndTests.cs +++ b/src/Content/Tests/Application.AI.Common.Tests/Governance/ToolPathScopingEndToEndTests.cs @@ -115,7 +115,7 @@ private static (ToolInvocationGovernor Governor, GovernanceTraceRecorder Trace) Mock.Of>(m => m.CurrentValue == new PermissionsConfig()), sandboxMonitor, NullLogger.Instance, - new CapabilityEnvelopeGrantResolver(lookup)); + new CapabilityEnvelopeGrantResolver(lookup, NullLogger.Instance)); return (governor, trace); } diff --git a/src/Content/Tests/Application.Core.Tests/Permissions/EnvelopePermissionRuleProviderTests.cs b/src/Content/Tests/Application.Core.Tests/Permissions/EnvelopePermissionRuleProviderTests.cs index 79653da7b..aa52f92ff 100644 --- a/src/Content/Tests/Application.Core.Tests/Permissions/EnvelopePermissionRuleProviderTests.cs +++ b/src/Content/Tests/Application.Core.Tests/Permissions/EnvelopePermissionRuleProviderTests.cs @@ -30,7 +30,8 @@ public sealed class EnvelopePermissionRuleProviderTests // #626: empty key set is fine — no case in this suite names a tool whose published name // disagrees with its key, so TryResolvePublishedName always falls back to the key itself. new CapabilityEnvelopeGrantResolver( - new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()))); + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()), + NullLogger.Instance)); /// /// The rules written for a specific tool name, i.e. everything except the closing catch-all. Most @@ -228,7 +229,8 @@ private static EnvelopePermissionRuleProvider ProviderWithDivergentTool(string k services.AddKeyedSingleton(key, mock.Object); var lookup = new FirstPartyToolLookup(services.BuildServiceProvider(), new HashSet { key }); return new EnvelopePermissionRuleProvider( - NullLogger.Instance, new CapabilityEnvelopeGrantResolver(lookup)); + NullLogger.Instance, + new CapabilityEnvelopeGrantResolver(lookup, NullLogger.Instance)); } [Fact] diff --git a/src/Content/Tests/Infrastructure.AI.Tests/Permissions/EnvelopeEnforcementIntegrationTests.cs b/src/Content/Tests/Infrastructure.AI.Tests/Permissions/EnvelopeEnforcementIntegrationTests.cs index 6951afa06..25188047d 100644 --- a/src/Content/Tests/Infrastructure.AI.Tests/Permissions/EnvelopeEnforcementIntegrationTests.cs +++ b/src/Content/Tests/Infrastructure.AI.Tests/Permissions/EnvelopeEnforcementIntegrationTests.cs @@ -146,7 +146,8 @@ private ThreePhasePermissionResolver Resolver( NullLogger.Instance), new EnvelopePermissionRuleProvider( NullLogger.Instance, - new CapabilityEnvelopeGrantResolver(firstPartyToolLookup)), + new CapabilityEnvelopeGrantResolver( + firstPartyToolLookup, NullLogger.Instance)), new ConfigBasedRuleProvider(options) ]; diff --git a/src/Content/Tests/Infrastructure.AI.Tests/Planner/StepExecutors/SubPlanEnvelopeConfinementTests.cs b/src/Content/Tests/Infrastructure.AI.Tests/Planner/StepExecutors/SubPlanEnvelopeConfinementTests.cs index 8cdac0ae0..991a1cfd7 100644 --- a/src/Content/Tests/Infrastructure.AI.Tests/Planner/StepExecutors/SubPlanEnvelopeConfinementTests.cs +++ b/src/Content/Tests/Infrastructure.AI.Tests/Planner/StepExecutors/SubPlanEnvelopeConfinementTests.cs @@ -162,7 +162,8 @@ private static ServiceProvider BuildChildServices(Dictionary decis // container so the governor's independent re-check, CapabilityEnvelopeGrantResolver, agrees // with the rule layer by construction — #626). var envelopeGrantResolver = new CapabilityEnvelopeGrantResolver( - new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet())); + new FirstPartyToolLookup(new ServiceCollection().BuildServiceProvider(), new HashSet()), + NullLogger.Instance); var services = new ServiceCollection(); services.AddSingleton(typeof(Microsoft.Extensions.Logging.ILogger<>), typeof(NullLogger<>));