From 7c3dfef2dd9b7a9995ca864a2f8113b7d3386e31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Insaurralde?= Date: Wed, 21 Oct 2026 15:45:11 -0300 Subject: [PATCH] chore(server): only enable gRPC reflection in development MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gRPC reflection advertises the already-public API schema and exists as a convenience for local debugging tooling such as grpcurl. No product client (CLI, web UI, or server-to-server traffic) depends on it, so register it only in development (Version == "dev") and call grpc.DisableReflection() in production. Local development is unchanged. Applies to both the control plane and the artifact CAS. Refs PFM-7977. Chainloop-Trace-Sessions: 009eae12-7fed-495c-8131-8171a788bebd Signed-off-by: Matías Insaurralde --- app/artifact-cas/internal/server/grpc.go | 9 +++ .../internal/server/reflection_env_test.go | 79 +++++++++++++++++++ app/controlplane/internal/server/grpc.go | 8 ++ .../internal/server/reflection_env_test.go | 77 ++++++++++++++++++ 4 files changed, 173 insertions(+) create mode 100644 app/artifact-cas/internal/server/reflection_env_test.go create mode 100644 app/controlplane/internal/server/reflection_env_test.go diff --git a/app/artifact-cas/internal/server/grpc.go b/app/artifact-cas/internal/server/grpc.go index b23657b7b..a8af60edb 100644 --- a/app/artifact-cas/internal/server/grpc.go +++ b/app/artifact-cas/internal/server/grpc.go @@ -230,6 +230,15 @@ func newGRPCServer(settings *Opts, grpcServerOpts []grpcLib.ServerOption, extraO grpc_prometheus.EnableHandlingTimeHistogram(grpc_prometheus.WithHistogramBuckets(prometheus.ExponentialBucketsRange(0.5, 60, 8))) opts = append(opts, extraOpts...) + + // gRPC reflection advertises the already-public API schema and exists as a + // convenience for local debugging tooling such as grpcurl. No product + // client depends on it, so register it in development only; production + // disables it. + if Version != "dev" { + opts = append(opts, grpc.DisableReflection()) + } + if c.GetGrpc().GetTimeout() != nil { opts = append(opts, grpc.Timeout(c.GetGrpc().GetTimeout().AsDuration())) } diff --git a/app/artifact-cas/internal/server/reflection_env_test.go b/app/artifact-cas/internal/server/reflection_env_test.go new file mode 100644 index 000000000..a7d25cd51 --- /dev/null +++ b/app/artifact-cas/internal/server/reflection_env_test.go @@ -0,0 +1,79 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package server + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + reflectionpb "google.golang.org/grpc/reflection/grpc_reflection_v1" +) + +// TestReflectionGatedByEnv verifies that gRPC reflection is served only in +// development (Version == "dev") and disabled in production, on the shared CAS +// endpoint. No product client depends on reflection; it is a local-development +// convenience. +func TestReflectionGatedByEnv(t *testing.T) { + orig := Version + t.Cleanup(func() { Version = orig }) + + cases := []struct { + name string + version string + wantReachable bool + }{ + {"dev keeps reflection for local grpcurl", "dev", true}, + {"production disables reflection", "v1.2.3", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + Version = tc.version + sharedURL, _ := startCASTestServers(t, casTestOpts(t)) + require.Equal(t, tc.wantReachable, reflectionReachable(t, sharedURL), + "Version=%q: reflection reachable should be %v", tc.version, tc.wantReachable) + }) + } +} + +// reflectionReachable reports whether the shared endpoint answers a gRPC +// reflection ListServices request with a non-empty service list. +func reflectionReachable(t *testing.T, sharedURL string) bool { + t.Helper() + addr := strings.TrimPrefix(strings.TrimPrefix(sharedURL, "http://"), "https://") + conn, err := grpc.NewClient(addr, grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + t.Cleanup(func() { _ = conn.Close() }) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + stream, err := reflectionpb.NewServerReflectionClient(conn).ServerReflectionInfo(ctx) + require.NoError(t, err) + if err := stream.Send(&reflectionpb.ServerReflectionRequest{ + MessageRequest: &reflectionpb.ServerReflectionRequest_ListServices{}, + }); err != nil { + return false // stream rejected -> reflection not served + } + resp, err := stream.Recv() + if err != nil { + return false // Unimplemented / unknown service -> reflection disabled + } + return len(resp.GetListServicesResponse().GetService()) > 0 +} diff --git a/app/controlplane/internal/server/grpc.go b/app/controlplane/internal/server/grpc.go index 74bc1231a..31425ed68 100644 --- a/app/controlplane/internal/server/grpc.go +++ b/app/controlplane/internal/server/grpc.go @@ -191,6 +191,14 @@ func newGRPCServer(opts *Opts, extraOpts ...grpc.ServerOption) *grpc.Server { ), }, extraOpts...) + // gRPC reflection advertises the already-public API schema and exists as a + // convenience for local debugging tooling such as grpcurl. No product + // client depends on it, so register it in development only; production + // disables it. + if Version != "dev" { + serverOpts = append(serverOpts, grpc.DisableReflection()) + } + if v := opts.ServerConfig.Grpc.Timeout; v != nil { serverOpts = append(serverOpts, grpc.Timeout(v.AsDuration())) } diff --git a/app/controlplane/internal/server/reflection_env_test.go b/app/controlplane/internal/server/reflection_env_test.go new file mode 100644 index 000000000..eb239a731 --- /dev/null +++ b/app/controlplane/internal/server/reflection_env_test.go @@ -0,0 +1,77 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package server + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + grpcLib "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + reflectionpb "google.golang.org/grpc/reflection/grpc_reflection_v1" +) + +// TestReflectionGatedByEnv verifies that gRPC reflection is served only in +// development (Version == "dev") and disabled in production, on the shared +// control-plane endpoint. No product client depends on reflection; it is a +// local-development convenience. +func TestReflectionGatedByEnv(t *testing.T) { + orig := Version + t.Cleanup(func() { Version = orig }) + + cases := []struct { + name string + version string + wantReachable bool + }{ + {"dev keeps reflection for local grpcurl", "dev", true}, + {"production disables reflection", "v1.2.3", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + Version = tc.version + addr, _, _ := startSinglePortApp(t, nil) + require.Equal(t, tc.wantReachable, cpReflectionReachable(t, addr), + "Version=%q: reflection reachable should be %v", tc.version, tc.wantReachable) + }) + } +} + +// cpReflectionReachable reports whether the shared endpoint answers a gRPC +// reflection ListServices request with a non-empty service list. +func cpReflectionReachable(t *testing.T, addr string) bool { + t.Helper() + conn, err := grpcLib.NewClient(addr, grpcLib.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + t.Cleanup(func() { _ = conn.Close() }) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + stream, err := reflectionpb.NewServerReflectionClient(conn).ServerReflectionInfo(ctx) + require.NoError(t, err) + if err := stream.Send(&reflectionpb.ServerReflectionRequest{ + MessageRequest: &reflectionpb.ServerReflectionRequest_ListServices{}, + }); err != nil { + return false // stream rejected -> reflection not served + } + resp, err := stream.Recv() + if err != nil { + return false // Unimplemented / unknown service -> reflection disabled + } + return len(resp.GetListServicesResponse().GetService()) > 0 +}