From 2ef43e7177e779a60360de713d10c66510ee33d9 Mon Sep 17 00:00:00 2001
From: lizerk <lizerk-dev@gmail.com>
Date: Fri, 20 Mar 2026 09:06:55 +0300
Subject: [PATCH] Feature: Add code quality audit tools and safe helpers

- Created CODE_QUALITY_CHECKLIST.md with comprehensive guide
- Added audit_code_patterns.sh for automated code scanning
- Created common/request_utils.py with safe_int(), safe_bool(), etc.
- Fixed unsafe request.data access in notifications/api/views.py
- Updated ldap_password_mixin.py docstring with safe example

Audit found and documented:
- 10 unsafe request.data['key'] accesses
- 8+ unsafe int() conversions without try/except
- Multiple .strip() calls without None checks
---
 backend/common/ldap_password_mixin.py         |   5 +-
 backend/common/request_utils.py               | 115 +++++++++
 backend/docs/guides/CODE_QUALITY_CHECKLIST.md | 235 ++++++++++++++++++
 backend/notifications/api/views.py            |  16 +-
 backend/scripts/audit_code_patterns.sh        |  27 ++
 5 files changed, 389 insertions(+), 9 deletions(-)
 create mode 100644 backend/common/request_utils.py
 create mode 100644 backend/docs/guides/CODE_QUALITY_CHECKLIST.md
 create mode 100755 backend/scripts/audit_code_patterns.sh

diff --git a/backend/common/ldap_password_mixin.py b/backend/common/ldap_password_mixin.py
index db8e6c2d..dc8e3af6 100644
--- a/backend/common/ldap_password_mixin.py
+++ b/backend/common/ldap_password_mixin.py
@@ -25,7 +25,10 @@ class LdapPasswordMixin:
         class ChangePasswordAPIView(LdapPasswordMixin, APIView):
             def post(self, request):
                 user = request.user
-                new_password = request.data['new_password']
+                # Безопасный доступ:
+                new_password = request.data.get('new_password')
+                if not new_password:
+                    return Response({'error': 'Password required'}, 400)
                 
                 # Меняем пароль в БД
                 user.set_password(new_password)
diff --git a/backend/common/request_utils.py b/backend/common/request_utils.py
new file mode 100644
index 00000000..2d6e86a6
--- /dev/null
+++ b/backend/common/request_utils.py
@@ -0,0 +1,115 @@
+"""Утилиты для безопасной обработки данных запросов."""
+from typing import Any, Optional, Union
+
+
+def safe_int(value: Any, default: int = 0) -> int:
+    """Безопасно конвертирует значение в int.
+    
+    Args:
+        value: Значение для конверсии (может быть str, int, None, ...)
+        default: Значение по умолчанию при ошибке конверсии
+    
+    Returns:
+        int: Сконвертированное значение или default
+    
+    Examples:
+        >>> safe_int('123')
+        123
+        >>> safe_int('abc', default=1)
+        1
+        >>> safe_int(None, default=10)
+        10
+        >>> safe_int('', default=5)
+        5
+    """
+    if value is None or value == '':
+        return default
+    
+    try:
+        return int(value)
+    except (ValueError, TypeError):
+        return default
+
+
+def safe_bool(value: Any, default: bool = False) -> bool:
+    """Безопасно конвертирует значение в bool.
+    
+    Поддерживает строки: 'true', 'false', '1', '0', 'yes', 'no'
+    
+    Args:
+        value: Значение для конверсии
+        default: Значение по умолчанию при ошибке
+    
+    Returns:
+        bool: Сконвертированное значение или default
+    """
+    if value is None:
+        return default
+    
+    if isinstance(value, bool):
+        return value
+    
+    if isinstance(value, (int, float)):
+        return bool(value)
+    
+    if isinstance(value, str):
+        value_lower = value.lower().strip()
+        if value_lower in ('true', '1', 'yes', 'on'):
+            return True
+        if value_lower in ('false', '0', 'no', 'off', ''):
+            return False
+    
+    return default
+
+
+def safe_float(value: Any, default: float = 0.0) -> float:
+    """Безопасно конвертирует значение в float.
+    
+    Args:
+        value: Значение для конверсии
+        default: Значение по умолчанию при ошибке
+    
+    Returns:
+        float: Сконвертированное значение или default
+    """
+    if value is None or value == '':
+        return default
+    
+    try:
+        return float(value)
+    except (ValueError, TypeError):
+        return default
+
+
+def safe_get_data(
+    data: Union[dict, 'QueryDict'],
+    key: str,
+    default: Any = None,
+    required: bool = False,
+) -> Any:
+    """Безопасно извлекает значение из request.data.
+    
+    Args:
+        data: request.data (может быть dict или QueryDict)
+        key: Ключ для извлечения
+        default: Значение по умолчанию
+        required: Если True, вызовет KeyError при отсутствии ключа
+    
+    Returns:
+        Значение или default
+    
+    Raises:
+        KeyError: Если required=True и ключа нет
+    
+    Examples:
+        >>> safe_get_data({'name': 'John'}, 'name')
+        'John'
+        >>> safe_get_data({}, 'age', default=18)
+        18
+        >>> safe_get_data({}, 'email', required=True)
+        KeyError: 'email'
+    """
+    if required and key not in data:
+        raise KeyError(f"Required field '{key}' is missing")
+    
+    return data.get(key, default)
diff --git a/backend/docs/guides/CODE_QUALITY_CHECKLIST.md b/backend/docs/guides/CODE_QUALITY_CHECKLIST.md
new file mode 100644
index 00000000..c4ce9e01
--- /dev/null
+++ b/backend/docs/guides/CODE_QUALITY_CHECKLIST.md
@@ -0,0 +1,235 @@
+# Контрольный список качества кода (Code Quality Checklist)
+
+## Обнаруженные проблемы и решения
+
+### 1. 🔴 Критично: Небезопасный доступ к `request.data`
+
+**Проблема:**
+```python
+value = request.data['key']  # ❌ KeyError если ключа нет
+```
+
+**Решение:**
+```python
+value = request.data.get('key')  # ✅ Вернёт None
+value = request.data.get('key', default)  # ✅ С дефолтом
+```
+
+**Найдено в:**
+- `notifications/api/views.py` (8 мест)
+- `common/ldap_password_mixin.py` (1 место)
+
+---
+
+### 2. 🟡 Важно: QueryDict vs dict несовместимость
+
+**Проблема:**
+```python
+# DRF request.data может быть dict (JSON) или QueryDict (form data)
+data = request.data.dict()  # ❌ dict не имеет метода .dict()
+```
+
+**Решение:**
+```python
+data = request.data
+result = data.dict() if hasattr(data, 'dict') else dict(data)  # ✅
+```
+
+**Статус:** ✅ Исправлено в `api/v1/employees/views/employees.py`
+
+---
+
+### 3. 🟡 Важно: Небезопасный `int()` без обработки
+
+**Проблема:**
+```python
+page = int(request.GET.get('page', 1))  # ❌ ValueError если 'abc'
+```
+
+**Решение:**
+```python
+# Вариант 1: try/except
+try:
+    page = int(request.GET.get('page', 1))
+except (ValueError, TypeError):
+    page = 1
+
+# Вариант 2: Helper функция
+def safe_int(value, default=0):
+    try:
+        return int(value)
+    except (ValueError, TypeError):
+        return default
+
+page = safe_int(request.GET.get('page'), 1)
+```
+
+**Найдено в:**
+- `communications/api/viewsets.py` (4 места)
+- `notifications/api/views.py` (2 места)
+- `api/v1/search/views.py` (1 место)
+- `api/v1/procurement/views.py` (1 место)
+
+---
+
+### 4. 🟢 Средне: `.strip()` без проверки на None
+
+**Проблема:**
+```python
+text = some_value.strip()  # ❌ AttributeError если None
+```
+
+**Решение:**
+```python
+text = (some_value or '').strip()  # ✅
+text = str(some_value).strip() if some_value else ''  # ✅
+```
+
+---
+
+## Автоматизация проверки
+
+### Скрипт для аудита
+
+Запуск:
+```bash
+./scripts/audit_code_patterns.sh
+```
+
+### Pre-commit hooks
+
+Добавить в `.pre-commit-config.yaml`:
+```yaml
+repos:
+  - repo: local
+    hooks:
+      - id: check-unsafe-data-access
+        name: Check unsafe request.data access
+        entry: bash -c 'grep -rn "request\.data\[" --include="*.py" . | grep -v "# noqa" && exit 1 || exit 0'
+        language: system
+        pass_filenames: false
+```
+
+### Линтеры
+
+**Flake8** с плагинами:
+```bash
+pip install flake8 flake8-bugbear flake8-comprehensions
+flake8 --max-line-length=100 --extend-ignore=E203,W503
+```
+
+**Pylint**:
+```bash
+pip install pylint
+pylint --disable=C0111,C0103 employees/
+```
+
+**MyPy** (статическая типизация):
+```bash
+pip install mypy django-stubs djangorestframework-stubs
+mypy --config-file=mypy.ini employees/
+```
+
+---
+
+## Тесты для обнаружения проблем
+
+### Unit-тесты для API
+
+```python
+from rest_framework.test import APITestCase
+
+class EmployeeAPITestCase(APITestCase):
+    def test_patch_with_json(self):
+        """Тест PATCH с JSON payload"""
+        response = self.client.patch(
+            '/api/v1/employees/1/',
+            {'first_name': 'John'},
+            format='json'
+        )
+        self.assertEqual(response.status_code, 200)
+    
+    def test_patch_with_form_data(self):
+        """Тест PATCH с form data"""
+        response = self.client.patch(
+            '/api/v1/employees/1/',
+            {'first_name': 'John'},
+            format='multipart'
+        )
+        self.assertEqual(response.status_code, 200)
+    
+    def test_missing_required_field(self):
+        """Тест обработки отсутствующего поля"""
+        response = self.client.patch(
+            '/api/v1/employees/1/',
+            {},
+            format='json'
+        )
+        self.assertEqual(response.status_code, 400)
+    
+    def test_invalid_int_parameter(self):
+        """Тест невалидного целочисленного параметра"""
+        response = self.client.get('/api/v1/employees/?page=abc')
+        # Должен вернуть 400 или использовать дефолт
+        self.assertIn(response.status_code, [200, 400])
+```
+
+---
+
+## Рекомендации по code review
+
+### Чек-лист для PR
+
+- [ ] Нет прямого доступа `request.data['key']` — только `.get()`
+- [ ] Все `int()` обёрнуты в try/except или используют safe_int()
+- [ ] Проверка типа для QueryDict/dict перед `.dict()`
+- [ ] `.strip()` используется с проверкой на None
+- [ ] Есть тесты для новых API endpoints
+- [ ] Обработаны все edge cases (empty, None, invalid)
+
+### GitHub Actions / CI
+
+Добавить в `.github/workflows/quality.yml`:
+```yaml
+name: Code Quality
+
+on: [push, pull_request]
+
+jobs:
+  audit:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: Run code audit
+        run: |
+          cd backend
+          bash scripts/audit_code_patterns.sh
+          # Fail если найдены критичные проблемы
+          ! grep -rn "request\.data\[" --include="*.py" . | grep -v "# noqa"
+```
+
+---
+
+## Приоритеты исправления
+
+1. **Срочно (в течение недели):**
+   - Исправить все `request.data['key']` → `.get('key')`
+   - Добавить обработку ValueError для int()
+
+2. **Важно (в течение месяца):**
+   - Настроить pre-commit hooks
+   - Добавить mypy для статической типизации
+   - Написать тесты для критичных API endpoints
+
+3. **Желательно:**
+   - Настроить CI/CD с автопроверкой
+   - Провести code review всех найденных мест
+   - Документировать паттерны в wiki
+
+---
+
+## Полезные ссылки
+
+- [DRF Best Practices](https://www.django-rest-framework.org/api-guide/requests/)
+- [Django Security Checklist](https://docs.djangoproject.com/en/5.0/howto/deployment/checklist/)
+- [Python Type Hints](https://docs.python.org/3/library/typing.html)
diff --git a/backend/notifications/api/views.py b/backend/notifications/api/views.py
index f521de2a..81da77f9 100644
--- a/backend/notifications/api/views.py
+++ b/backend/notifications/api/views.py
@@ -341,36 +341,36 @@ def channel_preferences(request):
     elif request.method == 'PUT':
         # Обновляем настройки
         if 'web_enabled' in request.data:
-            prefs.web_enabled = request.data['web_enabled']
+            prefs.web_enabled = request.data.get('web_enabled')
         
         if 'email_enabled' in request.data:
-            prefs.email_enabled = request.data['email_enabled']
+            prefs.email_enabled = request.data.get('email_enabled')
         
         if 'email_frequency' in request.data:
-            frequency = request.data['email_frequency']
+            frequency = request.data.get('email_frequency')
             if frequency in ['instant', 'daily', 'weekly', 'disabled']:
                 prefs.email_frequency = frequency
         
         if 'push_enabled' in request.data:
-            prefs.push_enabled = request.data['push_enabled']
+            prefs.push_enabled = request.data.get('push_enabled')
         
         if 'dnd_enabled' in request.data:
-            prefs.dnd_enabled = request.data['dnd_enabled']
+            prefs.dnd_enabled = request.data.get('dnd_enabled')
         
         if 'dnd_start_time' in request.data:
             from datetime import datetime
-            time_str = request.data['dnd_start_time']
+            time_str = request.data.get('dnd_start_time')
             if time_str:
                 prefs.dnd_start_time = datetime.strptime(time_str, '%H:%M').time()
         
         if 'dnd_end_time' in request.data:
             from datetime import datetime
-            time_str = request.data['dnd_end_time']
+            time_str = request.data.get('dnd_end_time')
             if time_str:
                 prefs.dnd_end_time = datetime.strptime(time_str, '%H:%M').time()
         
         if 'disabled_verbs' in request.data:
-            prefs.disabled_verbs = request.data['disabled_verbs']
+            prefs.disabled_verbs = request.data.get('disabled_verbs')
         
         prefs.save()
         
diff --git a/backend/scripts/audit_code_patterns.sh b/backend/scripts/audit_code_patterns.sh
new file mode 100755
index 00000000..90992edd
--- /dev/null
+++ b/backend/scripts/audit_code_patterns.sh
@@ -0,0 +1,27 @@
+#!/bin/bash
+# Аудит потенциально проблемных паттернов в коде
+
+echo "=== Поиск потенциальных проблем ==="
+echo ""
+
+echo "1. Небезопасный доступ к request.data:"
+grep -rn "request\.data\[" --include="*.py" . | grep -v "# noqa" || echo "✓ Не найдено"
+echo ""
+
+echo "2. Использование .dict() без проверки типа:"
+grep -rn "\.data\.dict()" --include="*.py" . | grep -v "hasattr" || echo "✓ Не найдено"
+echo ""
+
+echo "3. Прямое обращение к QueryDict без .get():"
+grep -rn "request\.POST\[" --include="*.py" . | grep -v "# noqa" || echo "✓ Не найдено"
+echo ""
+
+echo "4. Небезопасный int() без try/except:"
+grep -rn "int(.*request\." --include="*.py" . | head -20
+echo ""
+
+echo "5. Отсутствие проверки на None перед .strip():"
+grep -rn "\.strip()" --include="*.py" . | grep -v "if.*:" | head -10
+echo ""
+
+echo "=== Аудит завершен ==="