git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 8/9] fsck: warn about symlinked dotfiles we'll open with O_NOFOLLOW

From
Jeff King <peff@peff.net>
Date
May 1, 2021, 15:43 UTC
Message-ID
<YI13CDSp59PKkM9p@coredump.intra.peff.net>
In-Reply-To
<YI12hK4X/gfl3u29@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 4e142c798f..164282ab8f 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.875.g5dccece0aa
Previous: Jeff KingNext: Jeff King
Message 21 of 35 in “leftover bits from symlinked gitattributes, etc topics”
  1. 0/9 leftover bits from symlinked gitattributes, etc topicsJeff King, May 1, 2021
  2. 1/9 t7415: remove out-dated comment about translationJeff King, May 1, 2021
  3. Ævar Arnfjörð BjarmasonMay 3, 2021
  4. Jeff KingMay 3, 2021
  5. 2/9 fsck_tree(): fix shadowed variableJeff King, May 1, 2021
  6. Ævar Arnfjörð BjarmasonMay 3, 2021
  7. Jeff KingMay 3, 2021
  8. Ævar Arnfjörð BjarmasonMay 4, 2021
  9. 3/9 fsck_tree(): wrap some long linesJeff King, May 1, 2021
  10. Ævar Arnfjörð BjarmasonMay 3, 2021
  11. Jeff KingMay 3, 2021
  12. 4/9 t7415: rename to expand scopeJeff King, May 1, 2021
  13. 5/9 t7450: test verify_path() handling of gitmodulesJeff King, May 1, 2021
  14. Eric SunshineMay 1, 2021
  15. Eric SunshineMay 1, 2021
  16. Jeff KingMay 3, 2021
  17. Ævar Arnfjörð BjarmasonMay 3, 2021
  18. Jeff KingMay 3, 2021
  19. 6/9 t7450: test .gitmodules symlink matching against obscured namesJeff King, May 1, 2021
  20. 7/9 t0060: test ntfs/hfs-obscured dotfilesJeff King, May 1, 2021
  21. 8/9 fsck: warn about symlinked dotfiles we'll open with O_NOFOLLOWJeff King, May 1, 2021
  22. 9/9 docs: document symlink restrictions for dot-filesJeff King, May 1, 2021
  23. Eric SunshineMay 1, 2021
  24. Jeff KingMay 3, 2021
  25. Junio C HamanoMay 3, 2021
  26. 0/9 leftover bits from symlinked gitattributes, etc topicsJeff King, May 3, 2021
  27. 1/9 t7415: remove out-dated comment about translationJeff King, May 3, 2021
  28. 2/9 fsck_tree(): fix shadowed variableJeff King, May 3, 2021
  29. 3/9 fsck_tree(): wrap some long linesJeff King, May 3, 2021
  30. 4/9 t7415: rename to expand scopeJeff King, May 3, 2021
  31. 5/9 t7450: test verify_path() handling of gitmodulesJeff King, May 3, 2021
  32. 6/9 t7450: test .gitmodules symlink matching against obscured namesJeff King, May 3, 2021
  33. 7/9 t0060: test ntfs/hfs-obscured dotfilesJeff King, May 3, 2021
  34. 8/9 fsck: warn about symlinked dotfiles we'll open with O_NOFOLLOWJeff King, May 3, 2021
  35. 9/9 docs: document symlink restrictions for dot-filesJeff King, May 3, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.