diff --git a/models/forgejo_migrations/v15c_add_mirror_remoteaddressauth.go b/models/forgejo_migrations/v15c_add_mirror_remoteaddressauth.go new file mode 100644 index 0000000000..b2f30235ad --- /dev/null +++ b/models/forgejo_migrations/v15c_add_mirror_remoteaddressauth.go @@ -0,0 +1,30 @@ +// Copyright 2026 The Forgejo Authors. All rights reserved. +// SPDX-License-Identifier: GPL-3.0-or-later + +package forgejo_migrations + +import ( + "xorm.io/xorm" +) + +func init() { + registerMigration(&Migration{ + Description: "replace remote_address with encrypted_remote_address in table mirror", + Upgrade: addMirrorRemoteAddressAuth, + }) +} + +func addMirrorRemoteAddressAuth(x *xorm.Engine) error { + type Mirror struct { + EncryptedRemoteAddress []byte `xorm:"BLOB NULL"` + } + if _, err := x.SyncWithOptions(xorm.SyncOptions{IgnoreDropIndices: true}, new(Mirror)); err != nil { + return err + } + // No data migration is necessary or desired. `remote_address` contains sanitized URLs which don't have + // credentials, so they can't be migrated to `encrypted_remote_address`. Instead, as this data is accessed, + // `DecryptOrRecoverRemoteAddress` will recover the fully credentialed contents of the remote address from the git + // repo's `origin` remote address. + _, err := x.Exec("ALTER TABLE `mirror` DROP COLUMN `remote_address`") + return err +} diff --git a/models/repo/mirror.go b/models/repo/mirror.go index 1fe9afd8e9..c64f9dc734 100644 --- a/models/repo/mirror.go +++ b/models/repo/mirror.go @@ -6,10 +6,14 @@ package repo import ( "context" + "errors" + "net/url" "time" "forgejo.org/models/db" + "forgejo.org/modules/keying" "forgejo.org/modules/log" + "forgejo.org/modules/optional" "forgejo.org/modules/timeutil" "forgejo.org/modules/util" ) @@ -31,7 +35,9 @@ type Mirror struct { LFS bool `xorm:"lfs_enabled NOT NULL DEFAULT false"` LFSEndpoint string `xorm:"lfs_endpoint TEXT"` - RemoteAddress string `xorm:"VARCHAR(2048)"` + // Encrypted remote address w/ credentials; can be NULL if a mirror has not performed a sync since this field was + // introduced, in which case the remote address exists only in the repo's configured git remote on disk. + EncryptedRemoteAddress []byte `xorm:"BLOB NULL"` } func init() { @@ -73,6 +79,71 @@ func (m *Mirror) ScheduleNextUpdate() { } } +// InsertMirror inserts a mirror to database. RemoteAddress must be provided so that it can be encrypted and stored +// during the insert process. +func (m *Mirror) InsertWithAddress(ctx context.Context, addr string) error { + return db.WithTx(ctx, func(ctx context.Context) error { + if _, err := db.GetEngine(ctx).Insert(m); err != nil { + return err + } + return m.UpdateRemoteAddress(ctx, addr) + }) +} + +// Stores a credential-free version of the address in `RemoteAddress`, encrypts the original into `RemoteAddressAuth`, +// and stores both in the database. The ID of the mirror must be known, so this must be done after the mirror is +// inserted. +func (m *Mirror) UpdateRemoteAddress(ctx context.Context, addr string) error { + if m.ID == 0 { + return errors.New("must persist mirror to database before using UpdateRemoteAddress") + } + + m.EncryptedRemoteAddress = keying.PullMirror.Encrypt( + []byte(addr), + keying.ColumnAndID("remote_address_auth", m.ID), + ) + _, err := db.GetEngine(ctx).ID(m.ID).Cols("encrypted_remote_address").Update(m) + return err +} + +// Retrieves the encrypted remote address and decrypts it. Note that this field is expected to be absent for mirrors +// created before the introduction of EncryptedRemoteAddress, in which case credentials are not known to Forgejo +// directly (but may be on-disk in the repository's config file) and None will be returned. +func (m *Mirror) DecryptRemoteAddress() (optional.Option[string], error) { + if m.EncryptedRemoteAddress == nil { + return optional.None[string](), nil + } + + contents, err := keying.PullMirror.Decrypt(m.EncryptedRemoteAddress, keying.ColumnAndID("remote_address_auth", m.ID)) + if err != nil { + return optional.None[string](), err + } + return optional.Some(string(contents)), nil +} + +// Retrieves the remote address but sanitizes it of sensitive credentials. May be absent for mirrors created before the +// introduction of EncryptedRemoteAddress. +func (m *Mirror) SanitizedRemoteAddress() (optional.Option[string], error) { + maybeAddr, err := m.DecryptRemoteAddress() + if err != nil { + return optional.None[string](), err + } else if has, addr := maybeAddr.Get(); has { + parsedURL, err := url.Parse(addr) + if err != nil { + return optional.None[string](), err + } + + // Remove the password if present. Retain the username for consistency with `AddAuthCredentialHelperForRemote` + // which retains the username for the `git clone` command line, which ends up as the remote URL in the mirror's + // git config. + if parsedURL.User != nil { + parsedURL.User = url.User(parsedURL.User.Username()) + } + return optional.Some(parsedURL.String()), nil + } + return optional.None[string](), nil +} + // GetMirrorByRepoID returns mirror information of a repository. func GetMirrorByRepoID(ctx context.Context, repoID int64) (*Mirror, error) { m := &Mirror{RepoID: repoID} @@ -115,9 +186,3 @@ func MirrorsIterate(ctx context.Context, limit int, f func(idx int, bean any) er } return sess.Iterate(new(Mirror), f) } - -// InsertMirror inserts a mirror to database -func InsertMirror(ctx context.Context, mirror *Mirror) error { - _, err := db.GetEngine(ctx).Insert(mirror) - return err -} diff --git a/modules/git/command.go b/modules/git/command.go index bf1d624dbf..4ab081418b 100644 --- a/modules/git/command.go +++ b/modules/git/command.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "io" + "net/url" "os" "os/exec" "runtime/trace" @@ -446,6 +447,54 @@ func (c *Command) RunStdBytes(opts *RunOpts) (stdout, stderr []byte, runErr RunS return stdoutBuf.Bytes(), stderr, nil } +// If `remoteURL` is a URL with a password in it, add parameters to the git command that will read that password from a +// credential store file, and return the URL that should be used in the command instead of the original, and a cleanup +// function to call to remove the credential file. If `remoteURL` doesn't have a password, then it is returned as-is. +// This function must be invoked on the the git command before the git sub-command -- eg. before the `clone` or `fetch` +// parameter is added to the command's args. +func (c *Command) AddAuthCredentialHelperForRemote(remoteURL string) (commandURL string, cleanup func(), err error) { + parsedFromURL, _ := url.Parse(remoteURL) + + // If the clone URL has credentials, build a credential file for usage by git-credential-store + // to prevent credential leak in the process list. + // https://git-scm.com/docs/git-credential-store#_storage_format + // credential.helper adjustment must be set before the git subcommand + if strings.Contains(remoteURL, "://") && strings.Contains(remoteURL, "@") && parsedFromURL != nil { + credentialsFile, err := os.CreateTemp("", "forgejo-clone-credentials-") + if err != nil { + return "", nil, err + } + credentialsPath := credentialsFile.Name() + + cleanup := func() { + _ = credentialsFile.Close() + if err := util.Remove(credentialsPath); err != nil { + log.Warn("Unable to remove temporary file %q: %v", credentialsPath, err) + } + } + _, err = credentialsFile.Write([]byte(parsedFromURL.String())) + if err != nil { + cleanup() + return "", nil, err + } + err = credentialsFile.Close() + if err != nil { + cleanup() + return "", nil, err + } + + c.AddArguments("-c").AddDynamicArguments("credential.helper=store --file=" + credentialsPath) + + // remove the password from the URL argument + parsedFromURL.User = url.User(parsedFromURL.User.Username()) + commandURL = parsedFromURL.String() + + return commandURL, cleanup, nil + } + + return remoteURL, func() {}, nil +} + // AllowLFSFiltersArgs return globalCommandArgs with lfs filter, it should only be used for tests func AllowLFSFiltersArgs() TrustedCmdArgs { // Now here we should explicitly allow lfs filters to run diff --git a/modules/git/repo.go b/modules/git/repo.go index 6bd03f8e3c..8f9b95f1f2 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -18,7 +18,6 @@ import ( "strings" "time" - "forgejo.org/modules/log" "forgejo.org/modules/proxy" "forgejo.org/modules/setting" "forgejo.org/modules/util" @@ -141,44 +140,13 @@ func CloneWithArgs(ctx context.Context, args TrustedCmdArgs, from, to string, op envs = proxy.EnvWithProxy(parsedFromURL) } - fromURL := from - sanitizedFrom := from + sanitizedFrom := util.SanitizeCredentialURLs(from) - // If the clone URL has credentials, build a credential file for usage by git-credential-store - // to prevent credential leak in the process list. - // https://git-scm.com/docs/git-credential-store#_storage_format - // credential.helper adjustment must be set before the git subcommand - if strings.Contains(from, "://") && strings.Contains(from, "@") { - sanitizedFrom = util.SanitizeCredentialURLs(from) - if parsedFromURL != nil { - credentialsFile, err := os.CreateTemp("", "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) - } - }() - _, err = credentialsFile.Write([]byte(parsedFromURL.String())) - if err != nil { - return err - } - err = credentialsFile.Close() - if err != nil { - return err - } - - cmd.AddArguments("-c").AddDynamicArguments("credential.helper=store --file=" + credentialsPath) - - // remove the password from the URL argument - parsedFromURL.User = url.User(parsedFromURL.User.Username()) - fromURL = parsedFromURL.String() - } + fromURL, cleanup, err := cmd.AddAuthCredentialHelperForRemote(from) + if err != nil { + return err } + defer cleanup() cmd.AddArguments("clone") diff --git a/modules/keying/keying.go b/modules/keying/keying.go index 751fd77521..14fbaaca98 100644 --- a/modules/keying/keying.go +++ b/modules/keying/keying.go @@ -41,6 +41,8 @@ var ( MigrateTask = deriveKey("migrate_repo_task") // Used for the `webhook` table. Webhook = deriveKey("webhook") + // Used for the `mirror` table. + PullMirror = deriveKey("pullmirror") ) var ( diff --git a/modules/templates/util_misc.go b/modules/templates/util_misc.go index 60f918be47..8e1ef71f5d 100644 --- a/modules/templates/util_misc.go +++ b/modules/templates/util_misc.go @@ -17,12 +17,11 @@ import ( asymkey_model "forgejo.org/models/asymkey" repo_model "forgejo.org/models/repo" user_model "forgejo.org/models/user" - "forgejo.org/modules/git" - giturl "forgejo.org/modules/git/url" "forgejo.org/modules/json" "forgejo.org/modules/log" "forgejo.org/modules/repository" "forgejo.org/modules/svg" + mirror_service "forgejo.org/services/mirror" "github.com/editorconfig/editorconfig-core-go/v2" ) @@ -164,17 +163,11 @@ type remoteAddress struct { Password string } -func mirrorRemoteAddress(ctx context.Context, m *repo_model.Repository, remoteName string) remoteAddress { +func mirrorRemoteAddress(ctx context.Context, mirror *repo_model.Mirror) remoteAddress { ret := remoteAddress{} - remoteURL, err := git.GetRemoteAddress(ctx, m.RepoPath(), remoteName) + u, err := mirror_service.DecryptOrRecoverRemoteAddress(ctx, mirror) if err != nil { - log.Error("GetRemoteURL %v", err) - return ret - } - - u, err := giturl.Parse(remoteURL) - if err != nil { - log.Error("giturl.Parse %v", err) + log.Error("DecryptOrRecoverRemoteAddress %v", err) return ret } diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index f7d0348634..b001cf961e 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -462,12 +462,12 @@ func SettingsPost(ctx *context.Context) { return } - u, err := git.GetRemoteURL(ctx, ctx.Repo.Repository.RepoPath(), pullMirror.GetRemoteName()) + u, err := mirror_service.DecryptOrRecoverRemoteAddress(ctx, pullMirror) if err != nil { - ctx.Data["Err_MirrorAddress"] = true - handleSettingRemoteAddrError(ctx, err, form) + ctx.ServerError("DecryptOrRecoverRemoteAddress", err) return } + if u.User != nil && form.MirrorPassword == "" && form.MirrorUsername == u.User.Username() { form.MirrorPassword, _ = u.User.Password() } @@ -482,17 +482,25 @@ func SettingsPost(ctx *context.Context) { return } - if err := mirror_service.UpdateAddress(ctx, pullMirror, address); err != nil { - ctx.ServerError("UpdateAddress", err) - return - } - remoteAddress, err := util.SanitizeURL(address) - if err != nil { + if err := pullMirror.UpdateRemoteAddress(ctx, address); err != nil { ctx.Data["Err_MirrorAddress"] = true handleSettingRemoteAddrError(ctx, err, form) return } - pullMirror.RemoteAddress = remoteAddress + + // Update the unencrypted address stored in the git config, so that future `git fetch` will access the right + // address. pullMirror.RemoteAddress is the sanitized no-creds version from UpdateRemoteAddress. + if maybeSanitizedURL, err := pullMirror.SanitizedRemoteAddress(); err != nil { + ctx.ServerError("SanitizedRemoteAddress", err) + return + } else if has, sanitizedURL := maybeSanitizedURL.Get(); !has { + // SanitizedRemoteAddress must be present after we just stored it + ctx.ServerError("SanitizedRemoteAddress", err) + return + } else if err := mirror_service.UpdateAddress(ctx, pullMirror, sanitizedURL); err != nil { + ctx.ServerError("UpdateAddress", err) + return + } form.LFS = form.LFS && setting.LFS.StartServer diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index d6e1ff6c51..c5adc6105f 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -31,55 +31,27 @@ const gitShortEmptySha = "0000000" // UpdateAddress writes new address to Git repository and database func UpdateAddress(ctx context.Context, m *repo_model.Mirror, addr string) error { - u, err := giturl.Parse(addr) - if err != nil { - return fmt.Errorf("invalid addr: %v", err) - } - remoteName := m.GetRemoteName() repoPath := m.GetRepository(ctx).RepoPath() - // Remove old remote - _, _, err = git.NewCommand(ctx, "remote", "rm").AddDynamicArguments(remoteName).RunStdString(&git.RunOpts{Dir: repoPath}) - if err != nil && !git.IsRemoteNotExistError(err) { - return err - } - - cmd := git.NewCommand(ctx, "remote", "add").AddDynamicArguments(remoteName).AddArguments("--mirror=fetch").AddDynamicArguments(addr) - if strings.Contains(addr, "://") && strings.Contains(addr, "@") { - cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, util.SanitizeCredentialURLs(addr), repoPath)) - } else { - cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, addr, repoPath)) - } - _, _, err = cmd.RunStdString(&git.RunOpts{Dir: repoPath}) - if err != nil && !git.IsRemoteNotExistError(err) { + _, _, err := git.NewCommand(ctx, "remote", "set-url"). + AddDynamicArguments(remoteName, addr). + RunStdString(&git.RunOpts{Dir: repoPath}) + if err != nil { return err } if m.Repo.HasWiki() { wikiPath := m.Repo.WikiPath() wikiRemotePath := repo_module.WikiRemoteURL(ctx, addr) - // Remove old remote of wiki - _, _, err = git.NewCommand(ctx, "remote", "rm").AddDynamicArguments(remoteName).RunStdString(&git.RunOpts{Dir: wikiPath}) - if err != nil && !git.IsRemoteNotExistError(err) { - return err - } - - cmd = git.NewCommand(ctx, "remote", "add").AddDynamicArguments(remoteName).AddArguments("--mirror=fetch").AddDynamicArguments(wikiRemotePath) - if strings.Contains(wikiRemotePath, "://") && strings.Contains(wikiRemotePath, "@") { - cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, util.SanitizeCredentialURLs(wikiRemotePath), wikiPath)) - } else { - cmd.SetDescription(fmt.Sprintf("remote add %s --mirror=fetch %s [repo_path: %s]", remoteName, wikiRemotePath, wikiPath)) - } - _, _, err = cmd.RunStdString(&git.RunOpts{Dir: wikiPath}) - if err != nil && !git.IsRemoteNotExistError(err) { + _, _, err = git.NewCommand(ctx, "remote", "set-url"). + AddDynamicArguments(remoteName, wikiRemotePath). + RunStdString(&git.RunOpts{Dir: wikiPath}) + if err != nil { return err } } - // erase authentication before storing in database - u.User = nil - m.Repo.OriginalURL = u.String() - return repo_model.UpdateRepositoryCols(ctx, m.Repo, "original_url") + return nil } // mirrorSyncResult contains information of a updated reference. @@ -262,6 +234,46 @@ func checkRecoverableSyncError(stderrMessage string) bool { } } +// Decrypt RemoteAddressAuth from the mirror. If absent on the mirror database table, fallback to the older method where +// credentials are stored in the git config file as the remote's address, encrypt those credentials and store them in +// the database, and wipe them from the git config file so that they're only stored in the one encrypted location in DB. +func DecryptOrRecoverRemoteAddress(ctx context.Context, m *repo_model.Mirror) (*giturl.GitURL, error) { + decryptedRemoteURL, err := m.DecryptRemoteAddress() + if err != nil { + return nil, fmt.Errorf("failed to decrypt remote address: %w", err) + } + + if has, url := decryptedRemoteURL.Get(); has { + remoteURL, err := giturl.Parse(url) + if err != nil { + return nil, fmt.Errorf("failed to parse decrypted remote address: %w", err) + } + return remoteURL, nil + } + + // fallback to reading remote URL from the git config file for repos that predate DecryptRemoteAddress + repoPath := m.GetRepository(ctx).RepoPath() + remoteURL, err := git.GetRemoteURL(ctx, repoPath, m.GetRemoteName()) + if err != nil { + return nil, fmt.Errorf("GetRemoteAddress error: %w", err) + } + + // Store the full address in the database + if err := m.UpdateRemoteAddress(ctx, remoteURL.URL.String()); err != nil { + return nil, fmt.Errorf("UpdateRemoteAddress error: %w", err) + } + + // Update the git config file to just contain the sanitized address + if maybeSanitizedURL, err := m.SanitizedRemoteAddress(); err != nil { + return nil, fmt.Errorf("SanitizedRemoteAddress error: %w", err) + } else if has, sanitizedURL := maybeSanitizedURL.Get(); !has { + return nil, fmt.Errorf("SanitizedRemoteAddress must be present after we just stored it, but had error: %w", err) + } else if err := UpdateAddress(ctx, m, sanitizedURL); err != nil { + return nil, fmt.Errorf("UpdateAddress error: %w", err) + } + return remoteURL, nil +} + // runSync returns true if sync finished without error. func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bool) { repoPath := m.Repo.RepoPath() @@ -270,19 +282,29 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*mirrorSyncResult, bo log.Trace("SyncMirrors [repo: %-v]: running git remote update...", m.Repo) + remoteURL, err := DecryptOrRecoverRemoteAddress(ctx, m) + if err != nil { + log.Error("SyncMirrors [repo: %-v]: failed to get remote address: %v", m.Repo, err) + return nil, false + } + + cmd := git.NewCommand(ctx) + + // Setup credential helper to authenticate the fetch; needs to occur before the `fetch` arg. + _, credCleanup, err := cmd.AddAuthCredentialHelperForRemote(remoteURL.URL.String()) + if err != nil { + log.Error("SyncMirrors [repo: %-v]: AddAuthCredentialHelperForRemote Error %v", m.Repo, err) + return nil, false + } + defer credCleanup() + // use fetch but not remote update because git fetch support --tags but remote update doesn't - cmd := git.NewCommand(ctx, "fetch") + cmd.AddArguments("fetch") if m.EnablePrune { cmd.AddArguments("--prune") } cmd.AddArguments("--tags").AddDynamicArguments(m.GetRemoteName()) - remoteURL, remoteErr := git.GetRemoteURL(ctx, repoPath, m.GetRemoteName()) - if remoteErr != nil { - log.Error("SyncMirrors [repo: %-v]: GetRemoteAddress Error %v", m.Repo, remoteErr) - return nil, false - } - envs := proxy.EnvWithProxy(remoteURL.URL) stdoutBuilder := strings.Builder{} diff --git a/services/repository/migrate.go b/services/repository/migrate.go index 80f5d68231..46b48d02d3 100644 --- a/services/repository/migrate.go +++ b/services/repository/migrate.go @@ -182,17 +182,12 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, defer committer.Close() if opts.Mirror { - remoteAddress, err := util.SanitizeURL(opts.CloneAddr) - if err != nil { - return repo, err - } mirrorModel := repo_model.Mirror{ RepoID: repo.ID, Interval: setting.Mirror.DefaultInterval, EnablePrune: true, NextUpdateUnix: timeutil.TimeStampNow().AddDuration(setting.Mirror.DefaultInterval), LFS: opts.LFS, - RemoteAddress: remoteAddress, } if opts.LFS { mirrorModel.LFSEndpoint = opts.LFSEndpoint @@ -217,7 +212,7 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, } } - if err = repo_model.InsertMirror(ctx, &mirrorModel); err != nil { + if err = mirrorModel.InsertWithAddress(ctx, opts.CloneAddr); err != nil { return repo, fmt.Errorf("InsertOne: %w", err) } diff --git a/templates/repo/header.tmpl b/templates/repo/header.tmpl index 513758a149..44254aae82 100644 --- a/templates/repo/header.tmpl +++ b/templates/repo/header.tmpl @@ -77,9 +77,10 @@ {{end}} {{if $.PullMirror}} + {{$address := MirrorRemoteAddress $.Context $.PullMirror}}