Sanitize logs for mirror sync (#3057, #3082) #3078
Conversation
| LGTM |
tboerger added the lgtm/need 1 
| // GitConfigPath returns the repository git config path | ||
| func (repo *Repository) GitConfigPath() string { | ||
| return filepath.Join(repo.RepoPath(), "config") | ||
| return GitConfigPath(repo.RepoPath()) |
There was a problem hiding this comment.
I just moved the logic to a helper function (GitConfigPath) so that it can be reused.
| m.NextUpdate = time.Now().Add(m.Interval) | ||
| } | ||
| | ||
| func remoteAddress(repoPath string) (string, error) { |
There was a problem hiding this comment.
(not required change) This function should be moved to go-gitea/git at some point...
But, have it run git remote get-url origin instead of reading the config...
There was a problem hiding this comment.
Esp. since the config-file is not INI-format... If we wanna do this in pure-go we should consider using this: https://github.com/tcnksm/go-gitconfig
There was a problem hiding this comment.
I agree that parsing .git/config with a INI-parser is not ideal, but let's address it in a separate PR, so as to not block this backport.
models/repo_mirror.go Outdated
| m.address = cfg.Section("remote \"origin\"").Key("url").Value() | ||
| } | ||
| | ||
| // HandleCloneUserCredentials replaces user credentials from HTTP/HTTPS URL |
There was a problem hiding this comment.
Can we call this SanitizeUserCredentials instead? 😕
There was a problem hiding this comment.
This entire function should be rewritten to use url.Parse and do url.User = nil instead 🙄
There was a problem hiding this comment.
Fixed in #3082, which I've included in this backport.
| @bkcsoft Valid points; I'll open up a separate PR to address them, since this a backport. |
lafriks added the topic/security 
ethantkoenig changed the title 
| @bkcsoft could you review this again so that we may release v1.3.1 next week. |
| LGTM |
tboerger added lgtm/done 
| Make LG-TM work |
5 participants
Backport of #3057 and #3082.