Backport to branch(3.12) : Fix NullPointerException when a client is misconfigured with digital signature

[feeblefakie]

This is an automated backport of the following:

• Original PR: Fix NullPointerException when a client is misconfigured with digital signature #302
• Backported commit: e5ba069

Please merge this PR after all checks have passed.

[gemini-code-assist]

Summary of Changes

Hello @feeblefakie, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This backport addresses a potential NullPointerException that could occur when a client is misconfigured with digital signature authentication in a system expecting HMAC. The core change involves introducing a dedicated MissingSecretException to provide a more explicit and robust error handling mechanism. This prevents unexpected runtime errors by ensuring that the absence of a required secret is caught and communicated clearly, rather than leading to a NullPointerException. The secret management components have been refactored to integrate this new exception, improving overall system stability and diagnostic capabilities.

Highlights

• New Exception Type: Introduced a new MissingSecretException to specifically handle cases where a required secret (e.g., an HMAC key) is not found, providing clearer error messages than a generic NullPointerException.
• Secret Lookup Behavior: Modified the ScalarSecretRegistry's lookup method to throw the new MissingSecretException when a secret is not found, instead of returning null. This enforces explicit error handling.
• Secret Registration Logic: Updated the SecretManager's register method to properly handle the MissingSecretException during secret registration, ensuring that new secrets can be bound if they don't already exist.
• Error Code and Status: Added a new SECRET_NOT_FOUND error entry in CommonError and a corresponding StatusCode (415) to standardize the error reporting for missing secrets.
• Integration Test Coverage: Added a new integration test case to verify that a MissingSecretException is correctly thrown when a client is misconfigured, specifically when HMAC authentication is expected but a digital signature is provided.
• Code Cleanup: Removed the Constants.java file, which contained an unused constant.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request effectively addresses a NullPointerException arising from a client misconfiguration with digital signatures. The core change, modifying SecretRegistry.lookup to throw a specific MissingSecretException instead of returning null, is a robust solution that prevents this and similar null-related issues. The accompanying changes in error handling, status codes, and tests are consistent and thorough, including a new integration test that validates the fix. Overall, the changes improve code quality and reliability. I have a couple of suggestions to further enhance code clarity and maintainability.

[gemini-code-assist]

While this implementation is functionally correct, using a try-catch block for control flow to check for existence can be considered an anti-pattern as it can make the code's intent less clear. A more conventional approach would be to have an exists(Key) method in the SecretRegistry that returns a boolean. This would make the logic in register more explicit: check for existence, then either throw an exception or bind the new entry. This change would improve readability and maintainability.

[gemini-code-assist]

This implementation can be written more concisely using a more functional style with Optional.map(). This would avoid the need for the local result variable and make the flow more streamlined.

    try {
      return storage
          .get(get)
          .map(this::toSecretEntry)
          .orElseThrow(() -> new MissingSecretException(CommonError.SECRET_NOT_FOUND));
    } catch (ExecutionException e) {
      throw new DatabaseException(CommonError.GETTING_SECRET_KEY_FAILED, e, e.getMessage());
    }