Skip to content

Commit a6ae71a

Browse files
dannykoppingdannykopping
committed
Clean template destination path
Signed-off-by: Danny Kopping <danny@coder.com>
1 parent 5296611 commit a6ae71a

File tree

2 files changed

+83
-96
lines changed

2 files changed

+83
-96
lines changed

‎cli/templatepull.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"fmt"
66
"os"
7+
"path/filepath"
78
"sort"
89

910
"github.com/codeclysm/extract/v3"
@@ -130,6 +131,17 @@ func (r *RootCmd) templatePull() *clibase.Cmd {
130131
dest = templateName
131132
}
132133

134+
clean, err := filepath.Abs(filepath.Clean(dest))
135+
if err != nil {
136+
cliui.Error(inv.Stderr, fmt.Sprintf("cleaning destination path %s failed: %q", dest, err))
137+
return err
138+
}
139+
140+
if dest != clean {
141+
cliui.Warn(inv.Stderr, fmt.Sprintf("cleaning destination path from %s to %s", dest, clean))
142+
dest = clean
143+
}
144+
133145
err = os.MkdirAll(dest, 0o750)
134146
if err != nil {
135147
return xerrors.Errorf("mkdirall %q: %w", dest, err)

‎cli/templatepull_test.go

Lines changed: 71 additions & 96 deletions
Original file line numberDiff line numberDiff line change
@@ -230,118 +230,93 @@ func TestTemplatePull_LatestStdout(t *testing.T) {
230230

231231
// ToDir tests that 'templates pull' pulls down the active template
232232
// and writes it to the correct directory.
233+
// nolint: paralleltest
233234
func TestTemplatePull_ToDir(t *testing.T) {
234-
t.Parallel()
235-
236-
client := coderdtest.New(t, &coderdtest.Options{
237-
IncludeProvisionerDaemon: true,
238-
})
239-
owner := coderdtest.CreateFirstUser(t, client)
240-
templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin())
241-
242-
// Create an initial template bundle.
243-
source1 := genTemplateVersionSource()
244-
// Create an updated template bundle. This will be used to ensure
245-
// that templates are correctly returned in order from latest to oldest.
246-
source2 := genTemplateVersionSource()
247-
248-
expected, err := echo.Tar(source2)
249-
require.NoError(t, err)
250-
251-
version1 := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, source1)
252-
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version1.ID)
253-
254-
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version1.ID)
255-
256-
// Update the template version so that we can assert that templates
257-
// are being sorted correctly.
258-
updatedVersion := coderdtest.UpdateTemplateVersion(t, client, owner.OrganizationID, source2, template.ID)
259-
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, updatedVersion.ID)
260-
coderdtest.UpdateActiveTemplateVersion(t, client, template.ID, updatedVersion.ID)
261-
262-
dir := t.TempDir()
263-
264-
expectedDest := filepath.Join(dir, "expected")
265-
actualDest := filepath.Join(dir, "actual")
266-
ctx := context.Background()
267-
268-
err = extract.Tar(ctx, bytes.NewReader(expected), expectedDest, nil)
269-
require.NoError(t, err)
270-
271-
inv, root := clitest.New(t, "templates", "pull", template.Name, actualDest)
272-
clitest.SetupConfig(t, templateAdmin, root)
273-
274-
ptytest.New(t).Attach(inv)
235+
// Prevents the tests from running in parallel.
236+
tmp := t.TempDir()
237+
expectedDest := filepath.Join(tmp, "expected")
238+
239+
tests := []struct {
240+
name string
241+
givenPath string
242+
}{
243+
{
244+
name: "absolute path works",
245+
givenPath: filepath.Join(tmp, "actual"),
246+
},
247+
{
248+
name: "relative path is cleaned up",
249+
givenPath: "./pulltmp",
250+
},
251+
{
252+
name: "directory traversal is acceptable",
253+
givenPath: "../../../mytmpl",
254+
},
255+
{
256+
name: "empty path falls back to using template name",
257+
givenPath: "",
258+
},
259+
}
275260

