Skip to content

Conversation

@bdesoky
Copy link
Contributor

@bdesoky bdesoky commented Oct 20, 2025

Ticket: ANT-1033

Description

This change enhances the calculateHMACSubject function to support Buffers in addition to strings. This will allow us to pass buffers directly from WP instead of having to call toString() which incurs a performance cost. Existing tests pass, and new tests have been added to verify the HMAC generation/verification works with Buffers.

This change is part of the V3 auth improvements epic

Issue Number

ANT-1033

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Unit tests

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My code compiles correctly for both Node and Browser environments
  • I have commented my code, particularly in hard-to-understand areas
  • My commits follow Conventional Commits and I have properly described any BREAKING CHANGES
  • The ticket or github issue was included in the commit message as a reference
  • I have made corresponding changes to the documentation and on any new/updated functions and/or methods - jsdoc
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@bdesoky bdesoky requested review from a team as code owners October 20, 2025 17:42
@bdesoky bdesoky marked this pull request as draft October 20, 2025 17:51
@bdesoky bdesoky force-pushed the ANT-1033 branch 2 times, most recently from 918f1e4 to 273e6b7 Compare October 20, 2025 17:57
@bdesoky bdesoky changed the title feat: Add Buffer support for HMAC generation feat: add Buffer support for HMAC generation Oct 20, 2025
@bdesoky bdesoky marked this pull request as ready for review October 20, 2025 18:29
Copy link

Copilot AI left a comment

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 adds Buffer support to the HMAC generation functions to improve performance by eliminating unnecessary toString() conversions when working with binary data from WP.

  • Updated type definitions to accept string | Buffer for text parameters
  • Modified calculateHMACSubject to handle Buffer inputs and return Buffer when appropriate
  • Added comprehensive test coverage for Buffer-based HMAC generation and verification

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
modules/sdk-hmac/src/types.ts Updated type definitions to support Buffer in addition to string for text parameters across all HMAC-related interfaces
modules/sdk-hmac/src/hmac.ts Enhanced calculateHMACSubject function to handle Buffer inputs by concatenating prefix with Buffer data instead of string joining
modules/sdk-api/src/bitgoAPI.ts Updated return type of calculateHMACSubject method to reflect Buffer support
modules/sdk-hmac/test/hmac.ts Added test cases for Buffer input handling in request/response HMAC generation and verification

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@bdesoky bdesoky merged commit 2828138 into master Oct 22, 2025
14 checks passed
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.

4 participants