From bb1ea8cceb37dcc317b4598b8322fe80bb141dbb Mon Sep 17 00:00:00 2001 From: Subramaniyajothi6 Date: Tue, 4 Aug 2026 14:07:05 +0530 Subject: [PATCH 1/2] Export findings from the backend instead of loaded page state Findings export was built entirely in the browser from React state, so it could only ever contain findings already scrolled into memory. Selecting across pages exported whatever happened to be loaded. Add POST /api/v1/findings/export, which resolves a selection of finding ids against the database and streams CSV, JSON, or SARIF back. Omitting the ids exports everything the caller owns; an empty array exports nothing and is never read as "everything". Findings are read in batches and serialized as they go, so memory is bounded by SECUSCAN_EXPORT_BATCH_SIZE rather than by the size of the export. SARIF is the exception and is assembled in full, because its deduplicated rules array precedes the results that index into it. SECUSCAN_MAX_EXPORT_FINDINGS caps a request rather than truncating it, so an export is never silently partial. Every query is owner-scoped. Ids belonging to another owner are skipped rather than rejected, so the endpoint cannot be used to probe for them, and the reported count does not confirm them either. Free-text fields, evidence, and metadata go through the same redaction as task reports. The CSV keeps the columns the browser produced, so existing scripts still work. The client-side serializers they came from are removed; their column contract now lives in testing/backend/unit/test_finding_export.py. --- .env.example | 7 + backend/secuscan/config.py | 6 + backend/secuscan/finding_export.py | 198 +++++++++ backend/secuscan/models.py | 29 ++ backend/secuscan/routes.py | 147 ++++++- docs/API.md | 56 +++ docs/SECURE_DEPLOYMENT.md | 2 + frontend/src/api.ts | 64 +++ frontend/src/pages/Findings.tsx | 81 ++-- frontend/src/utils/exportUtils.ts | 67 +-- frontend/testing/unit/AppRoutes.test.tsx | 13 +- frontend/testing/unit/pages/Findings.test.tsx | 87 +++- .../testing/unit/utils/exportUtils.test.ts | 102 ++--- .../integration/test_findings_export.py | 413 ++++++++++++++++++ testing/backend/unit/test_finding_export.py | 243 +++++++++++ 15 files changed, 1337 insertions(+), 178 deletions(-) create mode 100644 backend/secuscan/finding_export.py create mode 100644 testing/backend/integration/test_findings_export.py create mode 100644 testing/backend/unit/test_finding_export.py diff --git a/.env.example b/.env.example index 0d4e08f97..caf4b3084 100644 --- a/.env.example +++ b/.env.example @@ -62,6 +62,13 @@ SECUSCAN_VAULT_KEY=replace-with-output-of-secrets.token_hex-32 # SECUSCAN_PARSER_SANDBOX_TIMEOUT_SECONDS=30 # SECUSCAN_PARSER_SANDBOX_MAX_OUTPUT_BYTES=8388608 +# Bulk Findings Export +# MAX_EXPORT_FINDINGS caps a single export; requests above it are rejected +# rather than truncated. EXPORT_BATCH_SIZE is how many findings are read per +# database round trip while streaming, and bounds memory, not export size. +# SECUSCAN_MAX_EXPORT_FINDINGS=10000 +# SECUSCAN_EXPORT_BATCH_SIZE=500 + # Frontend Overrides # Leave these unset for the default local dev flow. # VITE_API_PROXY_TARGET=http://127.0.0.1:8000 diff --git a/backend/secuscan/config.py b/backend/secuscan/config.py index 48c586172..0f73d6ebe 100644 --- a/backend/secuscan/config.py +++ b/backend/secuscan/config.py @@ -151,6 +151,12 @@ class Settings(BaseSettings): parser_sandbox_timeout_seconds: int = 30 parser_sandbox_max_output_bytes: int = 8 * 1024 * 1024 # 8 MB + # Bulk findings export. Findings are read from the database in batches of + # export_batch_size and streamed out, so the ceiling below bounds the work + # a single request can ask for, not the memory it costs to serve it. + max_export_findings: int = 10_000 + export_batch_size: int = 500 + # Workflow Configuration workflow_min_interval_seconds: int = 60 diff --git a/backend/secuscan/finding_export.py b/backend/secuscan/finding_export.py new file mode 100644 index 000000000..edde38bcd --- /dev/null +++ b/backend/secuscan/finding_export.py @@ -0,0 +1,198 @@ +""" +Bulk findings export serializers. + +Findings export used to be assembled entirely in the browser from whatever the +user had scrolled into memory, so anything not loaded could not be exported. +These serializers run against rows read straight from the database, which makes +the export independent of the client's page state. + +Everything here is chunk-at-a-time on purpose: ``stream_csv`` and +``stream_json`` consume an async iterator of finding batches and yield text as +they go, so the size of an export is bounded by the batch size rather than by +the number of findings. SARIF is the exception and is documented below. + +Redaction mirrors :mod:`backend.secuscan.reporting` — the same fields are +scrubbed with the same helpers, so a findings export and a task report never +disagree about what is safe to write out. +""" + +from __future__ import annotations + +import csv +import io +import json +from typing import Any, AsyncIterator, Dict, Iterable, List, Sequence + +from .redaction import _redact_value, redact, redact_dict +from .reporting import reporting + +EXPORT_FORMATS: tuple[str, ...] = ("csv", "json", "sarif") + +MEDIA_TYPES: Dict[str, str] = { + "csv": "text/csv; charset=utf-8", + "json": "application/json", + "sarif": "application/json", +} + +FILE_EXTENSIONS: Dict[str, str] = { + "csv": "csv", + "json": "json", + "sarif": "sarif", +} + +# Same columns, same order as the browser-side export, so moving the work to +# the backend does not change the shape of the file analysts already script +# against. (Line endings do change: csv writes RFC 4180 CRLF, matching the +# task report export rather than the browser's bare LF.) +CSV_COLUMNS: tuple[str, ...] = ( + "ID", + "Title", + "Severity", + "Category", + "Target", + "Discovered At", + "CVSS", + "CVE", + "Risk Score", + "Confidence", + "Validated", + "Analyst Status", + "Description", + "Remediation", +) + +# Free-text columns that can carry secrets lifted out of scanner output. +_REDACTED_TEXT_FIELDS: tuple[str, ...] = ( + "target", + "description", + "remediation", + "proof", + "confidence_reason", +) + +# ``owner_id`` is the caller's own identity repeated on every row: constant for +# the whole export and useless inside it. +_EXCLUDED_FIELDS: frozenset[str] = frozenset({"owner_id"}) + + +def redacted_finding(finding: Dict[str, Any]) -> Dict[str, Any]: + """Return a copy of ``finding`` safe to write into an export file.""" + exported = {key: value for key, value in finding.items() if key not in _EXCLUDED_FIELDS} + + for field in _REDACTED_TEXT_FIELDS: + value = exported.get(field) + if isinstance(value, str) and value: + exported[field] = redact(value) + + metadata = exported.get("metadata") + if isinstance(metadata, dict): + exported["metadata"] = redact_dict(metadata) + + evidence = exported.get("evidence") + if isinstance(evidence, list): + exported["evidence"] = _redact_value(evidence) + + return exported + + +def _text(value: Any) -> str: + if value is None: + return "" + return str(value) + + +def _number(value: Any) -> str: + """Render a numeric column, distinguishing 'absent' from 'zero'.""" + if value is None or value == "": + return "" + return str(value) + + +def finding_csv_row(finding: Dict[str, Any]) -> List[str]: + """Build one CSV row from an already-redacted finding.""" + return [ + _text(finding.get("id")), + _text(finding.get("title")), + _text(finding.get("severity")), + _text(finding.get("category")), + _text(finding.get("target")), + _text(finding.get("discovered_at")), + _number(finding.get("cvss")), + _text(finding.get("cve")), + _number(finding.get("risk_score")), + _number(finding.get("confidence")), + "true" if finding.get("validated") else "false", + _text(finding.get("analyst_status")), + _text(finding.get("description")), + _text(finding.get("remediation")), + ] + + +def _write_csv_rows(rows: Iterable[Sequence[Any]]) -> str: + buffer = io.StringIO() + try: + # Excel and most CSV readers expect CRLF, which is also what csv writes + # by default; setting it explicitly keeps the output identical whatever + # platform the backend runs on. + writer = csv.writer(buffer, lineterminator="\r\n") + writer.writerows(rows) + return buffer.getvalue() + finally: + buffer.close() + + +async def stream_csv(batches: AsyncIterator[List[Dict[str, Any]]]) -> AsyncIterator[str]: + """Yield a CSV document, one database batch at a time. + + The header is emitted before the first batch is fetched so an export with + no matching findings is still a valid, openable CSV file. + """ + yield _write_csv_rows([CSV_COLUMNS]) + async for batch in batches: + yield _write_csv_rows(finding_csv_row(redacted_finding(f)) for f in batch) + + +async def stream_json(batches: AsyncIterator[List[Dict[str, Any]]]) -> AsyncIterator[str]: + """Yield a JSON array of findings, one database batch at a time.""" + yield "[" + first = True + async for batch in batches: + for finding in batch: + yield ("" if first else ",") + json.dumps(redacted_finding(finding), default=str) + first = False + yield "]" + + +async def stream_sarif(batches: AsyncIterator[List[Dict[str, Any]]]) -> AsyncIterator[str]: + """Yield a SARIF v2.1.0 document. + + Unlike CSV and JSON this cannot stream: SARIF puts the deduplicated ``rules`` + array in the tool driver, ahead of the results that reference it by index, + so the whole set has to be known before the first byte is correct. The + batches are still consumed incrementally, but the document is assembled in + memory — which is why ``max_export_findings`` exists. + """ + collected: List[Dict[str, Any]] = [] + async for batch in batches: + collected.extend(redacted_finding(finding) for finding in batch) + + synthetic_task = { + "id": "findings-export", + "tool_name": "SecuScan", + "plugin_id": "secuscan", + "target": "multiple", + "status": "completed", + } + yield reporting.generate_sarif_report(synthetic_task, {"findings": collected}) + + +STREAMERS = { + "csv": stream_csv, + "json": stream_json, + "sarif": stream_sarif, +} + + +def export_filename(export_format: str, generated_on: str) -> str: + """Build the download filename, matching what the browser export used.""" + return f"secuscan_findings_{generated_on}.{FILE_EXTENSIONS[export_format]}" diff --git a/backend/secuscan/models.py b/backend/secuscan/models.py index 75661c3ab..917c1c250 100644 --- a/backend/secuscan/models.py +++ b/backend/secuscan/models.py @@ -381,3 +381,32 @@ class NotificationDiagnosticsResponse(BaseModel): class BulkDeleteRequest(RootModel[Annotated[List[str], Field(max_length=MAX_BULK_DELETE)]]): """Accepts a JSON array of task IDs directly. Max 500 per request.""" pass + + +class FindingExportFormat(str, Enum): + """Serialization formats offered by the bulk findings export.""" + CSV = "csv" + JSON = "json" + SARIF = "sarif" + + +class FindingExportRequest(BaseModel): + """Body for POST /findings/export. + + ``finding_ids`` distinguishes three cases deliberately: + + * omitted / ``null`` — export every finding the caller owns. This is what + makes "export across all pages" possible without the client first + loading those pages. + * a list of ids — export exactly those, in whatever order the database + returns them. Ids the caller does not own are skipped rather than + rejected, so the endpoint cannot be used to probe for their existence. + * ``[]`` — export nothing. An empty selection must never be read as + "everything". + + The upper bound on list length is enforced in the route against + ``settings.max_export_findings`` so operators can tune it without a code + change. + """ + finding_ids: Optional[List[str]] = None + format: FindingExportFormat = FindingExportFormat.CSV diff --git a/backend/secuscan/routes.py b/backend/secuscan/routes.py index 21dfd8f8b..a7679e2f8 100644 --- a/backend/secuscan/routes.py +++ b/backend/secuscan/routes.py @@ -3,7 +3,7 @@ """ from fastapi import APIRouter, HTTPException, BackgroundTasks, Response, Request, Depends, Body, Query -from fastapi.responses import JSONResponse +from fastapi.responses import JSONResponse, StreamingResponse from typing import Any, Optional, List, Dict, Callable import json import logging @@ -11,6 +11,7 @@ import os import uuid import asyncio +from datetime import datetime, timezone from pathlib import Path from urllib.parse import urlencode, urlparse @@ -98,8 +99,10 @@ def _json_payload(value: Any, fallback: str) -> str: ExecutionContext, WorkflowStep, ValidationMode, EvidenceLevel, NotificationDiagnosticsResponse, ScanWebhookSettingsRequest, ScanWebhookSettingsResponse, + FindingExportRequest, ) from .config import settings +from . import finding_export from .database import get_db from .plugins import get_plugin_manager, init_plugins from . import notification_service @@ -1193,6 +1196,148 @@ async def build(): return await get_or_set_cached(f"findings:list:{owner}:page={page}:per_page={per_page}", build) +def _id_batches(finding_ids: List[str], batch_size: int): + """Split ids into groups small enough to bind as SQL parameters.""" + for start in range(0, len(finding_ids), batch_size): + yield finding_ids[start:start + batch_size] + + +async def _iter_owner_findings( + db, + owner: str, + finding_ids: Optional[List[str]], + batch_size: int, +): + """Yield the caller's findings in batches. + + Every query is scoped by ``owner_id``, so an id belonging to someone else + simply never matches. Ordering carries an ``id`` tiebreaker because + ``discovered_at`` is not unique: LIMIT/OFFSET paging over rows the sort + cannot distinguish is only stable by accident. SQLite happens to be + consistent here, PostgreSQL is under no such obligation. + """ + if finding_ids is None: + offset = 0 + while True: + rows = await db.fetchall( + "SELECT * FROM findings WHERE owner_id = ? " + "ORDER BY discovered_at DESC, id LIMIT ? OFFSET ?", + (owner, batch_size, offset), + ) + if not rows: + return + yield deserialize_finding_rows(rows) + if len(rows) < batch_size: + return + offset += batch_size + + for chunk in _id_batches(finding_ids, batch_size): + placeholders = ",".join("?" for _ in chunk) + rows = await db.fetchall( + f"SELECT * FROM findings WHERE owner_id = ? AND id IN ({placeholders}) " + "ORDER BY discovered_at DESC, id", + (owner, *chunk), + ) + if rows: + yield deserialize_finding_rows(rows) + + +async def _count_owner_findings(db, owner: str, finding_ids: List[str], batch_size: int) -> int: + """Count how many of ``finding_ids`` the caller actually owns. + + Batched for the same reason the read is: an ``IN`` list is one bound + parameter per id, and every database has a ceiling on those. Ids are + de-duplicated before they get here, so summing the batches cannot + double-count. + """ + total = 0 + for chunk in _id_batches(finding_ids, batch_size): + placeholders = ",".join("?" for _ in chunk) + row = await db.fetchone( + f"SELECT COUNT(*) as count FROM findings WHERE owner_id = ? AND id IN ({placeholders})", + (owner, *chunk), + ) + total += row["count"] if row else 0 + return total + + +@router.post("/findings/export", dependencies=[Depends(report_download_limiter)]) +async def export_findings( + payload: FindingExportRequest, + owner: str = Depends(get_current_owner), +): + """Stream the caller's findings as CSV, JSON, or SARIF. + + Exports are built from the database rather than from whatever the client + has loaded, so a selection spanning pages the browser never fetched still + exports in full. + """ + export_format = payload.format.value + requested_ids = payload.finding_ids + + if requested_ids is not None: + # De-duplicate before the cap: the cap bounds how many findings get + # read and serialized, and a repeated id is still one finding. Order is + # preserved so the request stays recognisable in the audit log. The cost + # of parsing an oversized body is a request-size concern, not this one. + requested_ids = list(dict.fromkeys(requested_ids)) + if len(requested_ids) > settings.max_export_findings: + raise HTTPException( + status_code=400, + detail=( + f"Too many findings requested: {len(requested_ids)} " + f"(maximum {settings.max_export_findings})" + ), + ) + + db = await get_db() + + if requested_ids is None: + count_row = await db.fetchone( + "SELECT COUNT(*) as count FROM findings WHERE owner_id = ?", + (owner,), + ) + matched = count_row["count"] if count_row else 0 + if matched > settings.max_export_findings: + raise HTTPException( + status_code=400, + detail=( + f"Export would include {matched} findings " + f"(maximum {settings.max_export_findings}). Select a subset instead." + ), + ) + else: + matched = await _count_owner_findings( + db, owner, requested_ids, settings.export_batch_size + ) + + await db.log_audit( + "findings_exported", + f"Bulk findings export ({export_format}): {matched} findings", + context={ + "format": export_format, + "requested": "all" if requested_ids is None else len(requested_ids), + "exported": matched, + }, + ) + + batches = _iter_owner_findings(db, owner, requested_ids, settings.export_batch_size) + body = finding_export.STREAMERS[export_format](batches) + generated_on = datetime.now(timezone.utc).strftime("%Y-%m-%d") + + return StreamingResponse( + body, + media_type=finding_export.MEDIA_TYPES[export_format], + headers={ + "Content-Disposition": ( + f'attachment; filename="{finding_export.export_filename(export_format, generated_on)}"' + ), + "X-Export-Finding-Count": str(matched), + "Cache-Control": "no-store", + }, + ) + + @router.get("/finding-groups", dependencies=[Depends(read_heavy_limiter)]) async def get_finding_groups( owner: str = Depends(get_current_owner), diff --git a/docs/API.md b/docs/API.md index 24a97817e..69cb6c8a6 100644 --- a/docs/API.md +++ b/docs/API.md @@ -142,6 +142,62 @@ curl -H "X-Api-Key: $API_KEY" \ "http://localhost:8000/api/v1/search?q=sql+injection&limit=10" ``` +## Findings API + +### Bulk Export + +**Endpoint:** `POST /api/v1/findings/export` + +**Description:** Streams the caller's findings as a downloadable file. Findings +are read from the database rather than from a page of results, so an export can +cover findings the client never fetched. + +Results are owner-scoped (see +[Authentication and ownership](#authentication-and-ownership)). Ids belonging to +another `X-User-Id` are skipped silently rather than rejected, so the endpoint +cannot be used to test whether a given finding exists. + +Free-text fields (`target`, `description`, `remediation`, `proof`, +`confidence_reason`), evidence, and metadata pass through the same redaction as +task reports before they are written out. + +**Request Body:** + +| Field | Type | Required | Default | Description | +|-------|------|----------|---------|-------------| +| finding_ids | string[] \| null | No | `null` | Findings to export. Omit or send `null` to export everything the caller owns. An empty array exports nothing — it is never read as "everything". Duplicates are collapsed. | +| format | string | No | `csv` | One of `csv`, `json`, `sarif`. | + +**Response (200 OK):** the export file, with `Content-Disposition` set to +`attachment` and `X-Export-Finding-Count` carrying the number of findings +written. + +| Format | Media type | Contents | +|--------|-----------|----------| +| csv | `text/csv; charset=utf-8` | RFC 4180 CSV. The header row is always present, so an empty export is still a valid file. | +| json | `application/json` | A JSON array of finding objects, minus `owner_id`. | +| sarif | `application/json` | SARIF v2.1.0, the same schema as the per-task SARIF report. | + +**Errors:** + +| Status | Cause | +|--------|-------| +| 400 | More findings requested than `SECUSCAN_MAX_EXPORT_FINDINGS` allows (default 10000). | +| 422 | Unknown `format`. | +| 429 | Endpoint rate limit — shared with report downloads. | + +```bash +# Export a selection +curl -X POST -H "X-Api-Key: $API_KEY" -H "Content-Type: application/json" \ + -d '{"finding_ids": ["finding-1", "finding-2"], "format": "csv"}' \ + -OJ "http://localhost:8000/api/v1/findings/export" + +# Export everything the caller owns, as SARIF +curl -X POST -H "X-Api-Key: $API_KEY" -H "Content-Type: application/json" \ + -d '{"format": "sarif"}' \ + -OJ "http://localhost:8000/api/v1/findings/export" +``` + ## See Also * [API Authentication](api-authentication.md) — How requests are authenticated with the API key and authorized per owner (`X-User-Id` → `owner_id`), including the cross-owner test requirement. diff --git a/docs/SECURE_DEPLOYMENT.md b/docs/SECURE_DEPLOYMENT.md index 4459d3e82..fb6d38bae 100644 --- a/docs/SECURE_DEPLOYMENT.md +++ b/docs/SECURE_DEPLOYMENT.md @@ -353,6 +353,8 @@ network behavior of these variables. | `SECUSCAN_MAX_REQUESTS_PER_MINUTE` | `100` | Global API request-rate cap. | | `SECUSCAN_TRUSTED_PROXIES` | `127.0.0.1,::1` | Proxies trusted for client-IP resolution (rate-limit accuracy). Only list proxies you control. | | `SECUSCAN_TASK_START_MAX_BODY_BYTES` | `64000` | Max task-start JSON body in bytes (request-flood guard). | +| `SECUSCAN_MAX_EXPORT_FINDINGS` | `10000` | Ceiling on a single bulk findings export. Requests above it are rejected rather than truncated, so an export is never silently partial. | +| `SECUSCAN_EXPORT_BATCH_SIZE` | `500` | Findings read per database round trip while streaming an export. Bounds memory, not the export size. | ## Logging & Audit diff --git a/frontend/src/api.ts b/frontend/src/api.ts index 29401db05..b91b2d889 100644 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -466,6 +466,70 @@ export function getFindingGroups(page: number = 1, perPage: number = 50) { return request<{ groups: FindingGroup[]; total: number; page: number; per_page: number }>(`/finding-groups?page=${page}&per_page=${perPage}`) } +export type FindingExportFormat = 'csv' | 'json' | 'sarif' + +export interface FindingExportResult { + blob: Blob + /** How many findings the backend actually wrote, when it is readable. */ + count: number | null +} + +/** Exports run against the database, so they can outlast the 10s request timeout. */ +const EXPORT_TIMEOUT_MS = 120000 + +/** + * Export findings from the backend rather than from loaded page state. + * + * Pass `findingIds` to export a selection, or omit it to export everything the + * caller owns — which is how a selection spanning unloaded pages is possible. + * An empty array exports nothing; it is never treated as "everything". + */ +export async function exportFindings( + format: FindingExportFormat, + findingIds?: string[], +): Promise { + const controller = new AbortController() + const timeoutId = window.setTimeout(() => controller.abort(), EXPORT_TIMEOUT_MS) + + const apiKey = getApiKey() + const headers: Record = { 'Content-Type': 'application/json' } + if (apiKey) headers['X-Api-Key'] = apiKey + + try { + const response = await fetch(`${API_BASE}/findings/export`, { + method: 'POST', + headers, + credentials: 'include', + signal: controller.signal, + body: JSON.stringify( + findingIds === undefined ? { format } : { format, finding_ids: findingIds }, + ), + }) + + if (response.status === 401) { + _apiKey = null + window.dispatchEvent(new CustomEvent(AUTH_REQUIRED_EVENT)) + throw new Error('AUTH_REQUIRED') + } + + if (!response.ok) { + throw new Error(`Export failed: ${response.status}`) + } + + // Only readable same-origin (or with the header explicitly exposed by CORS); + // callers fall back to their own count when it is absent. + const reported = response.headers.get('X-Export-Finding-Count') + const count = reported === null ? null : Number(reported) + + return { + blob: await response.blob(), + count: count !== null && Number.isFinite(count) ? count : null, + } + } finally { + window.clearTimeout(timeoutId) + } +} + export function getReports() { return request('/reports') diff --git a/frontend/src/pages/Findings.tsx b/frontend/src/pages/Findings.tsx index 61e6d44e4..6b2ea696b 100644 --- a/frontend/src/pages/Findings.tsx +++ b/frontend/src/pages/Findings.tsx @@ -1,12 +1,13 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { AnimatePresence, motion } from 'framer-motion' import { useVirtualizer } from '@tanstack/react-virtual' -import { getFindings, FindingsResponse } from '../api' +import { getFindings, exportFindings, FindingsResponse, FindingExportFormat } from '../api' import { formatLocaleDate, parseDateSafe, getCurrentTimeZone } from '../utils/date' import SavedViewsPanel from '../components/SavedViewsPanel' import { useSavedViews, FilterPreset } from '../hooks/useSavedViews' import { useEscapeToClose } from '../hooks/useEscapeToClose' -import { exportFindingsAsCSV, exportFindingsAsJSON } from '../utils/exportUtils' +import { downloadBlob, findingsExportFilename } from '../utils/exportUtils' +import { useToast } from '../components/ToastContext' type RiskFactor = { factor: string @@ -177,6 +178,8 @@ export default function Findings() { // ── Multi-select export state & handlers ─────────────────────────────────── const [selectedIds, setSelectedIds] = useState>(new Set()) const [exportDropdownOpen, setExportDropdownOpen] = useState(false) + const [exporting, setExporting] = useState(false) + const { addToast } = useToast() const closeExportDropdown = useCallback(() => setExportDropdownOpen(false), []) useEscapeToClose(exportDropdownOpen, closeExportDropdown) @@ -425,14 +428,25 @@ export default function Findings() { }) } - const handleExportCSV = () => { - const selectedFindings = findings.filter((f) => selectedIds.has(f.id)) - exportFindingsAsCSV(selectedFindings) - } - - const handleExportJSON = () => { - const selectedFindings = findings.filter((f) => selectedIds.has(f.id)) - exportFindingsAsJSON(selectedFindings) + // Export runs on the backend against the database. Sending ids rather than + // finding objects is what lets a selection cover pages this component never + // loaded; sending none at all exports everything the caller owns. + const handleExport = async (format: FindingExportFormat) => { + if (exporting) return + const ids = selectedIds.size > 0 ? Array.from(selectedIds) : undefined + setExporting(true) + try { + const { blob, count } = await exportFindings(format, ids) + downloadBlob(blob, findingsExportFilename(format)) + const exported = count ?? ids?.length ?? totalItems + addToast(`Exported ${exported} finding${exported === 1 ? '' : 's'} as ${format.toUpperCase()}`) + } catch (err) { + if ((err as Error).message !== 'AUTH_REQUIRED') { + addToast('Export failed. Please try again.', 'error') + } + } finally { + setExporting(false) + } } const sortedFindings = useMemo(() => { @@ -1070,39 +1084,40 @@ export default function Findings() { )} - {selectedIds.size > 0 && ( + {totalItems > 0 && (
{exportDropdownOpen && (
- - + {(['csv', 'json', 'sarif'] as const).map((format, index, all) => ( + + ))}
)}
diff --git a/frontend/src/utils/exportUtils.ts b/frontend/src/utils/exportUtils.ts index 562c99ca8..c3d816749 100644 --- a/frontend/src/utils/exportUtils.ts +++ b/frontend/src/utils/exportUtils.ts @@ -1,55 +1,4 @@ -export function escapeCSV(val: any): string { - if (val === null || val === undefined) return '' - const str = String(val) - if (str.includes(',') || str.includes('"') || str.includes('\n') || str.includes('\r')) { - return `"${str.replace(/"/g, '""')}"` - } - return str -} - -export function serializeFindingsToCSV(findings: any[]): string { - const headers = [ - 'ID', - 'Title', - 'Severity', - 'Category', - 'Target', - 'Discovered At', - 'CVSS', - 'CVE', - 'Risk Score', - 'Confidence', - 'Validated', - 'Analyst Status', - 'Description', - 'Remediation' - ] - - const rows = findings.map((f) => [ - f.id || '', - f.title || '', - f.severity || '', - f.category || '', - f.target || '', - f.discovered_at || '', - f.cvss !== undefined && f.cvss !== null ? String(f.cvss) : '', - f.cve || '', - f.risk_score !== undefined && f.risk_score !== null ? String(f.risk_score) : '', - f.confidence !== undefined && f.confidence !== null ? String(f.confidence) : '', - f.validated ? 'true' : 'false', - f.analyst_status || '', - f.description || '', - f.remediation || '' - ]) - - return [ - headers.join(','), - ...rows.map((row) => row.map(escapeCSV).join(',')) - ].join('\n') -} - -export function downloadFile(content: string, filename: string, contentType: string): void { - const blob = new Blob([content], { type: contentType }) +export function downloadBlob(blob: Blob, filename: string): void { const url = URL.createObjectURL(blob) const a = document.createElement('a') a.href = url @@ -60,14 +9,12 @@ export function downloadFile(content: string, filename: string, contentType: str URL.revokeObjectURL(url) } -export function exportFindingsAsCSV(findings: any[]): void { - const csvContent = serializeFindingsToCSV(findings) - const dateStr = new Date().toISOString().split('T')[0] - downloadFile(csvContent, `secuscan_findings_${dateStr}.csv`, 'text/csv;charset=utf-8;') +export function downloadFile(content: string, filename: string, contentType: string): void { + downloadBlob(new Blob([content], { type: contentType }), filename) } -export function exportFindingsAsJSON(findings: any[]): void { - const jsonContent = JSON.stringify(findings, null, 2) - const dateStr = new Date().toISOString().split('T')[0] - downloadFile(jsonContent, `secuscan_findings_${dateStr}.json`, 'application/json') +/** Filename the backend also sets on Content-Disposition, rebuilt client-side + * because that header is not readable in cross-origin deployments. */ +export function findingsExportFilename(extension: string, now: Date = new Date()): string { + return `secuscan_findings_${now.toISOString().split('T')[0]}.${extension}` } diff --git a/frontend/testing/unit/AppRoutes.test.tsx b/frontend/testing/unit/AppRoutes.test.tsx index 07c8dc15c..95c803313 100644 --- a/frontend/testing/unit/AppRoutes.test.tsx +++ b/frontend/testing/unit/AppRoutes.test.tsx @@ -5,6 +5,7 @@ import { MemoryRouter, useLocation } from 'react-router-dom' import { AppRoutes } from '../../src/App' import { ThemeProvider } from '../../src/components/ThemeContext' import { AuthProvider } from '../../src/components/AuthContext' +import { ToastProvider } from '../../src/components/ToastContext' // Keep AppRoutes a focused routing test: stub the shell, mock the network. vi.mock('../../src/components/AppShell', () => ({ @@ -48,6 +49,7 @@ vi.mock('../../src/api', () => ({ ], }), cancelTask: vi.fn(), + exportFindings: vi.fn(), })) function PathProbe() { @@ -56,13 +58,16 @@ function PathProbe() { } function renderAt(path: string, extra?: React.ReactNode) { + // Provider stack mirrors App.tsx, which wraps AppRoutes in ToastProvider. return render( - - - {extra} - + + + + {extra} + + , ) diff --git a/frontend/testing/unit/pages/Findings.test.tsx b/frontend/testing/unit/pages/Findings.test.tsx index 596fce909..61d8d1bf7 100644 --- a/frontend/testing/unit/pages/Findings.test.tsx +++ b/frontend/testing/unit/pages/Findings.test.tsx @@ -8,14 +8,20 @@ import Findings from '../../../src/pages/Findings' vi.mock('../../../src/api', () => ({ getFindings: vi.fn(), + exportFindings: vi.fn(), })) vi.mock('../../../src/utils/exportUtils', () => ({ - exportFindingsAsCSV: vi.fn(), - exportFindingsAsJSON: vi.fn(), + downloadBlob: vi.fn(), + findingsExportFilename: (extension: string) => `secuscan_findings_2026-01-01.${extension}`, })) -import { exportFindingsAsCSV, exportFindingsAsJSON } from '../../../src/utils/exportUtils' +const mockAddToast = vi.fn() +vi.mock('../../../src/components/ToastContext', () => ({ + useToast: () => ({ addToast: mockAddToast }), +})) + +import { downloadBlob } from '../../../src/utils/exportUtils' vi.mock('../../../src/utils/date', async (importOriginal: any) => { const actual = await importOriginal() as typeof import('../../../src/utils/date') @@ -53,7 +59,7 @@ if (typeof global.ResizeObserver === 'undefined') { Object.defineProperty(HTMLElement.prototype, 'scrollHeight', { configurable: true, value: 800 }) Object.defineProperty(HTMLElement.prototype, 'offsetHeight', { configurable: true, value: 600 }) -import { getFindings } from '../../../src/api' +import { getFindings, exportFindings } from '../../../src/api' // ── Fixtures ───────────────────────────────────────────────────────────────── @@ -350,7 +356,7 @@ describe('Findings — virtualized list', () => { await userEvent.click(checkboxF2) expect(checkboxF2).toBeChecked() - expect(screen.getByRole('button', { name: /Bulk Export/i })).toBeInTheDocument() + expect(screen.getByRole('button', { name: /Export Selected \(1\)/i })).toBeInTheDocument() expect(screen.getByRole('heading', { name: /SQL Injection/i, level: 2 })).toBeInTheDocument() }) @@ -376,33 +382,68 @@ describe('Findings — virtualized list', () => { expect(screen.getByLabelText('Select CSRF Vulnerability')).not.toBeChecked() }) - it('trigger CSV and JSON bulk export calls utility function', async () => { - const findings = [ - makeFinding({ id: 'f1', title: 'SQL Injection', severity: 'critical' }), - ] - vi.mocked(getFindings).mockResolvedValue({ findings }) + // ── Bulk export (issue #1875) ────────────────────────────────────────────── + // Export is resolved by the backend from finding ids, so a selection is no + // longer limited to the findings this component has loaded. + async function renderWithFindings(findings: any[]) { + vi.mocked(getFindings).mockResolvedValue({ findings, total: findings.length }) + vi.mocked(exportFindings).mockResolvedValue({ + blob: new Blob(['exported']), + count: findings.length, + }) render() - await waitFor(() => expect(screen.queryByText('Synchronizing findings feed...')).not.toBeInTheDocument()) + await waitFor(() => + expect(screen.queryByText('Synchronizing findings feed...')).not.toBeInTheDocument(), + ) + } + + it('sends the selected ids to the backend instead of serializing loaded rows', async () => { + await renderWithFindings([ + makeFinding({ id: 'f1', title: 'SQL Injection', severity: 'critical' }), + makeFinding({ id: 'f2', title: 'CSRF Vulnerability', severity: 'high' }), + ]) await userEvent.click(screen.getByLabelText('Select SQL Injection')) + await userEvent.click(screen.getByRole('button', { name: /Export Selected \(1\)/i })) + await userEvent.click(screen.getByRole('button', { name: /Export as CSV/i })) - const bulkExportBtn = screen.getByRole('button', { name: /Bulk Export/i }) - await userEvent.click(bulkExportBtn) + await waitFor(() => expect(exportFindings).toHaveBeenCalledWith('csv', ['f1'])) + expect(downloadBlob).toHaveBeenCalled() + }) - const csvExportBtn = screen.getByRole('button', { name: /Export as CSV/i }) - const jsonExportBtn = screen.getByRole('button', { name: /Export as JSON/i }) + it('exports every owned finding when nothing is selected', async () => { + await renderWithFindings([ + makeFinding({ id: 'f1', title: 'SQL Injection', severity: 'critical' }), + ]) + + await userEvent.click(screen.getByRole('button', { name: /Export All \(1\)/i })) + await userEvent.click(screen.getByRole('button', { name: /Export as JSON/i })) + + // undefined ids — an empty array would mean "export nothing". + await waitFor(() => expect(exportFindings).toHaveBeenCalledWith('json', undefined)) + }) - expect(csvExportBtn).toBeInTheDocument() - expect(jsonExportBtn).toBeInTheDocument() + it('offers SARIF alongside CSV and JSON', async () => { + await renderWithFindings([makeFinding({ id: 'f1', title: 'SQL Injection' })]) - await userEvent.click(csvExportBtn) - expect(exportFindingsAsCSV).toHaveBeenCalled() + await userEvent.click(screen.getByRole('button', { name: /Export All/i })) + await userEvent.click(screen.getByRole('button', { name: /Export as SARIF/i })) - await userEvent.click(bulkExportBtn) - const newJsonExportBtn = await screen.findByRole('button', { name: /Export as JSON/i }) - await userEvent.click(newJsonExportBtn) - expect(exportFindingsAsJSON).toHaveBeenCalled() + await waitFor(() => expect(exportFindings).toHaveBeenCalledWith('sarif', undefined)) + }) + + it('reports a failed export instead of downloading an empty file', async () => { + await renderWithFindings([makeFinding({ id: 'f1', title: 'SQL Injection' })]) + vi.mocked(exportFindings).mockRejectedValue(new Error('Export failed: 500')) + + await userEvent.click(screen.getByRole('button', { name: /Export All/i })) + await userEvent.click(screen.getByRole('button', { name: /Export as CSV/i })) + + await waitFor(() => + expect(mockAddToast).toHaveBeenCalledWith('Export failed. Please try again.', 'error'), + ) + expect(downloadBlob).not.toHaveBeenCalled() }) }) diff --git a/frontend/testing/unit/utils/exportUtils.test.ts b/frontend/testing/unit/utils/exportUtils.test.ts index 3a2eae1bf..71e1030cc 100644 --- a/frontend/testing/unit/utils/exportUtils.test.ts +++ b/frontend/testing/unit/utils/exportUtils.test.ts @@ -1,66 +1,54 @@ -import { describe, test, expect } from "vitest"; -import { escapeCSV, serializeFindingsToCSV } from "../../../src/utils/exportUtils"; +import { describe, test, expect, vi, beforeEach, afterEach } from "vitest"; +import { downloadBlob, downloadFile, findingsExportFilename } from "../../../src/utils/exportUtils"; + +// The CSV/JSON serializers that used to live here moved to the backend with +// issue #1875 — the browser no longer builds export files, it downloads them. +// Their column contract is now pinned by testing/backend/unit/test_finding_export.py. describe("exportUtils utility", () => { - test("escapeCSV handles standard inputs", () => { - expect(escapeCSV("hello")).toBe("hello"); - expect(escapeCSV(123)).toBe("123"); - expect(escapeCSV(null)).toBe(""); - expect(escapeCSV(undefined)).toBe(""); + let createObjectURL: ReturnType; + let revokeObjectURL: ReturnType; + + beforeEach(() => { + createObjectURL = vi.fn(() => "blob:mock-url"); + revokeObjectURL = vi.fn(); + vi.stubGlobal("URL", { ...URL, createObjectURL, revokeObjectURL }); }); - test("escapeCSV escapes quotes, commas, and newlines", () => { - expect(escapeCSV('hello, world')).toBe('"hello, world"'); - expect(escapeCSV('hello "world"')).toBe('"hello ""world"""'); - expect(escapeCSV('hello\nworld')).toBe('"hello\nworld"'); + afterEach(() => { + vi.unstubAllGlobals(); }); - test("serializeFindingsToCSV generates correct headers and mapped rows", () => { - const sampleFindings = [ - { - id: "f-1", - title: "SQL Injection", - severity: "critical", - category: "Database", - target: "http://target1.local", - discovered_at: "2026-05-12T10:30:00Z", - cvss: 9.8, - cve: "CVE-2026-1234", - risk_score: 9.5, - confidence: 0.9, - validated: true, - analyst_status: "confirmed", - description: "An injection vulnerability in input parameter.", - remediation: "Use parameterized queries." - }, - { - id: "f-2", - title: "Information Disclosure, Version Leak", - severity: "info", - category: "Information", - target: "http://target2.local", - discovered_at: "2026-05-12T10:35:00Z", - cvss: null, - cve: undefined, - risk_score: 1.0, - confidence: 1.0, - validated: false, - analyst_status: "new", - description: "Version string \"1.2.3\" disclosed.", - remediation: "Disable version banners." - } - ]; + test("downloadBlob triggers a download and releases the object URL", () => { + const click = vi.fn(); + const anchor = document.createElement("a"); + anchor.click = click; + vi.spyOn(document, "createElement").mockReturnValueOnce(anchor); + + downloadBlob(new Blob(["payload"]), "findings.csv"); + + expect(anchor.download).toBe("findings.csv"); + expect(anchor.href).toBe("blob:mock-url"); + expect(click).toHaveBeenCalledOnce(); + // Leaving the URL alive would leak the blob for the lifetime of the document. + expect(revokeObjectURL).toHaveBeenCalledWith("blob:mock-url"); + expect(document.body.contains(anchor)).toBe(false); + }); + + test("downloadFile wraps its content in a blob of the given type", () => { + vi.spyOn(document, "createElement").mockReturnValueOnce( + Object.assign(document.createElement("a"), { click: vi.fn() }), + ); + + downloadFile("a,b,c", "findings.csv", "text/csv"); + + const blob = createObjectURL.mock.calls[0][0] as Blob; + expect(blob.type).toBe("text/csv"); + }); - const csvContent = serializeFindingsToCSV(sampleFindings); - - // Header check - expect(csvContent).toContain("ID,Title,Severity,Category,Target,Discovered At,CVSS,CVE,Risk Score,Confidence,Validated,Analyst Status,Description,Remediation"); - - // Row checks - expect(csvContent).toContain("f-1,SQL Injection,critical,Database,http://target1.local,2026-05-12T10:30:00Z,9.8,CVE-2026-1234,9.5,0.9,true,confirmed,An injection vulnerability in input parameter.,Use parameterized queries."); - - // Check comma escaping in title, quote escaping in description - expect(csvContent).toContain('"Information Disclosure, Version Leak"'); - expect(csvContent).toContain('"Version string ""1.2.3"" disclosed."'); + test("findingsExportFilename is dated and carries the format extension", () => { + const when = new Date("2026-05-12T10:30:00Z"); + expect(findingsExportFilename("csv", when)).toBe("secuscan_findings_2026-05-12.csv"); + expect(findingsExportFilename("sarif", when)).toBe("secuscan_findings_2026-05-12.sarif"); }); }); diff --git a/testing/backend/integration/test_findings_export.py b/testing/backend/integration/test_findings_export.py new file mode 100644 index 000000000..4ff5efdbb --- /dev/null +++ b/testing/backend/integration/test_findings_export.py @@ -0,0 +1,413 @@ +""" +testing/backend/integration/test_findings_export.py + +Issue #94 / #1875 — bulk-export findings across all pages, not just the ones +the browser has loaded. + +The export used to be assembled client-side from React state, so it could only +ever contain findings already fetched. These tests pin the backend endpoint +that replaces it: + + * selection is sent as ids and resolved against the database + * an omitted selection means "everything the caller owns" + * an *empty* selection means nothing — never everything + * owner scoping, redaction, and the request cap hold + * an export larger than one database batch comes out whole +""" + +import csv +import io +import json +import sqlite3 +import uuid + +import pytest + +from backend.secuscan.config import settings +from backend.secuscan.finding_export import CSV_COLUMNS + +ENDPOINT = "/api/v1/findings/export" + + +# --------------------------------------------------------------------------- +# Seeding helpers +# --------------------------------------------------------------------------- + +def _seed_task(task_id: str) -> None: + conn = sqlite3.connect(settings.database_path) + try: + conn.execute( + "INSERT INTO tasks (id, owner_id, plugin_id, tool_name, target, " + "status, inputs_json, structured_json, consent_granted) " + "VALUES (?, 'default', 'nmap', 'nmap', '127.0.0.1', " + "'completed', '{}', '{\"findings\": []}', 1)", + (task_id,), + ) + conn.commit() + finally: + conn.close() + + +def _seed_finding( + finding_id: str, + task_id: str, + *, + owner_id: str = "default", + title: str = "Test finding", + severity: str = "low", + description: str = "desc", + discovered_at: str = "2026-07-01 12:00:00", +) -> str: + conn = sqlite3.connect(settings.database_path) + try: + conn.execute( + "INSERT INTO findings (id, owner_id, task_id, plugin_id, title, category, " + "severity, target, description, remediation, discovered_at) " + "VALUES (?, ?, ?, 'nmap', ?, 'network', ?, '127.0.0.1', ?, 'fix', ?)", + (finding_id, owner_id, task_id, title, severity, description, discovered_at), + ) + conn.commit() + finally: + conn.close() + return finding_id + + +@pytest.fixture +def seeded_task(test_client): + """A completed task the seeded findings can hang off.""" + task_id = str(uuid.uuid4()) + _seed_task(task_id) + return task_id + + +def export(client, **body): + return client.post(ENDPOINT, json=body) + + +def csv_rows(response) -> list: + return list(csv.reader(io.StringIO(response.text))) + + +# --------------------------------------------------------------------------- +# 1. Selection is resolved server-side +# --------------------------------------------------------------------------- + +class TestSelectionResolution: + + def test_selected_ids_are_exported(self, test_client, seeded_task): + wanted = _seed_finding("f-wanted", seeded_task, title="Wanted") + _seed_finding("f-other", seeded_task, title="Other") + + r = export(test_client, finding_ids=[wanted], format="csv") + + assert r.status_code == 200 + rows = csv_rows(r) + assert rows[0] == list(CSV_COLUMNS) + assert len(rows) == 2 + assert rows[1][0] == "f-wanted" + + def test_omitted_selection_exports_everything_owned(self, test_client, seeded_task): + for i in range(4): + _seed_finding(f"f-{i}", seeded_task) + + r = export(test_client, format="csv") + + assert r.status_code == 200 + assert len(csv_rows(r)) == 5 # header + 4 + + def test_empty_selection_exports_nothing(self, test_client, seeded_task): + """An empty selection must never be read as 'export everything'.""" + for i in range(3): + _seed_finding(f"f-{i}", seeded_task) + + r = export(test_client, finding_ids=[], format="csv") + + assert r.status_code == 200 + rows = csv_rows(r) + assert rows[0] == list(CSV_COLUMNS) + assert len(rows) == 1, f"empty selection exported {len(rows) - 1} findings" + + def test_empty_export_is_still_a_valid_csv(self, test_client, seeded_task): + r = export(test_client, finding_ids=[], format="csv") + assert r.status_code == 200 + assert csv_rows(r)[0] == list(CSV_COLUMNS) + + def test_unknown_ids_are_skipped_not_rejected(self, test_client, seeded_task): + known = _seed_finding("f-known", seeded_task) + + r = export(test_client, finding_ids=[known, "does-not-exist"], format="csv") + + assert r.status_code == 200 + rows = csv_rows(r) + assert len(rows) == 2 + assert rows[1][0] == "f-known" + + def test_duplicate_ids_do_not_duplicate_rows(self, test_client, seeded_task): + """Contract guard: a repeated id must not repeat in the file. + + `IN (?, ?, ?)` already collapses duplicates, so this passes without the + explicit de-duplication too — it is here to catch a future rewrite that + resolves ids one query at a time. + """ + known = _seed_finding("f-known", seeded_task) + + r = export(test_client, finding_ids=[known, known, known], format="csv") + + assert r.status_code == 200 + assert len(csv_rows(r)) == 2 + + def test_duplicate_ids_are_collapsed_before_the_cap_is_applied( + self, test_client, seeded_task, monkeypatch + ): + """Repeats must not consume the request budget — one id is one finding.""" + monkeypatch.setattr(settings, "max_export_findings", 5) + known = _seed_finding("f-known", seeded_task) + + r = export(test_client, finding_ids=[known] * 20, format="csv") + + assert r.status_code == 200, "duplicates were counted against the export cap" + assert len(csv_rows(r)) == 2 + + +# --------------------------------------------------------------------------- +# 2. Owner scoping +# --------------------------------------------------------------------------- + +class TestOwnerScoping: + + def test_another_owners_finding_is_not_exported(self, test_client, seeded_task): + _seed_finding("f-theirs", seeded_task, owner_id="someone-else", title="Theirs") + + r = export(test_client, finding_ids=["f-theirs"], format="csv") + + assert r.status_code == 200 + assert len(csv_rows(r)) == 1, "exported a finding belonging to another owner" + + def test_export_all_excludes_other_owners(self, test_client, seeded_task): + _seed_finding("f-mine", seeded_task) + _seed_finding("f-theirs", seeded_task, owner_id="someone-else") + + r = export(test_client, format="csv") + + assert r.status_code == 200 + rows = csv_rows(r) + assert [row[0] for row in rows[1:]] == ["f-mine"] + + def test_count_header_does_not_confirm_foreign_ids(self, test_client, seeded_task): + """The reported count must not reveal that a foreign id exists.""" + _seed_finding("f-theirs", seeded_task, owner_id="someone-else") + + r = export(test_client, finding_ids=["f-theirs"], format="csv") + + assert r.headers["X-Export-Finding-Count"] == "0" + + +# --------------------------------------------------------------------------- +# 3. Formats +# --------------------------------------------------------------------------- + +class TestFormats: + + def test_csv_header_matches_declared_columns(self, test_client, seeded_task): + _seed_finding("f-1", seeded_task) + r = export(test_client, format="csv") + assert csv_rows(r)[0] == list(CSV_COLUMNS) + assert r.headers["content-type"].startswith("text/csv") + + def test_json_export_is_an_array_of_findings(self, test_client, seeded_task): + _seed_finding("f-1", seeded_task, title="First") + _seed_finding("f-2", seeded_task, title="Second") + + r = export(test_client, format="json") + + assert r.status_code == 200 + payload = json.loads(r.text) + assert isinstance(payload, list) + assert {f["id"] for f in payload} == {"f-1", "f-2"} + assert {f["title"] for f in payload} == {"First", "Second"} + + def test_empty_json_export_parses_as_empty_array(self, test_client, seeded_task): + r = export(test_client, finding_ids=[], format="json") + assert json.loads(r.text) == [] + + def test_json_export_omits_owner_id(self, test_client, seeded_task): + _seed_finding("f-1", seeded_task) + r = export(test_client, format="json") + assert all("owner_id" not in f for f in json.loads(r.text)) + + def test_sarif_export_is_valid_sarif(self, test_client, seeded_task): + _seed_finding("f-1", seeded_task, title="Open port 22") + + r = export(test_client, format="sarif") + + assert r.status_code == 200 + payload = json.loads(r.text) + assert payload["version"] == "2.1.0" + assert len(payload["runs"]) == 1 + assert len(payload["runs"][0]["results"]) == 1 + + def test_unknown_format_is_rejected(self, test_client, seeded_task): + r = export(test_client, format="xlsx") + assert r.status_code == 422 + + @pytest.mark.parametrize( + "export_format,extension", + [("csv", "csv"), ("json", "json"), ("sarif", "sarif")], + ) + def test_filename_extension_matches_format( + self, test_client, seeded_task, export_format, extension + ): + r = export(test_client, format=export_format) + disposition = r.headers["content-disposition"] + assert disposition.startswith("attachment;") + assert disposition.endswith(f'.{extension}"') + + +# --------------------------------------------------------------------------- +# 4. Redaction +# --------------------------------------------------------------------------- + +class TestRedaction: + + def test_secrets_in_description_are_redacted_in_csv(self, test_client, seeded_task): + _seed_finding( + "f-secret", + seeded_task, + description="Credential found: AKIAIOSFODNN7EXAMPLE in config", + ) + + r = export(test_client, format="csv") + + assert "AKIAIOSFODNN7EXAMPLE" not in r.text + assert "[REDACTED]" in r.text + + def test_secrets_in_description_are_redacted_in_json(self, test_client, seeded_task): + _seed_finding( + "f-secret", + seeded_task, + description="password=hunter2secret was accepted", + ) + + r = export(test_client, format="json") + + assert "hunter2secret" not in r.text + assert "[REDACTED]" in r.text + + def test_secrets_are_redacted_in_sarif(self, test_client, seeded_task): + _seed_finding( + "f-secret", + seeded_task, + description="Credential found: AKIAIOSFODNN7EXAMPLE in config", + ) + + r = export(test_client, format="sarif") + + assert "AKIAIOSFODNN7EXAMPLE" not in r.text + + +# --------------------------------------------------------------------------- +# 5. Volume — the acceptance criterion +# --------------------------------------------------------------------------- + +class TestLargeExports: + + def test_export_spanning_many_batches_is_complete(self, test_client, seeded_task, monkeypatch): + """500 selected findings export whole, across several database batches. + + Every finding shares one discovered_at, so batch boundaries land on + rows the sort cannot tell apart — the case where a missing ordering + tiebreaker would repeat or drop rows. + """ + monkeypatch.setattr(settings, "export_batch_size", 40) + + ids = [f"bulk-{i:04d}" for i in range(500)] + conn = sqlite3.connect(settings.database_path) + try: + conn.executemany( + "INSERT INTO findings (id, owner_id, task_id, plugin_id, title, category, " + "severity, target, description, remediation, discovered_at) " + "VALUES (?, 'default', ?, 'nmap', 'Bulk finding', 'network', " + "'low', '127.0.0.1', 'desc', 'fix', '2026-07-01 12:00:00')", + [(fid, seeded_task) for fid in ids], + ) + conn.commit() + finally: + conn.close() + + r = export(test_client, finding_ids=ids, format="csv") + + assert r.status_code == 200 + exported = [row[0] for row in csv_rows(r)[1:]] + assert len(exported) == 500 + assert sorted(exported) == sorted(ids) + assert r.headers["X-Export-Finding-Count"] == "500" + + def test_export_all_spanning_many_batches_is_complete( + self, test_client, seeded_task, monkeypatch + ): + monkeypatch.setattr(settings, "export_batch_size", 25) + + ids = [f"bulk-{i:04d}" for i in range(120)] + conn = sqlite3.connect(settings.database_path) + try: + conn.executemany( + "INSERT INTO findings (id, owner_id, task_id, plugin_id, title, category, " + "severity, target, description, remediation, discovered_at) " + "VALUES (?, 'default', ?, 'nmap', 'Bulk finding', 'network', " + "'low', '127.0.0.1', 'desc', 'fix', '2026-07-01 12:00:00')", + [(fid, seeded_task) for fid in ids], + ) + conn.commit() + finally: + conn.close() + + r = export(test_client, format="csv") + + exported = [row[0] for row in csv_rows(r)[1:]] + assert sorted(exported) == sorted(ids) + + +# --------------------------------------------------------------------------- +# 6. Request cap +# --------------------------------------------------------------------------- + +class TestExportCap: + + def test_too_many_ids_is_rejected(self, test_client, seeded_task, monkeypatch): + monkeypatch.setattr(settings, "max_export_findings", 5) + + r = export(test_client, finding_ids=[f"id-{i}" for i in range(6)], format="csv") + + assert r.status_code == 400 + assert "maximum 5" in r.json()["detail"] + + def test_export_all_beyond_cap_is_rejected(self, test_client, seeded_task, monkeypatch): + monkeypatch.setattr(settings, "max_export_findings", 2) + for i in range(3): + _seed_finding(f"f-{i}", seeded_task) + + r = export(test_client, format="csv") + + assert r.status_code == 400 + assert "Select a subset" in r.json()["detail"] + + def test_at_the_cap_is_accepted(self, test_client, seeded_task, monkeypatch): + monkeypatch.setattr(settings, "max_export_findings", 3) + for i in range(3): + _seed_finding(f"f-{i}", seeded_task) + + r = export(test_client, format="csv") + + assert r.status_code == 200 + assert len(csv_rows(r)) == 4 + + +# --------------------------------------------------------------------------- +# 7. Auth +# --------------------------------------------------------------------------- + +class TestAuthRequired: + + def test_export_requires_an_api_key(self, test_client, seeded_task): + r = test_client.post(ENDPOINT, json={"format": "csv"}, headers={"X-Api-Key": ""}) + assert r.status_code in (401, 403) diff --git a/testing/backend/unit/test_finding_export.py b/testing/backend/unit/test_finding_export.py new file mode 100644 index 000000000..845c80825 --- /dev/null +++ b/testing/backend/unit/test_finding_export.py @@ -0,0 +1,243 @@ +""" +testing/backend/unit/test_finding_export.py + +Issue #94 / #1875 — serializer-level tests for the bulk findings export. + +The CSV column contract asserted here was previously enforced by the frontend +test for ``serializeFindingsToCSV``. That serializer is gone: the file is built +on the backend now, so the contract has to be pinned on this side or nothing +catches a column being renamed, reordered, or dropped. +""" + +import csv +import io +import json + +import pytest + +from backend.secuscan.finding_export import ( + CSV_COLUMNS, + export_filename, + finding_csv_row, + redacted_finding, + stream_csv, + stream_json, +) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +async def _batches(*batches): + for batch in batches: + yield list(batch) + + +async def _collect(streamer, *batches) -> str: + return "".join([chunk async for chunk in streamer(_batches(*batches))]) + + +SAMPLE = { + "id": "f-1", + "owner_id": "default", + "title": "SQL Injection", + "severity": "critical", + "category": "Database", + "target": "http://target1.local", + "discovered_at": "2026-05-12T10:30:00Z", + "cvss": 9.8, + "cve": "CVE-2026-1234", + "risk_score": 9.5, + "confidence": 0.9, + "validated": True, + "analyst_status": "confirmed", + "description": "An injection vulnerability in input parameter.", + "remediation": "Use parameterized queries.", +} + + +# --------------------------------------------------------------------------- +# CSV column contract +# --------------------------------------------------------------------------- + +class TestCsvContract: + + def test_column_order_is_pinned(self): + assert list(CSV_COLUMNS) == [ + "ID", + "Title", + "Severity", + "Category", + "Target", + "Discovered At", + "CVSS", + "CVE", + "Risk Score", + "Confidence", + "Validated", + "Analyst Status", + "Description", + "Remediation", + ] + + def test_row_maps_every_column(self): + assert finding_csv_row(SAMPLE) == [ + "f-1", + "SQL Injection", + "critical", + "Database", + "http://target1.local", + "2026-05-12T10:30:00Z", + "9.8", + "CVE-2026-1234", + "9.5", + "0.9", + "true", + "confirmed", + "An injection vulnerability in input parameter.", + "Use parameterized queries.", + ] + + def test_row_length_matches_header(self): + assert len(finding_csv_row(SAMPLE)) == len(CSV_COLUMNS) + + def test_missing_values_become_empty_strings(self): + row = finding_csv_row({"id": "f-2"}) + assert row[0] == "f-2" + assert row[6] == "", "absent CVSS must be blank, not 'None'" + assert row[8] == "", "absent risk score must be blank, not 'None'" + + def test_zero_is_not_confused_with_absent(self): + row = finding_csv_row({"id": "f-3", "cvss": 0, "confidence": 0.0}) + assert row[6] == "0" + assert row[9] == "0.0" + + def test_unvalidated_finding_renders_false(self): + assert finding_csv_row({"id": "f-4"})[10] == "false" + + +# --------------------------------------------------------------------------- +# CSV escaping — commas, quotes and newlines must survive a round trip +# --------------------------------------------------------------------------- + +class TestCsvEscaping: + + @pytest.mark.anyio + async def test_commas_and_quotes_round_trip(self): + findings = [ + { + "id": "f-1", + "title": "Information Disclosure, Version Leak", + "description": 'Version string "1.2.3" disclosed.', + } + ] + + text = await _collect(stream_csv, findings) + rows = list(csv.reader(io.StringIO(text))) + + assert rows[1][1] == "Information Disclosure, Version Leak" + assert rows[1][12] == 'Version string "1.2.3" disclosed.' + + @pytest.mark.anyio + async def test_newline_in_a_field_does_not_break_the_row_count(self): + findings = [{"id": "f-1", "description": "line one\nline two"}] + + rows = list(csv.reader(io.StringIO(await _collect(stream_csv, findings)))) + + assert len(rows) == 2, "an embedded newline leaked into the row structure" + assert rows[1][12] == "line one\nline two" + + +# --------------------------------------------------------------------------- +# Streaming shape +# --------------------------------------------------------------------------- + +class TestStreaming: + + @pytest.mark.anyio + async def test_csv_header_is_emitted_before_any_batch(self): + chunks = [chunk async for chunk in stream_csv(_batches([SAMPLE]))] + assert chunks[0].startswith("ID,Title,Severity") + + @pytest.mark.anyio + async def test_csv_spans_multiple_batches(self): + text = await _collect( + stream_csv, + [{"id": "a"}, {"id": "b"}], + [{"id": "c"}], + ) + rows = list(csv.reader(io.StringIO(text))) + assert [row[0] for row in rows[1:]] == ["a", "b", "c"] + + @pytest.mark.anyio + async def test_empty_csv_is_header_only(self): + rows = list(csv.reader(io.StringIO(await _collect(stream_csv)))) + assert rows == [list(CSV_COLUMNS)] + + @pytest.mark.anyio + async def test_json_spans_multiple_batches(self): + text = await _collect(stream_json, [{"id": "a"}, {"id": "b"}], [{"id": "c"}]) + assert [f["id"] for f in json.loads(text)] == ["a", "b", "c"] + + @pytest.mark.anyio + async def test_empty_json_is_an_empty_array(self): + assert json.loads(await _collect(stream_json)) == [] + + @pytest.mark.anyio + async def test_json_survives_non_serializable_values(self): + """A stray datetime must not abort a 5000-row export mid-stream.""" + from datetime import datetime + + text = await _collect(stream_json, [{"id": "a", "seen": datetime(2026, 5, 12)}]) + + assert json.loads(text)[0]["seen"].startswith("2026-05-12") + + +# --------------------------------------------------------------------------- +# Redaction +# --------------------------------------------------------------------------- + +class TestRedaction: + + def test_secrets_in_free_text_are_scrubbed(self): + result = redacted_finding( + {"id": "f-1", "description": "Found AKIAIOSFODNN7EXAMPLE in the bucket policy"} + ) + assert "AKIAIOSFODNN7EXAMPLE" not in result["description"] + assert "[REDACTED]" in result["description"] + + def test_metadata_is_redacted(self): + result = redacted_finding({"id": "f-1", "metadata": {"note": "password=hunter2secret"}}) + assert "hunter2secret" not in json.dumps(result["metadata"]) + + def test_owner_id_is_dropped(self): + assert "owner_id" not in redacted_finding(SAMPLE) + + def test_the_input_finding_is_not_mutated(self): + original = {"id": "f-1", "description": "Found AKIAIOSFODNN7EXAMPLE here"} + redacted_finding(original) + assert original["description"] == "Found AKIAIOSFODNN7EXAMPLE here" + + def test_non_secret_content_is_preserved(self): + result = redacted_finding(dict(SAMPLE)) + assert result["target"] == "http://target1.local" + assert result["description"] == SAMPLE["description"] + + +# --------------------------------------------------------------------------- +# Filenames +# --------------------------------------------------------------------------- + +class TestFilenames: + + @pytest.mark.parametrize( + "export_format,expected", + [ + ("csv", "secuscan_findings_2026-05-12.csv"), + ("json", "secuscan_findings_2026-05-12.json"), + ("sarif", "secuscan_findings_2026-05-12.sarif"), + ], + ) + def test_filename_carries_date_and_extension(self, export_format, expected): + assert export_filename(export_format, "2026-05-12") == expected From bb99007ee6bb58ba9b3af814fa8e0b9ea5265bc1 Mon Sep 17 00:00:00 2001 From: Subramaniyajothi6 Date: Tue, 4 Aug 2026 16:13:45 +0530 Subject: [PATCH 2/2] Neutralize CSV formula injection in the findings export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moving CSV generation to the backend reintroduced CWE-1236: cells were written raw, so a finding titled =HYPERLINK("http://attacker","click") became a live formula when the export was opened in a spreadsheet. Finding titles, targets and descriptions carry scanner output — page titles, banners, reflected headers — so the content is attacker-influenced. Prefix any cell starting with =, +, - or @ with a single quote, which makes the spreadsheet read it as literal text. Applied to every column rather than the free-text ones: a column that is only safe while the data is well-formed is not a guarantee worth relying on. This deliberately mirrors ReportGenerator._sanitize_csv_cell, which #2394 adds for the task-report CSV. Both write findings into a spreadsheet and must not disagree about what is safe. They should be folded into one helper once both have landed; they are separate only because #2394 is unmerged and this path has to be safe on its own. --- backend/secuscan/finding_export.py | 63 ++++++++++++----- .../integration/test_findings_export.py | 39 ++++++++++- testing/backend/unit/test_finding_export.py | 67 +++++++++++++++++++ 3 files changed, 151 insertions(+), 18 deletions(-) diff --git a/backend/secuscan/finding_export.py b/backend/secuscan/finding_export.py index edde38bcd..2693e4288 100644 --- a/backend/secuscan/finding_export.py +++ b/backend/secuscan/finding_export.py @@ -95,6 +95,30 @@ def redacted_finding(finding: Dict[str, Any]) -> Dict[str, Any]: return exported +# Spreadsheet applications evaluate a cell beginning with any of these as a +# formula, so a finding title of ``=HYPERLINK("http://attacker","click")`` +# becomes a live link when the export is opened. +_FORMULA_PREFIXES: tuple[str, ...] = ("=", "+", "-", "@") + + +def sanitize_csv_cell(value: str) -> str: + """Neutralize CSV formula injection (CWE-1236). + + Findings carry scanner output — page titles, banners, reflected headers — + so cell content is attacker-influenced. Prefixing with a single quote makes + the spreadsheet treat the cell as literal text. + + Deliberately mirrors ``ReportGenerator._sanitize_csv_cell`` in + :mod:`backend.secuscan.reporting` (added by #2394 for the task-report CSV). + Both paths write findings to a spreadsheet and must not disagree about what + is safe; folding them into one helper is a worthwhile follow-up once both + have landed. + """ + if value and value[0] in _FORMULA_PREFIXES: + return "'" + value + return value + + def _text(value: Any) -> str: if value is None: return "" @@ -109,22 +133,31 @@ def _number(value: Any) -> str: def finding_csv_row(finding: Dict[str, Any]) -> List[str]: - """Build one CSV row from an already-redacted finding.""" + """Build one CSV row from an already-redacted finding. + + Every cell goes through :func:`sanitize_csv_cell`. The booleans and numeric + columns cannot start with a formula prefix in valid data, but they are not + exempted — a column that is only safe while the data is well-formed is not + a guarantee worth relying on. + """ return [ - _text(finding.get("id")), - _text(finding.get("title")), - _text(finding.get("severity")), - _text(finding.get("category")), - _text(finding.get("target")), - _text(finding.get("discovered_at")), - _number(finding.get("cvss")), - _text(finding.get("cve")), - _number(finding.get("risk_score")), - _number(finding.get("confidence")), - "true" if finding.get("validated") else "false", - _text(finding.get("analyst_status")), - _text(finding.get("description")), - _text(finding.get("remediation")), + sanitize_csv_cell(cell) + for cell in ( + _text(finding.get("id")), + _text(finding.get("title")), + _text(finding.get("severity")), + _text(finding.get("category")), + _text(finding.get("target")), + _text(finding.get("discovered_at")), + _number(finding.get("cvss")), + _text(finding.get("cve")), + _number(finding.get("risk_score")), + _number(finding.get("confidence")), + "true" if finding.get("validated") else "false", + _text(finding.get("analyst_status")), + _text(finding.get("description")), + _text(finding.get("remediation")), + ) ] diff --git a/testing/backend/integration/test_findings_export.py b/testing/backend/integration/test_findings_export.py index 4ff5efdbb..4a8173e6c 100644 --- a/testing/backend/integration/test_findings_export.py +++ b/testing/backend/integration/test_findings_export.py @@ -306,7 +306,40 @@ def test_secrets_are_redacted_in_sarif(self, test_client, seeded_task): # --------------------------------------------------------------------------- -# 5. Volume — the acceptance criterion +# 5. CSV formula injection (CWE-1236) +# --------------------------------------------------------------------------- + +class TestFormulaInjection: + """Same defence #2394 adds to the task-report CSV, for the findings CSV.""" + + def test_formula_injection_from_a_scan_is_neutralized(self, test_client, seeded_task): + """A hostile finding title must reach the CSV as text, not a formula. + + Titles come from scanner output, so this is reachable end to end by + anything that reflects a page title or a service banner. + """ + _seed_finding( + "f-formula", + seeded_task, + title='=HYPERLINK("http://attacker.example","click")', + ) + + r = export(test_client, format="csv") + + assert r.status_code == 200 + title_cell = csv_rows(r)[1][1] + assert title_cell.startswith("'="), f"formula written raw: {title_cell!r}" + + def test_description_is_defended_too(self, test_client, seeded_task): + _seed_finding("f-desc", seeded_task, description="@SUM(A1:A2)") + + r = export(test_client, format="csv") + + assert csv_rows(r)[1][12] == "'@SUM(A1:A2)" + + +# --------------------------------------------------------------------------- +# 6. Volume — the acceptance criterion # --------------------------------------------------------------------------- class TestLargeExports: @@ -368,7 +401,7 @@ def test_export_all_spanning_many_batches_is_complete( # --------------------------------------------------------------------------- -# 6. Request cap +# 7. Request cap # --------------------------------------------------------------------------- class TestExportCap: @@ -403,7 +436,7 @@ def test_at_the_cap_is_accepted(self, test_client, seeded_task, monkeypatch): # --------------------------------------------------------------------------- -# 7. Auth +# 8. Auth # --------------------------------------------------------------------------- class TestAuthRequired: diff --git a/testing/backend/unit/test_finding_export.py b/testing/backend/unit/test_finding_export.py index 845c80825..ffd81fa9e 100644 --- a/testing/backend/unit/test_finding_export.py +++ b/testing/backend/unit/test_finding_export.py @@ -20,6 +20,7 @@ export_filename, finding_csv_row, redacted_finding, + sanitize_csv_cell, stream_csv, stream_json, ) @@ -149,6 +150,72 @@ async def test_newline_in_a_field_does_not_break_the_row_count(self): assert rows[1][12] == "line one\nline two" +# --------------------------------------------------------------------------- +# CSV formula injection (CWE-1236) +# --------------------------------------------------------------------------- +# Findings carry scanner output, so cell content is attacker-influenced. Same +# defence as ReportGenerator._sanitize_csv_cell for the task-report CSV (#2394); +# the two paths must not disagree about what is safe to hand a spreadsheet. + +class TestFormulaInjection: + + @pytest.mark.parametrize( + "payload", + [ + '=HYPERLINK("http://attacker.example","click")', + "+cmd|'/C calc'!A0", + "-2+3", + "@SUM(A1:A2)", + ], + ) + def test_formula_prefixes_are_neutralized(self, payload): + assert sanitize_csv_cell(payload) == "'" + payload + + def test_ordinary_text_is_untouched(self): + assert sanitize_csv_cell("Open port 22") == "Open port 22" + assert sanitize_csv_cell("") == "" + assert sanitize_csv_cell("9.8") == "9.8" + + def test_every_text_column_is_defended(self): + """A guard on the title alone would leave eleven other ways in.""" + hostile = "=1+1" + row = finding_csv_row( + { + "id": hostile, + "title": hostile, + "severity": hostile, + "category": hostile, + "target": hostile, + "discovered_at": hostile, + "cve": hostile, + "analyst_status": hostile, + "description": hostile, + "remediation": hostile, + } + ) + assert all(cell in ("'=1+1", "", "false") for cell in row), row + + @pytest.mark.anyio + async def test_payload_survives_as_literal_text_through_the_csv(self): + payload = '=HYPERLINK("http://attacker.example","click")' + findings = [{"id": "f-1", "title": payload}] + + text = await _collect(stream_csv, findings) + rows = list(csv.reader(io.StringIO(text))) + + # Quoted and prefixed: a reader gets the original string back, but a + # spreadsheet sees text rather than a formula. + assert rows[1][1] == "'" + payload + assert not rows[1][1].startswith("=") + + @pytest.mark.anyio + async def test_json_export_is_not_quote_prefixed(self): + """The guard is a CSV concern — JSON consumers must get the real value.""" + payload = "=1+1" + text = await _collect(stream_json, [{"id": "f-1", "title": payload}]) + assert json.loads(text)[0]["title"] == payload + + # --------------------------------------------------------------------------- # Streaming shape # ---------------------------------------------------------------------------