276-
require.NoError(t, inv.Run())
261+
for _, tc := range tests {
262+
tc := tc
277263

278-
require.Equal(t,
279-
dirSum(t, expectedDest),
280-
dirSum(t, actualDest),
281-
)
282-
}
264+
t.Run(tc.name, func(t *testing.T) {
265+
t.Cleanup(func() {
266+
_ = os.RemoveAll(tc.givenPath)
267+
_ = os.RemoveAll(expectedDest)
268+
})
283269

284-
// ToDir tests that 'templates pull' pulls down the active template and writes
285-
// it to a directory with the name of the template if the path is not implicitly
286-
// supplied.
287-
// nolint: paralleltest
288-
func TestTemplatePull_ToImplicit(t *testing.T) {
289-
client := coderdtest.New(t, &coderdtest.Options{
290-
IncludeProvisionerDaemon: true,
291-
})
292-
owner := coderdtest.CreateFirstUser(t, client)
293-
templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin())
270+
client := coderdtest.New(t, &coderdtest.Options{
271+
IncludeProvisionerDaemon: true,
272+
})
273+
owner := coderdtest.CreateFirstUser(t, client)
274+
templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin())
294275

295-
// Create an initial template bundle.
296-
source1 := genTemplateVersionSource()
297-
// Create an updated template bundle. This will be used to ensure
298-
// that templates are correctly returned in order from latest to oldest.
299-
source2 := genTemplateVersionSource()
276+
// Create an initial template bundle.
277+
source1 := genTemplateVersionSource()
278+
// Create an updated template bundle. This will be used to ensure
279+
// that templates are correctly returned in order from latest to oldest.
280+
source2 := genTemplateVersionSource()
300281

301-
expected, err := echo.Tar(source2)
302-
require.NoError(t, err)
282+
expected, err := echo.Tar(source2)
283+
require.NoError(t, err)
303284

304-
version1 := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, source1)
305-
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version1.ID)
285+
version1 := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, source1)
286+
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version1.ID)
306287

307-
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version1.ID)
288+
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version1.ID)
308289

309-
// Update the template version so that we can assert that templates
310-
// are being sorted correctly.
311-
updatedVersion := coderdtest.UpdateTemplateVersion(t, client, owner.OrganizationID, source2, template.ID)
312-
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, updatedVersion.ID)
313-
coderdtest.UpdateActiveTemplateVersion(t, client, template.ID, updatedVersion.ID)
290+
// Update the template version so that we can assert that templates
291+
// are being sorted correctly.
292+
updatedVersion := coderdtest.UpdateTemplateVersion(t, client, owner.OrganizationID, source2, template.ID)
293+
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, updatedVersion.ID)
294+
coderdtest.UpdateActiveTemplateVersion(t, client, template.ID, updatedVersion.ID)
314295

315-
// create a tempdir and change the working directory to it for the duration of the test (cannot run in parallel)
316-
dir := t.TempDir()
317-
wd, err := os.Getwd()
318-
require.NoError(t, err)
319-
err = os.Chdir(dir)
320-
require.NoError(t, err)
321-
defer func() {
322-
err := os.Chdir(wd)
323-
require.NoError(t, err, "if this fails, it can break other subsequent tests due to wrong working directory")
324-
}()
296+
ctx := context.Background()
325297

326-
expectedDest := filepath.Join(dir, "expected")
327-
actualDest := filepath.Join(dir, template.Name)
298+
err = extract.Tar(ctx, bytes.NewReader(expected), expectedDest, nil)
299+
require.NoError(t, err)
328300

329-
ctx := context.Background()
301+
inv, root := clitest.New(t, "templates", "pull", template.Name, tc.givenPath)
302+
clitest.SetupConfig(t, templateAdmin, root)
330303

331-
err = extract.Tar(ctx, bytes.NewReader(expected), expectedDest, nil)
332-
require.NoError(t, err)
304+
ptytest.New(t).Attach(inv)
333305

334-
inv, root := clitest.New(t, "templates", "pull", template.Name)
335-
clitest.SetupConfig(t, templateAdmin, root)
336-
337-
ptytest.New(t).Attach(inv)
306+
require.NoError(t, inv.Run())
338307

339-
require.NoError(t, inv.Run())
308+
// Validate behaviour of choosing template name in the absence of an output path argument.
309+
destPath := tc.givenPath
310+
if destPath == "" {
311+
destPath = template.Name
312+
}
340313

341-
require.Equal(t,
342-
dirSum(t, expectedDest),
343-
dirSum(t, actualDest),
344-
)
314+
require.Equal(t,
315+
dirSum(t, expectedDest),
316+
dirSum(t, destPath),
317+
)
318+
})
319+
}
345320
}
346321

347322
// FolderConflict tests that 'templates pull' fails when a folder with has

0 commit comments

Comments
 (0)