fix: resolve envelope grant/declared-tool published names (#626)

[MCKRUZ]

Summary

Fixes #626: a capability envelope's grants and a bundle's declared tools can name a first-party tool
by its DI registration key, but the runtime permission resolver matches rules against the tool's
self-reported published name at invocation. When the two disagree — a legitimate, tested scenario
(see ToolCatalogTests.Catalog_ToolWhoseNameDisagreesWithItsKey_...) — a key-named grant silently
never matched, and a key-named declared tool was silently denied even when actually granted under its
published name.

This mirrors #612's fix for PluginPermissionRuleProvider.DeniedTools, applied to
EnvelopePermissionRuleProvider's two rule kinds (declared-but-ungranted Deny, and the
autonomy-ceiling baseline for granted tools).

What's included

• EnvelopePermissionRuleProvider: expands both the envelope's grants and a bundle's declared tools
  to also cover each name's resolved published name when it differs.
• A grouping bug found via mutation testing during implementation: the declared-tools Deny loop
  originally checked each name-form independently against the granted set, which could deny a tool
  declared by one form but granted by the other. Fixed by deciding once per declared tool across all
  its forms.
• PluginPermissionRuleProvider autonomy-baseline rules may have the same key-vs-published-name gap as #612's deny rules #625 (originally scoped as a companion fix for PluginPermissionRuleProvider's autonomy-baseline
  rules) was investigated and found to be based on a false premise — verified with a test before
  shipping, then reverted and closed as not planned. Full reasoning in the issue's closing comment.
• Code-review follow-up: extracted the duplicated TryResolvePublishedName logic (which had been
  copy-pasted between PluginPermissionRuleProvider and this provider) onto FirstPartyToolLookup,
  and split GetRulesAsync into three named helper methods to bring it under the repo's 50-line
  guideline.
