Skip to content

feat(mbe): Hide recovery APIs for mbe#85

Merged
pranavjain97 merged 2 commits intomasterfrom
WP-5318/hide-recovery-apis-mbe
Jul 25, 2025
Merged

feat(mbe): Hide recovery APIs for mbe#85
pranavjain97 merged 2 commits intomasterfrom
WP-5318/hide-recovery-apis-mbe

Conversation

@cpatino-intive
Copy link
Copy Markdown
Contributor

TASKS: WP-5318

@cpatino-intive cpatino-intive requested review from Copilot, mohammadalfaiyazbitgo and pranavjain97 and removed request for pranavjain97 July 24, 2025 13:05

This comment was marked as outdated.

Copy link
Copy Markdown

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

Implements a recovery mode feature that allows administrators to control access to wallet recovery APIs through configuration. The feature adds a recoveryMode boolean flag that must be enabled for recovery operations to execute.

  • Added recoveryMode configuration option to both enclaved and master express configurations
  • Implemented recovery mode validation in wallet recovery handlers to prevent unauthorized access
  • Added comprehensive test coverage for both enabled and disabled recovery mode scenarios

Reviewed Changes

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

Show a summary per file
File Description
src/shared/types/index.ts Adds recoveryMode boolean property to BaseConfig interface
src/initConfig.ts Implements recoveryMode configuration reading from RECOVERY_MODE environment variable
src/api/master/handlers/recoveryWallet.ts Adds recovery mode validation to wallet recovery handler
src/api/master/handlers/recoveryConsolidationsWallet.ts Adds recovery mode validation to consolidation recovery handler
src/api/master/handlerUtils.ts Implements checkRecoveryMode utility function with error handling
src/tests/config.test.ts Adds test for recovery mode configuration reading
src/tests/api/master/recoveryWallet.test.ts Updates test config to enable recovery mode
src/tests/api/master/recoveryConsolidationsWallet.test.ts Updates test config to enable recovery mode
src/tests/api/master/nonRecovery.test.ts Adds comprehensive test suite for disabled recovery mode scenarios
src/tests/api/master/musigRecovery.test.ts Updates test config to enable recovery mode
Comments suppressed due to low confidence (2)

src/tests/api/master/nonRecovery.test.ts:101

  • The test only verifies that an error occurs but doesn't validate the specific error message or type. Consider adding assertions to verify the exact error message matches the one thrown by checkRecoveryMode.
    it('should fail to run recovery if not in recovery mode', async () => {

src/tests/api/master/nonRecovery.test.ts:137

  • The test only checks the status code but doesn't validate the error message or details. Consider adding assertions to verify the response contains the expected error message from checkRecoveryMode.
    it('should fail to run recovery consolidation if not in recovery mode', async () => {

@cpatino-intive cpatino-intive force-pushed the WP-5318/hide-recovery-apis-mbe branch 2 times, most recently from 824a322 to 9db337e Compare July 24, 2025 19:10
pranavjain97
pranavjain97 previously approved these changes Jul 24, 2025
Copy link
Copy Markdown
Contributor

@pranavjain97 pranavjain97 left a comment

Choose a reason for hiding this comment

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

Rebase to master

getAsUser: sinon.stub(),
register: sinon.stub(),
} as unknown as BitGoAPI;

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.

this shoudn't be needed; you can just create a bitgo instance. See this for eg.

bitgo = new BitGoAPI({ env: 'test' });

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

fixed

@mohammadalfaiyazbitgo
Copy link
Copy Markdown
Contributor

update conflicts.

@pranavjain97 pranavjain97 merged commit 4d04e20 into master Jul 25, 2025
4 checks passed
@pranavjain97 pranavjain97 deleted the WP-5318/hide-recovery-apis-mbe branch July 25, 2025 16:45
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