Reject --exclude-ref-prefix values that drop branches or tags

52b5aae→main·

nodo·1mo ago·3 files·+101 added/-1 removed

The convert-sha256 docs promise that every branch and tag is always converted, since dropping any of them risks stranding cross-branch SHA1 references in commit and tag messages — the exact invariant the message-rewrite pass exists to maintain. The previous code piped req.ExcludeRefPrefixes straight into planner.BuildDesiredRefs, which applies it to branch and tag selection too, so the flag silently broke the promise.

Validate at the top of Run: refuse any prefix that, under the planner's HasPrefix matching, would catch a refs/heads/* or refs/tags/* name. The check covers bare "", "refs/", partial "refs/h", whole "refs/heads/" / "refs/tags/", and any sub- namespace under either. Help text on convert-sha256 now also calls the rejection out.

Sessions

Transcript data is unavailable for this checkpoint.

Changes

3

• cmd/git-sync
  • Mconvert_sha256.go+3/-1
  • internal/sha256convert
    • Msha256convert.go+40
    • Msha256convert_test.go+58

26 unmodified lines

27
28
29
30
30
31
32
33
34
35

26 unmodified lines

risks stranding cross-branch references in commit messages. Pass
--all-refs to also include refs/notes/*, refs/pull/*, and other custom
namespaces; pass --exclude-ref-prefix to subtract specific namespaces
from --all-refs.
from --all-refs. Exclude prefixes that would drop any branch or tag
e.g. refs/heads/feature/, refs/tags/, refs/) are rejected at run time
to preserve the always-convert invariant.

The conversion is destructive in two ways the caller should be aware of:
GPG signatures on commits and tags are dropped (they sign over the

Mcmd/git-sync/convert_sha256.go+3/-1

205 unmodified lines

206
207
208
209
210
211
212
213
214
215
216
217
218
385 unmodified lines

604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
626
627
628
629
630
631
632
633
634
635
636
637
638
639
640
641
642

205 unmodified lines

if req.TargetDir == "" {
        return Result{}, errors.New("convert-sha256 requires a target directory")
    }
    // Enforce the documented invariant: every branch and every tag is
    // always converted. Otherwise the partial set could strand
    // cross-branch hash references in commit and tag messages, which
    // the message-rewrite pass is built to keep intact.
    if bad := protectedExcludePrefixes(req.ExcludeRefPrefixes); len(bad) > 0 {
        return Result{}, fmt.Errorf("convert-sha256 refuses --exclude-ref-prefix values that would drop branches or tags: %s (only namespaces outside refs/heads/ and refs/tags/ may be excluded)", strings.Join(bad, ", "))
    }
    out := req.Out
    if out == nil {
        out = os.Stderr
    }
385 unmodified lines

attestationTagPrefix = "refs/tags/converted/"
}

// protectedExcludePrefixes returns the subset of prefixes that, under
// planner.IsRefExcluded's string-prefix semantics, would knock out at
// least one branch or tag. A prefix matches a branch if either side
// is a string-prefix of the other against "refs/heads/" (and likewise
// for "refs/tags/"). That covers:
//  
//   - bare "" (excludes every ref)
//   - "refs/" or "refs/h", "refs/heads/" (whole branch namespace)
//   - "refs/heads/feature/" (some branches)
//   - "refs/tags/" and any narrower suffix
//  
// Returned in input order, with duplicates removed, so the error
// message shows the user exactly which flag values to drop.
func protectedExcludePrefixes(prefixes []string) []string {
    protected := []string{"refs/heads/", "refs/tags/"}
    var bad []string
    seen := map[string]struct{}{}
    for _, raw := range prefixes {
        p := strings.TrimSpace(raw)
        if _, dup := seen[p]; dup {
            continue
        }
        for _, prot := range protected {
            if strings.HasPrefix(p, prot) || strings.HasPrefix(prot, p) {
                bad = append(bad, raw)
                seen[p] = struct{}{}
                break
            }
        }
    }
    return bad
}

// ensureEmptyTarget refuses to init into a non-empty directory so the user
// doesn't quietly accumulate objects into an existing repo.
func ensureEmptyTarget(path string) error {

Mcmd/git-sync/internal/sha256convert/sha256convert.go+40

970 unmodified lines

971
972
973
974
975
976
977
978
979
980
981
982
983
984
985
986
987
988
989
990
991
992
993
994
995
996
997
998
999
1000
1001
1002
1003
1004
1005
1006
1007
1008
1009
1010
1011
1012
1013
1014
1015
1016
1017
1018
1019
1020
1021
1022
1023
1024
1025
1026
1027
1028
1029
1030
1031
1032
1033
1034

970 unmodified lines

// against a real repo.
var _ = (*filesystem.Storage)(nil)

func TestProtectedExcludePrefixes(t *testing.T) {
    tests := []struct {
        name     string
        prefixes []string
        want     []string
    }{
        {"nil input", nil, nil},
        {"single benign namespace", []string{"refs/pull/"}, nil},
        {"multiple benign namespaces", []string{"refs/pull/", "refs/notes/", "refs/changes/"}, nil},
        {"whole branches namespace banned", []string{"refs/heads/"}, []string{"refs/heads/"}},
        {"whole tags namespace banned", []string{"refs/tags/"}, []string{"refs/tags/"}},
        {"branch sub-namespace banned", []string{"refs/heads/feature/"}, []string{"refs/heads/feature/"}},
        {"tag sub-namespace banned", []string{"refs/tags/v1/"}, []string{"refs/tags/v1/"}},
        {"refs/ banned because it would drop everything", []string{"refs/"}, []string{"refs/"}},
        {"empty string banned (would drop every ref)", []string{""}, []string{""}},
        {"partial refs/h banned (covers refs/heads/)" , []string{"refs/h"}, []string{"refs/h"}},
        {"mixed input reports only the bad ones, in order", []string{"refs/pull/", "refs/heads/", "refs/notes/", "refs/tags/v1.0"}, []string{"refs/heads/", "refs/tags/v1.0"}},
        {"duplicates collapsed", []string{"refs/heads/", "refs/heads/"}, []string{"refs/heads/"}},
        {"trims whitespace before matching", []string{"  refs/heads/  "}, []string{"  refs/heads/  "}},
    }
    for _, tt := range tests {
        t.Run(tt.name, func(t *testing.T) {
            got := protectedExcludePrefixes(tt.prefixes)
            if len(got) != len(tt.want) {
                t.Fatalf("protectedExcludePrefixes(%v) = %v, want %v", tt.prefixes, got, tt.want)
            }
            for i := range got {
                if got[i] != tt.want[i] {
                    t.Fatalf("protectedExcludePrefixes(%v)[%d] = %q, want %q", tt.prefixes, i, got[i], tt.want[i])
                }
            }
        }
    }
}

func TestRun_RejectsExcludePrefixesThatDropBranchesOrTags(t *testing.T) {
    // We never reach the network here — the validation fires before
    // any I/O — so a non-empty target dir is the only thing the early
    // path needs.
    dst := t.TempDir()
    req := Request{
        SourceURL:          "http://example.invalid/repo.git",
        TargetDir:          filepath.Join(dst, "out"),
        ExcludeRefPrefixes: []string{"refs/pull/", "refs/heads/feature/"},
    }
    _, err := Run(t.Context(), req)
    if err == nil {
            t.Fatalf("Run accepted --exclude-ref-prefix refs/heads/feature/, expected refusal")
    }
    msg := err.Error()
    if !strings.Contains(msg, "refs/heads/feature/") {
        t.Fatalf("error did not name the offending prefix: %v", err)
    }
    if !strings.Contains(msg, "exclude-ref-prefix") {
        t.Fatalf("error did not mention the flag: %v", err)
    }
}

func TestPickHEAD(t *testing.T) {
    branch := func(name string) planner.DesiredRef {
        ref := plumbing.ReferenceName("refs/heads/" + name)