Allow plugin settings to add connect-src CSP sources

[cc1234475]

Description

Adds a mechanism for plugins to contribute validated connect-src entries to Stash's page Content Security Policy via plugin settings, rather than only through the static ui.csp.connect-src in the plugin yml.

Any plugin setting whose key starts with csp_ and whose value is a valid concrete http/https URL is appended to the page's connect-src directive (per plugin, only for enabled plugins). This lets plugins that talk to a user-configurable backend ship a narrow default and have users set their exact backend endpoint in the plugin settings UI, applied automatically on the next page load — no hand-editing of plugin files, and the value survives plugin updates because it's stored in Stash's config rather than the shipped files.

Currently the static ui.csp.connect-src forces plugins with user-configurable backends to either ship a broad wildcard (e.g. http://*:7860) — poor security policy — or require users to edit the plugin yml, which is overwritten on every update.

Implementation details:

• internal/api/server.go: new pure helper cspConnectSrcFromSettings(settings) returns validated connect-src URLs plus keys skipped as invalid, and isValidConnectSrcURL(s) validates strictly via net/url. In setPageSecurityHeaders, inside the existing per-plugin loop, GetPluginConfiguration(plugin.ID) is read and validated csp_* values are appended to connect-src. Invalid values are skipped with a debug log (key + plugin ID).
• internal/api/server_test.go: table-driven tests covering valid URLs, disallowed schemes, non-URLs, non-string values, wildcard hosts, CSP directive-breakout attempts (whitespace/;/quotes), degenerate hosts, userinfo, empty strings, and mixed valid/invalid.

Validation intentionally rejects: anything other than concrete http/https URLs with a host, wildcards (http://*:7860), non-string values, empty strings, values containing CSP meta-characters, degenerate hosts, and URLs with userinfo — so a misconfigured or hostile setting can never corrupt or disable the CSP header.

pkg/plugin is unchanged; only connect-src is affected (not script-src/style-src). No reload or restart is needed — the header is rebuilt per request.

Related Issue

Closes #7165

Testing

Unit tests: go test ./internal/api/ -run TestCspConnectSrcFromSettings — 14/14 pass. Full go build ./... succeeds.

Manual verification (local instance, plugin with settings: csp_apiEndpoint):

1. Built the branch and ran a local Stash instance with a test plugin defining a csp_ setting.
2. Confirmed the served page Content-Security-Policy connect-src header initially excludes the setting's URL.
3. Set the setting to https://api.example.com via the configurePlugin GraphQL mutation — the URL appeared in the served connect-src header on the next request, without reload or restart.
4. Set the setting to a CSP-breakout value (https://evil.com/; script-src 'none') — it was rejected, and the header remained valid and unchanged.
5. Set the setting to a wildcard (http://*:7860) — rejected, not added to the header.
6. Confirmed the value persists in Stash's config.yml under the plugin ID.

Screenshots

No UI changes; this is a server-side CSP header change, so there are no screenshots.

Checklist

• I have read and understood the [Contributing](https://github.com/stashapp/stash/blob/develop/docs/CONTRIBUTING.md) document.
• I have read and understood the [AI Usage Policy](https://github.com/stashapp/stash/blob/develop/docs/AI_POLICY.md) document.
• I have made corresponding changes to the documentation (if applicable).

AI Usage Disclosure

• I have used AI tools to assist with this pull request, and I have disclosed the tools and how I used them below.

I used an AI coding assistant (OpenCode with an Deepseek v4 Flash) to help generate and refine the implementation and the accompanying tests. I have reviewed and understood every line and design decision, performed the manual testing described above, and take full responsibility for the change and its AGPL licensing.

Additional Context

This addresses a design gap where plugins with user-configurable backends cannot express per-user connect-src entries, forcing either broad wildcards or manual yml edits that are lost on plugin update. A follow-up would be for affected plugins (e.g. a face-recognition userscript) to adopt this mechanism and drop their wildcard entries.

[Gykes]

I will preface this by saying I don't have a massive amount of knowledge in this area. Feel free to push back and defend any decisions you made.

[Gykes]

Are commas filtered here? Could that potentially break headers if someone was misconfigured?

[cc1234475]

Good catch. Added , to the blocked character list in isValidConnectSrcURL. Commas are valid in URLs but could cause confusion if someone uses them as CSP list separators. No harm in rejecting them defensively.

Resolved in the latest commit.

[Gykes]

So, I think the prefix you went with will collide with the generic plugin-settings namespace. I think this would inject or degub on every page load and there's no opt in. Not sure the best way to handle this tbh.

[cc1234475]

Agreed, this was a real concern. Added an opt-in mechanism: plugins must now set csp-settings: true in their ui section to enable the csp_ setting prefix. Without it, csp_-prefixed settings are ignored for CSP purposes.

This prevents accidental namespace collisions. The flag is plumbed through UIConfig (yaml: csp-settings) and PluginUI (json: csp_settings), and the cspConnectSrcFromSettings call is gated on ui.CSPSettings.

Also updated Plugins.md to document the opt-in requirement.

[Gykes]

Would this be better as a warn?

[cc1234475]

Done. Changed Debugf to Warnf. A skipped CSP setting means a plugin author misconfigured something that silently breaks functionality — worth surfacing at warn level.

[Gykes]

Another static review

[Gykes]

This seems to run unconditionally so I think GetPluginConfiguration is called for every enabled plugin on every page load, regardless of the opt in. I think the opt-in flag should have a gate of some kind.

Here's a quick rough example not an actual fix:

if ui.CSPSettings {
       if settings := c.GetPluginConfiguration(plugin.ID); settings != nil {
          valid, skippedKeys := cspConnectSrcFromSettings(settings)
          ...
      }
  }

[cc1234475]

Good catch, you were right. The old settings != nil && ui.CSPSettings still called GetPluginConfiguration (and took its RLock) for every enabled plugin on every page request, and only then discarded the result. Gated it exactly as you wrote it in 70ded0a0:

// only read plugin settings if the plugin opted in to the csp_ prefix
if ui.CSPSettings {
    if settings := c.GetPluginConfiguration(plugin.ID); settings != nil {
        ...
    }
}

While in there I fixed two related things that your comment made obvious:

• The Warnf you asked for earlier fired on every page request for a misconfigured setting, which is log spam. It is now deduplicated per plugin ID + setting key, so it warns once and re-warns only if the user changes the setting to a different invalid value. The log records the key only, never the value.
• settings is a map, so the emitted connect-src ordering varied between requests. The validated URLs are now sorted, making the header stable.

[Gykes]

Unless im misunderstanding this test then it's not really testing anything. How it looks is you are setting true on the flag and then just calling that to validate that it's true. I think it would be better to call setPageSecurityHeaders and check if the csp_ url shows up in the header.

[cc1234475]

You understood it correctly, and you were right: that test asserted on the struct literal it had just built, not on any behaviour. Replaced in 70ded0a0 with one that calls setPageSecurityHeaders through httptest and asserts on the emitted Content-Security-Policy header:

func connectSrc(t *testing.T, plugins []*plugin.Plugin, pluginConfig map[string]interface{}) string {
    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")
}

Three cases, all against the real header:

• opted in: the valid csp_ URL is present, the wildcard value is not, and the non-csp_ setting value is not.
• not opted in: the valid csp_ URL is absent.
• disabled plugin: the valid csp_ URL is absent.

go test ./... passes (884 tests, 66 packages).

[cc1234475]

Pushed 70ded0a0, which addresses both outstanding review comments (the GetPluginConfiguration gate and the tautological opt-in test), plus two issues that fell out of them: the warning no longer repeats on every page request, and the emitted connect-src is now deterministically ordered.

One thing worth stating explicitly rather than leaving for a reviewer to find, since it is a deliberate design choice:

configurePlugin does not validate keys against the plugin's declared settings. It writes whatever map it is given. So with the csp_ prefix, a csp_* key can be added at runtime that the plugin yml never declared. That is intentional and is most of the point of #7165: a user can point a plugin at their own backend without editing plugin files that an update would overwrite. The settings UI itself only renders declared settings, so this only applies to the GraphQL mutation path.

The trade-off: anything able to call configurePlugin can widen connect-src for the whole page, for any enabled plugin that has opted in. Validation constrains it to concrete http/https origins, so the header cannot be corrupted or a directive broken out of, but it is still an added destination the plugin author did not sanction.

The stricter alternative is an explicit allowlist in the yml, e.g. ui.csp.connect-src-settings: [apiEndpoint], so only named settings feed the policy. It costs nothing for the settings-UI flow (the author writes one static line either way, the user still picks the value), and it closes the runtime-key path. It does lose the ability to add a source the plugin author never anticipated.

Happy to switch to the allowlist if you prefer the tighter contract, it is a small change. Flagging the choice rather than making it silently.