Refs #406: Reject dotless public URL hosts

[jtc268]

Summary

• reject dotless DNS hostnames in public URL validation
• keep malformed-host errors separate from non-public-host errors
• cover dotless host rejection in the URL validation tests

Validation

• ../mergework/.venv/bin/python -m pytest tests/test_security_url_validation.py -q
• ../mergework/.venv/bin/python -m pytest -q
• ../mergework/.venv/bin/python scripts/docs_smoke.py
• ../mergework/.venv/bin/python -m ruff check app/ledger/service.py tests/test_security_url_validation.py
• ../mergework/.venv/bin/python -m ruff format --check app/ledger/service.py tests/test_security_url_validation.py
• ../mergework/.venv/bin/python -m mypy app/ledger/service.py
• git diff --check

Summary by CodeRabbit

• Bug Fixes

  • Enhanced URL validation to enforce stricter hostname requirements and properly reject non-public hostnames.

• Tests

  • Extended hostname validation test coverage for additional rejection scenarios.

[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: 1921b87d-c1f1-45d1-97f2-d6bf9346837e

📥 Commits

Reviewing files that changed from the base of the PR and between d8532d4 and 9e8812d.

📒 Files selected for processing (2)

• app/ledger/service.py
• tests/test_security_url_validation.py

---

📝 Walkthrough

Walkthrough

The validate_public_url() function's non-IP hostname validation branch is refactored to explicitly split the IDNA-decoded hostname into DNS labels and validate each label individually. The refactored logic checks total length, label regex compliance, and minimum label count. Test coverage is extended to verify that non-public internal hosts are rejected.

Changes

Hostname Validation Refactor

Layer / File(s)	Summary
DNS label validation in validate_public_url() app/ledger/service.py, tests/test_security_url_validation.py	validate_public_url() now splits the ASCII hostname into labels and validates each against PUBLIC_DNS_LABEL_RE, with checks for total length, empty state, and minimum label count. Test coverage is extended to reject the https://internal/... host.

Possibly related PRs

• ramimbo/mergework#436: Refactors validate_public_url()'s non-IP hostname validation to enforce DNS label rules and updates URL security test coverage.
• ramimbo/mergework#415: Refactors validate_public_url() and updates test_bounty_urls_reject_non_public_hosts for URL security validation.

🚥 Pre-merge checks | ✅ 6

✅ Passed checks (6 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the specific change: rejecting dotless public URL hosts, directly matching the refactored validation logic in the changeset.
Description check	✅ Passed	The description covers the core changes (rejecting dotless hostnames, separating error types, adding tests) and includes comprehensive validation steps, though the Evidence section is minimally populated.
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, price, cash-out, or fabricated payout claims. MRWK correctly described as native coin with future capabilities requiring discussion.
Bounty Pr Focus	✅ Passed	PR changes match stated objectives: validates DNS labels individually, requires ≥2 labels (rejects dotless), keeps errors separate, includes test coverage for "https://internal".

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

[GHX5T-SOL]

Reviewed current head 9e8812d72f52543402b7b70091253783723d2138.

I do not see a blocker. The diff is focused on public URL validation for bounty #406: validate_public_url() now validates IDNA-decoded DNS labels once, keeps the existing malformed-label and length checks, and rejects dotless non-IP hosts with the same public-host error used for non-global IPs. Existing public DNS names and trailing-dot normalization remain in the existing validation path, and the regression adds https://internal/... to the non-public host cases without broadening unrelated ledger behavior.

Code paths checked:

• app/ledger/service.py: hostname normalization, DNS label validation, and the new len(labels) < 2 public-host guard.
• tests/test_security_url_validation.py: non-public host coverage now includes a dotless DNS hostname alongside localhost/private/shared IP cases.

Validation run locally on this exact head:

• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_security_url_validation.py -q -> 5 passed
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_security.py tests/test_security_url_validation.py -q -> 53 passed
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest -q -> 414 passed
• ./.venv/bin/python -m ruff check . -> passed
• ./.venv/bin/python -m ruff format --check . -> 79 files already formatted
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m mypy app -> success
• PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python scripts/docs_smoke.py -> docs smoke ok
• git diff --check upstream/main...HEAD -> clean
• diff-only Gitleaks scan -> no leaks found

GitHub readback before review: PR is open, non-draft, CLEAN, with Quality and CodeRabbit successful. No secrets, wallet material, private data, production mutation, signing, fund movement, price/exchange/liquidity claim, bridge promise, or off-ramp claim was used.

[weilixiong]

QUALITY_GATE: LOW risk ✅

2 files, +5/-1: Rejects dotless/single-label hostnames like https://internal/ as non-public, preventing private-network bounty URLs.

• app/ledger/service.py: +4/-1 (extract labels before check + len(labels) < 2 raises LedgerError)
• tests/test_security_url_validation.py: +1 line (adds https://internal/ test case)

Verification: pytest tests/test_security_url_validation.py -k non_public → 1/1 passed (0.76s)

[Thanhdn1984]

Reviewed PR #522 for bounty #447.\n\nEvidence:\n- ........................................................................ [ 72%]
........................... [100%]
99 passed in 13.11s → 99 passed.\n- All checks passed! → passed.\n- → clean.\n\nCode review notes:\n- The new reuse keeps the existing per-label regex validation while adding the intended dotless-host rejection.\n- Error split looks correct: malformed host still raises ; syntactically valid but non-public dotless host now raises .\n- Added regression coverage for through the create-bounty path.\n\nNo blockers found.