From 6bdda24229e7ce3ad6d525826c72d8e82daa1eb7 Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Wed, 10 Jul 2024 17:54:03 +0300 Subject: [PATCH 01/14] Remove production usage of GroupPrivateDir GroupPrivateDir is only used when creating the parent directory hierarchy of a repository. The storage contents of Gitaly don't need to be readable by anyone else than Gitaly. Replace the usage of GroupPrivateDir with standard storage permissions. --- internal/gitaly/repoutil/lock.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/gitaly/repoutil/lock.go b/internal/gitaly/repoutil/lock.go index fb5a06bdb2..315ed40c49 100644 --- a/internal/gitaly/repoutil/lock.go +++ b/internal/gitaly/repoutil/lock.go @@ -6,7 +6,6 @@ import ( "path/filepath" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm" "gitlab.com/gitlab-org/gitaly/v16/internal/log" "gitlab.com/gitlab-org/gitaly/v16/internal/safe" "gitlab.com/gitlab-org/gitaly/v16/internal/structerr" @@ -26,7 +25,7 @@ func Lock(ctx context.Context, logger log.Logger, locator storage.Locator, repos } // Create the parent directory in case it doesn't exist yet. - if err := os.MkdirAll(filepath.Dir(path), perm.GroupPrivateDir); err != nil { + if err := os.MkdirAll(filepath.Dir(path), storage.ModeDirectory.Perm()); err != nil { return nil, structerr.NewInternal("create directories: %w", err) } -- GitLab From 75db5476998394ba2298c454e75fb4ade7caefd5 Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Wed, 10 Jul 2024 18:03:26 +0300 Subject: [PATCH 02/14] Remove GroupPrivateDir permission GroupPrivateDir is only used in tests. Remove the permission type and replace it with the general permission set to use in tests. --- internal/gitaly/service/repository/create_fork_test.go | 6 +++--- .../service/repository/create_repository_from_url_test.go | 2 +- internal/helper/perm/perm.go | 4 ---- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/internal/gitaly/service/repository/create_fork_test.go b/internal/gitaly/service/repository/create_fork_test.go index 8c4f61facf..f1ff59f8dc 100644 --- a/internal/gitaly/service/repository/create_fork_test.go +++ b/internal/gitaly/service/repository/create_fork_test.go @@ -293,7 +293,7 @@ func TestCreateFork_targetExists(t *testing.T) { { desc: "empty target directory", seed: func(t *testing.T, targetPath string) { - require.NoError(t, os.MkdirAll(targetPath, perm.GroupPrivateDir)) + require.NoError(t, os.MkdirAll(targetPath, perm.PrivateDir)) }, expectedErr: func() error { if testhelper.IsWALEnabled() { @@ -306,7 +306,7 @@ func TestCreateFork_targetExists(t *testing.T) { { desc: "non-empty target directory", seed: func(t *testing.T, targetPath string) { - require.NoError(t, os.MkdirAll(targetPath, perm.GroupPrivateDir)) + require.NoError(t, os.MkdirAll(targetPath, perm.PrivateDir)) require.NoError(t, os.WriteFile( filepath.Join(targetPath, "config"), nil, @@ -324,7 +324,7 @@ func TestCreateFork_targetExists(t *testing.T) { { desc: "target file", seed: func(t *testing.T, targetPath string) { - require.NoError(t, os.MkdirAll(filepath.Dir(targetPath), perm.GroupPrivateDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(targetPath), perm.PrivateDir)) require.NoError(t, os.WriteFile(targetPath, nil, perm.SharedFile)) }, expectedErr: func() error { 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 eb5e609ce9..587bdd7365 100644 --- a/internal/gitaly/service/repository/create_repository_from_url_test.go +++ b/internal/gitaly/service/repository/create_repository_from_url_test.go @@ -139,7 +139,7 @@ testing of this scenario should be left to the relevant package. importedRepoPath := filepath.Join(cfg.Storages[0].Path, importedRepo.GetRelativePath()) if testCase.isDir { - require.NoError(t, os.MkdirAll(importedRepoPath, perm.GroupPrivateDir)) + require.NoError(t, os.MkdirAll(importedRepoPath, perm.PrivateDir)) } else { require.NoError(t, os.MkdirAll(filepath.Dir(importedRepoPath), perm.PublicDir)) require.NoError(t, os.WriteFile(importedRepoPath, nil, perm.SharedFile)) diff --git a/internal/helper/perm/perm.go b/internal/helper/perm/perm.go index 74deab6620..519c1a5d17 100644 --- a/internal/helper/perm/perm.go +++ b/internal/helper/perm/perm.go @@ -13,10 +13,6 @@ const ( // used by gitaly. PrivateDir fs.FileMode = 0o700 - // GroupPrivateDir is the permissions given for a directory that must only - // be used by gitaly and the git group. - GroupPrivateDir fs.FileMode = 0o770 - // SharedDir is the permission given for a directory that may be read // outside of gitaly. SharedDir fs.FileMode = 0o755 -- GitLab From a83025b24c2b1f90f64702b070dddbccf17ad5b9 Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Wed, 10 Jul 2024 18:13:05 +0300 Subject: [PATCH 03/14] Remove production usage of PublicDir Outside of tests, PublicDirs permission is used in two locations: 1. Quarantine directories are marked PublicDir. 2. `custom_hooks/` directories are marked PublicDir. Quarantines are internal runtime state of Gitaly and don't need to be readable by others. Custom hooks directories in the repositories also shouldn't need to be accessible by anything but Gitaly. Replace the usage of PublicDir with the standard storage permissions. --- internal/cli/gitaly/subcmd_hooks_test.go | 2 +- internal/git/quarantine/quarantine.go | 3 +-- internal/gitaly/repoutil/custom_hooks.go | 3 +-- internal/testhelper/directory.go | 1 + 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/internal/cli/gitaly/subcmd_hooks_test.go b/internal/cli/gitaly/subcmd_hooks_test.go index 5f2caafcae..37e3187b48 100644 --- a/internal/cli/gitaly/subcmd_hooks_test.go +++ b/internal/cli/gitaly/subcmd_hooks_test.go @@ -50,7 +50,7 @@ func TestSetHooksSubcommand(t *testing.T) { // TAR does not store the directory mode in the mode field. It's stored // in the type field of the header. Remove the directory mode bit. storage.ModeDirectory^fs.ModeDir, - umask.Mask(fs.ModePerm), + umask.Mask(perm.PrivateDir), ) expectedExecutableMode := testhelper.WithOrWithoutWAL( diff --git a/internal/git/quarantine/quarantine.go b/internal/git/quarantine/quarantine.go index ef53092804..44fab4a930 100644 --- a/internal/git/quarantine/quarantine.go +++ b/internal/git/quarantine/quarantine.go @@ -10,7 +10,6 @@ import ( "strings" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm" "gitlab.com/gitlab-org/gitaly/v16/internal/log" "gitlab.com/gitlab-org/gitaly/v16/internal/safe" "gitlab.com/gitlab-org/gitaly/v16/internal/structerr" @@ -128,7 +127,7 @@ func migrate(sourcePath, targetPath string) error { nestedSourcePath := filepath.Join(sourcePath, entry.Name()) if entry.IsDir() { - if err := os.Mkdir(nestedTargetPath, perm.PublicDir); err != nil { + if err := os.Mkdir(nestedTargetPath, storage.ModeDirectory.Perm()); err != nil { if !errors.Is(err, os.ErrExist) { return fmt.Errorf("creating target directory %q: %w", nestedTargetPath, err) } diff --git a/internal/gitaly/repoutil/custom_hooks.go b/internal/gitaly/repoutil/custom_hooks.go index ef274ed993..94c8956ce7 100644 --- a/internal/gitaly/repoutil/custom_hooks.go +++ b/internal/gitaly/repoutil/custom_hooks.go @@ -16,7 +16,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/command" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm" "gitlab.com/gitlab-org/gitaly/v16/internal/log" "gitlab.com/gitlab-org/gitaly/v16/internal/safe" "gitlab.com/gitlab-org/gitaly/v16/internal/structerr" @@ -154,7 +153,7 @@ func SetCustomHooks( // it means the repository should be set with an empty `custom_hooks` // directory. Create `custom_hooks` in the temporary directory so that any // existing repository hooks will be replaced with this empty directory. - if err := os.Mkdir(tempHooksPath, perm.PublicDir); err != nil && !errors.Is(err, fs.ErrExist) { + if err := os.Mkdir(tempHooksPath, storage.ModeDirectory.Perm()); err != nil && !errors.Is(err, fs.ErrExist) { return fmt.Errorf("making temp hooks directory: %w", err) } diff --git a/internal/testhelper/directory.go b/internal/testhelper/directory.go index 5acea75698..5964d20ded 100644 --- a/internal/testhelper/directory.go +++ b/internal/testhelper/directory.go @@ -147,6 +147,7 @@ func MustCreateCustomHooksTar(tb testing.TB) io.Reader { writer := tar.NewWriter(&buffer) defer MustClose(tb, writer) + require.NoError(tb, writer.WriteHeader(&tar.Header{Name: "custom_hooks/", Mode: int64(perm.PrivateDir)})) writeFile(writer, "custom_hooks/pre-commit", perm.SharedExecutable, "pre-commit content") writeFile(writer, "custom_hooks/pre-push", perm.SharedExecutable, "pre-push content") writeFile(writer, "custom_hooks/pre-receive", perm.SharedExecutable, "pre-receive content") -- GitLab From d1199c625ba0b292e48771141c572f0333cd845c Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Wed, 10 Jul 2024 18:24:41 +0300 Subject: [PATCH 04/14] Remove PublicDir permission PublicDir permission is only used in tests. Remove the permission type and replace it with the general permission set to use in tests. --- internal/archive/archive_test.go | 4 ++-- internal/backup/backup_test.go | 6 +++--- internal/bundleuri/sink_test.go | 2 +- internal/cgroups/handler_linux_test.go | 6 +++--- internal/git/housekeeping/worktrees_test.go | 2 +- internal/git/localrepo/paths_test.go | 2 +- internal/git/quarantine/quarantine_test.go | 2 +- internal/gitaly/config/config_test.go | 6 +++--- internal/gitaly/config/temp_dir_test.go | 2 +- internal/gitaly/maintenance/randomwalker_test.go | 4 ++-- internal/gitaly/repoutil/create_test.go | 12 ++++++------ .../gitaly/service/commit/list_all_commits_test.go | 4 ++-- .../repository/create_repository_from_url_test.go | 2 +- internal/gitaly/service/repository/replicate_test.go | 2 +- .../storage/storagemgr/transaction_manager_test.go | 2 +- internal/gitlab/test_server.go | 2 +- internal/helper/perm/perm.go | 4 ---- internal/testhelper/testcfg/binaries.go | 2 +- internal/testhelper/testhelper.go | 2 +- 19 files changed, 32 insertions(+), 36 deletions(-) diff --git a/internal/archive/archive_test.go b/internal/archive/archive_test.go index 39c2abbed4..d4baa58c33 100644 --- a/internal/archive/archive_test.go +++ b/internal/archive/archive_test.go @@ -25,7 +25,7 @@ func TestWriteTarball(t *testing.T) { // regular file writeFile(t, filepath.Join(srcDir, "a.txt"), []byte("a")) // empty dir - require.NoError(t, os.Mkdir(filepath.Join(srcDir, "empty_dir"), perm.PublicDir)) + require.NoError(t, os.Mkdir(filepath.Join(srcDir, "empty_dir"), perm.PrivateDir)) // file with long name writeFile(t, filepath.Join(srcDir, strings.Repeat("b", 150)+".txt"), []byte("b")) // regular file that is not expected to be part of the archive (not in the members list) @@ -70,7 +70,7 @@ func TestWriteTarball(t *testing.T) { cfg := testcfg.Build(t) dstDir := filepath.Join(tempDir, "dst") - require.NoError(t, os.Mkdir(dstDir, perm.PublicDir)) + require.NoError(t, os.Mkdir(dstDir, perm.PrivateDir)) output, err := exec.Command("tar", "-xf", tarPath, "-C", dstDir).CombinedOutput() require.NoErrorf(t, err, "%s", output) diff := gittest.ExecOpts(t, cfg, gittest.ExecConfig{ExpectedExitCode: 1}, "diff", "--no-index", "--name-only", "--exit-code", dstDir, srcDir) diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index db1300474f..233a0efbb8 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -121,7 +121,7 @@ func TestManager_Create(t *testing.T) { setup: func(tb testing.TB, vanityRepo storage.Repository) setupData { repo, repoPath := gittest.CreateRepository(tb, ctx, cfg) gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(git.DefaultBranch)) - require.NoError(tb, os.Mkdir(filepath.Join(repoPath, "custom_hooks"), perm.PublicDir)) + require.NoError(tb, os.Mkdir(filepath.Join(repoPath, "custom_hooks"), perm.PrivateDir)) require.NoError(tb, os.WriteFile(filepath.Join(repoPath, "custom_hooks/pre-commit.sample"), []byte("Some hooks"), perm.PublicFile)) return setupData{ @@ -503,7 +503,7 @@ custom_hooks_path = '%[2]s/custom_hooks.tar' relativePath + ".refs": repoRefs, }) - require.NoError(tb, os.MkdirAll(filepath.Join(backupRoot, relativePath), perm.PublicDir)) + require.NoError(tb, os.MkdirAll(filepath.Join(backupRoot, relativePath), perm.PrivateDir)) return repo, repoChecksum }, @@ -558,7 +558,7 @@ custom_hooks_path = '%[2]s/custom_hooks.tar' relativePath + ".refs": repoRefs, }) - require.NoError(tb, os.MkdirAll(filepath.Join(backupRoot, relativePath), perm.PublicDir)) + require.NoError(tb, os.MkdirAll(filepath.Join(backupRoot, relativePath), perm.PrivateDir)) testhelper.CopyFile(tb, mustCreateCustomHooksArchive(t, ctx), customHooksPath) return repo, repoChecksum diff --git a/internal/bundleuri/sink_test.go b/internal/bundleuri/sink_test.go index 7ab6798bbe..01aa6907af 100644 --- a/internal/bundleuri/sink_test.go +++ b/internal/bundleuri/sink_test.go @@ -98,7 +98,7 @@ func TestSink_SignedURL(t *testing.T) { desc: "signs bundle successfully", setup: func(t *testing.T, sinkDir string, sink *Sink) { path := filepath.Join(sinkDir, sink.relativePath(repo, "default")) - require.NoError(t, os.MkdirAll(filepath.Dir(path), perm.PublicDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(path), perm.PrivateDir)) require.NoError(t, os.WriteFile(path, []byte("hello"), perm.SharedFile)) }, }, diff --git a/internal/cgroups/handler_linux_test.go b/internal/cgroups/handler_linux_test.go index bce5c5f09c..550c32de67 100644 --- a/internal/cgroups/handler_linux_test.go +++ b/internal/cgroups/handler_linux_test.go @@ -665,8 +665,8 @@ func TestPruneOldCgroups(t *testing.T) { tc.cfg.HierarchyRoot, ) - require.NoError(t, os.MkdirAll(cpuRoot, perm.PublicDir)) - require.NoError(t, os.MkdirAll(memoryRoot, perm.PublicDir)) + require.NoError(t, os.MkdirAll(cpuRoot, perm.PrivateDir)) + require.NoError(t, os.MkdirAll(memoryRoot, perm.PrivateDir)) pid := tc.setup(t, tc.cfg, mock) @@ -703,7 +703,7 @@ func TestPruneOldCgroups(t *testing.T) { tc.cfg.Mountpoint, tc.cfg.HierarchyRoot, ) - require.NoError(t, os.MkdirAll(root, perm.PublicDir)) + require.NoError(t, os.MkdirAll(root, perm.PrivateDir)) pid := tc.setup(t, tc.cfg, mock) diff --git a/internal/git/housekeeping/worktrees_test.go b/internal/git/housekeeping/worktrees_test.go index 97fa927ce2..006785f976 100644 --- a/internal/git/housekeeping/worktrees_test.go +++ b/internal/git/housekeeping/worktrees_test.go @@ -71,7 +71,7 @@ func TestRemoveWorktree(t *testing.T) { require.NoError(t, os.RemoveAll(disconnectedWorktreePath)) orphanedWorktreePath := filepath.Join(repoPath, GitlabWorktreePrefix, "orphaned") - require.NoError(t, os.MkdirAll(orphanedWorktreePath, perm.PublicDir)) + require.NoError(t, os.MkdirAll(orphanedWorktreePath, perm.PrivateDir)) for _, tc := range []struct { worktree string diff --git a/internal/git/localrepo/paths_test.go b/internal/git/localrepo/paths_test.go index 43751d81d4..afee31555a 100644 --- a/internal/git/localrepo/paths_test.go +++ b/internal/git/localrepo/paths_test.go @@ -57,7 +57,7 @@ func TestRepo_Path(t *testing.T) { // Recreate the repository as a simple empty directory to simulate // that the repository is in a partially-created state. require.NoError(t, os.RemoveAll(repoPath)) - require.NoError(t, os.MkdirAll(repoPath, perm.PublicDir)) + require.NoError(t, os.MkdirAll(repoPath, perm.PrivateDir)) _, err := repo.Path(ctx) require.Equal(t, structerr.NewFailedPrecondition("%w: %q does not exist", storage.ErrRepositoryNotValid, "objects").WithMetadata("repository_path", repoPath), err) diff --git a/internal/git/quarantine/quarantine_test.go b/internal/git/quarantine/quarantine_test.go index 9e5390148f..7b70a5951a 100644 --- a/internal/git/quarantine/quarantine_test.go +++ b/internal/git/quarantine/quarantine_test.go @@ -27,7 +27,7 @@ func (e entry) create(t *testing.T, root string) { require.True(t, e.contents == "" || e.children == nil, "An entry cannot have both file contents and children") if e.children != nil { - require.NoError(t, os.Mkdir(root, perm.PublicDir)) + require.NoError(t, os.Mkdir(root, perm.PrivateDir)) for name, child := range e.children { child.create(t, filepath.Join(root, name)) diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go index b2a8b5179f..857f5dd51a 100644 --- a/internal/gitaly/config/config_test.go +++ b/internal/gitaly/config/config_test.go @@ -553,7 +553,7 @@ func TestValidateStorages(t *testing.T) { repositories := testhelper.TempDir(t) repositories2 := testhelper.TempDir(t) nestedRepositories := filepath.Join(repositories, "nested") - require.NoError(t, os.MkdirAll(nestedRepositories, perm.PublicDir)) + require.NoError(t, os.MkdirAll(nestedRepositories, perm.PrivateDir)) filePath := filepath.Join(testhelper.TempDir(t), "temporary-file") require.NoError(t, os.WriteFile(filePath, []byte{}, perm.PublicFile)) @@ -1068,7 +1068,7 @@ func TestSetupRuntimeDirectory_validateInternalSocket(t *testing.T) { desc: "symlinked runtime directory", setup: func(t *testing.T) string { runtimeDir := testhelper.TempDir(t) - require.NoError(t, os.Mkdir(filepath.Join(runtimeDir, "sock.d"), perm.PublicDir)) + require.NoError(t, os.Mkdir(filepath.Join(runtimeDir, "sock.d"), perm.PrivateDir)) // Create a symlink which points to the real runtime directory. symlinkDir := testhelper.TempDir(t) @@ -1095,7 +1095,7 @@ func TestSetupRuntimeDirectory_validateInternalSocket(t *testing.T) { runtimeDirTooLongForSockets := filepath.Join(tempDir, strings.Repeat("/nested_directory", 10)) socketDir := filepath.Join(runtimeDirTooLongForSockets, "sock.d") - require.NoError(t, os.MkdirAll(socketDir, perm.PublicDir)) + require.NoError(t, os.MkdirAll(socketDir, perm.PrivateDir)) return runtimeDirTooLongForSockets }, diff --git a/internal/gitaly/config/temp_dir_test.go b/internal/gitaly/config/temp_dir_test.go index 37db197510..c30427d358 100644 --- a/internal/gitaly/config/temp_dir_test.go +++ b/internal/gitaly/config/temp_dir_test.go @@ -74,7 +74,7 @@ func TestPruneOldGitalyProcessDirectories(t *testing.T) { "gitaly-invalidpid", } { dirPath := filepath.Join(baseDir, dirName) - require.NoError(t, os.Mkdir(dirPath, perm.PublicDir)) + require.NoError(t, os.Mkdir(dirPath, perm.PrivateDir)) expectedLogs[dirPath] = "could not prune entry" expectedErrs[dirPath] = errors.New("gitaly process directory contains an unexpected directory") nonPrunableDirs = append(nonPrunableDirs, dirPath) diff --git a/internal/gitaly/maintenance/randomwalker_test.go b/internal/gitaly/maintenance/randomwalker_test.go index 4fd93ced3e..846e46087e 100644 --- a/internal/gitaly/maintenance/randomwalker_test.go +++ b/internal/gitaly/maintenance/randomwalker_test.go @@ -151,7 +151,7 @@ func TestRandomWalk(t *testing.T) { root := testhelper.TempDir(t) for _, dir := range tc.dirs { - require.NoError(t, os.MkdirAll(filepath.Join(root, dir), perm.PublicDir)) + require.NoError(t, os.MkdirAll(filepath.Join(root, dir), perm.PrivateDir)) } for _, file := range tc.files { @@ -195,7 +195,7 @@ func TestRandomWalk_withRemovedDirs(t *testing.T) { root := testhelper.TempDir(t) for _, dir := range []string{"foo/bar", "foo/bar/deleteme", "foo/baz/qux", "foo/baz/other"} { - require.NoError(t, os.MkdirAll(filepath.Join(root, dir), perm.PublicDir)) + require.NoError(t, os.MkdirAll(filepath.Join(root, dir), perm.PrivateDir)) } walker := newRandomWalker(root, rand.New(rand.NewSource(1))) diff --git a/internal/gitaly/repoutil/create_test.go b/internal/gitaly/repoutil/create_test.go index 18670050f7..75f7633521 100644 --- a/internal/gitaly/repoutil/create_test.go +++ b/internal/gitaly/repoutil/create_test.go @@ -142,7 +142,7 @@ func TestCreate(t *testing.T) { { desc: "preexisting directory", setup: func(t *testing.T, repo *gitalypb.Repository, repoPath string) { - require.NoError(t, os.MkdirAll(repoPath, perm.PublicDir)) + require.NoError(t, os.MkdirAll(repoPath, perm.PrivateDir)) }, verify: func(t *testing.T, tempRepo *gitalypb.Repository, tempRepoPath string, realRepo *gitalypb.Repository, realRepoPath string) { require.NoDirExists(t, tempRepoPath) @@ -159,16 +159,16 @@ func TestCreate(t *testing.T) { { desc: "pre-lock stat fails", setup: func(t *testing.T, repo *gitalypb.Repository, repoPath string) { - require.NoError(t, os.MkdirAll(repoPath, perm.PublicDir)) + require.NoError(t, os.MkdirAll(repoPath, perm.PrivateDir)) parentDir := filepath.Dir(repoPath) // Drop permissions to trigger a stat failure. require.NoError(t, os.Chmod(parentDir, 0)) // Restore the permissions so the directory can be cleaned up. - t.Cleanup(func() { require.NoError(t, os.Chmod(parentDir, perm.PublicDir)) }) + t.Cleanup(func() { require.NoError(t, os.Chmod(parentDir, perm.PrivateDir)) }) }, verify: func(t *testing.T, tempRepo *gitalypb.Repository, tempRepoPath string, realRepo *gitalypb.Repository, realRepoPath string) { // Restore the permissions so the below checks work. - require.NoError(t, os.Chmod(filepath.Dir(realRepoPath), perm.PublicDir)) + require.NoError(t, os.Chmod(filepath.Dir(realRepoPath), perm.PrivateDir)) require.NoDirExists(t, tempRepoPath) @@ -190,7 +190,7 @@ func TestCreate(t *testing.T) { { desc: "locked", setup: func(t *testing.T, repo *gitalypb.Repository, repoPath string) { - require.NoError(t, os.MkdirAll(filepath.Dir(repoPath), perm.PublicDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(repoPath), perm.PrivateDir)) // Lock the target repository such that we must fail. lock, err := os.Create(repoPath + ".lock") @@ -270,7 +270,7 @@ func TestCreate(t *testing.T) { // should try locking the repository before casting any votes, we do // not expect to see a voting error. - require.NoError(t, os.MkdirAll(filepath.Dir(repoPath), perm.PublicDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(repoPath), perm.PrivateDir)) lock, err := os.Create(repoPath + ".lock") require.NoError(t, err) require.NoError(t, lock.Close()) diff --git a/internal/gitaly/service/commit/list_all_commits_test.go b/internal/gitaly/service/commit/list_all_commits_test.go index 862a5081de..774a59e8bf 100644 --- a/internal/gitaly/service/commit/list_all_commits_test.go +++ b/internal/gitaly/service/commit/list_all_commits_test.go @@ -120,7 +120,7 @@ func TestListAllCommits(t *testing.T) { gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("unquarantined")) quarantineDir := filepath.Join("objects", "incoming-123456") - require.NoError(t, os.Mkdir(filepath.Join(repoPath, quarantineDir), perm.PublicDir)) + require.NoError(t, os.Mkdir(filepath.Join(repoPath, quarantineDir), perm.PrivateDir)) repo.GitObjectDirectory = quarantineDir repo.GitAlternateObjectDirectories = nil @@ -139,7 +139,7 @@ func TestListAllCommits(t *testing.T) { gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("unquarantined")) quarantineDir := filepath.Join("objects", "incoming-123456") - require.NoError(t, os.Mkdir(filepath.Join(repoPath, quarantineDir), perm.PublicDir)) + require.NoError(t, os.Mkdir(filepath.Join(repoPath, quarantineDir), perm.PrivateDir)) repo.GitObjectDirectory = quarantineDir repo.GitAlternateObjectDirectories = nil 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 587bdd7365..9fdf60af3d 100644 --- a/internal/gitaly/service/repository/create_repository_from_url_test.go +++ b/internal/gitaly/service/repository/create_repository_from_url_test.go @@ -141,7 +141,7 @@ testing of this scenario should be left to the relevant package. if testCase.isDir { require.NoError(t, os.MkdirAll(importedRepoPath, perm.PrivateDir)) } else { - require.NoError(t, os.MkdirAll(filepath.Dir(importedRepoPath), perm.PublicDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(importedRepoPath), perm.PrivateDir)) require.NoError(t, os.WriteFile(importedRepoPath, nil, perm.SharedFile)) } t.Cleanup(func() { require.NoError(t, os.RemoveAll(importedRepoPath)) }) diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go index 4124c0e097..be135309c3 100644 --- a/internal/gitaly/service/repository/replicate_test.go +++ b/internal/gitaly/service/repository/replicate_test.go @@ -534,7 +534,7 @@ func TestReplicateRepository_transactional(t *testing.T) { // we use a temporary file here to figure out the expected permissions as they would in fact be subject // to change depending on the current umask. noHooksVoteData := [5]byte{'.', 0, 0, 0, 0} - binary.BigEndian.PutUint32(noHooksVoteData[1:], uint32(testhelper.Umask().Mask(perm.PublicDir|fs.ModeDir))) + binary.BigEndian.PutUint32(noHooksVoteData[1:], uint32(testhelper.Umask().Mask(perm.PrivateDir|fs.ModeDir))) noHooksVote := voting.VoteFromData(noHooksVoteData[:]) expectedVotes := []voting.Vote{ diff --git a/internal/gitaly/storage/storagemgr/transaction_manager_test.go b/internal/gitaly/storage/storagemgr/transaction_manager_test.go index a58590db10..96b43d47a1 100644 --- a/internal/gitaly/storage/storagemgr/transaction_manager_test.go +++ b/internal/gitaly/storage/storagemgr/transaction_manager_test.go @@ -57,7 +57,7 @@ func validCustomHooks(tb testing.TB) []byte { writer := tar.NewWriter(&hooks) require.NoError(tb, writer.WriteHeader(&tar.Header{ Name: "custom_hooks/", - Mode: int64(perm.PublicDir), + Mode: int64(perm.PrivateDir), })) require.NoError(tb, writer.WriteHeader(&tar.Header{ diff --git a/internal/gitlab/test_server.go b/internal/gitlab/test_server.go index c6969e0bf7..b0a2ec5ee4 100644 --- a/internal/gitlab/test_server.go +++ b/internal/gitlab/test_server.go @@ -26,7 +26,7 @@ var changeLineRegex = regexp.MustCompile("^[a-f0-9]{40} [a-f0-9]{40} refs/[^ ]+$ func WriteShellSecretFile(tb testing.TB, dir, secretToken string) string { tb.Helper() - require.NoError(tb, os.MkdirAll(dir, perm.PublicDir)) + require.NoError(tb, os.MkdirAll(dir, perm.PrivateDir)) filePath := filepath.Join(dir, ".gitlab_shell_secret") require.NoError(tb, os.WriteFile(filePath, []byte(secretToken), perm.SharedFile)) return filePath diff --git a/internal/helper/perm/perm.go b/internal/helper/perm/perm.go index 519c1a5d17..35351e5d8b 100644 --- a/internal/helper/perm/perm.go +++ b/internal/helper/perm/perm.go @@ -17,10 +17,6 @@ const ( // outside of gitaly. SharedDir fs.FileMode = 0o755 - // PublicDir is the permission given for a directory that may be read or - // written outside of gitaly. - PublicDir fs.FileMode = 0o777 - // PrivateWriteOnceFile is the most restrictive file permission. Given to // files that are expected to be written only once and must be read only by // gitaly. diff --git a/internal/testhelper/testcfg/binaries.go b/internal/testhelper/testcfg/binaries.go index 8d403e8b0b..fe48f88929 100644 --- a/internal/testhelper/testcfg/binaries.go +++ b/internal/testhelper/testcfg/binaries.go @@ -126,7 +126,7 @@ func BuildBinary(tb testing.TB, targetDir, sourcePath string) string { require.FileExists(tb, sharedBinaryPath, "%s does not exist", executableName) require.NoFileExists(tb, targetPath, "%s exists already -- do you try to build it twice?", executableName) - require.NoError(tb, os.MkdirAll(targetDir, perm.PublicDir)) + require.NoError(tb, os.MkdirAll(targetDir, perm.PrivateDir)) // We hard-link the file into place instead of copying it because copying used to cause // ETXTBSY errors in CI. This is likely caused by a bug in the overlay filesystem used by diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index bce4e73f0c..c7ece380a6 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -311,7 +311,7 @@ func ContextWithoutCancel(opts ...ContextOpt) context.Context { func CreateGlobalDirectory(tb testing.TB, name string) string { require.NotEmpty(tb, testDirectory, "global temporary directory does not exist") path := filepath.Join(testDirectory, name) - require.NoError(tb, os.Mkdir(path, perm.PublicDir)) + require.NoError(tb, os.Mkdir(path, perm.PrivateDir)) return path } -- GitLab From 69fb5a9214e501904ac654698ae461c2943b1f7e Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Wed, 10 Jul 2024 19:08:22 +0300 Subject: [PATCH 05/14] Remove production usage of SharedDir SharedDir permission is used in variety of locations such as cache directories, quarantine directories, temporary directories and object directories. Cache, quarantine and temporary directories are internal Gitaly runtime state and don't really need to be accesible to others. Object directories don't need to be accessible to others either as read access should go through Gitaly. --- internal/cache/diskcache.go | 3 +-- internal/cache/keyer.go | 5 ++--- internal/cache/walker.go | 12 ++++++------ internal/git/localrepo/commit.go | 4 ++-- internal/git/localrepo/repo.go | 3 +-- internal/git/objectpool/disconnect.go | 4 ++-- internal/gitaly/repoutil/remove.go | 3 +-- internal/gitaly/service/repository/replicate.go | 2 +- 8 files changed, 16 insertions(+), 20 deletions(-) diff --git a/internal/cache/diskcache.go b/internal/cache/diskcache.go index 65c7caf053..1816874d0f 100644 --- a/internal/cache/diskcache.go +++ b/internal/cache/diskcache.go @@ -12,7 +12,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/dontpanic" "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/helper/perm" "gitlab.com/gitlab-org/gitaly/v16/internal/log" "gitlab.com/gitlab-org/gitaly/v16/internal/safe" "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb" @@ -293,7 +292,7 @@ func (c *DiskCache) PutStream(ctx context.Context, repo *gitalypb.Repository, re } }() - if err := os.MkdirAll(filepath.Dir(reqPath), perm.SharedDir); err != nil { + if err := os.MkdirAll(filepath.Dir(reqPath), storage.ModeDirectory.Perm()); err != nil { return err } diff --git a/internal/cache/keyer.go b/internal/cache/keyer.go index 67ad8fb939..d27477e0b2 100644 --- a/internal/cache/keyer.go +++ b/internal/cache/keyer.go @@ -16,7 +16,6 @@ import ( "github.com/google/uuid" "gitlab.com/gitlab-org/gitaly/v16/internal/featureflag" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" - "gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm" "gitlab.com/gitlab-org/gitaly/v16/internal/log" "gitlab.com/gitlab-org/gitaly/v16/internal/safe" "gitlab.com/gitlab-org/gitaly/v16/internal/version" @@ -65,7 +64,7 @@ func (keyer leaseKeyer) updateLatest(ctx context.Context, repo *gitalypb.Reposit } lPath := latestPath(repoStatePath) - if err := os.MkdirAll(filepath.Dir(lPath), perm.SharedDir); err != nil { + if err := os.MkdirAll(filepath.Dir(lPath), storage.ModeDirectory.Perm()); err != nil { return "", err } @@ -182,7 +181,7 @@ func (keyer leaseKeyer) newPendingLease(ctx context.Context, repo *gitalypb.Repo } pDir := pendingDir(repoStatePath) - if err := os.MkdirAll(pDir, perm.SharedDir); err != nil { + if err := os.MkdirAll(pDir, storage.ModeDirectory.Perm()); err != nil { return "", err } diff --git a/internal/cache/walker.go b/internal/cache/walker.go index 84ccb6ce87..77a38db5b7 100644 --- a/internal/cache/walker.go +++ b/internal/cache/walker.go @@ -15,7 +15,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/dontpanic" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config" - "gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" ) func (c *DiskCache) logWalkErr(err error, path, msg string) { @@ -141,20 +141,20 @@ func (c *DiskCache) startCleanWalker(cacheDir, stateDir string) { // moveAndClear will move the cache to the storage location's // temporary folder, and then remove its contents asynchronously -func (c *DiskCache) moveAndClear(storage config.Storage) error { +func (c *DiskCache) moveAndClear(storageCfg config.Storage) error { if c.cacheConfig.disableMoveAndClear { return nil } - logger := c.logger.WithField("storage", storage.Name) + logger := c.logger.WithField("storage", storageCfg.Name) logger.Info("clearing disk cache object folder") - tempPath, err := c.locator.TempDir(storage.Name) + tempPath, err := c.locator.TempDir(storageCfg.Name) if err != nil { return fmt.Errorf("temp dir: %w", err) } - if err := os.MkdirAll(tempPath, perm.SharedDir); err != nil { + if err := os.MkdirAll(tempPath, storage.ModeDirectory.Perm()); err != nil { return err } @@ -179,7 +179,7 @@ func (c *DiskCache) moveAndClear(storage config.Storage) error { logger.Info("moving disk cache object folder") - cachePath, err := c.locator.CacheDir(storage.Name) + cachePath, err := c.locator.CacheDir(storageCfg.Name) if err != nil { return fmt.Errorf("cache dir: %w", err) } diff --git a/internal/git/localrepo/commit.go b/internal/git/localrepo/commit.go index 250f4cd9d0..2421b65c09 100644 --- a/internal/git/localrepo/commit.go +++ b/internal/git/localrepo/commit.go @@ -15,7 +15,7 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/git" "gitlab.com/gitlab-org/gitaly/v16/internal/git/catfile" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/config" - "gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm" + "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb" ) @@ -148,7 +148,7 @@ func (repo *Repo) WriteCommit(ctx context.Context, cfg WriteCommitConfig) (git.O return "", errors.New("alternate object directory must be an absolute path") } - if err := os.MkdirAll(cfg.AlternateObjectDir, perm.SharedDir); err != nil { + if err := os.MkdirAll(cfg.AlternateObjectDir, storage.ModeDirectory.Perm()); err != nil { return "", err } diff --git a/internal/git/localrepo/repo.go b/internal/git/localrepo/repo.go index 32462ce9a3..3249672e4f 100644 --- a/internal/git/localrepo/repo.go +++ b/internal/git/localrepo/repo.go @@ -18,7 +18,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/git/quarantine" "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/helper/perm" "gitlab.com/gitlab-org/gitaly/v16/internal/log" "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper" "gitlab.com/gitlab-org/gitaly/v16/internal/testhelper/testcfg" @@ -191,7 +190,7 @@ func (repo *Repo) StorageTempDir() (string, error) { return "", err } - if err := os.MkdirAll(tempPath, perm.SharedDir); err != nil { + if err := os.MkdirAll(tempPath, storage.ModeDirectory.Perm()); err != nil { return "", err } diff --git a/internal/git/objectpool/disconnect.go b/internal/git/objectpool/disconnect.go index 8a2e85fa3a..47d65e28b9 100644 --- a/internal/git/objectpool/disconnect.go +++ b/internal/git/objectpool/disconnect.go @@ -13,9 +13,9 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/git" "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/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage/storagectx" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm" "gitlab.com/gitlab-org/gitaly/v16/internal/helper/text" "gitlab.com/gitlab-org/gitaly/v16/internal/log" "gitlab.com/gitlab-org/gitaly/v16/internal/transaction/voting" @@ -90,7 +90,7 @@ func Disconnect(ctx context.Context, repo *localrepo.Repo, logger log.Logger, tx source := filepath.Join(altObjectDir, path) target := filepath.Join(repoPath, "objects", path) - if err := os.MkdirAll(filepath.Dir(target), perm.SharedDir); err != nil { + if err := os.MkdirAll(filepath.Dir(target), storage.ModeDirectory.Perm()); err != nil { return err } diff --git a/internal/gitaly/repoutil/remove.go b/internal/gitaly/repoutil/remove.go index b8cb2a85cc..4a2954f457 100644 --- a/internal/gitaly/repoutil/remove.go +++ b/internal/gitaly/repoutil/remove.go @@ -10,7 +10,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage/counter" "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/transaction" - "gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm" "gitlab.com/gitlab-org/gitaly/v16/internal/log" "gitlab.com/gitlab-org/gitaly/v16/internal/safe" "gitlab.com/gitlab-org/gitaly/v16/internal/structerr" @@ -54,7 +53,7 @@ func remove( return structerr.NewInternal("temporary directory: %w", err) } - if err := os.MkdirAll(tempDir, perm.SharedDir); err != nil { + if err := os.MkdirAll(tempDir, storage.ModeDirectory.Perm()); err != nil { return structerr.NewInternal("%w", err) } diff --git a/internal/gitaly/service/repository/replicate.go b/internal/gitaly/service/repository/replicate.go index 29775ae032..31e3f65ace 100644 --- a/internal/gitaly/service/repository/replicate.go +++ b/internal/gitaly/service/repository/replicate.go @@ -364,7 +364,7 @@ func (s *server) syncGitconfig(ctx context.Context, source, target *gitalypb.Rep func (s *server) writeFile(ctx context.Context, path string, mode os.FileMode, reader io.Reader) (returnedErr error) { parentDir := filepath.Dir(path) - if err := os.MkdirAll(parentDir, perm.SharedDir); err != nil { + if err := os.MkdirAll(parentDir, storage.ModeDirectory.Perm()); err != nil { return err } -- GitLab From f6cc4523d76f0cf5a224e5faff7530d8164b007e Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Wed, 10 Jul 2024 19:14:27 +0300 Subject: [PATCH 06/14] Remove SharedDir permission SharedDir permission is only used in tests. Remove the permission type and replace it with the general permission set to use in tests. --- internal/backup/locator_test.go | 14 +++++------ internal/cache/walker_test.go | 6 ++--- internal/cgroups/mock_linux_test.go | 10 ++++---- internal/git/gittest/commit.go | 2 +- internal/git/gittest/repo.go | 2 +- internal/git/gittest/testhelper_test.go | 6 ++--- .../manager/optimize_repository_ext_test.go | 2 +- .../manager/optimize_repository_test.go | 6 ++--- internal/git/localrepo/blob_test.go | 2 +- internal/git/localrepo/snapshot_test.go | 2 +- internal/git/objectpool/create_test.go | 2 +- internal/git/objectpool/pool_test.go | 2 +- internal/git/stats/repository_info_test.go | 24 +++++++++---------- internal/gitaly/config/locator_test.go | 2 +- internal/gitaly/hook/custom_test.go | 4 ++-- internal/gitaly/repoutil/custom_hooks_test.go | 2 +- .../gitaly/service/objectpool/create_test.go | 2 +- .../objectpool/fetch_into_object_pool_test.go | 2 +- .../gitaly/service/objectpool/get_test.go | 2 +- .../create_bundle_from_ref_list_test.go | 2 +- .../service/repository/create_bundle_test.go | 2 +- .../repository/info_attributes_test.go | 2 +- .../service/repository/optimize_test.go | 2 +- .../service/repository/replicate_test.go | 2 +- .../repository/set_custom_hooks_test.go | 2 +- .../service/repository/snapshot_test.go | 2 +- .../storagemgr/apply_operations_test.go | 4 ++-- internal/gitaly/storage/wal/entry_test.go | 6 ++--- internal/helper/perm/perm.go | 4 ---- internal/streamcache/cache_test.go | 2 +- internal/streamcache/filestore_test.go | 4 ++-- internal/tempdir/clean_test.go | 2 +- internal/testhelper/configure.go | 2 +- internal/testhelper/directory_test.go | 4 ++-- internal/testhelper/logger.go | 2 +- internal/testhelper/testcfg/gitaly.go | 14 +++++------ internal/testhelper/testhelper.go | 4 ++-- 37 files changed, 76 insertions(+), 80 deletions(-) diff --git a/internal/backup/locator_test.go b/internal/backup/locator_test.go index c8f41ef6f0..711bae2197 100644 --- a/internal/backup/locator_test.go +++ b/internal/backup/locator_test.go @@ -147,7 +147,7 @@ func TestPointerLocator(t *testing.T) { expectedBackupID: "abc123", expectedOffset: 1, setup: func(tb testing.TB, ctx context.Context, backupPath string) { - require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, "abc123"), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, "abc123"), perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte("abc123"), perm.SharedFile)) require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "abc123", "LATEST"), []byte("001"), perm.SharedFile)) }, @@ -219,7 +219,7 @@ func TestPointerLocator(t *testing.T) { _, err = l.FindLatest(ctx, repo) require.ErrorIs(t, err, ErrDoesntExist) - require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte(backupID), perm.SharedFile)) require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("003"), perm.SharedFile)) expected := &Backup{ @@ -280,7 +280,7 @@ func TestPointerLocator(t *testing.T) { require.NoError(t, err) require.Equal(t, expectedFallback, fallbackFull) - require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte(backupID), perm.SharedFile)) require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("001"), perm.SharedFile)) expected := &Backup{ @@ -314,7 +314,7 @@ func TestPointerLocator(t *testing.T) { _, err = l.FindLatest(ctx, repo) require.ErrorIs(t, err, ErrDoesntExist) - require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath), perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte("invalid"), perm.SharedFile)) _, err = l.FindLatest(ctx, repo) require.EqualError(t, err, "pointer locator: find latest: find: find latest ID: storage service sink: new reader for \"TestPointerLocator/invalid/LATEST\": doesn't exist") @@ -333,7 +333,7 @@ func TestPointerLocator(t *testing.T) { _, err = l.FindLatest(ctx, repo) require.ErrorIs(t, err, ErrDoesntExist) - require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte(backupID), perm.SharedFile)) require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("invalid"), perm.SharedFile)) @@ -369,7 +369,7 @@ func TestPointerLocator(t *testing.T) { Sink: sink, } - require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("003"), perm.SharedFile)) expected := &Backup{ ID: backupID, @@ -414,7 +414,7 @@ func TestPointerLocator(t *testing.T) { _, err = l.Find(ctx, repo, backupID) require.ErrorIs(t, err, ErrDoesntExist) - require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("invalid"), perm.SharedFile)) _, err = l.Find(ctx, repo, backupID) diff --git a/internal/cache/walker_test.go b/internal/cache/walker_test.go index f9b176f6cc..84c1cfdc08 100644 --- a/internal/cache/walker_test.go +++ b/internal/cache/walker_test.go @@ -37,7 +37,7 @@ func TestDiskCacheObjectWalker(t *testing.T) { require.NoError(t, err) path := filepath.Join(cacheDir, tt.name) - require.NoError(t, os.MkdirAll(filepath.Dir(path), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(path), perm.PrivateDir)) f, err := os.Create(path) require.NoError(t, err) @@ -78,7 +78,7 @@ func TestDiskCacheInitialClear(t *testing.T) { require.NoError(t, err) canary := filepath.Join(cacheDir, "canary.txt") - require.NoError(t, os.MkdirAll(filepath.Dir(canary), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(canary), perm.PrivateDir)) require.NoError(t, os.WriteFile(canary, []byte("chirp chirp"), perm.PublicFile)) cache := New(cfg, locator, testhelper.SharedLogger(t), withDisabledWalker()) @@ -114,7 +114,7 @@ func TestCleanWalkEmptyDirs(t *testing.T) { } { p := filepath.Join(tmp, tt.path) if strings.HasSuffix(tt.path, "/") { - require.NoError(t, os.MkdirAll(p, perm.SharedDir)) + require.NoError(t, os.MkdirAll(p, perm.PrivateDir)) } else { require.NoError(t, os.WriteFile(p, nil, perm.SharedFile)) if tt.stale { diff --git a/internal/cgroups/mock_linux_test.go b/internal/cgroups/mock_linux_test.go index 16f7c19004..88f0b44463 100644 --- a/internal/cgroups/mock_linux_test.go +++ b/internal/cgroups/mock_linux_test.go @@ -78,7 +78,7 @@ func newMockV1(t *testing.T) *mockCgroupV1 { require.NoError(t, err) for _, s := range subsystems { - require.NoError(t, os.MkdirAll(filepath.Join(root, string(s.Name())), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(root, string(s.Name())), perm.PrivateDir)) } return &mockCgroupV1{ @@ -130,7 +130,7 @@ func (m *mockCgroupV1) setupMockCgroupFiles( ) { for _, s := range m.subsystems { cgroupPath := filepath.Join(m.root, string(s.Name()), manager.currentProcessCgroup()) - require.NoError(t, os.MkdirAll(cgroupPath, perm.SharedDir)) + require.NoError(t, os.MkdirAll(cgroupPath, perm.PrivateDir)) content := map[string]string{} for _, ic := range inputContent { @@ -161,7 +161,7 @@ func (m *mockCgroupV1) setupMockCgroupFiles( for _, shard := range shards { shardPath := filepath.Join(cgroupPath, fmt.Sprintf("repos-%d", shard)) - require.NoError(t, os.MkdirAll(shardPath, perm.SharedDir)) + require.NoError(t, os.MkdirAll(shardPath, perm.PrivateDir)) for filename, content := range content { shardControlFilePath := filepath.Join(shardPath, filename) @@ -240,7 +240,7 @@ func (m *mockCgroupV2) setupMockCgroupFiles( } cgroupPath := filepath.Join(m.root, manager.currentProcessCgroup()) - require.NoError(t, os.MkdirAll(cgroupPath, perm.SharedDir)) + require.NoError(t, os.MkdirAll(cgroupPath, perm.PrivateDir)) for filename, content := range content { controlFilePath := filepath.Join(m.root, manager.cfg.HierarchyRoot, filename) @@ -254,7 +254,7 @@ func (m *mockCgroupV2) setupMockCgroupFiles( for _, shard := range shards { shardPath := filepath.Join(cgroupPath, fmt.Sprintf("repos-%d", shard)) - require.NoError(t, os.MkdirAll(shardPath, perm.SharedDir)) + require.NoError(t, os.MkdirAll(shardPath, perm.PrivateDir)) for filename, content := range content { shardControlFilePath := filepath.Join(shardPath, filename) diff --git a/internal/git/gittest/commit.go b/internal/git/gittest/commit.go index 2c507c9695..d6235fdc1c 100644 --- a/internal/git/gittest/commit.go +++ b/internal/git/gittest/commit.go @@ -220,7 +220,7 @@ func WriteCommit(tb testing.TB, cfg config.Cfg, repoPath string, opts ...WriteCo if writeCommitConfig.alternateObjectDir != "" { require.True(tb, filepath.IsAbs(writeCommitConfig.alternateObjectDir), "alternate object directory must be an absolute path") - require.NoError(tb, os.MkdirAll(writeCommitConfig.alternateObjectDir, perm.SharedDir)) + require.NoError(tb, os.MkdirAll(writeCommitConfig.alternateObjectDir, perm.PrivateDir)) env = append(env, fmt.Sprintf("GIT_OBJECT_DIRECTORY=%s", writeCommitConfig.alternateObjectDir), diff --git a/internal/git/gittest/repo.go b/internal/git/gittest/repo.go index c5dc141969..749e7e26ff 100644 --- a/internal/git/gittest/repo.go +++ b/internal/git/gittest/repo.go @@ -38,7 +38,7 @@ const ( // InitRepoDir creates a temporary directory for a repo, without initializing it func InitRepoDir(tb testing.TB, storagePath, relativePath string) *gitalypb.Repository { repoPath := filepath.Join(storagePath, relativePath, "..") - require.NoError(tb, os.MkdirAll(repoPath, perm.SharedDir), "making repo parent dir") + require.NoError(tb, os.MkdirAll(repoPath, perm.PrivateDir), "making repo parent dir") return &gitalypb.Repository{ StorageName: "default", RelativePath: relativePath, diff --git a/internal/git/gittest/testhelper_test.go b/internal/git/gittest/testhelper_test.go index f5c0a1327d..2665eb37ad 100644 --- a/internal/git/gittest/testhelper_test.go +++ b/internal/git/gittest/testhelper_test.go @@ -34,13 +34,13 @@ func setup(tb testing.TB) (config.Cfg, *gitalypb.Repository, string) { Path: filepath.Join(rootDir, "storage.d"), }, } - require.NoError(tb, os.Mkdir(cfg.Storages[0].Path, perm.SharedDir)) + require.NoError(tb, os.Mkdir(cfg.Storages[0].Path, perm.PrivateDir)) cfg.GitlabShell.Dir = filepath.Join(rootDir, "shell.d") - require.NoError(tb, os.Mkdir(cfg.GitlabShell.Dir, perm.SharedDir)) + require.NoError(tb, os.Mkdir(cfg.GitlabShell.Dir, perm.PrivateDir)) cfg.BinDir = filepath.Join(rootDir, "bin.d") - require.NoError(tb, os.Mkdir(cfg.BinDir, perm.SharedDir)) + require.NoError(tb, os.Mkdir(cfg.BinDir, perm.PrivateDir)) cfg.RuntimeDir = filepath.Join(rootDir, "run.d") require.NoError(tb, os.Mkdir(cfg.RuntimeDir, perm.PrivateDir)) diff --git a/internal/git/housekeeping/manager/optimize_repository_ext_test.go b/internal/git/housekeeping/manager/optimize_repository_ext_test.go index dc91822bf1..538993f699 100644 --- a/internal/git/housekeeping/manager/optimize_repository_ext_test.go +++ b/internal/git/housekeeping/manager/optimize_repository_ext_test.go @@ -157,7 +157,7 @@ func TestPruneIfNeeded(t *testing.T) { for _, looseObjectPath := range tc.looseObjects { looseObjectPath := filepath.Join(repoPath, "objects", looseObjectPath) - require.NoError(t, os.MkdirAll(filepath.Dir(looseObjectPath), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(looseObjectPath), perm.PrivateDir)) looseObjectFile, err := os.Create(looseObjectPath) require.NoError(t, err) diff --git a/internal/git/housekeeping/manager/optimize_repository_test.go b/internal/git/housekeeping/manager/optimize_repository_test.go index 7a6ebef536..cab3340de6 100644 --- a/internal/git/housekeeping/manager/optimize_repository_test.go +++ b/internal/git/housekeeping/manager/optimize_repository_test.go @@ -722,7 +722,7 @@ func TestOptimizeRepository(t *testing.T) { // The repack won't repack the following objects because they're // broken, and thus we'll retry to prune them afterwards. - require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "objects", "17"), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "objects", "17"), perm.PrivateDir)) // We set the object's mtime to be almost two weeks ago. Given that // our timeout is at exactly two weeks this shouldn't caused them to @@ -760,7 +760,7 @@ func TestOptimizeRepository(t *testing.T) { // The repack won't repack the following objects because they're // broken, and thus we'll retry to prune them afterwards. - require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "objects", "17"), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "objects", "17"), perm.PrivateDir)) moreThanTwoWeeksAgo := time.Now().Add(stats.StaleObjectsGracePeriod).Add(-time.Minute) @@ -1842,7 +1842,7 @@ func TestRepositoryManager_CleanStaleData_references(t *testing.T) { for _, ref := range tc.refs { path := filepath.Join(repoPath, ref.name) - require.NoError(t, os.MkdirAll(filepath.Dir(path), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(path), perm.PrivateDir)) require.NoError(t, os.WriteFile(path, bytes.Repeat([]byte{0}, ref.size), perm.SharedFile)) filetime := time.Now().Add(-ref.age) require.NoError(t, os.Chtimes(path, filetime, filetime)) diff --git a/internal/git/localrepo/blob_test.go b/internal/git/localrepo/blob_test.go index 0043819283..cb4586d9b2 100644 --- a/internal/git/localrepo/blob_test.go +++ b/internal/git/localrepo/blob_test.go @@ -68,7 +68,7 @@ func TestRepo_WriteBlob(t *testing.T) { // Apply the gitattributes // We should get rid of this with https://gitlab.com/groups/gitlab-org/-/epics/9006 attributesPath := filepath.Join(repoPath, "info", "attributes") - require.NoError(t, os.MkdirAll(filepath.Dir(attributesPath), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(attributesPath), perm.PrivateDir)) require.NoError(t, os.WriteFile(attributesPath, []byte(tc.attributes), perm.PublicFile)) sha, err := repo.WriteBlob(ctx, tc.input, WriteBlobConfig{ diff --git a/internal/git/localrepo/snapshot_test.go b/internal/git/localrepo/snapshot_test.go index 676749a6e8..b7bf639a7c 100644 --- a/internal/git/localrepo/snapshot_test.go +++ b/internal/git/localrepo/snapshot_test.go @@ -98,7 +98,7 @@ doesn't seem to test a realistic scenario.`) require.NoError(t, os.WriteFile(filepath.Join(repoPath, "shallow"), nil, perm.SharedFile)) // Custom Git hooks are not included in snapshots. - require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "hooks"), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "hooks"), perm.PrivateDir)) // Create a file in the objects directory that does not match the regex. require.NoError(t, os.WriteFile( diff --git a/internal/git/objectpool/create_test.go b/internal/git/objectpool/create_test.go index 24458382ec..aa859fff6a 100644 --- a/internal/git/objectpool/create_test.go +++ b/internal/git/objectpool/create_test.go @@ -84,7 +84,7 @@ func TestCreate(t *testing.T) { // We currently allow creating object pools when the target path is an empty // directory. This can be considered a bug, but for now we abide. - require.NoError(t, os.MkdirAll(fullPath, perm.SharedDir)) + require.NoError(t, os.MkdirAll(fullPath, perm.PrivateDir)) _, _, err := createPool(t, &gitalypb.ObjectPool{ Repository: &gitalypb.Repository{ diff --git a/internal/git/objectpool/pool_test.go b/internal/git/objectpool/pool_test.go index 02383d6b7f..47fad57215 100644 --- a/internal/git/objectpool/pool_test.go +++ b/internal/git/objectpool/pool_test.go @@ -112,7 +112,7 @@ func TestFromRepo_failures(t *testing.T) { repoPath, err := repo.Path(ctx) require.NoError(t, err) - require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "objects", "info"), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "objects", "info"), perm.PrivateDir)) alternateFilePath := filepath.Join(repoPath, "objects", "info", "alternates") require.NoError(t, os.WriteFile(alternateFilePath, tc.fileContent, perm.SharedFile)) poolFromRepo, err := FromRepo(ctx, logger, locator, pool.gitCmdFactory, nil, nil, nil, repo) diff --git a/internal/git/stats/repository_info_test.go b/internal/git/stats/repository_info_test.go index d1725eaa7c..e6a2b64a7c 100644 --- a/internal/git/stats/repository_info_test.go +++ b/internal/git/stats/repository_info_test.go @@ -1101,7 +1101,7 @@ func TestCountLooseObjects(t *testing.T) { repo, repoPath := createRepo(t) differentShard := filepath.Join(repoPath, "objects", "a0") - require.NoError(t, os.MkdirAll(differentShard, perm.SharedDir)) + require.NoError(t, os.MkdirAll(differentShard, perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(differentShard, "123456"), []byte("foobar"), perm.SharedFile)) requireLooseObjectsInfo(t, repo, time.Now(), LooseObjectsInfo{ @@ -1117,7 +1117,7 @@ func TestCountLooseObjects(t *testing.T) { for i, shard := range []string{"00", "17", "32", "ff"} { shardPath := filepath.Join(repoPath, "objects", shard) - require.NoError(t, os.MkdirAll(shardPath, perm.SharedDir)) + require.NoError(t, os.MkdirAll(shardPath, perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(shardPath, "123456"), make([]byte, i), perm.SharedFile)) } @@ -1133,7 +1133,7 @@ func TestCountLooseObjects(t *testing.T) { repo, repoPath := createRepo(t) shard := filepath.Join(repoPath, "objects", "17") - require.NoError(t, os.MkdirAll(shard, perm.SharedDir)) + require.NoError(t, os.MkdirAll(shard, perm.PrivateDir)) objectPaths := []string{ filepath.Join(shard, "123456"), @@ -1171,7 +1171,7 @@ func TestCountLooseObjects(t *testing.T) { repo, repoPath := createRepo(t) shard := filepath.Join(repoPath, "objects", "17") - require.NoError(t, os.MkdirAll(shard, perm.SharedDir)) + require.NoError(t, os.MkdirAll(shard, perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(shard, "012345"), []byte("valid"), perm.SharedFile)) require.NoError(t, os.WriteFile(filepath.Join(shard, "garbage"), []byte("garbage"), perm.SharedFile)) @@ -1212,7 +1212,7 @@ func BenchmarkCountLooseObjects(b *testing.B) { repo, repoPath := createRepo(b) objectPath := filepath.Join(repoPath, "objects", "17", "12345") - require.NoError(b, os.Mkdir(filepath.Dir(objectPath), perm.SharedDir)) + require.NoError(b, os.Mkdir(filepath.Dir(objectPath), perm.PrivateDir)) require.NoError(b, os.WriteFile(objectPath, nil, perm.SharedFile)) b.ResetTimer() @@ -1227,7 +1227,7 @@ func BenchmarkCountLooseObjects(b *testing.B) { for i := 0; i < 256; i++ { objectPath := filepath.Join(repoPath, "objects", fmt.Sprintf("%02x", i), "12345") - require.NoError(b, os.Mkdir(filepath.Dir(objectPath), perm.SharedDir)) + require.NoError(b, os.Mkdir(filepath.Dir(objectPath), perm.PrivateDir)) require.NoError(b, os.WriteFile(objectPath, nil, perm.SharedFile)) } @@ -1253,7 +1253,7 @@ func BenchmarkCountLooseObjects(b *testing.B) { for i := 0; i < 256; i++ { shardPath := filepath.Join(repoPath, "objects", fmt.Sprintf("%02x", i)) - require.NoError(b, os.Mkdir(shardPath, perm.SharedDir)) + require.NoError(b, os.Mkdir(shardPath, perm.PrivateDir)) for j := 0; j < looseObjectCount; j++ { objectPath := filepath.Join(shardPath, fmt.Sprintf("%d", j)) @@ -1273,7 +1273,7 @@ func BenchmarkCountLooseObjects(b *testing.B) { for i := 0; i < 256; i++ { shardPath := filepath.Join(repoPath, "objects", fmt.Sprintf("%02x", i)) - require.NoError(b, os.Mkdir(shardPath, perm.SharedDir)) + require.NoError(b, os.Mkdir(shardPath, perm.PrivateDir)) for j := 0; j < 1000; j++ { objectPath := filepath.Join(shardPath, fmt.Sprintf("%d", j)) @@ -1310,7 +1310,7 @@ func TestPackfileInfoForRepository(t *testing.T) { desc: "single packfile", seedRepository: func(t *testing.T, repoPath string) { packfileDir := filepath.Join(repoPath, "objects", "pack") - require.NoError(t, os.MkdirAll(packfileDir, perm.SharedDir)) + require.NoError(t, os.MkdirAll(packfileDir, perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.pack"), []byte("foobar"), perm.SharedFile)) }, expectedInfo: PackfilesInfo{ @@ -1322,7 +1322,7 @@ func TestPackfileInfoForRepository(t *testing.T) { desc: "keep packfile", seedRepository: func(t *testing.T, repoPath string) { packfileDir := filepath.Join(repoPath, "objects", "pack") - require.NoError(t, os.MkdirAll(packfileDir, perm.SharedDir)) + require.NoError(t, os.MkdirAll(packfileDir, perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.pack"), []byte("foobar"), perm.SharedFile)) require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.keep"), []byte("foobar"), perm.SharedFile)) }, @@ -1337,7 +1337,7 @@ func TestPackfileInfoForRepository(t *testing.T) { desc: "cruft packfile", seedRepository: func(t *testing.T, repoPath string) { packfileDir := filepath.Join(repoPath, "objects", "pack") - require.NoError(t, os.MkdirAll(packfileDir, perm.SharedDir)) + require.NoError(t, os.MkdirAll(packfileDir, perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.pack"), []byte("foobar"), perm.SharedFile)) require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.mtimes"), []byte("foobar"), perm.SharedFile)) }, @@ -1352,7 +1352,7 @@ func TestPackfileInfoForRepository(t *testing.T) { desc: "multiple packfiles", seedRepository: func(t *testing.T, repoPath string) { packfileDir := filepath.Join(repoPath, "objects", "pack") - require.NoError(t, os.MkdirAll(packfileDir, perm.SharedDir)) + require.NoError(t, os.MkdirAll(packfileDir, perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.pack"), []byte("foobar"), perm.SharedFile)) require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-bar.pack"), []byte("123"), perm.SharedFile)) }, diff --git a/internal/gitaly/config/locator_test.go b/internal/gitaly/config/locator_test.go index 3a1d39eb1b..a40f177e3a 100644 --- a/internal/gitaly/config/locator_test.go +++ b/internal/gitaly/config/locator_test.go @@ -41,7 +41,7 @@ func TestConfigLocator_GetRepoPath(t *testing.T) { // The repository path exists on the disk, but it is not a git repository. const notRepositoryFolder = "not-a-git-repo" - require.NoError(t, os.MkdirAll(filepath.Join(cfg.Storages[0].Path, notRepositoryFolder), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(cfg.Storages[0].Path, notRepositoryFolder), perm.PrivateDir)) for _, tc := range []struct { desc string diff --git a/internal/gitaly/hook/custom_test.go b/internal/gitaly/hook/custom_test.go index 947b1a1564..9e8bff0ed6 100644 --- a/internal/gitaly/hook/custom_test.go +++ b/internal/gitaly/hook/custom_test.go @@ -267,7 +267,7 @@ func TestCustomHooksWithSymlinks(t *testing.T) { // bad -> /path/to/nowhere BAD firstDir := filepath.Join(globalHooksPath, "first_dir") secondDir := filepath.Join(globalHooksPath, "second_dir") - require.NoError(t, os.MkdirAll(firstDir, perm.SharedDir)) + require.NoError(t, os.MkdirAll(firstDir, perm.PrivateDir)) require.NoError(t, os.Symlink(firstDir, secondDir)) filename := filepath.Join(firstDir, "update") @@ -450,7 +450,7 @@ type customHookResults struct { } func writeCustomHook(t *testing.T, hookName, dir string, content []byte) func() { - require.NoError(t, os.MkdirAll(dir, perm.SharedDir)) + require.NoError(t, os.MkdirAll(dir, perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(dir, hookName), content, perm.SharedExecutable)) return func() { diff --git a/internal/gitaly/repoutil/custom_hooks_test.go b/internal/gitaly/repoutil/custom_hooks_test.go index 2e7658e8a1..afadecb3a5 100644 --- a/internal/gitaly/repoutil/custom_hooks_test.go +++ b/internal/gitaly/repoutil/custom_hooks_test.go @@ -375,7 +375,7 @@ func mustWriteCustomHookDirectory(t *testing.T, files []testFile, dirName string tmpDir := testhelper.TempDir(t) hooksPath := filepath.Join(tmpDir, dirName) - err := os.Mkdir(hooksPath, perm.SharedDir) + err := os.Mkdir(hooksPath, perm.PrivateDir) require.NoError(t, err) for _, f := range files { diff --git a/internal/gitaly/service/objectpool/create_test.go b/internal/gitaly/service/objectpool/create_test.go index 45e7370b16..49452decdd 100644 --- a/internal/gitaly/service/objectpool/create_test.go +++ b/internal/gitaly/service/objectpool/create_test.go @@ -143,7 +143,7 @@ func TestCreate_unsuccessful(t *testing.T) { // gets honored as expected. lockedRelativePath := gittest.NewObjectPoolName(t) lockedFullPath := filepath.Join(cfg.Storages[0].Path, lockedRelativePath+".lock") - require.NoError(t, os.MkdirAll(filepath.Dir(lockedFullPath), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(lockedFullPath), perm.PrivateDir)) require.NoError(t, os.WriteFile(lockedFullPath, nil, perm.SharedFile)) // Create a preexisting object pool. diff --git a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go index bce15c8fc1..006b0af8c2 100644 --- a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go +++ b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go @@ -74,7 +74,7 @@ func TestFetchIntoObjectPool_Success(t *testing.T) { // break many Git commands, including git-fetch(1). We should know to prune stale broken // references though and thus be able to recover. brokenRef := filepath.Join(poolPath, "refs", "heads", "broken") - require.NoError(t, os.MkdirAll(filepath.Dir(brokenRef), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(brokenRef), perm.PrivateDir)) require.NoError(t, os.WriteFile(brokenRef, []byte{}, perm.PublicFile)) oldTime := time.Now().Add(-25 * time.Hour) require.NoError(t, os.Chtimes(brokenRef, oldTime, oldTime)) diff --git a/internal/gitaly/service/objectpool/get_test.go b/internal/gitaly/service/objectpool/get_test.go index 04124be505..6ef44351ba 100644 --- a/internal/gitaly/service/objectpool/get_test.go +++ b/internal/gitaly/service/objectpool/get_test.go @@ -54,7 +54,7 @@ func TestGetObjectPoolBadFile(t *testing.T) { _, repo, repoPath, _, client := setup(t, ctx) alternatesFilePath := filepath.Join(repoPath, "objects", "info", "alternates") - require.NoError(t, os.MkdirAll(filepath.Dir(alternatesFilePath), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(alternatesFilePath), perm.PrivateDir)) require.NoError(t, os.WriteFile(alternatesFilePath, []byte("not-a-directory"), perm.SharedFile)) resp, err := client.GetObjectPool(ctx, &gitalypb.GetObjectPoolRequest{ diff --git a/internal/gitaly/service/repository/create_bundle_from_ref_list_test.go b/internal/gitaly/service/repository/create_bundle_from_ref_list_test.go index 00fa22e08c..6ec20a2947 100644 --- a/internal/gitaly/service/repository/create_bundle_from_ref_list_test.go +++ b/internal/gitaly/service/repository/create_bundle_from_ref_list_test.go @@ -33,7 +33,7 @@ func TestCreateBundleFromRefList_success(t *testing.T) { masterOID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("master"), gittest.WithBranch("master")) sha := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("branch")) - require.NoError(t, os.MkdirAll(filepath.Join(repoPath, housekeeping.GitlabWorktreePrefix), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(repoPath, housekeeping.GitlabWorktreePrefix), perm.PrivateDir)) gittest.Exec(t, cfg, "-C", repoPath, "worktree", "add", filepath.Join(housekeeping.GitlabWorktreePrefix, "worktree1"), sha.String()) require.NoError(t, os.Chtimes(filepath.Join(repoPath, housekeeping.GitlabWorktreePrefix, "worktree1"), time.Now().Add(-7*time.Hour), time.Now().Add(-7*time.Hour))) diff --git a/internal/gitaly/service/repository/create_bundle_test.go b/internal/gitaly/service/repository/create_bundle_test.go index aff86c5c21..f1e655c56f 100644 --- a/internal/gitaly/service/repository/create_bundle_test.go +++ b/internal/gitaly/service/repository/create_bundle_test.go @@ -31,7 +31,7 @@ func TestCreateBundle_successful(t *testing.T) { // Create a work tree with a HEAD pointing to a commit that is missing. CreateBundle should // clean this up before creating the bundle. worktreeCommit := gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("worktree"), gittest.WithBranch("branch")) - require.NoError(t, os.MkdirAll(filepath.Join(repoPath, housekeeping.GitlabWorktreePrefix), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(repoPath, housekeeping.GitlabWorktreePrefix), perm.PrivateDir)) gittest.Exec(t, cfg, "-C", repoPath, "worktree", "add", filepath.Join(housekeeping.GitlabWorktreePrefix, "worktree1"), worktreeCommit.String()) require.NoError(t, os.Chtimes(filepath.Join(repoPath, housekeeping.GitlabWorktreePrefix, "worktree1"), time.Now().Add(-7*time.Hour), time.Now().Add(-7*time.Hour))) gittest.Exec(t, cfg, "-C", repoPath, "branch", "-D", "branch") diff --git a/internal/gitaly/service/repository/info_attributes_test.go b/internal/gitaly/service/repository/info_attributes_test.go index 8106fba70f..75a4e6a4a5 100644 --- a/internal/gitaly/service/repository/info_attributes_test.go +++ b/internal/gitaly/service/repository/info_attributes_test.go @@ -25,7 +25,7 @@ func TestGetInfoAttributesExisting(t *testing.T) { repo, repoPath := gittest.CreateRepository(t, ctx, cfg) infoPath := filepath.Join(repoPath, "info") - require.NoError(t, os.MkdirAll(infoPath, perm.SharedDir)) + require.NoError(t, os.MkdirAll(infoPath, perm.PrivateDir)) buffSize := streamio.WriteBufferSize + 1 data := bytes.Repeat([]byte("*.pbxproj binary\n"), buffSize) diff --git a/internal/gitaly/service/repository/optimize_test.go b/internal/gitaly/service/repository/optimize_test.go index 1bd04c40b3..212bc34f3c 100644 --- a/internal/gitaly/service/repository/optimize_test.go +++ b/internal/gitaly/service/repository/optimize_test.go @@ -224,7 +224,7 @@ func TestOptimizeRepository(t *testing.T) { // Git will leave behind empty refs directories at times. In order to not slow down // enumerating refs we want to make sure that they get cleaned up properly. emptyRefsDir := filepath.Join(repoPath, "refs", "merge-requests", "1") - require.NoError(t, os.MkdirAll(emptyRefsDir, perm.SharedDir)) + require.NoError(t, os.MkdirAll(emptyRefsDir, perm.PrivateDir)) // But we don't expect the first call to OptimizeRepository to do anything. This is // because we have a grace period so that we don't delete empty ref directories that diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go index be135309c3..fd6fe39e87 100644 --- a/internal/gitaly/service/repository/replicate_test.go +++ b/internal/gitaly/service/repository/replicate_test.go @@ -103,7 +103,7 @@ func TestReplicateRepository(t *testing.T) { // created in the target repository as expected. // We should get rid of this with https://gitlab.com/groups/gitlab-org/-/epics/9006 attrFilePath := filepath.Join(sourcePath, "info", "attributes") - require.NoError(t, os.MkdirAll(filepath.Dir(attrFilePath), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Dir(attrFilePath), perm.PrivateDir)) attributesData := []byte("*.pbxproj binary\n") require.NoError(t, os.WriteFile(attrFilePath, attributesData, perm.SharedFile)) diff --git a/internal/gitaly/service/repository/set_custom_hooks_test.go b/internal/gitaly/service/repository/set_custom_hooks_test.go index 823762046b..e10bf90ea4 100644 --- a/internal/gitaly/service/repository/set_custom_hooks_test.go +++ b/internal/gitaly/service/repository/set_custom_hooks_test.go @@ -268,7 +268,7 @@ func mustWriteCustomHookDirectory(t *testing.T, files []testFile, dirName string tmpDir := testhelper.TempDir(t) hooksPath := filepath.Join(tmpDir, dirName) - err := os.Mkdir(hooksPath, perm.SharedDir) + err := os.Mkdir(hooksPath, perm.PrivateDir) require.NoError(t, err) for _, f := range files { diff --git a/internal/gitaly/service/repository/snapshot_test.go b/internal/gitaly/service/repository/snapshot_test.go index 1c1f47953b..c2761f1039 100644 --- a/internal/gitaly/service/repository/snapshot_test.go +++ b/internal/gitaly/service/repository/snapshot_test.go @@ -142,7 +142,7 @@ func TestGetSnapshot(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(repoPath, "shallow"), nil, perm.SharedFile)) // Custom Git hooks are not included in snapshots. - require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "hooks"), perm.SharedDir)) + require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "hooks"), perm.PrivateDir)) // Create a file in the objects directory that does not match the regex. require.NoError(t, os.WriteFile( diff --git a/internal/gitaly/storage/storagemgr/apply_operations_test.go b/internal/gitaly/storage/storagemgr/apply_operations_test.go index f25f8e88fc..0f28269ebd 100644 --- a/internal/gitaly/storage/storagemgr/apply_operations_test.go +++ b/internal/gitaly/storage/storagemgr/apply_operations_test.go @@ -31,9 +31,9 @@ func TestApplyOperations(t *testing.T) { snapshotRoot := filepath.Join(t.TempDir(), "snapshot") testhelper.CreateFS(t, snapshotRoot, fstest.MapFS{ - ".": {Mode: fs.ModeDir | perm.SharedDir}, + ".": {Mode: fs.ModeDir | perm.PrivateDir}, "parent": {Mode: fs.ModeDir | perm.PrivateDir}, - "parent/relative-path": {Mode: fs.ModeDir | perm.SharedDir}, + "parent/relative-path": {Mode: fs.ModeDir | perm.PrivateDir}, "parent/relative-path/private-file": {Mode: perm.PrivateFile, Data: []byte("private")}, "parent/relative-path/shared-file": {Mode: perm.SharedFile, Data: []byte("shared")}, "parent/relative-path/empty-dir": {Mode: fs.ModeDir | perm.PrivateDir}, diff --git a/internal/gitaly/storage/wal/entry_test.go b/internal/gitaly/storage/wal/entry_test.go index 0de6524f15..10d8f8193b 100644 --- a/internal/gitaly/storage/wal/entry_test.go +++ b/internal/gitaly/storage/wal/entry_test.go @@ -26,7 +26,7 @@ func setupTestDirectory(t *testing.T, path string) { require.NoError(t, os.Mkdir(privateSubDir, perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(privateSubDir, "file-2"), []byte("file-2"), perm.SharedFile)) sharedSubDir := filepath.Join(path, "subdir-shared") - require.NoError(t, os.Mkdir(sharedSubDir, perm.SharedDir)) + require.NoError(t, os.Mkdir(sharedSubDir, perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(sharedSubDir, "file-3"), []byte("file-3"), perm.PrivateFile)) } @@ -269,7 +269,7 @@ func TestRecordAlternateUnlink(t *testing.T) { "objects/3f": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/3f/1": {Mode: perm.PrivateFile}, "objects/3f/2": {Mode: perm.SharedFile}, - "objects/4f": {Mode: fs.ModeDir | perm.SharedDir}, + "objects/4f": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/4f/3": {Mode: perm.SharedFile}, "objects/pack": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/pack/pack.pack": {Mode: perm.PrivateFile}, @@ -310,7 +310,7 @@ func TestRecordAlternateUnlink(t *testing.T) { "objects": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/3f": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/3f/1": {Mode: perm.PrivateFile}, - "objects/4f": {Mode: fs.ModeDir | perm.SharedDir}, + "objects/4f": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/4f/3": {Mode: perm.SharedFile}, "objects/pack": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/pack/pack.idx": {Mode: perm.SharedFile}, diff --git a/internal/helper/perm/perm.go b/internal/helper/perm/perm.go index 35351e5d8b..a1177af102 100644 --- a/internal/helper/perm/perm.go +++ b/internal/helper/perm/perm.go @@ -13,10 +13,6 @@ const ( // used by gitaly. PrivateDir fs.FileMode = 0o700 - // SharedDir is the permission given for a directory that may be read - // outside of gitaly. - SharedDir fs.FileMode = 0o755 - // PrivateWriteOnceFile is the most restrictive file permission. Given to // files that are expected to be written only once and must be read only by // gitaly. diff --git a/internal/streamcache/cache_test.go b/internal/streamcache/cache_test.go index c8fdc82a6f..421f6cc38f 100644 --- a/internal/streamcache/cache_test.go +++ b/internal/streamcache/cache_test.go @@ -166,7 +166,7 @@ func TestCache_deletedFile(t *testing.T) { require.True(t, created) require.NoError(t, os.RemoveAll(tmp), "wipe out underlying files of cache") - require.NoError(t, os.MkdirAll(tmp, perm.SharedDir)) + require.NoError(t, os.MkdirAll(tmp, perm.PrivateDir)) // File is gone from filesystem but not from cache requireCacheFiles(t, tmp, 0) diff --git a/internal/streamcache/filestore_test.go b/internal/streamcache/filestore_test.go index 40c8adca45..f722cc7edb 100644 --- a/internal/streamcache/filestore_test.go +++ b/internal/streamcache/filestore_test.go @@ -107,8 +107,8 @@ func TestFilestoreCleanwalk(t *testing.T) { dir1 := filepath.Join(tmp, "dir1") dir2 := filepath.Join(tmp, "dir2") file := filepath.Join(dir2, "file") - require.NoError(t, os.Mkdir(dir1, perm.SharedDir)) - require.NoError(t, os.Mkdir(dir2, perm.SharedDir)) + require.NoError(t, os.Mkdir(dir1, perm.PrivateDir)) + require.NoError(t, os.Mkdir(dir2, perm.PrivateDir)) require.NoError(t, os.WriteFile(file, nil, perm.SharedFile)) require.NoError(t, os.Chmod(dir2, 0), "create dir with pathological permissions") diff --git a/internal/tempdir/clean_test.go b/internal/tempdir/clean_test.go index 16557e1a71..75ce5a8145 100644 --- a/internal/tempdir/clean_test.go +++ b/internal/tempdir/clean_test.go @@ -22,7 +22,7 @@ func TestCleanSuccess(t *testing.T) { cleanRoot, err := locator.TempDir(cfg.Storages[0].Name) require.NoError(t, err) - require.NoError(t, os.MkdirAll(cleanRoot, perm.SharedDir), "create clean root before setup") + require.NoError(t, os.MkdirAll(cleanRoot, perm.PrivateDir), "create clean root before setup") testhelper.MustRunCommand(t, nil, "chmod", "-R", "0700", cleanRoot) require.NoError(t, os.RemoveAll(cleanRoot), "clean up test clean root") diff --git a/internal/testhelper/configure.go b/internal/testhelper/configure.go index 83b77d1c53..70690dfe5a 100644 --- a/internal/testhelper/configure.go +++ b/internal/testhelper/configure.go @@ -206,7 +206,7 @@ func configureTestDirectory(logger log.Logger) (_ func(), returnedErr error) { // around after our tests. To avoid this, we thus set the TMPDIR environment variable to // point into a directory inside of out test directory. globalTempDir := filepath.Join(testDirectory, "tmp") - if err := os.Mkdir(globalTempDir, perm.SharedDir); err != nil { + if err := os.Mkdir(globalTempDir, perm.PrivateDir); err != nil { return nil, fmt.Errorf("creating global temporary directory: %w", err) } if err := os.Setenv("TMPDIR", globalTempDir); err != nil { diff --git a/internal/testhelper/directory_test.go b/internal/testhelper/directory_test.go index 84424d4116..e667902730 100644 --- a/internal/testhelper/directory_test.go +++ b/internal/testhelper/directory_test.go @@ -188,7 +188,7 @@ func TestCreateFS(t *testing.T) { rootPath := filepath.Join(tmpDir, "root") CreateFS(t, rootPath, fstest.MapFS{ - ".": {Mode: fs.ModeDir | perm.SharedDir}, + ".": {Mode: fs.ModeDir | perm.PrivateDir}, "private-dir": {Mode: fs.ModeDir | perm.PrivateDir}, "private-dir/private-file": {Mode: perm.PrivateFile, Data: []byte("private-file")}, "private-dir/subdir": {Mode: fs.ModeDir | perm.PrivateDir}, @@ -199,7 +199,7 @@ func TestCreateFS(t *testing.T) { }) RequireDirectoryState(t, rootPath, "", DirectoryState{ - "/": {Mode: fs.ModeDir | perm.SharedDir}, + "/": {Mode: fs.ModeDir | perm.PrivateDir}, "/private-dir": {Mode: fs.ModeDir | perm.PrivateDir}, "/private-dir/private-file": {Mode: perm.PrivateFile, Content: []byte("private-file")}, "/private-dir/subdir": {Mode: fs.ModeDir | perm.PrivateDir}, diff --git a/internal/testhelper/logger.go b/internal/testhelper/logger.go index 891425ebeb..d59f27d987 100644 --- a/internal/testhelper/logger.go +++ b/internal/testhelper/logger.go @@ -177,7 +177,7 @@ func CreateTestLogDir(tb testing.TB) string { logDir := filepath.Join(testLogDir, tb.Name()) - require.NoError(tb, os.MkdirAll(logDir, perm.SharedDir)) + require.NoError(tb, os.MkdirAll(logDir, perm.PrivateDir)) return logDir } diff --git a/internal/testhelper/testcfg/gitaly.go b/internal/testhelper/testcfg/gitaly.go index ea7954e314..61cd68a609 100644 --- a/internal/testhelper/testcfg/gitaly.go +++ b/internal/testhelper/testcfg/gitaly.go @@ -78,7 +78,7 @@ func (gc *GitalyCfgBuilder) Build(tb testing.TB) config.Cfg { if cfg.BinDir == "" { cfg.BinDir = filepath.Join(root, "bin.d") - require.NoError(tb, os.Mkdir(cfg.BinDir, perm.SharedDir)) + require.NoError(tb, os.Mkdir(cfg.BinDir, perm.PrivateDir)) } if cfg.Logging.Dir == "" { @@ -87,19 +87,19 @@ func (gc *GitalyCfgBuilder) Build(tb testing.TB) config.Cfg { cfg.Logging.Dir = logDir } else { cfg.Logging.Dir = filepath.Join(root, "log.d") - require.NoError(tb, os.Mkdir(cfg.Logging.Dir, perm.SharedDir)) + require.NoError(tb, os.Mkdir(cfg.Logging.Dir, perm.PrivateDir)) } } if cfg.GitlabShell.Dir == "" { cfg.GitlabShell.Dir = filepath.Join(root, "shell.d") - require.NoError(tb, os.Mkdir(cfg.GitlabShell.Dir, perm.SharedDir)) + require.NoError(tb, os.Mkdir(cfg.GitlabShell.Dir, perm.PrivateDir)) } if cfg.RuntimeDir == "" { cfg.RuntimeDir = filepath.Join(root, "runtime.d") require.NoError(tb, os.Mkdir(cfg.RuntimeDir, perm.PrivateDir)) - require.NoError(tb, os.Mkdir(cfg.InternalSocketDir(), perm.SharedDir)) + require.NoError(tb, os.Mkdir(cfg.InternalSocketDir(), perm.PrivateDir)) } if len(cfg.Storages) != 0 && len(gc.storages) != 0 { @@ -116,7 +116,7 @@ func (gc *GitalyCfgBuilder) Build(tb testing.TB) config.Cfg { if cfg.Gitlab.SecretFile == "" { cfg.Gitlab.SecretFile = filepath.Join(root, "gitlab", "http.secret") - require.NoError(tb, os.MkdirAll(filepath.Dir(cfg.Gitlab.SecretFile), perm.SharedDir)) + require.NoError(tb, os.MkdirAll(filepath.Dir(cfg.Gitlab.SecretFile), perm.PrivateDir)) require.NoError(tb, os.WriteFile(cfg.Gitlab.SecretFile, nil, perm.PublicFile)) } @@ -126,7 +126,7 @@ func (gc *GitalyCfgBuilder) Build(tb testing.TB) config.Cfg { if len(cfg.Storages) == 0 { storagesDir := filepath.Join(root, "storages.d") - require.NoError(tb, os.Mkdir(storagesDir, perm.SharedDir)) + require.NoError(tb, os.Mkdir(storagesDir, perm.PrivateDir)) if len(gc.storages) == 0 { gc.storages = []string{"default"} @@ -136,7 +136,7 @@ func (gc *GitalyCfgBuilder) Build(tb testing.TB) config.Cfg { cfg.Storages = make([]config.Storage, len(gc.storages)) for i, storageName := range gc.storages { storagePath := filepath.Join(storagesDir, storageName) - require.NoError(tb, os.MkdirAll(storagePath, perm.SharedDir)) + require.NoError(tb, os.MkdirAll(storagePath, perm.PrivateDir)) cfg.Storages[i].Name = storageName cfg.Storages[i].Path = storagePath } diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index c7ece380a6..2ce8f21ecd 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -116,7 +116,7 @@ func WriteFiles(tb testing.TB, root string, files map[string]any) { for name, value := range files { path := filepath.Join(root, name) - require.NoError(tb, os.MkdirAll(filepath.Dir(path), perm.SharedDir)) + require.NoError(tb, os.MkdirAll(filepath.Dir(path), perm.PrivateDir)) switch content := value.(type) { case string: @@ -339,7 +339,7 @@ type Cleanup func() // executable. func WriteExecutable(tb testing.TB, path string, content []byte) string { dir := filepath.Dir(path) - require.NoError(tb, os.MkdirAll(dir, perm.SharedDir)) + require.NoError(tb, os.MkdirAll(dir, perm.PrivateDir)) tb.Cleanup(func() { assert.NoError(tb, os.RemoveAll(dir)) }) -- GitLab From a47bb8db98057d488f6ad3efe7ddc252e50ddfb3 Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Wed, 10 Jul 2024 19:53:11 +0300 Subject: [PATCH 07/14] Remove production usage of PrivateFile PrivateFile's only production usage is in TransactionManager. For both uses, the file doesn't need write permissions as it's only written to on creation. Replace the usage with general storage permissions. --- internal/gitaly/storage/storagemgr/transaction_manager.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/gitaly/storage/storagemgr/transaction_manager.go b/internal/gitaly/storage/storagemgr/transaction_manager.go index 158157311a..2c1a7be23c 100644 --- a/internal/gitaly/storage/storagemgr/transaction_manager.go +++ b/internal/gitaly/storage/storagemgr/transaction_manager.go @@ -1449,7 +1449,7 @@ func (mgr *TransactionManager) setupStagingRepository(ctx context.Context, trans if err := os.WriteFile( stats.AlternatesFilePath(mgr.getAbsolutePath(transaction.stagingSnapshot.RelativePath(transaction.relativePath))), []byte(alternatesContent), - perm.PrivateFile, + storage.ModeFile.Perm(), ); err != nil { return fmt.Errorf("insert modified alternate file: %w", err) } @@ -3095,7 +3095,7 @@ func (mgr *TransactionManager) appendLogEntry(objectDependencies map[git.ObjectI // Finalize the log entry by writing the MANIFEST file into the log entry's directory. manifestPath := manifestPath(logEntryPath) - if err := os.WriteFile(manifestPath, manifestBytes, perm.PrivateFile); err != nil { + if err := os.WriteFile(manifestPath, manifestBytes, storage.ModeFile.Perm()); err != nil { return fmt.Errorf("write manifest: %w", err) } -- GitLab From ee1a891265f52d0719c4c1badb6dbfecfc6a6d19 Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Wed, 10 Jul 2024 19:58:58 +0300 Subject: [PATCH 08/14] Remove PrivateFile permission PrivateFile permission is only used in tests. Remove the permission type and replace it with the general permission set to use in tests. --- internal/archive/archive_test.go | 2 +- internal/backup/log_entry_test.go | 12 ++++++------ internal/git/dirs_test.go | 8 +++++++- .../housekeeping/manager/optimize_repository_test.go | 2 +- internal/git/housekeeping/manager/testhelper_test.go | 3 ++- internal/git/housekeeping/objects_test.go | 6 +++--- internal/git/stats/repository_info_test.go | 12 ++++++------ internal/gitaly/config/config_test.go | 2 +- internal/gitaly/repoutil/custom_hooks_test.go | 6 +++--- .../service/repository/calculate_checksum_test.go | 2 +- .../service/repository/repository_info_test.go | 2 +- internal/gitaly/service/repository/size_test.go | 4 ++-- .../storage/storagemgr/apply_operations_test.go | 4 ++-- .../transaction_manager_housekeeping_test.go | 4 ++-- .../storage/storagemgr/transaction_manager_test.go | 4 ++-- internal/gitaly/storage/wal/entry_test.go | 10 +++++----- internal/gitaly/storage/walk/walk_test.go | 2 +- internal/helper/perm/perm.go | 4 ---- internal/praefect/config/config_test.go | 2 +- internal/safe/file_writer_test.go | 2 +- internal/testhelper/directory_test.go | 12 ++++++------ 21 files changed, 54 insertions(+), 51 deletions(-) diff --git a/internal/archive/archive_test.go b/internal/archive/archive_test.go index d4baa58c33..95fa1e97f2 100644 --- a/internal/archive/archive_test.go +++ b/internal/archive/archive_test.go @@ -89,5 +89,5 @@ func TestWriteTarball(t *testing.T) { func writeFile(tb testing.TB, path string, data []byte) { tb.Helper() require.NoError(tb, os.MkdirAll(filepath.Dir(path), perm.PrivateDir)) - require.NoError(tb, os.WriteFile(path, data, perm.PrivateFile)) + require.NoError(tb, os.WriteFile(path, data, perm.PrivateWriteOnceFile)) } diff --git a/internal/backup/log_entry_test.go b/internal/backup/log_entry_test.go index e9ad7b4b2a..bbeb9b5e12 100644 --- a/internal/backup/log_entry_test.go +++ b/internal/backup/log_entry_test.go @@ -344,8 +344,8 @@ func TestLogEntryArchiver(t *testing.T) { require.NoError(t, err) testhelper.RequireTarState(t, archive, testhelper.DirectoryState{ lsn.String() + "/": {Mode: umask.Mask(perm.PrivateDir)}, - filepath.Join(lsn.String(), "LSN"): {Mode: umask.Mask(perm.PrivateFile), Content: []byte(lsn.String())}, - filepath.Join(lsn.String(), "PARTITION_ID"): {Mode: umask.Mask(perm.PrivateFile), Content: []byte(fmt.Sprintf("%d", info.partitionID))}, + filepath.Join(lsn.String(), "LSN"): {Mode: umask.Mask(perm.PrivateWriteOnceFile), Content: []byte(lsn.String())}, + filepath.Join(lsn.String(), "PARTITION_ID"): {Mode: umask.Mask(perm.PrivateWriteOnceFile), Content: []byte(fmt.Sprintf("%d", info.partitionID))}, }) } } @@ -452,8 +452,8 @@ func TestLogEntryArchiver_retry(t *testing.T) { testhelper.RequireTarState(t, archive, testhelper.DirectoryState{ lsn.String() + "/": {Mode: umask.Mask(perm.PrivateDir)}, - filepath.Join(lsn.String(), "LSN"): {Mode: umask.Mask(perm.PrivateFile), Content: []byte(lsn.String())}, - filepath.Join(lsn.String(), "PARTITION_ID"): {Mode: umask.Mask(perm.PrivateFile), Content: []byte(fmt.Sprintf("%d", info.partitionID))}, + filepath.Join(lsn.String(), "LSN"): {Mode: umask.Mask(perm.PrivateWriteOnceFile), Content: []byte(lsn.String())}, + filepath.Join(lsn.String(), "PARTITION_ID"): {Mode: umask.Mask(perm.PrivateWriteOnceFile), Content: []byte(fmt.Sprintf("%d", info.partitionID))}, }) require.NoError(t, testutil.CollectAndCompare( @@ -474,8 +474,8 @@ func createEntryDir(t *testing.T, entryRootPath string, storageName string, part testhelper.CreateFS(t, filepath.Join(partitionPath, lsn.String()), fstest.MapFS{ ".": {Mode: fs.ModeDir | perm.PrivateDir}, - "LSN": {Mode: perm.PrivateFile, Data: []byte(lsn.String())}, - "PARTITION_ID": {Mode: perm.PrivateFile, Data: []byte(fmt.Sprintf("%d", partitionID))}, + "LSN": {Mode: perm.PrivateWriteOnceFile, Data: []byte(lsn.String())}, + "PARTITION_ID": {Mode: perm.PrivateWriteOnceFile, Data: []byte(fmt.Sprintf("%d", partitionID))}, }) } diff --git a/internal/git/dirs_test.go b/internal/git/dirs_test.go index 76fcfd7ac9..4323ae424b 100644 --- a/internal/git/dirs_test.go +++ b/internal/git/dirs_test.go @@ -1,6 +1,8 @@ package git import ( + "errors" + "io/fs" "os" "path/filepath" "testing" @@ -82,7 +84,11 @@ func TestObjectDirsOutsideStorage(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { ctx := testhelper.Context(t) - require.NoError(t, os.WriteFile(alternatesFile, []byte(tc.alternates), perm.PrivateFile)) + if err := os.Remove(alternatesFile); !errors.Is(err, fs.ErrNotExist) { + require.NoError(t, err) + } + + require.NoError(t, os.WriteFile(alternatesFile, []byte(tc.alternates), perm.PrivateWriteOnceFile)) out, err := ObjectDirectories(ctx, logger, storageRoot, repoPath) require.Equal(t, expectedErr, err) require.Nil(t, out) diff --git a/internal/git/housekeeping/manager/optimize_repository_test.go b/internal/git/housekeeping/manager/optimize_repository_test.go index cab3340de6..c25e4e126b 100644 --- a/internal/git/housekeeping/manager/optimize_repository_test.go +++ b/internal/git/housekeeping/manager/optimize_repository_test.go @@ -490,7 +490,7 @@ func TestOptimizeRepository(t *testing.T) { require.NoError(t, os.WriteFile( alternatesPath, []byte(alternatesContent), - perm.PrivateFile, + perm.PrivateWriteOnceFile, )) require.NoError(t, os.Chtimes(alternatesPath, date, date)) } diff --git a/internal/git/housekeeping/manager/testhelper_test.go b/internal/git/housekeeping/manager/testhelper_test.go index 0ac4d3c736..289067c31f 100644 --- a/internal/git/housekeeping/manager/testhelper_test.go +++ b/internal/git/housekeeping/manager/testhelper_test.go @@ -2,6 +2,7 @@ package manager import ( "fmt" + "io/fs" "os" "path/filepath" "strings" @@ -289,7 +290,7 @@ func expectDeletion(entry *fileEntry) { func f(name string, opts ...entryOption) *fileEntry { entry := &fileEntry{ name: name, - mode: perm.PrivateFile, + mode: fs.ModePerm, age: ancient, finalState: Keep, } diff --git a/internal/git/housekeeping/objects_test.go b/internal/git/housekeeping/objects_test.go index 6ec4be88de..66f7ced1e1 100644 --- a/internal/git/housekeeping/objects_test.go +++ b/internal/git/housekeeping/objects_test.go @@ -394,7 +394,7 @@ func TestRepackObjects(t *testing.T) { require.Len(t, packPath, 1) keepPath := strings.TrimSuffix(packPath[0], ".pack") + ".keep" - require.NoError(t, os.WriteFile(keepPath, nil, perm.PrivateFile)) + require.NoError(t, os.WriteFile(keepPath, nil, perm.PrivateWriteOnceFile)) }, repackCfg: config.RepackObjectsConfig{ Strategy: config.RepackObjectsStrategyGeometric, @@ -771,7 +771,7 @@ func TestRepackObjects_incrementalWithUnreachable(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(repoPath, "objects", "info", "alternates"), []byte(filepath.Join(alternateRepoPath, "objects")), - perm.PrivateFile, + perm.PrivateWriteOnceFile, )) return setupData{ @@ -807,7 +807,7 @@ func TestRepackObjects_incrementalWithUnreachable(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(repoPath, "objects", "info", "alternates"), []byte(filepath.Join(alternateRepoPath, "objects")), - perm.PrivateFile, + perm.PrivateWriteOnceFile, )) return setupData{ diff --git a/internal/git/stats/repository_info_test.go b/internal/git/stats/repository_info_test.go index e6a2b64a7c..910d2282fb 100644 --- a/internal/git/stats/repository_info_test.go +++ b/internal/git/stats/repository_info_test.go @@ -349,7 +349,7 @@ func TestRepositoryInfoForRepository(t *testing.T) { desc: "garbage", setup: func(t *testing.T, repoPath string) setupData { garbagePath := filepath.Join(repoPath, "objects", "pack", "garbage") - require.NoError(t, os.WriteFile(garbagePath, []byte("x"), perm.PrivateFile)) + require.NoError(t, os.WriteFile(garbagePath, []byte("x"), perm.PrivateWriteOnceFile)) return setupData{ expectedInfo: RepositoryInfo{ @@ -380,7 +380,7 @@ func TestRepositoryInfoForRepository(t *testing.T) { } garbagePath := filepath.Join(repoPath, "reftable", "garbage") - require.NoError(t, os.WriteFile(garbagePath, []byte("x"), perm.PrivateFile)) + require.NoError(t, os.WriteFile(garbagePath, []byte("x"), perm.PrivateWriteOnceFile)) return setupData{ expectedInfo: RepositoryInfo{ @@ -594,7 +594,7 @@ func TestRepositoryInfoForRepository(t *testing.T) { // everywhere. for _, file := range []string{"garbage1", "garbage2", "garbage3"} { garbagePath := filepath.Join(repoPath, "objects", "pack", file) - require.NoError(t, os.WriteFile(garbagePath, []byte("x"), perm.PrivateFile)) + require.NoError(t, os.WriteFile(garbagePath, []byte("x"), perm.PrivateWriteOnceFile)) } return setupData{ @@ -1802,7 +1802,7 @@ func TestMultiPackIndexInfoForPath(t *testing.T) { return func(t *testing.T) string { t.Helper() path := filepath.Join(testhelper.TempDir(t), "midx") - require.NoError(t, os.WriteFile(path, content, perm.PrivateFile)) + require.NoError(t, os.WriteFile(path, content, perm.PrivateWriteOnceFile)) return path } } @@ -1939,7 +1939,7 @@ func TestMultiPackIndexInfoForPath(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(repoPath, "objects", "info", "alternates"), []byte(filepath.Join(poolPath, "objects")), - perm.PrivateFile, + perm.PrivateWriteOnceFile, )) gittest.WriteCommit(t, cfg, repoPath, gittest.WithParents(sharedCommit), gittest.WithBranch("main")) gittest.Exec(t, cfg, "-C", repoPath, "repack", "-Adl", "--write-midx") @@ -2057,6 +2057,6 @@ func hashDependentSize(tb testing.TB, sha1, sha256 uint64) uint64 { func writeFileWithMtime(tb testing.TB, path string, content []byte, date time.Time) { tb.Helper() - require.NoError(tb, os.WriteFile(path, content, perm.PrivateFile)) + require.NoError(tb, os.WriteFile(path, content, perm.PrivateWriteOnceFile)) require.NoError(tb, os.Chtimes(path, date, date)) } diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go index 857f5dd51a..61f845d78d 100644 --- a/internal/gitaly/config/config_test.go +++ b/internal/gitaly/config/config_test.go @@ -306,7 +306,7 @@ func TestLoadConfigCommand(t *testing.T) { desc: "command points to non-executable file", setup: func(t *testing.T) setupData { cmd := filepath.Join(testhelper.TempDir(t), "script") - require.NoError(t, os.WriteFile(cmd, nil, perm.PrivateFile)) + require.NoError(t, os.WriteFile(cmd, nil, perm.PrivateWriteOnceFile)) return setupData{ cfg: Cfg{ diff --git a/internal/gitaly/repoutil/custom_hooks_test.go b/internal/gitaly/repoutil/custom_hooks_test.go index afadecb3a5..22ce7d018d 100644 --- a/internal/gitaly/repoutil/custom_hooks_test.go +++ b/internal/gitaly/repoutil/custom_hooks_test.go @@ -135,7 +135,7 @@ func TestExtractHooks(t *testing.T) { Name: "custom_hooks/subdirectory/", Mode: int64(perm.PrivateDir), })) - writeFile(writer, "custom_hooks/subdirectory/supporting-file", perm.PrivateFile, "supporting-file content") + writeFile(writer, "custom_hooks/subdirectory/supporting-file", perm.PrivateWriteOnceFile, "supporting-file content") writeFile(writer, "ignored_file", fs.ModePerm, "ignored content") writeFile(writer, "ignored_directory/ignored_file", fs.ModePerm, "ignored content") defer testhelper.MustClose(t, writer) @@ -193,7 +193,7 @@ func TestExtractHooks(t *testing.T) { "/custom_hooks": {Mode: umask.Mask(fs.ModeDir | fs.ModePerm)}, "/custom_hooks/pre-receive": {Mode: umask.Mask(fs.ModePerm), Content: []byte("pre-receive content")}, "/custom_hooks/subdirectory": {Mode: umask.Mask(fs.ModeDir | perm.PrivateDir)}, - "/custom_hooks/subdirectory/supporting-file": {Mode: umask.Mask(perm.PrivateFile), Content: []byte("supporting-file content")}, + "/custom_hooks/subdirectory/supporting-file": {Mode: umask.Mask(perm.PrivateWriteOnceFile), Content: []byte("supporting-file content")}, }, }, { @@ -204,7 +204,7 @@ func TestExtractHooks(t *testing.T) { "/": {Mode: umask.Mask(fs.ModeDir | fs.ModePerm)}, "/pre-receive": {Mode: umask.Mask(fs.ModePerm), Content: []byte("pre-receive content")}, "/subdirectory": {Mode: umask.Mask(fs.ModeDir | perm.PrivateDir)}, - "/subdirectory/supporting-file": {Mode: umask.Mask(perm.PrivateFile), Content: []byte("supporting-file content")}, + "/subdirectory/supporting-file": {Mode: umask.Mask(perm.PrivateWriteOnceFile), Content: []byte("supporting-file content")}, }, }, { diff --git a/internal/gitaly/service/repository/calculate_checksum_test.go b/internal/gitaly/service/repository/calculate_checksum_test.go index 9d860be32d..cacff01ee6 100644 --- a/internal/gitaly/service/repository/calculate_checksum_test.go +++ b/internal/gitaly/service/repository/calculate_checksum_test.go @@ -199,7 +199,7 @@ func TestCalculateChecksum(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(repoPath, "packed-refs"), []byte(fmt.Sprintf("# pack-refs with: peeled fully-peeled sorted\n%s refs/heads/broken:reference\n", commitID)), - perm.PrivateFile, + perm.PrivateWriteOnceFile, )) } diff --git a/internal/gitaly/service/repository/repository_info_test.go b/internal/gitaly/service/repository/repository_info_test.go index 7adda57093..a922286545 100644 --- a/internal/gitaly/service/repository/repository_info_test.go +++ b/internal/gitaly/service/repository/repository_info_test.go @@ -28,7 +28,7 @@ func TestRepositoryInfo(t *testing.T) { t.Helper() path := filepath.Join(pathComponents...) require.NoError(t, os.MkdirAll(filepath.Dir(path), perm.PrivateDir)) - require.NoError(t, os.WriteFile(path, bytes.Repeat([]byte{0}, byteCount), perm.PrivateFile)) + require.NoError(t, os.WriteFile(path, bytes.Repeat([]byte{0}, byteCount), perm.PrivateWriteOnceFile)) return path } diff --git a/internal/gitaly/service/repository/size_test.go b/internal/gitaly/service/repository/size_test.go index 02b421220c..09a897e3ed 100644 --- a/internal/gitaly/service/repository/size_test.go +++ b/internal/gitaly/service/repository/size_test.go @@ -67,11 +67,11 @@ func TestRepositorySize_normalRepository(t *testing.T) { requireRepositorySize(t, ctx, client, repo, 16) // Also, updating any other files should cause a size increase. - require.NoError(t, os.WriteFile(filepath.Join(repoPath, "packed-refs"), incompressibleData(7*1024), perm.PrivateFile)) + require.NoError(t, os.WriteFile(filepath.Join(repoPath, "packed-refs"), incompressibleData(7*1024), perm.PrivateWriteOnceFile)) requireRepositorySize(t, ctx, client, repo, 23) // Even garbage should increase the size. - require.NoError(t, os.WriteFile(filepath.Join(repoPath, "garbage"), incompressibleData(5*1024), perm.PrivateFile)) + require.NoError(t, os.WriteFile(filepath.Join(repoPath, "garbage"), incompressibleData(5*1024), perm.PrivateWriteOnceFile)) requireRepositorySize(t, ctx, client, repo, 28) } diff --git a/internal/gitaly/storage/storagemgr/apply_operations_test.go b/internal/gitaly/storage/storagemgr/apply_operations_test.go index 0f28269ebd..dddbf1b7e7 100644 --- a/internal/gitaly/storage/storagemgr/apply_operations_test.go +++ b/internal/gitaly/storage/storagemgr/apply_operations_test.go @@ -34,12 +34,12 @@ func TestApplyOperations(t *testing.T) { ".": {Mode: fs.ModeDir | perm.PrivateDir}, "parent": {Mode: fs.ModeDir | perm.PrivateDir}, "parent/relative-path": {Mode: fs.ModeDir | perm.PrivateDir}, - "parent/relative-path/private-file": {Mode: perm.PrivateFile, Data: []byte("private")}, + "parent/relative-path/private-file": {Mode: perm.PrivateWriteOnceFile, Data: []byte("private")}, "parent/relative-path/shared-file": {Mode: perm.SharedFile, Data: []byte("shared")}, "parent/relative-path/empty-dir": {Mode: fs.ModeDir | perm.PrivateDir}, "parent/relative-path/removed-dir": {Mode: fs.ModeDir | perm.PrivateDir}, "parent/relative-path/dir-with-removed-file": {Mode: fs.ModeDir | perm.PrivateDir}, - "parent/relative-path/dir-with-removed-file/removed-file": {Mode: perm.PrivateFile, Data: []byte("removed")}, + "parent/relative-path/dir-with-removed-file/removed-file": {Mode: perm.PrivateWriteOnceFile, Data: []byte("removed")}, }) umask := testhelper.Umask() diff --git a/internal/gitaly/storage/storagemgr/transaction_manager_housekeeping_test.go b/internal/gitaly/storage/storagemgr/transaction_manager_housekeeping_test.go index 9b25bb013a..5fd9951a6d 100644 --- a/internal/gitaly/storage/storagemgr/transaction_manager_housekeeping_test.go +++ b/internal/gitaly/storage/storagemgr/transaction_manager_housekeeping_test.go @@ -125,12 +125,12 @@ func generateHousekeepingPackRefsTests(t *testing.T, ctx context.Context, testPa require.NoError(t, os.WriteFile( filepath.Join(repoPath, "packed-refs.lock"), []byte{}, - perm.PrivateFile, + perm.PrivateWriteOnceFile, )) require.NoError(t, os.WriteFile( filepath.Join(repoPath, "packed-refs.new"), []byte{}, - perm.PrivateFile, + perm.PrivateWriteOnceFile, )) }, }, diff --git a/internal/gitaly/storage/storagemgr/transaction_manager_test.go b/internal/gitaly/storage/storagemgr/transaction_manager_test.go index 96b43d47a1..ebf74b930b 100644 --- a/internal/gitaly/storage/storagemgr/transaction_manager_test.go +++ b/internal/gitaly/storage/storagemgr/transaction_manager_test.go @@ -40,7 +40,7 @@ var errSimulatedCrash = errors.New("simulated crash") func manifestDirectoryEntry(expected *gitalypb.LogEntry) testhelper.DirectoryEntry { return testhelper.DirectoryEntry{ - Mode: perm.PrivateFile, + Mode: perm.PrivateWriteOnceFile, Content: expected, ParseContent: func(tb testing.TB, path string, content []byte) any { var logEntry gitalypb.LogEntry @@ -76,7 +76,7 @@ func validCustomHooks(tb testing.TB) []byte { require.NoError(tb, writer.WriteHeader(&tar.Header{ Name: "custom_hooks/private-dir/private-file", Size: int64(len("private content")), - Mode: int64(perm.PrivateFile), + Mode: int64(perm.PrivateWriteOnceFile), })) _, err = writer.Write([]byte("private content")) require.NoError(tb, err) diff --git a/internal/gitaly/storage/wal/entry_test.go b/internal/gitaly/storage/wal/entry_test.go index 10d8f8193b..bf7c3b0015 100644 --- a/internal/gitaly/storage/wal/entry_test.go +++ b/internal/gitaly/storage/wal/entry_test.go @@ -27,7 +27,7 @@ func setupTestDirectory(t *testing.T, path string) { require.NoError(t, os.WriteFile(filepath.Join(privateSubDir, "file-2"), []byte("file-2"), perm.SharedFile)) sharedSubDir := filepath.Join(path, "subdir-shared") require.NoError(t, os.Mkdir(sharedSubDir, perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(sharedSubDir, "file-3"), []byte("file-3"), perm.PrivateFile)) + require.NoError(t, os.WriteFile(filepath.Join(sharedSubDir, "file-3"), []byte("file-3"), perm.PrivateWriteOnceFile)) } func TestEntry(t *testing.T) { @@ -37,7 +37,7 @@ func TestEntry(t *testing.T) { firstLevelDir := "test-dir" secondLevelDir := "second-level/test-dir" - require.NoError(t, os.WriteFile(filepath.Join(storageRoot, "root-file"), []byte("root file"), perm.PrivateFile)) + require.NoError(t, os.WriteFile(filepath.Join(storageRoot, "root-file"), []byte("root file"), perm.PrivateWriteOnceFile)) setupTestDirectory(t, filepath.Join(storageRoot, firstLevelDir)) setupTestDirectory(t, filepath.Join(storageRoot, secondLevelDir)) @@ -267,12 +267,12 @@ func TestRecordAlternateUnlink(t *testing.T) { "objects": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/info": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/3f": {Mode: fs.ModeDir | perm.PrivateDir}, - "objects/3f/1": {Mode: perm.PrivateFile}, + "objects/3f/1": {Mode: perm.PrivateWriteOnceFile}, "objects/3f/2": {Mode: perm.SharedFile}, "objects/4f": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/4f/3": {Mode: perm.SharedFile}, "objects/pack": {Mode: fs.ModeDir | perm.PrivateDir}, - "objects/pack/pack.pack": {Mode: perm.PrivateFile}, + "objects/pack/pack.pack": {Mode: perm.PrivateWriteOnceFile}, "objects/pack/pack.idx": {Mode: perm.SharedFile}, }) } @@ -309,7 +309,7 @@ func TestRecordAlternateUnlink(t *testing.T) { ".": {Mode: fs.ModeDir | perm.PrivateDir}, "objects": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/3f": {Mode: fs.ModeDir | perm.PrivateDir}, - "objects/3f/1": {Mode: perm.PrivateFile}, + "objects/3f/1": {Mode: perm.PrivateWriteOnceFile}, "objects/4f": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/4f/3": {Mode: perm.SharedFile}, "objects/pack": {Mode: fs.ModeDir | perm.PrivateDir}, diff --git a/internal/gitaly/storage/walk/walk_test.go b/internal/gitaly/storage/walk/walk_test.go index 96de136380..dfe16bb187 100644 --- a/internal/gitaly/storage/walk/walk_test.go +++ b/internal/gitaly/storage/walk/walk_test.go @@ -75,7 +75,7 @@ func TestWalkStorage(t *testing.T) { err := os.MkdirAll(dir, perm.PrivateDir) require.NoError(t, err) - f, err := os.OpenFile(fullPath, os.O_WRONLY|os.O_CREATE, perm.PrivateFile) + f, err := os.OpenFile(fullPath, os.O_WRONLY|os.O_CREATE, perm.PrivateWriteOnceFile) require.NoError(t, err) require.NoError(t, f.Close()) } diff --git a/internal/helper/perm/perm.go b/internal/helper/perm/perm.go index a1177af102..274e6e26e3 100644 --- a/internal/helper/perm/perm.go +++ b/internal/helper/perm/perm.go @@ -18,10 +18,6 @@ const ( // gitaly. PrivateWriteOnceFile fs.FileMode = 0o400 - // PrivateFile is the permissions given for a file that must only be used - // by gitaly. - PrivateFile fs.FileMode = 0o600 - // SharedFile is the permission given for a file that may be read outside // of gitaly. SharedFile fs.FileMode = 0o644 diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go index 4818959d14..06d4d98c07 100644 --- a/internal/praefect/config/config_test.go +++ b/internal/praefect/config/config_test.go @@ -1026,7 +1026,7 @@ func TestConfig_ConfigCommand(t *testing.T) { desc: "command points to non-executable file", setup: func(t *testing.T) setupData { cmd := filepath.Join(testhelper.TempDir(t), "script") - require.NoError(t, os.WriteFile(cmd, nil, perm.PrivateFile)) + require.NoError(t, os.WriteFile(cmd, nil, perm.PrivateWriteOnceFile)) return setupData{ cfg: Config{ diff --git a/internal/safe/file_writer_test.go b/internal/safe/file_writer_test.go index 25e4c35f6d..5fc091399a 100644 --- a/internal/safe/file_writer_test.go +++ b/internal/safe/file_writer_test.go @@ -49,7 +49,7 @@ func TestFileWriter_mode(t *testing.T) { dir := testhelper.TempDir(t) target := filepath.Join(dir, "file") - require.NoError(t, os.WriteFile(target, []byte("contents"), perm.PrivateFile)) + require.NoError(t, os.WriteFile(target, []byte("contents"), perm.PrivateWriteOnceFile)) writer, err := safe.NewFileWriter(target, safe.FileWriterConfig{ FileMode: 0o060, diff --git a/internal/testhelper/directory_test.go b/internal/testhelper/directory_test.go index e667902730..86b8ec57e1 100644 --- a/internal/testhelper/directory_test.go +++ b/internal/testhelper/directory_test.go @@ -74,7 +74,7 @@ func TestRequireDirectoryState(t *testing.T) { os.WriteFile( filepath.Join(rootDir, relativePath, "parsed-file"), []byte("raw content"), - perm.PrivateFile, + perm.PrivateWriteOnceFile, ), ) @@ -144,7 +144,7 @@ func TestRequireDirectoryState(t *testing.T) { expectedState := DirectoryState{ "/assertion-root": {Mode: umask.Mask(fs.ModeDir | fs.ModePerm)}, "/assertion-root/parsed-file": { - Mode: umask.Mask(perm.PrivateFile), + Mode: umask.Mask(perm.PrivateWriteOnceFile), Content: "parsed content", ParseContent: func(tb testing.TB, path string, content []byte) any { require.Equal(t, filepath.Join(rootDir, "/assertion-root/parsed-file"), path) @@ -190,22 +190,22 @@ func TestCreateFS(t *testing.T) { CreateFS(t, rootPath, fstest.MapFS{ ".": {Mode: fs.ModeDir | perm.PrivateDir}, "private-dir": {Mode: fs.ModeDir | perm.PrivateDir}, - "private-dir/private-file": {Mode: perm.PrivateFile, Data: []byte("private-file")}, + "private-dir/private-file": {Mode: perm.PrivateWriteOnceFile, Data: []byte("private-file")}, "private-dir/subdir": {Mode: fs.ModeDir | perm.PrivateDir}, "private-dir/subdir/subdir-file": {Mode: perm.PrivateDir, Data: []byte("subdir-file")}, "shared-dir": {Mode: fs.ModeDir | perm.PrivateDir}, "shared-dir/shared-file": {Mode: perm.PrivateDir, Data: []byte("shared-file")}, - "root-file": {Mode: perm.PrivateFile, Data: []byte("root-file")}, + "root-file": {Mode: perm.PrivateWriteOnceFile, Data: []byte("root-file")}, }) RequireDirectoryState(t, rootPath, "", DirectoryState{ "/": {Mode: fs.ModeDir | perm.PrivateDir}, "/private-dir": {Mode: fs.ModeDir | perm.PrivateDir}, - "/private-dir/private-file": {Mode: perm.PrivateFile, Content: []byte("private-file")}, + "/private-dir/private-file": {Mode: perm.PrivateWriteOnceFile, Content: []byte("private-file")}, "/private-dir/subdir": {Mode: fs.ModeDir | perm.PrivateDir}, "/private-dir/subdir/subdir-file": {Mode: perm.PrivateDir, Content: []byte("subdir-file")}, "/shared-dir": {Mode: fs.ModeDir | perm.PrivateDir}, "/shared-dir/shared-file": {Mode: perm.PrivateDir, Content: []byte("shared-file")}, - "/root-file": {Mode: perm.PrivateFile, Content: []byte("root-file")}, + "/root-file": {Mode: perm.PrivateWriteOnceFile, Content: []byte("root-file")}, }) } -- GitLab From 509f396a938554b9d8a8513553143432f0ca97b1 Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Mon, 8 Jul 2024 15:17:24 +0300 Subject: [PATCH 09/14] Remove PublicFile permission PublicFile permission is only used within tests. Replace all usage of it with more general storage permissions. --- internal/backup/backup_test.go | 2 +- internal/bootstrap/bootstrap_test.go | 2 +- internal/cache/walker_test.go | 2 +- internal/cgroups/handler_linux_test.go | 2 +- internal/cli/praefect/main_test.go | 2 +- internal/errors/cfgerror/validate_test.go | 4 ++-- internal/git/localrepo/blob_test.go | 9 ++++++++- internal/git/quarantine/quarantine_test.go | 14 +++++++------- internal/gitaly/config/config_test.go | 6 +++--- internal/gitaly/config/temp_dir_test.go | 2 +- internal/gitaly/linguist/language_stats_test.go | 2 +- internal/gitaly/maintenance/randomwalker_test.go | 2 +- internal/gitaly/repoutil/create_test.go | 2 +- .../objectpool/fetch_into_object_pool_test.go | 2 +- .../gitaly/service/repository/replicate_test.go | 2 +- internal/gitaly/transaction/voting_test.go | 2 +- internal/helper/perm/perm.go | 4 ---- internal/praefect/config/config_test.go | 2 +- internal/testhelper/testcfg/gitaly.go | 2 +- internal/testhelper/testhelper.go | 5 +++-- packed_binaries_test.go | 2 +- 21 files changed, 38 insertions(+), 34 deletions(-) diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index 233a0efbb8..ca9046b39f 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -122,7 +122,7 @@ func TestManager_Create(t *testing.T) { repo, repoPath := gittest.CreateRepository(tb, ctx, cfg) gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch(git.DefaultBranch)) require.NoError(tb, os.Mkdir(filepath.Join(repoPath, "custom_hooks"), perm.PrivateDir)) - require.NoError(tb, os.WriteFile(filepath.Join(repoPath, "custom_hooks/pre-commit.sample"), []byte("Some hooks"), perm.PublicFile)) + require.NoError(tb, os.WriteFile(filepath.Join(repoPath, "custom_hooks/pre-commit.sample"), []byte("Some hooks"), perm.PrivateWriteOnceFile)) return setupData{ repo: repo, diff --git a/internal/bootstrap/bootstrap_test.go b/internal/bootstrap/bootstrap_test.go index 27c87e1215..ee508dc748 100644 --- a/internal/bootstrap/bootstrap_test.go +++ b/internal/bootstrap/bootstrap_test.go @@ -78,7 +78,7 @@ func TestBootstrap_unixListener(t *testing.T) { require.NoError(t, err) if tc.preexistingSocket { - require.NoError(t, os.WriteFile(socketPath, nil, perm.PublicFile)) + require.NoError(t, os.WriteFile(socketPath, nil, perm.PrivateWriteOnceFile)) } listener, err := b.listen("unix", socketPath) diff --git a/internal/cache/walker_test.go b/internal/cache/walker_test.go index 84c1cfdc08..6c5bc945c3 100644 --- a/internal/cache/walker_test.go +++ b/internal/cache/walker_test.go @@ -79,7 +79,7 @@ func TestDiskCacheInitialClear(t *testing.T) { canary := filepath.Join(cacheDir, "canary.txt") require.NoError(t, os.MkdirAll(filepath.Dir(canary), perm.PrivateDir)) - require.NoError(t, os.WriteFile(canary, []byte("chirp chirp"), perm.PublicFile)) + require.NoError(t, os.WriteFile(canary, []byte("chirp chirp"), perm.PrivateWriteOnceFile)) cache := New(cfg, locator, testhelper.SharedLogger(t), withDisabledWalker()) require.NoError(t, cache.StartWalkers()) diff --git a/internal/cgroups/handler_linux_test.go b/internal/cgroups/handler_linux_test.go index 550c32de67..ec3fef6991 100644 --- a/internal/cgroups/handler_linux_test.go +++ b/internal/cgroups/handler_linux_test.go @@ -892,7 +892,7 @@ func readCgroupFile(t *testing.T, path string) []byte { // The cgroups package defaults to permission 0 as it expects the file to be existing (the kernel creates the file) // and its testing override the permission private variable to something sensible, hence we have to chmod ourselves // so we can read the file. - require.NoError(t, os.Chmod(path, perm.PublicFile)) + require.NoError(t, os.Chmod(path, perm.PrivateWriteOnceFile)) return testhelper.MustReadFile(t, path) } diff --git a/internal/cli/praefect/main_test.go b/internal/cli/praefect/main_test.go index 42e764496d..f2df0227df 100644 --- a/internal/cli/praefect/main_test.go +++ b/internal/cli/praefect/main_test.go @@ -159,6 +159,6 @@ func writeConfigToFile(tb testing.TB, conf config.Config) string { require.NoError(tb, err) tmpDir := testhelper.TempDir(tb) confPath := filepath.Join(tmpDir, "config.toml") - require.NoError(tb, os.WriteFile(confPath, confData, perm.PublicFile)) + require.NoError(tb, os.WriteFile(confPath, confData, perm.PrivateWriteOnceFile)) return confPath } diff --git a/internal/errors/cfgerror/validate_test.go b/internal/errors/cfgerror/validate_test.go index 35c03cfd1d..9f0e88261c 100644 --- a/internal/errors/cfgerror/validate_test.go +++ b/internal/errors/cfgerror/validate_test.go @@ -122,7 +122,7 @@ func TestDirExists(t *testing.T) { t.Parallel() filePath := filepath.Join(testhelper.TempDir(t), "tmp-file") - require.NoError(t, os.WriteFile(filePath, []byte{}, perm.PublicFile)) + require.NoError(t, os.WriteFile(filePath, []byte{}, perm.PrivateWriteOnceFile)) existing := testhelper.TempDir(t) notExisting := filepath.Join(existing, "bad") @@ -140,7 +140,7 @@ func TestFileExists(t *testing.T) { dir := testhelper.TempDir(t) existing := filepath.Join(dir, "tmp-file") - require.NoError(t, os.WriteFile(existing, []byte{}, perm.PublicFile)) + require.NoError(t, os.WriteFile(existing, []byte{}, perm.PrivateWriteOnceFile)) notExisting := filepath.Join(dir, "bad") require.NoError(t, FileExists(existing)) diff --git a/internal/git/localrepo/blob_test.go b/internal/git/localrepo/blob_test.go index cb4586d9b2..8e19eaf4b2 100644 --- a/internal/git/localrepo/blob_test.go +++ b/internal/git/localrepo/blob_test.go @@ -1,7 +1,9 @@ package localrepo import ( + "errors" "io" + "io/fs" "os" "path/filepath" "strings" @@ -69,7 +71,12 @@ func TestRepo_WriteBlob(t *testing.T) { // We should get rid of this with https://gitlab.com/groups/gitlab-org/-/epics/9006 attributesPath := filepath.Join(repoPath, "info", "attributes") require.NoError(t, os.MkdirAll(filepath.Dir(attributesPath), perm.PrivateDir)) - require.NoError(t, os.WriteFile(attributesPath, []byte(tc.attributes), perm.PublicFile)) + + if err := os.Remove(attributesPath); !errors.Is(err, fs.ErrNotExist) { + require.NoError(t, err) + } + + require.NoError(t, os.WriteFile(attributesPath, []byte(tc.attributes), perm.PrivateWriteOnceFile)) sha, err := repo.WriteBlob(ctx, tc.input, WriteBlobConfig{ Path: "file-path", diff --git a/internal/git/quarantine/quarantine_test.go b/internal/git/quarantine/quarantine_test.go index 7b70a5951a..beb1f11243 100644 --- a/internal/git/quarantine/quarantine_test.go +++ b/internal/git/quarantine/quarantine_test.go @@ -33,7 +33,7 @@ func (e entry) create(t *testing.T, root string) { child.create(t, filepath.Join(root, name)) } } else { - require.NoError(t, os.WriteFile(root, []byte(e.contents), perm.PublicFile)) + require.NoError(t, os.WriteFile(root, []byte(e.contents), perm.PrivateWriteOnceFile)) } } @@ -123,7 +123,7 @@ func TestQuarantine_Migrate(t *testing.T) { quarantine, err := New(ctx, repo, logger, locator) require.NoError(t, err) - require.NoError(t, os.WriteFile(filepath.Join(quarantine.dir.Path(), "file"), []byte("foobar"), perm.PublicFile)) + require.NoError(t, os.WriteFile(filepath.Join(quarantine.dir.Path(), "file"), []byte("foobar"), perm.PrivateWriteOnceFile)) require.NoError(t, quarantine.Migrate(ctx)) newContents := listEntries(t, repoPath) @@ -154,7 +154,7 @@ func TestQuarantine_Migrate(t *testing.T) { recursiveQuarantine, err := New(ctx, quarantine.QuarantinedRepo(), logger, locator) require.NoError(t, err) - require.NoError(t, os.WriteFile(filepath.Join(recursiveQuarantine.dir.Path(), "file"), []byte("foobar"), perm.PublicFile)) + require.NoError(t, os.WriteFile(filepath.Join(recursiveQuarantine.dir.Path(), "file"), []byte("foobar"), perm.PrivateWriteOnceFile)) require.NoError(t, recursiveQuarantine.Migrate(ctx)) // The main repo should be untouched and still not contain the object. @@ -405,7 +405,7 @@ func TestFinalizeObjectFile(t *testing.T) { source := filepath.Join(dir, "a") target := filepath.Join(dir, "b") - require.NoError(t, os.WriteFile(source, []byte("a"), perm.PublicFile)) + require.NoError(t, os.WriteFile(source, []byte("a"), perm.PrivateWriteOnceFile)) require.NoError(t, finalizeObjectFile(source, target)) require.NoFileExists(t, source) @@ -418,7 +418,7 @@ func TestFinalizeObjectFile(t *testing.T) { source := filepath.Join(sourceDir, "a") target := filepath.Join(targetDir, "a") - require.NoError(t, os.WriteFile(source, []byte("a"), perm.PublicFile)) + require.NoError(t, os.WriteFile(source, []byte("a"), perm.PrivateWriteOnceFile)) require.NoError(t, finalizeObjectFile(source, target)) require.NoFileExists(t, source) @@ -429,10 +429,10 @@ func TestFinalizeObjectFile(t *testing.T) { dir := testhelper.TempDir(t) source := filepath.Join(dir, "a") - require.NoError(t, os.WriteFile(source, []byte("a"), perm.PublicFile)) + require.NoError(t, os.WriteFile(source, []byte("a"), perm.PrivateWriteOnceFile)) target := filepath.Join(dir, "b") - require.NoError(t, os.WriteFile(target, []byte("b"), perm.PublicFile)) + require.NoError(t, os.WriteFile(target, []byte("b"), perm.PrivateWriteOnceFile)) // We do not expect an error in case the target file exists: given that objects and // packs are content addressable, a file with the same name should have the same diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go index 61f845d78d..e9816a08f8 100644 --- a/internal/gitaly/config/config_test.go +++ b/internal/gitaly/config/config_test.go @@ -556,7 +556,7 @@ func TestValidateStorages(t *testing.T) { require.NoError(t, os.MkdirAll(nestedRepositories, perm.PrivateDir)) filePath := filepath.Join(testhelper.TempDir(t), "temporary-file") - require.NoError(t, os.WriteFile(filePath, []byte{}, perm.PublicFile)) + require.NoError(t, os.WriteFile(filePath, []byte{}, perm.PrivateWriteOnceFile)) invalidDir := filepath.Join(filepath.Dir(repositories), t.Name()) @@ -2356,7 +2356,7 @@ func TestHTTPSettings_Validate(t *testing.T) { tmpDir := testhelper.TempDir(t) tmpFile := filepath.Join(tmpDir, "tmpfile") - require.NoError(t, os.WriteFile(tmpFile, []byte{}, perm.PublicFile)) + require.NoError(t, os.WriteFile(tmpFile, []byte{}, perm.PrivateWriteOnceFile)) for _, tc := range []struct { name string @@ -2411,7 +2411,7 @@ func TestGitlab_Validate(t *testing.T) { tmpDir := testhelper.TempDir(t) tmpFile := filepath.Join(tmpDir, "tmpfile") - require.NoError(t, os.WriteFile(tmpFile, []byte{}, perm.PublicFile)) + require.NoError(t, os.WriteFile(tmpFile, []byte{}, perm.PrivateWriteOnceFile)) for _, tc := range []struct { name string diff --git a/internal/gitaly/config/temp_dir_test.go b/internal/gitaly/config/temp_dir_test.go index c30427d358..9b1c69e5c5 100644 --- a/internal/gitaly/config/temp_dir_test.go +++ b/internal/gitaly/config/temp_dir_test.go @@ -61,7 +61,7 @@ func TestPruneOldGitalyProcessDirectories(t *testing.T) { // Create an unexpected file in the runtime directory unexpectedFilePath := filepath.Join(baseDir, "unexpected-file") - require.NoError(t, os.WriteFile(unexpectedFilePath, []byte(""), perm.PublicFile)) + require.NoError(t, os.WriteFile(unexpectedFilePath, []byte(""), perm.PrivateWriteOnceFile)) expectedLogs[unexpectedFilePath] = "ignoring file found in gitaly process directory" nonPrunableDirs := []string{ownRuntimeDir} diff --git a/internal/gitaly/linguist/language_stats_test.go b/internal/gitaly/linguist/language_stats_test.go index 5ce129e6e3..0f003054b9 100644 --- a/internal/gitaly/linguist/language_stats_test.go +++ b/internal/gitaly/linguist/language_stats_test.go @@ -69,7 +69,7 @@ func TestInitLanguageStats(t *testing.T) { stats.Version = "faulty" // Copy save() behavior, but with a faulty version - file, err := os.OpenFile(filepath.Join(repoPath, languageStatsFilename), os.O_WRONLY|os.O_CREATE, perm.PublicFile) + file, err := os.OpenFile(filepath.Join(repoPath, languageStatsFilename), os.O_WRONLY|os.O_CREATE, perm.PrivateWriteOnceFile) require.NoError(t, err) w := zlib.NewWriter(file) require.NoError(t, json.NewEncoder(w).Encode(stats)) diff --git a/internal/gitaly/maintenance/randomwalker_test.go b/internal/gitaly/maintenance/randomwalker_test.go index 846e46087e..a9d9109cab 100644 --- a/internal/gitaly/maintenance/randomwalker_test.go +++ b/internal/gitaly/maintenance/randomwalker_test.go @@ -155,7 +155,7 @@ func TestRandomWalk(t *testing.T) { } for _, file := range tc.files { - require.NoError(t, os.WriteFile(filepath.Join(root, file), []byte{}, perm.PublicFile)) + require.NoError(t, os.WriteFile(filepath.Join(root, file), []byte{}, perm.PrivateWriteOnceFile)) } walker := newRandomWalker(root, rand.New(rand.NewSource(1))) diff --git a/internal/gitaly/repoutil/create_test.go b/internal/gitaly/repoutil/create_test.go index 75f7633521..0d34d5de64 100644 --- a/internal/gitaly/repoutil/create_test.go +++ b/internal/gitaly/repoutil/create_test.go @@ -304,7 +304,7 @@ func TestCreate(t *testing.T) { seed: func(t *testing.T, repo *gitalypb.Repository, repoPath string) error { // Objects should both be ignored. They may contain indeterministic data // that's different across replicas and would thus cause us to not reach quorum. - require.NoError(t, os.WriteFile(filepath.Join(repoPath, "objects", "object"), []byte("object"), perm.PublicFile)) + require.NoError(t, os.WriteFile(filepath.Join(repoPath, "objects", "object"), []byte("object"), perm.PrivateWriteOnceFile)) gittest.BackendSpecificRepoHash(t, ctx, cfg, hash, repoPath) diff --git a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go index 006b0af8c2..687bc6c27a 100644 --- a/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go +++ b/internal/gitaly/service/objectpool/fetch_into_object_pool_test.go @@ -75,7 +75,7 @@ func TestFetchIntoObjectPool_Success(t *testing.T) { // references though and thus be able to recover. brokenRef := filepath.Join(poolPath, "refs", "heads", "broken") require.NoError(t, os.MkdirAll(filepath.Dir(brokenRef), perm.PrivateDir)) - require.NoError(t, os.WriteFile(brokenRef, []byte{}, perm.PublicFile)) + require.NoError(t, os.WriteFile(brokenRef, []byte{}, perm.PrivateWriteOnceFile)) oldTime := time.Now().Add(-25 * time.Hour) require.NoError(t, os.Chtimes(brokenRef, oldTime, oldTime)) diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go index fd6fe39e87..a5f6464d8c 100644 --- a/internal/gitaly/service/repository/replicate_test.go +++ b/internal/gitaly/service/repository/replicate_test.go @@ -344,7 +344,7 @@ func TestReplicateRepository(t *testing.T) { // Remove the HEAD first as the files are read-only with transactions. headPath := filepath.Join(sourcePath, "HEAD") require.NoError(t, os.Remove(headPath)) - require.NoError(t, os.WriteFile(headPath, []byte("garbage"), perm.PublicFile)) + require.NoError(t, os.WriteFile(headPath, []byte("garbage"), perm.PrivateWriteOnceFile)) return setupData{ source: source, diff --git a/internal/gitaly/transaction/voting_test.go b/internal/gitaly/transaction/voting_test.go index 0116a6b237..9ee9613631 100644 --- a/internal/gitaly/transaction/voting_test.go +++ b/internal/gitaly/transaction/voting_test.go @@ -208,7 +208,7 @@ func TestCommitLockedFile(t *testing.T) { VoteFn: func(context.Context, txinfo.Transaction, voting.Vote, voting.Phase) error { // This shouldn't typically happen given that the file is locked, // but we concurrently update the file after our first vote. - require.NoError(t, os.WriteFile(file, []byte("something"), perm.PublicFile)) + require.NoError(t, os.WriteFile(file, []byte("something"), perm.PrivateWriteOnceFile)) return nil }, }, writer) diff --git a/internal/helper/perm/perm.go b/internal/helper/perm/perm.go index 274e6e26e3..1fbd979dd5 100644 --- a/internal/helper/perm/perm.go +++ b/internal/helper/perm/perm.go @@ -26,10 +26,6 @@ const ( // be read outside of Gitaly. SharedReadOnlyFile fs.FileMode = 0o444 - // PublicFile is the permission given for a file that may be read or - // written outside of gitaly. - PublicFile fs.FileMode = 0o666 - // PrivateExecutable is the permissions given for an executable that must // only be used by gitaly. PrivateExecutable fs.FileMode = 0o700 diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go index 06d4d98c07..e24c093c26 100644 --- a/internal/praefect/config/config_test.go +++ b/internal/praefect/config/config_test.go @@ -930,7 +930,7 @@ func TestConfig_ValidateV2(t *testing.T) { t.Run("invalid", func(t *testing.T) { tmpDir := testhelper.TempDir(t) tmpFile := filepath.Join(tmpDir, "file") - require.NoError(t, os.WriteFile(tmpFile, nil, perm.PublicFile)) + require.NoError(t, os.WriteFile(tmpFile, nil, perm.PrivateWriteOnceFile)) cfg := Config{ BackgroundVerification: BackgroundVerification{ VerificationInterval: duration.Duration(-1), diff --git a/internal/testhelper/testcfg/gitaly.go b/internal/testhelper/testcfg/gitaly.go index 61cd68a609..73fca75d41 100644 --- a/internal/testhelper/testcfg/gitaly.go +++ b/internal/testhelper/testcfg/gitaly.go @@ -117,7 +117,7 @@ func (gc *GitalyCfgBuilder) Build(tb testing.TB) config.Cfg { if cfg.Gitlab.SecretFile == "" { cfg.Gitlab.SecretFile = filepath.Join(root, "gitlab", "http.secret") require.NoError(tb, os.MkdirAll(filepath.Dir(cfg.Gitlab.SecretFile), perm.PrivateDir)) - require.NoError(tb, os.WriteFile(cfg.Gitlab.SecretFile, nil, perm.PublicFile)) + require.NoError(tb, os.WriteFile(cfg.Gitlab.SecretFile, nil, perm.PrivateWriteOnceFile)) } // cfg.SetDefaults() should only be called after cfg.Gitlab.SecretFile is set (so it doesn't override it with diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index 2ce8f21ecd..5822d40971 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "io/fs" "math/rand" "net" "net/http" @@ -120,9 +121,9 @@ func WriteFiles(tb testing.TB, root string, files map[string]any) { switch content := value.(type) { case string: - require.NoError(tb, os.WriteFile(path, []byte(content), perm.PublicFile)) + require.NoError(tb, os.WriteFile(path, []byte(content), fs.ModePerm)) case []byte: - require.NoError(tb, os.WriteFile(path, content, perm.PublicFile)) + require.NoError(tb, os.WriteFile(path, content, fs.ModePerm)) case io.Reader: func() { f, err := os.Create(path) diff --git a/packed_binaries_test.go b/packed_binaries_test.go index 5279011995..8f148203dd 100644 --- a/packed_binaries_test.go +++ b/packed_binaries_test.go @@ -38,7 +38,7 @@ func TestUnpackAuxiliaryBinaries_alreadyExists(t *testing.T) { destinationDir := t.TempDir() existingFile := filepath.Join(destinationDir, "gitaly-hooks") - require.NoError(t, os.WriteFile(existingFile, []byte("existing file"), perm.PublicFile)) + require.NoError(t, os.WriteFile(existingFile, []byte("existing file"), perm.PrivateWriteOnceFile)) err := UnpackAuxiliaryBinaries(destinationDir) require.EqualError(t, err, fmt.Sprintf(`open %s: file exists`, existingFile), "expected unpacking to fail if destination binary already existed") -- GitLab From e48bb97758ef9c194a4e3d518017e5cb48877464 Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Mon, 8 Jul 2024 15:19:24 +0300 Subject: [PATCH 10/14] Remove SharedReadOnlyFile permission The permission is unused. Remove it. --- internal/helper/perm/perm.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/helper/perm/perm.go b/internal/helper/perm/perm.go index 1fbd979dd5..dc32101e55 100644 --- a/internal/helper/perm/perm.go +++ b/internal/helper/perm/perm.go @@ -22,10 +22,6 @@ const ( // of gitaly. SharedFile fs.FileMode = 0o644 - // SharedReadOnlyFile is the permission given for a read only file that may also - // be read outside of Gitaly. - SharedReadOnlyFile fs.FileMode = 0o444 - // PrivateExecutable is the permissions given for an executable that must // only be used by gitaly. PrivateExecutable fs.FileMode = 0o700 -- GitLab From 42a076b7ba4a3dc6eee2953867a85508e38fd255 Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Mon, 8 Jul 2024 15:26:17 +0300 Subject: [PATCH 11/14] Use original binary's mode for the new binary `replace_buildid` replaces the temporary build ID of a Gitaly binary with the final one. It currently uses the SharedExecutable permissions for the new binary. Use the original permissions instead. After all, we want to just replace the build ID in the binary and our intention is not to change the permissions. --- tools/replace-buildid/main.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tools/replace-buildid/main.go b/tools/replace-buildid/main.go index e5e9e99785..d9544cb08d 100644 --- a/tools/replace-buildid/main.go +++ b/tools/replace-buildid/main.go @@ -16,8 +16,6 @@ import ( "io" "os" "path/filepath" - - "gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm" ) func main() { @@ -71,7 +69,7 @@ func replaceBuildID(inputPath, outputPath, inputBuildID, outputBuildID string) e return fmt.Errorf("could not replace build ID: %w", err) } - if err := writeBinary(outputPath, data); err != nil { + if err := writeBinary(inputPath, outputPath, data); err != nil { return fmt.Errorf("writing binary: %w", err) } @@ -97,7 +95,7 @@ func readAndReplace(binaryPath string, inputBuildID, outputBuildID []byte) ([]by return bytes.ReplaceAll(data, inputBuildID, outputBuildID), nil } -func writeBinary(binaryPath string, contents []byte) error { +func writeBinary(inputPath string, binaryPath string, contents []byte) error { f, err := os.CreateTemp(filepath.Dir(binaryPath), filepath.Base(binaryPath)) if err != nil { return fmt.Errorf("could not create binary: %w", err) @@ -107,14 +105,19 @@ func writeBinary(binaryPath string, contents []byte) error { f.Close() }() - if err := f.Chmod(perm.SharedExecutable); err != nil { - return fmt.Errorf("could not change permissions: %w", err) - } - if _, err := io.Copy(f, bytes.NewReader(contents)); err != nil { return fmt.Errorf("could not write binary: %w", err) } + inputStat, err := os.Stat(inputPath) + if err != nil { + return fmt.Errorf("stat input: %w", err) + } + + if err := f.Chmod(inputStat.Mode()); err != nil { + return fmt.Errorf("could not change permissions: %w", err) + } + if err := f.Close(); err != nil { return fmt.Errorf("could not close binary: %w", err) } -- GitLab From f222ad49c1396ce956a1a09867eef9fbe980fce4 Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Mon, 8 Jul 2024 15:30:09 +0300 Subject: [PATCH 12/14] Remove SharedExecutable permission SharedExecutable permission is only used in tests. Remove the permission ande replaace its usage with PrivateExecutable. --- internal/cli/gitaly/subcmd_hooks_test.go | 2 +- internal/git/execution_environment_test.go | 4 ++-- internal/gitaly/hook/custom_test.go | 2 +- internal/gitaly/repoutil/custom_hooks_test.go | 14 +++++++------- .../gitaly/service/repository/replicate_test.go | 2 +- internal/gitaly/service/ssh/receive_pack_test.go | 2 +- internal/helper/perm/perm.go | 4 ---- internal/testhelper/directory.go | 6 +++--- internal/testhelper/testhelper.go | 2 +- 9 files changed, 17 insertions(+), 21 deletions(-) diff --git a/internal/cli/gitaly/subcmd_hooks_test.go b/internal/cli/gitaly/subcmd_hooks_test.go index 37e3187b48..be4a2010d5 100644 --- a/internal/cli/gitaly/subcmd_hooks_test.go +++ b/internal/cli/gitaly/subcmd_hooks_test.go @@ -55,7 +55,7 @@ func TestSetHooksSubcommand(t *testing.T) { expectedExecutableMode := testhelper.WithOrWithoutWAL( storage.ModeExecutable, - umask.Mask(perm.SharedExecutable), + umask.Mask(perm.PrivateExecutable), ) for _, tc := range []struct { diff --git a/internal/git/execution_environment_test.go b/internal/git/execution_environment_test.go index 0be3c0df0c..6b9b077c68 100644 --- a/internal/git/execution_environment_test.go +++ b/internal/git/execution_environment_test.go @@ -74,7 +74,7 @@ func TestBundledGitEnvironmentConstructor(t *testing.T) { seedDirWithExecutables := func(t *testing.T, executableNames ...string) string { dir := testhelper.TempDir(t) for _, executableName := range executableNames { - require.NoError(t, os.WriteFile(filepath.Join(dir, executableName), nil, perm.SharedExecutable)) + require.NoError(t, os.WriteFile(filepath.Join(dir, executableName), nil, perm.PrivateExecutable)) } return dir } @@ -242,7 +242,7 @@ func TestFallbackGitEnvironmentConstructor(t *testing.T) { t.Run("successfully resolved executable", func(t *testing.T) { tempDir := testhelper.TempDir(t) gitPath := filepath.Join(tempDir, "git") - require.NoError(t, os.WriteFile(gitPath, nil, perm.SharedExecutable)) + require.NoError(t, os.WriteFile(gitPath, nil, perm.PrivateExecutable)) t.Setenv("PATH", "/does/not/exist:"+tempDir) diff --git a/internal/gitaly/hook/custom_test.go b/internal/gitaly/hook/custom_test.go index 9e8bff0ed6..f6991a1938 100644 --- a/internal/gitaly/hook/custom_test.go +++ b/internal/gitaly/hook/custom_test.go @@ -451,7 +451,7 @@ type customHookResults struct { func writeCustomHook(t *testing.T, hookName, dir string, content []byte) func() { require.NoError(t, os.MkdirAll(dir, perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(dir, hookName), content, perm.SharedExecutable)) + require.NoError(t, os.WriteFile(filepath.Join(dir, hookName), content, perm.PrivateExecutable)) return func() { require.NoError(t, os.RemoveAll(dir)) diff --git a/internal/gitaly/repoutil/custom_hooks_test.go b/internal/gitaly/repoutil/custom_hooks_test.go index 22ce7d018d..24d6a7b3ce 100644 --- a/internal/gitaly/repoutil/custom_hooks_test.go +++ b/internal/gitaly/repoutil/custom_hooks_test.go @@ -324,24 +324,24 @@ func TestNewDirectoryVote(t *testing.T) { { desc: "generated hash matches", files: []testFile{ - {name: "pre-commit.sample", content: "foo", mode: perm.SharedExecutable}, - {name: "pre-push.sample", content: "bar", mode: perm.SharedExecutable}, + {name: "pre-commit.sample", content: "foo", mode: perm.PrivateExecutable}, + {name: "pre-push.sample", content: "bar", mode: perm.PrivateExecutable}, }, expectedHash: "8ca11991268de4c9278488a674fc1a88db449566", }, { desc: "generated hash matches with changed file name", files: []testFile{ - {name: "pre-commit.sample.diff", content: "foo", mode: perm.SharedExecutable}, - {name: "pre-push.sample", content: "bar", mode: perm.SharedExecutable}, + {name: "pre-commit.sample.diff", content: "foo", mode: perm.PrivateExecutable}, + {name: "pre-push.sample", content: "bar", mode: perm.PrivateExecutable}, }, expectedHash: "b5ed58ced84103da1ed9d7813a9e39b3b5daf7d7", }, { desc: "generated hash matches with changed file content", files: []testFile{ - {name: "pre-commit.sample", content: "foo", mode: perm.SharedExecutable}, - {name: "pre-push.sample", content: "bar.diff", mode: perm.SharedExecutable}, + {name: "pre-commit.sample", content: "foo", mode: perm.PrivateExecutable}, + {name: "pre-push.sample", content: "bar.diff", mode: perm.PrivateExecutable}, }, expectedHash: "178083848c8a08e36c4f86c2d318a84b0bb845f2", }, @@ -349,7 +349,7 @@ func TestNewDirectoryVote(t *testing.T) { desc: "generated hash matches with changed file mode", files: []testFile{ {name: "pre-commit.sample", content: "foo", mode: perm.SharedFile}, - {name: "pre-push.sample", content: "bar", mode: perm.SharedExecutable}, + {name: "pre-push.sample", content: "bar", mode: perm.PrivateExecutable}, }, expectedHash: "c69574241b83496bb4005b4f7a0dfcda96cb317e", }, diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go index a5f6464d8c..f67ab3d5e7 100644 --- a/internal/gitaly/service/repository/replicate_test.go +++ b/internal/gitaly/service/repository/replicate_test.go @@ -713,7 +713,7 @@ func listenGitalySSHCalls(t *testing.T, conf config.Cfg) func() gitalySSHParams echo "$@" >%[1]q/arguments exec %[2]q "$@"`, tmpDir, updatedPath) - require.NoError(t, os.WriteFile(initialPath, []byte(script), perm.SharedExecutable)) + require.NoError(t, os.WriteFile(initialPath, []byte(script), perm.PrivateExecutable)) return func() gitalySSHParams { arguments := testhelper.MustReadFile(t, filepath.Join(tmpDir, "arguments")) diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go index 845303281b..604613c772 100644 --- a/internal/gitaly/service/ssh/receive_pack_test.go +++ b/internal/gitaly/service/ssh/receive_pack_test.go @@ -437,7 +437,7 @@ func TestReceivePack_hookFailure(t *testing.T) { remoteRepo, _ := gittest.CreateRepository(t, ctx, cfg) hookContent := []byte("#!/bin/sh\nexit 1") - require.NoError(t, os.WriteFile(filepath.Join(gitCmdFactory.HooksPath(ctx), "pre-receive"), hookContent, perm.SharedExecutable)) + require.NoError(t, os.WriteFile(filepath.Join(gitCmdFactory.HooksPath(ctx), "pre-receive"), hookContent, perm.PrivateExecutable)) _, _, err := setupRepoAndPush(t, ctx, cfg, &gitalypb.SSHReceivePackRequest{ Repository: remoteRepo, diff --git a/internal/helper/perm/perm.go b/internal/helper/perm/perm.go index dc32101e55..4a93e1583f 100644 --- a/internal/helper/perm/perm.go +++ b/internal/helper/perm/perm.go @@ -25,10 +25,6 @@ const ( // PrivateExecutable is the permissions given for an executable that must // only be used by gitaly. PrivateExecutable fs.FileMode = 0o700 - - // SharedExecutable is the permission given for an executable that may be - // executed outside of gitaly. - SharedExecutable fs.FileMode = 0o755 ) // Umask represents a umask that is used to mask mode bits. diff --git a/internal/testhelper/directory.go b/internal/testhelper/directory.go index 5964d20ded..57cd000881 100644 --- a/internal/testhelper/directory.go +++ b/internal/testhelper/directory.go @@ -148,9 +148,9 @@ func MustCreateCustomHooksTar(tb testing.TB) io.Reader { defer MustClose(tb, writer) require.NoError(tb, writer.WriteHeader(&tar.Header{Name: "custom_hooks/", Mode: int64(perm.PrivateDir)})) - writeFile(writer, "custom_hooks/pre-commit", perm.SharedExecutable, "pre-commit content") - writeFile(writer, "custom_hooks/pre-push", perm.SharedExecutable, "pre-push content") - writeFile(writer, "custom_hooks/pre-receive", perm.SharedExecutable, "pre-receive content") + writeFile(writer, "custom_hooks/pre-commit", perm.PrivateExecutable, "pre-commit content") + writeFile(writer, "custom_hooks/pre-push", perm.PrivateExecutable, "pre-push content") + writeFile(writer, "custom_hooks/pre-receive", perm.PrivateExecutable, "pre-receive content") return &buffer } diff --git a/internal/testhelper/testhelper.go b/internal/testhelper/testhelper.go index 5822d40971..e2972543aa 100644 --- a/internal/testhelper/testhelper.go +++ b/internal/testhelper/testhelper.go @@ -354,7 +354,7 @@ func WriteExecutable(tb testing.TB, path string, content []byte) string { // // We thus need to perform file locking to ensure that all writeable references to this // file have been closed before returning. - executable, err := os.OpenFile(path, os.O_CREATE|os.O_EXCL|os.O_WRONLY, perm.SharedExecutable) + executable, err := os.OpenFile(path, os.O_CREATE|os.O_EXCL|os.O_WRONLY, perm.PrivateExecutable) require.NoError(tb, err) _, err = io.Copy(executable, bytes.NewReader(content)) require.NoError(tb, err) -- GitLab From 1b4bba6f93639df283d400a4fb136b4f612611e0 Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Thu, 11 Jul 2024 13:00:20 +0300 Subject: [PATCH 13/14] Remove production usage of SharedFile Gitaly is using SharedFile in many places where PrivateWriteOnceFile would suffice. In production code, we're using it in a few locations: - streamcache for the cached file. They're never written to after creation, so no need to grant Gitaly the write bit. This is an internal cache, so no need to grant others permission to read the files. - ServerInfo RPC uses it to write out a file test whether Gitaly has permissions to write into the storage and removes it afterwards. No need for anyone to read the file nor for Gitaly to write into it again. - ReplicateRepository RPC uses it for git config. The config file should not be written into after creating it as this would break snapshot isolation with transactions. A new file should be created and the old file replaced with it. Gitaly's storage should not be read by others than Gitaly. The repositories are not guaranteed to be in consistent state if there is a WAL entry application in progress. Replace all the usage of SharedFile in production code with PrivateWriteOnceFile. --- internal/gitaly/service/repository/replicate.go | 2 +- internal/gitaly/service/server/info.go | 5 +++-- internal/streamcache/filestore.go | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/internal/gitaly/service/repository/replicate.go b/internal/gitaly/service/repository/replicate.go index 31e3f65ace..ce4e00662b 100644 --- a/internal/gitaly/service/repository/replicate.go +++ b/internal/gitaly/service/repository/replicate.go @@ -352,7 +352,7 @@ func (s *server) syncGitconfig(ctx context.Context, source, target *gitalypb.Rep } configPath := filepath.Join(repoPath, "config") - if err := s.writeFile(ctx, configPath, perm.SharedFile, streamio.NewReader(func() ([]byte, error) { + if err := s.writeFile(ctx, configPath, perm.PrivateWriteOnceFile, streamio.NewReader(func() ([]byte, error) { resp, err := stream.Recv() return resp.GetData(), err })); err != nil { diff --git a/internal/gitaly/service/server/info.go b/internal/gitaly/service/server/info.go index 8be3a77254..1253cbfdae 100644 --- a/internal/gitaly/service/server/info.go +++ b/internal/gitaly/service/server/info.go @@ -7,7 +7,6 @@ import ( "gitlab.com/gitlab-org/gitaly/v16/internal/gitaly/storage" "gitlab.com/gitlab-org/gitaly/v16/internal/helper/fstype" - "gitlab.com/gitlab-org/gitaly/v16/internal/helper/perm" "gitlab.com/gitlab-org/gitaly/v16/internal/structerr" "gitlab.com/gitlab-org/gitaly/v16/internal/version" "gitlab.com/gitlab-org/gitaly/v16/proto/go/gitalypb" @@ -54,8 +53,10 @@ func shardCheck(shardPath string) (readable bool, writeable bool) { // the path uses a `+` to avoid naming collisions testPath := filepath.Join(shardPath, "+testWrite") + _ = os.Remove(testPath) + content := []byte("testWrite") - if err := os.WriteFile(testPath, content, perm.SharedFile); err == nil { + if err := os.WriteFile(testPath, content, storage.ModeFile.Perm()); err == nil { writeable = true } _ = os.Remove(testPath) diff --git a/internal/streamcache/filestore.go b/internal/streamcache/filestore.go index 4d01e377e6..8ce3fb62b8 100644 --- a/internal/streamcache/filestore.go +++ b/internal/streamcache/filestore.go @@ -110,7 +110,7 @@ func (fs *filestore) Create() (namedWriteCloser, error) { return nil, fmt.Errorf("Create: mkdir: %w", err) } - f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm.SharedFile) + f, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm.PrivateWriteOnceFile) if err != nil { return nil, fmt.Errorf("Create: %w", err) } -- GitLab From d33225915a1259c042a1e442fb9a43c06ce2236c Mon Sep 17 00:00:00 2001 From: Sami Hiltunen Date: Thu, 11 Jul 2024 13:03:20 +0300 Subject: [PATCH 14/14] Remove SharedFile permission SharedFile permission is only used in tests. Remove the permission type and replace it with the general permission set to use in tests. --- cmd/gitaly-wrapper/main_test.go | 18 ++++---- internal/backup/locator_test.go | 22 +++++----- internal/backup/sink_test.go | 2 +- internal/bundleuri/sink_test.go | 2 +- internal/cache/walker_test.go | 2 +- internal/cgroups/mock_linux_test.go | 11 ++--- internal/git/conflict/parser_test.go | 2 +- internal/git/gittest/http_server.go | 2 +- .../git/housekeeping/clean_stale_data_test.go | 4 +- .../manager/optimize_repository_test.go | 14 +++---- internal/git/localrepo/refs_external_test.go | 2 +- internal/git/localrepo/snapshot_test.go | 10 ++--- internal/git/objectpool/disconnect_test.go | 8 ++-- internal/git/objectpool/link_test.go | 8 ++-- internal/git/objectpool/pool_test.go | 2 +- internal/git/remoterepo/repository_test.go | 2 +- internal/git/stats/repository_info_test.go | 42 +++++++++---------- internal/gitaly/config/config_test.go | 8 ++-- .../gitaly/linguist/language_stats_test.go | 2 +- internal/gitaly/linguist/linguist_test.go | 2 +- internal/gitaly/repoutil/custom_hooks_test.go | 10 ++--- internal/gitaly/repoutil/remove_test.go | 2 +- .../service/objectpool/alternates_test.go | 2 +- .../gitaly/service/objectpool/create_test.go | 2 +- .../gitaly/service/objectpool/get_test.go | 2 +- .../gitaly/service/objectpool/link_test.go | 2 +- .../service/repository/create_fork_test.go | 4 +- .../create_repository_from_url_test.go | 2 +- .../service/repository/fetch_remote_test.go | 2 +- .../gitaly/service/repository/fsck_test.go | 2 +- .../repository/info_attributes_test.go | 2 +- .../service/repository/object_format_test.go | 2 +- .../gitaly/service/repository/remove_test.go | 2 +- .../service/repository/replicate_test.go | 2 +- .../service/repository/snapshot_test.go | 6 +-- .../gitaly/service/smarthttp/inforefs_test.go | 2 +- .../gitaly/service/ssh/receive_pack_test.go | 2 +- .../gitaly/service/ssh/upload_pack_test.go | 2 +- .../storagemgr/apply_operations_test.go | 2 +- internal/gitaly/storage/wal/entry_test.go | 12 +++--- internal/gitlab/test_server.go | 2 +- internal/safe/locking_directory_test.go | 2 +- internal/safe/locking_file_writer_test.go | 21 +++++----- internal/streamcache/cache_test.go | 6 +-- internal/streamcache/filestore_test.go | 2 +- internal/tempdir/clean_test.go | 2 +- internal/tempdir/tempdir_test.go | 2 +- internal/testhelper/testcfg/gitaly.go | 2 +- 48 files changed, 135 insertions(+), 133 deletions(-) diff --git a/cmd/gitaly-wrapper/main_test.go b/cmd/gitaly-wrapper/main_test.go index ff113e0da7..dab9dd91e5 100644 --- a/cmd/gitaly-wrapper/main_test.go +++ b/cmd/gitaly-wrapper/main_test.go @@ -68,7 +68,7 @@ func TestFindProcess(t *testing.T) { t.Parallel() path := filepath.Join(testhelper.TempDir(t), "pid") - require.NoError(t, os.WriteFile(path, []byte("garbage"), perm.SharedFile)) + require.NoError(t, os.WriteFile(path, []byte("garbage"), perm.PrivateWriteOnceFile)) _, err := findProcess(path) _, expectedErr := strconv.Atoi("garbage") @@ -81,7 +81,7 @@ func TestFindProcess(t *testing.T) { // The below PID can exist, but chances are sufficiently low to hopefully not matter // in practice. path := filepath.Join(testhelper.TempDir(t), "pid") - require.NoError(t, os.WriteFile(path, []byte("7777777"), perm.SharedFile)) + require.NoError(t, os.WriteFile(path, []byte("7777777"), perm.PrivateWriteOnceFile)) // The process isn't alive, so we expect neither an error nor a process to be // returned. @@ -116,7 +116,7 @@ func TestFindProcess(t *testing.T) { require.NoError(t, err) path := filepath.Join(testhelper.TempDir(t), "pid") - require.NoError(t, os.WriteFile(path, []byte(strconv.FormatInt(int64(cmd.Process.Pid), 10)), perm.SharedFile)) + require.NoError(t, os.WriteFile(path, []byte(strconv.FormatInt(int64(cmd.Process.Pid), 10)), perm.PrivateWriteOnceFile)) process, err := findProcess(path) require.NotNil(t, process) @@ -174,7 +174,7 @@ func TestReadPIDFile(t *testing.T) { t.Parallel() path := filepath.Join(testhelper.TempDir(t), "pid") - require.NoError(t, os.WriteFile(path, nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(path, nil, perm.PrivateWriteOnceFile)) _, err := readPIDFile(path) _, expectedErr := strconv.Atoi("") require.Equal(t, expectedErr, err) @@ -184,7 +184,7 @@ func TestReadPIDFile(t *testing.T) { t.Parallel() path := filepath.Join(testhelper.TempDir(t), "pid") - require.NoError(t, os.WriteFile(path, []byte("invalid"), perm.SharedFile)) + require.NoError(t, os.WriteFile(path, []byte("invalid"), perm.PrivateWriteOnceFile)) _, err := readPIDFile(path) _, expectedErr := strconv.Atoi("invalid") require.Equal(t, expectedErr, err) @@ -194,7 +194,7 @@ func TestReadPIDFile(t *testing.T) { t.Parallel() path := filepath.Join(testhelper.TempDir(t), "pid") - require.NoError(t, os.WriteFile(path, []byte("12345"), perm.SharedFile)) + require.NoError(t, os.WriteFile(path, []byte("12345"), perm.PrivateWriteOnceFile)) pid, err := readPIDFile(path) require.NoError(t, err) require.Equal(t, 12345, pid) @@ -347,7 +347,7 @@ func TestRun(t *testing.T) { // Write the PID of the running process into the PID file. As a result, it should // get adopted by gitaly-wrapper, which means it wouldn't try to execute it anew. pidPath := filepath.Join(testhelper.TempDir(t), "pid") - require.NoError(t, os.WriteFile(pidPath, []byte(strconv.FormatInt(int64(scriptCmd.Process.Pid), 10)), perm.SharedFile)) + require.NoError(t, os.WriteFile(pidPath, []byte(strconv.FormatInt(int64(scriptCmd.Process.Pid), 10)), perm.PrivateWriteOnceFile)) // Run gitaly-script with a binary path whose basename matches, but which ultimately // doesn't exist. This proves that it doesn't try to execute the script again. @@ -411,7 +411,7 @@ func TestRun(t *testing.T) { `)) pidPath := filepath.Join(testhelper.TempDir(t), "pid") - require.NoError(t, os.WriteFile(pidPath, []byte("12345"), perm.SharedFile)) + require.NoError(t, os.WriteFile(pidPath, []byte("12345"), perm.PrivateWriteOnceFile)) cmd := exec.CommandContext(ctx, binary, script) cmd.Env = append(os.Environ(), fmt.Sprintf("%s=%s", bootstrap.EnvPidFile, pidPath)) @@ -442,7 +442,7 @@ func TestRun(t *testing.T) { require.NoError(t, err) pidPath := filepath.Join(testhelper.TempDir(t), "pid") - require.NoError(t, os.WriteFile(pidPath, []byte(strconv.FormatInt(int64(scriptCmd.Process.Pid), 10)), perm.SharedFile)) + require.NoError(t, os.WriteFile(pidPath, []byte(strconv.FormatInt(int64(scriptCmd.Process.Pid), 10)), perm.PrivateWriteOnceFile)) cmd := exec.CommandContext(ctx, binary, script) cmd.Env = append(os.Environ(), fmt.Sprintf("%s=%s", bootstrap.EnvPidFile, pidPath)) diff --git a/internal/backup/locator_test.go b/internal/backup/locator_test.go index 711bae2197..05d625aaf6 100644 --- a/internal/backup/locator_test.go +++ b/internal/backup/locator_test.go @@ -148,8 +148,8 @@ func TestPointerLocator(t *testing.T) { expectedOffset: 1, setup: func(tb testing.TB, ctx context.Context, backupPath string) { require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, "abc123"), perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte("abc123"), perm.SharedFile)) - require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "abc123", "LATEST"), []byte("001"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte("abc123"), perm.PrivateWriteOnceFile)) + require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "abc123", "LATEST"), []byte("001"), perm.PrivateWriteOnceFile)) }, }, } { @@ -220,8 +220,8 @@ func TestPointerLocator(t *testing.T) { require.ErrorIs(t, err, ErrDoesntExist) require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte(backupID), perm.SharedFile)) - require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("003"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte(backupID), perm.PrivateWriteOnceFile)) + require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("003"), perm.PrivateWriteOnceFile)) expected := &Backup{ ID: backupID, Repository: repo, @@ -281,8 +281,8 @@ func TestPointerLocator(t *testing.T) { require.Equal(t, expectedFallback, fallbackFull) require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte(backupID), perm.SharedFile)) - require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("001"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte(backupID), perm.PrivateWriteOnceFile)) + require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("001"), perm.PrivateWriteOnceFile)) expected := &Backup{ ID: backupID, Repository: repo, @@ -315,7 +315,7 @@ func TestPointerLocator(t *testing.T) { require.ErrorIs(t, err, ErrDoesntExist) require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath), perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte("invalid"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte("invalid"), perm.PrivateWriteOnceFile)) _, err = l.FindLatest(ctx, repo) require.EqualError(t, err, "pointer locator: find latest: find: find latest ID: storage service sink: new reader for \"TestPointerLocator/invalid/LATEST\": doesn't exist") }) @@ -334,8 +334,8 @@ func TestPointerLocator(t *testing.T) { require.ErrorIs(t, err, ErrDoesntExist) require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte(backupID), perm.SharedFile)) - require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("invalid"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, "LATEST"), []byte(backupID), perm.PrivateWriteOnceFile)) + require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("invalid"), perm.PrivateWriteOnceFile)) _, err = l.FindLatest(ctx, repo) require.EqualError(t, err, "pointer locator: find latest: find: determine increment ID: strconv.Atoi: parsing \"invalid\": invalid syntax") @@ -370,7 +370,7 @@ func TestPointerLocator(t *testing.T) { } require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("003"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("003"), perm.PrivateWriteOnceFile)) expected := &Backup{ ID: backupID, Repository: repo, @@ -415,7 +415,7 @@ func TestPointerLocator(t *testing.T) { require.ErrorIs(t, err, ErrDoesntExist) require.NoError(t, os.MkdirAll(filepath.Join(backupPath, repo.RelativePath, backupID), perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("invalid"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(backupPath, repo.RelativePath, backupID, "LATEST"), []byte("invalid"), perm.PrivateWriteOnceFile)) _, err = l.Find(ctx, repo, backupID) require.EqualError(t, err, "pointer locator: find: determine increment ID: strconv.Atoi: parsing \"invalid\": invalid syntax") diff --git a/internal/backup/sink_test.go b/internal/backup/sink_test.go index 1fbef67942..1e88ef8c24 100644 --- a/internal/backup/sink_test.go +++ b/internal/backup/sink_test.go @@ -53,7 +53,7 @@ func TestResolveSink(t *testing.T) { "token_uri": "https://accounts.google.com/o/oauth2/token", "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs", "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/303724477529-compute%40developer.gserviceaccount.com" -}`), perm.SharedFile)) +}`), perm.PrivateWriteOnceFile)) for _, tc := range []struct { desc string diff --git a/internal/bundleuri/sink_test.go b/internal/bundleuri/sink_test.go index 01aa6907af..322825610b 100644 --- a/internal/bundleuri/sink_test.go +++ b/internal/bundleuri/sink_test.go @@ -99,7 +99,7 @@ func TestSink_SignedURL(t *testing.T) { setup: func(t *testing.T, sinkDir string, sink *Sink) { path := filepath.Join(sinkDir, sink.relativePath(repo, "default")) require.NoError(t, os.MkdirAll(filepath.Dir(path), perm.PrivateDir)) - require.NoError(t, os.WriteFile(path, []byte("hello"), perm.SharedFile)) + require.NoError(t, os.WriteFile(path, []byte("hello"), perm.PrivateWriteOnceFile)) }, }, { diff --git a/internal/cache/walker_test.go b/internal/cache/walker_test.go index 6c5bc945c3..3cad56dec7 100644 --- a/internal/cache/walker_test.go +++ b/internal/cache/walker_test.go @@ -116,7 +116,7 @@ func TestCleanWalkEmptyDirs(t *testing.T) { if strings.HasSuffix(tt.path, "/") { require.NoError(t, os.MkdirAll(p, perm.PrivateDir)) } else { - require.NoError(t, os.WriteFile(p, nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(p, nil, perm.PrivateWriteOnceFile)) if tt.stale { require.NoError(t, os.Chtimes(p, time.Now(), time.Now().Add(-time.Hour))) } diff --git a/internal/cgroups/mock_linux_test.go b/internal/cgroups/mock_linux_test.go index 88f0b44463..a1fde9b81c 100644 --- a/internal/cgroups/mock_linux_test.go +++ b/internal/cgroups/mock_linux_test.go @@ -22,6 +22,7 @@ package cgroups import ( "fmt" + "io/fs" "os" "path/filepath" "testing" @@ -156,7 +157,7 @@ func (m *mockCgroupV1) setupMockCgroupFiles( for filename, content := range content { controlFilePath := filepath.Join(cgroupPath, filename) - require.NoError(t, os.WriteFile(controlFilePath, []byte(content), perm.SharedFile)) + require.NoError(t, os.WriteFile(controlFilePath, []byte(content), fs.ModePerm)) } for _, shard := range shards { @@ -165,7 +166,7 @@ func (m *mockCgroupV1) setupMockCgroupFiles( for filename, content := range content { shardControlFilePath := filepath.Join(shardPath, filename) - require.NoError(t, os.WriteFile(shardControlFilePath, []byte(content), perm.SharedFile)) + require.NoError(t, os.WriteFile(shardControlFilePath, []byte(content), fs.ModePerm)) } } } @@ -244,12 +245,12 @@ func (m *mockCgroupV2) setupMockCgroupFiles( for filename, content := range content { controlFilePath := filepath.Join(m.root, manager.cfg.HierarchyRoot, filename) - require.NoError(t, os.WriteFile(controlFilePath, []byte(content), perm.SharedFile)) + require.NoError(t, os.WriteFile(controlFilePath, []byte(content), fs.ModePerm)) } for filename, content := range content { controlFilePath := filepath.Join(cgroupPath, filename) - require.NoError(t, os.WriteFile(controlFilePath, []byte(content), perm.SharedFile)) + require.NoError(t, os.WriteFile(controlFilePath, []byte(content), fs.ModePerm)) } for _, shard := range shards { @@ -258,7 +259,7 @@ func (m *mockCgroupV2) setupMockCgroupFiles( for filename, content := range content { shardControlFilePath := filepath.Join(shardPath, filename) - require.NoError(t, os.WriteFile(shardControlFilePath, []byte(content), perm.SharedFile)) + require.NoError(t, os.WriteFile(shardControlFilePath, []byte(content), fs.ModePerm)) } } } diff --git a/internal/git/conflict/parser_test.go b/internal/git/conflict/parser_test.go index 31793e400a..4d965d16ce 100644 --- a/internal/git/conflict/parser_test.go +++ b/internal/git/conflict/parser_test.go @@ -112,7 +112,7 @@ we can both agree on this line though t.Run(tt.name, func(t *testing.T) { entry := Entry{ Path: tt.path, - Mode: uint(perm.SharedFile), + Mode: uint(perm.PrivateWriteOnceFile), Contents: []byte("something-with-trailing-newline\n"), } diff --git a/internal/git/gittest/http_server.go b/internal/git/gittest/http_server.go index 64757b3026..f1407d4464 100644 --- a/internal/git/gittest/http_server.go +++ b/internal/git/gittest/http_server.go @@ -19,7 +19,7 @@ import ( // prepared such that git-http-backend(1) will serve it by creating the "git-daemon-export-ok" magic // file. func HTTPServer(tb testing.TB, ctx context.Context, gitCmdFactory git.CommandFactory, repoPath string, middleware func(http.ResponseWriter, *http.Request, http.Handler)) int { - require.NoError(tb, os.WriteFile(filepath.Join(repoPath, "git-daemon-export-ok"), nil, perm.SharedFile)) + require.NoError(tb, os.WriteFile(filepath.Join(repoPath, "git-daemon-export-ok"), nil, perm.PrivateWriteOnceFile)) listener, err := net.Listen("tcp", "127.0.0.1:0") require.NoError(tb, err) diff --git a/internal/git/housekeeping/clean_stale_data_test.go b/internal/git/housekeeping/clean_stale_data_test.go index 254577ff53..c2341d7e1b 100644 --- a/internal/git/housekeeping/clean_stale_data_test.go +++ b/internal/git/housekeeping/clean_stale_data_test.go @@ -255,7 +255,7 @@ func TestPruneEmptyConfigSections(t *testing.T) { }, } { t.Run(tc.desc, func(t *testing.T) { - require.NoError(t, os.WriteFile(configPath, []byte(tc.configData), perm.SharedFile)) + require.NoError(t, os.WriteFile(configPath, []byte(tc.configData), perm.PrivateWriteOnceFile)) skippedSections, err := PruneEmptyConfigSections(ctx, repo) require.NoError(t, err) @@ -364,7 +364,7 @@ func TestRemoveGitLabFullPathConfig(t *testing.T) { }, } { t.Run(tc.desc, func(t *testing.T) { - require.NoError(t, os.WriteFile(configPath, []byte(tc.configData), perm.SharedFile)) + require.NoError(t, os.WriteFile(configPath, []byte(tc.configData), perm.PrivateWriteOnceFile)) cleanupCount, err := removeGitLabFullPathConfig(ctx, repo, &transaction.MockManager{}) require.NoError(t, err) diff --git a/internal/git/housekeeping/manager/optimize_repository_test.go b/internal/git/housekeeping/manager/optimize_repository_test.go index c25e4e126b..b5aa1a2baa 100644 --- a/internal/git/housekeeping/manager/optimize_repository_test.go +++ b/internal/git/housekeeping/manager/optimize_repository_test.go @@ -731,7 +731,7 @@ func TestOptimizeRepository(t *testing.T) { for i := 0; i < housekeeping.LooseObjectLimit+1; i++ { blobPath := filepath.Join(repoPath, "objects", "17", fmt.Sprintf("%d", i)) - require.NoError(t, os.WriteFile(blobPath, nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(blobPath, nil, perm.PrivateWriteOnceFile)) require.NoError(t, os.Chtimes(blobPath, almostTwoWeeksAgo, almostTwoWeeksAgo)) } @@ -766,7 +766,7 @@ func TestOptimizeRepository(t *testing.T) { for i := 0; i < housekeeping.LooseObjectLimit+1; i++ { blobPath := filepath.Join(repoPath, "objects", "17", fmt.Sprintf("%d", i)) - require.NoError(t, os.WriteFile(blobPath, nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(blobPath, nil, perm.PrivateWriteOnceFile)) require.NoError(t, os.Chtimes(blobPath, moreThanTwoWeeksAgo, moreThanTwoWeeksAgo)) } @@ -1733,7 +1733,7 @@ func TestRepositoryManager_CleanStaleData_reftable(t *testing.T) { path := filepath.Join(repoPath, "reftable", "tables.list.lock") - require.NoError(t, os.WriteFile(path, []byte{}, perm.SharedFile)) + require.NoError(t, os.WriteFile(path, []byte{}, perm.PrivateWriteOnceFile)) filetime := time.Now().Add(-tc.age) require.NoError(t, os.Chtimes(path, filetime, filetime)) @@ -1843,7 +1843,7 @@ func TestRepositoryManager_CleanStaleData_references(t *testing.T) { path := filepath.Join(repoPath, ref.name) require.NoError(t, os.MkdirAll(filepath.Dir(path), perm.PrivateDir)) - require.NoError(t, os.WriteFile(path, bytes.Repeat([]byte{0}, ref.size), perm.SharedFile)) + require.NoError(t, os.WriteFile(path, bytes.Repeat([]byte{0}, ref.size), perm.PrivateWriteOnceFile)) filetime := time.Now().Add(-ref.age) require.NoError(t, os.Chtimes(path, filetime, filetime)) } @@ -2492,7 +2492,7 @@ func TestRepositoryManager_CleanStaleData_unsetConfiguration(t *testing.T) { else = untouched [totally] unrelated = untouched -`), perm.SharedFile)) +`), perm.PrivateWriteOnceFile)) mgr := New(cfg.Prometheus, testhelper.SharedLogger(t), nil, nil) @@ -2588,7 +2588,7 @@ func TestRepositoryManager_CleanStaleData_pruneEmptyConfigSections(t *testing.T) [remote "tmp-03b5e8c765135b343214d471843a062a"] [remote "tmp-f57338181aca1d599669dbb71ce9ce57"] [remote "tmp-8c948ca94832c2725733e48cb2902287"] -`), perm.SharedFile)) +`), perm.PrivateWriteOnceFile)) mgr := New(cfg.Prometheus, testhelper.SharedLogger(t), nil, nil) @@ -2629,7 +2629,7 @@ func TestRepositoryManager_CleanStaleData_removeGitLabFullPathConfig(t *testing. [gitlab] fullpath = foo/bar other = config -`), perm.SharedFile)) +`), perm.PrivateWriteOnceFile)) mgr := New(cfg.Prometheus, testhelper.SharedLogger(t), nil, nil) diff --git a/internal/git/localrepo/refs_external_test.go b/internal/git/localrepo/refs_external_test.go index 4539a280ba..bf982a5401 100644 --- a/internal/git/localrepo/refs_external_test.go +++ b/internal/git/localrepo/refs_external_test.go @@ -179,7 +179,7 @@ func TestRepo_SetDefaultBranch_errors(t *testing.T) { require.NoError(t, updater.Prepare()) t.Cleanup(func() { require.NoError(t, updater.Close()) }) } else { - require.NoError(t, os.WriteFile(filepath.Join(repoPath, "HEAD.lock"), []byte(""), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(repoPath, "HEAD.lock"), []byte(""), perm.PrivateWriteOnceFile)) } err = repo.SetDefaultBranch(ctx, &transaction.MockManager{}, "refs/heads/branch") diff --git a/internal/git/localrepo/snapshot_test.go b/internal/git/localrepo/snapshot_test.go index b7bf639a7c..dec1aba7cc 100644 --- a/internal/git/localrepo/snapshot_test.go +++ b/internal/git/localrepo/snapshot_test.go @@ -95,7 +95,7 @@ doesn't seem to test a realistic scenario.`) ) // The shallow file, used if the repository is a shallow clone, is also included in snapshots. - require.NoError(t, os.WriteFile(filepath.Join(repoPath, "shallow"), nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(repoPath, "shallow"), nil, perm.PrivateWriteOnceFile)) // Custom Git hooks are not included in snapshots. require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "hooks"), perm.PrivateDir)) @@ -104,7 +104,7 @@ doesn't seem to test a realistic scenario.`) require.NoError(t, os.WriteFile( filepath.Join(repoPath, "objects/this-should-not-be-included"), nil, - perm.SharedFile, + perm.PrivateWriteOnceFile, )) return setupData{ @@ -136,7 +136,7 @@ doesn't seem to test a realistic scenario.`) require.NoError(t, os.WriteFile( altFile, []byte(fmt.Sprintf("%s\n", altObjectDir)), - perm.SharedFile, + perm.PrivateWriteOnceFile, )) refs := gittest.FilesOrReftables( @@ -198,7 +198,7 @@ doesn't seem to test a realistic scenario.`) require.NoError(t, os.WriteFile( altFile, []byte(fmt.Sprintf("%s\n", altObjectDir)), - perm.SharedFile, + perm.PrivateWriteOnceFile, )) gittest.RequireObjectExists(t, cfg, repoPath, commitID) @@ -245,7 +245,7 @@ doesn't seem to test a realistic scenario.`) require.NoError(t, os.WriteFile( altFile, []byte(fmt.Sprintf("%s\n", relAltObjectDir)), - perm.SharedFile, + perm.PrivateWriteOnceFile, )) gittest.RequireObjectExists(t, cfg, repoPath, commitID) diff --git a/internal/git/objectpool/disconnect_test.go b/internal/git/objectpool/disconnect_test.go index a4eecbef36..7521404738 100644 --- a/internal/git/objectpool/disconnect_test.go +++ b/internal/git/objectpool/disconnect_test.go @@ -92,7 +92,7 @@ func TestDisconnect(t *testing.T) { altPath, err := repo.InfoAlternatesPath(ctx) require.NoError(t, err) - require.NoError(t, os.WriteFile(altPath, []byte(altContent), perm.SharedFile)) + require.NoError(t, os.WriteFile(altPath, []byte(altContent), perm.PrivateWriteOnceFile)) return repo } @@ -188,7 +188,7 @@ func TestDisconnect(t *testing.T) { altPath, err := repo.InfoAlternatesPath(ctx) require.NoError(t, err) - require.NoError(t, os.WriteFile(altPath, []byte(altObjectDir), perm.SharedFile)) + require.NoError(t, os.WriteFile(altPath, []byte(altObjectDir), perm.PrivateWriteOnceFile)) return setupData{ repository: repo, @@ -382,7 +382,7 @@ func TestRemoveAlternatesIfOk(t *testing.T) { altPath, err := repo.InfoAlternatesPath(ctx) require.NoError(t, err) altContent := testhelper.TempDir(t) + "\n" - require.NoError(t, os.WriteFile(altPath, []byte(altContent), perm.SharedFile)) + require.NoError(t, os.WriteFile(altPath, []byte(altContent), perm.PrivateWriteOnceFile)) // Intentionally break the repository so that the consistency check will cause an // error. @@ -412,7 +412,7 @@ func TestRemoveAlternatesIfOk(t *testing.T) { altPath, err := repo.InfoAlternatesPath(ctx) require.NoError(t, err) altContent := testhelper.TempDir(t) + "\n" - require.NoError(t, os.WriteFile(altPath, []byte(altContent), perm.SharedFile)) + require.NoError(t, os.WriteFile(altPath, []byte(altContent), perm.PrivateWriteOnceFile)) // In order to test the scenario where a commit is in a commit graph but not in the // object database, we will first write a new commit, write the commit graph, then diff --git a/internal/git/objectpool/link_test.go b/internal/git/objectpool/link_test.go index 7c0c25b5c2..9f78257236 100644 --- a/internal/git/objectpool/link_test.go +++ b/internal/git/objectpool/link_test.go @@ -112,7 +112,7 @@ func TestLink(t *testing.T) { // Link the repository to object pool using the absolute path of the object pool. // The alternates file should be rewritten to use the relative path. poolObjectsPath := gittest.RepositoryPath(t, ctx, pool, "objects") - require.NoError(t, os.WriteFile(altPath, []byte(poolObjectsPath), perm.SharedFile)) + require.NoError(t, os.WriteFile(altPath, []byte(poolObjectsPath), perm.PrivateWriteOnceFile)) return setupData{ cfg: cfg, @@ -130,7 +130,7 @@ func TestLink(t *testing.T) { // nothing and completes normally. altPath, err := repo.InfoAlternatesPath(ctx) require.NoError(t, err) - require.NoError(t, os.WriteFile(altPath, []byte(getRelAltPath(t, repo, pool.Repo)), perm.SharedFile)) + require.NoError(t, os.WriteFile(altPath, []byte(getRelAltPath(t, repo, pool.Repo)), perm.PrivateWriteOnceFile)) return setupData{ cfg: cfg, @@ -148,7 +148,7 @@ func TestLink(t *testing.T) { // linking operation fails. altPath, err := repo.InfoAlternatesPath(ctx) require.NoError(t, err) - require.NoError(t, os.WriteFile(altPath, []byte("../different/object/pool"), perm.SharedFile)) + require.NoError(t, os.WriteFile(altPath, []byte("../different/object/pool"), perm.PrivateWriteOnceFile)) return setupData{ cfg: cfg, @@ -195,7 +195,7 @@ func TestLink(t *testing.T) { // to the same object pool. altPath, err := repo.InfoAlternatesPath(ctx) require.NoError(t, err) - require.NoError(t, os.WriteFile(altPath, []byte(getRelAltPath(t, repo, pool.Repo)), perm.SharedFile)) + require.NoError(t, os.WriteFile(altPath, []byte(getRelAltPath(t, repo, pool.Repo)), perm.PrivateWriteOnceFile)) return setupData{ cfg: cfg, diff --git a/internal/git/objectpool/pool_test.go b/internal/git/objectpool/pool_test.go index 47fad57215..397bfd6ac5 100644 --- a/internal/git/objectpool/pool_test.go +++ b/internal/git/objectpool/pool_test.go @@ -114,7 +114,7 @@ func TestFromRepo_failures(t *testing.T) { require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "objects", "info"), perm.PrivateDir)) alternateFilePath := filepath.Join(repoPath, "objects", "info", "alternates") - require.NoError(t, os.WriteFile(alternateFilePath, tc.fileContent, perm.SharedFile)) + require.NoError(t, os.WriteFile(alternateFilePath, tc.fileContent, perm.PrivateWriteOnceFile)) poolFromRepo, err := FromRepo(ctx, logger, locator, pool.gitCmdFactory, nil, nil, nil, repo) require.Equal(t, tc.expectedErr, err) require.Nil(t, poolFromRepo) diff --git a/internal/git/remoterepo/repository_test.go b/internal/git/remoterepo/repository_test.go index 513a1d1cf1..678f5405c4 100644 --- a/internal/git/remoterepo/repository_test.go +++ b/internal/git/remoterepo/repository_test.go @@ -117,7 +117,7 @@ func TestRepository_ObjectHash(t *testing.T) { "[extensions]", "objectFormat = blake2b", }, "\n"), - ), perm.SharedFile)) + ), perm.PrivateWriteOnceFile)) repo, err := remoterepo.New(ctx, repoProto, pool) require.NoError(t, err) diff --git a/internal/git/stats/repository_info_test.go b/internal/git/stats/repository_info_test.go index 910d2282fb..f9aa26878a 100644 --- a/internal/git/stats/repository_info_test.go +++ b/internal/git/stats/repository_info_test.go @@ -1029,7 +1029,7 @@ func TestReferencesInfoForRepository(t *testing.T) { // We just write some random garbage -- we don't verify contents // anyway, but just the size. And testing like that is at least // deterministic as we don't have to special-case hash sizes. - require.NoError(t, os.WriteFile(filepath.Join(repoPath, "packed-refs"), []byte("content"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(repoPath, "packed-refs"), []byte("content"), perm.PrivateWriteOnceFile)) }, expectedInfo: ReferencesInfo{ ReferenceBackendName: gittest.DefaultReferenceBackend.Name, @@ -1050,7 +1050,7 @@ func TestReferencesInfoForRepository(t *testing.T) { // We just write some random garbage -- we don't verify contents // anyway, but just the size. And testing like that is at least // deterministic as we don't have to special-case hash sizes. - require.NoError(t, os.WriteFile(filepath.Join(repoPath, "packed-refs"), []byte("content"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(repoPath, "packed-refs"), []byte("content"), perm.PrivateWriteOnceFile)) }, expectedInfo: ReferencesInfo{ ReferenceBackendName: gittest.DefaultReferenceBackend.Name, @@ -1102,7 +1102,7 @@ func TestCountLooseObjects(t *testing.T) { differentShard := filepath.Join(repoPath, "objects", "a0") require.NoError(t, os.MkdirAll(differentShard, perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(differentShard, "123456"), []byte("foobar"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(differentShard, "123456"), []byte("foobar"), perm.PrivateWriteOnceFile)) requireLooseObjectsInfo(t, repo, time.Now(), LooseObjectsInfo{ Count: 1, @@ -1118,7 +1118,7 @@ func TestCountLooseObjects(t *testing.T) { for i, shard := range []string{"00", "17", "32", "ff"} { shardPath := filepath.Join(repoPath, "objects", shard) require.NoError(t, os.MkdirAll(shardPath, perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(shardPath, "123456"), make([]byte, i), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(shardPath, "123456"), make([]byte, i), perm.PrivateWriteOnceFile)) } requireLooseObjectsInfo(t, repo, time.Now(), LooseObjectsInfo{ @@ -1173,8 +1173,8 @@ func TestCountLooseObjects(t *testing.T) { shard := filepath.Join(repoPath, "objects", "17") require.NoError(t, os.MkdirAll(shard, perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(shard, "012345"), []byte("valid"), perm.SharedFile)) - require.NoError(t, os.WriteFile(filepath.Join(shard, "garbage"), []byte("garbage"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(shard, "012345"), []byte("valid"), perm.PrivateWriteOnceFile)) + require.NoError(t, os.WriteFile(filepath.Join(shard, "garbage"), []byte("garbage"), perm.PrivateWriteOnceFile)) requireLooseObjectsInfo(t, repo, time.Now(), LooseObjectsInfo{ Count: 1, @@ -1213,7 +1213,7 @@ func BenchmarkCountLooseObjects(b *testing.B) { objectPath := filepath.Join(repoPath, "objects", "17", "12345") require.NoError(b, os.Mkdir(filepath.Dir(objectPath), perm.PrivateDir)) - require.NoError(b, os.WriteFile(objectPath, nil, perm.SharedFile)) + require.NoError(b, os.WriteFile(objectPath, nil, perm.PrivateWriteOnceFile)) b.ResetTimer() for i := 0; i < b.N; i++ { @@ -1228,7 +1228,7 @@ func BenchmarkCountLooseObjects(b *testing.B) { for i := 0; i < 256; i++ { objectPath := filepath.Join(repoPath, "objects", fmt.Sprintf("%02x", i), "12345") require.NoError(b, os.Mkdir(filepath.Dir(objectPath), perm.PrivateDir)) - require.NoError(b, os.WriteFile(objectPath, nil, perm.SharedFile)) + require.NoError(b, os.WriteFile(objectPath, nil, perm.PrivateWriteOnceFile)) } b.ResetTimer() @@ -1257,7 +1257,7 @@ func BenchmarkCountLooseObjects(b *testing.B) { for j := 0; j < looseObjectCount; j++ { objectPath := filepath.Join(shardPath, fmt.Sprintf("%d", j)) - require.NoError(b, os.WriteFile(objectPath, nil, perm.SharedFile)) + require.NoError(b, os.WriteFile(objectPath, nil, perm.PrivateWriteOnceFile)) } } @@ -1277,7 +1277,7 @@ func BenchmarkCountLooseObjects(b *testing.B) { for j := 0; j < 1000; j++ { objectPath := filepath.Join(shardPath, fmt.Sprintf("%d", j)) - require.NoError(b, os.WriteFile(objectPath, nil, perm.SharedFile)) + require.NoError(b, os.WriteFile(objectPath, nil, perm.PrivateWriteOnceFile)) } } @@ -1311,7 +1311,7 @@ func TestPackfileInfoForRepository(t *testing.T) { seedRepository: func(t *testing.T, repoPath string) { packfileDir := filepath.Join(repoPath, "objects", "pack") require.NoError(t, os.MkdirAll(packfileDir, perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.pack"), []byte("foobar"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.pack"), []byte("foobar"), perm.PrivateWriteOnceFile)) }, expectedInfo: PackfilesInfo{ Count: 1, @@ -1323,8 +1323,8 @@ func TestPackfileInfoForRepository(t *testing.T) { seedRepository: func(t *testing.T, repoPath string) { packfileDir := filepath.Join(repoPath, "objects", "pack") require.NoError(t, os.MkdirAll(packfileDir, perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.pack"), []byte("foobar"), perm.SharedFile)) - require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.keep"), []byte("foobar"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.pack"), []byte("foobar"), perm.PrivateWriteOnceFile)) + require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.keep"), []byte("foobar"), perm.PrivateWriteOnceFile)) }, expectedInfo: PackfilesInfo{ Count: 1, @@ -1338,8 +1338,8 @@ func TestPackfileInfoForRepository(t *testing.T) { seedRepository: func(t *testing.T, repoPath string) { packfileDir := filepath.Join(repoPath, "objects", "pack") require.NoError(t, os.MkdirAll(packfileDir, perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.pack"), []byte("foobar"), perm.SharedFile)) - require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.mtimes"), []byte("foobar"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.pack"), []byte("foobar"), perm.PrivateWriteOnceFile)) + require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.mtimes"), []byte("foobar"), perm.PrivateWriteOnceFile)) }, expectedInfo: PackfilesInfo{ Count: 1, @@ -1353,8 +1353,8 @@ func TestPackfileInfoForRepository(t *testing.T) { seedRepository: func(t *testing.T, repoPath string) { packfileDir := filepath.Join(repoPath, "objects", "pack") require.NoError(t, os.MkdirAll(packfileDir, perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.pack"), []byte("foobar"), perm.SharedFile)) - require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-bar.pack"), []byte("123"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-foo.pack"), []byte("foobar"), perm.PrivateWriteOnceFile)) + require.NoError(t, os.WriteFile(filepath.Join(packfileDir, "pack-bar.pack"), []byte("123"), perm.PrivateWriteOnceFile)) }, expectedInfo: PackfilesInfo{ Count: 2, @@ -1425,7 +1425,7 @@ func TestPackfileInfoForRepository(t *testing.T) { gittest.WriteCommit(t, cfg, repoPath, gittest.WithMessage("second"), gittest.WithBranch("second")) gittest.Exec(t, cfg, "-c", "pack.writeReverseIndex=true", "-C", repoPath, "repack", "-db", "--write-midx") - require.NoError(t, os.WriteFile(filepath.Join(repoPath, "objects", "pack", "garbage"), []byte("1"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(repoPath, "objects", "pack", "garbage"), []byte("1"), perm.PrivateWriteOnceFile)) }, expectedInfo: PackfilesInfo{ Count: 2, @@ -1750,7 +1750,7 @@ func TestBitmapInfoForPath(t *testing.T) { desc: "header is too short", setup: func(t *testing.T) string { bitmapPath := filepath.Join(testhelper.TempDir(t), "bitmap") - require.NoError(t, os.WriteFile(bitmapPath, []byte{0, 0, 0}, perm.SharedFile)) + require.NoError(t, os.WriteFile(bitmapPath, []byte{0, 0, 0}, perm.PrivateWriteOnceFile)) return bitmapPath }, expectedErr: fmt.Errorf("reading bitmap header: %w", io.ErrUnexpectedEOF), @@ -1761,7 +1761,7 @@ func TestBitmapInfoForPath(t *testing.T) { bitmapPath := filepath.Join(testhelper.TempDir(t), "bitmap") require.NoError(t, os.WriteFile(bitmapPath, []byte{ 'B', 'I', 'T', 'O', 0, 0, 0, 0, - }, perm.SharedFile)) + }, perm.PrivateWriteOnceFile)) return bitmapPath }, expectedErr: fmt.Errorf("invalid bitmap signature: %q", "BITO"), @@ -1772,7 +1772,7 @@ func TestBitmapInfoForPath(t *testing.T) { bitmapPath := filepath.Join(testhelper.TempDir(t), "bitmap") require.NoError(t, os.WriteFile(bitmapPath, []byte{ 'B', 'I', 'T', 'M', 0, 2, 0, 0, - }, perm.SharedFile)) + }, perm.PrivateWriteOnceFile)) return bitmapPath }, expectedErr: fmt.Errorf("unsupported version: 2"), diff --git a/internal/gitaly/config/config_test.go b/internal/gitaly/config/config_test.go index e9816a08f8..c3246a8e2b 100644 --- a/internal/gitaly/config/config_test.go +++ b/internal/gitaly/config/config_test.go @@ -1753,7 +1753,7 @@ func TestSetupRuntimeDirectory(t *testing.T) { t.Run("validation", func(t *testing.T) { dirPath := testhelper.TempDir(t) filePath := filepath.Join(dirPath, "file") - require.NoError(t, os.WriteFile(filePath, nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(filePath, nil, perm.PrivateWriteOnceFile)) for _, tc := range []struct { desc string @@ -2098,7 +2098,7 @@ func TestStorage_Validate(t *testing.T) { dirPath := testhelper.TempDir(t) filePath := filepath.Join(dirPath, "file") - require.NoError(t, os.WriteFile(filePath, nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(filePath, nil, perm.PrivateWriteOnceFile)) for _, tc := range []struct { name string storage Storage @@ -2140,7 +2140,7 @@ func TestTLS_Validate(t *testing.T) { tmpDir := testhelper.TempDir(t) tmpFile := filepath.Join(tmpDir, "file") - require.NoError(t, os.WriteFile(tmpFile, []byte("I am not a certificate"), perm.SharedFile)) + require.NoError(t, os.WriteFile(tmpFile, []byte("I am not a certificate"), perm.PrivateWriteOnceFile)) for _, tc := range []struct { name string @@ -2241,7 +2241,7 @@ func TestGitlabShell_Validate(t *testing.T) { tmpDir := testhelper.TempDir(t) tmpFile := filepath.Join(tmpDir, "file") - require.NoError(t, os.WriteFile(tmpFile, nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(tmpFile, nil, perm.PrivateWriteOnceFile)) for _, tc := range []struct { name string diff --git a/internal/gitaly/linguist/language_stats_test.go b/internal/gitaly/linguist/language_stats_test.go index 0f003054b9..0b5046513d 100644 --- a/internal/gitaly/linguist/language_stats_test.go +++ b/internal/gitaly/linguist/language_stats_test.go @@ -51,7 +51,7 @@ func TestInitLanguageStats(t *testing.T) { { desc: "corrupt cache", run: func(t *testing.T, repo *localrepo.Repo, repoPath string) { - require.NoError(t, os.WriteFile(filepath.Join(repoPath, languageStatsFilename), []byte("garbage"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(repoPath, languageStatsFilename), []byte("garbage"), perm.PrivateWriteOnceFile)) stats, err := initLanguageStats(ctx, repo) require.Errorf(t, err, "new language stats zlib reader: invalid header") diff --git a/internal/gitaly/linguist/linguist_test.go b/internal/gitaly/linguist/linguist_test.go index 73a8dcece2..bf3750d718 100644 --- a/internal/gitaly/linguist/linguist_test.go +++ b/internal/gitaly/linguist/linguist_test.go @@ -457,7 +457,7 @@ func TestInstance_Stats(t *testing.T) { gittest.TreeEntry{Path: "application.rb", Mode: "100644", Content: strings.Repeat("a", 2943)}, )) - require.NoError(t, os.WriteFile(filepath.Join(repoPath, languageStatsFilename), []byte("garbage"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(repoPath, languageStatsFilename), []byte("garbage"), perm.PrivateWriteOnceFile)) return repoProto, repoPath, commitID }, diff --git a/internal/gitaly/repoutil/custom_hooks_test.go b/internal/gitaly/repoutil/custom_hooks_test.go index 24d6a7b3ce..3e187d86f5 100644 --- a/internal/gitaly/repoutil/custom_hooks_test.go +++ b/internal/gitaly/repoutil/custom_hooks_test.go @@ -327,7 +327,7 @@ func TestNewDirectoryVote(t *testing.T) { {name: "pre-commit.sample", content: "foo", mode: perm.PrivateExecutable}, {name: "pre-push.sample", content: "bar", mode: perm.PrivateExecutable}, }, - expectedHash: "8ca11991268de4c9278488a674fc1a88db449566", + expectedHash: "3c0fd54e0428c5ee04c15ee5a52864694771fb20", }, { desc: "generated hash matches with changed file name", @@ -335,7 +335,7 @@ func TestNewDirectoryVote(t *testing.T) { {name: "pre-commit.sample.diff", content: "foo", mode: perm.PrivateExecutable}, {name: "pre-push.sample", content: "bar", mode: perm.PrivateExecutable}, }, - expectedHash: "b5ed58ced84103da1ed9d7813a9e39b3b5daf7d7", + expectedHash: "2d5080ef5ed0a52254a794915c8fbbec8c694224", }, { desc: "generated hash matches with changed file content", @@ -343,15 +343,15 @@ func TestNewDirectoryVote(t *testing.T) { {name: "pre-commit.sample", content: "foo", mode: perm.PrivateExecutable}, {name: "pre-push.sample", content: "bar.diff", mode: perm.PrivateExecutable}, }, - expectedHash: "178083848c8a08e36c4f86c2d318a84b0bb845f2", + expectedHash: "18e2d3f9cc9990747b27cf8a7fad281539856194", }, { desc: "generated hash matches with changed file mode", files: []testFile{ - {name: "pre-commit.sample", content: "foo", mode: perm.SharedFile}, + {name: "pre-commit.sample", content: "foo", mode: perm.PrivateWriteOnceFile}, {name: "pre-push.sample", content: "bar", mode: perm.PrivateExecutable}, }, - expectedHash: "c69574241b83496bb4005b4f7a0dfcda96cb317e", + expectedHash: "ad20a4fea20e9049bb70e084e757fcc5d2cf2cc7", }, } { t.Run(tc.desc, func(t *testing.T) { diff --git a/internal/gitaly/repoutil/remove_test.go b/internal/gitaly/repoutil/remove_test.go index 087cdc8a3c..13ab9ecce5 100644 --- a/internal/gitaly/repoutil/remove_test.go +++ b/internal/gitaly/repoutil/remove_test.go @@ -50,7 +50,7 @@ func TestRemove(t *testing.T) { // Simulate a concurrent RPC holding the repository lock. lockPath := repoPath + ".lock" - require.NoError(t, os.WriteFile(lockPath, []byte{}, perm.SharedFile)) + require.NoError(t, os.WriteFile(lockPath, []byte{}, perm.PrivateWriteOnceFile)) tb.Cleanup(func() { require.NoError(t, os.RemoveAll(lockPath)) }) diff --git a/internal/gitaly/service/objectpool/alternates_test.go b/internal/gitaly/service/objectpool/alternates_test.go index 2dfa3dbccc..fc482fcf8a 100644 --- a/internal/gitaly/service/objectpool/alternates_test.go +++ b/internal/gitaly/service/objectpool/alternates_test.go @@ -93,7 +93,7 @@ func TestDisconnectGitAlternatesUnexpectedAlternates(t *testing.T) { altPath, err := repo.InfoAlternatesPath(ctx) require.NoError(t, err) - require.NoError(t, os.WriteFile(altPath, []byte(tc.altContent), perm.SharedFile)) + require.NoError(t, os.WriteFile(altPath, []byte(tc.altContent), perm.PrivateWriteOnceFile)) _, err = client.DisconnectGitAlternates(ctx, &gitalypb.DisconnectGitAlternatesRequest{Repository: repoProto}) require.Error(t, err) diff --git a/internal/gitaly/service/objectpool/create_test.go b/internal/gitaly/service/objectpool/create_test.go index 49452decdd..d0f497ee6b 100644 --- a/internal/gitaly/service/objectpool/create_test.go +++ b/internal/gitaly/service/objectpool/create_test.go @@ -144,7 +144,7 @@ func TestCreate_unsuccessful(t *testing.T) { lockedRelativePath := gittest.NewObjectPoolName(t) lockedFullPath := filepath.Join(cfg.Storages[0].Path, lockedRelativePath+".lock") require.NoError(t, os.MkdirAll(filepath.Dir(lockedFullPath), perm.PrivateDir)) - require.NoError(t, os.WriteFile(lockedFullPath, nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(lockedFullPath, nil, perm.PrivateWriteOnceFile)) // Create a preexisting object pool. preexistingPool := &gitalypb.ObjectPool{ diff --git a/internal/gitaly/service/objectpool/get_test.go b/internal/gitaly/service/objectpool/get_test.go index 6ef44351ba..f05276c120 100644 --- a/internal/gitaly/service/objectpool/get_test.go +++ b/internal/gitaly/service/objectpool/get_test.go @@ -55,7 +55,7 @@ func TestGetObjectPoolBadFile(t *testing.T) { alternatesFilePath := filepath.Join(repoPath, "objects", "info", "alternates") require.NoError(t, os.MkdirAll(filepath.Dir(alternatesFilePath), perm.PrivateDir)) - require.NoError(t, os.WriteFile(alternatesFilePath, []byte("not-a-directory"), perm.SharedFile)) + require.NoError(t, os.WriteFile(alternatesFilePath, []byte("not-a-directory"), perm.PrivateWriteOnceFile)) resp, err := client.GetObjectPool(ctx, &gitalypb.GetObjectPoolRequest{ Repository: repo, diff --git a/internal/gitaly/service/objectpool/link_test.go b/internal/gitaly/service/objectpool/link_test.go index 031c73957e..3294b5216a 100644 --- a/internal/gitaly/service/objectpool/link_test.go +++ b/internal/gitaly/service/objectpool/link_test.go @@ -186,7 +186,7 @@ func TestLink_noClobber(t *testing.T) { require.NoFileExists(t, alternatesFile) contentBefore := "mock/objects\n" - require.NoError(t, os.WriteFile(alternatesFile, []byte(contentBefore), perm.SharedFile)) + require.NoError(t, os.WriteFile(alternatesFile, []byte(contentBefore), perm.PrivateWriteOnceFile)) request := &gitalypb.LinkRepositoryToObjectPoolRequest{ Repository: repoProto, diff --git a/internal/gitaly/service/repository/create_fork_test.go b/internal/gitaly/service/repository/create_fork_test.go index f1ff59f8dc..492ed63209 100644 --- a/internal/gitaly/service/repository/create_fork_test.go +++ b/internal/gitaly/service/repository/create_fork_test.go @@ -310,7 +310,7 @@ func TestCreateFork_targetExists(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(targetPath, "config"), nil, - perm.SharedFile, + perm.PrivateWriteOnceFile, )) }, expectedErr: func() error { @@ -325,7 +325,7 @@ func TestCreateFork_targetExists(t *testing.T) { desc: "target file", seed: func(t *testing.T, targetPath string) { require.NoError(t, os.MkdirAll(filepath.Dir(targetPath), perm.PrivateDir)) - require.NoError(t, os.WriteFile(targetPath, nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(targetPath, nil, perm.PrivateWriteOnceFile)) }, expectedErr: func() error { if testhelper.IsWALEnabled() { 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 9fdf60af3d..3720ac23ce 100644 --- a/internal/gitaly/service/repository/create_repository_from_url_test.go +++ b/internal/gitaly/service/repository/create_repository_from_url_test.go @@ -142,7 +142,7 @@ testing of this scenario should be left to the relevant package. require.NoError(t, os.MkdirAll(importedRepoPath, perm.PrivateDir)) } else { require.NoError(t, os.MkdirAll(filepath.Dir(importedRepoPath), perm.PrivateDir)) - require.NoError(t, os.WriteFile(importedRepoPath, nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(importedRepoPath, nil, perm.PrivateWriteOnceFile)) } t.Cleanup(func() { require.NoError(t, os.RemoveAll(importedRepoPath)) }) diff --git a/internal/gitaly/service/repository/fetch_remote_test.go b/internal/gitaly/service/repository/fetch_remote_test.go index f4ed58261a..be7964795a 100644 --- a/internal/gitaly/service/repository/fetch_remote_test.go +++ b/internal/gitaly/service/repository/fetch_remote_test.go @@ -1127,7 +1127,7 @@ func TestFetchRemote_pooledRepository(t *testing.T) { // Create the pooled repository and link it to its pool. This is the // repository we're fetching into. pooledRepoProto, pooledRepoPath := gittest.CreateRepository(t, ctx, cfg) - require.NoError(t, os.WriteFile(filepath.Join(pooledRepoPath, "objects", "info", "alternates"), []byte(filepath.Join(poolRepoPath, "objects")), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(pooledRepoPath, "objects", "info", "alternates"), []byte(filepath.Join(poolRepoPath, "objects")), perm.PrivateWriteOnceFile)) // And then finally create a third repository that emulates the remote side // we're fetching from. We need to create at least one reference so that Git diff --git a/internal/gitaly/service/repository/fsck_test.go b/internal/gitaly/service/repository/fsck_test.go index 283be0d5e2..365ef9a3ec 100644 --- a/internal/gitaly/service/repository/fsck_test.go +++ b/internal/gitaly/service/repository/fsck_test.go @@ -77,7 +77,7 @@ func TestFsck(t *testing.T) { // This makes the repo severely broken so that `git` does not // identify it as a proper repository anymore. require.NoError(t, os.RemoveAll(filepath.Join(repoPath, "objects"))) - require.NoError(t, os.WriteFile(filepath.Join(repoPath, "objects"), nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(repoPath, "objects"), nil, perm.PrivateWriteOnceFile)) setupData := setupData{ repo: repo, diff --git a/internal/gitaly/service/repository/info_attributes_test.go b/internal/gitaly/service/repository/info_attributes_test.go index 75a4e6a4a5..3af71bd576 100644 --- a/internal/gitaly/service/repository/info_attributes_test.go +++ b/internal/gitaly/service/repository/info_attributes_test.go @@ -30,7 +30,7 @@ func TestGetInfoAttributesExisting(t *testing.T) { buffSize := streamio.WriteBufferSize + 1 data := bytes.Repeat([]byte("*.pbxproj binary\n"), buffSize) attrsPath := filepath.Join(infoPath, "attributes") - err := os.WriteFile(attrsPath, data, perm.SharedFile) + err := os.WriteFile(attrsPath, data, perm.PrivateWriteOnceFile) require.NoError(t, err) gitattributesContent := "*.go diff=go text\n*.md text\n*.jpg -text" diff --git a/internal/gitaly/service/repository/object_format_test.go b/internal/gitaly/service/repository/object_format_test.go index d11a7e9cf5..1efc6acd7c 100644 --- a/internal/gitaly/service/repository/object_format_test.go +++ b/internal/gitaly/service/repository/object_format_test.go @@ -142,7 +142,7 @@ func TestObjectFormat(t *testing.T) { "[extensions]", "objectFormat = blake2b", }, "\n"), - ), perm.SharedFile)) + ), perm.PrivateWriteOnceFile)) return setupData{ request: &gitalypb.ObjectFormatRequest{ diff --git a/internal/gitaly/service/repository/remove_test.go b/internal/gitaly/service/repository/remove_test.go index 05d3f876e5..4f12587271 100644 --- a/internal/gitaly/service/repository/remove_test.go +++ b/internal/gitaly/service/repository/remove_test.go @@ -83,7 +83,7 @@ logic will be removed once transaction managements is always enabled.`) // Simulate a concurrent RPC holding the repository lock. lockPath := repoPath + ".lock" - require.NoError(t, os.WriteFile(lockPath, []byte{}, perm.SharedFile)) + require.NoError(t, os.WriteFile(lockPath, []byte{}, perm.PrivateWriteOnceFile)) defer func() { require.NoError(t, os.RemoveAll(lockPath)) }() _, err := client.RemoveRepository(ctx, &gitalypb.RemoveRepositoryRequest{Repository: repo}) diff --git a/internal/gitaly/service/repository/replicate_test.go b/internal/gitaly/service/repository/replicate_test.go index f67ab3d5e7..eaf6b3c691 100644 --- a/internal/gitaly/service/repository/replicate_test.go +++ b/internal/gitaly/service/repository/replicate_test.go @@ -105,7 +105,7 @@ func TestReplicateRepository(t *testing.T) { attrFilePath := filepath.Join(sourcePath, "info", "attributes") require.NoError(t, os.MkdirAll(filepath.Dir(attrFilePath), perm.PrivateDir)) attributesData := []byte("*.pbxproj binary\n") - require.NoError(t, os.WriteFile(attrFilePath, attributesData, perm.SharedFile)) + require.NoError(t, os.WriteFile(attrFilePath, attributesData, perm.PrivateWriteOnceFile)) return setupData{ source: source, diff --git a/internal/gitaly/service/repository/snapshot_test.go b/internal/gitaly/service/repository/snapshot_test.go index c2761f1039..0edbe895b4 100644 --- a/internal/gitaly/service/repository/snapshot_test.go +++ b/internal/gitaly/service/repository/snapshot_test.go @@ -139,7 +139,7 @@ func TestGetSnapshot(t *testing.T) { ) // The shallow file, used if the repository is a shallow clone, is also included in snapshots. - require.NoError(t, os.WriteFile(filepath.Join(repoPath, "shallow"), nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(repoPath, "shallow"), nil, perm.PrivateWriteOnceFile)) // Custom Git hooks are not included in snapshots. require.NoError(t, os.MkdirAll(filepath.Join(repoPath, "hooks"), perm.PrivateDir)) @@ -148,7 +148,7 @@ func TestGetSnapshot(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(repoPath, "objects/this-should-not-be-included"), nil, - perm.SharedFile, + perm.PrivateWriteOnceFile, )) return setupData{ @@ -227,7 +227,7 @@ func TestGetSnapshot(t *testing.T) { require.NoError(t, os.WriteFile( altFile, []byte(fmt.Sprintf("%s\n", altObjectDir)), - perm.SharedFile, + perm.PrivateWriteOnceFile, )) gittest.RequireObjectExists(t, cfg, repoPath, commitID) diff --git a/internal/gitaly/service/smarthttp/inforefs_test.go b/internal/gitaly/service/smarthttp/inforefs_test.go index f7cc7e4d7c..13c395fa6b 100644 --- a/internal/gitaly/service/smarthttp/inforefs_test.go +++ b/internal/gitaly/service/smarthttp/inforefs_test.go @@ -676,7 +676,7 @@ func withInfoRefCache(cache infoRefCache) ServerOpt { func replaceCachedResponse(tb testing.TB, ctx context.Context, cache *cache.DiskCache, req *gitalypb.InfoRefsRequest, newContents string) { path := pathToCachedResponse(tb, ctx, cache, req) - require.NoError(tb, os.WriteFile(path, []byte(newContents), perm.SharedFile)) + require.NoError(tb, os.WriteFile(path, []byte(newContents), perm.PrivateWriteOnceFile)) } func setInfoRefsUploadPackMethod(ctx context.Context) context.Context { diff --git a/internal/gitaly/service/ssh/receive_pack_test.go b/internal/gitaly/service/ssh/receive_pack_test.go index 604613c772..d3de1390ee 100644 --- a/internal/gitaly/service/ssh/receive_pack_test.go +++ b/internal/gitaly/service/ssh/receive_pack_test.go @@ -294,7 +294,7 @@ func TestReceivePack_invalidGitconfig(t *testing.T) { // Remove the config file first as files are read-only with transactions. configPath := filepath.Join(remoteRepoPath, "config") require.NoError(t, os.Remove(configPath)) - require.NoError(t, os.WriteFile(configPath, []byte("x x x foobar"), perm.SharedFile)) + require.NoError(t, os.WriteFile(configPath, []byte("x x x foobar"), perm.PrivateWriteOnceFile)) remoteRepo.GlProjectPath = "something" lHead, rHead, err := setupRepoAndPush(t, ctx, cfg, &gitalypb.SSHReceivePackRequest{ diff --git a/internal/gitaly/service/ssh/upload_pack_test.go b/internal/gitaly/service/ssh/upload_pack_test.go index f5d6069b56..b8fd592838 100644 --- a/internal/gitaly/service/ssh/upload_pack_test.go +++ b/internal/gitaly/service/ssh/upload_pack_test.go @@ -835,7 +835,7 @@ func testUploadPackGitFailure(t *testing.T, ctx context.Context) { // Remove the config file first as files are read-only with transactions. configPath := filepath.Join(repoPath, "config") require.NoError(t, os.Remove(configPath)) - require.NoError(t, os.WriteFile(configPath, []byte("Not a valid gitconfig"), perm.SharedFile)) + require.NoError(t, os.WriteFile(configPath, []byte("Not a valid gitconfig"), perm.PrivateWriteOnceFile)) stream, err := client.SSHUploadPack(ctx) require.NoError(t, err) diff --git a/internal/gitaly/storage/storagemgr/apply_operations_test.go b/internal/gitaly/storage/storagemgr/apply_operations_test.go index dddbf1b7e7..9d1c3ec0dc 100644 --- a/internal/gitaly/storage/storagemgr/apply_operations_test.go +++ b/internal/gitaly/storage/storagemgr/apply_operations_test.go @@ -35,7 +35,7 @@ func TestApplyOperations(t *testing.T) { "parent": {Mode: fs.ModeDir | perm.PrivateDir}, "parent/relative-path": {Mode: fs.ModeDir | perm.PrivateDir}, "parent/relative-path/private-file": {Mode: perm.PrivateWriteOnceFile, Data: []byte("private")}, - "parent/relative-path/shared-file": {Mode: perm.SharedFile, Data: []byte("shared")}, + "parent/relative-path/shared-file": {Mode: perm.PrivateWriteOnceFile, Data: []byte("shared")}, "parent/relative-path/empty-dir": {Mode: fs.ModeDir | perm.PrivateDir}, "parent/relative-path/removed-dir": {Mode: fs.ModeDir | perm.PrivateDir}, "parent/relative-path/dir-with-removed-file": {Mode: fs.ModeDir | perm.PrivateDir}, diff --git a/internal/gitaly/storage/wal/entry_test.go b/internal/gitaly/storage/wal/entry_test.go index bf7c3b0015..84284464b3 100644 --- a/internal/gitaly/storage/wal/entry_test.go +++ b/internal/gitaly/storage/wal/entry_test.go @@ -24,7 +24,7 @@ func setupTestDirectory(t *testing.T, path string) { require.NoError(t, os.WriteFile(filepath.Join(path, "file-1"), []byte("file-1"), perm.PrivateExecutable)) privateSubDir := filepath.Join(filepath.Join(path, "subdir-private")) require.NoError(t, os.Mkdir(privateSubDir, perm.PrivateDir)) - require.NoError(t, os.WriteFile(filepath.Join(privateSubDir, "file-2"), []byte("file-2"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(privateSubDir, "file-2"), []byte("file-2"), perm.PrivateWriteOnceFile)) sharedSubDir := filepath.Join(path, "subdir-shared") require.NoError(t, os.Mkdir(sharedSubDir, perm.PrivateDir)) require.NoError(t, os.WriteFile(filepath.Join(sharedSubDir, "file-3"), []byte("file-3"), perm.PrivateWriteOnceFile)) @@ -268,12 +268,12 @@ func TestRecordAlternateUnlink(t *testing.T) { "objects/info": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/3f": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/3f/1": {Mode: perm.PrivateWriteOnceFile}, - "objects/3f/2": {Mode: perm.SharedFile}, + "objects/3f/2": {Mode: perm.PrivateWriteOnceFile}, "objects/4f": {Mode: fs.ModeDir | perm.PrivateDir}, - "objects/4f/3": {Mode: perm.SharedFile}, + "objects/4f/3": {Mode: perm.PrivateWriteOnceFile}, "objects/pack": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/pack/pack.pack": {Mode: perm.PrivateWriteOnceFile}, - "objects/pack/pack.idx": {Mode: perm.SharedFile}, + "objects/pack/pack.idx": {Mode: perm.PrivateWriteOnceFile}, }) } @@ -311,9 +311,9 @@ func TestRecordAlternateUnlink(t *testing.T) { "objects/3f": {Mode: fs.ModeDir | perm.PrivateDir}, "objects/3f/1": {Mode: perm.PrivateWriteOnceFile}, "objects/4f": {Mode: fs.ModeDir | perm.PrivateDir}, - "objects/4f/3": {Mode: perm.SharedFile}, + "objects/4f/3": {Mode: perm.PrivateWriteOnceFile}, "objects/pack": {Mode: fs.ModeDir | perm.PrivateDir}, - "objects/pack/pack.idx": {Mode: perm.SharedFile}, + "objects/pack/pack.idx": {Mode: perm.PrivateWriteOnceFile}, }) }, expectedOperations: func() operations { diff --git a/internal/gitlab/test_server.go b/internal/gitlab/test_server.go index b0a2ec5ee4..f9fa31efae 100644 --- a/internal/gitlab/test_server.go +++ b/internal/gitlab/test_server.go @@ -28,7 +28,7 @@ func WriteShellSecretFile(tb testing.TB, dir, secretToken string) string { require.NoError(tb, os.MkdirAll(dir, perm.PrivateDir)) filePath := filepath.Join(dir, ".gitlab_shell_secret") - require.NoError(tb, os.WriteFile(filePath, []byte(secretToken), perm.SharedFile)) + require.NoError(tb, os.WriteFile(filePath, []byte(secretToken), perm.PrivateWriteOnceFile)) return filePath } diff --git a/internal/safe/locking_directory_test.go b/internal/safe/locking_directory_test.go index 717c1e498b..054684c601 100644 --- a/internal/safe/locking_directory_test.go +++ b/internal/safe/locking_directory_test.go @@ -29,7 +29,7 @@ func TestLockingDirectory(t *testing.T) { require.NoError(t, os.WriteFile( filepath.Join(path, "somefile"), []byte("data"), - perm.SharedFile), + perm.PrivateWriteOnceFile), ) assert.ErrorIs(t, secondLockingDir.Lock(), safe.ErrFileAlreadyLocked) require.NoError(t, lockingDir.Unlock()) diff --git a/internal/safe/locking_file_writer_test.go b/internal/safe/locking_file_writer_test.go index 2302aca4e5..627742c989 100644 --- a/internal/safe/locking_file_writer_test.go +++ b/internal/safe/locking_file_writer_test.go @@ -148,7 +148,7 @@ func TestLockingFileWriter_seedingWithExistingTarget(t *testing.T) { t.Parallel() target := filepath.Join(testhelper.TempDir(t), "file") - require.NoError(t, os.WriteFile(target, []byte("seed"), perm.SharedFile)) + require.NoError(t, os.WriteFile(target, []byte("seed"), perm.PrivateWriteOnceFile)) writer, err := safe.NewLockingFileWriter(target, safe.LockingFileWriterConfig{ SeedContents: true, @@ -166,7 +166,7 @@ func TestLockingFileWriter_modifiesExistingFiles(t *testing.T) { t.Parallel() target := filepath.Join(testhelper.TempDir(t), "file") - require.NoError(t, os.WriteFile(target, []byte("preexisting"), perm.SharedFile)) + require.NoError(t, os.WriteFile(target, []byte("preexisting"), perm.PrivateWriteOnceFile)) writer, err := safe.NewLockingFileWriter(target) require.NoError(t, err) @@ -182,7 +182,7 @@ func TestLockingFileWriter_modifiesExistingFilesWithMode(t *testing.T) { t.Parallel() target := filepath.Join(testhelper.TempDir(t), "file") - require.NoError(t, os.WriteFile(target, []byte("preexisting"), perm.SharedFile)) + require.NoError(t, os.WriteFile(target, []byte("preexisting"), perm.PrivateWriteOnceFile)) writer, err := safe.NewLockingFileWriter(target, safe.LockingFileWriterConfig{ FileWriterConfig: safe.FileWriterConfig{FileMode: 0o060}, @@ -205,7 +205,7 @@ func TestLockingFileWriter_concurrentCreation(t *testing.T) { require.NoError(t, err) // Create file concurrently. - require.NoError(t, os.WriteFile(target, []byte("concurrent"), perm.SharedFile)) + require.NoError(t, os.WriteFile(target, []byte("concurrent"), perm.PrivateWriteOnceFile)) require.Equal(t, fmt.Errorf("file concurrently created"), writer.Lock()) @@ -217,7 +217,7 @@ func TestLockingFileWriter_concurrentDeletion(t *testing.T) { target := filepath.Join(testhelper.TempDir(t), "file") - require.NoError(t, os.WriteFile(target, []byte("base"), perm.SharedFile)) + require.NoError(t, os.WriteFile(target, []byte("base"), perm.PrivateWriteOnceFile)) writer, err := safe.NewLockingFileWriter(target) require.NoError(t, err) @@ -234,12 +234,13 @@ func TestLockingFileWriter_concurrentModification(t *testing.T) { target := filepath.Join(testhelper.TempDir(t), "file") - require.NoError(t, os.WriteFile(target, []byte("base"), perm.SharedFile)) + require.NoError(t, os.WriteFile(target, []byte("base"), perm.PrivateWriteOnceFile)) writer, err := safe.NewLockingFileWriter(target) require.NoError(t, err) // Concurrently modify the file. - require.NoError(t, os.WriteFile(target, []byte("concurrent"), perm.SharedFile)) + require.NoError(t, os.Remove(target)) + require.NoError(t, os.WriteFile(target, []byte("concurrent"), perm.PrivateWriteOnceFile)) require.Equal(t, fmt.Errorf("file concurrently modified"), writer.Lock()) @@ -272,13 +273,13 @@ func TestLockingFileWriter_locked(t *testing.T) { t.Parallel() target := filepath.Join(testhelper.TempDir(t), "file") - require.NoError(t, os.WriteFile(target, []byte("base"), perm.SharedFile)) + require.NoError(t, os.WriteFile(target, []byte("base"), perm.PrivateWriteOnceFile)) writer, err := safe.NewLockingFileWriter(target) require.NoError(t, err) // Concurrently lock the file. - require.NoError(t, os.WriteFile(target+".lock", nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(target+".lock", nil, perm.PrivateWriteOnceFile)) require.Equal(t, safe.ErrFileAlreadyLocked, writer.Lock()) @@ -291,7 +292,7 @@ func TestLockingFileWriter_externalProcess(t *testing.T) { cfg := testcfg.Build(t) target := filepath.Join(testhelper.TempDir(t), "file") - require.NoError(t, os.WriteFile(target, []byte("base"), perm.SharedFile)) + require.NoError(t, os.WriteFile(target, []byte("base"), perm.PrivateWriteOnceFile)) writer, err := safe.NewLockingFileWriter(target) require.NoError(t, err) diff --git a/internal/streamcache/cache_test.go b/internal/streamcache/cache_test.go index 421f6cc38f..6fabb71d4f 100644 --- a/internal/streamcache/cache_test.go +++ b/internal/streamcache/cache_test.go @@ -358,7 +358,7 @@ func TestCache_unWriteableFile(t *testing.T) { c := newCache(t, tmp) innerCache(c).createFile = func() (namedWriteCloser, error) { - return os.OpenFile(filepath.Join(tmp, "unwriteable"), os.O_RDONLY|os.O_CREATE|os.O_EXCL, perm.SharedFile) + return os.OpenFile(filepath.Join(tmp, "unwriteable"), os.O_RDONLY|os.O_CREATE|os.O_EXCL, perm.PrivateWriteOnceFile) } _, _, err := c.Fetch(ctx, "key", io.Discard, func(w io.Writer) error { @@ -379,7 +379,7 @@ func TestCache_unCloseableFile(t *testing.T) { c := newCache(t, tmp) innerCache(c).createFile = func() (namedWriteCloser, error) { - f, err := os.OpenFile(filepath.Join(tmp, "uncloseable"), os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm.SharedFile) + f, err := os.OpenFile(filepath.Join(tmp, "uncloseable"), os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm.PrivateWriteOnceFile) if err != nil { return nil, err } @@ -401,7 +401,7 @@ func TestCache_cannotOpenFileForReading(t *testing.T) { c := newCache(t, tmp) innerCache(c).createFile = func() (namedWriteCloser, error) { - f, err := os.OpenFile(filepath.Join(tmp, "unopenable"), os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm.SharedFile) + f, err := os.OpenFile(filepath.Join(tmp, "unopenable"), os.O_WRONLY|os.O_CREATE|os.O_EXCL, perm.PrivateWriteOnceFile) if err != nil { return nil, err } diff --git a/internal/streamcache/filestore_test.go b/internal/streamcache/filestore_test.go index f722cc7edb..16404d4c7e 100644 --- a/internal/streamcache/filestore_test.go +++ b/internal/streamcache/filestore_test.go @@ -109,7 +109,7 @@ func TestFilestoreCleanwalk(t *testing.T) { file := filepath.Join(dir2, "file") require.NoError(t, os.Mkdir(dir1, perm.PrivateDir)) require.NoError(t, os.Mkdir(dir2, perm.PrivateDir)) - require.NoError(t, os.WriteFile(file, nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(file, nil, perm.PrivateWriteOnceFile)) require.NoError(t, os.Chmod(dir2, 0), "create dir with pathological permissions") require.NoError(t, fs.cleanWalk(time.Now().Add(time.Hour))) diff --git a/internal/tempdir/clean_test.go b/internal/tempdir/clean_test.go index 75ce5a8145..587efc76c3 100644 --- a/internal/tempdir/clean_test.go +++ b/internal/tempdir/clean_test.go @@ -159,7 +159,7 @@ func makeFile(t *testing.T, locator storage.Locator, storage config.Storage, fil require.NoError(t, err) fullPath := filepath.Join(root, filePath) - require.NoError(t, os.WriteFile(fullPath, nil, perm.SharedFile)) + require.NoError(t, os.WriteFile(fullPath, nil, perm.PrivateWriteOnceFile)) require.NoError(t, os.Chtimes(fullPath, mtime, mtime)) } diff --git a/internal/tempdir/tempdir_test.go b/internal/tempdir/tempdir_test.go index 391afb1128..1e09c1dd47 100644 --- a/internal/tempdir/tempdir_test.go +++ b/internal/tempdir/tempdir_test.go @@ -29,7 +29,7 @@ func TestNewRepositorySuccess(t *testing.T) { require.NoError(t, err) require.Equal(t, tempDir.Path(), calculatedPath) - require.NoError(t, os.WriteFile(filepath.Join(tempDir.Path(), "test"), []byte("hello"), perm.SharedFile)) + require.NoError(t, os.WriteFile(filepath.Join(tempDir.Path(), "test"), []byte("hello"), perm.PrivateWriteOnceFile)) require.DirExists(t, tempDir.Path()) diff --git a/internal/testhelper/testcfg/gitaly.go b/internal/testhelper/testcfg/gitaly.go index 73fca75d41..8343ba0e6c 100644 --- a/internal/testhelper/testcfg/gitaly.go +++ b/internal/testhelper/testcfg/gitaly.go @@ -165,7 +165,7 @@ func WriteTemporaryGitalyConfigFile(tb testing.TB, cfg config.Cfg) string { contents, err := toml.Marshal(cfg) require.NoError(tb, err) - require.NoError(tb, os.WriteFile(path, contents, perm.SharedFile)) + require.NoError(tb, os.WriteFile(path, contents, perm.PrivateWriteOnceFile)) return path } -- GitLab