From 7799061d2809fe8bf655bda65381d8289fa08468 Mon Sep 17 00:00:00 2001 From: "Mark S. Miller" Date: Tue, 1 Aug 2023 18:07:50 -0700 Subject: [PATCH] fix(env-options)!: env-options harmony --- packages/env-options/README.md | 16 ++++--- packages/env-options/src/env-options.js | 52 ++++++++++++++++++++--- packages/eventual-send/src/track-turns.js | 18 +++----- packages/exo/src/exo-makers.js | 8 +--- packages/ses/src/lockdown.js | 4 +- 5 files changed, 67 insertions(+), 31 deletions(-) diff --git a/packages/env-options/README.md b/packages/env-options/README.md index 1dc361eb95..1f0141567c 100644 --- a/packages/env-options/README.md +++ b/packages/env-options/README.md @@ -34,24 +34,28 @@ optionally holding a property with the same name as the option, whose value is the configuration setting of that option. ```js -import { makeEnvironmentCaptor } from '@endo/env-options'; -const { getEnvironmentOption } = makeEnvironmentCaptor(globalThis); +import { getEnvironmentOption } from '@endo/env-options'; const FooBarOption = getEnvironmentOption('FOO_BAR', 'absent'); ``` The first argument to `getEnvironmentOption` is the name of the option. The value of `FooBarOption` would then be the value of `globalThis.process.env.FOO_BAR`, if present. -If setting is either absent or `undefined`, the default `'absent'` -would be used instead. +If value is either absent or `undefined`, the second argument, +such as `'absent'`, would be used instead. In either case, reflecting Unix environment variable expectations, the resulting setting must be a string. This restriction also helps ensure that this channel is used only to pass data, not authority beyond the ability to read this global state. -The `makeEnvironmentCaptor` function also returns a -`getCapturedEnvironmentOptionNames` function for use to give feedback about +The `'@endo/env-options'` module also exports a lower-level +`makeEnvironmentCaptor` that you can apply to whatever object you wish to treat +as a global, such as the global of another compartment. It returns an entagled +pair of a `getEnvironmentOption` function as above, and a +`getCapturedEnvironmentOptionNames` function for that returns an array of +the option names used by that `getEnvironmentOption` function. This is +useful to give feedback about which environment variables were actually read, for diagnostic purposes. For example, the ses-shim `lockdown` once contained code such as the following, to explain which diff --git a/packages/env-options/src/env-options.js b/packages/env-options/src/env-options.js index b0047eee15..29b01f46ef 100644 --- a/packages/env-options/src/env-options.js +++ b/packages/env-options/src/env-options.js @@ -1,3 +1,4 @@ +/* global globalThis */ // @ts-check // `@endo/env-options` needs to be imported quite early, and so should @@ -17,6 +18,8 @@ const uncurryThis = (receiver, ...args) => apply(fn, receiver, args); const arrayPush = uncurryThis(Array.prototype.push); +const arrayIncludes = uncurryThis(Array.prototype.includes); +const stringSplit = uncurryThis(String.prototype.split); const q = JSON.stringify; @@ -47,13 +50,16 @@ export const makeEnvironmentCaptor = aGlobal => { * * @param {string} optionName * @param {string} defaultSetting + * @param {string[]} [optOtherNames] * @returns {string} */ - const getEnvironmentOption = (optionName, defaultSetting) => { - // eslint-disable-next-line @endo/no-polymorphic-call + const getEnvironmentOption = ( + optionName, + defaultSetting, + optOtherNames = undefined, + ) => { typeof optionName === 'string' || Fail`Environment option name ${q(optionName)} must be a string.`; - // eslint-disable-next-line @endo/no-polymorphic-call typeof defaultSetting === 'string' || Fail`Environment option default setting ${q( defaultSetting, @@ -68,7 +74,6 @@ export const makeEnvironmentCaptor = aGlobal => { if (optionName in globalEnv) { arrayPush(capturedEnvironmentOptionNames, optionName); const optionValue = globalEnv[optionName]; - // eslint-disable-next-line @endo/no-polymorphic-call typeof optionValue === 'string' || Fail`Environment option named ${q( optionName, @@ -79,15 +84,52 @@ export const makeEnvironmentCaptor = aGlobal => { } } } + optOtherNames === undefined || + setting === defaultSetting || + arrayIncludes(optOtherNames, setting) || + Fail`Unrecognized ${q(optionName)} value ${q( + setting, + )}. Expected one of ${q([defaultSetting, ...optOtherNames])}`; return setting; }; freeze(getEnvironmentOption); + /** + * @param {string} optionName + * @returns {string[]} + */ + const getEnvironmentOptionsList = optionName => { + const option = getEnvironmentOption(optionName, ''); + return option === '' ? [] : stringSplit(option, ','); + }; + freeze(getEnvironmentOptionsList); + + const environmentOptionsListHas = (optionName, element) => + arrayIncludes(getEnvironmentOptionsList(optionName), element); + const getCapturedEnvironmentOptionNames = () => { return freeze([...capturedEnvironmentOptionNames]); }; freeze(getCapturedEnvironmentOptionNames); - return freeze({ getEnvironmentOption, getCapturedEnvironmentOptionNames }); + return freeze({ + getEnvironmentOption, + getEnvironmentOptionsList, + environmentOptionsListHas, + getCapturedEnvironmentOptionNames, + }); }; freeze(makeEnvironmentCaptor); + +/** + * For the simple case, where the global in question is `globalThis` and no + * reporting of option names is desired. + * + * TODO For this simple case, we should not track the + * `capturedEnvironmentOptionNames` since no one can observe them. + */ +export const { + getEnvironmentOption, + getEnvironmentOptionsList, + environmentOptionsListHas, +} = makeEnvironmentCaptor(globalThis); diff --git a/packages/eventual-send/src/track-turns.js b/packages/eventual-send/src/track-turns.js index 795e7a03bd..cb8ae7ba0a 100644 --- a/packages/eventual-send/src/track-turns.js +++ b/packages/eventual-send/src/track-turns.js @@ -1,7 +1,8 @@ /* global globalThis */ -import { makeEnvironmentCaptor } from '@endo/env-options'; - -const { getEnvironmentOption } = makeEnvironmentCaptor(globalThis); +import { + environmentOptionsListHas, + getEnvironmentOption, +} from '@endo/env-options'; // NOTE: We can't import these because they're not in scope before lockdown. // import { assert, details as X, Fail } from '@agoric/assert'; @@ -17,18 +18,13 @@ let hiddenPriorError; let hiddenCurrentTurn = 0; let hiddenCurrentEvent = 0; -const DEBUG = getEnvironmentOption('DEBUG', ''); - // Turn on if you seem to be losing error logging at the top of the event loop -const VERBOSE = DEBUG.split(':').includes('track-turns'); +const VERBOSE = environmentOptionsListHas('DEBUG', 'track-turns'); // Track-turns is disabled by default and can be enabled by an environment // option. -const TRACK_TURNS = getEnvironmentOption('TRACK_TURNS', 'disabled'); -if (TRACK_TURNS !== 'enabled' && TRACK_TURNS !== 'disabled') { - throw TypeError(`unrecognized TRACK_TURNS ${JSON.stringify(TRACK_TURNS)}`); -} -const ENABLED = (TRACK_TURNS || 'disabled') === 'enabled'; +const ENABLED = + getEnvironmentOption('TRACK_TURNS', 'disabled', ['enabled']) === 'enabled'; // We hoist the following functions out of trackTurns() to discourage the // closures from holding onto 'args' or 'func' longer than necessary, diff --git a/packages/exo/src/exo-makers.js b/packages/exo/src/exo-makers.js index 5d2ca8a8c9..7f3cb63946 100644 --- a/packages/exo/src/exo-makers.js +++ b/packages/exo/src/exo-makers.js @@ -1,17 +1,13 @@ -/* global globalThis */ /// -import { makeEnvironmentCaptor } from '@endo/env-options'; +import { environmentOptionsListHas } from '@endo/env-options'; import { objectMap } from '@endo/patterns'; import { defendPrototype, defendPrototypeKit } from './exo-tools.js'; const { create, seal, freeze, defineProperty, values } = Object; -const { getEnvironmentOption } = makeEnvironmentCaptor(globalThis); -const DEBUG = getEnvironmentOption('DEBUG', ''); - // Turn on to give each exo instance its own toStringTag value. -const LABEL_INSTANCES = DEBUG.split(',').includes('label-instances'); +const LABEL_INSTANCES = environmentOptionsListHas('DEBUG', 'label-instances'); const makeSelf = (proto, instanceCount) => { const self = create(proto); diff --git a/packages/ses/src/lockdown.js b/packages/ses/src/lockdown.js index f0c39109a2..b0484f5dfc 100644 --- a/packages/ses/src/lockdown.js +++ b/packages/ses/src/lockdown.js @@ -14,7 +14,7 @@ // @ts-check -import { makeEnvironmentCaptor } from '@endo/env-options'; +import { getEnvironmentOption as getenv } from '@endo/env-options'; import { FERAL_FUNCTION, FERAL_EVAL, @@ -154,8 +154,6 @@ export const repairIntrinsics = (options = {}) => { // [`stackFiltering` options](https://github.com/Agoric/SES-shim/blob/master/packages/ses/docs/lockdown.md#stackfiltering-options) // for an explanation. - const { getEnvironmentOption: getenv } = makeEnvironmentCaptor(globalThis); - const { errorTaming = getenv('LOCKDOWN_ERROR_TAMING', 'safe'), errorTrapping = getenv('LOCKDOWN_ERROR_TRAPPING', 'platform'),