ux: return clear BitMidi outage message and handoff notes
This commit is contained in:
+55
@@ -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.
|
||||
@@ -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');
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -19,23 +19,29 @@ 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);
|
||||
|
||||
|
||||
+1
-1
@@ -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;
|
||||
|
||||
@@ -78,7 +78,16 @@ export class MIDIService {
|
||||
|
||||
async search(query: string): Promise<MIDISearchResult[]> {
|
||||
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;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user