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 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
