Skip to content

fix(no-ticket): stop 401 hint from asserting a permissions problem - #352

Open
BartoszBlizniak wants to merge 4 commits into
cloudsmith-io:masterfrom
BartoszBlizniak:issue-77-401-error-hint
Open

fix(no-ticket): stop 401 hint from asserting a permissions problem#352
BartoszBlizniak wants to merge 4 commits into
cloudsmith-io:masterfrom
BartoszBlizniak:issue-77-401-error-hint

Conversation

@BartoszBlizniak

Copy link
Copy Markdown
Member

Description

Fixes the misleading credentialed 401 hint from issue #77, plus an exit-code bug found while verifying it.

The hint. A 401 tells the CLI that authentication failed; it does not tell it why. The old hint asserted one specific cause — "you don't have the permission to perform this action" — which is 403 semantics, contradicts the 401 - Unauthorized status printed directly above it, and sends users chasing the wrong kind of bug (exactly what the issue reporter describes). get_401_error_hint now names the possible causes without picking one: This usually means your API key is invalid, expired, or lacks access to this resource - check your credentials and try again. The SSO-token, token-command, and no-credential branches are unchanged.

The exit codes. Writing the renderer tests surfaced that CliRunner reports exit_code == 0 for a 401. That turned out to be half artefact, half real bug. AliasGroup.main (cloudsmith_cli/cli/command.py:138) runs click with standalone_mode=False so it can render click's own errors as JSON, so click returns the code passed to ctx.exit() instead of raising SystemExit. Every entrypoint therefore has to wrap main() in sys.exit(). Three of the four did; python -m cloudsmith_cli called it bare and always exited 0, so a failed push or an unauthorised request looked like success to CI and to scripts.

EntrypointBeforeAfter
cloudsmith console script (pip / uv / pipx)sys.exit(main()) in the installer-generated scriptunchanged
Standalone binaries (PyInstaller)sys.exit(main()) in packaging/pyinstaller/entry.pyunchanged
Homebrewbin.write_exec_scriptexecs the bundled binary and inherits its statusunchanged
python -m cloudsmith_clibare main()always exited 0sys.exit(main())

This is the same bug #328 fixed for the PyInstaller entry point; __main__.py was the one caller it missed. AGENTS.md:21 documents python -m cloudsmith_cli ... as a supported invocation.

Type of Change

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

Additional Notes

Tests: test_exceptions.py asserts the complete hint text and renders it through the registered command tree in both text and JSON output, with --config-file and --credentials-file pointed at pytest's tmp_path so a real config on the machine cannot influence the result. test_entrypoint_exit_codes.py covers the exit-code contract for all three entrypoints — main() returns the status, python -m raises SystemExit(401), and the non-importable PyInstaller entry still wraps main() in sys.exit(). Reverting __main__.py to a bare main() fails the python -m test. The binaries stay covered by packaging/smoketest.sh:114, which already requires whoami against an unreachable API to exit non-zero.

Verified end to end against a local server returning HTTP 401: both python -m cloudsmith_cli whoami and the cloudsmith console script exit 145, which is 401 & 0xFF after POSIX truncation. Passing the raw status to sys.exit() is pre-existing behaviour and stays non-zero for every status the CLI can raise (403→147, 404→148, 422→166, 429→173, 500→244); no real HTTP status is a multiple of 256.

Full suite: 633 passed, 40 skipped. pre-commit hooks pass, including ruff format, ruff check, typos, and the credential/private-key detectors.

