test: add error-handling and sensitive key protection tests for ApiKeySetupModal (#1411)

backend/secuscan/parser_sandbox.py

@@ -82,6 +82,10 @@ def __init__(self, plugin_id: str, reason: str, stderr: str = "") -> None:
         # User-facing message: reason only — no stderr content.
         super().__init__(f"Parser sandbox failed for '{plugin_id}' ({reason})")
 
+    @property
+    def stderr_excerpt(self) -> str:
+        return self._stderr_diagnostic[:2000]
+
 
 # ---------------------------------------------------------------------------
 # Bootstrap script injected into the child process via -c
@@ -248,6 +252,7 @@ def _read_stderr() -> None:
         raise ParserSandboxError(
             plugin_id,
             f"output exceeded {max_output_bytes // (1024 * 1024)} MB limit",
+            stderr=stderr_text,
         )
 
     if timed_out:
@@ -262,7 +267,11 @@ def _read_stderr() -> None:
             plugin_id,
             _sanitize_stderr(stderr_text),
         )
-        raise ParserSandboxError(plugin_id, f"timed out after {timeout_seconds}s")
+        raise ParserSandboxError(
+            plugin_id,
+            f"timed out after {timeout_seconds}s",
+            stderr=stderr_text,
+        )
 
     if proc.returncode != 0:
         logger.error(
@@ -280,6 +289,7 @@ def _read_stderr() -> None:
         raise ParserSandboxError(
             plugin_id,
             f"subprocess exited with code {proc.returncode}",
+            stderr=stderr_text,
         )
 
     stdout_bytes = b"".join(stdout_chunks)
@@ -302,12 +312,14 @@ def _read_stderr() -> None:
         raise ParserSandboxError(
             plugin_id,
             f"parser returned non-JSON output: {exc}",
+            stderr=stderr_text,
         )
 
     if not isinstance(parsed, (dict, list)):
         raise ParserSandboxError(
             plugin_id,
             f"parser returned unexpected type {type(parsed).__name__}; expected dict or list",
+            stderr=stderr_text,
         )
     logger.info("Parser sandbox completed successfully for plugin '%s'", plugin_id)
     return parsed if isinstance(parsed, dict) else {"findings": parsed}

frontend/testing/unit/components/ApiKeySetupModal.test.tsx

@@ -0,0 +1,110 @@
+import React from 'react'
+import { render, screen, waitFor } from '@testing-library/react'
+import userEvent from '@testing-library/user-event'
+import { describe, it, expect, beforeEach, vi, type MockInstance } from 'vitest'
+import ApiKeySetupModal from '../../../src/components/ApiKeySetupModal'
+
+vi.mock('../../../src/api', () => ({
+  authenticateWithApiKey: vi.fn(),
+}))
+
+import { authenticateWithApiKey } from '../../../src/api'
+
+describe('ApiKeySetupModal Error Handling', () => {
+  const defaultProps = {
+    onSaved: vi.fn(),
+  }
+
+  let consoleLogSpy: MockInstance
+  let consoleErrorSpy: MockInstance
+  let consoleWarnSpy: MockInstance
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+    consoleLogSpy = vi.spyOn(console, 'log').mockImplementation(() => {})
+    consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
+    consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+  })
+
+  it('renders the dialog with required elements', () => {
+    render(<ApiKeySetupModal {...defaultProps} />)
+
+    expect(screen.getByRole('dialog')).toBeInTheDocument()
+    expect(screen.getByText('API Key Required')).toBeInTheDocument()
+    expect(screen.getByLabelText('Backend API Key')).toBeInTheDocument()
+    expect(screen.getByRole('button', { name: /save and connect/i })).toBeInTheDocument()
+  })
+
+  it('handles successful API key validation', async () => {
+    const user = userEvent.setup()
+    vi.mocked(authenticateWithApiKey).mockResolvedValue(undefined)
+
+    render(<ApiKeySetupModal {...defaultProps} />)
+
+    const input = screen.getByLabelText('Backend API Key')
+    await user.type(input, 'valid-key-123')
+    await user.click(screen.getByRole('button', { name: /save and connect/i }))
+
+    expect(authenticateWithApiKey).toHaveBeenCalledWith('valid-key-123')
+    await waitFor(() => {
+      expect(defaultProps.onSaved).toHaveBeenCalled()
+    })
+  })
+
+  it('shows standard validation error on empty submission', async () => {
+    const user = userEvent.setup()
+    render(<ApiKeySetupModal {...defaultProps} />)
+
+    await user.click(screen.getByRole('button', { name: /save and connect/i }))
+
+    const alert = await screen.findByRole('alert')
+    expect(alert).toHaveTextContent('Please enter the API key.')
+    expect(authenticateWithApiKey).not.toHaveBeenCalled()
+  })
+
+  it('shows backend error message when api key is rejected', async () => {
+    const user = userEvent.setup()
+    const backendErrorMsg = 'Invalid/Expired API token'
+    vi.mocked(authenticateWithApiKey).mockRejectedValue(new Error(backendErrorMsg))
+
+    render(<ApiKeySetupModal {...defaultProps} />)
+
+    const input = screen.getByLabelText('Backend API Key')
+    await user.type(input, 'bad-key')
+    await user.click(screen.getByRole('button', { name: /save and connect/i }))
+
+    const alert = await screen.findByRole('alert')
+    expect(alert).toHaveTextContent(backendErrorMsg)
+    expect(defaultProps.onSaved).not.toHaveBeenCalled()
+  })
+
+  it('ensures sensitive key text is not logged or echoed in failure UI', async () => {
+    const user = userEvent.setup()
+    const sensitiveKey = 'SUPER_SECRET_TOKEN_999'
+    vi.mocked(authenticateWithApiKey).mockRejectedValue(new Error('Authentication failed. Check the API key.'))
+
+    render(<ApiKeySetupModal {...defaultProps} />)
+
+    const input = screen.getByLabelText('Backend API Key')
+    await user.type(input, sensitiveKey)
+    await user.click(screen.getByRole('button', { name: /save and connect/i }))
+
+    // Assert useful error message is visible
+    const alert = await screen.findByRole('alert')
+    expect(alert).toBeInTheDocument()
+
+    // Assert sensitive key text is NOT in the error message / failure UI text content
+    expect(alert.textContent).not.toContain(sensitiveKey)
+
+    // Assert sensitive key text is NOT logged in console
+    const allLogCalls = [
+      ...consoleLogSpy.mock.calls,
+      ...consoleErrorSpy.mock.calls,
+      ...consoleWarnSpy.mock.calls,
+    ].flatMap((args) => args.map((arg) => (typeof arg === 'string' ? arg : JSON.stringify(arg))))
+
+    for (const logText of allLogCalls) {
+      expect(logText).not.toContain(sensitiveKey)
+    }
+  })
+})

