From e4f088e90b31c2fe62d7a83f4b6b81191c01c296 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Wed, 23 Sep 2026 01:26:52 -0500 Subject: [PATCH] fix(modules): a site runs one module; the installer refuses a second A site is one game, and the module contract already has singletons that assume it. registerTeamProvider holds one value per deployment, and a second module registering one fails that module's whole load. The loader scans alphabetically, so installing module-rust (which gains a Team provider in its phase 9) beside module-uo would have taken uo down, not rust. install() now refuses, with 409 and before the artifact is downloaded, any install whose id differs from a module already on the volume. An upgrade of the installed module is still accepted; to change game, remove the module first. Both install surfaces share this path, so a MODULES declaration naming two modules installs the first and reports the second as refused without failing the boot. "Installed" means what the loader would scan: a directory named with a module id that holds a module.json. An install's scratch directory and a swap's aside copy do not count. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01E14m6SuuY6i1vASFeGDBeY --- server/src/modules/declared.js | 5 ++ server/src/modules/install.js | 42 +++++++++++ server/src/router/v1/admin/modules.router.js | 3 +- server/swagger/swagger-output.json | 12 +++- server/test/moduleInstall.test.js | 73 ++++++++++++++++++++ 5 files changed, 133 insertions(+), 2 deletions(-) diff --git a/server/src/modules/declared.js b/server/src/modules/declared.js index 09bca4b..0fe4104 100644 --- a/server/src/modules/declared.js +++ b/server/src/modules/declared.js @@ -17,6 +17,11 @@ // through modules/install.js, the same fetch-verify-unpack path the admin panel // uses, under the same host allowlist. // +// **A site runs one module** (org lead, 2026-09-23), and install.js enforces it +// for this path too: a declaration naming a second module installs the first, +// and the second is refused — logged and kept for the admin screen like any +// other failed entry, never fatal to the boot. +// // Three things this file deliberately does not do: // // - **It does not decide whether a module RUNS.** Resolution owns what is on diff --git a/server/src/modules/install.js b/server/src/modules/install.js index 100e60a..8c22d2f 100644 --- a/server/src/modules/install.js +++ b/server/src/modules/install.js @@ -290,6 +290,28 @@ function isInstalled(id) { } } +/** + * Every module on the volume, by id — the directories the loader would scan. + * + * The loader's own rule, restated: a directory whose name is a module id and + * which holds a `module.json`. That excludes an install's scratch directory + * (`.install-*`) and a swap's aside copy (`.replaced-*`), neither of which + * is a module and both of which can briefly exist beside one. + */ +function installedIds() { + let entries + try { + entries = fs.readdirSync(loader.dir(), { withFileTypes: true }) + } catch { + return [] // no modules directory is the normal case for a bare core + } + return entries + .filter((e) => e.isDirectory() && ID.test(e.name)) + .filter((e) => fs.existsSync(path.join(loader.dir(), e.name, 'module.json'))) + .map((e) => e.name) + .sort() +} + /** * The absolute path of a module's `purge.sql`, or null. * @@ -348,6 +370,25 @@ async function install({ url, hosts, expect = null, fetchImpl = fetch }) { ) } + // One module per site (org lead, 2026-09-23). A site is one game, and the + // contract has singletons that assume it: `registerTeamProvider` holds ONE + // value per deployment, and a second module registering one fails its whole + // load — with modules loaded alphabetically, installing `rust` beside `uo` + // would have taken `uo` down, not `rust`. So an install is an UPGRADE of the + // module already here, or it is refused before a byte is downloaded. + // + // Refused here rather than in the admin controller so the declared module set + // (modules/declared.js) gets the same answer: an environment naming two + // modules installs the first and is told why the second was not. + const others = installedIds().filter((id) => id !== manifest.id) + if (others.length) { + throw new InstallError( + `this site already runs the module "${others.join('", "')}", and a site runs one module. ` + + `Upgrade it with its own release, or remove it before installing "${manifest.id}".`, + { status: 409 }, + ) + } + const target = moduleDir(manifest.id) const scratch = await fsp.mkdtemp(path.join(loader.dir(), `.install-${manifest.id}-`)) const tarball = path.join(scratch, 'bundle.tar.gz') @@ -444,6 +485,7 @@ module.exports = { removeDir, moduleDir, isInstalled, + installedIds, purgeFile, MAX_MANIFEST_BYTES, MAX_ARTIFACT_BYTES, diff --git a/server/src/router/v1/admin/modules.router.js b/server/src/router/v1/admin/modules.router.js index a5535d6..bcedc1c 100644 --- a/server/src/router/v1/admin/modules.router.js +++ b/server/src/router/v1/admin/modules.router.js @@ -41,11 +41,12 @@ modulesRouter.post( '/', // #swagger.tags = ['Admin · Modules'] // #swagger.summary = 'Install or upgrade a module from a release install-manifest URL' - // #swagger.description = 'Downloads the artifact the manifest names, verifies its sha256, inspects the archive in full and unpacks it onto the modules volume. The module mounts on the next restart.' + // #swagger.description = 'Downloads the artifact the manifest names, verifies its sha256, inspects the archive in full and unpacks it onto the modules volume. The module mounts on the next restart. A site runs ONE module: installing a module other than the one already on the volume is refused with 409 before anything is downloaded, and only an upgrade of the installed module is accepted.' // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] /* #swagger.requestBody = { required: true, content: { "application/json": { schema: { type: "object", required: ["url"], properties: { url: { type: "string", description: "https URL of the release install manifest, on an allowed host" } } } } } } */ /* #swagger.responses[201] = { description: 'Installed — restart to mount it', content: { "application/json": { schema: { type: "object", additionalProperties: true } } } } */ /* #swagger.responses[400] = { description: 'The URL, the manifest, the hash or the archive was refused', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + /* #swagger.responses[409] = { description: 'A different module is already installed; a site runs one module', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ /* #swagger.responses[502] = { description: 'The source host could not be reached or answered badly', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ adminOnly, body('url').isString().trim().isLength({ min: 1, max: 2048 }), diff --git a/server/swagger/swagger-output.json b/server/swagger/swagger-output.json index 9a744c3..ed6cad0 100644 --- a/server/swagger/swagger-output.json +++ b/server/swagger/swagger-output.json @@ -7211,7 +7211,7 @@ "Admin · Modules" ], "summary": "Install or upgrade a module from a release install-manifest URL", - "description": "Downloads the artifact the manifest names, verifies its sha256, inspects the archive in full and unpacks it onto the modules volume. The module mounts on the next restart.", + "description": "Downloads the artifact the manifest names, verifies its sha256, inspects the archive in full and unpacks it onto the modules volume. The module mounts on the next restart. A site runs ONE module: installing a module other than the one already on the volume is refused with 409 before anything is downloaded, and only an upgrade of the installed module is accepted.", "responses": { "201": { "description": "Installed — restart to mount it", @@ -7234,6 +7234,16 @@ } } }, + "409": { + "description": "A different module is already installed; a site runs one module", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, "500": { "description": "Internal Server Error" }, diff --git a/server/test/moduleInstall.test.js b/server/test/moduleInstall.test.js index ef3a9d2..56d5bba 100644 --- a/server/test/moduleInstall.test.js +++ b/server/test/moduleInstall.test.js @@ -395,6 +395,79 @@ test('a failed upgrade leaves the previous version in place', async () => { assert.deepEqual(fs.readdirSync(tmpRoot), ['uo']) }) +// ── One module per site ──────────────────────────────────────────────────── + +/** A second, different module's manifest and artifact, served beside the first. */ +function otherModuleRoutes(id = 'rust') { + const tarball = bundle({ id }) + const artifact = `https://releases.example.com/mod/${id}-1.0.0.tar.gz` + const url = `https://releases.example.com/mod/${id}-1.0.0.json` + return { + url, + routes: { + [url]: manifestFor(tarball, { id, name: id, artifact: `${id}-1.0.0.tar.gz`, url: artifact }), + [artifact]: tarball, + }, + artifact, + } +} + +test('a second, different module is refused before anything is downloaded', async () => { + const first = goodRoutes() + await install.install({ url: MANIFEST_URL, hosts: HOSTS, fetchImpl: fakeFetch(first.routes) }) + + const other = otherModuleRoutes('rust') + const fetchImpl = fakeFetch(other.routes) + await assert.rejects( + () => install.install({ url: other.url, hosts: HOSTS, fetchImpl }), + (err) => { + assert.equal(err.name, 'InstallError') + // 409: nothing is wrong with the URL; the SITE is not in a state to take it. + assert.equal(err.status, 409) + assert.match(err.message, /already runs the module "uo"/) + assert.match(err.message, /remove it before installing "rust"/) + return true + }, + ) + + // Refused on the manifest alone: the artifact was never fetched, and the + // volume holds exactly what it held before. + assert.ok(!fetchImpl.seen.includes(other.artifact), 'the artifact was not downloaded') + assert.deepEqual(fs.readdirSync(tmpRoot), ['uo']) +}) + +test('the same module is still an upgrade, and removing it frees the site for another', async () => { + const first = goodRoutes() + await install.install({ url: MANIFEST_URL, hosts: HOSTS, fetchImpl: fakeFetch(first.routes) }) + + // An upgrade of what is installed is exactly what the rule allows. + const second = goodRoutes({ version: '2.0.0', manifest: { version: '2.0.0' } }) + second.routes[MANIFEST_URL] = manifestFor(second.tarball, { version: '2.0.0' }) + const upgraded = await install.install({ url: MANIFEST_URL, hosts: HOSTS, fetchImpl: fakeFetch(second.routes) }) + assert.equal(upgraded.replaced, true) + + // And once it is gone, the site takes a different one. + await install.removeDir('uo') + const other = otherModuleRoutes('rust') + const result = await install.install({ url: other.url, hosts: HOSTS, fetchImpl: fakeFetch(other.routes) }) + assert.equal(result.id, 'rust') + assert.deepEqual(install.installedIds(), ['rust']) +}) + +test('what counts as installed is what the loader would scan', () => { + // A real module, an install's scratch directory, a swap's aside copy and a + // directory with no module.json. Only the first is a module. + fs.mkdirSync(path.join(tmpRoot, 'uo')) + fs.writeFileSync(path.join(tmpRoot, 'uo', 'module.json'), '{}') + fs.mkdirSync(path.join(tmpRoot, '.install-rust-abc')) + fs.writeFileSync(path.join(tmpRoot, '.install-rust-abc', 'module.json'), '{}') + fs.mkdirSync(path.join(tmpRoot, 'uo.replaced-123')) + fs.writeFileSync(path.join(tmpRoot, 'uo.replaced-123', 'module.json'), '{}') + fs.mkdirSync(path.join(tmpRoot, 'notes')) + + assert.deepEqual(install.installedIds(), ['uo']) +}) + // ── The volume ───────────────────────────────────────────────────────────── test('moduleDir refuses an id that is not one', () => {