Критика кода перевода средств в FastAPI

1. Критика кода перевода средств в FastAPI

Условие задачи:
Дан фрагмент кода на FastAPI и asyncpg, реализующий создание таблицы пользователей и эндпоинт для перевода денег между пользователями.

Нужно провести критический разбор этого кода: найти архитектурные, логические, SQL-, безопасностные и синтаксические проблемы, а также указать, почему такая реализация может приводить к ошибкам, уязвимостям или некорректной работе сервиса.

"import asyncpg
from fastapi import FastAPI, Body, HTTPException
app FastAPI()
DB_DSN = ""postgresql://test:test@localhost/testdb""
async def get_connection()-> asyncpg.Connection: 
return await asyncpg.connect(DB_DSN)

@app.on_event(""startup"")
async def on_startup():
conn = await get_connection()
await conn.execute(
‘’’CREATE TABLE IF NOT EXISTS ""user"" (
 id TEXT NOT NULL, 
name TEXT NOT NULL, 
balance DOUBLE PRECISION )’’’


@app.post(""/transfer"")
async def transfer(
from user: str = Body(),
to_user: str = Body(),
amount: float = Body(),):
conn await get_connection()
row = await conn.fetchrow(f'select id, balance from ""user"" where id = {from_user}')
if row[""balance""] < amount:
raise HTTPException(status_code=400, detail=""Insufficient funds"")
await conn.execute(f'UPDATE ""user"" SET balance = balance -{amount} WHERE id = (from_user}')
await.conn.execute(f'UPDATE ""user"" SET balance balance + {amount} WHERE id = {to_user}')
return {""success"": True}"
Спойлеры к решению
Подсказки
  • В коде есть синтаксические ошибки: app FastAPI(), conn await get_connection(), await.conn.execute(...).
  • Таблица названа "user" — это неудобное имя, лучше использовать users.
  • Подключение к БД создаётся на каждый запрос и не закрывается.
  • Для FastAPI-сервиса лучше использовать пул соединений.
  • SQL собирается через f-string, что создаёт SQL-инъекцию.
  • Денежные суммы нельзя хранить в DOUBLE PRECISION и принимать как float.
  • Перевод денег должен выполняться внутри транзакции.
  • При конкурентных запросах нужно блокировать строки пользователей.
  • Нужно проверять существование обоих пользователей.
  • Нужно проверять, что сумма положительная.
Решение

В коде смешаны сразу несколько типов проблем: синтаксис, SQL-инъекции, некорректная работа с соединениями, отсутствие транзакции, race condition при переводе денег и неправильный тип для баланса.

Исходный код в текущем виде даже не запустится.

1. Синтаксические ошибки #

Ошибка:

app FastAPI()

Должно быть:

app = FastAPI()

Ошибка:

conn await get_connection()

Должно быть:

conn = await get_connection()

Ошибка:

await.conn.execute(...)

Должно быть:

await conn.execute(...)

Ошибка в имени параметра:

from user: str = Body()

В Python нельзя использовать пробел в имени переменной. Также лучше не использовать имя from, потому что это ключевое слово языка.

Правильно:

from_user: str = Body()

Также в CREATE TABLE не закрыт вызов conn.execute(...): не хватает закрывающих скобок и закрытия соединения.

2. Неправильное управление соединениями #

В коде каждый раз создаётся новое соединение:

async def get_connection() -> asyncpg.Connection:
    return await asyncpg.connect(DB_DSN)

Но соединения нигде не закрываются. Это приведёт к утечке соединений и со временем база начнёт отказывать в новых подключениях.

Для серверных приложений лучше использовать пул соединений. В документации asyncpg прямо указано, что для приложений, которые часто обрабатывают запросы и берут соединение на короткое время, рекомендуется использовать connection pool. Там же показан типичный паттерн async with pool.acquire() as connection. ([Magic Stack][1])

3. Устаревший способ startup-логики #

В коде используется:

@app.on_event("startup")

Сейчас в FastAPI рекомендованный способ для startup/shutdown-логики — lifespan. В официальной документации FastAPI указано, что startup и shutdown event handlers являются альтернативным deprecated-подходом, а рекомендуется использовать lifespan параметр приложения. ([FastAPI][2])

Это не главная ошибка в задаче, но для нового кода лучше сразу использовать lifespan.

4. Ошибка в SQL-схеме #

Исходный SQL:

CREATE TABLE IF NOT EXISTS "user" (
    id TEXT NOT NULL,
    name TEXT NOT NULL,
    balance DOUBLE PRECISION
)

Проблемы:

1. Нет PRIMARY KEY.
2. balance допускает NULL.
3. balance может стать отрицательным.
4. Для денег используется DOUBLE PRECISION.
5. Таблица названа "user", лучше не использовать такое имя.

Для денег нельзя использовать DOUBLE PRECISION, потому что это тип с плавающей точкой. Он может давать ошибки округления. PostgreSQL рекомендует использовать numeric для денежных сумм и других значений, где нужна точность. ([PostgreSQL][3])

Лучше:

CREATE TABLE IF NOT EXISTS users (
    id TEXT PRIMARY KEY,
    name TEXT NOT NULL,
    balance NUMERIC(18, 2) NOT NULL DEFAULT 0,
    CONSTRAINT chk_users_balance_non_negative CHECK (balance >= 0)
);

5. SQL-инъекция #

Опасный код:

row = await conn.fetchrow(
    f'select id, balance from "user" where id = {from_user}'
)

Проблема: пользовательское значение вставляется прямо в SQL-строку. Если from_user придёт из запроса, злоумышленник сможет подставить SQL-код.

Нужно использовать параметры asyncpg:

row = await conn.fetchrow(
    "SELECT id, balance FROM users WHERE id = $1",
    from_user,
)

В документации asyncpg в примере запрос выполняется с передачей аргумента отдельно от SQL-строки: fetchval('select 2 ^ $1', power). Это и есть правильный стиль параметризации запросов. ([Magic Stack][1])

6. Отсутствие транзакции #

Перевод денег состоит минимум из двух операций:

1. Списать деньги с одного пользователя.
2. Зачислить деньги другому пользователю.

В исходном коде эти операции выполняются отдельно. Если первая операция выполнится, а вторая упадёт, деньги будут списаны, но не будут зачислены.

Нужна транзакция:

async with conn.transaction():
    ...

В asyncpg транзакции создаются через Connection.transaction(), а без явного transaction block изменения применяются сразу в режиме auto-commit. ([Magic Stack][1])

7. Race condition при одновременных переводах #

Опасный сценарий:

1. У пользователя баланс 100.
2. Одновременно приходят два перевода по 80.
3. Оба запроса читают баланс 100.
4. Оба считают, что денег хватает.
5. Оба списывают деньги.
6. Баланс становится некорректным.

Нужно блокировать строку отправителя на время транзакции:

SELECT id, balance
FROM users
WHERE id = $1
FOR UPDATE;

Тогда второй перевод будет ждать, пока первый завершится, и увидит уже обновлённый баланс.

8. Нет проверки существования пользователей #

В коде сразу используется:

if row["balance"] < amount:

Но если отправитель не найден, row будет None, и код упадёт с ошибкой:

TypeError: 'NoneType' object is not subscriptable

Нужно проверить:

if row is None:
    raise HTTPException(status_code=404, detail="Sender not found")

Также нужно проверить получателя.

9. Нет проверки суммы #

Сейчас можно передать:

{
  "from_user": "1",
  "to_user": "2",
  "amount": -100
}

Тогда выражение:

balance = balance - (-100)

увеличит баланс отправителя.

Нужно запретить:

amount <= 0

Также нужно ограничить точность суммы, например максимум 2 знака после запятой.

10. Ошибки в UPDATE-запросах #

Ошибка:

await conn.execute(f'UPDATE "user" SET balance = balance -{amount} WHERE id = (from_user}')

Проблемы:

1. from_user написан как текст внутри SQL, а не как значение переменной.
2. Нет параметризации.
3. Скобки некорректные.
4. Нет пробела между минусом и amount.

Ошибка:

await.conn.execute(f'UPDATE "user" SET balance balance + {amount} WHERE id = {to_user}')

Проблемы:

1. await.conn — синтаксически неверно.
2. Пропущен знак = после balance.
3. SQL-инъекция через to_user.

Правильно:

await conn.execute(
    "UPDATE users SET balance = balance - $1 WHERE id = $2",
    amount,
    from_user,
)

await conn.execute(
    "UPDATE users SET balance = balance + $1 WHERE id = $2",
    amount,
    to_user,
)

11. Неправильный API-контракт #

Сейчас параметры описаны так:

from_user: str = Body()
to_user: str = Body()
amount: float = Body()

Такой вариант возможен, но лучше описать отдельную Pydantic-модель запроса. Так проще валидировать данные.

Например:

from decimal import Decimal
from pydantic import BaseModel, Field


class TransferRequest(BaseModel):
    from_user: str
    to_user: str
    amount: Decimal = Field(gt=0, decimal_places=2)

12. Исправленный каркас #

from contextlib import asynccontextmanager
from decimal import Decimal

import asyncpg
from fastapi import FastAPI, HTTPException
from pydantic import BaseModel, Field


DB_DSN = "postgresql://test:test@localhost/testdb"


class TransferRequest(BaseModel):
    from_user: str
    to_user: str
    amount: Decimal = Field(gt=0, decimal_places=2)


@asynccontextmanager
async def lifespan(app: FastAPI):
    app.state.pool = await asyncpg.create_pool(DB_DSN)

    async with app.state.pool.acquire() as conn:
        await conn.execute(
            """
            CREATE TABLE IF NOT EXISTS users (
                id TEXT PRIMARY KEY,
                name TEXT NOT NULL,
                balance NUMERIC(18, 2) NOT NULL DEFAULT 0,
                CONSTRAINT chk_users_balance_non_negative CHECK (balance >= 0)
            )
            """
        )

    yield

    await app.state.pool.close()


app = FastAPI(lifespan=lifespan)


@app.post("/transfer")
async def transfer(data: TransferRequest):
    if data.from_user == data.to_user:
        raise HTTPException(
            status_code=400,
            detail="Cannot transfer money to the same user",
        )

    async with app.state.pool.acquire() as conn:
        async with conn.transaction():
            sender = await conn.fetchrow(
                """
                SELECT id, balance
                FROM users
                WHERE id = $1
                FOR UPDATE
                """,
                data.from_user,
            )

            if sender is None:
                raise HTTPException(
                    status_code=404,
                    detail="Sender not found",
                )

            receiver = await conn.fetchrow(
                """
                SELECT id
                FROM users
                WHERE id = $1
                FOR UPDATE
                """,
                data.to_user,
            )

            if receiver is None:
                raise HTTPException(
                    status_code=404,
                    detail="Receiver not found",
                )

            if sender["balance"] < data.amount:
                raise HTTPException(
                    status_code=400,
                    detail="Insufficient funds",
                )

            await conn.execute(
                """
                UPDATE users
                SET balance = balance - $1
                WHERE id = $2
                """,
                data.amount,
                data.from_user,
            )

            await conn.execute(
                """
                UPDATE users
                SET balance = balance + $1
                WHERE id = $2
                """,
                data.amount,
                data.to_user,
            )

    return {"success": True}

13. Что ещё нужно добавить в реальном сервисе #

1. Таблицу transfer_operations для истории переводов.
2. Idempotency-Key, чтобы повтор запроса не создавал повторный перевод.
3. Авторизацию: пользователь не должен переводить деньги с чужого счёта.
4. Логирование операций.
5. Метрики успешных и неуспешных переводов.
6. Обработку таймаутов БД.
7. Миграции через Alembic, а не CREATE TABLE внутри приложения.
8. Тесты на конкурентные переводы.
9. Индексы по from_user, to_user, created_at в таблице переводов.
10. Аудит изменения баланса.

Итог: главная проблема не только в синтаксисе. Даже если исправить синтаксис, код останется опасным: он уязвим к SQL-инъекции, теряет соединения, выполняет перевод без транзакции, допускает гонки при конкурентных запросах, использует float для денег и не валидирует входные данные.