Refs #576: Reject control chars in wallet hex inputs

[Squirbie]

Summary

• Reject raw control characters in wallet public-key, signature, and MRWK address inputs before trimming/normalization.
• Keep existing clean hex/address behavior unchanged, including ordinary leading/trailing spaces.
• Add wallet API regressions for tab/newline/C1-padded public keys plus control-character-padded transfer from-address, to-address, and signature values.

Evidence

Before this fix, wallet hex/address normalization accepted control-character-padded inputs as clean values. A focused repro showed /api/v1/wallets/register returned 200 OK for public_key_hex values prefixed with \t, suffixed with \n, and prefixed with C1 \x85. The same normalization path also accepted control-character-padded transfer addresses and signatures after trimming.

Distinctness

• Covers wallet hex-material and MRWK address normalization in app/wallets.py only.
• Does not overlap PR Refs #576: Reject control chars in wallet search #608 (/wallets?q=...), PR Refs #576: Reject control chars in wallet type filters #609 (/wallets/{address}?type=...), PR Refs #576: Reject control chars in account transaction filters #610 (/accounts/{account}?tx_type=...), PR Refs #576: Reject control chars in JSON integer fields #614 JSON integer parsing, PR Refs #576: Reject control chars in MRWK amounts #617 MRWK amount parsing, or PR Refs #406: Reject control chars in public search queries #572 public search-query validation.
• Submission quality gate passed and reported no similar open PRs.

Validation

• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_wallet_api.py::test_wallet_register_api_rejects_public_key_control_characters tests/test_wallet_api.py::test_wallet_transfer_api_rejects_address_and_signature_control_characters -q -> 6 passed, 1 warning
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_wallet_api.py tests/test_wallets.py -q -> 50 passed, 1 warning
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest -q -> 488 passed, 1 warning
• uv run --extra dev ruff check app/wallets.py tests/test_wallet_api.py -> passed
• uv run --extra dev ruff format --check app/wallets.py tests/test_wallet_api.py -> 2 files already formatted
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m mypy app/wallets.py -> success
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
• git diff --check -> clean
• submission quality gate -> PASS, no similar open PRs found

No private keys, wallet private material, tokens, signatures, production mutation, admin access, price/liquidity/exchange/bridge/off-ramp claims, or fabricated payout claims are included.

Summary by CodeRabbit

• Bug Fixes

  • Registration and transfer endpoints now reject inputs containing hidden/control characters (tabs, newlines, non-printable bytes), returning clear validation errors so malformed or tampered cryptographic fields are not accepted.

• Tests

  • Added comprehensive tests ensuring registration and transfer flows return proper 400 responses with precise error messages when control characters are present in public keys, addresses, or signatures.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 53de24de-4c67-4176-8e23-2d295202e0dd

📥 Commits

Reviewing files that changed from the base of the PR and between 755aa3d and e97ab0e.

📒 Files selected for processing (2)

• app/wallets.py
• tests/test_wallet_api.py

---

📝 Walkthrough

Walkthrough

Wallet input normalization now validates against ASCII control characters early. A regex pattern and rejection helper prevent inputs with hidden control bytes from reaching subsequent hex or address validation, with test coverage for wallet registration and transfer endpoints.

Changes

Control character validation in wallet inputs

Layer / File(s)	Summary
Control character validation implementation app/wallets.py	CONTROL_CHAR_RE regex detects control bytes; _reject_control_characters() helper validates input before normalization; three functions (normalize_public_key_hex(), normalize_signature_hex(), normalize_wallet_address()) call the helper and raise WalletError when control characters are present.
Endpoint test coverage tests/test_wallet_api.py	Parametrized test for /api/v1/wallets/register with control-character-containing public_key_hex inputs; transfer endpoint test verifies rejection of control characters in sender address, recipient address, and signature_hex with distinct 400 error details.

Possibly related PRs

