1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2025-09-30 19:22:08 +00:00

[v12.0/forgejo] fix: use credentials helpers for git clones (#9068)

**Backport**: https://codeberg.org/forgejo/forgejo/pulls/9067

When performing a `git clone` that requires credentials, they are temporarily stored in files and used with [Git credential](https://git-scm.com/docs/gitcredentials/2.50.0#_requesting_credentials). They were previously included in the URL that were readable by a user with shell access to the host running the Forgejo instance when, for instance, they ask for the list of process (`ps`).

Co-authored-by: Gusted <postmaster@gusted.xyz>
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9068
This commit is contained in:
Earl Warren 2025-08-30 18:45:56 +02:00
parent 1bc42842ba
commit b98109ee69
8 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)
}