Skip to content

.Net: Harden gRPC plugin address handling#13961

Merged
SergeyMenshykh merged 3 commits into
microsoft:mainfrom
SergeyMenshykh:fix-grpc
May 7, 2026
Merged

.Net: Harden gRPC plugin address handling#13961
SergeyMenshykh merged 3 commits into
microsoft:mainfrom
SergeyMenshykh:fix-grpc

Conversation

@SergeyMenshykh

Copy link
Copy Markdown
Contributor

Description

Add developer-controlled address configuration and validation for the gRPC plugin.

Motivation and Context

The gRPC plugin now supports GrpcFunctionExecutionParameters for configuring address handling, including an optional address override, allowed base addresses, and scheme restrictions.

Changes

  • Introduced GrpcFunctionExecutionParameters class for developer-controlled gRPC channel configuration
  • Added address validation with scheme and allowlist checks in GrpcOperationRunner
  • Plumbed execution parameters through all public methods in GrpcKernelExtensions
  • Updated and added unit tests

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Add GrpcFunctionExecutionParameters for developer-controlled address
configuration. Add address validation with scheme and allowlist checks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings May 7, 2026 10:35
@SergeyMenshykh
SergeyMenshykh requested a review from a team as a code owner May 7, 2026 10:35
@SergeyMenshykh SergeyMenshykh self-assigned this May 7, 2026
@SergeyMenshykh SergeyMenshykh moved this to In Progress in Agent Framework May 7, 2026
@SergeyMenshykh SergeyMenshykh moved this from In Progress to In Review in Agent Framework May 7, 2026
@moonbox3 moonbox3 added the .NET Issue or Pull requests regarding .NET code label May 7, 2026
@github-actions github-actions Bot changed the title Harden gRPC plugin address handling .Net: Harden gRPC plugin address handling May 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens the .NET gRPC plugin’s address handling by removing LLM-controlled address overrides and introducing developer-controlled execution parameters for channel configuration, including scheme restrictions and an address allowlist for SSRF mitigation.

Changes:

  • Introduced GrpcFunctionExecutionParameters to allow developer-supplied HttpClient, optional address override, allowed base addresses, and allowed schemes.
  • Updated GrpcOperationRunner to ignore address passed via KernelArguments and to validate the effective address (scheme + allowlist).
  • Updated unit tests and removed the address parameter from gRPC operation parameter metadata.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
dotnet/src/Functions/Functions.UnitTests/Grpc/GrpcRunnerTests.cs Updates/extends tests for the new address override + allowlist + scheme validation behavior.
dotnet/src/Functions/Functions.UnitTests/Grpc/Extensions/GrpcOperationExtensionsTests.cs Verifies the gRPC operation no longer exposes an address parameter.
dotnet/src/Functions/Functions.Grpc/Model/GrpcOperation.cs Removes the address argument constant and stops advertising address as an operation parameter.
dotnet/src/Functions/Functions.Grpc/GrpcOperationRunner.cs Implements developer-controlled address override and adds scheme/allowlist validation.
dotnet/src/Functions/Functions.Grpc/Extensions/GrpcKernelExtensions.cs Plumbs execution parameters through public extension methods and configures the runner accordingly.
dotnet/src/Functions/Functions.Grpc/Extensions/GrpcFunctionExecutionParameters.cs Adds a new public execution-parameters type for developer-controlled configuration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread dotnet/src/Functions/Functions.Grpc/GrpcOperationRunner.cs Outdated
Comment thread dotnet/src/Functions/Functions.Grpc/Extensions/GrpcKernelExtensions.cs Outdated
Comment thread dotnet/src/Functions/Functions.UnitTests/Grpc/GrpcRunnerTests.cs

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated Code Review

Reviewers: 3 | Confidence: 90%

✓ Security Reliability

This PR is a well-designed security hardening that removes the LLM-controllable 'address' parameter from gRPC operations (preventing SSRF), adds scheme validation defaulting to HTTPS-only, and provides an optional allowlist for defense-in-depth. The validation logic in GetAddress correctly uses Uri.TryCreate for normalization before comparing, handles path-boundary attacks, and the constructor properly defaults to HTTPS when no schemes are specified. The removal of AddressArgumentName from both the model and parameter registration cleanly eliminates the attack surface. No blocking security or reliability issues found.

✓ Test Coverage

The PR adds comprehensive test coverage for the new gRPC address hardening features including allowlist validation, scheme rejection, path-boundary prefix checks, and address override validation. However, there is one notable coverage gap: no test verifies that configuring custom AllowedSchemes (e.g., permitting http for local development) actually succeds. All scheme-related tests only verify the rejection path (default https-only), leaving the positive custom-scheme path untested.

✗ Design Approach

The address-hardening logic looks sound overall, but the new execution-parameter plumbing introduces one design regression: a caller-suplied HttpClient is still wrapped in using during plugin creation, so the new API disposes the developer-owned client immediately after import and leaves the returned plugin holding a dead runner.

Flagged Issues

  • GrpcKernelExtensions.cs:174 accepts executionParameters?.HttpClient but binds it to using var client, disposing the caller-owned HttpClient as soon as plugin creation returns. The plugin's GrpcOperationRunner retains this client for later invocation, so any subsequent function call will use a disposed instance. The repo's OpenAPI analogue explicitly avoids this pattern (see Functions.OpenApi/Extensions/OpenApiKernelExtensions.cs:122-124, 161-163, 204-206). Remove the using declaration for externally-provided clients.

Automated review by SergeyMenshykh's agents

Fix HttpClient disposal for externally-provided clients.
Fix allowlist boundary check for trailing-slash base URIs.
Add tests for custom schemes and sub-path allowlist matching.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@SergeyMenshykh
SergeyMenshykh enabled auto-merge May 7, 2026 13:26
@SergeyMenshykh
SergeyMenshykh added this pull request to the merge queue May 7, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 7, 2026
@SergeyMenshykh
SergeyMenshykh added this pull request to the merge queue May 7, 2026
Merged via the queue into microsoft:main with commit 52d4e5c May 7, 2026
18 checks passed
@SergeyMenshykh
SergeyMenshykh deleted the fix-grpc branch May 7, 2026 18:23
@github-project-automation github-project-automation Bot moved this from In Review to Done in Agent Framework May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

.NET Issue or Pull requests regarding .NET code

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants