chore: refactor permission rules and add additional validation by jeffsmale90 · Pull Request #7844 · MetaMask/core

jeffsmale90 · 2026-02-04T22:49:27Z

Explanation

It's critical that the GatorPermissionController's Permission decoding logic is strict, and will not decode EIP-712 payload to a permission unless the payload exactly meets the expectations of that permission.

This PR refactors the permission rules to be more self contained, and self describing. This allows each permission type's decoding rules to be more thoroughly tested in isolation. Validation and decoding logic is combined, as decoding is an implicit part of the validation step.

This PR also adds explicit validation of the "implicit" caveats for each permission type (valueLte for ERC20 permissions, exactCalldata for native token permissions), where previously we were ensuring that they caveats exist, but not validating their terms.

References

Checklist

I've updated the test suite for new or updated code as appropriate
I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
I've communicated my changes to consumers by updating changelogs for packages I've changed
I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

Medium Risk
Refactors core permission decoding/validation logic, which can cause previously accepted delegations to fail decoding if their caveat terms are malformed or slightly non-conforming. While largely additive hardening with extensive new tests, it touches security-sensitive permission interpretation paths.

Overview
Permission decoding is refactored to be rule-driven and stricter. GatorPermissionsController.decodePermissionFromPermissionContextForOrigin now builds per-chain permission rules, selects the single matching rule by caveat/enforcer addresses (findRuleWithMatchingCaveatAddresses), and validates+decodes terms via validateAndDecodePermission, failing decoding on any invalid terms.

Adds per-permission validation/decoding rules with expanded test coverage. Introduces createPermissionRulesForContracts and rule modules for each supported permission type (native/erc20 stream/periodic and erc20 revocation), adding explicit validation for “implicit” caveats (ExactCalldataEnforcer must be 0x, ValueLteEnforcer must be zero/32-bytes) plus stricter term length/field checks; replaces the older identifyPermissionByEnforcers/getPermissionDataAndExpiry approach and updates/expands tests accordingly.

^{Written by Cursor Bugbot for commit f956a28. This will update automatically on new commits. Configure here.}

…ion to ensure that permission data invariants are not violated.

…ype is self-describing and can be more easily tested in isolation. Add validation and test coverage for each permission type.

Plus minor changes: - Remove redundant amendment to ChecksumCaveat type - Remove unused ValidateDecodedPermission type - Fixes controller tests that expected the controller to self-report GatorPermissionsSnap id - Make decode functions internal, and rename to align with public interface

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

packages/gator-permissions-controller/src/GatorPermissionsController.ts

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts

…e ChecksumEnforcersByChainId type rather than explicitly declaring a new type

…ithout specifying snapId

…twice

mj-kiwi · 2026-03-03T04:47:45Z

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.ts

+  const startTime = hexToNumber(startTimeRaw);
+  const initialAmountBigInt = hexToBigInt(initialAmount);
+  const maxAmountBigInt = hexToBigInt(maxAmount);
+


Should we also add an amountPerSecond check here, similar to how it’s handled in native-token-stream ?

mj-kiwi · 2026-03-03T04:49:29Z

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenStream.test.ts

