Skip to content

fix(no-ticket): keep credential-helper list running on a bad config file - #549

Open
cloudsmith-iduffy wants to merge 1 commit into
masterfrom
iduffy/fix-helper-list-bad-config
Open

cloudsmith-iduffy wants to merge 1 commit into
masterfrom
iduffy/fix-helper-list-bad-config

Conversation

@cloudsmith-iduffy

Copy link
Copy Markdown
Collaborator

Description

cloudsmith credential-helper list stopped with an error when one helper's config file was odd, and the other helpers were not listed at all.

Before, with ~/.docker/config.json set to {"credHelpers": ["x"]}:

$ cloudsmith credential-helper list
Error: AttributeError: 'list' object has no attribute 'items'

After:

$ cloudsmith credential-helper list
cargo  (Cargo credential provider for Cloudsmith registries)
  launcher : not installed
  hosts    : none configured
docker  (Docker credential helper for Cloudsmith registries)
  launcher : not installed
  hosts    : none configured
pnpm  (pnpm credential helper for Cloudsmith registries)
  launcher : not installed
  hosts    : none configured
terraform  (Terraform credentials helper for Cloudsmith registries)
  launcher : not installed
  hosts    : none configured

I checked the status() method of all four helpers with bad input: wrong JSON/TOML types, files that are not UTF-8, directories in place of files, and files with no read permission. These cases crashed:

  • Docker: a credHelpers value that is not an object (list, string, null), and a config.json that is not UTF-8.
  • pnpm: an .npmrc that is not UTF-8, that is a directory, or that the CLI cannot read.
  • Terraform: a .terraformrc that is not UTF-8.

Cargo already handled all of these. Each status() now reports no hosts for a bad file, so list shows that helper as not configured and continues.

Type of Change

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

Additional Notes

The change stays inside the status() methods. It does not touch uninstall or merge_config_file, which other open PRs change.

🤖 Generated with Claude Code

A Docker credHelpers value that is not an object stopped the command
with an AttributeError. A Docker, pnpm, or Terraform config file that is
not UTF-8, or a pnpm .npmrc that the CLI cannot read, also stopped it.
The status() method of each helper now reports no hosts for these files.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 24, 2026 17:51

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.

🟢 Approval recommended

The localized error handling preserves valid-configuration behavior, with regression coverage and no identified blocking issues.

No open issues 🎉

What changed in this PR

Keeps cloudsmith credential-helper list running when a helper’s configuration is malformed or unreadable, so other helpers remain visible.

Changes:

  • Adds defensive configuration handling to Docker, pnpm, and Terraform status checks.
  • Adds regression tests confirming bad configurations report no hosts without interrupting the list.
  • Documents the fix in the changelog.
File Description
cloudsmith_cli/​credential_helpers/​terraform/​installer.py Handles invalid UTF-8 during status checks.
cloudsmith_cli/​credential_helpers/​pnpm/​installer.py Handles unreadable or undecodable .npmrc files.
cloudsmith_cli/​credential_helpers/​docker/​installer.py Guards malformed credHelpers values and decoding errors.
cloudsmith_cli/​cli/​tests/​commands/​test_credential_helper_install.py Tests continued listing with bad helper configurations.
CHANGELOG.md Describes the corrected listing behavior.

Review effort: Balanced

@cloudsmith-iduffy
cloudsmith-iduffy marked this pull request as ready for review October 24, 2026 17:58
@cloudsmith-iduffy
cloudsmith-iduffy requested a review from a team as a code owner October 24, 2026 17:58

This branch has not been deployed

No deployments
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