From 411ed21059d3c7dd23ea6d66543e3eb48e095c9c Mon Sep 17 00:00:00 2001 From: Jakub Domeracki Date: Mon, 20 Apr 2026 15:38:52 +0200 Subject: [PATCH] fix(coderd): omit frame-ancestors CSP for embed routes (#24529) --- coderd/coderd.go | 12 ++++++++---- coderd/httpmw/csp.go | 11 ++++++++++- coderd/httpmw/csp_test.go | 22 ++++++++++++++++++++-- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index b5a7ed6c84..e46df91cc2 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -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))) diff --git a/coderd/httpmw/csp.go b/coderd/httpmw/csp.go index a42567e342..1395d9ccdb 100644 --- a/coderd/httpmw/csp.go +++ b/coderd/httpmw/csp.go @@ -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 diff --git a/coderd/httpmw/csp_test.go b/coderd/httpmw/csp_test.go index 8af2755514..105abd0df1 100644 --- a/coderd/httpmw/csp_test.go +++ b/coderd/httpmw/csp_test.go @@ -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) {