git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>
Subject: [PATCH v2 8/9] fsck: warn about symlinked dotfiles we'll open with O_NOFOLLOW
Date: Mon, 3 May 2021 16:43:25 -0400	[thread overview]
Message-ID: <YJBgbfxayKvMAXaC@coredump.intra.peff.net> (raw)
In-Reply-To: <YJBgMP9eXq31INyN@coredump.intra.peff.net>

In the commits merged in via 204333b015 (Merge branch
'jk/open-dotgitx-with-nofollow', 2021-03-22), we stopped following
symbolic links for .gitattributes, .gitignore, and .mailmap files.

Let's teach fsck to warn that these symlinks are not going to do
anything. Note that this is just a warning, and won't block the objects
via transfer.fsckObjects, since there are reported to be cases of this
in the wild (and even once fixed, they will continue to exist in the
commit history of those projects, but are not particularly dangerous).

Note that we won't add these to the existing gitmodules block in the
fsck code. The logic for gitmodules is a bit more complicated, as we
also check the content of non-symlink instances we find. But for these
new files, there is no content check; we're just looking at the name and
mode of the tree entry (and we can avoid even the complicated name
checks in the common case that the mode doesn't indicate a symlink).

We can reuse the test helper function we defined for .gitmodules, though
(it needs some slight adjustments for the fsck error code, and because
we don't block these symlinks via verify_path()).

Note that I didn't explicitly test the transfer.fsckObjects case here
(nor does the existing .gitmodules test that it blocks a push). The
translation of fsck severities to outcomes is covered in general in
t5504.

Signed-off-by: Jeff King <peff@peff.net>
---
 fsck.c                      | 18 ++++++++++++++++++
 fsck.h                      |  3 +++
 t/t7450-bad-git-dotfiles.sh | 29 +++++++++++++++++++++++++++--
 3 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/fsck.c b/fsck.c
index db94817898..3ec500d707 100644
--- a/fsck.c
+++ b/fsck.c
@@ -614,6 +614,24 @@ static int fsck_tree(const struct object_id *tree_oid,
 						 ".gitmodules is a symbolic link");
 		}
 
+		if (S_ISLNK(mode)) {
+			if (is_hfs_dotgitignore(name) ||
+			    is_ntfs_dotgitignore(name))
+				retval += report(options, tree_oid, OBJ_TREE,
+						 FSCK_MSG_GITIGNORE_SYMLINK,
+						 ".gitignore is a symlink");
+			if (is_hfs_dotgitattributes(name) ||
+			    is_ntfs_dotgitattributes(name))
+				retval += report(options, tree_oid, OBJ_TREE,
+						 FSCK_MSG_GITATTRIBUTES_SYMLINK,
+						 ".gitattributes is a symlink");
+			if (is_hfs_dotmailmap(name) ||
+			    is_ntfs_dotmailmap(name))
+				retval += report(options, tree_oid, OBJ_TREE,
+						 FSCK_MSG_MAILMAP_SYMLINK,
+						 ".mailmap is a symlink");
+		}
+
 		if ((backslash = strchr(name, '\\'))) {
 			while (backslash) {
 				backslash++;
diff --git a/fsck.h b/fsck.h
index 7202c3c87e..d07f7a2459 100644
--- a/fsck.h
+++ b/fsck.h
@@ -67,6 +67,9 @@ enum fsck_msg_type {
 	FUNC(NUL_IN_COMMIT, WARN) \
 	/* infos (reported as warnings, but ignored by default) */ \
 	FUNC(GITMODULES_PARSE, INFO) \
+	FUNC(GITIGNORE_SYMLINK, INFO) \
+	FUNC(GITATTRIBUTES_SYMLINK, INFO) \
+	FUNC(MAILMAP_SYMLINK, INFO) \
 	FUNC(BAD_TAG_NAME, INFO) \
 	FUNC(MISSING_TAGGER_ENTRY, INFO) \
 	/* ignored (elevated when requested) */ \
diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh
index b494d72976..e2773bb06d 100755
--- a/t/t7450-bad-git-dotfiles.sh
+++ b/t/t7450-bad-git-dotfiles.sh
@@ -140,6 +140,18 @@ test_expect_success 'index-pack --strict works for non-repo pack' '
 '
 
 check_dotx_symlink () {
+	fsck_must_fail=test_must_fail
+	fsck_prefix=error
+	refuse_index=t
+	case "$1" in
+	--warning)
+		fsck_must_fail=
+		fsck_prefix=warning
+		refuse_index=
+		shift
+		;;
+	esac
+
 	name=$1
 	type=$2
 	path=$3
@@ -172,11 +184,12 @@ check_dotx_symlink () {
 
 			# Check not only that we fail, but that it is due to the
 			# symlink detector
-			test_must_fail git fsck 2>output &&
-			grep "tree $tree: ${name}Symlink" output
+			$fsck_must_fail git fsck 2>output &&
+			grep "$fsck_prefix.*tree $tree: ${name}Symlink" output
 		)
 	'
 
+	test -n "$refuse_index" &&
 	test_expect_success "refuse to load symlinked $name into index ($type)" '
 		test_must_fail \
 			git -C $dir \
@@ -193,6 +206,18 @@ check_dotx_symlink gitmodules vanilla .gitmodules
 check_dotx_symlink gitmodules ntfs ".gitmodules ."
 check_dotx_symlink gitmodules hfs ".${u200c}gitmodules"
 
+check_dotx_symlink --warning gitattributes vanilla .gitattributes
+check_dotx_symlink --warning gitattributes ntfs ".gitattributes ."
+check_dotx_symlink --warning gitattributes hfs ".${u200c}gitattributes"
+
+check_dotx_symlink --warning gitignore vanilla .gitignore
+check_dotx_symlink --warning gitignore ntfs ".gitignore ."
+check_dotx_symlink --warning gitignore hfs ".${u200c}gitignore"
+
+check_dotx_symlink --warning mailmap vanilla .mailmap
+check_dotx_symlink --warning mailmap ntfs ".mailmap ."
+check_dotx_symlink --warning mailmap hfs ".${u200c}mailmap"
+
 test_expect_success 'fsck detects non-blob .gitmodules' '
 	git init non-blob &&
 	(
-- 
2.31.1.926.gd12152deb6


  parent reply	other threads:[~2021-05-03 20:43 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-01 15:40 [PATCH 0/9] leftover bits from symlinked gitattributes, etc topics Jeff King
2021-05-01 15:41 ` [PATCH 1/9] t7415: remove out-dated comment about translation Jeff King
2021-05-03  9:46   ` Ævar Arnfjörð Bjarmason
2021-05-03 20:29     ` Jeff King
2021-05-01 15:41 ` [PATCH 2/9] fsck_tree(): fix shadowed variable Jeff King
2021-05-03 11:15   ` Ævar Arnfjörð Bjarmason
2021-05-03 20:13     ` Jeff King
2021-05-04 10:10       ` Ævar Arnfjörð Bjarmason
2021-05-01 15:41 ` [PATCH 3/9] fsck_tree(): wrap some long lines Jeff King
2021-05-03 11:22   ` Ævar Arnfjörð Bjarmason
2021-05-03 20:23     ` Jeff King
2021-05-01 15:42 ` [PATCH 4/9] t7415: rename to expand scope Jeff King
2021-05-01 15:42 ` [PATCH 5/9] t7450: test verify_path() handling of gitmodules Jeff King
2021-05-01 18:55   ` Eric Sunshine
2021-05-01 19:03     ` Eric Sunshine
2021-05-03 19:39       ` Jeff King
2021-05-03 10:12   ` Ævar Arnfjörð Bjarmason
2021-05-03 20:32     ` Jeff King
2021-05-01 15:42 ` [PATCH 6/9] t7450: test .gitmodules symlink matching against obscured names Jeff King
2021-05-01 15:42 ` [PATCH 7/9] t0060: test ntfs/hfs-obscured dotfiles Jeff King
2021-05-01 15:43 ` [PATCH 8/9] fsck: warn about symlinked dotfiles we'll open with O_NOFOLLOW Jeff King
2021-05-01 15:43 ` [PATCH 9/9] docs: document symlink restrictions for dot-files Jeff King
2021-05-01 19:16   ` Eric Sunshine
2021-05-03 20:33     ` Jeff King
2021-05-03  5:36 ` [PATCH 0/9] leftover bits from symlinked gitattributes, etc topics Junio C Hamano
2021-05-03 20:42 ` [PATCH v2 " Jeff King
2021-05-03 20:43   ` [PATCH v2 1/9] t7415: remove out-dated comment about translation Jeff King
2021-05-03 20:43   ` [PATCH v2 2/9] fsck_tree(): fix shadowed variable Jeff King
2021-05-03 20:43   ` [PATCH v2 3/9] fsck_tree(): wrap some long lines Jeff King
2021-05-03 20:43   ` [PATCH v2 4/9] t7415: rename to expand scope Jeff King
2021-05-03 20:43   ` [PATCH v2 5/9] t7450: test verify_path() handling of gitmodules Jeff King
2021-05-03 20:43   ` [PATCH v2 6/9] t7450: test .gitmodules symlink matching against obscured names Jeff King
2021-05-03 20:43   ` [PATCH v2 7/9] t0060: test ntfs/hfs-obscured dotfiles Jeff King
2021-05-03 20:43   ` Jeff King [this message]
2021-05-03 20:43   ` [PATCH v2 9/9] docs: document symlink restrictions for dot-files 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=YJBgbfxayKvMAXaC@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=sunshine@sunshineco.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).