fix(coderd): backport frame-ancestors CSP fixes to 2.31 (#24474, #24529) (#24807)

Cherry-pick backport of #24474 and #24529 to `release/2.31`.

- #24474: fix(coderd): add frame-ancestors CSP directive to prevent
clickjacking
- #24529: fix(coderd): omit frame-ancestors CSP for embed routes

Both commits cherry-picked cleanly with no conflicts.

> Generated by Coder Agents

coderd/coderd.go

@@ -1783,29 +1783,50 @@ 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 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{}
+	}
+	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

coderd/httpmw/csp.go

@@ -142,6 +142,22 @@ 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).
+			//
+			// 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
 			for src, vals := range cspSrcs {
 				_, _ = fmt.Fprintf(&csp, "%s %s; ", src, strings.Join(vals, " "))

coderd/httpmw/csp_test.go

@@ -12,6 +12,63 @@ 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: {"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 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) {
 	t.Parallel()