From f908f89186162df83b540f6aeb1f4c665c3a56b4 Mon Sep 17 00:00:00 2001
From: "Mark S. Miller" <erights@users.noreply.github.com>
Date: Mon, 17 Jun 2024 15:22:13 -0700
Subject: [PATCH] fix: endow with original unstructured `assert` (#9514)

closes: #9515
refs: #5672 #8332 #9513  https://github.com/endojs/endo/pull/2323

## Description

To address #5672 , we should change all uses of `assert` to obtain `assert` by importing it from the `@endo/errors` package. However, attempts to do so (#8332 #9513) ran into the symptoms reported at #9515 for the reasons reported there -- accidentally using the imported `assert` as the endowment for the global `assert` of new Compartments.

This PR prepares the ground for these fixes to #5672 by unambiguously endowing with the original unstructured `globalThis.assert`, which will remain the correct endowment even when other uses of `assert` have migrated to the one imported from `@endo/errors`. By itself, this PR, preceding those fixes to #5672 , is not actually fixing a bug in the sense of a behavioral change. Reviewers, let me know if you think this PR should be labeled a "refactor" because of this.

### Security Considerations
none
### Scaling Considerations
none
### Documentation Considerations
anyone gathering endowments for a new compartment should be aware of this issue, and be sure to use `globalThis.assert` as the `assert` endowment. Fortunately, this will only be of concern to advanced developers.
### Testing Considerations
Since this PR doesn't actually cause any behavioral change, it cannot be tested in place. Rather, its test will be whether #9513 still passes CI once rebased on this PR.

***Update***: #9513 is now passing CI well enough to consider this PR (#9514) to be adequately tested.

### Upgrade Considerations
This PR by itself does not have any dependence on @endo/errors existing, and so should be compatible even when linked against fairly ancient versions of endo.
---
 packages/SwingSet/src/controller/controller.js         |  3 ++-
 packages/SwingSet/src/kernel/kernel.js                 |  9 ++++++++-
 .../SwingSet/src/kernel/vat-loader/manager-local.js    |  3 ++-
 .../subprocess-node/supervisor-subprocess-node.js      |  5 +++--
 packages/SwingSet/test/zcf-ish-upgrade/pseudo-zcf.js   | 10 +++++++---
 packages/cosmic-swingset/scripts/clean-core-eval.js    |  5 ++++-
 packages/spawner/src/vat-spawned.js                    |  3 ++-
 .../lib/supervisor-subprocess-xsnap.js                 |  3 ++-
 packages/vats/src/core/chain-behaviors.js              |  3 ++-
 packages/vats/src/core/core-eval-env.d.ts              |  2 ++
 packages/vats/src/repl.js                              |  5 ++++-
 packages/xsnap/src/avaAssertXS.js                      |  2 ++
 packages/xsnap/test/boot-lockdown.test.js              |  6 +++++-
 packages/zoe/src/contractFacet/evalContractCode.js     |  4 ++--
 14 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/packages/SwingSet/src/controller/controller.js b/packages/SwingSet/src/controller/controller.js
index 404619164f2..32eaa4e40b5 100644
--- a/packages/SwingSet/src/controller/controller.js
+++ b/packages/SwingSet/src/controller/controller.js
@@ -211,7 +211,8 @@ export async function makeSwingsetController(
     filePrefix: 'kernel/...',
     endowments: {
       console: makeConsole(`${debugPrefix}SwingSet:kernel`),
-      assert,
+      // See https://github.com/Agoric/agoric-sdk/issues/9515
+      assert: globalThis.assert,
       require: kernelRequire,
       URL: globalThis.Base64, // Unavailable only on XSnap
       Base64: globalThis.Base64, // Available only on XSnap
diff --git a/packages/SwingSet/src/kernel/kernel.js b/packages/SwingSet/src/kernel/kernel.js
index ae24c76fb0b..23f0d9ec637 100644
--- a/packages/SwingSet/src/kernel/kernel.js
+++ b/packages/SwingSet/src/kernel/kernel.js
@@ -1,3 +1,5 @@
+/* global globalThis */
+
 import { assert, Fail } from '@agoric/assert';
 import { isNat } from '@endo/nat';
 import { importBundle } from '@endo/import-bundle';
@@ -1644,7 +1646,12 @@ export default function buildKernel(
       assert(bundle);
       const NS = await importBundle(bundle, {
         filePrefix: `dev-${name}/...`,
-        endowments: harden({ ...vatEndowments, console: devConsole, assert }),
+        endowments: harden({
+          ...vatEndowments,
+          console: devConsole,
+          // See https://github.com/Agoric/agoric-sdk/issues/9515
+          assert: globalThis.assert,
+        }),
       });
 
       if (deviceEndowments[name] || unendowed) {
diff --git a/packages/SwingSet/src/kernel/vat-loader/manager-local.js b/packages/SwingSet/src/kernel/vat-loader/manager-local.js
index ca94d76c7a9..46d61def5fb 100644
--- a/packages/SwingSet/src/kernel/vat-loader/manager-local.js
+++ b/packages/SwingSet/src/kernel/vat-loader/manager-local.js
@@ -74,7 +74,8 @@ export function makeLocalVatManagerFactory({
     const workerEndowments = harden({
       ...vatEndowments,
       console: makeVatConsole(makeLogMaker('vat')),
-      assert,
+      // See https://github.com/Agoric/agoric-sdk/issues/9515
+      assert: globalThis.assert,
       TextEncoder,
       TextDecoder,
       Base64: globalThis.Base64, // Available only on XSnap
diff --git a/packages/SwingSet/src/supervisors/subprocess-node/supervisor-subprocess-node.js b/packages/SwingSet/src/supervisors/subprocess-node/supervisor-subprocess-node.js
index 20e7ff4db23..2ad8c55e485 100644
--- a/packages/SwingSet/src/supervisors/subprocess-node/supervisor-subprocess-node.js
+++ b/packages/SwingSet/src/supervisors/subprocess-node/supervisor-subprocess-node.js
@@ -1,4 +1,4 @@
-/* global WeakRef, FinalizationRegistry */
+/* global globalThis, WeakRef, FinalizationRegistry */
 
 // this file is loaded at the start of a new subprocess
 import '@endo/init';
@@ -139,7 +139,8 @@ function handleSetBundle(margs) {
   // Enable or disable the console accordingly.
   const workerEndowments = {
     console: makeVatConsole(makeLogMaker(`SwingSet:vat:${vatID}`)),
-    assert,
+    // See https://github.com/Agoric/agoric-sdk/issues/9515
+    assert: globalThis.assert,
   };
 
   async function buildVatNamespace(lsEndowments, inescapableGlobalProperties) {
diff --git a/packages/SwingSet/test/zcf-ish-upgrade/pseudo-zcf.js b/packages/SwingSet/test/zcf-ish-upgrade/pseudo-zcf.js
index b2aac733211..62b69fb4123 100644
--- a/packages/SwingSet/test/zcf-ish-upgrade/pseudo-zcf.js
+++ b/packages/SwingSet/test/zcf-ish-upgrade/pseudo-zcf.js
@@ -1,11 +1,10 @@
 // @ts-nocheck
-/* global VatData */
+/* global globalThis, VatData */
 /* eslint-disable no-unused-vars */
 
 import { Far } from '@endo/far';
 import { importBundle } from '@endo/import-bundle';
 import { defineDurableKind } from '@agoric/vat-data';
-import { assert } from '@agoric/assert';
 import {
   provideHandle,
   provideBaggageSubset as provideBaggageSubTree,
@@ -24,7 +23,12 @@ export const buildRootObject = async (vatPowers, vatParameters, baggage) => {
   const { D } = vatPowers;
   const { contractBundleCap } = vatParameters;
   const contractBundle = D(contractBundleCap).getBundle();
-  const endowments = { console, assert, VatData };
+  const endowments = {
+    console,
+    // See https://github.com/Agoric/agoric-sdk/issues/9515
+    assert: globalThis.assert,
+    VatData,
+  };
   const contractNS = await importBundle(contractBundle, { endowments });
   const { setupInstallation, setup: _ } = contractNS;
   if (!setupInstallation) {
diff --git a/packages/cosmic-swingset/scripts/clean-core-eval.js b/packages/cosmic-swingset/scripts/clean-core-eval.js
index 58ad55094e0..b182992b3a3 100755
--- a/packages/cosmic-swingset/scripts/clean-core-eval.js
+++ b/packages/cosmic-swingset/scripts/clean-core-eval.js
@@ -1,4 +1,6 @@
 #! /usr/bin/env node
+/* global globalThis */
+
 import '@endo/init/debug.js';
 import * as farExports from '@endo/far';
 import { isEntrypoint } from '../src/helpers/is-entrypoint.js';
@@ -12,7 +14,8 @@ export const compartmentEvaluate = code => {
   const globals = harden({
     ...modules,
     ...farExports,
-    assert,
+    // See https://github.com/Agoric/agoric-sdk/issues/9515
+    assert: globalThis.assert,
     console: {
       // Ensure we don't pollute stdout.
       debug: console.warn,
diff --git a/packages/spawner/src/vat-spawned.js b/packages/spawner/src/vat-spawned.js
index fa33bfa3e47..8539abe4386 100644
--- a/packages/spawner/src/vat-spawned.js
+++ b/packages/spawner/src/vat-spawned.js
@@ -6,7 +6,8 @@ import { Far } from '@endo/marshal';
 const endowments = {
   VatData,
   console,
-  assert,
+  // See https://github.com/Agoric/agoric-sdk/issues/9515
+  assert: globalThis.assert,
   Base64: globalThis.Base64, // Present only on XSnap
   URL: globalThis.URL, // Absent only on XSnap
 };
diff --git a/packages/swingset-xsnap-supervisor/lib/supervisor-subprocess-xsnap.js b/packages/swingset-xsnap-supervisor/lib/supervisor-subprocess-xsnap.js
index 4f279879722..ca81d767176 100644
--- a/packages/swingset-xsnap-supervisor/lib/supervisor-subprocess-xsnap.js
+++ b/packages/swingset-xsnap-supervisor/lib/supervisor-subprocess-xsnap.js
@@ -255,7 +255,8 @@ function makeWorker(port) {
 
     const workerEndowments = {
       console: makeVatConsole(makeLogMaker('vat')),
-      assert,
+      // See https://github.com/Agoric/agoric-sdk/issues/9515
+      assert: globalThis.assert,
       // bootstrap provides HandledPromise
       HandledPromise: globalThis.HandledPromise,
       TextEncoder,
diff --git a/packages/vats/src/core/chain-behaviors.js b/packages/vats/src/core/chain-behaviors.js
index d40e0e48fd5..2cdaafc8489 100644
--- a/packages/vats/src/core/chain-behaviors.js
+++ b/packages/vats/src/core/chain-behaviors.js
@@ -39,7 +39,8 @@ export const bridgeCoreEval = async allPowers => {
   const endowments = {
     VatData: globalThis.VatData,
     console,
-    assert,
+    // See https://github.com/Agoric/agoric-sdk/issues/9515
+    assert: globalThis.assert,
     Base64: globalThis.Base64, // Present only on XSnap
     URL: globalThis.URL, // Absent only on XSnap
   };
diff --git a/packages/vats/src/core/core-eval-env.d.ts b/packages/vats/src/core/core-eval-env.d.ts
index 83b01732d40..a53028d74b5 100644
--- a/packages/vats/src/core/core-eval-env.d.ts
+++ b/packages/vats/src/core/core-eval-env.d.ts
@@ -29,6 +29,8 @@ declare global {
 
   // endowments
   var VatData: VatData;
+  // This is correctly the `Assert` type from `'ses'`, not from @endo/errors
+  // See https://github.com/Agoric/agoric-sdk/issues/9515
   var assert: Assert;
 
   // console is a VirtualConsole but this directive fails to override the extant global `console`
diff --git a/packages/vats/src/repl.js b/packages/vats/src/repl.js
index 1d00a23ad0d..e0d6fa27673 100644
--- a/packages/vats/src/repl.js
+++ b/packages/vats/src/repl.js
@@ -1,3 +1,5 @@
+/* global globalThis */
+
 import { isPromise } from '@endo/promise-kit';
 import { Far } from '@endo/far';
 import { V as E } from '@agoric/vow/vat.js';
@@ -194,7 +196,8 @@ export function getReplHandler(replObjects, send) {
     ...farExports,
     ...vowExports,
     E,
-    assert,
+    // See https://github.com/Agoric/agoric-sdk/issues/9515
+    assert: globalThis.assert,
     console: replConsole,
     commands,
     history,
diff --git a/packages/xsnap/src/avaAssertXS.js b/packages/xsnap/src/avaAssertXS.js
index 7522d1108ae..cddda9bda2f 100644
--- a/packages/xsnap/src/avaAssertXS.js
+++ b/packages/xsnap/src/avaAssertXS.js
@@ -228,6 +228,8 @@ function makeTester(htest, out) {
     fail(message) {
       assert(false, message);
     },
+    // Not the SES or @endo/errors `assert`
+    // See https://github.com/Agoric/agoric-sdk/issues/9515
     assert,
     truthy,
     /**
diff --git a/packages/xsnap/test/boot-lockdown.test.js b/packages/xsnap/test/boot-lockdown.test.js
index 189afd7458c..c5ebfc49124 100644
--- a/packages/xsnap/test/boot-lockdown.test.js
+++ b/packages/xsnap/test/boot-lockdown.test.js
@@ -1,3 +1,5 @@
+/* global globalThis */
+
 import test from 'ava';
 
 import * as proc from 'child_process';
@@ -173,7 +175,9 @@ test('console - objects should include detail', async t => {
       Error('oops!'),
     ];
 
-    const { Fail } = assert;
+    // Using `globalThis.assert` because of delayed SES lockdown
+    // See https://github.com/Agoric/agoric-sdk/issues/9515
+    const { Fail } = globalThis.assert;
 
     try {
       Fail`assertion text ${richStructure}`;
diff --git a/packages/zoe/src/contractFacet/evalContractCode.js b/packages/zoe/src/contractFacet/evalContractCode.js
index b357f3d2d00..b95a901056d 100644
--- a/packages/zoe/src/contractFacet/evalContractCode.js
+++ b/packages/zoe/src/contractFacet/evalContractCode.js
@@ -4,7 +4,6 @@
 /* global globalThis */
 
 import { importBundle } from '@endo/import-bundle';
-import { assert } from '@agoric/assert';
 import { handlePWarning } from '../handleWarning.js';
 
 const evalContractBundle = (bundle, additionalEndowments = {}) => {
@@ -16,7 +15,8 @@ const evalContractBundle = (bundle, additionalEndowments = {}) => {
 
   const defaultEndowments = {
     console: louderConsole,
-    assert,
+    // See https://github.com/Agoric/agoric-sdk/issues/9515
+    assert: globalThis.assert,
     VatData: globalThis.VatData,
   };