diff --git a/CHANGELOG.md b/CHANGELOG.md index dbf15c7b..d3da6d79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed +- The package metadata commands no longer wait without a limit for a v2 API that does not respond. A connection that takes more than 30 seconds fails. A request that gets no data for 30 seconds also fails, except a request to add or validate metadata. The CLI then retries the request as `--error-retry-max` sets. +- The CLI no longer sends the API key to another host when a request is redirected there. This applies to all API calls, uploads and downloads. + ## [1.28.0] - 2026-10-07 ### Added diff --git a/cloudsmith_cli/cli/commands/metadata.py b/cloudsmith_cli/cli/commands/metadata.py index 7f604044..1d91f0d3 100644 --- a/cloudsmith_cli/cli/commands/metadata.py +++ b/cloudsmith_cli/cli/commands/metadata.py @@ -183,7 +183,9 @@ def list_metadata( slug_perm = api_get_package_slug_perm( owner=owner, repo=repo, identifier=package ) - entry = api_get_metadata(slug_perm, metadata_slug_perm) + entry = api_get_metadata( + slug_perm, metadata_slug_perm, credential=opts.credential + ) click.secho("OK", fg="green", err=use_stderr) _print_metadata_entry(opts, entry) @@ -208,6 +210,7 @@ def list_metadata( page=page, page_size=page_size, package_slug_perm=slug_perm, + credential=opts.credential, source_kind=source_kind, classification=classification, ) @@ -328,6 +331,7 @@ def add_metadata( ) entry = api_create_metadata( slug_perm, + credential=opts.credential, content=metadata.content, content_type=metadata.content_type, source_identity=metadata.source_identity, @@ -434,7 +438,9 @@ def update_metadata( slug_perm = api_get_package_slug_perm( owner=owner, repo=repo, identifier=package ) - entry = api_update_metadata(slug_perm, metadata_slug_perm, **patch_kwargs) + entry = api_update_metadata( + slug_perm, metadata_slug_perm, credential=opts.credential, **patch_kwargs + ) click.secho("OK", fg="green", err=use_stderr) _print_metadata_entry(opts, entry) @@ -496,7 +502,7 @@ def remove_metadata(ctx, opts, owner_repo_package, metadata_slug_perm, yes): slug_perm = api_get_package_slug_perm( owner=owner, repo=repo, identifier=package ) - api_delete_metadata(slug_perm, metadata_slug_perm) + api_delete_metadata(slug_perm, metadata_slug_perm, credential=opts.credential) click.secho("OK", fg="green", err=use_stderr) diff --git a/cloudsmith_cli/cli/commands/push.py b/cloudsmith_cli/cli/commands/push.py index 7a0c642e..c94141d0 100644 --- a/cloudsmith_cli/cli/commands/push.py +++ b/cloudsmith_cli/cli/commands/push.py @@ -300,7 +300,9 @@ def validate_metadata_payload( try: with maybe_spinner(opts): - api_validate_metadata(content=content, content_type=content_type) + api_validate_metadata( + credential=opts.credential, content=content, content_type=content_type + ) except ApiException as exc: http_status = getattr(exc, "status", None) detail = ( @@ -385,6 +387,7 @@ def attach_metadata_to_package( with maybe_spinner(opts): entry = api_create_metadata( slug_perm, + credential=opts.credential, content=content, content_type=content_type, source_identity=source_identity, diff --git a/cloudsmith_cli/cli/tests/commands/test_metadata.py b/cloudsmith_cli/cli/tests/commands/test_metadata.py index fac05392..a230c1ea 100644 --- a/cloudsmith_cli/cli/tests/commands/test_metadata.py +++ b/cloudsmith_cli/cli/tests/commands/test_metadata.py @@ -2,7 +2,7 @@ import json import unittest -from unittest.mock import patch +from unittest.mock import ANY, patch import click from click.testing import CliRunner @@ -248,7 +248,7 @@ def test_single_fetch_calls_get_and_skips_list( ) self.assertEqual(result.exit_code, 0, msg=result.output) - mock_get.assert_called_once_with("pkg-slug-perm", "meta-slug") + mock_get.assert_called_once_with("pkg-slug-perm", "meta-slug", credential=ANY) mock_list.assert_not_called() @patch("cloudsmith_cli.cli.commands.metadata.api_get_metadata") @@ -654,7 +654,9 @@ def test_remove_calls_delete(self, mock_resolve, mock_delete): ) self.assertEqual(result.exit_code, 0, msg=result.output) - mock_delete.assert_called_once_with("pkg-slug-perm", "meta-slug") + mock_delete.assert_called_once_with( + "pkg-slug-perm", "meta-slug", credential=ANY + ) @patch("cloudsmith_cli.cli.commands.metadata.api_delete_metadata") @patch("cloudsmith_cli.cli.commands.metadata.api_get_package_slug_perm") @@ -680,7 +682,9 @@ def test_remove_alias_rm(self, mock_resolve, mock_delete): ) self.assertEqual(result.exit_code, 0, msg=result.output) - mock_delete.assert_called_once_with("pkg-slug-perm", "meta-slug") + mock_delete.assert_called_once_with( + "pkg-slug-perm", "meta-slug", credential=ANY + ) @patch("cloudsmith_cli.cli.commands.metadata.api_delete_metadata") @patch("cloudsmith_cli.cli.commands.metadata.api_get_package_slug_perm") diff --git a/cloudsmith_cli/cli/tests/test_push.py b/cloudsmith_cli/cli/tests/test_push.py index f5029705..16deab72 100644 --- a/cloudsmith_cli/cli/tests/test_push.py +++ b/cloudsmith_cli/cli/tests/test_push.py @@ -4,7 +4,7 @@ import tempfile import unittest from types import SimpleNamespace -from unittest.mock import MagicMock, patch +from unittest.mock import ANY, MagicMock, patch import click import pytest @@ -187,6 +187,7 @@ def test_upload_files_and_create_package_with_metadata(self): mock_create_metadata.assert_called_once_with( "slug-perm-abc", + credential=ANY, content={"git_sha": "abc123"}, content_type="application/vnd.jfrog.buildinfo+json", source_identity="github-actions@example", @@ -204,7 +205,7 @@ def test_upload_files_and_create_package_with_json_null_metadata(self): ): upload_files_and_create_package( self.mock_ctx, - MagicMock(spec=[]), + MagicMock(spec=["credential"]), self.package_type, [self.owner, self.repo], self.dry_run, @@ -232,7 +233,7 @@ def test_upload_attach_publishes_metadata_info_to_opts(self): "content": {"git_sha": "abc123"}, } - opts = MagicMock(spec=[]) + opts = MagicMock(spec=["credential"]) with ( patch("cloudsmith_cli.cli.commands.push.validate_create_package"), @@ -321,7 +322,7 @@ def test_upload_files_and_create_package_metadata_failure_warn_does_not_fail_pus status=422, detail="Schema validation failed" ) - opts = MagicMock(spec=[]) + opts = MagicMock(spec=["credential"]) # Push must complete without raising, returning the slug pair. result = upload_files_and_create_package( @@ -385,6 +386,7 @@ def test_upload_files_and_create_package_metadata_failure_default_aborts_push(se patched_env.pop("CLOUDSMITH_METADATA_FAILURE_MODE", None) opts = SimpleNamespace( output=None, + credential=None, push_metadata_info=None, verbose=False, debug=False, @@ -444,6 +446,7 @@ def test_prevalidate_metadata_404_aborts_before_upload_by_default(self): ): opts = SimpleNamespace( output=None, + credential=None, push_metadata_info=None, verbose=False, debug=False, @@ -503,7 +506,7 @@ def test_prevalidate_metadata_warn_skips_attach_but_uploads_package(self): {"CLOUDSMITH_METADATA_FAILURE_MODE": "warn"}, ), ): - opts = MagicMock(spec=[]) + opts = MagicMock(spec=["credential"]) result = upload_files_and_create_package( self.mock_ctx, opts, @@ -558,7 +561,7 @@ def test_local_metadata_content_warn_skips_attach_but_uploads_package(self): {"CLOUDSMITH_METADATA_FAILURE_MODE": "warn"}, ), ): - opts = MagicMock(spec=[]) + opts = MagicMock(spec=["credential"]) result = upload_files_and_create_package( self.mock_ctx, opts, @@ -608,6 +611,7 @@ def test_prevalidate_metadata_default_aborts_before_upload(self): patched_env.pop("CLOUDSMITH_METADATA_FAILURE_MODE", None) opts = SimpleNamespace( output=None, + credential=None, push_metadata_info=None, verbose=False, debug=False, @@ -895,7 +899,7 @@ def fake_get_text_stream(name): for package_file in ("path-1", "path-2"): upload_files_and_create_package( self.mock_ctx, - MagicMock(spec=[]), + MagicMock(spec=["credential"]), self.package_type, [self.owner, self.repo], self.dry_run, @@ -947,7 +951,7 @@ def test_metadata_content_file_round_trip(self): ): upload_files_and_create_package( self.mock_ctx, - MagicMock(spec=[]), + MagicMock(spec=["credential"]), self.package_type, [self.owner, self.repo], self.dry_run, @@ -996,7 +1000,7 @@ def test_metadata_default_source_identity(self): ): upload_files_and_create_package( self.mock_ctx, - MagicMock(spec=[]), + MagicMock(spec=["credential"]), self.package_type, [self.owner, self.repo], self.dry_run, @@ -1018,7 +1022,7 @@ def test_metadata_default_source_identity(self): def test_metadata_retry_hint_emitted_on_attach_warn_failure(self): """Attach failure (warn mode, file payload) routes to the hint helper.""" - opts = SimpleNamespace(output=None, push_metadata_info=None) + opts = SimpleNamespace(output=None, credential=None, push_metadata_info=None) metadata = ResolvedMetadata( provided=True, content={"x": 1}, @@ -1083,7 +1087,7 @@ def test_metadata_retry_hint_emitted_on_attach_warn_failure(self): def test_metadata_retry_hint_emitted_on_validation_warn_failure(self): """Pre-validation warn failure routes to the hint helper after create.""" - opts = SimpleNamespace(output=None, push_metadata_info=None) + opts = SimpleNamespace(output=None, credential=None, push_metadata_info=None) metadata = ResolvedMetadata( provided=True, content={"x": 1}, @@ -1276,6 +1280,7 @@ def _hint_opts(output=None): def _api_opts(output="json"): return SimpleNamespace( output=output, + credential=None, push_metadata_info=None, verbose=False, debug=False, diff --git a/cloudsmith_cli/core/api/metadata.py b/cloudsmith_cli/core/api/metadata.py index 5ce1fc1f..e8390f87 100644 --- a/cloudsmith_cli/core/api/metadata.py +++ b/cloudsmith_cli/core/api/metadata.py @@ -1,87 +1,17 @@ """API - Package metadata (v2) endpoints.""" -import json from typing import Any -import cloudsmith_api - -from .. import ratelimits, utils +from .. import utils +from ..credentials.models import CredentialResult from ..pagination import PageInfo -from ..rest import RestClient -from .exceptions import catch_raise_api_exception - - -class _MetadataApi: - """Small client for metadata endpoints not yet present in cloudsmith_api.""" - - def __init__(self): - self.config = cloudsmith_api.Configuration() - self.rest_client = RestClient( - error_retry_cb=getattr(self.config, "error_retry_cb", None), - respect_retry_after_header=getattr(self.config, "rate_limit", True), - ) - - -def get_metadata_api(): - """Get the metadata API client.""" - return _MetadataApi() - - -def _build_url(config, *parts): - host = (config.host or "").rstrip("/") - suffix = "/".join(p.strip("/") for p in parts if p) - return f"{host}/v2/{suffix}/" - - -def _build_headers(config): - """Build request headers from the configured cloudsmith_api.Configuration. - - Mirrors the auth resolution performed by core/api/init.py: an Authorization - header (SSO bearer or Basic) takes precedence; otherwise we fall back to - the X-Api-Key header. - """ - headers = {"Accept": "application/json", "Content-Type": "application/json"} - headers.update(getattr(config, "headers", None) or {}) - - user_agent = getattr(config, "user_agent", None) - if user_agent: - headers["User-Agent"] = user_agent - - if headers.get("Authorization"): - headers.pop("X-Api-Key", None) - else: - api_key = (config.api_key or {}).get("X-Api-Key") - if api_key: - headers["X-Api-Key"] = api_key - - return headers - - -def _request(client, method, *path_parts, query_params=None, body=None): - url = _build_url(client.config, *path_parts) - - with catch_raise_api_exception(): - response = client.rest_client.request( - method, - url, - query_params=query_params, - headers=_build_headers(client.config), - body=body, - ) - - ratelimits.maybe_rate_limit(client, response.getheaders()) - return response - - -def _response_json(response): - if not response.data: - return {} - return json.loads(response.data) +from . import v2 def list_metadata( package_slug_perm: str, *, + credential: CredentialResult | None, source_kind: str | None = None, classification: str | None = None, page: int | None = None, @@ -95,7 +25,7 @@ def list_metadata( Returns a (results, PageInfo) tuple. """ - client = get_metadata_api() + client = v2.get_client(credential) api_kwargs = {} source_kind_value = str(source_kind).strip().lower() if source_kind else "" @@ -107,7 +37,7 @@ def list_metadata( api_kwargs.update(utils.get_page_kwargs(page=page, page_size=page_size)) - response = _request( + response = v2.request( client, "GET", "metadata", @@ -116,16 +46,21 @@ def list_metadata( query_params=api_kwargs or None, ) - payload = _response_json(response) + payload = v2.response_json(response) results = payload.get("results", payload) if isinstance(payload, dict) else payload page_info = PageInfo.from_headers(response.getheaders()) return results, page_info -def get_metadata(package_slug_perm: str, metadata_slug_perm: str): +def get_metadata( + package_slug_perm: str, + metadata_slug_perm: str, + *, + credential: CredentialResult | None, +): """Retrieve a single metadata entry attached to a package.""" - client = get_metadata_api() - response = _request( + client = v2.get_client(credential) + response = v2.request( client, "GET", "metadata", @@ -133,33 +68,35 @@ def get_metadata(package_slug_perm: str, metadata_slug_perm: str): package_slug_perm, metadata_slug_perm, ) - return _response_json(response) + return v2.response_json(response) def create_metadata( package_slug_perm: str, *, + credential: CredentialResult | None, content: Any, content_type: str, source_identity: str, ): """Attach a new metadata entry to a package.""" - client = get_metadata_api() + client = v2.get_client(credential) body = { "content": content, "content_type": content_type, "source_identity": source_identity, } - response = _request( + response = v2.request( client, "POST", "metadata", "packages", package_slug_perm, body=body ) - return _response_json(response) + return v2.response_json(response) def update_metadata( package_slug_perm: str, metadata_slug_perm: str, *, + credential: CredentialResult | None, content: Any = None, source_identity: str | None = None, ): @@ -169,7 +106,7 @@ def update_metadata( attempts to change `content_type`. Fields left as None are omitted from the patch body so existing values are preserved. """ - client = get_metadata_api() + client = v2.get_client(credential) body = {} if content is not None: body["content"] = content @@ -180,7 +117,7 @@ def update_metadata( "update_metadata requires at least one of content or source_identity" ) - response = _request( + response = v2.request( client, "PATCH", "metadata", @@ -189,13 +126,18 @@ def update_metadata( metadata_slug_perm, body=body, ) - return _response_json(response) + return v2.response_json(response) -def delete_metadata(package_slug_perm: str, metadata_slug_perm: str): +def delete_metadata( + package_slug_perm: str, + metadata_slug_perm: str, + *, + credential: CredentialResult | None, +): """Remove a customer-owned metadata entry from a package.""" - client = get_metadata_api() - _request( + client = v2.get_client(credential) + v2.request( client, "DELETE", "metadata", @@ -205,13 +147,15 @@ def delete_metadata(package_slug_perm: str, metadata_slug_perm: str): ) -def validate_metadata(*, content: Any, content_type: str): +def validate_metadata( + *, credential: CredentialResult | None, content: Any, content_type: str +): """Validate a metadata payload against its content type schema. Hits POST /v2/metadata/validate/ which checks shape and schema without persisting. Server returns 200 on success and 422 on validation failure. """ - client = get_metadata_api() + client = v2.get_client(credential) body = {"content": content, "content_type": content_type} - _request(client, "POST", "metadata", "validate", body=body) + v2.request(client, "POST", "metadata", "validate", body=body) return True diff --git a/cloudsmith_cli/core/api/v2.py b/cloudsmith_cli/core/api/v2.py new file mode 100644 index 00000000..225cc794 --- /dev/null +++ b/cloudsmith_cli/core/api/v2.py @@ -0,0 +1,96 @@ +# Copyright 2026 Cloudsmith Ltd +"""API - Shared client for v2 endpoints not yet present in cloudsmith_api.""" + +import json +from urllib.parse import urlparse + +import cloudsmith_api + +from .. import ratelimits +from ..credentials.models import CredentialResult +from ..rest import RestClient +from .exceptions import ApiException, catch_raise_api_exception + +REQUEST_TIMEOUT_SECONDS = 30 + + +class V2Client: + """Small client for v2 endpoints not yet present in cloudsmith_api.""" + + def __init__(self, credential): + self.config = cloudsmith_api.Configuration() + self.credential = credential + self.rest_client = RestClient( + error_retry_cb=getattr(self.config, "error_retry_cb", None), + respect_retry_after_header=getattr(self.config, "rate_limit", True), + ) + + +def get_client(credential: CredentialResult | None): + """Get a v2 API client for the configured host and the resolved credential.""" + return V2Client(credential) + + +def _build_url(config, *parts): + host = (config.host or "").rstrip("/") + suffix = "/".join(p.strip("/") for p in parts if p) + return f"{host}/v2/{suffix}/" + + +def _build_headers(client): + headers = {"Accept": "application/json", "Content-Type": "application/json"} + headers.update(getattr(client.config, "headers", None) or {}) + + user_agent = getattr(client.config, "user_agent", None) + if user_agent: + headers["User-Agent"] = user_agent + + if client.credential: + headers.update(client.credential.auth_headers()) + + return headers + + +def _is_same_origin(url, host): + target, configured = urlparse(url), urlparse(host or "") + return (target.scheme, target.netloc) == (configured.scheme, configured.netloc) + + +def _timeout(method): + # A retry after a read timeout sends the request again, so a POST gets + # only a connect timeout and cannot create a second entry. + if method.upper() == "POST": + return (REQUEST_TIMEOUT_SECONDS, None) + return REQUEST_TIMEOUT_SECONDS + + +def request(client, method, *path_parts, query_params=None, body=None): + """Send a request to the v2 endpoint that `path_parts` names.""" + url = _build_url(client.config, *path_parts) + return request_url(client, method, url, query_params=query_params, body=body) + + +def request_url(client, method, url, query_params=None, body=None): + """Send a request to an absolute v2 URL, such as a pagination link.""" + if not _is_same_origin(url, client.config.host): + raise ApiException(status=0, detail=f"Refusing to send credentials to {url}") + + with catch_raise_api_exception(): + response = client.rest_client.request( + method, + url, + query_params=query_params, + headers=_build_headers(client), + body=body, + _request_timeout=_timeout(method), + ) + + ratelimits.maybe_rate_limit(client, response.getheaders()) + return response + + +def response_json(response): + """Decode a response body, or an empty dict if there is none.""" + if not response.data: + return {} + return json.loads(response.data) diff --git a/cloudsmith_cli/core/session.py b/cloudsmith_cli/core/session.py index 10a55df3..6f2b6f84 100644 --- a/cloudsmith_cli/core/session.py +++ b/cloudsmith_cli/core/session.py @@ -11,6 +11,15 @@ urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) +class ApiKeySafeSession(requests.Session): + """A session that drops the API key on a redirect to another host.""" + + def rebuild_auth(self, prepared_request, response): + super().rebuild_auth(prepared_request, response) + if self.should_strip_auth(response.request.url, prepared_request.url): + prepared_request.headers.pop("X-Api-Key", None) + + class RetryWithCallback(Retry): """A urllib3 Retry with a callback on retries.""" @@ -104,7 +113,7 @@ def create_requests_session( if proxy is None and config is not None: proxy = config.proxy - session = session or requests.Session() + session = session or ApiKeySafeSession() session.verify = ssl_verify session.cert = ssl_cert diff --git a/cloudsmith_cli/core/tests/test_metadata.py b/cloudsmith_cli/core/tests/test_metadata.py index 92e07c3a..a32e70c1 100644 --- a/cloudsmith_cli/core/tests/test_metadata.py +++ b/cloudsmith_cli/core/tests/test_metadata.py @@ -18,6 +18,7 @@ LIST_URL = f"{API_HOST}/v2/metadata/packages/{PKG}/" DETAIL_URL = f"{API_HOST}/v2/metadata/packages/{PKG}/{META}/" VALIDATE_URL = f"{API_HOST}/v2/metadata/validate/" +CREDENTIAL = CredentialResult(api_key="test-api-key", source_name="test") @pytest.fixture(autouse=True) @@ -40,14 +41,7 @@ def _setup_api(monkeypatch): lambda self, how: None, raising=False, ) - initialise_api( - host=API_HOST, - credential=CredentialResult( - api_key="test-api-key", - source_name="test", - auth_type="api_key", - ), - ) + initialise_api(host=API_HOST, credential=CREDENTIAL) def _last_request(): @@ -72,7 +66,7 @@ def test_success_returns_results_and_page_info(self): }, ) - results, page_info = metadata.list_metadata(PKG) + results, page_info = metadata.list_metadata(PKG, credential=CREDENTIAL) assert results == body["results"] assert page_info.is_valid @@ -93,7 +87,12 @@ def test_filters_sent_as_lowercased_names(self): ) metadata.list_metadata( - PKG, source_kind="custom", classification="GENERIC", page=2, page_size=50 + PKG, + credential=CREDENTIAL, + source_kind="custom", + classification="GENERIC", + page=2, + page_size=50, ) qs = _last_request().querystring # pylint: disable=no-member @@ -112,7 +111,7 @@ def test_non_positive_page_options_omitted(self): content_type="application/json", ) - metadata.list_metadata(PKG, page=0, page_size=0) + metadata.list_metadata(PKG, page=0, page_size=0, credential=CREDENTIAL) qs = _last_request().querystring # pylint: disable=no-member assert "page" not in qs @@ -130,7 +129,9 @@ def test_blank_filters_omitted(self): content_type="application/json", ) - metadata.list_metadata(PKG, source_kind=" ", classification="") + metadata.list_metadata( + PKG, source_kind=" ", classification="", credential=CREDENTIAL + ) qs = _last_request().querystring # pylint: disable=no-member assert "source_kind" not in qs @@ -147,7 +148,7 @@ def test_404_raises_api_exception(self): ) with pytest.raises(ApiException) as exc_info: - metadata.list_metadata(PKG) + metadata.list_metadata(PKG, credential=CREDENTIAL) assert exc_info.value.status == 404 assert exc_info.value.detail == "Not found." @@ -174,7 +175,7 @@ def test_invalid_filter_name_surfaces_backend_error(self): ) with pytest.raises(ApiException) as exc_info: - metadata.list_metadata(PKG, source_kind="bogus") + metadata.list_metadata(PKG, source_kind="bogus", credential=CREDENTIAL) qs = _last_request().querystring # pylint: disable=no-member assert qs["source_kind"] == ["bogus"] @@ -204,7 +205,7 @@ def test_success_returns_metadata_entry(self): content_type="application/json", ) - result = metadata.get_metadata(PKG, META) + result = metadata.get_metadata(PKG, META, credential=CREDENTIAL) assert result == body sent = _last_request() @@ -222,7 +223,7 @@ def test_404_on_unknown_metadata(self): ) with pytest.raises(ApiException) as exc_info: - metadata.get_metadata(PKG, META) + metadata.get_metadata(PKG, META, credential=CREDENTIAL) assert exc_info.value.status == 404 assert exc_info.value.detail == "Not found." @@ -242,7 +243,7 @@ def test_422_raises_with_fields(self): ) with pytest.raises(ApiException) as exc_info: - metadata.get_metadata(PKG, META) + metadata.get_metadata(PKG, META, credential=CREDENTIAL) assert exc_info.value.status == 422 assert exc_info.value.fields == { @@ -268,6 +269,7 @@ def test_success_posts_required_fields(self): result = metadata.create_metadata( PKG, + credential=CREDENTIAL, content={"foo": "bar"}, content_type="application/json", source_identity="cloudsmith-cli@1.16.0", @@ -294,6 +296,7 @@ def test_404_when_package_unknown(self): with pytest.raises(ApiException) as exc_info: metadata.create_metadata( PKG, + credential=CREDENTIAL, content={"x": 1}, content_type="application/json", source_identity="customer:test", @@ -317,6 +320,7 @@ def test_422_carries_field_errors(self): with pytest.raises(ApiException) as exc_info: metadata.create_metadata( PKG, + credential=CREDENTIAL, content="not-an-object", content_type="application/json", source_identity="customer:test", @@ -339,7 +343,9 @@ def test_success_sends_only_provided_fields(self): content_type="application/json", ) - result = metadata.update_metadata(PKG, META, source_identity="customer:new") + result = metadata.update_metadata( + PKG, META, source_identity="customer:new", credential=CREDENTIAL + ) assert result == updated assert json.loads(_last_request().body) == {"source_identity": "customer:new"} @@ -347,7 +353,7 @@ def test_success_sends_only_provided_fields(self): @httpretty.activate(allow_net_connect=False) def test_rejects_empty_patch(self): with pytest.raises(ValueError): - metadata.update_metadata(PKG, META) + metadata.update_metadata(PKG, META, credential=CREDENTIAL) @httpretty.activate(allow_net_connect=False) def test_404_on_unknown_metadata(self): @@ -360,7 +366,9 @@ def test_404_on_unknown_metadata(self): ) with pytest.raises(ApiException) as exc_info: - metadata.update_metadata(PKG, META, content={"k": "v"}) + metadata.update_metadata( + PKG, META, content={"k": "v"}, credential=CREDENTIAL + ) assert exc_info.value.status == 404 @httpretty.activate(allow_net_connect=False) @@ -380,7 +388,9 @@ def test_422_when_changing_content_type(self): ) with pytest.raises(ApiException) as exc_info: - metadata.update_metadata(PKG, META, content={"k": "v"}) + metadata.update_metadata( + PKG, META, content={"k": "v"}, credential=CREDENTIAL + ) assert exc_info.value.status == 422 assert exc_info.value.fields == { @@ -393,7 +403,7 @@ class TestDeleteMetadata: def test_success_returns_none(self): httpretty.register_uri(httpretty.DELETE, DETAIL_URL, status=204) - assert metadata.delete_metadata(PKG, META) is None + assert metadata.delete_metadata(PKG, META, credential=CREDENTIAL) is None assert _last_request().method == "DELETE" @httpretty.activate(allow_net_connect=False) @@ -407,7 +417,7 @@ def test_404_raises(self): ) with pytest.raises(ApiException) as exc_info: - metadata.delete_metadata(PKG, META) + metadata.delete_metadata(PKG, META, credential=CREDENTIAL) assert exc_info.value.status == 404 @httpretty.activate(allow_net_connect=False) @@ -425,7 +435,7 @@ def test_422_raises(self): ) with pytest.raises(ApiException) as exc_info: - metadata.delete_metadata(PKG, META) + metadata.delete_metadata(PKG, META, credential=CREDENTIAL) assert exc_info.value.status == 422 assert exc_info.value.fields == {"non_field_errors": ["Metadata is read-only."]} @@ -438,7 +448,9 @@ def test_success_returns_true(self): assert ( metadata.validate_metadata( - content={"foo": "bar"}, content_type="application/json" + credential=CREDENTIAL, + content={"foo": "bar"}, + content_type="application/json", ) is True ) @@ -468,7 +480,9 @@ def test_422_on_non_dict_content(self): with pytest.raises(ApiException) as exc_info: metadata.validate_metadata( - content="not-an-object", content_type="application/json" + credential=CREDENTIAL, + content="not-an-object", + content_type="application/json", ) assert exc_info.value.status == 422 @@ -499,6 +513,7 @@ def test_422_on_failing_schema(self): with pytest.raises(ApiException) as exc_info: metadata.validate_metadata( + credential=CREDENTIAL, content={"bad": "payload"}, content_type="application/vnd.jfrog.buildinfo+json", ) @@ -531,6 +546,7 @@ def test_422_on_non_customer_writable_content_type(self): with pytest.raises(ApiException) as exc_info: metadata.validate_metadata( + credential=CREDENTIAL, content={"foo": "bar"}, content_type="application/vnd.cloudsmith.system+json", ) @@ -553,7 +569,9 @@ def test_401_when_unauthenticated(self): with pytest.raises(ApiException) as exc_info: metadata.validate_metadata( - content={"foo": "bar"}, content_type="application/json" + credential=CREDENTIAL, + content={"foo": "bar"}, + content_type="application/json", ) assert exc_info.value.status == 401 @@ -562,21 +580,19 @@ def test_401_when_unauthenticated(self): class TestAuthHeaders: @staticmethod - def _override_config(monkeypatch, *, api_key=None, headers=None): + def _override_config(monkeypatch, *, headers=None): import cloudsmith_api cfg = cloudsmith_api.Configuration() - cfg.api_key = api_key if api_key is not None else cfg.api_key cfg.headers = headers if headers is not None else cfg.headers monkeypatch.setattr(cloudsmith_api.Configuration, "_default", cfg) @httpretty.activate(allow_net_connect=False) - def test_sso_authorization_header_takes_precedence(self, monkeypatch): - self._override_config( - monkeypatch, - api_key={"X-Api-Key": "test-api-key"}, - headers={"Authorization": "Bearer sso-token"}, + def test_saml_access_token_is_sent_as_a_bearer_token(self): + saml_token = CredentialResult( + api_key="sso-token", source_name="keyring", auth_type="bearer" ) + initialise_api(host=API_HOST) httpretty.register_uri( httpretty.GET, LIST_URL, @@ -585,7 +601,7 @@ def test_sso_authorization_header_takes_precedence(self, monkeypatch): content_type="application/json", ) - metadata.list_metadata(PKG) + metadata.list_metadata(PKG, credential=saml_token) sent = _last_request() assert sent.headers.get("Authorization") == "Bearer sso-token" @@ -605,7 +621,7 @@ def test_extra_config_headers_are_preserved(self, monkeypatch): content_type="application/json", ) - metadata.list_metadata(PKG) + metadata.list_metadata(PKG, credential=CREDENTIAL) sent = _last_request() assert sent.headers.get("X-Custom-Header") == "custom-value" diff --git a/cloudsmith_cli/core/tests/test_rest.py b/cloudsmith_cli/core/tests/test_rest.py index d9ab9fd0..3350a3ec 100644 --- a/cloudsmith_cli/core/tests/test_rest.py +++ b/cloudsmith_cli/core/tests/test_rest.py @@ -66,3 +66,32 @@ def test_sets_user_agent_header(self): def test_sets_extra_headers(self): session = create_requests_session(headers={"X-Custom": "value"}) assert session.headers["X-Custom"] == "value" + + @httpretty.activate(allow_net_connect=False) + def test_drops_the_api_key_on_a_redirect_to_another_host(self): + """The v1 client, uploads and downloads keep the API key off other hosts.""" + api_url = "https://api.example.com/v1/things/" + other_url = "https://other.example.com/things/" + httpretty.register_uri( + httpretty.GET, api_url, status=302, location=other_url, body="" + ) + httpretty.register_uri(httpretty.GET, other_url, body="{}", status=200) + + session = create_requests_session() + session.get(api_url, headers={"X-Api-Key": "test-api-key"}, timeout=5) + + assert "X-Api-Key" not in httpretty.last_request().headers + + @httpretty.activate(allow_net_connect=False) + def test_keeps_the_api_key_on_a_redirect_to_the_same_host(self): + """A redirect on the API host still gets the API key.""" + api_url = "https://api.example.com/v1/things" + httpretty.register_uri( + httpretty.GET, api_url, status=301, location=f"{api_url}/", body="" + ) + httpretty.register_uri(httpretty.GET, f"{api_url}/", body="{}", status=200) + + session = create_requests_session() + session.get(api_url, headers={"X-Api-Key": "test-api-key"}, timeout=5) + + assert httpretty.last_request().headers.get("X-Api-Key") == "test-api-key" diff --git a/cloudsmith_cli/core/tests/test_v2.py b/cloudsmith_cli/core/tests/test_v2.py new file mode 100644 index 00000000..93e7d9df --- /dev/null +++ b/cloudsmith_cli/core/tests/test_v2.py @@ -0,0 +1,170 @@ +# Copyright 2026 Cloudsmith Ltd +"""Tests for the shared v2 API client.""" + +import json + +import cloudsmith_api +import httpretty +import httpretty.core +import pytest + +from .. import keyring +from ..api import v2 +from ..api.exceptions import ApiException +from ..api.init import initialise_api +from ..credentials.models import CredentialResult +from ..rest import RestClient + +API_HOST = "https://api.example.com" +RESOURCE_URL = f"{API_HOST}/v2/things/" +API_KEY = CredentialResult(api_key="test-api-key", source_name="test") +SAML_TOKEN = CredentialResult( + api_key="test-access-token", source_name="keyring", auth_type="bearer" +) + + +@pytest.fixture(autouse=True) +def _setup_api(monkeypatch): + """Initialise the SDK Configuration and stub keyring lookups.""" + monkeypatch.setattr(keyring, "get_access_token", lambda host, profile=None: None) + monkeypatch.setattr(keyring, "get_refresh_token", lambda host, profile=None: None) + monkeypatch.setattr( + keyring, "should_refresh_access_token", lambda host, profile=None: False + ) + monkeypatch.setattr( + httpretty.core.fakesock.socket, + "shutdown", + lambda self, how: None, + raising=False, + ) + initialise_api(host=API_HOST) + + +@pytest.mark.parametrize( + "credential, sent_header, other_header", + [ + (API_KEY, ("X-Api-Key", "test-api-key"), "Authorization"), + (SAML_TOKEN, ("Authorization", "Bearer test-access-token"), "X-Api-Key"), + ], +) +@httpretty.activate(allow_net_connect=False) +def test_request_authenticates_with_the_resolved_credential( + credential, sent_header, other_header +): + """An API key and a SAML access token each go in their own header.""" + httpretty.register_uri(httpretty.GET, RESOURCE_URL, body="{}", status=200) + + v2.request(v2.get_client(credential), "GET", "things") + + headers = httpretty.last_request().headers + assert headers.get(sent_header[0]) == sent_header[1] + assert other_header not in headers + + +@httpretty.activate(allow_net_connect=False) +def test_request_without_a_credential_sends_no_auth_headers(): + """An anonymous request sends no API key and no access token.""" + httpretty.register_uri(httpretty.GET, RESOURCE_URL, body="{}", status=200) + + v2.request(v2.get_client(None), "GET", "things") + + headers = httpretty.last_request().headers + assert "X-Api-Key" not in headers + assert "Authorization" not in headers + + +@pytest.mark.parametrize( + "url", + [ + "http://api.example.com/v2/things/?page=2", + "https://other.example.com/v2/things/?page=2", + ], +) +@httpretty.activate(allow_net_connect=False) +def test_request_url_does_not_send_credentials_to_another_origin(url): + """A URL on another scheme or host does not get the API key.""" + with pytest.raises(ApiException): + v2.request_url(v2.get_client(API_KEY), "GET", url) + + assert not httpretty.latest_requests() + + +@pytest.mark.parametrize("credential", [API_KEY, SAML_TOKEN]) +@httpretty.activate(allow_net_connect=False) +def test_request_does_not_send_credentials_through_a_redirect_to_another_host( + credential, +): + """A redirect to another host does not get the API key or access token.""" + other_url = "https://other.example.com/v2/things/" + httpretty.register_uri( + httpretty.GET, RESOURCE_URL, status=302, location=other_url, body="" + ) + httpretty.register_uri(httpretty.GET, other_url, body="{}", status=200) + + v2.request(v2.get_client(credential), "GET", "things") + + headers = httpretty.last_request().headers + assert "X-Api-Key" not in headers + assert "Authorization" not in headers + + +@pytest.mark.parametrize( + "method, expected_timeout", + [ + ("GET", v2.REQUEST_TIMEOUT_SECONDS), + ("PATCH", v2.REQUEST_TIMEOUT_SECONDS), + ("DELETE", v2.REQUEST_TIMEOUT_SECONDS), + ("POST", (v2.REQUEST_TIMEOUT_SECONDS, None)), + ], +) +@httpretty.activate(allow_net_connect=False) +def test_request_sets_a_timeout(monkeypatch, method, expected_timeout): + """A stalled API does not block the CLI forever. + + A POST gets no read timeout, because the retry after a read timeout + sends the POST again and can create a second entry. + """ + httpretty.register_uri( + method, RESOURCE_URL, body=json.dumps({"results": []}), status=200 + ) + timeouts = [] + original_request = RestClient.request + + def _record_timeout(self, *args, **kwargs): + timeouts.append(kwargs.get("_request_timeout")) + return original_request(self, *args, **kwargs) + + monkeypatch.setattr(RestClient, "request", _record_timeout) + + v2.request(v2.get_client(API_KEY), method, "things") + + assert timeouts == [expected_timeout] + + +@httpretty.activate(allow_net_connect=False) +def test_request_sends_the_configured_user_agent_and_headers(): + """The --api-user-agent and --api-headers values reach the v2 API.""" + initialise_api( + host=API_HOST, user_agent="test-agent/1.0", headers={"X-Custom": "value"} + ) + httpretty.register_uri(httpretty.GET, RESOURCE_URL, body="{}", status=200) + + v2.request(v2.get_client(API_KEY), "GET", "things") + + headers = httpretty.last_request().headers + assert headers.get("User-Agent") == "test-agent/1.0" + assert headers.get("X-Custom") == "value" + + +def test_client_uses_the_configured_proxy_and_ssl_verification(monkeypatch): + """The --api-proxy and --api-ssl-verify values apply to the v2 client.""" + monkeypatch.setattr( + cloudsmith_api.Configuration, "_default", cloudsmith_api.Configuration() + ) + proxy = "http://proxy.example.com:3128" + initialise_api(host=API_HOST, proxy=proxy, ssl_verify=False) + + session = v2.get_client(API_KEY).rest_client.session + + assert session.proxies == {"http": proxy, "https": proxy} + assert session.verify is False