From b23f7e01f4ce100b13c1b0a4c802bcb9f937952b Mon Sep 17 00:00:00 2001 From: khanvilkarshravani27 Date: Sat, 25 Jul 2026 14:09:27 +0530 Subject: [PATCH 1/2] fix(frontend): sync totalItems from loadMore response (closes #1862) totalItems was only set from the first /findings call. Subsequent loadMore fetches never updated it, so the 'Load More (X/Y)' guard used a stale total whenever filters changed the server-side count between pages. Changes: - Import FindingsResponse type and use it instead of ny in the initial load callback; filter findings to those with string ids for safety - Add setTotalItems(data.total ?? moreFindings.length) inside loadMore after each successful paginated fetch, matching the same pattern already used on initial load; also apply the id-string filter to moreFindings - Add two unit tests for the totalItems sync: one verifies the button hides when totalItems drops to match findings.length after loadMore; the other verifies the counter keeps updating correctly across pages --- frontend/src/pages/Findings.tsx | 19 +++- frontend/testing/unit/pages/Findings.test.tsx | 96 +++++++++++++++++++ 2 files changed, 110 insertions(+), 5 deletions(-) diff --git a/frontend/src/pages/Findings.tsx b/frontend/src/pages/Findings.tsx index ca6fe1985..467618e56 100644 --- a/frontend/src/pages/Findings.tsx +++ b/frontend/src/pages/Findings.tsx @@ -1,7 +1,7 @@ import React, { useEffect, useMemo, useRef, useState } from 'react' import { AnimatePresence, motion } from 'framer-motion' import { useVirtualizer } from '@tanstack/react-virtual' -import { getFindings } from '../api' +import { getFindings, FindingsResponse } from '../api' import { formatLocaleDate, parseDateSafe, getCurrentTimeZone } from '../utils/date' import SavedViewsPanel from '../components/SavedViewsPanel' import { useSavedViews, FilterPreset } from '../hooks/useSavedViews' @@ -249,8 +249,10 @@ export default function Findings() { useEffect(() => { setLoading(true) getFindings(1, perPage) - .then((data: any) => { - const nextFindings = data.findings || [] + .then((data: FindingsResponse) => { + const nextFindings = (data.findings || []).filter( + (finding) => typeof finding.id === 'string', + ) as Finding[] setFindings(nextFindings) setTotalItems(data.total ?? nextFindings.length) setPage(1) @@ -601,11 +603,18 @@ export default function Findings() { const nextPage = page + 1 try { const data = await getFindings(nextPage, perPage) - const moreFindings = (data.findings || []) as Finding[] - if (moreFindings.length > 0) { + 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) } diff --git a/frontend/testing/unit/pages/Findings.test.tsx b/frontend/testing/unit/pages/Findings.test.tsx index e797bf45d..2bf1f0c8e 100644 --- a/frontend/testing/unit/pages/Findings.test.tsx +++ b/frontend/testing/unit/pages/Findings.test.tsx @@ -535,3 +535,99 @@ describe('Findings — virtualizer scrolling', () => { expect(mockScrollToIndex).not.toHaveBeenCalled() }) }) + +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(), + ) + }) +}) From aa6e271d148585dd30e135e60da9b22eca875add Mon Sep 17 00:00:00 2001 From: khanvilkarshravani27 Date: Thu, 6 Aug 2026 18:21:12 +0530 Subject: [PATCH 2/2] test: add malformed JSON fallback tests for Semgrep scanner parser (closes #1658) --- .../unit/test_semgrep_scanner_plugin.py | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) 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"] == []