diff --git a/backend/src/services/db.service.ts b/backend/src/services/db.service.ts index 0da2109..d22f288 100644 --- a/backend/src/services/db.service.ts +++ b/backend/src/services/db.service.ts @@ -176,6 +176,49 @@ export interface RepetitionRule { // Add new entries at the END. Never edit or remove existing entries. // Convention for id: "YYYYMMDD_short_description" // --------------------------------------------------------------------------- +/** + * Per-table allowlists of columns the HTTP `PUT /artists|albums|tracks/:id` + * endpoints may write. + * + * These endpoints pass `request.body as any` straight through, and the update + * builders interpolate `Object.keys(body)` into the SET clause as quoted + * identifiers — so without an allowlist a crafted key both closes the quoted + * identifier (SQL injection) and mass-assigns columns that are not user-editable. + * + * Deliberately excluded: + * - `tracks.path` / `tracks.hash` / `tracks.mtime` — owned by the scanner; the + * path is the only link to the file on the read-only `/music` bind. + * - `tracks.state` / `quarantined_at` / `deleted_at` — owned by the dislike + * lifecycle and the integrity sweep, not by metadata editing. + * - `play_count` / `skip_count` / `dislike_count` / `last_played_at` — learning + * signal; forgeable counters would poison the recommendation engine. + * - `source_type` — decides library vs. recommendation semantics. + * - `id` / `created_at` / `updated_at` and every generated column + * (`normalized_name`, `normalized_artist` — Postgres rejects writes anyway). + */ +const UPDATABLE_COLUMNS = { + artists: ['name', 'canonical_name', 'sort_name', 'mbid', 'discogs_id', 'image_path'], + albums: ['artist_id', 'title', 'year', 'release_date', 'artwork_id', 'mbid'], + tracks: ['title', 'artist', 'album_id', 'duration', 'release_date'], +} as const; + +/** + * Filter a partial update payload down to the allowlisted, defined columns for + * `table`. Unknown keys are dropped silently rather than raising: the callers + * take `Partial` and the routes do no error mapping, so a 500 on an extra + * key would be worse than a no-op. The subsequent "No fields to update" throw + * still surfaces a payload that was *entirely* rejected. + */ +function allowedFields( + table: keyof typeof UPDATABLE_COLUMNS, + data: T +): (keyof T & string)[] { + const allowed = UPDATABLE_COLUMNS[table] as readonly string[]; + return Object.keys(data).filter( + (k) => allowed.includes(k) && (data as any)[k] !== undefined + ) as (keyof T & string)[]; +} + const MIGRATIONS: { id: string; sql: string }[] = [ { id: '20260608_track_artists', @@ -1244,7 +1287,7 @@ export class DbService { } async updateArtist(id: string, data: Partial): Promise { - const fields = Object.keys(data).filter(k => data[k as keyof Artist] !== undefined); + const fields = allowedFields('artists', data); if (fields.length === 0) throw new Error('No fields to update'); // Normalize name if it's being updated (the normalized_name generated column @@ -1277,7 +1320,7 @@ export class DbService { } async updateAlbum(id: string, data: Partial): Promise { - const fields = Object.keys(data).filter(k => data[k as keyof Album] !== undefined); + const fields = allowedFields('albums', data); if (fields.length === 0) throw new Error('No fields to update'); const setClause = fields.map((f, i) => `"${f}" = $${i + 2}`).join(', '); @@ -1320,7 +1363,7 @@ export class DbService { } async updateTrack(id: string, data: Partial): Promise { - const fields = Object.keys(data).filter(k => data[k as keyof Track] !== undefined); + const fields = allowedFields('tracks', data); if (fields.length === 0) throw new Error('No fields to update'); const setClause = fields.map((f, i) => `"${f}" = $${i + 2}`).join(', ');