From 78f62cd17ddb4b2994e06bf4552cce640fb33941 Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Thu, 15 Oct 2026 10:26:49 +0100 Subject: [PATCH 1/2] egress: answer a credential provider fault with 502, not 403 Signed-off-by: Eric Curtin --- api/credprovider/v1/credprovider.proto | 5 ++ api/credprovider/v1/credprovider_grpc.pb.go | 10 ++++ .../internal/router/egress/credentials.go | 21 +++++---- .../router/egress/credentials_test.go | 47 +++++++++++++++++-- docs/egress-credential-injection.md | 12 +++-- internal/plugins/gcp-secret-manager/README.md | 2 +- 6 files changed, 76 insertions(+), 21 deletions(-) diff --git a/api/credprovider/v1/credprovider.proto b/api/credprovider/v1/credprovider.proto index ac4bc1fd0d..b6e146ec69 100644 --- a/api/credprovider/v1/credprovider.proto +++ b/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) {} } diff --git a/api/credprovider/v1/credprovider_grpc.pb.go b/api/credprovider/v1/credprovider_grpc.pb.go index bcba421a18..186eecfafd 100644 --- a/api/credprovider/v1/credprovider_grpc.pb.go +++ b/api/credprovider/v1/credprovider_grpc.pb.go @@ -47,6 +47,11 @@ const ( type CredentialProviderClient interface { // 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. FetchSecret(ctx context.Context, in *FetchSecretRequest, opts ...grpc.CallOption) (*FetchSecretResponse, error) } @@ -79,6 +84,11 @@ func (c *credentialProviderClient) FetchSecret(ctx context.Context, in *FetchSec type CredentialProviderServer interface { // 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. FetchSecret(context.Context, *FetchSecretRequest) (*FetchSecretResponse, error) mustEmbedUnimplementedCredentialProviderServer() } diff --git a/cmd/atenet/internal/router/egress/credentials.go b/cmd/atenet/internal/router/egress/credentials.go index 4814a155ad..3b85641582 100644 --- a/cmd/atenet/internal/router/egress/credentials.go +++ b/cmd/atenet/internal/router/egress/credentials.go @@ -38,20 +38,21 @@ 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, mirroring mapEgressIdentityError. 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) } } @@ -181,7 +182,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 } diff --git a/cmd/atenet/internal/router/egress/credentials_test.go b/cmd/atenet/internal/router/egress/credentials_test.go index b69fb4af84..0ed2afb1de 100644 --- a/cmd/atenet/internal/router/egress/credentials_test.go +++ b/cmd/atenet/internal/router/egress/credentials_test.go @@ -225,18 +225,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) { diff --git a/docs/egress-credential-injection.md b/docs/egress-credential-injection.md index daa1ea9adf..bbe5b4e4b9 100644 --- a/docs/egress-credential-injection.md +++ b/docs/egress-credential-injection.md @@ -162,8 +162,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 | @@ -334,9 +335,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 diff --git a/internal/plugins/gcp-secret-manager/README.md b/internal/plugins/gcp-secret-manager/README.md index f58695f378..ecdeb84e40 100644 --- a/internal/plugins/gcp-secret-manager/README.md +++ b/internal/plugins/gcp-secret-manager/README.md @@ -71,7 +71,7 @@ gateway sets no deadline of its own and Envoy abandons the request after its 5s ext_proc message timeout, so keep the flag below that. The gateway answers `Unavailable` and `DeadlineExceeded` with a retryable 503 -and every other code with a 403: +and the other codes below with a 403: | Condition | Code | |---|---| From ff0c5f980469751d6e250aa3be002bf5d2e7b4f8 Mon Sep 17 00:00:00 2001 From: Eric Curtin Date: Fri, 30 Oct 2026 14:14:37 +0000 Subject: [PATCH 2/2] egress: fix mapCredentialProviderError comment Signed-off-by: Eric Curtin --- cmd/atenet/internal/router/egress/credentials.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/cmd/atenet/internal/router/egress/credentials.go b/cmd/atenet/internal/router/egress/credentials.go index 54139b240f..9bea21711a 100644 --- a/cmd/atenet/internal/router/egress/credentials.go +++ b/cmd/atenet/internal/router/egress/credentials.go @@ -40,10 +40,9 @@ 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, will not release or rejects (NotFound, -// PermissionDenied, FailedPrecondition, InvalidArgument) is 403, since retrying -// cannot succeed. A provider that is +// 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.