Merge branch 'gmh-sanitize-function' into 'main'

refactor: Moving sanitize function to utilities

See merge request https://gitlab.com/gitlab-org/cli/-/merge_requests/1432

Merged-by: Ahmed Hemdan <ahemdan@gitlab.com>
Approved-by: Patrick Bajao <ebajao@gitlab.com>
Approved-by: Ahmed Hemdan <ahemdan@gitlab.com>
Reviewed-by: Patrick Bajao <ebajao@gitlab.com>
Co-authored-by: Gary Holtz <gholtz@gitlab.com>

commands/ci/artifact/artifact.go

@@ -14,6 +14,7 @@ import (
 	"gitlab.com/gitlab-org/cli/api"
 	"gitlab.com/gitlab-org/cli/commands/cmdutils"
 	"gitlab.com/gitlab-org/cli/internal/config"
+	"gitlab.com/gitlab-org/cli/pkg/utils"
 )
 
 func ensurePathIsCreated(filename string) error {
@@ -28,15 +29,6 @@ func ensurePathIsCreated(filename string) error {
 	return nil
 }
 
-func sanitizeAssetName(asset string) string {
-	if !strings.HasPrefix(asset, "/") {
-		// Prefix the asset with "/" ensures that filepath.Clean removes all `/..`
-		// See rule 4 of filepath.Clean for more information: https://pkg.go.dev/path/filepath#Clean
-		asset = "/" + asset
-	}
-	return filepath.Clean(asset)
-}
-
 func NewCmdRun(f *cmdutils.Factory) *cobra.Command {
 	jobArtifactCmd := &cobra.Command{
 		Use:     "artifact <refName> <jobName> [flags]",
@@ -83,7 +75,7 @@ func NewCmdRun(f *cmdutils.Factory) *cobra.Command {
 			}
 
 			for _, v := range zipReader.File {
-				sanitizedAssetName := sanitizeAssetName(v.Name)
+				sanitizedAssetName := utils.SanitizePathName(v.Name)
 
 				destDir, err := filepath.Abs(path)
 				if err != nil {

pkg/utils/utils.go

@@ -3,6 +3,7 @@ package utils
 import (
 	"fmt"
 	"net/url"
+	"path/filepath"
 	"strings"
 	"time"
 
@@ -22,6 +23,15 @@ func OpenInBrowser(url, browserType string) error {
 	return run.PrepareCmd(browseCmd).Run()
 }
 
+func SanitizePathName(path string) string {
+	if !strings.HasPrefix(path, "/") {
+		// Prefix the path with "/" ensures that filepath.Clean removes all `/..`
+		// See rule 4 of filepath.Clean for more information: https://pkg.go.dev/path/filepath#Clean
+		path = "/" + path
+	}
+	return filepath.Clean(path)
+}
+
 func RenderMarkdown(text, glamourStyle string) (string, error) {
 	opts := MarkdownRenderOpts{
 		glamour.WithStylePath(getStyle(glamourStyle)),

pkg/utils/utils_test.go

@@ -110,6 +110,44 @@ func Test_PresentInIntSlice(t *testing.T) {
 	}
 }
 
+func Test_SanitizePathName(t *testing.T) {
+	tests := []struct {
+		name     string
+		filename string
+		want     string
+	}{
+		{
+			name:     "A regular filename",
+			filename: "cli-v1.22.0.json",
+			want:     "/cli-v1.22.0.json",
+		},
+		{
+			name:     "A regular filename in a directory",
+			filename: "cli/cli-v1.22.0.json",
+			want:     "/cli/cli-v1.22.0.json",
+		},
+		{
+			name:     "A filename with directory traversal",
+			filename: "cli-v1.../../22.0.zip",
+			want:     "/22.0.zip",
+		},
+		{
+			name:     "A particularly nasty filename",
+			filename: "..././..././..././etc/password_file",
+			want:     "/.../.../.../etc/password_file",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			filePathWanted := SanitizePathName(tt.filename)
+
+			if filePathWanted != tt.want {
+				t.Errorf("SanitizePathName() got = %s, want = %s", filePathWanted, tt.want)
+			}
+		})
+	}
+}
+
 func Test_CommonElementsInStringSlice(t *testing.T) {
 	testCases := []struct {
 		name   string