rodin/review-bot
2
1
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases 6 Wiki Activity

Harden PostReview against unbounded diff size #92

New Issue
Closed
opened 2026-05-12 21:56:45 +00:00 by rodin · 0 comments
rodin commented 2026-05-12 21:56:45 +00:00
Owner
Copy Link
Copy Source

Context

Identified during review of PR #90 (Gitea adapter phase 2). Both the security-review-bot and self-review flagged this.

Problem

In gitea/adapter.go, PostReview calls GetPullRequestDiff without any size guard when translating comment positions. For very large PRs, this fetches the full diff into memory unconditionally, which could cause elevated memory/CPU usage (potential DoS vector).

Suggested Approach

  • Add a configurable max-diff-size threshold (e.g., 10 MB)
  • Return a clear error if the diff exceeds the threshold, skipping position translation
  • Alternatively, stream/limit reads in the HTTP client for successful GET responses
  • Consider Content-Length checking before reading the body

Acceptance Criteria

  • PostReview respects a maximum diff size
  • Clear error returned when diff exceeds limit
  • Test covering the size-exceeded path
## Context Identified during review of PR #90 (Gitea adapter phase 2). Both the security-review-bot and self-review flagged this. ## Problem In `gitea/adapter.go`, `PostReview` calls `GetPullRequestDiff` without any size guard when translating comment positions. For very large PRs, this fetches the full diff into memory unconditionally, which could cause elevated memory/CPU usage (potential DoS vector). ## Suggested Approach - Add a configurable max-diff-size threshold (e.g., 10 MB) - Return a clear error if the diff exceeds the threshold, skipping position translation - Alternatively, stream/limit reads in the HTTP client for successful GET responses - Consider `Content-Length` checking before reading the body ## Acceptance Criteria - [ ] `PostReview` respects a maximum diff size - [ ] Clear error returned when diff exceeds limit - [ ] Test covering the size-exceeded path
rodin self-assigned this 2026-05-13 11:52:19 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
ai-review
bug
needs-review
ready
Ready for human review
 self-reviewed
Self-review passed since last push
 wip
Work in progress - do not start new work on this PR
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
security-review-bot rodin (Rodin)
No Assignees rodin
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: rodin/review-bot#92
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?