From 92c95ef031335e968eaa00671c4c57e06e414dcd Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 2 Nov 2026 03:19:16 +0000 Subject: [PATCH] Hide credentials in Git endpoint diagnostics --- cmd/releasekit/main.go | 2 +- cmd/releasekit/main_test.go | 24 +++++++++++ docs/cli.md | 3 +- docs/consumers/publish.md | 5 ++- docs/roadmap/implementation.md | 2 +- internal/git/display.go | 39 ++++++++++++++++++ internal/git/display_test.go | 74 ++++++++++++++++++++++++++++++++++ internal/git/remote.go | 21 +++++----- internal/git/tag.go | 6 +-- internal/publish/publish.go | 8 ++-- publish.go | 2 +- publish_test.go | 21 ++++++++++ 12 files changed, 184 insertions(+), 23 deletions(-) create mode 100644 internal/git/display.go create mode 100644 internal/git/display_test.go diff --git a/cmd/releasekit/main.go b/cmd/releasekit/main.go index 1869463..92fee86 100644 --- a/cmd/releasekit/main.go +++ b/cmd/releasekit/main.go @@ -60,7 +60,7 @@ func run(ctx context.Context, args []string, stdout, stderr io.Writer) int { fmt.Fprintf(stderr, "validating and publishing module %s version %s\n", selectedModuleLabel(settings.ModuleDir), invocation.Version) result, err := releasekit.Publish(ctx, request) if err != nil { - fmt.Fprintf(stderr, "publication state=%s phase=%s module=%s module-dir=%s version=%s tag=%s commit=%s tag-object=%s endpoint=%q: %s\n", result.State, result.Phase, result.ModulePath, result.ModuleDir, result.Version, result.Tag, result.CandidateCommit, result.TagObject, result.Endpoint, candidateFailureText(err)) + fmt.Fprintf(stderr, "publication state=%s phase=%s module=%s module-dir=%s version=%s tag=%s commit=%s tag-object=%s endpoint=%q: %s\n", result.State, result.Phase, result.ModulePath, result.ModuleDir, result.Version, result.Tag, result.CandidateCommit, result.TagObject, git.DisplayURL(result.Endpoint), candidateFailureText(err)) if errors.Is(err, releasekit.ErrInvalidVersion) { return 2 } diff --git a/cmd/releasekit/main_test.go b/cmd/releasekit/main_test.go index ba91f54..b8ccfda 100644 --- a/cmd/releasekit/main_test.go +++ b/cmd/releasekit/main_test.go @@ -403,6 +403,30 @@ func TestCandidateFailurePresentationRetainsCallbackContext(t *testing.T) { } } +func TestPublishCommandRedactsCredentialsInEndpointMismatch(t *testing.T) { + repository := newCLIRepository(t) + writeRepositoryFile(t, repository, "docs/releases/v1.2.3.md", "# Example Product v1.2.3\n\nRelease note.\n") + settings := cli.Settings{Product: "Example Product", Remote: "origin", ExpectedPushURL: "https://expected-token@example.invalid/project.git"} + profile, err := json.Marshal(settings) + if err != nil { + t.Fatal(err) + } + writeRepositoryFile(t, repository, "releasekit.json", string(profile)) + cliGitRun(t, repository, "add", ".") + cliGitRun(t, repository, "commit", "-m", "configure release") + cliGitRun(t, repository, "remote", "add", "origin", "https://actual-user:actual-secret@example.invalid/project.git") + var stdout, stderr bytes.Buffer + status := run(context.Background(), []string{"publish", "--repo", repository, "v1.2.3"}, &stdout, &stderr) + if status != 1 || !strings.Contains(stderr.String(), "unexpected publication push URL") || !strings.Contains(stderr.String(), "example.invalid/project.git") { + t.Fatalf("status = %d, stdout = %q, stderr = %q", status, stdout.String(), stderr.String()) + } + for _, secret := range []string{"expected-token", "actual-user", "actual-secret"} { + if strings.Contains(stderr.String(), secret) || strings.Contains(stdout.String(), secret) { + t.Fatalf("CLI output exposes %q: %q %q", secret, stdout.String(), stderr.String()) + } + } +} + func TestPublishCommandPublishesConfiguredCandidate(t *testing.T) { raceSelected := racePrerequisitesAvailable() repository := newCLIRepository(t) diff --git a/docs/cli.md b/docs/cli.md index 9e86c31..ff632a4 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -91,7 +91,8 @@ and pushes only its exact full tag object. It prints the confirmed semantic version, full tag, candidate commit, and tag object on success. Validation progress and failed publication state, including the captured publication -endpoint for recovery, are written to standard error. A signal +endpoint for recovery, are written to standard error. HTTP(S) endpoint +userinfo is masked in diagnostics. A signal cancels the active operation; if it arrives during a push, the library's bounded reconciliation determines whether the exact tag was accepted. Shared validation failures use the same check and finding presentation as diff --git a/docs/consumers/publish.md b/docs/consumers/publish.md index 3cbe077..1c55247 100644 --- a/docs/consumers/publish.md +++ b/docs/consumers/publish.md @@ -118,5 +118,6 @@ query confirmed it after a push error. The local tag is retained after a failed push. `PublicationState` distinguishes a known failure, conflict, confirmed publication, and unresolved outcome. Resolve an unknown result against the captured endpoint before retrying; see the [maintainer procedure](../release.md) -for the recovery steps. The [CLI contract](../cli.md) documents command output -and exit statuses. +for the recovery steps. The result's endpoint is the exact URL and can contain +credentials; handle it accordingly. The [CLI contract](../cli.md) documents +command output and exit statuses. diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index d3accee..743876e 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -41,7 +41,7 @@ the agent policy; an intermediate compilation dependency is recorded accurately. | 3 | A02: source identity regression evidence | Complete — commit, tree, and blob replacement regressions and public publication, build, and delivery cases passed. | | 4 | A03: buffered command teardown | Complete — inherited-pipe, cancellation, healthy-command, native build, and publication recovery checks passed. | | 5 | A03, A18: piped lifetime and queued cancellation | Complete — process and Git lifecycle tests passed under the race detector; public preparation cancellation passed. | -| 6 | A04 | Pending | +| 6 | A04 | Complete — Git display, endpoint change, public mismatch, CLI, publication, and Git failure regressions passed. | | 7 | A05 | Pending | | 8 | A06, A14 | Pending | | 9 | A16 | Pending | diff --git a/internal/git/display.go b/internal/git/display.go new file mode 100644 index 0000000..fc5739f --- /dev/null +++ b/internal/git/display.go @@ -0,0 +1,39 @@ +package git + +import ( + "net/url" + "regexp" + "strings" +) + +var httpURLSpan = regexp.MustCompile(`(?i)https?://[^\s"'<>]+`) + +// DisplayURL hides HTTP userinfo without changing the endpoint used by Git. +func DisplayURL(raw string) string { + lower := strings.ToLower(raw) + if !strings.HasPrefix(lower, "http://") && !strings.HasPrefix(lower, "https://") { + return raw + } + authority := raw[strings.Index(raw, "://")+3:] + if end := strings.IndexAny(authority, "/?#"); end >= 0 { + authority = authority[:end] + } + if !strings.Contains(authority, "@") { + return raw + } + parsed, err := url.Parse(raw) + if err != nil || parsed.User == nil || parsed.Host == "" { + return "[redacted URL]" + } + parsed.User = url.User("redacted") + return parsed.String() +} + +func displayDiagnostic(message string) string { + return httpURLSpan.ReplaceAllStringFunc(message, DisplayURL) +} + +type displayError struct{ cause error } + +func (err displayError) Error() string { return displayDiagnostic(err.cause.Error()) } +func (err displayError) Unwrap() error { return err.cause } diff --git a/internal/git/display_test.go b/internal/git/display_test.go new file mode 100644 index 0000000..0ecb1e9 --- /dev/null +++ b/internal/git/display_test.go @@ -0,0 +1,74 @@ +package git + +import ( + "context" + "errors" + "os/exec" + "strings" + "testing" +) + +func TestDisplayURL(t *testing.T) { + for _, test := range []struct{ input, want string }{ + {"https://user:password@example.test/repo", "https://redacted@example.test/repo"}, + {"https://token@example.test/repo", "https://redacted@example.test/repo"}, + {"https://user%40example:secret%2Fvalue@example.test/repo", "https://redacted@example.test/repo"}, + {"http://broken%zz:secret@example.test/repo", "[redacted URL]"}, + {"https://example.test/repo", "https://example.test/repo"}, + {"git@example.test:owner/repo", "git@example.test:owner/repo"}, + } { + if got := DisplayURL(test.input); got != test.want { + t.Errorf("DisplayURL(%q) = %q, want %q", test.input, got, test.want) + } + } +} + +func TestGitFailuresRedactCredentialsAndPreserveCause(t *testing.T) { + installFakeGit(t, `case "$*" in + *check-ref-format*) exit 0 ;; +esac +printf '%s\n' "fatal: unable to access 'https://user:secret@example.test/repo': https://token@example.test/path" >&2 +exit 23`) + endpoint := Endpoint{PushURL: "https://user:secret@example.test/repo", ScopedName: "fixture"} + for _, operation := range []struct { + name string + run func() error + }{ + {"query", func() error { _, err := endpoint.Query(context.Background(), t.TempDir(), "v1.2.3"); return err }}, + {"push", func() error { + return endpoint.PushTag(context.Background(), t.TempDir(), "v1.2.3", strings.Repeat("a", 40)) + }}, + } { + t.Run(operation.name, func(t *testing.T) { + err := operation.run() + var exit *exec.ExitError + if !errors.As(err, &exit) || exit.ExitCode() != 23 { + t.Fatalf("error = %v, want Git exit 23", err) + } + if strings.Contains(err.Error(), "secret") || strings.Contains(err.Error(), "token") || strings.Contains(err.Error(), "user") || !strings.Contains(err.Error(), "example.test/repo") { + t.Fatalf("unsafe or unhelpful diagnostic: %v", err) + } + }) + } +} + +func TestDisplayErrorPreservesWrappedCause(t *testing.T) { + cause := errors.New("failed at https://token@example.test/path") + err := displayError{cause: cause} + if !errors.Is(err, cause) || strings.Contains(err.Error(), "token") { + t.Fatalf("sanitized error = %v", err) + } +} + +func TestEndpointChangeRedactsNewDestination(t *testing.T) { + fixture := newRemoteFixture(t) + endpoint, err := Resolve(t.Context(), fixture.work, "origin") + if err != nil { + t.Fatal(err) + } + gitRun(t, fixture.work, "remote", "set-url", "--push", "origin", "https://new-token@example.test/repo") + err = endpoint.Recheck(t.Context(), fixture.work) + if !errors.Is(err, ErrEndpointChanged) || strings.Contains(err.Error(), "new-token") || !strings.Contains(err.Error(), "example.test/repo") { + t.Fatalf("endpoint change error = %v", err) + } +} diff --git a/internal/git/remote.go b/internal/git/remote.go index 631a135..db3b791 100644 --- a/internal/git/remote.go +++ b/internal/git/remote.go @@ -60,14 +60,14 @@ type Snapshot struct { // command-scoped remote whose read and push URLs resolve to that same endpoint. func Resolve(ctx context.Context, repositoryDir, remoteName string) (Endpoint, error) { if strings.TrimSpace(remoteName) == "" || strings.HasPrefix(remoteName, "-") { - return Endpoint{}, fmt.Errorf("%w: %q", ErrInvalidRemote, remoteName) + return Endpoint{}, fmt.Errorf("%w: %q", ErrInvalidRemote, DisplayURL(remoteName)) } urls, err := pushURLs(ctx, repositoryDir, remoteName) if err != nil { return Endpoint{}, err } if len(urls) != 1 { - return Endpoint{}, fmt.Errorf("%w: %s resolves to %d distinct destinations", ErrMultiplePushURLs, remoteName, len(urls)) + return Endpoint{}, fmt.Errorf("%w: %s resolves to %d distinct destinations", ErrMultiplePushURLs, DisplayURL(remoteName), len(urls)) } scopedName, err := newScopedName() if err != nil { @@ -89,10 +89,10 @@ func Resolve(ctx context.Context, repositoryDir, remoteName string) (Endpoint, e func (endpoint Endpoint) Recheck(ctx context.Context, repositoryDir string) error { urls, err := pushURLs(ctx, repositoryDir, endpoint.RemoteName) if err != nil { - return fmt.Errorf("re-resolve remote %q: %w", endpoint.RemoteName, err) + return fmt.Errorf("re-resolve remote %q: %w", DisplayURL(endpoint.RemoteName), err) } if len(urls) != 1 || urls[0] != endpoint.PushURL { - return fmt.Errorf("%w: remote %q now resolves to %q", ErrEndpointChanged, endpoint.RemoteName, strings.Join(urls, ", ")) + return fmt.Errorf("%w: remote %q now resolves to %q", ErrEndpointChanged, DisplayURL(endpoint.RemoteName), displayDiagnostic(strings.Join(urls, ", "))) } return endpoint.verifyScopedURLs(ctx, repositoryDir) } @@ -109,7 +109,7 @@ func (endpoint Endpoint) Query(ctx context.Context, repositoryDir, tag string) ( output, err := run(ctx, repositoryDir, endpoint.scopedConfig(), "ls-remote", endpoint.ScopedName, mainRef, tagRef, peeledRef) if err != nil { snapshot := Snapshot{Main: RemoteRef{State: RefUnreadable}, Tag: RemoteRef{State: RefUnreadable}} - return snapshot, fmt.Errorf("%w: query authoritative refs at %s: %w", ErrRemoteStateUnreadable, endpoint.PushURL, err) + return snapshot, fmt.Errorf("%w: query authoritative refs at %s: %w", ErrRemoteStateUnreadable, DisplayURL(endpoint.PushURL), err) } main := RemoteRef{State: RefAbsent} remoteTag := RemoteRef{State: RefAbsent} @@ -119,7 +119,7 @@ func (endpoint Endpoint) Query(ctx context.Context, repositoryDir, tag string) ( } object, name, ok := strings.Cut(line, "\t") if !ok || !validObjectID(object) { - return Snapshot{Main: RemoteRef{State: RefUnreadable}, Tag: RemoteRef{State: RefUnreadable}}, fmt.Errorf("%w: malformed ls-remote output %q", ErrRemoteStateUnreadable, line) + return Snapshot{Main: RemoteRef{State: RefUnreadable}, Tag: RemoteRef{State: RefUnreadable}}, fmt.Errorf("%w: malformed ls-remote output %q", ErrRemoteStateUnreadable, displayDiagnostic(line)) } switch name { case mainRef: @@ -165,7 +165,7 @@ func (endpoint Endpoint) verifyScopedURLs(ctx context.Context, repositoryDir str } resolved := outputLines(readURL) if len(resolved) != 1 || resolved[0] != endpoint.PushURL { - return fmt.Errorf("%w: scoped URL resolves to %q, expected %q", ErrEndpointChanged, resolved, endpoint.PushURL) + return fmt.Errorf("%w: scoped URL resolves to %q, expected %q", ErrEndpointChanged, displayDiagnostic(strings.Join(resolved, ", ")), DisplayURL(endpoint.PushURL)) } // The scoped remote sets its push URL explicitly to the same captured value. // Git applies the same insteadOf rules to both explicit URLs; pushInsteadOf @@ -197,7 +197,7 @@ func transportConfig(ctx context.Context, repositoryDir, remoteName, scopedName if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { continue } - return nil, fmt.Errorf("read transport setting remote.%s.%s: %w", remoteName, key, err) + return nil, fmt.Errorf("read transport setting remote.%s.%s: %w", DisplayURL(remoteName), key, err) } for _, value := range outputLines(output) { config = append(config, "remote."+scopedName+"."+key+"="+value) @@ -209,7 +209,7 @@ func transportConfig(ctx context.Context, repositoryDir, remoteName, scopedName func pushURLs(ctx context.Context, repositoryDir, remoteName string) ([]string, error) { output, err := run(ctx, repositoryDir, nil, "remote", "get-url", "--push", "--all", remoteName) if err != nil { - return nil, fmt.Errorf("resolve push URL for remote %q: %w", remoteName, err) + return nil, fmt.Errorf("resolve push URL for remote %q: %w", DisplayURL(remoteName), err) } values := outputLines(output) unique := make([]string, 0, len(values)) @@ -239,9 +239,10 @@ func run(ctx context.Context, dir string, config []string, args ...string) (stri Stderr: &stderr, }) if err != nil { + err = displayError{cause: err} message := strings.TrimSpace(stderr.String()) if message != "" { - return stdout.String(), fmt.Errorf("%w: %s", err, message) + return stdout.String(), fmt.Errorf("%w: %s", err, displayDiagnostic(message)) } return stdout.String(), err } diff --git a/internal/git/tag.go b/internal/git/tag.go index b6d74c6..fef24ac 100644 --- a/internal/git/tag.go +++ b/internal/git/tag.go @@ -109,13 +109,13 @@ func (endpoint Endpoint) QueryTag(ctx context.Context, repositoryDir, tag string peeledRef := ref + "^{}" output, err := run(ctx, repositoryDir, endpoint.scopedConfig(), "ls-remote", endpoint.ScopedName, ref, peeledRef) if err != nil { - return RemoteRef{State: RefUnreadable}, fmt.Errorf("%w: query authoritative tag %s at %s: %w", ErrRemoteStateUnreadable, tag, endpoint.PushURL, err) + return RemoteRef{State: RefUnreadable}, fmt.Errorf("%w: query authoritative tag %s at %s: %w", ErrRemoteStateUnreadable, tag, DisplayURL(endpoint.PushURL), err) } remoteTag := RemoteRef{State: RefAbsent} for _, line := range outputLines(output) { object, name, ok := strings.Cut(line, "\t") if !ok || !validObjectID(object) { - return RemoteRef{State: RefUnreadable}, fmt.Errorf("%w: malformed ls-remote output %q", ErrRemoteStateUnreadable, line) + return RemoteRef{State: RefUnreadable}, fmt.Errorf("%w: malformed ls-remote output %q", ErrRemoteStateUnreadable, displayDiagnostic(line)) } switch name { case ref: @@ -137,7 +137,7 @@ func (endpoint Endpoint) PushTag(ctx context.Context, repositoryDir, tag, tagObj } refspec := tagObject + ":refs/tags/" + tag if _, err := run(ctx, repositoryDir, endpoint.scopedConfig(), "push", "--no-follow-tags", "--recurse-submodules=no", "--porcelain", endpoint.ScopedName, refspec); err != nil { - return fmt.Errorf("push annotated tag %s to %s: %w", tag, endpoint.PushURL, err) + return fmt.Errorf("push annotated tag %s to %s: %w", tag, DisplayURL(endpoint.PushURL), err) } return nil } diff --git a/internal/publish/publish.go b/internal/publish/publish.go index 1411263..53d990c 100644 --- a/internal/publish/publish.go +++ b/internal/publish/publish.go @@ -105,7 +105,7 @@ func Publish(ctx context.Context, request Request) (Result, error) { } result.Endpoint = endpoint.PushURL if request.ExpectedPushURL != "" && request.ExpectedPushURL != endpoint.PushURL { - return result, fmt.Errorf("%w: expected %q, resolved %q", ErrUnexpectedPushURL, request.ExpectedPushURL, endpoint.PushURL) + return result, fmt.Errorf("%w: expected %q, resolved %q", ErrUnexpectedPushURL, git.DisplayURL(request.ExpectedPushURL), git.DisplayURL(endpoint.PushURL)) } if err := endpoint.Recheck(ctx, captured.RepositoryDir); err != nil { return result, fmt.Errorf("pre-validation endpoint guard: %w", err) @@ -188,11 +188,11 @@ func reconcilePush(endpoint git.Endpoint, captured candidate.Candidate, tagObjec queryErr = git.ErrRemoteStateUnreadable } } - return result, fmt.Errorf("push annotated tag %s to %s failed; authoritative reconciliation failed (%w): %w", captured.Tag, endpoint.PushURL, queryErr, pushErr) + return result, fmt.Errorf("push annotated tag %s to %s failed; authoritative reconciliation failed (%w): %w", captured.Tag, git.DisplayURL(endpoint.PushURL), queryErr, pushErr) } if remoteTag.State == git.RefAbsent { result.State = NotPublished - return result, fmt.Errorf("push annotated tag %s failed and the authoritative tag is absent at %s: %w", captured.Tag, endpoint.PushURL, pushErr) + return result, fmt.Errorf("push annotated tag %s failed and the authoritative tag is absent at %s: %w", captured.Tag, git.DisplayURL(endpoint.PushURL), pushErr) } if remoteTag.State == git.RefPresent && remoteTag.Object == tagObject && (remoteTag.Peeled == "" || remoteTag.Peeled == captured.Commit) { result.Phase = Complete @@ -200,7 +200,7 @@ func reconcilePush(endpoint git.Endpoint, captured candidate.Candidate, tagObjec return result, nil } result.State = Conflict - return result, fmt.Errorf("push annotated tag %s failed and the authoritative tag identifies object %s, not %s at %s: %w", captured.Tag, remoteTag.Object, tagObject, endpoint.PushURL, pushErr) + return result, fmt.Errorf("push annotated tag %s failed and the authoritative tag identifies object %s, not %s at %s: %w", captured.Tag, remoteTag.Object, tagObject, git.DisplayURL(endpoint.PushURL), pushErr) } func isRemoteConflict(err error) bool { diff --git a/publish.go b/publish.go index b9431f0..7eaf1ee 100644 --- a/publish.go +++ b/publish.go @@ -46,7 +46,7 @@ type PublicationResult struct { Tag string // complete selected Git tag. Version string // selected stable version CandidateCommit string // captured main commit - Endpoint string // captured effective push URL + Endpoint string // captured effective push URL, which may contain credentials Phase PublicationPhase // last operation phase reached TagObject string // local annotated tag object ID, when created State PublicationState // publication outcome for this invocation diff --git a/publish_test.go b/publish_test.go index 9c7862d..897a77b 100644 --- a/publish_test.go +++ b/publish_test.go @@ -332,6 +332,27 @@ func TestPublishRejectsUnexpectedPushURLBeforeValidation(t *testing.T) { } } +func TestPublishRedactsCredentialsInEndpointMismatch(t *testing.T) { + repo := newPublishFixture(t) + resolved := "https://actual-user:actual-secret@example.invalid/project.git" + expected := "https://expected-token@example.invalid/project.git" + gitRun(t, repo.work, "remote", "set-url", "--push", "origin", resolved) + result, err := releasekit.Publish(context.Background(), releasekit.ReleaseRequest{ + RepositoryDir: repo.work, Product: "Sample App", Version: "v1.2.3", ExpectedPushURL: expected, + }) + if !errors.Is(err, releasekit.ErrUnexpectedPushURL) || result.Endpoint != resolved || result.State != releasekit.PublicationNotAttempted { + t.Fatalf("mismatch result = %#v, error = %v", result, err) + } + for _, secret := range []string{"actual-user", "actual-secret", "expected-token"} { + if strings.Contains(err.Error(), secret) { + t.Fatalf("diagnostic exposes %q: %v", secret, err) + } + } + if !strings.Contains(err.Error(), "example.invalid/project.git") { + t.Fatalf("diagnostic lacks endpoint context: %v", err) + } +} + func TestPublishComparesExpectedURLWithResolvedInsteadOfDestination(t *testing.T) { repo := newPublishFixture(t) alias := "releasekit-expected-push:"