From ff33b198547fd03446e78412168095e6dfb75f02 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 12 Nov 2026 15:21:23 -0600 Subject: [PATCH 1/4] feat(plugins): starter-pack kind and its checker (#1873 SP1) KINDS gains "starter-pack" (interface 1, export starterPack) with a new switch "none": namesFor refuses any setting for it with bad-setting, so an org file can never switch a pack on through plugins. starter-pack.mjs adds checkPack(doc, {origin}), narrowsTemplate and widerLines (step 7's line rules), installedStarterPacks (shipped and installed packs through loadInstalled, origin set; a refused core pack fails the listing, a refused installed pack is omitted and logged) and readSavedTemplate (account against ID_PATTERN, name against NAME, then lstat, no symlink, owner uid, mode 0600 or narrower, 256 KiB checked before the read, then parseJsonFileText and checkPack with origin saved). refusals.mjs gains starter-pack-invalid, starter-pack-not-found and starter-pack-template-changed. --- packages/bus/src/refusals.mjs | 17 + packages/plugins/src/kinds.mjs | 10 +- packages/plugins/src/starter-pack.mjs | 467 +++++++++++++++ packages/plugins/tests/kinds.test.mjs | 2 +- packages/plugins/tests/starter-pack.test.mjs | 561 +++++++++++++++++++ 5 files changed, 1055 insertions(+), 2 deletions(-) create mode 100644 packages/plugins/src/starter-pack.mjs create mode 100644 packages/plugins/tests/starter-pack.test.mjs diff --git a/packages/bus/src/refusals.mjs b/packages/bus/src/refusals.mjs index 4b9ab07c9..833e05919 100644 --- a/packages/bus/src/refusals.mjs +++ b/packages/bus/src/refusals.mjs @@ -184,6 +184,23 @@ group( "R3/role scope: configured project and service state", "Read the supported project/service status and ask its owner to repair missing configuration or state before retrying." ); +// #1873 SP1: starter packs. A pack copies items into an organization and grants nothing; a refusal +// names positions and checked names only, never a pack's text or a file path. +group( + "starter-pack-invalid", + "R8/R10: a pack copies no grant above the shipped templates", + "Read the refusal's position in the pack and correct it there. A role must narrow the template its from names; a pack never carries credentials, members, plugins, launch settings or a root hire. A shipped or installed pack needs a reviewed fix or its plug-in owner." +); +group( + "starter-pack-not-found", + "R10: only the caller's own saved templates", + "List your saved templates and use one of those names. A saved template is visible only to the owner who saved it." +); +group( + "starter-pack-template-changed", + "R8: a pack's hash pin must match the current template", + "The template the pack pins has changed since the pack was written. Review the current template, then re-pin the pack to its new hash, or use a pack that already matches." +); group( "not-resolver", "Role resolve right and decision route", diff --git a/packages/plugins/src/kinds.mjs b/packages/plugins/src/kinds.mjs index 294fe58f3..16cd83020 100644 --- a/packages/plugins/src/kinds.mjs +++ b/packages/plugins/src/kinds.mjs @@ -84,6 +84,12 @@ export const KINDS = Object.freeze({ "system-type": Object.freeze({ kind: "system-type", interfaceVersion: 1, exports: Object.freeze(["systemType"]), switch: "list", }), + // A starter pack (#1873 SP1): starterPack() takes nothing and returns a pack document, which + // starter-pack.mjs checks with the origin of where it was read. A pack is copied into an + // organization, never switched on, so its switch is "none": `plugins` can never name one. + "starter-pack": Object.freeze({ + kind: "starter-pack", interfaceVersion: 1, exports: Object.freeze(["starterPack"]), switch: "none", + }), // A Console colour theme (stream S4, PLAN C5). A module is data only: theme() takes nothing and returns // { displayName, tokens: { light, dim, dark } } // where each mode is a flat map of the Console's colour custom properties to #rrggbb strings. theme.mjs @@ -118,8 +124,10 @@ const raise = (code, message, name) => { }; // The module names one kind's setting switches on, checked against the -// kind's cardinality and the name pattern. A list may be empty. +// kind's cardinality and the name pattern. A list may be empty. A kind whose +// switch is "none" takes no setting at all. export function namesFor(spec, setting, where, fail = raise) { + if (spec.switch === "none") fail("bad-setting", `${where} can't be set; a ${spec.kind} is never switched on`); if (spec.switch === "one") { if (typeof setting !== "string") fail("bad-setting", `${where} must be one module name`); } else if (!Array.isArray(setting)) { diff --git a/packages/plugins/src/starter-pack.mjs b/packages/plugins/src/starter-pack.mjs new file mode 100644 index 000000000..7759ed0ee --- /dev/null +++ b/packages/plugins/src/starter-pack.mjs @@ -0,0 +1,467 @@ +// Starter packs, slice SP1 (#1873): the pack kind's checker and its three readers. Design: +// starter-packs-design section 2 and SP1 in section 8. +// +// A pack is a declarative document that lists definitions, hires, a board style, workflows and +// skills to copy into an organization or a project. It grants nothing and never stays switched on. +// Three sources, each with the origin its reader sets (a closed set, never a field the document +// supplies): +// shipped a core module packages/plugins/starter-pack//index.mjs, through the loader; +// installed a PL1 hash-pinned plug-in of kind starter-pack, through the same loader; +// saved /templates//.json, read as data, never imported. +// Origin `saved` relaxes exactly two rules: it may carry `packs`, and a definition may have no +// `from`. Every other check runs for every origin. +// +// A refusal names a position (a key, an index) and names that already passed a strict pattern, +// never a value from the document and never a host path. + +import { closeSync, constants, fstatSync, lstatSync, openSync, readSync } from "node:fs"; +import { dirname, isAbsolute, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +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"; +import { NAME, PluginError } from "./kinds.mjs"; +import { hasModule, loadInstalled } from "./loader.mjs"; + +export const KIND = "starter-pack"; +export const PACK_VERSION = 1; +export const ORIGINS = Object.freeze(["shipped", "installed", "saved"]); +// The task tracker's five states. The same list as STORE_STATES in packages/bus/src/broker.mjs; +// plugins does not import the bus, so the test pins the two together. +export const STORE_STATES = Object.freeze(["todo", "in-progress", "in-review", "blocked", "done"]); +export const LAYOUTS = Object.freeze(["kanban", "list"]); +export const HUMAN_STEP = "human"; +export const PACK_BOUNDS = Object.freeze({ + hires: 8, reason: 500, label: 64, description: 1000, columnLabel: 32, + steps: 20, stepTitle: 120, stepRoles: 8, workflowDescription: 1000, + skills: 64, packs: 32, version: 64, +}); +export const SAVED_MAX_BYTES = 256 * 1024; + +const TOP_KEYS = Object.freeze(["packVersion", "id", "version", "label", "description", "scope", "definitions", + "hires", "board", "workflows", "skills"]); +const SAVED_KEYS = Object.freeze([...TOP_KEYS, "packs"]); +const SCOPES = Object.freeze(["org", "project"]); +const REF_KEYS = Object.freeze(["template", "sha256"]); +// A pack definition is a T1 entry without rev; apply sets rev 1. +const PACK_ENTRY_KEYS = Object.freeze(ENTRY_KEYS.filter((key) => key !== "rev")); +const HIRE_KEYS = Object.freeze(["name", "definition", "reason"]); +const BOARD_KEYS = Object.freeze(["columns", "layout"]); +const COLUMN_KEYS = Object.freeze(["state", "label"]); +const WORKFLOW_KEYS = Object.freeze(["description", "steps"]); +const STEP_KEYS = Object.freeze(["id", "title", "roles", "state"]); +const PACKS_ENTRY_KEYS = Object.freeze(["id", "version"]); +const MESSAGING_KEYS = Object.freeze(["sendTo", "receiveFrom"]); +const HEX64 = /^[0-9a-f]{64}$/; +// Semantic Versioning 2.0.0, the pattern semver.org gives. +const SEMVER = /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$/; + +// The shipped roles, as organization-service.mjs resolves them. +const ROLES_DIR = resolve(dirname(fileURLToPath(import.meta.url)), "../../../roles"); + +const fail = (code, message, name = null) => { + throw new PluginError(code, message, name); +}; + +const plain = (value) => value !== null && typeof value === "object" && !Array.isArray(value) + && [Object.prototype, null].includes(Object.getPrototypeOf(value)); + +const isSemver = (value) => typeof value === "string" && value.length <= PACK_BOUNDS.version && SEMVER.test(value); + +// Non-blank, one line, no NUL, at most max characters. +const lineText = (value, max) => typeof value === "string" && value.trim().length > 0 && value.length <= max + && !/[\r\n\0]/.test(value); + +// ---- Narrowing ---- + +function credentialLines(credentials) { + const out = []; + for (const { service, scopes } of credentials ?? []) { + if (scopes === undefined) out.push([service, service]); + else if (Array.isArray(scopes)) for (const scope of scopes) out.push([service, scope]); + else for (const [group, verbs] of Object.entries(scopes)) for (const verb of verbs) out.push([service, `${group}.${verb}`]); + } + return out; +} + +const listOf = (value) => (Array.isArray(value) ? value : []); + +// Every line of `role` its template does not hold, as [{ field, value }], in the order +// checkComposition's step 7 reads them. A within-role action is held only within-role; a +// cross-role one either way. A template with no messaging key lets the role message anyone; a role +// with no messaging key against a template that has one is wider ("*"). `role` is a composition +// (checkDefinition's result, or a T1 role); `template` is a version 2 role document. +export function widerLines(role, template) { + const out = []; + const add = (field, value) => out.push({ field, value }); + const within = listOf(template.authority?.withinRole); + const any = [...within, ...listOf(template.authority?.crossRole)]; + listOf(role.authority?.withinRole).forEach((a) => { if (!within.includes(a)) add("authority.withinRole", a); }); + listOf(role.authority?.crossRole).forEach((a) => { if (!any.includes(a)) add("authority.crossRole", a); }); + const tools = listOf(template.tools); + listOf(role.tools).forEach((t) => { if (!tools.includes(t)) add("tools", t); }); + const network = NETWORKS.indexOf(role.network); + if (network === -1 || network > NETWORKS.indexOf(template.network)) add("network", role.network); + const held = new Set(credentialLines(template.credentials).map(([s, v]) => `${s}\n${v}`)); + for (const [service, value] of credentialLines(role.credentials)) { + if (!held.has(`${service}\n${value}`)) add(`credentials.${service}`, value); + } + const templateMessaging = template.messaging ?? null; + const roleMessaging = role.messaging ?? null; + if (templateMessaging) { + for (const key of MESSAGING_KEYS) { + if (roleMessaging === null) add(`messaging.${key}`, "*"); + else listOf(roleMessaging[key]).forEach((t) => { if (!listOf(templateMessaging[key]).includes(t)) add(`messaging.${key}`, t); }); + } + } + const skills = listOf(template.skills); + listOf(role.skills).forEach((s) => { if (!skills.includes(s)) add("skills", s); }); + return out; +} + +// True when every tool, action, skill, messaging target and credential request line of `role` is +// in `template`, and its network is no wider: "no grant above the templates". +export function narrowsTemplate(role, template) { + return widerLines(role, template).length === 0; +} + +// ---- The checker ---- + +function keysOnly(object, allowed, where, id) { + Object.keys(object).forEach((key, i) => { + if (!allowed.includes(key)) fail("starter-pack-invalid", `${where} has an unknown key (key ${i + 1}; allowed: ${allowed.join(", ")})`, id); + }); +} + +function requireKeys(object, required, where, id) { + for (const key of required) { + if (!Object.hasOwn(object, key)) fail("starter-pack-invalid", `${where} requires "${key}"`, id); + } +} + +// One template, read now: { role, sha256 } or null when the name is no usable version 2 template. +function templateOrNull(rolesDir, name) { + if (typeof name !== "string" || !ID_PATTERN.test(name)) return null; + try { + return readTemplate(rolesDir, name); + } catch (error) { + if (error?.name === "BusinessError") return null; + throw error; + } +} + +function checkDefinitions(definitions, { origin, rolesDir, id }) { + if (!plain(definitions)) fail("starter-pack-invalid", "starter pack definitions must be a JSON object", id); + const templates = new Map(); + const template = (name) => { + if (!templates.has(name)) templates.set(name, templateOrNull(rolesDir, name)); + return templates.get(name); + }; + Object.keys(definitions).forEach((name, i) => { + if (!ID_PATTERN.test(name)) fail("starter-pack-invalid", `starter pack definitions key ${i + 1} must be a definition name matching ${ID_PATTERN}`, id); + const at = `starter pack definitions.${name}`; + const entry = definitions[name]; + if (!plain(entry)) fail("starter-pack-invalid", `${at} must be a JSON object`, id); + const keys = Object.keys(entry); + if (keys.length === REF_KEYS.length && REF_KEYS.every((k) => keys.includes(k))) { + // A template reference: the whole template, pinned by its hash. + if (typeof entry.template !== "string" || !ID_PATTERN.test(entry.template)) fail("starter-pack-invalid", `${at}.template must be a template name matching ${ID_PATTERN}`, id); + if (typeof entry.sha256 !== "string" || !HEX64.test(entry.sha256)) fail("starter-pack-invalid", `${at}.sha256 must be 64 lowercase hex characters`, id); + const t = template(entry.template); + if (!t) fail("starter-pack-invalid", `${at} names template ${entry.template}, which is not a version 2 role`, id); + if (t.sha256 !== entry.sha256) fail("starter-pack-template-changed", `${at} pins template ${entry.template} at a hash it no longer has`, id); + return; + } + // A full entry in the T1 shape, without rev. + keysOnly(entry, PACK_ENTRY_KEYS, at, id); + let composition; + try { + ({ composition } = checkDefinition(name, { ...entry, rev: 1 }, { where: "starter pack definitions" })); + } catch (error) { + // definitions.mjs and composition.mjs quote only names that passed a strict pattern. + if (error?.name === "BusinessError") fail("starter-pack-invalid", error.message, id); + throw error; + } + if (entry.from === undefined) { + if (origin !== "saved") fail("starter-pack-invalid", `${at} has no "from"; only a saved template may carry a role that copies no template`, id); + return; + } + const t = template(entry.from.template); + if (!t) fail("starter-pack-invalid", `${at}.from names template ${entry.from.template}, which is not a version 2 role`, id); + if (t.sha256 !== entry.from.sha256) fail("starter-pack-template-changed", `${at}.from pins template ${entry.from.template} at a hash it no longer has`, id); + const wider = widerLines(composition, t.role); + if (wider.length) { + const fields = [...new Set(wider.map((l) => l.field))].join(", "); + fail("starter-pack-invalid", `${at}.role is wider than template ${entry.from.template} (${fields})`, id); + } + }); +} + +function checkHires(hires, definitions, id) { + if (!Array.isArray(hires)) fail("starter-pack-invalid", "starter pack hires must be an array", id); + if (hires.length > PACK_BOUNDS.hires) fail("starter-pack-invalid", `starter pack hires must hold at most ${PACK_BOUNDS.hires} entries`, id); + const names = new Set(); + hires.forEach((hire, i) => { + const at = `starter pack hires[${i}]`; + if (!plain(hire)) fail("starter-pack-invalid", `${at} must be a JSON object`, id); + // No level and no route: a hire is org-level in an org pack, project-level in a project pack, + // never root, and the raiser supplies the route. + keysOnly(hire, HIRE_KEYS, at, id); + requireKeys(hire, HIRE_KEYS, at, id); + if (typeof hire.name !== "string" || !ID_PATTERN.test(hire.name)) fail("starter-pack-invalid", `${at}.name must match ${ID_PATTERN}`, id); + if (names.has(hire.name)) fail("starter-pack-invalid", `${at}.name repeats an earlier hire`, id); + names.add(hire.name); + if (typeof hire.definition !== "string" || !Object.hasOwn(definitions, hire.definition)) { + fail("starter-pack-invalid", `${at}.definition must name a definition in this pack`, id); + } + if (typeof hire.reason !== "string" || hire.reason.trim().length === 0 || hire.reason.length > PACK_BOUNDS.reason || hire.reason.includes("\0")) { + fail("starter-pack-invalid", `${at}.reason must be a non-blank string of at most ${PACK_BOUNDS.reason} characters`, id); + } + }); +} + +function checkBoard(board, id) { + if (!plain(board)) fail("starter-pack-invalid", "starter pack board must be a JSON object", id); + keysOnly(board, BOARD_KEYS, "starter pack board", id); + requireKeys(board, BOARD_KEYS, "starter pack board", id); + if (!LAYOUTS.includes(board.layout)) fail("starter-pack-invalid", `starter pack board.layout must be one of ${LAYOUTS.join(", ")}`, id); + const columns = board.columns; + if (!Array.isArray(columns) || columns.length !== STORE_STATES.length) { + fail("starter-pack-invalid", `starter pack board.columns must hold the ${STORE_STATES.length} task states, each once`, id); + } + const seen = new Set(); + columns.forEach((column, i) => { + const at = `starter pack board.columns[${i}]`; + if (!plain(column)) fail("starter-pack-invalid", `${at} must be a JSON object`, id); + keysOnly(column, COLUMN_KEYS, at, id); + requireKeys(column, COLUMN_KEYS, at, id); + if (!STORE_STATES.includes(column.state)) fail("starter-pack-invalid", `${at}.state must be one of ${STORE_STATES.join(", ")}`, id); + if (seen.has(column.state)) fail("starter-pack-invalid", `${at}.state ${column.state} repeats an earlier column`, id); + seen.add(column.state); + if (!lineText(column.label, PACK_BOUNDS.columnLabel)) fail("starter-pack-invalid", `${at}.label must be one line of 1 to ${PACK_BOUNDS.columnLabel} characters`, id); + }); +} + +function checkWorkflows(workflows, definitions, { rolesDir, id }) { + if (!plain(workflows)) fail("starter-pack-invalid", "starter pack workflows must be a JSON object", id); + const templates = new Map(); + const isTemplate = (name) => { + if (!templates.has(name)) templates.set(name, templateOrNull(rolesDir, name) !== null); + return templates.get(name); + }; + Object.keys(workflows).forEach((name, i) => { + if (!NAME.test(name)) fail("starter-pack-invalid", `starter pack workflows key ${i + 1} must match ${NAME}`, id); + const at = `starter pack workflows.${name}`; + const workflow = workflows[name]; + if (!plain(workflow)) fail("starter-pack-invalid", `${at} must be a JSON object`, id); + keysOnly(workflow, WORKFLOW_KEYS, at, id); + requireKeys(workflow, WORKFLOW_KEYS, at, id); + if (typeof workflow.description !== "string" || workflow.description.trim().length === 0 + || workflow.description.length > PACK_BOUNDS.workflowDescription || workflow.description.includes("\0")) { + fail("starter-pack-invalid", `${at}.description must be a non-blank string of at most ${PACK_BOUNDS.workflowDescription} characters`, id); + } + const steps = workflow.steps; + if (!Array.isArray(steps) || steps.length < 1 || steps.length > PACK_BOUNDS.steps) { + fail("starter-pack-invalid", `${at}.steps must hold 1 to ${PACK_BOUNDS.steps} steps`, id); + } + const ids = new Set(); + steps.forEach((step, j) => { + const where = `${at}.steps[${j}]`; + if (!plain(step)) fail("starter-pack-invalid", `${where} must be a JSON object`, id); + keysOnly(step, STEP_KEYS, where, id); + requireKeys(step, STEP_KEYS, where, id); + if (typeof step.id !== "string" || !NAME.test(step.id)) fail("starter-pack-invalid", `${where}.id must match ${NAME}`, id); + if (ids.has(step.id)) fail("starter-pack-invalid", `${where}.id repeats an earlier step`, id); + ids.add(step.id); + if (!lineText(step.title, PACK_BOUNDS.stepTitle)) fail("starter-pack-invalid", `${where}.title must be one line of 1 to ${PACK_BOUNDS.stepTitle} characters`, id); + if (!STORE_STATES.includes(step.state)) fail("starter-pack-invalid", `${where}.state must be one of ${STORE_STATES.join(", ")}`, id); + const roles = step.roles; + if (!Array.isArray(roles) || roles.length < 1 || roles.length > PACK_BOUNDS.stepRoles) { + fail("starter-pack-invalid", `${where}.roles must hold 1 to ${PACK_BOUNDS.stepRoles} names`, id); + } + const seen = new Set(); + roles.forEach((role, k) => { + const ok = typeof role === "string" + && (role === HUMAN_STEP || Object.hasOwn(definitions, role) || isTemplate(role)); + if (!ok) fail("starter-pack-invalid", `${where}.roles[${k}] must be a definition in this pack, a version 2 template or "${HUMAN_STEP}"`, id); + if (seen.has(role)) fail("starter-pack-invalid", `${where}.roles[${k}] repeats an earlier name`, id); + seen.add(role); + }); + }); + }); +} + +function checkNameList(value, where, max, id) { + if (!Array.isArray(value)) fail("starter-pack-invalid", `${where} must be an array`, id); + if (value.length > max) fail("starter-pack-invalid", `${where} must hold at most ${max} entries`, id); + const seen = new Set(); + value.forEach((name, i) => { + if (typeof name !== "string" || !NAME.test(name)) fail("starter-pack-invalid", `${where}[${i}] must match ${NAME}`, id); + if (seen.has(name)) fail("starter-pack-invalid", `${where}[${i}] repeats an earlier entry`, id); + seen.add(name); + }); +} + +function checkPacksList(packs, id) { + if (!Array.isArray(packs)) fail("starter-pack-invalid", "starter pack packs must be an array", id); + if (packs.length > PACK_BOUNDS.packs) fail("starter-pack-invalid", `starter pack packs must hold at most ${PACK_BOUNDS.packs} entries`, id); + packs.forEach((entry, i) => { + const at = `starter pack packs[${i}]`; + if (!plain(entry)) fail("starter-pack-invalid", `${at} must be a JSON object`, id); + keysOnly(entry, PACKS_ENTRY_KEYS, at, id); + requireKeys(entry, PACKS_ENTRY_KEYS, at, id); + if (typeof entry.id !== "string" || !NAME.test(entry.id)) fail("starter-pack-invalid", `${at}.id must match ${NAME}`, id); + if (!isSemver(entry.version)) fail("starter-pack-invalid", `${at}.version must be a semantic version`, id); + }); +} + +// Checks one pack document for its origin and returns a checked copy; the input is not read again +// after the copy. options.rolesDir is the shipped roles directory (tests pass their own); +// options.name, for a module, is the module's name, which the pack's id must equal. +export function checkPack(doc, { origin, rolesDir = ROLES_DIR, name } = {}) { + if (!ORIGINS.includes(origin)) fail("starter-pack-invalid", `starter pack origin must be one of ${ORIGINS.join(", ")}`); + let pack; + try { + pack = JSON.parse(JSON.stringify(doc)); + } catch { + fail("starter-pack-invalid", "starter pack must be a JSON document"); + } + if (!plain(pack)) fail("starter-pack-invalid", "starter pack must be a JSON object"); + const id = typeof pack.id === "string" && NAME.test(pack.id) ? pack.id : null; + keysOnly(pack, origin === "saved" ? SAVED_KEYS : TOP_KEYS, "starter pack", id); + requireKeys(pack, TOP_KEYS, "starter pack", id); + if (pack.packVersion !== PACK_VERSION) fail("starter-pack-invalid", `starter pack packVersion must be ${PACK_VERSION}`, id); + if (id === null) fail("starter-pack-invalid", `starter pack id must match ${NAME}`); + if (name !== undefined && id !== name) fail("starter-pack-invalid", `starter pack id must equal its module name ${name}`, id); + if (!isSemver(pack.version)) fail("starter-pack-invalid", "starter pack version must be a semantic version", id); + if (!lineText(pack.label, PACK_BOUNDS.label)) fail("starter-pack-invalid", `starter pack label must be one line of 1 to ${PACK_BOUNDS.label} characters`, id); + if (!lineText(pack.description, PACK_BOUNDS.description)) fail("starter-pack-invalid", `starter pack description must be one line of 1 to ${PACK_BOUNDS.description} characters`, id); + if (!SCOPES.includes(pack.scope)) fail("starter-pack-invalid", `starter pack scope must be one of ${SCOPES.join(", ")}`, id); + checkDefinitions(pack.definitions, { origin, rolesDir, id }); + checkHires(pack.hires, pack.definitions, id); + checkBoard(pack.board, id); + checkWorkflows(pack.workflows, pack.definitions, { rolesDir, id }); + checkNameList(pack.skills, "starter pack skills", PACK_BOUNDS.skills, id); + if (Object.hasOwn(pack, "packs")) checkPacksList(pack.packs, id); + return pack; +} + +// ---- Shipped and installed packs ---- + +// Lists shipped and installed packs through the loader, each checked with the origin of where it +// came from. A refused core pack fails the listing, as system types do. A refused installed pack is +// omitted and logged. Returns { packs: [{ id, origin, pack }], refused }. +export async function installedStarterPacks({ dataRoot = null, rolesDir = ROLES_DIR, log = () => {} } = {}) { + const { modules, refused: loadRefused } = await loadInstalled(KIND, { dataRoot }); + const refused = []; + const omit = (name, error) => { + const entry = { event: "starter-pack-refused", kind: KIND, name, code: error.code, message: error.message }; + log(entry); + refused.push(entry); + }; + for (const entry of loadRefused) { + // A refusal with no name, or under a core module's name, is the core tree: fail closed. + if (entry.name === null || hasModule(KIND, entry.name)) { + fail("starter-pack-invalid", "a shipped starter pack was refused", entry.name); + } + omit(entry.name, entry); + } + const packs = []; + for (const { name, module, installed } of modules) { + const origin = installed ? "installed" : "shipped"; + try { + let doc; + try { + doc = module.starterPack(); + } catch { + fail("starter-pack-invalid", `starter pack ${name} failed to describe itself`, name); + } + packs.push({ id: name, origin, pack: checkPack(doc, { origin, rolesDir, name }) }); + } catch (error) { + if (!(error instanceof PluginError)) throw error; + if (origin === "shipped") throw error; + omit(name, error); + } + } + return { packs, refused }; +} + +// ---- Saved templates ---- + +function lstatOrNull(path) { + try { + return lstatSync(path); + } catch (error) { + if (error.code === "ENOENT" || error.code === "ENOTDIR") return null; + fail("starter-pack-invalid", "the saved template can't be read"); + } +} + +// A directory on the way to a saved template: a real directory owned by the instance's user that +// no group or other user can write. +function checkDirectory(path, uid) { + const st = lstatOrNull(path); + if (st === null) fail("starter-pack-not-found", "no saved template has that name"); + if (st.isSymbolicLink() || !st.isDirectory()) fail("starter-pack-invalid", "the saved templates directory must be a real directory"); + if (st.uid !== uid) fail("starter-pack-invalid", "the saved templates directory must belong to the instance's user"); + if ((st.mode & 0o022) !== 0) fail("starter-pack-invalid", "the saved templates directory must not be writable by group or others"); +} + +// Reads /templates//.json as data and checks it with origin `saved`. The +// account is the calling owner's id (s.human), checked against ID_PATTERN; the name is checked +// against NAME. Both are checked before any path is built, so neither ".." nor "/" reaches the file +// system. The file must be a regular file, not a symlink, owned by the instance's user, mode 0600 +// or narrower, at most 256 KiB, checked before the read. Returns { name, origin: "saved", pack }. +export function readSavedTemplate({ configDir, account, name, rolesDir = ROLES_DIR, uid = process.getuid() } = {}) { + if (typeof account !== "string" || !ID_PATTERN.test(account)) fail("starter-pack-invalid", `the saved template account must match ${ID_PATTERN}`); + if (typeof name !== "string" || !NAME.test(name)) fail("starter-pack-invalid", `the saved template name must match ${NAME}`); + if (typeof configDir !== "string" || !isAbsolute(configDir)) fail("starter-pack-invalid", "the configuration directory must be an absolute path"); + const templates = join(configDir, "templates"); + const accountDir = join(templates, account); + checkDirectory(templates, uid); + checkDirectory(accountDir, uid); + const file = join(accountDir, `${name}.json`); + const before = lstatOrNull(file); + if (before === null) fail("starter-pack-not-found", "no saved template has that name", name); + if (before.isSymbolicLink() || !before.isFile()) fail("starter-pack-invalid", "the saved template must be a regular file, not a symlink", name); + let fd; + try { + fd = openSync(file, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); + } catch (error) { + if (error.code === "ENOENT") fail("starter-pack-not-found", "no saved template has that name", name); + fail("starter-pack-invalid", "the saved template must be a regular file, not a symlink", name); + } + let bytes; + try { + 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.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); + // Read at most one byte past the limit, so a file that grew after the fstat still refuses. + const buffer = Buffer.alloc(SAVED_MAX_BYTES + 1); + let length = 0; + for (;;) { + const n = readSync(fd, buffer, length, buffer.length - length, null); + if (n === 0) break; + length += n; + if (length > SAVED_MAX_BYTES) fail("starter-pack-invalid", `the saved template must be at most ${SAVED_MAX_BYTES} bytes`, name); + } + bytes = buffer.subarray(0, length); + } finally { + closeSync(fd); + } + let text; + try { + text = new TextDecoder("utf-8", { fatal: true }).decode(bytes); + } catch { + fail("starter-pack-invalid", "the saved template is not valid UTF-8", name); + } + let doc; + try { + doc = parseJsonFileText(text, file, "saved template"); + } catch { + fail("starter-pack-invalid", "the saved template is not valid JSON", name); + } + return { name, origin: "saved", pack: checkPack(doc, { origin: "saved", rolesDir }) }; +} diff --git a/packages/plugins/tests/kinds.test.mjs b/packages/plugins/tests/kinds.test.mjs index 022a807d1..137b4239f 100644 --- a/packages/plugins/tests/kinds.test.mjs +++ b/packages/plugins/tests/kinds.test.mjs @@ -6,7 +6,7 @@ import { KINDS, NAME, PluginError, checkPlugins } from "../src/kinds.mjs"; const code = expected => e => e instanceof PluginError && e.code === expected; test("REQ-PLUG-1 REQ-LIMIT-4 the registry: usage-source at interface version 1, limit-check at 2 (row 125), each a list", () => { - assert.deepEqual(Object.keys(KINDS), ["usage-source", "limit-check", "search-source", "channel", "harness", "usage", "model-provider", "system-type", "theme", "task-tracker"]); + assert.deepEqual(Object.keys(KINDS), ["usage-source", "limit-check", "search-source", "channel", "harness", "usage", "model-provider", "system-type", "starter-pack", "theme", "task-tracker"]); assert.deepEqual({ ...KINDS["task-tracker"], exports: [...KINDS["task-tracker"].exports] }, { kind: "task-tracker", interfaceVersion: 1, exports: ["adapter"], switch: "one" }); assert.ok(Object.isFrozen(KINDS["task-tracker"]) && Object.isFrozen(KINDS["task-tracker"].exports)); // Stream S4 (PLAN C5): a theme is data. interfaceVersion 1, one export, a list switch. diff --git a/packages/plugins/tests/starter-pack.test.mjs b/packages/plugins/tests/starter-pack.test.mjs new file mode 100644 index 000000000..0a4a9958b --- /dev/null +++ b/packages/plugins/tests/starter-pack.test.mjs @@ -0,0 +1,561 @@ +// #1873 SP1: the starter-pack kind, checkPack, narrowsTemplate and the three readers. +// The starter-pack module is imported lazily so each test reports its own failure on a tree +// without it (the red-before run). +import test from "node:test"; +import assert from "node:assert/strict"; +import { chmodSync, cpSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { tmpdir } from "node:os"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { KINDS, PluginError, checkPlugins, namesFor } from "../src/kinds.mjs"; +import { createLoader } from "../src/loader.mjs"; +import { pluginHash } from "../src/pinned.mjs"; +import { installPlugin } from "../src/install.mjs"; +import { templateHash } from "../../organization/src/definitions.mjs"; +import { REFUSALS } from "../../bus/src/refusals.mjs"; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const REPO = resolve(HERE, "../../.."); +const ROLES = join(REPO, "roles"); +const MARK = "MARK-sp1-not-echoed-7c1f"; +let loaded; +const sp = async () => (loaded ??= await import("../src/starter-pack.mjs")); + +const code = (expected) => (e) => { + assert.equal(e?.name, "PluginError", e?.stack); + assert.equal(e.code, expected, e.message); + assert.ok(!e.message.includes(MARK), e.message); + assert.ok(!e.message.includes(REPO), e.message); + return true; +}; + +const H = { coder: templateHash(ROLES, "coder"), reviewer: templateHash(ROLES, "reviewer") }; + +const writer = () => ({ + from: { template: "coder", sha256: H.coder }, + role: { + tools: ["read", "grep"], network: "none", + authority: { withinRole: ["record.write"], crossRole: ["task.scope.change"] }, + credentials: [{ service: "gitea", scopes: ["read:user"] }, { service: "vikunja", scopes: { tasks: ["read_one"] } }], + skills: ["ms-unslop"], + }, + instructions: "Write the release notes for each merged change.", + personality: { summary: "Plain and short." }, +}); + +const columns = () => [ + { state: "todo", label: "Backlog" }, { state: "in-progress", label: "Building" }, + { state: "in-review", label: "Review" }, { state: "blocked", label: "Blocked" }, { state: "done", label: "Merged" }, +]; + +const pack = (patch = (p) => p) => { + const p = { + packVersion: 1, id: "coding", version: "1.0.0", label: "Coding", + description: "A small software team with review before merge.", scope: "org", + definitions: { coder: { template: "coder", sha256: H.coder }, reviewer: { template: "reviewer", sha256: H.reviewer }, "release-writer": writer() }, + hires: [ + { name: "coder-1", definition: "coder", reason: "writes the code" }, + { name: "reviewer-1", definition: "reviewer", reason: "reviews each change" }, + ], + board: { columns: columns(), layout: "kanban" }, + workflows: { + change: { + description: "One change from task to merge.", + steps: [ + { id: "plan", title: "Write the task", roles: ["pm"], state: "todo" }, + { id: "build", title: "Implement", roles: ["coder", "release-writer"], state: "in-progress" }, + { id: "review", title: "Review", roles: ["reviewer"], state: "in-review" }, + { id: "merge", title: "Approve the merge", roles: ["human"], state: "done" }, + ], + }, + }, + skills: ["ms-sdlc", "ms-unslop"], + }; + return patch(p) ?? p; +}; + +const shipped = (doc) => sp().then((m) => m.checkPack(doc, { origin: "shipped", rolesDir: ROLES })); +const saved = (doc) => sp().then((m) => m.checkPack(doc, { origin: "saved", rolesDir: ROLES })); + +// ---- Red before ---- + +test("SP1 red before: a definition carrying copyBindings refuses starter-pack-invalid", async () => { + const doc = pack((p) => { p.definitions["release-writer"].copyBindings = { gitea: { copyId: MARK } }; }); + await assert.rejects(shipped(doc), code("starter-pack-invalid")); +}); + +test("SP1 red before: plugins naming starter-pack refuses bad-setting, never switching a pack on", () => { + assert.throws(() => checkPlugins({ "starter-pack": ["coding"] }, "plugins"), (e) => e instanceof PluginError && e.code === "bad-setting"); +}); + +// ---- The kind ---- + +test("SP1 kind: starter-pack at interface 1, export starterPack, switch none; namesFor refuses every setting", async () => { + assert.deepEqual({ ...KINDS["starter-pack"], exports: [...KINDS["starter-pack"].exports] }, + { kind: "starter-pack", interfaceVersion: 1, exports: ["starterPack"], switch: "none" }); + assert.ok(Object.isFrozen(KINDS["starter-pack"]) && Object.isFrozen(KINDS["starter-pack"].exports)); + for (const setting of [[], ["coding"], "coding", null, {}]) { + assert.throws(() => namesFor(KINDS["starter-pack"], setting, "plugins.starter-pack"), (e) => e instanceof PluginError && e.code === "bad-setting"); + assert.throws(() => checkPlugins({ "starter-pack": setting }, "plugins"), (e) => e instanceof PluginError && e.code === "bad-setting"); + } + // The loader switches nothing on for the kind: present, the key is refused; absent, it loads []. + const log = []; + const loader = createLoader({ log: (e) => log.push(e) }); + const first = await loader.load({ "starter-pack": ["coding"] }); + assert.deepEqual(first.loaded["starter-pack"], []); + assert.deepEqual(first.refused.map((r) => [r.kind, r.code]), [["starter-pack", "bad-setting"]]); + const second = await loader.load({}); + assert.deepEqual(second.loaded["starter-pack"], []); + assert.deepEqual(second.refused, []); +}); + +test("SP1 refusal table: each new code has a rail and a next step that names no path", () => { + for (const c of ["starter-pack-invalid", "starter-pack-not-found", "starter-pack-template-changed"]) { + assert.ok(Object.hasOwn(REFUSALS, c), c); + assert.ok(REFUSALS[c].rail?.length > 5, c); + assert.ok(REFUSALS[c].nextStep?.length > 20, c); + assert.ok(!/(^|[\s("'`])(\/|~\/)/.test(REFUSALS[c].nextStep), c); + } +}); + +test("SP1 STORE_STATES matches the bus's task states", async () => { + const { STORE_STATES } = await sp(); + const broker = readFileSync(join(REPO, "packages/bus/src/broker.mjs"), "utf8"); + const m = /export const STORE_STATES = Object\.freeze\((\[[^\]]*\])\)/.exec(broker); + assert.ok(m); + assert.deepEqual([...STORE_STATES], JSON.parse(m[1].replaceAll("'", '"'))); +}); + +// ---- checkPack ---- + +test("SP1 a valid pack loads for every origin and comes back as a checked copy", async () => { + const { checkPack } = await sp(); + for (const origin of ["shipped", "installed", "saved"]) { + const doc = pack(); + const out = checkPack(doc, { origin, rolesDir: ROLES }); + assert.deepEqual(out, doc); + assert.notEqual(out, doc); + assert.notEqual(out.definitions, doc.definitions); + } + // The shipped roles are the default. + assert.deepEqual(checkPack(pack(), { origin: "shipped" }), pack()); + // A project pack, empty lists and a list layout load too. + const small = pack((p) => { p.scope = "project"; p.hires = []; p.workflows = {}; p.skills = []; p.board.layout = "list"; p.board.columns.reverse(); }); + assert.deepEqual(checkPack(small, { origin: "shipped", rolesDir: ROLES }), small); +}); + +test("SP1 the origin is a closed set and a module's pack id must equal its name", async () => { + const { checkPack } = await sp(); + for (const origin of [undefined, "core", "Saved", MARK]) { + assert.throws(() => checkPack(pack(), { origin, rolesDir: ROLES }), code("starter-pack-invalid")); + } + assert.throws(() => checkPack(pack(), { origin: "shipped", rolesDir: ROLES, name: "other" }), code("starter-pack-invalid")); + assert.ok(checkPack(pack(), { origin: "shipped", rolesDir: ROLES, name: "coding" })); +}); + +test("SP1 a forbidden key refuses, at the top, in a definition, a role, a hire, the board and a step", async () => { + const forbidden = [ + (p) => { p.plugins = { "task-tracker": MARK }; }, + (p) => { p.launch = { model: MARK }; }, + (p) => { p.members = [MARK]; }, + (p) => { p.credentials = [MARK]; }, + (p) => { p.definitions["release-writer"].credentials = { token: MARK }; }, + (p) => { p.definitions["release-writer"].rev = 1; }, + (p) => { p.definitions["release-writer"].role.copyBindings = MARK; }, + (p) => { p.definitions.coder.copyId = MARK; }, + (p) => { p.hires[0].route = MARK; }, + (p) => { p.board.theme = MARK; }, + (p) => { p.board.columns[0].color = MARK; }, + (p) => { p.workflows.change.title = MARK; }, + (p) => { p.workflows.change.steps[0].assignee = MARK; }, + ]; + for (const mutate of forbidden) await assert.rejects(shipped(pack(mutate)), code("starter-pack-invalid")); +}); + +test("SP1 a stale template hash refuses starter-pack-template-changed, for a reference and a from", async () => { + const stale = "0".repeat(64); + await assert.rejects(shipped(pack((p) => { p.definitions.coder.sha256 = stale; })), code("starter-pack-template-changed")); + await assert.rejects(shipped(pack((p) => { p.definitions["release-writer"].from.sha256 = stale; })), code("starter-pack-template-changed")); + await assert.rejects(saved(pack((p) => { p.definitions["release-writer"].from.sha256 = stale; })), code("starter-pack-template-changed")); + // A name that is no version 2 template, and a bad hash shape, are invalid, not changed. + await assert.rejects(shipped(pack((p) => { p.definitions.coder.template = "no-such-role"; })), code("starter-pack-invalid")); + await assert.rejects(shipped(pack((p) => { p.definitions.coder.template = "conductor-policy"; })), code("starter-pack-invalid")); + await assert.rejects(shipped(pack((p) => { p.definitions.coder.sha256 = MARK; })), code("starter-pack-invalid")); + await assert.rejects(shipped(pack((p) => { p.definitions.coder.template = `../${MARK}`; })), code("starter-pack-invalid")); +}); + +test("SP1 a role wider than its template refuses: a tool, an action, the network, a skill, within for cross", async () => { + const role = (f) => pack((p) => { f(p.definitions["release-writer"].role); }); + for (const f of [ + (r) => { r.network = "open"; }, + (r) => { r.skills.push("ms-git-operations"); }, + (r) => { r.authority.withinRole = ["record.write", "task.scope.change"]; r.authority.crossRole = []; }, + ]) await assert.rejects(shipped(role(f)), code("starter-pack-invalid")); + // An action the template does not hold. pm's within-role actions include one coder lacks. + const pm = JSON.parse(readFileSync(join(ROLES, "pm.json"), "utf8")); + const coder = JSON.parse(readFileSync(join(ROLES, "coder.json"), "utf8")); + const extra = pm.authority.withinRole.find((a) => ![...coder.authority.withinRole, ...coder.authority.crossRole].includes(a) + && !a.startsWith("decision.resolve.") && !["role.hire", "role.fire", "role.change"].includes(a)); + assert.ok(extra, "pm holds a within-role action coder lacks"); + await assert.rejects(shipped(role((r) => { r.authority.withinRole.push(extra); })), code("starter-pack-invalid")); + // A tool coder lacks. + await assert.rejects(shipped(role((r) => { r.tools.push("web"); r.network = "api-only"; })), code("starter-pack-invalid")); + // A saved role with a from narrows too. + await assert.rejects(saved(role((r) => { r.network = "open"; })), code("starter-pack-invalid")); + // A cross-role action of the template may be held cross-role; a within-role one may move to cross. + const ok = role((r) => { r.authority.withinRole = []; r.authority.crossRole = ["task.scope.change", "record.write"]; }); + assert.ok(await shipped(ok)); +}); + +test("SP1 a credential line wider than its template refuses", async () => { + const cred = (credentials) => pack((p) => { p.definitions["release-writer"].role.credentials = credentials; }); + for (const credentials of [ + [{ service: "gitea", scopes: ["write:user"] }], + [{ service: "gitea", scopes: ["read:user", "write:organization"] }], + [{ service: "vikunja", scopes: { tasks: ["create"] } }], + [{ service: "vikunja", scopes: { labels: ["read_all"] } }], + ]) await assert.rejects(shipped(cred(credentials)), code("starter-pack-invalid")); + assert.ok(await shipped(cred([]))); + assert.ok(await shipped(cred([{ service: "vikunja", scopes: { tasks: ["read_one", "update"], projects: ["views_buckets_tasks"] } }]))); +}); + +test("SP1 narrowsTemplate and widerLines read every line the way step 7 does", async () => { + const { narrowsTemplate, widerLines } = await sp(); + const template = { + tools: ["read", "grep"], network: "api-only", + authority: { withinRole: ["record.write"], crossRole: ["task.scope.change"] }, + credentials: [{ service: "gitea", scopes: ["read:user"] }, { service: "vikunja", scopes: { tasks: ["read_one"] } }, { service: "host.box" }], + messaging: { sendTo: ["pm"], receiveFrom: ["pm", "coder"] }, skills: ["ms-unslop"], + }; + const base = { tools: ["read"], network: "none", authority: { withinRole: [], crossRole: ["record.write"] }, + credentials: [{ service: "host.box" }, { service: "vikunja", scopes: { tasks: ["read_one"] } }], + messaging: { sendTo: [], receiveFrom: ["coder"] }, skills: [] }; + assert.equal(narrowsTemplate(base, template), true); + assert.deepEqual(widerLines({ ...base, messaging: null }, template), [ + { field: "messaging.sendTo", value: "*" }, { field: "messaging.receiveFrom", value: "*" }]); + assert.deepEqual(widerLines({ ...base, messaging: { sendTo: ["coder"], receiveFrom: [] } }, template), [{ field: "messaging.sendTo", value: "coder" }]); + // A template with no messaging lets the role message anyone. + assert.equal(narrowsTemplate({ ...base, messaging: { sendTo: ["x"], receiveFrom: ["y"] } }, { ...template, messaging: null }), true); + assert.deepEqual(widerLines({ ...base, network: "open", tools: ["bash"], skills: ["s"], + authority: { withinRole: ["task.scope.change"], crossRole: ["x.y"] }, + credentials: [{ service: "gitea" }, { service: "gitea", scopes: ["write:user"] }, { service: "vikunja", scopes: { tasks: ["update"] } }] }, template), [ + { field: "authority.withinRole", value: "task.scope.change" }, { field: "authority.crossRole", value: "x.y" }, + { field: "tools", value: "bash" }, { field: "network", value: "open" }, + { field: "credentials.gitea", value: "gitea" }, { field: "credentials.gitea", value: "write:user" }, + { field: "credentials.vikunja", value: "tasks.update" }, { field: "skills", value: "s" }, + ]); + assert.equal(narrowsTemplate({ ...base, network: "bogus" }, template), false); +}); + +test("SP1 a root hire, a ninth hire, a hire naming no pack definition and other bad hires refuse", async () => { + const hire = (i) => ({ name: `coder-${i}`, definition: "coder", reason: "writes code" }); + for (const mutate of [ + (p) => { p.hires[0].level = "root"; }, + (p) => { p.hires[0].level = "org"; }, + (p) => { p.hires = Array.from({ length: 9 }, (_, i) => hire(i + 1)); }, + (p) => { p.hires[0].definition = "pm"; }, + (p) => { p.hires[0].definition = "__proto__"; }, + (p) => { p.hires[1].name = p.hires[0].name; }, + (p) => { p.hires[0].name = "Root/../x"; }, + (p) => { p.hires[0].reason = " "; }, + (p) => { p.hires[0].reason = "x".repeat(501); }, + (p) => { delete p.hires[0].reason; }, + (p) => { p.hires = {}; }, + ]) await assert.rejects(shipped(pack(mutate)), code("starter-pack-invalid")); + assert.ok(await shipped(pack((p) => { p.hires = Array.from({ length: 8 }, (_, i) => hire(i + 1)); }))); +}); + +test("SP1 a bad state and a duplicate column refuse, and the board keeps its bounds", async () => { + for (const mutate of [ + (p) => { p.board.columns[0].state = "doing"; }, + (p) => { p.workflows.change.steps[1].state = "doing"; }, + (p) => { p.board.columns[1] = { state: "todo", label: "Again" }; }, + (p) => { p.board.columns.pop(); }, + (p) => { p.board.columns.push({ state: "done", label: "More" }); }, + (p) => { p.board.columns[0].label = ""; }, + (p) => { p.board.columns[0].label = "x".repeat(33); }, + (p) => { p.board.columns[0].label = "two\nlines"; }, + (p) => { p.board.layout = "grid"; }, + (p) => { delete p.board.layout; }, + ]) await assert.rejects(shipped(pack(mutate)), code("starter-pack-invalid")); + assert.ok(await shipped(pack((p) => { p.board.columns[0].label = "x".repeat(32); }))); +}); + +test("SP1 workflows: 1 to 20 steps, 1 to 8 known roles, bounded titles and descriptions", async () => { + const step = (i) => ({ id: `s${i}`, title: "Step", roles: ["human"], state: "todo" }); + for (const mutate of [ + (p) => { p.workflows.change.steps = []; }, + (p) => { p.workflows.change.steps = Array.from({ length: 21 }, (_, i) => step(i)); }, + (p) => { p.workflows.change.steps[0].roles = []; }, + (p) => { p.workflows.change.steps[0].roles = ["pm", "coder", "reviewer", "human", "cto", "assistant", "research", "ideas", "companion"]; }, + (p) => { p.workflows.change.steps[0].roles = ["no-such-role"]; }, + (p) => { p.workflows.change.steps[0].roles = ["conductor-policy"]; }, + (p) => { p.workflows.change.steps[0].roles = ["pm", "pm"]; }, + (p) => { p.workflows.change.steps[0].title = "x".repeat(121); }, + (p) => { p.workflows.change.steps[1].id = "plan"; }, + (p) => { p.workflows.change.description = "x".repeat(1001); }, + (p) => { p.workflows.change.description = ""; }, + (p) => { p.workflows.Change = p.workflows.change; }, + ]) await assert.rejects(shipped(pack(mutate)), code("starter-pack-invalid")); + assert.ok(await shipped(pack((p) => { + p.workflows.change.steps = Array.from({ length: 20 }, (_, i) => step(i)); + p.workflows.change.steps[0].roles = ["pm", "coder", "reviewer", "human", "cto", "assistant", "research", "ideas"]; + p.workflows.change.steps[0].title = "x".repeat(120); + p.workflows.change.description = "x".repeat(1000); + }))); +}); + +test("SP1 top-level fields: packVersion, id, semver version, label, description, scope and skills", async () => { + for (const mutate of [ + (p) => { p.packVersion = 2; }, + (p) => { delete p.packVersion; }, + (p) => { p.id = "Coding"; }, + (p) => { p.id = "../x"; }, + (p) => { p.version = "1.0"; }, + (p) => { p.version = "v1.0.0"; }, + (p) => { p.version = "01.0.0"; }, + (p) => { p.label = ""; }, + (p) => { p.label = "x".repeat(65); }, + (p) => { p.description = "a\nb"; }, + (p) => { p.scope = "root"; }, + (p) => { delete p.skills; }, + (p) => { p.skills = ["ms_sdlc"]; }, + (p) => { p.skills = ["ms-sdlc", "ms-sdlc"]; }, + (p) => { p.definitions = []; }, + (p) => { p.definitions = { "Bad Name": { template: "coder", sha256: H.coder } }; }, + ]) await assert.rejects(shipped(pack(mutate)), code("starter-pack-invalid")); + for (const version of ["1.0.0-rc.1", "2.10.3+build.5", "0.0.1"]) assert.ok(await shipped(pack((p) => { p.version = version; }))); + const { checkPack } = await sp(); + for (const doc of [null, [], "x", 1, { toJSON() { throw new Error(MARK); } }]) { + assert.throws(() => checkPack(doc, { origin: "shipped", rolesDir: ROLES }), code("starter-pack-invalid")); + } +}); + +test("SP1 a definition that fails checkDefinition refuses without quoting the document", async () => { + for (const mutate of [ + (p) => { p.definitions["release-writer"].instructions = ""; }, + (p) => { p.definitions["release-writer"].role.authority.withinRole.push("role.hire"); }, + (p) => { p.definitions["release-writer"].role.tools.push(MARK); }, + (p) => { p.definitions["release-writer"].from.template = MARK.toLowerCase(); }, + ]) await assert.rejects(shipped(pack(mutate)), code("starter-pack-invalid")); +}); + +test("SP1 a shipped or installed pack with packs or a role without from refuses; a saved one with either passes", async () => { + const withPacks = pack((p) => { p.packs = [{ id: "marketing", version: "1.2.0" }]; }); + const noFrom = pack((p) => { delete p.definitions["release-writer"].from; }); + const { checkPack } = await sp(); + for (const origin of ["shipped", "installed"]) { + assert.throws(() => checkPack(withPacks, { origin, rolesDir: ROLES }), code("starter-pack-invalid")); + assert.throws(() => checkPack(noFrom, { origin, rolesDir: ROLES }), code("starter-pack-invalid")); + } + assert.deepEqual(await saved(withPacks), withPacks); + assert.deepEqual(await saved(noFrom), noFrom); + // A saved role without from is still checked by checkDefinition. + await assert.rejects(saved(pack((p) => { delete p.definitions["release-writer"].from; p.definitions["release-writer"].role.authority.withinRole.push("role.fire"); })), code("starter-pack-invalid")); +}); + +test("SP1 a saved packs list: a 33rd entry, a key other than id and version, or a non-semver version refuses", async () => { + const entry = (i) => ({ id: `pack-${i}`, version: "1.0.0" }); + for (const packs of [ + Array.from({ length: 33 }, (_, i) => entry(i)), + [{ id: "coding", version: "1.0.0", sha256: "0".repeat(64) }], + [{ id: "coding", version: "1.0.0", origin: "shipped" }], + [{ id: "coding", version: "1.0" }], + [{ id: "coding", version: MARK }], + [{ id: "coding" }], + [{ id: "Coding", version: "1.0.0" }], + {}, + ]) await assert.rejects(saved(pack((p) => { p.packs = packs; })), code("starter-pack-invalid")); + const ok = pack((p) => { p.packs = Array.from({ length: 32 }, (_, i) => entry(i)); }); + assert.deepEqual(await saved(ok), ok); +}); + +// ---- readSavedTemplate ---- + +function savedFixture(t, account = "alice.b") { + const root = mkdtempSync(join(tmpdir(), "sp1-")); + t.after(() => rmSync(root, { recursive: true, force: true })); + const configDir = join(root, "cfg"); + const dir = join(configDir, "templates", account); + mkdirSync(dir, { recursive: true, mode: 0o700 }); + chmodSync(join(configDir, "templates"), 0o700); + chmodSync(dir, 0o700); + const put = (name, text, mode = 0o600) => { + const file = join(dir, `${name}.json`); + writeFileSync(file, text, { mode }); + chmodSync(file, mode); + return file; + }; + return { root, configDir, dir, put }; +} + +test("SP1 readSavedTemplate reads a saved template as origin saved; an account with a dot passes", async (t) => { + const { readSavedTemplate } = await sp(); + const f = savedFixture(t, "alice.b"); + const doc = pack((p) => { p.packs = [{ id: "coding", version: "1.0.0" }]; delete p.definitions["release-writer"].from; }); + f.put("my-team", JSON.stringify(doc)); + const out = readSavedTemplate({ configDir: f.configDir, account: "alice.b", name: "my-team", rolesDir: ROLES }); + assert.deepEqual(out, { name: "my-team", origin: "saved", pack: doc }); + // Owner ids may hold an underscore or start with a digit. + for (const account of ["bob_c", "9lives"]) { + const g = savedFixture(t, account); + g.put("t", JSON.stringify(pack())); + assert.equal(readSavedTemplate({ configDir: g.configDir, account, name: "t", rolesDir: ROLES }).pack.id, "coding"); + } +}); + +test("SP1 readSavedTemplate refuses .., a name or account holding /, a symlink, a 0644 file, a 300 KiB file; a missing one is not found", async (t) => { + const { readSavedTemplate } = await sp(); + const f = savedFixture(t, "alice.b"); + const read = (name, account = "alice.b", extra = {}) => () => readSavedTemplate({ configDir: f.configDir, account, name, rolesDir: ROLES, ...extra }); + f.put("good", JSON.stringify(pack())); + assert.ok(read("good")()); + for (const name of ["..", "a/b", "../alice.b/good", "/etc/passwd", "Good", "", null, 3]) assert.throws(read(name), code("starter-pack-invalid")); + for (const account of ["..", "a/b", ".", ".alice", "_alice", "-alice", "../alice.b", "Alice", "", null]) assert.throws(read("good", account), code("starter-pack-invalid")); + // A symlink to a good 0600 file refuses. + const real = join(f.root, "real.json"); + writeFileSync(real, JSON.stringify(pack()), { mode: 0o600 }); + symlinkSync(real, join(f.dir, "link.json")); + assert.throws(read("link"), code("starter-pack-invalid")); + // A 0644 file and a 0640 file refuse. + f.put("wide", JSON.stringify(pack()), 0o644); + assert.throws(read("wide"), code("starter-pack-invalid")); + f.put("group", JSON.stringify(pack()), 0o640); + assert.throws(read("group"), code("starter-pack-invalid")); + f.put("narrow", JSON.stringify(pack()), 0o400); + assert.ok(read("narrow")()); + // 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)); + assert.equal(lstatSync(join(f.dir, "big.json")).size, 300 * 1024); + assert.throws(read("big"), code("starter-pack-invalid")); + // A missing file, and a missing account directory, are not found. + assert.throws(read("missing"), code("starter-pack-not-found")); + assert.throws(read("good", "carol"), code("starter-pack-not-found")); + // A directory where the file should be, bad JSON and bad UTF-8 refuse invalid. + mkdirSync(join(f.dir, "dir.json")); + assert.throws(read("dir"), code("starter-pack-invalid")); + f.put("bad", `{"id": "${MARK}"`); + assert.throws(read("bad"), code("starter-pack-invalid")); + writeFileSync(join(f.dir, "utf.json"), Buffer.from([0x7b, 0xff, 0x7d]), { mode: 0o600 }); + assert.throws(read("utf"), code("starter-pack-invalid")); + // A file the pack checks refuse is invalid too. + f.put("forbidden", JSON.stringify(pack((p) => { p.definitions.coder.copyBindings = MARK; }))); + assert.throws(read("forbidden"), code("starter-pack-invalid")); + // Another user's file, and a relative configDir, refuse. + assert.throws(read("good", "alice.b", { uid: process.getuid() + 1 }), code("starter-pack-invalid")); + assert.throws(() => readSavedTemplate({ configDir: "cfg", account: "alice.b", name: "good", rolesDir: ROLES }), code("starter-pack-invalid")); +}); + +test("SP1 readSavedTemplate refuses a symlinked or group-writable templates directory", async (t) => { + const { readSavedTemplate } = await sp(); + const f = savedFixture(t, "alice.b"); + f.put("good", JSON.stringify(pack())); + const read = () => readSavedTemplate({ configDir: f.configDir, account: "alice.b", name: "good", rolesDir: ROLES }); + chmodSync(f.dir, 0o770); + assert.throws(read, code("starter-pack-invalid")); + chmodSync(f.dir, 0o700); + assert.ok(read()); + // The account directory as a symlink to a real one refuses. + const g = savedFixture(t, "other"); + g.put("good", JSON.stringify(pack())); + symlinkSync(g.dir, join(f.configDir, "templates", "linked")); + assert.throws(() => readSavedTemplate({ configDir: f.configDir, account: "linked", name: "good", rolesDir: ROLES }), code("starter-pack-invalid")); + // The templates directory as a symlink refuses. + const h = mkdtempSync(join(tmpdir(), "sp1-")); + t.after(() => rmSync(h, { recursive: true, force: true })); + mkdirSync(join(h, "cfg"), { mode: 0o700 }); + symlinkSync(join(f.configDir, "templates"), join(h, "cfg", "templates")); + assert.throws(() => readSavedTemplate({ configDir: join(h, "cfg"), account: "alice.b", name: "good", rolesDir: ROLES }), code("starter-pack-invalid")); +}); + +// ---- installedStarterPacks ---- + +// A scratch copy of the plugin and organization sources, so a core pack module can be added +// without touching the repository tree. +function tree(t) { + const root = mkdtempSync(join(tmpdir(), "sp1t-")); + t.after(() => rmSync(root, { recursive: true, force: true })); + for (const dir of ["packages/plugins/src", "packages/organization/src"]) cpSync(join(REPO, dir), join(root, dir), { recursive: true }); + const core = (name, body) => { + mkdirSync(join(root, "packages/plugins/starter-pack", name), { recursive: true }); + writeFileSync(join(root, "packages/plugins/starter-pack", name, "index.mjs"), body); + }; + const load = () => import(pathToFileURL(join(root, "packages/plugins/src/starter-pack.mjs")).href); + return { root, core, load }; +} + +const moduleText = (doc) => `export const interfaceVersion = 1;\nexport function starterPack() { return ${JSON.stringify(doc)}; }\n`; + +test("SP1 installedStarterPacks lists shipped packs with origin shipped; the repository ships none yet", async (t) => { + const { installedStarterPacks } = await sp(); + assert.deepEqual(await installedStarterPacks({ rolesDir: ROLES }), { packs: [], refused: [] }); + const f = tree(t); + f.core("coding", moduleText(pack())); + f.core("tiny", moduleText(pack((p) => { p.id = "tiny"; p.hires = []; }))); + const m = await f.load(); + const out = await m.installedStarterPacks({ rolesDir: ROLES }); + assert.deepEqual(out.refused, []); + assert.deepEqual(out.packs.map((p) => [p.id, p.origin]), [["coding", "shipped"], ["tiny", "shipped"]]); + assert.deepEqual(out.packs[0].pack, pack()); +}); + +test("SP1 a refused shipped pack fails the listing", async (t) => { + for (const body of [ + moduleText(pack((p) => { p.packs = []; })), + moduleText(pack((p) => { p.id = "other"; })), + `export const interfaceVersion = 1;\nexport function starterPack() { throw new Error(${JSON.stringify(MARK)}); }\n`, + "export const interfaceVersion = 2;\nexport function starterPack() { return {}; }\n", + "export const interfaceVersion = 1;\n", + ]) { + const f = tree(t); + f.core("coding", body); + const m = await f.load(); + await assert.rejects(m.installedStarterPacks({ rolesDir: ROLES }), code("starter-pack-invalid")); + } +}); + +function installed(t, name, body) { + const root = mkdtempSync(join(tmpdir(), "sp1i-")); + t.after(() => { + const thaw = (p) => { const st = lstatSync(p); if (st.isSymbolicLink()) return; if (st.isDirectory()) { chmodSync(p, 0o700); for (const n of readdirSync(p)) thaw(join(p, n)); } }; + thaw(root); + rmSync(root, { recursive: true, force: true }); + }); + const src = join(root, "src"), dataRoot = join(root, "data"); + mkdirSync(join(src, "starter-pack"), { recursive: true }); + mkdirSync(dataRoot); + writeFileSync(join(src, "starter-pack", "index.mjs"), body); + const manifest = { name, version: "1.0.0", kinds: ["starter-pack"], interfaceVersions: { "starter-pack": 1 }, sha256: "0".repeat(64), + source: { path: "fixture", version: "1.0.0" }, signature: null, tools: [], actions: [], services: [], roleTemplates: [] }; + writeFileSync(join(src, "plugin.json"), JSON.stringify(manifest)); + manifest.sha256 = pluginHash(src); + writeFileSync(join(src, "plugin.json"), JSON.stringify(manifest)); + installPlugin({ dataRoot, source: src, expectedHash: manifest.sha256 }); + return { root, dataRoot }; +} + +test("SP1 an installed pack lists with origin installed; a refused one is omitted and logged", async (t) => { + const { installedStarterPacks } = await sp(); + const good = installed(t, "acmepack", moduleText(pack((p) => { p.id = "acmepack"; }))); + const out = await installedStarterPacks({ dataRoot: good.dataRoot, rolesDir: ROLES }); + assert.deepEqual(out.refused, []); + assert.deepEqual(out.packs.map((p) => [p.id, p.origin]), [["acmepack", "installed"]]); + // An installed pack carrying packs, or a role without from, is omitted and logged. + for (const mutate of [(p) => { p.packs = []; }, (p) => { delete p.definitions["release-writer"].from; }, (p) => { p.hires[0].level = "root"; }]) { + const bad = installed(t, "badpack", moduleText(pack((p) => { p.id = "badpack"; mutate(p); }))); + const log = []; + const res = await installedStarterPacks({ dataRoot: bad.dataRoot, rolesDir: ROLES, log: (e) => log.push(e) }); + assert.deepEqual(res.packs, []); + assert.equal(res.refused.length, 1); + assert.deepEqual(log, res.refused); + assert.deepEqual([res.refused[0].event, res.refused[0].kind, res.refused[0].name, res.refused[0].code], + ["starter-pack-refused", "starter-pack", "badpack", "starter-pack-invalid"]); + } + // An installed module that fails to load is omitted and logged too. + const broken = installed(t, "brokenpack", "export const interfaceVersion = 1;\nexport const starterPack = 3;\n"); + const log = []; + const res = await installedStarterPacks({ dataRoot: broken.dataRoot, rolesDir: ROLES, log: (e) => log.push(e) }); + assert.deepEqual(res.packs, []); + assert.deepEqual(log.map((e) => [e.name, e.code]), [["brokenpack", "missing-export"]]); +}); -- 2.54.0 From 2a34603a2eab4749fb4b6f2fb1c430bbac19de5b Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 12 Nov 2026 15:21:23 -0600 Subject: [PATCH 2/4] test(plugins): loader maps list the starter-pack kind (#1873 SP1) Adding a kind adds its key to every loaded() map. Outside the SP1 owner list; kept as its own commit so it can be dropped or moved. --- packages/plugins/tests/loader.test.mjs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/plugins/tests/loader.test.mjs b/packages/plugins/tests/loader.test.mjs index 60d71d861..62a0c12ba 100644 --- a/packages/plugins/tests/loader.test.mjs +++ b/packages/plugins/tests/loader.test.mjs @@ -12,7 +12,7 @@ import { validateBusinessDocument } from "../../organization/src/organization-co const GOOD = { "usage-source": ["meter"], "limit-check": ["cap"] }; // What loaded() reports for GOOD: search-source is a kind too, switched off here. -const GOOD_LOADED = { ...GOOD, "search-source": [], channel: [], harness: [], usage: [], "model-provider": [], "system-type": [], theme: [], "task-tracker": [] }; +const GOOD_LOADED = { ...GOOD, "search-source": [], channel: [], harness: [], usage: [], "model-provider": [], "system-type": [], "starter-pack": [], theme: [], "task-tracker": [] }; // A tree with one good module of each kind, already loaded. async function loadedTree(t, options) { @@ -56,7 +56,7 @@ test("REQ-PLUG-1 fixture modules of each kind switch on from a test organization const loader = await s.loader(); const result = await loader.load(business.plugins); assert.deepEqual(result.refused, []); - assert.deepEqual(result.loaded, { ...doc.plugins, "search-source": [], channel: [], harness: [], usage: [], "model-provider": [], "system-type": [], theme: [], "task-tracker": [] }); + assert.deepEqual(result.loaded, { ...doc.plugins, "search-source": [], channel: [], harness: [], usage: [], "model-provider": [], "system-type": [], "starter-pack": [], theme: [], "task-tracker": [] }); assert.equal(loader.modules("usage-source")[0].readTurn(), "fixture-usage"); assert.equal(loader.modules("limit-check")[0].mayLaunch(), "fixture-limit"); assert.equal(loader.modules("limit-check")[0].mustStop(), false); @@ -171,7 +171,7 @@ test("REQ-PLUG-1 a path that becomes unreadable after the lstat checks refuses a assert.match(result.refused[0].message, /packages\/plugins can't be read \(EACCES\)/); assert.deepEqual(result.refused[0].kept, ["meter"]); assert.deepEqual(s.loader.logged.slice(before), result.refused); - assert.deepEqual(s.loader.loaded(), { "usage-source": ["meter"], "limit-check": [], "search-source": [], channel: [], harness: [], usage: [], "model-provider": [], "system-type": [], theme: [], "task-tracker": [] }); + assert.deepEqual(s.loader.loaded(), { "usage-source": ["meter"], "limit-check": [], "search-source": [], channel: [], harness: [], usage: [], "model-provider": [], "system-type": [], "starter-pack": [], theme: [], "task-tracker": [] }); } finally { if (lock) chmodSync(lock, 0o755); } @@ -233,7 +233,7 @@ test("REQ-PLUG-1 a kind refused on its first load has nothing switched on, and t const loader = await s.loader(); // meter loads, absent refuses: the whole kind stays off, not half on. const result = await loader.load({ "usage-source": ["meter", "absent"], "limit-check": ["cap"] }); - assert.deepEqual(result.loaded, { "usage-source": [], "limit-check": ["cap"], "search-source": [], channel: [], harness: [], usage: [], "model-provider": [], "system-type": [], theme: [], "task-tracker": [] }); + assert.deepEqual(result.loaded, { "usage-source": [], "limit-check": ["cap"], "search-source": [], channel: [], harness: [], usage: [], "model-provider": [], "system-type": [], "starter-pack": [], theme: [], "task-tracker": [] }); assert.deepEqual(loader.loaded(), result.loaded); assert.deepEqual(loader.modules("usage-source"), []); assert.equal(loader.logged.length, 1); @@ -261,7 +261,7 @@ test("REQ-PLUG-1 the setting's shape: unknown kinds refuse alone, a bad whole se await refusesKeepingOld(s, ["meter", "meter"], "bad-setting", "meter", /twice/); // An absent key switches the kind off. result = await s.loader.load({ "limit-check": ["cap"] }); - assert.deepEqual(result.loaded, { "usage-source": [], "limit-check": ["cap"], "search-source": [], channel: [], harness: [], usage: [], "model-provider": [], "system-type": [], theme: [], "task-tracker": [] }); + assert.deepEqual(result.loaded, { "usage-source": [], "limit-check": ["cap"], "search-source": [], channel: [], harness: [], usage: [], "model-provider": [], "system-type": [], "starter-pack": [], theme: [], "task-tracker": [] }); }); test("REQ-PLUG-1 loads run one at a time, so the last requested setting wins", async t => { -- 2.54.0 From f1625050561c802b5e00d9b84d4c6bdb29dd30f5 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 12 Nov 2026 15:31:41 -0600 Subject: [PATCH 3/4] fix(plugins): review round for the starter-pack reader (#1873 SP1) readSavedTemplate refuses a hard-linked file and an account directory open to group or other; an open error other than ELOOP reads "can't be read", not "symlink"; a file that grows during the read has its own message. installedStarterPacks refuses a promise from starterPack() and swallows its later rejection. Tests pin each refusal's message, so a check that a neighbour shadows is still covered. --- packages/plugins/src/starter-pack.mjs | 25 +++++++--- packages/plugins/tests/starter-pack.test.mjs | 48 ++++++++++++++++---- 2 files changed, 56 insertions(+), 17 deletions(-) diff --git a/packages/plugins/src/starter-pack.mjs b/packages/plugins/src/starter-pack.mjs index 7759ed0ee..02fa7584e 100644 --- a/packages/plugins/src/starter-pack.mjs +++ b/packages/plugins/src/starter-pack.mjs @@ -376,6 +376,12 @@ export async function installedStarterPacks({ dataRoot = null, rolesDir = ROLES_ } catch { 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 */ } + 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 }) }); } catch (error) { if (!(error instanceof PluginError)) throw error; @@ -398,20 +404,23 @@ function lstatOrNull(path) { } // A directory on the way to a saved template: a real directory owned by the instance's user that -// no group or other user can write. -function checkDirectory(path, uid) { +// no group or other user can write. `private` also refuses group and other read and list, for the +// account's own directory, which the writer creates 0700. +function checkDirectory(path, uid, { private: closed = false } = {}) { const st = lstatOrNull(path); if (st === null) fail("starter-pack-not-found", "no saved template has that name"); if (st.isSymbolicLink() || !st.isDirectory()) fail("starter-pack-invalid", "the saved templates directory must be a real directory"); if (st.uid !== uid) fail("starter-pack-invalid", "the saved templates directory must belong to the instance's user"); if ((st.mode & 0o022) !== 0) fail("starter-pack-invalid", "the saved templates directory must not be writable by group or others"); + if (closed && (st.mode & 0o077) !== 0) fail("starter-pack-invalid", "the account's saved templates directory must be mode 0700 or narrower"); } // Reads /templates//.json as data and checks it with origin `saved`. The // account is the calling owner's id (s.human), checked against ID_PATTERN; the name is checked // against NAME. Both are checked before any path is built, so neither ".." nor "/" reaches the file -// system. The file must be a regular file, not a symlink, owned by the instance's user, mode 0600 -// or narrower, at most 256 KiB, checked before the read. Returns { name, origin: "saved", pack }. +// system. The account directory must be mode 0700 or narrower. The file must be a regular file, not +// a symlink or a hard link, owned by the instance's user, mode 0600 or narrower, at most 256 KiB, +// checked before the read. Returns { name, origin: "saved", pack }. export function readSavedTemplate({ configDir, account, name, rolesDir = ROLES_DIR, uid = process.getuid() } = {}) { if (typeof account !== "string" || !ID_PATTERN.test(account)) fail("starter-pack-invalid", `the saved template account must match ${ID_PATTERN}`); if (typeof name !== "string" || !NAME.test(name)) fail("starter-pack-invalid", `the saved template name must match ${NAME}`); @@ -419,7 +428,7 @@ export function readSavedTemplate({ configDir, account, name, rolesDir = ROLES_D const templates = join(configDir, "templates"); const accountDir = join(templates, account); checkDirectory(templates, uid); - checkDirectory(accountDir, uid); + checkDirectory(accountDir, uid, { private: true }); const file = join(accountDir, `${name}.json`); const before = lstatOrNull(file); if (before === null) fail("starter-pack-not-found", "no saved template has that name", name); @@ -429,13 +438,15 @@ export function readSavedTemplate({ configDir, account, name, rolesDir = ROLES_D fd = openSync(file, constants.O_RDONLY | constants.O_NOFOLLOW | constants.O_NONBLOCK); } catch (error) { if (error.code === "ENOENT") fail("starter-pack-not-found", "no saved template has that name", name); - fail("starter-pack-invalid", "the saved template must be a regular file, not a symlink", name); + if (error.code === "ELOOP") fail("starter-pack-invalid", "the saved template must be a regular file, not a symlink", name); + fail("starter-pack-invalid", "the saved template can't be read", name); } let bytes; try { 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 !== 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); // Read at most one byte past the limit, so a file that grew after the fstat still refuses. @@ -445,7 +456,7 @@ export function readSavedTemplate({ configDir, account, name, rolesDir = ROLES_D const n = readSync(fd, buffer, length, buffer.length - length, null); if (n === 0) break; length += n; - if (length > SAVED_MAX_BYTES) fail("starter-pack-invalid", `the saved template must be at most ${SAVED_MAX_BYTES} bytes`, name); + if (length > SAVED_MAX_BYTES) fail("starter-pack-invalid", `the saved template grew past ${SAVED_MAX_BYTES} bytes while it was read`, name); } bytes = buffer.subarray(0, length); } finally { diff --git a/packages/plugins/tests/starter-pack.test.mjs b/packages/plugins/tests/starter-pack.test.mjs index 0a4a9958b..87f92e2a1 100644 --- a/packages/plugins/tests/starter-pack.test.mjs +++ b/packages/plugins/tests/starter-pack.test.mjs @@ -3,7 +3,7 @@ // without it (the red-before run). import test from "node:test"; import assert from "node:assert/strict"; -import { chmodSync, cpSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { chmodSync, cpSync, linkSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { dirname, join, resolve } from "node:path"; import { tmpdir } from "node:os"; import { fileURLToPath, pathToFileURL } from "node:url"; @@ -21,9 +21,10 @@ const MARK = "MARK-sp1-not-echoed-7c1f"; let loaded; const sp = async () => (loaded ??= await import("../src/starter-pack.mjs")); -const code = (expected) => (e) => { +const code = (expected, message) => (e) => { assert.equal(e?.name, "PluginError", e?.stack); assert.equal(e.code, expected, e.message); + if (message) assert.match(e.message, message); assert.ok(!e.message.includes(MARK), e.message); assert.ok(!e.message.includes(REPO), e.message); return true; @@ -416,19 +417,23 @@ test("SP1 readSavedTemplate refuses .., a name or account holding /, a symlink, const real = join(f.root, "real.json"); writeFileSync(real, JSON.stringify(pack()), { mode: 0o600 }); symlinkSync(real, join(f.dir, "link.json")); - assert.throws(read("link"), code("starter-pack-invalid")); + assert.throws(read("link"), code("starter-pack-invalid", /regular file, not a symlink/)); // A 0644 file and a 0640 file refuse. f.put("wide", JSON.stringify(pack()), 0o644); - assert.throws(read("wide"), code("starter-pack-invalid")); + assert.throws(read("wide"), code("starter-pack-invalid", /mode 0600 or narrower/)); f.put("group", JSON.stringify(pack()), 0o640); - assert.throws(read("group"), code("starter-pack-invalid")); + assert.throws(read("group"), code("starter-pack-invalid", /mode 0600 or narrower/)); + // A hard link to a file outside the account's directory refuses. + const solo = f.put("solo", JSON.stringify(pack())); + linkSync(solo, join(f.root, "alias.json")); + assert.throws(read("solo"), code("starter-pack-invalid", /hard link/)); f.put("narrow", JSON.stringify(pack()), 0o400); assert.ok(read("narrow")()); // 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)); assert.equal(lstatSync(join(f.dir, "big.json")).size, 300 * 1024); - assert.throws(read("big"), code("starter-pack-invalid")); + assert.throws(read("big"), code("starter-pack-invalid", /must be at most 262144 bytes/)); // A missing file, and a missing account directory, are not found. assert.throws(read("missing"), code("starter-pack-not-found")); assert.throws(read("good", "carol"), code("starter-pack-not-found")); @@ -443,7 +448,7 @@ test("SP1 readSavedTemplate refuses .., a name or account holding /, a symlink, f.put("forbidden", JSON.stringify(pack((p) => { p.definitions.coder.copyBindings = MARK; }))); assert.throws(read("forbidden"), code("starter-pack-invalid")); // Another user's file, and a relative configDir, refuse. - assert.throws(read("good", "alice.b", { uid: process.getuid() + 1 }), code("starter-pack-invalid")); + assert.throws(read("good", "alice.b", { uid: process.getuid() + 1 }), code("starter-pack-invalid", /directory must belong to the instance's user/)); assert.throws(() => readSavedTemplate({ configDir: "cfg", account: "alice.b", name: "good", rolesDir: ROLES }), code("starter-pack-invalid")); }); @@ -453,20 +458,31 @@ test("SP1 readSavedTemplate refuses a symlinked or group-writable templates dire f.put("good", JSON.stringify(pack())); const read = () => readSavedTemplate({ configDir: f.configDir, account: "alice.b", name: "good", rolesDir: ROLES }); chmodSync(f.dir, 0o770); - assert.throws(read, code("starter-pack-invalid")); + assert.throws(read, code("starter-pack-invalid", /not be writable by group or others/)); + // The account's directory is 0700 or narrower: group or other read or list refuses too. + for (const mode of [0o750, 0o705, 0o755]) { + chmodSync(f.dir, mode); + assert.throws(read, code("starter-pack-invalid", /mode 0700 or narrower/)); + } chmodSync(f.dir, 0o700); assert.ok(read()); + // The templates directory itself may be listed by others; it only must not be writable. + chmodSync(join(f.configDir, "templates"), 0o755); + assert.ok(read()); + chmodSync(join(f.configDir, "templates"), 0o775); + assert.throws(read, code("starter-pack-invalid", /not be writable by group or others/)); + chmodSync(join(f.configDir, "templates"), 0o700); // The account directory as a symlink to a real one refuses. const g = savedFixture(t, "other"); g.put("good", JSON.stringify(pack())); symlinkSync(g.dir, join(f.configDir, "templates", "linked")); - assert.throws(() => readSavedTemplate({ configDir: f.configDir, account: "linked", name: "good", rolesDir: ROLES }), code("starter-pack-invalid")); + assert.throws(() => readSavedTemplate({ configDir: f.configDir, account: "linked", name: "good", rolesDir: ROLES }), code("starter-pack-invalid", /must be a real directory/)); // The templates directory as a symlink refuses. const h = mkdtempSync(join(tmpdir(), "sp1-")); t.after(() => rmSync(h, { recursive: true, force: true })); mkdirSync(join(h, "cfg"), { mode: 0o700 }); symlinkSync(join(f.configDir, "templates"), join(h, "cfg", "templates")); - assert.throws(() => readSavedTemplate({ configDir: join(h, "cfg"), account: "alice.b", name: "good", rolesDir: ROLES }), code("starter-pack-invalid")); + assert.throws(() => readSavedTemplate({ configDir: join(h, "cfg"), account: "alice.b", name: "good", rolesDir: ROLES }), code("starter-pack-invalid", /must be a real directory/)); }); // ---- installedStarterPacks ---- @@ -558,4 +574,16 @@ test("SP1 an installed pack lists with origin installed; a refused one is omitte const res = await installedStarterPacks({ dataRoot: broken.dataRoot, rolesDir: ROLES, log: (e) => log.push(e) }); assert.deepEqual(res.packs, []); assert.deepEqual(log.map((e) => [e.name, e.code]), [["brokenpack", "missing-export"]]); + // An async starterPack is refused, and its later rejection never reaches the process. + const unhandled = []; + const onUnhandled = (reason) => unhandled.push(reason); + process.on("unhandledRejection", onUnhandled); + t.after(() => process.off("unhandledRejection", onUnhandled)); + const later = installed(t, "asyncpack", "export const interfaceVersion = 1;\nexport async function starterPack() { await new Promise((r) => setTimeout(r, 20)); throw new Error('late'); }\n"); + const res2 = await installedStarterPacks({ dataRoot: later.dataRoot, rolesDir: ROLES, log() {} }); + assert.deepEqual(res2.packs, []); + assert.deepEqual(res2.refused.map((e) => [e.name, e.code]), [["asyncpack", "starter-pack-invalid"]]); + assert.match(res2.refused[0].message, /not a promise/); + await new Promise((r) => setTimeout(r, 100)); + assert.deepEqual(unhandled, []); }); -- 2.54.0 From 755ba3fa8bb2f2733608dd44e0f61165a4d4c068 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 12 Nov 2026 15:37:50 -0600 Subject: [PATCH 4/4] 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, []); }); -- 2.54.0