feat(ENG-13683): add Maven credential helper - #339
Conversation
446fb8d to
8479726
Compare
8479726 to
ec1bba9
Compare
This branch shipped an API surface nothing in it calls. Every consumer is in the Maven helper (#339), which is stacked on top of this branch, so the code and its tests move there, where the first caller lives and where a reviewer can see what they are for. Moved: select_custom_domain and CustomDomain.serves_repository; the single-host resolvers default_host, default_host_for_type, builtin_host and builtin_host_for_type with their two private helpers; and domain_scope. With them go the twelve select_custom_domain tests, the six host-resolver tests and the persisted-scope test. Staying: _precedence_key and the CustomDomain.scope / is_bound_to pair it reads, because get_format_domains sorts by it and the Docker installer and runtime helper both call that. The DomainScope enum stays with them; only its string parser moves. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
d58abdb to
da913d2
Compare
da913d2 to
3dbc0f3
Compare
There was a problem hiding this comment.
Pull request overview
Adds Maven support to the credential-helper stack by installing an mvn shim that routes through a new cloudsmith exec command, which injects an ephemeral settings.xml (mode 0600) for authenticated dependency resolution and optional publishing, and cleans it up after each run. This extends the existing credential-helper infrastructure (launchers + default/custom domain handling) and adds targeted CLI wiring plus tests.
Changes:
- Added Maven credential-helper implementation (binding persistence, domain selection,
settings.xmlgeneration, and runtime wrapper that injects-s <temp settings.xml>for Maven invocations). - Added
cloudsmith exec -- <command>andcloudsmith credential-helper shell-initto support shim-based workflows and CI usage without PATH mutation. - Refactored/extended helper plumbing for frozen builds and default-domain resolution, plus comprehensive tests and CHANGELOG entries.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| cloudsmith_cli/templates/maven_settings.xml.tmpl | New template for injected Maven settings.xml (server credentials + active profile + repo URLs). |
| cloudsmith_cli/templates/maven_distribution_management.xml.tmpl | New template for opt-in distributionManagement snippet for publishing. |
| cloudsmith_cli/credential_helpers/maven/settings.py | Builds and securely writes ephemeral settings.xml; constructs download/upload URLs. |
| cloudsmith_cli/credential_helpers/maven/runner.py | Implements runtime command wrapper (cloudsmith exec) and Maven detection/settings injection logic. |
| cloudsmith_cli/credential_helpers/maven/installer.py | Implements install/uninstall/status for Maven shim and repository/domain binding. |
| cloudsmith_cli/credential_helpers/maven/config.py | Persists Maven binding in package-managers.ini and defines shims directory. |
| cloudsmith_cli/credential_helpers/maven/init.py | Adds Maven helper package entry point/module doc. |
| cloudsmith_cli/credential_helpers/launchers.py | Adds frozen-build-safe command construction and public launcher filename helper. |
| cloudsmith_cli/credential_helpers/docker/installer.py | Switches Docker launcher target command to shared cloudsmith_command(...) helper. |
| cloudsmith_cli/credential_helpers/default_domains.py | Adds default_host/default_host_for_type helpers that honor trusted [domains] overrides strictly. |
| cloudsmith_cli/credential_helpers/common.py | Adds default-host detection and repository path shaping based on host kind. |
| cloudsmith_cli/cli/tests/commands/test_default_domains.py | Adds coverage for new default-host resolution semantics (including strict override behavior). |
| cloudsmith_cli/cli/tests/commands/test_credential_helper_maven.py | New tests for Maven settings generation, binding persistence, and exec/shim runtime behavior. |
| cloudsmith_cli/cli/tests/commands/test_credential_helper_maven_installer.py | New tests for Maven install/uninstall/list wiring and domain-binding behavior. |
| cloudsmith_cli/cli/tests/commands/conftest.py | Adds fixtures for isolated CLI config dir and a resolved credential for helper tests. |
| cloudsmith_cli/cli/commands/exec_.py | New cloudsmith exec Click command that resolves credentials and runs the wrapper. |
| cloudsmith_cli/cli/commands/credential_helper/shell.py | New credential-helper shell-init command for PATH initialization snippets (bash/zsh/fish). |
| cloudsmith_cli/cli/commands/credential_helper/manage.py | Wires Maven into credential-helper install/uninstall; adds --repo and --server-id. |
| cloudsmith_cli/cli/commands/credential_helper/init.py | Registers shell-init and updates help text/examples to include Maven + generic usage. |
| cloudsmith_cli/cli/commands/init.py | Registers new exec_ command module for CLI import-time command registration. |
| CHANGELOG.md | Documents new Maven helper, exec, and shell-init, plus Maven server-id security note. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if not (org and credential): | ||
| return [] |
| def _write(parser: configparser.ConfigParser) -> None: | ||
| path = config_path() | ||
| path.parent.mkdir(parents=True, exist_ok=True) | ||
| with click.open_file(str(path), "w") as handle: | ||
| parser.write(handle) |
Maven has no credential-helper protocol, so `credential-helper install maven --org <org> --repo <repo>` writes an `mvn` shim that wraps every invocation in `cloudsmith exec`. Wrapped runs get a mode-0600 settings.xml injected via `mvn -s` and deleted when the run ends, so dependency resolution authenticates with no pom.xml edits and nothing is ever written to ~/.m2. `cloudsmith exec -- <command>` is the same machinery callable directly, for CI that would rather not touch PATH. `credential-helper shell-init` prints the PATH line that activates the shims directory. Publishing stays opt-in: install prints the distributionManagement snippet to paste into pom.xml, keyed by the same `<server>` id the generated settings.xml uses (`cloudsmith`, or `--server-id`). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
3dbc0f3 to
4a1cb9d
Compare
BartoszBlizniak
left a comment
There was a problem hiding this comment.
Pre-approving with some comments:
|
|
||
| # Flags that short-circuit Maven wherever they appear, so there is nothing to | ||
| # authenticate. | ||
| _SKIP_AUTH_ARGS = frozenset({"--help", "-h", "--version", "-v", "help"}) |
There was a problem hiding this comment.
Bare "help" in _SKIP_AUTH_ARGS matches anywhere in the args, so option values trip it: mvn -pl help clean install (a module named help) or mvn -P help ... silently runs without credential injection and fails on the first private dependency, with nothing pointing at the shim.
Since bare mvn help isn't a valid Maven 3 invocation anyway (help is only usable as a help:<goal> prefix, which this doesn't match), the entry buys nothing - I'd drop it and keep just the -h/--help/-v/--version flags.
| file=sys.stderr, | ||
| ) | ||
|
|
||
| temp_dir = tempfile.mkdtemp(prefix="cloudsmith-maven-") |
There was a problem hiding this comment.
On the acknowledged settings.xml-outlives-a-killed-run limitation (_run_with_settings): I reproduced it - kill -9 of cloudsmith exec mid-run leaves the 0600 token file in /tmp. Worth noting SIGTERM (the ordinary CI-cancel signal) leaks the same way, since Python's default SIGTERM disposition skips finally too, so this is the common cancellation path rather than just the unblockable-SIGKILL corner.
Rather than the planned signal handler (which can't cover SIGKILL), consider not putting the token in the file at all: Maven interpolates ${env.NAME} in settings.xml, so the generated file could carry <password>${env.CLOUDSMITH_MAVEN_TOKEN}</password> and the runner pass the token in the child's environment. A leftover file is then harmless.
There was a problem hiding this comment.
Fable finding ^ left it as it's interesting for children projects, maven can be nasty...
| summary = "Maven credential helper for Cloudsmith repositories" | ||
| requires_repo = True | ||
|
|
||
| def install( |
There was a problem hiding this comment.
install never checks that --repo actually exists, so a typo'd slug installs cleanly and only surfaces later as an opaque 401/404 from a wrapped mvn run. Since discovery already makes an authenticated API call at install time, a repo-existence check (or at least a warning when it can't be verified) would catch this where the user can still see the cause.
| # the machine and refuses to run one it has no binding for, so a shim | ||
| # written ahead of a failed set_binding would leave Maven unusable | ||
| # rather than merely uninstalled. | ||
| config.set_binding(binding) |
There was a problem hiding this comment.
config.set_binding(binding) silently replaces any existing binding, and the binding is machine-wide - so installing for project B quietly re-points every mvn on the machine away from project A's repo. Probably fine as a v1 constraint, but the install output should say it replaced old-org/old-repo when it did, so the developer working across two projects understands what just changed.
| } | ||
|
|
||
|
|
||
| def detect_shell(): |
There was a problem hiding this comment.
detect_shell falls back to bash, so on Windows (where $SHELL is unset) shell-init prints POSIX export PATH=... that neither cmd nor PowerShell can eval - even though the launcher machinery happily writes an mvn.cmd shim there. If Windows isn't meant to be supported yet, it'd be better for shell-init to say so explicitly than to print a statement that can't work; otherwise a powershell entry in _STATEMENTS would close the gap.
| \b | ||
| $ cloudsmith exec -- mvn clean install | ||
| """ | ||
| sys.exit(runner.run(list(command), credential=opts.credential)) |
There was a problem hiding this comment.
exec's help promises "a package-manager command authenticated against Cloudsmith", but an unsupported tool passes through with no credentials and no message - cloudsmith exec -- npm ci runs plain npm and the user has no signal that npm isn't wired up. A one-line stderr note ("npm is not a supported package manager; running unwrapped") would prevent people relying on authentication they aren't getting.
There was a problem hiding this comment.
Fable find - might be worth doublechecking docs vs execution
Description
The last of the stack, rebased onto master now that #336, #337 and #338 have
landed, and squashed to a single commit.
Maven has no credential-helper protocol, so this authenticates it with a shell
plugin instead.
credential-helper install maven --org <org> --repo <repo>writes an
mvnshim that wraps every invocation incloudsmith exec, whichinjects an ephemeral mode-0600
settings.xmlviamvn -sand deletes it whenthe run ends — no token is ever written to durable configuration.
credential-helper shell-initprints the shell initialisation (bash, zsh,fish) that puts the shims directory first on
PATH.cloudsmith exec -- <command>is callable directly, for CI that should nottouch
PATH.installprints thedistributionManagementsnippet toadd to
pom.xml.install time. Domains bound to a single repository are left out — they need
URLs of a different shape, which is its own change — so a repository-scoped
domain falls back to the default host. Which kind each host is gets recorded
in
package-managers.ini, so wrapped runs need no lookup — the run pathmakes no API call and no cache read.
Credential binding. One
<server>id covers download and upload. Itdefaults to the literal
cloudsmith, matching thedistributionManagementsnippet
installprints, so a team can share onepom.xml. Maven matches aserver's credentials to a repository by id alone, with no host check, so a
checked-out
pom.xmldeclaring a repository under that id receives the tokenon an ordinary
mvn compile— the same exposure as the~/.m2/settings.xmlaMaven user would otherwise keep. Pass
--server-idat install time to bindthe credential to an id a third-party
pom.xmlcannot guess; minting a randomid by default would break a team sharing one
pom.xml, so it is opt-in.Documented in the CHANGELOG's Security section and
settings.py's moduledocstring.
Type of Change
Additional Notes
Test suite: 682 passed, 40 skipped at this commit.
An earlier, larger draft of this branch was verified end to end in GitHub
Actions with OIDC as the only credential
(run):
a plain
mvn -B clean deployresolved a private dependency through theorganisation's discovered custom domain and published the built jar to
maven.cloudsmith.io/iduffy-demo/default/. That draft carried a cachedsettings.xml, a shell fast path and deploy-goal detection; this PR shipswithout them to halve the diff, and the shim path itself is unchanged.
Two things a reviewer should know:
~/.m2/settings.xml; mirrors, proxies and other<server>entries are not seen. Passing your own-s/--settingsruns Mavenunwrapped, with a warning. This is the designed mechanism — Maven has no
settings-merge — and is called out at install time and in the CHANGELOG.
settings.xmlis removedin a
finally, so a SIGKILL/SIGTERM ofcloudsmith exec(a cancelled CI step)leaves the 0600 token file in
/tmpuntil the agent is recycled; a signalhandler is the fix and wants its own change.