Skip to content

Commit bd5249d

Browse files
rekmarksclaude
andauthored
refactor(permission-controller)!: Add messenger alternative to restricted method hooks (MetaMask#8551)
## Explanation Adds a `messenger` option to permission specification builders as an alternative to `methodHooks`, aligning spec builders with the messenger-based method middleware introduced in MetaMask#8506. Restricted-method specs can now declare the root-messenger actions they need via `actionNames` and consume a minimally-scoped `Messenger`. Incidentally, also removes the `factoryHooks` and `validatorHooks` fields from the spec builder surface. Both of these were unused in practice. ## References - Related to MetaMask#8506 - Preview build PRs: - MetaMask/metamask-extension#42128 - Extension CI failing as of merge due to unrelated problems, but succeeds when testing manually. - MetaMask/metamask-mobile#29343 - MetaMask/snaps#3969 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [x] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Breaking type-surface changes to permission specification builders (removing `factoryHooks`/`validatorHooks` and adding optional scoped `messenger`) may require downstream updates; new messenger delegation logic could impact how restricted-method specs invoke host actions. > > **Overview** > Restricted-method permission specification builders can now receive an optional scoped `messenger` (constructed from spec-declared `actionNames`) as an alternative to relying on `methodHooks`, enabling specs to call only explicitly-delegated host actions. > > Introduces `createRestrictedMethodMessenger` (exported from the package) to create a child `Messenger` with a minimal delegated action surface, and adds tests covering delegation behavior and the new builder option. **BREAKING:** removes `factoryHooks`, `validatorHooks`, and related export fields from the permission specification builder API surface. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 1e2fe74. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 2c255a6 commit bd5249d

6 files changed

Lines changed: 322 additions & 28 deletions

File tree

packages/permission-controller/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Added
1111

1212
- Add `createPermissionMiddlewareV2`, a `JsonRpcEngineV2` variant of the standalone permission middleware factory ([#8532](https://github.com/MetaMask/core/pull/8532))
13+
- Add `messenger` option to permission specification builders, allowing restricted-method specs to receive a scoped messenger in place of `methodHooks` ([#8551](https://github.com/MetaMask/core/pull/8551))
14+
- Use the `actionNames` field on the specification builder and `createRestrictedMethodMessenger` to construct the scoped messenger.
1315

1416
### Changed
1517

@@ -25,6 +27,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2527

2628
- Deprecate `createPermissionMiddleware` in favor of `createPermissionMiddlewareV2`, which targets `JsonRpcEngineV2` ([#8532](https://github.com/MetaMask/core/pull/8532))
2729

30+
### Removed
31+
32+
- **BREAKING:** Remove `factoryHooks`, `validatorHooks`, and related fields from permission specification builders ([#8551](https://github.com/MetaMask/core/pull/8551))
33+
2834
## [12.3.0]
2935

3036
### Added

packages/permission-controller/src/Permission.test.ts

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
1-
import type { CaveatConstraint, PermissionConstraint } from '.';
2-
import { constructPermission } from '.';
1+
import { Messenger } from '@metamask/messenger';
2+
3+
import type {
4+
CaveatConstraint,
5+
PermissionConstraint,
6+
PermissionSpecificationBuilder,
7+
RestrictedMethodSpecificationConstraint,
8+
} from '.';
9+
import { constructPermission, PermissionType } from '.';
10+
import { createRestrictedMethodMessenger } from './createRestrictedMethodMessenger';
311
import { findCaveat } from './Permission';
412

513
describe('constructPermission', () => {
@@ -86,3 +94,73 @@ describe('findCaveat', () => {
8694
expect(findCaveat(permission, 'doesNotExist')).toBeUndefined();
8795
});
8896
});
97+
98+
describe('permission specification messenger option', () => {
99+
type HostAction = {
100+
type: 'Host:computeAnswer';
101+
handler: () => number;
102+
};
103+
104+
const targetName = 'wallet_getAnswer';
105+
106+
type HostRootMessenger = Messenger<'Root', HostAction>;
107+
108+
type SpecMessenger = ReturnType<
109+
typeof createRestrictedMethodMessenger<
110+
typeof targetName,
111+
HostRootMessenger,
112+
readonly ['Host:computeAnswer']
113+
>
114+
>;
115+
116+
const buildSpecificationBuilder = (): PermissionSpecificationBuilder<
117+
PermissionType.RestrictedMethod,
118+
{ messenger: SpecMessenger },
119+
RestrictedMethodSpecificationConstraint
120+
> => {
121+
return ({ messenger }) => ({
122+
permissionType: PermissionType.RestrictedMethod,
123+
targetName,
124+
allowedCaveats: null,
125+
methodImplementation: (): number => messenger.call('Host:computeAnswer'),
126+
});
127+
};
128+
129+
const getRootMessenger = (): HostRootMessenger => {
130+
const rootMessenger = new Messenger<'Root', HostAction>({
131+
namespace: 'Root',
132+
});
133+
const hostMessenger = new Messenger<
134+
'Host',
135+
HostAction,
136+
never,
137+
typeof rootMessenger
138+
>({ namespace: 'Host', parent: rootMessenger });
139+
hostMessenger.registerActionHandler('Host:computeAnswer', () => 42);
140+
return rootMessenger;
141+
};
142+
143+
it('invokes the spec-declared action via the scoped messenger', () => {
144+
const rootMessenger = getRootMessenger();
145+
const specificationBuilder = buildSpecificationBuilder();
146+
147+
const messenger = createRestrictedMethodMessenger({
148+
rootMessenger,
149+
namespace: targetName,
150+
actionNames: ['Host:computeAnswer'] as const,
151+
});
152+
153+
const specification = specificationBuilder({ messenger });
154+
155+
expect(specification.targetName).toBe(targetName);
156+
expect(specification.allowedCaveats).toBeNull();
157+
expect(specification.permissionType).toBe(PermissionType.RestrictedMethod);
158+
expect(
159+
specification.methodImplementation({
160+
method: targetName,
161+
params: [],
162+
context: { origin: 'example.com' },
163+
}),
164+
).toBe(42);
165+
});
166+
});

packages/permission-controller/src/Permission.ts

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -484,15 +484,19 @@ export type PermissionSpecificationConstraint =
484484
* Options for {@link PermissionSpecificationBuilder} functions.
485485
*/
486486
type PermissionSpecificationBuilderOptions<
487-
FactoryHooks extends Record<string, unknown>,
488487
MethodHooks extends Record<string, unknown>,
489-
ValidatorHooks extends Record<string, unknown>,
488+
SpecMessenger = unknown,
490489
> = {
491490
targetName?: string;
492491
allowedCaveats?: Readonly<NonEmptyArray<string>> | null;
493-
factoryHooks?: FactoryHooks;
494492
methodHooks?: MethodHooks;
495-
validatorHooks?: ValidatorHooks;
493+
/**
494+
* A messenger scoped to this permission specification. The messenger is
495+
* expected to have exactly the actions declared by the spec's `actionNames`
496+
* delegated to it; {@link createRestrictedMethodMessenger} is the canonical
497+
* way to construct it.
498+
*/
499+
messenger?: SpecMessenger;
496500
};
497501

498502
/**
@@ -504,35 +508,13 @@ type PermissionSpecificationBuilderOptions<
504508
export type PermissionSpecificationBuilder<
505509
Type extends PermissionType,
506510
Options extends PermissionSpecificationBuilderOptions<
507-
Record<string, unknown>,
508-
Record<string, unknown>,
509511
Record<string, unknown>
510512
>,
511513
Specification extends PermissionSpecificationConstraint & {
512514
permissionType: Type;
513515
},
514516
> = (options: Options) => Specification;
515517

516-
/**
517-
* A restricted method permission export object, containing the
518-
* {@link PermissionSpecificationBuilder} function and "hook name" objects.
519-
*/
520-
export type PermissionSpecificationBuilderExportConstraint = {
521-
targetName: string;
522-
specificationBuilder: PermissionSpecificationBuilder<
523-
PermissionType,
524-
PermissionSpecificationBuilderOptions<
525-
Record<string, unknown>,
526-
Record<string, unknown>,
527-
Record<string, unknown>
528-
>,
529-
PermissionSpecificationConstraint
530-
>;
531-
factoryHookNames?: Record<string, true>;
532-
methodHookNames?: Record<string, true>;
533-
validatorHookNames?: Record<string, true>;
534-
};
535-
536518
type ValidRestrictedMethodSpecification<
537519
Specification extends RestrictedMethodSpecificationConstraint,
538520
> =
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import { Messenger } from '@metamask/messenger';
2+
3+
import { createRestrictedMethodMessenger } from './createRestrictedMethodMessenger';
4+
5+
type FooAction = {
6+
type: 'Foo:ping';
7+
handler: () => string;
8+
};
9+
10+
type BarAction = {
11+
type: 'Bar:double';
12+
handler: (n: number) => number;
13+
};
14+
15+
type RootActions = FooAction | BarAction;
16+
17+
const getRootMessenger = (): Messenger<'Root', RootActions> => {
18+
const messenger = new Messenger<'Root', RootActions>({ namespace: 'Root' });
19+
const fooMessenger = new Messenger<'Foo', FooAction, never, typeof messenger>(
20+
{
21+
namespace: 'Foo',
22+
parent: messenger,
23+
},
24+
);
25+
const barMessenger = new Messenger<'Bar', BarAction, never, typeof messenger>(
26+
{
27+
namespace: 'Bar',
28+
parent: messenger,
29+
},
30+
);
31+
fooMessenger.registerActionHandler('Foo:ping', () => 'pong');
32+
barMessenger.registerActionHandler('Bar:double', (value) => value * 2);
33+
return messenger;
34+
};
35+
36+
describe('createRestrictedMethodMessenger', () => {
37+
it('returns undefined when actionNames is omitted', () => {
38+
const rootMessenger = getRootMessenger();
39+
40+
expect(
41+
createRestrictedMethodMessenger({
42+
rootMessenger,
43+
namespace: 'wallet_example',
44+
}),
45+
).toBeUndefined();
46+
});
47+
48+
it('returns undefined when actionNames is empty', () => {
49+
const rootMessenger = getRootMessenger();
50+
51+
expect(
52+
createRestrictedMethodMessenger({
53+
rootMessenger,
54+
namespace: 'wallet_example',
55+
// @ts-expect-error An empty array is rejected by the type system, but
56+
// the runtime handles it as a no-op.
57+
actionNames: [],
58+
}),
59+
).toBeUndefined();
60+
});
61+
62+
it('exposes the requested action on the returned messenger', () => {
63+
const rootMessenger = getRootMessenger();
64+
65+
const messenger = createRestrictedMethodMessenger({
66+
rootMessenger,
67+
namespace: 'wallet_example',
68+
actionNames: ['Foo:ping'] as const,
69+
});
70+
71+
expect(messenger.call('Foo:ping')).toBe('pong');
72+
});
73+
74+
it('uses the provided namespace for the returned messenger', () => {
75+
const rootMessenger = getRootMessenger();
76+
77+
const messenger = createRestrictedMethodMessenger({
78+
rootMessenger,
79+
namespace: 'wallet_example',
80+
actionNames: ['Foo:ping'] as const,
81+
});
82+
83+
// Registering an action under a different namespace must fail with an
84+
// error that names the messenger's configured namespace.
85+
expect(() =>
86+
// @ts-expect-error Deliberately registering outside the child's action
87+
// surface to probe its namespace.
88+
messenger.registerActionHandler('Other:noop', () => undefined),
89+
).toThrow(/wallet_example/u);
90+
});
91+
92+
it('rejects calls to actions that were not requested', () => {
93+
const rootMessenger = getRootMessenger();
94+
95+
const messenger = createRestrictedMethodMessenger({
96+
rootMessenger,
97+
namespace: 'wallet_example',
98+
actionNames: ['Foo:ping'] as const,
99+
});
100+
101+
expect(() =>
102+
// @ts-expect-error Intentionally calling an undelegated action.
103+
messenger.call('Bar:double', 2),
104+
).toThrow(/Bar:double/u);
105+
});
106+
107+
it('exposes every requested action when multiple are delegated', () => {
108+
const rootMessenger = getRootMessenger();
109+
110+
const messenger = createRestrictedMethodMessenger({
111+
rootMessenger,
112+
namespace: 'wallet_example',
113+
actionNames: ['Foo:ping', 'Bar:double'] as const,
114+
});
115+
116+
expect(messenger.call('Foo:ping')).toBe('pong');
117+
expect(messenger.call('Bar:double', 3)).toBe(6);
118+
});
119+
});
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import type {
2+
ActionConstraint,
3+
EventConstraint,
4+
MessengerActions,
5+
} from '@metamask/messenger';
6+
import { Messenger } from '@metamask/messenger';
7+
8+
/**
9+
* The subset of `RootMessenger`'s actions selected by `DelegatedActions`.
10+
*/
11+
type SelectedActions<
12+
RootMessenger extends Messenger<string, ActionConstraint, EventConstraint>,
13+
DelegatedActions extends readonly MessengerActions<RootMessenger>['type'][],
14+
> = Extract<
15+
MessengerActions<RootMessenger>,
16+
{ type: DelegatedActions[number] }
17+
>;
18+
19+
/**
20+
* Create a child messenger scoped to a restricted-method permission
21+
* specification, delegating only the spec's declared actions from the root
22+
* messenger. This produces a minimally-scoped messenger whose action surface
23+
* matches exactly what the spec has declared it needs.
24+
*
25+
* Returns `undefined` when `actionNames` is omitted — there is nothing to
26+
* scope, and the builder can be invoked without a messenger.
27+
*
28+
* @param args - The arguments.
29+
* @param args.rootMessenger - The root messenger to delegate actions from.
30+
* @param args.namespace - The namespace for the scoped child messenger,
31+
* typically the spec's `targetName`.
32+
* @param args.actionNames - The action types the specification requires,
33+
* typically the spec's declared `actionNames`. Must be a non-empty tuple of
34+
* action types that exist on the root messenger.
35+
* @returns A scoped child messenger with the requested actions delegated, or
36+
* `undefined` if no actions were requested.
37+
*/
38+
export function createRestrictedMethodMessenger<
39+
Namespace extends string,
40+
RootMessenger extends Messenger<string, ActionConstraint, EventConstraint>,
41+
DelegatedActions extends readonly [
42+
MessengerActions<RootMessenger>['type'],
43+
...MessengerActions<RootMessenger>['type'][],
44+
],
45+
>(args: {
46+
rootMessenger: RootMessenger;
47+
namespace: Namespace;
48+
actionNames: DelegatedActions;
49+
}): Messenger<
50+
Namespace,
51+
SelectedActions<RootMessenger, DelegatedActions>,
52+
never,
53+
RootMessenger
54+
>;
55+
56+
export function createRestrictedMethodMessenger<
57+
Namespace extends string,
58+
RootMessenger extends Messenger<string, ActionConstraint, EventConstraint>,
59+
>(args: {
60+
rootMessenger: RootMessenger;
61+
namespace: Namespace;
62+
actionNames?: undefined;
63+
}): undefined;
64+
65+
export function createRestrictedMethodMessenger<
66+
Namespace extends string,
67+
RootMessenger extends Messenger<string, ActionConstraint, EventConstraint>,
68+
DelegatedActions extends [
69+
MessengerActions<RootMessenger>['type'],
70+
...MessengerActions<RootMessenger>['type'][],
71+
],
72+
>({
73+
rootMessenger,
74+
namespace,
75+
actionNames,
76+
}: {
77+
rootMessenger: RootMessenger;
78+
namespace: Namespace;
79+
actionNames?: DelegatedActions;
80+
}):
81+
| Messenger<
82+
Namespace,
83+
SelectedActions<RootMessenger, DelegatedActions>,
84+
never,
85+
RootMessenger
86+
>
87+
| undefined {
88+
if (!actionNames?.length) {
89+
return undefined;
90+
}
91+
92+
const restrictedMethodMessenger = new Messenger<
93+
Namespace,
94+
SelectedActions<RootMessenger, DelegatedActions>,
95+
never,
96+
RootMessenger
97+
>({
98+
namespace,
99+
parent: rootMessenger,
100+
});
101+
102+
rootMessenger.delegate({
103+
actions: actionNames,
104+
messenger: restrictedMethodMessenger,
105+
});
106+
107+
return restrictedMethodMessenger;
108+
}

0 commit comments

Comments
 (0)