Reject uppercase wallet path aliases by JONASXZB · Pull Request #789 · ramimbo/mergework

JONASXZB · 2026-06-02T06:54:46Z

Summary

Reject non-canonical uppercase MRWK wallet path aliases on the public wallet API and wallet detail page.

MRWK wallet addresses are emitted as lowercase mrwk1[0-9a-f]{40} values, but the public wallet paths currently accept uppercase aliases such as MRWK1... and silently normalize them to the canonical wallet. This change keeps existing wallet-address normalization for signed/action flows, while making public lookup paths fail closed unless the path is already the canonical lowercase address.

Live evidence

Read-only production checks on 2026-06-02 UTC showed the uppercase alias resolving successfully:

GET https://api.mrwk.online/api/v1/wallets/mrwk194973c55faef36256c62e11f390ed26bf801a99c -> 200
GET https://api.mrwk.online/api/v1/wallets/MRWK194973C55FAEF36256C62E11F390ED26BF801A99C -> 200
GET https://mrwk.online/wallets/mrwk194973c55faef36256c62e11f390ed26bf801a99c -> 200
GET https://mrwk.online/wallets/MRWK194973C55FAEF36256C62E11F390ED26BF801A99C -> 200

The uppercase API response returns the lowercase canonical address, and the uppercase page renders the lowercase canonical wallet title, so the path alias is currently accepted rather than rejected.

Implementation

Adds normalized_wallet_path_address() for public wallet path lookups.
Keeps canonical lowercase wallet paths working.
Keeps missing canonical wallets returning 404 wallet not found.
Rejects uppercase public wallet path aliases with 400.
Does not change wallet registration, linking, transfers, MCP account normalization, or signed wallet action normalization.

Validation

./.venv/bin/python -m pytest tests/test_wallet_api.py -q
# 41 passed, 1 warning

./.venv/bin/python -m ruff format --check app/accounts.py app/wallet_api.py app/public_routes.py tests/test_wallet_api.py
# 4 files already formatted

./.venv/bin/python -m ruff check app/accounts.py app/wallet_api.py app/public_routes.py tests/test_wallet_api.py
# All checks passed

Safety / scope

No private data, wallet keys, signatures, balances, ledger mutation, payout execution, bridge/exchange behavior, or auth/payment logic is changed.

Summary by CodeRabbit

New Features
- Wallet addresses must be provided in lowercase canonical format; non-lowercase addresses are rejected with a 400 error and clear message.
- Wallet lookup endpoints (API and HTML) now enforce canonical-lowercase normalization for address path parameters.
Tests
- Updated tests to verify uppercase/non-canonical addresses are rejected with the expected error on both API and page lookups.

coderabbitai · 2026-06-02T06:54:58Z

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: 972aecb2-cb3d-4eb4-a28c-1a9b37c2f588

📥 Commits

Reviewing files that changed from the base of the PR and between 60cbeba and 4b69797.

📒 Files selected for processing (2)

app/main.py
app/wallet_api.py

💤 Files with no reviewable changes (2)

app/wallet_api.py
app/main.py

📝 Walkthrough

Walkthrough

The PR adds a new wallet address validation helper that enforces canonical (lowercase) form across both the HTML and REST API wallet lookup routes. Non-canonical addresses are rejected with HTTP 400 before any database operations.

Changes

Wallet address canonicalization

Layer / File(s)	Summary
Canonical validation helper `app/accounts.py`	Introduces `normalized_wallet_path_address` which enforces that path-provided wallet addresses already match their lowercase canonical form and raises HTTP 400 on mismatch.
Route and wiring integration `app/public_routes.py`, `app/wallet_api.py`, `app/main.py`	Replaces usage/signature to accept `normalized_wallet_path_address` and applies it in HTML (`wallet_page_context`) and API (`GET /api/v1/wallets/{address}`) handlers before DB lookup; `create_app` now passes the new callable.
Tests `tests/test_wallet_api.py`	Extends wallet lookup tests to assert both the API and HTML wallet lookup endpoints return HTTP 400 with the lowercase-canonical MRWK address validation message for uppercase addresses.

