Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -292,3 +292,4 @@ save-to-spotify list shows
| `SAVE_TO_SPOTIFY_TIMEOUT` | API request timeout (e.g. `30s`, `2m`) | `30s` |
| `SAVE_TO_SPOTIFY_CLIENT_ID` | OAuth client ID override | built-in |
| `SAVE_TO_SPOTIFY_NO_UPDATE_CHECK` | Disable passive update checks | off |
| `SAVE_TO_SPOTIFY_HEADERS` | Additional headers for backend requests (JSON object of header name/value pairs; only `X-STS-*` headers are accepted) | — |
14 changes: 12 additions & 2 deletions cmd/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,22 @@ func cliUserAgent() string {
return fmt.Sprintf("%s/%s %s/%s", binName, uaVersion, runtime.GOOS, runtime.GOARCH)
}

// backendTransport applies the CLI User-Agent to all requests and the
// additional backend headers (config.AdditionalHeaders) to requests targeting
// the backend host, delegating to http.DefaultTransport.
func backendTransport() http.RoundTripper {
return httpx.BackendHeadersTransport{
Base: httpx.UserAgentTransport{UserAgent: cliUserAgent()},
Headers: config.AdditionalHeaders,
BackendURL: func() string { return config.BackendBaseURL },
}
}

// httpClient is the shared HTTP client
// Its Timeout is set in Execute() after flag parsing (default: 30s, override via --timeout or config.EnvVarTimeout).
// The transport applies the CLI User-Agent while still delegating to http.DefaultTransport by default.
var httpClient = &http.Client{
Timeout: config.APITimeout(),
Transport: httpx.UserAgentTransport{UserAgent: cliUserAgent()},
Transport: backendTransport(),
}

// uploadClient is used for signed GCS PUTs. No timeout — large files can take many minutes.
Expand Down
98 changes: 98 additions & 0 deletions cmd/client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package cmd

import (
"context"
"net/http"
"net/http/httptest"
"testing"

"github.com/spotify/save-to-spotify/config"
)

// startBackendTestServer starts a test server that records request headers.
// If asBackend is true, config.BackendBaseURL is pointed at it so the
// additional-headers transport treats it as the backend host.
func startBackendTestServer(t *testing.T, asBackend bool) (*httptest.Server, *http.Header) {
t.Helper()

var gotHeaders http.Header
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
gotHeaders = r.Header
w.WriteHeader(200)
}))
t.Cleanup(srv.Close)

if asBackend {
origURL := config.BackendBaseURL
config.BackendBaseURL = srv.URL
t.Cleanup(func() { config.BackendBaseURL = origURL })
}

return srv, &gotHeaders
}

func setAdditionalHeaders(t *testing.T, headers map[string]string) {
t.Helper()
orig := config.AdditionalHeaders()
config.SetAdditionalHeaders(headers)
t.Cleanup(func() { config.SetAdditionalHeaders(orig) })
}

func doTestAPIRequest(t *testing.T, srv *httptest.Server) {
t.Helper()
req, _ := http.NewRequestWithContext(context.Background(), "GET", srv.URL+"/api/v1/shows", nil)
resp, err := doAPIRequest(req, &config.TokenData{AccessToken: "test-token"})
if err != nil {
t.Fatalf("doAPIRequest: %v", err)
}
resp.Body.Close()
}

func TestDoAPIRequest_NoAdditionalHeaders(t *testing.T) {
setAdditionalHeaders(t, nil)
srv, gotHeaders := startBackendTestServer(t, true)
doTestAPIRequest(t, srv)

if got := gotHeaders.Get("Authorization"); got != "Bearer test-token" {
t.Errorf("Authorization = %q, want %q", got, "Bearer test-token")
}
if got := gotHeaders.Get("X-STS-Test"); got != "" {
t.Errorf("X-STS-Test should be absent, got %q", got)
}
}

func TestDoAPIRequest_WithAdditionalHeaders(t *testing.T) {
setAdditionalHeaders(t, map[string]string{"X-STS-Test": "1"})
srv, gotHeaders := startBackendTestServer(t, true)
doTestAPIRequest(t, srv)

if got := gotHeaders.Get("Authorization"); got != "Bearer test-token" {
t.Errorf("Authorization = %q, want %q", got, "Bearer test-token")
}
if got := gotHeaders.Get("X-STS-Test"); got != "1" {
t.Errorf("X-STS-Test = %q, want %q", got, "1")
}
}

func TestDoAPIRequest_HeadersScopedToBackendHost(t *testing.T) {
setAdditionalHeaders(t, map[string]string{"X-STS-Test": "1"})
srv, gotHeaders := startBackendTestServer(t, false)
doTestAPIRequest(t, srv)

if got := gotHeaders.Get("X-STS-Test"); got != "" {
t.Errorf("X-STS-Test should not be sent to non-backend host, got %q", got)
}
}

