Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cherry-pick: Fix Snaps usage of PhishingController #27833

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

FrederikBolding
Copy link
Member

@FrederikBolding FrederikBolding commented Oct 14, 2024

Description

Fixes two problems with Snaps usage of PhishingController. Following #25839 the PhishingController expects full URLs instead of hostnames as the input to testOrigin. In that PR, the argument of isOnPhishingList was incorrectly changed. This PR also patches in some changes from the snaps repo that are currently blocked by a release: MetaMask/snaps#2835,
MetaMask/snaps#2750

This PR cherry-picks a commit from develop that fixes this: 1f1e142

Open in GitHub Codespaces

Manual testing steps

  1. Create a Snap that links to an URL blocked with eth-phishing-detect
  2. See that triggering the Snap is disallowed if the user has phishing detection enabled

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

Fixes two problems with Snaps usage of `PhishingController`. Following
#25839 the
PhishingController expects full URLs instead of hostnames as the input
to `testOrigin`. In that PR, the argument of `isOnPhishingList` was
incorrectly changed. This PR also patches in some changes from the
`snaps` repo that are currently blocked by a release:
MetaMask/snaps#2835,
MetaMask/snaps#2750

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27817?quickstart=1)

1. Create a Snap that links to an URL blocked with `eth-phishing-detect`
2. See that triggering the Snap is disallowed if the user has phishing
detection enabled
@FrederikBolding FrederikBolding added the team-snaps-platform Snaps Platform team label Oct 14, 2024
@FrederikBolding FrederikBolding requested a review from a team as a code owner October 14, 2024 14:50
Copy link

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: npm/@metamask/[email protected], npm/@metamask/[email protected]

View full report↗︎

@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Oct 14, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [9c7593e]
Page Load Metrics (1705 ± 51 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint29619851644332159
domContentLoaded14771931168210751
load15221942170510751
domInteractive16112452612

@danjm danjm merged commit 6e73681 into Version-v12.5.0 Oct 15, 2024
75 of 76 checks passed
@danjm danjm deleted the fb/snaps-phishing-detect-rc-cherry-pick branch October 15, 2024 11:50
@github-actions github-actions bot locked and limited conversation to collaborators Oct 15, 2024
@metamaskbot metamaskbot added the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Oct 15, 2024
@metamaskbot
Copy link
Collaborator

No release label on PR. Adding release label release-12.5.0 on PR, as PR was cherry-picked in branch 12.5.0.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
INVALID-PR-TEMPLATE PR's body doesn't match template release-12.5.0 Issue or pull request that will be included in release 12.5.0 team-snaps-platform Snaps Platform team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants