Centralize account-name validation into a shared module; apply to all creation paths #51

Open
opened 2026-06-16 21:59:38 +00:00 by padreug · 0 comments
Owner

Found in the high-effort code review of PR #46.

Two related altitude issues with the new _validate_account_name / _is_valid_account_component (views_api.py):

1. Only the admin endpoint validates. The sibling super-user path POST /api/v1/accounts (api_create_account, views_api.py:291crud.create_account) inserts name into libra's DB with no Beancount-grammar check. A malformed name can enter the DB there and later diverge from / break Fava during account-sync or an Open write. Validation belongs in a shared module (next to account_utils) that every creation path calls — not bolted onto the one endpoint the UI happens to use.

2. The five-type rule and the grammar rule are split. views_api.py:3725 does startswith(_VALID_ACCOUNT_PREFIXES) (the real five-root gate) and :3734 calls _validate_account_name, which accepts any uppercase-initial root. They run back-to-back with divergent 400 messages (e.g. name="Income" → "must start with Income:" instead of "needs a sub-account"). Fold the root-allowlist into the validator so there's one source of truth and one message.

Suggested fix

Add account_utils.validate_account_name(name) (grammar + five-root check, mirroring beancount.core.account — already cross-checked to match is_valid() across 20 cases) and call it from both creation endpoints. Collapses #1 and #2.

Found in the high-effort code review of PR #46. Two related altitude issues with the new `_validate_account_name` / `_is_valid_account_component` (`views_api.py`): **1. Only the admin endpoint validates.** The sibling super-user path `POST /api/v1/accounts` (`api_create_account`, `views_api.py:291` → `crud.create_account`) inserts `name` into libra's DB with **no** Beancount-grammar check. A malformed name can enter the DB there and later diverge from / break Fava during account-sync or an Open write. Validation belongs in a shared module (next to `account_utils`) that every creation path calls — not bolted onto the one endpoint the UI happens to use. **2. The five-type rule and the grammar rule are split.** `views_api.py:3725` does `startswith(_VALID_ACCOUNT_PREFIXES)` (the real five-root gate) and `:3734` calls `_validate_account_name`, which accepts *any* uppercase-initial root. They run back-to-back with divergent 400 messages (e.g. `name="Income"` → "must start with Income:" instead of "needs a sub-account"). Fold the root-allowlist into the validator so there's one source of truth and one message. ### Suggested fix Add `account_utils.validate_account_name(name)` (grammar + five-root check, mirroring `beancount.core.account` — already cross-checked to match `is_valid()` across 20 cases) and call it from both creation endpoints. Collapses #1 and #2.
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#51
No description provided.