Skip to content

Commit

Permalink
Merge pull request #618 from runatlantis/redact-cloneurl
Browse files Browse the repository at this point in the history 
Redact clone url during clone errors.
  • Loading branch information
@lkysow
lkysow authored May 7, 2019
2 parents 4ccb694 + d0308a5 commit c1c01ea
Show file tree
Hide file tree
Showing 5 changed files with 43 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
15 changes: 9 additions & 6 deletions server/events/working_dir.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,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 +228,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 c1c01ea

Please sign in to comment.