> It is unreasonable to drop the write-enable bit of > a file in a writable directory and expect it to stay unmodified. The > W-bit on the file is not usable as a security measure, and we do not > use it as such. The point here is not a matter of security - it is of expectations. When a user drops write access on the global ~/.gitconfig I think a reasonable user would expect future `git config --global` calls to fail by default. The possibility of an override is a different matter, and my initial proposal included the details of enabling direct override. I don't think there is any presumption that this is a security related discussion. > I do not offhand know how much a new feature "this repository can be > modified by pushing into and fetching from, but its configuration > cannot be modified" is a sensible thing to have. I agree that per-repository files almost never run into an issue with this. Our problem is strictly with the global ~/.gitconfig which in our use case is owned by a shared system account and used implicitly by many developers. Thus any one of those devs can call `git config` without any signal that they are changing something that ought not to be changed and should think carefully. This would be equivalent to dropping write access to a file that your account owns so that vi / emacs / etc.. will warn that the file is read-only before modifying it (useful for any number of sensitive files). Obviously from a security perspective you have a number of means of potential override, however all require additional steps that surface the initial intention that the file should not change - or should only change rarely after additional confirmation. > perhaps the lock_file() > function can have access(path, W_OK) check before it returns a > tempfile that has been successfully opened? That sounds ideal On Tue, Nov 8, 2016 at 8:22 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Jeff King <peff@xxxxxxxx> writes: > >> Probably converting "rename(from, to)" to first check "access(to, >> W_OK)". That's racy, but it's the best we could do. > > Hmph, if these (possibly problematic) callers are all following the > usual "lock, write to temp, rename" pattern, perhaps the lock_file() > function can have access(path, W_OK) check before it returns a > tempfile that has been successfully opened? > > Having said that, I share your assessment that this is not a code or > design problem. It is unreasonable to drop the write-enable bit of > a file in a writable directory and expect it to stay unmodified. The > W-bit on the file is not usable as a security measure, and we do not > use it as such. > > I do not offhand know how much a new feature "this repository can be > modified by pushing into and fetching from, but its configuration > cannot be modified" is a sensible thing to have. But it is quite > clear that even if we were to implement such feature, we wouldn't be > using W-bit on .git/config to signal that. >