Files
website/server/src/modules/archive.js
wtclaude b30e82cde2
All checks were successful
PR Checks / bot-install (pull_request) Successful in 17s
PR Checks / client-build (pull_request) Successful in 26s
PR Checks / server-tests (pull_request) Successful in 33s
feat(modules): install, uninstall, purge and restart (phase 4, slice 1)
The consumer half of a release module-uo's CI has been publishing since
phase 3 closed. Before this, core had the installed_modules provenance
columns and no code that could ever fill them: nothing fetched, verified,
unpacked, removed or purged anything, and there was no admin route at all.

Adds modules/archive.js, modules/install.js, schema.runPurge(),
lifecycle.stop(), loader.stopHook(), and /api/v1/admin/modules with eight
routes. 797 server tests (+76), manifest 158 -> 166 + 2 internal, OpenAPI
gains 8 operations and loses nothing.

Reject, never sanitise
----------------------
The download is the easy part: an https-only allowlist re-checked on every
redirect hop, a declared sha256 compared against the bytes that arrived, and
a byte cap. Unpacking is where the archive chooses the filenames, and core
writes into a directory bind-mounted from the host, so an escape is not
confined to the container.

archive.js inspects the whole archive before a byte is unpacked and refuses
absolute and drive-absolute paths, `..` segments, NUL bytes, backslashes,
anything that is not a regular file or a directory, more than one top-level
entry, and anything over the entry or byte caps. Refusing symlinks and
hardlinks outright is what keeps this off the majority of node-tar's
published advisories rather than depending on the library to contain them.

That two-pass shape is load-bearing, and it was measured rather than assumed:
extracting an archive whose fourth member escapes upward throws under
node-tar 7.5.22 -- and leaves the first three members on disk. The loader
scans that directory at require time on the next boot, so a half-unpacked
module is a module. Everything therefore happens in a scratch directory that
is removed on any failure, and the move into place is the last step.

`tar` is pinned to ^7.5.22 rather than the ^6 that installs by default: 6.x
is flagged critical, and reading the advisory list is what the file's header
now says out loud -- almost all of it is hardlink or symlink traversal and
PAX header interpretation differentials, which is exactly this feature's
threat model.

Two things the plan had wrong
-----------------------------
The bundle's top-level directory is `module-uo-<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>
2026-08-12 03:09:45 -05:00

222 lines
8.7 KiB
JavaScript

// ── 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-<version>/`, 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,
}