feat: add SES experiment toggle (iOS)

[leotm]

Description

A feature toggle for SES, within our settings menu under experiments, enabled by default

TODO

• add react-native-mmkv
• MVP
• ensure only visible on iOS not Android
  • show only on iOS
  • or show if storage key exists
• check next to Blockaid experiment enabled
• fix test, update snapshot
• add descriptive content
• translate text
• update styling
• replace react-native-mmkv with react-native-default-preference
  • attempted in ea5674c but only partially working
• fix Android debug bundling on Hermes
• consider content revision to include Hardened JS and SES (Secure EcmaScript) terms
• update to newly discussed text with link
• fix styling, move switch above next to title
• display both SES and Blockaid under same Security heading
• preserve Android UI
• update SES to be enabled by default

Related issues

Fixes: partially mitigates rollout risk

Follow-up to

• fix: regenerator-runtime and reenable SES (v1.1.0) on iOS (JSC) #8033

Manual testing steps

iOS

• boot app
• login / create wallet
• go to bottom tab Settings menu
• open Experimental menu
• SES enabled by default, like currently in (main) branch
• disable SES ft toggle
• reboot app
• SES now disabled
• verify in Experimental menu
• repeat steps above to re-enable

Android

• boot app
• login / create wallet
• go to bottom tab Settings menu
• open Experimental menu
• UI remains the same
  • Blockaid shown if feature is enabled

Screenshots/Recordings

Previous MVP video (old UI) demo showing the behaviour, a simple settings menu ft toggle that persists, enabling/disabling lockdown upon app reboot, on iOS only

Android remains unchanged (Blockaid disabled, Blockaid enabled)

Before

Blockaid ft disabled

Blockaid ft enabled

After

Blockaid ft disabled

Blockaid ft enabled

Navigating to Learn more

Pre-merge author checklist

• I’ve followed MetaMask Coding Standards.
• I've clearly explained what problem this PR is solving and how it is solved.
• I've linked related issues
• I've included manual testing steps
• I've included screenshots/recordings if applicable
• 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.
• I’ve properly set the pull request status:
  • In case it's not yet "ready for review", I've set it to "draft".
  • In case it's "ready for review", I've changed it from "draft" to "non-draft".

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]

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.

[codecov-commenter]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (a3ea469) 40.60% compared to head (c1f0d24) 40.62%.

Files	Patch %	Lines
...ents/Views/Settings/ExperimentalSettings/index.tsx	70.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8373      +/-   ##
==========================================
+ Coverage   40.60%   40.62%   +0.01%     
==========================================
  Files        1239     1239              
  Lines       29978    29989      +11     
  Branches     2868     2870       +2     
==========================================
+ Hits        12174    12182       +8     
- Misses      17107    17109       +2     
- Partials      697      698       +1     

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

[sethkfman]

@gauthierpetetin @Cal-L Can you take a look at this contribution to make sure it aligns with our platform for a product perspective?

@leotm Is there a more descriptive Header and content we can add so users and understand? Maybe something like Lockdown - This feature prevents dynamic modification of JavaScript running in the application.

[gauthierpetetin]

Hi @sethkfman , sorry I only see this now. I think end users won't understand the feature in its current form. I'll setup a meeting with @yanrong-chen @coreyjanssen @leotm and @hesterbruikman to discuss how we could present it.

[leotm]

@sethkfman i refactored to replace mmkv with default-preference ea5674c
but had to revert to mmkv, since default-preference only partially works here as a toggle
(fine in settings menu, but in InitializeCore .get is unable to return the storage value)

[github-actions]

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/7161d54a-73e6-4e27-aa77-f1e084500449
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

ios_e2e_build ✅
android_e2e_build ❌

