security: cap server-driven pagination to prevent client DoS

[cyberjunky]

Summary

Fixes a client-side denial-of-service from an external security audit (report 3995's sibling, report 3993, CVSS 4.0 8.2 / High).

get_activities_by_date() and get_goals() paginated with while True: and broke only when the server returned an empty (falsy) page. There was no cap on page count, total rows, or the start offset — loop termination depended entirely on the server.

Impact

A hostile / compromised / MITM server that returns a non-empty page for every start offset hangs the embedding application indefinitely with unbounded memory growth (report PoC: ~3.2M iterations and 3.2M+ accumulated list items in 5 seconds; eventual OOM). Availability only — no confidentiality/integrity impact.

The other while True: loops in the package (fit.py) iterate over a local BytesIO and are bounded; get_activities is already caller-bounded by limit. Only these two paginators were affected.

Fix

New module constant MAX_PAGINATED_REQUESTS = 2000 (at 20–30 items/page that is 40k–60k items — far beyond any legitimate account). Both loops are now bounded for _ in range(MAX_PAGINATED_REQUESTS) with a for/else that raises GarminConnectConnectionError("Pagination exceeded …") when the cap is hit — failing loudly rather than silently truncating, since a server feeding that many pages is hostile or broken and the data would be suspect anyway. Normal termination (empty page → break) is unchanged.

Tests

• test_get_activities_by_date_pagination_is_capped — never-empty server → raises GarminConnectConnectionError after exactly MAX_PAGINATED_REQUESTS calls
• test_get_goals_pagination_is_capped — same for goals
• Existing test_get_activities_by_date_paginates_until_empty still passes (normal termination unchanged)

Full suite: 261 passed; the 13 test_workout_constants failures and the test_typed.py import error (missing optional pydantic) are pre-existing — verified identical on a clean tree.

Summary by CodeRabbit

• Bug Fixes
  • Added a safeguard to prevent activity and goal retrieval from running indefinitely when the service returns continuous paginated results.
  • Requests now stop after a defined limit and report a connection error when pagination cannot complete.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: adb6c1ae-43da-40ff-8f60-f4850673e6f1

📥 Commits

Reviewing files that changed from the base of the PR and between b4c7a26 and 7d92f57.

📒 Files selected for processing (2)

• garminconnect/__init__.py
• tests/test_garmin_unit.py

---

Walkthrough

The module adds a public limit of 2,000 pagination requests. Activity-by-date and goal pagination now raise GarminConnectConnectionError when the limit is reached. Unit tests cover bounded and normal pagination.

Changes

Pagination safety

Layer / File(s)	Summary
Bound activity pagination garminconnect/__init__.py, tests/test_garmin_unit.py	The activity-by-date loop uses MAX_PAGINATED_REQUESTS and raises GarminConnectConnectionError after the limit. Tests verify the request count and error message.
Bound goal pagination garminconnect/__init__.py, tests/test_garmin_unit.py	The goal pagination loop uses the shared limit. Tests verify bounded failure, request offsets, accumulated results, and termination on an empty page.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: rifusaki, tamcore, mannmann2

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the pagination cap and its security purpose, which matches the main change.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch security/pagination-cap

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/test_garmin_unit.py`:
- Around line 1562-1572: Add a normal-termination pagination test alongside
test_get_goals_pagination_is_capped, mocking garmin.connectapi to return
non-empty pages followed by an empty page. Assert garmin.get_goals() completes
successfully, verifies the expected pagination requests including the advanced
start values, and exercises the empty-page break path.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 3e18f957-1c61-4cb9-bc2d-5c7663b972e2

📥 Commits

Reviewing files that changed from the base of the PR and between 6da91d3 and 118ce7e.

📒 Files selected for processing (1)

• tests/test_garmin_unit.py