Re: [PATCH 0/2] Revert defense-in-depth patches breaking Git LFS
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- May 29, 2024, 12:17 UTC
- Message-ID
- <1cbdeb41-2ad3-05e4-ab27-1f84086b7f43@gmx.de>
- In-Reply-To
- <20240529085401.GA1098944@coredump.intra.peff.net>
Hi Jeff,
On Wed, 29 May 2024, Jeff King wrote:
> [...] But of course most sites just use the defaults, so all warnings > are effectively errors.
I wish that had been pointed out on the git-security mailing list when I offered this patch up for review.
> In the meantime, we also have an "INFO" severity which gets reported but > not upgraded via strict. It sounds like that's what was intended here.
Precisely.
So this is what the fix-up patch would look like to make the code match my intention:
-- snipsnap -- Subject: [PATCH] fsck: demote the newly-introduced symlink issues from WARN -> IGNORE
The idea of the symlink check to prevent overly-long symlink targets and targets inside the `.git/` directory was to _warn_, but not to prevent any operation.
However, that's not how Git works, I was confused by the label `WARN`. What we need instead is the `IGNORE` label, which still warns (confusingly so ;-)), but does not prevent any operations from continuing.
Adjust t1450 accordingly, documenting that `git fsck` unfortunately no longer warns about these issues by default.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- Documentation/fsck-msgids.txt | 4 ++-- fsck.h | 4 ++-- t/t1450-fsck.sh | 13 ++++++++++++- 3 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt index b06ec385aff..f5016ecda6a 100644 --- a/Documentation/fsck-msgids.txt +++ b/Documentation/fsck-msgids.txt @@ -158,13 +158,13 @@ (WARN) Tree contains entries pointing to a null sha1. `symlinkPointsToGitDir`:: - (WARN) Symbolic link points inside a gitdir. + (INFO) Symbolic link points inside a gitdir. `symlinkTargetBlob`:: (ERROR) A non-blob found instead of a symbolic link's target. `symlinkTargetLength`:: - (WARN) Symbolic link target longer than maximum path length. + (INFO) Symbolic link target longer than maximum path length. `symlinkTargetMissing`:: (ERROR) Unable to read symbolic link target's blob. diff --git a/fsck.h b/fsck.h index 130fa8d8f91..d41ec98064b 100644 --- a/fsck.h +++ b/fsck.h @@ -74,8 +74,6 @@ enum fsck_msg_type { FUNC(NULL_SHA1, WARN) \ FUNC(ZERO_PADDED_FILEMODE, WARN) \ FUNC(NUL_IN_COMMIT, WARN) \ - FUNC(SYMLINK_TARGET_LENGTH, WARN) \ - FUNC(SYMLINK_POINTS_TO_GIT_DIR, WARN) \ /* infos (reported as warnings, but ignored by default) */ \ FUNC(BAD_FILEMODE, INFO) \ FUNC(GITMODULES_PARSE, INFO) \ @@ -84,6 +82,8 @@ enum fsck_msg_type { FUNC(MAILMAP_SYMLINK, INFO) \ FUNC(BAD_TAG_NAME, INFO) \ FUNC(MISSING_TAGGER_ENTRY, INFO) \ + FUNC(SYMLINK_TARGET_LENGTH, INFO) \ + FUNC(SYMLINK_POINTS_TO_GIT_DIR, INFO) \ /* ignored (elevated when requested) */ \ FUNC(EXTRA_HEADER_ENTRY, IGNORE) diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index 5669872bc80..8339e60efb2 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -1032,7 +1032,18 @@ test_expect_success 'fsck warning on symlink target with excessive length' ' warning in blob $symlink_target: symlinkTargetLength: symlink target too long EOF git fsck --no-dangling >actual 2>&1 && - test_cmp expected actual + test_cmp expected actual && + + test_when_finished "git tag -d symlink-target-length" && + git tag symlink-target-length $tree && + test_when_finished "rm -rf throwaway.git" && + git init --bare throwaway.git && + git --git-dir=throwaway.git config receive.fsckObjects true && + git --git-dir=throwaway.git config receive.fsck.symlinkTargetLength error && + test_must_fail git push throwaway.git symlink-target-length && + git --git-dir=throwaway.git config --unset receive.fsck.symlinkTargetLength && + git push throwaway.git symlink-target-length 2>err && + grep "warning.*symlinkTargetLength" err ' test_expect_success 'fsck warning on symlink target pointing inside git dir' '