• ramimbo/mergework#239: Updates MCP get_wallet path to use the normalized/validated address wrapper that integrates with the shared normalize_wallet_address() validation.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly names the changed surface (wallet hex inputs) and the specific change (rejecting control characters), matching the concrete naming requirement.
Description check	✅ Passed	The description covers all required template sections: summary of changes, evidence of the bug/issue, distinction from other PRs, and comprehensive test validation results. All checklist items are documented as passed.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Mergework Public Artifact Hygiene	✅ Passed	README describes MRWK as native to ledger, disclaims operating bridges/exchanges/off-ramps, and states future enhancements require maintainer discussion. No prohibited claims in PR.
Bounty Pr Focus	✅ Passed	PR diff matches stated scope: control character validation added to wallet hex functions in app/wallets.py and comprehensive parametrized test coverage in tests/test_wallet_api.py.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c9e1e15c-b79e-4852-a81c-670c9ac1e76f

📥 Commits

Reviewing files that changed from the base of the PR and between e00dc75 and a9aa863.

📒 Files selected for processing (2)

• app/wallets.py
• tests/test_wallet_api.py

[peterxing]

Review note on current head a9aa8635e498b6d4f12754c9e29a6019a76cd941:

The implementation in app/wallets.py applies _reject_control_characters() to normalize_wallet_address(), so it should protect both transfer from_address and to_address. The new endpoint regression in tests/test_wallet_api.py only exercises a tab-padded from_address and a tab-padded signature_hex; it does not prove to_address rejects raw control characters, and it does not cover newline/C1 padding on transfer inputs.

Please extend test_wallet_transfer_api_rejects_address_and_signature_control_characters to cover to_address as well as from_address, ideally with the same "\t", "\n", and "\x85" pads already used for public_key_hex. That would pin the full transfer-address contract claimed by the PR body.

Evidence inspected: PR diff for app/wallets.py and tests/test_wallet_api.py, PR head/status via GitHub readback, and the existing CodeRabbit transfer-test note. I did not run local tests for this comment.

[Squirbie]

Thanks, that is a fair catch. I extended the same parametrized transfer regression to cover to_address along with from_address and signature_hex for \t, \n, and \x85.

Re-ran validation after the update:

• targeted wallet control-character regressions -> 6 passed, 1 warning
• tests/test_wallet_api.py tests/test_wallets.py -> 50 passed, 1 warning
• full pytest -> 488 passed, 1 warning
• ruff check/format, mypy app/wallets.py, docs smoke, and git diff --check -> passed

[eliasx45]

Reviewed current head e97ab0e9910a62bb4024d8e6e60c3422ce01154f.

Verdict: approve.

The updated wallet validation is focused and complete for this slice. Public key hex, signature hex, and wallet address normalization now reject raw control characters before trimming/normalization, and the transfer regression covers from_address, to_address, and signature_hex across tab/newline/C1 padding.

Validation:

git diff --check origin/main...HEAD
# clean

git merge-tree --write-tree origin/main HEAD
# 94cee3ca2fb14a29063f7460d32b13c82fd49c16

PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ..\mergework\.venv\Scripts\python.exe -m pytest tests\test_wallet_api.py::test_wallet_register_api_rejects_public_key_control_characters tests\test_wallet_api.py::test_wallet_transfer_api_rejects_address_and_signature_control_characters -q
# 6 passed

PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ..\mergework\.venv\Scripts\python.exe -m pytest tests\test_wallet_api.py tests\test_wallets.py -q
# 50 passed

..\mergework\.venv\Scripts\python.exe -m py_compile app\wallets.py tests\test_wallet_api.py
# passed

..\mergework\.venv\Scripts\python.exe -m ruff check app\wallets.py tests\test_wallet_api.py
# All checks passed

..\mergework\.venv\Scripts\python.exe -m ruff format --check app\wallets.py tests\test_wallet_api.py
# 2 files already formatted

PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ..\mergework\.venv\Scripts\python.exe -m mypy app\wallets.py
# Success: no issues found in 1 source file

PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ..\mergework\.venv\Scripts\python.exe scripts\docs_smoke.py
# docs smoke ok

No blockers found.