diff --git a/cli/templatepull.go b/cli/templatepull.go index 36fd214561f00..455ce624ce1f7 100644 --- a/cli/templatepull.go +++ b/cli/templatepull.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "os" + "path/filepath" "sort" "github.com/codeclysm/extract/v3" @@ -130,6 +131,13 @@ func (r *RootCmd) templatePull() *clibase.Cmd { dest = templateName } + clean, err := filepath.Abs(filepath.Clean(dest)) + if err != nil { + return xerrors.Errorf("cleaning destination path %s failed: %w", dest, err) + } + + dest = clean + err = os.MkdirAll(dest, 0o750) if err != nil { return xerrors.Errorf("mkdirall %q: %w", dest, err) diff --git a/cli/templatepull_test.go b/cli/templatepull_test.go index 32dd18019dacb..ec7beb619606e 100644 --- a/cli/templatepull_test.go +++ b/cli/templatepull_test.go @@ -230,118 +230,116 @@ func TestTemplatePull_LatestStdout(t *testing.T) { // ToDir tests that 'templates pull' pulls down the active template // and writes it to the correct directory. +// +// nolint: paralleltest // The subtests cannot be run in parallel; see the inner loop. func TestTemplatePull_ToDir(t *testing.T) { - t.Parallel() - - client := coderdtest.New(t, &coderdtest.Options{ - IncludeProvisionerDaemon: true, - }) - owner := coderdtest.CreateFirstUser(t, client) - templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin()) - - // Create an initial template bundle. - source1 := genTemplateVersionSource() - // Create an updated template bundle. This will be used to ensure - // that templates are correctly returned in order from latest to oldest. - source2 := genTemplateVersionSource() - - expected, err := echo.Tar(source2) - require.NoError(t, err) - - version1 := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, source1) - _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version1.ID) - - template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version1.ID) - - // Update the template version so that we can assert that templates - // are being sorted correctly. - updatedVersion := coderdtest.UpdateTemplateVersion(t, client, owner.OrganizationID, source2, template.ID) - _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, updatedVersion.ID) - coderdtest.UpdateActiveTemplateVersion(t, client, template.ID, updatedVersion.ID) - - dir := t.TempDir() - - expectedDest := filepath.Join(dir, "expected") - actualDest := filepath.Join(dir, "actual") - ctx := context.Background() - - err = extract.Tar(ctx, bytes.NewReader(expected), expectedDest, nil) - require.NoError(t, err) - - inv, root := clitest.New(t, "templates", "pull", template.Name, actualDest) - clitest.SetupConfig(t, templateAdmin, root) - - ptytest.New(t).Attach(inv) - - require.NoError(t, inv.Run()) - - require.Equal(t, - dirSum(t, expectedDest), - dirSum(t, actualDest), - ) -} - -// ToDir tests that 'templates pull' pulls down the active template and writes -// it to a directory with the name of the template if the path is not implicitly -// supplied. -// nolint: paralleltest -func TestTemplatePull_ToImplicit(t *testing.T) { - client := coderdtest.New(t, &coderdtest.Options{ - IncludeProvisionerDaemon: true, - }) - owner := coderdtest.CreateFirstUser(t, client) - templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin()) - - // Create an initial template bundle. - source1 := genTemplateVersionSource() - // Create an updated template bundle. This will be used to ensure - // that templates are correctly returned in order from latest to oldest. - source2 := genTemplateVersionSource() - - expected, err := echo.Tar(source2) - require.NoError(t, err) - - version1 := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, source1) - _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version1.ID) - - template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version1.ID) - - // Update the template version so that we can assert that templates - // are being sorted correctly. - updatedVersion := coderdtest.UpdateTemplateVersion(t, client, owner.OrganizationID, source2, template.ID) - _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, updatedVersion.ID) - coderdtest.UpdateActiveTemplateVersion(t, client, template.ID, updatedVersion.ID) - - // create a tempdir and change the working directory to it for the duration of the test (cannot run in parallel) - dir := t.TempDir() - wd, err := os.Getwd() - require.NoError(t, err) - err = os.Chdir(dir) - require.NoError(t, err) - defer func() { - err := os.Chdir(wd) - require.NoError(t, err, "if this fails, it can break other subsequent tests due to wrong working directory") - }() - - expectedDest := filepath.Join(dir, "expected") - actualDest := filepath.Join(dir, template.Name) - - ctx := context.Background() - - err = extract.Tar(ctx, bytes.NewReader(expected), expectedDest, nil) - require.NoError(t, err) - - inv, root := clitest.New(t, "templates", "pull", template.Name) - clitest.SetupConfig(t, templateAdmin, root) - - ptytest.New(t).Attach(inv) - - require.NoError(t, inv.Run()) + tests := []struct { + name string + destPath string + useDefaultDest bool + }{ + { + name: "absolute path works", + useDefaultDest: true, + }, + { + name: "relative path to specific dir is sanitized", + destPath: "./pulltmp", + }, + { + name: "relative path to current dir is sanitized", + destPath: ".", + }, + { + name: "directory traversal is acceptable", + destPath: "../mytmpl", + }, + { + name: "empty path falls back to using template name", + destPath: "", + }, + } - require.Equal(t, - dirSum(t, expectedDest), - dirSum(t, actualDest), - ) + // nolint: paralleltest // These tests change the current working dir, and is therefore unsuitable for parallelisation. + for _, tc := range tests { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + dir := t.TempDir() + + cwd, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, os.Chdir(cwd)) + }) + + // Change working directory so that relative path tests don't affect the original working directory. + newWd := filepath.Join(dir, "new-cwd") + require.NoError(t, os.MkdirAll(newWd, 0o750)) + require.NoError(t, os.Chdir(newWd)) + + expectedDest := filepath.Join(dir, "expected") + actualDest := tc.destPath + if tc.useDefaultDest { + actualDest = filepath.Join(dir, "actual") + } + + client := coderdtest.New(t, &coderdtest.Options{ + IncludeProvisionerDaemon: true, + }) + owner := coderdtest.CreateFirstUser(t, client) + templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin()) + + // Create an initial template bundle. + source1 := genTemplateVersionSource() + // Create an updated template bundle. This will be used to ensure + // that templates are correctly returned in order from latest to oldest. + source2 := genTemplateVersionSource() + + expected, err := echo.Tar(source2) + require.NoError(t, err) + + version1 := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, source1) + _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version1.ID) + + template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version1.ID) + + // Update the template version so that we can assert that templates + // are being sorted correctly. + updatedVersion := coderdtest.UpdateTemplateVersion(t, client, owner.OrganizationID, source2, template.ID) + _ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, updatedVersion.ID) + coderdtest.UpdateActiveTemplateVersion(t, client, template.ID, updatedVersion.ID) + + ctx := context.Background() + + err = extract.Tar(ctx, bytes.NewReader(expected), expectedDest, nil) + require.NoError(t, err) + + ents, _ := os.ReadDir(actualDest) + if len(ents) > 0 { + t.Logf("%s is not empty", actualDest) + t.FailNow() + } + + inv, root := clitest.New(t, "templates", "pull", template.Name, actualDest) + clitest.SetupConfig(t, templateAdmin, root) + + ptytest.New(t).Attach(inv) + + require.NoError(t, inv.Run()) + + // Validate behaviour of choosing template name in the absence of an output path argument. + destPath := actualDest + if destPath == "" { + destPath = template.Name + } + + require.Equal(t, + dirSum(t, expectedDest), + dirSum(t, destPath), + ) + }) + } } // FolderConflict tests that 'templates pull' fails when a folder with has