Skip to content
Open

cutting #8824

Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion apps/api/Dockerfile.api
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ RUN apk add --no-cache --virtual .build-deps \
"libc-dev" \
"linux-headers" \
&& \
pip install -r requirements.txt --compile --no-cache-dir \
pip install -r requirements.txt \
--trusted-host pypi.org \
--trusted-host pypi.python.org \
--trusted-host files.pythonhosted.org \
--compile --no-cache-dir --default-timeout=100 --retries 5 \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 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-host flags 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
-    pip install -r requirements.txt \
-    --trusted-host pypi.org \
-    --trusted-host pypi.python.org \
-    --trusted-host files.pythonhosted.org \
-    --compile --no-cache-dir --default-timeout=100 --retries 5 \
+    pip install -r requirements.txt \
+    --compile --no-cache-dir --default-timeout=100 --retries 5 \
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
pip install -r requirements.txt \
--trusted-host pypi.org \
--trusted-host pypi.python.org \
--trusted-host files.pythonhosted.org \
--compile --no-cache-dir --default-timeout=100 --retries 5 \
pip install -r requirements.txt \
--compile --no-cache-dir --default-timeout=100 --retries 5 \
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/api/Dockerfile.api` around lines 35 - 39, Remove the insecure
"--trusted-host" flags from the pip invocation (the "pip install -r
requirements.txt" command) and instead ensure the image has proper CA
certificates installed; update the Dockerfile to delete the three --trusted-host
entries and add installation of the system CA bundle (e.g., install the
distribution package that provides ca-certificates) before running pip so TLS
verification succeeds without bypassing certificate checks.


&& \
apk del .build-deps \
&& \
Expand Down
4 changes: 2 additions & 2 deletions apps/api/plane/api/rate_limit.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 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.py

Repository: 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 -i

Repository: makeplane/plane

Length of output: 845


🏁 Script executed:

# Look for environment variable documentation
fd -i readme -o -i config -o -i env | head -20

Repository: 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 -20

Repository: makeplane/plane

Length of output: 162


🏁 Script executed:

# Search all documentation for rate limit mentions
rg -i "rate.*limit|throttle" --type md . | head -40

Repository: 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 md

Repository: 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.py

Repository: 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 -20

Repository: 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 rate_limit.py are set to 10000/minute but the deployment documentation (deployments/aio/community/README.md) specifies API_KEY_RATE_LIMIT should default to 60/minute. At 10000/minute (~166 req/sec per token), these limits provide minimal protection against leaked or compromised credentials. Additionally, ServiceTokenRateThrottle hardcodes the rate with no environment override, unlike ApiKeyRateThrottle.

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
Verify each finding against the current code and only fix it if needed.

In `@apps/api/plane/api/rate_limit.py` around lines 12 - 14, The rate defaults are
insecure and inconsistent: change ApiKeyRateThrottle.rate from "10000/minute" to
use os.environ.get("API_KEY_RATE_LIMIT", "60/minute") and update
ServiceTokenRateThrottle to stop hardcoding its rate and instead read from an
environment variable (e.g., os.environ.get("SERVICE_TOKEN_RATE_LIMIT",
"60/minute")), keeping their scope names (ApiKeyRateThrottle,
ServiceTokenRateThrottle) unchanged so both throttle classes honor the
documented 60/minute default and allow env overrides.


def get_cache_key(self, request, view):
# Retrieve the API key from the request header
Expand Down Expand Up @@ -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
Expand Down
91 changes: 91 additions & 0 deletions apps/api/plane/api/rate_limit.py.backup
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
7 changes: 4 additions & 3 deletions apps/api/plane/authentication/rate_limit.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
# See the LICENSE file for details.

# Third party imports
import os
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
from rest_framework.throttling import AnonRateThrottle, UserRateThrottle
from rest_framework import status
from rest_framework.response import Response
Expand All @@ -15,7 +16,7 @@


class AuthenticationThrottle(AnonRateThrottle):
rate = "30/minute"
rate = "10000/minute"
scope = "authentication"

def throttle_failure_view(self, request, *args, **kwargs):
Expand All @@ -31,10 +32,10 @@ def throttle_failure_view(self, request, *args, **kwargs):
class EmailVerificationThrottle(UserRateThrottle):
"""
Throttle for email verification code generation.
Limits to 3 requests per hour per user to prevent abuse.
Limits to 10000 requests per hour per user to prevent abuse.
"""

rate = "3/hour"
rate = "10000/hour"
scope = "email_verification"

def throttle_failure_view(self, request, *args, **kwargs):
Expand Down
47 changes: 47 additions & 0 deletions apps/api/plane/authentication/rate_limit.py.backup
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)
2 changes: 1 addition & 1 deletion apps/api/plane/db/models/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 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.py

Repository: 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 -100

Repository: 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 -20

Repository: 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.py

Repository: 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 -30

Repository: 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.py

Repository: makeplane/plane

Length of output: 611


Missing migration for changing the field default—existing rows won't be updated.

The model default changed from 10 to 0, but there's no migration file to apply this change to the database schema. Existing WorkspaceUserProperties rows will retain their current navigation_project_limit values (the old default or whatever was previously set). Create an AlterField migration to change the database default. If you also need to backfill existing rows to the new default, add a RunPython operation in the same migration.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/api/plane/db/models/workspace.py` at line 329, The model field
navigation_project_limit on WorkspaceUserProperties was changed from default=10
to default=0 but no migration was added; create a Django migration that uses
migrations.AlterField to set navigation_project_limit to
models.IntegerField(default=0, help_text="0 means unlimited") and, if you need
existing rows updated, include a migrations.RunPython forward function to
backfill any rows where navigation_project_limit is the old value (set them to
0) and a no-op reverse. Name the migration following your app’s sequence and
import migrations.AlterField and migrations.RunPython so the DB default and
existing data are updated.

navigation_control_preference = models.CharField(
max_length=25,
choices=NavigationControlPreference.choices,
Expand Down
Loading