fix(plugins): starter-pack re-check round (#1873 SP1)

Recognise only a native promise from starterPack() and attach the catch
through Promise.prototype.then, so an installed pack's own then or then
getter never runs; such a pack is omitted and logged again instead of
failing the listing. A link count of 0 after the open reports the file
changed, not a hard link. Tests: a throwing then getter, a self-resolving
thenable, and a 0000 file refusing as unreadable.

packages/plugins/src/starter-pack.mjs

@@ -17,6 +17,7 @@
 import { closeSync, constants, fstatSync, lstatSync, openSync, readSync } from "node:fs";
 import { dirname, isAbsolute, join, resolve } from "node:path";
 import { fileURLToPath } from "node:url";
+import { types } from "node:util";
 import { ENTRY_KEYS, checkDefinition, readTemplate } from "../../organization/src/definitions.mjs";
 import { parseJsonFileText } from "../../organization/src/util.mjs";
 import { ID_PATTERN, NETWORKS } from "../../organization/src/vocabulary.mjs";
@@ -377,9 +378,11 @@ export async function installedStarterPacks({ dataRoot = null, rolesDir = ROLES_
         fail("starter-pack-invalid", `starter pack ${name} failed to describe itself`, name);
       }
       // A pack is returned, never awaited: a promise refuses below, and a later rejection of it
-      // must not reach the process as an unhandled one.
+      // must not reach the process as an unhandled one. Only a native promise is recognised, and
-      if (doc !== null && (typeof doc === "object" || typeof doc === "function") && typeof doc.then === "function") {
+      // the built-in then is used, so the module's own then or getter never runs here; any other
-        try { Promise.resolve(doc).catch(() => {}); } catch { /* the refusal below stands */ }
+      // object goes to checkPack, whose closed key sets refuse it.
+      if (types.isPromise(doc)) {
+        Promise.prototype.then.call(doc, undefined, () => {});
         fail("starter-pack-invalid", `starter pack ${name} must return its document, not a promise`, name);
       }
       packs.push({ id: name, origin, pack: checkPack(doc, { origin, rolesDir, name }) });
@@ -446,6 +449,7 @@ export function readSavedTemplate({ configDir, account, name, rolesDir = ROLES_D
     const st = fstatSync(fd);
     if (!st.isFile() || st.dev !== before.dev || st.ino !== before.ino) fail("starter-pack-invalid", "the saved template changed while it was read", name);
     if (st.uid !== uid) fail("starter-pack-invalid", "the saved template must belong to the instance's user", name);
+    if (st.nlink === 0) fail("starter-pack-invalid", "the saved template changed while it was read", name);
     if (st.nlink !== 1) fail("starter-pack-invalid", "the saved template must not be a hard link", name);
     if ((st.mode & 0o077) !== 0) fail("starter-pack-invalid", "the saved template must be mode 0600 or narrower", name);
     if (st.size > SAVED_MAX_BYTES) fail("starter-pack-invalid", `the saved template must be at most ${SAVED_MAX_BYTES} bytes`, name);

packages/plugins/tests/starter-pack.test.mjs

@@ -429,6 +429,11 @@ test("SP1 readSavedTemplate refuses .., a name or account holding /, a symlink,
   assert.throws(read("solo"), code("starter-pack-invalid", /hard link/));
   f.put("narrow", JSON.stringify(pack()), 0o400);
   assert.ok(read("narrow")());
+  // A file its owner can't open refuses as unreadable, not as a symlink.
+  if (process.getuid() !== 0) {
+    f.put("shut", JSON.stringify(pack()), 0o000);
+    assert.throws(read("shut"), code("starter-pack-invalid", /can't be read/));
+  }
   // 300 KiB refuses before the read; JSON padding keeps the file otherwise valid.
   const big = JSON.stringify(pack());
   f.put("big", big + " ".repeat(300 * 1024 - big.length));
@@ -586,4 +591,19 @@ test("SP1 an installed pack lists with origin installed; a refused one is omitte
   assert.match(res2.refused[0].message, /not a promise/);
   await new Promise((r) => setTimeout(r, 100));
   assert.deepEqual(unhandled, []);
+  // A thenable that is not a promise never has its then read or called: a throwing then getter
+  // and a then that resolves with itself are both omitted and logged, and a shipped pack beside
+  // them is unaffected.
+  const thenables = {
+    getterpack: "export const interfaceVersion = 1;\nexport function starterPack() { return { get then() { throw new Error('getter'); } }; }\n",
+    selfpack: "export const interfaceVersion = 1;\nlet calls = 0;\nexport function starterPack() { const o = { then(r) { calls += 1; if (calls > 3) throw new Error('then called'); r(o); } }; return o; }\n",
+  };
+  for (const [name, text] of Object.entries(thenables)) {
+    const odd = installed(t, name, text);
+    const res3 = await installedStarterPacks({ dataRoot: odd.dataRoot, rolesDir: ROLES, log() {} });
+    assert.deepEqual(res3.packs, []);
+    assert.deepEqual(res3.refused.map((e) => [e.name, e.code]), [[name, "starter-pack-invalid"]]);
+  }
+  await new Promise((r) => setTimeout(r, 50));
+  assert.deepEqual(unhandled, []);
 });