fix: use credentials helpers for git clones (#9067)

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9067
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
This commit is contained in:
Earl Warren 2025-08-30 13:12:11 +02:00
commit 829062808a
9 changed files with 291 additions and 15 deletions

View file

@ -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,

View file

@ -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)
}

View file

@ -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\" \\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))
})
}
}

View file

@ -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)()

View file

@ -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()

View file

@ -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()

View file

@ -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
}

View file

@ -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
}

View file

@ -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,
})
})
})
}