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/matching logic and adds stricter term validation, which may cause previously-accepted permission contexts to be rejected if they are even slightly malformed. The change is well-covered by new rule-focused and adversarial tests but touches a security-sensitive boundary (EIP-712 permission decoding).

Overview
Refactors permission decoding to be rule-driven: GatorPermissionsController.decodePermissionFromPermissionContextForOrigin now builds permission rules per chain, finds the single matching rule by caveat/enforcer addresses, and uses the rule to validate and decode caveat terms in one step (replacing the older identifyPermissionByEnforcers + getPermissionDataAndExpiry flow).

Adds a new decodePermission/rules layer with per-permission validateAndDecodePermission implementations and shared utilities, tightening validation of caveat terms (e.g., ExactCalldataEnforcer must be 0x, ValueLteEnforcer must be 32-byte zero, term byte-length checks, positive periodAmount/durations/start times, and hex tokenAddress for ERC20 types). Extensive new tests cover each rule plus adversarial edge cases, and the changelog is updated accordingly.

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

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

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

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

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;

I think it is 40 hex chars:

> "0xdddddddddddddddddddddddddddddddddddddddd".length 42

"0x" followed by 40 "d"s is 42 chars

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!

Actually, the existing string if 42 chars, which is 20 bytes, which is correct.

I added validation to the erc20 permission decoders to ensure that the token addresses are the correct length.

Actually, I didn't, because we can't validate individual component lengths (the bytes are concatenated), but we do have terms length validation for each permission type.

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

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?

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

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

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

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

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

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

…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

…e ChecksumEnforcersByChainId type rather than explicitly declaring a new type

…ithout specifying snapId

…twice

- use metamask/utils isHexAddress instead of regex to validate addresses - use hexToBigInt to validate tokenPeriod instead of hexToNumber

…). Add missing initialAmount validation.

cursor

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.}

cursor · 2026-03-05T06:32:43Z

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

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


Token address has 42 hex chars (21 bytes) instead of 40

Medium Severity

The tokenAddress '0xdddddddddddddddddddddddddddddddddddddddd' contains 42 d characters (21 bytes) instead of the required 40 (20 bytes). When these tests manually construct terms via `0x${tokenAddress.slice(2)}...`, the resulting byte length is 1 byte too long. This causes the byte-length validation to reject the terms before reaching the intended validation (e.g., startTime must be a positive number or amountPerSecond must be a positive number), so these tests pass for the wrong reason and don't actually cover the validation they intend to test.

Additional Locations (2)

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

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

MoMannn

isSubset is now dead code and can be removed

MoMannn · 2026-03-05T07:19:10Z

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

+    );
+  }
+
+  if (periodAmountBigInt <= 0n) {


These can check only if === 0n. There are no negative numbers in solidity so its not possible for them to be less then 0.

MoMannn · 2026-03-05T07:20:51Z

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

+
+  const EXPECTED_VALUE_LTE_TERMS_BYTELENGTH = 32;
+
+  if (getByteLength(valueLteTerms) !== EXPECTED_VALUE_LTE_TERMS_BYTELENGTH) {


This check is redundant. The next if is validating valueLteTerms !== ZERO_32_BYTES which also validates the length.

MoMannn · 2026-03-05T07:28:25Z

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

+ *   const result = matchingRules[0].validateAndDecodePermission(caveats);
+ *   if (result.isValid) { ... result.expiry, result.data ... }
+ *
+ * getPermissionRuleMatchingCaveatTypes and getPermissionDataAndExpiry use these rules


I believe these functions got removed now?

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 3 times, most recently 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

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

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

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

mj-kiwi reviewed Mar 3, 2026

View reviewed changes

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

mj-kiwi reviewed Mar 3, 2026

View reviewed changes

cursor bot reviewed Mar 4, 2026

View reviewed changes

packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenPeriodic.ts Outdated Show resolved Hide resolved

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

cursor bot reviewed Mar 4, 2026

View reviewed changes

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

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch from 9990e4f to 5ff2c48 Compare March 4, 2026 01:30

jeffsmale90 requested a review from mj-kiwi March 4, 2026 01:30

cursor bot reviewed Mar 4, 2026

View reviewed changes

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

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

cursor bot reviewed Mar 4, 2026

View reviewed changes

packages/gator-permissions-controller/src/decodePermission/rules/nativeTokenStream.ts Outdated Show resolved Hide resolved

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

jeffsmale90 added 8 commits March 5, 2026 19:12

Add adversarial tests for permission decoding. Add additional validat…

2ed88c2

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

Refactor PermissionRule and rule generation so that each permission t…

bfb1bde

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

Linting fixes

7c1ec70

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

926429e

…e ChecksumEnforcersByChainId type rather than explicitly declaring a new type

Add test to ensure that the GatorPermissionsController instantiates w…

af21adb

…ithout specifying snapId

Update GatorPermissionsController test to not instantiate controller …

5b8f5d8

…twice

Minor fixups:

25a1116

- use metamask/utils isHexAddress instead of regex to validate addresses - use hexToBigInt to validate tokenPeriod instead of hexToNumber

jeffsmale90 added 4 commits March 5, 2026 19:14

Add amountPerSecond validation to erc20tokenstream

b8886fe

Trim whitespace

a175e39

Add missing validation to nativeTokenStream

92eba5c

Remove unnecessary maxAmount validation (already indirectly validated…

4b81a92

…). Add missing initialAmount validation.

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch 3 times, most recently from 830e176 to feff8e1 Compare March 5, 2026 06:26

Don't inline the hexToBigInt when validating

c7ba989

jeffsmale90 force-pushed the chore/gator_permissions_decoding_adversarial_tests branch from feff8e1 to c7ba989 Compare March 5, 2026 06:29

cursor bot reviewed Mar 5, 2026

View reviewed changes

MoMannn requested changes Mar 5, 2026

View reviewed changes

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


		const EXPECTED_VALUE_LTE_TERMS_BYTELENGTH = 32;

		if (getByteLength(valueLteTerms) !== EXPECTED_VALUE_LTE_TERMS_BYTELENGTH) {

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!

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Mar 5, 2026

Choose a reason for hiding this comment

Token address has 42 hex chars (21 bytes) instead of 40

Uh oh!

MoMannn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jeffsmale90 commented Feb 4, 2026 •

edited by cursor bot

Loading

mj-kiwi Mar 3, 2026 •

edited

Loading