Reject whitespace-padded account and wallet paths

[JONASXZB]

Refs #656
Related report: #656 (comment)
Related proposed work: #778

Summary

• Add a small path-parameter guard for leading/trailing ordinary whitespace before identifier normalization.
• Apply it to public account API/page routes, account accepted-work route, wallet API route, and wallet detail page.
• Preserve clean mixed-case account/wallet normalization and MCP text-argument behavior, while failing closed for %20-padded HTTP path identifiers.

Why

Live public checks showed /api/v1/accounts/%20github:..., /accounts/...%20, /api/v1/wallets/%20mrwk1..., and /wallets/...%20 normalized to canonical resources with HTTP 200. That makes malformed generated URLs look canonical. This PR keeps normal clean values working and rejects whitespace-padded path aliases with bounded 400 responses.

Verification

• .venv/bin/python -m pytest tests/test_account_validation.py tests/test_wallet_api.py::test_wallet_lookup_rejects_path_whitespace_padding tests/test_wallet_api.py::test_wallet_lookup_rejects_invalid_addresses_before_lookup -q -> 49 passed, 1 warning
• .venv/bin/python -m pytest tests/test_api_mcp.py::test_github_account_views_normalize_mixed_case_logins tests/test_account_validation.py tests/test_wallet_api.py::test_wallet_lookup_rejects_path_whitespace_padding -q -> 49 passed, 1 warning
• .venv/bin/python -m pytest -q -> 684 passed, 1 warning
• .venv/bin/python -m ruff format --check . -> 104 files already formatted
• .venv/bin/python -m ruff check . -> All checks passed
• git diff --check -> clean

No secrets, wallet material, private vulnerability details, deployment credentials, payout details, MRWK price/exchange, or bridge/liquidity claims are included.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Stricter validation for account identifiers and wallet addresses in URLs; requests with leading or trailing whitespace in these path parameters are now rejected with HTTP 400 and a clear error message.

• Tests

  • Added comprehensive test coverage for whitespace validation across account and wallet endpoints.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces a centralized path-parameter validation helper to reject whitespace-padded path segments and applies it consistently to account and wallet route handlers across both API and HTML endpoints. All affected routes now validate and reject leading or trailing whitespace before processing requests.

Changes

Path Whitespace Padding Validation

Layer / File(s)	Summary
Path whitespace validation helper app/path_params.py	New reject_path_whitespace_padding(value, field) function detects leading or trailing whitespace in path values and raises HTTP 400 with a field-specific error message.
Account route whitespace validation app/accounts.py, tests/test_account_validation.py, tests/test_api_mcp.py	Import and apply validation to /api/v1/accounts/{account}, /api/v1/accounts/{account}/accepted-work, and /accounts/{account} route handlers. Updated account tests assert exact error details and verify that REST endpoints reject whitespace while MCP calls normalize it.
Wallet route whitespace validation app/wallet_api.py, app/public_routes.py, tests/test_wallet_api.py	Import and apply validation to GET /api/v1/wallets/{address} and /wallets/{address} route handlers. Added test coverage for both API and non-API wallet endpoints rejecting whitespace-padded addresses.

Possibly related issues

• ramimbo/mergework#778: Addresses the same path-parameter guard requirement for account and wallet routes using the unified whitespace validation approach.

Possibly related PRs

