Skip to content

Commit 0e94de1

Browse files
mislavmislav
committed
Address run download feedback
- With no arguments in TTY mode, prompt which artifacts to download - Change `--pattern` argument to be just `--name` and only do exact matching - For multi-archive downloads, prefix the destination path with the name of the artifact - Add tests exercising HTTP functionality - Avoid "zipslip" path injection when extracting ZIP files - Add tests for ZIP extraction
1 parent a4a41c2 commit 0e94de1

File tree

6 files changed

+399
-96
lines changed

6 files changed

+399
-96
lines changed

‎pkg/cmd/run/download/download.go‎

Lines changed: 80 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -5,25 +5,33 @@ import (
55
"fmt"
66
"path/filepath"
77

8+
"github.com/AlecAivazis/survey/v2"
89
"github.com/MakeNowJust/heredoc"
910
"github.com/cli/cli/pkg/cmdutil"
1011
"github.com/cli/cli/pkg/iostreams"
12+
"github.com/cli/cli/pkg/prompt"
13+
"github.com/cli/cli/pkg/set"
1114
"github.com/spf13/cobra"
1215
)
1316

1417
type DownloadOptions struct {
1518
IO *iostreams.IOStreams
1619
Platform platform
20+
Prompter prompter
1721

22+
DoPrompt bool
1823
RunID string
1924
DestinationDir string
20-
FilePatterns []string
25+
Names []string
2126
}
2227

2328
type platform interface {
2429
List(runID string) ([]Artifact, error)
2530
Download(url string, dir string) error
2631
}
32+
type prompter interface {
33+
Prompt(message string, options []string, result interface{}) error
34+
}
2735

