Subject: Re: [PATCH] Revert "osxkeychain: state to skip unnecessary store operations"

From: "brian m. carlson" <[email-masked]>
Date: Thu, 13 Nov 2025 23:30:54 +0000

On 2025-11-12 at 07:01:21, Koji Nakamaru via GitGitGadget wrote:
> From: Koji Nakamaru <[email-masked]>
> 
> This reverts commit e1ab45b2dab51f94db9548666dfd7af626d2aa7e.
> 
> That commit was trying to skip to store a credential returned by
> "git-credential-osxkeychain get" by setting
> "state[]=osxkeychain:seen=1". However, this state[] is kept even if a
> credential returned by "git-credential-osxkeychain get" is invalid and
> another subsequent helper's "get" returns a valid credential. Another
> subsequent helper (such as [1]) may expect git-credential-osxkeychain to
> store the valid credential so that "store" cannot be skipped by just
> checking "state[]=osxkeychain:seen=1".

I believe the intended approach here is that if we do a get and the
credential is invalid, we return the same state[] header to erase, but
we should not send it to subsequent gets for a new credential.  However,
we do need to send it to subsequent gets (which will not have an
intervening erase) if this is a multistage request because otherwise
multistage requests will not be able to keep state, which NTLM and
Kerberos require.  Does that make sense?

My guess is that the problem here is that we reuse the credential
structure without resetting it somewhere in the HTTP code rather than a
problem in this particular helper.  That is probably my fault, but in my
defence I would not say that the structure of the HTTP code is very easy
to follow.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA