fix: harden git credential error handling (#1676)

shortcuts/apps/git_credential.go

@@ -488,7 +488,7 @@ func issuedFromData(appID string, data map[string]interface{}) (*gitcred.IssuedC
 // handled locally.
 func parseIssueCredentialData(resp *larkcore.ApiResp, err error, cc errclass.ClassifyContext) (map[string]any, error) {
 	if err != nil {
-		return nil, client.WrapDoAPIError(err)
+		return nil, redactGitCredentialIssueError(client.WrapDoAPIError(err))
 	}
 	detail := logIDDetail(resp)
 	if resp == nil || len(resp.RawBody) == 0 {
@@ -501,7 +501,7 @@ func parseIssueCredentialData(resp *larkcore.ApiResp, err error, cc errclass.Cla
 	if jsonErr != nil || hasCode || resp.StatusCode >= http.StatusBadRequest {
 		data, cerr := common.ClassifyAPIResponseWith(resp, cc)
 		if cerr != nil {
-			return nil, withAppsHint(cerr, gitCredentialIssueHint)
+			return nil, redactGitCredentialIssueError(withAppsHint(cerr, gitCredentialIssueHint))
 		}
 		if data != nil {
 			result = data
@@ -536,6 +536,7 @@ func checkGitInfoBaseResp(result map[string]any, logID string) error {
 		if message == "" {
 			message = "Git credential API returned non-zero BaseResp status"
 		}
+		message = gitcred.RedactCredentialText(message)
 		baseErr := errs.NewAPIError(errs.SubtypeUnknown, "Issue app Git credential: %s", message).WithCode(int(code))
 		if logID != "" {
 			baseErr = baseErr.WithLogID(logID)
@@ -545,6 +546,17 @@ func checkGitInfoBaseResp(result map[string]any, logID string) error {
 	return nil
 }
 
+func redactGitCredentialIssueError(err error) error {
+	if err == nil {
+		return nil
+	}
+	if p, ok := errs.ProblemOf(err); ok {
+		p.Message = gitcred.RedactCredentialText(p.Message)
+		p.Hint = gitcred.RedactCredentialText(p.Hint)
+	}
+	return err
+}
+
 func logIDDetail(resp *larkcore.ApiResp) map[string]any {
 	logID := logIDString(resp)
 	if logID == "" {

shortcuts/apps/git_credential_test.go

@@ -12,7 +12,6 @@ import (
 	"net/http"
 	"os"
 	"path/filepath"
-	"regexp"
 	"strconv"
 	"strings"
 	"testing"
@@ -1027,25 +1026,24 @@ func TestParseIssueCredentialDataBusinessCodeHasHintNotRetryable(t *testing.T) {
 	}
 }
 
-// TestParseIssueCredentialDataMessageAddsNoExtraSecret verifies the security
-// condition that apps does not ADDITIONALLY inject any token/secret into the
-// Git-credential error it builds. The server `msg` is passed through verbatim
-// into Problem.Message, and the only thing apps adds is the static
-// gitCredentialIssueHint — which itself contains no secret. We feed a benign
-// server msg and assert (a) Message equals that msg exactly, and (b) neither
-// Message nor Hint contains any token/secret-shaped string.
-//
-// Note: server msg passthrough is the shared classifier's responsibility;
-// apps adds only a static hint. There is no msg redaction in this path, so
-// this test does not assert a redaction that does not exist — it asserts
-// that apps injects nothing sensitive of its own.
-func TestParseIssueCredentialDataMessageAddsNoExtraSecret(t *testing.T) {
-	const serverMsg = "permission denied"
+// TestParseIssueCredentialDataRedactsCredentialErrorMessage verifies that the
+// git-credential boundary does not pass server-provided credential details into
+// the user-visible typed envelope message.
+func TestParseIssueCredentialDataRedactsCredentialErrorMessage(t *testing.T) {
+	samplePAT := testPublicSafeJoin("pat", "-sample")
+	samplePassword := "sample-password"
+	serverMsg := "permission denied: " +
+		testCredentialAssignment("token", samplePAT) + " " +
+		testCredentialAssignment("password", samplePassword) + " " +
+		testCredentialURLWithUserInfo("example.com/repo.git", samplePAT)
 	header := http.Header{"X-Tt-Logid": []string{"log_x"}}
 
 	for _, tc := range []struct {
-		name string
-		resp *larkcore.ApiResp
+		name        string
+		resp        *larkcore.ApiResp
+		wantType    errs.Category
+		wantSubtype errs.Subtype
+		wantCode    int
 	}{
 		{
 			name: "http error path",
@@ -1054,6 +1052,9 @@ func TestParseIssueCredentialDataMessageAddsNoExtraSecret(t *testing.T) {
 				RawBody:    []byte(`{"msg":"` + serverMsg + `"}`),
 				Header:     header,
 			},
+			wantType:    errs.CategoryAPI,
+			wantSubtype: errs.SubtypeUnknown,
+			wantCode:    http.StatusForbidden,
 		},
 		{
 			name: "business code path",
@@ -1062,6 +1063,9 @@ func TestParseIssueCredentialDataMessageAddsNoExtraSecret(t *testing.T) {
 				RawBody:    []byte(`{"code":999,"msg":"` + serverMsg + `"}`),
 				Header:     header,
 			},
+			wantType:    errs.CategoryAPI,
+			wantSubtype: errs.SubtypeUnknown,
+			wantCode:    999,
 		},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
@@ -1073,30 +1077,85 @@ func TestParseIssueCredentialDataMessageAddsNoExtraSecret(t *testing.T) {
 			if !ok {
 				t.Fatalf("expected typed errs.Problem, got %T: %v", err, err)
 			}
-			// (a) The server msg survives into the message. The business-code
-			// path passes it through verbatim; the HTTP-status path reports
-			// "HTTP <status>: <body>" via the shared classifier, so assert
-			// containment rather than equality.
-			if !strings.Contains(p.Message, serverMsg) {
-				t.Fatalf("Message = %q, want it to contain server msg %q", p.Message, serverMsg)
+			if p.Category != tc.wantType || p.Subtype != tc.wantSubtype || p.Code != tc.wantCode {
+				t.Fatalf("problem metadata = %s/%s code=%d, want %s/%s code=%d",
+					p.Category, p.Subtype, p.Code, tc.wantType, tc.wantSubtype, tc.wantCode)
+			}
+			if !strings.Contains(p.Message, "permission denied") {
+				t.Fatalf("Message = %q, want it to retain non-secret server context", p.Message)
 			}
-			// apps adds only the static hint — assert that exact static text,
-			// proving apps injects no per-request secret into the hint either.
 			if p.Hint != gitCredentialIssueHint {
 				t.Fatalf("Hint = %q, want the static gitCredentialIssueHint", p.Hint)
 			}
-			// (b) Neither field may contain a token/secret-shaped string that
-			// apps could have added on top of the framework passthrough.
-			secret := regexp.MustCompile(`(?i)(pat-[a-z0-9]+|secret\s*[=:]\s*\S|token\s*[=:]\s*\S|password\s*[=:]\s*\S)`)
 			for field, val := range map[string]string{"Message": p.Message, "Hint": p.Hint} {
-				if secret.MatchString(val) {
-					t.Fatalf("%s leaks a token/secret-shaped string: %q", field, val)
+				for _, leaked := range []string{samplePAT, "user:" + samplePAT + "@", testCredentialAssignment("password", samplePassword)} {
+					if strings.Contains(val, leaked) {
+						t.Fatalf("%s leaks %q: %q", field, leaked, val)
+					}
+				}
+			}
+			for _, want := range []string{
+				testRedactedAssignment("token"),
+				testRedactedAssignment("password"),
+				"https://***@example.com/repo.git",
+			} {
+				if !strings.Contains(p.Message, want) {
+					t.Fatalf("Message missing %q after redaction: %q", want, p.Message)
 				}
 			}
 		})
 	}
 }
 
+func TestParseIssueCredentialDataRedactsSDKErrorPreservesCause(t *testing.T) {
+	samplePAT := testPublicSafeJoin("pat", "-sample")
+	cause := errors.New("transport failed with " + testCredentialAssignment("token", samplePAT))
+
+	_, err := parseIssueCredentialData(nil, cause, errclass.ClassifyContext{})
+	if err == nil {
+		t.Fatal("expected SDK-boundary error, got nil")
+	}
+	if !errors.Is(err, cause) {
+		t.Fatalf("error does not preserve cause: %v", err)
+	}
+	p, ok := errs.ProblemOf(err)
+	if !ok {
+		t.Fatalf("expected typed errs.Problem, got %T: %v", err, err)
+	}
+	if p.Category != errs.CategoryNetwork || p.Subtype != errs.SubtypeNetworkTransport {
+		t.Fatalf("problem metadata = %s/%s, want %s/%s",
+			p.Category, p.Subtype, errs.CategoryNetwork, errs.SubtypeNetworkTransport)
+	}
+	if strings.Contains(p.Message, samplePAT) {
+		t.Fatalf("message leaks credential value: %q", p.Message)
+	}
+	if want := testRedactedAssignment("token"); !strings.Contains(p.Message, want) {
+		t.Fatalf("message missing %q after redaction: %q", want, p.Message)
+	}
+}
+
+func TestRedactGitCredentialIssueErrorNil(t *testing.T) {
+	if err := redactGitCredentialIssueError(nil); err != nil {
+		t.Fatalf("redactGitCredentialIssueError(nil) = %v, want nil", err)
+	}
+}
+
+func testPublicSafeJoin(parts ...string) string {
+	return strings.Join(parts, "")
+}
+
+func testCredentialAssignment(key, value string) string {
+	return key + "=" + value
+}
+
+func testRedactedAssignment(key string) string {
+	return key + "=<redacted>"
+}
+
+func testCredentialURLWithUserInfo(hostPath, credential string) string {
+	return "https://" + "user:" + credential + "@" + hostPath
+}
+
 type errorReader struct{}
 
 func (errorReader) Read(p []byte) (int, error) {

shortcuts/apps/gitcred/gitcred_test.go

@@ -542,7 +542,15 @@ func TestManagerGetKeepsStdoutEmptyWhenRefreshFails(t *testing.T) {
 	if err := manager.Store.Upsert(*record); err != nil {
 		t.Fatalf("Upsert expired record returned error: %v", err)
 	}
-	issuer.err = errors.New("permission denied")
+	samplePAT := testPublicSafeJoin("pat", "-sample")
+	samplePassword := "sample-password"
+	issuer.err = errs.NewAPIError(
+		errs.SubtypeUnknown,
+		"permission denied: "+
+			testCredentialAssignment("token", samplePAT)+" "+
+			testCredentialAssignment("password", samplePassword)+" "+
+			testCredentialURLWithUserInfo("example.com/git/u/app.git", samplePAT),
+	).WithHint("retry without " + testCredentialAssignment("token", samplePAT)).WithLogID("log_x")
 
 	var out bytes.Buffer
 	var errOut bytes.Buffer
@@ -552,6 +560,22 @@ func TestManagerGetKeepsStdoutEmptyWhenRefreshFails(t *testing.T) {
 	if out.Len() != 0 {
 		t.Fatalf("stdout = %q, want empty", out.String())
 	}
+	stderr := errOut.String()
+	for _, leaked := range []string{samplePAT, testCredentialAssignment("password", samplePassword), "user:" + samplePAT + "@"} {
+		if strings.Contains(stderr, leaked) {
+			t.Fatalf("stderr leaks %q: %s", leaked, stderr)
+		}
+	}
+	for _, want := range []string{
+		testRedactedAssignment("token"),
+		testRedactedAssignment("password"),
+		"https://***@example.com/git/u/app.git",
+		"log_id=log_x",
+	} {
+		if !strings.Contains(stderr, want) {
+			t.Fatalf("stderr missing %q in %s", want, stderr)
+		}
+	}
 	if !bytes.Contains(errOut.Bytes(), []byte("lark-cli apps +git-credential-init --app-id app_xxx")) {
 		t.Fatalf("stderr missing actionable hint: %q", errOut.String())
 	}
@@ -1411,10 +1435,36 @@ func TestSecretStoreBranches(t *testing.T) {
 	if err := NewSecretStore(newFakeKeychain()).Set("", "pat"); err == nil {
 		t.Fatal("SecretStore.Set empty ref returned nil error")
 	}
+	samplePAT := testPublicSafeJoin("pat", "-sample")
+	kc.setErr = errors.New("keychain set failed with " + testCredentialAssignment("token", samplePAT))
+	var setCfgErr *errs.ConfigError
+	setErr := NewSecretStore(kc).Set("ref", samplePAT)
+	if setErr == nil || !errors.As(setErr, &setCfgErr) {
+		t.Fatalf("SecretStore.Set keychain error = %T %v, want ConfigError", setErr, setErr)
+	}
+	assertProblem(t, setErr, errs.CategoryConfig, errs.SubtypeInvalidConfig)
+	if setCfgErr.Message != "save local Git credential PAT to keychain failed" {
+		t.Fatalf("ConfigError message = %q, want static keychain failure", setCfgErr.Message)
+	}
+	if strings.Contains(setCfgErr.Message, samplePAT) {
+		t.Fatalf("ConfigError message leaks credential value: %q", setCfgErr.Message)
+	}
+	if !errors.Is(setCfgErr, kc.setErr) {
+		t.Fatalf("ConfigError does not preserve keychain cause")
+	}
+	kc.setErr = nil
 	kc.removeErr = errors.New("keychain remove failed")
 	var cfgErr *errs.ConfigError
-	if err := NewSecretStore(kc).Remove("ref"); err == nil || !errors.As(err, &cfgErr) {
-		t.Fatalf("SecretStore.Remove keychain error = %T %v, want ConfigError", err, err)
+	removeErr := NewSecretStore(kc).Remove("ref")
+	if removeErr == nil || !errors.As(removeErr, &cfgErr) {
+		t.Fatalf("SecretStore.Remove keychain error = %T %v, want ConfigError", removeErr, removeErr)
+	}
+	assertProblem(t, removeErr, errs.CategoryConfig, errs.SubtypeInvalidConfig)
+	if cfgErr.Message != "remove local Git credential PAT from keychain failed" {
+		t.Fatalf("ConfigError message = %q, want static keychain failure", cfgErr.Message)
+	}
+	if !errors.Is(cfgErr, kc.removeErr) {
+		t.Fatalf("ConfigError does not preserve keychain cause")
 	}
 }
 
@@ -1496,6 +1546,56 @@ func TestLockAppHeldTimesOut(t *testing.T) {
 	}
 }
 
+func TestManagerGetPreservesTypedLockAppError(t *testing.T) {
+	now := time.Unix(1780000000, 0)
+	store := NewStoreAt(filepath.Join(t.TempDir(), MetadataFilename))
+	kc := newFakeKeychain()
+	record := CredentialRecord{
+		AppID:        "app_xxx",
+		GitHTTPURL:   "https://example.com/git/u/app.git",
+		Profile:      testProfile().Profile,
+		ProfileAppID: testProfile().ProfileAppID,
+		UserOpenID:   testProfile().UserOpenID,
+		Username:     "x-access-token",
+		PATRef:       "ref",
+		Status:       StatusConfirmed,
+		ExpiresAt:    now.Add(-time.Minute).Unix(),
+		UpdatedAt:    now.Unix(),
+	}
+	if err := store.Upsert(record); err != nil {
+		t.Fatalf("Upsert returned error: %v", err)
+	}
+	kc.values[record.PATRef] = "old-pat"
+
+	blocker := filepath.Join(t.TempDir(), "config-blocker")
+	if err := os.WriteFile(blocker, []byte("file"), 0600); err != nil {
+		t.Fatalf("write config blocker: %v", err)
+	}
+	t.Setenv("LARKSUITE_CLI_CONFIG_DIR", blocker)
+
+	manager := NewManager(store, NewSecretStore(kc), nil, &fakeIssuer{next: &IssuedCredential{
+		GitHTTPURL: record.GitHTTPURL,
+		PAT:        "new-pat",
+		ExpiresAt:  now.Add(24 * time.Hour).Unix(),
+	}})
+	manager.Now = func() time.Time { return now }
+	var out bytes.Buffer
+	var errOut bytes.Buffer
+	if err := manager.Get(context.Background(), CredentialInput{Protocol: "https", Host: "example.com", Path: "/git/u/app.git"}, testProfile(), &out, &errOut); err != nil {
+		t.Fatalf("Get returned error: %v", err)
+	}
+	if out.Len() != 0 {
+		t.Fatalf("stdout = %q, want empty", out.String())
+	}
+	stderr := errOut.String()
+	if !strings.Contains(stderr, "create Git credential lock dir") {
+		t.Fatalf("stderr = %q, want typed lock-dir setup error", stderr)
+	}
+	if strings.Contains(stderr, "acquire Git credential lock") {
+		t.Fatalf("stderr rewrapped typed lock error: %q", stderr)
+	}
+}
+
 func TestManagerInitStoreAndSecretReadErrors(t *testing.T) {
 	now := time.Unix(1780000000, 0)
 	path := filepath.Join(t.TempDir(), MetadataFilename)
@@ -1771,8 +1871,15 @@ func TestManagerGetBranches(t *testing.T) {
 	if err := manager.Get(context.Background(), CredentialInput{Protocol: "https", Host: "example.com", Path: "/git/u/app.git"}, testProfile(), &out, &errOut); err != nil {
 		t.Fatalf("Get keychain set error returned error: %v", err)
 	}
-	if !strings.Contains(errOut.String(), "keychain locked") {
-		t.Fatalf("stderr = %q, want keychain error", errOut.String())
+	stderr := errOut.String()
+	if !strings.Contains(stderr, "save local Git credential PAT to keychain failed") {
+		t.Fatalf("stderr = %q, want static keychain error", stderr)
+	}
+	if !strings.Contains(stderr, "lark-cli apps +git-credential-init") {
+		t.Fatalf("stderr = %q, want init retry hint", stderr)
+	}
+	if strings.Contains(stderr, "keychain locked") {
+		t.Fatalf("stderr leaks keychain cause: %q", stderr)
 	}
 
 	kc.setErr = nil
@@ -2165,6 +2272,189 @@ func TestNilManagerUsesTimeNow(t *testing.T) {
 	}
 }
 
+// TestRedactCredentialText focuses on the redaction regex, asserting it
+// covers credential shapes across forms and does not over-match concatenated
+// words. JSON-quoted forms are common in server-provided error bodies and must
+// be covered; concatenated words like mytoken must not be treated as token.
+func TestRedactCredentialText(t *testing.T) {
+	samplePAT := testPublicSafeJoin("pat", "-sample")
+	samplePassword := "sample-password"
+	sampleSecret := "sample-secret"
+	githubLikeToken := testPublicSafeJoin("gh", "p_") + strings.Repeat("x", 20)
+	cases := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{
+			name: "bare assignment",
+			in: "permission denied: " +
+				testCredentialAssignment("token", samplePAT) + " " +
+				testCredentialAssignment("password", samplePassword),
+			want: "permission denied: " +
+				testRedactedAssignment("token") + " " +
+				testRedactedAssignment("password"),
+		},
+		{
+			name: "json double-quoted value",
+			in: "body={" +
+				testDoubleQuotedAssignment("password", samplePassword) + "," +
+				testDoubleQuotedAssignment("token", samplePAT) +
+				"}",
+			want: "body={" +
+				testDoubleQuotedRedactedAssignment("password") + "," +
+				testDoubleQuotedRedactedAssignment("token") +
+				"}",
+		},
+		{
+			name: "json single-quoted value",
+			in:   "body={" + testSingleQuotedAssignment("secret", sampleSecret) + "}",
+			want: "body={" + testSingleQuotedRedactedAssignment("secret") + "}",
+		},
+		{
+			name: "colon separator with quoted value",
+			in:   testCredentialColon("token", `"`+samplePAT+`"`),
+			want: testRedactedColon("token"),
+		},
+		{
+			name: "url userinfo",
+			in:   "clone " + testCredentialURLWithUserInfo("example.com/repo.git", samplePAT),
+			want: "clone https://***@example.com/repo.git",
+		},
+		{
+			name: "bearer header",
+			in:   testAuthorizationBearer(githubLikeToken),
+			want: testRedactedAuthorizationBearer(),
+		},
+		{
+			name: "pat-like standalone",
+			in:   "issued " + samplePAT + " for app",
+			want: "issued <redacted> for app",
+		},
+		{
+			name: "concatenated key not redacted",
+			in:   testCredentialAssignment("mytoken", "abc123") + " " + testCredentialAssignment("secret_field", "see"),
+			want: testCredentialAssignment("mytoken", "abc123") + " " + testCredentialAssignment("secret_field", "see"),
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := RedactCredentialText(tc.in); got != tc.want {
+				t.Fatalf("RedactCredentialText(%q) = %q, want %q", tc.in, got, tc.want)
+			}
+		})
+	}
+}
+
+func TestSafeCredentialErrorMessageFallbacks(t *testing.T) {
+	if got := safeCredentialErrorMessage(nil); got != "" {
+		t.Fatalf("safeCredentialErrorMessage(nil) = %q, want empty", got)
+	}
+
+	if got := safeCredentialErrorMessage(&errs.ConfigError{Problem: errs.Problem{
+		Category: errs.CategoryConfig,
+		Subtype:  errs.SubtypeInvalidConfig,
+	}}); got != "config/invalid_config" {
+		t.Fatalf("safeCredentialErrorMessage typed fallback = %q, want config/invalid_config", got)
+	}
+
+	samplePAT := testPublicSafeJoin("pat", "-sample")
+	got := safeCredentialErrorMessage(errors.New("transport failed with " + testCredentialAssignment("token", samplePAT)))
+	if strings.Contains(got, samplePAT) {
+		t.Fatalf("safeCredentialErrorMessage leaks credential value: %q", got)
+	}
+	if want := testRedactedAssignment("token"); !strings.Contains(got, want) {
+		t.Fatalf("safeCredentialErrorMessage missing %q after redaction: %q", want, got)
+	}
+}
+
+func TestWriteCredentialErrorRedactsTypedMessageHint(t *testing.T) {
+	samplePAT := testPublicSafeJoin("pat", "-sample")
+	err := errs.NewInternalError(errs.SubtypeStorage, "save failed with %s", testCredentialAssignment("token", samplePAT)).
+		WithHint("retry without %s", testCredentialAssignment("password", samplePAT)).
+		WithLogID("log_x")
+
+	var buf bytes.Buffer
+	writeCredentialError(&buf, "Git credential refresh failed", err)
+	got := buf.String()
+	for _, leaked := range []string{samplePAT, testCredentialAssignment("token", samplePAT), testCredentialAssignment("password", samplePAT)} {
+		if strings.Contains(got, leaked) {
+			t.Fatalf("writeCredentialError leaks credential value %q in %q", leaked, got)
+		}
+	}
+	for _, want := range []string{
+		"Git credential refresh failed: save failed with " + testRedactedAssignment("token"),
+		"log_id=log_x",
+		"hint: retry without " + testRedactedAssignment("password"),
+	} {
+		if !strings.Contains(got, want) {
+			t.Fatalf("writeCredentialError output missing %q: %q", want, got)
+		}
+	}
+
+	writeCredentialError(nil, "ignored", err)
+	writeCredentialError(&buf, "ignored", nil)
+}
+
+func assertProblem(t *testing.T, err error, wantCategory errs.Category, wantSubtype errs.Subtype) {
+	t.Helper()
+	p, ok := errs.ProblemOf(err)
+	if !ok {
+		t.Fatalf("expected typed errs.Problem, got %T: %v", err, err)
+	}
+	if p.Category != wantCategory || p.Subtype != wantSubtype {
+		t.Fatalf("problem metadata = %s/%s, want %s/%s", p.Category, p.Subtype, wantCategory, wantSubtype)
+	}
+}
+
+func testPublicSafeJoin(parts ...string) string {
+	return strings.Join(parts, "")
+}
+
+func testCredentialAssignment(key, value string) string {
+	return key + "=" + value
+}
+
+func testRedactedAssignment(key string) string {
+	return key + "=<redacted>"
+}
+
+func testCredentialColon(key, value string) string {
+	return key + ": " + value
+}
+
+func testRedactedColon(key string) string {
+	return key + ": <redacted>"
+}
+
+func testDoubleQuotedAssignment(key, value string) string {
+	return `"` + key + `"` + ":" + `"` + value + `"`
+}
+
+func testDoubleQuotedRedactedAssignment(key string) string {
+	return `"` + key + `"` + ":<redacted>"
+}
+
+func testSingleQuotedAssignment(key, value string) string {
+	return `'` + key + `'` + ":" + `'` + value + `'`
+}
+
+func testSingleQuotedRedactedAssignment(key string) string {
+	return `'` + key + `'` + ":<redacted>"
+}
+
+func testCredentialURLWithUserInfo(hostPath, credential string) string {
+	return "https://" + "user:" + credential + "@" + hostPath
+}
+
+func testAuthorizationBearer(value string) string {
+	return "Authorization" + ": " + "Bearer " + value
+}
+
+func testRedactedAuthorizationBearer() string {
+	return "Authorization" + ": " + "Bearer <redacted>"
+}
+
 func testProfile() ProfileContext {
 	return ProfileContext{Profile: "default", ProfileAppID: "cli_xxx", UserOpenID: "ou_xxx"}
 }

shortcuts/apps/gitcred/helper.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"regexp"
 	"strings"
 	"time"
 
@@ -27,6 +28,25 @@ type Manager struct {
 	Now       func() time.Time
 }
 
+// credentialKeys is the shared list of credential field names to redact; the
+// bare, double-quoted (JSON), and single-quoted forms all reuse it.
+const credentialKeys = `access_token|refresh_token|app_secret|token|pat|password|secret`
+
+var (
+	credentialURLUserinfoRE = regexp.MustCompile(`(?i)(https?://)[^/\s]+@`)
+	// credentialAssignmentRE matches credential key assignments, including JSON
+	// quoted forms. Capture group 1 is the key and separator; only the value is
+	// replaced with <redacted>. The key is one of three forms — double-quoted,
+	// single-quoted, or bare with a word boundary — so concatenated words like
+	// mytoken are not matched. Each form wraps the key list in (?:...) so the |
+	// alternation does not bind the quote/boundary to only the first and last key.
+	credentialAssignmentRE = regexp.MustCompile(
+		`(?i)((?:"(?:` + credentialKeys + `)"|'(?:` + credentialKeys + `)'|\b(?:` + credentialKeys + `)\b)\s*[:=]\s*)(?:"[^"]*"|'[^']*'|[^\s,;]+)`,
+	)
+	credentialBearerRE  = regexp.MustCompile(`(?i)(authorization\s*:\s*bearer\s+)[^\s,;]+`)
+	credentialPATLikeRE = regexp.MustCompile(`(?i)\b(?:gh[pousr]_[A-Za-z0-9_]{20,}|pat-[A-Za-z0-9._-]+)\b`)
+)
+
 func NewManager(store *Store, secrets *SecretStore, gitConfig GitConfig, issuer Issuer) *Manager {
 	return &Manager{
 		Store:     store,
@@ -172,12 +192,12 @@ func (m *Manager) List() (*ListResult, error) {
 func (m *Manager) Get(ctx context.Context, input CredentialInput, current ProfileContext, out, errOut io.Writer) error {
 	url, err := NormalizeCredentialInput(input)
 	if err != nil {
-		fmt.Fprintf(errOut, "Git credential unavailable: %s\n", err)
+		writeCredentialError(errOut, "Git credential unavailable", err)
 		return nil
 	}
 	record, pat, ok, err := m.readConfirmed(url, current)
 	if err != nil {
-		fmt.Fprintf(errOut, "Git credential unavailable: %s\n", err)
+		writeCredentialError(errOut, "Git credential unavailable", err)
 		return nil
 	}
 	if !ok {
@@ -187,18 +207,28 @@ func (m *Manager) Get(ctx context.Context, input CredentialInput, current Profil
 		return writeGitCredential(out, record.Username, pat)
 	}
 
-	unlock := lockURL(url)
-	defer unlock()
+	// Lock ordering convention (see lock.go package comment): always acquire
+	// lockApp before lockURL. lockApp is a cross-process file lock with a
+	// timeout and possible setup failure; acquiring it first avoids holding an
+	// in-process mutex on the failure path, which would risk a deadlock.
 	unlockApp, err := lockApp(record.AppID)
 	if err != nil {
-		fmt.Fprintf(errOut, "Git credential refresh failed: acquire lock for %s: %s\n", record.AppID, err)
+		// lockApp may already return a typed error, for example when creating
+		// the lock directory fails. Preserve those classifications and only wrap
+		// raw lockfile errors to add app context.
+		if _, ok := errs.ProblemOf(err); !ok {
+			err = errs.NewInternalError(errs.SubtypeStorage, "acquire Git credential lock for %s: %v", record.AppID, err).WithCause(err)
+		}
+		writeCredentialError(errOut, "Git credential refresh failed", err)
 		return nil
 	}
 	defer unlockApp()
+	unlockURL := lockURL(url)
+	defer unlockURL()
 
 	record, pat, ok, err = m.readConfirmed(url, current)
 	if err != nil {
-		fmt.Fprintf(errOut, "Git credential unavailable: %s\n", err)
+		writeCredentialError(errOut, "Git credential unavailable", err)
 		return nil
 	}
 	if !ok {
@@ -213,16 +243,17 @@ func (m *Manager) Get(ctx context.Context, input CredentialInput, current Profil
 	}
 	issued, err := m.Issuer.Issue(ctx, record.AppID, current)
 	if err != nil {
-		fmt.Fprintf(errOut, "Git credential refresh failed: %s\nNext step: lark-cli apps +git-credential-init --app-id %s\n", err, record.AppID)
+		writeCredentialError(errOut, "Git credential refresh failed", err)
+		fmt.Fprintf(errOut, "Next step: lark-cli apps +git-credential-init --app-id %s\n", record.AppID)
 		return nil
 	}
 	issuedURL, urlErr := NormalizeGitHTTPURL(issued.GitHTTPURL)
 	if urlErr != nil {
-		fmt.Fprintf(errOut, "Git credential refresh failed: %s\n", urlErr)
+		writeCredentialError(errOut, "Git credential refresh failed", urlErr)
 		return nil
 	}
 	if err := validateIssuedCredential(record.AppID, issuedURL, issued, m.nowUnix()); err != nil {
-		fmt.Fprintf(errOut, "Git credential refresh failed: %s\n", err)
+		writeCredentialError(errOut, "Git credential refresh failed", err)
 		return nil
 	}
 	if issuedURL != url {
@@ -232,7 +263,7 @@ func (m *Manager) Get(ctx context.Context, input CredentialInput, current Profil
 	if issued.ExpiresAt < record.ExpiresAt {
 		latest, latestPAT, found, readErr := m.readConfirmed(url, current)
 		if readErr != nil {
-			fmt.Fprintf(errOut, "Git credential unavailable: %s\n", readErr)
+			writeCredentialError(errOut, "Git credential unavailable", readErr)
 			return nil
 		}
 		if found && m.usable(latest, latestPAT) {
@@ -247,17 +278,64 @@ func (m *Manager) Get(ctx context.Context, input CredentialInput, current Profil
 	record.Status = StatusConfirmed
 	oldPAT := pat
 	if err := m.Secrets.Set(record.PATRef, issued.PAT); err != nil {
-		fmt.Fprintf(errOut, "Git credential refresh failed: %s\n", err)
+		writeCredentialError(errOut, "Git credential refresh failed", err)
 		return nil
 	}
 	if err := m.Store.Upsert(record); err != nil {
 		_ = m.Secrets.Set(record.PATRef, oldPAT)
-		fmt.Fprintf(errOut, "Git credential refresh failed: %s\n", err)
+		writeCredentialError(errOut, "Git credential refresh failed", err)
 		return nil
 	}
 	return writeGitCredential(out, record.Username, issued.PAT)
 }
 
+func writeCredentialError(w io.Writer, prefix string, err error) {
+	if w == nil || err == nil {
+		return
+	}
+	fmt.Fprintf(w, "%s: %s\n", prefix, safeCredentialErrorMessage(err))
+}
+
+func safeCredentialErrorMessage(err error) string {
+	if err == nil {
+		return ""
+	}
+	if p, ok := errs.ProblemOf(err); ok {
+		message := RedactCredentialText(p.Message)
+		if p.LogID != "" {
+			if message != "" {
+				message += "; "
+			}
+			message += "log_id=" + p.LogID
+		}
+		if p.Hint != "" {
+			if message != "" {
+				message += "; "
+			}
+			message += "hint: " + RedactCredentialText(p.Hint)
+		}
+		if message != "" {
+			return message
+		}
+		if p.Category != "" || p.Subtype != "" {
+			return strings.Trim(strings.TrimSpace(string(p.Category)+"/"+string(p.Subtype)), "/")
+		}
+	}
+	return RedactCredentialText(err.Error())
+}
+
+// RedactCredentialText masks credential fragments that may appear in free
+// text, covering URL userinfo, Authorization bearer headers, credential
+// assignments including JSON-quoted forms, and PAT-shaped strings. Shared by
+// the gitcred and apps packages so the redaction logic does not fork.
+func RedactCredentialText(text string) string {
+	text = credentialURLUserinfoRE.ReplaceAllString(text, "${1}***@")
+	text = credentialBearerRE.ReplaceAllString(text, "${1}<redacted>")
+	text = credentialAssignmentRE.ReplaceAllString(text, "${1}<redacted>")
+	text = credentialPATLikeRE.ReplaceAllString(text, "<redacted>")
+	return text
+}
+
 func (m *Manager) currentAppRecord(appID string) (*CredentialRecord, error) {
 	records, err := m.Store.FindByAppID(appID, ProfileContext{})
 	if err != nil || len(records) == 0 {

shortcuts/apps/gitcred/keychain.go

@@ -42,7 +42,15 @@ func (s *SecretStore) Set(ref, pat string) error {
 			Message:  "keychain PAT reference is empty",
 		}}
 	}
-	return s.kc.Set(KeychainService, ref, pat)
+	if err := s.kc.Set(KeychainService, ref, pat); err != nil {
+		return &errs.ConfigError{Problem: errs.Problem{
+			Category: errs.CategoryConfig,
+			Subtype:  errs.SubtypeInvalidConfig,
+			Message:  "save local Git credential PAT to keychain failed",
+			Hint:     "make sure the system credential store is available, then retry lark-cli apps +git-credential-init",
+		}, Cause: err}
+	}
+	return nil
 }
 
 func (s *SecretStore) Remove(ref string) error {
@@ -64,7 +72,7 @@ func (s *SecretStore) Remove(ref string) error {
 		return &errs.ConfigError{Problem: errs.Problem{
 			Category: errs.CategoryConfig,
 			Subtype:  errs.SubtypeInvalidConfig,
-			Message:  "remove local Git credential PAT from keychain failed: " + err.Error(),
+			Message:  "remove local Git credential PAT from keychain failed",
 			Hint:     "make sure the system credential store is available, then retry lark-cli apps +git-credential-remove",
 		}, Cause: err}
 	}

shortcuts/apps/gitcred/lock.go

@@ -1,6 +1,24 @@
 // Copyright (c) 2026 Lark Technologies Pte. Ltd.
 // SPDX-License-Identifier: MIT
 
+// Package gitcred manages the lifecycle of app Git credentials.
+//
+// Lock ordering convention — read this before adding any new lock acquisition:
+//
+//	ALWAYS acquire lockApp BEFORE lockURL. Never invert this order.
+//
+// Rationale:
+//   - lockApp is a cross-process file lock with bounded timeout (2s) and a
+//     possible setup error; acquiring it first keeps the failure surface
+//     outside any in-process lock and avoids holding the in-process mutex
+//     while waiting on I/O / another process.
+//   - lockURL is an in-process sync.Mutex that never fails and blocks
+//     indefinitely; holding it while waiting on lockApp would risk
+//     deadlocking with a concurrent goroutine that held lockApp first.
+//
+// Paths that only manipulate per-app state (Init, Remove, Erase) only need
+// lockApp. Get() is the only path that touches per-URL state in addition to
+// per-app state, so it is the only caller that takes both locks.
 package gitcred
 
 import (
@@ -20,6 +38,11 @@ var urlLocks sync.Map
 
 var safeLockNameChars = regexp.MustCompile(`[^a-zA-Z0-9._-]`)
 
+// lockURL acquires an in-process, per-URL mutex. It never returns an error
+// and blocks until the mutex is available.
+//
+// Lock ordering: lockURL MUST NOT be held while calling lockApp. See package
+// comment for the full convention.
 func lockURL(url string) func() {
 	actual, _ := urlLocks.LoadOrStore(url, &sync.Mutex{})
 	mu := actual.(*sync.Mutex)
@@ -27,6 +50,12 @@ func lockURL(url string) func() {
 	return mu.Unlock
 }
 
+// lockApp acquires a cross-process file lock scoped to the given appID. It
+// returns an unlock function or an error if the lock directory cannot be
+// created or the lock cannot be acquired within the 2s timeout.
+//
+// Lock ordering: when both lockApp and lockURL are needed, lockApp must be
+// taken FIRST. See package comment for the full convention.
 func lockApp(appID string) (func(), error) {
 	dir := filepath.Join(core.GetConfigDir(), "locks")
 	if err := vfs.MkdirAll(dir, 0700); err != nil {