From d31bf3551c855ee7f3cf02b5fd9aa0efbd2d3508 Mon Sep 17 00:00:00 2001 From: Sebastian Date: Wed, 7 Oct 2026 20:03:13 +0200 Subject: [PATCH] Allow CORS origins from CORS_ALLOWED_ORIGINS Echo compares allowed origins literally, so "https://*" and "http://*" never matched a real origin and no CORS headers were sent. The allowed origins now come from CORS_ALLOWED_ORIGINS (comma-separated, checked at startup). When it is empty, only same-origin requests work, which is enough for the bundled frontend. Credentials are no longer allowed since the API uses Bearer tokens. Browsers may now send Range and X-Queue-Id and read X-Queue-Id and Content-Range. --- internal/config/config.go | 31 +++++++++++++++++++++++++++++++ internal/config/config_test.go | 16 ++++++++++++++++ internal/server/routes.go | 19 +++++++++++-------- internal/server/routes_test.go | 27 +++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 8 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 3828113..d19b962 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "net" + "net/url" "os" "strconv" "strings" @@ -38,6 +39,10 @@ type Config struct { // TrustedProxies are CIDR ranges whose X-Forwarded-For header is trusted when resolving client IPs. TrustedProxies []*net.IPNet + // CORSAllowedOrigins are the browser origins, like https://music.example.com, that may call the API + // from another site. Empty means only same-origin requests, which is enough for the bundled frontend. + CORSAllowedOrigins []string + ReleasesURL string } @@ -91,6 +96,11 @@ func load(getenv func(string) string) (Config, error) { errs = append(errs, fmt.Errorf("TRUSTED_PROXIES: %w", err)) } + cfg.CORSAllowedOrigins, err = parseOrigins(getenv("CORS_ALLOWED_ORIGINS")) + if err != nil { + errs = append(errs, fmt.Errorf("CORS_ALLOWED_ORIGINS: %w", err)) + } + return cfg, errors.Join(errs...) } @@ -110,6 +120,27 @@ func parseCIDRs(value string) ([]*net.IPNet, error) { return nets, nil } +// parseOrigins parses a comma-separated list of origins (scheme://host[:port], nothing else). +func parseOrigins(value string) ([]string, error) { + var origins []string + for part := range strings.SplitSeq(value, ",") { + part = strings.TrimSpace(part) + if part == "" { + continue + } + u, err := url.Parse(part) + if err != nil { + return nil, err + } + if (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" || u.Path != "" || u.RawQuery != "" || + u.Fragment != "" || u.User != nil || strings.Contains(u.Host, "*") { + return nil, fmt.Errorf("%q is not an origin like https://music.example.com", part) + } + origins = append(origins, u.Scheme+"://"+u.Host) + } + return origins, nil +} + func withDefault(value, fallback string) string { if value == "" { return fallback diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 4098642..63c9583 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -31,6 +31,7 @@ func TestLoadDefaults(t *testing.T) { assert.Equal(t, "songs", cfg.SoundCheckPath) assert.Equal(t, DefaultReleasesURL, cfg.ReleasesURL) assert.Empty(t, cfg.TrustedProxies) + assert.Empty(t, cfg.CORSAllowedOrigins) } func TestLoadMissingRequired(t *testing.T) { @@ -56,6 +57,21 @@ func TestLoadTrustedProxies(t *testing.T) { assert.ErrorContains(t, err, "TRUSTED_PROXIES") } +func TestLoadCORSAllowedOrigins(t *testing.T) { + values := validEnv() + values["CORS_ALLOWED_ORIGINS"] = "https://music.example.com, http://localhost:5173," + cfg, err := load(env(values)) + require.NoError(t, err) + assert.Equal(t, []string{"https://music.example.com", "http://localhost:5173"}, cfg.CORSAllowedOrigins) + + for _, invalid := range []string{"music.example.com", "ftp://example.com", "https://example.com/app", + "https://*.example.com", "https://*", "*", "https://example.com?x=1", "https://user@example.com"} { + values["CORS_ALLOWED_ORIGINS"] = invalid + _, err = load(env(values)) + assert.ErrorContains(t, err, "CORS_ALLOWED_ORIGINS", invalid) + } +} + func TestLoadInvalidPort(t *testing.T) { values := validEnv() values["PORT"] = "abc" diff --git a/internal/server/routes.go b/internal/server/routes.go index 8e654fe..9602522 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -40,14 +40,17 @@ func (s *Server) routes() (*echo.Echo, huma.API) { e.Use(logging.RequestLogger()) e.Use(echoMiddleware.Recover()) - e.Use(echoMiddleware.CORSWithConfig(echoMiddleware.CORSConfig{ - AllowOrigins: []string{"https://*", "http://*"}, - AllowMethods: []string{"GET", "POST", "PUT", "DELETE", "OPTIONS", "PATCH"}, - AllowHeaders: []string{"Accept", "Authorization", "Content-Type", "X-CSRF-Token", middleware.QueueHeader}, - ExposeHeaders: []string{middleware.QueueHeader}, - AllowCredentials: true, - MaxAge: 300, - })) + // Without allowed origins no CORS headers are sent, so only same-origin browser requests work. + // The API uses Bearer tokens, not cookies, so credentials are not allowed. + if len(s.cfg.CORSAllowedOrigins) > 0 { + e.Use(echoMiddleware.CORSWithConfig(echoMiddleware.CORSConfig{ + AllowOrigins: s.cfg.CORSAllowedOrigins, + AllowMethods: []string{"GET", "POST", "PUT", "DELETE", "OPTIONS", "PATCH"}, + AllowHeaders: []string{"Accept", "Authorization", "Content-Type", "Range", middleware.QueueHeader}, + ExposeHeaders: []string{middleware.QueueHeader, "Content-Range", "Accept-Ranges"}, + MaxAge: 300, + })) + } // The API: /api/v1 is documented at /docs and /openapi.json. a := newAPI(e) diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go index ba51978..97b6142 100644 --- a/internal/server/routes_test.go +++ b/internal/server/routes_test.go @@ -196,3 +196,30 @@ func TestTrustedProxies(t *testing.T) { assert.Equal(t, http.StatusTooManyRequests, e.doBody(http.MethodPost, "/api/v1/token", `{}`, "X-Forwarded-For", "10.0.0.1").Code) assert.Equal(t, http.StatusOK, e.doBody(http.MethodPost, "/api/v1/token", `{}`, "X-Forwarded-For", "10.0.0.2").Code) } + +func TestCORSAllowedOrigins(t *testing.T) { + e := newTestEnv(t, func(c *config.Config) { c.CORSAllowedOrigins = []string{"https://music.example.com"} }) + + rec := e.do(http.MethodGet, "/api/v1/music/list", "Origin", "https://music.example.com") + assert.Equal(t, http.StatusOK, rec.Code) + assert.Equal(t, "https://music.example.com", rec.Header().Get("Access-Control-Allow-Origin")) + assert.Contains(t, rec.Header().Get("Access-Control-Expose-Headers"), "X-Queue-Id") + assert.Empty(t, rec.Header().Get("Access-Control-Allow-Credentials")) + + preflight := e.do(http.MethodOptions, "/api/v1/music/list", "Origin", "https://music.example.com", + "Access-Control-Request-Method", "GET", "Access-Control-Request-Headers", "authorization,x-queue-id") + assert.Equal(t, http.StatusNoContent, preflight.Code) + assert.Equal(t, "https://music.example.com", preflight.Header().Get("Access-Control-Allow-Origin")) + assert.Contains(t, preflight.Header().Get("Access-Control-Allow-Headers"), "X-Queue-Id") + assert.Contains(t, preflight.Header().Get("Access-Control-Allow-Headers"), "Authorization") + + other := e.do(http.MethodGet, "/api/v1/music/list", "Origin", "https://evil.example.com") + assert.Empty(t, other.Header().Get("Access-Control-Allow-Origin"), "other origins get no CORS headers") +} + +func TestCORSWithoutAllowedOrigins(t *testing.T) { + e := newTestEnv(t) + rec := e.do(http.MethodGet, "/api/v1/version", "Origin", "https://music.example.com") + assert.Equal(t, http.StatusOK, rec.Code, "the request itself works; the browser blocks reading it") + assert.Empty(t, rec.Header().Get("Access-Control-Allow-Origin")) +}