git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: Blake Burkhart <bburky@bburky.com>,
	Junio C Hamano <gitster@pobox.com>, git <git@vger.kernel.org>
Subject: Re: [PATCH 1/2] fsck: make symlinked .gitignore and .gitattributes a warning
Date: Tue, 16 Feb 2021 07:48:22 -0500	[thread overview]
Message-ID: <YCu/FoLl8p15mwio@coredump.intra.peff.net> (raw)
In-Reply-To: <YCsc0OePtrotjeg5@coredump.intra.peff.net>

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

  parent reply	other threads:[~2021-02-16 12:51 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-13 17:49 Limited local file inclusion with .mailmap symlinks and git-archive Blake Burkhart
2021-02-15 23:17 ` Jeff King
2021-02-15 23:18   ` [PATCH 1/2] fsck: make symlinked .gitignore and .gitattributes a warning Jeff King
2021-02-16  0:38     ` Ævar Arnfjörð Bjarmason
2021-02-16  1:16       ` Jeff King
2021-02-16  1:56         ` Junio C Hamano
2021-02-16 12:54           ` Jeff King
2021-02-16 12:48         ` Jeff King [this message]
2021-02-16 14:43           ` [PATCH 0/6] open in-tree files with O_NOFOLLOW Jeff King
2021-02-16 14:44             ` [PATCH 1/6] add open_nofollow() helper Jeff King
2021-02-16 14:54               ` Jeff King
2021-02-16 15:44                 ` Taylor Blau
2021-02-16 16:02                   ` Jeff King
2021-02-16 16:07                     ` Taylor Blau
2021-02-16 16:11                       ` Taylor Blau
2021-02-16 16:19                         ` Jeff King
2021-02-16 14:44             ` [PATCH 2/6] attr: convert "macro_ok" into a flags field Jeff King
2021-02-16 14:44             ` [PATCH 3/6] exclude: add flags parameter to add_patterns() Jeff King
2021-02-16 14:44             ` [PATCH 4/6] attr: do not respect symlinks for in-tree .gitattributes Jeff King
2021-02-16 14:44             ` [PATCH 5/6] exclude: do not respect symlinks for in-tree .gitignore Jeff King
2021-02-16 14:44             ` [PATCH 6/6] mailmap: do not respect symlinks for in-tree .mailmap Jeff King
2021-02-16 14:57               ` Jeff King
2021-02-25 19:25             ` [PATCH 0/6] open in-tree files with O_NOFOLLOW Junio C Hamano
2021-02-26  6:35               ` Jeff King
2021-02-15 23:19   ` [PATCH 2/2] disallow symlinked .mailmap files Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YCu/FoLl8p15mwio@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=bburky@bburky.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).