Skip to content

fix(api): paginate GET /task/{task_id}/result to prevent OOM on large scans#1697

Open
anshul23102 wants to merge 1 commit into
utksh1:mainfrom
anshul23102:fix/1621-findings-pagination
Open

fix(api): paginate GET /task/{task_id}/result to prevent OOM on large scans#1697
anshul23102 wants to merge 1 commit into
utksh1:mainfrom
anshul23102:fix/1621-findings-pagination

Conversation

@anshul23102

Copy link
Copy Markdown
Contributor

Description

Fixes #1621

A wide-range scan (e.g. full-range nmap against a /16 subnet) can produce tens of thousands of finding rows for a single task. GET /task/{task_id}/result loaded every one of them into memory with no LIMIT before the response was serialised — this can OOM-crash the backend process and freeze the frontend rendering thousands of table rows at once.

Root cause: the query SELECT * FROM findings WHERE owner_id = ? AND task_id = ? ORDER BY ... had no bound, and severity_counts/finding_groups/asset_summary were all computed by iterating the full in-memory list.

Approach

  • Added page/per_page query params (default 100, max 500 — matching the convention already used by GET /findings) and applied LIMIT/OFFSET to the findings list actually returned to the client.
  • total_findings now comes from a lightweight COUNT(*) query instead of len(findings), so it reflects the whole scan even though only one page of full finding objects is loaded.
  • Aggregate views (severity_counts, finding_groups, asset_summary) must reflect the entire scan, not just the current page — but loading every row to get there would reintroduce the exact OOM this fixes. They're computed from a bounded sample (capped at 5000 findings) only for scans large enough to need it, and from the full set otherwise, so aggregates stay exactly accurate for the overwhelming majority of real scans while memory use is capped for pathological ones.
  • Cache key now includes page/per_page so different pages don't collide.
  • Response includes total_findings, page, per_page, and has_more_findings so a future frontend change can page through results instead of receiving a silently truncated list.

Related Issues

Closes #1621

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

New test file testing/backend/integration/test_task_result_pagination.py (7 tests):

  • default page size (100) and has_more_findings
  • page navigation (page 2, last page)
  • severity_counts reflects the whole scan regardless of which page is requested
  • per_page upper bound rejects values above 500 with 422
  • small scans (fewer than one page) return everything on page 1
  • a 6000-finding scan exercises the aggregation-sample cap and confirms it never loads more than 5000 rows for aggregate computation

Full suite run locally:

pytest testing/backend/unit -q -m "not benchmark"        # 2210 passed (6 pre-existing failures, confirmed identical on unmodified main — sandbox/subprocess tests, unrelated to this change)
pytest testing/backend/integration -q -m "not benchmark"  # 291 passed, 9 skipped (includes the 7 new tests)
ruff check backend testing/backend                          # all checks passed
scripts/check-artifacts.sh origin/main                       # clean

Checklist

  • My code follows the code style of this project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation.
  • My changes generate no new warnings.

Additional Notes

I'm contributing this through GSSoC. While investigating this issue I found the same pattern (loading every finding row to build aggregate groups with no bound) in GET /findings and GET /finding-groups. I've flagged that separately rather than expanding this PR's scope, since this issue's reproduction steps specifically describe the per-task result view.

… scans

A wide-range scan (e.g. full-range nmap against a /16 subnet) can produce
tens of thousands of finding rows for a single task. GET /task/{task_id}/result
loaded every one of them into memory with no LIMIT before the response was
serialised, which can OOM-crash the backend process and freeze the frontend
rendering thousands of table rows at once.

Root cause: the query 'SELECT * FROM findings WHERE owner_id = ? AND
task_id = ? ORDER BY ...' had no bound, and severity_counts/finding_groups/
asset_summary were all computed by iterating the full in-memory list.

Fix:
- Add page/per_page query params (default 100, max 500 -- same convention
  already used by GET /findings) and apply LIMIT/OFFSET to the findings
  list actually returned to the client.
- Compute total_findings via a lightweight COUNT(*) query instead of
  len(findings), so it reflects the whole scan even though only one page
  of full finding objects is loaded.
- Aggregate views (severity_counts, finding_groups, asset_summary) must
  reflect the entire scan, not just the current page, but loading every
  row to get there would reintroduce the exact OOM this fixes. They're
  computed from a bounded sample (capped at 5000 findings) for scans large
  enough to need it, and from the full set for everything else -- so
  aggregates stay accurate for the overwhelming majority of real scans
  while memory use is capped for pathological ones.
- Cache key now includes page/per_page so different pages don't collide.
- Response includes total_findings, page, per_page, and has_more_findings
  so the frontend can page through results instead of receiving a
  silently truncated list.

Testing:
- testing/backend/integration/test_task_result_pagination.py: 7 new tests
  covering default page size, page navigation, last-page has_more_findings,
  severity_counts reflecting the whole scan regardless of page, the
  per_page upper bound (422 above 500), small scans returning everything
  on page 1, and the aggregation-sample cap on a 6000-finding scan.
- pytest testing/backend/unit -q -m "not benchmark" -- 2210 passed (6
  pre-existing failures unrelated to this change, confirmed identical on
  unmodified main -- sandbox/subprocess tests failing in this environment)
- pytest testing/backend/integration -q -m "not benchmark" -- 291 passed,
  9 skipped
- ruff check backend testing/backend -- all checks passed
- scripts/check-artifacts.sh origin/main -- clean

Fixes utksh1#1621
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: findings endpoint loads full result set into memory, causes OOM on large scans

1 participant