feat: Add metrics to track where signature rejection occurred #26469

pnarayanaswamy · 2024-08-16T13:26:54Z

Add a property location to the Signature Rejected event depending on where it was rejected from.
location: confirmation or alert_friction_modal

Description

Related issues

Fixes: #2703

Manual testing steps

NOTE: All scenarios below have an E2E test in place along with this PR.

TC1:

Go to the test dapp
Initiate any signature
Reject it from the confirmation screen
Look for the Signature Rejected event in the network tab, the request should have the property location: "confirmation"

TC2:

Go to the test dapp
Initiate the signature 'SIGN IN WITH EHTEREUM (BAD DOMAIN)
Click on confirm
Reject it from the alert
Look for the Signature Rejected event in the network tab, the request should have the property location: "alert_friction_modal"

Screenshots/Recordings

Before

After

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.

github-actions · 2024-08-16T13:27:08Z

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 · 2024-08-19T14:56:32Z

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@metamask/[email protected]	None	`0`	252 kB	metamaskbot

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

View full report↗︎

socket-security · 2024-08-19T14:56:35Z

👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring: npm/@metamask/[email protected], npm/@types/[email protected], npm/@types/[email protected], npm/[email protected], npm/[email protected], npm/[email protected], npm/[email protected]

View full report↗︎

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of ecosystem/package-name@version specifiers. e.g. @SocketSecurity ignore npm/[email protected] or ignore all packages with @SocketSecurity ignore-all

pnarayanaswamy · 2024-08-21T08:30:22Z

@SocketSecurity ignore-all

matthewwalsh0 · 2024-08-21T05:45:06Z

shared/constants/metametrics.ts

