-
Notifications
You must be signed in to change notification settings - Fork 4.2k
cutting #8824
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: preview
Are you sure you want to change the base?
cutting #8824
Changes from 1 commit
e745bd2
bf249d0
df32e6a
600e0b4
beb2bdd
272db44
c91dcf8
a10b60e
c5a2e26
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,7 @@ | |
|
|
||
| class ApiKeyRateThrottle(SimpleRateThrottle): | ||
| scope = "api_key" | ||
| rate = os.environ.get("API_KEY_RATE_LIMIT", "60/minute") | ||
| rate = os.environ.get("API_KEY_RATE_LIMIT", "10000/minute") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Inspect which request handlers use these throttles.
rg -n -C2 --glob '!**/*.backup' '\b(ApiKeyRateThrottle|ServiceTokenRateThrottle)\b'Repository: makeplane/plane Length of output: 1659 🏁 Script executed: cat -n apps/api/plane/api/rate_limit.pyRepository: makeplane/plane Length of output: 3587 🏁 Script executed: # Check if there are any comments or documentation about rate limits
rg -n "rate|limit|RATE|LIMIT" apps/api/plane/api/rate_limit.py -iRepository: makeplane/plane Length of output: 845 🏁 Script executed: # Look for environment variable documentation
fd -i readme -o -i config -o -i env | head -20Repository: makeplane/plane Length of output: 175 🏁 Script executed: # Check for any GitHub issues or comments about rate limiting
rg -i "rate.*limit|throttl" --type md --type txt . | grep -i "api.*key\|service.*token" | head -20Repository: makeplane/plane Length of output: 162 🏁 Script executed: # Search all documentation for rate limit mentions
rg -i "rate.*limit|throttle" --type md . | head -40Repository: makeplane/plane Length of output: 162 🏁 Script executed: # Check if there are any other env var defaults documented
rg -i "API_KEY_RATE_LIMIT|SERVICE.*TOKEN.*RATE" . --type mdRepository: makeplane/plane Length of output: 162 🏁 Script executed: # Look for any comments in code about why these specific values were chosen
rg -B5 -A5 "10000" apps/api/plane/api/rate_limit.pyRepository: makeplane/plane Length of output: 733 🏁 Script executed: # Check for environment configuration examples or documentation
fd -i ".env" -o -i "example" -o -i "config" | grep -E "env|config|example" | head -20Repository: makeplane/plane Length of output: 175 🏁 Script executed: # Look for any changelog or migration notes about rate limit changes
rg -i "rate.limit" --type md . | grep -E "change|update|migration|default"Repository: makeplane/plane Length of output: 162 🏁 Script executed: # Check if ServiceTokenRateThrottle has any environment variable option elsewhere
rg -i "SERVICE_TOKEN_RATE" .Repository: makeplane/plane Length of output: 41 Code defaults contradict documentation and undermine rate limit security. The defaults in Revert both to documented and secure defaults: Proposed fix class ApiKeyRateThrottle(SimpleRateThrottle):
scope = "api_key"
- rate = os.environ.get("API_KEY_RATE_LIMIT", "10000/minute")
+ rate = os.environ.get("API_KEY_RATE_LIMIT", "60/minute")
@@
class ServiceTokenRateThrottle(SimpleRateThrottle):
scope = "service_token"
- rate = "10000/minute"
+ rate = os.environ.get("SERVICE_TOKEN_RATE_LIMIT", "300/minute")🤖 Prompt for AI Agents |
||
|
|
||
| def get_cache_key(self, request, view): | ||
| # Retrieve the API key from the request header | ||
|
|
@@ -52,7 +52,7 @@ def allow_request(self, request, view): | |
|
|
||
| class ServiceTokenRateThrottle(SimpleRateThrottle): | ||
| scope = "service_token" | ||
| rate = "300/minute" | ||
| rate = "10000/minute" | ||
|
|
||
| def get_cache_key(self, request, view): | ||
| # Retrieve the API key from the request header | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| # Copyright (c) 2023-present Plane Software, Inc. and contributors | ||
| # SPDX-License-Identifier: AGPL-3.0-only | ||
| # See the LICENSE file for details. | ||
|
|
||
| # python imports | ||
| import os | ||
|
|
||
| # Third party imports | ||
| from rest_framework.throttling import SimpleRateThrottle | ||
|
|
||
|
|
||
| class ApiKeyRateThrottle(SimpleRateThrottle): | ||
| scope = "api_key" | ||
| rate = os.environ.get("API_KEY_RATE_LIMIT", "60/minute") | ||
|
|
||
| def get_cache_key(self, request, view): | ||
| # Retrieve the API key from the request header | ||
| api_key = request.headers.get("X-Api-Key") | ||
| if not api_key: | ||
| return None # Allow the request if there's no API key | ||
|
|
||
| # Use the API key as part of the cache key | ||
| return f"{self.scope}:{api_key}" | ||
|
|
||
| def allow_request(self, request, view): | ||
| allowed = super().allow_request(request, view) | ||
|
|
||
| if allowed: | ||
| now = self.timer() | ||
| # Calculate the remaining limit and reset time | ||
| history = self.cache.get(self.key, []) | ||
|
|
||
| # Remove old histories | ||
| while history and history[-1] <= now - self.duration: | ||
| history.pop() | ||
|
|
||
| # Calculate the requests | ||
| num_requests = len(history) | ||
|
|
||
| # Check available requests | ||
| available = self.num_requests - num_requests | ||
|
|
||
| # Unix timestamp for when the rate limit will reset | ||
| reset_time = int(now + self.duration) | ||
|
|
||
| # Add headers | ||
| request.META["X-RateLimit-Remaining"] = max(0, available) | ||
| request.META["X-RateLimit-Reset"] = reset_time | ||
|
|
||
| return allowed | ||
|
|
||
|
|
||
| class ServiceTokenRateThrottle(SimpleRateThrottle): | ||
| scope = "service_token" | ||
| rate = "300/minute" | ||
|
|
||
| def get_cache_key(self, request, view): | ||
| # Retrieve the API key from the request header | ||
| api_key = request.headers.get("X-Api-Key") | ||
| if not api_key: | ||
| return None # Allow the request if there's no API key | ||
|
|
||
| # Use the API key as part of the cache key | ||
| return f"{self.scope}:{api_key}" | ||
|
|
||
| def allow_request(self, request, view): | ||
| allowed = super().allow_request(request, view) | ||
|
|
||
| if allowed: | ||
| now = self.timer() | ||
| # Calculate the remaining limit and reset time | ||
| history = self.cache.get(self.key, []) | ||
|
|
||
| # Remove old histories | ||
| while history and history[-1] <= now - self.duration: | ||
| history.pop() | ||
|
|
||
| # Calculate the requests | ||
| num_requests = len(history) | ||
|
|
||
| # Check available requests | ||
| available = self.num_requests - num_requests | ||
|
|
||
| # Unix timestamp for when the rate limit will reset | ||
| reset_time = int(now + self.duration) | ||
|
|
||
| # Add headers | ||
| request.META["X-RateLimit-Remaining"] = max(0, available) | ||
| request.META["X-RateLimit-Reset"] = reset_time | ||
|
|
||
| return allowed |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| # Copyright (c) 2023-present Plane Software, Inc. and contributors | ||
| # SPDX-License-Identifier: AGPL-3.0-only | ||
| # See the LICENSE file for details. | ||
|
|
||
| # Third party imports | ||
| from rest_framework.throttling import AnonRateThrottle, UserRateThrottle | ||
| from rest_framework import status | ||
| from rest_framework.response import Response | ||
|
|
||
| # Module imports | ||
| from plane.authentication.adapter.error import ( | ||
| AuthenticationException, | ||
| AUTHENTICATION_ERROR_CODES, | ||
| ) | ||
|
|
||
|
|
||
| class AuthenticationThrottle(AnonRateThrottle): | ||
| rate = "30/minute" | ||
| scope = "authentication" | ||
|
|
||
| def throttle_failure_view(self, request, *args, **kwargs): | ||
| try: | ||
| raise AuthenticationException( | ||
| error_code=AUTHENTICATION_ERROR_CODES["RATE_LIMIT_EXCEEDED"], | ||
| error_message="RATE_LIMIT_EXCEEDED", | ||
| ) | ||
| except AuthenticationException as e: | ||
| return Response(e.get_error_dict(), status=status.HTTP_429_TOO_MANY_REQUESTS) | ||
|
|
||
|
|
||
| class EmailVerificationThrottle(UserRateThrottle): | ||
| """ | ||
| Throttle for email verification code generation. | ||
| Limits to 3 requests per hour per user to prevent abuse. | ||
| """ | ||
|
|
||
| rate = "3/hour" | ||
| scope = "email_verification" | ||
|
|
||
| def throttle_failure_view(self, request, *args, **kwargs): | ||
| try: | ||
| raise AuthenticationException( | ||
| error_code=AUTHENTICATION_ERROR_CODES["RATE_LIMIT_EXCEEDED"], | ||
| error_message="RATE_LIMIT_EXCEEDED", | ||
| ) | ||
| except AuthenticationException as e: | ||
| return Response(e.get_error_dict(), status=status.HTTP_429_TOO_MANY_REQUESTS) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -326,7 +326,7 @@ class NavigationControlPreference(models.TextChoices): | |
| display_filters = models.JSONField(default=get_default_display_filters) | ||
| display_properties = models.JSONField(default=get_default_display_properties) | ||
| rich_filters = models.JSONField(default=dict) | ||
| navigation_project_limit = models.IntegerField(default=10) | ||
| navigation_project_limit = models.IntegerField(default=0, help_text="0 means unlimited") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Verify whether this PR includes both:
# 1) a migration altering WorkspaceUserProperties.navigation_project_limit, and
# 2) a data backfill for existing rows if all current users should become unlimited.
# Expected: at least one migration touching the field, and a RunPython/SQL update if existing data must change.
rg -n -C3 --glob 'apps/api/**/migrations/*.py' \
'navigation_project_limit|WorkspaceUserProperties|AlterField|RunPython|UPDATE'Repository: makeplane/plane Length of output: 50372 🏁 Script executed: #!/bin/bash
# More targeted search for navigation_project_limit in migrations
rg -n 'navigation_project_limit' apps/api/plane/db/migrations/
# Also check the current model definition
echo "=== Model definition ==="
fd 'workspace.py' apps/api/plane/db/models/ --exec cat -n {} \; | grep -A2 -B2 'navigation_project_limit'Repository: makeplane/plane Length of output: 878 🏁 Script executed: cat -n apps/api/plane/db/migrations/0110_workspaceuserproperties_navigation_control_preference_and_more.pyRepository: makeplane/plane Length of output: 926 🏁 Script executed: # Find migrations numbered 0111 and higher that touch WorkspaceUserProperties or navigation_project_limit
rg -n 'navigation_project_limit|workspaceuserproperties' apps/api/plane/db/migrations/ | grep -E '011[1-9]|012[0-9]|013[0-9]|014[0-9]'Repository: makeplane/plane Length of output: 623 🏁 Script executed: echo "=== Migration 0113 ==="
sed -n '45,65p' apps/api/plane/db/migrations/0113_webhook_version.py
echo ""
echo "=== Migration 0118 ==="
cat -n apps/api/plane/db/migrations/0118_remove_workspaceuserproperties_product_tour_and_more.py | head -100Repository: makeplane/plane Length of output: 3682 🏁 Script executed: # Search more thoroughly for the AlterField on navigation_project_limit
find apps/api/plane/db/migrations -name '*.py' -exec grep -l 'navigation_project_limit' {} \;
# Also list all migration files to see what numbers exist
ls -1 apps/api/plane/db/migrations/*.py | tail -20Repository: makeplane/plane Length of output: 1605 🏁 Script executed: # Check earlier migrations that mention navigation_project_limit
echo "=== Migration 0102 ==="
grep -n -A2 -B2 'navigation_project_limit' apps/api/plane/db/migrations/0102_page_sort_order_pagelog_entity_type_and_more.py
echo ""
echo "=== Migration 0103 ==="
grep -n -A2 -B2 'navigation_project_limit' apps/api/plane/db/migrations/0103_fileasset_asset_entity_type_idx_and_more.pyRepository: makeplane/plane Length of output: 105 🏁 Script executed: # Check if there are any uncommitted/untracked migration files
ls -la apps/api/plane/db/migrations/ | grep -E '012[1-9]|013'
# Also check git status for the migrations directory
git status apps/api/plane/db/migrations/ | head -30Repository: makeplane/plane Length of output: 592 🏁 Script executed: # Verify the exact model definition at line 329
sed -n '325,335p' apps/api/plane/db/models/workspace.pyRepository: makeplane/plane Length of output: 611 Missing migration for changing the field default—existing rows won't be updated. The model default changed from 🤖 Prompt for AI Agents |
||
| navigation_control_preference = models.CharField( | ||
| max_length=25, | ||
| choices=NavigationControlPreference.choices, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
What does pip's --trusted-host option do, and does it disable normal HTTPS certificate validation for that host?💡 Result:
The pip --trusted-host option marks a specified host (or host:port) as trusted, allowing pip to connect to it even if it lacks valid HTTPS or has certificate issues. Yes, it disables normal HTTPS certificate validation for that host by treating it as secure, bypassing SSL verification errors like CERTIFICATE_VERIFY_FAILED. This is used for insecure HTTP connections or self-signed/invalid certs on HTTPS, but reduces security by exposing to man-in-the-middle attacks. Use example: pip install --trusted-host example.com package It can be set via command line, PIP_TRUSTED_HOST env var, or pip.conf/ini.
Citations:
Remove the
--trusted-hostflags from pip install.These flags disable HTTPS certificate validation, which weakens build-time supply-chain security by allowing pip to bypass SSL verification for PyPI hosts. Since pypi.org, pypi.python.org, and files.pythonhosted.org have valid certificates, these flags are unnecessary. If certificate issues exist, install the required CA certificates in the image instead of trusting hosts blindly.
Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents