Repository navigation
fix(no-ticket): give a usage error for an api_host or proxy URL that does not parse - #589
Open
cloudsmith-iduffy wants to merge 3 commits into
Open
cloudsmith-iduffy wants to merge 3 commits into
cloudsmith-iduffy wants to merge 3 commits into
Conversation
…does not parse A proxy such as http://[bad made every API command stop with LocationParseError, and credential-helper install terraform stop with ValueError: Invalid IPv6 URL. The CLI sent both to Sentry, but the cause is the value that the user gave. Parse api_host and proxy with urllib3, the parser that requests uses, when the session starts. A value that does not parse now gives a usage error with exit status 2. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A malformed proxy URL in config.ini still causes the root callback to crash before validation because resolve_default_proxy() lets urlsplit() raise an uncaught ValueError.
1 open issue
What changed in this PR
This PR adds early URL parsing to session initialization so malformed API host and proxy options produce usage errors instead of crashes.
Changes:
- Add a URL validator using a lazy import of
urllib3.util.parse_url. - Validate session endpoints before the untrusted-config guard.
- Add malformed-URL regression tests and document the behavior.
| File | Description |
|---|---|
cloudsmith_cli/cli/validators.py |
Add URL parsing with usage-error reporting. |
cloudsmith_cli/cli/tests/test_decorators.py |
Test malformed API host and proxy options. |
cloudsmith_cli/cli/decorators.py |
Validate URLs during session initialization. |
CHANGELOG.md |
Document malformed-URL error handling. |
Review effort: Balanced
| opts.api_headers = kwargs.pop("api_headers") | ||
|
|
||
| validators.validate_url("api_host", opts.api_host) | ||
| validators.validate_url("proxy", opts.proxy) |
Collaborator
Author
There was a problem hiding this comment.
Confirmed and fixed. host_matches_suffixes now returns False for a URL that does not parse, so the untrusted-proxy guard rejects it and the session check gives the usage error. test_malformed_proxy_in_cwd_config_is_a_usage_error runs the real main with proxy = http://[bad in the cwd config.ini. It failed with ValueError: Invalid IPv6 URL before the fix.
…nt directory's config The root callback runs the untrusted-proxy guard before the session check. host_matches_suffixes let the urlsplit ValueError escape, so the CLI still crashed and sent a report. A URL that does not parse now matches no suffix. The guard rejects it, and the session check gives the usage error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
cloudsmith-iduffy
marked this pull request as ready for review
November 5, 2026 11:21
Copilot started work to resolve merge conflicts on behalf of
cloudsmith-iduffy
November 11, 2026 21:39
…/fix-malformed-url-usage-error # Conflicts: # CHANGELOG.md Co-authored-by: cloudsmith-iduffy <178375997+cloudsmith-iduffy@users.noreply.github.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Sentry CLOUDSMITH-CLI-26, 27 and 28 are
ValueError: Invalid IPv6 URLfrom adoctorcommand that is not on master yet. Master has the same class of problem in released code. Both errors below came from a value that the user typed, but the CLI sent them to Sentry as crashes:cloudsmith whoami --proxy http://[bad:3128LocationParseError, exit 1, reportedcloudsmith credential-helper install terraform --proxy http://[badValueError: Invalid IPv6 URL, exit 1, reportedcloudsmith whoami --api-host https://[badinitialise_sessionnow parsesapi_hostandproxywithurllib3.util.parse_urlbefore it creates the session. That function is the parser that requests uses, so a value that passes the check also works for the requests that follow. A value that does not parse gives:The check covers values from the command line, the environment and config files. It runs before the untrusted-config guard, which also parses these URLs.
Type of Change
Additional Notes
urllib3is imported insidevalidate_url, becausetest_startup_importsforbids it at module level.The
doctorcode that raised CLI-26 to 28 parses upstream URLs, not these options. It needs its own handling before it merges.🤖 Generated with Claude Code