The Express shell set a 100kb limit and stopped applying it the moment the
Nest instance took over parsing: bootstrap passed bodyParser:false and
nothing replaced it. The number happened to stay the same, because it is also
the framework default - but it lived nowhere anybody would look for it, and
one NestFactory default away from silently becoming something else.
It goes through Nest's own parser rather than a second express.json(),
because that parser carries the verify hook that keeps req.rawBody, and the
plugin webhook routes need the exact payload bytes to check a provider's
HMAC. Adding a parser in front of it would have taken rawBody away without
failing anything.
MISC-009 and MISC-010 pin it from both sides.
The bodyParser flag on applyGlobalMiddleware goes with it: one caller, which
passed false.
Not moved: cookieParser stays on the raw Express layer rather than becoming a
configure(consumer) middleware. The platform routes - the OAuth consent flow
among them - are registered on that instance before Nest routing exists, so a
Nest-slot cookie parser would simply not run for them. helmet, CORS,
compression, the forced-HTTPS 301 and the res.on('finish') logging stay for
the same reason: they have to see requests Nest never 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.
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.
Sixteen of the thirty-four entries in the body-contract allow list were the
JourneyController, and a strict schema already existed for six of them. That
is the trap: eight of these handlers validate their own body and answer with
their own text - 'Title is required', 'trip_id required', 'provider and
asset_id required', 'entry_date is required', 'user_id required' - and those
bodies are pinned by e2e and integration cases. Adopting the strict schemas
would have the pipe answer first, with a different body. That is a
client-visible change wearing the costume of a refactor.
So the schemas follow the memories doctrine instead: loose objects, and every
field the handler type-checks itself is unknown here. They describe the shape
that already works, which is what the boot gate needs, and every hand-rolled
400 keeps answering. The spec inverts accordingly - it now asserts that each
schema ACCEPTS what the handler still rejects, which is the property that
makes the claim checkable.
Two of them are optional at the top level, and that is not decoration: a
multipart upload with files and no text fields arrives with no body at all,
and JOURNEY-INT-045 pins 'No files uploaded' for it. Without the optional the
pipe answered 'body: Invalid input: expected object' instead. The suite
caught it.
Backup joins them - its schema was already a passthrough, because the service
normalises the body itself.
Allow list: 34 entries down to 17. Tags and the two OAuth entries stay for
now; tags reads @Body('name') per property, which is a signature change
rather than a contract one.
src/nest/** had no owner guard at all. Handing a trip over and creating or
deleting guests each asserted access.user_id !== user.id inline, and the
string they answer with differs per route, so the message travels in the
metadata rather than living in the guard.
TripOwnerGuard deliberately does not inject PermissionsService.
checkPermission returns true for every admin and the trip actions are
admin-lowerable, so routing ownership through it would quietly let any admin
transfer somebody else's trip. OWNER-004 pins that: an admin who is not the
owner gets 403.
places gets protection on the five routes where a guard is provably safe:
list, get, unrate, image and delete. Not on create, update, the imports or
the uploads. A guard runs before the pipe, so guarding a route with a
DTO-typed body would answer the trip 404 where the suite pins a 400 - that
includes PUT :id/rating, which is why the rating pair is split. The three
multipart routes are excluded for the other reason: answering while the
client is still streaming destroys the socket, and the caller sees
ECONNRESET instead of a status.
DELETE :id carries @RequirePermission('place_edit'). The first attempt at
this lost that check and answered 200 where 403 belonged, so the e2e asserts
the argument list rather than the status: a check that returns the right code
for the wrong reason fails now.
TripsService injected fourteen other services. Almost all of them were there
for two read aggregates - the MCP trip summary and the offline bundle - and
for the member roster, not for anything the write path does. Reading is what
fans out; writing is not.
Membership goes to trip-members/: listMembers, addMember, removeMember,
transferOwnership and the four guest methods, plus the six routes, which keep
their paths. It is a NEW module, not the trip-membership/ leaf next door:
deleting a guest erases their plugin data and re-splits their expenses, so it
needs auth and budget, and AuthModule already imports that leaf. Giving it
those dependencies there would have closed a hard cycle. This one is a sink -
nothing imports it back except trips.
The read aggregates go to trip-read-model/, which is where the nine-service
fan-out now lives.
What is left is the write core plus day generation, on eight parameters
instead of fourteen: database, reservations, days, permissions, budget,
vacay, realtime, unsplash. The last four are all reachable from updateTrip's
date-shift transaction, which is why the count does not fall to three.
Trips still reaches budget through ReservationsModule, so trips.bridge.ts
survives this. Saying otherwise would have been the easy claim to make and it
would not have been true.
The routes lived in reservations/ while the SQL lived on DaysService, so the
controller delegated every call across a domain boundary to reach its own
data, and neither module owned the thing outright. The seven statements move
into AccommodationsService unchanged and the domain gets its own directory,
module, DTO, RPC and MCP surface.
What stays in days/ is the reorder side: assertNoInvertedAccommodation and
resyncAccommodationDays are about re-dating day rows, and the stay is the
thing being carried, not the thing doing the carrying.
Two edges fall out of this. AccommodationsService drops its DaysService
dependency, and DaysMcp drops DatabaseService and PlacesService, because
create_place_accommodation was their only user - which lets DaysModule stop
importing PlacesModule and closes one leg of the latent
Days -> Places -> Assignments -> Days cycle.
DAY-SVC-016..026, 029 and 035 move to accommodations.service.test.ts with
their case ids intact. The reservations e2e container assembles both modules
now: it builds a single domain module on purpose, which is exactly what
catches a move like this, and it caught it.
days/ carried two fachlichkeiten with a full file set each: days.controller,
days.service, days.mcp, days.rpc, days.dto next to the five day-notes ones.
The note files move out unchanged; because the new directory sits at the same
depth, not one import path inside them changed.
The two modules deliberately do not reference each other in either direction:
dayExists is raw SQL on the days table in the note service, and DaysService
reads day_notes the same way, so a module edge would buy nothing and cost a
cycle.
DayNotesModule has to be registered in PluginsRuntimeModule as well as
AppModule - DayNotesRpc owns four of the 113 wire methods and the registry
boots with requireTotalCoverage, so leaving it out would stop every test app
that assembles the plugin runtime.
The days e2e container assembles both modules now: the note routes still nest
under the days prefix, and a container that builds only DaysModule would 404
on them while production served them fine.
The second flake recorded as 'costs' is not a timing flake. It is a live
HTTPS call: budget.service.ts settlement() reaches getRates()
unconditionally, unlike the two guarded call sites above it, and
ExchangeRatesService fetches api.frankfurter.dev with a 10 s abort.
tests/integration/budget.test.ts stubbed fetch nowhere at all, so the suite
talked to the internet - fast when the API answers, ten seconds per call
when it does not. The stub fails closed, which changes no assertion: every
settlement case here is single-currency, so rates never enter the
arithmetic. tools-budget-advanced had the stub inside a single case; hoisted
so a second settlement case cannot quietly reintroduce the call.
The five ReDoS budgets in maps.service.test.ts measured wall-clock time,
which on a loaded machine reports how long the test waited for a core.
MAPS-024 has gone red at 523 ms against an unchanged file for exactly that
reason. They now measure CPU time, which is what catastrophic backtracking
actually burns. This does not soften them: CPU time is always <= wall-clock
time, so every budget got stricter, and pool:'forks' keeps the measurement
to this file.
The all-trips feed merges many trips into one calendar. It did that by
generating each trip's finished ICS and then scanning the text back apart:
one pass for the VEVENTs, one for the VTIMEZONEs, plus a string replace to
splice the refresh hints into the header. The scans had to be structural
rather than regexes, because a user-supplied SUMMARY can legitimately
contain the literal 'END:VEVENT' and a lazy match would truncate the event
there.
CalendarService now returns the parts - calendar name, filename, VTIMEZONE
blocks keyed by TZID, and the VEVENT blocks - and exportICS serialises them.
Both scanners are gone.
CAL-010 pins the whole serialised document as a snapshot, with DTSTAMP and
the UID counter normalised so it pins the calendar rather than the minute it
ran in. It is what proves this changed no bytes: it was taken before the
model went in and still matches.
Two tests still pointed at the health controller deleted in d37af204:
health.di.test.ts loses its only subject, and the AppModule wiring test now
resolves FeaturesController instead - it is the one test that builds the
whole container, so it should resolve something that still exists.
exportICS and its four module helpers were 330 lines of RFC 5545 inside
TripsService, and FeedsService had to import the whole trip aggregate to get
an ICS string out of it. They move to a calendar domain unchanged - same
statements, same escaping, same folding, same VTIMEZONE fallback - so the
emitted bytes are identical on both the download and the feed.
The 21 unit cases move with the code and keep every assertion, which is what
makes that claim checkable rather than asserted. FeedsModule now imports
CalendarModule instead of TripsModule, and its e2e container drops from the
trip aggregate to two small modules.
NotFoundError/ValidationError move to common/ so a second domain can raise
them without importing the first one for its error types; trips re-exports
them, so the nine files that import them from there are untouched.
Fixes a harness that tsc cannot see on the way past: mcp-test-controllers
built TripsService with 12 arguments for 14 parameters, leaving unsplash and
userCleanup undefined.
GET /api/health/features lived in booking-import, which meant the client's
capability probe was reachable only because that domain happened to be
registered. Move it to health/ and give health an actual module instead of
two loose entries in app.module.
The extractor probe follows into a leaf module rather than health importing
BookingImportModule: features() needs isAvailable(), not llm-parse,
reservations, budget, maps and places.
The /api/_nest/{health,me,echo} smoke endpoints go. They proved boot, DI,
the cookie guard and the Zod pipe back when nothing else did; every one of
those now has real domain coverage, and nothing outside their own test ever
called them. The container probe is GET /api/health on the raw express layer
and is untouched.
Three things you could see on dev1.
Pictures were "roughly right" rather than right, because a coordinate search
only knows what was photographed within 300 m. Wikidata's P18 is one picture
somebody chose to represent that exact object - "Zurletzteninstanz.jpg" for the
restaurant, not a statue down the road - so it now leads, then the Commons
category, and geosearch is the last resort it should always have been. Small
venues carry a wikidata tag far more often than a Commons category, so this is
also what reaches the places the other sources miss.
Descriptions stayed on Wikipedia after the switch to Wikivoyage because the
week-long result cache predates it. It carries a format version now; a release
that changes the shape or the sources invalidates its own entries instead of
serving last week's for another six days.
Ratings and opening hours were being fetched from Google and then dropped on the
floor - the fact list only looked at OpenStreetMap tags. Both are shown now,
whichever provider supplied them.
Also: the column follows a place opened from the map, not only one picked from
search. A POI tap arrives as prefillCoords and never touched the selection, so
the column kept showing the previously added place.
Pictures are a tight three-column grid and only the picture in play is credited
in full, on one line. Crediting all of them at two lines each drowned the
column, and the licence attaches to the one that gets used - whose credit also
stays visible in the inspector after saving.
The request carries the place record the dialog already fetched, so the server
skips a details lookup it was making in parallel with the client's own. That
lookup was measured at 12.8 s for a large OSM relation.
A restaurant got nothing: no wiki article, and a coordinate search around it
returns whatever was photographed nearby rather than the place itself. Three
changes, all on free sources:
OpenStreetMap facts. A tagged restaurant carries its cuisine, its opening hours,
a link to its menu, outdoor seating, step-free access and diet tags. Those
arrive with the details lookup the dialog already makes, so showing them as
chips costs nothing and is the single biggest gain for exactly the places that
were empty before. Only positive facts are listed - "no outdoor seating" is
noise.
Commons categories. Where a place has a wikimedia_commons tag, its category is
pictures OF it; geosearch falls back in when the category is empty.
Wikivoyage before Wikipedia. Same API, same licence, but it describes a place
for someone about to visit rather than opening with area in square kilometres.
Two sentences instead of three, because this sits beside a form.
The column also follows the section-heading idiom Vacay and the dashboard share
(uppercase overline, wide tracking), its header matches the Saved places panel
beside it, and a lead picture with a row underneath replaces the two-column
grid that left a hole on every odd count. Credits wrap over two lines instead
of truncating - a cut-off name credits nobody.
Saved places stops after ten with a Show more button; search and filters still
run over everything, not just the visible page.
Form spacing is one rhythm (12px between groups, 8px inside, 4px label to
field) and Notes matches Description in height. The OSM notice fits on one line
and no longer claims there are no photos, which stopped being true when the
column started fetching them from Wikimedia. The clipboard hint is gone; paste
still works.
The nightly photo sweep deletes every candidate nobody picked, which is most of
them. The cached result still pointed at those files, and the read filtered them
out - so the morning after, a place with perfectly available pictures showed an
empty column, and kept showing one for the rest of the week the entry lives.
A missing candidate now invalidates the whole entry instead of shrinking it.
The column showed "details could not be loaded" for every place. The endpoint
was answering correctly, just after ten seconds, and the shared axios instance
gives up at eight.
The downloads sat in a loop: five Commons hits meant five round trips and five
decodes in sequence, even though the slot limiter already allows five at once.
Both provider listings now run together and the bytes are fetched concurrently,
which brings a warm request back to about a second.
Candidate keys move with it. They now number the provider's list rather than the
surviving strip, so a picture that fails to download leaves a gap instead of
renumbering the ones behind it - a key has to point at the same file on the next
request or the cached result goes stale against the photo cache.
The client asks for enrichment with a longer timeout, and the Wikimedia calls
carry one of their own: a cold TLS handshake from a fresh container was measured
at eight seconds on its own, and no pictures is a better answer than a request
that never comes back.
NOTICE knew about geoBoundaries, OSM and OurAirports but not about Wikimedia,
Wikipedia or Google Places, all of which TREK has been calling for a while.
Also fixes two dead paths in the place contract - it still pointed at
server/src/routes/places.ts and services/placeService.ts, both long gone.
Accommodation cards read the place from page-local state, so a new thumbnail
only showed up there after a reload. place:updated now asks the planner to
reload them, but only when the image actually changed - renaming a place must
not trigger a refetch.
The inspector shows the stored credit under the avatar. Crediting a CC BY-SA
picture only in the moment it is chosen is not enough; the obligation travels
with the picture.
Also lets a GIF hero through to the PDF. Uploads accept gif, the PDF extension
check did not, so one silently fell back to the category icon.
Picking a search result fills a column on the left of the add-place dialog -
pictures and a description, no extra click. Someone who already knows the place
ignores it and saves exactly as before.
The column stays mounted with an empty state instead of appearing on the first
pick, otherwise the dialog jumps sideways mid-typing. Requests are cached in
memory and in sessionStorage and aborted when the selection changes, so clicking
back and forth between two results does not pay twice.
Every thumbnail carries its author, its licence and a link to the file page.
That is not decoration - most of these are CC BY-SA, and reusing one without
naming the author does not satisfy the licence. The description is shown but
never written into the place on its own; a button copies it across, and it is
disabled while the user already has a description of their own.
Desktop only. The mobile sheet is untouched.
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.
Fans the new provider primitives out into one answer for a place that is being
looked at but not saved yet: a strip of pictures and a description, each with
its source.
Free sources are the normal case - most instances have no Google key at all, so
Commons and the OSM tags have to carry this on their own and Google is what an
admin adds on top. Pictures are copied into the existing photo cache under
"<placeId>~pN" keys rather than hotlinked: Google's photo URLs expire (migration
107 had to repair exactly that once), and a provider URL in the strip would send
every viewer's address to Google or Wikimedia. Whatever nobody picks is
unreferenced and the nightly sweep clears it out.
The result is cached for a week in place_details_cache under a third expanded
value, so no migration. Rate limited per user, and a provider being down answers
empty rather than failing the dialog.
Commons geosearch already returned five files and we threw four away, so a
whole strip costs nothing extra. It now comes back with the licence metadata
too: without LicenseShortName and the file description page there is no way to
credit a CC BY-SA image, and a picker full of them needs that.
Wikipedia descriptions resolve through the OSM wikipedia tag rather than
guessing the article from the place name, which lands on the wrong article for
anything ambiguous. Google photo references and photo bytes are separate calls
now so a caller can list without downloading, and the editorial summary has its
own lean field mask - asking for it through the expanded details would drag
reviews along and move the call into the pricier tier.
Asserting the module registration meant importing AtlasModule, which pulls
AtlasMcp and the whole MCP SDK behind it into a file that otherwise
constructs a controller with a stub. It does not resolve when the file runs
on its own.
PROFILE-014 already covers the registration, and covers it better: it calls
GET /api/auth/travel-stats through the full app.
atlas.service.test.ts carries the admin1 GeoJSON bundle warm-up from #48, and
its ATLAS-UNIT-021..024 cases sit close to the 15s limit whenever the machine
is busy. Eighteen more cases in the same file is not what they need.
Same DB-per-file pattern, no shared state, case IDs unchanged.
The last of AdminService's borrowed SQL. oauth_tokens belongs to oauth/,
which already had a user-facing view of the same table; AdminService carried
a second one only because the panel route is under /api/admin.
Both survive, under names that say which is which. They are not duplicates:
the user-facing listOAuthSessions is scoped to one user, the admin one spans
all of them and joins the owner username, and the admin revoke kills the
client session for whoever owned the token.
OauthModule gains its first export. Its header said nothing inside the
container injects the domain, which stops being true here.
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.
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.
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.
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.
default-user-settings, notification-preferences and oidc leave AdminController.
The first two were pure pass-throughs on AdminService that did nothing but
forward and cast; the third had its SQL there for a domain that already had a
module of its own. AdminService loses SettingsService and
NotificationPreferencesService entirely.
Two things came out of the move rather than going into it.
The OIDC lockout guard now has tests. It refuses to clear the SSO config while
password login is disabled — otherwise the instance has no SSO to log in through
and no password form either — and nothing covered that branch before, because it
sat among admin tests that only exercised the happy path.
The admin e2e app had to learn about the split. It assembles AdminModule alone,
so the moved routes 404'd there while working in production; it now imports the
three domains too. That is the same failure mode the module-provider guards
catch for RPC classes, and it is worth knowing that an e2e app which builds a
subset will find it first.
Also fixes the audit route note: audit-log stays on AdminController on purpose.
AuthModule imports AuditModule, so an admin-gated controller in AuditModule
closes a cycle — the same trap the addons toggles hit.
Twelve admin packing-template routes sat on AdminController and reached
PackingService through eleven pass-through methods on AdminService that did
nothing but forward and cast. PackingService already owns all three template
tables, so the routes move to AdminPackingTemplatesController next to it and the
pass-through block is gone. AdminService no longer injects PackingService at all.
The three places flags were raw app_settings SQL on AdminService, sitting beside
bag-tracking and collab-features which it already delegated to AddonsService.
They move to AddonsService, which owns every other flag, and AdminController now
calls it directly rather than through delegating methods.
The toggle ROUTES stay on AdminController, which is a departure from the plan and
worth stating: an admin-gated controller in AddonsModule makes that module import
AuthModule, and PluginGuardsModule imports AddonsModule precisely because it is a
leaf — Nest refused to build the graph. Same for the addon CRUD, whose PUT
cascades into PluginRuntimeService. Both notes are in the code.
Paths, response shapes, the {error,status} envelope, the create-201 split and
every audit action are unchanged.
assignments, budget, collab, accommodations, reservations and booking-import
drop their copied requireTrip/requireEdit helpers for the guard. Status codes and
bodies are unchanged.
places is deliberately NOT converted, and that is the finding of this PR rather
than an omission. Its e2e suite documents that the body pipe answers 400 ahead of
the trip 404 — a parity shift somebody made on purpose — and a Nest guard always
runs before the pipe, so converting it silently reverses that ordering. The first
attempt also dropped the permission check on DELETE, which the integration suite
caught as a 200 where a 403 belonged. places keeps its in-handler check, and
README now says why so the next person does not rediscover it the same way.
collab applies the guard per handler for the same reason files does: its note-file
upload route would otherwise be refused before multer had read the body, and a
response sent mid-upload destroys the socket.
Two coverage notes. The per-domain thresholds are regenerated because deleting a
well-covered helper lowers a file's ratio without anything becoming less tested —
the drops are 0.4 to 1.2 points across six domains and match the helpers removed.
And the atlas integration suite gets the same admin1-bundle warm-up the unit suite
got: ATLAS-013 was paying 3.1s for the one-off decompression inside its own 5s
budget, which is why it only ever failed under load. It now runs in 116ms.
Every trip-scoped controller carried its own three-line requireTrip, and a
requireEdit next to it: eighteen copies of a security check, each of which had to
remember that no access is a 404 'Trip not found' and never a 403 — the
distinction that stops a stranger learning which trip ids exist. One of them
getting it wrong would have looked like nothing.
TripAccessGuard resolves :tripId once, refuses what the user cannot reach and
hands the row to the handler through a @Trip() param decorator (a param
decorator rather than a req.trip cast, because the request type that used to be
widened for that is gone). @RequirePermission carries the same action string the
domain services pass to checkPermission, so the HTTP and MCP paths cannot drift
apart on which right they demand. Status codes and bodies are unchanged.
Piloted on days, day-notes, todo, packing and files. Two things worth knowing:
Files applies the guard PER HANDLER and leaves the upload route on its own
check. Guards run before interceptors, so a class-level guard would answer 404
before multer had read the multipart body, and a response sent while the client
is still streaming gets the socket destroyed — the caller sees ECONNRESET
instead of the 404. Refusing after the body has been read costs the bandwidth
and keeps the answer readable.
The services keep verifyTripAccess and canEdit: the large majority of their
callers are *.mcp.ts tools, which never pass through an HTTP guard. canEdit lost
its incidental controller-test coverage in the move, so it now has direct tests
asserting the action and the shared flag — the same pair the guard reproduces.
Also fixes the backup zip-slip test that failed only on CI: it asserted a
drive-letter path, which is absolute on win32 and an ordinary relative segment on
Linux, so the branch it was guarding never fired there.
Maurice