@@ -820,6 +820,8 @@ export enum MetaMetricsEventLocation {
  TokenDetection = 'token_detection',
  TokenMenu = 'token_menu',
  Transaction = 'transaction',
+  Confirmation = 'confirmation',
+  AlertFrictionModal = 'alert_friction_modal',


Minor, but should we keep this enum in alphabetical order?

matthewwalsh0 · 2024-08-21T09:26:51Z

test/e2e/tests/confirmations/signatures/malicious-signatures.spec.ts

+        await driver.clickElement('#siweBadDomain');
+        await driver.switchToWindowWithTitle(WINDOW_TITLES.Dialog);
+
+        const alert = await driver.findElement('[data-testid="inline-alert"]');


Pending the full page object model, could large parts of this go into reusable helpers such as expectAlert or acknowledgeAlert or confirmWithAlertModal?

ui/components/app/alert-system/confirm-alert-modal/confirm-alert-modal.tsx

ui/pages/confirmations/components/confirm/footer/footer.tsx

metamaskbot · 2024-08-21T10:24:12Z

Builds ready [443ca41]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (79 ± 9 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	66	172	107	26	12
		domContentLoaded	41	117	73	20	10
		load	48	117	79	20	9
		domInteractive	9	62	29	13	6

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 66.23 KiB (1.86%)
ui: 545 Bytes (0.01%)
common: 74 Bytes (0.00%)

codecov · 2024-08-21T10:24:17Z

Codecov Report

Attention: Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.

Project coverage is 70.05%. Comparing base (c3069ee) to head (7bb6852).

Files	Patch %	Lines
...confirmations/components/confirm/footer/footer.tsx	88.89%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #26469   +/-   ##
========================================
  Coverage    70.05%   70.05%           
========================================
  Files         1414     1414           
  Lines        49317    49325    +8     
  Branches     13780    13780           
========================================
+ Hits         34546    34554    +8     
  Misses       14771    14771

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

metamaskbot · 2024-08-22T08:34:05Z

Builds ready [2427b8c]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (84 ± 10 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	75	174	114	24	11
		domContentLoaded	44	121	81	21	10
		load	50	132	84	21	10
		domInteractive	11	62	30	11	5

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 66.23 KiB (1.86%)
ui: 545 Bytes (0.01%)
common: 74 Bytes (0.00%)

metamaskbot · 2024-08-22T09:20:02Z

Builds ready [6d8e279]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (76 ± 10 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	67	147	103	20	10
		domContentLoaded	41	130	71	21	10
		load	47	133	76	21	10
		domInteractive	9	67	30	15	7

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 66.23 KiB (1.86%)
ui: 548 Bytes (0.01%)
common: 74 Bytes (0.00%)

ui/pages/confirmations/components/confirm/footer/footer.tsx

metamaskbot · 2024-08-23T13:53:18Z

Builds ready [d2bf9fb]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (83 ± 12 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	77	242	112	39	19
		domContentLoaded	57	148	79	24	12
		load	57	156	83	24	12
		domInteractive	19	66	32	13	6

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 66.23 KiB (1.83%)
ui: 548 Bytes (0.01%)
common: 74 Bytes (0.00%)

metamaskbot · 2024-08-26T08:16:47Z

Builds ready [cbcfc11]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (73 ± 6 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	69	130	96	16	8
		domContentLoaded	42	99	68	14	7
		load	49	99	73	13	6
		domInteractive	9	53	28	9	4

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 66.23 KiB (1.83%)
ui: 548 Bytes (0.01%)
common: 74 Bytes (0.00%)

metamaskbot · 2024-08-26T12:06:59Z

Builds ready [cfee05f]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (77 ± 11 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	69	141	102	19	9
		domContentLoaded	41	124	73	24	11
		load	48	124	77	22	11
		domInteractive	10	62	30	15	7

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 66.23 KiB (1.83%)
ui: 548 Bytes (0.01%)
common: 74 Bytes (0.00%)

metamaskbot · 2024-08-28T12:28:22Z

Builds ready [0c30dc2]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (86 ± 7 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	84	170	119	23	11
		domContentLoaded	48	116	83	17	8
		load	58	116	86	15	7
		domInteractive	9	67	34	13	6

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 33.3 KiB (0.96%)
ui: 548 Bytes (0.01%)
common: 74 Bytes (0.00%)

sonarqubecloud · 2024-08-28T14:18:06Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

metamaskbot · 2024-08-28T14:55:11Z

Builds ready [7bb6852]

builds: chrome, firefox
builds (beta): chrome
builds (flask): chrome, firefox
builds (MMI): chrome, firefox
builds (test): chrome, firefox
builds (test-flask): chrome, firefox
build viz: Build System
mv3: Background Module Init Stats
mv3: UI Init Stats
mv3: Module Load Stats
mv3: Bundle Size Stats
mv2: E2e Actions Stats
code coverage: Report
storybook: Storybook
typescript migration: Dashboard
all artifacts
bundle viz:
- background: 0, 1, 2, 3, 4, 5
- common: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9
- content-script: 0
- offscreen: 0
- ui: 0, 1, 10, 11, 2, 3, 4, 5, 6, 7, 8, 9

Page Load Metrics (87 ± 13 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	82	215	120	33	16
		domContentLoaded	46	169	84	29	14
		load	53	170	87	28	13
		domInteractive	11	75	30	17	8

Bundle size diffs [🚨 Warning! Bundle size has increased!]

background: 33.3 KiB (0.96%)
ui: 548 Bytes (0.01%)
common: 74 Bytes (0.00%)

pnarayanaswamy added 3 commits August 14, 2024 11:51

feat: add location to signature rejected event metric

ac7dd7f

Merge branch 'develop' into signature-alert-metrics

039a84f

test: add and update e2e for checking signature rejected events

81d6caa

pnarayanaswamy requested review from a team as code owners August 16, 2024 13:26

test: lint

07c3947

pnarayanaswamy added the team-confirmations Push issues to confirmations team label Aug 16, 2024

pnarayanaswamy added 2 commits August 19, 2024 16:51

chore: update package version of signature controller

e3e77f6

Merge branch 'develop' into signature-alert-metrics

6c0ebc0

pnarayanaswamy added 6 commits August 20, 2024 09:11

Merge branch 'develop' into signature-alert-metrics

8427c34

chore: update yarn lock file

26a2780

fix: ci errors lavamoat and yarn dedupe

e1aff87

Merge branch 'develop' into signature-alert-metrics

d64bed8

fix:restore lavamot policy json

b3cd861

fix:restore lavamot policy json files

443ca41

matthewwalsh0 requested changes Aug 21, 2024

View reviewed changes

Merge branch 'develop' into signature-alert-metrics

2427b8c

refactor: address review comments

6d8e279

matthewwalsh0 previously approved these changes Aug 22, 2024

View reviewed changes

ui/pages/confirmations/components/confirm/footer/footer.tsx Outdated Show resolved Hide resolved

Merge branch 'develop' into signature-alert-metrics

9d6b454

refactor: lint

0a313f7

pnarayanaswamy dismissed matthewwalsh0’s stale review via 0a313f7 August 23, 2024 11:36

pnarayanaswamy added 7 commits August 23, 2024 13:36

refactor: prettier

92dd044

refactor: use helper function

df4af4e

fix: yarn lock

029664c

fix: lint

1290c9f

refactor: rename onCancelType

e259b12

fix: flaky malicious signature test

a213b30

fix: failing malicious test

d2bf9fb

Merge branch 'develop' into signature-alert-metrics

cbcfc11

matthewwalsh0 previously approved these changes Aug 27, 2024

View reviewed changes

Merge branch 'develop' into signature-alert-metrics

0c30dc2

pnarayanaswamy dismissed matthewwalsh0’s stale review via 0c30dc2 August 28, 2024 11:49

pnarayanaswamy force-pushed the signature-alert-metrics branch from cfee05f to 0c30dc2 Compare August 28, 2024 11:49

matthewwalsh0 approved these changes Aug 28, 2024

View reviewed changes

jpuri approved these changes Aug 28, 2024

View reviewed changes

Merge branch 'develop' into signature-alert-metrics

7bb6852

pnarayanaswamy merged commit 99d2c86 into develop Aug 28, 2024
77 of 78 checks passed

pnarayanaswamy deleted the signature-alert-metrics branch August 28, 2024 14:49

github-actions bot locked and limited conversation to collaborators Aug 28, 2024

metamaskbot added the release-12.5.0 Issue or pull request that will be included in release 12.5.0 label Aug 28, 2024

gauthierpetetin added release-12.4.0 Issue or pull request that will be included in release 12.4.0 and removed release-12.5.0 Issue or pull request that will be included in release 12.5.0 labels Sep 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add metrics to track where signature rejection occurred #26469

feat: Add metrics to track where signature rejection occurred #26469

pnarayanaswamy commented Aug 16, 2024 •

edited by github-actions bot

Loading

github-actions bot commented Aug 16, 2024

socket-security bot commented Aug 19, 2024 •

edited

Loading

socket-security bot commented Aug 19, 2024 •

edited

Loading

pnarayanaswamy commented Aug 21, 2024

matthewwalsh0 Aug 21, 2024

pnarayanaswamy Aug 22, 2024

matthewwalsh0 Aug 21, 2024

pnarayanaswamy Aug 22, 2024

metamaskbot commented Aug 21, 2024

codecov bot commented Aug 21, 2024 •

edited

Loading

metamaskbot commented Aug 22, 2024

metamaskbot commented Aug 22, 2024

metamaskbot commented Aug 23, 2024

metamaskbot commented Aug 26, 2024

metamaskbot commented Aug 26, 2024

metamaskbot commented Aug 28, 2024

sonarqubecloud bot commented Aug 28, 2024

metamaskbot commented Aug 28, 2024

feat: Add metrics to track where signature rejection occurred #26469

feat: Add metrics to track where signature rejection occurred #26469

Conversation

pnarayanaswamy commented Aug 16, 2024 • edited by github-actions bot Loading

Description

Related issues

Manual testing steps

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

github-actions bot commented Aug 16, 2024

socket-security bot commented Aug 19, 2024 • edited Loading

socket-security bot commented Aug 19, 2024 • edited Loading

Next steps

pnarayanaswamy commented Aug 21, 2024

matthewwalsh0 Aug 21, 2024

Choose a reason for hiding this comment

pnarayanaswamy Aug 22, 2024

Choose a reason for hiding this comment

matthewwalsh0 Aug 21, 2024

Choose a reason for hiding this comment

pnarayanaswamy Aug 22, 2024

Choose a reason for hiding this comment

metamaskbot commented Aug 21, 2024

codecov bot commented Aug 21, 2024 • edited Loading

Codecov Report

metamaskbot commented Aug 22, 2024

metamaskbot commented Aug 22, 2024

metamaskbot commented Aug 23, 2024

metamaskbot commented Aug 26, 2024

metamaskbot commented Aug 26, 2024

metamaskbot commented Aug 28, 2024

sonarqubecloud bot commented Aug 28, 2024

Quality Gate passed

metamaskbot commented Aug 28, 2024

pnarayanaswamy commented Aug 16, 2024 •

edited by github-actions bot

Loading

socket-security bot commented Aug 19, 2024 •

edited

Loading

socket-security bot commented Aug 19, 2024 •

edited

Loading

codecov bot commented Aug 21, 2024 •

edited

Loading