Skip to content

feat(no-ticket): add Maven shell-plugin credential helper - #314

Closed
cloudsmith-iduffy wants to merge 5 commits into
masterfrom
iduffy/credential-helper-maven
Closed

feat(no-ticket): add Maven shell-plugin credential helper#314
cloudsmith-iduffy wants to merge 5 commits into
masterfrom
iduffy/credential-helper-maven

Conversation

@cloudsmith-iduffy

@cloudsmith-iduffy cloudsmith-iduffy commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Description

Adds a Maven credential helper to the CLI. Maven has no native credential-helper protocol, so this uses a shell-plugin approach: a mvn shim (placed first on PATH via cloudsmith credential-helper shell-init) transparently forwards to cloudsmith exec -- mvn …, which provisions an ephemeral settings.xml from the resolved credential (API key or OIDC), runs the real mvn, and cleans up afterwards.

Highlights:

  • cloudsmith exec — a generic, top-level command that runs any command with Cloudsmith credentials provisioned for it; the package manager is inferred from the command (no format argument), and commands with no integration run unchanged. Plus credential-helper shell-init and install maven (with --repo / --server-id).
  • A single injected <server> authenticates both dependency resolution (download CDN) and distributionManagement (native Maven upload) — Maven matches it by <id>, not host.
  • Custom-domain aware, generically: a reusable common.repo_path_segment helper org-scopes Cloudsmith URLs (a custom domain is bound to one org, so the <org> path segment is dropped; the *.cloudsmith.io defaults keep it). Used by both the download and upload URLs.
  • settings.xml lives in cloudsmith_cli/templates/maven_settings.xml.tmpl (real XML, editor-validated) and is rendered via the shared templates.render() extracted in refactor(no-ticket): extract shared templates.render() from webserver #329 (stacked base PR).
  • Shell-plugin state (package-managers.ini, shims) lives in the CLI config dir (get_default_config_path()), consistent with the custom-domain cache.
  • Built test-first: full suite (552) and pre-commit hooks green.

Proven end-to-end with GitHub OIDC (no API key anywhere) — install, build (download), and a native mvn clean deploy (upload), with ~/.m2 caching:

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Refactoring
  • Other (please describe)

Additional Notes

  • The linked test-repo workflow doubles as a real-world usage example and a living smoke test (OIDC auth, custom-domain discovery, dependency caching). The whole Cloudsmith story is three bare commands: install maven, eval "$(… shell-init)", mvn clean deploy.
  • New subpackages and the template file are picked up by find_packages / existing package_data; no packaging changes needed.

@cloudsmith-iduffy
cloudsmith-iduffy force-pushed the iduffy/credential-helper-maven branch from 5d8b024 to cbb6c35 Compare June 13, 2026 00:09
@cloudsmith-iduffy

Copy link
Copy Markdown
Contributor Author

@claude review once

cloudsmith-iduffy added a commit that referenced this pull request Jul 30, 2026
…ed binding

