diff --git a/website/BACKEND_DESIGN.md b/website/BACKEND_DESIGN.md index d463eb2..eb25ad9 100644 --- a/website/BACKEND_DESIGN.md +++ b/website/BACKEND_DESIGN.md @@ -275,7 +275,15 @@ Seeded with the 8 spec categories: `new-player-guide, maps-atlas, systems, items Seeded keys: `site_mode` (default `maintenance`), `site_mode_changed_at`, `site_mode_changed_by`, `maintenance_message`, `status_message`, `homepage_teaser`, `contact_email` (=UOMysticmoon@gmail.com), `site_title`, `player_registration` -(default `disabled`), `mobile_app_links_enabled`. +(default `disabled`), `mobile_app_links_enabled`, `module_source_hosts`. + +`module_source_hosts` is the allowlist of hostnames a module may be installed from +(MODULE_SYSTEM.md §2.7.2 decision 6), edited in Admin → Modules and audited as +`module.sources`. It is **bootstrapped** from `MODULE_SOURCE_HOSTS` and not owned by +it: `seedDefault` is an `INSERT IGNORE`, so the environment supplies a default on a +fresh install and changing the variable later cannot reach back in and overwrite what +an operator chose. Installs are `https`-only, every redirect hop is re-checked against +this list, and an empty value forbids every install rather than allowing every host. **Keys a MODULE seeds into this table.** `settings` is core's, but a module's schema fragment may `INSERT IGNORE` its own rows into it, and module-uo seeds two: @@ -731,7 +739,7 @@ are authoritative, and they answer different questions: | Artifact | Source of truth for | Generated by | |---|---|---| -| `server/routes.manifest.json` — mirrored as [api-route-inventory.json](./api-route-inventory.json) | **What URLs CORE serves.** 158 public routes + 2 on the internal listener, sorted, method + path only. | `npm run routes:manifest`, by walking the live Express stack | +| `server/routes.manifest.json` — mirrored as [api-route-inventory.json](./api-route-inventory.json) | **What URLs CORE serves.** 166 public routes + 2 on the internal listener, sorted, method + path only. | `npm run routes:manifest`, by walking the live Express stack | | `server/swagger/swagger-output.json` — merged into `/api/docs` | **What each core route means.** Parameters, bodies, response codes, security. | `npm run swagger`, from `#swagger.*` annotations | Both are **core's**. An installed module's routes are in neither: they are in that module's own diff --git a/website/MODULE_SYSTEM.md b/website/MODULE_SYSTEM.md index 89f2be5..9288f8c 100644 --- a/website/MODULE_SYSTEM.md +++ b/website/MODULE_SYSTEM.md @@ -1511,15 +1511,19 @@ far more than it appeared to be. Core writes into a **bind-mounted directory it host** (Phase 2 PR 9), so an escape is not confined to the container's own filesystem. The extractor therefore rejects rather than sanitises — an archive that needs correcting is an -archive that should not be trusted — on: any absolute path, any `..` segment, any entry that is not a -regular file or a directory (so no symlinks, hardlinks, devices or FIFOs), more than one top-level -directory, a top-level directory whose name is not the manifest's `id`, and a total unpacked size or -entry count over its cap. Unpacking goes to a temporary directory beside the target and is moved into -place only once the whole archive has been accepted, so a rejected or interrupted install never +archive that should not be trusted — on: any absolute path (POSIX, drive-relative or UNC), any `..` +segment, a NUL byte or a backslash in a path, any entry that is not a regular file or a directory (so +no symlinks, hardlinks, devices or FIFOs), more than one top-level entry, and a total unpacked size +or entry count over its cap. Unpacking goes to a temporary directory beside the target and is moved +into place only once the whole archive has been accepted, so a rejected or interrupted install never leaves a half-module on the volume for the next boot's scan to find. `tar` is a real dependency rather than a hand-rolled parser: the format has enough shape (long-name extensions, PAX headers, sparse entries) that parsing it is exactly the kind of code this list is trying to defend against. +*(Two corrections from building it, in slice 1 below: the top-level directory's NAME is not checked +against the module id — it is `module-uo-`, not `uo` — and `tar` has to be pinned forward +rather than merely depended on.)* + ##### What Phase 4 must prove | # | Criterion | How | @@ -1529,6 +1533,81 @@ sparse entries) that parsing it is exactly the kind of code this list is trying | 3 | A hostile archive cannot write outside `modules//` | Unit tests over the rejection list above, each with a real crafted tar | | 4 | A compose-declared module resolves at container start, offline | Container boots with the network down and an already-unpacked module, and comes up unchanged | +#### Slice 1 — the install service and the admin API (website#142, 2026-08-12) + +`modules/archive.js`, `modules/install.js`, `schema.runPurge()`, `lifecycle.stop()`, +`loader.stopHook()` and eight routes under `/api/v1/admin/modules`. 797 server tests (+76), manifest +158 → 166 public + 2 internal, OpenAPI gains eight operations and loses none. + +##### The two-pass unpack is measured, not assumed + +The reason `archive.js` inspects an entire archive before `tar.x` sees it is not that node-tar fails +to reject an escaping member — it rejects it. It rejects it **late**. An archive whose fourth member +is `mod/../../ESCAPED.txt`, extracted with `strict: true` under node-tar 7.5.22: + +``` +threw: TAR_ENTRY_ERROR: path contains '..' +LEFT ON DISK after the throw: [ 'first.txt', 'second.txt' ] +``` + +The loader scans a module directory at require time and asks only whether `module.json` is there +(§1.12), so a half-unpacked bundle is a module as far as the next boot is concerned. A `filter` +callback cannot fix this either: by the time it is asked about entry 400, the first 399 are written. +Hence inspect-then-extract, and hence the scratch directory that is removed on any failure with the +move into place as the last step. + +##### `tar` is pinned forward, and its advisory list is the argument + +Installing `tar` gets 6.x, which `npm audit` reports as **critical**. Reading the list is the useful +part, because almost every entry is this feature's own threat model: hardlink path traversal via +drive-relative linkpath, symlink poisoning through insufficient path sanitisation, hardlink target +escape through a symlink chain, PAX size override on GNU long-name headers causing a parser +interpretation differential, decompression DoS via unlimited input. The dependency is pinned to +`^7.5.22`, and — more to the point — refusing symlink and hardlink entry types **outright** is what +takes this extractor off most of that list rather than leaving it depending on the library to contain +them. + +##### The bundle's top-level directory is not named after the module + +`module-uo`'s release workflow packs `dist/module-uo-/`, so §2.7.2's original rule — reject +a top-level directory whose name is not the manifest's `id` — would have refused every real bundle. +Corrected above: exactly one top-level entry is required and that level is **stripped**, because its +name belongs to whoever published the bundle while the directory it lands in has to be the id the +loader scans for. What replaces the check is stronger anyway: the **unpacked** `module.json` must +agree with the install manifest about both id and version, so a manifest promising `uo` and +delivering something else is refused rather than installed under the name it promised. + +##### Enable is pinned by a test, because "fixing" it is a one-line change + +`lifecycle.stop()` dispatches one module's `onShutdown` and then moves the record — in that order, +since while the hook runs the module is still `started`, the only state in which its routes and the +world it is tearing down agree with each other. A hook that throws does not stop the disable, which +is the opposite of the boot path's rule and deliberate: there a failure means the module never became +safe to use, here the operator has asked for it to stop answering. + +There is no `start(id)` beside it, and `enable` deliberately does not touch the loader. That is +invisible in the code — it looks like an omission — so a test asserts `loader.setState` is never +called, with the reason written next to it. + +##### Two defects, both of a class already written down + +- **The controller destructured `runPurge` at require time.** `const { runPurge } = require(…)` + captures the function, not the module, so the one dependency whose *order* matters — purge must run + before the directory containing `purge.sql` is deleted — was the one that could not be substituted + in a test. Same class as `module-uo/server/core.js`'s never-destructure-a-getter rule. Every other + import in that file was already a namespace import; the destructure was the odd one out. +- **Two `#swagger` annotations carried an apostrophe inside a quoted string.** swagger-autogen + re-quotes to `'` before evaluating, so `module's` breaks the annotation and it is dropped. Found + only because slice 5 taught the generator to fail loudly; the fix is the typographic `’`, as before. + +##### Proved against the real release, not a fixture + +The published `module-uo` v0.3.0 install manifest, fetched over the real Gitea host and its redirect +chain: sha256 verified, the 252,517-byte artifact inspected and unpacked to 82 files, no scratch +directory left behind — and then core booted against the result and the module registered its five +mounts, seven streams and eight capabilities with its client chunk resolved. That is criterion 1 +minus the screen and the restart, both of which arrive in slice 2. + ### 2.8 SPA URL namespacing — a deliberate break **Decision: module pages are namespaced, and old paths are not redirected.** The site is not public diff --git a/website/api-route-inventory.json b/website/api-route-inventory.json index c3ee498..3860ae1 100644 --- a/website/api-route-inventory.json +++ b/website/api-route-inventory.json @@ -177,6 +177,38 @@ "method": "POST", "path": "/api/v1/admin/moderation/user/:discordId/notes" }, + { + "method": "GET", + "path": "/api/v1/admin/modules" + }, + { + "method": "POST", + "path": "/api/v1/admin/modules" + }, + { + "method": "DELETE", + "path": "/api/v1/admin/modules/:id" + }, + { + "method": "POST", + "path": "/api/v1/admin/modules/:id/disable" + }, + { + "method": "POST", + "path": "/api/v1/admin/modules/:id/enable" + }, + { + "method": "POST", + "path": "/api/v1/admin/modules/:id/purge" + }, + { + "method": "POST", + "path": "/api/v1/admin/modules/restart" + }, + { + "method": "PUT", + "path": "/api/v1/admin/modules/sources" + }, { "method": "GET", "path": "/api/v1/admin/pages"