endojs / endo

Endo is a distributed secure JavaScript sandbox, based on SES
Apache License 2.0
829 stars 72 forks source link

feat(compartment-mapper): add policy-related types #1839

Closed boneskull closed 10 months ago

boneskull commented 1 year ago

Description

This adds/narrows types for policies and exports them from the compartment-mapper entry point. It also updates some usages of existing and new types (e.g., removing "@typedef references", squashing some null-reference bugs, etc.).

[!NOTE] This does not change the fact that @endo/compartment-mapper does not export types properly to node16/nodenext-resolution consumers (see #1803).

Reviewers: please scrutinize the types added to src/types.js closely. The generics allow an attenuator implementation to define the types of custom params which it expects. For example:

// policy-converter.js

/**
 * @typedef {'$root'} RootPolicy
 * @typedef {'write'} WritePolicy
 */

/**
 * Extends Endo's `PolicyItem` with the special {@link RootPolicy} and {@link WritePolicy}
 * @typedef {RootPolicy | WritePolicy} LavaMoatGlobalPolicyItem
 */

/**
 * Extends Endo's `PolicyItem` with the special {@link RootPolicy}
 * @typedef {RootPolicy} LavaMoatPackagePolicyItem
 */

/**
 * An Endo policy tailored to LavaMoat's default attenuator
 * @typedef {import('@endo/compartment-mapper').Policy<LavaMoatPackagePolicyItem, LavaMoatGlobalPolicyItem>} LavaMoatEndoPolicy
 */

/**
 * Package policy based on {@link LavaMoatPackagePolicyItem} and {@link LavaMoatGlobalPolicyItem}.
 *
 * Member of {@link LavaMoatEndoPolicy}
 * @typedef {import('@endo/compartment-mapper').PackagePolicy<LavaMoatPackagePolicyItem, LavaMoatGlobalPolicyItem>} LavaMoatPackagePolicy
 */

And in the attenuator:

// my-attenuator.js

export default {
  /**
   * @type {import('@endo/compartment-mapper').GlobalAttenuatorFn<[import('./policy-converter.js').LavaMoatPackagePolicy['globals']]>}
   */
  attenuateGlobals(params, originalGlobalThis, packageCompartmentGlobalThis) {
    // ...
  },
  /**
   * @type {import('@endo/compartment-mapper').ModuleAttenuatorFn<[import('./policy-converter.js').LavaMoatPackagePolicy['packages']]>}
   */
  attenuateModule(params, originalObject) {
    // ...
  }
}

Documentation Considerations

Does Endo consider type changes to be breaking? These types will likely change in the future, and am curious about whether that should be a major bump.

Testing Considerations

I could use tsd to test some of these to avoid regressions, if desired.

Upgrade Considerations

none

boneskull commented 1 year ago

cc @naugtur

naugtur commented 11 months ago

I wanted to rebase this and add cleanup to one of mine functions but noticed this is from a fork. @boneskull want me to fold this into a branch on the endo repo?

patch I wish to apply after rebase on master:

diff --git a/packages/compartment-mapper/src/policy.js b/packages/compartment-mapper/src/policy.js
index 8604bcb2a..c646c37c3 100644
--- a/packages/compartment-mapper/src/policy.js
+++ b/packages/compartment-mapper/src/policy.js
@@ -339,26 +339,28 @@ const diagnoseModulePolicy = errorHint => {
  *
  * @param {string} specifier
  * @param {import('./types.js').CompartmentDescriptor} compartmentDescriptor
- * @param {object} [info]
+ * @param {object} [options]
+ * @param {boolean} [options.exit] - Whether it is an exit module
+ * @param {string} [options.errorHint] - Error hint message
  */
 export const enforceModulePolicy = (
   specifier,
   compartmentDescriptor,
-  info = {},
+  { exit, errorHint } = {},
 ) => {
   const { policy, modules, label } = compartmentDescriptor;
   if (!policy) {
     return;
   }

-  if (!info.exit) {
+  if (!exit) {
     if (!modules[specifier]) {
       throw Error(
         `Importing ${q(specifier)} in ${q(
           label,
         )} was not allowed by packages policy ${q(
           policy.packages,
-        )}${diagnoseModulePolicy(info.errorHint)}`,
+        )}${diagnoseModulePolicy(errorHint)}`,
       );
     }
     return;
@@ -368,7 +370,7 @@ export const enforceModulePolicy = (
     throw Error(
       `Importing ${q(specifier)} was not allowed by policy builtins:${q(
         policy.builtins,
-      )}${diagnoseModulePolicy(info.errorHint)}`,
+      )}${diagnoseModulePolicy(errorHint)}`,
     );
   }
 };
boneskull commented 11 months ago

@naugtur Resolved

boneskull commented 11 months ago

@naugtur OK, so, it's slightly different than the diff you gave me, since the diff you gave me did not match the snapshots; I retained the use of the label prop from the compartment descriptor

Please take another look at the changes I made based on your comments.