Validate ledger account identifiers

[SihyeonJeon]

/claim #228

Summary

• Validate direct ledger from_account and to_account values before account creation and ledger writes.
• Reject blank and control-character account identifiers at the shared add_ledger_entry() / ensure_account() boundary.
• Trim valid account identifiers before persisting ledger entries.

Repro before fix

A direct service caller could write a ledger entry with an invalid account id containing a control character:

add_ledger_entry(
    session,
    entry_type="probe",
    from_account="treasury:mrwk",
    to_account="github:alice\nadmin",
    amount_microunits=1,
    reference="probe",
)

That created a public ledger/account value with to_account == "github:alice\nadmin" instead of rejecting malformed account metadata before hashing and persistence.

Verification

• ./.venv/bin/python -m pytest tests/test_ledger.py::test_add_ledger_entry_rejects_blank_or_control_character_accounts tests/test_ledger.py::test_add_ledger_entry_trims_account_ids tests/test_ledger.py::test_bounty_reserve_and_payout_conserve_supply -q -> 3 passed
• ./.venv/bin/python -m pytest tests/test_ledger.py tests/test_wallets.py tests/test_wallet_api.py tests/test_api_mcp.py tests/test_security.py -q -> 122 passed, 1 warning
• ./.venv/bin/python -m pytest -q -> 207 passed, 2 warnings
• ./.venv/bin/ruff check . -> all checks passed
• ./.venv/bin/ruff format --check . -> 37 files already formatted
• ./.venv/bin/python -m mypy app -> success
• ./.venv/bin/python scripts/docs_smoke.py -> docs smoke ok
• ./.venv/bin/python scripts/check_agents.py -> AGENTS.md ok
• git diff --check -> clean

No secrets, wallet material, private payout details, deployment values, private vulnerability details, or MRWK price claims are included.

[TateLyman]

No blockers from my pass. This is a narrow account-id validation change at the shared ledger boundary, and it keeps the stored account ids normalized before account creation and ledger hashing.

Evidence checked:

• Inspected ensure_account() and add_ledger_entry() in app/ledger/service.py; both now route direct account identifiers through _clean_required_text(..., 128) before lookup, account creation, and ledger entry persistence.
• Inspected the new regressions in tests/test_ledger.py for blank/control-character account ids and trimming behavior.
• Ran the targeted regression set: ./.venv/bin/python -m pytest tests/test_ledger.py::test_add_ledger_entry_rejects_blank_or_control_character_accounts tests/test_ledger.py::test_add_ledger_entry_trims_account_ids tests/test_ledger.py::test_bounty_reserve_and_payout_conserve_supply -q -> 3 passed.
• Ran ./.venv/bin/ruff check app/ledger/service.py tests/test_ledger.py, ./.venv/bin/ruff format --check app/ledger/service.py tests/test_ledger.py, and git diff --check -> passed.
• Checked hosted Quality, readiness, docs, and image checks -> passing.

Non-blocking maintainer note: PR #278 overlaps part of this area with broader add_ledger_entry() public-field validation, so the final merge order may need a quick conflict/duplication choice.

[Karry2019web]

Solid validation patch. Verified correctness and edge cases.

Files inspected (in main branch context):

• app/ledger/service.py — ensure_account() (lines 218-228), add_ledger_entry() (lines 324-353), _clean_required_text() (lines 141-149)
• tests/test_ledger.py — the two new tests (lines 84-137 of the PR diff)

Verified:

• _clean_required_text(account_id, "account_id", 128) correctly matches the model column size (Account.id = String(128) at models.py:20)
• Redundant cleaning of from_account/to_account in add_ledger_entry is intentional — it ensures clean values are persisted in the ledger entry row, even if ensure_account() also cleans internally (defense-in-depth)
• All three bad-case patterns are tested: blank ( ), control characters (\n), and whitespace-trimming
• Supply conservation implicitly verified (test creates entries with valid trimmed accounts and checks balances)
• No side effects beyond the _clean_required_text dependency (already imported in module scope)

Edge cases checked:

• from_account=None still works (to_account with whitespace gets trimmed)
• Empty strings after stripping are correctly rejected
• Account IDs exceeding 128 chars are caught by the shared helper
• Only from_account/to_account cleaned, not entry_type or reference (those are caller responsibility via separate cleaning paths)

Risk assessment: Low. Follows existing validation patterns (_clean_proof_metadata, _clean_required_text), adds regression tests with supply conservation checks, and the only new path is early rejection of malformed inputs before DB write.

Bounty: #228

[rebel117]

Review

Inspected the changes to ensure_account and add_ledger_entry in app/ledger/service.py plus the two new tests.

Good: Adding _clean_required_text to ensure_account and add_ledger_entry closes the validation gap at the ledger layer. This means any caller that creates accounts or ledger entries gets sanitized inputs without having to remember to pre-validate. The two tests cover both rejection (blank, control chars) and normalization (whitespace trimming) paths.

Finding 1 — double normalization: Since ensure_account also now cleans the account_id, and add_ledger_entry calls ensure_account after already cleaning from_account/to_account, the account IDs go through _clean_required_text twice. This is harmless (idempotent operation) but worth being aware of.

Finding 2 — None vs empty string for optional accounts: In add_ledger_entry, the clean_from_account/clean_to_account use a conditional to handle None. If a caller passes an empty string instead of None, _clean_required_text will raise 'from_account is required'. This is likely the desired behavior but changes the previous semantics where empty string would be silently passed through. Callers that previously passed empty strings will now get errors.

Checked locally: ran pytest tests/test_ledger.py -q — 19 passed (including the 2 new tests). Supply conservation still holds after the normalization changes.

Files inspected: app/ledger/service.py (ensure_account line 221, add_ledger_entry lines 335-370), tests/test_ledger.py (new tests lines 86-132).

[ramimbo]

Closing this as superseded by accepted #278, which validates the shared public ledger entry fields including account identifiers and keeps the hash-chain coverage in one place.