Add input validation to gRPC service implementations#7402
Merged
Conversation
Add defensive validation to gRPC service methods to prevent panics
and undefined behavior from nil or invalid requests.
Changes:
- compose_service: Replace panic("unimplemented") with proper
codes.Unimplemented gRPC status error
- environment_service: Add nil/empty validation on GetValue, SetValue
- prompt_service: Add nil checks on req.Options in Confirm, Select,
MultiSelect; validate wire.Scope in createAzureContext
- container_service: Add nil/empty validation on Build, Package, Publish
- account_service: Add nil/empty validation on LookupTenant
- copilot_service: Document graceful degradation, improve error log context
- event_service: Add nil/empty validation on subscribe handlers,
modernize for-range loops
- event_service_test: Update test expectation for new empty event
name validation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
wbreza
requested review from
JeffreyCA,
hemarina,
jongio,
rajeshkamal5050,
tg-msft,
vhvb1989 and
weikanglim
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Hardens the azd gRPC extension framework by adding defensive request validation and more robust error handling, reducing the chance of panics and improving behavior when extensions send malformed inputs.
Changes:
- Added request/parameter validation (returning
codes.InvalidArgument) to multiple gRPC service methods. - Replaced an explicit panic in
composeService.GetResourceTypewith a proper gRPCUnimplementedstatus. - Improved
copilot_servicelogging/context and updated event subscription tests for new validation behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/internal/grpcserver/prompt_service.go | Adds nil validation for several prompt RPCs and for Azure context creation. |
| cli/azd/internal/grpcserver/event_service.go | Validates subscription event name lists and entries; modernizes counted loops. |
| cli/azd/internal/grpcserver/event_service_test.go | Updates expectation to treat empty event subscriptions as an error. |
| cli/azd/internal/grpcserver/environment_service.go | Validates env key/value requests before resolving environments. |
| cli/azd/internal/grpcserver/copilot_service.go | Documents graceful degradation for event conversion and improves log context. |
| cli/azd/internal/grpcserver/container_service.go | Adds request/service name validation to container build/package/publish RPCs. |
| cli/azd/internal/grpcserver/compose_service.go | Replaces panic("unimplemented") with codes.Unimplemented. |
| cli/azd/internal/grpcserver/account_service.go | Validates lookup tenant request and subscription ID. |
gRPC framework guarantees non-nil request objects (generated handlers use new(RequestType) before calling methods). Remove redundant nil checks while keeping field-level validation (empty string checks). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add tests verifying the new validation paths across gRPC services: - compose_service_test: GetResourceType returns codes.Unimplemented - environment_service_test: GetValue/SetValue reject empty keys with codes.InvalidArgument - prompt_service_test: Confirm/Select/MultiSelect reject nil Options with codes.InvalidArgument - event_service_test: Subscribe rejects empty event names in array Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address PR review feedback: - container_service: Use codes.NotFound for unknown service names in Build, Package, Publish instead of plain fmt.Errorf - prompt_service: Wrap ParseResourceID failures as codes.InvalidArgument in createAzureContext for consistent client semantics Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
reviewed
Mar 31, 2026
Member
jongio
left a comment
jongio
left a comment
There was a problem hiding this comment.
PR Review - #7402
Add input validation to gRPC service implementations by @wbreza
Summary
What: Adds defensive input validation to gRPC service methods in the extension framework - nil checks, empty string checks, proper gRPC status codes (InvalidArgument, NotFound, Unimplemented), replacing a panic and upgrading fmt.Errorf to status.Errorf across 8 service files.
Why: Extension framework GA hardening (H-4: gRPC Input Validation Gaps). Prevents panics and undefined behavior from malformed extension requests.
Assessment: Solid hardening PR. Validation additions are consistent, well-tested, and use proper gRPC status codes. The panic-to-Unimplemented fix in compose_service is the highest-value change. Two minor improvement opportunities noted inline.
Findings
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Idiomatic | 0 | 0 | 1 | 0 |
| Quality | 0 | 0 | 0 | 1 |
| Total | 0 | 0 | 1 | 1 |
Test Coverage Estimate
- Well covered: All new validation paths have corresponding tests across 4 test files
- copilot_service.go logging changes are doc-only, don't need tests
- Good edge case coverage (empty strings, nil options, nil scope, empty event arrays, index-specific error messages)
What's Done Well
- panic("unimplemented") replaced with proper codes.Unimplemented gRPC status - eliminates a real crash risk
- Consistent use of codes.InvalidArgument and codes.NotFound with descriptive messages
- Tests verify both gRPC status code AND message content
- Loop modernization from C-style to range-over-integer
2 inline comments below.
- event_service: Use for i, eventName := range instead of range len() with manual indexing in both subscribe handlers - event_service_test: Use t.Context() per Go 1.26 conventions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds defensive input validation to gRPC service methods in the extension framework to prevent panics and undefined behavior from nil or invalid requests. This is part of the extension framework GA hardening effort (H-4: gRPC Input Validation Gaps).
Changes
Critical Fix
Input Validation
Error Handling
Tests
Validation