crud.create_account leaks sqlalchemy IntegrityError on duplicate name #36

Open
opened 2026-06-06 21:20:20 +00:00 by padreug · 0 comments
Owner

What

crud.create_account() does an unchecked db.insert("accounts", account). When the name is already in the accounts table — which happens routinely via the background account-sync task that auto-imports Beancount Open directives — the underlying SQLite UNIQUE constraint raises:

sqlite3.IntegrityError: UNIQUE constraint failed: accounts.name

That bubbles out of the FastAPI endpoint as a 500 Internal Server Error with the raw SQL exception text.

Reproduce

Easy to hit from the test harness:

  1. The MINIMAL_LEDGER declares 2020-01-01 open Assets:Cash EUR,SATS.
  2. LNbits starts; libra's wait_for_account_sync background task ingests that Open into libra's accounts table.
  3. A fixture POSTs /libra/api/v1/accounts with {"name": "Assets:Cash", "account_type": "asset"} to register it via the API.
  4. The second write hits the UNIQUE constraint → 500.

Caller can't easily distinguish "the account I wanted to make already exists" from "the database is broken."

Fix

Catch IntegrityError in crud.create_account (or in the endpoint wrapper) and translate to a clean 409 Conflict with a structured detail. A minimal change:

async def create_account(data: CreateAccount) -> Account:
    ...
    try:
        await db.insert("accounts", account)
    except sqlalchemy.exc.IntegrityError as exc:
        if "UNIQUE constraint failed" in str(exc):
            raise HTTPException(
                status_code=HTTPStatus.CONFLICT,
                detail=f"Account '{data.name}' already exists",
            ) from exc
        raise
    ...

Workaround in place

The test harness (tests/conftest.py's _create) currently does a get-then-create — list /accounts first and only POST if missing. That avoids the race but masks the underlying bug from any production caller. Filing this so the proper fix lands.

Scope

  • crud.py: create_account
  • Possibly the /api/v1/accounts POST endpoint if we want the HTTPException translation there instead of in CRUD.
  • Audit for similar unchecked inserts in libra's other CRUD methods.

Small cleanup, not blocking.

## What `crud.create_account()` does an unchecked `db.insert("accounts", account)`. When the name is already in the `accounts` table — which happens routinely via the background account-sync task that auto-imports Beancount Open directives — the underlying SQLite UNIQUE constraint raises: ``` sqlite3.IntegrityError: UNIQUE constraint failed: accounts.name ``` That bubbles out of the FastAPI endpoint as a 500 Internal Server Error with the raw SQL exception text. ## Reproduce Easy to hit from the test harness: 1. The `MINIMAL_LEDGER` declares `2020-01-01 open Assets:Cash EUR,SATS`. 2. LNbits starts; libra's `wait_for_account_sync` background task ingests that Open into libra's `accounts` table. 3. A fixture POSTs `/libra/api/v1/accounts` with `{"name": "Assets:Cash", "account_type": "asset"}` to register it via the API. 4. The second write hits the UNIQUE constraint → 500. Caller can't easily distinguish "the account I wanted to make already exists" from "the database is broken." ## Fix Catch `IntegrityError` in `crud.create_account` (or in the endpoint wrapper) and translate to a clean 409 Conflict with a structured detail. A minimal change: ```python async def create_account(data: CreateAccount) -> Account: ... try: await db.insert("accounts", account) except sqlalchemy.exc.IntegrityError as exc: if "UNIQUE constraint failed" in str(exc): raise HTTPException( status_code=HTTPStatus.CONFLICT, detail=f"Account '{data.name}' already exists", ) from exc raise ... ``` ## Workaround in place The test harness (`tests/conftest.py`'s `_create`) currently does a get-then-create — list `/accounts` first and only POST if missing. That avoids the race but masks the underlying bug from any production caller. Filing this so the proper fix lands. ## Scope - `crud.py: create_account` - Possibly the `/api/v1/accounts` POST endpoint if we want the HTTPException translation there instead of in CRUD. - Audit for similar unchecked inserts in libra's other CRUD methods. Small cleanup, not blocking.
Sign in to join this conversation.
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
aiolabs/libra#36
No description provided.