Skip to content

refactor: migrate PPOMController to @metamask/messenger#249

Merged
cryptodev-2s merged 11 commits intomainfrom
mikesposito/messenger
Oct 27, 2025
Merged

refactor: migrate PPOMController to @metamask/messenger#249
cryptodev-2s merged 11 commits intomainfrom
mikesposito/messenger

Conversation

@mikesposito
Copy link
Member

@mikesposito mikesposito commented Sep 29, 2025

This PR migrates PPOMController to @metamask/messenger instead of @metamask/base-controller. This is part of a larger effort to migrate all controllers to use @metamask/messenger.


Note

Migrates PPOMController to the new @metamask/messenger API, updates state metadata and tests, and bumps related MetaMask dependencies (breaking).

  • Controller (PPOMController):
    • Migrate from RestrictedMessenger (base-controller) to Messenger (@metamask/messenger); update types (PPOMControllerMessenger) and method calls (messenger.call/subscribe/registerActionHandler).
    • Use StateMetadata<PPOMState> and rename metadata field anonymous to includeInDebugSnapshot.
  • Tests:
    • Rework messenger setup to use @metamask/messenger with namespacing/delegation; update expectations and deriveStateFromMetadata keys.
  • Dependencies:
    • Add @metamask/messenger and bump @metamask/base-controller to ^9.0.0.
    • Bump peer deps: @metamask/network-controller to ^25.0.0, @metamask/error-reporting-service to ^3.0.0.
  • Changelog:
    • Document breaking migration to @metamask/messenger, metadata rename, and peer dependency bumps.

Written by Cursor Bugbot for commit 2c3383f. This will update automatically on new commits. Configure here.

@mikesposito mikesposito requested a review from a team as a code owner September 29, 2025 17:30
@socket-security
Copy link

socket-security bot commented Sep 29, 2025

@mikesposito
Copy link
Member Author

@metamaskbot publish-preview

@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: Missing Parent Messenger Type in Definition

The PPOMControllerMessenger type definition is missing the generic parameter that specifies the parent messenger type. The new @metamask/messenger Messenger type requires 3-4 generic parameters: namespace, actions, events, and optionally a parent messenger. Looking at the test file (test-utils.ts lines 193-201), the correct pattern should be:

Messenger<'PPOMController', AllActions, AllEvents, RootMessenger>

However, the current definition only has 3 parameters and doesn't specify a parent messenger type, which will cause type errors when trying to use this messenger with a parent messenger as shown in the test setup.

Fix in Cursor Fix in Web

@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: Incorrect Delegation of Parent Actions

The messenger.delegate() call is attempting to delegate actions and events that don't belong to the child messenger's namespace. The delegate() method should only be called to grant access to actions/events from OTHER namespaces (like 'NetworkController'), not to register the parent's own actions. This code will likely fail because NetworkController:getNetworkClientById and NetworkController:networkDidChange are not actions/events owned by the PPOMController messenger being delegated. The delegation should be removed or restructured - the parent messenger should register these NetworkController handlers directly, not delegate them to the PPOMController messenger.

Fix in Cursor Fix in Web

@github-actions
Copy link
Contributor

A preview build for this branch has been published.

You can configure your project to use the preview build with this identifier:

npm:@metamask-previews/ppom-validator@0.38.0-preview-a101cca

See these instructions for more information about preview builds.

cryptodev-2s
cryptodev-2s previously approved these changes Oct 26, 2025
Copy link
Contributor

@cryptodev-2s cryptodev-2s left a comment

Choose a reason for hiding this comment

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

LGTM!

@socket-security
Copy link

socket-security bot commented Oct 27, 2025

Caution

MetaMask internal reviewing guidelines:

  • Do not ignore-all
  • Each alert has instructions on how to review if you don't know what it means. If lost, ask your Security Liaison or the supply-chain group
  • Copy-paste ignore lines for specific packages or a group of one kind with a note on what research you did to deem it safe.
    @SocketSecurity ignore npm/PACKAGE@VERSION
Action Severity Alert  (click "▶" to expand/collapse)
Block Medium
@metamask/eth-json-rpc-infura@10.3.0 has Network access.

Module: globalThis["fetch"]

Location: Package overview

From: ?npm/@metamask/network-controller@25.0.0npm/@metamask/eth-json-rpc-infura@10.3.0

ℹ Read more on: This package | This alert | What is network access?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/@metamask/eth-json-rpc-infura@10.3.0. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Block Medium
loglevel@1.9.1 has Network access.

Module: globalThis["fetch"]

Location: Package overview

From: ?npm/@metamask/network-controller@25.0.0npm/loglevel@1.9.1

ℹ Read more on: This package | This alert | What is network access?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Packages should remove all network access that is functionally unnecessary. Consumers should audit network access to ensure legitimate use.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/loglevel@1.9.1. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Block Low
@metamask/eth-json-rpc-infura@10.3.0 has a New author.

New Author: metamaskbot

Previous Author: gudahtt

From: ?npm/@metamask/network-controller@25.0.0npm/@metamask/eth-json-rpc-infura@10.3.0

ℹ Read more on: This package | This alert | What is new author?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: Scrutinize new collaborator additions to packages because they now have the ability to publish code into your dependency tree. Packages should avoid frequent or unnecessary additions or changes to publishing rights.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/@metamask/eth-json-rpc-infura@10.3.0. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn Low
klona@2.0.6 is a AI-detected potential code anomaly.

Notes: The analyzed code is a deep clone utility (klona) with careful handling to avoid prototype pollution and to clone common JavaScript types. The primary concern is that objects with non-plain constructors are instantiated via new x.constructor(), which could execute user-defined constructor logic and potentially have side effects. Aside from that, there are no malicious behaviors such as data exfiltration, backdoors, or network activity. Overall security risk is low to moderate depending on trust in input objects; the constructor path is the main anomaly to monitor in usage.

Confidence: 1.00

Severity: 0.60

From: ?npm/@metamask/network-controller@25.0.0npm/klona@2.0.6

ℹ Read more on: This package | This alert | What is an AI-detected potential code anomaly?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: An AI system found a low-risk anomaly in this package. It may still be fine to use, but you should check that it is safe before proceeding.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/klona@2.0.6. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

Warn Low
uuid@8.3.2 is a AI-detected potential code anomaly.

Notes: The code is a straightforward MD5 hashing utility with standard input normalization. It does not exhibit malicious behavior or data exfiltration. The primary concern is MD5’s cryptographic weaknesses for security-sensitive contexts rather than any malware or backdoors. Suitable for non-security-critical hashing tasks; avoid MD5 for password storage or integrity checks in security-sensitive applications.

Confidence: 1.00

Severity: 0.60

From: ?npm/@metamask/utils@11.8.1npm/@metamask/network-controller@25.0.0npm/uuid@8.3.2

ℹ Read more on: This package | This alert | What is an AI-detected potential code anomaly?

Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

Suggestion: An AI system found a low-risk anomaly in this package. It may still be fine to use, but you should check that it is safe before proceeding.

Mark the package as acceptable risk. To ignore this alert only in this pull request, reply with the comment @SocketSecurity ignore npm/uuid@8.3.2. You can also ignore all packages with @SocketSecurity ignore-all. To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

View full report

Co-authored-by: Mark Stacey <markjstacey@gmail.com>
Gudahtt
Gudahtt previously approved these changes Oct 27, 2025
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@cryptodev-2s cryptodev-2s requested a review from Gudahtt October 27, 2025 21:19
@cryptodev-2s cryptodev-2s merged commit e1ed516 into main Oct 27, 2025
14 of 15 checks passed
@cryptodev-2s cryptodev-2s deleted the mikesposito/messenger branch October 27, 2025 21:21
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.

Replace RestrictedMessenger with delegated Messenger

3 participants