From bf24034c32272b5a7697e7238977d3f9d16f0684 Mon Sep 17 00:00:00 2001 From: matiasdaloia <66310421+matiasdaloia@users.noreply.github.com> Date: Wed, 21 Oct 2026 12:00:29 +0200 Subject: [PATCH 1/2] fix(scanoss): drop the API key on a cross-host redirect net/http strips only its own auth and cookie headers when a redirect changes host, so a 3xx from the API sent x-api-key, x-Session and every Config.Headers entry to the host it named. New now wraps the HTTP client, the SDK's or the caller's, in a copy whose CheckRedirect deletes those headers once a redirect leaves the API's host:port. Same-host redirects keep them, and a caller's own CheckRedirect still runs. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 9 ++++ pkg/scanoss/client.go | 5 +++ pkg/scanoss/config.go | 23 ++++++++++ pkg/scanoss/identification_test.go | 69 ++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b39c75..cc0a9fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Security + +- **A redirect to another host no longer receives the API key.** Go drops only + `Authorization`, `Cookie` and `WWW-Authenticate` when a redirect changes host, so a 3xx + from the API sent `x-api-key`, `x-Session` and every `Config.Headers` entry to the host it + named. The SDK now removes them whenever a redirect leaves the API's host and port, + including through a caller's `Config.HTTPClient`, whose own `CheckRedirect` still applies. + Same-host redirects keep them. + ## [0.14.0] - 2026-10-21 ### Added diff --git a/pkg/scanoss/client.go b/pkg/scanoss/client.go index 326d216..678ed24 100644 --- a/pkg/scanoss/client.go +++ b/pkg/scanoss/client.go @@ -100,6 +100,11 @@ func New(cfg Config) (*Client, error) { if err != nil { return nil, err } + identifying := []string{"X-Api-Key", "X-Session"} + for name := range cfg.Headers { + identifying = append(identifying, name) + } + httpClient = dropOnHostChange(httpClient, identifying) apiURL := DefaultAPIURL if cfg.APIURL != "" { apiURL = strings.TrimRight(cfg.APIURL, "/") diff --git a/pkg/scanoss/config.go b/pkg/scanoss/config.go index bb67082..9fe0ba7 100644 --- a/pkg/scanoss/config.go +++ b/pkg/scanoss/config.go @@ -155,6 +155,29 @@ func (cfg Config) httpClient() (*http.Client, error) { }) } +// net/http strips only its built-in auth and cookie headers on a cross-host redirect and +// forwards every other header, X-Api-Key included. The 10-hop cap mirrors net/http's +// unexported defaultCheckRedirect. +func dropOnHostChange(client *http.Client, names []string) *http.Client { + wrapped := *client + next := client.CheckRedirect + wrapped.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if req.URL.Host != via[0].URL.Host { + for _, name := range names { + req.Header.Del(name) + } + } + if next != nil { + return next(req, via) + } + if len(via) >= 10 { + return errors.New("stopped after 10 redirects") + } + return nil + } + return &wrapped +} + // retryCount resolves Config.MaxRetries: unset takes the default, and a negative value // disables retries — the convention Timeout already uses in this Config. func retryCount(n int) int { diff --git a/pkg/scanoss/identification_test.go b/pkg/scanoss/identification_test.go index c14613d..0508911 100644 --- a/pkg/scanoss/identification_test.go +++ b/pkg/scanoss/identification_test.go @@ -149,3 +149,72 @@ func TestHTTPClientRejectsTransportSettings(t *testing.T) { }) } } + +var identifyingHeaders = []string{"X-Api-Key", "X-Session", "X-Scanoss-Client-Instance"} + +func redirectServer(t *testing.T, target func(self string) string) (*httptest.Server, func() http.Header) { + t.Helper() + var got http.Header + var srv *httptest.Server + srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/final" { + got = r.Header.Clone() + _, _ = w.Write([]byte("{}")) + return + } + http.Redirect(w, r, target(srv.URL), http.StatusTemporaryRedirect) + })) + t.Cleanup(srv.Close) + return srv, func() http.Header { return got } +} + +func identifyingConfig(apiURL string, client *http.Client) Config { + return Config{ + APIURL: apiURL, + APIKey: "secret", + Headers: http.Header{"X-Scanoss-Client-Instance": {"0192f3c4-7b1e-7c3a-9a55-5f0c2f9f6a10"}}, + HTTPClient: client, + } +} + +func TestCrossHostRedirectDropsIdentity(t *testing.T) { + for name, client := range map[string]*http.Client{"sdk client": nil, "caller's client": {}} { + t.Run(name, func(t *testing.T) { + other, otherHeaders := captureServer(t) + api, _ := redirectServer(t, func(string) string { return other.URL }) + callAPI(t, mustNew(t, identifyingConfig(api.URL, client))) + got := otherHeaders() + if got == nil { + t.Fatal("redirect target saw no request") + } + for _, h := range identifyingHeaders { + if v := got.Get(h); v != "" { + t.Errorf("%s = %q reached the other host", h, v) + } + } + }) + } +} + +func TestSameHostRedirectKeepsIdentity(t *testing.T) { + api, final := redirectServer(t, func(self string) string { return self + "/final" }) + callAPI(t, mustNew(t, identifyingConfig(api.URL, nil))) + got := final() + if got == nil { + t.Fatal("redirect target saw no request") + } + for _, h := range identifyingHeaders { + if got.Get(h) == "" { + t.Errorf("%s dropped on a same-host redirect", h) + } + } +} + +func TestRedirectHonorsCallersPolicy(t *testing.T) { + api, final := redirectServer(t, func(self string) string { return self + "/final" }) + client := &http.Client{CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }} + _, err := mustNew(t, identifyingConfig(api.URL, client)).Components.Versions(context.Background(), "pkg:github/scanoss/engine", 1) + if err == nil || final() != nil { + t.Errorf("redirect followed despite the caller's CheckRedirect (err = %v)", err) + } +} From 287cfa5e19c17c7f65092202edde43270a501e69 Mon Sep 17 00:00:00 2001 From: matiasdaloia <66310421+matiasdaloia@users.noreply.github.com> Date: Wed, 21 Oct 2026 12:11:48 +0200 Subject: [PATCH 2/2] fix(scanoss): drop the API key on a downgrade or a return hop The redirect check compared only URL.Host against the first request. An https to http redirect on the same host kept x-api-key and sent it in clear, and a chain that left the API and came back got the key again, because net/http re-copies the original headers on every hop. Keep the identifying headers only while every hop shares the API's hostname (case-insensitive) and effective port, allowing an http to https upgrade on the default ports. An explicit :443 now counts as the same origin. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 +- pkg/scanoss/config.go | 43 ++++++++++++++- pkg/scanoss/redirect_test.go | 103 +++++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 4 deletions(-) create mode 100644 pkg/scanoss/redirect_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index cc0a9fe..bac1175 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 from the API sent `x-api-key`, `x-Session` and every `Config.Headers` entry to the host it named. The SDK now removes them whenever a redirect leaves the API's host and port, including through a caller's `Config.HTTPClient`, whose own `CheckRedirect` still applies. - Same-host redirects keep them. + Same-host redirects keep them, as does an upgrade from `http` to `https`. A downgrade to + `http`, or any later hop once a redirect has left the API, drops them. ## [0.14.0] - 2026-10-21 diff --git a/pkg/scanoss/config.go b/pkg/scanoss/config.go index 9fe0ba7..f8e56c7 100644 --- a/pkg/scanoss/config.go +++ b/pkg/scanoss/config.go @@ -27,6 +27,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "strings" "time" @@ -156,13 +157,14 @@ func (cfg Config) httpClient() (*http.Client, error) { } // net/http strips only its built-in auth and cookie headers on a cross-host redirect and -// forwards every other header, X-Api-Key included. The 10-hop cap mirrors net/http's -// unexported defaultCheckRedirect. +// forwards every other header, X-Api-Key included. It also re-copies the original headers +// on every hop, so a chain that leaves the API and comes back would restore them unless +// every hop is checked. The 10-hop cap mirrors net/http's unexported defaultCheckRedirect. func dropOnHostChange(client *http.Client, names []string) *http.Client { wrapped := *client next := client.CheckRedirect wrapped.CheckRedirect = func(req *http.Request, via []*http.Request) error { - if req.URL.Host != via[0].URL.Host { + if leftOrigin(req, via) { for _, name := range names { req.Header.Del(name) } @@ -178,6 +180,41 @@ func dropOnHostChange(client *http.Client, names []string) *http.Client { return &wrapped } +func leftOrigin(req *http.Request, via []*http.Request) bool { + if !sameOrigin(via[0].URL, req.URL) { + return true + } + for _, hop := range via[1:] { + if !sameOrigin(via[0].URL, hop.URL) { + return true + } + } + return false +} + +// sameOrigin reports whether a redirect from api to dest may carry the API key. An upgrade +// from http to https on the default ports keeps it; a downgrade would send it in clear. +func sameOrigin(api, dest *url.URL) bool { + if !strings.EqualFold(api.Hostname(), dest.Hostname()) { + return false + } + if api.Scheme == dest.Scheme { + return effectivePort(api) == effectivePort(dest) + } + return api.Scheme == "http" && dest.Scheme == "https" && + effectivePort(api) == "80" && effectivePort(dest) == "443" +} + +func effectivePort(u *url.URL) string { + if p := u.Port(); p != "" { + return p + } + if u.Scheme == "https" { + return "443" + } + return "80" +} + // retryCount resolves Config.MaxRetries: unset takes the default, and a negative value // disables retries — the convention Timeout already uses in this Config. func retryCount(n int) int { diff --git a/pkg/scanoss/redirect_test.go b/pkg/scanoss/redirect_test.go new file mode 100644 index 0000000..dfc62cd --- /dev/null +++ b/pkg/scanoss/redirect_test.go @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: MIT + +package scanoss + +import ( + "context" + "crypto/tls" + "net" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// hops serves api.test on ports 80, 443 and 8443, and other.test, each from a real +// httptest server. A request to /to/ redirects there; any other request records its +// headers under " ". +type hops struct { + seen map[string]http.Header + client *http.Client +} + +func newHops(t *testing.T) *hops { + t.Helper() + h := &hops{seen: map[string]http.Header{}} + handler := func(scheme string) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + if to, ok := strings.CutPrefix(r.URL.Path, "/to/"); ok { + http.Redirect(w, r, to, http.StatusTemporaryRedirect) + return + } + h.seen[scheme+" "+r.URL.Path] = r.Header.Clone() + _, _ = w.Write([]byte("{}")) + } + } + plain := httptest.NewServer(handler("http")) + secure := httptest.NewTLSServer(handler("https")) + t.Cleanup(plain.Close) + t.Cleanup(secure.Close) + addr := map[string]string{ + "api.test:80": plain.Listener.Addr().String(), + "api.test:443": secure.Listener.Addr().String(), + "api.test:8443": secure.Listener.Addr().String(), + "other.test:443": secure.Listener.Addr().String(), + } + var d net.Dialer + h.client = &http.Client{Transport: &http.Transport{ + TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, + DialContext: func(ctx context.Context, network, a string) (net.Conn, error) { + if mapped, ok := addr[strings.ToLower(a)]; ok { + a = mapped + } + return d.DialContext(ctx, network, a) + }, + }} + return h +} + +func TestRedirectIdentityByOrigin(t *testing.T) { + tests := []struct { + name, apiURL, redirect, final string + keep bool + }{ + {"https to http on the same host drops it", "https://api.test", "http://api.test/final", "http /final", false}, + {"http to https on the same host keeps it", "http://api.test", "https://api.test/final", "https /final", true}, + {"an explicit default port is the same origin", "https://api.test", "https://api.test:443/final", "https /final", true}, + {"hostname case is ignored", "https://api.test", "https://API.test/final", "https /final", true}, + {"another port drops it", "https://api.test", "https://api.test:8443/final", "https /final", false}, + {"another host drops it", "https://api.test", "https://other.test/final", "https /final", false}, + {"leaving and coming back does not restore it", "https://api.test", + "https://other.test/to/https://api.test/final", "https /final", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + h := newHops(t) + c := mustNew(t, identifyingConfig(tt.apiURL, h.client)) + req, err := c.newRequest(http.MethodGet, "/to/"+tt.redirect, nil) + if err != nil { + t.Fatal(err) + } + if _, err := c.transport.do(context.Background(), req); err != nil { + t.Fatalf("do: %v", err) + } + got, ok := h.seen[tt.final] + if !ok { + t.Fatalf("%s saw no request", tt.redirect) + } + for _, name := range identifyingHeaders { + if kept := got.Get(name) != ""; kept != tt.keep { + t.Errorf("%s kept = %v, want %v", name, kept, tt.keep) + } + } + }) + } +} + +func TestNewLeavesCallersClientUnchanged(t *testing.T) { + client := &http.Client{} + _ = mustNew(t, identifyingConfig("https://api.test", client)) + if client.CheckRedirect != nil { + t.Error("New installed a CheckRedirect on the caller's client") + } +}