diff options
author | Karthik Nayak <knayak@gitlab.com> | 2022-09-29 10:25:22 +0300 |
---|---|---|
committer | John Cai <jcai@gitlab.com> | 2022-11-02 21:40:46 +0300 |
commit | c34800b226f9a23b8236b4adc1c2f3e59565a272 (patch) | |
tree | dd912631bf5379ce1d291769c73811603e10b8bb | |
parent | 8e8d639d420088b9cd0ee8103d487e0952472f53 (diff) |
repository: Use `GetURLAndResolveConfig()`
In the `cloneFromURLCommand()` function lets use the
`GetURLAndResolveConfig()` function to obtain configPair and the
modified URL to prevent DNS rebinding. Add tests for the same.
3 files changed, 50 insertions, 11 deletions
diff --git a/internal/gitaly/service/repository/create_repository_from_url.go b/internal/gitaly/service/repository/create_repository_from_url.go index 47929bacc..57acb49e5 100644 --- a/internal/gitaly/service/repository/create_repository_from_url.go +++ b/internal/gitaly/service/repository/create_repository_from_url.go @@ -18,7 +18,7 @@ import ( func (s *server) cloneFromURLCommand( ctx context.Context, - repoURL, repoHost, repositoryFullPath, authorizationToken string, mirror bool, + repoURL, repoHost, resolvedAddress, repositoryFullPath, authorizationToken string, mirror bool, opts ...git.CmdOpt, ) (*command.Command, error) { cloneFlags := []git.Option{ @@ -55,6 +55,18 @@ func (s *server) cloneFromURLCommand( config = append(config, git.ConfigPair{Key: "http.extraHeader", Value: authHeader}) } + urlString := u.String() + + if resolvedAddress != "" { + modifiedURL, resolveConfig, err := git.GetURLAndResolveConfig(u.String(), resolvedAddress) + if err != nil { + return nil, helper.ErrInvalidArgumentf("couldn't get curloptResolve config: %w", err) + } + + urlString = modifiedURL + config = append(config, resolveConfig...) + } + if repoHost != "" { config = append(config, git.ConfigPair{ Key: "http.extraHeader", @@ -66,7 +78,7 @@ func (s *server) cloneFromURLCommand( git.SubCmd{ Name: "clone", Flags: cloneFlags, - Args: []string{u.String(), repositoryFullPath}, + Args: []string{urlString, repositoryFullPath}, }, append(opts, git.WithConfigEnv(config...))..., ) @@ -93,6 +105,7 @@ func (s *server) CreateRepositoryFromURL(ctx context.Context, req *gitalypb.Crea req.GetUrl(), //nolint:staticcheck req.GetHttpHost(), + req.GetResolvedAddress(), targetPath, req.GetHttpAuthorizationHeader(), req.GetMirror(), diff --git a/internal/gitaly/service/repository/create_repository_from_url_test.go b/internal/gitaly/service/repository/create_repository_from_url_test.go index 3f40a8a72..cc3aaed26 100644 --- a/internal/gitaly/service/repository/create_repository_from_url_test.go +++ b/internal/gitaly/service/repository/create_repository_from_url_test.go @@ -229,10 +229,13 @@ func TestServer_CloneFromURLCommand(t *testing.T) { user, password := "example_user", "pass%21%3F%40" for _, tc := range []struct { - desc string - url string - token string - expectedAuthHeader string + desc string + url string + resolvedAddress string + token string + expectedAuthHeader string + expectedCurloptResolveHeader string + expectedURL string }{ { desc: "user credentials", @@ -242,6 +245,7 @@ func TestServer_CloneFromURLCommand(t *testing.T) { "Authorization: Basic %s", base64.StdEncoding.EncodeToString([]byte("example_user:pass!?@")), ), + expectedURL: "https://192.0.2.1/secretrepo.git", }, { desc: "token", @@ -250,6 +254,16 @@ func TestServer_CloneFromURLCommand(t *testing.T) { expectedAuthHeader: fmt.Sprintf( "Authorization: %s", "some-token", ), + expectedURL: "https://192.0.2.1/secretrepo.git", + }, + { + desc: "with resolved address", + url: "https://gitlab.com/secretrepo.git", + token: "some-token", + expectedAuthHeader: fmt.Sprintf("Authorization: %s", "some-token"), + resolvedAddress: "192.0.1.1", + expectedURL: "https://gitlab.com/secretrepo.git", + expectedCurloptResolveHeader: "*:443:192.0.1.1", }, } { t.Run(tc.desc, func(t *testing.T) { @@ -259,6 +273,7 @@ func TestServer_CloneFromURLCommand(t *testing.T) { ctx, tc.url, "www.example.com", + tc.resolvedAddress, "full/path/to/repository", tc.token, false, @@ -275,7 +290,7 @@ func TestServer_CloneFromURLCommand(t *testing.T) { args := cmd.Args() require.Contains(t, args, "--bare") - require.Contains(t, args, "https://192.0.2.1/secretrepo.git") + require.Contains(t, args, tc.expectedURL) for _, arg := range args { require.NotContains(t, arg, user) require.NotContains(t, arg, password) @@ -285,9 +300,21 @@ func TestServer_CloneFromURLCommand(t *testing.T) { require.Subset(t, cmd.Env(), []string{ "GIT_CONFIG_KEY_0=http.extraHeader", "GIT_CONFIG_VALUE_0=" + tc.expectedAuthHeader, - "GIT_CONFIG_KEY_1=http.extraHeader", - "GIT_CONFIG_VALUE_1=Host: www.example.com", }) + + if tc.expectedCurloptResolveHeader != "" { + require.Subset(t, cmd.Env(), []string{ + "GIT_CONFIG_KEY_1=http.curloptResolve", + "GIT_CONFIG_VALUE_1=" + tc.expectedCurloptResolveHeader, + "GIT_CONFIG_KEY_2=http.extraHeader", + "GIT_CONFIG_VALUE_2=Host: www.example.com", + }) + } else { + require.Subset(t, cmd.Env(), []string{ + "GIT_CONFIG_KEY_1=http.extraHeader", + "GIT_CONFIG_VALUE_1=Host: www.example.com", + }) + } }) } } @@ -301,7 +328,7 @@ func TestServer_CloneFromURLCommand_withMirror(t *testing.T) { cfg := testcfg.Build(t) s := server{cfg: cfg, gitCmdFactory: gittest.NewCommandFactory(t, cfg)} - cmd, err := s.cloneFromURLCommand(ctx, url, "", repositoryFullPath, "", true, git.WithDisabledHooks()) + cmd, err := s.cloneFromURLCommand(ctx, url, "", "", repositoryFullPath, "", true, git.WithDisabledHooks()) require.NoError(t, err) args := cmd.Args() diff --git a/internal/gitaly/service/repository/fetch_remote.go b/internal/gitaly/service/repository/fetch_remote.go index 9b4aa308e..5f7ceda05 100644 --- a/internal/gitaly/service/repository/fetch_remote.go +++ b/internal/gitaly/service/repository/fetch_remote.go @@ -67,7 +67,6 @@ func (s *server) FetchRemote(ctx context.Context, req *gitalypb.FetchRemoteReque opts.CommandOptions = append(opts.CommandOptions, git.WithConfigEnv(config...)) - //nolint:staticcheck sshCommand, cleanup, err := git.BuildSSHInvocation(ctx, req.GetSshKey(), req.GetKnownHosts()) if err != nil { return nil, err |