diff --git a/README.md b/README.md index 7ab1274..0a97709 100644 --- a/README.md +++ b/README.md @@ -43,7 +43,16 @@ the credential.helper config sequence is important, git will match against it un so organization specific token should goes first. +If a system-level config already defines a helper (for example, +`osxkeychain` on macOS), reset the inherited helper list before adding scoped +and fallback helpers. An empty `helper` resets every helper seen before it, so +its position is significant. + ```ini +# reset inherited helpers before adding the ordered helper list +[credential] + helper = + # organization specific token [credential "https://github.com/your-org/"] helper = readonly --file ~/.git-credentials-org @@ -79,6 +88,10 @@ you can also use personal and org tokens in one file: the config: ```ini +# reset inherited helpers before adding the GitHub-specific helper +[credential] + helper = + # github specific token [credential "https://github.com/"] helper = readonly --file ~/.git-credentials-org diff --git a/main.go b/main.go index d0e8d6a..8b6cfce 100644 --- a/main.go +++ b/main.go @@ -7,10 +7,10 @@ import ( "fmt" "io" "log" + "net/url" "os" "os/user" "strings" - "net/url" ) const ( @@ -100,51 +100,42 @@ func (c *credential) match(req *credential) bool { } if req.path != "" { - // get username or org from repo path like `username-or-org/reponame.git` - reqOrg, _, hasSep := strings.Cut(req.path, "/") - if hasSep && reqOrg != "" { - matchReqPath := strings.TrimRight(reqOrg, "/") - matchConfigPath := strings.TrimRight(c.path, "/") - match = match && matchReqPath == matchConfigPath - log.Printf("match path by username or org: req.path=%v,config.path=%v,result=%v", - matchReqPath, matchConfigPath, match) - } else { - match = match && c.path == req.path - log.Printf("match path: req.path=%v,other.path=%v,result=%v", - c.path, req.path, match) - } + pathMatch := matchCredentialPath(c.path, req.path) + match = match && pathMatch + log.Printf("match path: req.path=%v,config.path=%v,result=%v", + req.path, c.path, match) } return match } -func parseGitCredentialRequest(r io.Reader) (*credential, error) { - rd := bufio.NewReader(r) - req := &credential{} - for { - key, err := rd.ReadString('=') - if err != nil { - if err == io.EOF { - if key == "" { - return req, nil - } +func matchCredentialPath(configPath, requestPath string) bool { + configPath = strings.TrimRight(configPath, "/") + requestPath = strings.TrimRight(requestPath, "/") - return nil, io.ErrUnexpectedEOF - } + // Preserve the standard credential-store behavior for repository-specific + // entries before trying the owner/organization shorthand supported here. + if configPath == requestPath { + return true + } - return nil, err - } + requestOwner, _, hasSeparator := strings.Cut(requestPath, "/") + return hasSeparator && requestOwner != "" && configPath == requestOwner +} - key = strings.TrimSuffix(key, "=") - val, err := rd.ReadString('\n') - if err != nil { - if errors.Is(err, io.EOF) { - err = io.ErrUnexpectedEOF - } +func parseGitCredentialRequest(r io.Reader) (*credential, error) { + scanner := bufio.NewScanner(r) + req := &credential{} + for scanner.Scan() { + line := scanner.Text() + if line == "" { + break + } - return nil, err + key, val, found := strings.Cut(line, "=") + if !found { + return nil, errors.New("malformed credential attribute") } - val = strings.TrimSuffix(val, "\n") switch key { case "protocol": req.protocol = val @@ -158,6 +149,11 @@ func parseGitCredentialRequest(r io.Reader) (*credential, error) { req.password = val } } + if err := scanner.Err(); err != nil { + return nil, err + } + + return req, nil } func parseCredential(line string) *credential { @@ -181,31 +177,31 @@ func parseCredential(line string) *credential { // malformed line, ignore return nil } - username,err := url.QueryUnescape(credFields[0]) - if err != nil { - return nil + username, err := url.QueryUnescape(credFields[0]) + if err != nil { + return nil } - password,err := url.QueryUnescape(credFields[1]) - if err != nil { - return nil + password, err := url.QueryUnescape(credFields[1]) + if err != nil { + return nil } - + hostAndPath := fields[1] hostFields := strings.SplitN(hostAndPath, "/", 2) if len(hostFields) != 1 && len(hostFields) != 2 { // malformed line, ignore return nil } - host,err := url.QueryUnescape(hostFields[0]) + host, err := url.QueryUnescape(hostFields[0]) if err != nil { - return nil + return nil } - + var path string if len(hostFields) == 2 { - path,err = url.QueryUnescape(hostFields[1]) + path, err = url.QueryUnescape(hostFields[1]) if err != nil { - return nil + return nil } } diff --git a/main_test.go b/main_test.go index 3f904e1..ca5a0c2 100644 --- a/main_test.go +++ b/main_test.go @@ -2,40 +2,168 @@ package main import ( "fmt" - "io/ioutil" "os" + "path/filepath" + "strings" "testing" ) func TestGetCredential(t *testing.T) { - // create a temporary credential file - credFile, err := ioutil.TempFile("", "test-cred") + credFile := filepath.Join(t.TempDir(), "credentials") + credentials := []string{ + "https://john:password@github.com/foo/bar", + "https://octocat:org-password@github.com/acme", + "https://jane:password@bitbucket.org/foo/bar.git", + } + file, err := os.Create(credFile) if err != nil { t.Fatal(err) } - defer os.Remove(credFile.Name()) - - // write some credentials to the file - creds := []string{ - "https://john:password@github.com/foo/bar", - "https://jane:password@bitbucket.org/foo/bar.git", + for _, value := range credentials { + fmt.Fprintln(file, value) } - for _, cred := range creds { - fmt.Fprintln(credFile, cred) + if err := file.Close(); err != nil { + t.Fatal(err) } - // test getting a credential that exists in the file - c := getCredential(&credential{username: "john", protocol: "https", host: "github.com", path: "foo/bar"}, credFile.Name()) - if c == nil { - t.Errorf("expected to find a credential for github.com/foo/bar") + tests := []struct { + name string + request *credential + username string + password string + }{ + { + name: "full repository path", + request: &credential{protocol: "https", host: "github.com", path: "foo/bar"}, + username: "john", + password: "password", + }, + { + name: "owner path", + request: &credential{protocol: "https", host: "github.com", path: "acme/widgets.git"}, + username: "octocat", + password: "org-password", + }, } - if c.username != "john" || c.password != "password" { - t.Errorf("unexpected credential found: %+v", c) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := getCredential(tt.request, credFile) + if got == nil { + t.Fatal("expected to find a credential") + } + if got.username != tt.username || got.password != tt.password { + t.Errorf("unexpected credential: %+v", got) + } + }) } - // test getting a credential that does not exist in the file - c = getCredential(&credential{username: "john", protocol: "https", host: "bitbucket.org", path: "foo/bar"}, credFile.Name()) - if c != nil { + got := getCredential( + &credential{username: "john", protocol: "https", host: "bitbucket.org", path: "foo/bar"}, + credFile, + ) + if got != nil { t.Errorf("expected to not find a credential for bitbucket.org/foo/bar") } } + +func TestMatchCredentialPath(t *testing.T) { + tests := []struct { + name string + configPath string + requestPath string + want bool + }{ + { + name: "full repository path", + configPath: "acme/widgets.git", + requestPath: "acme/widgets.git", + want: true, + }, + { + name: "owner path", + configPath: "acme", + requestPath: "acme/widgets.git", + want: true, + }, + { + name: "trailing slash", + configPath: "acme/", + requestPath: "acme/widgets.git/", + want: true, + }, + { + name: "different repository", + configPath: "acme/widgets.git", + requestPath: "acme/gadgets.git", + want: false, + }, + { + name: "different owner", + configPath: "acme", + requestPath: "other/widgets.git", + want: false, + }, + { + name: "empty config path", + requestPath: "acme/widgets.git", + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := matchCredentialPath(tt.configPath, tt.requestPath); got != tt.want { + t.Errorf("matchCredentialPath(%q, %q) = %v, want %v", + tt.configPath, tt.requestPath, got, tt.want) + } + }) + } +} + +func TestParseGitCredentialRequest(t *testing.T) { + want := credential{ + protocol: "https", + host: "github.com", + path: "acme/widgets.git", + username: "octocat", + } + + tests := []struct { + name string + input string + }{ + { + name: "EOF terminated", + input: "protocol=https\n" + + "host=github.com\n" + + "path=acme/widgets.git\n" + + "username=octocat\n", + }, + { + name: "blank line terminated", + input: "protocol=https\n" + + "host=github.com\n" + + "path=acme/widgets.git\n" + + "username=octocat\n\nignored=value\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := parseGitCredentialRequest(strings.NewReader(tt.input)) + if err != nil { + t.Fatalf("parse credential request: %v", err) + } + if *got != want { + t.Errorf("unexpected credential request: got %+v, want %+v", *got, want) + } + }) + } +} + +func TestParseGitCredentialRequestRejectsMalformedAttribute(t *testing.T) { + _, err := parseGitCredentialRequest(strings.NewReader("protocol=https\nmalformed\n")) + if err == nil { + t.Fatal("expected malformed attribute to return an error") + } +}