feat(import-bundle): Test bundle format #2719

[kriskowal]

Closes: #2719

Description

This change introduces a new bundle format to simplify tests for subsystems that take a bundle that they will ultimately use importBundle to extract a mock module exports namespace.
The new bundle format takes care to render impossible any accidental or malicious use of a test h
To that end, the format captures the mock exports namespace on a symbol-named property of then bundle, so it will be elided or rejected when serialized.
And, to compensate for the cryptic protocol, this change provides a tiny utility function for making test bundles from mock exports.

Security Considerations

Existing bundle importers expect only to be able to confine execution of local, serialized bundles.
This change takes some care to maintain the expectation by limiting exposure to live objects in a way that’s only reachable by test code.
If an adversary were able to present a test bundle, it’s not clear that this would constitute any escalation in privilege, given that an attacker would need to arrange for a live object by some other escalator.

Scaling Considerations

This change introduces a very small amount of new code to the Agoric kernel and, out of an abundance of caution, is not reached in the common case of importing the endoZipBase64 bundle format.

Documentation Considerations

The change includes relevant documentation in NEWS.md and README.md for the import-bundle package. This is a new platform API and may be relevant in the context of tutorials for testing applications on Agoric and other Endo platforms.

Testing Considerations

Includes unit tests exercising the new behavior and invariants.

Compatibility Considerations

New features, no breaking changes.

Upgrade Considerations

None.

[mhofman]

I think we can be a little more conformant without a much higher cost

[mhofman]

Oh interesting in French it's "ressemble"

[mhofman]

Some conformance feedback:

• Object.assign would copy any string or symbol own properties, but really we should only allow string properties as exports.
  • It does only copy enumerable properties, and sets values (doesn't copy accessors), which is nice.
• The exports string properties are supposed to be sorted
• The symbol property is supposed to enumerate after the string export properties
• The namespace object should be sealed
• While the exotic behavior is to have properties writable: true and deny any external changes to the properties, I think we should be fine with non-writable regular data properties
  • This would effectively make the namespace object frozen

There is a question of whether we should trigger getters on the test bundle exports at all, or we should deny this. In a perfect world we'd detect proxies and deny that as well.

[mhofman]

We should probably enforce string only data properties here too to catch mistakes early.

[dckc]

looks like fun!

To partially validate that it should help with these sorts of things...

• coverage analysis of bundled code Agoric/agoric-sdk#1817
• ability to start a contract without a bundle Agoric/agoric-sdk#10558

I did yarn add --dev @agoric/fast-usdc and then ran this test:

import test from '@endo/ses-ava/prepare-endo.js';
import { importBundle } from '../src/index.js';
import * as f from '@agoric/fast-usdc/src/fast-usdc.contract.js';

test('get live contract code using importBundle', async t => {
  const bundle = {
    moduleFormat: 'test',
    [Symbol.for('exports')]: f,
  };
  const ns = await importBundle(bundle);
  t.log(ns);
  t.is(typeof ns.start, 'function');
});

works as expected:

~/projects/endo/packages/import-bundle$ yarn test test/contract-fun.test.js 

  ✔ get live contract code using importBundle
    ℹ @Module {
        contract: AsyncFunction {},
        meta: {
          customTermsShape: {
            usdcDenom: Object @match:string { … },
          },
          privateArgsShape: {
            agoricNames: Object @match:kind { … },
            assetInfo: Object @match:arrayOf { … },
            chainInfo: Object @match:recordOf { … },
            feeConfig: Object { … },
            localchain: Object @match:kind { … },
            marshaller: Object @match:kind { … },
            orchestrationService: Object @match:kind { … },
            poolMetricsNode: Object @match:kind { … },
            storageNode: Object @match:kind { … },
            timerService: Object @match:kind { … },
          },
        },
        start: AsyncFunction {},
      }
  ─

  1 test passed

[kriskowal]

I’ve added an early error detection guard for symbol-named properties to the bundleTestExports utility method and also a defensive guard for symbol-named exports to importBundle, and sorted the properties of the simulated module exports namespace. Back to my reviewers in the studio.

[mhofman]

Suggested change

	* A utility function for producing test bundles, which are not seerializable
	* A utility function for producing test bundles, which are not serializable

[mhofman]

Nit: String keys on Module namespace objects are actually all writable but we purposely deviate here.

[mhofman]

Let's be flexible and allow a Symbol.toStringTag own prop so that a module namespace object can be used directly.

[mhofman]

Should we freeze the object (not harden it since we don't care about the stability of exports at this point)

[kriskowal]

That would be a departure from the norm for bundleSource. I think it’s reasonable to defer that choice to the caller. But, if we at some point reörganize the bundler infrastructure and create a new entry-point, I’d be open to changing the contract. Other reasons to consider reorganizing around an @endo/bundler module maybe someday would be to consolidate @endo/import-bundle, @endo/bundle-source, and @endo/check-bundle into a single package so they can more easily cross-test, do a better job of isolating powers and decoupling them from the underlying platform (like node-powers.js in the Compartment Mapper), remove deprecated powers like the ones taken by Rollup, one README for all the bundle formats and how to read and write them, &c. The list is growing but not pressing.

[mhofman]

LGTM