Sanitize logs for mirror sync (#3057, #3082) (#3078)

* Sanitize logs for mirror sync

* Fix error message sanitiziation (#3082)

diff --git a/models/repo.go b/models/repo.go
index 8d57ae51a53076416aeb73f267c72c667ec00963..c570acd6f8a71f1734569b9f5d26eb4866e86956 100644 (file)
--- a/models/repo.go
+++ b/models/repo.go
@@ -605,9 +605,14 @@ func (repo *Repository) RepoPath() string {
        return repo.repoPath(x)
 }
 
+// GitConfigPath returns the path to a repository's git config/ directory
+func GitConfigPath(repoPath string) string {
+       return filepath.Join(repoPath, "config")
+}
+
 // GitConfigPath returns the repository git config path
 func (repo *Repository) GitConfigPath() string {
-       return filepath.Join(repo.RepoPath(), "config")
+       return GitConfigPath(repo.RepoPath())
 }
 
 // RelLink returns the repository relative link

diff --git a/models/repo_mirror.go b/models/repo_mirror.go
index 77cd98faa81c190f175a81f2a159b1cbbd70ee74..f52b3eb452ad71536d107796d7ab0bfdf56a27b0 100644 (file)
--- a/models/repo_mirror.go
+++ b/models/repo_mirror.go
@@ -6,18 +6,18 @@ package models
 
 import (
        "fmt"
-       "strings"
        "time"
 
-       "github.com/Unknwon/com"
-       "github.com/go-xorm/xorm"
-       "gopkg.in/ini.v1"
-
        "code.gitea.io/git"
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/process"
        "code.gitea.io/gitea/modules/setting"
        "code.gitea.io/gitea/modules/sync"
+       "code.gitea.io/gitea/modules/util"
+
+       "github.com/Unknwon/com"
+       "github.com/go-xorm/xorm"
+       "gopkg.in/ini.v1"
 )
 
 // MirrorQueue holds an UniqueQueue object of the mirror
@@ -76,41 +76,41 @@ func (m *Mirror) ScheduleNextUpdate() {
        m.NextUpdate = time.Now().Add(m.Interval)
 }
 
+func remoteAddress(repoPath string) (string, error) {
+       cfg, err := ini.Load(GitConfigPath(repoPath))
+       if err != nil {
+               return "", err
+       }
+       return cfg.Section("remote \"origin\"").Key("url").Value(), nil
+}
+
 func (m *Mirror) readAddress() {
        if len(m.address) > 0 {
                return
        }
-
-       cfg, err := ini.Load(m.Repo.GitConfigPath())
+       var err error
+       m.address, err = remoteAddress(m.Repo.RepoPath())
        if err != nil {
-               log.Error(4, "Load: %v", err)
-               return
+               log.Error(4, "remoteAddress: %v", err)
        }
-       m.address = cfg.Section("remote \"origin\"").Key("url").Value()
 }
 
-// HandleCloneUserCredentials replaces user credentials from HTTP/HTTPS URL
-// with placeholder <credentials>.
-// It will fail for any other forms of clone addresses.
-func HandleCloneUserCredentials(url string, mosaics bool) string {
-       i := strings.Index(url, "@")
-       if i == -1 {
-               return url
-       }
-       start := strings.Index(url, "://")
-       if start == -1 {
-               return url
-       }
-       if mosaics {
-               return url[:start+3] + "<credentials>" + url[i:]
+// sanitizeOutput sanitizes output of a command, replacing occurrences of the
+// repository's remote address with a sanitized version.
+func sanitizeOutput(output, repoPath string) (string, error) {
+       remoteAddr, err := remoteAddress(repoPath)
+       if err != nil {
+               // if we're unable to load the remote address, then we're unable to
+               // sanitize.
+               return "", err
        }
-       return url[:start+3] + url[i+1:]
+       return util.SanitizeMessage(output, remoteAddr), nil
 }
 
 // Address returns mirror address from Git repository config without credentials.
 func (m *Mirror) Address() string {
        m.readAddress()
-       return HandleCloneUserCredentials(m.address, false)
+       return util.SanitizeURLCredentials(m.address, false)
 }
 
 // FullAddress returns mirror address from Git repository config.
@@ -145,7 +145,14 @@ func (m *Mirror) runSync() bool {
        if _, stderr, err := process.GetManager().ExecDir(
                timeout, repoPath, fmt.Sprintf("Mirror.runSync: %s", repoPath),
                "git", gitArgs...); err != nil {
-               desc := fmt.Sprintf("Failed to update mirror repository '%s': %s", repoPath, stderr)
+               // sanitize the output, since it may contain the remote address, which may
+               // contain a password
+               message, err := sanitizeOutput(stderr, repoPath)
+               if err != nil {
+                       log.Error(4, "sanitizeOutput: %v", err)
+                       return false
+               }
+               desc := fmt.Sprintf("Failed to update mirror repository '%s': %s", repoPath, message)
                log.Error(4, desc)
                if err = CreateRepositoryNotice(desc); err != nil {
                        log.Error(4, "CreateRepositoryNotice: %v", err)
@@ -170,7 +177,14 @@ func (m *Mirror) runSync() bool {
                if _, stderr, err := process.GetManager().ExecDir(
                        timeout, wikiPath, fmt.Sprintf("Mirror.runSync: %s", wikiPath),
                        "git", "remote", "update", "--prune"); err != nil {
-                       desc := fmt.Sprintf("Failed to update mirror wiki repository '%s': %s", wikiPath, stderr)
+                       // sanitize the output, since it may contain the remote address, which may
+                       // contain a password
+                       message, err := sanitizeOutput(stderr, wikiPath)
+                       if err != nil {
+                               log.Error(4, "sanitizeOutput: %v", err)
+                               return false
+                       }
+                       desc := fmt.Sprintf("Failed to update mirror wiki repository '%s': %s", wikiPath, message)
                        log.Error(4, desc)
                        if err = CreateRepositoryNotice(desc); err != nil {
                                log.Error(4, "CreateRepositoryNotice: %v", err)

diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go
new file mode 100644 (file)
index 0000000..b1c17b2
--- /dev/null
+++ b/modules/util/sanitize.go
@@ -0,0 +1,48 @@
+// Copyright 2017 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package util
+
+import (
+       "net/url"
+       "strings"
+)
+
+// urlSafeError wraps an error whose message may contain a sensitive URL
+type urlSafeError struct {
+       err            error
+       unsanitizedURL string
+}
+
+func (err urlSafeError) Error() string {
+       return SanitizeMessage(err.err.Error(), err.unsanitizedURL)
+}
+
+// URLSanitizedError returns the sanitized version an error whose message may
+// contain a sensitive URL
+func URLSanitizedError(err error, unsanitizedURL string) error {
+       return urlSafeError{err: err, unsanitizedURL: unsanitizedURL}
+}
+
+// SanitizeMessage sanitizes a message which may contains a sensitive URL
+func SanitizeMessage(message, unsanitizedURL string) string {
+       sanitizedURL := SanitizeURLCredentials(unsanitizedURL, true)
+       return strings.Replace(message, unsanitizedURL, sanitizedURL, -1)
+}
+
+// SanitizeURLCredentials sanitizes a url, either removing user credentials
+// or replacing them with a placeholder.
+func SanitizeURLCredentials(unsanitizedURL string, usePlaceholder bool) string {
+       u, err := url.Parse(unsanitizedURL)
+       if err != nil {
+               // don't log the error, since it might contain unsanitized URL.
+               return "(unparsable url)"
+       }
+       if u.User != nil && usePlaceholder {
+               u.User = url.User("<credentials>")
+       } else {
+               u.User = nil
+       }
+       return u.String()
+}

diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go
index 36e644373c7ca855108f72f5dfe68072c37bca2b..b86921be6da31af88533ac5d1b385bf83b46c971 100644 (file)
--- a/routers/api/v1/repo/repo.go
+++ b/routers/api/v1/repo/repo.go
@@ -9,8 +9,6 @@ import (
        "net/http"
        "strings"
 
-       api "code.gitea.io/sdk/gitea"
-
        "code.gitea.io/gitea/models"
        "code.gitea.io/gitea/modules/auth"
        "code.gitea.io/gitea/modules/context"
@@ -18,6 +16,7 @@ import (
        "code.gitea.io/gitea/modules/setting"
        "code.gitea.io/gitea/modules/util"
        "code.gitea.io/gitea/routers/api/v1/convert"
+       api "code.gitea.io/sdk/gitea"
 )
 
 // Search repositories via options
@@ -322,12 +321,13 @@ func Migrate(ctx *context.APIContext, form auth.MigrateRepoForm) {
                RemoteAddr:  remoteAddr,
        })
        if err != nil {
+               err = util.URLSanitizedError(err, remoteAddr)
                if repo != nil {
                        if errDelete := models.DeleteRepository(ctx.User, ctxUser.ID, repo.ID); errDelete != nil {
                                log.Error(4, "DeleteRepository: %v", errDelete)
                        }
                }
-               ctx.Error(500, "MigrateRepository", models.HandleCloneUserCredentials(err.Error(), true))
+               ctx.Error(500, "MigrateRepository", err)
                return
        }
 

diff --git a/routers/repo/repo.go b/routers/repo/repo.go
index dbe78f6d1e9497c122ae0b26b805e373dc6dbaf5..36105bfe177bab9d0d5242292093bbfd6dc7465b 100644 (file)
--- a/routers/repo/repo.go
+++ b/routers/repo/repo.go
@@ -20,6 +20,7 @@ import (
        "code.gitea.io/gitea/modules/context"
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/setting"
+       "code.gitea.io/gitea/modules/util"
 )
 
 const (
@@ -232,6 +233,9 @@ func MigratePost(ctx *context.Context, form auth.MigrateRepoForm) {
                return
        }
 
+       // remoteAddr may contain credentials, so we sanitize it
+       err = util.URLSanitizedError(err, remoteAddr)
+
        if repo != nil {
                if errDelete := models.DeleteRepository(ctx.User, ctxUser.ID, repo.ID); errDelete != nil {
                        log.Error(4, "DeleteRepository: %v", errDelete)
@@ -241,11 +245,11 @@ func MigratePost(ctx *context.Context, form auth.MigrateRepoForm) {
        if strings.Contains(err.Error(), "Authentication failed") ||
                strings.Contains(err.Error(), "could not read Username") {
                ctx.Data["Err_Auth"] = true
-               ctx.RenderWithErr(ctx.Tr("form.auth_failed", models.HandleCloneUserCredentials(err.Error(), true)), tplMigrate, &form)
+               ctx.RenderWithErr(ctx.Tr("form.auth_failed", err.Error()), tplMigrate, &form)
                return
        } else if strings.Contains(err.Error(), "fatal:") {
                ctx.Data["Err_CloneAddr"] = true
-               ctx.RenderWithErr(ctx.Tr("repo.migrate.failed", models.HandleCloneUserCredentials(err.Error(), true)), tplMigrate, &form)
+               ctx.RenderWithErr(ctx.Tr("repo.migrate.failed", err.Error()), tplMigrate, &form)
                return
        }