feat(modules): install, uninstall, purge and restart (phase 4, slice 1) #142
Reference in New Issue
Block a user
No description provided.
Delete Branch "feature/module-install-service"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The consumer half of a release
module-uo's CI has been publishing since Phase 3 closed. Plan: docs#141 §2.7.2.Before this, core had the
installed_modulesprovenance columns and no code that could ever fill them — nothing fetched, verified, unpacked, removed or purged anything, and there was no admin route at all.Adds
modules/archive.js,modules/install.js,schema.runPurge(),lifecycle.stop(),loader.stopHook()and/api/v1/admin/moduleswith eight routes.npm run check:modulesReject, never sanitise
The download is the easy part:
https-only against the allowlist, re-checked on every redirect hop, a declaredsha256compared against the bytes that arrived, and a byte cap. Unpacking is where the archive chooses the filenames — and core writes into a directory bind-mounted from the host, so an escape is not confined to the container.archive.jsinspects the whole archive before a byte is unpacked and refuses absolute and drive-absolute paths,..segments, NUL bytes, backslashes, anything that is not a regular file or directory, more than one top-level entry, and anything over the entry or byte caps. Refusing symlinks and hardlinks outright is what keeps this off the majority of node-tar's published advisories instead of depending on the library to contain them.The two-pass shape is load-bearing, and it was measured rather than assumed. Extracting an archive whose fourth member escapes upward throws under node-tar 7.5.22 — and leaves the first three members on disk:
The loader scans that directory at require time on the next boot, so a half-unpacked module is a module. Everything therefore happens in a scratch directory removed on any failure, and the move into place is the last step.
taris pinned to ^7.5.22, not the^6that installs by default: 6.x is flagged critical, and reading that advisory list is what the file's header now says out loud — almost all of it is hardlink/symlink traversal and PAX header interpretation differentials, which is precisely this feature's threat model.Two things the plan had wrong
module-uo-<version>, not the module id. The plan's "top-level name must equal the id" rule was checking against nothing real. The extractor strips that level instead — its name belongs to whoever published the bundle, the directory it lands in is core's. What is checked instead is the unpackedmodule.json: a manifest promisinguoand delivering something else is refused rather than installed under the name it promised.purge.sqllives inside the directory uninstall deletes. It is offered in the uninstall flow and as a standalone action on a still-installed module — and the standalone one refuses unless the module is already disabled, because dropping tables under something that is still serving leaves it answering out of a world that no longer exists.Disable now means stopped
lifecycle.stop()dispatches that one module'sonShutdownbefore flipping the guard, so a module an operator switches off actually releases its sockets and closes its streams instead of merely becoming unreachable. The hook runs first and the state moves after it — whileonShutdownruns the module is stillstarted, the only state in which its routes and the world it is tearing down agree. A hook that throws does not stop the disable: the opposite of the boot path's rule, and deliberately.Enable is not its mirror, and there is no
start(id)beside it. There is noonBootre-dispatch and the hooks were never promised re-entrant, so enable moves the row and the restart route starts it. A test pins that enable does not touch the loader, because "fixing" that is a one-line change which would put a module with closed sockets back on the nav.restartraises SIGTERM against its own process rather than calling the shutdown path directly, soserver.js's handler stays the one graceful-shutdown path and this route cannot drift from it.The allowlist bootstraps from
MODULE_SOURCE_HOSTSinto a settings row and is admin-managed after that.seedDefaultisINSERT IGNORE, so changing the variable on an existing deployment is a no-op by design. An empty list forbids every install rather than allowing every host — the safe direction for a value someone might blank by accident.Verified against the real v0.3.0 release
Not a fixture. Fetched the published install manifest over the real Gitea host and its redirect chain, verified the sha256, inspected and unpacked the 252,517-byte artifact — then booted core against the result:
Two defects this slice's own tooling caught
Both had already been written down as classes:
runPurgeat require time, capturing the function rather than the module — which made the one dependency whose order matters the one that could not be substituted in a test. Same class asmodule-uo/server/core.js's "never destructure a ctx getter at require time".Not in this slice
The admin screen (slice 2), and the declarative Docker path (slice 3). This is server-only; there is no UI for any of it yet.
AI disclosure
🤖 Generated with Claude Code
The consumer half of a release module-uo's CI has been publishing since phase 3 closed. Before this, core had the installed_modules provenance columns and no code that could ever fill them: nothing fetched, verified, unpacked, removed or purged anything, and there was no admin route at all. Adds modules/archive.js, modules/install.js, schema.runPurge(), lifecycle.stop(), loader.stopHook(), and /api/v1/admin/modules with eight routes. 797 server tests (+76), manifest 158 -> 166 + 2 internal, OpenAPI gains 8 operations and loses nothing. Reject, never sanitise ---------------------- The download is the easy part: an https-only allowlist re-checked on every redirect hop, a declared sha256 compared against the bytes that arrived, and a byte cap. Unpacking is where the archive chooses the filenames, and core writes into a directory bind-mounted from the host, so an escape is not confined to the container. archive.js inspects the whole archive before a byte is unpacked and refuses absolute and drive-absolute paths, `..` segments, NUL bytes, backslashes, anything that is not a regular file or a directory, more than one top-level entry, and anything over the entry or byte caps. Refusing symlinks and hardlinks outright is what keeps this off the majority of node-tar's published advisories rather than depending on the library to contain them. That two-pass shape is load-bearing, and it was measured rather than assumed: extracting an archive whose fourth member escapes upward throws under node-tar 7.5.22 -- and leaves the first three members on disk. The loader scans that directory at require time on the next boot, so a half-unpacked module is a module. Everything therefore happens in a scratch directory that is removed on any failure, and the move into place is the last step. `tar` is pinned to ^7.5.22 rather than the ^6 that installs by default: 6.x is flagged critical, and reading the advisory list is what the file's header now says out loud -- almost all of it is hardlink or symlink traversal and PAX header interpretation differentials, which is exactly this feature's threat model. Two things the plan had wrong ----------------------------- The bundle's top-level directory is `module-uo-<version>`, not the module id -- so "the top-level name must equal the id" was checked against nothing real. The extractor strips that level instead, because its name belongs to whoever published the bundle and the directory it lands in is core's. What is checked instead is the unpacked module.json: a manifest promising `uo` and delivering something else is refused rather than installed under the name it promised. And purge cannot be a follow-up action (decision 5): purge.sql lives inside the directory uninstall deletes. It is offered in the uninstall flow and as a standalone action on a still-installed module, and the standalone one refuses unless the module is already disabled -- dropping tables under something that is still serving leaves it answering out of a world that no longer exists. Disable now means stopped ------------------------- lifecycle.stop() dispatches that one module's onShutdown before flipping the guard, so a module an operator switches off actually releases its sockets and closes its streams instead of merely becoming unreachable. The hook runs first and the state moves after it, because while onShutdown runs the module is still `started` and that is the only state in which its routes and the world it is tearing down agree. A hook that throws does not stop the disable -- the opposite of the boot path's rule, and deliberately. Enable is not its mirror and there is no start(id) beside it. There is no onBoot re-dispatch and the hooks were never promised re-entrant, so enable moves the row and the restart route starts it. A test pins that enable does not touch the loader, because "fixing" it is a one-line change that would put a module with closed sockets back on the nav. Restart raises SIGTERM against its own process rather than calling the shutdown path directly, so server.js's handler stays the one graceful-shutdown path and this route cannot drift from it. The allowlist bootstraps from MODULE_SOURCE_HOSTS into a settings row and is admin-managed after that (decision 6); seedDefault is INSERT IGNORE, so changing the variable on an existing deployment is a no-op by design. An empty list forbids every install rather than allowing every host -- the safe direction for a value someone might blank by accident. Verified against the real v0.3.0 release ---------------------------------------- Not a fixture: fetched the published install manifest over the real Gitea host and its redirect chain, verified the sha256, inspected and unpacked the 252,517-byte artifact to 82 files, and then booted core against the result -- the module registered its five mounts, seven streams and eight capabilities and resolved its client chunk, with no scratch directory left behind. Two defects this slice's own tooling caught, both of which had already been written down as classes: - the controller destructured runPurge at require time, capturing the function rather than the module, which made the one dependency whose ORDER matters the one that could not be substituted; - two swagger annotations carried an apostrophe inside a quoted string, dropped silently by swagger-autogen before slice 5 taught it to fail loudly. Co-Authored-By: Claude <noreply@anthropic.com>