Allow plugin settings to add connect-src CSP sources

internal/api/server.go

@@ -9,12 +9,15 @@ import (
 	"io"
 	"io/fs"
 	"net/http"
+	"net/url"
 	"os"
 	"path"
 	"path/filepath"
 	"runtime/debug"
+	"sort"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	gqlHandler "github.com/99designs/gqlgen/graphql/handler"
@@ -552,6 +555,60 @@ func isURL(s string) bool {
 	return strings.HasPrefix(s, "http://") || strings.HasPrefix(s, "https://")
 }
 
+const cspSettingPrefix = "csp_"
+
+// cspConnectSrcFromSettings returns validated http(s) connect-src URLs from
+// the plugin's settings, plus the keys that were skipped as invalid.
+// Settings keys beginning with "csp_" are treated as connect-src sources.
+func cspConnectSrcFromSettings(settings map[string]interface{}) (valid []string, skipped map[string]string) {
+	for k, v := range settings {
+		if !strings.HasPrefix(k, cspSettingPrefix) {
+			continue
+		}
+		s, ok := v.(string)
+		if !ok || !isValidConnectSrcURL(s) {
+			if skipped == nil {
+				skipped = make(map[string]string)
+			}
+			skipped[k] = fmt.Sprintf("%v", v)
+			continue
+		}
+		valid = append(valid, s)
+	}
+
+	// settings is a map, so sort to keep the emitted header stable between requests
+	sort.Strings(valid)
+
+	return valid, skipped
+}
+
+// warnedCSPSettings tracks the invalid csp_ settings already logged, so that a
+// misconfigured plugin does not emit a warning on every page request. The value
+// is re-logged if the user changes the setting to another invalid value.
+var warnedCSPSettings sync.Map
+
+func warnInvalidCSPSettings(pluginID string, skipped map[string]string) {
+	for key, value := range skipped {
+		k := pluginID + "\x00" + key
+		if prev, ok := warnedCSPSettings.Load(k); ok && prev == value {
+			continue
+		}
+		warnedCSPSettings.Store(k, value)
+		logger.Warnf("plugin %q: ignoring setting %q: not a valid connect-src URL", pluginID, key)
+	}
+}
+
+func isValidConnectSrcURL(s string) bool {
+	if strings.ContainsAny(s, " ,\t\r\n;\"'") {
+		return false
+	}
+	u, err := url.Parse(s)
+	if err != nil {
+		return false
+	}
+	return (u.Scheme == "http" || u.Scheme == "https") && u.Host != "" && u.Hostname() != "" && u.User == nil && !strings.Contains(u.Host, "*")
+}
+
 func setPageSecurityHeaders(w http.ResponseWriter, r *http.Request, plugins []*plugin.Plugin) {
 	c := config.GetInstance()
 
@@ -609,6 +666,16 @@ func setPageSecurityHeaders(w http.ResponseWriter, r *http.Request, plugins []*p
 		}
 
 		connectSrcSlice = append(connectSrcSlice, ui.CSP.ConnectSrc...)
+
+		// only read plugin settings if the plugin opted in to the csp_ prefix
+		if ui.CSPSettings {
+			if settings := c.GetPluginConfiguration(plugin.ID); settings != nil {
+				valid, skipped := cspConnectSrcFromSettings(settings)
+				connectSrcSlice = append(connectSrcSlice, valid...)
+				warnInvalidCSPSettings(plugin.ID, skipped)
+			}
+		}
+
 		scriptSrcSlice = append(scriptSrcSlice, ui.CSP.ScriptSrc...)
 		styleSrcSlice = append(styleSrcSlice, ui.CSP.StyleSrc...)
 	}

internal/api/server_test.go

@@ -0,0 +1,148 @@
+package api
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stashapp/stash/internal/manager/config"
+	"github.com/stashapp/stash/pkg/plugin"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCspConnectSrcFromSettings(t *testing.T) {
+	for _, tt := range []struct {
+		name     string
+		settings map[string]interface{}
+		valid    []string
+		skipped  map[string]string
+	}{
+		{
+			name:     "no csp keys",
+			settings: map[string]interface{}{"foo": "bar", "other_setting": "https://x.com"},
+		},
+		{
+			name:     "valid https url",
+			settings: map[string]interface{}{"csp_x": "https://api.example.com"},
+			valid:    []string{"https://api.example.com"},
+		},
+		{
+			name:     "valid http url",
+			settings: map[string]interface{}{"csp_x": "http://localhost:7860"},
+			valid:    []string{"http://localhost:7860"},
+		},
+		{
+			name:     "disallowed scheme",
+			settings: map[string]interface{}{"csp_x": "ftp://example.com"},
+			skipped:  map[string]string{"csp_x": "ftp://example.com"},
+		},
+		{
+			name:     "not a url",
+			settings: map[string]interface{}{"csp_x": "not a url"},
+			skipped:  map[string]string{"csp_x": "not a url"},
+		},
+		{
+			name:     "non string value",
+			settings: map[string]interface{}{"csp_x": 123},
+			skipped:  map[string]string{"csp_x": "123"},
+		},
+		{
+			name:     "wildcard host",
+			settings: map[string]interface{}{"csp_x": "http://*:7860"},
+			skipped:  map[string]string{"csp_x": "http://*:7860"},
+		},
+		{
+			name:     "csp directive breakout via semicolon in path",
+			settings: map[string]interface{}{"csp_x": "https://evil.com/; script-src 'none'"},
+			skipped:  map[string]string{"csp_x": "https://evil.com/; script-src 'none'"},
+		},
+		{
+			name:     "whitespace in path",
+			settings: map[string]interface{}{"csp_x": "https://evil.com/a b"},
+			skipped:  map[string]string{"csp_x": "https://evil.com/a b"},
+		},
+		{
+			name:     "comma in url",
+			settings: map[string]interface{}{"csp_x": "https://evil.com/a,b"},
+			skipped:  map[string]string{"csp_x": "https://evil.com/a,b"},
+		},
+		{
+			name:     "degenerate host port only",
+			settings: map[string]interface{}{"csp_x": "https://:7860"},
+			skipped:  map[string]string{"csp_x": "https://:7860"},
+		},
+		{
+			name:     "userinfo in url",
+			settings: map[string]interface{}{"csp_x": "https://user@attacker.com"},
+			skipped:  map[string]string{"csp_x": "https://user@attacker.com"},
+		},
+		{
+			name:     "empty string",
+			settings: map[string]interface{}{"csp_x": ""},
+			skipped:  map[string]string{"csp_x": ""},
+		},
+		{
+			name:     "mixed valid and invalid",
+			settings: map[string]interface{}{"csp_a": "https://api.example.com", "csp_b": "javascript:alert(1)", "csp_c": "http://localhost:7860", "other": "ignored"},
+			valid:    []string{"http://localhost:7860", "https://api.example.com"},
+			skipped:  map[string]string{"csp_b": "javascript:alert(1)"},
+		},
+	} {
+		t.Run(tt.name, func(t *testing.T) {
+			valid, skipped := cspConnectSrcFromSettings(tt.settings)
+			// valid is sorted, so the emitted header is stable between requests
+			assert.Equal(t, tt.valid, valid)
+			assert.Equal(t, tt.skipped, skipped)
+		})
+	}
+}
+
+// connectSrc returns the connect-src directive of the CSP header emitted for a
+// page request, given the supplied plugins and stored plugin configuration.
+func connectSrc(t *testing.T, plugins []*plugin.Plugin, pluginConfig map[string]interface{}) string {
+	t.Helper()
+
+	c := config.InitializeEmpty()
+	for _, p := range plugins {
+		c.SetPluginConfiguration(p.ID, pluginConfig)
+	}
+
+	w := httptest.NewRecorder()
+	setPageSecurityHeaders(w, httptest.NewRequest(http.MethodGet, "/", nil), plugins)
+
+	return w.Header().Get("Content-Security-Policy")
+}
+
+func TestSetPageSecurityHeadersCSPSettings(t *testing.T) {
+	settings := map[string]interface{}{
+		"csp_endpoint": "https://api.example.com",
+		"csp_bad":      "http://*:7860",
+		"apiKey":       "secret",
+	}
+
+	pluginWith := func(cspSettings bool) []*plugin.Plugin {
+		return []*plugin.Plugin{{
+			ID:      "test-plugin",
+			Enabled: true,
+			UI:      plugin.PluginUI{CSPSettings: cspSettings},
+		}}
+	}
+
+	t.Run("opted in", func(t *testing.T) {
+		csp := connectSrc(t, pluginWith(true), settings)
+		assert.Contains(t, csp, "https://api.example.com")
+		assert.NotContains(t, csp, "http://*:7860")
+		assert.NotContains(t, csp, "secret")
+	})
+
+	t.Run("not opted in", func(t *testing.T) {
+		csp := connectSrc(t, pluginWith(false), settings)
+		assert.NotContains(t, csp, "https://api.example.com")
+	})
+
+	t.Run("disabled plugin", func(t *testing.T) {
+		plugins := pluginWith(true)
+		plugins[0].Enabled = false
+		assert.NotContains(t, connectSrc(t, plugins, settings), "https://api.example.com")
+	})
+}

pkg/plugin/config.go

@@ -82,6 +82,13 @@ type UIConfig struct {
 	// Content Security Policy configuration for the plugin.
 	CSP PluginCSP `yaml:"csp"`
 
+	// CSPSettings enables the csp_ plugin setting prefix for this plugin.
+	// When true, any plugin setting whose key starts with "csp_" and whose
+	// value is a valid http/https URL will be added to the connect-src
+	// CSP directive. This is an opt-in mechanism to prevent accidental
+	// namespace collisions with non-CSP settings.
+	CSPSettings bool `yaml:"csp-settings"`
+
 	// Javascript files that will be injected into the stash UI.
 	// These may be URLs or paths to files relative to the plugin configuration file.
 	Javascript []string `yaml:"javascript"`
@@ -260,6 +267,7 @@ func (c Config) toPlugin() *Plugin {
 			Javascript:     c.UI.getJavascriptFiles(c),
 			CSS:            c.UI.getCSSFiles(c),
 			CSP:            c.UI.CSP,
+			CSPSettings:    c.UI.CSPSettings,
 			Assets:         c.UI.Assets,
 		},
 		Settings:   c.getPluginSettings(),

pkg/plugin/plugins.go

@@ -53,6 +53,11 @@ type PluginUI struct {
 	// Content Security Policy configuration for the plugin.
 	CSP PluginCSP `json:"csp"`
 
+	// CSPSettings indicates whether the plugin has opted in to the csp_
+	// setting prefix mechanism. When true, settings with keys starting
+	// with "csp_" are treated as connect-src URLs for the CSP header.
+	CSPSettings bool `json:"csp_settings"`
+
 	// External Javascript files that will be injected into the stash UI.
 	ExternalScript []string `json:"external_script"`
 

ui/v2.5/src/docs/en/Manual/Plugins.md

@@ -103,6 +103,9 @@ ui:
     connect-src:
       - http://alloweddomain.com
 
+  # enable csp_ setting prefix for dynamic connect-src sources
+  csp-settings: true
+
 # map of setting names to be displayed in the plugins page in the UI
 settings:
   # internal name
@@ -130,6 +133,12 @@ The `exec`, `interface`, `errLog` and `tasks` fields are used only for plugins w
 
 The `settings` field is used to display plugin settings on the plugins page. Plugin settings can also be set using the graphql mutation `configurePlugin` - the settings set this way do _not_ need to be specified in the `settings` field unless they are to be displayed in the stock plugin settings UI.
 
+Settings whose key begins with `csp_` and whose value is a valid, concrete `http` or `https` URL are automatically added to the plugin's `connect-src` content security policy on the next page load. This is useful for plugins with a user-configurable backend endpoint: users can set the exact host in the plugin settings UI (or via `configurePlugin`) without editing the plugin configuration file, and the value survives plugin updates because it is stored in Stash's configuration rather than the plugin files.
+
+**This feature is opt-in.** The plugin must set `csp-settings: true` in its `ui` section (see below) to enable the `csp_` setting prefix. This prevents accidental namespace collisions with non-CSP settings.
+
+Only values that are valid `http`/`https` URLs with a host are accepted. Wildcard hosts, URLs containing whitespace, commas, or `;` characters, and other invalid values are ignored and logged, so a misconfigured setting cannot weaken or corrupt the page content security policy.
+
 ### UI configuration
 
 The `css` and `javascript` field values may be relative paths to the plugin configuration file, or
@@ -156,7 +165,10 @@ Mappings that try to go outside of the directory containing the plugin configura
 ignored.
 
 The `csp` field contains overrides to the content security policies. The URLs in `script-src`,
-`style-src` and `connect-src` will be added to the applicable content security policy.
+`style-src` and `connect-src` will be added to the applicable content security policy. In addition
+to the URLs listed here, if `csp-settings: true` is set in the `ui` section, any setting whose key
+begins with `csp_` and whose value is a valid `http`/`https` URL is also added to the plugin's
+`connect-src` policy (see the `settings` section above).
 
 See [External Plugins](/help/ExternalPlugins.md) for details for making plugins with external tasks.