OWASP / OWASP/OpenCRE

/rest/v1/tags and db.get_by_tags() return unbounded results — pagination TODO never implemented

Open
#1,087 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
180
Forks
137
Avg merge
3d 23h
Merged PRs (30d)
21

Description

What is the issue?

GET /rest/v1/tags (find_document_by_tag in application/web/web_main.py) calls db.get_by_tags(tags), which runs two unbounded queries:

Node.query.filter(*nodes_where_clause).all()
CRE.query.filter(*cre_where_clause).all()

Neither has a LIMIT. Both the route and the DB method carry an explicit TODO from the pagination never having been implemented:

  • application/web/web_main.py:300# TODO: (spyros) paginate
  • application/database/db.py:1615# TODO: (spyros), when we have useful tags this needs to be refactored so both standards and CREs become the same query and it gets paginated

A tag matched by many nodes/CREs (tag matching is a LIKE "%tag%" substring match, so even a short/common tag string can match broadly) returns every matching row in one response — unbounded response size on a public endpoint.

Expected behaviour

/rest/v1/tags should support the same page / items_per_page query params as the sibling /rest/v1/id/... route, bounded by the existing
ITEMS_PER_PAGE (20) / MAX_ITEMS_PER_PAGE (100) constants.

Proposed solution

Add get_by_tags_with_pagination() alongside the existing get_by_tags() (same pattern as get_nodes() / get_nodes_with_pagination() already in db.py), so non-paginated callers (e.g. line 109's internal tag-linking use) are unaffected.

Design decision: get_by_tags combines two separate queries (Node, CRE) into one list, so a single .paginate() call (as used in get_nodes_with_pagination) doesn't map directly onto it. Proposed v1: paginate the Node and CRE queries independently with the same
page/items_per_page, and return them as two labeled lists rather than one merged list:

{"nodes": {...}, "cres": {...}, "page": ..., "total_pages": ...}

This keeps each query's pagination correct and avoids manual offset math across heterogeneous result sets. Happy to adjust if a merged/interleaved result is preferred.

CSV/Markdown/OSCAL export formats (opt_format) would keep using the existing unpaginated get_by_tags(), matching how /rest/v1/id/... already treats exports as an unpaginated special case.

Acceptance criteria

  • /rest/v1/tags accepts page / items_per_page, bounded by MAX_ITEMS_PER_PAGE.
  • Export formats (CSV/Markdown/OSCAL) unaffected.
  • New tests in db_test.py / web_main_test.py covering pagination boundaries (page 1, last page, page beyond range, custom items_per_page).
  • Existing test_get_by_tags / test_find_document_by_tag still pass unmodified (backward compatible for non-paginated callers).
  • make lint / make mypy / make test green.

I'd like to work on this

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with find_document_by_tag in application/web/web_main.py and compare its pagination handling with the sibling /rest/v1/id/... route. Then inspect get_by_tags() and get_nodes_with_pagination() in application/database/db.py, followed by the related tests in db_test.py and web_main_test.py. Done means bounded page results for the JSON endpoint, unchanged export and non-paginated behavior, passing boundary tests, and green make lint, make mypy, and make test.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend, database, testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.