From 63fd1ef73d6092d9908bdda8cf80ecf8e4af191e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mat=C3=ADas=20Insaurralde?= Date: Tue, 13 Oct 2026 14:01:16 -0300 Subject: [PATCH] fix(controlplane): restrict GUAC credentials to service account keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GUAC integration passed the supplied credentials to the Google Cloud Storage SDK without checking their type. GUAC only needs a service account key, so validate the credential and accept only that type, at registration and on every dispatch. Other credential types (external_account, impersonated_service_account, authorized_user, gdch_service_account) are no longer accepted. The service account key is the only type the GUAC docs describe. Chainloop-Trace-Sessions: d139ac66-b95f-4920-ac38-12997996037d Signed-off-by: Matías Insaurralde --- app/controlplane/plugins/core/guac/v1/guac.go | 39 ++++++++- .../plugins/core/guac/v1/guac_test.go | 86 +++++++++++++++++++ 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/app/controlplane/plugins/core/guac/v1/guac.go b/app/controlplane/plugins/core/guac/v1/guac.go index d4ba3d388..b31ed61ec 100644 --- a/app/controlplane/plugins/core/guac/v1/guac.go +++ b/app/controlplane/plugins/core/guac/v1/guac.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. @@ -80,6 +80,31 @@ func New(l log.Logger) (sdk.FanOut, error) { return &Integration{base}, err } +// gcsServiceAccountType is the only Google credential type GUAC accepts. +const gcsServiceAccountType = "service_account" + +// validateGCSCredentials checks that the supplied credentials are a Google +// service account key, the only type GUAC supports, and rejects anything else. +// +// The Google SDK dispatches on the credential "type" field and handles each +// type differently when it builds the client. GUAC only ever needs a service +// account key to write to a bucket, so the type is checked up front and every +// other type is refused, keeping the credential path narrow and predictable. +func validateGCSCredentials(raw string) error { + var parsed struct { + Type string `json:"type"` + } + if err := json.Unmarshal([]byte(raw), &parsed); err != nil { + return fmt.Errorf("credentials are not valid JSON: %w", err) + } + + if parsed.Type != gcsServiceAccountType { + return fmt.Errorf("unsupported Google credential type %q: only a service account key is accepted", parsed.Type) + } + + return nil +} + // Register is executed when a operator wants to register a specific instance of this integration with their Chainloop organization func (i *Integration) Register(ctx context.Context, req *sdk.RegistrationRequest) (*sdk.RegistrationResponse, error) { i.Logger.Info("registration requested") @@ -90,6 +115,12 @@ func (i *Integration) Register(ctx context.Context, req *sdk.RegistrationRequest return nil, fmt.Errorf("invalid registration request: %w", err) } + // GUAC supports only a service account key. Validate the credential type + // before the Google SDK builds a client from it. + if err := validateGCSCredentials(request.Credentials); err != nil { + return nil, fmt.Errorf("invalid credentials: %w", err) + } + // Check that the credentials are valid and the bucket exists client, err := storage.NewClient(ctx, option.WithCredentialsJSON([]byte(request.Credentials))) if err != nil { @@ -136,6 +167,12 @@ func (i *Integration) Execute(ctx context.Context, req *sdk.ExecutionRequest) er return errors.New("missing expected credentials") } + // Re-check the stored credential on every dispatch, so a registration created + // before this validation is refused here rather than passed to the SDK as-is. + if err := validateGCSCredentials(req.RegistrationInfo.Credentials.Password); err != nil { + return fmt.Errorf("invalid credentials: %w", err) + } + client, err := storage.NewClient(ctx, option.WithCredentialsJSON([]byte(req.RegistrationInfo.Credentials.Password))) if err != nil { return fmt.Errorf("creating storage client: %w", err) diff --git a/app/controlplane/plugins/core/guac/v1/guac_test.go b/app/controlplane/plugins/core/guac/v1/guac_test.go index 265ea64a8..c2e7c454f 100644 --- a/app/controlplane/plugins/core/guac/v1/guac_test.go +++ b/app/controlplane/plugins/core/guac/v1/guac_test.go @@ -135,7 +135,93 @@ func (s *testSuite) TearDownTest() { s.server.Stop() } +// TestRegisterRejectsNonServiceAccountCredentials proves the credential type is +// checked before the Google SDK is built: a non-service-account credential is +// refused by Register with no client created. +func (s *testSuite) TestRegisterRejectsNonServiceAccountCredentials() { + integration, err := New(log.NewStdLogger(io.Discard)) + require.NoError(s.T(), err) + + payload, err := sdk.ToConfig(®istrationRequest{ + Bucket: "some-bucket", + Credentials: `{"type":"external_account"}`, + }) + require.NoError(s.T(), err) + + _, err = integration.(*Integration).Register(context.Background(), &sdk.RegistrationRequest{Payload: payload}) + require.Error(s.T(), err) + assert.ErrorContains(s.T(), err, "invalid credentials") + assert.ErrorContains(s.T(), err, "service account key") +} + // Run all the tests func TestGuacIntegration(t *testing.T) { suite.Run(t, new(testSuite)) } + +func TestValidateGCSCredentials(t *testing.T) { + const wantUnsupportedType = "unsupported Google credential type" + + testCases := []struct { + name string + creds string + wantErr string // substring to expect; empty means the credential is accepted + }{ + { + name: "service account key is accepted", + creds: `{"type":"service_account","project_id":"p"}`, + }, + { + name: "external account is rejected", + creds: `{"type":"external_account"}`, + wantErr: wantUnsupportedType, + }, + { + name: "impersonated service account is rejected", + creds: `{"type":"impersonated_service_account"}`, + wantErr: wantUnsupportedType, + }, + { + name: "external account authorized user is rejected", + creds: `{"type":"external_account_authorized_user"}`, + wantErr: wantUnsupportedType, + }, + { + name: "authorized user is rejected", + creds: `{"type":"authorized_user"}`, + wantErr: wantUnsupportedType, + }, + { + name: "gdch service account is rejected", + creds: `{"type":"gdch_service_account"}`, + wantErr: wantUnsupportedType, + }, + { + name: "missing type is rejected", + creds: `{"project_id":"p"}`, + wantErr: wantUnsupportedType, + }, + { + name: "non-JSON is rejected", + creds: `-----BEGIN PRIVATE KEY-----`, + wantErr: "credentials are not valid JSON", + }, + { + name: "empty string is rejected", + creds: ``, + wantErr: "credentials are not valid JSON", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := validateGCSCredentials(tc.creds) + if tc.wantErr == "" { + assert.NoError(t, err) + return + } + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr) + }) + } +}