From 755ba3fa8bb2f2733608dd44e0f61165a4d4c068 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 12 Nov 2026 15:37:50 -0600 Subject: [PATCH] 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 | 10 +++++++--- packages/plugins/tests/starter-pack.test.mjs | 20 ++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/packages/plugins/src/starter-pack.mjs b/packages/plugins/src/starter-pack.mjs index 02fa7584e..27781f9c6 100644 --- a/packages/plugins/src/starter-pack.mjs +++ b/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. - if (doc !== null && (typeof doc === "object" || typeof doc === "function") && typeof doc.then === "function") { - try { Promise.resolve(doc).catch(() => {}); } catch { /* the refusal below stands */ } + // must not reach the process as an unhandled one. Only a native promise is recognised, and + // the built-in then is used, so the module's own then or getter never runs here; any other + // 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); diff --git a/packages/plugins/tests/starter-pack.test.mjs b/packages/plugins/tests/starter-pack.test.mjs index 87f92e2a1..c778b28d1 100644 --- a/packages/plugins/tests/starter-pack.test.mjs +++ b/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, []); });