-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat: Update SES lockdown options #8413
Conversation
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. |
e6d6785
to
a0ce650
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8413 +/- ##
=======================================
Coverage 40.43% 40.43%
=======================================
Files 1239 1239
Lines 29976 29976
Branches 2875 2875
=======================================
Hits 12122 12122
Misses 17157 17157
Partials 697 697 ☔ View full report in Codecov by Sentry. |
a0ce650
to
164b9cd
Compare
- Make error stacks also available by the error instance stack - Preserve error stack filtered content - Like we do in metamask-extension - Improve error debugging in Sentry https://github.com/endojs/endo/blob/master/packages/ses/docs/reference.md#options-quick-reference https://github.com/MetaMask/metamask-extension/blob/develop/app/scripts/lockdown-run.js#L6
164b9cd
to
d9a9c6e
Compare
- Show full raw error info for each deep stack lvl - Preserve 'noise' that the default 'concise' option removes - Improve error debugging in Sentry https://github.com/endojs/endo/blob/master/packages/ses/docs/reference.md#options-quick-reference
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
E2E test started on Bitrise: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/7526129e-61ba-40cf-af57-62d2bf4e8f50 ios_e2e_build ✅ run_smoke_e2e_ios_android_stage ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Missing release label release-7.16.0 on PR. Adding release label release-7.16.0 on PR and removing other release labels(release-7.17.0), as PR was cherry-picked in branch 7.16.0. |
Description
Update SES lockdown options to improve error stacks
Set error taming to unsafe
Set stack filtering to verbose
concise
option was removingFollow-up to
Ref: https://github.com/endojs/endo/blob/master/packages/ses/docs/reference.md#options-quick-reference
Nb: we're looking into a lockdown/repairIntrinsics option to disable touching errors entirely (for cases like ours involving React Native surfacing JS/Android/iOS errors, then later newer engine Hermes)
Related issues
Manual testing steps
Local testing in debug-mode:
new Error('test')
Sentry.captureException(new Error('test'))
But note above we're looking into a better lockdown option for React Native debug-mode,
since we disabled SES in debug-mode earlier from React Devtools interfering
Production testing:
After we merge #8373, we'll be able to ft toggle lockdown via our in-app settings menu, which will persist the choice and apply after the app has been rebooted
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist