Skip to content

feat(no-ticket): add cloudsmith repos privileges command group - #389

Closed
BartoszBlizniak wants to merge 1 commit into
masterfrom
claude/latch-feedback-impl-4e1033
Closed

feat(no-ticket): add cloudsmith repos privileges command group#389
BartoszBlizniak wants to merge 1 commit into
masterfrom
claude/latch-feedback-impl-4e1033

Conversation

@BartoszBlizniak

Copy link
Copy Markdown
Member

Description

Adds a cloudsmith repos privileges command group for managing who has explicit access to a repository: teams, users and service accounts, all treated the same way.

Four subcommands, split by what they can take away:

Command Method Effect on access Prompts
list GET Read-only no
set PATCH Adds or raises only, anything unnamed is untouched no
revoke PUT Revokes the named teams, users and services yes
replace PUT The file becomes the whole truth, anything absent is revoked yes

Only revoke and replace can remove access, so only those two confirm before writing. Both take -y.

$ cloudsmith repos privileges set your-org/your-repo \
    --team your-team --service your-ci --privilege write

Granting Write on your-repo in the your-org namespace to team your-team, service your-ci ... OK

Type    | Name      | Privilege
Service | your-ci   | Write
Team    | your-team | Write

Results: 2 privileges
$ cloudsmith repos privileges revoke your-org/your-repo --team your-team --user someone-else

Getting list of repository privileges ... OK
No explicit privilege for user someone-else, skipping.
Revoke the privileges of team your-team on your-repo in the your-org namespace? [y/N]: y
Revoking the privileges of team your-team ... OK

Type | Name       | Privilege
User | their-user | Read

Results: 1 privilege
$ cloudsmith repos privileges list your-org/your-repo -F json \
    | jq '.data' \
    | cloudsmith repos privileges replace your-org/your-repo - -y

Three API constraints shaped the design, and are worth knowing before reviewing:

  • No paging. The endpoint returns every privilege in one response and ignores page parameters, so list deliberately offers no page options rather than advertising paging that never happens.
  • No single delete. There is no way to delete one privilege, so revoke reads the current set and writes back what is being kept. A concurrent change by someone else can therefore be lost, which the command's help says out loud.
  • One target per privilege. A privilege names a team or a user or a service, never two. That is why the table reads Type | Name | Privilege instead of a sparse column per kind, and why both set and replace reject an entry that names more than one.

Two things outside the command module:

  • cli/utils.py confirm_operation() now accepts an explicit empty prefix so a command can ask its question directly instead of through the shared "Are you absolutely certain you want to..." preamble. Existing callers pass no prefix and are unaffected.
  • cli/exceptions.py handle_api_exceptions() takes an optional summarise_error callable, which receives the exception and can decline by returning None. The privileges commands use it to turn the API's field-indexed 422 into one sentence and decline everything else, so a 403, a 404, or a 422 the API declines to explain all keep their status code and hint. Every other caller keeps the existing block unchanged.
# Before
Failed to set the repository privileges! (status: 422 - Unprocessable Entity)

Detail: Invalid input.
Privileges Field: Invalid team(s) specified ['no-such-team']

# After
Could not set privileges for your-repo: invalid team(s) specified ['no-such-team']

Type of Change

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

Additional Notes

Exercised end to end against a live organisation on a scratch repository, which is where the one real bug turned up: a listed privilege carries an explicit null for the two target kinds that do not apply, and the PUT endpoint rejects those nulls outright, so revoke failed on its write-back. Writes now send only the key that applies. Unit tests cover that shape directly, since a mocked response that omits the nulls would not have caught it.

Also verified by hand, because the unit tests only assert the shape and not the API's actual behaviour: set really is an upsert (naming one team leaves every other privilege alone), running revoke twice is a no-op that still exits 0, and list -F json | jq '.data' | replace - -y round-trips because listing and replacing share an entry shape.

set does not report that a named target already had the privilege it was given. The PATCH returns 204 with no after-state, so saying "already had this" would mean a GET before every write to produce an advisory line nobody acts on. It prints the entries it wrote rather than the repository's resulting state; list is there for that.

