Skip to content

Commit

Permalink
DEVPROD-10509 Update git.clone project command to redact for modules (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
ZackarySantana authored Sep 25, 2024
1 parent 7920d7c commit dc993be
Show file tree
Hide file tree
Showing 10 changed files with 230 additions and 91 deletions.
5 changes: 1 addition & 4 deletions agent/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
38 changes: 15 additions & 23 deletions agent/command/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ const (
// Valid types of performing git clone
cloneMethodOAuth = "oauth"
cloneMethodAccessToken = "access-token"

generatedTokenKey = "EVERGREEN_GENERATED_GITHUB_TOKEN"
)

var (
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand All @@ -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)
})
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 6 additions & 3 deletions agent/command/git_push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand Down
128 changes: 83 additions & 45 deletions agent/command/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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",
}))
Expand All @@ -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",
}))
Expand All @@ -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
Expand All @@ -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),
}))
}

Expand Down Expand Up @@ -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 ",
Expand Down Expand Up @@ -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'",
Expand All @@ -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
Expand All @@ -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-",
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion agent/internal/client/base_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit dc993be

Please sign in to comment.