Catalin Marinas <catalin.marinas@xxxxxxx> writes:
> Every time an option is set, the config file protection mode is changed
> to 0666 & ~umask even if it was different before. This patch is useful
> if people store passwords (SMTP server in the StGit case) and do not
> want others to read the .gitconfig file.
>
> Signed-off-by: Catalin Marinas <catalin.marinas@xxxxxxx>
> ---
> lockfile.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/lockfile.c b/lockfile.c
> index eb931ed..87ee233 100644
> --- a/lockfile.c
> +++ b/lockfile.c
> @@ -134,7 +138,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags)
> if (!(flags & LOCK_NODEREF))
> resolve_symlink(lk->filename, sizeof(lk->filename)-5);
> strcat(lk->filename, ".lock");
> - lk->fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666);
> + lk->fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, st.st_mode);
> if (0 <= lk->fd) {
> if (!lock_file_list) {
> sigchain_push_common(remove_lock_file_on_signal);
Your log message talks about .git/config and nothing else, but I think
this codepath affects everything that is created under the lock, such as
the index and refs.
Later in the function we call adjust_shared_perm(), and I had to wonder if
this change have an adverse effect in a shared repository setting.
For example, in a repository shared (with core.sharedrepository = true)
between users whose umask is 002, if somebody makes a ref unreadable by
others by mistake, the current code fixes the mistake in a later update,
but with your patch, the ref will kept unreadable.
This change in behaviour is justifiable only because the only thing the
user who said "core.sharedrepository = true" cares about is that refs are
readable by the group members (otherwise s/he would have used a more
explicit setting like "core.sharedrepository = 0660", and the
adjust_shared_perm() code will do the right thing, with or without your
patch).
The patch description must defend itself a bit better, perhaps by saying
something like this at the end.
This patch touches the codepath that affects not just .git/config
but other files like the index and the loose refs, so they also
inherit the original protection bits. In a private repository,
this is not an issue exactly because the repository is private,
In a shared repository, a later call made in this function to
adjust_shared_perm() widens the permission bits as configured.
Because adjust_shared_perm() is designed to do so from any mode
limited by user's umask, even though this patch changes the
behaviour in the strict sense, it should not affect the outcome in
a negative way and what is explicitly marked as allowed in the
configuration will still be allowed.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html