git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Michael J Gruber <git@drmicha.warpmail.net>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] t/lib-gpg: adjust permissions for gnupg 2.1
Date: Tue, 2 Dec 2014 16:07:53 -0500	[thread overview]
Message-ID: <20141202210753.GD23461@peff.net> (raw)
In-Reply-To: <9c28f16c677bbc774e5b8dfc79b6ffe2c55d1720.1417527514.git.git@drmicha.warpmail.net>

On Tue, Dec 02, 2014 at 02:40:27PM +0100, Michael J Gruber wrote:

> Before gnupg 2.1 (aka "modern branch"), gpghome would contain only files
> which allowed t/lib-gpg.sh to set permissions explicitely, and we did
> that since
> 28a1b07 (t/lib-gpg: adjust permissions for gnupg 2.1, 2014-12-02)
> in order to adjust wrong permissions from a checkout on ro file systems.

I think this 28a1b07 is wrong. Did you mean e7f224f?

> gnupg 2.1 creates a new directory in gpghome which would get its x bit removed.

Thanks for digging in this. The story is a little more tricky, though,
and I do not think this patch is strictly necessary.

We copy lib-gpg/* to the trash directory, and only run gpg on it there.
So it is there that gpg2.1 will munge the files, _after_ we have
copied and done our chmod. And that works fine with the current code.

The problem came when I was trying to test/debug, and outside of the
tests did "cd lib-gpg && gpg2 ...". That munged my lib-gpg directory,
and the resulting breakage was copied into each subsequent trash
directory.

So while your patch is not necessary, it is a nice defense against this
sort of manual munging, or against future patches which add more
directories. But...

> Adjust and use +X so that any directory would get its x bit set. This
> also keeps the x bit on files which had it set for whatever wrong
> reason, but we care only about having at least the necessary
> permissions for the tests to run.

Taking a step back, though, I am not sure I understand the reasoning
behind the original e7f224f. The rationale in the commit message is that
we want to make sure that the files are writable. But why would they not
be? They are created by "cp -R", so unless your umask does not allow the
owner to write to the files, they should be writable, no? And if your
umask is set that way, lots of things are going to break.

And indeed, if I remove the chmods completely, like:

diff --git a/t/lib-gpg.sh b/t/lib-gpg.sh
index cd2baef..6ee4bb6 100755
--- a/t/lib-gpg.sh
+++ b/t/lib-gpg.sh
@@ -17,8 +17,6 @@ else
 		# Name and email: C O Mitter <committer@example.com>
 		# No password given, to enable non-interactive operation.
 		cp -R "$TEST_DIRECTORY"/lib-gpg ./gpghome
-		chmod 0700 gpghome
-		chmod 0600 gpghome/*
 		GNUPGHOME="$(pwd)/gpghome"
 		export GNUPGHOME
 		test_set_prereq GPG

the tests run fine for me. What am I missing?

I do think the original "0700" chmod _is_ useful, though. But not
because it makes sure things are writable, but because it makes sure
that it is _not_ world-readable. GPG complains about the lax permissions
(of course it does not know that the keyrings are not really secrets in
this case). However, this does not actually prevent the tests from
running successfully.

So from my perspective, the simplest thing is to keep the original
"chmod 0700" for that reason (or make it "chmod go-rwx", if you like),
and drop the inner chmod completely (effectively reverting e7f224f). But
again, perhaps there is some case that it covers that I do not
understand.

-Peff

  reply	other threads:[~2014-12-02 21:08 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-26 23:09 [ANNOUNCE] Git v2.2.0 Junio C Hamano
2014-11-27 21:32 ` Steven Noonan
2014-11-28  4:46   ` Jeff King
2014-11-28  9:48   ` Michael J Gruber
2014-11-28 16:50     ` tests do not work with gpg 2.1 Jeff King
2014-12-02 12:55       ` Michael J Gruber
2014-12-02 13:40         ` [PATCH] t/lib-gpg: adjust permissions for gnupg 2.1 Michael J Gruber
2014-12-02 21:07           ` Jeff King [this message]
2014-12-02 23:57             ` Junio C Hamano
2014-12-03  0:05               ` Jeff King
2014-12-03 16:21                 ` Junio C Hamano
2014-12-03 11:23             ` Michael J Gruber
2014-12-03 16:45               ` Junio C Hamano
2014-12-02 21:21         ` tests do not work with gpg 2.1 Jeff King
2014-12-02 21:30           ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141202210753.GD23461@peff.net \
    --to=peff@peff.net \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).