From f93b7946eaa7265468fa502ef391215f9d9739ce Mon Sep 17 00:00:00 2001
From: Eric Wong <e@80x24.org>
Date: Tue, 21 Mar 2023 23:07:42 +0000
Subject: [PATCH] cindex: respect existing permissions

For internal ($GIT_DIR/public-inbox-cindex) Xapian DBs, we can
rely on core.sharedRepository.  For external ones, we'll just
rely on existing permissions if the directory already exists.
---
 lib/PublicInbox/CodeSearchIdx.pm | 29 ++++++++++++++++++++++++++++-
 t/cindex.t                       | 22 +++++++++++++++++++---
 2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/lib/PublicInbox/CodeSearchIdx.pm b/lib/PublicInbox/CodeSearchIdx.pm
index 21c43973d..704baa9c2 100644
--- a/lib/PublicInbox/CodeSearchIdx.pm
+++ b/lib/PublicInbox/CodeSearchIdx.pm
@@ -710,8 +710,35 @@ sub init_tmp_git_dir ($) {
 	$TMP_GIT->{-tmp} = $tmp;
 }
 
+sub prep_umask ($) {
+	my ($self) = @_;
+	my $um;
+	my $cur = umask;
+	if ($self->{-internal}) { # respect core.sharedRepository
+		@{$self->{git_dirs}} == 1 or die 'BUG: only for GIT_DIR';
+		# yuck, FIXME move umask handling out of inbox-specific stuff
+		require PublicInbox::InboxWritable;
+		my $git = PublicInbox::Git->new($self->{git_dirs}->[0]);
+		chomp($um = $git->qx('config', 'core.sharedRepository') // '');
+		$um = PublicInbox::InboxWritable::_git_config_perm(undef, $um);
+		$um = PublicInbox::InboxWritable::_umask_for($um);
+		umask == $um or progress($self, 'umask from git: ',
+						sprintf('0%03o', $um));
+	} elsif (-d $self->{cidx_dir}) { # respect existing perms
+		my @st = stat(_);
+		$um = (~$st[2] & 0777);
+		umask == $um or progress($self, 'using umask from ',
+						$self->{cidx_dir}, ': ',
+						sprintf('0%03o', $um));
+	}
+	defined($um) ?
+		PublicInbox::OnDestroy->new(\&CORE::umask, umask($um)) :
+		undef;
+}
+
 sub cidx_run { # main entry point
 	my ($self) = @_;
+	my $restore_umask = prep_umask($self);
 	local $self->{todo} = [];
 	local $DEFER = $self->{todo};
 	local $SIGSET = PublicInbox::DS::block_signals();
@@ -800,7 +827,7 @@ sub shard_done_wait { # awaitpid cb via ipc_worker_reap
 	++$self->{shard_err} if defined($self->{shard_err});
 }
 
-sub with_umask { # TODO
+sub with_umask { # TODO get rid of this treewide and rely on OnDestroy
 	my ($self, $cb, @arg) = @_;
 	$cb->(@arg);
 }
diff --git a/t/cindex.t b/t/cindex.t
index eb66b2e6f..9da0ba692 100644
--- a/t/cindex.t
+++ b/t/cindex.t
@@ -12,9 +12,10 @@ my $pwd = getcwd();
 
 # I reworked CodeSearchIdx->shard_worker to handle empty trees
 # in the initial commit generated by cvs2svn for xapian.git
-create_coderepo 'empty-tree-root', tmpdir => "$tmp/wt0", sub {
+create_coderepo 'empty-tree-root-0600', tmpdir => "$tmp/wt0", sub {
 	xsys_e([qw(/bin/sh -c), <<'EOM']);
 git init -q &&
+git config core.sharedRepository 0600
 tree=$(git mktree </dev/null) &&
 head=$(git symbolic-ref HEAD) &&
 cmt=$(echo 'empty root' | git commit-tree $tree) &&
@@ -27,8 +28,14 @@ EOM
 }; # /create_coderepo
 
 ok(run_script([qw(-cindex --dangerous -q), "$tmp/wt0"]), 'cindex internal');
-ok(-e "$tmp/wt0/.git/public-inbox-cindex/cidx.lock", 'internal dir created');
-
+{
+	my $exists = -e "$tmp/wt0/.git/public-inbox-cindex/cidx.lock";
+	my @st = stat(_);
+	ok($exists, 'internal dir created');
+	is($st[2] & 0600, 0600, 'mode respects core.sharedRepository');
+	@st = stat("$tmp/wt0/.git/public-inbox-cindex");
+	is($st[2] & 0700, 0700, 'dir mode respects core.sharedRepository');
+}
 
 # it's possible for git to emit NUL characters in diffs
 # (see c4201214cbf10636e2c1ab9131573f735b42c8d4 in linux.git)
@@ -115,4 +122,13 @@ if ('--prune') {
 		'hit stays pruned since GIT_DIR was previously pruned');
 }
 
+File::Path::remove_tree("$tmp/ext");
+ok(mkdir("$tmp/ext", 0707), 'create $tmp/ext with odd permissions');
+ok(run_script([qw(-cindex --dangerous -q -d), "$tmp/ext", $zp]),
+	'external on existing dir');
+{
+	my @st = stat("$tmp/ext/cidx.lock");
+	is($st[2] & 0777, 0604, 'created lock respects odd permissions');
+}
+
 done_testing;
-- 
2.47.2