fix(compartment-mapper): policy, parser type fixes

.changeset/huge-mammals-give.md

@@ -0,0 +1,9 @@
+---
+'@endo/compartment-mapper': patch
+---
+
+Fixes `CompartmentDescriptor` so that it is generic on the `PackagePolicy`; externally-defined `ParseFn`s can now refer to the specific contents of a custom `PackagePolicy` present in a `CompartmentDescriptor`.
+
+Introduces `ParseSourceMapHook`; differentiated from `@endo/module-source`'s `SourceMapHook`.
+
+Fixes type of `PolicyItem`; eliminates confusion between `void` (no extra union members) and `any` (`SomePackagePolicy`).

packages/compartment-mapper/src/parse-archive-mjs.js

@@ -25,6 +25,7 @@ export const parseArchiveMjs = (
   const record = new ModuleSource(source, {
     sourceMap,
     sourceMapUrl: sourceUrl,
+    // @ts-expect-error: fixed in https://github.com/endojs/endo/pull/3218
     sourceMapHook,
   });
   const pre = textEncoder.encode(JSON.stringify(record));

packages/compartment-mapper/src/parse-mjs.js

@@ -20,6 +20,7 @@ export const parseMjs = (
     sourceUrl: archiveOnly ? undefined : sourceUrl,
     sourceMap,
     sourceMapUrl: sourceUrl,
+    // @ts-expect-error: fixed in https://github.com/endojs/endo/pull/3218
     sourceMapHook,
   });
   return {

packages/compartment-mapper/src/types/compartment-map-schema.ts

@@ -14,7 +14,7 @@ import type {
 } from '../policy-format.js';
 import type { CanonicalName } from './canonical-name.js';
 import type { FileUrlString } from './external.js';
-import type { SomePackagePolicy } from './policy-schema.js';
+import type { PackagePolicy, SomePackagePolicy } from './policy-schema.js';
 import type { PatternDescriptor } from './pattern-replacement.js';
 import type { LiteralUnion } from './typescript.js';
 
@@ -120,23 +120,24 @@ export interface PackageCompartmentDescriptor extends CompartmentDescriptor<Comp
  * one for a given library or application `package.json`.
  */
 export interface CompartmentDescriptor<
-  T extends ModuleConfiguration = ModuleConfiguration,
-  U extends string = string,
+  TModuleConfiguration extends ModuleConfiguration = ModuleConfiguration,
+  TCompartmentName extends string = string,
+  TPackagePolicy extends SomePackagePolicy = SomePackagePolicy,
 > {
-  label: CanonicalName<U>;
+  label: CanonicalName<TCompartmentName>;
   /**
    * the name of the originating package suitable for constructing a sourceURL
    * prefix that will match it to files in a developer workspace.
    */
   name: string;
-  modules: Record<string, T>;
+  modules: Record<string, TModuleConfiguration>;
   scopes?: Record<string, ScopeDescriptor>;
   /** language for extension */
   parsers?: LanguageForExtension;
   /** language for module specifier */
   types?: LanguageForModuleSpecifier;
   /** policy specific to compartment */
-  policy?: SomePackagePolicy;
+  policy?: TPackagePolicy;
 
   location: string;
   /**
@@ -152,9 +153,32 @@ export interface CompartmentDescriptor<
   retained?: true;
 }
 
+/**
+ * Any {@link CompartmentDescriptor}
+ */
+export type SomeCompartmentDescriptor = CompartmentDescriptor<any, any, any>;
+
+/**
+ * Any {@link CompartmentDescriptor} with a non-nullish
+ * {@link CompartmentDescriptor.policy} property
+ */
+export type SomeCompartmentDescriptorWithPolicy =
+  CompartmentDescriptorWithPolicy<any, any, any>;
+
+/**
+ * A {@link CompartmentDescriptor} with a non-nullish
+ * {@link CompartmentDescriptor.policy} property
+ */
 export type CompartmentDescriptorWithPolicy<
-  T extends ModuleConfiguration = ModuleConfiguration,
-> = Omit<CompartmentDescriptor<T>, 'policy'> & { policy: SomePackagePolicy };
+  TModuleConfiguration extends ModuleConfiguration = ModuleConfiguration,
+  TCompartmentName extends string = string,
+  TPackagePolicy extends SomePackagePolicy = SomePackagePolicy,
+> = Omit<
+  CompartmentDescriptor<TModuleConfiguration, TCompartmentName, TPackagePolicy>,
+  'policy'
+> & {
+  policy: TPackagePolicy;
+};
 
 /**
  * A compartment descriptor digested by `digestCompartmentMap()`

packages/compartment-mapper/src/types/external.ts

@@ -18,6 +18,7 @@ import type {
 } from '../policy-format.js';
 import type { CanonicalName } from './canonical-name.js';
 import type {
+  SomeCompartmentDescriptor,
   CompartmentDescriptor,
   CompartmentMapDescriptor,
   DigestedCompartmentMapDescriptor,
@@ -684,6 +685,15 @@ export type SourceMapHookDetails = {
   sha512: string;
 };
 
+/**
+ * Source map hook as received by {@link ParseFn}.
+ *
+ * The import hook wraps the public {@link SourceMapHook} into this shape; it
+ * receives the raw source map object from the code generator, not a JSON
+ * string.
+ */
+export type ParseSourceMapHook = (sourceMapObject: object) => void;
+
 export type ModuleTransforms = Record<string, ModuleTransform>;
 
 export type SyncModuleTransforms = Record<string, SyncModuleTransform>;
@@ -759,29 +769,51 @@ interface BaseParserImplementation {
   heuristicImports: boolean;
 }
 
-export interface ParserImplementation extends BaseParserImplementation {
-  parse: ParseFn;
+export interface ParserImplementation<
+  TCompartmentDescriptor extends SomeCompartmentDescriptor =
+    SomeCompartmentDescriptor,
+> extends BaseParserImplementation {
+  parse: ParseFn<TCompartmentDescriptor>;
   synchronous: true;
 }
 
-export interface AsyncParserImplementation extends BaseParserImplementation {
-  parse: AsyncParseFn;
+export interface AsyncParserImplementation<
+  TCompartmentDescriptor extends SomeCompartmentDescriptor =
+    SomeCompartmentDescriptor,
+> extends BaseParserImplementation {
+  parse: AsyncParseFn<TCompartmentDescriptor>;
   synchronous: false;
 }
 
-type ParseArguments = [
+/**
+ * Options bag for a {@link ParseFn} or {@link AsyncParseFn}.
+ *
+ * @template TCompartmentDescriptor The compartment descriptor to use for the parse
+ */
+export type ParseOptions<
+  TCompartmentDescriptor extends SomeCompartmentDescriptor =
+    SomeCompartmentDescriptor,
+> = Partial<{
+  sourceMap: string | undefined;
+  sourceMapHook: ParseSourceMapHook | undefined;
+  sourceMapUrl: string | undefined;
+  readPowers: ReadFn | ReadPowers | undefined;
+  compartmentDescriptor: TCompartmentDescriptor | undefined;
+}> &
+  ArchiveOnlyOption;
+
+/**
+ * Arguments for a {@link ParseFn} or {@link AsyncParseFn}.
+ */
+export type ParseArguments<
+  TCompartmentDescriptor extends SomeCompartmentDescriptor =
+    SomeCompartmentDescriptor,
+> = [
   bytes: Uint8Array,
   specifier: string,
   moduleLocation: string,
   packageLocation: string,
-  options?: Partial<{
-    sourceMap: string | undefined;
-    sourceMapHook: SourceMapHook | undefined;
-    sourceMapUrl: string | undefined;
-    readPowers: ReadFn | ReadPowers | undefined;
-    compartmentDescriptor: CompartmentDescriptor | undefined;
-  }> &
-    ArchiveOnlyOption,
+  options?: ParseOptions<TCompartmentDescriptor>,
 ];
 
 /**
@@ -804,17 +836,17 @@ export type ParseResult = {
  * Because {@link ParseResult} contains {@link FinalStaticModuleType} from
  * `ses`, those types would want to be moved out of `ses` with it.
  */
-export type ParseFn = { isSyncParser?: true } & ((
-  ...args: ParseArguments
-) => ParseResult);
+export interface ParseFn<
+  TCompartmentDescriptor extends SomeCompartmentDescriptor =
+    SomeCompartmentDescriptor,
+> {
+  isSyncParser?: true;
+  (...args: ParseArguments<TCompartmentDescriptor>): ParseResult;
+}
 
 /**
  * An asynchronous module parsing function.
  */
-export type AsyncParseFn = { isSyncParser?: false } & ((
-  ...args: ParseArguments
-) => Promise<ParseResult>);
-
 /**
  * Mapping of `Language` to synchronous {@link ParserImplementation}s only.
  *
@@ -824,6 +856,13 @@ export type SyncParserForLanguage = Record<
   Language | string,
   ParserImplementation
 >;
+export interface AsyncParseFn<
+  TCompartmentDescriptor extends SomeCompartmentDescriptor =
+    SomeCompartmentDescriptor,
+> {
+  isSyncParser?: false;
+  (...args: ParseArguments<TCompartmentDescriptor>): Promise<ParseResult>;
+}
 
 /**
  * Mapping of `Language` to {@link ParserImplementation

packages/compartment-mapper/src/types/policy-schema.ts

@@ -6,6 +6,7 @@
  */
 
 import type { WILDCARD_POLICY_VALUE } from '../policy-format.js';
+import type { IsAny } from './typescript.js';
 
 /* eslint-disable no-use-before-define */
 
@@ -31,9 +32,16 @@
 export type AttenuationDefinition =
   | FullAttenuationDefinition
   | ImplicitAttenuationDefinition;
+
+/**
+ * Information about the attenuator implementation
+ */
 export type UnifiedAttenuationDefinition = {
+  /** Name of the attenuator (for error messages) */
   displayName: string;
+  /** The module specifier of the implementation */
   specifier: string | null;
+  /** Parameters to pass to the attenuator at invocation */
   params?: any[] | undefined;
 };
 
@@ -52,10 +60,24 @@
  * A type representing a policy item, which can be a {@link WildcardPolicy
  * wildcard policy}, a property policy, `undefined`, or defined by an
  * attenuator
+ *
+ * @remarks
+ * The void-vs-custom `T` branch was originally `[T] extends [void] ? … : …`, but
+ * the type `any` also makes that test succeed, so `PolicyItem<any>` used to
+ * reduce to the same as `void` and
+ * `PackagePolicy<any, any, any, any> = AnyPackagePolicy` was not a supertype of
+ * policies with extra string literals (for example, LavaMoat's {@code "root"} on
+ * package imports). A separate branch for a wide
+ * `any` type parameter yields
+ * `PolicyItem<any> = WildcardPolicy | PropertyPolicy | any` so
+ * `AnyPackagePolicy` correctly accepts all package policy item shapes.
  */
-export type PolicyItem<T = void> = [T] extends [void]
-  ? WildcardPolicy | PropertyPolicy
-  : WildcardPolicy | PropertyPolicy | T;
+export type PolicyItem<T = void> =
+  IsAny<T> extends true
+    ? WildcardPolicy | PropertyPolicy | T
+    : [T] extends [void]
+      ? WildcardPolicy | PropertyPolicy
+      : WildcardPolicy | PropertyPolicy | T;
 
 /**
  * An object representing a nested attenuation definition.
@@ -69,10 +91,10 @@
  * An object representing a base package policy.
  */
 export type PackagePolicy<
-  PackagePolicyItem = void,
-  GlobalsPolicyItem = void,
-  BuiltinsPolicyItem = void,
-  ExtraOptions = unknown,
+  PackagePolicyExtra = void,
+  GlobalsPolicyExtra = void,
+  BuiltinsPolicyExtra = void,
+  Options = unknown,
 > = {
   /**
    * The default attenuator, if any.
@@ -81,17 +103,17 @@
   /**
    * The policy item for packages.
    */
-  packages?: PolicyItem<PackagePolicyItem> | undefined;
+  packages?: PolicyItem<PackagePolicyExtra> | undefined;
   /**
    * The policy item or full attenuation definition for globals.
    */
-  globals?: AttenuationDefinition | PolicyItem<GlobalsPolicyItem> | undefined;
+  globals?: AttenuationDefinition | PolicyItem<GlobalsPolicyExtra> | undefined;
   /**
    * The policy item or nested attenuation definition for builtins.
    */
   builtins?:
     | NestedAttenuationDefinition
-    | PolicyItem<BuiltinsPolicyItem>
+    | PolicyItem<BuiltinsPolicyExtra>
     | undefined;
   /**
    * Whether to disable global freeze.
@@ -104,43 +126,47 @@
   /**
    * Any additional user-defined options can be added to the policy here
    */
-  options?: ExtraOptions | undefined;
+  options?: Options | undefined;
 };
 
 /**
  * An object representing a base policy.
  */
 export type Policy<
-  PackagePolicyItem = void,
-  GlobalsPolicyItem = void,
-  BuiltinsPolicyItem = void,
-  ExtraOptions = unknown,
+  PackagePolicyExtra = void,
+  GlobalsPolicyExtra = void,
+  BuiltinsPolicyExtra = void,
+  Options = unknown,
 > = {
   /** The package policies for the resources. */
   resources: Record<
     string,
     PackagePolicy<
-      PackagePolicyItem,
-      GlobalsPolicyItem,
-      BuiltinsPolicyItem,
-      ExtraOptions
+      PackagePolicyExtra,
+      GlobalsPolicyExtra,
+      BuiltinsPolicyExtra,
+      Options
     >
   >;
   /** The default attenuator. */
   defaultAttenuator?: string | undefined;
   /** The package policy for the entry. */
   entry?:
     | PackagePolicy<
-        PackagePolicyItem,
-        GlobalsPolicyItem,
-        BuiltinsPolicyItem,
-        ExtraOptions
+        PackagePolicyExtra,
+        GlobalsPolicyExtra,
+        BuiltinsPolicyExtra,
+        Options
       >
     | undefined;
 };
 
-/** Any {@link Policy} */
+/**
+ * Any {@link Policy}
+ */
 export type SomePolicy = Policy<any, any, any, any>;
 
-/** Any {@link PackagePolicy} */
-export type SomePackagePolicy = PackagePolicy<void, void, void, unknown>;
+/**
+ * Any {@link PackagePolicy}
+ */
+export type SomePackagePolicy = PackagePolicy<any, any, any, any>;

packages/compartment-mapper/src/types/typescript.ts

@@ -77,3 +77,11 @@ export type UnionToIntersection<U> = (
  * Makes a nicer tooltip for `T` in IDEs (most of the time).
  */
 export type Simplify<T> = { [K in keyof T]: T[K] } & {};
+
+/**
+ * `true` when the type parameter is a wide `any`
+ *
+ * `0 extends 1 & T` is true for `T = any` and false for `void` and specific literals
+ * @see {@link https://github.com/microsoft/TypeScript/issues/30029}
+ */
+export type IsAny<T> = 0 extends 1 & T ? true : false;