diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f29e005..1f398e42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `cloudsmith auth --token`, `--force`, and `--json`, and the `cloudsmith login` and `cloudsmith token` commands, now use the same warning format. The warning prefix is now `Warning:` instead of `DEPRECATION WARNING:`. `cloudsmith login` still stops working on November 20, 2026. - When you give an old and a new option on the same command line, such as `--org a --workspace b`, the new option now wins. Previously, the last option won. +### 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. - A `[profile:NAME]` section in `config.ini` now keeps the `[default]` values of `telemetry`, `check_for_update`, `ssl_verify`, `api_ssl_verify`, and `ssl_verify_strict` when it does not set them. Previously, a profile that left out one of these keys reset it to `true`, so `--profile`/`-P` turned telemetry and the update check back on, and turned certificate verification back on. A key that no section sets is still `true`. - `cloudsmith credential-helper install pnpm` now ends `.npmrc` with a newline. Previously, a line that a shell appended to the file, such as `echo '//host/:_authToken=...' >> ~/.npmrc`, joined the last `tokenHelper` line. This made the `tokenHelper` path incorrect, and npm did not read the appended token. The install now also shows each `tokenHelper` line that it sets and the file that it writes to. To add the newline to an existing `.npmrc`, run the install again. If a line is already joined, split it into two lines before you run the install. If you do not, the install removes the appended token. - OIDC authentication now works when the CLI cannot create its config directory, for example in a CI container with a read-only home directory. Previously, the CLI did not exchange the OIDC token and ran the command without credentials. Now the CLI exchanges the token. If the CLI cannot write the token to the disk cache, it continues without the cache. A malformed entry in the disk cache no longer stops the token exchange. diff --git a/cloudsmith_cli/cli/config.py b/cloudsmith_cli/cli/config.py index 4b67868e..82398c3f 100644 --- a/cloudsmith_cli/cli/config.py +++ b/cloudsmith_cli/cli/config.py @@ -1,6 +1,8 @@ """CLI - Configuration.""" +import codecs import configparser +import locale import os import re import stat @@ -8,7 +10,14 @@ 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 deprecation, utils, validators @@ -115,6 +124,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.""" @@ -182,13 +215,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") @@ -203,7 +238,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 @@ -309,6 +344,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) @@ -381,7 +431,7 @@ def find_existing_files(cls): def _set_api_key(cls, path: str | os.PathLike[str], api_key: str = "") -> None: """Write api_key value in a credentials file, preserving structure.""" # Text mode reads CRLF as LF, so no "\r" reaches the pattern. - with open(path) as f: + with open(path, encoding=_get_file_encoding(path)) as f: content = f.read() line = f"api_key = {api_key}" if api_key else "api_key =" # "[ \t]" and not "\s": "\s" also matches a newline, so an empty @@ -398,7 +448,7 @@ def _set_api_key(cls, path: str | os.PathLike[str], api_key: str = "") -> None: content = _set_section_value(content, "default", "api_key", api_key) # "r+" keeps a read-only file unchanged. The mode changes through the open # file, so it applies to the file that gets the key. - with open(path, "r+") as f: + with open(path, "r+", encoding=_get_file_encoding(path)) as f: # Restrict the mode before the write, so the new key is never readable # under the old mode. if hasattr(os, "fchmod"): 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 00000000..838e32d5 --- /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().replace(b"\r\n", b"\n") == ( + 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().replace(b"\r\n", b"\n") == ( + 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 3d56d030..eaefb2e4 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 os import runpy import subprocess @@ -414,7 +415,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"), } @@ -471,6 +471,21 @@ 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, 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") + + 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"