🐛 constant-time HMAC security fix

[ianardee]

Description

Types of changes

• Docs change / refactoring / dependency upgrade
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Requires a change to the official Guide documentation.

[Copilot]

🟡 Changes recommended

The new implementation can do avoidable work on oversized attacker-controlled signatures and may accept an empty signature if HMAC generation fails, so it needs small guard adjustments before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR hardens BaseLocalResponse#isValidHmacSignature by replacing a potentially timing-leaky String.equals signature comparison with a constant-time comparison approach, improving HMAC verification safety.

Changes:

• Use MessageDigest.isEqual for constant-time signature comparison.
• Add null handling for secretKey and signature.
• Normalize incoming signatures to lowercase before comparison.

File summaries

File	Description
src/main/java/com/mindee/parsing/BaseLocalResponse.java	Switches HMAC signature verification to a constant-time byte comparison and adds input normalization/validation.

Review details

Suppressed comments (1)

src/main/java/com/mindee/parsing/BaseLocalResponse.java:114

• signature.toLowerCase(...).getBytes(...) allocates proportional to the incoming header size; unlike the previous String.equals, this can turn an oversized X-Mindee-Hmac-Signature into avoidable CPU/memory work. Also, getHmacSignature can return an empty string on internal errors, which would currently validate an empty signature. Consider short-circuiting on empty/length mismatch before normalizing/allocating.

    byte[] expectedBytes = getHmacSignature(secretKey).getBytes(StandardCharsets.UTF_8);
    byte[] actualBytes = signature
      .toLowerCase(java.util.Locale.ROOT)
      .getBytes(StandardCharsets.UTF_8);

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The updated v2 parsing test asserts the wrong object for nullability (and contains a helper naming inconsistency), which can mask failures and should be corrected before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/test/java/com/mindee/v2/parsing/LocalResponseTest.java:54

• Update the helper invocation to match the renamed assertLocalResponse method.

    AssertLocalResponse(localResponse);

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The new signature verification path should also guard against empty computed signatures and is missing a regression test for the newly introduced case-insensitive signature handling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

The current MessageDigest.isEqual usage can still return early on length mismatch, so the intended constant-time comparison isn’t fully achieved for variable-length signature inputs.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite