From 36d622ed48d8c68d471529d1433534bd8f2f1dd2 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Tue, 20 Aug 2024 23:42:57 -0400 Subject: [PATCH 01/33] Prototype licensing workflow --- .github/licenses.tmpl | 13 +++++++++++ .github/workflows/licenses.yml | 40 ++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 .github/licenses.tmpl create mode 100644 .github/workflows/licenses.yml diff --git a/.github/licenses.tmpl b/.github/licenses.tmpl new file mode 100644 index 000000000..4a9083e92 --- /dev/null +++ b/.github/licenses.tmpl @@ -0,0 +1,13 @@ +# GitHub CLI dependencies + +The following open source dependencies are used to build the [GitHub CLI _(`gh`)_](https://github.com/cli/cli). + +## Go Packages + +Some packages may only be included on certain architectures or operating systems. + +| **Module** | **Version** | **License** | **License File** | **Package Docs** +| ---------- | ----------- | ----------- | ---------------- | ---------------- +{{- range . }} +| {{ .Name }} | {{ .Version }} | {{ .LicenseName }} | {{ .LicenseURL }} | [pkg.go.dev](https://pkg.go.dev/{{ .Name }}@{{ .Version }}) +{{- end }} diff --git a/.github/workflows/licenses.yml b/.github/workflows/licenses.yml new file mode 100644 index 000000000..1769d5bb3 --- /dev/null +++ b/.github/workflows/licenses.yml @@ -0,0 +1,40 @@ +name: Licensing +on: + push: + tags: + - "v*" +env: + GOPACKAGE: github.com/cli/cli/v2 + LICENSE_DIR: licenses + LICENSE_ARCHIVE: licenses.tgz +jobs: + go-licenses: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Set up Go + uses: actions/setup-go@v5 + with: + go-version-file: 'go.mod' + + - name: Generate Go license notices + working-directory: ${{ env.LICENSE_DIR }} + run: | + go install github.com/google/go-licenses@latest + go-licenses report "$GOPACKAGE" --template=../.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > README.md + go-licenses save "$GOPACKAGE" --stderrthreshold=ERROR --logtostderr=false + + - name: Upload Go license notices + run: | + tar czf "$LICENSE_ARCHIVE" "$LICENSE_DIR" + gh release upload "$GITHUB_REF_NAME" --clobber -- "$LICENSE_ARCHIVE" + + if gh release view "$GITHUB_REF_NAME" >/dev/null; then + echo "uploading assets to an existing release..." + gh release upload "$GITHUB_REF_NAME" --clobber -- "$LICENSE_ARCHIVE" + else + echo "cannot upload as something else should create the release first..." + exit 1 + fi From e42d44994e870516b70aa48d34ffc8bbd845e1f8 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Tue, 20 Aug 2024 23:48:55 -0400 Subject: [PATCH 02/33] Fix working directory error --- .github/workflows/licenses.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/licenses.yml b/.github/workflows/licenses.yml index 1769d5bb3..e1308f6ee 100644 --- a/.github/workflows/licenses.yml +++ b/.github/workflows/licenses.yml @@ -20,11 +20,10 @@ jobs: go-version-file: 'go.mod' - name: Generate Go license notices - working-directory: ${{ env.LICENSE_DIR }} run: | go install github.com/google/go-licenses@latest - go-licenses report "$GOPACKAGE" --template=../.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > README.md - go-licenses save "$GOPACKAGE" --stderrthreshold=ERROR --logtostderr=false + go-licenses report "$GOPACKAGE" --template=../.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > "$LICENSE_DIR"/README.md + go-licenses save "$GOPACKAGE" --stderrthreshold=ERROR --logtostderr=false --save_path "$LICENSE_DIR" - name: Upload Go license notices run: | From 67d7b0752ffa1ba76b12a5d0155f7201f6089a9e Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Tue, 20 Aug 2024 23:51:27 -0400 Subject: [PATCH 03/33] Fix working directory error --- .github/workflows/licenses.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/licenses.yml b/.github/workflows/licenses.yml index e1308f6ee..3b3b6eccd 100644 --- a/.github/workflows/licenses.yml +++ b/.github/workflows/licenses.yml @@ -22,6 +22,7 @@ jobs: - name: Generate Go license notices run: | go install github.com/google/go-licenses@latest + mkdir "$LICENSE_DIR" go-licenses report "$GOPACKAGE" --template=../.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > "$LICENSE_DIR"/README.md go-licenses save "$GOPACKAGE" --stderrthreshold=ERROR --logtostderr=false --save_path "$LICENSE_DIR" From 98294a6840e186ac6785e82f53bee38c53ba84c6 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Tue, 20 Aug 2024 23:53:44 -0400 Subject: [PATCH 04/33] Fix go-licenses package targeting --- .github/workflows/licenses.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/licenses.yml b/.github/workflows/licenses.yml index 3b3b6eccd..c6aa1f151 100644 --- a/.github/workflows/licenses.yml +++ b/.github/workflows/licenses.yml @@ -4,7 +4,6 @@ on: tags: - "v*" env: - GOPACKAGE: github.com/cli/cli/v2 LICENSE_DIR: licenses LICENSE_ARCHIVE: licenses.tgz jobs: @@ -23,8 +22,8 @@ jobs: run: | go install github.com/google/go-licenses@latest mkdir "$LICENSE_DIR" - go-licenses report "$GOPACKAGE" --template=../.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > "$LICENSE_DIR"/README.md - go-licenses save "$GOPACKAGE" --stderrthreshold=ERROR --logtostderr=false --save_path "$LICENSE_DIR" + go-licenses report ./... --template=../.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > "$LICENSE_DIR"/README.md + go-licenses save ./... --stderrthreshold=ERROR --logtostderr=false --save_path "$LICENSE_DIR" - name: Upload Go license notices run: | From df7c38f8d52835f9d5c9604eb19214f48931d052 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Tue, 20 Aug 2024 23:56:47 -0400 Subject: [PATCH 05/33] Fix Go root path issue --- .github/workflows/licenses.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/licenses.yml b/.github/workflows/licenses.yml index c6aa1f151..ba88e3cd3 100644 --- a/.github/workflows/licenses.yml +++ b/.github/workflows/licenses.yml @@ -22,8 +22,8 @@ jobs: run: | go install github.com/google/go-licenses@latest mkdir "$LICENSE_DIR" - go-licenses report ./... --template=../.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > "$LICENSE_DIR"/README.md - go-licenses save ./... --stderrthreshold=ERROR --logtostderr=false --save_path "$LICENSE_DIR" + GOROOT=$(go env GOROOT) go-licenses report ./... --template=../.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > "$LICENSE_DIR"/README.md + GOROOT=$(go env GOROOT) go-licenses save ./... --stderrthreshold=ERROR --logtostderr=false --save_path "$LICENSE_DIR" - name: Upload Go license notices run: | From 341864ec25f36481f0b198ef20907bacb32825ca Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Tue, 20 Aug 2024 23:58:44 -0400 Subject: [PATCH 06/33] Fix go-licenses template path error --- .github/workflows/licenses.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/licenses.yml b/.github/workflows/licenses.yml index ba88e3cd3..68fefacf6 100644 --- a/.github/workflows/licenses.yml +++ b/.github/workflows/licenses.yml @@ -22,7 +22,7 @@ jobs: run: | go install github.com/google/go-licenses@latest mkdir "$LICENSE_DIR" - GOROOT=$(go env GOROOT) go-licenses report ./... --template=../.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > "$LICENSE_DIR"/README.md + GOROOT=$(go env GOROOT) go-licenses report ./... --template=.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > "$LICENSE_DIR"/README.md GOROOT=$(go env GOROOT) go-licenses save ./... --stderrthreshold=ERROR --logtostderr=false --save_path "$LICENSE_DIR" - name: Upload Go license notices From 0ca266bc9a0f0e60d4a9de97c5a311be8c2e08f7 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 21 Aug 2024 00:02:58 -0400 Subject: [PATCH 07/33] Fix go-licenses ordering --- .github/workflows/licenses.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/licenses.yml b/.github/workflows/licenses.yml index 68fefacf6..62bf7b8dd 100644 --- a/.github/workflows/licenses.yml +++ b/.github/workflows/licenses.yml @@ -21,9 +21,8 @@ jobs: - name: Generate Go license notices run: | go install github.com/google/go-licenses@latest - mkdir "$LICENSE_DIR" - GOROOT=$(go env GOROOT) go-licenses report ./... --template=.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > "$LICENSE_DIR"/README.md GOROOT=$(go env GOROOT) go-licenses save ./... --stderrthreshold=ERROR --logtostderr=false --save_path "$LICENSE_DIR" + GOROOT=$(go env GOROOT) go-licenses report ./... --template=.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > "$LICENSE_DIR"/README.md - name: Upload Go license notices run: | From 23acba65f9c8642115be6455ac91e64cb199871a Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 21 Aug 2024 00:07:28 -0400 Subject: [PATCH 08/33] Fix gh token usage --- .github/workflows/licenses.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/licenses.yml b/.github/workflows/licenses.yml index 62bf7b8dd..479349714 100644 --- a/.github/workflows/licenses.yml +++ b/.github/workflows/licenses.yml @@ -25,6 +25,8 @@ jobs: GOROOT=$(go env GOROOT) go-licenses report ./... --template=.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > "$LICENSE_DIR"/README.md - name: Upload Go license notices + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | tar czf "$LICENSE_ARCHIVE" "$LICENSE_DIR" gh release upload "$GITHUB_REF_NAME" --clobber -- "$LICENSE_ARCHIVE" From 47d77bd51b589ccc1a6d588ea691401db92899a4 Mon Sep 17 00:00:00 2001 From: Andrew Feller Date: Sat, 2 Nov 2024 13:14:05 -0400 Subject: [PATCH 09/33] Add version checking when executing extensions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Building on logic from the `gh ext list` for retrieving and assessing extension release information, this commit enhances the logic around invoking extensions to check for new releases. Using the same user experience from checking `gh` version, this should only output information when the extension is used and gives the user information on how to upgrade depending on the type of extension and whether it is pinned or not. ```shell andrewfeller@Andrews-MacBook-Pro cli % gh ext install dlvhdr/gh-dash --pin v4.6.0 ✓ Installed extension dlvhdr/gh-dash ✓ Pinned extension at v4.6.0 andrewfeller@Andrews-MacBook-Pro cli % ./bin/gh dash A new release of dash is available: 4.6.0 → 4.7.0 To upgrade, run: gh extension upgrade dash --force https://github.com/dlvhdr/gh-dash ``` --- pkg/cmd/root/extension.go | 43 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/pkg/cmd/root/extension.go b/pkg/cmd/root/extension.go index d6d495103..89f0085f3 100644 --- a/pkg/cmd/root/extension.go +++ b/pkg/cmd/root/extension.go @@ -4,9 +4,11 @@ import ( "errors" "fmt" "os/exec" + "strings" "github.com/cli/cli/v2/pkg/extensions" "github.com/cli/cli/v2/pkg/iostreams" + "github.com/mgutz/ansi" "github.com/spf13/cobra" ) @@ -14,10 +16,33 @@ type ExternalCommandExitError struct { *exec.ExitError } +type extensionReleaseInfo struct { + CurrentVersion string + LatestVersion string + Pinned bool + URL string +} + func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ext extensions.Extension) *cobra.Command { + // Setup channel containing information about potential latest release info + updateMessageChan := make(chan *extensionReleaseInfo) + return &cobra.Command{ Use: ext.Name(), Short: fmt.Sprintf("Extension %s", ext.Name()), + // PreRun handles looking up whether extension has a latest version only when the command is ran. + PreRun: func(c *cobra.Command, args []string) { + go func() { + if ext.UpdateAvailable() { + updateMessageChan <- &extensionReleaseInfo{ + CurrentVersion: ext.CurrentVersion(), + LatestVersion: ext.LatestVersion(), + Pinned: ext.IsPinned(), + URL: ext.URL(), + } + } + }() + }, RunE: func(c *cobra.Command, args []string) error { args = append([]string{ext.Name()}, args...) if _, err := em.Dispatch(args, io.In, io.Out, io.ErrOut); err != nil { @@ -29,6 +54,24 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex } return nil }, + // PostRun handles communicating extension release information if found + PostRun: func(c *cobra.Command, args []string) { + releaseInfo := <-updateMessageChan + if releaseInfo != nil { + stderr := io.ErrOut + fmt.Fprintf(stderr, "\n\n%s %s → %s\n", + ansi.Color(fmt.Sprintf("A new release of %s is available:", ext.Name()), "yellow"), + ansi.Color(strings.TrimPrefix(releaseInfo.CurrentVersion, "v"), "cyan"), + ansi.Color(strings.TrimPrefix(releaseInfo.LatestVersion, "v"), "cyan")) + if releaseInfo.Pinned { + fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s --force\n", ext.Name()) + } else { + fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s\n", ext.Name()) + } + fmt.Fprintf(stderr, "%s\n\n", + ansi.Color(releaseInfo.URL, "yellow")) + } + }, GroupID: "extension", Annotations: map[string]string{ "skipAuthCheck": "true", From 6d5a26cfd194e261e11015506c357d175fa4946a Mon Sep 17 00:00:00 2001 From: Sarah Barili Date: Wed, 6 Nov 2024 14:45:41 -0700 Subject: [PATCH 10/33] adding username validation to the invoker ssh server --- internal/codespaces/rpc/invoker.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/internal/codespaces/rpc/invoker.go b/internal/codespaces/rpc/invoker.go index b9d321802..e00b6c304 100644 --- a/internal/codespaces/rpc/invoker.go +++ b/internal/codespaces/rpc/invoker.go @@ -8,6 +8,7 @@ import ( "fmt" "net" "os" + "regexp" "strconv" "strings" "time" @@ -241,6 +242,9 @@ func (i *invoker) StartSSHServerWithOptions(ctx context.Context, options StartSS return 0, "", fmt.Errorf("failed to parse SSH server port: %w", err) } + if !isUsernameValid(response.User) { + return 0, "", fmt.Errorf("invalid username: %s", response.User) + } return port, response.User, nil } @@ -300,3 +304,10 @@ func (i *invoker) notifyCodespaceOfClientActivity(ctx context.Context, activity return nil } + +func isUsernameValid(username string) bool { + // assuming valid usernames are alphanumeric, with these special characters allowed: . _ - + var validUsernamePattern = `^[a-zA-Z0-9._-]+$` + re := regexp.MustCompile(validUsernamePattern) + return re.MatchString(username) +} From ddf7287ab893b7d672e089e0eaffebf40f03471c Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 6 Nov 2024 22:49:02 -0500 Subject: [PATCH 11/33] Test extension command update behaviors This commit expands upon the previous work by creating tests around extension command execution and how various extension update scenarios are handled. Along the way, the logic handling formatting update messaging has been switched to use `ColorScheme` in order to honor color behavior flags. --- pkg/cmd/root/extension.go | 34 ++++---- pkg/cmd/root/extension_test.go | 146 +++++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+), 15 deletions(-) create mode 100644 pkg/cmd/root/extension_test.go diff --git a/pkg/cmd/root/extension.go b/pkg/cmd/root/extension.go index 89f0085f3..fcf0549c2 100644 --- a/pkg/cmd/root/extension.go +++ b/pkg/cmd/root/extension.go @@ -5,10 +5,10 @@ import ( "fmt" "os/exec" "strings" + "time" "github.com/cli/cli/v2/pkg/extensions" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/mgutz/ansi" "github.com/spf13/cobra" ) @@ -24,8 +24,8 @@ type extensionReleaseInfo struct { } func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ext extensions.Extension) *cobra.Command { - // Setup channel containing information about potential latest release info updateMessageChan := make(chan *extensionReleaseInfo) + cs := io.ColorScheme() return &cobra.Command{ Use: ext.Name(), @@ -56,20 +56,24 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex }, // PostRun handles communicating extension release information if found PostRun: func(c *cobra.Command, args []string) { - releaseInfo := <-updateMessageChan - if releaseInfo != nil { - stderr := io.ErrOut - fmt.Fprintf(stderr, "\n\n%s %s → %s\n", - ansi.Color(fmt.Sprintf("A new release of %s is available:", ext.Name()), "yellow"), - ansi.Color(strings.TrimPrefix(releaseInfo.CurrentVersion, "v"), "cyan"), - ansi.Color(strings.TrimPrefix(releaseInfo.LatestVersion, "v"), "cyan")) - if releaseInfo.Pinned { - fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s --force\n", ext.Name()) - } else { - fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s\n", ext.Name()) + select { + case releaseInfo := <-updateMessageChan: + if releaseInfo != nil { + stderr := io.ErrOut + fmt.Fprintf(stderr, "\n\n%s %s → %s\n", + cs.Yellowf("A new release of %s is available:", ext.Name()), + cs.Cyan(strings.TrimPrefix(releaseInfo.CurrentVersion, "v")), + cs.Cyan(strings.TrimPrefix(releaseInfo.LatestVersion, "v"))) + if releaseInfo.Pinned { + fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s --force\n", ext.Name()) + } else { + fmt.Fprintf(stderr, "To upgrade, run: gh extension upgrade %s\n", ext.Name()) + } + fmt.Fprintf(stderr, "%s\n\n", + cs.Yellow(releaseInfo.URL)) } - fmt.Fprintf(stderr, "%s\n\n", - ansi.Color(releaseInfo.URL, "yellow")) + case <-time.After(3 * time.Second): + // Bail on checking for new extension update as its taking too long } }, GroupID: "extension", diff --git a/pkg/cmd/root/extension_test.go b/pkg/cmd/root/extension_test.go new file mode 100644 index 000000000..e756bf504 --- /dev/null +++ b/pkg/cmd/root/extension_test.go @@ -0,0 +1,146 @@ +package root_test + +import ( + "io" + "testing" + + "github.com/MakeNowJust/heredoc" + "github.com/cli/cli/v2/pkg/cmd/root" + "github.com/cli/cli/v2/pkg/extensions" + "github.com/cli/cli/v2/pkg/iostreams" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewCmdExtension_Updates(t *testing.T) { + tests := []struct { + name string + extCurrentVersion string + extIsPinned bool + extLatestVersion string + extName string + extUpdateAvailable bool + extURL string + wantStderr string + }{ + { + name: "no update available", + extName: "no-update", + extUpdateAvailable: false, + extCurrentVersion: "1.0.0", + extLatestVersion: "1.0.0", + }, + { + name: "major update", + extName: "major-update", + extUpdateAvailable: true, + extCurrentVersion: "1.0.0", + extLatestVersion: "2.0.0", + wantStderr: heredoc.Doc(` + A new release of major-update is available: 1.0.0 → 2.0.0 + To upgrade, run: gh extension upgrade major-update + `), + }, + { + name: "major update, pinned", + extName: "major-update", + extUpdateAvailable: true, + extCurrentVersion: "1.0.0", + extLatestVersion: "2.0.0", + extIsPinned: true, + wantStderr: heredoc.Doc(` + A new release of major-update is available: 1.0.0 → 2.0.0 + To upgrade, run: gh extension upgrade major-update --force + `), + }, + { + name: "minor update", + extName: "minor-update", + extUpdateAvailable: true, + extCurrentVersion: "1.0.0", + extLatestVersion: "1.1.0", + wantStderr: heredoc.Doc(` + A new release of minor-update is available: 1.0.0 → 1.1.0 + To upgrade, run: gh extension upgrade minor-update + `), + }, + { + name: "minor update, pinned", + extName: "minor-update", + extUpdateAvailable: true, + extCurrentVersion: "1.0.0", + extLatestVersion: "1.1.0", + extIsPinned: true, + wantStderr: heredoc.Doc(` + A new release of minor-update is available: 1.0.0 → 1.1.0 + To upgrade, run: gh extension upgrade minor-update --force + `), + }, + { + name: "patch update", + extName: "patch-update", + extUpdateAvailable: true, + extCurrentVersion: "1.0.0", + extLatestVersion: "1.0.1", + wantStderr: heredoc.Doc(` + A new release of patch-update is available: 1.0.0 → 1.0.1 + To upgrade, run: gh extension upgrade patch-update + `), + }, + { + name: "patch update, pinned", + extName: "patch-update", + extUpdateAvailable: true, + extCurrentVersion: "1.0.0", + extLatestVersion: "1.0.1", + extIsPinned: true, + wantStderr: heredoc.Doc(` + A new release of patch-update is available: 1.0.0 → 1.0.1 + To upgrade, run: gh extension upgrade patch-update --force + `), + }, + } + + for _, tt := range tests { + ios, _, _, stderr := iostreams.Test() + + em := &extensions.ExtensionManagerMock{ + DispatchFunc: func(args []string, stdin io.Reader, stdout io.Writer, stderr io.Writer) (bool, error) { + // Assume extension executed / dispatched without problems as test is focused on upgrade checking. + return true, nil + }, + } + + ext := &extensions.ExtensionMock{ + CurrentVersionFunc: func() string { + return tt.extCurrentVersion + }, + IsPinnedFunc: func() bool { + return tt.extIsPinned + }, + LatestVersionFunc: func() string { + return tt.extLatestVersion + }, + NameFunc: func() string { + return tt.extName + }, + UpdateAvailableFunc: func() bool { + return tt.extUpdateAvailable + }, + URLFunc: func() string { + return tt.extURL + }, + } + + cmd := root.NewCmdExtension(ios, em, ext) + + _, err := cmd.ExecuteC() + require.NoError(t, err) + + if tt.wantStderr == "" { + assert.Emptyf(t, stderr.String(), "executing extension command should output nothing to stderr") + } else { + assert.Containsf(t, stderr.String(), tt.wantStderr, "executing extension command should output message about upgrade to stderr") + } + } +} From e629443a2c1451a6dbe6d99897a310f76b1af59e Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 6 Nov 2024 23:18:32 -0500 Subject: [PATCH 12/33] Delete .github/licenses.tmpl --- .github/licenses.tmpl | 13 ------------- 1 file changed, 13 deletions(-) delete mode 100644 .github/licenses.tmpl diff --git a/.github/licenses.tmpl b/.github/licenses.tmpl deleted file mode 100644 index 4a9083e92..000000000 --- a/.github/licenses.tmpl +++ /dev/null @@ -1,13 +0,0 @@ -# GitHub CLI dependencies - -The following open source dependencies are used to build the [GitHub CLI _(`gh`)_](https://github.com/cli/cli). - -## Go Packages - -Some packages may only be included on certain architectures or operating systems. - -| **Module** | **Version** | **License** | **License File** | **Package Docs** -| ---------- | ----------- | ----------- | ---------------- | ---------------- -{{- range . }} -| {{ .Name }} | {{ .Version }} | {{ .LicenseName }} | {{ .LicenseURL }} | [pkg.go.dev](https://pkg.go.dev/{{ .Name }}@{{ .Version }}) -{{- end }} From 54c073965934a9bd285b913ec25ab96445850c8a Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 6 Nov 2024 23:18:43 -0500 Subject: [PATCH 13/33] Delete .github/workflows/licenses.yml --- .github/workflows/licenses.yml | 40 ---------------------------------- 1 file changed, 40 deletions(-) delete mode 100644 .github/workflows/licenses.yml diff --git a/.github/workflows/licenses.yml b/.github/workflows/licenses.yml deleted file mode 100644 index 479349714..000000000 --- a/.github/workflows/licenses.yml +++ /dev/null @@ -1,40 +0,0 @@ -name: Licensing -on: - push: - tags: - - "v*" -env: - LICENSE_DIR: licenses - LICENSE_ARCHIVE: licenses.tgz -jobs: - go-licenses: - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Set up Go - uses: actions/setup-go@v5 - with: - go-version-file: 'go.mod' - - - name: Generate Go license notices - run: | - go install github.com/google/go-licenses@latest - GOROOT=$(go env GOROOT) go-licenses save ./... --stderrthreshold=ERROR --logtostderr=false --save_path "$LICENSE_DIR" - GOROOT=$(go env GOROOT) go-licenses report ./... --template=.github/licenses.tmpl --stderrthreshold=ERROR --logtostderr=false > "$LICENSE_DIR"/README.md - - - name: Upload Go license notices - env: - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - tar czf "$LICENSE_ARCHIVE" "$LICENSE_DIR" - gh release upload "$GITHUB_REF_NAME" --clobber -- "$LICENSE_ARCHIVE" - - if gh release view "$GITHUB_REF_NAME" >/dev/null; then - echo "uploading assets to an existing release..." - gh release upload "$GITHUB_REF_NAME" --clobber -- "$LICENSE_ARCHIVE" - else - echo "cannot upload as something else should create the release first..." - exit 1 - fi From ff9b6bb883d993e4a33cc583ccfc109694962b14 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 7 Nov 2024 14:39:11 -0700 Subject: [PATCH 14/33] refactor fetch attestations funcs Signed-off-by: Meredith Lancaster --- .../attestation/verification/attestation.go | 36 +++---------- pkg/cmd/attestation/verify/attestation.go | 51 +++++++++++++++++++ pkg/cmd/attestation/verify/verify.go | 33 ++---------- 3 files changed, 63 insertions(+), 57 deletions(-) create mode 100644 pkg/cmd/attestation/verify/attestation.go diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go index 0ea91c2f7..dd885a1c1 100644 --- a/pkg/cmd/attestation/verification/attestation.go +++ b/pkg/cmd/attestation/verification/attestation.go @@ -9,8 +9,8 @@ import ( "path/filepath" "github.com/cli/cli/v2/pkg/cmd/attestation/api" + "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" - "github.com/google/go-containerregistry/pkg/name" protobundle "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1" "github.com/sigstore/sigstore-go/pkg/bundle" ) @@ -21,31 +21,11 @@ var ErrUnrecognisedBundleExtension = errors.New("bundle file extension not suppo var ErrEmptyBundleFile = errors.New("provided bundle file is empty") type FetchAttestationsConfig struct { - APIClient api.Client - BundlePath string - Digest string - Limit int - Owner string - Repo string - OCIClient oci.Client - UseBundleFromRegistry bool - NameRef name.Reference -} - -func (c *FetchAttestationsConfig) IsBundleProvided() bool { - return c.BundlePath != "" -} - -func GetAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error) { - if c.IsBundleProvided() { - return GetLocalAttestations(c.BundlePath) - } - - if c.UseBundleFromRegistry { - return GetOCIAttestations(c) - } - - return GetRemoteAttestations(c) + APIClient api.Client + Digest string + Limit int + Owner string + Repo string } // GetLocalAttestations returns a slice of attestations read from a local bundle file. @@ -138,8 +118,8 @@ func GetRemoteAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error return nil, fmt.Errorf("owner or repo must be provided") } -func GetOCIAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error) { - attestations, err := c.OCIClient.GetAttestations(c.NameRef, c.Digest) +func GetOCIAttestations(client oci.Client, artifact artifact.DigestedArtifact) ([]*api.Attestation, error) { + attestations, err := client.GetAttestations(artifact.NameRef(), artifact.Digest()) if err != nil { return nil, fmt.Errorf("failed to fetch OCI attestations: %w", err) } diff --git a/pkg/cmd/attestation/verify/attestation.go b/pkg/cmd/attestation/verify/attestation.go new file mode 100644 index 000000000..a588d9764 --- /dev/null +++ b/pkg/cmd/attestation/verify/attestation.go @@ -0,0 +1,51 @@ +package verify + +import ( + "fmt" + + "github.com/cli/cli/v2/internal/text" + "github.com/cli/cli/v2/pkg/cmd/attestation/api" + "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" + "github.com/cli/cli/v2/pkg/cmd/attestation/verification" +) + +func getAttestations(o *Options, a artifact.DigestedArtifact) ([]*api.Attestation, string, error) { + if o.BundlePath != "" { + attestations, err := verification.GetLocalAttestations(o.BundlePath) + if err != nil { + msg := fmt.Sprintf("✗ Loading attestations from %s failed\n", a.URL) + return nil, msg, err + } + pluralAttestation := text.Pluralize(len(attestations), "attestation") + msg := fmt.Sprintf("Loaded %s from %s\n", pluralAttestation, o.BundlePath) + return attestations, msg, nil + } + + if o.UseBundleFromRegistry { + attestations, err := verification.GetOCIAttestations(o.OCIClient, a) + if err != nil { + msg := "✗ Loading attestations from OCI registry failed\n" + return nil, msg, err + } + pluralAttestation := text.Pluralize(len(attestations), "attestation") + msg := fmt.Sprintf("Loaded %s from %s\n", pluralAttestation, o.ArtifactPath) + return attestations, msg, nil + } + + c := verification.FetchAttestationsConfig{ + APIClient: o.APIClient, + Digest: a.DigestWithAlg(), + Limit: o.Limit, + Owner: o.Owner, + Repo: o.Repo, + } + + attestations, err := verification.GetRemoteAttestations(c) + if err != nil { + msg := "✗ Loading attestations from GitHub API failed\n" + return nil, msg, err + } + pluralAttestation := text.Pluralize(len(attestations), "attestation") + msg := fmt.Sprintf("Loaded %s from GitHub API\n", pluralAttestation) + return attestations, msg, nil +} diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 47b52bb30..e5dd12f59 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -6,7 +6,6 @@ import ( "regexp" "github.com/cli/cli/v2/internal/ghinstance" - "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/cmd/attestation/api" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact" "github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci" @@ -222,42 +221,18 @@ func runVerify(opts *Options) error { opts.Logger.Printf("Loaded digest %s for %s\n", artifact.DigestWithAlg(), artifact.URL) - c := verification.FetchAttestationsConfig{ - APIClient: opts.APIClient, - BundlePath: opts.BundlePath, - Digest: artifact.DigestWithAlg(), - Limit: opts.Limit, - Owner: opts.Owner, - Repo: opts.Repo, - OCIClient: opts.OCIClient, - UseBundleFromRegistry: opts.UseBundleFromRegistry, - NameRef: artifact.NameRef(), - } - attestations, err := verification.GetAttestations(c) + attestations, logMsg, err := getAttestations(opts, *artifact) if err != nil { if ok := errors.Is(err, api.ErrNoAttestations{}); ok { opts.Logger.Printf(opts.Logger.ColorScheme.Red("✗ No attestations found for subject %s\n"), artifact.DigestWithAlg()) return err } - if c.IsBundleProvided() { - opts.Logger.Printf(opts.Logger.ColorScheme.Red("✗ Loading attestations from %s failed\n"), artifact.URL) - } else if c.UseBundleFromRegistry { - opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Loading attestations from OCI registry failed")) - } else { - opts.Logger.Println(opts.Logger.ColorScheme.Red("✗ Loading attestations from GitHub API failed")) - } + opts.Logger.Printf(opts.Logger.ColorScheme.Red(logMsg)) return err } - - pluralAttestation := text.Pluralize(len(attestations), "attestation") - if c.IsBundleProvided() { - opts.Logger.Printf("Loaded %s from %s\n", pluralAttestation, opts.BundlePath) - } else if c.UseBundleFromRegistry { - opts.Logger.Printf("Loaded %s from %s\n", pluralAttestation, opts.ArtifactPath) - } else { - opts.Logger.Printf("Loaded %s from GitHub API\n", pluralAttestation) - } + // Print the message signifying success fetching attestations + opts.Logger.Printf(logMsg) // Apply predicate type filter to returned attestations filteredAttestations := verification.FilterAttestations(ec.PredicateType, attestations) From 8ab5f247aff3fe65a1864d2fd9d6aba704b7769e Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 7 Nov 2024 14:47:53 -0700 Subject: [PATCH 15/33] rename type Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/download/download.go | 2 +- pkg/cmd/attestation/verification/attestation.go | 4 ++-- pkg/cmd/attestation/verify/attestation.go | 2 +- pkg/cmd/attestation/verify/verify.go | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/attestation/download/download.go b/pkg/cmd/attestation/download/download.go index 77c928093..ef5a11799 100644 --- a/pkg/cmd/attestation/download/download.go +++ b/pkg/cmd/attestation/download/download.go @@ -122,7 +122,7 @@ func runDownload(opts *Options) error { opts.Logger.VerbosePrintf("Downloading trusted metadata for artifact %s\n\n", opts.ArtifactPath) - c := verification.FetchAttestationsConfig{ + c := verification.FetchRemoteAttestations{ APIClient: opts.APIClient, Digest: artifact.DigestWithAlg(), Limit: opts.Limit, diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go index dd885a1c1..bdee3a014 100644 --- a/pkg/cmd/attestation/verification/attestation.go +++ b/pkg/cmd/attestation/verification/attestation.go @@ -20,7 +20,7 @@ const SLSAPredicateV1 = "https://slsa.dev/provenance/v1" var ErrUnrecognisedBundleExtension = errors.New("bundle file extension not supported, must be json or jsonl") var ErrEmptyBundleFile = errors.New("provided bundle file is empty") -type FetchAttestationsConfig struct { +type FetchRemoteAttestations struct { APIClient api.Client Digest string Limit int @@ -96,7 +96,7 @@ func loadBundlesFromJSONLinesFile(path string) ([]*api.Attestation, error) { return attestations, nil } -func GetRemoteAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error) { +func GetRemoteAttestations(c FetchRemoteAttestations) ([]*api.Attestation, error) { if c.APIClient == nil { return nil, fmt.Errorf("api client must be provided") } diff --git a/pkg/cmd/attestation/verify/attestation.go b/pkg/cmd/attestation/verify/attestation.go index a588d9764..483d4cf97 100644 --- a/pkg/cmd/attestation/verify/attestation.go +++ b/pkg/cmd/attestation/verify/attestation.go @@ -32,7 +32,7 @@ func getAttestations(o *Options, a artifact.DigestedArtifact) ([]*api.Attestatio return attestations, msg, nil } - c := verification.FetchAttestationsConfig{ + c := verification.FetchRemoteAttestations{ APIClient: o.APIClient, Digest: a.DigestWithAlg(), Limit: o.Limit, diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index e5dd12f59..90149f7ca 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -227,7 +227,7 @@ func runVerify(opts *Options) error { opts.Logger.Printf(opts.Logger.ColorScheme.Red("✗ No attestations found for subject %s\n"), artifact.DigestWithAlg()) return err } - + // Print the message signifying failure fetching attestations opts.Logger.Printf(opts.Logger.ColorScheme.Red(logMsg)) return err } From e4cd729a7b9dc6f0394d5ab9a7a0e1af401cdc7a Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 7 Nov 2024 14:59:21 -0700 Subject: [PATCH 16/33] simplify verifyCertExtensions Signed-off-by: Meredith Lancaster --- .../attestation/verification/extensions.go | 31 ++++++++----------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/pkg/cmd/attestation/verification/extensions.go b/pkg/cmd/attestation/verification/extensions.go index e302d89c9..540c96284 100644 --- a/pkg/cmd/attestation/verification/extensions.go +++ b/pkg/cmd/attestation/verification/extensions.go @@ -20,7 +20,7 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, ec Enforcement var lastErr error for _, attestation := range results { - err := verifyCertExtensions(*attestation.VerificationResult.Signature.Certificate, ec) + err := verifyCertExtensions(*attestation.VerificationResult.Signature.Certificate, ec.Certificate) if err == nil { // if at least one attestation is verified, we're good as verification // is defined as successful if at least one attestation is verified @@ -34,28 +34,23 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, ec Enforcement return lastErr } -func verifyCertExtensions(verifiedCert certificate.Summary, criteria EnforcementCriteria) error { - sourceRepositoryOwnerURI := verifiedCert.Extensions.SourceRepositoryOwnerURI - if !strings.EqualFold(criteria.Certificate.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) { - return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", criteria.Certificate.SourceRepositoryOwnerURI, sourceRepositoryOwnerURI) +func verifyCertExtensions(verified, expected certificate.Summary) error { + if !strings.EqualFold(expected.SourceRepositoryOwnerURI, verified.SourceRepositoryOwnerURI) { + return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", expected.SourceRepositoryOwnerURI, verified.SourceRepositoryOwnerURI) } - // if repo is set, check the SourceRepositoryURI field - if criteria.Certificate.SourceRepositoryURI != "" { - sourceRepositoryURI := verifiedCert.Extensions.SourceRepositoryURI - if !strings.EqualFold(criteria.Certificate.SourceRepositoryURI, sourceRepositoryURI) { - return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", criteria.Certificate.SourceRepositoryURI, sourceRepositoryURI) - } + // if repo is set, compare the SourceRepositoryURI fields + if expected.SourceRepositoryURI != "" && !strings.EqualFold(expected.SourceRepositoryURI, verified.SourceRepositoryURI) { + return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", expected.SourceRepositoryURI, verified.SourceRepositoryURI) } - // if issuer is anything other than the default, use the user-provided value; - // otherwise, select the appropriate default based on the tenant - certIssuer := verifiedCert.Extensions.Issuer - if !strings.EqualFold(criteria.Certificate.Issuer, certIssuer) { - if strings.Index(certIssuer, criteria.Certificate.Issuer+"/") == 0 { - return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", criteria.Certificate.Issuer, certIssuer) + // compare the OIDC issuers. If not equal, return an error depending + // on if there is a partial match + if !strings.EqualFold(expected.Issuer, verified.Issuer) { + if strings.Index(verified.Issuer, expected.Issuer+"/") == 0 { + return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", expected.Issuer, verified.Issuer) } - return fmt.Errorf("expected Issuer to be %s, got %s", criteria.Certificate.Issuer, certIssuer) + return fmt.Errorf("expected Issuer to be %s, got %s", expected.Issuer, verified.Issuer) } return nil From 3ec657d087a963b11987b5950ecbe9eae9504cb4 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Thu, 7 Nov 2024 17:34:34 -0500 Subject: [PATCH 17/33] Enhance extension upgrade tests for URL --- pkg/cmd/root/extension_test.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/cmd/root/extension_test.go b/pkg/cmd/root/extension_test.go index e756bf504..ef94dcc71 100644 --- a/pkg/cmd/root/extension_test.go +++ b/pkg/cmd/root/extension_test.go @@ -29,6 +29,7 @@ func TestNewCmdExtension_Updates(t *testing.T) { extUpdateAvailable: false, extCurrentVersion: "1.0.0", extLatestVersion: "1.0.0", + extURL: "https//github.com/dne/no-update", }, { name: "major update", @@ -36,9 +37,11 @@ func TestNewCmdExtension_Updates(t *testing.T) { extUpdateAvailable: true, extCurrentVersion: "1.0.0", extLatestVersion: "2.0.0", + extURL: "https//github.com/dne/major-update", wantStderr: heredoc.Doc(` A new release of major-update is available: 1.0.0 → 2.0.0 To upgrade, run: gh extension upgrade major-update + https//github.com/dne/major-update `), }, { @@ -48,9 +51,11 @@ func TestNewCmdExtension_Updates(t *testing.T) { extCurrentVersion: "1.0.0", extLatestVersion: "2.0.0", extIsPinned: true, + extURL: "https//github.com/dne/major-update", wantStderr: heredoc.Doc(` A new release of major-update is available: 1.0.0 → 2.0.0 To upgrade, run: gh extension upgrade major-update --force + https//github.com/dne/major-update `), }, { @@ -59,9 +64,11 @@ func TestNewCmdExtension_Updates(t *testing.T) { extUpdateAvailable: true, extCurrentVersion: "1.0.0", extLatestVersion: "1.1.0", + extURL: "https//github.com/dne/minor-update", wantStderr: heredoc.Doc(` A new release of minor-update is available: 1.0.0 → 1.1.0 To upgrade, run: gh extension upgrade minor-update + https//github.com/dne/minor-update `), }, { @@ -70,10 +77,12 @@ func TestNewCmdExtension_Updates(t *testing.T) { extUpdateAvailable: true, extCurrentVersion: "1.0.0", extLatestVersion: "1.1.0", + extURL: "https//github.com/dne/minor-update", extIsPinned: true, wantStderr: heredoc.Doc(` A new release of minor-update is available: 1.0.0 → 1.1.0 To upgrade, run: gh extension upgrade minor-update --force + https//github.com/dne/minor-update `), }, { @@ -82,9 +91,11 @@ func TestNewCmdExtension_Updates(t *testing.T) { extUpdateAvailable: true, extCurrentVersion: "1.0.0", extLatestVersion: "1.0.1", + extURL: "https//github.com/dne/patch-update", wantStderr: heredoc.Doc(` A new release of patch-update is available: 1.0.0 → 1.0.1 To upgrade, run: gh extension upgrade patch-update + https//github.com/dne/patch-update `), }, { @@ -93,10 +104,12 @@ func TestNewCmdExtension_Updates(t *testing.T) { extUpdateAvailable: true, extCurrentVersion: "1.0.0", extLatestVersion: "1.0.1", + extURL: "https//github.com/dne/patch-update", extIsPinned: true, wantStderr: heredoc.Doc(` A new release of patch-update is available: 1.0.0 → 1.0.1 To upgrade, run: gh extension upgrade patch-update --force + https//github.com/dne/patch-update `), }, } From 43e5abbcd8c7691694dc9a64f28edb2d56b6cfb7 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Thu, 7 Nov 2024 15:50:46 -0700 Subject: [PATCH 18/33] use logger println method Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verify/attestation.go | 12 ++++++------ pkg/cmd/attestation/verify/verify.go | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/attestation/verify/attestation.go b/pkg/cmd/attestation/verify/attestation.go index 483d4cf97..c067afe9b 100644 --- a/pkg/cmd/attestation/verify/attestation.go +++ b/pkg/cmd/attestation/verify/attestation.go @@ -13,22 +13,22 @@ func getAttestations(o *Options, a artifact.DigestedArtifact) ([]*api.Attestatio if o.BundlePath != "" { attestations, err := verification.GetLocalAttestations(o.BundlePath) if err != nil { - msg := fmt.Sprintf("✗ Loading attestations from %s failed\n", a.URL) + msg := fmt.Sprintf("✗ Loading attestations from %s failed", a.URL) return nil, msg, err } pluralAttestation := text.Pluralize(len(attestations), "attestation") - msg := fmt.Sprintf("Loaded %s from %s\n", pluralAttestation, o.BundlePath) + msg := fmt.Sprintf("Loaded %s from %s", pluralAttestation, o.BundlePath) return attestations, msg, nil } if o.UseBundleFromRegistry { attestations, err := verification.GetOCIAttestations(o.OCIClient, a) if err != nil { - msg := "✗ Loading attestations from OCI registry failed\n" + msg := "✗ Loading attestations from OCI registry failed" return nil, msg, err } pluralAttestation := text.Pluralize(len(attestations), "attestation") - msg := fmt.Sprintf("Loaded %s from %s\n", pluralAttestation, o.ArtifactPath) + msg := fmt.Sprintf("Loaded %s from %s", pluralAttestation, o.ArtifactPath) return attestations, msg, nil } @@ -42,10 +42,10 @@ func getAttestations(o *Options, a artifact.DigestedArtifact) ([]*api.Attestatio attestations, err := verification.GetRemoteAttestations(c) if err != nil { - msg := "✗ Loading attestations from GitHub API failed\n" + msg := "✗ Loading attestations from GitHub API failed" return nil, msg, err } pluralAttestation := text.Pluralize(len(attestations), "attestation") - msg := fmt.Sprintf("Loaded %s from GitHub API\n", pluralAttestation) + msg := fmt.Sprintf("Loaded %s from GitHub API", pluralAttestation) return attestations, msg, nil } diff --git a/pkg/cmd/attestation/verify/verify.go b/pkg/cmd/attestation/verify/verify.go index 90149f7ca..1d34fdf99 100644 --- a/pkg/cmd/attestation/verify/verify.go +++ b/pkg/cmd/attestation/verify/verify.go @@ -228,11 +228,11 @@ func runVerify(opts *Options) error { return err } // Print the message signifying failure fetching attestations - opts.Logger.Printf(opts.Logger.ColorScheme.Red(logMsg)) + opts.Logger.Println(opts.Logger.ColorScheme.Red(logMsg)) return err } // Print the message signifying success fetching attestations - opts.Logger.Printf(logMsg) + opts.Logger.Println(logMsg) // Apply predicate type filter to returned attestations filteredAttestations := verification.FilterAttestations(ec.PredicateType, attestations) From a02f84528a43d7cb5e68bf7060e7b7abeecb00ee Mon Sep 17 00:00:00 2001 From: Sarah Barili <103978419+sarahbarili@users.noreply.github.com> Date: Fri, 8 Nov 2024 09:11:44 -0700 Subject: [PATCH 19/33] Update internal/codespaces/rpc/invoker.go Co-authored-by: Caleb Brose <5447118+cmbrose@users.noreply.github.com> --- internal/codespaces/rpc/invoker.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/codespaces/rpc/invoker.go b/internal/codespaces/rpc/invoker.go index e00b6c304..6ba8843ac 100644 --- a/internal/codespaces/rpc/invoker.go +++ b/internal/codespaces/rpc/invoker.go @@ -307,7 +307,7 @@ func (i *invoker) notifyCodespaceOfClientActivity(ctx context.Context, activity func isUsernameValid(username string) bool { // assuming valid usernames are alphanumeric, with these special characters allowed: . _ - - var validUsernamePattern = `^[a-zA-Z0-9._-]+$` + var validUsernamePattern = `^[a-zA-Z0-9_][-.a-zA-Z0-9_]*$` re := regexp.MustCompile(validUsernamePattern) return re.MatchString(username) } From 4a7f2e57b0c58fd2c0af5ad101d54189eb3ce805 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 13:57:05 +0100 Subject: [PATCH 20/33] Support bare repo creation --- pkg/cmd/repo/create/create.go | 8 ++- pkg/cmd/repo/create/create_test.go | 95 ++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index 2fc127aba..684ba78ba 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -748,10 +748,12 @@ func isLocalRepo(gitClient *git.Client) (bool, error) { return false, projectDirErr } } - if projectDir != ".git" { - return false, nil + + if projectDir == ".git" || projectDir == "." { + return true, nil } - return true, nil + + return false, nil } // clone the checkout branch to specified path diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index cc0ec602a..cef86db9c 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -443,6 +443,68 @@ func Test_createRun(t *testing.T) { }, wantStdout: "✓ Created repository OWNER/REPO on GitHub\n https://github.com/OWNER/REPO\n", }, + { + name: "interactive with existing bare repository public", + opts: &CreateOptions{Interactive: true}, + tty: true, + promptStubs: func(p *prompter.PrompterMock) { + p.ConfirmFunc = func(message string, defaultValue bool) (bool, error) { + switch message { + case "Add a remote?": + return false, nil + default: + return false, fmt.Errorf("unexpected confirm prompt: %s", message) + } + } + p.InputFunc = func(message, defaultValue string) (string, error) { + switch message { + case "Path to local repository": + return defaultValue, nil + case "Repository name": + return "REPO", nil + case "Description": + return "my new repo", nil + default: + return "", fmt.Errorf("unexpected input prompt: %s", message) + } + } + p.SelectFunc = func(message, defaultValue string, options []string) (int, error) { + switch message { + case "What would you like to do?": + return prompter.IndexFor(options, "Push an existing local repository to GitHub") + case "Visibility": + return prompter.IndexFor(options, "Private") + default: + return 0, fmt.Errorf("unexpected select prompt: %s", message) + } + } + }, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`query UserCurrent\b`), + httpmock.StringResponse(`{"data":{"viewer":{"login":"someuser","organizations":{"nodes": []}}}}`)) + reg.Register( + httpmock.GraphQL(`mutation RepositoryCreate\b`), + httpmock.StringResponse(` + { + "data": { + "createRepository": { + "repository": { + "id": "REPOID", + "name": "REPO", + "owner": {"login":"OWNER"}, + "url": "https://github.com/OWNER/REPO" + } + } + } + }`)) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git -C . rev-parse --git-dir`, 0, ".") + cs.Register(`git -C . rev-parse HEAD`, 0, "commithash") + }, + wantStdout: "✓ Created repository OWNER/REPO on GitHub\n https://github.com/OWNER/REPO\n", + }, { name: "interactive with existing repository public add remote and push", opts: &CreateOptions{Interactive: true}, @@ -696,6 +758,39 @@ func Test_createRun(t *testing.T) { }, wantStdout: "https://github.com/OWNER/REPO\n", }, + { + name: "noninteractive create bare from source", + opts: &CreateOptions{ + Interactive: false, + Source: ".", + Name: "REPO", + Visibility: "PRIVATE", + }, + tty: false, + httpStubs: func(reg *httpmock.Registry) { + reg.Register( + httpmock.GraphQL(`mutation RepositoryCreate\b`), + httpmock.StringResponse(` + { + "data": { + "createRepository": { + "repository": { + "id": "REPOID", + "name": "REPO", + "owner": {"login":"OWNER"}, + "url": "https://github.com/OWNER/REPO" + } + } + } + }`)) + }, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git -C . rev-parse --git-dir`, 0, ".") + cs.Register(`git -C . rev-parse HEAD`, 0, "commithash") + cs.Register(`git -C . remote add origin https://github.com/OWNER/REPO`, 0, "") + }, + wantStdout: "https://github.com/OWNER/REPO\n", + }, { name: "noninteractive clone from scratch", opts: &CreateOptions{ From bc85e11d05ec2af5e322d2e324405e6d412d1e6a Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 14:11:16 +0100 Subject: [PATCH 21/33] Backfill repo creation failure tests --- pkg/cmd/repo/create/create_test.go | 36 +++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index cef86db9c..490a976ba 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -791,6 +791,36 @@ func Test_createRun(t *testing.T) { }, wantStdout: "https://github.com/OWNER/REPO\n", }, + { + name: "noninteractive create from cwd that isn't a git repo", + opts: &CreateOptions{ + Interactive: false, + Source: ".", + Name: "REPO", + Visibility: "PRIVATE", + }, + tty: false, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git -C . rev-parse --git-dir`, 128, "") + }, + wantErr: true, + errMsg: "current directory is not a git repository. Run `git init` to initialize it", + }, + { + name: "noninteractive create from cwd that isn't a git repo", + opts: &CreateOptions{ + Interactive: false, + Source: "some-dir", + Name: "REPO", + Visibility: "PRIVATE", + }, + tty: false, + execStubs: func(cs *run.CommandStubber) { + cs.Register(`git -C some-dir rev-parse --git-dir`, 128, "") + }, + wantErr: true, + errMsg: "some-dir is not a git repository. Run `git -C \"some-dir\" init` to initialize it", + }, { name: "noninteractive clone from scratch", opts: &CreateOptions{ @@ -951,11 +981,11 @@ func Test_createRun(t *testing.T) { defer reg.Verify(t) err := createRun(tt.opts) if tt.wantErr { - assert.Error(t, err) - assert.Equal(t, tt.errMsg, err.Error()) + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errMsg) return } - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, tt.wantStdout, stdout.String()) assert.Equal(t, "", stderr.String()) }) From f515e9c1e7cc0472e3c15e73c55e6e110a204e71 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 14:12:25 +0100 Subject: [PATCH 22/33] Use errWithExitCode interface in repo create isLocalRepo --- pkg/cmd/repo/create/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index 684ba78ba..c3a16956d 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -740,7 +740,7 @@ func hasCommits(gitClient *git.Client) (bool, error) { func isLocalRepo(gitClient *git.Client) (bool, error) { projectDir, projectDirErr := gitClient.GitDir(context.Background()) if projectDirErr != nil { - var execError *exec.ExitError + var execError errWithExitCode if errors.As(projectDirErr, &execError) { if exitCode := int(execError.ExitCode()); exitCode == 128 { return false, nil From 2efb9935db2ba3429559c21a60d19c9504a1751b Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 14:18:26 +0100 Subject: [PATCH 23/33] Doc isLocalRepo and git.Client IsLocalRepo differences --- pkg/cmd/repo/create/create.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index c3a16956d..77961c987 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -736,7 +736,11 @@ func hasCommits(gitClient *git.Client) (bool, error) { return false, nil } -// check if path is the top level directory of a git repo +// Check if path is the top level directory of a git repo +// This is subtly different from the git.Client IsLocalRepo method, +// which returns true if we are _anywhere_ inside of a git repository. +// I'm not sure whether this distinction really matters for repo create, +// but I'm not confident enough right now to make that change. func isLocalRepo(gitClient *git.Client) (bool, error) { projectDir, projectDirErr := gitClient.GitDir(context.Background()) if projectDirErr != nil { From 6a97dbfadf0322cd024091efc8fc96a2a71ce23f Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 15:34:43 +0100 Subject: [PATCH 24/33] Add acceptance test for bare repo create --- .../testdata/repo/repo-create-bare.txtar | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 acceptance/testdata/repo/repo-create-bare.txtar diff --git a/acceptance/testdata/repo/repo-create-bare.txtar b/acceptance/testdata/repo/repo-create-bare.txtar new file mode 100644 index 000000000..b835c420b --- /dev/null +++ b/acceptance/testdata/repo/repo-create-bare.txtar @@ -0,0 +1,35 @@ +# It's unclear what we want to do with these acceptance tests beyond our GHEC discovery, so skip new ones by default +skip + +# Set up env var +env REPO=${SCRIPT_NAME}-${RANDOM_STRING} + +# Use gh as a credential helper +exec gh auth setup-git + +# Initialise a local repository with two branches +# We expect a bare repo to have all refs pushed with --mirror +mkdir ${REPO} +cd ${REPO} +exec git init +exec git checkout -b feature-1 +exec git commit --allow-empty -m 'Empty Commit 1' + +exec git checkout -b feature-2 +exec git commit --allow-empty -m 'Empty Commit 2' + +# Clone a bare repo from that local repo +cd .. +exec git clone --bare ${REPO} ${REPO}-bare +cd ${REPO}-bare + +# Create a GitHub repository from that bare repo +exec gh repo create ${ORG}/${REPO} --private --source . --push --remote bare + +# Defer repo cleanup +defer gh repo delete --yes ${ORG}/${REPO} + +# Check the remote repo has both branches +exec gh api /repos/${ORG}/${REPO}/branches +stdout 'feature-1' +stdout 'feature-2' From e3665955a5143325f497cdf10c2076d5e76b69c0 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 16:08:50 +0100 Subject: [PATCH 25/33] Push --mirror on bare repo create --- pkg/cmd/repo/create/create.go | 52 +++++++++++++++++++++--------- pkg/cmd/repo/create/create_test.go | 36 +++++++++++++-------- 2 files changed, 59 insertions(+), 29 deletions(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index 77961c987..ac4ef39c9 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -556,11 +556,11 @@ func createFromLocal(opts *CreateOptions) error { return err } - isRepo, err := isLocalRepo(opts.GitClient) + repoType, err := localRepoType(opts.GitClient) if err != nil { return err } - if !isRepo { + if repoType == unknown { if repoPath == "." { return fmt.Errorf("current directory is not a git repository. Run `git init` to initialize it") } @@ -659,15 +659,31 @@ func createFromLocal(opts *CreateOptions) error { } } - if opts.Push { + if opts.Push && repoType == working { err := opts.GitClient.Push(context.Background(), baseRemote, "HEAD") if err != nil { return err } + if isTTY { fmt.Fprintf(stdout, "%s Pushed commits to %s\n", cs.SuccessIcon(), remoteURL) } } + + if opts.Push && repoType == bare { + cmd, err := opts.GitClient.AuthenticatedCommand(context.Background(), "push", baseRemote, "--mirror") + if err != nil { + return err + } + if err = cmd.Run(); err != nil { + return err + } + + if isTTY { + fmt.Fprintf(stdout, "%s Mirrored all refs to %s\n", cs.SuccessIcon(), remoteURL) + } + } + return nil } @@ -736,28 +752,34 @@ func hasCommits(gitClient *git.Client) (bool, error) { return false, nil } -// Check if path is the top level directory of a git repo -// This is subtly different from the git.Client IsLocalRepo method, -// which returns true if we are _anywhere_ inside of a git repository. -// I'm not sure whether this distinction really matters for repo create, -// but I'm not confident enough right now to make that change. -func isLocalRepo(gitClient *git.Client) (bool, error) { +type repoType int + +const ( + unknown repoType = iota + working + bare +) + +func localRepoType(gitClient *git.Client) (repoType, error) { projectDir, projectDirErr := gitClient.GitDir(context.Background()) if projectDirErr != nil { var execError errWithExitCode if errors.As(projectDirErr, &execError) { if exitCode := int(execError.ExitCode()); exitCode == 128 { - return false, nil + return unknown, nil } - return false, projectDirErr + return unknown, projectDirErr } } - if projectDir == ".git" || projectDir == "." { - return true, nil + switch projectDir { + case ".": + return bare, nil + case ".git": + return working, nil + default: + return unknown, nil } - - return false, nil } // clone the checkout branch to specified path diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index 490a976ba..a0f9ac207 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -444,14 +444,16 @@ func Test_createRun(t *testing.T) { wantStdout: "✓ Created repository OWNER/REPO on GitHub\n https://github.com/OWNER/REPO\n", }, { - name: "interactive with existing bare repository public", + name: "interactive with existing bare repository public and push", opts: &CreateOptions{Interactive: true}, tty: true, promptStubs: func(p *prompter.PrompterMock) { p.ConfirmFunc = func(message string, defaultValue bool) (bool, error) { switch message { case "Add a remote?": - return false, nil + return true, nil + case `Would you like to push commits from the current branch to "origin"?`: + return true, nil default: return false, fmt.Errorf("unexpected confirm prompt: %s", message) } @@ -464,6 +466,8 @@ func Test_createRun(t *testing.T) { return "REPO", nil case "Description": return "my new repo", nil + case "What should the new remote be called?": + return defaultValue, nil default: return "", fmt.Errorf("unexpected input prompt: %s", message) } @@ -502,8 +506,10 @@ func Test_createRun(t *testing.T) { execStubs: func(cs *run.CommandStubber) { cs.Register(`git -C . rev-parse --git-dir`, 0, ".") cs.Register(`git -C . rev-parse HEAD`, 0, "commithash") + cs.Register(`git -C . remote add origin https://github.com/OWNER/REPO`, 0, "") + cs.Register(`git -C . push origin --mirror`, 0, "") }, - wantStdout: "✓ Created repository OWNER/REPO on GitHub\n https://github.com/OWNER/REPO\n", + wantStdout: "✓ Created repository OWNER/REPO on GitHub\n https://github.com/OWNER/REPO\n✓ Added remote https://github.com/OWNER/REPO.git\n✓ Mirrored all refs to https://github.com/OWNER/REPO.git\n", }, { name: "interactive with existing repository public add remote and push", @@ -759,10 +765,11 @@ func Test_createRun(t *testing.T) { wantStdout: "https://github.com/OWNER/REPO\n", }, { - name: "noninteractive create bare from source", + name: "noninteractive create bare from source and push", opts: &CreateOptions{ Interactive: false, Source: ".", + Push: true, Name: "REPO", Visibility: "PRIVATE", }, @@ -771,23 +778,24 @@ func Test_createRun(t *testing.T) { reg.Register( httpmock.GraphQL(`mutation RepositoryCreate\b`), httpmock.StringResponse(` - { - "data": { - "createRepository": { - "repository": { - "id": "REPOID", - "name": "REPO", - "owner": {"login":"OWNER"}, - "url": "https://github.com/OWNER/REPO" + { + "data": { + "createRepository": { + "repository": { + "id": "REPOID", + "name": "REPO", + "owner": {"login":"OWNER"}, + "url": "https://github.com/OWNER/REPO" + } } } - } - }`)) + }`)) }, execStubs: func(cs *run.CommandStubber) { cs.Register(`git -C . rev-parse --git-dir`, 0, ".") cs.Register(`git -C . rev-parse HEAD`, 0, "commithash") cs.Register(`git -C . remote add origin https://github.com/OWNER/REPO`, 0, "") + cs.Register(`git -C . push origin --mirror`, 0, "") }, wantStdout: "https://github.com/OWNER/REPO\n", }, From 8e63268aba2c643771c6427c31abce21ad4fe95a Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 16:10:32 +0100 Subject: [PATCH 26/33] Doc push behaviour for bare repo create --- pkg/cmd/repo/create/create.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index ac4ef39c9..de99c4bc3 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -94,7 +94,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co To create a remote repository from an existing local repository, specify the source directory with %[1]s--source%[1]s. By default, the remote repository name will be the name of the source directory. - Pass %[1]s--push%[1]s to push any local commits to the new repository. + Pass %[1]s--push%[1]s to push any local commits to the new repository. If the repo is bare, this will mirror all refs. For language or platform .gitignore templates to use with %[1]s--gitignore%[1]s, . From 7bcb0633914e635692fe7a1e31d96d713bf48392 Mon Sep 17 00:00:00 2001 From: William Martin Date: Mon, 11 Nov 2024 16:17:06 +0100 Subject: [PATCH 27/33] Modify push prompt on repo create when bare --- pkg/cmd/repo/create/create.go | 7 ++++++- pkg/cmd/repo/create/create_test.go | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/cmd/repo/create/create.go b/pkg/cmd/repo/create/create.go index de99c4bc3..79c349aa4 100644 --- a/pkg/cmd/repo/create/create.go +++ b/pkg/cmd/repo/create/create.go @@ -652,8 +652,13 @@ func createFromLocal(opts *CreateOptions) error { // don't prompt for push if there are no commits if opts.Interactive && committed { + msg := fmt.Sprintf("Would you like to push commits from the current branch to %q?", baseRemote) + if repoType == bare { + msg = fmt.Sprintf("Would you like to mirror all refs to %q?", baseRemote) + } + var err error - opts.Push, err = opts.Prompter.Confirm(fmt.Sprintf("Would you like to push commits from the current branch to %q?", baseRemote), true) + opts.Push, err = opts.Prompter.Confirm(msg, true) if err != nil { return err } diff --git a/pkg/cmd/repo/create/create_test.go b/pkg/cmd/repo/create/create_test.go index a0f9ac207..c33cfdad6 100644 --- a/pkg/cmd/repo/create/create_test.go +++ b/pkg/cmd/repo/create/create_test.go @@ -452,7 +452,7 @@ func Test_createRun(t *testing.T) { switch message { case "Add a remote?": return true, nil - case `Would you like to push commits from the current branch to "origin"?`: + case `Would you like to mirror all refs to "origin"?`: return true, nil default: return false, fmt.Errorf("unexpected confirm prompt: %s", message) From b8ef951de1cff45e56c1847bd0a2ed08e9173e60 Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Wed, 13 Nov 2024 13:04:01 -0500 Subject: [PATCH 28/33] Shorten extension release checking from 3s to 1s Addressing feedback from extension author demonstration about a noticable pause waiting for extension execution to complete due to amount of time waiting on channel. --- pkg/cmd/root/extension.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/root/extension.go b/pkg/cmd/root/extension.go index fcf0549c2..52250a432 100644 --- a/pkg/cmd/root/extension.go +++ b/pkg/cmd/root/extension.go @@ -72,7 +72,7 @@ func NewCmdExtension(io *iostreams.IOStreams, em extensions.ExtensionManager, ex fmt.Fprintf(stderr, "%s\n\n", cs.Yellow(releaseInfo.URL)) } - case <-time.After(3 * time.Second): + case <-time.After(1 * time.Second): // Bail on checking for new extension update as its taking too long } }, From d4262f818386ba4eea60ebdbb1951bf95c284a9f Mon Sep 17 00:00:00 2001 From: Andy Feller Date: Thu, 14 Nov 2024 10:31:36 -0500 Subject: [PATCH 29/33] Mention GitHub CLI team on discussion issues --- .github/workflows/triage.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/triage.yml b/.github/workflows/triage.yml index 6cd9d981d..849beebad 100644 --- a/.github/workflows/triage.yml +++ b/.github/workflows/triage.yml @@ -35,6 +35,8 @@ jobs: --- + cc: @github/cli + > $BODY EOF @@ -63,5 +65,7 @@ jobs: --- + cc: @github/cli + > $BODY EOF From c518a3b1f57d179b40eb40991741720b344e5d61 Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Mon, 18 Nov 2024 08:18:04 -0700 Subject: [PATCH 30/33] Update pkg/cmd/attestation/verification/extensions.go Co-authored-by: Phill MV --- pkg/cmd/attestation/verification/extensions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/attestation/verification/extensions.go b/pkg/cmd/attestation/verification/extensions.go index 540c96284..130a301e3 100644 --- a/pkg/cmd/attestation/verification/extensions.go +++ b/pkg/cmd/attestation/verification/extensions.go @@ -34,7 +34,7 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, ec Enforcement return lastErr } -func verifyCertExtensions(verified, expected certificate.Summary) error { +func verifyCertExtensions(given, expected certificate.Summary) error { if !strings.EqualFold(expected.SourceRepositoryOwnerURI, verified.SourceRepositoryOwnerURI) { return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", expected.SourceRepositoryOwnerURI, verified.SourceRepositoryOwnerURI) } From 762e99d151c3c5e3927ad7c34aa7d758a59b05ce Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Mon, 18 Nov 2024 08:19:07 -0700 Subject: [PATCH 31/33] fix function param calls Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/verification/extensions.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/cmd/attestation/verification/extensions.go b/pkg/cmd/attestation/verification/extensions.go index 130a301e3..a0827e9ec 100644 --- a/pkg/cmd/attestation/verification/extensions.go +++ b/pkg/cmd/attestation/verification/extensions.go @@ -35,22 +35,22 @@ func VerifyCertExtensions(results []*AttestationProcessingResult, ec Enforcement } func verifyCertExtensions(given, expected certificate.Summary) error { - if !strings.EqualFold(expected.SourceRepositoryOwnerURI, verified.SourceRepositoryOwnerURI) { - return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", expected.SourceRepositoryOwnerURI, verified.SourceRepositoryOwnerURI) + if !strings.EqualFold(expected.SourceRepositoryOwnerURI, given.SourceRepositoryOwnerURI) { + return fmt.Errorf("expected SourceRepositoryOwnerURI to be %s, got %s", expected.SourceRepositoryOwnerURI, given.SourceRepositoryOwnerURI) } // if repo is set, compare the SourceRepositoryURI fields - if expected.SourceRepositoryURI != "" && !strings.EqualFold(expected.SourceRepositoryURI, verified.SourceRepositoryURI) { - return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", expected.SourceRepositoryURI, verified.SourceRepositoryURI) + if expected.SourceRepositoryURI != "" && !strings.EqualFold(expected.SourceRepositoryURI, given.SourceRepositoryURI) { + return fmt.Errorf("expected SourceRepositoryURI to be %s, got %s", expected.SourceRepositoryURI, given.SourceRepositoryURI) } // compare the OIDC issuers. If not equal, return an error depending // on if there is a partial match - if !strings.EqualFold(expected.Issuer, verified.Issuer) { - if strings.Index(verified.Issuer, expected.Issuer+"/") == 0 { - return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", expected.Issuer, verified.Issuer) + if !strings.EqualFold(expected.Issuer, given.Issuer) { + if strings.Index(given.Issuer, expected.Issuer+"/") == 0 { + return fmt.Errorf("expected Issuer to be %s, got %s -- if you have a custom OIDC issuer policy for your enterprise, use the --cert-oidc-issuer flag with your expected issuer", expected.Issuer, given.Issuer) } - return fmt.Errorf("expected Issuer to be %s, got %s", expected.Issuer, verified.Issuer) + return fmt.Errorf("expected Issuer to be %s, got %s", expected.Issuer, given.Issuer) } return nil From 30ae1388e4e4010f3b02620506638d813f3c884d Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Mon, 18 Nov 2024 08:19:41 -0700 Subject: [PATCH 32/33] Update pkg/cmd/attestation/download/download.go Co-authored-by: Phill MV --- pkg/cmd/attestation/download/download.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/attestation/download/download.go b/pkg/cmd/attestation/download/download.go index ef5a11799..a79af5935 100644 --- a/pkg/cmd/attestation/download/download.go +++ b/pkg/cmd/attestation/download/download.go @@ -122,7 +122,7 @@ func runDownload(opts *Options) error { opts.Logger.VerbosePrintf("Downloading trusted metadata for artifact %s\n\n", opts.ArtifactPath) - c := verification.FetchRemoteAttestations{ + c := verification.FetchRemoteAttestationsParams{ APIClient: opts.APIClient, Digest: artifact.DigestWithAlg(), Limit: opts.Limit, From 63f37eb36996fbf496673052386be215b14b992d Mon Sep 17 00:00:00 2001 From: Meredith Lancaster Date: Mon, 18 Nov 2024 08:24:25 -0700 Subject: [PATCH 33/33] pr feedback Signed-off-by: Meredith Lancaster --- pkg/cmd/attestation/download/download.go | 13 +++++---- .../attestation/verification/attestation.go | 27 +++++++++---------- pkg/cmd/attestation/verify/attestation.go | 13 +++++---- 3 files changed, 25 insertions(+), 28 deletions(-) diff --git a/pkg/cmd/attestation/download/download.go b/pkg/cmd/attestation/download/download.go index a79af5935..143912308 100644 --- a/pkg/cmd/attestation/download/download.go +++ b/pkg/cmd/attestation/download/download.go @@ -122,14 +122,13 @@ func runDownload(opts *Options) error { opts.Logger.VerbosePrintf("Downloading trusted metadata for artifact %s\n\n", opts.ArtifactPath) - c := verification.FetchRemoteAttestationsParams{ - APIClient: opts.APIClient, - Digest: artifact.DigestWithAlg(), - Limit: opts.Limit, - Owner: opts.Owner, - Repo: opts.Repo, + params := verification.FetchRemoteAttestationsParams{ + Digest: artifact.DigestWithAlg(), + Limit: opts.Limit, + Owner: opts.Owner, + Repo: opts.Repo, } - attestations, err := verification.GetRemoteAttestations(c) + attestations, err := verification.GetRemoteAttestations(opts.APIClient, params) if err != nil { if errors.Is(err, api.ErrNoAttestations{}) { fmt.Fprintf(opts.Logger.IO.Out, "No attestations found for %s\n", opts.ArtifactPath) diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go index bdee3a014..07083a5c0 100644 --- a/pkg/cmd/attestation/verification/attestation.go +++ b/pkg/cmd/attestation/verification/attestation.go @@ -20,12 +20,11 @@ const SLSAPredicateV1 = "https://slsa.dev/provenance/v1" var ErrUnrecognisedBundleExtension = errors.New("bundle file extension not supported, must be json or jsonl") var ErrEmptyBundleFile = errors.New("provided bundle file is empty") -type FetchRemoteAttestations struct { - APIClient api.Client - Digest string - Limit int - Owner string - Repo string +type FetchRemoteAttestationsParams struct { + Digest string + Limit int + Owner string + Repo string } // GetLocalAttestations returns a slice of attestations read from a local bundle file. @@ -96,22 +95,22 @@ func loadBundlesFromJSONLinesFile(path string) ([]*api.Attestation, error) { return attestations, nil } -func GetRemoteAttestations(c FetchRemoteAttestations) ([]*api.Attestation, error) { - if c.APIClient == nil { +func GetRemoteAttestations(client api.Client, params FetchRemoteAttestationsParams) ([]*api.Attestation, error) { + if client == nil { return nil, fmt.Errorf("api client must be provided") } // check if Repo is set first because if Repo has been set, Owner will be set using the value of Repo. // If Repo is not set, the field will remain empty. It will not be populated using the value of Owner. - if c.Repo != "" { - attestations, err := c.APIClient.GetByRepoAndDigest(c.Repo, c.Digest, c.Limit) + if params.Repo != "" { + attestations, err := client.GetByRepoAndDigest(params.Repo, params.Digest, params.Limit) if err != nil { - return nil, fmt.Errorf("failed to fetch attestations from %s: %w", c.Repo, err) + return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Repo, err) } return attestations, nil - } else if c.Owner != "" { - attestations, err := c.APIClient.GetByOwnerAndDigest(c.Owner, c.Digest, c.Limit) + } else if params.Owner != "" { + attestations, err := client.GetByOwnerAndDigest(params.Owner, params.Digest, params.Limit) if err != nil { - return nil, fmt.Errorf("failed to fetch attestations from %s: %w", c.Owner, err) + return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Owner, err) } return attestations, nil } diff --git a/pkg/cmd/attestation/verify/attestation.go b/pkg/cmd/attestation/verify/attestation.go index c067afe9b..f3f2792c4 100644 --- a/pkg/cmd/attestation/verify/attestation.go +++ b/pkg/cmd/attestation/verify/attestation.go @@ -32,15 +32,14 @@ func getAttestations(o *Options, a artifact.DigestedArtifact) ([]*api.Attestatio return attestations, msg, nil } - c := verification.FetchRemoteAttestations{ - APIClient: o.APIClient, - Digest: a.DigestWithAlg(), - Limit: o.Limit, - Owner: o.Owner, - Repo: o.Repo, + params := verification.FetchRemoteAttestationsParams{ + Digest: a.DigestWithAlg(), + Limit: o.Limit, + Owner: o.Owner, + Repo: o.Repo, } - attestations, err := verification.GetRemoteAttestations(c) + attestations, err := verification.GetRemoteAttestations(o.APIClient, params) if err != nil { msg := "✗ Loading attestations from GitHub API failed" return nil, msg, err