fix: Enable enforcement that far must be declared

packages/marshal/src/helpers/environment-options.js

@@ -0,0 +1,131 @@
+// @ts-check
+/* global globalThis */
+
+// Note: Might need to be imported quite early, so this module should
+// avoid importing other things.
+
+const { details: X, quote: q } = assert;
+
+/**
+ * JavaScript module semantics resists attempts to parameterize a module's
+ * initialization behavior. A module initializes in order according to
+ * the path by which it is first imported, and then the initialized module
+ * is reused by all the other times it is imported. Compartments give us
+ * the opportunity to bind the same import name to different imported
+ * modules, depending on the package/compartment doing the import. Compartments
+ * also address the difficulty of parameterizing a module's initialization
+ * logic, but not in a pleasant manner.
+ *
+ * A pleasant parameterization would be for a static module to be function-like
+ * with explicit parameters, and for the parameterization to be like
+ * calling the static module with parameters in order to derive from it a
+ * module instance. Compartments instead lets us parameterize the meaning
+ * of a module instance derived from a static module according to the
+ * three namespaces provided by the JavaScript semantics, effecting the
+ * meaning of a module instance.
+ *    * The global variable namespaces.
+ *       * The global scope, aliased to properties of the global object.
+ *         This is necessarily compartment-wide, and therefore in our
+ *         recommened usage pattern, packages-wide.
+ *       * The global lexical scope. The SES-shim compartments supports
+ *         these both compartment-wide as well as per-module. But it is
+ *         not yet clear what we will propose in the Compartment proposal.
+ *    * The import namespace.
+ *    * The host hooks.
+ *
+ * This `environment-options.js` module looks for a setting of of an
+ * `optionName` parameter rooted in the global scope. If follows the Node
+ * precedent for finding Unix environment variable settings, looking for a
+ * global `process` object holding an `env` object,
+ * optionally holding a property named for the `optionName` whose value is the
+ * configuration setting of that option. For example, for the optionName
+ * `ALLOW_IMPLICIT_REMOTABLES` it would look in
+ * `globalThis.process.env.ALLOW_IMPLICIT_REMOTABLES`.
+ *
+ * If setting is either absent or `undefined`, that indicates that
+ * this configuration option should have its default behavior, whatever that is.
+ * Otherwise, reflecting Unix environment variables, the setting must be a
+ * string. This also helps ensure that this channel is used only to pass data,
+ * not authority beyond the ability to read this global state.
+ *
+ * The current placement of this `environment-options.js` module in the
+ * `@agoric/marshal` package is a stopgap measure.
+ * TODO the intention is to migrate it into Endo, and to migrate all our
+ * direct uses of `process.env` for configuration parameters to use it
+ * instead.
+ *
+ * Even after that migration, this module will still not be used for
+ * `LOCKDOWN_OPTIONS` itself, since that must happen before `lockdown`,
+ * whereas this module must initialize after `lockdown`.
+ *
+ * @param {string} optionName
+ * @param {string=} defaultSetting
+ */
+export const getEnvironmentOption = (
+  optionName,
+  defaultSetting = undefined,
+) => {
+  assert.typeof(
+    optionName,
+    'string',
+    X`Environment option name ${q(optionName)} must be a string.`,
+  );
+  assert(
+    defaultSetting === undefined || typeof defaultSetting === 'string',
+    X`Environment option default setting ${q(
+      defaultSetting,
+    )}, if present, must be a string.`,
+  );
+
+  let setting = defaultSetting;
+  const globalProcess = globalThis.process;
+  if (typeof process === 'object' && typeof globalProcess.env === 'object') {
+    if (optionName in globalProcess.env) {
+      console.log(
+        `Environment options sniffed and found an apparent ${q(
+          optionName,
+        )} environment variable.'\n`,
+      );
+      setting = globalProcess.env[optionName];
+    }
+  }
+  assert(
+    setting === undefined || typeof setting === 'string',
+    X`Environment option value ${q(setting)}, if present, must be a string.`,
+  );
+  return setting;
+};
+harden(getEnvironmentOption);
+
+/**
+ * Set `globalThis.process.env[optionName]` to `setting`.
+ *
+ * This function takes care of the complexity that `process` may or may not
+ * already exist, `process.env` may or may not already exist, and if both
+ * exist, this function should mutate only this one property setting, minimizing
+ * other damage to a shared `globalThis.process.env`.
+ *
+ * @param {string} optionName
+ * @param {string=} setting
+ */
+export const setEnvironmentOption = (optionName, setting) => {
+  assert.typeof(optionName, 'string');
+  assert(setting === undefined || typeof setting === 'string');
+  if (!('process' in globalThis)) {
+    // @ts-ignore TS assumes this is the Node process object.
+    globalThis.process = {};
+  }
+  const globalProcess = globalThis.process;
+  assert.typeof(globalProcess, 'object');
+  if (!('env' in globalProcess)) {
+    // @ts-ignore TS assumes this is the Node process object.
+    globalProcess.env = {};
+  }
+  const env = globalProcess.env;
+  assert.typeof(env, 'object');
+  if (optionName in env) {
+    console.log(`Overwriting apparent environment variable ${q(optionName)}`);
+  }
+  env[optionName] = setting;
+};
+harden(setEnvironmentOption);

packages/marshal/src/helpers/remotable.js

@@ -19,6 +19,7 @@ import {
   PASS_STYLE,
   checkTagRecord,
 } from './passStyleHelpers.js';
+import { getEnvironmentOption } from './environment-options.js';
 
 const { details: X, quote: q } = assert;
 const { ownKeys } = Reflect;
