Skip to content

Commit

Permalink
Try not to log credentials in repo URL
Browse files Browse the repository at this point in the history
This extracts the username and password from the URL and replaces it
with the bare URL, using the usual credential mechanism for user/pass.
  • Loading branch information
thockin committed Dec 14, 2023
1 parent 65e0953 commit d5c61f7
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 3 deletions.
26 changes: 23 additions & 3 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,11 @@ func main() {
if *flPassword != "" && *flPasswordFile != "" {
handleConfigError(log, true, "ERROR: only one of --password and --password-file may be specified")
}
if u, err := url.Parse(*flRepo); err == nil { // it may not even parse as a URL, that's OK
if u.User != nil {
handleConfigError(log, true, "ERROR: credentials may not be specified in --repo when --username is specified")
}
}
} else {
if *flPassword != "" {
handleConfigError(log, true, "ERROR: --password may only be specified when --username is specified")
Expand Down Expand Up @@ -564,6 +569,21 @@ func main() {
absTouchFile := makeAbsPath(*flTouchFile, absRoot)

// Merge credential sources.
if *flUsername == "" {
// username and user@host URLs are validated as mutually exclusive
if u, err := url.Parse(*flRepo); err == nil { // it may not even parse as a URL, that's OK
if u.User != nil {
if user := u.User.Username(); user != "" {
*flUsername = user
}
if pass, found := u.User.Password(); found {
*flPassword = pass
}
u.User = nil
*flRepo = u.String()
}
}
}
if *flUsername != "" {
cred := credential{
URL: *flRepo,
Expand Down Expand Up @@ -1535,7 +1555,7 @@ func (git *repoSync) currentWorktree() (worktree, error) {
// and tries to clean up any detritus. This function returns whether the
// current hash has changed and what the new hash is.
func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Context) error) (bool, string, error) {
git.log.V(3).Info("syncing", "repo", git.repo)
git.log.V(3).Info("syncing", "repo", redactURL(git.repo))

if err := refreshCreds(ctx); err != nil {
return false, "", fmt.Errorf("credential refresh failed: %w", err)
Expand Down Expand Up @@ -1660,7 +1680,7 @@ func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Con

// fetch retrieves the specified ref from the upstream repo.
func (git *repoSync) fetch(ctx context.Context, ref string) error {
git.log.V(2).Info("fetching", "ref", ref, "repo", git.repo)
git.log.V(2).Info("fetching", "ref", ref, "repo", redactURL(git.repo))

// Fetch the ref and do some cleanup, setting or un-setting the repo's
// shallow flag as appropriate.
Expand Down Expand Up @@ -1711,7 +1731,7 @@ func md5sum(s string) string {

// StoreCredentials stores a username and password for later use.
func (git *repoSync) StoreCredentials(ctx context.Context, url, username, password string) error {
git.log.V(1).Info("storing git credential", "url", url)
git.log.V(1).Info("storing git credential", "url", redactURL(url))
git.log.V(9).Info("md5 of credential", "url", url, "username", md5sum(username), "password", md5sum(password))

creds := fmt.Sprintf("url=%v\nusername=%v\npassword=%v\n", url, username, password)
Expand Down
40 changes: 40 additions & 0 deletions test_e2e.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1812,6 +1812,46 @@ function e2e::auth_http_password() {
assert_file_eq "$ROOT/link/file" "$FUNCNAME"
}

##############################################
# Test HTTP basicauth with a password in the URL
##############################################
function e2e::auth_http_password_in_url() {
# Run a git-over-HTTP server.
CTR=$(docker_run \
-v "$REPO":/git/repo:ro \
e2e/test/httpd)
IP=$(docker_ip "$CTR")

# Try with wrong username
assert_fail \
GIT_SYNC \
--one-time \
--repo="http://wrong:testpass@$IP/repo" \
--root="$ROOT" \
--link="link"
assert_file_absent "$ROOT/link/file"

# Try with wrong password
assert_fail \
GIT_SYNC \
--one-time \
--repo="http://testuser:wrong@$IP/repo" \
--root="$ROOT" \
--link="link"
assert_file_absent "$ROOT/link/file"

# Try with the right password
GIT_SYNC \
--one-time \
--repo="http://testuser:testpass@$IP/repo" \
--root="$ROOT" \
--link="link"

assert_link_exists "$ROOT/link"
assert_file_exists "$ROOT/link/file"
assert_file_eq "$ROOT/link/file" "$FUNCNAME"
}

##############################################
# Test HTTP basicauth with a password-file
##############################################
Expand Down

0 comments on commit d5c61f7

Please sign in to comment.