Emitted 4 errors. exiting.
> Task :app:createBundleProdDebugJsAndAssets FAILED
> Task :react-native:ReactAndroid:hermes-engine:buildCMakeRelease[x86_64][libhermes]
C/C++: Hard link from '/root/.gradle/caches/transforms-3/67c92f014f4b560624ef31d7bf26af2b/transformed/fbjni-0.3.0/prefab/modules/fbjni/libs/android.x86_64/libfbjni.so' to '/bitrise/src/node_modules/react-native/ReactAndroid/hermes-engine/build/intermediates/cxx/Release/5v3r1w6e/obj/x86_64/libfbjni.so' failed. Doing a slower copy instead.
C/C++: Hard link from '/root/.gradle/caches/transforms-3/67c92f014f4b560624ef31d7bf26af2b/transformed/fbjni-0.3.0/prefab/modules/fbjni/libs/android.x86_64/libfbjni.so' to '/bitrise/src/node_modules/react-native/ReactAndroid/hermes-engine/build/intermediates/cxx/Release/5v3r1w6e/obj/x86_64/libfbjni.so' failed. Doing a slower copy instead.
C/C++: Hard link from '/opt/android-sdk-linux/ndk/23.1.7779620/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/x86_64-linux-android/libc++_shared.so' to '/bitrise/src/node_modules/react-native/ReactAndroid/hermes-engine/build/intermediates/cxx/Release/5v3r1w6e/obj/x86_64/libc++_shared.so' failed. Doing a slower copy instead.
C/C++: Hard link from '/opt/android-sdk-linux/ndk/23.1.7779620/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/x86_64-linux-android/libc++_shared.so' to '/bitrise/src/node_modules/react-native/ReactAndroid/hermes-engine/build/intermediates/cxx/Release/5v3r1w6e/obj/x86_64/libc++_shared.so' failed. Doing a slower copy instead.
FAILURE: Build completed with 2 failures.
1: Task failed with an exception.
-----------
* What went wrong:
java.lang.StackOverflowError (no error message)

test locally and re-run

nvm use && export METAMASK_BUILD_TYPE='main' && export METAMASK_ENVIRONMENT='local' && IGNORE_BOXLOGS_DEVELOPMENT="true" FORCE_BUNDLING=true yarn test:e2e:android:bitrise:run --testNamePattern="Smoke"

[github-actions]

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/6cae41a9-ae95-49d0-8e58-8fec25000b9a
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

ios_e2e_build ✅
android_e2e_build ✅

run_smoke_e2e_ios_android_stage ❌

