feat:revoke all permissions on site disconnect #36776
Open
+1,390
−231
Conversation
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
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** While trying to add release-pr-approval to required status checks, I noticed the release-pr-approval job is currently called `check` which is not accurately describing what the job is doing. This PR renames it to `release-pr-approval`. Similar problem as this [PR](#36028). [Same PR for Mobile repo](MetaMask/metamask-mobile#20856) <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/36624?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: null ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Renames the GitHub Actions job in `.github/workflows/release-pr-approval.yml` from `check` to `release-pr-approval`. > > - **CI/Workflows**: > - **Release PR Approval workflow** (`.github/workflows/release-pr-approval.yml`): > - Rename job `jobs.check` to `jobs.release-pr-approval`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit cd3df91. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…ct decimals (#36580) <!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Corrects the mUSD decimal value from 18 -> 6 in the default common token pair constants file. The incorrect decimal was causing erroneous quotes when the fromToken <-> toToken switch was flipped. <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/36580?quickstart=1) ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: fixed a bug that was causing incorrect quotes for mUSD to be displayed. ## **Related issues** Fixes: #36581 ## **Manual testing steps** 1. Go to ethereum or linea swaps 2. prefill a quote from eth -> mUSD 3. flip the tokens and observe a normal quote ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Updates mUSD decimals from 18 to 6 for mainnet and Linea bridge defaults and aligns selector test expectations. > > - **Bridge constants**: > - Update `BRIDGE_CHAINID_COMMON_TOKEN_PAIR` mUSD `decimals` to `6` for `eip155:1` (mainnet) and `eip155:59144` (Linea) in `shared/constants/bridge.ts`. > - **Tests**: > - Align `getToToken` default mUSD `decimals` expectation to `6` in `ui/ducks/bridge/selectors.test.ts`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 6bbd62c. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: SteP-n-s <[email protected]>
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/36628?quickstart=1) Should make the lavamoat policy generation less flaky on CI ## **Changelog** <!-- If this PR is not End-User-Facing and should not show up in the CHANGELOG, you can choose to either: 1. Write `CHANGELOG entry: null` 2. Label with `no-changelog` If this PR is End-User-Facing, please write a short User-Facing description in the past tense like: `CHANGELOG entry: Added a new tab for users to see their NFTs` `CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker` (This helps the Release Engineer do their job more quickly and accurately) --> CHANGELOG entry: ## **Related issues** Fixes: #36600 ## **Manual testing steps** 1. CI shouldn't flake ## **Screenshots/Recordings** Not applicable ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Upgrade lavamoat-core to 16.7.1 with a patch to skip writing the merged policy, and align Lavamoat policy and lockfile. > > - **Build/Lavamoat**: > - **lavamoat-core**: Replace patch for `^16.2.2` with patched `16.7.1`; map `^15.2.1` and `^16.7.1` to `16.7.1` in `package.json`. > - **Patch changes**: Keep `src/loadPolicy.js` behavior to skip writing `policy.json` after merge; remove prior patch file for `16.2.2`. > - **Policy**: Update `lavamoat/build-system/policy.json` to allow `node:fs/promises` for `lavamoat>lavamoat-core` (replaces `node:fs/promises.writeFile`). > - **Lockfile**: Refresh `yarn.lock` to reflect `[email protected]` and related dependency adjustments. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 3148218. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…36541) ## **Description** Remove the deprecated `BackgroundColor.backgroundAlternativeSoft` design token and replace all usage with `BackgroundColor.backgroundAlternative`. This addresses part of the technical debt cleanup by removing the final deprecated design token that was incorrectly added to the design system. The changes include: - Removing `backgroundAlternativeSoft = 'background-alternative-soft'` from the BackgroundColor enum - Replacing all component usage of `BackgroundColor.backgroundAlternativeSoft` with `BackgroundColor.backgroundAlternative` - Removing associated CSS custom properties and classes for `background-alternative-soft` - Updating test snapshots to reflect the CSS class changes from `mm-box--background-color-background-alternative-soft` to `mm-box--background-color-background-alternative` **Note:** This PR completes the cleanup of deprecated alternative-soft tokens. Previous PRs addressed `textAlternativeSoft` and `iconAlternativeSoft`. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: #30144 ## **Manual testing steps** 1. Verify all bridge components render correctly with proper background colors 2. Check prepare bridge page displays backgrounds with appropriate styling 3. Confirm switch tokens button background appears as expected 4. Ensure overall bridge page styling is unchanged visually ## **Screenshots/Recordings** Not applicable - this is a technical debt cleanup with no visual changes expected. ## **Pre-merge author checklist** - [x] I've followed MetaMask Contributor Docs and MetaMask Extension Coding Standards - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [ ] I've documented my code using JSDoc format if applicable - [ ] I've applied the right labels on the PR (see labeling guidelines). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Removes `BackgroundColor.backgroundAlternativeSoft` and deletes related `background-alternative-soft` CSS vars/classes from bridge styles. > > - **Design System**: > - Remove `BackgroundColor.backgroundAlternativeSoft` from `ui/helpers/constants/design-system.ts`. > - **Bridge Styles** (`ui/pages/bridge/index.scss`): > - Delete theme-specific CSS variables for `--color-background-alternative-soft` (light/dark). > - Remove `.mm-box--background-color-background-alternative-soft` rule. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 82cfbb0. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
## **Description** This PR updates carousel component to improve spacing and layout by updating the margin bottom from `8px` to `12px` ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: N/A (UI improvement) ## **Manual testing steps** 1. Go to home page with carousel slides 2. Verify the carousel now has 12px margin bottom ## **Screenshots/Recordings** ### **Before** - Carousel had 8px margin <img width="418" height="622" alt="Screenshot 2025-10-03 at 2 52 53 PM" src="https://github.com/user-attachments/assets/343574e7-807e-41bb-90e0-e55593b2e739" /> <img width="1512" height="863" alt="Screenshot 2025-10-03 at 2 53 09 PM" src="https://github.com/user-attachments/assets/ce83cb90-da2e-4e8c-8c93-78a2a832a3bd" /> ### **After** - Carousel has 12px margin for better spacing - Same visual result but using Tailwind classes for better maintainability <img width="414" height="612" alt="Screenshot 2025-10-03 at 2 51 38 PM" src="https://github.com/user-attachments/assets/f9827fdf-0a0e-4ef3-baa4-0ba81e1e2eb8" /> <img width="1512" height="864" alt="Screenshot 2025-10-03 at 2 52 01 PM" src="https://github.com/user-attachments/assets/856efaf4-8634-413c-b350-120f4d2cdeb3" /> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Removes SCSS padding/margin from `carousel-container` and applies `px-4` and `mb-3` (12px) spacing directly on `CarouselWithEmptyState`. > > - **UI** > - `ui/components/multichain/account-overview/account-overview-layout.tsx` > - Adds `className="mb-3 px-4"` to `CarouselWithEmptyState` to control spacing via utility classes. > - **Styles** > - `ui/components/multichain/carousel/index.scss` > - Removes `padding: 0 16px` and `margin-bottom: 8px` from `.carousel-container` (spacing now handled by component props). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e1cc3f1. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
…into feat/revoke-all-permissions-on-site-disconnect
…https://github.com/MetaMask/metamask-extension into feat/revoke-all-permissions-on-site-disconnect
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
](https://codespaces.new/MetaMask/metamask-extension/pull/36624?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/36580?quickstart=1){kind=link}
](https://codespaces.new/MetaMask/metamask-extension/pull/36628?quickstart=1){kind=link}
✨ Files requiring CODEOWNER review ✨🔑 @MetaMask/accounts-engineers (3 files, +119 -10)
👨🔧 @MetaMask/core-extension-ux (8 files, +531 -2)
🧩 @MetaMask/extension-devs (1 files, +1 -1)
📜 @MetaMask/policy-reviewers (1 files, +1 -1)
Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. 🔗 @MetaMask/supply-chain (1 files, +1 -1)
🔄 @MetaMask/swaps-engineers (2 files, +1 -19)
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Results generated automatically by MetaMask CI |
Builds ready [b44b4f3]
UI Startup Metrics (1270 ± 57 ms)
|
|
❌ test-e2e-chrome-api-specs failed. View the html report here. |
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Results generated automatically by MetaMask CI |
Builds ready [ee1f5b1]
UI Startup Metrics (1244 ± 81 ms)
|
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Results generated automatically by MetaMask CI |
Builds ready [e3c20d1]
UI Startup Metrics (1141 ± 71 ms)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
Description
This PR adds a modal before disconnecting from a site that shows permissions we have granted for this site and offers and option to revoke all the permissions.
Related issues
Is dependent on: #36635
Manual testing steps
Note that removing permissions does create an onchain transaction but does not yet actually delete permissions from the storage. This will be addresses in a separate PR.
Screenshots/Recordings
Before
Screen.Recording.2025-10-10.at.16.03.22.mov
After
Screen.Recording.2025-10-10.at.15.58.17.mov
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Adds a modal when disconnecting a site to review/remove token-transfer permissions and implements multi-chain revocation, with i18n updates and minor build/UI tweaks.
DisconnectPermissionsModalwith Skip/Remove all; integrated intoMultichainReviewPermissionsdisconnect flow, including token/amount formatting and account/network context.PermissionsCellrendering logic gating; exports modal; minor layout tweaks (carousel padding/classes).shared/lib/gator-permissions(token info fetch/caching, period/amount formatting, display metadata).getTokenTransferPermissionsByOriginand tests; page tests updated.useRevokeGatorPermissionsaddsrevokeGatorPermissionsBatchMultiChainusingfindNetworkClientIdByChainId.daily/weekly/monthly,everyX*,perSecond,tokenStream,tokenSubscription,skip,removeAll, and copy for "Other permissions on this site" (en, en_GB).mUSDdecimals to6on Mainnet and Linea; updates corresponding test.[email protected]with patch to skip policy write; policy allowsnode:fs/promises; remove old patch references.release-pr-approval.Written by Cursor Bugbot for commit e3c20d1. This will update automatically on new commits. Configure here.