Remove silent fallbacks found in the 2026-10-03 audit
- /api/ai/ask: the stub answer is reachable only under DEBUG or RAG_FORCE_MOCK. Without RAG keys on the server it used to return 200 with "this is a stub, set OPENAI_API_KEY" and spend the student's quota; now 503. - /api/home: the `grade` alias is gone and sending it is a 400 instead of a silent fallback to the user's own grade. No client sent it. - activate_subscription logs ERROR when the paid order's plan no longer exists (it silently became 30 days). - resolve_grade_title logs instead of `except: pass`; curriculum_io no longer reads an `intro_text` key no file contains. - CLAUDE.md: "No silent fallbacks" rules for future agents. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
41
CLAUDE.md
41
CLAUDE.md
@@ -22,6 +22,47 @@ at checkout, `select_for_update` guards that SQLite silently ignores).
|
||||
race-free / the right architecture"; write "guards X; not tested under Y" instead.
|
||||
Record uncertainty plainly so the next reader re-checks rather than trusts.
|
||||
|
||||
## No silent fallbacks, no speculative backward compatibility
|
||||
|
||||
Agents keep adding "helpful" fallbacks (`x or default`, `except: pass`, accepting an old
|
||||
field name "just in case") and they cause the hardest bugs here, because the
|
||||
failure never shows up anywhere. An audit on 2026-10-03 found and reproduced these:
|
||||
|
||||
- With RAG keys missing from `.env` and `DEBUG=0`, `/api/ai/ask` answered **200** with
|
||||
«این یک پاسخ نمونه (stub)… OPENAI_API_KEY را تنظیم کنید» and still used up the
|
||||
student's daily quota. It now returns 503 (`StubNeverReachesProductionTests`).
|
||||
- A paid order whose plan had been deleted quietly became 30 days (a yearly buyer got
|
||||
a month). It still activates, but now logs ERROR.
|
||||
- `?grade=` on `/api/home` was an alias "for clients already out there". No such client
|
||||
existed. Removed, and sending it is a 400, not a silent fallback to the user's own grade.
|
||||
- `resolve_grade_title` had `except Exception: pass` (now logged); `curriculum_io` read
|
||||
`intro_text` that no file ever contains (removed).
|
||||
|
||||
Rules:
|
||||
|
||||
1. **Fail loudly over guessing.** A missing setting, row or field in production should
|
||||
be an error the user and `/api/health` can see, not a default that looks like
|
||||
success. A default is fine only when it is a real product choice. When you
|
||||
add one, say in the comment why that value is right, not "just in case".
|
||||
2. **Demo/stub paths only behind `DEBUG` or `RAG_FORCE_MOCK`.** Never reachable on the server.
|
||||
3. **No backward compatibility without a named client.** Before keeping an old field, param
|
||||
or code path, check who sends it (`~/Desktop/MathApp_frontend/src/api/`, the server
|
||||
DB). The only API client is that frontend; there is no Android app yet. If nobody
|
||||
sends it, delete it. If you must keep one, write who uses it and when it can go.
|
||||
4. **Never `except: pass`.** At least `logger.exception`. Swallowing is acceptable only
|
||||
where the comment names the bigger thing it protects (e.g. `affiliate/services.py`:
|
||||
commission failure must not undo the user's payment) and it still logs.
|
||||
5. **Removing a fallback means making the old input fail visibly** (400/503), not
|
||||
silently routing it somewhere else.
|
||||
|
||||
Fallbacks that remain on purpose (re-check before relying on them): settings boot with
|
||||
default hosts/CORS/cache/secret key (`/api/health` warns); `LocMemCache` without
|
||||
`CACHE_URL`; `_annotation` defaults for answer-cache entries older than a field; Bazaar's
|
||||
OAuth path beside the Pishkhan token (server still uses OAuth); `birthYear` alone in
|
||||
`PATCH /api/user/profile` (frontend type still allows it); `totalQuestions` accepted but
|
||||
ignored (frontend sends it); orders with no `plan_id` from before 2026-09-25 (the
|
||||
server DB was not checked for any).
|
||||
|
||||
## Before any real user: read `docs/security-before-production.md`
|
||||
|
||||
The server is deliberately left in demo mode for the internship presentation: the token
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
from unittest.mock import patch
|
||||
|
||||
from django.core.cache import cache
|
||||
from django.test import TestCase
|
||||
from django.test import TestCase, override_settings
|
||||
from rest_framework.test import APIClient
|
||||
from rest_framework.throttling import ScopedRateThrottle
|
||||
|
||||
@@ -514,3 +514,18 @@ class AskAIViewGuestThrottleBypassTests(TestCase):
|
||||
response_b = self._ask(issue_access_token(user_b.id), 'user-b-key-1')
|
||||
|
||||
self.assertEqual(response_b.status_code, 200)
|
||||
|
||||
|
||||
class StubNeverReachesProductionTests(TestCase):
|
||||
"""بی‌کلیدِ RAG و بیرون از DEBUG، دانش‌آموز ۵۰۳ می‌گیرد، نه متنِ «پاسخ نمونه (stub)»."""
|
||||
|
||||
@override_settings(DEBUG=False, OPENAI_API_KEY='', QDRANT_API_KEY='', RAG_FORCE_MOCK=False)
|
||||
def test_missing_keys_are_a_503_and_cost_nothing(self):
|
||||
cache.clear()
|
||||
user = User.objects.create(device_id='device-no-keys', is_guest=True, name='')
|
||||
response = APIClient().post(
|
||||
'/api/ai/ask', {'userQuestion': '۲+۲ چند است؟'}, format='json',
|
||||
HTTP_AUTHORIZATION=f'Bearer {issue_access_token(user.id)}',
|
||||
)
|
||||
self.assertEqual(response.status_code, 503)
|
||||
self.assertFalse(Message.objects.filter(conversation__user_id=user.id).exists())
|
||||
|
||||
@@ -230,10 +230,14 @@ def generate_tutor_answer(
|
||||
تولید می‌کنه. مالکیتِ تاریخچه‌ی مکالمه همیشه دستِ دیتابیسِ خودمونه، نه
|
||||
provider.
|
||||
|
||||
اگه RAG پیکربندی نشده باشه (نبودِ OPENAI_API_KEY/QDRANT_API_KEY یا
|
||||
RAG_FORCE_MOCK)، همون پاسخِ stubِ قطعیِ قبلی برمی‌گرده تا اجرای بدونِ
|
||||
کلید و تست‌ها دست‌نخورده کار کنن.
|
||||
اگه RAG پیکربندی نشده باشه، پاسخِ stub فقط در DEBUG یا با RAG_FORCE_MOCK
|
||||
(تست‌ها) برمی‌گرده. بیرون از آن خطا می‌دهد و AskAIView آن را ۵۰۳ می‌کند.
|
||||
تا ۲۰۲۶-۱۰-۰۳ stub در پروداکشن هم برمی‌گشت: اگر کلیدی از .env سرور می‌افتاد،
|
||||
دانش‌آموز با ۲۰۰ متنِ «این یک پاسخ نمونه (stub)… OPENAI_API_KEY را تنظیم
|
||||
کنید» می‌گرفت و سهمیه‌ی روزانه‌اش هم کم می‌شد (با تستِ دورریختنی بازتولید شد).
|
||||
"""
|
||||
from django.conf import settings
|
||||
|
||||
from rag.services.config import rag_is_configured
|
||||
|
||||
if rag_is_configured():
|
||||
@@ -247,6 +251,9 @@ def generate_tutor_answer(
|
||||
answer, follow_ups, _meta = generate_answer(user_question, context_chunks)
|
||||
return answer, follow_ups
|
||||
|
||||
if not (settings.DEBUG or settings.RAG_FORCE_MOCK):
|
||||
raise RuntimeError('RAG پیکربندی نشده (OPENAI_API_KEY/QDRANT_API_KEY)؛ stub فقط در DEBUG.')
|
||||
|
||||
context_note = f' (با context: {context})' if context else ''
|
||||
answer = (
|
||||
f'این یک پاسخ نمونه (stub) برای سوال «{user_question}»{context_note} است. '
|
||||
|
||||
@@ -92,7 +92,7 @@ def parse(raw: bytes) -> list[dict]:
|
||||
'lessons': [
|
||||
{
|
||||
'title': _text(lesson.get('title'), 'title', f'{nwhere} › نوع {li + 1}'),
|
||||
'intro_text': (lesson.get('introText') or lesson.get('intro_text') or ''),
|
||||
'intro_text': (lesson.get('introText') or ''),
|
||||
'example': str(lesson.get('example') or ''),
|
||||
'steps': [str(x) for x in (lesson.get('steps') or []) if str(x).strip()][:12],
|
||||
}
|
||||
|
||||
@@ -587,7 +587,7 @@ class HomeViewScalabilityTests(TestCase):
|
||||
|
||||
def _home(self, grade_id):
|
||||
return self.client.get(
|
||||
f'/api/home?grade={grade_id}', HTTP_AUTHORIZATION=f'Bearer {self.token}')
|
||||
f'/api/home?gradeId={grade_id}', HTTP_AUTHORIZATION=f'Bearer {self.token}')
|
||||
|
||||
def test_query_count_is_flat_regardless_of_node_count(self):
|
||||
g_small = self._grade_with_nodes(2)
|
||||
@@ -726,7 +726,7 @@ class ReadThrottleTests(TestCase):
|
||||
codes = set()
|
||||
for _ in range(122):
|
||||
codes.add(self.client.get(
|
||||
f'/api/home?grade={self.grade.id}',
|
||||
f'/api/home?gradeId={self.grade.id}',
|
||||
HTTP_AUTHORIZATION=f'Bearer {self.token}').status_code)
|
||||
self.assertIn(429, codes)
|
||||
|
||||
@@ -774,7 +774,7 @@ class HomeGradeFallbackTests(TestCase):
|
||||
self.user.grade_id = self.grade.id
|
||||
self.user.save(update_fields=['grade_id'])
|
||||
|
||||
response = self._home(f'?grade={other.id}')
|
||||
response = self._home(f'?gradeId={other.id}')
|
||||
|
||||
self.assertEqual(response.data['units'][0]['title'], 'فصل مثلثات')
|
||||
|
||||
@@ -785,7 +785,7 @@ class HomeGradeFallbackTests(TestCase):
|
||||
self.assertEqual(response.data['units'], [])
|
||||
|
||||
def test_unknown_explicit_grade_is_still_404(self):
|
||||
self.assertEqual(self._home('?grade=999999').status_code, 404)
|
||||
self.assertEqual(self._home('?gradeId=999999').status_code, 404)
|
||||
|
||||
def test_a_deleted_saved_grade_degrades_to_an_empty_map(self):
|
||||
# پایه‌ای که کاربر ذخیره کرده حذف شده — این خطای کاربر نیست.
|
||||
|
||||
@@ -142,8 +142,8 @@ class HomeGradeParamTests(TestCase):
|
||||
def test_grade_id_selects_that_grade(self):
|
||||
self.assertEqual(self._titles(f'?gradeId={self.b.id}'), ['فصلِ نهم'])
|
||||
|
||||
def test_the_old_grade_name_still_works(self):
|
||||
self.assertEqual(self._titles(f'?grade={self.b.id}'), ['فصلِ نهم'])
|
||||
def test_the_removed_grade_name_is_a_400_not_a_silent_fallback(self):
|
||||
self.assertEqual(self.client.get(f'/api/home?grade={self.b.id}', **self.auth).status_code, 400)
|
||||
|
||||
def test_without_a_parameter_the_users_own_grade_is_used(self):
|
||||
self.assertEqual(self._titles(), ['فصلِ هفتم'])
|
||||
|
||||
@@ -253,7 +253,7 @@ def _node_status(lesson_ids, completed_ids, is_first_unlocked_candidate, quiz_co
|
||||
|
||||
|
||||
class HomeView(APIView):
|
||||
"""GET /api/home?grade — نقشه‌ی مهارت‌ها + آمار (ادغام‌شده)"""
|
||||
"""GET /api/home?gradeId — نقشه‌ی مهارت‌ها + آمار (ادغام‌شده)"""
|
||||
|
||||
# userId همیشه از request.user.id (توکن معتبر) گرفته می‌شود، نه ورودی کلاینت (IDOR).
|
||||
permission_classes = [IsAuthenticated]
|
||||
@@ -265,8 +265,6 @@ class HomeView(APIView):
|
||||
OpenApiParameter('gradeId', OpenApiTypes.INT, required=False,
|
||||
description='پایه‌ای که نقشه‌اش خواسته می‌شود؛ '
|
||||
'بدونِ آن، پایه‌ی ذخیره‌شده‌ی خودِ کاربر.'),
|
||||
OpenApiParameter('grade', OpenApiTypes.INT, required=False,
|
||||
deprecated=True, description='نامِ قدیمیِ gradeId.'),
|
||||
],
|
||||
responses=HomeResponseSerializer,
|
||||
tags=['Curriculum (content)'],
|
||||
@@ -278,13 +276,12 @@ class HomeView(APIView):
|
||||
# خودِ کاربر نادیده می‌ماند؛ نتیجه این بود که اپ در «آشنایی» پایه را ثبت
|
||||
# می‌کرد و بعد صفحه‌ی خانه خالی برمی‌گشت. حالا پارامتر اولویت دارد (برای
|
||||
# مرورِ پایه‌ی دیگر) و در نبودش پایه‌ی خودِ کاربر ملاک است.
|
||||
# هر دو نام پذیرفته می‌شوند. نامِ درست `gradeId` است — با بقیه‌ی API
|
||||
# یکی است (profile و /user/grade هم gradeId می‌گویند) — ولی نسخه‌ی
|
||||
# اولِ این اندپوینت `grade` بود و کلاینت‌هایی با همان بیرون رفته‌اند.
|
||||
# پذیرفتنِ هر دو ارزان است؛ نپذیرفتن یعنی انتخابِ پایه بی‌صدا نادیده
|
||||
# گرفته می‌شود و کاربر نقشه‌ی پایه‌ی دیگری را می‌بیند.
|
||||
grade_param = (request.query_params.get('gradeId')
|
||||
or request.query_params.get('grade'))
|
||||
# فقط `gradeId` (هم‌نام با بقیه‌ی API). نامِ قدیمیِ `grade` در ۲۰۲۶-۱۰-۰۳
|
||||
# حذف شد: تنها کلاینت (MathApp_frontend) آن را نمی‌فرستد. فرستادنش ۴۰۰ است،
|
||||
# نه نادیده گرفتنِ بی‌صدا (که کاربر را به نقشه‌ی پایه‌ی خودش می‌برد).
|
||||
if 'grade' in request.query_params:
|
||||
raise ValidationError({'grade': 'این پارامتر حذف شده؛ gradeId بفرستید.'})
|
||||
grade_param = request.query_params.get('gradeId')
|
||||
if grade_param:
|
||||
try:
|
||||
grade_id = int(grade_param)
|
||||
|
||||
@@ -296,7 +296,7 @@ export function createClient(options: ClientOptions = {}) {
|
||||
/* ── صفحه‌ی خانه و نقشه‌ی مهارت ────────────────────────────────────── */
|
||||
/** بدونِ gradeId، پایه‌ی ذخیره‌شده‌ی خودِ کاربر ملاک است. */
|
||||
home: (gradeId?: number) =>
|
||||
raw<HomeResponse>('GET', gradeId ? `/home?grade=${gradeId}` : '/home'),
|
||||
raw<HomeResponse>('GET', gradeId ? `/home?gradeId=${gradeId}` : '/home'),
|
||||
summary: () => raw<UserSummary>('GET', '/user/summary'),
|
||||
hearts: () => raw<Hearts>('GET', '/user/hearts'),
|
||||
streakCalendar: () =>
|
||||
|
||||
@@ -7,8 +7,12 @@
|
||||
(کاربر قبلاً با اسمِ نادرست به مشکل خورده بود). کلیدِ فیلترِ درس/فصل خودکار
|
||||
از روی subject انتخاب می‌شه — نگاه کنید به rag/services/known_collections.py.
|
||||
"""
|
||||
import logging
|
||||
|
||||
from django.conf import settings
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def rag_is_configured() -> bool:
|
||||
return bool(
|
||||
@@ -39,8 +43,9 @@ def resolve_grade_title(grade_id, fallback: str = '') -> str:
|
||||
grade = Grade.objects.filter(pk=grade_id).only('title').first()
|
||||
if grade is not None:
|
||||
return normalize_grade_title(grade.title)
|
||||
except Exception:
|
||||
pass
|
||||
except Exception: # noqa: BLE001
|
||||
# پیش‌تر `pass` بود: خطای دیتابیس retrieval را بی‌صدا خالی می‌کرد.
|
||||
logger.exception('resolve_grade_title: خواندنِ پایه‌ی %s شکست خورد', grade_id)
|
||||
return fallback
|
||||
|
||||
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import logging
|
||||
|
||||
from django.db import models, transaction
|
||||
from django.utils import timezone
|
||||
|
||||
@@ -5,6 +7,8 @@ from statuslog.signals import status_changed
|
||||
|
||||
from .models import HeartsPass, QuestionCredit, Subscription
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def has_unlimited_hearts(user_id: int) -> bool:
|
||||
"""
|
||||
@@ -126,6 +130,11 @@ def activate_subscription(subscription_id: int, plan_id: int | None = None):
|
||||
return None
|
||||
old_status = subscription.status
|
||||
plan = Plan.objects.filter(pk=plan_id).first() if plan_id else None
|
||||
if plan_id and plan is None:
|
||||
# پلنِ سفارشِ پرداخت‌شده پاک شده؛ جایگزین (پلنِ Subscription یا ۳۰ روز)
|
||||
# ممکن است کمتر از آن‌چه خریده شده باشد. کاربر بی‌اشتراک نمی‌ماند، ولی
|
||||
# باید دیده شود. پلن را پاک نکنید؛ on_sale=False کنید.
|
||||
logger.error('activate_subscription: پلنِ %s برای اشتراکِ %s پیدا نشد', plan_id, subscription_id)
|
||||
plan = plan or subscription.plan
|
||||
duration_days = plan.duration_days if plan else 30
|
||||
now = timezone.now()
|
||||
|
||||
Reference in New Issue
Block a user