internal/ai/llm/openai/openai.go

@@ -526,31 +526,77 @@ func redactKey(s, key string) string {
 // key, because the key is part of the replacement marker.
 const withheldText = "openai: error text withheld: it contains the API key"
 
-// redactedError is an error whose text had the API key removed. It unwraps
-// to the original error, so errors.Is and errors.As still see the chain.
+// redactedError stands in for one layer of an error chain that holds the
+// API key. Its text is the layer's text with the key redacted, and Unwrap
+// returns the layer's causes passed through redact, so no error reached by
+// unwrapping has text holding the key. Is and As consult the original
+// layer and its chain, so sentinels and typed errors still match.
 type redactedError struct {
 	text string
 	err  error
+	key  string
 }
 
 func (e *redactedError) Error() string { return e.text }
 
-func (e *redactedError) Unwrap() error { return e.err }
+func (e *redactedError) Unwrap() error { return redact(errors.Unwrap(e.err), e.key) }
+
+func (e *redactedError) Is(target error) bool { return errors.Is(e.err, target) }
+
+func (e *redactedError) As(target any) bool { return errors.As(e.err, target) }
+
+// redactedJoin is redactedError for a layer with several causes, such as
+// one built by errors.Join or by fmt.Errorf with more than one %w.
+type redactedJoin struct {
+	redactedError
+}
+
+func (e *redactedJoin) Unwrap() []error {
+	causes := e.err.(interface{ Unwrap() []error }).Unwrap()
+	safe := make([]error, len(causes))
+	for i, c := range causes {
+		safe[i] = redact(c, e.key)
+	}
+	return safe
+}
 
 // redact is the backstop every exported method applies to its error: when
-// err's text, wrapped errors included, holds a non-empty key, it returns err
-// with that text redacted. Any other err is returned unchanged.
+// the text of err or of any error in its chain holds a non-empty key, it
+// returns a stand-in whose text, and the text of every error it unwraps
+// to, has the key redacted. Any other err is returned unchanged.
 func redact(err error, key string) error {
-	if err == nil || key == "" {
+	if err == nil || key == "" || !holdsKey(err, key) {
 		return err
 	}
-	text := err.Error()
-	if !strings.Contains(text, key) {
-		return err
-	}
-	text = redactKey(text, key)
+	text := redactKey(err.Error(), key)
 	if strings.Contains(text, key) {
 		text = withheldText
 	}
-	return &redactedError{text: text, err: err}
+	re := redactedError{text: text, err: err, key: key}
+	if _, ok := err.(interface{ Unwrap() []error }); ok {
+		return &redactedJoin{re}
+	}
+	return &re
+}
+
+// holdsKey reports whether the text of err or of any error in its chain
+// contains key.
+func holdsKey(err error, key string) bool {
+	if err == nil {
+		return false
+	}
+	if strings.Contains(err.Error(), key) {
+		return true
+	}
+	switch u := err.(type) {
+	case interface{ Unwrap() error }:
+		return holdsKey(u.Unwrap(), key)
+	case interface{ Unwrap() []error }:
+		for _, c := range u.Unwrap() {
+			if holdsKey(c, key) {
+				return true
+			}
+		}
+	}
+	return false
 }

internal/ai/llm/openai/openai_test.go

@@ -868,8 +868,8 @@ func TestProviderErrorBodyNeverLeaksKey(t *testing.T) {
 func TestRedactKeepsChainAndHidesKey(t *testing.T) {
 	inner := fmt.Errorf("decode: body %q: %w", "Bearer "+testKey, llm.ErrBadOutput)
 	err := redact(inner, testKey)
-	assertNoSecret(t, err)
-	if !errors.Is(err, llm.ErrBadOutput) || errors.Unwrap(err) != inner {
+	assertChainNoKey(t, err, testKey)
+	if !errors.Is(err, llm.ErrBadOutput) {
 		t.Fatalf("redact broke the chain of %v", err)
 	}
 	if redact(inner, "") != inner {
@@ -884,3 +884,89 @@ func TestRedactKeepsChainAndHidesKey(t *testing.T) {
 		t.Fatalf("redact kept the key: %q", got)
 	}
 }
+
+// assertChainNoKey fails if err, or any error reached from it by unwrapping,
+// has text holding key or "Bearer "+key.
+func assertChainNoKey(t *testing.T, err error, key string) {
+	t.Helper()
+	if err == nil {
+		return
+	}
+	assertNoKey(t, err, key)
+	switch u := err.(type) {
+	case interface{ Unwrap() error }:
+		assertChainNoKey(t, u.Unwrap(), key)
+	case interface{ Unwrap() []error }:
+		for _, c := range u.Unwrap() {
+			assertChainNoKey(t, c, key)
+		}
+	}
+}
+
+// TestGenerateInvalidJSONWithKeyNeverLeaksKeyByUnwrapping sends an invalid
+// JSON body holding the key and walks the returned chain with errors.Unwrap.
+func TestGenerateInvalidJSONWithKeyNeverLeaksKeyByUnwrapping(t *testing.T) {
+	p := keyServer(t, 200, `{"choices": Bearer `+headerNameKey+` `+headerNameKey+`}`)
+	_, err := p.Generate(context.Background(), schemaRequest())
+	if err == nil {
+		t.Fatal("Generate accepted an invalid JSON body")
+	}
+	var syntax *json.SyntaxError
+	if !errors.As(err, &syntax) {
+		t.Fatalf("err = %v, want a *json.SyntaxError in the chain", err)
+	}
+	for layer := err; layer != nil; layer = errors.Unwrap(layer) {
+		assertNoKey(t, layer, headerNameKey)
+	}
+}
+
+// TestRedactMakesEveryUnwrappedLayerSafe covers chains whose inner layers
+// hold the key, including a layer whose own text does not show it and a
+// layer with several causes, and checks Is and As still match.
+func TestRedactMakesEveryUnwrappedLayerSafe(t *testing.T) {
+	pe := llm.NewProviderError(500, []byte(`{"error":"down"}`), testKey)
+	hidden := &opaqueError{err: fmt.Errorf("body Bearer %s: %w", testKey, llm.ErrAuth)}
+	inner := fmt.Errorf("decode %s: %w", testKey, llm.ErrBadOutput)
+	joined := errors.Join(inner, pe, context.Canceled, fmt.Errorf("limit: %w", llm.ErrRateLimited))
+	cases := []error{
+		fmt.Errorf("openai: generate: %w", inner),
+		fmt.Errorf("openai: generate: %w", hidden),
+		fmt.Errorf("openai: generate: %w", joined),
+		fmt.Errorf("openai: %w and %w", inner, &transportFailure{detail: "slow", kind: llm.ErrTimeout}),
+	}
+	for _, c := range cases {
+		err := redact(c, testKey)
+		if err == c {
+			t.Fatalf("redact left %v unchanged", c)
+		}
+		assertChainNoKey(t, err, testKey)
+		for layer := err; layer != nil; layer = errors.Unwrap(layer) {
+			assertNoKey(t, layer, testKey)
+		}
+	}
+	err := redact(fmt.Errorf("openai: %w", joined), testKey)
+	var got *llm.ProviderError
+	if !errors.As(err, &got) || got != pe {
+		t.Fatalf("errors.As lost the ProviderError in %v", err)
+	}
+	for _, target := range []error{llm.ErrBadOutput, llm.ErrProvider, llm.ErrRateLimited, context.Canceled} {
+		if !errors.Is(err, target) {
+			t.Fatalf("errors.Is(%v, %v) = false", err, target)
+		}
+	}
+	err = redact(fmt.Errorf("openai: %w", hidden), testKey)
+	if !errors.Is(err, llm.ErrAuth) {
+		t.Fatalf("errors.Is lost ErrAuth in %v", err)
+	}
+	err = redact(fmt.Errorf("openai %s: %w", testKey, &transportFailure{detail: "slow", kind: llm.ErrTimeout}), testKey)
+	if !errors.Is(err, llm.ErrTimeout) {
+		t.Fatalf("errors.Is lost ErrTimeout in %v", err)
+	}
+}
+
+// opaqueError hides its cause's text but unwraps to it.
+type opaqueError struct{ err error }
+
+func (e *opaqueError) Error() string { return "opaque" }
+
+func (e *opaqueError) Unwrap() error { return e.err }