What should gh do if `gitcreds_get()` returns a password that is not a PAT?

[gaborcsardi]

• Error?
• Warn?

Sending a password that is not a PAT can be a security issue, although we are sending it to the very same host we would send the PAT to, so maybe not a very serious issue?

Also, at the time of writing we don't know any situation when sending a non-PAT password in the Authorization header would do anything useful.

So probably we should error?

[jennybc]

gh's validation around the gh_pat class would catch a lot of non-PAT creds. You probably already thought of that.

[gaborcsardi]

Yeah, AFAICT right now you get a silent "no pat". Should this be an error or a warning instead?

[jennybc]

Although it's part of a user-facing API for putting creds into the store, credentials certainly tries to catch this case:

https://github.com/r-lib/credentials/blob/c86ae4b43771c5096a45265bbe4006f69a30237f/R/github-pat.R#L41

and even goes on to reject that cred (presumably to make sure there's no entry with a real username + a PAT in the store).

[gaborcsardi]

I think it would reasonable to do that in gh and usethis, when you need a GH PAT. So maybe gh should have a set_pat() function?

But the git credential store is actually used for a lot of things already, not just PATs, so I don't think gitcreds should have such restrictions.

[jennybc]

So maybe gh should have a set_pat() function?

Yes, I think so. I think usethis may also need one. I think any package that pulls from the store potentially needs to offer a humane way to put something there as well. You can, of course, recommend a function in another package for this. But with usethis, for example, we generally try to provide lots of scaffolding and make usethis feel like the storefront.

[gaborcsardi]

I think any package that pulls from the store potentially needs to offer a humane way to put something there as well.

Yeah, gitcreds_set() can put anything there. The question is whether to have one specialized to PATs.

[jennybc]

I think our current gh_pat validation would be a good bare minimum for a function in gh and/or usethis that helps a user store a PAT.

https://github.com/r-lib/gh/blob/d41174a449daf6906dd6fefd25ba8494969a30c5/R/gh_token.R#L63-L84

Which, I think, suggests that gh should offer this and usethis should recommend or wrap that. Because the gh_pat validation stuff won't be exported.

[gaborcsardi]

Closed wrong issue....

[gaborcsardi]

@jennybc Do you remember if we wanted to do anything more about this? Or it is fine as it is now?

[jennybc]

I have definitely heard of people who enter a password when asked for a PAT, then are confused when things don't work. So I'm still a general fan of trying to catch this and emit a message that suggests what might be wrong.

[Sumidu]

I was really confused about this error. Since github changed the format of the PATs creating a new one does not create a legal PAT for gh. However, updating gh helped. It was confusing for me (and others) because updating usethis and git_creds did not help. Which were the only packages we were actively using.

TLDR: update gh, git_creds, and usethis