Umask and xapian db file permissions

Umask and xapian db file permissions

[Konstantin Ryabitsev]

[-- Attachment #1.1: Type: text/plain, Size: 1645 bytes --]

Hello:

For some reason, when public-inbox-mda reindexes the repository
past-delivery, it creates some files as 0660, even though umask is
specifically set to 0002:

$ ls -al public-inbox/xapian15/
total 21932
drwxrwxr-x. 2 archiver archiver    4096 May 24 16:52 .
drwxrwxr-x. 3 archiver archiver      42 May 24 16:52 ..
-rw-rw-r--. 1 archiver archiver       0 May 24 16:52 flintlock
-rw-rw-r--. 1 archiver archiver      28 May 24 15:56 iamchert
-rw-rw-r--. 1 archiver archiver 1190912 May 24 16:52 over.sqlite3
-rw-rw-r--. 1 archiver archiver       0 May 24 16:52 over.sqlite3-journal
-rw-rw----. 1 archiver archiver     150 May 24 16:48 position.baseA
-rw-rw----. 1 archiver archiver     150 May 24 16:52 position.baseB
-rw-rw-r--. 1 archiver archiver 8626176 May 24 16:52 position.DB
-rw-rw----. 1 archiver archiver     149 May 24 16:48 postlist.baseA
-rw-rw----. 1 archiver archiver     149 May 24 16:52 postlist.baseB
-rw-rw-r--. 1 archiver archiver 8642560 May 24 16:52 postlist.DB
-rw-rw----. 1 archiver archiver      20 May 24 16:48 record.baseA
-rw-rw----. 1 archiver archiver      20 May 24 16:52 record.baseB
-rw-rw-r--. 1 archiver archiver  368640 May 24 16:52 record.DB
-rw-rw----. 1 archiver archiver      71 May 24 16:48 termlist.baseA
-rw-rw----. 1 archiver archiver      71 May 24 16:52 termlist.baseB
-rw-rw-r--. 1 archiver archiver 3579904 May 24 16:52 termlist.DB

Since the daemon is running as user "publicinbox", this causes the web
interface to break due to not being able to access the index.

Best,
-- 
Konstantin Ryabitsev
Director, IT Infrastructure Security
The Linux Foundation


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH] respect umask if core.sharedRepository is not set

[Eric Wong]

Konstantin Ryabitsev <konstantin@linuxfoundation.org> wrote:
> Hello:
> 
> For some reason, when public-inbox-mda reindexes the repository
> past-delivery, it creates some files as 0660, even though umask is
> specifically set to 0002:

(sorry for the late reply, haven't been well)

Oops, I misread/misunderstood how git handles the
core.sharedRepository unset case :x

The following should fix it:

------8<------
Subject: [PATCH] respect umask if core.sharedRepository is not set

This is consistent with git itself and the previous behavior
was a result of misunderstanding of how git interprets this.
And adjust tests slightly to match the new behavior.

Reported-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
	<38873789-ab42-65a1-20c9-12c30b171f4f@linuxfoundation.org>
---
 lib/PublicInbox/InboxWritable.pm | 2 +-
 t/search.t                       | 5 +++--
 t/v2writable.t                   | 1 +
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/lib/PublicInbox/InboxWritable.pm b/lib/PublicInbox/InboxWritable.pm
index 5c11a36..9b0cdfd 100644
--- a/lib/PublicInbox/InboxWritable.pm
+++ b/lib/PublicInbox/InboxWritable.pm
@@ -175,7 +175,7 @@ sub _read_git_config_perm {
 sub _git_config_perm {
 	my $self = shift;
 	my $perm = scalar @_ ? $_[0] : _read_git_config_perm($self);
-	return PERM_GROUP if (!defined($perm) || $perm eq '');
+	return PERM_UMASK if (!defined($perm) || $perm eq '');
 	return PERM_UMASK if ($perm eq 'umask');
 	return PERM_GROUP if ($perm eq 'group');
 	if ($perm =~ /\A(?:all|world|everybody)\z/) {
diff --git a/t/search.t b/t/search.t
index 9a90fd5..c971fe3 100644
--- a/t/search.t
+++ b/t/search.t
@@ -11,7 +11,7 @@ my $tmpdir = tempdir('pi-search-XXXXXX', TMPDIR => 1, CLEANUP => 1);
 my $git_dir = "$tmpdir/a.git";
 my ($root_id, $last_id);
 
-is(0, system(qw(git init -q --bare), $git_dir), "git init (main)");
+is(0, system(qw(git init --shared -q --bare), $git_dir), "git init (main)");
 eval { PublicInbox::Search->new($git_dir) };
 ok($@, "exception raised on non-existent DB");
 
@@ -422,11 +422,12 @@ $ibx->with_umask(sub {
 });
 
 foreach my $f ("$git_dir/public-inbox/msgmap.sqlite3",
+		"$git_dir/public-inbox",
 		glob("$git_dir/public-inbox/xapian*/"),
 		glob("$git_dir/public-inbox/xapian*/*")) {
 	my @st = stat($f);
 	my ($bn) = (split(m!/!, $f))[-1];
-	is($st[2] & 07777, -f _ ? 0660 : 0770,
+	is($st[2] & 07777, -f _ ? 0660 : 02770,
 		"sharedRepository respected for $bn");
 }
 
diff --git a/t/v2writable.t b/t/v2writable.t
index 00b08e0..9e3bb75 100644
--- a/t/v2writable.t
+++ b/t/v2writable.t
@@ -11,6 +11,7 @@ foreach my $mod (qw(DBD::SQLite Search::Xapian)) {
 	plan skip_all => "$mod missing for nntpd.t" if $@;
 }
 use_ok 'PublicInbox::V2Writable';
+umask 007;
 my $mainrepo = tempdir('pi-v2writable-XXXXXX', TMPDIR => 1, CLEANUP => 1);
 my $ibx = {
 	mainrepo => $mainrepo,
-- 
EW

Re: [PATCH] respect umask if core.sharedRepository is not set

[Konstantin Ryabitsev]

On Wed, May 30, 2018 at 02:54:48AM +0000, Eric Wong wrote:
>> For some reason, when public-inbox-mda reindexes the repository
>> past-delivery, it creates some files as 0660, even though umask is
>> specifically set to 0002:
>
>(sorry for the late reply, haven't been well)

Eh, no worries!

>Oops, I misread/misunderstood how git handles the
>core.sharedRepository unset case :x
>
>The following should fix it:

It does, thanks!

-K

>
>------8<------
>Subject: [PATCH] respect umask if core.sharedRepository is not set
>
>This is consistent with git itself and the previous behavior
>was a result of misunderstanding of how git interprets this.
>And adjust tests slightly to match the new behavior.
>
>Reported-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>
>	<38873789-ab42-65a1-20c9-12c30b171f4f@linuxfoundation.org>
>---
> lib/PublicInbox/InboxWritable.pm | 2 +-
> t/search.t                       | 5 +++--
> t/v2writable.t                   | 1 +
> 3 files changed, 5 insertions(+), 3 deletions(-)
>
>diff --git a/lib/PublicInbox/InboxWritable.pm b/lib/PublicInbox/InboxWritable.pm
>index 5c11a36..9b0cdfd 100644
>--- a/lib/PublicInbox/InboxWritable.pm
>+++ b/lib/PublicInbox/InboxWritable.pm
>@@ -175,7 +175,7 @@ sub _read_git_config_perm {
> sub _git_config_perm {
> 	my $self = shift;
> 	my $perm = scalar @_ ? $_[0] : _read_git_config_perm($self);
>-	return PERM_GROUP if (!defined($perm) || $perm eq '');
>+	return PERM_UMASK if (!defined($perm) || $perm eq '');
> 	return PERM_UMASK if ($perm eq 'umask');
> 	return PERM_GROUP if ($perm eq 'group');
> 	if ($perm =~ /\A(?:all|world|everybody)\z/) {
>diff --git a/t/search.t b/t/search.t
>index 9a90fd5..c971fe3 100644
>--- a/t/search.t
>+++ b/t/search.t
>@@ -11,7 +11,7 @@ my $tmpdir = tempdir('pi-search-XXXXXX', TMPDIR => 1, CLEANUP => 1);
> my $git_dir = "$tmpdir/a.git";
> my ($root_id, $last_id);
>
>-is(0, system(qw(git init -q --bare), $git_dir), "git init (main)");
>+is(0, system(qw(git init --shared -q --bare), $git_dir), "git init (main)");
> eval { PublicInbox::Search->new($git_dir) };
> ok($@, "exception raised on non-existent DB");
>
>@@ -422,11 +422,12 @@ $ibx->with_umask(sub {
> });
>
> foreach my $f ("$git_dir/public-inbox/msgmap.sqlite3",
>+		"$git_dir/public-inbox",
> 		glob("$git_dir/public-inbox/xapian*/"),
> 		glob("$git_dir/public-inbox/xapian*/*")) {
> 	my @st = stat($f);
> 	my ($bn) = (split(m!/!, $f))[-1];
>-	is($st[2] & 07777, -f _ ? 0660 : 0770,
>+	is($st[2] & 07777, -f _ ? 0660 : 02770,
> 		"sharedRepository respected for $bn");
> }
>
>diff --git a/t/v2writable.t b/t/v2writable.t
>index 00b08e0..9e3bb75 100644
>--- a/t/v2writable.t
>+++ b/t/v2writable.t
>@@ -11,6 +11,7 @@ foreach my $mod (qw(DBD::SQLite Search::Xapian)) {
> 	plan skip_all => "$mod missing for nntpd.t" if $@;
> }
> use_ok 'PublicInbox::V2Writable';
>+umask 007;
> my $mainrepo = tempdir('pi-v2writable-XXXXXX', TMPDIR => 1, CLEANUP => 1);
> my $ibx = {
> 	mainrepo => $mainrepo,
>-- 
>EW