Two edges worth a reviewer's attention, both found by running this against a live org rather than by the tests:

  • replace with a file that lists nothing revokes every explicit privilege, and that includes the caller's own. On a private repository that means losing sight of it entirely, so the confirmation for an empty file drops the "replace all 0 privileges" phrasing and asks Revoke all explicit access to your-repo in the your-org namespace? instead.
  • replace your-org/your-repo - reads the document from stdin, which leaves nothing for the confirmation to read an answer from. It now refuses up front and asks for -y rather than aborting with no explanation.

revoke has to write the whole list back, so every privilege it keeps has to be one it can express. If it reads an entry it cannot (no principal, or a kind a future API adds), it refuses and points at replace rather than silently revoking it as a side effect. Declining either confirmation writes nothing and prints nothing, including under -F json; the paths that run to completion always emit a document.

Adds list/set/revoke/replace for the explicit team, user and service
account privileges on a repository. Only revoke and replace can take
access away, so only those two confirm before writing.

The API returns every privilege in one response and ignores page
parameters, so list offers no page options. It also has no way to delete
a single privilege, so revoke reads the current set and writes back what
is being kept.

confirm_operation() now accepts an explicit empty prefix so these
commands can ask their question directly, and handle_api_exceptions()
takes an optional summarise_error callable so the API's field-indexed
422 reads as one sentence naming the repository.

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

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 new cloudsmith repos privileges command group to manage explicit repository access (teams/users/service accounts) via list/set/revoke/replace workflows, aligned with the API’s whole-list semantics and lack of single-delete.
  • Improves CLI UX for destructive operations and API error rendering to better support scripting and clearer human output.

Changes:

  • Introduces new core API wrappers for repository privileges (list, patch, put) plus focused unit tests.
  • Adds repos privileges CLI commands (with JSON-friendly output behavior and confirmation for destructive operations).
  • Extends shared CLI utilities: optional empty confirmation prefix and optional one-line API error summarisation.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
cloudsmith_cli/core/tests/test_repos_privileges.py Adds core-level tests for privileges list/update/replace API wrappers.
cloudsmith_cli/core/api/repos.py Adds list_repo_privileges, update_repo_privileges, and replace_repo_privileges wrappers around SDK calls.
cloudsmith_cli/cli/utils.py Allows confirm_operation() to accept an explicit empty prefix (ask question directly).
cloudsmith_cli/cli/tests/test_utils.py Adds tests covering the new confirm_operation() prefix behavior.
cloudsmith_cli/cli/tests/test_exceptions.py Adds tests for the new one-sentence API error summarisation hook.
cloudsmith_cli/cli/tests/commands/test_repos_privileges.py Adds CLI command tests for list/set/revoke/replace behaviors and JSON/stderr semantics.
cloudsmith_cli/cli/exceptions.py Adds summarise_error hook to handle_api_exceptions() to optionally replace verbose 422 field blocks.
cloudsmith_cli/cli/commands/repos.py Implements the new repos privileges group and subcommands, including input validation and output rendering.
CHANGELOG.md Documents the new cloudsmith repos privileges feature under Unreleased.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +409 to +416
for name in names:
if not name.strip():
raise click.UsageError(f"Specify a slug for --{kind}.")

if (kind, name) in seen:
raise click.UsageError(f"Specified more than once: {kind} {name}.")
seen.add((kind, name))
targets.append((kind, name))
Comment on lines 40 to +43
context_msg = context_msg or "Failed to perform operation!"
detail, fields = get_details(exc)
hint = get_error_hint(ctx, opts, exc)
summary = summarise_error(exc, detail, fields) if summarise_error else None
@BartoszBlizniak
BartoszBlizniak deleted the claude/latch-feedback-impl-4e1033 branch August 25, 2026 16:01
@BartoszBlizniak

Copy link
Copy Markdown
Member Author

Superseded by #393 - the head branch was renamed to eng-14008-cli-add-repo-privileges-command, which closed this one. Same commit, same content.

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