You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tooling.apache.org by "asf-tooling (via GitHub)" <gi...@apache.org> on 2026/04/03 18:40:39 UTC
[I] Unbounded Response Sizes on Multiple List Endpoints (tooling-trusted-releases)
asf-tooling opened a new issue, #1062:
URL: https://github.com/apache/tooling-trusted-releases/issues/1062
**ASVS Level(s):** L2-only
**Description:**
### Summary
Multiple endpoints return unbounded result sets without pagination or limits. Affected endpoints include: (1) `/api/checks/list/<project>/<version>` which may return thousands of check results for releases with large archives, (2) `/api/release/paths/<project>/<version>` which collects all file paths in memory before returning, (3) `/admin/data/<model>` which loads all database records, and (4) `/admin/consistency` which walks entire filesystem. Code comments acknowledge the issue ('TODO: We should perhaps paginate this') but it remains unaddressed.
### Details
The issue exists in:
- `atr/api/__init__.py` checks_list() function
- `atr/api/__init__.py` release_paths() function
- `atr/admin/__init__.py` _data_browse() function
- `atr/admin/__init__.py` consistency() function
### Recommended Remediation
Add pagination to API endpoints using query parameters (limit/offset):
```python
# For checks_list() and release_paths()
_MAX_RESULTS = 100
@api.get
async def checks_list(
session: web.Public,
project_key: safe.ProjectKey,
version_key: safe.VersionKey,
limit: int = _MAX_RESULTS,
offset: int = 0
) -> dict:
# Validate pagination parameters (after fixing typo in FINDING-155)
_pagination_args_validate(limit, offset)
# Apply limit and offset to query
checks = # ... query with .limit(limit).offset(offset)
total_count = # ... count query without limit/offset
return {
'checks': checks,
'total': total_count,
'limit': limit,
'offset': offset
}
# For admin endpoints
_MAX_BROWSE_RECORDS = 500
@admin.get
async def _data_browse(
session: web.Admin,
model: str,
page: int = 1
) -> web.QuartResponse:
limit = _MAX_BROWSE_RECORDS
offset = (page - 1) * limit
records = # ... query with .limit(limit).offset(offset)
total_count = # ... count query
return await render("admin/data_browse.html",
records=records,
total=total_count,
page=page,
total_pages=(total_count + limit - 1) // limit)
```
### Acceptance Criteria
- [ ] Pagination added to /api/checks/list endpoint
- [ ] Pagination added to /api/release/paths endpoint
- [ ] Pagination added to /admin/data/<model> endpoint
- [ ] Page size limits enforced on all endpoints
- [ ] Total count included in paginated responses
- [ ] Unit tests verify pagination works correctly
- [ ] Unit tests verify page size limits are enforced
- [ ] Integration tests verify large result sets are paginated
- [ ] Documentation updated with pagination parameters
### References
- Source reports: L2:15.1.3.md
- Related findings: FINDING-012, FINDING-052, FINDING-207
- ASVS sections: 15.1.3
- CWE: CWE-770
### Priority
Medium
---
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tooling.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tooling.apache.org
For additional commands, e-mail: dev-help@tooling.apache.org
Re: [I] Unbounded Response Sizes on Multiple List Endpoints (tooling-trusted-releases)
Posted by "sbp (via GitHub)" <gi...@apache.org>.
sbp closed issue #1062: Unbounded Response Sizes on Multiple List Endpoints
URL: https://github.com/apache/tooling-trusted-releases/issues/1062
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tooling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Unbounded Response Sizes on Multiple List Endpoints (tooling-trusted-releases)
Posted by "sbp (via GitHub)" <gi...@apache.org>.
sbp commented on issue #1062:
URL: https://github.com/apache/tooling-trusted-releases/issues/1062#issuecomment-5499268769
Resolved by 4aa2f5289e2512d9323ca7caac37e3456903ccec.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@tooling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Unbounded Response Sizes on Multiple List Endpoints (tooling-trusted-releases)
Posted by "asf-tooling (via GitHub)" <gi...@apache.org>.
asf-tooling commented on issue #1062:
URL: https://github.com/apache/tooling-trusted-releases/issues/1062#issuecomment-4409859432
<!-- gofannon-issue-triage-bot v2 -->
**Automated triage** — analyzed at `main@2da7807a`
**Type:** `new_feature` • **Classification:** `actionable` • **Confidence:** `high`
**Application domain(s):** `web_api_infrastructure`, `admin_operations`, `shared_infrastructure`
### Summary
Multiple API endpoints return unbounded result sets without pagination. The issue is confirmed by a TODO comment in checks_list() and by the absence of limit/offset on several endpoints. The codebase already has an established pagination pattern (ReleasesListQuery, SshKeysListQuery, TasksListQuery dataclasses with limit/offset) and a pagination_args_validate utility, so this is a matter of applying the existing pattern to the affected endpoints.
### Where this lives in the code today
#### `atr/api/__init__.py` — `checks_list` (lines 67-98)
_needs modification_
This endpoint explicitly acknowledges the need for pagination via the TODO comment and returns all check results unbounded.
```python
@api.typed
@quart_schema.validate_response(models.api.ChecksListResults, 200)
async def checks_list(
_checks_list: Literal["checks/list"],
project_key: safe.ProjectKey,
version_key: safe.VersionKey,
) -> DictResponse:
"""
URL: GET /checks/list/<project_key>/<version_key>
List checks by project and version.
Checks are only conducted during the compose a draft phase. This endpoint
only returns the checks for the most recent draft revision. Once a release
has been promoted to the vote phase or beyond, the checks returned are
still those for the compose phase.
Warning: the check results include results for archive members, so there
may potentially be thousands or results or more.
"""
# TODO: We should perhaps paginate this
async with db.session() as data:
release_key = sql.release_key(str(project_key), str(version_key))
release = await data.release(key=release_key).demand(exceptions.NotFound(f"Release {release_key} not found"))
check_results = await interaction.checks_for(release, caller_data=data)
return models.api.ChecksListResults(
endpoint="/checks/list",
checks=check_results,
checks_revision=release.unwrap_revision_number,
current_phase=release.phase,
).model_dump(mode="json"), 200
```
#### `atr/models/validation.py` — `pagination_args_validate` (lines 70-86)
_currently does this_
Pagination validation utility already exists and enforces max limit of 1000 and max offset of 1000000.
```python
def pagination_args_validate(query_args: Any) -> None:
# Users could request any amount using limit=N with arbitrarily high N
# We therefore limit the maximum limit to 1000
if hasattr(query_args, "limit"):
limit = query_args.limit
if limit > 1000:
raise ValueError("Maximum limit of 1000 exceeded")
elif limit < 1:
raise ValueError("Minimum limit less than 1 is nonsense")
# Users could request any amount using offset=N with arbitrarily high N
# We therefore limit the maximum offset to 1000000
if hasattr(query_args, "offset"):
offset = query_args.offset
if offset > 1000000:
raise ValueError("Maximum offset of 1000000 exceeded")
elif offset < 0:
raise ValueError("Minimum offset less than 0 is nonsense")
```
#### `atr/models/api.py` — `ReleasesListQuery` (lines 550-554)
_currently does this_
Established pattern for pagination query parameters using a dataclass parsed from query string.
```python
@dataclasses.dataclass
class ReleasesListQuery:
offset: int = 0
limit: int = 20
phase: str | None = None
```
#### `atr/models/api.py` — `ReleasesListResults` (lines 557-560)
_currently does this_
Established pattern for paginated API response including total count.
```python
class ReleasesListResults(schema.Strict):
endpoint: Literal["/releases/list"] = schema.alias("endpoint")
data: Sequence[sql.Release]
count: int
```
#### `atr/models/api.py` — `ChecksListResults` (lines 36-45)
_needs modification_
Response model needs additional fields for pagination metadata (count, limit, offset).
```python
class ChecksListResults(schema.Strict):
endpoint: Literal["/checks/list"] = schema.alias("endpoint")
checks: Sequence[sql.CheckResult]
checks_revision: str = schema.example("00005")
current_phase: sql.ReleasePhase = schema.example(sql.ReleasePhase.RELEASE_CANDIDATE)
@pydantic.field_validator("current_phase", mode="before")
@classmethod
def current_phase_to_enum(cls, v):
return sql.ReleasePhase(v) if isinstance(v, str) else v
```
#### `atr/admin/__init__.py` — `consistency` (lines 145-173)
_needs modification_
Admin consistency endpoint loads all releases and walks the entire filesystem without limits.
```python
@admin.typed
async def consistency(_session: web.Committer, _consistency: Literal["consistency"]) -> web.TextResponse:
"""
URL: GET /consistency
Check for consistency between the database and the filesystem.
"""
# Get all releases from the database
async with db.session() as data:
releases = await data.release().all()
database_dirs = []
for release in releases:
path = paths.release_directory_version(release)
database_dirs.append(str(path))
if len(set(database_dirs)) != len(database_dirs):
raise base.ASFQuartException("Duplicate release directories in database", errorcode=500)
# Get all releases from the filesystem
filesystem_dirs = await _get_filesystem_dirs()
# Pair them up where possible
paired_dirs = []
for database_dir in database_dirs[:]:
for filesystem_dir in filesystem_dirs[:]:
if database_dir == filesystem_dir:
paired_dirs.append(database_dir)
database_dirs.remove(database_dir)
filesystem_dirs.remove(filesystem_dir)
break
```
#### `atr/blueprints/common.py` — `build_api_path` (lines 122-136)
_currently does this_
The API path builder already supports dataclass parameters parsed from query strings, which is how pagination query params are passed.
```python
def build_api_path(
func: Callable[..., Any],
) -> tuple[
str,
list[tuple[str, type]],
dict[str, str],
tuple[str, type[pydantic.BaseModel]] | None,
tuple[str, type] | None,
tuple[str, type] | None,
list[str],
]:
"""Inspect a function's type hints to build a URL path for an API route.
Accepts URL path params for data, Literal strings for plain URL text, dataclasses for GET query params
and Pydantic model params for POST bodies
```
### Where new code would go
- `atr/models/api.py` — after symbol ReleasesListQuery
Add ChecksListQuery dataclass following the established pattern for pagination query parameters.
### Proposed approach
Apply the existing pagination pattern (dataclass query params + validation + count in response) to the checks_list endpoint as the highest-priority fix. The codebase already has ReleasesListQuery, SshKeysListQuery, and TasksListQuery as precedents. Add a ChecksListQuery dataclass with limit/offset defaults, call pagination_args_validate on it, then slice the results from interaction.checks_for() and return the total count. The ChecksListResults model gets additional optional fields (count, limit, offset) so existing clients aren't broken.
For the admin endpoints (_data_browse and consistency), these are lower priority since they're admin-only and already behind authentication. A simple hard limit on _data_browse records and a note about consistency being inherently unbounded (it's a diagnostic tool) would suffice for now.
### Suggested patches
#### `atr/models/api.py`
Add ChecksListQuery dataclass for pagination params and extend ChecksListResults with pagination metadata.
````diff
--- a/atr/models/api.py
+++ b/atr/models/api.py
@@ -30,11 +30,19 @@ class ResultsTypeError(TypeError):
pass
+@dataclasses.dataclass
+class ChecksListQuery:
+ offset: int = 0
+ limit: int = 100
+
+
class ChecksListResults(schema.Strict):
endpoint: Literal["/checks/list"] = schema.alias("endpoint")
checks: Sequence[sql.CheckResult]
checks_revision: str = schema.example("00005")
current_phase: sql.ReleasePhase = schema.example(sql.ReleasePhase.RELEASE_CANDIDATE)
+ count: int = schema.example(42)
+ limit: int = schema.example(100)
+ offset: int = schema.example(0)
@pydantic.field_validator("current_phase", mode="before")
@classmethod
````
#### `atr/api/__init__.py`
Add pagination support to checks_list using the new ChecksListQuery dataclass and validation.
````diff
--- a/atr/api/__init__.py
+++ b/atr/api/__init__.py
@@ -63,6 +63,7 @@ async def checks_list(
_checks_list: Literal["checks/list"],
project_key: safe.ProjectKey,
version_key: safe.VersionKey,
+ query: models.api.ChecksListQuery = models.api.ChecksListQuery(),
) -> DictResponse:
"""
URL: GET /checks/list/<project_key>/<version_key>
@@ -76,16 +77,21 @@ async def checks_list(
Warning: the check results include results for archive members, so there
may potentially be thousands or results or more.
"""
- # TODO: We should perhaps paginate this
+ validation.pagination_args_validate(query)
async with db.session() as data:
release_key = sql.release_key(str(project_key), str(version_key))
release = await data.release(key=release_key).demand(exceptions.NotFound(f"Release {release_key} not found"))
check_results = await interaction.checks_for(release, caller_data=data)
+ total_count = len(check_results)
+ paginated_checks = check_results[query.offset : query.offset + query.limit]
+
return models.api.ChecksListResults(
endpoint="/checks/list",
- checks=check_results,
+ checks=paginated_checks,
checks_revision=release.unwrap_revision_number,
current_phase=release.phase,
+ count=total_count,
+ limit=query.limit,
+ offset=query.offset,
).model_dump(mode="json"), 200
````
#### `atr/api/__init__.py`
Add pagination support to checks_list_revision using the same pattern.
````diff
--- a/atr/api/__init__.py
+++ b/atr/api/__init__.py
@@ -90,6 +90,7 @@ async def checks_list_revision(
project_key: safe.ProjectKey,
version_key: safe.VersionKey,
revision: safe.RevisionNumber,
+ query: models.api.ChecksListQuery = models.api.ChecksListQuery(),
) -> DictResponse:
"""
URL: GET /checks/list/<project_key>/<version_key>/<revision>
@@ -102,6 +103,7 @@ async def checks_list_revision(
Warning: the check results include results for archive members, so there
may potentially be thousands or results or more.
"""
+ validation.pagination_args_validate(query)
async with db.session() as data:
release_key = sql.release_key(str(project_key), str(version_key))
release_result = await data.release(key=release_key).demand(
@@ -114,9 +116,14 @@ async def checks_list_revision(
check_results = await interaction.checks_for(release_result, revision=revision, caller_data=data)
+ total_count = len(check_results)
+ paginated_checks = check_results[query.offset : query.offset + query.limit]
+
return models.api.ChecksListResults(
endpoint="/checks/list",
- checks=check_results,
+ checks=paginated_checks,
checks_revision=str(revision),
current_phase=release_result.phase,
+ count=total_count,
+ limit=query.limit,
+ offset=query.offset,
).model_dump(mode="json"), 200
````
### Open questions
- The interaction.checks_for() function (in atr/db/interaction.py, not provided) fetches all results in memory before slicing. For truly large result sets, database-level LIMIT/OFFSET would be more efficient. Need to check if interaction.checks_for can accept limit/offset parameters.
- The release_paths() endpoint is referenced in the issue but the source wasn't provided in the truncated file. Need to see its implementation to add pagination.
- The _data_browse() admin function implementation was not visible in the truncated source. Its pagination would follow the same pattern but needs the actual code.
- Adding count/limit/offset to ChecksListResults is additive but changes the response schema. Need to confirm this won't break existing API consumers expecting exactly the old shape.
- The consistency admin endpoint is inherently a diagnostic tool that needs to see all data. A hard cap there may hide real inconsistencies. Perhaps a streaming response or summary-only mode would be more appropriate.
### Files examined
- `atr/api/__init__.py`
- `atr/admin/__init__.py`
- `atr/db/__init__.py`
- `atr/models/validation.py`
- `atr/models/api.py`
- `atr/blueprints/api.py`
- `atr/blueprints/common.py`
- `atr/models/sql.py`
### Related issues
This issue appears related to: #1072.
_Both address unbounded response sizes and pagination issues on API endpoints_
---
*Draft from a triage agent. A human reviewer should validate before merging any change. The agent did not run tests or verify diffs apply.*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@tooling.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tooling.apache.org
For additional commands, e-mail: dev-help@tooling.apache.org