Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On Mon, Mar 12, 2018 at 04:37:47PM -0700, Jonathan Nieder wrote: > Jeff King wrote: > > > We could even give it an environment variable, which would allow > > something like: > > > > tar xf maybe-evil.git.tar > > cd maybe-evil > > export GIT_TRUST_REPO=false > > git log > [...] > As an

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

Jeff King wrote: > We could even give it an environment variable, which would allow > something like: > > tar xf maybe-evil.git.tar > cd maybe-evil > export GIT_TRUST_REPO=false > git log Interesting idea. Putting it in an envvar means it gets inherited by child processes, which if I

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On Mon, Mar 12, 2018 at 03:43:55PM -0700, Jonathan Nieder wrote: > Hi, > > Jeff King wrote: > > On Thu, Feb 22, 2018 at 01:26:34PM -0800, Jonathan Nieder wrote: > > >> Keep in mind that git upload-archive (a read-only command, just like > >> git upload-pack) also already has the same issues. >

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

Hi, Jeff King wrote: > On Thu, Feb 22, 2018 at 01:26:34PM -0800, Jonathan Nieder wrote: >> Keep in mind that git upload-archive (a read-only command, just like >> git upload-pack) also already has the same issues. > > Yuck. I don't think we've ever made a historical promise about that. But >

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On Fri, Feb 23, 2018 at 01:09:04PM -0800, Brandon Williams wrote: > > By the way, any decision here would presumably need to be extended to > > git-serve, etc. The current property is that it's safe to fetch from an > > untrusted repository, even over ssh. If we're keeping that for protocol > >

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On 02/22, Jeff King wrote: > On Thu, Feb 22, 2018 at 03:19:40PM -0500, Jeff King wrote: > > > > To be clear, which of the following are you (most) worried about? > > > > > > 1. being invoked with --help and spawning a pager > > > 2. receiving and acting on options between 'git' and

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On Thu, Feb 22, 2018 at 06:05:15PM -0500, Jeff King wrote: > On Thu, Feb 22, 2018 at 02:42:35PM -0800, Jonathan Nieder wrote: > > > > I couldn't quite get it to work, but I think it's because I'm doing > > > something wrong with the submodules. But I also think this attack would > > > _have_ to

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On Thu, Feb 22, 2018 at 02:42:35PM -0800, Jonathan Nieder wrote: > > I couldn't quite get it to work, but I think it's because I'm doing > > something wrong with the submodules. But I also think this attack would > > _have_ to be done over ssh, because on a local system the submodule > > clone

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

Jeff King wrote: > All of that said, I think the current code is quite dangerous already, > and maybe even broken. upload-pack may run sub-commands like rev-list > or pack-objects, which are themselves builtins. Sounds like more commands to set the IGNORE_PAGER_CONFIG flag for in git.c. Thanks

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On Thu, Feb 22, 2018 at 04:44:02PM -0500, Jeff King wrote: > But I don't think it _is_ an accident waiting to happen for the rest of > the commands. upload-pack is special. The point is that people may touch > git.c thinking they are adding a nice new feature (like pager config, or > aliases, or

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On Thu, Feb 22, 2018 at 01:26:34PM -0800, Jonathan Nieder wrote: > Keep in mind that git upload-archive (a read-only command, just like > git upload-pack) also already has the same issues. Yuck. I don't think we've ever made a historical promise about that. But then, I don't think the promise

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On Thu, Feb 22, 2018 at 01:24:02PM -0800, Jonathan Nieder wrote: > > But my greater concern is that people who > > work on git.c should not have to worry about accidentally violating this > > principle when they add a new feature or config option. > > That sounds

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

Jeff King wrote: > The current property is that it's safe to fetch from an > untrusted repository, even over ssh. If we're keeping that for protocol > v1, we'd want it to apply to protocol v2, as well. Ah, this is what I had been missing (the non-ssh case). I see your point. I

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

Hi, Jeff King wrote: > On Thu, Feb 22, 2018 at 11:38:14AM -0800, Jonathan Nieder wrote: >> To be clear, which of the following are you (most) worried about? >> >> 1. being invoked with --help and spawning a pager >> 2. receiving and acting on options between 'git' and 'upload-pack' >> 3.

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On Thu, Feb 22, 2018 at 03:19:40PM -0500, Jeff King wrote: > > To be clear, which of the following are you (most) worried about? > > > > 1. being invoked with --help and spawning a pager > > 2. receiving and acting on options between 'git' and 'upload-pack' > > 3. repository discovery > > 4.

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On Thu, Feb 22, 2018 at 11:38:14AM -0800, Jonathan Nieder wrote: > >>> And possibly respecting pager.upload-pack, which would violate our rule > >>> that it is safe to run upload-pack in untrusted repositories. > >> > >> And this isn't an issue with receive-pack because this same guarantee > >>

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

Hi, Jeff King wrote: > On Thu, Feb 22, 2018 at 10:07:15AM -0800, Brandon Williams wrote: >> On 02/22, Jeff King wrote: >>> On Wed, Feb 21, 2018 at 01:44:22PM -0800, Jonathan Tan wrote: On Tue, 6 Feb 2018 17:12:41 -0800 Brandon Williams wrote: > In order to

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On Thu, Feb 22, 2018 at 10:07:15AM -0800, Brandon Williams wrote: > On 02/22, Jeff King wrote: > > On Wed, Feb 21, 2018 at 01:44:22PM -0800, Jonathan Tan wrote: > > > > > On Tue, 6 Feb 2018 17:12:41 -0800 > > > Brandon Williams wrote: > > > > > > > In order to allow for

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On 02/22, Jeff King wrote: > On Wed, Feb 21, 2018 at 01:44:22PM -0800, Jonathan Tan wrote: > > > On Tue, 6 Feb 2018 17:12:41 -0800 > > Brandon Williams wrote: > > > > > In order to allow for code sharing with the server-side of fetch in > > > protocol-v2 convert upload-pack

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On Wed, Feb 21, 2018 at 01:44:22PM -0800, Jonathan Tan wrote: > On Tue, 6 Feb 2018 17:12:41 -0800 > Brandon Williams wrote: > > > In order to allow for code sharing with the server-side of fetch in > > protocol-v2 convert upload-pack to be a builtin. > > > > Signed-off-by:

Re: [PATCH v3 04/35] upload-pack: convert to a builtin

On Tue, 6 Feb 2018 17:12:41 -0800 Brandon Williams wrote: > In order to allow for code sharing with the server-side of fetch in > protocol-v2 convert upload-pack to be a builtin. > > Signed-off-by: Brandon Williams As Stefan mentioned in [1], also

[PATCH v3 04/35] upload-pack: convert to a builtin

In order to allow for code sharing with the server-side of fetch in protocol-v2 convert upload-pack to be a builtin. Signed-off-by: Brandon Williams --- Makefile | 3 +- builtin.h | 1 + builtin/upload-pack.c | 67 +++