Re: [RFC/PATCH] verify-tag: add --check-name flag

From: Santiago Torres <santiago@nyu.edu>
Date: Jun 7, 2016, 21:30 UTC
Message-ID: <20160607213050.GF24676@LykOS.localdomain>
In-Reply-To: <20160607211707.GA7981@sigill.intra.peff.net>

On Tue, Jun 07, 2016 at 05:17:07PM -0400, Jeff King wrote:

Show 11 quoted lines

> That is much more flexible, as they could even do some more complicated
> matching than a single string (though in practice, for security things,
> I think simpler is better).
> 
> I think this option is going to become a blueprint for other "extended"
> checks, too. E.g., you might also want to check that the tagger ident
> matches the uid on the signing key.
> 
> My main worry is that we'll accrue a whole bunch of such logic. And even
> though each one is relatively simple, it would be nice for callers to be
> able to ask us to just do the standard safety checks.

I agree with this. I can't think of other checks off the top of my head, but I wouldn't be surprised if this is the case.

I think that having custom flags for each check can also derive in each package manager/user picking each check based on many different rationales, which might lead to people overcomplicating things?

Show 10 quoted lines

> 
> If we do go with the "print it out and let the caller do their own
> checks" strategy, I think I'd prefer rather than "--show-tagname" to
> just respect the "--format" we use for tag-listing. That would let you
> do:
> 
>   git tag -v --format='%(tag)%n%(tagger)'
> 
> or similar. In fact you can already do that with a separate step (modulo
> %n, which we do not seem to understand here), but like your example:

It worries me that, in this case, the patches for upstream managers might be harder to integrate/pitch for users.

Also, maybe we could take both strategies? add a --check-name for verify-tag and a --format for tag -v (I think either change is easy enough to do).

Show 15 quoted lines

> 
> > Or it could even do this:
> > 
> > 	tag="$1"
> > 	if ! git tag -v "$tag"
> > 	if ! git tag -v "$tag"
> >         then
> > 		echo >&2 "Bad tag."
> >                 exit 1
> > 	fi
> > +	tag=$(git tag --show-tagname $tag)
> > 	make dest=/usr/local/$package/$tag install
> 
> It is racy. That probably doesn't matter for most callers, but it would
> be nice to be able to get a custom format out of the "-v" invocation.

Oh yeah, I didn't consider this either. I also don't think it's such an issue, but it sounds like a good idea not to have these races.

> 
> -Peff

Thanks! -Santiago.