From 19d1519828c7d17436190d60c9c723e0d17a4f6a Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Wed, 10 Jun 2026 21:05:31 +0100 Subject: [PATCH 1/3] feat: add OIDC detector enable/order controls Add two controls for OIDC detector selection, resolved through the credential context rather than read ad hoc from the environment: - CLOUDSMITH_OIDC__DISABLED skips a specific detector (only the literal "true", case-insensitive, disables). The credentials decorator resolves these into context.oidc_disabled_detectors. - --oidc-detector-order (env var CLOUDSMITH_OIDC_DETECTOR_ORDER) overrides which detectors are considered and the order they are tried in (comma-separated ids; unlisted/unknown ids are skipped). When both are set the order list defines the candidate set and sequence, then the disabled set is applied on top, so a disabled detector is always skipped. Each detector gains a stable `id` attribute and a public `registered_detectors()` accessor is added. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + README.md | 9 ++ cloudsmith_cli/cli/config.py | 10 ++ cloudsmith_cli/cli/decorators.py | 13 ++ cloudsmith_cli/core/credentials/models.py | 2 + .../credentials/oidc/detectors/__init__.py | 67 ++++++++- .../core/credentials/oidc/detectors/aws.py | 1 + .../oidc/detectors/azure_devops.py | 1 + .../core/credentials/oidc/detectors/base.py | 1 + .../oidc/detectors/bitbucket_pipelines.py | 1 + .../credentials/oidc/detectors/circleci.py | 1 + .../credentials/oidc/detectors/generic.py | 1 + .../oidc/detectors/github_actions.py | 1 + .../credentials/oidc/detectors/gitlab_ci.py | 1 + .../core/tests/test_detector_controls.py | 137 ++++++++++++++++++ 15 files changed, 246 insertions(+), 1 deletion(-) create mode 100644 cloudsmith_cli/core/tests/test_detector_controls.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 22e5dc42..0cfae406 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Added GitHub Actions to OIDC credential auto-discovery. When running in GitHub Actions (with `id-token: write` permission), the CLI fetches an OIDC token from the Actions runtime endpoint and exchanges it for a Cloudsmith access token. Works out of the box with no extra dependencies. - Added a generic fallback to OIDC credential auto-discovery. When no dedicated environment is detected, the CLI reads an OIDC token from the `CLOUDSMITH_OIDC_TOKEN` environment variable (useful for Jenkins or any custom CI/CD) and exchanges it for a Cloudsmith access token. Works out of the box with no extra dependencies. - Added GitLab CI to OIDC credential auto-discovery. When running in GitLab CI/CD, the CLI reads the OIDC token from the `CLOUDSMITH_OIDC_TOKEN` environment variable (configured via `id_tokens` in `.gitlab-ci.yml`) and exchanges it for a Cloudsmith access token. Works out of the box with no extra dependencies. +- Added controls for OIDC detector selection. Set `CLOUDSMITH_OIDC__DISABLED=true` to skip a specific detector (only the literal `true` disables), or use `--oidc-detector-order` (env var `CLOUDSMITH_OIDC_DETECTOR_ORDER`) with a comma-separated list of detector ids to override which detectors are considered and the order they are tried in. When both are set, disable flags take precedence over the order list. Detector ids: `aws`, `azure_devops`, `bitbucket`, `circleci`, `generic`, `github`, `gitlab`. ## [1.18.0] - 2026-06-09 diff --git a/README.md b/README.md index 34fd3ff1..7bc79260 100644 --- a/README.md +++ b/README.md @@ -211,6 +211,15 @@ See the [Cloudsmith GitLab CI/CD integration guide](https://docs.cloudsmith.com/ As a fallback for environments without a dedicated detector (for example Jenkins with the [credentials binding plugin](https://plugins.jenkins.io/credentials-binding/), or any custom CI/CD system), set the `CLOUDSMITH_OIDC_TOKEN` environment variable to an OIDC JWT and the CLI will exchange it for a Cloudsmith access token. This detector runs last, so a dedicated environment is always preferred when present. See the [Cloudsmith Jenkins OIDC guide](https://docs.cloudsmith.com/authentication/setup-jenkins-to-authenticate-to-cloudsmith-using-oidc). +#### Controlling OIDC Detector Selection + +By default the CLI tries each detector in a fixed priority order and uses the first that matches. Two controls let you override this when a detector matches an environment you don't want it to (for example, the AWS detector matching ambient instance credentials): + +- **Disable a detector** — set `CLOUDSMITH_OIDC__DISABLED=true` to skip it entirely. Only the literal value `true` (case-insensitive) disables; anything else leaves the detector enabled. For example, `CLOUDSMITH_OIDC_AWS_DISABLED=true` skips the AWS detector so an explicitly-set `CLOUDSMITH_OIDC_TOKEN` is picked up by the generic detector instead. +- **Reorder evaluation** — use `--oidc-detector-order` (or the `CLOUDSMITH_OIDC_DETECTOR_ORDER` environment variable) with a comma-separated list of detector ids to control both which detectors are considered and the order they are tried in (first match wins). Ids not listed are skipped; unrecognised ids are ignored. For example, `--oidc-detector-order=generic,aws` tries the generic detector first and considers only those two. + +When both are set, the order list defines the candidate set and sequence, then the `*_DISABLED` flags are applied on top — so a disabled detector is always skipped even if it appears in the order list. Detector ids are: `aws`, `azure_devops`, `bitbucket`, `circleci`, `generic`, `github`, `gitlab`. + ## Configuration There are two configuration files used by the CLI: diff --git a/cloudsmith_cli/cli/config.py b/cloudsmith_cli/cli/config.py index f0de222e..3f273492 100644 --- a/cloudsmith_cli/cli/config.py +++ b/cloudsmith_cli/cli/config.py @@ -489,6 +489,16 @@ def oidc_discovery_disabled(self, value): "oidc_discovery_disabled", bool(value) if value is not None else False ) + @property + def oidc_detector_order(self): + """Get value for the OIDC detector evaluation order.""" + return self._get_option("oidc_detector_order") + + @oidc_detector_order.setter + def oidc_detector_order(self, value): + """Set value for the OIDC detector evaluation order.""" + self._set_option("oidc_detector_order", value) + @property def metadata_failure_mode(self): """Get value for push-time metadata failure mode.""" diff --git a/cloudsmith_cli/cli/decorators.py b/cloudsmith_cli/cli/decorators.py index ecefa3c1..8cd556b9 100644 --- a/cloudsmith_cli/cli/decorators.py +++ b/cloudsmith_cli/cli/decorators.py @@ -1,6 +1,7 @@ """CLI - Decorators.""" import functools +import os import click from click.core import ParameterSource @@ -10,6 +11,7 @@ from ..core.api.init import initialise_api as _initialise_api from ..core.credentials.chain import CredentialProviderChain from ..core.credentials.models import CredentialContext +from ..core.credentials.oidc.detectors import disabled_detectors_from_env from ..core.mcp import server from ..core.rest import create_requests_session as _create_session from . import config, utils @@ -332,6 +334,12 @@ def resolve_credentials(f): envvar="CLOUDSMITH_OIDC_DISCOVERY_DISABLED", help="Disable OIDC auto-discovery.", ) + @click.option( + "--oidc-detector-order", + envvar="CLOUDSMITH_OIDC_DETECTOR_ORDER", + help="Comma-separated OIDC detector ids to control which detectors " + "are considered and the order they are tried in.", + ) @click.pass_context @functools.wraps(f) def wrapper(ctx, *args, **kwargs): @@ -342,6 +350,7 @@ def wrapper(ctx, *args, **kwargs): oidc_org = kwargs.pop("oidc_org") oidc_service_slug = kwargs.pop("oidc_service_slug") oidc_discovery_disabled = _pop_boolean_flag(kwargs, "oidc_discovery_disabled") + oidc_detector_order = kwargs.pop("oidc_detector_order") if oidc_audience: opts.oidc_audience = oidc_audience @@ -351,6 +360,8 @@ def wrapper(ctx, *args, **kwargs): opts.oidc_service_slug = oidc_service_slug if oidc_discovery_disabled: opts.oidc_discovery_disabled = oidc_discovery_disabled + if oidc_detector_order: + opts.oidc_detector_order = oidc_detector_order context = CredentialContext( session=opts.session, @@ -365,6 +376,8 @@ def wrapper(ctx, *args, **kwargs): oidc_org=opts.oidc_org, oidc_service_slug=opts.oidc_service_slug, oidc_discovery_disabled=opts.oidc_discovery_disabled, + oidc_detector_order=opts.oidc_detector_order, + oidc_disabled_detectors=disabled_detectors_from_env(os.environ), ) chain = CredentialProviderChain() diff --git a/cloudsmith_cli/core/credentials/models.py b/cloudsmith_cli/core/credentials/models.py index 6fc5ad0b..8a46a426 100644 --- a/cloudsmith_cli/core/credentials/models.py +++ b/cloudsmith_cli/core/credentials/models.py @@ -30,6 +30,8 @@ class CredentialContext: oidc_org: str | None = None oidc_service_slug: str | None = None oidc_discovery_disabled: bool = False + oidc_detector_order: str | None = None + oidc_disabled_detectors: frozenset[str] = frozenset() @dataclass diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/__init__.py b/cloudsmith_cli/core/credentials/oidc/detectors/__init__.py index 0de3c445..993eeee4 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/__init__.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/__init__.py @@ -15,6 +15,8 @@ from .gitlab_ci import GitLabCIDetector if TYPE_CHECKING: + from collections.abc import Mapping + from ... import CredentialContext logger = logging.getLogger(__name__) @@ -30,11 +32,74 @@ ] +def registered_detectors() -> list[type[EnvironmentDetector]]: + """Return the registered OIDC detectors in their default priority order.""" + return list(_DETECTORS) + + +def disable_env_var(identifier: str) -> str: + """The environment variable that disables the detector with this id.""" + return f"CLOUDSMITH_OIDC_{identifier.upper()}_DISABLED" + + +def _is_disabled_value(value: str | None) -> bool: + """Only the literal string ``true`` (case-insensitive) disables a detector.""" + return (value or "").strip().lower() == "true" + + +def disabled_detectors_from_env(environ: Mapping[str, str]) -> frozenset[str]: + """Detector ids disabled via their CLOUDSMITH_OIDC__DISABLED variable.""" + return frozenset( + detector_cls.id + for detector_cls in _DETECTORS + if _is_disabled_value(environ.get(disable_env_var(detector_cls.id))) + ) + + +def _ordered_detectors(order: str | None) -> list[type[EnvironmentDetector]]: + """Detectors in evaluation order, honouring an explicit order string. + + When ``order`` is unset/empty the default registration order is used. + Otherwise only the listed ids are considered, in the listed order; + unknown ids are logged and skipped. + """ + raw_order = (order or "").strip() + if not raw_order: + return list(_DETECTORS) + + detectors_by_id = {detector_cls.id: detector_cls for detector_cls in _DETECTORS} + ordered: list[type[EnvironmentDetector]] = [] + for token in raw_order.split(","): + identifier = token.strip().lower() + if not identifier: + continue + detector_cls = detectors_by_id.get(identifier) + if detector_cls is None: + logger.debug("Ignoring unknown OIDC detector id: %s", identifier) + continue + ordered.append(detector_cls) + return ordered + + +def _enabled_detectors( + order: str | None, disabled: frozenset[str] +) -> list[type[EnvironmentDetector]]: + """Ordered detectors with disabled ones removed (disable always wins).""" + return [ + detector_cls + for detector_cls in _ordered_detectors(order) + if detector_cls.id not in disabled + ] + + def detect_environment( context: CredentialContext, ) -> EnvironmentDetector | None: """Try each detector in order, returning the first that matches.""" - for detector_cls in _DETECTORS: + enabled = _enabled_detectors( + context.oidc_detector_order, context.oidc_disabled_detectors + ) + for detector_cls in enabled: detector = detector_cls(context=context) try: if detector.detect(): diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/aws.py b/cloudsmith_cli/core/credentials/oidc/detectors/aws.py index 81c2a765..3bba2ef6 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/aws.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/aws.py @@ -24,6 +24,7 @@ class AWSDetector(EnvironmentDetector): """Detects AWS environments and obtains a JWT via STS GetWebIdentityToken.""" name = "AWS" + id = "aws" def __init__(self, context): super().__init__(context) diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/azure_devops.py b/cloudsmith_cli/core/credentials/oidc/detectors/azure_devops.py index bc080d89..eba5ce19 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/azure_devops.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/azure_devops.py @@ -30,6 +30,7 @@ class AzureDevOpsDetector(EnvironmentDetector): """Detects Azure DevOps and fetches an OIDC token via HTTP POST.""" name = "Azure DevOps" + id = "azure_devops" def detect(self) -> bool: return bool(os.environ.get("SYSTEM_OIDCREQUESTURI")) and bool( diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/base.py b/cloudsmith_cli/core/credentials/oidc/detectors/base.py index 9bdfb249..2d1eb1af 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/base.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/base.py @@ -12,6 +12,7 @@ class EnvironmentDetector: """Base class for OIDC environment detectors.""" name: str = "base" + id: str = "base" def __init__(self, context: CredentialContext): self.context = context diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/bitbucket_pipelines.py b/cloudsmith_cli/core/credentials/oidc/detectors/bitbucket_pipelines.py index fec58f27..6d20f6ea 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/bitbucket_pipelines.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/bitbucket_pipelines.py @@ -20,6 +20,7 @@ class BitbucketPipelinesDetector(EnvironmentDetector): """Detects Bitbucket Pipelines and reads its OIDC token from environment.""" name = "Bitbucket Pipelines" + id = "bitbucket" def detect(self) -> bool: return bool(os.environ.get("BITBUCKET_STEP_OIDC_TOKEN")) diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/circleci.py b/cloudsmith_cli/core/credentials/oidc/detectors/circleci.py index acc07f86..fe8c44fa 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/circleci.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/circleci.py @@ -20,6 +20,7 @@ class CircleCIDetector(EnvironmentDetector): """Detects CircleCI and reads OIDC token from environment variable.""" name = "CircleCI" + id = "circleci" def detect(self) -> bool: return os.environ.get("CIRCLECI") == "true" and bool( diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/generic.py b/cloudsmith_cli/core/credentials/oidc/detectors/generic.py index e8b876d4..638bb0cb 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/generic.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/generic.py @@ -27,6 +27,7 @@ class GenericDetector(EnvironmentDetector): """ name = "Generic" + id = "generic" def detect(self) -> bool: return bool((os.environ.get(TOKEN_ENV_VAR) or "").strip()) diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/github_actions.py b/cloudsmith_cli/core/credentials/oidc/detectors/github_actions.py index 16febff6..c2985d6c 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/github_actions.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/github_actions.py @@ -25,6 +25,7 @@ class GitHubActionsDetector(EnvironmentDetector): """Detects GitHub Actions and fetches an OIDC token via HTTP request.""" name = "GitHub Actions" + id = "github" def detect(self) -> bool: return ( diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/gitlab_ci.py b/cloudsmith_cli/core/credentials/oidc/detectors/gitlab_ci.py index b5090f5e..242adc8b 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/gitlab_ci.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/gitlab_ci.py @@ -29,6 +29,7 @@ class GitLabCIDetector(EnvironmentDetector): """ name = "GitLab CI" + id = "gitlab" TOKEN_ENV_VAR = "CLOUDSMITH_OIDC_TOKEN" diff --git a/cloudsmith_cli/core/tests/test_detector_controls.py b/cloudsmith_cli/core/tests/test_detector_controls.py new file mode 100644 index 00000000..19c34f1b --- /dev/null +++ b/cloudsmith_cli/core/tests/test_detector_controls.py @@ -0,0 +1,137 @@ +# Copyright 2026 Cloudsmith Ltd +"""Tests for OIDC detector selection controls (disable set + order).""" + +from unittest import mock + +import pytest + +from cloudsmith_cli.core.credentials.models import CredentialContext +from cloudsmith_cli.core.credentials.oidc import detectors as detectors_pkg +from cloudsmith_cli.core.credentials.oidc.detectors import ( + detect_environment, + disable_env_var, + disabled_detectors_from_env, + registered_detectors, +) +from cloudsmith_cli.core.credentials.oidc.detectors.base import EnvironmentDetector + + +class _FakeDetector(EnvironmentDetector): + detects = True + + def detect(self) -> bool: + return self.detects + + def get_token(self) -> str: + return f"{self.id}-token" + + +class AlphaDetector(_FakeDetector): + name = "Alpha" + id = "alpha" + + +class BravoDetector(_FakeDetector): + name = "Bravo" + id = "bravo" + + +@pytest.fixture +def fake_detectors(): + """Register two always-matching fake detectors, Alpha before Bravo.""" + with mock.patch.object(detectors_pkg, "_DETECTORS", [AlphaDetector, BravoDetector]): + yield + + +def _context(**kwargs): + return CredentialContext(**kwargs) + + +class TestDefaultBehaviour: + def test_first_in_default_order_wins(self, fake_detectors): + detector = detect_environment(_context()) + assert isinstance(detector, AlphaDetector) + + def test_returns_none_when_nothing_detects(self, fake_detectors): + with mock.patch.object(AlphaDetector, "detects", False), mock.patch.object( + BravoDetector, "detects", False + ): + assert detect_environment(_context()) is None + + +class TestDisabledDetectors: + def test_disabled_detector_is_skipped(self, fake_detectors): + detector = detect_environment( + _context(oidc_disabled_detectors=frozenset({"alpha"})) + ) + assert isinstance(detector, BravoDetector) + + def test_disabling_all_returns_none(self, fake_detectors): + assert ( + detect_environment( + _context(oidc_disabled_detectors=frozenset({"alpha", "bravo"})) + ) + is None + ) + + +class TestDisabledDetectorsFromEnv: + def test_env_var_name(self): + assert disable_env_var("alpha") == "CLOUDSMITH_OIDC_ALPHA_DISABLED" + + @pytest.mark.parametrize("value", ["true", "TRUE", " true ", "True"]) + def test_truthy_values_disable(self, fake_detectors, value): + disabled = disabled_detectors_from_env( + {"CLOUDSMITH_OIDC_ALPHA_DISABLED": value} + ) + assert disabled == frozenset({"alpha"}) + + @pytest.mark.parametrize("value", ["false", "", " ", "1", "yes", "on", "0"]) + def test_non_true_values_do_not_disable(self, fake_detectors, value): + disabled = disabled_detectors_from_env( + {"CLOUDSMITH_OIDC_ALPHA_DISABLED": value} + ) + assert disabled == frozenset() + + def test_unset_means_enabled(self, fake_detectors): + assert disabled_detectors_from_env({}) == frozenset() + + +class TestDetectorOrder: + def test_order_reorders_evaluation(self, fake_detectors): + detector = detect_environment(_context(oidc_detector_order="bravo,alpha")) + assert isinstance(detector, BravoDetector) + + def test_order_limits_candidate_set(self, fake_detectors): + # Only bravo is listed; alpha must not be considered even though it + # would otherwise match first. + with mock.patch.object(BravoDetector, "detects", False): + assert detect_environment(_context(oidc_detector_order="bravo")) is None + + def test_unknown_id_is_ignored(self, fake_detectors): + detector = detect_environment(_context(oidc_detector_order="nope,alpha")) + assert isinstance(detector, AlphaDetector) + + def test_empty_order_falls_back_to_default(self, fake_detectors): + detector = detect_environment(_context(oidc_detector_order=" ")) + assert isinstance(detector, AlphaDetector) + + +class TestOrderAndDisableCompose: + def test_disable_wins_over_order(self, fake_detectors): + # alpha is listed first in the order but disabled, so bravo wins. + detector = detect_environment( + _context( + oidc_detector_order="alpha,bravo", + oidc_disabled_detectors=frozenset({"alpha"}), + ) + ) + assert isinstance(detector, BravoDetector) + + +class TestRealDetectorIds: + def test_all_registered_detectors_have_unique_ids(self): + ids = [cls.id for cls in registered_detectors()] + assert "base" not in ids + assert len(ids) == len(set(ids)) + assert "generic" in ids From 61dc52dcde3a2ab142cfcfc446b3e75476b6b50a Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Wed, 10 Jun 2026 21:43:55 +0100 Subject: [PATCH 2/3] fix: deduplicate ids in --oidc-detector-order Duplicate detector ids in the order list previously ran the same detector once per occurrence, which is wasteful for detectors that hit metadata endpoints (e.g. AWS STS/IMDS). Duplicates now keep their first position so each detector is evaluated at most once. Also log at debug when the order/disable controls leave no detectors enabled, so an order string with no usable ids is diagnosable. Co-Authored-By: Claude Fable 5 --- .../core/credentials/oidc/detectors/__init__.py | 13 ++++++++----- .../core/tests/test_detector_controls.py | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/__init__.py b/cloudsmith_cli/core/credentials/oidc/detectors/__init__.py index 993eeee4..210140cb 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/__init__.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/__init__.py @@ -61,24 +61,25 @@ def _ordered_detectors(order: str | None) -> list[type[EnvironmentDetector]]: When ``order`` is unset/empty the default registration order is used. Otherwise only the listed ids are considered, in the listed order; - unknown ids are logged and skipped. + unknown ids are logged and skipped, and duplicate ids keep their first + position so each detector is evaluated at most once. """ raw_order = (order or "").strip() if not raw_order: return list(_DETECTORS) detectors_by_id = {detector_cls.id: detector_cls for detector_cls in _DETECTORS} - ordered: list[type[EnvironmentDetector]] = [] + ordered: dict[str, type[EnvironmentDetector]] = {} for token in raw_order.split(","): identifier = token.strip().lower() - if not identifier: + if not identifier or identifier in ordered: continue detector_cls = detectors_by_id.get(identifier) if detector_cls is None: logger.debug("Ignoring unknown OIDC detector id: %s", identifier) continue - ordered.append(detector_cls) - return ordered + ordered[identifier] = detector_cls + return list(ordered.values()) def _enabled_detectors( @@ -99,6 +100,8 @@ def detect_environment( enabled = _enabled_detectors( context.oidc_detector_order, context.oidc_disabled_detectors ) + if not enabled: + logger.debug("No OIDC detectors enabled after applying order/disable controls") for detector_cls in enabled: detector = detector_cls(context=context) try: diff --git a/cloudsmith_cli/core/tests/test_detector_controls.py b/cloudsmith_cli/core/tests/test_detector_controls.py index 19c34f1b..7f927ef4 100644 --- a/cloudsmith_cli/core/tests/test_detector_controls.py +++ b/cloudsmith_cli/core/tests/test_detector_controls.py @@ -1,6 +1,7 @@ # Copyright 2026 Cloudsmith Ltd """Tests for OIDC detector selection controls (disable set + order).""" +import logging from unittest import mock import pytest @@ -116,6 +117,21 @@ def test_empty_order_falls_back_to_default(self, fake_detectors): detector = detect_environment(_context(oidc_detector_order=" ")) assert isinstance(detector, AlphaDetector) + def test_duplicate_ids_run_each_detector_once(self, fake_detectors): + with mock.patch.object( + AlphaDetector, "detect", return_value=False + ) as alpha_detect: + detector = detect_environment( + _context(oidc_detector_order="alpha,alpha,bravo") + ) + assert isinstance(detector, BravoDetector) + assert alpha_detect.call_count == 1 + + def test_order_with_no_usable_ids_disables_detection(self, fake_detectors, caplog): + with caplog.at_level(logging.DEBUG): + assert detect_environment(_context(oidc_detector_order="nope,!")) is None + assert "No OIDC detectors enabled" in caplog.text + class TestOrderAndDisableCompose: def test_disable_wins_over_order(self, fake_detectors): From 7906cdcc6e023fda19b729238f7a65f0099a4e42 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Thu, 11 Jun 2026 10:10:12 +0100 Subject: [PATCH 3/3] feat: support OIDC detector controls in config file + warn on misconfig Address PR #311 review feedback: - Allow oidc_detector_order and oidc_disabled_detectors to be set in config.ini (under [default] or a profile). The config disabled list is additive with the per-detector CLOUDSMITH_OIDC__DISABLED env vars; the --oidc-detector-order flag / env var still override the config order. - Surface a warning (in the CLI layer, where click lives) when --oidc-detector-order names unknown ids, or when the order/disable controls leave no detector enabled. Advisory only: the credential fallback chain is preserved rather than aborting with a UsageError, and the core detectors module stays click-free. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 +- README.md | 12 ++- cloudsmith_cli/cli/config.py | 12 +++ cloudsmith_cli/cli/decorators.py | 58 +++++++++++++- .../cli/tests/test_oidc_detector_config.py | 77 +++++++++++++++++++ 5 files changed, 157 insertions(+), 4 deletions(-) create mode 100644 cloudsmith_cli/cli/tests/test_oidc_detector_config.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cfae406..9c8f6ae0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Added GitHub Actions to OIDC credential auto-discovery. When running in GitHub Actions (with `id-token: write` permission), the CLI fetches an OIDC token from the Actions runtime endpoint and exchanges it for a Cloudsmith access token. Works out of the box with no extra dependencies. - Added a generic fallback to OIDC credential auto-discovery. When no dedicated environment is detected, the CLI reads an OIDC token from the `CLOUDSMITH_OIDC_TOKEN` environment variable (useful for Jenkins or any custom CI/CD) and exchanges it for a Cloudsmith access token. Works out of the box with no extra dependencies. - Added GitLab CI to OIDC credential auto-discovery. When running in GitLab CI/CD, the CLI reads the OIDC token from the `CLOUDSMITH_OIDC_TOKEN` environment variable (configured via `id_tokens` in `.gitlab-ci.yml`) and exchanges it for a Cloudsmith access token. Works out of the box with no extra dependencies. -- Added controls for OIDC detector selection. Set `CLOUDSMITH_OIDC__DISABLED=true` to skip a specific detector (only the literal `true` disables), or use `--oidc-detector-order` (env var `CLOUDSMITH_OIDC_DETECTOR_ORDER`) with a comma-separated list of detector ids to override which detectors are considered and the order they are tried in. When both are set, disable flags take precedence over the order list. Detector ids: `aws`, `azure_devops`, `bitbucket`, `circleci`, `generic`, `github`, `gitlab`. +- Added controls for OIDC detector selection. Set `CLOUDSMITH_OIDC__DISABLED=true` to skip a specific detector (only the literal `true` disables), or use `--oidc-detector-order` (env var `CLOUDSMITH_OIDC_DETECTOR_ORDER`) with a comma-separated list of detector ids to override which detectors are considered and the order they are tried in. When both are set, disable flags take precedence over the order list. Both controls can also be set in `config.ini` via the `oidc_detector_order` and `oidc_disabled_detectors` keys (the latter additive with the `*_DISABLED` env vars). Unknown ids in the order, or controls that leave no detector enabled, are surfaced as a warning. Detector ids: `aws`, `azure_devops`, `bitbucket`, `circleci`, `generic`, `github`, `gitlab`. ## [1.18.0] - 2026-06-09 diff --git a/README.md b/README.md index 7bc79260..f235cf2e 100644 --- a/README.md +++ b/README.md @@ -216,10 +216,20 @@ As a fallback for environments without a dedicated detector (for example Jenkins By default the CLI tries each detector in a fixed priority order and uses the first that matches. Two controls let you override this when a detector matches an environment you don't want it to (for example, the AWS detector matching ambient instance credentials): - **Disable a detector** — set `CLOUDSMITH_OIDC__DISABLED=true` to skip it entirely. Only the literal value `true` (case-insensitive) disables; anything else leaves the detector enabled. For example, `CLOUDSMITH_OIDC_AWS_DISABLED=true` skips the AWS detector so an explicitly-set `CLOUDSMITH_OIDC_TOKEN` is picked up by the generic detector instead. -- **Reorder evaluation** — use `--oidc-detector-order` (or the `CLOUDSMITH_OIDC_DETECTOR_ORDER` environment variable) with a comma-separated list of detector ids to control both which detectors are considered and the order they are tried in (first match wins). Ids not listed are skipped; unrecognised ids are ignored. For example, `--oidc-detector-order=generic,aws` tries the generic detector first and considers only those two. +- **Reorder evaluation** — use `--oidc-detector-order` (or the `CLOUDSMITH_OIDC_DETECTOR_ORDER` environment variable) with a comma-separated list of detector ids to control both which detectors are considered and the order they are tried in (first match wins). Ids not listed are skipped; unrecognised ids are ignored with a warning. For example, `--oidc-detector-order=generic,aws` tries the generic detector first and considers only those two. When both are set, the order list defines the candidate set and sequence, then the `*_DISABLED` flags are applied on top — so a disabled detector is always skipped even if it appears in the order list. Detector ids are: `aws`, `azure_devops`, `bitbucket`, `circleci`, `generic`, `github`, `gitlab`. +Both controls can also be set in `config.ini`, under `[default]` or a profile section: + +```ini +[default] +oidc_detector_order = github, aws +oidc_disabled_detectors = aws, gitlab +``` + +The `--oidc-detector-order` flag (or the `CLOUDSMITH_OIDC_DETECTOR_ORDER` environment variable) overrides the `oidc_detector_order` config value. The `oidc_disabled_detectors` config key is additive with the per-detector `CLOUDSMITH_OIDC__DISABLED` environment variables — a detector disabled by either is skipped. + ## Configuration There are two configuration files used by the CLI: diff --git a/cloudsmith_cli/cli/config.py b/cloudsmith_cli/cli/config.py index 3f273492..d9d6a928 100644 --- a/cloudsmith_cli/cli/config.py +++ b/cloudsmith_cli/cli/config.py @@ -69,6 +69,8 @@ class Default(SectionSchema): oidc_audience = ConfigParam(name="oidc_audience", type=str) oidc_org = ConfigParam(name="oidc_org", type=str) oidc_service_slug = ConfigParam(name="oidc_service_slug", type=str) + oidc_detector_order = ConfigParam(name="oidc_detector_order", type=str) + oidc_disabled_detectors = ConfigParam(name="oidc_disabled_detectors", type=str) metadata_failure_mode = ConfigParam(name="metadata_failure_mode", type=str) @matches_section("profile:*") @@ -499,6 +501,16 @@ def oidc_detector_order(self, value): """Set value for the OIDC detector evaluation order.""" self._set_option("oidc_detector_order", value) + @property + def oidc_disabled_detectors(self): + """Get the comma-separated OIDC detector ids disabled via config.""" + return self._get_option("oidc_disabled_detectors") + + @oidc_disabled_detectors.setter + def oidc_disabled_detectors(self, value): + """Set the comma-separated OIDC detector ids disabled via config.""" + self._set_option("oidc_disabled_detectors", value) + @property def metadata_failure_mode(self): """Get value for push-time metadata failure mode.""" diff --git a/cloudsmith_cli/cli/decorators.py b/cloudsmith_cli/cli/decorators.py index 8cd556b9..6113b282 100644 --- a/cloudsmith_cli/cli/decorators.py +++ b/cloudsmith_cli/cli/decorators.py @@ -11,7 +11,10 @@ from ..core.api.init import initialise_api as _initialise_api from ..core.credentials.chain import CredentialProviderChain from ..core.credentials.models import CredentialContext -from ..core.credentials.oidc.detectors import disabled_detectors_from_env +from ..core.credentials.oidc.detectors import ( + disabled_detectors_from_env, + registered_detectors, +) from ..core.mcp import server from ..core.rest import create_requests_session as _create_session from . import config, utils @@ -309,6 +312,50 @@ def wrapper(ctx, *args, **kwargs): return wrapper +def _parse_detector_ids(value): + """Split a comma-separated detector id string into stripped, lowercased ids.""" + return [ + token.strip().lower() for token in (value or "").split(",") if token.strip() + ] + + +def _config_disabled_detectors(value): + """Detector ids disabled via the config-file ``oidc_disabled_detectors`` key.""" + return frozenset(_parse_detector_ids(value)) + + +def _warn_on_oidc_detector_controls(order, disabled): + """Warn when the OIDC detector order/disable controls look misconfigured. + + Advisory only — detection itself is resolved in the core detectors module; + these warnings surface user-facing mistakes (a typo'd id, or controls that + leave nothing enabled) without aborting the credential fallback chain. + """ + valid_ids = [detector_cls.id for detector_cls in registered_detectors()] + order_ids = _parse_detector_ids(order) + + unknown_ids = [ + identifier for identifier in order_ids if identifier not in valid_ids + ] + if unknown_ids: + click.secho( + "OIDC: ignoring unknown detector id(s) in --oidc-detector-order: " + f"{', '.join(unknown_ids)}. Valid ids: {', '.join(valid_ids)}.", + fg="yellow", + err=True, + ) + + candidate_ids = [i for i in order_ids if i in valid_ids] if order_ids else valid_ids + enabled_ids = [i for i in candidate_ids if i not in disabled] + if (order_ids or disabled) and not enabled_ids: + click.secho( + "OIDC: no detectors are enabled after applying the detector " + "order/disable controls; OIDC auto-discovery will be skipped.", + fg="yellow", + err=True, + ) + + def resolve_credentials(f): """Resolve credentials via the provider chain. Depends on initialise_session.""" @@ -363,6 +410,13 @@ def wrapper(ctx, *args, **kwargs): if oidc_detector_order: opts.oidc_detector_order = oidc_detector_order + oidc_disabled_detectors = disabled_detectors_from_env( + os.environ + ) | _config_disabled_detectors(opts.oidc_disabled_detectors) + _warn_on_oidc_detector_controls( + opts.oidc_detector_order, oidc_disabled_detectors + ) + context = CredentialContext( session=opts.session, api_key_from_flag=opts.api_key_from_flag, @@ -377,7 +431,7 @@ def wrapper(ctx, *args, **kwargs): oidc_service_slug=opts.oidc_service_slug, oidc_discovery_disabled=opts.oidc_discovery_disabled, oidc_detector_order=opts.oidc_detector_order, - oidc_disabled_detectors=disabled_detectors_from_env(os.environ), + oidc_disabled_detectors=oidc_disabled_detectors, ) chain = CredentialProviderChain() diff --git a/cloudsmith_cli/cli/tests/test_oidc_detector_config.py b/cloudsmith_cli/cli/tests/test_oidc_detector_config.py new file mode 100644 index 00000000..7755fb3d --- /dev/null +++ b/cloudsmith_cli/cli/tests/test_oidc_detector_config.py @@ -0,0 +1,77 @@ +# Copyright 2026 Cloudsmith Ltd +"""Tests for config-file + CLI-layer OIDC detector controls.""" + +import pytest + +from ..config import ConfigReader, Options +from ..decorators import _config_disabled_detectors, _warn_on_oidc_detector_controls + + +@pytest.fixture +def config_file(tmp_path): + """Yield a writer for a temporary config.ini, restoring reader state.""" + original_files = list(ConfigReader.config_files) + + def write(body): + path = tmp_path / "config.ini" + path.write_text(body) + return str(path) + + yield write + ConfigReader.config_files = original_files + + +class TestConfigFileControls: + def test_detector_order_is_read_from_config(self, config_file): + path = config_file("[default]\noidc_detector_order = github, aws\n") + opts = Options() + opts.load_config_file(path) + assert opts.oidc_detector_order == "github, aws" + + def test_disabled_detectors_is_read_from_config(self, config_file): + path = config_file("[default]\noidc_disabled_detectors = aws,gitlab\n") + opts = Options() + opts.load_config_file(path) + assert opts.oidc_disabled_detectors == "aws,gitlab" + + +class TestConfigDisabledDetectors: + def test_parses_comma_separated_ids(self): + assert _config_disabled_detectors("aws, gitlab") == frozenset({"aws", "gitlab"}) + + def test_lowercases_and_strips(self): + assert _config_disabled_detectors(" AWS , GitLab ") == frozenset( + {"aws", "gitlab"} + ) + + def test_empty_or_none_is_empty_set(self): + assert _config_disabled_detectors(None) == frozenset() + assert _config_disabled_detectors(" ") == frozenset() + + +class TestDetectorControlWarnings: + def test_unknown_id_warns(self, capsys): + _warn_on_oidc_detector_controls("github,nope", frozenset()) + err = capsys.readouterr().err + assert "nope" in err + assert "github" not in err.split("Valid ids", 1)[0] + + def test_no_warning_when_all_ids_known(self, capsys): + _warn_on_oidc_detector_controls("github,aws", frozenset()) + assert capsys.readouterr().err == "" + + def test_no_warning_without_controls(self, capsys): + _warn_on_oidc_detector_controls(None, frozenset()) + assert capsys.readouterr().err == "" + + def test_warns_when_no_detectors_enabled_via_order(self, capsys): + _warn_on_oidc_detector_controls("nope", frozenset()) + assert "no detectors are enabled" in capsys.readouterr().err.lower() + + def test_warns_when_order_fully_disabled(self, capsys): + _warn_on_oidc_detector_controls("aws", frozenset({"aws"})) + assert "no detectors are enabled" in capsys.readouterr().err.lower() + + def test_no_enabled_warning_in_normal_case(self, capsys): + _warn_on_oidc_detector_controls("github,aws", frozenset()) + assert "no detectors are enabled" not in capsys.readouterr().err.lower()