From d704b3c46bbf93ce63b21191494ceead732a1bf8 Mon Sep 17 00:00:00 2001 From: Kris Kowal Date: Sat, 6 Jun 2026 22:47:42 -0700 Subject: [PATCH] test(ses): pin namespace mutation parity with Node.js --- packages/ses/test/_namespace-mutation/a.js | 1 + packages/ses/test/_namespace-mutation/b.js | 29 ++++ packages/ses/test/_namespace-mutation/c.js | 4 + packages/ses/test/_namespace-mutation/main.js | 6 + .../ses/test/_namespace-mutation/package.json | 7 + packages/ses/test/namespace-mutation.test.js | 130 ++++++++++++++++++ 6 files changed, 177 insertions(+) create mode 100644 packages/ses/test/_namespace-mutation/a.js create mode 100644 packages/ses/test/_namespace-mutation/b.js create mode 100644 packages/ses/test/_namespace-mutation/c.js create mode 100644 packages/ses/test/_namespace-mutation/main.js create mode 100644 packages/ses/test/_namespace-mutation/package.json create mode 100644 packages/ses/test/namespace-mutation.test.js diff --git a/packages/ses/test/_namespace-mutation/a.js b/packages/ses/test/_namespace-mutation/a.js new file mode 100644 index 0000000000..d228111ab3 --- /dev/null +++ b/packages/ses/test/_namespace-mutation/a.js @@ -0,0 +1 @@ +export const x = 'foo'; diff --git a/packages/ses/test/_namespace-mutation/b.js b/packages/ses/test/_namespace-mutation/b.js new file mode 100644 index 0000000000..b131720251 --- /dev/null +++ b/packages/ses/test/_namespace-mutation/b.js @@ -0,0 +1,29 @@ +// The whole point of this fixture is to observe the runtime's response to +// writing to an `import * as` namespace, so the no-import-assign lint must +// not interfere. +/* eslint-disable no-import-assign */ + +import * as foo from './a.js'; + +const result = { + before: foo.x, + isFrozen: Object.isFrozen(foo), + isExtensible: Object.isExtensible(foo), + descriptor: Object.getOwnPropertyDescriptor(foo, 'x'), +}; + +try { + // @ts-expect-error TS2540 — namespace properties are read-only; the throw + // (or its absence) is exactly what we are measuring. + foo.x = 'bar'; + result.assignThrew = false; + result.afterAssign = foo.x; +} catch (e) { + result.assignThrew = true; + result.assignErrorName = /** @type {Error} */ (e).name; +} + +result.reflectSetReturn = Reflect.set(foo, 'x', 'bar'); +result.afterReflectSet = foo.x; + +export { result }; diff --git a/packages/ses/test/_namespace-mutation/c.js b/packages/ses/test/_namespace-mutation/c.js new file mode 100644 index 0000000000..d31b18a58d --- /dev/null +++ b/packages/ses/test/_namespace-mutation/c.js @@ -0,0 +1,4 @@ +import { x as namedX } from './a.js'; +import * as foo from './a.js'; + +export const seenByLaterImport = { namedX, starX: foo.x }; diff --git a/packages/ses/test/_namespace-mutation/main.js b/packages/ses/test/_namespace-mutation/main.js new file mode 100644 index 0000000000..29cd42b5e0 --- /dev/null +++ b/packages/ses/test/_namespace-mutation/main.js @@ -0,0 +1,6 @@ +import process from 'node:process'; + +import { result } from './b.js'; +import { seenByLaterImport } from './c.js'; + +process.stdout.write(JSON.stringify({ result, seenByLaterImport })); diff --git a/packages/ses/test/_namespace-mutation/package.json b/packages/ses/test/_namespace-mutation/package.json new file mode 100644 index 0000000000..25b4509ae5 --- /dev/null +++ b/packages/ses/test/_namespace-mutation/package.json @@ -0,0 +1,7 @@ +{ + "type": "module", + "private": true, + "scripts": { + "preinstall": "echo DO NOT INSTALL TEST FIXTURES; exit -1" + } +} diff --git a/packages/ses/test/namespace-mutation.test.js b/packages/ses/test/namespace-mutation.test.js new file mode 100644 index 0000000000..c37824badb --- /dev/null +++ b/packages/ses/test/namespace-mutation.test.js @@ -0,0 +1,130 @@ +// @ts-nocheck +// Verifies that observable behavior of a module exports namespace object +// differs between Node.js native ESM and a SES Compartment when one module +// attempts `import * as foo from './a'; foo.x = 'bar'` and another module +// later imports `x` from the same module. +// +// The vulnerability hypothesis: the assignment in one module leaks across to +// override `x` as imported by a later module. Both runtimes prevent that +// override, but the *shape* of the protection differs in observable ways +// (descriptor form, frozen state, error name on assignment), and this test +// pins down those differences so future changes to either side are noticed. + +import test from 'ava'; +import { execFile } from 'node:child_process'; +import { readFile } from 'node:fs/promises'; +import process from 'node:process'; +import { fileURLToPath } from 'node:url'; +import { promisify } from 'node:util'; +import { ModuleSource } from '@endo/module-source'; + +import '../index.js'; + +const execFileP = promisify(execFile); + +const fixtureDir = fileURLToPath( + new URL('_namespace-mutation/', import.meta.url), +); + +const runInNode = async () => { + const { stdout } = await execFileP(process.execPath, ['./main.js'], { + cwd: fixtureDir, + }); + return JSON.parse(stdout); +}; + +const runInCompartment = async () => { + // Load the same .js sources Node.js executes (the fixture's package.json + // declares "type": "module"), so the Compartment's linker sees byte- + // identical module bodies for a.js, b.js, and c.js. + const fixtureUrl = new URL('_namespace-mutation/', import.meta.url); + const readSource = name => readFile(new URL(name, fixtureUrl), 'utf8'); + const sources = { + './a.js': await readSource('a.js'), + './b.js': await readSource('b.js'), + './c.js': await readSource('c.js'), + // main.js writes to process.stdout in Node.js; in the Compartment we pull + // values straight off the namespace, so substitute a re-exporting entry. + './main.js': ` + export { result } from './b.js'; + export { seenByLaterImport } from './c.js'; + `, + }; + + const compartment = new Compartment({ + __options__: true, + __noNamespaceBox__: true, + resolveHook: spec => spec, + importHook: async spec => { + const src = sources[spec]; + if (src === undefined) throw Error(`not found: ${spec}`); + return new ModuleSource(src); + }, + }); + + // Round-trip through JSON to drop functions (e.g. accessor descriptors) and + // make a fair structural comparison with the Node.js subprocess output, + // which can only emit JSON. + const ns = await compartment.import('./main.js'); + return JSON.parse( + JSON.stringify({ + result: ns.result, + seenByLaterImport: ns.seenByLaterImport, + }), + ); +}; + +test('cross-module namespace mutation: Node.js vs SES Compartment', async t => { + const [nodeOut, sesOut] = await Promise.all([ + runInNode(), + runInCompartment(), + ]); + + // Both runtimes agree: x is 'foo' before any attempted mutation. + t.is(nodeOut.result.before, 'foo'); + t.is(sesOut.result.before, 'foo'); + + // Both runtimes agree: assignment via `foo.x = 'bar'` throws TypeError + // (module bodies are strict, and the namespace's [[Set]] returns false). + t.true(nodeOut.result.assignThrew); + t.true(sesOut.result.assignThrew); + t.is(nodeOut.result.assignErrorName, 'TypeError'); + t.is(sesOut.result.assignErrorName, 'TypeError'); + + // Both runtimes agree: Reflect.set returns false (no throw, but no-op). + t.false(nodeOut.result.reflectSetReturn); + t.false(sesOut.result.reflectSetReturn); + t.is(nodeOut.result.afterReflectSet, 'foo'); + t.is(sesOut.result.afterReflectSet, 'foo'); + + // Both runtimes agree: the later importer still sees the original value. + // The override claim ("foo.x = 'bar' overrides x as imported later") is + // false in both Node.js and SES. + t.deepEqual(nodeOut.seenByLaterImport, { namedX: 'foo', starX: 'foo' }); + t.deepEqual(sesOut.seenByLaterImport, { namedX: 'foo', starX: 'foo' }); + + // Differences in how the protection is implemented: + + // 1. Property descriptor shape. + // Node.js exposes a data descriptor (writable: true, with a value). + // SES exposes an accessor descriptor (get/set, no value). + t.deepEqual(nodeOut.result.descriptor, { + value: 'foo', + writable: true, + enumerable: true, + configurable: false, + }); + t.deepEqual(sesOut.result.descriptor, { + enumerable: true, + configurable: false, + }); + + // 2. Frozen state. + // Node.js: namespace is non-extensible but NOT frozen + // (writable: true on its data properties is what unfreezes it). + // SES: namespace is fully frozen (accessor properties + frozen target). + t.false(nodeOut.result.isFrozen); + t.false(nodeOut.result.isExtensible); + t.true(sesOut.result.isFrozen); + t.false(sesOut.result.isExtensible); +});