Repository: endojs/endo
Pull request #2374 (2024-07-23 20:34:27)

chore(compartment-mapper): Restore backward compatibility for search
Refs: 592a485e22 #2344

## Description

#2344 introduced a regression in backward compatibility which has not yet been released. The change improved our ability to differentiate ENOENT and EISDIR from other error classes when searching for package.json. This adjusts that change to tolerate the currently accepted `read` function and allow read powers that included a more precise `maybeRead` function instead.

### Security Considerations

None.

### Scaling Considerations

None.

### Documentation Considerations

None.

### Testing Considerations

Coverage test added for `search`.

### Compatibility Considerations

This restores backward compatibility.

### Upgrade Considerations

Nnoe.

turadg: > https://github.com/endojs/endo/pull/2344 introduced a regression in backward compatibility which has not yet been released

Incidentally, I think the `fix` commit message will be interpreted by the release pipeline as a "bug fixes" entry for the changelog, which won't be accurate because there was no such bug in a versioned release.

Patch:
diff --git a/packages/compartment-mapper/src/bundle.js b/packages/compartment-mapper/src/bundle.js
index c7bcd78d0..1c431c58a 100644
--- a/packages/compartment-mapper/src/bundle.js
+++ b/packages/compartment-mapper/src/bundle.js
@@ -219,7 +219,7 @@ const getBundlerKitForModule = module => {
  * @returns {Promise<string>}
  */
 export const makeBundle = async (readPowers, moduleLocation, options) => {
-  const { read, maybeRead } = unpackReadPowers(readPowers);
+  const { read } = unpackReadPowers(readPowers);
 
   const {
     moduleTransforms,
@@ -250,7 +250,7 @@ export const makeBundle = async (readPowers, moduleLocation, options) => {
     packageDescriptorText,
     packageDescriptorLocation,
     moduleSpecifier,
-  } = await search(maybeRead, moduleLocation);
+  } = await search(readPowers, moduleLocation);
 
   const packageDescriptor = parseLocatedJson(
     packageDescriptorText,
diff --git a/packages/compartment-mapper/src/node-modules.js b/packages/compartment-mapper/src/node-modules.js
index 9138eb807..786d93a06 100644
--- a/packages/compartment-mapper/src/node-modules.js
+++ b/packages/compartment-mapper/src/node-modules.js
@@ -820,14 +820,12 @@ export const mapNodeModules = async (
     policy,
   } = options;
 
-  const { maybeRead } = unpackReadPowers(readPowers);
-
   const {
     packageLocation,
     packageDescriptorText,
     packageDescriptorLocation,
     moduleSpecifier,
-  } = await search(maybeRead, moduleLocation);
+  } = await search(readPowers, moduleLocation);
 
   const packageDescriptor = parseLocatedJson(
     packageDescriptorText,
diff --git a/packages/compartment-mapper/src/search.js b/packages/compartment-mapper/src/search.js
index 23abe753e..a8ec3b04d 100644
--- a/packages/compartment-mapper/src/search.js
+++ b/packages/compartment-mapper/src/search.js
@@ -4,10 +4,13 @@
 
 // @ts-check
 
-/** @import {MaybeReadFn} from './types.js' */
+/** @import {ReadFn} from './types.js' */
+/** @import {ReadPowers} from './types.js' */
+/** @import {MaybeReadPowers} from './types.js' */
 
 import { relativize } from './node-module-specifier.js';
 import { relative } from './url.js';
+import { unpackReadPowers } from './powers.js';
 
 // q, as in quote, for enquoting strings in error messages.
 const q = JSON.stringify;
@@ -59,15 +62,16 @@ export const searchDescriptor = async (location, maybeReadDescriptor) => {
 };
 
 /**
- * @param {MaybeReadFn} maybeRead
+ * @param {ReadFn | ReadPowers | MaybeReadPowers} readPowers
  * @param {string} packageDescriptorLocation
  * @returns {Promise<string | undefined>}
  */
 const maybeReadDescriptorDefault = async (
-  maybeRead,
+  readPowers,
   packageDescriptorLocation,
   // eslint-disable-next-line consistent-return
 ) => {
+  const { maybeRead } = unpackReadPowers(readPowers);
   const packageDescriptorBytes = await maybeRead(packageDescriptorLocation);
   if (packageDescriptorBytes !== undefined) {
     const packageDescriptorText = decoder.decode(packageDescriptorBytes);
@@ -82,7 +86,7 @@ const maybeReadDescriptorDefault = async (
  * To avoid duplicate work later, returns the text of the package.json for
  * inevitable later use.
  *
- * @param {MaybeReadFn} maybeRead
+ * @param {ReadFn | ReadPowers | MaybeReadPowers} readPowers
  * @param {string} moduleLocation
  * @returns {Promise<{
  *   packageLocation: string,
@@ -91,7 +95,8 @@ const maybeReadDescriptorDefault = async (
  *   moduleSpecifier: string,
  * }>}
  */
-export const search = async (maybeRead, moduleLocation) => {
+export const search = async (readPowers, moduleLocation) => {
+  const { maybeRead } = unpackReadPowers(readPowers);
   const { data, directory, location, packageDescriptorLocation } =
     await searchDescriptor(moduleLocation, loc =>
       maybeReadDescriptorDefault(maybeRead, loc),


Test patch:
diff --git a/packages/compartment-mapper/test/search.test.js b/packages/compartment-mapper/test/search.test.js
new file mode 100644
index 000000000..3730a579b
--- /dev/null
+++ b/packages/compartment-mapper/test/search.test.js
@@ -0,0 +1,59 @@
+import test from 'ava';
+import { search } from '../src/search.js';
+
+const makeFakeReadPowers = files => {
+  return {
+    async read(location) {
+      const bytes = files[location];
+      if (bytes === undefined) {
+        throw new Error(`File not found: ${location}`);
+      }
+      return bytes;
+    },
+    async maybeRead(location) {
+      return files[location];
+    },
+  };
+};
+
+test('search should find own package.json with read power', async t => {
+  const sought = new URL('../package.json', import.meta.url).href;
+  const readPowers = makeFakeReadPowers({
+    [sought]: new TextEncoder().encode('{}'),
+  });
+  const { read } = readPowers;
+  const { packageDescriptorLocation: found } = await search(
+    read,
+    import.meta.url,
+  );
+  t.is(found, sought);
+});
+
+test('search should find own package.json with read powers (plural)', async t => {
+  const sought = new URL('../package.json', import.meta.url).href;
+  const readPowers = makeFakeReadPowers({
+    [sought]: new TextEncoder().encode('{}'),
+  });
+  const { packageDescriptorLocation: found } = await search(
+    readPowers,
+    import.meta.url,
+  );
+  t.is(found, sought);
+});
+
+test('search should fail to find package.json if nowhere to be found', async t => {
+  const nothing = new URL('child/package.json', import.meta.url).href;
+  const readPowers = makeFakeReadPowers({
+    [nothing]: new Uint8Array(),
+  });
+  const { read } = readPowers;
+  await t.throwsAsync(() => search(read, import.meta.url));
+});
+
+test('search should fail to find package.json if nowhere to be found with read powers (plural)', async t => {
+  const nothing = new URL('child/package.json', import.meta.url).href;
+  const readPowers = makeFakeReadPowers({
+    [nothing]: new Uint8Array(),
+  });
+  await t.throwsAsync(() => search(readPowers, import.meta.url));
+});