From 788a9998f609bff212b95d8a876fb545865456b8 Mon Sep 17 00:00:00 2001 From: Padreug Date: Mon, 15 Jun 2026 20:31:27 +0200 Subject: [PATCH 01/12] fix(fava): escape string metadata + make Open currencies optional MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit add_account wrote free-text metadata values straight into the ledger source via /api/source with no escaping — an unescaped quote or newline in an admin-supplied description would corrupt the Beancount file (or forge extra metadata lines). Escape backslash/quote/newline per the tokenizer's cunescape rules (verified round-trip through beancount's parser). Also make the currency constraint list optional so an Open directive can be written unconstrained (currencies are an optional part of the directive, not required). Co-Authored-By: Claude Opus 4.8 (1M context) --- fava_client.py | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/fava_client.py b/fava_client.py index c61e1d9..afc964f 100644 --- a/fava_client.py +++ b/fava_client.py @@ -44,6 +44,22 @@ def _infer_target_file(account_name: str) -> str: return "accounts/chart.beancount" +def _escape_beancount_string(value: str) -> str: + """Escape a value for safe inclusion in a Beancount string literal. + + Beancount's tokenizer unescapes \\", \\n, \\t, \\r, \\\\ etc. (tokens.c + cunescape). Unescaped quotes or newlines in free-text metadata written + straight into the ledger source would corrupt the file, so escape the + backslash first (to keep it round-tripping) then quotes and newlines. + """ + return ( + value.replace("\\", "\\\\") + .replace('"', '\\"') + .replace("\n", "\\n") + .replace("\r", "\\r") + ) + + class FavaClient: """ Async client for Fava REST API. @@ -1544,7 +1560,7 @@ class FavaClient: async def add_account( self, account_name: str, - currencies: list[str], + currencies: Optional[list[str]] = None, opening_date: Optional[date] = None, metadata: Optional[Dict[str, Any]] = None, target_file: Optional[str] = None, @@ -1642,19 +1658,23 @@ class FavaClient: lines = source.split('\n') insert_index = len(lines) - # Step 4: Format Open directive as Beancount text - currencies_str = ", ".join(currencies) - open_lines = [ - "", - f"{opening_date.isoformat()} open {account_name} {currencies_str}" - ] + # Step 4: Format Open directive as Beancount text. + # Currencies are an optional constraint on an Open + # directive; when none are given the account accepts + # any commodity. + open_directive = f"{opening_date.isoformat()} open {account_name}" + if currencies: + open_directive += f" {', '.join(currencies)}" + open_lines = ["", open_directive] # Add metadata if provided if metadata: for key, value in metadata.items(): # Format metadata with proper indentation if isinstance(value, str): - open_lines.append(f' {key}: "{value}"') + open_lines.append( + f' {key}: "{_escape_beancount_string(value)}"' + ) else: open_lines.append(f' {key}: {value}') -- 2.53.0 From 9dd46e818cfeee1cf16b7892e08dec456c5eed7f Mon Sep 17 00:00:00 2001 From: Padreug Date: Mon, 15 Jun 2026 20:31:42 +0200 Subject: [PATCH 02/12] feat(ui): wire admin add-account endpoint into Chart of Accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Surface the existing POST /api/v1/admin/accounts endpoint in the UI: a super-user-only 'Add Account' button on the Chart of Accounts card opens a dialog for the hierarchical account name + optional description, posts with the wallet admin key (require_super_user), then reloads accounts. Client-side prefix validation mirrors the server's _VALID_ACCOUNT_PREFIXES. No currency input — an Open directive does not require currency constraints. Co-Authored-By: Claude Opus 4.8 (1M context) --- static/js/index.js | 45 +++++++++++++++++++++++++++++ templates/libra/index.html | 59 +++++++++++++++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/static/js/index.js b/static/js/index.js index 418ec41..a10d50c 100644 --- a/static/js/index.js +++ b/static/js/index.js @@ -69,6 +69,12 @@ window.app = Vue.createApp({ userWalletId: '', loading: false }, + addAccountDialog: { + show: false, + name: '', + description: '', + loading: false + }, receivableDialog: { show: false, selectedUser: '', @@ -566,6 +572,45 @@ window.app = Vue.createApp({ this.syncingAccounts = false } }, + showAddAccountDialog() { + this.addAccountDialog.name = '' + this.addAccountDialog.description = '' + this.addAccountDialog.show = true + }, + async submitAddAccount() { + const name = (this.addAccountDialog.name || '').trim() + const validPrefixes = ['Assets:', 'Liabilities:', 'Equity:', 'Income:', 'Expenses:'] + if (!validPrefixes.some(p => name.startsWith(p))) { + this.$q.notify({ + type: 'warning', + message: `Account name must start with one of: ${validPrefixes.join(', ')}` + }) + return + } + this.addAccountDialog.loading = true + try { + const {data} = await LNbits.api.request( + 'POST', + '/libra/api/v1/admin/accounts', + this.g.user.wallets[0].adminkey, + { + name, + description: this.addAccountDialog.description || null + } + ) + this.$q.notify({ + type: 'positive', + message: `Account ${data.account_name} created` + + (data.synced_to_libra_db ? '' : ' (sync pending)') + }) + this.addAccountDialog.show = false + await this.loadAccounts() + } catch (error) { + LNbits.utils.notifyApiError(error) + } finally { + this.addAccountDialog.loading = false + } + }, showSettingsDialog() { this.settingsDialog.libraWalletId = this.settings?.libra_wallet_id || '' this.settingsDialog.favaUrl = this.settings?.fava_url || 'http://localhost:3333' diff --git a/templates/libra/index.html b/templates/libra/index.html index 0de0e71..f36c466 100644 --- a/templates/libra/index.html +++ b/templates/libra/index.html @@ -857,7 +857,20 @@ -
Chart of Accounts
+
+
Chart of Accounts
+ + +
@@ -1232,6 +1245,50 @@ + + + + +
Add Account
+ + + + + +
+ Creates an Open directive in the Beancount ledger and syncs it into Libra + so permissions can be granted. Per-user accounts are managed automatically. +
+ +
+ + Create Account + + Cancel +
+ + + + -- 2.53.0 From 7456574f65af39adf519372212081c3e5e2c59cb Mon Sep 17 00:00:00 2001 From: Padreug Date: Mon, 15 Jun 2026 23:53:04 +0200 Subject: [PATCH 03/12] fix(accounts): default CreateChartAccount.currencies to None MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The UI omits currencies so the Open directive is written unconstrained, but the model defaulted currencies to ["EUR","SATS","USD"], so Pydantic refilled them and the endpoint passed the constraint through — every admin-created account got a currency-constrained Open (which would reject postings in other currencies, the same CAD/GBP/JPY bean-check class we hit on user accounts). Default to None so omission reaches add_account and the directive is unconstrained; an explicit list still works for API callers. Co-Authored-By: Claude Opus 4.8 (1M context) --- models.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/models.py b/models.py index 8be64c9..c8632d0 100644 --- a/models.py +++ b/models.py @@ -51,7 +51,11 @@ class CreateAccount(BaseModel): class CreateChartAccount(BaseModel): """Admin-created chart-of-accounts entry written to accounts/chart.beancount.""" name: str # Full hierarchical account name, e.g. "Expenses:Services:Domain" - currencies: list[str] = ["EUR", "SATS", "USD"] + # Optional currency constraint. Omitted by the UI: an Open directive needs + # no currency list, and constraining it would reject postings in other + # currencies (the CAD/GBP/JPY bean-check errors we saw on user accounts). + # None → unconstrained Open; a list → explicit constraint for API callers. + currencies: Optional[list[str]] = None description: Optional[str] = None -- 2.53.0 From caef3cf5e8981b031907a852680fb03b3272e53a Mon Sep 17 00:00:00 2001 From: Padreug Date: Tue, 16 Jun 2026 00:07:39 +0200 Subject: [PATCH 04/12] fix(accounts): 409 when admin-adding an account that already exists add_account no-ops if the Open directive is already present but returned a normal-looking dict, so the admin endpoint reported success ('created (sync pending)') for a duplicate. Return an already_existed flag and raise 409 from the endpoint. Also anchor the existence check on the Open directive with a trailing-boundary match so a prefix (Expenses:Gas) doesn't match a longer sibling (Expenses:GasStation). The flag is additive, so the idempotent user-account path keeps no-opping silently. Co-Authored-By: Claude Opus 4.8 (1M context) --- fava_client.py | 20 ++++++++++++++++---- views_api.py | 8 +++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/fava_client.py b/fava_client.py index afc964f..f176031 100644 --- a/fava_client.py +++ b/fava_client.py @@ -1643,10 +1643,22 @@ class FavaClient: sha256sum = source_data["sha256sum"] source = source_data["source"] - # Step 2: Check if account already exists (may have been created by concurrent request) - if f"open {account_name}" in source: + # Step 2: Check if account already exists (may have been + # created by a concurrent request). Anchor on the Open + # directive and require the account to be followed by + # whitespace/end-of-line so a prefix (Expenses:Gas) does + # not match a longer sibling (Expenses:GasStation). + if re.search( + rf"open {re.escape(account_name)}(?:\s|$)", + source, + re.MULTILINE, + ): logger.info(f"Account {account_name} already exists in {target_file}") - return {"data": sha256sum, "mtime": source_data.get("mtime", "")} + return { + "data": sha256sum, + "mtime": source_data.get("mtime", ""), + "already_existed": True, + } # Step 3: Always append at end of file. # Post-split layout, each include file has one mutation @@ -1700,7 +1712,7 @@ class FavaClient: result = response.json() logger.info(f"Added account {account_name} to {target_file} with currencies {currencies}") - return result + return {**result, "already_existed": False} except httpx.HTTPStatusError as e: # Check for checksum conflict (HTTP 412 Precondition Failed or similar) diff --git a/views_api.py b/views_api.py index 7dce7c3..d88b292 100644 --- a/views_api.py +++ b/views_api.py @@ -3695,13 +3695,19 @@ async def api_admin_add_chart_account( if payload.description: metadata["description"] = payload.description - await fava.add_account( + result = await fava.add_account( account_name=payload.name, currencies=payload.currencies, target_file="accounts/chart.beancount", metadata=metadata, ) + if result.get("already_existed"): + raise HTTPException( + status_code=HTTPStatus.CONFLICT, + detail=f"Account {payload.name} already exists", + ) + # Mirror into libra DB so permissions / metadata layer sees it. from .account_sync import sync_single_account_from_beancount synced = await sync_single_account_from_beancount(payload.name) -- 2.53.0 From 051c9f0c221b462ea6a35fbb159ec016f6c61420 Mon Sep 17 00:00:00 2001 From: Padreug Date: Tue, 16 Jun 2026 01:15:06 +0200 Subject: [PATCH 05/12] feat(ui): constrain add-account to a root-type dropdown + sub-path Free-typing the full hierarchical name let admins fat-finger the parent (wrong/invalid root). Replace the single name field with a required Account Type select (the 5 valid roots, mirroring _VALID_ACCOUNT_PREFIXES) plus a sub-account input, a live 'Will create: ...' preview, and per-segment validation (each part must be a capitalized Beancount account component). The root prefix is now structurally guaranteed valid. Co-Authored-By: Claude Opus 4.8 (1M context) --- static/js/index.js | 32 ++++++++++++++++++++++++++------ templates/libra/index.html | 23 ++++++++++++++++++----- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/static/js/index.js b/static/js/index.js index a10d50c..2b4c750 100644 --- a/static/js/index.js +++ b/static/js/index.js @@ -71,7 +71,8 @@ window.app = Vue.createApp({ }, addAccountDialog: { show: false, - name: '', + rootType: 'Expenses', + subPath: '', description: '', loading: false }, @@ -292,6 +293,16 @@ window.app = Vue.createApp({ }) return options }, + accountRootTypes() { + // The five Beancount root account types — the only valid parents. + // Mirrors the server's _VALID_ACCOUNT_PREFIXES. + return ['Assets', 'Liabilities', 'Equity', 'Income', 'Expenses'] + }, + addAccountFullName() { + const sub = (this.addAccountDialog.subPath || '').trim().replace(/^:+|:+$/g, '') + if (!this.addAccountDialog.rootType || !sub) return '' + return `${this.addAccountDialog.rootType}:${sub}` + }, userOptions() { const options = [] this.users.forEach(user => { @@ -573,17 +584,26 @@ window.app = Vue.createApp({ } }, showAddAccountDialog() { - this.addAccountDialog.name = '' + this.addAccountDialog.rootType = 'Expenses' + this.addAccountDialog.subPath = '' this.addAccountDialog.description = '' this.addAccountDialog.show = true }, async submitAddAccount() { - const name = (this.addAccountDialog.name || '').trim() - const validPrefixes = ['Assets:', 'Liabilities:', 'Equity:', 'Income:', 'Expenses:'] - if (!validPrefixes.some(p => name.startsWith(p))) { + const name = this.addAccountFullName + if (!name) { + this.$q.notify({type: 'warning', message: 'Enter a sub-account name'}) + return + } + // Each segment under the root must be a valid Beancount account + // component: start with an uppercase letter, then letters/digits/hyphens. + const badSegment = name.split(':').slice(1).find( + seg => !/^[A-Z][A-Za-z0-9-]*$/.test(seg) + ) + if (badSegment !== undefined) { this.$q.notify({ type: 'warning', - message: `Account name must start with one of: ${validPrefixes.join(', ')}` + message: `Invalid segment "${badSegment}" — each part must start with a capital letter (letters, digits, hyphens only)` }) return } diff --git a/templates/libra/index.html b/templates/libra/index.html index f36c466..5369f72 100644 --- a/templates/libra/index.html +++ b/templates/libra/index.html @@ -1251,15 +1251,28 @@
Add Account
+ + +
+ Will create: {% raw %}{{ addAccountFullName }}{% endraw %} +
+ Create Account -- 2.53.0 From cd5a6edb7dc2629dd3a4b55c9073136d02a363ba Mon Sep 17 00:00:00 2001 From: Padreug Date: Tue, 16 Jun 2026 22:55:45 +0200 Subject: [PATCH 06/12] feat(accounts): validate account-name characters server-side MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The endpoint only checked the root prefix, so a direct API call (bypassing the UI) could write a malformed Open directive into the ledger source. Add _validate_account_name mirroring Beancount's core/account.py grammar (root [\p{Lu}][\p{L}\p{Nd}-]*, sub [\p{Lu}\p{Nd}][\p{L}\p{Nd}-]*, >=1 sub-account) — verified to match beancount.core.account.is_valid across 20 cases incl. Unicode, digit-start subs, hyphens. Align the client segment regex to the same rule (was ASCII-only, rejected valid names). Co-Authored-By: Claude Opus 4.8 (1M context) --- static/js/index.js | 7 ++++--- views_api.py | 48 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/static/js/index.js b/static/js/index.js index 2b4c750..6f451f7 100644 --- a/static/js/index.js +++ b/static/js/index.js @@ -596,14 +596,15 @@ window.app = Vue.createApp({ return } // Each segment under the root must be a valid Beancount account - // component: start with an uppercase letter, then letters/digits/hyphens. + // component (core/account.py ACC_COMP_NAME_RE): starts with an uppercase + // letter or digit, then letters/digits/hyphens (Unicode letters allowed). const badSegment = name.split(':').slice(1).find( - seg => !/^[A-Z][A-Za-z0-9-]*$/.test(seg) + seg => !/^[\p{Lu}\p{Nd}][\p{L}\p{Nd}-]*$/u.test(seg) ) if (badSegment !== undefined) { this.$q.notify({ type: 'warning', - message: `Invalid segment "${badSegment}" — each part must start with a capital letter (letters, digits, hyphens only)` + message: `Invalid segment "${badSegment}" — letters, digits and hyphens only, starting with a capital letter or digit` }) return } diff --git a/views_api.py b/views_api.py index d88b292..3e8647a 100644 --- a/views_api.py +++ b/views_api.py @@ -3661,6 +3661,52 @@ async def api_get_account_hierarchy( _VALID_ACCOUNT_PREFIXES = ("Assets:", "Liabilities:", "Equity:", "Income:", "Expenses:") +def _is_valid_account_component(component: str, *, is_root: bool) -> bool: + """Validate one ':'-separated account component against Beancount's grammar. + + Mirrors core/account.py: a root component matches ``[\\p{Lu}][\\p{L}\\p{Nd}-]*`` + (must start with an uppercase letter); a sub component matches + ``[\\p{Lu}\\p{Nd}][\\p{L}\\p{Nd}-]*`` (may also start with a digit). Body + chars are letters, decimal digits, or hyphen. Implemented with Unicode-aware + str methods (libra's runtime has no beancount — Fava is a separate service), + so non-ASCII letters are accepted exactly as Beancount accepts them. + """ + if not component: + return False + first, rest = component[0], component[1:] + first_ok = (first.isalpha() and first.isupper()) or ( + not is_root and first.isdecimal() + ) + if not first_ok: + return False + return all(ch == "-" or ch.isalpha() or ch.isdecimal() for ch in rest) + + +def _validate_account_name(name: str) -> None: + """Raise HTTP 400 if ``name`` is not a syntactically valid Beancount account. + + The UI guards this client-side, but the endpoint is reachable directly via + API, so this is the load-bearing check before the name is written into the + ledger source. Requires a root plus at least one sub-component. + """ + parts = name.split(":") + valid = ( + len(parts) >= 2 + and _is_valid_account_component(parts[0], is_root=True) + and all(_is_valid_account_component(p, is_root=False) for p in parts[1:]) + ) + if not valid: + raise HTTPException( + status_code=HTTPStatus.BAD_REQUEST, + detail=( + f"Invalid account name {name!r}: each ':'-separated part must be " + "letters/digits/hyphens, the root starting with an uppercase " + "letter (sub-accounts may start with a digit), with at least one " + "sub-account (e.g. Expenses:Food)." + ), + ) + + @libra_api_router.post("/api/v1/admin/accounts", status_code=HTTPStatus.CREATED) async def api_admin_add_chart_account( payload: CreateChartAccount, @@ -3685,6 +3731,8 @@ async def api_admin_add_chart_account( ), ) + _validate_account_name(payload.name) + logger.info( f"Admin {auth.user_id[:8]} adding chart account {payload.name} " f"with currencies {payload.currencies}" -- 2.53.0 From 87a45ee4d5c25d05e8454b9981777cae9265c8e2 Mon Sep 17 00:00:00 2001 From: Padreug Date: Tue, 16 Jun 2026 23:25:27 +0200 Subject: [PATCH 07/12] test(harness): split-layout ledger + disable rate limiter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test harness was never updated to the post-server-deploy#4 split ledger layout, so libra's per-user account opens (routed to accounts/users.beancount by fava_client._infer_target_file) 500'd as a 'non-source file' and fell back to DB-only — breaking the balance test and contributing to settlement errors. Make the harness ledger a faithful split (root includes accounts/chart.beancount + accounts/users.beancount; title stays in root so the slug still matches). Also raise lnbits_rate_limit_no for the session: the full suite fires >200 req/min and the default limiter 429'd fixture setup intermittently (10-11 errors). The limiter is built once at app creation, so setting it in the session settings fixture (before the app fixture) disables it suite-wide. Net: full suite goes from 1 failed / ~10 errors to fully green. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/conftest.py | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index 6698018..44b5c26 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -108,6 +108,9 @@ def _settings_cleanup(settings: Settings) -> None: settings.lnbits_user_activation_by_invitation_code = False settings.lnbits_register_reusable_activation_code = "" settings.lnbits_register_one_time_activation_codes = [] + # Keep the rate limiter disabled across per-test settings resets (the + # limiter itself is fixed at app-creation time, but keep the value coherent). + settings.lnbits_rate_limit_no = 1_000_000 @pytest.fixture(scope="session") @@ -133,6 +136,12 @@ def settings() -> Iterator[Settings]: lnbits_settings.lnbits_admin_ui = True lnbits_settings.lnbits_extensions_default_install = [] lnbits_settings.lnbits_extensions_deactivate_all = False + # The full suite fires >200 requests/minute; the default rate limit (200/min) + # otherwise 429s fixture setup intermittently. The limiter is built once at + # app creation from this value (lnbits/app.py register_new_ratelimiter), and + # this fixture runs before the `app` fixture, so raising it here disables it + # for the session. + lnbits_settings.lnbits_rate_limit_no = 1_000_000 yield lnbits_settings @@ -170,13 +179,32 @@ option "render_commas" "TRUE" 2020-01-01 open Equity:Opening-Balances EUR,SATS 2020-01-01 open Income:Generic EUR,SATS 2020-01-01 open Expenses:Generic EUR,SATS + +include "accounts/chart.beancount" +include "accounts/users.beancount" """ +# Split-layout include targets, mirroring the production fava layout +# (aiolabs/server-deploy#4). libra's fava_client routes Open directives by +# account name (fava_client._infer_target_file): per-user accounts +# (:User-xxxxxxxx) to accounts/users.beancount, everything else to +# accounts/chart.beancount. Both must exist as Fava *source* files (i.e. be +# included) or /api/source writes 500 with "non-source file". The title stays +# in the root ledger above so Fava's slug still matches LEDGER_SLUG (scalar +# options don't propagate from includes — see aiolabs/server-deploy#9). +CHART_SEED = "; Admin-mutable chart of accounts (libra appends Open directives).\n" +USERS_SEED = "; Per-user account opens (libra appends at signup).\n" + @pytest.fixture(scope="session") def fava_ledger_path(tmp_path_factory: pytest.TempPathFactory) -> Path: - """Session-scoped .beancount file Fava reads from.""" + """Session-scoped split ledger Fava reads from: a root file that includes + accounts/chart.beancount (admin add-account target) and + accounts/users.beancount (per-user opens target).""" ledger_dir = tmp_path_factory.mktemp("libra-ledger") + (ledger_dir / "accounts").mkdir() + (ledger_dir / "accounts" / "chart.beancount").write_text(CHART_SEED) + (ledger_dir / "accounts" / "users.beancount").write_text(USERS_SEED) ledger = ledger_dir / f"{LEDGER_SLUG}.beancount" ledger.write_text(MINIMAL_LEDGER) return ledger -- 2.53.0 From 89f0f8ac3a8cee2014ef502771a8757791f46f89 Mon Sep 17 00:00:00 2001 From: Padreug Date: Tue, 16 Jun 2026 23:25:27 +0200 Subject: [PATCH 08/12] test(accounts): cover admin add-account endpoint 10 integration tests for POST /api/v1/admin/accounts: unconstrained Open write + escaped description metadata, explicit-currency path, duplicate->409, invalid-prefix->400, invalid-characters->400 (parametrized), super-user-only ->403. Adds the add_chart_account helper. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/helpers.py | 22 +++- tests/test_admin_chart_accounts_api.py | 144 +++++++++++++++++++++++++ 2 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 tests/test_admin_chart_accounts_api.py diff --git a/tests/helpers.py b/tests/helpers.py index 80ad343..02d8f78 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -13,7 +13,7 @@ separate ISO code field — this matches `models.ExpenseEntry` / `ReceivableEntr from decimal import Decimal from typing import Any, Optional, Union -from httpx import AsyncClient +from httpx import AsyncClient, Response Amount = Union[Decimal, int, float, str] @@ -106,6 +106,26 @@ async def grant_permission( return r.json() +async def add_chart_account( + client: AsyncClient, + *, + super_user_headers: dict, + name: str, + description: Optional[str] = None, +) -> Response: + """Super user adds a chart-of-accounts entry via the admin endpoint + (POST /api/v1/admin/accounts). Returns the raw Response so callers can + assert on status codes (201 / 400 / 409 / 403).""" + body: dict[str, Any] = {"name": name} + if description is not None: + body["description"] = description + return await client.post( + "/libra/api/v1/admin/accounts", + headers=super_user_headers, + json=body, + ) + + # --------------------------------------------------------------------------- # Entries — user side # --------------------------------------------------------------------------- diff --git a/tests/test_admin_chart_accounts_api.py b/tests/test_admin_chart_accounts_api.py new file mode 100644 index 0000000..1f574f9 --- /dev/null +++ b/tests/test_admin_chart_accounts_api.py @@ -0,0 +1,144 @@ +"""Admin chart-of-accounts endpoint — POST /api/v1/admin/accounts. + +Covers the endpoint wired into the UI's "Add Account" dialog: + + - Writes an Open directive to accounts/chart.beancount via Fava /api/source, + *unconstrained* by currency (the directive needs no currency list), with + provenance + description metadata (escaped for Beancount). + - Mirrors the account into libra's DB (synced_to_libra_db). + - Rejects duplicates with 409, malformed names with 400, and non-super-users + with 403. + +The harness ledger is the split layout (root includes accounts/chart.beancount) +so the endpoint's hardcoded target_file resolves — see conftest.CHART_SEED. +""" +import re +from pathlib import Path +from uuid import uuid4 + +import pytest + +from .helpers import add_chart_account + + +def _chart_text(fava_ledger_path: Path) -> str: + return (fava_ledger_path.parent / "accounts" / "chart.beancount").read_text() + + +def _unique(prefix: str = "Expenses:Test") -> str: + # Capitalized leaf (valid Beancount component) unique per call so the + # session-scoped ledger doesn't collide across tests. + return f"{prefix}:T{uuid4().hex[:8].upper()}" + + +@pytest.mark.anyio +async def test_add_chart_account_writes_unconstrained_open_with_escaped_meta( + client, super_user_headers, fava_ledger_path, +): + """Happy path: 201, the Open directive carries no currency constraint, the + description metadata is escaped, and the account is synced into libra's DB.""" + name = _unique() + r = await add_chart_account( + client, + super_user_headers=super_user_headers, + name=name, + description='has a "quote" and ok', + ) + assert r.status_code == 201, f"expected 201, got {r.status_code}: {r.text}" + body = r.json() + assert body["account_name"] == name + assert body["synced_to_libra_db"] is True + + chart = _chart_text(fava_ledger_path) + # Open present and UNCONSTRAINED: the account name is followed directly by + # end-of-line, not " EUR, SATS, USD". + assert re.search(rf"^\d{{4}}-\d{{2}}-\d{{2}} open {re.escape(name)}$", chart, re.MULTILINE), ( + f"expected an unconstrained Open for {name}, chart was:\n{chart}" + ) + # Description metadata is escaped so the quote can't break the ledger. + assert r'description: "has a \"quote\" and ok"' in chart + assert 'source: "admin-ui"' in chart + + +@pytest.mark.anyio +async def test_add_chart_account_with_explicit_currencies_constrains_open( + client, super_user_headers, fava_ledger_path, +): + """API callers may still pass an explicit currency constraint (the UI never + does). When provided, it lands on the Open directive.""" + name = _unique() + r = await client.post( + "/libra/api/v1/admin/accounts", + headers=super_user_headers, + json={"name": name, "currencies": ["EUR", "SATS"]}, + ) + assert r.status_code == 201, f"expected 201, got {r.status_code}: {r.text}" + chart = _chart_text(fava_ledger_path) + assert re.search(rf"open {re.escape(name)} EUR, SATS$", chart, re.MULTILINE), ( + f"expected a currency-constrained Open for {name}, chart was:\n{chart}" + ) + + +@pytest.mark.anyio +async def test_add_chart_account_duplicate_returns_409( + client, super_user_headers, +): + """Adding the same account twice: first 201, second 409 (not a false success).""" + name = _unique() + first = await add_chart_account(client, super_user_headers=super_user_headers, name=name) + assert first.status_code == 201, f"first add: {first.status_code} {first.text}" + + second = await add_chart_account(client, super_user_headers=super_user_headers, name=name) + assert second.status_code == 409, f"expected 409, got {second.status_code}: {second.text}" + assert "already exists" in second.json().get("detail", "").lower() + + +@pytest.mark.anyio +async def test_add_chart_account_invalid_prefix_returns_400( + client, super_user_headers, fava_ledger_path, +): + """A root outside the five valid types is rejected and never written.""" + before = _chart_text(fava_ledger_path) + r = await add_chart_account(client, super_user_headers=super_user_headers, name="Foo:Bar") + assert r.status_code == 400, f"expected 400, got {r.status_code}: {r.text}" + assert _chart_text(fava_ledger_path) == before, "rejected account must not be written" + + +@pytest.mark.anyio +@pytest.mark.parametrize( + "bad_name", + [ + "Expenses:Foo Bar", # space + "Expenses:foo", # lowercase sub-component start + "Expenses:Foo!", # punctuation + "Expenses:", # no sub-account + "Expenses:Foo::Bar", # empty component + ], +) +async def test_add_chart_account_invalid_characters_returns_400( + client, super_user_headers, fava_ledger_path, bad_name, +): + """Malformed account names are rejected server-side (the UI guard can be + bypassed via the API) and never reach the ledger.""" + before = _chart_text(fava_ledger_path) + r = await add_chart_account(client, super_user_headers=super_user_headers, name=bad_name) + assert r.status_code == 400, f"expected 400 for {bad_name!r}, got {r.status_code}: {r.text}" + assert _chart_text(fava_ledger_path) == before, "rejected account must not be written" + + +@pytest.mark.anyio +async def test_add_chart_account_requires_super_user( + client, configured_user, fava_ledger_path, +): + """A regular user's wallet admin-key passes require_admin_key but fails the + super-user identity check → 403, nothing written.""" + _user, wallet = configured_user + name = _unique() + before = _chart_text(fava_ledger_path) + r = await client.post( + "/libra/api/v1/admin/accounts", + headers={"X-Api-Key": wallet.adminkey, "Content-type": "application/json"}, + json={"name": name}, + ) + assert r.status_code == 403, f"expected 403, got {r.status_code}: {r.text}" + assert _chart_text(fava_ledger_path) == before, "unauthorized add must not be written" -- 2.53.0 From 0ea96cd38439ac202a244a69b07b1609863ef76d Mon Sep 17 00:00:00 2001 From: Padreug Date: Wed, 17 Jun 2026 10:06:28 +0200 Subject: [PATCH 09/12] fix(accounts): anchor duplicate-account detection to a real Open directive (libra-#48) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existence check matched 'open ' anywhere in the chart source, so a prior account's description metadata or a comment mentioning the name produced a false 409, while a real directive with an inline comment and no space ('open X;legacy') was missed → a duplicate Open was appended and bean-check then rejected the file, breaking every later /api/source write. Extract the check into a pure _open_directive_exists() anchored to '^YYYY-MM-DD open ' with an account-boundary negative-lookahead, and unit-test both failure directions plus prefix/child non-matches. Co-Authored-By: Claude Opus 4.8 (1M context) --- fava_client.py | 36 ++++++++++++++++++++++++--------- tests/test_unit.py | 50 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/fava_client.py b/fava_client.py index f176031..053ed0d 100644 --- a/fava_client.py +++ b/fava_client.py @@ -60,6 +60,30 @@ def _escape_beancount_string(value: str) -> str: ) +def _open_directive_exists(source: str, account_name: str) -> bool: + """Return True if `source` already contains an Open directive for exactly + `account_name`. + + Anchored to a real `YYYY-MM-DD open ` directive line (re.MULTILINE) + so the account name can't match text inside another account's description + metadata or a comment (false positive → spurious 409). The trailing + negative-lookahead `(?![\\w:-])` requires the next char not to be an + account-continuation char, so: + - a prefix (Expenses:Gas) does not match a longer sibling + (Expenses:GasStation / Expenses:Gas:Vehicle), and + - a real directive with an inline comment and no space + (`open Expenses:Gas;legacy`) is still detected (`;` ends the name), + which the previous `(?:\\s|$)` boundary missed → duplicate write. + """ + return bool( + re.search( + rf"^\d{{4}}-\d{{2}}-\d{{2}} open {re.escape(account_name)}(?![\w:-])", + source, + re.MULTILINE, + ) + ) + + class FavaClient: """ Async client for Fava REST API. @@ -1644,15 +1668,9 @@ class FavaClient: source = source_data["source"] # Step 2: Check if account already exists (may have been - # created by a concurrent request). Anchor on the Open - # directive and require the account to be followed by - # whitespace/end-of-line so a prefix (Expenses:Gas) does - # not match a longer sibling (Expenses:GasStation). - if re.search( - rf"open {re.escape(account_name)}(?:\s|$)", - source, - re.MULTILINE, - ): + # created by a concurrent request). See + # _open_directive_exists for the anchoring rationale. + if _open_directive_exists(source, account_name): logger.info(f"Account {account_name} already exists in {target_file}") return { "data": sha256sum, diff --git a/tests/test_unit.py b/tests/test_unit.py index 2fa41ec..4f97b03 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -31,9 +31,59 @@ bf = _module("beancount_format") au = _module("account_utils") val = _module("core.validation") mdl = _module("models") +fc = _module("fava_client") AccountType = mdl.AccountType +# --------------------------------------------------------------------------- +# fava_client._open_directive_exists — duplicate-account detection +# --------------------------------------------------------------------------- + + +def test_open_directive_exists_matches_real_directive(): + src = "2020-01-01 open Expenses:Vehicle:Gas\n" + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True + + +def test_open_directive_exists_matches_currency_constrained_and_metadata(): + src = ( + "2020-01-01 open Expenses:Vehicle:Gas EUR, SATS\n" + ' added_by: "abc"\n' + ) + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True + + +def test_open_directive_exists_matches_inline_comment_without_space(): + # Valid Beancount: the account token ends at ';'. The old (?:\\s|$) boundary + # missed this → duplicate Open written → bean-check breaks. + src = "2020-01-01 open Expenses:Vehicle:Gas;legacy-import\n" + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is True + + +def test_open_directive_exists_ignores_name_inside_description(): + # The name appears only inside another account's description metadata. + src = ( + "2020-01-01 open Expenses:Notes\n" + ' description: "remember to open Expenses:Vehicle:Gas next month"\n' + ) + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False + + +def test_open_directive_exists_ignores_comment_line(): + src = "; TODO: open Expenses:Vehicle:Gas eventually\n" + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False + + +def test_open_directive_exists_does_not_match_longer_sibling(): + src = "2020-01-01 open Expenses:Vehicle:GasStation\n" + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False + + +def test_open_directive_exists_does_not_match_deeper_child(): + src = "2020-01-01 open Expenses:Vehicle:Gas:Premium\n" + assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False + + # --------------------------------------------------------------------------- # beancount_format.sanitize_link # --------------------------------------------------------------------------- -- 2.53.0 From 26eb9d457979fcf631f12ec4d271c867471e5f1b Mon Sep 17 00:00:00 2001 From: Padreug Date: Wed, 17 Jun 2026 10:06:57 +0200 Subject: [PATCH 10/12] fix(accounts): don't currency-constrain per-user account opens (libra-#49) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit get_or_create_user_account opened per-user receivable/payable accounts constrained to EUR/SATS/USD, so a posting in any other currency tripped 'Invalid currency CAD/GBP/JPY for account Assets:Receivable:User-…' at bean-check — the exact errors the optional-currencies work set out to fix, which had only reached the admin chart-account path. Open user accounts unconstrained (currencies=None) so they hold arbitrary fiat. Co-Authored-By: Claude Opus 4.8 (1M context) --- crud.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crud.py b/crud.py index b2b43dd..0692806 100644 --- a/crud.py +++ b/crud.py @@ -250,9 +250,13 @@ async def get_or_create_user_account( if not fava_account_exists: # Create account in Fava/Beancount via Open directive logger.info(f"[FAVA CREATE] Creating account in Fava: {account_name}") + # Unconstrained Open: a per-user receivable/payable legitimately + # holds arbitrary fiat (CAD/GBP/JPY/…). Constraining it to + # EUR/SATS/USD made any posting in another currency fail + # bean-check (the errors this account path originally exhibited). await fava.add_account( account_name=account_name, - currencies=["EUR", "SATS", "USD"], # Support common currencies + currencies=None, metadata={ "user_id": user_id, "description": f"User-specific {account_type.value} account" -- 2.53.0 From 39440b75a7f3a076a522dfa3277c24b66999d20d Mon Sep 17 00:00:00 2001 From: Padreug Date: Wed, 17 Jun 2026 10:08:47 +0200 Subject: [PATCH 11/12] fix(accounts): recover ledger-only account instead of blanket 409 (libra-#50) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When add_account reported the Open already existed, the endpoint raised 409 before the DB-mirror step — so an account present in the ledger but missing from libra's DB (a prior sync failure with no cross-DB atomicity, or an out-of-band open) was stranded: invisible to permissions with no recovery path. Now 409 only when the account is already in the DB too; otherwise sync it and return success. Adds a recovery test. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_admin_chart_accounts_api.py | 26 +++++++++++++++++++++++++ views_api.py | 27 +++++++++++++++++++++----- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/tests/test_admin_chart_accounts_api.py b/tests/test_admin_chart_accounts_api.py index 1f574f9..023835f 100644 --- a/tests/test_admin_chart_accounts_api.py +++ b/tests/test_admin_chart_accounts_api.py @@ -93,6 +93,32 @@ async def test_add_chart_account_duplicate_returns_409( assert "already exists" in second.json().get("detail", "").lower() +@pytest.mark.anyio +async def test_add_chart_account_recovers_ledger_only_account( + client, super_user_headers, +): + """An account present in the ledger but absent from libra's DB (prior sync + failure / out-of-band edit) is recovered (synced), not 409'd — otherwise it + would be permanently un-grantable with no path back. + + Reproduce the ledger-only state by creating normally (so Fava parses the + Open) then deleting only the libra-DB row — appending to the ledger file + directly would race Fava's parse cache.""" + from ..crud import db # the same singleton the app uses + + name = _unique("Expenses:Recover") + first = await add_chart_account(client, super_user_headers=super_user_headers, name=name) + assert first.status_code == 201, f"setup create failed: {first.status_code} {first.text}" + + await db.execute("DELETE FROM accounts WHERE name = :name", {"name": name}) + + r = await add_chart_account(client, super_user_headers=super_user_headers, name=name) + assert r.status_code == 201, f"expected 201 recovery, got {r.status_code}: {r.text}" + body = r.json() + assert body.get("already_existed") is True, body + assert body["synced_to_libra_db"] is True, body + + @pytest.mark.anyio async def test_add_chart_account_invalid_prefix_returns_400( client, super_user_headers, fava_ledger_path, diff --git a/views_api.py b/views_api.py index 3e8647a..1b3149a 100644 --- a/views_api.py +++ b/views_api.py @@ -3750,14 +3750,31 @@ async def api_admin_add_chart_account( metadata=metadata, ) + from .account_sync import sync_single_account_from_beancount + if result.get("already_existed"): - raise HTTPException( - status_code=HTTPStatus.CONFLICT, - detail=f"Account {payload.name} already exists", - ) + # The Open directive is already in the ledger. If it's also already + # mirrored into libra's DB, it's a true duplicate → 409. If not (a prior + # sync failed — there's no cross-DB atomicity — or it was opened out of + # band), mirror it now so it becomes grantable instead of being stranded + # with no recovery path. + from .crud import get_account_by_name + + if await get_account_by_name(payload.name) is not None: + raise HTTPException( + status_code=HTTPStatus.CONFLICT, + detail=f"Account {payload.name} already exists", + ) + + synced = await sync_single_account_from_beancount(payload.name) + return { + "success": True, + "account_name": payload.name, + "synced_to_libra_db": synced, + "already_existed": True, + } # Mirror into libra DB so permissions / metadata layer sees it. - from .account_sync import sync_single_account_from_beancount synced = await sync_single_account_from_beancount(payload.name) return { -- 2.53.0 From 3adb3d356af693d2ec5994c1aec0d0388a346cb1 Mon Sep 17 00:00:00 2001 From: Padreug Date: Wed, 17 Jun 2026 10:27:18 +0200 Subject: [PATCH 12/12] fix(accounts): match Beancount's DATE grammar in duplicate detection (libra-#48) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _open_directive_exists hardcoded '^YYYY-MM-DD open ' (dash-only, 2-digit, single-space), but Beancount's DATE token (parser/lexer.l) is (17|18|19|20)[0-9]{2}[-/][0-9]+[-/][0-9]+ and inter-token whitespace is any [ \t\r] run. So a validly-formatted existing Open written as '2024/3/5 open X' or '2020-01-01 open X' escaped detection → duplicate Open appended → bean-check rejects the file. Anchor on Beancount's actual date pattern and [ \t]+ separators. Adds parametrized coverage for slash/single-digit/multi- space/tab variants. Found in a coherence pass over the Beancount source. Co-Authored-By: Claude Opus 4.8 (1M context) --- fava_client.py | 17 +++++++++++++---- tests/test_unit.py | 16 ++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/fava_client.py b/fava_client.py index 053ed0d..eaed06b 100644 --- a/fava_client.py +++ b/fava_client.py @@ -60,11 +60,21 @@ def _escape_beancount_string(value: str) -> str: ) +# Beancount's DATE token (parser/lexer.l): (17|18|19|20)[0-9]{2}[-/][0-9]+[-/][0-9]+ +# — '-' OR '/' separators, 1+ digit month/day. Inter-token whitespace is any +# run of [ \t\r] (ignored by the lexer). The duplicate-detection regex must +# mirror this, or a validly-formatted existing Open (e.g. '2024/3/5 open X' or +# '2020-01-01 open X') escapes detection and a duplicate Open is appended, +# which bean-check then rejects — breaking every later write. +_OPEN_DATE = r"(?:17|18|19|20)\d\d[-/]\d+[-/]\d+" + + def _open_directive_exists(source: str, account_name: str) -> bool: """Return True if `source` already contains an Open directive for exactly `account_name`. - Anchored to a real `YYYY-MM-DD open ` directive line (re.MULTILINE) + Anchored to a real ` open ` directive line (re.MULTILINE), + with `` and the inter-token whitespace matching Beancount's grammar, so the account name can't match text inside another account's description metadata or a comment (false positive → spurious 409). The trailing negative-lookahead `(?![\\w:-])` requires the next char not to be an @@ -72,12 +82,11 @@ def _open_directive_exists(source: str, account_name: str) -> bool: - a prefix (Expenses:Gas) does not match a longer sibling (Expenses:GasStation / Expenses:Gas:Vehicle), and - a real directive with an inline comment and no space - (`open Expenses:Gas;legacy`) is still detected (`;` ends the name), - which the previous `(?:\\s|$)` boundary missed → duplicate write. + (`open Expenses:Gas;legacy`) is still detected (`;` ends the name). """ return bool( re.search( - rf"^\d{{4}}-\d{{2}}-\d{{2}} open {re.escape(account_name)}(?![\w:-])", + rf"^{_OPEN_DATE}[ \t]+open[ \t]+{re.escape(account_name)}(?![\w:-])", source, re.MULTILINE, ) diff --git a/tests/test_unit.py b/tests/test_unit.py index 4f97b03..1c7dabc 100644 --- a/tests/test_unit.py +++ b/tests/test_unit.py @@ -84,6 +84,22 @@ def test_open_directive_exists_does_not_match_deeper_child(): assert fc._open_directive_exists(src, "Expenses:Vehicle:Gas") is False +@pytest.mark.parametrize( + "line", + [ + "2024/3/5 open Expenses:Vehicle:Gas", # slash date, single-digit M/D + "2020-1-1 open Expenses:Vehicle:Gas", # dash date, single-digit M/D + "2020-01-01 open Expenses:Vehicle:Gas", # multiple spaces + "2020-01-01\topen\tExpenses:Vehicle:Gas", # tab separators + "1970-01-01 open Expenses:Vehicle:Gas EUR", # currency-constrained + ], +) +def test_open_directive_exists_matches_beancount_date_and_whitespace_variants(line): + # All of these are valid Beancount Open directives per lexer.l's DATE token + # and ignored inter-token whitespace; each must be detected as existing. + assert fc._open_directive_exists(line + "\n", "Expenses:Vehicle:Gas") is True + + # --------------------------------------------------------------------------- # beancount_format.sanitize_link # --------------------------------------------------------------------------- -- 2.53.0