From 35b218fe10eac0683249a6fb6334dfeaf0e12021 Mon Sep 17 00:00:00 2001 From: Matheus Macabu Date: Fri, 21 Feb 2025 14:28:21 +0100 Subject: [PATCH 1/2] Chore: Bump glog in pkg/codegen to v1.2.4 to fix vulnerability (#101126) Chore: Bump glog pkg/codegen to v1.2.4 to fix vulnerability --- go.work.sum | 6 ++---- pkg/codegen/go.mod | 2 +- pkg/codegen/go.sum | 4 ++-- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/go.work.sum b/go.work.sum index 5ef1c177e66..7dc97c7b7bb 100644 --- a/go.work.sum +++ b/go.work.sum @@ -578,6 +578,8 @@ github.com/gofrs/flock v0.8.1/go.mod h1:F1TvTiK9OcQqauNUHlbJvyl9Qa1QvF/gOUDKA14j github.com/golang-jwt/jwt v3.2.1+incompatible h1:73Z+4BJcrTC+KczS6WvTPvRGOp1WmfEP4Q1lOd9Z/+c= github.com/golang-jwt/jwt v3.2.1+incompatible/go.mod h1:8pz2t5EyA70fFQQSrl6XZXzqecmYZeUEB8OUGHkxJ+I= github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0 h1:DACJavvAHhabrF08vX0COfcOBJRhZ8lUbR+ZWIs0Y5g= +github.com/golang/glog v1.2.3/go.mod h1:6AhwSGph0fcJtXVM/PEHPqZlFeoLxhs7/t5UDAwmO+w= +github.com/golang/glog v1.2.4/go.mod h1:6AhwSGph0fcJtXVM/PEHPqZlFeoLxhs7/t5UDAwmO+w= github.com/gomarkdown/markdown v0.0.0-20230716120725-531d2d74bc12 h1:uK3X/2mt4tbSGoHvbLBHUny7CKiuwUip3MArtukol4E= github.com/gomarkdown/markdown v0.0.0-20230716120725-531d2d74bc12/go.mod h1:JDGcbDT52eL4fju3sZ4TeHGsQwhG9nbDV21aMyhwPoA= github.com/gomodule/redigo v1.8.9 h1:Sl3u+2BI/kk+VEatbj0scLdrFhjPmbxOc1myhDP41ws= @@ -617,10 +619,6 @@ github.com/grafana/cog v0.0.23/go.mod h1:jrS9indvWuDs60RHEZpLaAkmZdgyoLKMOEUT0ji github.com/grafana/go-gelf/v2 v2.0.1 h1:BOChP0h/jLeD+7F9mL7tq10xVkDG15he3T1zHuQaWak= github.com/grafana/go-gelf/v2 v2.0.1/go.mod h1:lexHie0xzYGwCgiRGcvZ723bSNyNI8ZRD4s0CLobh90= github.com/grafana/grafana/apps/advisor v0.0.0-20250220154326-6e5de80ef295/go.mod h1:9I1dKV3Dqr0NPR9Af0WJGxOytp5/6W3JLiNChOz8r+c= -github.com/grafana/grafana/apps/advisor v0.0.0-20250220163425-b4c4b9abbdc8/go.mod h1:9I1dKV3Dqr0NPR9Af0WJGxOytp5/6W3JLiNChOz8r+c= -github.com/grafana/grafana/apps/alerting/notifications v0.0.0-20250220163425-b4c4b9abbdc8/go.mod h1:dHhFF484qs1cmdIShKCB3kl+tMJyc4yuwgTQ3Afz37o= -github.com/grafana/grafana/apps/investigations v0.0.0-20250220163425-b4c4b9abbdc8/go.mod h1:ygFcJP2McdSeMJVj/3YrKafZMc/lZBsp54HO51MtJYw= -github.com/grafana/grafana/apps/playlist v0.0.0-20250220164708-c8d4ff28a450/go.mod h1:KKIsWpbv88Lwwcvdjon73zFL7vNJvuXLtsSoUjJErTw= github.com/grafana/grafana/pkg/build v0.0.0-20250220114259-be81314e2118/go.mod h1:STVpVboMYeBAfyn6Zw6XHhTHqUxzMy7pzRiVgk1l0W0= github.com/grafana/tail v0.0.0-20230510142333-77b18831edf0 h1:bjh0PVYSVVFxzINqPFYJmAmJNrWPgnVjuSdYJGHmtFU= github.com/grafana/tail v0.0.0-20230510142333-77b18831edf0/go.mod h1:7t5XR+2IA8P2qggOAHTj/GCZfoLBle3OvNSYh1VkRBU= diff --git a/pkg/codegen/go.mod b/pkg/codegen/go.mod index abb8a0fcbf1..acc83fd50d8 100644 --- a/pkg/codegen/go.mod +++ b/pkg/codegen/go.mod @@ -20,7 +20,7 @@ require ( github.com/getkin/kin-openapi v0.129.0 // indirect github.com/go-openapi/jsonpointer v0.21.0 // indirect github.com/go-openapi/swag v0.23.0 // indirect - github.com/golang/glog v1.2.3 // indirect + github.com/golang/glog v1.2.4 // indirect github.com/google/go-cmp v0.6.0 // indirect github.com/google/uuid v1.6.0 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect diff --git a/pkg/codegen/go.sum b/pkg/codegen/go.sum index ef8c7f75b3d..445e9e138eb 100644 --- a/pkg/codegen/go.sum +++ b/pkg/codegen/go.sum @@ -22,8 +22,8 @@ github.com/go-openapi/swag v0.23.0/go.mod h1:esZ8ITTYEsH1V2trKHjAN8Ai7xHb8RV+YSZ github.com/go-test/deep v1.0.8 h1:TDsG77qcSprGbC6vTN8OuXp5g+J+b5Pcguhf7Zt61VM= github.com/go-test/deep v1.0.8/go.mod h1:5C2ZWiW0ErCdrYzpqxLbTX7MG14M9iiw8DgHncVwcsE= github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q= -github.com/golang/glog v1.2.3 h1:oDTdz9f5VGVVNGu/Q7UXKWYsD0873HXLHdJUNBsSEKM= -github.com/golang/glog v1.2.3/go.mod h1:6AhwSGph0fcJtXVM/PEHPqZlFeoLxhs7/t5UDAwmO+w= +github.com/golang/glog v1.2.4 h1:CNNw5U8lSiiBk7druxtSHHTsRWcxKoac6kZKm2peBBc= +github.com/golang/glog v1.2.4/go.mod h1:6AhwSGph0fcJtXVM/PEHPqZlFeoLxhs7/t5UDAwmO+w= github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= From 77350ce84f68611da904f321399f1bc231f6cea7 Mon Sep 17 00:00:00 2001 From: Matheus Macabu Date: Fri, 21 Feb 2025 14:29:52 +0100 Subject: [PATCH 2/2] CloudMigrations: Address CodeQL issue on unsanitized request params (#101125) --- .../cloudmigration/gmsclient/gms_client.go | 44 +++++++++++++++---- .../gmsclient/gms_client_test.go | 22 +++++++++- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/pkg/services/cloudmigration/gmsclient/gms_client.go b/pkg/services/cloudmigration/gmsclient/gms_client.go index 441e8b1d9e6..18f4c0539a7 100644 --- a/pkg/services/cloudmigration/gmsclient/gms_client.go +++ b/pkg/services/cloudmigration/gmsclient/gms_client.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "net/http" + "net/url" "strings" "sync" "time" @@ -40,7 +41,10 @@ type gmsClientImpl struct { func (c *gmsClientImpl) ValidateKey(ctx context.Context, cm cloudmigration.CloudMigrationSession) (err error) { // TODO: there is a lot of boilerplate code in these methods, we should consolidate them when we have a gardening period - path := fmt.Sprintf("%s/api/v1/validate-key", c.buildBasePath(cm.ClusterSlug)) + path, err := c.buildURL(cm.ClusterSlug, "/api/v1/validate-key") + if err != nil { + return err + } ctx, cancel := context.WithTimeout(ctx, c.cfg.CloudMigration.GMSValidateKeyTimeout) defer cancel() @@ -78,7 +82,10 @@ func (c *gmsClientImpl) ValidateKey(ctx context.Context, cm cloudmigration.Cloud } func (c *gmsClientImpl) StartSnapshot(ctx context.Context, session cloudmigration.CloudMigrationSession) (out *cloudmigration.StartSnapshotResponse, err error) { - path := fmt.Sprintf("%s/api/v1/start-snapshot", c.buildBasePath(session.ClusterSlug)) + path, err := c.buildURL(session.ClusterSlug, "/api/v1/start-snapshot") + if err != nil { + return nil, err + } ctx, cancel := context.WithTimeout(ctx, c.cfg.CloudMigration.GMSStartSnapshotTimeout) defer cancel() @@ -123,7 +130,11 @@ func (c *gmsClientImpl) GetSnapshotStatus(ctx context.Context, session cloudmigr c.getStatusMux.Lock() defer c.getStatusMux.Unlock() - path := fmt.Sprintf("%s/api/v1/snapshots/%s/status?offset=%d", c.buildBasePath(session.ClusterSlug), snapshot.GMSSnapshotUID, offset) + path, err := c.buildURL(session.ClusterSlug, fmt.Sprintf("/api/v1/snapshots/%s/status?offset=%d", snapshot.GMSSnapshotUID, offset)) + if err != nil { + c.log.Error("error parsing snapshot status url", "err", err.Error()) + return nil, err + } ctx, cancel := context.WithTimeout(ctx, c.cfg.CloudMigration.GMSGetSnapshotStatusTimeout) defer cancel() @@ -167,7 +178,11 @@ func (c *gmsClientImpl) GetSnapshotStatus(ctx context.Context, session cloudmigr } func (c *gmsClientImpl) CreatePresignedUploadUrl(ctx context.Context, session cloudmigration.CloudMigrationSession, snapshot cloudmigration.CloudMigrationSnapshot) (string, error) { - path := fmt.Sprintf("%s/api/v1/snapshots/%s/create-upload-url", c.buildBasePath(session.ClusterSlug), snapshot.GMSSnapshotUID) + path, err := c.buildURL(session.ClusterSlug, fmt.Sprintf("/api/v1/snapshots/%s/create-upload-url", snapshot.GMSSnapshotUID)) + if err != nil { + c.log.Error("error parsing upload url", "err", err.Error()) + return "", err + } ctx, cancel := context.WithTimeout(ctx, c.cfg.CloudMigration.GMSCreateUploadUrlTimeout) defer cancel() @@ -217,7 +232,11 @@ func (c *gmsClientImpl) ReportEvent(ctx context.Context, session cloudmigration. ctx, cancel := context.WithTimeout(ctx, c.cfg.CloudMigration.GMSReportEventTimeout) defer cancel() - path := fmt.Sprintf("%s/api/v1/events", c.buildBasePath(session.ClusterSlug)) + path, err := c.buildURL(session.ClusterSlug, "/api/v1/events") + if err != nil { + c.log.Error("parsing events url", "err", err.Error()) + return + } var buf bytes.Buffer if err := json.NewEncoder(&buf).Encode(event); err != nil { @@ -255,12 +274,21 @@ func (c *gmsClientImpl) ReportEvent(ctx context.Context, session cloudmigration. }() } -func (c *gmsClientImpl) buildBasePath(clusterSlug string) string { +func (c *gmsClientImpl) buildURL(clusterSlug, path string) (string, error) { domain := c.cfg.CloudMigration.GMSDomain + baseURL := fmt.Sprintf("https://cms-%s.%s/cloud-migrations", clusterSlug, domain) + + // Override the host if we are configuring it with a scheme prefix. if strings.HasPrefix(domain, "http://") || strings.HasPrefix(domain, "https://") { - return domain + baseURL = domain } - return fmt.Sprintf("https://cms-%s.%s/cloud-migrations", clusterSlug, domain) + + parsed, err := url.Parse(baseURL + path) + if err != nil { + return "", fmt.Errorf("building url: %w", err) + } + + return parsed.String(), nil } // handleGMSErrors parses the error message from GMS and translates it to an appropriate error message diff --git a/pkg/services/cloudmigration/gmsclient/gms_client_test.go b/pkg/services/cloudmigration/gmsclient/gms_client_test.go index d89d8d2acfb..449c64decf7 100644 --- a/pkg/services/cloudmigration/gmsclient/gms_client_test.go +++ b/pkg/services/cloudmigration/gmsclient/gms_client_test.go @@ -10,7 +10,7 @@ import ( "github.com/stretchr/testify/require" ) -func Test_buildBasePath(t *testing.T) { +func Test_buildURL(t *testing.T) { t.Parallel() // Domain is required @@ -38,6 +38,7 @@ func Test_buildBasePath(t *testing.T) { description string domain string clusterSlug string + path string expected string }{ { @@ -52,17 +53,34 @@ func Test_buildBasePath(t *testing.T) { clusterSlug: "anything", expected: "https://some-domain:8080", }, + { + description: "domain starts with https://, should return domain", + domain: "https://some-domain:8080", + clusterSlug: "anything", + path: "/test?foo=bar&baz=qax#fragment", + expected: "https://some-domain:8080/test?foo=bar&baz=qax#fragment", + }, { description: "domain doesn't start with http or https, should build a string using the domain and clusterSlug", domain: "gms-dev", clusterSlug: "us-east-1", expected: "https://cms-us-east-1.gms-dev/cloud-migrations", }, + { + description: "it parses and escapes the path when building the URL", + domain: "gms-dev", + clusterSlug: "use-east-1", + path: `/this//is//a/\very-Nice_páTh?x=/çç&y=/éé#aaaa`, + expected: "https://cms-use-east-1.gms-dev/cloud-migrations/this//is//a/%5Cvery-Nice_p%C3%A1Th?x=/çç&y=/éé#aaaa", + }, } for _, tt := range tests { t.Run(tt.description, func(t *testing.T) { client.cfg.CloudMigration.GMSDomain = tt.domain - assert.Equal(t, tt.expected, client.buildBasePath(tt.clusterSlug)) + + url, err := client.buildURL(tt.clusterSlug, tt.path) + assert.NoError(t, err) + assert.Equal(t, tt.expected, url) }) } }