Re: [PATCH v3 04/35] upload-pack: convert to a builtin
From: Jeff King <hidden>
Date: 2018-03-12 23:52:06
On Mon, Mar 12, 2018 at 04:37:47PM -0700, Jonathan Nieder wrote:
Jeff King wrote:quoted
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 internal implementation detail, this is so obviously fragile that it wouldn't give me any feeling of security. ;-) So it should be strictly an improvement. As a public-facing feature, I suspect it's a bad idea for exactly that reason.
So that pretty much kills off the GIT_TRUST_REPO idea, I guess.
FWIW for pager specifically I am going for a whitelisting approach: new commands would have to explicitly set ALLOW_PAGER if they want to respect pager config. That doesn't guarantee people think about it again as things evolve but it should at least help with getting the right setting for new plumbing.
I suspect we'd be about as well off with the "don't trust the repo" internal flag. Touching the ALLOW_PAGER setup code is about as likely to set off red flags for the developers (or reviewers) as code that checks the "trust" flag. Forcing a whitelist on ALLOW_PAGER _is_ more likely to catch people adding new commands. But I don't think we actually want to add more commands to the "safe to run in a malicious repo" list. It's already a slightly sketchy concept. This is really all about upload-pack and its existing promises. But ALLOW_PAGER would _just_ fix the pager issue. When we inevitably find another problem spot, it won't help us there. But a global "trust" flag might. I dunno. I guess I'm OK with either approach, but it seems like the global trust flag has more room to grow. -Peff