mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-22 06:56:35 +00:00
62d4cdcc183d42e6ab0023b929ede47dde83753c
55 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Maurice
|
218d8baf0a |
feat(server): read visits and recorded routes from Dawarich
A new `integrations/dawarich` domain behind its own addon. Each reader connects their own instance; the API key is encrypted at rest and never returned. Nothing is written back to Dawarich, and no raw position is kept in TREK: the recorded route is fetched per request for the window being looked at and thrown away again. What it produces are suggestions, never facts. A recorded stay becomes a proposed place or journal entry, a country or city becomes a proposed Atlas mark, and a stay near a wish becomes a proposed tick-off; each one waits for the reader to confirm it. Identity is (user, Dawarich visit), so a second sync neither duplicates nor overwrites what was accepted, and deleting the result puts the suggestion back on the list. The sync runs as a job on the central scheduler, one user at a time, guarded against overlapping runs. |
||
|
A1phaN
|
57eac32f9f |
feat(map): amap as map api provider and display template
|
||
|
Maurice
|
b3d69aa760 |
feat(places): record which search result was picked, on this instance only
The corpus behind "is our own index good enough to replace Nominatim". When somebody searches and then picks a result, the client posts the query, the rank and the pick. Nothing else: no user, no trip, no session, and coordinates only to three decimals. Deliberately decoupled from the search endpoint. Correlating a response with a later pick would mean handing out a ticket on POST /api/maps/search and threading it through all fifteen call sites; the client already holds everything the row needs at the moment of the pick. Evaluation happens offline afterwards: export the corpus, run it against a candidate index, count how often the pick came back in the top five. That is why collection can start before such an index exists, which is the whole point, because the corpus needs weeks of real use and the index needs a server, and those two waits should not be serial. Off unless an admin switches it on, and it reads === 'true' rather than !== 'false'. The three older places switches fail open because a migration backfilled a row for installs already using them; nothing uses this one, so an instance that upgrades and never visits the admin panel logs nothing. Reading is admin only, retention is 180 days, and the switch state is on the unauthenticated app-config endpoint on purpose: whether an instance records what its users search for is something a visitor may know before logging in. |
||
|
Maurice
|
c026c5b1c9 |
fix(auth): passkey_configured stops advertising the phantom localhost config (#2147)
Without any configuration getAppUrl() invents http://localhost as the relying party, and the UI advertised passkeys that every click answered with not configured. A localhost RP only counts as configured now when the operator declared one. The origin check also binds to the expected origins again instead of accepting every subdomain of the RP id; that widening never shipped in a release. |
||
|
Maurice
|
4704bf5903 | fix(auth): fail fast on passkey origins outside the relying party scope (#2147) | ||
|
Maurice
|
4b120d39c8 |
feat(api): separate API keys from MCP tokens, add travellers
An MCP token drives every assistant tool; an API key reads trips over HTTP. Handing the same credential to a third-party integration gives it the whole MCP surface, so the two are now separate kinds on `mcp_tokens` and the kind sits in the WHERE clause — a token of the wrong kind is indistinguishable from one that does not exist. Also adds `travellers` to the trip payload: display name and an owner flag, no ids and no email addresses. An integration key reads its owner's itinerary; it is not a way to enumerate the people around them. Documents the surface in the wiki, since integrators need something to read that is not the source. |
||
|
Maurice
|
1ad2e92142 |
fix: make the app keyboard-reachable, and close four DoS-able patterns
The bulk of what SonarQube still had open on the reliability side, read site by
site rather than swept. Two clusters are worth calling out because they are
defects, not lint.
**Four regexes could be made to hang the process.** The worst is
`/.+@.+\..+/` in the forgot-password path: three unbounded `.+` that all
overlap, unanchored, on an endpoint that needs no login and whose schema is a
bare z.string() under a 100kb body cap. A thousand '@' characters had not
finished after 100 seconds — one request pins the event loop, and Node has one.
EMAIL_REGEX in auth.helpers is the same shape on register and profile update
(310ms for 8kb). The two tag strippers over an uploaded KML description and over
Commons metadata took 0.52s on a 16k run of '<'. Each rewrite was proven
equivalent by brute force against the original — millions of strings for the
coordinate patterns, 671,876 for the tag stripper — because a subtly different
regex on a parsing path is worse than the finding.
`<[^>]+>` could not be fixed in the pattern: `[^>]` matches `<` too, so the only
unambiguous form changes what happens to malformed markup. It is now a scan in
common/stripHtmlTags.ts, quirks included — `<>` is left alone, `<a<b>` is one
tag — with the equivalence pinned by a test that compares against the old regex
over every string up to length six.
**And roughly 500 places could only be reached with a mouse.** Clickable divs
became real buttons where the markup allowed (52 of them), took
role + tabIndex + Enter/Space where they had to stay divs, and modal backdrops
took role="presentation" — the honest role, since a backdrop is a mouse shortcut
for a Cancel control that is already in the tab order, and making it a button
plants a nameless full-screen tab stop around its own contents.
Alongside those: `.sort()` calls that now say the order they always had, `{n &&
<X/>}` guards that rendered a literal "0" for a zero-byte file and for a place at
latitude 0, an optimistic budget write that stored a boolean where the contract
and both transports use 0/1, and the Number statics.
sonar-project.properties documents the six that were deliberately left: two wiki
patterns with no overlapping quantifier pair whose "fix" would stop parsing
`[see [1]](url)`, four tar header fields bounded at 12-155 bytes by the format,
two djb2 hashes where the 32-bit wrap is the algorithm, two `| 0` that are also
coercing undefined to 0 on a path where the API sends no user_id, and the MCP
session maps Sonar reads as empty because they are filled from another module.
|
||
|
Maurice
|
2c2d008dc2 |
fix: clear every open Sonar security issue, and half the reliability ones (#2037)
* fix(security): mint idempotency keys properly, and resolve the extractor by path Three findings out of a sweep of every open Sonar security issue. The other 49 turned out to be false positives and are documented in sonar-project.properties rather than changed — each entry scoped to a file, never a whole rule, so a future Math.random() on an auth path still fails. The idempotency key is the one that can lose data. Both producers fell back to Math.random when crypto.randomUUID was missing, and randomUUID needs a secure context — TREK's own quickstart is http://localhost:3000 and LAN installs run on http://192.168.x.x, so on the deployments the README documents the fallback is what actually runs. The axios one was `Math.random().toString(36).slice(2)`, which is not length-stable: 0.5 yields a single character and 0 yields the empty string, which the interceptor reads as "no key" and skips deduplication for entirely. Two writes sharing a key collapse into one — the second gets the first one's cached response and the edit is gone with no error anywhere. Both now come from utils/randomId, which has the getRandomValues rung that works in insecure contexts too. The extractor probe stored the bare binary name, so every extraction re-resolved it through whatever PATH held at that moment. The official image sets KITINERARY_EXTRACTOR_PATH and never reaches that branch, but a bare-metal install without it ran an unqualified name from a PATH it does not control. It now walks the search path itself and keeps the absolute path of the file it verified; execFileSync also drops the /bin/sh hop the old probe needed. PATH comes through app-config because process.env is walled off in src/. * fix: four tests and sorts that were not doing what they claimed Sonar reported 43 high-impact reliability issues. Reading all of them, four are real; the rest are cosmetic or would be made worse by the suggested change. budget.test.ts hand-built `paid: true`, but both emitters send `paid: body.paid ? 1 : 0` and the handler stores the value verbatim — the test asserted a payload the server never sends. The line below it already used a number. admin.controller.test.ts had a `.resolves` assertion with no await. The promise was dropped, so that line could not fail the test. The two to-do category lists sorted with a bare .sort(), which is byte order: every accented name files behind the whole ASCII range, so in a German trip "Übernachtung" lands after "Zelt". TREK ships 23 locales, so this is real data. One side effect worth knowing: katColor picks a category's dot colour by its index, so re-sorting can move which category shows which colour — it already moves whenever a category is added, and the colour carries no meaning. Deliberately not changed: the two `| 0` truncations (Math.trunc is not equivalent — `| 0` wraps at 32 bits and these carry millisecond timestamps), the two .map() callbacks whose second parameter is declared and used, and the sha1 in thumbnail.service (its own comment says a swap would orphan every existing thumbnail). * refactor: give every button a type, and prefer the Number statics Two mechanical sweeps, no behaviour change intended in either. A <button> with no type attribute is type="submit" per the HTML spec. Outside a form that does nothing today, but it turns into a surprise the moment the markup is wrapped in one, which is what Sonar's S9011 is warning about. 870 buttons get an explicit type="button". Two things were left alone on purpose: buttons that take a {...spread}, because the spread may carry a type and the merge order decides which wins, and anything inside a <form> — there the answer is a judgement call, and checking all eleven form files found every real submit button already carrying type="submit". The Number sweep covers only the three globals that are exactly equivalent to their Number twin: parseInt, parseFloat and NaN. isNaN is NOT in it — the global coerces its argument, so isNaN('abc') is true where Number.isNaN('abc') is false, and renaming it would change what the code does. It reaches server and plugin-sdk as well as the client; both copies of egress-policy.ts got the identical edit, so the mirror the SDK's parity test guards stays intact. |
||
|
Maurice
|
23196aa038 |
refactor(demo): keep the demo password in one place
It was written out twice — once in the seeder that creates the account, once in the config payload the login screen reads it from. The two have to match exactly or the published demo credentials stop working, which is a poor thing to leave to copy-paste. It moves next to DEMO_EMAIL_PRIMARY, which the same comment block already argues for. Also clears a Sonar S2068 "hard-coded password" on the auth service: the value is public on purpose, but a literal next to a `demo_password` key is exactly what the rule looks for. |
||
|
Maurice
|
38f3cb3101 |
fix(server): the second pass over this branch's own fixes
An adversarial review of what had already landed here turned up three that
were wrong, and they are the reason for this commit.
The CSP change removed 'unsafe-eval', which is not leftover: heic-to's
libheif build initialises embind through new Function(), and without it every
iPhone .heic upload fails in the browser. 'wasm-unsafe-eval' alone was tried
once already and was not enough (
|
||
|
Maurice
|
8e5fdaa877 |
fix(server): tighten the guard, audit and token paths
Permissions cached an empty map before it had loaded, so a DB error during the load cached permissive defaults and every later check answered from that cache - fail-open, and it never expired. The rate limiter grew its per-IP map forever; the GC the legacy version had was dropped in the fold. Idempotency did a check-then-insert, so two concurrent duplicates both ran the handler. Audit rows recorded whatever X-Forwarded-For said, which the client writes, and the request-body debug logger redacted by exact key match - it missed refresh tokens, api keys and PKCE verifiers. FORCE_HTTPS built its redirect from the client's Host header. CSP asked for unsafe-eval on top of the wasm-unsafe-eval it actually needs. The OAuth deny path redirected to an unvalidated URI, ws-token minting had no rate limit at all, and PKCE was skipped when code_verifier was absent rather than refused. An MCP session resumed with a token whose scopes had since been reduced kept the capability set it was created with, and the session cap was checked before the async registration that increments it. Plugin frame containment was lexical, so a symlink out of the directory was never noticed, and instance config accepted keys the manifest never declared. |
||
|
jubnl
|
76c8c349b1 |
Fold nest mcp (#2017)
* refactor(server): fold @trek/nest-mcp into server/src/nest-mcp * docs(server): point nest-mcp references at server/src/nest-mcp * remove tracked md file |
||
|
jubnl
|
305b113d5f |
Feat/storage driver (#1949)
* feat(storage): storage key validation, driver contract suite, LocalDriver * feat(storage): MirrorDriver composite backend * feat(server): add TREK_UPLOADS_DIR env var for the uploads root * feat(storage): settings-backed registry, StorageService facade, module wiring * test(storage): pin src/nest/storage coverage thresholds * feat(storage): multer upload options factory * refactor(files): spool trip-file uploads through storage * refactor(collab): spool note attachments through storage; normalize files/ prefix * refactor(journey): spool journey media uploads through storage * refactor(trips): spool trip cover upload through storage * refactor(places): spool place-image and collection uploads through storage * refactor(auth): spool avatar upload through storage * fix(server): consolidate allowed_file_types onto AllowedFileTypesService * fix(storage): default defParamCharset to utf8 in the upload factory * test(db): re-point the trailing-migration guard at the trip_files prefix strip * fix(storage): age-gate the boot spool sweep * test(files): key FILE-P02 residue checks on upload size * test(storage): raise src/nest/storage coverage pin * refactor(platform): serve /uploads routes through storage * refactor(files): serve trip-file downloads through storage * refactor(memories): serve photo bytes through storage * refactor(maps): stream place-photo bytes through storage * refactor(share): stream shared place-photo bytes through storage * refactor(journey): serve public journey assets through storage * fix(share): answer place-photo cache miss with 204 like the maps proxy * test(server): close serving-swap gaps surfaced by the full tier * test(storage): raise src/nest/storage branches pin * refactor(place-photos): route place-photo cache through storage * refactor(places): reclaim place images and collection covers through storage * refactor(trips): trip covers through storage * refactor(auth): avatar reclaim through storage * refactor(collab): note-file deletion through storage * refactor(files): trash deletion through storage * refactor(memories): trek photo cache internals through storage * refactor(memories): thumbnail service category-addressed * refactor(memories): immich upload reads through storage; drop uploads-root * refactor(journey): photo deletion through storage * fix(memories): reclaim orphaned trek photo cache objects in the ttl sweep * feat(journey): journey-thumbs orphan sweep and delete-site derived-artifact reclaim * test(server): close slice-4 gaps surfaced by the full tier * test(coverage): raise slice-4 domain pins * refactor(backup): list, existence and delete through storage * refactor(backup): download through storage sendToResponse * refactor(backup): create backup through per-category storage enumeration * refactor(backup): restore and upload-restore through storage * refactor(backup): auto-backup retention through storage * refactor(server): boot dir-creation to driver init * test(server): close slice-5 gaps surfaced by the coverage tier * test(coverage): raise slice-5 backup pin * test(server): resolve async backupFileExists mocks for the tests typecheck * test(server): pin ranged-read stat and zero-byte roundtrip in the storage driver contract * feat(server): TREK_S3_* env wiring — schema, s3 namespace, all-or-nothing boot rule * feat(server): S3 driver core — aws-lite client seam, stat/delete, driver-owned deadlines * refactor(server): type the aws-lite S3 client via s3-types reference * feat(server): S3 driver getStream and list — ranged full-object stat, iterator pagination * feat(server): S3 driver put — bounded-peek routing, Upload inactivity deadline * fix(server): S3 driver Upload monitoring — Transform interposition, stream cleanup * feat(server): env-declared s3-main storage backend; settings-declared s3 rejected * test(server): env-gated S3 driver contract suite against MinIO * fix(server): S3 driver zero-byte put — work around aws4 duplicate Content-Length signing defect * fix(server): scope the zero-byte put workaround to buffered bodies; bind test MinIO to localhost * ci(server): MinIO-backed S3 driver contract job * ci(server): align s3-contract shim comment and MinIO bind with references * docs: S3 storage backend configuration and off-box backup mirroring * docs: restore README formatting, keep S3 additions only * ci(server): make live-MinIO contract runs manual-only (AIStor licensing) MinIO's community line is discontinued entirely; the maintained successor, AIStor, requires a license key that can't safely live in a shared CI runner. The s3-contract CI job is removed; live-provider contract runs against MinIO/AIStor are now a manual, license-gated procedure documented in docker-compose.minio-test.yml's header. * chore(server): final-review polish — stale CI refs, dockerignore key, boundary pins, zero-byte tempfile contract case * expose admin ui * feat(shared): storage backend type registry, wire schemas, category relocation * feat(server): accept settings-declared s3 backends — shared-schema options, decrypt-at-build, registry preview() * feat(server): remove TREK_S3_*/TREK_UPLOADS_DIR — tombstone boot rule, built-in override as the relocation path * feat(server): deriveSecurity.encryptionKeySet and storage secret plumbing (gate, encrypt, unmask, mask, redact) * chore(server): drop storage-secrets import that only Task 8's probe helper will use * feat(server): seed-once storage-config.json boot import — fail closed on any invalid seed * feat(server): registry snapshot() — effective storage config with provenance * feat(server): api/admin/storage GET/PUT — masked secrets, preview-gated saves, wired reload, audits * feat(server): api/admin/storage/test — per-target ephemeral probes, mirror targets probed individually * test(server): e2e coverage for api/admin/storage — guards, managed refusal, envelope, probes * test(server): give e2e harness DBs the app_settings table the storage registry reads at boot * test(coverage): raise pins after the storage admin-config slice * fix(server): final-review polish — encryption-gate wording, all-field sentinel guard, README/e2e notes * test(server): storage slice-1 review follow-ups — mirror s3-replica probe decrypt, all-encrypted seed without key, test-route 401 * feat(i18n): storage admin domain + tab label in every locale * feat(client): adminApi storage endpoints — getStorage, updateStorage, testStorageBackend * feat(client): storage admin draft-document helpers + relative-time util * feat(client): schema-driven storage BackendForm — fields by kind, mask echo, encryption banner * feat(client): admin storage panel — backends list, category map, health strip, one-PUT save * feat(client): storage tab on the admin page * feat(client): storage admin panel and section on the phone shell * fix(client): block renaming a storage backend onto an existing name * feat(server): allow mirrors on any storage category — lift the backups-only rule * feat(i18n): replicas-on-primary mirror strings in every locale * feat(client): mirror fold/synthesize model helpers for replicas-on-primary * feat(client): BackendForm mirror-targets composer; raw mirror type hidden * feat(client): desktop storage panel folds mirrors into their primaries * feat(client): phone storage panel folds mirrors into their primaries * fix(client): strip removed backends from remaining mirror replicas — no phantom refs * feat(storage): retire photos as a configurable category — served-only via the ServedCategory seam * feat(i18n): descriptive storage category names and descriptions in every locale * feat(client): descriptive category names, descriptions and id badges in the storage panel * docs(storage): fix stale nine-category comments after the eight-category shrink * docs: storage configuration via the admin UI and seed file — README + compose * docs(wiki): Admin Storage page and storage cross-links * docs(server): storage domain blueprint entry; TREK_PLACE_PHOTO_DIR tombstone clarifier * refactor(server): drop the storage env-var tombstones — the variables never shipped * docs: storage config is simply new in v4 — drop the removed-variables story * feat(shared): backfill-status and usage wire schemas on the storage admin state * feat(server): MirrorDriver.backfill — prefix-scoped, size-compared, cancellable replica catch-up * feat(server): one-at-a-time storage backfill jobs + the storage events bridge * feat(server): storage usage scan — nightly cron, on-demand service, persisted with computedAt * feat(server): replica_failure admin notifications — events bridge, hourly debounce, channel registration * feat(i18n): replica-failure notification text in every locale * feat(server): storage backfill and stats-refresh admin endpoints * feat(client): adminApi backfill and stats-refresh endpoints * feat(i18n): storage sync and usage strings in every locale * feat(client): desktop storage panel — sync now, usage display, backfill polling * feat(client): phone storage panel — sync now, usage display, backfill polling * fix(client): per-category usage line on the phone storage panel * docs: sync now, usage stats and replica-failure notifications * test(server): admin scope now includes replica_failure, not just version_available Commit |
||
|
Maurice
|
1a0834b866 |
fix(maps): resolve the Places key per instance, not per whoever is asking
Searching worked for the admin and 403'd for every other member. getMapsKey read the calling user's own row first and then fell through to "any admin" via LIMIT 1 with no ORDER BY, so the admin who pasted the key hit their own row while everyone else got a non-deterministic stranger's. That fallback is what CLAUDE.md forbids. The Places and Unsplash keys bill the install, not the account that pasted them in, so they become instance configuration: an encrypted app_settings row, resolved as env -> instance -> the caller's own row, the same shape #1772 chose for the LLM endpoint. The per-user column stays as the last step because /me/api-keys has always accepted a personal key. The trailing migration carries an existing install's key into the instance row so nothing degrades to OpenStreetMap on upgrade, but only where that key already was the whole install's: as soon as a second row holds one, it is left alone, because the instance row outranks everybody's own column and an upgrade must not move a member onto someone else's key and bill. Two things from the same report: The key test button probed with field mask places.displayName while the search asks for places.rating and more, which is a separate SKU and can be restricted on its own, so the button could pass while every search failed. It now probes the resolved key with the search's own mask. Saving API keys wrote no audit line. It now writes one settings.api_keys_update row naming which fields changed, never their values, comparing cleartext because the random IV makes every ciphertext look different. Closes #1939 |
||
|
jubnl
|
2bffee8fd4 | fix(auth): preserve remember-me when password change re-issues the session (#1927) | ||
|
jubnl
|
72221a2e45 | feat(auth): sliding session renewal past half the token lifetime (#1927) | ||
|
Maurice
|
3a0604b3f5 |
feat(config): put the operator-owned surfaces behind TREK_MANAGED (#1946)
* feat(config): put the operator-owned surfaces behind TREK_MANAGED The switch landed last release with nothing behind it. This is what it turns off, plus the four fixes that came out of walking every one of these surfaces. Nothing here changes anything for an install that does not set the flag. That is not a hope, it is what managed-inert.test.ts asserts: the mode stays off for any unparseable value, no other variable can turn it on, every settings key stays writable, and splitManagedKeys hands back the same object rather than a copy so no write path can quietly diverge on self-hosted installs only. WHAT THE MODE WITHHOLDS Fourteen routes carry @ManagedForbidden: key validation, static MCP tokens, JWT rotation, the OIDC admin pair, restore and auto-backup, plugin sideload and reload, the Ollama model list and pull, and the SMTP test send. Each names the reason it is not the instance admin's, and the reason is mandatory. Eighteen settings keys are filtered instead of refused. SMTP, the OIDC toggles and the WebAuthn pair share one handler with keys the admin does own, and the llm_* family is reachable through per-user settings, the instance defaults and the addon config alike, so a route-level refusal would be both too coarse and incomplete. A locked key is skipped and reported as managed_keys; the rest of the save goes through. Refusing the request would take the registration toggles down with the SMTP block. GET /api/auth/me/settings stops returning the three API keys in the clear. It is the only endpoint that decrypts a stored credential back to the caller, which is fine when the admin pasted it in and wrong when the operator supplied it. Two multipart routes check in the handler rather than the guard: a guard throws before the parser drains the body and the client gets an ECONNRESET instead of the 403 (PROFILE-015). They still carry the marker so the boot gate sees them. WHAT KEEPS IT FROM DRIFTING validate-managed-routes.ts refuses to boot on a marking that is not on the reviewed list, a list entry whose route lost its marking, or a marking with no reason. The third case has no equivalent in the guard gate next door and is the one that matters most: a blank reason destroys the only record of why a control was taken away, and nothing recovers that later. It found two wrong entries in my own list while I was writing this. managed-keys.test.ts asserts that every key in ADMIN_SETTINGS_KEYS and DEFAULTABLE_USER_SETTING_KEYS belongs to exactly one of two sets. A key added to either list belongs to neither until somebody decides, and that is the failure this prevents: the lists grow, the mode does not. PLACES_API_BASE AND PLACES_API_KEY The nine Places endpoints spell out googleapis.com, so an install that wants those calls to leave through something of its own has no way to say so. One variable, substituted where every call already funnels through. A second one supplies the key, and when it is set the users table is not consulted at all — a per-user key would route around whatever the endpoint counts, and those columns are readable by any admin of the instance. FOUR FIXES THAT ARE NOT ABOUT THE MODE An admin setting somebody's password now bumps password_version and clears their MCP tokens. An admin sets a password for one reason, and until now verifyJwtAndLoadUser kept accepting every cookie the intruder already had: the one action taken to lock them out was the one action that did not. The four OIDC provider calls go through the SSRF guard. Three are worse than they look — the token, userinfo and JWKS URLs come out of the discovery response, so they are chosen by the other end, not by an admin. safeFetchLlm is renamed safeFetchAdminConfigured and keeps its old name as an alias, because it is the variant that allows loopback and LAN: a self-hoster with Authentik on localhost:9000 must keep working. The token exchange follows no redirect at all, since one would hand client_secret to a second host. DELETE /api/admin/users/:id/mfa clears another account's TOTP. The passkey half has existed since passkeys landed; the TOTP half never did, which left "somebody on the trip lost their phone" with no answer short of an operator in the database. Never for the caller themselves. TREK_PLUGIN_PERMISSIONS=off refuses to boot when the install is centrally administered. A warning is right for a machine its owner chose to trust; where the person who set the flag and the people whose data is in the database are not the same person, fail closed. TEXTS Four strings lose the phrase "self-hosted" because they describe somebody else's software — an Ollama endpoint, an AirTrail instance, a service a plugin talks to — and read better without it in both modes. The one string that describes TREK itself keeps it, and the mode picks a second key instead, so self-hosters lose nothing. A grep gate in shared/ keeps the canonical locale honest, because the parity check compares the file set and the top-level keys and never looks at a value: re-wording a string is invisible to CI today, and so is putting it back. * feat(maps): supply the map credential with the instance when centrally administered MAPBOX_ACCESS_TOKEN joins PLACES_API_KEY as an operator-supplied value. SettingsService injects it when the settings are read, and picks GL as the provider if the reader is still on Leaflet, since a token is always present. The key moves from the customer set to the locked set. It is the one locked key whose value is public by design — a pk.* token has to reach the browser to be useful — so this is not about secrecy: a per-user save landing on top of the instance token would only break the map for that user. Both settings fields for it go away with it. * fix(auth): count the operator key when reporting whether Google features work has_maps_key only looked at the users table, but getMapsKey prefers the PLACES_API_KEY env over anything stored there. On an install that supplies the key that way and has an empty column, every lookup worked while the client was told there was no key and hid the search, the autocomplete and the place photos. Found by walking the running instance rather than the code: the map came up, the search box did not. * feat(admin): hide the settings a centrally administered install cannot act on A control that answers 403 is worse than no control: the AI section rendered the refusal as "Request failed with status code 403", which tells an admin they broke something rather than that the decision is not theirs. So the surfaces go rather than being disabled in place: - AI parsing and AirTrail leave the addon list, and the AI addon route now refuses outright. The endpoint, the model and what a document costs come with the instance, so there is no instance decision left — not even whether it runs. - Immich and Synology Photos leave it too. Both connect to a server the reader runs at home, and there is no route from here to one. - The SMTP block, the admin webhook and ntfy targets, and the passkey RP ID and origins follow the same rule: they configure something the operator owns, and a wrong RP ID on a shared parent domain reaches past this instance entirely. - The About tab goes. It is about the project — what TREK is, where to file a bug, where to support it — and is aimed at the person running the software. The version and the link to the source move to the sidebar footer, on both the desktop page and the mobile twin, because AGPL 13 wants that offer wherever people reach the software over a network. - The update check stops asking GitHub and reports nothing available. Separately, the version footers lose the "self-hosted" suffix everywhere, not just here. It read as a label on the software rather than on the install, and the version alone says what it needs to. * feat(config): refuse to boot centrally administered without an at-rest key Measured on a running instance rather than assumed: with ENCRYPTION_KEY unset, the at-rest key falls back to a file in the data volume, and backupService bundles that file into every archive it builds. The archive is downloadable by any instance admin, so the encryption ships with its own key in the same zip. The skip in backupService already handles this — it only omits the file when the key came from the env — but nothing made sure the env actually carried one. Provisioning sets it; forgetting it was silent. validateEnvAtBoot now carries a second kind of check next to the schema: cross- field rules that only apply with the flag set, for combinations where every value is individually valid and the result is wrong. Empty without the flag, so an install that never asked to be centrally administered cannot meet it. * feat(client): add an attachment point for install-specific screens Two empty exports on the client, an empty module on the server, and the four places that read them. Nothing here adds a route, a nav entry or an endpoint — the arrays are empty and the module contributes nothing, which managed-ext.test pins so it stays that way. Read unconditionally, never behind a flag. On the server a conditional import would tie the module graph to an environment variable, and the graph is built once per app while the flag is read live; on the client an empty array costs nothing to spread, so every build walks the same path instead of one build walking a path nobody tests. Anything attached here follows the same rules as the rest of src/nest and src/components: a real module with a controller and a provider, bodies through the Zod pipe, routes under the global guards, tests. * test(shared): move the wording gate out of src so shared typechecks again CI installs shared with its own dependencies only (npm ci --workspace shared), so there is no @types/node and the `node:fs` import inside src/ failed to compile. The tests passed the whole time — vitest supplies the module — which is why this only showed up in the typecheck step. The walk moves to scripts/, next to i18n-parity.mjs, and the spec imports it the same way that one does. It also runs on its own now (`node scripts/i18n-selfhost-wording.mjs`), which is what you want when CI names a line you have to go find. * test(server): repair the call sites typecheck:tests caught tests/ sits outside the build tsconfig, so none of this showed up in a green test run — which is exactly the gap `npm run typecheck:tests` exists to close. Three kinds, all mine: - Seven `import type` paths for RuntimeEnvService carried one `../` too many. Type-only imports are erased, so vitest never resolved them and nothing failed. - VersionCheckJob and PluginsController grew a constructor parameter and eleven hand-wired `new` calls kept passing the old count. The argument arrived as undefined and no assertion happened to read it. - ConditionContext grew `managed`, and the shared context object in the conditions suite did not. The condition also had no test of its own, so it has one now: both directions, since the registry uses `is: false` to keep a notice away and a notice that only belongs on a centrally administered install would use `is: true`. |
||
|
Maurice
|
bfb3bcd63c |
feat(config): mark an install as centrally administered
Some installs are run by somebody other than the person who administers them. The operator owns the credentials, the configuration and the upgrade schedule, and the admin of the instance is a user of the result rather than the one who set it up. Nothing in the app knows that today, so it keeps offering settings whose values are not the admin's to change, and the honest answer to half of them would be a 403 the UI never anticipated. TREK_MANAGED says it once. It is a single boolean and the surfaces that care read it themselves, so adding one never comes back to the config layer. Runtime-toggled, in the same class as DEMO_MODE, rather than a registerAs token: roughly sixty tests mutate env mid-lifetime, and a value frozen per built app breaks all of them. `=== true` on the way in, so an unset, empty or misspelled value means off — a managed install sets it deliberately, and a typo must not quietly take settings away from an admin who never asked for it. The guard keys on a decorator, not on a path. MfaPolicyGuard documents where the two literal path lists it replaced ended up: they stopped being maintained, and every endpoint added afterwards silently fell on the wrong side. A marker on the handler cannot drift away from the handler. It carries a mandatory reason, which is for the next reader of the source rather than for the client, so the marks stay auditable. It registers third, after the auth and MFA guards, so a stranger still gets the 401 those two would have given them instead of a 403 that would confirm the route exists. And it checks the mode before it touches metadata, because it runs on every request of every install and almost all of them are self-hosted. One 403 body, shared by REST and MCP. DEMO_MODE grew two texts for the same condition and neither can be changed now without hunting for the other. The flag reaches the client through app-config so the UI can stop offering what the server would refuse anyway, but it is deliberately not persisted the way demo_mode is: two installs can share a browser profile, and a stale "this one is managed" would take settings away on an install that never set it. No route carries the marker yet. The per-domain marks follow. Unset, which is every self-hosted install, nothing about this changes anything. |
||
|
jubnl
|
954a3ca0a7 | refactor(server): serve /mcp from a Nest controller; delete auth, audit, addons and oauth bridges | ||
|
jubnl
|
8b7b91d814 | refactor(server): let user-cleanup inject BudgetService and delete budget.bridge | ||
|
jubnl
|
0718bd1b0c | refactor(server): shrink the seam bridges to their live exports | ||
|
bear
|
1edb531549 |
fix(auth): send Referer on the Google Maps key validation request (#1862)
The admin panel's test button reported a key with an HTTP referrer restriction as invalid, because the validation call was the one Google request that did not send a Referer. It sends the same header googleFetch already sets for every real request. |
||
|
Maurice
|
4dce8b9a7f |
Correct the ephemeral-token store's own justification
The docblock said the ws handshake consumes tokens from outside the container. That was true when it was written and stopped being true one PR later, when the transport became a gateway that injects this service. The store still has to be module state, for a reason the comment did not give: four bridges build EphemeralTokenService by hand, and per-instance state would give each of them an empty Map, so a download token minted through a bridge would never validate. An audit lens flagged the stale reasoning; the requirement survives it. |
||
|
jubnl
|
0ec611eb98 |
refactor(server)!: read the reminder gates per tick and drop the app-settings restart
The trip/todo reminder enable gates (notify_trip_reminder, notify_todo_due) are now read inside each 9 AM tick instead of once at boot, so toggling them takes effect at the next run without a restart. Saving notification settings no longer re-arms anything: the auth-side startTripReminders() call and the shouldRestartScheduler result field are gone (no controller consumed it, so the PUT /api/auth/app-settings response shape is unchanged). The boot 'enabled/disabled' log lines are informational banners now. |
||
|
Maurice
|
49c3c57c31 |
Give the ephemeral tokens a provider, and clear the next wave of type errors
ephemeral-tokens.ts was free functions, and AuthService owned the cleanup timer only because it happened to be the domain that mints tokens. Issuing them and sweeping them are different jobs, and FilesService and TokenService mint or consume them too. EphemeralTokenService owns the lifecycle and hands the three operations out. The store and the interval stay module state next door, and that is load-bearing rather than a leftover: src/websocket.ts consumes ws tokens on the upgrade handshake, outside the container, before any injector is in reach. Per-instance state would hand it an empty Map and close every connection with 4001, which no test would catch because the ws suites drive the transport directly. WS-027 through WS-029 pin the session gate that runs through exactly that path. The MAX_STORE_SIZE back-pressure is the second reason: createEphemeralToken returns null at 10,000 live tokens and TokenService turns that into a 503. A second store would raise the real ceiling with nobody noticing. Its own leaf module rather than a provider in AuthModule: TokensModule mints download and ws tokens as well, AuthModule already imports TokensModule, and TokensModule is documented as a deliberate leaf that three graphs reach for. Same shape as MailerModule, which broke AuthModule <-> NotificationsModule. Twelve hand-wired construction sites in tests needed the new collaborator. The typecheck named all twelve; before this branch they would have been undefined at runtime. Sixteen more test files are cleared here too, each at its root cause. Four of them were passing for the wrong reason: files.controller.test.ts drove two handlers with shifted arguments, a string tripId where the trip row belongs and undefined where tripId and id belong. It stayed green because FilesService is fully stubbed and the stubs ignore what they are given, so DELETE /:id/permanent and DELETE /trash/empty never covered that the trip row reaches the permission check. conflictUpdate.test.ts was missing real collaborators on PlacesService and PackingService. None of the exercised paths reach them, so it passed by luck. assignments.service.test.ts builds its service by hand, and the one consumer of the collaborator it was missing swallows every throw in a bare catch. A broken journey reconciliation would still have shown 28 green tests. scheduler.test.ts mocks two modules with paths that resolve one directory above the server root, so they mock nothing at all. Harmless only because src/scheduler.ts imports neither. |
||
|
Maurice
|
be8081c3b4 |
Enforce the MFA policy as a guard, and stop 403ing public routes
The last live non-shell middleware verified the JWT and read the users row a second time on every /api request, then threw the loaded user away - two jwt.verify calls and two SELECTs per authenticated request, because it ran before Nest and could not see what the auth guard had already resolved. The guard reads req.user. It also corrects an asymmetry that had grown quietly. The middleware decided on two literal path lists, and every public /api endpoint added after those lists were written - /api/config, /api/help/*, the public journey and share routes, /api/health/features - answered a logged-in user without MFA with a 403 while answering the same request from a stranger perfectly well. Nothing intended that; the lists simply stopped being maintained. Keying on @Public() means the exemption cannot drift from the route again. This is a deliberate behaviour change and the only one here. @MfaExempt carries the reason for the other kind of exemption: the routes that let somebody complete setup. Without them the policy locks out every new account, which is why the middleware had its second list. One ordering trap on the way: global guards run before route guards, so the auth guard standing down on a controller with its own @UseGuards chain left req.user unset when the MFA guard ran behind it, and it waved everything through. AUTH-020 caught it. It resolves the user now without refusing - the declared chain still owns the 401. |
||
|
Maurice
|
ffb73fac20 |
Turn protection from opt-in into default-deny
Nothing in src/nest/** was an APP_GUARD. Every protected route said so itself, 62 times, which means a forgotten @UseGuards was a silent bypass rather than an error. GlobalAuthGuard inverts that: a route is authenticated unless it carries @Public() or @OptionalAuth(), or declares its own guard chain. Reading the declared chain is what makes this behaviour-neutral. Nest runs global guards before route guards, so without that check the global guard would 401 an anonymous request to an addon-gated controller before its AddonGuard could answer 404 - inverting an order four e2e suites pin, and leaking which addons are installed. A controller that declared a chain keeps deciding for itself. It is a new class rather than JwtAuthGuard gaining a Reflector: that guard is constructed positionally in its own test with a context stub that has no getHandler, and the two want opposite behaviour anyway. @Public() takes a required reason, and the reasons are the diff worth reading: the invite token IS the credential, help assets are loaded by <img> which cannot send one, logout has to work with an expired session or nobody can sign out. The real deliverable is the inventory. validateRouteGuards runs in buildApp next to the body-contract gate and refuses to start when a route answers anonymously without being on a reviewed list of 25 - or when a listed one no longer does. An allow list nobody prunes stops being a list. Default-deny found two controllers on its own: SharedController and OidcController both sit in a file next to a guarded controller, so a file-level search for unguarded ones missed them. 37 tests went red and named them. That is the mechanism working before the change shipped, not after. |
||
|
Maurice
|
f2368f86cf |
Add an admin switch for place enrichment
Reads fail-open, unlike the three older places_* switches. Those read === 'true', which is why migration 185 had to backfill a row on every existing install; there is nothing here that warrants repeating that. The three hand-rolled toggles next to it move to ToggleSwitch while I am in there - they were bare buttons with an inline transform and no aria-pressed or label, so a screen reader could not tell their state. |
||
|
Maurice
|
93bbc65520 |
Return the registration invites to the domain that consumes them
invite_tokens gates signup. Its consumer is the registration path in AuthService, not anything in admin/, which held the four management methods only because the routes sit under /api/admin. Worth spelling out because the names invite it: invite_tokens and trip_invite_tokens are different tables for different jobs, and this one has nothing to do with adding a member to a trip. An earlier reading of the plan had these going to trip-invite/, which would have been the wrong home. The routes keep their paths and their admin guard; AdminController injects the service instead of carrying another domain's SQL. Also recorded, in the file, why the app-settings pair did NOT move to settings/ with the other app_settings readers: updateAppSettings runs the lockout guard that stops an admin disabling every login method, and that guard is auth's own resolveAuthToggles. Moving it would put an AuthModule import inside a leaf module half the container pulls in. |
||
|
Maurice
|
45611e2514 |
Separate the profile a user administers from the identity they log in with
The second cut into AuthService. Display settings, avatar, the third-party API keys a user pastes in and the member directory are not identity: no password is checked, no session is issued, no MFA secret is touched. UserProfileService takes those eight methods verbatim, with DatabaseService as its only injected dependency. AuthService keeps register, login, JWT, MFA, password reset and account deletion, and is now roughly 900 lines lighter than it started this branch. Not exported from AuthModule: only its own controller needs it, and an export nobody imports is noise. The controller test harness builds AuthController positionally, so the two new parameters landed in optional slots and the stubs for the moved methods moved with them. |
||
|
Maurice
|
03fbced20b |
Move tokens out of AuthService into a domain of their own
AuthService carried identity, profile, settings and tokens at once. Tokens are the cleanest of those to cut loose: they touch one table plus the ephemeral-token store, and nothing in them needs to know how a password is hashed or how a session is established. TokenService takes the six user-facing methods plus verifyMcpToken, and the two admin-side ones that only lived on AdminService because the admin route did. The pairs are genuinely different queries rather than duplicates, so both survive under names that say which is which: the user-facing ones scope every statement by user_id and treat the session sweep as best-effort, the admin ones do neither. Left behind on purpose: verifyJwtToken, which is login identity and belongs next to the cookie logic, and isDemoUser, which is a demo gate that happens to read the users table. TokensModule is a leaf with nothing but DatabaseService, because auth, admin and the MCP transport all import it and anything added here lands in three graphs at once. Both bridges and both controller test harnesses construct these classes positionally and had to be pulled along; the new argument goes in an optional slot so untouched call sites stay untouched. |
||
|
Maurice
|
e8d4278899 |
Give travel stats to the domain whose data it reads
getTravelStats sat on AuthService for historical reasons only. It reads places, trips, visited_countries, place_regions and reservation_endpoints, and subtracts the countries hidden in Atlas, so every one of its sources is an Atlas concern. Keeping it there had a real cost: AuthModule had to import AtlasModule, and that edge is the reason atlas.mcp.ts reaches AuthService through auth.bridge instead of injecting it. The direction is now free to reverse. GET /api/auth/travel-stats keeps its path. The client calls it there, so the route moves to a small controller in atlas/ that holds the /api/auth prefix rather than changing the URL. PROFILE-014 runs against the full app and proves the endpoint is untouched. KNOWN_COUNTRIES moves along, since getTravelStats was its only reader. Both bridges construct AuthService by hand and needed the argument dropped. |
||
|
Maurice
|
3efedd5e48 |
refactor(server): decorate the last three MCP prompts
The prompts registrar is gone. Each prompt moves to the domain that owns its data instead of a shared file that had to import three bridges to serve them. packing-list goes to PackingMcp and budget-overview to BudgetMcp, both reusing the `when` addon gate their tools already carry — so a disabled addon keeps the prompt out of the listing, exactly as the `if (isAddonEnabled(...))` around the registration call did. Both keep taking the trip title through trips.bridge: TripsModule imports PackingModule and BudgetModule, so injecting TripsService would close a cycle. Same documented in-container exception as atlas.mcp.ts, and the registrar reached for it the same way. token_auth_notice goes to a new AuthMcp — it is a statement about how the session authenticated, not about trip data, and it injects nothing because the message is static. Its `if (isStaticToken)` guard becomes `when: (ctx) => ctx.isStaticToken`; the registry hands `when` the session context, so an OAuth 2.1 session never sees the prompt listed at all, which is what the registrar achieved by skipping registration. src/mcp/tools/ is down to _shared.ts and journey.ts. journey stays until services/journeyService folds, which waits on memories. |
||
|
Maurice
|
ac96644508 |
refactor(server): fold the notifications domain out of services/
Four files taken apart rather than moved, because they only worked as a knot. The mailer becomes its own leaf module. AuthService sends the password-reset mail while NotificationsModule imports AuthModule for the demo gate in NotificationsMcp - a mailer inside the notifications domain would have closed a hard cycle that only a forwardRef could open again. Webhook and ntfy stay with notifications; auth has no business importing them. The channel registry stays a module singleton and does NOT become a provider. PluginRuntimeService pushes its channel getter in at onModuleInit, and notifications.bridge.ts builds its own NotificationsService for the scheduler crons and the memories senders. A provider would hand the bridge a second, empty registry - every plugin channel would then go quiet with no error anywhere. CHOVR-015 drives that seam end to end: a channel published through the runtime, delivered through the bridge. Nothing pinned it before; the existing cases all set the source themselves. The built-ins no longer self-register by side-effect import. They used to exist only because notificationPreferencesService happened to `import './notifications/builtins'`; removing that line would have silenced email, webhook and ntfy without a failure. They are built from the injected transports and registered from NotificationsService's constructor, so both instances have them. No cached nodemailer transport: MailerService.createTransport re-reads the SMTP config and skipTlsVerify per send, exactly as the three inline call sites did, so an admin changing them does not have to restart. Preferences become a provider over DatabaseService. Eight places built NotificationsService by hand, so a tests/helpers/notifications.ts factory keeps the next constructor parameter a one-file diff instead of an eight-file one. |
||
|
Maurice
|
03493a2edd |
refactor(server): fold webauthn, account erasure, oauth and the wiki out of services/ (#1840)
* refactor(server): move the webauthn config and the account erasure into nest/auth webauthnConfig resolved the RP ID through its own getSetting() on the db singleton; it is a WebauthnConfigService over DatabaseService now, injected into AuthService (passkey_configured) and PasskeyService (four ceremonies). readEnv() is still read per call, so an operator flipping the RP ID at runtime takes effect on the next ceremony instead of after a restart. userCleanupService follows as UserCleanupService. It keeps taking removeUserFromBudgetItems from budget.bridge rather than injecting BudgetService: BudgetModule imports AuthModule for the demo guard in BudgetMcp, so AuthModule cannot import BudgetModule back, and breaking that edge the other way drags BudgetController, BudgetMcp and its trips.bridge chain into every graph that imports AuthModule. The legacy module had no tests of its own - the erasure only ever ran through AdminService.deleteUser and TripsService.deleteGuest - so USER-CLEANUP-001..009 pin both halves, including the orphan data dir that no permissions row can describe and the all-or-nothing deletion transaction. * refactor(server): fold the oauth 2.1 server out of services/ 694 lines move into OauthService over DatabaseService, AddonsService and AuditService. The thin wrapper that only forwarded is gone, and so is the second path to the MCP addon flag - validateAuthorizeRequest read it through addons.bridge while the wrapper read it through the injected service, which would have let the two drift. The pending authorization codes stay a module singleton in oauth.pending-codes.ts instead of becoming instance state. The SDK still serves /oauth/authorize, /oauth/register and /oauth/consent from its own router, mounted on Express before app.init(), so it reaches the domain through the new oauth.bridge.ts - two maps would break the authorization-code flow silently, since the SDK only ever answers "Authorization grant is invalid.". The import-time sweep keeps its .unref() and is a named function now so a test can drive it. scopes and sessionManager are imported deep, never through the src/mcp barrel: the barrel re-exports revokeUserSessionsForClient and pulls the domain bridges back in, which is the nest -> mcp -> nest cycle. Same comment as admin.service and auth.service carry. The 70 cases of the legacy suite moved over unchanged, bound to a hand-built service. Fifteen more cover what free functions never let anyone reach: the code store at capacity, an expired code, an expired access token, refresh against a public and an unknown client, the RFC 8707 resource checks, and that a code written through the container is read back through the bridge. * refactor(server): move the wiki reader into the help module wikiService had exactly one consumer, HelpController, and touches no database - it stays a plain module rather than becoming a provider, the way auth/ephemeral-tokens.ts does, so the text and asset caches keep being one per process without any ceremony around it. The WIKI_DIR anchor needed one more level: src/services sits three below the repo root, src/nest/help sits four. The existing path-resolution case caught it, which is what it is there for - a wrong anchor makes help degrade to the GitHub fallback without anybody noticing. Also brings the nest README back in line: services/ is down to five top-level files, and the passkey and budget entries no longer point at paths that were deleted since they were written. |
||
|
Maurice
|
5eab8128bd |
refactor(server): fold the ephemeral-token store and the in-app action registry (#1836)
Two small leftovers, moved to the domains that own them. ephemeralTokens holds the short-lived tokens the WebSocket handshake and the file-download links use. It already had startTokenCleanup/stopTokenCleanup with an unref-ed handle — the right shape. What was missing is anyone calling stop: index.ts kicked the sweep off with a bare require() after listen() and nothing ever ended it. AuthService owns that lifecycle now, through onModuleInit/onModuleDestroy, which is where it belongs — it is the service that issues the tokens. The store itself stays module state. websocket.ts consumes tokens from outside the container, and a token store is only useful if the issuer and the consumer share one. Same reasoning as the weather cache in #1835. inAppNotificationActions is a 22-line handler registry with exactly one consumer; it moves next to it as notifications/in-app-actions.ts. services/ is down to 9 files, from 44. |
||
|
Maurice
|
9c46f31c57 |
refactor(server): fold airports and distance, drop the boot-time backfill cycle (#1834)
Two thirds of be-weather-airports-fold. weatherService (513 LOC, a process cache and a setInterval at import) is left for its own PR. airportService split along the line that was already there. The dataset lookup touches no database — it reads one JSON file and keeps it in memory — and both MCP registrars call it from outside the container, so findByIata/searchAirports are free functions in nest/airports/airports.data.ts. backfillFlightEndpoints needs the connection and became a method on AirportsService. That backfill was the interesting part. db/database.ts ran it at module eval through a bare require() back into services/, so the module graph closed a cycle that only held because `db` happened to be initialised by the time the require resolved. It runs in onApplicationBootstrap now: it repairs rows, nothing boots on it, and the container is fully built by then. The cycle had a visible cost. tests/setup.console-noise.ts carried a suppression for `Cannot find module '../services/airportService'`, because vitest's transform pipeline could not resolve that require at all — it fired in roughly sixty test files. That suppression is now narrowed to the only case left: an e2e TestingModule whose fixture DB has no reservations table. distanceService split the same way: haversineKm is arithmetic and moved to common/geo.ts; getFlightDistanceKm reads the database and became a private method on AuthService, its only caller. AirportsModule imports DatabaseModule explicitly. @Global only reaches modules that are in the graph, and the airports e2e builds its container from AirportsModule alone — exactly the trap the nest README now documents. The search ranking gets its first direct tests. It sat in services/, outside the coverage gate, and only ever ran incidentally through the route; moving it into src/nest made the gap measurable (16% branches). The ten cases pin the scoring ladder — exact IATA short-circuit, ICAO over prefix, IATA prefix over city prefix, city prefix over substring — and the missing-dataset fallback, which no test had reached. 90% branches now. |
||
|
Maurice
|
22d178d2f9 |
refactor(server): one addon guard, one demo-mode condition (#1829)
Two pieces of copy-paste in the guard layer. Three addon guards — collections, airtrail, journey — did the same thing and differed only in the addon id they read and the word leading the 404. They are one AddonGuard plus a @RequireAddon(addonId, label) decorator now, read through the Reflector. A handler-level decorator overrides the controller's, and a route without the decorator is not gated at all. The demo-mode upload block was copy-pasted into six handlers, each carrying its own copy of the message. All six call isDemoWriteBlocked now, against the same DEMO_WRITE_ERROR body. That one is deliberately NOT a guard, though all six sites are handlers and a guard is the obvious shape — and this PR did build one first. Guards run before the multipart parser, so throwing from one leaves the request body unread and Node resets the connection: the client gets ECONNRESET instead of the 403. PROFILE-015 caught it. The check has to sit inside the handler, after the interceptor has drained the upload, for the response to arrive at all. The function carries that reasoning so the next person does not rediscover it. Two smaller findings on the way there. The DemoWriteGuard attempt broke eight e2e suites that never touch demo mode: AuthModule listed it as a provider, and RuntimeEnvService was not resolvable inside a TestingModule built around one domain module. AppConfigModule is @Global, but @Global only applies to modules that are in the graph. The five modules whose controllers now inject RuntimeEnvService import it explicitly, the same way PermissionsModule is handled. And both new guards get a test asserting the decorators are actually applied. Once a check moves out of a handler, that handler's own tests stop noticing if the decorator is dropped — the assertions on __guards__ and the @RequireAddon metadata are what keeps the four upload endpoints and the four addon route groups gated, and gated on the right addon. |
||
|
Maurice
|
9814480a96 |
refactor(server): move the JWT verify into nest/auth, delete four dead middlewares (#1828)
middleware/auth.ts exported seven things. Two of them — extractToken and verifyJwtAndLoadUser — are the canonical session check every auth surface goes through. The other five (authenticate, requireCookieAuth, optionalAuth, adminOnly, demoUploadBlock) are Express middlewares with no production caller left; the guards in nest/auth replaced them and only comments still mention them. The two live functions move to nest/auth/jwt-verify.ts and the file goes, along with middleware/validate.ts (nothing but its own test imported it) and middleware/idempotency.ts (its only caller was `authenticate`, and IdempotencyInterceptor has replaced it). AuthRequest and OptionalAuthRequest go with them — src/typings/express.d.ts is what types req.user now. Eight importers repoint: the three guards, auth/files/plugins-proxy services, platform.routes and the global MFA policy. Two deliberate departures from the plan for this package. JWT_SECRET does not move to app-config. It is not an env value: it is generated into data/.jwt_secret, and the admin panel rotates it at runtime through updateJwtSecret. `export let` is precisely what makes a rotation take effect in-process — a registerAs token would freeze the boot value and silently break rotation. The verify also keeps reading the db singleton rather than DatabaseService. That change is not a one-line swap: it gives the three guards a constructor dependency, which means registering them as providers everywhere they are used. Measured: 38 directories use those guards and 21 of them do not import AuthModule, on top of 34 e2e tests that each build a container around a single domain module. That is its own PR, not a rider on one that deletes dead files. tests/unit/middleware/auth.test.ts covered real behaviour, so it is not simply deleted. Its extractToken cases and the five verify cases it drove through `authenticate` come back as tests/unit/nest/auth/jwt-verify.test.ts, now hitting verifyJwtAndLoadUser directly. Four cases are new: the password_version gate — the whole reason this function exists — had no direct coverage at all. Its three adminOnly cases are dropped because AdminGuard, their successor, is covered by three e2e suites. |
||
|
Maurice
|
939452f9d8 |
refactor(server): consolidate trip access and fold the query helpers into the container (#1824)
* refactor(server): route every trip-access check through DatabaseService
Twelve Nest services carry the same verifyTripAccess/canEdit pair. Six already
asked DatabaseService.canAccessTrip; the other six went through
services/tripAccess, a nine-line re-export of the identical function. Same
query, same result, two ways to reach it.
All twelve now take the injected route, so services/tripAccess is gone.
The bigger duplication was in the types. Thirteen files declared their own
`Trip` for the same return value, in three different shapes:
NonNullable<ReturnType<typeof verifyTripAccess>> 6x
{ user_id: number } 4x
TripAccess 3x
The narrow one is why assignments, days, places and accommodations all wrote
`as Trip | null | undefined` on the call — the cast existed to shrink
TripAccess down to a local type that was never meant to differ from it. All
thirteen now say TripAccess, and the four casts are gone with no other change.
middleware/tripAccess.ts goes too: it was already dead in production — nothing
under src/ imported requireTripAccess or requireTripOwner, only comments in the
services that had absorbed them. Its test went with it.
* refactor(server): fold the query helpers and trip joins into the container
Two more legacy files whose only reason for staying outside the container was
that they predated it.
services/queryHelpers.ts held five exports doing two different jobs. Three are
the batch loaders that keep the list endpoints off N+1 queries — they hit the
database and are now QueryHelpersService over the injected connection, imported
by assignments, days, places and share. The other two, formatAssignmentWithPlace
and ratingAggregate, touch no database at all; they sat next to the loaders only
because the loaders were free functions too. Those stay free functions, in
common/rowShape.ts.
The README note on the assignments service claimed the loaders had to stay in
services/ because the day, share and place services were unmigrated. All three
have since moved, so the reason is gone.
services/tripMembership.ts becomes TripMembershipService in its own module. It
could have folded into auth or trip-invite, but both call it — as do oidc and
the plugin RPC host — and putting it in either would have made AuthModule and
TripInviteModule import each other.
Neither module is @Global, matching PermissionsModule: e2e TestingModules
resolve them transitively through each consumer's explicit import.
Test seams changed shape accordingly. Two path mocks of services/tripMembership
became injected stubs; the trip-invite e2e overrides the provider in the
container instead. The oidc test keeps the real implementation, which is what
OIDC-SVC-045 was always asserting.
ratingAggregate moved out of the ungated services/ tree into the coverage-gated
one, so it gets the direct tests it never had — as does the tags/participants
pass-through, which two long-dead fixture variables in the moved test file had
been waiting for.
Also drops four stale comments that outlived their subject: the requireTripAccess
middleware they pointed at was deleted in the previous commit.
|
||
|
Maurice
|
6dda05f672 |
refactor(server): move the stateless helpers into nest/common (#1823)
Eight files in services/ hold no state and touch no database: avatarUrl, conflictResult, demo, passwordPolicy, timezoneService, cookie, and the two crypto helpers. They are imported from 65 places, 62 of them already inside src/nest, so having them live in the legacy layer was pure inertia. They stay plain functions rather than becoming providers. Three importers sit outside the container and would have no way to reach an injectable one: db/migrations.ts runs before the app is built, and middleware/auth.ts and middleware/mfaPolicy.ts run beside it. Wrapping these in DI would buy ceremony and nothing else. mfaCrypto and apiKeyCrypto move together into common/crypto — they read the same ENCRYPTION_KEY, and the directory says so. services/ drops from 44 files to 36, 9,870 lines to 8,493. One trap worth recording: src/app-config and src/nest/app-config both exist. cookie.ts imported '../app-config' from services/, which resolved to the former; after the move the same string silently pointed at the latter, which has no readEnv. It is now explicit about which one it wants. Nothing else changes — no signatures, no behaviour, no tests rewritten beyond their import paths. |
||
|
Maurice
|
e606ebd9b9 |
chore(server): pin the WebSocket envelope, dedupe the rate limiter (#1803)
* chore(nest): register LlmParseModule in AppModule
The module was only in the container because BookingImportModule and
PluginsModule happen to import it, so /api/admin/llm/local depended on those two
keeping that import. Register it where every other module is registered.
* refactor(nest): give the rate limiter one home and one instance
RateLimitService keeps its counters in a Map on the instance, but it was listed
in the providers of auth, oauth, transit and trip-invite, so each of them got its
own. Nothing is broken today — the bucket names in use (login/mfa/forgot/reset,
oauth_*, transit_*, trip_invite) don't overlap — but two modules picking the same
name would have quietly allowed twice the configured attempts.
Move it to nest/common, where it belongs now that four domains use it, and hand
it out through a small module they import. Not @Global: the e2e suites build a
container around one domain module, so a global AppModule never pulled in isn't
there at all.
transit.mcp.ts keeps its own instance on purpose — it keys by userId under
mcp_transit_* while the REST path keys by IP.
* test(ws): pin the wire envelope before touching the transport
Every frame on the socket is flat: {type, tripId, ...payload} out, {type, tripId}
in. Nest's WsAdapter dispatches on message.event and answers {event, data}, so
moving this onto a gateway without a custom serialiser would leave join silently
unhandled — no welcome.socketId to send back as X-Socket-Id, and every client
would then receive its own writes.
Nothing catches that today: the 100+ tests that touch realtime mock src/websocket
and never look at the bytes. WS-032 sends the exact shape the stock adapter would
and asserts the server ignores it.
* docs(nest): drop the parts of the README that no longer exist
strangler.ts, DEFAULT_NEST_PREFIXES/NEST_PREFIXES and tests/parity/ are all gone —
every request goes through Nest now — but the README still walks a new contributor
through adding a prefix and writing a parity test against the Express route.
Also spell out that the 80% gate is an average over src/nest/**, not a floor per
file, so a large untested module can sit behind the well-covered small ones.
|
||
|
Maurice
|
d9778ed7c0 |
fix(server): keep the passport card in step with the atlas
getTravelStats derives its countries independently of the atlas service, so without the same date filter the dashboard would have kept counting trips the map no longer treats as visited — the drift #1490 closed. Manually marked countries and the hidden-country tombstones are untouched. |
||
|
jubnl
|
c814653f4e | fix(server): tidy passkey quirks after the DI fold | ||
|
jubnl
|
5d4b2abd26 | feat(server): validate passkey request bodies | ||
|
jubnl
|
e65f851446 | chore: move passkey backend to proper DI | ||
|
jubnl
|
ec0709741a | chore: move oidc backend to proper DI | ||
|
jubnl
|
d892b61a59 |
fix(server): tidy auth quirks after the DI fold
Verified defects the relocation carried byte-for-byte, each pinned by a regression test (AUTH-DB-089..093): - getTravelStats kept dropping lat/lng of exactly 0 (equator / prime meridian) through its falsy check — now explicit != null. - registerUser's INSERT user -> invite used_count bump -> joinTripAsMember ran statement-by-statement; the whole signup now runs in db.transaction() so a mid-sequence throw rolls everything back. - verifyMfaLogin's backup-code burn and last_login/login_count bump now commit as one atomic pair. - deleteMcpToken's revokeUserSessions call is best-effort (try/catch), matching the changePassword/resetPassword revocations. - updateApiKeys' current! non-null assertions became current?. ?? null — a user row deleted mid-request degrades to a 0-row UPDATE, not a TypeError/500. Deliberately preserved (documented in migration-graph.md): the two divergent "Password authentication is disabled" strings and enableMfa's 15-min pending-secret TTL after a wrong code. |
||
|
jubnl
|
fb87202683 |
feat(server): validate auth request bodies
DTO ratchet for the auth domain (collections sibling-commit precedent):
- shared/src/auth/auth.schema.ts gains the six missing request schemas —
mapsKey (nullable: the client clears with an explicit null), apiKeys and
settings (per-key partials, nullable key values), appSettings (open
record — the server's ADMIN_SETTINGS_KEYS allow-list and its bespoke
lockout/self-MFA 400s stay service-owned), mfaDisable, resourceToken
(the 'Invalid purpose' 400 stays service-owned) — plus spec cases.
- New nest/auth/auth.dto.ts createZodDto wrappers; all 14 raw @Body()
params typed (9 AuthController + 5 AuthPublicController), validated by
the global ZodValidationPipe.
- All 14 auth entries removed from body-contract-allow-list.ts
(PasskeyController's 4 stay until passkeyService migrates).
- auth.e2e mirrors the production pipe and pins the { error: 'field: ...' }
envelope on a shapeless login body; auth.controller.ts drops its
duplicated avatarDir const in favour of the auth.helpers export.
|
||
|
jubnl
|
45564b0a8a |
chore: move auth backend to proper DI
Fold the 1497-line services/authService.ts into the DI-native nest/auth
domain (Wave-5 chain opener, unblocked by the atlas fold):
- Pure crypto half (backup-code hash/match/generate, stripUserForClient,
key masking, the import-time DUMMY_PASSWORD_HASH timing equaliser and
avatarDir mkdir — documented parity exceptions) moves verbatim to the
plain module nest/auth/auth.helpers.ts.
- DB half folds into AuthService over DatabaseService + injected
PermissionsService (ex permissions.bridge) + AtlasService (ex
atlas.bridge, deleted on schedule). mfaSetupPending and the per-email
reset throttle stay module-scoped so the bridge and container instances
share them; avatarDir and require('../../../package.json') re-anchor one
directory deeper — with the two injection swaps, the only non-verbatim
lines. Controller-facing method surface unchanged.
- New 8-export auth.bridge.ts for out-of-container consumers: mcp/index.ts
token verification, the journey/notifications/transports registrars'
isDemoUser, and the still-legacy adminService/oidcService/passkeyService.
- 15 domain *.mcp.ts demo guards now inject AuthService (their modules
import AuthModule); atlas.mcp.ts alone stays on the bridge because
AuthService injects AtlasService and the reverse module edge would close
an AuthModule<->AtlasModule cycle (places.mcp precedent). OidcService and
PasskeyEnabledGuard inject AuthService too.
- Tests move with case IDs preserved: authService.test.ts ->
auth.helpers.test.ts, authServiceDb.test.ts -> auth.service.test.ts
(+ AUTH-BR-001..007 bridge delegation, + AUTH-DB-050..088 pinning the
previously untested getAppConfig/login/MFA/reset/admin-settings branches
now under the src/nest coverage gate). auth.e2e converts DI-native (real
bcrypt, real audit rows); oidc.e2e swaps its dead path mock for an
instance spy; websocket/integration suites repoint to bridge/helpers.
Quirks preserved byte-for-byte (0-coord drop in getTravelStats,
un-transactioned registerUser/verifyMfaLogin multi-writes, unguarded
revokeUserSessions in deleteMcpToken, current! assertions in
updateApiKeys) — fixes land separately.
|