Skip to content
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(ses): expect more properties to censor #2070

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

erights
Copy link
Contributor

@erights erights commented Feb 14, 2024

closes: #XXXX
refs: Moddable-OpenSource/moddable#523 (comment)

Description

Initializing SES on XS reports additional non-standard properties to remove, beyond those previously mentioned by Moddable-OpenSource/moddable#523 . Initializing SES on QuickJS reveals a different set of additional non-standard properties to remove. Without this PR, all these extra properties are still safely removed, but with a console warning because they were unexpected. This PR simply adds a propertyName: false for each of these to permits.js to record that we now expect them and do not need the warning.

Security Considerations

The purpose of the warning is so that we, the SES developers, are alerted to surprising additions to the platform in case they raise the possibility of a dangerous change in semantics of the parts not removed. From the names of the additional XS properties, we can guess at their purpose and semantics well enough that they don't raise such alarms about the unremoved parts of the platform.

For the QuickJS properties, a few are more mysterious, in particular __getClass and operatorSet. However, we have not yet qualified the base QuickJS engine as being SES safe. These additional properties are not temporally new for us compared to a QuickJS baseline we've already understood without them. They are in the first version of QuickJS we've seriously examined. Nevertheless, they may indicate some novel semantics that we should worry about. This PR, by listed them as expected and suppressing the warning, also risks allowing us to become complacent rather than worrying.

Attn @raphdev @LuqiPan @ivanlei

Scaling Considerations

none

Documentation Considerations

XS or QuickJS users may expect some of these properties. Indeed, once we understand what they do, we might judge them to be safe and eventually enable them in permits.js. But as of this PR we wish to continue removing all of them. Just silently. XS or QuickJS users should be able to find out about this surprise, and an explanation, from some of our documentation.

Testing Considerations

none

Compatibility Considerations

none

Upgrade Considerations

none

  • Includes *BREAKING*: in the commit message with migration instructions for any breaking change.
  • Updates NEWS.md for user-facing changes.

@erights erights self-assigned this Feb 14, 2024
@erights erights marked this pull request as ready for review February 14, 2024 00:58
@erights erights merged commit 4e5a88b into master Feb 14, 2024
14 checks passed
@erights erights deleted the markm-expect-more-props-to-censor branch February 14, 2024 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants