From e7b7b6eb8d2565886f9bbf7021223bcf96dc9173 Mon Sep 17 00:00:00 2001 From: Kris Kowal Date: Mon, 13 Jul 2020 11:43:57 -0700 Subject: [PATCH] fix(ses)!: Remove evaluate endowments option (#368) --- .../ses-integration-test/test/sanity.test.js | 8 +-- packages/ses/NEWS.md | 5 ++ packages/ses/src/compartment-shim.js | 25 +------- packages/ses/test/compartment.test.js | 14 ++--- packages/ses/test/leak-endowments.test.js | 62 ------------------- packages/ses/test/ses.test.js | 14 ++--- 6 files changed, 20 insertions(+), 108 deletions(-) delete mode 100644 packages/ses/test/leak-endowments.test.js diff --git a/packages/ses-integration-test/test/sanity.test.js b/packages/ses-integration-test/test/sanity.test.js index 21723504ad..25380aa39e 100644 --- a/packages/ses-integration-test/test/sanity.test.js +++ b/packages/ses-integration-test/test/sanity.test.js @@ -5,12 +5,8 @@ import "ses"; test("sanity", t => { t.equal(lockdown(), true, "lockdown runs successfully"); - const c = new Compartment(); + const c = new Compartment({ abc: 456 }); t.equal(c.evaluate("123"), 123, "simple evaluate succeeds"); - t.equal( - c.evaluate("abc", { endowments: { abc: 456 } }), - 456, - "endowment succeeds" - ); + t.equal(c.evaluate("abc"), 456, "endowment succeeds"); t.end(); }); diff --git a/packages/ses/NEWS.md b/packages/ses/NEWS.md index 3badcff20a..9bc768383e 100644 --- a/packages/ses/NEWS.md +++ b/packages/ses/NEWS.md @@ -2,6 +2,11 @@ User-visible changes in SES: ## Next release +* BREAKING CHANGE: The compartment `evaluate` method no longer accepts an + `endowments` option. + Use `compartment.globalThis`, `endowments`, or `globalLexicals`. + If per-evaluation `globalLexicals` or `endowments` are necessary, + each evaluation will need a fresh `Compartment`. * The Compartment constructor now accepts a `globalLexicals` option. The own enumerable properties of the global lexicals are captured and presented as constants in the scope of all calls to `evaluate` and all diff --git a/packages/ses/src/compartment-shim.js b/packages/ses/src/compartment-shim.js index 3ca0e0c3ce..beef5cb409 100644 --- a/packages/ses/src/compartment-shim.js +++ b/packages/ses/src/compartment-shim.js @@ -13,15 +13,7 @@ import * as babel from '@agoric/babel-standalone'; // Both produce: // Error: 'default' is not exported by .../@agoric/babel-standalone/babel.js import { makeModuleAnalyzer } from '@agoric/transform-module'; -import { - assign, - create, - defineProperties, - entries, - getOwnPropertyNames, - getOwnPropertyDescriptors, - freeze, -} from './commons.js'; +import { assign, entries, getOwnPropertyNames, freeze } from './commons.js'; import { createGlobalObject } from './global-object.js'; import { performEval } from './evaluate.js'; import { getCurrentRealmRec } from './realm-rec.js'; @@ -185,7 +177,6 @@ export class Compartment { /** * @param {string} source is a JavaScript program grammar construction. * @param {{ - * endowments: Object, * transforms: Array, * sloppyGlobalsMode: bool, * }} options. @@ -197,11 +188,7 @@ export class Compartment { } // Extract options, and shallow-clone transforms. - const { - endowments = {}, - transforms = [], - sloppyGlobalsMode = false, - } = options; + const { transforms = [], sloppyGlobalsMode = false } = options; const localTransforms = [...transforms]; const { @@ -211,13 +198,7 @@ export class Compartment { } = privateFields.get(this); const realmRec = getCurrentRealmRec(); - // TODO just pass globalLexicals as localObject - // https://github.com/Agoric/SES-shim/issues/365 - const localObject = create(null); - defineProperties(localObject, getOwnPropertyDescriptors(globalLexicals)); - defineProperties(localObject, getOwnPropertyDescriptors(endowments)); - - return performEval(realmRec, source, globalObject, localObject, { + return performEval(realmRec, source, globalObject, globalLexicals, { globalTransforms, localTransforms, sloppyGlobalsMode, diff --git a/packages/ses/test/compartment.test.js b/packages/ses/test/compartment.test.js index 68a29ca9c8..439dfbf529 100644 --- a/packages/ses/test/compartment.test.js +++ b/packages/ses/test/compartment.test.js @@ -48,8 +48,8 @@ test('SES compartment also has compartments', t => { // }); test('SES compartment has harden', t => { - const c = new Compartment(); - const obj = c.evaluate(`harden({a})`, { endowments: { a: 123 } }); + const c = new Compartment({ a: 123 }); + const obj = c.evaluate(`harden({a})`); t.equal(obj.a, 123, `expected object`); t.throws(() => (obj.a = 'ignored')); t.equal(obj.a, 123, `hardened object retains value`); @@ -88,7 +88,6 @@ test('SES compartment has harden', t => { // }); test('main use case', t => { - const c = new Compartment(); function power(a) { return a + 1; } @@ -98,15 +97,12 @@ test('main use case', t => { } return power(arg); } - const attenuatedPower = c.evaluate(`(${attenuate})`, { - endowments: { power }, - }); + const attenuatedPower = new Compartment({ power }).evaluate(`(${attenuate})`); function use(arg) { return power(arg); } - const user = c.evaluate(`(${use})`, { - endowments: { power: attenuatedPower }, - }); + const c = new Compartment({ power: attenuatedPower }); + const user = c.evaluate(`(${use})`); t.equal(user(1), 2); t.throws(() => user(-1), c.globalThis.TypeError); t.end(); diff --git a/packages/ses/test/leak-endowments.test.js b/packages/ses/test/leak-endowments.test.js deleted file mode 100644 index c21d58aad0..0000000000 --- a/packages/ses/test/leak-endowments.test.js +++ /dev/null @@ -1,62 +0,0 @@ -import tap from 'tap'; -import sinon from 'sinon'; -import { Compartment } from '../src/compartment-shim.js'; -import stubFunctionConstructors from './stub-function-constructors.js'; - -const { test } = tap; - -test('endowments are not shared between calls to c.evaluate', t => { - t.plan(3); - - // Mimic repairFunctions. - stubFunctionConstructors(sinon); - - globalThis.bar = {}; - - const c = new Compartment(); - t.equal(c.evaluate(`4`, { endow1: 1 }), 4); - t.throws(() => c.evaluate(`endow1`), ReferenceError); - t.throws(() => c.evaluate(`endow2`), ReferenceError); - - delete globalThis.bar; - sinon.restore(); -}); - -test('endowments are mutable but not shared between calls to c.evaluate', t => { - t.plan(9); - - // Mimic repairFunctions. - stubFunctionConstructors(sinon); - - globalThis.bar = {}; - - const c = new Compartment(); - - // assignment to endowments works - t.equal(c.evaluate(`endow1 = 4; endow1`, { endowments: { endow1: 1 } }), 4); - t.equal(c.evaluate(`endow1 += 4; endow1`, { endowments: { endow1: 1 } }), 5); - - // the global is not modified when an endowment shadows it - t.throws(() => c.evaluate(`endow1`), ReferenceError); - t.equal(c.globalThis.endow1, undefined); - - // assignment to global works even when an endowment shadows it - t.equal( - c.evaluate(`this.endow1 = 4; this.endow1`, { endowments: { endow1: 1 } }), - 4, - ); - t.equal( - c.evaluate(`this.endow1 = 4; endow1`, { endowments: { endow1: 1 } }), - 1, - ); - - // the modified global is now visible when there is no endowment to shadow it - t.equal(c.globalThis.endow1, 4); - t.equal(c.evaluate(`endow1`), 4); - - // endowments shadow globals - t.equal(c.evaluate(`endow1`, { endowments: { endow1: 44 } }), 44); - - delete globalThis.bar; - sinon.restore(); -}); diff --git a/packages/ses/test/ses.test.js b/packages/ses/test/ses.test.js index 29af6a62a5..59a0961388 100644 --- a/packages/ses/test/ses.test.js +++ b/packages/ses/test/ses.test.js @@ -120,8 +120,8 @@ test('SES compartment also has compartments', t => { // }); test('SES compartment has harden', t => { - const c = new Compartment(); - const obj = c.evaluate(`harden({a})`, { endowments: { a: 123 } }); + const c = new Compartment({ a: 123 }); + const obj = c.evaluate(`harden({a})`); t.equal(obj.a, 123, `expected object`); t.throws(() => (obj.a = 'ignored')); t.equal(obj.a, 123, `hardened object retains value`); @@ -160,7 +160,6 @@ test('SES compartment has harden', t => { // }); test('main use case', t => { - const c = new Compartment(); function power(a) { return a + 1; } @@ -170,15 +169,12 @@ test('main use case', t => { } return power(arg); } - const attenuatedPower = c.evaluate(`(${attenuate})`, { - endowments: { power }, - }); + const attenuatedPower = new Compartment({ power }).evaluate(`(${attenuate})`); function use(arg) { return power(arg); } - const user = c.evaluate(`(${use})`, { - endowments: { power: attenuatedPower }, - }); + const c = new Compartment({ power: attenuatedPower }); + const user = c.evaluate(`(${use})`); t.equal(user(1), 2); t.throws(() => user(-1), c.globalThis.TypeError); t.end();