From 8e8fe57703968eb3f32b7ec258077e936c08bcf1 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Sat, 24 Oct 2026 19:12:17 +0100 Subject: [PATCH 1/4] fix(no-ticket): read and rewrite config files as UTF-8 The CLI read config.ini and credentials.ini with the locale encoding. On Windows that is cp1252, so a UTF-8 file with Cyrillic text failed to load. The CLI now reads a file as UTF-8 and uses the locale encoding only when UTF-8 decoding fails. When the CLI changes a file, it writes the file back in the same encoding. A file that the CLI cannot decode now gives an error with the file path. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 5 ++ cloudsmith_cli/cli/config.py | 64 +++++++++++++-- .../cli/tests/test_config_encoding.py | 78 +++++++++++++++++++ .../cli/tests/test_unexpected_errors.py | 14 +++- pyproject.toml | 1 + 5 files changed, 154 insertions(+), 8 deletions(-) create mode 100644 cloudsmith_cli/cli/tests/test_config_encoding.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0127743c7..e7deb518c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - After a SAML login, `cloudsmith auth` saves the Workspace to `config.ini`. It writes to the `[profile:NAME]` section of the selected profile, or to `[default]` when you do not select a profile. The CLI writes to the file or directory that `--config-file` gives, or else to your user-level `config.ini`. If the `config.ini` does not exist, the CLI creates it. +### Fixed + +- The CLI now reads `config.ini` and `credentials.ini` as UTF-8. On Windows, a UTF-8 file with characters such as Cyrillic letters caused a `UnicodeDecodeError` in every command. A file in the Windows code page (for example cp1252) still loads. When the CLI changes a file, it keeps the encoding of that file and its UTF-8 byte order mark. The CLI creates new files as UTF-8. +- When the CLI cannot decode a `config.ini` or `credentials.ini`, for example a UTF-16 file, the error now gives the file path and tells you to save the file as UTF-8. + ## [1.30.0] - 2026-10-22 ### Added diff --git a/cloudsmith_cli/cli/config.py b/cloudsmith_cli/cli/config.py index 82f5a8a4a..d7b274a4c 100644 --- a/cloudsmith_cli/cli/config.py +++ b/cloudsmith_cli/cli/config.py @@ -1,13 +1,22 @@ """CLI - Configuration.""" +import codecs import configparser +import locale import os import re import threading from typing import ClassVar import click -from click_configfile import ConfigFileReader, Param, SectionSchema, matches_section +from click_configfile import ( + ConfigFileReader, + Param, + SectionSchema, + generate_configfile_names, + matches_section, + select_config_sections, +) from ..core.utils import get_data_path, read_file from . import utils, validators @@ -105,6 +114,30 @@ def select_config_schema_for(cls, section_name): v.ctx = cls return section_schema + @classmethod + def read_config(cls) -> dict[str, dict[str, object]]: + """Read the config files, each with its own text encoding.""" + # click_configfile reads all files with the locale encoding only. + parser = configparser.ConfigParser() + parser.optionxform = str + for path in generate_configfile_names(cls.config_files, cls.config_searchpath): + encoding = _get_file_encoding(path) + try: + parser.read(path, encoding=encoding) + except UnicodeDecodeError as exc: + raise click.ClickException( + f"Cannot read {path} as {encoding} text. Save the file as UTF-8." + ) from exc + + if not cls.config_sections: + cls.config_sections = cls.collect_config_sections_from_schemas() + storage: dict[str, dict[str, object]] = {} + for section_name in select_config_sections( + parser.sections(), cls.config_sections + ): + cls.process_config_section(parser[section_name], storage) + return storage + @classmethod def get_storage_name_for(cls, section_name): """Get storage name for a configuration section.""" @@ -168,13 +201,15 @@ def get_user_filepath(cls): return (existing_paths or user_paths)[0] @classmethod - def save_workspace(cls, workspace, profile=None, path=None): + def save_workspace( + cls, workspace: str, profile: str | None = None, path: str | None = None + ) -> str | None: """Write the Workspace to the profile section; return the path if it changed.""" if path and os.path.isdir(path): path = os.path.join(path, "config.ini") path = path or cls.get_user_filepath() if os.path.exists(path): - with open(path) as config_file: + with open(path, encoding=_get_file_encoding(path)) as config_file: content = config_file.read() else: content = read_file(get_data_path(), "config.ini") @@ -189,7 +224,7 @@ def save_workspace(cls, workspace, profile=None, path=None): raise ValueError(f"the CLI cannot change the [{section}] section of {path}") os.makedirs(os.path.dirname(path), exist_ok=True) - with open(path, "w") as config_file: + with open(path, "w", encoding=_get_file_encoding(path)) as config_file: config_file.write(updated) return path @@ -287,6 +322,21 @@ def _load_values_into_opts(opts, values): setattr(opts, k, v) +def _get_file_encoding(path: str | os.PathLike[str]) -> str: + """Get the text encoding of a config file; a new file is UTF-8.""" + try: + with open(path, "rb") as config_file: + data = config_file.read() + except OSError: + return "utf-8" + try: + data.decode("utf-8") + except UnicodeDecodeError: + # A file in the locale encoding (for example cp1252 on Windows) must still load. + return locale.getpreferredencoding(False) + return "utf-8-sig" if data.startswith(codecs.BOM_UTF8) else "utf-8" + + def _get_section_value(content, section, key): """Get `key` from the INI `section` of `content`, or None if it does not parse.""" parser = configparser.ConfigParser(interpolation=None) @@ -356,9 +406,9 @@ def find_existing_files(cls): return paths @classmethod - def _set_api_key(cls, path, api_key=""): + def _set_api_key(cls, path: str | os.PathLike[str], api_key: str = "") -> None: """Write api_key value in a credentials file, preserving structure.""" - with open(path) as f: + with open(path, encoding=_get_file_encoding(path)) as f: content = f.read() replacement = rf"\1 = {api_key}" if api_key else r"\1 =" content = re.sub( @@ -367,7 +417,7 @@ def _set_api_key(cls, path, api_key=""): content, flags=re.MULTILINE, ) - with open(path, "w") as f: + with open(path, "w", encoding=_get_file_encoding(path)) as f: f.write(content) @classmethod diff --git a/cloudsmith_cli/cli/tests/test_config_encoding.py b/cloudsmith_cli/cli/tests/test_config_encoding.py new file mode 100644 index 000000000..74c1b2550 --- /dev/null +++ b/cloudsmith_cli/cli/tests/test_config_encoding.py @@ -0,0 +1,78 @@ +"""Tests for the text encoding of config.ini and credentials.ini.""" + +from __future__ import annotations + +import builtins +import configparser +import io +import locale +from pathlib import Path +from typing import IO + +import pytest + +from cloudsmith_cli.cli import config as cli_config + +FILE_TEXTS = [ + pytest.param("utf-8", "путь с", id="utf-8"), + pytest.param("utf-8-sig", "путь с", id="utf-8-with-bom"), + pytest.param("cp1252", "José", id="cp1252"), +] + + +@pytest.fixture(name="cp1252_locale") +def fixture_cp1252_locale(monkeypatch: pytest.MonkeyPatch) -> None: + """Use cp1252 as the locale encoding, as Windows does without UTF-8 mode.""" + + def open_with_cp1252_default( + file: str | Path, + mode: str = "r", + *args: object, + encoding: str | None = None, + **kwargs: object, + ) -> IO: + if "b" not in mode and encoding in (None, "locale"): + encoding = "cp1252" + return builtins.open(file, mode, *args, encoding=encoding, **kwargs) + + monkeypatch.setattr(io, "text_encoding", lambda encoding, *_: encoding or "locale") + monkeypatch.setattr(configparser, "open", open_with_cp1252_default, raising=False) + monkeypatch.setattr(cli_config, "open", open_with_cp1252_default, raising=False) + monkeypatch.setattr(locale, "getpreferredencoding", lambda *_: "cp1252") + + +@pytest.mark.windows +@pytest.mark.usefixtures("cp1252_locale") +@pytest.mark.parametrize(("encoding", "text"), FILE_TEXTS) +def test_config_file_loads_and_keeps_its_encoding( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, encoding: str, text: str +) -> None: + path = tmp_path / "config.ini" + path.write_bytes(f"[default]\n# {text}\nworkspace = old\n".encode(encoding)) + monkeypatch.setattr(cli_config.ConfigReader, "config_files", ["config.ini"]) + monkeypatch.setattr(cli_config.ConfigReader, "config_searchpath", [str(tmp_path)]) + opts = cli_config.Options() + + cli_config.ConfigReader.load_config(opts) + cli_config.ConfigReader.save_workspace("new", path=str(path)) + + assert opts.workspace == "old" + assert path.read_bytes() == ( + f"[default]\n# {text}\nworkspace = new\n".encode(encoding) + ) + + +@pytest.mark.windows +@pytest.mark.usefixtures("cp1252_locale") +@pytest.mark.parametrize(("encoding", "text"), FILE_TEXTS) +def test_credentials_file_keeps_its_encoding( + tmp_path: Path, encoding: str, text: str +) -> None: + path = tmp_path / "credentials.ini" + path.write_bytes(f"[default]\n# {text}\napi_key = old\n".encode(encoding)) + + cli_config.CredentialsReader.update_api_key(str(path), "new") + + assert path.read_bytes() == ( + f"[default]\n# {text}\napi_key = new\n".encode(encoding) + ) diff --git a/cloudsmith_cli/cli/tests/test_unexpected_errors.py b/cloudsmith_cli/cli/tests/test_unexpected_errors.py index 6bd6fc5c0..915d5caa2 100644 --- a/cloudsmith_cli/cli/tests/test_unexpected_errors.py +++ b/cloudsmith_cli/cli/tests/test_unexpected_errors.py @@ -346,7 +346,6 @@ def test_user_error_is_not_reported(runner, tmp_path, sentry_events): ), "duplicate-section": (b"[default]\n[default]\n", "DuplicateSectionError"), "unparsable-line": (b"[default]\n=ap\n", "ParsingError"), - "not-utf8": (b"\xff\xfe\x00garbage", "UnicodeDecodeError"), } @@ -403,6 +402,19 @@ def test_parsing_error_message_is_unchanged(runner, tmp_path, sentry_events): assert sentry_events == [] +@pytest.mark.usefixtures("isolated_config_readers") +def test_undecodable_ini_names_the_file(runner, tmp_path, sentry_events): + bad = tmp_path / "config.ini" + bad.write_bytes(b"[default]\napi_host = \x81\n") + + result = runner.invoke(main, ["whoami", "--config-file", str(bad)]) + + assert result.exit_code == 1 + assert f"Error: Cannot read {bad} as " in result.stderr + assert "Save the file as UTF-8." in result.stderr + assert sentry_events == [] + + @pytest.mark.usefixtures("isolated_config_readers") def test_valid_ini_still_loads(runner, tmp_path, sentry_events): config = tmp_path / "config.ini" diff --git a/pyproject.toml b/pyproject.toml index 3436ce781..5947f3e4c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -128,6 +128,7 @@ norecursedirs = ["bin", ".git", ".venv"] testpaths = ["cloudsmith_cli"] markers = [ "integration: tests that call the live Cloudsmith service", + "windows: tests for Windows behavior that CI also runs on a Windows runner", ] [tool.coverage.run] From e586f577f6c1200c15bcc483bfb6d9bf52530330 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Wed, 28 Oct 2026 10:20:42 +0000 Subject: [PATCH 2/4] Update assertion to normalize line endings Fix line endings in workspace path assertion Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- cloudsmith_cli/cli/tests/test_config_encoding.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudsmith_cli/cli/tests/test_config_encoding.py b/cloudsmith_cli/cli/tests/test_config_encoding.py index 74c1b2550..4cd137e8f 100644 --- a/cloudsmith_cli/cli/tests/test_config_encoding.py +++ b/cloudsmith_cli/cli/tests/test_config_encoding.py @@ -57,7 +57,7 @@ def test_config_file_loads_and_keeps_its_encoding( cli_config.ConfigReader.save_workspace("new", path=str(path)) assert opts.workspace == "old" - assert path.read_bytes() == ( + assert path.read_bytes().replace(b"\r\n", b"\n") == ( f"[default]\n# {text}\nworkspace = new\n".encode(encoding) ) From 41f49c873cfc0af64626977fc0cc12bfe410f9c4 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 28 Oct 2026 10:57:09 +0000 Subject: [PATCH 3/4] Normalize credentials test line endings Co-authored-by: cloudsmith-iduffy <178375997+cloudsmith-iduffy@users.noreply.github.com> --- cloudsmith_cli/cli/tests/test_config_encoding.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudsmith_cli/cli/tests/test_config_encoding.py b/cloudsmith_cli/cli/tests/test_config_encoding.py index 4cd137e8f..838e32d52 100644 --- a/cloudsmith_cli/cli/tests/test_config_encoding.py +++ b/cloudsmith_cli/cli/tests/test_config_encoding.py @@ -73,6 +73,6 @@ def test_credentials_file_keeps_its_encoding( cli_config.CredentialsReader.update_api_key(str(path), "new") - assert path.read_bytes() == ( + assert path.read_bytes().replace(b"\r\n", b"\n") == ( f"[default]\n# {text}\napi_key = new\n".encode(encoding) ) From 03e092e0464d5b13ee7c14a81b63d13d90aafdf7 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Tue, 3 Nov 2026 10:38:53 +0000 Subject: [PATCH 4/4] Pin the locale encoding in the undecodable INI test Byte 0x81 is valid in cp1251, so the test failed on that locale. Co-Authored-By: Claude Opus 5.5 --- cloudsmith_cli/cli/tests/test_unexpected_errors.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cloudsmith_cli/cli/tests/test_unexpected_errors.py b/cloudsmith_cli/cli/tests/test_unexpected_errors.py index 1fe5fb520..537bbf364 100644 --- a/cloudsmith_cli/cli/tests/test_unexpected_errors.py +++ b/cloudsmith_cli/cli/tests/test_unexpected_errors.py @@ -6,6 +6,7 @@ """ import json +import locale import runpy import sys from unittest.mock import patch @@ -418,7 +419,9 @@ def test_parsing_error_message_is_unchanged(runner, tmp_path, sentry_events): @pytest.mark.usefixtures("isolated_config_readers") -def test_undecodable_ini_names_the_file(runner, tmp_path, sentry_events): +def test_undecodable_ini_names_the_file(runner, tmp_path, sentry_events, monkeypatch): + # Byte 0x81 is valid in some locale encodings, such as cp1251. + monkeypatch.setattr(locale, "getpreferredencoding", lambda *_: "utf-8") bad = tmp_path / "config.ini" bad.write_bytes(b"[default]\napi_host = \x81\n")