testing/backend/unit/test_parser_sandbox_timeout_cleanup.py

@@ -13,6 +13,7 @@
 from __future__ import annotations
 
 import subprocess
+import sys
 import pytest
 import time
 from pathlib import Path
@@ -34,8 +35,8 @@ def test_timeout_expired_raises_parser_sandbox_error(self, tmp_path):
 
         with pytest.raises(subprocess.TimeoutExpired):
             proc = subprocess.Popen(
-                ["python3", "-c", _BOOTSTRAP_TEMPLATE.format(
-                    parser_path=str(parser),
+                [sys.executable, "-c", _BOOTSTRAP_TEMPLATE.safe_substitute(
+                    parser_path_repr=repr(str(parser)),
                     max_input_bytes=1024,
                 )],
                 stdin=subprocess.PIPE,
@@ -90,12 +91,13 @@ def test_timeout_error_includes_stderr(self, tmp_path):
             encoding="utf-8",
         )
 
-        with pytest.raises(Exception) as exc_info:
+        from backend.secuscan.parser_sandbox import ParserSandboxError
+        with pytest.raises(ParserSandboxError) as exc_info:
             run_parser_in_sandbox(parser, "stderr_plugin", "data", timeout_seconds=1)
 
-        exc_message = str(exc_info.value)
-        assert "early error" in exc_message
+        assert "early error" in exc_info.value.stderr_excerpt
 
+    @pytest.mark.skipif(sys.platform == "win32", reason="resource module not available on Windows")
     def test_multiple_consecutive_timeouts_do_not_leak_resources(self, tmp_path):
         """Running multiple timeouts in sequence must not accumulate open file handles."""
         from backend.secuscan.parser_sandbox import run_parser_in_sandbox