Skip to content

Increase JWT abuse limit#11012

Merged
Meldiron merged 1 commit into1.8.xfrom
chore-increase-jwt-abuse
Dec 23, 2025
Merged

Increase JWT abuse limit#11012
Meldiron merged 1 commit into1.8.xfrom
chore-increase-jwt-abuse

Conversation

@Meldiron
Copy link
Copy Markdown
Contributor

What does this PR do?

(Provide a description of what this PR does and why it's needed.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)

Related PRs and Issues

  • (Related PR or issue)

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 23, 2025

📝 Walkthrough

Walkthrough

The pull request updates abuse rate limiting metadata for two API endpoints in app/controllers/api/account.php. The hardcoded abuse-limit value of 100 is replaced with APP_LIMIT_WRITE_RATE_DEFAULT * 2 for the sendSessionAlert endpoint and the POST /v1/account (jwts) endpoint. Additionally, a new abuse-time metadata label is introduced using APP_LIMIT_WRITE_RATE_PERIOD_DEFAULT for both endpoints. These changes consolidate rate limiting configuration to use application-level constants instead of hardcoded values.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description contains only a template with uncompleted sections and no actual explanation of the changes, making it impossible to understand the PR's purpose from the description alone. Fill in the description template with a clear explanation of why the JWT abuse limit was increased, the test plan used to verify changes, and any related issues or PRs.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Increase JWT abuse limit' is clear and directly related to the main change in the changeset, which updates abuse rate limiting metadata for JWT-related endpoints.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore-increase-jwt-abuse

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.

@github-actions
Copy link
Copy Markdown

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
libpng 1.6.51-r0 CVE-2025-66293 HIGH
libpng-dev 1.6.51-r0 CVE-2025-66293 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
app/controllers/api/account.php (1)

2958-2959: The constants are properly defined and the pattern follows established conventions across the codebase.

The concerns about constant definitions are resolved: APP_LIMIT_WRITE_RATE_DEFAULT and APP_LIMIT_WRITE_RATE_PERIOD_DEFAULT are defined in app/init/constants.php with appropriate values (both 60).

However, a valid secondary concern exists: while this JWT endpoint now uses constants, most other endpoints in account.php continue to use hardcoded abuse-limit values (10, 50, 100, 60). The * 2 multiplier and explicit abuse-time label are not unique to this change—they follow an established pattern across the codebase (databases, tokens, storage modules), where heavier write operations (create, update, upsert) use doubled limits. If modernizing account.php to use constants is desired, consider applying this pattern consistently across all endpoints rather than piecemeal.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cdbd80c and e111b4c.

📒 Files selected for processing (1)
  • app/controllers/api/account.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: Setup & Build Appwrite Image
  • GitHub Check: CodeQL
  • GitHub Check: scan

@Meldiron Meldiron merged commit d6a40a3 into 1.8.x Dec 23, 2025
36 of 43 checks passed
@arielweinberger arielweinberger self-requested a review December 23, 2025 14:48
@github-actions
Copy link
Copy Markdown

✨ Benchmark results

  • Requests per second: 1,186
  • Requests with 200 status code: 213,500
  • P99 latency: 0.164956623

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,186 1,257
200 213,500 226,256
P99 0.164956623 0.16423512

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants