From 7307bfcadb22d318047872de1573affc8b50ffc3 Mon Sep 17 00:00:00 2001 From: Zareian Date: Sat, 3 Oct 2026 10:38:42 +0330 Subject: [PATCH] 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 --- CLAUDE.md | 41 ++++++++++++++++++++++++++++++ ai_tutor/tests.py | 17 ++++++++++++- ai_tutor/views.py | 13 +++++++--- content/curriculum_io.py | 2 +- content/tests.py | 8 +++--- content/tests_frontend_contract.py | 4 +-- content/views.py | 17 +++++-------- docs/frontend/api.ts | 2 +- rag/services/config.py | 9 +++++-- subscription/services.py | 9 +++++++ 10 files changed, 98 insertions(+), 24 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 95c7c38..cbc7f17 100644 --- a/CLAUDE.md +++ b/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 diff --git a/ai_tutor/tests.py b/ai_tutor/tests.py index 01f328c..fc8f5cf 100644 --- a/ai_tutor/tests.py +++ b/ai_tutor/tests.py @@ -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()) diff --git a/ai_tutor/views.py b/ai_tutor/views.py index 7a9400c..66c6a1d 100644 --- a/ai_tutor/views.py +++ b/ai_tutor/views.py @@ -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} است. ' diff --git a/content/curriculum_io.py b/content/curriculum_io.py index 5bbbfa2..8569f6f 100644 --- a/content/curriculum_io.py +++ b/content/curriculum_io.py @@ -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], } diff --git a/content/tests.py b/content/tests.py index f7c8460..02c2bd7 100644 --- a/content/tests.py +++ b/content/tests.py @@ -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): # پایه‌ای که کاربر ذخیره کرده حذف شده — این خطای کاربر نیست. diff --git a/content/tests_frontend_contract.py b/content/tests_frontend_contract.py index 1999e72..7f04ae6 100644 --- a/content/tests_frontend_contract.py +++ b/content/tests_frontend_contract.py @@ -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(), ['فصلِ هفتم']) diff --git a/content/views.py b/content/views.py index a16e117..1f2348b 100644 --- a/content/views.py +++ b/content/views.py @@ -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) diff --git a/docs/frontend/api.ts b/docs/frontend/api.ts index f51b4a1..9dea420 100644 --- a/docs/frontend/api.ts +++ b/docs/frontend/api.ts @@ -296,7 +296,7 @@ export function createClient(options: ClientOptions = {}) { /* ── صفحه‌ی خانه و نقشه‌ی مهارت ────────────────────────────────────── */ /** بدونِ gradeId، پایه‌ی ذخیره‌شده‌ی خودِ کاربر ملاک است. */ home: (gradeId?: number) => - raw('GET', gradeId ? `/home?grade=${gradeId}` : '/home'), + raw('GET', gradeId ? `/home?gradeId=${gradeId}` : '/home'), summary: () => raw('GET', '/user/summary'), hearts: () => raw('GET', '/user/hearts'), streakCalendar: () => diff --git a/rag/services/config.py b/rag/services/config.py index 841a194..bb66bb3 100644 --- a/rag/services/config.py +++ b/rag/services/config.py @@ -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 diff --git a/subscription/services.py b/subscription/services.py index eb3f07b..8adb425 100644 --- a/subscription/services.py +++ b/subscription/services.py @@ -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()