Egress gateway answers a credential provider fault with 502, not 403

api/credprovider/v1/credprovider.proto

@@ -25,6 +25,11 @@ option go_package = "go.ate.dev/substrate/api/credprovider/v1;credprovider";
 service CredentialProvider {
   // FetchSecret resolves a credential URI to its secret material, subject to
   // whatever authorization the provider applies to the request context.
+  //
+  // The egress gateway answers the actor with 403 for NotFound,
+  // PermissionDenied, FailedPrecondition and InvalidArgument, 503 for
+  // Unavailable, DeadlineExceeded and ResourceExhausted, and 502 for any other
+  // code.
   rpc FetchSecret(FetchSecretRequest) returns (FetchSecretResponse) {}
 }
 

cmd/atenet/internal/router/egress/credentials.go

@@ -40,20 +40,20 @@ import (
 const injectionTimeout = 4 * time.Second
 
 // mapCredentialProviderError converts a FetchSecret failure into a
-// client-facing ext_proc denial, mirroring mapEgressIdentityError: a credential
-// the provider does not hold or will not release (NotFound, PermissionDenied)
-// denies as 403 — retrying cannot succeed — while a transient provider failure
-// (Unavailable, DeadlineExceeded) fails closed as a retryable 503. Anything
-// unexpected denies rather than inviting retries of a request that cannot be
-// completed as the policy promised.
+// client-facing ext_proc denial. A credential the provider does not hold, will
+// not release or rejects (NotFound, PermissionDenied, FailedPrecondition,
+// InvalidArgument) is 403, since retrying cannot succeed. A provider that is
+// down, slow or overloaded (Unavailable, DeadlineExceeded, ResourceExhausted)
+// is a retryable 503. Any other code is a provider fault, not a refusal, and is
+// 502.
 func mapCredentialProviderError(err error) error {
 	switch status.Code(err) {
-	case codes.NotFound, codes.PermissionDenied:
+	case codes.NotFound, codes.PermissionDenied, codes.FailedPrecondition, codes.InvalidArgument:
 		return extproc.WrapReqError(envoy_type.StatusCode_Forbidden, err, deniedBody)
-	case codes.Unavailable, codes.DeadlineExceeded:
+	case codes.Unavailable, codes.DeadlineExceeded, codes.ResourceExhausted:
 		return extproc.WrapReqError(envoy_type.StatusCode_ServiceUnavailable, err, deniedBody)
 	default:
-		return extproc.WrapReqError(envoy_type.StatusCode_Forbidden, err, deniedBody)
+		return extproc.WrapReqError(envoy_type.StatusCode_BadGateway, err, deniedBody)
 	}
 }
 
@@ -185,7 +185,7 @@ func (h *Handler) providerCredential(ctx context.Context, ref resources.ActorRef
 	if err != nil {
 		slog.ErrorContext(ctx, "egress denied: unusable credential",
 			slog.Any("actor", ref), slog.String("host", dest.Hostname), slog.String("uri", uri), slog.Any("err", err))
-		return nil, extproc.WrapReqError(envoy_type.StatusCode_ServiceUnavailable, err, deniedBody)
+		return nil, extproc.WrapReqError(envoy_type.StatusCode_BadGateway, err, deniedBody)
 	}
 	return secret, nil
 }

cmd/atenet/internal/router/egress/credentials_test.go

@@ -293,18 +293,55 @@ func TestInjectionDenials(t *testing.T) {
 			want:         envoy_type.StatusCode_Forbidden,
 		},
 		{
-			// An unclassified error denies rather than inviting retries.
-			name:         "unexpected provider error denies",
-			provider:     &fakeProvider{err: errors.New("provider down")},
+			name:         "precondition failure denies as non-retryable",
+			provider:     &fakeProvider{err: status.Error(codes.FailedPrecondition, "secret not ready")},
 			providerName: injectionProviderName,
 			want:         envoy_type.StatusCode_Forbidden,
 		},
 		{
-			name:         "empty secret fails closed",
-			provider:     &fakeProvider{resp: bearerTokenResponse("")},
+			name:         "URI the provider rejects denies as non-retryable",
+			provider:     &fakeProvider{err: status.Error(codes.InvalidArgument, "malformed URI")},
+			providerName: injectionProviderName,
+			want:         envoy_type.StatusCode_Forbidden,
+		},
+		{
+			name:         "provider overloaded is retryable",
+			provider:     &fakeProvider{err: status.Error(codes.ResourceExhausted, "quota")},
 			providerName: injectionProviderName,
 			want:         envoy_type.StatusCode_ServiceUnavailable,
 		},
+		{
+			// A provider fault is not a refusal, so it is not a 403.
+			name:         "provider internal error is a bad gateway",
+			provider:     &fakeProvider{err: status.Error(codes.Internal, "bug")},
+			providerName: injectionProviderName,
+			want:         envoy_type.StatusCode_BadGateway,
+		},
+		{
+			name:         "provider without FetchSecret is a bad gateway",
+			provider:     &fakeProvider{err: status.Error(codes.Unimplemented, "no FetchSecret")},
+			providerName: injectionProviderName,
+			want:         envoy_type.StatusCode_BadGateway,
+		},
+		{
+			name:         "unclassified provider error is a bad gateway",
+			provider:     &fakeProvider{err: errors.New("provider down")},
+			providerName: injectionProviderName,
+			want:         envoy_type.StatusCode_BadGateway,
+		},
+		{
+			// The provider answered, so a retry cannot help.
+			name:         "empty secret is a bad gateway",
+			provider:     &fakeProvider{resp: bearerTokenResponse("")},
+			providerName: injectionProviderName,
+			want:         envoy_type.StatusCode_BadGateway,
+		},
+		{
+			name:         "secret with a control character is a bad gateway",
+			provider:     &fakeProvider{resp: bearerTokenResponse("a\x00b")},
+			providerName: injectionProviderName,
+			want:         envoy_type.StatusCode_BadGateway,
+		},
 	}
 	for _, leg := range []string{extproc.EgressTLSMITMFilterChainName, extproc.EgressCleartextFilterChainName} {
 		t.Run(leg, func(t *testing.T) {

docs/guides/egress-credential-injection.md

@@ -170,8 +170,9 @@ Concurrent requests that need the same token share one mint.
 | Request decided by an `http` or `https` rule, does not carry the header | Forwarded unchanged; no credential is fetched |
 | Request decided by an `http` or `https` rule, carries the header, no provider configured (`--credential-provider='{"enabled":false}'`) | **500** (Envoy) or **403** (agentgateway), fail closed |
 | Secret missing, or namespace not authorized for the atespace | **403**, fail closed |
-| Provider unreachable or timed out | **503**, fail closed but retryable |
-| Provider returns an empty credential, or one containing control characters | **503**, fail closed |
+| Provider unreachable, timed out or overloaded | **503**, fail closed but retryable |
+| Provider fails with any other code, such as `Internal` or `Unimplemented` | **502**, fail closed |
+| Provider returns an empty credential, or one containing control characters | **502**, fail closed |
 | URI names a provider class this gateway does not serve; unusable header name; unparseable URI | **500** (Envoy) or **403** (agentgateway), fail closed |
 | Request decided by an `http` or `https` rule, carries the header of an `actorJwt` entry, mint succeeds | Header value replaced with the prefix and the actor's JWT, with or without a provider |
 | `actorJwt` entry, the actor no longer exists | **403**, fail closed |
@@ -342,9 +343,10 @@ A provider should:
   the provider's decision.
 
 **Status codes.** The gateway maps a `FetchSecret` failure onto the actor's
-request: `NotFound` and `PermissionDenied` deny with 403 (retrying cannot
-succeed); `Unavailable` and `DeadlineExceeded` fail closed with a retryable
-503; any other code denies with 403.
+request: `NotFound`, `PermissionDenied`, `FailedPrecondition` and
+`InvalidArgument` deny with 403 (retrying cannot succeed); `Unavailable`,
+`DeadlineExceeded` and `ResourceExhausted` fail closed with a retryable 503;
+any other code is a provider fault and fails closed with 502.
 
 ### The reference provider