Skip to content

fix: add error for unsupported credential provider version #12590

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

arlosi
Copy link
Contributor

@arlosi arlosi commented Aug 29, 2023

Cargo currently ignores the version in the CredentialHello message, and proceeds to use version 1 regardless of what the credential provider claims it can support.

This change does the following:

  • Adds a new error if Cargo doesn't support any of the supported protocol versions offered by the provider.
  • Kills the credential provider subprocess if it fails. This prevents it from hanging or printing spurious errors such as "broken pipe" when it's attempting to read the next JSON message.
  • Adds a new test for an unsupported credential provider protocol.

@rustbot
Copy link
Collaborator

rustbot commented Aug 29, 2023

r? @ehuss

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-registry-authentication Area: registry authentication and authorization (authn authz) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2023
@arlosi arlosi added the A-credential-provider Area: credential provider for storing and retreiving credentials label Aug 29, 2023
Copy link
Contributor

@epage epage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to r= me after the version bump

@arlosi arlosi force-pushed the cred-unsupported-error branch from 24e4214 to 286f350 Compare August 29, 2023 21:00
@arlosi
Copy link
Contributor Author

arlosi commented Aug 29, 2023

@bors r=epage

@bors
Copy link
Contributor

bors commented Aug 29, 2023

📌 Commit 286f350 has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 29, 2023
@bors
Copy link
Contributor

bors commented Aug 29, 2023

⌛ Testing commit 286f350 with merge e45f5d2...

bors added a commit that referenced this pull request Aug 29, 2023
fix: add error for unsupported credential provider version

Cargo currently ignores the version in the `CredentialHello` message, and proceeds to use version `1` regardless of what the credential provider claims it can support.

This change does the following:
* Adds a new error if Cargo doesn't support any of the supported protocol versions offered by the provider.
* Kills the credential provider subprocess if it fails. This prevents it from hanging or printing spurious errors such as "broken pipe" when it's attempting to read the next JSON message.
* Adds a new test for an unsupported credential provider protocol.
@bors
Copy link
Contributor

bors commented Aug 29, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 29, 2023
@arlosi arlosi force-pushed the cred-unsupported-error branch from 286f350 to 39db61e Compare August 30, 2023 02:22
@arlosi
Copy link
Contributor Author

arlosi commented Aug 30, 2023

@bors r=epage

@bors
Copy link
Contributor

bors commented Aug 30, 2023

📌 Commit 39db61e has been approved by epage

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 30, 2023
@bors
Copy link
Contributor

bors commented Aug 30, 2023

⌛ Testing commit 39db61e with merge 40f1f67...

@bors
Copy link
Contributor

bors commented Aug 30, 2023

☀️ Test successful - checks-actions
Approved by: epage
Pushing 40f1f67 to master...

@bors bors merged commit 40f1f67 into rust-lang:master Aug 30, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 6, 2023
Update cargo

21 commits in 96fe1c9e1aecd8f57063e3753969bb6418fd2fd5..d14c85f4e6e7671673b1a1bc87231ff7164761e1
2023-08-29 20:10:34 +0000 to 2023-09-05 22:28:10 +0000
- fix(resolver): Make resolver behavior independent of package order (rust-lang/cargo#12602)
- cargo-credential: change serialization of cache expiration (rust-lang/cargo#12622)
- Update registry-web-api.md yank/unyank comments (rust-lang/cargo#12619)
- test: new options of debuginfo are no longer unstable (rust-lang/cargo#12618)
- use split_once for cleaner code (rust-lang/cargo#12615)
- stop using lazy_static (rust-lang/cargo#12616)
- doc: adjust all doc headings one level up (rust-lang/cargo#12595)
- chore(deps): update compatible (rust-lang/cargo#12609)
- chore(deps): update rust crate cargo_metadata to 0.17.0 (rust-lang/cargo#12610)
- Prepare for partial-version package specs (rust-lang/cargo#12591)
- refactor: Use more serde_untagged (rust-lang/cargo#12581)
- fix(cli): Help users know possible `--target` values (rust-lang/cargo#12607)
- Tab completion for --target uses rustup but fallsback to rustc (rust-lang/cargo#12606)
- Fewer temporary needless strings (rust-lang/cargo#12604)
- fix(help): Provide better commands heading for styling (rust-lang/cargo#12593)
- fix(update): Clarify meaning of --aggressive as --recursive (rust-lang/cargo#12544)
- docs(changelog): Clarify language for Cargo.lock policy (rust-lang/cargo#12601)
- fix typo: "default branch branch" -> "default branch" (rust-lang/cargo#12598)
- fix: add error for unsupported credential provider version (rust-lang/cargo#12590)
- fix(help): Explain --explain (rust-lang/cargo#12592)
- fix(help): Remove redundant information from new/init (rust-lang/cargo#12594)

r? ghost
@ehuss ehuss added this to the 1.74.0 milestone Sep 30, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-credential-provider Area: credential provider for storing and retreiving credentials A-registry-authentication Area: registry authentication and authorization (authn authz) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants