Re: [PATCH 4/6] fsck: check tag objects' headers

Jeff King [off-list ref] writes:

Hmm. But that is because "git tag" always makes one type of tag: one in
which the "tag" field is the same as the refname in which we store it.
So the name must be a valid refname there to meet the ref storage
requirement, and therefore the tag name must, too.

But is that something we necessarily need or want to enforce? Is it OK
for me to have refs/tags/foo pointing to a tag object that is not
related to "foo" (either semantically or syntactically)?

I dunno. I cannot think of a reason you would want to do such a thing,
but this seems like outlawing it because git does not generate it, not
because it is necessarily a problematic thing to be doing.

Thanks for straightening me out.

If "git fsck" were a tool to validate that the objects and refs are
in line with how "git-core" plumbing and Porcelain toolset uses the
underlying Git data model, it makes sense to insist a tag has a name
that is suitable for a refname, and the tag is pointed by a ref in
"refs/tags/" followed by its name.  The rules such a "git fsck" should
implement would be stricter than what the underlying Git data model
could represent and existing Git tools could handle (i.e. a commit
with broken ident line may not be usable with "shortlog -e" and would
be flagged as corrupt).

But tightening rules in that direction may risk hindering future
progress in an unnecessary way.  We may want to be a bit lenient
when we see something _unusual_ but not necessarily _wrong_, and the
line between them would be blurry in places, as Git is an evolving
software.  It is good to warn about an unsual ones, but we probably
would not want to error on them.

This tightening may be too strict without a very good reason.  For
example, a tentative signed tag (e.g. "for-linus") often used in a
pull request to have it recorded in the resulting merge by the
integrator does not inherently need to be named at all; the ref is
only necessary as a means to transfer the signature from the
contributor to the integrator, and once merged, there is no need for
the tag to have any name.  When we try to improve the workflow to
integrate authenticated work done on the side branch, we may come up
with a way to do so _without_ having to actually have a tag name
(i.e. the "tag" contributor creates for such a purpose may not be
done by "git tag -s" when asking the result to be pulled but do
something different, and it may be perfectly fine for such a
tentative tag to lack the "tag " name line), but still allows us to
record the same merge-tag in the resulting merge commit.

So...

On Sun, Aug 31, 2014 at 03:46:42PM -0700, Junio C Hamano wrote:

If "git fsck" were a tool to validate that the objects and refs are
in line with how "git-core" plumbing and Porcelain toolset uses the
underlying Git data model, it makes sense to insist a tag has a name
that is suitable for a refname, and the tag is pointed by a ref in
"refs/tags/" followed by its name.  The rules such a "git fsck" should
implement would be stricter than what the underlying Git data model
could represent and existing Git tools could handle (i.e. a commit
with broken ident line may not be usable with "shortlog -e" and would
be flagged as corrupt).

This is a bit of an aside, but why do we have the "tag" line in the tag
object in the first place?

It is part of the object contents, and therefore is part of the
signature (which the refname is not). That's somewhat redundant with the
tag message itself. E.g., the git v2.0.4 tag says:

  object 32f56600bb6ac6fc57183e79d2c1515dfa56672f
  type commit
  tag v2.0.4
  tagger Junio C Hamano [off-list ref] 1406755201 -0700

  Git 2.0.4
  -----BEGIN PGP SIGNATURE-----
  ...

Imagine an evil person pushed the signed v2.0.4 tag to refs/tags/v2.1.0
(perhaps because there is a bug in v2.0.4, and they want you to run the
wrong version so they can exploit it). You can check with "git show"
that the "tag" field is actually v2.0.4, but then you could similarly
check that the message says "Git 2.0.4".

The main advantage of the "tag" field is that it is machine-readable,
and that your verification process can check that "git verify-tag
v2.1.0" actually returns a tag that says "tag v2.1.0". But I do not
think we do that verification at all. I wonder if that is something we
should add support for.

You gave examples later in your email of tags that would not necessarily
care about this tag field (and anyway, if "for-linus" is used over and
over, it is subject to these sorts of replays), so I do not think it is
something we would want unconditionally in verify-tag.

I think this may need to be filed under "possible policy flags for
verifying" that we discussed earlier (i.e., in the same boat as "does
the committer ident match the commit signature", as it is a
porcelain-ish policy, not an integral part of the plumbing).

So this is mostly food for thought at this point.

-Peff