feat(issue-83): add cloudsmith repos gpg command group - #390
Closed
BartoszBlizniak wants to merge 7 commits into
Closed
feat(issue-83): add cloudsmith repos gpg command group#390BartoszBlizniak wants to merge 7 commits into
cloudsmith repos gpg command group#390BartoszBlizniak wants to merge 7 commits into
Conversation
Adds CLI support for managing a repository's GPG signing key, closing the gap noted in cloudsmith-io#83: the API has supported this since repos_gpg_list/create/regenerate were added to the SDK, but the CLI never exposed it. - core/api/repos.py: list_repo_gpg_key, create_repo_gpg_key and regenerate_repo_gpg_key wrappers around the SDK's repos_gpg_* endpoints, translating SDK exceptions to ApiException per existing convention. - cli/commands/repos.py: `cloudsmith repos gpg get|upload|regenerate OWNER/REPO`. Key/passphrase material is only ever read from a file (or stdin via '-') or an interactive hide_input prompt, never a bare command-line flag, so it can't leak into shell history or the process list. `regenerate` asks for confirmation first (like `repos delete`), since it invalidates the repository's current key. - Tests: httpretty-mocked API tests in core/tests/test_repos.py, and mock-patched CLI tests in cli/tests/commands/test_repos.py. There is no delete/rotate-off endpoint on the backend, so there's no `delete` subcommand - `get`/`upload`/`regenerate` is the full surface.
`config.get_or_create_options` caches the Options object in a thread-local, so state that is sticky by design - `--debug` in particular - leaked from one test's CLI invocation into every later one in the same process. Two GPG tests already failed because of it, and any test that runs after one passing `--debug` was at risk. The two tests that needed a clean Options object cleared the thread-local themselves; do it for every CLI test instead.
Applies the command-design review of `cloudsmith repos gpg`: - `regenerate` now requires the word "regenerate" to be typed, instead of a y/N answer, and states what is irrevocable about it before asking. Anything else typed declines with "Not confirmed. No changes made." and sends nothing. With no terminal attached the command fails with a usage error rather than blocking on a question nobody can answer, so `-y/--yes` stays the way to run it unattended. - Both mutating subcommands accept `-n/--dry-run`, which resolves and validates the inputs (so an empty key file still errors) and reports what would change without calling the API. - The GPG failures a person can act on now read as one sentence, e.g. "Could not set GPG key for your-repo: custom GPG keys require a paid plan." `handle_api_exceptions` takes an optional per-status summary map for this; unmapped statuses and JSON output keep the existing rendering, so nothing else changes shape. - `upload` only prompts for a passphrase when a terminal is attached; without one it takes the key to be unencrypted instead of aborting on EOF, which is what the documented behaviour always claimed. The prompt also goes to stderr under `-F json` so stdout stays a single parseable document.
The `gpg upload` guards that stop the private key and the passphrase
both being read from stdin compared the streams click returned against
`click.get_text_stream("stdin")`. `click.File` builds a fresh
`_NonClosingTextIOWrapper` for `-` on every conversion, so that identity
only ever holds under `CliRunner`, where the runner's own stdin object is
handed back unchanged. In a real process both guards were inert:
`--private-key-file - --passphrase-file -` read the key and then took the
passphrase from an already-drained stdin, and `--private-key-file -` on
its own silently assumed the key was unencrypted.
Record the literal value each secret file option was given on the click
context instead, and key the guards off that. This is exact in both a
real process and under `CliRunner`, so the existing tests now prove the
behaviour they claim to.
Also sort the conftest imports, which the new `OPTIONS` import left
out of order.
A dry run that only echoed its own arguments could not catch the two mistakes it is there to catch: a mistyped repository and a credential that no longer works. Both mutating subcommands now read the key currently in place first, report the fingerprint the real run would replace, and stop before the mutating request - so those failures land in the rehearsal, in the same voice the real command would use. `upload --dry-run` also no longer prompts for the passphrase. Nothing is being sent, so there is no reason to make anyone type a real secret; the line says which source the real run would use instead.
BartoszBlizniak
force-pushed
the
claude/latch-feedback-impl-75c24b
branch
from
August 25, 2026 15:54
4941c22 to
ec875e3
Compare
Member
Author
|
Superseded by #391 - the branch was renamed to |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds cloudsmith repos gpg commands for viewing, uploading, and regenerating repository signing keys, with secure input handling, confirmations, dry runs, tests, and documentation.
Changes:
- Added GPG API wrappers and CLI workflows.
- Added secret handling, JSON output, dry-run support, and regeneration confirmation.
- Added error summaries, test isolation, comprehensive tests, and changelog updates.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Summary and final comments |
|---|---|
cloudsmith_cli/core/tests/test_repos.py |
Added API endpoint tests for GPG operations. |
cloudsmith_cli/core/api/repos.py |
Added GPG repository API wrappers. |
cloudsmith_cli/cli/tests/conftest.py |
Added per-test CLI options-state isolation. |
cloudsmith_cli/cli/tests/commands/test_repos.py |
Added CLI behavior and security tests. Nit (2 votes): The human-readable get test should assert the armored public key block and displayed fields. |
cloudsmith_cli/cli/exceptions.py |
Added configurable API error summaries. |
cloudsmith_cli/cli/commands/repos.py |
Added the GPG command group and safety logic. Moderate (2 votes): Confirmation currently accepts surrounding whitespace despite requiring the exact word. Moderate (2 votes): The 400 summary incorrectly describes invalid provided keys for regenerate and dry-run operations. |
CHANGELOG.md |
Documented the GPG command group and related behavior. |
Suppressed comments (1)
cloudsmith_cli/cli/commands/repos.py:560
- The refusal runs only after
@decorators.initialise_apihas executed. In a realupload --debug -F jsoninvocation, API initialization writes its debug credential message to stdout before thisBadParameteris rendered, so stdout is no longer a single parseable JSON document. Reject debug before API initialization, or route that initialization message to stderr when JSON output is selected.
if opts.debug:
raise click.BadParameter(
"Debug output is disabled for this command because the request "
"contains private key material and a passphrase.",
param_hint="--debug",
)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| err=err, | ||
| ) | ||
|
|
||
| if answer.strip() == REGENERATE_CONFIRMATION_WORD: |
| ctx, | ||
| opts, | ||
| "regenerate", | ||
| GPG_WRITE_ERROR_REASONS, |
Comment on lines
+253
to
+255
| assert _GPG_KEY["fingerprint"] in result.output | ||
| assert _GPG_KEY["fingerprint_short"] in result.output | ||
|
|
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
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
Adds
cloudsmith repos gpgfor managing the GPG key a repository signs its package indexes with, and applies the command-design review of it.getshows the active key and its armored public block.uploadinstalls a key you supply.regeneratereplaces the current key with a freshly generated Cloudsmith one. There is nodelete- the API has no way to remove a repository's key, so offering one would imply a capability that doesn't exist.Three things in here are worth a reviewer's attention, because they are judgement calls rather than plumbing.
Regenerating asks for a typed word, not a y/N. It discards the old key with no undo, and every consumer pinned to the old fingerprint has to pick up the new one before their next install succeeds, so a reflexive
yis too cheap:Anything else typed declines with
Not confirmed. No changes made.and sends nothing. With no terminal attached the command fails with a usage error instead of blocking on a question nobody can answer --y/--yesremains the way to run it in a pipeline.Both mutating subcommands take
-n/--dry-run. It validates the local inputs - an empty key file still errors - then reads the key currently in place and names the fingerprint the real run would replace, so a mistyped repository or an expired credential fails in the rehearsal rather than on the attempt. It stops before the mutating request, and it never asks for a passphrase, since nothing is being sent.The actionable failures read as one sentence.
handle_api_exceptionsgained an optional per-status summary map, which the GPG commands use for the three statuses a person can act on:Any other status, and JSON output in every case, keeps the existing context/detail/hint rendering and the full error envelope - so no other command's output changes.
--private-key-file -reads the key from stdin, which means the passphrase cannot also come from stdin - stdin has already been drained. That conflict is rejected up front, and the rejection is keyed off the literal-recorded at parse time rather than off the identity of the returned stream:click.Filehands back a fresh stream object for-on every conversion, so anis-comparison againstclick.get_text_stream("stdin")holds underCliRunnerbut never in a real process, which would have left the check inert exactly where it matters.Secret handling is deliberately narrow: key material and passphrases are only ever read from a file, stdin, or a hidden prompt, never from a command-line value that would land in shell history and the process list.
--debugis refused onupload, because debug mode logs the raw request. The passphrase prompt only appears when a terminal is attached; without one the key is taken to be unencrypted rather than aborting on EOF, and under-F jsonthe prompt goes to stderr so stdout stays a single parseable document.Type of Change
Additional Notes
One fix in here is unrelated to GPG and worth flagging:
config.get_or_create_optionscaches the Options object in a thread-local, so state that is sticky by design (--debug) leaked from one test's CLI invocation into every later one in the same process. Three tests this branch doesn't touch (two intest_whoami.py, one intest_exceptions.py) fail without it, and any test running after one that passes--debugwas exposed - which test depends on ordering. An autouse fixture now clears the thread-local for every CLI test. Nothing about production behaviour changes - a real invocation only ever builds Options once.Verified against a live organisation on a throwaway repository (since deleted): reading the key, the typed confirmation both accepted and declined, a real
regenerateproducing a new fingerprint, both dry-runs, the non-terminal refusal, and the 400 and 404 error voices. The interactive confirmation was driven over a pty;CliRunnerhas no terminal, so the tests that exercise the prompt patch the terminal check explicitly.N/A: no screenshots - this is a CLI change, and the terminal output above covers it.