mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
fix(coderd): omit frame-ancestors CSP for embed routes (#24529)
This commit is contained in:
+8
-4
@@ -2037,16 +2037,20 @@ func New(options *Options) *API {
|
||||
cspMW := httpmw.CSPHeaders(options.Telemetry.Enabled(), cspProxyHosts, additionalCSPHeaders)
|
||||
|
||||
// Embed routes (e.g. VS Code extension chat) are designed to be
|
||||
// loaded inside iframes, so they need a relaxed frame-ancestors
|
||||
// policy instead of the default 'self'. However, if the operator
|
||||
// explicitly configured frame-ancestors via CODER_ADDITIONAL_CSP_POLICY,
|
||||
// loaded inside iframes, so they must not include frame-ancestors
|
||||
// in their CSP. The CSP wildcard '*' only matches network schemes
|
||||
// (http, https, ws, wss) and cannot cover custom schemes like
|
||||
// vscode-webview://, so the only way to allow all embedders is
|
||||
// to omit the directive entirely. If the operator explicitly
|
||||
// configured frame-ancestors via CODER_ADDITIONAL_CSP_POLICY,
|
||||
// respect that setting.
|
||||
|
||||
embedCSPHeaders := make(map[httpmw.CSPFetchDirective][]string, len(additionalCSPHeaders))
|
||||
for k, v := range additionalCSPHeaders {
|
||||
embedCSPHeaders[k] = v
|
||||
}
|
||||
if _, ok := additionalCSPHeaders[httpmw.CSPFrameAncestors]; !ok {
|
||||
embedCSPHeaders[httpmw.CSPFrameAncestors] = []string{"*"}
|
||||
embedCSPHeaders[httpmw.CSPFrameAncestors] = []string{}
|
||||
}
|
||||
embedCSPMW := httpmw.CSPHeaders(options.Telemetry.Enabled(), cspProxyHosts, embedCSPHeaders)
|
||||
embedHandler := embedCSPMW(compressHandler(httpmw.HSTS(api.SiteHandler, options.StrictTransportSecurityCfg)))
|
||||
|
||||
+10
-1
@@ -145,8 +145,17 @@ func CSPHeaders(telemetry bool, proxyHosts func() []*proxyhealth.ProxyHost, stat
|
||||
// Default to 'self' to prevent clickjacking unless
|
||||
// explicitly overridden via staticAdditions (e.g. for
|
||||
// embeddable routes).
|
||||
if _, ok := cspSrcs[CSPFrameAncestors]; !ok {
|
||||
//
|
||||
// An explicit empty value means "omit frame-ancestors
|
||||
// entirely", which is needed for embed routes where
|
||||
// non-network-scheme parents (e.g. vscode-webview://)
|
||||
// must be able to frame the page. The CSP wildcard '*'
|
||||
// only matches network schemes (http, https, ws, wss)
|
||||
// so it cannot cover custom schemes.
|
||||
if vals, ok := cspSrcs[CSPFrameAncestors]; !ok {
|
||||
cspSrcs[CSPFrameAncestors] = []string{"'self'"}
|
||||
} else if len(vals) == 0 {
|
||||
delete(cspSrcs, CSPFrameAncestors)
|
||||
}
|
||||
|
||||
var csp strings.Builder
|
||||
|
||||
@@ -40,15 +40,33 @@ func TestCSPFrameAncestors(t *testing.T) {
|
||||
httpmw.CSPHeaders(false, func() []*proxyhealth.ProxyHost {
|
||||
return nil
|
||||
}, map[httpmw.CSPFetchDirective][]string{
|
||||
httpmw.CSPFrameAncestors: {"*"},
|
||||
httpmw.CSPFrameAncestors: {"https://example.com"},
|
||||
})(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
|
||||
rw.WriteHeader(http.StatusOK)
|
||||
})).ServeHTTP(rw, r)
|
||||
|
||||
csp := rw.Header().Get("Content-Security-Policy")
|
||||
require.Contains(t, csp, "frame-ancestors *")
|
||||
require.Contains(t, csp, "frame-ancestors https://example.com")
|
||||
require.NotContains(t, csp, "frame-ancestors 'self'")
|
||||
})
|
||||
|
||||
t.Run("OmitWhenEmpty", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
r := httptest.NewRequest(http.MethodGet, "/", nil)
|
||||
rw := httptest.NewRecorder()
|
||||
|
||||
httpmw.CSPHeaders(false, func() []*proxyhealth.ProxyHost {
|
||||
return nil
|
||||
}, map[httpmw.CSPFetchDirective][]string{
|
||||
httpmw.CSPFrameAncestors: {},
|
||||
})(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
|
||||
rw.WriteHeader(http.StatusOK)
|
||||
})).ServeHTTP(rw, r)
|
||||
|
||||
csp := rw.Header().Get("Content-Security-Policy")
|
||||
require.NotContains(t, csp, "frame-ancestors")
|
||||
})
|
||||
}
|
||||
|
||||
func TestCSP(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user