From 50efd9af681cf0d1d933662cfd8f8ac87f8f604b Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Fri, 6 Mar 2026 18:37:27 -0800 Subject: [PATCH 01/15] refactor: use parsed *url.URL types instead of string endpoints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - config struct: unexport fields, replace Endpoint/Proxy strings with endpointURL/proxyURL (*url.URL) and proxyPath, parse once in readConfig - api.ClientOpts: Endpoint string → EndpointURL *url.URL - login: accept *url.URL, move endpoint conflict check to handler - code_intel_upload: simplify makeCodeIntelUploadURL with stack copy, remove error return - api.go: use JoinPath for URL construction instead of string concat - Update all call sites and tests Amp-Thread-ID: https://ampcode.com/threads/T-019cc5da-fb05-71f8-962b-689cfc5b494d Co-authored-by: Amp --- cmd/src/batch_common.go | 4 +- cmd/src/batch_remote.go | 2 +- cmd/src/batch_repositories.go | 2 +- cmd/src/code_intel_upload.go | 28 +-- cmd/src/debug_compose.go | 2 +- cmd/src/debug_kube.go | 2 +- cmd/src/debug_server.go | 2 +- cmd/src/login.go | 34 ++- cmd/src/login_oauth.go | 16 +- cmd/src/login_test.go | 66 +++--- cmd/src/login_validate.go | 25 +-- cmd/src/main.go | 127 +++++++---- cmd/src/main_test.go | 235 ++++++++++++--------- cmd/src/search.go | 2 +- cmd/src/search_jobs.go | 4 +- cmd/src/search_jobs_logs.go | 2 +- cmd/src/search_jobs_results.go | 2 +- cmd/src/search_stream.go | 6 +- cmd/src/search_stream_test.go | 4 +- cmd/src/users_prune.go | 4 +- internal/api/api.go | 11 +- internal/batches/executor/executor_test.go | 7 +- internal/batches/repozip/fetcher_test.go | 16 +- 23 files changed, 327 insertions(+), 276 deletions(-) diff --git a/cmd/src/batch_common.go b/cmd/src/batch_common.go index 21fdce2e21..e294347dcf 100644 --- a/cmd/src/batch_common.go +++ b/cmd/src/batch_common.go @@ -537,7 +537,7 @@ func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) (err error if err != nil { return execUI.CreatingBatchSpecError(lr.MaxUnlicensedChangesets, err) } - previewURL := cfg.Endpoint + url + previewURL := cfg.endpointURL.JoinPath(url).String() execUI.CreatingBatchSpecSuccess(previewURL) hasWorkspaceFiles := false @@ -567,7 +567,7 @@ func executeBatchSpec(ctx context.Context, opts executeBatchSpecOpts) (err error if err != nil { return err } - execUI.ApplyingBatchSpecSuccess(cfg.Endpoint + batch.URL) + execUI.ApplyingBatchSpecSuccess(cfg.endpointURL.JoinPath(batch.URL).String()) return nil } diff --git a/cmd/src/batch_remote.go b/cmd/src/batch_remote.go index 7dd7628c03..86ad3650d1 100644 --- a/cmd/src/batch_remote.go +++ b/cmd/src/batch_remote.go @@ -157,7 +157,7 @@ Examples: executionURL := fmt.Sprintf( "%s/%s/batch-changes/%s/executions/%s", - strings.TrimSuffix(cfg.Endpoint, "/"), + cfg.endpointURL, strings.TrimPrefix(namespace.URL, "/"), batchChangeName, batchSpecID, diff --git a/cmd/src/batch_repositories.go b/cmd/src/batch_repositories.go index b02a4f5e58..97a7ea6cfe 100644 --- a/cmd/src/batch_repositories.go +++ b/cmd/src/batch_repositories.go @@ -131,7 +131,7 @@ Examples: Max: max, RepoCount: len(repos), Repos: repos, - SourcegraphEndpoint: cfg.Endpoint, + SourcegraphEndpoint: cfg.endpointURL.String(), }); err != nil { return err } diff --git a/cmd/src/code_intel_upload.go b/cmd/src/code_intel_upload.go index 303e2199e0..f110eace50 100644 --- a/cmd/src/code_intel_upload.go +++ b/cmd/src/code_intel_upload.go @@ -7,7 +7,6 @@ import ( "flag" "fmt" "io" - "net/url" "os" "strings" "time" @@ -87,10 +86,7 @@ func handleCodeIntelUpload(args []string) error { return handleUploadError(uploadOptions.SourcegraphInstanceOptions.AccessToken, err) } - uploadURL, err := makeCodeIntelUploadURL(uploadID) - if err != nil { - return err - } + uploadURL := makeCodeIntelUploadURL(uploadID) if codeintelUploadFlags.json { serialized, err := json.Marshal(map[string]any{ @@ -132,7 +128,7 @@ func codeintelUploadOptions(out *output.Output) upload.UploadOptions { associatedIndexID = &codeintelUploadFlags.associatedIndexID } - cfg.AdditionalHeaders["Content-Type"] = "application/x-protobuf+scip" + cfg.additionalHeaders["Content-Type"] = "application/x-protobuf+scip" logger := upload.NewRequestLogger( os.Stdout, @@ -153,9 +149,9 @@ func codeintelUploadOptions(out *output.Output) upload.UploadOptions { AssociatedIndexID: associatedIndexID, }, SourcegraphInstanceOptions: upload.SourcegraphInstanceOptions{ - SourcegraphURL: cfg.Endpoint, - AccessToken: cfg.AccessToken, - AdditionalHeaders: cfg.AdditionalHeaders, + SourcegraphURL: cfg.endpointURL.String(), + AccessToken: cfg.accessToken, + AdditionalHeaders: cfg.additionalHeaders, MaxRetries: 5, RetryInterval: time.Second, Path: codeintelUploadFlags.uploadRoute, @@ -191,16 +187,12 @@ func printInferredArguments(out *output.Output) { // makeCodeIntelUploadURL constructs a URL to the upload with the given internal identifier. // The base of the URL is constructed from the configured Sourcegraph instance. -func makeCodeIntelUploadURL(uploadID int) (string, error) { - url, err := url.Parse(cfg.Endpoint) - if err != nil { - return "", err - } - +func makeCodeIntelUploadURL(uploadID int) string { + u := *cfg.endpointURL graphqlID := base64.URLEncoding.EncodeToString(fmt.Appendf(nil, `SCIPUpload:%d`, uploadID)) - url.Path = codeintelUploadFlags.repo + "/-/code-intelligence/uploads/" + graphqlID - url.User = nil - return url.String(), nil + u.Path = codeintelUploadFlags.repo + "/-/code-intelligence/uploads/" + graphqlID + u.User = nil + return u.String() } type errorWithHint struct { diff --git a/cmd/src/debug_compose.go b/cmd/src/debug_compose.go index 8e26d95b04..32c746065d 100644 --- a/cmd/src/debug_compose.go +++ b/cmd/src/debug_compose.go @@ -75,7 +75,7 @@ Examples: return errors.Wrap(err, "failed to get containers for subcommand with err") } // Safety check user knows what they are targeting with this debug command - log.Printf("This command will archive docker-cli data for %d containers\n SRC_ENDPOINT: %v\n Output filename: %v", len(containers), cfg.Endpoint, base) + log.Printf("This command will archive docker-cli data for %d containers\n SRC_ENDPOINT: %v\n Output filename: %v", len(containers), cfg.endpointURL, base) if verified, _ := verify("Do you want to start writing to an archive?"); !verified { return nil } diff --git a/cmd/src/debug_kube.go b/cmd/src/debug_kube.go index 69af7571e9..24f0b955c0 100644 --- a/cmd/src/debug_kube.go +++ b/cmd/src/debug_kube.go @@ -84,7 +84,7 @@ Examples: return errors.Wrapf(err, "failed to get current-context") } // Safety check user knows what they've targeted with this command - log.Printf("Archiving kubectl data for %d pods\n SRC_ENDPOINT: %v\n Context: %s Namespace: %v\n Output filename: %v", len(pods.Items), cfg.Endpoint, kubectx, namespace, base) + log.Printf("Archiving kubectl data for %d pods\n SRC_ENDPOINT: %v\n Context: %s Namespace: %v\n Output filename: %v", len(pods.Items), cfg.endpointURL, kubectx, namespace, base) if verified, _ := verify("Do you want to start writing to an archive?"); !verified { return nil } diff --git a/cmd/src/debug_server.go b/cmd/src/debug_server.go index 8ef59fc02a..d219ec7fdb 100644 --- a/cmd/src/debug_server.go +++ b/cmd/src/debug_server.go @@ -72,7 +72,7 @@ Examples: defer zw.Close() // Safety check user knows what they are targeting with this debug command - log.Printf("This command will archive docker-cli data for container: %s\n SRC_ENDPOINT: %s\n Output filename: %s", container, cfg.Endpoint, base) + log.Printf("This command will archive docker-cli data for container: %s\n SRC_ENDPOINT: %s\n Output filename: %s", container, cfg.endpointURL, base) if verified, _ := verify("Do you want to start writing to an archive?"); !verified { return nil } diff --git a/cmd/src/login.go b/cmd/src/login.go index c4989605f4..068879ef0d 100644 --- a/cmd/src/login.go +++ b/cmd/src/login.go @@ -54,12 +54,16 @@ Examples: if err := flagSet.Parse(args); err != nil { return err } - endpoint := cfg.Endpoint + if flagSet.NArg() >= 1 { - endpoint = flagSet.Arg(0) - } - if endpoint == "" { - return cmderrors.Usage("expected exactly one argument: the Sourcegraph URL, or SRC_ENDPOINT to be set") + arg := flagSet.Arg(0) + parsed, err := parseEndpoint(arg) + if err != nil { + return cmderrors.Usage(fmt.Sprintf("invalid endpoint URL: %s", arg)) + } + if parsed.String() != cfg.endpointURL.String() { + return cmderrors.Usage(fmt.Sprintf("The configured endpoint is %s, not %s", cfg.endpointURL, parsed)) + } } client := cfg.apiClient(apiFlags, io.Discard) @@ -67,7 +71,6 @@ Examples: return loginCmd(context.Background(), loginParams{ cfg: cfg, client: client, - endpoint: endpoint, out: os.Stdout, useOAuth: *useOAuth, apiFlags: apiFlags, @@ -85,7 +88,6 @@ Examples: type loginParams struct { cfg *config client api.Client - endpoint string out io.Writer useOAuth bool apiFlags *api.Flags @@ -99,16 +101,15 @@ type loginFlowKind int const ( loginFlowOAuth loginFlowKind = iota loginFlowMissingAuth - loginFlowEndpointConflict loginFlowValidate ) var loadStoredOAuthToken = oauth.LoadToken func loginCmd(ctx context.Context, p loginParams) error { - if p.cfg.ConfigFilePath != "" { + if p.cfg.configFilePath != "" { fmt.Fprintln(p.out) - fmt.Fprintf(p.out, "⚠️ Warning: Configuring src with a JSON file is deprecated. Please migrate to using the env vars SRC_ENDPOINT, SRC_ACCESS_TOKEN, and SRC_PROXY instead, and then remove %s. See https://github.com/sourcegraph/src-cli#readme for more information.\n", p.cfg.ConfigFilePath) + fmt.Fprintf(p.out, "⚠️ Warning: Configuring src with a JSON file is deprecated. Please migrate to using the env vars SRC_ENDPOINT, SRC_ACCESS_TOKEN, and SRC_PROXY instead, and then remove %s. See https://github.com/sourcegraph/src-cli#readme for more information.\n", p.cfg.configFilePath) } _, flow := selectLoginFlow(ctx, p) @@ -117,28 +118,23 @@ func loginCmd(ctx context.Context, p loginParams) error { // selectLoginFlow decides what login flow to run based on flags and config. func selectLoginFlow(ctx context.Context, p loginParams) (loginFlowKind, loginFlow) { - endpointArg := cleanEndpoint(p.endpoint) - if p.useOAuth { return loginFlowOAuth, runOAuthLogin } - if !hasEffectiveAuth(ctx, p.cfg, endpointArg) { + if !hasEffectiveAuth(ctx, p.cfg) { return loginFlowMissingAuth, runMissingAuthLogin } - if endpointArg != p.cfg.Endpoint { - return loginFlowEndpointConflict, runEndpointConflictLogin - } return loginFlowValidate, runValidatedLogin } // hasEffectiveAuth determines whether we have auth credentials to continue. It first checks for a resolved Access Token in // config, then it checks for a stored OAuth token. -func hasEffectiveAuth(ctx context.Context, cfg *config, resolvedEndpoint string) bool { - if cfg.AccessToken != "" { +func hasEffectiveAuth(ctx context.Context, cfg *config) bool { + if cfg.accessToken != "" { return true } - if _, err := loadStoredOAuthToken(ctx, resolvedEndpoint); err == nil { + if _, err := loadStoredOAuthToken(ctx, cfg.endpointURL.String()); err == nil { return true } diff --git a/cmd/src/login_oauth.go b/cmd/src/login_oauth.go index df3f912f62..279e19e64c 100644 --- a/cmd/src/login_oauth.go +++ b/cmd/src/login_oauth.go @@ -14,15 +14,15 @@ import ( ) func runOAuthLogin(ctx context.Context, p loginParams) error { - endpointArg := cleanEndpoint(p.endpoint) - client, err := oauthLoginClient(ctx, p, endpointArg) + endpoint := p.cfg.endpointURL.String() + client, err := oauthLoginClient(ctx, p, endpoint) if err != nil { printLoginProblem(p.out, fmt.Sprintf("OAuth Device flow authentication failed: %s", err)) - fmt.Fprintln(p.out, loginAccessTokenMessage(endpointArg)) + fmt.Fprintln(p.out, loginAccessTokenMessage(endpoint)) return cmderrors.ExitCode1 } - if err := validateCurrentUser(ctx, client, p.out, endpointArg); err != nil { + if err := validateCurrentUser(ctx, client, p.out, endpoint); err != nil { return err } @@ -44,12 +44,12 @@ func oauthLoginClient(ctx context.Context, p loginParams, endpoint string) (api. } return api.NewClient(api.ClientOpts{ - Endpoint: p.cfg.Endpoint, - AdditionalHeaders: p.cfg.AdditionalHeaders, + EndpointURL: p.cfg.endpointURL, + AdditionalHeaders: p.cfg.additionalHeaders, Flags: p.apiFlags, Out: p.out, - ProxyURL: p.cfg.ProxyURL, - ProxyPath: p.cfg.ProxyPath, + ProxyURL: p.cfg.proxyURL, + ProxyPath: p.cfg.proxyPath, OAuthToken: token, }), nil } diff --git a/cmd/src/login_test.go b/cmd/src/login_test.go index a34288576f..e9e0a7943c 100644 --- a/cmd/src/login_test.go +++ b/cmd/src/login_test.go @@ -7,6 +7,7 @@ import ( "io" "net/http" "net/http/httptest" + "net/url" "strings" "testing" @@ -15,7 +16,7 @@ import ( ) func TestLogin(t *testing.T) { - check := func(t *testing.T, cfg *config, endpointArg string) (output string, err error) { + check := func(t *testing.T, cfg *config) (output string, err error) { t.Helper() restoreStoredOAuthLoader(t, func(context.Context, string) (*oauth.Token, error) { @@ -26,26 +27,15 @@ func TestLogin(t *testing.T) { err = loginCmd(context.Background(), loginParams{ cfg: cfg, client: cfg.apiClient(nil, io.Discard), - endpoint: endpointArg, out: &out, oauthClient: oauth.NewClient(oauth.DefaultClientID), }) return strings.TrimSpace(out.String()), err } - t.Run("different endpoint in config vs. arg", func(t *testing.T) { - out, err := check(t, &config{Endpoint: "https://example.com"}, "https://sourcegraph.example.com") - if err != cmderrors.ExitCode1 { - t.Fatal(err) - } - wantOut := "❌ Problem: No access token is configured.\n\n🛠 To fix: Create an access token by going to https://sourcegraph.example.com/user/settings/tokens, then set the following environment variables in your terminal:\n\n export SRC_ENDPOINT=https://sourcegraph.example.com\n export SRC_ACCESS_TOKEN=(your access token)\n\n To verify that it's working, run the login command again.\n\n Alternatively, you can try logging in using OAuth by running: src login --oauth https://sourcegraph.example.com" - if out != wantOut { - t.Errorf("got output %q, want %q", out, wantOut) - } - }) - t.Run("no access token", func(t *testing.T) { - out, err := check(t, &config{Endpoint: "https://example.com"}, "https://sourcegraph.example.com") + endpoint := &url.URL{Scheme: "https", Host: "sourcegraph.example.com"} + out, err := check(t, &config{endpointURL: endpoint}) if err != cmderrors.ExitCode1 { t.Fatal(err) } @@ -56,7 +46,8 @@ func TestLogin(t *testing.T) { }) t.Run("warning when using config file", func(t *testing.T) { - out, err := check(t, &config{Endpoint: "https://example.com", ConfigFilePath: "f"}, "https://example.com") + endpoint := &url.URL{Scheme: "https", Host: "example.com"} + out, err := check(t, &config{endpointURL: endpoint, configFilePath: "f"}) if err != cmderrors.ExitCode1 { t.Fatal(err) } @@ -72,13 +63,13 @@ func TestLogin(t *testing.T) { })) defer s.Close() - endpoint := s.URL - out, err := check(t, &config{Endpoint: endpoint, AccessToken: "x"}, endpoint) + u, _ := url.ParseRequestURI(s.URL) + out, err := check(t, &config{endpointURL: u, accessToken: "x"}) if err != cmderrors.ExitCode1 { t.Fatal(err) } wantOut := "❌ Problem: Invalid access token.\n\n🛠 To fix: Create an access token by going to $ENDPOINT/user/settings/tokens, then set the following environment variables in your terminal:\n\n export SRC_ENDPOINT=$ENDPOINT\n export SRC_ACCESS_TOKEN=(your access token)\n\n To verify that it's working, run the login command again.\n\n Alternatively, you can try logging in using OAuth by running: src login --oauth $ENDPOINT\n\n (If you need to supply custom HTTP request headers, see information about SRC_HEADER_* and SRC_HEADERS env vars at https://github.com/sourcegraph/src-cli/blob/main/AUTH_PROXY.md)" - wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", endpoint) + wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", s.URL) if out != wantOut { t.Errorf("got output %q, want %q", out, wantOut) } @@ -90,13 +81,13 @@ func TestLogin(t *testing.T) { })) defer s.Close() - endpoint := s.URL - out, err := check(t, &config{Endpoint: endpoint, AccessToken: "x"}, endpoint) + u, _ := url.ParseRequestURI(s.URL) + out, err := check(t, &config{endpointURL: u, accessToken: "x"}) if err != nil { t.Fatal(err) } - wantOut := "✔︎ Authenticated as alice on $ENDPOINT" - wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", endpoint) + wantOut := "✔️ Authenticated as alice on $ENDPOINT" + wantOut = strings.ReplaceAll(wantOut, "$ENDPOINT", s.URL) if out != wantOut { t.Errorf("got output %q, want %q", out, wantOut) } @@ -104,14 +95,21 @@ func TestLogin(t *testing.T) { } func TestSelectLoginFlow(t *testing.T) { + mustParseURL := func(s string) *url.URL { + u, err := url.Parse(s) + if err != nil { + t.Fatal(err) + } + return u + } + restoreStoredOAuthLoader(t, func(context.Context, string) (*oauth.Token, error) { return nil, fmt.Errorf("not found") }) t.Run("uses oauth flow when oauth flag is set", func(t *testing.T) { params := loginParams{ - cfg: &config{Endpoint: "https://example.com"}, - endpoint: "https://example.com", + cfg: &config{endpointURL: mustParseURL("https://example.com")}, useOAuth: true, } @@ -122,8 +120,7 @@ func TestSelectLoginFlow(t *testing.T) { t.Run("uses missing auth flow when auth is unavailable", func(t *testing.T) { params := loginParams{ - cfg: &config{Endpoint: "https://example.com"}, - endpoint: "https://sourcegraph.example.com", + cfg: &config{endpointURL: mustParseURL("https://example.com")}, } if got, _ := selectLoginFlow(context.Background(), params); got != loginFlowMissingAuth { @@ -131,21 +128,9 @@ func TestSelectLoginFlow(t *testing.T) { } }) - t.Run("uses endpoint conflict flow when auth exists for a different endpoint", func(t *testing.T) { - params := loginParams{ - cfg: &config{Endpoint: "https://example.com", AccessToken: "x"}, - endpoint: "https://sourcegraph.example.com", - } - - if got, _ := selectLoginFlow(context.Background(), params); got != loginFlowEndpointConflict { - t.Fatalf("flow = %v, want %v", got, loginFlowEndpointConflict) - } - }) - t.Run("uses validation flow when auth exists for the selected endpoint", func(t *testing.T) { params := loginParams{ - cfg: &config{Endpoint: "https://example.com", AccessToken: "x"}, - endpoint: "https://example.com", + cfg: &config{endpointURL: mustParseURL("https://example.com"), accessToken: "x"}, } if got, _ := selectLoginFlow(context.Background(), params); got != loginFlowValidate { @@ -159,8 +144,7 @@ func TestSelectLoginFlow(t *testing.T) { }) params := loginParams{ - cfg: &config{Endpoint: "https://example.com"}, - endpoint: "https://example.com", + cfg: &config{endpointURL: mustParseURL("https://example.com")}, } if got, _ := selectLoginFlow(context.Background(), params); got != loginFlowValidate { diff --git a/cmd/src/login_validate.go b/cmd/src/login_validate.go index 1a0ddfa050..d2753589b7 100644 --- a/cmd/src/login_validate.go +++ b/cmd/src/login_validate.go @@ -11,28 +11,19 @@ import ( ) func runMissingAuthLogin(_ context.Context, p loginParams) error { - endpointArg := cleanEndpoint(p.endpoint) + endpoint := p.cfg.endpointURL.String() fmt.Fprintln(p.out) printLoginProblem(p.out, "No access token is configured.") - fmt.Fprintln(p.out, loginAccessTokenMessage(endpointArg)) - return cmderrors.ExitCode1 -} - -func runEndpointConflictLogin(_ context.Context, p loginParams) error { - endpointArg := cleanEndpoint(p.endpoint) - - fmt.Fprintln(p.out) - printLoginProblem(p.out, fmt.Sprintf("The configured endpoint is %s, not %s.", p.cfg.Endpoint, endpointArg)) - fmt.Fprintln(p.out, loginAccessTokenMessage(endpointArg)) + fmt.Fprintln(p.out, loginAccessTokenMessage(endpoint)) return cmderrors.ExitCode1 } func runValidatedLogin(ctx context.Context, p loginParams) error { - return validateCurrentUser(ctx, p.client, p.out, cleanEndpoint(p.endpoint)) + return validateCurrentUser(ctx, p.client, p.out, p.cfg.endpointURL.String()) } -func validateCurrentUser(ctx context.Context, client api.Client, out io.Writer, endpointArg string) error { +func validateCurrentUser(ctx context.Context, client api.Client, out io.Writer, endpoint string) error { query := `query CurrentUser { currentUser { username } }` var result struct { CurrentUser *struct{ Username string } @@ -41,9 +32,9 @@ func validateCurrentUser(ctx context.Context, client api.Client, out io.Writer, if strings.HasPrefix(err.Error(), "error: 401 Unauthorized") || strings.HasPrefix(err.Error(), "error: 403 Forbidden") { printLoginProblem(out, "Invalid access token.") } else { - printLoginProblem(out, fmt.Sprintf("Error communicating with %s: %s", endpointArg, err)) + printLoginProblem(out, fmt.Sprintf("Error communicating with %s: %s", endpoint, err)) } - fmt.Fprintln(out, loginAccessTokenMessage(endpointArg)) + fmt.Fprintln(out, loginAccessTokenMessage(endpoint)) fmt.Fprintln(out, " (If you need to supply custom HTTP request headers, see information about SRC_HEADER_* and SRC_HEADERS env vars at https://github.com/sourcegraph/src-cli/blob/main/AUTH_PROXY.md)") return cmderrors.ExitCode1 } @@ -51,11 +42,11 @@ func validateCurrentUser(ctx context.Context, client api.Client, out io.Writer, if result.CurrentUser == nil { // This should never happen; we verified there is an access token, so there should always be // a user. - printLoginProblem(out, fmt.Sprintf("Unable to determine user on %s.", endpointArg)) + printLoginProblem(out, fmt.Sprintf("Unable to determine user on %s.", endpoint)) return cmderrors.ExitCode1 } fmt.Fprintln(out) - fmt.Fprintf(out, "✔︎ Authenticated as %s on %s\n", result.CurrentUser.Username, endpointArg) + fmt.Fprintf(out, "✔️ Authenticated as %s on %s\n", result.CurrentUser.Username, endpoint) fmt.Fprintln(out) return nil } diff --git a/cmd/src/main.go b/cmd/src/main.go index 41e5c55cd0..51e128b043 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -4,9 +4,11 @@ import ( "context" "encoding/json" "flag" + "fmt" "io" "log" "net" + "net/http" "net/url" "os" "path/filepath" @@ -109,34 +111,47 @@ func normalizeDashHelp(args []string) []string { return args } +func parseEndpoint(endpoint string) (*url.URL, error) { + u, err := url.ParseRequestURI(strings.TrimSuffix(endpoint, "/")) + if err != nil { + return nil, err + } + if !(u.Scheme == "http" || u.Scheme == "https") { + return nil, errors.Newf("Invalid scheme %s. Require http or https", u.Scheme) + } + if u.Host == "" { + return nil, errors.Newf("Empty host") + } + return u, nil +} + var cfg *config -// config represents the config format. +// config holds the resolved configuration used at runtime. type config struct { - Endpoint string `json:"endpoint"` - AccessToken string `json:"accessToken"` - AdditionalHeaders map[string]string `json:"additionalHeaders"` - Proxy string `json:"proxy"` - ProxyURL *url.URL - ProxyPath string - ConfigFilePath string + accessToken string + additionalHeaders map[string]string + proxyURL *url.URL + proxyPath string + configFilePath string + endpointURL *url.URL } // apiClient returns an api.Client built from the configuration. func (c *config) apiClient(flags *api.Flags, out io.Writer) api.Client { opts := api.ClientOpts{ - Endpoint: c.Endpoint, - AccessToken: c.AccessToken, - AdditionalHeaders: c.AdditionalHeaders, + EndpointURL: c.endpointURL, + AccessToken: c.accessToken, + AdditionalHeaders: c.additionalHeaders, Flags: flags, Out: out, - ProxyURL: c.ProxyURL, - ProxyPath: c.ProxyPath, + ProxyURL: c.proxyURL, + ProxyPath: c.proxyPath, } // Only use OAuth if we do not have SRC_ACCESS_TOKEN set - if c.AccessToken == "" { - if t, err := oauth.LoadToken(context.Background(), c.Endpoint); err == nil { + if c.accessToken == "" { + if t, err := oauth.LoadToken(context.Background(), c.endpointURL.String()); err == nil { opts.OAuthToken = t } } @@ -146,14 +161,14 @@ func (c *config) apiClient(flags *api.Flags, out io.Writer) api.Client { // readConfig reads the config file from the given path. func readConfig() (*config, error) { - cfgFile := *configPath + cfgFilePath := *configPath userSpecified := *configPath != "" if !userSpecified { - cfgFile = "~/src-config.json" + cfgFilePath = "~/src-config.json" } - cfgPath, err := expandHomeDir(cfgFile) + cfgPath, err := expandHomeDir(cfgFilePath) if err != nil { return nil, err } @@ -162,12 +177,24 @@ func readConfig() (*config, error) { if err != nil && (!os.IsNotExist(err) || userSpecified) { return nil, err } + var cfgFile struct { + Endpoint string `json:"endpoint"` + AccessToken string `json:"accessToken"` + AdditionalHeaders map[string]string `json:"additionalHeaders"` + Proxy string `json:"proxy"` + } var cfg config + var endpointStr string + var proxyStr string if err == nil { - cfg.ConfigFilePath = cfgPath - if err := json.Unmarshal(data, &cfg); err != nil { + cfg.configFilePath = cfgPath + if err := json.Unmarshal(data, &cfgFile); err != nil { return nil, err } + endpointStr = cfgFile.Endpoint + cfg.accessToken = cfgFile.AccessToken + cfg.additionalHeaders = cfgFile.AdditionalHeaders + proxyStr = cfgFile.Proxy } envToken := os.Getenv("SRC_ACCESS_TOKEN") @@ -188,21 +215,32 @@ func readConfig() (*config, error) { // Apply config overrides. if envToken != "" { - cfg.AccessToken = envToken + cfg.accessToken = envToken } if envEndpoint != "" { - cfg.Endpoint = envEndpoint + endpointStr = envEndpoint } - if cfg.Endpoint == "" { - cfg.Endpoint = "https://sourcegraph.com" + if endpointStr == "" { + endpointStr = "https://sourcegraph.com" } if envProxy != "" { - cfg.Proxy = envProxy + proxyStr = envProxy } - if cfg.Proxy != "" { + // Lastly, apply endpoint flag if set + if endpoint != nil && *endpoint != "" { + endpointStr = *endpoint + } - parseEndpoint := func(endpoint string) (scheme string, address string) { + if endpointURL, err := parseEndpoint(endpointStr); err != nil { + return nil, errors.Newf("invalid endpoint: %s", endpointStr) + } else { + cfg.endpointURL = endpointURL + } + + if proxyStr != "" { + + parseProxyEndpoint := func(endpoint string) (scheme string, address string) { parts := strings.SplitN(endpoint, "://", 2) if len(parts) == 2 { return parts[0], parts[1] @@ -216,15 +254,15 @@ func readConfig() (*config, error) { return slices.Contains(urlSchemes, scheme) } - scheme, address := parseEndpoint(cfg.Proxy) + scheme, address := parseProxyEndpoint(proxyStr) if isURLScheme(scheme) { - endpoint := cfg.Proxy + endpoint := proxyStr // assume socks means socks5, because that's all we support if scheme == "socks" { endpoint = "socks5://" + address } - cfg.ProxyURL, err = url.Parse(endpoint) + cfg.proxyURL, err = url.Parse(endpoint) if err != nil { return nil, err } @@ -240,33 +278,30 @@ func readConfig() (*config, error) { if !isValidUDS { return nil, errors.Newf("invalid proxy socket: %s", path) } - cfg.ProxyPath = path + cfg.proxyPath = path + } else { + return nil, errors.Newf("Invalid proxy endpoint: %s", proxyStr) + } + } else { + // no SRC_PROXY; check for the standard proxy env variables HTTP_PROXY, HTTPS_PROXY, and NO_PROXY + if u, err := http.ProxyFromEnvironment(&http.Request{URL: cfg.endpointURL}); err != nil { + // when there's an error, the value for the env variable is not a legit URL + return nil, fmt.Errorf("Invalid HTTP_PROXY or HTTPS_PROXY value: %w", err) } else { - return nil, errors.Newf("invalid proxy endpoint: %s", cfg.Proxy) + cfg.proxyURL = u } } - cfg.AdditionalHeaders = parseAdditionalHeaders() + cfg.additionalHeaders = parseAdditionalHeaders() // Ensure that we're not clashing additonal headers - _, hasAuthorizationAdditonalHeader := cfg.AdditionalHeaders["authorization"] - if cfg.AccessToken != "" && hasAuthorizationAdditonalHeader { + _, hasAuthorizationAdditonalHeader := cfg.additionalHeaders["authorization"] + if cfg.accessToken != "" && hasAuthorizationAdditonalHeader { return nil, errConfigAuthorizationConflict } - // Lastly, apply endpoint flag if set - if endpoint != nil && *endpoint != "" { - cfg.Endpoint = *endpoint - } - - cfg.Endpoint = cleanEndpoint(cfg.Endpoint) - return &cfg, nil } -func cleanEndpoint(urlStr string) string { - return strings.TrimSuffix(urlStr, "/") -} - // isValidUnixSocket checks if the given path is a valid Unix socket. // // Parameters: diff --git a/cmd/src/main_test.go b/cmd/src/main_test.go index c37c36792a..4f0fa067e3 100644 --- a/cmd/src/main_test.go +++ b/cmd/src/main_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "github.com/sourcegraph/src-cli/internal/api" ) @@ -29,7 +30,7 @@ func TestReadConfig(t *testing.T) { tests := []struct { name string - fileContents *config + fileContents map[string]any envToken string envFooHeader string envHeaders string @@ -42,24 +43,29 @@ func TestReadConfig(t *testing.T) { { name: "defaults", want: &config{ - Endpoint: "https://sourcegraph.com", - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + additionalHeaders: map[string]string{}, }, }, { name: "config file, no overrides, trim slash", - fileContents: &config{ - Endpoint: "https://example.com/", - AccessToken: "deadbeef", - Proxy: "https://proxy.com:8080", + fileContents: map[string]any{ + "endpoint": "https://example.com/", + "accessToken": "deadbeef", + "proxy": "https://proxy.com:8080", }, want: &config{ - Endpoint: "https://example.com", - AccessToken: "deadbeef", - AdditionalHeaders: map[string]string{}, - Proxy: "https://proxy.com:8080", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "example.com", + }, + accessToken: "deadbeef", + additionalHeaders: map[string]string{}, + proxyPath: "", + proxyURL: &url.URL{ Scheme: "https", Host: "proxy.com:8080", }, @@ -67,9 +73,9 @@ func TestReadConfig(t *testing.T) { }, { name: "config file, token override only", - fileContents: &config{ - Endpoint: "https://example.com/", - AccessToken: "deadbeef", + fileContents: map[string]any{ + "endpoint": "https://example.com/", + "accessToken": "deadbeef", }, envToken: "abc", want: nil, @@ -77,9 +83,9 @@ func TestReadConfig(t *testing.T) { }, { name: "config file, endpoint override only", - fileContents: &config{ - Endpoint: "https://example.com/", - AccessToken: "deadbeef", + fileContents: map[string]any{ + "endpoint": "https://example.com/", + "accessToken": "deadbeef", }, envEndpoint: "https://exmaple2.com", want: nil, @@ -87,77 +93,89 @@ func TestReadConfig(t *testing.T) { }, { name: "config file, proxy override only (allow)", - fileContents: &config{ - Endpoint: "https://example.com/", - AccessToken: "deadbeef", - Proxy: "https://proxy.com:8080", + fileContents: map[string]any{ + "endpoint": "https://example.com/", + "accessToken": "deadbeef", + "proxy": "https://proxy.com:8080", }, envProxy: "socks5://other.proxy.com:9999", want: &config{ - Endpoint: "https://example.com", - AccessToken: "deadbeef", - Proxy: "socks5://other.proxy.com:9999", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "example.com", + }, + accessToken: "deadbeef", + proxyPath: "", + proxyURL: &url.URL{ Scheme: "socks5", Host: "other.proxy.com:9999", }, - AdditionalHeaders: map[string]string{}, + additionalHeaders: map[string]string{}, }, }, { name: "config file, all override", - fileContents: &config{ - Endpoint: "https://example.com/", - AccessToken: "deadbeef", - Proxy: "https://proxy.com:8080", + fileContents: map[string]any{ + "endpoint": "https://example.com/", + "accessToken": "deadbeef", + "proxy": "https://proxy.com:8080", }, envToken: "abc", envEndpoint: "https://override.com", envProxy: "socks5://other.proxy.com:9999", want: &config{ - Endpoint: "https://override.com", - AccessToken: "abc", - Proxy: "socks5://other.proxy.com:9999", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "override.com", + }, + accessToken: "abc", + proxyPath: "", + proxyURL: &url.URL{ Scheme: "socks5", Host: "other.proxy.com:9999", }, - AdditionalHeaders: map[string]string{}, + additionalHeaders: map[string]string{}, }, }, { name: "no config file, token from environment", envToken: "abc", want: &config{ - Endpoint: "https://sourcegraph.com", - AccessToken: "abc", - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + accessToken: "abc", + additionalHeaders: map[string]string{}, }, }, { name: "no config file, endpoint from environment", envEndpoint: "https://example.com", want: &config{ - Endpoint: "https://example.com", - AccessToken: "", - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "example.com", + }, + accessToken: "", + additionalHeaders: map[string]string{}, }, }, { name: "no config file, proxy from environment", envProxy: "https://proxy.com:8080", want: &config{ - Endpoint: "https://sourcegraph.com", - AccessToken: "", - Proxy: "https://proxy.com:8080", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + accessToken: "", + proxyPath: "", + proxyURL: &url.URL{ Scheme: "https", Host: "proxy.com:8080", }, - AdditionalHeaders: map[string]string{}, + additionalHeaders: map[string]string{}, }, }, { @@ -166,79 +184,92 @@ func TestReadConfig(t *testing.T) { envToken: "abc", envProxy: "https://proxy.com:8080", want: &config{ - Endpoint: "https://example.com", - AccessToken: "abc", - Proxy: "https://proxy.com:8080", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "example.com", + }, + accessToken: "abc", + proxyPath: "", + proxyURL: &url.URL{ Scheme: "https", Host: "proxy.com:8080", }, - AdditionalHeaders: map[string]string{}, + additionalHeaders: map[string]string{}, }, }, { name: "UNIX Domain Socket proxy using scheme and absolute path", envProxy: "unix://" + socketPath, want: &config{ - Endpoint: "https://sourcegraph.com", - Proxy: "unix://" + socketPath, - ProxyPath: socketPath, - ProxyURL: nil, - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + proxyPath: socketPath, + proxyURL: nil, + additionalHeaders: map[string]string{}, }, }, { name: "UNIX Domain Socket proxy with absolute path", envProxy: socketPath, want: &config{ - Endpoint: "https://sourcegraph.com", - Proxy: socketPath, - ProxyPath: socketPath, - ProxyURL: nil, - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + proxyPath: socketPath, + proxyURL: nil, + additionalHeaders: map[string]string{}, }, }, { name: "socks --> socks5", envProxy: "socks://localhost:1080", want: &config{ - Endpoint: "https://sourcegraph.com", - Proxy: "socks://localhost:1080", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + proxyPath: "", + proxyURL: &url.URL{ Scheme: "socks5", Host: "localhost:1080", }, - AdditionalHeaders: map[string]string{}, + additionalHeaders: map[string]string{}, }, }, { name: "socks5h", envProxy: "socks5h://localhost:1080", want: &config{ - Endpoint: "https://sourcegraph.com", - Proxy: "socks5h://localhost:1080", - ProxyPath: "", - ProxyURL: &url.URL{ + endpointURL: &url.URL{ + Scheme: "https", + Host: "sourcegraph.com", + }, + proxyPath: "", + proxyURL: &url.URL{ Scheme: "socks5h", Host: "localhost:1080", }, - AdditionalHeaders: map[string]string{}, + additionalHeaders: map[string]string{}, }, }, { name: "endpoint flag should override config", flagEndpoint: "https://override.com/", - fileContents: &config{ - Endpoint: "https://example.com/", - AccessToken: "deadbeef", - AdditionalHeaders: map[string]string{}, + fileContents: map[string]any{ + "endpoint": "https://example.com/", + "accessToken": "deadbeef", + "additionalHeaders": map[string]string{}, }, want: &config{ - Endpoint: "https://override.com", - AccessToken: "deadbeef", - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "override.com", + }, + accessToken: "deadbeef", + additionalHeaders: map[string]string{}, }, }, { @@ -247,9 +278,12 @@ func TestReadConfig(t *testing.T) { envEndpoint: "https://example.com", envToken: "abc", want: &config{ - Endpoint: "https://override.com", - AccessToken: "abc", - AdditionalHeaders: map[string]string{}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "override.com", + }, + accessToken: "abc", + additionalHeaders: map[string]string{}, }, }, { @@ -259,9 +293,12 @@ func TestReadConfig(t *testing.T) { envToken: "abc", envFooHeader: "bar", want: &config{ - Endpoint: "https://override.com", - AccessToken: "abc", - AdditionalHeaders: map[string]string{"foo": "bar"}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "override.com", + }, + accessToken: "abc", + additionalHeaders: map[string]string{"foo": "bar"}, }, }, { @@ -271,9 +308,12 @@ func TestReadConfig(t *testing.T) { envToken: "abc", envHeaders: "foo:bar\nfoo-bar:bar-baz", want: &config{ - Endpoint: "https://override.com", - AccessToken: "abc", - AdditionalHeaders: map[string]string{"foo-bar": "bar-baz", "foo": "bar"}, + endpointURL: &url.URL{ + Scheme: "https", + Host: "override.com", + }, + accessToken: "abc", + additionalHeaders: map[string]string{"foo-bar": "bar-baz", "foo": "bar"}, }, }, { @@ -311,7 +351,7 @@ func TestReadConfig(t *testing.T) { oldConfigPath := *configPath t.Cleanup(func() { *configPath = oldConfigPath }) - data, err := json.Marshal(*test.fileContents) + data, err := json.Marshal(test.fileContents) if err != nil { t.Fatal(err) } @@ -331,8 +371,11 @@ func TestReadConfig(t *testing.T) { t.Fatal(err) } - config, err := readConfig() - if diff := cmp.Diff(test.want, config); diff != "" { + got, err := readConfig() + if diff := cmp.Diff(test.want, got, + cmp.AllowUnexported(config{}), + cmpopts.IgnoreFields(config{}, "configFilePath"), + ); diff != "" { t.Errorf("config: %v", diff) } var errMsg string diff --git a/cmd/src/search.go b/cmd/src/search.go index 385ac7c72d..a2cd1a0bbe 100644 --- a/cmd/src/search.go +++ b/cmd/src/search.go @@ -266,7 +266,7 @@ Other tips: } improved := searchResultsImproved{ - SourcegraphEndpoint: cfg.Endpoint, + SourcegraphEndpoint: cfg.endpointURL.String(), Query: queryString, Site: result.Site, searchResults: result.Search.Results, diff --git a/cmd/src/search_jobs.go b/cmd/src/search_jobs.go index 96c5d9070a..15bf5d8a25 100644 --- a/cmd/src/search_jobs.go +++ b/cmd/src/search_jobs.go @@ -156,8 +156,8 @@ func parseColumns(columnsFlag string) []string { // createSearchJobsClient creates a reusable API client for search jobs commands func createSearchJobsClient(out *flag.FlagSet, apiFlags *api.Flags) api.Client { return api.NewClient(api.ClientOpts{ - Endpoint: cfg.Endpoint, - AccessToken: cfg.AccessToken, + EndpointURL: cfg.endpointURL, + AccessToken: cfg.accessToken, Out: out.Output(), Flags: apiFlags, }) diff --git a/cmd/src/search_jobs_logs.go b/cmd/src/search_jobs_logs.go index 2fe9b6bfee..6327a609ab 100644 --- a/cmd/src/search_jobs_logs.go +++ b/cmd/src/search_jobs_logs.go @@ -22,7 +22,7 @@ func fetchJobLogs(jobID string, logURL string) (io.ReadCloser, error) { return nil, err } - req.Header.Add("Authorization", "token "+cfg.AccessToken) + req.Header.Add("Authorization", "token "+cfg.accessToken) resp, err := http.DefaultClient.Do(req) if err != nil { diff --git a/cmd/src/search_jobs_results.go b/cmd/src/search_jobs_results.go index 2e45f8219f..9d8bc7a9ab 100644 --- a/cmd/src/search_jobs_results.go +++ b/cmd/src/search_jobs_results.go @@ -22,7 +22,7 @@ func fetchJobResults(jobID string, resultsURL string) (io.ReadCloser, error) { return nil, err } - req.Header.Add("Authorization", "token "+cfg.AccessToken) + req.Header.Add("Authorization", "token "+cfg.accessToken) resp, err := http.DefaultClient.Do(req) if err != nil { diff --git a/cmd/src/search_stream.go b/cmd/src/search_stream.go index 512e7f9f75..9b5415851a 100644 --- a/cmd/src/search_stream.go +++ b/cmd/src/search_stream.go @@ -160,7 +160,7 @@ func textDecoder(query string, t *template.Template, w io.Writer) streaming.Deco SourcegraphEndpoint string *streaming.EventRepoMatch }{ - SourcegraphEndpoint: cfg.Endpoint, + SourcegraphEndpoint: cfg.endpointURL.String(), EventRepoMatch: match, }) if err != nil { @@ -172,7 +172,7 @@ func textDecoder(query string, t *template.Template, w io.Writer) streaming.Deco SourcegraphEndpoint string *streaming.EventCommitMatch }{ - SourcegraphEndpoint: cfg.Endpoint, + SourcegraphEndpoint: cfg.endpointURL.String(), EventCommitMatch: match, }) if err != nil { @@ -184,7 +184,7 @@ func textDecoder(query string, t *template.Template, w io.Writer) streaming.Deco SourcegraphEndpoint string *streaming.EventSymbolMatch }{ - SourcegraphEndpoint: cfg.Endpoint, + SourcegraphEndpoint: cfg.endpointURL.String(), EventSymbolMatch: match, }, ) diff --git a/cmd/src/search_stream_test.go b/cmd/src/search_stream_test.go index 1653b273ac..cb2d91af9f 100644 --- a/cmd/src/search_stream_test.go +++ b/cmd/src/search_stream_test.go @@ -5,6 +5,7 @@ import ( "io" "net" "net/http" + "net/url" "net/http/httptest" "os" "testing" @@ -126,8 +127,9 @@ func TestSearchStream(t *testing.T) { s := testServer(t, http.HandlerFunc(mockStreamHandler)) defer s.Close() + u, _ := url.ParseRequestURI(s.URL) cfg = &config{ - Endpoint: s.URL, + endpointURL: u, } defer func() { cfg = nil }() diff --git a/cmd/src/users_prune.go b/cmd/src/users_prune.go index 90de530c68..d67fff6be0 100644 --- a/cmd/src/users_prune.go +++ b/cmd/src/users_prune.go @@ -225,7 +225,7 @@ type UserToDelete struct { // Verify user wants to remove users with table of users and a command prompt for [y/N] func confirmUserRemoval(usersToDelete []UserToDelete, daysThreshold int, displayUsers bool) (bool, error) { if displayUsers { - fmt.Printf("Users to remove from %s\n", cfg.Endpoint) + fmt.Printf("Users to remove from %s\n", cfg.endpointURL) t := table.NewWriter() t.SetOutputMirror(os.Stdout) t.AppendHeader(table.Row{"Username", "Email", "Days Since Last Active"}) @@ -243,7 +243,7 @@ func confirmUserRemoval(usersToDelete []UserToDelete, daysThreshold int, display } input := "" for strings.ToLower(input) != "y" && strings.ToLower(input) != "n" { - fmt.Printf("%v users were inactive for more than %v days on %v.\nDo you wish to proceed with user removal [y/N]: ", len(usersToDelete), daysThreshold, cfg.Endpoint) + fmt.Printf("%v users were inactive for more than %v days on %v.\nDo you wish to proceed with user removal [y/N]: ", len(usersToDelete), daysThreshold, cfg.endpointURL) if _, err := fmt.Scanln(&input); err != nil { return false, err } diff --git a/internal/api/api.go b/internal/api/api.go index ef9f822a7a..d0463370cb 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -12,7 +12,6 @@ import ( "net/url" "os" "runtime" - "strings" ioaux "github.com/jig/teereadcloser" "github.com/kballard/go-shellquote" @@ -72,7 +71,7 @@ type request struct { // ClientOpts encapsulates the options given to NewClient. type ClientOpts struct { - Endpoint string + EndpointURL *url.URL AccessToken string AdditionalHeaders map[string]string @@ -139,7 +138,7 @@ func NewClient(opts ClientOpts) Client { return &client{ opts: ClientOpts{ - Endpoint: opts.Endpoint, + EndpointURL: opts.EndpointURL, AccessToken: opts.AccessToken, AdditionalHeaders: opts.AdditionalHeaders, Flags: flags, @@ -174,7 +173,7 @@ func (c *client) NewHTTPRequest(ctx context.Context, method, p string, body io.R } func (c *client) createHTTPRequest(ctx context.Context, method, p string, body io.Reader) (*http.Request, error) { - req, err := http.NewRequestWithContext(ctx, method, strings.TrimRight(c.opts.Endpoint, "/")+"/"+p, body) + req, err := http.NewRequestWithContext(ctx, method, c.opts.EndpointURL.JoinPath(p).String(), body) if err != nil { return nil, err } @@ -269,7 +268,7 @@ func (r *request) do(ctx context.Context, result any) (bool, error) { if oauth.IsOAuthTransport(r.client.httpClient.Transport) { fmt.Println("The OAuth token is invalid. Please check that the Sourcegraph CLI client is still authorized.") fmt.Println("") - fmt.Printf("To re-authorize, run: src login --oauth %s\n", r.client.opts.Endpoint) + fmt.Printf("To re-authorize, run: src login --oauth %s\n", r.client.opts.EndpointURL) fmt.Println("") fmt.Println("Learn more at https://github.com/sourcegraph/src-cli#readme") fmt.Println("") @@ -360,6 +359,6 @@ func (r *request) curlCmd() (string, error) { s += fmt.Sprintf(" %s \\\n", shellquote.Join("-H", k+": "+v)) } s += fmt.Sprintf(" %s \\\n", shellquote.Join("-d", string(data))) - s += fmt.Sprintf(" %s", shellquote.Join(r.client.opts.Endpoint+"/.api/graphql")) + s += fmt.Sprintf(" %s", shellquote.Join(r.client.opts.EndpointURL.JoinPath(".api/graphql").String())) return s, nil } diff --git a/internal/batches/executor/executor_test.go b/internal/batches/executor/executor_test.go index 9fc96d927d..f54d298807 100644 --- a/internal/batches/executor/executor_test.go +++ b/internal/batches/executor/executor_test.go @@ -6,6 +6,7 @@ import ( "fmt" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" "runtime" @@ -406,7 +407,8 @@ func TestExecutor_Integration(t *testing.T) { // Setup an api.Client that points to this test server var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + tsURL, _ := url.Parse(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) // Temp dir for log files and downloaded archives testTempDir := t.TempDir() @@ -827,7 +829,8 @@ func testExecuteTasks(t *testing.T, tasks []*Task, archives ...mock.RepoArchive) t.Cleanup(ts.Close) var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + tsURL, _ := url.Parse(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) // Prepare images // diff --git a/internal/batches/repozip/fetcher_test.go b/internal/batches/repozip/fetcher_test.go index f871237e45..883db5dc66 100644 --- a/internal/batches/repozip/fetcher_test.go +++ b/internal/batches/repozip/fetcher_test.go @@ -5,6 +5,7 @@ import ( "context" "net/http" "net/http/httptest" + "net/url" "os" "path" "path/filepath" @@ -44,7 +45,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + tsURL, _ := url.Parse(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, @@ -89,7 +91,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + tsURL, _ := url.Parse(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, @@ -153,7 +156,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + tsURL, _ := url.Parse(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, @@ -193,7 +197,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + tsURL, _ := url.Parse(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, @@ -262,7 +267,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - client := api.NewClient(api.ClientOpts{Endpoint: ts.URL, Out: &clientBuffer}) + tsURL, _ := url.Parse(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, From 7cf81b871e9adaab2ae7d15a140b27e395a7bb07 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Fri, 6 Mar 2026 19:06:16 -0800 Subject: [PATCH 02/15] fix formatting and error messages --- cmd/src/main.go | 13 ++++++------- cmd/src/search_stream_test.go | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/cmd/src/main.go b/cmd/src/main.go index 51e128b043..3d447866f9 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "flag" - "fmt" "io" "log" "net" @@ -117,10 +116,10 @@ func parseEndpoint(endpoint string) (*url.URL, error) { return nil, err } if !(u.Scheme == "http" || u.Scheme == "https") { - return nil, errors.Newf("Invalid scheme %s. Require http or https", u.Scheme) + return nil, errors.Newf("invalid scheme %s: require http or https", u.Scheme) } if u.Host == "" { - return nil, errors.Newf("Empty host") + return nil, errors.Newf("empty host") } return u, nil } @@ -273,20 +272,20 @@ func readConfig() (*config, error) { } isValidUDS, err := isValidUnixSocket(path) if err != nil { - return nil, errors.Newf("Invalid proxy configuration: %w", err) + return nil, errors.Newf("invalid proxy configuration: %w", err) } if !isValidUDS { return nil, errors.Newf("invalid proxy socket: %s", path) } cfg.proxyPath = path } else { - return nil, errors.Newf("Invalid proxy endpoint: %s", proxyStr) + return nil, errors.Newf("invalid proxy endpoint: %s", proxyStr) } } else { // no SRC_PROXY; check for the standard proxy env variables HTTP_PROXY, HTTPS_PROXY, and NO_PROXY if u, err := http.ProxyFromEnvironment(&http.Request{URL: cfg.endpointURL}); err != nil { // when there's an error, the value for the env variable is not a legit URL - return nil, fmt.Errorf("Invalid HTTP_PROXY or HTTPS_PROXY value: %w", err) + return nil, errors.Newf("invalid HTTP_PROXY or HTTPS_PROXY value: %w", err) } else { cfg.proxyURL = u } @@ -321,7 +320,7 @@ func isValidUnixSocket(path string) (bool, error) { if os.IsNotExist(err) { return false, nil } - return false, errors.Newf("Not a UNIX Domain Socket: %v: %w", path, err) + return false, errors.Newf("not a UNIX domain socket: %v: %w", path, err) } defer conn.Close() diff --git a/cmd/src/search_stream_test.go b/cmd/src/search_stream_test.go index cb2d91af9f..71b099abb9 100644 --- a/cmd/src/search_stream_test.go +++ b/cmd/src/search_stream_test.go @@ -5,8 +5,8 @@ import ( "io" "net" "net/http" - "net/url" "net/http/httptest" + "net/url" "os" "testing" From f0044875225642f6494644e40a9925ea4b8734fd Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 14:01:28 -0800 Subject: [PATCH 03/15] clear any auth from the endpoint url at program load --- cmd/src/code_intel_upload.go | 1 - cmd/src/main.go | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/cmd/src/code_intel_upload.go b/cmd/src/code_intel_upload.go index f110eace50..031f38d4f0 100644 --- a/cmd/src/code_intel_upload.go +++ b/cmd/src/code_intel_upload.go @@ -191,7 +191,6 @@ func makeCodeIntelUploadURL(uploadID int) string { u := *cfg.endpointURL graphqlID := base64.URLEncoding.EncodeToString(fmt.Appendf(nil, `SCIPUpload:%d`, uploadID)) u.Path = codeintelUploadFlags.repo + "/-/code-intelligence/uploads/" + graphqlID - u.User = nil return u.String() } diff --git a/cmd/src/main.go b/cmd/src/main.go index 3d447866f9..af10496e45 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -121,6 +121,9 @@ func parseEndpoint(endpoint string) (*url.URL, error) { if u.Host == "" { return nil, errors.Newf("empty host") } + // auth in the URL is not used, and could be explosed in log output. + // Explicitly clear it in case it's accidentally set in SRC_ENDPOINT or the config file. + u.User = nil return u, nil } From 2d77f7b581ae4015048dc1153845c0c7b63f4610 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 14:06:17 -0800 Subject: [PATCH 04/15] rename variables --- cmd/src/main.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/cmd/src/main.go b/cmd/src/main.go index af10496e45..38a71ad69b 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -163,14 +163,14 @@ func (c *config) apiClient(flags *api.Flags, out io.Writer) api.Client { // readConfig reads the config file from the given path. func readConfig() (*config, error) { - cfgFilePath := *configPath + cfgFile := *configPath userSpecified := *configPath != "" if !userSpecified { - cfgFilePath = "~/src-config.json" + cfgFile = "~/src-config.json" } - cfgPath, err := expandHomeDir(cfgFilePath) + cfgPath, err := expandHomeDir(cfgFile) if err != nil { return nil, err } @@ -179,7 +179,7 @@ func readConfig() (*config, error) { if err != nil && (!os.IsNotExist(err) || userSpecified) { return nil, err } - var cfgFile struct { + var configFromFile struct { Endpoint string `json:"endpoint"` AccessToken string `json:"accessToken"` AdditionalHeaders map[string]string `json:"additionalHeaders"` @@ -190,13 +190,13 @@ func readConfig() (*config, error) { var proxyStr string if err == nil { cfg.configFilePath = cfgPath - if err := json.Unmarshal(data, &cfgFile); err != nil { + if err := json.Unmarshal(data, &configFromFile); err != nil { return nil, err } - endpointStr = cfgFile.Endpoint - cfg.accessToken = cfgFile.AccessToken - cfg.additionalHeaders = cfgFile.AdditionalHeaders - proxyStr = cfgFile.Proxy + endpointStr = configFromFile.Endpoint + cfg.accessToken = configFromFile.AccessToken + cfg.additionalHeaders = configFromFile.AdditionalHeaders + proxyStr = configFromFile.Proxy } envToken := os.Getenv("SRC_ACCESS_TOKEN") From 921b34b87d7ac97bff250d1bfa89976f01a3f401 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 15:25:07 -0800 Subject: [PATCH 05/15] fixup main tests --- cmd/src/main.go | 27 +++++++++++++++----------- cmd/src/main_test.go | 46 ++++++++++++++++++++++---------------------- 2 files changed, 39 insertions(+), 34 deletions(-) diff --git a/cmd/src/main.go b/cmd/src/main.go index 38a71ad69b..0c0e807c58 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -139,6 +139,15 @@ type config struct { endpointURL *url.URL } +// configFromFile holds the config as read from the config file, +// which is validated and parsed into the config struct +type configFromFile struct { + Endpoint string `json:"endpoint"` + AccessToken string `json:"accessToken"` + AdditionalHeaders map[string]string `json:"additionalHeaders"` + Proxy string `json:"proxy"` +} + // apiClient returns an api.Client built from the configuration. func (c *config) apiClient(flags *api.Flags, out io.Writer) api.Client { opts := api.ClientOpts{ @@ -179,24 +188,20 @@ func readConfig() (*config, error) { if err != nil && (!os.IsNotExist(err) || userSpecified) { return nil, err } - var configFromFile struct { - Endpoint string `json:"endpoint"` - AccessToken string `json:"accessToken"` - AdditionalHeaders map[string]string `json:"additionalHeaders"` - Proxy string `json:"proxy"` - } + + var cfgFromFile configFromFile var cfg config var endpointStr string var proxyStr string if err == nil { cfg.configFilePath = cfgPath - if err := json.Unmarshal(data, &configFromFile); err != nil { + if err := json.Unmarshal(data, &cfgFromFile); err != nil { return nil, err } - endpointStr = configFromFile.Endpoint - cfg.accessToken = configFromFile.AccessToken - cfg.additionalHeaders = configFromFile.AdditionalHeaders - proxyStr = configFromFile.Proxy + endpointStr = cfgFromFile.Endpoint + cfg.accessToken = cfgFromFile.AccessToken + cfg.additionalHeaders = cfgFromFile.AdditionalHeaders + proxyStr = cfgFromFile.Proxy } envToken := os.Getenv("SRC_ACCESS_TOKEN") diff --git a/cmd/src/main_test.go b/cmd/src/main_test.go index 4f0fa067e3..6d597caea4 100644 --- a/cmd/src/main_test.go +++ b/cmd/src/main_test.go @@ -30,7 +30,7 @@ func TestReadConfig(t *testing.T) { tests := []struct { name string - fileContents map[string]any + fileContents *configFromFile envToken string envFooHeader string envHeaders string @@ -52,10 +52,10 @@ func TestReadConfig(t *testing.T) { }, { name: "config file, no overrides, trim slash", - fileContents: map[string]any{ - "endpoint": "https://example.com/", - "accessToken": "deadbeef", - "proxy": "https://proxy.com:8080", + fileContents: &configFromFile{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + Proxy: "https://proxy.com:8080", }, want: &config{ endpointURL: &url.URL{ @@ -73,9 +73,9 @@ func TestReadConfig(t *testing.T) { }, { name: "config file, token override only", - fileContents: map[string]any{ - "endpoint": "https://example.com/", - "accessToken": "deadbeef", + fileContents: &configFromFile{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", }, envToken: "abc", want: nil, @@ -83,9 +83,9 @@ func TestReadConfig(t *testing.T) { }, { name: "config file, endpoint override only", - fileContents: map[string]any{ - "endpoint": "https://example.com/", - "accessToken": "deadbeef", + fileContents: &configFromFile{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", }, envEndpoint: "https://exmaple2.com", want: nil, @@ -93,10 +93,10 @@ func TestReadConfig(t *testing.T) { }, { name: "config file, proxy override only (allow)", - fileContents: map[string]any{ - "endpoint": "https://example.com/", - "accessToken": "deadbeef", - "proxy": "https://proxy.com:8080", + fileContents: &configFromFile{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + Proxy: "https://proxy.com:8080", }, envProxy: "socks5://other.proxy.com:9999", want: &config{ @@ -115,10 +115,10 @@ func TestReadConfig(t *testing.T) { }, { name: "config file, all override", - fileContents: map[string]any{ - "endpoint": "https://example.com/", - "accessToken": "deadbeef", - "proxy": "https://proxy.com:8080", + fileContents: &configFromFile{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + Proxy: "https://proxy.com:8080", }, envToken: "abc", envEndpoint: "https://override.com", @@ -258,10 +258,10 @@ func TestReadConfig(t *testing.T) { { name: "endpoint flag should override config", flagEndpoint: "https://override.com/", - fileContents: map[string]any{ - "endpoint": "https://example.com/", - "accessToken": "deadbeef", - "additionalHeaders": map[string]string{}, + fileContents: &configFromFile{ + Endpoint: "https://example.com/", + AccessToken: "deadbeef", + AdditionalHeaders: map[string]string{}, }, want: &config{ endpointURL: &url.URL{ From c45082ee5aaa6df38593cfbbee64ce0e4abe8ea9 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 16:16:46 -0800 Subject: [PATCH 06/15] restore indirection --- cmd/src/main_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/src/main_test.go b/cmd/src/main_test.go index 6d597caea4..95bccf5cbd 100644 --- a/cmd/src/main_test.go +++ b/cmd/src/main_test.go @@ -351,7 +351,7 @@ func TestReadConfig(t *testing.T) { oldConfigPath := *configPath t.Cleanup(func() { *configPath = oldConfigPath }) - data, err := json.Marshal(test.fileContents) + data, err := json.Marshal(*test.fileContents) if err != nil { t.Fatal(err) } From 26a0b641427823680f1a63e515002dce47108cbd Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 16:17:20 -0800 Subject: [PATCH 07/15] fix capitalization --- cmd/src/main.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/src/main.go b/cmd/src/main.go index 0c0e807c58..973e2618a2 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -140,7 +140,7 @@ type config struct { } // configFromFile holds the config as read from the config file, -// which is validated and parsed into the config struct +// which is validated and parsed into the config struct. type configFromFile struct { Endpoint string `json:"endpoint"` AccessToken string `json:"accessToken"` @@ -328,7 +328,7 @@ func isValidUnixSocket(path string) (bool, error) { if os.IsNotExist(err) { return false, nil } - return false, errors.Newf("not a UNIX domain socket: %v: %w", path, err) + return false, errors.Newf("not a UNIX Domain Socket: %v: %w", path, err) } defer conn.Close() From 4bfcd6f252806ffc781f77cf22c31780e8c713d0 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 16:17:59 -0800 Subject: [PATCH 08/15] standardize url parsing --- internal/batches/executor/executor_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/batches/executor/executor_test.go b/internal/batches/executor/executor_test.go index f54d298807..03f25e08a8 100644 --- a/internal/batches/executor/executor_test.go +++ b/internal/batches/executor/executor_test.go @@ -407,8 +407,8 @@ func TestExecutor_Integration(t *testing.T) { // Setup an api.Client that points to this test server var clientBuffer bytes.Buffer - tsURL, _ := url.Parse(ts.URL) - client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) // Temp dir for log files and downloaded archives testTempDir := t.TempDir() @@ -829,8 +829,8 @@ func testExecuteTasks(t *testing.T, tasks []*Task, archives ...mock.RepoArchive) t.Cleanup(ts.Close) var clientBuffer bytes.Buffer - tsURL, _ := url.Parse(ts.URL) - client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) // Prepare images // From 39be8262eaf7c679a164df587247ca2de5a0025e Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 16:18:29 -0800 Subject: [PATCH 09/15] standardize url parsing --- internal/batches/repozip/fetcher_test.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/batches/repozip/fetcher_test.go b/internal/batches/repozip/fetcher_test.go index 883db5dc66..56d03d85a6 100644 --- a/internal/batches/repozip/fetcher_test.go +++ b/internal/batches/repozip/fetcher_test.go @@ -45,8 +45,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - tsURL, _ := url.Parse(ts.URL) - client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, @@ -91,8 +91,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - tsURL, _ := url.Parse(ts.URL) - client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, @@ -156,8 +156,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - tsURL, _ := url.Parse(ts.URL) - client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, @@ -197,8 +197,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - tsURL, _ := url.Parse(ts.URL) - client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, @@ -267,8 +267,8 @@ func TestArchive_Ensure(t *testing.T) { defer ts.Close() var clientBuffer bytes.Buffer - tsURL, _ := url.Parse(ts.URL) - client := api.NewClient(api.ClientOpts{EndpointURL: tsURL, Out: &clientBuffer}) + u, _ := url.ParseRequestURI(ts.URL) + client := api.NewClient(api.ClientOpts{EndpointURL: u, Out: &clientBuffer}) rf := &archiveRegistry{ client: client, From f42f16a71caa848ad55bb70619f2dcfb72b5e7e3 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 17:14:24 -0800 Subject: [PATCH 10/15] remove support for HTTP(S)_PROXY - that will come later --- cmd/src/main.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/cmd/src/main.go b/cmd/src/main.go index 973e2618a2..8bd1a5fe77 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -7,7 +7,6 @@ import ( "io" "log" "net" - "net/http" "net/url" "os" "path/filepath" @@ -289,14 +288,6 @@ func readConfig() (*config, error) { } else { return nil, errors.Newf("invalid proxy endpoint: %s", proxyStr) } - } else { - // no SRC_PROXY; check for the standard proxy env variables HTTP_PROXY, HTTPS_PROXY, and NO_PROXY - if u, err := http.ProxyFromEnvironment(&http.Request{URL: cfg.endpointURL}); err != nil { - // when there's an error, the value for the env variable is not a legit URL - return nil, errors.Newf("invalid HTTP_PROXY or HTTPS_PROXY value: %w", err) - } else { - cfg.proxyURL = u - } } cfg.additionalHeaders = parseAdditionalHeaders() From ed3146553e5e0f30c6d927d882eff8b7f0da0492 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Mon, 9 Mar 2026 09:45:22 -0700 Subject: [PATCH 11/15] revert JoinPath in createHTTPRequest because the input path could contain a query string --- internal/api/api.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/api/api.go b/internal/api/api.go index d0463370cb..07b28230c6 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -173,7 +173,8 @@ func (c *client) NewHTTPRequest(ctx context.Context, method, p string, body io.R } func (c *client) createHTTPRequest(ctx context.Context, method, p string, body io.Reader) (*http.Request, error) { - req, err := http.NewRequestWithContext(ctx, method, c.opts.EndpointURL.JoinPath(p).String(), body) + // Can't use c.opts.EndpointURL.JoinPaht(p) here because `p` could contain a query string + req, err := http.NewRequestWithContext(ctx, method, c.opts.EndpointURL.String()+"/"+p, body) if err != nil { return nil, err } From 97c1b0ab3c7c9ce1551b4c2e5b8434676e200d71 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Mon, 9 Mar 2026 16:40:41 -0700 Subject: [PATCH 12/15] added a comment about the URL shallow copy --- cmd/src/code_intel_upload.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/src/code_intel_upload.go b/cmd/src/code_intel_upload.go index 031f38d4f0..04c772d115 100644 --- a/cmd/src/code_intel_upload.go +++ b/cmd/src/code_intel_upload.go @@ -188,6 +188,7 @@ func printInferredArguments(out *output.Output) { // makeCodeIntelUploadURL constructs a URL to the upload with the given internal identifier. // The base of the URL is constructed from the configured Sourcegraph instance. func makeCodeIntelUploadURL(uploadID int) string { + // Careful: copy by dereference makes a shallow copy, so User is not duplicated. u := *cfg.endpointURL graphqlID := base64.URLEncoding.EncodeToString(fmt.Appendf(nil, `SCIPUpload:%d`, uploadID)) u.Path = codeintelUploadFlags.repo + "/-/code-intelligence/uploads/" + graphqlID From e62a619079cebe50f643c9e9eeea569835584e36 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Mon, 9 Mar 2026 19:13:25 -0700 Subject: [PATCH 13/15] fix typo in comment --- internal/api/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/api.go b/internal/api/api.go index 07b28230c6..c38af19d25 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -173,7 +173,7 @@ func (c *client) NewHTTPRequest(ctx context.Context, method, p string, body io.R } func (c *client) createHTTPRequest(ctx context.Context, method, p string, body io.Reader) (*http.Request, error) { - // Can't use c.opts.EndpointURL.JoinPaht(p) here because `p` could contain a query string + // Can't use c.opts.EndpointURL.JoinPath(p) here because `p` could contain a query string req, err := http.NewRequestWithContext(ctx, method, c.opts.EndpointURL.String()+"/"+p, body) if err != nil { return nil, err From 8ecf5d667a7a36374cbcfe0989ca91c76d3ea7e5 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Tue, 10 Mar 2026 09:20:46 -0700 Subject: [PATCH 14/15] re-apply JoinPath lost during merge --- cmd/src/batch_remote.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cmd/src/batch_remote.go b/cmd/src/batch_remote.go index 86ad3650d1..c1d7209675 100644 --- a/cmd/src/batch_remote.go +++ b/cmd/src/batch_remote.go @@ -5,7 +5,6 @@ import ( "flag" "fmt" cliLog "log" - "strings" "time" "github.com/sourcegraph/sourcegraph/lib/errors" @@ -155,13 +154,14 @@ Examples: } ui.ExecutingBatchSpecSuccess() - executionURL := fmt.Sprintf( - "%s/%s/batch-changes/%s/executions/%s", - cfg.endpointURL, - strings.TrimPrefix(namespace.URL, "/"), - batchChangeName, - batchSpecID, - ) + executionURL := cfg.endpointURL.JoinPath( + fmt.Sprintf( + "%s/batch-changes/%s/executions/%s", + namespace.URL, + batchChangeName, + batchSpecID, + ), + ).String() ui.RemoteSuccess(executionURL) return nil From f976adb4ba4694c8af60dd76c67e24157ad4948e Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Tue, 10 Mar 2026 09:24:18 -0700 Subject: [PATCH 15/15] rename variable --- cmd/src/code_intel_upload.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/src/code_intel_upload.go b/cmd/src/code_intel_upload.go index 04c772d115..d5e686babc 100644 --- a/cmd/src/code_intel_upload.go +++ b/cmd/src/code_intel_upload.go @@ -189,10 +189,10 @@ func printInferredArguments(out *output.Output) { // The base of the URL is constructed from the configured Sourcegraph instance. func makeCodeIntelUploadURL(uploadID int) string { // Careful: copy by dereference makes a shallow copy, so User is not duplicated. - u := *cfg.endpointURL + url := *cfg.endpointURL graphqlID := base64.URLEncoding.EncodeToString(fmt.Appendf(nil, `SCIPUpload:%d`, uploadID)) - u.Path = codeintelUploadFlags.repo + "/-/code-intelligence/uploads/" + graphqlID - return u.String() + url.Path = codeintelUploadFlags.repo + "/-/code-intelligence/uploads/" + graphqlID + return url.String() } type errorWithHint struct {