From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-3.8 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_PASS, SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id 6B07F1F9FD for ; Tue, 16 Feb 2021 12:51:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230010AbhBPMtG (ORCPT ); Tue, 16 Feb 2021 07:49:06 -0500 Received: from cloud.peff.net ([104.130.231.41]:33972 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229993AbhBPMtF (ORCPT ); Tue, 16 Feb 2021 07:49:05 -0500 Received: (qmail 12955 invoked by uid 109); 16 Feb 2021 12:48:23 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Tue, 16 Feb 2021 12:48:23 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 23586 invoked by uid 111); 16 Feb 2021 12:48:22 -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; Tue, 16 Feb 2021 07:48:22 -0500 Authentication-Results: peff.net; auth=none Date: Tue, 16 Feb 2021 07:48:22 -0500 From: Jeff King To: =?utf-8?B?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason Cc: Blake Burkhart , Junio C Hamano , git Subject: Re: [PATCH 1/2] fsck: make symlinked .gitignore and .gitattributes a warning Message-ID: References: <87y2foaltl.fsf@evledraar.gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Mon, Feb 15, 2021 at 08:16:00PM -0500, Jeff King wrote: > That said, they'd probably want to checkout those old commits, too. So > we probably do need a config override, even if it's a broad one ("trust > me, this repo is OK, just allow symlinks for these special files"). Another option here is setting core.symlinks to false. That works more broadly than just the one symlink, though. It might be possible to apply that same setting (perhaps automatically, even) to just these .gitattributes, etc, metafiles. That may get tricky, though, as we'd need to do it not just on checkout, but any time we're considering the file (because we wouldn't want "git add" to re-add it as a non-symlink, nor git-diff to report it, etc). > > So aren't we both making the fsck check too loose and the client too > > strict? Would anyone care if this was an error on fsck if we did the "is > > outside repo?" check? > > An outside-the-repo check would probably be less invasive, but: > > - it still allows broken setups > > - it's significantly more complex. I know that the implementation I > showed errs on the side of complaining in at least some cases > (because it doesn't know if intermediate paths are themselves > symlinks). But I'd worry there are also cases where it covers too > little, nullifying the protection. Adding to the "complexity" point: it's also impossible to implement via fsck, where we do not have the full path of the tree entry. We could live without the fsck support if need be, though. I am beginning to wonder if just opening them all with O_NOFOLLOW (and a hacky 2-syscall fallback for portability) might be less ugly than all of this. -Peff