From 2b799f8b1f0a1beb6f6841652ff2fa92e8458d23 Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Wed, 21 Aug 2024 18:03:18 -0700 Subject: [PATCH] fix(compartment-mapper): fix #2407 include symbol-named properties (#2408) Closes: #2407 Refs: https://github.com/Agoric/agoric-sdk/pull/9874 https://github.com/Agoric/agoric-sdk/pull/9431 https://github.com/Agoric/agoric-sdk/pull/9781 #2377 ## Description While working on https://github.com/Agoric/agoric-sdk/pull/9874 , I found that it still failed to endow `passStyleOf` via a symbol-named property as recently agree. Turns out there was still a lurking `Object.keys` on the compartment endowment pathway that needs to be changed to enumerate all enumerable own properties, whether string-named or symbol-named. ### Security Considerations None ### Scaling Considerations For this PR, none. But it enables liveslots to endow its `passStyleOf` to the compartments it makes, which has the scaling benefits explained at https://github.com/Agoric/agoric-sdk/issues/9781 ### Documentation Considerations For the normal developer, none. Once endowed, `passStyleOf` will have performance that is less surprising. ### Testing Considerations https://github.com/Agoric/agoric-sdk/pull/9874 both patches in the equivalent of this PR, uses it to endow `passStyleOf`, and test that it has done so, across both node and XS. However, after this fix, https://github.com/Agoric/agoric-sdk/pull/9874 works on the Node host. But it still fails on the XS host for undiagnosed reasons, so there may be another bug like #2407 still lurking somewhere. Until diagnosed, it isn't clear if that remaining problem is an endo problem or an agoric-sdk problem. ### Compatibility and Upgrade Considerations Except for code meant only to test this, `passStyleOf` should not have any observable difference. We have not attempted to make an other use of symbol-named global properties, so there should be no compat or upgrade issues. --- packages/compartment-mapper/src/policy.js | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/compartment-mapper/src/policy.js b/packages/compartment-mapper/src/policy.js index ee2a8fb52d..7306ae7496 100644 --- a/packages/compartment-mapper/src/policy.js +++ b/packages/compartment-mapper/src/policy.js @@ -10,7 +10,9 @@ import { policyLookupHelper, } from './policy-format.js'; -const { create, entries, values, assign, keys, freeze } = Object; +const { create, entries, values, assign, freeze, getOwnPropertyDescriptors } = + Object; +const { ownKeys } = Reflect; const q = JSON.stringify; /** @@ -19,7 +21,9 @@ const q = JSON.stringify; export const ATTENUATORS_COMPARTMENT = ''; /** - * Copies properties (optionally limited to a specific list) from one object to another. + * Copies properties (optionally limited to a specific list) from one object + * to another. By default, copies all enumerable, own, string- and + * symbol-named properties. * * @param {object} from * @param {object} to @@ -28,7 +32,12 @@ export const ATTENUATORS_COMPARTMENT = ''; */ const selectiveCopy = (from, to, list) => { if (!list) { - list = keys(from); + const descs = getOwnPropertyDescriptors(from); + list = ownKeys(from).filter( + key => + // @ts-expect-error TypeScript still confused about a symbol as index + descs[key].enumerable, + ); } for (let index = 0; index < list.length; index += 1) { const key = list[index];