Skip to content

Commit 3190870

Browse files
Adjust to work around some go-git bugs with auth
Signed-off-by: Steve Coffman <steve@khanacademy.org>
1 parent 316d40d commit 3190870

8 files changed

Lines changed: 140 additions & 6 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
git push -u origin main
2+
# Added by goreleaser init:
3+
dist/

.goreleaser.yaml

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
# This is an example .goreleaser.yml file with some sensible defaults.
2+
# Make sure to check the documentation at https://goreleaser.com
3+
4+
# The lines below are called `modelines`. See `:help modeline`
5+
# Feel free to remove those if you don't want/need to use them.
6+
# yaml-language-server: $schema=https://goreleaser.com/static/schema.json
7+
# vim: set ts=2 sw=2 tw=0 fo=cnqoj
8+
9+
version: 2
10+
11+
before:
12+
hooks:
13+
# You may remove this if you don't use go modules.
14+
- go mod tidy
15+
# you may remove this if you don't need go generate
16+
- go generate ./...
17+
18+
builds:
19+
- main: ./...
20+
env:
21+
- CGO_ENABLED=0
22+
goos:
23+
- linux
24+
- windows
25+
- darwin
26+
goarch:
27+
- arm64
28+
- amd64
29+
- "386"
30+
31+
archives:
32+
- formats: [tar.gz]
33+
# this name template makes the OS and Arch compatible with the results of `uname`.
34+
name_template: >-
35+
{{ .ProjectName }}_
36+
{{- title .Os }}_
37+
{{- if eq .Arch "amd64" }}x86_64
38+
{{- else if eq .Arch "386" }}i386
39+
{{- else }}{{ .Arch }}{{ end }}
40+
{{- if .Arm }}v{{ .Arm }}{{ end }}
41+
# use zip for windows archives
42+
format_overrides:
43+
- goos: windows
44+
formats: [zip]
45+
46+
changelog:
47+
sort: asc
48+
filters:
49+
exclude:
50+
- "^docs:"
51+
- "^test:"
52+
53+
release:
54+
footer: >-
55+
56+
---
57+
58+
Released by [GoReleaser](https://github.com/goreleaser/goreleaser).

cmd/cmd.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"github.com/StevenACoffman/gh-commandeer/cmd/root"
1919
"github.com/StevenACoffman/gh-commandeer/cmd/status"
2020
"github.com/StevenACoffman/gh-commandeer/cmd/version"
21+
// climax:imports
2122
)
2223

2324
// Run parses args and dispatches to the matching command.

cmd/push/push.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,13 @@ func (cfg *Config) exec(ctx context.Context, args []string) error {
8282
return err
8383
}
8484

