Skip to content

Return GRPC codes.InvalidArgument if JWK is invalid.#860

Open
ggreer wants to merge 2 commits into
mainfrom
ggreer/jwk-grpc-status
Open

Return GRPC codes.InvalidArgument if JWK is invalid.#860
ggreer wants to merge 2 commits into
mainfrom
ggreer/jwk-grpc-status

Conversation

@ggreer
Copy link
Copy Markdown
Contributor

@ggreer ggreer commented May 22, 2026

No description provided.

@ggreer ggreer requested a review from a team May 22, 2026 22:19
err := jwk.UnmarshalJSON(jwkBytes)
if err != nil {
return nil, fmt.Errorf("jwk: failed to unmarshal jwk: %w", err)
return nil, status.Errorf(codes.InvalidArgument, "jwk: failed to unmarshal jwk: %v", err)
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.

🟡 Suggestion: No test covers the unmarshalJWK failure path. TestInvalidKeyType exercises an unsupported key type after successful unmarshal, not an unmarshal failure. Consider adding a test that passes invalid bytes to Encrypt and asserts status.Code(err) == codes.InvalidArgument.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 22, 2026

General PR Review: Return GRPC codes.InvalidArgument if JWK is invalid.

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Review mode: incremental since 4f04212
View review run

Review Summary

The new commit adds a unit test for the unmarshalJWK failure path, directly addressing the previous review suggestion. The test passes invalid bytes and asserts the returned error carries gRPC status code codes.InvalidArgument. No new issues found.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

No blocking issues found.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

No blocking issues found.

@ggreer ggreer enabled auto-merge (squash) May 24, 2026 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant