diff --git a/commit-graph.c b/commit-graph.c index 6dc777e2f3..aa3adb912f 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -1576,7 +1576,8 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) if (ctx->split) { char *lock_name = get_chain_filename(ctx->odb); - hold_lock_file_for_update(&lk, lock_name, LOCK_DIE_ON_ERROR); + hold_lock_file_for_update_mode(&lk, lock_name, + LOCK_DIE_ON_ERROR, 0444); fd = git_mkstemp_mode(ctx->graph_name, 0444); if (fd < 0) { @@ -1584,9 +1585,16 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) return -1; } + if (adjust_shared_perm(ctx->graph_name)) { + error(_("unable to adjust shared permissions for '%s'"), + ctx->graph_name); + return -1; + } + f = hashfd(fd, ctx->graph_name); } else { - hold_lock_file_for_update(&lk, ctx->graph_name, LOCK_DIE_ON_ERROR); + hold_lock_file_for_update_mode(&lk, ctx->graph_name, + LOCK_DIE_ON_ERROR, 0444); fd = lk.tempfile->fd; f = hashfd(lk.tempfile->fd, lk.tempfile->filename.buf); } diff --git a/lockfile.c b/lockfile.c index 8e8ab4f29f..cc9a4b8428 100644 --- a/lockfile.c +++ b/lockfile.c @@ -70,7 +70,8 @@ static void resolve_symlink(struct strbuf *path) } /* Make sure errno contains a meaningful value on error */ -static int lock_file(struct lock_file *lk, const char *path, int flags) +static int lock_file(struct lock_file *lk, const char *path, int flags, + int mode) { struct strbuf filename = STRBUF_INIT; @@ -79,7 +80,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags) resolve_symlink(&filename); strbuf_addstr(&filename, LOCK_SUFFIX); - lk->tempfile = create_tempfile(filename.buf); + lk->tempfile = create_tempfile_mode(filename.buf, mode); strbuf_release(&filename); return lk->tempfile ? lk->tempfile->fd : -1; } @@ -99,7 +100,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags) * exactly once. If timeout_ms is -1, try indefinitely. */ static int lock_file_timeout(struct lock_file *lk, const char *path, - int flags, long timeout_ms) + int flags, long timeout_ms, int mode) { int n = 1; int multiplier = 1; @@ -107,7 +108,7 @@ static int lock_file_timeout(struct lock_file *lk, const char *path, static int random_initialized = 0; if (timeout_ms == 0) - return lock_file(lk, path, flags); + return lock_file(lk, path, flags, mode); if (!random_initialized) { srand((unsigned int)getpid()); @@ -121,7 +122,7 @@ static int lock_file_timeout(struct lock_file *lk, const char *path, long backoff_ms, wait_ms; int fd; - fd = lock_file(lk, path, flags); + fd = lock_file(lk, path, flags, mode); if (fd >= 0) return fd; /* success */ @@ -169,10 +170,11 @@ NORETURN void unable_to_lock_die(const char *path, int err) } /* This should return a meaningful errno on failure */ -int hold_lock_file_for_update_timeout(struct lock_file *lk, const char *path, - int flags, long timeout_ms) +int hold_lock_file_for_update_timeout_mode(struct lock_file *lk, + const char *path, int flags, + long timeout_ms, int mode) { - int fd = lock_file_timeout(lk, path, flags, timeout_ms); + int fd = lock_file_timeout(lk, path, flags, timeout_ms, mode); if (fd < 0) { if (flags & LOCK_DIE_ON_ERROR) unable_to_lock_die(path, errno); diff --git a/lockfile.h b/lockfile.h index 9843053ce8..db93e6ba73 100644 --- a/lockfile.h +++ b/lockfile.h @@ -90,6 +90,15 @@ * functions. In particular, the state diagram and the cleanup * machinery are all implemented in the tempfile module. * + * Permission bits + * --------------- + * + * If you call either `hold_lock_file_for_update_mode` or + * `hold_lock_file_for_update_timeout_mode`, you can specify a suggested + * mode for the underlying temporary file. Note that the file isn't + * guaranteed to have this exact mode, since it may be limited by either + * the umask, 'core.sharedRepository', or both. See `adjust_shared_perm` + * for more. * * Error handling * -------------- @@ -156,12 +165,20 @@ struct lock_file { * file descriptor for writing to it, or -1 on error. If the file is * currently locked, retry with quadratic backoff for at least * timeout_ms milliseconds. If timeout_ms is 0, try exactly once; if - * timeout_ms is -1, retry indefinitely. The flags argument and error - * handling are described above. + * timeout_ms is -1, retry indefinitely. The flags argument, error + * handling, and mode are described above. */ -int hold_lock_file_for_update_timeout( +int hold_lock_file_for_update_timeout_mode( struct lock_file *lk, const char *path, - int flags, long timeout_ms); + int flags, long timeout_ms, int mode); + +static inline int hold_lock_file_for_update_timeout( + struct lock_file *lk, const char *path, + int flags, long timeout_ms) +{ + return hold_lock_file_for_update_timeout_mode(lk, path, flags, + timeout_ms, 0666); +} /* * Attempt to create a lockfile for the file at `path` and return a @@ -175,6 +192,13 @@ static inline int hold_lock_file_for_update( return hold_lock_file_for_update_timeout(lk, path, flags, 0); } +static inline int hold_lock_file_for_update_mode( + struct lock_file *lk, const char *path, + int flags, int mode) +{ + return hold_lock_file_for_update_timeout_mode(lk, path, flags, 0, mode); +} + /* * Return a nonzero value iff `lk` is currently locked. */ diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh index 39e2918a32..424599959c 100755 --- a/t/t5318-commit-graph.sh +++ b/t/t5318-commit-graph.sh @@ -14,6 +14,10 @@ test_expect_success 'setup full repo' ' test_oid_init ' +test_expect_success POSIXPERM 'tweak umask for modebit tests' ' + umask 022 +' + test_expect_success 'verify graph with no graph file' ' cd "$TRASH_DIRECTORY/full" && git commit-graph verify @@ -98,6 +102,13 @@ test_expect_success 'write graph' ' graph_read_expect "3" ' +test_expect_success POSIXPERM 'write graph has correct permissions' ' + test_path_is_file $objdir/info/commit-graph && + echo "-r--r--r--" >expect && + test_modebits $objdir/info/commit-graph >actual && + test_cmp expect actual +' + graph_git_behavior 'graph exists' full commits/3 commits/1 test_expect_success 'Add more commits' ' @@ -423,7 +434,8 @@ GRAPH_BYTE_FOOTER=$(($GRAPH_OCTOPUS_DATA_OFFSET + 4 * $NUM_OCTOPUS_EDGES)) corrupt_graph_setup() { cd "$TRASH_DIRECTORY/full" && test_when_finished mv commit-graph-backup $objdir/info/commit-graph && - cp $objdir/info/commit-graph commit-graph-backup + cp $objdir/info/commit-graph commit-graph-backup && + chmod u+w $objdir/info/commit-graph } corrupt_graph_verify() { @@ -437,6 +449,7 @@ corrupt_graph_verify() { fi && git status --short && GIT_TEST_COMMIT_GRAPH_DIE_ON_LOAD=true git commit-graph write && + chmod u+w $objdir/info/commit-graph && git commit-graph verify } diff --git a/t/t5324-split-commit-graph.sh b/t/t5324-split-commit-graph.sh index 594edb7307..269d0964a3 100755 --- a/t/t5324-split-commit-graph.sh +++ b/t/t5324-split-commit-graph.sh @@ -37,6 +37,10 @@ graph_read_expect() { test_cmp expect output } +test_expect_success POSIXPERM 'tweak umask for modebit tests' ' + umask 022 +' + test_expect_success 'create commits and write commit-graph' ' for i in $(test_seq 3) do @@ -401,4 +405,24 @@ test_expect_success ULIMIT_FILE_DESCRIPTORS 'handles file descriptor exhaustion' ) ' +while read mode modebits +do + test_expect_success POSIXPERM "split commit-graph respects core.sharedrepository $mode" ' + rm -rf $graphdir $infodir/commit-graph && + git reset --hard commits/1 && + test_config core.sharedrepository "$mode" && + git commit-graph write --split --reachable && + ls $graphdir/graph-*.graph >graph-files && + test_line_count = 1 graph-files && + echo "$modebits" >expect && + test_modebits $graphdir/graph-*.graph >actual && + test_cmp expect actual && + test_modebits $graphdir/commit-graph-chain >actual && + test_cmp expect actual + ' +done <<\EOF +0666 -r--r--r-- +0600 -r-------- +EOF + test_done diff --git a/t/t6600-test-reach.sh b/t/t6600-test-reach.sh index b24d850036..475564bee7 100755 --- a/t/t6600-test-reach.sh +++ b/t/t6600-test-reach.sh @@ -51,8 +51,10 @@ test_expect_success 'setup' ' done && git commit-graph write --reachable && mv .git/objects/info/commit-graph commit-graph-full && + chmod u+w commit-graph-full && git show-ref -s commit-5-5 | git commit-graph write --stdin-commits && mv .git/objects/info/commit-graph commit-graph-half && + chmod u+w commit-graph-half && git config core.commitGraph true ' diff --git a/tempfile.c b/tempfile.c index d43ad8c191..94aa18f3f7 100644 --- a/tempfile.c +++ b/tempfile.c @@ -130,17 +130,17 @@ static void deactivate_tempfile(struct tempfile *tempfile) } /* Make sure errno contains a meaningful value on error */ -struct tempfile *create_tempfile(const char *path) +struct tempfile *create_tempfile_mode(const char *path, int mode) { struct tempfile *tempfile = new_tempfile(); strbuf_add_absolute_path(&tempfile->filename, path); tempfile->fd = open(tempfile->filename.buf, - O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC, 0666); + O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC, mode); if (O_CLOEXEC && tempfile->fd < 0 && errno == EINVAL) /* Try again w/o O_CLOEXEC: the kernel might not support it */ tempfile->fd = open(tempfile->filename.buf, - O_RDWR | O_CREAT | O_EXCL, 0666); + O_RDWR | O_CREAT | O_EXCL, mode); if (tempfile->fd < 0) { deactivate_tempfile(tempfile); return NULL; diff --git a/tempfile.h b/tempfile.h index cddda0a33c..4de3bc77d2 100644 --- a/tempfile.h +++ b/tempfile.h @@ -88,8 +88,16 @@ struct tempfile { * Attempt to create a temporary file at the specified `path`. Return * a tempfile (whose "fd" member can be used for writing to it), or * NULL on error. It is an error if a file already exists at that path. + * Note that `mode` will be further modified by the umask, and possibly + * `core.sharedRepository`, so it is not guaranteed to have the given + * mode. */ -struct tempfile *create_tempfile(const char *path); +struct tempfile *create_tempfile_mode(const char *path, int mode); + +static inline struct tempfile *create_tempfile(const char *path) +{ + return create_tempfile_mode(path, 0666); +} /* * Register an existing file as a tempfile, meaning that it will be