Skip to content

Commit

Permalink
Redact clone url during clone errors.
Browse files Browse the repository at this point in the history 
If there was an error during cloning, we would sometimes print the clone
credentials to the pull request and in the Atlantis logs. This change
ensures we print the redacted clone url.

We also change the redacted clone url to contain "username:<redacted>"
so that if users see the redacted url in logs, they'll know we were
using a credentialed form and so if they try to replicate with curl
they'll know to substitute the correct credentials.
  • Loading branch information
@lkysow
lkysow committed May 7, 2019
1 parent 4ccb694 commit e9c2210
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 36 deletions.
44 changes: 22 additions & 22 deletions server/events/event_parser_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestParseGithubRepo(t *testing.T) {
Owner: "owner",
FullName: "owner/repo",
CloneURL: "https://github-user:[email protected]/owner/repo.git",
SanitizedCloneURL: Repo.GetCloneURL(),
SanitizedCloneURL: "https://github-user:<redacted>@github.com/owner/repo.git",
Name: "repo",
VCSHost: models.VCSHost{
Hostname: "github.com",
Expand Down Expand Up @@ -96,7 +96,7 @@ func TestParseGithubIssueCommentEvent(t *testing.T) {
Owner: *comment.Repo.Owner.Login,
FullName: *comment.Repo.FullName,
CloneURL: "https://github-user:[email protected]/owner/repo.git",
SanitizedCloneURL: *comment.Repo.CloneURL,
SanitizedCloneURL: "https://github-user:<redacted>@github.com/owner/repo.git",
Name: "repo",
VCSHost: models.VCSHost{
Hostname: "github.com",
Expand Down Expand Up @@ -134,7 +134,7 @@ func TestParseGithubPullEvent(t *testing.T) {
Owner: "owner",
FullName: "owner/repo",
CloneURL: "https://github-user:[email protected]/owner/repo.git",
SanitizedCloneURL: Repo.GetCloneURL(),
SanitizedCloneURL: "https://github-user:<redacted>@github.com/owner/repo.git",
Name: "repo",
VCSHost: models.VCSHost{
Hostname: "github.com",
Expand Down Expand Up @@ -252,7 +252,7 @@ func TestParseGithubPull(t *testing.T) {
Owner: "owner",
FullName: "owner/repo",
CloneURL: "https://github-user:[email protected]/owner/repo.git",
SanitizedCloneURL: Repo.GetCloneURL(),
SanitizedCloneURL: "https://github-user:<redacted>@github.com/owner/repo.git",
Name: "repo",
VCSHost: models.VCSHost{
Hostname: "github.com",
Expand Down Expand Up @@ -287,7 +287,7 @@ func TestParseGitlabMergeEvent(t *testing.T) {
expBaseRepo := models.Repo{
FullName: "lkysow/atlantis-example",
Name: "atlantis-example",
SanitizedCloneURL: "https://gitlab.com/lkysow/atlantis-example.git",
SanitizedCloneURL: "https://gitlab-user:<redacted>@gitlab.com/lkysow/atlantis-example.git",
Owner: "lkysow",
CloneURL: "https://gitlab-user:[email protected]/lkysow/atlantis-example.git",
VCSHost: models.VCSHost{
Expand All @@ -312,7 +312,7 @@ func TestParseGitlabMergeEvent(t *testing.T) {
Equals(t, models.Repo{
FullName: "sourceorg/atlantis-example",
Name: "atlantis-example",
SanitizedCloneURL: "https://gitlab.com/sourceorg/atlantis-example.git",
SanitizedCloneURL: "https://gitlab-user:<redacted>@gitlab.com/sourceorg/atlantis-example.git",
Owner: "sourceorg",
CloneURL: "https://gitlab-user:[email protected]/sourceorg/atlantis-example.git",
VCSHost: models.VCSHost{
Expand Down Expand Up @@ -344,7 +344,7 @@ func TestParseGitlabMergeEvent_Subgroup(t *testing.T) {
expBaseRepo := models.Repo{
FullName: "lkysow-test/subgroup/sub-subgroup/atlantis-example",
Name: "atlantis-example",
SanitizedCloneURL: "https://gitlab.com/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
SanitizedCloneURL: "https://gitlab-user:<redacted>@gitlab.com/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
Owner: "lkysow-test/subgroup/sub-subgroup",
CloneURL: "https://gitlab-user:[email protected]/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
VCSHost: models.VCSHost{
Expand All @@ -369,7 +369,7 @@ func TestParseGitlabMergeEvent_Subgroup(t *testing.T) {
Equals(t, models.Repo{
FullName: "lkysow-test/subgroup/sub-subgroup/atlantis-example",
Name: "atlantis-example",
SanitizedCloneURL: "https://gitlab.com/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
SanitizedCloneURL: "https://gitlab-user:<redacted>@gitlab.com/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
Owner: "lkysow-test/subgroup/sub-subgroup",
CloneURL: "https://gitlab-user:[email protected]/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
VCSHost: models.VCSHost{
Expand Down Expand Up @@ -440,7 +440,7 @@ func TestParseGitlabMergeRequest(t *testing.T) {
repo := models.Repo{
FullName: "gitlabhq/gitlab-test",
Name: "gitlab-test",
SanitizedCloneURL: "https://example.com/gitlabhq/gitlab-test.git",
SanitizedCloneURL: "https://gitlab-user:<redacted>@example.com/gitlabhq/gitlab-test.git",
Owner: "gitlabhq",
CloneURL: "https://gitlab-user:[email protected]/gitlabhq/gitlab-test.git",
VCSHost: models.VCSHost{
Expand Down Expand Up @@ -479,7 +479,7 @@ func TestParseGitlabMergeRequest_Subgroup(t *testing.T) {
repo := models.Repo{
FullName: "lkysow-test/subgroup/sub-subgroup/atlantis-example",
Name: "atlantis-example",
SanitizedCloneURL: "https://gitlab.com/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
SanitizedCloneURL: "https://gitlab-user:<redacted>@gitlab.com/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
Owner: "lkysow-test/subgroup/sub-subgroup",
CloneURL: "https://gitlab-user:[email protected]/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
VCSHost: models.VCSHost{
Expand Down Expand Up @@ -513,7 +513,7 @@ func TestParseGitlabMergeCommentEvent(t *testing.T) {
Equals(t, models.Repo{
FullName: "gitlabhq/gitlab-test",
Name: "gitlab-test",
SanitizedCloneURL: "https://example.com/gitlabhq/gitlab-test.git",
SanitizedCloneURL: "https://gitlab-user:<redacted>@example.com/gitlabhq/gitlab-test.git",
Owner: "gitlabhq",
CloneURL: "https://gitlab-user:[email protected]/gitlabhq/gitlab-test.git",
VCSHost: models.VCSHost{
Expand All @@ -524,7 +524,7 @@ func TestParseGitlabMergeCommentEvent(t *testing.T) {
Equals(t, models.Repo{
FullName: "gitlab-org/gitlab-test",
Name: "gitlab-test",
SanitizedCloneURL: "https://example.com/gitlab-org/gitlab-test.git",
SanitizedCloneURL: "https://gitlab-user:<redacted>@example.com/gitlab-org/gitlab-test.git",
Owner: "gitlab-org",
CloneURL: "https://gitlab-user:[email protected]/gitlab-org/gitlab-test.git",
VCSHost: models.VCSHost{
Expand All @@ -551,7 +551,7 @@ func TestParseGitlabMergeCommentEvent_Subgroup(t *testing.T) {
Equals(t, models.Repo{
FullName: "lkysow-test/subgroup/sub-subgroup/atlantis-example",
Name: "atlantis-example",
SanitizedCloneURL: "https://gitlab.com/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
SanitizedCloneURL: "https://gitlab-user:<redacted>@gitlab.com/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
Owner: "lkysow-test/subgroup/sub-subgroup",
CloneURL: "https://gitlab-user:[email protected]/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
VCSHost: models.VCSHost{
Expand All @@ -562,7 +562,7 @@ func TestParseGitlabMergeCommentEvent_Subgroup(t *testing.T) {
Equals(t, models.Repo{
FullName: "lkysow-test/subgroup/sub-subgroup/atlantis-example",
Name: "atlantis-example",
SanitizedCloneURL: "https://gitlab.com/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
SanitizedCloneURL: "https://gitlab-user:<redacted>@gitlab.com/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
Owner: "lkysow-test/subgroup/sub-subgroup",
CloneURL: "https://gitlab-user:[email protected]/lkysow-test/subgroup/sub-subgroup/atlantis-example.git",
VCSHost: models.VCSHost{
Expand Down Expand Up @@ -707,7 +707,7 @@ func TestParseBitbucketCloudCommentEvent_ValidEvent(t *testing.T) {
Owner: "lkysow",
Name: "atlantis-example",
CloneURL: "https://bitbucket-user:[email protected]/lkysow/atlantis-example.git",
SanitizedCloneURL: "https://bitbucket.org/lkysow/atlantis-example.git",
SanitizedCloneURL: "https://bitbucket-user:<redacted>@bitbucket.org/lkysow/atlantis-example.git",
VCSHost: models.VCSHost{
Hostname: "bitbucket.org",
Type: models.BitbucketCloud,
Expand All @@ -729,7 +729,7 @@ func TestParseBitbucketCloudCommentEvent_ValidEvent(t *testing.T) {
Owner: "lkysow-fork",
Name: "atlantis-example",
CloneURL: "https://bitbucket-user:[email protected]/lkysow-fork/atlantis-example.git",
SanitizedCloneURL: "https://bitbucket.org/lkysow-fork/atlantis-example.git",
SanitizedCloneURL: "https://bitbucket-user:<redacted>@bitbucket.org/lkysow-fork/atlantis-example.git",
VCSHost: models.VCSHost{
Hostname: "bitbucket.org",
Type: models.BitbucketCloud,
Expand Down Expand Up @@ -793,7 +793,7 @@ func TestParseBitbucketCloudPullEvent_ValidEvent(t *testing.T) {
Owner: "lkysow",
Name: "atlantis-example",
CloneURL: "https://bitbucket-user:[email protected]/lkysow/atlantis-example.git",
SanitizedCloneURL: "https://bitbucket.org/lkysow/atlantis-example.git",
SanitizedCloneURL: "https://bitbucket-user:<redacted>@bitbucket.org/lkysow/atlantis-example.git",
VCSHost: models.VCSHost{
Hostname: "bitbucket.org",
Type: models.BitbucketCloud,
Expand All @@ -815,7 +815,7 @@ func TestParseBitbucketCloudPullEvent_ValidEvent(t *testing.T) {
Owner: "lkysow-fork",
Name: "atlantis-example",
CloneURL: "https://bitbucket-user:[email protected]/lkysow-fork/atlantis-example.git",
SanitizedCloneURL: "https://bitbucket.org/lkysow-fork/atlantis-example.git",
SanitizedCloneURL: "https://bitbucket-user:<redacted>@bitbucket.org/lkysow-fork/atlantis-example.git",
VCSHost: models.VCSHost{
Hostname: "bitbucket.org",
Type: models.BitbucketCloud,
Expand Down Expand Up @@ -894,7 +894,7 @@ func TestParseBitbucketServerCommentEvent_ValidEvent(t *testing.T) {
Owner: "atlantis",
Name: "atlantis-example",
CloneURL: "http://bitbucket-user:[email protected]:7490/scm/at/atlantis-example.git",
SanitizedCloneURL: "http://mycorp.com:7490/scm/at/atlantis-example.git",
SanitizedCloneURL: "http://bitbucket-user:<redacted>@mycorp.com:7490/scm/at/atlantis-example.git",
VCSHost: models.VCSHost{
Hostname: "mycorp.com",
Type: models.BitbucketServer,
Expand All @@ -916,7 +916,7 @@ func TestParseBitbucketServerCommentEvent_ValidEvent(t *testing.T) {
Owner: "atlantis-fork",
Name: "atlantis-example",
CloneURL: "http://bitbucket-user:[email protected]:7490/scm/fk/atlantis-example.git",
SanitizedCloneURL: "http://mycorp.com:7490/scm/fk/atlantis-example.git",
SanitizedCloneURL: "http://bitbucket-user:<redacted>@mycorp.com:7490/scm/fk/atlantis-example.git",
VCSHost: models.VCSHost{
Hostname: "mycorp.com",
Type: models.BitbucketServer,
Expand Down Expand Up @@ -976,7 +976,7 @@ func TestParseBitbucketServerPullEvent_ValidEvent(t *testing.T) {
Owner: "atlantis",
Name: "atlantis-example",
CloneURL: "http://bitbucket-user:[email protected]:7490/scm/at/atlantis-example.git",
SanitizedCloneURL: "http://mycorp.com:7490/scm/at/atlantis-example.git",
SanitizedCloneURL: "http://bitbucket-user:<redacted>@mycorp.com:7490/scm/at/atlantis-example.git",
VCSHost: models.VCSHost{
Hostname: "mycorp.com",
Type: models.BitbucketServer,
Expand All @@ -998,7 +998,7 @@ func TestParseBitbucketServerPullEvent_ValidEvent(t *testing.T) {
Owner: "atlantis-fork",
Name: "atlantis-example",
CloneURL: "http://bitbucket-user:[email protected]:7490/scm/fk/atlantis-example.git",
SanitizedCloneURL: "http://mycorp.com:7490/scm/fk/atlantis-example.git",
SanitizedCloneURL: "http://bitbucket-user:<redacted>@mycorp.com:7490/scm/fk/atlantis-example.git",
VCSHost: models.VCSHost{
Hostname: "mycorp.com",
Type: models.BitbucketServer,
Expand Down
10 changes: 7 additions & 3 deletions server/events/models/models.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ type Repo struct {
// CloneURL is the full HTTPS url for cloning with username and token string
// ex. "https://username:[email protected]/atlantis/atlantis.git".
CloneURL string
// SanitizedCloneURL is the full HTTPS url for cloning without the username and password.
// ex. "https://github.com/atlantis/atlantis.git".
// SanitizedCloneURL is the full HTTPS url for cloning with the password
// redacted.
// ex. "https://user:<redacted>@github.com/atlantis/atlantis.git".
SanitizedCloneURL string
// VCSHost is where this repo is hosted.
VCSHost VCSHost
Expand Down Expand Up @@ -95,11 +96,14 @@ func NewRepo(vcsHostType VCSHostType, repoFullName string, cloneURL string, vcsU
escapedVCSUser := url.QueryEscape(vcsUser)
escapedVCSToken := url.QueryEscape(vcsToken)
auth := fmt.Sprintf("%s:%s@", escapedVCSUser, escapedVCSToken)
redactedAuth := fmt.Sprintf("%s:<redacted>@", escapedVCSUser)

// Construct clone urls with http and https auth. Need to do both
// because Bitbucket supports http.
authedCloneURL := strings.Replace(cloneURL, "https://", "https://"+auth, -1)
authedCloneURL = strings.Replace(authedCloneURL, "http://", "http://"+auth, -1)
sanitizedCloneURL := strings.Replace(cloneURL, "https://", "https://"+redactedAuth, -1)
sanitizedCloneURL = strings.Replace(sanitizedCloneURL, "http://", "http://"+redactedAuth, -1)

// Get the owner and repo names from the full name.
owner, repo := SplitRepoFullName(repoFullName)
Expand All @@ -120,7 +124,7 @@ func NewRepo(vcsHostType VCSHostType, repoFullName string, cloneURL string, vcsU
Owner: owner,
Name: repo,
CloneURL: authedCloneURL,
SanitizedCloneURL: cloneURL,
SanitizedCloneURL: sanitizedCloneURL,
VCSHost: VCSHost{
Type: vcsHostType,
Hostname: cloneURLParsed.Hostname(),
Expand Down
8 changes: 4 additions & 4 deletions server/events/models/models_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestNewRepo_CloneURLBitbucketServer(t *testing.T) {
Owner: "owner",
Name: "repo",
CloneURL: "http://u:[email protected]:7990/scm/at/atlantis-example.git",
SanitizedCloneURL: "http://mycorp.com:7990/scm/at/atlantis-example.git",
SanitizedCloneURL: "http://u:<redacted>@mycorp.com:7990/scm/at/atlantis-example.git",
VCSHost: models.VCSHost{
Hostname: "mycorp.com",
Type: models.BitbucketServer,
Expand Down Expand Up @@ -104,7 +104,7 @@ func TestNewRepo_MissingDotGit(t *testing.T) {
repo, err := models.NewRepo(models.BitbucketCloud, "owner/repo", "https://bitbucket.org/owner/repo", "u", "p")
Ok(t, err)
Equals(t, repo.CloneURL, "https://u:[email protected]/owner/repo.git")
Equals(t, repo.SanitizedCloneURL, "https://bitbucket.org/owner/repo.git")
Equals(t, repo.SanitizedCloneURL, "https://u:<redacted>@bitbucket.org/owner/repo.git")
}

func TestNewRepo_HTTPAuth(t *testing.T) {
Expand All @@ -116,7 +116,7 @@ func TestNewRepo_HTTPAuth(t *testing.T) {
Hostname: "github.com",
Type: models.Github,
},
SanitizedCloneURL: "http://github.com/owner/repo.git",
SanitizedCloneURL: "http://u:<redacted>@github.com/owner/repo.git",
CloneURL: "http://u:[email protected]/owner/repo.git",
FullName: "owner/repo",
Owner: "owner",
Expand All @@ -133,7 +133,7 @@ func TestNewRepo_HTTPSAuth(t *testing.T) {
Hostname: "github.com",
Type: models.Github,
},
SanitizedCloneURL: "https://github.com/owner/repo.git",
SanitizedCloneURL: "https://u:<redacted>@github.com/owner/repo.git",
CloneURL: "https://u:[email protected]/owner/repo.git",
FullName: "owner/repo",
Owner: "owner",
Expand Down
17 changes: 11 additions & 6 deletions server/events/working_dir.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ func (w *FileWorkspace) forceClone(log *logging.SimpleLogger,

// During testing, we mock some of this out.
headCloneURL := headRepo.CloneURL
headCloneURL = "https://lkysow:[email protected]/lkysow/automergetest.git"
headRepo.CloneURL = headCloneURL
if w.TestingOverrideHeadCloneURL != "" {
headCloneURL = w.TestingOverrideHeadCloneURL
}
Expand Down Expand Up @@ -179,12 +181,14 @@ func (w *FileWorkspace) forceClone(log *logging.SimpleLogger,
"GIT_COMMITTER_NAME=atlantis",
}...)

cmdStr := w.cmdAsSanitizedStr(cmd, p.BaseRepo, headRepo)
cmdStr := w.sanitizeGitCredentials(strings.Join(cmd.Args, " "), p.BaseRepo, headRepo)
output, err := cmd.CombinedOutput()
sanitizedOutput := w.sanitizeGitCredentials(string(output), p.BaseRepo, headRepo)
if err != nil {
return "", errors.Wrapf(err, "running %s: %s", cmdStr, string(output))
sanitizedErrMsg := w.sanitizeGitCredentials(err.Error(), p.BaseRepo, headRepo)
return "", fmt.Errorf("running %s: %s: %s", cmdStr, sanitizedOutput, sanitizedErrMsg)
}
log.Debug("ran: %s. Output: %s", cmdStr, strings.TrimSuffix(string(output), "\n"))
log.Debug("ran: %s. Output: %s", cmdStr, strings.TrimSuffix(sanitizedOutput, "\n"))
}
return cloneDir, nil
}
Expand Down Expand Up @@ -226,8 +230,9 @@ func (w *FileWorkspace) cloneDir(r models.Repo, p models.PullRequest, workspace
return filepath.Join(w.repoPullDir(r, p), workspace)
}

func (w *FileWorkspace) cmdAsSanitizedStr(cmd *exec.Cmd, base models.Repo, head models.Repo) string {
cmdAsStr := strings.Join(cmd.Args, " ")
baseReplaced := strings.Replace(cmdAsStr, base.CloneURL, base.SanitizedCloneURL, -1)
// sanitizeGitCredentials replaces any git clone urls that contain credentials
// in s with the sanitized versions.
func (w *FileWorkspace) sanitizeGitCredentials(s string, base models.Repo, head models.Repo) string {
baseReplaced := strings.Replace(s, base.CloneURL, base.SanitizedCloneURL, -1)
return strings.Replace(baseReplaced, head.CloneURL, head.SanitizedCloneURL, -1)
}
2 changes: 1 addition & 1 deletion server/events_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ func TestPost_BBServerPullClosed(t *testing.T) {
Owner: "project",
Name: "repository",
CloneURL: "https://bb-user:[email protected]/scm/proj/repository.git",
SanitizedCloneURL: "https://bbserver.com/scm/proj/repository.git",
SanitizedCloneURL: "https://bb-user:<redacted>@bbserver.com/scm/proj/repository.git",
VCSHost: models.VCSHost{
Hostname: "bbserver.com",
Type: models.BitbucketServer,
Expand Down

0 comments on commit e9c2210

Please sign in to comment.