BartoszBlizniakand others added 4 commits August 12, 2026 23:38
A 401 means authentication failed, not that authorization was denied,
so the hint for a credentialed 401 shouldn't claim it's specifically a
permissions issue -- that contradicts the "Unauthorized" status line
it's paired with and sends users chasing the wrong kind of bug
(issue cloudsmith-io#77). Reworded to point at the credential itself (invalid,
expired, or lacking access) without asserting which of those it is.
The command-tree tests pinned --config-file at /dev/null, which is
POSIX-only and left credentials.ini discovery untouched, so a real
credentials file on the machine could feed the run. Both paths now
point at pytest's tmp_path, and the tests use the shared runner
fixture and class layout that cli/tests/commands/test_whoami.py
already uses. CHANGELOG gains the Unreleased entry the wording change
should have carried.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
AliasGroup.main runs click with standalone_mode=False so it can render
click's own errors as JSON, which means click returns the code passed
to ctx.exit() instead of raising SystemExit. The module entrypoint
called main() bare and discarded that return value, so a failed push
or an unauthorised request exited 0 -- CI and scripts saw a failure as
success. The console script and the PyInstaller entry point already
wrapped main() in sys.exit(); the module now does too.
Verified end to end against a local server returning 401: both
`python -m cloudsmith_cli whoami` and the console script exit 145
(401 & 0xFF). No HTTP status the CLI can raise truncates to 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

CopilotAI 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

  • This PR improves the Cloudsmith CLI’s user-facing handling of HTTP 401 responses by removing a misleading “permissions” assertion from the credentialed hint, and fixes python -m cloudsmith_cli to correctly propagate non-zero exit codes (matching the other shipped entrypoints).

Changes:

  • Update the 401 “credential present” hint to describe likely causes (invalid/expired/insufficient access) without asserting a 403-style permissions error.
  • Wrap cloudsmith_cli.__main__ with sys.exit(main()) so python -m cloudsmith_cli reports failures via exit status.
  • Add tests covering both the rendered hint (text + JSON) and the exit-code contract across entrypoints.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
FileDescription
cloudsmith_cli/cli/tests/test_exceptions.pyAdds assertions for the updated 401 hint and verifies it survives both text and JSON renderers via real command invocation.
cloudsmith_cli/cli/tests/test_entrypoint_exit_codes.pyAdds regression tests ensuring main()’s returned status is propagated by python -m and that the PyInstaller entrypoint wraps main() in sys.exit().
cloudsmith_cli/cli/exceptions.pyUpdates the credentialed 401 hint text to avoid implying a specific authorization failure.
cloudsmith_cli/main.pyFixes module entrypoint to sys.exit(main()) so failures are non-zero when invoked with python -m.
CHANGELOG.mdDocuments both fixes under Unreleased.

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

CopilotAI 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

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

cloudsmith_cli/cli/tests/test_entrypoint_exit_codes.py:46

  • The new helper name and docstring use British “unauthorised”; the project guideline requires Americanized spelling. Rename this to unauthorized_api and update its call sites in this test.
def unauthorised_api():
"""Patch the whoami API call to raise a 401."""

cloudsmith_cli/cli/tests/test_entrypoint_exit_codes.py:31

  • The new helper name and docstring use British “unauthorised”; the project guideline requires Americanized spelling. Rename this to unauthorized_args and update its call sites in this test.

This issue also appears on line 45 of the same file.

def unauthorised_args(config_dir):
"""Return args for a whoami that will raise a 401, isolated from real config."""

CHANGELOG.md:12

  • Use the project’s required Americanized spelling: “unauthorized.”
- `python -m cloudsmith_cli` now exits non-zero when a command fails. `AliasGroup.main` runs click with `standalone_mode=False` so click returns the exit code from `ctx.exit()` rather than raising `SystemExit`, and the module entrypoint discarded that return value — so a failed push, or an unauthorised request, exited 0. The `cloudsmith` console script and the standalone binaries already wrapped `main()` in `sys.exit()` and were unaffected.

@BartoszBlizniak
BartoszBlizniak marked this pull request as ready for review August 14, 2026 12:09
@BartoszBlizniak
BartoszBlizniak requested a review from a team as a code ownerAugust 14, 2026 12:09
Sign up for freeto 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.

3 participants

@BartoszBlizniak@cloudsmith-iduffy