git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 0/8] tightening ref handling outside of refs/

From
Jeff King <peff@peff.net>
Date
Apr 29, 2024, 08:15 UTC
Message-ID
<20240429081512.GA4130242@coredump.intra.peff.net>
This is picking up the discussion from:
  https://lore.kernel.org/git/20240426211529.GD13703@coredump.intra.peff.net/

The basic issue is that we don't really enforce pseudoref syntax for names outside of "refs/", so you update random files in .git like:

  git update-ref objects/info/commit-graphs/commit-graph-chain HEAD
This is mitigated a bit by:
  1. You can't usually _overwrite_ files unless they look vaguely
     sha1-ish (in this case the chain file contains hashes of graph
     files, which is enough). So high-ticket items like "config" should
     be immune.
  2. Receive-pack is a bit more careful here, and refuses anything
     outside of "refs/". So you can't get up to any mischief via "git
     push".

But I still find it a bit scary/weird in general. And as noted in that thread, there's some attempt to enforce this that is done inconsistently. So you can update and read such refs, but are forbidden to delete them.

Of course all of this becomes a non-issue with reftables, where those names are not used in the filesystem at all. But even there I think we'd probably want to consistently enforce the syntax rules (both between delete/update/read, but also consistency with the files backend).

This series teaches check_refname_format() to enforce these rules (when instructed; see patch 6 for a discussion of all sorts of complications).

These changes are not backwards-compatible! But that is kind of the point. This is stuff that was never supposed to work. My concern would just be that somehow somebody is relying on it. Pay attention specifically to patches 4, 7, and 8, which are where the behavior changes are.

  [1/8]: t0600: don't create ref outside of refs/
  [2/8]: t5619: use fully qualified refname for branch
  [3/8]: refs: move is_pseudoref_syntax() definition earlier
  [4/8]: refs: disallow dash in pseudoref syntax
  [5/8]: refs: use is_pseudoref_syntax() in refname_is_safe()
  [6/8]: check_refname_format(): add FULLY_QUALIFIED flag
  [7/8]: refs: check refnames as fully qualified when writing
  [8/8]: refs: check refnames as fully qualified when resolving
 refs.c                                     | 59 ++++++++++++----------
 refs.h                                     |  1 +
 t/t0600-reffiles-backend.sh                |  2 +-
 t/t1430-bad-ref-name.sh                    | 20 ++++++++
 t/t5619-clone-local-ambiguous-transport.sh |  4 +-
 5 files changed, 57 insertions(+), 29 deletions(-)
-Peff
Next: Jeff King
Message 1 of 7 in “tightening ref handling outside of refs/”
  1. 0/8 tightening ref handling outside of refs/Jeff King, Apr 29, 2024
  2. 1/8 t0600: don't create ref outside of refs/Jeff King, Apr 29, 2024
  3. Jeff KingApr 29, 2024
  4. Jeff KingApr 29, 2024
  5. Patrick SteinhardtApr 29, 2024
  6. Jeff KingApr 30, 2024
  7. Junio C HamanoApr 29, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.