Skip to content
This repository was archived by the owner on Oct 22, 2024. It is now read-only.

Chore/prepare silent release#603

Merged
cryptotavares merged 35 commits intomainfrom
chore/prepare-silent-release
May 11, 2023
Merged

Chore/prepare silent release#603
cryptotavares merged 35 commits intomainfrom
chore/prepare-silent-release

Conversation

@cryptotavares
Copy link
Copy Markdown
Contributor

@cryptotavares cryptotavares commented Apr 11, 2023

Overview

Update submodule to latest Extension master. This is a preparatory PR for a silent release.

Changes

Root

  • Added a babel plugin to handle import/require statements for explicit TS files (example: import { test } from 'test.ts'). There are a couple of cases like this on the Extension codebase because when bundling it creates a reference for the test.ts explicitly. However on node, when building the TS code we end up with just the JS file. In this case, if the import/require statement is referencing the TS file, then it will fail to import.
  • Aligned dependencies with the Extension.
  • Patched Snaps node thread execution bug - Math was importing crypto instead of the endowment crypto.

Pipeline

  • Updated config.yml to fix the pipeline that gets triggered whenever a new version of the Extension gets released.

Common

None

App

  • Updated chromedriver dep as well as chrome version installed for e2e tests. Aligned versions with extension.
  • Added classnames.d.ts file to types folder - tsc was failing because we now are referencing (indirectly a file that is importing classnames)
  • Added a global fetch wrapper that uses node native fetch to non http/https requests - node-fetch only supports http/https protocol, and the NFTController is passing images data (data://) to fetch (which works out fine on the window).
  • Preventing other packages from overriding the global.fetch (using lavamoat policies). There is a bug on lavamoat policies where write does not include read.
  • Set the right browser.runtime.id on the desktop (once we have an extension client connection). This is required due to this. If the runtime.id is not the same as the Extension Id, then the confirmation will have subjectMetadata associated with. This will trigger a snaps getName which will break.
  • Explicit set node dns preferred option to ipv4 - Node 18 dns resolution defaults to whatever the OS resolves and not ipv4. This was causing issues with running the e2e tests.
  • Sets the infura project id is to 00000000000000000000000000000000 for tests. Otherwise Extension mocks will not be used.
  • Update the browser.runtime.getURL to synchronously create the URL manually for if the path that we are getting is home.html (also needed to figure out if the Extension connecting is running on chrome or firefox). In Desktop, due to proxying the request to the browser, browser.runtime.getURL is an async method. This was preventing going back from the Phishing blocker page back into the Extension home page.

@cryptotavares cryptotavares requested a review from a team April 11, 2023 07:49
@github-actions
Copy link
Copy Markdown

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.

@socket-security
Copy link
Copy Markdown

socket-security bot commented Apr 11, 2023

New dependency changes detected. Learn more about Socket for GitHub ↗︎


👍 No new dependency issues detected in pull request

Bot Commands

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

Ignoring: react-syntax-highlighter@15.5.0, @metamask/eth-json-rpc-provider@1.0.0, @metamask/message-manager@2.1.0, @metamask/swappable-obj-proxy@2.1.0, @metamask/eth-json-rpc-middleware@11.0.0, fault@1.0.4, format@0.2.2

Pull request alert summary
Issue Status
Critical CVE ✅ 0 issues
CVE ✅ 0 issues
Mild CVE ✅ 0 issues
Install scripts ✅ 0 issues
Native code ✅ 0 issues
Bin script confusion ✅ 0 issues
Bin script shell injection ✅ 0 issues
Filesystem access ✅ 0 issues
Network access ✅ 0 issues
Shell access ✅ 0 issues
Unresolved require ✅ 0 issues
Invalid package.json ✅ 0 issues
HTTP dependency ✅ 0 issues
Git dependency ✅ 0 issues
GitHub dependency ✅ 0 issues
No bug tracker ✅ 0 issues
No contributors or author data ✅ 0 issues
No README ✅ 0 issues
Deprecated ✅ 0 issues
New author ✅ 0 issues
Unstable ownership ✅ 0 issues
Non-existent author ✅ 0 issues
Unmaintained ✅ 0 issues
Unpublished package ✅ 0 issues
Potential typo squat ✅ 0 issues
Known Malware ✅ 0 issues
Telemetry ✅ 0 issues
Protestware/Troll package ✅ 0 issues
AI detected security risk ✅ 0 issues
AI warning ✅ 0 issues

📊 Modified Dependency Overview:

➕ Added Package Capability Access +/- Transitive Count Publisher
@metamask/eth-json-rpc-provider@1.0.0 None +0 gudahtt
@metamask/swappable-obj-proxy@2.1.0 None +0 metamaskbot
@metamask/obs-store@8.1.0 None +0 gudahtt
@metamask/approval-controller@2.1.1 None +3 metamaskbot
@metamask/message-manager@2.1.0 None +3 metamaskbot
@metamask/controller-utils@3.4.0 None +1 metamaskbot
react-syntax-highlighter@15.5.0 filesystem +7 simmerer
@types/node@18.16.3 None +0 types
⬆️ Updated Package Version Diff Added Capability Access +/- Transitive Count Publisher
@metamask/snaps-ui@0.32.2 0.28.0...0.32.2 None +0/-0 metamaskbot
@metamask/snaps-utils@0.32.2 0.28.0...0.32.2 None +6/-1 metamaskbot
@metamask/snaps-controllers@0.32.2 0.28.0...0.32.2 None +11/-6 metamaskbot
@metamask/rpc-methods@0.32.2 0.28.0...0.32.2 None +8/-4 metamaskbot
@metamask/eth-json-rpc-infura@8.0.0 7.0.0...8.0.0 None +1/-1 gudahtt
@metamask/eth-json-rpc-middleware@11.0.0 10.0.0...11.0.0 None +2/-0 metamaskbot
@metamask/key-tree@7.0.0 6.2.1...7.0.0 None +0/-0 metamaskbot
@metamask/permission-controller@3.2.0 1.0.2...3.2.0 None +4/-0 metamaskbot
chromedriver@111.0.0 110.0.0...111.0.0 None +1/-0 giggio
eth-block-tracker@7.0.1 6.1.0...7.0.1 None +1/-0 gudahtt

Currently the extension has explicit ts
import files (example - import * 'test.ts').
This is not a good pattern for node, as the
ts build will output js files. And thus during
the runtime the node process breaks as it cannot
find the imported file. On the Extension it does not
have any issues because of the bundling that we have
in place. It creates a reference to the path and as so
it can have ts and js files beig explicitly imported,
but in reality it is only running js.
The fix that we are doing here, is just to
remove the .ts file extension from any import (as the
ts file itself is already transpiled into js and
has the js file extension)
@cryptotavares cryptotavares force-pushed the chore/prepare-silent-release branch from cf5018a to 6b79e22 Compare April 11, 2023 15:13
@cryptotavares cryptotavares force-pushed the chore/prepare-silent-release branch from 3a6c8d8 to 15785e7 Compare April 12, 2023 13:30
@cryptotavares
Copy link
Copy Markdown
Contributor Author

@SocketSecurity ignore chromedriver@111.0.0 fsevents@1.2.13 playwright@1.32.3

There's a bug on snaps node
execution services that prevents
the Math to be used within a snap
env.
Issue: MetaMask/snaps#1347
This is in line with the node version
that electron 23 is using
The NFTController is always fetching the
NFT image, even if the image is a data base64
encoded. Fetching anything other than http/https
is supported in the browser but not by node-fetch
(which uses the underlying http/https node native modules).
To fix this we will use the native node fetch
(available by default since node 18) to perform
any fetch actions that are not http/https.
Update the node browser runtime id
whenever we get an internal connection
and the sender id does not match the
browser id
There's an issue with node 18
where dns will resolve using ipv6
and not fallback to ipv4. This was
causing the tests to fail, because
localhost resolves to ::1:<port>
instead of 127.0.0.1:<port>. This
was preventing from running the tests
locally
When running the extension e2e tests
it is expected that the infura project
id is set to 00000000000000000000000000000000.
If not we might not be hitting the mockserver
Firefox generates a different runtime id
for the extension. In order to get the
actual runtime id, we can parse the url
from an internal connection created by either
the popup, notification or fullscreen extension
This method is not async in the browser.
Due to that on the Extension we are calling
it synchronously to go back to the home page
(from the phishing blocked script). When on Desktop
we proxy this requests to the actual browser, meaning
that the request will be async. This was causing
the phishing detection to not return to the extension
home page when clicking on the back to safety button.
Also, it is worth mentioning that the
protocol for chrome and firefox extensions are different.
So added a method to update the browser info
accordingly and route to the correct url.
snaps 0.32.2 updated the bundled from
webpack to browserify, so we need to reintroduce
this patch (now adpated to browserify)
trezor connect and isomorphic fetch were
overriding global.fetch. Open a topic
with lavamoat team to clarify why
setting the policy to write
would not identify that the global.fetch
was previously set.
Mock connectRemote env type constants
@cryptotavares
Copy link
Copy Markdown
Contributor Author

cryptotavares commented May 4, 2023

@SocketSecurity ignore @metamask/eth-json-rpc-provider@1.0.0 @metamask/message-manager@2.1.0 @metamask/swappable-obj-proxy@2.1.0 react-syntax-highlighter@15.5.0 fault@1.0.4 format@0.2.2

This test is skiped in the extension.
There is a bung in the snaps controller.
@cryptotavares
Copy link
Copy Markdown
Contributor Author

@SocketSecurity ignore @metamask/eth-json-rpc-middleware@11.0.0

@MetaMask MetaMask deleted a comment May 8, 2023
"dotenv-cli": "^6.0.0",
"duplexify": "^4.1.1",
"electron": "^23.0.0",
"electron": "^23.2.2",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

Icon,
ICON_NAMES,
} from '../../../../submodules/extension/ui/components/component-library';
} from '../../../../submodules/extension/ui/components/component-library/icon/deprecated';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
} from '../../../../submodules/extension/ui/components/component-library/icon/deprecated';
} from '../../../../submodules/extension/ui/components/component-library/icon';

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are breaking changes between the icons/deprecated and the new icons... I decided to keep the deprecated one, otherwise I would need to do more changes to the Desktop UI components. We can do that on subsequent PR

Copy link
Copy Markdown
Contributor

@vinistevam vinistevam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing PR @cryptotavares, so many challenges overcome like the ip6, congrats!
Just a minor suggestion that might be worth double-checking.


if (['https:', 'http:'].includes(requestURL.protocol)) {
// Use node-fetch which supports proxying with global-agent rather than built-in fetch provided by Node 18
return nodeFetch(url, options);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@cryptotavares cryptotavares merged commit 62fa235 into main May 11, 2023
@cryptotavares cryptotavares deleted the chore/prepare-silent-release branch May 11, 2023 14:38
@github-actions github-actions bot locked and limited conversation to collaborators May 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants