From c87106f2971a5c4eeed265eb4a7113cc9bf20ed1 Mon Sep 17 00:00:00 2001 From: cc1234475 Date: Fri, 14 Aug 2026 23:44:23 +0100 Subject: [PATCH 1/6] feat: allow plugin settings to add connect-src CSP sources --- internal/api/server.go | 38 ++++++++++++++++++++ internal/api/server_test.go | 69 +++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 internal/api/server_test.go diff --git a/internal/api/server.go b/internal/api/server.go index c29cd2a64e..9637c7acd2 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -9,6 +9,7 @@ import ( "io" "io/fs" "net/http" + "net/url" "os" "path" "path/filepath" @@ -552,6 +553,34 @@ 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, skippedKeys []string) { + for k, v := range settings { + if !strings.HasPrefix(k, cspSettingPrefix) { + continue + } + s, ok := v.(string) + if !ok || !isValidConnectSrcURL(s) { + skippedKeys = append(skippedKeys, k) + continue + } + valid = append(valid, s) + } + return valid, skippedKeys +} + +func isValidConnectSrcURL(s string) bool { + u, err := url.Parse(s) + if err != nil { + return false + } + return (u.Scheme == "http" || u.Scheme == "https") && u.Host != "" && !strings.Contains(u.Host, "*") +} + func setPageSecurityHeaders(w http.ResponseWriter, r *http.Request, plugins []*plugin.Plugin) { c := config.GetInstance() @@ -609,6 +638,15 @@ func setPageSecurityHeaders(w http.ResponseWriter, r *http.Request, plugins []*p } connectSrcSlice = append(connectSrcSlice, ui.CSP.ConnectSrc...) + + if settings := config.GetInstance().GetPluginConfiguration(plugin.ID); settings != nil { + valid, skippedKeys := cspConnectSrcFromSettings(settings) + connectSrcSlice = append(connectSrcSlice, valid...) + for _, key := range skippedKeys { + logger.Debugf("skipping invalid csp_ setting %q for plugin %q", key, plugin.ID) + } + } + scriptSrcSlice = append(scriptSrcSlice, ui.CSP.ScriptSrc...) styleSrcSlice = append(styleSrcSlice, ui.CSP.StyleSrc...) } diff --git a/internal/api/server_test.go b/internal/api/server_test.go new file mode 100644 index 0000000000..7c1b80b01a --- /dev/null +++ b/internal/api/server_test.go @@ -0,0 +1,69 @@ +package api + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestCspConnectSrcFromSettings(t *testing.T) { + for _, tt := range []struct { + name string + settings map[string]interface{} + valid []string + skippedKeys []string + }{ + { + name: "no csp keys", + settings: map[string]interface{}{"foo": "bar", "other_setting": "https://x.com"}, + valid: nil, + }, + { + 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"}, + skippedKeys: []string{"csp_x"}, + }, + { + name: "not a url", + settings: map[string]interface{}{"csp_x": "not a url"}, + skippedKeys: []string{"csp_x"}, + }, + { + name: "non string value", + settings: map[string]interface{}{"csp_x": 123}, + skippedKeys: []string{"csp_x"}, + }, + { + name: "wildcard host", + settings: map[string]interface{}{"csp_x": "http://*:7860"}, + skippedKeys: []string{"csp_x"}, + }, + { + name: "empty string", + settings: map[string]interface{}{"csp_x": ""}, + skippedKeys: []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{"https://api.example.com", "http://localhost:7860"}, + skippedKeys: []string{"csp_b"}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + valid, skippedKeys := cspConnectSrcFromSettings(tt.settings) + assert.ElementsMatch(t, tt.valid, valid) + assert.ElementsMatch(t, tt.skippedKeys, skippedKeys) + }) + } +} From 119c512d63f32890c7e6ff292bf97ade9586a3d0 Mon Sep 17 00:00:00 2001 From: cc1234475 Date: Fri, 14 Aug 2026 23:59:03 +0100 Subject: [PATCH 2/6] fix: reject CSP-breaking characters in plugin csp_ setting URLs --- internal/api/server.go | 7 +++++-- internal/api/server_test.go | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/internal/api/server.go b/internal/api/server.go index 9637c7acd2..35681f3752 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -574,11 +574,14 @@ func cspConnectSrcFromSettings(settings map[string]interface{}) (valid []string, } 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 != "" && !strings.Contains(u.Host, "*") + 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) { @@ -639,7 +642,7 @@ func setPageSecurityHeaders(w http.ResponseWriter, r *http.Request, plugins []*p connectSrcSlice = append(connectSrcSlice, ui.CSP.ConnectSrc...) - if settings := config.GetInstance().GetPluginConfiguration(plugin.ID); settings != nil { + if settings := c.GetPluginConfiguration(plugin.ID); settings != nil { valid, skippedKeys := cspConnectSrcFromSettings(settings) connectSrcSlice = append(connectSrcSlice, valid...) for _, key := range skippedKeys { diff --git a/internal/api/server_test.go b/internal/api/server_test.go index 7c1b80b01a..125a39d1d3 100644 --- a/internal/api/server_test.go +++ b/internal/api/server_test.go @@ -48,6 +48,26 @@ func TestCspConnectSrcFromSettings(t *testing.T) { settings: map[string]interface{}{"csp_x": "http://*:7860"}, skippedKeys: []string{"csp_x"}, }, + { + name: "csp directive breakout via semicolon in path", + settings: map[string]interface{}{"csp_x": "https://evil.com/; script-src 'none'"}, + skippedKeys: []string{"csp_x"}, + }, + { + name: "whitespace in path", + settings: map[string]interface{}{"csp_x": "https://evil.com/a b"}, + skippedKeys: []string{"csp_x"}, + }, + { + name: "degenerate host port only", + settings: map[string]interface{}{"csp_x": "https://:7860"}, + skippedKeys: []string{"csp_x"}, + }, + { + name: "userinfo in url", + settings: map[string]interface{}{"csp_x": "https://user@attacker.com"}, + skippedKeys: []string{"csp_x"}, + }, { name: "empty string", settings: map[string]interface{}{"csp_x": ""}, From c3ebc8528646440261be5069fc186f9fd67123f2 Mon Sep 17 00:00:00 2001 From: cc1234475 Date: Sat, 15 Aug 2026 11:09:03 +0100 Subject: [PATCH 3/6] docs: document csp_ plugin settings as connect-src sources --- ui/v2.5/src/docs/en/Manual/Plugins.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/ui/v2.5/src/docs/en/Manual/Plugins.md b/ui/v2.5/src/docs/en/Manual/Plugins.md index 6a13487bcf..d8062a0f1c 100644 --- a/ui/v2.5/src/docs/en/Manual/Plugins.md +++ b/ui/v2.5/src/docs/en/Manual/Plugins.md @@ -130,6 +130,10 @@ 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. + +Only values that are valid `http`/`https` URLs with a host are accepted. Wildcard hosts, URLs containing whitespace 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 +160,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, 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. From c77bef4bbee10d2fe1c006ee4bb8eef6693ac9bb Mon Sep 17 00:00:00 2001 From: cc1234475 Date: Sat, 15 Aug 2026 11:16:45 +0100 Subject: [PATCH 4/6] style: gofmt server_test.go --- internal/api/server_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/api/server_test.go b/internal/api/server_test.go index 125a39d1d3..88994b1667 100644 --- a/internal/api/server_test.go +++ b/internal/api/server_test.go @@ -74,9 +74,9 @@ func TestCspConnectSrcFromSettings(t *testing.T) { skippedKeys: []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{"https://api.example.com", "http://localhost:7860"}, + 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{"https://api.example.com", "http://localhost:7860"}, skippedKeys: []string{"csp_b"}, }, } { From cf532bfea4ab305105c7fb1bd8d5db9c787d84e2 Mon Sep 17 00:00:00 2001 From: cc1234475 Date: Mon, 17 Aug 2026 23:28:57 +0100 Subject: [PATCH 5/6] fix: address PR #7166 review comments - Reject commas in connect-src URLs to prevent CSP header breakage - Change Debugf to Warnf for invalid csp_ plugin settings - Add csp-settings opt-in flag to prevent namespace collisions: plugins must set csp-settings: true in their ui section to enable the csp_ setting prefix for dynamic connect-src sources --- internal/api/server.go | 6 ++--- internal/api/server_test.go | 36 +++++++++++++++++++++++++++ pkg/plugin/config.go | 8 ++++++ pkg/plugin/plugins.go | 5 ++++ ui/v2.5/src/docs/en/Manual/Plugins.md | 13 +++++++--- 5 files changed, 61 insertions(+), 7 deletions(-) diff --git a/internal/api/server.go b/internal/api/server.go index 35681f3752..58a0094ae3 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -574,7 +574,7 @@ func cspConnectSrcFromSettings(settings map[string]interface{}) (valid []string, } func isValidConnectSrcURL(s string) bool { - if strings.ContainsAny(s, " \t\r\n;\"'") { + if strings.ContainsAny(s, " ,\t\r\n;\"'") { return false } u, err := url.Parse(s) @@ -642,11 +642,11 @@ func setPageSecurityHeaders(w http.ResponseWriter, r *http.Request, plugins []*p connectSrcSlice = append(connectSrcSlice, ui.CSP.ConnectSrc...) - if settings := c.GetPluginConfiguration(plugin.ID); settings != nil { + if settings := c.GetPluginConfiguration(plugin.ID); settings != nil && ui.CSPSettings { valid, skippedKeys := cspConnectSrcFromSettings(settings) connectSrcSlice = append(connectSrcSlice, valid...) for _, key := range skippedKeys { - logger.Debugf("skipping invalid csp_ setting %q for plugin %q", key, plugin.ID) + logger.Warnf("skipping invalid csp_ setting %q for plugin %q", key, plugin.ID) } } diff --git a/internal/api/server_test.go b/internal/api/server_test.go index 88994b1667..58c1e769e2 100644 --- a/internal/api/server_test.go +++ b/internal/api/server_test.go @@ -3,6 +3,7 @@ package api import ( "testing" + "github.com/stashapp/stash/pkg/plugin" "github.com/stretchr/testify/assert" ) @@ -58,6 +59,11 @@ func TestCspConnectSrcFromSettings(t *testing.T) { settings: map[string]interface{}{"csp_x": "https://evil.com/a b"}, skippedKeys: []string{"csp_x"}, }, + { + name: "comma in url", + settings: map[string]interface{}{"csp_x": "https://evil.com/a,b"}, + skippedKeys: []string{"csp_x"}, + }, { name: "degenerate host port only", settings: map[string]interface{}{"csp_x": "https://:7860"}, @@ -87,3 +93,33 @@ func TestCspConnectSrcFromSettings(t *testing.T) { }) } } + +func TestSetPageSecurityHeaders_CSPSettingsOptIn(t *testing.T) { + // Plugin with CSPSettings enabled — csp_ settings should appear in connect-src + t.Run("opt-in enabled", func(t *testing.T) { + plugins := []*plugin.Plugin{ + { + Enabled: true, + UI: plugin.PluginUI{ + CSPSettings: true, + }, + }, + } + + assert.True(t, plugins[0].UI.CSPSettings) + }) + + // Plugin without CSPSettings — csp_ settings should NOT be scanned + t.Run("opt-in disabled", func(t *testing.T) { + plugins := []*plugin.Plugin{ + { + Enabled: true, + UI: plugin.PluginUI{ + CSPSettings: false, + }, + }, + } + + assert.False(t, plugins[0].UI.CSPSettings) + }) +} diff --git a/pkg/plugin/config.go b/pkg/plugin/config.go index 333997b7cf..a240ca9081 100644 --- a/pkg/plugin/config.go +++ b/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(), diff --git a/pkg/plugin/plugins.go b/pkg/plugin/plugins.go index 9671f89019..e3bbe15bf3 100644 --- a/pkg/plugin/plugins.go +++ b/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"` diff --git a/ui/v2.5/src/docs/en/Manual/Plugins.md b/ui/v2.5/src/docs/en/Manual/Plugins.md index d8062a0f1c..bb2e4428b6 100644 --- a/ui/v2.5/src/docs/en/Manual/Plugins.md +++ b/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 @@ -132,7 +135,9 @@ The `settings` field is used to display plugin settings on the plugins page. Plu 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. -Only values that are valid `http`/`https` URLs with a host are accepted. Wildcard hosts, URLs containing whitespace or `;` characters, and other invalid values are ignored and logged, so a misconfigured setting cannot weaken or corrupt the page content security policy. +**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 @@ -161,9 +166,9 @@ 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. In addition -to the URLs listed here, 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). +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. From 70ded0a059c3def54a826e3b9e57d62fac5e2b35 Mon Sep 17 00:00:00 2001 From: cc1234475 Date: Tue, 18 Aug 2026 07:44:38 +0100 Subject: [PATCH 6/6] fix: gate plugin config read on opt-in, dedupe CSP warnings, test real header - Only call GetPluginConfiguration when the plugin sets csp-settings: true, instead of reading it for every enabled plugin on every page request. - Sort the validated URLs so the emitted connect-src is stable between requests (settings is a map). - Warn once per invalid setting value instead of on every page request. - Replace the tautological opt-in test with one that calls setPageSecurityHeaders and asserts on the emitted CSP header. Co-Authored-By: Claude Opus 5 --- internal/api/server.go | 42 ++++++++-- internal/api/server_test.go | 153 +++++++++++++++++++++--------------- 2 files changed, 122 insertions(+), 73 deletions(-) diff --git a/internal/api/server.go b/internal/api/server.go index 58a0094ae3..369ba3dab9 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -14,8 +14,10 @@ import ( "path" "path/filepath" "runtime/debug" + "sort" "strconv" "strings" + "sync" "time" gqlHandler "github.com/99designs/gqlgen/graphql/handler" @@ -558,19 +560,42 @@ 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, skippedKeys []string) { +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) { - skippedKeys = append(skippedKeys, k) + if skipped == nil { + skipped = make(map[string]string) + } + skipped[k] = fmt.Sprintf("%v", v) continue } valid = append(valid, s) } - return valid, skippedKeys + + // 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 { @@ -642,11 +667,12 @@ func setPageSecurityHeaders(w http.ResponseWriter, r *http.Request, plugins []*p connectSrcSlice = append(connectSrcSlice, ui.CSP.ConnectSrc...) - if settings := c.GetPluginConfiguration(plugin.ID); settings != nil && ui.CSPSettings { - valid, skippedKeys := cspConnectSrcFromSettings(settings) - connectSrcSlice = append(connectSrcSlice, valid...) - for _, key := range skippedKeys { - logger.Warnf("skipping invalid csp_ setting %q for plugin %q", key, plugin.ID) + // 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) } } diff --git a/internal/api/server_test.go b/internal/api/server_test.go index 58c1e769e2..a97655f664 100644 --- a/internal/api/server_test.go +++ b/internal/api/server_test.go @@ -1,23 +1,25 @@ 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 - skippedKeys []string + 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"}, - valid: nil, }, { name: "valid https url", @@ -30,96 +32,117 @@ func TestCspConnectSrcFromSettings(t *testing.T) { valid: []string{"http://localhost:7860"}, }, { - name: "disallowed scheme", - settings: map[string]interface{}{"csp_x": "ftp://example.com"}, - skippedKeys: []string{"csp_x"}, + 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"}, - skippedKeys: []string{"csp_x"}, + 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}, - skippedKeys: []string{"csp_x"}, + 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"}, - skippedKeys: []string{"csp_x"}, + 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'"}, - skippedKeys: []string{"csp_x"}, + 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"}, - skippedKeys: []string{"csp_x"}, + 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"}, - skippedKeys: []string{"csp_x"}, + 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"}, - skippedKeys: []string{"csp_x"}, + 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"}, - skippedKeys: []string{"csp_x"}, + 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": ""}, - skippedKeys: []string{"csp_x"}, + 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{"https://api.example.com", "http://localhost:7860"}, - skippedKeys: []string{"csp_b"}, + 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, skippedKeys := cspConnectSrcFromSettings(tt.settings) - assert.ElementsMatch(t, tt.valid, valid) - assert.ElementsMatch(t, tt.skippedKeys, skippedKeys) + 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) }) } } -func TestSetPageSecurityHeaders_CSPSettingsOptIn(t *testing.T) { - // Plugin with CSPSettings enabled — csp_ settings should appear in connect-src - t.Run("opt-in enabled", func(t *testing.T) { - plugins := []*plugin.Plugin{ - { - Enabled: true, - UI: plugin.PluginUI{ - CSPSettings: true, - }, - }, - } +// 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() - assert.True(t, plugins[0].UI.CSPSettings) + 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") }) - // Plugin without CSPSettings — csp_ settings should NOT be scanned - t.Run("opt-in disabled", func(t *testing.T) { - plugins := []*plugin.Plugin{ - { - Enabled: true, - UI: plugin.PluginUI{ - CSPSettings: false, - }, - }, - } + t.Run("not opted in", func(t *testing.T) { + csp := connectSrc(t, pluginWith(false), settings) + assert.NotContains(t, csp, "https://api.example.com") + }) - assert.False(t, plugins[0].UI.CSPSettings) + 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") }) }