diff --git a/cli/azd/extensions/azure.ai.content-understanding/CHANGELOG.md b/cli/azd/extensions/azure.ai.content-understanding/CHANGELOG.md index 5bf23016a53..487bea7c652 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/CHANGELOG.md +++ b/cli/azd/extensions/azure.ai.content-understanding/CHANGELOG.md @@ -9,6 +9,16 @@ displayed as tables or labeled cards. Default output and saved results remain unchanged. +### Bugs Fixed + +- Show sanitized service failure reasons and nested error codes in terminal + diagnostics and per-input reports, including HTTP, polling, and inline failures. + Keep useful model names and provide actionable guidance for `ModelNotSupported` + instead of replacing the root cause with a generic error. Redact credentials + and sensitive diagnostic values, and explicitly mark missing or shortened details. + Show accurate success/failure/skip counts and only mention retained result files + when files were actually saved. + ### Other Changes - Make `azd ai cu version` easier to read with a descriptive title, labeled diff --git a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/AZD_PROVENANCE.md b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/AZD_PROVENANCE.md index 8a104d70432..62c4f2e7f02 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/AZD_PROVENANCE.md +++ b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/AZD_PROVENANCE.md @@ -13,7 +13,11 @@ following behavior with the extension's adapter and command tests. - Mark malformed successful responses with `ErrInvalidResponse`. - Preserve unsuccessful terminal status and safe service codes in `OperationError`, wrapping `azcore.ResponseError` while leaving CLI error classification to the adapter. -- Retain sanitized nested operation diagnostic codes without service messages or targets. +- Retain sanitized service diagnostic messages and nested codes from both `details` and + `innererror`, without targets, credentials, or raw response bodies. Normalize inner errors into + diagnostic `Details`, bound depth/count, and explicitly mark truncation. +- Share diagnostic filtering with the command boundary so safe model identifiers + and root-cause explanations survive repeated sanitization. - Bound and sanitize service and polling errors before creating `azcore.ResponseError` values. - Use a same-resource polling handler that retains terminal envelopes without diff --git a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/README.md b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/README.md index b7a4e587760..385c0e435bc 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/README.md +++ b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/README.md @@ -27,8 +27,15 @@ authentication contract. - Reads and polling use the configured retry policy and honor cancellation. Write submissions are not automatically retried or replayed after an authentication challenge. -- Service and operation failures preserve typed status and safe diagnostic codes - without exposing raw service messages. +- Service and operation failures preserve typed status, diagnostic codes, and + sanitized messages. They retain useful model identifiers while redacting + credentials, URLs, paths, quoted customer values, and labeled document data. + `ServiceError` and `OperationError` both wrap `azcore.ResponseError`. + Terminal operation diagnostics normalize + `innererror` into nested `Details` alongside `details`, with limits of eight + levels and 32 diagnostics. `DiagnosticsTruncated` marks omitted diagnostics. + The shared `RedactDiagnostic` helper also protects the extension's command + error boundary without modifying successful result content. See [client integration requirements](https://github.com/Azure/azure-dev/blob/main/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/AZD_PROVENANCE.md) for the behavior to verify when changing the client dependency. diff --git a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/client.go b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/client.go index 5d057b78f58..cf8e0bc5653 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/client.go +++ b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/client.go @@ -207,6 +207,9 @@ func (c *Client) send(req *policy.Request, codes ...int) (*http.Response, error) if err != nil { return nil, err } + if resp.Request == nil { + resp.Request = req.Raw() + } if !runtime.HasStatusCode(resp, codes...) { if resp.StatusCode >= 200 && resp.StatusCode < 300 { return nil, ErrInvalidResponse @@ -218,6 +221,7 @@ func (c *Client) send(req *policy.Request, codes ...int) (*http.Response, error) func safeResponseError(resp *http.Response, explicitCode string) error { code := explicitCode + var diagnostic *OperationDiagnostic if code == "" { code = resp.Header.Get("x-ms-error-code") } @@ -231,9 +235,19 @@ func safeResponseError(resp *http.Response, explicitCode string) error { if bodyCode, nested := deepestErrorCode(payload); bodyCode != "" && (code == "" || nested) { code = bodyCode } + var envelope struct { + Error *OperationDiagnostic `json:"error"` + } + if json.Unmarshal(payload, &envelope) == nil && envelope.Error != nil { + diagnostic = &safeOperationDiagnostics([]OperationDiagnostic{*envelope.Error}, resp)[0] + } + } else { + diagnostic = &OperationDiagnostic{ + Message: "Service diagnostic omitted because the response exceeds the safe size limit.", + } } } - if !serviceErrorCodePattern.MatchString(code) { + if !serviceErrorCodePattern.MatchString(code) || redactResponseDiagnostic(code, resp) != code { code = fmt.Sprintf("http_%d", resp.StatusCode) } safe := &http.Response{ @@ -241,7 +255,11 @@ func safeResponseError(resp *http.Response, explicitCode string) error { ProtoMajor: resp.ProtoMajor, ProtoMinor: resp.ProtoMinor, Header: resp.Header.Clone(), Body: http.NoBody, ContentLength: 0, } - return &azcore.ResponseError{ErrorCode: code, StatusCode: resp.StatusCode, RawResponse: safe} + responseError := &azcore.ResponseError{ErrorCode: code, StatusCode: resp.StatusCode, RawResponse: safe} + if diagnostic != nil { + return &ServiceError{Diagnostic: *diagnostic, ResponseError: responseError} + } + return responseError } func deepestErrorCode(payload []byte) (string, bool) { @@ -255,7 +273,7 @@ func deepestErrorCode(payload []byte) (string, bool) { code := "" nested := false depth := 0 - for len(current) != 0 { + for len(current) != 0 && depth < 8 { var item struct { Code string `json:"code"` InnerError json.RawMessage `json:"innererror"` diff --git a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/diagnostics.go b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/diagnostics.go new file mode 100644 index 00000000000..aa72379c897 --- /dev/null +++ b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/diagnostics.go @@ -0,0 +1,85 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package curestclient + +import ( + "net/http" + "regexp" + "strings" + "unicode" + "unicode/utf8" +) + +const MaxDiagnosticBytes = 2048 + +var ( + diagnosticAuthorization = regexp.MustCompile(`(?im)\b(?:proxy-)?authorization\s*[:=]\s*[^\r\n]*`) + diagnosticSecret = regexp.MustCompile( + `(?im)\b(?:[a-z_]*token|api[-_]?key|ocp-apim-subscription-key|account[-_]?key|client_secret|` + + `password|secret|sas|sig|schema|content|text|payload|body|path)\s*[:=]\s*[^\r\n]*`) + diagnosticURL = regexp.MustCompile(`(?i)\b[a-z][a-z0-9+.-]*://[^\s"'<>]+`) + diagnosticWindowsPath = regexp.MustCompile(`(?i)(?:[a-z]:[\\/]|\\\\)[^\s"'<>]+`) + diagnosticPOSIXPath = regexp.MustCompile(`(^|[\s"'=(])(?:/|~/|\.\.?/)[^\s"'<>]+`) + diagnosticQuotedValue = regexp.MustCompile(`(?m)"[^"\r\n]*(?:"|$)|(^|[\s:=(])'[^'\r\n]*(?:'|$)`) + diagnosticFieldPath = regexp.MustCompile(`\b(fieldSchema\.fields|fields|properties)\.[^\s:]+`) + diagnosticModelName = regexp.MustCompile( + `(?i)\b(model(?: name)?\s+)(?:'([a-z0-9][a-z0-9._-]{0,127})'|"([a-z0-9][a-z0-9._-]{0,127})")`) +) + +// RedactDiagnostic preserves diagnostic prose and model identifiers while removing +// credentials, URLs, paths, quoted customer values, and labeled document data. +// It must not be used to alter successful service results. +func RedactDiagnostic(message string) string { + if len(message) > 64*1024 || !utf8.ValidString(message) { + return "Diagnostic omitted because it exceeds the safe text limit." + } + + if start := strings.IndexByte(message, '{'); start >= 0 { + message = message[:start] + "[redacted payload]" + } + message = diagnosticAuthorization.ReplaceAllString(message, "Authorization: [redacted]") + message = diagnosticSecret.ReplaceAllString(message, "[redacted value]") + message = diagnosticURL.ReplaceAllString(message, "[redacted URL]") + message = diagnosticWindowsPath.ReplaceAllString(message, "[redacted path]") + message = diagnosticPOSIXPath.ReplaceAllString(message, "${1}[redacted path]") + // Remove quotes only around explicitly identified, syntactically safe model names. + message = diagnosticModelName.ReplaceAllString(message, "${1}${2}${3}") + message = diagnosticQuotedValue.ReplaceAllStringFunc(message, func(value string) string { + if value[0] == '"' { + return "[redacted value]" + } + return value[:strings.IndexByte(value, '\'')] + "[redacted value]" + }) + message = diagnosticFieldPath.ReplaceAllString(message, "${1}.[redacted]") + message = strings.Map(func(r rune) rune { + if unicode.IsControl(r) && r != '\n' && r != '\t' || unicode.Is(unicode.Bidi_Control, r) { + return -1 + } + return r + }, message) + message = strings.TrimSpace(message) + if len(message) > MaxDiagnosticBytes { + return "Diagnostic omitted because it exceeds the safe text limit." + } + return message +} + +func redactResponseDiagnostic(message string, response *http.Response) string { + if response != nil && response.Request != nil { + for _, name := range []string{"Authorization", "Proxy-Authorization", "Ocp-Apim-Subscription-Key"} { + value := response.Request.Header.Get(name) + if value == "" { + continue + } + message = strings.ReplaceAll(message, value, "[redacted credential]") + if name != "Ocp-Apim-Subscription-Key" { + _, credential, found := strings.Cut(value, " ") + if found && strings.TrimSpace(credential) != "" { + message = strings.ReplaceAll(message, strings.TrimSpace(credential), "[redacted credential]") + } + } + } + } + return RedactDiagnostic(message) +} diff --git a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/models.go b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/models.go index 2792e9a59b1..aa520144761 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/models.go +++ b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/models.go @@ -20,14 +20,28 @@ var ErrInvalidResponse = errors.New("invalid Content Understanding response") type OperationError struct { Status string Code string + Message string Details []OperationDiagnostic ResponseError *azcore.ResponseError } -// OperationDiagnostic retains nested service codes without customer messages or targets. +// ServiceError retains sanitized HTTP failure diagnostics and wraps azcore.ResponseError. +type ServiceError struct { + Diagnostic OperationDiagnostic + ResponseError *azcore.ResponseError +} + +func (e *ServiceError) Error() string { return e.ResponseError.Error() } + +func (e *ServiceError) Unwrap() error { return e.ResponseError } + +// OperationDiagnostic retains bounded service codes and sanitized messages, never targets. +// Returned diagnostics normalize InnerError into Details. type OperationDiagnostic struct { - Code string `json:"code"` - Details []OperationDiagnostic `json:"details,omitempty"` + Code string `json:"code"` + Message string `json:"message,omitempty"` + Details []OperationDiagnostic `json:"details,omitempty"` + InnerError *OperationDiagnostic `json:"innererror,omitempty"` } func (e *OperationError) Error() string { diff --git a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/polling.go b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/polling.go index fddb3043238..fe9fb0c6116 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/polling.go +++ b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/polling.go @@ -21,9 +21,8 @@ type operationPoller[T any] struct { location string done bool failed bool - code string status string - details []OperationDiagnostic + problem OperationDiagnostic operationID string response *http.Response payload []byte @@ -84,7 +83,7 @@ func (p *operationPoller[T]) Result(_ context.Context, out *T) error { return errors.New("operation has not completed") } if p.failed { - return newOperationError(p.response, p.status, OperationDiagnostic{Code: p.code, Details: p.details}) + return newOperationError(p.response, p.status, p.problem) } if err := json.Unmarshal(p.payload, out); err != nil { return errors.Join(ErrInvalidResponse, err) @@ -120,9 +119,8 @@ func (p *operationPoller[T]) accept(resp *http.Response, polled bool) error { case "failed", "canceled", "cancelled": p.done = true p.failed = true - p.code = state.Error.Code p.status = strings.ToLower(state.Status) - p.details = state.Error.Details + p.problem = state.Error case "notstarted", "running": default: if state.AnalyzerID != "" && (p.location == "" || strings.EqualFold(state.Status, "ready")) { @@ -139,7 +137,7 @@ func (p *operationPoller[T]) accept(resp *http.Response, polled bool) error { func newOperationError(resp *http.Response, status string, problem OperationDiagnostic) error { code := problem.Code - if !serviceErrorCodePattern.MatchString(code) { + if !serviceErrorCodePattern.MatchString(code) || redactResponseDiagnostic(code, resp) != code { code = "" } responseCode := code @@ -151,24 +149,39 @@ func newOperationError(resp *http.Response, status string, problem OperationDiag if !errors.As(err, &responseError) { return err } + diagnostic := safeOperationDiagnostics([]OperationDiagnostic{problem}, resp)[0] return &OperationError{ - Status: status, Code: code, Details: safeOperationDiagnostics(problem.Details), ResponseError: responseError, + Status: status, Code: code, + Message: diagnostic.Message, Details: diagnostic.Details, ResponseError: responseError, } } -func safeOperationDiagnostics(values []OperationDiagnostic) []OperationDiagnostic { - if values == nil { - return nil - } - result := make([]OperationDiagnostic, len(values)) - for i, value := range values { - code := value.Code - if !serviceErrorCodePattern.MatchString(code) { - code = "service_error" +func safeOperationDiagnostics(values []OperationDiagnostic, response *http.Response) []OperationDiagnostic { + remaining := 32 + var visit func([]OperationDiagnostic, int) []OperationDiagnostic + visit = func(values []OperationDiagnostic, depth int) []OperationDiagnostic { + var result []OperationDiagnostic + for _, value := range values { + if remaining == 0 || depth == 8 { + result = append(result, OperationDiagnostic{Code: "DiagnosticsTruncated"}) + break + } + remaining-- + code := value.Code + if !serviceErrorCodePattern.MatchString(code) || redactResponseDiagnostic(code, response) != code { + code = "service_error" + } + children := value.Details + if value.InnerError != nil { + children = append([]OperationDiagnostic{*value.InnerError}, children...) + } + result = append(result, OperationDiagnostic{ + Code: code, Message: redactResponseDiagnostic(value.Message, response), Details: visit(children, depth+1), + }) } - result[i] = OperationDiagnostic{Code: code, Details: safeOperationDiagnostics(value.Details)} + return result } - return result + return visit(values, 0) } func (c *Client) operationLocation(value, baseURL string) (string, error) { diff --git a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/tests/unit/client_test.go b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/tests/unit/client_test.go index 3a74b5319d0..8a4fca1462c 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/tests/unit/client_test.go +++ b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/tests/unit/client_test.go @@ -559,8 +559,9 @@ func TestFailedAndCanceledPolling(t *testing.T) { _, _ = io.WriteString(w, `{"status":"Running"}`) return } - _, _ = io.WriteString(w, `{"status":"`+state+`","error":{"code":"AnalysisFailed","message":"private value",`+ - `"details":[{"code":"InvalidField","message":"private value","target":"private target",`+ + _, _ = io.WriteString(w, `{"status":"`+state+`","error":{"code":"AnalysisFailed",`+ + `"message":"content: private value",`+ + `"details":[{"code":"InvalidField","message":"content: private value","target":"private target",`+ `"details":[{"code":"private value"}]}]}}`) }) server = srv @@ -573,8 +574,10 @@ func TestFailedAndCanceledPolling(t *testing.T) { require.ErrorAs(t, err, &operationErr) assert.Equal(t, strings.ToLower(state), operationErr.Status) assert.Equal(t, "AnalysisFailed", operationErr.Code) + assert.Equal(t, "[redacted value]", operationErr.Message) assert.Equal(t, []restclient.OperationDiagnostic{{ - Code: "InvalidField", Details: []restclient.OperationDiagnostic{{Code: "service_error"}}, + Code: "InvalidField", Message: "[redacted value]", + Details: []restclient.OperationDiagnostic{{Code: "service_error"}}, }}, operationErr.Details) var responseErr *azcore.ResponseError require.ErrorAs(t, err, &responseErr) @@ -594,9 +597,9 @@ func TestInlineTerminalOperationErrors(t *testing.T) { payload, err := json.Marshal(map[string]any{ "status": state, "error": map[string]any{ - "code": code, "message": "private value", "target": "private target", + "code": code, "message": "content: private value", "target": "private target", "details": []map[string]any{{ - "code": "InvalidField", "message": "private value", + "code": "InvalidField", "message": "content: private value", "details": []map[string]string{{"code": "private value"}}, }}, }, @@ -611,7 +614,8 @@ func TestInlineTerminalOperationErrors(t *testing.T) { require.ErrorAs(t, err, &operationErr) assert.Equal(t, strings.ToLower(state), operationErr.Status) assert.Equal(t, []restclient.OperationDiagnostic{{ - Code: "InvalidField", Details: []restclient.OperationDiagnostic{{Code: "service_error"}}, + Code: "InvalidField", Message: "[redacted value]", + Details: []restclient.OperationDiagnostic{{Code: "service_error"}}, }}, operationErr.Details) var responseErr *azcore.ResponseError require.ErrorAs(t, err, &responseErr) @@ -630,6 +634,94 @@ func TestInlineTerminalOperationErrors(t *testing.T) { } } +func TestOperationInnerErrorsRetainSafeCodes(t *testing.T) { + for _, inline := range []bool{false, true} { + t.Run(fmt.Sprint(inline), func(t *testing.T) { + client, _ := testClient(t, restclient.APIVersionPreview, func(w http.ResponseWriter, r *http.Request) { + if !inline && r.Method == http.MethodPost { + w.Header().Set("Operation-Location", + "/contentunderstanding/analyzerResults/op?api-version="+restclient.APIVersionPreview) + w.WriteHeader(http.StatusAccepted) + _, _ = io.WriteString(w, `{"status":"Running"}`) + return + } + _, _ = io.WriteString(w, `{"status":"Failed","error":{"code":"InvalidRequest","message":"Invalid request.",`+ + `"details":[{"code":"InvalidField","innererror":{"code":"NestedDetail",`+ + `"message":"content: private detail"}}],`+ + `"innererror":{"code":"ModelNotSupported","message":"Provided model 'gpt-6-luna' is unsupported.",`+ + `"innererror":{"code":"private code","target":"private target"}}}}`) + }) + request := restclient.AnalyzeRequest{Inputs: []restclient.AnalysisInput{{URL: "https://example.test/a"}}} + var err error + if inline { + _, err = client.AnalyzeInline(t.Context(), "a", request) + } else { + poller, beginErr := client.BeginAnalyze(t.Context(), "a", request) + require.NoError(t, beginErr) + _, err = poller.PollUntilDone(t.Context(), &runtime.PollUntilDoneOptions{Frequency: time.Millisecond}) + } + var operationErr *restclient.OperationError + require.ErrorAs(t, err, &operationErr) + assert.Equal(t, "InvalidRequest", operationErr.Code) + assert.Equal(t, "Invalid request.", operationErr.Message) + assert.Equal(t, []restclient.OperationDiagnostic{ + {Code: "ModelNotSupported", Message: "Provided model gpt-6-luna is unsupported.", + Details: []restclient.OperationDiagnostic{{Code: "service_error"}}}, + {Code: "InvalidField", Details: []restclient.OperationDiagnostic{{ + Code: "NestedDetail", Message: "[redacted value]", + }}}, + }, operationErr.Details) + body, marshalErr := json.Marshal(operationErr) + require.NoError(t, marshalErr) + assert.NotContains(t, string(body), "private") + assert.NotContains(t, err.Error(), "private") + }) + } +} + +func TestOperationDiagnosticLimits(t *testing.T) { + for _, deep := range []bool{false, true} { + t.Run(fmt.Sprint(deep), func(t *testing.T) { + problem := map[string]any{"code": "InvalidRequest"} + if deep { + current := problem + for range 100 { + next := map[string]any{"code": "NestedFailure", "message": "content: private"} + current["innererror"] = next + current = next + } + } else { + var details []map[string]string + for range 100 { + details = append(details, map[string]string{"code": "NestedFailure", "message": "content: private"}) + } + problem["details"] = details + problem["innererror"] = map[string]string{ + "code": "ModelNotSupported", "message": "Provided model 'gpt-6-luna' is unsupported.", + } + } + payload, err := json.Marshal(map[string]any{"status": "Failed", "error": problem}) + require.NoError(t, err) + client, _ := testClient(t, restclient.APIVersionPreview, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(payload) + }) + _, err = client.AnalyzeInline(t.Context(), "a", restclient.AnalyzeRequest{ + Inputs: []restclient.AnalysisInput{{URL: "https://example.test/a"}}, + }) + var operationErr *restclient.OperationError + require.ErrorAs(t, err, &operationErr) + body, marshalErr := json.Marshal(operationErr.Details) + require.NoError(t, marshalErr) + assert.Contains(t, string(body), "DiagnosticsTruncated") + if !deep { + assert.Contains(t, string(body), "model gpt-6-luna is unsupported.") + } + assert.NotContains(t, string(body), "private") + assert.LessOrEqual(t, strings.Count(string(body), `"Code"`)+strings.Count(string(body), `"code"`), 40) + }) + } +} + func TestContinuationOriginAndCancellation(t *testing.T) { client, _ := testClient(t, "", func(w http.ResponseWriter, r *http.Request) { _, _ = io.WriteString(w, `{"value":[],"nextLink":"https://other.test/next"}`) diff --git a/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/tests/unit/diagnostics_test.go b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/tests/unit/diagnostics_test.go new file mode 100644 index 00000000000..dbec7390263 --- /dev/null +++ b/cli/azd/extensions/azure.ai.content-understanding/cu-rest-client/tests/unit/diagnostics_test.go @@ -0,0 +1,47 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package curestclient_test + +import ( + "strings" + "testing" + + restclient "azure.ai.content-understanding/cu-rest-client" + "github.com/stretchr/testify/assert" +) + +func TestDiagnosticRedactionRetainsReasonsAndModelNames(t *testing.T) { + for _, sample := range []struct { + message, expected string + }{ + {`Provided model 'gpt-6-luna' is unsupported.`, "model gpt-6-luna is unsupported."}, + {`Provided model "gpt-6-luna" is unsupported.`, "model gpt-6-luna is unsupported."}, + {"Analyzer's configuration conflicts with the resource's mapping.", "resource's mapping."}, + {"The processing region is unavailable.", "processing region is unavailable."}, + {"Invalid value 'private-document'.", "Invalid value [redacted value]."}, + {"Invalid value 'private-document", "Invalid value [redacted value]"}, + {"Read https://user:private-password@example.test/private-file?sig=private-signature", "[redacted URL]"}, + {"Authorization: Bearer private-token", "Authorization: [redacted]"}, + {"api_key=private-key", "[redacted value]"}, + {"content: private-document", "[redacted value]"}, + {"path: /private-document.pdf", "[redacted value]"}, + {"File C:\\private-folder\\private-file.pdf is unreadable.", "[redacted path]"}, + {"Bad fieldSchema.fields.private-key.type", "fieldSchema.fields.[redacted]"}, + {"Bad\x1b[31m\u202e document.", "document."}, + } { + t.Run(sample.message, func(t *testing.T) { + safe := restclient.RedactDiagnostic(sample.message) + assert.Contains(t, safe, sample.expected) + assert.NotContains(t, safe, "private") + assert.NotContains(t, safe, "\x1b") + assert.NotContains(t, safe, "\u202e") + assert.Equal(t, safe, restclient.RedactDiagnostic(safe), "repeated boundary filtering is stable") + }) + } + for _, value := range []string{string([]byte{0xff}), strings.Repeat("x", 3000)} { + safe := restclient.RedactDiagnostic(value) + assert.Contains(t, safe, "Diagnostic omitted") + assert.LessOrEqual(t, len(safe), restclient.MaxDiagnosticBytes) + } +} diff --git a/cli/azd/extensions/azure.ai.content-understanding/docs/usage.md b/cli/azd/extensions/azure.ai.content-understanding/docs/usage.md index 6d817f306ec..dfa614651e3 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/docs/usage.md +++ b/cli/azd/extensions/azure.ai.content-understanding/docs/usage.md @@ -256,7 +256,23 @@ Long diagnostics are shortened; use `--report-file` on the original run to retai all per-input outcomes. Diagnosing a failure does not issue another analysis request. Missing-resource errors suggest checking the endpoint and available analyzers; creation conflicts suggest inspecting the existing analyzer and choosing an unused ID. -Service response bodies and authored content remain hidden. +Raw service response bodies are not printed. Diagnostic messages are filtered +to remove credentials, URLs, paths, quoted customer values, and labeled document data. + +Sanitized service messages and nested error codes appear in both the terminal diagnostic and each +failed input's report `error` string; the `cu-cli/analyze-report/v1` schema is +unchanged. This applies to HTTP errors, failed polling operations, and inline +analysis. Unknown service codes retain their diagnostic explanation rather than +being replaced with a generic failure. Missing and truncated diagnostics are +identified explicitly; deeper causes take priority over long outer messages. +For `ModelNotSupported`, the diagnostic retains the incompatible model name when +provided in the service message. Inspect `supportedModels` with +`azd ai cu analyzer show ` and the resource's `modelDeployments` +with `azd ai cu defaults show`. With resource-owner approval, map the analyzer's +model alias to a supported deployed model; defaults changes affect the whole +resource. Successful result content is not changed by diagnostic filtering. +The terminal summary includes succeeded, failed, and skipped counts and only +mentions retained result files when a result file was successfully saved. - Batch concurrency defaults to 4; `--concurrency` accepts 1-32. - More than 50 selected inputs require confirmation or explicit `--yes`. diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/analysis/analysis_test.go b/cli/azd/extensions/azure.ai.content-understanding/internal/analysis/analysis_test.go index b3ca13f6b72..a23dc11f2eb 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/analysis/analysis_test.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/analysis/analysis_test.go @@ -464,11 +464,19 @@ func TestExecutionPartialFailureSkipAndRedaction(t *testing.T) { }} result, err := Execute(t.Context(), client, plan, runOptions()) requireCode(t, err, "analysis_failed") + assert.Contains(t, err.Error(), "1 succeeded, 1 failed, 1 skipped") + assert.Contains(t, err.Error(), "Successful result files were retained.") + assert.Contains(t, err.Error(), "Input 2:") + assert.NotContains(t, err.Error(), "secret") assert.EqualValues(t, 2, calls.Load()) require.Len(t, result.Records, 3) assert.Equal(t, "succeeded", result.Records[0].Status) assert.Equal(t, "failed", result.Records[1].Status) assert.Equal(t, "skipped", result.Records[2].Status) + // #nosec G304 -- Reads only the successful output planned in this test's private directory. + saved, readErr := os.ReadFile(*result.Records[0].Output) + require.NoError(t, readErr) + assert.JSONEq(t, terminal, string(saved)) // #nosec G304 -- Reads only this test's private temporary report. body, err := os.ReadFile(reportPath) require.NoError(t, err) diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/analysis/run.go b/cli/azd/extensions/azure.ai.content-understanding/internal/analysis/run.go index 06019fe0a30..fc9691013f1 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/analysis/run.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/analysis/run.go @@ -161,6 +161,16 @@ func Execute(ctx context.Context, client cuclient.Client, plan Plan, options Run } } if summary.Counts.Failed != 0 { + var message strings.Builder + fmt.Fprintf(&message, "%d analysis input(s) failed (analyzer: %s).", summary.Counts.Failed, options.Analyzer) + for _, record := range result.Records { + if record.Status == "succeeded" && record.Output != nil { + message.WriteString(" Successful result files were retained.") + break + } + } + fmt.Fprintf(&message, "\nSummary: %d succeeded, %d failed, %d skipped.", + summary.Counts.Succeeded, summary.Counts.Failed, summary.Counts.Skipped) var details strings.Builder shown := 0 for index, record := range result.Records { @@ -181,9 +191,7 @@ func Execute(ctx context.Context, client cuclient.Client, plan Plan, options Run } return result, &cuclient.Error{ Kind: cuclient.ErrorService, Code: "analysis_failed", - Message: fmt.Sprintf( - "%d analysis input(s) failed; successful result files were retained.%s", - summary.Counts.Failed, details.String()), + Message: message.String() + details.String(), Suggestion: "Correct the reported input or analyzer problem before retrying. " + "Use --report-file on future runs to save all per-input outcomes.", } diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/cmd/analysis_playback_test.go b/cli/azd/extensions/azure.ai.content-understanding/internal/cmd/analysis_playback_test.go index 67ac5853798..3da9868f6f0 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/cmd/analysis_playback_test.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/cmd/analysis_playback_test.go @@ -127,6 +127,104 @@ func TestRecordedAnalysisDefaultLLMInput(t *testing.T) { }) } +func TestAnalyzeServiceFailureReachesTerminalAndReport(t *testing.T) { + for _, sample := range []struct { + name, body string + expected []string + }{ + { + "model", `{"status":"Failed","error":{"code":"InvalidRequest","message":"Invalid request.", + "innererror":{"code":"ModelNotSupported", + "message":"Provided model 'gpt-6-luna' is not supported by this analyzer.\n` + + `https://user:private-password@example.test/doc?sig=private-signature\ncontent: private-document", + "target":"private-target"}}}`, + []string{"InvalidRequest", "Invalid request.", "ModelNotSupported", "gpt-6-luna", + "not supported by this analyzer", "supportedModels", "modelDeployments"}, + }, + { + "unknown-code", `{"status":"Failed","error":{"code":"InvalidRequest","message":"Invalid request.", + "innererror":{"code":"FutureServiceFailure", + "message":"Processing region is unavailable. Select another supported region."}}}`, + []string{"FutureServiceFailure", "Processing region is unavailable. Select another supported region."}, + }, + { + "long-outer-with-many-details", + `{"status":"Failed","error":{"code":"InvalidRequest","message":"` + + strings.Repeat("Long outer explanation. ", 80) + `","innererror":{"code":"ModelNotSupported",` + + `"message":"Provided model 'gpt-6-luna' is not supported by this analyzer."},"details":[` + + strings.Repeat(`{"code":"SiblingFailure","message":"Secondary diagnostic."},`, 40) + `{}]}}`, + []string{"ModelNotSupported", "gpt-6-luna", "not supported by this analyzer", + "Additional service diagnostics omitted.", "supportedModels", "modelDeployments"}, + }, + { + "missing-details", `{"status":"Failed"}`, + []string{"The analysis operation failed.", "No further service diagnostic was provided."}, + }, + } { + for _, flags := range [][]string{nil, {"--pretty"}, {"--output", "json"}, {"--output", "none"}} { + t.Run(sample.name+"/"+strings.Join(flags, " "), func(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + fixture := loadRecordedFixture(t, "analyze_single") + fixture.Interactions[len(fixture.Interactions)-1].Response.Body = json.RawMessage(sample.body) + transport := &recordedTransport{fixture: fixture, binaryBody: []byte(recordedBinaryInput)} + root := newRecordedCommand(t, transport, "prebuilt-layout") + inputDir := filepath.Join(testutil.TempDir(t), "private") + require.NoError(t, os.Mkdir(inputDir, 0o700)) + input := filepath.Join(inputDir, "invoice.pdf") + require.NoError(t, os.WriteFile(input, []byte(recordedBinaryInput), 0o600)) + reportPath := filepath.Join(testutil.TempDir(t), "report.json") + args := append([]string{"analyze", input, "--report-file", reportPath, + "--endpoint", "https://sanitized.services.ai.azure.com", "--analyzer", "prebuilt-layout", + "--api-version", cuclient.DefaultAPIVersion, "--no-prompt"}, flags...) + root.SetArgs(args) + var stdout, stderr bytes.Buffer + root.SetOut(&stdout) + root.SetErr(&stderr) + err := root.ExecuteContext(t.Context()) + require.Error(t, err) + require.NoError(t, transport.complete()) + assert.Empty(t, stdout.String()) + assert.Empty(t, stderr.String()) + expectedMessages := append([]string{"Input 1:", "0 succeeded, 1 failed, 0 skipped"}, sample.expected...) + for _, expected := range expectedMessages { + assert.Contains(t, err.Error(), expected) + } + assert.NotContains(t, err.Error(), "successful result files were retained") + for _, secret := range []string{ + "preview-user", "preview-password", "private-signature", "private-document", "private-target", + } { + assert.NotContains(t, err.Error(), secret) + } + // #nosec G304 -- Reads only this test's private temporary report. + body, readErr := os.ReadFile(reportPath) + require.NoError(t, readErr) + var saved struct { + Schema string `json:"schema"` + Results []struct { + Input string `json:"input"` + Status string `json:"status"` + Error string `json:"error"` + } `json:"results"` + } + require.NoError(t, json.Unmarshal(body, &saved)) + assert.Equal(t, "cu-cli/analyze-report/v1", saved.Schema) + require.Len(t, saved.Results, 1) + assert.Equal(t, input, saved.Results[0].Input) + assert.Equal(t, "failed", saved.Results[0].Status) + for _, expected := range sample.expected { + assert.Contains(t, saved.Results[0].Error, expected) + } + for _, secret := range []string{ + "preview-user", "preview-password", "private-signature", "private-document", "private-target", + } { + assert.NotContains(t, saved.Results[0].Error, secret) + } + }) + }) + } + } +} + func TestRecordedAnalysisSyntheticURLContract(t *testing.T) { synctest.Test(t, func(t *testing.T) { fixture := loadRecordedFixture(t, "analyze_single") diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/cmd/analyzer_testing_playback_test.go b/cli/azd/extensions/azure.ai.content-understanding/internal/cmd/analyzer_testing_playback_test.go index e6c4f0779f8..0e3b411173a 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/cmd/analyzer_testing_playback_test.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/cmd/analyzer_testing_playback_test.go @@ -77,7 +77,8 @@ func TestRecordedAnalyzerFieldReport(t *testing.T) { } else { envelope["status"] = json.RawMessage(`"Failed"`) envelope["error"] = json.RawMessage(`{ - "code":"InvalidRequest","message":"synthetic-private-message-not-for-field-report" + "code":"InvalidRequest","message":"Unsupported document format.\n` + + `content: synthetic-private-message-not-for-field-report" }`) } body, err := json.Marshal(envelope) @@ -114,8 +115,12 @@ func TestRecordedAnalyzerFieldReport(t *testing.T) { }, "samples": []any{map[string]any{ "input": "sample_invoice.pdf", "status": "error", - "error": "The analysis operation failed. Service code: InvalidRequest. " + - "Check the analyzer configuration, input, and dependencies before retrying.", + "error": "The analysis operation failed. Service code: InvalidRequest.\n" + + "InvalidRequest: Unsupported document format.\n[redacted value] " + + "Correct the reported service problem before retrying. " + + "Use azd ai cu analyzer show " + + "to inspect the analyzer configuration. " + + "If the cause is unclear, report the error codes to support.", "fields": map[string]any{}, }}, }) diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/cmd/diagnostics_schema_playback_test.go b/cli/azd/extensions/azure.ai.content-understanding/internal/cmd/diagnostics_schema_playback_test.go index 080598c7184..566bcd24e08 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/cmd/diagnostics_schema_playback_test.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/cmd/diagnostics_schema_playback_test.go @@ -66,7 +66,9 @@ func TestRecordedDoctorCommands(t *testing.T) { code = "ResourceNotFound" } body, err := json.Marshal(map[string]any{ - "error": map[string]any{"code": code, "message": "synthetic-private-doctor-detail"}, + "error": map[string]any{ + "code": code, "message": "content: synthetic-private-doctor-detail", + }, }) require.NoError(t, err) fixture.Interactions[0].Response.Body = body diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/analysis_test.go b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/analysis_test.go index ac46eeeb167..14234d47c0e 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/analysis_test.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/analysis_test.go @@ -273,7 +273,7 @@ func TestAnalysisSubmissionsAreNotReplayed(t *testing.T) { t.Run(fmt.Sprintf("%s/%s/binary_%t/%d", version, mode, binary, status), func(t *testing.T) { request, step, reader := analysisSubmission(mode, binary) step.status = status - step.response = `{"error":{"code":"ServiceCode","message":"private-service-error"}}` + step.response = `{"error":{"code":"ServiceCode","message":"content: private-service-error"}}` step.headers = http.Header{ "Retry-After": {"1"}, "Www-Authenticate": {`Bearer error="insufficient_claims", claims="private-claims"`}, @@ -420,8 +420,8 @@ func TestAnalysisTerminalFailures(t *testing.T) { for _, state := range []string{"Failed", "Canceled", "cancelled"} { t.Run(string(mode)+"/"+state, func(t *testing.T) { request, submit, _ := analysisSubmission(mode, false) - body := `{"status":"` + state + `","error":{"code":"OperationCode","message":"private-token", - "target":"private-token","details":[{"code":"InvalidField","message":"private-token", + body := `{"status":"` + state + `","error":{"code":"OperationCode","message":"token: private-token", + "target":"private-token","details":[{"code":"InvalidField","message":"token: private-token", "target":"private-token"}]}}` steps := []wireStep{submit} if mode == AnalysisModeAsync { diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/analyzers_test.go b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/analyzers_test.go index 151c6a5b9b5..f40c9f78ad3 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/analyzers_test.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/analyzers_test.go @@ -401,7 +401,7 @@ func TestAnalyzerHTTPFailures(t *testing.T) { client := newWireClient(t, DefaultAPIVersion, wireStep{ method: http.MethodPut, path: "/analyzers/custom", query: url.Values{"allowReplace": {"false"}}, body: `{}`, contentType: "application/json", status: test.status, - response: `{"error":{"code":"ServiceCode","message":"private-token"}}`, + response: `{"error":{"code":"ServiceCode","message":"token: private-token"}}`, headers: http.Header{"Retry-After": {"86400"}}, }) result, err := client.CreateAnalyzer(t.Context(), CreateAnalyzerRequest{ diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/pipeline.go b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/pipeline.go index 9482cb75237..7457e10ebfa 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/pipeline.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/pipeline.go @@ -61,7 +61,7 @@ func (t *boundedErrorTransport) Do(request *http.Request) (*http.Response, error return response, nil } -func serviceResponseError(statusCode int, code, retryAfterHeader string) error { +func serviceResponseError(statusCode int, code, retryAfterHeader string) *Error { if !serviceCodePattern.MatchString(code) { code = fmt.Sprintf("http_%d", statusCode) } diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/pipeline_test.go b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/pipeline_test.go index 3f773f7cb27..3bc748353b2 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/pipeline_test.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/pipeline_test.go @@ -6,6 +6,7 @@ package cuclient import ( "bytes" "context" + "encoding/json" "errors" "fmt" "io" @@ -382,7 +383,8 @@ func (b *measuredBody) Close() error { b.closed = true; return nil } func TestBoundedErrorBodyAndRedaction(t *testing.T) { for _, payload := range []string{ - `{"error":{"code":"Forbidden","message":"private-body-token","details":[{"message":"private-detail"}]}}`, + `{"error":{"code":"Forbidden","message":"token: private-body-token",` + + `"details":[{"message":"content: private-detail"}]}}`, `private malformed response`, strings.Repeat("private-body-token", maxErrorBodyBytes), `{"error":{"code":"invalid code private-token"}}`, } { @@ -420,7 +422,7 @@ func TestActionableServiceFailureDiagnostics(t *testing.T) { client := newRESTTransport(t, DefaultAPIVersion, func(*http.Request) (*http.Response, error) { calls++ return response(test.status, fmt.Sprintf( - `{"error":{"code":%q,"message":"private response body"}}`, test.code)), nil + `{"error":{"code":%q,"message":"content: private response body"}}`, test.code)), nil }) var err error if test.status == http.StatusConflict { @@ -445,11 +447,12 @@ func TestActionableServiceFailureDiagnostics(t *testing.T) { } } -func TestOperationFailureDiagnosticsDoNotExposeServiceMessages(t *testing.T) { +func TestOperationFailureDiagnosticsPreserveSafeMessages(t *testing.T) { for _, code := range []string{"InvalidRequest", "invalid private code"} { domain := operationError("analysis", "Failed", &wireDiagnostic{ - Code: code, Message: "private authored content", + Code: code, Message: "The document format is unsupported.\ncontent: private authored content", }) + require.Contains(t, domain.Message, "The document format is unsupported.") require.NotContains(t, domain.Message, "private") if code == "InvalidRequest" { require.Contains(t, domain.Message, "Service code: InvalidRequest") @@ -458,6 +461,155 @@ func TestOperationFailureDiagnosticsDoNotExposeServiceMessages(t *testing.T) { } } } + +func TestHTTPServiceFailuresPreserveSanitizedRootCause(t *testing.T) { + for _, phase := range []string{"submission", "poll", "inline"} { + for _, code := range []string{"ModelNotSupported", "FutureServiceFailure"} { + t.Run(phase+"/"+code, func(t *testing.T) { + mode := AnalysisModeAsync + if phase == "inline" { + mode = AnalysisModeInline + } + request, submit, _ := analysisSubmission(mode, false) + body := `{"error":{"code":"InvalidRequest","message":"Invalid request.",` + + `"innererror":{"code":"` + code + `","message":"Provided model 'gpt-6-luna' cannot be used.\n` + + `Authorization: Bearer private-token\ncontent: private-document"}}}` + steps := []wireStep{submit} + if phase == "poll" { + steps = append(steps, wireStep{ + method: http.MethodGet, path: "/analyzerResults/op1", status: http.StatusBadRequest, response: body, + }) + } else { + steps[0].status, steps[0].response = http.StatusBadRequest, body + } + client := newWireClient(t, PreviewAPIVersion, steps...) + _, err := client.Analyze(t.Context(), request) + var domain *Error + require.ErrorAs(t, err, &domain) + require.Equal(t, ErrorValidation, domain.Kind) + require.Equal(t, http.StatusBadRequest, domain.StatusCode) + require.Equal(t, code, domain.Code) + require.Contains(t, domain.Message, "InvalidRequest") + require.Contains(t, domain.Message, "Invalid request.") + require.Contains(t, domain.Message, "gpt-6-luna") + require.Contains(t, domain.Message, "cannot be used") + require.NotContains(t, domain.Message, "private") + require.NotNil(t, domain.Details) + if code == "ModelNotSupported" { + require.Contains(t, domain.Suggestion, "supportedModels") + require.Contains(t, domain.Suggestion, "modelDeployments") + } + }) + } + } +} + +func TestOperationFailureIncludesNestedCauseAndGuidance(t *testing.T) { + for _, code := range []string{"InvalidRequest", "ModelNotSupported", "invalid private code"} { + domain := operationError("analysis", "Failed", &wireDiagnostic{ + Code: code, Message: "Invalid request.\ncontent: private service message", + Details: []wireDiagnostic{{ + Code: "NestedFailure", Details: []wireDiagnostic{{ + Code: "ModelNotSupported", + Message: "Provided model 'gpt-6-luna' is not supported by this analyzer.\n" + + "content: private document content", + }}, + }}, + }) + require.Contains(t, domain.Message, "ModelNotSupported") + require.Contains(t, domain.Message, "gpt-6-luna") + require.Contains(t, domain.Message, "not supported by this analyzer") + require.Contains(t, domain.Suggestion, "supportedModels") + require.Contains(t, domain.Suggestion, "modelDeployments") + require.Contains(t, domain.Suggestion, "resource-wide") + require.NotContains(t, domain.Message+domain.Suggestion, "private") + } + var details []wireDiagnostic + for index := range 20 { + details = append(details, wireDiagnostic{Code: fmt.Sprintf("%s%d", strings.Repeat("x", 120), index)}) + } + details = append(details, wireDiagnostic{Code: "ModelNotSupported"}) + domain := operationError("analysis", "Failed", &wireDiagnostic{Code: "InvalidRequest", Details: details}) + require.Contains(t, domain.Message, "ModelNotSupported") + require.Contains(t, domain.Message, "Additional service diagnostics omitted.") + require.Contains(t, domain.Suggestion, "supportedModels") + require.Less(t, len(domain.Message+domain.Suggestion), 1400) +} + +func TestOperationFailureWithoutDiagnosticDetails(t *testing.T) { + for _, problem := range []*wireDiagnostic{nil, {}, {Code: "invalid private code"}} { + domain := operationError("analysis", "Failed", problem) + require.Equal(t, "analysis_operation_failed", domain.Code) + require.Equal(t, "The analysis operation failed. No further service diagnostic was provided.", domain.Message) + require.NotEmpty(t, domain.Suggestion) + require.NotContains(t, domain.Message+domain.Suggestion, "private") + require.NotContains(t, domain.Suggestion, "modelDeployments") + } +} + +func TestOperationDiagnosticPrioritiesAndLimits(t *testing.T) { + problem := &wireDiagnostic{ + Code: "InvalidRequest", Message: strings.Repeat("Long outer explanation. ", 80), + Details: []wireDiagnostic{{ + Code: "ModelNotSupported", Message: "Provided model 'gpt-6-luna' is not supported by this analyzer.", + }}, + } + domain := operationError("analysis", "Failed", problem) + require.Contains(t, domain.Message, "model gpt-6-luna is not supported") + require.Contains(t, domain.Message, "[diagnostic truncated]") + require.Less(t, strings.Index(domain.Message, "model gpt-6-luna"), strings.Index(domain.Message, "Long outer")) + + current := problem + for range 12 { + current.Details = []wireDiagnostic{{Code: "NestedFailure"}} + current = ¤t.Details[0] + } + domain = operationError("analysis", "Failed", problem) + require.Contains(t, domain.Message, "Additional service diagnostics omitted.") +} + +func TestReflectedBearerCredentialsAreRedacted(t *testing.T) { + for _, mode := range []string{"http", "poll", "inline"} { + t.Run(mode, func(t *testing.T) { + calls := 0 + client := newRESTTransport(t, PreviewAPIVersion, func(request *http.Request) (*http.Response, error) { + calls++ + if mode == "poll" && calls == 1 { + result := response(http.StatusAccepted, `{"status":"Running"}`) + result.Header.Set("Operation-Location", "/contentunderstanding/analyzerResults/op") + return result, nil + } + credential := strings.TrimPrefix(request.Header.Get("Authorization"), "Bearer ") + require.NotEmpty(t, credential) + body := fmt.Sprintf( + `{"status":"Failed","error":{"code":%q,"message":%q,"innererror":{"code":"InvalidModel","message":%q}}}`, + credential, "Unsupported model. "+credential, "Bearer "+credential, + ) + status := http.StatusOK + if mode == "http" { + status = http.StatusBadRequest + } + // Deliberately omit Response.Request, as custom transports may do. + return response(status, body), nil + }) + request := AnalyzeRequest{ + AnalyzerID: "custom", Mode: AnalysisModeAsync, Input: Input{URL: "https://example.test/a"}, + } + if mode == "inline" { + request.Mode = AnalysisModeInline + } + _, err := client.Analyze(t.Context(), request) + var domain *Error + require.ErrorAs(t, err, &domain) + require.Contains(t, domain.Message, "Unsupported model.") + require.NotContains(t, domain.Message+domain.Code, "private-access-token") + payload, marshalErr := json.Marshal(domain.Details) + require.NoError(t, marshalErr) + require.NotContains(t, string(payload), "private-access-token") + }) + } +} + func TestResponseErrorCategories(t *testing.T) { for _, test := range []struct { status int diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/polling.go b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/polling.go index caf04b9bb53..cd3dc19f71d 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/polling.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/polling.go @@ -5,9 +5,9 @@ package cuclient import ( "fmt" - "net/url" "strings" + "unicode/utf8" ) func serviceLink(endpoint, apiVersion, value, currentPath string) (string, url.Values, error) { @@ -45,8 +45,10 @@ func serviceLink(endpoint, apiVersion, value, currentPath string) (string, url.V func operationError(operation, status string, problem *wireDiagnostic) *Error { result := &Error{ Kind: ErrorService, Code: operation + "_operation_failed", - Message: fmt.Sprintf("The %s operation failed.", operation), - Suggestion: "Check the analyzer configuration, input, and dependencies before retrying.", + Message: fmt.Sprintf("The %s operation failed.", operation), + Suggestion: "Correct the reported service problem before retrying. " + + "Use azd ai cu analyzer show to inspect the analyzer configuration. " + + "If the cause is unclear, report the error codes to support.", } if strings.EqualFold(status, "canceled") || strings.EqualFold(status, "cancelled") { result.Kind = ErrorCancelled @@ -58,7 +60,83 @@ func operationError(operation, status string, problem *wireDiagnostic) *Error { result.Code = problem.Code result.Message += fmt.Sprintf(" Service code: %s.", problem.Code) } - result.Details = diagnostics(problem.Details, true) } + appendServiceDiagnostics(result, problem) return result } + +func appendServiceDiagnostics(result *Error, problem *wireDiagnostic) { + var values []Diagnostic + remaining := 32 + omitted := false + var visit func(Diagnostic, int) + visit = func(value Diagnostic, depth int) { + if remaining == 0 || depth == 8 { + omitted = true + return + } + remaining-- + for _, child := range value.Details { + visit(child, depth+1) + } + values = append(values, value) + } + if problem != nil { + result.Details = diagnostics(problem.Details, true) + visit(Diagnostic{ + Code: problem.Code, Message: RedactDiagnostic(problem.Message), Details: result.Details, + }, 0) + } + modelNotSupported := result.Code == "ModelNotSupported" + modelMessage := false + shown := false + // Prefer the deepest causes so a long outer error cannot hide the actionable reason. + for _, value := range values { + if value.Code == "ModelNotSupported" { + modelNotSupported = true + } + if value.Message == "" && (value.Code == result.Code || !serviceCodePattern.MatchString(value.Code)) { + continue + } + code := value.Code + if !serviceCodePattern.MatchString(code) { + code = "service_error" + } + line := "\n" + code + if value.Message != "" { + message := value.Message + if len(message) > 500 { + end := 470 + for !utf8.RuneStart(message[end]) { + end-- + } + message = message[:end] + "... [diagnostic truncated]" + } + line += ": " + message + } + if len(result.Message)+len(line) > 900 { + omitted = true + continue + } + result.Message += line + shown = true + if value.Code == "ModelNotSupported" && value.Message != "" { + modelMessage = true + } + } + if omitted { + result.Message += "\nAdditional service diagnostics omitted." + } + if !shown && !omitted { + result.Message += " No further service diagnostic was provided." + } + if modelNotSupported && result.Kind != ErrorCancelled { + if !modelMessage { + result.Message += " ModelNotSupported: The configured model is not supported by this analyzer." + } + result.Suggestion = "Inspect supportedModels with azd ai cu analyzer show " + + "and resource modelDeployments with azd ai cu defaults show. " + + "With resource-owner approval, map the analyzer model alias to a supported deployed model; " + + "defaults changes are resource-wide." + } +} diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/polling_test.go b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/polling_test.go index 667108d6a08..9e29430ffd9 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/polling_test.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/polling_test.go @@ -162,9 +162,9 @@ func TestAnalyzerPollingFailuresAndMalformedResponses(t *testing.T) { code string kind ErrorKind }{ - {`{"status":"Failed","error":{"code":"InvalidRequest","message":"private-token","target":"private-token", - "details":[{"code":"InvalidField","message":"private-token","target":"private-token", - "details":[{"code":"private token","message":"private-token"}]}]}}`, "InvalidRequest", ErrorService}, + {`{"status":"Failed","error":{"code":"InvalidRequest","message":"token: private-token","target":"private-token", + "details":[{"code":"InvalidField","message":"token: private-token","target":"private-token", + "details":[{"code":"private token","message":"token: private-token"}]}]}}`, "InvalidRequest", ErrorService}, {`{"status":"Failed","error":{"code":"private token"}}`, "analyzer_operation_failed", ErrorService}, {`{"status":"Failed"}`, "analyzer_operation_failed", ErrorService}, {`{"status":"Canceled"}`, "analyzer_operation_canceled", ErrorCancelled}, diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/rest.go b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/rest.go index e20dd6e3a75..c72145adc3b 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/rest.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/rest.go @@ -326,7 +326,7 @@ func parseUsage(raw json.RawMessage) (*Usage, error) { func restOperationDomainError(err error, operation string) error { if failure, ok := errors.AsType[*restclient.OperationError](err); ok { domain := operationError(operation, failure.Status, &wireDiagnostic{ - Code: failure.Code, Details: operationDiagnosticDetails(failure.Details), + Code: failure.Code, Message: failure.Message, Details: operationDiagnosticDetails(failure.Details), }) domain.StatusCode = failure.ResponseError.StatusCode return domain @@ -340,7 +340,9 @@ func operationDiagnosticDetails(values []restclient.OperationDiagnostic) []wireD } result := make([]wireDiagnostic, len(values)) for i, value := range values { - result[i] = wireDiagnostic{Code: value.Code, Details: operationDiagnosticDetails(value.Details)} + result[i] = wireDiagnostic{ + Code: value.Code, Message: value.Message, Details: operationDiagnosticDetails(value.Details), + } } return result } @@ -407,11 +409,25 @@ func restDomainError(err error) error { if response.RawResponse != nil { retryAfterHeader = response.RawResponse.Header.Get("Retry-After") } - return serviceResponseError(response.StatusCode, response.ErrorCode, retryAfterHeader) + domain := serviceResponseError(response.StatusCode, response.ErrorCode, retryAfterHeader) + if service, ok := errors.AsType[*restclient.ServiceError](err); ok { + appendServiceDiagnostics(domain, &wireDiagnostic{ + Code: service.Diagnostic.Code, Message: service.Diagnostic.Message, + Details: operationDiagnosticDetails(service.Diagnostic.Details), + }) + } else { + appendServiceDiagnostics(domain, nil) + } + return domain } return transportError(err) } +// RedactDiagnostic shares diagnostic safety rules with the REST client and command boundary. +func RedactDiagnostic(message string) string { + return restclient.RedactDiagnostic(message) +} + type safeTokenCredential struct { inner azcore.TokenCredential } diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/rest_test.go b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/rest_test.go index 32f0e69e1d9..51fb75a394d 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/rest_test.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/rest_test.go @@ -332,7 +332,7 @@ func TestRESTTransportSafeErrorsAndRetryPolicy(t *testing.T) { t.Run("nested service code is safe", func(t *testing.T) { client := newRESTTransport(t, DefaultAPIVersion, func(request *http.Request) (*http.Response, error) { response := restResponse(request, http.StatusBadRequest, - `{"error":{"code":"InvalidRequest","message":"private value",`+ + `{"error":{"code":"InvalidRequest","message":"content: private value",`+ `"innererror":{"code":"DefaultsNotSet"}}}`) response.Header.Set("x-ms-error-code", "InvalidRequest") return response, nil @@ -444,9 +444,10 @@ func TestRESTTransportTerminalOperationErrors(t *testing.T) { response.Header.Set("Operation-Location", location) return response, nil } - payload := fmt.Sprintf(`{"status":%q,"error":{"code":%q,"message":"private value",`+ - `"details":[{"code":"InvalidField","message":"private value","target":"private target",`+ - `"details":[{"code":"private value","message":"private value"}]}]}}`, + payload := fmt.Sprintf(`{"status":%q,"error":{"code":%q,"message":"content: private value",`+ + `"details":[{"code":"InvalidField","message":"content: private value",`+ + `"target":"private target",`+ + `"details":[{"code":"private value","message":"content: private value"}]}]}}`, tc.status, tc.code) status := http.StatusOK if mode == "analysis" && !polled { @@ -497,8 +498,8 @@ func TestRESTTransportTerminalOperationErrors(t *testing.T) { assert.NotContains(t, domain.Message, "HTTP 200") assert.NotContains(t, err.Error(), "private value") assert.Equal(t, []Diagnostic{{ - Code: "InvalidField", Message: "The service reported an operation error.", - Details: []Diagnostic{{Code: "service_error", Message: "The service reported an operation error."}}, + Code: "InvalidField", Message: "[redacted value]", + Details: []Diagnostic{{Code: "service_error", Message: "[redacted value]"}}, }}, domain.Details) expectedCalls := 1 if polled { diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/wire.go b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/wire.go index 87d8971542b..756eb8e4a8b 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/wire.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/cuclient/wire.go @@ -40,7 +40,7 @@ func diagnostics(values []wireDiagnostic, redact bool) []Diagnostic { if !serviceCodePattern.MatchString(result[i].Code) { result[i].Code = "service_error" } - result[i].Message = "The service reported an operation error." + result[i].Message = RedactDiagnostic(value.Message) result[i].Target = "" } } diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/exterrors/redact.go b/cli/azd/extensions/azure.ai.content-understanding/internal/exterrors/redact.go index 54ac5348588..35c52094e3e 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/exterrors/redact.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/exterrors/redact.go @@ -6,9 +6,8 @@ package exterrors import ( "errors" "regexp" - "strings" - "unicode" - "unicode/utf8" + + "azure.ai.content-understanding/internal/cuclient" "github.com/azure/azure-dev/cli/azd/pkg/azdext" "google.golang.org/grpc/status" @@ -16,46 +15,12 @@ import ( const MaxDiagnosticBytes = 2048 -var ( - authorization = regexp.MustCompile(`(?im)\b(?:proxy-)?authorization\s*[:=]\s*[^\r\n]*`) - secretValue = regexp.MustCompile( - `(?im)\b(?:[a-z_]*token|api[-_]?key|ocp-apim-subscription-key|accountkey|client_secret|` + - `password|secret|sas|sig|schema|content|text|payload|body|path)\s*[:=]\s*[^\r\n]*`) - diagnosticURL = regexp.MustCompile(`(?i)\b[a-z][a-z0-9+.-]*://[^\s"'<>]+`) - windowsPath = regexp.MustCompile(`(?i)(?:[a-z]:[\\/]|\\\\)[^\s"'<>]+`) - posixPath = regexp.MustCompile(`(^|[\s"'=(])(?:/|~/|\.\.?/)[^\s"'<>]+`) - quotedValue = regexp.MustCompile(`"[^"\r\n]*"|'[^'\r\n]*'`) - fieldPath = regexp.MustCompile(`\b(fieldSchema\.fields|fields|properties)\.[^\s:]+`) - diagnosticCode = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$`) -) +var diagnosticCode = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]{0,127}$`) // Redact is for diagnostics only, never command results or service payloads. // Unknown unstructured errors must still use FromError's fixed fallback message. func Redact(message string) string { - if len(message) > 64*1024 || !utf8.ValidString(message) { - return "Diagnostic omitted because it exceeds the safe text limit." - } - if start := strings.IndexByte(message, '{'); start >= 0 { - message = message[:start] + "[redacted payload]" - } - message = authorization.ReplaceAllString(message, "Authorization: [redacted]") - message = secretValue.ReplaceAllString(message, "[redacted value]") - message = diagnosticURL.ReplaceAllString(message, "[redacted URL]") - message = windowsPath.ReplaceAllString(message, "[redacted path]") - message = posixPath.ReplaceAllString(message, "${1}[redacted path]") - message = quotedValue.ReplaceAllString(message, "[redacted value]") - message = fieldPath.ReplaceAllString(message, "${1}.[redacted]") - message = strings.Map(func(r rune) rune { - if unicode.IsControl(r) && r != '\n' && r != '\t' { - return -1 - } - return r - }, message) - message = strings.TrimSpace(message) - if len(message) > MaxDiagnosticBytes { - return "Diagnostic omitted because it exceeds the safe text limit." - } - return message + return cuclient.RedactDiagnostic(message) } func safeCode(value string) string { diff --git a/cli/azd/extensions/azure.ai.content-understanding/internal/exterrors/redact_test.go b/cli/azd/extensions/azure.ai.content-understanding/internal/exterrors/redact_test.go index f9fe4f8cbb9..b9defefc157 100644 --- a/cli/azd/extensions/azure.ai.content-understanding/internal/exterrors/redact_test.go +++ b/cli/azd/extensions/azure.ai.content-understanding/internal/exterrors/redact_test.go @@ -76,6 +76,21 @@ func TestRedactionAtStructuredErrorBoundary(t *testing.T) { } } +func TestModelDiagnosticSurvivesRepeatedRedaction(t *testing.T) { + message := "Provided model 'gpt-6-luna' is not supported by this analyzer.\n" + + "Analyzer's configuration is incompatible with the resource's model mapping.\n" + + "Invalid value 'private-document'.\n" + + "https://user:private-password@example.test/private-file?sig=private-signature\n" + + "Authorization: Bearer private-token" + for range 4 { + message = Redact(message) + assert.Contains(t, message, "gpt-6-luna") + assert.Contains(t, message, "is not supported by this analyzer") + assert.Contains(t, message, "Analyzer's configuration is incompatible with the resource's model mapping.") + assert.NotContains(t, message, "private") + } +} + func TestDiagnosticMetadataAndCancellationRemainSafe(t *testing.T) { for _, original := range []error{ &azdext.LocalError{Code: "safe_code", Message: "Safe failure.", CauseTypes: []string{"private-path"},