feat(security): add audit logging to clients in pipelines-as-code

Log access using access tokens which allows users to review those logs.

Author: mbpavan

pkg/provider/bitbucketcloud/bitbucket.go

@@ -203,6 +203,10 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, event *info.Eve
 		return fmt.Errorf("no git_provider.user has been in repo crd")
 	}
 	v.bbClient = bitbucket.NewBasicAuth(event.Provider.User, event.Provider.Token)
+
+	// Added log for security audit purposes to log client access when a token is used
+	run.Clients.Log.Infof("bitbucket-cloud: initialized client with provided token for user=%s", event.Provider.User)
+
 	v.Token = &event.Provider.Token
 	v.Username = &event.Provider.User
 	v.run = run

pkg/provider/bitbucketcloud/bitbucket_test.go

@@ -1,11 +1,13 @@
 package bitbucketcloud
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
 	"github.com/ktrysmt/go-bitbucket"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/clients"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
@@ -145,14 +147,29 @@ func TestSetClient(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			ctx, _ := rtesting.SetupFakeContext(t)
+			core, observer := zapobserver.New(zap.InfoLevel)
+			testLog := zap.New(core).Sugar()
+
+			fakeRun := &params.Run{
+				Clients: clients.Clients{
+					Log: testLog,
+				},
+			}
+
 			v := Provider{}
-			err := v.SetClient(ctx, nil, tt.event, nil, nil)
+			err := v.SetClient(ctx, fakeRun, tt.event, nil, nil)
+
 			if tt.wantErrSubstr != "" {
 				assert.ErrorContains(t, err, tt.wantErrSubstr)
 				return
 			}
 			assert.Equal(t, tt.event.Provider.Token, *v.Token)
 			assert.Equal(t, tt.event.Provider.User, *v.Username)
+
+			logs := observer.TakeAll()
+			assert.Assert(t, len(logs) > 0, "expected a log entry, got none")
+			expected := fmt.Sprintf("bitbucket-cloud: initialized client with provided token for user=%s", tt.event.Provider.User)
+			assert.Equal(t, expected, logs[0].Message)
 		})
 	}
 }

pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go

@@ -307,6 +307,9 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
 			},
 		}
 		v.client = client
+
+		// Added for security audit purposes to log client access when a token is used
+		run.Clients.Log.Infof("bitbucket-datacenter: initialized client with provided token for user=%s providerURL=%s", event.Provider.User, event.Provider.URL)
 	}
 	v.run = run
 	v.repo = repo

pkg/provider/bitbucketdatacenter/bitbucketdatacenter_test.go

@@ -15,6 +15,7 @@ import (
 	"testing"
 
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params"
+	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/clients"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/info"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/settings"
 	"github.com/openshift-pipelines/pipelines-as-code/pkg/params/triggertype"
@@ -362,14 +363,21 @@ func TestSetClient(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			observer, _ := zapobserver.New(zap.InfoLevel)
+			testLog := zap.New(observer).Sugar()
+			fakeRun := &params.Run{
+				Clients: clients.Clients{
+					Log: testLog,
+				},
+			}
 			ctx, _ := rtesting.SetupFakeContext(t)
 			client, mux, tearDown, tURL := bbtest.SetupBBDataCenterClient()
 			defer tearDown()
 			if tt.muxUser != nil {
 				mux.HandleFunc("/users/foo", tt.muxUser)
 			}
 			v := &Provider{client: client, baseURL: tURL}
-			err := v.SetClient(ctx, nil, tt.opts, nil, nil)
+			err := v.SetClient(ctx, fakeRun, tt.opts, nil, nil)
 			if tt.wantErrSubstr != "" {
 				assert.ErrorContains(t, err, tt.wantErrSubstr)
 				return

pkg/provider/gitea/gitea.go

@@ -159,6 +159,10 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, runevent *info.
 	if err != nil {
 		return err
 	}
+
+	// Added log for security audit purposes to log client access when a token is used
+	run.Clients.Log.Infof("gitea: initialized API client with provided credentials user=%s providerURL=%s", runevent.Provider.User, apiURL)
+
 	v.giteaInstanceURL = runevent.Provider.URL
 	v.eventEmitter = emitter
 	v.repo = repo

pkg/provider/github/github.go

@@ -301,6 +301,13 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
 		return fmt.Errorf("no github client has been initialized")
 	}
 
+	// Added log for security audit purposes to log client access when a token is used
+	integration := "github-webhook"
+	if event.InstallationID != 0 {
+		integration = "github-app"
+	}
+	run.Clients.Log.Infof(integration+": initialized OAuth2 client for providerName=%s providerURL=%s", v.providerName, event.Provider.URL)
+
 	v.APIURL = apiURL
 
 	if event.Provider.WebhookSecretFromRepo {

pkg/provider/github/github_test.go

@@ -657,10 +657,11 @@ func TestGithubGetCommitInfo(t *testing.T) {
 
 func TestGithubSetClient(t *testing.T) {
 	tests := []struct {
-		name        string
-		event       *info.Event
-		expectedURL string
-		isGHE       bool
+		name           string
+		event          *info.Event
+		expectedURL    string
+		isGHE          bool
+		installationID int64
 	}{
 		{
 			name: "api url set",
@@ -669,20 +670,30 @@ func TestGithubSetClient(t *testing.T) {
 					URL: "foo.com",
 				},
 			},
-			expectedURL: "https://foo.com",
-			isGHE:       true,
+			expectedURL:    "https://foo.com",
+			isGHE:          true,
+			installationID: 0,
 		},
 		{
-			name:        "default to public github",
-			expectedURL: fmt.Sprintf("%s/", keys.PublicGithubAPIURL),
-			event:       info.NewEvent(),
+			name:           "default to public github",
+			expectedURL:    fmt.Sprintf("%s/", keys.PublicGithubAPIURL),
+			event:          info.NewEvent(),
+			installationID: 12345,
 		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			tt.event.InstallationID = tt.installationID
 			ctx, _ := rtesting.SetupFakeContext(t)
+			core, observer := zapobserver.New(zap.InfoLevel)
+			testLog := zap.New(core).Sugar()
+			fakeRun := &params.Run{
+				Clients: clients.Clients{
+					Log: testLog,
+				},
+			}
 			v := Provider{}
-			err := v.SetClient(ctx, nil, tt.event, nil, nil)
+			err := v.SetClient(ctx, fakeRun, tt.event, nil, nil)
 			assert.NilError(t, err)
 			assert.Equal(t, tt.expectedURL, *v.APIURL)
 			assert.Equal(t, "https", v.Client().BaseURL.Scheme)
@@ -691,6 +702,33 @@ func TestGithubSetClient(t *testing.T) {
 			} else {
 				assert.Equal(t, "/", v.Client().BaseURL.Path)
 			}
+
+			logs := observer.TakeAll()
+			assert.Assert(t, len(logs) == 1, "expected exactly one log entry, got %d", len(logs))
+
+			prefix := "github-webhook"
+			if tt.installationID != 0 {
+				prefix = "github-app"
+			}
+			wantStart := fmt.Sprintf("%s: initialized OAuth2 client", prefix)
+			got := logs[0].Message
+			assert.Assert(t, strings.HasPrefix(got, wantStart), "log entry should start with %q, got %q", wantStart, got)
+
+			// Determine expected providerName based on whether it's GHE or public GitHub.
+			expectedProviderName := "github"
+			if tt.isGHE {
+				expectedProviderName = "github-enterprise"
+			}
+
+			// Build the full expected log message.
+			fullExpected := fmt.Sprintf(
+				"%s: initialized OAuth2 client for providerName=%s providerURL=%s",
+				prefix,
+				expectedProviderName,
+				tt.event.Provider.URL,
+			)
+
+			assert.Equal(t, fullExpected, logs[0].Message)
 		})
 	}
 }

pkg/provider/gitlab/gitlab.go

@@ -201,6 +201,7 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, runevent *info.
 	}
 	v.Token = &runevent.Provider.Token
 
+	run.Clients.Log.Infof("gitlab: initialized for client with token for apiURL=%s, org=%s, repo=%s", apiURL, runevent.Organization, runevent.Repository)
 	// In a scenario where the source repository is forked and a merge request (MR) is created on the upstream
 	// repository, runevent.SourceProjectID will not be 0 when SetClient is called from the pac-watcher code.
 	// This is because, in the controller, SourceProjectID is set in the annotation of the pull request,

pkg/provider/gitlab/gitlab_test.go

@@ -210,6 +210,7 @@ func TestCreateStatus(t *testing.T) {
 			run := &params.Run{
 				Clients: clients.Clients{
 					Kube: stdata.Kube,
+					Log:  logger,
 				},
 			}
 			v := &Provider{
@@ -261,23 +262,54 @@ func TestGetConfig(t *testing.T) {
 
 func TestSetClient(t *testing.T) {
 	ctx, _ := rtesting.SetupFakeContext(t)
+	core, observer := zapobserver.New(zap.InfoLevel)
+	fakelogger := zap.New(core).Sugar()
+
+	run := &params.Run{
+		Clients: clients.Clients{
+			Log: fakelogger,
+		},
+	}
+
 	v := &Provider{}
-	assert.Assert(t, v.SetClient(ctx, nil, info.NewEvent(), nil, nil) != nil)
+	assert.Assert(t, v.SetClient(ctx, run, info.NewEvent(), nil, nil) != nil)
 
 	client, _, tearDown := thelp.Setup(t)
 	defer tearDown()
+
 	vv := &Provider{gitlabClient: client}
-	err := vv.SetClient(ctx, nil, &info.Event{
+	err := vv.SetClient(ctx, run, &info.Event{
 		Provider: &info.Provider{
 			Token: "hello",
 		},
+		Organization:    "my-org",
+		Repository:      "my-repo",
+		SourceProjectID: 1234,
+		TargetProjectID: 1234,
 	}, nil, nil)
+
 	assert.NilError(t, err)
 	assert.Assert(t, *vv.Token != "")
+
+	logs := observer.TakeAll()
+	assert.Assert(t, len(logs) > 0, "expected a log entry, got none")
+
+	expected := fmt.Sprintf(
+		"gitlab: initialized for client with token for apiURL=%s, org=%s, repo=%s",
+		vv.apiURL, "my-org", "my-repo")
+
+	assert.Equal(t, expected, logs[0].Message)
 }
 
 func TestSetClientDetectAPIURL(t *testing.T) {
 	ctx, _ := rtesting.SetupFakeContext(t)
+	observer, _ := zapobserver.New(zap.InfoLevel)
+	fakelogger := zap.New(observer).Sugar()
+	run := &params.Run{
+		Clients: clients.Clients{
+			Log: fakelogger,
+		},
+	}
 	mockClient, _, tearDown := thelp.Setup(t)
 	defer tearDown()
 
@@ -394,7 +426,7 @@ func TestSetClientDetectAPIURL(t *testing.T) {
 
 			// Execute the function under test
 			// Using placeholder nil values for arguments not directly related to URL detection
-			err := v.SetClient(ctx, nil, event, nil, nil)
+			err := v.SetClient(ctx, run, event, nil, nil)
 
 			// Assertions
 			if tc.expectedError != "" {

pkg/reconciler/reconciler.go

@@ -173,6 +173,9 @@ func (r *Reconciler) reportFinalStatus(ctx context.Context, logger *zap.SugaredL
 		}
 	}
 
+	if r.run.Clients.Log == nil {
+		r.run.Clients.Log = logger
+	}
 	err = provider.SetClient(ctx, r.run, event, repo, r.eventEmitter)
 	if err != nil {
 		return repo, fmt.Errorf("cannot set client: %w", err)