docs(website): phase 4 decisions 5-6, slices 1-2, and a rule main currently gets wrong #142

wtclaude · 2026-08-12T08:57:43Z

wtclaude commented

2026-08-12 08:57:43 +00:00

docs#141 merged at its first commit, so three commits pushed to that branch afterwards never landed. This is them. Same thing happened to Phase 3 slice 1's docs — worth noting that a push to an already-merged PR's branch is silent.

One of them matters more than a write-up: main currently documents a rule the code does not implement.

The correction

§2.7.2's threat-model list on main says the extractor rejects "a top-level directory whose name is not the manifest's id". It does not, and must not — module-uo's release workflow packs module-uo-<version>/, so that rule would refuse every bundle that exists. The extractor requires exactly one top-level entry and strips it, and checks the unpacked module.json against the install manifest instead, which is a stronger check.

Also corrected there: tar has to be pinned forward (^7.5.22), not merely depended on — the default ^6 is npm-audit critical, and the advisory list is this feature's own threat model.

What else is in here

Decisions 5 and 6 (settled during slice 1, already implemented in the merged code):

Purge is offered inside the uninstall flow, because purge.sql lives inside the directory uninstall deletes and there is no later. §2.5 is amended — "never bundled into uninstall" becomes "never implied by uninstall".
The host allowlist bootstraps from MODULE_SOURCE_HOSTS into a settings row and is admin-owned thereafter.

Slice 1's record (website#142), including the measurement behind the two-pass unpack: node-tar does reject an escaping member, but late — an archive whose fourth member escapes throws and leaves the first three on disk.

Slice 2's record (website#143), and the three defects standing the screen up against a live server exposed. Two are older than Phase 4 and none was reachable from a test:

A fresh install over a startup_failed row rendered the old failure — "module directory not present on the volume", one second after writing the files there — and suppressed the restart banner the install had just asked for.
The boot refresh had been nulling every install's provenance. Unfindable before Phase 4 wrote the first non-null value those columns ever had, and the model's test fake reproduced the defect faithfully.
process.kill(pid, 'SIGTERM') kills outright on Windows, where POSIX signals do not exist — no onShutdown, no pool close, no log flush. process.emit('SIGTERM') reaches the same listener everywhere. Deployment is Linux containers and would never have shown it.

Plus BACKEND_DESIGN.md: the module_source_hosts seeded key, the COALESCE on installed_modules' provenance, the refreshed route count (166), and api-route-inventory.json re-mirrored.

Docs only.

AI disclosure

This contribution was AI-assisted (Claude Code).

🤖 Generated with Claude Code

**docs#141 merged at its first commit**, so three commits pushed to that branch afterwards never landed. This is them. Same thing happened to Phase 3 slice 1's docs — worth noting that a push to an already-merged PR's branch is silent. One of them matters more than a write-up: **`main` currently documents a rule the code does not implement.** ## The correction §2.7.2's threat-model list on `main` says the extractor rejects "a top-level directory whose name is not the manifest's `id`". It does not, and must not — `module-uo`'s release workflow packs `module-uo-<version>/`, so that rule would refuse every bundle that exists. The extractor requires exactly one top-level entry and **strips** it, and checks the *unpacked* `module.json` against the install manifest instead, which is a stronger check. Also corrected there: `tar` has to be pinned forward (`^7.5.22`), not merely depended on — the default `^6` is npm-audit **critical**, and the advisory list is this feature's own threat model. ## What else is in here **Decisions 5 and 6** (settled during slice 1, already implemented in the merged code): - Purge is offered **inside the uninstall flow**, because `purge.sql` lives inside the directory uninstall deletes and there is no later. §2.5 is amended — "never bundled into uninstall" becomes "never *implied* by uninstall". - The host allowlist **bootstraps from `MODULE_SOURCE_HOSTS` into a settings row** and is admin-owned thereafter. **Slice 1's record** (website#142), including the measurement behind the two-pass unpack: node-tar *does* reject an escaping member, but late — an archive whose fourth member escapes throws and leaves the first three on disk. **Slice 2's record** (website#143), and the three defects standing the screen up against a live server exposed. Two are older than Phase 4 and none was reachable from a test: - **A fresh install over a `startup_failed` row rendered the old failure** — "module directory not present on the volume", one second after writing the files there — and suppressed the restart banner the install had just asked for. - **The boot refresh had been nulling every install's provenance.** Unfindable before Phase 4 wrote the first non-null value those columns ever had, and the model's test fake reproduced the defect faithfully. - **`process.kill(pid, 'SIGTERM')` kills outright on Windows**, where POSIX signals do not exist — no `onShutdown`, no pool close, no log flush. `process.emit('SIGTERM')` reaches the same listener everywhere. *Deployment is Linux containers and would never have shown it.* Plus `BACKEND_DESIGN.md`: the `module_source_hosts` seeded key, the `COALESCE` on `installed_modules`' provenance, the refreshed route count (166), and `api-route-inventory.json` re-mirrored. --- Docs only. ### AI disclosure - [x] This contribution was AI-assisted (Claude Code). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

