From patchwork Sat May 1 15:41:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 12234827 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id EBF66C433B4 for ; Sat, 1 May 2021 15:41:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C2B6D61606 for ; Sat, 1 May 2021 15:41:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231656AbhEAPmK (ORCPT ); Sat, 1 May 2021 11:42:10 -0400 Received: from cloud.peff.net ([104.130.231.41]:42106 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230195AbhEAPmK (ORCPT ); Sat, 1 May 2021 11:42:10 -0400 Received: (qmail 26619 invoked by uid 109); 1 May 2021 15:41:20 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sat, 01 May 2021 15:41:20 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 10761 invoked by uid 111); 1 May 2021 15:41:20 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Sat, 01 May 2021 11:41:20 -0400 Authentication-Results: peff.net; auth=none Date: Sat, 1 May 2021 11:41:19 -0400 From: Jeff King To: git@vger.kernel.org Subject: [PATCH 1/9] t7415: remove out-dated comment about translation Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Since GETTEXT_POISON does not exist anymore, there is no point warning people about whether we should use test_i18ngrep. This is doubly confusing because the comment was describing why it was OK to use grep, but it got caught up in the mass conversion of 674ba34038 (fsck: mark strings for translation, 2018-11-10). Note there are other uses of test_i18ngrep in this script which are now obsolete; I'll save those for a mass-cleanup. My goal here was just to fix the confusing comment in code I'm about to refactor. Signed-off-by: Jeff King --- t/t7415-submodule-names.sh | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh index f70368bc2e..fef6561d80 100755 --- a/t/t7415-submodule-names.sh +++ b/t/t7415-submodule-names.sh @@ -151,10 +151,9 @@ test_expect_success 'fsck detects symlinked .gitmodules file' ' } | git mktree && # Check not only that we fail, but that it is due to the - # symlink detector; this grep string comes from the config - # variable name and will not be translated. + # symlink detector test_must_fail git fsck 2>output && - test_i18ngrep gitmodulesSymlink output + grep gitmodulesSymlink output ) ' From patchwork Sat May 1 15:41:38 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 12234829 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 23A4BC433B4 for ; Sat, 1 May 2021 15:41:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 03A846102A for ; Sat, 1 May 2021 15:41:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231670AbhEAPma (ORCPT ); Sat, 1 May 2021 11:42:30 -0400 Received: from cloud.peff.net ([104.130.231.41]:42112 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230051AbhEAPm3 (ORCPT ); Sat, 1 May 2021 11:42:29 -0400 Received: (qmail 26626 invoked by uid 109); 1 May 2021 15:41:39 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sat, 01 May 2021 15:41:39 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 10783 invoked by uid 111); 1 May 2021 15:41:39 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Sat, 01 May 2021 11:41:39 -0400 Authentication-Results: peff.net; auth=none Date: Sat, 1 May 2021 11:41:38 -0400 From: Jeff King To: git@vger.kernel.org Subject: [PATCH 2/9] fsck_tree(): fix shadowed variable Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Commit b2f2039c2b (fsck: accept an oid instead of a "struct tree" for fsck_tree(), 2019-10-18) introduced a new "oid" parameter to fsck_tree(), and we pass it to the report() function when we find problems. However, that is shadowed within the tree-walking loop by the existing "oid" variable which we use to store the oid of each tree entry. As a result, we may report the wrong oid for some problems we detect within the loop (the entry oid, instead of the tree oid). Our tests didn't catch this because they checked only that we found the expected fsck problem, not that it was attached to the correct object. Let's rename both variables in the function to avoid confusion. This makes the diff a little noisy (e.g., all of the report() calls outside the loop were already correct but need to be touched), but makes sure we catch all cases and will avoid similar confusion in the future. And we can update the test to be a bit more specific and catch this problem. Signed-off-by: Jeff King --- fsck.c | 42 ++++++++++++++++++++------------------ t/t7415-submodule-names.sh | 5 +++-- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/fsck.c b/fsck.c index f5ed6a2635..dd31ed1413 100644 --- a/fsck.c +++ b/fsck.c @@ -558,7 +558,7 @@ static int verify_ordered(unsigned mode1, const char *name1, return c1 < c2 ? 0 : TREE_UNORDERED; } -static int fsck_tree(const struct object_id *oid, +static int fsck_tree(const struct object_id *tree_oid, const char *buffer, unsigned long size, struct fsck_options *options) { @@ -579,7 +579,7 @@ static int fsck_tree(const struct object_id *oid, struct name_stack df_dup_candidates = { NULL }; if (init_tree_desc_gently(&desc, buffer, size)) { - retval += report(options, oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); return retval; } @@ -589,11 +589,11 @@ static int fsck_tree(const struct object_id *oid, while (desc.size) { unsigned short mode; const char *name, *backslash; - const struct object_id *oid; + const struct object_id *entry_oid; - oid = tree_entry_extract(&desc, &name, &mode); + entry_oid = tree_entry_extract(&desc, &name, &mode); - has_null_sha1 |= is_null_oid(oid); + has_null_sha1 |= is_null_oid(entry_oid); has_full_path |= !!strchr(name, '/'); has_empty_name |= !*name; has_dot |= !strcmp(name, "."); @@ -603,10 +603,11 @@ static int fsck_tree(const struct object_id *oid, if (is_hfs_dotgitmodules(name) || is_ntfs_dotgitmodules(name)) { if (!S_ISLNK(mode)) - oidset_insert(&options->gitmodules_found, oid); + oidset_insert(&options->gitmodules_found, + entry_oid); else retval += report(options, - oid, OBJ_TREE, + tree_oid, OBJ_TREE, FSCK_MSG_GITMODULES_SYMLINK, ".gitmodules is a symbolic link"); } @@ -617,9 +618,10 @@ static int fsck_tree(const struct object_id *oid, has_dotgit |= is_ntfs_dotgit(backslash); if (is_ntfs_dotgitmodules(backslash)) { if (!S_ISLNK(mode)) - oidset_insert(&options->gitmodules_found, oid); + oidset_insert(&options->gitmodules_found, + entry_oid); else - retval += report(options, oid, OBJ_TREE, + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_GITMODULES_SYMLINK, ".gitmodules is a symbolic link"); } @@ -628,7 +630,7 @@ static int fsck_tree(const struct object_id *oid, } if (update_tree_entry_gently(&desc)) { - retval += report(options, oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); break; } @@ -676,25 +678,25 @@ static int fsck_tree(const struct object_id *oid, name_stack_clear(&df_dup_candidates); if (has_null_sha1) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_NULL_SHA1, "contains entries pointing to null sha1"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_NULL_SHA1, "contains entries pointing to null sha1"); if (has_full_path) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_FULL_PATHNAME, "contains full pathnames"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_FULL_PATHNAME, "contains full pathnames"); if (has_empty_name) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_EMPTY_NAME, "contains empty pathname"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_EMPTY_NAME, "contains empty pathname"); if (has_dot) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_HAS_DOT, "contains '.'"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOT, "contains '.'"); if (has_dotdot) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_HAS_DOTDOT, "contains '..'"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOTDOT, "contains '..'"); if (has_dotgit) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_HAS_DOTGIT, "contains '.git'"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOTGIT, "contains '.git'"); if (has_zero_pad) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_ZERO_PADDED_FILEMODE, "contains zero-padded file modes"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_ZERO_PADDED_FILEMODE, "contains zero-padded file modes"); if (has_bad_modes) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_BAD_FILEMODE, "contains bad file modes"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_FILEMODE, "contains bad file modes"); if (has_dup_entries) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_DUPLICATE_ENTRIES, "contains duplicate file entries"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_DUPLICATE_ENTRIES, "contains duplicate file entries"); if (not_properly_sorted) - retval += report(options, oid, OBJ_TREE, FSCK_MSG_TREE_NOT_SORTED, "not properly sorted"); + retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_TREE_NOT_SORTED, "not properly sorted"); return retval; } diff --git a/t/t7415-submodule-names.sh b/t/t7415-submodule-names.sh index fef6561d80..6a8cf3f47b 100755 --- a/t/t7415-submodule-names.sh +++ b/t/t7415-submodule-names.sh @@ -148,12 +148,13 @@ test_expect_success 'fsck detects symlinked .gitmodules file' ' { printf "100644 blob $content\t$tricky\n" && printf "120000 blob $target\t.gitmodules\n" - } | git mktree && + } >bad-tree && + tree=$(git mktree output && - grep gitmodulesSymlink output + grep "tree $tree: gitmodulesSymlink" output ) ' From patchwork Sat May 1 15:41:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 12234831 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0EF49C433B4 for ; Sat, 1 May 2021 15:41:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DAC576102A for ; Sat, 1 May 2021 15:41:45 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231717AbhEAPmf (ORCPT ); Sat, 1 May 2021 11:42:35 -0400 Received: from cloud.peff.net ([104.130.231.41]:42118 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230051AbhEAPme (ORCPT ); Sat, 1 May 2021 11:42:34 -0400 Received: (qmail 26632 invoked by uid 109); 1 May 2021 15:41:44 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sat, 01 May 2021 15:41:44 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 10799 invoked by uid 111); 1 May 2021 15:41:44 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Sat, 01 May 2021 11:41:44 -0400 Authentication-Results: peff.net; auth=none Date: Sat, 1 May 2021 11:41:43 -0400 From: Jeff King To: git@vger.kernel.org Subject: [PATCH 3/9] fsck_tree(): wrap some long lines Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Many calls to report() in fsck_tree() are kept on a single line and are quite long. Most were pretty big to begin with, but have gotten even longer over the years as we've added more parameters. Let's accept the churn of wrapping them in order to conform to our usual line limits. Signed-off-by: Jeff King --- fsck.c | 48 ++++++++++++++++++++++++++++++++++++------------ 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/fsck.c b/fsck.c index dd31ed1413..db94817898 100644 --- a/fsck.c +++ b/fsck.c @@ -579,7 +579,9 @@ static int fsck_tree(const struct object_id *tree_oid, struct name_stack df_dup_candidates = { NULL }; if (init_tree_desc_gently(&desc, buffer, size)) { - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_BAD_TREE, + "cannot be parsed as a tree"); return retval; } @@ -630,7 +632,9 @@ static int fsck_tree(const struct object_id *tree_oid, } if (update_tree_entry_gently(&desc)) { - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_TREE, "cannot be parsed as a tree"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_BAD_TREE, + "cannot be parsed as a tree"); break; } @@ -678,25 +682,45 @@ static int fsck_tree(const struct object_id *tree_oid, name_stack_clear(&df_dup_candidates); if (has_null_sha1) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_NULL_SHA1, "contains entries pointing to null sha1"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_NULL_SHA1, + "contains entries pointing to null sha1"); if (has_full_path) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_FULL_PATHNAME, "contains full pathnames"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_FULL_PATHNAME, + "contains full pathnames"); if (has_empty_name) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_EMPTY_NAME, "contains empty pathname"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_EMPTY_NAME, + "contains empty pathname"); if (has_dot) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOT, "contains '.'"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_HAS_DOT, + "contains '.'"); if (has_dotdot) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOTDOT, "contains '..'"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_HAS_DOTDOT, + "contains '..'"); if (has_dotgit) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_HAS_DOTGIT, "contains '.git'"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_HAS_DOTGIT, + "contains '.git'"); if (has_zero_pad) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_ZERO_PADDED_FILEMODE, "contains zero-padded file modes"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_ZERO_PADDED_FILEMODE, + "contains zero-padded file modes"); if (has_bad_modes) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_BAD_FILEMODE, "contains bad file modes"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_BAD_FILEMODE, + "contains bad file modes"); if (has_dup_entries) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_DUPLICATE_ENTRIES, "contains duplicate file entries"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_DUPLICATE_ENTRIES, + "contains duplicate file entries"); if (not_properly_sorted) - retval += report(options, tree_oid, OBJ_TREE, FSCK_MSG_TREE_NOT_SORTED, "not properly sorted"); + retval += report(options, tree_oid, OBJ_TREE, + FSCK_MSG_TREE_NOT_SORTED, + "not properly sorted"); return retval; } From patchwork Sat May 1 15:42:08 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 12234833 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E28B3C433ED for ; Sat, 1 May 2021 15:42:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AC8D5614A7 for ; Sat, 1 May 2021 15:42:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231517AbhEAPnC (ORCPT ); Sat, 1 May 2021 11:43:02 -0400 Received: from cloud.peff.net ([104.130.231.41]:42124 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231416AbhEAPnA (ORCPT ); Sat, 1 May 2021 11:43:00 -0400 Received: (qmail 26639 invoked by uid 109); 1 May 2021 15:42:08 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sat, 01 May 2021 15:42:08 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 10816 invoked by uid 111); 1 May 2021 15:42:08 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Sat, 01 May 2021 11:42:08 -0400 Authentication-Results: peff.net; auth=none Date: Sat, 1 May 2021 11:42:08 -0400 From: Jeff King To: git@vger.kernel.org Subject: [PATCH 4/9] t7415: rename to expand scope Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org This script has already expanded beyond its original intent of ".. in submodule names" to include other malicious submodule bits. Let's update the name and description to reflect that, as well as the fact that we'll soon be adding similar tests for other dotfiles (.gitattributes, etc). We'll also renumber it to move it out of the group of submodule-specific tests. Signed-off-by: Jeff King --- ...submodule-names.sh => t7450-bad-git-dotfiles.sh} | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) rename t/{t7415-submodule-names.sh => t7450-bad-git-dotfiles.sh} (95%) diff --git a/t/t7415-submodule-names.sh b/t/t7450-bad-git-dotfiles.sh similarity index 95% rename from t/t7415-submodule-names.sh rename to t/t7450-bad-git-dotfiles.sh index 6a8cf3f47b..34d4dc6def 100755 --- a/t/t7415-submodule-names.sh +++ b/t/t7450-bad-git-dotfiles.sh @@ -1,9 +1,16 @@ #!/bin/sh -test_description='check handling of .. in submodule names +test_description='check broken or malicious patterns in .git* files -Exercise the name-checking function on a variety of names, and then give a -real-world setup that confirms we catch this in practice. +Such as: + + - presence of .. in submodule names; + Exercise the name-checking function on a variety of names, and then give a + real-world setup that confirms we catch this in practice. + + - nested submodule names + + - symlinked .gitmodules, etc ' . ./test-lib.sh . "$TEST_DIRECTORY"/lib-pack.sh From patchwork Sat May 1 15:42:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 12234835 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 71499C433B4 for ; Sat, 1 May 2021 15:42:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4E7B461477 for ; Sat, 1 May 2021 15:42:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231724AbhEAPnR (ORCPT ); Sat, 1 May 2021 11:43:17 -0400 Received: from cloud.peff.net ([104.130.231.41]:42130 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231416AbhEAPnR (ORCPT ); Sat, 1 May 2021 11:43:17 -0400 Received: (qmail 26645 invoked by uid 109); 1 May 2021 15:42:26 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sat, 01 May 2021 15:42:26 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 10832 invoked by uid 111); 1 May 2021 15:42:26 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Sat, 01 May 2021 11:42:26 -0400 Authentication-Results: peff.net; auth=none Date: Sat, 1 May 2021 11:42:25 -0400 From: Jeff King To: git@vger.kernel.org Subject: [PATCH 5/9] t7450: test verify_path() handling of gitmodules Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Commit 10ecfa7649 (verify_path: disallow symlinks in .gitmodules, 2018-05-04) made it impossible to load a symlink .gitmodules file into the index. However, there are no tests of this behavior. Let's make sure this case is covered. We can easily reuse the test setup created by the matching b7b1fca175 (fsck: complain when .gitmodules is a symlink, 2018-05-04). Signed-off-by: Jeff King --- t/t7450-bad-git-dotfiles.sh | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh index 34d4dc6def..c021d4e752 100755 --- a/t/t7450-bad-git-dotfiles.sh +++ b/t/t7450-bad-git-dotfiles.sh @@ -139,7 +139,7 @@ test_expect_success 'index-pack --strict works for non-repo pack' ' grep gitmodulesName output ' -test_expect_success 'fsck detects symlinked .gitmodules file' ' +test_expect_success 'set up repo with symlinked .gitmodules file' ' git init symlink && ( cd symlink && @@ -155,8 +155,14 @@ test_expect_success 'fsck detects symlinked .gitmodules file' ' { printf "100644 blob $content\t$tricky\n" && printf "120000 blob $target\t.gitmodules\n" - } >bad-tree && - tree=$(git mktree bad-tree + ) && + tree=$(git -C symlink mktree err && + test_i18ngrep "invalid path.*gitmodules" err && + git -C symlink ls-files >out && + test_must_be_empty out +' + test_expect_success 'fsck detects non-blob .gitmodules' ' git init non-blob && ( From patchwork Sat May 1 15:42:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 12234839 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7002AC433B4 for ; Sat, 1 May 2021 15:42:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 44B8961477 for ; Sat, 1 May 2021 15:42:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231739AbhEAPnc (ORCPT ); Sat, 1 May 2021 11:43:32 -0400 Received: from cloud.peff.net ([104.130.231.41]:42136 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231416AbhEAPnc (ORCPT ); Sat, 1 May 2021 11:43:32 -0400 Received: (qmail 26651 invoked by uid 109); 1 May 2021 15:42:41 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sat, 01 May 2021 15:42:41 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 10849 invoked by uid 111); 1 May 2021 15:42:41 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Sat, 01 May 2021 11:42:41 -0400 Authentication-Results: peff.net; auth=none Date: Sat, 1 May 2021 11:42:40 -0400 From: Jeff King To: git@vger.kernel.org Subject: [PATCH 6/9] t7450: test .gitmodules symlink matching against obscured names Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In t7450 we check that both verify_path() and fsck catch malformed .gitmodules entries in trees. However, we don't check that we catch filesystem-equivalent forms of these (e.g., ".GITMOD~1" on Windows). Our name-matching functions are exercised well in t0060, but there's nothing to test that we correctly call the matching functions from the actual fsck and verify_path() code. So instead of testing just .gitmodules, let's repeat our tests for a few basic cases. We don't need to be exhaustive here (t0060 handles that), but just make sure we hit one name of each type. Besides pushing the tests into a function that takes the path as a parameter, we'll need to do a few things: - adjust the directory name to accommodate the tests running multiple times - set core.protecthfs for index checks. Fsck always protects all types by default, but we want to be able to exercise the HFS routines on every system. Note that core.protectntfs is already the default these days, but it doesn't hurt to explicitly label our need for it. - we'll also take the filename ("gitmodules") as a parameter. All calls use the same name for now, but a future patch will extend this to handle other .gitfoo files. Note that our fake-content symlink destination is somewhat .gitmodules specific. But it isn't necessary for other files (which don't do a content check). And it happens to be a valid attribute and ignore file anyway. Signed-off-by: Jeff King --- t/t7450-bad-git-dotfiles.sh | 91 +++++++++++++++++++++---------------- 1 file changed, 53 insertions(+), 38 deletions(-) diff --git a/t/t7450-bad-git-dotfiles.sh b/t/t7450-bad-git-dotfiles.sh index c021d4e752..4e142c798f 100755 --- a/t/t7450-bad-git-dotfiles.sh +++ b/t/t7450-bad-git-dotfiles.sh @@ -139,44 +139,59 @@ test_expect_success 'index-pack --strict works for non-repo pack' ' grep gitmodulesName output ' -test_expect_success 'set up repo with symlinked .gitmodules file' ' - git init symlink && - ( - cd symlink && - - # Make the tree directly to avoid index restrictions. - # - # Because symlinks store the target as a blob, choose - # a pathname that could be parsed as a .gitmodules file - # to trick naive non-symlink-aware checking. - tricky="[foo]bar=true" && - content=$(git hash-object -w ../.gitmodules) && - target=$(printf "$tricky" | git hash-object -w --stdin) && - { - printf "100644 blob $content\t$tricky\n" && - printf "120000 blob $target\t.gitmodules\n" - } >bad-tree - ) && - tree=$(git -C symlink mktree output && - grep "tree $tree: gitmodulesSymlink" output - ) -' - -test_expect_success 'refuse to load symlinked .gitmodules into index' ' - test_must_fail git -C symlink read-tree $tree 2>err && - test_i18ngrep "invalid path.*gitmodules" err && - git -C symlink ls-files >out && - test_must_be_empty out -' +check_dotx_symlink () { + name=$1 + type=$2 + path=$3 + dir=symlink-$name-$type + + test_expect_success "set up repo with symlinked $name ($type)" ' + git init $dir && + ( + cd $dir && + + # Make the tree directly to avoid index restrictions. + # + # Because symlinks store the target as a blob, choose + # a pathname that could be parsed as a .gitmodules file + # to trick naive non-symlink-aware checking. + tricky="[foo]bar=true" && + content=$(git hash-object -w ../.gitmodules) && + target=$(printf "$tricky" | git hash-object -w --stdin) && + { + printf "100644 blob $content\t$tricky\n" && + printf "120000 blob $target\t$path\n" + } >bad-tree + ) && + tree=$(git -C $dir mktree <$dir/bad-tree) + ' + + test_expect_success "fsck detects symlinked $name ($type)" ' + ( + cd $dir && + + # 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 + ) + ' + + test_expect_success "refuse to load symlinked $name into index ($type)" ' + test_must_fail \ + git -C $dir \ + -c core.protectntfs \ + -c core.protecthfs \ + read-tree $tree 2>err && + test_i18ngrep "invalid path.*$name" err && + git -C $dir ls-files -s >out && + test_must_be_empty out + ' +} + +check_dotx_symlink gitmodules vanilla .gitmodules +check_dotx_symlink gitmodules ntfs ".gitmodules ." +check_dotx_symlink gitmodules hfs ".${u200c}gitmodules" test_expect_success 'fsck detects non-blob .gitmodules' ' git init non-blob && From patchwork Sat May 1 15:42:54 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 12234841 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DC7C4C433ED for ; Sat, 1 May 2021 15:42:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id BDB566102A for ; Sat, 1 May 2021 15:42:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231761AbhEAPnq (ORCPT ); Sat, 1 May 2021 11:43:46 -0400 Received: from cloud.peff.net ([104.130.231.41]:42142 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231416AbhEAPno (ORCPT ); Sat, 1 May 2021 11:43:44 -0400 Received: (qmail 26656 invoked by uid 109); 1 May 2021 15:42:54 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sat, 01 May 2021 15:42:54 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 10867 invoked by uid 111); 1 May 2021 15:42:54 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Sat, 01 May 2021 11:42:54 -0400 Authentication-Results: peff.net; auth=none Date: Sat, 1 May 2021 11:42:54 -0400 From: Jeff King To: git@vger.kernel.org Subject: [PATCH 7/9] t0060: test ntfs/hfs-obscured dotfiles Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org We have tests that cover various filesystem-specific spellings of ".gitmodules", because we need to reliably identify that path for some security checks. These are from dc2d9ba318 (is_{hfs,ntfs}_dotgitmodules: add tests, 2018-05-12), with the actual code coming from e7cb0b4455 (is_ntfs_dotgit: match other .git files, 2018-05-11) and 0fc333ba20 (is_hfs_dotgit: match other .git files, 2018-05-02). Those latter two commits also added similar matching functions for .gitattributes and .gitignore. These ended up not being used in the final series, and are currently dead code. But in preparation for them being used in some fsck checks, let's make sure they actually work by throwing a few basic tests at them. Likewise, let's cover .mailmap (which does need matching code added). I didn't bother with the whole battery of tests that we cover for .gitmodules. These functions are all based on the same generic matcher, so it's sufficient to test most of the corner cases just once. Note that the ntfs magic prefix names in the tests come from the algorithm described in e7cb0b4455 (and are different for each file). Signed-off-by: Jeff King --- cache.h | 1 + path.c | 5 +++++ t/helper/test-path-utils.c | 46 +++++++++++++++++++++++++++----------- t/t0060-path-utils.sh | 30 +++++++++++++++++++++++++ utf8.c | 5 +++++ utf8.h | 1 + 6 files changed, 75 insertions(+), 13 deletions(-) diff --git a/cache.h b/cache.h index b785ffb383..e6dda88fb0 100644 --- a/cache.h +++ b/cache.h @@ -1271,6 +1271,7 @@ int is_ntfs_dotgit(const char *name); int is_ntfs_dotgitmodules(const char *name); int is_ntfs_dotgitignore(const char *name); int is_ntfs_dotgitattributes(const char *name); +int is_ntfs_dotmailmap(const char *name); /* * Returns true iff "str" could be confused as a command-line option when diff --git a/path.c b/path.c index 9e883eb524..7bccd830e9 100644 --- a/path.c +++ b/path.c @@ -1493,6 +1493,11 @@ int is_ntfs_dotgitattributes(const char *name) return is_ntfs_dot_str(name, "gitattributes", "gi7d29"); } +int is_ntfs_dotmailmap(const char *name) +{ + return is_ntfs_dot_str(name, "mailmap", "maba30"); +} + int looks_like_command_line_option(const char *str) { return str && str[0] == '-'; diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c index 313a153209..229ed416b0 100644 --- a/t/helper/test-path-utils.c +++ b/t/helper/test-path-utils.c @@ -172,9 +172,22 @@ static struct test_data dirname_data[] = { { NULL, NULL } }; -static int is_dotgitmodules(const char *path) +static int check_dotfile(const char *x, const char **argv, + int (*is_hfs)(const char *), + int (*is_ntfs)(const char *)) { - return is_hfs_dotgitmodules(path) || is_ntfs_dotgitmodules(path); + int res = 0, expect = 1; + for (; *argv; argv++) { + if (!strcmp("--not", *argv)) + expect = !expect; + else if (expect != (is_hfs(*argv) || is_ntfs(*argv))) + res = error("'%s' is %s.git%s", *argv, + expect ? "not " : "", x); + else + fprintf(stderr, "ok: '%s' is %s.git%s\n", + *argv, expect ? "" : "not ", x); + } + return !!res; } static int cmp_by_st_size(const void *a, const void *b) @@ -382,17 +395,24 @@ int cmd__path_utils(int argc, const char **argv) return test_function(dirname_data, posix_dirname, argv[1]); if (argc > 2 && !strcmp(argv[1], "is_dotgitmodules")) { - int res = 0, expect = 1, i; - for (i = 2; i < argc; i++) - if (!strcmp("--not", argv[i])) - expect = !expect; - else if (expect != is_dotgitmodules(argv[i])) - res = error("'%s' is %s.gitmodules", argv[i], - expect ? "not " : ""); - else - fprintf(stderr, "ok: '%s' is %s.gitmodules\n", - argv[i], expect ? "" : "not "); - return !!res; + return check_dotfile("modules", argv + 2, + is_hfs_dotgitmodules, + is_ntfs_dotgitmodules); + } + if (argc > 2 && !strcmp(argv[1], "is_dotgitignore")) { + return check_dotfile("ignore", argv + 2, + is_hfs_dotgitignore, + is_ntfs_dotgitignore); + } + if (argc > 2 && !strcmp(argv[1], "is_dotgitattributes")) { + return check_dotfile("attributes", argv + 2, + is_hfs_dotgitattributes, + is_ntfs_dotgitattributes); + } + if (argc > 2 && !strcmp(argv[1], "is_dotmailmap")) { + return check_dotfile("mailmap", argv + 2, + is_hfs_dotmailmap, + is_ntfs_dotmailmap); } if (argc > 2 && !strcmp(argv[1], "file-size")) { diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index 0ff06b5d1b..de4960783f 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -468,6 +468,36 @@ test_expect_success 'match .gitmodules' ' .gitmodules,:\$DATA ' +test_expect_success 'match .gitattributes' ' + test-tool path-utils is_dotgitattributes \ + .gitattributes \ + .git${u200c}attributes \ + .Gitattributes \ + .gitattributeS \ + GITATT~1 \ + GI7D29~1 +' + +test_expect_success 'match .gitignore' ' + test-tool path-utils is_dotgitignore \ + .gitignore \ + .git${u200c}ignore \ + .Gitignore \ + .gitignorE \ + GITIGN~1 \ + GI250A~1 +' + +test_expect_success 'match .mailmap' ' + test-tool path-utils is_dotmailmap \ + .mailmap \ + .mail${u200c}map \ + .Mailmap \ + .mailmaP \ + MAILMA~1 \ + MABA30~1 +' + test_expect_success MINGW 'is_valid_path() on Windows' ' test-tool path-utils is_valid_path \ win32 \ diff --git a/utf8.c b/utf8.c index 5b39361ada..de4ce5c0e6 100644 --- a/utf8.c +++ b/utf8.c @@ -777,6 +777,11 @@ int is_hfs_dotgitattributes(const char *path) return is_hfs_dot_str(path, "gitattributes"); } +int is_hfs_dotmailmap(const char *path) +{ + return is_hfs_dot_str(path, "mailmap"); +} + const char utf8_bom[] = "\357\273\277"; int skip_utf8_bom(char **text, size_t len) diff --git a/utf8.h b/utf8.h index fcd5167baf..9a16c8679d 100644 --- a/utf8.h +++ b/utf8.h @@ -61,6 +61,7 @@ int is_hfs_dotgit(const char *path); int is_hfs_dotgitmodules(const char *path); int is_hfs_dotgitignore(const char *path); int is_hfs_dotgitattributes(const char *path); +int is_hfs_dotmailmap(const char *path); typedef enum { ALIGN_LEFT, From patchwork Sat May 1 15:43:04 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 12234843 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1E72EC433ED for ; Sat, 1 May 2021 15:43:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id F3EE7614A7 for ; Sat, 1 May 2021 15:43:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231774AbhEAPn4 (ORCPT ); Sat, 1 May 2021 11:43:56 -0400 Received: from cloud.peff.net ([104.130.231.41]:42148 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231416AbhEAPnz (ORCPT ); Sat, 1 May 2021 11:43:55 -0400 Received: (qmail 26662 invoked by uid 109); 1 May 2021 15:43:05 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sat, 01 May 2021 15:43:05 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 10883 invoked by uid 111); 1 May 2021 15:43:05 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Sat, 01 May 2021 11:43:05 -0400 Authentication-Results: peff.net; auth=none Date: Sat, 1 May 2021 11:43:04 -0400 From: Jeff King To: git@vger.kernel.org Subject: [PATCH 8/9] fsck: warn about symlinked dotfiles we'll open with O_NOFOLLOW Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org 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 --- 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 && ( From patchwork Sat May 1 15:43:09 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 12234845 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 67889C433ED for ; Sat, 1 May 2021 15:43:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 3FDFB61477 for ; Sat, 1 May 2021 15:43:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231670AbhEAPoB (ORCPT ); Sat, 1 May 2021 11:44:01 -0400 Received: from cloud.peff.net ([104.130.231.41]:42154 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230450AbhEAPoA (ORCPT ); Sat, 1 May 2021 11:44:00 -0400 Received: (qmail 26667 invoked by uid 109); 1 May 2021 15:43:10 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sat, 01 May 2021 15:43:10 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 10899 invoked by uid 111); 1 May 2021 15:43:10 -0000 Received: from coredump.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Sat, 01 May 2021 11:43:10 -0400 Authentication-Results: peff.net; auth=none Date: Sat, 1 May 2021 11:43:09 -0400 From: Jeff King To: git@vger.kernel.org Subject: [PATCH 9/9] docs: document symlink restrictions for dot-files Message-ID: References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org We stopped allowing symlinks for .gitmodules files in 10ecfa7649 (verify_path: disallow symlinks in .gitmodules, 2018-05-04), and we stopped following symlinks for .gitattributes, .gitignore, and .mailmap in the commits from 204333b015 (Merge branch 'jk/open-dotgitx-with-nofollow', 2021-03-22). The reasons are discussed in detail there, but we never adjusted the documentation to let users know. This hasn't been a big deal since the point is that such setups were mildly broken and thought to be unusual anyway. But it certainly doesn't hurt to be clear and explicit about it. Suggested-by: Philip Oakley Signed-off-by: Jeff King --- Documentation/gitattributes.txt | 7 +++++++ Documentation/gitignore.txt | 5 +++++ Documentation/gitmailmap.txt | 7 +++++++ Documentation/gitmodules.txt | 8 ++++++++ 4 files changed, 27 insertions(+) diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt index cfcfa800c2..dfda94d996 100644 --- a/Documentation/gitattributes.txt +++ b/Documentation/gitattributes.txt @@ -1247,6 +1247,13 @@ to: [attr]binary -diff -merge -text ------------ +NOTES +----- + +Note that Git does not follow symbolic links when accessing a +`.gitattributes` file in the working tree. This keeps behavior +consistent when the file is accessed from the index or a tree versus +from the filesystem. EXAMPLES -------- diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt index 5751603b13..4b6fd8d2cd 100644 --- a/Documentation/gitignore.txt +++ b/Documentation/gitignore.txt @@ -149,6 +149,11 @@ not tracked by Git remain untracked. To stop tracking a file that is currently tracked, use 'git rm --cached'. +Note that Git does not follow symbolic links when accessing a +`.gitignore` file in the working tree. This keeps behavior consistent +when the file is accessed from the index or a tree versus from the +filesystem. + EXAMPLES -------- diff --git a/Documentation/gitmailmap.txt b/Documentation/gitmailmap.txt index 3fb39f801f..eb65eeb37f 100644 --- a/Documentation/gitmailmap.txt +++ b/Documentation/gitmailmap.txt @@ -55,6 +55,13 @@ this would also match the 'Commit Name ' above: Proper Name CoMmIt NaMe -- +NOTES +----- + +Note that Git does not follow symbolic links when accessing a `.mailmap` +file in the working tree. This keeps behavior consistent when the file +is accessed from the index or a tree versus from the filesystem. + EXAMPLES -------- diff --git a/Documentation/gitmodules.txt b/Documentation/gitmodules.txt index 8e333dde1b..ca1c42b405 100644 --- a/Documentation/gitmodules.txt +++ b/Documentation/gitmodules.txt @@ -98,6 +98,14 @@ submodule..shallow:: shallow clone (with a history depth of 1) unless the user explicitly asks for a non-shallow clone. +NOTES +----- + +Note that Git does not allow the `.gitmodules` file within a working +tree to be a symbolic link, and will refuse to check out such a tree +entry. This keeps behavior consistent when the file is accessed from the +index or a tree versus from the filesystem, and helps Git reliably +enforce security checks of the file contents. EXAMPLES --------