fix: Properly merge structs (#2512)

This PR introduces `mergeStructs` which merges structs, including
refinements, which `superstruct`s `assign` does not do by default. This
also fixes a bug where `endowment:rpc: {}` would not be reported as
invalid until attempting to install the Snap.

Fixes #2405

packages/snaps-utils/coverage.json

@@ -1,6 +1,6 @@
 {
   "branches": 96.77,
-  "functions": 98.76,
+  "functions": 98.77,
   "lines": 98.85,
-  "statements": 94.92
+  "statements": 95.11
 }

packages/snaps-utils/src/manifest/validation.test.ts

@@ -9,6 +9,7 @@ import {
   CurveStruct,
   EmptyObjectStruct,
   isSnapManifest,
+  PermissionsStruct,
   SnapIdsStruct,
 } from './validation';
 
@@ -259,3 +260,9 @@ describe('createSnapManifest', () => {
     expect(() => createSnapManifest(value)).toThrow(StructError);
   });
 });
+
+describe('PermissionsStruct', () => {
+  it('disallows empty endowment:rpc', () => {
+    expect(is({ 'endowment:rpc': {} }, PermissionsStruct)).toBe(false);
+  });
+});

packages/snaps-utils/src/manifest/validation.ts

@@ -27,7 +27,6 @@ import {
   type,
   union,
   intersection,
-  assign,
 } from 'superstruct';
 
 import { isEqual } from '../array';
@@ -36,7 +35,7 @@ import { SIP_6_MAGIC_VALUE, STATE_ENCRYPTION_MAGIC_VALUE } from '../entropy';
 import { KeyringOriginsStruct, RpcOriginsStruct } from '../json-rpc';
 import { ChainIdStruct } from '../namespace';
 import { SnapIdStruct } from '../snaps';
-import type { InferMatching } from '../structs';
+import { mergeStructs, type InferMatching } from '../structs';
 import { NameStruct, NpmSnapFileNames, uri } from '../types';
 
 // BIP-43 purposes that cannot be used for entropy derivation. These are in the