• ramimbo/mergework#388: Initial account-route boundary logic that this PR extends with path-parameter validation.
• ramimbo/mergework#394: Refactored wallet public-page routing that this PR augments with whitespace validation for the address path parameter.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly and concretely names the changed surface: rejecting whitespace-padded account and wallet paths, which accurately reflects the main changes across the modified files.
Description check	✅ Passed	The description provides a clear summary, references the related issue (#656), explains the rationale, documents verification steps with test results, and confirms all pre-merge checks 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	PR modifies only code/test files; no README, docs, or public artifact changes that could contain investment, price, cash-out, payout, or security claims.
Bounty Pr Focus	✅ Passed	PR does not reference "Bounty #N" or "Refs #N" format; uses generic issue refs (#656, #778). Check is not applicable to this PR context.

_{✏️ 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: bbb71272-8265-4b61-b10c-c309ad34e646

📥 Commits

Reviewing files that changed from the base of the PR and between 68845a6 and 8ce87f2.

📒 Files selected for processing (7)

• app/accounts.py
• app/path_params.py
• app/public_routes.py
• app/wallet_api.py
• tests/test_account_validation.py
• tests/test_api_mcp.py
• tests/test_wallet_api.py

[coderabbitai]

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Consider adding a comment explaining the control-character bypass.

The early return when control characters are present is intentional and correct—it allows control-character validation to take precedence when both issues exist. However, the rationale is subtle: the range check excludes ordinary space (U+0020, ord=32) while catching C0/C1 controls, so that contains_control_character downstream can provide a more specific error for strings like " \tgithub:alice".

📝 Suggested comment

+# Skip whitespace validation if control characters are present; those will be
+# caught by contains_control_character with a more specific error message.
+# Ordinary space (U+0020) is not in these ranges and will still be validated.
 if any(ord(char) < 32 or 127 <= ord(char) < 160 for char in value):
     return

[shaiananvari8]

Reviewed current head 8ce87f23f31d680b099d11c5058e6538fa18e6a7 as a non-author.

I checked the account and wallet path surfaces touched by this PR:

• app/accounts.py, app/path_params.py, app/public_routes.py, app/wallet_api.py
• tests/test_account_validation.py, tests/test_api_mcp.py, tests/test_wallet_api.py

Local validation passed:

• pytest tests/test_account_validation.py tests/test_wallet_api.py::test_wallet_lookup_rejects_path_whitespace_padding tests/test_wallet_api.py::test_wallet_lookup_rejects_invalid_addresses_before_lookup -q -> 49 passed, 1 warning
• targeted Ruff check passed
• targeted Ruff format check passed (7 files already formatted)
• git diff --check origin/main...HEAD passed
• git merge-tree --write-tree origin/main HEAD passed with clean tree dd3e7592d989866a7ea9e21f268c341489fca97f

I also ran a small TestClient probe that verified clean mixed-case identifiers still normalize while space-padded HTTP path identifiers fail closed:

• /api/v1/accounts/GitHub:Alice -> 200 / github:alice
• /api/v1/accounts/%20GitHub:Alice%20 -> 400 / account must not contain leading or trailing whitespace
• /accounts/%20GitHub:Alice%20 -> 400 / same detail
• /api/v1/wallets/<uppercase valid wallet> -> 200 / normalized lowercase address
• /api/v1/wallets/%20<wallet>%20 -> 400 / MRWK wallet address must not contain leading or trailing whitespace
• /wallets/%20<wallet>%20 -> 400 / same detail

GitHub reports the PR clean; hosted Quality/readiness/docs/image check and CodeRabbit are successful. I did not find a blocking issue in the current head. The CodeRabbit note about documenting the control-character precedence is reasonable but non-blocking because the downstream account/wallet validators still produce the specific control-character errors.

[MyTH-zyxeon]

Current-head review for the #654 review bounty.

I reviewed head 8ce87f23f31d680b099d11c5058e6538fa18e6a7 as a non-author.

What I checked:

• app/path_params.py adds reject_path_whitespace_padding() for path identifiers before account/wallet normalization.
• app/accounts.py, app/public_routes.py, and app/wallet_api.py apply the helper to account API routes, account accepted-work, account pages, wallet API lookup, and wallet pages.
• The helper deliberately returns early for ASCII/C1 control characters so the existing control-character validation keeps the more specific error path.
• Tests cover padded account paths, accepted-work paths, account pages, wallet API/page paths, empty GitHub login behavior, and the fact that MCP get_balance still normalizes text arguments rather than changing MCP behavior.
• Current checks are green: Quality, readiness, docs, and image checks succeeded, and CodeRabbit is successful.

Public live probe:

• GET https://mrwk.online/api/v1/accounts/%20github:ckeplinger199%20 still returns 200 and normalizes to github:ckeplinger199.
• GET https://mrwk.online/api/v1/accounts/%20github:ckeplinger199/accepted-work still returns 200.
• GET https://mrwk.online/accounts/%20github:ckeplinger199%20 still returns 200.
• GET https://mrwk.online/api/v1/wallets/%20mrwk1fb1437aec45b46ec640f44b2e2aced55dc23556e%20 still returns 200 and normalizes to the wallet address.
• GET https://mrwk.online/wallets/%20mrwk1fb1437aec45b46ec640f44b2e2aced55dc23556e%20 still returns 200.

I did not find a blocker in this patch. The change is scoped to HTTP path identifiers, while leaving MCP string argument normalization and existing control-character errors intact.

[aunysillyme]

Approved PR #782.

Evidence & Findings:

• Checked out and inspected current-head commit 8ce87f23f31d680b099d11c5058e6538fa18e6a7 on branch pr-782.
• Inspected files app/accounts.py, app/path_params.py, app/public_routes.py, app/wallet_api.py, tests/test_account_validation.py, tests/test_api_mcp.py, and tests/test_wallet_api.py.
• Verified the stricter validation of URL path parameters in app/path_params.py: JONASXZB introduced reject_path_whitespace_padding(value: str, field: str) which correctly throws an HTTP 400 Bad Request error if a path parameter contains leading or trailing ordinary whitespace.
• Verified that this guard is applied cleanly to public account details routes, accepted work details routes, and wallet details API/page routes. Padded or malformed path variables (e.g. %20github:alice or github:alice%20) are now correctly rejected upfront before normalization, preventing loose URL alias mapping.
• MCP tool integration was verified: mixed-case normalization and balance lookup for arguments (like " GitHub:Alice ") remain fully functional as intended.
• Verified all targeted unit tests pass successfully: ran PYTHONPATH=. .venv/bin/pytest tests/test_account_validation.py tests/test_api_mcp.py tests/test_wallet_api.py and got 190/190 green tests passing cleanly under 18s.
• Verified ruff check formatting is 100% compliant with no lints across the modified codebase.
• Confirmed no trailing whitespaces (git diff --check passes cleanly).

No secrets, wallet private credentials, private vulnerability details, deployment keys, private payout details, or MRWK price claims were included.