@@ -39,8 +40,11 @@ const {
 // TODO: once the policy changes to force remotables to be explicit, remove this
 // flag entirely and fix code that uses it (as if it were always `false`).
 //
-// Exported only for testing during the transition
-export const ALLOW_IMPLICIT_REMOTABLES = true;
+// Exported only for testing during the transition. The first step
+// will be to change the default, the second argument to `getEnvironmentOption`
+// below, from `'true'` to `'false'`.
+export const ALLOW_IMPLICIT_REMOTABLES =
+  getEnvironmentOption('ALLOW_IMPLICIT_REMOTABLES', 'true') === 'true';
 
 /**
  * @param {InterfaceSpec} iface
@@ -94,7 +98,7 @@ const checkRemotableProtoOf = (original, check = x => x) => {
         // would always return true.
         // @ts-ignore TypeScript assumes what we're trying to check
         proto !== objectPrototype,
-        X`Remotables must now be explicitly declared: ${q(original)}`,
+        X`Remotables must be explicitly declared: ${q(original)}`,
       ) && checkTagRecord(proto, 'remotable', check)
     )
   ) {

packages/marshal/test/allow_implicit_remotables.js

@@ -0,0 +1,9 @@
+// @ts-check
+
+// This one is designed to be imported this early
+import { setEnvironmentOption } from '../src/helpers/environment-options.js';
+
+// Import this module early, so it initializes before any module whose
+// initialization reads this option.
+
+setEnvironmentOption('ALLOW_IMPLICIT_REMOTABLES', 'true');

packages/marshal/test/default_implicit_remotables.js

@@ -0,0 +1,9 @@
+// @ts-check
+
+// This one is designed to be imported this early
+import { setEnvironmentOption } from '../src/helpers/environment-options.js';
+
+// Import this module early, so it initializes before any module whose
+// initialization reads this option.
+
+setEnvironmentOption('ALLOW_IMPLICIT_REMOTABLES', undefined);

packages/marshal/test/disallow_implicit_remotables.js

@@ -0,0 +1,9 @@
+// @ts-check
+
+// This one is designed to be imported this early
+import { setEnvironmentOption } from '../src/helpers/environment-options.js';
+
+// Import this module early, so it initializes before any module whose
+// initialization reads this option.
+
+setEnvironmentOption('ALLOW_IMPLICIT_REMOTABLES', 'false');

packages/marshal/test/test-allow-implicit-remotables.js

@@ -0,0 +1,11 @@
+// @ts-check
+
+import { test } from './prepare-test-env-ava.js';
+// Import early, before remotable.js might initialize.
+import './allow_implicit_remotables.js';
+
+import { passStyleOf } from '../src/passStyleOf.js';
+
+test('environment options', t => {
+  t.notThrows(() => passStyleOf(harden({ toString: () => 'foo' })));
+});

packages/marshal/test/test-default-implicit-remotables.js

@@ -0,0 +1,20 @@
+// @ts-check
+
+import { test } from './prepare-test-env-ava.js';
+// Import early, before remotable.js might initialize.
+import './default_implicit_remotables.js';
+
+import { passStyleOf } from '../src/passStyleOf.js';
+import { ALLOW_IMPLICIT_REMOTABLES } from '../src/helpers/remotable.js';
+
+// Whatever ALLOW_IMPLICIT_REMOTABLES defaults to, ensure that still works.
+
+test('environment options', t => {
+  if (ALLOW_IMPLICIT_REMOTABLES) {
+    t.notThrows(() => passStyleOf(harden({ toString: () => 'foo' })));
+  } else {
+    t.throws(() => passStyleOf(harden({ toString: () => 'foo' })), {
+      message: /Remotables must be explicitly declared/,
+    });
+  }
+});

packages/marshal/test/test-disallow-implicit-remotables.js

@@ -0,0 +1,13 @@
+// @ts-check
+
+import { test } from './prepare-test-env-ava.js';
+// Import early, before remotable.js might initialize.
+import './disallow_implicit_remotables.js';
+
+import { passStyleOf } from '../src/passStyleOf.js';
+
+test('environment options', t => {
+  t.throws(() => passStyleOf(harden({ toString: () => 'foo' })), {
+    message: /Remotables must be explicitly declared/,
+  });
+});

packages/marshal/test/test-existing-implicit-remotables.js

@@ -0,0 +1,18 @@
+// @ts-check
+
+import { test } from './prepare-test-env-ava.js';
+import { passStyleOf } from '../src/passStyleOf.js';
+import { ALLOW_IMPLICIT_REMOTABLES } from '../src/helpers/remotable.js';
+
+// Whatever ALLOW_IMPLICIT_REMOTABLES is set to in the current test
+// environment, ensure that still works.
+
+test('environment options', t => {
+  if (ALLOW_IMPLICIT_REMOTABLES) {
+    t.notThrows(() => passStyleOf(harden({ toString: () => 'foo' })));
+  } else {
+    t.throws(() => passStyleOf(harden({ toString: () => 'foo' })), {
+      message: /Remotables must be explicitly declared/,
+    });
+  }
+});

packages/marshal/test/test-marshal-far-obj.js

@@ -240,7 +240,7 @@ test('transitional remotables', t => {
   }
   const FAR_NOACC = /cannot serialize Remotables with accessors/;
   const FAR_ONLYMETH = /cannot serialize Remotables with non-methods/;
-  const FAR_EXPLICIT = /Remotables must now be explicitly declared/;
+  const FAR_EXPLICIT = /Remotables must be explicitly declared/;
 
   // Far('iface', {})
   // all cases: pass-by-ref