diff --git a/cloudsmith_cli/cli/commands/credential_helper/manage.py b/cloudsmith_cli/cli/commands/credential_helper/manage.py index 60697fda..34475753 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/manage.py +++ b/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,6 +359,9 @@ 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 ( @@ -318,6 +369,7 @@ def install_cmd( TerraformrcConflictError, TerraformHelperExeNotFound, TerraformPluginDirError, + UnsafeLauncherValue, ) as exc: raise click.ClickException(str(exc)) except OSError as exc: diff --git a/cloudsmith_cli/cli/tests/commands/test_credential_helper_install.py b/cloudsmith_cli/cli/tests/commands/test_credential_helper_install.py index 9264fb87..2c584f58 100644 --- a/cloudsmith_cli/cli/tests/commands/test_credential_helper_install.py +++ b/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 # --------------------------------------------------------------------------- diff --git a/cloudsmith_cli/credential_helpers/launchers.py b/cloudsmith_cli/credential_helpers/launchers.py index 8086e6f2..ceabd26d 100644 --- a/cloudsmith_cli/credential_helpers/launchers.py +++ b/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,6 +132,8 @@ 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 ------- @@ -78,6 +141,7 @@ def write_launcher(bin_dir: Path, name: str, target_cmd: str, dry_run=False) -> 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)