Merge branch 'origin/dev' into feat/storage-driver

Both sides appended a migration to the same schema_version slot: dev's
reservation_day_positions cross-trip cleanup keeps its published index,
the storage slice-2 trip_files prefix strip moves after it. The trailing-
migration guard now rewinds two versions and asserts both. Dev's new
places image_url/website tests gain the StorageService constructor arg
and async update() handling from the storage slices.
This commit is contained in:
jubnl
2026-08-17 08:00:41 +02:00
48 changed files with 938 additions and 124 deletions
@@ -550,4 +550,30 @@ describe('JourneyMap', () => {
expect(colors).not.toContain('#ff0000');
expect(colors).not.toContain('#ffffff');
});
// A string handed to bindTooltip becomes innerHTML. A track name is a place
// name off a shared trip and an entry label is a journey title, so both are
// written by someone who is not necessarily the person reading the map — and
// this component is what the public journey page renders.
it('FE-COMP-JOURNEYMAP-044: escapes a track name before it becomes tooltip markup', () => {
vi.mocked(L.polyline).mockClear();
render(<JourneyMap checkins={[]} entries={entriesWithCoords} tracks={[track({ name: '<img src=x onerror="alert(1)">' })]} />);
const line = vi.mocked(L.polyline).mock.results
.map(r => r.value as any)
.find(v => v.bindTooltip.mock.calls.length > 0);
const [label] = line.bindTooltip.mock.calls[0];
expect(label).not.toContain('<img');
expect(label).not.toContain('onerror="');
expect(label).toBe('&lt;img src=x onerror=&quot;alert(1)&quot;&gt;');
});
it('FE-COMP-JOURNEYMAP-045: escapes an entry title before it becomes tooltip markup', () => {
const hostile = [{ id: 'e9', lat: 48.1, lng: 2.1, title: '<img src=x onerror="alert(1)">', mood: null, entry_date: '2025-06-04' }];
render(<JourneyMap checkins={[]} entries={hostile} />);
const labels = vi.mocked(mockedMarker().bindTooltip).mock.calls.map(c => c[0]);
expect(labels.some(l => typeof l === 'string' && l.includes('<img'))).toBe(false);
expect(labels).toContain('&lt;img src=x onerror=&quot;alert(1)&quot;&gt;');
});
});
+7 -4
View File
@@ -1,7 +1,7 @@
import { useEffect, useRef, useImperativeHandle, useCallback, type Ref } from 'react'
import L from 'leaflet'
import { useSettingsStore } from '../../store/settingsStore'
import type { JourneyTrack } from '@trek/shared'
import { escapeHtml, type JourneyTrack } from '@trek/shared'
export interface MapMarkerItem {
id: string
@@ -212,8 +212,9 @@ function JourneyMap(
const line = L.polyline(coords, { color, weight: 3.5, opacity: 0.95, lineCap: 'round', lineJoin: 'round' })
// Same tooltip the markers on this map use, rather than Leaflet's default box:
// it follows the appearance tokens, so it lands right in dark mode and with
// transparency switched off.
if (track.name) line.bindTooltip(track.name, { sticky: true, direction: 'top', className: 'map-tooltip' })
// transparency switched off. Escaped because a string handed to bindTooltip
// becomes innerHTML, and a track name is a place name off a shared trip.
if (track.name) line.bindTooltip(escapeHtml(track.name), { sticky: true, direction: 'top', className: 'map-tooltip' })
line.addTo(map)
coords.forEach(c => allCoords.push(c))
}
@@ -243,7 +244,9 @@ function JourneyMap(
})
const marker = L.marker(pos, { icon }).addTo(map)
marker.bindTooltip(item.label, {
// Escaped for the same reason as the track tooltip above: the label is an
// entry title, and this map is what the public journey page renders.
marker.bindTooltip(escapeHtml(item.label), {
direction: 'top',
offset: [0, -MARKER_H],
className: 'map-tooltip',
@@ -851,6 +851,23 @@ describe('MapViewGL', () => {
expect(glMarkers.created[1].lngLat).toEqual([2.12, 48.12])
})
// The Leaflet twin of this is FE-COMP-MAPVIEW-074. Both renderers build their
// marker as an HTML string, so both need the escape pinned — otherwise it can
// be dropped from one of them without a single test going red.
it('FE-COMP-MAPVIEWGL-023b: an image_url that passes the prefix check is still escaped', async () => {
loadOnAttach()
render(<MapViewGL places={[buildMapPlace({
id: 13, lat: 48.13, lng: 2.13, image_url: '/uploads/x" onerror="alert(1)" y="',
})]} fitKey={1} />)
await act(async () => {})
const marker = glMarkers.created[0].element
expect(marker.innerHTML).not.toContain('onerror="alert(1)"')
expect(marker.innerHTML).toContain('&quot;')
// The escape must not break the ordinary case: the src still resolves.
expect(marker.querySelector('img')?.getAttribute('src')).toBe('/uploads/x" onerror="alert(1)" y="')
})
it('FE-COMP-MAPVIEWGL-024: the hover card shows name, category and address and follows the cursor', async () => {
loadOnAttach()
const places = [buildMapPlace({
@@ -3332,7 +3332,7 @@ describe('DayPlanSidebar', () => {
render(<DayPlanSidebar {...makeDefaultProps({ days: [day], places: [place], assignments: { '10': [a] } })} />)
fireEvent.contextMenu(dragRow(screen.getByText('Louvre')))
await user.click(screen.getByRole('button', { name: 'Open Website' }))
expect(openSpy).toHaveBeenCalledWith('https://louvre.fr', '_blank')
expect(openSpy).toHaveBeenCalledWith('https://louvre.fr', '_blank', 'noopener,noreferrer')
fireEvent.contextMenu(dragRow(screen.getByText('Louvre')))
await user.click(screen.getByRole('button', { name: 'Google Maps' }))
@@ -4,6 +4,7 @@ declare global { interface Window { __dragData: DragDataPayload | null } }
import React, { useState, useEffect, useLayoutEffect, useRef, useMemo, useCallback } from 'react'
import { avatarSrc } from '../../utils/avatarSrc'
import { safeHttpUrl } from '../../utils/safeUrl'
import { ChevronDown, ChevronRight, ChevronUp, Compass, Navigation, RotateCcw, ExternalLink, Clock, Pencil, GripVertical, Ticket, Plus, FileText, Trash2, Car, Lock, Hotel, Footprints, Route as RouteIcon, Bookmark, TramFront, Zap } from 'lucide-react'
import { type PickedPlace } from './TransitSearchPanel'
import { assignmentsApi, reservationsApi, daysApi } from '../../api/client'
@@ -1871,7 +1872,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP
ctxMenu.open(e, [
canEditDays && onEditPlace && { label: t('common.edit'), icon: Pencil, onClick: () => onEditPlace(place, assignment.id) },
canEditDays && onRemoveAssignment && { label: t('planner.removeFromDay'), icon: Trash2, onClick: () => onRemoveAssignment(day.id, assignment.id) },
place.website && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(place.website, '_blank') },
safeHttpUrl(place.website) && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(safeHttpUrl(place.website)!, '_blank', 'noopener,noreferrer') },
...navTargets.map(target => ({ label: target.label, icon: Navigation, onClick: () => openNavigationTarget(target) })),
collectionsEnabled && { label: t('inspector.saveToCollection'), icon: Bookmark, onClick: () => useSaveToCollectionStore.getState().open(placeToSaveTarget(place)) },
{ divider: true },
@@ -953,7 +953,7 @@ describe('PlaceInspector', () => {
fireEvent.click(screen.getByText('Open Website').closest('button')!);
expect(open).toHaveBeenCalledTimes(2);
expect(open.mock.calls[1]).toEqual(['https://example.org', '_blank']);
expect(open.mock.calls[1]).toEqual(['https://example.org', '_blank', 'noopener,noreferrer']);
vi.unstubAllGlobals();
});
@@ -1,5 +1,6 @@
import React, { useState, useEffect, useRef, useCallback, useMemo } from 'react'
import { avatarSrc } from '../../utils/avatarSrc'
import { safeHttpUrl } from '../../utils/safeUrl'
import { openFile } from '../../utils/fileDownload'
import Markdown from 'react-markdown'
import remarkGfm from 'remark-gfm'
@@ -318,6 +319,10 @@ export default function PlaceInspector({
detailLng,
googleDetails?.open_now,
)
// Allow-listed rather than passed straight through: window.open runs a
// javascript: URL in this origin, and the stored value predates the check the
// server does on the way in now.
const websiteUrl = safeHttpUrl(place.website) ?? safeHttpUrl(googleDetails?.website)
// Prefer the place's stored ftid; if it has none yet, use the one just fetched from Google.
const navigationTargets = getNavigationTargets(
place ? { ...place, google_ftid: place.google_ftid || googleDetails?.google_ftid || null } : null,
@@ -513,8 +518,8 @@ export default function PlaceInspector({
)}
</>
)}
{(place.website || googleDetails?.website) && (
<ActionButton onClick={() => window.open(place.website || googleDetails?.website, '_blank')} variant="ghost" icon={<ExternalLink size={13} />}
{websiteUrl && (
<ActionButton onClick={() => window.open(websiteUrl, '_blank', 'noopener,noreferrer')} variant="ghost" icon={<ExternalLink size={13} />}
label={<span className="hidden sm:inline">{t('inspector.website')}</span>} />
)}
<div style={{ flex: 1 }} />
@@ -643,7 +643,7 @@ describe('usePlacesSidebar context menu', () => {
fireEvent.contextMenu(screen.getByTestId('row-3'));
await user.click(screen.getByRole('button', { name: 'Open Website' }));
expect(open).toHaveBeenCalledWith('https://cafe.example', '_blank');
expect(open).toHaveBeenCalledWith('https://cafe.example', '_blank', 'noopener,noreferrer');
fireEvent.contextMenu(screen.getByTestId('row-3'));
await user.click(screen.getByRole('button', { name: 'Google Maps' }));
@@ -14,6 +14,7 @@ import { useSaveToCollectionStore } from '../../store/saveToCollectionStore'
import { placeToSaveTarget } from '../Collections/saveTarget'
import type { Place, Category, Day, AssignmentsMap } from '../../types'
import { getGoogleMapsUrlForPlace } from './placeGoogleMaps'
import { safeHttpUrl } from '../../utils/safeUrl'
export interface PlacesSidebarProps {
tripId: number
@@ -276,7 +277,7 @@ export function usePlacesSidebar(props: PlacesSidebarProps) {
ctxMenu.open(e, [
canEditPlaces && { label: t('common.edit'), icon: Pencil, onClick: () => props.onEditPlace(place) },
selDayId && { label: t('planner.addToDay'), icon: CalendarDays, onClick: () => props.onAssignToDay(place.id, selDayId) },
place.website && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(place.website, '_blank') },
safeHttpUrl(place.website) && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(safeHttpUrl(place.website)!, '_blank', 'noopener,noreferrer') },
googleMapsUrl && { label: t('inspector.google'), icon: Navigation, onClick: () => window.open(googleMapsUrl, '_blank') },
collectionsEnabled && { label: t('inspector.saveToCollection'), icon: Bookmark, onClick: () => useSaveToCollectionStore.getState().open(placeToSaveTarget(place)) },
{ divider: true },
@@ -17,6 +17,7 @@ import PlaceRating from '../../../../components/shared/StarRating'
import TrackColorPicker from '../../../../components/shared/TrackColorPicker'
import { resolveTrackColor, inheritedTrackColor } from '../../../../components/Map/trackColors'
import { avatarSrc } from '../../../../utils/avatarSrc'
import { safeHttpUrl } from '../../../../utils/safeUrl'
import { openFile } from '../../../../utils/fileDownload'
import { getNavigationTargets, openNavigationTarget } from '../../../../components/Planner/placeNavigation'
import { NavigationMenu } from '../../../../components/shared/NavigationMenu'
@@ -527,9 +528,9 @@ export default function MPlaceSheet({ planner, shell }: MTripSheetsProps) {
<MapIcon size={15} strokeWidth={2} />
</ActionCircle>
)}
{place.website && (
{safeHttpUrl(place.website) && (
<ActionCircle
onClick={() => window.open(place.website!, '_blank', 'noopener,noreferrer')}
onClick={() => window.open(safeHttpUrl(place.website)!, '_blank', 'noopener,noreferrer')}
label={t('inspector.website')}
>
<ExternalLink size={15} strokeWidth={2} />
+55
View File
@@ -1000,4 +1000,59 @@ describe('DashboardPage', () => {
expect(within(grid()).queryByText('Paris Adventure')).not.toBeInTheDocument();
});
});
// Two people reported in a row that they could not find how to unarchive or
// rename a trip. The controls sat at opacity 0 until the card was hovered, and
// the archive button carried the same icon whether it would archive or restore.
describe('FE-PAGE-DASH-036: the cover controls are findable', () => {
const activeOnly = (trips: unknown[]) =>
server.use(
http.get('/api/trips', ({ request }) => {
const url = new URL(request.url);
if (url.searchParams.get('archived')) return HttpResponse.json({ trips: [] });
return HttpResponse.json({ trips });
}),
);
it('names every action on the card, so none of them is icon-only guesswork', async () => {
activeOnly([buildTrip({ title: 'Lisbon 2025', start_date: '2025-05-01', end_date: '2025-05-08' })]);
const user = userEvent.setup();
render(<DashboardPage />);
await waitFor(() => expect(screen.getAllByText('Lisbon 2025').length).toBeGreaterThan(0));
await user.click(screen.getByText('Completed'));
const card = (await screen.findAllByText('Lisbon 2025'))
.map(n => n.closest('.trip-card'))
.find(Boolean) as HTMLElement;
for (const label of ['Edit', 'Duplicate', 'Archive', 'Delete']) {
const button = card.querySelector(`[aria-label="${label}"]`) as HTMLElement;
expect(button).toBeTruthy();
// A tooltip as well as the screen-reader label — the icons alone did not read.
expect(button.getAttribute('title')).toBe(label);
}
});
it('swaps the archive icon for a restore one once the trip is archived', async () => {
const archived = buildTrip({ title: 'Old Rome Trip', start_date: '2024-01-01', end_date: '2024-01-07', is_archived: 1 });
server.use(
http.get('/api/trips', ({ request }) => {
const url = new URL(request.url);
return HttpResponse.json({ trips: url.searchParams.get('archived') ? [archived] : [] });
}),
);
const user = userEvent.setup();
render(<DashboardPage />);
await user.click(screen.getByText('Archived'));
await waitFor(() => expect(screen.getByText('Old Rome Trip')).toBeInTheDocument());
const card = screen.getByText('Old Rome Trip').closest('.trip-card') as HTMLElement;
const restore = card.querySelector('[aria-label="Restore"]') as HTMLElement;
expect(restore).toBeTruthy();
// lucide stamps the icon name on the svg, which is how the archive and the
// restore variant tell themselves apart.
expect(restore.querySelector('svg')?.getAttribute('class')).toContain('archive-restore');
expect(card.querySelector('[aria-label="Archive"]')).toBeNull();
});
});
});
+11 -9
View File
@@ -15,7 +15,7 @@ import {
MS_PER_DAY, daysUntil, getTripStatus,
} from './dashboard/dashboardModel'
import {
Plus, Edit2, Trash2, Archive, Copy, ArrowRight, MapPin,
Plus, Edit2, Trash2, Archive, ArchiveRestore, Copy, ArrowRight, MapPin,
Plane, Hotel, Utensils, Clock, RefreshCw, ArrowRightLeft, Calendar,
LayoutGrid, List, Ticket, X, CalendarPlus, ParkingSquare,
} from 'lucide-react'
@@ -304,6 +304,7 @@ function BoardingPassHero({ trip, bundle, locale, onOpen, onEdit, onCopy, onArch
const heroPlugins = usePluginStore(s => s.plugins).filter(p => p.type === 'widget' && p.slot === 'hero')
const stop = (e: React.MouseEvent, fn: () => void) => { e.stopPropagation(); fn() }
const status = getTripStatus(trip)
const archiveLabel = trip.is_archived ? t('dashboard.restore') : t('dashboard.archive')
const start = splitDate(trip.start_date, locale)
const end = splitDate(trip.end_date, locale)
@@ -404,10 +405,10 @@ function BoardingPassHero({ trip, bundle, locale, onOpen, onEdit, onCopy, onArch
{badge}
</div>
<div className="hero-tools">
<button className="hero-tool" aria-label={t('common.edit')} onClick={(e) => stop(e, onEdit)}><Edit2 size={16} /></button>
<button className="hero-tool" aria-label={t('dashboard.aria.duplicate')} onClick={(e) => stop(e, onCopy)}><Copy size={16} /></button>
<button className="hero-tool" aria-label={trip.is_archived ? t('dashboard.restore') : t('dashboard.archive')} onClick={(e) => stop(e, onArchive)}><Archive size={16} /></button>
<button className="hero-tool" aria-label={t('common.delete')} onClick={(e) => stop(e, onDelete)}><Trash2 size={16} /></button>
<button className="hero-tool" aria-label={t('common.edit')} title={t('common.edit')} onClick={(e) => stop(e, onEdit)}><Edit2 size={17} strokeWidth={2.2} /></button>
<button className="hero-tool" aria-label={t('dashboard.aria.duplicate')} title={t('dashboard.aria.duplicate')} onClick={(e) => stop(e, onCopy)}><Copy size={17} strokeWidth={2.2} /></button>
<button className="hero-tool" aria-label={archiveLabel} title={archiveLabel} onClick={(e) => stop(e, onArchive)}>{trip.is_archived ? <ArchiveRestore size={17} strokeWidth={2.2} /> : <Archive size={17} strokeWidth={2.2} />}</button>
<button className="hero-tool" aria-label={t('common.delete')} title={t('common.delete')} onClick={(e) => stop(e, onDelete)}><Trash2 size={17} strokeWidth={2.2} /></button>
</div>
</div>
@@ -535,6 +536,7 @@ function TripCard({ trip, locale, badges, onOpen, onEdit, onCopy, onArchive, onD
}): React.ReactElement {
const { t } = useTranslation()
const status = getTripStatus(trip)
const archiveLabel = trip.is_archived ? t('dashboard.restore') : t('dashboard.archive')
const start = splitDate(trip.start_date, locale)
const end = splitDate(trip.end_date, locale)
const until = daysUntil(trip.start_date)
@@ -557,10 +559,10 @@ function TripCard({ trip, locale, badges, onOpen, onEdit, onCopy, onArchive, onD
: <div style={{ width: '100%', height: '100%', background: tripGradient(trip.id) }} />}
<div className={`trip-status ${statusClass}`}><span className="indicator" /> {statusLabel}</div>
<div className="trip-actions">
<button className="trip-action-btn" aria-label={t('common.edit')} onClick={(e) => stop(e, onEdit)}><Edit2 size={16} /></button>
<button className="trip-action-btn" aria-label={t('dashboard.aria.duplicate')} onClick={(e) => stop(e, onCopy)}><Copy size={16} /></button>
<button className="trip-action-btn" aria-label={trip.is_archived ? t('dashboard.restore') : t('dashboard.archive')} onClick={(e) => stop(e, onArchive)}><Archive size={16} /></button>
<button className="trip-action-btn" aria-label={t('common.delete')} onClick={(e) => stop(e, onDelete)}><Trash2 size={16} /></button>
<button className="trip-action-btn" aria-label={t('common.edit')} title={t('common.edit')} onClick={(e) => stop(e, onEdit)}><Edit2 size={17} strokeWidth={2.2} /></button>
<button className="trip-action-btn" aria-label={t('dashboard.aria.duplicate')} title={t('dashboard.aria.duplicate')} onClick={(e) => stop(e, onCopy)}><Copy size={17} strokeWidth={2.2} /></button>
<button className="trip-action-btn" aria-label={archiveLabel} title={archiveLabel} onClick={(e) => stop(e, onArchive)}>{trip.is_archived ? <ArchiveRestore size={17} strokeWidth={2.2} /> : <Archive size={17} strokeWidth={2.2} />}</button>
<button className="trip-action-btn" aria-label={t('common.delete')} title={t('common.delete')} onClick={(e) => stop(e, onDelete)}><Trash2 size={17} strokeWidth={2.2} /></button>
</div>
<div className="trip-cover-content">
<h3 className="trip-name">{trip.title}</h3>
+2 -2
View File
@@ -6,7 +6,7 @@ import apiClient, { mapsApi, pluginsApi, type PluginAtlasLayer } from '../../api
import L from 'leaflet'
import type { GeoJsonFeatureCollection } from '../../types'
import { A2_TO_A3, countryStatus, findBucketDuplicate, isBucketDuplicateError, isCountryVisible, normalizeRegionName, withCountryMarkedVisited, wishlistA3Codes, countryColor, type AtlasData, type AtlasPlaceHit, type CountryDetail, type BucketItem } from './atlasModel'
import { continentForCountry, type VisitStatus } from '@trek/shared'
import { continentForCountry, escapeHtml, type VisitStatus } from '@trek/shared'
import { useToast } from '../../components/shared/Toast'
import { getApiErrorMessage } from '../../types'
@@ -941,7 +941,7 @@ export function useAtlas() {
iconAnchor: [14, 14],
})
return L.marker([b.lat!, b.lng!], { icon }).bindTooltip(
`<div style="font-size:12px;font-weight:600">${b.name}</div>${b.notes ? `<div style="font-size:10px;opacity:0.7;margin-top:2px">${b.notes}</div>` : ''}`,
`<div style="font-size:12px;font-weight:600">${escapeHtml(b.name)}</div>${b.notes ? `<div style="font-size:10px;opacity:0.7;margin-top:2px">${escapeHtml(b.notes)}</div>` : ''}`,
{ className: 'atlas-tooltip', direction: 'top', offset: [0, -14] }
)
})
+31 -17
View File
@@ -191,11 +191,12 @@
.trek-dash .hero-top { display: flex; justify-content: space-between; align-items: start; }
.trek-dash .hero-badge {
display: inline-flex; align-items: center; gap: 8px;
background: oklch(1 0 0 / .14);
background: oklch(0.22 0.01 260 / .5);
backdrop-filter: blur(14px) saturate(1.4); -webkit-backdrop-filter: blur(14px) saturate(1.4);
border: 1px solid oklch(1 0 0 / .2); padding: 8px 14px 8px 12px;
border: 1px solid oklch(1 0 0 / .32); padding: 8px 14px 8px 12px;
border-radius: 999px; font-size: 12px; font-weight: 500;
letter-spacing: 0.06em; text-transform: uppercase;
box-shadow: 0 2px 8px oklch(0 0 0 / .26);
}
.trek-dash .hero-badge .pulse {
width: 7px; height: 7px; border-radius: 50%;
@@ -208,15 +209,18 @@
100% { box-shadow: 0 0 0 0 oklch(0.84 0.18 85 / 0); }
}
.trek-dash .hero-tools { display: flex; gap: 8px; }
/* Same story as .trip-action-btn: a dark scrim instead of a white .14 fill, so
the tools read against a bright hero photo too. */
.trek-dash .hero-tool {
width: 38px; height: 38px; border-radius: 50%;
background: oklch(1 0 0 / .14);
width: 40px; height: 40px; border-radius: 50%;
background: oklch(0.22 0.01 260 / .5);
backdrop-filter: blur(14px) saturate(1.4); -webkit-backdrop-filter: blur(14px) saturate(1.4);
border: 1px solid oklch(1 0 0 / .2); color: #fff;
border: 1px solid oklch(1 0 0 / .32); color: #fff;
box-shadow: 0 2px 8px oklch(0 0 0 / .26);
display: grid; place-items: center; transition: background .15s, transform .12s;
}
.trek-dash .hero-tool:hover { background: oklch(1 0 0 / .26); transform: scale(1.05); }
.trek-dash .hero-tool svg { width: 16px; height: 16px; }
.trek-dash .hero-tool:hover { background: oklch(0.22 0.01 260 / .78); transform: scale(1.05); }
.trek-dash .hero-tool svg { width: 17px; height: 17px; }
.trek-dash .hero-title-block { align-self: end; padding-bottom: 24px; }
.trek-dash .hero-eyebrow {
display: inline-flex; align-items: center; gap: 10px;
@@ -435,26 +439,36 @@
.trek-dash .trip-cover img { width: 100%; height: 100%; object-fit: cover; transition: transform .6s cubic-bezier(.2,.7,.2,1); }
.trek-dash .trip-card:hover .trip-cover img { transform: scale(1.04); }
.trek-dash .trip-cover::after { content: ""; position: absolute; inset: 0; background: linear-gradient(180deg, oklch(0 0 0 / 0) 40%, oklch(0 0 0 / .55) 100%); }
/* Shares the dark scrim of .trip-action-btn so both overlays on the cover read
as one set, and the label survives a bright photo behind it. */
.trek-dash .trip-status {
position: absolute; top: 16px; left: 16px; z-index: 1;
display: inline-flex; align-items: center; gap: 7px; padding: 6px 11px 6px 9px;
border-radius: 999px; font-size: 11.5px; font-weight: 500; letter-spacing: 0.02em;
background: oklch(1 0 0 / .18); backdrop-filter: blur(14px) saturate(1.4); -webkit-backdrop-filter: blur(14px) saturate(1.4);
border: 1px solid oklch(1 0 0 / .22); color: #fff; text-transform: uppercase;
background: oklch(0.22 0.01 260 / .58); backdrop-filter: blur(14px) saturate(1.4); -webkit-backdrop-filter: blur(14px) saturate(1.4);
border: 1px solid oklch(1 0 0 / .34); color: #fff; text-transform: uppercase;
box-shadow: 0 2px 8px oklch(0 0 0 / .28);
}
.trek-dash .trip-status .indicator { width: 6px; height: 6px; border-radius: 50%; background: oklch(0.84 0.16 145); }
.trek-dash .trip-status.completed .indicator { background: oklch(0.75 0.02 220); }
.trek-dash .trip-status.upcoming .indicator { background: oklch(0.84 0.18 85); }
.trek-dash .trip-status.idea .indicator { background: oklch(0.78 0.14 280); }
.trek-dash .trip-actions { position: absolute; top: 14px; right: 14px; z-index: 1; display: flex; gap: 6px; opacity: 0; transition: opacity .2s; }
/* The actions used to sit at opacity 0 until the card was hovered, over a white
.18 glass fill. Two people in a row reported they never found the archive and
rename buttons — invisible at rest, and on a light cover the white-on-white
fill hid them even while hovering. They stay on the card now, and the fill is
a dark scrim so the white icons carry against any photo. */
.trek-dash .trip-actions { position: absolute; top: 14px; right: 14px; z-index: 1; display: flex; gap: 6px; opacity: .92; transition: opacity .2s; }
.trek-dash .trip-card:hover .trip-actions { opacity: 1; }
.trek-dash .trip-action-btn {
width: 36px; height: 36px; border-radius: 50%;
background: oklch(1 0 0 / .18); backdrop-filter: blur(14px); -webkit-backdrop-filter: blur(14px);
border: 1px solid oklch(1 0 0 / .22); color: #fff; display: grid; place-items: center; transition: background .15s;
width: 38px; height: 38px; border-radius: 50%;
background: oklch(0.22 0.01 260 / .58); backdrop-filter: blur(14px); -webkit-backdrop-filter: blur(14px);
border: 1px solid oklch(1 0 0 / .34); color: #fff; display: grid; place-items: center;
box-shadow: 0 2px 8px oklch(0 0 0 / .28);
transition: background .15s, transform .12s;
}
.trek-dash .trip-action-btn:hover { background: oklch(1 0 0 / .3); }
.trek-dash .trip-action-btn svg { width: 16px; height: 16px; }
.trek-dash .trip-action-btn:hover { background: oklch(0.22 0.01 260 / .82); transform: scale(1.06); }
.trek-dash .trip-action-btn svg { width: 17px; height: 17px; }
.trek-dash .trip-cover-content { position: absolute; left: 18px; right: 18px; bottom: 16px; z-index: 1; color: #fff; }
.trek-dash .trip-name { font-size: 26px; font-weight: 600; letter-spacing: -0.025em; line-height: 1.05; margin: 0; text-shadow: 0 1px 7px oklch(0 0 0 / .3), 0 1px 2px oklch(0 0 0 / .38); }
.trek-dash .trip-where { margin-top: 4px; font-size: 13px; opacity: .85; display: flex; align-items: center; gap: 6px; }
@@ -686,8 +700,8 @@
.trek-dash .trips { grid-template-columns: 1fr; gap: 16px; margin-bottom: 28px; }
.trek-dash .add-trip-card { min-height: 180px; }
/* Touch devices have no hover — keep the edit/copy/archive/delete actions
visible at all times instead of revealing them on hover. */
/* Touch devices have no hover state to lift the actions to full strength, so
skip the resting dim entirely. */
.trek-dash .trip-actions { opacity: 1; }
/* Compact list row on mobile — keeps the list view distinct from the grid. The
+35
View File
@@ -0,0 +1,35 @@
import { describe, it, expect } from 'vitest'
import { safeHttpUrl } from './safeUrl'
describe('safeHttpUrl', () => {
it('FE-UTIL-SAFEURL-001: passes an ordinary https homepage through', () => {
expect(safeHttpUrl('https://louvre.fr/en/visit')).toBe('https://louvre.fr/en/visit')
})
it('FE-UTIL-SAFEURL-002: passes plain http through — plenty of small venues still run it', () => {
expect(safeHttpUrl('http://pension-alpenblick.at')).toBe('http://pension-alpenblick.at')
})
it('FE-UTIL-SAFEURL-003: rejects javascript:, which window.open would run in this origin', () => {
expect(safeHttpUrl('javascript:fetch("/api/trips")')).toBeNull()
expect(safeHttpUrl('JavaScript:alert(1)')).toBeNull()
expect(safeHttpUrl(' javascript:alert(1)')).toBeNull()
})
it('FE-UTIL-SAFEURL-004: rejects the other schemes that are not a web page', () => {
expect(safeHttpUrl('data:text/html,<script>alert(1)</script>')).toBeNull()
expect(safeHttpUrl('vbscript:msgbox(1)')).toBeNull()
expect(safeHttpUrl('file:///etc/passwd')).toBeNull()
})
it('FE-UTIL-SAFEURL-005: rejects a bare host, since window.open would treat it as a relative path', () => {
expect(safeHttpUrl('louvre.fr')).toBeNull()
expect(safeHttpUrl('//evil.example')).toBeNull()
})
it('FE-UTIL-SAFEURL-006: treats absent and empty as nothing to open', () => {
expect(safeHttpUrl(null)).toBeNull()
expect(safeHttpUrl(undefined)).toBeNull()
expect(safeHttpUrl('')).toBeNull()
})
})
+18
View File
@@ -0,0 +1,18 @@
import { placeWebsiteSchema } from '@trek/shared'
/**
* Allow-list a URL before handing it to window.open.
*
* A place's website is a plain string in the older contracts, and it reaches
* window.open from the inspector, both sidebar context menus and the mobile
* place sheet. `window.open('javascript:…')` does not open a page — it evaluates
* the script in a fresh document that inherits this origin, so a co-traveller
* with place_edit could plant one and wait for someone to click Website.
*
* Server-side validation closes the write path. This closes the read path, which
* is what covers rows written before that validation existed and instances that
* have not updated yet — the same split as safeHexColor.
*/
export function safeHttpUrl(value: string | null | undefined): string | null {
return typeof value === 'string' && placeWebsiteSchema.safeParse(value).success ? value : null
}
+18
View File
@@ -3891,6 +3891,24 @@ function runMigrations(db: Database.Database): void {
db.exec('ALTER TABLE budget_items ADD COLUMN place_id INTEGER REFERENCES places(id) ON DELETE SET NULL DEFAULT NULL');
}
},
// A reservation_day_positions row only makes sense when its reservation and
// its day are on the same trip. The table carries no trip_id and its two
// foreign keys only ask that the ids exist, so pairs that never belonged
// together could accumulate; the writer refuses them now, and this clears
// whatever an older build let through. This slot keeps the index it shipped
// with on dev — the array is index-addressed against schema_version.
() => {
db.exec(`
DELETE FROM reservation_day_positions
WHERE rowid IN (
SELECT rdp.rowid
FROM reservation_day_positions rdp
JOIN reservations r ON r.id = rdp.reservation_id
JOIN days d ON d.id = rdp.day_id
WHERE d.trip_id <> r.trip_id
)
`);
},
// Storage slice 2 — collab note attachments historically stored 'files/<name>'
// in trip_files.filename while the file manager stored bare names in the same
// column; the storage layer addresses objects as category + bare name, so
@@ -113,9 +113,14 @@ export class DayAssignmentsController {
/**
* /api/trips/:tripId/assignments/:id/* — per-assignment ops (move, time,
* participants), independent of the day path. Same parity rules as above.
*
* Same guard pair as the day controller, and for the same reason: TripAccessGuard
* is what resolves :tripId and what reads @RequirePermission. The per-handler
* getAssignmentForTrip calls answer a different question — whether the assignment
* sits on the trip in the URL, not whether the caller may be on that trip.
*/
@Controller('api/trips/:tripId/assignments')
@UseGuards(JwtAuthGuard)
@UseGuards(JwtAuthGuard, TripAccessGuard)
export class AssignmentOpsController {
constructor(private readonly assignments: AssignmentsService) {}
@@ -142,6 +147,9 @@ export class AssignmentOpsController {
@Get(':id/participants')
participants(@CurrentUser() user: User, @Param('tripId') tripId: string, @Param('id') id: string) {
if (!this.assignments.getAssignmentForTrip(id, tripId)) {
throw new HttpException({ error: 'Assignment not found' }, 404);
}
return { participants: this.assignments.getParticipants(id) };
}
@@ -191,7 +199,10 @@ export class AssignmentOpsController {
@Body() body: AssignmentParticipantsDto,
@Headers('x-socket-id') socketId?: string,
) {
const participants = this.assignments.setParticipants(id, body.user_ids);
if (!this.assignments.getAssignmentForTrip(id, tripId)) {
throw new HttpException({ error: 'Assignment not found' }, 404);
}
const participants = this.assignments.setParticipants(id, body.user_ids, tripId);
this.assignments.broadcast(tripId, 'assignment:participants', { assignmentId: Number(id), participants }, socketId);
return { participants };
}
@@ -187,7 +187,7 @@ export class AssignmentsMcp {
if (!this.assignments.verifyTripAccess(tripId, ctx.userId)) return noAccess();
if (!this.guards.hasTripPermission('day_edit', tripId, ctx.userId)) return permissionDenied();
if (!this.assignments.getAssignmentForTrip(assignmentId, tripId)) return errorResult('Assignment not found.');
const participants = this.assignments.setParticipants(assignmentId, userIds);
const participants = this.assignments.setParticipants(assignmentId, userIds, tripId);
this.guards.safeBroadcast(tripId, 'assignment:participants', { assignmentId, participants });
return ok({ participants });
}
@@ -283,12 +283,21 @@ export class AssignmentsService {
return this.getAssignmentWithPlace(Number(id));
}
setParticipants(assignmentId: string | number, userIds: number[]) {
/**
* Both callers have already proven the assignment sits on `tripId`; this
* settles the other half, that the ids in the body do too. Off-roster ids drop
* silently rather than 400, matching bag members and reservation travellers —
* the participants box sends the whole list back on every edit, so rejecting
* the request would strand a trip whose membership changed underneath it.
*/
setParticipants(assignmentId: string | number, userIds: number[], tripId: string | number) {
const roster = this.dbs.rosterUserIds(tripId);
const scoped = userIds.filter(id => roster.has(id));
this.dbs.transaction(() => {
this.dbs.run('DELETE FROM assignment_participants WHERE assignment_id = ?', assignmentId);
if (userIds.length > 0) {
if (scoped.length > 0) {
const insert = this.dbs.prepare('INSERT OR IGNORE INTO assignment_participants (assignment_id, user_id) VALUES (?, ?)');
for (const userId of userIds) insert.run(assignmentId, userId);
for (const userId of scoped) insert.run(assignmentId, userId);
}
});
@@ -94,6 +94,11 @@ export class BudgetController {
{ from_user_id: body.from_user_id, to_user_id: body.to_user_id, amount: body.amount, currency: body.currency },
user.id,
);
// A party who is not on this trip gets the same answer as a settlement that
// does not exist, so the endpoint cannot be used to probe for user ids.
if (!settlement) {
throw new HttpException({ error: 'Settlement not found' }, 404);
}
this.budget.broadcast(tripId, 'budget:settlement-created', { settlement }, socketId);
return { settlement };
}
+1
View File
@@ -330,6 +330,7 @@ export class BudgetMcp {
// Freeze-then-write composite, same as the REST path (no-op while the
// schema has no currency input — see the #1445 note in the class doc).
const settlement = await this.budget.createSettlement(tripId, { from_user_id, to_user_id, amount }, ctx.userId);
if (!settlement) return errorResult('Settlement not found.');
this.guards.safeBroadcast(tripId, 'budget:settlement-created', { settlement });
return ok({ settlement });
}
+37 -18
View File
@@ -126,21 +126,25 @@ export class BudgetService {
return rows.map(p => ({ ...p, avatar_url: avatarUrl(p) }));
}
private knownUserIds(userIds: number[]): Set<number> {
const unique = [...new Set(userIds)];
if (unique.length === 0) return new Set();
const rows = this.db.all<{ id: number }>(
`SELECT id FROM users WHERE id IN (${unique.map(() => '?').join(',')})`,
...unique,
);
return new Set(rows.map(r => r.id));
/**
* The subset of `userIds` that is actually on this trip. Used to be a plain
* existence check against `users`, which let any id on the instance become a
* payer or a split member and come back out through the read-back join with a
* name and an avatar attached. Off-roster ids drop silently, the way the
* packing and reservations assignee paths handle them.
*/
private rosterMemberIds(tripId: string | number, userIds: number[]): Set<number> {
const unique = new Set(userIds);
if (unique.size === 0) return new Set();
const roster = this.db.rosterUserIds(tripId);
return new Set([...unique].filter(id => roster.has(id)));
}
/** Replace the payer rows of an item and keep total_price = sum of payer amounts. */
private writeItemPayers(itemId: number | string, payers: { user_id: number; amount: number }[]) {
private writeItemPayers(itemId: number | string, tripId: string | number, payers: { user_id: number; amount: number }[]) {
this.db.run('DELETE FROM budget_item_payers WHERE budget_item_id = ?', itemId);
const insert = this.db.prepare('INSERT OR IGNORE INTO budget_item_payers (budget_item_id, user_id, amount) VALUES (?, ?, ?)');
const known = this.knownUserIds(payers.map(p => p.user_id));
const known = this.rosterMemberIds(tripId, payers.map(p => p.user_id));
let total = 0;
for (const p of payers) {
if (!(p.amount > 0) || !known.has(p.user_id)) continue;
@@ -347,9 +351,9 @@ export class BudgetService {
const payerTotal = (data.payers || []).reduce((a, p) => a + (p.amount > 0 ? p.amount : 0), 0);
const total = data.payers && data.payers.length > 0 ? payerTotal : (data.total_price || 0);
const knownMembers = data.members ? this.knownUserIds(data.members.map(m => m.user_id)) : null;
const knownMembers = data.members ? this.rosterMemberIds(tripId, data.members.map(m => m.user_id)) : null;
const members = data.members && knownMembers ? data.members.filter(m => knownMembers.has(m.user_id)) : undefined;
const knownIds = data.member_ids ? this.knownUserIds(data.member_ids) : null;
const knownIds = data.member_ids ? this.rosterMemberIds(tripId, data.member_ids) : null;
const memberIds = data.member_ids && knownIds ? data.member_ids.filter(uid => knownIds.has(uid)) : undefined;
const { note, ticket } = splitLegacyTicketNote(data.note, data.ticket_json);
@@ -373,7 +377,7 @@ export class BudgetService {
);
const itemId = result.lastInsertRowid as number;
if (data.payers && data.payers.length > 0) this.writeItemPayers(itemId, data.payers);
if (data.payers && data.payers.length > 0) this.writeItemPayers(itemId, tripId, data.payers);
if (members && members.length > 0) {
const insert = this.db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid, amount) VALUES (?, ?, 0, ?)');
for (const m of members) insert.run(itemId, m.user_id, m.amount !== undefined && m.amount !== null ? m.amount : null);
@@ -461,7 +465,7 @@ export class BudgetService {
// Optional inline payer/member replacement (the edit modal saves all at once).
if (data.payers !== undefined) {
this.writeItemPayers(id, data.payers);
this.writeItemPayers(id, tripId, data.payers);
// writeItemPayers derives total_price from the payer sum (0 for no payers).
// A "recorded total, nobody assigned" expense clears payers but still carries
// an explicit total_price — re-apply it so it isn't clobbered to 0.
@@ -470,14 +474,14 @@ export class BudgetService {
}
}
if (data.members !== undefined) {
const known = this.knownUserIds(data.members.map(m => m.user_id));
const known = this.rosterMemberIds(tripId, data.members.map(m => m.user_id));
const members = data.members.filter(m => known.has(m.user_id));
this.db.run('DELETE FROM budget_item_members WHERE budget_item_id = ?', id);
const insert = this.db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid, amount) VALUES (?, ?, 0, ?)');
for (const m of members) insert.run(id, m.user_id, m.amount !== undefined && m.amount !== null ? m.amount : null);
this.db.run('UPDATE budget_items SET persons = ? WHERE id = ?', members.length || null, id);
} else if (data.member_ids !== undefined) {
const known = this.knownUserIds(data.member_ids);
const known = this.rosterMemberIds(tripId, data.member_ids);
const memberIds = data.member_ids.filter(uid => known.has(uid));
this.db.run('DELETE FROM budget_item_members WHERE budget_item_id = ?', id);
const insert = this.db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid, amount) VALUES (?, ?, 0, NULL)');
@@ -510,7 +514,7 @@ export class BudgetService {
return this.db.transaction(() => {
const item = this.db.get('SELECT id FROM budget_items WHERE id = ? AND trip_id = ?', id, tripId);
if (!item) return null;
this.writeItemPayers(id, payers);
this.writeItemPayers(id, tripId, payers);
const updated = this.db.get<BudgetItem>('SELECT * FROM budget_items WHERE id = ?', id)!;
updated.members = this.loadItemMembers(id);
updated.payers = this.loadItemPayers(id);
@@ -540,7 +544,7 @@ export class BudgetService {
this.db.run('DELETE FROM budget_item_members WHERE budget_item_id = ?', id);
const known = this.knownUserIds(userIds);
const known = this.rosterMemberIds(tripId, userIds);
const memberIds = userIds.filter(uid => known.has(uid));
if (memberIds.length > 0) {
const insert = this.db.prepare('INSERT OR IGNORE INTO budget_item_members (budget_item_id, user_id, paid) VALUES (?, ?, ?)');
@@ -949,7 +953,21 @@ export class BudgetService {
return this.setItemPayers(id, tripId, payers);
}
/**
* Unlike a split member, a settlement cannot drop an off-roster id: it has
* exactly two named parties and both columns are NOT NULL, so a stranger in
* either slot makes the row meaningless rather than merely wider. Refuse the
* whole write. Returning null lands on the caller's existing "Settlement not
* found" 404, which is also what keeps the endpoint from confirming whether
* an id it rejected exists at all.
*/
private settlementPartiesOnTrip(tripId: string | number, data: { from_user_id: number; to_user_id: number }): boolean {
const roster = this.db.rosterUserIds(tripId);
return roster.has(data.from_user_id) && roster.has(data.to_user_id);
}
async createSettlement(tripId: string | number, data: { from_user_id: number; to_user_id: number; amount: number; currency?: string | null }, userId: number) {
if (!this.settlementPartiesOnTrip(tripId, data)) return null;
// Freeze the FX rate for the display currency the amount was entered in so the
// transfer keeps cancelling its expense when live rates drift (#1445).
await this.freezeForeignRate(tripId, data);
@@ -960,6 +978,7 @@ export class BudgetService {
// Pass the settlement's stored currency so an edit that doesn't change it keeps
// the already-frozen rate (#1445) — otherwise a live-rate drift would re-open a
// settled position on an unrelated edit.
if (!this.settlementPartiesOnTrip(tripId, data)) return null;
const existing = this.getSettlement(id, tripId);
await this.freezeForeignRate(tripId, data, undefined, existing?.currency ?? null);
return this.applySettlementUpdate(id, tripId, data);
@@ -446,10 +446,23 @@ export class CollectionsService {
// Saved places CRUD
// -------------------------------------------------------------------------
/**
* A tag has no list of its own, only a `user_id`, so "belongs here" resolves
* through the people on the list — the same shape places.service uses against
* a trip roster. Off-list tag ids drop silently rather than being stored and
* read straight back out: the tag read-back ships `tags.user_id`, so an
* unfiltered id answers who owns a tag the caller cannot otherwise see.
*/
private attachTags(collectionPlaceId: number, tagIds: number[] | undefined): void {
if (!tagIds || tagIds.length === 0) return;
const unique = [...new Set(tagIds)];
const eligible = this.collectionMemberIds(this.collectionIdOfPlace(collectionPlaceId));
const owned = this.db.all<{ id: number; user_id: number }>(
`SELECT id, user_id FROM tags WHERE id IN (${unique.map(() => '?').join(',')})`,
...unique,
);
const stmt = this.db.prepare('INSERT OR IGNORE INTO collection_place_tags (collection_place_id, tag_id) VALUES (?, ?)');
for (const tid of tagIds) stmt.run(collectionPlaceId, tid);
for (const t of owned) if (eligible.has(t.user_id)) stmt.run(collectionPlaceId, t.id);
}
/** Owner + accepted members — the users whose votes may live in this list. */
@@ -56,6 +56,25 @@ export class DatabaseService {
return isOwner(tripId, userId);
}
/**
* The user ids a trip may refer to: its members plus the owner. Guests count —
* a guest is a credential-less users row joined into trip_members, and #1362
* makes it assignable everywhere a real member is.
*
* This is canAccessTrip's question asked as a set rather than a predicate, so
* it lives beside it. Domains that accept user ids in a write body intersect
* against it: holding the permission says you may edit the trip, not that any
* id you name belongs to it. Runs on the connection rather than delegating to
* db/database, because it has no legacy export to stay parity with.
*/
rosterUserIds(tripId: number | string): Set<number> {
const rows = this.all<{ user_id: number }>(
'SELECT user_id FROM trip_members WHERE trip_id = ? UNION SELECT user_id FROM trips WHERE id = ?',
tripId, tripId,
);
return new Set(rows.map(r => r.user_id));
}
getPlaceWithTags(placeId: number | string): PlaceWithTags | null {
return getPlaceWithTags(placeId);
}
+14 -6
View File
@@ -231,7 +231,8 @@ export class PackingService {
// "Shared with specific people" — record the recipients it covers.
if (data.visibility === 'shared' && Array.isArray(data.recipient_ids)) {
const ins = this.db.prepare('INSERT OR IGNORE INTO packing_item_recipients (item_id, user_id) VALUES (?, ?)');
for (const uid of data.recipient_ids) if (uid !== ownerId) ins.run(id, uid);
const roster = this.tripRosterIds(tripId);
for (const uid of data.recipient_ids) if (uid !== ownerId && roster.has(uid)) ins.run(id, uid);
}
return id;
});
@@ -350,7 +351,8 @@ export class PackingService {
if (visibility === 'shared') {
const ins = this.db.prepare('INSERT OR IGNORE INTO packing_item_recipients (item_id, user_id) VALUES (?, ?)');
const owner = item.owner_id ?? actingUserId;
for (const uid of recipientIds) if (uid !== owner) ins.run(id, uid);
const roster = this.tripRosterIds(tripId);
for (const uid of recipientIds) if (uid !== owner && roster.has(uid)) ins.run(id, uid);
}
// Leaving the Common tier drops any co-contributors (they only apply to Common).
if (visibility !== 'common') this.db.run('DELETE FROM packing_item_contributors WHERE item_id = ?', id);
@@ -479,10 +481,14 @@ export class PackingService {
}));
}
/** Owner + collaborators of a trip — the only user ids that may be assigned to a bag. */
/**
* Owner + collaborators of a trip, guests included — the only user ids
* assignable anywhere on it, not just to a bag. The wording used to say
* "assigned to a bag", which is why the category and recipient writes below
* grew up without it.
*/
private tripRosterIds(tripId: string | number): Set<number> {
const rows = this.db.all<{ user_id: number }>('SELECT user_id FROM trip_members WHERE trip_id = ? UNION SELECT user_id FROM trips WHERE id = ?', tripId, tripId);
return new Set(rows.map(r => r.user_id));
return this.db.rosterUserIds(tripId);
}
setBagMembers(tripId: string | number, bagId: string | number, userIds: number[]) {
@@ -665,7 +671,9 @@ export class PackingService {
if (Array.isArray(userIds) && userIds.length > 0) {
const insert = this.db.prepare('INSERT OR IGNORE INTO packing_category_assignees (trip_id, category_name, user_id) VALUES (?, ?, ?)');
for (const uid of userIds) insert.run(tripId, categoryName, uid);
// Same rule as setBagMembers: only people on this trip may be assigned.
const roster = this.tripRosterIds(tripId);
for (const uid of userIds) if (roster.has(uid)) insert.run(tripId, categoryName, uid);
}
});
+22 -1
View File
@@ -20,7 +20,7 @@ import type { Response } from 'express';
import { isDemoWriteBlocked, DEMO_WRITE_ERROR } from '../common/demo-write';
import { RuntimeEnvService } from '../app-config/runtime-env.service';
import { memoryStorage } from 'multer';
import { hexColorSchema } from '@trek/shared';
import { hexColorSchema, placeImageUrlSchema, placeWebsiteSchema } from '@trek/shared';
import type { User } from '../../types';
import { PlacesService } from './places.service';
import { isUpdateConflict } from '../common/conflictResult';
@@ -64,6 +64,25 @@ function validateRouteColor(body: Record<string, unknown>): void {
}
}
// Same reason as route_color: these two leave the database for a renderer that
// treats them as a URL — the thumbnail into marker HTML, the homepage into
// window.open — and the open record means the pipe never sees them.
function validateUrlFields(body: Record<string, unknown>): void {
const image = body.image_url;
if (image !== undefined && image !== null) {
if (typeof image !== 'string' || !placeImageUrlSchema.safeParse(image).success) {
throw new HttpException({ error: 'image_url must be an uploaded path, a photo-proxy path, an inline image or an https URL' }, 400);
}
}
const website = body.website;
// '' is how the UI clears the field; treat it like an absent value.
if (website !== undefined && website !== null && website !== '') {
if (typeof website !== 'string' || !placeWebsiteSchema.safeParse(website).success) {
throw new HttpException({ error: 'website must be an http or https URL' }, 400);
}
}
}
function parseBool(v: unknown, defaultVal: boolean): boolean {
return v === undefined || v === null ? defaultVal : String(v) === 'true';
}
@@ -133,6 +152,7 @@ export class PlacesController {
const trip = this.requireTrip(tripId, user);
validateLengths(body);
validateRouteColor(body);
validateUrlFields(body);
this.requireEdit(trip, user);
const place = this.places.create(tripId, body as never);
this.places.broadcast(tripId, 'place:created', { place }, socketId);
@@ -433,6 +453,7 @@ export class PlacesController {
const trip = this.requireTrip(tripId, user);
validateLengths(body);
validateRouteColor(body);
validateUrlFields(body);
this.requireEdit(trip, user);
const result = await this.places.update(tripId, id, body as never, ifMatch);
if (!result) {
+5 -4
View File
@@ -5,6 +5,7 @@ import {
demoDenied, ok,
} from '@trek/nest-mcp';
import { McpToolGuardsService } from '../mcp-shared/mcp-tool-guards.service';
import { placeWebsiteSchema } from '@trek/shared';
import { z } from 'zod';
import { AuthService } from '../auth/auth.service';
import { AssignmentsService } from '../assignments/assignments.service';
@@ -63,7 +64,7 @@ export class PlacesMcp {
google_ftid: z.string().optional().describe('Google Maps feature ID from search_place — enables direct Google Maps links'),
osm_id: z.string().optional().describe('OpenStreetMap ID from search_place (e.g. "way:12345") — enables opening hours if no Google ID'),
notes: z.string().max(2000).optional(),
website: z.string().max(500).optional(),
website: placeWebsiteSchema.optional(),
phone: z.string().max(50).optional(),
price: z.number().nonnegative().optional().describe('Cost of this place/activity (e.g. ticket price, entry fee)'),
currency: z.string().length(3).optional().describe('ISO 4217 currency code (e.g. "EUR", "USD")'),
@@ -103,7 +104,7 @@ export class PlacesMcp {
google_ftid: z.string().optional().describe('Google Maps feature ID from search_place — enables direct Google Maps links'),
osm_id: z.string().optional().describe('OpenStreetMap ID from search_place (e.g. "way:12345")'),
place_notes: z.string().max(2000).optional().describe('Notes for the place'),
website: z.string().max(500).optional(),
website: placeWebsiteSchema.optional(),
phone: z.string().max(50).optional(),
assignment_notes: z.string().max(500).optional().describe('Notes for this day assignment'),
price: z.number().nonnegative().optional().describe('Cost of this place/activity (e.g. ticket price, entry fee)'),
@@ -158,7 +159,7 @@ export class PlacesMcp {
end_time: z.string().max(50).optional().describe('End time (e.g. "11:00")'),
duration_minutes: z.number().int().positive().optional(),
notes: z.string().max(2000).optional(),
website: z.string().max(500).optional(),
website: placeWebsiteSchema.optional(),
phone: z.string().max(50).optional(),
transport_mode: z.enum(['walking', 'driving', 'cycling', 'transit', 'flight']).optional(),
osm_id: z.string().optional().describe('OpenStreetMap ID (e.g. "way:12345")'),
@@ -366,7 +367,7 @@ export class PlacesMcp {
end_time: z.string().max(50).optional().describe('End time (e.g. "11:00")'),
duration_minutes: z.number().int().positive().optional(),
notes: z.string().max(2000).optional(),
website: z.string().max(500).optional(),
website: placeWebsiteSchema.optional(),
phone: z.string().max(50).optional(),
description: z.string().max(2000).optional(),
},
+20 -1
View File
@@ -1,4 +1,4 @@
import { placeCreateRequestSchema, placeUpdateRequestSchema } from '@trek/shared';
import { placeCreateRequestSchema, placeImageUrlSchema, placeUpdateRequestSchema, placeWebsiteSchema } from '@trek/shared';
import { PluginController, PluginMethod } from '../plugins/host/rpc-kit/decorators';
import { PluginGuards } from '../plugins/host/plugin-guards.service';
import { BadParams, ForbiddenResource } from '../plugins/host/rpc-errors';
@@ -18,6 +18,23 @@ const PLACE_EDIT_ACTION = 'place_edit';
*/
const PLACE_STR_LIMITS: Record<string, number> = { name: 200, description: 2000, address: 500, notes: 2000 };
/**
* The two URL fields the REST controller pins in validateUrlFields. Same reason
* they are checked there rather than in the contract: the write schemas are open
* records, so a plugin could otherwise store a thumbnail or a homepage the web
* app refuses — and both end up somewhere that treats them as a URL.
*/
function capUrls(input: Record<string, unknown>): void {
const image = input.image_url;
if (image !== undefined && image !== null && !placeImageUrlSchema.safeParse(image).success) {
throw new BadParams('invalid place: image_url must be an uploaded path, a photo-proxy path, an inline image or an https URL');
}
const website = input.website;
if (website !== undefined && website !== null && website !== '' && !placeWebsiteSchema.safeParse(website).success) {
throw new BadParams('invalid place: website must be an http or https URL');
}
}
/**
* The place surface a plugin may reach (#plugins).
*
@@ -41,6 +58,7 @@ export class PlacesRpc {
const parsed = placeCreateRequestSchema.safeParse(params.input);
if (!parsed.success) throw new BadParams(`invalid place: ${schemaMessage(parsed.error)}`);
this.guards.capStrings(parsed.data as Record<string, unknown>, PLACE_STR_LIMITS);
capUrls(parsed.data as Record<string, unknown>);
this.guards.requireTripEdit(tripId, actor, PLACE_EDIT_ACTION);
const place = this.places.create(String(tripId), parsed.data as unknown as PlaceCreateInput);
this.realtime.broadcast(tripId, 'place:created', { place });
@@ -56,6 +74,7 @@ export class PlacesRpc {
const parsed = placeUpdateRequestSchema.safeParse(params.input);
if (!parsed.success) throw new BadParams(`invalid place: ${schemaMessage(parsed.error)}`);
this.guards.capStrings(parsed.data as Record<string, unknown>, PLACE_STR_LIMITS);
capUrls(parsed.data as Record<string, unknown>);
this.guards.requireTripEdit(tripId, actor, PLACE_EDIT_ACTION);
const place = this.places.update(String(tripId), String(placeId), parsed.data as PlaceUpdateInput);
if (place === null) throw new ForbiddenResource(`no place ${placeId} on trip ${tripId}`);
+25 -2
View File
@@ -124,6 +124,29 @@ export class PlacesService {
return this.permissions.checkPermission('place_edit', user.role, trip.user_id, user.id, trip.user_id !== user.id);
}
/**
* The subset of `tagIds` that someone on this trip owns. A tag has no trip of
* its own, only a `user_id`, so "belongs to this trip" resolves through the
* roster — which keeps the shared case working: a member tags a place, another
* member re-saves it and sends the id back, and the tag survives because its
* owner is still on the trip. Scoping to the caller instead would quietly strip
* a co-traveller's tag on every foreign edit.
*
* Off-roster ids drop silently. The place body is an open record, so an id can
* arrive from an older client that had no business knowing it existed, and the
* read-back join hands `tags.user_id` straight to the caller.
*/
private tagsOnTrip(tripId: string | number, tagIds: number[]): number[] {
const unique = [...new Set(tagIds)];
if (unique.length === 0) return [];
const roster = this.dbs.rosterUserIds(tripId);
const owned = this.dbs.all<{ id: number; user_id: number }>(
`SELECT id, user_id FROM tags WHERE id IN (${unique.map(() => '?').join(',')})`,
...unique,
);
return owned.filter(t => roster.has(t.user_id)).map(t => t.id);
}
broadcast<E extends TrekWsTripEventName>(tripId: string, event: E, payload: TrekWsPayload<E>, socketId: string | undefined): void {
this.realtime.broadcast(tripId, event, payload, socketId);
}
@@ -225,7 +248,7 @@ export class PlacesService {
if (tags && tags.length > 0) {
const insertTag = this.dbs.prepare('INSERT OR IGNORE INTO place_tags (place_id, tag_id) VALUES (?, ?)');
for (const tagId of tags) {
for (const tagId of this.tagsOnTrip(tripId, tags)) {
insertTag.run(placeId, tagId);
}
}
@@ -342,7 +365,7 @@ export class PlacesService {
this.dbs.run('DELETE FROM place_tags WHERE place_id = ?', placeId);
if (tags.length > 0) {
const insertTag = this.dbs.prepare('INSERT OR IGNORE INTO place_tags (place_id, tag_id) VALUES (?, ?)');
for (const tagId of tags) {
for (const tagId of this.tagsOnTrip(tripId, tags)) {
insertTag.run(placeId, tagId);
}
}
@@ -468,6 +468,13 @@ export class ReservationsMcp {
if (this.auth.isDemoUser(ctx.userId)) return demoDenied();
if (!this.reservations.verifyTripAccess(tripId, ctx.userId)) return noAccess();
if (!this.guards.hasTripPermission('reservation_edit', tripId, ctx.userId)) return permissionDenied();
// The service scopes the write to the trip on its own, so a foreign id is
// already harmless — say so rather than reporting a success that moved
// nothing, the way the sibling tools above do.
if (dayId && !this.days.getDay(dayId, tripId))
return errorResult('dayId does not belong to this trip.');
this.reservations.updatePositions(tripId, positions, dayId);
this.guards.safeBroadcast(tripId, 'reservation:positions', { positions, dayId });
return ok({ success: true });
@@ -527,13 +527,26 @@ export class ReservationsService {
return { reservation, accommodationCreated };
}
updatePositions(tripId: string | number, positions: { id: number; day_plan_position: number }[], dayId?: number | string | null) {
updatePositions(tripId: string | number, positions: { id: number; day_plan_position?: number }[], dayId?: number | string | null) {
if (dayId) {
// Per-day positions for multi-day reservations
const stmt = this.db.prepare('INSERT OR REPLACE INTO reservation_day_positions (reservation_id, day_id, position) VALUES (?, ?, ?)');
// Per-day positions for multi-day reservations, scoped the way the legacy
// branch below already scopes its update. The table carries no trip_id and
// its two foreign keys only ask that the ids exist, so the trip has to come
// from the join: the row materialises only when the reservation and the day
// agree on it. Doing that in the statement rather than as a pre-check also
// makes a stale id a quiet no-op instead of a foreign-key error surfacing
// as a 500.
const stmt = this.db.prepare(`
INSERT OR REPLACE INTO reservation_day_positions (reservation_id, day_id, position)
SELECT r.id, d.id, ?
FROM reservations r
JOIN days d ON d.trip_id = r.trip_id
WHERE r.id = ? AND d.id = ? AND r.trip_id = ?
`);
this.db.transaction(() => {
for (const item of positions) {
stmt.run(item.id, dayId, item.day_plan_position);
// position is NOT NULL while the wire contract leaves the value optional.
stmt.run(item.day_plan_position ?? 0, item.id, dayId, tripId);
}
});
} else {
+6 -1
View File
@@ -136,7 +136,12 @@ export class TodoService {
if (Array.isArray(userIds) && userIds.length > 0) {
const insert = this.db.prepare('INSERT OR IGNORE INTO todo_category_assignees (trip_id, category_name, user_id) VALUES (?, ?, ?)');
for (const uid of userIds) insert.run(tripId, categoryName, uid);
// Only people on this trip may be assigned, the way packing filters bag
// members and reservations filter travellers. Dropped rather than
// rejected: a copied trip carries assignee ids across before its members
// exist, and a 400 would make the picker unusable there.
const roster = this.db.rosterUserIds(tripId);
for (const uid of userIds) if (roster.has(uid)) insert.run(tripId, categoryName, uid);
}
});
+88
View File
@@ -194,4 +194,92 @@ describe('Assignments e2e (real auth guard + temp SQLite)', () => {
expect(res.status).toBe(200);
expect(res.body.assignment).toMatchObject({ id, day_id: 4, order_index: 0 });
});
// The per-assignment controller declared @RequirePermission but not the guard
// that reads it, so the decorators were inert metadata and :tripId was never
// checked against the caller at all. The handlers that ask
// getAssignmentForTrip(id, tripId) were happy as long as the assignment sat on
// the trip in the URL — which is true for the owner's trip too.
describe('a trip the caller cannot see', () => {
const FOREIGN_TRIP = 9;
beforeEach(() => {
canAccessTrip.mockImplementation((tripId: unknown) => (Number(tripId) === 5 ? { id: 5, user_id: 1 } : undefined));
db.prepare('INSERT OR IGNORE INTO days (id, trip_id) VALUES (30, ?), (31, ?)').run(FOREIGN_TRIP, FOREIGN_TRIP);
db.prepare('INSERT OR IGNORE INTO places (id, trip_id, name) VALUES (20, ?, ?)').run(FOREIGN_TRIP, 'Their hotel');
});
const seedForeignAssignment = () =>
Number(db.prepare('INSERT INTO day_assignments (day_id, place_id, order_index) VALUES (30, 20, 0)').run().lastInsertRowid);
it('404s a move instead of reordering their itinerary', async () => {
const id = seedForeignAssignment();
const res = await request(server)
.put(`/api/trips/${FOREIGN_TRIP}/assignments/${id}/move`)
.set('Cookie', sessionCookie(1))
.send({ new_day_id: 31, order_index: 0 });
expect(res.status).toBe(404);
expect(db.prepare('SELECT day_id FROM day_assignments WHERE id = ?').get(id)).toEqual({ day_id: 30 });
});
it('404s a time change instead of rewriting their schedule', async () => {
const id = seedForeignAssignment();
const res = await request(server)
.put(`/api/trips/${FOREIGN_TRIP}/assignments/${id}/time`)
.set('Cookie', sessionCookie(1))
.send({ place_time: '23:00', end_time: null });
expect(res.status).toBe(404);
expect(db.prepare('SELECT assignment_time FROM day_assignments WHERE id = ?').get(id)).toEqual({ assignment_time: null });
});
it('404s a transport change', async () => {
const id = seedForeignAssignment();
const res = await request(server)
.put(`/api/trips/${FOREIGN_TRIP}/assignments/${id}/transport`)
.set('Cookie', sessionCookie(1))
.send({ transport_mode: 'driving' });
expect(res.status).toBe(404);
});
it('404s setting participants instead of writing to their assignment', async () => {
const id = seedForeignAssignment();
const res = await request(server)
.put(`/api/trips/${FOREIGN_TRIP}/assignments/${id}/participants`)
.set('Cookie', sessionCookie(1))
.send({ user_ids: [1] });
expect(res.status).toBe(404);
expect(db.prepare('SELECT COUNT(*) AS n FROM assignment_participants WHERE assignment_id = ?').get(id)).toEqual({ n: 0 });
});
it('404s reading participants instead of disclosing who is on it', async () => {
const id = seedForeignAssignment();
db.prepare('INSERT INTO assignment_participants (assignment_id, user_id) VALUES (?, 2)').run(id);
const res = await request(server)
.get(`/api/trips/${FOREIGN_TRIP}/assignments/${id}/participants`)
.set('Cookie', sessionCookie(1));
expect(res.status).toBe(404);
});
// The guard has to reject an assignment borrowed from elsewhere even when the
// caller is legitimately on the trip named in the URL.
it('404s an assignment that belongs to another trip than the URL says', async () => {
const id = seedForeignAssignment();
const res = await request(server)
.put(`/api/trips/5/assignments/${id}/participants`)
.set('Cookie', sessionCookie(1))
.send({ user_ids: [1] });
expect(res.status).toBe(404);
expect(db.prepare('SELECT COUNT(*) AS n FROM assignment_participants WHERE assignment_id = ?').get(id)).toEqual({ n: 0 });
});
it('403s when the caller is on the trip but lacks day_edit', async () => {
const id = seedAssignment();
checkPermission.mockReturnValue(false);
const res = await request(server)
.put(`/api/trips/5/assignments/${id}/time`)
.set('Cookie', sessionCookie(1))
.send({ place_time: '09:00', end_time: null });
expect(res.status).toBe(403);
});
});
});
+3
View File
@@ -73,6 +73,9 @@ describe('Budget e2e (real auth guard + temp SQLite, real budget SQL)', () => {
"INSERT INTO users (id, username, email, password_hash, role, password_version) VALUES (2, 'e2e-peer', 'peer@example.test', 'x', 'user', 0)",
).run();
tripId = Number(db.prepare("INSERT INTO trips (user_id, title, currency) VALUES (1, 'E2E Trip', 'EUR')").run().lastInsertRowid);
// The peer settles up with the owner below, so they have to be on the trip:
// a settlement between people who do not share one is refused.
db.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, 2)').run(tripId);
app = await build();
checkPermission = vi.spyOn(app.get(PermissionsService), 'checkPermission');
server = app.getHttpServer();
+88
View File
@@ -185,4 +185,92 @@ describe('Reservations + accommodations e2e (real auth guard + temp SQLite, real
expect(res.status).toBe(400);
expect(res.body).toEqual({ error: 'place_id, start_day_id, and end_day_id are required' });
});
// The per-day branch of updatePositions used to bind the caller's ids straight
// into reservation_day_positions without ever looking at the tripId it was
// handed, so a request authorised on your own trip could rewrite the ordering
// of someone else's. The legacy branch three lines below it always scoped by
// trip_id — these pin that the two behave the same way now.
describe('positions are confined to the trip in the URL', () => {
let foreignTripId: number;
let foreignReservationId: number;
let foreignDayId: number;
let ownDayId: number;
// Days are unique per (trip_id, day_number) and this block seeds a fresh pair
// for every case, so the numbers have to keep climbing.
let dayNumber = 100;
beforeEach(() => {
db.prepare(
"INSERT OR IGNORE INTO users (id, username, email, password_hash, role, password_version) VALUES (2, 'victim', 'victim@example.test', 'x', 'user', 0)",
).run();
foreignTripId = Number(db.prepare("INSERT INTO trips (user_id, title) VALUES (2, 'Someone else')").run().lastInsertRowid);
foreignReservationId = Number(db.prepare("INSERT INTO reservations (trip_id, title, type) VALUES (?, 'Secret', 'other')").run(foreignTripId).lastInsertRowid);
dayNumber += 1;
foreignDayId = Number(db.prepare("INSERT INTO days (trip_id, day_number, date) VALUES (?, ?, '2026-05-01')").run(foreignTripId, dayNumber).lastInsertRowid);
ownDayId = Number(db.prepare("INSERT INTO days (trip_id, day_number, date) VALUES (?, ?, '2026-05-01')").run(tripId, dayNumber).lastInsertRowid);
});
function positionRow(reservationId: number, dayId: number) {
return db.prepare('SELECT position FROM reservation_day_positions WHERE reservation_id = ? AND day_id = ?').get(reservationId, dayId);
}
it('writes nothing when both ids belong to another trip', async () => {
const res = await request(server)
.put(`/api/trips/${tripId}/reservations/positions`)
.set('Cookie', sessionCookie(1))
.send({ positions: [{ id: foreignReservationId, day_plan_position: 999 }], day_id: foreignDayId });
expect(res.status).toBe(200);
expect(positionRow(foreignReservationId, foreignDayId)).toBeUndefined();
});
it('writes nothing when only the reservation is foreign', async () => {
const res = await request(server)
.put(`/api/trips/${tripId}/reservations/positions`)
.set('Cookie', sessionCookie(1))
.send({ positions: [{ id: foreignReservationId, day_plan_position: 5 }], day_id: ownDayId });
expect(res.status).toBe(200);
expect(positionRow(foreignReservationId, ownDayId)).toBeUndefined();
});
it('writes nothing when only the day is foreign', async () => {
const ownReservationId = Number(db.prepare("INSERT INTO reservations (trip_id, title, type) VALUES (?, 'Mine', 'other')").run(tripId).lastInsertRowid);
const res = await request(server)
.put(`/api/trips/${tripId}/reservations/positions`)
.set('Cookie', sessionCookie(1))
.send({ positions: [{ id: ownReservationId, day_plan_position: 5 }], day_id: foreignDayId });
expect(res.status).toBe(200);
expect(positionRow(ownReservationId, foreignDayId)).toBeUndefined();
});
it('still stores a position when both ids are on the trip', async () => {
const ownReservationId = Number(db.prepare("INSERT INTO reservations (trip_id, title, type) VALUES (?, 'Mine', 'other')").run(tripId).lastInsertRowid);
const res = await request(server)
.put(`/api/trips/${tripId}/reservations/positions`)
.set('Cookie', sessionCookie(1))
.send({ positions: [{ id: ownReservationId, day_plan_position: 3 }], day_id: ownDayId });
expect(res.status).toBe(200);
expect(positionRow(ownReservationId, ownDayId)).toEqual({ position: 3 });
});
// An id that exists nowhere used to raise a foreign-key error, which the
// exception filter turned into a 500 — telling the caller apart from the
// 200 a real id returns, for any id on the instance.
it('answers a nonexistent reservation the same way it answers a foreign one', async () => {
const res = await request(server)
.put(`/api/trips/${tripId}/reservations/positions`)
.set('Cookie', sessionCookie(1))
.send({ positions: [{ id: 999999, day_plan_position: 1 }], day_id: ownDayId });
expect(res.status).toBe(200);
});
it('does not fall over when day_plan_position is omitted', async () => {
const ownReservationId = Number(db.prepare("INSERT INTO reservations (trip_id, title, type) VALUES (?, 'Mine', 'other')").run(tripId).lastInsertRowid);
const res = await request(server)
.put(`/api/trips/${tripId}/reservations/positions`)
.set('Cookie', sessionCookie(1))
.send({ positions: [{ id: ownReservationId }], day_id: ownDayId });
expect(res.status).toBe(200);
});
});
});
@@ -35,31 +35,47 @@ describe('incoming_leg_transport_mode migration', () => {
// The case above starts from an empty database and runs every migration, so it
// passes wherever a migration sits in the array.
it('the trailing migration lands on a database that is one version behind', () => {
it('the trailing migrations land on a database that is behind', () => {
// What actually has to hold: existing installs replay only the slots above
// their schema_version, so a migration inserted mid-array is silently
// skipped on every one of them. Undoing what the LAST migration did and
// rewinding one version is the only way to prove it still runs.
// skipped on every one of them. Undoing what the trailing migrations did
// and rewinding is the only way to prove they still run.
//
// >>> Appending a migration? Re-point the undo below at whatever yours
// >>> does. That is the whole maintenance cost of this guard.
// Trailing migration today: the storage slice-2 trip_files 'files/' prefix
// strip — undo it by re-inserting a prefixed row.
// Trailing migrations today (a dev/feat-storage-driver merge landed two
// in one release window, so the rewind spans both):
// version-2 — reservation_day_positions cross-trip cleanup: undone by
// re-inserting a mismatched reservation/day pair.
// version-1 — storage slice-2 trip_files 'files/' prefix strip: undone
// by re-inserting a prefixed row.
const upgraded = new Database(':memory:');
upgraded.exec('PRAGMA foreign_keys = ON');
createTables(upgraded);
runMigrations(upgraded);
const { version } = upgraded.prepare('SELECT version FROM schema_version').get() as { version: number };
upgraded.prepare("INSERT INTO users (id, username, email, password_hash) VALUES (1, 'u', 'u@example.test', 'x')").run();
upgraded.prepare("INSERT INTO trips (id, user_id, title) VALUES (1, 1, 'T')").run();
upgraded.prepare("INSERT INTO trip_files (trip_id, filename, original_name) VALUES (1, 'files/aaa.pdf', 'a.pdf')").run();
upgraded.prepare('UPDATE schema_version SET version = ?').run(version - 1);
upgraded.prepare("INSERT INTO users (id, username, email, password_hash) VALUES (1, 'a', 'a@test.local', 'x')").run();
upgraded.prepare("INSERT INTO users (id, username, email, password_hash) VALUES (2, 'b', 'b@test.local', 'x')").run();
const tripA = upgraded.prepare("INSERT INTO trips (user_id, title) VALUES (1, 'A')").run().lastInsertRowid;
const tripB = upgraded.prepare("INSERT INTO trips (user_id, title) VALUES (2, 'B')").run().lastInsertRowid;
const reservationOnA = upgraded.prepare("INSERT INTO reservations (trip_id, title, type) VALUES (?, 'r', 'other')").run(tripA).lastInsertRowid;
const dayOnA = upgraded.prepare("INSERT INTO days (trip_id, day_number, date) VALUES (?, 1, '2026-01-01')").run(tripA).lastInsertRowid;
const dayOnB = upgraded.prepare("INSERT INTO days (trip_id, day_number, date) VALUES (?, 1, '2026-01-01')").run(tripB).lastInsertRowid;
const insertPosition = upgraded.prepare('INSERT INTO reservation_day_positions (reservation_id, day_id, position) VALUES (?, ?, ?)');
insertPosition.run(reservationOnA, dayOnB, 1); // the mismatched pair the migration clears
insertPosition.run(reservationOnA, dayOnA, 2); // a legitimate one it must leave alone
upgraded.prepare("INSERT INTO trip_files (trip_id, filename, original_name) VALUES (?, 'files/aaa.pdf', 'a.pdf')").run(tripA);
upgraded.prepare('UPDATE schema_version SET version = ?').run(version - 2);
runMigrations(upgraded);
const rows = upgraded.prepare('SELECT filename FROM trip_files').all() as { filename: string }[];
expect(rows.map(r => r.filename)).toEqual(['aaa.pdf']);
const positions = upgraded.prepare('SELECT day_id FROM reservation_day_positions ORDER BY day_id').all() as { day_id: number }[];
expect(positions).toEqual([{ day_id: Number(dayOnA) }]);
const files = upgraded.prepare('SELECT filename FROM trip_files').all() as { filename: string }[];
expect(files.map(r => r.filename)).toEqual(['aaa.pdf']);
expect(upgraded.prepare('SELECT version FROM schema_version').get()).toEqual({ version });
upgraded.close();
});
@@ -135,6 +135,8 @@ const ALLOWED_DESTRUCTIVE: Record<string, string> = {
'Atlas enclave fix: DELETE ... WHERE place_id IN (places inside specific enclave boxes) — invalidate stale region cache; re-resolved on next request.',
'DELETE FROM visited_regions':
'Atlas geoBoundaries swap (#1119): DELETE ... WHERE id = ? — after UPDATE OR IGNORE re-codes a manually-marked region to its current code, drop only the single leftover row whose UNIQUE(user_id, region_code) collision caused the update to be skipped (a duplicate of a region the user already has).',
'DELETE FROM reservation_day_positions':
'DELETE ... WHERE the row joins a reservation and a day sitting on different trips. The table has no trip_id and its two foreign keys only require the ids to exist, so a pair that never belonged together was storable; the writer scopes to the trip now and this clears what earlier builds allowed. Bounded by the join — a row whose reservation and day agree on their trip is untouched.',
};
describe('migration hygiene — destructive operation guard', () => {
@@ -78,8 +78,10 @@ describe('AssignmentOpsController (parity with the per-assignment op routes)', (
expect(reconcile).toHaveBeenCalledWith('5', 'sock');
});
it('GET /:id/participants returns participants (access-only)', () => {
const s = svc({ getParticipants: vi.fn().mockReturnValue([{ user_id: 2 }]) } as Partial<AssignmentsService>);
it('GET /:id/participants 404 when the assignment is on another trip, else returns participants (access-only)', () => {
expect(thrown(() => new AssignmentOpsController(svc({ getAssignmentForTrip: vi.fn().mockReturnValue(undefined) } as Partial<AssignmentsService>)).participants(user, '5', '9')))
.toEqual({ status: 404, body: { error: 'Assignment not found' } });
const s = svc({ getAssignmentForTrip: vi.fn().mockReturnValue({ id: 9 }), getParticipants: vi.fn().mockReturnValue([{ user_id: 2 }]) } as Partial<AssignmentsService>);
expect(new AssignmentOpsController(s).participants(user, '5', '9')).toEqual({ participants: [{ user_id: 2 }] });
});
@@ -92,10 +94,15 @@ describe('AssignmentOpsController (parity with the per-assignment op routes)', (
expect(reconcile).toHaveBeenCalledWith('5', 'sock');
});
it('PUT /:id/participants sets + broadcasts (non-array bodies are the Zod pipe\'s 400, covered in e2e)', () => {
it('PUT /:id/participants 404 on a foreign assignment, else sets + broadcasts (non-array bodies are the Zod pipe\'s 400, covered in e2e)', () => {
const setParticipants = vi.fn().mockReturnValue([{ user_id: 2 }]); const broadcast = vi.fn();
expect(new AssignmentOpsController(svc({ setParticipants, broadcast } as Partial<AssignmentsService>)).setParticipants(user, '5', '9', { user_ids: [2] }, 'sock')).toEqual({ participants: [{ user_id: 2 }] });
expect(setParticipants).toHaveBeenCalledWith('9', [2]);
expect(thrown(() => new AssignmentOpsController(svc({ getAssignmentForTrip: vi.fn().mockReturnValue(undefined), setParticipants } as Partial<AssignmentsService>)).setParticipants(user, '5', '9', { user_ids: [2] })))
.toEqual({ status: 404, body: { error: 'Assignment not found' } });
expect(setParticipants).not.toHaveBeenCalled();
const s = svc({ getAssignmentForTrip: vi.fn().mockReturnValue({ id: 9 }), setParticipants, broadcast } as Partial<AssignmentsService>);
expect(new AssignmentOpsController(s).setParticipants(user, '5', '9', { user_ids: [2] }, 'sock')).toEqual({ participants: [{ user_id: 2 }] });
// The trip comes along so the service can confine the ids to its roster.
expect(setParticipants).toHaveBeenCalledWith('9', [2], '5');
expect(broadcast).toHaveBeenCalledWith('5', 'assignment:participants', { assignmentId: 9, participants: [{ user_id: 2 }] }, 'sock');
});
});
@@ -205,7 +205,7 @@ describe('listDayAssignments', () => {
testDb.prepare('INSERT INTO place_tags (place_id, tag_id) VALUES (?, ?)').run(place.id, tag.id);
const a1 = createDayAssignment(testDb, day.id, place.id, { order_index: 1 });
const a2 = createDayAssignment(testDb, day.id, second.id, { order_index: 0 });
svc.setParticipants(a1.id, [user.id]);
svc.setParticipants(a1.id, [user.id], trip.id);
const list = svc.listDayAssignments(day.id);
expect(list.map(a => a.id)).toEqual([a2.id, a1.id]);
@@ -284,45 +284,64 @@ describe('moveAssignment', () => {
describe('getParticipants / setParticipants', () => {
it('ASG-SVC-017: setParticipants replaces the list and returns the joined rows', () => {
const { user, day, place } = fixture();
const { user, trip, day, place } = fixture();
const { user: peer } = createUser(testDb);
addTripMember(testDb, trip.id, peer.id);
const a = createDayAssignment(testDb, day.id, place.id);
svc.setParticipants(a.id, [user.id]);
const replaced = svc.setParticipants(a.id, [peer.id]);
svc.setParticipants(a.id, [user.id], trip.id);
const replaced = svc.setParticipants(a.id, [peer.id], trip.id);
expect(replaced).toEqual([{ user_id: peer.id, username: peer.username, avatar: null }]);
expect(svc.getParticipants(a.id)).toEqual(replaced);
});
it('ASG-SVC-018: empty array clears all participants', () => {
const { user, day, place } = fixture();
const { user, trip, day, place } = fixture();
const a = createDayAssignment(testDb, day.id, place.id);
svc.setParticipants(a.id, [user.id]);
expect(svc.setParticipants(a.id, [])).toEqual([]);
svc.setParticipants(a.id, [user.id], trip.id);
expect(svc.setParticipants(a.id, [], trip.id)).toEqual([]);
expect(svc.getParticipants(a.id)).toEqual([]);
});
it('ASG-SVC-019: username COALESCEs display_name over username', () => {
const { user, day, place } = fixture();
const { user, trip, day, place } = fixture();
testDb.prepare('UPDATE users SET display_name = ? WHERE id = ?').run('Fancy Name', user.id);
const a = createDayAssignment(testDb, day.id, place.id);
const rows = svc.setParticipants(a.id, [user.id]);
const rows = svc.setParticipants(a.id, [user.id], trip.id);
expect(rows).toEqual([{ user_id: user.id, username: 'Fancy Name', avatar: null }]);
});
it('ASG-SVC-020: duplicate user ids collapse via INSERT OR IGNORE', () => {
const { user, day, place } = fixture();
const { user, trip, day, place } = fixture();
const a = createDayAssignment(testDb, day.id, place.id);
const rows = svc.setParticipants(a.id, [user.id, user.id]);
const rows = svc.setParticipants(a.id, [user.id, user.id], trip.id);
expect(rows).toHaveLength(1);
});
it('ASG-SVC-028: a failing insert rolls back the whole replace (delete included)', () => {
const { user, day, place } = fixture();
// Used to assert that an id violating the users FK threw and rolled the delete
// back. It cannot reach the insert any more: the roster filter drops an id that
// is not on the trip, so a list of nothing but strangers is simply an empty
// list, and the delete stands. The rollback itself is still covered — the
// transaction wraps delete and insert together, and ASG-SVC-018 pins that an
// explicit empty list clears.
it('ASG-SVC-028: a list of ids that are not on the trip clears rather than throwing', () => {
const { user, trip, day, place } = fixture();
const a = createDayAssignment(testDb, day.id, place.id);
svc.setParticipants(a.id, [user.id]);
// 999999 violates the users FK — the transaction must undo the delete too.
expect(() => svc.setParticipants(a.id, [999999])).toThrow();
expect(svc.getParticipants(a.id)).toEqual([{ user_id: user.id, username: user.username, avatar: null }]);
svc.setParticipants(a.id, [user.id], trip.id);
expect(() => svc.setParticipants(a.id, [999999], trip.id)).not.toThrow();
expect(svc.getParticipants(a.id)).toEqual([]);
});
it('ASG-SVC-029: keeps the trip members and drops the stranger in one call', () => {
const { user, trip, day, place } = fixture();
const { user: member } = createUser(testDb);
const { user: stranger } = createUser(testDb);
addTripMember(testDb, trip.id, member.id);
const a = createDayAssignment(testDb, day.id, place.id);
const rows = svc.setParticipants(a.id, [user.id, stranger.id, member.id], trip.id) as { user_id: number }[];
expect(rows.map(r => r.user_id).sort()).toEqual([user.id, member.id].sort());
expect(JSON.stringify(rows)).not.toContain(stranger.username);
});
});
@@ -55,7 +55,7 @@ vi.mock('../../../src/nest/budget/exchange-rates.service', () => ({
import { createTables } from '../../../src/db/schema';
import { runMigrations } from '../../../src/db/migrations';
import { resetTestDb } from '../../helpers/test-db';
import { createUser, createTrip } from '../../helpers/factories';
import { createUser, createTrip, addTripMember } from '../../helpers/factories';
import { BudgetService } from '../../../src/nest/budget/budget.service';
import { DatabaseService } from '../../../src/nest/database/database.service';
import { PermissionsService } from '../../../src/nest/permissions/permissions.service';
@@ -230,6 +230,7 @@ describe('calculateSettlement custom splits', () => {
const { user: alice } = createUser(testDb, { username: 'alice' });
const { user: bob } = createUser(testDb, { username: 'bob' });
const trip = createTrip(testDb, alice.id, { title: 'Trip' });
addTripMember(testDb, trip.id, bob.id);
// 100 total, custom split: Alice owes 90, Bob owes 10. Alice paid the whole bill.
budget.createBudgetItem(trip.id, {
@@ -261,6 +262,8 @@ describe('calculateSettlement squares up to the cent (#1382)', () => {
const { user: bob } = createUser(testDb, { username: 'bob' });
const { user: carol } = createUser(testDb, { username: 'carol' });
const trip = createTrip(testDb, alice.id, { title: 'Trip' });
addTripMember(testDb, trip.id, bob.id);
addTripMember(testDb, trip.id, carol.id);
const members = [{ user_id: alice.id }, { user_id: bob.id }, { user_id: carol.id }];
// Totals that never divide by three, so every expense leaves a remainder cent
@@ -306,6 +309,8 @@ function seedIssue1543Trip(tripCurrency: string) {
const { user: danil } = createUser(testDb, { username: 'danil' });
const { user: serega } = createUser(testDb, { username: 'serega' });
const trip = createTrip(testDb, me.id, { title: 'Trip' });
addTripMember(testDb, trip.id, danil.id);
addTripMember(testDb, trip.id, serega.id);
testDb.prepare('UPDATE trips SET currency = ? WHERE id = ?').run(tripCurrency, trip.id);
const members = [{ user_id: me.id }, { user_id: danil.id }, { user_id: serega.id }];
@@ -384,6 +389,7 @@ describe('rebaseTripCurrency', () => {
const { user: alice } = createUser(testDb, { username: 'alice' });
const { user: bob } = createUser(testDb, { username: 'bob' });
const trip = createTrip(testDb, alice.id, { title: 'Trip' });
addTripMember(testDb, trip.id, bob.id);
testDb.prepare("UPDATE trips SET currency = 'EUR' WHERE id = ?").run(trip.id);
const members = [{ user_id: alice.id }, { user_id: bob.id }];
@@ -524,6 +530,57 @@ describe('composite service paths (ex budget.bridge delegation)', () => {
});
});
// A settlement names two parties and both columns are NOT NULL, so an id that is
// not on the trip cannot simply be dropped the way a split member can — the whole
// write is refused. Refusing also means the endpoint stops answering "does this
// user id exist", which it used to do by 500ing on the foreign key.
describe('settlement parties are confined to the trip', () => {
it('BUDGET-SVC-DB-028: refuses a payer who is not on the trip', async () => {
const { user: alice } = createUser(testDb, { username: 'alice' });
const { user: outsider } = createUser(testDb, { username: 'outsider' });
const trip = createTrip(testDb, alice.id);
const created = await budget.createSettlement(trip.id, { from_user_id: outsider.id, to_user_id: alice.id, amount: 10 }, alice.id);
expect(created).toBeNull();
expect(budget.listSettlements(trip.id)).toEqual([]);
});
it('BUDGET-SVC-DB-029: answers a nonexistent user the same way, without throwing', async () => {
const { user: alice } = createUser(testDb, { username: 'alice' });
const trip = createTrip(testDb, alice.id);
await expect(budget.createSettlement(trip.id, { from_user_id: 999999, to_user_id: alice.id, amount: 10 }, alice.id))
.resolves.toBeNull();
});
it('BUDGET-SVC-DB-030: still records a settlement between the owner and a member', async () => {
const { user: alice } = createUser(testDb, { username: 'alice' });
const { user: bob } = createUser(testDb, { username: 'bob' });
const trip = createTrip(testDb, alice.id);
addTripMember(testDb, trip.id, bob.id);
const created = await budget.createSettlement(trip.id, { from_user_id: bob.id, to_user_id: alice.id, amount: 10 }, alice.id);
expect(created).toMatchObject({ from_user_id: bob.id, to_user_id: alice.id });
});
it('BUDGET-SVC-DB-031: drops an off-trip payer from an item instead of refusing it', () => {
const { user: alice } = createUser(testDb, { username: 'alice' });
const { user: outsider } = createUser(testDb, { username: 'outsider' });
const trip = createTrip(testDb, alice.id);
const item = budget.createBudgetItem(trip.id, {
name: 'Dinner',
payers: [{ user_id: alice.id, amount: 40 }, { user_id: outsider.id, amount: 60 }],
}) as { payers: { user_id: number }[]; total_price: number };
expect(item.payers.map(p => p.user_id)).toEqual([alice.id]);
// total_price is the sum of the payers that actually landed.
expect(item.total_price).toBe(40);
});
});
describe('post-fold quirk fixes', () => {
it('BUDGET-SVC-DB-019: settlements prefer display_name over username (quirk fix)', () => {
const { user: alice } = createUser(testDb, { username: 'alice' });
@@ -139,8 +139,14 @@ describe('BudgetService', () => {
});
describe('settlement ledger wrappers (#1445 freeze-then-write)', () => {
// Both wrappers refuse a party who is not on the trip before they freeze
// anything, so the roster lookup has to answer for these to reach the write
// at all. Users 1 and 2 are the two parties every case here settles between.
const rosterHas = (...userIds: number[]) => dbMock._stmt.all.mockReturnValue(userIds.map(user_id => ({ user_id })));
it('createSettlement freezes the FX rate (await) before the raw insert', async () => {
const s = svc();
rosterHas(1, 2);
const freezeSpy = vi.spyOn(s, 'freezeForeignRate').mockResolvedValue();
const insertSpy = vi.spyOn(s, 'insertSettlement').mockReturnValue({ id: 7 } as never);
await s.createSettlement('5', { from_user_id: 1, to_user_id: 2, amount: 10 }, 3);
@@ -150,6 +156,7 @@ describe('BudgetService', () => {
it('updateSettlement threads the stored currency through the freeze', async () => {
const s = svc();
rosterHas(1, 2);
const freezeSpy = vi.spyOn(s, 'freezeForeignRate').mockResolvedValue();
// Quirk fix: the stored-currency lookup is a targeted getSettlement, not
// a full listSettlements scan.
@@ -161,6 +168,18 @@ describe('BudgetService', () => {
expect(freezeSpy).toHaveBeenCalledWith('5', { from_user_id: 1, to_user_id: 2, amount: 12, currency: 'USD' }, undefined, 'USD');
expect(applySpy).toHaveBeenCalledWith('7', '5', { from_user_id: 1, to_user_id: 2, amount: 12, currency: 'USD' });
});
it('refuses a party who is not on the trip, before freezing or writing', async () => {
const s = svc();
rosterHas(1); // user 2 is not on this trip
const freezeSpy = vi.spyOn(s, 'freezeForeignRate').mockResolvedValue();
const insertSpy = vi.spyOn(s, 'insertSettlement').mockReturnValue({ id: 7 } as never);
await expect(s.createSettlement('5', { from_user_id: 1, to_user_id: 2, amount: 10 }, 3)).resolves.toBeNull();
expect(freezeSpy).not.toHaveBeenCalled();
expect(insertSpy).not.toHaveBeenCalled();
});
});
describe('syncReservationPrice', () => {
@@ -515,6 +515,7 @@ describe('three-tier packing sharing (#858)', () => {
const { user: friend } = createUser(testDb);
const { user: stranger } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
addTripMember(testDb, trip.id, friend.id);
const item = svc.createItem(trip.id, { name: 'Power bank', visibility: 'shared', recipient_ids: [friend.id] }, owner.id) as any;
expect(item.is_private).toBe(1);
@@ -539,6 +540,7 @@ describe('three-tier packing sharing (#858)', () => {
const { user: owner } = createUser(testDb);
const { user: friend } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
addTripMember(testDb, trip.id, friend.id);
const item = svc.createItem(trip.id, { name: 'First aid', visibility: 'personal' }, owner.id) as any;
// A non-owner who cannot see the item at all gets the missing-item answer, so
@@ -1007,6 +1009,8 @@ describe('packing item object-level authorization', () => {
const { user: intruder } = createUser(testDb);
const { user: friend } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
// The recipient is a fellow traveller; the intruder stays off the trip on purpose.
addTripMember(testDb, trip.id, friend.id);
const personal = svc.createItem(trip.id, { name: 'Diary', visibility: 'personal' }, owner.id) as any;
const shared = svc.createItem(trip.id, { name: 'Power bank', visibility: 'shared', recipient_ids: [friend.id] }, owner.id) as any;
const common = svc.createItem(trip.id, { name: 'Tent', visibility: 'common' }, owner.id) as any;
@@ -304,6 +304,60 @@ describe('PlacesController (parity with the legacy /api/trips/:tripId/places rou
});
});
// image_url and website leave the database for something that treats them as a
// URL — the thumbnail into hand-built marker HTML, the homepage into
// window.open. The write body is an open record, so the Zod pipe never sees
// either one; they are checked here for the same reason route_color is.
// (update() is async since the storage slices; create() is not.)
describe('image_url and website', () => {
const imageErr = { status: 400, body: { error: 'image_url must be an uploaded path, a photo-proxy path, an inline image or an https URL' } };
const siteErr = { status: 400, body: { error: 'website must be an http or https URL' } };
const ctl = (over: Partial<PlacesService> = {}) => new PlacesController(svc(over), new RuntimeEnvService(), storageStub);
it('400s an image_url the marker builders were never meant to receive, before the permission check', async () => {
const canEdit = vi.fn().mockReturnValue(false); // would 403 if it got that far
for (const image_url of [
'javascript:alert(1)',
'data:text/html,<script>alert(1)</script>',
'http://insecure.example/p.png',
'//evil.example/p.png',
123,
]) {
expect(await thrownAsync(() => ctl({ canEdit }).update(user, '5', '9', { image_url }))).toEqual(imageErr);
}
expect(thrown(() => ctl({ canEdit }).create(user, '5', { name: 'T', image_url: 'javascript:alert(1)' }))).toEqual(imageErr);
expect(canEdit).not.toHaveBeenCalled();
});
it('keeps accepting every shape the app actually stores', async () => {
const update = vi.fn().mockReturnValue({ id: 9 });
for (const image_url of [
'/uploads/places/eiffel.jpg',
'/api/maps/place-photo/abc123',
'data:image/jpeg;base64,/9j/4AAQSkZJRg==',
'https://images.example/photo.jpg',
null,
]) {
expect(await ctl({ update } as Partial<PlacesService>).update(user, '5', '9', { image_url })).toEqual({ place: { id: 9 } });
}
});
it('400s a website that window.open would not treat as a page', async () => {
const canEdit = vi.fn().mockReturnValue(false);
for (const website of ['javascript:fetch("/api/trips")', 'data:text/html,x', 'louvre.fr', 42]) {
expect(await thrownAsync(() => ctl({ canEdit }).update(user, '5', '9', { website }))).toEqual(siteErr);
}
expect(canEdit).not.toHaveBeenCalled();
});
it('accepts http and https, and treats the empty string as clearing the field', async () => {
const update = vi.fn().mockReturnValue({ id: 9 });
for (const website of ['https://louvre.fr', 'http://pension.at', '', null]) {
expect(await ctl({ update } as Partial<PlacesService>).update(user, '5', '9', { website })).toEqual({ place: { id: 9 } });
}
});
});
it('PUT /:id forwards the base-version token and 409s on a conflict (#1135)', async () => {
const update = vi.fn().mockReturnValue({ conflict: true, server: { id: 9, name: 'Theirs' } });
const onUpdated = vi.fn(); const broadcast = vi.fn();
+28 -1
View File
@@ -64,7 +64,7 @@ const photoCacheStub = { removeIfUnreferenced: removeIfUnreferencedSpy } as unkn
import { createTables } from '../../../src/db/schema';
import { runMigrations } from '../../../src/db/migrations';
import { resetTestDb } from '../../helpers/test-db';
import { createUser, createTrip, createPlace, createCategory, createTag } from '../../helpers/factories';
import { createUser, createTrip, createPlace, createCategory, createTag, addTripMember } from '../../helpers/factories';
import path from 'path';
import fs from 'fs';
import { DatabaseService } from '../../../src/nest/database/database.service';
@@ -204,6 +204,33 @@ describe('create', () => {
expect(place.tags[0].id).toBe(tag.id);
});
// A tag belongs to a user, not a trip, and the place body is an open record, so
// an id from someone outside the trip could be attached and then read straight
// back — the tag projection carries the owner's user_id with it.
it('PLACE-SVC-008a — drops a tag owned by someone outside the trip', () => {
const { user } = createUser(testDb);
const { user: outsider } = createUser(testDb);
const trip = createTrip(testDb, user.id);
const mine = createTag(testDb, user.id, { name: 'Mine' }) as any;
const theirs = createTag(testDb, outsider.id, { name: 'Theirs' }) as any;
const place = svc.create(String(trip.id), { name: 'Tagged Place', tags: [mine.id, theirs.id] }) as any;
expect(place.tags.map((t: any) => t.id)).toEqual([mine.id]);
});
it('PLACE-SVC-008b — keeps a co-traveller tag, so a shared place survives a foreign re-save', () => {
const { user: owner } = createUser(testDb);
const { user: member } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
addTripMember(testDb, trip.id, member.id);
const theirTag = createTag(testDb, member.id, { name: 'Theirs' }) as any;
const place = svc.create(String(trip.id), { name: 'Shared Place', tags: [theirTag.id] }) as any;
expect(place.tags.map((t: any) => t.id)).toEqual([theirTag.id]);
});
it('PLACE-SVC-009 — place is associated with correct trip', () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
@@ -270,6 +270,35 @@ describe('getCategoryAssignees / updateCategoryAssignees', () => {
expect(assignees['Logistics']).toHaveLength(1);
});
// Holding todo_edit says you may edit this trip's list, not that any user id
// you name belongs to it. The read-back joins users and hands the caller a
// display name and an avatar, so an unfiltered id turns the endpoint into a
// lookup for every account on the instance.
it('TODO-SVC-017a: drops a user who is not on the trip, and says nothing about them', () => {
const { user: owner } = createUser(testDb);
const { user: member } = createUser(testDb);
const { user: stranger } = createUser(testDb);
const trip = createTrip(testDb, owner.id);
addTripMember(testDb, trip.id, member.id);
const rows = svc.updateCategoryAssignees(trip.id, 'Packing', [owner.id, stranger.id, member.id]) as { user_id: number }[];
expect(rows.map(r => r.user_id).sort()).toEqual([owner.id, member.id].sort());
expect(JSON.stringify(rows)).not.toContain(stranger.username);
expect(testDb.prepare('SELECT COUNT(*) AS n FROM todo_category_assignees WHERE user_id = ?').get(stranger.id)).toEqual({ n: 0 });
});
it('TODO-SVC-017b: keeps a guest, who is a trip member like any other', () => {
const { user: owner } = createUser(testDb);
const { user: guest } = createUser(testDb);
testDb.prepare('UPDATE users SET is_guest = 1 WHERE id = ?').run(guest.id);
const trip = createTrip(testDb, owner.id);
addTripMember(testDb, trip.id, guest.id);
const rows = svc.updateCategoryAssignees(trip.id, 'Packing', [guest.id]) as { user_id: number }[];
expect(rows.map(r => r.user_id)).toEqual([guest.id]);
});
it('TODO-SVC-020: updateCategoryAssignees replaces existing assignees (not append)', () => {
const { user: owner } = createUser(testDb);
const { user: member } = createUser(testDb);
+7 -4
View File
@@ -1,4 +1,4 @@
import { placeCategorySchema, placeRatingVoteSchema } from '../place/place.schema';
import { placeCategorySchema, placeImageUrlSchema, placeRatingVoteSchema, placeWebsiteSchema } from '../place/place.schema';
import { tagSchema } from '../tag/tag.schema';
import { z } from 'zod';
@@ -53,6 +53,9 @@ export const collectionPlaceSchema = z.object({
price: z.number().nullable().optional(),
currency: z.string().nullable().optional(),
notes: z.string().nullable().optional(),
// Deliberately open on the way out, pinned on the way in (see the save/update
// request schemas below): rows written before the request-side check existed
// still have to round-trip.
image_url: z.string().nullable().optional(),
google_place_id: z.string().nullable().optional(),
google_ftid: z.string().nullable().optional(),
@@ -150,11 +153,11 @@ export const collectionSavePlaceRequestSchema = z.object({
price: z.number().nullable().optional(),
currency: z.string().nullable().optional(),
notes: z.string().nullable().optional(),
image_url: z.string().nullable().optional(),
image_url: placeImageUrlSchema.nullable().optional(),
google_place_id: z.string().nullable().optional(),
google_ftid: z.string().nullable().optional(),
osm_id: z.string().nullable().optional(),
website: z.string().nullable().optional(),
website: placeWebsiteSchema.nullable().optional(),
phone: z.string().nullable().optional(),
status: collectionStatusSchema.optional(),
links: collectionLinksSchema.optional(),
@@ -203,7 +206,7 @@ export const collectionPlaceUpdateRequestSchema = z.object({
// Replace the place's per-collection label assignments (omit to leave unchanged).
label_ids: z.array(z.number()).optional(),
// Custom thumbnail (#1136): null clears it (falls back to the auto-fetched photo).
image_url: z.string().nullable().optional(),
image_url: placeImageUrlSchema.nullable().optional(),
});
export type CollectionPlaceUpdateRequest = z.infer<typeof collectionPlaceUpdateRequestSchema>;
+28
View File
@@ -19,6 +19,34 @@ const open = z.record(z.string(), z.unknown());
/** `#rgb` / `#rrggbb`, the form both map renderers and CSS accept. */
export const hexColorSchema = z.string().regex(/^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$/);
/**
* The four shapes a place thumbnail is ever allowed to take: a file we stored, a
* photo-proxy path, an inline thumbnail, or a remote https image.
*
* The map markers build their HTML as a string, so what lands here is one input
* away from a renderer that forgets to escape. Pinning the scheme on the way in
* means a stored value cannot carry anything a future consumer has to defend
* against — the escaping in the marker builders stops being the only thing
* standing between the database and the DOM.
*/
export const placeImageUrlSchema = z.string().max(2048).refine(
v =>
v.startsWith('/uploads/')
|| v.startsWith('/api/maps/place-photo/')
|| /^data:image\/(png|jpe?g|webp|gif|avif);base64,/i.test(v)
|| /^https:\/\//i.test(v),
{ message: 'must be an uploaded path, a photo-proxy path, an inline image or an https URL' },
);
/**
* A place's homepage. It reaches window.open() on the client, where a
* javascript: value would run in this origin rather than opening a page.
*/
export const placeWebsiteSchema = z.string().max(500).refine(
v => /^https?:\/\//i.test(v),
{ message: 'must be an http or https URL' },
);
/**
* Embedded category as returned on a place — a trimmed projection of the
* categories row (id/name/color/icon), built inline by placeService and