chore(ses): Revert ModuleSource shared intrinsic #2468
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 0d210a3. This change caused a failure for Lockdown on XS in Agoric SDK, indicating that it is a breaking change. Reverting this change is a temporary mitigation. We will first address the gap in XS CI coverage and then persue a fix.
gibson042
approved these changes
Sep 24, 2024
kriskowal
added a commit
that referenced
this pull request
Sep 26, 2024
Refs: #2463, #2252 ## Description In #2463 we introduced ModuleSource to the SES permits, but this interacted catastrophically with the native XS ModuleSource. We reverted this change #2468 to unbreak Agoric SDK integration. This change reintroduces the ModuleSource permits, such that they are compatible with both XS and the `@endo/module-source/shim.js`, which anticipates the introduction of an AbstractModuleSource base class. Because SES can more gracefully tolerate the absence of an permitted [[Proto]] than the presence of a non-permitted [[Proto]], this adjusts the shim to ensure that the AbstractModuleSource shape exists as a side-effect of repairs/taming, before permits are applied. ### Security Considerations Increase in memory safety exposure in native code implementation of ModuleSource where applicable. ### Scaling Considerations None. ### Documentation Considerations This change reintroduces a note in NEWS.md for the next release. ### Testing Considerations The prior regression went unnoticed because we did not yet have CI for XS #2465. With this change, `yarn test:xs` in SES validates the shim on XS. We also test `@endo/module-source/shim.js` in `ses/test/module-source.test.js` on Node.js. ### Compatibility Considerations This change is designed to tolerate either path forward for the language, whether or not it accepts an AbstractModuleSource base class. ### Upgrade Considerations None.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit 0d210a3.
Description
This change caused a failure for Lockdown on XS in Agoric SDK, indicating that it is a breaking change.
Testing Considerations
Reverting this change is a temporary mitigation.
We will first address the gap in XS CI coverage and then persue a fix.
Compatibility Considerations
We will pursue a follow-up change that addresses XS coverage in CI #2463 and then consider repairing the native
ModuleSourceintrinsic underrepairIntrinsicsin SES.