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, []) +})