diff --git a/server/src/db/json-columns.ts b/server/src/db/json-columns.ts new file mode 100644 index 000000000..f9c0e22f6 --- /dev/null +++ b/server/src/db/json-columns.ts @@ -0,0 +1,90 @@ +import { z } from 'zod'; +import type { JsonColumn } from '../utils/json-column'; + +/** + * The JSON-in-TEXT columns the services decode, one declaration each: the shape + * the column holds and what a reader gets when a row is empty or broken. Read + * them with `decodeJson`/`decodeJsonResult` and write server-built values with + * `encodeJson` (`utils/json-column.ts`), never with a bare `JSON.parse` at the + * call site. + * + * These are storage shapes, not API contracts: `reservations.metadata` goes out + * as the stored string, and the scope lists are plain string arrays whose + * values the OAuth layer checks against its own scope table. + */ + +/** The free-form booking details (times, carrier, legs, the mirrored price). Old rows can hold a JSON string of the JSON. */ +export const RESERVATION_METADATA: JsonColumn> = { + column: 'reservations.metadata', + schema: z.record(z.string(), z.unknown()), + fallback: () => ({}), + unwrapDoubleEncoded: true, +}; + +const stringList = z.array(z.string()); + +/** The scopes an OAuth token was issued with. A broken row grants nothing. */ +export const OAUTH_TOKEN_SCOPES: JsonColumn = { + column: 'oauth_tokens.scopes', + schema: stringList, + fallback: () => [], +}; + +/** The scopes an OAuth client may ask for. A broken row allows nothing. */ +export const OAUTH_CLIENT_ALLOWED_SCOPES: JsonColumn = { + column: 'oauth_clients.allowed_scopes', + schema: stringList, + fallback: () => [], +}; + +/** The redirect URIs an OAuth client registered. A broken row matches none. */ +export const OAUTH_CLIENT_REDIRECT_URIS: JsonColumn = { + column: 'oauth_clients.redirect_uris', + schema: stringList, + fallback: () => [], +}; + +/** The scopes a user consented to for a client. A broken row counts as consent to nothing, so the user is asked again. */ +export const OAUTH_CONSENT_SCOPES: JsonColumn = { + column: 'oauth_consents.scopes', + schema: stringList, + fallback: () => [], +}; + +/** A limited API token's scopes; the reader keeps the strings it knows. */ +export const MCP_TOKEN_API_SCOPES: JsonColumn = { + column: 'mcp_tokens.api_scopes', + schema: z.array(z.unknown()), + fallback: () => [], +}; + +/** The hashed MFA backup codes; the reader keeps the strings. A broken row holds no codes. */ +export const USER_MFA_BACKUP_CODES: JsonColumn = { + column: 'users.mfa_backup_codes', + schema: z.array(z.unknown()), + fallback: () => [], +}; + +/** A journey entry's tags. */ +export const JOURNEY_ENTRY_TAGS: JsonColumn = { + column: 'journey_entries.tags', + schema: z.array(z.string()), + fallback: () => [], +}; + +/** + * A journey entry's pros and cons (`{ pros, cons }`). Checked as an object + * only: the lists inside go out as they were stored. + */ +export const JOURNEY_ENTRY_PROS_CONS: JsonColumn | null> = { + column: 'journey_entries.pros_cons', + schema: z.record(z.string(), z.unknown()).nullable(), + fallback: () => null, +}; + +/** A poll's options: plain labels, or `{ label }` objects from older rows. Checked as a list only. */ +export const COLLAB_POLL_OPTIONS: JsonColumn = { + column: 'collab_polls.options', + schema: z.array(z.unknown()), + fallback: () => [], +}; diff --git a/server/src/nest/accommodations/accommodations.service.ts b/server/src/nest/accommodations/accommodations.service.ts index 641c50cb8..f3310d553 100644 --- a/server/src/nest/accommodations/accommodations.service.ts +++ b/server/src/nest/accommodations/accommodations.service.ts @@ -24,6 +24,8 @@ import { BudgetItems } from '../../db/entities/BudgetItems.entity'; import type { BudgetItemsRepository } from '../../db/repositories/BudgetItems.repository'; import { carryViasWith, locatedStopIdsWith, reseatOwnStopWith, seatHolds, seatIndexAmong, seatIndexWith, type Night } from './night-seat'; import type { User } from '../../types'; +import { RESERVATION_METADATA } from '../../db/json-columns'; +import { decodeJson, encodeJson } from '../../utils/json-column'; type Trip = TripAccess; @@ -587,7 +589,7 @@ export class AccommodationsService { confirmation_number: confirmation || null, notes: notes || null, accommodation_id: legacyBoundIntegerText(newId), - metadata: Object.keys(meta).length > 0 ? JSON.stringify(meta) : null, + metadata: Object.keys(meta).length > 0 ? encodeJson(RESERVATION_METADATA, meta) : null, }); return { accommodationId: newId, mirror: await this.mirrorStay(newId, place_id ?? null, start_day_id, check_in) }; @@ -646,11 +648,11 @@ export class AccommodationsService { // would silently leave the others on the old times. const linkedRes = await this.reservationsRepo.listIdMetadataByStay(existing.id); for (const res of linkedRes) { - const meta = res.metadata ? JSON.parse(res.metadata) : {}; + const meta = decodeJson(RESERVATION_METADATA, res.metadata, `reservation ${res.id}`); if (newCheckIn) meta.check_in_time = newCheckIn; if (newCheckInEnd) meta.check_in_end_time = newCheckInEnd; if (newCheckOut) meta.check_out_time = newCheckOut; - await this.reservationsRepo.setMetadataAndConfirmation(res.id, JSON.stringify(meta), newConfirmation || null); + await this.reservationsRepo.setMetadataAndConfirmation(res.id, encodeJson(RESERVATION_METADATA, meta), newConfirmation || null); } return moved; }); diff --git a/server/src/nest/airports/airports.service.ts b/server/src/nest/airports/airports.service.ts index d9f4c5d7a..ab88a76f5 100644 --- a/server/src/nest/airports/airports.service.ts +++ b/server/src/nest/airports/airports.service.ts @@ -8,6 +8,8 @@ import { ReservationEndpoints } from '../../db/entities/ReservationEndpoints.ent import { ReservationEndpointsRepository } from '../../db/repositories/ReservationEndpoints.repository'; import { CronRegistrarService } from '../scheduling/cron-registrar.service'; import { UnitOfWork } from '../database/unit-of-work'; +import { RESERVATION_METADATA } from '../../db/json-columns'; +import { decodeJsonResult, logJsonFailure } from '../../utils/json-column'; /** * The in-container face of the airport dataset, plus the flight-endpoint @@ -96,8 +98,13 @@ export class AirportsService implements OnApplicationBootstrap { /** One flight's endpoints, or its review flag. True when it was filled. */ private async backfillOne(r: { id: number; metadata: string | null; reservation_time: string | null; reservation_end_time: string | null }): Promise { if (!r.metadata) { await this.reservationsRepo.markNeedsReview(r.id); return false; } - let meta: any; - try { meta = JSON.parse(r.metadata); } catch { await this.reservationsRepo.markNeedsReview(r.id); return false; } + const decoded = decodeJsonResult(RESERVATION_METADATA, r.metadata); + if (!decoded.ok) { + logJsonFailure(RESERVATION_METADATA, decoded.reason, `reservation ${r.id}`); + await this.reservationsRepo.markNeedsReview(r.id); + return false; + } + const meta = decoded.value; const dep = meta.departure_airport ? findByIata(String(meta.departure_airport).slice(0, 3)) : null; const arr = meta.arrival_airport ? findByIata(String(meta.arrival_airport).slice(0, 3)) : null; diff --git a/server/src/nest/auth/auth.helpers.ts b/server/src/nest/auth/auth.helpers.ts index f40df04b2..bdafd31fd 100644 --- a/server/src/nest/auth/auth.helpers.ts +++ b/server/src/nest/auth/auth.helpers.ts @@ -2,6 +2,8 @@ import bcrypt from 'bcryptjs'; import crypto from 'crypto'; import { User } from '../../types'; import { utcSuffix } from '../../db/types'; +import { USER_MFA_BACKUP_CODES } from '../../db/json-columns'; +import { decodeJson } from '../../utils/json-column'; import { decrypt_api_key } from '../common/crypto/apiKeyCrypto'; /** @@ -140,11 +142,5 @@ export function generateBackupCodes(count = MFA_BACKUP_CODE_COUNT): string[] { } export function parseBackupCodeHashes(raw: string | null | undefined): string[] { - if (!raw) return []; - try { - const parsed = JSON.parse(raw); - return Array.isArray(parsed) ? parsed.filter(v => typeof v === 'string') : []; - } catch { - return []; - } + return decodeJson(USER_MFA_BACKUP_CODES, raw).filter((v): v is string => typeof v === 'string'); } diff --git a/server/src/nest/budget/budget.service.ts b/server/src/nest/budget/budget.service.ts index 7c36f54dc..c561279cf 100644 --- a/server/src/nest/budget/budget.service.ts +++ b/server/src/nest/budget/budget.service.ts @@ -26,6 +26,8 @@ import { Trips } from '../../db/entities/Trips.entity'; import type { TripsRepository, TripAccess } from '../../db/repositories/Trips.repository'; import { TripMembers } from '../../db/entities/TripMembers.entity'; import type { TripMembersRepository } from '../../db/repositories/TripMembers.repository'; +import { RESERVATION_METADATA } from '../../db/json-columns'; +import { decodeJson, encodeJson } from '../../utils/json-column'; type Trip = TripAccess; @@ -926,19 +928,16 @@ export class BudgetService { await this.syncReservationPrice(String(tripId), reservationId, total, socketId, allInTripCurrency ? null : codeOf(linked[0].currency) || null); } - /** - * The counterpart to syncReservationPrice: take the mirrored total back off - * the booking. Non-fatal, exactly like the writer. - */ + /** The counterpart to syncReservationPrice: take the mirrored total back off the booking. Non-fatal, like the writer. */ private async clearReservationPrice(tripId: string | number, reservationId: number): Promise { try { const reservation = await this.reservationsRepo.getIdAndMetadata(reservationId, tripId); if (!reservation?.metadata) return; - const meta = JSON.parse(reservation.metadata); - if (!meta || typeof meta !== 'object' || meta.price === undefined) return; + const meta = decodeJson(RESERVATION_METADATA, reservation.metadata, `reservation ${reservation.id}`); + if (meta.price === undefined) return; delete meta.price; delete meta.priceCurrency; - await this.reservationsRepo.setMetadata(reservation.id, JSON.stringify(meta)); + await this.reservationsRepo.setMetadata(reservation.id, encodeJson(RESERVATION_METADATA, meta)); const updatedRes = await this.reservationsRepo.getFull(reservation.id); this.realtime.broadcast(String(tripId), 'reservation:updated', { reservation: updatedRes }, undefined); } catch (err) { @@ -1637,7 +1636,7 @@ export class BudgetService { try { const reservation = await this.reservationsRepo.getIdAndMetadata(reservationId, tripId); if (!reservation) return; - const meta = reservation.metadata ? JSON.parse(reservation.metadata) : {}; + const meta = decodeJson(RESERVATION_METADATA, reservation.metadata, `reservation ${reservation.id}`); // Cent-clean, so a booking never inherits float noise from the expense // it is linked to — and so a row stamped before #1964 heals on the next // edit. The panels print this string as it stands. @@ -1648,7 +1647,7 @@ export class BudgetService { if (currency) meta.priceCurrency = currency.toUpperCase(); else delete meta.priceCurrency; } - await this.reservationsRepo.setMetadata(reservation.id, JSON.stringify(meta)); + await this.reservationsRepo.setMetadata(reservation.id, encodeJson(RESERVATION_METADATA, meta)); const updatedRes = await this.reservationsRepo.getFull(reservation.id); this.realtime.broadcast(tripId, 'reservation:updated', { reservation: updatedRes }, socketId); } catch (err) { diff --git a/server/src/nest/calendar/calendar.service.ts b/server/src/nest/calendar/calendar.service.ts index 5961d5688..471ea803a 100644 --- a/server/src/nest/calendar/calendar.service.ts +++ b/server/src/nest/calendar/calendar.service.ts @@ -12,6 +12,8 @@ import { DayNotes } from '../../db/entities/DayNotes.entity'; import type { DayNotesRepository } from '../../db/repositories/DayNotes.repository'; import { Reservations } from '../../db/entities/Reservations.entity'; import type { ReservationsRepository } from '../../db/repositories/Reservations.repository'; +import { RESERVATION_METADATA } from '../../db/json-columns'; +import { decodeJson } from '../../utils/json-column'; /** The VCALENDAR preamble every TREK calendar starts with, single-trip or merged. */ export const CALENDAR_HEADER = @@ -613,7 +615,7 @@ export class CalendarService { interface LegTimes { from: string | null; to: string | null; label: string | null; confirmation: string | null; dep: { date: string; time: string; zone: string | null }; arr: { date: string; time: string; zone: string | null } | null } const legsOf = (r: any): LegTimes[] | null => { if (r.type !== 'flight' && r.type !== 'train' && r.type !== 'cruise') return null; - const meta = r.metadata ? (typeof r.metadata === 'string' ? JSON.parse(r.metadata) : r.metadata) : {}; + const meta = decodeJson(RESERVATION_METADATA, r.metadata, `reservation ${r.id}`); const legs = Array.isArray(meta.legs) ? meta.legs : []; if (legs.length < 2) return null; const eps = endpointsMap.get(r.id); @@ -681,7 +683,7 @@ export class CalendarService { // lines: dropping the block must not drop the route or the confirmation with // it (#2068). function describeReservation(r: any): string { - const meta = r.metadata ? (typeof r.metadata === 'string' ? JSON.parse(r.metadata) : r.metadata) : {}; + const meta = decodeJson(RESERVATION_METADATA, r.metadata, `reservation ${r.id}`); let desc = r.type ? `Type: ${r.type}` : ''; if (r.confirmation_number) desc += `\nConfirmation: ${r.confirmation_number}`; if (meta.airline) desc += `\nAirline: ${meta.airline}`; diff --git a/server/src/nest/collab/collab.service.ts b/server/src/nest/collab/collab.service.ts index e27c88931..3095cec28 100644 --- a/server/src/nest/collab/collab.service.ts +++ b/server/src/nest/collab/collab.service.ts @@ -28,6 +28,8 @@ import { CollabMessages } from '../../db/entities/CollabMessages.entity'; import type { CollabMessagesRepository, CollabMessageJoinRow } from '../../db/repositories/CollabMessages.repository'; import { Trips } from '../../db/entities/Trips.entity'; import type { TripsRepository } from '../../db/repositories/Trips.repository'; +import { COLLAB_POLL_OPTIONS } from '../../db/json-columns'; +import { decodeJson } from '../../utils/json-column'; type Trip = TripAccess; @@ -366,7 +368,7 @@ export class CollabService { const poll = await this.pollsRepo.findWithUser(pollId); if (!poll) return null; - const options: (string | { label: string })[] = JSON.parse(poll.options); + const options = decodeJson(COLLAB_POLL_OPTIONS, poll.options, `poll ${poll.id}`) as (string | { label: string })[]; const votes = await this.pollVotesRepo.listForPoll(poll.id); @@ -419,7 +421,7 @@ export class CollabService { if (!poll) return { error: 'not_found' }; if (poll.closed) return { error: 'closed' }; - const options = JSON.parse(poll.options); + const options = decodeJson(COLLAB_POLL_OPTIONS, poll.options, `poll ${poll.id}`); if (!Number.isInteger(optionIndex) || optionIndex < 0 || optionIndex >= options.length) { return { error: 'invalid_index' }; } diff --git a/server/src/nest/integrations/airtrail-import.service.ts b/server/src/nest/integrations/airtrail-import.service.ts index 01d69891b..23833402c 100644 --- a/server/src/nest/integrations/airtrail-import.service.ts +++ b/server/src/nest/integrations/airtrail-import.service.ts @@ -14,6 +14,8 @@ import { AirtrailRequestError, type AirtrailFlightRaw } from './airtrail.client' import { AirtrailClient } from './airtrail.client'; import { AirtrailService } from './airtrail.service'; import { canonicalHash, mapFlightToReservation, mapFlightsToMultiLegReservation, normalizeFlight } from './airtrail.mapper'; +import { RESERVATION_METADATA } from '../../db/json-columns'; +import { decodeJson } from '../../utils/json-column'; interface ExistingFlightRow { id: number; @@ -165,12 +167,8 @@ export class AirtrailImportService { const linkedRows = await this.reservationsRepo.listAirtrailLinkedForTrip(tripIdNum); for (const row of linkedRows) { if (row.external_id) linkedIds.add(row.external_id); - try { - const ids = row.metadata ? JSON.parse(row.metadata).airtrail_ids : null; - if (Array.isArray(ids)) for (const id of ids) linkedIds.add(String(id)); - } catch { - /* malformed metadata — ignore */ - } + const ids = decodeJson(RESERVATION_METADATA, row.metadata, `airtrail ${row.external_id}`).airtrail_ids; + if (Array.isArray(ids)) for (const id of ids) linkedIds.add(String(id)); } const existing = await this.reservationsRepo.listFlightReservationsForTrip(tripIdNum); @@ -194,12 +192,7 @@ export class AirtrailImportService { const existingSigs = new Set(); for (const row of existing) { - let meta: Record = {}; - try { - meta = row.metadata ? JSON.parse(row.metadata) : {}; - } catch { - /* malformed metadata — ignore */ - } + const meta = decodeJson(RESERVATION_METADATA, row.metadata, `reservation ${row.id}`); const eps = endpointsByReservation.get(row.id) ?? []; const legs: any[] | null = Array.isArray(meta.legs) ? meta.legs : null; if (legs && legs.length > 1) { @@ -218,7 +211,7 @@ export class AirtrailImportService { } else { const from = eps[0]?.code ?? null; const to = eps.length > 1 ? eps[eps.length - 1].code : null; - const sig = softSignature(depDate(row.reservation_time), meta.flight_number ?? null, from, to); + const sig = softSignature(depDate(row.reservation_time), (meta.flight_number as string | null | undefined) ?? null, from, to); if (sig) existingSigs.add(sig); } } diff --git a/server/src/nest/integrations/airtrail-link.service.ts b/server/src/nest/integrations/airtrail-link.service.ts index e4a54d876..362023406 100644 --- a/server/src/nest/integrations/airtrail-link.service.ts +++ b/server/src/nest/integrations/airtrail-link.service.ts @@ -16,6 +16,8 @@ import { AirtrailClient } from './airtrail.client'; import { AirtrailService } from './airtrail.service'; import { canonicalHash } from './airtrail.mapper'; import { buildSavePayload } from './airtrail-sync.helpers'; +import { RESERVATION_METADATA } from '../../db/json-columns'; +import { decodeJson } from '../../utils/json-column'; /** * The AirTrail link lifecycle — the enablement gate, the detach policy, the @@ -70,12 +72,9 @@ export class AirtrailLinkService { * AirTrail flight to span the whole route (#1535). */ async hasLocalMultiLegShape(reservationId: number, metadataJson: string | null | undefined): Promise { - try { - const meta = metadataJson ? JSON.parse(metadataJson) : {}; - if (Array.isArray(meta?.legs) && meta.legs.length > 1) return true; - } catch { - /* malformed metadata — fall through to the endpoint count */ - } + // Malformed metadata falls through to the endpoint count. + const legs = decodeJson(RESERVATION_METADATA, metadataJson, `reservation ${reservationId}`).legs; + if (Array.isArray(legs) && legs.length > 1) return true; const n = await this.endpointsRepo.count({ reservation: reservationId }); return n > 2; } diff --git a/server/src/nest/integrations/airtrail-sync.helpers.ts b/server/src/nest/integrations/airtrail-sync.helpers.ts index a174cbf79..370d0b5c8 100644 --- a/server/src/nest/integrations/airtrail-sync.helpers.ts +++ b/server/src/nest/integrations/airtrail-sync.helpers.ts @@ -1,6 +1,8 @@ import { entityCode } from './airtrail.mapper'; import { flightPassengers, ownPassenger } from './airtrail.client'; import type { AirtrailFlightRaw, AirtrailPassengerWrite, AirtrailSavePayload } from './airtrail.client'; +import { RESERVATION_METADATA } from '../../db/json-columns'; +import { decodeJson } from '../../utils/json-column'; /** * The pure half of the AirTrail push: turning a TREK reservation plus the flight @@ -27,12 +29,7 @@ function splitLocal(dt: string | null | undefined): { date: string | null; time: * (#1240). */ export function buildSavePayload(reservation: any, existing: AirtrailFlightRaw): AirtrailSavePayload | null { - let meta: Record; - try { - meta = reservation.metadata ? JSON.parse(reservation.metadata) : {}; - } catch { - meta = {}; - } + const meta = decodeJson(RESERVATION_METADATA, reservation.metadata, `reservation ${reservation.id}`); const endpoints: any[] = reservation.endpoints || []; const fromEp = endpoints.find((e) => e.role === 'from'); const toEp = endpoints.find((e) => e.role === 'to'); diff --git a/server/src/nest/journey/journey-entry-row.ts b/server/src/nest/journey/journey-entry-row.ts index 6e0116956..47eb37631 100644 --- a/server/src/nest/journey/journey-entry-row.ts +++ b/server/src/nest/journey/journey-entry-row.ts @@ -1,4 +1,6 @@ import type { JourneyEntry } from '../../types'; +import { JOURNEY_ENTRY_PROS_CONS, JOURNEY_ENTRY_TAGS } from '../../db/json-columns'; +import { decodeJson } from '../../utils/json-column'; /** * A journey entry on the way out. @@ -39,8 +41,8 @@ export function decodeEntryRow(row: JourneyEntry): JourneyEntryWire { const { tags, pros_cons, stats_excluded, dismissed, is_draft, ...rest } = row; return { ...rest, - tags: tags ? JSON.parse(tags) : [], - pros_cons: pros_cons ? JSON.parse(pros_cons) : null, + tags: decodeJson(JOURNEY_ENTRY_TAGS, tags, `journey entry ${row.id}`), + pros_cons: decodeJson(JOURNEY_ENTRY_PROS_CONS, pros_cons, `journey entry ${row.id}`) as JourneyEntryWire['pros_cons'], stats_excluded: !!stats_excluded, dismissed: !!dismissed, is_draft: !!is_draft, diff --git a/server/src/nest/oauth/oauth-public.controller.ts b/server/src/nest/oauth/oauth-public.controller.ts index 5a4c6b5b3..bf3921d21 100644 --- a/server/src/nest/oauth/oauth-public.controller.ts +++ b/server/src/nest/oauth/oauth-public.controller.ts @@ -6,6 +6,8 @@ import { getClientIp } from '../audit/client-ip'; import { logWarn } from '../audit/audit-log.logger'; import { AuditService } from '../audit/audit.service'; import { Public } from '../auth/public.decorator'; +import { OAUTH_CLIENT_ALLOWED_SCOPES } from '../../db/json-columns'; +import { decodeJson } from '../../utils/json-column'; const MIN = 60_000; @@ -112,7 +114,7 @@ export class OauthPublicController { res.status(400).json({ error: 'unauthorized_client', error_description: 'This client is not authorized for the client_credentials grant' }); return; } - const allowedScopes: string[] = JSON.parse(client.allowed_scopes); + const allowedScopes = decodeJson(OAUTH_CLIENT_ALLOWED_SCOPES, client.allowed_scopes, `client ${client_id}`); let grantedScopes: string[]; if (body.scope) { const requested = body.scope.split(' ').filter(Boolean); diff --git a/server/src/nest/oauth/oauth-sdk.provider.ts b/server/src/nest/oauth/oauth-sdk.provider.ts index 0475c35c4..7358d07e5 100644 --- a/server/src/nest/oauth/oauth-sdk.provider.ts +++ b/server/src/nest/oauth/oauth-sdk.provider.ts @@ -10,6 +10,8 @@ import { classifyRedirectUri } from './oauth.helpers'; import { AuditService } from '../audit/audit.service'; import { ALL_SCOPES, DEFAULT_CLIENT_SCOPES } from '../../mcp/scopes'; import { getMcpSafeUrl } from '../../app-config'; +import { OAUTH_CLIENT_ALLOWED_SCOPES, OAUTH_CLIENT_REDIRECT_URIS } from '../../db/json-columns'; +import { decodeJson } from '../../utils/json-column'; /** * TREK's adapters behind the MCP SDK's OAuth server interfaces, wrapping the @@ -48,8 +50,8 @@ function rowToInfo(row: NonNullable { const row = await this.consents.findScopes(clientId, userId); - return row ? JSON.parse(row.scopes) : null; + return row ? decodeJson(OAUTH_CONSENT_SCOPES, row.scopes, `client ${clientId} user ${userId}`) : null; } async saveConsent(clientId: string, userId: number, scopes: string[], ip?: string | null): Promise { // Union existing consent with newly approved scopes (M5: never narrow stored consent) const existing = (await this.getConsent(clientId, userId)) ?? []; const merged = Array.from(new Set([...existing, ...scopes])); - await this.consents.upsertGrant(clientId, userId, JSON.stringify(merged)); + await this.consents.upsertGrant(clientId, userId, encodeJson(OAUTH_CONSENT_SCOPES, merged)); await this.audit.writeAudit({ userId, action: 'oauth.consent.grant', details: { client_id: clientId, scopes: merged }, ip }); } @@ -327,7 +334,7 @@ export class OauthService { user_id: userId, access_token_hash: accessHash, refresh_token_hash: refreshHash, - scopes: JSON.stringify(scopes), + scopes: encodeJson(OAUTH_TOKEN_SCOPES, scopes), audience, access_token_expires_at: dbNow(accessExpiry), refresh_token_expires_at: dbNow(refreshExpiry), @@ -373,7 +380,7 @@ export class OauthService { user_id: userId, access_token_hash: accessHash, refresh_token_hash: placeholderHash, - scopes: JSON.stringify(scopes), + scopes: encodeJson(OAUTH_TOKEN_SCOPES, scopes), audience, access_token_expires_at: dbNow(accessExpiry), refresh_token_expires_at: dbNow(now), @@ -415,7 +422,7 @@ export class OauthService { return { user: { id: row.user_id, username: row.username, email: row.email, role: row.role as 'admin' | 'user' }, - scopes: JSON.parse(row.scopes), + scopes: decodeJson(OAUTH_TOKEN_SCOPES, row.scopes, `client ${row.client_id} user ${row.user_id}`), clientId: row.client_id, audience: row.audience ?? null, }; @@ -489,7 +496,7 @@ export class OauthService { // to take the whole chain down with it. Issue a sibling pair off the same // parent so each client walks away with its own token. if (await this.isConcurrentRotation(row)) { - const tokens = await this.issueTokens(clientId, row.user_id, JSON.parse(row.scopes), row.id, row.audience ?? null); + const tokens = await this.issueTokens(clientId, row.user_id, decodeJson(OAUTH_TOKEN_SCOPES, row.scopes, `token ${row.id}`), row.id, row.audience ?? null); await this.audit.writeAudit({ userId: row.user_id, action: 'oauth.token.refresh', @@ -529,7 +536,7 @@ export class OauthService { // to leave the old token revoked with no successor, logging the client out. const tokens = await this.uow.transactional(async () => { await this.tokens.revokeById(row.id); - return await this.issueTokens(clientId, row.user_id, JSON.parse(row.scopes), row.id, row.audience ?? null); + return await this.issueTokens(clientId, row.user_id, decodeJson(OAUTH_TOKEN_SCOPES, row.scopes, `token ${row.id}`), row.id, row.audience ?? null); }); await this.audit.writeAudit({ userId: row.user_id, action: 'oauth.token.refresh', details: { client_id: clientId }, ip }); @@ -565,7 +572,7 @@ export class OauthService { async listOAuthSessions(userId: number): Promise[]> { const rows = await this.tokens.listActiveByUser(userId); - return rows.map(r => ({ ...r, ...expiriesAsIso(r), scopes: JSON.parse(r.scopes) })); + return rows.map(r => ({ ...r, ...expiriesAsIso(r), scopes: decodeJson(OAUTH_TOKEN_SCOPES, r.scopes, `token ${r.id}`) })); } async revokeSession( @@ -628,7 +635,7 @@ export class OauthService { return { valid: false, error: 'invalid_client', error_description: 'Unknown client_id' }; } - const allowedUris: string[] = JSON.parse(client.redirect_uris); + const allowedUris = decodeJson(OAUTH_CLIENT_REDIRECT_URIS, client.redirect_uris, `client ${client.client_id}`); // Exact match except for the loopback port, which RFC 8252 §7.3 leaves to // the OS, and which the SDK's authorize handler already relaxes, so a // native client got a 302 to consent and an invalid_redirect_uri from this @@ -658,7 +665,7 @@ export class OauthService { return { valid: false, error: 'invalid_scope', error_description: 'At least one scope is required' }; } - const allowedScopes: string[] = JSON.parse(client.allowed_scopes); + const allowedScopes = decodeJson(OAUTH_CLIENT_ALLOWED_SCOPES, client.allowed_scopes, `client ${client.client_id}`); // Narrow to the intersection: drop scopes the client isn't permitted for rather // than rejecting the whole request (per OAuth 2.0 §3.3 scope narrowing). const grantedScopes = requestedScopes.filter(s => allowedScopes.includes(s)); @@ -728,12 +735,10 @@ export class OauthService { const rows = await this.tokens.listAllActiveWithClientAndUser(); // One malformed row must not 500 the whole admin OAuth-sessions panel. return rows.map((r) => { - let scopes: unknown; - try { - scopes = JSON.parse(r.scopes); - } catch { - scopes = null; - } + // The admin panel has always shown null for a row it cannot read. + const decoded = decodeJsonResult(OAUTH_TOKEN_SCOPES, r.scopes); + if (!decoded.ok && decoded.reason !== 'empty') logJsonFailure(OAUTH_TOKEN_SCOPES, decoded.reason, `token ${r.id}`); + const scopes = decoded.ok ? decoded.value : null; return { ...r, ...expiriesAsIso(r), scopes }; }); } diff --git a/server/src/nest/reservations/reservation-metadata.ts b/server/src/nest/reservations/reservation-metadata.ts index bcfc3cfd0..b3985ba32 100644 --- a/server/src/nest/reservations/reservation-metadata.ts +++ b/server/src/nest/reservations/reservation-metadata.ts @@ -1,3 +1,6 @@ +import { RESERVATION_METADATA } from '../../db/json-columns'; +import { decodeJsonResult, logJsonFailure } from '../../utils/json-column'; + /** * Metadata keys on a reservation that the booking forms do not own. * @@ -14,15 +17,10 @@ /** Read a stored metadata column into an object, or null if it is not one. */ function parseStored(stored: string | null | undefined): Record | null { - if (!stored) return null; - try { - const parsed: unknown = JSON.parse(stored); - return parsed && typeof parsed === 'object' && !Array.isArray(parsed) - ? (parsed as Record) - : null; - } catch { - return null; - } + const decoded = decodeJsonResult(RESERVATION_METADATA, stored); + if (decoded.ok) return decoded.value; + if (decoded.reason !== 'empty') logJsonFailure(RESERVATION_METADATA, decoded.reason); + return null; } /** diff --git a/server/src/nest/reservations/reservations.mcp.ts b/server/src/nest/reservations/reservations.mcp.ts index e71120a87..8e69e447b 100644 --- a/server/src/nest/reservations/reservations.mcp.ts +++ b/server/src/nest/reservations/reservations.mcp.ts @@ -15,6 +15,8 @@ import { findByIata } from '../airports/airports.data'; import type { EndpointInput } from './reservations.service'; import { AssignmentsService } from '../assignments/assignments.service'; import { transportLegInputSchema, reservationUrlSchema, type TransportLegInput } from '@trek/shared'; +import { RESERVATION_METADATA } from '../../db/json-columns'; +import { decodeJson } from '../../utils/json-column'; // What counts as a transport booking, for the update_transport gate. Every value // ReservationsPanel renders with a transport icon, so a stored `transit` row is @@ -138,14 +140,7 @@ type MetaRecord = Record; * still has to accept a legs update. */ function parseStoredMetadata(raw: unknown): MetaRecord { - if (typeof raw !== 'string' || !raw) return {}; - try { - let parsed: unknown = JSON.parse(raw); - if (typeof parsed === 'string') parsed = JSON.parse(parsed); - return typeof parsed === 'object' && parsed !== null ? parsed as MetaRecord : {}; - } catch { - return {}; - } + return typeof raw === 'string' ? decodeJson(RESERVATION_METADATA, raw) : {}; } interface LegPlan { diff --git a/server/src/nest/reservations/reservations.service.ts b/server/src/nest/reservations/reservations.service.ts index 260bd52c9..50a9a6fec 100644 --- a/server/src/nest/reservations/reservations.service.ts +++ b/server/src/nest/reservations/reservations.service.ts @@ -1,6 +1,6 @@ import { BadRequestException, Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; -import type { TripAccess } from '../../db/repositories/Trips.repository'; +import type { TripAccess, TripsRepository } from '../../db/repositories/Trips.repository'; import { UnitOfWork } from '../database/unit-of-work'; import type { TrekWsPayload, TrekWsTripEventName } from '@trek/shared'; import { RealtimeService } from '../realtime/realtime.service'; @@ -36,8 +36,9 @@ import type { UsersRepository } from '../../db/repositories/Users.repository'; import { Trips } from '../../db/entities/Trips.entity'; import { BudgetItems } from '../../db/entities/BudgetItems.entity'; import type { BudgetItemsRepository } from '../../db/repositories/BudgetItems.repository'; -import type { TripsRepository } from '../../db/repositories/Trips.repository'; import { appClock } from '../common/timezoneService'; +import { RESERVATION_METADATA } from '../../db/json-columns'; +import { decodeJson } from '../../utils/json-column'; type Trip = TripAccess; type BudgetEntry = { total_price?: number; category?: string; currency?: string | null; exchange_rate?: number } | undefined; @@ -787,7 +788,7 @@ export class ReservationsService { // whose accommodation was just auto-created above never received its // metadata check-in/out times or confirmation. if (resolvedAccommodationId && metadata) { - const meta = (typeof metadata === 'string' ? JSON.parse(metadata) : metadata) as AccommodationTimesMeta; + const meta = decodeJson(RESERVATION_METADATA, metadata) as AccommodationTimesMeta; if (meta.check_in_time || meta.check_in_end_time || meta.check_out_time) { await this.dayAccommodationsRepo.patchTimes( resolvedAccommodationId, meta.check_in_time || null, meta.check_in_end_time || null, meta.check_out_time || null, @@ -1036,9 +1037,9 @@ export class ReservationsService { } // Sync check-in/out to accommodation if linked - const resolvedMeta = nextMetadata !== undefined ? nextMetadata : (current.metadata ? JSON.parse(current.metadata as string) : null); + const resolvedMeta = nextMetadata !== undefined ? nextMetadata : (current.metadata ? decodeJson(RESERVATION_METADATA, current.metadata, `reservation ${idNum}`) : null); if (resolvedAccId && resolvedMeta) { - const meta = (typeof resolvedMeta === 'string' ? JSON.parse(resolvedMeta) : resolvedMeta) as AccommodationTimesMeta; + const meta = decodeJson(RESERVATION_METADATA, resolvedMeta) as AccommodationTimesMeta; const resolvedAccIdNum = accIdForRead(resolvedAccId)!; if (meta.check_in_time || meta.check_in_end_time || meta.check_out_time) { await this.dayAccommodationsRepo.patchTimes( diff --git a/server/src/nest/tokens/token.service.ts b/server/src/nest/tokens/token.service.ts index 70a1addec..c63217a50 100644 --- a/server/src/nest/tokens/token.service.ts +++ b/server/src/nest/tokens/token.service.ts @@ -17,6 +17,8 @@ import { toRowId } from '../common/row-id'; // every consumer of this module — a nest→mcp→nest module cycle. import { revokeUserSessions } from '../../mcp/sessionManager'; import { User } from '../../types'; +import { MCP_TOKEN_API_SCOPES } from '../../db/json-columns'; +import { decodeJson } from '../../utils/json-column'; /** * What a token is allowed to drive. Stored on the row so each surface can accept @@ -321,13 +323,7 @@ function sanitizeScopes(scopes: readonly string[] | undefined): PublicApiScope[] */ function resolveGrant(mode: string | null, raw: string | null): PublicApiGrant { if (mode !== 'limited') return { mode: 'all', scopes: [...PUBLIC_API_SCOPES] }; - let parsed: unknown; - try { - parsed = raw ? JSON.parse(raw) : null; - } catch { - parsed = null; - } - if (!Array.isArray(parsed)) return { mode: 'limited', scopes: [] }; + const parsed = decodeJson(MCP_TOKEN_API_SCOPES, raw); const kept = sanitizeScopes(parsed.filter((value): value is string => typeof value === 'string')); return { mode: 'limited', scopes: kept ?? [] }; } diff --git a/server/src/utils/json-column.ts b/server/src/utils/json-column.ts new file mode 100644 index 000000000..d3d36d069 --- /dev/null +++ b/server/src/utils/json-column.ts @@ -0,0 +1,74 @@ +import type { ZodType } from 'zod'; +import { logWarn } from '../nest/audit/audit-log.logger'; + +/** + * One JSON-in-TEXT column: where it lives, the shape it holds, and what a reader + * gets when the stored text is missing or broken. + * + * The storage model keeps JSON in TEXT (see server/CLAUDE.md, "SQL dialect"), and + * the service boundary turns it into a value. Declaring the column once keeps + * every reader of it on the same answer: one schema, one fallback, one log line + * when a row does not hold what it should, instead of a `JSON.parse` per call + * site that throws in one place and shrugs in the next. + */ +export interface JsonColumn { + /** `table.column`, for the log line. */ + readonly column: string; + readonly schema: ZodType; + /** What a reader gets for an empty column or a value that does not decode. A function, so a mutable default is fresh per call. */ + readonly fallback: () => T; + /** + * The stored text may be a JSON string of the JSON (an old double-encoding + * bug). When set, one level of that is unwrapped before the schema check. + */ + readonly unwrapDoubleEncoded?: boolean; +} + +export type JsonDecodeFailure = 'empty' | 'invalid-json' | 'schema'; + +export type JsonDecodeResult = + | { ok: true; value: T; reason?: never } + | { ok: false; value?: never; reason: JsonDecodeFailure }; + +/** + * Decode a stored value without a fallback, for the readers that react to a + * broken row themselves (flag it for review, skip it). A value a driver already + * handed back parsed (an object rather than text) is checked as it is. + */ +export function decodeJsonResult(col: JsonColumn, raw: unknown): JsonDecodeResult { + if (raw === null || raw === undefined || raw === '') return { ok: false, reason: 'empty' }; + let value: unknown = raw; + if (typeof raw === 'string') { + try { + value = JSON.parse(raw); + if (col.unwrapDoubleEncoded && typeof value === 'string') value = JSON.parse(value); + } catch { + return { ok: false, reason: 'invalid-json' }; + } + } + const parsed = col.schema.safeParse(value); + return parsed.success ? { ok: true, value: parsed.data } : { ok: false, reason: 'schema' }; +} + +/** + * Decode a stored value, falling back to the column's default. An empty column + * is ordinary and falls back quietly; text that is not JSON, or JSON of the + * wrong shape, falls back too and is logged with `context` (a row id, say), so + * broken data shows up in trek.log instead of disappearing. + */ +export function decodeJson(col: JsonColumn, raw: unknown, context?: string): T { + const result = decodeJsonResult(col, raw); + if (result.ok) return result.value; + if (result.reason !== 'empty') logJsonFailure(col, result.reason, context); + return col.fallback(); +} + +/** Log a row whose JSON did not decode; for callers of {@link decodeJsonResult} that handle the failure themselves. */ +export function logJsonFailure(col: JsonColumn, reason: JsonDecodeFailure, context?: string): void { + logWarn(`[json] ${col.column}${context ? ` (${context})` : ''}: stored value ${reason === 'schema' ? 'has the wrong shape' : 'is not valid JSON'}, using the fallback`); +} + +/** The text to store for a value, checked against the column's shape first (a mismatch throws: it is a bug in the writer). */ +export function encodeJson(col: JsonColumn, value: T): string { + return JSON.stringify(col.schema.parse(value)); +} diff --git a/server/tests/unit/nest/accommodations.service.test.ts b/server/tests/unit/nest/accommodations.service.test.ts index 29dbfbf44..72fa943d7 100644 --- a/server/tests/unit/nest/accommodations.service.test.ts +++ b/server/tests/unit/nest/accommodations.service.test.ts @@ -227,6 +227,23 @@ describe('updateAccommodation', () => { spy.mockRestore(); }); + it('DAY-SVC-023b — a linked booking with unreadable metadata no longer fails the update; it gets the new times', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const day = createDay(testDb, trip.id) as { id: number }; + const place = createPlace(testDb, trip.id, { name: 'Hotel' }) as { id: number }; + const { accommodation: accom } = (await svc.createAccommodation(trip.id, { + place_id: place.id, start_day_id: day.id, end_day_id: day.id, + })) as { accommodation: { id: number } }; + const before = await linkedReservation(accom.id); + await updateRows(await orm(), Reservations, { id: before.id }, { metadata: '{not json' }); + + const existing = (await svc.getAccommodation(accom.id, trip.id))!; + await svc.updateAccommodation(accom.id, existing as Parameters[1], { check_in: '16:00' }); + + expect(JSON.parse((await linkedReservation(accom.id)).metadata)).toEqual({ check_in_time: '16:00' }); + }); + it('DAY-SVC-023 — updates check-in and check-out times', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id); diff --git a/server/tests/unit/nest/budget.service.db.test.ts b/server/tests/unit/nest/budget.service.db.test.ts index 501285e93..61d11e682 100644 --- a/server/tests/unit/nest/budget.service.db.test.ts +++ b/server/tests/unit/nest/budget.service.db.test.ts @@ -974,6 +974,17 @@ describe('linking existing expenses to bookings and places (#2084)', () => { expect(await metadataOf(reservationId)).toEqual({ seat: '12A', price: '30.3', priceCurrency: 'USD' }); }); + it('BUDGET-SVC-DB-072b: a booking whose metadata is not JSON gets the mirrored price instead of a logged failure', async () => { + const { trip, reservationId } = await tripWithBooking(); + await updateRows(await orm(), Reservations, { id: reservationId }, { metadata: '{not json' }); + const item = await budget.createBudgetItem(trip.id, { name: 'Fare', total_price: 42, currency: 'usd' }); + await linkItems(reservationId, [item.id]); + + await budget.resyncReservationPrice(trip.id, reservationId); + + expect(await metadataOf(reservationId)).toEqual({ price: '42', priceCurrency: 'USD' }); + }); + it('BUDGET-SVC-DB-052: an expense without a currency and one in the trip currency by code add up', async () => { // No currency means the trip's; the two rows are in the same money and the // booking shows their sum, not the first one alone. diff --git a/server/tests/unit/nest/calendar.service.test.ts b/server/tests/unit/nest/calendar.service.test.ts index 66ba56400..4c8194d89 100644 --- a/server/tests/unit/nest/calendar.service.test.ts +++ b/server/tests/unit/nest/calendar.service.test.ts @@ -523,6 +523,18 @@ describe('exportICS', () => { expect(ics).not.toContain('DESCRIPTION'); }); + it('CAL-017b: a booking whose metadata is not JSON still exports, without the metadata lines', async () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id, { title: 'Broken' }); + const reservation = createReservation(testDb, trip.id, { title: 'Garbled Flight', type: 'flight' }); + await updateRows(orm, Reservations, { id: reservation.id }, { reservation_time: '2025-06-02T09:00', metadata: '{not json' }); + + const { ics } = await svc.exportICS(trip.id); + + expect(ics).toContain('SUMMARY:Garbled Flight'); + expect(ics).toContain('DESCRIPTION:Type: flight\r\n'); + }); + it('CAL-018: flight metadata with only one airport emits only that side of the route', async () => { const { user } = createUser(testDb); const trip = createTrip(testDb, user.id, { title: 'Half Routes' }); diff --git a/server/tests/unit/nest/collab.service.test.ts b/server/tests/unit/nest/collab.service.test.ts index f6faed5ec..b0e9f09a5 100644 --- a/server/tests/unit/nest/collab.service.test.ts +++ b/server/tests/unit/nest/collab.service.test.ts @@ -78,6 +78,7 @@ import { RateLimitService } from '../../../src/nest/common/rate-limit.service'; import { createTestUnitOfWork, createTestAppSettingsRepo, createTestTripsRepo, sharedTestOrm } from '../../helpers/test-uow'; import type { TestOrm } from '../../helpers/test-orm'; import { countRows, findRow, findRows, insertRow, updateRows } from '../../helpers/factories/rows'; +import { CollabPolls } from '../../../src/db/entities/CollabPolls.entity'; import { dbNow } from '../../../src/db/types/db-timestamp.type'; import { CollabMessageReactions } from '../../../src/db/entities/CollabMessageReactions.entity'; import { CollabMessages } from '../../../src/db/entities/CollabMessages.entity'; @@ -221,6 +222,16 @@ describe('votePoll', () => { expect(result.error).toBe('invalid_index'); }); + it('COLLAB-SVC-006b: a poll whose stored options are not JSON lists with no options and refuses every vote', async () => { + const { user1, trip } = setup(); + const poll = await svc.createPoll(trip.id, user1.id, { question: 'Q?', options: ['A', 'B'] }); + await updateRows(orm, CollabPolls, { id: poll!.id }, { options: '{broken' }); + + const [listed] = (await svc.listPolls(trip.id)) as Array<{ options: unknown[] }>; + expect(listed.options).toEqual([]); + expect((await svc.votePoll(trip.id, poll!.id, user1.id, 0)).error).toBe('invalid_index'); + }); + it('COLLAB-SVC-007: returns error "not_found" for nonexistent poll', async () => { const { user1, trip } = setup(); const result = await svc.votePoll(trip.id, 9999, user1.id, 0); diff --git a/server/tests/unit/nest/journey-entry-row.test.ts b/server/tests/unit/nest/journey-entry-row.test.ts new file mode 100644 index 000000000..712f0a63b --- /dev/null +++ b/server/tests/unit/nest/journey-entry-row.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it, vi } from 'vitest'; + +const logMock = vi.hoisted(() => ({ logInfo: vi.fn(), logError: vi.fn(), logWarn: vi.fn(), logDebug: vi.fn() })); +vi.mock('../../../src/nest/audit/audit-log.logger', () => logMock); + +import { decodeEntryRow } from '../../../src/nest/journey/journey-entry-row'; +import type { JourneyEntry } from '../../../src/types'; + +const row = (over: Partial = {}): JourneyEntry => + ({ + id: 5, + journey_id: 1, + author_id: 1, + type: 'entry', + entry_date: '2026-01-01', + visibility: 'private', + sort_order: 0, + tags: null, + pros_cons: null, + stats_excluded: 0, + dismissed: 0, + is_draft: 1, + ...over, + }) as JourneyEntry; + +describe('decodeEntryRow', () => { + it('JENTRYROW-001: decodes the JSON columns and turns the flags into booleans', () => { + const wire = decodeEntryRow(row({ tags: '["beach","food"]', pros_cons: '{"pros":["sun"],"cons":["crowds"]}' })); + expect(wire.tags).toEqual(['beach', 'food']); + expect(wire.pros_cons).toEqual({ pros: ['sun'], cons: ['crowds'] }); + expect(wire.stats_excluded).toBe(false); + expect(wire.dismissed).toBe(false); + expect(wire.is_draft).toBe(true); + }); + + it('JENTRYROW-002: empty columns are an empty tag list and no pros/cons', () => { + const wire = decodeEntryRow(row()); + expect(wire.tags).toEqual([]); + expect(wire.pros_cons).toBeNull(); + expect(logMock.logWarn).not.toHaveBeenCalled(); + }); + + it('JENTRYROW-003: unreadable JSON no longer throws: it reads as empty and is logged', () => { + const wire = decodeEntryRow(row({ tags: '{oops', pros_cons: 'not json' })); + expect(wire.tags).toEqual([]); + expect(wire.pros_cons).toBeNull(); + expect(logMock.logWarn).toHaveBeenCalledWith('[json] journey_entries.tags (journey entry 5): stored value is not valid JSON, using the fallback'); + }); +}); diff --git a/server/tests/unit/nest/oauth.service.test.ts b/server/tests/unit/nest/oauth.service.test.ts index e73032532..d03fe18cf 100644 --- a/server/tests/unit/nest/oauth.service.test.ts +++ b/server/tests/unit/nest/oauth.service.test.ts @@ -1419,4 +1419,28 @@ describe('admin OAuth sessions', () => { expect(sessions).toHaveLength(1); expect(sessions[0].scopes).toBeNull(); }); + + it('a row with malformed scopes no longer fails the user list, the token check or the refresh: it grants nothing', async () => { + const { user } = createUser(testDb); + const created = await makeClient(user.id); + const client = created.client as { client_id: string; client_secret: string }; + const tokens = await issueTokens(client.client_id, user.id, ['trips:read']); + await updateRows(t, OauthTokens, { user: user.id }, { scopes: 'not-json{' }); + + const [session] = await listOAuthSessions(user.id); + expect(session.scopes).toEqual([]); + expect((await getUserByAccessToken(tokens.access_token))?.scopes).toEqual([]); + }); + + it('a malformed consent row counts as consent to nothing, and a new grant replaces it', async () => { + const { user } = createUser(testDb); + const created = await makeClient(user.id); + const clientId = created.client!.client_id as string; + await saveConsent(clientId, user.id, ['trips:read']); + await updateRows(t, OauthConsents, { user: user.id }, { scopes: '{broken' }); + + expect(await getConsent(clientId, user.id)).toEqual([]); + await saveConsent(clientId, user.id, ['trips:write']); + expect(await getConsent(clientId, user.id)).toEqual(['trips:write']); + }); }); diff --git a/server/tests/unit/utils/json-column.test.ts b/server/tests/unit/utils/json-column.test.ts new file mode 100644 index 000000000..c5f936548 --- /dev/null +++ b/server/tests/unit/utils/json-column.test.ts @@ -0,0 +1,118 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { z } from 'zod'; + +const logMock = vi.hoisted(() => ({ logInfo: vi.fn(), logError: vi.fn(), logWarn: vi.fn(), logDebug: vi.fn() })); +vi.mock('../../../src/nest/audit/audit-log.logger', () => logMock); + +import { decodeJson, decodeJsonResult, encodeJson, logJsonFailure, type JsonColumn } from '../../../src/utils/json-column'; +import { + COLLAB_POLL_OPTIONS, + JOURNEY_ENTRY_PROS_CONS, + JOURNEY_ENTRY_TAGS, + MCP_TOKEN_API_SCOPES, + OAUTH_CLIENT_ALLOWED_SCOPES, + OAUTH_CLIENT_REDIRECT_URIS, + OAUTH_CONSENT_SCOPES, + OAUTH_TOKEN_SCOPES, + RESERVATION_METADATA, + USER_MFA_BACKUP_CODES, +} from '../../../src/db/json-columns'; + +const LIST: JsonColumn = { column: 't.list', schema: z.array(z.string()), fallback: () => [] }; + +beforeEach(() => vi.clearAllMocks()); + +describe('decodeJsonResult', () => { + it('JSONCOL-001: decodes text of the right shape', () => { + expect(decodeJsonResult(LIST, '["a","b"]')).toEqual({ ok: true, value: ['a', 'b'] }); + }); + + it('JSONCOL-002: names why a value did not decode', () => { + expect(decodeJsonResult(LIST, null)).toEqual({ ok: false, reason: 'empty' }); + expect(decodeJsonResult(LIST, undefined)).toEqual({ ok: false, reason: 'empty' }); + expect(decodeJsonResult(LIST, '')).toEqual({ ok: false, reason: 'empty' }); + expect(decodeJsonResult(LIST, '{not json')).toEqual({ ok: false, reason: 'invalid-json' }); + expect(decodeJsonResult(LIST, '{"a":1}')).toEqual({ ok: false, reason: 'schema' }); + }); + + it('JSONCOL-003: checks a value a driver handed back already parsed', () => { + expect(decodeJsonResult(LIST, ['x'])).toEqual({ ok: true, value: ['x'] }); + expect(decodeJsonResult(LIST, { a: 1 })).toEqual({ ok: false, reason: 'schema' }); + }); + + it('JSONCOL-004: unwraps one level of double encoding only where the column says so', () => { + const doubled = JSON.stringify(JSON.stringify({ price: '12' })); + expect(decodeJsonResult(RESERVATION_METADATA, doubled)).toEqual({ ok: true, value: { price: '12' } }); + expect(decodeJsonResult(LIST, JSON.stringify(JSON.stringify(['a'])))).toEqual({ ok: false, reason: 'schema' }); + expect(decodeJsonResult(RESERVATION_METADATA, JSON.stringify('{broken'))).toEqual({ ok: false, reason: 'invalid-json' }); + }); +}); + +describe('decodeJson', () => { + it('JSONCOL-010: returns the decoded value without logging', () => { + expect(decodeJson(LIST, '["a"]')).toEqual(['a']); + expect(logMock.logWarn).not.toHaveBeenCalled(); + }); + + it('JSONCOL-011: an empty column falls back quietly', () => { + expect(decodeJson(LIST, null)).toEqual([]); + expect(logMock.logWarn).not.toHaveBeenCalled(); + }); + + it('JSONCOL-012: broken text and the wrong shape fall back and are logged with the column and context', () => { + expect(decodeJson(LIST, '{not json', 'row 7')).toEqual([]); + expect(logMock.logWarn).toHaveBeenLastCalledWith('[json] t.list (row 7): stored value is not valid JSON, using the fallback'); + expect(decodeJson(LIST, '{"a":1}')).toEqual([]); + expect(logMock.logWarn).toHaveBeenLastCalledWith('[json] t.list: stored value has the wrong shape, using the fallback'); + }); + + it('JSONCOL-013: every fallback is a fresh value', () => { + const a = decodeJson(RESERVATION_METADATA, null); + a.price = '1'; + expect(decodeJson(RESERVATION_METADATA, null)).toEqual({}); + }); +}); + +describe('logJsonFailure / encodeJson', () => { + it('JSONCOL-020: logs the column, the context and the reason', () => { + logJsonFailure(LIST, 'schema'); + expect(logMock.logWarn).toHaveBeenCalledWith('[json] t.list: stored value has the wrong shape, using the fallback'); + }); + + it('JSONCOL-021: encodes a value of the right shape and refuses another', () => { + expect(encodeJson(LIST, ['a', 'b'])).toBe('["a","b"]'); + expect(() => encodeJson(LIST, [1] as unknown as string[])).toThrow(); + }); +}); + +describe('the declared columns', () => { + it('JSONCOL-030: reservation metadata is an object, and anything else reads as an empty one', () => { + expect(decodeJson(RESERVATION_METADATA, '{"airline":"LH","legs":[1,2]}')).toEqual({ airline: 'LH', legs: [1, 2] }); + expect(decodeJson(RESERVATION_METADATA, '[1,2]')).toEqual({}); + expect(decodeJson(RESERVATION_METADATA, 'null')).toEqual({}); + expect(decodeJson(RESERVATION_METADATA, '{oops')).toEqual({}); + }); + + it('JSONCOL-031: the OAuth lists fail closed to an empty list', () => { + for (const col of [OAUTH_TOKEN_SCOPES, OAUTH_CLIENT_ALLOWED_SCOPES, OAUTH_CLIENT_REDIRECT_URIS, OAUTH_CONSENT_SCOPES]) { + expect(decodeJson(col, '["trips:read"]')).toEqual(['trips:read']); + expect(decodeJson(col, 'garbage')).toEqual([]); + expect(decodeJson(col, '[1]')).toEqual([]); + } + }); + + it('JSONCOL-032: the lenient lists keep whatever list is stored', () => { + expect(decodeJson(MCP_TOKEN_API_SCOPES, '["a",1]')).toEqual(['a', 1]); + expect(decodeJson(USER_MFA_BACKUP_CODES, '["h1",null]')).toEqual(['h1', null]); + expect(decodeJson(COLLAB_POLL_OPTIONS, '["Yes",{"label":"No"}]')).toEqual(['Yes', { label: 'No' }]); + expect(decodeJson(COLLAB_POLL_OPTIONS, '{"a":1}')).toEqual([]); + }); + + it('JSONCOL-033: journey entry tags and pros/cons', () => { + expect(decodeJson(JOURNEY_ENTRY_TAGS, '["beach"]')).toEqual(['beach']); + expect(decodeJson(JOURNEY_ENTRY_TAGS, '{bad')).toEqual([]); + expect(decodeJson(JOURNEY_ENTRY_PROS_CONS, '{"pros":["sun"],"cons":[]}')).toEqual({ pros: ['sun'], cons: [] }); + expect(decodeJson(JOURNEY_ENTRY_PROS_CONS, 'null')).toBeNull(); + expect(decodeJson(JOURNEY_ENTRY_PROS_CONS, '{bad')).toBeNull(); + }); +});