feat(ENG-13683): add Maven credential helper via shell plugin - #339
Draft
cloudsmith-iduffy wants to merge 11 commits into
Draft
feat(ENG-13683): add Maven credential helper via shell plugin#339cloudsmith-iduffy wants to merge 11 commits into
cloudsmith-iduffy wants to merge 11 commits into
Conversation
Maven has no credential-helper protocol, so authenticate it with a shell plugin instead: `credential-helper install maven --org <org> --repo <repo>` writes an `mvn` shim that wraps every invocation in `cloudsmith exec`, which injects an ephemeral mode-0600 settings.xml via `mvn -s` and deletes it when the run ends. No token is ever written to durable configuration. `credential-helper shell-init` prints the shell initialisation that puts the shims directory first on PATH. `cloudsmith exec -- <command>` is callable directly for CI, without touching PATH. Publishing is opt-in: install prints the distributionManagement snippet to add to pom.xml. Tests are split by subject from the outset — _plugin (binding, settings.xml, shims, shell-init), _runner (exec) and _installer (install/uninstall plus CLI wiring) — since one module covering all of it would exceed the 1000-line limit pylint enforces here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolve the Maven-side findings from a review of the credential helper: - Discard a stored binding's repo and hosts when --org names a different organisation, instead of pasting the old repo slug under the new org. - Detect Maven's attached -s<file> form, which defeated the guard against shadowing a user's own settings.xml. - Point shims at sys.executable for frozen builds, which are not on PATH under the name `cloudsmith`. - Probe the platform's actual shim filename, so status and uninstall find mvn.cmd on Windows. - Route each --domain value to the download or upload host by its discovered backend kind, reporting a superseded value rather than dropping it. - Strip CLOUDSMITH_ORG/CLOUDSMITH_REPO, which click passes through verbatim, so a padded value cannot become a slug. The installer's custom-domain fixtures also carry the organisation the record was discovered under, which the domain layer now requires. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Record on the Maven bullet: a domain bound to the repository being installed is preferred over an organisation-wide one and a domain bound to a different repository is never used; which kind each host is gets recorded alongside it so wrapped runs need no lookup; a repository-scoped domain's URLs carry neither an organisation nor a repository segment; and `exec --repo <other>` resets such a host rather than reusing it for an unrelated repository. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…host repo_path_segment decided whether to include <org> by calling is_standard_cloudsmith_host(), which only recognises *.cloudsmith.io/.com. A host declared in a trusted [domains] table is a deployment's own service host - the on-prem/dedicated equivalent of dl.cloudsmith.io - not a genuinely discovered custom domain, so it must be org-qualified the same way. Previously it took the custom-domain branch and dropped the org, resolving every wrapped mvn run on such a deployment against the wrong path. Adds is_default_host() in common.py, checking the effective default-domain table (load_default_domains(), builtins or a trusted override) rather than hostname suffixes. is_standard_cloudsmith_host() is unchanged - it still decides token eligibility in is_cloudsmith_domain() - and repo_path_segment now honours either check. Reading config.ini from disk is fine on the provision() run path (matching default_cdn_host() already doing so); no network call is introduced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- shellplugin/maven.py: create settings.xml 0600 via os.open(O_CREAT| O_EXCL|O_WRONLY) instead of open() + chmod, closing the brief window at the process umask. - maven/installer.py: _select_domain sorts candidates by host before picking, so choosing among several organisation-scoped domains no longer depends on the discovery API's return order. - maven/installer.py: install() now also warns when `cloudsmith` itself is unresolvable (and the build is not frozen) - the shim execs `cloudsmith exec -- mvn`, so an inactive venv breaks every wrapped mvn run, not just discoverability of the shims dir. - maven/installer.py: _deploy_snippet notes that `exec --org`/`--repo` overrides retarget downloads only; `mvn deploy` still publishes to the pom's distributionManagement, written for the install-time repository. - maven/installer.py: _select_domain/_is_eligible's repo parameter is now annotated str | None, matching _resolve_hosts's caller annotation. - shellplugin/maven.py: download_url/upload_url parameters are now annotated, matching the rest of the module. - Added direct coverage for _drop_repository_scoped_hosts's upload_host/ upload_scope branch (previously unobservable through `exec`, which only injects cdn_host into settings.xml) and for an explicit `exec --repo` matching the already-stored repo leaving a repository-scoped host intact. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tion id Maven matches a <server>'s credentials to a repository by id alone, with no host check, and the injected settings.xml used the fixed, guessable "cloudsmith" default id for every wrapped run - so any checked-out project whose pom.xml declared a repository under that id received the live token on an ordinary `mvn compile`. The download profile/repository/ server we author ourselves now get a fresh secrets.token_hex id every provision() call, and the stable registry_id server that matches the user's distributionManagement is emitted only for deploy invocations (deploy, deploy:*, release:perform), since that is the only case that needs it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…dinates Ruled by the project owner: mvn org.apache.maven.plugins:maven-deploy-plugin:3.1.1:deploy is the same deploy goal as `deploy:deploy`, spelled out in full rather than via shorthand, so it must also unlock the stable distributionManagement server - matching on args ending in ":deploy" catches it while a goal that merely contains the word (deploy-helper:run, -Dmaven.deploy.skip=true) still does not count. Under-matching breaks publishing with an invisible 401; over-matching only emits the stable server on a goal the user explicitly aimed at publishing, so the asymmetry favours widening. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The injected download credential now carries an unguessable per-invocation id, but the stable server emitted for deploys still uses the default `cloudsmith` id, so a hostile checkout can claim it during a deploy's dependency resolution. Note the exposure and the fix for it where the next maintainer will be looking, rather than leaving it in a review thread. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
6 tasks
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| assert config.get_plugin("maven").cdn_host == "second.cdn.example.com" | ||
| assert any( | ||
| a.startswith("WARNING") and "first.cdn.example.com" in a for a in actions |
cloudsmith-iduffy
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Stack 4/4 — the Maven helper itself. Base: #338; the custom-domain work it
relies on is #337.
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.bound to the repository being installed beats an organisation-wide one, and
one bound to a different repository is never used. Which kind each host is
gets recorded in
package-managers.ini, so wrapped runs need no lookup — therun path makes no API call and no cache read.
Credential binding. Maven matches a
<server>to a repository by id alone,with no host check, and the shim applies to every
mvnrun in every directory.A fixed, guessable server id would therefore let any checked-out
pom.xmldeclaring a repository under that id receive the token on an ordinary
mvn compile. So a wrapped run binds the injected download credential to arandom id generated fresh per invocation, and the stable id your
distributionManagementnames is supplied only when the invocation is actuallya deploy.
Type of Change
Additional Notes
Test suite: 775 passed, 40 skipped at this commit.
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.
literal
cloudsmith, so on a deploy a hostile checkout can still claim itduring dependency resolution. Minting it as
cloudsmith-<random>at installtime would close it; that changes a default users paste into
pom.xml, so itis deliberately left to its own change. Documented in
build_settings_xml'sdocstring.