Uh oh!
There was an error while loading. Please reload this page.
feat(wallet)!: wire PermissionController and SubjectMetadataController into default initialization - #9300
Closed
sirtimid wants to merge 6 commits into
Closed
Conversation
sirtimid added a commit
that referenced
this pull request
Jun 29, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sirtimid
marked this pull request as ready for review
June 29, 2026 11:08
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1762832. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
sirtimid added a commit
that referenced
this pull request
Jun 29, 2026
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sirtimidforce-pushed
the
sirtimid/wire-permission-and-subject-metadata-controllers
branch
from
June 29, 2026 16:07
1762832 to
0b8cac1Comparesirtimid
commented
Jun 29, 2026
MemberAuthor
@metamaskbot publish-previews |
Contributor
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
…ller` into default initialization
Wire `PermissionController` and `SubjectMetadataController` (both from
`@metamask/permission-controller`) into the default wallet initialization.
The two reference each other's messenger actions and are wired as a pair;
`PermissionController` also consumes the already-wired `ApprovalController`
request actions.
Client-varying values are injectable `instanceOptions` with platform-agnostic
defaults: `permissionController.{caveatSpecifications,permissionSpecifications,
unrestrictedMethods}` (each defaulting to an empty set) and
`subjectMetadataController.subjectCacheLimit` (defaulting to 100).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…view - Add `Wallet.test.ts` integration coverage: both controllers are reachable on the assembled wallet, and persisted subject metadata hydrates without throwing (proving the default wiring initializes `PermissionController` before `SubjectMetadataController`, which calls `PermissionController:hasPermissions` during hydration). - Verify `PermissionController` can reach its delegated `ApprovalController` and `SubjectMetadataController` actions, so a dropped allowlist entry now fails. - Prove the injected permission specifications are forwarded (via `grantPermissions`) rather than only exercising the empty default. - Replace the weak "defaults to 100" test with a boundary test (100 retained, 101st evicts the oldest) and add coverage for retaining subjects that hold permissions past the cache limit. - Document the construction-order dependency and soften cross-repo comments. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…position `initialize` concatenated consumer-supplied `initializationConfigurations` ahead of the defaults, so a configuration overriding a default ran before the other defaults. A custom `SubjectMetadataController` with persisted state would then call `PermissionController:hasPermissions` during hydration before the default `PermissionController` registered that handler, throwing on construction. Overrides now occupy their default's position, preserving construction-order dependencies between defaults. Configurations that don't match a default remain additive and are initialized first. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rebasing #9300 onto `main` after five weeks of drift required these adjustments, none of which change the feature being wired: - `codeowners.ts` is now the source of truth for `.github/CODEOWNERS`, which is generated. `initializationPath` accepts an array so `permission-controller` can claim both of the instance directories it supplies, and rules are sorted by pattern so the generated section stays alphabetical. - `Wallet` tests pass the `gasFeeController` instance option, required since #9527. - Regenerate `README.md` and both `tsconfig`s via their fixers. - Repair the `[6.0.0]` changelog section, which a conflict resolution had left with duplicated content. - Spell out the client-facing consequences of the wiring in the changelog: the duplicate-registration collision for consumers passing their own messenger, and the fact that permission specifications with side effects (the Snaps specifications reach `SnapController:*`) need a wider delegation allowlist than the default configuration provides. - Cover `caveatSpecifications` injection, the one instance option without a behavioural assertion. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sirtimidforce-pushed
the
sirtimid/wire-permission-and-subject-metadata-controllers
branch
from
August 3, 2026 15:42
0b8cac1 to
3e7952bComparesirtimid added a commit
that referenced
this pull request
Aug 3, 2026
Correct the construction-order rationale, harden `initialize` against ambiguous configuration lists, and close the documentation and test gaps found while reviewing #9300. - The ordering comment claimed a module namespace object sorts its keys regardless of declaration order. That holds only for the ESM build; the CommonJS build — what Jest runs and what `require` consumers load — preserves declaration order. Record the real invariant on `instances/index.ts` and assert the two controllers' relative order. - `initialize` now throws on duplicate names in `initializationConfigurations` instead of silently keeping the last, which could drop an override carrying a wider allowlist. - Mark the override-ordering change `**BREAKING:**`: it applies to any overridden default, not just the two controllers wired here. - Correct the changelog claim that the default `PermissionController` "gates nothing" — with no specifications and no unrestricted methods it is maximally restrictive — and document that subject metadata is retained only for origins present in `PermissionController` state. - Export `InitializationConfiguration`, which a consumer needs to write the override the Snaps guidance calls for, and state what that override actually costs. - Document that `subjectCacheLimit` must be a positive integer, drop the `ConstructorParameters` indirection that resolved to `number`, and cover all five delegated actions plus the reversed-override ordering. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sirtimid added a commit
that referenced
this pull request
Aug 3, 2026
Correct the construction-order rationale, harden `initialize` against ambiguous configuration lists, and close the documentation and test gaps found while reviewing #9300. - The ordering comment claimed a module namespace object sorts its keys regardless of declaration order. That holds only for the ESM build; the CommonJS build — what Jest runs and what `require` consumers load — preserves declaration order. Record the real invariant on `instances/index.ts` and assert the two controllers' relative order. - `initialize` now throws on duplicate names in `initializationConfigurations` instead of silently keeping the last, which could drop an override carrying a wider allowlist. - Mark the override-ordering change `**BREAKING:**`: it applies to any overridden default, not just the two controllers wired here. - Correct the changelog claim that the default `PermissionController` "gates nothing" — with no specifications and no unrestricted methods it is maximally restrictive — and document that subject metadata is retained only for origins present in `PermissionController` state. - Export `InitializationConfiguration`, which a consumer needs to write the override the Snaps guidance calls for, and state what that override actually costs. - Document that `subjectCacheLimit` must be a positive integer, drop the `ConstructorParameters` indirection that resolved to `number`, and cover all five delegated actions plus the reversed-override ordering. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sirtimidforce-pushed
the
sirtimid/wire-permission-and-subject-metadata-controllers
branch
from
August 3, 2026 16:58
8c322dc to
14956e2CompareCorrect the construction-order rationale, harden `initialize` against ambiguous configuration lists, and close the documentation and test gaps found while reviewing #9300. - The ordering comment claimed a module namespace object sorts its keys regardless of declaration order. That holds only for the ESM build; the CommonJS build — what Jest runs and what `require` consumers load — preserves declaration order. Record the real invariant on `instances/index.ts` and assert the two controllers' relative order. - `initialize` now throws on duplicate names in `initializationConfigurations` instead of silently keeping the last, which could drop an override carrying a wider allowlist. - Mark the override-ordering change `**BREAKING:**`: it applies to any overridden default, not just the two controllers wired here. - Correct the changelog claim that the default `PermissionController` "gates nothing" — with no specifications and no unrestricted methods it is maximally restrictive — and document that subject metadata is retained only for origins present in `PermissionController` state. - Export `InitializationConfiguration`, which a consumer needs to write the override the Snaps guidance calls for, and state what that override actually costs. - Document that `subjectCacheLimit` must be a positive integer, drop the `ConstructorParameters` indirection that resolved to `number`, and cover all five delegated actions plus the reversed-override ordering. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
sirtimidforce-pushed
the
sirtimid/wire-permission-and-subject-metadata-controllers
branch
from
August 3, 2026 17:03
14956e2 to
8eb2c7fComparesirtimid
commented
Aug 3, 2026
MemberAuthor
@metamaskbot publish-previews |
Contributor
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
sirtimid
commented
Aug 12, 2026
MemberAuthor
stale |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
@metamask/walletis the shared controller-integration layer adopted by extension, mobile, and other clients. Values that differ between clients are injectableinstanceOptionswith platform-agnostic defaults rather than hardcoded.This PR wires
PermissionControllerandSubjectMetadataController(both from@metamask/permission-controller) into the default initialization. They reference each other's messenger actions, so they are wired together as a pair, andPermissionControlleralso consumes the already-wiredApprovalControllerrequest actions.The permission/caveat specifications and the unrestricted-method list vary substantially between clients (CAIP-25 account permissions, Snaps endowments, different method sets), so they are injected via
instanceOptions.permissionControllerand each defaults to an empty set.subjectCacheLimitdefaults to100, the value both clients use.Per-environment options
permissionController.caveatSpecifications{}permissionController.permissionSpecifications{}permissionController.unrestrictedMethods[]subjectMetadataController.subjectCacheLimit100100100Construction order
SubjectMetadataControllertrims persisted metadata in its constructor by callingPermissionController:hasPermissions, soPermissionControllermust be constructed first. Two options were available: make the dependency explicit ininitialize, or makeSubjectMetadataControllerorder-agnostic upstream (the approach taken forGasFeeController). The upstream fix is substantially larger here because the trim happens inside the constructor, so this PR relies on order instead:initializebuilds defaults in the orderinstances/index.tsexports them, and now slots an overriding configuration into its default's position so an override cannot break that.Note this ordering rests on the export order of
instances/index.ts, not on a language guarantee — the ESM build sorts namespace keys per spec, but the CommonJS build preserves declaration order.instances/index.tscarries a comment saying so, andinitialization.test.tsasserts the two controllers' relative order.References
Extension:
permission-controller-init.tsmessengers/permission-controller-messenger.tssubject-metadata-controller-init.tsmessengers/subject-metadata-controller-messenger.tsMobile:
controllers/permission-controller-init.tsmessengers/permission-controller-messenger.tscontrollers/subject-metadata-controller-init.tsmessengers/subject-metadata-controller-messenger.tsJira: N/A
Checklist
Note
High Risk
Touches permission gating and wallet bootstrap with multiple breaking consumer contracts (duplicate controllers, state keys, override ordering). Incorrect construction order or client migration could break dapp permission flows.
Overview
Breaking:
@metamask/walletnow buildsPermissionControllerandSubjectMetadataControllerby default, with messenger wiring toApprovalControllerand each other. Clients that already register either controller must drop their own wiring to avoid duplicate messenger actions.New optional
instanceOptions:permissionController(empty specs/methods until injected) andsubjectMetadataController.subjectCacheLimit(default 100). Persisted wallet state must include both controller keys when restoring subject metadata.initializebehavior changes: overrides ininitializationConfigurationsslot into each default’s position (soPermissionControllerstill runs beforeSubjectMetadataController), additive configs still run first, and duplicate configurationnamevalues throw.InitializationConfigurationis exported for custom overrides (e.g. Snaps specs needing a wider messenger allowlist).Supporting updates:
@metamask/permission-controllerdependency, initialization modules/tests, CODEOWNERS for both instance paths, README dependency graph.Reviewed by Cursor Bugbot for commit 8eb2c7f. Bugbot is set up for automated code reviews on this repo. Configure here.