Refuse source refs that collide with side outputs · Entire
Refuse source refs that collide with side outputs
91a8626→main·
nodo·1mo ago·2 files·+137 added/-13 removed
writeRefs lands the source ref set on the target, then writeOriginNotes and signBranchTips publish their own refs on top. A source repo that already advertised refs/notes/sha1-origin (under --all-refs) or any refs/tags/converted/* (always, since tags are mandatory) would have those refs silently clobbered.
Add checkSideOutputCollision: run after planner.BuildDesiredRefs and before any object work, refusing with an actionable message that names the offending ref(s) and points at --no-origin-notes / --exclude-ref-prefix / dropping --sign as escapes.
Also tighten --check to skip only the side-output refs we actually wrote: pass the {origin notes, signed tags} set into runChecks instead of pattern-matching by prefix, so a legitimate source ref that happened to share a namespace is not silently hidden from the resolved/expected fraction.
Sessions
Transcript data is unavailable for this checkpoint.
Changes
2
cmd/git-sync/internal/sha256convert
Msha256convert.go+59/-13
Msha256convert_test.go+78
262 unmodified lines
263
264
265
266
267
268
269
270
271
272
273
274
275
276
126 unmodified lines
403
404
405
398
406
407
408
409
410
411
412
413
414
415
416
417
418
419
75 unmodified lines
495
496
497
480
498
499
500
501
502
503
504
505
506
32 unmodified lines
539
540
541
519
520
521
522
523
542
543
544
545
546
547
548
549
550
4 unmodified lines
555
556
557
534
535
536
537
538
558
559
560
561
562
543
563
564
565
566
90 unmodified lines
657
658
659
660
661
662
663
664
665
666
667
668
669
670
671
672
673
674
675
676
677
678
679
680
681
682
683
684
685
686
687
688
262 unmodified lines
return Result{}, errors.New("no source refs matched the requested scope")
}
// Refuse before any further I/O if the source carries refs that
// would collide with our side outputs. writeRefs runs before
// writeOriginNotes / signBranchTips, so without this check the
// later side-output write would silently clobber the source ref.
if err := checkSideOutputCollision(desired, req.SkipOriginNotes, req.Sign); err != nil {
return Result{}, err
}
// Fetch into temp SHA1 store ------------------------------------------
fmt.Fprintf(out, "fetching %d ref(s) from %s ...\n", len(desired), req.SourceURL)
gpDesired := convert.DesiredRefs(desired)
262 unmodified lines
if req.Check {
fmt.Fprintln(out, "verifying output ...")
res.Checks = runChecks(ctx, req.TargetDir, dstRepo, refsWritten)
// Collect the side outputs this run actually wrote so the
// refs check knows which target refs to ignore. Anything not
// in here is assumed to be a translated source ref.
sideOutputs := make(map[plumbing.ReferenceName]struct{}, 1+len(res.SignedTags))
if res.OriginNotesRef != "" {
sideOutputs[plumbing.ReferenceName(res.OriginNotesRef)] = struct{}{}
}
for _, tag := range res.SignedTags {
sideOutputs[plumbing.ReferenceName(tag)] = struct{}{}
}
res.Checks = runChecks(ctx, req.TargetDir, dstRepo, refsWritten, sideOutputs)
for _, c := range res.Checks {
mark := "✓"
if !c.OK {
75 unmodified lines
// Returns one Check per step. Callers print and/or fail-on-error
// based on these. No early return so users see the full picture even when an
// earlier check fails.
func runChecks(ctx context.Context, targetDir string, repo *git.Repository, refsExpected int) []Check {
//
// sideOutputs holds the exact refs the run created on top of the
// source set (the origin-notes ref, any --sign attestation tags), so
// the refs check can omit them from the resolved/expected fraction
// without false-positive-skipping a same-named source ref.
func runChecks(ctx context.Context, targetDir string, repo *git.Repository, refsExpected int, sideOutputs map[plumbing.ReferenceName]struct{}) []Check {
checks := []Check{}
// 1. Config: extensions.objectformat = sha256. Parse the file
32 unmodified lines
}
}
// 3. Every written ref resolves to an existing object. Skip refs we
// add as side outputs (the origin-notes ref and any
// refs/tags/converted/* attestation tags from --sign), since they
// are accounted for in their own Result fields and would otherwise
// make the displayed fraction misleading.
// 3. Every written ref resolves to an existing object. Skip the
// specific refs this run created as side outputs — they're
// accounted for in their own Result fields and would otherwise
// make the displayed fraction misleading. Skipping by exact name
// (not by prefix) avoids hiding a legitimate source ref that
// happened to share a namespace.
resolved := 0
missing := ""
refs, err := repo.References()
4 unmodified lines
if r.Type() != plumbing.HashReference {
return nil
}
name := r.Name()
if name == plumbing.ReferenceName(originNotesRef) {
return nil
}
if strings.HasPrefix(string(name), attestationTagPrefix) {
if _, skip := sideOutputs[r.Name()]; skip {
return nil
}
if _, err := repo.Storer.EncodedObject(plumbing.AnyObject, r.Hash()); err != nil {
if missing == "" {
missing = fmt.Sprintf("%s → %s: %v", name, r.Hash(), err)
missing = fmt.Sprintf("%s → %s: %v", r.Name(), r.Hash(), err)
}
return nil
}
90 unmodified lines
return bad
}
// checkSideOutputCollision refuses the conversion when the source set
// already contains a ref name this run would later write as a side
// output. Without this guard, writeRefs would publish the source's
// value first and writeOriginNotes / signBranchTips would silently
// overwrite it — losing the source ref and hiding the conflict.
func checkSideOutputCollision(desired map[plumbing.ReferenceName]planner.DesiredRef, skipOriginNotes, sign bool) error {
if !skipOriginNotes {
if _, conflict := desired[plumbing.ReferenceName(originNotesRef)]; conflict {
return fmt.Errorf("source already advertises %s; pass --no-origin-notes to keep that source ref, or --exclude-ref-prefix %s to drop it from the conversion", originNotesRef, originNotesRef)
}
}
if sign {
var clashes []string
for name := range desired {
if strings.HasPrefix(string(name), attestationTagPrefix) {
clashes = append(clashes, string(name))
}
}
if len(clashes) > 0 {
sort.Strings(clashes)
return fmt.Errorf("source has %s under %s, which collides with the attestation tags --sign would create; drop --sign or rename the source tag(s)", strings.Join(clashes, ", "), attestationTagPrefix)
}
}
return nil
}
// 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+59/-13
1028 unmodified lines
1029
1030
1031
1032
1033
1034
1035
1036
1037
1038
1039
1040
1041
1042
1043
1044
1045
1046
1047
1048
1049
1050
1051
1052
1053
1054
1055
1056
1057
1058
1059
1060
1061
1062
1063
1064
1065
1066
1067
1068
1069
1070
1071
1072
1073
1074
1075
1076
1077
1078
1079
1080
1081
1082
1083
1084
1085
1086
1087
1088
1089
1090
1091
1092
1093
1094
1095
1096
1097
1098
1099
1100
1101
1102
1103
1104
1105
1106
1107
1108
1109
1110
1111
1112
1028 unmodified lines
}
}
func TestCheckSideOutputCollision(t *testing.T) {
mk := func(name string) planner.DesiredRef {
ref := plumbing.ReferenceName(name)
return planner.DesiredRef{SourceRef: ref, TargetRef: ref}
}
tests := []struct {
name string
desired map[plumbing.ReferenceName]planner.DesiredRef
skipOriginNotes bool
sign bool
wantErrSubstring string
}{
{
name: "no collisions accepted",
desired: map[plumbing.ReferenceName]planner.DesiredRef{
"refs/heads/main": mk("refs/heads/main"),
"refs/tags/v1": mk("refs/tags/v1"),
},
wantErrSubstring: "",
},
{
name: "origin-notes collision refused by default",
desired: map[plumbing.ReferenceName]planner.DesiredRef{
"refs/heads/main": mk("refs/heads/main"),
"refs/notes/sha1-origin": mk("refs/notes/sha1-origin"),
},
wantErrSubstring: "refs/notes/sha1-origin",
},
{
name: "origin-notes collision allowed when --no-origin-notes set",
desired: map[plumbing.ReferenceName]planner.DesiredRef{
"refs/notes/sha1-origin": mk("refs/notes/sha1-origin"),
},
skipOriginNotes: true,
wantErrSubstring: "",
},
{
name: "converted-tag collision refused only when --sign",
desired: map[plumbing.ReferenceName]planner.DesiredRef{
"refs/heads/main": mk("refs/heads/main"),
"refs/tags/converted/main": mk("refs/tags/converted/main"),
},
sign: true,
wantErrSubstring: "refs/tags/converted/main",
},
{
name: "converted-tag without --sign passes through",
desired: map[plumbing.ReferenceName]planner.DesiredRef{
"refs/tags/converted/main": mk("refs/tags/converted/main"),
},
sign: false,
wantErrSubstring: "",
},
{
name: "multiple converted-tag collisions listed in sorted order",
desired: map[plumbing.ReferenceName]planner.DesiredRef{
"refs/tags/converted/zeta": mk("refs/tags/converted/zeta"),
"refs/tags/converted/alpha": mk("refs/tags/converted/alpha"),
},
sign: true,
wantErrSubstring: "refs/tags/converted/alpha, refs/tags/converted/zeta",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := checkSideOutputCollision(tt.desired, tt.skipOriginNotes, tt.sign)
switch {
case tt.wantErrSubstring == "" && err != nil:
t.Fatalf("unexpected error: %v", err)
case tt.wantErrSubstring != "" && err == nil:
t.Fatalf("expected error containing %q, got nil", tt.wantErrSubstring)
case tt.wantErrSubstring != "" && !strings.Contains(err.Error(), tt.wantErrSubstring):
t.Fatalf("error %q does not contain %q", err.Error(), tt.wantErrSubstring)
}
})
}
}
func TestPickHEAD(t *testing.T) {
branch := func(name string) planner.DesiredRef {
ref := plumbing.ReferenceName("refs/heads/" + name)