Re: [PATCH] osxkeychain: avoid incorrectly skipping store operation

>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > "Koji Nakamaru via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> >> +/*
> >> + * NOTE: We could use functions in strbuf.h and/or wrapper.h, but those
> >> + * introduce significant dependencies. Therefore, we define simplified
> >> + * versions here to keep this code self-contained.
> >> + */
> >
> > Sorry, but I do not quite understand this comment.  The program is
> > shipped as a part of Git, and using these functions and linking with
> > libgit.a may pull strbuf.o and some other *.o files out of libgit.a
> > to link with git-credential-osxkeychain.o to produce the executable,
> > but how can that be "significant dependencies"?  For anybody who is
> > building git-credential-osxkeychain, the necessary sources come for
> > free.
> >
> > It is not like we are forcing git-credential-osxkeychain to link
> > with a shared object libgit.so and making git-credential-osxkeychain
> > depend on it, or anything like that, which may require consumers of
> > binary distribution of git-credential-osxkeychain to also install
> > another package that has libgit.so in it (which is likely to be the
> > "git" package).  Even if it were the case (which is not), what good
> > would it be to have git-credential-osxkeychain on your system
> > without having git on the same system?

> The rest of the patch, excluding the poor-man's reimplementation of
> helper functions, looked like they match what the proposed log
> message described.
>
> It seems that credential material like username and password are
> included in plaintext as part of the state[], but is this a safe
> thing to do?  The keychain will give out the credential material in
> a way the requestor with sufficient priviledges can read, and this
> state[] is stored in the same place, so I am guessing that this is
> not adding any extra security concerns, but I just wanted to make
> sure you've considered any security implications.