feat(ENG-15016): let credential-helper launchers keep install-time settings

cloudsmith_cli/cli/commands/credential_helper/manage.py

@@ -12,10 +12,12 @@
 from urllib.parse import urlsplit
 
 import click
+from click.core import ParameterSource
 
 from cloudsmith_cli.core.cache_utils import ConfigFileParseError
 from cloudsmith_cli.credential_helpers.cargo.installer import CargoInstaller
 from cloudsmith_cli.credential_helpers.generic import PartialInstallError
+from cloudsmith_cli.credential_helpers.launchers import UnsafeLauncherValue
 from cloudsmith_cli.credential_helpers.pnpm.installer import PNPMInstaller
 from cloudsmith_cli.credential_helpers.terraform.installer import (
     TerraformHelperExeNotFound,
@@ -135,6 +137,52 @@ def _terraform_proxy_next_steps(opts) -> list[str]:
     ]
 
 
+# Other users can read a launcher, so it never holds a secret.  A proxy URL can
+# hold credentials.
+_NEVER_KEPT = ("api_key", "api_headers", "keyring_key", "proxy")
+
+_TLS_ENVVARS = ("CLOUDSMITH_SSL_VERIFY", "CLOUDSMITH_WITHOUT_API_SSL_VERIFY")
+
+
+def _main_envvar(option) -> str:
+    """Return the main environment variable of *option*."""
+    names = option.envvar
+    return names if isinstance(names, str) else names[0]
+
+
+def _launcher_settings_from_cli(ctx, opts) -> dict[str, str]:
+    """Return the ``CLOUDSMITH_*`` settings that a launcher keeps.
+
+    An option with a ``CLOUDSMITH_*`` environment variable is kept when its
+    value came from a flag or from the environment.  A value from a config
+    file is not kept, because the helper reads that file itself.  An option
+    of this command replaces the same option of a parent command.
+    """
+    contexts = []
+    while ctx is not None:
+        contexts.insert(0, ctx)
+        ctx = ctx.parent
+
+    supplied = (ParameterSource.COMMANDLINE, ParameterSource.ENVIRONMENT)
+    settings = {}
+    for context in contexts:
+        for option in context.command.params:
+            if option.name in _NEVER_KEPT or not option.envvar:
+                continue
+            envvar = _main_envvar(option)
+            source = context.get_parameter_source(option.name)
+            if envvar.startswith("CLOUDSMITH_") and source in supplied:
+                value = context.params[option.name]
+                settings[envvar] = (
+                    str(value).lower() if isinstance(value, bool) else value
+                )
+    # A -S flag overrides CLOUDSMITH_SSL_VERIFY.  Two environment variables do
+    # not keep that precedence, so the launcher keeps only the resolved value.
+    if [settings.pop(name) for name in _TLS_ENVVARS if name in settings]:
+        settings["CLOUDSMITH_SSL_VERIFY"] = str(opts.ssl_verify).lower()
+    return settings
+
+
 _REPO_FLAGS = ("-r", "--repo", "--repository")
 
 
@@ -311,13 +359,17 @@ def install_cmd(
         helper_args = _terraform_helper_args(ctx, opts, repo)
         install_kwargs["helper_args"] = helper_args
 
+    if getattr(installer, "KEEPS_INSTALL_TIME_SETTINGS", False):
+        install_kwargs["launcher_env"] = _launcher_settings_from_cli(ctx, opts)
+
     try:
         actions = installer.install(**install_kwargs)
     except (
         ConfigFileParseError,
         TerraformrcConflictError,
         TerraformHelperExeNotFound,
         TerraformPluginDirError,
+        UnsafeLauncherValue,
     ) as exc:
         raise click.ClickException(str(exc))
     except OSError as exc:

cloudsmith_cli/cli/tests/commands/test_credential_helper_install.py

@@ -18,9 +18,11 @@
 from cloudsmith_cli.credential_helpers.pnpm.installer import PNPMInstaller
 
 from ....cli.commands.credential_helper import credential_helper
+from ....core import telemetry
 from ....core.credentials.models import CredentialResult
 from ....credential_helpers.docker.installer import DockerInstaller
 from ....credential_helpers.launchers import (
+    UnsafeLauncherValue,
     _launcher_content,
     _launcher_filename,
     is_on_path,
@@ -43,6 +45,20 @@ def runner():
     return click.testing.CliRunner()
 
 
+@pytest.fixture(autouse=True)
+def clear_cloudsmith_env(monkeypatch):
+    """Keep the CLOUDSMITH_* settings of the developer shell out of the installs."""
+    for name in [name for name in os.environ if name.startswith("CLOUDSMITH_")]:
+        monkeypatch.delenv(name)
+    monkeypatch.setenv(telemetry.NO_TELEMETRY_ENV, "1")
+    monkeypatch.setenv(telemetry.DSN_ENV, "")
+
+
+def test_cleared_install_settings_keep_the_error_report_controls():
+    assert os.environ[telemetry.NO_TELEMETRY_ENV] == "1"
+    assert os.environ[telemetry.DSN_ENV] == ""
+
+
 # ---------------------------------------------------------------------------
 # 1. launcher filename + content (pure, platform-parameterised)
 # ---------------------------------------------------------------------------
@@ -96,6 +112,106 @@ def test_launcher_filename_and_content(
     )
 
 
+@pytest.mark.parametrize(
+    "profile,value,expected",
+    [
+        ("C:\\Users\\jo", "C:\\Users\\jo\\a b&c!", "C:\\Users\\jo\\a b&c!"),
+        (
+            "C:\\Users\\José",
+            "c:\\users\\josé\\cs\\config.ini",
+            "%USERPROFILE%\\cs\\config.ini",
+        ),
+    ],
+)
+def test_windows_launcher_settings_are_ascii(monkeypatch, profile, value, expected):
+    """cmd.exe reads a batch file in the console code page, not UTF-8.
+
+    Only a non-ASCII path under the user profile is written as %USERPROFILE%,
+    because another account that runs the package manager has a different
+    profile.
+    """
+    monkeypatch.setenv("USERPROFILE", profile)
+    content = _launcher_content("cmd", windows=True, env={"CLOUDSMITH_X": value})
+    assert content == (
+        "@echo off\r\nsetlocal DisableDelayedExpansion\r\n"
+        f'set "CLOUDSMITH_X={expected}"\r\ncmd %*\r\n'
+    )
+
+
+@pytest.mark.parametrize("value", ["%PATH%", "D:\\Données"])
+def test_windows_launcher_refuses_a_value_a_batch_file_cannot_hold(monkeypatch, value):
+    monkeypatch.setenv("USERPROFILE", "C:\\Users\\José")
+    with pytest.raises(UnsafeLauncherValue, match="CLOUDSMITH_PROFILE"):
+        _launcher_content("cmd", windows=True, env={"CLOUDSMITH_PROFILE": value})
+
+
+def test_install_passes_flag_and_environment_settings_to_the_launcher(
+    runner, monkeypatch, tmp_path
+):
+    """Settings from flags and CLOUDSMITH_* variables go to the launcher.
+
+    A value read from a config file does not, because the helper reads that
+    file itself.  Secrets and the proxy never go to the launcher.  A flag on
+    the parent command also counts.
+    """
+    from ....cli.commands.credential_helper import manage
+    from ....cli.commands.main import main
+
+    launcher_env = {}
+
+    class SettingsInstaller:
+        KEEPS_INSTALL_TIME_SETTINGS = True
+
+        def install(self, **kwargs):
+            launcher_env.update(kwargs["launcher_env"])
+            return []
+
+    monkeypatch.setitem(manage._INSTALLERS, "settings", SettingsInstaller)
+    config = tmp_path / "config.ini"
+    config.write_text("[default]\nssl_verify_strict = false\n")
+    result = runner.invoke(
+        main,
+        args=[
+            "-P",
+            "work",
+            "credential-helper",
+            "install",
+            "settings",
+            "-w",
+            "acme",
+            "--config-file",
+            str(config),
+            "--api-host",
+            "https://api.example.com",
+            "--proxy",
+            "http://proxy.example.com:3128",
+            "-S",
+        ],
+        env={
+            "CLOUDSMITH_API_KEY": "k_secret",
+            "CLOUDSMITH_API_HEADERS": "X-Secret=1",
+            "CLOUDSMITH_KEYRING_KEY": "keyring_secret",
+            "CLOUDSMITH_KEYRING_DIR": "/var/keyring",
+            "CLOUDSMITH_OIDC_AUDIENCE": "cloudsmith-test",
+            "CLOUDSMITH_SERVICE_SLUG": "ci-bot",
+            "CLOUDSMITH_SSL_VERIFY": "1",
+        },
+        catch_exceptions=False,
+    )
+
+    assert result.exit_code == 0, result.output
+    assert launcher_env == {
+        "CLOUDSMITH_WORKSPACE": "acme",
+        "CLOUDSMITH_PROFILE": "work",
+        "CLOUDSMITH_CONFIG_FILE": str(config.resolve()),
+        "CLOUDSMITH_API_HOST": "https://api.example.com",
+        "CLOUDSMITH_SSL_VERIFY": "false",
+        "CLOUDSMITH_OIDC_AUDIENCE": "cloudsmith-test",
+        "CLOUDSMITH_SERVICE_SLUG": "ci-bot",
+        "CLOUDSMITH_KEYRING_DIR": "/var/keyring",
+    }
+
+
 # ---------------------------------------------------------------------------
 # 2. write_launcher / remove_launcher — end-to-end on the host platform
 # ---------------------------------------------------------------------------

cloudsmith_cli/credential_helpers/launchers.py

@@ -29,10 +29,49 @@ def _launcher_filename(name: str, *, windows: bool) -> str:
     return f"{name}.cmd" if windows else name
 
 
-def launcher_arg(value: str) -> str:
+# A batch file expands % even inside quotes and cannot hold " or a line break.
+_UNSAFE_IN_BATCH = ('"', "%", "\r", "\n")
+
+
+class UnsafeLauncherValue(ValueError):
+    """Raised when a value cannot go into a launcher script."""
+
+
+def batch_value(name: str, value: str) -> str:
+    """Return *value* in a form that a batch file holds without change.
+
+    ``cmd.exe`` reads a batch file in the console code page, so the file must
+    be ASCII.  A path under the user profile is written as ``%USERPROFILE%``,
+    which ``cmd.exe`` expands to the correct Unicode path when it runs.  An
+    ASCII path stays literal, because another account that runs the package
+    manager has a different profile.
+    """
+    if any(char in value for char in _UNSAFE_IN_BATCH):
+        raise UnsafeLauncherValue(
+            f"{name} {value!r} cannot go into the launcher: a batch file cannot "
+            "hold a '\"', a '%' or a line break."
+        )
+    profile = os.environ.get("USERPROFILE", "")
+    lowered = value.lower()
+    if (
+        not value.isascii()
+        and profile
+        and (lowered == profile.lower() or lowered.startswith(profile.lower() + "\\"))
+    ):
+        value = "%USERPROFILE%" + value[len(profile) :]
+    if not value.isascii():
+        raise UnsafeLauncherValue(
+            f"{name} {value!r} cannot go into the launcher: outside the user "
+            "profile, a batch file can only hold ASCII characters. Set it in "
+            "the environment that runs the package manager instead."
+        )
+    return value
+
+
+def launcher_arg(name: str, value: str) -> str:
     """Return *value* quoted as one argument of a launcher command."""
     if is_windows():
-        return f'"{value}"'
+        return f'"{batch_value(name, value)}"'
     return shlex.quote(value)
 
 
@@ -43,23 +82,45 @@ def helper_command(helper: str) -> str:
     ``cloudsmith``, so the launcher runs the absolute executable.
     """
     if getattr(sys, "frozen", False):
-        return f"{launcher_arg(sys.executable)} credential-helper {helper}"
+        executable = launcher_arg("cloudsmith", sys.executable)
+        return f"{executable} credential-helper {helper}"
     return f"cloudsmith credential-helper {helper}"
 
 
-def _launcher_content(target_cmd: str, *, windows: bool) -> str:
+def _launcher_content(
+    target_cmd: str, *, windows: bool, env: dict[str, str] | None = None
+) -> str:
     """Return the launcher script body for the platform.
 
     Windows uses a ``.cmd`` batch file (``@echo off`` keeps stdout clean for
     Docker's credential JSON); Unix uses a ``#!/bin/sh`` script that ``exec``s
-    the target so signals and the exit code pass straight through.
+    the target so signals and the exit code pass straight through.  The
+    ``env`` variables replace the caller's values, so the helper gives the
+    same result as the install.
     """
+    env = env or {}
     if windows:
-        return f"@echo off\r\n{target_cmd} %*\r\n"
-    return f'#!/bin/sh\nexec {target_cmd} "$@"\n'
+        settings = "".join(
+            f'set "{name}={batch_value(name, value)}"\r\n'
+            for name, value in env.items()
+        )
+        if settings:
+            # Keep "!" literal and the settings local to the launcher.
+            settings = f"setlocal DisableDelayedExpansion\r\n{settings}"
+        return f"@echo off\r\n{settings}{target_cmd} %*\r\n"
+    settings = "".join(
+        f"export {name}={shlex.quote(value)}\n" for name, value in env.items()
+    )
+    return f'#!/bin/sh\n{settings}exec {target_cmd} "$@"\n'
 
 
-def write_launcher(bin_dir: Path, name: str, target_cmd: str, dry_run=False) -> Path:
+def write_launcher(
+    bin_dir: Path,
+    name: str,
+    target_cmd: str,
+    dry_run=False,
+    env: dict[str, str] | None = None,
+) -> Path:
     """Write a launcher script for *name* in *bin_dir* that execs *target_cmd*.
 
     Parameters
@@ -71,13 +132,16 @@ def write_launcher(bin_dir: Path, name: str, target_cmd: str, dry_run=False) ->
     target_cmd:
         The command the launcher forwards to (e.g.
         ``cloudsmith credential-helper docker``).
+    env:
+        Environment variables that the launcher sets.
 
     Returns
     -------
     Path
         The path of the written file.
     """
     windows = is_windows()
+    content = _launcher_content(target_cmd, windows=windows, env=env)
     if dry_run:
         if windows:
             return bin_dir / f"{name}.cmd"
@@ -87,9 +151,7 @@ def write_launcher(bin_dir: Path, name: str, target_cmd: str, dry_run=False) ->
     bin_dir.mkdir(parents=True, exist_ok=True)
 
     dest = bin_dir / _launcher_filename(name, windows=windows)
-    dest.write_text(
-        _launcher_content(target_cmd, windows=windows), encoding="utf-8", newline=""
-    )
+    dest.write_text(content, encoding="utf-8", newline="")
     if not windows:
         dest.chmod(0o755)