From dc993be1a6db2fe65aa8e0ba1d157d4c9f2b797d Mon Sep 17 00:00:00 2001 From: Zackary Santana <64446617+ZackarySantana@users.noreply.github.com> Date: Wed, 25 Sep 2024 11:22:58 -0700 Subject: [PATCH] DEVPROD-10509 Update git.clone project command to redact for modules (#8330) --- agent/agent.go | 5 +- agent/command/git.go | 38 ++++---- agent/command/git_push_test.go | 9 +- agent/command/git_test.go | 128 +++++++++++++++++---------- agent/internal/client/base_client.go | 2 +- agent/internal/client/client_test.go | 98 ++++++++++++++++++++ agent/internal/client/mock.go | 9 +- agent/util/expansions.go | 25 ++++-- config.go | 2 +- model/project.go | 5 -- 10 files changed, 230 insertions(+), 91 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 43430148bf2..f6b376a0816 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -600,11 +600,8 @@ func (a *Agent) startLogging(ctx context.Context, tc *taskContext) error { taskLogDir := filepath.Join(a.opts.WorkingDirectory, taskLogDirectory) grip.Error(errors.Wrapf(os.RemoveAll(taskLogDir), "removing task log directory '%s'", taskLogDir)) tc.logger, err = a.makeLoggerProducer(ctx, tc, "") - if err != nil { - return errors.Wrap(err, "making the logger producer") - } - return nil + return errors.Wrap(err, "making the logger producer") } // runTask runs a task. It returns true if the agent should exit. diff --git a/agent/command/git.go b/agent/command/git.go index b3c781df96d..77ff7e2911e 100644 --- a/agent/command/git.go +++ b/agent/command/git.go @@ -42,6 +42,8 @@ const ( // Valid types of performing git clone cloneMethodOAuth = "oauth" cloneMethodAccessToken = "access-token" + + generatedTokenKey = "EVERGREEN_GENERATED_GITHUB_TOKEN" ) var ( @@ -158,6 +160,10 @@ func getProjectMethodAndToken(ctx context.Context, comm client.Communicator, td owner := conf.ProjectRef.Owner repo := conf.ProjectRef.Repo appToken, err := comm.CreateInstallationToken(ctx, td, owner, repo) + if appToken != "" { + // Redact the token from the logs. + conf.NewExpansions.Redact(generatedTokenKey, appToken) + } // TODO EVG-21022: Remove fallback once we delete GitHub tokens as expansions. grip.Warning(message.WrapError(err, message.Fields{ "message": "error creating GitHub app token, falling back to legacy clone methods", @@ -241,10 +247,9 @@ func (opts cloneOpts) buildHTTPCloneCommand(forApp bool) ([]string, error) { clone = fmt.Sprintf("%s --branch '%s'", clone, opts.branch) } - redactedClone := strings.Replace(clone, opts.token, "[redacted github token]", -1) return []string{ "set +o xtrace", - fmt.Sprintf(`echo %s`, strconv.Quote(redactedClone)), + fmt.Sprintf(`echo %s`, strconv.Quote(clone)), clone, "set -o xtrace", fmt.Sprintf("cd %s", opts.dir), @@ -543,16 +548,11 @@ func (c *gitFetchProject) fetchSource(ctx context.Context, // return early and will stop waiting for the command to exit. In the // context error case, this thread and the still-running command may race to // read/write the buffer, so the buffer has to be thread-safe. - stdErr := utility.MakeSafeBuffer(bytes.Buffer{}) fetchSourceCmd := jpm.CreateCommand(ctx).Add([]string{"bash", "-c", fetchScript}).Directory(conf.WorkDir). - SetOutputSender(level.Info, logger.Task().GetSender()).SetErrorWriter(stdErr) + SetOutputSender(level.Info, logger.Task().GetSender()).SetErrorSender(level.Error, logger.Execution().GetSender()) logger.Execution().Info("Fetching source from git...") - redactedCmds := fetchScript - if opts.token != "" { - redactedCmds = strings.Replace(redactedCmds, opts.token, "[redacted github token]", -1) - } - logger.Execution().Debugf("Commands are: %s", redactedCmds) + logger.Execution().Debugf("Commands are: %s", fetchScript) ctx, span := getTracer().Start(ctx, "clone_source", trace.WithAttributes( attribute.String(cloneOwnerAttribute, opts.owner), @@ -563,15 +563,7 @@ func (c *gitFetchProject) fetchSource(ctx context.Context, )) defer span.End() - err = fetchSourceCmd.Run(ctx) - out := stdErr.String() - if out != "" { - if opts.token != "" { - out = strings.Replace(out, opts.token, "[redacted github token]", -1) - } - logger.Execution().Error(out) - } - return err + return fetchSourceCmd.Run(ctx) }) } @@ -725,17 +717,19 @@ func (c *gitFetchProject) fetchModuleSource(ctx context.Context, return errors.Wrap(err, "setting location to clone from") } - if opts.method == cloneMethodOAuth { + if opts.method == cloneMethodOAuth || opts.method == cloneMethodAccessToken { // If user provided a token, use that token. opts.token = projectToken } else { - // Otherwise, create an installation token for to clone the module. // Fallback to the legacy global token if the token cannot be created. appToken, err := comm.CreateInstallationToken(ctx, td, opts.owner, opts.repo) if err == nil { opts.token = appToken opts.method = cloneMethodAccessToken + + // After generating, redact the token from the logs. + conf.NewExpansions.Redact(generatedTokenKey, appToken) } else { // If a token cannot be created, fallback to the legacy global token. opts.method = cloneMethodOAuth @@ -791,9 +785,7 @@ func (c *gitFetchProject) fetchModuleSource(ctx context.Context, errOutput := stdErr.String() if errOutput != "" { - if opts.token != "" { - errOutput = strings.Replace(errOutput, opts.token, "[redacted github token]", -1) - } + errOutput = strings.ReplaceAll(errOutput, "\n", fmt.Sprintf("\n%s: ", module.Name)) logger.Execution().Error(fmt.Sprintf("%s: %s", module.Name, errOutput)) } return err diff --git a/agent/command/git_push_test.go b/agent/command/git_push_test.go index 87a9870e0da..500bb228df0 100644 --- a/agent/command/git_push_test.go +++ b/agent/command/git_push_test.go @@ -11,6 +11,7 @@ import ( "github.com/evergreen-ci/evergreen/agent/internal" "github.com/evergreen-ci/evergreen/agent/internal/client" + agentutil "github.com/evergreen-ci/evergreen/agent/util" "github.com/evergreen-ci/evergreen/model" "github.com/evergreen-ci/evergreen/model/patch" "github.com/evergreen-ci/evergreen/model/task" @@ -31,10 +32,12 @@ func TestGitPush(t *testing.T) { } comm := client.NewMock("http://localhost.com") comm.CreateInstallationTokenResult = "token" + expansions := util.Expansions{} conf := &internal.TaskConfig{ - Task: task.Task{}, - ProjectRef: model.ProjectRef{Branch: "main"}, - Expansions: util.Expansions{}, + Task: task.Task{}, + ProjectRef: model.ProjectRef{Branch: "main"}, + Expansions: expansions, + NewExpansions: agentutil.NewDynamicExpansions(expansions), } logger, err := comm.GetLoggerProducer(context.Background(), &conf.Task, nil) require.NoError(t, err) diff --git a/agent/command/git_test.go b/agent/command/git_test.go index 60323010c86..083ae113ac0 100644 --- a/agent/command/git_test.go +++ b/agent/command/git_test.go @@ -14,6 +14,7 @@ import ( "github.com/evergreen-ci/evergreen" "github.com/evergreen-ci/evergreen/agent/internal" "github.com/evergreen-ci/evergreen/agent/internal/client" + "github.com/evergreen-ci/evergreen/agent/internal/redactor" agenttestutil "github.com/evergreen-ci/evergreen/agent/internal/testutil" agentutil "github.com/evergreen-ci/evergreen/agent/util" "github.com/evergreen-ci/evergreen/db" @@ -193,7 +194,7 @@ func (s *GitGetProjectSuite) TestBuildSourceCommandUsesHTTPS() { } s.Require().NoError(opts.setLocation()) cmds, _ := c.buildSourceCloneCommand(s.ctx, s.comm, logger, conf, opts) - s.True(utility.StringSliceContains(cmds, "git clone https://PROJECTTOKEN:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'")) + s.True(utility.StringSliceContains(cmds, fmt.Sprintf("git clone https://%s:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'", projectGitHubToken))) } func (s *GitGetProjectSuite) TestBuildSourceCommandWithHTTPSNeedsToken() { @@ -321,45 +322,80 @@ func (s *GitGetProjectSuite) TestGitFetchRetries() { s.Error(err) } -func (s *GitGetProjectSuite) TestTokenScrubbedFromLogger() { - conf := s.taskConfig1 - logger, err := s.comm.GetLoggerProducer(s.ctx, &conf.Task, nil) - s.Require().NoError(err) +func (s *GitGetProjectSuite) TestTokenIsRedactedWhenGenerated() { + conf := s.taskConfig5 conf.ProjectRef.Repo = "invalidRepo" conf.Distro = nil token := "abcdefghij" s.comm.CreateInstallationTokenResult = token + s.comm.CreateInstallationTokenFail = false ctx, cancel := context.WithCancel(context.Background()) defer cancel() - for _, task := range conf.Project.Tasks { - s.NotEqual(len(task.Commands), 0) - for _, command := range task.Commands { - pluginCmds, err := Render(command, &conf.Project, BlockInfo{}) - s.NoError(err) - s.NotNil(pluginCmds) - pluginCmds[0].SetJasperManager(s.jasper) - err = pluginCmds[0].Execute(ctx, s.comm, logger, conf) - s.Error(err) + runCommands := func(logger client.LoggerProducer) { + for _, task := range conf.Project.Tasks { + s.NotEqual(len(task.Commands), 0) + for _, command := range task.Commands { + pluginCmds, err := Render(command, &conf.Project, BlockInfo{}) + s.NoError(err) + s.NotNil(pluginCmds) + pluginCmds[0].SetJasperManager(s.jasper) + err = pluginCmds[0].Execute(ctx, s.comm, logger, conf) + s.Error(err) + } } } - s.NoError(logger.Close()) - foundCloneCommand := false - foundCloneErr := false - for _, line := range s.comm.GetTaskLogs(conf.Task.Id) { - if strings.Contains(line.Data, "https://[redacted github token]:x-oauth-basic@github.com/evergreen-ci/invalidRepo.git") { - foundCloneCommand = true + findTokenInLogs := func() bool { + for _, line := range s.comm.GetTaskLogs(conf.Task.Id) { + if strings.Contains(line.Data, token) { + return true + } } - if strings.Contains(line.Data, "Authentication failed for") { - foundCloneErr = true - } - if strings.Contains(line.Data, token) { - s.FailNow("token was leaked") + return false + } + + findTokenInRedacted := func() bool { + for _, redacted := range conf.NewExpansions.GetRedacted() { + if redacted.Key == generatedTokenKey && redacted.Value == token { + return true + } } + return false } - s.True(foundCloneCommand) - s.True(foundCloneErr) + + // This is to ensure that the token would be leaked if not redacted. + s.Run("WithoutRedactorShouldLeak", func() { + logger, err := s.comm.GetLoggerProducer(s.ctx, &conf.Task, nil) + s.Require().NoError(err) + runCommands(logger) + s.NoError(logger.Close()) + + // Token should be leaked in logs. + s.True(findTokenInLogs()) + + // Token should be in the redacted list (the + // redactor logger sender is just not using it). + s.True(findTokenInRedacted()) + }) + + s.Run("WithRedactorShouldNotLeak", func() { + logger, err := s.comm.GetLoggerProducer(s.ctx, &conf.Task, &client.LoggerConfig{ + RedactorOpts: redactor.RedactionOptions{ + Expansions: conf.NewExpansions, + }, + }) + s.Require().NoError(err) + + runCommands(logger) + s.NoError(logger.Close()) + + // Token should not be leaked in the logs. + s.False(findTokenInLogs()) + + // Token should be in redacted list. + s.True(findTokenInRedacted()) + }) } func (s *GitGetProjectSuite) TestStdErrLogged() { @@ -461,8 +497,8 @@ func (s *GitGetProjectSuite) TestBuildHTTPCloneCommand() { s.Require().Len(cmds, 5) s.True(utility.ContainsOrderedSubset(cmds, []string{ "set +o xtrace", - "echo \"git clone https://[redacted github token]:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'\"", - "git clone https://PROJECTTOKEN:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'", + fmt.Sprintf("echo \"git clone https://%s:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'\"", projectGitHubToken), + fmt.Sprintf("git clone https://%s:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'", projectGitHubToken), "set -o xtrace", "cd dir", })) @@ -473,8 +509,8 @@ func (s *GitGetProjectSuite) TestBuildHTTPCloneCommand() { s.Require().Len(cmds, 5) s.True(utility.ContainsOrderedSubset(cmds, []string{ "set +o xtrace", - "echo \"git clone https://[redacted github token]:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir'\"", - "git clone https://PROJECTTOKEN:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir'", + fmt.Sprintf("echo \"git clone https://%s:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir'\"", projectGitHubToken), + fmt.Sprintf("git clone https://%s:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir'", projectGitHubToken), "set -o xtrace", "cd dir", })) @@ -487,8 +523,8 @@ func (s *GitGetProjectSuite) TestBuildHTTPCloneCommand() { s.NoError(err) s.Require().Len(cmds, 5) s.True(utility.ContainsOrderedSubset(cmds, []string{ - "echo \"git clone https://[redacted github token]:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'\"", - "git clone https://PROJECTTOKEN:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'", + fmt.Sprintf("echo \"git clone https://%s:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'\"", projectGitHubToken), + fmt.Sprintf("git clone https://%s:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'", projectGitHubToken), })) // ensure that we aren't sending the github oauth token to other @@ -498,8 +534,8 @@ func (s *GitGetProjectSuite) TestBuildHTTPCloneCommand() { s.NoError(err) s.Require().Len(cmds, 5) s.True(utility.ContainsOrderedSubset(cmds, []string{ - "echo \"git clone https://[redacted github token]:x-oauth-basic@someothergithost.com/evergreen-ci/sample.git 'dir' --branch 'main'\"", - "git clone https://PROJECTTOKEN:x-oauth-basic@someothergithost.com/evergreen-ci/sample.git 'dir' --branch 'main'", + fmt.Sprintf("echo \"git clone https://%s:x-oauth-basic@someothergithost.com/evergreen-ci/sample.git 'dir' --branch 'main'\"", projectGitHubToken), + fmt.Sprintf("git clone https://%s:x-oauth-basic@someothergithost.com/evergreen-ci/sample.git 'dir' --branch 'main'", projectGitHubToken), })) } @@ -535,8 +571,8 @@ func (s *GitGetProjectSuite) TestBuildSourceCommand() { "set -o errexit", "rm -rf dir", "set +o xtrace", - "echo \"git clone https://[redacted github token]:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'\"", - "git clone https://PROJECTTOKEN:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'", + fmt.Sprintf("echo \"git clone https://%s:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'\"", projectGitHubToken), + fmt.Sprintf("git clone https://%s:x-oauth-basic@github.com/evergreen-ci/sample.git 'dir' --branch 'main'", projectGitHubToken), "set -o xtrace", "cd dir", "git reset --hard ", @@ -655,8 +691,8 @@ func (s *GitGetProjectSuite) TestBuildModuleCommand() { "set -o xtrace", "set -o errexit", "set +o xtrace", - "echo \"git clone https://[redacted github token]:x-oauth-basic@github.com/evergreen-ci/sample.git 'module'\"", - "git clone https://PROJECTTOKEN:x-oauth-basic@github.com/evergreen-ci/sample.git 'module'", + fmt.Sprintf("echo \"git clone https://%s:x-oauth-basic@github.com/evergreen-ci/sample.git 'module'\"", projectGitHubToken), + fmt.Sprintf("git clone https://%s:x-oauth-basic@github.com/evergreen-ci/sample.git 'module'", projectGitHubToken), "set -o xtrace", "cd module", "git checkout 'main'", @@ -668,8 +704,8 @@ func (s *GitGetProjectSuite) TestBuildModuleCommand() { s.NoError(err) s.Require().Len(cmds, 8) s.True(utility.ContainsOrderedSubset(cmds, []string{ - "echo \"git clone https://[redacted github token]:x-oauth-basic@github.com/evergreen-ci/sample.git 'module'\"", - "git clone https://PROJECTTOKEN:x-oauth-basic@github.com/evergreen-ci/sample.git 'module'", + fmt.Sprintf("echo \"git clone https://%s:x-oauth-basic@github.com/evergreen-ci/sample.git 'module'\"", projectGitHubToken), + fmt.Sprintf("git clone https://%s:x-oauth-basic@github.com/evergreen-ci/sample.git 'module'", projectGitHubToken), })) conf = s.taskConfig4 @@ -689,7 +725,7 @@ func (s *GitGetProjectSuite) TestBuildModuleCommand() { s.True(utility.StringSliceContainsOrderedPrefixSubset(cmds, []string{ "set -o xtrace", "set -o errexit", - "git clone https://x-access-token:PROJECTTOKEN@github.com/evergreen-ci/sample.git 'module'", + fmt.Sprintf("git clone https://x-access-token:%s@github.com/evergreen-ci/sample.git 'module'", projectGitHubToken), "cd module", "git fetch origin \"pull/1234/merge:evg-merge-test-", "git checkout 'evg-merge-test-", @@ -994,14 +1030,16 @@ func (s *GitGetProjectSuite) TestGetProjectMethodAndToken() { td := client.TaskData{ID: s.taskConfig1.Task.Id, Secret: s.taskConfig1.Task.Secret} + expansions := map[string]string{ + evergreen.GlobalGitHubTokenExpansion: globalGitHubToken, + } conf := &internal.TaskConfig{ ProjectRef: model.ProjectRef{ Owner: "valid-owner", Repo: "valid-repo", }, - Expansions: map[string]string{ - evergreen.GlobalGitHubTokenExpansion: globalGitHubToken, - }, + Expansions: expansions, + NewExpansions: agentutil.NewDynamicExpansions(expansions), } method, token, err = getProjectMethodAndToken(s.ctx, s.comm, td, conf, projectGitHubToken, true) diff --git a/agent/internal/client/base_client.go b/agent/internal/client/base_client.go index 8e9e0eaeacf..d0f2143aceb 100644 --- a/agent/internal/client/base_client.go +++ b/agent/internal/client/base_client.go @@ -388,7 +388,7 @@ func (c *baseCommunicator) makeSender(ctx context.Context, tsk *task.Task, confi levelInfo := send.LevelInfo{Default: level.Info, Threshold: level.Debug} var senders []send.Sender if config.SendToGlobalSender { - senders = append(senders, grip.GetSender()) + senders = append(senders, redactor.NewRedactingSender(grip.GetSender(), config.RedactorOpts)) } var sender send.Sender diff --git a/agent/internal/client/client_test.go b/agent/internal/client/client_test.go index f7d63e2a79c..d0759e394fd 100644 --- a/agent/internal/client/client_test.go +++ b/agent/internal/client/client_test.go @@ -2,12 +2,19 @@ package client import ( "context" + "fmt" + "os" + "path/filepath" "testing" "github.com/evergreen-ci/evergreen" + "github.com/evergreen-ci/evergreen/agent/internal/redactor" + agentutil "github.com/evergreen-ci/evergreen/agent/util" "github.com/evergreen-ci/evergreen/model/task" "github.com/evergreen-ci/evergreen/taskoutput" + "github.com/evergreen-ci/evergreen/util" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestEvergreenCommunicatorConstructor(t *testing.T) { @@ -23,6 +30,97 @@ func TestEvergreenCommunicatorConstructor(t *testing.T) { assert.Equal(t, defaultTimeoutMax, c.retry.MaxDelay) } +func TestLoggerProducerRedactorOptions(t *testing.T) { + secret_key := "secret_key" + secret_key_redaction := fmt.Sprintf("", secret_key) + secret := "super_soccer_ball" + createTask := func() *task.Task { + return &task.Task{ + Id: "task", + Project: "project", + TaskOutputInfo: &taskoutput.TaskOutput{ + TaskLogs: taskoutput.TaskLogOutput{ + Version: 1, + BucketConfig: evergreen.BucketConfig{ + Name: t.TempDir(), + Type: evergreen.BucketTypeLocal, + }, + }, + }, + } + } + + readLogs := func(t *testing.T, task *task.Task) string { + logFile := filepath.Join(task.TaskOutputInfo.TaskLogs.BucketConfig.Name, "project", "task", "0", "task_logs", "task") + entries, err := os.ReadDir(logFile) + require.NoError(t, err) + require.Len(t, entries, 1) + + data, err := os.ReadFile(filepath.Join(logFile, entries[0].Name())) + require.NoError(t, err) + + return string(data) + } + + t.Run("LeaksWithoutRedactor", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + task := createTask() + comm := newBaseCommunicator("whatever", map[string]string{}) + logger, err := comm.GetLoggerProducer(ctx, task, nil) + require.NoError(t, err) + + logger.Task().Alert("Fluff 1") + logger.Task().Alert("More fluff") + require.NoError(t, err) + + logger.Task().Info(secret) + logger.Task().Alert("Even more fluff") + require.NoError(t, logger.Close()) + + data := readLogs(t, task) + assert.Contains(t, data, secret) + assert.NotContains(t, data, secret_key_redaction) + + // Make sure it has the other log lines. + assert.Contains(t, data, "Fluff 1") + assert.Contains(t, data, "More fluff") + assert.Contains(t, data, "Even more fluff") + }) + + t.Run("RedactsWhenAdded", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + task := createTask() + comm := newBaseCommunicator("whatever", map[string]string{}) + e := agentutil.NewDynamicExpansions(util.Expansions{}) + logger, err := comm.GetLoggerProducer(ctx, task, &LoggerConfig{ + RedactorOpts: redactor.RedactionOptions{ + Expansions: e, + }, + }) + logger.Task().Alert("Fluff 1") + e.PutAndRedact(secret_key, secret) + logger.Task().Alert("More fluff") + require.NoError(t, err) + + logger.Task().Info(secret) + logger.Task().Alert("Even more fluff") + require.NoError(t, logger.Close()) + + data := readLogs(t, task) + assert.NotContains(t, data, secret) + assert.Contains(t, data, secret_key_redaction) + + // Make sure it has the other log lines. + assert.Contains(t, data, "Fluff 1") + assert.Contains(t, data, "More fluff") + assert.Contains(t, data, "Even more fluff") + }) +} + func TestLoggerClose(t *testing.T) { tsk := &task.Task{ Id: "task", diff --git a/agent/internal/client/mock.go b/agent/internal/client/mock.go index d5f39f1cf92..589c0ee94f6 100644 --- a/agent/internal/client/mock.go +++ b/agent/internal/client/mock.go @@ -11,6 +11,7 @@ import ( "time" "github.com/evergreen-ci/evergreen" + "github.com/evergreen-ci/evergreen/agent/internal/redactor" "github.com/evergreen-ci/evergreen/apimodels" "github.com/evergreen-ci/evergreen/cloud" serviceModel "github.com/evergreen-ci/evergreen/model" @@ -335,7 +336,7 @@ func (c *Mock) GetCedarGRPCConn(ctx context.Context) (*grpc.ClientConn, error) { } // GetLoggerProducer constructs a single channel log producer. -func (c *Mock) GetLoggerProducer(ctx context.Context, tsk *task.Task, _ *LoggerConfig) (LoggerProducer, error) { +func (c *Mock) GetLoggerProducer(ctx context.Context, tsk *task.Task, config *LoggerConfig) (LoggerProducer, error) { if c.GetLoggerProducerShouldFail { return nil, errors.New("operation run in fail mode.") } @@ -351,7 +352,11 @@ func (c *Mock) GetLoggerProducer(ctx context.Context, tsk *task.Task, _ *LoggerC return c.sendTaskLogLine(taskID, line) } - return NewSingleChannelLogHarness(taskID, newMockSender("mock", appendLine)), nil + var sender send.Sender = newMockSender("mock", appendLine) + if config != nil { + sender = redactor.NewRedactingSender(sender, config.RedactorOpts) + } + return NewSingleChannelLogHarness(taskID, sender), nil } // sendTaskLogLine appends a new log line to the task log cache. diff --git a/agent/util/expansions.go b/agent/util/expansions.go index 651876a85b9..df1c2028515 100644 --- a/agent/util/expansions.go +++ b/agent/util/expansions.go @@ -8,6 +8,7 @@ import ( // DynamicExpansions wraps expansions for safe concurrent access as they are // dynamically updated. +// It also contains logic to redact values that should not be exposed. // // This should be expanded to support better expansion handling during a task // run. @@ -15,13 +16,14 @@ type DynamicExpansions struct { util.Expansions mu sync.RWMutex - // redact stores expansions that should be redacted. - // These expansions can be from `expansion.update` or - // from project commands that generate private expansions. - // Since updates can be done dynamically and override previous - // expansions that should be redacted, we need to store them - // separately. Otherwise, previously redacted values can leak - // if they are updated. + // redact stores information about key and value pairs that + // should be redacted. These come from `expansion.update` or + // from project commands that generate sensitive expansions or + // internally generated sensitive expansions that aren't exposed + // to the user. + // Since we can't trust the current state of `util.Expansions` to + // have all the necessary information (because of overwriting), + // we store the redacted information separately. redact []RedactInfo } @@ -91,6 +93,15 @@ func (e *DynamicExpansions) PutAndRedact(key, value string) { } } +// Redact adds a key and value pair to be redacted. This should only be +// used for generated keys not exposed to the task. +func (e *DynamicExpansions) Redact(key, value string) { + e.mu.Lock() + defer e.mu.Unlock() + + e.redact = append(e.redact, RedactInfo{Key: key, Value: value}) +} + // UpdateFromYamlAndRedact updates the expansions from the given yaml file // and then marks the expansions for redaction. func (e *DynamicExpansions) UpdateFromYamlAndRedact(filename string) ([]string, error) { diff --git a/config.go b/config.go index 361ef85e09c..8ee2ed9c60b 100644 --- a/config.go +++ b/config.go @@ -38,7 +38,7 @@ var ( // Agent version to control agent rollover. The format is the calendar date // (YYYY-MM-DD). - AgentVersion = "2024-09-25b" + AgentVersion = "2024-09-25c" ) const ( diff --git a/model/project.go b/model/project.go index 5b681eb2098..4c2b072d9a3 100644 --- a/model/project.go +++ b/model/project.go @@ -620,11 +620,6 @@ func (c *PluginCommandConf) unmarshalParams() error { return nil } -type ArtifactInstructions struct { - Include []string `yaml:"include,omitempty" bson:"include"` - ExcludeFiles []string `yaml:"excludefiles,omitempty" bson:"exclude_files"` -} - type YAMLCommandSet struct { SingleCommand *PluginCommandConf `yaml:"single_command,omitempty" bson:"single_command,omitempty"` MultiCommand []PluginCommandConf `yaml:"multi_command,omitempty" bson:"multi_command,omitempty"`