Thread (329 messages) 329 messages, 12 authors, 2018-03-14

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

From: Jeff King <hidden>
Date: 2018-02-22 20:19:46

On Thu, Feb 22, 2018 at 11:38:14AM -0800, Jonathan Nieder wrote:
quoted
quoted
quoted
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
doesn't exist?
Yes, exactly (which is confusing and weird, yes, but that's how it is).
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. pager config
 5. alias discovery
 6. increased code surface / unknown threats
My immediate concern is (4). 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.

In other words, it seems like an accident waiting to happen. I'd be more
amenable to it if there was some compelling reason for it to be a
builtin, but I don't see one listed in the commit message. I see only
"let's make it easier to share the code", which AFAICT is equally served
by just lib-ifying the code and calling it from the standalone
upload-pack.c.
Although in most setups the user does not control the config files on
a server, item (4) looks like a real issue worth solving.  I think we
should introduce a flag to skip looking for pager config.  We could
use it for receive-pack, too.
There's not much point for receive-pack. It respects hooks, so any
security ship has already sailed there.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help