diff --git a/client/src/App.jsx b/client/src/App.jsx index 2accd99..a627509 100644 --- a/client/src/App.jsx +++ b/client/src/App.jsx @@ -41,6 +41,7 @@ import AuthProvidersAdmin from './routes/admin/views/AuthProvidersAdmin.jsx' import UsersAdmin from './routes/admin/views/UsersAdmin.jsx' import UserDetail from './routes/admin/views/UserDetail.jsx' import InvitesAdmin from './routes/admin/views/InvitesAdmin.jsx' +import ModulesAdmin from './routes/admin/views/ModulesAdmin.jsx' import AccountAdmin from './routes/admin/views/AccountAdmin.jsx' import Moderation from './routes/admin/views/Moderation.jsx' import ModerationUser from './routes/admin/views/ModerationUser.jsx' @@ -169,6 +170,10 @@ export default function App() { } /> } /> } /> + {/* Core's own screen, and it has to be: it is how a module reaches + the volume in the first place. Declared here with the rest of + core's routes, above the module-supplied ones below. */} + } /> } /> {/* Installed modules' admin pages, at /admin//…, already inside RequireAuth + AdminLayout. A module cannot supply its own auth diff --git a/client/src/api/client.js b/client/src/api/client.js index 2314bfe..b21db5a 100644 --- a/client/src/api/client.js +++ b/client/src/api/client.js @@ -233,6 +233,20 @@ export const api = { req('/admin/invites', { method: 'POST', body: { email, role, sendEmail } }), revokeInvite: (id) => req(`/admin/invites/${id}`, { method: 'DELETE' }), + // Installed modules (MODULE_SYSTEM.md §2.7.2). `uninstallModule`'s purge flag + // is a query parameter rather than a body because it hangs off a DELETE, and + // it is spelled out at the call site rather than defaulted, so the + // destructive branch is never the one you get by forgetting an argument. + listModules: () => req('/admin/modules'), + installModule: (url) => req('/admin/modules', { method: 'POST', body: { url } }), + enableModule: (id) => req(`/admin/modules/${encodeURIComponent(id)}/enable`, { method: 'POST' }), + disableModule: (id) => req(`/admin/modules/${encodeURIComponent(id)}/disable`, { method: 'POST' }), + uninstallModule: (id, { purge } = {}) => + req(`/admin/modules/${encodeURIComponent(id)}${purge ? '?purge=true' : ''}`, { method: 'DELETE' }), + purgeModule: (id) => req(`/admin/modules/${encodeURIComponent(id)}/purge`, { method: 'POST' }), + setModuleSources: (hosts) => req('/admin/modules/sources', { method: 'PUT', body: { hosts } }), + restartServer: () => req('/admin/modules/restart', { method: 'POST' }), + // ----- moderation dashboard (admin + moderator) ----- modSummary: () => req('/admin/moderation/stats/summary'), modRecent: (params = {}) => { diff --git a/client/src/lib/moduleAdmin.js b/client/src/lib/moduleAdmin.js new file mode 100644 index 0000000..38cbf19 --- /dev/null +++ b/client/src/lib/moduleAdmin.js @@ -0,0 +1,197 @@ +// What an admin should be told about one installed module, and what they may do +// to it — derived, not spelled out at each button. +// +// Phase 4, slice 2 of docs/website/MODULE_SYSTEM.md §2.7.2. Plain JS rather than +// a hook or a chunk of JSX, for the same reason `lib/adminNav.js` is: the test +// runner here has no DOM, and this is the part of the Modules screen that is +// actually worth testing. +// +// **The screen has three sources of truth and they are allowed to disagree** +// (MODULE_SYSTEM.md §2.4): +// +// state what the DATABASE row records — what the operator decided, and +// what the last boot ended up doing +// liveState what the LOADER has mounted in this process and is answering with +// onVolume whether there is still a directory there at all +// +// Picking one and rendering it would be simpler and would lie. The case that +// makes this concrete is the one decision 3 creates on purpose: an operator +// disables a module (its onShutdown runs, its routes 404) and then enables it +// again. The row says `enabled`; the loader still says `disabled`, because +// there is no `onBoot` re-dispatch and nothing can start it before a restart. +// It is neither running nor off, and the honest thing to show is "enabled — +// restart to start it". + +/** + * The one-line status of a module, and whether that status is waiting on a + * restart. + * + * Ordering matters here. The checks run most-alarming first, so a module whose + * directory has been deleted is described that way rather than by whatever its + * row happens to still say. + * + * @param {object} m a row from GET /admin/modules + * @returns {{ label: string, tone: 'ok'|'warn'|'bad'|'idle', pending: boolean, detail: string }} + */ +export function statusOf(m) { + // Gone from the volume, but still known. Either a hand-deleted directory (the + // boot reconcile marks that `startup_failed`) or an uninstall waiting for its + // restart. Both are "there is nothing to run here". + if (!m.onVolume) { + return { + label: m.state === 'disabled' ? 'Uninstalled' : 'Missing from the volume', + tone: m.state === 'disabled' ? 'idle' : 'bad', + pending: m.liveState !== null, + detail: m.state === 'disabled' + ? 'The files are gone. Its data was kept, and reinstalling brings it back.' + : 'A row exists but there is no module directory. Reinstall it, or uninstall to clear the row.', + } + } + + // **Installed since this process booted**, and this check has to come before + // the failure one. `liveState` is the loader's record, and the loader scans + // the volume once at require time — so a module that is on the volume NOW and + // has no live record was put there after the scan. Anything the row still says + // about it therefore predates the install and is stale by definition. + // + // Found by the §7.7 browser smoke, and no unit test here had modelled it: + // installing over a row left `startup_failed` by the previous boot rendered + // "Failed at the require stage: module directory not present on the volume" + // one second after the file had been written to the volume — and, because that + // branch is not pending, suppressed the restart banner the install had just + // told the operator to use. + if (m.liveState === null) { + return { + label: 'Restart to start', + tone: 'warn', + pending: true, + detail: 'Installed. It mounts when the server next starts.', + } + } + + if (m.state === 'startup_failed' || m.liveState === 'startup_failed') { + return { + label: 'Failed to start', + tone: 'bad', + pending: false, + detail: m.failureReason + ? `Failed at the ${m.failureStage || 'unknown'} stage: ${m.failureReason}` + : 'It failed to start and recorded no reason.', + } + } + + if (m.state === 'disabled') { + return { + label: 'Disabled', + tone: 'idle', + pending: false, + detail: 'Stopped and switched off. Its routes answer 404 and it stays off across restarts.', + } + } + + // The row has been switched on but the loader has not started it — the + // decision-3 case: disable ran its onShutdown, and nothing can start it again + // before a restart. + if (m.liveState !== 'started') { + return { + label: 'Restart to start', + tone: 'warn', + pending: true, + detail: m.liveState === 'disabled' + ? 'Enabled, but still stopped in the running server — it cannot be restarted in place.' + : 'Enabled. It mounts when the server next starts.', + } + } + + // Running, but not the version that is installed. An upgrade writes new files + // and a new row while the old code stays loaded, so the row's `version` is a + // promise about the next boot rather than a description of this one — and + // "Running v2.0.0" beside a process serving v1.0.0 is the same lie as the + // stale-failure one above, in a different place. + if (m.liveVersion && m.liveVersion !== m.version) { + return { + label: 'Restart to finish upgrading', + tone: 'warn', + pending: true, + detail: `v${m.version} is installed; v${m.liveVersion} is still running.`, + } + } + + return { + label: 'Running', + tone: 'ok', + pending: false, + detail: 'Mounted and serving.', + } +} + +/** + * Which actions are offered for a module, and why the others are not. + * + * Returned as a map of `{ shown, reason }` rather than a list of shown actions, + * so a disabled button can say what would make it available. Every rule here + * mirrors one the server enforces — this is presentation, never the boundary. + * + * @param {object} m a row from GET /admin/modules + */ +export function actionsFor(m) { + const running = m.liveState === 'started' + const disabled = m.state === 'disabled' + + return { + // Only offered while something is actually running: disabling a module that + // is already stopped has nothing to stop and no guard to flip. + disable: { + shown: !disabled && m.onVolume, + reason: disabled ? 'Already disabled.' : 'Nothing is running to stop.', + }, + enable: { + shown: disabled && m.onVolume, + reason: 'Only a disabled module can be enabled.', + }, + uninstall: { + shown: m.onVolume, + reason: 'There are no files left to remove.', + }, + // The server refuses a standalone purge unless the module is disabled, so + // the button says so rather than offering a click that 409s. + purge: { + shown: m.onVolume && m.canPurge, + enabled: disabled, + reason: !m.canPurge + ? 'This module ships no purge.sql, so its data cannot be deleted.' + : 'Disable it first, so nothing is serving out of the tables being dropped.', + }, + // A row with no directory is the one thing an uninstall cannot tidy through + // the normal path — offer clearing it instead. + forget: { + shown: !m.onVolume && m.state !== null, + reason: 'The module is still installed.', + }, + running, + } +} + +/** + * Does anything on this list need a restart before it matches what is running? + * + * Drives the one banner at the top of the screen rather than a badge per row: + * the restart is a property of the SERVER, not of a module, and offering it + * five times would suggest otherwise. + */ +export const needsRestart = (modules) => modules.some((m) => statusOf(m).pending) + +/** + * Split a hosts string the way the server will. + * + * Duplicated from `install.parseHosts` deliberately — it is four lines, and the + * alternative is an API round trip to preview what the field is going to mean. + * The server remains the one that decides; this only shows the operator how + * their typing will be read. + */ +export function parseHosts(value) { + return String(value || '') + .split(/[,\s]+/) + .map((h) => h.trim().toLowerCase()) + .filter(Boolean) +} diff --git a/client/src/routes/admin/AdminLayout.jsx b/client/src/routes/admin/AdminLayout.jsx index 9f53c38..c7b9136 100644 --- a/client/src/routes/admin/AdminLayout.jsx +++ b/client/src/routes/admin/AdminLayout.jsx @@ -45,6 +45,7 @@ const IconPulse = () => const IconUser = () => const IconNav = () => const IconPalette = () => +const IconModules = () => // Nav is grouped into collapsible categories. A group with no `title` renders // its items ungrouped (Dashboard at top, Account at bottom). Each item's `roles` @@ -83,6 +84,10 @@ export const NAV = [ { to: '/admin/users', label: 'Users', icon: IconUsers, roles: ['admin'] }, { to: '/admin/invites', label: 'Invites', icon: IconUsers, roles: ['admin'] }, { to: '/admin/settings', label: 'Settings', icon: IconGear, roles: ['admin'] }, + // Admin-only, matching the server: every route under /admin/modules + // re-gates to `admin` on top of the group's staff gate, because installing + // a module runs its code in this process. + { to: '/admin/modules', label: 'Modules', icon: IconModules, roles: ['admin'] }, { to: '/admin/appearance', label: 'Appearance', icon: IconPalette, roles: ['admin'] }, { to: '/admin/navigation', label: 'Navigation', icon: IconNav, roles: ['admin'] }, { to: '/admin/hero', label: 'Hero Editor', icon: IconHero, roles: ['admin'] }, diff --git a/client/src/routes/admin/views/ModulesAdmin.jsx b/client/src/routes/admin/views/ModulesAdmin.jsx new file mode 100644 index 0000000..77865c5 --- /dev/null +++ b/client/src/routes/admin/views/ModulesAdmin.jsx @@ -0,0 +1,402 @@ +import { useCallback, useEffect, useState } from 'react' +import { Loading, ErrorState } from '../../../components/PageState.jsx' +import { dateTime } from '../../../lib/format.js' +import { statusOf, actionsFor, needsRestart, parseHosts } from '../../../lib/moduleAdmin.js' +import { api } from '../../../api/client.js' + +// Installed modules: install from a release URL, enable, disable, uninstall, +// purge, and restart the server so the changes take effect. +// +// Phase 4, slice 2 of docs/website/MODULE_SYSTEM.md §2.7.2. Everything that +// decides what a row SAYS and which buttons it offers lives in +// lib/moduleAdmin.js, which is plain JS and has tests; this file renders it. +// +// Two things about this screen are unlike the rest of the admin panel and are +// deliberate: +// +// 1. **Restart is a banner, not a per-row button.** A restart is a property of +// the server, not of a module. Offering it on five rows would suggest +// otherwise, and an operator who installed three modules should restart +// once. +// 2. **Disable is the only action that takes effect immediately.** Everything +// else is "true after the next boot", because the loader reads the volume +// at require time (§1.12). The buttons say which they are. + +const TONE = { + ok: '#7fd0a4', + warn: 'var(--accent)', + bad: '#d98b84', + idle: 'var(--muted)', +} + +const DANGER = { color: '#d98b84', borderColor: '#5b2020' } + +function Pill({ tone, children }) { + return ( + + {children} + + ) +} + +// ── Install ──────────────────────────────────────────────────────────────── + +function InstallForm({ sourceHosts, onInstalled }) { + const [url, setUrl] = useState('') + const [busy, setBusy] = useState(false) + const [error, setError] = useState('') + const [result, setResult] = useState(null) + + async function submit(e) { + e.preventDefault() + setError('') + setResult(null) + if (!url.trim()) return setError('Paste the URL of a release install manifest.') + setBusy(true) + try { + const res = await api.admin.installModule(url.trim()) + setResult(res) + setUrl('') + await onInstalled() + } catch (err) { + // The server's message is written to be read by whoever pasted the URL — + // which host was refused, which hash did not match, what the archive + // contained. Replacing it with something friendlier would throw away the + // only part that helps. + setError(err.message || 'Could not install that module.') + } finally { + setBusy(false) + } + } + + return ( +
+
Install a module
+
+ + +
+ +

+ The bundle is downloaded, checked against the sha256 its release published, and + unpacked onto the modules volume. It starts serving after a restart.{' '} + {sourceHosts.length === 0 + ? 'No source hosts are allowed yet — add one below before installing.' + : `Allowed hosts: ${sourceHosts.join(', ')}.`} +

+ + {error &&

{error}

} + {result && ( +

+ {result.replaced ? 'Upgraded' : 'Installed'} {result.module?.name} v{result.module?.version}. Restart to load it. +

+ )} +
+ ) +} + +// ── The restart banner ───────────────────────────────────────────────────── + +function RestartBanner({ onDone }) { + const [busy, setBusy] = useState(false) + const [sent, setSent] = useState(false) + + async function restart() { + // Said plainly, because it is true and because the failure mode is bad: a + // deployment with no supervisor does not come back on its own. + const ok = window.confirm( + 'Restart the server now?\n\n' + + 'The site will be briefly unavailable. It comes back on its own only if something is ' + + 'supervising the process — the shipped Docker Compose file does. If you are running ' + + '`npm start` by hand, you will have to start it again yourself.', + ) + if (!ok) return + setBusy(true) + try { + await api.admin.restartServer() + setSent(true) + // Nothing is coming back on this connection: the process is exiting. Give + // the supervisor a moment and then reload, which is what the operator was + // about to do anyway. + setTimeout(() => { if (onDone) onDone() }, 6000) + } catch { + // A failed request here is expected as often as not — the process can win + // the race and drop the socket before the response lands. + setSent(true) + setTimeout(() => { if (onDone) onDone() }, 6000) + } finally { + setBusy(false) + } + } + + return ( +
+
+
+
Restart needed
+

