From b30e82cde2aeb3360bf35420ad09d5c18ade0498 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Wed, 12 Aug 2026 03:09:45 -0500 Subject: [PATCH] feat(modules): install, uninstall, purge and restart (phase 4, slice 1) The consumer half of a release module-uo's CI has been publishing since phase 3 closed. Before this, core had the installed_modules provenance columns and no code that could ever fill them: nothing fetched, verified, unpacked, removed or purged anything, and there was no admin route at all. Adds modules/archive.js, modules/install.js, schema.runPurge(), lifecycle.stop(), loader.stopHook(), and /api/v1/admin/modules with eight routes. 797 server tests (+76), manifest 158 -> 166 + 2 internal, OpenAPI gains 8 operations and loses nothing. Reject, never sanitise ---------------------- The download is the easy part: an https-only allowlist re-checked on every redirect hop, a declared sha256 compared against the bytes that arrived, and a byte cap. Unpacking is where the archive chooses the filenames, and core writes into a directory bind-mounted from the host, so an escape is not confined to the container. archive.js inspects the whole archive before a byte is unpacked and refuses absolute and drive-absolute paths, `..` segments, NUL bytes, backslashes, anything that is not a regular file or a directory, more than one top-level entry, and anything over the entry or byte caps. Refusing symlinks and hardlinks outright is what keeps this off the majority of node-tar's published advisories rather than depending on the library to contain them. That two-pass shape is load-bearing, and it was measured rather than assumed: extracting an archive whose fourth member escapes upward throws under node-tar 7.5.22 -- and leaves the first three members on disk. The loader scans that directory at require time on the next boot, so a half-unpacked module is a module. Everything therefore happens in a scratch directory that is removed on any failure, and the move into place is the last step. `tar` is pinned to ^7.5.22 rather than the ^6 that installs by default: 6.x is flagged critical, and reading the advisory list is what the file's header now says out loud -- almost all of it is hardlink or symlink traversal and PAX header interpretation differentials, which is exactly this feature's threat model. Two things the plan had wrong ----------------------------- The bundle's top-level directory is `module-uo-`, not the module id -- so "the top-level name must equal the id" was checked against nothing real. The extractor strips that level instead, because its name belongs to whoever published the bundle and the directory it lands in is core's. What is checked instead is the unpacked module.json: a manifest promising `uo` and delivering something else is refused rather than installed under the name it promised. And purge cannot be a follow-up action (decision 5): purge.sql lives inside the directory uninstall deletes. It is offered in the uninstall flow and as a standalone action on a still-installed module, and the standalone one refuses unless the module is already disabled -- dropping tables under something that is still serving leaves it answering out of a world that no longer exists. Disable now means stopped ------------------------- lifecycle.stop() dispatches that one module's onShutdown before flipping the guard, so a module an operator switches off actually releases its sockets and closes its streams instead of merely becoming unreachable. The hook runs first and the state moves after it, because while onShutdown runs the module is still `started` and that is the only state in which its routes and the world it is tearing down agree. A hook that throws does not stop the disable -- the opposite of the boot path's rule, and deliberately. Enable is not its mirror and there is no start(id) beside it. There is no onBoot re-dispatch and the hooks were never promised re-entrant, so enable moves the row and the restart route starts it. A test pins that enable does not touch the loader, because "fixing" it is a one-line change that would put a module with closed sockets back on the nav. Restart raises SIGTERM against its own process rather than calling the shutdown path directly, so server.js's handler stays the one graceful-shutdown path and this route cannot drift from it. The allowlist bootstraps from MODULE_SOURCE_HOSTS into a settings row and is admin-managed after that (decision 6); seedDefault is INSERT IGNORE, so changing the variable on an existing deployment is a no-op by design. An empty list forbids every install rather than allowing every host -- the safe direction for a value someone might blank by accident. Verified against the real v0.3.0 release ---------------------------------------- Not a fixture: fetched the published install manifest over the real Gitea host and its redirect chain, verified the sha256, inspected and unpacked the 252,517-byte artifact to 82 files, and then booted core against the result -- the module registered its five mounts, seven streams and eight capabilities and resolved its client chunk, with no scratch directory left behind. Two defects this slice's own tooling caught, both of which had already been written down as classes: - the controller destructured runPurge at require time, capturing the function rather than the module, which made the one dependency whose ORDER matters the one that could not be substituted; - two swagger annotations carried an apostrophe inside a quoted string, dropped silently by swagger-autogen before slice 5 taught it to fail loudly. Co-Authored-By: Claude --- server/.env.example | 16 + server/db/seed.js | 11 + server/package-lock.json | 68 +++ server/package.json | 1 + server/routes.guards.json | 84 +++ server/routes.manifest.json | 32 ++ server/src/modules/archive.js | 221 ++++++++ server/src/modules/install.js | 421 +++++++++++++++ server/src/modules/lifecycle.js | 65 ++- server/src/modules/loader.js | 26 + server/src/modules/schema.js | 46 +- server/src/router/v1/admin/index.js | 6 + .../src/router/v1/admin/modules.controller.js | 379 ++++++++++++++ server/src/router/v1/admin/modules.router.js | 143 ++++++ server/swagger/swagger-output.json | 484 ++++++++++++++++++ server/test/adminModules.test.js | 463 +++++++++++++++++ server/test/moduleArchive.test.js | 252 +++++++++ server/test/moduleInstall.test.js | 448 ++++++++++++++++ server/test/moduleLifecycle.test.js | 153 ++++++ server/test/moduleSchema.test.js | 65 ++- 20 files changed, 3381 insertions(+), 3 deletions(-) create mode 100644 server/src/modules/archive.js create mode 100644 server/src/modules/install.js create mode 100644 server/src/router/v1/admin/modules.controller.js create mode 100644 server/src/router/v1/admin/modules.router.js create mode 100644 server/test/adminModules.test.js create mode 100644 server/test/moduleArchive.test.js create mode 100644 server/test/moduleInstall.test.js diff --git a/server/.env.example b/server/.env.example index 2a1e208..d78b055 100644 --- a/server/.env.example +++ b/server/.env.example @@ -129,3 +129,19 @@ ANNOUNCE_POLL_MS=15000 # NTFY_PUBLIC_URL=https://ntfy.example.com # NTFY_ALLOWED_ORIGINS=https://ntfy.example.com # NTFY_PUBLISH_TOKEN= + +# Modules (MODULE_SYSTEM.md §2.5) — where installable modules live, and where +# they may be installed from. +# MODULES_DIR Directory the loader scans at require time. Defaults to +# /modules; docker-compose.yml sets it to /app/modules, +# which is the bind mount that makes it meaningful. +# MODULE_SOURCE_HOSTS BOOTSTRAP ONLY. Comma-separated hostnames the admin panel +# may install a module from, seeded into the `module_source_hosts` +# setting the first time the site boots without one. From then +# on the SETTING is authoritative and is edited in +# Admin → Modules — changing this variable on an existing +# deployment does nothing, deliberately, so a redeploy cannot +# silently undo an operator's choice. Installs are https-only +# and an empty list forbids all of them. +# MODULES_DIR=/app/modules +# MODULE_SOURCE_HOSTS=gitea.whitlocktech.com diff --git a/server/db/seed.js b/server/db/seed.js index 1a22ce9..4f8201a 100644 --- a/server/db/seed.js +++ b/server/db/seed.js @@ -23,6 +23,17 @@ const DEFAULT_SETTINGS = { site_title: brand.name, // Android App Links opt-in — off until an admin enables it (docs/android/APP_LINKS.md). mobile_app_links_enabled: 'false', + // Hosts a module may be installed from (MODULE_SYSTEM.md §2.7.2 decision 6). + // + // The environment BOOTSTRAPS this and does not own it: seedDefault is an + // INSERT IGNORE, so the variable supplies a sane default on a fresh install + // and never reaches back in to overwrite what an admin later chose in + // Admin → Modules. Changing MODULE_SOURCE_HOSTS on an existing deployment is + // therefore a no-op, which is the intended behaviour and not an oversight. + // + // An empty stored value forbids every install rather than allowing every host + // — the safe direction for a setting someone might blank by accident. + module_source_hosts: process.env.MODULE_SOURCE_HOSTS || 'gitea.whitlocktech.com', } // Starter wiki sections (editable later via the admin panel). diff --git a/server/package-lock.json b/server/package-lock.json index b2a1db3..9430645 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -27,6 +27,7 @@ "sanitize-html": "^2.17.5", "speakeasy": "^2.0.0", "swagger-ui-express": "^5.0.1", + "tar": "^7.5.22", "ws": "^8.21.0" }, "devDependencies": { @@ -34,6 +35,18 @@ "swagger-autogen": "^2.23.7" } }, + "node_modules/@isaacs/fs-minipass": { + "version": "4.0.1", + "resolved": "https://registry.npmjs.org/@isaacs/fs-minipass/-/fs-minipass-4.0.1.tgz", + "integrity": "sha512-wgm9Ehl2jpeqP3zw/7mo3kRHFp5MEDhqAdwy1fTGkHAwnkGOVsgpvQhL8B5n1qlb01jV3n/bI0ZfZp5lWA1k4w==", + "license": "ISC", + "dependencies": { + "minipass": "^7.0.4" + }, + "engines": { + "node": ">=18.0.0" + } + }, "node_modules/@scarf/scarf": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/@scarf/scarf/-/scarf-1.4.0.tgz", @@ -330,6 +343,15 @@ "fsevents": "~2.3.2" } }, + "node_modules/chownr": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/chownr/-/chownr-3.0.0.tgz", + "integrity": "sha512-+IxzY9BZOQd/XuYPRmrvEVjF/nqj5kgT4kEq7VofrDoM1MxoRjEWkrCC3EtLi59TVawxTAn+orJwFQcrqEN1+g==", + "license": "BlueOak-1.0.0", + "engines": { + "node": ">=18" + } + }, "node_modules/cliui": { "version": "6.0.0", "resolved": "https://registry.npmjs.org/cliui/-/cliui-6.0.0.tgz", @@ -1488,6 +1510,27 @@ "url": "https://github.com/sponsors/isaacs" } }, + "node_modules/minipass": { + "version": "7.1.3", + "resolved": "https://registry.npmjs.org/minipass/-/minipass-7.1.3.tgz", + "integrity": "sha512-tEBHqDnIoM/1rXME1zgka9g6Q2lcoCkxHLuc7ODJ5BxbP5d4c2Z5cGgtXAku59200Cx7diuHTOYfSBD8n6mm8A==", + "license": "BlueOak-1.0.0", + "engines": { + "node": ">=16 || 14 >=14.17" + } + }, + "node_modules/minizlib": { + "version": "3.1.0", + "resolved": "https://registry.npmjs.org/minizlib/-/minizlib-3.1.0.tgz", + "integrity": "sha512-KZxYo1BUkWD2TVFLr0MQoM8vUUigWD3LlD83a/75BqC+4qE0Hb1Vo5v1FgcfaNXvfXzr+5EhQ6ing/CaBijTlw==", + "license": "MIT", + "dependencies": { + "minipass": "^7.1.2" + }, + "engines": { + "node": ">= 18" + } + }, "node_modules/morgan": { "version": "1.11.0", "resolved": "https://registry.npmjs.org/morgan/-/morgan-1.11.0.tgz", @@ -2254,6 +2297,22 @@ "express": ">=4.0.0 || >=5.0.0-beta" } }, + "node_modules/tar": { + "version": "7.5.22", + "resolved": "https://registry.npmjs.org/tar/-/tar-7.5.22.tgz", + "integrity": "sha512-MFO/QzvtAOmJbkhOaCTvbGcFN9L9b+JunIsDwaKljSOdcLMea3NJ1k9Usz/rjdfSXTq4dfzfeS7W4p4YOAAHeA==", + "license": "BlueOak-1.0.0", + "dependencies": { + "@isaacs/fs-minipass": "^4.0.0", + "chownr": "^3.0.0", + "minipass": "^7.1.2", + "minizlib": "^3.1.0", + "yallist": "^5.0.0" + }, + "engines": { + "node": ">=18" + } + }, "node_modules/to-regex-range": { "version": "5.0.1", "resolved": "https://registry.npmjs.org/to-regex-range/-/to-regex-range-5.0.1.tgz", @@ -2414,6 +2473,15 @@ "integrity": "sha512-JKhqTOwSrqNA1NY5lSztJ1GrBiUodLMmIZuLiDaMRJ+itFd+ABVE8XBjOvIWL+rSqNDC74LCSFmlb/U4UZ4hJQ==", "license": "ISC" }, + "node_modules/yallist": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/yallist/-/yallist-5.0.0.tgz", + "integrity": "sha512-YgvUTfwqyc7UXVMrB+SImsVYSmTS8X/tSrtdNZMImM+n7+QTriRXyXim0mBrTXNeqzVF0KWGgHPeiyViFFrNDw==", + "license": "BlueOak-1.0.0", + "engines": { + "node": ">=18" + } + }, "node_modules/yargs": { "version": "15.4.1", "resolved": "https://registry.npmjs.org/yargs/-/yargs-15.4.1.tgz", diff --git a/server/package.json b/server/package.json index b2ae7c7..2380ed4 100644 --- a/server/package.json +++ b/server/package.json @@ -38,6 +38,7 @@ "sanitize-html": "^2.17.5", "speakeasy": "^2.0.0", "swagger-ui-express": "^5.0.1", + "tar": "^7.5.22", "ws": "^8.21.0" }, "devDependencies": { diff --git a/server/routes.guards.json b/server/routes.guards.json index c9f4520..05cefc5 100644 --- a/server/routes.guards.json +++ b/server/routes.guards.json @@ -427,6 +427,90 @@ "validate" ] }, + { + "method": "GET", + "path": "/api/v1/admin/modules", + "handlers": 2, + "gates": [ + "noindex", + "requireAuth" + ] + }, + { + "method": "POST", + "path": "/api/v1/admin/modules", + "handlers": 4, + "gates": [ + "noindex", + "requireAuth", + "middleware", + "validate" + ] + }, + { + "method": "DELETE", + "path": "/api/v1/admin/modules/:id", + "handlers": 5, + "gates": [ + "noindex", + "requireAuth", + "middleware", + "validate" + ] + }, + { + "method": "POST", + "path": "/api/v1/admin/modules/:id/disable", + "handlers": 4, + "gates": [ + "noindex", + "requireAuth", + "middleware", + "validate" + ] + }, + { + "method": "POST", + "path": "/api/v1/admin/modules/:id/enable", + "handlers": 4, + "gates": [ + "noindex", + "requireAuth", + "middleware", + "validate" + ] + }, + { + "method": "POST", + "path": "/api/v1/admin/modules/:id/purge", + "handlers": 4, + "gates": [ + "noindex", + "requireAuth", + "middleware", + "validate" + ] + }, + { + "method": "POST", + "path": "/api/v1/admin/modules/restart", + "handlers": 2, + "gates": [ + "noindex", + "requireAuth" + ] + }, + { + "method": "PUT", + "path": "/api/v1/admin/modules/sources", + "handlers": 4, + "gates": [ + "noindex", + "requireAuth", + "middleware", + "validate" + ] + }, { "method": "GET", "path": "/api/v1/admin/pages", diff --git a/server/routes.manifest.json b/server/routes.manifest.json index c3ee498..3860ae1 100644 --- a/server/routes.manifest.json +++ b/server/routes.manifest.json @@ -177,6 +177,38 @@ "method": "POST", "path": "/api/v1/admin/moderation/user/:discordId/notes" }, + { + "method": "GET", + "path": "/api/v1/admin/modules" + }, + { + "method": "POST", + "path": "/api/v1/admin/modules" + }, + { + "method": "DELETE", + "path": "/api/v1/admin/modules/:id" + }, + { + "method": "POST", + "path": "/api/v1/admin/modules/:id/disable" + }, + { + "method": "POST", + "path": "/api/v1/admin/modules/:id/enable" + }, + { + "method": "POST", + "path": "/api/v1/admin/modules/:id/purge" + }, + { + "method": "POST", + "path": "/api/v1/admin/modules/restart" + }, + { + "method": "PUT", + "path": "/api/v1/admin/modules/sources" + }, { "method": "GET", "path": "/api/v1/admin/pages" diff --git a/server/src/modules/archive.js b/server/src/modules/archive.js new file mode 100644 index 0000000..297f35a --- /dev/null +++ b/server/src/modules/archive.js @@ -0,0 +1,221 @@ +// ── The hardened bundle extractor ────────────────────────────────────────── +// +// Phase 4, slice 1 of docs/website/MODULE_SYSTEM.md §2.7.2. This is the part of +// the install path that handles input an attacker chose, and it is separated +// from install.js so that it can be tested against crafted archives without a +// network, a database or a filesystem layout. +// +// **The download is not the dangerous part.** An allowlisted host, a declared +// sha256 and a size cap between them settle where the bytes came from and that +// they are the bytes that were published. Unpacking is different: the ARCHIVE +// chooses the filenames, and core writes into a directory bind-mounted from the +// host (MODULE_SYSTEM.md §2.5), so an escape is not confined to the container. +// +// The rule this file follows is **reject, never sanitise.** node-tar will +// happily strip a leading `/` and drop a `..` for you, which turns a hostile +// archive into a slightly different archive that then gets installed. An archive +// that needs correcting is an archive that should not be trusted, so anything on +// the list below fails the whole install and nothing is written. +// +// - an absolute path, POSIX (`/etc/…`) or Windows (`C:\…`, `\\server\…`) +// - any `..` segment, anywhere +// - anything that is not a regular file or a directory — so no symlinks, no +// hardlinks, no devices, no FIFOs. A module bundle has no legitimate use for +// any of them, and every one is a documented escape primitive +// - more than one top-level entry +// - more entries, or more unpacked bytes, than the caps below +// +// That third rule deserves its own note, because it is why the `tar` dependency +// is pinned forward rather than merely present: **the majority of node-tar's +// published advisories are hardlink or symlink path traversal**, several of them +// through interpretation differences in PAX and GNU long-name headers rather +// than through anything the calling code did wrong. Refusing those entry types +// outright means this extractor is not relying on the library to get their +// containment right. The remaining advisories are parser denial-of-service, and +// those are what the caps and the two-pass shape address. +// +// Two passes, deliberately: `inspect()` reads the archive and decides, and only +// an archive that survived that is handed to `extract()`. A `filter` callback +// during extraction cannot reject — by the time it is asked about entry 400, the +// first 399 are already on disk. + +const fs = require('fs') +const fsp = require('fs/promises') +const path = require('path') +const tar = require('tar') + +// Caps. Generous against a real bundle (module-uo's is a few megabytes, a few +// hundred files) and small enough that a decompression bomb is refused rather +// than paged in. Both are checked DURING the inspect pass, not after it, so a +// hostile archive stops being read at the limit instead of at its end. +const MAX_BYTES = 128 * 1024 * 1024 +const MAX_ENTRIES = 20000 + +// tar entry types worth naming. node-tar reports these as strings; anything not +// in this set is refused, including the ones no one has thought of, which is the +// point of an allowlist here rather than a list of the types known to be bad. +const ALLOWED_TYPES = new Set(['File', 'Directory']) + +class ArchiveError extends Error { + constructor(message) { + super(message) + this.name = 'ArchiveError' + } +} + +/** + * Is this entry path safe to unpack anywhere? + * + * Returns a reason string, or null when the path is fine. Written as a reason + * rather than a boolean because the operator pasting a URL needs to be told what + * was wrong with what they were served, and "the archive is invalid" is not that. + */ +function pathProblem(entryPath) { + const p = String(entryPath) + + // NUL is not a path character. node-tar has had uncaught-exception advisories + // for NUL bytes inside PAX records, so this is checked here rather than left + // to the parser. + if (p.includes('\0')) return 'an entry path contains a NUL byte' + + // A backslash is a path separator on the platform this may be unpacked on, so + // an entry that contains one is not the single path component it looks like. + if (p.includes('\\')) return `an entry path contains a backslash: "${p}"` + + if (p.startsWith('/')) return `an entry path is absolute: "${p}"` + // Drive-relative and UNC. Both are the subject of their own node-tar + // advisories, and neither is refused by a leading-slash check. + if (/^[a-zA-Z]:/.test(p)) return `an entry path is drive-absolute: "${p}"` + + const segments = p.split('/') + if (segments.includes('..')) return `an entry path escapes upward: "${p}"` + + return null +} + +/** + * Read an archive without unpacking it, and decide whether it may be unpacked. + * + * @param {string} file absolute path to the .tar.gz + * @returns {Promise<{root: string, entries: number, bytes: number}>} the single + * top-level directory name, and what is inside it. + * @throws {ArchiveError} on anything in this file's header list. + */ +async function inspect(file) { + const roots = new Set() + let entries = 0 + let bytes = 0 + // The first problem found, kept rather than thrown from inside the callback: + // throwing out of `onentry` escapes through the parser's stream machinery and + // arrives as an unhelpful wrapped error, when it arrives at all. + let problem = null + + const note = (message) => { + if (!problem) problem = message + } + + await tar.t({ + file, + onentry(entry) { + if (problem) return + + entries += 1 + if (entries > MAX_ENTRIES) { + note(`the archive has more than ${MAX_ENTRIES} entries`) + return + } + + if (!ALLOWED_TYPES.has(entry.type)) { + // The message names the type because "symbolic link" is a much more + // useful thing for an operator to read than "invalid entry". + note(`the archive contains a ${entry.type} ("${entry.path}") — a module bundle may only contain files and directories`) + return + } + + const bad = pathProblem(entry.path) + if (bad) { + note(bad) + return + } + + // A negative or absurd size is a parser-confusion primitive in its own + // right; clamping at zero keeps the running total honest. + bytes += Math.max(0, Number(entry.size) || 0) + if (bytes > MAX_BYTES) { + note(`the archive unpacks to more than ${Math.round(MAX_BYTES / 1024 / 1024)} MB`) + return + } + + const [root] = String(entry.path).split('/') + if (root) roots.add(root) + }, + }) + + if (problem) throw new ArchiveError(problem) + if (entries === 0) throw new ArchiveError('the archive is empty') + if (roots.size !== 1) { + // A bundle is one directory. More than one top-level entry means `strip: 1` + // below would silently merge or discard things, and a bundle that needs + // interpreting is not a bundle. + throw new ArchiveError( + `the archive must contain exactly one top-level directory, found ${roots.size}` + + (roots.size > 1 ? ` (${[...roots].slice(0, 4).join(', ')})` : ''), + ) + } + + return { root: [...roots][0], entries, bytes } +} + +/** + * Unpack an inspected archive into `dest`, stripping its single top level. + * + * The top-level directory is stripped rather than kept, because its name belongs + * to whoever published the bundle — module-uo's release workflow packs + * `module-uo-/`, not `uo/` — while the directory it lands in is core's + * decision and has to be the module id the loader scans for. What is inside the + * bundle is the module; what the wrapper is called is packaging. + * + * `dest` must not exist. Callers unpack to a temporary directory and move it + * into place, so a failure halfway through leaves nothing for the next boot's + * scan to find. + */ +async function extract(file, dest) { + if (fs.existsSync(dest)) throw new ArchiveError(`extraction target already exists: ${dest}`) + await fsp.mkdir(dest, { recursive: true }) + + await tar.x({ + file, + cwd: dest, + strip: 1, + // Belt and braces on top of inspect(): the library's own refusal to write + // outside cwd, and its refusal to overwrite through a link. Neither is + // load-bearing here — inspect() has already rejected every entry that could + // exercise them — but a second lock on a door that is already locked costs + // nothing, and this is the door. + preservePaths: false, + // Reject rather than warn on anything the parser itself objects to. + strict: true, + }) + + return dest +} + +/** + * Inspect, then extract. The only entry point install.js uses. + */ +async function unpack(file, dest) { + const stats = await inspect(file) + await extract(file, dest) + return stats +} + +module.exports = { + ArchiveError, + inspect, + extract, + unpack, + pathProblem, + MAX_BYTES, + MAX_ENTRIES, + ALLOWED_TYPES, +} diff --git a/server/src/modules/install.js b/server/src/modules/install.js new file mode 100644 index 0000000..973b05a --- /dev/null +++ b/server/src/modules/install.js @@ -0,0 +1,421 @@ +// ── Installing and removing a module bundle ──────────────────────────────── +// +// Phase 4, slice 1 of docs/website/MODULE_SYSTEM.md §2.7.2 — the consumer half +// of a release the module's own CI already publishes. Nothing here touches the +// database or the loader: this file moves bytes onto the volume and off it, and +// the caller (router/v1/admin/modules.controller.js) writes down what happened. +// +// The shape of an install, and why it is this shape: +// +// 1. The operator pastes the URL of a release's install manifest — a small +// JSON document naming the artifact, its sha256 and its size (decision 2). +// There is no catalog, because a catalog would make core's release cadence +// decide which modules can exist. +// 2. Every URL fetched — the manifest, the artifact, and every redirect hop — +// is checked against the admin-managed host allowlist (decision 6). That is +// what keeps a pasted URL from also being an SSRF primitive. +// 3. The artifact is streamed to a temporary file under a byte cap, hashed as +// it arrives, and compared against the manifest's `sha256`. The hash is the +// trust anchor; releases are unsigned and say so. +// 4. modules/archive.js inspects the archive in full before a single byte is +// unpacked, and only then unpacks it — into a TEMPORARY directory. +// 5. The unpacked tree's own `module.json` must agree with the manifest about +// what it is. A manifest that promises `uo` and delivers something else is +// refused rather than installed under the name it was promised. +// 6. Only then is anything moved into `modules//`, and the directory it +// replaces is kept aside until the move has succeeded. +// +// **Nothing is written into the modules directory until every check has passed**, +// and that is not belt-and-braces. Verified against node-tar 7.5.22 while writing +// this: extracting an archive whose fourth member escapes upward throws — and +// leaves the first three members on disk. A loader scans that directory at +// require time on the next boot; a half-unpacked module is a module. +// +// One thing this file deliberately does NOT do: mount anything. Installing puts +// a directory on the volume, and §1.12 means the volume is read at require time, +// so the module appears when the process restarts. The admin screen offers that +// restart (decision 1); it is not implied here. + +const crypto = require('crypto') +const fs = require('fs') +const fsp = require('fs/promises') +const path = require('path') +const { Readable, Transform } = require('stream') +const { pipeline } = require('stream/promises') + +const archive = require('./archive') +const loader = require('./loader') + +const log = require('../utils/logger')('modules') + +// An install manifest is a small JSON document. A megabyte of it is not a +// manifest, and reading it into memory unbounded is the one place this file +// would otherwise trust a remote length. +const MAX_MANIFEST_BYTES = 256 * 1024 +// The artifact cap is the archive's own unpacked cap — a compressed bundle +// larger than what it is allowed to unpack to has nothing to offer. +const MAX_ARTIFACT_BYTES = archive.MAX_BYTES +const FETCH_TIMEOUT_MS = 30000 +// Gitea serves a release asset through at least one redirect. Following them is +// necessary; following them blindly is how an allowlist gets bypassed, so each +// hop is re-checked and the chain is bounded. +const MAX_REDIRECTS = 5 + +// Same id rule the loader enforces when scanning, restated rather than imported +// so an install cannot put a directory on the volume that the loader would then +// refuse to look at. +const ID = /^[a-z][a-z0-9-]{1,31}$/ +const SHA256 = /^[0-9a-f]{64}$/ + +class InstallError extends Error { + constructor(message, { status = 400 } = {}) { + super(message) + this.name = 'InstallError' + // Carried so the controller can answer 400 for "your URL is wrong" and 502 + // for "the host you named misbehaved" without re-deriving it from the text. + this.status = status + } +} + +// ── The allowlist ────────────────────────────────────────────────────────── + +/** + * Parse the stored allowlist setting into hostnames. + * + * Comma or whitespace separated, case-insensitive, empty entries dropped. An + * empty list means nothing may be installed from anywhere — a refusal, never a + * wildcard. That direction matters: a setting an admin accidentally blanks + * should stop installs, not permit every host on the internet. + */ +function parseHosts(value) { + return String(value || '') + .split(/[,\s]+/) + .map((h) => h.trim().toLowerCase()) + .filter(Boolean) +} + +/** + * Check one URL against the allowlist, and return it parsed. + * + * `https` only. A plaintext fetch of code this process is going to execute is + * not something an operator should be able to opt into by typing a URL, and the + * sha256 does not help — whoever can rewrite the artifact in flight can rewrite + * the manifest that declares its hash. + */ +function checkUrl(raw, hosts) { + let url + try { + url = new URL(String(raw)) + } catch { + throw new InstallError(`"${raw}" is not a valid URL`) + } + if (url.protocol !== 'https:') { + throw new InstallError(`only https URLs may be installed from (got "${url.protocol}")`) + } + if (!hosts.length) { + throw new InstallError( + 'no module source hosts are allowed — set one in Admin → Modules before installing', + ) + } + if (!hosts.includes(url.hostname.toLowerCase())) { + throw new InstallError( + `"${url.hostname}" is not an allowed module source host (allowed: ${hosts.join(', ')})`, + ) + } + return url +} + +// ── Fetching ─────────────────────────────────────────────────────────────── + +/** + * GET a URL, following redirects by hand so every hop is re-checked. + * + * `fetch`'s own redirect following would take the first hop off the allowlist + * and the rest wherever it was pointed, which is exactly the hole the allowlist + * exists to close. + * + * `fetchImpl` is the same injection seam `replayFragments({query})` and + * `lifecycle.boot({model})` use, and it exists for the same reason: the rules + * this file enforces — https only, allowlisted host, allowlisted REDIRECT host, + * bounded body, matching hash — are all decisions about a response, and testing + * them against a real TLS server would mean testing Node's certificate handling + * instead. Production never passes it. + * + * @returns {Promise} a response whose body has not been read. + */ +async function get(rawUrl, hosts, fetchImpl = fetch) { + let url = checkUrl(rawUrl, hosts) + + for (let hop = 0; hop <= MAX_REDIRECTS; hop += 1) { + let res + try { + res = await fetchImpl(url, { + redirect: 'manual', + signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), + headers: { accept: '*/*' }, + }) + } catch (err) { + throw new InstallError(`could not reach ${url.hostname}: ${err.message}`, { status: 502 }) + } + + if (res.status >= 300 && res.status < 400) { + const location = res.headers.get('location') + if (!location) throw new InstallError(`${url.hostname} redirected without a location`, { status: 502 }) + // Resolved against the current URL, then re-checked — a relative redirect + // is normal and a cross-host one is the thing being guarded against. + url = checkUrl(new URL(location, url).toString(), hosts) + continue + } + + if (!res.ok) { + throw new InstallError(`${url.href} returned ${res.status}`, { status: 502 }) + } + return res + } + + throw new InstallError(`too many redirects (more than ${MAX_REDIRECTS})`, { status: 502 }) +} + +/** Read a bounded response body as text. */ +async function readText(res, cap, what) { + const declared = Number(res.headers.get('content-length')) + if (Number.isFinite(declared) && declared > cap) { + throw new InstallError(`${what} is larger than ${cap} bytes`) + } + const buf = Buffer.from(await res.arrayBuffer()) + // Checked again after reading: content-length is the server's claim, not a + // limit it is obliged to honour. + if (buf.length > cap) throw new InstallError(`${what} is larger than ${cap} bytes`) + return buf.toString('utf8') +} + +/** + * Stream a response body to a file, hashing as it goes and stopping at the cap. + * + * The hash is computed from the bytes that were written rather than by re-reading + * the file, so there is no window in which the file could differ from what was + * verified. + */ +async function download(res, dest, cap) { + const hash = crypto.createHash('sha256') + let bytes = 0 + + const body = Readable.fromWeb(res.body) + const counter = new Transform({ + transform(chunk, _enc, cb) { + bytes += chunk.length + if (bytes > cap) { + cb(new InstallError(`the artifact is larger than ${Math.round(cap / 1024 / 1024)} MB`)) + return + } + hash.update(chunk) + cb(null, chunk) + }, + }) + + await pipeline(body, counter, fs.createWriteStream(dest)) + return { sha256: hash.digest('hex'), bytes } +} + +// ── The install manifest ─────────────────────────────────────────────────── + +/** + * Fetch and validate a release's install manifest. + * + * The shape is the one module-uo's release workflow already writes: + * `{schema, id, name, version, coreApi, artifact, url, sha256, size}`. Only the + * fields this side needs are required — a module publisher may carry more. + */ +async function fetchManifest(manifestUrl, hosts, fetchImpl = fetch) { + const res = await get(manifestUrl, hosts, fetchImpl) + const text = await readText(res, MAX_MANIFEST_BYTES, 'the install manifest') + + let manifest + try { + manifest = JSON.parse(text) + } catch (err) { + throw new InstallError(`the install manifest is not valid JSON: ${err.message}`) + } + if (!manifest || typeof manifest !== 'object' || Array.isArray(manifest)) { + throw new InstallError('the install manifest is not a JSON object') + } + + const { id, name, version, sha256 } = manifest + if (!ID.test(String(id || ''))) { + throw new InstallError(`the install manifest has an invalid module id: ${JSON.stringify(id)}`) + } + if (!name || !version) throw new InstallError('the install manifest is missing name or version') + if (!SHA256.test(String(sha256 || '').toLowerCase())) { + throw new InstallError('the install manifest has no valid sha256 for its artifact') + } + + // `url` is the absolute artifact URL the publisher wrote; `artifact` is its + // filename. Prefer the URL, fall back to resolving the filename beside the + // manifest — which is where a release's assets sit — so a manifest that + // travelled without its absolute URL still installs. + const artifactUrl = new URL(manifest.url || manifest.artifact || '', manifestUrl).toString() + + return { + id: String(id), + name: String(name), + version: String(version), + sha256: String(sha256).toLowerCase(), + size: Number(manifest.size) || null, + artifactUrl, + coreApi: manifest.coreApi ? String(manifest.coreApi) : null, + } +} + +// ── Paths on the volume ──────────────────────────────────────────────────── + +/** Where a module with this id lives. Rejects an id that is not one. */ +function moduleDir(id) { + if (!ID.test(String(id || ''))) throw new InstallError(`invalid module id: ${JSON.stringify(id)}`) + return path.join(loader.dir(), String(id)) +} + +/** Is there a directory for this module on the volume right now? */ +function isInstalled(id) { + try { + return fs.statSync(moduleDir(id)).isDirectory() + } catch { + return false + } +} + +/** + * The absolute path of a module's `purge.sql`, or null. + * + * Read from the module's own `module.json` on disk rather than from the loader, + * because purge has to work for a module that never loaded — a `startup_failed` + * one is exactly when an operator wants its tables gone. + */ +function purgeFile(id) { + try { + const manifest = JSON.parse(fs.readFileSync(path.join(moduleDir(id), 'module.json'), 'utf8')) + if (!manifest.purge) return null + const file = path.resolve(moduleDir(id), manifest.purge) + // The same containment check the loader applies to `client.entry`: a + // manifest may not point core at a file outside the module. + if (!file.startsWith(moduleDir(id) + path.sep)) return null + return fs.existsSync(file) ? file : null + } catch { + return null + } +} + +// ── Install ──────────────────────────────────────────────────────────────── + +/** + * Download, verify, unpack and install a module from an install-manifest URL. + * + * Never leaves a partial module on the volume: everything happens under a + * scratch directory that is removed on any failure, and the move into place is + * the last step. + * + * @param {object} args + * @param {string} args.url the install manifest URL the admin pasted + * @param {string[]} args.hosts the allowlist, already parsed + * @returns {Promise<{id,name,version,sha256,source,bytes,replaced}>} + */ +async function install({ url, hosts, fetchImpl = fetch }) { + const manifest = await fetchManifest(url, hosts, fetchImpl) + 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') + const unpacked = path.join(scratch, 'unpacked') + + try { + const res = await get(manifest.artifactUrl, hosts, fetchImpl) + const { sha256, bytes } = await download(res, tarball, MAX_ARTIFACT_BYTES) + + if (sha256 !== manifest.sha256) { + // The whole trust model in one comparison. Deliberately does not name the + // computed hash in a way that reads like a value to copy into the + // manifest — a mismatch means stop, not reconcile. + throw new InstallError( + `the downloaded artifact does not match the sha256 in the install manifest — refusing to install`, + ) + } + if (manifest.size && bytes !== manifest.size) { + throw new InstallError(`the artifact is ${bytes} bytes but the manifest declares ${manifest.size}`) + } + + await archive.unpack(tarball, unpacked) + + // What the bundle says it is, checked against what the manifest promised. + let inner + try { + inner = JSON.parse(await fsp.readFile(path.join(unpacked, 'module.json'), 'utf8')) + } catch { + throw new InstallError('the bundle has no readable module.json at its root') + } + if (inner.id !== manifest.id) { + throw new InstallError( + `the bundle declares module id "${inner.id}" but the install manifest promised "${manifest.id}"`, + ) + } + if (inner.version !== manifest.version) { + throw new InstallError( + `the bundle declares version "${inner.version}" but the install manifest promised "${manifest.version}"`, + ) + } + + // The swap. The outgoing directory is moved aside rather than deleted first, + // so a failed rename leaves the previous version recoverable instead of + // leaving no module at all — the same reasoning the installer repo applies + // to a ServUO tree. + const replaced = fs.existsSync(target) + const aside = `${target}.replaced-${Date.now()}` + if (replaced) await fsp.rename(target, aside) + try { + await fsp.rename(unpacked, target) + } catch (err) { + if (replaced) await fsp.rename(aside, target).catch(() => {}) + throw err + } + if (replaced) await fsp.rm(aside, { recursive: true, force: true }) + + log.info(`installed module "${manifest.id}" v${manifest.version}`, { + source: url, + sha256: manifest.sha256, + bytes, + replaced, + }) + + return { ...manifest, source: url, bytes, replaced } + } finally { + await fsp.rm(scratch, { recursive: true, force: true }).catch(() => {}) + } +} + +/** + * Remove a module's directory from the volume. + * + * Returns whether there was one. Does not touch the database, does not run + * purge.sql, and does not stop the running module — the controller sequences + * all three, because the order matters and only it knows what the operator + * asked for. + */ +async function removeDir(id) { + const dir = moduleDir(id) + if (!fs.existsSync(dir)) return false + await fsp.rm(dir, { recursive: true, force: true }) + log.info(`removed module directory for "${id}"`) + return true +} + +module.exports = { + InstallError, + parseHosts, + checkUrl, + get, + fetchManifest, + install, + removeDir, + moduleDir, + isInstalled, + purgeFile, + MAX_MANIFEST_BYTES, + MAX_ARTIFACT_BYTES, +} diff --git a/server/src/modules/lifecycle.js b/server/src/modules/lifecycle.js index f6f837a..737773b 100644 --- a/server/src/modules/lifecycle.js +++ b/server/src/modules/lifecycle.js @@ -205,4 +205,67 @@ async function shutdown({ modules, budgetMs = SHUTDOWN_BUDGET_MS } = {}) { } } -module.exports = { boot, shutdown, SHUTDOWN_BUDGET_MS } +/** + * Stop ONE module and mark it disabled — the admin panel's Disable (§2.7.2 + * decision 3). + * + * Phase 2 built disable as a pure state flip: the record moved to `disabled` and + * the dispatch guard started answering 404. That makes a module invisible, not + * stopped. Everything a module does that is not a response to a request — the + * sockets and timers its `onBoot` armed — carried on running, so an operator + * disabling a module *because* it was misbehaving got nothing until the next + * restart, which is the one thing the button was supposed to save them. + * + * So the hook runs first, and the state moves after it: while `onShutdown` is + * running the module is still `started`, which is the only state in which its + * own routes and the things it is tearing down are consistent with each other. + * The hook gets the same budget the exit path gives it. + * + * A hook that throws does NOT stop the disable. The operator asked for this + * module to stop answering; a module that could not close cleanly is a reason to + * log loudly, not a reason to leave it serving. That is the opposite of the boot + * path's rule, and deliberately: there, a failure means the module never became + * safe to use. + * + * **Enable is not the mirror of this, and there is no `start(id)` beside it.** + * There is no `onBoot` re-dispatch, and MODULE_API.md has never promised the + * hooks are re-entrant — no module author has written `onBoot` to be safe to run + * twice in one process. Re-enabling therefore moves the row and waits for a + * restart, which the admin screen offers. + * + * @returns {Promise<{stopped: boolean, error: string|null}>} whether a hook ran. + */ +async function stop(id, { modules, model, budgetMs = SHUTDOWN_BUDGET_MS } = {}) { + /* eslint-disable global-require */ + const loader = modules || require('./loader') + const rows = model || require('../model/modules/modules.model') + /* eslint-enable global-require */ + + let error = null + let stopped = false + + if (loader.isLoaded()) { + const target = loader.stopHook(id) + if (target) { + try { + await withBudget(target.hook, budgetMs) + stopped = true + log.info(`module "${id}" stopped by an operator`) + } catch (err) { + error = err.message + log.warn(`module "${id}" onShutdown failed or timed out while being disabled — disabling anyway`, { + error: err.message, + }) + } + } + // Unconditional, and after the hook: this is what makes its routes and its + // client chunk answer 404 (§4.5). A module that is not loaded in this + // process has no record to move, and setState ignores an unknown id. + loader.setState(id, 'disabled') + } + + await safe(`disabling module "${id}"`, () => rows.disable(id)) + return { stopped, error } +} + +module.exports = { boot, shutdown, stop, SHUTDOWN_BUDGET_MS } diff --git a/server/src/modules/loader.js b/server/src/modules/loader.js index e986ac5..53c7aad 100644 --- a/server/src/modules/loader.js +++ b/server/src/modules/loader.js @@ -796,6 +796,31 @@ function shutdownHooks() { .reverse() } +/** + * One module's `onShutdown`, for stopping it on its own rather than at exit. + * + * Phase 4 (§2.7.2 decision 3) gave the admin panel's Disable a real meaning. + * Until then, disabling flipped this record's state and the dispatch guard began + * answering 404 — which made the module invisible without making it stop. A + * module's `onBoot` is where it opens its sockets and arms its timers, and none + * of that is reachable through a URL, so an operator disabling a misbehaving + * module got no relief from it at all until the next restart. + * + * `started` only, the same rule shutdownHooks() applies and for the same reason: + * a module whose `onBoot` threw has a half-built world its `onShutdown` was + * never written to tear down. Returns null when there is nothing to run — which + * covers "not started", "no hook", and "no such module", none of which is an + * error the caller can act on differently. + * + * @returns {{id: string, hook: Function}|null} + */ +function stopHook(id) { + assertLoaded('stopHook') + const record = modules.get(id) + if (!record || record.state !== 'started' || !record.hooks.onShutdown) return null + return { id: record.id, hook: record.hooks.onShutdown } +} + /** * Every schema fragment waiting to be replayed, in scan order. * @@ -896,6 +921,7 @@ module.exports = { fragments, bootable, shutdownHooks, + stopHook, clientChunks, clientEntryUrls, specFragments, diff --git a/server/src/modules/schema.js b/server/src/modules/schema.js index b1df79c..d948231 100644 --- a/server/src/modules/schema.js +++ b/server/src/modules/schema.js @@ -81,4 +81,48 @@ async function replayFragments({ query, modules } = {}) { } } -module.exports = { replayFragments } +/** + * Run one module's `purge.sql` — the destructive twin of the replay above + * (MODULE_SYSTEM.md §2.5, and §2.7.2 decision 5 for when it is offered). + * + * It lives beside replayFragments because they are the same operation pointed in + * opposite directions: a file of statements the module ships, split by the same + * splitter, run serially on the same pool. Keeping them together is what makes + * it obvious that the fragment's leading-verb allowlist does NOT apply here — + * `purge.sql` is the one file a module may put a DROP in, precisely because it + * is the one file that never runs on a boot. + * + * Unlike the replay, this **throws**. A replay failure is one module failing to + * start, which the site survives by 503ing that module; a purge failure is an + * operator's explicit destructive request not having happened, and reporting + * success for that would leave them believing data is gone when it is not. + * + * Statements run serially and are not wrapped in a transaction, for the reason + * the replay's header already gives: MariaDB commits DDL implicitly, so there is + * no rollback to have. A purge that fails halfway has dropped some tables — the + * error names the statement that stopped it, and running it again is safe + * because a purge script is required to be idempotent in the same way a fragment + * is (`DROP TABLE IF EXISTS`). + * + * @param {string} file absolute path to the module's purge.sql + * @param {object} [deps] injection seam for tests + * @returns {Promise} how many statements ran + */ +async function runPurge(file, { query } = {}) { + // eslint-disable-next-line global-require + const run = query || require('../utils/db').query + + const statements = splitStatements(fs.readFileSync(file, 'utf8')) + let ran = 0 + for (const statement of statements) { + try { + await run(statement) + ran += 1 + } catch (err) { + throw new Error(`purge failed at statement ${ran + 1} of ${statements.length}: ${err.message}`) + } + } + return ran +} + +module.exports = { replayFragments, runPurge } diff --git a/server/src/router/v1/admin/index.js b/server/src/router/v1/admin/index.js index 5452660..24c5fed 100644 --- a/server/src/router/v1/admin/index.js +++ b/server/src/router/v1/admin/index.js @@ -30,6 +30,7 @@ const pagesRouter = require('./pages.router') const emailRouter = require('./email.router') const discordBotRouter = require('./discordBot.router') const settingsRouter = require('./settings.router') +const modulesRouter = require('./modules.router') const dashboardRouter = require('./dashboard.router') const adminRouter = express.Router() @@ -73,6 +74,11 @@ adminRouter.use('/pages', pagesRouter) adminRouter.use('/email', emailRouter) adminRouter.use('/discord-bot', discordBotRouter) adminRouter.use('/settings', settingsRouter) +// The Modules screen (Phase 4). Core's, not a module's — and it has to be +// core's: it is how a module gets onto the volume in the first place. Mounted +// here alongside the other configuration capabilities, and admin-only per route +// rather than at this line, so the gate sits next to what it is guarding. +adminRouter.use('/modules', modulesRouter) // The two singletons that own no path segment of their own: GET /dashboard and // PUT /site-mode. Mounted at the group root, last, exactly where the residual diff --git a/server/src/router/v1/admin/modules.controller.js b/server/src/router/v1/admin/modules.controller.js new file mode 100644 index 0000000..d01f5e5 --- /dev/null +++ b/server/src/router/v1/admin/modules.controller.js @@ -0,0 +1,379 @@ +// ── Admin: installed modules ─────────────────────────────────────────────── +// +// Phase 4, slice 1 of docs/website/MODULE_SYSTEM.md §2.7.2. Admin-only, and more +// so than anything else in this directory: installing a module puts JavaScript on +// the volume that core will `require` into its own process on the next boot. That +// is the feature — it is what "an operator never builds anything" means (§1.14) — +// but it is worth being plain that this controller is remote code execution with +// an audit trail, not a settings screen. +// +// What guards it, in the order an attacker would meet them: +// +// 1. `requireRole('admin')` on every route, on top of the group's staff gate. +// 2. An `https`-only host allowlist, re-checked on every redirect hop, so a +// pasted URL cannot be pointed at the compose network or a metadata service +// (install.js). +// 3. The sha256 the release published, compared against the bytes that arrived. +// 4. A full inspection of the archive before a byte of it is unpacked, and an +// unpack into a scratch directory that is only moved into place once the +// bundle has agreed with the manifest about what it is (archive.js). +// 5. Every action here writes to core's one audit log. +// +// The one thing this file cannot do is mount anything. §1.12 makes the volume the +// mounting source of truth, read once at require time, so install and uninstall +// take effect on the next boot — which is why `restart` is a route here rather +// than a sentence in a tooltip (decision 1). + +const modules = require('../../../model/modules/modules.model') +const activity = require('../../../model/activity/activity.model') +const settings = require('../../../model/settings/settings.model') +const loader = require('../../../modules/loader') +const lifecycle = require('../../../modules/lifecycle') +const install = require('../../../modules/install') +// A namespace import, like every other require in this file, and not +// `const { runPurge } = …`: destructuring at require time captures the function +// rather than the module, which makes it the one dependency here that cannot be +// substituted. That matters because the two tests worth having about purge are +// about the ORDER it runs in relative to the directory being removed. +const schema = require('../../../modules/schema') + +const log = require('../../../utils/logger')('admin-modules') + +// The allowlist setting. Seeded from MODULE_SOURCE_HOSTS on first boot and +// admin-managed from then on (decision 6) — db/seed.js writes it once and never +// overwrites it, so changing the variable later does not silently reach in and +// undo an operator's choice. +const HOSTS_KEY = 'module_source_hosts' + +// A hostname, not a URL: no scheme, no path, no port, no wildcard. Deliberately +// strict — every character allowed here is a character that can appear in the +// host of a URL this server will fetch and execute the contents of. +const HOSTNAME = /^[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)*$/ + +async function allowedHosts() { + return install.parseHosts(await settings.get(HOSTS_KEY)) +} + +/** + * One module, as the admin screen needs it. + * + * Three sources have to be reconciled, and which one answers which question is + * the whole of §2.4: + * + * - the ROW says what the operator decided and what the last boot recorded; + * - the LOADER says what is mounted and answering right now; + * - the VOLUME says whether there is still a directory there at all. + * + * They can legitimately disagree, and the screen has to show that rather than + * pick a winner. A row `enabled` with a loader state of `disabled` is a module + * the operator has just switched back on and which is waiting for a restart — + * exactly the case decision 3 creates, and it would be a lie to render it as + * either "running" or "off". + */ +function present(row, live, onVolume) { + return { + id: row ? row.id : live.id, + name: row ? row.name : live.name, + version: row ? row.version : live.version, + // What the database records. + state: row ? row.state : null, + failureStage: row ? row.failureStage : (live && live.stage) || null, + failureReason: row ? row.failureReason : (live && live.reason) || null, + source: row ? row.source : null, + sha256: row ? row.sha256 : null, + installedAt: row ? row.installedAt : null, + startedAt: row ? row.startedAt : null, + // What is actually mounted in this process, and what it is answering. + liveState: live ? live.state : null, + capabilities: live ? live.capabilities : [], + // What is on the volume. + onVolume, + canPurge: onVolume && Boolean(install.purgeFile(row ? row.id : live.id)), + } +} + +// GET /admin/modules — every module core knows about, from all three sources, +// plus the source allowlist the install form needs. +async function list(req, res) { + try { + const rows = await modules.list() + // The loader throws rather than returning [] before load() has run (§7.6), + // and this controller is reachable from a process where that is true — + // `npm run seed` never gets here, but a test harness might. + const live = loader.isLoaded() ? loader.list() : [] + const byId = new Map(live.map((m) => [m.id, m])) + + const seen = new Set() + const out = [] + for (const row of rows) { + seen.add(row.id) + out.push(present(row, byId.get(row.id) || null, install.isInstalled(row.id))) + } + // A directory on the volume that has no row yet — a hand-placed install + // before its first boot. It has to be listed, or the screen would show + // nothing for a module whose routes are already being served. + for (const m of live) { + if (!seen.has(m.id)) out.push(present(null, m, true)) + } + + return res.json({ modules: out, sourceHosts: await allowedHosts() }) + } catch (err) { + log.error('list modules', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + +// POST /admin/modules — install (or upgrade) from an install-manifest URL. +async function create(req, res) { + const url = String(req.body.url || '').trim() + try { + const hosts = await allowedHosts() + const result = await install.install({ url, hosts }) + + // Provenance is written here and nowhere else: the boot reconcile records a + // module with NULL source/sha256 and leaves what it is not given, precisely + // so that a refresh cannot overwrite what an install knew (lifecycle.js). + const row = await modules.recordInstalled({ + id: result.id, + name: result.name, + version: result.version, + source: result.source, + sha256: result.sha256, + }) + + await activity.log({ + req, + userId: req.user.id, + action: 'module.install', + detail: { id: result.id, version: result.version, source: url, sha256: result.sha256, replaced: result.replaced }, + }) + log.warn('module installed — it will mount on the next restart', { + id: result.id, + version: result.version, + by: req.user.username, + }) + + return res.status(201).json({ module: row, restartRequired: true, replaced: result.replaced }) + } catch (err) { + if (err.name === 'InstallError' || err.name === 'ArchiveError') { + // The operator pasted a URL and something about what came back was wrong. + // The message is the useful part and is written to be read by them. + log.warn('module install refused', { url, reason: err.message }) + return res.status(err.status || 400).json({ message: err.message }) + } + log.error('install module', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + +// POST /admin/modules/:id/enable — switch a module back on, for the next boot. +// +// Deliberately does NOT touch the loader's record. Disable ran the module's +// onShutdown (decision 3), and there is no onBoot re-dispatch to undo that: a +// module whose sockets were closed and timers cleared cannot be made to serve +// again by flipping a flag, and pretending otherwise would put it back on the +// nav with a torn-down world behind it. The row moves; the restart starts it. +async function enable(req, res) { + const { id } = req.params + try { + const row = await modules.enable(id) + if (!row) return res.status(404).json({ message: 'No such module.' }) + + await activity.log({ req, userId: req.user.id, action: 'module.enable', detail: { id } }) + return res.json({ module: row, restartRequired: true }) + } catch (err) { + if (err.name === 'ModuleStateError') return res.status(409).json({ message: err.message }) + log.error('enable module', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + +// POST /admin/modules/:id/disable — stop it now. +// +// The one action on this screen that takes effect without a restart, and the +// reason it does is that it is the one an operator reaches for when something is +// going wrong. Its routes answer 404 from the moment this returns, and its +// onShutdown has already run. +async function disable(req, res) { + const { id } = req.params + try { + const current = await modules.get(id) + if (!current) return res.status(404).json({ message: 'No such module.' }) + + const { stopped, error } = await lifecycle.stop(id) + const row = await modules.get(id) + + await activity.log({ + req, + userId: req.user.id, + action: 'module.disable', + detail: { id, hookRan: stopped, hookError: error }, + }) + log.warn('module disabled by an operator', { id, hookRan: stopped, by: req.user.username }) + + // `shutdownError` is reported rather than swallowed: the module IS disabled + // either way, and an operator whose module could not close cleanly should be + // told so while they still have the logs to look at. + return res.json({ module: row, stopped, shutdownError: error }) + } catch (err) { + log.error('disable module', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + +// DELETE /admin/modules/:id[?purge=true] — uninstall. +// +// Non-destructive by default (§2.5): the directory goes, the row stays +// `disabled`, and the module's tables and data are left alone. +// +// The purge option is here rather than as a follow-up action because it cannot +// be a follow-up action (decision 5): `purge.sql` is a file inside the directory +// this is about to delete, so after an uninstall there is nothing left to purge +// with. Ticking the box is the last moment the file exists. +// +// The order below is the whole of it, and each step depends on the one above: +// purge while the SQL is still readable, stop while the code is still loaded, +// then delete. +async function remove(req, res) { + const { id } = req.params + const purge = req.query.purge === 'true' || req.query.purge === '1' + try { + const current = await modules.get(id) + const onVolume = install.isInstalled(id) + if (!current && !onVolume) return res.status(404).json({ message: 'No such module.' }) + + let purged = null + if (purge) { + const file = install.purgeFile(id) + if (!file) { + return res.status(400).json({ + message: 'This module ships no purge.sql, so its data cannot be deleted. Uninstall without purging instead.', + }) + } + purged = await schema.runPurge(file) + } + + // Stop it before its files vanish. A module whose directory is deleted out + // from under a running onShutdown is being asked to tear down a world whose + // code may already be half-unreadable — and its sockets would otherwise stay + // open until the restart, holding a connection on behalf of a module that no + // longer exists on disk. + await lifecycle.stop(id) + const removed = await install.removeDir(id) + + // A purge leaves nothing: no directory, no tables, no data. Keeping a + // `disabled` row for that is a tombstone with nothing to offer and a Purge + // button that would fail. A plain uninstall keeps its row, which is what + // makes the retained data visible and reinstallable. + if (purge) await modules.remove(id) + + await activity.log({ + req, + userId: req.user.id, + action: purge ? 'module.purge' : 'module.uninstall', + detail: { id, purged, removed }, + }) + log.warn(`module ${purge ? 'uninstalled and purged' : 'uninstalled'}`, { + id, + statements: purged, + by: req.user.username, + }) + + return res.json({ id, removed, purged, restartRequired: true }) + } catch (err) { + log.error('uninstall module', err) + return res.status(500).json({ message: err.message || 'Internal Server Error' }) + } +} + +// POST /admin/modules/:id/purge — drop a still-installed module's data. +// +// Refuses unless the module is already disabled, and that guard is the point: +// dropping the tables under a module that is still serving requests leaves it +// answering out of a world that no longer exists. Disabling first is one click +// and makes the destructive step happen against something that has stopped. +async function purge(req, res) { + const { id } = req.params + try { + const current = await modules.get(id) + if (!current) return res.status(404).json({ message: 'No such module.' }) + if (current.state !== 'disabled') { + return res.status(409).json({ + message: 'Disable this module before purging its data, so nothing is serving out of the tables being dropped.', + }) + } + + const file = install.purgeFile(id) + if (!file) { + return res.status(400).json({ message: 'This module ships no purge.sql, so its data cannot be deleted.' }) + } + + const statements = await schema.runPurge(file) + await activity.log({ req, userId: req.user.id, action: 'module.purge', detail: { id, statements } }) + log.warn('module data purged', { id, statements, by: req.user.username }) + + return res.json({ id, purged: statements }) + } catch (err) { + log.error('purge module', err) + return res.status(500).json({ message: err.message || 'Internal Server Error' }) + } +} + +// PUT /admin/modules/sources — the host allowlist. +async function setSources(req, res) { + const hosts = install.parseHosts(req.body.hosts) + const bad = hosts.find((h) => !HOSTNAME.test(h)) + if (bad) return res.status(400).json({ message: `"${bad}" is not a valid hostname.` }) + + try { + const before = await allowedHosts() + await settings.set(HOSTS_KEY, hosts.join(','), req.user.id) + await activity.log({ + req, + userId: req.user.id, + action: 'module.sources', + detail: { before, after: hosts }, + }) + log.warn('module source allowlist changed', { before, after: hosts, by: req.user.username }) + return res.json({ sourceHosts: hosts }) + } catch (err) { + log.error('set module sources', err) + return res.status(500).json({ message: 'Internal Server Error' }) + } +} + +// POST /admin/modules/restart — restart the server process. +// +// Decision 1. Install, uninstall and re-enable all only take effect at boot +// because §1.12 reads the volume at require time, and §2.4 promises recovery +// "with no shell access to the box" — which a banner saying "please restart your +// container" does not deliver. +// +// It raises SIGTERM against its own process rather than calling the shutdown +// path directly. server.js already has a handler that stops the modules, the +// workers and the listeners in the right order and closes the pool and the log +// file before exiting 0; reaching that through the signal means there is exactly +// one graceful-shutdown path and this route cannot drift from it. +// +// What brings the process BACK is the supervisor, not this. The shipped +// docker-compose.yml declares `restart: unless-stopped` on `app`, which restarts +// on a clean exit as well as a crash. A bare `npm start` does not come back, and +// the screen says so before it asks. +function restart(req, res) { + log.warn('restart requested from the admin panel', { by: req.user.username }) + + // Logged and answered first. Once the signal is raised the response has no + // listener left to flush through, so the operator would be told nothing. + res.status(202).json({ restarting: true }) + + activity + .log({ req, userId: req.user.id, action: 'module.restart', detail: {} }) + .catch((err) => log.error('failed to record the restart in the audit log', err)) + .finally(() => { + // A beat, so the 202 is on the wire. `unref` so this timer is not itself + // something keeping the process alive. + setTimeout(() => process.kill(process.pid, 'SIGTERM'), 250).unref() + }) +} + +module.exports = { list, create, enable, disable, remove, purge, setSources, restart, HOSTS_KEY } diff --git a/server/src/router/v1/admin/modules.router.js b/server/src/router/v1/admin/modules.router.js new file mode 100644 index 0000000..a5535d6 --- /dev/null +++ b/server/src/router/v1/admin/modules.router.js @@ -0,0 +1,143 @@ +// Admin · Modules — install, enable, disable, uninstall, purge and restart. +// +// Mounted at /api/v1/admin/modules by admin/index.js, which has already applied +// `noindex, isLoggedIn, staffOnly`. Every route here re-gates to `admin`: an +// editor or moderator has no business installing code into the server process, +// and the group gate alone would let them. +// +// Route order matters in one place. `/restart` and `/sources` are declared +// BEFORE the `/:id/...` routes, because express matches in declaration order and +// a module whose id was `restart` would otherwise shadow — or be shadowed by — +// the literal path. The id pattern below makes that unreachable in practice; the +// ordering makes it unreachable by construction. + +const express = require('express') +const { body, param, query } = require('express-validator') + +const controller = require('./modules.controller') +const { requireRole } = require('../../../utils/auth') +const validate = require('../../../middleware/validate') + +const modulesRouter = express.Router() +const adminOnly = requireRole('admin') + +// The loader's own id rule (MODULE_API.md §2.1). Applied at the edge so a +// traversal-shaped id never reaches a path join, even though install.js checks +// it again — this one produces a 400 with a readable message, that one is the +// guarantee. +const ID = /^[a-z][a-z0-9-]{1,31}$/ + +modulesRouter.get( + '/', + // #swagger.tags = ['Admin · Modules'] + // #swagger.summary = 'List installed modules, their live state, and the source allowlist' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[200] = { description: 'Modules and the install source allowlist', content: { "application/json": { schema: { type: "object", properties: { modules: { type: "array", items: { type: "object", additionalProperties: true } }, sourceHosts: { type: "array", items: { type: "string" } } } } } } } */ + adminOnly, + controller.list, +) + +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.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[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 }), + validate, + controller.create, +) + +modulesRouter.put( + '/sources', + // #swagger.tags = ['Admin · Modules'] + // #swagger.summary = 'Replace the allowlist of hosts modules may be installed from' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.requestBody = { required: true, content: { "application/json": { schema: { type: "object", required: ["hosts"], properties: { hosts: { type: "string", description: "Comma- or space-separated hostnames. An empty list forbids all installs." } } } } } } */ + /* #swagger.responses[200] = { description: 'The new allowlist', content: { "application/json": { schema: { type: "object", properties: { sourceHosts: { type: "array", items: { type: "string" } } } } } } } */ + /* #swagger.responses[400] = { description: 'One of the entries is not a hostname', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + adminOnly, + body('hosts').isString().isLength({ max: 2048 }), + validate, + controller.setSources, +) + +modulesRouter.post( + '/restart', + // #swagger.tags = ['Admin · Modules'] + // #swagger.summary = 'Restart the server process so module changes take effect' + // #swagger.description = 'Runs the same graceful shutdown a SIGTERM does. The process is brought back by the supervisor, which the shipped docker-compose.yml provides; a bare `npm start` will not come back.' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + /* #swagger.responses[202] = { description: 'Shutting down', content: { "application/json": { schema: { type: "object", properties: { restarting: { type: "boolean" } } } } } } */ + adminOnly, + controller.restart, +) + +modulesRouter.post( + '/:id/enable', + // #swagger.tags = ['Admin · Modules'] + // #swagger.summary = 'Enable a module (takes effect on the next restart)' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + // #swagger.parameters['id'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'Module id.' } + /* #swagger.responses[200] = { description: 'Enabled — restart to start it', content: { "application/json": { schema: { type: "object", additionalProperties: true } } } } */ + /* #swagger.responses[404] = { description: 'No such module', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + adminOnly, + param('id').matches(ID), + validate, + controller.enable, +) + +modulesRouter.post( + '/:id/disable', + // #swagger.tags = ['Admin · Modules'] + // #swagger.summary = 'Stop a module now — runs its onShutdown, then its routes answer 404' + // #swagger.description = 'The only module action that takes effect without a restart. Re-enabling needs one, because there is no onBoot re-dispatch.' + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + // #swagger.parameters['id'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'Module id.' } + /* #swagger.responses[200] = { description: 'Disabled', content: { "application/json": { schema: { type: "object", additionalProperties: true } } } } */ + /* #swagger.responses[404] = { description: 'No such module', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + adminOnly, + param('id').matches(ID), + validate, + controller.disable, +) + +modulesRouter.post( + '/:id/purge', + // #swagger.tags = ['Admin · Modules'] + // #swagger.summary = "Run a disabled module’s purge.sql, dropping its tables and data" + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + // #swagger.parameters['id'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'Module id.' } + /* #swagger.responses[200] = { description: 'Purged', content: { "application/json": { schema: { type: "object", properties: { id: { type: "string" }, purged: { type: "integer" } } } } } } */ + /* #swagger.responses[400] = { description: 'The module ships no purge.sql', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + /* #swagger.responses[409] = { description: 'The module must be disabled first', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + adminOnly, + param('id').matches(ID), + validate, + controller.purge, +) + +modulesRouter.delete( + '/:id', + // #swagger.tags = ['Admin · Modules'] + // #swagger.summary = 'Uninstall a module, optionally deleting its data too' + // #swagger.description = "Removes the module directory and leaves its row disabled. With purge=true it also runs purge.sql first — which is the only moment it can, since purge.sql lives inside the directory being deleted." + // #swagger.security = [{ "cookieAuth": [] }, { "bearerAuth": [] }] + // #swagger.parameters['id'] = { in: 'path', required: true, schema: { type: 'string' }, description: 'Module id.' } + // #swagger.parameters['purge'] = { in: 'query', required: false, schema: { type: 'boolean' }, description: "Also run the module’s purge.sql and drop its row. Destructive and irreversible." } + /* #swagger.responses[200] = { description: 'Uninstalled — restart to unmount it', content: { "application/json": { schema: { type: "object", additionalProperties: true } } } } */ + /* #swagger.responses[400] = { description: 'Purge was asked for and the module ships no purge.sql', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + /* #swagger.responses[404] = { description: 'No such module', content: { "application/json": { schema: { $ref: "#/components/schemas/Error" } } } } */ + adminOnly, + param('id').matches(ID), + query('purge').optional().isIn(['true', 'false', '1', '0']), + validate, + controller.remove, +) + +module.exports = modulesRouter diff --git a/server/swagger/swagger-output.json b/server/swagger/swagger-output.json index dcafd53..0a5a789 100644 --- a/server/swagger/swagger-output.json +++ b/server/swagger/swagger-output.json @@ -2353,6 +2353,490 @@ } } }, + "/api/v1/admin/modules": { + "get": { + "tags": [ + "Admin · Modules" + ], + "summary": "List installed modules, their live state, and the source allowlist", + "description": "", + "responses": { + "200": { + "description": "Modules and the install source allowlist", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "modules": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": true + } + }, + "sourceHosts": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } + } + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ] + }, + "post": { + "tags": [ + "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.", + "responses": { + "201": { + "description": "Installed — restart to mount it", + "content": { + "application/json": { + "schema": { + "type": "object", + "additionalProperties": true + } + } + } + }, + "400": { + "description": "The URL, the manifest, the hash or the archive was refused", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "500": { + "description": "Internal Server Error" + }, + "502": { + "description": "The source host could not be reached or answered badly", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ], + "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" + } + } + } + } + } + } + } + }, + "/api/v1/admin/modules/restart": { + "post": { + "tags": [ + "Admin · Modules" + ], + "summary": "Restart the server process so module changes take effect", + "description": "Runs the same graceful shutdown a SIGTERM does. The process is brought back by the supervisor, which the shipped docker-compose.yml provides; a bare `npm start` will not come back.", + "responses": { + "202": { + "description": "Shutting down", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "restarting": { + "type": "boolean" + } + } + } + } + } + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ] + } + }, + "/api/v1/admin/modules/sources": { + "put": { + "tags": [ + "Admin · Modules" + ], + "summary": "Replace the allowlist of hosts modules may be installed from", + "description": "", + "responses": { + "200": { + "description": "The new allowlist", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "sourceHosts": { + "type": "array", + "items": { + "type": "string" + } + } + } + } + } + } + }, + "400": { + "description": "One of the entries is not a hostname", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "hosts" + ], + "properties": { + "hosts": { + "type": "string", + "description": "Comma- or space-separated hostnames. An empty list forbids all installs." + } + } + } + } + } + } + } + }, + "/api/v1/admin/modules/{id}": { + "delete": { + "tags": [ + "Admin · Modules" + ], + "summary": "Uninstall a module, optionally deleting its data too", + "description": "Removes the module directory and leaves its row disabled. With purge=true it also runs purge.sql first — which is the only moment it can, since purge.sql lives inside the directory being deleted.", + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "string" + }, + "description": "Module id." + }, + { + "name": "purge", + "in": "query", + "required": false, + "schema": { + "type": "boolean" + }, + "description": "Also run the module’s purge.sql and drop its row. Destructive and irreversible." + } + ], + "responses": { + "200": { + "description": "Uninstalled — restart to unmount it", + "content": { + "application/json": { + "schema": { + "type": "object", + "additionalProperties": true + } + } + } + }, + "400": { + "description": "Purge was asked for and the module ships no purge.sql", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "404": { + "description": "No such module", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ] + } + }, + "/api/v1/admin/modules/{id}/disable": { + "post": { + "tags": [ + "Admin · Modules" + ], + "summary": "Stop a module now — runs its onShutdown, then its routes answer 404", + "description": "The only module action that takes effect without a restart. Re-enabling needs one, because there is no onBoot re-dispatch.", + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "string" + }, + "description": "Module id." + } + ], + "responses": { + "200": { + "description": "Disabled", + "content": { + "application/json": { + "schema": { + "type": "object", + "additionalProperties": true + } + } + } + }, + "400": { + "description": "Bad Request" + }, + "404": { + "description": "No such module", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ] + } + }, + "/api/v1/admin/modules/{id}/enable": { + "post": { + "tags": [ + "Admin · Modules" + ], + "summary": "Enable a module (takes effect on the next restart)", + "description": "", + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "string" + }, + "description": "Module id." + } + ], + "responses": { + "200": { + "description": "Enabled — restart to start it", + "content": { + "application/json": { + "schema": { + "type": "object", + "additionalProperties": true + } + } + } + }, + "400": { + "description": "Bad Request" + }, + "404": { + "description": "No such module", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "409": { + "description": "Conflict" + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ] + } + }, + "/api/v1/admin/modules/{id}/purge": { + "post": { + "tags": [ + "Admin · Modules" + ], + "summary": "Run a disabled module’s purge.sql, dropping its tables and data", + "description": "", + "parameters": [ + { + "name": "id", + "in": "path", + "required": true, + "schema": { + "type": "string" + }, + "description": "Module id." + } + ], + "responses": { + "200": { + "description": "Purged", + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "id": { + "type": "string" + }, + "purged": { + "type": "integer" + } + } + } + } + } + }, + "400": { + "description": "The module ships no purge.sql", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "404": { + "description": "Not Found" + }, + "409": { + "description": "The module must be disabled first", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "500": { + "description": "Internal Server Error" + } + }, + "security": [ + { + "cookieAuth": [] + }, + { + "bearerAuth": [] + } + ] + } + }, "/api/v1/admin/pages": { "get": { "tags": [ diff --git a/server/test/adminModules.test.js b/server/test/adminModules.test.js new file mode 100644 index 0000000..1e482d4 --- /dev/null +++ b/server/test/adminModules.test.js @@ -0,0 +1,463 @@ +// ── Admin · Modules: the delivery surface ────────────────────────────────── +// +// Phase 4, slice 1 of MODULE_SYSTEM.md §2.7.2. The controller is tested directly +// with a mock `res` and stubbed models — the same shape adminUsers.test.js uses — +// because what is interesting here is not the HTTP plumbing but the ORDER of +// operations and which of the three sources of truth answers which question. +// +// Two of these tests exist to pin decisions that are easy to "fix" back into +// being wrong: +// +// - **enable must not touch the loader.** Disable ran the module's onShutdown; +// there is no onBoot re-dispatch, so flipping the record back would put a +// module with closed sockets and cleared timers back on the nav. +// - **purge must run before the directory is removed.** purge.sql lives inside +// that directory. Reorder those two lines and the feature silently stops +// working, with a 200 and no data deleted. +// +// Point the DB at a closed port BEFORE requiring anything that builds the pool. +process.env.DB_HOST = '127.0.0.1' +process.env.DB_PORT = '59999' + +const { test, beforeEach, after } = require('node:test') +const assert = require('node:assert/strict') + +const ctrl = require('../src/router/v1/admin/modules.controller') +const modules = require('../src/model/modules/modules.model') +const activity = require('../src/model/activity/activity.model') +const settings = require('../src/model/settings/settings.model') +const loader = require('../src/modules/loader') +const lifecycle = require('../src/modules/lifecycle') +const install = require('../src/modules/install') +const schema = require('../src/modules/schema') +const db = require('../src/utils/db') + +after(() => db.close()) + +function mockRes() { + return { + statusCode: 200, + body: null, + status(c) { this.statusCode = c; return this }, + json(b) { this.body = b; return this }, + } +} + +const req = (extra = {}) => ({ + user: { id: 1, username: 'admin' }, + params: {}, + query: {}, + body: {}, + ...extra, +}) + +// Everything the controller reaches for, replaced wholesale per test. Restored +// from these originals rather than from a snapshot taken mid-run, so one test +// leaking a stub cannot quietly become another test's fixture. +const originals = { + modules: { ...modules }, + activity: { log: activity.log }, + settings: { get: settings.get, set: settings.set }, + loader: { isLoaded: loader.isLoaded, list: loader.list }, + lifecycle: { stop: lifecycle.stop }, + install: { + install: install.install, + isInstalled: install.isInstalled, + purgeFile: install.purgeFile, + removeDir: install.removeDir, + }, + schema: { runPurge: schema.runPurge }, +} + +let logged + +beforeEach(() => { + Object.assign(modules, originals.modules) + Object.assign(activity, originals.activity) + Object.assign(settings, originals.settings) + Object.assign(loader, originals.loader) + Object.assign(lifecycle, originals.lifecycle) + Object.assign(install, originals.install) + Object.assign(schema, originals.schema) + + logged = [] + activity.log = async (entry) => { logged.push(entry) } + settings.get = async () => 'gitea.whitlocktech.com' + loader.isLoaded = () => true + loader.list = () => [] +}) + +// ── list ─────────────────────────────────────────────────────────────────── + +test('list reconciles the row, the loader and the volume without picking a winner', async () => { + // The case §2.4 creates and decision 3 makes routine: the row says `enabled` + // because the operator just switched it back on, the loader still says + // `disabled` because its onShutdown has run and there is no way back without a + // restart. Rendering either one alone would be a lie. + modules.list = async () => [{ + id: 'uo', name: 'UO', version: '1.0.0', state: 'enabled', + failureStage: null, failureReason: null, source: 'https://x/y.json', sha256: 'a'.repeat(64), + installedAt: null, startedAt: null, + }] + loader.list = () => [{ id: 'uo', name: 'UO', version: '1.0.0', state: 'disabled', stage: null, reason: null, capabilities: ['shard'] }] + install.isInstalled = () => true + install.purgeFile = () => '/modules/uo/server/db/purge.sql' + + const res = mockRes() + await ctrl.list(req(), res) + + const [m] = res.body.modules + assert.equal(m.state, 'enabled', 'what the operator decided') + assert.equal(m.liveState, 'disabled', 'what is actually answering') + assert.equal(m.onVolume, true) + assert.equal(m.canPurge, true) + assert.deepEqual(m.capabilities, ['shard']) + assert.deepEqual(res.body.sourceHosts, ['gitea.whitlocktech.com']) +}) + +test('list includes a module on the volume that has no row yet', async () => { + // A hand-placed directory before its first boot. §2.5 keeps that a supported + // install, and its routes are already being served — a screen showing nothing + // for it would be showing the wrong thing. + modules.list = async () => [] + loader.list = () => [{ id: 'byhand', name: 'By Hand', version: '0.1.0', state: 'started', stage: null, reason: null, capabilities: [] }] + install.isInstalled = () => true + install.purgeFile = () => null + + const res = mockRes() + await ctrl.list(req(), res) + + assert.equal(res.body.modules.length, 1) + assert.equal(res.body.modules[0].id, 'byhand') + assert.equal(res.body.modules[0].state, null, 'no row means no recorded state, not a guessed one') + assert.equal(res.body.modules[0].liveState, 'started') +}) + +test('list survives a process where the loader never scanned', async () => { + loader.isLoaded = () => false + loader.list = () => { throw new Error('modules.list() before modules.load()') } + modules.list = async () => [{ id: 'uo', name: 'UO', version: '1', state: 'disabled' }] + install.isInstalled = () => false + install.purgeFile = () => null + + const res = mockRes() + await ctrl.list(req(), res) + + assert.equal(res.statusCode, 200) + assert.equal(res.body.modules[0].liveState, null) +}) + +// ── install ──────────────────────────────────────────────────────────────── + +test('install records provenance and says a restart is needed', async () => { + const calls = [] + install.install = async ({ url, hosts }) => { + calls.push({ url, hosts }) + return { id: 'uo', name: 'UO', version: '1.0.0', sha256: 'b'.repeat(64), source: url, replaced: false } + } + modules.recordInstalled = async (row) => { calls.push(row); return { ...row, state: 'installed' } } + + const res = mockRes() + await ctrl.create(req({ body: { url: 'https://gitea.whitlocktech.com/x/uo.json' } }), res) + + assert.equal(res.statusCode, 201) + assert.equal(res.body.restartRequired, true) + assert.deepEqual(calls[0].hosts, ['gitea.whitlocktech.com'], 'the allowlist comes from the setting') + // Provenance is written HERE and nowhere else — the boot reconcile records a + // module with null source/sha256 and leaves what it is not given. + assert.equal(calls[1].source, 'https://gitea.whitlocktech.com/x/uo.json') + assert.equal(calls[1].sha256, 'b'.repeat(64)) + assert.equal(logged[0].action, 'module.install') +}) + +test('an install refusal is reported to the operator, with its own status', async () => { + install.install = async () => { + const err = new Error('"evil.net" is not an allowed module source host') + err.name = 'InstallError' + err.status = 400 + throw err + } + + const res = mockRes() + await ctrl.create(req({ body: { url: 'https://evil.net/x.json' } }), res) + + assert.equal(res.statusCode, 400) + // The message is the useful part: the operator pasted a URL and needs to know + // what was wrong with what came back. + assert.match(res.body.message, /not an allowed module source host/) + assert.equal(logged.length, 0, 'a refused install is not an audit-log entry') +}) + +test('an unreachable host is a 502, not a 400', async () => { + install.install = async () => { + const err = new Error('could not reach x: timeout') + err.name = 'InstallError' + err.status = 502 + throw err + } + + const res = mockRes() + await ctrl.create(req({ body: { url: 'https://gitea.whitlocktech.com/x.json' } }), res) + assert.equal(res.statusCode, 502) +}) + +test('an unexpected failure is a 500 and does not leak its message', async () => { + install.install = async () => { throw new Error('ENOENT /some/internal/path') } + + const res = mockRes() + await ctrl.create(req({ body: { url: 'https://gitea.whitlocktech.com/x.json' } }), res) + + assert.equal(res.statusCode, 500) + assert.equal(res.body.message, 'Internal Server Error') +}) + +// ── enable / disable ─────────────────────────────────────────────────────── + +test('enable moves the row and does NOT touch the loader', async () => { + // The decision-3 invariant. Re-enabling cannot restart a module: its + // onShutdown has run, and MODULE_API.md has never promised onBoot is safe to + // run twice. Flipping the record would put it back on the nav with a + // torn-down world behind it. + let setStateCalled = false + loader.setState = () => { setStateCalled = true } + modules.enable = async (id) => ({ id, state: 'enabled' }) + + const res = mockRes() + await ctrl.enable(req({ params: { id: 'uo' } }), res) + + assert.equal(res.body.module.state, 'enabled') + assert.equal(res.body.restartRequired, true) + assert.equal(setStateCalled, false, 'enable must not move the in-memory record') + assert.equal(logged[0].action, 'module.enable') + loader.setState = originals.loader.setState +}) + +test('enabling a module with no row is a 404', async () => { + modules.enable = async () => null + const res = mockRes() + await ctrl.enable(req({ params: { id: 'ghost' } }), res) + assert.equal(res.statusCode, 404) +}) + +test('an illegal transition is a 409, not a 500', async () => { + modules.enable = async () => { + const err = new Error("module 'uo': cannot move from 'x' to 'enabled'") + err.name = 'ModuleStateError' + throw err + } + const res = mockRes() + await ctrl.enable(req({ params: { id: 'uo' } }), res) + assert.equal(res.statusCode, 409) +}) + +test('disable stops the module and reports whether the hook ran', async () => { + const calls = [] + modules.get = async (id) => ({ id, state: 'started' }) + lifecycle.stop = async (id) => { calls.push(id); return { stopped: true, error: null } } + + const res = mockRes() + await ctrl.disable(req({ params: { id: 'uo' } }), res) + + assert.deepEqual(calls, ['uo']) + assert.equal(res.body.stopped, true) + // No restart: this is the one action that takes effect immediately, and it is + // the one an operator reaches for when something is going wrong. + assert.equal(res.body.restartRequired, undefined) + assert.equal(logged[0].action, 'module.disable') +}) + +test('a shutdown hook that failed is reported rather than swallowed', async () => { + modules.get = async (id) => ({ id, state: 'started' }) + lifecycle.stop = async () => ({ stopped: false, error: 'socket would not close' }) + + const res = mockRes() + await ctrl.disable(req({ params: { id: 'uo' } }), res) + + // It IS disabled either way; the operator should be told it did not close + // cleanly while they still have the logs in front of them. + assert.equal(res.statusCode, 200) + assert.match(res.body.shutdownError, /socket would not close/) +}) + +// ── uninstall and purge ──────────────────────────────────────────────────── + +test('uninstall purges BEFORE it removes the directory', async () => { + // The ordering that makes decision 5 work at all: purge.sql is a file inside + // the directory being deleted. Swap these two and the endpoint still answers + // 200 and deletes nothing. + const order = [] + modules.get = async (id) => ({ id, state: 'started' }) + install.isInstalled = () => true + install.purgeFile = () => '/modules/uo/server/db/purge.sql' + schema.runPurge = async () => { order.push('purge'); return 12 } + lifecycle.stop = async () => { order.push('stop'); return { stopped: true, error: null } } + install.removeDir = async () => { order.push('removeDir'); return true } + modules.remove = async () => { order.push('removeRow') } + + const res = mockRes() + await ctrl.remove(req({ params: { id: 'uo' }, query: { purge: 'true' } }), res) + + assert.deepEqual(order, ['purge', 'stop', 'removeDir', 'removeRow']) + assert.equal(res.body.purged, 12) + assert.equal(res.body.restartRequired, true) + assert.equal(logged[0].action, 'module.purge') +}) + +test('a plain uninstall keeps the row and does not purge', async () => { + const order = [] + modules.get = async (id) => ({ id, state: 'started' }) + install.isInstalled = () => true + schema.runPurge = async () => { order.push('purge'); return 1 } + lifecycle.stop = async () => { order.push('stop'); return { stopped: true, error: null } } + install.removeDir = async () => { order.push('removeDir'); return true } + modules.remove = async () => { order.push('removeRow') } + + const res = mockRes() + await ctrl.remove(req({ params: { id: 'uo' } }), res) + + // §2.5's default: the directory goes, the data stays, and the disabled row is + // what keeps the retained data visible and the module reinstallable. + assert.deepEqual(order, ['stop', 'removeDir']) + assert.equal(res.body.purged, null) + assert.equal(logged[0].action, 'module.uninstall') +}) + +test('asking to purge a module that ships no purge.sql refuses instead of pretending', async () => { + modules.get = async (id) => ({ id, state: 'started' }) + install.isInstalled = () => true + install.purgeFile = () => null + let removed = false + install.removeDir = async () => { removed = true; return true } + + const res = mockRes() + await ctrl.remove(req({ params: { id: 'uo' }, query: { purge: 'true' } }), res) + + assert.equal(res.statusCode, 400) + assert.match(res.body.message, /ships no purge.sql/) + // Nothing happened. The operator asked for the module AND its data to go; the + // data cannot go, so doing half of it silently would be the worst answer. + assert.equal(removed, false) +}) + +test('uninstalling something that is neither on the volume nor in a row is a 404', async () => { + modules.get = async () => null + install.isInstalled = () => false + + const res = mockRes() + await ctrl.remove(req({ params: { id: 'ghost' } }), res) + assert.equal(res.statusCode, 404) +}) + +test('standalone purge refuses while the module is still running', async () => { + // Dropping the tables under a module that is still serving leaves it answering + // out of a world that no longer exists. Disabling first is one click. + modules.get = async (id) => ({ id, state: 'started' }) + let ran = false + schema.runPurge = async () => { ran = true; return 1 } + + const res = mockRes() + await ctrl.purge(req({ params: { id: 'uo' } }), res) + + assert.equal(res.statusCode, 409) + assert.match(res.body.message, /Disable this module before purging/) + assert.equal(ran, false) +}) + +test('standalone purge runs on a disabled module', async () => { + modules.get = async (id) => ({ id, state: 'disabled' }) + install.purgeFile = () => '/modules/uo/server/db/purge.sql' + schema.runPurge = async () => 7 + + const res = mockRes() + await ctrl.purge(req({ params: { id: 'uo' } }), res) + + assert.equal(res.body.purged, 7) + assert.equal(logged[0].action, 'module.purge') +}) + +// ── the allowlist ────────────────────────────────────────────────────────── + +test('setSources stores a normalised list and audits the change', async () => { + let stored = null + settings.set = async (key, value) => { stored = { key, value } } + + const res = mockRes() + await ctrl.setSources(req({ body: { hosts: 'Gitea.Example.com, releases.example.org' } }), res) + + assert.deepEqual(res.body.sourceHosts, ['gitea.example.com', 'releases.example.org']) + assert.equal(stored.key, ctrl.HOSTS_KEY) + assert.equal(stored.value, 'gitea.example.com,releases.example.org') + // Before AND after: this setting decides what code the site will execute, so + // the audit entry has to say what it used to be. + assert.equal(logged[0].action, 'module.sources') + assert.deepEqual(logged[0].detail.before, ['gitea.whitlocktech.com']) +}) + +test('setSources refuses anything that is not a bare hostname', async () => { + let stored = false + settings.set = async () => { stored = true } + + for (const bad of ['https://x.com', 'x.com/path', 'x.com:8443', '*.x.com', 'x_y.com']) { + const res = mockRes() + // eslint-disable-next-line no-await-in-loop + await ctrl.setSources(req({ body: { hosts: bad } }), res) + assert.equal(res.statusCode, 400, `${bad} should be refused`) + } + assert.equal(stored, false) +}) + +test('an empty allowlist is storable, and means no installs', async () => { + // Not a wildcard, and not an error: "nothing may be installed" is a position + // an operator is entitled to take. + let stored = null + settings.set = async (key, value) => { stored = value } + + const res = mockRes() + await ctrl.setSources(req({ body: { hosts: '' } }), res) + + assert.equal(res.statusCode, 200) + assert.deepEqual(res.body.sourceHosts, []) + assert.equal(stored, '') +}) + +// ── restart ──────────────────────────────────────────────────────────────── + +test('restart answers before it signals, and signals its own process', async () => { + // It raises SIGTERM rather than calling the shutdown path directly, so that + // server.js's handler stays the ONE graceful-shutdown path and this route + // cannot drift from it. + const originalKill = process.kill + const signals = [] + process.kill = (pid, signal) => { signals.push({ pid, signal }) } + + try { + const res = mockRes() + ctrl.restart(req(), res) + + // Answered synchronously: once the signal lands there is no listener left to + // flush a response through, so the operator would be told nothing. + assert.equal(res.statusCode, 202) + assert.equal(res.body.restarting, true) + + await new Promise((resolve) => setTimeout(resolve, 400)) + assert.deepEqual(signals, [{ pid: process.pid, signal: 'SIGTERM' }]) + assert.equal(logged[0].action, 'module.restart') + } finally { + process.kill = originalKill + } +}) + +test('a failure to write the audit entry does not cancel the restart', async () => { + const originalKill = process.kill + const signals = [] + process.kill = (pid, signal) => { signals.push(signal) } + activity.log = async () => { throw new Error('database is gone') } + + try { + ctrl.restart(req(), mockRes()) + await new Promise((resolve) => setTimeout(resolve, 400)) + assert.deepEqual(signals, ['SIGTERM']) + } finally { + process.kill = originalKill + } +}) diff --git a/server/test/moduleArchive.test.js b/server/test/moduleArchive.test.js new file mode 100644 index 0000000..a5fc521 --- /dev/null +++ b/server/test/moduleArchive.test.js @@ -0,0 +1,252 @@ +// modules/archive.js — the hardened bundle extractor (MODULE_SYSTEM.md §2.7.2). +// +// Every rejection case is exercised against a REAL archive rather than a mocked +// tar parser, because the thing being tested is what the parser reports for a +// given sequence of bytes. A fake that returns `{type: 'SymbolicLink'}` proves +// only that the `if` is spelled correctly. +// +// The hostile archives are written here as raw ustar headers instead of being +// produced with `tar`, for two reasons that both bit during this slice: +// +// 1. `ln -s` needs a privilege Windows does not hand out by default, so a +// symlink fixture built with the shell is a fixture that silently is not +// one, and the test passes for the wrong reason on the machine most of this +// work happens on. +// 2. GNU tar will not emit `../escape` or `/etc/passwd` as a member name — it +// strips them and tells you so. The archives worth defending against are +// exactly the ones a cooperative archiver refuses to produce. +// +// A ustar header is 512 bytes of fixed-offset fields, so writing one is less +// code than persuading a tool to misbehave. + +const test = require('node:test') +const assert = require('node:assert/strict') +const fs = require('fs') +const os = require('os') +const path = require('path') +const zlib = require('zlib') + +const archive = require('../src/modules/archive') + +// ── A minimal ustar writer ───────────────────────────────────────────────── + +const BLOCK = 512 + +function octal(value, width) { + // ustar numeric fields are NUL-terminated octal, right-aligned with zeros. + return Number(value).toString(8).padStart(width - 1, '0') + '\0' +} + +/** + * One 512-byte header plus its padded data. + * + * @param {object} entry + * @param {string} entry.name member path + * @param {string} [entry.type] '0' file · '5' dir · '2' symlink · '1' hardlink + * · '3' char dev · '6' FIFO + * @param {string} [entry.linkname] target, for the link types + * @param {string} [entry.body] file contents + * @param {number} [entry.size] declared size — defaults to the body's, and + * may be set independently to build a header + * that lies about its payload + */ +function member({ name, type = '0', linkname = '', body = '', size = null }) { + const header = Buffer.alloc(BLOCK, 0) + const data = Buffer.from(body, 'utf8') + const declared = size === null ? data.length : size + + header.write(name, 0, 100, 'utf8') + header.write(octal(0o644, 8), 100, 8, 'ascii') // mode + header.write(octal(0, 8), 108, 8, 'ascii') // uid + header.write(octal(0, 8), 116, 8, 'ascii') // gid + header.write(octal(declared, 12), 124, 12, 'ascii') + header.write(octal(0, 12), 136, 12, 'ascii') // mtime + header.write(' ', 148, 8, 'ascii') // checksum field is spaces while summing + header.write(type, 156, 1, 'ascii') + header.write(linkname, 157, 100, 'utf8') + header.write('ustar\0', 257, 6, 'ascii') + header.write('00', 263, 2, 'ascii') + + let sum = 0 + for (const byte of header) sum += byte + header.write(`${sum.toString(8).padStart(6, '0')}\0 `, 148, 8, 'ascii') + + const padding = Buffer.alloc((BLOCK - (data.length % BLOCK)) % BLOCK, 0) + return Buffer.concat([header, data, padding]) +} + +/** Gzip a set of members into a .tar.gz on disk, and return its path. */ +function writeArchive(dir, filename, members) { + const tarball = Buffer.concat([ + ...members.map(member), + Buffer.alloc(BLOCK * 2, 0), // two zero blocks end the archive + ]) + const file = path.join(dir, filename) + fs.writeFileSync(file, zlib.gzipSync(tarball)) + return file +} + +/** A well-formed bundle: one top-level directory, ordinary files inside it. */ +function goodMembers(root = 'module-uo-1.0.0') { + return [ + { name: `${root}/`, type: '5' }, + { name: `${root}/module.json`, body: '{"id":"uo","name":"UO","version":"1.0.0"}' }, + { name: `${root}/server/`, type: '5' }, + { name: `${root}/server/index.js`, body: 'module.exports = () => {}\n' }, + ] +} + +// One scratch directory for the whole file, removed at the end. +const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-archive-')) +test.after(() => fs.rmSync(tmp, { recursive: true, force: true })) + +/** Assert that inspecting `members` fails, and that the message says why. */ +async function rejects(name, members, matcher) { + const file = writeArchive(tmp, `${name}.tar.gz`, members) + await assert.rejects( + () => archive.inspect(file), + (err) => { + assert.equal(err.name, 'ArchiveError', `expected an ArchiveError, got ${err.name}: ${err.message}`) + assert.match(err.message, matcher) + return true + }, + ) +} + +// ── What a good bundle does ──────────────────────────────────────────────── + +test('inspect accepts a well-formed bundle and reports its single root', async () => { + const file = writeArchive(tmp, 'good.tar.gz', goodMembers()) + const stats = await archive.inspect(file) + + assert.equal(stats.root, 'module-uo-1.0.0') + assert.equal(stats.entries, 4) + assert.ok(stats.bytes > 0) +}) + +test('extract strips the top level, so the bundle lands as the module id', async () => { + // The wrapper directory is the publisher's naming (module-uo's release + // workflow packs `module-uo-/`); the directory it lands in is core's, + // and has to be the id the loader scans for. + const file = writeArchive(tmp, 'strip.tar.gz', goodMembers()) + const dest = path.join(tmp, 'unpacked-uo') + + await archive.unpack(file, dest) + + assert.ok(fs.existsSync(path.join(dest, 'module.json')), 'module.json should be at the root of the destination') + assert.ok(fs.existsSync(path.join(dest, 'server', 'index.js'))) + assert.ok(!fs.existsSync(path.join(dest, 'module-uo-1.0.0')), 'the wrapper directory should not survive') +}) + +test('extract refuses a destination that already exists', async () => { + const file = writeArchive(tmp, 'exists.tar.gz', goodMembers()) + const dest = path.join(tmp, 'already-there') + fs.mkdirSync(dest) + + await assert.rejects(() => archive.extract(file, dest), /already exists/) +}) + +// ── What a hostile bundle does ───────────────────────────────────────────── + +test('an absolute member path is refused', async () => { + await rejects('absolute', [ + { name: 'mod/', type: '5' }, + { name: '/etc/cron.d/pwned', body: '* * * * * root sh\n' }, + ], /absolute/) +}) + +test('a drive-absolute member path is refused', async () => { + // Its own node-tar advisory, and invisible to a leading-slash check. + await rejects('drive', [ + { name: 'mod/', type: '5' }, + { name: 'C:\\Windows\\Temp\\pwned', body: 'x' }, + ], /backslash|drive-absolute/) +}) + +test('an upward-escaping member path is refused', async () => { + await rejects('escape', [ + { name: 'mod/', type: '5' }, + { name: 'mod/../../../etc/passwd', body: 'root::0:0\n' }, + ], /escapes upward/) +}) + +test('a symlink member is refused, and the message names the type', async () => { + await rejects('symlink', [ + { name: 'mod/', type: '5' }, + { name: 'mod/passwd', type: '2', linkname: '/etc/passwd' }, + ], /SymbolicLink/) +}) + +test('a hardlink member is refused', async () => { + // The single most-published node-tar escape primitive. Refusing the type + // outright is what keeps this file from depending on the library getting + // hardlink containment right. + await rejects('hardlink', [ + { name: 'mod/', type: '5' }, + { name: 'mod/shadow', type: '1', linkname: '../../../etc/shadow' }, + ], /Link/) +}) + +test('a device node is refused', async () => { + await rejects('device', [ + { name: 'mod/', type: '5' }, + { name: 'mod/zero', type: '3', linkname: '' }, + ], /may only contain files and directories/) +}) + +test('a FIFO is refused', async () => { + await rejects('fifo', [ + { name: 'mod/', type: '5' }, + { name: 'mod/pipe', type: '6' }, + ], /may only contain files and directories/) +}) + +test('two top-level directories are refused', async () => { + await rejects('two-roots', [ + { name: 'mod-a/', type: '5' }, + { name: 'mod-a/module.json', body: '{}' }, + { name: 'mod-b/', type: '5' }, + { name: 'mod-b/module.json', body: '{}' }, + ], /exactly one top-level directory, found 2/) +}) + +test('a bundle whose declared sizes exceed the cap is refused before it is read to the end', async () => { + // The header lies: it declares a gigabyte and carries nothing. That is the + // decompression-bomb shape, and the point is that inspect() decides on the + // DECLARED size without ever materialising the payload. + await rejects('bomb', [ + { name: 'mod/', type: '5' }, + { name: 'mod/big', size: archive.MAX_BYTES + 1, body: '' }, + ], /unpacks to more than/) +}) + +test('an empty archive is refused', async () => { + await rejects('empty', [], /empty/) +}) + +// ── The path check on its own ────────────────────────────────────────────── +// +// pathProblem is exported so the cases that are awkward to express as archive +// bytes can still be asserted directly. + +test('pathProblem accepts ordinary bundle paths', () => { + for (const ok of ['mod/module.json', 'mod/server/router/x.js', 'mod/a.b-c_d/e.js']) { + assert.equal(archive.pathProblem(ok), null, `${ok} should be accepted`) + } +}) + +test('pathProblem rejects the escape shapes', () => { + assert.match(archive.pathProblem('/etc/passwd'), /absolute/) + assert.match(archive.pathProblem('C:/Windows/x'), /drive-absolute/) + assert.match(archive.pathProblem('mod/../../x'), /escapes upward/) + assert.match(archive.pathProblem('..'), /escapes upward/) + assert.match(archive.pathProblem('mod\\x'), /backslash/) + assert.match(archive.pathProblem('mod/\0/x'), /NUL byte/) +}) + +test('pathProblem does not reject a filename that merely contains two dots', () => { + // `..` is a SEGMENT, not a substring — a file called `version..js` is fine, + // and a check written with `includes('..')` would refuse it. + assert.equal(archive.pathProblem('mod/version..js'), null) + assert.equal(archive.pathProblem('mod/..hidden'), null) +}) diff --git a/server/test/moduleInstall.test.js b/server/test/moduleInstall.test.js new file mode 100644 index 0000000..ef3a9d2 --- /dev/null +++ b/server/test/moduleInstall.test.js @@ -0,0 +1,448 @@ +// modules/install.js — fetching, verifying and placing a module bundle. +// +// Phase 4, slice 1 of MODULE_SYSTEM.md §2.7.2. The rules under test are all +// refusals, and each one is a step an attacker would otherwise walk through: +// a non-https URL, a host that is not allowed, a REDIRECT to a host that is not +// allowed, a body larger than declared, a hash that does not match, an archive +// that does not agree with the manifest about what it is. +// +// `fetchImpl` is injected rather than a TLS server being stood up, for the same +// reason `replayFragments` takes a `query`: what is being tested is what this +// file decides about a response, and a real server would mostly test Node's +// certificate handling. The responses below are real `Response` objects with +// real bodies, so the streaming, the hashing and the byte cap are exercised for +// real — only the transport is stubbed. +process.env.DB_HOST = '127.0.0.1' +process.env.DB_PORT = '59999' + +const crypto = require('crypto') +const fs = require('fs') +const os = require('os') +const path = require('path') +const zlib = require('zlib') + +const { test, beforeEach, after } = require('node:test') +const assert = require('node:assert/strict') + +const db = require('../src/utils/db') + +after(() => db.close()) + +const HOSTS = ['releases.example.com'] +const MANIFEST_URL = 'https://releases.example.com/mod/uo-1.0.0.json' + +let tmpRoot +let install + +/** + * A fresh install.js bound to a fresh modules directory. + * + * install.js reads `loader.dir()`, which is resolved once at require time from + * MODULES_DIR — so both have to be re-required per test, exactly as + * moduleLifecycle.test.js does. + */ +function freshInstall(dir) { + process.env.MODULES_DIR = dir + delete require.cache[require.resolve('../src/modules/loader')] + delete require.cache[require.resolve('../src/modules/install')] + // eslint-disable-next-line global-require + return require('../src/modules/install') +} + +// ── Building a bundle to serve ───────────────────────────────────────────── + +const BLOCK = 512 + +function octal(value, width) { + return Number(value).toString(8).padStart(width - 1, '0') + '\0' +} + +function member({ name, type = '0', body = '' }) { + const header = Buffer.alloc(BLOCK, 0) + const data = Buffer.from(body, 'utf8') + header.write(name, 0, 100, 'utf8') + header.write(octal(0o644, 8), 100, 8, 'ascii') + header.write(octal(0, 8), 108, 8, 'ascii') + header.write(octal(0, 8), 116, 8, 'ascii') + header.write(octal(data.length, 12), 124, 12, 'ascii') + header.write(octal(0, 12), 136, 12, 'ascii') + header.write(' ', 148, 8, 'ascii') + header.write(type, 156, 1, 'ascii') + header.write('ustar\0', 257, 6, 'ascii') + header.write('00', 263, 2, 'ascii') + let sum = 0 + for (const byte of header) sum += byte + header.write(`${sum.toString(8).padStart(6, '0')}\0 `, 148, 8, 'ascii') + const padding = Buffer.alloc((BLOCK - (data.length % BLOCK)) % BLOCK, 0) + return Buffer.concat([header, data, padding]) +} + +/** A bundle tarball whose root directory is named the way a release names it. */ +function bundle({ id = 'uo', version = '1.0.0', extra = [] } = {}) { + const root = `module-${id}-${version}` + return zlib.gzipSync(Buffer.concat([ + member({ name: `${root}/`, type: '5' }), + member({ + name: `${root}/module.json`, + body: JSON.stringify({ id, name: 'Ultima Online', version, coreApi: '^1.0.0', server: 'server/index.js' }), + }), + member({ name: `${root}/server/`, type: '5' }), + member({ name: `${root}/server/index.js`, body: 'module.exports = () => {}\n' }), + ...extra.map(member), + Buffer.alloc(BLOCK * 2, 0), + ])) +} + +const sha256 = (buf) => crypto.createHash('sha256').update(buf).digest('hex') + +/** + * A fake transport serving one manifest and one artifact. + * + * `routes` maps an absolute URL to either a Buffer/string body or + * `{ status, location }` for a redirect, so a test can describe exactly what the + * remote host does without describing how it does it. + */ +function fakeFetch(routes) { + const seen = [] + const impl = async (url) => { + const href = String(url) + seen.push(href) + const route = routes[href] + if (route === undefined) return new Response('not found', { status: 404 }) + if (route && route.status) { + return new Response(null, { + status: route.status, + headers: route.location ? { location: route.location } : {}, + }) + } + return new Response(route, { status: 200 }) + } + impl.seen = seen + return impl +} + +/** The manifest a release publishes, with whatever a test wants to change. */ +function manifestFor(tarball, overrides = {}) { + return JSON.stringify({ + schema: 1, + id: 'uo', + name: 'Ultima Online', + version: '1.0.0', + coreApi: '^1.0.0', + artifact: 'uo-1.0.0.tar.gz', + url: 'https://releases.example.com/mod/uo-1.0.0.tar.gz', + sha256: sha256(tarball), + size: tarball.length, + ...overrides, + }) +} + +/** The happy-path pair: a valid manifest and the artifact it describes. */ +function goodRoutes(options = {}) { + const tarball = bundle(options) + return { + tarball, + routes: { + [MANIFEST_URL]: manifestFor(tarball, options.manifest || {}), + 'https://releases.example.com/mod/uo-1.0.0.tar.gz': tarball, + }, + } +} + +beforeEach(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'rg-install-')) + install = freshInstall(tmpRoot) +}) + +// ── The allowlist ────────────────────────────────────────────────────────── + +test('parseHosts accepts comma and whitespace separation, and lower-cases', () => { + assert.deepEqual(install.parseHosts('a.com, B.com\nc.com'), ['a.com', 'b.com', 'c.com']) + assert.deepEqual(install.parseHosts(''), []) + assert.deepEqual(install.parseHosts(null), []) +}) + +test('an empty allowlist forbids everything rather than allowing everything', () => { + // The direction matters: a setting someone blanks by accident must stop + // installs, not open the door to every host on the internet. + assert.throws(() => install.checkUrl('https://releases.example.com/x.json', []), /no module source hosts are allowed/) +}) + +test('only https may be installed from', () => { + // The sha256 is no help against a plaintext fetch: whoever can rewrite the + // artifact in flight can rewrite the manifest that declares its hash. + assert.throws(() => install.checkUrl('http://releases.example.com/x.json', HOSTS), /only https/) + assert.throws(() => install.checkUrl('file:///etc/passwd', HOSTS), /only https/) +}) + +test('a host that is not on the allowlist is refused, and the message says which are', () => { + assert.throws( + () => install.checkUrl('https://evil.example.net/x.json', HOSTS), + /"evil.example.net" is not an allowed module source host \(allowed: releases.example.com\)/, + ) +}) + +test('the allowlist is matched on the host, not on a substring of the URL', () => { + // `https://evil.com/?x=releases.example.com` must not pass, and neither must a + // subdomain nobody listed. + assert.throws(() => install.checkUrl('https://evil.com/?releases.example.com', HOSTS), /not an allowed/) + assert.throws(() => install.checkUrl('https://sub.releases.example.com/x', HOSTS), /not an allowed/) + assert.throws(() => install.checkUrl('https://releases.example.com.evil.net/x', HOSTS), /not an allowed/) +}) + +// ── Redirects ────────────────────────────────────────────────────────────── + +test('a redirect is followed, and re-checked against the allowlist', async () => { + const { tarball } = goodRoutes() + const impl = fakeFetch({ + [MANIFEST_URL]: { status: 302, location: 'https://releases.example.com/cdn/uo-1.0.0.json' }, + 'https://releases.example.com/cdn/uo-1.0.0.json': manifestFor(tarball), + }) + + const manifest = await install.fetchManifest(MANIFEST_URL, HOSTS, impl) + + assert.equal(manifest.id, 'uo') + assert.deepEqual(impl.seen, [MANIFEST_URL, 'https://releases.example.com/cdn/uo-1.0.0.json']) +}) + +test('a redirect to a host that is not allowed is refused', async () => { + // The hole the allowlist exists to close. `fetch`'s own redirect following + // would check the first hop and then go wherever it was pointed — which is + // why get() follows them by hand. + const impl = fakeFetch({ + [MANIFEST_URL]: { status: 302, location: 'http://169.254.169.254/latest/meta-data/' }, + }) + + await assert.rejects(() => install.fetchManifest(MANIFEST_URL, HOSTS, impl), /only https/) +}) + +test('a redirect loop is bounded rather than followed forever', async () => { + const impl = fakeFetch({ + [MANIFEST_URL]: { status: 302, location: MANIFEST_URL }, + }) + + await assert.rejects(() => install.fetchManifest(MANIFEST_URL, HOSTS, impl), /too many redirects/) +}) + +// ── The install manifest ─────────────────────────────────────────────────── + +test('a manifest that is not JSON is refused with a readable reason', async () => { + const impl = fakeFetch({ [MANIFEST_URL]: 'this is not json' }) + await assert.rejects(() => install.fetchManifest(MANIFEST_URL, HOSTS, impl), /not valid JSON/) +}) + +test('a manifest with an invalid module id is refused', async () => { + // The loader would refuse to scan a directory called `../etc`, but the point + // is that one is never created: the id becomes a path segment. + for (const id of ['../etc', 'UO', '', 'x', 'a'.repeat(40)]) { + const impl = fakeFetch({ [MANIFEST_URL]: JSON.stringify({ id, name: 'n', version: '1', sha256: 'a'.repeat(64) }) }) + // eslint-disable-next-line no-await-in-loop + await assert.rejects(() => install.fetchManifest(MANIFEST_URL, HOSTS, impl), /invalid module id/, `id ${JSON.stringify(id)}`) + } +}) + +test('a manifest with no usable sha256 is refused', async () => { + const impl = fakeFetch({ + [MANIFEST_URL]: JSON.stringify({ id: 'uo', name: 'n', version: '1.0.0', sha256: 'not-a-hash' }), + }) + await assert.rejects(() => install.fetchManifest(MANIFEST_URL, HOSTS, impl), /no valid sha256/) +}) + +test('the artifact URL is resolved relative to the manifest when it carries no absolute one', async () => { + const { tarball } = goodRoutes() + const impl = fakeFetch({ [MANIFEST_URL]: manifestFor(tarball, { url: undefined }) }) + + const manifest = await install.fetchManifest(MANIFEST_URL, HOSTS, impl) + + // Release assets sit beside their manifest, so a manifest that travelled + // without its absolute URL still resolves to the right place. + assert.equal(manifest.artifactUrl, 'https://releases.example.com/mod/uo-1.0.0.tar.gz') +}) + +// ── The install ──────────────────────────────────────────────────────────── + +test('a good bundle installs into modules/, stripped of its wrapper', async () => { + const { routes, tarball } = goodRoutes() + + const result = await install.install({ url: MANIFEST_URL, hosts: HOSTS, fetchImpl: fakeFetch(routes) }) + + assert.equal(result.id, 'uo') + assert.equal(result.version, '1.0.0') + assert.equal(result.sha256, sha256(tarball)) + assert.equal(result.source, MANIFEST_URL) + assert.equal(result.replaced, false) + + // The directory is named for the module id, not for the archive's root — the + // loader scans for the former and the publisher chose the latter. + const dir = path.join(tmpRoot, 'uo') + assert.ok(fs.existsSync(path.join(dir, 'module.json'))) + assert.ok(fs.existsSync(path.join(dir, 'server', 'index.js'))) + assert.ok(!fs.existsSync(path.join(tmpRoot, 'module-uo-1.0.0'))) +}) + +test('a hash that does not match refuses the install and writes nothing', async () => { + const { routes } = goodRoutes() + // The bytes are fine; the manifest lies about them. Which is the same thing an + // artifact swapped after publication looks like. + routes[MANIFEST_URL] = manifestFor(Buffer.from('different'), {}) + + await assert.rejects( + () => install.install({ url: MANIFEST_URL, hosts: HOSTS, fetchImpl: fakeFetch(routes) }), + /does not match the sha256/, + ) + assert.deepEqual(fs.readdirSync(tmpRoot), [], 'nothing may be left on the volume') +}) + +test('a bundle whose module.json disagrees with the manifest is refused', async () => { + // A manifest promising `uo` and delivering something else would otherwise be + // installed into `modules/uo/` under a name it is not. + const tarball = bundle({ id: 'rust', version: '1.0.0' }) + const routes = { + [MANIFEST_URL]: manifestFor(tarball), + 'https://releases.example.com/mod/uo-1.0.0.tar.gz': tarball, + } + + await assert.rejects( + () => install.install({ url: MANIFEST_URL, hosts: HOSTS, fetchImpl: fakeFetch(routes) }), + /declares module id "rust" but the install manifest promised "uo"/, + ) + assert.deepEqual(fs.readdirSync(tmpRoot), []) +}) + +test('a bundle whose version disagrees with the manifest is refused', async () => { + const tarball = bundle({ id: 'uo', version: '9.9.9' }) + const routes = { + [MANIFEST_URL]: manifestFor(tarball), + 'https://releases.example.com/mod/uo-1.0.0.tar.gz': tarball, + } + + await assert.rejects( + () => install.install({ url: MANIFEST_URL, hosts: HOSTS, fetchImpl: fakeFetch(routes) }), + /declares version "9.9.9"/, + ) +}) + +test('an artifact whose length differs from the declared size is refused', async () => { + const tarball = bundle() + const routes = { + [MANIFEST_URL]: manifestFor(tarball, { size: tarball.length + 10 }), + 'https://releases.example.com/mod/uo-1.0.0.tar.gz': tarball, + } + + await assert.rejects( + () => install.install({ url: MANIFEST_URL, hosts: HOSTS, fetchImpl: fakeFetch(routes) }), + /but the manifest declares/, + ) +}) + +test('a hostile archive is refused, and nothing of it reaches the volume', async () => { + // The case node-tar alone does NOT cover: it throws on the escaping member, + // but only once it reaches it — the members before it are already on disk. + // archive.inspect() decides before extract() runs, and the unpack happens in a + // scratch directory that is removed either way. + const tarball = bundle({ + extra: [ + { name: 'module-uo-1.0.0/../../ESCAPED.txt', body: 'escaped' }, + ], + }) + const routes = { + [MANIFEST_URL]: manifestFor(tarball), + 'https://releases.example.com/mod/uo-1.0.0.tar.gz': tarball, + } + + await assert.rejects( + () => install.install({ url: MANIFEST_URL, hosts: HOSTS, fetchImpl: fakeFetch(routes) }), + /escapes upward/, + ) + assert.deepEqual(fs.readdirSync(tmpRoot), [], 'no scratch directory, no partial module, no escape') +}) + +test('an upgrade replaces the directory and reports that it did', async () => { + const first = goodRoutes() + await install.install({ url: MANIFEST_URL, hosts: HOSTS, fetchImpl: fakeFetch(first.routes) }) + fs.writeFileSync(path.join(tmpRoot, 'uo', 'STALE.txt'), 'from the old version') + + const second = goodRoutes({ version: '2.0.0', manifest: { version: '2.0.0' } }) + second.routes[MANIFEST_URL] = manifestFor(second.tarball, { version: '2.0.0' }) + const result = await install.install({ url: MANIFEST_URL, hosts: HOSTS, fetchImpl: fakeFetch(second.routes) }) + + assert.equal(result.replaced, true) + assert.equal(result.version, '2.0.0') + // A replace, not a merge: a file the previous version left behind must not + // survive into the new one, or an upgrade quietly keeps dead code loadable. + assert.ok(!fs.existsSync(path.join(tmpRoot, 'uo', 'STALE.txt'))) + assert.deepEqual(fs.readdirSync(tmpRoot), ['uo'], 'the aside copy is cleaned up') +}) + +test('a failed upgrade leaves the previous version in place', async () => { + const first = goodRoutes() + await install.install({ url: MANIFEST_URL, hosts: HOSTS, fetchImpl: fakeFetch(first.routes) }) + + const badTarball = bundle({ id: 'uo', version: '2.0.0', extra: [{ name: 'evil/', type: '5' }] }) + await assert.rejects(() => install.install({ + url: MANIFEST_URL, + hosts: HOSTS, + fetchImpl: fakeFetch({ + [MANIFEST_URL]: manifestFor(badTarball, { version: '2.0.0' }), + 'https://releases.example.com/mod/uo-1.0.0.tar.gz': badTarball, + }), + })) + + // The installed module is untouched: the swap is the last step, so a bundle + // rejected before it never got near the live directory. + assert.ok(fs.existsSync(path.join(tmpRoot, 'uo', 'module.json'))) + assert.equal(JSON.parse(fs.readFileSync(path.join(tmpRoot, 'uo', 'module.json'), 'utf8')).version, '1.0.0') + assert.deepEqual(fs.readdirSync(tmpRoot), ['uo']) +}) + +// ── The volume ───────────────────────────────────────────────────────────── + +test('moduleDir refuses an id that is not one', () => { + for (const id of ['../escape', 'a/b', '', 'UO', '.']) { + assert.throws(() => install.moduleDir(id), /invalid module id/, `id ${JSON.stringify(id)}`) + } + assert.equal(install.moduleDir('uo'), path.join(tmpRoot, 'uo')) +}) + +test('isInstalled and removeDir report what they did', async () => { + const { routes } = goodRoutes() + assert.equal(install.isInstalled('uo'), false) + + await install.install({ url: MANIFEST_URL, hosts: HOSTS, fetchImpl: fakeFetch(routes) }) + assert.equal(install.isInstalled('uo'), true) + + assert.equal(await install.removeDir('uo'), true) + assert.equal(install.isInstalled('uo'), false) + // Removing what is not there is not an error — an uninstall of a module whose + // directory was already deleted by hand should still tidy up the row. + assert.equal(await install.removeDir('uo'), false) +}) + +test('purgeFile resolves the manifest declaration, and refuses one that escapes', async () => { + const dir = path.join(tmpRoot, 'uo') + fs.mkdirSync(path.join(dir, 'server', 'db'), { recursive: true }) + fs.writeFileSync(path.join(dir, 'server', 'db', 'purge.sql'), 'DROP TABLE IF EXISTS x;') + + const write = (purge) => fs.writeFileSync( + path.join(dir, 'module.json'), + JSON.stringify({ id: 'uo', name: 'UO', version: '1.0.0', purge }), + ) + + write('server/db/purge.sql') + assert.equal(install.purgeFile('uo'), path.join(dir, 'server', 'db', 'purge.sql')) + + // The same containment rule the loader applies to client.entry: a manifest may + // not point core at a file outside the module it belongs to. + write('../../../../etc/passwd') + assert.equal(install.purgeFile('uo'), null) + + // Declared but absent, and not declared at all, are both "nothing to run". + write('server/db/missing.sql') + assert.equal(install.purgeFile('uo'), null) + write(undefined) + assert.equal(install.purgeFile('uo'), null) +}) + +test('purgeFile is null for a module that is not on the volume', () => { + assert.equal(install.purgeFile('nothere'), null) +}) diff --git a/server/test/moduleLifecycle.test.js b/server/test/moduleLifecycle.test.js index b741548..434b0f3 100644 --- a/server/test/moduleLifecycle.test.js +++ b/server/test/moduleLifecycle.test.js @@ -125,6 +125,11 @@ function fakeModel(seed = []) { if (!row || row.state === 'disabled') return Object.assign(row, { state: 'startup_failed', failureStage: stage, failureReason: reason }) }, + async disable(id) { + calls.push(`disable:${id}`) + const row = rows.get(id) + if (row) Object.assign(row, { state: 'disabled', failureStage: null, failureReason: null }) + }, } return model } @@ -374,3 +379,151 @@ test('a hook that throws does not stop the ones behind it', async () => { await assert.doesNotReject(() => lifecycle.shutdown({ modules: loader })) assert.deepEqual(noted(file), ['shutdown:zzz', 'shutdown:aaa']) }) + +// ── stop(): the admin panel's Disable ────────────────────────────────────── +// +// Phase 4, §2.7.2 decision 3. Phase 2's disable moved a record and left the +// module running; the whole point of these tests is that it no longer does. + +test('stop runs that one module\'s onShutdown and leaves the others alone', async () => { + const file = path.join(tmpRoot, 'log.txt') + writeModule('aaa', { boot: '', shutdown: '', log: file }) + writeModule('bbb', { boot: '', shutdown: '', log: file }) + const loader = freshLoader(tmpRoot) + const model = fakeModel() + + await lifecycle.boot({ modules: loader, model }) + fs.writeFileSync(file, '') + + const result = await lifecycle.stop('aaa', { modules: loader, model }) + + assert.deepEqual(result, { stopped: true, error: null }) + // Only aaa. This is the difference from shutdown(), which runs everything. + assert.deepEqual(noted(file), ['shutdown:aaa']) + assert.equal(stateOf(loader, 'aaa').state, 'disabled') + assert.equal(stateOf(loader, 'bbb').state, 'started', 'the other module keeps running') + assert.equal(model.rows.get('aaa').state, 'disabled') +}) + +test('stop moves the record only after the hook has run', async () => { + // While onShutdown runs, the module is still `started` — the only state in + // which its routes and the world it is tearing down agree with each other. The + // module reports its own view of itself, so a record moved too early shows up + // here as `disabled` instead of `started`. + const file = path.join(tmpRoot, 'log.txt') + const dir = path.join(tmpRoot, 'aaa') + fs.mkdirSync(dir, { recursive: true }) + fs.writeFileSync(path.join(dir, 'module.json'), JSON.stringify({ + id: 'aaa', name: 'A', version: '1.0.0', coreApi: '^1.0.0', server: 'index.js', + })) + fs.writeFileSync(path.join(dir, 'index.js'), ` + module.exports = (ctx, api) => { + api.onBoot(async () => {}) + api.onShutdown(async () => { + const loader = require(${JSON.stringify(require.resolve('../src/modules/loader'))}) + const me = loader.list().find((m) => m.id === 'aaa') + require('fs').appendFileSync(${JSON.stringify(file)}, 'state-during-hook:' + me.state + ${JSON.stringify('\n')}) + }) + }`) + + const loader = freshLoader(tmpRoot) + await lifecycle.boot({ modules: loader, model: fakeModel() }) + await lifecycle.stop('aaa', { modules: loader, model: fakeModel([{ id: 'aaa', state: 'started' }]) }) + + assert.deepEqual(noted(file), ['state-during-hook:started']) + assert.equal(stateOf(loader, 'aaa').state, 'disabled') +}) + +test('a hook that throws does not prevent the disable', async () => { + // The opposite of the boot path's rule, deliberately. There, a failure means + // the module never became safe to use; here, the operator has asked for it to + // stop answering and a module that could not close cleanly is a reason to log + // loudly, not a reason to leave it serving. + writeModule('aaa', { boot: '', shutdown: 'throw new Error("socket stuck")' }) + const loader = freshLoader(tmpRoot) + const model = fakeModel() + + await lifecycle.boot({ modules: loader, model }) + const result = await lifecycle.stop('aaa', { modules: loader, model }) + + assert.equal(result.stopped, false) + assert.match(result.error, /socket stuck/) + assert.equal(stateOf(loader, 'aaa').state, 'disabled', 'disabled anyway') + assert.equal(model.rows.get('aaa').state, 'disabled') +}) + +test('a hook that hangs costs its budget, and the module is still disabled', async () => { + writeModule('aaa', { boot: '', shutdown: 'await new Promise(() => {})' }) + const loader = freshLoader(tmpRoot) + const model = fakeModel() + + await lifecycle.boot({ modules: loader, model }) + const started = Date.now() + const result = await lifecycle.stop('aaa', { modules: loader, model, budgetMs: 50 }) + + assert.equal(result.stopped, false) + assert.match(result.error, /budget/) + assert.equal(stateOf(loader, 'aaa').state, 'disabled') + assert.ok(Date.now() - started < 2000) +}) + +test('stopping a module with no onShutdown still disables it', async () => { + // A hookless module has nothing to run and must still stop answering, or the + // guard and the row disagree about what is serving. + writeModule('aaa', { boot: '' }) + const loader = freshLoader(tmpRoot) + const model = fakeModel() + + await lifecycle.boot({ modules: loader, model }) + const result = await lifecycle.stop('aaa', { modules: loader, model }) + + assert.deepEqual(result, { stopped: false, error: null }) + assert.equal(stateOf(loader, 'aaa').state, 'disabled') + assert.equal(model.rows.get('aaa').state, 'disabled') +}) + +test('stopping a module whose onBoot failed does not run its onShutdown', async () => { + // Same rule shutdownHooks() applies: a module that never finished warming up + // has a half-built world its onShutdown was not written for. It is still + // disabled — it just is not asked to tear anything down. + const file = path.join(tmpRoot, 'log.txt') + writeModule('aaa', { boot: 'throw new Error("no")', shutdown: '', log: file }) + const loader = freshLoader(tmpRoot) + const model = fakeModel() + + await lifecycle.boot({ modules: loader, model }) + const before = noted(file) + const result = await lifecycle.stop('aaa', { modules: loader, model }) + + assert.equal(result.stopped, false) + // Compared against what was already there rather than against an empty file: + // `noted` splits, so a blanked file reads as [''] and an emptiness assertion + // would pass for the wrong reason. + assert.deepEqual(noted(file), before, 'no new hook output') + assert.ok(!noted(file).includes('shutdown:aaa')) + assert.equal(stateOf(loader, 'aaa').state, 'disabled') +}) + +test('stopping an unknown id writes the row and does not throw', async () => { + // A row can exist for a module that is not on the volume, and disabling it is + // exactly what an operator would do about that. + writeModule('aaa', { boot: '' }) + const loader = freshLoader(tmpRoot) + const model = fakeModel([{ id: 'ghost', state: 'startup_failed' }]) + + await assert.doesNotReject(() => lifecycle.stop('ghost', { modules: loader, model })) + assert.equal(model.rows.get('ghost').state, 'disabled') +}) + +test('a row that will not write does not stop the module from being disabled', async () => { + // The same rule the boot path follows: bookkeeping failure is not the + // operation failing. The guard is what stops traffic, and it has already moved. + writeModule('aaa', { boot: '', shutdown: '' }) + const loader = freshLoader(tmpRoot) + const model = fakeModel() + await lifecycle.boot({ modules: loader, model }) + model.disable = async () => { throw new Error('database is gone') } + + await assert.doesNotReject(() => lifecycle.stop('aaa', { modules: loader, model })) + assert.equal(stateOf(loader, 'aaa').state, 'disabled') +}) diff --git a/server/test/moduleSchema.test.js b/server/test/moduleSchema.test.js index 66e714f..6dffd0e 100644 --- a/server/test/moduleSchema.test.js +++ b/server/test/moduleSchema.test.js @@ -25,7 +25,7 @@ const assert = require('node:assert/strict') const express = require('express') const db = require('../src/utils/db') -const { replayFragments } = require('../src/modules/schema') +const { replayFragments, runPurge } = require('../src/modules/schema') const { splitStatements } = require('../src/utils/sqlStatements') const { startApp } = require('./_helper') @@ -245,3 +245,66 @@ test('replay is skipped, not thrown, when no scan happened in this process', asy assert.deepEqual(rec.ran, []) assert.equal(loader.isLoaded(), false) }) + +// ── runPurge: the destructive twin ───────────────────────────────────────── +// +// Phase 4, slice 1. Same splitter, same pool, same serial execution as the +// replay above — pointed the other way. The two differences are deliberate and +// are what these tests are for. + +test('purge runs every statement in the file, in order', async () => { + const file = path.join(tmpRoot, 'purge.sql') + fs.writeFileSync(file, [ + '-- drop everything this module owns', + 'DROP TABLE IF EXISTS mod_b;', + 'DROP TABLE IF EXISTS mod_a;', + "DELETE FROM settings WHERE `key` = 'mod_thing';", + ].join('\n')) + const rec = recorder() + + const ran = await runPurge(file, { query: rec.query }) + + assert.equal(ran, 3) + assert.match(rec.ran[0], /DROP TABLE IF EXISTS mod_b/) + assert.match(rec.ran[2], /DELETE FROM settings/) +}) + +test('purge is not held to the fragment allowlist — DROP is the point of it', async () => { + // §2.6's leading-verb allowlist exists because a fragment replays on every + // boot. purge.sql never does, which is exactly why it is the one file a module + // may put a DROP in. + const file = path.join(tmpRoot, 'purge.sql') + fs.writeFileSync(file, 'DROP TABLE IF EXISTS mod_a;\nTRUNCATE TABLE mod_b;') + const rec = recorder() + + assert.equal(await runPurge(file, { query: rec.query }), 2) +}) + +test('purge THROWS on failure, unlike the replay', async () => { + // A replay failure is one module failing to start, which the site survives by + // 503ing it. A purge failure is an operator's explicit destructive request not + // having happened — reporting success would leave them believing data is gone + // when it is not. + const file = path.join(tmpRoot, 'purge.sql') + fs.writeFileSync(file, 'DROP TABLE IF EXISTS mod_a;\nDROP TABLE mod_missing;') + const query = async (sql) => { + if (sql.includes('mod_missing')) throw new Error("Unknown table 'mod_missing'") + } + + await assert.rejects( + () => runPurge(file, { query }), + // The message names WHICH statement stopped it, because a purge is not + // transactional — the ones before it have already committed and the operator + // needs to know where it got to. + /purge failed at statement 2 of 2: Unknown table 'mod_missing'/, + ) +}) + +test('an empty purge file runs nothing and does not throw', async () => { + const file = path.join(tmpRoot, 'purge.sql') + fs.writeFileSync(file, '-- nothing to drop yet\n') + const rec = recorder() + + assert.equal(await runPurge(file, { query: rec.query }), 0) + assert.deepEqual(rec.ran, []) +})