From b91a0d1914ef4a8e23106939fb368ea3b053d9e6 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sun, 1 Nov 2026 21:13:54 +0000 Subject: [PATCH] Validate derived candidate tags before checks --- docs/roadmap/implementation.md | 4 +++- internal/candidate/check.go | 3 +++ internal/git/prepare.go | 4 ++-- internal/git/remote.go | 5 +++-- internal/git/source.go | 2 +- internal/git/source_test.go | 8 ++++---- internal/git/tag.go | 8 ++++---- publish_test.go | 34 ++++++++++++++++++++++++++++++++++ 8 files changed, 54 insertions(+), 14 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index c8e8c6f..2fa26fd 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -280,7 +280,9 @@ consumption uses an owned cache. ## Stage 6: Reject invalid derived candidate tags before validation -**Status:** pending. +**Status:** complete. Git and candidate package tests and public invalid-tag +and publication regressions pass offline; malformed derived tags stop before +Go validation and callbacks while retaining candidate identity. **Files:** `internal/git/remote.go` and its current tag-helper callers/tests, `internal/candidate/check.go`, public candidate tests, and candidate guidance diff --git a/internal/candidate/check.go b/internal/candidate/check.go index a1424ce..2a711a4 100644 --- a/internal/candidate/check.go +++ b/internal/candidate/check.go @@ -105,6 +105,9 @@ func Capture(ctx context.Context, request Request) (Candidate, error) { if err := validateNote(note, request.Product, request.Version); err != nil { return captured, fmt.Errorf("release note %q: %w", identity.NotePath, err) } + if err := git.CheckTagRef(ctx, repositoryDir, identity.Tag); err != nil { + return captured, fmt.Errorf("validate local candidate tag: %w", err) + } if exists, err := hasLocalTag(ctx, repositoryDir, identity.Tag); err != nil { return captured, fmt.Errorf("inspect local tag %q: %w", identity.Tag, err) } else if exists { diff --git a/internal/git/prepare.go b/internal/git/prepare.go index 34e1604..5549e24 100644 --- a/internal/git/prepare.go +++ b/internal/git/prepare.go @@ -9,7 +9,7 @@ import ( // PrepareTaggedCheckout fetches one explicit tag ref, checks its peeled commit // against expectedCommit, and checks out that commit detached. func PrepareTaggedCheckout(ctx context.Context, repositoryDir, tag, remote, expectedCommit string) error { - if err := checkTagRef(ctx, repositoryDir, tag); err != nil { + if err := CheckTagRef(ctx, repositoryDir, tag); err != nil { return err } if !validObjectID(expectedCommit) { @@ -35,7 +35,7 @@ func PrepareTaggedCheckout(ctx context.Context, repositoryDir, tag, remote, expe // VerifyTaggedCheckout confirms that the existing local tag, HEAD, and clean // worktree all identify the expected commit. It does not fetch or check out. func VerifyTaggedCheckout(ctx context.Context, repositoryDir, tag, expectedCommit string) error { - if err := checkTagRef(ctx, repositoryDir, tag); err != nil { + if err := CheckTagRef(ctx, repositoryDir, tag); err != nil { return err } if !validObjectID(expectedCommit) { diff --git a/internal/git/remote.go b/internal/git/remote.go index 7fa1fdf..ccd9834 100644 --- a/internal/git/remote.go +++ b/internal/git/remote.go @@ -100,7 +100,7 @@ func (endpoint Endpoint) Recheck(ctx context.Context, repositoryDir string) erro // Query reads exact main and tag refs. A command failure is unreadable, // never evidence that a ref is absent. func (endpoint Endpoint) Query(ctx context.Context, repositoryDir, tag string) (Snapshot, error) { - if err := checkTagRef(ctx, repositoryDir, tag); err != nil { + if err := CheckTagRef(ctx, repositoryDir, tag); err != nil { return Snapshot{}, err } mainRef := "refs/heads/main" @@ -301,7 +301,8 @@ func finalizeRef(ref RemoteRef) RemoteRef { return ref } -func checkTagRef(ctx context.Context, repositoryDir, tag string) error { +// CheckTagRef validates a complete tag name using Git ref rules. +func CheckTagRef(ctx context.Context, repositoryDir, tag string) error { if tag == "" { return fmt.Errorf("invalid Git tag name %q", tag) } diff --git a/internal/git/source.go b/internal/git/source.go index 14d2362..2b6e586 100644 --- a/internal/git/source.go +++ b/internal/git/source.go @@ -151,7 +151,7 @@ func CaptureHead(ctx context.Context, repositoryDir string) (string, error) { // InspectTaggedSource reads the local HEAD, worktree state, and selected tag. // Both lightweight historical tags and annotated tags are supported. func InspectTaggedSource(ctx context.Context, repositoryDir, tag string) (TaggedSource, error) { - if err := checkTagRef(ctx, repositoryDir, tag); err != nil { + if err := CheckTagRef(ctx, repositoryDir, tag); err != nil { return TaggedSource{}, err } var source TaggedSource diff --git a/internal/git/source_test.go b/internal/git/source_test.go index 2bb59bb..a5b8d1b 100644 --- a/internal/git/source_test.go +++ b/internal/git/source_test.go @@ -35,8 +35,8 @@ func TestInspectTaggedSourceSupportsLightweightAndAnnotatedTags(t *testing.T) { func TestTagOperationsAcceptFullSlashContainingRefs(t *testing.T) { fixture := newRemoteFixture(t) tag := "integrations/check/v2.3.4" - if err := checkTagRef(context.Background(), fixture.work, tag); err != nil { - t.Fatalf("checkTagRef(%q): %v", tag, err) + if err := CheckTagRef(context.Background(), fixture.work, tag); err != nil { + t.Fatalf("CheckTagRef(%q): %v", tag, err) } object, err := CreateAnnotatedTag(context.Background(), fixture.work, tag, fixture.commit, []byte("release note\n")) if err != nil { @@ -57,8 +57,8 @@ func TestTagOperationsAcceptFullSlashContainingRefs(t *testing.T) { func TestTagReferenceValidationUsesGitRules(t *testing.T) { fixture := newRemoteFixture(t) for _, tag := range []string{"", "v1..2.3", "v1.2.3.lock", "v1.2.3/", "v1.2.3\\bad"} { - if err := checkTagRef(context.Background(), fixture.work, tag); err == nil { - t.Errorf("checkTagRef(%q) succeeded", tag) + if err := CheckTagRef(context.Background(), fixture.work, tag); err == nil { + t.Errorf("CheckTagRef(%q) succeeded", tag) } } } diff --git a/internal/git/tag.go b/internal/git/tag.go index 2788a97..19af2db 100644 --- a/internal/git/tag.go +++ b/internal/git/tag.go @@ -11,7 +11,7 @@ import ( // CreateAnnotatedTag records the captured note as an unsigned annotated tag // on the captured commit and returns the tag object's ID. func CreateAnnotatedTag(ctx context.Context, repositoryDir, tag, commit string, note []byte) (string, error) { - if err := checkTagRef(ctx, repositoryDir, tag); err != nil { + if err := CheckTagRef(ctx, repositoryDir, tag); err != nil { return "", err } if !validObjectID(commit) { @@ -71,7 +71,7 @@ func CreateAnnotatedTag(ctx context.Context, repositoryDir, tag, commit string, // InspectLocalTag confirms the selected tag still points to the exact // annotated tag object captured before publication. func InspectLocalTag(ctx context.Context, repositoryDir, tag, expectedObject string) error { - if err := checkTagRef(ctx, repositoryDir, tag); err != nil { + if err := CheckTagRef(ctx, repositoryDir, tag); err != nil { return err } if !validObjectID(expectedObject) { @@ -99,7 +99,7 @@ func InspectLocalTag(ctx context.Context, repositoryDir, tag, expectedObject str // one exact authoritative version tag and its peeled commit. Failed // verification or reads are unreadable and never interpreted as absence. func (endpoint Endpoint) QueryTag(ctx context.Context, repositoryDir, tag string) (RemoteRef, error) { - if err := checkTagRef(ctx, repositoryDir, tag); err != nil { + if err := CheckTagRef(ctx, repositoryDir, tag); err != nil { return RemoteRef{}, err } if err := endpoint.verifyScopedURLs(ctx, repositoryDir); err != nil { @@ -129,7 +129,7 @@ func (endpoint Endpoint) QueryTag(ctx context.Context, repositoryDir, tag string // PushTag publishes only the captured tag object to its exact tag ref. func (endpoint Endpoint) PushTag(ctx context.Context, repositoryDir, tag, tagObject string) error { - if err := checkTagRef(ctx, repositoryDir, tag); err != nil { + if err := CheckTagRef(ctx, repositoryDir, tag); err != nil { return err } if !validObjectID(tagObject) { diff --git a/publish_test.go b/publish_test.go index ca6b57f..587bbdb 100644 --- a/publish_test.go +++ b/publish_test.go @@ -18,6 +18,40 @@ import ( const releaseNote = "# Sample App v1.2.3\n\nA source release.\n\nUpgrade notes stay intact.\n" +func TestCheckRejectsInvalidDerivedTagBeforeValidation(t *testing.T) { + for _, module := range []string{"module space", "module.lock"} { + t.Run(module, func(t *testing.T) { + repo := newPublishFixture(t) + writeFile(t, repo.work, module+"/go.mod", "module example.com/nested\n\ngo 1.25.0\n") + writeFile(t, repo.work, module+"/docs/releases/v1.2.3.md", releaseNote) + gitRun(t, repo.work, "add", module) + gitRun(t, repo.work, "commit", "-m", "add module") + bin := t.TempDir() + marker := filepath.Join(t.TempDir(), "go-ran") + if err := os.WriteFile(filepath.Join(bin, "go"), []byte("#!/bin/sh\nprintf ran > \"$CANDIDATE_GO_MARKER\"\nexit 1\n"), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("CANDIDATE_GO_MARKER", marker) + called := false + got, err := releasekit.Check(context.Background(), releasekit.ReleaseRequest{ + RepositoryDir: repo.work, ModuleDir: module, Product: "Sample App", Version: "v1.2.3", + AdditionalChecks: func(context.Context, releasekit.Candidate) error { called = true; return nil }, + Build: &releasekit.BuildSpec{MainPackage: ".", BinaryName: "fixture", Targets: []releasekit.Target{{OS: "linux", Arch: "amd64"}}}, + }) + if err == nil || errors.Is(err, releasekit.ErrInvalidVersion) || !strings.Contains(err.Error(), "invalid Git tag name") || got.Tag != module+"/v1.2.3" || got.ModulePath != "example.com/nested" { + t.Fatalf("candidate = %#v, error = %v", got, err) + } + if called { + t.Fatal("additional checks ran") + } + if _, err := os.Stat(marker); !os.IsNotExist(err) { + t.Fatalf("Go validation or build ran: %v", err) + } + }) + } +} + func TestCandidateOperationsDiscoverGoOnlyWhenLaterWorkNeedsIt(t *testing.T) { gitPath := mustLookPath(t, "git") gitOnly := t.TempDir()