2836
func NewCmdDownload(f *cmdutil.Factory, runF func(*DownloadOptions) error) *cobra.Command {
2937
opts := &DownloadOptions{
@@ -33,22 +41,29 @@ func NewCmdDownload(f *cmdutil.Factory, runF func(*DownloadOptions) error) *cobr
3341
cmd := &cobra.Command{
3442
Use: "download [<run-id>]",
3543
Short: "Download artifacts generated by a workflow run",
36-
Args: cobra.MaximumNArgs(1),
44+
Long: heredoc.Doc(`
45+
Download artifacts generated by a GitHub Actions workflow run.
46+
47+
The contents of each artifact will be extracted under separate directories based on
48+
the artifact name. If only a single artifact is specified, it will be extracted into
49+
the current directory.
50+
`),
51+
Args: cobra.MaximumNArgs(1),
3752
Example: heredoc.Doc(`
3853
# Download all artifacts generated by a workflow run
3954
$ gh run download <run-id>
4055
4156
# Download a specific artifact within a run
42-
$ gh run download <run-id> -p <name>
57+
$ gh run download <run-id> -n <name>
4358
4459
# Download specific artifacts across all runs in a repository
45-
$ gh run download -p <name1> -p <name2>
60+
$ gh run download -n <name1> -n <name2>
4661
`),
4762
RunE: func(cmd *cobra.Command, args []string) error {
4863
if len(args) > 0 {
4964
opts.RunID = args[0]
50-
} else if len(opts.FilePatterns) == 0 {
51-
return &cmdutil.FlagError{Err: errors.New("either run ID or `--pattern` is required")}
65+
} else if len(opts.Names) == 0 && opts.IO.CanPrompt() {
66+
opts.DoPrompt = true
5267
}
5368

5469
// support `-R, --repo` override
@@ -64,6 +79,7 @@ func NewCmdDownload(f *cmdutil.Factory, runF func(*DownloadOptions) error) *cobr
6479
client: httpClient,
6580
repo: baseRepo,
6681
}
82+
opts.Prompter = &surveyPrompter{}
6783

6884
if runF != nil {
6985
return runF(opts)
@@ -73,57 +89,98 @@ func NewCmdDownload(f *cmdutil.Factory, runF func(*DownloadOptions) error) *cobr
7389
}
7490

7591
cmd.Flags().StringVarP(&opts.DestinationDir, "dir", "D", ".", "The directory to download artifacts into")
76-
cmd.Flags().StringArrayVarP(&opts.FilePatterns, "pattern", "p", nil, "Download only artifacts that match a glob pattern")
92+
cmd.Flags().StringArrayVarP(&opts.Names, "name", "n", nil, "Only download artifacts that match any of the given names")
7793

7894
return cmd
7995
}
8096

8197
func runDownload(opts *DownloadOptions) error {
8298
opts.IO.StartProgressIndicator()
83-
defer opts.IO.StopProgressIndicator()
84-
8599
artifacts, err := opts.Platform.List(opts.RunID)
100+
opts.IO.StopProgressIndicator()
86101
if err != nil {
87102
return fmt.Errorf("error fetching artifacts: %w", err)
88103
}
89104

90-
// track downloaded artifacts and avoid re-downloading any of the same name
91-
downloaded := map[string]struct{}{}
92-
numArtifacts := 0
105+
numValidArtifacts := 0
106+
for _, a := range artifacts {
107+
if a.Expired {
108+
continue
109+
}
110+
numValidArtifacts++
111+
}
112+
if numValidArtifacts == 0 {
113+
return errors.New("no valid artifacts found to download")
114+
}
93115

116+
wantNames := opts.Names
117+
if opts.DoPrompt {
118+
artifactNames := set.NewStringSet()
119+
for _, a := range artifacts {
120+
if !a.Expired {
121+
artifactNames.Add(a.Name)
122+
}
123+
}
124+
options := artifactNames.ToSlice()
125+
if len(options) > 10 {
126+
options = options[:10]
127+
}
128+
err := opts.Prompter.Prompt("Select artifacts to download:", options, &wantNames)
129+
if err != nil {
130+
return err
131+
}
132+
if len(wantNames) == 0 {
133+
return errors.New("no artifacts selected")
134+
}
135+
}
136+
137+
opts.IO.StartProgressIndicator()
138+
defer opts.IO.StopProgressIndicator()
139+
140+
// track downloaded artifacts and avoid re-downloading any of the same name
141+
downloaded := set.NewStringSet()
94142
for _, a := range artifacts {
95143
if a.Expired {
96144
continue
97145
}
98-
numArtifacts++
99-
if _, found := downloaded[a.Name]; found {
146+
if downloaded.Contains(a.Name) {
100147
continue
101148
}
102-
if len(opts.FilePatterns) > 0 && !matchAny(opts.FilePatterns, a.Name) {
149+
if len(wantNames) > 0 && !matchAny(wantNames, a.Name) {
103150
continue
104151
}
105-
err := opts.Platform.Download(a.DownloadURL, opts.DestinationDir)
152+
destDir := opts.DestinationDir
153+
if len(wantNames) != 1 {
154+
destDir = filepath.Join(destDir, a.Name)
155+
}
156+
err := opts.Platform.Download(a.DownloadURL, destDir)
106157
if err != nil {
107158
return fmt.Errorf("error downloading %s: %w", a.Name, err)
108159
}
109-
downloaded[a.Name] = struct{}{}
160+
downloaded.Add(a.Name)
110161
}
111162

112-
if numArtifacts == 0 {
113-
return errors.New("no valid artifacts found to download")
114-
}
115-
if len(downloaded) == 0 {
116-
return errors.New("no artifact matches any of the patterns provided")
163+
if downloaded.Len() == 0 {
164+
return errors.New("no artifact matches any of the names provided")
117165
}
118166

119167
return nil
120168
}
121169

122170
func matchAny(patterns []string, name string) bool {
123171
for _, p := range patterns {
124-
if isMatch, err := filepath.Match(p, name); err == nil && isMatch {
172+
if name == p {
125173
return true
126174
}
127175
}
128176
return false
129177
}
178+
179+
type surveyPrompter struct{}
180+
181+
func (sp *surveyPrompter) Prompt(message string, options []string, result interface{}) error {
182+
return prompt.SurveyAskOne(&survey.MultiSelect{
183+
Message: message,
184+
Options: options,
185+
}, result)
186+
}

0 commit comments

Comments
 (0)