feat(SafeShield): Malicious adresses in Shield threat review#6776
feat(SafeShield): Malicious adresses in Shield threat review#6776vmosharova merged 21 commits intodevfrom
Conversation
vmosharova
changed the title
|
@codex review this draft |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Branch preview✅ Deploy successful! Storybook: |
vmosharova
changed the title
📦 Next.js Bundle Analysis for @safe-global/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖
|
| Page | Size (compressed) |
|---|---|
global |
1.38 MB (🟡 +1 B) |
Details
The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.
Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis
If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!
Thirteen Pages Changed Size
The following pages changed size from the code in this PR compared to its base branch:
| Page | Size (compressed) | First Load |
|---|---|---|
/apps/open |
89.93 KB (-1 B) |
1.47 MB |
/bridge |
2.58 KB (🟢 -1 B) |
1.38 MB |
/earn |
630 B (🟢 -1 B) |
1.38 MB |
/home |
59.88 KB (🟡 +1 B) |
1.44 MB |
/settings/security |
20.82 KB (🟡 +1 B) |
1.4 MB |
/stake |
633 B (🟢 -1 B) |
1.38 MB |
/swap |
852 B (🟢 -3 B) |
1.38 MB |
/transactions |
137.87 KB (-3 B) |
1.52 MB |
/transactions/history |
137.83 KB (-2 B) |
1.52 MB |
/transactions/messages |
96.72 KB (-1 B) |
1.48 MB |
/transactions/msg |
93.18 KB (-2 B) |
1.47 MB |
/transactions/queue |
85.37 KB (-2 B) |
1.47 MB |
/transactions/tx |
84.73 KB (-2 B) |
1.47 MB |
Details
Only the gzipped size is provided here based on an expert tip.
First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.
Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis
Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this.
Coverage report for
|
St.❔ |
Category | Percentage | Covered / Total |
|---|---|---|---|
| 🟡 | Statements | 76.77% (-0.05% 🔻) |
17331/22576 |
| 🔴 | Branches | 54.21% (+0% 🔼) |
4671/8616 |
| 🟡 | Functions | 61.44% (-0.01% 🔻) |
2468/4017 |
| 🟡 | Lines | 78.14% (-0.09% 🔻) |
15756/20164 |
Show files with reduced coverage 🔻
St.❔ |
File | Statements | Branches | Functions | Lines |
|---|---|---|---|---|---|
| 🔴 | ... / withGuardCheck.tsx |
16.67% (-83.33% 🔻) |
0% (-100% 🔻) |
0% (-100% 🔻) |
16.67% (-83.33% 🔻) |
| 🔴 | ... / withOwnerCheck.tsx |
16.67% (-83.33% 🔻) |
0% (-100% 🔻) |
0% (-100% 🔻) |
16.67% (-83.33% 🔻) |
| 🔴 | ... / index.tsx |
50% (-50% 🔻) |
0% (-100% 🔻) |
50% (-50% 🔻) |
50% (-50% 🔻) |
| 🔴 | ... / index.tsx |
57.14% (-42.86% 🔻) |
0% (-100% 🔻) |
66.67% (-33.33% 🔻) |
57.14% (-42.86% 🔻) |
| 🟢 | ... / AnalysisIssuesDisplay.tsx |
93.94% (-6.06% 🔻) |
83.33% (-16.67% 🔻) |
85.71% (-14.29% 🔻) |
96.67% (-3.33% 🔻) |
Test suite run success
2554 tests passing in 306 suites.
Report generated by 🧪jest coverage report action from 92dc220
📸 Page ScreenshotsFound 1 screenshot(s) for pages affected by changes in this PR. Balances
Screenshots are automatically captured from pages affected by changed files. |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/utils/src/features/safe-shield/utils/transformThreatAnalysisResponse.ts
Show resolved
Hide resolved
…y component to support the new address type
…order to reuse it in the IssueDisplay component
… prop in the analysis results
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
...res/SafeShield/components/AnalysisGroup/AnalysisDisplay/components/AnalysisIssuesDisplay.tsx
Outdated
Show resolved
Hide resolved
…to avoid resetting for every severity group
apps/web/src/features/safe-shield/components/AnalysisIssuesDisplay/AnalysisIssuesDisplay.tsx
Outdated
Show resolved
Hide resolved
|
love the new design! 💟 |
.../mobile/src/features/SafeShield/components/AnalysisGroup/AnalysisDisplay/AnalysisDisplay.tsx
Show resolved
Hide resolved
...res/SafeShield/components/AnalysisGroup/AnalysisDisplay/components/AnalysisIssuesDisplay.tsx
Outdated
Show resolved
Hide resolved
...res/SafeShield/components/AnalysisGroup/AnalysisDisplay/components/AnalysisIssuesDisplay.tsx
Show resolved
Hide resolved
… undefined addresses
* feat: show malicious addresses in the Shield UI * feat: (packages) show malicious addresses in the Shield UI * chore: linting; fix tests * fix: handle edge cases with undefined issues and empty arrays; fix tests * fix: preserve all metadata fields (request_id, etc.) in the response destructuring * feat(mobile): Add address property in the malicious threat builders and storybook * feat(mobile): change the underlying layout of the AnalysisIssueDisplay component to support the new address type * feat(mobile): extract reusable analysis address logic to useAnalysisAddress hook * feat(mobile): move AddressItem wrapper to AnalysisPaper component in order to reuse it in the IssueDisplay component * fix(mobile): do not render 'Show All' component if there are 'issues' prop in the analysis results * fix(mobile): unit tests * fix(mobile): prettier * fix(mobile): Fixed the index collision in the mobile issues analysis to avoid resetting for every severity group * refactor: remove obsolete description check as it is always present in the objects in ThreatIssues[] * fix: new styles for the Threat description * fix(mobile): check result.issues truthiness and fix explorer link for undefined addresses --------- Co-authored-by: Clovis da Silva Neto <clovisdasilvaneto@gmail.com>
5 participants
What it solves
Displays malicious addresses from threat analysis issues in the UI. When the backend returns threat analysis results with issues containing addresses (e.g.,
{description: "Malicious contract interaction", address: "0x1234..."}), these addresses were not shown in the UI.Resolves: COR-937
The changes are implemented to reflect the API schema change in the BE (2824 and 2829
How this PR fixes it
Type definitions updated:
ThreatIssuetype with{description: string, address?: string}structureMaliciousOrModerateThreatAnalysisResultto useThreatIssue[]instead ofstring[]for issuesTransformation logic created:
transformThreatAnalysisResponseutility that extracts addresses fromThreatIssueobjects in theissuespropertyaddressesarray of each threat resultHook integration:
useThreatAnalysisto usetransformThreatAnalysisResponseto process threat data before returning itWeb + Mobile update:
The components + UI are updated both for Web and Mobile.
Test coverage:
Screenshots
How to test it
Checklist
transformThreatAnalysisResponseCLA signature
With the submission of this Pull Request, I confirm that I have read and agree to the terms of the Contributor License Agreement.
Note
Displays addresses from threat analysis issues across web and mobile by transforming API responses and updating UI to show/copy/open addresses with conditional address lists.
ThreatIssue({ description, address? }) and update threatissuestyping.transformThreatAnalysisResponseto extract issue addresses intoresult.addresses; integrate inuseThreatAnalysis.AnalysisDisplay: renderAnalysisIssuesDisplaywhenissuesexist; otherwise showShowAllAddress.AnalysisPaperwrapper anduseAnalysisAddresshook (copy-to-clipboard, explorer open).AddressListItemandShowAllAddressto use new hook and wrapper; minor key fixes inAnalysisGroup.AnalysisGroupCardItem: only showShowAllAddresswhen noissues; always renderAnalysisIssuesDisplayand address changes as applicable.AnalysisIssuesDisplay: show issue address (copy + optional explorer link) with description; ignore empty issues.Written by Cursor Bugbot for commit e0b44f3. This will update automatically on new commits. Configure here.