Redact quoted credentials in Git diagnostics

docs/roadmap/remediation.md

@@ -4,14 +4,20 @@ Reviewed `2f95753`, including the implementation commits after `a1dc705`,
 against the accepted [audit target](audit.md) and completed
 [implementation plan](implementation.md). A follow-up integration audit reviewed
 `c8fa165`, whose production code is identical, and added R02 and R03. Findings
-below identify remaining work; production code and existing tests were unchanged
-during both reviews.
+and completion records below track the remediation. Production code and existing
+tests were unchanged during both reviews.
 
 ## R01. Git diagnostic redaction misses apostrophes in credentials
 
-**Priority:** P2. **Status:** Pending. This is a remaining gap in A04 and
+**Priority:** P2. **Status:** Complete. This addressed a gap in A04 and
 [Stage 6](implementation.md#stage-6-sanitize-git-endpoint-diagnostics).
 
+**Completion:** Diagnostic matching recognizes complete userinfo before
+surrounding quotation, preserving error context and wrapped causes. Native Git,
+literal/encoded apostrophe, malformed userinfo, and wrapped-error regressions
+pass; the new regressions failed before the fix. The complete `internal/git`
+suite and public/CLI endpoint-mismatch redaction tests passed offline.
+
 **Relevant code:** `httpURLSpan` and `displayDiagnostic` in
 [internal/git/display.go](../../internal/git/display.go), lines 9 and 32–39;
 `pushURLs` and `run` in
@@ -143,10 +149,10 @@ sequences; build settings have one internal representation; artifact declaration
 matching is shared; neutral delivery policy remains separate from Gitea protocol
 operations; and private CLI handlers own presentation.
 
-R01–R03 are the remaining findings from the initial review and follow-up
-integration audit. Package ownership and public facade shape match the target;
-the findings concern diagnostic and subprocess-environment behavior across
-existing boundaries. The review identified no inadvertently strengthened public
+R01–R03 record findings and remediation status from the initial review and
+follow-up integration audit. Package ownership and public facade shape match
+the target; the findings concern diagnostic and subprocess-environment behavior
+across existing boundaries. The review identified no inadvertently strengthened public
 contract. The selected redirect and diagnostic limits match the accepted scope.
 JSON response caps and production exclusion indexing remain explicitly deferred
 with assessment evidence, as planned.
@@ -187,7 +193,8 @@ inherited-pipe cancellation, forwarding errors, joined cleanup outcomes,
 bounded asset reads, upload reconciliation, and tree-reader cleanup precedence.
 
 R02 and R03 were reproduced independently with temporary Go test overlays.
-Their assertions failed against the current implementation; R02's comparison
-using the earlier Git environment behavior passed. The checkout contains no
-probe code or production changes. The previously passing full-suite, static,
-and compiled API/dependency checks apply to the unchanged production sources.
+Their assertions failed against the reviewed implementation; R02's comparison
+using the earlier Git environment behavior passed. Both audits left production
+code and existing tests unchanged. The full-suite, static, and compiled
+API/dependency results above record the audit baseline; completion notes record
+verification of the subsequent repairs.

internal/git/display.go

@@ -6,7 +6,8 @@ import (
 	"strings"
 )
 
-var httpURLSpan = regexp.MustCompile(`(?i)https?://[^\s"'<>]+`)
+// Match userinfo through @ before treating quotes as diagnostic delimiters.
+var httpURLSpan = regexp.MustCompile(`(?i)https?://(?:[^\s/?#<>]*@)?[^\s"'<>]*`)
 
 // DisplayURL hides HTTP userinfo without changing the endpoint used by Git.
 func DisplayURL(raw string) string {

internal/git/display_test.go

@@ -12,6 +12,7 @@ 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:secret'part@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"},
@@ -23,6 +24,32 @@ func TestDisplayURL(t *testing.T) {
 	}
 }
 
+func TestDisplayDiagnosticRedactsUserinfoBeforeQuotedContext(t *testing.T) {
+	for _, test := range []struct{ input, want string }{
+		{"fatal: 'https://user:secret'part@example.test/repo': denied", "fatal: 'https://redacted@example.test/repo': denied"},
+		{"failed at https://token'part@example.test", "failed at https://redacted@example.test"},
+		{"fatal: 'https://user:secret%27part@example.test/repo': denied", "fatal: 'https://redacted@example.test/repo': denied"},
+		{"fatal: 'https://broken%zz:secret'part@example.test/repo': denied", "fatal: '[redacted URL]': denied"},
+		{"fatal: 'https://token'part@': denied", "fatal: '[redacted URL]': denied"},
+		{"see 'https://example.test/path' and git@example.test:repo", "see 'https://example.test/path' and git@example.test:repo"},
+	} {
+		if got := displayDiagnostic(test.input); got != test.want {
+			t.Errorf("displayDiagnostic(%q) = %q, want %q", test.input, got, test.want)
+		}
+	}
+}
+
+func TestResolveRedactsCredentialsInNativeGitErrors(t *testing.T) {
+	fixture := newRemoteFixture(t)
+	for _, remote := range []string{"https://user:secret'part@example.test/repo", "https://user:secret%27part@example.test/repo"} {
+		_, err := Resolve(t.Context(), fixture.work, remote)
+		var exit *exec.ExitError
+		if !errors.As(err, &exit) || strings.Contains(err.Error(), "secret") || strings.Contains(err.Error(), "user:") || !strings.Contains(err.Error(), "example.test/repo") {
+			t.Fatalf("unsafe or incomplete native Git diagnostic: %v", err)
+		}
+	}
+}
+
 func TestGitFailuresRedactCredentialsAndPreserveCause(t *testing.T) {
 	installFakeGit(t, `case "$*" in
   *check-ref-format*) exit 0 ;;
@@ -53,10 +80,12 @@ exit 23`)
 }
 
 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)
+	for _, userinfo := range []string{"token", "token'part", "token%27part"} {
+		cause := errors.New("failed at https://" + userinfo + "@example.test/path")
+		err := displayError{cause: cause}
+		if !errors.Is(err, cause) || strings.Contains(err.Error(), "token") {
+			t.Fatalf("sanitized error = %v", err)
+		}
 	}
 }