diff --git a/cmd/releasekit/main.go b/cmd/releasekit/main.go index fb3c0b2..a656611 100644 --- a/cmd/releasekit/main.go +++ b/cmd/releasekit/main.go @@ -108,7 +108,7 @@ func reportPublicationRecord(result releasekit.PublicationResult, publicationErr if result.RecordPath == "" { return } - fmt.Fprintf(stderr, "publication record saved: %s\n", result.RecordPath) + fmt.Fprintf(stderr, "publication record saved: %s\n", git.DisplayDiagnostic(result.RecordPath)) if publicationErr == nil || !publicationOffersRecovery(result.Phase) { return } @@ -118,7 +118,7 @@ func reportPublicationRecord(result releasekit.PublicationResult, publicationErr fmt.Fprintln(stderr, "Use your selected supported ReleaseKit CLI with this record if the original executable is unavailable.") return } - fmt.Fprintf(stderr, "recovery command: %s\n", command) + fmt.Fprintf(stderr, "recovery command: %s\n", git.DisplayDiagnostic(command)) fmt.Fprintln(stderr, "If this executable is unavailable, use your selected supported ReleaseKit CLI with the same record.") } @@ -140,7 +140,8 @@ func publicationRecoveryCommand(recordPath string) (string, error) { if err != nil { return "", fmt.Errorf("resolve current executable path: %w", err) } - recordPath, err = filepath.Abs(recordPath) + // Redact before path cleaning can change a pasted URL's recognizable syntax. + recordPath, err = filepath.Abs(git.DisplayDiagnostic(recordPath)) if err != nil { return "", fmt.Errorf("resolve publication record path: %w", err) } diff --git a/cmd/releasekit/recovery.go b/cmd/releasekit/recovery.go index 967b8ec..ebba622 100644 --- a/cmd/releasekit/recovery.go +++ b/cmd/releasekit/recovery.go @@ -60,16 +60,16 @@ type attemptsDocument struct { func safe(e releasekit.PublicationEvidence) safeEvidence { return safeEvidence{ - RepositoryDir: e.RepositoryDir, - ModuleDir: e.ModuleDir, - ModulePath: e.ModulePath, - Version: e.Version, - Tag: e.Tag, - CandidateCommit: e.CandidateCommit, - TagObject: e.TagObject, - Endpoint: git.DisplayURL(e.Endpoint), - Remote: e.Remote, - RecordedAt: e.RecordedAt, + RepositoryDir: git.DisplayDiagnostic(e.RepositoryDir), + ModuleDir: git.DisplayDiagnostic(e.ModuleDir), + ModulePath: git.DisplayDiagnostic(e.ModulePath), + Version: git.DisplayDiagnostic(e.Version), + Tag: git.DisplayDiagnostic(e.Tag), + CandidateCommit: git.DisplayDiagnostic(e.CandidateCommit), + TagObject: git.DisplayDiagnostic(e.TagObject), + Endpoint: git.DisplayDiagnostic(e.Endpoint), + Remote: git.DisplayDiagnostic(e.Remote), + RecordedAt: git.DisplayDiagnostic(e.RecordedAt), } } @@ -79,9 +79,13 @@ func diagnostic(err error) *recoveryDiagnostic { } var typed *releasekit.PublicationRecoveryError if errors.As(err, &typed) { - return &recoveryDiagnostic{Kind: typed.Kind, Message: err.Error(), RecordPath: typed.RecordPath, MissingFields: typed.MissingFields} + missing := make([]string, len(typed.MissingFields)) + for i, field := range typed.MissingFields { + missing[i] = git.DisplayDiagnostic(field) + } + return &recoveryDiagnostic{Kind: git.DisplayDiagnostic(typed.Kind), Message: git.DisplayError(err).Error(), RecordPath: git.DisplayDiagnostic(typed.RecordPath), MissingFields: missing} } - return &recoveryDiagnostic{Kind: "invocation", Message: err.Error()} + return &recoveryDiagnostic{Kind: "invocation", Message: git.DisplayError(err).Error()} } func runRecovery(ctx context.Context, invocation cli.Invocation, stdout, stderr io.Writer) int { @@ -92,7 +96,7 @@ func runRecovery(ctx context.Context, invocation cli.Invocation, stdout, stderr if invocation.JSON { attempts := make([]attemptDocument, 0, len(listed.Attempts)) for _, item := range listed.Attempts { - attempts = append(attempts, attemptDocument{RecordPath: item.RecordPath, Evidence: safe(item.Evidence)}) + attempts = append(attempts, attemptDocument{RecordPath: git.DisplayDiagnostic(item.RecordPath), Evidence: safe(item.Evidence)}) } diagnostics := make([]*recoveryDiagnostic, 0, len(listed.Diagnostics)) for _, item := range listed.Diagnostics { @@ -101,24 +105,24 @@ func runRecovery(ctx context.Context, invocation cli.Invocation, stdout, stderr writeJSON(stdout, attemptsDocument{ SchemaVersion: 1, Command: "publication-attempts", - RecordDir: listed.RecordDir, + RecordDir: git.DisplayDiagnostic(listed.RecordDir), Attempts: attempts, Diagnostics: diagnostics, Diagnostic: diagnostic(err), }) } else { - fmt.Fprintf(stdout, "publication records in %s\n", listed.RecordDir) + fmt.Fprintf(stdout, "publication records in %s\n", git.DisplayDiagnostic(listed.RecordDir)) for _, item := range listed.Attempts { - fmt.Fprintf(stdout, "%s module=%s tag=%s commit=%s recorded-at=%s\n", item.RecordPath, item.Evidence.ModulePath, item.Evidence.Tag, item.Evidence.CandidateCommit, item.Evidence.RecordedAt) + fmt.Fprintf(stdout, "%s module=%s tag=%s commit=%s recorded-at=%s\n", git.DisplayDiagnostic(item.RecordPath), git.DisplayDiagnostic(item.Evidence.ModulePath), git.DisplayDiagnostic(item.Evidence.Tag), git.DisplayDiagnostic(item.Evidence.CandidateCommit), git.DisplayDiagnostic(item.Evidence.RecordedAt)) } if err != nil || len(listed.Diagnostics) > 0 { fmt.Fprintln(stderr, "discovery is incomplete") } for _, item := range listed.Diagnostics { - fmt.Fprintf(stderr, "record %s: %s\n", item.RecordPath, humanDiagnostic(item)) + fmt.Fprintf(stderr, "record %s: %s\n", git.DisplayDiagnostic(item.RecordPath), humanDiagnostic(item)) } if err != nil { - fmt.Fprintf(stderr, "discovery failed: %v\n", err) + fmt.Fprintf(stderr, "discovery failed: %v\n", git.DisplayError(err)) } } if err != nil || len(listed.Diagnostics) > 0 { @@ -136,7 +140,7 @@ func runReconcile(ctx context.Context, invocation cli.Invocation, stdout, stderr } status := reconciliationStatus(result.State, err) if invocation.JSON { - dto := reconciliationDocument{SchemaVersion: 1, Command: "reconcile", RecordPath: invocation.RecordPath} + dto := reconciliationDocument{SchemaVersion: 1, Command: "reconcile", RecordPath: git.DisplayDiagnostic(invocation.RecordPath)} if loadErr == nil { safeEvidence := safe(evidence) dto.Evidence = &safeEvidence @@ -147,9 +151,9 @@ func runReconcile(ctx context.Context, invocation cli.Invocation, stdout, stderr dto.Diagnostic = diagnostic(err) writeJSON(stdout, dto) } else if err == nil { - fmt.Fprintf(stdout, "reconciled %s module=%s tag=%s state=%s\n", invocation.RecordPath, evidence.ModulePath, evidence.Tag, result.State) + fmt.Fprintf(stdout, "reconciled %s module=%s tag=%s state=%s\n", git.DisplayDiagnostic(invocation.RecordPath), git.DisplayDiagnostic(evidence.ModulePath), git.DisplayDiagnostic(evidence.Tag), result.State) if result.State == releasekit.PublicationConflict { - fmt.Fprintf(stdout, "expected tag-object=%s commit=%s; observed tag-object=%s commit=%s\n", evidence.TagObject, evidence.CandidateCommit, result.ObservedTagObject, result.ObservedCommit) + fmt.Fprintf(stdout, "expected tag-object=%s commit=%s; observed tag-object=%s commit=%s\n", git.DisplayDiagnostic(evidence.TagObject), git.DisplayDiagnostic(evidence.CandidateCommit), result.ObservedTagObject, result.ObservedCommit) } else if result.ObservedTagObject != "" || result.ObservedCommit != "" { fmt.Fprintf(stdout, "observed tag-object=%s commit=%s\n", result.ObservedTagObject, result.ObservedCommit) } @@ -159,12 +163,12 @@ func runReconcile(ctx context.Context, invocation cli.Invocation, stdout, stderr } if err != nil && !invocation.JSON { if loadErr == nil { - fmt.Fprintf(stdout, "reconciled %s module=%s tag=%s state=%s\n", invocation.RecordPath, evidence.ModulePath, evidence.Tag, result.State) + fmt.Fprintf(stdout, "reconciled %s module=%s tag=%s state=%s\n", git.DisplayDiagnostic(invocation.RecordPath), git.DisplayDiagnostic(evidence.ModulePath), git.DisplayDiagnostic(evidence.Tag), result.State) if result.ObservedTagObject != "" || result.ObservedCommit != "" { fmt.Fprintf(stdout, "observed tag-object=%s commit=%s\n", result.ObservedTagObject, result.ObservedCommit) } } - fmt.Fprintf(stderr, "reconcile %s: %s\n", invocation.RecordPath, humanDiagnostic(err)) + fmt.Fprintf(stderr, "reconcile %s: %s\n", git.DisplayDiagnostic(invocation.RecordPath), humanDiagnostic(err)) if guidance := recoveryGuidance(err); guidance != "" { fmt.Fprintf(stderr, "%s\n", guidance) } @@ -234,9 +238,9 @@ func reconciliationGuidance(state releasekit.PublicationState) string { func humanDiagnostic(err error) string { var typed *releasekit.PublicationRecoveryError if errors.As(err, &typed) && len(typed.MissingFields) > 0 { - return fmt.Sprintf("%s (missing fields: %s): %s", typed.Kind, strings.Join(typed.MissingFields, ", "), err.Error()) + return git.DisplayDiagnostic(fmt.Sprintf("%s (missing fields: %s): %s", typed.Kind, strings.Join(typed.MissingFields, ", "), err.Error())) } - return err.Error() + return git.DisplayError(err).Error() } func writeJSON(writer io.Writer, value any) { diff --git a/cmd/releasekit/recovery_test.go b/cmd/releasekit/recovery_test.go index c8ea0c4..44b0757 100644 --- a/cmd/releasekit/recovery_test.go +++ b/cmd/releasekit/recovery_test.go @@ -10,9 +10,57 @@ import ( "strings" "testing" + releasekit "go.maximumdirect.net/releasekit" "go.maximumdirect.net/releasekit/internal/publish" ) +func TestRecoveryDisplaysRedactCredentialPathsAndMetadata(t *testing.T) { + secret := "audit-secret" + raw := "https://audit-user:" + secret + "@example.invalid/attempt.json" + for _, jsonOutput := range []bool{false, true} { + args := []string{"reconcile", "--record", raw} + if jsonOutput { + args = append(args, "--json") + } + var out, diagnostics bytes.Buffer + if status := run(t.Context(), args, &out, &diagnostics); status != 2 { + t.Fatalf("load status=%d", status) + } + if strings.Contains(out.String()+diagnostics.String(), secret) { + t.Fatalf("credential leaked: %s%s", &out, &diagnostics) + } + } + repo := newCLIRepository(t) + store := t.TempDir() + evidence := publish.Evidence{RepositoryDir: raw, ModuleDir: raw, ModulePath: raw, Version: raw, Tag: raw, CandidateCommit: raw, TagObject: raw, Remote: raw, RecordedAt: raw} + path, err := publish.SaveEvidence(t.Context(), repo, store, evidence) + if err != nil { + t.Fatal(err) + } + for _, command := range [][]string{{"reconcile", "--record", path}, {"publication-attempts", "--record-dir", store}} { + for _, jsonOutput := range []bool{false, true} { + args := append([]string{}, command...) + if jsonOutput { + args = append(args, "--json") + } + var out, diagnostics bytes.Buffer + run(t.Context(), args, &out, &diagnostics) + if strings.Contains(out.String()+diagnostics.String(), secret) { + t.Fatalf("metadata leaked: %s%s", &out, &diagnostics) + } + } + } + loaded, err := publish.LoadEvidence(path) + if err != nil || loaded != evidence { + t.Fatalf("private metadata changed: %#v, %v", loaded, err) + } + var out bytes.Buffer + reportPublicationRecord(releasekit.PublicationResult{RecordPath: raw, Phase: releasekit.PublicationPush}, fmt.Errorf("push failed"), &out) + if strings.Contains(out.String(), secret) { + t.Fatalf("handoff leaked credentials: %s", &out) + } +} + func TestReconcileCommandClassifiesRecordsFromAnotherWorkingDirectory(t *testing.T) { repository := newCLIRepository(t) remote := filepath.Join(t.TempDir(), "bare remote.git") diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 353fbd5..d1c4209 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -174,6 +174,11 @@ the existing immediate-recovery timeout semantics. ## R05: Redact recovery paths and diagnostic fields at display boundaries — P2 +**Completed:** Facade errors and CLI recovery displays now redact recognized +HTTP credentials in paths, metadata, diagnostics, and handoff commands. Public +error and CLI human/JSON regressions passed; raw typed causes and private record +contents remain intact. + **Relevant code:** `recovery.go:53`; `cmd/releasekit/recovery.go:76`, its JSON DTO construction, and human path displays; `cmd/releasekit/main.go`'s record displays. diff --git a/internal/git/display.go b/internal/git/display.go index e80ad66..bcc5bbf 100644 --- a/internal/git/display.go +++ b/internal/git/display.go @@ -34,6 +34,9 @@ func displayDiagnostic(message string) string { return httpURLSpan.ReplaceAllStringFunc(message, DisplayURL) } +// DisplayDiagnostic redacts recognized HTTP credentials in a display string. +func DisplayDiagnostic(message string) string { return displayDiagnostic(message) } + type displayError struct{ cause error } func (err displayError) Error() string { return displayDiagnostic(err.cause.Error()) } diff --git a/recovery.go b/recovery.go index 395563c..b77d55b 100644 --- a/recovery.go +++ b/recovery.go @@ -56,12 +56,12 @@ func (e *PublicationRecoveryError) Error() string { } message := fmt.Sprintf("publication recovery %s", e.Kind) if e.RecordPath != "" { - message += fmt.Sprintf(" for %q", e.RecordPath) + message += fmt.Sprintf(" for %q", git.DisplayDiagnostic(e.RecordPath)) } if e.cause != nil { message += ": " + git.DisplayError(e.cause).Error() } - return message + return git.DisplayDiagnostic(message) } // Unwrap returns the underlying recovery or storage cause. diff --git a/recovery_test.go b/recovery_test.go index 232e4ec..5917927 100644 --- a/recovery_test.go +++ b/recovery_test.go @@ -5,11 +5,25 @@ import ( "errors" "os" "path/filepath" + "strings" "testing" releasekit "go.maximumdirect.net/releasekit" ) +func TestRecoveryFacadeRedactsSelectedPathAndRetainsRawCause(t *testing.T) { + path := "https://audit-user:audit-secret@example.invalid/attempt.json" + _, err := releasekit.LoadPublicationEvidence(path) + var recovery *releasekit.PublicationRecoveryError + var pathErr *os.PathError + if !errors.As(err, &recovery) || recovery.RecordPath != path || !errors.As(err, &pathErr) || pathErr.Path != path { + t.Fatalf("raw diagnostic context lost: %v", err) + } + if strings.Contains(err.Error(), "audit-secret") || strings.Contains(err.Error(), "audit-user") { + t.Fatalf("credentials in displayed error: %v", err) + } +} + func TestRecoveryFacadeLoadsPartialEvidenceAndPreservesCause(t *testing.T) { path := filepath.Join(t.TempDir(), "attempt.json") if err := os.WriteFile(path, []byte(`{"format_version":1,"evidence":{}}`), 0600); err != nil {