wtclaude added 3 commits 2026-08-12 08:57:46 +00:00

docs(website): two more phase 4 decisions, one of them a correction 6bca9adf49

Both came out of writing slice 1 against §2.5 rather than from planning, and
one of them means §2.5 was wrong rather than merely vague.

5. Purge is offered inside the uninstall flow, because it cannot be offered
   after it. §2.5 promised a disabled row an operator could purge later — but
   purge.sql is a file INSIDE the module directory that uninstall deletes, so
   "later" would have left a Purge button with nothing to run. The destructive
   choice now sits at the one moment the file is still there: an opt-in
   checkbox in the uninstall dialog, beside the standalone Purge action on a
   module that is still installed. "Never bundled into uninstall" becomes
   "never implied by uninstall". The cost is stated rather than hidden — an
   operator who does not tick it keeps their tables, and reinstalling is how
   they get the ability to drop them back.

6. The host allowlist bootstraps from MODULE_SOURCE_HOSTS and then lives in
   the database, admin-editable, with the variable no longer overriding it —
   the shape core already uses for seeded settings. The argument for keeping
   it in the environment does not survive contact with what this screen is: an
   admin who can install a module already has code execution in the process,
   so an admin who can name a host has gained nothing they did not have.

Also renames the subsection to six decisions and updates acceptance criterion
2 to name the checkbox.

Co-Authored-By: Claude <noreply@anthropic.com>

docs(website): record slice 1, and correct two rules it disproved 88b6257333

The install service and admin API as built (website#142), plus the two
things §2.7.2 got wrong before there was any code to check them against.

The top-level directory rule was checking against nothing real. module-uo's
release workflow packs module-uo-<version>/, not uo/, so "reject a top-level
directory whose name is not the manifest's id" would have refused every
bundle that exists. That level is stripped instead - its name belongs to
whoever published the bundle, the directory it lands in has to be the id the
loader scans for - and what replaces the check is stronger: the UNPACKED
module.json must agree with the install manifest about id and version.

And tar has to be pinned forward rather than merely depended on. Installing
it gets 6.x, which npm audit reports as critical, and the advisory list reads
as this feature's own threat model: hardlink traversal via drive-relative
linkpath, symlink poisoning, hardlink escape through a symlink chain, PAX
size override on GNU long-name headers, decompression DoS. Refusing symlink
and hardlink entry types outright is what takes the extractor off most of
that list rather than depending on the library to contain them.

Also records the measurement behind the two-pass unpack, which was assumed in
the plan and is now known: node-tar DOES reject an escaping member, but late.
An archive whose fourth member escapes throws and leaves the first three on
disk - and the loader only asks whether module.json is present, so a
half-unpacked bundle is a module as far as the next boot is concerned.

Refreshes api-route-inventory.json (158 -> 166 public) and the route count in
BACKEND_DESIGN, and documents module_source_hosts beside the other seeded
settings keys - bootstrapped from the environment, owned by the database.

Co-Authored-By: Claude <noreply@anthropic.com>

docs(website): record slice 2, and the three defects a browser found f753dfcff8

The Modules screen as built (website#143), and the three things standing it
up against a live server exposed. Two of them are older than this phase and
neither was reachable from a test.

- A fresh install over a row the previous boot left `startup_failed` rendered
  the old failure - "module directory not present on the volume", one second
  after the files were written there - and, because that branch is not
  pending, suppressed the restart banner the install had just told the
  operator to use. Fixed by a derivation rather than a special case: the
  loader scans once at require time, so a module on the volume with no live
  record arrived after that scan and anything the row says predates it.

- The boot refresh had been nulling every install's provenance. source and
  sha256 exist for this screen and never survived a restart, because
  lifecycle.boot() re-records with neither and the upsert assigned both
  unconditionally. It could not have been found before Phase 4 wrote the first
  non-null value those columns ever had - and the model's test fake
  reproduced the defect faithfully, assigning exactly like the SQL.

- The restart killed the server outright on Windows. process.kill(pid,
  'SIGTERM') reaches the graceful handler on Linux and is unconditional
  termination where POSIX signals do not exist, so a Windows host got no
  module onShutdown, no pool close and no log flush. process.emit('SIGTERM')
  reaches the same listener everywhere. The test had stubbed process.kill and
  asserted the call - precisely the call whose meaning differs by platform.

That last one generalises: deployment is Linux containers and would never
have shown it. A smoke that only runs where the code ships cannot find a class
of defect that only bites the people developing it.

Also documents the COALESCE in BACKEND_DESIGN's installed_modules section.

Co-Authored-By: Claude <noreply@anthropic.com>

whitlocktech merged commit 245be5dc91 into main

2026-08-12 09:11:29 +00:00

whitlocktech referenced this issue from a commit

2026-08-12 09:11:29 +00:00

Merge pull request 'docs(website): phase 4 decisions 5-6, slices 1-2, and a rule main currently gets wrong' (#142) from docs/module-phase4-slices into main

whitlocktech deleted branch docs/module-phase4-slices

2026-08-12 09:11:30 +00:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: RunicGateway/docs#142