From bd05dea467690d6c0647bf819da52a5381501dc3 Mon Sep 17 00:00:00 2001 From: Jakub Domeracki Date: Mon, 20 Apr 2026 13:01:46 +0200 Subject: [PATCH 1/2] fix(coderd): add frame-ancestors CSP directive to prevent clickjacking (#24474) (cherry picked from commit 615be176b82070a729e51852ede41bc6f7151257) --- coderd/coderd.go | 57 +++++++++++++++++++++++++-------------- coderd/httpmw/csp.go | 7 +++++ coderd/httpmw/csp_test.go | 39 +++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 20 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index aaabf5873b555..a0709e43b697a 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1982,29 +1982,46 @@ func New(options *Options) *API { // Add CSP headers to all static assets and pages. CSP headers only affect // browsers, so these don't make sense on api routes. - cspMW := httpmw.CSPHeaders( - options.Telemetry.Enabled(), func() []*proxyhealth.ProxyHost { - if api.DeploymentValues.Dangerous.AllowAllCors { - // In this mode, allow all external requests. - return []*proxyhealth.ProxyHost{ - { - Host: "*", - AppHost: "*", - }, - } - } - // Always add the primary, since the app host may be on a sub-domain. - proxies := []*proxyhealth.ProxyHost{ + cspProxyHosts := func() []*proxyhealth.ProxyHost { + if api.DeploymentValues.Dangerous.AllowAllCors { + // In this mode, allow all external requests. + return []*proxyhealth.ProxyHost{ { - Host: api.AccessURL.Host, - AppHost: appurl.ConvertAppHostForCSP(api.AccessURL.Host, api.AppHostname), + Host: "*", + AppHost: "*", }, } - if f := api.WorkspaceProxyHostsFn.Load(); f != nil { - proxies = append(proxies, (*f)()...) - } - return proxies - }, additionalCSPHeaders) + } + // Always add the primary, since the app host may be on a sub-domain. + proxies := []*proxyhealth.ProxyHost{ + { + Host: api.AccessURL.Host, + AppHost: appurl.ConvertAppHostForCSP(api.AccessURL.Host, api.AppHostname), + }, + } + if f := api.WorkspaceProxyHostsFn.Load(); f != nil { + proxies = append(proxies, (*f)()...) + } + return proxies + } + 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, + // 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{"*"} + } + embedCSPMW := httpmw.CSPHeaders(options.Telemetry.Enabled(), cspProxyHosts, embedCSPHeaders) + embedHandler := embedCSPMW(compressHandler(httpmw.HSTS(api.SiteHandler, options.StrictTransportSecurityCfg))) + r.Get("/agents/{agentId}/embed", embedHandler.ServeHTTP) + r.Get("/agents/{agentId}/embed/*", embedHandler.ServeHTTP) // Static file handler must be wrapped with HSTS handler if the // StrictTransportSecurityAge is set. We only need to set this header on diff --git a/coderd/httpmw/csp.go b/coderd/httpmw/csp.go index f39781ad51b03..a42567e3429b8 100644 --- a/coderd/httpmw/csp.go +++ b/coderd/httpmw/csp.go @@ -142,6 +142,13 @@ func CSPHeaders(telemetry bool, proxyHosts func() []*proxyhealth.ProxyHost, stat cspSrcs.Append(directive, values...) } + // Default to 'self' to prevent clickjacking unless + // explicitly overridden via staticAdditions (e.g. for + // embeddable routes). + if _, ok := cspSrcs[CSPFrameAncestors]; !ok { + cspSrcs[CSPFrameAncestors] = []string{"'self'"} + } + var csp strings.Builder for src, vals := range cspSrcs { _, _ = fmt.Fprintf(&csp, "%s %s; ", src, strings.Join(vals, " ")) diff --git a/coderd/httpmw/csp_test.go b/coderd/httpmw/csp_test.go index ba88320e6fac9..8af2755514858 100644 --- a/coderd/httpmw/csp_test.go +++ b/coderd/httpmw/csp_test.go @@ -12,6 +12,45 @@ import ( "github.com/coder/coder/v2/coderd/proxyhealth" ) +func TestCSPFrameAncestors(t *testing.T) { + t.Parallel() + + t.Run("DefaultSelf", func(t *testing.T) { + t.Parallel() + + r := httptest.NewRequest(http.MethodGet, "/", nil) + rw := httptest.NewRecorder() + + httpmw.CSPHeaders(false, func() []*proxyhealth.ProxyHost { + return nil + }, nil)(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 'self'") + }) + + t.Run("OverrideViaStaticAdditions", 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.Contains(t, csp, "frame-ancestors *") + require.NotContains(t, csp, "frame-ancestors 'self'") + }) +} + func TestCSP(t *testing.T) { t.Parallel() From 2aa083012e01d08aeeee9a13eb8bed541a9ec114 Mon Sep 17 00:00:00 2001 From: Jakub Domeracki Date: Mon, 20 Apr 2026 15:38:52 +0200 Subject: [PATCH 2/2] fix(coderd): omit frame-ancestors CSP for embed routes (#24529) (cherry picked from commit 411ed21059d3c7dd23ea6d66543e3eb48e095c9c) --- 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 a0709e43b697a..7d7a6121432f7 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -2007,16 +2007,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 a42567e3429b8..1395d9ccdb705 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 8af2755514858..105abd0df18f1 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) {