Validate ref names arriving from a remote

Commit

Soph2w ago

Nothing checked the names a remote advertises, and git-sync acts on them in two ways that make a malformed one dangerous.

convert-sha256 writes them to disk. go-git resolves a ref name through the repository filesystem, so "refs/heads/../../config" escapes refs/heads. go-billy clamps the traversal at the repository root, so this is repository corruption rather than arbitrary file write — but config, HEAD, and packed-refs are all in range, with the content constrained to an object hash.

Ref-update commands embed them in the receive-pack request. receive-pack reads a feature list from everything after the first NUL on a command line, so a source advertising a name containing NUL can inject capabilities into the push git-sync sends to the target.

Validation defers to plumbing.ReferenceName.Validate, which implements git's check_refname_format: "..", NUL, CR, LF, other control characters, DEL, space, "~^:?[", backslash, "@{", a ".lock" suffix, a leading dot, empty components, and a leading dash on a branch or tag. Deferring rather than hand-rolling keeps git-sync's notion of a valid ref identical to git's, so nothing git considers legitimate gets skipped — verified against refs/pull/, refs/notes/*, and nested branch names.

It runs at the two decode boundaries (AdvRefsToSlice and decodeV2LSRefs) rather than at each call site, so every path that reads an advertisement is covered by construction, on both the source and target side. A skipped target ref is also one the planner never sees, so it is never picked as a prune candidate — the safe direction.

Bad names are skipped with a warning rather than failing the run, matching how per-ref push rejections behave under BestEffort: one malformed ref upstream should not stop a mirror. The warning prints names with %q, because they are remote-controlled and an embedded escape sequence would otherwise reach the terminal; the individual list is capped while the count still covers everything skipped.

--map values get the same validation but are rejected outright rather than skipped. They are configuration, and a mapped target ref reaches the same places an advertised one does, so failing at startup beats silently not mirroring what was asked for.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com Entire-Checkpoint: 01M0D2HM0JN5QVYSWAPFZCSPNR

Checkpoints

Security Review and Vulnerability Fixes

Claude CodeOpus 5.[1m]
View session
Checkpoint 1