Two of the three files in be-places-helpers-fold. placePhotoCache is left for
its own PR — it holds three module-level caches and its sweep hangs off the
cron, so it needs both a provider with real state and a second entry in the
scheduler contract.
kmlImport.ts is 212 lines with no state, no database and no I/O — parsing,
sanitising and coordinate arithmetic. It is nest/places/kml-import.helpers.ts
now, unchanged, next to the places.helpers.ts that already consumes it.
placeImage.ts moves to nest/places/place-image.ts as free functions. Making
reclaimPlaceImage a provider would have split it from PLACE_IMAGES_DIR, which
cannot be injected: nest/common/place-image-upload.ts reads it at module scope
to configure multer's storage, before any container exists.
That constant is the one thing here that could have broken quietly:
path.resolve(__dirname, '../../uploads/places')
__dirname moves with the file. From src/services that resolved to
<server>/uploads/places; from src/nest/places the same string would have
resolved to <server>/src/uploads/places — a directory multer would have happily
created, leaving every uploaded place image in the wrong place and every
existing one unreachable. The path is now '../../../uploads/places', verified to
resolve to the identical absolute directory, and uploads-dirs.test.ts covers it.
Two of the three leftovers in be-misc-fold. The third (wikiService) needs a Zod
contract in @trek/shared first and is left for its own PR.
llmConfig.ts was pure movement: 70 lines of encryption-aware config shaping with
no state and no database, sitting in services/ while all four of its callers are
in src/nest. It is nest/llm-parse/llm-config.ts now, same functions, same file.
unsplashService.ts needed an actual provider. Its key resolution reads both the
env and the users table (env var, then the caller's stored key, then any
admin's), so the injected connection was unavoidable. Its own module because
trips and places both reach it, and because folding it into either would have
made the other import a trip/place service for an image search.
The four exports became methods on UnsplashService. The SSRF guard in
saveUnsplashCover is untouched — it still refuses any host but the Unsplash
image CDN and still goes through safeFetch — as is the unauthenticated-endpoint
fallback when no key resolves (#1449).
The env read stays live through RuntimeEnvService rather than a registerAs
token. readEnv() is what the function did before, and a boot-frozen token would
change when an operator's UNSPLASH_ACCESS_KEY takes effect.
Two test seams follow the code: the unsplash unit tests bind the four methods so
every existing assertion reads unchanged, and the trips controller's path mock of
the module becomes an injected stub.
services/ is down to 17 top-level files.
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.
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.
The scheduler runs outside the Nest container but needs four things inside it:
backupService, trekPhotoCache, placePhotoCache and airtrailSync. It reaches all
four with a require() or await import() at call time. This settles how that
should work before four separate folds each invent their own answer, and wires
one consumer as proof.
It takes the dependencies from the container rather than growing four more
bridges, for a reason that showed up while writing the previous PRs: adding one
constructor parameter to a service meant hand-editing five bridge files that
rebuild their services with `new`. app.get() has no such cost, and hands back
the container singleton instead of a second instance.
The scheduler declares what it needs structurally — a `backups` with a
createBackup — and never names BackupService. That is not fastidiousness: an
import of the class would pull in src/nest/backup → src/services/backupService →
src/scheduler, which is the exact cycle the lazy `await import()` in runBackup
existed to dodge. With the dependency handed in, the scheduler stops importing
backupService entirely and the cycle is gone; backupService still imports the
scheduler for VALID_INTERVALS, but now only in one direction.
index.ts fills the contract from the app buildApp() already initialised, right
before the first cron starts, the way bootstrap.ts hands the /mcp handler its
registry.
Two smaller things fall out. The Nest BackupService was swallowing
createBackup's prefix parameter, so it could only ever produce 'backup' names;
it forwards it now. And the scheduler can be started without a container at all
— its unit tests do exactly that — so runBackup says so in the log instead of
throwing, with a case covering it.
The other three consumers stay on their require() calls; they move with their
own folds.
* 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.
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.
* fix(map): drop the casing layer before its source on teardown
Tearing down the reservation overlay logged an error on every map unmount:
Source "trek-reservations" cannot be removed while layer
"trek-reservations-lines-transit-casing" is using it.
The casing layer arrived with the transit paths in 3.2.0 but was never added to
destroy(), which only removed the main line layer. The try/catch around it gave
no protection either — the engine reports this by firing an error event rather
than throwing — so the source was silently left behind on the style.
The layer id is a named constant now, so the place that creates it and the place
that removes it can no longer drift apart.
The lifecycle test grew the assertion that would have caught it: the fake map
now refuses to remove a source that a layer still references, the same way the
real one does. Verified against the unfixed code — two tests fail without the
change.
* fix(map): stop untrusted values reaching the marker HTML
Both map renderers build their marker HTML as a string and hand it to
L.divIcon / innerHTML. Two of the interpolations carry values a user controls,
and neither was escaped.
place.image_url is the worse of the two, because it is not admin-only: any trip
member with edit rights can set it. placeUpdateRequestSchema is a plain record
of unknown values, and the length guard in places.controller covers name,
description, address and notes but not image_url. The renderers do check the
value, but only with startsWith('/uploads/') — a payload like
/uploads/x" onerror="…" y="
satisfies that and then breaks out of the src="…" attribute.
category.color is the same shape one line up, reachable by an admin. It also
reaches SharedTripPage, which builds its own divIcon and answers without a
guard, so that payload lands in front of people who have no account here.
Colours are allow-listed rather than escaped. Escaping stops the attribute
breakout but leaves a working CSS value behind, and `url(https://…)` in a
background is a tracking pixel. Anything that is not #rgb/#rrggbb falls back.
image_url is a URL, not an enum, so that one is escaped; the prefix check stays.
Server side, the category contract now demands a hex colour, and the endpoints
validate through DTO classes instead of @Body('color') — a property read carries
no metatype, so the global pipe never saw it. That retires two entries from the
body-contract allow list. It replaces the hand-rolled 400 for a missing name
with the pipe's envelope, the same trade the places and trips migrations made.
The response schema deliberately still accepts any string: the write path was
open for a long time, so tightening it would make an instance fail on its own
stored rows. The client allow-list is what covers those.
Verified against the unpatched renderer — the three new cases fail without it.
* perf(client): serialize the map icons without react-dom/server
Six modules imported react-dom/server statically to turn a lucide icon into an
SVG string. That put Fizz — roughly 190 kB raw, 57 kB gzip — into a chunk three
lazy routes share, the Leaflet renderer among them, which is the default. For
output that is always a single <svg> with a flat list of shape children.
mapViewport chunk 189,532 -> 3,710 B
Tooltip chunk 194,333 -> 1,970 + 4,860 B
precache total 17,856 -> 17,675 KiB
utils/iconMarkup.ts serializes React elements directly. It is generic over
elements rather than over lucide's icon tables on purpose: iconNode is closed
over inside createLucideIcon and never exported, so calling the forwardRef
render function is what survives a lucide upgrade.
Byte-compatibility is checked, not asserted. iconMarkup.parity.test.ts diffs the
output against real Fizz for every exported icon in three prop shapes — the test
file is the one place still allowed to import react-dom/server, and it never
reaches a bundle. Two traps it covers: viewBox must not be kebab-cased into
view-box, and Fizz emits <path …></path> rather than a self-closing tag.
TripPDF loses its lazy renderer entirely — ensureRenderer, the module-level
handle and the two "not ready yet" guards that returned an empty string. There
is nothing left to load.
An eslint rule keeps the import from creeping back. Without it the regression is
invisible: the build stays green, the app keeps working, and the chunk quietly
grows by 190 kB again.
* 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.
Four tones cannot keep a twenty-stop trip's legs apart, which is most of
the point of colour-coding one. A contribution can now carry `#rrggbb`
alongside `tone`, per region or as a shorthand.
The split of control is: the plugin picks the hue, the host picks the
weight. A colour rides the same per-theme, per-region alphas as a tone,
and its OKLCH lightness is clamped into a per-theme band first, so no
contribution can hand a user a day card they can't read. The band
brackets the built-in tones rather than tightening around them, so an
ordinary colour passes through untouched and a hex equal to a tone
renders exactly like that tone. Hue and chroma are never touched - it
bounds a colour, it doesn't boost one, so a near-white grey still lands
as a faint tint.
Only `#rrggbb` survives normalization, and that is a security boundary
rather than tidying: the value ends up inside a `color-mix()` in an
inline `background`, a shorthand that takes a layer list, so a string
closing the paren early could append a `url(...)` and beacon everyone
viewing the trip.
A region gets exactly one paint instruction - what it names beats the
shorthand, a colour beats a tone at the same level - so the client never
has a tie to break. A region naming a colour the host can't use degrades
to `default` like a bogus tone already did.
Lets a plugin colour-code days in the Plan sidebar. trip-segments can
already announce "Kanazawa begins" in the day plan, but it can't make
day 12 look like it belongs to Kanazawa.
dayScheduleProvider can't cover this: it renders a fixed row, and it's
capped at 60 items per provider, so one item per day would tint the
first 60 days of a six-month trip and silently stop. This hook is
bounded by day count instead.
GET /api/day-tints/:tripId returns per-day tones for three regions of a
day card - the number badge (also the mobile day chip), the header row,
and the expanded activity list. `tone` is a shorthand filling every
region a contribution doesn't name; unnamed regions render as they do
today. Tones only, never hex, so the host can pick the alpha per theme
and per region.
One contribution per day, resolved whole: first entry wins within a
provider, first granted provider wins across plugins.
Newer OpenAI models (gpt-5.x) reject max_tokens with a 400 and demand
max_completion_tokens, so AI booking import failed and no reservations
were created. Send max_tokens first (understood by the classic chat API
and every local server), and only on a 400 naming max_completion_tokens
resend the whole request with that parameter instead — mirroring the
existing json_schema->json_object fallback beside it. Detection is by
API response, not a model-name allowlist.
Fixes#1760
createItem never accepted weight_grams or bag_id — not in its signature, not
in the INSERT — so cloneItem had nothing to pass on and every copy arrived
empty. For a group trip that meant re-entering every weight by hand for each
traveller, which is what people were doing.
The bag needs more care than the weight. Bags belong to a trip, but they can
be assigned to a person, so inheriting the original bag would drop your copy
into somebody else luggage and inflate their total. A copy keeps the bag only
when nobody owns it or when the copier is one of its members.
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.
Every country now carries the status of the strongest source it came from:
a country reached by both a past and a future trip is visited, one that only
appears in upcoming trips is planned. Marking a country by hand still wins
over everything, since that is a statement of fact rather than a date.
totalCountries drops to the visited ones and totalCountriesPlanned sits
beside it; continents and mostVisited follow the same rule. countries[]
stays a superset of what it returned before, so an older client keeps
painting the map exactly as it did.
The endpoint query has to select trip_id to know which trip a leg belongs
to, which makes its DISTINCT per trip — the coordinates are collapsed again
in JS so the point-in-polygon lookup still runs once per location.
The 851-line services/adminService folds into the wrapper AdminService over
DatabaseService plus injected Settings/Addons/Passkey/Packing/Auth/Permissions/
Notifications services; the auth, notifications and permissions bridge imports
all become injections, PERMISSION_ACTIONS stays a plain const, and the
mcp/sessionManager deep import keeps its anti-cycle comment. Ahead of the fold
the 11 packing-template functions move to PackingService, which already owned all
three template tables — resolving the admin-2 residual with no bridge. The pure
and module-scoped half moves to admin.helpers.ts, the version cache staying
module-scoped so admin.bridge's instance and the container singleton share one
GitHub fetch. A 1-export admin.bridge.ts serves scheduler.ts's cron, the only
out-of-container consumer. Recipe steps 3 and 4 are no-ops: no src/mcp/tools/
admin.ts has ever existed and the plugin host never imported the domain.
Four lines are non-verbatim, all path re-anchoring one directory deeper, both
resolved paths verified against the emitted dist layout.
admin.dto.ts clears all twenty AdminController allow-list entries, retiring the
three 'enabled must be a boolean' checks plus 'permissions object required' and
'Object body required' for the pipe envelope. The module e2e goes DI-native: the
3-method whole-module mock dies, 6 cases become 15 over real SQL.
Quirk fixes on top, each pinned: the three places toggles go fail-closed with an
append-only backfill migration; the template item routes honour :templateId;
updateUser rejects an empty username/email; createInvite 404s an unresolvable
trip_id; listOAuthSessions guards its scopes parse; GitHub fetches gain a timeout
and size cap and failures cache briefly; updateOidcSettings/updateAddon/updateUser
become transactional; admin.oauth_session.revoke is renamed to match its
siblings; two mutations gain their missing audit rows; getAuditLog drops the
_parse_error sentinel.
Tests move with IDs preserved (ADMIN-SVC-001..069 incl. the pre-existing 029/030
gap and duplicated 069, VNOTIF-001..007), plus ADMIN-BR-001 and ADMIN-SVC-070..076.
notifications.dto.ts wraps the @trek/shared request schemas via createZodDto
and types all five @Body() params; the five NotificationsController entries
leave the body-contract allow-list (ratchet gate enforced). Contract catch:
the client sends server/token: null on test-ntfy to mean 'use the saved
value', so testNtfyRequestSchema gains .nullable() there (spec pinned). The
inline response-enum and url-type checks die — malformed bodies now get the
pipe's standard envelope; valid bodies behave byte-identically. Two
integration cases pinning pre-ratchet tolerances (flat notify_* preferences
body, body-less test-smtp POST) update to the current wire contract.
The send() dispatcher (services/notificationService.ts) and the in-app store
(services/inAppNotifications.ts) fold together into the DI-native
NotificationsService over DatabaseService + RealtimeService; the prefs
matrix, transports, channel registry and inAppNotificationActions stay plain
infra modules. A 1-export notifications.bridge covers scheduler, legacy
adminService/memories and the six lazy fire-and-forget sends; AdminController
and the plugin RPC host (24th factory dep) inject. The 5-tool registrar +
notifications-in-app resource move to notifications.mcp.ts. Tests move with
case IDs preserved (NSVC-*, INOTIF-*, + NSVC-020 bridge pin); ~18 suites
repoint path mocks/warm-ups; the module e2e goes DI-native.
Also carries the verified-quirk fixes that would normally trail as a
separate fix(server) commit (file-overlap with the fold): toUtcIso on all
three created_at paths (INOTIF-013/014), respond claims the response CAS
before running the action handler with release-on-failure (INOTIF-011/012),
synology's fire-and-forget send gains .catch, the dispatch-failure log
unwraps Error messages (NSVC-021), and the MCP resource description drops
its false 'unread first' claim.
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.
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.
Fold the 1612-line services/atlasService.ts into the DI-native AtlasService:
the stats aggregation, visited countries/regions with the #1490 tombstone and
cascade logic, and the bucket-list CRUD become methods over the injected
DatabaseService. The ~750-line pure-geo half — the bundled admin0/admin1
boundary stores with their #1576 streaming builders, the point-in-polygon
indexes, Nominatim geocoding with its shared throttle, and every module-scoped
cache including the import-time unref'd cleanup interval — moves verbatim to
the plain module atlas-geo.ts so the caches stay process-global across the
container instance, the bridge instance and test helpers (assetPath
re-anchored one directory deeper, the only non-verbatim line).
The 10-tool mcp/tools/atlas.ts registrar plus all four atlas resources in
mcp/resources.ts move onto the decorator registry as atlas.mcp.ts — the when:
atlas-addon gate is parity here (legacy tools and resources both gated; the
REST controller deliberately does not). resources.test.ts retires with its
last two cases, which move to tools-atlas-expanded.test.ts. The plugin RPC
host swaps its 9 atlas imports for the injected AtlasService (23rd constructor
dep); a minimal 2-export atlas.bridge.ts serves the one legacy consumer,
authService.getTravelStats, and dies when authService migrates.
Adopts atlas.dto.ts over the existing shared schemas and clears all three
AtlasController entries from body-contract-allow-list.ts: the hand-rolled
'name and country_code are required' 400 becomes the ZodValidationPipe
envelope (todo/places trade) while the whitespace-only bucket name keeps its
legacy 'Name is required' trim guard.
Fixes the verified defects the relocation had faithfully carried, each with a
regression test (ATLAS-SVC-031..036 plus two MCP casing cases): the four
multi-statement mark/unmark writes now run in db.transaction() (the region
cascade nests as a savepoint), the trip-less countryPlaces early return
honours manually_marked, the '|| null' bindings that dropped lat/lng 0 and
empty-string notes on bucket update become '?? null', the mutating bucket SQL
is user-scoped, and the MCP region/country-places tools uppercase their codes
to match REST.
Quirks deliberately preserved: the detached GET-path place_regions backfill
writes, getStats' two divergent return shapes, the import-time cache-cleanup
interval, getCountryFromAddress's unvalidated bare 2-letter codes, and the
whitespace-only-name silent no-op on bucket update.
Tests move with their case IDs: atlasService.test.ts becomes
unit/nest/atlas.service.test.ts (pure-geo imports from atlas-geo, DB calls
through the constructed service, plus bridge-delegation cases), the atlas e2e
converts to the DI-native pattern (real SQL against the full temp-db schema),
and the plugin-host path mock becomes a constructor stub.
The set of uploads subdirectories is hand-mirrored between the image build
and the boot-time mkdir in server/src/index.ts, and drift is invisible
until a user whose bind-mounted uploads dir is not writable by node hits an
EACCES on the first upload to the missing dir (#1762).
Parse both lists and assert they match, with a guard case so the regexes
cannot silently match nothing and pass vacuously.
The place fold capped the Google list-import body at 8 MB but left the Naver
importer reading its pages unbounded — same attacker-influenced URL (the folder
id), and worse in practice because that path is a pager that buffers a fresh
body on every iteration.
fetchPage now checks the declared content-length before reading and the actual
length after, returning the existing 'Failed to fetch list from Naver Maps' 502
on either. Reading the body as text and parsing it explicitly (rather than
apiRes.json()) is what makes the post-read check possible; the malformed-payload
branch keeps its 'Invalid list data received from Naver Maps' 400 because the
JSON.parse stays inside the same try.
The post-read check is the one that matters for a hostile server: a chunked
response carries no content-length, so the declared check alone is evadable.
Covered for both providers now — PLACE-SVC-073b (Google, chunked) and
PLACE-SVC-074..077 (Naver: declared, chunked, malformed, happy path).
The five Naver fetch mocks in the integration suite move from json() to text().
Fold the 1029-line services/placeService.ts into the DI-native PlacesService:
the CRUD + ratings SQL, the GPX/KML/KMZ importers and the Google/Naver list
importers become methods over the injected DatabaseService, byte-identical
statements, COALESCE semantics and the If-Match conflict protocol (#1135).
The pure half — frozen XML parsers, the KMZ unpacker, the dedup predicates,
the Google hex-id parsers, reclaimPhotoCache — moves to places.helpers.ts
(maps.helpers precedent).
The 10-tool mcp/tools/places.ts registrar plus the trek://trips/{tripId}/places
resource move onto the decorator registry as places.mcp.ts. search_place travels
with them because its gate is places:read, not the read-only geo group, and now
injects MapsService. TripsService, DaysMcp, BookingImportService and the plugin
RPC host (PluginHostDepsFactory's 21st constructor dep) all inject PlacesService,
so the domain needs no bridge at all. The two assignments.bridge imports that
places.mcp.ts keeps are a deliberate cycle break: AssignmentsModule imports
DaysModule and DaysModule now imports PlacesModule for days.mcp.ts's place
creation, so injecting AssignmentsService there would close
DaysModule -> PlacesModule -> AssignmentsModule -> DaysModule. Same seam
reservations.mcp.ts uses, same trade trips.bridge.ts documents.
Also folds services/placeEnrichment.ts in — its DB/websocket/Maps half becomes
PlacesService methods over the injected DatabaseService/RealtimeService/
MapsService, its pure match selector joins places.helpers.ts — which retires
nest/maps/maps.bridge.ts with its last consumer.
Adopts places.dto.ts for the seven grandfathered body contracts and clears every
PlacesController entry from body-contract-allow-list.ts. Two consequences worth
knowing: the bespoke 400s 'Place name is required', 'ids must be an array of
numbers' and 'URL is required' are now the ZodValidationPipe envelope, and
because a pipe runs before the handler a malformed body 400s ahead of the
trip-access 404 it used to follow — the same trade the todo and trips migrations
took. placeBulkUpdateRequestSchema.ids drops .min(1) so the endpoint's
empty-list short-circuit stays reachable.
Fixes four defects the relocation had faithfully carried, each with a
failing-first regression test:
- create dropped lat/lng of exactly 0, so a place on the equator or the prime
meridian lost its coordinates (`x || null` cannot tell absent from zero).
- duration_minutes: 0 was unsettable — replaced by the 60-minute default on
create, and read as "absent" by COALESCE on update.
- the journey delete hook fired on unscoped ids: DELETE /:id, POST /bulk-delete
and the delete_place MCP tool could detach another trip's journey entries for
an id they then refused. All three scope first via the new
PlacesService.scopedIds, mirroring the guard PluginHostDepsFactory already had.
bulk_delete_places also moves its hook ahead of the DELETE —
journey_entries.source_place_id is ON DELETE SET NULL, so running it afterwards
left the entries as orphans.
- the place search treated % and _ as LIKE wildcards; a bare % returned the whole
trip. The three clauses now carry ESCAPE with the term escaped.
Plus hardening on the Google list fetch: the response is capped at 8 MB
(declared content-length and post-read length, the transit.service precedent)
and its JSON.parse is guarded, so a malformed provider payload produces the
existing 'Invalid list data received from Google Maps' 400 instead of throwing.
Quirks deliberately preserved: the non-COALESCE route_color (#776),
currency/transport_mode staying unclearable, importGpx returning null for a file
that parses but yields no usable geometry, update skipping the If-Match check
when the stored updated_at is null, and every string-valued `x || null`.
Tests move with their case IDs: placeService.test.ts and the wrapper suite merge
into unit/nest/places.service.test.ts, kmzUnpack into places.helpers.test.ts,
placeEnrichment into places.enrichment.test.ts. The places e2e converts to the
DI-native pattern (real SQL against temp-db DDL), tools-places.test.ts is kept
and extended so it exercises the decorator path through the test registry, and
the plugin-host and booking-import path mocks become constructor stubs.
WAL was hardcoded, and on network-backed storage — Azure App Service Linux,
SMB/NFS volumes on NAS and PaaS — SQLite documents it as unsafe, because the
coordination runs through the -shm file and mmap. Operators had no way out.
TREK_DB_JOURNAL_MODE and TREK_DB_SYNCHRONOUS now set both pragmas, defaulting
to today's behaviour. The mode lives in the file header, so all three
processes that open travel.db read the same configuration — the server,
reset-admin.js and migrate-encryption.ts, which used to hard-set WAL and would
have silently reverted a deliberate DELETE setup on the first key rotation.
An invalid value falls back and warns rather than refusing to start, and the
mode that actually took effect is read back and logged, so an operator can see
whether the setting landed.
The scheduled backup archived travel.db directly. The archiver reads its
entries during finalize(), so a WAL checkpoint writing pages back mid-stream
tore the copy — and the -wal that would make it recoverable isn't in the zip.
The manual path was moved to VACUUM INTO for exactly this reason; the
scheduler never followed. It calls the same createBackup() now, which also
gets it the encryption key and plugin data it was missing, so a scheduled
backup is finally restorable onto another install.
The pre-rotation copy in migrate-encryption.ts had the same problem: a plain
file copy of a live WAL database, and it is the operator's only rollback if a
key rotation goes wrong.
Auto-backups now exclude the re-derivable photo caches, matching manual ones.
Transitous exposes routeShortName, which many operators — German long
distance among them — do not fill. displayName carries the identifier
passengers actually see, with routeShortName as the fallback.
assign broadcast, unassign did not, so the place stayed on the Plan tab until
a manual refresh. It emits the same event as the REST route now. createPlace,
updatePlace and deletePlace in the same file had the identical gap.
Attachments were treated as ready-made HTML, but a real mail is multipart
with base64 or quoted-printable transfer encoding. What reached the model was
the plaintext headers followed by a wall of base64, truncated before any
content — which is why a sender name from the Subject line still showed up
while the booking never did.
The html part is preferred, plain text is the fallback, transfer encoding and
charset are decoded and entities resolved before the text goes out. No new
dependency. If no text part is found the previous raw path still runs.
Opening one trip fires a 404 per place without a photo, 30 to 60 of them from
one IP in seconds. CrowdSec and fail2ban read that as scanning; the reporter
had three users banned within five minutes. A place having no photo is not an
error and no longer answers as one.
OSM places were also sent to Google as node:/way:/relation: ids, which can
only return 400 INVALID_ARGUMENT — a billed round-trip and about 1.3s per
place. getPlacePhoto already guarded coords: this way; the OSM prefixes now
get the same treatment, as does getPlaceDetailsExpanded, which had no guard.
The negative cache separates a place having no photo from a failed provider
call instead of expiring both after five minutes and replaying the storm.
The a/b/c shards stopped being necessary in 2022 and d has since lost its
DNS record. The map never asked for d — Leaflet defaults to abc — but the
offline prefetcher rotated over a to d, which is where the failing requests
came from, and it cached most tiles under a host the map never requests.
Presets and the prefetcher use the apex host now. A URL already saved in
user_settings is rewritten where it is read and written back, so existing
instances heal without a migration touching what someone typed themselves.
The CSP only allowed https://*.tile.openstreetmap.org, and a wildcard host
never matches the apex domain, so the prefetch fetch would have been blocked.
The service worker rule had the same gap and would have stopped filling the
tile cache.
214 items came out of writing the tests. Real defects are corrected, dead code
is removed rather than covered, and the tests that pinned the old behaviour
move with them.
Also drops two agent coverage scratch files that slipped into the coverage PR
and adds the ignore rule that keeps them out.
Swap the factory's six tripService imports for the injected TripsService; delete services/tripService.ts and the collab/vacay bridges with it; prune the days/budget bridges to their surviving exports; update the migration docs and regenerate migration-graph.md.
Fold the legacy tripService SQL into TripsService; port the trips MCP registrar/resources/trip-summary prompt to trips.mcp.ts and the share-link tools to share.mcp.ts (predicate access for the trips:delete/trips:share/canReadTrips gates, deprecation notice via the attach ctx); adopt trips.dto.ts and clear the seven allow-list entries; delete todo/share bridges with their last consumers. Includes the trailing quirk fixes: transactional deleteTrip/deleteGuest, owner display_name in listMembers.
move_assignment omitted newDayId (moved assignments vanished from
collaborator views), reorder_day_assignments sent assignmentIds where
the client reads orderedIds (reorders emptied the day remotely), and
delete_day sent { id } where the client reads dayId (days never
disappeared). Registry drift findings from the WS contract work.
Nest services inject RealtimeService (call-time delegation to the
websocket module, DatabaseService pattern) instead of importing
broadcast/broadcastToUser globals. Zero behavior change; test edits
are constructor-arg additions plus RealtimeModule in e2e harness
imports only.
Maurice