Files
website/server/test/adminModules.test.js
wtclaude 732927a6bb fix(modules): three defects a real install exposed (phase 4, slice 1)
Standing the slice-2 screen up against a live server and installing the
published module-uo v0.3.0 through it found three things, none of which any
unit test in this repo could have caught. Two of them are older than this
phase.

1. The boot refresh nulled every install's provenance
--------------------------------------------------------
`installed_modules.source` and `.sha256` exist so the admin panel can say
where a module came from. They never survived a restart.

`lifecycle.boot()` re-records every scanned module with no source and no
sha256 -- correctly, because a scan finds a directory and never where it came
from -- and `upsert` assigned both columns unconditionally. So an install's
provenance lasted exactly until the restart that install asked for, and the
screen then described a module installed from a URL as "placed on the volume
by hand". Verified live: install, restart, provenance gone.

Nothing could have caught it before now. Phase 4 wrote the first non-null
value these columns had ever had, so lifecycle.js's comment asserting that
"recordInstalled leaves what it is not given" described an intention rather
than the statement below it -- and modules.model.test.js's fake reproduced
the defect faithfully, assigning unconditionally just like the SQL.

Fixed with COALESCE(VALUES(col), col): a value overwrites, a NULL leaves what
is there. The fake now matches, and two tests pin both directions -- a boot
refresh must not wipe it, and a re-install from a new URL must still replace
it, or the column would become write-once and an upgrade would for ever show
where the first version came from.

2. The restart killed the server on Windows instead of stopping it
------------------------------------------------------------------
The route called `process.kill(process.pid, 'SIGTERM')` to reach server.js's
graceful-shutdown handler. That works on Linux. **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 listener close, no pool close, no log
flush. Observed exactly that: the process was gone and the shutdown handler
had logged nothing at all.

`process.on('SIGTERM', ...)` is an ordinary EventEmitter listener, so
`process.emit('SIGTERM')` reaches the same handler on every platform without
involving the OS. One shutdown path, still; it just gets there by an event.

Deployment is Linux containers and would never have shown this. Development
is not, and neither is the smoke that found it.

The test was worse than useless: it stubbed `process.kill` and asserted it
had been called with SIGTERM, which is precisely the call whose MEANING
differs by platform. It now waits for the SIGTERM EVENT -- what server.js is
actually subscribed to -- so a pass here means the handler would run.

3. `present()` did not publish the running version
--------------------------------------------------
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. Adds `liveVersion` from the loader beside `liveState`, so the
screen can tell the two apart instead of reporting the new version as running.

723 server tests (+2), manifest and OpenAPI both unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-12 03:48:27 -05:00

477 lines
19 KiB
JavaScript

// ── 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 ────────────────────────────────────────────────────────────────
// 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)
})
}
test('restart answers first, then triggers the one graceful-shutdown path', async () => {
const fired = onceSigterm()
const res = mockRes()
ctrl.restart(req(), res)
// 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 () => {
activity.log = async () => { throw new Error('database is gone') }
const fired = onceSigterm()
ctrl.restart(req(), mockRes())
assert.equal(await fired, true)
})