| Message ID | 20201005071751.GA2290770@coredump.intra.peff.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | forbidding symlinked .gitattributes and .gitignore | expand |
Jeff King wrote: > About 2 years ago as part of a security release we made it illegal to > have a symlinked .gitmodules file (refusing it both in the index and via > fsck). At the time we discussed (on the security list) outlawing > symlinks for other .git files in the same way, but we decided not to do > so as part of the security release, as it wasn't strictly necessary. > > We publicly revisited the topic in: > > https://lore.kernel.org/git/20190114230902.GG162110@google.com/ > > but there were a few fixes needed, and it got forgotten. So here it is > again, with those fixes: Oh! I'm excited --- at $DAYJOB we've been carrying that patch since then; I'll be happy to drop it. :) Jonathan
On Mon, Oct 05, 2020 at 12:32:02AM -0700, Jonathan Nieder wrote: > Jeff King wrote: > > > About 2 years ago as part of a security release we made it illegal to > > have a symlinked .gitmodules file (refusing it both in the index and via > > fsck). At the time we discussed (on the security list) outlawing > > symlinks for other .git files in the same way, but we decided not to do > > so as part of the security release, as it wasn't strictly necessary. > > > > We publicly revisited the topic in: > > > > https://lore.kernel.org/git/20190114230902.GG162110@google.com/ > > > > but there were a few fixes needed, and it got forgotten. So here it is > > again, with those fixes: > > Oh! I'm excited --- at $DAYJOB we've been carrying that patch since > then; I'll be happy to drop it. :) Part of my ulterior motive is that we've been carrying a patch at GitHub to pass O_NOFOLLOW when opening in-tree attributes and ignore files (which we don't normally do on our servers, but do for things like GitHub Pages). But I think O_NOFOLLOW isn't perfectly portable (despite being in POSIX), and the patch is rather invasive. I also looked at one point into preventing just out-of-tree symlinks. That helps with out-of-repo reads, but it doesn't change the fact that in-index reads of symlinks are broken. We _could_ change that with a lot of work, but I don't think anybody cares enough about the feature to do so. -Peff
About 2 years ago as part of a security release we made it illegal to have a symlinked .gitmodules file (refusing it both in the index and via fsck). At the time we discussed (on the security list) outlawing symlinks for other .git files in the same way, but we decided not to do so as part of the security release, as it wasn't strictly necessary. We publicly revisited the topic in: https://lore.kernel.org/git/20190114230902.GG162110@google.com/ but there were a few fixes needed, and it got forgotten. So here it is again, with those fixes: [1/7]: fsck_tree(): fix shadowed variable [2/7]: fsck_tree(): wrap some long lines These first two are actually an unrelated fix and cleanup in the nearby code. Could be picked up independently. [3/7]: t7415: rename to expand scope [4/7]: t7450: test verify_path() handling of gitmodules Preparatory test cleanup and improvement for existing features. [5/7]: t0060: test obscured .gitattributes and .gitignore matching [6/7]: verify_path(): disallow symlinks in .gitattributes and .gitignore [7/7]: fsck: complain when .gitattributes or .gitignore is a symlink The actual feature, covering the index and fsck. fsck.c | 79 ++++++++++++++----- read-cache.c | 12 ++- t/helper/test-path-utils.c | 41 +++++++--- t/t0060-path-utils.sh | 20 +++++ ...odule-names.sh => t7450-bad-meta-files.sh} | 69 ++++++++++++++-- 5 files changed, 179 insertions(+), 42 deletions(-) rename t/{t7415-submodule-names.sh => t7450-bad-meta-files.sh} (77%) -Peff