TEST_SUITE_FOLDER value is: ./e2e/specs/confirmations/*
TEST_SUITE_TAG value is: .*SmokeConfirmations.*
v18.18.2
yarn run v1.22.19
$ IS_TEST='[REDACTED]' detox test -c ios.sim.debug './e2e/specs/confirmations/*' '--testNamePattern=.*SmokeConfirmations.*'
/bin/sh: emulator: command not found

---

https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/2c926904-9ef9-4e82-b7cd-5f3652ce10b6

ios_e2e_build ✅
android_e2e_build ✅

run_smoke_e2e_ios_android_stage ✅

[naugtur]

@leotm SES must be on by default. the toggle is a safety valve.

@sethkfman SES is on by default in the RC. You wanted the toggle. There's not a lot of review activity here and RC is cut. Is this not going in then?

[hesterbruikman]

@naugtur IMO we should not release SES without a toggle to disable. My understanding is that conventional dapp use will not break, but we can't possibly test for all dapp use cases and so users need to be able to turn this feature off.

Is there a reason this PR is in draft?

If SES is already in 7.16.0 RC I imagine we want to add merge and add this PR asap.

• Has any Learn more copy been prepared? @naugtur please let me know so we can involve the documentation team to add knowledge base content to link out to.
• Believe this needs a translation PR for the copy

[leotm]

@naugtur IMO we should not release SES without a toggle to disable. My understanding is that conventional dapp use will not break, but we can't possibly test for all dapp use cases and so users need to be able to turn this feature off.

Is there a reason this PR is in draft?

If SES is already in 7.16.0 RC I imagine we want to add merge and add this PR asap.

• Has any Learn more copy been prepared? @naugtur please let me know so we can involve the documentation team to add knowledge base content to link out to.
• Believe this needs a translation PR for the copy

agreed would be ideal, been pushing to get this merged last week prior our Thurs RC cut (extended to Fri)
so presuming this needs cherry-pick into 7.16.0, preferably also #8413

i converted this PR to draft only now, to make the change of SES enabled by default (instead of disabled)
then updating the screenshots and re-readying up
(i think this is more convenient than merging this, then 2 PRs to be cherry-picked into 7.16.0)

Suggestion agreed with @coreyjanssen @leotm and @hesterbruikman :

• Title: Code lockdown
• Description: This is a test feature that blocks any changes to the app's javascript code without permission. Learn more.

last Fri we agreed the Learn more link to be this above (see screenshot), who else should i speak to (on docs team) to clarify this is ok?

RE translations i checked with @tommasini last week, i've understood we usually do translations in a separate PR
but since already included them here last week, it's not worth removing them now to create a separate translations PR

[github-actions]

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/626c71dd-b9ce-44b0-b622-4a1d214e733e
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

[hesterbruikman]

Thanks @leotm ! And apologies, hadn't noticed the link for Learn more. Can you please explain the trade-off in enabling this feature?

I'd like to understand if we can treat the toggle as a temporary Experimental fallback, in case of unforeseen issues. Or if long-term there might always be end-user use cases to disable the feature under Advanced. In case of the latter we can reach out to the documentation team to create Knowledge Base content.

[leotm]

Thanks @leotm ! And apologies, hadn't noticed the link for Learn more. Can you please explain the trade-off in enabling this feature?

np! if by feature we mean this toggle for SES, the trade-off: our users might disable SES (reduce security), in the hopes of fixing an edge case of functionality (not yet covered by our tests)

or if by feature we mean SES, the trade-off: better security, at the compromise of possibly encountering an edge case of functionality not working as expected (i.e. discovery of a library we're using that calls non-standard JS)

I'd like to understand if we can treat the toggle as a temporary Experimental fallback, in case of unforeseen issues. Or if long-term there might always be end-user use cases to disable the feature under Advanced. In case of the latter we can reach out to the documentation team to create Knowledge Base content.

exactly the toggle is a temp Experimental fallback, long-term we want SES enabled indefinitely (like in mm-extension) for better security (permanently prevent prototype pollution - now that's a tongue twister), then ofc run LavaMoat (which runs on SES)

[sethkfman]

LGTM

[github-actions]

E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/977b8808-bfd5-4a59-ab1c-fc31e0bc5c57
You can also kick off another Bitrise E2E smoke test by removing and re-applying the (Run Smoke E2E) label

[hesterbruikman]

Thanks @leotm ! And apologies, hadn't noticed the link for Learn more. Can you please explain the trade-off in enabling this feature?

np! if by feature we mean this toggle for SES, the trade-off: our users might disable SES (reduce security), in the hopes of fixing an edge case of functionality (not yet covered by our tests)

or if by feature we mean SES, the trade-off: better security, at the compromise of possibly encountering an edge case of functionality not working as expected (i.e. discovery of a library we're using that calls non-standard JS)

I'd like to understand if we can treat the toggle as a temporary Experimental fallback, in case of unforeseen issues. Or if long-term there might always be end-user use cases to disable the feature under Advanced. In case of the latter we can reach out to the documentation team to create Knowledge Base content.

exactly the toggle is a temp Experimental fallback, long-term we want SES enabled indefinitely (like in mm-extension) for better security (permanently prevent prototype pollution - now that's a tongue twister), then ofc run LavaMoat (which runs on SES)

Just to conclude on this: I don't think we need further Learn more end-user documentation.

[sonarcloud]

Quality Gate passed

The SonarCloud Quality Gate passed, but some issues were introduced.

1 New issue
0 Security Hotspots
80.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud