Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redact clone url during clone errors. #618

Merged
merged 1 commit into from
May 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions server/events/event_parser_test.go
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
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:/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
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:/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:/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
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
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