• Two remaining code-review findings need their own design pass and are filed as follow-ups: EnvelopePermissionRuleProvider.GetRulesAsync constructs first-party tools on every call (no caching) #651
  (no caching on GetRulesAsync, unlike the plugin provider's StateVersion-keyed cache) and PermissionRulesSectionProvider.FormatRules shows duplicate entries for a tool covered by both key and published-name forms #652
  (the LLM-facing permission summary doesn't dedupe a tool covered by both its key and published-name
  forms).

Test plan

• dotnet build src/AgenticHarness.slnx — clean
• Application.Core.Tests permission-provider suite (45 tests) — pass
• Infrastructure.AI.Tests envelope-enforcement + sub-plan-confinement integration tests (19
  tests) — pass
• New regression tests: grant-by-key/published-name-disagreement baseline coverage, and the
  declared-by-key-granted-by-published-name grouping fix (mutation-tested against the original bug)
• Local review gate: run-gates.sh was killed twice by genuine host memory pressure (not a test
  failure) — pushed with the sanctioned RAILS_SKIP_REVIEW_GATE=1 bypass, deferring to CI's
  independent gates

🤖 Generated with Claude Code

https://claude.ai/code/session_01Aao7Y3hU22v6VH1RdiSxYu

[github-actions]

Grader verdict — PR #653

Intent: Fix #626 — an envelope's AllowedTools grants and a bundle's declared tools can name a first-party tool by its DI registration key, but ThreePhasePermissionResolver.Matches matches against the tool's self-reported published name, so a key-named grant silently never matched and a key-named declared tool was silently denied even when actually granted. Mirrors #612's fix for PluginPermissionRuleProvider.DeniedTools.

Check-by-check

Claim	Verdict	Evidence
Envelope grants expanded to also cover resolved published name	✅ met	EnvelopePermissionRuleProvider.cs:131 (ExpandWithPublishedNameCoverage(ValidGrants(envelope))), test EnvelopePermissionRuleProviderTests.cs:216-234 (GrantByKeyDisagreeingWithPublishedName_...)
Declared tools expanded to also cover resolved published name (Deny loop)	✅ met	EnvelopePermissionRuleProvider.cs:146-160 (AddDeclaredButUngrantedDenyRules), test EnvelopePermissionRuleProviderTests.cs:236-254 (DeclaredByKey_GrantedByPublishedName_IsNotSpuriouslyDenied)
Grouping bug fix: decide grant-membership once per declared tool across all its forms, not independently	✅ met	EnvelopePermissionRuleProvider.cs:162-163 (declaredForms.Any(granted.Contains) before emitting any Deny) — matches the mutation-testing finding described in the PR body
No double-emission when key == published name	✅ met	EnvelopePermissionRuleProviderTests.cs:266-274 (GrantByKeyMatchingPublishedName_EmitsOnlyOneBaselineRule) plus WithPublishedNameForms's equality guard at EnvelopePermissionRuleProvider.cs:263-264
Shared TryResolvePublishedName extracted onto FirstPartyToolLookup, dedup'd from PluginPermissionRuleProvider	✅ met	FirstPartyToolLookup.cs:100-129; PluginPermissionRuleProvider.cs:377-380 now delegates to it
GetRulesAsync split under the 50-line guideline	✅ met	EnvelopePermissionRuleProvider.cs:123-137 (body ≈15 lines) delegates to AddDeclaredButUngrantedDenyRules/AddAutonomyCeilingBaselineRules/AddClosingDenyRule
#625 (companion fix for PluginPermissionRuleProvider autonomy baseline) is not included, per "false premise, reverted"	✅ met	No baseline-related change in PluginPermissionRuleProvider.cs diff (only the TryResolvePublishedName delegation at lines 370-392); confirmed via git log --grep=625 — no such commit on this branch
Test plan counts (45 / 19 tests)	⚠️ partial	Re-ran locally: Application.Core.Tests --filter Permissions → 70 passed (broader filter than "permission-provider suite," includes PluginPermissionRuleProviderTests too — not a discrepancy, just a wider net); Infrastructure.AI.Tests envelope+sub-plan filter → 19 passed, matches exactly

Holes

• All three consuming call sites for the new EnvelopePermissionRuleProvider(logger, firstPartyToolLookup) constructor were updated (EnvelopeEnforcementIntegrationTests.cs:133-147, SubPlanEnvelopeConfinementTests.cs:166-168, plus the unit-test field) — grepped for new EnvelopePermissionRuleProvider( repo-wide, found no stale 1-arg call sites. No omission here.
• Local review gate was bypassed (RAILS_SKIP_REVIEW_GATE=1, disclosed in the PR body as "killed twice by host memory pressure"). This is the sanctioned emergency path per .claude/rules/review-cadence.md, and CI's independent gates are the non-forgeable backstop — flagging only because the PR itself asks CI to catch what local gates couldn't confirm; the human Checker should confirm CI's code-review/security-review/grader checks are green before merge, not just that this comment is green.
• Follow-ups correctly filed rather than scope-crept in: EnvelopePermissionRuleProvider.GetRulesAsync constructs first-party tools on every call (no caching) #651 (no caching on GetRulesAsync, unlike PluginPermissionRuleProvider's StateVersion-keyed cache) and PermissionRulesSectionProvider.FormatRules shows duplicate entries for a tool covered by both key and published-name forms #652 (LLM-facing permission summary doesn't dedupe key/published-name duplicate coverage) are named in the PR body as deliberately out of scope — consistent with the diff, which adds no caching and doesn't touch the summary-rendering path. No scope creep found.
• No regression test for the no-op case where FirstPartyToolLookup.TryResolve throws during envelope rule generation (a manifest-declared/granted key whose tool constructor fails) — TryResolvePublishedName (EnvelopePermissionRuleProvider.cs:275-289) logs and falls back to the key-only form, but nothing in this PR's test diff exercises that path for the envelope provider specifically (the analogous PluginPermissionRuleProvider case already had coverage pre-PR). Low severity — behavior is a safe fallback, not a functional gap — but it's the one untested branch in the new code.

Standards check

• Functions: all new/extracted methods are well under 50 lines (GetRulesAsync ~15, AddDeclaredButUngrantedDenyRules ~18, others shorter).
• Files: EnvelopePermissionRuleProvider.cs is 363 lines (under 400). PluginPermissionRuleProvider.cs is 464 lines — over 400, but pre-existing (466 lines on main); this PR shrinks it by 2 lines, doesn't introduce the violation.
• XML docs: new public member FirstPartyToolLookup.TryResolvePublishedName is fully documented (FirstPartyToolLookup.cs:100-114); new constructor parameter documented (EnvelopePermissionRuleProvider.cs:107-110).
• No Result<T>/skill-training/keyed-DI concerns apply to this diff — it's pure rule-generation logic, no new commands or tool registrations.
• Build: dotnet build on Application.Core and Application.AI.Common — clean, 0 warnings. Targeted test runs (Application.Core.Tests Permissions filter: 70/70 pass; Infrastructure.AI.Tests envelope+sub-plan filter: 19/19 pass) all green.

Bottom line

LOOKS GOOD. The fix is narrowly scoped, mirrors the established #612 pattern, includes a mutation-tested regression for the grouping bug it found along the way, and correctly declines to bundle the reverted #625. The one thing the human Checker should verify before merging: that CI's own code-review/security-review/grader gates are green, since this PR explicitly bypassed the local pre-push gate due to host resource pressure rather than a clean pass.

[github-actions]

🔐 Security review — PR #653

Bottom line: PASS — no HIGH findings. Two MEDIUM/LOW advisories below; neither blocks.

Scope reviewed: EnvelopePermissionRuleProvider, FirstPartyToolLookup.TryResolvePublishedName, PluginPermissionRuleProvider, plus the test wiring. This PR widens a grant boundary (the envelope allowlist), which is the dangerous direction, so I traced each widening to a concrete invocation path.

Why the widening is not exploitable

GetRulesAsync now expands every envelope grant and every declared tool to {key, publishedName} (EnvelopePermissionRuleProvider.cs:139, :257), and skips the bypass-immune Deny when any form is granted (:166). Three escalation paths were checked and all fail closed:

• Attacker-controlled published name. FirstPartyToolLookup.Resolve is gated on the bounded _registeredFirstPartyToolKeys set, so only host-registered first-party keys resolve. ITool.Name is compile-time host code — an MCP/bundle-owned name never reaches the expansion.
• Wildcard smuggled past ValidGrants. ValidGrants rejects * (:313) but the expansion adds tool.Name unchecked. Not reachable today (see above) — noted as LOW.
• Dropped bypass-immune Deny. Every case where the new declaredForms.Any(granted.Contains) skips a Deny that the old literal check emitted is either the same tool under its other name, or is still refused downstream by ToolInvocationGovernor.EnvelopeGrantsToolWhenArmed (ToolInvocationGovernor.cs:150), which does exact membership against the raw AllowedTools and blocks with GovernanceDenials.NotPermitted.

That governor check is what carries the PASS: it is strictly narrower than the resolver after this change, so every newly-granted name that the operator did not literally write is still blocked at invocation.

---

MEDIUM · EnvelopePermissionRuleProvider.cs:139 — the resolver and the defence-in-depth gate no longer agree by construction

ToolInvocationGovernor.EnvelopeGrantsToolWhenArmed's own remarks state the invariant this PR invalidates:

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.

After this change the resolver's rules are built from an expanded set, not the same list. Two consequences:

1. The stated defect is not actually fixed end-to-end. An operator who grants a tool by its DI key now gets a resolver Allow for the published name — and is then refused by the governor, since GrantsTool(publishedName) is false. ToolCatalog (:103, :114) and DirectToolInvoker.Mcp.cs:91 filter the same way. The tool remains uninvokable; the PR moves the denial from the resolver to the governor rather than removing it.
2. The gate's signal is degraded. Its refusal was previously diagnostic of an arbitration bug. It will now also fire on legitimate, envelope-consulting Allows, which is exactly how a fail-closed backstop gets reclassified as noise and then relaxed — at which point the resolver-side widening is load-bearing with nothing behind it.

Fix: apply the same published-name resolution on the GrantsTool side (or introduce a shared canonicalization both halves call), so the two remain one meaning — matching the "held to one meaning" rule this type's own class remarks set out for the wildcard case. If that is deferred, update EnvelopeGrantsToolWhenArmed's remarks so the next reader does not act on an invariant that no longer holds.

LOW · EnvelopePermissionRuleProvider.cs:257 — published name bypasses the wildcard rejection

ValidGrants rejects any AllowedTools entry containing * because a wildcard grant would confer the reserved plan capabilities (llm_call, rag_retrieval) the envelope exists to gate. WithPublishedNameForms emits tool.Name into the same rule-pattern position with no equivalent check. Not exploitable today (ITool.Name is host code and the governor backstops it), but the guard is one line and keeps a future dynamically-named tool from reopening the case the wildcard rejection was written for.

LOW · FirstPartyToolLookup.cs:118 (via :57) — comparer mismatch silently skips resolution

The bounded key set is built with StringComparer.Ordinal, while grant/declared-name matching throughout EnvelopePermissionRuleProvider uses OrdinalIgnoreCase. A grant or declaration differing from the registration key only in case ("Bash" vs "bash") misses the key-set membership test, falls back to the key itself, and gets no published-name coverage — the exact gap #626 is closing, reintroduced for case-divergent authoring. Fail-closed (under-grant, not over-grant), so advisory only.

---

No findings on PluginPermissionRuleProvider — the extraction is behaviour-preserving (!resolved && constructionError is not null is equivalent to the prior tool is null guard, since TryResolvePublishedName returns false on exactly that branch), and its expansion is in the Deny direction.

[github-actions]

Correctness review — PR #653

Bottom line: CORRECT. No blocking defects. Verified locally: Application.Core builds clean (0 warnings), EnvelopePermissionRuleProviderTests 16/16 pass, Application.AI.Common.Tests Governance filter 518/518 pass.

Blocking defects

None.

What I checked and cleared

Area	Anchor	Result
Governor/rule-layer agreement on key↔published divergence	ToolInvocationGovernor.cs:158, EnvelopePermissionRuleProvider.cs:139	Both sides now route through CapabilityEnvelopeGrantResolver. Traced all four grant/invocation combinations (grant-by-key + invoke-by-published, grant-by-published + invoke-by-published, neither, both) — the two halves return the same answer in each.
Deny-expansion not weakening confinement	EnvelopePermissionRuleProvider.cs:165-178	declaredForms.Any(granted.Contains) is a single decision per declared tool (correct — checking forms independently would spuriously deny a tool granted under its other form), and when ungranted it now emits Deny for both forms. Strictly tighter than the pre-change single-form Deny; no path loses a Deny it previously had.
DI wiring	DependencyInjection.cs:133-140, EnvelopePermissionRuleProvider.cs:112	CapabilityEnvelopeGrantResolver is a singleton registered in the same method as FirstPartyToolLookup; EnvelopePermissionRuleProvider (Application.Core, singleton) and ToolInvocationGovernor (scoped) both resolve it — same cross-assembly dependency shape PluginPermissionRuleProvider already has, so no host gains a new unsatisfiable graph. No captive-dependency inversion (singleton into scoped).
Null / bounded-probe safety	CapabilityEnvelopeGrantResolver.cs:112-131	ExpandWithPublishedNameCoverage has no null/whitespace guard on elements, but both call sites pre-filter (ValidGrants, EnumerateDeclaredTools). TryResolvePublishedName goes through FirstPartyToolLookup.Resolve's bounded key set, so the unbounded-keyed-DI-probe hazard in that type's remarks is respected.
PluginPermissionRuleProvider refactor equivalence	PluginPermissionRuleProvider.cs:370-392	Extracted TryResolvePublishedName is behaviour-identical to the inlined version, including the publishedName = toolKey fallback and the constructionError is not null guard on the Error log.

Advisory (not blocking)

1. CapabilityEnvelopeGrantResolver.cs:69 — construction failure is logged at Error on every call, uncached. PluginPermissionRuleProvider's equivalent path deliberately caches the resolution (its remarks at PluginPermissionRuleProvider.cs:367-373 say a ctor failure "stays uncovered … not retried on every call" because it's deterministic). The new resolver has no such cache: Grants runs per tool invocation while an envelope is armed, and ExpandWithPublishedNameCoverage runs per GetRulesAsync. A first-party tool whose constructor throws will therefore emit one Error log per grant entry per tool call for the life of the bundle run. Correct, but noisy — consider memoizing the key→published-name map.

2. CapabilityEnvelopeGrantResolver.cs:32 — the "every consumer should go through this type" claim isn't yet true. ToolCatalog.ListGranted/FindGranted and DirectToolInvoker.Mcp.cs:91 still call CapabilityEnvelope.GrantsTool directly. ToolCatalog is entirely key-space (it filters registered keys), so it happens to agree with a key-authored grant and MCP names have no key/published split — hence no defect today. But the doc comment reads as an enforced invariant when it is a convention with three known exceptions; per this repo's own standing rule about controls that nothing invokes, either narrow the claim or add a guard that fails when a new raw GrantsTool call site appears.

3. CapabilityEnvelopeGrantResolver.cs:116-127 — blind expansion widens on a name collision. If a grant string happens to be another first-party tool's DI key, that other tool's published name is added to grantedNames and gets an Allow baseline. Both halves agree (the governor's Grants resolves it the same way), so this is not a fail-open divergence, and it requires one tool's grant-authored name to equal a second tool's registration key — contrived enough that I'm not treating it as a defect. Worth a line in the remarks.

4. Test coverage gap. GrantByKeyDisagreeingWithPublishedName_AlsoEmitsBaselineForThePublishedName and EnvelopeGrantsByKey_InvocationUsesDivergentPublishedName_IsAllowed each prove one half. There is no test that drives a divergent-name tool through both layers in one assertion — the "must agree by construction" property the whole PR exists to restore is asserted twice separately rather than once jointly. SubPlanEnvelopeConfinementTests wires a shared resolver into both (SubPlanEnvelopeConfinementTests.cs:161-167) but registers it with an empty key set, so no divergence is exercised there.

The single thing a human should look at: advisory 1 — the deliberate no-cache/log-every-call divergence from PluginPermissionRuleProvider's explicitly-documented caching decision, on a path that runs per tool invocation.