From 1738d6e88aa4e6ecd00002926cfb1097a349806d Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Fri, 6 Mar 2026 18:37:27 -0800 Subject: [PATCH 01/10] 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 | 43 ++-- cmd/src/login_test.go | 34 ++- cmd/src/main.go | 123 +++++++---- 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 | 9 +- internal/batches/executor/executor_test.go | 7 +- internal/batches/repozip/fetcher_test.go | 16 +- 21 files changed, 300 insertions(+), 233 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 ab5a097c71..d3d540cb50 100644 --- a/cmd/src/login.go +++ b/cmd/src/login.go @@ -44,17 +44,21 @@ 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("endpoint argument %s conflicts with configured endpoint %s", parsed, cfg.endpointURL)) + } } client := cfg.apiClient(apiFlags, io.Discard) - return loginCmd(context.Background(), cfg, client, endpoint, os.Stdout) + return loginCmd(context.Background(), cfg, client, os.Stdout) } commands = append(commands, &command{ @@ -64,9 +68,7 @@ Examples: }) } -func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg string, out io.Writer) error { - endpointArg = cleanEndpoint(endpointArg) - +func loginCmd(ctx context.Context, cfg *config, client api.Client, out io.Writer) error { printProblem := func(problem string) { fmt.Fprintf(out, "❌ Problem: %s\n", problem) } @@ -77,23 +79,16 @@ func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg s export SRC_ACCESS_TOKEN=(your access token) To verify that it's working, run the login command again. -`, endpointArg, endpointArg) +`, cfg.endpointURL, cfg.endpointURL) - if cfg.ConfigFilePath != "" { + if cfg.configFilePath != "" { fmt.Fprintln(out) - fmt.Fprintf(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", cfg.ConfigFilePath) + fmt.Fprintf(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", cfg.configFilePath) } - noToken := cfg.AccessToken == "" - endpointConflict := endpointArg != cfg.Endpoint - if noToken || endpointConflict { + if cfg.accessToken == "" { fmt.Fprintln(out) - switch { - case noToken: - printProblem("No access token is configured.") - case endpointConflict: - printProblem(fmt.Sprintf("The configured endpoint is %s, not %s.", cfg.Endpoint, endpointArg)) - } + printProblem("No access token is configured.") fmt.Fprintln(out, createAccessTokenMessage) return cmderrors.ExitCode1 } @@ -107,7 +102,7 @@ func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg s if strings.HasPrefix(err.Error(), "error: 401 Unauthorized") || strings.HasPrefix(err.Error(), "error: 403 Forbidden") { printProblem("Invalid access token.") } else { - printProblem(fmt.Sprintf("Error communicating with %s: %s", endpointArg, err)) + printProblem(fmt.Sprintf("Error communicating with %s: %s", cfg.endpointURL, err)) } fmt.Fprintln(out, createAccessTokenMessage) 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)") @@ -117,11 +112,11 @@ func loginCmd(ctx context.Context, cfg *config, client api.Client, endpointArg s if result.CurrentUser == nil { // This should never happen; we verified there is an access token, so there should always be // a user. - printProblem(fmt.Sprintf("Unable to determine user on %s.", endpointArg)) + printProblem(fmt.Sprintf("Unable to determine user on %s.", cfg.endpointURL)) 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, cfg.endpointURL) fmt.Fprintln(out) return nil } diff --git a/cmd/src/login_test.go b/cmd/src/login_test.go index 37fbf7a703..99d2c3d492 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" @@ -14,27 +15,17 @@ 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() var out bytes.Buffer - err = loginCmd(context.Background(), cfg, cfg.apiClient(nil, io.Discard), endpointArg, &out) + err = loginCmd(context.Background(), cfg, cfg.apiClient(nil, io.Discard), &out) 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." - 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) } @@ -45,7 +36,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) } @@ -62,13 +54,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 (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) } @@ -81,13 +73,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 = strings.ReplaceAll(wantOut, "$ENDPOINT", s.URL) if out != wantOut { t.Errorf("got output %q, want %q", out, wantOut) } diff --git a/cmd/src/main.go b/cmd/src/main.go index edfb1073d7..da25ddf54b 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -3,9 +3,11 @@ package main import ( "encoding/json" "flag" + "fmt" "io" "log" "net" + "net/http" "net/url" "os" "path/filepath" @@ -107,42 +109,55 @@ 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 { return api.NewClient(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, }) } // 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 } @@ -151,12 +166,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") @@ -177,21 +204,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] @@ -205,15 +243,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 } @@ -229,33 +267,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 5f750c1d4a..40ca5c6750 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" @@ -71,7 +70,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 @@ -124,7 +123,7 @@ func NewClient(opts ClientOpts) Client { return &client{ opts: ClientOpts{ - Endpoint: opts.Endpoint, + EndpointURL: opts.EndpointURL, AccessToken: opts.AccessToken, AdditionalHeaders: opts.AdditionalHeaders, Flags: flags, @@ -159,7 +158,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 } @@ -334,6 +333,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 a6113fb8718b22af12a4b14ff25827fc1dc56c45 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Fri, 6 Mar 2026 19:06:16 -0800 Subject: [PATCH 02/10] 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 da25ddf54b..c12670b561 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -3,7 +3,6 @@ package main import ( "encoding/json" "flag" - "fmt" "io" "log" "net" @@ -115,10 +114,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 } @@ -262,20 +261,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 } @@ -310,7 +309,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 fb49d7a71ff763c07d8cf7419973b35d2dcd6174 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 14:01:28 -0800 Subject: [PATCH 03/10] 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 c12670b561..5c72c5b24b 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -119,6 +119,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 5c98127f187dde1d975b8fe257918b063defc1ae Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 14:06:17 -0800 Subject: [PATCH 04/10] 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 5c72c5b24b..044e3d3543 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -152,14 +152,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 } @@ -168,7 +168,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"` @@ -179,13 +179,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 2cd44d078c439ee8ec341bd0b49f356b2fe86b50 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 15:25:07 -0800 Subject: [PATCH 05/10] 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 044e3d3543..188c61c40c 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -137,6 +137,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 { return api.NewClient(api.ClientOpts{ @@ -168,24 +177,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 e2b8d62b7b7dcd50ac859d7cfb0ed8ceeaa0ed50 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 16:16:46 -0800 Subject: [PATCH 06/10] 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 23d6f0c8a98cf712f0a398a0ad4142fcb63c2084 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 16:17:20 -0800 Subject: [PATCH 07/10] 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 188c61c40c..3e48d35e13 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -138,7 +138,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"` @@ -317,7 +317,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 b7faaa335bd9f44bbaf46beca12bf848e6b67ba3 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 16:17:59 -0800 Subject: [PATCH 08/10] 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 a1620bdb6f8eeb918585620d92d4a609364df0bf Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 16:18:29 -0800 Subject: [PATCH 09/10] 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 247573413e99a034aaeb9bdd07c4209c956e4133 Mon Sep 17 00:00:00 2001 From: Peter Guy Date: Sat, 7 Mar 2026 17:14:24 -0800 Subject: [PATCH 10/10] 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 3e48d35e13..547b6f5d58 100644 --- a/cmd/src/main.go +++ b/cmd/src/main.go @@ -6,7 +6,6 @@ import ( "io" "log" "net" - "net/http" "net/url" "os" "path/filepath" @@ -278,14 +277,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()