Refs #576: Reject control chars in account transaction filters#610
Refs #576: Reject control chars in account transaction filters#610Squirbie wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds validation to the transaction-type filtering endpoint in accounts. The ChangesControl Character Validation
Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
eliasx45
left a comment
There was a problem hiding this comment.
Reviewed current head 5a37055b52b63e325f15148540d6eb2eeaa0397e. Verdict: approve.
The change is scoped and applies the existing ledger control-character validator before account transaction-type normalization. That matters because %09 previously trimmed down into the default/all path instead of being rejected as malformed input. The new regression proves the account page now returns a bounded 400 for that case while preserving the existing valid bounty_payment / github_claim filters and invalid-type error.
Validation on this head:
git diff --check origin/main...HEAD
# clean
git merge-tree --write-tree origin/main HEAD
# f5c1672930c7c4bed024585ccc600026d4516938
..\mergework\.venv\Scripts\python.exe -m pytest tests/test_account_routes.py::test_account_page_filters_transactions_by_type -q
# 1 passed
..\mergework\.venv\Scripts\python.exe -m pytest tests/test_account_routes.py -q
# 4 passed
..\mergework\.venv\Scripts\python.exe -m ruff check app/accounts.py tests/test_account_routes.py
# All checks passed!
..\mergework\.venv\Scripts\python.exe -m ruff format --check app/accounts.py tests/test_account_routes.py
# 2 files already formatted
..\mergework\.venv\Scripts\python.exe -m mypy app/accounts.py
# Success: no issues found in 1 source file
..\mergework\.venv\Scripts\python.exe scripts/docs_smoke.py
# docs smoke ok
Hosted quality checks and CodeRabbit are green.
barnacleagent-svg
left a comment
There was a problem hiding this comment.
Verdict: APPROVED
Scope: Adds control character validation to account transaction type filter. Uses existing CONTROL_CHAR_RE pattern. Consistent with prior PRs (#609, #608, etc.).
Checklist:
- Diff: +4/-0, focused validation
- Uses established CONTROL_CHAR_RE
- Test coverage
- CI passing
Conclusion: Clean, consistent input validation. Ready to merge.
Summary
/accounts/{account}?tx_type=...before trimming the transaction-type filtertx_type=%09returns a bounded 400 instead of falling back to the all-transactions viewValidation
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_account_routes.py::test_account_page_filters_transactions_by_type -qPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_account_routes.py -qPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest -quv run --extra dev ruff check .uv run --extra dev ruff check app/accounts.py tests/test_account_routes.pyuv run --extra dev ruff format --check app/accounts.py tests/test_account_routes.pyuv run --extra dev mypy app/accounts.pyuv run --extra dev python scripts/docs_smoke.pygit diff --checkSummary by CodeRabbit
Bug Fixes
Tests