diff --git a/cmd/gitaly-hooks/hooks_test.go b/cmd/gitaly-hooks/hooks_test.go index 840290f9061316239dfa93bbd1a6fe22378fabc0..91962b3a85457ddc2a32b5a4bcefb8a370c8bd27 100644 --- a/cmd/gitaly-hooks/hooks_test.go +++ b/cmd/gitaly-hooks/hooks_test.go @@ -18,6 +18,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/git" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gitcmd" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest" + "gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v16/internal/git/pktline" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config/auth" @@ -995,9 +996,11 @@ func TestGitalyServerReturnsError_packObjects(t *testing.T) { Hooks: config.Hooks{CustomHooksDir: testhelper.TempDir(t)}, })) - repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ + repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ SkipCreationViaService: true, }) + repo := localrepo.NewTestRepo(t, cfg, repoProto) + gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(git.DefaultBranch)) testcfg.BuildGitalyHooks(t, cfg) @@ -1023,8 +1026,11 @@ func TestGitalyServerReturnsError_packObjects(t *testing.T) { ctx = featureflag.IncomingCtxWithFeatureFlag(ctx, enabledFeatureFlag, true) ctx = featureflag.IncomingCtxWithFeatureFlag(ctx, disabledFeatureFlag, false) + objectHash, err := repo.ObjectHash(ctx) + require.NoError(t, err) + var stderr, stdout bytes.Buffer - cmd, err := cmdFactory.New(ctx, repo, gitcmd.Command{ + cmd, err := cmdFactory.New(ctx, repoProto, gitcmd.Command{ Name: "clone", Flags: []gitcmd.Option{ gitcmd.ValueFlag{Name: "-u", Value: "git -c uploadpack.allowFilter -c uploadpack.packObjectsHook=" + cfg.BinaryPath("gitaly-hooks") + " upload-pack"}, @@ -1034,7 +1040,7 @@ func TestGitalyServerReturnsError_packObjects(t *testing.T) { }, Args: []string{repoPath, testhelper.TempDir(t)}, }, - gitcmd.WithPackObjectsHookEnv(repo, "ssh"), + gitcmd.WithPackObjectsHookEnv(repoProto, objectHash, "ssh"), gitcmd.WithStdout(&stdout), gitcmd.WithStderr(&stderr), ) diff --git a/internal/backup/repository.go b/internal/backup/repository.go index 1509bc27565adea35ac8f3d19ee95625fe4e1823..518b1c21a7a45ce9a9b3b8ff9a5b5c9731c0b53c 100644 --- a/internal/backup/repository.go +++ b/internal/backup/repository.go @@ -605,6 +605,10 @@ func (r *localRepository) HeadReference(ctx context.Context) (git.ReferenceName, // ResetRefs attempts to reset the list of refs in the repository to match the // specified refs slice. Do not include the symbolic HEAD reference in the list. func (r *localRepository) ResetRefs(ctx context.Context, refs []git.Reference) (returnedErr error) { + if err := r.locator.ValidateRepository(ctx, r.repo); err != nil { + return fmt.Errorf("invalid repository: %w", err) + } + u, err := updateref.New(ctx, r.repo) if err != nil { return fmt.Errorf("error when running creating new updater: %w", err) diff --git a/internal/git/gitcmd/hooks_options.go b/internal/git/gitcmd/hooks_options.go index 7aa3a5f282afe77601f95c4c357ae263c6941224..0f5bafbd35fb4bd3cd12cafed7c0e327cde0ffdd 100644 --- a/internal/git/gitcmd/hooks_options.go +++ b/internal/git/gitcmd/hooks_options.go @@ -6,6 +6,7 @@ import ( "fmt" "gitlab.com/gitlab-org/gitaly/v16/internal/featureflag" + "gitlab.com/gitlab-org/gitaly/v16/internal/git" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/grpc/metadata" @@ -30,7 +31,7 @@ func WithDisabledHooks() CmdOpt { // WithRefTxHook returns an option that populates the safe command with the // environment variables necessary to properly execute a reference hook for // repository changes that may possibly update references -func WithRefTxHook(repo storage.Repository) CmdOpt { +func WithRefTxHook(repo storage.Repository, objectHash git.ObjectHash) CmdOpt { return func(ctx context.Context, cfg config.Cfg, gitCmdFactory CommandFactory, cc *cmdCfg) error { if repo == nil { return fmt.Errorf("missing repo: %w", ErrInvalidArg) @@ -45,7 +46,7 @@ func WithRefTxHook(repo storage.Repository) CmdOpt { GitAlternateObjectDirectories: repo.GetGitAlternateObjectDirectories(), GitObjectDirectory: repo.GetGitObjectDirectory(), RelativePath: repo.GetRelativePath(), - }, cfg, gitCmdFactory, nil, ReferenceTransactionHook); err != nil { + }, objectHash, cfg, gitCmdFactory, nil, ReferenceTransactionHook); err != nil { return fmt.Errorf("ref hook env var: %w", err) } @@ -54,7 +55,7 @@ func WithRefTxHook(repo storage.Repository) CmdOpt { } // WithPackObjectsHookEnv provides metadata for gitaly-hooks so it can act as a pack-objects hook. -func WithPackObjectsHookEnv(repo *gitalypb.Repository, protocol string) CmdOpt { +func WithPackObjectsHookEnv(repo *gitalypb.Repository, objectHash git.ObjectHash, protocol string) CmdOpt { return func(ctx context.Context, cfg config.Cfg, gitCmdFactory CommandFactory, cc *cmdCfg) error { if !cfg.PackObjectsCache.Enabled { return nil @@ -74,6 +75,7 @@ func WithPackObjectsHookEnv(repo *gitalypb.Repository, protocol string) CmdOpt { if err := cc.configureHooks( ctx, repo, + objectHash, cfg, gitCmdFactory, userDetails, @@ -103,7 +105,7 @@ type ReceivePackRequest interface { // WithReceivePackHooks returns an option that populates the safe command with the environment // variables necessary to properly execute the pre-receive, update, post-receive, and proc-receive // (if enabled) hooks for git-receive-pack(1). -func WithReceivePackHooks(req ReceivePackRequest, protocol string, enableProcReceive bool) CmdOpt { +func WithReceivePackHooks(req ReceivePackRequest, objectHash git.ObjectHash, protocol string, enableProcReceive bool) CmdOpt { return func(ctx context.Context, cfg config.Cfg, gitCmdFactory CommandFactory, cc *cmdCfg) error { requestedHooks := ReceivePackHooks if enableProcReceive { @@ -112,7 +114,7 @@ func WithReceivePackHooks(req ReceivePackRequest, protocol string, enableProcRec requestedHooks = PreReceiveHook | ProcReceiveHook } - if err := cc.configureHooks(ctx, req.GetRepository(), cfg, gitCmdFactory, &UserDetails{ + if err := cc.configureHooks(ctx, req.GetRepository(), objectHash, cfg, gitCmdFactory, &UserDetails{ UserID: req.GetGlId(), Username: req.GetGlUsername(), Protocol: protocol, @@ -130,6 +132,7 @@ func WithReceivePackHooks(req ReceivePackRequest, protocol string, enableProcRec func (cc *cmdCfg) configureHooks( ctx context.Context, repo *gitalypb.Repository, + objectHash git.ObjectHash, cfg config.Cfg, gitCmdFactory CommandFactory, userDetails *UserDetails, @@ -139,11 +142,6 @@ func (cc *cmdCfg) configureHooks( return errors.New("hooks already configured") } - objectHash, err := DetectObjectHash(ctx, gitCmdFactory, repo) - if err != nil { - return fmt.Errorf("detecting object hash: %w", err) - } - var transaction *txinfo.Transaction if tx, err := txinfo.TransactionFromContext(ctx); err == nil { transaction = &tx diff --git a/internal/git/gitcmd/hooks_options_test.go b/internal/git/gitcmd/hooks_options_test.go index 76746c110f731844f77cdaaea2181bfeb0fb8fd6..9129d45286a885ba9a188cdafe2044d3e73ef1d8 100644 --- a/internal/git/gitcmd/hooks_options_test.go +++ b/internal/git/gitcmd/hooks_options_test.go @@ -8,6 +8,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/command" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gitcmd" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gittest" + "gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v16/internal/grpc/metadata" "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg" @@ -20,12 +21,17 @@ func TestWithRefHook(t *testing.T) { ctx := testhelper.Context(t) cfg := testcfg.Build(t) - repo, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ + repoProto, repoPath := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ SkipCreationViaService: true, }) + + repo := localrepo.NewTestRepo(t, cfg, repoProto) + objectHash, err := repo.ObjectHash(ctx) + require.NoError(t, err) + gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("refs/heads/master")) - opt := gitcmd.WithRefTxHook(repo) + opt := gitcmd.WithRefTxHook(repoProto, objectHash) subCmd := gitcmd.Command{Name: "update-ref", Args: []string{"refs/heads/master", gittest.DefaultObjectHash.ZeroOID.String()}} for _, tt := range []struct { @@ -35,7 +41,7 @@ func TestWithRefHook(t *testing.T) { { name: "NewCommand", fn: func() (*command.Command, error) { - return gittest.NewCommandFactory(t, cfg, gitcmd.WithSkipHooks()).New(ctx, repo, subCmd, opt) + return gittest.NewCommandFactory(t, cfg, gitcmd.WithSkipHooks()).New(ctx, repoProto, subCmd, opt) }, }, } { @@ -74,22 +80,26 @@ func TestWithPackObjectsHookEnv(t *testing.T) { cfg := testcfg.Build(t) cfg.PackObjectsCache.Enabled = true - repo, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ + repoProto, _ := gittest.CreateRepository(t, ctx, cfg, gittest.CreateRepositoryConfig{ SkipCreationViaService: true, }) + repo := localrepo.NewTestRepo(t, cfg, repoProto) + + objectHash, err := repo.ObjectHash(ctx) + require.NoError(t, err) userID := "user-123" username := "username" protocol := "protocol" remoteIP := "1.2.3.4" - opt := gitcmd.WithPackObjectsHookEnv(repo, protocol) + opt := gitcmd.WithPackObjectsHookEnv(repoProto, objectHash, protocol) subCmd := gitcmd.Command{Name: "upload-pack", Args: []string{"a/b/c"}} ctx = grpcmetadata.AppendToOutgoingContext(ctx, "user_id", userID, "username", username, "remote_ip", remoteIP) ctx = metadata.OutgoingToIncoming(ctx) - cmd, err := gittest.NewCommandFactory(t, cfg, gitcmd.WithSkipHooks()).New(ctx, repo, subCmd, opt) + cmd, err := gittest.NewCommandFactory(t, cfg, gitcmd.WithSkipHooks()).New(ctx, repoProto, subCmd, opt) require.NoError(t, err) payload, err := gitcmd.HooksPayloadFromEnv(cmd.Env()) diff --git a/internal/git/housekeeping/worktrees.go b/internal/git/housekeeping/worktrees.go index 6d081d9bcf63db376ca3ec613be486a80106921f..dd29f2ae341a0356a41412ac29d5e949a6e77395 100644 --- a/internal/git/housekeeping/worktrees.go +++ b/internal/git/housekeeping/worktrees.go @@ -100,14 +100,19 @@ func cleanStaleWorktrees(ctx context.Context, repo *localrepo.Repo, threshold ti var errUnknownWorktree = errors.New("unknown worktree") func removeWorktree(ctx context.Context, repo *localrepo.Repo, name string) error { + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("detecting object hash: %w", err) + } + var stderr bytes.Buffer - err := repo.ExecAndWait(ctx, gitcmd.Command{ + err = repo.ExecAndWait(ctx, gitcmd.Command{ Name: "worktree", Action: "remove", Flags: []gitcmd.Option{gitcmd.Flag{Name: "--force"}}, Args: []string{name}, }, - gitcmd.WithRefTxHook(repo), + gitcmd.WithRefTxHook(repo, objectHash), gitcmd.WithStderr(&stderr), ) if isExitWithCode(err, 128) && strings.HasPrefix(stderr.String(), "fatal: '"+name+"' is not a working tree") { @@ -134,6 +139,11 @@ func cleanDisconnectedWorktrees(ctx context.Context, repo *localrepo.Repo) error return err } + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("detecting object hash: %w", err) + } + // Spawning a command is expensive. We thus try to avoid the overhead by first // determining if there could possibly be any work to be done by git-worktree(1). We do so // by reading the directory in which worktrees are stored, and if it's empty then we know @@ -167,5 +177,5 @@ func cleanDisconnectedWorktrees(ctx context.Context, repo *localrepo.Repo) error return repo.ExecAndWait(ctx, gitcmd.Command{ Name: "worktree", Action: "prune", - }, gitcmd.WithRefTxHook(repo)) + }, gitcmd.WithRefTxHook(repo, objectHash)) } diff --git a/internal/git/localrepo/bundle.go b/internal/git/localrepo/bundle.go index ad98ec06be212b17dbcd9fad1c9e6a7808e07920..220345f8e725a4350e4654382098deeab1f8bfd8 100644 --- a/internal/git/localrepo/bundle.go +++ b/internal/git/localrepo/bundle.go @@ -83,6 +83,11 @@ func (repo *Repo) CloneBundle(ctx context.Context, reader io.Reader) error { return err } + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("getting object hash: %w", err) + } + repoPath, err := repo.locator.GetRepoPath(ctx, repo, storage.WithRepositoryVerificationSkipped()) if err != nil { return fmt.Errorf("getting repo path: %w", err) @@ -124,7 +129,7 @@ func (repo *Repo) CloneBundle(ctx context.Context, reader io.Reader) error { Args: []string{"remove", "origin"}, }, gitcmd.WithStderr(&remoteErr), - gitcmd.WithRefTxHook(repo), + gitcmd.WithRefTxHook(repo, objectHash), ) if err != nil { return fmt.Errorf("spawning git-remote: %w", err) diff --git a/internal/git/localrepo/refs_external_test.go b/internal/git/localrepo/refs_external_test.go index eeb916b98d60d5b0bbb384c9208515f8d2fe8e4d..1d92ad4204f0ba98f33b527d5cc60b0352321c7b 100644 --- a/internal/git/localrepo/refs_external_test.go +++ b/internal/git/localrepo/refs_external_test.go @@ -213,6 +213,9 @@ func TestRepo_SetDefaultBranch_errors(t *testing.T) { _, repo := setupRepoWithHooksServer(t, ctx, cfg, testserver.WithTransactionManager(&blockingManager{ch})) + objectHash, err := repo.ObjectHash(ctx) + require.NoError(t, err) + go func() { _ = repo.SetDefaultBranch(ctx, &blockingManager{ch}, "refs/heads/branch") doneCh <- struct{}{} @@ -223,7 +226,7 @@ func TestRepo_SetDefaultBranch_errors(t *testing.T) { err = repo.ExecAndWait(ctx, gitcmd.Command{ Name: "symbolic-ref", Args: []string{"HEAD", "refs/heads/otherbranch"}, - }, gitcmd.WithRefTxHook(repo), gitcmd.WithStderr(&stderr)) + }, gitcmd.WithRefTxHook(repo, objectHash), gitcmd.WithStderr(&stderr)) code, ok := command.ExitStatus(err) require.True(t, ok) diff --git a/internal/git/localrepo/remote.go b/internal/git/localrepo/remote.go index 07d528f3c37578b81c3fe35fa1acd0f2131bc7b7..770803c698ecb04944afea734870058a6ad4d6cb 100644 --- a/internal/git/localrepo/remote.go +++ b/internal/git/localrepo/remote.go @@ -81,6 +81,11 @@ func (repo *Repo) FetchRemote(ctx context.Context, remoteName string, opts Fetch return err } + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("detecting object hash: %w", err) + } + var stderr bytes.Buffer if opts.Stderr == nil { opts.Stderr = &stderr @@ -99,7 +104,7 @@ func (repo *Repo) FetchRemote(ctx context.Context, remoteName string, opts Fetch if opts.DisableTransactions { commandOptions = append(commandOptions, gitcmd.WithDisabledHooks()) } else { - commandOptions = append(commandOptions, gitcmd.WithRefTxHook(repo)) + commandOptions = append(commandOptions, gitcmd.WithRefTxHook(repo, objectHash)) } commandOptions = append(commandOptions, opts.CommandOptions...) @@ -139,6 +144,11 @@ func (repo *Repo) FetchInternal( opts.Stderr = &stderr } + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("detecting object hash: %w", err) + } + commandOptions := []gitcmd.CmdOpt{ gitcmd.WithEnv(opts.Env...), gitcmd.WithStderr(opts.Stderr), @@ -159,7 +169,7 @@ func (repo *Repo) FetchInternal( if opts.DisableTransactions { commandOptions = append(commandOptions, gitcmd.WithDisabledHooks()) } else { - commandOptions = append(commandOptions, gitcmd.WithRefTxHook(repo)) + commandOptions = append(commandOptions, gitcmd.WithRefTxHook(repo, objectHash)) } commandOptions = append(commandOptions, opts.CommandOptions...) diff --git a/internal/git/objectpool/create.go b/internal/git/objectpool/create.go index 0dd6c832c73f193f0710234942276e8acc5598d9..3ab2dac5bbb9d167b331f288eec89cd582d1c240 100644 --- a/internal/git/objectpool/create.go +++ b/internal/git/objectpool/create.go @@ -76,7 +76,7 @@ func Create( }, Args: []string{sourceRepoPath, objectPoolPath}, }, - gitcmd.WithRefTxHook(sourceRepo), + gitcmd.WithRefTxHook(sourceRepo, objectHash), gitcmd.WithStderr(&stderr), // When cloning an empty repository then Git isn't capable to figure out the correct // object hash that the new repository needs to use and just uses the default object diff --git a/internal/git/objectpool/fetch.go b/internal/git/objectpool/fetch.go index ebb5449fc2d1d8e5e94f16cb1db88c8113cdbf61..6c930a571be1c85f514e62689a0e22fe3d87bb8b 100644 --- a/internal/git/objectpool/fetch.go +++ b/internal/git/objectpool/fetch.go @@ -66,6 +66,11 @@ func (o *ObjectPool) FetchFromOrigin(ctx context.Context, origin *localrepo.Repo return fmt.Errorf("pruning references: %w", err) } + objectHash, err := o.Repo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("detecting object hash: %w", err) + } + var stderr bytes.Buffer if err := o.Repo.ExecAndWait(ctx, gitcmd.Command{ @@ -88,7 +93,7 @@ func (o *ObjectPool) FetchFromOrigin(ctx context.Context, origin *localrepo.Repo }, Args: []string{originPath, objectPoolRefspec}, }, - gitcmd.WithRefTxHook(o.Repo), + gitcmd.WithRefTxHook(o.Repo, objectHash), gitcmd.WithStderr(&stderr), gitcmd.WithConfig(gitcmd.ConfigPair{ // Git is so kind to point out that we asked it to not show forced updates diff --git a/internal/git/updateref/updateref.go b/internal/git/updateref/updateref.go index 697bb1b988af0c44e48d874e3233710db64da3f4..cfd2e2e8ff9e9b35b62bb11410e485db250072c1 100644 --- a/internal/git/updateref/updateref.go +++ b/internal/git/updateref/updateref.go @@ -300,7 +300,12 @@ func New(ctx context.Context, repo gitcmd.RepositoryExecutor, opts ...UpdaterOpt opt(&cfg) } - txOption := gitcmd.WithRefTxHook(repo) + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return nil, fmt.Errorf("detecting object hash: %w", err) + } + + txOption := gitcmd.WithRefTxHook(repo, objectHash) if cfg.disableTransactions { txOption = gitcmd.WithDisabledHooks() } @@ -325,11 +330,6 @@ func New(ctx context.Context, repo gitcmd.RepositoryExecutor, opts ...UpdaterOpt return nil, err } - objectHash, err := repo.ObjectHash(ctx) - if err != nil { - return nil, fmt.Errorf("detecting object hash: %w", err) - } - referenceBackend, err := repo.ReferenceBackend(ctx) if err != nil { return nil, fmt.Errorf("detecting reference backend: %w", err) diff --git a/internal/gitaly/service/cleanup/rewrite_history.go b/internal/gitaly/service/cleanup/rewrite_history.go index 6d05ed820a4d291a3883aa0e816293448e699bab..4be48149df6e9773786dbee28fafe244e0b67b6d 100644 --- a/internal/gitaly/service/cleanup/rewrite_history.go +++ b/internal/gitaly/service/cleanup/rewrite_history.go @@ -140,6 +140,11 @@ func (s *server) rewriteHistory( ) } + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("detecting object hash: %w", err) + } + var stderr strings.Builder if err := repo.ExecAndWait(ctx, gitcmd.Command{ @@ -167,7 +172,7 @@ func (s *server) rewriteHistory( git.MirrorRefSpec, ), }, - gitcmd.WithRefTxHook(repo), + gitcmd.WithRefTxHook(repo, objectHash), gitcmd.WithStderr(&stderr), gitcmd.WithConfig(gitcmd.ConfigPair{ Key: "advice.fetchShowForcedUpdates", Value: "false", diff --git a/internal/gitaly/service/operations/apply_patch.go b/internal/gitaly/service/operations/apply_patch.go index 33f90e40dfd3c57badfa15d29eb4348773b0e0c9..5d4eb9a211ee8e2ba6badd81a3e1188864622aa4 100644 --- a/internal/gitaly/service/operations/apply_patch.go +++ b/internal/gitaly/service/operations/apply_patch.go @@ -109,7 +109,7 @@ func (s *Server) userApplyPatch(ctx context.Context, header *gitalypb.UserApplyP defer cancel() worktreeName := filepath.Base(worktreePath) - if err := s.removeWorktree(ctx, header.Repository, worktreeName); err != nil { + if err := s.removeWorktree(ctx, repo, worktreeName); err != nil { s.logger.WithField("worktree_name", worktreeName).WithError(err).ErrorContext(ctx, "failed to remove worktree") } }() @@ -134,7 +134,7 @@ func (s *Server) userApplyPatch(ctx context.Context, header *gitalypb.UserApplyP })), gitcmd.WithStdout(&stdout), gitcmd.WithStderr(&stderr), - gitcmd.WithRefTxHook(header.Repository), + gitcmd.WithRefTxHook(repo, objectHash), gitcmd.WithWorktree(worktreePath), ); err != nil { // The Ruby implementation doesn't include stderr in errors, which makes @@ -238,20 +238,30 @@ func (s *Server) addWorktree(ctx context.Context, repo *localrepo.Repo, worktree flags = append(flags, gitcmd.Flag{Name: "--no-checkout"}) } + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("detecting object hash: %w", err) + } + var stderr bytes.Buffer if err := repo.ExecAndWait(ctx, gitcmd.Command{ Name: "worktree", Action: "add", Flags: flags, Args: args, - }, gitcmd.WithStderr(&stderr), gitcmd.WithRefTxHook(repo)); err != nil { + }, gitcmd.WithStderr(&stderr), gitcmd.WithRefTxHook(repo, objectHash)); err != nil { return fmt.Errorf("adding worktree: %w", gitError{ErrMsg: stderr.String(), Err: err}) } return nil } -func (s *Server) removeWorktree(ctx context.Context, repo *gitalypb.Repository, worktreeName string) error { +func (s *Server) removeWorktree(ctx context.Context, repo *localrepo.Repo, worktreeName string) error { + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("detecting object hash: %w", err) + } + cmd, err := s.gitCmdFactory.New(ctx, repo, gitcmd.Command{ Name: "worktree", @@ -259,7 +269,7 @@ func (s *Server) removeWorktree(ctx context.Context, repo *gitalypb.Repository, Flags: []gitcmd.Option{gitcmd.Flag{Name: "--force"}}, Args: []string{worktreeName}, }, - gitcmd.WithRefTxHook(repo), + gitcmd.WithRefTxHook(repo, objectHash), ) if err != nil { return fmt.Errorf("creation of 'worktree remove': %w", err) diff --git a/internal/gitaly/service/ref/find_tag.go b/internal/gitaly/service/ref/find_tag.go index debe94013af645adf2091d61028a6ee0faf0ef8f..e6f3765259d9aa9759314ba21841be5bf663312f 100644 --- a/internal/gitaly/service/ref/find_tag.go +++ b/internal/gitaly/service/ref/find_tag.go @@ -66,6 +66,12 @@ func parseTagLine(ctx context.Context, objectReader catfile.ObjectContentReader, } func (s *server) findTag(ctx context.Context, repo gitcmd.RepositoryExecutor, tagName []byte) (*gitalypb.Tag, error) { + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return nil, fmt.Errorf("detecting object hash: %w", err) + } + + tagCmd, err := repo.Exec(ctx, gitcmd.Command{ Name: "tag", @@ -75,7 +81,7 @@ func (s *server) findTag(ctx context.Context, repo gitcmd.RepositoryExecutor, ta }, Args: []string{string(tagName)}, }, - gitcmd.WithRefTxHook(repo), + gitcmd.WithRefTxHook(repo, objectHash), gitcmd.WithSetupStdout(), ) if err != nil { diff --git a/internal/gitaly/service/repository/create_fork.go b/internal/gitaly/service/repository/create_fork.go index 66bfd14e96f1c91e3ebd1b46fa8ebb280095e2c0..8d9eaaee6748b169d7f28ccd00cf076d8c705764 100644 --- a/internal/gitaly/service/repository/create_fork.go +++ b/internal/gitaly/service/repository/create_fork.go @@ -30,8 +30,10 @@ func (s *server) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest targetRepository := req.Repository sourceRepository := req.SourceRepository - if err := repoutil.Create(ctx, s.logger, s.locator, s.gitCmdFactory, s.txManager, s.repositoryCounter, targetRepository, func(repo *gitalypb.Repository) error { - targetPath, err := s.locator.GetRepoPath(ctx, repo, storage.WithRepositoryVerificationSkipped()) + if err := repoutil.Create(ctx, s.logger, s.locator, s.gitCmdFactory, s.txManager, s.repositoryCounter, targetRepository, func(repoProto *gitalypb.Repository) error { + repo := s.localrepo(repoProto) + + targetPath, err := s.locator.GetRepoPath(ctx, repoProto, storage.WithRepositoryVerificationSkipped()) if err != nil { return err } @@ -98,7 +100,7 @@ func (s *server) CreateFork(ctx context.Context, req *gitalypb.CreateForkRequest return fmt.Errorf("fetching source repo: %w, stderr: %q", err, stderr.String()) } - if target, err := gitcmd.GetSymbolicRef(ctx, s.localrepo(repo), "HEAD"); err != nil { + if target, err := gitcmd.GetSymbolicRef(ctx, repo, "HEAD"); err != nil { return fmt.Errorf("checking whether HEAD reference is sane: %w", err) } else if req.GetRevision() != nil && target.Target != string(req.GetRevision()) { return structerr.NewInternal("HEAD points to unexpected reference").WithMetadata("expected_target", string(req.GetRevision())).WithMetadata("actual_target", target) diff --git a/internal/gitaly/service/repository/create_repository_from_url.go b/internal/gitaly/service/repository/create_repository_from_url.go index b52bea258cf2310b2fe52fb63eae4c4ab803f15a..a055993bf8907036e4ce3eb3cf44c132aba9897d 100644 --- a/internal/gitaly/service/repository/create_repository_from_url.go +++ b/internal/gitaly/service/repository/create_repository_from_url.go @@ -89,8 +89,10 @@ func (s *server) CreateRepositoryFromURL(ctx context.Context, req *gitalypb.Crea return nil, structerr.NewInvalidArgument("%w", err) } - if err := repoutil.Create(ctx, s.logger, s.locator, s.gitCmdFactory, s.txManager, s.repositoryCounter, req.GetRepository(), func(repo *gitalypb.Repository) error { - targetPath, err := s.locator.GetRepoPath(ctx, repo, storage.WithRepositoryVerificationSkipped()) + if err := repoutil.Create(ctx, s.logger, s.locator, s.gitCmdFactory, s.txManager, s.repositoryCounter, req.GetRepository(), func(repoProto *gitalypb.Repository) error { + repo := s.localrepo(repoProto) + + targetPath, err := s.locator.GetRepoPath(ctx, repoProto, storage.WithRepositoryVerificationSkipped()) if err != nil { return fmt.Errorf("getting temporary repository path: %w", err) } diff --git a/internal/gitaly/service/repository/util.go b/internal/gitaly/service/repository/util.go index f2e3ae7363cfa35c20b9721ffcebd8678a104e02..019d52ae1359edfb49a69b26304db812467e327d 100644 --- a/internal/gitaly/service/repository/util.go +++ b/internal/gitaly/service/repository/util.go @@ -5,11 +5,18 @@ import ( "fmt" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gitcmd" - "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb" + "gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo" ) -func (s *server) removeOriginInRepo(ctx context.Context, repository *gitalypb.Repository) error { - cmd, err := s.gitCmdFactory.New(ctx, repository, gitcmd.Command{Name: "remote", Args: []string{"remove", "origin"}}, gitcmd.WithRefTxHook(repository)) +func (s *server) removeOriginInRepo(ctx context.Context, repo *localrepo.Repo) error { + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("detecting object hash: %w", err) + } + + cmd, err := s.gitCmdFactory.New(ctx, repo, + gitcmd.Command{Name: "remote", Args: []string{"remove", "origin"}}, + gitcmd.WithRefTxHook(repo, objectHash)) if err != nil { return fmt.Errorf("remote cmd start: %w", err) } diff --git a/internal/gitaly/service/smarthttp/receive_pack.go b/internal/gitaly/service/smarthttp/receive_pack.go index da9e3544285719928b0f658a44afebda0e67b9b2..f0a3e021ba5d0fe50fa03c8758d7ed6f1ae8fb2d 100644 --- a/internal/gitaly/service/smarthttp/receive_pack.go +++ b/internal/gitaly/service/smarthttp/receive_pack.go @@ -3,6 +3,7 @@ package smarthttp import ( "context" "errors" + "fmt" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gitcmd" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/hook" @@ -76,7 +77,10 @@ func (s *server) postReceivePack( return stream.Send(&gitalypb.PostReceivePackResponse{Data: p}) }) - repoPath, err := s.locator.GetRepoPath(ctx, req.Repository) + repoProto := req.GetRepository() + repo := s.localrepo(repoProto) + + repoPath, err := repo.Path(ctx) if err != nil { return err } @@ -89,7 +93,6 @@ func (s *server) postReceivePack( transactionID := storage.ExtractTransactionID(ctx) transactionsEnabled := transactionID > 0 if transactionsEnabled { - repo := s.localrepo(req.GetRepository()) procReceiveCleanup, err := receivepack.RegisterProcReceiveHook( ctx, s.logger, s.cfg, req, repo, s.hookManager, hook.NewTransactionRegistry(s.txRegistry), transactionID, ) @@ -103,7 +106,12 @@ func (s *server) postReceivePack( }() } - cmd, err := s.gitCmdFactory.New(ctx, req.GetRepository(), + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("detecting object hash: %w", err) + } + + cmd, err := s.gitCmdFactory.New(ctx, repo, gitcmd.Command{ Name: "receive-pack", Flags: []gitcmd.Option{gitcmd.Flag{Name: "--stateless-rpc"}}, @@ -111,7 +119,7 @@ func (s *server) postReceivePack( }, gitcmd.WithStdin(stdin), gitcmd.WithStdout(stdout), - gitcmd.WithReceivePackHooks(req, "http", transactionsEnabled), + gitcmd.WithReceivePackHooks(req, objectHash, "http", transactionsEnabled), gitcmd.WithGitProtocol(s.logger, req), gitcmd.WithConfig(config...), ) diff --git a/internal/gitaly/service/smarthttp/upload_pack.go b/internal/gitaly/service/smarthttp/upload_pack.go index e5d2c4a686eca3ffa25d5892e0a92277f636bd11..6ac5d7a1e3cbaab0d7490880d93063d5907884ed 100644 --- a/internal/gitaly/service/smarthttp/upload_pack.go +++ b/internal/gitaly/service/smarthttp/upload_pack.go @@ -10,6 +10,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/bundleuri" "gitlab.com/gitlab-org/gitaly/v16/internal/command" "gitlab.com/gitlab-org/gitaly/v16/internal/git/gitcmd" + "gitlab.com/gitlab-org/gitaly/v16/internal/git/localrepo" "gitlab.com/gitlab-org/gitaly/v16/internal/git/stats" "gitlab.com/gitlab-org/gitaly/v16/internal/grpc/sidechannel" "gitlab.com/gitlab-org/gitaly/v16/internal/structerr" @@ -17,7 +18,10 @@ import ( ) func (s *server) PostUploadPackWithSidechannel(ctx context.Context, req *gitalypb.PostUploadPackWithSidechannelRequest) (*gitalypb.PostUploadPackWithSidechannelResponse, error) { - repoPath, gitConfig, err := s.validateUploadPackRequest(ctx, req) + repoProto := req.GetRepository() + repo := s.localrepo(repoProto) + + repoPath, gitConfig, err := s.validateUploadPackRequest(ctx, repo, req) if err != nil { return nil, structerr.NewInvalidArgument("%w", err) } @@ -35,7 +39,7 @@ func (s *server) PostUploadPackWithSidechannel(ctx context.Context, req *gitalyp } defer conn.Close() - stats, err := s.runUploadPack(ctx, req, repoPath, gitConfig, conn, conn) + stats, err := s.runUploadPack(ctx, req, repo, repoPath, gitConfig, conn, conn) if err != nil { return nil, structerr.NewInternal("running upload-pack: %w", err) } @@ -83,17 +87,16 @@ func (s *server) runStatsCollector(ctx context.Context, r io.Reader) (io.Reader, return io.TeeReader(r, pw), sc } -func (s *server) validateUploadPackRequest(ctx context.Context, req *gitalypb.PostUploadPackWithSidechannelRequest) (string, []gitcmd.ConfigPair, error) { - repository := req.GetRepository() - if err := s.locator.ValidateRepository(ctx, repository); err != nil { +func (s *server) validateUploadPackRequest(ctx context.Context, repo *localrepo.Repo, req *gitalypb.PostUploadPackWithSidechannelRequest) (string, []gitcmd.ConfigPair, error) { + if err := s.locator.ValidateRepository(ctx, repo); err != nil { return "", nil, err } - repoPath, err := s.locator.GetRepoPath(ctx, repository) + repoPath, err := repo.Path(ctx) if err != nil { return "", nil, err } - gitcmd.WarnIfTooManyBitmaps(ctx, s.logger, s.locator, repository.GetStorageName(), repoPath) + gitcmd.WarnIfTooManyBitmaps(ctx, s.logger, s.locator, repo.GetStorageName(), repoPath) config, err := gitcmd.ConvertConfigOptions(req.GetGitConfigOptions()) if err != nil { @@ -103,7 +106,15 @@ func (s *server) validateUploadPackRequest(ctx context.Context, req *gitalypb.Po return repoPath, config, nil } -func (s *server) runUploadPack(ctx context.Context, req *gitalypb.PostUploadPackWithSidechannelRequest, repoPath string, gitConfig []gitcmd.ConfigPair, stdin io.Reader, stdout io.Writer) (stats *stats.PackfileNegotiation, _ error) { +func (s *server) runUploadPack( + ctx context.Context, + req *gitalypb.PostUploadPackWithSidechannelRequest, + repo *localrepo.Repo, + repoPath string, + gitConfig []gitcmd.ConfigPair, + stdin io.Reader, + stdout io.Writer, +) (stats *stats.PackfileNegotiation, _ error) { h := sha1.New() stdin = io.TeeReader(stdin, h) @@ -116,18 +127,23 @@ func (s *server) runUploadPack(ctx context.Context, req *gitalypb.PostUploadPack gitConfig = append(gitConfig, bundleuri.CapabilitiesGitConfig(ctx)...) - uploadPackConfig, err := bundleuri.UploadPackGitConfig(ctx, s.bundleURISink, req.GetRepository()) + uploadPackConfig, err := bundleuri.UploadPackGitConfig(ctx, s.bundleURISink, repo) if err != nil { } else { gitConfig = append(gitConfig, uploadPackConfig...) } + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return nil, fmt.Errorf("detecting object hash: %w", err) + } + commandOpts := []gitcmd.CmdOpt{ gitcmd.WithStdin(stdin), gitcmd.WithSetupStdout(), gitcmd.WithGitProtocol(s.logger, req), gitcmd.WithConfig(gitConfig...), - gitcmd.WithPackObjectsHookEnv(req.GetRepository(), "http"), + gitcmd.WithPackObjectsHookEnv(req.GetRepository(), objectHash, "http"), } cmd, err := s.gitCmdFactory.New(ctx, req.GetRepository(), gitcmd.Command{ diff --git a/internal/gitaly/service/ssh/receive_pack.go b/internal/gitaly/service/ssh/receive_pack.go index 7229dc26336d67f74710724df674a1bb56456abc..3fd136fe6f6ede4443098c653cfa9f9d9b651cd8 100644 --- a/internal/gitaly/service/ssh/receive_pack.go +++ b/internal/gitaly/service/ssh/receive_pack.go @@ -86,7 +86,10 @@ func (s *server) sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer, }) stderr = io.MultiWriter(&stderrBuilder, stderr) - repoPath, err := s.locator.GetRepoPath(ctx, req.Repository) + repoProto := req.GetRepository() + repo := s.localrepo(repoProto) + + repoPath, err := repo.Path(ctx) if err != nil { return err } @@ -122,7 +125,6 @@ func (s *server) sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer, transactionID := storage.ExtractTransactionID(ctx) transactionsEnabled := transactionID > 0 if transactionsEnabled { - repo := s.localrepo(req.GetRepository()) procReceiveCleanup, err := receivepack.RegisterProcReceiveHook( ctx, s.logger, s.cfg, req, repo, s.hookManager, hook.NewTransactionRegistry(s.txRegistry), transactionID, ) @@ -136,7 +138,12 @@ func (s *server) sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer, }() } - cmd, err := s.gitCmdFactory.New(ctx, req.GetRepository(), + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return fmt.Errorf("detecting object hash: %w", err) + } + + cmd, err := s.gitCmdFactory.New(ctx, repoProto, gitcmd.Command{ Name: "receive-pack", Args: []string{repoPath}, @@ -144,7 +151,7 @@ func (s *server) sshReceivePack(stream gitalypb.SSHService_SSHReceivePackServer, gitcmd.WithStdin(pr), gitcmd.WithStdout(stdout), gitcmd.WithStderr(stderr), - gitcmd.WithReceivePackHooks(req, "ssh", transactionsEnabled), + gitcmd.WithReceivePackHooks(req, objectHash, "ssh", transactionsEnabled), gitcmd.WithGitProtocol(s.logger, req), gitcmd.WithConfig(config...), ) diff --git a/internal/gitaly/service/ssh/upload_pack.go b/internal/gitaly/service/ssh/upload_pack.go index cde3cdde7cd96a6bb15ff76e82c98df2a2d9bff8..29600e797764e2303653eb1e8fc1d5a7163582a4 100644 --- a/internal/gitaly/service/ssh/upload_pack.go +++ b/internal/gitaly/service/ssh/upload_pack.go @@ -74,13 +74,15 @@ type sshUploadPackRequest interface { } func (s *server) sshUploadPack(ctx context.Context, req sshUploadPackRequest, stdin io.Reader, stdout, stderr io.Writer) (negotiation *stats.PackfileNegotiation, _ int, _ error) { - repo := req.GetRepository() - repoPath, err := s.locator.GetRepoPath(ctx, repo) + repoProto := req.GetRepository() + repo := s.localrepo(repoProto) + + repoPath, err := repo.Path(ctx) if err != nil { return nil, 0, err } - gitcmd.WarnIfTooManyBitmaps(ctx, s.logger, s.locator, repo.StorageName, repoPath) + gitcmd.WarnIfTooManyBitmaps(ctx, s.logger, s.locator, repoProto.StorageName, repoPath) config, err := gitcmd.ConvertConfigOptions(req.GetGitConfigOptions()) if err != nil { @@ -114,17 +116,22 @@ func (s *server) sshUploadPack(ctx context.Context, req sshUploadPackRequest, st config = append(config, bundleuri.CapabilitiesGitConfig(ctx)...) - uploadPackConfig, err := bundleuri.UploadPackGitConfig(ctx, s.bundleURISink, req.GetRepository()) + uploadPackConfig, err := bundleuri.UploadPackGitConfig(ctx, s.bundleURISink, repo) if err != nil { log.AddFields(ctx, log.Fields{"bundle_uri_error": err}) } else { config = append(config, uploadPackConfig...) } + objectHash, err := repo.ObjectHash(ctx) + if err != nil { + return nil, 0, fmt.Errorf("detecting object hash: %w", err) + } + commandOpts := []gitcmd.CmdOpt{ gitcmd.WithGitProtocol(s.logger, req), gitcmd.WithConfig(config...), - gitcmd.WithPackObjectsHookEnv(repo, "ssh"), + gitcmd.WithPackObjectsHookEnv(repoProto, objectHash, "ssh"), } timeoutTicker := s.uploadPackRequestTimeoutTickerFactory() @@ -135,7 +142,7 @@ func (s *server) sshUploadPack(ctx context.Context, req sshUploadPackRequest, st // "flush" tells the server it can terminate, while "done" tells it to start // generating a packfile. Add a timeout to the second case to mitigate // use-after-check attacks. - if err := s.runUploadCommand(ctx, repo, stdin, stdout, stderr, timeoutTicker, pktline.PktDone(), gitcmd.Command{ + if err := s.runUploadCommand(ctx, repoProto, stdin, stdout, stderr, timeoutTicker, pktline.PktDone(), gitcmd.Command{ Name: "upload-pack", Args: []string{repoPath}, }, commandOpts...); err != nil {