All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Locke <kevin@kevinlocke.name>
To: git@vger.kernel.org
Cc: Elijah Newren <newren@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Derrick Stolee <derrickstolee@github.com>
Subject: [PATCH v4] setup: don't die if realpath(3) fails on getcwd(3)
Date: Tue, 24 May 2022 13:20:12 -0600	[thread overview]
Message-ID: <8b20840014d214023c50ee62439147f798e6f9cc.1653419993.git.kevin@kevinlocke.name> (raw)
In-Reply-To: <68c66aef7ca4dba53faec9e6d2f3b70fe58ac33e.1653403877.git.kevin@kevinlocke.name>

Prior to Git 2.35.0, git could be run from an inaccessible working
directory so long as the git repository specified by options and/or
environment variables was accessible.  For example:

    git init repo
    mkdir -p a/b
    cd a/b
    chmod u-x ..
    git -C "${PWD%/a/b}/repo" status

If this example seems a bit contrived, consider running with the
repository owner as a substitute UID (e.g. with runuser(1) or sudo(8))
without ensuring the working directory is accessible by that user.

The code added by e6f8861bd4 ("setup: introduce
startup_info->original_cwd") to preserve the working directory attempts
to normalize the path using strbuf_realpath().  If that fails, as in the
case above, it is treated as a fatal error.

This commit treats strbuf_realpath() errors as non-fatal.  If an error
occurs, setup_original_cwd() will continue without applying removal
prevention for cwd, resulting in the pre-2.35.0 behavior.  The risk
should be minimal, since git will not operate on a repository with
inaccessible ancestors, this behavior is only known to occur when cwd is
a descendant of the repository, an ancestor of cwd is inaccessible, and
no ancestors of the repository are inaccessible.

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---

Changes since v3:
 * Free tmp_original_cwd in both codepaths.
 * Return after strbuf_realpath() fails, rather than jumping to
   no_prevention_needed, to avoid unnecessary free(NULL) and NULL
   reassignment.
 * Invert the condition and remove the else block to match the
   return-on-error code style for better readability.
 * Stop adding "Try" to comment, since strbuf_realpath() hasn't
   been optional since v1.

Changes since v2:
 * Use trace2_data_string(), rather than trace_printf(), to report
   realpath failure.

Changes since v1:
 * Set startup_info->original_cwd = NULL when strbuf_realpath() fails,
   rather than setting it to the un-normalized path.
 * Add a trace message when realpath fails to aid debugging.
 * Remove potential realpath failure cause from comment before it.
 * Improve format for reference to e6f8861bd4 in commit message.
 * Clarify when the pre-2.35.0 behavior may occur as a result of this
   commit in the commit message.
 * Remove 'Fixes:' tag from commit message.

 setup.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/setup.c b/setup.c
index a7b36f3ffbf..e0a99df512f 100644
--- a/setup.c
+++ b/setup.c
@@ -459,7 +459,16 @@ static void setup_original_cwd(void)
 	 */
 
 	/* Normalize the directory */
-	strbuf_realpath(&tmp, tmp_original_cwd, 1);
+	if (!strbuf_realpath(&tmp, tmp_original_cwd, 0)) {
+		trace2_data_string("setup", the_repository,
+				   "realpath-path", tmp_original_cwd);
+		trace2_data_string("setup", the_repository,
+				   "realpath-failure", strerror(errno));
+		free((char*)tmp_original_cwd);
+		tmp_original_cwd = NULL;
+		return;
+	}
+
 	free((char*)tmp_original_cwd);
 	tmp_original_cwd = NULL;
 	startup_info->original_cwd = strbuf_detach(&tmp, NULL);
-- 
2.35.1


  parent reply	other threads:[~2022-05-24 19:20 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-19 23:39 [PATCH] setup: don't die if realpath(3) fails on getcwd(3) Kevin Locke
2022-05-20 18:38 ` Junio C Hamano
2022-05-21  0:14 ` Elijah Newren
2022-05-21 13:02   ` Kevin Locke
2022-05-23 18:44     ` Derrick Stolee
2022-05-21 13:53 ` [PATCH v2] " Kevin Locke
2022-05-23 18:57   ` Derrick Stolee
2022-05-24 14:02     ` Kevin Locke
2022-05-24 15:20       ` Elijah Newren
2022-05-24 17:38         ` Derrick Stolee
2022-05-25  3:47           ` Elijah Newren
2022-05-27  7:48         ` Ævar Arnfjörð Bjarmason
2022-05-28  1:27           ` Elijah Newren
2022-05-24 14:51   ` [PATCH v3] " Kevin Locke
2022-05-24 15:21     ` Elijah Newren
2022-05-24 17:41     ` Derrick Stolee
2022-05-24 18:00       ` Kevin Locke
2022-05-24 19:20     ` Kevin Locke [this message]
2022-05-24 20:40       ` [PATCH v4] " Derrick Stolee
2022-05-24 21:25       ` Junio C Hamano
2022-05-25  3:51         ` Elijah Newren
2022-05-25  5:11           ` Junio C Hamano

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=8b20840014d214023c50ee62439147f798e6f9cc.1653419993.git.kevin@kevinlocke.name \
    --to=kevin@kevinlocke.name \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.