Skip to content

Fix negative indexing and ReadTimeOut errors#3474

Closed
OM-JADHAV25 wants to merge 10 commits intoOWASP:mainfrom
OM-JADHAV25:fix/fuzz-test
Closed

Fix negative indexing and ReadTimeOut errors#3474
OM-JADHAV25 wants to merge 10 commits intoOWASP:mainfrom
OM-JADHAV25:fix/fuzz-test

Conversation

@OM-JADHAV25
Copy link
Contributor

Proposed change

Resolves #3414

This PR fixes a fuzz-test failure caused by negative limit values being passed into recentMilestones, which resulted in Django raising: Negative indexing is not supported

Now, if a negative (or zero) limit is provided, the resolver safely returns an empty list instead of slicing with a negative value.

Additional improvement
During fuzz testing, the logs also showed ReadTimeout errors when extremely large limit values were generated, causing huge responses and expensive database queries.
By clamping limits, queries remain bounded and the GraphQL API becomes more stable under fuzzing, resolving these timeout issues.

Checklist

  • Required: I followed the contributing workflow
  • Required: I verified that my code works as intended and resolves the issue as described
  • Required: I ran make check-test locally: all warnings addressed, tests passed
  • I used AI for code, documentation, tests, or communication related to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 22, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Query limit validation improved: invalid/negative limits return empty results and limits are clamped to safe ranges.
  • Improvements

    • Unified and stricter pagination limits across APIs for more consistent performance.
    • Health metrics now show most recently created items first.
    • Project lookup now normalizes and validates keys to prevent malformed queries.
    • Snapshot lists (projects, releases, users, chapters) are capped to recent-item limits.
  • Improvements

    • Project search returns up to a larger, controlled number of results.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Validations and hard limits were added across GraphQL resolvers and queries: new constants introduced, negative/non-positive limits now return empty lists, per-request limits are clamped, some ordering and projection adjustments made, and tests updated to accommodate queryset .only(...) usage.

Changes

Cohort / File(s) Summary
GitHub repository node
backend/apps/github/api/internal/nodes/repository.py
Added MAX_RECENT_MILESTONES_LIMIT = 100; recent_milestones now returns [] for negative limits and clamps positive limits to the max.
OWASP project nodes & queries
backend/apps/owasp/api/internal/nodes/project.py, backend/apps/owasp/api/internal/queries/project.py
Reduced project MAX limits (1000→100), added MAX_PROJECT_KEY_LENGTH = 50, trimmed/validated project key input, reject non-positive recent_projects limits, clamp limits, expanded .only(...) projections, and guard blank login in is_project_leader.
OWASP snapshot nodes & queries
backend/apps/owasp/api/internal/nodes/snapshot.py, backend/apps/owasp/api/internal/queries/snapshot.py
Introduced RECENT_* constants (50) and removed field new_chapters in favor of resolver with prefetch; applied slice limits on new_projects/releases/users/chapters; reduced snapshots MAX_LIMIT (100→10), validate/clamp limits, and simplify query with field projection and descending order.
Tests
backend/tests/apps/owasp/api/internal/queries/project_test.py
Updated mocks to account for .only(...) queryset chaining—mock queryset (mock_qs) whose get() is asserted instead of mocking Project.objects.get.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • arkid15r
  • kasya
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Out of Scope Changes check ❓ Inconclusive Most changes align with issue #3414, but some modifications appear broader than the stated issue scope (e.g., MAX_LIMIT reduced 1000→10 in snapshot.py, sort order changes in project.py, new_chapters resolver refactoring). Clarify whether snapshot MAX_LIMIT reduction to 10, sort order changes, and new_chapters refactoring are intentional scope expansions or secondary fixes related to fuzz testing resilience.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix negative indexing and ReadTimeOut errors' directly summarizes the main change—handling negative limits and clamping large limits to prevent errors.
Description check ✅ Passed The description is relevant and explains the issue (negative limit causing Django errors), the fix (return empty list for negative limits), and additional improvement (clamping limits to prevent ReadTimeout).
Linked Issues check ✅ Passed The PR meets all objectives from issue #3414: it prevents negative indexing by returning empty lists for non-positive limits, clamps limits to defined maximums across multiple resolvers, and hardened the API against extreme values.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link

@arkid15r
Copy link
Collaborator

Closing in favor of #3478

@arkid15r arkid15r closed this Jan 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Solve recentProjects query negative indexing

2 participants

Comments