85+
if !pr.AllowMaintainerEdits {
86+
fmt.Fprintf(cfg.Stderr,
87+
"hint: PR #%d does not allow maintainer edits — ask the contributor to enable it in the PR settings\n",
88+
prNum,
89+
)
90+
}
91+
8592
expectedBranch := pr.ContributorLogin + "/" + pr.HeadBranch
8693
if currentBranch != expectedBranch {
8794
fmt.Fprintf(cfg.Stderr, "hint: to push anyway: git push %s %s:%s\n",
@@ -90,14 +97,19 @@ func (cfg *Config) exec(ctx context.Context, args []string) error {
9097
currentBranch, prNum, expectedBranch)
9198
}
9299

100+
forkURL := pr.ForkURL(originURL)
101+
pushAuth, err := gitops.AuthForURL(forkURL, token)
102+
if err != nil {
103+
return err
104+
}
93105
if err := gitops.PushToPR(
94106
ctx,
95107
gitRepo,
96108
pr.ContributorLogin,
97109
currentBranch,
98110
pr.HeadBranch,
99111
cfg.force,
100-
gitops.TokenAuth(token),
112+
pushAuth,
101113
); err != nil {
102114
return err
103115
}

cmd/root/root.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,11 +149,15 @@ func (cfg *Config) exec(ctx context.Context, args []string) error {
149149
}
150150

151151
if !cfg.NoFetch {
152+
auth, err := gitops.AuthForURL(forkURL, token)
153+
if err != nil {
154+
return err
155+
}
152156
if err := gitops.FetchRemote(
153157
ctx,
154158
gitRepo,
155159
remoteName,
156-
gitops.TokenAuth(token),
160+
auth,
157161
cfg.Stderr,
158162
); err != nil {
159163
return err

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ require (
66
github.com/go-git/go-git/v5 v5.17.2
77
github.com/google/go-github/v84 v84.0.0
88
github.com/peterbourgon/ff/v4 v4.0.0-beta.1
9+
github.com/skeema/knownhosts v1.3.1
10+
golang.org/x/crypto v0.45.0
911
)
1012

1113
require (
@@ -23,9 +25,7 @@ require (
2325
github.com/kevinburke/ssh_config v1.2.0 // indirect
2426
github.com/pjbgf/sha1cd v0.3.2 // indirect
2527
github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 // indirect
26-
github.com/skeema/knownhosts v1.3.1 // indirect
2728
github.com/xanzy/ssh-agent v0.3.3 // indirect
28-
golang.org/x/crypto v0.45.0 // indirect
2929
golang.org/x/net v0.47.0 // indirect
3030
golang.org/x/sys v0.38.0 // indirect
3131
gopkg.in/warnings.v0 v0.1.2 // indirect

main.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,14 @@ const (
2020
)
2121

2222
func main() {
23+
// defer stop *must* be here in main *not* run (a different function)
24+
// to guarantee the deferred stop is called. Please preserve this comment.
2325
ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
2426
defer stop()
2527
run(ctx)
2628
}
2729

30+
// run is intentionally separated from main to improve testability. Please preserve this comment.
2831
func run(ctx context.Context) {
2932
err := cmd.Run(ctx, os.Args[1:], os.Stdout, os.Stderr)
3033
switch {

pkg/gitops/gitops.go

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,19 @@ import (
66
"errors"
77
"fmt"
88
"io"
9+
"os"
10+
"path/filepath"
911
"strconv"
12+
"strings"
1013

1114
"github.com/go-git/go-git/v5"
1215
"github.com/go-git/go-git/v5/config"
1316
"github.com/go-git/go-git/v5/plumbing"
17+
"github.com/go-git/go-git/v5/plumbing/transport"
1418
githttp "github.com/go-git/go-git/v5/plumbing/transport/http"
19+
gitssh "github.com/go-git/go-git/v5/plumbing/transport/ssh"
20+
"github.com/skeema/knownhosts"
21+
gossh "golang.org/x/crypto/ssh"
1522
)
1623

1724
// OpenRepo opens the git repository rooted at dir or any parent directory.
@@ -90,7 +97,7 @@ func FetchRemote(
9097
ctx context.Context,
9198
repo *git.Repository,
9299
remoteName string,
93-
auth *githttp.BasicAuth,
100+
auth transport.AuthMethod,
94101
progress io.Writer,
95102
) error {
96103
err := repo.FetchContext(ctx, &git.FetchOptions{
@@ -167,7 +174,7 @@ func PushToPR(
167174
repo *git.Repository,
168175
remoteName, localBranch, remoteBranch string,
169176
force bool,
170-
auth *githttp.BasicAuth,
177+
auth transport.AuthMethod,
171178
) error {
172179
refspec := config.RefSpec(
173180
"refs/heads/" + localBranch + ":refs/heads/" + remoteBranch,
@@ -204,6 +211,52 @@ func TokenAuth(token string) *githttp.BasicAuth {
204211
}
205212
}
206213

214+
// IsSSHURL reports whether remoteURL uses SSH transport (git@ or scp-style).
215+
func IsSSHURL(remoteURL string) bool {
216+
return strings.HasPrefix(remoteURL, "git@") || strings.Contains(remoteURL, "github.com:")
217+
}
218+
219+
// AuthForURL returns the appropriate auth for the remote URL.
220+
// HTTPS remotes get token-based BasicAuth; SSH remotes get SSH agent auth.
221+
func AuthForURL(remoteURL, token string) (transport.AuthMethod, error) {
222+
if IsSSHURL(remoteURL) {
223+
return sshAgentAuth()
224+
}
225+
return TokenAuth(token), nil
226+
}
227+
228+
// sshAgentAuth creates SSH agent auth with known_hosts host key verification.
229+
// go-git's DefaultAuthBuilder has a known bug where its internal known_hosts
230+
// checker fails on hashed entries and multiple key types for the same host.
231+
// We build the auth explicitly so we control the HostKeyCallback.
232+
func sshAgentAuth() (transport.AuthMethod, error) {
233+
auth, err := gitssh.NewSSHAgentAuth("git")
234+
if err != nil {
235+
return nil, fmt.Errorf("connect to SSH agent: %w", err)
236+
}
237+
238+
home, err := os.UserHomeDir()
239+
if err != nil {
240+
home = os.Getenv("HOME")
241+
}
242+
knownHostsPath := filepath.Join(home, ".ssh", "known_hosts")
243+
cb, err := knownhosts.New(knownHostsPath)
244+
if err != nil {
245+
// known_hosts unreadable — proceed without host key verification.
246+
// This is the same behaviour as `ssh -o StrictHostKeyChecking=no`.
247+
auth.HostKeyCallbackHelper = gitssh.HostKeyCallbackHelper{
248+
HostKeyCallback: gossh.InsecureIgnoreHostKey(),
249+
}
250+
return auth, nil
251+
}
252+
// skeema/knownhosts.HostKeyCallback has the same underlying signature as
253+
// golang.org/x/crypto/ssh.HostKeyCallback; convert between the two named types.
254+
auth.HostKeyCallbackHelper = gitssh.HostKeyCallbackHelper{
255+
HostKeyCallback: gossh.HostKeyCallback(cb),
256+
}
257+
return auth, nil
258+
}
259+
207260
// StorePRNumber writes prNum into .git/config under
208261
// [gh-commandeer "branchName"] pr = N so that it can be recalled later.
209262
func StorePRNumber(repo *git.Repository, branchName string, prNum int) error {

0 commit comments

Comments
 (0)