diff --git a/CHANGELOG.md b/CHANGELOG.md index ef96825a..6643c183 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `cloudsmith credential-helper install` and `uninstall` no longer replace a Docker `config.json` or a Cargo `config.toml` that does not parse. Previously, the CLI read such a file as empty and wrote over it, so your settings were lost. Now the command stops with an error that names the file, and it does not write to the file. Correct or remove the file. Then run the command again. (#540) - `cloudsmith credential-helper install` and `uninstall` no longer rewrite a config file when they make no change to its data. Previously, the CLI always rewrote a file in a different formatting, such as the tab-indented `config.json` that Docker writes. The command also made a `.bak` copy and reported changes that it did not make. (#540) - When the API host name or the proxy host name does not exist, for example because of a typo in `--api-host`, commands now fail immediately. Previously, they retried for approximately 62 seconds without output. The CLI still retries a DNS failure that the operating system reports as temporary. (#542) +- An `api_host` or `proxy` with a host or port that is not valid, such as `http://[bad` or `https://host:99999`, now gives a usage error. Before, the CLI stopped with `ValueError` or `LocationParseError` and sent an error report. - When the CLI waits before it retries a failed connection or a 5xx response, it now writes `Request failed: retrying in N second(s) ...` to stderr. Previously, it waited without output. (#542) - `cloudsmith mcp start` now stops without an error when the MCP client closes the connection while the server writes a reply. Previously, the command stopped with `BrokenPipeError` and exit status 1. (#583) - The CLI now exits with status 1 when you interrupt it with Ctrl+C after the reader of its stderr has closed the pipe. Before, the CLI sent an error report and exited with status 120. (#567) diff --git a/cloudsmith_cli/cli/decorators.py b/cloudsmith_cli/cli/decorators.py index 24f149e4..9062766e 100644 --- a/cloudsmith_cli/cli/decorators.py +++ b/cloudsmith_cli/cli/decorators.py @@ -635,6 +635,8 @@ def wrapper(ctx, *args, **kwargs): opts.api_user_agent = kwargs.pop("api_user_agent") opts.api_headers = kwargs.pop("api_headers") + validators.validate_url("api_host", opts.api_host) + validators.validate_url("proxy", opts.proxy) _guard_untrusted_endpoints( ctx, opts, host_suffixes, proxy_suffixes, proxy_trusted=proxy_from_alias ) diff --git a/cloudsmith_cli/cli/tests/test_decorators.py b/cloudsmith_cli/cli/tests/test_decorators.py index ef3ae672..7a83d9a3 100644 --- a/cloudsmith_cli/cli/tests/test_decorators.py +++ b/cloudsmith_cli/cli/tests/test_decorators.py @@ -434,3 +434,28 @@ def command(opts): assert all(isinstance(choice, str) for choice in color_option.type.choices) assert result.output == "False\n" + + +@pytest.mark.parametrize("option", ["--api-host", "--proxy"]) +def test_malformed_url_is_a_usage_error(option): + @click.command() + @initialise_session + def command(opts): + pass + + result = click.testing.CliRunner().invoke( + command, [option, "http://[bad"], obj=Options() + ) + + assert result.exit_code == 2, result.output + assert '"http://[bad" is not a valid URL' in result.output + + +def test_malformed_proxy_in_cwd_config_is_a_usage_error(tmp_path, monkeypatch): + (tmp_path / "config.ini").write_text("[default]\nproxy = http://[bad\n") + monkeypatch.chdir(tmp_path) + + result = click.testing.CliRunner().invoke(main, ["whoami"]) + + assert result.exit_code == 2, result.output + assert '"http://[bad" is not a valid URL' in result.output diff --git a/cloudsmith_cli/cli/validators.py b/cloudsmith_cli/cli/validators.py index 47b53735..c6a6fbc1 100644 --- a/cloudsmith_cli/cli/validators.py +++ b/cloudsmith_cli/cli/validators.py @@ -121,7 +121,10 @@ def host_matches_suffixes(url, suffixes): Parsing the hostname (rather than substring-matching the URL) ensures userinfo, port, and path components cannot smuggle a host past the check. """ - hostname = (urlsplit(url).hostname or "").lower() + try: + hostname = (urlsplit(url).hostname or "").lower() + except ValueError: + return False return bool(hostname) and any( hostname == suffix or hostname.endswith(f".{suffix}") for suffix in suffixes ) @@ -132,6 +135,20 @@ def is_trusted_api_host(host, extra_suffixes=()): return host_matches_suffixes(host, PUBLIC_API_HOST_SUFFIXES + tuple(extra_suffixes)) +def validate_url(name: str, url: str | None) -> None: + """Raise a usage error if the host or port of ``url`` cannot be parsed.""" + from urllib3.util import parse_url + + if not url: + return + try: + parse_url(url) + except ValueError as error: + raise click.UsageError( + f'{name} "{url}" is not a valid URL. Check the host and the port.' + ) from error + + def validate_untrusted_api_host(host, extra_suffixes=()): """Raise if an api_host from an untrusted config is not an allowed host.""" if is_trusted_api_host(host, extra_suffixes):