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

fix(ses): Limit scope proxy exposure to discernably owned properties … #2743

Merged
merged 1 commit into from
Mar 19, 2025

Conversation

kriskowal
Copy link
Member

…of host globalThis

Closes #1305

Description

This change circles back to an old observation about the scope proxy, where we deliberately allow guest code to sense which properties of the host globalThis exist in order to provide a higher fidelity behavior for typeof style feature detection. This change reflects a shift in our preference to avoid this information leak at the expense of ecosystem compatibility. We have come to believe closing the leak is worth the expense, especially given that the work-around of checking globalThis properties explicitly has become a norm.

Security Considerations

This change increases confinement in exchange for a minor loss of ecosystem compatibility.

Scaling Considerations

None.

Documentation Considerations

Forthcoming.

Testing Considerations

This change will be validated with an integration PR with Agoric SDK to expose any extant compatibility risks.

Compatibility Considerations

This change may uncover existing code that relied on typeof for feature detection, which is not strictly compatible. We are treating these considerations as bug fixes.

Upgrade Considerations

These changes will become observable to vats upon restart after this software arrives in an Agoric chain software update.

@kriskowal
Copy link
Member Author

Integration with Agoric SDK Agoric/agoric-sdk#11133

Copy link
Contributor

@erights erights left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@mhofman mhofman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM once the 2 nits are addressed.

@kriskowal kriskowal enabled auto-merge March 19, 2025 23:13
@kriskowal kriskowal merged commit 82dc112 into master Mar 19, 2025
16 checks passed
@kriskowal kriskowal deleted the kriskowal-1305 branch March 19, 2025 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revisit behavior of has proxy trap in safe evaluator’s terminal scope
3 participants