@@ -190,18 +189,18 @@ export const EmptyObjectStruct = object<EmptyObject>({}) as unknown as Struct<
 /* eslint-disable @typescript-eslint/naming-convention */
 export const PermissionsStruct: Describe<InitialPermissions> = type({
   'endowment:cronjob': optional(
-    assign(
+    mergeStructs(
       HandlerCaveatsStruct,
       object({ jobs: CronjobSpecificationArrayStruct }),
     ),
   ),
   'endowment:ethereum-provider': optional(EmptyObjectStruct),
   'endowment:keyring': optional(
-    assign(HandlerCaveatsStruct, KeyringOriginsStruct),
+    mergeStructs(HandlerCaveatsStruct, KeyringOriginsStruct),
   ),
   'endowment:lifecycle-hooks': optional(HandlerCaveatsStruct),
   'endowment:name-lookup': optional(
-    assign(
+    mergeStructs(
       HandlerCaveatsStruct,
       object({
         chains: optional(ChainIdsStruct),
@@ -211,17 +210,19 @@ export const PermissionsStruct: Describe<InitialPermissions> = type({
   ),
   'endowment:network-access': optional(EmptyObjectStruct),
   'endowment:page-home': optional(HandlerCaveatsStruct),
-  'endowment:rpc': optional(assign(HandlerCaveatsStruct, RpcOriginsStruct)),
+  'endowment:rpc': optional(
+    mergeStructs(HandlerCaveatsStruct, RpcOriginsStruct),
+  ),
   'endowment:signature-insight': optional(
-    assign(
+    mergeStructs(
       HandlerCaveatsStruct,
       object({
         allowSignatureOrigin: optional(boolean()),
       }),
     ),
   ),
   'endowment:transaction-insight': optional(
-    assign(
+    mergeStructs(
       HandlerCaveatsStruct,
       object({
         allowTransactionOrigin: optional(boolean()),

packages/snaps-utils/src/structs.test.ts

@@ -11,8 +11,11 @@ import superstruct, {
   validate,
   union as superstructUnion,
   array,
+  is,
 } from 'superstruct';
 
+import { RpcOriginsStruct } from './json-rpc';
+import { HandlerCaveatsStruct } from './manifest';
 import {
   arrayToGenerator,
   createFromStruct,
@@ -26,6 +29,7 @@ import {
   named,
   validateUnion,
   createUnion,
+  mergeStructs,
 } from './structs';
 
 /**
@@ -473,3 +477,22 @@ describe('createUnion', () => {
     expect(value).toStrictEqual({ type: 'a', value: 'bar' });
   });
 });
+
+describe('mergeStructs', () => {
+  it('merges objects', () => {
+    const struct1 = object({ a: string(), b: string(), c: string() });
+    const struct2 = object({ b: number() });
+    const struct3 = object({ a: number() });
+
+    const mergedStruct = mergeStructs(struct1, struct2, struct3);
+    expect(is({ a: 1, b: 2, c: 'c' }, mergedStruct)).toBe(true);
+    expect(is({ a: 'a', b: 2, c: 'c' }, mergedStruct)).toBe(false);
+    expect(is({ a: 1, b: 2, c: 3 }, mergedStruct)).toBe(false);
+  });
+
+  it('keeps refinements', () => {
+    const struct = mergeStructs(HandlerCaveatsStruct, RpcOriginsStruct);
+    expect(is({}, struct)).toBe(false);
+    expect(is({ dapps: true }, struct)).toBe(true);
+  });
+});

packages/snaps-utils/src/structs.ts

@@ -10,8 +10,14 @@ import {
   Struct,
   StructError,
   create,
+  assign,
 } from 'superstruct';
-import type { AnyStruct } from 'superstruct/dist/utils';
+import type {
+  AnyStruct,
+  Assign,
+  ObjectSchema,
+  ObjectType,
+} from 'superstruct/dist/utils';
 
 import { indent } from './strings';
 
@@ -461,3 +467,78 @@ export function createUnion<Type, Schema extends readonly Struct<any, any>[]>(
 ) {
   return validateUnion(value, struct, structKey, true);
 }
+
+// These types are copied from Superstruct, to mirror `assign`.
+export function mergeStructs<
+  ObjectA extends ObjectSchema,
+  ObjectB extends ObjectSchema,
+>(
+  A: Struct<ObjectType<ObjectA>, ObjectA>,
+  B: Struct<ObjectType<ObjectB>, ObjectB>,
+): Struct<ObjectType<Assign<ObjectA, ObjectB>>, Assign<ObjectA, ObjectB>>;
+export function mergeStructs<
+  ObjectA extends ObjectSchema,
+  ObjectB extends ObjectSchema,
+  ObjectC extends ObjectSchema,
+>(
+  A: Struct<ObjectType<ObjectA>, ObjectA>,
+  B: Struct<ObjectType<ObjectB>, ObjectB>,
+  C: Struct<ObjectType<ObjectC>, ObjectC>,
+): Struct<
+  ObjectType<Assign<Assign<ObjectA, ObjectB>, ObjectC>>,
+  Assign<Assign<ObjectA, ObjectB>, ObjectC>
+>;
+export function mergeStructs<
+  ObjectA extends ObjectSchema,
+  ObjectB extends ObjectSchema,
+  ObjectC extends ObjectSchema,
+  ObjectD extends ObjectSchema,
+>(
+  A: Struct<ObjectType<ObjectA>, ObjectA>,
+  B: Struct<ObjectType<ObjectB>, ObjectB>,
+  C: Struct<ObjectType<ObjectC>, ObjectC>,
+  D: Struct<ObjectType<ObjectD>, ObjectD>,
+): Struct<
+  ObjectType<Assign<Assign<Assign<ObjectA, ObjectB>, ObjectC>, ObjectD>>,
+  Assign<Assign<Assign<ObjectA, ObjectB>, ObjectC>, ObjectD>
+>;
+export function mergeStructs<
+  ObjectA extends ObjectSchema,
+  ObjectB extends ObjectSchema,
+  ObjectC extends ObjectSchema,
+  ObjectD extends ObjectSchema,
+  ObjectE extends ObjectSchema,
+>(
+  A: Struct<ObjectType<ObjectA>, ObjectA>,
+  B: Struct<ObjectType<ObjectB>, ObjectB>,
+  C: Struct<ObjectType<ObjectC>, ObjectC>,
+  D: Struct<ObjectType<ObjectD>, ObjectD>,
+  E: Struct<ObjectType<ObjectE>, ObjectE>,
+): Struct<
+  ObjectType<
+    Assign<Assign<Assign<Assign<ObjectA, ObjectB>, ObjectC>, ObjectD>, ObjectE>
+  >,
+  Assign<Assign<Assign<Assign<ObjectA, ObjectB>, ObjectC>, ObjectD>, ObjectE>
+>;
+
+/**
+ * Merge multiple structs into one, using superstruct `assign`.
+ *
+ * Differently from plain `assign`, this function also copies over refinements from each struct.
+ *
+ * @param structs - The `superstruct` structs to merge.
+ * @returns The merged struct.
+ */
+export function mergeStructs(...structs: Struct<any>[]): Struct<any> {
+  const mergedStruct = (assign as (...structs: Struct<any>[]) => Struct)(
+    ...structs,
+  );
+  return new Struct({
+    ...mergedStruct,
+    *refiner(value, ctx) {
+      for (const struct of structs) {
+        yield* struct.refiner(value, ctx);
+      }
+    },
+  });
+}