diff --git a/modules/git/repo.go b/modules/git/repo.go index 23e9337615..4f2b05bca5 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -18,6 +18,7 @@ import ( "strings" "time" + "forgejo.org/modules/log" "forgejo.org/modules/proxy" "forgejo.org/modules/setting" "forgejo.org/modules/util" @@ -160,24 +161,89 @@ func CloneWithArgs(ctx context.Context, args TrustedCmdArgs, from, to string, op if len(opts.Branch) > 0 { cmd.AddArguments("-b").AddDynamicArguments(opts.Branch) } - cmd.AddDashesAndList(from, to) - if strings.Contains(from, "://") && strings.Contains(from, "@") { - cmd.SetDescription(fmt.Sprintf("clone branch %s from %s to %s (shared: %t, mirror: %t, depth: %d)", opts.Branch, util.SanitizeCredentialURLs(from), to, opts.Shared, opts.Mirror, opts.Depth)) - } else { - cmd.SetDescription(fmt.Sprintf("clone branch %s from %s to %s (shared: %t, mirror: %t, depth: %d)", opts.Branch, from, to, opts.Shared, opts.Mirror, opts.Depth)) + envs := os.Environ() + parsedFromURL, err := url.Parse(from) + if err == nil { + envs = proxy.EnvWithProxy(parsedFromURL) } + fromURL := from + sanitizedFrom := from + + // If the clone URL has credentials, sanitize it and store the credentials in + // a temporary file that git will access. + if strings.Contains(from, "://") && strings.Contains(from, "@") { + sanitizedFrom = util.SanitizeCredentialURLs(from) + if parsedFromURL != nil { + if pwd, has := parsedFromURL.User.Password(); has { + parsedFromURL.User = url.User(parsedFromURL.User.Username()) + fromURL = parsedFromURL.String() + + credentialsFile, err := os.CreateTemp(os.TempDir(), "forgejo-clone-credentials") + if err != nil { + return err + } + credentialsPath := credentialsFile.Name() + + defer func() { + _ = credentialsFile.Close() + if err := util.Remove(credentialsPath); err != nil { + log.Warn("Unable to remove temporary file %q: %v", credentialsPath, err) + } + }() + + // Make it read-write. + if err := credentialsFile.Chmod(0o600); err != nil { + return err + } + + // Write the password. + if _, err := fmt.Fprint(credentialsFile, pwd); err != nil { + return err + } + + askpassFile, err := os.CreateTemp(os.TempDir(), "forgejo-askpass") + if err != nil { + return err + } + askpassPath := askpassFile.Name() + + defer func() { + _ = askpassFile.Close() + if err := util.Remove(askpassPath); err != nil { + log.Warn("Unable to remove temporary file %q: %v", askpassPath, err) + } + }() + + // Make it executable. + if err := askpassFile.Chmod(0o700); err != nil { + return err + } + + // Write the password script. + if _, err := fmt.Fprintf(askpassFile, "exec cat %s", credentialsPath); err != nil { + return err + } + + // Close it, so that Git can use it and no busy errors arise. + _ = askpassFile.Close() + _ = credentialsFile.Close() + + // Use environments to specify that git should ask for credentials, this + // takes precedences over anything else https://git-scm.com/docs/gitcredentials#_requesting_credentials. + envs = append(envs, "GIT_ASKPASS="+askpassPath) + } + } + } + + cmd.SetDescription(fmt.Sprintf("clone branch %s from %s to %s (shared: %t, mirror: %t, depth: %d)", opts.Branch, sanitizedFrom, to, opts.Shared, opts.Mirror, opts.Depth)) + cmd.AddDashesAndList(fromURL, to) + if opts.Timeout <= 0 { opts.Timeout = -1 } - envs := os.Environ() - u, err := url.Parse(from) - if err == nil { - envs = proxy.EnvWithProxy(u) - } - stderr := new(bytes.Buffer) if err = cmd.Run(&RunOpts{ Timeout: opts.Timeout, diff --git a/modules/git/repo_test.go b/modules/git/repo_test.go index c3971b2a5f..b07fc3732d 100644 --- a/modules/git/repo_test.go +++ b/modules/git/repo_test.go @@ -4,7 +4,15 @@ package git import ( + "bytes" + "encoding/base64" + "io/fs" + "net/http" + "net/http/httptest" + "net/url" + "os" "path/filepath" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -54,3 +62,80 @@ func TestRepoGetDivergingCommits(t *testing.T) { Behind: 2, }, do) } + +func TestCloneCredentials(t *testing.T) { + calledWithoutPassword := false + askpassFile := "" + credentialsFile := "" + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + if req.URL.Path != "/info/refs" { + return + } + + // Get basic authorization. + auth, ok := strings.CutPrefix(req.Header.Get("Authorization"), "Basic ") + if !ok { + w.Header().Set("WWW-Authenticate", `Basic realm="Forgejo"`) + http.Error(w, "require credentials", http.StatusUnauthorized) + return + } + + rawAuth, err := base64.StdEncoding.DecodeString(auth) + require.NoError(t, err) + + user, password, ok := bytes.Cut(rawAuth, []byte{':'}) + assert.True(t, ok) + + // First time around Git tries without password (that's specified in the clone URL). + // It demonstrates it doesn't immediately uses askpass. + if len(password) == 0 { + assert.EqualValues(t, "oauth2", user) + calledWithoutPassword = true + + w.Header().Set("WWW-Authenticate", `Basic realm="Forgejo"`) + http.Error(w, "require credentials", http.StatusUnauthorized) + return + } + + assert.EqualValues(t, "oauth2", user) + assert.EqualValues(t, "some_token", password) + + tmpDir := os.TempDir() + + // Verify that the askpass implementation was used. + files, err := fs.Glob(os.DirFS(tmpDir), "forgejo-askpass*") + require.NoError(t, err) + for _, fileName := range files { + fileContent, err := os.ReadFile(filepath.Join(tmpDir, fileName)) + require.NoError(t, err) + + credentialsPath, ok := bytes.CutPrefix(fileContent, []byte(`exec cat `)) + assert.True(t, ok) + + fileContent, err = os.ReadFile(string(credentialsPath)) + require.NoError(t, err) + assert.EqualValues(t, "some_token", fileContent) + + askpassFile = filepath.Join(tmpDir, fileName) + credentialsFile = string(credentialsPath) + } + })) + + serverURL, err := url.Parse(server.URL) + require.NoError(t, err) + + serverURL.User = url.UserPassword("oauth2", "some_token") + + require.NoError(t, Clone(t.Context(), serverURL.String(), t.TempDir(), CloneRepoOptions{})) + + assert.True(t, calledWithoutPassword) + assert.NotEmpty(t, askpassFile) + assert.NotEmpty(t, credentialsFile) + + // Check that the helper files are gone. + _, err = os.Stat(askpassFile) + require.ErrorIs(t, err, fs.ErrNotExist) + _, err = os.Stat(credentialsFile) + require.ErrorIs(t, err, fs.ErrNotExist) +} diff --git a/modules/util/shellquote_test.go b/modules/util/shellquote_test.go index 969998c592..6c1b778a08 100644 --- a/modules/util/shellquote_test.go +++ b/modules/util/shellquote_test.go @@ -3,7 +3,11 @@ package util -import "testing" +import ( + "testing" + + "github.com/stretchr/testify/assert" +) func TestShellEscape(t *testing.T) { tests := []struct { @@ -79,13 +83,23 @@ func TestShellEscape(t *testing.T) { "Single quotes don't need to escape except for '...", "~/ ${gitea} `gitea` (gitea) !gitea! \"gitea\" \\gitea\\ 'gitea'", "~/' ${gitea} `gitea` (gitea) !gitea! \"gitea\" \\gitea\\ '\\''gitea'\\'", + }, { + "Inline command", + "some`echo foo`thing", + "\"some\\`echo foo\\`thing\"", + }, { + "Substitution", + `;${HOME}`, + `";\${HOME}"`, + }, { + "ANSI Escape codes (not escaped)", + "\033[31;1;4mHello\033[0m", + "\"\x1b[31;1;4mHello\x1b[0m\"", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := ShellEscape(tt.toEscape); got != tt.want { - t.Errorf("ShellEscape(%q):\nGot: %s\nWanted: %s", tt.toEscape, got, tt.want) - } + assert.Equal(t, tt.want, ShellEscape(tt.toEscape)) }) } } diff --git a/tests/integration/codeowner_test.go b/tests/integration/codeowner_test.go index b85a5f213d..51468ffe09 100644 --- a/tests/integration/codeowner_test.go +++ b/tests/integration/codeowner_test.go @@ -47,6 +47,7 @@ func CodeOwnerTestCommon(t *testing.T, u *url.URL, codeownerTest CodeownerTest) cloneURL, _ := url.Parse(r) cloneURL.User = url.UserPassword("user2", userPassword) require.NoError(t, git.CloneWithArgs(t.Context(), nil, cloneURL.String(), dstPath, git.CloneRepoOptions{})) + doGitSetRemoteURL(dstPath, "origin", cloneURL)(t) t.Run("Normal", func(t *testing.T) { defer tests.PrintCurrentTest(t)() diff --git a/tests/integration/git_helper_for_declarative_test.go b/tests/integration/git_helper_for_declarative_test.go index 89453296ca..5be5c6088c 100644 --- a/tests/integration/git_helper_for_declarative_test.go +++ b/tests/integration/git_helper_for_declarative_test.go @@ -105,6 +105,8 @@ func doGitClone(dstLocalPath string, u *url.URL) func(*testing.T) { exist, err := util.IsExist(filepath.Join(dstLocalPath, "README.md")) require.NoError(t, err) assert.True(t, exist) + // Set user:password + doGitSetRemoteURL(dstLocalPath, "origin", u)(t) } } @@ -117,6 +119,8 @@ func doPartialGitClone(dstLocalPath string, u *url.URL) func(*testing.T) { exist, err := util.IsExist(filepath.Join(dstLocalPath, "README.md")) require.NoError(t, err) assert.True(t, exist) + // Set user:password + doGitSetRemoteURL(dstLocalPath, "origin", u)(t) } } @@ -162,6 +166,14 @@ func doGitAddRemote(dstPath, remoteName string, u *url.URL) func(*testing.T) { } } +func doGitSetRemoteURL(dstPath, remoteName string, u *url.URL) func(*testing.T) { + return func(t *testing.T) { + t.Helper() + _, _, err := git.NewCommand(git.DefaultContext, "remote", "set-url").AddDynamicArguments(remoteName, u.String()).RunStdString(&git.RunOpts{Dir: dstPath}) + require.NoError(t, err) + } +} + func doGitPushTestRepository(dstPath string, args ...string) func(*testing.T) { return func(t *testing.T) { t.Helper() diff --git a/tests/integration/patch_status_test.go b/tests/integration/patch_status_test.go index 3ce1dc4cb9..4a5edccf59 100644 --- a/tests/integration/patch_status_test.go +++ b/tests/integration/patch_status_test.go @@ -78,6 +78,7 @@ func TestPatchStatus(t *testing.T) { // Clone repository. dstPath := t.TempDir() require.NoError(t, git.Clone(t.Context(), u.String(), dstPath, git.CloneRepoOptions{})) + doGitSetRemoteURL(dstPath, "origin", u)(t) // Add `fork` remote. u.Path = forkRepo.FullName() diff --git a/tests/integration/pull_diff_test.go b/tests/integration/pull_diff_test.go index 7442909aec..0f28bbbd49 100644 --- a/tests/integration/pull_diff_test.go +++ b/tests/integration/pull_diff_test.go @@ -93,6 +93,7 @@ func TestPullDiff_AGitNotEditable(t *testing.T) { cloneURL, _ := url.Parse(clone) cloneURL.User = url.UserPassword("user2", userPassword) require.NoError(t, git.CloneWithArgs(t.Context(), nil, cloneURL.String(), dstPath, git.CloneRepoOptions{})) + doGitSetRemoteURL(dstPath, "origin", cloneURL)(t) return dstPath } diff --git a/tests/integration/pull_review_test.go b/tests/integration/pull_review_test.go index 35f287958c..b385916f96 100644 --- a/tests/integration/pull_review_test.go +++ b/tests/integration/pull_review_test.go @@ -791,6 +791,7 @@ func TestPullRequestStaleReview(t *testing.T) { cloneURL, _ := url.Parse(clone) cloneURL.User = url.UserPassword("user2", userPassword) require.NoError(t, git.CloneWithArgs(t.Context(), nil, cloneURL.String(), dstPath, git.CloneRepoOptions{})) + doGitSetRemoteURL(dstPath, "origin", cloneURL)(t) return dstPath } diff --git a/tests/integration/repo_migrate_credentials_test.go b/tests/integration/repo_migrate_credentials_test.go new file mode 100644 index 0000000000..b63ca9b29e --- /dev/null +++ b/tests/integration/repo_migrate_credentials_test.go @@ -0,0 +1,95 @@ +// Copyright 2025 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package integration + +import ( + "net/http" + "net/url" + "testing" + + "forgejo.org/models/admin" + repo_model "forgejo.org/models/repo" + "forgejo.org/models/unittest" + user_model "forgejo.org/models/user" + "forgejo.org/modules/setting" + "forgejo.org/modules/structs" + "forgejo.org/modules/test" + "forgejo.org/services/migrations" + + "github.com/stretchr/testify/require" +) + +func TestRepoMigrateWithCredentials(t *testing.T) { + onGiteaRun(t, func(t *testing.T, u *url.URL) { + defer test.MockVariableValue(&setting.Migrations.AllowLocalNetworks, true)() + require.NoError(t, migrations.Init()) + + user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + session := loginUser(t, "user2") + + t.Run("Incorrect credentials", func(t *testing.T) { + session.MakeRequest(t, NewRequestWithValues(t, "POST", "/repo/migrate", map[string]string{ + "_csrf": GetCSRF(t, session, "/repo/migrate?service_type=1"), + "clone_addr": u.JoinPath("/user2/repo2").String(), + "auth_username": "user2", + "auth_password": userPassword + "1", + "uid": "2", + "repo_name": "migrating-with-credentials", + "service": "1", + }), http.StatusSeeOther) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{Name: "migrating-with-credentials"}, "is_empty = true") + unittest.AssertExistsAndLoadBean(t, &admin.Task{ + RepoID: repo.ID, + Type: structs.TaskTypeMigrateRepo, + Status: structs.TaskStatusFailed, + }) + }) + + t.Run("Normal", func(t *testing.T) { + session.MakeRequest(t, NewRequestWithValues(t, "POST", "/repo/migrate", map[string]string{ + "_csrf": GetCSRF(t, session, "/repo/migrate?service_type=1"), + "clone_addr": u.JoinPath("/user2/repo2").String(), + "auth_username": "user2", + "auth_password": userPassword, + "uid": "2", + "repo_name": "migrating-with-credentials-2", + "service": "1", + }), http.StatusSeeOther) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{Name: "migrating-with-credentials-2"}, "is_empty = false") + unittest.AssertExistsAndLoadBean(t, &admin.Task{ + RepoID: repo.ID, + Type: structs.TaskTypeMigrateRepo, + Status: structs.TaskStatusFinished, + }) + }) + + t.Run("Dangerous credential", func(t *testing.T) { + // Temporaily change the password + dangerousPassword := "some`echo foo`thing" + require.NoError(t, user2.SetPassword(dangerousPassword)) + require.NoError(t, user_model.UpdateUserCols(t.Context(), user2, "passwd", "passwd_hash_algo", "salt")) + + session = loginUserWithPassword(t, "user2", dangerousPassword) + + session.MakeRequest(t, NewRequestWithValues(t, "POST", "/repo/migrate", map[string]string{ + "_csrf": GetCSRF(t, session, "/repo/migrate?service_type=1"), + "clone_addr": u.JoinPath("/user2/repo2").String(), + "auth_username": "user2", + "auth_password": dangerousPassword, + "uid": "2", + "repo_name": "migrating-with-credentials-3", + "service": "1", + }), http.StatusSeeOther) + + repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{Name: "migrating-with-credentials-3"}, "is_empty = false") + unittest.AssertExistsAndLoadBean(t, &admin.Task{ + RepoID: repo.ID, + Type: structs.TaskTypeMigrateRepo, + Status: structs.TaskStatusFinished, + }) + }) + }) +}