fix: address CodeRabbit review comments for PR #77

- Add SSRF validation to TTS route with baseUrl allowlist
- Fix concurrency.ts: validate maxConcurrent >= 1, fill aborted slots
- Add .env existence check to shell script with error messaging
- Add language identifier to README code block (markdownlint MD040)
- Add input validation and 30s timeout to groq-tts route
- Preserve previous summary on error in SummarizeModal
- Pass maxLength parameter through to summarize API
- Fix unused docType parameter in dexie.ts getSummary
- Reject whitespace-only text in summarize API

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

scripts/README.md

@@ -19,7 +19,7 @@ http://localhost:3003
 
 ## Architecture
 
-```
+```text
 OpenReader WebUI (:3003)
         ↓
 Built-in /api/groq-tts route

scripts/openreader-webui.sh

@@ -10,14 +10,20 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 PROJECT_DIR="$(dirname "$SCRIPT_DIR")"
 
 # Load GROQ_API_KEY
-source "$SCRIPT_DIR/.env"
+if [ -f "$SCRIPT_DIR/.env" ]; then
-export GROQ_API_KEY
+    source "$SCRIPT_DIR/.env"
+    export GROQ_API_KEY
+else
+    echo "ERROR: $SCRIPT_DIR/.env file not found"
+    echo "Please create the .env file with your GROQ_API_KEY"
+    exit 1
+fi
 
 # Stop existing service
 fuser -k 3003/tcp 2>/dev/null || true
 
 # Wait for port to be released
-for i in {1..10}; do
+for _ in {1..10}; do
     if ! fuser 3003/tcp 2>/dev/null; then
         break
     fi

src/app/api/groq-tts/route.ts

