[MindOfqc]

Symptom

GET /api/v1/knowledge-bases/{kb_id}/chunks (and the legacy GET /api/chunk/list) build the SQL offset straight from the caller-supplied page / page_size with no bounds check. Out-of-range values are forwarded verbatim into the document store:

• ?page=0 sends OFFSET -20, which SQLite treats as 0, so page 0 returns page 1's rows while the response body still echoes "page": 0. Any client that keeps advancing until it sees its own page number back loops forever on the same rows.
• ?page_size=-1 sends LIMIT -1, which SQLite interprets as "no limit", so a single request returns every chunk of the document instead of one page.
• ?page_size=0 sends LIMIT 0 and returns an empty page even when total is non-zero.

Trigger input

GET /api/v1/knowledge-bases/kb-1/chunks?document_id=doc-1&page=0&page_size=20
GET /api/v1/knowledge-bases/kb-1/chunks?document_id=doc-1&page=1&page_size=-1

Both routes parse the query with _to_int(request.query_params.get("page"), 1), which only supplies a default when the value is missing or unparsable — it does not bound a supplied negative or zero value. They are reachable by any authenticated dashboard user / Open API key with the knowledge-base scope.

Root cause

astrbot/dashboard/services/knowledge_base_service.py:785 (before the fix):

offset = (page - 1) * page_size

The sibling handlers in the same service validate their inputs first — list_kbs at astrbot/dashboard/services/knowledge_base_service.py:324 and list_documents at astrbot/dashboard/services/knowledge_base_service.py:506 both run page = max(page, 1) / page_size = max(page_size, 1) with the comment "Clamp page and page_size to at least 1 before calculating offsets/slices." list_chunks was simply missing that guard, so the un-clamped pair is passed straight to DocumentStorage.get_documents() → SQLAlchemy .limit(limit).offset(offset) (astrbot/core/db/vec_db/faiss_impl/document_storage.py:270-273).

Fix

Apply the same clamp that its siblings already use, so page/page_size are at least 1 before the offset is computed and before they are echoed back in the response. No API surface, schema, or route signature changes, so the generated dashboard API client is unaffected.

Red (unmodified origin/master)

E       AssertionError: assert [{'doc_id': '... 'limit': -1}] == [{'doc_id': '..., 'limit': 1}]
E
E         At index 0 diff: {'doc_id': 'doc-1', 'offset': 0, 'limit': -1} != {'doc_id': 'doc-1', 'offset': 0, 'limit': 1}
...
FAILED tests/unit/test_kb_chunk_pagination_clamp.py::test_list_chunks_clamps_zero_page_to_first_page
FAILED tests/unit/test_kb_chunk_pagination_clamp.py::test_list_chunks_clamps_non_positive_page_size
2 failed, 1 warning in 25.05s

Green

..                                                                       [100%]
2 passed, 1 warning in 28.21s

Regression check

tests/unit/test_kb_chunk_pagination_clamp.py tests/unit/test_knowledge_base_service_contract.py
tests/unit/test_kb_document_cleanup.py tests/unit/test_kb_manager_resilience.py
tests/unit/test_kb_name_uuid_lookup.py tests/unit/test_kb_upload_atomicity.py
-> 55 passed, 1 warning in 42.34s

ruff format . and ruff check . pass on the touched files. The change is pure Python logic and stays compatible with Python 3.10+.

Prepared with an AI coding agent.

Summary by Sourcery

Prevent invalid knowledge-base chunk pagination parameters from producing incorrect or unbounded query results.

Bug Fixes:

• Clamp knowledge-base chunk pagination inputs to valid minimums so non-positive pages and page sizes cannot produce invalid SQL offsets or unlimited results.

Tests:

• Add regression coverage verifying chunk pagination is clamped and normalized in service responses and storage calls.

[sourcery-ai]

Hey - I've reviewed your changes and they look great!

Sourcery assessment

Approved.

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook