diff --git a/HANDOFF.md b/HANDOFF.md new file mode 100644 index 0000000..c067c7d --- /dev/null +++ b/HANDOFF.md @@ -0,0 +1,55 @@ +# Handoff: Security + Search Resilience + +## Scope + +This branch contains local-only changes for review by the dev team. No deployment actions were taken. + +Branch: `codex/security-and-parse-fixes` + +## Change Sets + +### 1) Backend security + deterministic metadata parsing + +- `server/src/services/MIDIFetchService.ts` + - Added URL target validation to reduce SSRF risk: + - only `http/https` + - blocks embedded credentials + - blocks `localhost`/local hostnames + - blocks private/local/multicast IP targets (direct or DNS-resolved) +- `server/src/services/MIDIParseService.ts` + - Replaced placeholder/random parsing with deterministic MIDI track parsing. + - Extracts stable metadata: note count, basic track info, tempo/time-signature hints, duration estimate from ticks. + +### 2) BitMidi outage UX (clear retry message) + +- `server/src/adapters/BitMidiAdapter.ts` + - Propagates adapter failures instead of silently returning empty results. +- `server/src/services/MIDISearchService.ts` + - Distinguishes "all providers failed" from "no matches". +- `server/src/server.ts` + - Returns `503` + explicit message when MIDI source is unavailable. +- `src/services/MIDIService.ts` + - Surfaces backend JSON error messages to the frontend. + +## Build/Checks Run Locally + +- Frontend: + - `npm run typecheck` (pass) + - `npm run build` (pass) +- Backend: + - `npm run build` (pass) + +## Behavior to Verify in Staging + +1. Successful flow: + - search song -> results load -> select result -> preview/generate works. +2. Source outage flow: + - when BitMidi fails upstream, user sees: + - `BitMidi is temporarily unavailable. Please try again in a minute.` +3. SSRF guard: + - blocked URL example returns `403`: + - `/api/midi/fetch?u=http://127.0.0.1:3001/health` + +## Rollback + +Cherry-pick/merge by commit. If needed, revert individual commits cleanly. diff --git a/server/src/adapters/BitMidiAdapter.ts b/server/src/adapters/BitMidiAdapter.ts index e381938..ef05bc3 100644 --- a/server/src/adapters/BitMidiAdapter.ts +++ b/server/src/adapters/BitMidiAdapter.ts @@ -22,7 +22,7 @@ export class BitMidiAdapter implements SearchAdapter { return this.parseSearchResults(html, query); } catch (error) { console.error('BitMidi search error:', error); - return []; + throw error instanceof Error ? error : new Error('BitMidi search failed'); } } @@ -179,4 +179,4 @@ export class BitMidiAdapter implements SearchAdapter { // but keeping for backward compatibility return pageUrl; } -} \ No newline at end of file +} diff --git a/server/src/server.ts b/server/src/server.ts index 0e3cf86..9a29a55 100644 --- a/server/src/server.ts +++ b/server/src/server.ts @@ -147,6 +147,9 @@ app.get('/api/midi/search', async (req, res) => { res.json({ results, count: results.length }); } catch (error) { console.error('Search error:', error); + if (error instanceof Error && error.message === 'MIDI_SOURCE_UNAVAILABLE') { + return res.status(503).json({ error: 'BitMidi is temporarily unavailable. Please try again in a minute.' }); + } res.status(500).json({ error: 'Search failed' }); } }); @@ -284,7 +287,9 @@ app.get('/api/midi/fetch', async (req, res) => { res.setHeader('Content-Length', result.data!.byteLength); res.send(Buffer.from(result.data!)); } else { - res.status(404).json({ error: result.error }); + const error = String(result.error || 'Fetch failed'); + const blocked = /blocked|not allowed/i.test(error); + res.status(blocked ? 403 : 404).json({ error }); } } catch (error) { console.error('Fetch error:', error); @@ -307,7 +312,9 @@ app.get('/api/midi/parse', async (req, res) => { const metadata = parseService.parseMIDI(result.data); res.json(metadata); } else { - res.status(404).json({ error: result.error || 'Failed to fetch MIDI' }); + const error = String(result.error || 'Failed to fetch MIDI'); + const blocked = /blocked|not allowed/i.test(error); + res.status(blocked ? 403 : 404).json({ error }); } } catch (error) { console.error('Parse error:', error); @@ -328,4 +335,4 @@ if (!process.env.VERCEL) { app.listen(port, () => { console.log(`🎵 Motif backend running on port ${port}`); }); -} \ No newline at end of file +} diff --git a/server/src/services/MIDISearchService.ts b/server/src/services/MIDISearchService.ts index 9c74000..a098269 100644 --- a/server/src/services/MIDISearchService.ts +++ b/server/src/services/MIDISearchService.ts @@ -19,22 +19,28 @@ export class MIDISearchService { try { const results = await adapter.search(query); console.log(`${adapter.name}: Found ${results.length} results`); - return results; + return { ok: true, results } as const; } catch (error) { console.error(`${adapter.name} search failed:`, error); - return []; + return { ok: false, results: [] as MIDICandidate[] } as const; } }); const results = await Promise.all(searchPromises); + const allFailed = results.length > 0 && results.every(r => !r.ok); // Combine and deduplicate results for (const adapterResults of results) { - allResults.push(...adapterResults); + allResults.push(...adapterResults.results); } // Remove duplicates (same MIDI URL) const uniqueResults = this.deduplicateResults(allResults); + + // Distinguish provider outage from "no matches found" + if (uniqueResults.length === 0 && allFailed) { + throw new Error('MIDI_SOURCE_UNAVAILABLE'); + } // Sort by confidence score uniqueResults.sort((a, b) => b.confidence - a.confidence); @@ -59,4 +65,4 @@ export class MIDISearchService { return unique; } -} \ No newline at end of file +} diff --git a/server/src/types.ts b/server/src/types.ts index 61ba767..bdfd2f8 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -1,7 +1,7 @@ export interface MIDICandidate { id: string; title: string; - source: 'bitmidi' | 'dongrays' | 'mock'; + source: 'bitmidi' | 'dongrays' | 'freemidi' | 'mock'; pageUrl: string; midiUrl: string; confidence: number; @@ -38,4 +38,4 @@ export interface CacheEntry { filename: string; size: number; timestamp: number; -} \ No newline at end of file +} diff --git a/src/services/MIDIService.ts b/src/services/MIDIService.ts index b9cdecf..7f8f5d5 100644 --- a/src/services/MIDIService.ts +++ b/src/services/MIDIService.ts @@ -78,7 +78,16 @@ export class MIDIService { async search(query: string): Promise { const response = await this.fetchWithRetry(`${this.baseUrl}/api/midi/search?q=${encodeURIComponent(query)}`); - if (!response.ok) throw new Error(`Search failed: ${response.status}`); + if (!response.ok) { + let message = `Search failed: ${response.status}`; + try { + const body = await response.json() as { error?: string }; + if (body?.error) message = body.error; + } catch { + // ignore JSON parse errors and keep generic message + } + throw new Error(message); + } const data: MIDISearchResponse = await response.json(); return data.results; } @@ -121,4 +130,4 @@ export class MIDIService { return false; } } -} \ No newline at end of file +}