Refs #576: Reject control chars in wallet search

[aiautotool]

Summary

• Reject raw control characters in the public /wallets?q=... search query before trimming.
• Add a regression for tab-only wallet search returning a bounded 400 instead of being treated as an empty search.

Validation

• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/python -m pytest tests/test_wallet_api.py::test_wallet_search_rejects_control_characters tests/test_wallet_api.py::test_wallet_pages_expose_transfer_and_github_claim_flows -q -> 2 passed
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/python -m pytest tests/test_wallet_api.py -q -> 34 passed
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/python -m ruff check app/public_routes.py tests/test_wallet_api.py -> passed
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/python -m ruff format --check app/public_routes.py tests/test_wallet_api.py -> 2 files already formatted
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/python -m mypy app/public_routes.py -> success
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 .venv/bin/python scripts/docs_smoke.py -> docs smoke ok
• git diff --check -> clean

No private data, wallet material, production mutation, price/liquidity/exchange/bridge/off-ramp claim, or fabricated payout claim is included.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • API now rejects wallet search requests containing control characters with an HTTP 400 error.

• Tests

  • Added test coverage for control character validation in wallet search.

[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: 30bb328d-016e-4322-bea5-fb4e8d426c12

📥 Commits

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

📒 Files selected for processing (2)

• app/public_routes.py
• tests/test_wallet_api.py

---

📝 Walkthrough

Walkthrough

Wallet search now rejects the q query parameter if it contains ASCII control characters. A regex pattern is defined and imported, then applied in wallets_page_context to raise HTTP 400 with detail "q must not contain control characters". A test validates the rejection.

Changes

Control character validation in wallet search

Layer / File(s)	Summary
Regex pattern and import app/public_routes.py	Module imports re and defines CONTROL_CHARACTER_RE to detect ASCII control characters for validation.
Validation logic and test app/public_routes.py, tests/test_wallet_api.py	wallets_page_context validates q and rejects control characters with HTTP 400; test confirms the endpoint returns 400 and the correct error message when control characters appear in the query.

Possibly related PRs

• ramimbo/mergework#520: Both PRs modify the q parameter handling in wallets_page_context—Bounty #427: Add wallet search filters #520 adds wallet search filters on q, and this PR adds control-character validation in the same function.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Refs #576: Reject control chars in wallet search' is concrete and clearly names the changed surface—input validation for control characters in wallet search.
Description check	✅ Passed	The description covers the main change, provides evidence of validation (multiple test and lint command outputs), and confirms no security issues. Template structure is present with appropriate sections.
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	PR contains only technical security validation changes with no public artifact claims about MRWK investment, price, cash-out, fabricated payouts, or private security details.
Bounty Pr Focus	✅ Passed	PR diff matches stated files and scope: app/public_routes.py adds control char validation with test that confirms 400 response on tab input. No unrelated changes detected.

_{✏️ 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.}

[Squirbie]

Reviewed PR #608 at current head 150bf7034be5f6b8ef353077774b80822ad87cf8 and approved.

Evidence: inspected app/public_routes.py and tests/test_wallet_api.py; verified the new wallet-search validation rejects control characters before .strip() so q=%09 no longer becomes an empty/widened wallet search, while normal wallet search and transfer/claim page flows remain covered. I also checked nearby overlap with PR #572: #572 covers bounty/activity/MCP query validation and is currently dirty, while this PR is limited to the wallet page context.

Validation: git diff --check origin/main...HEAD clean; merge-tree clean 728ac9bbdd919ff0fffac8bec9c85b77e0ca605b; changed files are only app/public_routes.py and tests/test_wallet_api.py; targeted wallet tests 2 passed, 1 warning; full tests/test_wallet_api.py 34 passed, 1 warning; ruff check passed; ruff format --check reports 2 files already formatted; mypy app/public_routes.py success; docs smoke ok; hosted CI passed and CodeRabbit passed/review skipped.

No private data, secrets, wallet material, production mutation, price/liquidity/exchange/bridge/off-ramp claims, or private vulnerability details were used.

[peterxing]

Reviewed current head 150bf7034be5f6b8ef353077774b80822ad87cf8 as a non-author.

Verdict: no blocker found.

Evidence: inspected the PR diff for app/public_routes.py and tests/test_wallet_api.py. The validation runs before q.strip(), so a raw tab query is rejected instead of being normalized into an empty search; the added regression exercises the public /wallets route through TestClient and checks the bounded 400 response text. I also checked the current PR metadata: only those two files changed, merge state is CLEAN, hosted Quality/readiness/docs/image check is successful, and CodeRabbit reported no actionable comments on this head.

Scope note: this is limited to wallet search query validation and does not overlap the broader dirty PR #572, which covers bounty/activity/MCP query handling. No wallet material, private data, tokens, production mutation, price/liquidity/exchange/bridge/off-ramp claims, or private vulnerability details were used.

[barnacleagent-svg]

Review for MRWK Bounty #578

Target: PR #608 — Refs #576: Reject control chars in wallet search
Author: @aiautotool

What I inspected

• Diff in app/public_routes.py and tests/test_wallet_api.py
• CI status: Quality, readiness, docs, and image checks ✅
• Test coverage: test_wallet_search_rejects_control_characters tests tab (\t) control char → 400 response
• The regex [\x00-\x1f\x7f] covers all ASCII control characters (0x00-0x1F, DEL/0x7F)

Behavior verified

• Control characters in ?q= parameter → HTTP 400 with clear error
• Code is minimal (+15/-0, 2 files)
• No private data, wallet material, or production mutation

Verdict

Approved. Focused, tested, documented, addresses a real input-validation gap. Eligible for MRWK bounty #578.

[eliasx45]

Reviewed current head 150bf7034be5f6b8ef353077774b80822ad87cf8 for Refs #576.

Verdict: approve.

The change is focused on /wallets?q=... search handling. Validation now rejects raw ASCII control characters before q.strip(), so a tab-only search cannot normalize into an empty/widened wallet search. The added regression exercises the public /wallets route through TestClient and checks the bounded 400 response. Normal wallet page behavior remains covered by the existing transfer/GitHub-claim flow test.

Validation on this checkout:

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

git merge-tree --write-tree origin/main HEAD
# 728ac9bbdd919ff0fffac8bec9c85b77e0ca605b

PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ..\mergework\.venv\Scripts\python.exe -m pytest tests\test_wallet_api.py::test_wallet_search_rejects_control_characters tests\test_wallet_api.py::test_wallet_pages_expose_transfer_and_github_claim_flows -q
# 2 passed

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

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

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

PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ..\mergework\.venv\Scripts\python.exe -m mypy app\public_routes.py
# success

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

I used an isolated local SQLite database for the pytest rerun so import-time app setup could not collide with another local pytest process. GitHub readback: PR is open/non-draft, merge state is clean, hosted CI is green, and CodeRabbit has no actionable comments.