+  });
+
+  it('rejects when startTime is 0', () => {
+    const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddddd' as Hex;


I think the valid 20-byte addresses should be 40 hex chars.

Suggested change

const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddddd' as Hex;

const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddd' as Hex;

mj-kiwi · 2026-03-03T04:50:33Z

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts

+  });
+
+  it('rejects when startTime is 0', () => {
+    const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddddd' as Hex;


Suggested change

const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddddd' as Hex;

const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddd' as Hex;

Lol, I counted those ds very carefully!

mj-kiwi · 2026-03-03T04:51:17Z

packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts

+  const [periodAmount, periodDurationRaw, startTimeRaw] = splitHex(
+    terms,
+    [32, 32, 32],
+  );
+  const periodDuration = hexToNumber(periodDurationRaw);
+  const startTime = hexToNumber(startTimeRaw);
+
+  if (periodDuration <= 0) {
+    throw new Error(
+      'Invalid native-token-periodic terms: periodDuration must be a positive number',
+    );
+  }
+
+  if (startTime <= 0) {
+    throw new Error(
+      'Invalid native-token-periodic terms: startTime must be a positive number',
+    );
+  }
+
+  return { periodAmount, periodDuration, startTime };


should we add periodAmount > 0 check here?

mj-kiwi · 2026-03-03T04:51:30Z

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.ts

+    splitHex(terms, [20, 32, 32, 32]);
+  const periodDuration = hexToNumber(periodDurationRaw);
+  const startTime = hexToNumber(startTimeRaw);
+


should we add periodAmount > 0 check here?

jeffsmale90 changed the title ~~Add adversarial tests for permission decoding. Add additional validation for token permission types.~~ chore: add adversarial tests for permission decoding. Add additional validation for token permission types. Feb 10, 2026

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch from 4cd5553 to df72ecd Compare March 2, 2026 03:27

jeffsmale90 changed the title ~~chore: add adversarial tests for permission decoding. Add additional validation for token permission types.~~ chore: refactor permission rules and add additional validation Mar 2, 2026

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch 2 times, most recently from 1bc5acb to a9450f5 Compare March 2, 2026 21:33

jeffsmale90 added 4 commits March 3, 2026 10:33

Add adversarial tests for permission decoding. Add additional validat…

7d34f4c

…ion to ensure that permission data invariants are not violated.

Refactor PermissionRule and rule generation so that each permission t…

a7d55ff

…ype is self-describing and can be more easily tested in isolation. Add validation and test coverage for each permission type.

Linting fixes

022efc4

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch from a9450f5 to 8700f23 Compare March 2, 2026 21:34

jeffsmale90 marked this pull request as ready for review March 2, 2026 21:34

jeffsmale90 requested a review from a team as a code owner March 2, 2026 21:34

cursor bot reviewed Mar 2, 2026

View reviewed changes

packages/gator-permissions-controller/src/GatorPermissionsController.ts Show resolved Hide resolved

packages/gator-permissions-controller/src/decodePermission/rules/erc20TokenPeriodic.test.ts Show resolved Hide resolved

Each permission rule now derives it's internal enforcers type from th…

e1cb91e

…e ChecksumEnforcersByChainId type rather than explicitly declaring a new type

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch from 52df231 to e1cb91e Compare March 2, 2026 23:29

jeffsmale90 added 3 commits March 3, 2026 13:16

Add test to ensure that the GatorPermissionsController instantiates w…

b747495

…ithout specifying snapId

Update GatorPermissionsController test to not instantiate controller …

a394291

…twice

Add changelog entry

f956a28

jeffsmale90 requested a review from a team as a code owner March 3, 2026 01:17

mj-kiwi reviewed Mar 3, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: refactor permission rules and add additional validation#7844

chore: refactor permission rules and add additional validation#7844
jeffsmale90 wants to merge 8 commits intomainfrom
chore/gator_permissions_decoding_adversarial_tests

jeffsmale90 commented Feb 4, 2026 •

edited by cursor bot

Loading

Uh oh!

cursor bot left a comment

Uh oh!

Uh oh!

Uh oh!

mj-kiwi Mar 3, 2026

Uh oh!

mj-kiwi Mar 3, 2026 •

edited

Loading

Uh oh!

mj-kiwi Mar 3, 2026

Uh oh!

jeffsmale90 Mar 3, 2026

Uh oh!

mj-kiwi Mar 3, 2026

Uh oh!

mj-kiwi Mar 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddddd' as Hex;
	const tokenAddress = '0xdddddddddddddddddddddddddddddddddddddd' as Hex;

Uh oh!

Conversation

jeffsmale90 commented Feb 4, 2026 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

References

Checklist

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mj-kiwi Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

mj-kiwi Mar 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mj-kiwi Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

jeffsmale90 Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

mj-kiwi Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

mj-kiwi Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jeffsmale90 commented Feb 4, 2026 •

edited by cursor bot

Loading

mj-kiwi Mar 3, 2026 •

edited

Loading