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

fix: Fix Snaps usage of PhishingController #27817

Merged
merged 1 commit into from
Oct 14, 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

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

@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 10:44
Copy link

socket-security bot commented Oct 14, 2024

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
@FrederikBolding FrederikBolding force-pushed the fb/fix-phishing-detect-with-snaps branch from d5dd7b3 to 2b891cb Compare October 14, 2024 13:53
Copy link

sonarcloud bot commented Oct 14, 2024

@FrederikBolding FrederikBolding added this pull request to the merge queue Oct 14, 2024
@metamaskbot
Copy link
Collaborator

Builds ready [2b891cb]
Page Load Metrics (1928 ± 87 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint25222841742510245
domContentLoaded15112277190018890
load15242283192818287
domInteractive208046188
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: -4 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 154 Bytes (0.00%)

Merged via the queue into develop with commit 1f1e142 Oct 14, 2024
78 checks passed
@FrederikBolding FrederikBolding deleted the fb/fix-phishing-detect-with-snaps branch October 14, 2024 14:46
@github-actions github-actions bot locked and limited conversation to collaborators Oct 14, 2024
@metamaskbot metamaskbot added the release-12.7.0 Issue or pull request that will be included in release 12.7.0 label Oct 14, 2024
@gauthierpetetin gauthierpetetin added release-12.6.0 Issue or pull request that will be included in release 12.6.0 and removed release-12.7.0 Issue or pull request that will be included in release 12.7.0 labels Oct 21, 2024
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.6.0 Issue or pull request that will be included in release 12.6.0 team-snaps-platform Snaps Platform team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants