Skip to content

Commit f1780e0

Browse files
SophSoph
authored
Merge pull request #65 from entireio/soph/issue-63-deferred-credential-helper
auth: defer credential helper until 401, match git's behaviour
2 parents ffe0aed + 1c9f791 commit f1780e0

9 files changed

Lines changed: 1974 additions & 92 deletions

File tree

‎cmd/git-sync/main_test.go‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"context"
66
"encoding/json"
7+
"errors"
78
"fmt"
89
"io"
910
"net/http"
@@ -14,6 +15,7 @@ import (
1415
"testing"
1516
"time"
1617

18+
"entire.io/entire/git-sync/internal/auth"
1719
"entire.io/entire/git-sync/internal/syncertest"
1820
"entire.io/entire/git-sync/unstable"
1921
billy "github.com/go-git/go-billy/v6"
@@ -29,6 +31,27 @@ import (
2931
"github.com/go-git/go-git/v6/storage/memory"
3032
)
3133

34+
// TestMain isolates the package's tests from the developer's local
35+
// credential helper. EnsureAuthForService probes /git-receive-pack with a
36+
// flush-packet POST unconditionally (required to discover cross-host
37+
// auth challenges and auth-on-POST-only gates), so without stubbing the
38+
// helper, `git credential fill` could find stored credentials for
39+
// 127.0.0.1 (e.g. cached from an earlier test run) and attach them,
40+
// changing the wire shape of the push the test under inspection.
41+
//
42+
// The probe itself still happens — receive-pack POST counts include it —
43+
// but the stub guarantees no credentials are attached and the probe
44+
// returns without further side effects on the helper.
45+
//
46+
// Tests that need to exercise helper behaviour explicitly should
47+
// restore auth.GitCredentialCommand in their own setup.
48+
func TestMain(m *testing.M) {
49+
auth.GitCredentialCommand = func(_ context.Context, _ auth.CredentialOp, _ string) ([]byte, error) {
50+
return nil, errors.New("no helper configured (test default)")
51+
}
52+
os.Exit(m.Run())
53+
}
54+
3255
const testBranch = "master"
3356
const modeReplicate = "replicate"
3457

@@ -241,8 +264,11 @@ func TestRun_Replicate_SubcommandExecutesAgainstEmptyTarget(t *testing.T) {
241264
t.Fatalf("expected relayReason=empty-target-managed-refs, got %#v", result["relayReason"])
242265
}
243266

244-
if got := targetServer.Count("git-receive-pack"); got != 1 {
245-
t.Fatalf("expected one receive-pack POST, got %d", got)
267+
// Two receive-pack POSTs: the auth-probe (a flush-packet POST that
268+
// EnsureAuthForService always sends to detect auth-on-POST-only gates and
269+
// cross-host challenges) plus the real push.
270+
if got := targetServer.Count("git-receive-pack"); got != 2 {
271+
t.Fatalf("expected two receive-pack POSTs (auth-probe + real push), got %d", got)
246272
}
247273
sourceHead, err := sourceRepo.Reference(plumbing.NewBranchReferenceName(testBranch), true)
248274
if err != nil {

‎internal/auth/auth.go‎

Lines changed: 107 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -29,25 +29,20 @@ type Endpoint struct {
2929
}
3030

3131
// Resolve resolves the auth method for the given endpoint configuration.
32-
// Order: explicit flags → Entire DB token → git credential helper → anonymous.
32+
// Order: explicit flags → Entire DB token → anonymous (with the git credential
33+
// helper deferred until the server returns 401, matching git's own behaviour).
3334
func Resolve(raw Endpoint, ep *url.URL) (Method, error) {
3435
if auth := explicitAuth(raw); auth != nil {
3536
return auth, nil
3637
}
37-
if ep == nil {
38-
return nil, nil //nolint:nilnil // nil signals no auth method found at this stage
39-
}
40-
if ep.Scheme != "http" && ep.Scheme != "https" {
38+
if !isHTTPEndpoint(ep) {
4139
return nil, nil //nolint:nilnil // nil signals no auth method found at this stage
4240
}
4341
if username, password, ok, err := LookupEntireDBCredential(raw, ep); err != nil {
4442
return nil, err // issue #7: surface refresh failure explicitly
4543
} else if ok {
4644
return &transporthttp.BasicAuth{Username: username, Password: password}, nil
4745
}
48-
if username, password, ok := lookupGitCredential(ep); ok {
49-
return &transporthttp.BasicAuth{Username: username, Password: password}, nil
50-
}
5146
return nil, nil //nolint:nilnil // nil signals no auth method found at this stage
5247
}
5348

@@ -65,39 +60,119 @@ func explicitAuth(raw Endpoint) Method {
6560
return nil
6661
}
6762

68-
// GitCredentialFillCommand is replaceable for testing.
69-
var GitCredentialFillCommand = func(ctx context.Context, input string) ([]byte, error) {
70-
cmd := exec.CommandContext(ctx, "git", "credential", "fill")
63+
// CredentialOp identifies a `git credential` subcommand.
64+
type CredentialOp string
65+
66+
const (
67+
CredentialOpFill CredentialOp = "fill"
68+
CredentialOpApprove CredentialOp = "approve"
69+
CredentialOpReject CredentialOp = "reject"
70+
)
71+
72+
// newGitCredentialCmd builds the `git credential <op>` invocation. Extracted
73+
// so tests can inspect the command's environment without exec'ing git.
74+
//
75+
// We inherit the parent environment unchanged — in particular, we do NOT
76+
// force GIT_TERMINAL_PROMPT=0. The original #63 symptom (interactive prompt
77+
// on a public-and-anonymous repo) is already prevented by Resolve no longer
78+
// invoking the helper proactively: with no 401 there's no Lookup, no
79+
// `git credential fill`, and so no prompt. Once the server actually
80+
// challenges with a 401, prompting is the right behaviour when there's a
81+
// terminal and a helper that has no entry for the host yet — same as
82+
// vanilla `git push`. Non-interactive callers (CI, daemons, the syncer
83+
// background loop) set GIT_TERMINAL_PROMPT=0 in their own environment the
84+
// same way they would for plain git, and we pass that through.
85+
func newGitCredentialCmd(ctx context.Context, op CredentialOp, input string) *exec.Cmd {
86+
cmd := exec.CommandContext(ctx, "git", "credential", string(op))
7187
cmd.Stdin = strings.NewReader(input)
72-
return cmd.Output()
88+
return cmd
89+
}
90+
91+
// GitCredentialCommand invokes `git credential <op>` with the given input
92+
// (git-credential text format). Replaceable for testing.
93+
var GitCredentialCommand = func(ctx context.Context, op CredentialOp, input string) ([]byte, error) {
94+
return newGitCredentialCmd(ctx, op, input).Output()
7395
}
7496

75-
func lookupGitCredential(ep *url.URL) (string, string, bool) {
76-
input := credentialFillInput(ep)
97+
// GitCredentialHelper bridges Git's credential helper protocol to HTTP auth.
98+
// Best-effort: a missing or misbehaving helper denies credentials rather
99+
// than failing the surrounding sync.
100+
type GitCredentialHelper struct{}
101+
102+
// Lookup queries the git credential helper for credentials for ep. Returns
103+
// ok=false if no credentials are available so the caller can surface a
104+
// clean 401. A non-nil error means the lookup itself couldn't complete
105+
// (e.g. the context was cancelled) and the caller should surface that
106+
// rather than fall back to the original 401.
107+
//
108+
// Lookup may block on user interaction when the helper falls through to a
109+
// terminal prompt (vanilla `git credential fill` behaviour). Callers that
110+
// must not block should set GIT_TERMINAL_PROMPT=0 in the process
111+
// environment; the credential subprocess inherits it. See
112+
// newGitCredentialCmd for the rationale on not forcing that ourselves.
113+
func (GitCredentialHelper) Lookup(ctx context.Context, ep *url.URL) (username, password string, ok bool, err error) {
114+
if !isHTTPEndpoint(ep) {
115+
return "", "", false, nil
116+
}
117+
input := credentialInput(ep, "", "")
77118
if input == "" {
78-
return "", "", false
119+
return "", "", false, nil
79120
}
80-
output, err := GitCredentialFillCommand(context.Background(), input)
81-
if err != nil {
82-
return "", "", false
121+
output, helperErr := GitCredentialCommand(ctx, CredentialOpFill, input)
122+
if helperErr != nil {
123+
// A cancelled or timed-out context kills the `git credential fill`
124+
// subprocess; surface that as the real cause instead of masking it
125+
// as "no credentials available", which would report the original
126+
// HTTP 401 rather than context.Canceled/DeadlineExceeded.
127+
if ctxErr := ctx.Err(); ctxErr != nil {
128+
return "", "", false, fmt.Errorf("git credential fill: %w", ctxErr)
129+
}
130+
return "", "", false, nil
83131
}
84132
values := parseCredentialOutput(output)
85-
password := values["password"]
133+
password = values["password"]
86134
if password == "" {
87-
return "", "", false
135+
return "", "", false, nil
88136
}
89-
username := values["username"]
137+
username = values["username"]
90138
if username == "" {
91139
if ep.User != nil && ep.User.Username() != "" {
92140
username = ep.User.Username()
93141
} else {
94142
username = defaultGitUsername
95143
}
96144
}
97-
return username, password, true
145+
return username, password, true, nil
98146
}
99147

100-
func credentialFillInput(ep *url.URL) string {
148+
// Approve tells the helper the credentials worked.
149+
func (h GitCredentialHelper) Approve(ctx context.Context, ep *url.URL, username, password string) {
150+
h.signal(ctx, CredentialOpApprove, ep, username, password)
151+
}
152+
153+
// Reject tells the helper the credentials failed.
154+
func (h GitCredentialHelper) Reject(ctx context.Context, ep *url.URL, username, password string) {
155+
h.signal(ctx, CredentialOpReject, ep, username, password)
156+
}
157+
158+
func (GitCredentialHelper) signal(ctx context.Context, op CredentialOp, ep *url.URL, username, password string) {
159+
input := credentialInput(ep, username, password)
160+
if input == "" {
161+
return
162+
}
163+
_, _ = GitCredentialCommand(ctx, op, input) //nolint:errcheck // advisory signal; helper failures swallowed
164+
}
165+
166+
func isHTTPEndpoint(ep *url.URL) bool {
167+
return ep != nil && (ep.Scheme == "http" || ep.Scheme == "https")
168+
}
169+
170+
// credentialInput builds a git-credential format request body for the given
171+
// endpoint. When username/password are set, they are appended (for use with
172+
// `git credential approve`/`reject`). When both are empty, the result is a
173+
// query body suitable for `git credential fill`. Explicit username overrides
174+
// any user embedded in the endpoint URL.
175+
func credentialInput(ep *url.URL, username, password string) string {
101176
if ep == nil || ep.Hostname() == "" {
102177
return ""
103178
}
@@ -106,8 +181,15 @@ func credentialFillInput(ep *url.URL) string {
106181
if path := strings.TrimPrefix(ep.Path, "/"); path != "" {
107182
fmt.Fprintf(&b, "path=%s\n", path)
108183
}
109-
if ep.User != nil && ep.User.Username() != "" {
110-
fmt.Fprintf(&b, "username=%s\n", ep.User.Username())
184+
user := username
185+
if user == "" && ep.User != nil {
186+
user = ep.User.Username()
187+
}
188+
if user != "" {
189+
fmt.Fprintf(&b, "username=%s\n", user)
190+
}
191+
if password != "" {
192+
fmt.Fprintf(&b, "password=%s\n", password)
111193
}
112194
b.WriteString("\n")
113195
return b.String()

0 commit comments

Comments
 (0)