Review follow-ups from PR #314:
- `install maven` now errors without --org (previously produced malformed
  empty-org URLs like /basic//prod/maven/).
- `exec --org/--repo` now override the stored package-managers.ini binding
  for that invocation instead of being silently ignored.
- Update stale plugins.json docstring references to package-managers.ini.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cloudsmith-iduffy

Copy link
Copy Markdown
Contributor Author

Review (requested via @claude review once)

Overview

The PR adds a Maven credential helper built on a shell-plugin approach: an mvn shim (first on PATH via credential-helper shell-init) forwards to cloudsmith exec -- mvn …, which provisions an ephemeral 0600 settings.xml (API key or OIDC), runs the real mvn (resolved with the shims dir excluded to prevent recursion), and cleans up. The layering is clean — shellplugin (generic protocol/runner/config/shims) vs maven (format specifics) — and the test coverage is thorough (config round-trips, XML escaping, org-scoping of custom domains, provisioning failure cleanup, dry-run, PATH warnings, CLI wiring).

Issues found — fixed in b7a91dc

  1. install maven accepted a missing --org (manage.py only validated --repo), producing malformed empty-org URLs like https://dl.cloudsmith.io/basic//prod/maven/ and a configured maven for None/prod action line. Now errors clearly, matching the message that already said "(and --org)".
  2. cloudsmith exec --org/--repo were silently ignored once a stored binding existedrunner._resolve_entry returned the package-managers.ini entry unconditionally, so the documented flags had no effect after install maven. Explicit flags now override the stored owner/repo for that invocation.
  3. Stale plugins.json references in test docstrings (the state file is package-managers.ini) — renamed; also fixed in the PR description.

Notes, no action needed

  • _needs_auth matches help/--version anywhere in the argv; documented as intentional and the failure mode (skipping auth on an odd mvn … help combo) is benign for these flags.
  • The security posture is sound: token only ever lands in a 0600 file inside a private mkdtemp dir, XML-escaped, removed after the child exits (including on provisioning failure); it never appears in argv or env.
  • --bin-dir is accepted-but-inert for shell-plugin helpers (shims live in a fixed dir); a comment + test pin this down. Worth a warning some day if it confuses anyone.
  • Windows: shims reuse write_launcher (which emits .cmd there), but shell-init only speaks bash/zsh/fish — PowerShell users would need to add the shims dir to PATH manually. Fine to defer.

Full suite + pre-commit green after the fixes. LGTM — marking ready for review.

🤖 Generated with Claude Code

@cloudsmith-iduffy
cloudsmith-iduffy marked this pull request as ready for review July 30, 2026 21:10
@cloudsmith-iduffy
cloudsmith-iduffy requested a review from a team as a code owner July 30, 2026 21:10
Copilot AI review requested due to automatic review settings July 30, 2026 21:11
cloudsmith-iduffy added a commit that referenced this pull request Jul 30, 2026
…ed binding

Review follow-ups from PR #314:
- `install maven` now errors without --org (previously produced malformed
  empty-org URLs like /basic//prod/maven/).
- `exec --org/--repo` now override the stored package-managers.ini binding
  for that invocation instead of being silently ignored.
- Update stale plugins.json docstring references to package-managers.ini.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cloudsmith-iduffy
cloudsmith-iduffy force-pushed the iduffy/credential-helper-maven branch from b7a91dc to 8781273 Compare July 30, 2026 21:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a Maven “shell-plugin” credential helper implemented via cloudsmith exec + PATH shims, allowing Maven commands to run with ephemeral, per-invocation Cloudsmith credentials (API key or OIDC) and generated Maven configuration.

Changes:

  • Introduces cloudsmith exec plus a shell-init mechanism that prepends a shims directory to PATH so mvn invocations are transparently wrapped.
  • Implements the shell-plugin framework (persistent config, shim writing, plugin registry/runner) and a Maven plugin that provisions a temporary settings.xml.
  • Adds Maven installer support (including custom-domain discovery) and a comprehensive test suite covering config, runner, shims, installer, and CLI wiring.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
cloudsmith_cli/templates/maven_settings.xml.tmpl New Maven settings.xml template with placeholder markers for server/token/URL.
cloudsmith_cli/templates/maven_distribution_management.xml.tmpl New distributionManagement snippet template for Maven deploy configuration.
cloudsmith_cli/templates/init.py Adds shared template path + placeholder rendering utility.
cloudsmith_cli/credential_helpers/shellplugin/shims.py Writes/removes shims that forward to cloudsmith exec.
cloudsmith_cli/credential_helpers/shellplugin/shellinit.py Emits shell init snippets to put shims dir first on PATH.
cloudsmith_cli/credential_helpers/shellplugin/runner.py Core execution wrapper: resolves real binary, provisions creds via plugin, runs child, cleans up.
cloudsmith_cli/credential_helpers/shellplugin/plugin.py Defines shell-plugin protocol + registry lookup by name/binary.
cloudsmith_cli/credential_helpers/shellplugin/maven.py Maven plugin: generates ephemeral settings.xml and prepends -s <path>.
cloudsmith_cli/credential_helpers/shellplugin/config.py Persists per-plugin bindings/hosts/ids in package-managers.ini; defines shims dir.
cloudsmith_cli/credential_helpers/shellplugin/init.py Declares shellplugin package.
cloudsmith_cli/credential_helpers/maven/installer.py Maven helper installer: domain discovery, shim install, config persistence, deploy snippet output.
cloudsmith_cli/credential_helpers/maven/init.py Declares Maven helper package.
cloudsmith_cli/credential_helpers/common.py Adds standard-host detection + repo path segment helper; reuses in domain checks.
cloudsmith_cli/cli/webserver.py Switches webserver template rendering to the shared templates module.
cloudsmith_cli/cli/tests/commands/test_credential_helper_maven.py Extensive tests for Maven helper: config, runner, shims, shell-init, installer, and CLI wiring.
cloudsmith_cli/cli/commands/exec_.py Adds cloudsmith exec command wiring to shellplugin runner.
cloudsmith_cli/cli/commands/credential_helper/shell.py Adds credential-helper shell-init command.
cloudsmith_cli/cli/commands/credential_helper/manage.py Extends helper management to support Maven, repo binding, and registry id.
cloudsmith_cli/cli/commands/credential_helper/init.py Registers shell-init under credential-helper.
cloudsmith_cli/cli/commands/init.py Registers the new exec_ command module for CLI import-time registration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +119 to +126
entry = _resolve_entry(impl.name, owner, repo, api_host)
try:
result = impl.provision(entry, token or "", args)
except OSError as exc:
# Boundary: a failed provisioning must not crash the wrapped tool with
# a traceback. provision() cleans up its own temp dir before raising.
print(f"cloudsmith: failed to provision credentials: {exc}", file=sys.stderr)
return 1
cloudsmith-iduffy and others added 3 commits July 30, 2026 22:19
Move the placeholder-based template renderer from cloudsmith_cli/cli/webserver.py
into the templates package so other callers (upcoming Maven credential helper)
can reuse it. webserver keeps thin wrappers; behavior is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Maven has no native credential-helper protocol, so add a shell-plugin
approach: a `mvn` shim (placed first on PATH via `credential-helper
shell-init`) transparently forwards to the generic top-level `cloudsmith
exec`, which provisions an ephemeral settings.xml from the resolved
credential (API key or OIDC), runs the real `mvn`, and cleans up.

- `cloudsmith exec -- <cmd>`: runs a package-manager command authenticated
  against Cloudsmith. The plugin is inferred from the command's binary name;
  unmatched commands run unchanged. Resolves the real binary while excluding
  the shims dir to avoid recursion, and cleans up temp dirs (even on failure,
  with provisioning errors surfaced as a clean message, never a traceback).
- `credential-helper install maven --org --repo [--registry-id]` and
  `credential-helper shell-init`, wired into the existing installer registry.
- One injected <server> authenticates both dependency resolution (download
  CDN) and distributionManagement (native Maven upload); Maven matches it by
  id. Install prints the distributionManagement snippet for opt-in deploy.
- Custom Cloudsmith domains are org-scoped via a reusable
  common.repo_path_segment (drop <org> for custom domains, keep it for
  *.cloudsmith.io); download custom domains carry backend_kind=None, upload
  custom domains carry BackendKind.MAVEN.
- settings.xml / distributionManagement live in cloudsmith_cli/templates/*.tmpl
  rendered via a shared templates.render() (also adopted by webserver.py).
- Shell-plugin state is stored as [package-manager:<format>] sections in
  package-managers.ini in the CLI config dir, via configparser.
- The `--registry-id` flag and the shared PluginEntry field use a
  package-manager-neutral name (Maven <server> id, NuGet source key, etc.).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ed binding

Review follow-ups from PR #314:
- `install maven` now errors without --org (previously produced malformed
  empty-org URLs like /basic//prod/maven/).
- `exec --org/--repo` now override the stored package-managers.ini binding
  for that invocation instead of being silently ignored.
- Update stale plugins.json docstring references to package-managers.ini.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Base automatically changed from iduffy/templates-render to master July 31, 2026 08:21
* fix(no-ticket): sync BackendKind with the server-side enum

`credential_helpers/backends.py` mirrors `BackendKind` from the platform's
`cloudsmith/package/enums.py`, but stopped at `MCP = 30`. The server also
defines `NIX = 31`, so any Nix repository's `backend_kind` arrived as a value
the CLI could not name.

These integers are wire values, so 31 is not a new allocation — it is the
value the server already sends. Added in numeric order; nothing else is
renumbered.

* feat(ENG-13681): add credential-helper generic command

Emits a resolved credential as a versioned JSON document on stdout, so tools
that cannot import the CLI can still authenticate through the full provider
chain (API key, credentials.ini, system keyring, OIDC).

The command takes no arguments. A Cloudsmith token is organisation-wide, so
the host it will be used against cannot change which credential resolves;
asking which hosts are Cloudsmith is a separate question, answered by
`credential-helper domains`.

The document is serialised in one step and a single broad except guards the
protocol boundary, so a caller invoking this as a subprocess can never see a
partial document or a traceback. Refusals exit non-zero with a human-readable
line on stderr and nothing on stdout.

* fix(ENG-13681): harden generic helper tests and help text

Make the two CLI wiring tests hermetic by resolving the credential from
the --api-key flag, so the chain never reads the developer's real
credentials.ini or keyring and cannot emit an SSO-refresh warning on
stderr (the test_whoami.py pattern).

Drop the --help cross-reference to 'credential-helper domains', which
does not exist in this PR; the stacked domains PR reinstates it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(ENG-13681): trim comments and docstrings

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat(ENG-13681): add credential-helper domains command (#333)

* feat(ENG-13681): add credential-helper domains command

Answers the question `credential-helper generic` deliberately does not: which
hosts can Cloudsmith authenticate? A consumer needs both, and a token is
org-wide, so the host check belongs in its own command.

Emits a versioned JSON document listing the built-in Cloudsmith service hosts
alongside the organisation's custom domains, with the package format each one
serves. Custom domains are not knowable without an API call; shipping the
built-in table here means consumers do not have to hardcode and re-sync a host
list of their own. Like `generic`, this is for programmatic consumers only and
has no human-readable output mode.

Disabled and unvalidated custom domains are listed too, with their state
intact, so a domain that exists but does not work is visible rather than
absent.

The built-in table can be replaced wholesale by a `[domains]` section in
config.ini, for dedicated and on-premise deployments. That override is honoured
only from trusted configuration. config.ini is searched in the working
directory first, so a repository can ship one, and this list decides which
hosts may receive a credential — the same reasoning behind the existing
api_host guard. A directory-relative section is ignored with a warning on
stderr rather than silently.

* docs(ENG-13681): cross-reference domains from generic --help

Reinstates the sentence deferred from the generic PR, now that the
domains command it refers to exists.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(ENG-13681): keep the developer's real environment out of domains tests

The command tests invoked the full credential chain and, in two cases,
could reach the live custom-domains API: nothing cleared an inherited
CLOUDSMITH_ORG, so on a developer machine with that set (and real
credentials) test_explicit_config_file_supplies_default_domains and
test_untrusted_config_warning_does_not_contaminate_stdout would fetch the
org's actual domains - failing the former whenever the org has any. The
default-domain assertions were similarly exposed to a [domains] section in
the developer's real trusted config.ini.

Mirror the generic command's hermetic pattern: pass --api-key so the chain
resolves from the flag provider (never keyring, credentials.ini, OIDC or
the network), and add an autouse fixture that clears CLOUDSMITH_ORG and
CLOUDSMITH_CONFIG_FILE and pins the trusted-config lookup to "nothing
found". Also correct the domains module docstring, which still described
the command as listing only custom domains.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat(ENG-13681): derive shellplugin hosts from the shared domain table

Export CDN_HOST and a builtin_host lookup from default_domains and use
them for the shellplugin's DEFAULT_CDN_HOST/DEFAULT_UPLOAD_HOST, so the
built-in table stays the single place that knows the service hosts. Also
rewords the default_domains module docstring, per review, to describe
the table and its trusted-config override rather than its consumers.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* refactor(ENG-13681): resolve the organisation from click context, drop flags

Review feedback: the command should take no arguments. --custom-only is
gone - consumers that want only custom domains filter on type. --org is
gone too, along with the hand-rolled CLOUDSMITH_ORG lookup: the
organisation now comes from click context via opts.oidc_org, which click
populates from CLOUDSMITH_ORG or oidc_org in config.ini - values anyone
authenticating via SAML has typically already set. With no organisation
configured the built-in hosts are still listed, which needs no auth.

Also from review: drop the PROTOCOL_VERSION comment, the keyring-backend
asides in the command help and module docstrings, the HERMETIC_ARGS
comment, and the numbered section banners in the tests.

The new config-based org test exposed real cross-test pollution: the
Options object is a process-wide thread-local, and
ConfigReader.load_config permanently prepends an explicit --config-file
to class-level state, so one test's organisation leaked into the next.
The hermetic fixture now pins both per test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
cloudsmith-iduffy added a commit that referenced this pull request Jul 31, 2026
…ed binding

Review follow-ups from PR #314:
- `install maven` now errors without --org (previously produced malformed
  empty-org URLs like /basic//prod/maven/).
- `exec --org/--repo` now override the stored package-managers.ini binding
  for that invocation instead of being silently ignored.
- Update stale plugins.json docstring references to package-managers.ini.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants