diff --git a/frontend/src/pages/Findings.tsx b/frontend/src/pages/Findings.tsx index 61e6d44e4..39050aa03 100644 --- a/frontend/src/pages/Findings.tsx +++ b/frontend/src/pages/Findings.tsx @@ -602,6 +602,25 @@ export default function Findings() { } async function loadMore() { + if (loadingMore) return + setLoadingMore(true) + const nextPage = page + 1 + try { + const data = await getFindings(nextPage, perPage) + const rawFindings = data.findings || [] + const moreFindings = rawFindings.filter( + (finding) => typeof finding.id === 'string', + ) as Finding[] + if (rawFindings.length > 0) { + setFindings((prev) => [...prev, ...moreFindings]) + setPage(nextPage) + } + // Fix #1862: keep totalItems in sync with each /findings response so + // the "Load More" guard (findings.length < totalItems) stays accurate + // even when filters change the server-side total between pages. + setTotalItems(data.total ?? moreFindings.length) + } finally { + setLoadingMore(false) if (loadingMore) return setLoadingMore(true) const nextPage = page + 1 diff --git a/frontend/testing/unit/pages/Findings.test.tsx b/frontend/testing/unit/pages/Findings.test.tsx index 596fce909..7b5b5563a 100644 --- a/frontend/testing/unit/pages/Findings.test.tsx +++ b/frontend/testing/unit/pages/Findings.test.tsx @@ -537,6 +537,100 @@ describe('Findings — virtualizer scrolling', () => { }) it('scrolls to the correct fresh index after sort order changes then selection changes', async () => { + const findings = [ + makeFinding({ id: 'f1', title: 'Finding Alpha', severity: 'critical', discovered_at: '2024-01-01T00:00:00Z' }), + makeFinding({ id: 'f2', title: 'Finding Beta', severity: 'high', discovered_at: '2024-01-03T00:00:00Z' }), + makeFinding({ id: 'f3', title: 'Finding Gamma', severity: 'medium', discovered_at: '2024-01-02T00:00:00Z' }), + ] + vi.mocked(getFindings).mockResolvedValue({ findings }) + + render() + await waitFor(() => expect(screen.queryByText('Synchronizing findings feed...')).not.toBeInTheDocument()) + + // Switch to "newest" sort — new order is Beta(0), Gamma(1), Alpha(2) + const selects = screen.getAllByRole('combobox') + const sortSelect = selects.find((s) => + Array.from(s.querySelectorAll('option')).some((o) => /Newest First/i.test(o.textContent || '')), + ) + await userEvent.selectOptions(sortSelect!, 'newest') + + mockScrollToIndex.mockClear() + + // Now select Gamma — should scroll to its *post-sort* index (1), not a stale pre-sort index + const gammaOption = await screen.findByRole('option', { name: /Finding Gamma/i }) + await userEvent.click(gammaOption) + + expect(mockScrollToIndex).toHaveBeenCalledWith(1, { align: 'auto', behavior: 'smooth' }) +}) + +describe('Findings — load more totalItems sync (#1862)', () => { + beforeEach(() => { + vi.clearAllMocks() + localStorage.clear() + }) + + it('updates totalItems after each loadMore fetch so the button guard stays accurate', async () => { + // Initial load: 2 findings, server reports 10 total + const page1 = [ + makeFinding({ id: 'p1-f1', title: 'Page 1 Finding A' }), + makeFinding({ id: 'p1-f2', title: 'Page 1 Finding B' }), + ] + // loadMore call: 2 more findings, server now reports total=4 (filter narrowed) + const page2 = [ + makeFinding({ id: 'p2-f1', title: 'Page 2 Finding A' }), + makeFinding({ id: 'p2-f2', title: 'Page 2 Finding B' }), + ] + + vi.mocked(getFindings) + .mockResolvedValueOnce({ findings: page1, total: 10 }) + .mockResolvedValueOnce({ findings: page2, total: 4 }) + + render() + await waitFor(() => + expect(screen.queryByText('Synchronizing findings feed...')).not.toBeInTheDocument(), + ) + + // After initial load: 2 findings loaded, server total=10, button shows "Load More (2/10)" + const loadMoreBtn = screen.getByRole('button', { name: /Load More/i }) + expect(loadMoreBtn).toHaveTextContent('Load More (2/10)') + + // Click Load More — triggers second fetch (total updates to 4) + await userEvent.click(loadMoreBtn) + + // After loadMore: 4 findings loaded, totalItems updated to 4 → button hidden (4 >= 4) + await waitFor(() => + expect(screen.queryByRole('button', { name: /Load More/i })).not.toBeInTheDocument(), + ) + }) + + it('shows Load More button when loadMore response total exceeds current findings count', async () => { + const page1 = [ + makeFinding({ id: 'p1-f1', title: 'Page 1 Finding' }), + ] + const page2 = [ + makeFinding({ id: 'p2-f1', title: 'Page 2 Finding' }), + ] + + vi.mocked(getFindings) + .mockResolvedValueOnce({ findings: page1, total: 5 }) + .mockResolvedValueOnce({ findings: page2, total: 5 }) + + render() + await waitFor(() => + expect(screen.queryByText('Synchronizing findings feed...')).not.toBeInTheDocument(), + ) + + // Initial state: 1/5 loaded, button visible + expect(screen.getByRole('button', { name: /Load More \(1\/5\)/i })).toBeInTheDocument() + + await userEvent.click(screen.getByRole('button', { name: /Load More/i })) + + // After loadMore: 2/5, totalItems stays 5, button still visible + await waitFor(() => + expect(screen.getByRole('button', { name: /Load More \(2\/5\)/i })).toBeInTheDocument(), + ) + }) +}) const findings = [ makeFinding({ id: 'f1', title: 'Finding Alpha', severity: 'critical', discovered_at: '2024-01-01T00:00:00Z' }), makeFinding({ id: 'f2', title: 'Finding Beta', severity: 'high', discovered_at: '2024-01-03T00:00:00Z' }), diff --git a/testing/backend/unit/test_semgrep_scanner_plugin.py b/testing/backend/unit/test_semgrep_scanner_plugin.py index 032f2c938..8814a438e 100644 --- a/testing/backend/unit/test_semgrep_scanner_plugin.py +++ b/testing/backend/unit/test_semgrep_scanner_plugin.py @@ -114,3 +114,80 @@ def test_semgrep_parser_severity_mapping(): parsed = parser.parse(json_data) assert parsed["findings"][0]["severity"] == expected_secuscan_sev + + +class TestSemgrepParserMalformedJsonFallback: + """ + Verify the Semgrep parser's silent fallback behaviour when JSON is malformed. + + The parser wraps all parsing in ``except Exception: pass``, so every + malformed-input variant must deterministically return + ``{"count": 0, "findings": []}``. + """ + + def test_truncated_json_returns_empty_findings(self): + """Truncated JSON (open object never closed) must return count=0, findings=[]. + + Simulates a scanner process that was killed mid-write, leaving an + incomplete JSON payload in stdout. + """ + parser = _load_semgrep_parser() + truncated = '{"results": [{' + + parsed = parser.parse(truncated) + + assert parsed["count"] == 0 + assert parsed["findings"] == [] + + def test_mixed_stdout_with_json_fragment_returns_deterministic_empty_result(self): + """Mixed stdout containing log lines + a JSON fragment must return count=0. + + Real Semgrep invocations may print warning/info lines to stdout before + the JSON block. If the full stdout is fed to the parser the result + must still be a deterministic empty-findings dict, not a crash. + """ + parser = _load_semgrep_parser() + mixed_stdout = ( + "Running semgrep...\n" + "Loading rules from registry...\n" + '{"results": [{"check_id": "rule-x"' # fragment — never closed + ) + + parsed = parser.parse(mixed_stdout) + + assert parsed["count"] == 0 + assert parsed["findings"] == [] + # Call twice to confirm determinism + parsed_again = parser.parse(mixed_stdout) + assert parsed_again["count"] == 0 + assert parsed_again["findings"] == [] + + def test_valid_json_missing_top_level_results_key_returns_empty(self): + """Valid JSON that lacks the top-level ``results`` key must return count=0. + + The parser calls ``data.get("results", [])``, so missing the key + should yield an empty findings list rather than raise. + """ + parser = _load_semgrep_parser() + no_results_key = json.dumps({"version": "1.0", "errors": []}) + + parsed = parser.parse(no_results_key) + + assert parsed["count"] == 0 + assert parsed["findings"] == [] + + def test_valid_json_with_null_in_critical_fields_returns_empty_without_crash(self): + """Null values in critical fields must not raise and must return count=0. + + Some Semgrep builds (or mocked environments) can emit ``null`` for + ``results`` itself. The parser must absorb this gracefully because + ``null`` is valid JSON but iteration over it raises ``TypeError``, + which the broad ``except Exception`` clause catches. + """ + parser = _load_semgrep_parser() + null_results = json.dumps({"results": None}) + + parsed = parser.parse(null_results) + + assert parsed["count"] == 0 + assert parsed["findings"] == []