func TestDoAPIRequest_InvalidHeaderValueDoesNotBreakRequests(t *testing.T) {
setAdditionalHeaders(t, map[string]string{"X-STS-Bad": "a\nb", "X-STS-Ok": "1"})
srv, gotHeaders := startBackendTestServer(t, true)
doTestAPIRequest(t, srv)

if got := gotHeaders.Get("X-STS-Bad"); got != "" {
t.Errorf("X-STS-Bad should have been dropped, got %q", got)
}
if got := gotHeaders.Get("X-STS-Ok"); got != "1" {
t.Errorf("X-STS-Ok = %q, want %q", got, "1")
}
}
1 change: 1 addition & 0 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,5 +179,6 @@ Environment variables:
SAVE_TO_SPOTIFY_NO_UPDATE_CHECK Disable the passive update check that runs after successful commands
SAVE_TO_SPOTIFY_RELEASES_URL Override the releases download URL
SAVE_TO_SPOTIFY_RELEASES_API_URL Override the version check URL
SAVE_TO_SPOTIFY_HEADERS Additional backend headers (JSON object, X-STS-* only)
`, binName)
}
3 changes: 1 addition & 2 deletions cmd/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import (
"time"

"github.com/spotify/save-to-spotify/config"
"github.com/spotify/save-to-spotify/internal/httpx"
)

var (
Expand Down Expand Up @@ -148,7 +147,7 @@ func fetchLatestVersion() (latestReleaseResponse, error) {

resp, err := (&http.Client{
Timeout: releaseMetadataTimeout,
Transport: httpx.UserAgentTransport{UserAgent: cliUserAgent()},
Transport: backendTransport(),
}).Do(req)
if err != nil {
return latestReleaseResponse{}, fmt.Errorf("failed to check for updates: %w", err)
Expand Down
97 changes: 97 additions & 0 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ import (
"errors"
"fmt"
"io/fs"
"net/textproto"
"net/url"
"os"
"path/filepath"
"sort"
"strings"
"time"
)
Expand All @@ -23,6 +25,7 @@ const (
EnvVarClientID = "SAVE_TO_SPOTIFY_CLIENT_ID"
EnvVarNoUpdateCheck = "SAVE_TO_SPOTIFY_NO_UPDATE_CHECK"
EnvVarReleasesAPIURL = "SAVE_TO_SPOTIFY_RELEASES_API_URL"
EnvVarHeaders = "SAVE_TO_SPOTIFY_HEADERS"

Scopes = "sts-content-management"

Expand Down Expand Up @@ -285,3 +288,97 @@ func GetClientID() string {
}
return ClientID
}

// additionalHeaderPrefix is the canonical MIME form of the required "X-STS-"
// header prefix. Keys are canonicalized before the prefix check, so any casing
// of X-STS-* is accepted.
const additionalHeaderPrefix = "X-Sts-"

var additionalHeaders = parseAdditionalHeaders()

// AdditionalHeaders returns extra HTTP headers to send on backend API requests.
// Parsed from SAVE_TO_SPOTIFY_HEADERS, a JSON object of header name/value
// pairs. Keys are in canonical MIME form. Only X-STS-* headers (any casing)
// with valid HTTP header names and values are accepted; others are silently
// dropped.
func AdditionalHeaders() map[string]string { return additionalHeaders }

// SetAdditionalHeaders replaces the additional backend headers, applying the
// same X-STS-* validation as env parsing. Used in tests.
func SetAdditionalHeaders(h map[string]string) { additionalHeaders = filterAdditionalHeaders(h) }

func parseAdditionalHeaders() map[string]string {
raw := os.Getenv(EnvVarHeaders)
if raw == "" {
return nil
}

var entries map[string]string
if err := json.Unmarshal([]byte(raw), &entries); err != nil {
fmt.Fprintf(os.Stderr, "Warning: %s contains invalid JSON, ignoring: %v\n", EnvVarHeaders, err)
return nil

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this would happen when the SAVE_TO_SPOTIFY_HEADERS var is set but contains invalid json, would be good to log when that happens

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call — added a warning to stderr when the JSON is malformed: Warning: SAVE_TO_SPOTIFY_HEADERS contains invalid JSON, ignoring: <err>

}
return filterAdditionalHeaders(entries)
}

// filterAdditionalHeaders canonicalizes keys and keeps only valid X-STS-*
// headers. Keys are visited in sorted order so two spellings of the same
// header collapse deterministically.
func filterAdditionalHeaders(in map[string]string) map[string]string {
keys := make([]string, 0, len(in))
for k := range in {
keys = append(keys, k)
}
sort.Strings(keys)

headers := make(map[string]string)
for _, k := range keys {
key := textproto.CanonicalMIMEHeaderKey(strings.TrimSpace(k))
val := strings.TrimSpace(in[k])
if val == "" || !strings.HasPrefix(key, additionalHeaderPrefix) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The X-STS-* prefix check is critical, without it a value like {"Authorization": "Bearer ..."} in SAVE_TO_SPOTIFY_HEADERS would silently overwrite the CLI's own auth header, since the transport runs after doAPIRequest sets Authorization.

continue
}
if !isValidHeaderName(key) || !isValidHeaderValue(val) {
continue
}
headers[key] = val
}
if len(headers) == 0 {
return nil
}
return headers
}

// isValidHeaderName reports whether s is a valid RFC 7230 header field name
// (a token). CanonicalMIMEHeaderKey returns invalid names unchanged, so names
// it could not canonicalize are rejected here.
func isValidHeaderName(s string) bool {
if s == "" {
return false
}
for i := 0; i < len(s); i++ {
c := s[i]
switch {
case 'a' <= c && c <= 'z', 'A' <= c && c <= 'Z', '0' <= c && c <= '9':
case c == '!' || c == '#' || c == '$' || c == '%' || c == '&' || c == '\'' ||
c == '*' || c == '+' || c == '-' || c == '.' || c == '^' || c == '_' ||
c == '`' || c == '|' || c == '~':
default:
return false
}
}
return true
}

