diff --git a/CHANGELOG.md b/CHANGELOG.md index 44875f45..86b27702 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Changed - 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. +- `cloudsmith credential-helper install docker` now keeps the `CLOUDSMITH_*` settings of the install, such as the profile, Workspace and API host, in the launcher. Docker then gets the same credentials from Docker Desktop, an IDE or a new shell. The launcher never contains API keys, custom headers or the proxy URL. An install with a profile or a Workspace writes its own helper, for example `docker-credential-cloudsmith--`. To remove it, run `cloudsmith credential-helper uninstall docker -P -w `. An install without them writes `docker-credential-cloudsmith`, as before. - Using `#!/usr/bin/env sh` as shebang path for unix credential helpers. - The minimum version of `cloudsmith-api` is now 2.0.42. @@ -91,6 +92,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - `cloudsmith mcp start` now stops without an error when the MCP client closes the connection while the server writes a reply. Previously, the command stopped with `BrokenPipeError` and exit status 1. - The CLI now exits with status 1 when you interrupt it with Ctrl+C after the reader of its stderr has closed the pipe. Before, the CLI sent an error report and exited with status 120. - `cloudsmith check rates` no longer stops with `TypeError` when the API does not give a value for an interval, limit, remaining count, reset time or throttled status. The command now shows `None` for that value. +- A `-P` flag before the command, for example `cloudsmith -P meta whoami`, now overrides `CLOUDSMITH_PROFILE`. Before, the environment variable took precedence. ## [1.30.0] - 2026-10-22 diff --git a/cloudsmith_cli/cli/commands/credential_helper/manage.py b/cloudsmith_cli/cli/commands/credential_helper/manage.py index e76fa75d..02ff71bf 100644 --- a/cloudsmith_cli/cli/commands/credential_helper/manage.py +++ b/cloudsmith_cli/cli/commands/credential_helper/manage.py @@ -38,8 +38,11 @@ ) from ....core.session import create_requests_session -from ....credential_helpers.docker.installer import DockerInstaller -from ... import utils +from ....credential_helpers.docker.installer import ( + DockerInstaller, + HelperScopeConflict, +) +from ... import deprecation, utils from ...decorators import ( common_api_auth_options, common_cli_config_options, @@ -186,8 +189,15 @@ 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") +# hold credentials. The colour settings are for the terminal of the install. +_NEVER_KEPT = ( + "api_key", + "api_headers", + "keyring_key", + "proxy", + "color", + "force_color_env", +) _TLS_ENVVARS = ("CLOUDSMITH_SSL_VERIFY", "CLOUDSMITH_WITHOUT_API_SSL_VERIFY") @@ -198,6 +208,24 @@ def _main_envvar(option) -> str: return names if isinstance(names, str) else names[0] +# A deprecated spelling is a separate option that sets the value of the option +# that replaces it. +_DEPRECATED_SPELLINGS = { + "org": deprecation.WORKSPACE_ALIASES, + "ssl_verify": deprecation.SSL_VERIFY_ALIASES, + "allowed_proxy_suffixes": deprecation.PROXY_SUFFIXES_ALIASES, +} + + +def _sources(context, option) -> set: + """Return the sources of the value of *option* and its deprecated spellings.""" + names = [option.name] + [ + deprecation.alias_param_name(spelling) + for spelling in _DEPRECATED_SPELLINGS.get(option.name, ()) + ] + return {context.get_parameter_source(name) for name in names} + + def _launcher_settings_from_cli(ctx, opts) -> dict[str, str]: """Return the ``CLOUDSMITH_*`` settings that a launcher keeps. @@ -215,11 +243,16 @@ def _launcher_settings_from_cli(ctx, opts) -> dict[str, str]: settings = {} for context in contexts: for option in context.command.params: - if option.name in _NEVER_KEPT or not option.envvar: + # The command removes the deprecated spellings from ctx.params. + if ( + option.name in _NEVER_KEPT + or not option.envvar + or option.name not in context.params + ): continue envvar = _main_envvar(option) - source = context.get_parameter_source(option.name) - if envvar.startswith("CLOUDSMITH_") and source in supplied: + sources = _sources(context, option) + if envvar.startswith("CLOUDSMITH_") and not sources.isdisjoint(supplied): value = context.params[option.name] settings[envvar] = ( str(value).lower() if isinstance(value, bool) else value @@ -228,9 +261,21 @@ def _launcher_settings_from_cli(ctx, opts) -> dict[str, str]: # 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() + # Keep the profile that the install used to load the config files. + if "CLOUDSMITH_PROFILE" in settings: + settings["CLOUDSMITH_PROFILE"] = contexts[-1].meta["profile"] return settings +def _profile_flag(ctx) -> str | None: + """Return the ``-P`` value of this command or of a parent command.""" + while ctx is not None: + if ctx.get_parameter_source("profile") == ParameterSource.COMMANDLINE: + return ctx.params["profile"] + ctx = ctx.parent + return None + + _REPO_FLAGS = ("-r", "--repo", "--repository") @@ -447,6 +492,7 @@ def install_cmd( ExtensionsFileError, ConfigFileConflict, UnsafeLauncherValue, + HelperScopeConflict, ) as exc: raise click.ClickException(str(exc)) except OSError as exc: @@ -520,11 +566,25 @@ def install_cmd( help="Maven and sbt only: remove the setup of the project in the current " "directory.", ) +@click.option( + "-w", + "--workspace", + "workspace", + help="Docker only: remove the helper that was installed for this Workspace.", +) +@deprecation.alias_options(deprecation.WORKSPACE_ALIASES) @common_cli_config_options @common_cli_output_options @click.pass_context def uninstall_cmd( - ctx, opts, helper: str, bin_dir: str | None, dry_run: bool, local: bool + ctx, + opts, + helper: str, + bin_dir: str | None, + dry_run: bool, + local: bool, + workspace: str | None, + **workspace_aliases, ) -> None: """Uninstall a credential helper launcher and remove its config entries. @@ -541,13 +601,27 @@ def uninstall_cmd( $ cloudsmith credential-helper uninstall docker --dry-run """ installer = _get_installer(helper) + alias_workspace = deprecation.pop_alias_options( + ctx, workspace_aliases, deprecation.WORKSPACE_ALIASES + ) + uninstall_kwargs = { + "bin_dir": bin_dir, + "dry_run": dry_run, + **_helper_options(ctx, helper), + } + # The scope comes only from flags, so that a shell with CLOUDSMITH_PROFILE + # or CLOUDSMITH_WORKSPACE can still remove the unscoped helper. + if helper == "docker": + uninstall_kwargs["profile"] = _profile_flag(ctx) + uninstall_kwargs["workspace"] = workspace or alias_workspace try: - actions = installer.uninstall( - bin_dir=bin_dir, - dry_run=dry_run, - **_helper_options(ctx, helper), - ) - except (ConfigFileParseError, ProjectNotFound, ConfigFileConflict) as exc: + actions = installer.uninstall(**uninstall_kwargs) + except ( + ConfigFileParseError, + ProjectNotFound, + ConfigFileConflict, + HelperScopeConflict, + ) as exc: raise click.ClickException(str(exc)) except OSError as exc: raise click.ClickException( @@ -599,14 +673,17 @@ def list_cmd(ctx, opts) -> None: data = [] for name, cls in sorted(_INSTALLERS.items()): installer = cls() - st = installer.status() - data.append( + # Docker can have one helper for each profile and Workspace. + statuses = getattr(installer, "statuses", None) + data.extend( { "helper": name, "summary": installer.summary, + "cred_helper": st.get("name"), "launcher": st.get("launcher"), "hosts": st.get("hosts", []), } + for st in (statuses() if statuses else [installer.status()]) ) if utils.maybe_print_as_json(opts, data): @@ -619,6 +696,8 @@ def list_cmd(ctx, opts) -> None: summary = entry["summary"] click.echo(f"{name} ({summary})", err=use_stderr) + if entry["cred_helper"]: + click.echo(f" helper : {entry['cred_helper']}", err=use_stderr) if launcher: click.echo(f" launcher : {launcher}", err=use_stderr) else: diff --git a/cloudsmith_cli/cli/decorators.py b/cloudsmith_cli/cli/decorators.py index 8f790458..0fe9aa61 100644 --- a/cloudsmith_cli/cli/decorators.py +++ b/cloudsmith_cli/cli/decorators.py @@ -231,7 +231,10 @@ def common_cli_config_options(f): def wrapper(ctx, *args, **kwargs): # pylint: disable=missing-docstring opts = config.get_or_create_options(ctx) - profile = kwargs.pop("profile") or ctx.meta.get("profile") + profile = kwargs.pop("profile") + # A -P flag of a parent command overrides CLOUDSMITH_PROFILE. + if ctx.get_parameter_source("profile") != ParameterSource.COMMANDLINE: + profile = ctx.meta.get("profile") or profile config_file = kwargs.pop("config_file") or ctx.meta.get("config_file") creds_file = kwargs.pop("credentials_file") or ctx.meta.get("creds_file") 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 41fcca7d..ea75ede8 100644 --- a/cloudsmith_cli/cli/tests/commands/test_credential_helper_install.py +++ b/cloudsmith_cli/cli/tests/commands/test_credential_helper_install.py @@ -5,6 +5,7 @@ import json import os +import shlex import stat import sys from pathlib import Path @@ -20,12 +21,17 @@ 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.docker.installer import ( + DockerInstaller, + HelperScopeConflict, + _check_scope, +) from ....credential_helpers.launchers import ( UnsafeLauncherValue, _launcher_content, _launcher_filename, is_on_path, + parse_launcher_env, remove_launcher, resolve_bin_dir, write_launcher, @@ -145,14 +151,20 @@ def test_windows_launcher_refuses_a_value_a_batch_file_cannot_hold(monkeypatch, _launcher_content("cmd", windows=True, env={"CLOUDSMITH_PROFILE": value}) +@pytest.mark.parametrize("windows", [False, True]) +def test_parse_launcher_env_reads_the_settings_of_a_launcher(windows): + env = {"CLOUDSMITH_PROFILE": "it's a b", "CLOUDSMITH_WORKSPACE": "x=y!"} + assert parse_launcher_env(_launcher_content("cmd", windows=windows, env=env)) == env + + 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. + file itself. Secrets, the proxy and the colour settings 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 @@ -190,6 +202,8 @@ def install(self, **kwargs): env={ "CLOUDSMITH_API_KEY": "k_secret", "CLOUDSMITH_API_HEADERS": "X-Secret=1", + "CLOUDSMITH_COLOR": "never", + "CLOUDSMITH_FORCE_COLOR": "1", "CLOUDSMITH_KEYRING_KEY": "keyring_secret", "CLOUDSMITH_KEYRING_DIR": "/var/keyring", "CLOUDSMITH_OIDC_AUDIENCE": "cloudsmith-test", @@ -353,6 +367,34 @@ def test_list_finds_a_launcher_installed_to_the_default_dir( ) +def test_list_shows_each_docker_helper_by_name(runner, tmp_path, monkeypatch): + """A scoped launcher stays in `list` after another install takes its hosts.""" + from ....cli.commands.credential_helper.manage import list_cmd + + monkeypatch.setattr(Path, "home", staticmethod(lambda: tmp_path)) + monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) + for workspace in ("acme", "beta"): + DockerInstaller().install( + discover=False, launcher_env={"CLOUDSMITH_WORKSPACE": workspace} + ) + + result = runner.invoke(list_cmd, ["-F", "json"], catch_exceptions=False) + + bin_dir = tmp_path / ".cloudsmith" / "bin" + assert [ + (entry["cred_helper"], entry["launcher"], entry["hosts"]) + for entry in json.loads(result.stdout)["data"] + if entry["helper"] == "docker" + ] == [ + ( + "cloudsmith-beta", + str(bin_dir / "docker-credential-cloudsmith-beta"), + ["docker.cloudsmith.io"], + ), + ("cloudsmith-acme", str(bin_dir / "docker-credential-cloudsmith-acme"), []), + ] + + # --------------------------------------------------------------------------- # 4. is_on_path # --------------------------------------------------------------------------- @@ -409,6 +451,334 @@ def test_docker_installer_install(tmp_path, monkeypatch): assert (bin_dir / "docker-credential-cloudsmith").exists() +def test_docker_install_scopes_the_helper_to_the_install_time_settings( + tmp_path, monkeypatch +): + """The profile and Workspace name the helper, so installs can coexist.""" + monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) + monkeypatch.delattr(sys, "frozen", raising=False) + bin_dir = tmp_path / "bin" + monkeypatch.setenv("PATH", str(bin_dir)) + installer = DockerInstaller() + installer.install( + bin_dir=str(bin_dir), domains=("legacy.example.com",), discover=False + ) + + actions = installer.install( + bin_dir=str(bin_dir), + discover=False, + launcher_env={ + "CLOUDSMITH_PROFILE": "Meta", + "CLOUDSMITH_WORKSPACE": "iduffy_demo", + "CLOUDSMITH_API_HOST": "https://api.example.com", + }, + ) + + name = "cloudsmith-meta-iduffy-demo" + assert (bin_dir / f"docker-credential-{name}").read_text() == ( + "#!/usr/bin/env sh\n" + "export CLOUDSMITH_PROFILE=Meta\n" + "export CLOUDSMITH_WORKSPACE=iduffy_demo\n" + "export CLOUDSMITH_API_HOST=https://api.example.com\n" + 'exec cloudsmith credential-helper docker "$@"\n' + ) + cfg = json.loads((tmp_path / ".docker" / "config.json").read_text()) + assert cfg["credHelpers"] == { + "docker.cloudsmith.io": name, + "legacy.example.com": "cloudsmith", + } + assert ( + "WARNING: credHelpers['docker.cloudsmith.io'] changes from 'cloudsmith'" + f" to {name!r}" + ) in actions + with patch( + "cloudsmith_cli.credential_helpers.docker.installer.resolve_bin_dir", + return_value=bin_dir, + ): + assert installer.statuses() == [ + { + "name": name, + "launcher": str(bin_dir / f"docker-credential-{name}"), + "hosts": ["docker.cloudsmith.io"], + }, + { + "name": "cloudsmith", + "launcher": str(bin_dir / "docker-credential-cloudsmith"), + "hosts": ["legacy.example.com"], + }, + ] + + +def test_docker_launcher_keeps_the_resolved_tls_setting(runner, tmp_path, monkeypatch): + """-S overrides CLOUDSMITH_SSL_VERIFY=1, so the launcher turns verification off.""" + from ....cli.commands.credential_helper.manage import install_cmd + + monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) + monkeypatch.delattr(sys, "frozen", raising=False) + bin_dir = tmp_path / "bin" + + result = runner.invoke( + install_cmd, + ["docker", "--bin-dir", str(bin_dir), "--no-discover", "-S"], + env={"CLOUDSMITH_SSL_VERIFY": "1"}, + ) + + assert result.exit_code == 0, result.output + assert (bin_dir / "docker-credential-cloudsmith").read_text() == ( + "#!/usr/bin/env sh\n" + "export CLOUDSMITH_SSL_VERIFY=false\n" + 'exec cloudsmith credential-helper docker "$@"\n' + ) + + +def test_docker_launcher_keeps_the_settings_of_deprecated_spellings( + runner, tmp_path, monkeypatch +): + """--org and CLOUDSMITH_WITHOUT_API_SSL_VERIFY set the same values as -w and -S.""" + from ....cli.commands.credential_helper.manage import install_cmd + + monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) + monkeypatch.delattr(sys, "frozen", raising=False) + bin_dir = tmp_path / "bin" + + result = runner.invoke( + install_cmd, + ["docker", "--bin-dir", str(bin_dir), "--no-discover", "--org", "acme"], + env={"CLOUDSMITH_WITHOUT_API_SSL_VERIFY": "1"}, + ) + + assert result.exit_code == 0, result.output + assert (bin_dir / "docker-credential-cloudsmith-acme").read_text() == ( + "#!/usr/bin/env sh\n" + "export CLOUDSMITH_WORKSPACE=acme\n" + "export CLOUDSMITH_SSL_VERIFY=false\n" + 'exec cloudsmith credential-helper docker "$@"\n' + ) + + +def test_docker_dry_run_shows_the_scoped_launcher_and_its_settings( + tmp_path, monkeypatch +): + monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) + bin_dir = tmp_path / "bin" + + actions = DockerInstaller().install( + bin_dir=str(bin_dir), + discover=False, + dry_run=True, + launcher_env={"CLOUDSMITH_WORKSPACE": "acme", "CLOUDSMITH_NO_KEYRING": "1"}, + ) + + assert actions[:2] == [ + f"would write launcher {bin_dir / 'docker-credential-cloudsmith-acme'}", + "launcher sets CLOUDSMITH_WORKSPACE, CLOUDSMITH_NO_KEYRING", + ] + assert not bin_dir.exists() + + +def test_docker_uninstall_takes_the_scope_from_flags_only( + runner, tmp_path, monkeypatch +): + """Without flags, uninstall removes the unscoped helper, as before. + + A -P flag on the root command also counts. + """ + from ....cli.commands.credential_helper.manage import install_cmd, uninstall_cmd + from ....cli.commands.main import main + + monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) + bin_dir = tmp_path / "bin" + DockerInstaller().install(bin_dir=str(bin_dir), domains=("legacy.example.com",)) + args = ["docker", "--bin-dir", str(bin_dir)] + env = {"CLOUDSMITH_PROFILE": "work", "CLOUDSMITH_WORKSPACE": "acme"} + installed = runner.invoke( + install_cmd, [*args, "--domain", "acme.example.com", "--no-discover"], env=env + ) + + unscoped = runner.invoke(uninstall_cmd, args, env=env) + scoped_launchers = sorted(path.name for path in bin_dir.iterdir()) + scoped = runner.invoke( + main, ["-P", "work", "credential-helper", "uninstall", *args, "-o", "acme"] + ) + + assert [installed.exit_code, unscoped.exit_code, scoped.exit_code] == [0, 0, 0] + assert scoped_launchers == ["docker-credential-cloudsmith-work-acme"] + assert not list(bin_dir.iterdir()) + cfg = json.loads((tmp_path / ".docker" / "config.json").read_text()) + assert "credHelpers" not in cfg + + +def test_docker_refuses_the_helper_of_another_install(runner, tmp_path, monkeypatch): + """-P acme and -w acme give the same helper name.""" + from ....cli.commands.credential_helper.manage import uninstall_cmd + + monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) + bin_dir = tmp_path / "bin" + installer = DockerInstaller() + installer.install( + bin_dir=str(bin_dir), + discover=False, + launcher_env={"CLOUDSMITH_PROFILE": "acme"}, + ) + launcher = bin_dir / "docker-credential-cloudsmith-acme" + content = launcher.read_text() + + with pytest.raises(HelperScopeConflict, match="docker -P acme --bin-dir "): + installer.install( + bin_dir=str(bin_dir), + discover=False, + launcher_env={"CLOUDSMITH_WORKSPACE": "acme"}, + ) + result = runner.invoke( + uninstall_cmd, ["docker", "--bin-dir", str(bin_dir), "-w", "acme"] + ) + + assert result.exit_code == 1 + assert "uninstall docker -P acme" in result.output + assert launcher.read_text() == content + + +def test_docker_reinstalls_a_launcher_with_a_line_break_in_a_setting( + tmp_path, monkeypatch +): + """A quoted value in a Unix launcher can hold a line break.""" + monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) + bin_dir = tmp_path / "bin" + launcher_env = { + "CLOUDSMITH_CONFIG_FILE": str(tmp_path / "a\nb"), + "CLOUDSMITH_WORKSPACE": "acme", + } + installer = DockerInstaller() + installer.install(bin_dir=str(bin_dir), discover=False, launcher_env=launcher_env) + + installer.install(bin_dir=str(bin_dir), discover=False, launcher_env=launcher_env) + installer.uninstall(bin_dir=str(bin_dir), workspace="acme") + + assert not list(bin_dir.iterdir()) + + +def test_docker_uninstall_removes_a_launcher_with_an_empty_profile( + tmp_path, monkeypatch +): + """An empty CLOUDSMITH_PROFILE gives the helper of an install without a profile.""" + monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) + bin_dir = tmp_path / "bin" + installer = DockerInstaller() + installer.install( + bin_dir=str(bin_dir), discover=False, launcher_env={"CLOUDSMITH_PROFILE": ""} + ) + + installer.uninstall(bin_dir=str(bin_dir)) + + assert not list(bin_dir.iterdir()) + + +def test_docker_conflict_command_removes_the_conflicting_launcher( + runner, tmp_path, monkeypatch +): + """The command in the conflict message names the --bin-dir of the launcher.""" + from ....cli.commands.main import main + + monkeypatch.setenv("HOME", str(tmp_path / "home")) + monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) + bin_dir = tmp_path / "bin" + installer = DockerInstaller() + installer.install( + bin_dir=str(bin_dir), + discover=False, + launcher_env={"CLOUDSMITH_PROFILE": "acme"}, + ) + with pytest.raises(HelperScopeConflict) as conflict: + installer.install( + bin_dir=str(bin_dir), + discover=False, + launcher_env={"CLOUDSMITH_WORKSPACE": "acme"}, + ) + + command = shlex.split(str(conflict.value).partition("run: ")[2]) + result = runner.invoke(main, command[1:]) + + assert result.exit_code == 0, result.output + assert not list(bin_dir.iterdir()) + + +def test_docker_conflict_command_uses_windows_quoting(tmp_path, monkeypatch): + """cmd.exe reads only double quotes, and splits a command at an unquoted &.""" + monkeypatch.setattr( + "cloudsmith_cli.credential_helpers.docker.installer.is_windows", lambda: True + ) + bin_dir = tmp_path / "my bin&tools" + bin_dir.mkdir() + launcher = bin_dir / "docker-credential-cloudsmith-work-team.cmd" + launcher.write_text( + _launcher_content("cmd", windows=True, env={"CLOUDSMITH_PROFILE": "work&team"}) + ) + + with pytest.raises(HelperScopeConflict) as conflict: + _check_scope(launcher, (None, "acme")) + + assert str(conflict.value).endswith(f' -P "work&team" --bin-dir "{bin_dir}"') + + +def test_docker_names_a_launcher_that_it_cannot_read(tmp_path): + """A launcher with an unclosed quote stops the install with its path.""" + launcher = tmp_path / "docker-credential-cloudsmith-acme" + launcher.write_text("#!/bin/sh\nexport CLOUDSMITH_WORKSPACE='acme\n") + + with pytest.raises(HelperScopeConflict) as conflict: + _check_scope(launcher, (None, "acme")) + + assert str(launcher) in str(conflict.value) + + +def test_docker_root_profile_flag_overrides_the_environment( + runner, tmp_path, monkeypatch +): + """A -P flag on the root command overrides CLOUDSMITH_PROFILE.""" + from ....cli.commands.main import main + + monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) + bin_dir = tmp_path / "bin" + args = ["docker", "--bin-dir", str(bin_dir), "-w", "acme"] + env = {"CLOUDSMITH_PROFILE": "other"} + + installed = runner.invoke( + main, + ["-P", "work", "credential-helper", "install", *args, "--no-discover"], + env=env, + ) + launchers = sorted(path.name for path in bin_dir.iterdir()) + uninstalled = runner.invoke( + main, ["-P", "work", "credential-helper", "uninstall", *args], env=env + ) + + assert installed.exit_code == 0, installed.output + assert uninstalled.exit_code == 0, uninstalled.output + assert launchers == ["docker-credential-cloudsmith-work-acme"] + assert not list(bin_dir.iterdir()) + + +def test_docker_uninstall_accepts_the_oidc_org_alias(runner, tmp_path, monkeypatch): + """--oidc-org works, but warns that it is deprecated.""" + from ....cli.commands.credential_helper.manage import uninstall_cmd + + monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) + bin_dir = tmp_path / "bin" + DockerInstaller().install( + bin_dir=str(bin_dir), + discover=False, + launcher_env={"CLOUDSMITH_WORKSPACE": "acme"}, + ) + + result = runner.invoke( + uninstall_cmd, ["docker", "--bin-dir", str(bin_dir), "--oidc-org", "acme"] + ) + + assert result.exit_code == 0, result.output + assert not list(bin_dir.iterdir()) + assert "`--oidc-org` is deprecated" in result.stderr + + def test_pnpm_installer_install(tmp_path: Path, monkeypatch: MonkeyPatch): """install sets default+extra domains, preserves foreign entries, writes the launcher.""" npm_path = tmp_path / ".npmrc" @@ -738,7 +1108,7 @@ def test_pnpm_uninstall_removes_launcher_without_npmrc( def test_docker_installer_status_type_contract(tmp_path, monkeypatch): - """status()['launcher'] is str when installed and None when not — never a Path. + """statuses()[0]['launcher'] is a str when installed, else None, never a Path. Retained guard: the -F json Path-serialization regression. """ @@ -753,7 +1123,7 @@ def test_docker_installer_status_type_contract(tmp_path, monkeypatch): "cloudsmith_cli.credential_helpers.docker.installer.resolve_bin_dir", return_value=bin_dir, ): - result_before = installer.status() + [result_before] = installer.statuses() assert result_before["launcher"] is None assert not isinstance(result_before["launcher"], Path) @@ -764,7 +1134,7 @@ def test_docker_installer_status_type_contract(tmp_path, monkeypatch): "cloudsmith_cli.credential_helpers.docker.installer.resolve_bin_dir", return_value=bin_dir, ): - result_after = installer.status() + [result_after] = installer.statuses() launcher = result_after["launcher"] assert launcher is not None @@ -1300,11 +1670,13 @@ def test_output_format_json( Retained guard: list -F json serialises a launcher path (str), not a Path object. """ - def _docker_stub_status_fn(_self): - return { - "launcher": "/some/bin/docker-credential-cloudsmith", - "hosts": ["docker.cloudsmith.io"], - } + def _docker_stub_statuses_fn(_self): + return [ + { + "launcher": "/some/bin/docker-credential-cloudsmith", + "hosts": ["docker.cloudsmith.io"], + } + ] def _pnpm_stub_status_fn(_self): return { @@ -1314,7 +1686,7 @@ def _pnpm_stub_status_fn(_self): monkeypatch.setenv("DOCKER_CONFIG", str(tmp_path / ".docker")) monkeypatch.setenv("NPM_CONFIG_USERCONFIG", str(tmp_path / ".npmrc")) - monkeypatch.setattr(DockerInstaller, "status", _docker_stub_status_fn) + monkeypatch.setattr(DockerInstaller, "statuses", _docker_stub_statuses_fn) monkeypatch.setattr(PNPMInstaller, "status", _pnpm_stub_status_fn) from ....cli.commands.credential_helper import manage as manage_mod diff --git a/cloudsmith_cli/credential_helpers/docker/installer.py b/cloudsmith_cli/credential_helpers/docker/installer.py index 75884ee6..02e09371 100644 --- a/cloudsmith_cli/credential_helpers/docker/installer.py +++ b/cloudsmith_cli/credential_helpers/docker/installer.py @@ -1,7 +1,7 @@ # Copyright 2026 Cloudsmith Ltd """Installer for the Docker credential helper. -Manages writing/removing the ``docker-credential-cloudsmith`` launcher and +Manages writing/removing the ``docker-credential-cloudsmith*`` launchers and patching ``~/.docker/config.json`` to enable the helper for Cloudsmith registry hosts. """ @@ -11,6 +11,8 @@ import json import logging import os +import re +import shlex from pathlib import Path from typing import TYPE_CHECKING @@ -22,6 +24,7 @@ helper_command, is_on_path, is_windows, + parse_launcher_env, remove_launcher, resolve_bin_dir, write_launcher, @@ -45,10 +48,100 @@ def _docker_config_path() -> Path: return Path.home() / ".docker" / "config.json" +# The profile and the Workspace of an install name its helper. +_SCOPE_ENVVARS = ("CLOUDSMITH_PROFILE", "CLOUDSMITH_WORKSPACE") +_SCOPE_FLAGS = ("-P", "-w") + + +class HelperScopeConflict(ValueError): + """Raised when the launcher of another profile or Workspace has the name.""" + + +def _helper_name(scope: tuple[str | None, ...]) -> str: + """Return the ``credHelpers`` value for an install with this scope. + + Each profile and Workspace gets its own launcher, so that installs for + different configurations can coexist. + """ + segments = [ + re.sub(r"[^a-z0-9-]+", "-", value.lower()).strip("-") + for value in scope + if value + ] + return "-".join(["cloudsmith", *filter(None, segments)]) + + +def _shell_quote(value: str) -> str: + """Return *value* quoted as one argument for the shell of the user.""" + if not is_windows(): + return shlex.quote(value) + # cmd.exe reads & | < > ^ as text only in double quotes. A backslash + # before the closing quote must be doubled, or it escapes the quote. + trailing_backslashes = "\\" * (len(value) - len(value.rstrip("\\"))) + return f'"{value}{trailing_backslashes}"' + + +def _check_scope(launcher_path: Path, scope: tuple[str | None, ...]) -> None: + """Raise :class:`HelperScopeConflict` if another scope wrote the launcher. + + Different scopes can give the same name, for example ``-P acme`` and + ``-w acme``. + """ + if not launcher_path.exists(): + return + try: + env = parse_launcher_env(launcher_path.read_text(encoding="utf-8")) + except ValueError as exc: + raise HelperScopeConflict( + f"Cannot read the settings in {launcher_path} ({exc}). Delete the" + " file, then run the command again." + ) from exc + # An empty value gives the same helper name as no value. + installed = tuple(env.get(name) or None for name in _SCOPE_ENVVARS) + if installed != tuple(value or None for value in scope): + flags = "".join( + f" {flag} {_shell_quote(value)}" + for flag, value in zip(_SCOPE_FLAGS, installed) + if value + ) + raise HelperScopeConflict( + f"{launcher_path} is the helper of an install with a different" + " profile or Workspace. To remove it, run: cloudsmith" + f" credential-helper uninstall docker{flags}" + f" --bin-dir {_shell_quote(str(launcher_path.parent))}" + ) + + +def _cred_helpers() -> dict: + """Return the ``credHelpers`` block of ``config.json``, or ``{}``.""" + config_path = _docker_config_path() + if not config_path.exists(): + return {} + try: + data = json.loads(config_path.read_text(encoding="utf-8")) + except (ValueError, OSError): + return {} + cred_helpers = data.get("credHelpers") if isinstance(data, dict) else None + return cred_helpers if isinstance(cred_helpers, dict) else {} + + +def _is_cloudsmith_helper(value) -> bool: + return value == "cloudsmith" or str(value).startswith("cloudsmith-") + + +def _launcher_name(helper: str) -> str: + return f"docker-credential-{helper}" + + +def _launcher_path(target_dir: Path, helper: str) -> Path: + name = _launcher_name(helper) + return target_dir / (f"{name}.cmd" if is_windows() else name) + + class DockerInstaller: """Manages installation of the Docker credential helper for Cloudsmith. - This installer writes a ``docker-credential-cloudsmith`` launcher binary + This installer writes a ``docker-credential-cloudsmith*`` launcher binary and patches ``~/.docker/config.json`` to route the configured registry hosts through the Cloudsmith credential helper. @@ -60,9 +153,10 @@ class DockerInstaller: print(action) """ - LAUNCHER_NAME = "docker-credential-cloudsmith" - HELPER_VALUE = "cloudsmith" DEFAULT_HOST = "docker.cloudsmith.io" + # Docker runs the helper from Docker Desktop, IDEs and other shells, so + # the launcher keeps the install-time settings. + KEEPS_INSTALL_TIME_SETTINGS = True name = "docker" summary = "Docker credential helper for Cloudsmith registries" @@ -77,6 +171,7 @@ def install( org: str | None = None, credential: CredentialResult | None = None, api_host: str | None = None, + launcher_env: dict[str, str] | None = None, dry_run: bool = False, ) -> list[str]: """Install the Docker credential helper. @@ -105,6 +200,9 @@ def install( Resolved credential used for custom-domain discovery. api_host: Cloudsmith API host URL override. + launcher_env: + ``CLOUDSMITH_*`` settings that the launcher sets. The profile and + the Workspace in them name the helper. dry_run: When ``True``, compute and return planned actions without writing any files. @@ -117,6 +215,11 @@ def install( """ target_dir = resolve_bin_dir(bin_dir) config_path = _docker_config_path() + launcher_env = launcher_env or {} + scope = tuple(launcher_env.get(name) for name in _SCOPE_ENVVARS) + helper = _helper_name(scope) + launcher_name = _launcher_name(helper) + _check_scope(_launcher_path(target_dir, helper), scope) actions: list[str] = [] @@ -179,58 +282,83 @@ def install( deduped.append(h) hosts = deduped + replaced: list[str] = [] + def mutate(config: dict) -> None: helpers = config.get("credHelpers") if not isinstance(helpers, dict): helpers = config["credHelpers"] = {} for host in hosts: - helpers[host] = self.HELPER_VALUE + current = helpers.get(host) + if current != helper and _is_cloudsmith_helper(current): + replaced.append( + f"WARNING: credHelpers[{host!r}] changes from {current!r}" + f" to {helper!r}" + ) + helpers[host] = helper + + launcher_settings = ( + [f"launcher sets {', '.join(launcher_env)}"] if launcher_env else [] + ) if dry_run: - if os.name == "nt": - launcher_path = target_dir / f"{self.LAUNCHER_NAME}.cmd" - else: - launcher_path = target_dir / self.LAUNCHER_NAME + launcher_path = write_launcher( + target_dir, + launcher_name, + helper_command(self.name), + dry_run=True, + env=launcher_env, + ) actions.append(f"would write launcher {launcher_path}") + actions.extend(launcher_settings) would_change = merge_config_file(config_path, mutate, dry_run=True) for host in hosts: if would_change: actions.append( - f"would set credHelpers[{host!r}]={self.HELPER_VALUE!r}" - f" in {config_path}" + f"would set credHelpers[{host!r}]={helper!r} in {config_path}" ) else: actions.append( f"credHelpers[{host!r}] already set" f" in {config_path} (no change)" ) - return actions + return actions + replaced # Real install + # The caller resolves -S and CLOUDSMITH_SSL_VERIFY into one + # CLOUDSMITH_SSL_VERIFY value, so the launcher keeps the TLS setting + # of the install. launcher_path = write_launcher( - target_dir, self.LAUNCHER_NAME, helper_command(self.name) + target_dir, launcher_name, helper_command(self.name), env=launcher_env ) actions.append(f"wrote launcher {launcher_path}") + actions.extend(launcher_settings) changed = merge_config_file(config_path, mutate) if changed: actions.extend( - f"set credHelpers[{host!r}]={self.HELPER_VALUE!r} in {config_path}" + f"set credHelpers[{host!r}]={helper!r} in {config_path}" for host in hosts ) else: actions.append(f"config.json already up to date ({config_path})") + actions.extend(replaced) if not is_on_path(target_dir): actions.append( - path_warning(target_dir, "Docker", self.LAUNCHER_NAME, is_windows()) + path_warning(target_dir, "Docker", launcher_name, is_windows()) ) return actions def uninstall( - self, *, bin_dir: str | None = None, dry_run: bool = False + self, + *, + bin_dir: str | None = None, + profile: str | None = None, + workspace: str | None = None, + dry_run: bool = False, ) -> list[str]: """Uninstall the Docker credential helper. @@ -244,6 +372,9 @@ def uninstall( Defaults to :func:`resolve_bin_dir` auto-detection. Pass the same value that was given to :meth:`install` so the correct launcher file is found and removed. + profile, workspace: + The settings of the install to remove. Without them, the helper + of an install without them is removed. dry_run: When ``True``, return planned actions without writing any files. @@ -256,23 +387,23 @@ def uninstall( config_path = _docker_config_path() # A missing config has no entries. A merge would create an empty file. config_exists: bool = config_path.exists() + scope = (profile, workspace) + helper = _helper_name(scope) + launcher_name = _launcher_name(helper) + _check_scope(_launcher_path(target_dir, helper), scope) def mutate(config: dict[str, object]) -> None: helpers = config.get("credHelpers") if not isinstance(helpers, dict): return - removed = [k for k, v in helpers.items() if v == self.HELPER_VALUE] + removed = [k for k, v in helpers.items() if v == helper] for key in removed: del helpers[key] if removed and not helpers: del config["credHelpers"] actions: list[str] = [] - - if os.name == "nt": - launcher_path = target_dir / f"{self.LAUNCHER_NAME}.cmd" - else: - launcher_path = target_dir / self.LAUNCHER_NAME + launcher_path = _launcher_path(target_dir, helper) if dry_run: if launcher_path.exists(): @@ -288,14 +419,14 @@ def mutate(config: dict[str, object]) -> None: if would_change: actions.append( f"would remove credHelpers entries with value" - f" {self.HELPER_VALUE!r} from {config_path}" + f" {helper!r} from {config_path}" ) else: actions.append(f"no credHelpers entries to remove from {config_path}") return actions # Real uninstall - removed = remove_launcher(target_dir, self.LAUNCHER_NAME) + removed = remove_launcher(target_dir, launcher_name) if removed: actions.append(f"removed launcher {launcher_path}") else: @@ -304,53 +435,53 @@ def mutate(config: dict[str, object]) -> None: changed = config_exists and merge_config_file(config_path, mutate) if changed: actions.append( - f"removed credHelpers entries with value" - f" {self.HELPER_VALUE!r} from {config_path}" + f"removed credHelpers entries with value {helper!r} from {config_path}" ) else: actions.append(f"no credHelpers entries to remove from {config_path}") return actions - def status(self) -> dict[str, str | list[str] | None]: - """Return current installation status. + def statuses(self) -> list[dict]: + """Return the status of each Cloudsmith helper. Returns ------- - dict - A dict with keys: + list[dict] + One dict for each ``cloudsmith`` or ``cloudsmith-*`` helper, in the + order of the ``credHelpers`` block, then one for each other scoped + launcher in the default bin directory, with keys: + ``"name"`` + The ``credHelpers`` value of the helper. ``"launcher"`` - The :class:`~pathlib.Path` of the launcher if it exists, - else ``None``. + The path of the launcher of the helper, or ``None`` if it does + not exist. ``"hosts"`` - List of hostnames in ``config.json``'s ``credHelpers`` block - whose value equals ``"cloudsmith"``. - """ - target_dir = resolve_bin_dir() - if os.name == "nt": - launcher_path: Path | None = target_dir / f"{self.LAUNCHER_NAME}.cmd" - else: - launcher_path = target_dir / self.LAUNCHER_NAME + List of hostnames that use the helper. - if launcher_path is not None and not launcher_path.exists(): - launcher_path = None + Without such a helper, the list has only the unscoped helper. + """ + hosts_by_helper: dict[str, list[str]] = {} + for host, helper in _cred_helpers().items(): + if _is_cloudsmith_helper(helper): + hosts_by_helper.setdefault(helper, []).append(host) - config_path = _docker_config_path() - hosts: list[str] = [] - if config_path.exists(): - try: - data = json.loads(config_path.read_text(encoding="utf-8")) - if isinstance(data, dict): - helpers = data.get("credHelpers", {}) - if isinstance(helpers, dict): - hosts = [ - k for k, v in helpers.items() if v == self.HELPER_VALUE - ] - except (ValueError, OSError): - pass - - return { - "launcher": str(launcher_path) if launcher_path is not None else None, - "hosts": hosts, - } + target_dir = resolve_bin_dir() + # A scoped launcher stays when a later install moves all of its hosts. + pattern = _launcher_path(target_dir, "cloudsmith-*") + for launcher_path in sorted(target_dir.glob(pattern.name)): + name = launcher_path.name.removesuffix(pattern.suffix) + hosts_by_helper.setdefault(name.removeprefix(_launcher_name("")), []) + + statuses = [] + for helper, hosts in (hosts_by_helper or {"cloudsmith": []}).items(): + launcher_path = _launcher_path(target_dir, helper) + statuses.append( + { + "name": helper, + "launcher": str(launcher_path) if launcher_path.exists() else None, + "hosts": hosts, + } + ) + return statuses diff --git a/cloudsmith_cli/credential_helpers/launchers.py b/cloudsmith_cli/credential_helpers/launchers.py index a3fc42da..870ea9e3 100644 --- a/cloudsmith_cli/credential_helpers/launchers.py +++ b/cloudsmith_cli/credential_helpers/launchers.py @@ -16,6 +16,7 @@ import os import shlex import sys +from itertools import pairwise from pathlib import Path @@ -114,6 +115,25 @@ def _launcher_content( return f'#!/usr/bin/env sh\n{settings}exec {target_cmd} "$@"\n' +def parse_launcher_env(content: str) -> dict[str, str]: + """Return the environment variables that a launcher body sets.""" + if content.startswith("@echo off"): + settings = [ + line[len('set "') : -1] + for line in content.splitlines() + if line.startswith('set "') + ] + else: + # A quoted value can hold a line break, so split the whole script. + words = shlex.split(content) + settings = [word for prev, word in pairwise(words) if prev == "export"] + env = {} + for setting in settings: + name, _, value = setting.partition("=") + env[name] = value + return env + + def write_launcher( bin_dir: Path, name: str,