fix(no-ticket): read and rewrite config files as UTF-8

CHANGELOG.md

@@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ### 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.
 - `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.
 - `cloudsmith vulnerabilities` no longer stops with `IndexError` when a package has no vulnerability scan results. The command now shows the error "The package has no vulnerability scan results." and exits with the status of a 404 error. With `-F json`, it prints this error in the usual JSON error format.

cloudsmith_cli/cli/config.py

@@ -1,14 +1,23 @@
 """CLI - Configuration."""
 
+import codecs
 import configparser
+import locale
 import os
 import re
 import stat
 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
@@ -106,6 +115,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."""
@@ -173,13 +206,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")
@@ -194,7 +229,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
 
@@ -292,6 +327,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)
@@ -364,7 +414,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
@@ -381,7 +431,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"):

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)
+    )

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
@@ -361,7 +362,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"),
 }
 
 
@@ -418,6 +418,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"