mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-22 06:56:35 +00:00
f460ae4fcbdb526975bea4274e565efd6be01fba
70 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 jubnl
|
6b755314d8 | refactor(collab, collections, maps): async DB-touching methods; transactions through UnitOfWork | ||
|
jubnl
|
4df2d83137 | refactor(notifications, place-photos, backup): async DB-touching methods; transactions through UnitOfWork | ||
|
jubnl
|
953e2526bb | refactor(atlas, school-holidays, settings): async DB-touching methods; transactions through UnitOfWork | ||
|
Maurice
|
ed18c37903 |
fix: the findings of the final 4.3.0 audit
Every file the release touches was read once more, and what survived an independent check against the code is fixed here, with a test for each. Data and credentials - A document sync connection edited to a new address no longer carries the stored token along: a blank form under another origin is refused, the same rule the probe route already had. Dawarich and AirTrail testConnection send the stored key only to the host it was stored for. - The key rotation script now covers document_connections.secrets and trip_document_links.webhook_secret, which it had skipped. - Turning a booked night into a pause in the road trip popup asks first and says that the booking and its expense go with it. Deleting a place says the same when a night is booked there, on the desktop and on the phone. - A moved version marker over unchanged bytes no longer replaces the TREK file and its attachments; a real revision carries the attachments over. Planner - A drop in the day plan names the row it lands ahead of in the full day, so a hidden hotel stop no longer shifts it one slot early; the store sorts before it splices. Via re-anchoring, the booked-night guard and the sidebar's day membership read the stored day, the index space the server uses. - A hotel stop placed mid-day re-pins the road trip vias of that day, and the booking session receives the new stop order too. The Public API and the iCal feed list a booked hotel once. A plugin can no longer write a stay against another trip's day. Road trip and map - The route overview paces its requests to the public routers and retries a rate limit, and it says which legs it could not route instead of quietly reporting a shorter trip. A click on a via handle no longer drops a second via; handles hide when zooming out. Read-only members cannot edit the driving limits. The phone shows a way back when no day is picked and prints the night drive departure in the chosen clock format. Integrations and search - Google transit keeps conventional trains when the train chip is on. TREK_PLACES_ENABLED=false stops the details and enrichment calls too, and accepts the whole boolean family. A caller-supplied description no longer lands in the instance-wide enrichment cache. Nominatim searches carry a timeout. An OSM place id is checked for shape before it reaches Overpass. - Dawarich puts a stay on the trip whose dates hold it, the Atlas hand-off knows what the Atlas already shows, and a host change with a new key clears the old instance's state. - Document sync: a webhook address pasted by hand works on the token alone, deleting a connection unregisters its subscriptions, Nextcloud under a sub-path connects, Synology answers a name collision before uploading and verifies a move into the bin, Paperless titles are capped at 128 characters, a DNS failure reads as unreachable rather than as a policy verdict, videos from the store are accepted like uploads, and remote_missing has a way out. Conflict buttons only for the owner, a refused choice is reported, a refused credential gets a Reconnect button, disconnecting asks first on the desktop as it does on the phone, and the folder picker no longer spins forever. Release - The server holds the release notice back from a bundle that cannot draw it, so the previous shell served from the service worker cache after an update no longer shows bare keys and no longer lets a click use the notice up. - Import into an existing list gets the same body limit as a new list. Chat images say when the limit is reached and the new controls are translated. The transit backend switch reports a failed save. The Helm chart carries the Amap credentials in its Secret; the shipped templates document the Places API and Amap variables and the Overpass default. The wiki follows. |
||
|
Maurice
|
08df1812be |
fix(sonar): spell out the order these five sorts already have
SonarCloud's S2871 fires on a bare .sort() over strings and asks for localeCompare, and it had five of them on new code, which is what put new reliability at D and failed the gate. All five sort ISO dates or query parameter names, where the default lexicographic order is exactly the one wanted. Taking the rule's suggestion literally would have been the wrong fix twice over: a locale-aware comparison reorders dates differently per runtime, and on the Amap signature it would produce a canonical string Amap itself does not compute, so every signed request would come back rejected. So the comparator is written out rather than changed. Byte order, explicitly, with a line saying why it has to stay byte order. Behaviour is identical: the 43 Amap cases, the 40 track cases and the 30 client cases over these modules all pass unchanged. |
||
|
Maurice
|
c8cb88133f |
fix(maps): the projection, the labels and two unbounded reads
A map keeps the datum it was built with, and Leaflet cannot change one afterwards. The basemap could still turn into Amap's after the fact: a first settings request that fails leaves the store on defaults for the session and is healed later, and the raster layer then drew GCJ-02 tiles under a map still projecting WGS-84, putting every marker, route and cluster a few hundred metres off the street beneath it until the planner was left and reopened. The datum is part of the element's identity now, so a flip rebuilds the map. Only the datum: keying on the whole tile URL would throw the view away on every template edit. Index POIs were labelled by an exact lookup of a term the index matches as a substring, so any leaf that is not literally a term missed and took the first category of the request instead. In a corridor search that is whichever pill was tapped first, and the panel groups and colours on that field: a trattoria came back as a petrol station, or a petrol station as tonight's hotel. The label now comes from the term that actually matched, longest first. Two outbound reads were missing what the house rule asks for. The brand-logo fetch had no deadline at all next to a Wikidata hop that allows six seconds, so a stalled CDN held a request context and a socket for undici's five-minute default, and its size cap only applied after the bytes were in memory. The Wikimedia photo proxy beside it had the same shape. And the Amap provider buffered every answer uncapped and trusted a cast for its arrays, so a `pois` object rather than a list threw a TypeError that surfaced as a 500 instead of the OpenStreetMap answer. |
||
|
Maurice
|
f2bd116044 |
docs(maps): an Amap link needs a coordinate
A bare amap.com/place page carries an id and nothing else, and turning that id into a location would need a keyed detail lookup the URL resolver has no user for. It answers 400, which AMAP-084 pins. The wiki offered the link as paste-able and the parseAmapUrl comment still promised the lookup, so both now say what the code does. The provider header also claimed photoRefs/photoBytes were deliberately left out, naming two methods the places-provider interface no longer has. The reason is unchanged: an Amap picture comes without a licence statement we could show, so an Amap place gets its picture from Wikimedia. |
||
|
Maurice
|
ba535e5aab |
fix(maps): ask Amap only where Amap can answer
With Amap in the keyed slot every reverse lookup went to Amap first, worldwide. Outside its box that buys a round trip and an empty answer before Nominatim is asked anyway, which is latency on every right click in Lisbon. The client side of the same question already filters by the box (placeNavigation.ts); isOutsideChina is the shared helper both use. The two fixtures in the file that were cast to any now carry a type: the key row and the fetch spy whose calls the reverse case reads back. |
||
|
Maurice
|
1944bdf0a5 |
feat(maps): let Amap take the keyed provider slot beside the TREK index
The rebase kept maps.service.ts as dev has it: the TREK index and OpenStreetMap answer every search first, and a keyed provider only fills what they leave empty. Amap now sits in that slot next to Google instead of behind an extracted Google provider, which is dropped. places_provider (auto | google | amap | openstreetmap) decides which keyed provider holds the slot; auto keeps whatever the install already used. An amap: id opens against the Amap key regardless of the setting, takes the plain details lookup on expand, never reaches Google's photo call, and a pasted Amap link resolves through parseAmapUrl. Reverse geocoding asks Amap at the instance key before Nominatim. The migration carries users.amap_api_key, which the rebase had left at a stale array position, and places.amap_poi_id for the next commit. |
||
|
A1phaN
|
57eac32f9f |
feat(map): amap as map api provider and display template
|
||
|
Maurice
|
ff5fa55fad | add Google Maps route import and plugin corridor search | ||
|
Maurice
|
7ba7338f5f |
fix(maps): sag bei jedem Vorschlag, welcher Index geantwortet hat
Seit die Tipp-Suche beide Schichten fragt, ist ihre Liste gemischt — die Antwort nennt aber weiterhin EINE Quelle fuer die ganze Liste, und die heisst 'trek-places'. Der Client beschriftet damit jede Zeile, auch die aus OpenStreetMap. Wer tippt, liest also acht Mal "TREK" und schliesst daraus, die OSM-Schicht antworte gar nicht. Sie antwortet, sie steht nur unter falschem Namen da. Auf der Suchstrecke daneben stimmt es laengst: dort traegt jeder Ort sein eigenes `source`, und genau deshalb liest sich diese Liste richtig. Also traegt es jetzt auch die Zeile. Das Feld ist optional, weil Google und der Nominatim-Rueckfall je aus einer Quelle antworten; eine Zeile ohne eigene Angabe faellt weiter auf den Namen der Liste zurueck. Dazu die Koordinaten, die der Vorschlag ohnehin schon kennt. Bisher wurden sie weggeworfen, und wenn der Details-Aufruf nicht antwortete — Kill-Switch, ein ueberlasteter Overpass-Spiegel, irgendein Fehler oben — baute der Client eine Textsuche aus Name und zweiter Zeile. Bei einer OSM-Zeile ist die zweite Zeile aber keine Adresse, sondern der ortsuebliche Name: gesucht wurde dann nach "Tokio Hauptbahnhof, 東京駅丸の内駅舎", und dessen erstes Ergebnis wurde kommentarlos als der Ort genommen, den der Nutzer angeklickt hatte. Jetzt nimmt der Rueckfall den Punkt, den die Zeile mitbringt. Und das Abzeichen gibt es endlich auch mobil. Die Marke und ihre Tabelle liegen dafuer in client/src/utils/placeSource.ts, weil beide Oberflaechen dieselben Listen zeigen und ein Zeichen, das nur auf einer davon steht, schlimmer ist als keines. Was hier NICHT drin ist, obwohl es dazugehoert: dass der richtige Bahnhof in der Liste steht. Das liegt am Dienst, nicht an TREK, und ist dort gemessen und behoben — Rangfolge nach Art des Ortes plus die Verkehrsknoten, die der OSM-Schicht ganz fehlten. |
||
|
Maurice
|
fab3d2bdf8 |
fix(maps): ask the OpenStreetMap layer on the autocomplete path
Typing "Tokio station" from Tokyo suggested a weigh station in Ritzville and a
station in Tuxtla Gutiérrez, and typing "Tokyo Station" put Japan in seventh
place behind Markham, Arlington, Calgary, Milano and Melbourne. The station
itself was never in the list.
The service has had the answer all along. Its OpenStreetMap layer carries 14.6M
travel places with every name form each is known under, including the exonyms a
traveller actually types, and it is not the same data as the index: Overture
holds businesses, so a search for a station finds the shops inside it. The layer
returns "Tokio Hauptbahnhof" for the German spelling and 東京駅 for the Japanese.
It was never asked. `sources` defaults to the index alone and trekPlacesSearch
never sent the parameter, so no caller here has ever seen a layer row.
Sent on the autocomplete path only. The explicit search asks Nominatim in
parallel and would get the same OpenStreetMap places twice; this path asks
nobody else, because Nominatim's usage policy names autocomplete as
unacceptable use. So the layer is not a second opinion here, it is the only
place those names live.
A layer row has a different shape, and that shape is why this is more than one
parameter. It has no `gers` and no address block, and its id reads
`osm:node/9712313` where this file resolves `node:9712313`. Passing it through
untranslated would hand the client an id getPlaceDetails does not know, and the
failure would land after the user had already picked the suggestion. The
secondary line falls back to the local name, which is what is written on the
building and more use under a translated label than an empty line.
Measured against the service before the change, same query, same coordinates:
"Tokio station" index only -> Tokio Weigh Station, Ritzville US
with layer -> Tokio Hauptbahnhof in second place
"東京駅" index only -> six restaurants inside the station
with layer -> 東京駅 itself in the list
|
||
|
Maurice
|
4a88dbcbcf |
fix(places): say where a result came from, and stop asking questions in the form
Six changes to the place picker and the card behind it, all of them about a panel saying less and meaning more. The TREK place index has no switch in the admin panel any more. It decides whether a search leaves the instance at all, which is a property of the deployment rather than a taste: an operator pins TREK_PLACES_ENABLED=false in their compose file, and it cannot be turned off from a browser by whoever holds an admin account that day. The switch never shipped, so nothing to migrate. The standing "Using OpenStreetMap" notice above the search box is gone. It was a sentence about configuration in the middle of a form about a place, and it answered a question nobody asked while leaving the real one open. Each result now carries a small mark instead: TREK, OpenStreetMap or Google. It sits on the row rather than above the list, because a list can be two indexes interleaved and a heading could only name one of them. The saved-places filters open across the full width of the row. Two of them sit side by side, only one can be open at a time, and list names were being cut at about ten characters for no reason. Three empty states use the mascot the rest of TREK uses. The place details panel no longer tells the reader to go and ask an administrator for a Google key, the saved-places list and the road trip rail no longer explain themselves in two lines of grey text. That needed one new mood, `sad`: `sleepy` reads as falling asleep, which is a different thing from finding nothing. The Kind of stop field is out of the place form. It is a road trip concept, and the road trip view already sets it from the stop's own number, where the choice is next to what it changes. |
||
|
Maurice
|
41e256bf0f |
feat(roadtrip): answer the corridor search from the place index
Search along the route went to Overpass and only to Overpass, on the one path where that hurts most: one search fans out over sixteen boxes, and each of those races four public mirrors. The index answers the map's explore pills in milliseconds where those mirrors take seconds, and the corridor was the reason the index exists. Two independent things kept it out. The corridor sends its categories as a comma-separated list, and the index branch looked the whole string up as one key, so it missed every time. And the four kinds a drive needs, fuel, charging, rest areas and campsites, had no Overture terms mapped at all. They do now: gas_station and fueling_station, ev_charging_station, rest_stop, and campground with rv_park. The branch is list-aware the way searchOverpassPois already was, and it labels each hit with the category that produced it rather than with the list that was asked for, because the client colours and groups its markers by that field and "fuel,charging" is not a category. It answers all-or-nothing: a category the index has no terms for sends the whole query to Overpass, since answering the half it knows would silently drop the rest while the panel still reported a finished search. The caller's limit is passed through instead of a hardcoded 50, so a mixed search gets the same per-category budget the Overpass path spends. The other outbound path of this addon changes with it. Importing a shared Google route geocoded its waypoints by calling searchNominatim directly, which walked past geocodeQuery and its index lookup. Through geocodeQuery an index hit costs no throttle slot at all, and what the index does not know still falls through on the background lane. A place answered from the index also carries its chain and that chain's Wikidata item, so a branch gets its logo on the pin here exactly as it does on the Overpass path. Sockets stay an OSM field: a charging station answered from the index reports "not stated" rather than claiming it offers nothing. |
||
|
Maurice
|
6fca238440 |
feat(places): import the stops of a shared Google Maps route
Pasting a /maps/dir/ link into the list-import box answered "could not extract list ID", which is the whole of the complaint: people share a drive far more often than they share a list, and it is the same gesture — somebody pressed Share in Google Maps. Which screen they were on is the URL's business, so the box takes both and the controller decides. No API key and no call to Google for the link itself: the stops are in the URL. Three shapes arrive — the path form with its data blob, the documented ?api=1&origin= form, and a short link that is one of those after a redirect — and a stop the link spells out in coordinates needs no geocoder at all. One that is only a name is geocoded, keeping the name from the link rather than the one the geocoder answers with: somebody who typed a nickname into Google should not find a street address on their trip. A name nobody can place is left out and counted rather than failing the import, because a route of six stops with five findable is five stops more than the traveller had. Coordinates are read as 1d/2d, longitude before latitude, which is the reverse of a place link and the one way to put Berlin in Somalia. A blob that does not line up with the stops is ignored entirely: half a link read confidently is worse than one read as names. |
||
|
Maurice
|
061259b3d9 |
feat(maps): pass through what OSM knows about a charging station
The corridor query has always ended in `out center tags`, and the projection has always kept six keys out of the whole tag set. Everything an electric car needs to know was in the answer and went in the bin: which plug families the station has, how many of each, at what power, how many vehicles fit, and whether it charges. Reading them costs no extra request, no key and no second query. The filters treat "not stated" as its own answer, which is the only honest way to build them: OSM states a socket type on roughly a third of charging stations and a power on fewer still, so a power filter keeps every station that says nothing rather than reading silence as too slow. The plug dropdown offers only the families this search actually turned up, because CHAdeMO in a list for a corridor with three Type 2 posts on it is a filter that can only return nothing. Socket names stay as they are printed on the plug. "Type 2" is not a string anyone translates, and the lowercase tag key is not what is written on the charger. The tool description follows, as parity requires, including the coverage caveat — a model that reads a missing power as "no fast charging here" would be wrong about most of the map. |
||
|
Maurice
|
1db55f734c |
fix(maps): wait longer than the query we ask Overpass to run
The query carries `[timeout:20]`, so the mirror is allowed twenty seconds of work on it — and the fetch around it was aborted after twelve. A mirror under load that would have answered at fifteen never got the chance, and the caller saw a failed area. That is what makes a corridor search along a long drive report half its stretches as unsearchable: the stretches were being searched, we just hung up. Both numbers now come from one constant, the client budget is the query's own plus room to hand the answer back, and a test holds the relationship rather than the two literals. |
||
|
Maurice
|
d6955f20a6 |
test(roadtrip): cover the routing, corridor and panel behaviour
The addon shipped with only its two pure modules under test, which left the parts that actually talk to the network — and the panel a user reads the answer from — resting on nothing. Covers the routing hook (one request per travel-mode run, day default over trip fallback, retry then give up, no re-fetch on a rename, abort on leave), the corridor search (one query per box carrying every kind, the perpendicular filter, dedupe, the retry pass before a stretch counts as unsearched), the corridor state (roads driven over the line between stops, day follow-along) and both panels. Two findings from writing them, fixed here: - A dead straight leg is reduced to its two ends by the simplifier, and tiling walks point to point — so a 250 km motorway stretch produced a box at each end and nothing in between, while the search still reported itself complete. Steps longer than half a box are now split before tiling. - projectOnSegment guarded a degenerate segment with `lenSq === 0`, which two ends that came through a chain of floating point arithmetic never hit. Also splits the anchor resolution out of computeSchedule, which was three levels of nesting for one question, and sorts the POI cache key with a comparator. |
||
|
Maurice
|
6aa7884874 |
perf(maps): search a corridor with one query per box, not one per category
The road trip corridor asks for fuel, charging, rest areas and campsites over a dozen boxes, and asking per category was four times the requests for the same answer — slow, and four times the chance a shared Overpass mirror times out and reports the stretch as unsearched. `category` now takes a comma-separated list. Each OSM selector belongs to exactly one category, so a hit is still labelled with the kind it answered, and the result cap is shared out per category so one dense kind can't crowd the rest. The MCP tool takes an array for the same reason. Also puts the POI, plugin and route via-point pins on the map's render clock, the same fix the planned-place pins already had: a library marker repositions on every pointer sample while the canvas draws once a frame, so the suggestions swam during a drag and snapped back on release. |
||
|
Maurice
|
37179cb608 |
feat(maps): show a chain's own logo on its map pin
A corridor full of petrol stations was a row of identical coloured dots, and the one thing that tells them apart is the brand. OSM carries brand:wikidata on most of them — 88% of the fuel stations in a Berlin box — Wikidata carries the logo as property P154, and Commons serves it. The bytes are proxied and cached rather than linked: one self-hosted instance asking Wikimedia for a handful of logos is a very different egress profile from every visitor's browser announcing which petrol stations they are looking at. The cache is in memory because a GET must not write to the database, and there are only so many fuel brands; a brand with no logo is remembered too, or every pan would ask again. A logo-less or unreachable brand answers 204 and the pin keeps its category icon, so nothing depends on Wikidata being reachable. Both renderers show it, since both read the same category table. |
||
|
Maurice
|
07a4447bbd |
feat(roadtrip): find fuel, rest areas and sights along the drive
The road trip mode could show the route but not what sits beside it, which is half of what planning a drive is about (#1797). The right column becomes a corridor search: pick what you need, pick how far off the route you will go for it, and the day's drive is searched box by box. The boxes come from the route itself rather than the viewport, so the answer is "on the way", not "on screen"; each hit reports how far off the route it lies and how far into the drive it comes, and adding one opens the normal place form so it lands on the day like any other stop. Overpass is asked in plain bounding boxes rather than with a polyline filter, because that is what the existing /api/maps/pois route accepts and what the mirrors can answer; the geometry work is pure and tested. Long routes stop at ten boxes and say so instead of quietly searching half the drive. Fuel, charging, rest areas and campsites join the server's POI categories, so the explore pill and the MCP search_pois tool gain them too. A car booking's stops now also reach the PDF, which printed pick-up and return with everything in between missing. |
||
|
Maurice
|
7cdd61e7fb |
fix(maps): say which source answered, and stop the stubs driving the index
Two halves of one mistake, both found by the suite. Letting each side of the merged search catch its own failure meant the index could contribute nothing while the answer still called itself "trek-places+openstreetmap". That label is not decoration: the search log writes it into the corpus a candidate index gets scored against later, so a list that is entirely OpenStreetMap must not be recorded as though the index had a hand in it. The source now names what actually contributed. And maps.service.test.ts drove the index for real. Its fetch stubs answer with json() and no text(), which is not what the index client reads, so the branch was entered and then failed on the stub — "uses Nominatim when user has no API key" was really testing that a stub is incomplete. With the new circuit breaker those failures also accumulated, and the breaker is process state, so it leaked into whatever file ran next. trekPlacesSearch is stubbed there now, beside the nearby stub that was already there for the same reason. The index paths keep their own files. |
||
|
Maurice
|
ed5769fc89 |
fix(maps): name the index as the source, and stop it being asked twice
Attribution first, because this branch argues the point itself: the card names the sources with the comment that naming them is a licence obligation rather than decoration, and the copy beside it promises that every field says where it came from. The POI path then stamped Overture rows as OpenStreetMap. Overture is not OpenStreetMap — it carries OSM among other sources under other licences — so that is a wrong attribution rather than a rounded one. The wire contract keeps `source` an open string and nothing on the client switches on it, so saying `trek-places` costs nothing. The MCP tool description no longer calls itself an OpenStreetMap tool either, and says which source answered. Adding one place cost two round trips to the index. The details lookup fetches the place and its answer carries the description; toPlaceRecord dropped that field, so the enrichment fetched the same place again a moment later to read it. It is carried through now and read from what the caller already has, with the second lookup left as the fallback for when there is nothing to read. That description's source URL becomes an href on the client and was taken on `typeof === 'string'` alone. It now goes through the same allow-list a place's website goes through — the file already does this for menu_url, with the comment explaining why — so anything but http(s) loses the link and keeps the text. The instance token was hashed from raw APP_URL, which the shipped compose file leaves commented out. Every default install therefore sent the same token, and a token every caller shares is not a caller: one instance running a large import would rate-limit all the others, who would fall back to Nominatim with nothing to point at. getAppUrl() falls through to ALLOWED_ORIGINS and then to the port, so unconfigured instances still differ. Two client-side leaks. A pick made out of the offline cache was recorded into the shadow corpus, where it moved the two figures the corpus exists to measure — and offline is a setting, so the network is usually reachable and the row landed. And the prefetch never re-checked the session after its request came back: a logout in that window repoints the handle at the anonymous database, so one person's trip area was written where the next person's offline search reads. The cache path also ignored the abort signal, so an answer for a keystroke two keystrokes old could overwrite a newer suggestion list. It rejects the way both surfaces already know to swallow. Left alone, and said in a comment rather than hidden: the index cannot localise. The Overpass path picks name:<lang> and falls back to int_name; the service has no language parameter at all, so a German reader exploring Tokyo gets the Japanese primary names on this path. |
||
|
Maurice
|
b0aa7f8d95 |
fix(maps): stop asking an index that is not answering, and untie the place cache
Three things an install feels rather than reads. A service that cannot be reached cost the full timeout on every call. Each call site falls back to the old path, but pays 3.5 s first — so an instance behind a firewall that drops rather than refuses, or one that simply has no route to the service, pays it on every keystroke of autocomplete and on all three queries a booking import runs per venue. Ten venues is over a hundred seconds of waiting for an answer that was never coming. A process-wide breaker now fails instantly after four consecutive failures and tries again a minute later, so an unreachable index costs one timeout instead of one per request. A 404 does not count against it, because "no such place" is an answer, and neither does an oversized body, because the service replied. The offline place cache hung off the map-tile switch. That setting is about tiles — its own text says "off keeps the cache to trip data + documents only" — and it gets turned off for space, tens of megabytes of it. A trip's places are about one. Somebody who turned tiles off therefore got an empty offline search with no explanation, and "prepare for offline" reported success having stored no place at all. The two run independently now; the tile progress phase stays gated on the tile setting, so a run without tiles still reports no tile phase. Tests for the merged search path, which had none: the wiring around mergeSearchResults was uncovered, so which list goes in first and what `source` says were both free to change. Neither is cosmetic — index-first is the whole point of the feature, and the search log writes `source` into the corpus a candidate index gets scored against later. Two of the eight cases pin the fix from the previous commit: an index failure keeps the OpenStreetMap answer, and neither source is asked twice. place-shadow lands on its own coverage entry rather than the 80 catch-all, where a new domain could shed twenty points unnoticed. Pinned under its measured figure on purpose; the exact ratchet gets regenerated with the repo's own script after a full run. |
||
|
Maurice
|
5aa36aaf3d |
fix(maps): stop the index costing a second search, a duplicate and a lost cache
Six things the merged search and the offline cache got wrong. An index failure took the OpenStreetMap answer down with it. Only the OSM half of the Promise.all had a catch, so a rejection on the index side discarded ten results that had already arrived and sent the same query to Nominatim a second time, behind its 1.1 s process-wide throttle. The index refusing a query is ordinary traffic rather than an outage — a common single word without coordinates is turned down upstream as too expensive — so this fired in normal use. Each side now catches its own failure, and the keyless fallback reuses the answer OpenStreetMap already gave instead of asking again for a list that cannot have changed. Dedup never fired for a non-Latin name. namesOverlap splits on [^a-z0-9], so 長谷寺, Кремль and مسجد الشيخ زايد all tokenise to nothing and the function returned false — the same place came back once from each source, one row under the other, and picking either gave it a different id. That is the corpus this whole feature was measured on. Two names with no Latin words are now compared strictly, folded through NFKC so the width variants the sources disagree on still match; sharing a character is deliberately not enough, because deduping 東京タワー against 東京タワー駅前郵便局 would swallow a real result. expand=1 answered nothing for an index place while expand=0 answered in full. The expanded lookup degrades OSM ids to the plain one and had no branch for the new family, so the richer request was the poorer one, over REST and MCP alike. The bias circle could exceed Google's 50 km ceiling. A trip spread over more than that produced a radius Google rejects outright, turning a search into an error toast. Clamped on both sides — the client because it draws the box, the server because the radius arrives over the wire. The offline cache was rebuilt on every sync and never deleted. syncTrip wrote its meta row with put(), which replaced the whole record and dropped the fingerprint saying the cached area was current, so every login, reconnect and manual sync re-downloaded megabytes that had not changed. And clearTripData never touched areaPlaces, so a trip switched off for offline use kept answering offline searches and its rows stayed for the life of the install. clearCachedPlaces, which claimed to do that job, was called from nowhere; its only test asserted with .every() on an empty table and passed on nothing. Picking an offline suggestion ended in an error toast. The details lookup had no cache path, so it failed and the callers fell back to searching for "name, address" — which the cache matches on the folded name alone, so it found nothing for a place that was sitting right there. A miss stays a miss. |
||
|
Maurice
|
273bac8917 |
fix(maps): keep index ids away from Google, and let the area download be big
Four things the index changed and nothing followed: `gers:` was missing from NON_GOOGLE_PLACE_ID, so isGooglePlaceId said true for every id the index hands out. Five call sites gate on that function to keep unresolvable ids away from Google, which answers them with a billable 400 INVALID_ARGUMENT: photo refs, the editorial summary, the place-photo route, the expanded details lookup and the enrichment photo pass. Since the index answers first for search and autocomplete, a GERS id is what an ordinary new place now carries, so this was the normal case rather than an edge one — three billed lookups per added place, none of which could ever return anything. The area download shared the search path's megabyte cap and its 3.5 s timeout. Both are right for a keystroke and wrong for the offline prefetch, which asks for 3000 rows at once: a city-sized answer sits on that megabyte, readCapped threw, and the caller could only report "index unavailable" — the trip then went offline with an empty place cache and nothing said so. The cap now follows the row count the caller asked for, with a floor, and the bulk call gets a budget that fits a bulk download. It is still capped, so a broken upstream cannot hand us an endless body. searchPlaces returned before the Google branch as soon as the index or OpenStreetMap had anything, and both set google_place_id to null. That is right for a search and wrong for the one caller that wants an identity rather than an answer: list-import enrichment (#886) exists to attach a google_place_id to a bare imported pin, and pickEnrichmentMatch discards every candidate without one. So the import silently stayed unenriched on an instance that pays for a key. That caller now asks for the Google identity explicitly. Nothing changes for an instance without a key — enrichment could never resolve anything there, before or after. Tests cover the three functions this branch had left untested, which is what put the maps ratchet under its functions threshold: the autocomplete index path (a new file, MAPS-AUTO-001..007), the gers details branch when Overpass is down (MAPS-GERS-008), and the client's abort timeout — including that the signal exists at all, which nothing asserted before. |
||
|
Maurice
|
d613c91e16 |
fix(places): keep the chain out of the field that means "this place"
toPlaceRecord wrote the brand's Wikidata item into `wikidata`, and getPlaceDetails spreads the record over the OSM details, so for a chain branch picked from the index that value replaced the building's own tag - and nulled a real one when the place had no brand. resolveIdentity reads that field as the place's own identity, so the photo ladder returned the chain's Commons files as pictures of this branch and the description came back as the company's article without the aboutBrand flag. That is precisely the failure WIKI_IDENTITY_TAGS and readBrandIdentity were written to prevent, in their own words: "a chain's logo passed off as a photo of the restaurant would be a plain lie". The item now travels under brand:wikidata, the key OSM uses and readBrandIdentity already reads, and resolveIdentity picks it up from the payload for a place OSM does not know - so a branch still gets the chain's description, flagged as such, and never its logo. The client test also covers trekPlacesArea, the streaming half of readCapped including the size cap, the configured base URL and the nearby defaults, none of which any test reached. |
||
|
Maurice
|
0991fa538b |
fix(maps): stop pois() dropping opening hours and reporting a narrowing as none
The index-first branch hardcoded opening_hours: null while the payload it maps carries them, and search_pois advertises "opening hours wherever OSM carries them". It also capped the radius at 20km and then answered clamped: false, so a wide viewport came back narrowed and said it was not - the same field the Overpass path fills honestly. The three red MCP cases were the same code telling the truth from the other side: they stub Overpass one level below the facade, so pois() ran for real, and because trekPlacesEnabled() fails open on a database with no row, a unit test was sending live requests to places.liketrek.com from CI. That is what answered with 50 places instead of the stubbed one. The index client is now mocked in that suite, with the gate left open so the cases still run on the shipping configuration, and the index-first path gets cases of its own here and in a new maps.pois suite: field mapping, the empty and the failing index both dropping through to Overpass, and an outage under both providers still reaching the caller as an error rather than an empty list. maps.service.test.ts drove pois() with no stub at all and leaked the same way. |
||
|
Maurice
|
fb363f22c8 |
fix(maps): alternate between the index and OpenStreetMap instead of ranking them
Measured on 128 places out of a real trip to Japan, all saved through the old search. Index alone: 51.6 percent inside the top five. Index and OpenStreetMap alternating: 72.7 percent. Two cleverer orderings were tried first and both lost. A score threshold cannot separate the cases at all: at 0.75 it catches 19 of 62 misses and throws away 6 of 66 hits. Promoting results whose name matches what was typed is worse than it sounds, because "Hase Station" shares a word with "Hase-dera" while OpenStreetMap answers in the local language and returns the right temple under a name that shares nothing: that rule put seven weak index matches ahead of the correct answer, which landed at position eight. Alternating puts it at position two. So the merge does not rank. Each source keeps its own ordering and they take turns, index first. An exact business match still opens the list; a landmark Overture does not carry is second rather than eleventh. |
||
|
Maurice
|
3caafe0548 |
fix(maps): show OpenStreetMap alongside the index, not instead of it
Measured against 128 places out of a real trip to Japan, all saved through the old search: the index answered 51.6 percent inside its top five. For most of the rest it returned the shops AROUND the landmark rather than nothing, which is worse than nothing, because ten plausible wrong answers look like an answer. Overture Places is a dataset of businesses and is very good at those. It largely does not carry temples, bridges, riverside walks, viewpoints or observation decks, which is a fair share of what somebody planning a trip searches for. Two ways to detect "the index does not have this one" were measured and both failed. Its own score does not separate the cases: a 0.75 threshold catches 19 of 62 misses and throws away 6 of 66 hits. Name overlap does not either: 57 false positives, because "Hase Station" shares a word with "Hase-dera". So this does not classify. Explicit search asks both concurrently and merges, name matches first and the index ahead of OpenStreetMap within each group. Autocomplete is untouched, because Nominatim's policy names it as unacceptable use. Nominatim also gets the location bias it never had, without which "Hase-dera" returns the temple in Nara rather than the one in Kamakura the user is standing beside. Two booking-import cases still stubbed searchNominatim after geocoding moved to geocodeQuery, and stubbed it with the old array shape, which made "has coordinates" pass on an undefined lat. |
||
|
Maurice
|
bac85cb98a |
feat(places): opening hours from the index, with OSM still first
The index can now carry opening hours, from two sources: an OpenStreetMap layer the service keeps as a separate database file, and the schema.org data a place publishes on its own site, read in the same page fetch the description comes from. OSM stays the first choice where it has them. Measured across seven countries it covers 27.5 percent of gastronomy, and an OSM entry describes this exact object and gets corrected by people who walked past it, where a chain's website often carries one set of hours for every branch. The rest is what the operator publishes, which measured 8.3 percent on the same kind of place. Both go through the same expansion into weekday lines, so a card reading `opening_hours` gets one shape and not two. Five maps.service cases went red when search started answering from the index ahead of Google; they are about the shape of the Google request, so they now switch the index off first. |
||
|
Maurice
|
87a765b763 |
feat(offline): cache the trip's area so place search works with no network
An offline instance had the trip's own places and nothing else, so "find a pharmacy near the hotel" had nothing to answer from: every lookup went to a server. The index can hand over a bounding box in one request, so it does, once, next to the map tiles. A trip to Rostock is roughly a megabyte, refreshed only when the trip's area actually moves. Search and autocomplete fall back to it at the API layer, which is why all ten call sites got the offline path without any of them learning about it, and the answer says `source: 'offline-cache'` rather than pretending to be live. Country packages stay what they were: a SQLite file for anyone who would rather run their own copy of the service. A browser cannot query one without shipping a WASM SQLite, and Germany alone would not fit in IndexedDB, so that door stays where it belongs. /v1/packages lists them, because "you can leave" only means something if the door is visible. |
||
|
Maurice
|
715993feaf |
feat(places): descriptions from the place's own website
Overture has no descriptions, and the encyclopaedias only describe the places that have an article, which is a fraction of a percent. What Overture does have is the address of the page the description is on, for 43 percent of places, and those pages publish a summary in JSON-LD or og:description precisely so machines can read it. The API fetches each page once and keeps the summary; every instance reads it from there. TREK never opens a business's website itself, which would mean one request per instance per place against small servers. In the ladder it sits below the encyclopaedias and above Google: an article about this exact place beats the operator's own copy, but for a restaurant or a shop the operator's summary is the only one that exists. Credited by host and linked back, with no licence claimed. |
||
|
Maurice
|
7f2a63e7f7 |
feat(maps): search places through the TREK Places API
Adds a provider for the project's own place index and puts it in front of the existing ones for search, autocomplete, details and the explore pills. It needs no key, has no quota, and what it returns may be stored, which is true of neither of the two providers it sits in front of. Nothing about the Google path changes. Every call site falls through to exactly what it did before when the index has no answer or the service is unreachable, so a working search keeps working. Two details worth knowing while reading: - Details for a place from the index also resolve its OpenStreetMap record by name and coordinate, because the index has no opening hours yet and never has cuisine or a menu link. Without that the hours a user saw while adding a place vanished from its card afterwards. - The explore pills keep reporting source 'openstreetmap'. It is a typed contract the client switches on, and the places are the same places. TREK_PLACES_URL points at a different instance; unset means the public one. |
||
|
Maurice
|
2d315df8bf |
fix(security): keep the operator's ntfy token in and re-check every redirect hop
Two reported advisories, and a third path neither of them covers. GHSA-7pqc-fj3c-9346 — the ntfy token fell back to the operator's whenever a request did not carry one, so an authenticated user who named their own server was handed the operator's decrypted token as `Authorization: Bearer`. The report covers `test-ntfy`, but the live send path in channels/builtins.ts does the same thing off a per-user `ntfy_server` setting: no test route needed, just wait for the next notification. Gating the route on `role === 'admin'` would have closed one of the two and broken the documented shared-server setup, where the operator supplies server plus token and users only set a topic. The invariant is the target, not the role: the operator's token travels to the operator's server and nowhere else, and that rule fits both paths unchanged. Scheme is part of the comparison — an http twin of an https server would put it on the wire in clear. GHSA-8mw6-xphx-886m — both transports checked only the first URL and then let the platform follow redirects on a dispatcher pinned to that first IP. Pinning does not help: Node skips the pinned lookup for an IP-literal host, so a 307 to 127.0.0.1 or 169.254.169.254 connected straight through. They go through safeFetchFollow now, which re-checks and re-pins per hop. safeFetch itself had the same shape and carried it to sixteen more callers, several with a URL out of a per-user setting, and one (pipeAsset) streams the response back — which would have made a redirect into an internal service readable rather than blind. It now routes through the same follower. The third: following by hand opts out of the platform's own rules, and both manual followers replayed the caller's init verbatim. undici drops `Authorization` across origins itself, so converting ntfy to safeFetchFollow would have re-opened the first advisory through a redirect. Credential headers are now dropped on an origin change (same-host http to https stays, self-hosted setups depend on it), and a 303 — or a 301/302 on a non-idempotent method — drops the body, so a plugin's client_secret is not re-POSTed at a host the first one merely pointed at. plugin-oauth pins maxRedirects to 0 like its OIDC twin, and the Google Places calls get the timeout none of their nine call sites passed. |
||
|
Maurice
|
5d0fac3026 |
feat(mcp): place pictures, POI search, GPX export and a biased place search
image_url was in no places tool although the service persists it and the picker in the UI sets a URL, which is not a file transport question. search_place passed two of the service's four arguments, so an unbiased query resolved to the wrong continent. get_place_details gained expand for reviews and the editorial summary. It defaults to off because that field mask bills as a Google Enterprise SKU, and it now honours the admin kill switch the REST route checks: an instance owner who turns Place Details off to stop the billing was still billable through this one surface. |
||
|
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
|
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
|
97ebe23d3b |
fix(server): bound every outbound fetch
The rule is a timeout, a size cap and boundary validation on anything that leaves the process, and several call sites had only the timeout. A link preview, a maps URL resolve, a wiki fallback and a holiday lookup all read the whole body into memory, so a caller who picks the URL picks how much memory the server spends - a few concurrent multi-hundred-megabyte responses is the whole heap. Two of them also fetched more than their name suggests. resolve-url fell through to a full page GET for any host once it found no coordinates, which makes the server a general-purpose outbound proxy wearing its own IP. The weather URLs took lat/lng straight from the caller into the query string. readCapped/exceedsDeclaredLength collect the pattern transit.service.ts and the plugin installer had each grown separately, so there is one of it now. Callers that only scan the head of a document (og: tags, a coordinate regex) keep the truncated bytes and find fewer fields; callers that must have the whole thing reject it. |
||
|
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
|
28765e220b |
fix(places): keep the Google feature id, and give list imports the free address lookup
Two halves of #1954, both about a place arriving through a list coming out thinner than the same place pasted as a single link. The feature id was never dropped on purpose. googleFtidFromMapsUrl only looked at the `?ftid=` query parameter, while a share link that resolveGoogleMapsUrl has followed to its final hop carries the id in the `data=!…!1s0x…:0x…` path blob — the same run the coordinates are already parsed out of two lines further down. So every pasted maps.app.goo.gl link lost it, and the Maps button opened a name search rather than the entry the link pointed at. The parser now reads the blob too, but only on a /place/ URL: a /maps/dir/ route carries one !1s per waypoint and the first is the origin, which would attach the wrong place. List imports never had a reverse lookup at all. The only pass behind them is the Google enrichment, which returns immediately without a Maps key, so an instance without one imported a list and left every address empty forever. They now get the same free Nominatim lookup the single-link path has always had: detached after the import answers, on the throttled background lane so it never queues in front of someone's search, through COALESCE so it can only fill an empty column, and each row pushed over the websocket so the sidebar fills in without a reload. Capped at 250 places — past that it stops being a backfill and starts being bulk geocoding. Not included: an osm_id for imported places. Single links do not produce one either, and one derived from a zoom-18 reverse lookup would name the building or the street rather than the POI. |
||
|
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 |
||
|
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
|
c9193e0621 |
fix(maps): answer a keyless place lookup with an empty result, not a 400
An instance without a Google key already runs on the OpenStreetMap stack: search falls back to Nominatim, autocomplete does too, photos fall back to Wikimedia. The two details lookups were the exception. Both threw a 400 with "Google Maps API key not configured" the moment they were handed a Google place id. A Google id has no OpenStreetMap equivalent, so without a key there really is nothing to resolve. That is an empty result, not a caller mistake. The callers already treat a null place as a miss and fall back to the text-search path, so they were doing the right thing with the wrong signal, at the cost of an error round-trip and a logged exception each time. It bites whenever a place that was saved while a key existed is opened after the key is gone, and on any instance that never had one but imported Google-sourced places from a shared trip. The declared return type said the place was non-nullable while the OSM branch could already return null and the wire contract has always had it nullable. It now says what it does. |
||
|
Maurice
|
f02a4b5654 |
feat(maps): bill a place search as one session instead of per keystroke
Google charges autocomplete per request unless the requests carry a session token. With one, the keystrokes of a single search and the details lookup that ends it are charged together, which is also how a user experiences it: they looked up one place. TREK sent no token at all, so every debounced keystroke was its own billable request. On a busy trip that is the largest line on the Places bill, and it is paid by whoever owns the key, which for a self-hosted instance is the person running it. A session opens with the first suggestion request and closes when the user picks a result. It has to close on the abandoned paths too: a token carried into the next search would merge two searches into one billed session and scope the suggestions wrongly. So the search box clearing it, the text-search fallback and the error paths all end it. The token is validated on the way in. The autocomplete body goes through the Zod pipe, the details query does not, so the controller shape-checks it there. Anything that does not match Google's URL-safe 36-character form is dropped rather than forwarded, which falls back to per-request billing instead of breaking a lookup. A cached details hit never reaches Google and so never closes its session. That is billing-neutral: an unclosed session is charged as a plain autocomplete session, which is still cheaper than the per-request behaviour it replaces. |
||
|
Maurice
|
a39c31cb48 |
Hide permanently closed places from search (#1887)
A place that has shut down for good is never the answer to "where should we go", so it should not be in the results at all. Google knows this and says so in businessStatus, we just never asked for the field. It goes into the field mask and CLOSED_PERMANENTLY drops out. CLOSED_TEMPORARILY stays - a restaurant on holiday next month is still worth planning around - and results without the field are parks and viewpoints rather than businesses, so they stay too. OSM states it a few different ways. Re-tagged objects (disused:amenity, abandoned:shop) never matched the selectors to begin with, but plenty keep their original tag and gain a marker instead, and those did come back. Same for opening_hours=closed/off. |
||
|
Maurice
|
3a7c8e1d26 |
Make a bind failure fatal again, and undo four contract tightenings
An adversarial audit over the three merged migration PRs. Sixteen findings
survived triple refutation; these are the ones with a defect behind them.
The serious one is mine, from the gateway move. ws registers
`error: this.emit.bind(this, 'error')` on the http server it attaches to, and
the adapter now runs inside buildApp, before listen(). So from that moment the
http server HAS an error listener and Node stops throwing on one. The adapter's
handler was `() => {}`, which meant EADDRINUSE was forwarded there and dropped:
the process survived, the listen callback never ran, so the banner never
printed and the scheduler never started. A container that looks healthy and
serves nothing. Reproduced, then fixed at both ends: index.ts owns the fatal
decision on the http server, and the adapter logs instead of swallowing.
Four DTOs were tightening contracts rather than describing them, which is the
thing the pipe-before-handler ordering makes easy to do by accident:
- OauthClientCreateDto replaced 'Name is required' with a Zod field report, and
pre-empted the handler's 403 on an instance with the MCP addon off.
- BookingImportConfirmDto demanded per-item fields the service never reads, so
a caller assembling items itself got a 400 about `source`.
- TagCreateDto/TagUpdateDto typed name and color as strings; the legacy route
bound whatever arrived, so a numeric name used to create a tag.
- PluginUserSettingsUpdateDto typed config as a record, where the handler
answers 200 for anything it cannot use.
Two of my own tests were false-green, which the audit demonstrated by mutation
rather than by reading. WSGW-008 asserted on frames that never carry the user
object at all, so it passed with the password_version strip deleted; it reads
the retained identity now. GEO-009 made the same assertion before and after the
write; it distinguishes a cached negative from a miss now, through a hasCached
seam that says what getCached cannot. Both were re-run against the mutation
they exist to catch, and both fail on it.
booking-import geocodes every venue and endpoint in one request loop, up to
thirty sequential calls. Folding maps onto the shared Nominatim cursor put that
loop on the interactive lane, so an import starved every concurrent keystroke
for half a minute. It takes the background lane now, which is what the two-lane
split was built for.
Also: the README's in-container bridge list had outlived one entry and was
missing another, and the journey e2e still mocked a module this branch deleted.
|