Skip to content

feat: add opt-out flags for token negative cache and rate limiting #121

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.

Sign up for GitHub

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

Merged
TimilsinaBimal merged 1 commit into from
Mar 23, 2026
Merged

feat: add opt-out flags for token negative cache and rate limiting #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions app/core/app.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ async def lifespan(app: FastAPI):

@app.middleware("http")
async def block_missing_token_middleware(request: Request, call_next):
if not settings.ENABLE_TOKEN_RATE_LIMIT:
return await call_next(request)
# Extract first path segment which is commonly the token in addon routes
path = request.url.path.lstrip("/")
seg = path.split("/", 1)[0] if path else ""
Expand Down
3 changes: 3 additions & 0 deletions app/core/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ class Settings(BaseSettings):
RECOMMENDATION_SOURCE_ITEMS_LIMIT: int = 10
LIBRARY_ITEMS_LIMIT: int = 20

ENABLE_TOKEN_NEGATIVE_CACHE: bool = True
ENABLE_TOKEN_RATE_LIMIT: bool = True

CATALOG_CACHE_TTL: int = 43200 # 12 hours
CATALOG_STALE_TTL: int = 604800 # 7 days (soft expiration fallback)

Expand Down
22 changes: 12 additions & 10 deletions app/services/token_store.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,23 +230,25 @@ async def _migrate_poster_rating_format_raw(self, token: str, redis_key: str, da
@alru_cache(maxsize=2000, ttl=43200)
async def get_user_data(self, token: str) -> dict[str, Any] | None:
# Short-circuit for tokens known to be missing
try:
if token in self._missing_tokens:
logger.debug(f"[REDIS] Negative cache hit for missing token {token}")
return None
except Exception as e:
logger.debug(f"Failed to check negative cache for {token}: {e}")
if settings.ENABLE_TOKEN_NEGATIVE_CACHE:
try:
Comment on lines 230 to +234
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

With @alru_cache on get_user_data, results are cached per token at this layer. That means disabling ENABLE_TOKEN_NEGATIVE_CACHE may still leave a replica returning a cached “missing” (None) response for some period, so it may not achieve the goal of always re-checking Redis for previously-missing tokens in multi-replica deployments. Consider conditionally bypassing the alru_cache when negative caching is disabled, or ensuring that missing-token results are not cached / have a very short TTL.

Copilot uses AI. Check for mistakes.
if token in self._missing_tokens:
logger.debug(f"[REDIS] Negative cache hit for missing token {token}")
return None
Comment on lines +235 to +237
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

The negative-cache hit log line prints the full token value, which can leak credentials into logs. Please redact the token (e.g., via redact_token(token)) or avoid including it in log messages.

Copilot uses AI. Check for mistakes.
except Exception as e:
logger.debug(f"Failed to check negative cache for {token}: {e}")
Comment on lines +233 to +239
Copy link

@gemini-code-assist gemini-code-assist bot Mar 23, 2026

Choose a reason for hiding this comment

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

medium

To improve readability and reduce nesting, you can combine the feature flag check with the cache lookup. Python's and operator uses short-circuit evaluation, so token in self._missing_tokens will only be evaluated if settings.ENABLE_TOKEN_NEGATIVE_CACHE is True, making this change safe.

        try:
            if settings.ENABLE_TOKEN_NEGATIVE_CACHE and token in self._missing_tokens:
                logger.debug(f"[REDIS] Negative cache hit for missing token {token}")
                return None
        except Exception as e:
            logger.debug(f"Failed to check negative cache for {token}: {e}")


logger.debug(f"[REDIS] Cache miss. Fetching data from redis for {token}")
key = self._format_key(token)
Comment on lines +239 to 242
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

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

These debug logs include the raw token value. Tokens appear to be treated as sensitive elsewhere (e.g., redact_token(...) in other log lines), so logging the full token can leak credentials into log aggregation. Consider using redact_token(token) (or removing the token entirely) in these messages.

Copilot uses AI. Check for mistakes.
data_raw = await redis_service.get(key)

if not data_raw:
# remember negative result briefly
try:
self._missing_tokens[token] = True
except Exception as e:
logger.debug(f"Failed to set negative cache for missing token {token}: {e}")
if settings.ENABLE_TOKEN_NEGATIVE_CACHE:
try:
self._missing_tokens[token] = True
except Exception as e:
logger.debug(f"Failed to set negative cache for missing token {token}: {e}")
return None

try:
Expand Down
Loading