Plugins, Java, telemetry and deregister over HTTP (#53) - #94
Conversation
Closes the last IPC-only gap in the operations surface.
Mods are gated on `files`, not a new `mods` scope: installing a jar writes
into the server directory and deleting one removes a file from it, both of
which `files` already permits outright. A separate scope would be a strict
subset of one the caller must already hold — a boundary in name only.
Java runtimes and telemetry retention are host-wide, so no per-server scope
can express them; they are owner-session only. That has a consequence worth
stating: `principalForKey` always builds `role: 'user'`, so no API key can
reach them however it is scoped. The smoke proves it with a key holding
every scope on every server.
The telemetry patch is validated rather than cast (`shared/ops.ts`). It is
persisted, so a bad value does not fail the request that set it — it fails
every metrics prune afterwards, across restarts. `{ enabled: "false" }` is
the sharp case: truthy, so it reads as on while the operator believes they
turned it off.
Server lifecycle is exposed by halves. `DELETE /api/servers/:id` deregisters
and keeps every file; creating a server (a host path chosen by the caller)
and deleting its files (a recursive rmSync) stay off the surface, for the
same reason `javaPath` did in #93. The reversible half ships, the
irreversible half does not, and the docs say so.
All twelve smoke gates green.
Two findings on the deregister route, one of them a real parity bug.
`ipc/register.ts` pairs `registry.removeServer(id, …)` with
`alerts.dropServer(id)` by hand — `removeServer` itself does not drop rules,
only metrics and events. The new HTTP route made the first call and not the
second, so a server deregistered over the API left every rule aimed at it in
the store, evaluated against a server that no longer exists, until the next
launch swept them in `initAlerts`. The route now makes both calls, with a
comment saying why they are separate so the next route does not repeat it.
Second: `{ filesKept: true }` reads as "nothing was lost", and that is not
what happened. Deregistering deletes this server's metrics folder, its event
timeline and its alert rules; a rescan re-adds the folder under a new id with
no history attached. The response now carries `historyDropped` and a count of
`alertRulesRemoved` — a count rather than a flag, because an integration that
created those rules through the API is the one caller who can be surprised by
losing them.
The smoke seeds a rule on the throwaway fixture and asserts both the store and
the reported count, which is what caught the missing call in the first place.
Self-reviewTwo findings, fixed in 9d176dc. 1. The route dropped the server but not its alert rules. registry.removeServer(id, deleteFiles)
alerts.dropServer(id)The new HTTP route made the first call and not the second. A server 2. Finding 1 is the one worth noting: the assertion written for finding 2 — Gates re-run after the fix: |
Closes the last IPC-only gap in the operations surface (#53, part 3 of 3).
What is now reachable over HTTP
GET/POST/DELETE /api/servers/:id/mods…(list, search, detail, updates, install, update, toggle, delete)filesGET /api/java,POST /api/java/installGET/POST /api/telemetryDELETE /api/servers/:id?confirm=true(deregister, files kept)Decisions worth reviewing
Mods are gated on
files, not a newmodsscope. Installing a pluginwrites a jar into the server directory; deleting one removes a file from it.
filesalready permits both outright, by hand. Amodsscope would be a strictsubset of a scope the caller must already hold — it would look like a boundary
while being none.
No API key can reach the host-wide routes.
principalForKeyalways buildsrole: 'user'— a key carries scopes, never a role — sorole !== 'owner'issession-only by construction. That is easy to read as an oversight, so it is now
stated in the code and in the docs, and the smoke pins it with a key holding
every scope on every server: it still gets 403. Breaking the check
deliberately made that assertion fail, so it is not vacuous.
The telemetry patch is validated, not cast.
sanitizeTelemetryPatchinshared/ops.ts:enabledboolean, three integers in range, unknown keysrefused rather than dropped. This config is persisted, so a bad value does not
fail the request that set it — it fails every metrics prune afterwards, across
restarts, from a config file nobody suspects.
{ enabled: "false" }is thesharp case: truthy, so it reads as on while the operator believes they turned
it off.
Server lifecycle is exposed by halves.
addServer(path)takes a hostfilesystem path chosen by the caller and
removeServer(id, true)is a recursivermSync— the same class of thingjavaPathwas refused for in #93. Deregisterkeeps every file and is recoverable by a rescan, so it ships; the other half
does not, and
docs/api-operations.mdsays why rather than leaving a gap.Smaller ones
invalid-mod-pathanswers400, not409— it is the caller namingsomething they may not name, not a conflict with the server's state.
DELETE …/modschecksrelbeforeconfirm, so a malformed call is notaudited as a refused delete of
""— a decision nobody made. Asserted.409: dropping it from the registryorphans the process, because every lookup that would reach for its config to
stop it then 404s.
Verify
All twelve gates exit 0. A new
MSMS_SMOKE_WEBblock covers the scope split,the traversal refusal, the audit entries, every telemetry rejection (with the
field named), and a deregister that leaves
server.jaron disk. Modrinth isonly exercised through its parameter validation, so the gate does not need the
network.