Skip to content

feat(eng-14008): add cloudsmith repos privileges command group - #393

Merged
BartoszBlizniak merged 6 commits into
masterfrom
eng-14008-cli-add-repo-privileges-command
Aug 26, 2026
Merged

feat(eng-14008): add cloudsmith repos privileges command group#393
BartoszBlizniak merged 6 commits into
masterfrom
eng-14008-cli-add-repo-privileges-command

Conversation

@BartoszBlizniak

@BartoszBlizniak BartoszBlizniak commented Aug 25, 2026

Copy link
Copy Markdown
Member

Description

Adds cloudsmith repos privileges for managing who has explicit access to a repository. Teams, users and service accounts are all grantable, revocable and listed side by side.

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

Anything that can take access away confirms first, and nothing else does. All three writing commands 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

Changes outside the privileges commands

Two shared files change. Both are additive and no existing caller is affected, but they are worth a look because they sit outside the new command group.

  • 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. No existing caller passes prefix, so every other prompt is unchanged.
  • cli/exceptions.py - handle_api_exceptions() takes an optional summarise_error callable that can replace the context/detail/fields block with one sentence, or decline by returning None. Only the three new mutating commands pass it, and it is scoped to the API's field-indexed 422; a 403, a 404, or a 422 with no field detail all keep their status code and hint.
# 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']

Worth knowing before reviewing

  • set can lower access, so it reads before it writes. The PATCH endpoint sets a named target to whatever level it is given, in either direction, so set --privilege read on a team holding Admin is a revoke in all but name. set therefore reads the current privileges and asks before lowering one, naming who drops from what. Granting or raising never asks.
  • No paging on list. The endpoint returns everything in one response and ignores page parameters, so the command deliberately offers no page options.
  • revoke is read-modify-write. The API cannot delete a single privilege, so revoking reads the current set and writes back what is kept. A concurrent change by someone else can be lost, and an entry the CLI cannot express stops the command rather than being dropped silently.
  • An empty replace file revokes everything, including your own access. The confirmation says that in those words rather than the misleading "replace all 0 privileges".

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, which is where the two bugs the mocked tests could not have caught turned up: a listed privilege carries an explicit null for the kinds that do not apply and the write endpoints reject those nulls, and replace OWNER/REPO - consumed stdin for the document, leaving the confirmation with nothing to read so it aborted with no explanation. Writes now send only the key that applies, and reading from stdin asks for -y up front.

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

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 repository privilege management commands for teams, users, and service accounts.

Changes:

  • Adds list, set, revoke, and replace commands.
  • Adds API wrappers, validation, confirmations, and concise API errors.
  • Adds comprehensive CLI/API tests and changelog documentation.

Reviewed changes

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

Show a summary per file
File Description
CHANGELOG.md Documents privilege commands.
cloudsmith_cli/cli/commands/repos.py Implements the command group.
cloudsmith_cli/cli/exceptions.py Adds optional error summarization.
cloudsmith_cli/cli/utils.py Supports confirmation prompts without a prefix.
cloudsmith_cli/cli/tests/commands/test_repos_privileges.py Tests command behavior.
cloudsmith_cli/cli/tests/test_exceptions.py Tests summarized errors.
cloudsmith_cli/cli/tests/test_utils.py Tests confirmation prompts.
cloudsmith_cli/core/api/repos.py Adds privilege API wrappers.
cloudsmith_cli/core/tests/test_repos_privileges.py Tests API requests and errors.

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

Comment thread cloudsmith_cli/cli/commands/repos.py
BartoszBlizniak and others added 2 commits August 25, 2026 17:10
The privileges PATCH endpoint sets a named target to whatever level it is
given, in either direction, so `set --privilege read` on a team holding
Admin was a revoke with no confirmation, contradicting the documented
"adds or raises only" behaviour. Verified against a live org.

set now reads the current privileges, and asks before lowering one,
naming who drops from what. Granting or raising is unchanged and never
asks. -y skips the prompt like it does on revoke and replace.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@BartoszBlizniak
BartoszBlizniak marked this pull request as ready for review August 25, 2026 16:28
@BartoszBlizniak
BartoszBlizniak requested a review from a team as a code owner August 25, 2026 16:28
@cloudsmith-iduffy

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Co-authored-by: cloudsmith-iduffy <178375997+cloudsmith-iduffy@users.noreply.github.com>

Copilot AI commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved the merge conflicts by merging origin/master and fixing the CHANGELOG.md conflict in commit 9274ea8.

@tigh-latte tigh-latte 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.

pulled down, working well in my testing

BartoszBlizniak and others added 2 commits August 26, 2026 15:42
- privilege_rank now ranks an unrecognised non-empty level above Admin
  instead of below Read, so `set` over an unexpressible level always
  confirms rather than silently lowering it - the opposite of what the
  original ranking did.
- `set` skips the pre-write GET entirely when `-y` is passed, since its
  only purpose is powering the lowering confirmation that `-y` bypasses.
- `set`'s docstring now documents the same read-confirm-write race that
  `revoke`'s docstring already calls out.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@BartoszBlizniak
BartoszBlizniak merged commit 88a24e0 into master Aug 26, 2026
22 checks passed
@BartoszBlizniak
BartoszBlizniak deleted the eng-14008-cli-add-repo-privileges-command branch August 26, 2026 14:54
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.

5 participants