From 136dff6e6bad532931e9d00bdcf25bcda91c4c55 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sat, 14 Mar 2026 10:26:09 +0100 Subject: [PATCH 1/3] feat(cas): add configurable CORS headers to HTTP download endpoint The platform frontend needs to fetch() content from CAS signed URLs to render evidence inline (SBOMs, SARIF, etc.). Currently browsers block these requests because the CAS HTTP endpoint returns no CORS headers. Add a configurable CORS middleware that sits outside auth so OPTIONS preflights are handled before token validation. Exposed via Helm chart as cas.allowedOrigins (supports both YAML list and comma-separated string). When empty (default), CORS is disabled and the middleware is a passthrough. Closes #2848 Signed-off-by: Miguel Martinez Trivino --- app/artifact-cas/configs/config.devel.yaml | 3 + app/artifact-cas/internal/conf/conf.pb.go | 109 +++++++++---- app/artifact-cas/internal/conf/conf.proto | 6 +- app/artifact-cas/internal/server/cors.go | 76 +++++++++ app/artifact-cas/internal/server/cors_test.go | 144 ++++++++++++++++++ app/artifact-cas/internal/server/http.go | 5 +- .../chainloop/templates/cas/configmap.yaml | 15 ++ deployment/chainloop/values.yaml | 9 ++ 8 files changed, 338 insertions(+), 29 deletions(-) create mode 100644 app/artifact-cas/internal/server/cors.go create mode 100644 app/artifact-cas/internal/server/cors_test.go diff --git a/app/artifact-cas/configs/config.devel.yaml b/app/artifact-cas/configs/config.devel.yaml index ce8488d15..654b0519b 100644 --- a/app/artifact-cas/configs/config.devel.yaml +++ b/app/artifact-cas/configs/config.devel.yaml @@ -8,6 +8,9 @@ server: # Timeouts for http downloads # grpc downloads/uploads don't require this because they don't have timeouts timeout: 300s + # cors: + # allow_origins: + # - "http://localhost:3000" grpc: addr: 0.0.0.0:9001 # Some unary RPCs are slow, so we need to increase the timeout diff --git a/app/artifact-cas/internal/conf/conf.pb.go b/app/artifact-cas/internal/conf/conf.pb.go index 2d0fe2979..136759bed 100644 --- a/app/artifact-cas/internal/conf/conf.pb.go +++ b/app/artifact-cas/internal/conf/conf.pb.go @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-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. @@ -329,6 +329,7 @@ type Server_HTTP struct { Network string `protobuf:"bytes,1,opt,name=network,proto3" json:"network,omitempty"` Addr string `protobuf:"bytes,2,opt,name=addr,proto3" json:"addr,omitempty"` Timeout *durationpb.Duration `protobuf:"bytes,3,opt,name=timeout,proto3" json:"timeout,omitempty"` + Cors *Server_CORS `protobuf:"bytes,4,opt,name=cors,proto3" json:"cors,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -384,6 +385,57 @@ func (x *Server_HTTP) GetTimeout() *durationpb.Duration { return nil } +func (x *Server_HTTP) GetCors() *Server_CORS { + if x != nil { + return x.Cors + } + return nil +} + +type Server_CORS struct { + state protoimpl.MessageState `protogen:"open.v1"` + AllowOrigins []string `protobuf:"bytes,1,rep,name=allow_origins,json=allowOrigins,proto3" json:"allow_origins,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *Server_CORS) Reset() { + *x = Server_CORS{} + mi := &file_conf_proto_msgTypes[6] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *Server_CORS) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*Server_CORS) ProtoMessage() {} + +func (x *Server_CORS) ProtoReflect() protoreflect.Message { + mi := &file_conf_proto_msgTypes[6] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use Server_CORS.ProtoReflect.Descriptor instead. +func (*Server_CORS) Descriptor() ([]byte, []int) { + return file_conf_proto_rawDescGZIP(), []int{1, 1} +} + +func (x *Server_CORS) GetAllowOrigins() []string { + if x != nil { + return x.AllowOrigins + } + return nil +} + type Server_TLS struct { state protoimpl.MessageState `protogen:"open.v1"` // path to certificate and private key @@ -395,7 +447,7 @@ type Server_TLS struct { func (x *Server_TLS) Reset() { *x = Server_TLS{} - mi := &file_conf_proto_msgTypes[6] + mi := &file_conf_proto_msgTypes[7] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -407,7 +459,7 @@ func (x *Server_TLS) String() string { func (*Server_TLS) ProtoMessage() {} func (x *Server_TLS) ProtoReflect() protoreflect.Message { - mi := &file_conf_proto_msgTypes[6] + mi := &file_conf_proto_msgTypes[7] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -420,7 +472,7 @@ func (x *Server_TLS) ProtoReflect() protoreflect.Message { // Deprecated: Use Server_TLS.ProtoReflect.Descriptor instead. func (*Server_TLS) Descriptor() ([]byte, []int) { - return file_conf_proto_rawDescGZIP(), []int{1, 1} + return file_conf_proto_rawDescGZIP(), []int{1, 2} } func (x *Server_TLS) GetCertificate() string { @@ -449,7 +501,7 @@ type Server_GRPC struct { func (x *Server_GRPC) Reset() { *x = Server_GRPC{} - mi := &file_conf_proto_msgTypes[7] + mi := &file_conf_proto_msgTypes[8] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -461,7 +513,7 @@ func (x *Server_GRPC) String() string { func (*Server_GRPC) ProtoMessage() {} func (x *Server_GRPC) ProtoReflect() protoreflect.Message { - mi := &file_conf_proto_msgTypes[7] + mi := &file_conf_proto_msgTypes[8] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -474,7 +526,7 @@ func (x *Server_GRPC) ProtoReflect() protoreflect.Message { // Deprecated: Use Server_GRPC.ProtoReflect.Descriptor instead. func (*Server_GRPC) Descriptor() ([]byte, []int) { - return file_conf_proto_rawDescGZIP(), []int{1, 2} + return file_conf_proto_rawDescGZIP(), []int{1, 3} } func (x *Server_GRPC) GetNetwork() string { @@ -520,15 +572,18 @@ const file_conf_proto_rawDesc = "" + "\x06sentry\x18\x01 \x01(\v2\x1f.Bootstrap.Observability.SentryR\x06sentry\x1a<\n" + "\x06Sentry\x12\x10\n" + "\x03dsn\x18\x01 \x01(\tR\x03dsn\x12 \n" + - "\venvironment\x18\x02 \x01(\tR\venvironment\"\xca\x03\n" + + "\venvironment\x18\x02 \x01(\tR\venvironment\"\x9a\x04\n" + "\x06Server\x12 \n" + "\x04http\x18\x01 \x01(\v2\f.Server.HTTPR\x04http\x12 \n" + "\x04grpc\x18\x02 \x01(\v2\f.Server.GRPCR\x04grpc\x12/\n" + - "\fhttp_metrics\x18\x03 \x01(\v2\f.Server.HTTPR\vhttpMetrics\x1ai\n" + + "\fhttp_metrics\x18\x03 \x01(\v2\f.Server.HTTPR\vhttpMetrics\x1a\x8b\x01\n" + "\x04HTTP\x12\x18\n" + "\anetwork\x18\x01 \x01(\tR\anetwork\x12\x12\n" + "\x04addr\x18\x02 \x01(\tR\x04addr\x123\n" + - "\atimeout\x18\x03 \x01(\v2\x19.google.protobuf.DurationR\atimeout\x1aH\n" + + "\atimeout\x18\x03 \x01(\v2\x19.google.protobuf.DurationR\atimeout\x12 \n" + + "\x04cors\x18\x04 \x01(\v2\f.Server.CORSR\x04cors\x1a+\n" + + "\x04CORS\x12#\n" + + "\rallow_origins\x18\x01 \x03(\tR\fallowOrigins\x1aH\n" + "\x03TLS\x12 \n" + "\vcertificate\x18\x01 \x01(\tR\vcertificate\x12\x1f\n" + "\vprivate_key\x18\x02 \x01(\tR\n" + @@ -555,7 +610,7 @@ func file_conf_proto_rawDescGZIP() []byte { return file_conf_proto_rawDescData } -var file_conf_proto_msgTypes = make([]protoimpl.MessageInfo, 8) +var file_conf_proto_msgTypes = make([]protoimpl.MessageInfo, 9) var file_conf_proto_goTypes = []any{ (*Bootstrap)(nil), // 0: Bootstrap (*Server)(nil), // 1: Server @@ -563,28 +618,30 @@ var file_conf_proto_goTypes = []any{ (*Bootstrap_Observability)(nil), // 3: Bootstrap.Observability (*Bootstrap_Observability_Sentry)(nil), // 4: Bootstrap.Observability.Sentry (*Server_HTTP)(nil), // 5: Server.HTTP - (*Server_TLS)(nil), // 6: Server.TLS - (*Server_GRPC)(nil), // 7: Server.GRPC - (*v1.Credentials)(nil), // 8: credentials.v1.Credentials - (*durationpb.Duration)(nil), // 9: google.protobuf.Duration + (*Server_CORS)(nil), // 6: Server.CORS + (*Server_TLS)(nil), // 7: Server.TLS + (*Server_GRPC)(nil), // 8: Server.GRPC + (*v1.Credentials)(nil), // 9: credentials.v1.Credentials + (*durationpb.Duration)(nil), // 10: google.protobuf.Duration } var file_conf_proto_depIdxs = []int32{ 1, // 0: Bootstrap.server:type_name -> Server 2, // 1: Bootstrap.auth:type_name -> Auth 3, // 2: Bootstrap.observability:type_name -> Bootstrap.Observability - 8, // 3: Bootstrap.credentials_service:type_name -> credentials.v1.Credentials + 9, // 3: Bootstrap.credentials_service:type_name -> credentials.v1.Credentials 5, // 4: Server.http:type_name -> Server.HTTP - 7, // 5: Server.grpc:type_name -> Server.GRPC + 8, // 5: Server.grpc:type_name -> Server.GRPC 5, // 6: Server.http_metrics:type_name -> Server.HTTP 4, // 7: Bootstrap.Observability.sentry:type_name -> Bootstrap.Observability.Sentry - 9, // 8: Server.HTTP.timeout:type_name -> google.protobuf.Duration - 9, // 9: Server.GRPC.timeout:type_name -> google.protobuf.Duration - 6, // 10: Server.GRPC.tls_config:type_name -> Server.TLS - 11, // [11:11] is the sub-list for method output_type - 11, // [11:11] is the sub-list for method input_type - 11, // [11:11] is the sub-list for extension type_name - 11, // [11:11] is the sub-list for extension extendee - 0, // [0:11] is the sub-list for field type_name + 10, // 8: Server.HTTP.timeout:type_name -> google.protobuf.Duration + 6, // 9: Server.HTTP.cors:type_name -> Server.CORS + 10, // 10: Server.GRPC.timeout:type_name -> google.protobuf.Duration + 7, // 11: Server.GRPC.tls_config:type_name -> Server.TLS + 12, // [12:12] is the sub-list for method output_type + 12, // [12:12] is the sub-list for method input_type + 12, // [12:12] is the sub-list for extension type_name + 12, // [12:12] is the sub-list for extension extendee + 0, // [0:12] is the sub-list for field type_name } func init() { file_conf_proto_init() } @@ -598,7 +655,7 @@ func file_conf_proto_init() { GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: unsafe.Slice(unsafe.StringData(file_conf_proto_rawDesc), len(file_conf_proto_rawDesc)), NumEnums: 0, - NumMessages: 8, + NumMessages: 9, NumExtensions: 0, NumServices: 0, }, diff --git a/app/artifact-cas/internal/conf/conf.proto b/app/artifact-cas/internal/conf/conf.proto index 5827f5c12..065aedded 100644 --- a/app/artifact-cas/internal/conf/conf.proto +++ b/app/artifact-cas/internal/conf/conf.proto @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-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. @@ -42,6 +42,10 @@ message Server { string network = 1; string addr = 2; google.protobuf.Duration timeout = 3; + CORS cors = 4; + } + message CORS { + repeated string allow_origins = 1; } message TLS { // path to certificate and private key diff --git a/app/artifact-cas/internal/server/cors.go b/app/artifact-cas/internal/server/cors.go new file mode 100644 index 000000000..665bcc4fd --- /dev/null +++ b/app/artifact-cas/internal/server/cors.go @@ -0,0 +1,76 @@ +// +// 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 "net/http" + +// CORSMiddleware returns an http.Handler that applies CORS headers based on allowedOrigins. +// If allowedOrigins is empty, the middleware is a passthrough (CORS disabled). +// If "*" is in the list, any origin is allowed. +// Otherwise, only origins in the list are echoed back. +// OPTIONS preflight requests are short-circuited with 204 No Content before reaching the next handler. +func CORSMiddleware(allowedOrigins []string, next http.Handler) http.Handler { + if len(allowedOrigins) == 0 { + return next + } + + wildcard := false + allowed := make(map[string]struct{}, len(allowedOrigins)) + for _, o := range allowedOrigins { + if o == "*" { + wildcard = true + } + allowed[o] = struct{}{} + } + + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + origin := r.Header.Get("Origin") + if origin == "" { + next.ServeHTTP(w, r) + return + } + + var matchedOrigin string + switch { + case wildcard: + matchedOrigin = "*" + default: + if _, ok := allowed[origin]; ok { + matchedOrigin = origin + } + } + + if matchedOrigin == "" { + next.ServeHTTP(w, r) + return + } + + w.Header().Set("Access-Control-Allow-Origin", matchedOrigin) + if matchedOrigin != "*" { + w.Header().Set("Vary", "Origin") + } + + if r.Method == http.MethodOptions { + w.Header().Set("Access-Control-Allow-Methods", "GET, HEAD, OPTIONS") + w.Header().Set("Access-Control-Allow-Headers", "Authorization, Content-Type") + w.Header().Set("Access-Control-Max-Age", "86400") + w.WriteHeader(http.StatusNoContent) + return + } + + next.ServeHTTP(w, r) + }) +} diff --git a/app/artifact-cas/internal/server/cors_test.go b/app/artifact-cas/internal/server/cors_test.go new file mode 100644 index 000000000..9a83652b1 --- /dev/null +++ b/app/artifact-cas/internal/server/cors_test.go @@ -0,0 +1,144 @@ +// +// 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 ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCORSMiddleware(t *testing.T) { + handlerCalled := false + inner := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + handlerCalled = true + w.WriteHeader(http.StatusOK) + }) + + tests := []struct { + name string + allowedOrigins []string + method string + origin string + wantStatus int + wantACHeader string // expected Access-Control-Allow-Origin value, empty if absent + wantVary bool + wantHandler bool // whether inner handler should be called + }{ + { + name: "empty origins - passthrough", + allowedOrigins: nil, + method: http.MethodGet, + origin: "http://example.com", + wantStatus: http.StatusOK, + wantHandler: true, + }, + { + name: "no Origin header - passthrough", + allowedOrigins: []string{"http://example.com"}, + method: http.MethodGet, + origin: "", + wantStatus: http.StatusOK, + wantHandler: true, + }, + { + name: "matching origin GET", + allowedOrigins: []string{"http://example.com"}, + method: http.MethodGet, + origin: "http://example.com", + wantStatus: http.StatusOK, + wantACHeader: "http://example.com", + wantVary: true, + wantHandler: true, + }, + { + name: "matching origin OPTIONS preflight", + allowedOrigins: []string{"http://example.com"}, + method: http.MethodOptions, + origin: "http://example.com", + wantStatus: http.StatusNoContent, + wantACHeader: "http://example.com", + wantVary: true, + wantHandler: false, + }, + { + name: "non-matching origin", + allowedOrigins: []string{"http://example.com"}, + method: http.MethodGet, + origin: "http://evil.com", + wantStatus: http.StatusOK, + wantHandler: true, + }, + { + name: "wildcard allows any origin", + allowedOrigins: []string{"*"}, + method: http.MethodGet, + origin: "http://anything.com", + wantStatus: http.StatusOK, + wantACHeader: "*", + wantHandler: true, + }, + { + name: "wildcard OPTIONS preflight", + allowedOrigins: []string{"*"}, + method: http.MethodOptions, + origin: "http://anything.com", + wantStatus: http.StatusNoContent, + wantACHeader: "*", + wantHandler: false, + }, + { + name: "multiple origins - match second", + allowedOrigins: []string{"http://a.example.com", "http://b.example.com"}, + method: http.MethodGet, + origin: "http://b.example.com", + wantStatus: http.StatusOK, + wantACHeader: "http://b.example.com", + wantVary: true, + wantHandler: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + handlerCalled = false + handler := CORSMiddleware(tc.allowedOrigins, inner) + + req := httptest.NewRequest(tc.method, "/download/sha256:abc", nil) + if tc.origin != "" { + req.Header.Set("Origin", tc.origin) + } + + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + assert.Equal(t, tc.wantStatus, rec.Code) + assert.Equal(t, tc.wantHandler, handlerCalled) + + if tc.wantACHeader != "" { + assert.Equal(t, tc.wantACHeader, rec.Header().Get("Access-Control-Allow-Origin")) + } else { + assert.Empty(t, rec.Header().Get("Access-Control-Allow-Origin")) + } + + if tc.wantVary { + assert.Equal(t, "Origin", rec.Header().Get("Vary")) + } + }) + } +} diff --git a/app/artifact-cas/internal/server/http.go b/app/artifact-cas/internal/server/http.go index 67d690505..cbcf14e29 100644 --- a/app/artifact-cas/internal/server/http.go +++ b/app/artifact-cas/internal/server/http.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-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. @@ -65,7 +65,8 @@ func NewHTTPServer(c *conf.Server, authConf *conf.Auth, downloadSvc *service.Dow srv := http.NewServer(opts...) - srv.Handle(service.DownloadPath, middlewares_http.AuthFromQueryParam(loadPublicKey(rawKey), claimsFunc(), casJWT.SigningMethod, downloadSvc)) + downloadHandler := middlewares_http.AuthFromQueryParam(loadPublicKey(rawKey), claimsFunc(), casJWT.SigningMethod, downloadSvc) + srv.Handle(service.DownloadPath, CORSMiddleware(c.Http.GetCors().GetAllowOrigins(), downloadHandler)) api.RegisterStatusServiceHTTPServer(srv, service.NewStatusService(Version, providers)) return srv, nil } diff --git a/deployment/chainloop/templates/cas/configmap.yaml b/deployment/chainloop/templates/cas/configmap.yaml index b94ac4db7..e77416d4d 100644 --- a/deployment/chainloop/templates/cas/configmap.yaml +++ b/deployment/chainloop/templates/cas/configmap.yaml @@ -20,6 +20,21 @@ data: # Timeouts for http downloads # grpc downloads/uploads don't require this because they don't have timeouts timeout: 300s + {{- if .Values.cas.allowedOrigins }} + cors: + allow_origins: + {{- if kindIs "string" .Values.cas.allowedOrigins }} + {{- range splitList "," .Values.cas.allowedOrigins }} + {{- if . }} + - {{ trim . | quote }} + {{- end }} + {{- end }} + {{- else }} + {{- range .Values.cas.allowedOrigins }} + - {{ . | quote }} + {{- end }} + {{- end }} + {{- end }} grpc: {{- if include "cas.tls-secret-name" . }} tls_config: diff --git a/deployment/chainloop/values.yaml b/deployment/chainloop/values.yaml index 22317cd60..442fda0b9 100644 --- a/deployment/chainloop/values.yaml +++ b/deployment/chainloop/values.yaml @@ -990,6 +990,15 @@ cas: ## @extra cas.externalURL Optional External URL for the CAS service. If not set it will be derived from the ingress and service configuration # externalURL: "" + ## @param cas.allowedOrigins List of allowed origins for CORS on the HTTP download endpoint. If empty, CORS is disabled. + ## Accepts both a YAML list and a comma-separated string: + ## allowedOrigins: + ## - "https://app.chainloop.dev" + ## - "https://console.example.com" + ## or: + ## allowedOrigins: "https://app.chainloop.dev,https://console.example.com" + allowedOrigins: [] + ## @param cas.image.registry [default: REGISTRY_NAME] Image registry ## @param cas.image.repository [default: REPOSITORY_NAME] Image repository ## @skip cas.image.tag From b554c3516f0f46301b3e2c80905fe6a7c963db0e Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sat, 14 Mar 2026 10:35:45 +0100 Subject: [PATCH 2/3] docs(cas): clarify that allowedOrigins requires full origin with scheme Signed-off-by: Miguel Martinez Trivino --- deployment/chainloop/values.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/deployment/chainloop/values.yaml b/deployment/chainloop/values.yaml index 442fda0b9..04b0d5a63 100644 --- a/deployment/chainloop/values.yaml +++ b/deployment/chainloop/values.yaml @@ -991,6 +991,8 @@ cas: # externalURL: "" ## @param cas.allowedOrigins List of allowed origins for CORS on the HTTP download endpoint. If empty, CORS is disabled. + ## Each value must be a full origin (scheme + host + optional port), e.g. "https://app.chainloop.dev". + ## Bare hostnames like "app.chainloop.dev" will NOT work — browsers always send the scheme in the Origin header. ## Accepts both a YAML list and a comma-separated string: ## allowedOrigins: ## - "https://app.chainloop.dev" From e50733b29f81d7348321e8ea2c6902ea4f977d99 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Sat, 14 Mar 2026 10:55:25 +0100 Subject: [PATCH 3/3] fix(cas): address CORS PR review feedback Move cors config from Server.HTTP to Server level so it doesn't appear supported on http_metrics. Set Vary: Origin before the early return for non-matching origins (HTTP cache correctness) using Add instead of Set. Fix whitespace-only segments in Helm configmap template with trim. Signed-off-by: Miguel Martinez Trivino --- app/artifact-cas/configs/config.devel.yaml | 6 ++-- app/artifact-cas/internal/conf/conf.pb.go | 33 ++++++++++--------- app/artifact-cas/internal/conf/conf.proto | 3 +- app/artifact-cas/internal/server/cors.go | 8 +++-- app/artifact-cas/internal/server/cors_test.go | 1 + app/artifact-cas/internal/server/http.go | 2 +- .../chainloop/templates/cas/configmap.yaml | 30 ++++++++--------- 7 files changed, 44 insertions(+), 39 deletions(-) diff --git a/app/artifact-cas/configs/config.devel.yaml b/app/artifact-cas/configs/config.devel.yaml index 654b0519b..8496219ab 100644 --- a/app/artifact-cas/configs/config.devel.yaml +++ b/app/artifact-cas/configs/config.devel.yaml @@ -8,9 +8,6 @@ server: # Timeouts for http downloads # grpc downloads/uploads don't require this because they don't have timeouts timeout: 300s - # cors: - # allow_origins: - # - "http://localhost:3000" grpc: addr: 0.0.0.0:9001 # Some unary RPCs are slow, so we need to increase the timeout @@ -21,6 +18,9 @@ server: # private_key: "../../devel/devkeys/selfsigned/cas.key" http_metrics: addr: 0.0.0.0:5001 + # cors: + # allow_origins: + # - "http://localhost:3000" credentials_service: # we will check that we can read there diff --git a/app/artifact-cas/internal/conf/conf.pb.go b/app/artifact-cas/internal/conf/conf.pb.go index 136759bed..6033422df 100644 --- a/app/artifact-cas/internal/conf/conf.pb.go +++ b/app/artifact-cas/internal/conf/conf.pb.go @@ -113,7 +113,9 @@ type Server struct { // GRPC API endpoint Grpc *Server_GRPC `protobuf:"bytes,2,opt,name=grpc,proto3" json:"grpc,omitempty"` // hHTTP server where the prometheus metrics will get exposed - HttpMetrics *Server_HTTP `protobuf:"bytes,3,opt,name=http_metrics,json=httpMetrics,proto3" json:"http_metrics,omitempty"` + HttpMetrics *Server_HTTP `protobuf:"bytes,3,opt,name=http_metrics,json=httpMetrics,proto3" json:"http_metrics,omitempty"` + // CORS configuration for the HTTP download endpoint + Cors *Server_CORS `protobuf:"bytes,4,opt,name=cors,proto3" json:"cors,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -169,6 +171,13 @@ func (x *Server) GetHttpMetrics() *Server_HTTP { return nil } +func (x *Server) GetCors() *Server_CORS { + if x != nil { + return x.Cors + } + return nil +} + type Auth struct { state protoimpl.MessageState `protogen:"open.v1"` // Public key used to verify the received JWT token @@ -329,7 +338,6 @@ type Server_HTTP struct { Network string `protobuf:"bytes,1,opt,name=network,proto3" json:"network,omitempty"` Addr string `protobuf:"bytes,2,opt,name=addr,proto3" json:"addr,omitempty"` Timeout *durationpb.Duration `protobuf:"bytes,3,opt,name=timeout,proto3" json:"timeout,omitempty"` - Cors *Server_CORS `protobuf:"bytes,4,opt,name=cors,proto3" json:"cors,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -385,13 +393,6 @@ func (x *Server_HTTP) GetTimeout() *durationpb.Duration { return nil } -func (x *Server_HTTP) GetCors() *Server_CORS { - if x != nil { - return x.Cors - } - return nil -} - type Server_CORS struct { state protoimpl.MessageState `protogen:"open.v1"` AllowOrigins []string `protobuf:"bytes,1,rep,name=allow_origins,json=allowOrigins,proto3" json:"allow_origins,omitempty"` @@ -572,16 +573,16 @@ const file_conf_proto_rawDesc = "" + "\x06sentry\x18\x01 \x01(\v2\x1f.Bootstrap.Observability.SentryR\x06sentry\x1a<\n" + "\x06Sentry\x12\x10\n" + "\x03dsn\x18\x01 \x01(\tR\x03dsn\x12 \n" + - "\venvironment\x18\x02 \x01(\tR\venvironment\"\x9a\x04\n" + + "\venvironment\x18\x02 \x01(\tR\venvironment\"\x99\x04\n" + "\x06Server\x12 \n" + "\x04http\x18\x01 \x01(\v2\f.Server.HTTPR\x04http\x12 \n" + "\x04grpc\x18\x02 \x01(\v2\f.Server.GRPCR\x04grpc\x12/\n" + - "\fhttp_metrics\x18\x03 \x01(\v2\f.Server.HTTPR\vhttpMetrics\x1a\x8b\x01\n" + + "\fhttp_metrics\x18\x03 \x01(\v2\f.Server.HTTPR\vhttpMetrics\x12 \n" + + "\x04cors\x18\x04 \x01(\v2\f.Server.CORSR\x04cors\x1ai\n" + "\x04HTTP\x12\x18\n" + "\anetwork\x18\x01 \x01(\tR\anetwork\x12\x12\n" + "\x04addr\x18\x02 \x01(\tR\x04addr\x123\n" + - "\atimeout\x18\x03 \x01(\v2\x19.google.protobuf.DurationR\atimeout\x12 \n" + - "\x04cors\x18\x04 \x01(\v2\f.Server.CORSR\x04cors\x1a+\n" + + "\atimeout\x18\x03 \x01(\v2\x19.google.protobuf.DurationR\atimeout\x1a+\n" + "\x04CORS\x12#\n" + "\rallow_origins\x18\x01 \x03(\tR\fallowOrigins\x1aH\n" + "\x03TLS\x12 \n" + @@ -632,9 +633,9 @@ var file_conf_proto_depIdxs = []int32{ 5, // 4: Server.http:type_name -> Server.HTTP 8, // 5: Server.grpc:type_name -> Server.GRPC 5, // 6: Server.http_metrics:type_name -> Server.HTTP - 4, // 7: Bootstrap.Observability.sentry:type_name -> Bootstrap.Observability.Sentry - 10, // 8: Server.HTTP.timeout:type_name -> google.protobuf.Duration - 6, // 9: Server.HTTP.cors:type_name -> Server.CORS + 6, // 7: Server.cors:type_name -> Server.CORS + 4, // 8: Bootstrap.Observability.sentry:type_name -> Bootstrap.Observability.Sentry + 10, // 9: Server.HTTP.timeout:type_name -> google.protobuf.Duration 10, // 10: Server.GRPC.timeout:type_name -> google.protobuf.Duration 7, // 11: Server.GRPC.tls_config:type_name -> Server.TLS 12, // [12:12] is the sub-list for method output_type diff --git a/app/artifact-cas/internal/conf/conf.proto b/app/artifact-cas/internal/conf/conf.proto index 065aedded..ff07c967c 100644 --- a/app/artifact-cas/internal/conf/conf.proto +++ b/app/artifact-cas/internal/conf/conf.proto @@ -42,7 +42,6 @@ message Server { string network = 1; string addr = 2; google.protobuf.Duration timeout = 3; - CORS cors = 4; } message CORS { repeated string allow_origins = 1; @@ -64,6 +63,8 @@ message Server { GRPC grpc = 2; // hHTTP server where the prometheus metrics will get exposed HTTP http_metrics = 3; + // CORS configuration for the HTTP download endpoint + CORS cors = 4; } message Auth { diff --git a/app/artifact-cas/internal/server/cors.go b/app/artifact-cas/internal/server/cors.go index 665bcc4fd..c32bef2c5 100644 --- a/app/artifact-cas/internal/server/cors.go +++ b/app/artifact-cas/internal/server/cors.go @@ -53,15 +53,17 @@ func CORSMiddleware(allowedOrigins []string, next http.Handler) http.Handler { } } + // Always set Vary: Origin when the response depends on the Origin header, + // so HTTP caches don't serve a cached response for the wrong origin. + // Use Add to avoid clobbering any existing Vary values. + w.Header().Add("Vary", "Origin") + if matchedOrigin == "" { next.ServeHTTP(w, r) return } w.Header().Set("Access-Control-Allow-Origin", matchedOrigin) - if matchedOrigin != "*" { - w.Header().Set("Vary", "Origin") - } if r.Method == http.MethodOptions { w.Header().Set("Access-Control-Allow-Methods", "GET, HEAD, OPTIONS") diff --git a/app/artifact-cas/internal/server/cors_test.go b/app/artifact-cas/internal/server/cors_test.go index 9a83652b1..505a5d879 100644 --- a/app/artifact-cas/internal/server/cors_test.go +++ b/app/artifact-cas/internal/server/cors_test.go @@ -82,6 +82,7 @@ func TestCORSMiddleware(t *testing.T) { method: http.MethodGet, origin: "http://evil.com", wantStatus: http.StatusOK, + wantVary: true, wantHandler: true, }, { diff --git a/app/artifact-cas/internal/server/http.go b/app/artifact-cas/internal/server/http.go index cbcf14e29..56b0a902d 100644 --- a/app/artifact-cas/internal/server/http.go +++ b/app/artifact-cas/internal/server/http.go @@ -66,7 +66,7 @@ func NewHTTPServer(c *conf.Server, authConf *conf.Auth, downloadSvc *service.Dow srv := http.NewServer(opts...) downloadHandler := middlewares_http.AuthFromQueryParam(loadPublicKey(rawKey), claimsFunc(), casJWT.SigningMethod, downloadSvc) - srv.Handle(service.DownloadPath, CORSMiddleware(c.Http.GetCors().GetAllowOrigins(), downloadHandler)) + srv.Handle(service.DownloadPath, CORSMiddleware(c.GetCors().GetAllowOrigins(), downloadHandler)) api.RegisterStatusServiceHTTPServer(srv, service.NewStatusService(Version, providers)) return srv, nil } diff --git a/deployment/chainloop/templates/cas/configmap.yaml b/deployment/chainloop/templates/cas/configmap.yaml index e77416d4d..fef2c170e 100644 --- a/deployment/chainloop/templates/cas/configmap.yaml +++ b/deployment/chainloop/templates/cas/configmap.yaml @@ -20,21 +20,6 @@ data: # Timeouts for http downloads # grpc downloads/uploads don't require this because they don't have timeouts timeout: 300s - {{- if .Values.cas.allowedOrigins }} - cors: - allow_origins: - {{- if kindIs "string" .Values.cas.allowedOrigins }} - {{- range splitList "," .Values.cas.allowedOrigins }} - {{- if . }} - - {{ trim . | quote }} - {{- end }} - {{- end }} - {{- else }} - {{- range .Values.cas.allowedOrigins }} - - {{ . | quote }} - {{- end }} - {{- end }} - {{- end }} grpc: {{- if include "cas.tls-secret-name" . }} tls_config: @@ -46,3 +31,18 @@ data: timeout: 5s http_metrics: addr: "0.0.0.0:{{ .Values.cas.containerPorts.metrics}}" + {{- if .Values.cas.allowedOrigins }} + cors: + allow_origins: + {{- if kindIs "string" .Values.cas.allowedOrigins }} + {{- range splitList "," .Values.cas.allowedOrigins }} + {{- if (trim .) }} + - {{ trim . | quote }} + {{- end }} + {{- end }} + {{- else }} + {{- range .Values.cas.allowedOrigins }} + - {{ . | quote }} + {{- end }} + {{- end }} + {{- end }}