| Message ID | 20201005072503.GG2291074@coredump.intra.peff.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | forbidding symlinked .gitattributes and .gitignore | expand |
Jeff King wrote: > The previous commit made it impossible to have a symlinked > .gitattributes or .gitignore file via verify_path(). Let's add the same > check to fsck, which matches how we handle .gitmodules symlinks, via > b7b1fca175 (fsck: complain when .gitmodules is a symlink, 2018-05-04). > > Note that we won't add these to the existing gitmodules block. Its logic > 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). On the subject of where the check gets added, the old description said It's easier to handle than .gitmodules, because we don't care about checking the blob content. This is really just about whether the name and mode for the tree entry are valid. which I think was self-explanatory enough. The new text is a little more confusing because I get lost in figuring out what the "it" in "Its" refers to. > Signed-off-by: Jeff King <peff@peff.net> > --- > fsck.c | 15 +++++++++++++++ > t/t7450-bad-meta-files.sh | 9 +++++++++ > 2 files changed, 24 insertions(+) Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Thanks for a pleasant read.
On Mon, Oct 05, 2020 at 01:12:33AM -0700, Jonathan Nieder wrote: > Jeff King wrote: > > > The previous commit made it impossible to have a symlinked > > .gitattributes or .gitignore file via verify_path(). Let's add the same > > check to fsck, which matches how we handle .gitmodules symlinks, via > > b7b1fca175 (fsck: complain when .gitmodules is a symlink, 2018-05-04). > > > > Note that we won't add these to the existing gitmodules block. Its logic > > 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). > > On the subject of where the check gets added, the old description said > > It's easier to handle than .gitmodules, because we don't care > about checking the blob content. This is really just about > whether the name and mode for the tree entry are valid. > > which I think was self-explanatory enough. The new text is a little > more confusing because I get lost in figuring out what the "it" in > "Its" refers to. I rewrote it because I found your original text confusing. ;) I'll write "The logic for gitmodules is a bit more complicated...". -Peff
diff --git a/fsck.c b/fsck.c index 024810139b..fcd3f268b1 100644 --- a/fsck.c +++ b/fsck.c @@ -67,6 +67,8 @@ static struct oidset gitmodules_done = OIDSET_INIT; FUNC(GITMODULES_URL, ERROR) \ FUNC(GITMODULES_PATH, ERROR) \ FUNC(GITMODULES_UPDATE, ERROR) \ + FUNC(GITIGNORE_SYMLINK, ERROR) \ + FUNC(GITATTRIBUTES_SYMLINK, ERROR) \ /* warnings */ \ FUNC(BAD_FILEMODE, WARN) \ FUNC(EMPTY_NAME, WARN) \ @@ -688,6 +690,19 @@ 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 ((backslash = strchr(name, '\\'))) { while (backslash) { backslash++; diff --git a/t/t7450-bad-meta-files.sh b/t/t7450-bad-meta-files.sh index 6a038ed55b..c7201803ec 100755 --- a/t/t7450-bad-meta-files.sh +++ b/t/t7450-bad-meta-files.sh @@ -281,6 +281,11 @@ test_expect_success 'refuse to load symlinked .gitattributes into index' ' test_i18ngrep "invalid path.*gitattributes" err ' +test_expect_success 'fsck detects symlinked .gitattributes file' ' + test_must_fail git -C symlink-attr fsck 2>err && + test_i18ngrep "tree $tree: gitattributesSymlink" err +' + test_expect_success 'create repo with symlinked .gitignore file' ' git init symlink-ignore && target=$(echo target | git -C symlink-ignore hash-object -w --stdin) && @@ -295,5 +300,9 @@ test_expect_success 'refuse to load symlinked .gitignore into index' ' test_i18ngrep "invalid path.*gitignore" err ' +test_expect_success 'fsck detects symlinked .gitignore file' ' + test_must_fail git -C symlink-ignore fsck 2>err && + test_i18ngrep "tree $tree: gitignoreSymlink" err +' test_done
The previous commit made it impossible to have a symlinked .gitattributes or .gitignore file via verify_path(). Let's add the same check to fsck, which matches how we handle .gitmodules symlinks, via b7b1fca175 (fsck: complain when .gitmodules is a symlink, 2018-05-04). Note that we won't add these to the existing gitmodules block. Its logic 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). Signed-off-by: Jeff King <peff@peff.net> --- fsck.c | 15 +++++++++++++++ t/t7450-bad-meta-files.sh | 9 +++++++++ 2 files changed, 24 insertions(+)