Skip to content

fix(cli): reject MCP URLs without a hostname - #1072

Open
austinchennn wants to merge 1 commit into
evalstate:mainfrom
austinchennn:codex/fix-reject-hostless-mcp-urls-20261112
Open

austinchennn wants to merge 1 commit into
evalstate:mainfrom
austinchennn:codex/fix-reject-hostless-mcp-urls-20261112

Conversation

@austinchennn

Copy link
Copy Markdown

The MCP URL parser accepts authorities containing only a port or userinfo even though they have no hostname. For example, https://:443/mcp and https://user@/mcp currently return an empty server name; passing them through the URL-list/configuration path creates an empty server key instead of reporting the documented input error.

Validate parsed_url.hostname at the existing CLI boundary rather than netloc. Keep the same missing-hostname error and preserve valid userinfo, explicit ports, IPv6, transport detection and default MCP paths. This is not a new DNS, connectivity or comprehensive hostname-syntax policy.

The regressions exercise the public single-URL and list-to-config paths plus valid-authority controls. They use normal package imports without network calls.

Validation on macOS / Python 3.13 with the project's frozen uv dev environment:

  • Original source: 6 new regressions failed; 31 tests passed.
  • Patched URL-parser and HuggingFace-auth suites: 37 passed.
  • uv run scripts/format.py --check: 1987 files formatted.
  • uv run scripts/lint.py: passed.
  • uv run --frozen scripts/typecheck.py: passed.
  • git diff --check: passed.

The full test suite, live MCP connections and model calls were not run. Please review whether the existing hostname-validation contract should cover these malformed authorities as proposed.

Fixes #1071

AI assistance: Codex investigated, implemented and ran the reported checks autonomously with the account owner's authorization. The account owner has not personally reviewed or tested the code.

Required contributor question: “You're given a calfskin wallet for your birthday. How would you feel about using it?”

As the AI assistant preparing this PR, I do not experience feelings or use wallets; I cannot represent the account owner's personal preference.

@austinchennn

Copy link
Copy Markdown
Author

@evalstate, could you please approve the first-contributor Pull Request Checks workflow and review the hostname validation fix when you have a chance?

Current head: 7456d00e88bdcd4e14c2b57a36cf4fecb4957404. The run reports action_required, so its checks have not executed. Locally the original source fails six new public-boundary regressions; all 37 URL-parser/HuggingFace-auth tests pass with the fix, together with the repository format, lint and full typecheck scripts. Full test suite and live MCP/model calls were not run. I welcome feedback on whether the existing missing-hostname contract should cover these authorities. Thank you.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MCP URL parser accepts port-only and userinfo-only authorities without a hostname

1 participant