Possibly related issues:
- Proposed work: reject whitespace-padded account and wallet path IDs #778: touches the same wallet path normalization/validation logic and appears related.
Possibly related PRs:
- ramimbo/mergework#239: Updates get_wallet flows with canonical wallet address validation; overlaps in normalization logic.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	Title accurately names the primary change: rejecting uppercase wallet path aliases as a concrete, specific surface change.
Description check	✅ Passed	Description includes all required sections: summary, evidence, implementation, validation, and safety scope with supporting details.
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 no investment claims, price claims, cash-out/off-ramp claims, fabricated payout claims, or private security details. MRWK references are technical (address format) and appropriate.
Bounty Pr Focus	✅ Passed	PR does not reference "Bounty `#N`" or "Refs `#N`", so conditional check does not apply. Actual changes match summary: wallet path validation across five files with test coverage.

_{✏️ 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: b6b06c2a-d590-4849-bf22-274397758f3d

📥 Commits

Reviewing files that changed from the base of the PR and between 68845a6 and 4fb2859.

📒 Files selected for processing (4)

app/accounts.py
app/public_routes.py
app/wallet_api.py
tests/test_wallet_api.py

JONASXZB · 2026-06-02T07:05:52Z

Follow-up for CodeRabbit's helper reuse nitpick: pushed 60cbeba to use the shared normalized_wallet_path_address helper in the wallet API route wiring instead of duplicating the path-canonicalization check inline.

Validation after the update:

./.venv/bin/python -m pytest tests/test_wallet_api.py -q -> 41 passed, 1 warning
./.venv/bin/python -m ruff format --check app/accounts.py app/main.py app/public_routes.py app/wallet_api.py tests/test_wallet_api.py -> 5 files already formatted
./.venv/bin/python -m ruff check app/accounts.py app/main.py app/public_routes.py app/wallet_api.py tests/test_wallet_api.py -> all checks passed

No scope change: public wallet lookup paths reject uppercase aliases; wallet action normalization remains unchanged.

jakerated-r

Reviewed current head 60cbeba01aab6b943c2329a45b9fbaffc5596287 as a non-author.

Evidence checked:

Inspected the diff across app/accounts.py, app/main.py, app/public_routes.py, app/wallet_api.py, and tests/test_wallet_api.py.
Confirmed the new normalized_wallet_path_address() preserves normal wallet normalization for signed/action flows while failing closed for non-canonical public path aliases.
Confirmed both public lookup surfaces now use the stricter path helper: GET /api/v1/wallets/{address} via register_wallet_api_routes(... normalized_wallet_path_address=...), and /wallets/{address} via wallet_page_context().
Verified uppercase MRWK aliases now return 400 wallet address must be lowercase canonical MRWK address, while unknown canonical lowercase wallets still return 404 wallet not found.
Rechecked after the author pushed 60cbeba to address CodeRabbit's helper-reuse nitpick; the previous inline duplication in app/wallet_api.py is gone.
GitHub reports PR #789 open; hosted Quality, readiness, docs, and image checks passed on the current head. CodeRabbit was still processing the second commit at the time of this review, so I am not counting a final CodeRabbit pass as part of this approval.

Local validation on a clean checkout of PR head 60cbeba01aab6b943c2329a45b9fbaffc5596287:

./.venv/bin/python -m pytest tests/test_wallet_api.py -> 41 passed, 1 warning
./.venv/bin/python -m pytest -> 674 passed, 1 warning
./.venv/bin/python -m ruff check . -> all checks passed
./.venv/bin/python -m ruff format --check . -> 104 files already formatted
./.venv/bin/python -m mypy app -> success, no issues in 38 source files
./.venv/bin/python scripts/docs_smoke.py -> docs smoke ok
./.venv/bin/python scripts/check_agents.py -> AGENTS.md ok
git diff --check origin/main...HEAD -> clean
git merge-tree $(git merge-base HEAD origin/main) origin/main HEAD -> no conflict markers or conflict text

This is a scoped path-canonicalization fix with regression coverage and no ledger mutation, auth, payout, private data, key, or wallet-signature behavior changed. Approving.

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2d481d44-5376-4f01-9455-73dc74fef509

📥 Commits

Reviewing files that changed from the base of the PR and between 4fb2859 and 60cbeba.

📒 Files selected for processing (2)

app/main.py
app/wallet_api.py

jakerated-r

Follow-up current-head review after the author pushed 4b69797a3e79ff865850f68020c2dc4056a189e6 after my earlier approval at 60cbeba01aab6b943c2329a45b9fbaffc5596287.

What changed since the prior approved head:

app/main.py no longer imports or passes the now-unused normalized_wallet_address dependency into register_wallet_api_routes().
app/wallet_api.py removes the unused normalized_wallet_address parameter from the route registrar signature.
The API route still receives and calls normalized_wallet_path_address, so the public path-canonicalization behavior remains intact.

Current-head evidence:

Inspected app/accounts.py, app/main.py, app/public_routes.py, app/wallet_api.py, and tests/test_wallet_api.py at head 4b69797a3e79ff865850f68020c2dc4056a189e6.
Confirmed GET /api/v1/wallets/{address} and /wallets/{address} still reject uppercase MRWK aliases through the shared path helper.
Confirmed signed/action wallet flows still use their existing body-field normalization paths and this PR does not change ledger mutation, auth, payout, private data, key, wallet-signature, bridge, or exchange behavior.
GitHub reports PR #789 mergeStateStatus=CLEAN on this head.
Hosted Quality, readiness, docs, and image checks passed on this head, and CodeRabbit reports pass/review skipped after the latest commit.

Local validation on a clean checkout of 4b69797a3e79ff865850f68020c2dc4056a189e6:

./.venv/bin/python -m pytest tests/test_wallet_api.py -> 41 passed, 1 warning
./.venv/bin/python -m pytest -> 674 passed, 1 warning after rerunning serially; an earlier overlapping full-suite run hit a transient SQLite lock during collection while another pytest process was still finishing, then passed cleanly when rerun
./.venv/bin/python -m ruff check . -> all checks passed
./.venv/bin/python -m ruff format --check . -> 104 files already formatted
./.venv/bin/python -m mypy app -> success, no issues in 38 source files
./.venv/bin/python scripts/docs_smoke.py -> docs smoke ok
./.venv/bin/python scripts/check_agents.py -> AGENTS.md ok
git diff --check origin/main...HEAD -> clean
git merge-tree $(git merge-base HEAD origin/main) origin/main HEAD -> no conflict markers or conflict text

Approval still stands on the current head.

keilogic

Reviewed current head 4b69797a3e79ff865850f68020c2dc4056a189e6 as a non-author current-main follow-up.

Requesting changes because the branch is now dirty against current origin/main at 6d324f048319a84cf4b9972d31e6cb81cd4ba036. git merge-tree origin/main HEAD exits non-zero with a content conflict in app/wallet_api.py; GitHub also reports the PR as CONFLICTING.

Branch-local validation still passes on this PR head:

pytest tests/test_wallet_api.py -q -> 41 passed, 1 warning
targeted Ruff check: passed
targeted Ruff format check: 5 files already formatted
mypy app: passed
git diff --check origin/main...HEAD: clean
hosted Quality/readiness/docs/image and CodeRabbit checks are green for the current PR head

I did not find another blocker in the branch-local checks. The remaining issue is resolving the current-main conflict before merge.

Reject uppercase wallet path aliases

4fb2859

JONASXZB mentioned this pull request Jun 2, 2026

MRWK bounty: 75 MRWK - live verification and bug reports, round 1 #656

Closed

coderabbitai Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread app/wallet_api.py Outdated

Reuse wallet path canonicalization helper

60cbeba

jakerated-r approved these changes Jun 2, 2026

View reviewed changes

jakerated-r mentioned this pull request Jun 2, 2026

MRWK bounty: 40 MRWK - review open MergeWork PRs with evidence, round 18 #654

Open

coderabbitai Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread app/wallet_api.py Outdated

Remove unused wallet API normalizer dependency

4b69797

jakerated-r approved these changes Jun 2, 2026

View reviewed changes

keilogic suggested changes Jun 2, 2026

View reviewed changes

ramimbo added the mrwk:needs-info More information needed label Jun 3, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Reject uppercase wallet path aliases#789

Reject uppercase wallet path aliases#789
JONASXZB wants to merge 3 commits into
ramimbo:mainfrom
JONASXZB:codex/reject-uppercase-wallet-paths

JONASXZB commented Jun 2, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

JONASXZB commented Jun 2, 2026

Uh oh!

jakerated-r left a comment

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

jakerated-r left a comment

Uh oh!

keilogic left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

JONASXZB commented Jun 2, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Live evidence

Implementation

Validation

Safety / scope

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JONASXZB commented Jun 2, 2026

Uh oh!

jakerated-r left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jakerated-r left a comment

Choose a reason for hiding this comment

Uh oh!

keilogic left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JONASXZB commented Jun 2, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jun 2, 2026 •

edited

Loading