// isValidHeaderValue reports whether s is a valid RFC 7230 header field value:
// no control characters other than horizontal tab. A value that fails this
// check would make net/http reject every request it is attached to.
func isValidHeaderValue(s string) bool {
for i := 0; i < len(s); i++ {
c := s[i]
if (c < 0x20 && c != '\t') || c == 0x7f {
return false
}
}
return true
}
99 changes: 99 additions & 0 deletions config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,102 @@ func TestDPoPKeyPath(t *testing.T) {
t.Errorf("DPoPKeyPath = %q, want %q", path, want)
}
}

func setHeadersEnv(t *testing.T, val string) {
t.Helper()
t.Setenv(EnvVarHeaders, val)
orig := additionalHeaders
additionalHeaders = parseAdditionalHeaders()
t.Cleanup(func() { additionalHeaders = orig })
}

func TestParseAdditionalHeaders(t *testing.T) {
setHeadersEnv(t, `{"X-STS-Test":"true","X-STS-Foo":"bar"}`)

if got := AdditionalHeaders()["X-Sts-Test"]; got != "true" {
t.Errorf("X-Sts-Test = %q, want %q", got, "true")
}
if got := AdditionalHeaders()["X-Sts-Foo"]; got != "bar" {
t.Errorf("X-Sts-Foo = %q, want %q", got, "bar")
}
}

func TestParseAdditionalHeaders_Empty(t *testing.T) {
setHeadersEnv(t, "")

if AdditionalHeaders() != nil {
t.Errorf("expected nil, got %v", AdditionalHeaders())
}
}

func TestParseAdditionalHeaders_InvalidJSON(t *testing.T) {
setHeadersEnv(t, "not json")

if AdditionalHeaders() != nil {
t.Errorf("expected nil for invalid JSON, got %v", AdditionalHeaders())
}
}

func TestParseAdditionalHeaders_RejectsNonSTS(t *testing.T) {
setHeadersEnv(t, `{"X-STS-Test":"true","X-Custom":"bad","Authorization":"evil"}`)

if got := AdditionalHeaders()["X-Sts-Test"]; got != "true" {
t.Errorf("X-Sts-Test = %q, want %q", got, "true")
}
if len(AdditionalHeaders()) != 1 {
t.Errorf("expected 1 header, got %d: %v", len(AdditionalHeaders()), AdditionalHeaders())
}
}

func TestParseAdditionalHeaders_CanonicalizesAnyCasing(t *testing.T) {
setHeadersEnv(t, `{"x-sts-trace-id":"123"}`)

if got := AdditionalHeaders()["X-Sts-Trace-Id"]; got != "123" {
t.Errorf("X-Sts-Trace-Id = %q, want %q", got, "123")
}
}

func TestParseAdditionalHeaders_CaseCollisionIsDeterministic(t *testing.T) {
setHeadersEnv(t, `{"X-STS-TEST":"upper","X-STS-Test":"mixed"}`)

if len(AdditionalHeaders()) != 1 {
t.Fatalf("expected 1 header, got %v", AdditionalHeaders())
}
// Keys are filtered in sorted order, so the last sorted spelling wins.
if got := AdditionalHeaders()["X-Sts-Test"]; got != "mixed" {
t.Errorf("X-Sts-Test = %q, want %q", got, "mixed")
}
}

func TestParseAdditionalHeaders_RejectsInvalidName(t *testing.T) {
setHeadersEnv(t, `{"X-Sts-Trace Id":"abc"}`)

if AdditionalHeaders() != nil {
t.Errorf("expected nil for invalid header name, got %v", AdditionalHeaders())
}
}

func TestParseAdditionalHeaders_RejectsInvalidValue(t *testing.T) {
setHeadersEnv(t, `{"X-STS-Test":"a\nb"}`)

if AdditionalHeaders() != nil {
t.Errorf("expected nil for invalid header value, got %v", AdditionalHeaders())
}
}

func TestSetAdditionalHeaders_AppliesFiltering(t *testing.T) {
orig := additionalHeaders
t.Cleanup(func() { additionalHeaders = orig })

SetAdditionalHeaders(map[string]string{
"x-sts-test": "1",
"Authorization": "evil",
})

if got := AdditionalHeaders()["X-Sts-Test"]; got != "1" {
t.Errorf("X-Sts-Test = %q, want %q", got, "1")
}
if len(AdditionalHeaders()) != 1 {
t.Errorf("expected 1 header, got %v", AdditionalHeaders())
}
}
Loading
Loading