@@ -17,6 +17,14 @@ export async function POST(req: NextRequest) {
 
     const body = await req.json();
 
+    // Validate input text
+    if (!body.input || typeof body.input !== 'string' || !body.input.trim()) {
+      return NextResponse.json(
+        { error: 'Missing or empty input text' },
+        { status: 400 }
+      );
+    }
+
     // Set default model if not provided or not a canopylabs model
     let model = body.model || '';
     if (!model.startsWith('canopylabs/')) {
@@ -36,6 +44,10 @@ export async function POST(req: NextRequest) {
       response_format: responseFormat,
     };
 
+    // Create abort controller with 30-second timeout
+    const controller = new AbortController();
+    const timeoutId = setTimeout(() => controller.abort(), 30000);
+
     const response = await fetch(`${GROQ_BASE}/audio/speech`, {
       method: 'POST',
       headers: {
@@ -43,12 +55,21 @@ export async function POST(req: NextRequest) {
         'Authorization': `Bearer ${apiKey}`,
       },
       body: JSON.stringify(groqBody),
+      signal: controller.signal,
     });
 
+    clearTimeout(timeoutId);
+
     if (!response.ok) {
       const errorText = await response.text();
+      console.error('Groq API error:', errorText);
+      // Return sanitized error message to avoid exposing internal details
+      const sanitizedError = response.status === 401 ? 'Invalid API key' :
+                            response.status === 429 ? 'Rate limit exceeded' :
+                            response.status >= 500 ? 'Groq service error' :
+                            'Failed to generate speech';
       return NextResponse.json(
-        { error: errorText },
+        { error: sanitizedError },
         { status: response.status }
       );
     }
@@ -64,8 +85,15 @@ export async function POST(req: NextRequest) {
     });
   } catch (error) {
     console.error('Groq TTS error:', error);
+    // Handle timeout specifically
+    if (error instanceof Error && error.name === 'AbortError') {
+      return NextResponse.json(
+        { error: 'Request timed out' },
+        { status: 504 }
+      );
+    }
     return NextResponse.json(
-      { error: error instanceof Error ? error.message : 'Failed to generate speech' },
+      { error: 'Failed to generate speech' },
       { status: 500 }
     );
   }

src/app/api/summarize/route.ts

@@ -145,7 +145,7 @@ export async function POST(req: NextRequest) {
 
     console.log('Received summarize request:', { provider, modelId, mode, textLength: text?.length, isChunk, isFinalPass });
 
-    if (!text) {
+    if (!text || !text.trim()) {
       const errorBody: SummarizeError = {
         code: 'MISSING_TEXT',
         message: 'No text provided for summarization',

src/app/api/tts/route.ts

@@ -17,6 +17,72 @@ type AudioBufferValue = TTSAudioBuffer;
 const TTS_CACHE_MAX_SIZE_BYTES = Number(process.env.TTS_CACHE_MAX_SIZE_BYTES || 256 * 1024 * 1024); // 256MB
 const TTS_CACHE_TTL_MS = Number(process.env.TTS_CACHE_TTL_MS || 1000 * 60 * 30); // 30 minutes
 
+// Default provider endpoints - these are the only allowed baseUrls in production
+// unless TTS_ALLOWED_BASE_URLS is configured
+const PROVIDER_DEFAULT_ENDPOINTS: Record<string, string> = {
+  openai: 'https://api.openai.com/v1',
+  groq: 'https://api.groq.com/openai/v1',
+  deepinfra: 'https://api.deepinfra.com/v1/openai',
+};
+
+// Parse allowed base URLs from environment variable (comma-separated)
+function getAllowedBaseUrls(): string[] {
+  const envUrls = process.env.TTS_ALLOWED_BASE_URLS;
+  if (!envUrls) return [];
+  return envUrls.split(',').map(url => url.trim()).filter(Boolean);
+}
+
+// Validate baseUrl against allowlist in production
+function validateBaseUrl(baseUrl: string, provider: string): string | null {
+  // Empty baseUrl is always allowed - will use provider defaults
+  if (!baseUrl) {
+    return null;
+  }
+
+  // In development, allow any baseUrl
+  if (process.env.NODE_ENV !== 'production') {
+    return baseUrl;
+  }
+
+  // Parse and normalize the URL
+  let parsedUrl: URL;
+  try {
+    parsedUrl = new URL(baseUrl);
+  } catch {
+    return null; // Invalid URL
+  }
+
+  // Only allow https in production
+  if (parsedUrl.protocol !== 'https:') {
+    return null;
+  }
+
+  const normalizedUrl = parsedUrl.origin + parsedUrl.pathname.replace(/\/$/, '');
+
+  // Check against provider defaults
+  const providerDefault = PROVIDER_DEFAULT_ENDPOINTS[provider];
+  if (providerDefault && normalizedUrl === providerDefault) {
+    return baseUrl;
+  }
+
+  // Check against configured allowlist
+  const allowedUrls = getAllowedBaseUrls();
+  for (const allowed of allowedUrls) {
+    try {
+      const allowedParsed = new URL(allowed);
+      const normalizedAllowed = allowedParsed.origin + allowedParsed.pathname.replace(/\/$/, '');
+      if (normalizedUrl === normalizedAllowed || normalizedUrl.startsWith(normalizedAllowed + '/')) {
+        return baseUrl;
+      }
+    } catch {
+      continue;
+    }
+  }
+
+  // URL not in allowlist - return null to use provider default
+  return null;
+}
+
 const ttsAudioCache = new LRUCache<string, AudioBufferValue>({
   maxSize: TTS_CACHE_MAX_SIZE_BYTES,
   sizeCalculation: (value) => value.byteLength,
@@ -240,7 +306,13 @@ export async function POST(req: NextRequest) {
 
     // Get API key and base URL based on provider
     let apiKey = req.headers.get('x-openai-key') || '';
-    let baseUrl = req.headers.get('x-openai-base-url') || '';
+    const requestedBaseUrl = req.headers.get('x-openai-base-url') || '';
+
+    // Validate and sanitize baseUrl - in production, only allow configured endpoints
+    const validatedBaseUrl = validateBaseUrl(requestedBaseUrl, provider);
+    if (requestedBaseUrl && !validatedBaseUrl && process.env.NODE_ENV === 'production') {
+      console.warn(`[TTS] Rejected untrusted baseUrl: ${requestedBaseUrl} for provider: ${provider}`);
+    }
 
     if (!apiKey) {
       switch (provider) {
@@ -256,6 +328,8 @@ export async function POST(req: NextRequest) {
       }
     }
 
+    // Use validated baseUrl or fall back to provider defaults
+    let baseUrl = validatedBaseUrl || '';
     if (!baseUrl) {
       switch (provider) {
         case 'groq':

src/components/SummarizeModal.tsx

@@ -93,6 +93,10 @@ export function SummarizeModal({
   }, [isOpen]);
 
   const handleGenerate = async () => {
+    // Preserve previous state in case of error
+    const previousSummary = summary;
+    const previousSavedSummary = savedSummary;
+
     setIsGenerating(true);
     setError(null);
     setSummary('');
@@ -133,6 +137,9 @@ export function SummarizeModal({
     } catch (err) {
       console.error('Error generating summary:', err);
       setError(err instanceof Error ? err.message : 'Failed to generate summary');
+      // Restore previous summary on error
+      setSummary(previousSummary);
+      setSavedSummary(previousSavedSummary);
     } finally {
       setIsGenerating(false);
       setChunkProgress(null);

src/lib/concurrency.ts

@@ -14,7 +14,7 @@ export type ConcurrencyResult<R> =
  *
  * @param items - Array of items to process
  * @param processor - Async function to process each item
- * @param maxConcurrent - Maximum number of concurrent processors
+ * @param maxConcurrent - Maximum number of concurrent processors (must be >= 1)
  * @param signal - Optional AbortSignal for cancellation
  * @returns Array of results in the same order as input items
  */
@@ -24,6 +24,11 @@ export async function processWithConcurrencyLimit<T, R>(
   maxConcurrent: number,
   signal?: AbortSignal
 ): Promise<ConcurrencyResult<R>[]> {
+  // Validate maxConcurrent to prevent crashes
+  if (maxConcurrent < 1 || !Number.isFinite(maxConcurrent)) {
+    throw new Error(`maxConcurrent must be >= 1, got ${maxConcurrent}`);
+  }
+
   const results: ConcurrencyResult<R>[] = new Array(items.length);
   let currentIndex = 0;
 
@@ -46,5 +51,12 @@ export async function processWithConcurrencyLimit<T, R>(
   const workerCount = Math.min(maxConcurrent, items.length);
   await Promise.all(Array(workerCount).fill(null).map(() => worker()));
 
+  // Fill any undefined slots (from aborted operations) with rejected results
+  for (let i = 0; i < items.length; i++) {
+    if (results[i] === undefined) {
+      results[i] = { status: 'rejected', reason: new Error('Operation aborted') };
+    }
+  }
+
   return results;
 }

src/lib/dexie.ts

@@ -1053,7 +1053,7 @@ export async function saveSummary(summary: Omit<SummaryRow, 'id'>): Promise<stri
 
 export async function getSummary(
   docId: string,
-  docType: 'pdf' | 'epub' | 'html',
+  _docType: 'pdf' | 'epub' | 'html',
   pageNumber?: number | null
 ): Promise<SummaryRow | null> {
   return withDB(async () => {

src/lib/summarize.ts

@@ -79,7 +79,8 @@ async function callSummarizeAPI(
   text: string,
   mode: SummarizeMode,
   options: SummarizeOptions,
-  flags?: { isChunk?: boolean; isFinalPass?: boolean }
+  flags?: { isChunk?: boolean; isFinalPass?: boolean },
+  maxLength?: number
 ): Promise<SummarizeResponse> {
   const headers: Record<string, string> = {
     'Content-Type': 'application/json',
@@ -92,7 +93,7 @@ async function callSummarizeAPI(
   const response = await fetch('/api/summarize', {
     method: 'POST',
     headers,
-    body: JSON.stringify({ text, mode, ...flags }),
+    body: JSON.stringify({ text, mode, maxLength, ...flags }),
   });
 
   const data = await response.json();
@@ -112,7 +113,7 @@ export async function generateSummary(
 
   // Direct summarization for small texts or non-whole-book modes
   if (mode !== 'whole_book' || !needsChunking(text, contextLimit)) {
-    return callSummarizeAPI(text, mode, options);
+    return callSummarizeAPI(text, mode, options, undefined, maxLength);
   }
 
   // Hierarchical summarization for large documents