From 451cea7e8202c3aa68f54ff1e0a8975fcad2d7bc Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sat, 19 Sep 2026 01:20:29 -0400 Subject: [PATCH 1/2] fix: preserve actionable gateway errors at the handler boundary --- .github/workflows/go.yml | 24 +++++++++++ README.md | 17 ++++++++ gateway_error.go | 64 ++++++++++++++++++++++++++++ gateway_error_test.go | 80 +++++++++++++++++++++++++++++++++++ solution.go | 18 +++----- solution_registration_test.go | 6 ++- solution_test.go | 16 ++++--- 7 files changed, 207 insertions(+), 18 deletions(-) create mode 100644 .github/workflows/go.yml create mode 100644 gateway_error.go create mode 100644 gateway_error_test.go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml new file mode 100644 index 0000000..1b29515 --- /dev/null +++ b/.github/workflows/go.yml @@ -0,0 +1,24 @@ +name: Go runtime + +on: + push: + branches: [main] + pull_request: + merge_group: + +permissions: + contents: read + +jobs: + test: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: go.mod + - run: go mod download && go mod verify + - run: go test -race ./... -count=1 -timeout=5m + - run: go vet ./... + - run: test -z "$(gofmt -l *.go)" diff --git a/README.md b/README.md index 81b453a..3edd125 100644 --- a/README.md +++ b/README.md @@ -19,6 +19,23 @@ solution.New(solution.Manifest{ID: "my-solution", Title: "My Solution"}). The gateway URL, the caller's bearer, and the wire protocol are hidden. +## Handler errors + +Return `ClientError` for an explicitly public validation message. Connect errors +returned by `Unary` retain actionable statuses: authentication 401, permission +403, missing resource 404, invalid input 400, conflict 409, precondition 412, +quota 429, unavailable 503 and deadline 504. Other RPC failures return 502. +Only generic status text reaches the browser; wrapped/upstream details remain +available to the handler. No request is retried or replayed. + +REST adapters can return `&solution.GatewayError{StatusCode: resp.StatusCode}` +after a non-success response through the gateway. The runtime preserves 4xx, +503 and 504; unexpected statuses and other server failures become 502. This +error carries no raw body, URL or headers. Work Context mint refusals use this +same path. Untyped errors retain the existing 502/public-error-string behavior, +so handlers must not put private details in them. Consumers using an older runtime +or flattening failures into strings must adopt these typed errors to benefit. + ## Configuration The runtime hardcodes nothing. On boot `Serve` calls diff --git a/gateway_error.go b/gateway_error.go new file mode 100644 index 0000000..f087fa3 --- /dev/null +++ b/gateway_error.go @@ -0,0 +1,64 @@ +package solution + +import ( + "errors" + "fmt" + "net/http" + + "connectrpc.com/connect" +) + +// GatewayError carries a non-success HTTP status received through the host +// gateway. REST adapters should return this rather than flattening the status +// into a string. It never carries response bodies, credentials or headers. +// Handlers may wrap it; only a generic status message reaches the browser. +type GatewayError struct { + StatusCode int +} + +func (e *GatewayError) Error() string { + return fmt.Sprintf("gateway returned HTTP %d", e.StatusCode) +} + +func handlerErrorResponse(err error) (int, string) { + var clientErr *ClientError + if errors.As(err, &clientErr) && clientErr != nil && clientErr.StatusCode >= 400 && clientErr.StatusCode <= 499 { + return clientErr.StatusCode, clientErr.Message + } + var gatewayErr *GatewayError + if errors.As(err, &gatewayErr) && gatewayErr != nil { + status := gatewayErr.StatusCode + if (status >= 400 && status <= 499) || status == http.StatusServiceUnavailable || status == http.StatusGatewayTimeout { + return status, http.StatusText(status) + } + return http.StatusBadGateway, http.StatusText(http.StatusBadGateway) + } + var rpcErr *connect.Error + if errors.As(err, &rpcErr) && rpcErr != nil { + status := http.StatusBadGateway + switch rpcErr.Code() { + case connect.CodeInvalidArgument, connect.CodeOutOfRange: + status = http.StatusBadRequest + case connect.CodeUnauthenticated: + status = http.StatusUnauthorized + case connect.CodePermissionDenied: + status = http.StatusForbidden + case connect.CodeNotFound: + status = http.StatusNotFound + case connect.CodeAlreadyExists, connect.CodeAborted: + status = http.StatusConflict + case connect.CodeFailedPrecondition: + status = http.StatusPreconditionFailed + case connect.CodeResourceExhausted: + status = http.StatusTooManyRequests + case connect.CodeDeadlineExceeded: + status = http.StatusGatewayTimeout + case connect.CodeUnavailable: + status = http.StatusServiceUnavailable + } + return status, http.StatusText(status) + } + // Preserve the existing untyped-handler contract. Callers must not put + // private details in ordinary errors returned to the runtime. + return http.StatusBadGateway, err.Error() +} diff --git a/gateway_error_test.go b/gateway_error_test.go new file mode 100644 index 0000000..da63f0b --- /dev/null +++ b/gateway_error_test.go @@ -0,0 +1,80 @@ +package solution + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" + + "connectrpc.com/connect" + "google.golang.org/protobuf/types/known/emptypb" +) + +func TestUnaryErrorStatusAcrossHTTPBoundary(t *testing.T) { + for _, tc := range []struct { + code connect.Code + status int + }{ + {connect.CodeUnauthenticated, 401}, {connect.CodePermissionDenied, 403}, + {connect.CodeNotFound, 404}, {connect.CodeInvalidArgument, 400}, + {connect.CodeOutOfRange, 400}, {connect.CodeAlreadyExists, 409}, + {connect.CodeAborted, 409}, {connect.CodeFailedPrecondition, 412}, + {connect.CodeResourceExhausted, 429}, {connect.CodeDeadlineExceeded, 504}, + {connect.CodeUnavailable, 503}, {connect.CodeInternal, 502}, + {connect.CodeUnknown, 502}, {connect.CodeDataLoss, 502}, + {connect.CodeCanceled, 502}, {connect.CodeUnimplemented, 502}, + } { + t.Run(tc.code.String(), func(t *testing.T) { + var calls atomic.Int32 + upstream := httptest.NewServer(connect.NewUnaryHandler("/example.Service/Read", + func(_ context.Context, req *connect.Request[emptypb.Empty]) (*connect.Response[emptypb.Empty], error) { + calls.Add(1) + if req.Header().Get("Authorization") != "Bearer viewer-token" { + t.Error("viewer credential lost") + } + return nil, connect.NewError(tc.code, errors.New("private upstream diagnostic")) + })) + defer upstream.Close() + server := serveHandler(t, upstream.URL, func(ctx context.Context, gw *Gateway) (any, error) { + _, err := Unary[emptypb.Empty, emptypb.Empty](ctx, gw, "/example.Service/Read", &emptypb.Empty{}) + return nil, fmt.Errorf("private handler context: %w", err) + }) + resp := viewerRequest(t, server.URL) + defer drainAndClose(resp) + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + if resp.StatusCode != tc.status { + t.Fatalf("status %d, want %d: %s", resp.StatusCode, tc.status, body) + } + if strings.Contains(string(body), "private") { + t.Fatalf("diagnostic disclosed: %s", body) + } + if calls.Load() != 1 { + t.Fatalf("dispatches = %d, want exactly one", calls.Load()) + } + }) + } +} + +func TestGatewayErrorStatusValidation(t *testing.T) { + for _, status := range []int{0, 103, 200, 302, 400, 401, 403, 404, 409, 422, 429, 500, 501, 502, 503, 504, 999} { + t.Run(fmt.Sprint(status), func(t *testing.T) { + want := http.StatusBadGateway + if status >= 400 && status <= 499 || status == 503 || status == 504 { + want = status + } + err := fmt.Errorf("private request URL and response body: %w", &GatewayError{StatusCode: status}) + got, message := handlerErrorResponse(err) + if got != want || message != http.StatusText(want) { + t.Fatalf("got %d %q, want %d", got, message, want) + } + }) + } +} diff --git a/solution.go b/solution.go index 7d04666..2324fc6 100644 --- a/solution.go +++ b/solution.go @@ -53,14 +53,14 @@ type Manifest struct { } // Handler is a solution endpoint. It receives a Gateway bound to the caller's -// bearer and returns any JSON-serializable value. Errors return 502 unless they -// contain a ClientError specifying a 4xx response. +// bearer and returns any JSON-serializable value. ClientError, GatewayError and +// Connect errors retain actionable statuses; untyped errors return 502. type Handler func(ctx context.Context, gw *Gateway) (any, error) // RequestHandler receives the incoming request and the same caller-bound Gateway // as Handler. Implementations must bound and validate request bodies before use. -// Return a ClientError to reject invalid input with a 4xx status; other errors -// use the runtime's normal 502 JSON error response. +// Return a ClientError to reject invalid input with a 4xx status. GatewayError +// and Connect errors retain actionable statuses; untyped errors return 502. type RequestHandler func(r *http.Request, gw *Gateway) (any, error) // ClientError rejects a request with StatusCode (400–499) and a public Message. @@ -680,11 +680,7 @@ func (s *Server) wrapRequest(handler RequestHandler) http.HandlerFunc { } result, err := handler(r, newGateway(s.cfg.gatewayURL, bearer, r.Header.Get(orgHeader), r.Header.Get(sessionHeader))) if err != nil { - status, message := http.StatusBadGateway, err.Error() - var clientErr *ClientError - if errors.As(err, &clientErr) && clientErr != nil && clientErr.StatusCode >= 400 && clientErr.StatusCode <= 499 { - status, message = clientErr.StatusCode, clientErr.Message - } + status, message := handlerErrorResponse(err) writeJSON(w, status, map[string]string{"error": message}) return } @@ -1777,8 +1773,8 @@ func (g *Gateway) mint(ctx context.Context, ask startTaskRequest) (codefly.WorkC Message string `json:"message"` } _ = json.NewDecoder(io.LimitReader(resp.Body, 4<<10)).Decode(&refusal) - return codefly.WorkContextToken{}, time.Time{}, fmt.Errorf("work context mint for %q rejected (status %d, %q): %s", - ask.Audience, resp.StatusCode, refusal.Code, refusal.Message) + return codefly.WorkContextToken{}, time.Time{}, fmt.Errorf("work context mint for %q rejected (status %d, %q): %s: %w", + ask.Audience, resp.StatusCode, refusal.Code, refusal.Message, &GatewayError{StatusCode: resp.StatusCode}) } var issued struct { Token string `json:"token"` diff --git a/solution_registration_test.go b/solution_registration_test.go index 1ebf8f6..7133b77 100644 --- a/solution_registration_test.go +++ b/solution_registration_test.go @@ -117,8 +117,10 @@ func (h *fakeHost) serve(w http.ResponseWriter, r *http.Request) { lifetime = 5 * time.Minute } writeJSON(w, http.StatusOK, map[string]string{ - "token": token, - "expiresAt": time.Now().Add(lifetime).UTC().Format(time.RFC3339), + "token": token, + // Preserve the requested lifetime: truncating to seconds can put + // a 1.5s token below the 1s admission floor before the beat starts. + "expiresAt": time.Now().Add(lifetime).UTC().Format(time.RFC3339Nano), }) case solutionRegisterPath, "/api/solutions/register": if h.registerDelay > 0 { diff --git a/solution_test.go b/solution_test.go index 7689e44..b114b70 100644 --- a/solution_test.go +++ b/solution_test.go @@ -2037,9 +2037,8 @@ func TestGatewayTrafficIsNeverProxied(t *testing.T) { } } -// TestForModuleSurfacesARefusedMint keeps the Connect code and message the -// authority gave. Without them a refusal is a bare 502 at the solution, which -// is exactly the dead end this issue started from. +// A refused mint retains diagnostics for the handler but returns an actionable +// status, without disclosing upstream details, to the browser. func TestForModuleSurfacesARefusedMint(t *testing.T) { gw := newWorkContextGateway(t, &workContextGateway{mintStatus: http.StatusForbidden}) var mintErr error @@ -2050,8 +2049,8 @@ func TestForModuleSurfacesARefusedMint(t *testing.T) { resp := viewerRequest(t, solution.URL) defer drainAndClose(resp) - if resp.StatusCode != http.StatusBadGateway { - t.Fatalf("solution answered %d, want 502", resp.StatusCode) + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("solution answered %d, want 403", resp.StatusCode) } if mintErr == nil { t.Fatal("ForModule succeeded against a refusing authority") @@ -2061,6 +2060,13 @@ func TestForModuleSurfacesARefusedMint(t *testing.T) { t.Errorf("mint error %q does not mention %q", mintErr, want) } } + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(body), "caller holds no such authority") || strings.Contains(string(body), "documents") { + t.Fatalf("upstream diagnostic disclosed: %s", body) + } select { case call := <-gw.calls: t.Errorf("module was read anyway, with work context %q", call.WorkContext) From 0f52d5ab247bbb73d978cfa3a9fdb4b02bac6d60 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sat, 19 Sep 2026 01:20:44 -0400 Subject: [PATCH 2/2] ci: use option-safe formatting glob --- .github/workflows/go.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 1b29515..d69d509 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -21,4 +21,4 @@ jobs: - run: go mod download && go mod verify - run: go test -race ./... -count=1 -timeout=5m - run: go vet ./... - - run: test -z "$(gofmt -l *.go)" + - run: test -z "$(gofmt -l ./*.go)"