+ {sent + ? 'Restarting. This page will reload once the server is back.' + : 'Modules are read from disk when the server starts, so an install, an uninstall or a re-enable only takes effect after a restart.'} +

+
+ +
+
+ ) +} + +// ── The source allowlist ─────────────────────────────────────────────────── + +function SourceHosts({ hosts, onSaved }) { + const [value, setValue] = useState(hosts.join(', ')) + const [busy, setBusy] = useState(false) + const [error, setError] = useState('') + const [saved, setSaved] = useState(false) + + useEffect(() => { setValue(hosts.join(', ')) }, [hosts]) + + async function save(e) { + e.preventDefault() + setError('') + setSaved(false) + setBusy(true) + try { + await api.admin.setModuleSources(value) + setSaved(true) + await onSaved() + } catch (err) { + setError(err.message || 'Could not save the allowlist.') + } finally { + setBusy(false) + } + } + + const parsed = parseHosts(value) + + return ( +
+
Where modules may be installed from
+
+ + +
+ +

+ Installing a module runs its code inside this server, so only hosts listed here may be + installed from — over HTTPS, and re-checked on every redirect. An empty list blocks all + installs.{' '} + {parsed.length > 0 && <>Will be saved as: {parsed.join(', ')}.} +

+ + {error &&

{error}

} + {saved && !error &&

Saved.

} +
+ ) +} + +// ── One module ───────────────────────────────────────────────────────────── + +function ModuleRow({ m, onChanged, onError }) { + const [busy, setBusy] = useState('') + const status = statusOf(m) + const actions = actionsFor(m) + + async function run(name, fn) { + setBusy(name) + try { + await fn() + await onChanged() + } catch (err) { + onError(err.message || `Could not ${name} ${m.id}.`) + } finally { + setBusy('') + } + } + + const disable = () => run('disable', () => api.admin.disableModule(m.id)) + const enable = () => run('enable', () => api.admin.enableModule(m.id)) + + function uninstall() { + // The purge choice is made HERE and only here, because purge.sql lives + // inside the directory the uninstall is about to delete — there is no + // "purge it later" (§2.7.2 decision 5). Two prompts rather than one, so + // "delete the data too" is never something you agree to by reflex. + if (!window.confirm(`Uninstall ${m.name}?\n\nIts files are removed. Its data is kept unless you ask otherwise next.`)) return + let purge = false + if (m.canPurge) { + purge = window.confirm( + `Also permanently delete ${m.name}'s data?\n\n` + + 'This drops its tables and cannot be undone. This is the only moment it can be offered — ' + + 'the script that does it is part of the files being removed.\n\n' + + 'OK deletes the data. Cancel keeps it.', + ) + } + return run('uninstall', () => api.admin.uninstallModule(m.id, { purge })) + } + + function purge() { + if (!window.confirm(`Permanently delete ${m.name}'s data?\n\nThis drops its tables and cannot be undone.`)) return + return run('purge', () => api.admin.purgeModule(m.id)) + } + + const forget = () => run('forget', () => api.admin.uninstallModule(m.id)) + + return ( + + +
{m.name}
+
+ {m.id} · v{m.version} +
+ {m.capabilities?.length > 0 && ( +
{m.capabilities.join(' · ')}
+ )} + + + + {status.label} +
{status.detail}
+ + + + {/* Provenance. Null for a directory placed on the volume by hand, which + stays a supported install — so it is shown as that, not as missing. */} + {m.source ? ( + <> +
{m.source}
+ {m.sha256 &&
sha256 {m.sha256.slice(0, 12)}…
} + + ) : ( + Placed on the volume by hand + )} + {m.installedAt &&
{dateTime(m.installedAt)}
} + + + +
+ {actions.disable.shown && ( + + )} + {actions.enable.shown && ( + + )} + {actions.purge.shown && ( + + )} + {actions.uninstall.shown && ( + + )} + {actions.forget.shown && ( + + )} +
+ + + ) +} + +// ── The screen ───────────────────────────────────────────────────────────── + +export default function ModulesAdmin() { + const [data, setData] = useState(null) + const [error, setError] = useState('') + const [actionError, setActionError] = useState('') + + const load = useCallback(async () => { + setError('') + try { + setData(await api.admin.listModules()) + } catch { + setError('Could not load installed modules.') + } + }, []) + useEffect(() => { load() }, [load]) + + if (error) return + if (!data) return + + const modules = data.modules || [] + const sourceHosts = data.sourceHosts || [] + + return ( +
+ {needsRestart(modules) && window.location.reload()} />} + + + + {actionError && ( +

{actionError}

+ )} + +
+ + + + + + + + + + {modules.length === 0 && ( + + + + )} + {modules.map((m) => ( + + ))} + +
ModuleStatusInstalled from +
+ No modules installed. Paste a release install-manifest URL above to add one. +
+
+ + +
+ ) +} diff --git a/client/test/apiClient.test.js b/client/test/apiClient.test.js index 69716fa..fbf606f 100644 --- a/client/test/apiClient.test.js +++ b/client/test/apiClient.test.js @@ -140,3 +140,48 @@ test('DELETE self-service session revoke encodes the id and uses the DELETE meth assert.equal(calls[0].opts.method, 'DELETE') assert.match(calls[0].url, /\/auth\/me\/sessions\/a%20b%2Fc$/) }) + +// ── admin: installed modules (MODULE_SYSTEM.md §2.7.2) ────────────────── +// +// These pin the URLs, because the destructive one differs from the harmless one +// by a query parameter and nothing else. + +test('module actions hit the right paths and methods', async () => { + const cases = [ + [() => api.admin.listModules(), 'GET', '/api/v1/admin/modules'], + [() => api.admin.installModule('https://x/y.json'), 'POST', '/api/v1/admin/modules'], + [() => api.admin.enableModule('uo'), 'POST', '/api/v1/admin/modules/uo/enable'], + [() => api.admin.disableModule('uo'), 'POST', '/api/v1/admin/modules/uo/disable'], + [() => api.admin.purgeModule('uo'), 'POST', '/api/v1/admin/modules/uo/purge'], + [() => api.admin.setModuleSources('a.com'), 'PUT', '/api/v1/admin/modules/sources'], + [() => api.admin.restartServer(), 'POST', '/api/v1/admin/modules/restart'], + ] + for (const [call, method, url] of cases) { + calls = [] + willReply({ body: {} }) + await call() + assert.equal(calls[0].url, url) + assert.equal(calls[0].opts.method || 'GET', method) + } +}) + +test('uninstall only asks for a purge when it is told to', async () => { + // The difference between "remove the module" and "remove the module and drop + // every table it owns" is this query parameter, so a default that leaned the + // wrong way would be irreversible. + willReply({ body: {} }) + await api.admin.uninstallModule('uo') + assert.equal(calls[0].url, '/api/v1/admin/modules/uo') + assert.equal(calls[0].opts.method, 'DELETE') + + calls = [] + willReply({ body: {} }) + await api.admin.uninstallModule('uo', { purge: true }) + assert.equal(calls[0].url, '/api/v1/admin/modules/uo?purge=true') +}) + +test('a module id is URL-encoded on the way into the path', async () => { + willReply({ body: {} }) + await api.admin.disableModule('a b/c') + assert.equal(calls[0].url, '/api/v1/admin/modules/a%20b%2Fc/disable') +}) diff --git a/client/test/moduleAdmin.test.js b/client/test/moduleAdmin.test.js new file mode 100644 index 0000000..e6e667d --- /dev/null +++ b/client/test/moduleAdmin.test.js @@ -0,0 +1,226 @@ +import { test } from 'node:test' +import assert from 'node:assert/strict' +import { statusOf, actionsFor, needsRestart, parseHosts } from '../src/lib/moduleAdmin.js' + +// lib/moduleAdmin.js — what the Modules screen says about a module and what it +// lets you do to it. Phase 4, slice 2 of MODULE_SYSTEM.md §2.7.2. +// +// This is the part of the screen worth testing, and it is plain JS so this +// runner can reach it (there is no DOM here). What it encodes is §2.4's rule +// that the row, the loader and the volume are three sources of truth which are +// ALLOWED to disagree — so most of these cases are combinations that a screen +// picking one source would render as a lie. + +/** A module as GET /admin/modules returns it, with the running case as default. */ +const mod = (over = {}) => ({ + id: 'uo', + name: 'Ultima Online', + version: '1.0.0', + state: 'started', + failureStage: null, + failureReason: null, + source: 'https://gitea.example.com/x/uo.json', + sha256: 'a'.repeat(64), + installedAt: null, + startedAt: null, + liveState: 'started', + liveVersion: '1.0.0', + capabilities: [], + onVolume: true, + canPurge: true, + ...over, +}) + +// ── statusOf ─────────────────────────────────────────────────────────────── + +test('a mounted, started module is Running and needs nothing', () => { + const s = statusOf(mod()) + assert.equal(s.label, 'Running') + assert.equal(s.tone, 'ok') + assert.equal(s.pending, false) +}) + +test('enabled in the row but disabled in the loader is "Restart to start"', () => { + // THE case decision 3 creates on purpose: disable ran the module's onShutdown, + // then the operator enabled it again. The row says enabled; nothing can start + // it before a restart. Showing either "Running" or "Disabled" would be false. + const s = statusOf(mod({ state: 'enabled', liveState: 'disabled' })) + assert.equal(s.label, 'Restart to start') + assert.equal(s.tone, 'warn') + assert.equal(s.pending, true) + assert.match(s.detail, /cannot be restarted in place/) +}) + +test('freshly installed and never booted into is also "Restart to start"', () => { + const s = statusOf(mod({ state: 'installed', liveState: null })) + assert.equal(s.label, 'Restart to start') + assert.equal(s.pending, true) + assert.match(s.detail, /mounts when the server next starts/) +}) + +test('a disabled module is Disabled, and that is not pending anything', () => { + // Disable takes effect immediately — it is the one action that does — so there + // is nothing for a restart banner to be about. + const s = statusOf(mod({ state: 'disabled', liveState: 'disabled' })) + assert.equal(s.label, 'Disabled') + assert.equal(s.pending, false) +}) + +test('a fresh install over a failed row is pending, not failed', () => { + // THE defect the §7.7 browser smoke found, and one no test here had modelled. + // Installing over a row the previous boot left `startup_failed` rendered + // "Failed at the require stage: module directory not present on the volume" a + // second after the files had been written — and suppressed the restart banner + // the install had just told the operator to use. + // + // `liveState === null` with the module on the volume means the loader's scan + // never saw it, so it arrived after boot and everything the row says predates + // it. + const s = statusOf(mod({ + state: 'startup_failed', + liveState: null, + failureStage: 'require', + failureReason: 'module directory not present on the volume', + })) + assert.equal(s.label, 'Restart to start') + assert.equal(s.pending, true) + assert.doesNotMatch(s.detail, /not present on the volume/, 'the stale reason must not survive the install') +}) + +test('the restart banner appears for that install', () => { + // The second half of the same defect: the banner is driven by `pending`, so a + // row wrongly classified as failed silently removed the only way to act on it. + assert.equal(needsRestart([mod({ state: 'startup_failed', liveState: null })]), true) +}) + +test('an upgrade that has not been restarted into says so', () => { + // Same class as the stale-failure defect: the row is a promise about the next + // boot, not a description of this one. Reporting "Running v2.0.0" while the + // process is serving v1.0.0 would hide the only action that fixes it. + const s = statusOf(mod({ version: '2.0.0', liveVersion: '1.0.0' })) + assert.equal(s.label, 'Restart to finish upgrading') + assert.equal(s.pending, true) + assert.match(s.detail, /v2\.0\.0 is installed; v1\.0\.0 is still running/) +}) + +test('reinstalling the SAME version is not an upgrade in progress', () => { + assert.equal(statusOf(mod({ version: '1.0.0', liveVersion: '1.0.0' })).label, 'Running') +}) + +test('a failed module reports the stage and the reason it recorded', () => { + const s = statusOf(mod({ + state: 'startup_failed', + liveState: 'startup_failed', + failureStage: 'schema', + failureReason: "Unknown column 'x' in 'field list'", + })) + assert.equal(s.label, 'Failed to start') + assert.equal(s.tone, 'bad') + assert.match(s.detail, /schema stage/) + assert.match(s.detail, /Unknown column/) +}) + +test('a failure with no recorded reason says so rather than showing a blank', () => { + const s = statusOf(mod({ state: 'startup_failed', liveState: 'startup_failed' })) + assert.match(s.detail, /recorded no reason/) +}) + +test('a row whose directory is gone by hand is bad, not merely disabled', () => { + // The boot reconcile marks this `startup_failed` because a row claiming to be + // enabled for a module that is not on the volume is simply untrue. + const s = statusOf(mod({ state: 'startup_failed', liveState: null, onVolume: false, failureStage: 'require', failureReason: 'module directory not present on the volume' })) + assert.equal(s.label, 'Missing from the volume') + assert.equal(s.tone, 'bad') +}) + +test('an uninstalled module reads as uninstalled, and says the data was kept', () => { + // Uninstall leaves the row `disabled` and the data alone — which is the whole + // point of keeping the row, so the screen has to say it. + const s = statusOf(mod({ state: 'disabled', liveState: 'disabled', onVolume: false })) + assert.equal(s.label, 'Uninstalled') + assert.equal(s.tone, 'idle') + assert.match(s.detail, /data was kept/i) +}) + +test('missing-from-the-volume beats every other status', () => { + // Ordering: a module with no files is described that way whatever its row + // still claims, because there is nothing there to be running. + for (const state of ['started', 'enabled', 'installed', 'startup_failed']) { + assert.match(statusOf(mod({ state, onVolume: false })).label, /Missing from the volume/) + } +}) + +// ── actionsFor ───────────────────────────────────────────────────────────── + +test('a running module offers disable, uninstall and a blocked purge', () => { + const a = actionsFor(mod()) + assert.equal(a.disable.shown, true) + assert.equal(a.enable.shown, false) + assert.equal(a.uninstall.shown, true) + assert.equal(a.purge.shown, true) + // Shown but not clickable: the server refuses a standalone purge on anything + // that is not disabled, so offering the click would only produce a 409. + assert.equal(a.purge.enabled, false) + assert.match(a.purge.reason, /Disable it first/) +}) + +test('a disabled module offers enable, and purge is now live', () => { + const a = actionsFor(mod({ state: 'disabled', liveState: 'disabled' })) + assert.equal(a.enable.shown, true) + assert.equal(a.disable.shown, false) + assert.equal(a.purge.enabled, true) +}) + +test('a module with no purge.sql never offers purge, and says why', () => { + const a = actionsFor(mod({ state: 'disabled', liveState: 'disabled', canPurge: false })) + assert.equal(a.purge.shown, false) + assert.match(a.purge.reason, /ships no purge.sql/) +}) + +test('a module with no files offers only clearing the row', () => { + const a = actionsFor(mod({ state: 'disabled', liveState: null, onVolume: false })) + assert.equal(a.uninstall.shown, false) + assert.equal(a.disable.shown, false) + assert.equal(a.enable.shown, false) + assert.equal(a.purge.shown, false, 'there is no purge.sql left to run') + assert.equal(a.forget.shown, true) +}) + +test('a directory with no row yet is actionable, and offers nothing to forget', () => { + // A hand-placed install before its first boot: it has no row, so `state` is + // null. Its routes are already being served, so it must be disableable. + const a = actionsFor(mod({ state: null, liveState: 'started' })) + assert.equal(a.disable.shown, true) + assert.equal(a.uninstall.shown, true) + assert.equal(a.forget.shown, false) +}) + +// ── needsRestart ─────────────────────────────────────────────────────────── + +test('the restart banner is driven by the list, not by any one module', () => { + // A restart is a property of the SERVER. One pending module is enough, and + // three do not mean three restarts. + assert.equal(needsRestart([mod(), mod({ id: 'b' })]), false) + assert.equal(needsRestart([mod(), mod({ id: 'b', state: 'installed', liveState: null })]), true) + assert.equal(needsRestart([]), false) +}) + +test('a disabled module does not ask for a restart', () => { + // Disable is immediate; a banner here would be asking for a restart that + // would change nothing. + assert.equal(needsRestart([mod({ state: 'disabled', liveState: 'disabled' })]), false) +}) + +test('a failed module does not ask for a restart either', () => { + // It is retried on every boot anyway, and the operator has to fix the cause + // first — a banner would suggest restarting is the remedy. + assert.equal(needsRestart([mod({ state: 'startup_failed', liveState: 'startup_failed' })]), false) +}) + +// ── parseHosts ───────────────────────────────────────────────────────────── + +test('parseHosts previews exactly what the server will store', () => { + assert.deepEqual(parseHosts('A.com, b.com\n c.com'), ['a.com', 'b.com', 'c.com']) + assert.deepEqual(parseHosts(' '), []) + assert.deepEqual(parseHosts(undefined), []) +}) diff --git a/server/src/model/modules/modules.db.js b/server/src/model/modules/modules.db.js index 66474bb..d7e2bba 100644 --- a/server/src/model/modules/modules.db.js +++ b/server/src/model/modules/modules.db.js @@ -17,6 +17,23 @@ const getOne = (id) => query(`SELECT ${COLS} FROM installed_modules WHERE id = ? // module must not silently disable it, and re-installing a disabled one must not // silently switch it back on. A brand-new row lands in `installed`, the transient // state the next restart resolves. +// Provenance is COALESCEd, and that is the whole difference between this +// working and not. +// +// `lifecycle.boot()` re-records every module it scanned with no source and no +// sha256 — a directory placed on the volume by hand genuinely has neither, and +// the boot has no way to know where one came from. With a plain +// `source = VALUES(source)` that refresh overwrote both columns with NULL on +// EVERY boot, so an admin-panel install's provenance survived exactly until the +// restart that install asked for. Nothing could catch it before Phase 4: until +// then no caller ever passed a non-null value, and lifecycle.js's comment +// asserting that "recordInstalled leaves what it is not given" was a description +// of an intention rather than of this statement. +// +// COALESCE makes that comment true: a value overwrites, a NULL leaves what is +// there. The cost is that re-placing a DIFFERENT bundle by hand over a row that +// was once installed from a URL keeps the old provenance — which is wrong but +// stale, and strictly better than the alternative, which was wrong and blank. const upsert = ({ id, name, version, source, sha256 }) => query( `INSERT INTO installed_modules (id, name, version, source, sha256, state) @@ -24,8 +41,8 @@ const upsert = ({ id, name, version, source, sha256 }) => ON DUPLICATE KEY UPDATE name = VALUES(name), version = VALUES(version), - source = VALUES(source), - sha256 = VALUES(sha256)`, + source = COALESCE(VALUES(source), source), + sha256 = COALESCE(VALUES(sha256), sha256)`, [id, name, version, source ?? null, sha256 ?? null], ) diff --git a/server/src/modules/lifecycle.js b/server/src/modules/lifecycle.js index 737773b..637df88 100644 --- a/server/src/modules/lifecycle.js +++ b/server/src/modules/lifecycle.js @@ -101,10 +101,16 @@ async function boot({ modules, model } = {}) { id: m.id, name: m.name, version: m.version, - // Null provenance is what a hand-placed directory looks like. An install - // performed through the admin panel (§2.5, a later phase) writes the row - // with its source and hash first; this refresh deliberately does not - // overwrite either, because recordInstalled leaves what it is not given. + // Null provenance is what a hand-placed directory looks like, and it is + // all this step can honestly say: a scan finds a directory, never where it + // came from. An install through the admin panel writes source and sha256 + // first, and this refresh must not undo that. + // + // It used to. `upsert` assigned both columns unconditionally, so every + // boot nulled them and an install's provenance survived only until the + // restart it asked for. The statement now COALESCEs — see the note on + // modules.db.js's upsert. Nothing could have caught it before Phase 4: + // this was the only caller, and it has never had a value to pass. })) } diff --git a/server/src/router/v1/admin/modules.controller.js b/server/src/router/v1/admin/modules.controller.js index d01f5e5..2c79dc5 100644 --- a/server/src/router/v1/admin/modules.controller.js +++ b/server/src/router/v1/admin/modules.controller.js @@ -85,6 +85,11 @@ function present(row, live, onVolume) { startedAt: row ? row.startedAt : null, // What is actually mounted in this process, and what it is answering. liveState: live ? live.state : null, + // The version RUNNING, which is not always the version installed: an upgrade + // writes new files and a new row while the old code stays loaded until the + // restart. Without this the screen would report the new version as + // "Running", which is the same lie in a different place. + liveVersion: live ? live.version : null, capabilities: live ? live.capabilities : [], // What is on the volume. onVolume, @@ -349,11 +354,24 @@ async function setSources(req, res) { // "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. +// It reaches server.js's existing SIGTERM handler rather than doing the work +// itself: that handler stops the modules, the workers and the listeners in the +// right order and closes the pool and the log file before exiting 0, and going +// through it means there is exactly one graceful-shutdown path that this route +// cannot drift from. +// +// It gets there by EMITTING the event, not by signalling the process, and that +// is not a detail. `process.kill(process.pid, 'SIGTERM')` is what this did +// first, and it works on Linux — but **Windows has no POSIX signals, and Node +// documents SIGTERM there as unconditional termination of the target process**. +// So on a Windows host the restart killed the server outright: no module +// `onShutdown`, no pool close, no log flush. Verified by running it — the +// process was gone and the shutdown handler had logged nothing. +// +// `process.on('SIGTERM', …)` is an ordinary EventEmitter listener, so +// `process.emit('SIGTERM')` invokes exactly the same handler on every platform +// without involving the OS at all. Deployment is Linux containers and would +// never have shown this; development is not. // // What brings the process BACK is the supervisor, not this. The shipped // docker-compose.yml declares `restart: unless-stopped` on `app`, which restarts @@ -372,7 +390,7 @@ function restart(req, res) { .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() + setTimeout(() => process.emit('SIGTERM'), 250).unref() }) } diff --git a/server/test/adminModules.test.js b/server/test/adminModules.test.js index 1e482d4..46d3e00 100644 --- a/server/test/adminModules.test.js +++ b/server/test/adminModules.test.js @@ -422,42 +422,55 @@ test('an empty allowlist is storable, and means no installs', async () => { // ── 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 }) } +// These listen for the SIGTERM EVENT rather than stubbing `process.kill`, and +// that is the whole point of them now. +// +// The first version of this route called `process.kill(process.pid, 'SIGTERM')` +// and the first version of these tests stubbed `process.kill` and asserted it +// had been called with SIGTERM. Both passed. Both were wrong: Windows has no +// POSIX signals, and Node documents SIGTERM there as unconditional termination — +// so on a Windows host the route killed the server outright, with no module +// `onShutdown`, no pool close and no log flush. A stub of `process.kill` cannot +// see that, because what it asserts is precisely the call whose MEANING differs +// by platform. +// +// Asserting on the event closes the gap: it is what server.js's handler is +// actually subscribed to, so a test passing here means the handler would run. +function onceSigterm() { + return new Promise((resolve, reject) => { + const timer = setTimeout(() => { + process.removeListener('SIGTERM', handler) + reject(new Error('no SIGTERM was emitted within 1s')) + }, 1000) + function handler() { + clearTimeout(timer) + process.removeListener('SIGTERM', handler) + resolve(true) + } + process.on('SIGTERM', handler) + }) +} - try { - const res = mockRes() - ctrl.restart(req(), res) +test('restart answers first, then triggers the one graceful-shutdown path', async () => { + const fired = onceSigterm() + const res = mockRes() - // 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) + ctrl.restart(req(), res) - 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 - } + // Answered synchronously: once the shutdown starts 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) + + assert.equal(await fired, true) + assert.equal(logged[0].action, 'module.restart') }) 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') } + const fired = onceSigterm() - try { - ctrl.restart(req(), mockRes()) - await new Promise((resolve) => setTimeout(resolve, 400)) - assert.deepEqual(signals, ['SIGTERM']) - } finally { - process.kill = originalKill - } + ctrl.restart(req(), mockRes()) + + assert.equal(await fired, true) }) diff --git a/server/test/modules.model.test.js b/server/test/modules.model.test.js index 4147acc..ec61263 100644 --- a/server/test/modules.model.test.js +++ b/server/test/modules.model.test.js @@ -38,7 +38,17 @@ modulesDb.getOne = async (id) => (rows.has(id) ? [rows.get(id)] : []) modulesDb.upsert = async ({ id, name, version, source, sha256 }) => { const existing = rows.get(id) if (existing) { - Object.assign(existing, { name, version, source: source ?? null, sha256: sha256 ?? null }) + // COALESCE, matching the real statement: a value overwrites, a NULL leaves + // what is there. This fake used to assign unconditionally — faithfully + // reproducing the defect it was supposed to be able to catch, which is why + // the boot refresh nulling an install's provenance survived until a browser + // showed it. + Object.assign(existing, { + name, + version, + source: source ?? existing.source ?? null, + sha256: sha256 ?? existing.sha256 ?? null, + }) return { affectedRows: 1 } } rows.set(id, { @@ -116,6 +126,46 @@ test('a hand-placed module records with no provenance', async () => { assert.equal(mod.sha256, null) }) +test('the boot refresh does not wipe an install\'s provenance', async () => { + // The defect a browser found in Phase 4, and the reason the upsert COALESCEs. + // + // lifecycle.boot() re-records every scanned module with NO source and NO + // sha256, because a hand-placed directory genuinely has neither. That refresh + // used to write both columns as NULL, so an admin-panel install's provenance + // survived exactly until the restart the install asked for — and the screen + // then described a module installed from a URL as "placed on the volume by + // hand". Nothing could see it until Phase 4: no caller had ever passed a + // non-null value. + await modules.recordInstalled({ + id: 'uo', + name: 'Ultima Online', + version: '1.0.0', + source: 'https://gitea.example/x/uo-1.0.0.json', + sha256: 'b'.repeat(64), + }) + + // What the next boot does. + const after = await modules.recordInstalled({ id: 'uo', name: 'Ultima Online', version: '1.0.0' }) + + assert.equal(after.source, 'https://gitea.example/x/uo-1.0.0.json') + assert.equal(after.sha256, 'b'.repeat(64)) +}) + +test('a re-install from a new URL does replace the provenance', async () => { + // The other half: COALESCE must not make the columns write-once, or an upgrade + // would for ever show where the FIRST version came from. + await modules.recordInstalled({ + id: 'uo', name: 'Ultima Online', version: '1.0.0', source: 'https://old/x.json', sha256: 'c'.repeat(64), + }) + const after = await modules.recordInstalled({ + id: 'uo', name: 'Ultima Online', version: '2.0.0', source: 'https://new/y.json', sha256: 'd'.repeat(64), + }) + + assert.equal(after.source, 'https://new/y.json') + assert.equal(after.sha256, 'd'.repeat(64)) + assert.equal(after.version, '2.0.0') +}) + test('recordInstalled refuses a manifest missing id, name or version', async () => { await assert.rejects( () => modules.recordInstalled({ id: 'uo', version: '1.0.0' }),